[jira] [Commented] (SANTUARIO-253) org.apache.xml.security.utils.resolver.ResourceResolver is not thread safe

["Colm O hEigeartaigh (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/SANTUARIO-253?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13022048#comment-13022048 ] 

Colm O hEigeartaigh commented on SANTUARIO-253:
-----------------------------------------------


I've committed a fix for this issue. The core problem is that when iterating over the static list in getInstance, it clones the list on a successful hit, and attempts to insert the found ResourceResolver into first place in the list, before assigning the cloned list back to the static list. The thread safety problem happens if the list gets modified when another thread is iterating over it...it is not guaranteed to find all of the ResourceResolvers, hence the exception about not finding the resource.

The simple fix is just to avoid modifying the list on a successful hit. getInstance does not modify the list at all, and hence there are no thread-safety issues with it. There is still a potential thread-safety problem with the "register" method, if the user attempts to register a new ResourceResolver when another thread is in getInstance. However I'm not going to fix this issue for the 1.4.5 release, and instead solve it as part of a rework of ResourceResolvers in [SANTUARIO-254]. New ResourceResolvers are only registered in practice when the library is initialized, and so it has not proven to be a problem thus far.

Colm.

> org.apache.xml.security.utils.resolver.ResourceResolver is not thread safe
> --------------------------------------------------------------------------
>
>                 Key: SANTUARIO-253
>                 URL: https://issues.apache.org/jira/browse/SANTUARIO-253
>             Project: Santuario
>          Issue Type: Bug
>          Components: Java
>    Affects Versions: Java 1.4.4
>         Environment: Irrelevant - the bug is fully portable.
>            Reporter: Krzysztof Benedyczak
>            Assignee: Colm O hEigeartaigh
>            Priority: Critical
>             Fix For: Java 1.4.5
>
>         Attachments: santuario-253.patch
>
>
> Here are additional details, but hopefully the title is enough.
> My library is using xmlsec to sign and verify xml signatures. Sometimes I get the following exception when invoking DOMXMLSignature.sign():
> javax.xml.crypto.dsig.XMLSignatureException: javax.xml.crypto.URIReferenceException: org.apache.xml.security.utils.
> resolver.ResourceResolverException: Could not find a resolver for URI #SAMLblahblah and Base null
> which is caused by an exception thrown in ResourceResolver.getInstance(Attr uri, String BaseURI) (last line).
> This is clearly a bug as ResolverFragment is a correct resolver for the reported URI (and my implementation is working 99,x% of time).
> I'm pretty sure that cause is in the aforementioned method getInstance(). ResourceResolver maintains a static Vector which is modified at runtime. During initialization (it is a bug but from practical PoV not that severe) and unfortunately also in getInstance() method, what is a problem. When two threads call it the vector might be permuted by first thread when the latter is iterating over it.
> Please notice that it can't be fixed in any code using your library as the method is public and static and also called many times from xmlsec itself. Even if I synchronized each and every call to xmlsec API (forgetting for a moment about terrible performance loss) I'd have no guarantee that other code in my final app won't use your library directly.

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira