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

Reply via email to