You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@santuario.apache.org by Colm O hEigeartaigh <co...@apache.org> on 2011/12/16 12:29:12 UTC

XML Security Java 1.5.0-RC1 available

All,

A RC1 of the forthcoming XML Security Java 1.5.0 release is now
available. The distribution is here:

http://people.apache.org/~coheigea/stage/xmlsec/1.5.0-RC1/dist

The maven artifacts are here:

http://people.apache.org/~coheigea/stage/xmlsec/1.5.0-RC1/maven

Please test and let me know if there are any problems. I'll get some
release notes done shortly.

Colm.

-- 
Colm O hEigeartaigh

Talend Community Coder
http://coders.talend.com

Re: XML Security Java 1.5.0-RC1 available

Posted by Chad La Joie <la...@itumi.biz>.
Right, I am not at all trying to suggest it's a bug with this library.
It most certainly is not.

On Tue, Dec 20, 2011 at 10:12, Cantor, Scott <ca...@osu.edu> wrote:
> On 12/20/11 4:58 AM, "Colm O hEigeartaigh" <co...@apache.org> wrote:
>>
>>It is up to the application calling the signature verification code to
>>ensure that ID's are unique.
>
> This isn't Santuario's fault, but it's the parser's job if it supports
> either schema validation OR DOM 3 (the latter being the problem). Not
> enforcing uniqueness on setIdAttribute is either a bug or just blatantly
> silly, depending on how strictly one wants to read the spec.
>
> -- Scott
>



-- 
Chad La Joie
www.itumi.biz
trusted identities, delivered

Re: XML Security Java 1.5.0-RC1 available

Posted by Chad La Joie <la...@itumi.biz>.
I think, by "checks that each (local) Reference URI in a Signature is
in the document tree and is unique" and "It would not check for
duplicate Ids in other namespaces" what you're saying is the
following, is this correct?

Prior to any processing people would need to register the QName of
attributes they consider to be ID attributes.  You'd then walk the
tree once and ensure that for those attributes there is no duplication
(and presumably create a ID->element mapping for use later during ID
resolution).  Those attributes which are not registered are ignore and
may include a duplicate ID value and if apps pick that up and make
some assumption about, it's their own issue to deal with.

On Tue, Dec 20, 2011 at 11:36, Colm O hEigeartaigh <co...@apache.org> wrote:
> What we could do is add some functionality that checks that each
> (local) Reference URI in a Signature is in the document tree and is
> unique. The retrieved element could be set on IdResolver. This way,
> the tree is only walked once, instead of each time IdResolver is
> called when resolving a reference, as is the case for 1.4.x. This
> behaviour could be controlled by a property, possibly defaulting to
> true.
> It would not check for duplicate Ids in other namespaces (e.g.
> wsu:Id), or support retrieving elements in these namespaces - it would
> be up to the calling code to support this.
>
> Would this address your concerns?

-- 
Chad La Joie
www.itumi.biz
trusted identities, delivered

Re: XML Security Java 1.5.0-RC1 available

Posted by "Cantor, Scott" <ca...@osu.edu>.
On 12/20/11 11:36 AM, "Colm O hEigeartaigh" <co...@apache.org> wrote:

>What we could do is add some functionality that checks that each
>(local) Reference URI in a Signature is in the document tree and is
>unique.

The reference itself is the result of resolution of the ID. If there's a
duplicate, the results could be unique, but still wrong. The key point is
that the app and the library have to resolve to the same reference element
(and check transforms, etc.)

One way is to expose the resolved element (see what was signed). Another
is to enforce a totally app-controlled algorithm over resolution. I'm
saying that DOM calls render the latter pretty difficult to pull off if
the parser is allowed to be broken.

> The retrieved element could be set on IdResolver. This way,
>the tree is only walked once, instead of each time IdResolver is
>called when resolving a reference, as is the case for 1.4.x. This
>behaviour could be controlled by a property, possibly defaulting to
>true.

If you mean that the app tells the library what element a given ID should
map to, I think that's what you're already doing, minus the fact that
you're allowing for DOM lookup also.

>Would this address your concerns?

I think cutting off DOM support is a major step to take, since it's makes
schema validation insufficient to establish IDness and will break existing
apps. But if that's the conclusion (perhaps as an option or by default),
then we need to clearly communicate that change, and I should implement
something similar in C++.

So far, I haven't taken any new steps because I'm waiting to see what the
"right" answer is.

-- Scott


Re: XML Security Java 1.5.0-RC1 available

Posted by Colm O hEigeartaigh <co...@apache.org>.
What we could do is add some functionality that checks that each
(local) Reference URI in a Signature is in the document tree and is
unique. The retrieved element could be set on IdResolver. This way,
the tree is only walked once, instead of each time IdResolver is
called when resolving a reference, as is the case for 1.4.x. This
behaviour could be controlled by a property, possibly defaulting to
true.
It would not check for duplicate Ids in other namespaces (e.g.
wsu:Id), or support retrieving elements in these namespaces - it would
be up to the calling code to support this.

Would this address your concerns?

Colm.

On Tue, Dec 20, 2011 at 4:21 PM, Cantor, Scott <ca...@osu.edu> wrote:
> On 12/20/11 11:12 AM, "Sean Mullan" <se...@oracle.com> wrote:
>>
>>The code does still call DOM Document.getElementById, but how does that
>>make it possible to do an attack?
>
> If you mark "known" ID attributes as IDs by calling setIdAtribute, Xerces
> will allow duplicates. If you insist that the application do a tree walk
> to prevent duplicates, then you're telling us that every app should
> implement the thing you claim is too slow for Santuario to implement.
>
> Additionally, parsers have bugs enforcing uniqueness on IDs during schema
> validation. Xerces-C for example has a nasty one involving wildcards and
> lax schemas. All because they allow something that shouldn't ever be legal
> in the first place, duplicates in the Document's ID map. Fix that, in one
> place, and all of this goes away.
>
> Thus my point. The Xerces team is wrong. Somebody needs to explain that to
> them, somebody they'll listen to.
>
>> The trusted validation code should be
>>creating the Document and registering the IDs. If you are letting
>>untrusted code create the Document for you and register arbitrary IDs,
>>then that is a bug.
>
> If you claim schema validation is "one way", then the parser has to be
> trusted. Alternatively, the application can't just register the IDs, but
> must also track and search for duplicates, or it can't rely on
> setIdAttributeNS to register them.
>
> That's why I'm asking about the algorithm. You're basically duplicating
> the DOM3 ID API in Santuario. So my advice is to stop relying on the
> original. Since I don't think that's really a good thing either, I
> continue to argue that the fix belongs in the parser.
>
> -- Scott
>



