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<[email protected]> wrote:
On 12/20/2011 10:19 AM, Chad La Joie wrote:
On Tue, Dec 20, 2011 at 04:58, Colm O hEigeartaigh<[email protected]>
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
