[BUG] Invalid document signature after upgrading from 2.2.0 to 4.0.1

[ericwooley]

After upgrading from 2.2.0 -> 4.0.1, I am getting an error:

Error: Invalid document signature
    at SAML.validatePostResponseAsync (/Users/ericwooley/projects/sbx/api/dist/api.bundle.js:83155:23)
    at processTicksAndRejections (node:internal/process/task_queues:96:5)

according to https://www.samltool.com/validate_response.php, the saml response is valid.

To Reproduce

1. I setup an azure active directory saml app.
2. I upgraded the lib to 4.0.1
3. I get a saml response that throws the above error.
4. Here are all the settings and certs.

EDIT: Removed recreation just in case.

Expected behavior

I switched branches to the version running 2.2.0 and replayed the exact same post request, and everything worked as expeced, with a successful verification.

Environment

• Node.js version: 16.13.1
• passport-saml version: 4.0.1

[cjbarth]

Could you please create a PR with a test that fails so we can more easily have a look at this problem?

[srd90]

@ericwooley Based on the content of base64 encoded SAML response you added to issue report your IdP has signed assertion. Top level response is not signed.

Starting from 4.0.0 @node-saml/passport-saml requires by default signed top level response and signed assertion.

This issue is duplicate of node-saml/node-saml#211 (starting from @node-saml/passport-saml 4.0.0 core saml functionality is provided for @node-saml/passport-saml by @node-saml/node-saml) .

See comment node-saml/node-saml#211 (comment) for more information how to handle this case (this is related to following breaking changes: node-saml/node-saml#83 and node-saml/node-saml#177).

This is the content of your SAML auth response in more readable format:

echo -n 'PH...saml auth response from issue report...T4=' | base64 -d | xmllint --format -

Your personal informations are masked with ...:

<?xml version="1.0"?>
<samlp:Response xmlns:samlp="urn:oasis:names:tc:SAML:2.0:protocol" ID="_32b79587-ee4e-45d9-a445-d4ed3e1a6b1c" Version="2.0" IssueInstant="2022-11-21T23:38:14.014Z" Destination="...">
  <Issuer xmlns="urn:oasis:names:tc:SAML:2.0:assertion">...</Issuer>
  <samlp:Status>
    <samlp:StatusCode Value="urn:oasis:names:tc:SAML:2.0:status:Success"/>
  </samlp:Status>
  <Assertion xmlns="urn:oasis:names:tc:SAML:2.0:assertion" ID="_afa5e63d-649a-4d95-ba27-a980fe41bd00" IssueInstant="2022-11-21T23:38:14.014Z" Version="2.0">
    <Issuer>...</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/2001/04/xmldsig-more#rsa-sha256"/>
        <Reference URI="#_afa5e63d-649a-4d95-ba27-a980fe41bd00">
          <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/2001/04/xmlenc#sha256"/>
          <DigestValue>...</DigestValue>
        </Reference>
      </SignedInfo>
      <SignatureValue>...</SignatureValue>
      <KeyInfo>
        <X509Data>
          <X509Certificate>...</X509Certificate>
        </X509Data>
      </KeyInfo>
    </Signature>
    <Subject>
      <NameID Format="urn:oasis:names:tc:SAML:1.1:nameid-format:unspecified">...</NameID>
      <SubjectConfirmation Method="urn:oasis:names:tc:SAML:2.0:cm:bearer">
        <SubjectConfirmationData NotOnOrAfter="2022-11-22T00:38:13.811Z" Recipient="..."/>
      </SubjectConfirmation>
    </Subject>
    <Conditions NotBefore="2022-11-21T23:33:13.811Z" NotOnOrAfter="2022-11-22T00:38:13.811Z">
      <AudienceRestriction>
        <Audience>...</Audience>
      </AudienceRestriction>
    </Conditions>
    <AttributeStatement>
      <Attribute Name="http://schemas.microsoft.com/identity/claims/tenantid">
        <AttributeValue>...</AttributeValue>
      </Attribute>
      <Attribute Name="http://schemas.microsoft.com/identity/claims/objectidentifier">
        <AttributeValue>...</AttributeValue>
      </Attribute>
      <Attribute Name="http://schemas.microsoft.com/identity/claims/displayname">
        <AttributeValue>...</AttributeValue>
      </Attribute>
      <Attribute Name="http://schemas.microsoft.com/identity/claims/identityprovider">
        <AttributeValue>...</AttributeValue>
      </Attribute>
      <Attribute Name="http://schemas.microsoft.com/claims/authnmethodsreferences">
        <AttributeValue>http://schemas.microsoft.com/ws/2008/06/identity/authenticationmethod/password</AttributeValue>
        <AttributeValue>http://schemas.microsoft.com/ws/2008/06/identity/authenticationmethod/unspecified</AttributeValue>
      </Attribute>
      <Attribute Name="http://schemas.xmlsoap.org/ws/2005/05/identity/claims/givenname">
        <AttributeValue>...</AttributeValue>
      </Attribute>
      <Attribute Name="http://schemas.xmlsoap.org/ws/2005/05/identity/claims/surname">
        <AttributeValue>...</AttributeValue>
      </Attribute>
      <Attribute Name="http://schemas.xmlsoap.org/ws/2005/05/identity/claims/emailaddress">
        <AttributeValue>...</AttributeValue>
      </Attribute>
      <Attribute Name="http://schemas.xmlsoap.org/ws/2005/05/identity/claims/name">
        <AttributeValue>...</AttributeValue>
      </Attribute>
    </AttributeStatement>
    <AuthnStatement AuthnInstant="2022-11-21T22:35:58.022Z" SessionIndex="_afa5e63d-649a-4d95-ba27-a980fe41bd00">
      <AuthnContext>
        <AuthnContextClassRef>urn:oasis:names:tc:SAML:2.0:ac:classes:Password</AuthnContextClassRef>
      </AuthnContext>
    </AuthnStatement>
  </Assertion>
