Explicit Serialization API and Security

[Peter Levart]

Hi Peter and others,

A assume your approach described here (to use constructors for 
deserialization) is motivated mainly by the fact that only constructors 
are allowed to set the final fields. Otherwise the explicit features of 
your ReadSerial API are more or less accessible even now by using 
"serialPersistentFields " static field for declaration of "serialized" 
fields, combined with ObjectOutputStream.PutField and 
ObjectInputStream.GetField API, accessible from writeObject/readObject 
methods. readObject() method already acts like a constructor, whith the 
following notable differences:

1 - it is not chained explicitly to superclass (either in source or 
automatically by javac) like constructor, but is invoked by 
(de)serialization infrastructure in correct order for each class in the 
hierarchy that declares it.
2 - it is a normal method, so it can be called at any time by code in 
the declaring class or inner classes.
3 - it can't set final fields (without using clumsy reflection)


The 1st point is actually in favor of readObject() method, since the 
(de)serialization API need not be caller-sensitive. The correct 
per-class context is established before each readObject call-back.
2nd and 3rd points are readObject drawbacks and are both interconnected. 
If readObject was not universally callable, then the restriction that it 
can't set final fields could be lifted.

Perhaps it is too late from compatibility perspective to change the 
readObject's 2nd and 3rd point now, but we could compatibly change the 
1st point of a special kind of constructor.

Say that a constructor declared with exactly one parameter of type 
ObjectInputStream is treated specially by javac and reflection:
- constructor code can't chain to superclass constructor
- javac does not automatically generate default constructor chaining code
- javac does not allow calling such constructor from any code
- this is optional: javac does not allow calling instance methods and 
explicit use of 'this' keyword in deserialization constructor code 
(preventing escape of such object before de-serialization ends) - to be 
really safe, this would probably have to be accompanied with verifier 
checks too.
- public reflection API does not allow invoking such constructor (only 
deserialization infrastructure can - like it can invoke the default 
constructor of a non-Serializable superclass on an already allocated 
instance of a sublclass)

No additional changes to VM are apparently necessary (except verifier). 
Declaring such constructor as an alternative to readObject() method is 
then possible to make deserialization more safe.

Regarding finalizability (and finalizer attacks):

The serialization specifies that for 1st (most specific) 
non-Serializable superclass of a Serializable subclass, default 
constructor is invoked to initialize the non-Serializable superclass 
state before deserializing subclass state. As each Serializable class 
has such a non-Serializable superclass (at least in the form of Object 
class) and normal constructors are chained, it is apparent that Object's 
default constructor is called before any deserialization of state 
begins. As soon as Object's constructor completes, the instance becomes 
eligible for finalization:

  "An object o is not finalizable until its constructor has invoked the 
constructor for Object on o and that invocation has completed 
successfully (that is, without throwing an exception).”

The rule of calling non-Serializable superclass default constructor 
could be weakened. If 1st (most specific) non-Serializable superclass of 
a Serializable subclass is Object, then it's constructor is not called. 
Obviously it is not needed since Object has no state to initialize. What 
we achieve by this is that such object does not become eligible for 
finalization just yet. Serialization infrastructure must then explicitly 
register such objects for finalization, but only after their 
de-serialization completes normally. This would prevent finalization 
attacks.

Regards, Peter


On 01/05/2015 01:01 PM, Peter Firmstone wrote:
> My mistake, thank you.
>
> Peter.
>
> On 5/01/2015 9:57 PM, David Holmes wrote:
>> Hi Peter,
>>
>> Did you send this to the list? I haven't seen it turn up yet.
>>
>> David
>>
>> On 5/01/2015 6:51 PM, Peter Firmstone wrote:
>>> Thanks Dave,
>>>
>>> That's another way of checking invariants, you could expose A's check
>>> method but you'd also need a couple of additional static methods to
>>> obtain integers upper and lower from ReadSerial, so that B can ensure
>>> its invariant.
>>>
>>> ReadSerial is caller sensitive so B can't obtain A's stream values via
>>> ReadSerial and must do so via A's API.  This prevents the 
>>> publication of
>>> A's implementation, you don't wan't B depending on A's serial form.
>>> Instead A can reorder and rename it's fields and change their values,
>>> have multiple serial forms and be able to remain compatible with all
>>> forms relatively easily.
>>>
>>> There are some benefits to using a temporary object instance of A; it
>>> reduces the amount of code required, eg it would be a beneficial for 
>>> RMI
>>> to minimise code downloads, the JVM is free to inline access to these
>>> fields and the temporary instance of A never leaves the cpu cache, so
>>> it's not likely to become a performance issue.  Well it might on real
>>> time java :)
>>>
>>> If circular relationships are mutable, or effectively mutable after
>>> publication, then we could eventually deprecate the requirement to
>>> support special treatment of final fields for Serializable.
>>>
>>> Cheers,
>>>
>>> Peter.
>>>
>>> On 5/01/2015 10:38 AM, David Holmes wrote:
>>>>
>>>>>>       - 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
>>>>>> }
>>>>>>
>>>>>
>>>>> 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");
>>>>>      }
>>>>> }
>>>>>
>>>>>
>>>
>