Issue #206: Support signing AuthnRequests using the HTTP-POST Binding

[richardTowers]

This commit adds support for signing AuthnRequests in the SAML HTTP-POST
binding. In the POST Binding the signature sits inside the SAML message
(as opposed to the Redirect binding, where the signture lives in the
URL's query string).

This will help suppport identity providers that require signed
AuthnRequests over the HTTP-POST binding.

Two new configuration options have been added:

• digestAlgorithm: allows you to specify the digest algorithm for the
  signature.
• xmlSignatureTransforms: allows you to configure which XML transforms
  to use.

[markstos]

For further consideration, please reference the portions of the SAML spec that relate to the changes are making. Also, the branch needs to be updated to resolve conflicts with the current master.

[tusharbudhe0302]

Thanks @richardTowers . How I can get your version of this code ? I have "passport-saml": "^0.33.0"

These are missing from from my saml.js. Please confirm when you will release it ?

[markstos]

@tusharbudhe0302 If you want to help move this toward release, you can provide specific references to the part of the SAML spec that this pull request implements.

Should digestAlgorithm be named signatureDigestAlgorithm? Are there other digest algorithms that are used in SAML?

[markstos]

@richardTowers The docs need be clearer about the difference between the signatureAlgorithm and the digestAlgorithm The former is defined as:

"signature algorithm for signing requests"

While the latter is defined as:

"digest algorithm for signing requests"

And the values and defaults are exactly the same set of digest algorithms. If they are both digest algorithms for signing requests, how are they different?

[pzavolinsky]

@richardTowers @markstos any work is being done on this PR?

I'm by no means a SAML2 expert but for what is worth, the code looks good to me.

For further consideration, please reference the portions of the SAML spec that relate to the changes are making

Here my grain of sand, protocol-wise:
(from https://www.oasis-open.org/committees/download.php/35711/sstc-saml-core-errata-2.0-wd-06-diff.pdf)

<Signature> element in <AuthnRequest>

(2064) 3.4.1 Element <AuthnRequest>
...
(2075) The <AuthnRequest> message SHOULD be signed or otherwise authenticated and integrity protected
(2076) by the protocol binding used to deliver the message.
(2077) This message has the complex type AuthnRequestType, which extends RequestAbstractType...

(1521) 3.2.1 Complex Type RequestAbstractType
...
(1551) <ds:Signature> [Optional]
(1552)   An XML Signature that authenticates the requester and provides message integrity, as described
(1553) below and in Section 5.
...
(1560) Depending on the requirements of particular protocols or profiles, a SAML requester may often need to
(1561) authenticate itself, and message integrity may often be required. Authentication and message integrity
(1562) MAY be provided by mechanisms provided by the protocol binding (see [SAMLBind]). The SAML request
(1563) MAY be signed, which provides both authentication of the requester and message integrity.
(1564) If such a signature is used, then the <ds:Signature> element MUST be present, and the SAML 
(1565) responder MUST verify that the signature is valid (that is, that the message has not been tampered with)
(1566) in accordance with [XMLSig]...

<CanonicalizationMethod>

See:

(3075) 5.4.3 Canonicalization Method

<Transforms>

See:

(3080) 5.4.4 Transforms

Example:

See full example in the spec:

(3094) 5.4.6 Example

[markstos]

@pzavolinsky If you want to move this toward release, you could address the feedback I left about the PR back in February, 2018. Thanks.

[pzavolinsky]

Digest algorithm: http://docs.oasis-open.org/security/saml/Post2.0/sstc-saml-metadata-algsupport-v1.0-cs01.html#__RefHeading__5805_234507477

Also: https://github.com/yaronn/xml-crypto#api specifically:

addReference(xpath, [transforms], [digestAlgorithm])

Which produces a DigestMethod as per the SAML spec above.

[pzavolinsky]

Regarding the difference between the two: my understanding is that the digest allows you to verify the message integrity even if you lack the signing certificate. The default values look the same but they are not.

The digest algorithm is a simple hash function (e.g. sha1), while the signature algorithm combines the hash function and cryptographic signature used on the message (e.g. rsa-sha1).

Some info I've found on the subject:

• https://www.w3.org/TR/2001/PR-xmldsig-core-20010820/#sec-Overview
• https://ping.force.com/Support/PingFederate/SAML/What-is-the-Digest-Value-element-in-a-SAML-Assertion
• https://docs.intersystems.com/irislatest/csp/docbook/DocBook.UI.Page.cls?KEY=GSOAPSEC_dsig#GSOAPSEC_dsig_overview
• https://stackoverflow.com/questions/31358612/signaturemethod-algorithm-vs-digestmethod-algorithm-in-xml-signature

[richardTowers]

Hi folks, apologies for dropping this and disappearing. I'll try and make some time to rebase this onto master and address @markstos' review comments.

[raphaelurban]

Hi there. Is there a plan in which release this will be included? I just would like to know if I should change my current development to Redirect as this will still take some time or if this can be expected to be released soon. @richardTowers

[crutley]

Regarding the digestAlgorithm, from the SAML spec:

5.4.2 References
SAML assertions and protocol messages MUST supply a value for the ID attribute on the root element of the assertion or protocol message being signed. The assertion’s or protocol message's root element may or may not be the root element of the actual XML document containing the signed assertion or protocolmmessage (e.g., it might be contained within a SOAP envelope).
Signatures MUST contain a single ds:Reference containing a same-document reference to the ID attribute value of the root element of the assertion or protocol message being signed. For example, if the ID attribute value is "foo", then the URI attribute in the ds:Reference element MUST be "#foo".

Example:

<samlp:AuthnRequest xmlns:samlp="urn:oasis:names:tc:SAML:2.0:protocol"
                    ID="_84a005071becc4d0feed"
                    Version="2.0"
                    IssueInstant="2020-01-14T19:35:38.549Z"
                    ProtocolBinding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST"
                    AssertionConsumerServiceURL="https://mywebsite.com/saml2"
                    Destination="https://sso.myidp.com/idp/SSO.saml2">
  <saml:Issuer xmlns:saml="urn:oasis:names:tc:SAML:2.0:assertion">mywebsite.com</saml:Issuer>
  <samlp:NameIDPolicy xmlns:samlp="urn:oasis:names:tc:SAML:2.0:protocol"
                      Format="urn:oasis:names:tc:SAML:1.1:nameid-format:unspecified"
                      AllowCreate="true" />
  <samlp:RequestedAuthnContext xmlns:samlp="urn:oasis:names:tc:SAML:2.0:protocol"
                               Comparison="exact">
    <saml:AuthnContextClassRef xmlns:saml="urn:oasis:names:tc:SAML:2.0:assertion">urn:oasis:names:tc:SAML:2.0:ac:classes:PasswordProtectedTransport</saml:AuthnContextClassRef>
  </samlp:RequestedAuthnContext>
  <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="#_84a005071becc4d0feed">
        <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>FEfuENsmyiUGY0Mo6uvbhEtvbvg=</DigestValue>
      </Reference>
    </SignedInfo>
    <SignatureValue>
asdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdf
</SignatureValue>
  </Signature>
</samlp:AuthnRequest>

The Reference URI is the same as the AuthnRequest ID. This seems to be mandatory.

The W3 link from above implies the possibility/capability to include more References in the signature, although I'm not sure what they would be exactly:

[s05-11] Each Reference element includes the digest method and resulting digest value calculated over the identified data object. It also may include transformations that produced the input to the digest operation. A data object is signed by computing its digest value and a signature over that value.

also

[s05-08] This identification, along with the transforms, is a description provided by the signer on how they obtained the signed data object in the form it was digested (i.e. the digested content). The verifier may obtain the digested content in another method so long as the digest verifies. In particular, the verifier may obtain the content from a different location such as a local store than that specified in the URI.

Anyway I might propose that the digestAlgorithm be defined as something like:

The algorithm applied to data objects identified by a Reference

On a related note, PR #274 seems to address the same POST+signing issue, but this solution is potentially more correct by having a separate DigestAlgorithm.

[crutley]

I have tried testing the code submitted back in 2017, and it does add a signature to the AuthnRequest. However, my IdP (a PingFederate server) is still not accepting the request, apparently because the Signature is in the wrong order. It seems that, to be 100% correct, the signature should immediately follow the Issuer element, before other elements such as NameIDPolicy. See this link:

https://support.pingidentity.com/s/article/How-to-fix-Invalid-XML-of-Expected-element-Scoping-errorNEW

You can see this "error" reflected in my AuthnRequest above which was generated using the code from this PR.

If @richardTowers could help with this that would be excellent as this is all new to me. Regardless I will likely try looking into it myself as HTTP-POST signing is important functionality for me.

[raphaelurban]

It seems that, to be 100% correct, the signature should immediately follow the Issuer element, before other elements such as NameIDPolicy. See this link:

You are right here, it has to directly follow the Issuer tag. I can also try to support with general SAML questions and information, as I am creating an infrastructure (IdP and SPs) with SAML based on POST-Binding and had to study this functionality quite a bit.

[richardTowers]

👋 hello from the future!

I've rebased my work against current master, and the tests still seem to pass, so maybe this still works 🤷‍♂. I'm not working full time on a SAML solution these days, so I haven't re-tested the code against any third party code.

I've also attempted to make the description of digestAlgorithm a bit less confusing, but at the end of the day it's SAML, so 🤷‍♂.

If there are more changes needed I think it would be best for someone else to volunteer to take over on this PR, as it will probably take me at least another 3 years to respond to any feedback.

[richardTowers]

Regarding the order of the elements and the position of the signature (as @crutley mentions): I think this is something that needs handling in the lower level xml-crypto library.

As far as I can tell we just call sig.computeSignature(samlMessage), sig.getSignedXml() and it spits out "signed" XML - it doesn't let us control where the signature element appears.

[crutley]

I thought the same looking at the code here, but didn't want to say it! Anyway I took a look at xml-crypto fearing the worst, and it seems that it does support signature location selection!

• computeSignature(xml, [options]) - compute the signature of the given xml where:
  • xml - a string containing a xml document
  • options - an object with the following properties:
    • prefix - adds this value as a prefix for the generated signature tags
    • attrs - a hash of attributes and values attrName: value to add to the signature root node
    • location - customize the location of the signature, pass an object with a reference key which should contain a XPath expression to a reference node, an action key which should contain one of the following values: append, prepend, before, after
    • existingPrefixes - A hash of prefixes and namespaces prefix: namespace that shouldn't be in the signature because they already exist in the xml

So, I'm just now looking into XPath for the first time, so I'm not 100% sure on the best/most robust/most flexible method, but that line could look something like:

sig.computeSignature(samlMessage, {'location': {'reference': './samlp:AuthnRequest/saml:Issuer[1]', 'action': 'after'}});

At least, I'm guessing that 'after' is the 'action' we'd want.

I don't know however if AuthnRequest and Issuer are always prefixed by samlp: and saml:, respectively. It seems one could use something like substring-after() to help there but I'll have to look into that tomorrow.

[raphaelurban]

@crutley you are right.
The samlify package for setting up SPs and IDPs also uses this functionality:
location: { reference: "/*[local-name(.)='AuthnRequest']/*[local-name(.)='Issuer']", action: 'after' }

[richardTowers]

👍 - good spot!

I've added that to this PR and written a test that the signature ends up in the right place.

[crutley]

I've tested the newest commit against my company's PingFederate server. The HTTP-POST signing works, the signature is in the right place, and I'm getting a response back with attributes and everything! Now I just need to figure out how to get what I need out of the SAML response (edit: after fixing a dumb oversight on my part, it's all working!).

Thanks a ton @richardTowers, this has been a big help for me!

Hopefully the maintainers can get this code reviewed and merged so it can live its life in my project as a full-fledged citizen of npm, lol.

[markstos]

This PR now includes code, tests, docs, spec references and some peer review and testing as well. Looks good to me. Merging.

[dickiedyce]

@crutley - just wondering if I could drop you a mail about getting passport-saml to work with PingID... I'm having a hell of a time, and you are the only person I can find who admits to getting it working...

[crutley]

@dickiedyce Sorry for the late response, I'll send you an email.