-- 
Colm O hEigeartaigh

Talend Community Coder
http://coders.talend.com

Re: XML Security Java 1.5.0-RC1 available

Posted by Chad La Joie <la...@itumi.biz>.
Okay, so what you've just said is that you can use schema validation
and xmlsec together.  Is that really what is intended?

On Tue, Dec 20, 2011 at 11:12, Sean Mullan <se...@oracle.com> wrote:
> The code does still call DOM Document.getElementById, but how does that make
> it possible to do an attack? The trusted validation code should be creating
> the Document and registering the IDs. If you are letting untrusted code
> create the Document for you and register arbitrary IDs, then that is a bug.


-- 
Chad La Joie
www.itumi.biz
trusted identities, delivered

Re: XML Security Java 1.5.0-RC1 available

Posted by Sean Mullan <se...@oracle.com>.
On 12/20/2011 02:29 PM, Cantor, Scott wrote:
> On 12/20/11 2:11 PM, "Sean Mullan"<se...@oracle.com>  wrote:
>>
>> I did a grep of the source tree, and our code never calls
>> Element.setIdAttribute.
>
> Yes, but apps do. My code does. That's currently the way for schema
> validation *and* manual ID setting to result in the same outcome, assuming
> you call getElementById on your end.

Yep, although we do offer our own API alternatives for manual 
registration that don't use getElementById():

JSR 105: DOMCryptoContext.setIdAttributeNS()
XMLSec:  IdResolver.registerElementById()

>> As I understand the wrapping attacks, it happens after the signature is
>> validated, when the application actually acts on the element content
>> that is mapped to that ID. Then, it needs to find that element, and if
>> there are duplicate IDs and it gets the wrong one, then oops. As Colm
>> mentioned, we do have a mechanism to return the Elements that were
>> actually validated.
>
> Right. I agree that it's obviously better to do that, although I wonder
> about the performance when dealing with transformed node sets.
>
> I don't have it yet in C++, and I've been hesitant because it's a lot of
> work, and I don't have a lot of time to fix things that aren't broken in
> my code. I'm particularly unclear how to do it for the general case, not
> just a simple ID reference to an element subtree. All I can see to do is
> clone the nodes to save them off and return them. Or save the octets I
> guess.
>
> Point being, the API can't be just "here's the Element", but rather the
> node set or stream.

Right, it can be costly to cache the data as well, that's why we made it 
optional. We return an octet stream or a node-set, depending on what was 
dereferenced.

>>> Thus my point. The Xerces team is wrong. Somebody needs to explain that
>>> to
>>> them, somebody they'll listen to.
>>
>> If they won't listen to you, I'm sure they won't listen to me ;)
>
> Well, we tried (in fairness Xerces-C is more or less dead, so the open bug
> is all I really expected there). Now I think it's down to us defining a
> suitable algorithm. It may be that abandoning the DOM API is the right
> thing, but I don't think we should do that without some deprecation time.

Yes, breaking compatibility is a concern.

>> Hmm, I suppose we could stop calling Document.getElementById if the
>> document was not validated against a schema. Let me think about that
>> some more.
 >
> I'm not sure if you can tell, actually. Maybe in Java.

Possibly not just Java. Looking at the DOM APIs, I think you could get a 
DOMConfiguration object from the Document object, and if the validate 
parameter is set to true, assume that it has been validated.

> I think the most logical thing to do if you're going to deprecate that
> call is to make it an application option. Basically I'd have a set of
> IdResolvers with different, defined behavior, choose a default for the
> time being, possibly deprecate some of them, etc. I think that's cleaner
> than trying to create a bunch of options.

Yes, just need someone to write the code now :)

--Sean


Re: XML Security Java 1.5.0-RC1 available

Posted by "Cantor, Scott" <ca...@osu.edu>.
On 12/20/11 2:11 PM, "Sean Mullan" <se...@oracle.com> wrote:
>
>I did a grep of the source tree, and our code never calls
>Element.setIdAttribute.

Yes, but apps do. My code does. That's currently the way for schema
validation *and* manual ID setting to result in the same outcome, assuming
you call getElementById on your end.

>Usually an app won't have to do that - it is aware of the schema, and
>where the element IDs should be in the document - Colm can comment some
>more on how the WSS library does this.

You have to do a partial tree walk any time you have open extension points
that could be carrying objects that could have IDs. It's not complete
traversal, but for many types of documents, the difference is minimal.

> Our library won't find anything
>that isn't registered, so if you stick something way down in the guts of
>the document, it simply won't find it. (It used to, but not as of 1.5).
>But I can see the duplicate ID issues you mention, if the app uses
>Element.setIdAttribute to register the ID attributes.

Yes. You will find anything that isn't registered *by you*, as long as
it's registered with the DOM.

>As I understand the wrapping attacks, it happens after the signature is
>validated, when the application actually acts on the element content
>that is mapped to that ID. Then, it needs to find that element, and if
>there are duplicate IDs and it gets the wrong one, then oops. As Colm
>mentioned, we do have a mechanism to return the Elements that were
>actually validated.

Right. I agree that it's obviously better to do that, although I wonder
about the performance when dealing with transformed node sets.

