Explicit Serialization API and Security

[Peter Firmstone]

Thanks Brian,

Those are good questions, some thoughts and examples inline:

----- Original message -----
> Overall the direction seems promising.   Poking at it a bit...
>
>     - ReadSerial methods are caller-sensitive and only show a class a view
> of its own fields.
>     - Invariant checking is separate from deserialization, and does not
> seem entirely built-in -- subclass constructors seem responsible for
> asking parents to do validity-checking?

My mistake, the last example wasn't quite right, BaseFoo, should call it's own static check method from within it's constructor, rather than from SubFoo.

Each class in the heirarchy needs to check it's invarients using a static method, before calling a superclass constructor, this prevents object construction if an invarient isn't satisfied, because the object isn't created an attacker cannot hold a reference to it.

By the time Object's default constructor is called, even though no fields have been set, invarients have already been checked for every member class in the Object.

An attacker may choose to call another constructor and not pass on ReadSerial, so all constructors need to perform these checks.


>     - I don't see how this invariant-checking mechanism can enforce
> invariants between superclass fields and subclass fields.   For example:
>
> class A {
>           int lower, upper;   // invariant: lower <= upper
> }
>
> class B extends A {
>           int cur;   // invariant: lower <= cur <= upper
> }
>
> To check such an invariant, the serialization library would have to
> construct the object (in a potentially bad state), invoke the checker at
> each layer, and then fail deserialization if any checker said no. 

The serialization framework should only construct an objects fields and make these available from ReadSerial, each class then calls a static method before calling a superclass constructor that's responsible for an objects creation, to prevent construction of the object, if necessary.

Eg, some complexity, but bullet proof:

public class A (

    public final int lower, upper;

     private static boolean check(ReadSerial rs){
        if (rs.getInt("lower") > rs.getInt("upper")) 
             throw new IllegalArgumentException(
                "lower bound must be less than or equal to upper");
       return true;
    } 

    public A(ReadSerial rs){
        this(check(rs), rs);
    }

    private A(boolean checked, ReadSerial rs){
        super();
        lower = rs.getInt("lower");
        upper = rs.getInt("upper");
    }

// other constructors omitted must also check invarients
}

class B extends A {

    public final int cur;

    private static ReadSerial check(ReadSerial rs) {
        A a = new A(rs);
        int cur = rs.getInt("cur");
        if ( a.lower > cur || cur > a.upper ) 
             throw new IllegalArgumentException(
                 "cur outside lower and upper bounds");
        return rs;
    }

    public B(ReadSerial rs) {
        super(check(rs));
        cur = rs.getInt("cur");
    }
}



 But,
> an evil checker could still squirrel away a reference under the carpet.

Not with the above example, if an invarient isn't satisfied, an object instance of B cannot be created.

But circular links are still a challenge...

>
> Another challenge in invariant checking is circular data structures.   If
> you have two objects:
>
> class Brother {
>           final Brother brother;
> }
>
> that refer to each other, an invariant you might want to check after
> deserialization is that
>       this.brother.brother == this
>
> Obviously you have to patch one or the other instance after construction
> to retain the circular references; at what point do you do invariant
> checking?

There are two cases here I think:

One where there is a trust relationship between two classes and the circular relationship is known at compilation time.  At least one will provide a mutable setter method, or a Constructor that accepts an argument with a matching argument type, and the other invokes it during construction, I say trusted because 'this' is allowed to escape.

In Brother's, because the brother field is final, this was constructed with a trust relationship, because 'this' escapes from at least one Brother during construction.  I would argue that the object that allows this to escape should also be responsible for rebuilding the circular relation during deserialisation.

However the Brother that didn't let this escape could be serialised first, in any case both are instances of Brother, so either instance could be a subclass, each can correctly create the other.

public class Brother {

    private static Constructor getBroConstructor(ReadSerial rs){
        // we can check our subclass has permission here if we like,
        // even though an object hasn't been created, its class is
        // in the current execution context.
        Class type = rs.getType("brother");
        if ( !Brother.class.isAssignableFrom(type))
            throw new Exception("not my bro");
        if ( !rs.samePackageAndClassLoader(Brother.class, type){
            Permission toBeMyBro = new BroPermission();
            // gets type's ProtectionDomain and checks it has
            // permission, throws SecurityException
            rs.checkPermission(type, toBeMyBro); 
        }
        Class [] paramTypes = {Brother.class};
        // Check if constructor is accessible.
        // and throw exception here if not.
        return rs.getConstructor("brother", paramTypes);
    }

    final Brother brother;

    public Brother(ReadSerial rs){
        this(getBroConstructor(rs));
    }

    private Brother(Constructor c){
        Object [] params = {this};
        brother = c.newInstance(params);
    }
}

Another option might be to provide something like this

