DeserializationPermission Proposal

[Peter Firmstone]

Hi Dave,

You've pretty much got it right, although I'm not proposing making the 
permissions fine grained to class name level.

There are different DeSerializationPermission's however:

"PROXY" - controls dynamic proxy creation (Some basic invariants are 
checked prior, the InvocationHandler field object is passed to the 
dynamic proxy's constructor).
"ATOMIC" - controls which domains are allowed to use atomic 
deserialization constructor.
"MARSHAL" - allows deserialization of MarshalledObject instances (Again 
invariants are checked prior).
"EXTERNAL" - allows deserialization of or Externalizable objects.

Serializable classes that are stateless or have only primitive fields 
don't require any permission and remain unchanged.

Serializable classes that contain Object fields or reads information 
directly from the stream require an atomic constructor, identified with 
@AtomicSerial, otherwise they are not Serializable.

There is also an atomic constructor available for Externalizable 
objects, identified with @AtomicExternal, however this isn't required.

AtomicMarshalInputStream also provides a <T> T readObject(Class<T> type) 
method that reads ahead and prevents deserialization if the object isn't 
the correct type. There's another annotation for readResolve methods to 
identify the replacement object type, as readResolve doesn't allow 
covariant return types.


Some debugging output:

NonActGrp-out: access: access allowed ("java.util.PropertyPermission" 
"sun.net.maxDatagramSockets" "read")
NonActGrp-out: access: access allowed ("java.net.SocketPermission" 
"localhost:4160" "listen,resolve")
NonActGrp-out: access: access allowed ("java.net.SocketPermission" 
"224.0.1.84" "connect,accept,resolve")
NonActGrp-out: access: access allowed ("java.io.FilePermission" 
"\C:\Users\peter\Documents\NetBeansProjects\input-validation-for-serialization\lib\mahalo.jar" 
"read")
NonActGrp-out: access: access allowed ("java.lang.RuntimePermission" 
"accessClassInPackage.sun.reflect")
NonActGrp-out: access: access allowed ("java.net.SocketPermission" 
"localhost:0" "listen,resolve")
NonActGrp-out: access: access allowed ("java.lang.RuntimePermission" 
"accessDeclaredMembers")
NonActGrp-out: access: access allowed 
("net.jini.discovery.DiscoveryPermission" 
"QATestDefaultGroup_medusa_1455617825772")
NonActGrp-out: access: access allowed ("java.io.SerializablePermission" 
"enableSubclassImplementation")
NonActGrp-out: access: access allowed ("java.io.SerializablePermission" 
"enableSubstitution")
NonActGrp-out: access: AccessControlContext invoking the Combiner
NonActGrp-out: access: access allowed 
("org.apache.river.api.io.DeSerializationPermission" "ATOMIC")
NonActGrp-out: access: AccessControlContext invoking the Combiner
NonActGrp-out: access: access allowed 
("org.apache.river.api.io.DeSerializationPermission" "PROXY")
NonActGrp-out: access: AccessControlContext invoking the Combiner
NonActGrp-out: access: access allowed 
("org.apache.river.api.io.DeSerializationPermission" "ATOMIC")
NonActGrp-out: access: access allowed ("java.lang.RuntimePermission" 
"createSecurityManager")
NonActGrp-out: access: access allowed 
("org.apache.river.api.io.DeSerializationPermission" "ATOMIC")
NonActGrp-out: access: access allowed ("java.io.SerializablePermission" 
"enableSubclassImplementation")
NonActGrp-out: access: access allowed ("java.io.SerializablePermission" 
"enableSubstitution")

Regards,

Peter.

> Things are getting confused (for me anyway) so I'm going to try and
> spell out each idea separately.
>
> Given a class hierarchy H of:
>
> C ->  B ->  A ->  NonSerializableClass ->  Object
>
> where C, B, and A are serializable classes, and protection domain M
> which is initiating deserialization, I think the following requirements
> definitely apply:
>
> * When M deserializes an instance of A, both M and the protection domain
> of A must be sufficient to allow the deserialization of A
> * When M deserializes an instance of B, both M and the protection domain
> of B must be sufficient to allow the deserialization of B
> * When M deserializes an instance of C, both M and the protection domain
> of C must be sufficient to allow the deserialization of C
> * Extrapolate generally
>
> The following requirements have definite pros and cons, which is what I
> think we're touching on in this thread:
>
> * When M deserializes an instance of B (or any subtype thereof), the
> protection domain of A must also be sufficient to allow the
> deserialization of B
> * When M deserializes an instance of C (or any subtype thereof), the
> protection domains of both A and B must also be sufficient to allow the
> deserialization of C
> * Extrapolate generally
>
> Based on the above, the following requirements might or might not make
> sense:
>
> * When M deserializes an instance of A, the protection domains of
> NonSerializableClass and also Object must also be sufficient to allow
> the deserialization of A
> * Likewise for B&  C, extrapolate generally
>
> Regardless of which is the best set of requirements, one of the major
> complicating questions is, what is the ideal representation of the
> permission(s) needed to allow the above?  I can see several use cases:
>
> 1. Grant the ability for a serializable class S to deserialize itself
> 2. Grant the ability for M to deserialize a specific class (by name?)
> 3. Grant the ability to any (serializable?) class X to allow
> deserialization of a specific subtype (by name?)
> 4. Grant the ability to any (serializable?) class X to allow
> deserialization of any subtype (think java.lang.Object, or general (but
> harmless) base classes like java.util.EventObject)
>
> The big problem with permissions by name is that names are completely
> relative.  It is totally valid (and happens in the wild today) that a
> serialization graph contains classes from a variety of class loaders
> which are isolated from one another - in fact, it's even possible to
> have more than one class with the same class name and the same
> superclass in the same graph!  Given this fact, a flat class namespace
> *cannot* be assumed.
>
> I think that the *only* possible remedy for this is that it should be up
> to each class' class loader (and/or module) to determine what permission
> is required in order to deserialize that class.  In this way, the JDK
> (9) can use a permission of the form class+module name for installed
> module classes, Java EE systems could use permissions in the form of
> application+module+jar+class name for deployment, OSGi systems could use
> permissions including bundle identification information, etc.
>
> To be secure, the default class loader implementation of this method
> would have to produce a result which causes all checks to fail,
> preventing deserialization in a security manager environment of classes
> whose class loaders do not explicitly allow for it.
>
> On 02/12/2016 12:30 AM, Peter Firmstone wrote:
> >/  Thanks David,
> />/
> />/  I'd originally considered something like that, but I later realised I'd
> />/  need to grant DeserializationPermission to other domains I might not
> />/  want allow objects to be deserialized with, simply because their domain
> />/  is included in the call stack.  It looked like it would widen the scope,
> />/  which was contrary to the intended goal of minimising the attack
> />/  surface.  In the end I decided to limit the context to only the domains
> />/  of classes involved in deserialization.
> />/
> />/  Cheers,
> />/
> />/  Peter.
> />/
> />>/  On 02/11/2016 03:52 AM, Peter Firmstone wrote:
> />>/  >/  An example might be more useful.
> />>/  />/
> />>/  />/  Traditionally, when the first non serializable superclass zero
> />>/  argument
> />>/  />/  constructor is called, the domains of the subclasses aren't
> />>/  present on
> />>/  />/  the call stack.  Any security checks performed in the constructor
> />>/  of the
> />>/  />/  superclass will not include the subclasses domains.
> />>/  />/
> />>/  />/  In the proposed case, prior to construction, all domains in the
> />>/  class
> />>/  />/  heirarchy of the to be deserialized object via the local
> />>/  />/  ObjectStreamClass, are added to an AccessControlContext, which is
> />>/  then
> />>/  />/  passed to the SecurityManager two argument permission check.
> />>/  /
> />>/  Sure, that makes sense; in fact this could be a very good
> />>/  standalone/incremental enhancement.
> />>/
> />>/  >/  Now an attacker will not be able to construct this object unless all
> />>/  />/  domains have DeserializationPermission.
> />>/  />/
> />>/  />/  If we use the stack context, it won't contain the yet to be
> />>/  deserialized
> />>/  />/  classes.
> />>/  /
> />>/  True; perhaps the ideal solution would use the initial context plus a
> />>/  per-deserializing-class context, so that both the original caller and
> />>/  the class being deserialized have permissions.  This would have the
> />>/  advantage of consistent behavior, and would also allow the PD of each
> />>/  class to restrict whether it can be deserialized (which would apply
> />>/  globally no matter who was doing the deserialization in the VM).
> />>/
> />>/  --
> />>/  - DML
> /
> -- 
> - DML