krakenjs / passport-saml-encrypted

A strategy for Passport authentication that supports encrypted SAML responses
MIT License
14 stars 26 forks source link

Remove Signature element from the xml before verifying it #13

Open zccmg opened 8 years ago

zccmg commented 8 years ago

in the saml.js file you can see this code

sig.loadSignature(signature.toString());
return sig.checkSignature(xml);

if the signature element was not removed from the assertion the digest value will be different. so the correct code will be

sig.loadSignature(signature.toString());
//remove the signature element
doc.removeChild(signature);
return sig.checkSignature(doc.toString());
lmarkus commented 8 years ago

Is it? I haven't touched this in ages, but I believe that you need to check against the complete document, and the underlying function handled that logic.

Have you tried it in production? And if so, can you send a PR?

On Mar 31, 2016, at 1:32 AM, zccaliDev notifications@github.com<mailto:notifications@github.com> wrote:

in the saml.js file you can see this code

sig.loadSignature(signature.toString()); return sig.checkSignature(xml);

if the signature element was not removed from the assertion the digest value will be different. so the correct code will be

sig.loadSignature(signature.toString()); //remove the signature element doc.removeChild(signature); return sig.checkSignature(doc.toString());

You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHubhttps://github.com/lmarkus/passport-saml-encrypted/issues/13

zccmg commented 8 years ago

Yes, I tried. it's working 100%. I modified some other functionality, I will send the Pull

lmarkus commented 8 years ago

:+1:

markstos commented 8 years ago

I think @zccaliDev may be referring to this part of the XML signature spec in 6.6.3 XPath Filtering

Each signature must omit itself from its own digest calculations, but it is also necessary to exclude the second signature element from the digest calculations of the first signature so that adding the second signature does not break the first signature.

zccmg commented 8 years ago

May it look that, but when I debug the process of checking the Signature it won't remove the signature element and try to validate the data and it cause an error