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

Signature of Assertion from issuer was not valid and invalid destination for SAML response by multiple simultaneous login #14885

Open
nojanbakh opened this issue Apr 11, 2024 · 3 comments
Assignees
Labels
status: feedback-provided Feedback has been provided

Comments

@nojanbakh
Copy link

Describe the bug
I have encountered an issue with the Spring SAML library that leads to incorrect token validation during the authentication process. The problem occurs under concurrent authentication scenarios, wherein the response from the Identity Provider (IdP) intended for one user may be erroneously processed by another user's authentication thread, resulting in invalid error messages. After refreshing the page, a new authentication succeeds.
It seems that by requesting /authenticate the relyingPartyRegistration creates a registration with a registrationId in one thread and waits for response from external IdP to validate the request. But it can happen that a new request creates a new registrationId in the same thread, and it causes invalid destination and invalid signature.

To Reproduce
Initiate concurrent authentication requests for multiple users.
Observe the processing of authentication tokens and IdP responses.
Note instances where responses are incorrectly validated against tokens belonging to different users.
It can be reproduced with a script that sends /login and /authenticate requests for different idPs together in a loop.

Expected behavior
The Spring SAML library should ensure proper association of authentication tokens with their respective users, preventing cross-thread validation issues.

http.authorizeRequests()
                        .saml2Login()
                        .failureHandler(samlAuthenticationFailureHandler)
                        `.successHandler(samlAuthenticationSuccessHandler);`

RelyingPartyRegistrations
                .fromMetadataLocation(metadataUrl())
                .registrationId(idpId)
                .assertionConsumerServiceLocation( ".../login/" + idpId)
                .entityId( ".../login/" + idpId)
                .build();
@nojanbakh nojanbakh added status: waiting-for-triage An issue we've not yet triaged type: bug A general bug labels Apr 11, 2024
@jzheaux
Copy link
Contributor

jzheaux commented Apr 26, 2024

Hi, @nojanbakh, thanks for reaching out. I'm not sure that I understand the context in which RelyingPartyRegistrations is being used here. Generally speaking, it is not for use when processing an authentication request, but instead when the application starts up. Thus, a RelyingPartyRegistration is only created once per asserting party.

But it can happen that a new request creates a new registrationId in the same thread, and it causes invalid destination and invalid signature.

It's expected that you would give the same registrationId to the same asserting party. Can you explain why they are changing?

@jzheaux jzheaux self-assigned this Apr 26, 2024
@jzheaux jzheaux added status: waiting-for-feedback We need additional information before we can continue and removed status: waiting-for-triage An issue we've not yet triaged type: bug A general bug labels Apr 26, 2024
@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 May 3, 2024
@nojanbakh
Copy link
Author

nojanbakh commented May 8, 2024

I am new in this field. maybe i can explain it to you with this logs

2024-05-08 07:00:27.591 [nio-8080-exec-4] c.InDBRelyingPartyRegistrationRepository : IdP for registrationId 0921cd83 loaded

2024-05-08 07:00:30.691 [nio-8080-exec-9] c.InDBRelyingPartyRegistrationRepository : IdP for registrationId a4a11374 loaded 

2024-05-08 07:00:30.834 [nio-8080-exec-9] o.o.s.s.a.SAML20AssertionValidator : Signature of Assertion '_e47a996f-6d11-44e9-94f7-73a5a3434900' from Issuer was not valid

2024-05-08 07:00:30.838 ERROR [nio-8080-exec-9] l.s.c.c.SAMLAuthenticationFailureHandler : SAML user login failed
org.springframework.security.saml2.provider.service.authentication.Saml2AuthenticationException: Invalid destination [https://login/sso/0921cd83] for SAML response 

2024-05-08 07:00:31.832 [nio-8080-exec-2] c.InDBRelyingPartyRegistrationRepository : IdP for registrationId 0921cd83 loaded

2024-05-08 07:00:32.023 [nio-8080-exec-2] l.s.c.c.SAMLAuthenticationSuccessHandler : onAuthenticationSuccess process received Saml2 token

As you can see, it seems that the first person tries to authenticate and trying to log in with azure and the second person from the other IDP tries to log in too. It causes the invalid signature by checking the token. And when he tries again, it works.

@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 status: feedback-reminder We've sent a reminder that we need additional information before we can continue labels May 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: feedback-provided Feedback has been provided
Projects
None yet
Development

No branches or pull requests

3 participants