SAML-Toolkits / python3-saml

MIT License
683 stars 304 forks source link

Add option to explicitly check that InResponseTo is not present #263

Closed m-novikov closed 3 years ago

m-novikov commented 3 years ago

This case is relevant when the application has IdP initiated login flow enabled. We know for sure in this case that we don't have an associated request_id available and it must not be present in the assertion.

The proposed solution looks like this:

from onelogin.saml2.auth import OneLogin_Saml2_Auth
from onelogin.saml.response import OneLogin_Saml2_Response

auth = OneLogin_Saml2_Auth(req)
#   I am unsure with regard of location of NO_REQUEST_ID object
auth.process_response(OneLogin_Saml2_Response.NO_REQUEST_ID)
pitbulk commented 3 years ago

The request_id is an optional parameter, if you don't provide it, the response validator will not check it: https://github.com/onelogin/python3-saml/blob/master/src/onelogin/saml2/response.py#L126

m-novikov commented 3 years ago

Hey @pitbulk thanks for taking the time to review the pr. Please consider what happens in the following case. We provided expected request_id as an argument but there is no InResponseTo in assertion. Validation will be successful which is incorrect. We expected to receive a concrete response but received the something different one.

pitbulk commented 3 years ago

Extracted from SAML core specs

ID [Required]
An identifier for the request. It is of type xs:ID and MUST follow the requirements specified in Section
1.3.4 for identifier uniqueness. The values of the ID attribute in a request and the InResponseTo
attribute in the corresponding response MUST match.

InResponseTo [Optional]
A reference to the identifier of the request to which the response corresponds, if any. If the response
is not generated in response to a request, or if the ID attribute value of a request cannot be
determined (for example, the request is malformed), then this attribute MUST NOT be present.
Otherwise, it MUST be present and its value MUST match the value of the corresponding request's
ID attribute

InResponseTo [Optional]
The ID of a SAML protocol message in response to which an attesting entity can present the
assertion. For example, this attribute might be used to correlate the assertion to a SAML request that
resulted in its presentation

So basically, if there is a request, it should be a response with InResponseTo value. If is a IdP-initiated flow, there is no InResponseTo value. If the SP initiates the flow, you can store the Request ID and then provide it to validate the InResponseTo value.

If you provide a request_id in order to validate the InResponseTo, but there is no InResponseTo, that means that meanwhile you generated the request, an IdP-initiated flow happened, that SAMLResponse is legit and it should not be invalidated by the request_id match, in my opinion.

paulwouters commented 3 years ago

I am a bit confused here:

If the response is not generated in response to a request, or if the ID attribute value of a request cannot be
determined (for example, the request is malformed), then this attribute MUST NOT be present.
Otherwise, it MUST be present and its value MUST match the value of the corresponding request's
ID attribute

In the case mentioned:

We provided expected request_id as an argument but there is no InResponseTo in assertion.

It seems the the ID attribute value should be able to be determined (we sent it) and it is not malformed. So it MUST be present, but it is not. Shouldn't that violation of a "MUST" not trigger an error/assertion ?

paulwouters commented 2 years ago

pinging @pitbulk