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 22:04:42 GMT
BTW - I'm really interested in the reasoning why deserialization code
does not call the non-serializable superclass constructor in the
security context of the subclass(es) - so that it really mimics the
normal constructor call chain.

Michal

Michał Kłeczek (XPro) wrote:
> Isn't the issue with non-serializable superclass constructor call this
> one? :
> http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2008-5353
>
> 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.
>
> Thanks,
> Michal
>
> 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.

Mime
View raw message