</samlp:Response>

[cjbarth]

Thanks for that extra research @srd90 . Unless we see a PR with a repro proving a bug actually exists, we'll close this issue.

[ericwooley]

Yes, I agree that this should be closed as not a bug.

Thank you sooo much for the thorough response @srd90. Do you mind letting me know what steps you took to figure that out? I see the problem, but I have no idea how you got from A -> B.

Maybe a debug mode that has more errors about why a document has an invalid signature? Perhaps that has bad security implications, or needs to be implemented at the node-saml layer?

I haven't familiarized myself that well with the inner workings of the codebase, but if that is something that would be interesting to you, I could take a swing at it.

[srd90]

Do you mind letting me know what steps you took to figure that out?

@ericwooley By connecting the dots:

• you mentioned that you are updating 2.2.0 -> 4.0.1
• there is previously mentioned breaking change in 4.0.0
• most IdPs sign top level document or assertion (but not both unless configured / requested to do so)
  • checking your authn response data verified situation

fwiw, you might want to enable audience verification. Expected audience defaults to value of issuer (@node-saml/node-saml 4.0.0 README.md).
Your IdP seems to be placing following value to Audience:

...
     <AudienceRestriction>
       <Audience>...</Audience>
     </AudienceRestriction>
...

so instead of setting audience to false consider setting value to "..."

EDIT 1: replaced value of Audience field with ... because author of this issue had decided Nov 26, 2022 to remove that information from original issue report (see "edited" information from original issue report).
EDIT 2: updated quoted text to match with new version of it. Author of the quoted text had updated Nov 24, 2022 that text at by changing one word to another.

[ericwooley]

@srd90

most IdPs sign top level document or assertion (but not both unless configured / requested to do so)

Got it, so it would seem azure active directory does not sign top level document by default. For anyone coming here from google or whatever, the option to sign response and assertion is under saml signing certificate options in the enterprise application config.

By connecting the dots:

you mentioned that you are updating 2.2.0 -> 4.0.1
there is previously mentioned breaking change in 4.0.0
most IdPs sign top level document or assertion (but not both unless configured / requested to do so)
checking your authn response data verified situation

That makes sense, thanks for helping out. I just poorly assumed that breaking changes wouldn't require sP config updates, but I see what happened now, and I should have paid more attention to those breaking change notes. Most of my test scenarios passed, just a few didn't, so I started going down the wrong rabbit holes.

