Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorrect REDIRECT binding URL #91

Open
RokLenarcic opened this issue Feb 17, 2025 · 0 comments
Open

Incorrect REDIRECT binding URL #91

RokLenarcic opened this issue Feb 17, 2025 · 0 comments

Comments

@RokLenarcic
Copy link

The function saml/idp-redirect-response generates an incorrect URL. It signs the request XML element, but that is done for POST bindings, for REDIRECT bindings you are supposed to put signature into the URL itself.

See: spring-projects/spring-security#7711

The reason is that SAML 2 expects different signature for different bindings (POST or Redirect) - at least that's how I understand it.

If a POST binding is used the signature is embedded in the XML.

If a Redirect binding is used the signature is part of the URL query parameters.
(e.g. https://idp/?SAMLRequest=...&RelayState=...&SigAlg=...&Signature=...)

I am using Keycloak and it rejects the redirect URL for missing SigAlg parameter.

edpaget added a commit that referenced this issue Feb 20, 2025
Previously we could not generate correctly signed AuthnRequests using
HTTP-Redirect bindings. This happened for three reasons:

1. We were inserting the signature into the body of the SAML
AuthnRequest. When sending an AuthnRequest using HTTP-Redirect the
signature is included as a query param alongside the encoded SAML
request

2. We were specifying the wrong binding type. Our requests used
HTTP-Post binding. I think most SAML idps are pretty forgiving about
what a request looks like but get more string when you put them into the
verification mode that checks signatures Okta included here.

3. We did not specify a NameIDPolicy. Basically the same kind of thing
as point number 2.

As part of fixing those issues I decided to make our AuthnRequest flow
use more of the OpenSAML library instead of using the hiccup-based XML
generation. I plan to follow up to convert our Logout/SLO handling to
work in the same way and clean up unused functions in the crypto/coerce
namespaces once that is done.

Closes #91
edpaget added a commit that referenced this issue Feb 21, 2025
Previously we could not generate correctly signed AuthnRequests using
HTTP-Redirect bindings. This happened for three reasons:

1. We were inserting the signature into the body of the SAML
AuthnRequest. When sending an AuthnRequest using HTTP-Redirect the
signature is included as a query param alongside the encoded SAML
request

2. We were specifying the wrong binding type. Our requests used
HTTP-Post binding. I think most SAML idps are pretty forgiving about
what a request looks like but get more string when you put them into the
verification mode that checks signatures Okta included here.

3. We did not specify a NameIDPolicy. Basically the same kind of thing
as point number 2.

As part of fixing those issues I decided to make our AuthnRequest flow
use more of the OpenSAML library instead of using the hiccup-based XML
generation. I plan to follow up to convert our Logout/SLO handling to
work in the same way and clean up unused functions in the crypto/coerce
namespaces once that is done.

Closes #91
edpaget added a commit that referenced this issue Feb 24, 2025
Previously we could not generate correctly signed AuthnRequests using
HTTP-Redirect bindings. This happened for three reasons:

1. We were inserting the signature into the body of the SAML
AuthnRequest. When sending an AuthnRequest using HTTP-Redirect the
signature is included as a query param alongside the encoded SAML
request

2. We were specifying the wrong binding type. Our requests used
HTTP-Post binding. I think most SAML idps are pretty forgiving about
what a request looks like but get more string when you put them into the
verification mode that checks signatures Okta included here.

3. We did not specify a NameIDPolicy. Basically the same kind of thing
as point number 2.

As part of fixing those issues I decided to make our AuthnRequest flow
use more of the OpenSAML library instead of using the hiccup-based XML
generation. I plan to follow up to convert our Logout/SLO handling to
work in the same way and clean up unused functions in the crypto/coerce
namespaces once that is done.

Closes #91
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant