Explicit Serialization API and Security

[Chris Hegarty]

Hi Peter,

On 2 Feb 2015, at 11:16, Peter Firmstone <peter.firmstone at zeus.net.au> wrote:

> As mentioned I've been experimenting with an invariant validating ObjectInputStream, that's backward and forward compatible with Java's Object Serialization stream protocol.
> 
> No changes have been made to ObjectOutputStream.
> 
> ObjectInputStream has been overridden and reading from the stream has been completely reimplemented.
> 
> Classes are still required to implement Serializable, however readObject methods are not called and fields are not set reflectively after construction.
> 
> After considering all possibilities, I still find myself favouring constructors.

With the use of constructors:
 1) there is no way to reconstruct objects with truly private state
     ( not exposed through the constructor ), 
 2) there is no way to enforce constraints on mutable state which
     may have constraints enforced through the API
 3) Serializable classes are required to expose a public/protected
     single args GetArg constructor, for subclasses to call ( this is
     an issue if you do not control the whole hierarchy )
 4) Subclasses need to make assumptions about abstract
     superclasses, so they can create “fake” instances for checking

See my, not factually correct, example below [*]

> Definition of "Cumbersome":
>    Slow or complicated and therefore inefficient.

With larger hierarchies, and abstract classes, it becomes more difficult [*].

> During implementation, I've found that static invarient check methods are often shared with other constructors.

Yes, they can be somewhat, but will most likely throw different exceptions [*].

> If another constructor already has a static invariant checking method, you only need call that constructor.
> 
> Performance wise, constructors significantly outperform setting every field using reflection, the more fields an Object has, the greater the performance benefit.

Interesting observation.

> My experience is using constructors is often easier to understand than readObject methods.

With larger hierarchies it comes complicated very quickly [*], and easy to miss a call to a check method. That said, I agree readObject methods can be hard to understand sometimes, but they can enforce invariants on truly private, or mutable, state.

> Because constructors can be chained, I can perform final freezes in one constructor, then publish safely using another, existing readObject methods cannot provide this functionality.  If a final freeze occurs after the readObject method terminates, there is no way to fix existing code that uses unsafe publication.

I think we can make the existing serialization mechanism much better when it comes to the setting of finals. Peter Levart and I are already looking at this, and hopefully will come up with a proposal soon.

> See that attached example, this is actual production code (with the addition of @AtomicSerial), Java's RMI also has a DGC implementation.  In this example using standard Serialization, because a final freeze occurs after readObject exits, the implementation uses unsafe publication, all guarantees are off.

Yes, just like the construction of any object, unsafe publication is... well unsafe.

> The annotations I've used are:
> @AtomicSerial - indicates serial constructor is present.
> @ReadInput - provides access to a ReadObject implementation for direct interaction with the stream, prior to object construction, provided for backward compatiblility.
> 
> However the existing readObject alternative is all too often:
>    Insecure and unsafely published.
> 
> The real question:
> 
> Is secure Serialization worth the additional work?

Yes, I think it is worth exploring the possibility.  We have already discussed a number of ideas/alternatives in this email thread, and work is progressing on bringing a number of them to fruition.

> To those who would value it, I think the answer will be yes, to those that don't, perhaps not?
> 
> Regards,
> 
> Peter.

-Chris.


[*]

abstract class Animal implements Serializable {
    private final String category;
    // serializable mutable state
    private long age;
    // serializable state not passed as an arg to the constructor
    private final Object ageLock = new Object();
    protected final boolean hasLegs;

    private static Void check(String category) {
        requireNonNull(category);
        return null;
    }

    public Animal(String category) {
        this(check(category), category);
    }

    private Animal(Void check, String category) {
        this.category = category;
        hasLegs = hasLegs();
    }

    private static Void checkSerial(String category) throws InvalidObjectException {
        try {
            check(category);
        } catch (Exception x) {
            InvalidObjectException e = new InvalidObjectException("Invalid Object");
            e.addSuppressed(x);
            throw e;
        }
        return null;
    }

    protected Animal(GetArg arg) throws InvalidObjectException {
        this(checkSerial(arg.get("category", null)), arg.get("category", null));
    }

    void setAge(long age) {
        if (age < 0)
            throw new IllegalArgumentException();
        synchronized(ageLock) { this.age = age; }
    }
    long getAge() { synchronized(ageLock) { return age; } }

    abstract boolean hasLegs();
}

abstract class Mammal extends Animal implements Serializable {
    private final int numberOfLegs;

    private static Void check(int numberOfLegs) {
        if (numberOfLegs <= 0)   // All mammals must have legs!
            throw new IllegalArgumentException("Invalid number of legs");
        return null;
    }

    public Mammal(String category, int numberOfLegs) {
        this(check(numberOfLegs), category, numberOfLegs);
    }

    private Mammal(Void check, String category, int numberOfLegs) {
        super(category);
        this.numberOfLegs = numberOfLegs;
        assert hasLegs() == hasLegs;
    }

    private static Void checkSerial(GetArg arg) throws InvalidObjectException {
        Animal animal = new Animal(arg) {
            @Override boolean hasLegs() { return false; /* or true, how what this will impact? * */ }
        };
        try {
            check(arg.get("numberOfLegs", -1));
        } catch (Exception x) {
            InvalidObjectException e = new InvalidObjectException("Invalid Object");
            e.addSuppressed(x);
            throw e;
        }
        return null;
    }

    protected Mammal(GetArg arg) throws InvalidObjectException {
        this(checkSerial(arg), arg.get("category", null), arg.get("numberOfLegs", -1));
    }

    @Override boolean hasLegs() { return true; }
    
    abstract boolean hasFur();
}

class Dog extends Mammal implements Serializable {
    private final String breed;

    private static Void check(String breed) {
        requireNonNull(breed); return null;
    }

    public Dog(String breed) {
        this(check(breed), breed);
    }

    private Dog(Void check, String breed) {
        super("canine", 4);
        this.breed = breed;
    }

    private static Void checkSerial(GetArg arg) throws InvalidObjectException {
        Mammal mammal = new Mammal(arg) {
            @Override boolean hasLegs() { return false; /* or true, how what this will impact? * */ }
            @Override boolean hasFur() { return false; /* or true, how what this will impact? * */ }
        };
        try {
            check(arg.get("breed", null));
        } catch (Exception x) {
            InvalidObjectException e = new InvalidObjectException("Invalid Object");
            e.addSuppressed(x);
            throw e;
        }
        return null;
    }

    protected Dog(GetArg arg) throws IOException {
        this(checkSerial(arg), arg.get("breed", null));
    }

    @Override boolean hasLegs() { return true; }
    @Override boolean hasFur() { return true; }
}