And thank you for the tips on the config, I typically have several more options per client, I just widdled these options down the the most simple for trying to diagnose why this SAML response was being rejected.

[cjbarth]

@ericwooley We are trying to remove duplicate information from the passport-saml README that is properly maintained in the node-saml README. We are eager to have PRs to help with that process. If you discover something, please feel free to put up a PR to help us all out.

[srd90]

@ericwooley I thought a while about posting this but decided that this is good real life example what not to do with SAML authn response messages.

One should not/must not post signed authn response messages to any public forum or to any 3rd party services. Unsigned messages or messages which signature value is removed but rest of the information is not masked should not be posted either because those contains identification information ... assertions.

Furthermore unmasked authn response message contains all the information (even without writing that information explicitly open to issue) needed to replay that message against your system (i.e. to log in to your system via replay attack) just like you replayed (posted) that message to your callback (according to your issue report).

Based on your sample configuration you have not enabled validateInResponseTo functionality which would prevent replay attacks once authn response is consumed once or if it was not triggered at all by your currently running system instance.

Without validateInReponseTo validation authn response can be used for replay until NotOnOrAfter time is passed unless that functionality (notonorafter validation) is turned off. Sidenote: Lots of ADFS based integrations have just copy pasted unsecure example configuration from passport-saml's old ADFS documentation which suggests turning it off (there is lot of ADFS examples at SO / internet which suggest configuration which disables NotOnOrAfter validation).

Your authn response message was valid between

<Conditions NotBefore="2022-11-21T23:33:13.811Z" NotOnOrAfter="2022-11-22T00:38:13.811Z">

Based on github issue metadata you created / posted issue 2022-11-21T23:49:45Z (if I spotted correct timestamp from this github issue's HTML sources).

So timeline would/could be:

1. 2022-11-21T23:33:13.811Z <-- NotBefore
2. 2022-11-21T23:49:45Z <-- issue posted
3. 2022-11-22T00:38:13.811Z <-- NotOnOrAfter

i.e. authn response message would/could have been valid about 40min after you made it public by posting it to github.

That authn response's NotOnOrAfter has not been valid since 2022-11-22T00:38:13.811Z but it would still be usable for attackers. Attacker could post it to possible other SPs which use same IdP than your service (authn response signature would match with IdP's cetificate) and if some of those SPs are incorrecly configured (i.e. if those do not validate NotOnOrAfter or inResponseTo) attacker could gain authenticated session (due nature of SAML SSO login...authn response is like "authentication token" until it is not considered valid anymore). If your assertion would have been encrypted (i.e. if you would have configured your IdP to encrypt assertions) that example message would not be re-usable against other SPs (not even against incorrectly configured ones if those are not configured to use same private key to decrypt assertion).

Note: some IdP services use customer specific private keys to sign authn responses which narrows usability of possibly leaked authn response messages. I do not know how our IdP works.

[srd90]

In addition to previous message: At least passport-saml SP instances are at least partly "incorrectly configured" because Recipient is not validated ( #509 ) and Audience validation is turned off by default in older versions. Both - and especially Recipient validation - in other SPs would prevent re-using leaked unencrypted authn response messages to gain access to those SPs (in case they share same IdP with same IdP cert).

[ericwooley]

@srd90

Great looking out, and I appreciate the info.

In this case, I created an azure SAML sP/organization and user, specifically to post the info online. The service consuming the saml response was also running on a local dev environment for debugging purposes, and had no access to anything, nor anyone else info.

But it is good to keep that info in mind, especially when debugging these things with strangers on the internet.

[ericwooley]

@cjbarth

@ericwooley We are trying to remove duplicate information from the passport-saml README that is properly maintained in the node-saml README. We are eager to have PRs to help with that process. If you discover something, please feel free to put up a PR to help us all out.

One thing that keeps tripping me up, and expect it will trip up other people setting this up, is that wantAuthnResponseSigned is not mentioned as an option in the passport-saml repo. But is mentioned in the options for node-saml. wantAssertionsSigned is mentioned in the passport-saml repo though.

Not sure if that is intentional or not?

[cjbarth]

The README for passport-saml it out-of-date. We want to remove all the settings that are from node-saml from the passport-saml README so as to avoid such confusion. We are accepting PRs for this.