I don't have it yet in C++, and I've been hesitant because it's a lot of
work, and I don't have a lot of time to fix things that aren't broken in
my code. I'm particularly unclear how to do it for the general case, not
just a simple ID reference to an element subtree. All I can see to do is
clone the nodes to save them off and return them. Or save the octets I
guess.

Point being, the API can't be just "here's the Element", but rather the
node set or stream.

>But I guess I see an issue in that it is hard for the app to do all
>these extra checks to prevent wrapping attacks. It sounds like what we
>need is an additional optional "sanity" check on the entire document
>looking for duplicate IDs.

My feeling has been that it's a difficult/impossible problem in the XPath
case (you really have to just return the exact nodes) but in the ID case,
if you can guarantee some sort of predictable behavior plus have the app
do transform checking, you have a shot of offloading some of the
significant steps.

>>Thus my point. The Xerces team is wrong. Somebody needs to explain that
>>to
>> them, somebody they'll listen to.
>
>If they won't listen to you, I'm sure they won't listen to me ;)

Well, we tried (in fairness Xerces-C is more or less dead, so the open bug
is all I really expected there). Now I think it's down to us defining a
suitable algorithm. It may be that abandoning the DOM API is the right
thing, but I don't think we should do that without some deprecation time.

>Hmm, I suppose we could stop calling Document.getElementById if the
>document was not validated against a schema. Let me think about that
>some more.

I'm not sure if you can tell, actually. Maybe in Java.

I think the most logical thing to do if you're going to deprecate that
call is to make it an application option. Basically I'd have a set of
IdResolvers with different, defined behavior, choose a default for the
time being, possibly deprecate some of them, etc. I think that's cleaner
than trying to create a bunch of options.

-- Scott


Re: XML Security Java 1.5.0-RC1 available

Posted by Sean Mullan <se...@oracle.com>.
On 12/20/2011 11:21 AM, Cantor, Scott wrote:
> On 12/20/11 11:12 AM, "Sean Mullan"<se...@oracle.com>  wrote:
>>
>> The code does still call DOM Document.getElementById, but how does that
>> make it possible to do an attack?
>
> If you mark "known" ID attributes as IDs by calling setIdAtribute, Xerces
> will allow duplicates.

I did a grep of the source tree, and our code never calls 
Element.setIdAttribute.

> If you insist that the application do a tree walk
> to prevent duplicates, then you're telling us that every app should
> implement the thing you claim is too slow for Santuario to implement.

Usually an app won't have to do that - it is aware of the schema, and 
where the element IDs should be in the document - Colm can comment some 
more on how the WSS library does this. Our library won't find anything 
that isn't registered, so if you stick something way down in the guts of 
the document, it simply won't find it. (It used to, but not as of 1.5). 
But I can see the duplicate ID issues you mention, if the app uses 
Element.setIdAttribute to register the ID attributes.

As I understand the wrapping attacks, it happens after the signature is 
validated, when the application actually acts on the element content 
that is mapped to that ID. Then, it needs to find that element, and if 
there are duplicate IDs and it gets the wrong one, then oops. As Colm 
mentioned, we do have a mechanism to return the Elements that were 
actually validated.

But I guess I see an issue in that it is hard for the app to do all 
these extra checks to prevent wrapping attacks. It sounds like what we 
need is an additional optional "sanity" check on the entire document 
looking for duplicate IDs.

> Additionally, parsers have bugs enforcing uniqueness on IDs during schema
> validation. Xerces-C for example has a nasty one involving wildcards and
> lax schemas. All because they allow something that shouldn't ever be legal
> in the first place, duplicates in the Document's ID map. Fix that, in one
> place, and all of this goes away.
>
> Thus my point. The Xerces team is wrong. Somebody needs to explain that to
> them, somebody they'll listen to.

If they won't listen to you, I'm sure they won't listen to me ;)

>> The trusted validation code should be
>> creating the Document and registering the IDs. If you are letting
>> untrusted code create the Document for you and register arbitrary IDs,
>> then that is a bug.
>
> If you claim schema validation is "one way", then the parser has to be
> trusted. Alternatively, the application can't just register the IDs, but
> must also track and search for duplicates, or it can't rely on
> setIdAttributeNS to register them.
>
> That's why I'm asking about the algorithm. You're basically duplicating
> the DOM3 ID API in Santuario. So my advice is to stop relying on the
> original. Since I don't think that's really a good thing either, I
> continue to argue that the fix belongs in the parser.

Hmm, I suppose we could stop calling Document.getElementById if the 
document was not validated against a schema. Let me think about that 
some more.

--Sean

Re: XML Security Java 1.5.0-RC1 available

Posted by "Cantor, Scott" <ca...@osu.edu>.
On 12/20/11 11:12 AM, "Sean Mullan" <se...@oracle.com> wrote:
>
>The code does still call DOM Document.getElementById, but how does that
>make it possible to do an attack?

If you mark "known" ID attributes as IDs by calling setIdAtribute, Xerces
will allow duplicates. If you insist that the application do a tree walk
to prevent duplicates, then you're telling us that every app should
implement the thing you claim is too slow for Santuario to implement.

Additionally, parsers have bugs enforcing uniqueness on IDs during schema
validation. Xerces-C for example has a nasty one involving wildcards and
lax schemas. All because they allow something that shouldn't ever be legal
in the first place, duplicates in the Document's ID map. Fix that, in one
place, and all of this goes away.

Thus my point. The Xerces team is wrong. Somebody needs to explain that to
them, somebody they'll listen to.

> The trusted validation code should be
>creating the Document and registering the IDs. If you are letting
>untrusted code create the Document for you and register arbitrary IDs,
>then that is a bug.

If you claim schema validation is "one way", then the parser has to be
trusted. Alternatively, the application can't just register the IDs, but
must also track and search for duplicates, or it can't rely on
setIdAttributeNS to register them.

That's why I'm asking about the algorithm. You're basically duplicating
the DOM3 ID API in Santuario. So my advice is to stop relying on the
original. Since I don't think that's really a good thing either, I
continue to argue that the fix belongs in the parser.

-- Scott


Re: XML Security Java 1.5.0-RC1 available

