Sustainsys / Saml2

Saml2 Authentication services for ASP.NET
Other
960 stars 602 forks source link

Validate InResponseTo from SubjectConfirmationData #482

Closed hi20100 closed 8 years ago

hi20100 commented 8 years ago

Hi @AndersAbel,

I have a situation where the SAML response from a vendor doesn't include the "InResponseTo" attribute on the "samlp:Response" node, which is the location Kentor.AuthServices is looking at.

Instead, they include the attribute only on the "saml:SubjectConfirmationData" node. Based on the OASIS specs, this is technically passable/valid.

I was hoping that setting "allowUnsolicitedAuthnResponse" to true would essentially skip the validation of the attribute, but this has no effect.

Would it make sense to modify Kentor.AuthServices.Saml2P.Saml2Response.ReadAndValidateInResponseTo() to validate InResponseTo from both locations in the response?

Thanks,

albinsunnanbo commented 8 years ago

Hi If InResponseTo is not included in the response it should pass with allowUnsolicitedAuthnResponse = true. However if InResponseTo is set to a value it is validated regardless of the value of allowUnsolicitedAuthnResponse

Can you provide the error message you get?

Can you also capture the Saml2Response with https://addons.mozilla.org/en-US/firefox/addon/saml-tracer/ or https://chrome.google.com/webstore/detail/saml-chrome-panel/paijfdbeoenhembfhkhllainmocckace

albinsunnanbo commented 8 years ago

From line 729 in Saml2 core specification:

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.

"Might be" is the among the weaker forms of requirements in the specification. Below "MAY be". I'm not sure this is a good idea. But on the other hand I can't see that it would hurt either. /cc @AndersAbel

albinsunnanbo commented 8 years ago

Could it be that the InResponseTo attribute on the Response is empty string? According to spec. it should not be present at all if it does not contain a correct value.

hi20100 commented 8 years ago

Hi,

The error message: Expected message to contain InResponseTo "idbb0082403987404dbf3f7b79d3bb62ff", but found none.

Stack trace:

[Saml2ResponseFailedValidationException: Expected message to contain InResponseTo "idbb0082403987404dbf3f7b79d3bb62ff", but found none.]
   Kentor.AuthServices.Saml2P.Saml2Response.ReadAndValidateInResponseTo(XmlElement xml, Saml2Id expectedInResponseTo) +579
   Kentor.AuthServices.Saml2P.Saml2Response..ctor(XmlElement xml, Saml2Id expectedInResponseTo) +534
   Kentor.AuthServices.WebSso.AcsCommand.Run(HttpRequestData request, IOptions options) +1089
   Kentor.AuthServices.HttpModule.Saml2AuthenticationModule.OnPostAuthenticateRequest(Object sender, EventArgs e) +368
   System.Web.SyncEventExecutionStep.System.Web.HttpApplication.IExecutionStep.Execute() +141
   System.Web.HttpApplication.ExecuteStep(IExecutionStep step, Boolean& completedSynchronously) +69

