Santuario change

[Brent Putman]

On 11/4/15 9:17 PM, Cantor, Scott wrote:
> On 11/4/15, 9:04 PM, "dev on behalf of Cantor, Scott" <dev-bounces at shibboleth.net on behalf of cantor.2 at osu.edu> wrote:
>
>
>
>> We have a regression because of the bump of Santuario causing a failure on some encryption operations. 

I will look at in more detail tomorrow, but this feels not unlike the
issue with signature verification, where the document has to be a part
of the node tree rooted as the document element, or it doesn't work
(e.g. can't resolve an ID attribute if it's not in the tree).  That's
why there's a rootInNewDocument param on the decryption methods.


>> It's possible it's my bug in the EncryptXXX actions, but Brent would have to weigh in on that.
> Not sure if it's a bug, but it seems to work now if I change the NameID clone I'm doing so that it roots the clone in a new Document (adding the "true" parameter):
>
> final NameID nameId = XMLObjectSupport.cloneXMLObject(saml2Session.getNameID(), true);
>             object.setNameID(nameId);
>
> I wasn't doing that before and it was working.

That seems consistent with the Santuario change you found. Without
doing the rooting, you have a DOM, but the element doesn't have a
parent Node - within the original Document it's not in the node tree
and so it's "disconnected".


>
> I'm not sure why assertion encryption hadn't broken. I wasn't signing them, so they really shouldn't have been marshalled yet, so shouldn't have had a parent either.

I don't think the issue is whether it's marshalled or not, exactly. 
Not marshalled (no DOM) is fine, the Encrypter will do that internally
if necessary.  Marshalled and rooted (or at least a parent Node) also fine.

AFAICT the problem case is having a DOM Element that's not rooted
and/or no parent.  You're getting the latter specifically because of
the cloning.  Non-clone usage either wouldn't have a DOM at all
(freshly built object) or would have a "standard" unmarshalled DOM
Element which lives in the document root element's tree (so no funny
business).

Fwiw, in the delegation code, I might have run into something similar
vis-a-vis cloning.  At least I remember something about it...    There
are 2 cases where I have to clone stuff from the inbound Assertion, to
add to the new one.  There, rather than using the rootInNewDocument
feature, I instead just dropped the whole DOM on the newly cloned
element.  The cost of building a new one may not be terribly different
than the root-ing, since the latter involves creating a new Document
and importing, etc. So I figured the fuzziness of the un-rooted Element
wasn't worth it.  I was even thinking perhaps we needed new options on
the clone methods to just do that by default, since seems a common
case.  But for the record here's what I did:

AuthnStatement statement =
XMLObjectSupport.cloneXMLObject(sourceStatement);
statement.releaseDOM();
statement.releaseChildrenDOM(true);
return statement;

>
> Anyway, this may be something the encrypting code needs to compensate for if we use 2.0.5. I don't know the innards in the Java well enough, but since I don't think I was violating any contract here...

I'll have to think about it more, but I don't know if the Encrypter
really should (or even could) do anything here.  Might be invasive as
to side effects on the passed in data and its owning Document.

As I mentioned, we already have similar issues with
decryption+validation (e.g. decrypt an Assertion then validate its
signature).  That is documented extensively in the Decrypter Javadocs. 
This may just have to be a documentation thing.

>
> I was just ignoring DOM issues and treating it as an object I could pass to an encryption method, and get back a new object. Didn't think I'd need to care about the underlying details.

Unfortunately that turned out not to be true with the
decryption+validation, so may be an issue here as well.  It's an
unfortunate consequence of wanting to work at the higher domain object
level (XMLObject), but having to drop into the low-level DOM stuff to
do the crypto things.



-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://shibboleth.net/pipermail/dev/attachments/20151105/19e9415e/attachment.html>