search

perl-net-saml2 / perl-XML-Sig

XML::Sig - A Perl toolkit to help sign and verify XML Digital Signatures.
https://metacpan.org/pod/XML::Sig
1 stars 2 forks source link

XML::Sig disagrees with SamlTool.com #5

Closed philiprbrenan closed 3 years ago

philiprbrenan commented 3 years ago

Please see the test in the attached zip file:

test.zip

https://www.samltool.com/validate_response.php claims that the signed.xml is a valid SAML response. However, XML::Sig claims that it is not.

image

Please tell me which of the two is correct?

philiprbrenan commented 3 years ago

Timing issue!

philiprbrenan commented 3 years ago

I have fixed the timing issue but the discrepancy between XML::Sig and https://www.samltool.com/validate_response.php still exists. Please tell me which is correct?

Test case using XML::Sig - test.zip which says the signature is invalid.

Screenshot of saml tool which claims the signature is valid.

image

timlegge commented 3 years ago

I will take a look, I am working on something else tonight so it might be this weekend

timlegge commented 3 years ago

Problem appears to be related to XML::CanonicalizeXML looking now

Ignore this update I am working through the issue

timlegge commented 3 years ago

I have not found the issue yet. It would be useful to know what generated the signed xml. I have found another issue though with signing. The unsigned.xml that yu have in the zip does not appear to be the same as the signed. Can you generate a new unsigned and signed.

philiprbrenan commented 3 years ago

The XML was signed with:

https://www.samltool.com/sign_response.php

timlegge commented 3 years ago

The XML was signed with:

https://www.samltool.com/sign_response.php

Which mode? Sign assertion I assume?

philiprbrenan commented 3 years ago

I have repeated the process with the same result. The signed XML is included in test.pl - the signed XML contains the public key to verify with. The screenshot shows that https://www.samltool.com/sign_response.php believes the XML is valid. xmlSig.zip

However, XML::Sig->verify returns false indicating that the XML is not valid.

The XML being signed is a SAML assertion. Only the assertion is being signed - not the whole document. Possibly this is the reason for the difference?

philiprbrenan commented 3 years ago

Success! xmlSig.zip

In this version I had:

https://capriza.github.io/samling/samling.html

sign the entire message as well as the SAML assertion. Now:

https://www.samltool.com/validate_response.php

says the XML is a valid signed SAML response per the attached screenshot.

XML::Sig now agrees. Possibly XML::Sig is validating the signature for the entire document but not the signature for the contained SAML assertion?

If this is the case: might it be worth noting this difference in the documentation for XML::Sig? Might it be worth adding a check to see if the XML contains a SAML response - and - if it does - perhaps a message from XML::Sig saying that it has checked the signature of the entire document but not the signature of the contained SAML assertion would help reduce the confusion that can so easily arise in this case as to what exactly is being checked?

timlegge commented 3 years ago

Thanks, that should help. I have seen a few issues in XML::Sig as I review and I am going to sort it out but it might take a bit. However, knowing that the entire document works will help me track it down.

timlegge commented 3 years ago

Just a update, I am making some progress but it will be a little longer

philiprbrenan commented 3 years ago

Jolly good!

On Wed, Nov 25, 2020 at 12:19 PM Timothy Legge notifications@github.com wrote:

Just a update, I am making some progress but it will be a little longer

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/perl-net-saml2/perl-XML-Sig/issues/5#issuecomment-733673604, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABZS3WXK2RD6ADH5MAZVSITSRTY67ANCNFSM4T4AN6IQ .

-- Thanks,

Phil https://metacpan.org/author/PRBRENAN

Philip R Brenan https://metacpan.org/author/PRBRENAN

timlegge commented 3 years ago

Hi

This is currently very rough and likely breaks signing but it does fix a number of the files you were having and issue with.

XML-Sig.zip

It currently spews a lot of debuging info and I need to cleanup and fix some of the way I did a few things. Goes without saying that it not ready for prod but should allow you to test some signed XML.

timlegge commented 3 years ago

So far it has validated everything I have thrown at it, More testing, refactoring and signing changes to come.

timlegge commented 3 years ago

Hi

This was a little more work than I thought. There were issues with the way that XML::Sig was doing signing and verifying with respect to canonization. In general some documents would not be canonized correctly. Most documents would not be affected but if the document had something that the correct canonization method would have changed it would fail validation.

I am close to releaseing 0.29-TRIAL which should address all validation issues that you saw. https://github.com/perl-net-saml2/perl-XML-Sig/tree/validation-issues has the fixes.

Tim