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>
Index: TXFMEnvelope.cpp
===================================================================
--- TXFMEnvelope.cpp    (revision 450386)
+++ TXFMEnvelope.cpp    (working copy)
@@ -27,8 +27,12 @@
 #include <xsec/framework/XSECException.hpp>
 #include <xsec/utils/XSECDOMUtils.hpp>
 
+#include <xercesc/util/XMLString.hpp>
+#include <xercesc/util/XMLUniDefs.hpp>
+
 XERCES_CPP_NAMESPACE_USE
 
+
 TXFMEnvelope::TXFMEnvelope(DOMDocument *doc) :
 TXFMBase(doc) {
 
@@ -152,7 +156,41 @@
        }
 }
 
+void addEnvelopeParentNSNodes(DOMNode *startNode, XSECXPathNodeList & 
XPathMap) {
 
+       XSEC_USING_XERCES(DOMNamedNodeMap);
+       
+       DOMNode *tmp;
+       DOMNamedNodeMap *atts;
+       int attsSize, i;
+       
+       if (startNode == NULL)
+               return;
+
+       if (startNode->getNodeType() == DOMNode::ELEMENT_NODE) {
+
+               atts = startNode->getAttributes();
+               if (atts != NULL)
+                       attsSize = atts->getLength();
+               else
+                       attsSize = 0;
+
+               for (i = 0; i < attsSize; ++i) {
+
+                       tmp = atts->item(i);
+                       if (XMLString::compareNString(tmp->getNodeName(), 
DSIGConstants::s_unicodeStrXmlns, 5) == 0 &&
+                               (tmp->getNodeName()[5] == chNull || 
tmp->getNodeName()[5] == chColon))
+                               XPathMap.addNode(tmp);
+
+               }
+
+       }
+
+       // Now do parent
+       addEnvelopeParentNSNodes(startNode->getParentNode(), XPathMap);
+       
+}
+
 void TXFMEnvelope::evaluateEnvelope(DOMNode *t) {
 
        DOMNode *sigNode;
@@ -182,6 +220,7 @@
        }
 
        addEnvelopeNode(mp_startNode, m_XPathMap, sigNode);
+       addEnvelopeParentNSNodes(mp_startNode->getParentNode(), m_XPathMap);
 
 }
 

Reply via email to