You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@santuario.apache.org by Scott Cantor <ca...@osu.edu> on 2006/10/11 06:38:51 UTC
Possible signature verify bug?
Berin,
I think there's a really blatant bug in the C++ c14n code that's running
during signature verification. I think I only missed it before because I had
so much defensive code in place to output namespaces. I had a bug in my new
code that caused a namespace to be left off because the parent declared it,
and stumbled on to this test case.
I ran it through a vanilla test case that just parses the DOM and verifies
the signature, and the Reference isn't digesting properly. When I debugged
it, the bytes fed into the hash were missing a namespace declaration that
should have been pulled in from the parent of the node being referenced.
I've attached the test case, which is a nested SAML message with the
signature on the second-level element.
What's happening is that I have this declared (edited for brevity):
<samlp:ArtifactResponse xmlns:samlp="..." >
<samlp:Response xmlns:saml="...">
<saml:Issuer>
<saml:Assertion>
...
The signature references the <samlp:Response> by ID.
The bug is that the transform chain produces this:
<samlp:Response>
<saml:Issuer xmlns:saml="...">
<saml:Assertion xmlns:saml="...">
As you can see, xmlns:samlp isn't included from the parent/root element.
I think it should be, even though the reference is being transformed with
exclusive c14n. It's visibly used by the Response element, and so it should
get pulled in from the enclosing node. If I parse the DOM with that
namespace declaration manually added, the signature verifies, which tells me
that's the missing piece.
The document I attached verifies with the Java xmlsec code from what I can
tell. Oxygen is ok with it, anyway.
I haven't done any tests of the c14n engine by itself to produce test
output, but I would assume that's where the bug is.
-- Scott
Re: Possible signature verify bug?
Posted by Berin Lautenbach <be...@wingsofhermes.org>.
That definitely looks wrong. I will take a look. It's very wierd -
this is straight excl c14n and a very simple case. It's exactly the
sort of thing the basic test cases look at.
Cheers,
Berin
Scott Cantor wrote:
> Berin,
>
> I think there's a really blatant bug in the C++ c14n code that's running
> during signature verification. I think I only missed it before because I had
> so much defensive code in place to output namespaces. I had a bug in my new
> code that caused a namespace to be left off because the parent declared it,
> and stumbled on to this test case.
>
> I ran it through a vanilla test case that just parses the DOM and verifies
> the signature, and the Reference isn't digesting properly. When I debugged
> it, the bytes fed into the hash were missing a namespace declaration that
> should have been pulled in from the parent of the node being referenced.
>
> I've attached the test case, which is a nested SAML message with the
> signature on the second-level element.
>
> What's happening is that I have this declared (edited for brevity):
>
> <samlp:ArtifactResponse xmlns:samlp="..." >
> <samlp:Response xmlns:saml="...">
> <saml:Issuer>
> <saml:Assertion>
> ...
>
> The signature references the <samlp:Response> by ID.
>
> The bug is that the transform chain produces this:
>
> <samlp:Response>
> <saml:Issuer xmlns:saml="...">
> <saml:Assertion xmlns:saml="...">
>
> As you can see, xmlns:samlp isn't included from the parent/root element.
> I think it should be, even though the reference is being transformed with
> exclusive c14n. It's visibly used by the Response element, and so it should
> get pulled in from the enclosing node. If I parse the DOM with that
> namespace declaration manually added, the signature verifies, which tells me
> that's the missing piece.
>
> The document I attached verifies with the Java xmlsec code from what I can
> tell. Oxygen is ok with it, anyway.
>
> I haven't done any tests of the c14n engine by itself to produce test
> output, but I would assume that's where the bug is.
>
> -- Scott
>
>
> ------------------------------------------------------------------------
>
> <samlp:ArtifactResponse xmlns:samlp="urn:oasis:names:tc:SAML:2.0:protocol" ID="_e5e553f51fe1a009374bdf2186a685d8" IssueInstant="2006-10-11T03:12:59Z" Version="2.0"><samlp:Status><samlp:StatusCode Value="urn:oasis:names:tc:SAML:2.0:status:Success"/></samlp:Status><samlp:Response Destination="https://sp.example.org/SAML/POST" ID="rident" IssueInstant="1970-01-02T01:01:02.100Z" Version="2.0" xmlns:saml="urn:oasis:names:tc:SAML:2.0:assertion"><saml:Issuer>https://idp.example.org/</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="#rident">
> <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>U562AIbwQ8i5kQcxOjTfYAsqbOs=</ds:DigestValue>
> </ds:Reference>
> </ds:SignedInfo>
> <ds:SignatureValue>n5AGrOWy1WkZWaIVrXlr1iBeZ8YsGJLHsS+n472wmFxHn/GT8PsCDS78UjpIFVxY
> qK4G3O5p7rQwe8IGYeWeG3pjclvXcP6KH1CwjlJL3ndaZqu1tVdYqx3fbTAK5QRV
> gDUlyje2TRAgF1YSyyunRmJgOEXJ+JTe8brCmTrkr0E=</ds:SignatureValue>
> <ds:KeyInfo><ds:X509Data><ds:X509Certificate>MIICjzCCAfigAwIBAgIJAKk8t1hYcMkhMA0GCSqGSIb3DQEBBAUAMDoxCzAJBgNV
> BAYTAlVTMRIwEAYDVQQKEwlJbnRlcm5ldDIxFzAVBgNVBAMTDnNwLmV4YW1wbGUu
> b3JnMB4XDTA1MDYyMDE1NDgzNFoXDTMyMTEwNTE1NDgzNFowOjELMAkGA1UEBhMC
> VVMxEjAQBgNVBAoTCUludGVybmV0MjEXMBUGA1UEAxMOc3AuZXhhbXBsZS5vcmcw
> gZ8wDQYJKoZIhvcNAQEBBQADgY0AMIGJAoGBANlZ1L1mKzYbUVKiMQLhZlfGDyYa
> /jjCiaXP0WhLNgvJpOTeajvsrApYNnFX5MLNzuC3NeQIjXUNLN2Yo2MCSthBIOL5
> qE5dka4z9W9zytoflW1LmJ8vXpx8Ay/meG4z//J5iCpYVEquA0xl28HUIlownZUF
> 7w7bx0cF/02qrR23AgMBAAGjgZwwgZkwHQYDVR0OBBYEFJZiO1qsyAyc3HwMlL9p
> JpN6fbGwMGoGA1UdIwRjMGGAFJZiO1qsyAyc3HwMlL9pJpN6fbGwoT6kPDA6MQsw
> CQYDVQQGEwJVUzESMBAGA1UEChMJSW50ZXJuZXQyMRcwFQYDVQQDEw5zcC5leGFt
> cGxlLm9yZ4IJAKk8t1hYcMkhMAwGA1UdEwQFMAMBAf8wDQYJKoZIhvcNAQEEBQAD
> gYEAMFq/UeSQyngE0GpZueyD2UW0M358uhseYOgGEIfm+qXIFQF6MYwNoX7WFzhC
> LJZ2E6mEvZZFHCHUtl7mGDvsRwgZ85YCtRbvleEpqfgNQToto9pLYe+X6vvH9Z6p
> gmYsTmak+kxO93JprrOd9xp8aZPMEprL7VCdrhbZEfyYER0=</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="aident" IssueInstant="1970-01-02T01:01:02.100Z" Version="2.0"><saml:Issuer>https://idp.example.org/</saml:Issuer><saml:Subject><saml:NameID>John Doe</saml:NameID></saml:Subject><saml:AuthnStatement AuthnInstant="1970-01-02T01:01:02.100Z"><saml:AuthnContext><saml:AuthnContextClassRef>foo</saml:AuthnContextClassRef></saml:AuthnContext></saml:AuthnStatement></saml:Assertion></samlp:Response></samlp:ArtifactResponse>
Re: Possible signature verify bug?
Posted by Berin Lautenbach <be...@wingsofhermes.org>.
Scott Cantor wrote:
> If you mean the XML itself, you're welcome to that, and it shouldn't be that
> difficult to write a small test to just verify the signature in it.
Yup - just the XML. I have a simple harness that takes standalone XML
signature/encryption files and then calls the appropriate tool to
validate or decrypt the file.
And many thanks!
Cheers,
Berin
RE: Possible signature verify bug?
Posted by Scott Cantor <ca...@osu.edu>.
> Do you mind if I add the test case into our data set? I'd like to have
> it as one of the standard interop tests.
The code itself isn't a stand alone test, it's written on top of my library,
which doesn't directly expose the xmlsec APIs.
If you mean the XML itself, you're welcome to that, and it shouldn't be that
difficult to write a small test to just verify the signature in it.
-- Scott
Re: Possible signature verify bug?
Posted by Berin Lautenbach <be...@wingsofhermes.org>.
Do you mind if I add the test case into our data set? I'd like to have
it as one of the standard interop tests.
(And yes the fix will go into 1.3.1 :>.)
Cheers,
Berin
Scott Cantor wrote:
> Berin,
>
> The patch you sent fixes my test case, thanks.
>
> -- Scott
>
>
>
RE: Possible signature verify bug?
Posted by Scott Cantor <ca...@osu.edu>.
Berin,
The patch you sent fixes my test case, thanks.
-- Scott
RE: Possible signature verify bug?
Posted by Scott Cantor <ca...@osu.edu>.
> That's probably not explained it well - suffice to say the attached
> patch will add ancestor namespaces into the XPath nodeset that is
> handled by the canonicaliser.
I think I follow you. I'll pull the latest code and apply the patch and
report back if it doesn't work.
I'm assuming this can get slipped into the 1.3.1 release?
I haven't had time to do any automake build testing, but I'll try and get to
that.
-- Scott
Re: Possible signature verify bug?
Posted by Berin Lautenbach <be...@wingsofhermes.org>.
Scott,
Very interesting.
The issue is because you take an identifier (essentially a document
subset) and then apply an envelope transform. In dsig terms that means
there is an intersection of two nodesets, so the XPath handling kicks in
within the canonicaliser.
Unfortunately, there was a bug in the way the envelope transform built
its input nodeset from the document fragment. By definition, namespace
nodes promulgate from where they are defined into each child element
etc. DOM doesn't do that - it's a continual bugbear. So in XPath
terms, the namespace node that is in question *is* defined in the input
document fragment - even though in DOM terms it's defined earlier in the
document.
That's probably not explained it well - suffice to say the attached
patch will add ancestor namespaces into the XPath nodeset that is
handled by the canonicaliser.
Cheers,
Berin
Scott Cantor wrote:
> Berin,
>
> I think there's a really blatant bug in the C++ c14n code that's running
> during signature verification. I think I only missed it before because I had
> so much defensive code in place to output namespaces. I had a bug in my new
> code that caused a namespace to be left off because the parent declared it,
> and stumbled on to this test case.
>
> I ran it through a vanilla test case that just parses the DOM and verifies
> the signature, and the Reference isn't digesting properly. When I debugged
> it, the bytes fed into the hash were missing a namespace declaration that
> should have been pulled in from the parent of the node being referenced.
>
> I've attached the test case, which is a nested SAML message with the
> signature on the second-level element.
>
> What's happening is that I have this declared (edited for brevity):
>
> <samlp:ArtifactResponse xmlns:samlp="..." >
> <samlp:Response xmlns:saml="...">
> <saml:Issuer>
> <saml:Assertion>
> ...
>
> The signature references the <samlp:Response> by ID.
>
> The bug is that the transform chain produces this:
>
> <samlp:Response>
> <saml:Issuer xmlns:saml="...">
> <saml:Assertion xmlns:saml="...">
>
> As you can see, xmlns:samlp isn't included from the parent/root element.
> I think it should be, even though the reference is being transformed with
> exclusive c14n. It's visibly used by the Response element, and so it should
> get pulled in from the enclosing node. If I parse the DOM with that
> namespace declaration manually added, the signature verifies, which tells me
> that's the missing piece.
>
> The document I attached verifies with the Java xmlsec code from what I can
> tell. Oxygen is ok with it, anyway.
>
> I haven't done any tests of the c14n engine by itself to produce test
> output, but I would assume that's where the bug is.
>
> -- Scott
>
>
> ------------------------------------------------------------------------
>
> <samlp:ArtifactResponse xmlns:samlp="urn:oasis:names:tc:SAML:2.0:protocol" ID="_e5e553f51fe1a009374bdf2186a685d8" IssueInstant="2006-10-11T03:12:59Z" Version="2.0"><samlp:Status><samlp:StatusCode Value="urn:oasis:names:tc:SAML:2.0:status:Success"/></samlp:Status><samlp:Response Destination="https://sp.example.org/SAML/POST" ID="rident" IssueInstant="1970-01-02T01:01:02.100Z" Version="2.0" xmlns:saml="urn:oasis:names:tc:SAML:2.0:assertion"><saml:Issuer>https://idp.example.org/</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="#rident">
> <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>U562AIbwQ8i5kQcxOjTfYAsqbOs=</ds:DigestValue>
> </ds:Reference>
> </ds:SignedInfo>
> <ds:SignatureValue>n5AGrOWy1WkZWaIVrXlr1iBeZ8YsGJLHsS+n472wmFxHn/GT8PsCDS78UjpIFVxY
> qK4G3O5p7rQwe8IGYeWeG3pjclvXcP6KH1CwjlJL3ndaZqu1tVdYqx3fbTAK5QRV
> gDUlyje2TRAgF1YSyyunRmJgOEXJ+JTe8brCmTrkr0E=</ds:SignatureValue>
> <ds:KeyInfo><ds:X509Data><ds:X509Certificate>MIICjzCCAfigAwIBAgIJAKk8t1hYcMkhMA0GCSqGSIb3DQEBBAUAMDoxCzAJBgNV
> BAYTAlVTMRIwEAYDVQQKEwlJbnRlcm5ldDIxFzAVBgNVBAMTDnNwLmV4YW1wbGUu
> b3JnMB4XDTA1MDYyMDE1NDgzNFoXDTMyMTEwNTE1NDgzNFowOjELMAkGA1UEBhMC
> VVMxEjAQBgNVBAoTCUludGVybmV0MjEXMBUGA1UEAxMOc3AuZXhhbXBsZS5vcmcw
> gZ8wDQYJKoZIhvcNAQEBBQADgY0AMIGJAoGBANlZ1L1mKzYbUVKiMQLhZlfGDyYa
> /jjCiaXP0WhLNgvJpOTeajvsrApYNnFX5MLNzuC3NeQIjXUNLN2Yo2MCSthBIOL5
> qE5dka4z9W9zytoflW1LmJ8vXpx8Ay/meG4z//J5iCpYVEquA0xl28HUIlownZUF
> 7w7bx0cF/02qrR23AgMBAAGjgZwwgZkwHQYDVR0OBBYEFJZiO1qsyAyc3HwMlL9p
> JpN6fbGwMGoGA1UdIwRjMGGAFJZiO1qsyAyc3HwMlL9pJpN6fbGwoT6kPDA6MQsw
> CQYDVQQGEwJVUzESMBAGA1UEChMJSW50ZXJuZXQyMRcwFQYDVQQDEw5zcC5leGFt
> cGxlLm9yZ4IJAKk8t1hYcMkhMAwGA1UdEwQFMAMBAf8wDQYJKoZIhvcNAQEEBQAD
> gYEAMFq/UeSQyngE0GpZueyD2UW0M358uhseYOgGEIfm+qXIFQF6MYwNoX7WFzhC
> LJZ2E6mEvZZFHCHUtl7mGDvsRwgZ85YCtRbvleEpqfgNQToto9pLYe+X6vvH9Z6p
> gmYsTmak+kxO93JprrOd9xp8aZPMEprL7VCdrhbZEfyYER0=</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="aident" IssueInstant="1970-01-02T01:01:02.100Z" Version="2.0"><saml:Issuer>https://idp.example.org/</saml:Issuer><saml:Subject><saml:NameID>John Doe</saml:NameID></saml:Subject><saml:AuthnStatement AuthnInstant="1970-01-02T01:01:02.100Z"><saml:AuthnContext><saml:AuthnContextClassRef>foo</saml:AuthnContextClassRef></saml:AuthnContext></saml:AuthnStatement></saml:Assertion></samlp:Response></samlp:ArtifactResponse>