7:33 a.m.
Implementation of artifact binding (JIRA KEYCLOAK-831)
Hello,
Last week I did a PoC implementation of the SAML artifact binding in a branch off keycloak
4.3.0.Final. The implementation can be seen here at
https://github.com/AlistairDoswald/keycloak/tree/projectathon (don't judge me too
harshly for the quality of the code if you look at it, I had about 2 days to have a
working implementation, which included finding out how that part of the protocol worked).
However, I now want to write a "correct" implementation against keycloak/master
and if possible I'd like some feedback/advice on my intended implementation.
1. General implementation
From the description in the SAML specification (see here section 3.6,
https://docs.oasis-open.org/security/saml/v2.0/saml-bindings-2.0-os.pdf), artifact binding
can be used for transmitting the request message, the response message or both.
Initially, I intend only to do the implementation for the response messages. If I'm
not mistaken, this means only for the Response and LogoutResponse messages. Would this be
considered a suitable implementation of the JIRA?
2. User interface
When a SP requests an artifact, it can do so by specifying HTTP-Artifact instead of
HTTP-POST or HTTP-redirect, and the process is then transparent with regard to the
configuration of the client. However, I believe that the client should have a "Force
artifact binding" binary slider and also a field to specify an artifact binding
address. In this manner, the artifact binding can be used in conjunction with the IdP
initiated login method.
Importing must also set the artifact binding address if it is present in the SP metadata.
3. IdP metadata
IdP metadata must contain at least one ArtifactResolutionService, I intend to have only
one, with its index set to 0 and isDefault=true, and the binding set to the same address
as the HTTP-POST (as for ECP)
4. Sending an artifact instead of the normal saml message
This is the section for which I have the greatest uncertainty with respect to a correct
implementation.
Broadly this means intercepting the output response, and sending a 302 redirect or a
POSTed form with the artifact instead. Considering the length of the artifact, I see no
reason to use a form, but should this be an option in the GUI?
More practically, this means generating the response, saving it in the cache, and sending
the redirect (or form) instead. I believe that the client's cache would be the best
place to save this information (through the AuthenticatedClientSessionModel to be
precise), but I'm not certain because it's the first time I'm seeking to store
some new information in the cache. The key would be the artifact, and the value in my view
should be the document, as that way we can create a complete signed/encrypted
ArtifactResponse containing the Response or LogoutResponse.
For the implementation details I'm not sure if it would be best to make the changes
directly in the SamlProtocol class, or to do something similar to the
SamlECPProfileService which overrides the methods of the SamlProtocol. For
SamlECPProfileService the current implementation makes sense, but for artifact binding I
fear there would be significant code duplication (of course, I could also do a mix with
some small modifications in the SamlProtocol class and a SamlArtifactProfileService, or
something similar).
For triggering this artifact workflow, it would either be if the AuthnRequest has a
ProtocolBinding set to urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Artifact, or if the
client has "force artifact binding" set to true.
5. Receiving an ArtifactResolve message
For this part, my current implementation seems correct to me: the soapBinding method in
class SamlService is modified to check the contents of the soap message arriving: if it is
an ArtifactResolve, the corresponding ArtifactResponse generated earlier is packaged in a
soap message and sent as a response. If not, the ECP profile is tried.
The key-ArtifactResponse pair is removed from the cache during this operation. I am,
however, not sure yet how the cache should handle purging of expired ArtifactResponse
messages that are never asked for.
6. Errors, logging and audit
Obviously, the error handling should work as described in the protocol, but also be logged
as such. I don't think there's any messages to log in INFO, but the DEBUG logs
should show the messages and allow an admin to easily put the entire sequence together.
Also, I don't think there's any need for any extra information in the audit logs.
7. Tests
Obviously, I'll have to add some tests for these functions, which should be:
- Standard unit tests for individual functions that can be separated from objects that
would otherwise have to be mocked
- Tests with arquillian to test the flow with artifact binding (sp initiated and idp
initiated), the options available in the GUI (extra field, forced) as well as the error
cases (i.e. asking twice for the same artifact, for an artifact that doesn't exist,
etc...).
If you have any comments (anything missing, things that should be implemented differently
in your view, etc...) feel free to let me know.
Best regards,
Alistair Doswald
6:13 a.m.
Hi Alistair,
thank you for your willingness to contribute!
However the ARTIFACT binding would need to be implemented in full, with a
sufficient test coverage. Partial implementation cannot be accepted. It
would also need some changes in the SAML code since currently it is
basically expecting either POST or REDIRECT. One of the implications is
that boolean has been widely used to discriminate the two while enum would
be more appropriate. Such places would need to be cleaned up first.
If you would like to do that, we could start with such refactorings once
the feature freeze phase [1] finishes.
Thank you
--Hynek
[1] http://lists.jboss.org/pipermail/keycloak-dev/2018-September/011263.html
On Fri, Sep 28, 2018 at 2:35 PM Doswald Alistair <alistair.doswald(a)elca.ch>
wrote:
Implementation of artifact binding (JIRA KEYCLOAK-831)
Hello,
Last week I did a PoC implementation of the SAML artifact binding in a
branch off keycloak 4.3.0.Final. The implementation can be seen here at
https://github.com/AlistairDoswald/keycloak/tree/projectathon (don't
judge me too harshly for the quality of the code if you look at it, I had
about 2 days to have a working implementation, which included finding out
how that part of the protocol worked).
However, I now want to write a "correct" implementation against
keycloak/master and if possible I'd like some feedback/advice on my
intended implementation.
1. General implementation
>From the description in the SAML specification (see here section 3.6,
https://docs.oasis-open.org/security/saml/v2.0/saml-bindings-2.0-os.pdf),
artifact binding can be used for transmitting the request message, the
response message or both.
Initially, I intend only to do the implementation for the response
messages. If I'm not mistaken, this means only for the Response and
LogoutResponse messages. Would this be considered a suitable implementation
of the JIRA?
2. User interface
When a SP requests an artifact, it can do so by specifying HTTP-Artifact
instead of HTTP-POST or HTTP-redirect, and the process is then transparent
with regard to the configuration of the client. However, I believe that the
client should have a "Force artifact binding" binary slider and also a
field to specify an artifact binding address. In this manner, the artifact
binding can be used in conjunction with the IdP initiated login method.
Importing must also set the artifact binding address if it is present in
the SP metadata.
3. IdP metadata
IdP metadata must contain at least one ArtifactResolutionService, I intend
to have only one, with its index set to 0 and isDefault=true, and the
binding set to the same address as the HTTP-POST (as for ECP)
4. Sending an artifact instead of the normal saml message
This is the section for which I have the greatest uncertainty with respect
to a correct implementation.
Broadly this means intercepting the output response, and sending a 302
redirect or a POSTed form with the artifact instead. Considering the length
of the artifact, I see no reason to use a form, but should this be an
option in the GUI?
More practically, this means generating the response, saving it in the
cache, and sending the redirect (or form) instead. I believe that the
client's cache would be the best place to save this information (through
the AuthenticatedClientSessionModel to be precise), but I'm not certain
because it's the first time I'm seeking to store some new information in
the cache. The key would be the artifact, and the value in my view should
be the document, as that way we can create a complete signed/encrypted
ArtifactResponse containing the Response or LogoutResponse.
For the implementation details I'm not sure if it would be best to make
the changes directly in the SamlProtocol class, or to do something similar
to the SamlECPProfileService which overrides the methods of the
SamlProtocol. For SamlECPProfileService the current implementation makes
sense, but for artifact binding I fear there would be significant code
duplication (of course, I could also do a mix with some small modifications
in the SamlProtocol class and a SamlArtifactProfileService, or something
similar).
For triggering this artifact workflow, it would either be if the
AuthnRequest has a ProtocolBinding set to
urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Artifact, or if the client has
"force artifact binding" set to true.
5. Receiving an ArtifactResolve message
For this part, my current implementation seems correct to me: the
soapBinding method in class SamlService is modified to check the contents
of the soap message arriving: if it is an ArtifactResolve, the
corresponding ArtifactResponse generated earlier is packaged in a soap
message and sent as a response. If not, the ECP profile is tried.
The key-ArtifactResponse pair is removed from the cache during this
operation. I am, however, not sure yet how the cache should handle purging
of expired ArtifactResponse messages that are never asked for.
6. Errors, logging and audit
Obviously, the error handling should work as described in the protocol,
but also be logged as such. I don't think there's any messages to log in
INFO, but the DEBUG logs should show the messages and allow an admin to
easily put the entire sequence together.
Also, I don't think there's any need for any extra information in the
audit logs.
7. Tests
Obviously, I'll have to add some tests for these functions, which should
be:
- Standard unit tests for individual functions that can be separated from
objects that would otherwise have to be mocked
- Tests with arquillian to test the flow with artifact binding (sp
initiated and idp initiated), the options available in the GUI (extra
field, forced) as well as the error cases (i.e. asking twice for the same
artifact, for an artifact that doesn't exist, etc...).
If you have any comments (anything missing, things that should be
implemented differently in your view, etc...) feel free to let me know.
Best regards,
Alistair Doswald
_______________________________________________
keycloak-dev mailing list
keycloak-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/keycloak-dev
10:26 a.m.
Hello,
I think that a full implementation of SAML’s artifact binding can be functionally divided
into three separate parts:
1. Answering a Redirect or POSTed form containing an artifact from an SP with an
ArtifactResolve, and then handling the contents of the received ArtifactResponse
2. Upon receiving an AuthnRequest, performing the login, but answering with an artifact
rather than saml Response. Then, upon receiving the ArtifactResolve from the SP, sending
the Reponse in an ArtifactResponse.
3. Upon receiving a Logout request, answering with an artifact. Then, upon receiving
the ArtrifactResolve, completing the logout, and sending the LogoutResponse in an
ArtifactResponse. This step is a bit complicated by the fact that there may be multiple
clients to which to send a logout for the SSO, and we must track which sessions are using
artifact binding.
Currently I have the part 2) completely written, commented and fully automatically tested.
To fully implement the JIRA KEYCLOAK-831, all 3 need to be implemented, but I’m already
going to do a pull request with the current code, to allow for the existing code to be
revised. From what you said, such a PR wouldn’t be accepted but from my point of view it
wouldn’t be a problem if it were, as it is already functional as-is.
The rest of the code will be coming, since Artifact Binding one of our required features
on the Cloudtrust project. I hope to have it ready within the next two weeks. If I can
submit within this timeframe, would it be in time before the feature freeze?
Best regards,
Alistair Doswald
From: Hynek Mlnarik <hmlnarik(a)redhat.com>
Sent: lundi 15 octobre 2018 13:14
To: Doswald Alistair <alistair.doswald(a)elca.ch>
Cc: keycloak-dev <keycloak-dev(a)lists.jboss.org>
Subject: Re: [keycloak-dev] Implementation of artifact binding (JIRA KEYCLOAK-831)
Hi Alistair,
thank you for your willingness to contribute!
However the ARTIFACT binding would need to be implemented in full, with a sufficient test
coverage. Partial implementation cannot be accepted. It would also need some changes in
the SAML code since currently it is basically expecting either POST or REDIRECT. One of
the implications is that boolean has been widely used to discriminate the two while enum
would be more appropriate. Such places would need to be cleaned up first.
If you would like to do that, we could start with such refactorings once the feature
freeze phase [1] finishes.
Thank you
--Hynek
[1] http://lists.jboss.org/pipermail/keycloak-dev/2018-September/011263.html
On Fri, Sep 28, 2018 at 2:35 PM Doswald Alistair
<alistair.doswald@elca.ch<mailto:alistair.doswald@elca.ch>> wrote:
Implementation of artifact binding (JIRA KEYCLOAK-831)
Hello,
Last week I did a PoC implementation of the SAML artifact binding in a branch off keycloak
4.3.0.Final. The implementation can be seen here at
https://github.com/AlistairDoswald/keycloak/tree/projectathon (don't judge me too
harshly for the quality of the code if you look at it, I had about 2 days to have a
working implementation, which included finding out how that part of the protocol
worked).
However, I now want to write a "correct" implementation against keycloak/master
and if possible I'd like some feedback/advice on my intended implementation.
1. General implementation
From the description in the SAML specification (see here section 3.6,
https://docs.oasis-open.org/security/saml/v2.0/saml-bindings-2.0-os.pdf), artifact binding
can be used for transmitting the request message, the response message or both.
Initially, I intend only to do the implementation for the response messages. If I'm
not mistaken, this means only for the Response and LogoutResponse messages. Would this be
considered a suitable implementation of the JIRA?
2. User interface
When a SP requests an artifact, it can do so by specifying HTTP-Artifact instead of
HTTP-POST or HTTP-redirect, and the process is then transparent with regard to the
configuration of the client. However, I believe that the client should have a "Force
artifact binding" binary slider and also a field to specify an artifact binding
address. In this manner, the artifact binding can be used in conjunction with the IdP
initiated login method.
Importing must also set the artifact binding address if it is present in the SP
metadata.
3. IdP metadata
IdP metadata must contain at least one ArtifactResolutionService, I intend to have only
one, with its index set to 0 and isDefault=true, and the binding set to the same address
as the HTTP-POST (as for ECP)
4. Sending an artifact instead of the normal saml message
This is the section for which I have the greatest uncertainty with respect to a correct
implementation.
Broadly this means intercepting the output response, and sending a 302 redirect or a
POSTed form with the artifact instead. Considering the length of the artifact, I see no
reason to use a form, but should this be an option in the GUI?
More practically, this means generating the response, saving it in the cache, and sending
the redirect (or form) instead. I believe that the client's cache would be the best
place to save this information (through the AuthenticatedClientSessionModel to be
precise), but I'm not certain because it's the first time I'm seeking to store
some new information in the cache. The key would be the artifact, and the value in my view
should be the document, as that way we can create a complete signed/encrypted
ArtifactResponse containing the Response or LogoutResponse.
For the implementation details I'm not sure if it would be best to make the changes
directly in the SamlProtocol class, or to do something similar to the
SamlECPProfileService which overrides the methods of the SamlProtocol. For
SamlECPProfileService the current implementation makes sense, but for artifact binding I
fear there would be significant code duplication (of course, I could also do a mix with
some small modifications in the SamlProtocol class and a SamlArtifactProfileService, or
something similar).
For triggering this artifact workflow, it would either be if the AuthnRequest has a
ProtocolBinding set to urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Artifact, or if the
client has "force artifact binding" set to true.
5. Receiving an ArtifactResolve message
For this part, my current implementation seems correct to me: the soapBinding method in
class SamlService is modified to check the contents of the soap message arriving: if it is
an ArtifactResolve, the corresponding ArtifactResponse generated earlier is packaged in a
soap message and sent as a response. If not, the ECP profile is tried.
The key-ArtifactResponse pair is removed from the cache during this operation. I am,
however, not sure yet how the cache should handle purging of expired ArtifactResponse
messages that are never asked for.
6. Errors, logging and audit
Obviously, the error handling should work as described in the protocol, but also be logged
as such. I don't think there's any messages to log in INFO, but the DEBUG logs
should show the messages and allow an admin to easily put the entire sequence together.
Also, I don't think there's any need for any extra information in the audit
logs.
7. Tests
Obviously, I'll have to add some tests for these functions, which should be:
- Standard unit tests for individual functions that can be separated from objects that
would otherwise have to be mocked
- Tests with arquillian to test the flow with artifact binding (sp initiated and idp
initiated), the options available in the GUI (extra field, forced) as well as the error
cases (i.e. asking twice for the same artifact, for an artifact that doesn't exist,
etc...).
If you have any comments (anything missing, things that should be implemented differently
in your view, etc...) feel free to let me know.
Best regards,
Alistair Doswald
_______________________________________________
keycloak-dev mailing list
keycloak-dev@lists.jboss.org<mailto:keycloak-dev@lists.jboss.org>
https://lists.jboss.org/mailman/listinfo/keycloak-dev
2256
days inactive
2281
days old
2 comments
2 participants
participants (2)
-
Doswald Alistair -
Hynek Mlnarik