You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@santuario.apache.org by Samuel Meder <me...@mcs.anl.gov> on 2004/11/13 00:46:14 UTC
Problems with exclusive c14n and default namespaces
We just upgraded to the xmlsec 1.2 RC1 release and started noticing
problems with the exclusive c14n implementation. In particular SAML
assertion signature validation started to fail after the assertion was
sent over the wire (using axis).
The assertion when initially generated declared several redundant
default namespaces on the elements which Axis subsequently removed when
sending the actual message. Furthermore, the code lists #default in the
inclusive namespaces, so as far as I can tell this should make it ok for
axis to remove redundant NS declarations.
Now in particular, I'm seeing the following problems with the code:
* If the element being processed lacks a default ns declaration and does
not use a prefix then xmlns="" is added unconditionally even if the
element is declared in the default namespace and the immediate parent
element defines a default NS.
* CanonicalizerBase.java has the following code:
Element currentElement = (Element) currentNode;
OutputStream writer=this._writer;
String tagName=currentElement.getTagName();
if (currentNodeIsVisible) {
//This is an outputNode.
ns.outputNodePush();
writer.write('<');
writeStringToUtf8(tagName,writer);
} else {
//Not an outputNode.
ns.push();
}
// we output all Attrs which are available
Iterator attrs = handleAttributes(currentElement,ns);
while (attrs.hasNext()) {
Attr attr = (Attr) attrs.next();
outputAttrToWriter(attr.getNodeName(), attr.getNodeValue(), writer);
}
i.e. attributes are printed even if the element is not visible. This
seems to result in spurious xmlns="" being output in random places.
Maybe this code:
Iterator it=this._inclusiveNSSet.iterator();
while (it.hasNext()) {
String s=(String)it.next();
Attr key=ns.getMappingWithoutRendered(s);
if (key==null) {
continue;
}
result.add(key);
}
in Canonicalizer20010315Excl.java and the fact that the empty default
map seems to be added to ns (why is that done btw? Seems wrong to me) by
default has something to do with it?
The inclusive code seems to behave a little better (not seeing
validation failures anymore), but I am a bit skeptical about the
following:
if (isRealVisible) {
//The element is visible, handle the xmlns definition
Attr xmlns = E.getAttributeNodeNS(Constants.NamespaceSpecNS, "xmlns");
Node n=null;
if (xmlns == null) {
//No xmlns def just get the already defined.
n=ns.getMapping(XMLNS);
} else if ( !this._xpathNodeSet.contains(xmlns)) {
//There is a definition but the xmlns is not selected by the xpath.
//then xmlns=""
n=ns.addMappingAndRenderXNodeSet(XMLNS,"",nullNode,true);
}
//output the xmlns def if needed.
if (n!=null) {
result.add(n);
}
//Float all xml:* attributes of the unselected parent elements to this one.
addXmlAttributes(E,result);
}
Shouldn't the xmlns="" mapping only be added if this mapping is in scope
and the ancestor element declaring the mapping is not visible (and since
NameSpaceSymbolTable always adds that mapping...). Also I'm not quite
sure that I understand the "else if":
CanonicalizerBase:canonicalizeXPathNodeSet seems to throw a exception if
it finds a attribute node in the _xpathNodeSet which would lead me to
assume that the node set would never contain the xmlns node?
Might also be that I am completely misunderstanding things.
Canonicalization makes my brain hurt...
/Sam
Re: Problems with exclusive c14n and default namespaces
Posted by Samuel Meder <me...@mcs.anl.gov>.
On Wed, 2004-11-17 at 22:43 -0600, Samuel Meder wrote:
> This may very well have fixed one of the problems, but it still did not
> fix the signature mismatch error I'm getting due to c14n. I'll follow up
> with a lot more detail in a bit.
I've attached the following:
assertion_service.xml - the signed saml assertion as created by the
service before sending using axis
assertion_client.xml - the assertion as received by the client
assertion_transformed_server.xml - the assertion on service with all
transforms applied
assertion_transformed_client.xml - the assertion on client with all
transforms applied
The assertions were dumped using:
try
{
System.out.println("After Transforms:\n\n");
System.out.println(new String(sig.getSignedInfo().getReferencedContentAfterTransformsItem(0).getBytes()));
}
catch(IOException e)
{
}
Obviously something is still very wrong.
/Sam
> /Sam
>
> On Mon, 2004-11-15 at 21:26 +0100, Raul Benito wrote:
> > Raul Benito wrote:
> >
> > >
> > >>
> > >>
> > >> Shouldn't the xmlns="" mapping only be added if this mapping is in scope
> > >> and the ancestor element declaring the mapping is not visible (and since
> > >> NameSpaceSymbolTable always adds that mapping...). Also I'm not quite
> > >> sure that I understand the "else if":
> > >> CanonicalizerBase:canonicalizeXPathNodeSet seems to throw a exception if
> > >> it finds a attribute node in the _xpathNodeSet which would lead me to
> > >> assume that the node set would never contain the xmlns node?
> > >>
> > >> Might also be that I am completely misunderstanding things.
> > >> Canonicalization makes my brain hurt...
> > >>
> > >>
> > >>
> > >
> > Here it is the patch that I think fix your problem(if you don't know or
> > is difficult to you to apply the patch ask me privately and I can
> > provide you a compiled jar). It is not the deffenitive one(the attr
> > casting are redundant for the HEAD tree) as I'm currently working with a
> > different thing but it will work for you.
> > Regards,
> >
> > Raul
> > http://r-bg.com
> >
> > Index: Canonicalizer20010315Excl.java
> > ===================================================================
> > RCS file:
> > /home/cvs/xml-security/src/org/apache/xml/security/c14n/implementations/Canonicalizer20010315Excl.java,v
> > retrieving revision 1.17
> > diff -u -r1.17 Canonicalizer20010315Excl.java
> > --- Canonicalizer20010315Excl.java 24 Sep 2004 20:54:29 -0000 1.17
> > +++ Canonicalizer20010315Excl.java 15 Nov 2004 20:22:50 -0000
> > @@ -165,7 +165,7 @@
> > Iterator it=visiblyUtilized.iterator();
> > while (it.hasNext()) {
> > String s=(String)it.next();
> > - Attr key=ns.getMapping(s);
> > + Attr key=(Attr)ns.getMapping(s);
> > if (key==null) {
> > continue;
> > }
> > @@ -262,7 +262,7 @@
> > }
> > }
> > }
> > - if (!xmlnsDef ) {
> > + if (isOutputElement && !xmlnsDef ) {
> > ns.addMapping(XMLNS,"",nullNode);
> > }
> >
> > @@ -282,7 +282,7 @@
> > Iterator it=visiblyUtilized.iterator();
> > while (it.hasNext()) {
> > String s=(String)it.next();
> >
> > - Attr key=ns.getMapping(s);
> > + Attr key=(Attr)ns.getMapping(s);
> > if (key==null) {
> > continue;
> > }
> > @@ -292,7 +292,7 @@
> > Iterator it=this._inclusiveNSSet.iterator();
> > while (it.hasNext()) {
> > String s=(String)it.next();
> > - Attr key=ns.getMappingWithoutRendered(s);
> > + Attr key=(Attr)ns.getMappingWithoutRendered(s);
> > if (key==null) {
> > continue;
> > }
> >
> >
>
Re: Problems with exclusive c14n and default namespaces
Posted by Samuel Meder <me...@mcs.anl.gov>.
This may very well have fixed one of the problems, but it still did not
fix the signature mismatch error I'm getting due to c14n. I'll follow up
with a lot more detail in a bit.
/Sam
On Mon, 2004-11-15 at 21:26 +0100, Raul Benito wrote:
> Raul Benito wrote:
>
> >
> >>
> >>
> >> Shouldn't the xmlns="" mapping only be added if this mapping is in scope
> >> and the ancestor element declaring the mapping is not visible (and since
> >> NameSpaceSymbolTable always adds that mapping...). Also I'm not quite
> >> sure that I understand the "else if":
> >> CanonicalizerBase:canonicalizeXPathNodeSet seems to throw a exception if
> >> it finds a attribute node in the _xpathNodeSet which would lead me to
> >> assume that the node set would never contain the xmlns node?
> >>
> >> Might also be that I am completely misunderstanding things.
> >> Canonicalization makes my brain hurt...
> >>
> >>
> >>
> >
> Here it is the patch that I think fix your problem(if you don't know or
> is difficult to you to apply the patch ask me privately and I can
> provide you a compiled jar). It is not the deffenitive one(the attr
> casting are redundant for the HEAD tree) as I'm currently working with a
> different thing but it will work for you.
> Regards,
>
> Raul
> http://r-bg.com
>
> Index: Canonicalizer20010315Excl.java
> ===================================================================
> RCS file:
> /home/cvs/xml-security/src/org/apache/xml/security/c14n/implementations/Canonicalizer20010315Excl.java,v
> retrieving revision 1.17
> diff -u -r1.17 Canonicalizer20010315Excl.java
> --- Canonicalizer20010315Excl.java 24 Sep 2004 20:54:29 -0000 1.17
> +++ Canonicalizer20010315Excl.java 15 Nov 2004 20:22:50 -0000
> @@ -165,7 +165,7 @@
> Iterator it=visiblyUtilized.iterator();
> while (it.hasNext()) {
> String s=(String)it.next();
> - Attr key=ns.getMapping(s);
> + Attr key=(Attr)ns.getMapping(s);
> if (key==null) {
> continue;
> }
> @@ -262,7 +262,7 @@
> }
> }
> }
> - if (!xmlnsDef ) {
> + if (isOutputElement && !xmlnsDef ) {
> ns.addMapping(XMLNS,"",nullNode);
> }
>
> @@ -282,7 +282,7 @@
> Iterator it=visiblyUtilized.iterator();
> while (it.hasNext()) {
> String s=(String)it.next();
>
> - Attr key=ns.getMapping(s);
> + Attr key=(Attr)ns.getMapping(s);
> if (key==null) {
> continue;
> }
> @@ -292,7 +292,7 @@
> Iterator it=this._inclusiveNSSet.iterator();
> while (it.hasNext()) {
> String s=(String)it.next();
> - Attr key=ns.getMappingWithoutRendered(s);
> + Attr key=(Attr)ns.getMappingWithoutRendered(s);
> if (key==null) {
> continue;
> }
>
>
Re: Problems with exclusive c14n and default namespaces
Posted by Raul Benito <ra...@r-bg.com>.
Raul Benito wrote:
>
>>
>>
>> Shouldn't the xmlns="" mapping only be added if this mapping is in scope
>> and the ancestor element declaring the mapping is not visible (and since
>> NameSpaceSymbolTable always adds that mapping...). Also I'm not quite
>> sure that I understand the "else if":
>> CanonicalizerBase:canonicalizeXPathNodeSet seems to throw a exception if
>> it finds a attribute node in the _xpathNodeSet which would lead me to
>> assume that the node set would never contain the xmlns node?
>>
>> Might also be that I am completely misunderstanding things.
>> Canonicalization makes my brain hurt...
>>
>>
>>
>
Here it is the patch that I think fix your problem(if you don't know or
is difficult to you to apply the patch ask me privately and I can
provide you a compiled jar). It is not the deffenitive one(the attr
casting are redundant for the HEAD tree) as I'm currently working with a
different thing but it will work for you.
Regards,
Raul
http://r-bg.com
Index: Canonicalizer20010315Excl.java
===================================================================
RCS file:
/home/cvs/xml-security/src/org/apache/xml/security/c14n/implementations/Canonicalizer20010315Excl.java,v
retrieving revision 1.17
diff -u -r1.17 Canonicalizer20010315Excl.java
--- Canonicalizer20010315Excl.java 24 Sep 2004 20:54:29 -0000 1.17
+++ Canonicalizer20010315Excl.java 15 Nov 2004 20:22:50 -0000
@@ -165,7 +165,7 @@
Iterator it=visiblyUtilized.iterator();
while (it.hasNext()) {
String s=(String)it.next();
- Attr key=ns.getMapping(s);
+ Attr key=(Attr)ns.getMapping(s);
if (key==null) {
continue;
}
@@ -262,7 +262,7 @@
}
}
}
- if (!xmlnsDef ) {
+ if (isOutputElement && !xmlnsDef ) {
ns.addMapping(XMLNS,"",nullNode);
}
@@ -282,7 +282,7 @@
Iterator it=visiblyUtilized.iterator();
while (it.hasNext()) {
String s=(String)it.next();
- Attr key=ns.getMapping(s);
+ Attr key=(Attr)ns.getMapping(s);
if (key==null) {
continue;
}
@@ -292,7 +292,7 @@
Iterator it=this._inclusiveNSSet.iterator();
while (it.hasNext()) {
String s=(String)it.next();
- Attr key=ns.getMappingWithoutRendered(s);
+ Attr key=(Attr)ns.getMappingWithoutRendered(s);
if (key==null) {
continue;
}
Re: Problems with exclusive c14n and default namespaces
Posted by Samuel Meder <me...@mcs.anl.gov>.
On Mon, 2004-11-15 at 20:50 +0100, Raul Benito wrote:
> Samuel Meder wrote:
>
> >We just upgraded to the xmlsec 1.2 RC1 release and started noticing
> >problems with the exclusive c14n implementation. In particular SAML
> >assertion signature validation started to fail after the assertion was
> >sent over the wire (using axis).
> >
> >
> First of all thanks for the bug report. You have hit the code that is
> not very well exercised by the test suite, so thanks a lot. Second you
> are using the slow path (i.e. using xpath transformations) and that's good.
>
> >The assertion when initially generated declared several redundant
> >default namespaces on the elements which Axis subsequently removed when
> >sending the actual message. Furthermore, the code lists #default in the
> >inclusive namespaces, so as far as I can tell this should make it ok for
> >axis to remove redundant NS declarations.
> >
> >
> >
> The #default namespace means treat the xmlns prefix as it was done in
> the inclusive c14n
Right, I understand that.
> >Now in particular, I'm seeing the following problems with the code:
> >
> >* If the element being processed lacks a default ns declaration and does
> >not use a prefix then xmlns="" is added unconditionally even if the
> >element is declared in the default namespace and the immediate parent
> >element defines a default NS.
> >
> >
> >
> That is the way is defined if xmlns is added in inclusive(and that's
> thing seem to be stressed by the tests).
I think I disagree here. The standard only says:
"The XPath data model represents an empty default namespace with the
absence of a node, not with the presence of a default namespace node
having an empty value. Thus, with respect to the fact that element e3 in
the following examples is not namespace qualified, we cannot tell the
difference between <e1 xmlns="a:b"><e2 xmlns=""><e3/></e2></e1> versus
<e1 xmlns="a:b"><e2><e3 xmlns=""/></e2></e1>. All we know is that e3 was
not namespace qualified on input, so we preserve this information on
output if e2 is omitted so that e3 does not take on the default
namespace qualification of e1."
Now my interpretation of the above is that xmlns="" should only be added
if the element is in the empty namespace to begin with and it should not
be added if the element is in e.g. xmlns="http://foobar.com", although
you may well have to add a xmlns="http://foobar.com" depending on the
visibility of the element declaring that default namespace.
> >* CanonicalizerBase.java has the following code:
> >
> > Element currentElement = (Element) currentNode;
> > OutputStream writer=this._writer;
> > String tagName=currentElement.getTagName();
> > if (currentNodeIsVisible) {
> > //This is an outputNode.
> > ns.outputNodePush();
> > writer.write('<');
> > writeStringToUtf8(tagName,writer);
> > } else {
> > //Not an outputNode.
> > ns.push();
> > }
> >
> > // we output all Attrs which are available
> > Iterator attrs = handleAttributes(currentElement,ns);
> > while (attrs.hasNext()) {
> > Attr attr = (Attr) attrs.next();
> > outputAttrToWriter(attr.getNodeName(), attr.getNodeValue(), writer);
> > }
> >
> >i.e. attributes are printed even if the element is not visible. This
> >seems to result in spurious xmlns="" being output in random places.
> >Maybe this code:
> >
> >
> >
> That's needed for rare xpath transformations but you have hit a bug
> because it should only output when the xpath select the xmlns but not in
> when it is added as inclusive(but i need to check with the spec to be sure).
>
> > Iterator it=this._inclusiveNSSet.iterator();
> > while (it.hasNext()) {
> > String s=(String)it.next();
> > Attr key=ns.getMappingWithoutRendered(s);
> > if (key==null) {
> > continue;
> > }
> > result.add(key);
> > }
> >
> >in Canonicalizer20010315Excl.java and the fact that the empty default
> >map seems to be added to ns (why is that done btw? Seems wrong to me) by
> >default has something to do with it?
> >
> >
> >
> The #default in inclusive prefix makes to do this behavior, as expected.
Again, I'm not sure why you believe that.
> >The inclusive code seems to behave a little better (not seeing
> >validation failures anymore), but I am a bit skeptical about the
> >following:
> >
> > if (isRealVisible) {
> > //The element is visible, handle the xmlns definition
> > Attr xmlns = E.getAttributeNodeNS(Constants.NamespaceSpecNS, "xmlns");
> > Node n=null;
> > if (xmlns == null) {
> > //No xmlns def just get the already defined.
> > n=ns.getMapping(XMLNS);
> > } else if ( !this._xpathNodeSet.contains(xmlns)) {
> > //There is a definition but the xmlns is not selected by the xpath.
> > //then xmlns=""
> > n=ns.addMappingAndRenderXNodeSet(XMLNS,"",nullNode,true);
> > }
> > //output the xmlns def if needed.
> > if (n!=null) {
> > result.add(n);
> > }
> > //Float all xml:* attributes of the unselected parent elements to this one.
> > addXmlAttributes(E,result);
> > }
> >
> >Shouldn't the xmlns="" mapping only be added if this mapping is in scope
> >and the ancestor element declaring the mapping is not visible (and since
> >NameSpaceSymbolTable always adds that mapping...). Also I'm not quite
> >sure that I understand the "else if":
> >CanonicalizerBase:canonicalizeXPathNodeSet seems to throw a exception if
> >it finds a attribute node in the _xpathNodeSet which would lead me to
> >assume that the node set would never contain the xmlns node?
Any comment on the above?
/Sam
> >Might also be that I am completely misunderstanding things.
> >Canonicalization makes my brain hurt...
> >
> >
> >
> Yes it is really difficult,and now that I have arrived home I will try
> to fix it, thanks for your report(but try to don't use the xpath code,
> it will be good for your performance).
>
> Thanks again,
>
> Regards,
>
>
> Raul
>
Re: Problems with exclusive c14n and default namespaces
Posted by Raul Benito <ra...@r-bg.com>.
Samuel Meder wrote:
>We just upgraded to the xmlsec 1.2 RC1 release and started noticing
>problems with the exclusive c14n implementation. In particular SAML
>assertion signature validation started to fail after the assertion was
>sent over the wire (using axis).
>
>
First of all thanks for the bug report. You have hit the code that is
not very well exercised by the test suite, so thanks a lot. Second you
are using the slow path (i.e. using xpath transformations) and that's good.
>The assertion when initially generated declared several redundant
>default namespaces on the elements which Axis subsequently removed when
>sending the actual message. Furthermore, the code lists #default in the
>inclusive namespaces, so as far as I can tell this should make it ok for
>axis to remove redundant NS declarations.
>
>
>
The #default namespace means treat the xmlns prefix as it was done in
the inclusive c14n
>Now in particular, I'm seeing the following problems with the code:
>
>* If the element being processed lacks a default ns declaration and does
>not use a prefix then xmlns="" is added unconditionally even if the
>element is declared in the default namespace and the immediate parent
>element defines a default NS.
>
>
>
That is the way is defined if xmlns is added in inclusive(and that's
thing seem to be stressed by the tests).
>* CanonicalizerBase.java has the following code:
>
> Element currentElement = (Element) currentNode;
> OutputStream writer=this._writer;
> String tagName=currentElement.getTagName();
> if (currentNodeIsVisible) {
> //This is an outputNode.
> ns.outputNodePush();
> writer.write('<');
> writeStringToUtf8(tagName,writer);
> } else {
> //Not an outputNode.
> ns.push();
> }
>
> // we output all Attrs which are available
> Iterator attrs = handleAttributes(currentElement,ns);
> while (attrs.hasNext()) {
> Attr attr = (Attr) attrs.next();
> outputAttrToWriter(attr.getNodeName(), attr.getNodeValue(), writer);
> }
>
>i.e. attributes are printed even if the element is not visible. This
>seems to result in spurious xmlns="" being output in random places.
>Maybe this code:
>
>
>
That's needed for rare xpath transformations but you have hit a bug
because it should only output when the xpath select the xmlns but not in
when it is added as inclusive(but i need to check with the spec to be sure).
> Iterator it=this._inclusiveNSSet.iterator();
> while (it.hasNext()) {
> String s=(String)it.next();
> Attr key=ns.getMappingWithoutRendered(s);
> if (key==null) {
> continue;
> }
> result.add(key);
> }
>
>in Canonicalizer20010315Excl.java and the fact that the empty default
>map seems to be added to ns (why is that done btw? Seems wrong to me) by
>default has something to do with it?
>
>
>
The #default in inclusive prefix makes to do this behavior, as expected.
>The inclusive code seems to behave a little better (not seeing
>validation failures anymore), but I am a bit skeptical about the
>following:
>
> if (isRealVisible) {
> //The element is visible, handle the xmlns definition
> Attr xmlns = E.getAttributeNodeNS(Constants.NamespaceSpecNS, "xmlns");
> Node n=null;
> if (xmlns == null) {
> //No xmlns def just get the already defined.
> n=ns.getMapping(XMLNS);
> } else if ( !this._xpathNodeSet.contains(xmlns)) {
> //There is a definition but the xmlns is not selected by the xpath.
> //then xmlns=""
> n=ns.addMappingAndRenderXNodeSet(XMLNS,"",nullNode,true);
> }
> //output the xmlns def if needed.
> if (n!=null) {
> result.add(n);
> }
> //Float all xml:* attributes of the unselected parent elements to this one.
> addXmlAttributes(E,result);
> }
>
>Shouldn't the xmlns="" mapping only be added if this mapping is in scope
>and the ancestor element declaring the mapping is not visible (and since
>NameSpaceSymbolTable always adds that mapping...). Also I'm not quite
>sure that I understand the "else if":
>CanonicalizerBase:canonicalizeXPathNodeSet seems to throw a exception if
>it finds a attribute node in the _xpathNodeSet which would lead me to
>assume that the node set would never contain the xmlns node?
>
>Might also be that I am completely misunderstanding things.
>Canonicalization makes my brain hurt...
>
>
>
Yes it is really difficult,and now that I have arrived home I will try
to fix it, thanks for your report(but try to don't use the xpath code,
it will be good for your performance).
Thanks again,
Regards,
Raul