Explicit Serialization API and Security

[David M. Lloyd]

On 01/08/2015 02:32 AM, Peter Firmstone wrote:
> Thank you all for participating in this discussion.
>
> Initially a constructor signature for deserialization was proposed to
> enforce invariants and encapsulation, however there appears to be
> interest in using alternative methods, although they appear to be
> improvements over the status quo, I'm having trouble working out how to:
>
>     * Enforce intra class invariants with alternate proposals without
>       breaking encapsulation.
>     * How the static method proposal can be used to replace final fields
>       referents without needing to implement readObject().
>     * And how the alternative GetFields readObject() implementation can
>       enforce invariants and prevent finalizer attack, while also
>       allowing subclassing?
>
> What started me down this path, is our project (Apache River) is heavily
> dependant on Serialization, I wanted to create a secure
> ObjectInputStream subclass that restricted deserialized classes to those
> I had audited.  My aim was to allow deserialization to enforce security
> using verification, similar to how an http server verifies its input.

Now we're getting down to brass tacks. :-)

Maybe I missed this part of the discussion, but can you elaborate on why 
ObjectInputValidation is inadequate for validating intra-class 
invariants?  I was thinking this is really the only way to effectively 
achieve intra-class validation, because (barring complex graph traversal 
algorithms) there isn't really any better way to guarantee that any 
given subgraph of the object graph is actually fully initialized.

As far as replacing final fields, I think the seed of the best idea came 
from Chris Hegarty - the problem isn't the readObject() method, the 
problem is that there's no way to initialize your final fields if you 
use it.  By adding a "defaultReadFields()" (name not important) method 
like he proposes, you can validate your single class.  But I don't see 
any reason why this couldn't be taken another step farther, and allow 
the readObject method to actually change, add, and remove fields from 
its internal map before doing so.  This would allow readObject to 
arbitrarily transform its content, and assign to final fields, while 
only minimally changing the API and implementation.

> Unfortunately before I can achieve the goal of secure deserialization,
> there's a denial of service issue in ObjectInputStream: I'd also like to
> propose a system property, to allow limiting the size of arrays, created
> during deserialization.   Presently I can craft an inputstream to take
> down the JVM, if I can do it, so can an attacker, I'd like to get that
> fixed if possible.

I agree this would be a very good (and easy to implement) idea.  You can 
limit the underlying stream but that does you no good if the attacker 
can cause the construction of any number of very large objects before 
the end of stream is hit.

I wonder if this could be as simple as adding a protected method on 
ObjectInputStream in the vein of:

   protected boolean validateArrayCreation(Class<?> elementType, int 
dimension) {
       return true;
   }

Or elementType could be arrayType; I don't think it matters too much. 
Method returns false and InvalidObjectException is thrown. 
Implementations could do a simple global max array size, different max 
sizes by primitive vs ref, different by type, or even have a "budget" of 
heap and use a combination of this method and overridden resovleClass() 
to create a rough estimate of memory usage, failing at a heuristic 
threshold.

> I've attached a text file that contains three classes using the original
> proposed serial constructor, A, B and C which have intra class
> invariants that must be satiisfied.  C also contains a circular link and
> implements an interface called Circular, the Serialization framework
> calls it after construction to provide access to fields with circular
> links.  Can the other proposals provide similar safety?
>
> Notes:
>
>    1. The class with the circular link is final and every method checks
>       if invariants are satisfied.
>    2. The method with @SerialConstructor annotation, is the only
>       constructor called by the serialization frame work, other than the
>       Circular interface method, there are no other methods that need to
>       be implemented.
>    3. Encapsulation is preserved, classes have full control over their
>       invariants, their internal implementation and what's serialized.

I was previously firmly of the opinion that a serialization constructor 
is the best way to accomplish this, but this discussion thread has at 
least given me quite some doubt if not outright changed my mind, though 
I think there's still an unanswered question regarding validation (above).

If nothing else, this discussion has yielded some good ideas for 
enhancements to JBoss Marshalling. :-)

-- 
- DML