Xpath search failing - failure to validate assertion in xmerl_dsig:verify

[chefsalim]

Using Erlang 18, and Okta dev endpoint for a test SAML response. Handler code is using the esaml SP sample as the reference.

Response validation fails with this message:

Access denied, assertion failed validation:
{{badmatch,[]},
 [{xmerl_dsig,verify,2,
              [{file,"/mnt/sync/delivery/server/deps/esaml/src/xmerl_dsig.erl"},
               {line,169}]},
  {esaml_sp,'-validate_assertion/3-fun-1-',4,
            [{file,"src/esaml_sp.erl"},{line,209}]},
  {esaml_util,threaduntil,2,[{file,"src/esaml_util.erl"},{line,88}]},
  {esaml_cowboy,validate_assertion,3,
                [{file,"src/esaml_cowboy.erl"},{line,168}]},
  {deliv_hand_saml_consume,handle_post,3,
                           [{file,"src/deliv_hand_saml_consume.erl"},
                            {line,41}]},
  {cowboy_handler,handler_handle,4,
                  [{file,"src/cowboy_handler.erl"},{line,111}]},
  {cowboy_protocol,execute,4,[{file,"src/cowboy_protocol.erl"},{line,435}]}]}

Debugging into the code, the failure is at line 169, in xmerl_dsig.erl:

 [#xmlAttribute{value = SignatureMethodAlgorithm}] = xmerl_xpath:string("ds:Signature/ds:SignedInfo/ds:SignatureMethod/@Algorithm", Element, [{namespace, DsNs}]),

The problem is that the xpath is not finding anything, just returning an empty element.
Changing the search string to be "//ds:Signature/ds:SignedInfo/ds:SignatureMethod/@Algorithm" fixes the problem.

A sample response XML that illustrates the problem is below.

<samlp:Assertion xmlns:samlp="urn:oasis:names:tc:SAML:2.0:protocol" xmlns:saml="urn:oasis:names:tc:SAML:2.0:assertion" Version="2.0" ID="_nmT8siYgDw0GAxhwwyEIJR29z50CCCJv" IssueInstant="2016-05-05T21:14:48.906Z">
  <saml:Issuer>http://example.com/saml/acs/example</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="#_nmT8siYgDw0GAxhwwyEIJR29z50CCCJv">
        <Transforms>
          <Transform Algorithm="http://www.w3.org/2000/09/xmldsig#enveloped-signature" />
          <Transform Algorithm="http://www.w3.org/2001/10/xml-exc-c14n#" />
        </Transforms>
        <DigestMethod Algorithm="http://www.w3.org/2000/09/xmldsig#sha1" />
        <DigestValue>717x5bSFCJyKOf04gycEXhWSyLs=</DigestValue>
      </Reference>
    </SignedInfo>
    <SignatureValue>Uo7qsqdQ8+DoPkQIQ6JKMeKLPvh3/KwtKogneQr9eZr8lMoeWJwjw8HYpQoc45/2lCjDh5bugJf952688ug9Gyc5Bvfqvi0ms/OVagyOFBmoNG/hgtw9uvPD/Z8jI/WLWQUcA/zGMZFhUoVdputNRHtU7vr53Sr9Gh9EtrqMv9cbbT3yQKGAwFNAYKJNb/znSG16xEoAVs4QZxSBPcSCGNoTtNpGfuKgtdMNnQKHFvyqq3gtGdhRIeqQHy2Q6C0xTYXfzoqvpRBFsIZiPfxi1rdNW/O9NRO4bwI5CWG1ssjTjTAvdhYBFLMyPdLXjvDl9qT51dXtuIeSVIuo0XolPg==</SignatureValue>
    <KeyInfo>
      <X509Data>
        <X509Certificate>MIIDPDCCAiQCCQDydJgOlszqbzANBgkqhkiG9w0BAQUFADBgMQswCQYDVQQGEwJVUzETMBEGA1UECBMKQ2FsaWZvcm5pYTEWMBQGA1UEBxMNU2FuIEZyYW5jaXNjbzEQMA4GA1UEChMHSmFua3lDbzESMBAGA1UEAxMJbG9jYWxob3N0MB4XDTE0MDMxMjE5NDYzM1oXDTI3MTExOTE5NDYzM1owYDELMAkGA1UEBhMCVVMxEzARBgNVBAgTCkNhbGlmb3JuaWExFjAUBgNVBAcTDVNhbiBGcmFuY2lzY28xEDAOBgNVBAoTB0phbmt5Q28xEjAQBgNVBAMTCWxvY2FsaG9zdDCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBAMGvJpRTTasRUSPqcbqCG+ZnTAurnu0vVpIG9lzExnh11o/BGmzu7lB+yLHcEdwrKBBmpepDBPCYxpVajvuEhZdKFx/Fdy6j5mH3rrW0Bh/zd36CoUNjbbhHyTjeM7FN2yF3u9lcyubuvOzr3B3gX66IwJlU46+wzcQVhSOlMk2tXR+fIKQExFrOuK9tbX3JIBUqItpI+HnAow509CnM134svw8PTFLkR6/CcMqnDfDK1m993PyoC1Y+N4X9XkhSmEQoAlAHPI5LHrvuujM13nvtoVYvKYoj7ScgumkpWNEvX652LfXOnKYlkB8ZybuxmFfIkzedQrbJsyOhfL03cMECAwEAATANBgkqhkiG9w0BAQUFAAOCAQEAeHwzqwnzGEkxjzSD47imXaTqtYyETZow7XwBc0ZaFS50qRFJUgKTAmKS1xQBP/qHpStsROT35DUxJAE6NY1Kbq3ZbCuhGoSlY0L7VzVT5tpu4EY8+Dq/u2EjRmmhoL7UkskvIZ2n1DdERtd+YUMTeqYl9co43csZwDno/IKomeN5qaPc39IZjikJ+nUC6kPFKeu/3j9rgHNlRtocI6S1FdtFz9OZMQlpr0JbUt2T3xS/YoQJn6coDmJL5GTiiKM6cOe+Ur1VwzS1JEDbSS2TWWhzq8ojLdrotYLGd9JOsoQhElmz+tMfCFQUFLExinPAyy7YHlSiVX13QH2XTu/iQQ==</X509Certificate>
      </X509Data>
    </KeyInfo>
  </Signature>
  <saml:Subject>
    <saml:NameID Format="urn:oasis:names:tc:SAML:1.1:nameid-format:unspecified">ilangoc@perillonworkspace.com</saml:NameID>
    <saml:SubjectConfirmation Method="urn:oasis:names:tc:SAML:2.0:cm:bearer">
      <saml:SubjectConfirmationData NotOnOrAfter="2016-05-05T22:14:48.906Z" Recipient="http://192.168.33.66/api/v0/e/cd/saml/consume" InResponseTo="5e0b9802-1306-11e6-9037-080027c97816" />
    </saml:SubjectConfirmation>
  </saml:Subject>
  <saml:Conditions NotBefore="2016-05-05T21:14:48.906Z" NotOnOrAfter="2016-05-05T22:14:48.906Z">
    <saml:AudienceRestriction>
      <saml:Audience>http://example.com/saml/acs/example</saml:Audience>
    </saml:AudienceRestriction>
  </saml:Conditions>
  <saml:AttributeStatement xmlns:xs="http://www.w3.org/2001/XMLSchema" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance">
    <saml:Attribute Name="urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress">
      <saml:AttributeValue xsi:type="xs:anyType">ilangoc@perillonworkspace.com</saml:AttributeValue>
    </saml:Attribute>
    <saml:Attribute Name="Email">
      <saml:AttributeValue xsi:type="xs:anyType">ilangoc@perillonworkspace.com</saml:AttributeValue>
    </saml:Attribute>
    <saml:Attribute Name="FirstName">
      <saml:AttributeValue xsi:type="xs:anyType">Ilango</saml:AttributeValue>
    </saml:Attribute>
    <saml:Attribute Name="LastName">
      <saml:AttributeValue xsi:type="xs:anyType">Chinnasamy</saml:AttributeValue>
    </saml:Attribute>
  </saml:AttributeStatement>
  <saml:AuthnStatement AuthnInstant="2016-05-05T21:14:48.906Z">
    <saml:AuthnContext>
      <saml:AuthnContextClassRef>urn:oasis:names:tc:SAML:2.0:ac:classes:unspecified</saml:AuthnContextClassRef>
    </saml:AuthnContext>
  </saml:AuthnStatement>
</samlp:Assertion>

[arekinath]

Changing the xpath to have // at the front is not the correct solution, unfortunately -- this would break nested signatures. The problem here is that we've somehow given xmerl_dsig:verify a #xmlDocument{} rather than a #xmlElement{} as it's meant to be given.

The correct solution is to make sure that the code in esaml_sp can only ever pass an #xmlElement{} through to here (unpack the #xmlDocument{} if we have one). It's not very clear when xmerl_scan:string returns #xmlDocument{} and when it returns #xmlElement{}, unfortunately, and against the XML documents that this has been developed and tested against so far it looks like it's always produced the latter, which is why this has been missed until now.

[sdelano]

Hi Alex. Thanks for the quick response! Based on what you've said, I've got a few follow up questions while I continue debugging this.

The problem here is that we've somehow given xmerl_dsig:verify a #xmlDocument{} rather than a #xmlElement{} as it's meant to be given

This is true. We've given xmerl_dsig:verify an #xmlDocument{}, but it appears to be intentional unless this is a bug.

In esaml_cowboy:validate_assertion/3, we call esaml_binding:decode_response/2 and pass the #xmlDocument{} record to esaml_sp:validate_assertion/2 (code link):

    case (catch esaml_binding:decode_response(SAMLEncoding, SAMLResponse)) of
        {'EXIT', Reason} ->
            {error, {bad_decode, Reason}, Req2};
        Xml ->
            case SP:validate_assertion(Xml, DuplicateFun) of
                {ok, A} -> {ok, A, RelayState, Req2};
                {error, E} -> {error, E, Req2}
            end
    end.

It's clear from the specs that esaml_binding:deocde_response/2 returns an #xmlDocument{}.

In esaml_sp:validate_assertion/3 we've got a list of funs that we iterate over and apply to the output of the previous fun. Pretty straightforward, but this might be there the bug is (code link):

validate_assertion(Xml, DuplicateFun, SP = #esaml_sp{}) ->
    Ns = [{"samlp", 'urn:oasis:names:tc:SAML:2.0:protocol'},
          {"saml", 'urn:oasis:names:tc:SAML:2.0:assertion'}],
    esaml_util:threaduntil([
        fun(X) ->
            case xmerl_xpath:string("/samlp:Response/saml:Assertion", X, [{namespace, Ns}]) of
                [A] -> A;
                _ -> {error, bad_assertion}
            end
        end,
        fun(A) ->
            if SP#esaml_sp.idp_signs_envelopes ->
                case xmerl_dsig:verify(Xml, SP#esaml_sp.trusted_fingerprints) of
                    ok -> A;
                    OuterError -> {error, {envelope, OuterError}}
                end;
            true -> A
            end
        end,
        fun(A) ->
            if SP#esaml_sp.idp_signs_assertions ->
                case xmerl_dsig:verify(A, SP#esaml_sp.trusted_fingerprints) of
                    ok -> A;
                    InnerError -> {error, {assertion, InnerError}}
                end;
            true -> A
            end
        end,
        fun(A) ->
            case esaml:validate_assertion(A, SP#esaml_sp.consume_uri, SP#esaml_sp.metadata_uri) of
                {ok, AR} -> AR;
                {error, Reason} -> {error, Reason}
            end
        end,
        fun(AR) ->
            case DuplicateFun(AR, xmerl_dsig:digest(Xml)) of
                ok -> AR;
                _ -> {error, duplicate}
            end
        end
    ], Xml).

The second function above is where we're failing. We're passing in the #xmlDocument{} that's given as the input to esaml_sp:validate_assertion/3.

This "outer verify" precedes the "inner verify" that comes next, which indeed takes as input the #xmlElement{} returned from the first fun in the list.

All other calls to xmerl_dsig:verify also appear to take as input #xmlDocument{} instead of #xmlElement{}.

I'll see what I can do about coercing the #xmlDocument{} records into #xmlElement{} ones.

[arekinath]

OK. I'm trying to get back into the headspace I had when I wrote this and why there's a mismatch here. Reading back over the xmldsig and c14n specs, it seems to say that we are meant to deal with miscellaneous PIs like <?xml-stylesheet by including them in the canonical XML. We have to strip out DOCTYPEs and the <?xml initial PI, however. The xmerl_c14n code deals with this correctly at the moment (and accepts #xmlDocument{} inputs)

So, I think it might actually be more correct after all to redo the xmerl_dsig:verify logic to deal with xmlDocuments. And then amend esaml_binding:decode_response so that it always returns xmlDocuments no matter what (using the {document, true} option?). That way we handle inputs with PIs and such correctly too (even though I don't know of any SAML IdPs that produce any, I'm constantly surprised by strange things they do). Sorry for barking up the wrong tree to start with here, I think your initial suggestion is actually better than what I proposed in my last comment. :)

[sdelano]

Thanks! Also, after debugging some more and reading the code more thoroughly, we may very well be in a state where the IdP doesn't sign the envelope and that part simply doesn't exist. It's unfortunate that this bubbles up as a badmatch deep inside instead of the {error, {envelope, OuterError}} tuple that the code is trying to catch.

[arekinath]

The sample XML you posted above is envelope-signed correctly. Is that not the payload causing the issue for you?

[sdelano]

Apologies, here's the actual XML response where the envelope is not signed:

<samlp:Response xmlns:samlp="urn:oasis:names:tc:SAML:2.0:protocol" ID="_5c67bfccebd7b58fbef3" InResponseTo="5e0b9802-1306-11e6-9037-080027c97816" Version="2.0" IssueInstant="2016-05-05T21:14:48Z" Destination="http://192.168.33.66/api/v0/e/cd/saml/consume">
<saml:Issuer xmlns:saml="urn:oasis:names:tc:SAML:2.0:assertion">http://example.com/saml/acs/example</saml:Issuer>
<samlp:Status>
    <samlp:StatusCode Value="urn:oasis:names:tc:SAML:2.0:status:Success"/>
</samlp:Status>
<saml:Assertion xmlns:saml="urn:oasis:names:tc:SAML:2.0:assertion" Version="2.0" ID="_nmT8siYgDw0GAxhwwyEIJR29z50CCCJv" IssueInstant="2016-05-05T21:14:48.906Z">
    <saml:Issuer>http://example.com/saml/acs/example</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="#_nmT8siYgDw0GAxhwwyEIJR29z50CCCJv">
            <Transforms>
                <Transform Algorithm="http://www.w3.org/2000/09/xmldsig#enveloped-signature"/>
                <Transform Algorithm="http://www.w3.org/2001/10/xml-exc-c14n#"/>
            </Transforms>
            <DigestMethod Algorithm="http://www.w3.org/2000/09/xmldsig#sha1"/>
            <DigestValue>717x5bSFCJyKOf04gycEXhWSyLs=</DigestValue>
        </Reference>
    </SignedInfo>
    <SignatureValue>Uo7qsqdQ8+DoPkQIQ6JKMeKLPvh3/KwtKogneQr9eZr8lMoeWJwjw8HYpQoc45/2lCjDh5bugJf952688ug9Gyc5Bvfqvi0ms/OVagyOFBmoNG/hgtw9uvPD/Z8jI/WLWQUcA/zGMZFhUoVdputNRHtU7vr53Sr9Gh9EtrqMv9cbbT3yQKGAwFNAYKJNb/znSG16xEoAVs4QZxSBPcSCGNoTtNpGfuKgtdMNnQKHFvyqq3gtGdhRIeqQHy2Q6C0xTYXfzoqvpRBFsIZiPfxi1rdNW/O9NRO4bwI5CWG1ssjTjTAvdhYBFLMyPdLXjvDl9qT51dXtuIeSVIuo0XolPg==</SignatureValue>
    <KeyInfo>
        <X509Data>
            <X509Certificate>MIIDPDCCAiQCCQDydJgOlszqbzANBgkqhkiG9w0BAQUFADBgMQswCQYDVQQGEwJVUzETMBEGA1UECBMKQ2FsaWZvcm5pYTEWMBQGA1UEBxMNU2FuIEZyYW5jaXNjbzEQMA4GA1UEChMHSmFua3lDbzESMBAGA1UEAxMJbG9jYWxob3N0MB4XDTE0MDMxMjE5NDYzM1oXDTI3MTExOTE5NDYzM1owYDELMAkGA1UEBhMCVVMxEzARBgNVBAgTCkNhbGlmb3JuaWExFjAUBgNVBAcTDVNhbiBGcmFuY2lzY28xEDAOBgNVBAoTB0phbmt5Q28xEjAQBgNVBAMTCWxvY2FsaG9zdDCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBAMGvJpRTTasRUSPqcbqCG+ZnTAurnu0vVpIG9lzExnh11o/BGmzu7lB+yLHcEdwrKBBmpepDBPCYxpVajvuEhZdKFx/Fdy6j5mH3rrW0Bh/zd36CoUNjbbhHyTjeM7FN2yF3u9lcyubuvOzr3B3gX66IwJlU46+wzcQVhSOlMk2tXR+fIKQExFrOuK9tbX3JIBUqItpI+HnAow509CnM134svw8PTFLkR6/CcMqnDfDK1m993PyoC1Y+N4X9XkhSmEQoAlAHPI5LHrvuujM13nvtoVYvKYoj7ScgumkpWNEvX652LfXOnKYlkB8ZybuxmFfIkzedQrbJsyOhfL03cMECAwEAATANBgkqhkiG9w0BAQUFAAOCAQEAeHwzqwnzGEkxjzSD47imXaTqtYyETZow7XwBc0ZaFS50qRFJUgKTAmKS1xQBP/qHpStsROT35DUxJAE6NY1Kbq3ZbCuhGoSlY0L7VzVT5tpu4EY8+Dq/u2EjRmmhoL7UkskvIZ2n1DdERtd+YUMTeqYl9co43csZwDno/IKomeN5qaPc39IZjikJ+nUC6kPFKeu/3j9rgHNlRtocI6S1FdtFz9OZMQlpr0JbUt2T3xS/YoQJn6coDmJL5GTiiKM6cOe+Ur1VwzS1JEDbSS2TWWhzq8ojLdrotYLGd9JOsoQhElmz+tMfCFQUFLExinPAyy7YHlSiVX13QH2XTu/iQQ==</X509Certificate>
        </X509Data>
    </KeyInfo>
</Signature>
<saml:Subject>
    <saml:NameID Format="urn:oasis:names:tc:SAML:1.1:nameid-format:unspecified">ilangoc@perillonworkspace.com</saml:NameID>
    <saml:SubjectConfirmation Method="urn:oasis:names:tc:SAML:2.0:cm:bearer">
        <saml:SubjectConfirmationData NotOnOrAfter="2016-05-05T22:14:48.906Z" Recipient="http://192.168.33.66/api/v0/e/cd/saml/consume" InResponseTo="5e0b9802-1306-11e6-9037-080027c97816"/>
    </saml:SubjectConfirmation>
</saml:Subject>
<saml:Conditions NotBefore="2016-05-05T21:14:48.906Z" NotOnOrAfter="2016-05-05T22:14:48.906Z">
    <saml:AudienceRestriction>
        <saml:Audience>http://example.com/saml/acs/example</saml:Audience>
    </saml:AudienceRestriction>
</saml:Conditions>
<saml:AttributeStatement xmlns:xs="http://www.w3.org/2001/XMLSchema" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance">
<saml:Attribute Name="urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress">
    <saml:AttributeValue xsi:type="xs:anyType">ilangoc@perillonworkspace.com</saml:AttributeValue>
</saml:Attribute>
<saml:Attribute Name="Email">
    <saml:AttributeValue xsi:type="xs:anyType">ilangoc@perillonworkspace.com</saml:AttributeValue>
</saml:Attribute>
<saml:Attribute Name="FirstName">
    <saml:AttributeValue xsi:type="xs:anyType">Ilango</saml:AttributeValue>
</saml:Attribute>
<saml:Attribute Name="LastName">
    <saml:AttributeValue xsi:type="xs:anyType">Chinnasamy</saml:AttributeValue>
</saml:Attribute>
</saml:AttributeStatement>
<saml:AuthnStatement AuthnInstant="2016-05-05T21:14:48.906Z">
    <saml:AuthnContext>
        <saml:AuthnContextClassRef>urn:oasis:names:tc:SAML:2.0:ac:classes:unspecified</saml:AuthnContextClassRef>
    </saml:AuthnContext>
</saml:AuthnStatement>
</saml:Assertion>
</samlp:Response>