Response (scrubbed for anonymity:

<samlp:Response ID="_f8887ec5-4c2a-485a-8ca5-44c607ed5077"
                Version="2.0"
                IssueInstant="2016-05-26T19:18:54.02Z"
                Destination="https://localhost:44302/SamplePath/AuthServices/Acs"
                xmlns:samlp="urn:oasis:names:tc:SAML:2.0:protocol"
                >
    <saml:Issuer xmlns:saml="urn:oasis:names:tc:SAML:2.0:assertion">https://SCRUBBED</saml:Issuer>
    <Signature xmlns="http://www.w3.org/2000/09/xmldsig#">
        <SignedInfo>
            <CanonicalizationMethod Algorithm="http://www.w3.org/2001/10/xml-exc-c14n#" />
            <SignatureMethod Algorithm="http://www.w3.org/2000/09/xmldsig#rsa-sha1" />
            <Reference URI="#_f8887ec5-4c2a-485a-8ca5-44c607ed5077">
                <Transforms>
                    <Transform Algorithm="http://www.w3.org/2000/09/xmldsig#enveloped-signature" />
                    <Transform Algorithm="http://www.w3.org/2001/10/xml-exc-c14n#">
                        <InclusiveNamespaces PrefixList="#default samlp saml ds xs xsi"
                                             xmlns="http://www.w3.org/2001/10/xml-exc-c14n#"
                                             />
                    </Transform>
                </Transforms>
                <DigestMethod Algorithm="http://www.w3.org/2000/09/xmldsig#sha1" />
                <DigestValue>SiQLufuCgl2aqNT6dj0xJIVjgiA=</DigestValue>
            </Reference>
        </SignedInfo>
        <SignatureValue>SCRUBBED</SignatureValue>
        <KeyInfo>
            <X509Data>
                <X509Certificate>SCRUBBED</X509Certificate>
            </X509Data>
        </KeyInfo>
    </Signature>
    <samlp:Status>
        <samlp:StatusCode Value="urn:oasis:names:tc:SAML:2.0:status:Success" />
    </samlp:Status>
    <saml:Assertion Version="2.0"
                    ID="_32591aaf-acb9-4ef6-8779-3e5db60418e6"
                    IssueInstant="2016-05-26T19:18:54.02Z"
                    xmlns:saml="urn:oasis:names:tc:SAML:2.0:assertion"
                    >
        <saml:Issuer>https://SCRUBBED</saml:Issuer>
        <saml:Subject>
            <saml:NameID>SCRUBBED</saml:NameID>
            <saml:SubjectConfirmation Method="urn:oasis:names:tc:SAML:2.0:cm:bearer">
                <saml:SubjectConfirmationData NotOnOrAfter="2016-05-26T20:18:54.02Z"
                                              Recipient="https://localhost:44302/SamplePath/AuthServices/Acs"
                                              InResponseTo="idbb0082403987404dbf3f7b79d3bb62ff"
                                              />
            </saml:SubjectConfirmation>
        </saml:Subject>
        <saml:Conditions NotBefore="2016-05-26T19:18:54.02Z"
                         NotOnOrAfter="2016-05-26T19:28:54.02Z"
                         >
            <saml:AudienceRestriction>
                <saml:Audience>https://localhost:44302/SamplePath/AuthServices/Acs</saml:Audience>
                <saml:Audience>https://localhost:44302/SamplePath/AuthServices</saml:Audience>
            </saml:AudienceRestriction>
        </saml:Conditions>
        <saml:AuthnStatement AuthnInstant="2016-05-26T19:18:54.02Z">
            <saml:AuthnContext>
                <saml:AuthnContextClassRef>urn:oasis:names:tc:SAML:2.0:ac:classes:Password</saml:AuthnContextClassRef>
            </saml:AuthnContext>
        </saml:AuthnStatement>
        <saml:AttributeStatement>
            <saml:Attribute Name="FirstName">
                <saml:AttributeValue>SCRUBBED</saml:AttributeValue>
            </saml:Attribute>
            <saml:Attribute Name="LastName">
                <saml:AttributeValue>SCRUBBED</saml:AttributeValue>
            </saml:Attribute>
            <saml:Attribute Name="Email">
                <saml:AttributeValue>SCRUBBED</saml:AttributeValue>
            </saml:Attribute>
        </saml:AttributeStatement>
    </saml:Assertion>
</samlp:Response>

Let me know if you need anything else.

Thanks,

albinsunnanbo commented 8 years ago

Thanks, that made it easier to figure out what was going on.

I checked the code and traced back from the error message. The problem is the relay state (decoded in https://github.com/KentorIT/authservices/blob/master/Kentor.AuthServices/WebSSO/HttpRequestData.cs#L71 ) from the query string or posted form values.

When AuthServices detects a relay state it knows the request is from itself and not unsolicited. Relying on Core specification line 1546

Otherwise, it MUST be present and its value MUST match the value of the corresponding request's ID attribute.

AuthServices knows that it sent an ID on a request and really expects an InResponseTo back.

It's up to @AndersAbel to decide if we could allow an option "we don't care about the specification in this case".

Would it be possible to ask your vendor if they could adapt to the specification?

hi20100 commented 8 years ago

That's great information, thanks!

I had made the request to the vendor prior to creating this issue. Its in their backlog to look into, but nothing immediate.

Please let me know on next steps. Or I can fork this repository and implement my own fix.

Thanks,

albinsunnanbo commented 8 years ago

Yes, you can create your own fork, just removing the check at https://github.com/KentorIT/authservices/blob/71a6fdd63d92c5a9f8576cae28cac14a9b11c91b/Kentor.AuthServices/SAML2P/Saml2Response.cs#L137

Beware that you should keep it up to date with the main repository to get future security updates into your code.

AndersAbel commented 8 years ago

Looks like you've already sorted out the issue. If you want to have a possibility to ignore a missing InResponseTo in the main release you can make it a setting in SPOptions.Compatibility.

hi20100 commented 8 years ago

Great, thanks guys - really appreciated!