Posted by Sean Mullan <se...@oracle.com>.
On 12/20/2011 10:59 AM, Cantor, Scott wrote:
> On 12/20/11 10:55 AM, "Sean Mullan"<se...@oracle.com>  wrote:
>
>> It no longer searches. All IDs have to be pre-registered. It knows about
>> IDs in the XML signature namespace so pre-registers those itself.
>
> Does that imply you no longer rely on getElementById either? Because
> that's a search you don't control, and we know Xerces allows duplicates,
> ergo so does Santuario if it uses that API.

The code does still call DOM Document.getElementById, but how does that 
make it possible to do an attack? The trusted validation code should be 
creating the Document and registering the IDs. If you are letting 
untrusted code create the Document for you and register arbitrary IDs, 
then that is a bug.

--Sean

>> We could search the entire document every time for duplicate IDs but
>> then nobody would use the library because it would be too slow.
>
> It would work fine in many applications that favor guarantees over speed.
>
>> This is an issue that we can solve partially, but in my opinion higher
>> level APIs need to also do their job and register the IDs in their own
>> namespaces (or use a validating schema). Then wrapping attacks are not
>> possible.
>
> Unless you're not using the DOM ID APIs anymore, they're still possible
> because Xerces remains broken.
>
> -- Scott
>


Re: XML Security Java 1.5.0-RC1 available

Posted by "Cantor, Scott" <ca...@osu.edu>.
On 12/20/11 10:55 AM, "Sean Mullan" <se...@oracle.com> wrote:

>It no longer searches. All IDs have to be pre-registered. It knows about
>IDs in the XML signature namespace so pre-registers those itself.

Does that imply you no longer rely on getElementById either? Because
that's a search you don't control, and we know Xerces allows duplicates,
ergo so does Santuario if it uses that API.

>We could search the entire document every time for duplicate IDs but
>then nobody would use the library because it would be too slow.

It would work fine in many applications that favor guarantees over speed.

>This is an issue that we can solve partially, but in my opinion higher
>level APIs need to also do their job and register the IDs in their own
>namespaces (or use a validating schema). Then wrapping attacks are not
>possible.

Unless you're not using the DOM ID APIs anymore, they're still possible
because Xerces remains broken.

-- Scott


Re: XML Security Java 1.5.0-RC1 available

Posted by Chad La Joie <la...@itumi.biz>.
Sure, what could be more fun over christmas?  ;)

No, seriously though I don't mind doing up a patch if we can agree on
an approach.  As I responded to Colm, I'm also trying to make sure I'm
on the same page as he is and not making any assumptions about what he
means.

On Tue, Dec 20, 2011 at 13:45, Sean Mullan <se...@oracle.com> wrote:
> Ok, I can see how this could make things easier and help screen out
> potentially dangerous documents. Would you have some time to writeup a patch
> that we could then do some more testing on?



-- 
Chad La Joie
www.itumi.biz
trusted identities, delivered

Re: XML Security Java 1.5.0-RC1 available

Posted by Sean Mullan <se...@oracle.com>.
On 12/20/2011 11:06 AM, Chad La Joie wrote:
> On Tue, Dec 20, 2011 at 10:55, Sean Mullan<se...@oracle.com>  wrote:
>> It no longer searches. All IDs have to be pre-registered. It knows about IDs
>> in the XML signature namespace so pre-registers those itself.
>
> I guess I'm missing something.  How is this done?  After a parse
> (without schema validation) no attributes would be marked as ID
> attributes.  So how does the library "pre-register" anything?

It has builtin knowledge of the XML Signature schema, so it is able to 
register all elements with ID attributes in the dsig namespace as it is 
parsing the XML Signature. If there are any duplicates, as of 1.5 it 
throws an exception.

> And are
> you saying that prior to signature validation (or encrypted key
> resolution), that the app must go through and register every
> ID/element mapping itself?

Just the ones that aren't in the dsig namespace. We don't have any 
knowledge of other schemas. I'm not sure about the enc namespace, will 
need to check that.

>> We could search the entire document every time for duplicate IDs but then
>> nobody would use the library because it would be too slow.
>
> Not to be flippant, but do you actually have anything to back that up?
>   Relatively speaking, a treewalk is pretty fast (when compared to
> things like canonicalization and various crypto functions).

We used to do a recursive walk of the tree every time to fix namespace 
attributes when canonicalizing to workaround bug 2560 in Xerces - the 
code is actually still in there - search for circumventBug2560.

I remember that when we removed the dependency on this method, there was 
a performance improvement, how much I don't recall but it was somewhat 
significant if I remember - these numbers could be in the archives 
somewhere, when I have some more time I'll check.

I would want to do some performance testing before we added something 
like this.

>> This is an issue that we can solve partially, but in my opinion higher level
>> APIs need to also do their job and register the IDs in their own namespaces
>> (or use a validating schema). Then wrapping attacks are not possible.
>
> Sure, and everyone should always completely bug free code.  They
> don't.  All I'm trying to say is that we could provide a real fix for
> this that protects people against an attack that is known to be in the
> wild and which all tested users of Santuario were vulnerable to.

Ok, I can see how this could make things easier and help screen out 
potentially dangerous documents. Would you have some time to writeup a 
patch that we could then do some more testing on?

--Sean

Re: XML Security Java 1.5.0-RC1 available

Posted by Chad La Joie <la...@itumi.biz>.
On Tue, Dec 20, 2011 at 10:55, Sean Mullan <se...@oracle.com> wrote:
> It no longer searches. All IDs have to be pre-registered. It knows about IDs
> in the XML signature namespace so pre-registers those itself.

I guess I'm missing something.  How is this done?  After a parse
(without schema validation) no attributes would be marked as ID
attributes.  So how does the library "pre-register" anything?  And are
you saying that prior to signature validation (or encrypted key
resolution), that the app must go through and register every
ID/element mapping itself?

> We could search the entire document every time for duplicate IDs but then
> nobody would use the library because it would be too slow.