    rs.circularFieldNewInstance("brother", this); 

But that encourages bad practise; 'this' to escape, although it would allow 'this' to be passed as one of the filelds in another ReadSerial instance, but probably not a great idea.

In other cases, there is a circular relationship between untrusted classes and the framework must provide a thread of execution to classes to wire up circular references, after construction.

Circular field references that can only be written after construction must be mutable, or if final, their referent must be mutable, depending on the depth of the circular relationship.

To enable wiring up the circular relationship, a class needs to implement a method for the framework to call after construction, a suggestion:

public interface Circular {
    void setMutableFields(ReadSerial rs);
}

Circular fields missing from ReadSerial during construction are populated after. 

An attacking subclass can override setMutableFields, so the implementing class should make it final, provide and call a protected method for subclasses to implement, should they too contain circular links.

The alternative is to use a private method, but it's a lot more convenient to use an interface.

Unfortunately the only remaining option to protect invarients is to make all methods throw an exception if invarients haven't been satisfied after construction, even if this method throws an exception and halts deserialisation, an attacker can hold a reference to it. 

Cheers,

Peter.

>
> On 1/1/2015 7:43 AM, Peter Firmstone wrote:
> > Subclass example:
> >
> > class SubFoo extends BaseFoo {
> >
> > public static ReadSerial check(ReadSerial rs){
> > if (rs.getInt("y") > 128) throw Exception("too big");
> > return rs;
> > }
> >
> > private final int y;
> >
> > public SubFoo( int x , int y) {
> > super(x);
> > this.y = y;
> > }
> >
> > public SubFoo( ReadSerial rs ){
> > super(BaseFoo.check(check(rs)));
> > // SubFoo can't get at BaseFoo's rs.getInt("x"),
> > // it's visible only to BaseFoo. Instead SubFoo would get
> > // the default int value of 0. Just in case both classes have
> > // private fields named "x".
> > // ReadSerial is caller sensitive.
> > this.y = rs.getInt("y");
> > }
> > }
> >
> > Classes in the heirarchy can provide a static method that throws an
> > exception to check invarients while preventing a finaliser attack. We'd
> > want to check invarients for other constructors also, but for
> > berevity...
> >
> > Eg:
> >
> > class BaseFoo implements Serializable{
> >
> > public static ReadSerial check(ReadSerial rs) throws Exception
> > {
> > if (rs.getInt("x") < 1)
> > throw IllegalArgumentException("message");
> > return rs;
> > }
> > ....
> >
> >
> > Sent from my Nokia N900.
> >
> > ----- Original message -----
> > So, if I understand this correctly, the way this would get used is:
> >
> > class BaseFoo implements Serializable {
> > private final int x;
> >
> > public BaseFoo(ReadSerial rs) {
> > this(rs.getInt("x"));
> > }
> >
> > public BaseFoo(int x) {
> > this.x = x;
> > }
> > }
> >
> > Right?
> >
> > What happens with subclasses?   I think then I need an extra RS arg in
> > my constructors:
> >
> > class SubFoo extends BaseFoo {
> > private final int y;
> >
> > public SubFoo(ReadSerial rs) {
> > this(rs.getInt("y"));
> > }
> >
> > public BaseFoo(ReadSerial rs, int y) {
> > super(rs);
> > this.y = y;
> > }
> > }
> >
> > Is this what you envision?
> >
> >
> >
> >
> >
> > On 12/27/2014 8:03 PM, Peter Firmstone wrote:
> > Is there any interest in developing an explicit API for
> > Serialization?:
> >
> > 1. Use a public constructor signature with a single argument,
> > ReadSerialParameters (read only, writable only by the
> > serialization framework) to recreate objects, subclasses (when
> > permitted) call this first from their own constructor, they have
> > an identical constructor signature.   ReadSerialParameters that are
> > null may contain a circular reference and will be available after
> > construction, see #3 below.
> > 2. Use a factory method (defined by an interface) with one parameter,
> > WriteSerialParameters (write only, readable only by the
> > serialization framework), this method can be overridden by
> > subclasses (when permitted)
> > 3. For circular links, a public method (defined by an interface) that
> > accepts one argument, ReadSerialParameters, this method is called
> > after the constructor completes, subclasses overriding this should
> > call the superclass method.   If this method is not called, an
> > implementation, if known to possibly contain circular links,
> > should check it has been fully initialized in each object method
> > called.
> > 4. Retains compatibility with current serialization stream format.
> > 5. Each serial field has a name, calling class and object reference,
> > similar to explicitly declaring "private static final
> > ObjectStreamField[] serialPersistentFields ".
> >
> > Benefits:
> >
> > 1. An object's internal form is not publicised.
> > 2. Each class in an object's heirarchy can use a static method to
> > check invarients and throw an exception, prior to
> > java.lang.Object's constructor being called, preventing
> > construction and avoiding finalizer attacks.
> > 3. Final field friendly.
> > 4. Compatible with existing serial form.
> > 5. Flexible serial form evolution.
> > 6. All methods are public and explicitly defined.
> > 7. All class ProtectionDomain's exist in the current execution
> > context, allowing an object to throw a SecurityException before
> > construction.
> > 8. Less susceptible to deserialization attacks.
> >
> > Problems:
> >
> > 1. Implementations cannot be package private or private.   Implicit
> > serialization publicises internal form, any thoughts?
> >
> > Recommendations:
> >
> > 1. Create a security check in the serialization framework for
> > implicit serialization, allowing administrators to reduce their
> > deserialization attack surface.
> > 2. For improved security, disallow classes implementing explicit
> > serialization from having static state and static initializer
> > blocks, only allow static methods, this would require complier and
> > verifier changes.
> > 3. Alternative to #2, allow final static fields, but don't allow
> > static initializer blocks or mutable static fields, similar to
> > interfaces.
> >
> > Penny for your thoughts?
> >
> > Regards,
> >
> > Peter Firmstone.
> >