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

ValidationContext is lost unexpectedly causing InResponseTo validation to fail #16392

Open
tyler555g opened this issue Jan 9, 2025 · 6 comments
Assignees
Labels
in: saml2 An issue in SAML2 modules status: feedback-reminder We've sent a reminder that we need additional information before we can continue status: waiting-for-feedback We need additional information before we can continue

Comments

@tyler555g
Copy link

Describe the bug
ValidationContext is lost unexpectedly causing InResponseTo validation to fail

To Reproduce
Set a breakpoint here:
https://github.com/tyler555g/SpringSecuritySAMLPOC/blob/main/src/main/java/com/SpringSecuritySSOPOC/saml/config/Saml2LoginSecurityConfig.java#L162
Run backend. Create a login call. Wait for a couple minutes. Create another login call. Login call occasionally loses ValidationContext. See logs attached and working code example.

Expected behavior
InResponseTo validation passes correctly

Sample
https://github.com/tyler555g/SpringSecuritySAMLPOC/blob/main/src/main/java/com/SpringSecuritySSOPOC/saml/config/Saml2LoginSecurityConfig.java#L162
SpringSecurityLogsNoResponseTo20250109.txt

Log snippets:

2025-01-09T14:53:11.642-05:00 TRACE 29112 --- [nio-8091-exec-5] o.o.s.s.a.SAML20AssertionValidator : SAML 2 Assertion ValidationContext - static parameters: {saml2.ValidIssuers=[redacted], saml2.SubjectConfirmation.ValidInResponseTo=null, saml2.ClockSkew=PT5M, saml2.Conditions.ValidAudiences=[http://localhost:8091/saml2/service-provider-metadata/azure-ad], saml2.SubjectConfirmation.ValidRecipients=[http://localhost:8091/login/saml2/sso/azure-ad]}
2025-01-09T14:53:11.642-05:00 TRACE 29112 --- [nio-8091-exec-5] o.o.s.s.a.SAML20AssertionValidator : SAML 2 Assertion ValidationContext - dynamic parameters: {}
2025-01-09T14:53:11.642-05:00 DEBUG 29112 --- [nio-8091-exec-5] o.o.s.s.a.SAML20AssertionValidator : Evaluating Assertion Issuer of : redacted
2025-01-09T14:53:11.642-05:00 DEBUG 29112 --- [nio-8091-exec-5] o.o.s.s.a.SAML20AssertionValidator : Matched valid issuer: redacted
2025-01-09T14:53:11.642-05:00 DEBUG 29112 --- [nio-8091-exec-5] o.o.s.s.a.SAML20AssertionValidator : No Conditions were indicated as required
...

@tyler555g tyler555g added status: waiting-for-triage An issue we've not yet triaged type: bug A general bug labels Jan 9, 2025
@jzheaux jzheaux self-assigned this Jan 17, 2025
@jzheaux
Copy link
Contributor

jzheaux commented Jan 17, 2025

Hi, @tyler555g, thanks for the report.

I can't use the sample yet because it doesn't appear to start up. If I use the local profile, then I get the following exception:

Caused by: java.lang.IllegalArgumentException: Could not resolve placeholder 'saml.decrypt' in value "${saml.decrypt}"
	at org.springframework.util.PropertyPlaceholderHelper.parseStringValue(PropertyPlaceholderHelper.java:180) ~[spring-core-6.1.13.jar:6.1.13]
	at org.springframework.util.PropertyPlaceholderHelper.replacePlaceholders(PropertyPlaceholderHelper.java:126) ~[spring-core-6.1.13.jar:6.1.13]
	at org.springframework.core.env.AbstractPropertyResolver.doResolvePlaceholders(AbstractPropertyResolver.java:239) ~[spring-core-6.1.13.jar:6.1
        ....

When I use the default profile, then I get the following:

Caused by: java.lang.IllegalArgumentException: Could not resolve placeholder 'backend.url' in value "${backend.url}"
	at org.springframework.util.PropertyPlaceholderHelper.parseStringValue(PropertyPlaceholderHelper.java:180) ~[spring-core-6.1.13.jar:6.1.13]
	at org.springframework.util.PropertyPlaceholderHelper.replacePlaceholders(PropertyPlaceholderHelper.java:126) ~[spring-core-6.1.13.jar:6.1.13]
...

Can you update the code so that it starts up, and then I'd be happy to try and reproduce?

@jzheaux jzheaux added status: waiting-for-feedback We need additional information before we can continue in: saml2 An issue in SAML2 modules and removed status: waiting-for-triage An issue we've not yet triaged type: bug A general bug labels Jan 17, 2025
@jzheaux
Copy link
Contributor

jzheaux commented Jan 17, 2025

One thing that strikes me is that OpenSAML's InResponseTo property is present, but null. That likely means that Spring Security couldn't find the associated AuthnRequest to compare to the value.

It makes me wonder if the issue is a race condition, perhaps via the browser, where the session cookie is different, so the Saml2AuthenticationRequestRepository is empty.

@tyler555g
Copy link
Author

tyler555g commented Jan 17, 2025

Hi, @tyler555g, thanks for the report.

I can't use the sample yet because it doesn't appear to start up. If I use the local profile, then I get the following exception:

Caused by: java.lang.IllegalArgumentException: Could not resolve placeholder 'saml.decrypt' in value "${saml.decrypt}"
at org.springframework.util.PropertyPlaceholderHelper.parseStringValue(PropertyPlaceholderHelper.java:180) ~[spring-core-6.1.13.jar:6.1.13]
at org.springframework.util.PropertyPlaceholderHelper.replacePlaceholders(PropertyPlaceholderHelper.java:126) ~[spring-core-6.1.13.jar:6.1.13]
at org.springframework.core.env.AbstractPropertyResolver.doResolvePlaceholders(AbstractPropertyResolver.java:239) ~[spring-core-6.1.13.jar:6.1
....
When I use the default profile, then I get the following:

Caused by: java.lang.IllegalArgumentException: Could not resolve placeholder 'backend.url' in value "${backend.url}"
at org.springframework.util.PropertyPlaceholderHelper.parseStringValue(PropertyPlaceholderHelper.java:180) ~[spring-core-6.1.13.jar:6.1.13]
at org.springframework.util.PropertyPlaceholderHelper.replacePlaceholders(PropertyPlaceholderHelper.java:126) ~[spring-core-6.1.13.jar:6.1.13]
...
Can you update the code so that it starts up, and then I'd be happy to try and reproduce?

https://github.com/tyler555g/SpringSecuritySAMLPOC/blob/main/src/main/resources/application-local.properties

#You must either replace SAML_DECRYPTION_CERTIFICATE, SAML_SIGNING_KEY and IDP_METADATA_URL with your own values or specify them in environment variables
#X509 certificate used to decrypt incoming SAML messages
saml.decryption.certificate=${SAML_DECRYPTION_CERTIFICATE}
#RSA key used to sign AuthnRequest messages
saml.signing.key=${SAML_SIGNING_KEY}
security.jwt.token.expire-hour-length=12
webapp.url=http://localhost:4200/
backend.url=http://localhost:8091/
saml.idp.metadata-url=${IDP_METADATA_URL}

I've updated the application-local.properties with a comment explaining use of the environment variables. Please replace with your own as configured for your specific IDP.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Jan 17, 2025
@tyler555g
Copy link
Author

One thing that strikes me is that OpenSAML's InResponseTo property is present, but null. That likely means that Spring Security couldn't find the associated AuthnRequest to compare to the value.

It makes me wonder if the issue is a race condition, perhaps via the browser, where the session cookie is different, so the Saml2AuthenticationRequestRepository is empty.

Could you point to additional logging or tracing I can do to support this hypothesis? I am able to reproduce and capture the entire session in HTTP Toolkit.

@jzheaux
Copy link
Contributor

jzheaux commented Jan 18, 2025

I am able to reproduce and capture the entire session in HTTP Toolkit.

Yes, I believe that may help. Are you able to post the HTTP request and response headers of the full authentication flow? Specifically, the /saml2/authenticate/azure-ad request and response, the IdP's response to the AuthnRequest, and the /login/saml2/sso/azure-ad request and response.

@jzheaux jzheaux added status: waiting-for-feedback We need additional information before we can continue and removed status: feedback-provided Feedback has been provided labels Jan 18, 2025
@spring-projects-issues
Copy link

If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.

@spring-projects-issues spring-projects-issues added the status: feedback-reminder We've sent a reminder that we need additional information before we can continue label Jan 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: saml2 An issue in SAML2 modules status: feedback-reminder We've sent a reminder that we need additional information before we can continue status: waiting-for-feedback We need additional information before we can continue
Projects
None yet
Development

No branches or pull requests

3 participants