Explicit Serialization API and Security

Peter Firmstone peter.firmstone at zeus.net.au
Sat Feb 7 02:38:52 UTC 2015


A simpler example of production code modified to support @AtomicSerial 
that satisfies invariants follows.  Note how we can't type check 
Generics with the static validator method, but the compiler does it for 
us with the constructor :)

I'm wondering if there's scope for secure alternatives of 
ObjectInputStream and ObjectOutputStream, that are backward and forward 
protocol compatible.

The disadvantage of using a constructor is circular links, however if 
these are delayed until after object construction, then the fact that 
the object was constructed successfully in the first case indicates that 
it is a legal object, with satisfied invariants.

classes interested in circular links could annotate a method, that 
accepts fields after construction, such as:

@Circular
private void setField(String name, Object value) throws 
InvalidObjectException

In this case, the class in question, can check if its invariants are 
still satisfed with the given field and only set it if they are.

Regards,

Peter.


/*
  * Licensed to the Apache Software Foundation (ASF) under one
  * or more contributor license agreements.  See the NOTICE file
  * distributed with this work for additional information
  * regarding copyright ownership. The ASF licenses this file
  * to you under the Apache License, Version 2.0 (the
  * "License"); you may not use this file except in compliance
  * with the License. You may obtain a copy of the License at
  *
  *      http://www.apache.org/licenses/LICENSE-2.0
  *
  * Unless required by applicable law or agreed to in writing, software
  * distributed under the License is distributed on an "AS IS" BASIS,
  * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */

package com.artima.lookup.util;

import java.io.IOException;
import java.io.Serializable;
import java.util.Map;
import org.apache.river.api.io.AtomicSerial;
import org.apache.river.api.io.AtomicSerial.GetArg;

/**
  * An implementation of the <code>java.util.Map.Entry</code> interface 
that has
  * a serialized form consistent in all virtual machines. 
<code>ConsistentMapEntry</code>
  * instances are unmodifiable. The <code>setValue</code> mutator method
  * throws <code>UnsupportedOperationException</code>.  This class 
permits <code>null</code>
  * for values and keys.
  *
  * <p>
  * Although instances of this class are unmodifiable, they are not 
necessarily
  * immutable. If a client retrieves a mutable object (either a key or 
value) contained in a
  * <code>ConsistentMapEntry</code> and mutates that object, the client 
in effect
  * mutates the state of the <code>ConsistentMapEntry</code>. In this 
case, the
  * serialized form of the <code>ConsistentMapEntry</code> will most 
likely also
  * have been mutated. A <code>ConsistentMapEntry</code> that contains 
only immutable
  * objects will maintain a consistent serialized form indefinitely. But a
  * <code>ConsistentMapEntry</code> that contains mutable objects will 
maintain a
  * consistent serialized form only so long as the mutable objects are not
  * mutated.
  *
  * @author Bill Venners
  */
@AtomicSerial
final class ConsistentMapEntry<K,V> implements Map.Entry<K,V>, 
Serializable {

     private static final long serialVersionUID = -8633627011729114409L;

     /**
      * @serial An <code>Object</code> key, or <code>null</code>
      */
     private final K key;

     /**
      * @serial An <code>Object</code> value, or <code>null</code>
      */
     private final V value;

     /**
      * {@link AtomicSerial} constructor.
      * @param arg
      * @throws IOException
      * @throws ClassCastException if types don't match <K,V>
      */
     public ConsistentMapEntry(GetArg arg) throws IOException {
     this((K) arg.get("key", null), (V) arg.get("value", null));
     }

     /**
      * Constructs a new <code>ConsistentMapEntry</code> with passed
      * <code>key</code> and <code>value</code>. <code>null</code> is
      * allowed in either (or both) parameters.
      *
      * @param key the key (<code>null</code> key is OK)
      * @param value the value (<code>null</code> value is OK) 
associated with the key
      */
     public ConsistentMapEntry(K key, V value) {
         this.key = key;
         this.value = value;
     }

     /**
      * Returns the key.
      *
      * @return the key.
      */
     @Override
     public K getKey() {
         return key;
     }

     /**
      * Returns the value.
      *
      * @return the value.
      */
     @Override
     public V getValue() {
         return value;
     }

     /**
      * Replaces the value corresponding to this entry with the 
specified value. Because
      * all instances of this class are unmodifiable, this method always 
throws
      * <code>UnsupportedOperationException</code>.
      *
      * @throws UnsupportedOperationException always
      */
     @Override
     public V setValue(V value) {
         throw new UnsupportedOperationException();
     }

     /**
      * Compares the specified object (the <CODE>Object</CODE> passed
      * in <CODE>o</CODE>) with this <CODE>ConsistentMapEntry</CODE>
      * object for equality. Returns true if the specified object
      * is not <code>null</code>, if the specified object's class is
      * <CODE>ConsistentMapEntry</CODE>, if the keys of this object and
      * the specified object are either both <code>null</code> or 
semantically
      * equal, and the values of this object and the specified object 
are either
      * both <code>null</code> or semantically equal.
      *
      * @param o the object to compare against
      * @return <code>true</code> if the objects are the semantically equal,
      * <code>false</code> otherwise.
      */
     @Override
     public boolean equals(Object o) {

         if (o == null) {
             return false;
         }

         if (o == this) {
             return true;
         }

         // TODO: ASK JOSH SHOULD EQUALS CHECK FOR INSTANCEOF OR EXACT 
CLASS?
         if (o.getClass() != ConsistentMapEntry.class) {
             return false;
         }

         ConsistentMapEntry unmod = (ConsistentMapEntry) o;

         boolean keysEqual = equalsOrNull(key, unmod.key);
         boolean valsEqual = equalsOrNull(value, unmod.value);

         return keysEqual && valsEqual;
     }

     private static boolean equalsOrNull(Object o1, Object o2) {
         return (o1 == null ? o2 == null : o1.equals(o2));
     }

     /**
      * Returns the hash code value for this 
<CODE>ConsistentMapEntry</CODE> object.
      *
      * @return the hashcode for this object
      */
     @Override
     public int hashCode() {

         int keyHash = (key == null ? 0 : key.hashCode());
         int valueHash = (value == null ? 0 : value.hashCode());

         return keyHash ^ valueHash;
     }
}



On 4/02/2015 1:13 AM, Peter Firmstone wrote:
> 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