Not to be flippant, but do you actually have anything to back that up?
 Relatively speaking, a treewalk is pretty fast (when compared to
things like canonicalization and various crypto functions).

> This is an issue that we can solve partially, but in my opinion higher level
> APIs need to also do their job and register the IDs in their own namespaces
> (or use a validating schema). Then wrapping attacks are not possible.

Sure, and everyone should always completely bug free code.  They
don't.  All I'm trying to say is that we could provide a real fix for
this that protects people against an attack that is known to be in the
wild and which all tested users of Santuario were vulnerable to.

-- 
Chad La Joie
www.itumi.biz
trusted identities, delivered

Re: XML Security Java 1.5.0-RC1 available

Posted by Sean Mullan <se...@oracle.com>.
On 12/20/2011 10:37 AM, Chad La Joie wrote:
> That code would be part of the solution.  But does the entire tree get
> walked or does the IdResolver just stop once it finds a match (which I
> thought was what happened).

It no longer searches. All IDs have to be pre-registered. It knows about 
IDs in the XML signature namespace so pre-registers those itself.

We could search the entire document every time for duplicate IDs but 
then nobody would use the library because it would be too slow.

This is an issue that we can solve partially, but in my opinion higher 
level APIs need to also do their job and register the IDs in their own 
namespaces (or use a validating schema). Then wrapping attacks are not 
possible.

--Sean

