Bug in SAML2Signature in PicketLInk 2.1.4 (and 2.0.x).

[magsy]

Please consider:

SAML2Signature samlSignature = new SAML2Signature();

KeyPair keypair = keyManager.getSigningKeyPair();

samlSignature.signSAMLDocument(samlRequest, keypair);

This does not produce a valid signed AuthnRequestt. The issue is best explained by looking at the end of the XML message:

</dsig:RSAKeyValue></dsig:KeyValue></dsig:KeyInfo></dsig:Signature></samlp:AuthnRequest>

The <dsig:Signature> was inserted at the end of the elements under the root node, yet it should be placed between samlp:Issuer and samlp:NameID.

The problem is probably something to do with this line from XMLSignatureUtil.sign:

DOMSignContext dsc = new DOMSignContext(signingKey, doc.getDocumentElement());

John

[pcraveiro]

Hi Jonh,

    The test case org.picketlink.test.identity.federation.web.saml.handlers.SAML2SignatureHandlerUnitTestCase is used to test the handler responsible for signing saml request and response. This handler uses the SAML2Signature to do that.

    When running this test the signature is properly generated.

<saml:Issuer xmlns:saml="urn:oasis:names:tc:SAML:2.0:assertion">http://localhost:8080/idp/</saml:Issuer>

<dsig:Signature xmlns:dsig="http://www.w3.org/2000/09/xmldsig#">

    ....

</dsig:Signature>

<samlp:NameIDPolicy

                    AllowCreate="true"

                    Format="urn:oasis:names:tc:SAML:2.0:nameid-format:transient"/>

    I think you should try that:

        SAML2Signature samlSignature = new SAML2Signature();

        Node nextSibling = samlSignature.getNextSiblingOfIssuer(samlDocument);

        samlSignature.setNextSibling(nextSibling); // ADD THIS TO YOUR CODE. It will make the signature go after the Issuer.

        samlSignature.signSAMLDocument(samlDocument, keypair);

Regards.
Pedro Igor

[magsy]

Pedro

Your suggestion means the code calling signSAMLDocument is unnecessarily complicated. I would suggest that the method could be improved to figure out where the signature should be placed to avoid returning an invalid Document.

The test only works because it's been written to meet the unclear behaviour of the method.

John

[pcraveiro]

Hi John,

    The SAML2Signature provides others methods for signing without the need to set the sibling node. I think that you are looking for something like the SAML2Signature.sign(RequestAbstractType request, KeyPair keypair).

      But with a DOM Document instance as an argument. Am I right ?

Regards.

Pedro Igor