Explicit Serialization API and Security

[Chris Hegarty]

On 15/01/15 20:33, Peter Firmstone wrote:
> Thanks Chris,
> 
> WRT circular references, is it possible to detect and delay setting
> these until after all verifiers run?

It is possible to detect the circular reference.

Currently you can retrieve a circular reference ( in readObject ) from readFields/GetFields, and all its fields will be their default value, null, 0, etc. How useful is it to be able to retrieve this reference? Probably not very useful.

Could a static validator(GetField) API throw, or just return null, for a not yet created circular reference field value? It doesn’t seem useful for validation to be able to retrieve the circular reference.

> An option might be to provide a protected method in ObjectInputStream to
> opt out or switch off support for circular references.

As of today, you can explicitly call readUnshared ( rather then readObject ), and if there is a cyclic reference to the object being deserializing will fail. Alternatively, if you want to protect individual fields, you can use serialPersistentFields and explicitly mark the field as unshared. Both mechanisms are inflexible.

Note: unshared above is not limited to cyclic references, but all references in the stream.

Maybe there is scope for such a switch, or similar, to disable circular references, for part of the graph, without completely disabling other references?

> Apart from Serialization, another common use for readObject is for class
> evolution.

True, validation and sometimes evolution.

> Our projects uses Serialization more than most, it was originally
> written by the same people who earlier implemented Java's Serialization
> framework.
> 
> It's also the root cause of our security problems, I'm presently working
> on a complete reimplimentation of ObjectInputStream using Apache
> Harmony's code, as you might imagine I'm running into a lot of trouble
> instantiating objects in a secure and cross platform compatible fashion.
> 
> Can I suggest we have the static verifier return GetFields, to leave the
> door open to supporting a constructor in future?

If we finally agree upon a static validate(GetField) API, are you suggesting that the return type be GetField, and that an implementation of it would typically return the given arg? Or something else.

-Chris.

> Regards,
> 
> Peter.
> 
> ----- Original message -----
> > On 15 Jan 2015, at 12:25, Peter Firmstone
> <peter.firmstone at zeus.net.au <mailto:peter.firmstone at zeus.net.au>>
> > wrote:
> >
> > > Chris,
> > >
> > > Can you explain some of the detail in your failure atomicity work?
> >
> > Certainly.
> >
> > The current implementation of defaultReadObject sets non-primitive field
> > values one at a time, without first checking that all their types are
> > assignable. If, for example, the assignment of the last non-primitive
> > value fails, a CCE is thrown, and the previously set fields remain set.
> > The setting of field values can be deferred until after they have been
> > "validated", at a minimum that the non-primitive types are assignable.
> > This is achievable per Class in the hierarchy, and provides some
> > additional level of confidence. A change along the lines of this should
> > not be controversial ( either all fields will be set, or contain their
> > default values ), but it is only per Class in the hierarchy.
> >
> > Pushing on this a little, and ignoring cyclic references for now.  For
> > the first Serializable type in the hierarchy ( closest to j.l.Object ),
> > there is no need to eagerly create the instance before “validating” the
> > field values. If the field values are assignable, then the object can
> > created and the values assigned. If a field value is found to be
> > unassignable, then CCE is thrown, and there is no dangling object.
> >
> > Pushing a little more. If there are no readObjectXXX methods in the
> > hierarchy, then it would seem possible to read the field values for all
> > the types in the hierarchy, and “validate” them ( ensure they are
> > assignable ), before creating the object and assigning the values. I do
> > not believe that there are any observable affects from doing this.
> >
> >
> > Cyclic references: Assumption, we do not want to leak our secure
> > deserialized objects. It seems reasonable to not share the reference in
> > the stream. This would rule out cyclic references.  If you want sharing,
> > then the implementation would create the referenced object, and assign
> > the already read field values, when it encounters a cyclic reference.
> > But you open the object to anything in the stream referencing it.
> >
> > readObjectXX:  Assumption, many readObject methods exist to check
> > invariants of values read from the stream. If we had another mechanism,
> > similar to the static validate(GetFields) method we discussed
> > previously, then may readObject methods could be eliminated. In which
> > case we can achieve whole hierarchy failure atomicity ( as described
> > above ).
> >
> > Given this, it seems like the natural place for a static invariant
> > checker is between the reading and reconstitution of field values, and
> > the assigning of them. If the invariant checker could be called by the
> > serialization mechanism then the user code can use final fields, without
> > needing to muck around with reflection, and also have the added benefit
> > of, in many cases, not potential leaving the object in an inconsistent
> > state.
> >
> > There is no support in this proposal for subclass to superclass
> > invariant checking.
> >
> > I also believe that we can resolve the Finalization issue separately.
> >
> > -Chris.
>