>
>
> On Tue, Dec 20, 2011 at 10:35, Sean Mullan<se...@oracle.com>  wrote:
>> On 12/20/2011 10:19 AM, Chad La Joie wrote:
>>>
>>> On Tue, Dec 20, 2011 at 04:58, Colm O hEigeartaigh<co...@apache.org>
>>>   wrote:
>>>>
>>>> This is already available via the JSR-105 API by setting the
>>>> "javax.xml.crypto.dsig.cacheReference" property to true. Apache WSS4J
>>>> uses this to build a set of protected element results, that can be
>>>> subsequently compared to an XPath expression via WS-SecurityPolicy.
>>>
>>>
>>> Thanks for the pointer.
>>>
>>>> It is up to the application calling the signature verification code to
>>>> ensure that ID's are unique. The 1.5.0 release tightens this
>>>> requirement by not searching the document tree for any IDs in "known"
>>>> namespaces. The calling code must find the desired elements and
>>>> register them on the context/IdResolver for signature validation to
>>>> work.
>>>
>>>
>>> I really think the library should support this directly and by
>>> default.  Given *zero* systems using the library did the right thing
>>> in the review done by the researchers, I think it's safe to say this
>>> is non-obvious.
>>>
>>> Let me ask this a different way.  What speaks against adding this
>>> check in if, via an option, it can be disabled and remove the
>>> performance hit that would be caused?
>>
>>
>> This is actually fixed in 1.5, unless I'm misunderstanding the issue. See
>> the code for registerElementById() in [1]
>>
>>     public static void registerElementById(Element element, String idValue) {
>>         Document doc = element.getOwnerDocument();
>>         synchronized (docMap) {
>>             Map<String, WeakReference<Element>>  elementMap = docMap.get(doc);
>>             if (elementMap == null) {
>>                 elementMap = new WeakHashMap<String,
>> WeakReference<Element>>();
>>                 docMap.put(doc, elementMap);
>>                 elementMap.put(idValue, new WeakReference<Element>(element));
>>             } else {
>>                 WeakReference<Element>  ref = elementMap.get(idValue);
>>                 if (ref != null) {
>>                     if (!ref.get().equals(element)) {
>>                         throw new IllegalArgumentException("ID is already
>> registered");
>>                     }
>>                 } else {
>>                     elementMap.put(idValue, new
>> WeakReference<Element>(element));
>>                 }
>>             }
>>         }
>>     }
>>
>> Note the lines where it checks if the ID is already registered, and throws
>> an IllegalArgumentExc.
>>
>> --Sean
>>
>> [1]
>> http://svn.apache.org/viewvc/santuario/xml-security-java/tags/1.5.0-RC1/src/main/java/org/apache/xml/security/utils/IdResolver.java?view=markup
>
>
>


Re: XML Security Java 1.5.0-RC1 available

Posted by Colm O hEigeartaigh <co...@apache.org>.
Hi Chad,

The tree does not get walked at all in 1.5.0. The calling code must
take care of this and set the appropriate elements on the context. The
1.4.x IdResolver code *does* walk the tree looking for elements with
Ids in certain namespaces - this has been removed due to security
concerns.

For example, WSS4J walks the tree looking for the appropriate Ids, and
throws an error if there are multiple elements matching the same Id.

Colm.

On Tue, Dec 20, 2011 at 3:37 PM, Chad La Joie <la...@itumi.biz> wrote:
> That code would be part of the solution.  But does the entire tree get
> walked or does the IdResolver just stop once it finds a match (which I
> thought was what happened).
>
>
> On Tue, Dec 20, 2011 at 10:35, Sean Mullan <se...@oracle.com> wrote:
>> On 12/20/2011 10:19 AM, Chad La Joie wrote:
>>>
>>> On Tue, Dec 20, 2011 at 04:58, Colm O hEigeartaigh<co...@apache.org>
>>>  wrote:
>>>>
>>>> This is already available via the JSR-105 API by setting the
>>>> "javax.xml.crypto.dsig.cacheReference" property to true. Apache WSS4J
>>>> uses this to build a set of protected element results, that can be
>>>> subsequently compared to an XPath expression via WS-SecurityPolicy.
>>>
>>>
>>> Thanks for the pointer.
>>>
>>>> It is up to the application calling the signature verification code to
>>>> ensure that ID's are unique. The 1.5.0 release tightens this
>>>> requirement by not searching the document tree for any IDs in "known"
>>>> namespaces. The calling code must find the desired elements and
>>>> register them on the context/IdResolver for signature validation to
>>>> work.
>>>
>>>
>>> I really think the library should support this directly and by
>>> default.  Given *zero* systems using the library did the right thing
>>> in the review done by the researchers, I think it's safe to say this
>>> is non-obvious.
>>>
>>> Let me ask this a different way.  What speaks against adding this
>>> check in if, via an option, it can be disabled and remove the
>>> performance hit that would be caused?
>>
>>
>> This is actually fixed in 1.5, unless I'm misunderstanding the issue. See
>> the code for registerElementById() in [1]
>>
>>    public static void registerElementById(Element element, String idValue) {
>>        Document doc = element.getOwnerDocument();
>>        synchronized (docMap) {
>>            Map<String, WeakReference<Element>> elementMap = docMap.get(doc);
>>            if (elementMap == null) {
>>                elementMap = new WeakHashMap<String,
>> WeakReference<Element>>();
>>                docMap.put(doc, elementMap);
>>                elementMap.put(idValue, new WeakReference<Element>(element));
>>            } else {
>>                WeakReference<Element> ref = elementMap.get(idValue);
>>                if (ref != null) {
>>                    if (!ref.get().equals(element)) {
>>                        throw new IllegalArgumentException("ID is already
>> registered");
>>                    }
>>                } else {
>>                    elementMap.put(idValue, new
>> WeakReference<Element>(element));
>>                }
>>            }
>>        }
>>    }
>>
>> Note the lines where it checks if the ID is already registered, and throws
>> an IllegalArgumentExc.
>>
>> --Sean
>>
>> [1]
>> http://svn.apache.org/viewvc/santuario/xml-security-java/tags/1.5.0-RC1/src/main/java/org/apache/xml/security/utils/IdResolver.java?view=markup
>
>
>
> --
> Chad La Joie
> www.itumi.biz
> trusted identities, delivered



-- 
Colm O hEigeartaigh

Talend Community Coder
http://coders.talend.com

Re: XML Security Java 1.5.0-RC1 available

Posted by Chad La Joie <la...@itumi.biz>.
That code would be part of the solution.  But does the entire tree get
walked or does the IdResolver just stop once it finds a match (which I
thought was what happened).


On Tue, Dec 20, 2011 at 10:35, Sean Mullan <se...@oracle.com> wrote:
> On 12/20/2011 10:19 AM, Chad La Joie wrote:
>>
>> On Tue, Dec 20, 2011 at 04:58, Colm O hEigeartaigh<co...@apache.org>
>>  wrote:
>>>
>>> This is already available via the JSR-105 API by setting the
>>> "javax.xml.crypto.dsig.cacheReference" property to true. Apache WSS4J
>>> uses this to build a set of protected element results, that can be
>>> subsequently compared to an XPath expression via WS-SecurityPolicy.
>>
>>
>> Thanks for the pointer.
>>
>>> It is up to the application calling the signature verification code to
>>> ensure that ID's are unique. The 1.5.0 release tightens this
>>> requirement by not searching the document tree for any IDs in "known"
>>> namespaces. The calling code must find the desired elements and
>>> register them on the context/IdResolver for signature validation to
>>> work.
>>
>>
>> I really think the library should support this directly and by
>> default.  Given *zero* systems using the library did the right thing
>> in the review done by the researchers, I think it's safe to say this
>> is non-obvious.
>>
>> Let me ask this a different way.  What speaks against adding this
>> check in if, via an option, it can be disabled and remove the
>> performance hit that would be caused?
>
>
> This is actually fixed in 1.5, unless I'm misunderstanding the issue. See
> the code for registerElementById() in [1]
>
>    public static void registerElementById(Element element, String idValue) {
>        Document doc = element.getOwnerDocument();
>        synchronized (docMap) {
>            Map<String, WeakReference<Element>> elementMap = docMap.get(doc);
>            if (elementMap == null) {
>                elementMap = new WeakHashMap<String,
> WeakReference<Element>>();
>                docMap.put(doc, elementMap);
>                elementMap.put(idValue, new WeakReference<Element>(element));
>            } else {
>                WeakReference<Element> ref = elementMap.get(idValue);
>                if (ref != null) {
>                    if (!ref.get().equals(element)) {
>                        throw new IllegalArgumentException("ID is already
> registered");
>                    }
>                } else {
>                    elementMap.put(idValue, new
> WeakReference<Element>(element));
>                }
>            }
>        }
>    }
>
> Note the lines where it checks if the ID is already registered, and throws
> an IllegalArgumentExc.
>
> --Sean
>
> [1]
> http://svn.apache.org/viewvc/santuario/xml-security-java/tags/1.5.0-RC1/src/main/java/org/apache/xml/security/utils/IdResolver.java?view=markup



-- 
Chad La Joie
www.itumi.biz
trusted identities, delivered

Re: XML Security Java 1.5.0-RC1 available

Posted by Sean Mullan <se...@oracle.com>.
On 12/20/2011 10:19 AM, Chad La Joie wrote:
> On Tue, Dec 20, 2011 at 04:58, Colm O hEigeartaigh<co...@apache.org>  wrote:
>> This is already available via the JSR-105 API by setting the
>> "javax.xml.crypto.dsig.cacheReference" property to true. Apache WSS4J
>> uses this to build a set of protected element results, that can be
>> subsequently compared to an XPath expression via WS-SecurityPolicy.
>
> Thanks for the pointer.
>
>> It is up to the application calling the signature verification code to
>> ensure that ID's are unique. The 1.5.0 release tightens this
>> requirement by not searching the document tree for any IDs in "known"
>> namespaces. The calling code must find the desired elements and
>> register them on the context/IdResolver for signature validation to
>> work.
>
> I really think the library should support this directly and by
> default.  Given *zero* systems using the library did the right thing
> in the review done by the researchers, I think it's safe to say this
> is non-obvious.
>
> Let me ask this a different way.  What speaks against adding this
> check in if, via an option, it can be disabled and remove the
> performance hit that would be caused?

This is actually fixed in 1.5, unless I'm misunderstanding the issue. 
See the code for registerElementById() in [1]

     public static void registerElementById(Element element, String 
idValue) {
         Document doc = element.getOwnerDocument();
         synchronized (docMap) {
             Map<String, WeakReference<Element>> elementMap = 
docMap.get(doc);
             if (elementMap == null) {
                 elementMap = new WeakHashMap<String, 
WeakReference<Element>>();
                 docMap.put(doc, elementMap);
                 elementMap.put(idValue, new 
WeakReference<Element>(element));
             } else {
                 WeakReference<Element> ref = elementMap.get(idValue);
                 if (ref != null) {
                     if (!ref.get().equals(element)) {
                         throw new IllegalArgumentException("ID is 
already registered");
                     }
                 } else {
                     elementMap.put(idValue, new 
WeakReference<Element>(element));
                 }
             }
         }
     }

Note the lines where it checks if the ID is already registered, and 
throws an IllegalArgumentExc.

--Sean

[1] 
http://svn.apache.org/viewvc/santuario/xml-security-java/tags/1.5.0-RC1/src/main/java/org/apache/xml/security/utils/IdResolver.java?view=markup

Re: XML Security Java 1.5.0-RC1 available

Posted by Chad La Joie <la...@itumi.biz>.
On Tue, Dec 20, 2011 at 04:58, Colm O hEigeartaigh <co...@apache.org> wrote:
> This is already available via the JSR-105 API by setting the
> "javax.xml.crypto.dsig.cacheReference" property to true. Apache WSS4J
> uses this to build a set of protected element results, that can be
> subsequently compared to an XPath expression via WS-SecurityPolicy.

Thanks for the pointer.

> It is up to the application calling the signature verification code to
> ensure that ID's are unique. The 1.5.0 release tightens this
> requirement by not searching the document tree for any IDs in "known"
> namespaces. The calling code must find the desired elements and
> register them on the context/IdResolver for signature validation to
> work.

I really think the library should support this directly and by
default.  Given *zero* systems using the library did the right thing
in the review done by the researchers, I think it's safe to say this
is non-obvious.

Let me ask this a different way.  What speaks against adding this
check in if, via an option, it can be disabled and remove the
performance hit that would be caused?

-- 
Chad La Joie
www.itumi.biz
trusted identities, delivered

Re: XML Security Java 1.5.0-RC1 available

Posted by "Cantor, Scott" <ca...@osu.edu>.
On 12/20/11 4:58 AM, "Colm O hEigeartaigh" <co...@apache.org> wrote:
>
>It is up to the application calling the signature verification code to
>ensure that ID's are unique.

This isn't Santuario's fault, but it's the parser's job if it supports
either schema validation OR DOM 3 (the latter being the problem). Not
enforcing uniqueness on setIdAttribute is either a bug or just blatantly
silly, depending on how strictly one wants to read the spec.

-- Scott


Re: XML Security Java 1.5.0-RC1 available

Posted by Colm O hEigeartaigh <co...@apache.org>.
Hi Chad,

> Second, the signature validation process could make available the nodes
> that were covered by the signature and the application could check to
> make sure those are the ones it thought were supposed to be covered.
> This is more efficient but may require changes to the xmlsec APIs and
> would require the application to a) know they had to do something and b)
> actually do it properly.

This is already available via the JSR-105 API by setting the
"javax.xml.crypto.dsig.cacheReference" property to true. Apache WSS4J
uses this to build a set of protected element results, that can be
subsequently compared to an XPath expression via WS-SecurityPolicy.

It is up to the application calling the signature verification code to
ensure that ID's are unique. The 1.5.0 release tightens this
requirement by not searching the document tree for any IDs in "known"
namespaces. The calling code must find the desired elements and
register them on the context/IdResolver for signature validation to
work.

Colm.

On Mon, Dec 19, 2011 at 6:10 PM, Chad La Joie <la...@itumi.biz> wrote:
> Hey Colm,
>
> As I'm sure you're aware, there has recently been a set of papers
> published that show various forms of attacks against XML
> signature/encryption operations and one such attack was a signature
> wrapping attack.
>
> While investigating this issue within our code we came across some
> unexpected behavior with the resolution of IDs during the content
> reference resolution phase of signature validation.  In particular,
> there was no check that IDs were actually unique.  This allowed an
> attacker to take a valid signature over some ID'ed content, create
> malicious content with the same ID, and have our code report that the
> malicious content passed signature validation.
>
> As part of trying to address this, I asked the Xerces folks if some
> functionality could be added to the parser that would throw an exception
> if multiple elements were registered with the same ID during parsing.
> They said no.  Such an approach would only work if content was being
> schema validated during parsing so it wasn't a comprehensive solution
> anyways.
>
> I do think it would be good if xmlsec did have a comprehensive solution.
>  As far as I'm aware there are two ways to implement such a thing.
>
> First, the IdResolver could walk the entire tree to ensure that the ID
> was in fact unique.  This is obviously expensive since it requires a
> full tree walk but has the benefit of not requiring any API changes.
>
> Second, the signature validation process could make available the nodes
> that were covered by the signature and the application could check to
> make sure those are the ones it thought were supposed to be covered.
> This is more efficient but may require changes to the xmlsec APIs and
> would require the application to a) know they had to do something and b)
> actually do it properly.
>
> I personally feel like the best approach would be to allow for both
> options.  Make the first option the default behavior but allow it to be
> turned off if applications are willing to take on the extra work
> necessary for the second option.
>
> On 12/19/11 11:18 AM, Colm O hEigeartaigh wrote:
>> Here are some initial release notes for 1.5.0:
>>
>> http://coheigea.blogspot.com/2011/12/apache-santuario-xml-security-for-java.html
>>
>> Colm.
>>
>> On Fri, Dec 16, 2011 at 11:29 AM, Colm O hEigeartaigh
>> <co...@apache.org> wrote:
>>> All,
>>>
>>> A RC1 of the forthcoming XML Security Java 1.5.0 release is now
>>> available. The distribution is here:
>>>
>>> http://people.apache.org/~coheigea/stage/xmlsec/1.5.0-RC1/dist
>>>
>>> The maven artifacts are here:
>>>
>>> http://people.apache.org/~coheigea/stage/xmlsec/1.5.0-RC1/maven
>>>
>>> Please test and let me know if there are any problems. I'll get some
>>> release notes done shortly.
>>>
>>> Colm.
>>>
>>> --
>>> Colm O hEigeartaigh
>>>
>>> Talend Community Coder
>>> http://coders.talend.com
>>
>>
>>
>
> --
> Chad La Joie
> www.itumi.biz
> trusted identities, delivered



