Re: [DISCUSS] Proposed fix for RIVER-149 - ServerContext should not rely on system classloader

[Greg Trasuk <tr...@stratuscom.com>]

On Mon, 2011-11-28 at 11:16, Gregg Wonderly wrote:
> On 11/27/2011 6:31 PM, Greg Trasuk wrote:
> > Hi all:
> >
> > I've attached a patch to RIVER-149
> > (https://issues.apache.org/jira/browse/RIVER-149), to switch to usage of
> > the current thread's context classloader.
> >
> > I realize that commit-then-review is the usual practice, but since
> > classloading is fundamental to Jini, I thought I'd seek review first.
> >
> > The JIRA comments give a good background on the bug and the patch.
> One of the things that I wondered when looking at this, was if we wanted to make 
> use of a permission which involved the class name that will be loaded.  In a 
> sense, that's a pretty big barrier for ease of use, but it does provide a 
> control point to deal with security issues.  The use of the "jars" that the 
> class loader is using, is an externally configured bit.  So, from a permission 
> perspective, there has already be a form of a "grant" made.   The question is, 
> whether or not the use of a specific SPI class is another control point, worth 
> using.
> 
> Gregg Wonderly

Looking a little more closely into the usage of ServerContext, I suspect
that this bug is sort of a non-issue.

The ServerContext class is used by a service instance while processing a
remote method call, to find out about the context of the remote method
call.  It seems a little misnamed, since context would typically contain
implementations of classes defined in 'net.jini.io.context', such as
'ClientHost' and 'ClientSubject'.  In any case, the server context would
usually be set by an invocation target (local bridge to the service
instance).  For instance, a JERI exporter creates a target that sets the
context explicitly, to contain things like "ClientSubject",
"ClientHost", etc.

If, however, the server context is not set by the invocation target,
then ServerContext uses the Spi mechanism, looking up a list of
providers in META-INF.  This situation only occurs if a non-JERI
exporter, like JRMP is used (there might be other possibilities, but
JRMP is the only one currently in River), that handles its own endpoints
and does not invoke the service implementation through
ServerContext.doWithServerContext().  The ServerContext.Spi
implementation provides a way of inserting code that can talk to the
underlying JRMP implementation, and allows for extension to other
non-JERI providers.

JRMP is not exactly deprecated, but we mostly want to use the JERI
mechanism, so the dependence on the system classloader should not
manifest itself.  However the search for providers is executed on
initialization of ServerContext.  Altering ServerContext to initialize
the provider list in a lazy fashion (ie only if it is asked to provide a
ServerContext and a ServerContext isn't already set) would prevent the
bug appearing, at least in the container that I'm working on.

To Gregg's point on security:

Presumably an exploit would involve something like this:
- Application running inside the container finds a service proxy to some
malicious service.
-  You might be able to weed this out using the proxy trust mechanism
- Application makes a call on the malicious proxy, which creates a false
ServerContext and makes a call to some code using it.  One way of doing
this might be to override the list of ServiceContext providers.  Another
way would be to simply call ServerContext.doWithServerContext (currently
unprotected).  This call would have to be done on a thread that was not
part of a remote request, because presumably the service was exported
using JERI, so the server context would already be set, giving an
exception if the malicious code attempted to set it again (ServerContext
does not nest).

If this were a concern, the application code could go ahead and set an
empty context before calling the proxy.

I'll have to do more thinking on the security implications, but I don't
think that an extra permission check as Gregg describes would be
necessary.

I'm now leaning towards leaving the reference to the system classloader
(just in case some existing systems use that dependency) and switching
to lazy instantiation of the list of Service Context providers, which
would eliminate manifestations of the bug inside a JERI service dispatch
cycle, solving the container problem for most use cases.

Thoughts?

Greg Trasuk.