Repository navigation
signed assertion + signed message + "ds" prefix defined only at the root of the XML #84
Copy link
Copy link
Closed
Description
Hi guys,
I have a scenario where I need to validate a saml response with the following conditions:
- Assertion was signed
- Message (saml response) was signed too
- The "ds" prefix was defined only at the root of the XML (it's not in the Signature node)
Now, time to validate both signatures with the SignedXml class (sig.checkSignature):
- For the message signature validation, everything works as expected
- But for the assertion signature validation, I have the following error:
[ 'invalid signature: for uri #pfxabec2b1c-3915-1115-d115-6003c7afd15e calculated digest is X but the xml to validate supplies digest Y' ]
Do you know if this scenario is supported for your library? I mean signed assertion + signed message + "ds" prefix defined only at the root of the XML
<?xml version="1.0" encoding="UTF-8"?>
<samlp:Response xmlns:samlp="urn:oasis:names:tc:SAML:2.0:protocol" xmlns:ds="http://www.w3.org/2000/09/xmldsig#" xmlns:saml="urn:oasis:names:tc:SAML:2.0:assertion" xmlns:xs="http://www.w3.org/2001/XMLSchema" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" Destination="http://sp.example.com/demo1/index.php?acs" ID="pfx4b172e4e-9db1-5b0b-01e9-5f50031ab711" IssueInstant="2016-01-29T15:33:31Z" Version="2.0">
<saml:Issuer Format="urn:oasis:names:tc:SAML:2.0:nameid-format:entity">http://idp.example.com/metadata.php</saml:Issuer><ds:Signature>
<ds:SignedInfo><ds:CanonicalizationMethod Algorithm="http://www.w3.org/2001/10/xml-exc-c14n#"/>
<ds:SignatureMethod Algorithm="http://www.w3.org/2000/09/xmldsig#rsa-sha1"/>
<ds:Reference URI="#pfx4b172e4e-9db1-5b0b-01e9-5f50031ab711"><ds:Transforms><ds:Transform Algorithm="http://www.w3.org/2000/09/xmldsig#enveloped-signature"/><ds:Transform Algorithm="http://www.w3.org/2001/10/xml-exc-c14n#"/></ds:Transforms><ds:DigestMethod Algorithm="http://www.w3.org/2000/09/xmldsig#sha1"/><ds:DigestValue>v71AK2FW1QltA8vQ9EJi6OdpqFM=</ds:DigestValue></ds:Reference></ds:SignedInfo><ds:SignatureValue>nHNUcs5iE1upZT/bB7J08WyiOxsy0T89SBr3VC9r1wuQgvYM1fOXCuyYKgsEAlCunzs+EQBVxP3wdNghmrTwLv+3M3BeMIx3o7dfjNJcr9oL3K6JjtqlPMFuAoaEPyNukcLvXOsDjIj0XtbHuz8zL31fjKMkMYe0lQJolToqPDJVTi03u06Qv+24zAKL3R196p3HQzSvjpKvwQRvsLyn3U6kZe27xvz7dJK0kizbnLJWyaHnmOimuhg6pm9osWUzL3IFwngMXM/6XRNWKJEfRGkToy9FTqXjlgACVXK5NL9iIHlRO0zSbRepRZdpuEqSUg+Q5LuFNDeJWE2DZSV2qg==</ds:SignatureValue>
<ds:KeyInfo><ds:X509Data><ds:X509Certificate>MIIC7DCCAdSgAwIBAgIJOVW7hb0ZTkGPMA0GCSqGSIb3DQEBBQUAMB0xGzAZBgNVBAMTEmxvZ2luMC5teWF1dGgwLmNvbTAeFw0xNTEyMTEyMTQ5MTVaFw0yOTA4MTkyMDQ5MTVaMB0xGzAZBgNVBAMTEmxvZ2luMC5teWF1dGgwLmNvbTCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBAMvF/7JY9/G6edCE4qi44ODhlWoI71xVWflWNy8cRm8P6Wxgm2Z9AjJYsxsZJG2QecJeciX+WrkJ4nJxvb9CW4IIOhUy/BpMWxCVvSxLqKcvKxt0UI9jCThxTQZW7m0F9lhgXte0hr7Azr52wlUm9a5R2z5b1LtSDsLPQTwEWSCpVV4Hu1jZ5BsTbrF0poA+yMWvNScJjedKrMAG4C60WlqBULAMCO0LnHsGJgm+XFATy+XNRxFUiFJD2HkrhNlwrimx1EuKPqOMxMl+HOQOlMs+LVebvaJ+hZ1Xsi05dOPkSQc5QH8tFXAphecOtvDyZ583/qFgttLBtAK7f0Hn0JsCAwEAAaMvMC0wDAYDVR0TBAUwAwEB/zAdBgNVHQ4EFgQU3vBV4ENIG5bJIdd6dIuYZIUSyMkwDQYJKoZIhvcNAQEFBQADggEBABR47xeLlgmarCYcSFheV5N6DKcJT2grcKRHKUNAwbGnFZ1jmpZG8EYrQPpTRjFy82n1E6cxUlcBLUnlOYWpJ4VhmM1g+PJr+H3nCdcNCRgoPA2NyNqYmV1qr4vQHc3Iyja+UQDdFiWaMM+I11fy1uGKF345tZwBANl2tfENIjNVRXIEiPXr4xlGrZbuAoTHm+gv7GOcyqOikWj7N9smYTUyItwyzN+GO8vZeBmpiA18qQpXNOvXzFyT1/EBdQmIl4MFIUIQudUxObd0/K2IbJd6wXEzOUpfvmNuXmo+XtwLMQ3B7IpK/d/zHk8YMnxqA2fGoqpczNajshZ7gbPLvXM=</ds:X509Certificate></ds:X509Data></ds:KeyInfo></ds:Signature>
<samlp:Status>
<samlp:StatusCode Value="urn:oasis:names:tc:SAML:2.0:status:Success"/>
</samlp:Status>
<saml:Assertion ID="pfxabec2b1c-3915-1115-d115-6003c7afd15e" IssueInstant="2016-01-29T15:33:31Z" Version="2.0">
<saml:Issuer Format="urn:oasis:names:tc:SAML:2.0:nameid-format:entity">http://idp.example.com/metadata.php</saml:Issuer><ds:Signature>
<ds:SignedInfo><ds:CanonicalizationMethod Algorithm="http://www.w3.org/2001/10/xml-exc-c14n#"/>
<ds:SignatureMethod Algorithm="http://www.w3.org/2000/09/xmldsig#rsa-sha1"/>
<ds:Reference URI="#pfxabec2b1c-3915-1115-d115-6003c7afd15e"><ds:Transforms><ds:Transform Algorithm="http://www.w3.org/2000/09/xmldsig#enveloped-signature"/><ds:Transform Algorithm="http://www.w3.org/2001/10/xml-exc-c14n#"/></ds:Transforms><ds:DigestMethod Algorithm="http://www.w3.org/2000/09/xmldsig#sha1"/><ds:DigestValue>adowtTeoldhOVr2xyRL/wd9GssA=</ds:DigestValue></ds:Reference></ds:SignedInfo><ds:SignatureValue>Y25O9LZHY8ltQReTNPh9+Vc8tZpPcq/HyCl4a2v9I9D3RBSZEWNbdZXrJ40APm6ph+iGqZFoEw90jZo0S7UpKFB/vLT2rdigulBugQg7BPzgFN4LL690qdegUbApt1FQqnhPlcZQGAzODg6zDAyJ+GzDFxso8oqh6RoaZLWZAEvfnrlWakI2ammrBquI59XGn0mUBrT47K77NA/U6u3AJtACqpFFslcjR6yhjUssW102IRDxfxVy44bHslse0rHp553/DifyDFSc8pNiqtCbfy0XnKqzLY+YEn2h90BlGocI6Fg0tMQUjCc9y/lHoZ5XwKka1l2tVJGOrUPMLgFvBw==</ds:SignatureValue>
<ds:KeyInfo><ds:X509Data><ds:X509Certificate>MIIC7DCCAdSgAwIBAgIJOVW7hb0ZTkGPMA0GCSqGSIb3DQEBBQUAMB0xGzAZBgNVBAMTEmxvZ2luMC5teWF1dGgwLmNvbTAeFw0xNTEyMTEyMTQ5MTVaFw0yOTA4MTkyMDQ5MTVaMB0xGzAZBgNVBAMTEmxvZ2luMC5teWF1dGgwLmNvbTCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBAMvF/7JY9/G6edCE4qi44ODhlWoI71xVWflWNy8cRm8P6Wxgm2Z9AjJYsxsZJG2QecJeciX+WrkJ4nJxvb9CW4IIOhUy/BpMWxCVvSxLqKcvKxt0UI9jCThxTQZW7m0F9lhgXte0hr7Azr52wlUm9a5R2z5b1LtSDsLPQTwEWSCpVV4Hu1jZ5BsTbrF0poA+yMWvNScJjedKrMAG4C60WlqBULAMCO0LnHsGJgm+XFATy+XNRxFUiFJD2HkrhNlwrimx1EuKPqOMxMl+HOQOlMs+LVebvaJ+hZ1Xsi05dOPkSQc5QH8tFXAphecOtvDyZ583/qFgttLBtAK7f0Hn0JsCAwEAAaMvMC0wDAYDVR0TBAUwAwEB/zAdBgNVHQ4EFgQU3vBV4ENIG5bJIdd6dIuYZIUSyMkwDQYJKoZIhvcNAQEFBQADggEBABR47xeLlgmarCYcSFheV5N6DKcJT2grcKRHKUNAwbGnFZ1jmpZG8EYrQPpTRjFy82n1E6cxUlcBLUnlOYWpJ4VhmM1g+PJr+H3nCdcNCRgoPA2NyNqYmV1qr4vQHc3Iyja+UQDdFiWaMM+I11fy1uGKF345tZwBANl2tfENIjNVRXIEiPXr4xlGrZbuAoTHm+gv7GOcyqOikWj7N9smYTUyItwyzN+GO8vZeBmpiA18qQpXNOvXzFyT1/EBdQmIl4MFIUIQudUxObd0/K2IbJd6wXEzOUpfvmNuXmo+XtwLMQ3B7IpK/d/zHk8YMnxqA2fGoqpczNajshZ7gbPLvXM=</ds:X509Certificate></ds:X509Data></ds:KeyInfo></ds:Signature>
<saml:Subject>
<saml:NameID Format="urn:ibm:names:ITFIM:5.1:accessmanager">jdoe@us.example.org</saml:NameID>
<saml:SubjectConfirmation Method="urn:oasis:names:tc:SAML:2.0:cm:bearer">
<saml:SubjectConfirmationData NotOnOrAfter="2016-01-29T15:43:31Z" Recipient="http://sp.example.com/demo1/index.php?acs"/>
</saml:SubjectConfirmation>
</saml:Subject>
<saml:Conditions NotBefore="2016-01-29T15:32:31Z" NotOnOrAfter="2016-01-29T15:43:31Z">
<saml:AudienceRestriction>
<saml:Audience>http://sp.example.com/demo1/metadata.php</saml:Audience>
</saml:AudienceRestriction>
</saml:Conditions>
<saml:AuthnStatement AuthnInstant="2016-01-29T15:33:31Z" SessionIndex="_be9967abd904ddcae3c0eb4189adbe3f71e327cf93" SessionNotOnOrAfter="2016-01-30T04:33:30Z">
<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="EmailAddress" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:basic">
<saml:AttributeValue xsi:type="xs:string">jdoe@us.example.org</saml:AttributeValue>
</saml:Attribute>
<saml:Attribute Name="UserID" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:basic">
<saml:AttributeValue xsi:type="xs:string">4G1441786</saml:AttributeValue>
</saml:Attribute>
<saml:Attribute Name="FirstName" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:basic">
<saml:AttributeValue xsi:type="xs:string">jon</saml:AttributeValue>
</saml:Attribute>
<saml:Attribute Name="LastName" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:basic">
<saml:AttributeValue xsi:type="xs:string">doe</saml:AttributeValue>
</saml:Attribute>
</saml:AttributeStatement>
</saml:Assertion>
</samlp:Response>
Thanks!
Metadata
Metadata
Assignees
Labels
No labels
Activity
yaronn commentedon Jan 30, 2016
Can you try adding manually (or via code) the ds prefix to the saml (the root from which you validate)? This might make the large signature not valid but this is just for testing.Seems a similar issue to #48. Possibly when canonicalizing a namespace definition which is outside the signed element scope is ignored (though it's strange). You can also try to print the canonicalized xml to verify this.
bjrmatos commentedon Jan 30, 2016
@siacomuzzi i would like to help you to debug this scenario. i need three things (if possible)
if you can publish these three things behind a repository would be awesome!!
i think i know what is happening here..
siacomuzzi commentedon Jan 30, 2016
Thanks for the help guys. I will send you more information tomorrow. For now, let me show you the canonicalized xml for each case:
For the message signature validation (where everything works as expected), this is the canonicalized xml (signature is not present and
xmlns:dsdeclaration is at the root, like the original SAML Response):For the assertion signature validation, this is the canonicalized xml (signature node is present with the
xmlns:dsdeclaration):I tried (manually) removing the signature node and putting the
xmlns:ds="http://www.w3.org/2000/09/xmldsig#"at the root (<saml:Assertion>) but the digest error continues.bjrmatos commentedon Jan 30, 2016
according to the
InclusiveNamespacetransform i see that it definesPrefixList="xs saml xsi"but i only see thesamlnamespace in the canonical form of<saml:Assertion>.@yaronn as i say before i don't know much about inclusive namespaces but following your comments in #48 the
<saml:Assertion>tag need to have<saml:Assertion xmlns:xs="..." xmlns:saml="..." xmlns:xsi="..." ..other attributes..></saml:Assertion>am i wrong?yaronn commentedon Jan 30, 2016
@siacomuzzi - what do you mean that the signature node is not present? I see it in both cases. Please publish the original xml, and the canonicalized xml in both cases. Just to verify - did you try tto add the ds prefix definition on the saml:Assertion node in the original xml (not one which is an output of xml-crypto) and without deleting any other nodes?
@bjrmatos - exactly like you say, we need to identify which namespaces are in scope and add them (in case of inclusive namespace definition) via code or as a workaround for testing manually.
siacomuzzi commentedon Jan 31, 2016
I published the original xml in the issue's description and both canonicalized xmls in my last comment.
Please note that for the message signature validation (samlp:Response), the canonicalized xml does not contain the
<ds:Signature>node between</saml:Issuer>and<samlp:Status>, which is OK.But, for the assertion signature validation (
saml:Assertion), the canonicalized xml contains the<ds:Signature>nodesiacomuzzi commentedon Jan 31, 2016
Ok, validation is working if I perform the following 2 changes:
Add (manually) the
dsprefix definition on thesaml:Assertionnode in the original xml:Or via code (before call loadSignature/checkSignature methods):
Identify which namespaces are in scope and add them before canon xml generation:
bjrmatos commentedon Feb 1, 2016
so it is working, cool! now lets fix it in core.
can i ask you to setup a repository with these things? i would like to test it in my PC and find a solution in core.
siacomuzzi commentedon Feb 1, 2016
Sure, I'm on it.
siacomuzzi commentedon Feb 1, 2016
@bjrmatos let me know if this helps: https://gist.github.com/siacomuzzi/fbcc33183330b4b2f845
(just to simplify things, I have signed only the saml assertion)
bjrmatos commentedon Feb 2, 2016
@siacomuzzi thnk you! i will play with the example to see how we can fix this
bjrmatos commentedon Feb 2, 2016
@siacomuzzi the embeded xml in the gist is a bit different from what you have show to us.
it doesn't have a signature in
<samlp:Response>and the signature in saml assertion doesn't have aInclusiveNamespacestransform, anyway the gist always give me an invalid signature, it works for you? (i have uncommented the workaround code)bjrmatos commentedon Feb 2, 2016
the error i have is:
not related to the digest check, related to the signature check. the cert in the code is correct?
siacomuzzi commentedon Feb 2, 2016
As I said before, I signed only the saml assertion in order to reproduce only the first issue (
saml assertion signature has a prefix but namespace is defined on parent).You are right. I'll check the signature.
But, if you have commented the workaround code, the error is:
invalid signature: for uri #_In4F5FniO45yNLZU4kM7bm6NLtraUwsJ calculated digest is mXQ0tu6WeQNf3z3a8xDa3I43xArpx7YiN7Wj3o3Hgbs= but the xml to validate supplies digest Lbc/cRjr1V0HS6ZA5VO4yq7TMvH+H8yDjw2cSIG1OYc=When you uncomment the workaround code, the digest error is fixed (
validateReferencesmethod) and the problem is now with the signature value (validateSignatureValuemethod).bjrmatos commentedon Feb 2, 2016
yes! but i've found a solution that don't require any workaround.
i have updated
checkSignaturemethod to accept a node object instead of a plain xml string, so instead of doing thisvar valid = sig.checkSignature(assertion.toString());you can dovar valid = sig.checkSignature(assertion);. xml serialization inxmldomis too problematic when trying to serialize a child node (saml assertion node in this case), the child node ends up losing the context defined in any parent node1 remaining item
bjrmatos commentedon Feb 2, 2016
with this change the only error i'm receiving is about the invalid signature
siacomuzzi commentedon Feb 2, 2016
You are right! With your solution I can remove my workaround and validation works fine.
Sorry about the invalid signature error, since I can't share with you the real SAML Response, I'm trying to create a fake one with a script.
bjrmatos commentedon Feb 2, 2016
@siacomuzzi excellent news! one error fixed! lets go with the
InclusiveNamespaceerror.if you can share with me a gist like the previous one but with an xml with
InclusiveNamespacetransform i can fix it. (i'll publish all these changes when theInclusiveNamespaceerror is fixed)siacomuzzi commentedon Feb 2, 2016
Sure, I can prepare it for tomorrow. Thanks for the help!
siacomuzzi commentedon Feb 2, 2016
@bjrmatos the gist for the first issue was fixed and it's working with your solution (update SignedXml.prototype.checkSignature in order to support a xml node object)
I'm trying to reproduce the 2nd issue (
InclusiveNamespacetransform).bjrmatos commentedon Feb 2, 2016
@siacomuzzi cool! i will try it when i get back home
siacomuzzi commentedon Feb 2, 2016
The script for the 2nd issue (
InclusiveNamespacetransform) is available here.It's the same SAMLResponse I used for the 1st issue, but adding
<xc14n:InclusiveNamespaces xmlns:xc14n="http://www.w3.org/2001/10/xml-exc-c14n#" PrefixList="samlp xs saml xsi ds" />node before sign the assertion.bjrmatos commentedon Feb 2, 2016
@siacomuzzi thank you a lot! i will dive into it!
bjrmatos commentedon Feb 3, 2016
i see that the workaround for
InclusiveNamespacesdoesn't work here event ifsupported_definitionsis changed tobut if PrefixList is
"xs saml xsi"it works, right? (with the workaround)bjrmatos commentedon Feb 3, 2016
@siacomuzzi is there any possibility that you can see what is the content that gets hashed? (when signing the document) that would help to compare against the canonXML that we get in
xml-cryptolittleski commentedon Aug 1, 2017
The way i see it, in
SignedXml.prototype.getCanonXml
There is a :
var canonXml = nodewhich doesn't copy the node but copy the reference. Hence, when it's doing the Canonicalization, it modify the source XML as well.
Replacing it with something like
var canonXml = node.cloneNode(true);will do the trick and preserve the signature of the assertion within the message.
However, in the case of SAML2, for what i have been testing, it doesn't solve my issue since it seems that the signature for the signed message is invalid. What i did it is to stop using enveloped-signature as first canonicalization algorithm, this is more of a workaround, but works for simple SAML messages. If you wish to transfer very complex assertion, you will have to dig deeper to find the bug, will spend some time if i reach this point.
cmordue commentedon Dec 13, 2017
I ran into this and it turns out the lib I was following the old example on the README which was fixed here: #105
Once calling loadSignature with the node rather than string, this wasn't an issue for me.
dovsons commentedon Jan 1, 2018
I am looking for some help in same type of issue I am facing.
this what example i got as example,
Signature xmlns="http://www.w3.org/2000/09/xmldsig#">
saml:Issuerhttps://app.onelogin.com/saml/metadata/410172</saml:Issuer>
-<ds:Signature xmlns:ds="http://www.w3.org/2000/09/xmldsig#">
-<ds:SignedInfo>
<ds:CanonicalizationMethod Algorithm="http://www.w3.org/2001/10/xml-exc-c14n#"/>
<ds:SignatureMethod Algorithm="http://www.w3.org/2000/09/xmldsig#rsa-sha1"/>
-<ds:Reference URI="#pfx23f713be-6f61-5d8b-0d9b-7334006ed717">
-ds:Transforms
<ds:Transform Algorithm="http://www.w3.org/2000/09/xmldsig#enveloped-signature"/>
<ds:Transform Algorithm="http://www.w3.org/2001/10/xml-exc-c14n#"/>
</ds:Transforms>
<ds:DigestMethod Algorithm="http://www.w3.org/2000/09/xmldsig#sha1"/>
ds:DigestValuexAKoCcIUspJ646Ec6mQIpNCoUW0=</ds:DigestValue>
</ds:Reference>
but while I am creating I am getting this value----
-
-
-
-
</Transform
my question is if it make any difference if ds: prefix not exist with signedinfo ?
Your help is appropriated thanks in advance.
cjbarth commentedon May 29, 2023
It appears that the simple cases of this bug are fixed and that there are some me-too posts. I'm going to close this. If there is a continuing issue, we'd love to see a PR with a test that fails so that we can more effectively work on a solution and make sure that it stays fixed.