Describe the bug The InResponseTo field is not validated and the repsonse is not rejected when this does not correspond to a sent request.
To Reproduce 1. Set up SAML2 login and run your server locally which redirects to an IDP which will then redirect to an actual deployed SAML2 authentication endpoint. 2. Set the host of your deployed app to 127.0.0.1 in /etc/hosts 3. Ping the host and verify that localhost responds 4. Start the local app and try to log in, you will be directed to your IDP which will redirect back to the authentication endpoint which will not exist on your local machine, thereby resulting in a not found error. 5. Copy the unresponded request as cURL from the console and paste it in a document. 6. Add flags .L and -i and remove cookies headers to be on the safe side. 7. Remove the entry in /etc/hosts and verify that the actual deployed application responds on ping. 7. Run the command with cURL and verify that the deployed application authentication endpoint accepts the cURL command which is a response to a request NOT initiated by the deployed application but by the local application.
Expected behavior If the request contains a InResponseTo which is incorrect, it should be rejected.
Sample
A link to a GitHub repository with a minimal, reproducible sample.
Reports that include a sample will take priority over reports that do not. At times, we may require a sample, so it is good to try and include a sample up front.
Comment From: jzheaux
@fast-reflexes, you are correct - this is because the AuthNRequest is not saved anywhere after the relying party generates and sends it to the asserting party.
I've created a ticket to add that support - https://github.com/spring-projects/spring-security/issues/9185 - where your contribution would be most welcome.
Once that is complete, we can take a closer look at this ticket to see how to best validate InResponseTo. In the meantime, would you mind if I changed this ticket to an enhancement and reworded it to Add InResponseTo validation support?
Comment From: fast-reflexes
@jzheaux Thanks for answering! No, go ahead and reword it!
Comment From: jzheaux
@fast-reflexes Now that Saml2AuthenticationRequestRepository is in place, I think we have what's needed to proceed with this ticket.
As I see it, there are two things needed:
- Add an
idattribute toAbstractSaml2AuthenticationRequestand populate it inOpenSamlAuthenticationRequestResolver - Add a validation step in
OpenSaml4AuthenticationProviderthat comparesResponse#inResponseTotoSaml2AuthenticationToken#getAuthenticationRequest#getIdif both are set
Are you able to provide a PR along those lines? If not, I believe I have some time to do it.
Comment From: bitrecycling
@jzheaux I have the same problem currently. I have 2 thoughts on that topic currently: 1) I think validation is only really relevant for Responses to authn Request and logout request or am I mistaken? Also replay attacks with "inResponseTo"-Responses can be mitigated if "inResponseTo" is validated.
2) However I was wondering, if an additional validation in the ID itself makes sense, to prevent using the same response i.e. if no "inResponseTo" is present. If any Request ID is stored for the time the request is valid and any new request's ID is checked against the list of the already consumed (and hence ID stored) IDs, then everything should be fine.
What do you think?
Comment From: fast-reflexes
@jzheaux Yes, I would really like to start contributing so could you give me a week to do it and I will either do it in that time or let someone else ahead?
Comment From: jzheaux
@bitrecycling, good questions:
I think validation is only really relevant for Responses to authn Request and logout request
I think that it's important to confirm that the InResponseTo value in a SAML 2.0 Response actually matches the AuthnRequest that initiated the request. Same for logout. Yes, I believe it's a way to mitigate replay attacks.
However I was wondering, if an additional validation in the ID itself makes sense, to prevent using the same response i.e. if no "inResponseTo" is present.
A SAML Response has an expiry. Given that, you should be able to create a cache of IDs that you can check against for replay attacks. For the time being, I think Spring Security's default validation should remain minimal so that it's easy to extend and replace, but I do think a component like that could be interesting for you to share, should you create one.
Comment From: bitrecycling
@jzheaux after thinking it through, I currently would argue against ID validation at least for login where it's most critical. Reasoning: 1) Spring security saml2 currently only supports Web Browser SSO profile, that means all requests / responses are handed over through the user's browser and are (at least the logins) user "initiated". Even true for SSO logins to another SP, because an AuthnRequest is issued still if "my" SP sees an anonymous user. 2) That in turn means any reponse from an IDP will be aligned into the same session as the originating request. Hence the request must reside already in the Saml2AuthenticationRequestRepository that can be session-specific. 3) If 1 and 2 are true, any response to a login must have a valid inResponseTo that can be checked against the stored AuthnRequest. 4) this renders an ID validation using a storage of already "seen" and still valid request IDs somehow unnecessary. It might be useful to prevent unwanted SLO logouts that are not initiated on "my" SP. But I doubt it's value.
So given the high value of and (relatively) easy and lightweight validation of "inResponseTo" I would go that path to prevent false or doubly issued login responses. And rather not implement the difficult to handle and maintain IDs Storage to check against especially given that it might not even provide real value. I am sorry this got lengthy, but I would really appreciate your opinion. If you still think it could be useful, I might come up with a solution.
I definitely need a solution for the inReponseTo validation though, probably even earlier than @fast-reflexes can provide, so I will likely come up with my own implementation.
/Robert
Comment From: fast-reflexes
@jzheaux I've started working on it. So we should always validate if it is present in the response, no user flag to indicate whether the validation functionality is desired or not?
Comment From: jzheaux
Good question, @fast-reflexes. I think you'd validate it if both the request and response are present and the response has an InResponseTo. Applications may choose to not store the AuthnRequest.
To turn off that validation, applications can always either:
- Not store the AuthnRequest, or
- Wire their own response validator. It could either be completely custom or could delegate to the default and remove the unwanted error
Comment From: fast-reflexes
A question, if the login is unsolicited, there won't be a request stored, but if there IS a request stored but the response does NOT have InResponseTo attribute set, then we should reject as well right?