-- 
Colm O hEigeartaigh

Talend Community Coder
http://coders.talend.com

Re: XML Security Java 1.5.0-RC1 available

Posted by Chad La Joie <la...@itumi.biz>.
Hey Colm,

As I'm sure you're aware, there has recently been a set of papers
published that show various forms of attacks against XML
signature/encryption operations and one such attack was a signature
wrapping attack.

While investigating this issue within our code we came across some
unexpected behavior with the resolution of IDs during the content
reference resolution phase of signature validation.  In particular,
there was no check that IDs were actually unique.  This allowed an
attacker to take a valid signature over some ID'ed content, create
malicious content with the same ID, and have our code report that the
malicious content passed signature validation.

As part of trying to address this, I asked the Xerces folks if some
functionality could be added to the parser that would throw an exception
if multiple elements were registered with the same ID during parsing.
They said no.  Such an approach would only work if content was being
schema validated during parsing so it wasn't a comprehensive solution
anyways.

I do think it would be good if xmlsec did have a comprehensive solution.
 As far as I'm aware there are two ways to implement such a thing.

First, the IdResolver could walk the entire tree to ensure that the ID
was in fact unique.  This is obviously expensive since it requires a
full tree walk but has the benefit of not requiring any API changes.

Second, the signature validation process could make available the nodes
that were covered by the signature and the application could check to
make sure those are the ones it thought were supposed to be covered.
This is more efficient but may require changes to the xmlsec APIs and
would require the application to a) know they had to do something and b)
actually do it properly.

