You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@santuario.apache.org by "Eric Johnson (JIRA)" <ji...@apache.org> on 2012/07/31 20:18:34 UTC

[jira] [Created] (SANTUARIO-337) ResourceResolver does thread-unsafe handling of the "secureValidation" flag

Eric Johnson created SANTUARIO-337:
--------------------------------------

             Summary: ResourceResolver does thread-unsafe handling of the "secureValidation" flag
                 Key: SANTUARIO-337
                 URL: https://issues.apache.org/jira/browse/SANTUARIO-337
             Project: Santuario
          Issue Type: Bug
          Components: Java
    Affects Versions: Java 1.5.2
            Reporter: Eric Johnson
            Assignee: Colm O hEigeartaigh
            Priority: Minor


>From ResourceResolver.getInstance(), with parts elided for brevity & clarity:

            for (ResourceResolver resolver : resolverList) {
                ResourceResolver resolverTmp = resolver;
                if (!resolver.resolverSpi.engineIsThreadSafe()) {
                    // generate a new instances of resolverSpi ...
                }

                // ...

                resolverTmp.resolverSpi.secureValidation = secureValidation;
                if ((resolverTmp != null) && resolverTmp.canResolve(uri, baseURI)) {
                    // Check to see whether the Resolver is allowed
                    // check for certain types ....
                    return resolverTmp;
                }
            }

In case you didn't see it, the trouble is the juxtaposition of "resolverSpi.engineIsThreadSafe()" followed by code that sets "secureValidation" on the very same instance of the spi, whether or not it is thread safe.

Meaning, if two threads resolve at the same time, and one thread is attempting secure resolution while the other is not, all the "thread safe" resolvers risk a race condition where they will now be in an uncertain state. Of course, it turns out that all the included resolvers declare themselves thread-safe, so this potentially magnifies the problem.

In practice, this is not likely ever to occur, because any given application will likely share the same notion of "secureValidation".

Three observations

#1 - the secureValidation flag needs only be set for the resolver that is actually chosen

#2 - the secureValidation flag should instead be passed as a parameter ResourceResolverSpi.engineResolve() method, not stored as data in the SPI instance.

#3 - if there were ever a resolver to show up that isn't thread safe, and it also has properties, the logic above which creates a new instance of the SPI would discard the properties set on the registered spi. It turns out only the HTTP resolver uses these properties for anything, so this may not be problematic in the real world.

Originally reported via email:
http://article.gmane.org/gmane.text.xml.security.devel/7647

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (SANTUARIO-337) ResourceResolver does thread-unsafe handling of the "secureValidation" flag

Posted by "Colm O hEigeartaigh (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/SANTUARIO-337?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13426502#comment-13426502 ] 

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

Hi Eric,

Could you submit a patch for this issue? One downside to adding the secureValidation flag to the ResourceResolverSpi.engineResolve() method is that it breaks backwards compatibility for any custom ResourceResolverSpi implementations. I don't see a way around this though unfortunately.

Colm.
                
> ResourceResolver does thread-unsafe handling of the "secureValidation" flag
> ---------------------------------------------------------------------------
>
>                 Key: SANTUARIO-337
>                 URL: https://issues.apache.org/jira/browse/SANTUARIO-337
>             Project: Santuario
>          Issue Type: Bug
>          Components: Java
>    Affects Versions: Java 1.5.2
>            Reporter: Eric Johnson
>            Assignee: Colm O hEigeartaigh
>            Priority: Minor
>
> From ResourceResolver.getInstance(), with parts elided for brevity & clarity:
>             for (ResourceResolver resolver : resolverList) {
>                 ResourceResolver resolverTmp = resolver;
>                 if (!resolver.resolverSpi.engineIsThreadSafe()) {
>                     // generate a new instances of resolverSpi ...
>                 }
>                 // ...
>                 resolverTmp.resolverSpi.secureValidation = secureValidation;
>                 if ((resolverTmp != null) && resolverTmp.canResolve(uri, baseURI)) {
>                     // Check to see whether the Resolver is allowed
>                     // check for certain types ....
>                     return resolverTmp;
>                 }
>             }
> In case you didn't see it, the trouble is the juxtaposition of "resolverSpi.engineIsThreadSafe()" followed by code that sets "secureValidation" on the very same instance of the spi, whether or not it is thread safe.
> Meaning, if two threads resolve at the same time, and one thread is attempting secure resolution while the other is not, all the "thread safe" resolvers risk a race condition where they will now be in an uncertain state. Of course, it turns out that all the included resolvers declare themselves thread-safe, so this potentially magnifies the problem.
> In practice, this is not likely ever to occur, because any given application will likely share the same notion of "secureValidation".
> Three observations
> #1 - the secureValidation flag needs only be set for the resolver that is actually chosen
> #2 - the secureValidation flag should instead be passed as a parameter ResourceResolverSpi.engineResolve() method, not stored as data in the SPI instance.
> #3 - if there were ever a resolver to show up that isn't thread safe, and it also has properties, the logic above which creates a new instance of the SPI would discard the properties set on the registered spi. It turns out only the HTTP resolver uses these properties for anything, so this may not be problematic in the real world.
> Originally reported via email:
> http://article.gmane.org/gmane.text.xml.security.devel/7647

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira