On 3/01/2015 9:24 PM, Peter Firmstone wrote:
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);

It doesn't seem practical in general to have to create an instance of your supertype to validate the passed in serialized form. Why not expose the check method?

David
-----

         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.


Reply via email to