I personally feel like the best approach would be to allow for both
options.  Make the first option the default behavior but allow it to be
turned off if applications are willing to take on the extra work
necessary for the second option.

On 12/19/11 11:18 AM, Colm O hEigeartaigh wrote:
> Here are some initial release notes for 1.5.0:
> 
> http://coheigea.blogspot.com/2011/12/apache-santuario-xml-security-for-java.html
> 
> Colm.
> 
> On Fri, Dec 16, 2011 at 11:29 AM, Colm O hEigeartaigh
> <co...@apache.org> wrote:
>> All,
>>
>> A RC1 of the forthcoming XML Security Java 1.5.0 release is now
>> available. The distribution is here:
>>
>> http://people.apache.org/~coheigea/stage/xmlsec/1.5.0-RC1/dist
>>
>> The maven artifacts are here:
>>
>> http://people.apache.org/~coheigea/stage/xmlsec/1.5.0-RC1/maven
>>
>> Please test and let me know if there are any problems. I'll get some
>> release notes done shortly.
>>
>> Colm.
>>
>> --
>> Colm O hEigeartaigh
>>
>> Talend Community Coder
>> http://coders.talend.com
> 
> 
> 

-- 
Chad La Joie
www.itumi.biz
trusted identities, delivered

Re: XML Security Java 1.5.0-RC1 available

Posted by Eric Johnson <er...@tibco.com>.
Hi Colm,

Looks interesting. Based on the release notes, it sounds like I'll have 
a bunch of work to update the GenXDM port.

It also sounds like you've addressed some of the points that my port 
also touches on (for example, Java Generics), so that gets me to hope 
that the code bases will diverge less over time.

Cool.

I'm hoping that I will soon try to migrate the GenXDM-based port against 
RC1, so I can give you some useful feedback. Although it may not be this 
week, which means the first week of January, for me, at the earliest.

-Eric.

On 12/19/11 5:18 PM, Colm O hEigeartaigh wrote:
> Here are some initial release notes for 1.5.0:
>
> http://coheigea.blogspot.com/2011/12/apache-santuario-xml-security-for-java.html
>
> Colm.
>
> On Fri, Dec 16, 2011 at 11:29 AM, Colm O hEigeartaigh
> <co...@apache.org>  wrote:
>> All,
>>
>> A RC1 of the forthcoming XML Security Java 1.5.0 release is now
>> available. The distribution is here:
>>
>> http://people.apache.org/~coheigea/stage/xmlsec/1.5.0-RC1/dist
>>
>> The maven artifacts are here:
>>
>> http://people.apache.org/~coheigea/stage/xmlsec/1.5.0-RC1/maven
>>
>> Please test and let me know if there are any problems. I'll get some
>> release notes done shortly.
>>
>> Colm.
>>
>> --
>> Colm O hEigeartaigh
>>
>> Talend Community Coder
>> http://coders.talend.com
>
>

Re: XML Security Java 1.5.0-RC1 available

Posted by Colm O hEigeartaigh <co...@apache.org>.
Here are some initial release notes for 1.5.0:

http://coheigea.blogspot.com/2011/12/apache-santuario-xml-security-for-java.html

Colm.

On Fri, Dec 16, 2011 at 11:29 AM, Colm O hEigeartaigh
<co...@apache.org> wrote:
> All,
>
> A RC1 of the forthcoming XML Security Java 1.5.0 release is now
> available. The distribution is here:
>
> http://people.apache.org/~coheigea/stage/xmlsec/1.5.0-RC1/dist
>
> The maven artifacts are here:
>
> http://people.apache.org/~coheigea/stage/xmlsec/1.5.0-RC1/maven
>
> Please test and let me know if there are any problems. I'll get some
> release notes done shortly.
>
> Colm.
>
> --
> Colm O hEigeartaigh
>
> Talend Community Coder
> http://coders.talend.com



-- 
Colm O hEigeartaigh

Talend Community Coder
http://coders.talend.com