Explicit Serialization API and Security
Peter Firmstone
peter.firmstone at zeus.net.au
Tue Feb 3 15:13:34 UTC 2015
Thanks Chris,
see below...
On 4/02/2015 12:14 AM, Chris Hegarty wrote:
> 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 ),
We can with caller sensitive methods, child classes don't have much
choice other than to call a constructor. Child classes don't have
access to super class fields or state, unless via public api by creating
an object instance. Internal state is not exposed, the superclass can
copy mutable state to ensure the child class cannot gain a reference
using a modified stream.
Implementation of caller sensitivity:
/**
* Dummy security manager providing access to getClassContext method.
*/
private static class ClassContextAccess extends SecurityManager {
/**
* Returns caller's caller class.
*/
Class caller() {
return getClassContext()[2];
}
}
private static final ClassContextAccess context
= AccessController.doPrivileged(
new PrivilegedAction<ClassContextAccess>(){
@Override
public ClassContextAccess run() {
return new ClassContextAccess();
}
});
private static class GetArgImpl extends AtomicSerial.GetArg {
final Map<Class,GetField> classFields;
final Map<Class,ReadObject> readers;
final ObjectInput in;
GetArgImpl(Map<Class,GetField> args, Map<Class,ReadObject> readers,
ObjectInput in){
super(false); // Avoids permission check.
classFields = args;
this.readers = readers;
this.in = in;
}
@Override
public ObjectStreamClass getObjectStreamClass() {
return classFields.get(context.caller()).getObjectStreamClass();
}
@Override
public boolean defaulted(String name) throws IOException {
return classFields.get(context.caller()).defaulted(name);
}
@Override
public boolean get(String name, boolean val) throws IOException {
return classFields.get(context.caller()).get(name, val);
}
@Override
public byte get(String name, byte val) throws IOException {
return classFields.get(context.caller()).get(name, val);
}
@Override
public char get(String name, char val) throws IOException {
return classFields.get(context.caller()).get(name, val);
}
@Override
public short get(String name, short val) throws IOException {
return classFields.get(context.caller()).get(name, val);
}
@Override
public int get(String name, int val) throws IOException {
return classFields.get(context.caller()).get(name, val);
}
@Override
public long get(String name, long val) throws IOException {
return classFields.get(context.caller()).get(name, val);
}
@Override
public float get(String name, float val) throws IOException {
return classFields.get(context.caller()).get(name, val);
}
@Override
public double get(String name, double val) throws IOException {
return classFields.get(context.caller()).get(name, val);
}
@Override
public Object get(String name, Object val) throws IOException {
return classFields.get(context.caller()).get(name, val);
}
@Override
public Collection getObjectStreamContext() {
if (in instanceof ObjectStreamContext)
return ((ObjectStreamContext)in).getObjectStreamContext();
return Collections.emptyList();
}
@Override
public Class[] serialClasses() {
return classFields.keySet().toArray(new Class[classFields.size()]);
}
@Override
public ReadObject getReader() { //TODO capture any Exceptions and
rethrow here.
Class c = context.caller();
return readers.get(context.caller());
}
}
>
> 2) there is no way to enforce constraints on mutable state which
> may have constraints enforced through the API
Can you elaborate for me please?
> 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 )
Only if these classes are intended to be public api and subclassed. A
method serialClasses() has been provided to allow implementors to check
the class hierarchy. Otherwise yes, you must provide a public, default
or protected constructor. At present protected constructor's are not
called by the atomic serial implementation.
To instantiate a GetArg instance requires
SerializablePermission("enableSubclassImplementation").
The subclass can't tamper with GetArg state without permission, it can
only call this constructor using a null reference or GetArg instance
passed to it's constructor, otherwise it can call another constructor.
For example it might create a superclass instance, call some getters ,
then pass these references to another superclass constructor instead of
GetArg.
> 4) Subclasses need to make assumptions about abstract
> superclasses, so they can create “fake” instances for checking
>
> See my, not factually correct, example below [*]
Yes, that's true, how well this works depends on how the api is designed
or expressed. I find this is often much simpler in practise, after
implementing it on a number of existing classes.
>> Definition of "Cumbersome":
>> Slow or complicated and therefore inefficient.
> With larger hierarchies, and abstract classes, it becomes more difficult [*].
Less so if you only rely on proper encapsulation and public api.
Duplicate method calls and invariant checks don't hurt performance.
>> 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 [*].
Yes, I typically catch these exceptions and throw a
IllegalObjectException with the original exception as the cause.
I see you've picked that up too :)
>> 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.
Constructors provide wider and more flexible capabilities. Invariants
are much more difficult to enforce with readObject() methods, and it
isn't always possible to enforce them after the object is created.
>> 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.
Yes any progress on this front is good, however constructors do provide
more flexibility and wider scope.
>> 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.
Unfortunately we can't chain readObejct() methods, so can't invoke final
freezes (like we can with constructors) to allow safe publication from
within readObject(). Although I think it's possible in most cases to
use a validator after the object graph has been reconstructed for
publication, but requries additional knowledge. I think developers are
more familiar with constructors, final freeze behaviour and safe
publication.
Another problem with readObject() methods arises if you need to modify
your serial form, reflection is required to set final fields, which
requires privileges, which places the code at risk of privilege
escallation due to programmer errror.
>> 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.
Glad to see there's interest.
>> 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; }
> }
>
A rather complex example (the existing validators and constructors are
production code):
public MethodDesc(GetArg arg) throws IOException{
this(checkSerial(
(String) arg.get("name", null),
(Class []) arg.get("types", null),
(InvocationConstraints) arg.get("constraints", null)
),
(String) arg.get("name", null),
(Class[]) arg.get("types", null),
(InvocationConstraints) arg.get("constraints", null)
);
}
/**
* Creates a descriptor that only matches methods with exactly the
* specified name and parameter types. The constraints can be
* <code>null</code>, which is treated the same as an empty
* instance. The array passed to the constructor is neither modified
* nor retained; subsequent changes to that array have no effect on
* the instance created.
*
* @param name the name of the method
* @param types the formal parameter types of the method, in declared
* order
* @param constraints the constraints, or <code>null</code>
* @throws NullPointerException if <code>name</code> or
* <code>types</code> is <code>null</code> or any element of
* <code>types</code> is <code>null</code>
* @throws IllegalArgumentException if <code>name</code> is not a
* syntactically valid method name
*/
public MethodDesc(String name,
Class[] types,
InvocationConstraints constraints)
{
this(check(name, types),
name,
types,
constraints
);
}
/**
* Creates a descriptor that matches all methods with names that
* equal the specified name or that match the specified pattern,
* regardless of their parameter types. If the specified name starts
* with the character '*', then this descriptor matches all methods
* with names that end with the rest of the specified name. If the
* specified name ends with the character '*', then this descriptor
* matches all methods with names that start with the rest of the
* specified name. Otherwise, this descriptor matches all methods
* with names that equal the specified name. The constraints can be
* <code>null</code>, which is treated the same as an empty instance.
*
* @param name the name of the method, with a prefix or suffix '*'
* permitted for pattern matching
* @param constraints the constraints, or <code>null</code>
* @throws NullPointerException if <code>name</code> is
* <code>null</code>
* @throws IllegalArgumentException if <code>name</code> does not
* match any syntactically valid method name
*/
public MethodDesc(String name, InvocationConstraints constraints) {
this(check(name, null),
name,
null,
constraints
);
}
/**
* Invariant checks for de-serialization.
* @param name
* @param types
* @param constraints
* @return
* @throws InvalidObjectException
*/
private static boolean checkSerial(
String name,
Class[] types,
InvocationConstraints constraints) throws InvalidObjectException
{
if (name == null) {
if (types != null) {
throw new InvalidObjectException(
"cannot have types with null name");
}
} else {
try {
return check(name, types);
} catch (RuntimeException e) {
rethrow(e);
}
}
if (constraints != null && constraints.isEmpty()) {
throw new InvalidObjectException(
"constraints cannot be empty");
}
return true;
}
/**
* Verifies that the name is a syntactically valid method name, or
* (if types is null) if the name is a syntactically valid method name
* with a '*' appended or could be constructed from some syntactically
* valid method name containing more than two characters by replacing
* the first character of that name with '*', and verifies that none
* of the elements of types are null.
*/
private static boolean check(String name, Class[] types) {
boolean star = types == null;
int len = name.length();
if (len == 0) {
throw new IllegalArgumentException(
"method name cannot be empty");
}
char c = name.charAt(0);
if (!Character.isJavaIdentifierStart(c) &&
!(star && c == '*' && len > 1))
{
throw new IllegalArgumentException("invalid method name");
}
if (star && c != '*' && name.charAt(len - 1) == '*') {
len--;
}
while (--len >= 1) {
if (!Character.isJavaIdentifierPart(name.charAt(len))) {
throw new IllegalArgumentException("invalid method name");
}
}
if (types != null) {
for (int i = types.length; --i >= 0; ) {
if (types[i] == null) {
throw new NullPointerException("class cannot be null");
}
}
}
return true;
}
More information about the core-libs-dev
mailing list