river-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Michał Kłeczek (XPro)" <michal.klec...@xpro.biz>
Subject Re: Security
Date Thu, 19 Feb 2015 21:55:05 GMT
Isn't the issue with non-serializable superclass constructor call this
one? :

If so - I don't really see how it relates to River - to be able to
expoit this kind of vulnerability an attacker must have already
downloaded and run his code - otherwise the exploiting subclass could
not have been loaded hence no construction would take place.
I think if we can make sure only trusted (whatever that means :) ) code
is ever run - we don't have to do anything more with (de)serialization -
except making DoS more difficult by restricting size of the stream.


Peter Firmstone wrote:
> Continuing on ...
> Lets say for example, we have a secure OS and we provide a service on
> a public port and we have a determined attacker attempting to use
> deserialization to take over our system or bring it to its knees with
> denial of service.
> We know this is relatively easy with standard ObjectInputStream.
> When we invoke a remote method, the return value is what you call a
> root object, that is it's the root of a serialized object tree,
> originating through the root object, via its fields.
> Most Serializable objects have invariants.
> We can consider the java serialization stream format as a string of
> commands that allows creation of any object, bypassing all levels of
> visibility.  That is an attacker can create package private classes or
> private internal classes, provided they implement Serializable.  So
> think of Serializable as a public constructor that lacks a context
> representing the attacker.  That's right, there is no context
> representing the remote endpoint in the thread call stack.
> Now we can place restrictions on the stream, such as a requirement
> that the stream is reset before a limit on the number of objects
> deserialized is reached.  We can also place a limit on the count of
> all array elements read in from the stream, until the stream is
> reset.  These measures ensure the stream cannot go on forever, they
> also prevent stack overflow and out of memory errors.  At some point
> the caller will regain control of the stream without running out of
> resources.
> But getting back to the previous problem, the attacker has command
> line access to create any Serializable object.
> Now most objects have invariants that should be satisfied, for correct
> functional state, but a major problem with Serialization is it creates
> an instance, using the first zero arg constructor of the first non
> serializable superclass.  Yep that's right, that could be ClassLoader,
> you clever attacker you!  The poor old object hasn't even had a chance
> to check it's invariants and it's game over, that's not fair!
> This will never do, I had to create a new contract for atomic object
> construction:
>   1. The class must implement Serializable (can be inherited) AND one
>      of the following conditions must be met:
>   2. The class must be annotated with @AtomicSerial OR
>   3. The class must be stateless and can be created by calling
>      class.newInstance() OR
>   4. The class must have DeSerializationPermission
> Now if the class is annotated with @AtomicSerial, it must also have a
> constructor signature that has public or default visibility:
> SomeClass(GetArg arg) throws IOException{
>     // The first thing I must do is to call a static method that
> checks invariants, before calling a superclass constructor, the static
> method should return the argument for another constructor.
> }
> Some simple rules for our object input stream:
>   1. Though shalt not publish a reference to a partially constructed
>      object.
>   2. Though shalt not publish a reference if an object fails
>      construction, where readObject, readObjectNoData and readResolve
>      methods are considered to be constructors for the purpose of
>      deserialization of conventional Serializable objects.
>   3. Though shalt not attempt to construct a Serializable object that
>      doesn't have an @AtomicSerial annotation, or has serial fields
>      (state) and doesn't have DeSerializationPermission.
>   4. Only call a protected constructor if the class doesn't implement
>      @AtomicSerial and has DeSerializationPermission.
>   5. Do not honour the standard java Serialization construction
>      contract (not all Serializable classes can be constructed even if
>      they have DeSerializationPermission).
>   6. If a standard Serializable class has DeSerializationPermission for
>      it to be constructed it must have a zero arg constructor or a
>      constructor that accepts null object arguments and default
>      primitive values.
>   7. If an any object in a serialized object graph fails it's invariant
>      checks, deserialization of the object graph fails at that point
>      and control is returned to the caller, by way of an
>      InvalidObjectException (a child class of IOException).
>   8. Honour the standard java serialization stream protocol.
>   9. Count number of objects received and throw a
>      StreamCorruptedException if limit is exceeded.
>  10. Count number of array elements received and throw
>      StreamCorruptedException if limit is exceeded.
>  11. readResolve() can be called on @AtomicSerial instances but,
>      readObject() is never called and neither is readObjectNoData().
> Obligations of our object output stream:
>   1. Reset the stream before the limit is reached.
>   2. Replace java collections and maps with safe @AtomicSerial
>      implementations, it is the developers obligation to replace them
>      with their preferred implementations during construction, these
>      are functional but are only immutable containers for keys, values
>      and comparators.
>   3. Honour java's serial stream protocol.
> Some simple rules for developers implementing @AtomicSerial:
>   1. Check all invariants from a static method called by your class
>      constructor prior to calling a superclass constructor.
>   2. Check types of values in collections and maps and keys in maps.
>   3. Check field types.
>   4. Copy or clone arrays and collections.
>   5. Do not call a super class constructor until all invariants have
>      been checked.
>   6. Handle failed invariants by throwing an InvalidObjectException,
>      again make sure you did not call the super class constructor.
>   7. Implement the interface ReadObject if you want to duplicate
>      existing readObject method functionality, this will allow you to
>      communicate with the stream prior to calling a super class
>      constructor, so you can check invariants.  Annotate a static
>      method that returns an instance of ReadObject with @ReadInput.
>   8. Your ReadObject instance can be retrieved by from GetArg, your
>      ReadObject instance will have read the stream prior to your
>      constructor having been called.
>   9. Don't call defaultReadObject() from your ReadObject implementation!
>  10. Check all fields can be retrieved, prior to calling a super class
>      constructor.
>  11. Beware of the implicit super class constructor.
>  12. Enum, and Proxy instances are considered secure, don't bother
>      subclassing proxy, your class won't be deserialized.
>  13. InvocationHandler's need to implement @AtomicSerial.
> Note an object instance is not created until the default constructor
> in java.lang.Object is called.
> Golden rules:
>   1. Do not create arrays or collections blindly by reading in an
>      integer or long, length or size from the stream to pass as an
>      argument to an array or collection constructor.
>   2. Let the stream be responsible for array creation.
>   3. Clone or copy arrays and collections.
>   4. Clone or copy mutable objects, these may be shared by other
>      objects in the stream.
>   5. Don't grant DeSerializationPermission unless you're really really
>      sure you know what your doing, you must have access to the source
>      code of the object you want to deserialize, you must make sure it
>      is secure, if in doubt ask first.
> Now before anyone goes off thinking "Oh no I don't need this
> functionality, do I have to use it?"  The answer is no, it will be set
> via configuration constraints.  If an endpoint doesn't support
> AtomicValidation, it will prevent ObjectInputStream creation.  If an
> object in the stream doesn't support it, deserialization will fail and
> control will be returned to the caller.
> Other than configuration, this is invisible to clients, only service
> api parameters and return values and private service classes, such as
> smart proxy's and proxy verifiers need implement it.
> Note: I have tested both Reggie and Outrigger, standard serial form is
> preserved for all objects, all tests pass with AtomicValidation.YES
> enabled.
> Clients will be able to be configured with InvocationConstraints;
> AtomicValidation.YES, Integrity.YES and Confidentiality.YES.
> The client can also require DownloadPermission by specifying either of
> the properties:
> net.jini.loader.ClassLoading.provider=net.jini.loader.pref.RequireDlPermProvider
> OR
> java.rmi.server.RMIClassLoaderSpi=net.jini.loader.pref.RequireDlPermProvider
> If the first property is specified, it isn't required to be loaded by
> the system ClassLoader.
> The next step is to provide a service interface for a bootstrap proxy
> to return a smart proxy and to define a suitable Entry for the
> bootstrap proxy to declare the interfaces it's smart proxy implements.
> Clients can then lookup bootstrap proxy, based on the Entry,
> authenticate the bootstrap proxy and grant it DownloadPermission
> dynamically.  The client will then need to prepare the smart proxy,
> placing constraints on it.
> While the bootstrap proxy should have an AtomicVerification.YES
> constraint, the endpoint for the smart proxy doesn't have to, so after
> trust has been established, it is possible to run with standard
> serialization.  In this case the smart proxy itself must be
> serializable using @AtomicSerial, but objects serialized over the
> smart proxy's endpoint's need not.  I've also have made it possible
> for proxy codebases to declare the permissions they require so these
> can be granted dynamically after trust is established.
> This would also provide the client with delayed unmarshalling
> functionality, while preserving the standard ServiceRegistrar
> interface, which can be managed using ServiceDiscoveryManager, and
> ServiceItemFilter.
> In other words, this additional functionality requires minimal effort
> on behalf of the developer, while those who don't need it can remain
> blissfully ignorant and implement it later if they want to.
> I haven't uploaded to svn, would you like to see some patches?
> Regards,
> Peter.

View raw message