Marshalling a SAML assertion into a new Document loses its ID

[Brent Putman]

On 2/4/16 4:04 AM, detelinyordanov at gmail.com wrote:
>    I have encountered an issue in Opensaml when marshalling an
> existing SAML assertion into a newly created empty Document.
> Apparently AbstractXMLObjectMarshaller.marshall(assertion, Document)
> uses Document.adoptNode() followed by Document.appendNode(..) but
> neither of these preserve the IDness of the assertion and if
> Document.getElementById(assertionId) is used on the resulting
> document, it returns null.

This sounded familiar.  I went and looked and someone encountered a
similar issue back in April 2012, discussed on this list.


> but then I noticed it is reproducible with Xerces as well and I saw
> that Xerces folks do not consider this a bug in Xerces DOM
> implementation - they argue that according to spec, the adoptNode()
> is not required to preserve the ID attributes, see XERCESJ-1022
> <https://issues.apache.org/jira/browse/XERCESJ-1022> and respective
> discussion: importNode()/adoptNode() and getElementById()
> <https://mail-archives.apache.org/mod_mbox/xerces-j-users/200701.mbox/%3C20070125062002.01E3310FB003@herse.apache.org%3E>

Yep, I came across XERCESJ-1022 back in 2012.  Sadly that seems to be
the official answer.


>
> Does this mean that this is a bug in Opensaml/Xmltooling

Yes, I think it is a bug.  I don't know why an issue was never opened
in 2012, but it wasn't and so we never really looked at fixing this. 
Please open a bug against the v3 project (OSJ).

As luck (or lack thereof) would have it, we literally just yesterday
released the last non-security patch release of the v2 project (JOST in
Jira), prior to its full EOL in July.  So there wouldn't be any plans
to release it for v2.  Unless there were some security vulnerability
aspect to this, but off-hand I don't see any.

> and Xmltooling node adoption code is required to ensure the ID
> attributes are preserved when marshalling into a new Document?

Aside from the fix in OpenSAML itself:  Yes, after marshaling an
existing XMObject, you would reset the IDness the same way the
marshallers do it.  For example, the AssertionMarshaller does this:

domElement.setIdAttributeNS(null, Assertion.ID_ATTRIB_NAME, true);


The fix in OpenSAML is likely to involve moving these
setIdAttributeNS(...) calls out of the marshallAttributes(...) methods
(which only get called when there is no existing DOM), and into a new
one like marshallAttributeIDNess(...).  Then the latter gets called in
both the new DOM and existing DOM cases.  Or something like that. 
Doesn't look too bad to fix.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://shibboleth.net/pipermail/dev/attachments/20160204/8a02a8e7/attachment.html>