Re: Explicit Serialization API and Security
Some performance figures, atomic de-serialization using loopback local network sockets, codebase downloads, dynamic class loading, with security manager and policies in force and Jini extensible remote method invocation, there's communication going on between 4 different jvm instances, the one consuming the most cpu% shown: Hot Spots - Method,Self time [%],Self time,Self time (CPU),Samples java.net.DualStackPlainSocketImpl.accept0[native](),32.93283,835657.833 ms,835657.833 ms,9 java.net.TwoStacksPlainDatagramSocketImpl.peekData[native](),23.122272,586718.719 ms,586718.719 ms,7 java.net.TwoStacksPlainDatagramSocketImpl.socketNativeSetOption[native](),19.53974,495813.33 ms,495813.33 ms,31 java.net.SocketInputStream.socketRead0[native](),17.010353,431631.101 ms,431631.101 ms,49 sun.management.ThreadImpl.dumpThreads0[native](),5.5209646,140092.335 ms,140092.335 ms,18 sun.misc.Unsafe.unpark[native](),0.82664025,20975.676 ms,20975.676 ms,25 java.io.ObjectOutputStream.writeObject(),0.2538986,6442.578 ms,6442.578 ms,11 sun.reflect.Reflection.getCallerClass[native](),0.22958349,5825.592 ms,5825.592 ms,1 java.io.ObjectOutputStream$BlockDataOutputStream.getUTFLength(),0.19049801,4833.813 ms,4833.813 ms,1 java.security.AccessController.doPrivileged[native](),0.10722659,2720.833 ms,2720.833 ms,111 java.net.DualStackPlainSocketImpl.connect0[native](),0.049821686,1264.206 ms,1264.206 ms,2 java.net.SocketOutputStream.socketWrite0[native](),0.037383154,948.583 ms,948.583 ms,3 java.lang.Class.isArray[native](),0.027840897,706.452 ms,706.452 ms,1 org.apache.river.api.io.AtomicObjectInputStream.readObjectOverride(),0.02323,20588.805 ms,589.565 ms,39 java.lang.Thread.interrupt0[native](),0.019693227,499.708 ms,499.708 ms,1 java.net.URLClassLoader.isSealed(),0.019693227,499.708 ms,499.708 ms,1 java.util.AbstractCollection.toArray(),0.018976605,481.524 ms,481.524 ms,1 sun.management.ThreadImpl.getThreadInfo1[native](),0.01107343,280.984 ms,280.984 ms,3 org.apache.river.api.security.URIGrant.implies(),0.010591452,268.754 ms,268.754 ms,2 java.lang.Thread.join(),0.009474391,113558.68 ms,240.409 ms,20 java.lang.Class.forName0[native](),0.007706837,195.558 ms,195.558 ms,2 sun.misc.Unsafe.park[native](),0.006058181,5991413.846 ms,153.724 ms,166 sun.rmi.transport.ObjectTable.getTarget(),0.004627933,117.432 ms,117.432 ms,1 javax.management.Attribute.init(),0.0039669964,100.661 ms,100.661 ms,1 java.util.concurrent.FutureTask.get(),0.0030666084,77.814 ms,77.814 ms,35 org.apache.river.api.net.Uri.constructor1(),0.002617104,66.408 ms,66.408 ms,1 java.util.concurrent.locks.AbstractQueuedSynchronizer$ConditionObject.addConditionWaiter(),0.0023084884,58.577 ms,58.577 ms,1 java.lang.Thread.isInterrupted[native](),0.0021556192,54.698 ms,54.698 ms,3 java.util.concurrent.ConcurrentHashMap.get(),0.0020567013,52.188 ms,52.188 ms,1 java.util.concurrent.FutureTask.awaitDone(),0.0011767274,29.859 ms,29.859 ms,34 java.lang.System.identityHashCode[native](),0.0010494348,26.629 ms,26.629 ms,1 java.util.concurrent.ThreadPoolExecutor.runWorker(),8.1751E-4,20.744 ms,20.744 ms,63 java.io.BufferedOutputStream.flushBuffer(),5.996545E-4,15.216 ms,15.216 ms,4 java.util.concurrent.locks.LockSupport.park(),0.0,0.0 ms,0.0 ms,143 java.util.concurrent.locks.AbstractQueuedSynchronizer$ConditionObject.await(),0.0,0.0 ms,0.0 ms,117 java.util.concurrent.ThreadPoolExecutor.getTask(),0.0,0.0 ms,0.0 ms,108 java.lang.Thread.run(),0.0,0.0 ms,0.0 ms,107 java.net.SocketInputStream.read(),0.0,0.0 ms,0.0 ms,101 java.util.concurrent.LinkedBlockingQueue.take(),0.0,0.0 ms,0.0 ms,99 sun.rmi.transport.Transport$1.run(),0.0,0.0 ms,0.0 ms,76 sun.reflect.DelegatingMethodAccessorImpl.invoke(),0.0,0.0 ms,0.0 ms,70 java.lang.reflect.Method.invoke(),0.0,0.0 ms,0.0 ms,70 java.util.concurrent.ThreadPoolExecutor$Worker.run(),0.0,0.0 ms,0.0 ms,63 java.io.ObjectInputStream.readObject(),0.0,0.0 ms,0.0 ms,59 java.util.concurrent.FutureTask.run(),0.0,0.0 ms,0.0 ms,54 java.io.BufferedInputStream.read(),0.0,0.0 ms,0.0 ms,45 java.io.BufferedInputStream.fill(),0.0,0.0 ms,0.0 ms,45 java.io.FilterInputStream.read(),0.0,0.0 ms,0.0 ms,44 com.sun.jmx.mbeanserver.MXBeanIntrospector.invokeM2(),0.0,0.0 ms,0.0 ms,42 com.sun.jmx.mbeanserver.ConvertingMethod.invokeWithOpenReturn(),0.0,0.0 ms,0.0 ms,42 net.jini.discovery.AbstractLookupDiscovery$14.run(),0.0,0.0 ms,0.0 ms,40 sun.rmi.transport.Transport.serviceCall(),0.0,0.0 ms,0.0 ms,40 net.jini.discovery.AbstractLookupDiscovery.doUnicastDiscovery(),0.0,0.0 ms,0.0 ms,40 sun.rmi.server.UnicastServerRef.dispatch(),0.0,0.0 ms,0.0 ms,38 org.apache.river.api.io.AtomicObjectInputStream.readObject(),0.0,0.0 ms,0.0 ms,35 net.jini.loader.LoadClass.forName(),0.0,0.0 ms,0.0 ms,35 org.apache.river.api.io.AtomicObjectInputStream.readNewObject(),0.0,0.0 ms,0.0 ms,35 org.apache.river.api.io.AtomicObjectInputStream.readNonPrimitiveContent(),0.0,0.0 ms,0.0 ms,35
Re: Explicit Serialization API and Security
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 codejava.util.Map.Entry/code interface
that has
* a serialized form consistent in all virtual machines.
codeConsistentMapEntry/code
* instances are unmodifiable. The codesetValue/code mutator method
* throws codeUnsupportedOperationException/code. This class
permits codenull/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
* codeConsistentMapEntry/code and mutates that object, the client
in effect
* mutates the state of the codeConsistentMapEntry/code. In this
case, the
* serialized form of the codeConsistentMapEntry/code will most
likely also
* have been mutated. A codeConsistentMapEntry/code that contains
only immutable
* objects will maintain a consistent serialized form indefinitely. But a
* codeConsistentMapEntry/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 ConsistentMapEntryK,V implements Map.EntryK,V,
Serializable {
private static final long serialVersionUID = -8633627011729114409L;
/**
* @serial An codeObject/code key, or codenull/code
*/
private final K key;
/**
* @serial An codeObject/code value, or codenull/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 codeConsistentMapEntry/code with passed
* codekey/code and codevalue/code. codenull/code is
* allowed in either (or both) parameters.
*
* @param key the key (codenull/code key is OK)
* @param value the value (codenull/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
* codeUnsupportedOperationException/code.
*
* @throws UnsupportedOperationException always
*/
@Override
public V setValue(V value) {
throw new UnsupportedOperationException();
}
/**
* Compares the specified object (the CODEObject/CODE passed
* in
Re: Explicit Serialization API and Security
Hi Peter,
On 2 Feb 2015, at 11:16, Peter Firmstone peter.firmst...@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
Re: Explicit Serialization API and Security
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.
Definition of Cumbersome:
Slow or complicated and therefore inefficient.
During implementation, I've found that static invarient check methods
are often shared with other constructors.
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.
My experience is using constructors is often easier to understand than
readObject methods.
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.
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.
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?
To those who would value it, I think the answer will be yes, to those
that don't, perhaps not?
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 net.jini.jeri;
import com.sun.jini.jeri.internal.runtime.DgcClient;
import com.sun.jini.jeri.internal.runtime.Util;
import com.sun.jini.logging.Levels;
import java.io.EOFException;
import java.io.IOException;
import java.io.InvalidObjectException;
import java.io.ObjectInput;
import java.io.ObjectInputStream;
import java.io.ObjectInputValidation;
import java.io.ObjectOutputStream;
import java.io.Serializable;
import java.rmi.NoSuchObjectException;
import java.rmi.RemoteException;
import java.rmi.UnmarshalException;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.Iterator;
import java.util.Map;
import java.util.NoSuchElementException;
import java.util.WeakHashMap;
import java.util.logging.Level;
import java.util.logging.Logger;
import net.jini.core.constraint.InvocationConstraints;
import net.jini.id.Uuid;
import net.jini.id.UuidFactory;
import net.jini.io.ObjectStreamContext;
import net.jini.io.context.AcknowledgmentSource;
import net.jini.security.proxytrust.TrustEquivalence;
import org.apache.river.api.io.AtomicSerial;
import org.apache.river.api.io.AtomicSerial.GetArg;
import org.apache.river.api.io.AtomicSerial.ReadInput;
import org.apache.river.api.io.AtomicSerial.ReadObject;
/**
* References a remote object with an {@link Endpoint Endpoint} for
* sending requests to the object and a {@link Uuid Uuid} to identify
* the object at that codeEndpoint/code.
*
* pIn addition to the codeEndpoint/code and the
* codeUuid/code, codeBasicObjectEndpoint/code instances also
* contain a flag indicating whether or not the instance participates
* in distributed garbage collection (DGC).
*
* pThe {@link #newCall newCall} method can be used to send a
* request to the remote object that this object references.
*
* h4Distributed Garbage Collection/h4
*
Re: Explicit Serialization API and Security
Logging output from failed validation: NonActGrp-out: Jan 29, 2015 7:36:16 PM net.jini.jeri.BasicInvocationHandler invoke NonActGrp-out: Jan 29, 2015 7:36:16 PM net.jini.jeri.BasicInvocationHandler invoke NonActGrp-out: FAILED: outbound call net.jini.core.event.RemoteEventListener.notify remotely throws NonActGrp-out: FAILED: outbound call net.jini.core.event.RemoteEventListener.notify remotely throws NonActGrp-out: java.rmi.ServerException: RemoteException in server thread; nested exception is: NonActGrp-out: java.rmi.ServerException: RemoteException in server thread; nested exception is: NonActGrp-out: java.rmi.UnmarshalException: unmarshalling method/arguments; nested exception is: NonActGrp-out: java.rmi.UnmarshalException: unmarshalling method/arguments; nested exception is: NonActGrp-out: java.io.InvalidObjectException: name must be equal to Object when superclass is null NonActGrp-out: java.io.InvalidObjectException: name must be equal to Object when superclass is null NonActGrp-out: at net.jini.jeri.BasicInvocationDispatcher.dispatch(BasicInvocationDispatcher.java:646) NonActGrp-out: at com.sun.jini.jeri.internal.runtime.Target$2.run(Target.java:493) NonActGrp-out: at net.jini.export.ServerContext.doWithServerContext(ServerContext.java:108) NonActGrp-out: at com.sun.jini.jeri.internal.runtime.Target.dispatch(Target.java:490) NonActGrp-out: at com.sun.jini.jeri.internal.runtime.Target.access$000(Target.java:57) NonActGrp-out: at com.sun.jini.jeri.internal.runtime.Target$1.run(Target.java:466) NonActGrp-out: at java.security.AccessController.doPrivileged(Native Method) NonActGrp-out: at java.security.AccessController.doPrivileged(Native Method) NonActGrp-out: at com.sun.jini.jeri.internal.runtime.Target.dispatch(Target.java:463) NonActGrp-out: at com.sun.jini.jeri.internal.runtime.Target.dispatch(Target.java:428) NonActGrp-out: at com.sun.jini.jeri.internal.runtime.DgcRequestDispatcher.dispatch(DgcRequestDispatcher.java:210) NonActGrp-out: at net.jini.jeri.connection.ServerConnectionManager$Dispatcher.dispatch(ServerConnectionManager.java:147) NonActGrp-out: at com.sun.jini.jeri.internal.mux.MuxServer$1$1.run(MuxServer.java:247) NonActGrp-out: at java.security.AccessController.doPrivileged(Native Method) NonActGrp-out: at java.security.AccessController.doPrivileged(Native Method) NonActGrp-out: at com.sun.jini.jeri.internal.mux.MuxServer$1.run(MuxServer.java:243) NonActGrp-out: at com.sun.jini.thread.ThreadPool$Task.run(ThreadPool.java:215) NonActGrp-out: at com.sun.jini.thread.ThreadPool$Worker.run(ThreadPool.java:277) NonActGrp-out: at java.lang.Thread.run(Thread.java:744) NonActGrp-out: at com.sun.jini.jeri.internal.runtime.Util.__EXCEPTION_RECEIVED_FROM_SERVER__(Util.java:110) NonActGrp-out: at com.sun.jini.jeri.internal.runtime.Util.exceptionReceivedFromServer(Util.java:103) NonActGrp-out: at net.jini.jeri.BasicInvocationHandler.unmarshalThrow(BasicInvocationHandler.java:1320) NonActGrp-out: at net.jini.jeri.BasicInvocationHandler.invokeRemoteMethodOnce(BasicInvocationHandler.java:834) NonActGrp-out: at net.jini.jeri.BasicInvocationHandler.invokeRemoteMethod(BasicInvocationHandler.java:661) NonActGrp-out: at net.jini.jeri.BasicInvocationHandler.invoke(BasicInvocationHandler.java:530) NonActGrp-out: at com.sun.proxy.$Proxy5.notify(Unknown Source) NonActGrp-out: at com.sun.jini.reggie.RegistrarImpl$EventTask.call(RegistrarImpl.java:2076) NonActGrp-out: at com.sun.jini.reggie.RegistrarImpl$EventTask.call(RegistrarImpl.java:2032) NonActGrp-out: at org.apache.river.impl.thread.SynchronousExecutors$Task.call(SynchronousExecutors.java:421) NonActGrp-out: at java.util.concurrent.FutureTask.run(FutureTask.java:266) NonActGrp-out: at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.access$201(ScheduledThreadPoolExecutor.java:180) NonActGrp-out: at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:293) NonActGrp-out: at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142) NonActGrp-out: at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617) NonActGrp-out: at java.lang.Thread.run(Thread.java:744) NonActGrp-out: Caused by: java.rmi.UnmarshalException: unmarshalling method/arguments; nested exception is: NonActGrp-out: Caused by: java.rmi.UnmarshalException: unmarshalling method/arguments; nested exception is: NonActGrp-out: java.io.InvalidObjectException: name must be equal to Object when superclass is null NonActGrp-out: java.io.InvalidObjectException: name must be equal to Object when superclass is null NonActGrp-out: at net.jini.jeri.BasicInvocationDispatcher.dispatch(BasicInvocationDispatcher.java:621) NonActGrp-out: at
Re: Explicit Serialization API and Security
Although not directly relevant to this discussion, here are some
functional examples of deserialization constructors, I now have a fully
functional validating ObjectInputStream.
Unfortunately in our project we have intra object dependencies as
demonstrated by this example; a static validator, without a constructor
falls short.
I also have an interface that provides direct access to the
ObjectInputStream before the deserializing object has been constructed.
The following two classes are a good example as they show how to check
invariants and provide full backward compatibility with existing Serial
Form. In this case validation is provided for unauthenticated network
connections, so the classes also implement standard serialization for
trusted connections, where we can relax validation.
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.sun.jini.reggie;
import com.sun.jini.proxy.MarshalledWrapper;
import java.io.IOException;
import java.io.InvalidObjectException;
import java.io.ObjectInput;
import java.io.ObjectInputStream;
import java.io.ObjectOutputStream;
import java.io.ObjectStreamException;
import java.io.Serializable;
import java.rmi.MarshalledObject;
import java.rmi.RemoteException;
import java.rmi.UnmarshalException;
import java.util.logging.Level;
import java.util.logging.Logger;
import net.jini.admin.Administrable;
import net.jini.core.constraint.RemoteMethodControl;
import net.jini.core.discovery.LookupLocator;
import net.jini.core.event.EventRegistration;
import net.jini.core.event.RemoteEventListener;
import net.jini.core.lookup.ServiceID;
import net.jini.core.lookup.ServiceItem;
import net.jini.core.lookup.ServiceMatches;
import net.jini.core.lookup.ServiceRegistrar;
import net.jini.core.lookup.ServiceRegistration;
import net.jini.core.lookup.ServiceTemplate;
import net.jini.id.ReferentUuid;
import net.jini.id.ReferentUuids;
import net.jini.id.Uuid;
import net.jini.id.UuidFactory;
import org.apache.river.api.io.AtomicSerial;
import org.apache.river.api.io.AtomicSerial.GetArg;
import org.apache.river.api.io.AtomicSerial.ReadInput;
import org.apache.river.api.io.AtomicSerial.ReadObject;
/**
* A RegistrarProxy is a proxy for a registrar. Clients only see instances
* via the ServiceRegistrar, Administrable and ReferentUuid interfaces.
*
* @author Sun Microsystems, Inc.
*
*/
@AtomicSerial
class RegistrarProxy
implements ServiceRegistrar, Administrable, ReferentUuid, Serializable
{
private static final long serialVersionUID = 2L;
private static final Logger logger =
Logger.getLogger(com.sun.jini.reggie);
/**
* The registrar.
*
* @serial
*/
final Registrar server;
/**
* The registrar's service ID.
*/
transient ServiceID registrarID;
/**
* Returns RegistrarProxy or ConstrainableRegistrarProxy instance,
* depending on whether given server implements RemoteMethodControl.
*/
static RegistrarProxy getInstance(Registrar server,
ServiceID registrarID)
{
return (server instanceof RemoteMethodControl) ?
new ConstrainableRegistrarProxy(server, registrarID, null) :
new RegistrarProxy(server, registrarID);
}
@ReadInput
private static RO getRO(){
return new RO();
}
private static boolean check(GetArg arg) throws IOException{
Registrar server = (Registrar) arg.get(server, null);
if (server == null) throw new InvalidObjectException(null server);
RO r = (RO) arg.getReader();
if (r.registrarID == null) throw new InvalidObjectException(null
ServiceID);
return true;
}
RegistrarProxy(GetArg arg) throws IOException{
this(arg, check(arg));
}
RegistrarProxy(GetArg arg, boolean check) throws IOException{
server = (Registrar) arg.get(server, null);
RO r = (RO) arg.getReader();
registrarID = r.registrarID;
}
/** Constructor for use by getInstance(),
ConstrainableRegistrarProxy. */
RegistrarProxy(Registrar server, ServiceID registrarID) {
this.server = server;
this.registrarID = registrarID;
}
// Inherit javadoc
@Override
public Object getAdmin() throws RemoteException {
Re: Explicit Serialization API and Security
Just a quick note, to avoid stream corruption, object that aren't
instantiated are replaced by an enum marker, Reference.DISCARDED, and
null is returned in their place, fields are read and any trailling
writeObject data is discarded. Object that are not allowed to be
deserialized are still read in, just not created.
This way, a stream may be able to avoid a MIM attack, by discarding
unvalidated data inserted into the stream. Altough it would be better
to use a secure connection, there is a brief period where a proxy is
used to establish trust with a third party service provider, after being
obtained from a lookup service, this was premised on java's secure
sandbox and serialization, when it was believed to be secure.
Regards,
Peter.
On 29/01/2015 7:08 PM, Peter Firmstone wrote:
Although not directly relevant to this discussion, here are some
functional examples of deserialization constructors, I now have a
fully functional validating ObjectInputStream.
Unfortunately in our project we have intra object dependencies as
demonstrated by this example; a static validator, without a
constructor falls short.
I also have an interface that provides direct access to the
ObjectInputStream before the deserializing object has been constructed.
The following two classes are a good example as they show how to check
invariants and provide full backward compatibility with existing
Serial Form. In this case validation is provided for unauthenticated
network connections, so the classes also implement standard
serialization for trusted connections, where we can relax validation.
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.sun.jini.reggie;
import com.sun.jini.proxy.MarshalledWrapper;
import java.io.IOException;
import java.io.InvalidObjectException;
import java.io.ObjectInput;
import java.io.ObjectInputStream;
import java.io.ObjectOutputStream;
import java.io.ObjectStreamException;
import java.io.Serializable;
import java.rmi.MarshalledObject;
import java.rmi.RemoteException;
import java.rmi.UnmarshalException;
import java.util.logging.Level;
import java.util.logging.Logger;
import net.jini.admin.Administrable;
import net.jini.core.constraint.RemoteMethodControl;
import net.jini.core.discovery.LookupLocator;
import net.jini.core.event.EventRegistration;
import net.jini.core.event.RemoteEventListener;
import net.jini.core.lookup.ServiceID;
import net.jini.core.lookup.ServiceItem;
import net.jini.core.lookup.ServiceMatches;
import net.jini.core.lookup.ServiceRegistrar;
import net.jini.core.lookup.ServiceRegistration;
import net.jini.core.lookup.ServiceTemplate;
import net.jini.id.ReferentUuid;
import net.jini.id.ReferentUuids;
import net.jini.id.Uuid;
import net.jini.id.UuidFactory;
import org.apache.river.api.io.AtomicSerial;
import org.apache.river.api.io.AtomicSerial.GetArg;
import org.apache.river.api.io.AtomicSerial.ReadInput;
import org.apache.river.api.io.AtomicSerial.ReadObject;
/**
* A RegistrarProxy is a proxy for a registrar. Clients only see
instances
* via the ServiceRegistrar, Administrable and ReferentUuid interfaces.
*
* @author Sun Microsystems, Inc.
*
*/
@AtomicSerial
class RegistrarProxy
implements ServiceRegistrar, Administrable, ReferentUuid,
Serializable
{
private static final long serialVersionUID = 2L;
private static final Logger logger =
Logger.getLogger(com.sun.jini.reggie);
/**
* The registrar.
*
* @serial
*/
final Registrar server;
/**
* The registrar's service ID.
*/
transient ServiceID registrarID;
/**
* Returns RegistrarProxy or ConstrainableRegistrarProxy instance,
* depending on whether given server implements RemoteMethodControl.
*/
static RegistrarProxy getInstance(Registrar server,
ServiceID registrarID)
{
return (server instanceof RemoteMethodControl) ?
new ConstrainableRegistrarProxy(server, registrarID, null) :
new RegistrarProxy(server, registrarID);
}
@ReadInput
private static RO getRO(){
return new RO();
}
private static boolean check(GetArg arg) throws IOException{
Registrar server = (Registrar)
Re: Explicit Serialization API and Security
For the sharper eyed, you'll have noticed the readObjectNoData() method
I forgot to check:
private static boolean check(GetArg arg) throws IOException{
// If this class is not in the heirarchy of classes the stream
// may have been tampered with, see the Serialization Specification
// section 3.5, this is equivalent to readObjectNoData().
Class [] serialClasses = arg.serialClasses();
for (Class serialClass : serialClasses){
if (serialClass.equals(AvailabilityEvent.class)){
RemoteEvent sup = new RemoteEvent(arg);
if (sup.getSource() == null)
throw new InvalidObjectException(null source reference);
if (!(sup.getSource() instanceof JavaSpace))
throw new InvalidObjectException(source is not a JavaSpace);
return arg.get(visibilityTransition, false);
}
}
throw new InvalidObjectException(
AvailabilityEvent should always have data);
}
On 29/01/2015 8:10 PM, Peter Firmstone wrote:
Another example of intra dependencies, turns out we have a lot of
these intra class dependencies in our project.
Cheers,
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 net.jini.space;
import java.io.IOException;
import java.io.InvalidObjectException;
import java.io.ObjectInputStream;
import java.rmi.MarshalledObject;
import net.jini.core.entry.Entry;
import net.jini.core.entry.UnusableEntryException;
import net.jini.core.event.RemoteEvent;
import org.apache.river.api.io.AtomicSerial;
import org.apache.river.api.io.AtomicSerial.GetArg;
/**
* A codeRemoteEvent/code marking the transition of an
* codeEntry/code from {@link
* JavaSpace05#registerForAvailabilityEvent emunavailable/em to
* emavailable/em}.p
*
* Note, by the time the event is delivered, the
* codeEntry/code whose transition triggered this event may
* have transitioned to a state where it is no longer visible
* and/or available.p
*
* @see JavaSpace05
* @since 2.1
*/
@AtomicSerial
public abstract class AvailabilityEvent extends RemoteEvent {
private static final long serialVersionUID = 1L;
/**
* codetrue/code if this event signals a
* transition from invisible to visible as well
* as unavailable to available.
*
* @serial
*/
private final boolean visibilityTransition;
private static boolean check(GetArg arg) throws IOException{
RemoteEvent sup = new RemoteEvent(arg);
if (sup.getSource() == null)
throw new InvalidObjectException(null source reference);
if (!(sup.getSource() instanceof JavaSpace))
throw new InvalidObjectException(source is not a JavaSpace);
return arg.get(visibilityTransition, false);
}
private AvailabilityEvent(GetArg arg, boolean
visibilityTransition) throws IOException{
super(arg);
this.visibilityTransition = visibilityTransition;
}
protected AvailabilityEvent(GetArg arg) throws IOException {
this(arg, check(arg));
}
/**
* Create a new codeAvailabilityEvent/code instance.
* @param sourcethe event source
* @param eventID the event identifier
* @param seqNumthe event sequence number
* @param handback the handback object
* @param visibilityTransition codetrue/code if this event
* must also signal a transition from
* invisible to visible
* @throws NullPointerException if codesource/code is
* codenull/code
*/
protected AvailabilityEvent(JavaSpacesource,
long eventID,
long seqNum,
MarshalledObject handback,
boolean visibilityTransition)
{
super(source, eventID, seqNum, handback);
this.visibilityTransition = visibilityTransition;
}
/**
* @throws InvalidObjectException if {@link #source} is
codenull/code
* or is not a {@link JavaSpace}
*/
private void readObject(ObjectInputStream in)
throws IOException, ClassNotFoundException
{
in.defaultReadObject();
if (getSource() == null)
throw new InvalidObjectException(null source reference);
if (!(getSource() instanceof JavaSpace))
Re: Explicit Serialization API and Security
Two examples:
1. of a child class with super class invariants and
2. Checking List and Map contents for type correctness.
/*
* 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 net.jini.lease;
import java.io.IOException;
import java.io.InvalidObjectException;
import java.rmi.MarshalledObject;
import net.jini.core.event.RemoteEvent;
import net.jini.core.lease.Lease;
import org.apache.river.api.io.AtomicSerial;
import org.apache.river.api.io.AtomicSerial.GetArg;
/**
* Event generated by a lease renewal set when its lease is about to
* expire.
*
* @author Sun Microsystems, Inc.
* @see LeaseRenewalSet
*/
@AtomicSerial
public class ExpirationWarningEvent extends RemoteEvent {
private static final long serialVersionUID = -2020487536756927350L;
private static GetArg check(GetArg arg) throws IOException {
RemoteEvent sup = new RemoteEvent(arg);
if (sup.getID() != LeaseRenewalSet.EXPIRATION_WARNING_EVENT_ID)
throw new InvalidObjectException(Illegal object state);
return arg;
}
public ExpirationWarningEvent(GetArg arg) throws IOException {
super(check(arg));
}
/**
* Simple constructor. Note event id is fixed to
* codeLeaseRenewalSet.EXPIRATION_WARNING_EVENT_ID/code.
*
* @param source the codeLeaseRenewalSet/code that generated the
* event
* @param seqNum the sequence number of this event
* @param handback the codeMarshalledObject/code passed in as
* part of the event registration
*/
public ExpirationWarningEvent(LeaseRenewalSet source,
long seqNum,
MarshalledObject handback)
{
super(source, LeaseRenewalSet.EXPIRATION_WARNING_EVENT_ID, seqNum,
handback);
}
/**
* Convenience method to retrieve the codeLease/code associated
* with the source of this event. This is the codeLease/code
* which is about to expire.
* p
* The codeLease/code object returned will be equivalent (in the
* sense of codeequals/code) to other codeLease/code objects
* associated with the set, but may not be the same object. One
* notable consequence of having two different objects is that the
* codegetExpiration/code method of the codeLease/code
* object returned by this method may return a different time than
* the codegetExpiration/code methods of other
* codeLease/code objects granted on the same set.
* p
* The expiration time associated with the codeLease/code object
* returned by this method will reflect the expiration the lease had
* when the event occurred. Renewal calls may have changed the
* expiration time of the underlying lease between the time when the
* event was generated and when it was delivered.
*
* @return the lease associated with the source of this event
*/
public Lease getRenewalSetLease() {
return ((LeaseRenewalSet) getSource()).getRenewalSetLease();
}
}
/*
* 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 net.jini.discovery;
import com.sun.jini.proxy.MarshalledWrapper;
import java.io.IOException;
import java.io.InvalidObjectException;
import java.io.ObjectInputStream;
import java.rmi.MarshalledObject;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.TreeMap;
import net.jini.core.event.RemoteEvent;
Re: Explicit Serialization API and Security
Another example of intra dependencies, turns out we have a lot of these
intra class dependencies in our project.
Cheers,
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 net.jini.space;
import java.io.IOException;
import java.io.InvalidObjectException;
import java.io.ObjectInputStream;
import java.rmi.MarshalledObject;
import net.jini.core.entry.Entry;
import net.jini.core.entry.UnusableEntryException;
import net.jini.core.event.RemoteEvent;
import org.apache.river.api.io.AtomicSerial;
import org.apache.river.api.io.AtomicSerial.GetArg;
/**
* A codeRemoteEvent/code marking the transition of an
* codeEntry/code from {@link
* JavaSpace05#registerForAvailabilityEvent emunavailable/em to
* emavailable/em}.p
*
* Note, by the time the event is delivered, the
* codeEntry/code whose transition triggered this event may
* have transitioned to a state where it is no longer visible
* and/or available.p
*
* @see JavaSpace05
* @since 2.1
*/
@AtomicSerial
public abstract class AvailabilityEvent extends RemoteEvent {
private static final long serialVersionUID = 1L;
/**
* codetrue/code if this event signals a
* transition from invisible to visible as well
* as unavailable to available.
*
* @serial
*/
private final boolean visibilityTransition;
private static boolean check(GetArg arg) throws IOException{
RemoteEvent sup = new RemoteEvent(arg);
if (sup.getSource() == null)
throw new InvalidObjectException(null source reference);
if (!(sup.getSource() instanceof JavaSpace))
throw new InvalidObjectException(source is not a JavaSpace);
return arg.get(visibilityTransition, false);
}
private AvailabilityEvent(GetArg arg, boolean visibilityTransition)
throws IOException{
super(arg);
this.visibilityTransition = visibilityTransition;
}
protected AvailabilityEvent(GetArg arg) throws IOException {
this(arg, check(arg));
}
/**
* Create a new codeAvailabilityEvent/code instance.
* @param sourcethe event source
* @param eventID the event identifier
* @param seqNumthe event sequence number
* @param handback the handback object
* @param visibilityTransition codetrue/code if this event
* must also signal a transition from
* invisible to visible
* @throws NullPointerException if codesource/code is
* codenull/code
*/
protected AvailabilityEvent(JavaSpacesource,
long eventID,
long seqNum,
MarshalledObject handback,
boolean visibilityTransition)
{
super(source, eventID, seqNum, handback);
this.visibilityTransition = visibilityTransition;
}
/**
* @throws InvalidObjectException if {@link #source} is
codenull/code
* or is not a {@link JavaSpace}
*/
private void readObject(ObjectInputStream in)
throws IOException, ClassNotFoundException
{
in.defaultReadObject();
if (getSource() == null)
throw new InvalidObjectException(null source reference);
if (!(getSource() instanceof JavaSpace))
throw new InvalidObjectException(source is not a JavaSpace);
}
/**
* @throws InvalidObjectException if called
*/
private void readObjectNoData() throws InvalidObjectException {
throw new InvalidObjectException(
AvailabilityEvent should always have data);
}
/**
* Returns a copy of the {@link Entry} whose transition
* triggered this event. The returned codeEntry/code must
* be unmarshalled in accordance with the a
* href=http://www.jini.org/standards/index.htmlJini
* Entry Specification/a.
*
* @return a copy of the {@link Entry} whose transition
* triggered this event
* @throws UnusableEntryException if the codeEntry/code
* can't be unmarshalled in the client. The next call
* must re-attempt unmarshalling the
* codeEntry/code
*/
public abstract Entry getEntry() throws UnusableEntryException;
/**
* Returns a emsnapshot/em of the {@link Entry} whose
Re: Explicit Serialization API and Security
I decided to sample cpu load (see attached), with debugging enabled for the validating ObjectInputStream and JERI, so heaps of output to the console. There are no performance optimisations with stream validation, I've just focused on correctness and security. Thank you HotSpot developers, nice job :) To give you some background in the tests, there's a heap of dynamic class loading going on with codebase downloads, Remote Invocations etc. I'll profile it on Sparc T2+ in the near future with Oracle express. Sure miss the sparc gear, OBP and when Solaris was open for a brief snapshot in time, can only use it for dev testing now, can't afford to use it for production. Cheers, Peter.
Re: Explicit Serialization API and Security
I have attempted to capture some of the ideas that we have discussed so far. https://bugs.openjdk.java.net/browse/JDK-8071471 -Chris. On 21/01/15 21:43, David M. Lloyd wrote: At some point, the responsibility *must* fall on the author of the serializable class in question to avoid constructs which are exploitable - like finalizers, and classes that can have side-effects before validation can be completed. On 01/21/2015 02:26 PM, Peter Firmstone wrote: Don't forget that null may also be an invalid state. What I have learnt so far: 1. An attacker can craft a stream to obtain a reference to any object created during deserialization, using finalizers or circular links (there may be yet other undiscovered methods). 2. An attacker can craft a stream that deliberately doesn't satisfy invariants, in order to use an object to perform a function that wasn't intended by its developer. 3. Objects that interact with the stream directly using readObject et al, are often prone to DOS. Example, many objects read a length integer from the stream when creating an array or collection, without first validating it. 4. Objects that interact directly with the stream become an implicit part of the stream protocol. 5. Once you allow an object to be created, it's too late to invalidate invariants, unless the class is final and invariants are checked in every method call. 6. We need to be able to restrict classes used for deserialization to those we trust to check invariants properly (but we haven't provided a way for them to avoid object construction yet). 7. A static validator method can ONLY be used to check field invariants, not other objects and primitives that are read directly from the stream by an arbitrary Object during the process of deserialization. 8. The jvm can be modified to delay finalizer registration for deserialization. 9. Circular links can be disallowed. Ultimately however, all proposed changes add complexity, but when an object has been created with invalid invariants, an attacker will find a way. Thank you all for your time, this has been a very good discussion. Regards, Peter. On 22/01/2015 2:27 AM, Chris Hegarty wrote: On 20/01/15 20:22, Peter Levart wrote: Hi Chris and Peter, It just occurred to me that the following scheme would provide failure atomicity for the whole Object regardless of whether readObject methods are used or not for various classes in hierarchy: - allocate uninitialized instance - call default accessible constructor of the most specific non-Serializable class - deserialize (by calling readObject methods where provided) the fields of all classes in hierarchy like normally (up to this point, nothing is changed from what we have now) - if deserialization fails anywhere, undo everything by setting all the fields in the Serializable part of the hierarchy to default values (null for references, 0 for primitives), abandon the object and propagate failure. I think this is a good idea, and I can prototype something to this affect. -Chris. While deserializing, the object is in inconsistent state. If deserialization fails, this state is rolled-back to uninitialized state. finalize() can still get to the instance, but it will be uninitialized. Peter On 01/14/2015 01:58 PM, Peter Firmstone wrote: Hi Chris, Sorry, no. Currently when an ObjectStreamClass is read in from the stream, the framework searches for the first zero arg constructor of a non serializable class and creates and instance of the class read and resolved from the stream, however it does so using a super class constructor. Then from the super class down, fields are read in and set in order for each class in the object's inheritance hierarchy. The alternative I propose, doesn't create the instance, instead it reads the fields from the stream, one by one and without instantiating them, if they are newly read objects, stores them temporarily into byte [] arrays in a Map with reference handle keys, otherwise it just holds the reference handle. What it does next is wrap this information into a caller sensitive api, GetFields or ReadSerial instance, that is passed as a constructor parameter to the child class serial constructor. The child class checks invariants and reads each field it needs using a static method prior to calling a superclass constructor, each class in the inheritance hierarchy for the object then checks its invariants until it gets to the first non serializable superclass. The benefit of this order is that each class is present in the thread security context, so protection domain security and invariants are enforced before instantiating an object. Hope this helps illuminate it a little better, regards, Peter. - Original message - Peter F, I am still struggling with the basic concept of you proposal. Let me see if I understand it correctly. Does the
Re: Explicit Serialization API and Security
And that is the point of this discussion, to determine how to do just that. But considering it's such a difficult problem, we haven't figured out how to yet. Peter. - Original message - Message: 2 Date: Wed, 21 Jan 2015 15:43:13 -0600 From: David M. Lloyd david.ll...@redhat.com At some point, the responsibility *must* fall on the author of the serializable class in question to avoid constructs which are exploitable - like finalizers, and classes that can have side-effects before validation can be completed. On 01/21/2015 02:26 PM, Peter Firmstone wrote: Don't forget that null may also be an invalid state. What I have learnt so far: 1. An attacker can craft a stream to obtain a reference to any object created during deserialization, using finalizers or circular links (there may be yet other undiscovered methods). 2. An attacker can craft a stream that deliberately doesn't satisfy invariants, in order to use an object to perform a function that wasn't intended by its developer. 3. Objects that interact with the stream directly using readObject et al, are often prone to DOS. Example, many objects read a length integer from the stream when creating an array or collection, without first validating it. 4. Objects that interact directly with the stream become an implicit part of the stream protocol. 5. Once you allow an object to be created, it's too late to invalidate invariants, unless the class is final and invariants are checked in every method call. 6. We need to be able to restrict classes used for deserialization to those we trust to check invariants properly (but we haven't provided a way for them to avoid object construction yet). 7. A static validator method can ONLY be used to check field invariants, not other objects and primitives that are read directly from the stream by an arbitrary Object during the process of deserialization. 8. The jvm can be modified to delay finalizer registration for deserialization. 9. Circular links can be disallowed. Ultimately however, all proposed changes add complexity, but when an object has been created with invalid invariants, an attacker will find a way. Thank you all for your time, this has been a very good discussion. Regards, Peter. On 22/01/2015 2:27 AM, Chris Hegarty wrote: On 20/01/15 20:22, Peter Levart wrote: Hi Chris and Peter, It just occurred to me that the following scheme would provide failure atomicity for the whole Object regardless of whether readObject methods are used or not for various classes in hierarchy: - allocate uninitialized instance - call default accessible constructor of the most specific non-Serializable class - deserialize (by calling readObject methods where provided) the fields of all classes in hierarchy like normally (up to this point, nothing is changed from what we have now) - if deserialization fails anywhere, undo everything by setting all the fields in the Serializable part of the hierarchy to default values (null for references, 0 for primitives), abandon the object and propagate failure. I think this is a good idea, and I can prototype something to this affect. -Chris. While deserializing, the object is in inconsistent state. If deserialization fails, this state is rolled-back to uninitialized state. finalize() can still get to the instance, but it will be uninitialized. Peter On 01/14/2015 01:58 PM, Peter Firmstone wrote: Hi Chris, Sorry, no. Currently when an ObjectStreamClass is read in from the stream, the framework searches for the first zero arg constructor of a non serializable class and creates and instance of the class read and resolved from the stream, however it does so using a super class constructor. Then from the super class down, fields are read in and set in order for each class in the object's inheritance hierarchy. The alternative I propose, doesn't create the instance, instead it reads the fields from the stream, one by one and without instantiating them, if they are newly read objects, stores them temporarily into byte [] arrays in a Map with reference handle keys, otherwise it just holds the reference handle. What it does next is wrap this information into a caller sensitive api, GetFields or ReadSerial instance, that is passed as a constructor parameter to the child class serial constructor. The child class checks invariants and reads each field it needs using a static method prior to calling a superclass constructor, each class in the inheritance hierarchy for the object then checks its invariants until it gets to the first non serializable superclass. The benefit of this order is that
Re: Explicit Serialization API and Security
On 20/01/15 20:22, Peter Levart wrote: Hi Chris and Peter, It just occurred to me that the following scheme would provide failure atomicity for the whole Object regardless of whether readObject methods are used or not for various classes in hierarchy: - allocate uninitialized instance - call default accessible constructor of the most specific non-Serializable class - deserialize (by calling readObject methods where provided) the fields of all classes in hierarchy like normally (up to this point, nothing is changed from what we have now) - if deserialization fails anywhere, undo everything by setting all the fields in the Serializable part of the hierarchy to default values (null for references, 0 for primitives), abandon the object and propagate failure. I think this is a good idea, and I can prototype something to this affect. -Chris. While deserializing, the object is in inconsistent state. If deserialization fails, this state is rolled-back to uninitialized state. finalize() can still get to the instance, but it will be uninitialized. Peter On 01/14/2015 01:58 PM, Peter Firmstone wrote: Hi Chris, Sorry, no. Currently when an ObjectStreamClass is read in from the stream, the framework searches for the first zero arg constructor of a non serializable class and creates and instance of the class read and resolved from the stream, however it does so using a super class constructor. Then from the super class down, fields are read in and set in order for each class in the object's inheritance hierarchy. The alternative I propose, doesn't create the instance, instead it reads the fields from the stream, one by one and without instantiating them, if they are newly read objects, stores them temporarily into byte [] arrays in a Map with reference handle keys, otherwise it just holds the reference handle. What it does next is wrap this information into a caller sensitive api, GetFields or ReadSerial instance, that is passed as a constructor parameter to the child class serial constructor. The child class checks invariants and reads each field it needs using a static method prior to calling a superclass constructor, each class in the inheritance hierarchy for the object then checks its invariants until it gets to the first non serializable superclass. The benefit of this order is that each class is present in the thread security context, so protection domain security and invariants are enforced before instantiating an object. Hope this helps illuminate it a little better, regards, Peter. - Original message - Peter F, I am still struggling with the basic concept of you proposal. Let me see if I understand it correctly. Does the following describe a similar scenario as you envisage: 1) For each Serializable type, T, in the deserialized types hierarchy, starting with the top most ( closest to j.l.Object ), 1a) Read T's fields from the stream, fields 1b) validate(t, fields) // t will be null first time 1c) allocate a new instance of T, and assign to t 1d) set fields in t 2) Return t; So for each level in the hierarchy, an instance of a type is created only after its invariants have been checked. This instance is then passed to the next level so it can participate in that levels invariants validation. If this scenario is along the same lines as yours, then I just don't see how 1c above will always be possible. If we could somehow make the object caller sensitive until after deserialization completes, then could avoid having to try to allocate multiple instance down the hierarchy. -Chris. On 13/01/15 10:24, Peter Firmstone wrote: Could we use a static validator method and generate bytecode for constructors dynamically? The developer can optionally implement the constructors. static GetField invariantCheck(GetField f); Create a caller sensitive GetField implementation and add a two new methods to GetField: abstract Object createSuper(); // to access superclass object methods for inavariant checking. abstract Class getType(String name); Set fields from within constructors. The generated constructors are straight forward: 1. Call static method. 2. Call super class constructor with result from static method. 3. Set final fields 4. How to set transient fields, implement a private method called from within the constructor? Require a permission to extend GetField?
Re: Explicit Serialization API and Security
Don't forget that null may also be an invalid state. What I have learnt so far: 1. An attacker can craft a stream to obtain a reference to any object created during deserialization, using finalizers or circular links (there may be yet other undiscovered methods). 2. An attacker can craft a stream that deliberately doesn't satisfy invariants, in order to use an object to perform a function that wasn't intended by its developer. 3. Objects that interact with the stream directly using readObject et al, are often prone to DOS. Example, many objects read a length integer from the stream when creating an array or collection, without first validating it. 4. Objects that interact directly with the stream become an implicit part of the stream protocol. 5. Once you allow an object to be created, it's too late to invalidate invariants, unless the class is final and invariants are checked in every method call. 6. We need to be able to restrict classes used for deserialization to those we trust to check invariants properly (but we haven't provided a way for them to avoid object construction yet). 7. A static validator method can ONLY be used to check field invariants, not other objects and primitives that are read directly from the stream by an arbitrary Object during the process of deserialization. 8. The jvm can be modified to delay finalizer registration for deserialization. 9. Circular links can be disallowed. Ultimately however, all proposed changes add complexity, but when an object has been created with invalid invariants, an attacker will find a way. Thank you all for your time, this has been a very good discussion. Regards, Peter. On 22/01/2015 2:27 AM, Chris Hegarty wrote: On 20/01/15 20:22, Peter Levart wrote: Hi Chris and Peter, It just occurred to me that the following scheme would provide failure atomicity for the whole Object regardless of whether readObject methods are used or not for various classes in hierarchy: - allocate uninitialized instance - call default accessible constructor of the most specific non-Serializable class - deserialize (by calling readObject methods where provided) the fields of all classes in hierarchy like normally (up to this point, nothing is changed from what we have now) - if deserialization fails anywhere, undo everything by setting all the fields in the Serializable part of the hierarchy to default values (null for references, 0 for primitives), abandon the object and propagate failure. I think this is a good idea, and I can prototype something to this affect. -Chris. While deserializing, the object is in inconsistent state. If deserialization fails, this state is rolled-back to uninitialized state. finalize() can still get to the instance, but it will be uninitialized. Peter On 01/14/2015 01:58 PM, Peter Firmstone wrote: Hi Chris, Sorry, no. Currently when an ObjectStreamClass is read in from the stream, the framework searches for the first zero arg constructor of a non serializable class and creates and instance of the class read and resolved from the stream, however it does so using a super class constructor. Then from the super class down, fields are read in and set in order for each class in the object's inheritance hierarchy. The alternative I propose, doesn't create the instance, instead it reads the fields from the stream, one by one and without instantiating them, if they are newly read objects, stores them temporarily into byte [] arrays in a Map with reference handle keys, otherwise it just holds the reference handle. What it does next is wrap this information into a caller sensitive api, GetFields or ReadSerial instance, that is passed as a constructor parameter to the child class serial constructor. The child class checks invariants and reads each field it needs using a static method prior to calling a superclass constructor, each class in the inheritance hierarchy for the object then checks its invariants until it gets to the first non serializable superclass. The benefit of this order is that each class is present in the thread security context, so protection domain security and invariants are enforced before instantiating an object. Hope this helps illuminate it a little better, regards, Peter. - Original message - Peter F, I am still struggling with the basic concept of you proposal. Let me see if I understand it correctly. Does the following describe a similar scenario as you envisage: 1) For each Serializable type, T, in the deserialized types hierarchy, starting with the top most ( closest to j.l.Object ), 1a) Read T's fields from the stream, fields 1b) validate(t, fields) // t will be null first time 1c) allocate a new instance of T, and assign to t 1d) set fields in t 2) Return t; So for each level in the hierarchy, an instance of a
Re: Explicit Serialization API and Security
At some point, the responsibility *must* fall on the author of the serializable class in question to avoid constructs which are exploitable - like finalizers, and classes that can have side-effects before validation can be completed. On 01/21/2015 02:26 PM, Peter Firmstone wrote: Don't forget that null may also be an invalid state. What I have learnt so far: 1. An attacker can craft a stream to obtain a reference to any object created during deserialization, using finalizers or circular links (there may be yet other undiscovered methods). 2. An attacker can craft a stream that deliberately doesn't satisfy invariants, in order to use an object to perform a function that wasn't intended by its developer. 3. Objects that interact with the stream directly using readObject et al, are often prone to DOS. Example, many objects read a length integer from the stream when creating an array or collection, without first validating it. 4. Objects that interact directly with the stream become an implicit part of the stream protocol. 5. Once you allow an object to be created, it's too late to invalidate invariants, unless the class is final and invariants are checked in every method call. 6. We need to be able to restrict classes used for deserialization to those we trust to check invariants properly (but we haven't provided a way for them to avoid object construction yet). 7. A static validator method can ONLY be used to check field invariants, not other objects and primitives that are read directly from the stream by an arbitrary Object during the process of deserialization. 8. The jvm can be modified to delay finalizer registration for deserialization. 9. Circular links can be disallowed. Ultimately however, all proposed changes add complexity, but when an object has been created with invalid invariants, an attacker will find a way. Thank you all for your time, this has been a very good discussion. Regards, Peter. On 22/01/2015 2:27 AM, Chris Hegarty wrote: On 20/01/15 20:22, Peter Levart wrote: Hi Chris and Peter, It just occurred to me that the following scheme would provide failure atomicity for the whole Object regardless of whether readObject methods are used or not for various classes in hierarchy: - allocate uninitialized instance - call default accessible constructor of the most specific non-Serializable class - deserialize (by calling readObject methods where provided) the fields of all classes in hierarchy like normally (up to this point, nothing is changed from what we have now) - if deserialization fails anywhere, undo everything by setting all the fields in the Serializable part of the hierarchy to default values (null for references, 0 for primitives), abandon the object and propagate failure. I think this is a good idea, and I can prototype something to this affect. -Chris. While deserializing, the object is in inconsistent state. If deserialization fails, this state is rolled-back to uninitialized state. finalize() can still get to the instance, but it will be uninitialized. Peter On 01/14/2015 01:58 PM, Peter Firmstone wrote: Hi Chris, Sorry, no. Currently when an ObjectStreamClass is read in from the stream, the framework searches for the first zero arg constructor of a non serializable class and creates and instance of the class read and resolved from the stream, however it does so using a super class constructor. Then from the super class down, fields are read in and set in order for each class in the object's inheritance hierarchy. The alternative I propose, doesn't create the instance, instead it reads the fields from the stream, one by one and without instantiating them, if they are newly read objects, stores them temporarily into byte [] arrays in a Map with reference handle keys, otherwise it just holds the reference handle. What it does next is wrap this information into a caller sensitive api, GetFields or ReadSerial instance, that is passed as a constructor parameter to the child class serial constructor. The child class checks invariants and reads each field it needs using a static method prior to calling a superclass constructor, each class in the inheritance hierarchy for the object then checks its invariants until it gets to the first non serializable superclass. The benefit of this order is that each class is present in the thread security context, so protection domain security and invariants are enforced before instantiating an object. Hope this helps illuminate it a little better, regards, Peter. - Original message - Peter F, I am still struggling with the basic concept of you proposal. Let me see if I understand it correctly. Does the following describe a similar scenario as you envisage: 1) For each Serializable type, T, in the deserialized types hierarchy, starting with the top most ( closest
Re: Explicit Serialization API and Security
Hi Chris and Peter, It just occurred to me that the following scheme would provide failure atomicity for the whole Object regardless of whether readObject methods are used or not for various classes in hierarchy: - allocate uninitialized instance - call default accessible constructor of the most specific non-Serializable class - deserialize (by calling readObject methods where provided) the fields of all classes in hierarchy like normally (up to this point, nothing is changed from what we have now) - if deserialization fails anywhere, undo everything by setting all the fields in the Serializable part of the hierarchy to default values (null for references, 0 for primitives), abandon the object and propagate failure. While deserializing, the object is in inconsistent state. If deserialization fails, this state is rolled-back to uninitialized state. finalize() can still get to the instance, but it will be uninitialized. Peter On 01/14/2015 01:58 PM, Peter Firmstone wrote: Hi Chris, Sorry, no. Currently when an ObjectStreamClass is read in from the stream, the framework searches for the first zero arg constructor of a non serializable class and creates and instance of the class read and resolved from the stream, however it does so using a super class constructor. Then from the super class down, fields are read in and set in order for each class in the object's inheritance hierarchy. The alternative I propose, doesn't create the instance, instead it reads the fields from the stream, one by one and without instantiating them, if they are newly read objects, stores them temporarily into byte [] arrays in a Map with reference handle keys, otherwise it just holds the reference handle. What it does next is wrap this information into a caller sensitive api, GetFields or ReadSerial instance, that is passed as a constructor parameter to the child class serial constructor. The child class checks invariants and reads each field it needs using a static method prior to calling a superclass constructor, each class in the inheritance hierarchy for the object then checks its invariants until it gets to the first non serializable superclass. The benefit of this order is that each class is present in the thread security context, so protection domain security and invariants are enforced before instantiating an object. Hope this helps illuminate it a little better, regards, Peter. - Original message - Peter F, I am still struggling with the basic concept of you proposal. Let me see if I understand it correctly. Does the following describe a similar scenario as you envisage: 1) For each Serializable type, T, in the deserialized types hierarchy, starting with the top most ( closest to j.l.Object ), 1a) Read T's fields from the stream, fields 1b) validate(t, fields) // t will be null first time 1c) allocate a new instance of T, and assign to t 1d) set fields in t 2) Return t; So for each level in the hierarchy, an instance of a type is created only after its invariants have been checked. This instance is then passed to the next level so it can participate in that levels invariants validation. If this scenario is along the same lines as yours, then I just don't see how 1c above will always be possible. If we could somehow make the object caller sensitive until after deserialization completes, then could avoid having to try to allocate multiple instance down the hierarchy. -Chris. On 13/01/15 10:24, Peter Firmstone wrote: Could we use a static validator method and generate bytecode for constructors dynamically? The developer can optionally implement the constructors. static GetField invariantCheck(GetField f); Create a caller sensitive GetField implementation and add a two new methods to GetField: abstract Object createSuper(); // to access superclass object methods for inavariant checking. abstract Class getType(String name); Set fields from within constructors. The generated constructors are straight forward: 1. Call static method. 2. Call super class constructor with result from static method. 3. Set final fields 4. How to set transient fields, implement a private method called from within the constructor? Require a permission to extend GetField?
Re: Explicit Serialization API and Security
- Original message - 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. You're right, circular links can't be checked until after construction. A class with one of these links will have to check its invariants on every method invocation (perhaps lazily). Could a static validator(GetField) API throw, or just return null, for a not yet created circular reference field value? Probably better to fail early and throw an exception, if the developer is aware of the circularity, they won't attempt to retrieve it before their object has been constructed. It doesn’t seem useful for validation to be able to retrieve the circular reference. I agree, not much use for validation. 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. Unfortunately read unshared has some issues too. 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? Yes. One problem with our validator, is we can only verify fields there's nothing we can do with readObject methods that read values directly from the stream. Interacting directly with the stream in readObject or writeObject methods is brittle, it requires class bytecode. Also browsing readObject implementations in openjdk, I've found dos vulnerabilities. For our project, I'm looking at implementing a stream that sacrifices compatibility on the alter of security. This will be constraint based and only used for unauthenticated network connections. I'll be using the serialization protocol, but with a public api that requires caller sensitive GetField and PutField as parameters, no direct access to the stream and no magic. The interface will have a different name, so as not to conflict with Serializable. Hindsight and learning from mistakes is 20:20. The Objects that can be serialized will be minimal: arrays String, Enum, Object primitives. Stateless Objects Proxy Maps and Collections will be converted to an immutable safe implementation that the receiver can use as an argument to their preferred map or collection, preserving Comparator's if they exist. All other Object types will have to implement the new interface. Implementers will be able to implement Serializable as well as the new interface, for compatibility. I'm not sure of what name, perhaps AtomicSerial. The receiver will be able to place size constriants on the stream, arrays, Collections and Maps. This will need to be performed over non blocking sockets as well. Sctp looks very promising for this. Peter. Or something else. -Chris. Regards, Peter. - Original message - On 15 Jan 2015, at 12:25, Peter Firmstone peter.firmst...@zeus.net.au mailto:peter.firmst...@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
Re: Explicit Serialization API and Security
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.firmst...@zeus.net.au mailto:peter.firmst...@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,
Re: Explicit Serialization API and Security
On 15 Jan 2015, at 12:25, Peter Firmstone peter.firmst...@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.
Re: Explicit Serialization API and Security
Thanks Chris, WRT circular references, is it possible to detect and delay setting these until after all verifiers run? An option might be to provide a protected method in ObjectInputStream to opt out or switch off support for circular references. Apart from Serialization, another common use for readObject is for class 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? Regards, Peter. - Original message - On 15 Jan 2015, at 12:25, Peter Firmstone peter.firmst...@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.
Re: Explicit Serialization API and Security
Chris, Can you explain some of the detail in your failure atomicity work? Thanks, Peter.
Re: Explicit Serialization API and Security
On 13/01/2015 1:56 AM, Chris Hegarty wrote: On 10/01/15 07:00, Peter Firmstone wrote: As shown in my earlier example, intra class invariants can be enforced using Serialization constructors, from which static methods are called to check invariants prior to super class constructors being called. Yes, but it is cumbersome and easy to make mistakes. I think if the user has unit tests for the invariants, then the liklihood of mistakes is very low. Cumbersome, I don't think we need to worry about that. I've had to do so much work, implementing work arounds, it would be like all my prayers answered just to be able to prevent reference stealing attacks and enforce invariants with failure atomicity. Did you know that .NET uses de-serialization constructors? A third option that hasn't yet been considered would be to investigate an api that various already existing frameworks can implement providers for, so they no longer need to find creative ways to construct instances of java platform classes when unmarshalling. Presently, I override ObjectInputStream and use a Permission called DeserializationPermission to limit classes that can be deserialized. Untrusted connections are run from unprivileged context to limit classes that can be instantiated, while those with trusted connections are run with a Subject that allows any class to be deserialized. Interesting. I think there is overlap here with confinement. The good thing about using a permission check is it allows administrative based changed when a new vulnerability is discovered. Regards, Peter.
Re: Explicit Serialization API and Security
On 15 January 2015 at 13:01, Peter Firmstone peter.firmst...@zeus.net.au wrote: A third option that hasn't yet been considered would be to investigate an api that various already existing frameworks can implement providers for, so they no longer need to find creative ways to construct instances of java platform classes when unmarshalling. Which is part of what this project is trying to build (for the JDK) https://github.com/jodastephen/property-alliance Stephen
Re: Explicit Serialization API and Security
It's only top down if there are intra class invariants, in this case invariants are checked more than once. This done simply because we can't move up the constructor call chain untill all child class invariants have been satisfied. For simpler class relationships, invariant check order is bottom up. The child class cannot allow a super class constructor to be called until its own invariants have been satisfied, in order to prevent an illegal instance of itself being created. If we check invariants after the object has been created, unless we introduce a check initialized method into every method, we loose. Unfortunately we can't share this check with child classes, so the class really should be declared final, or all its methods should be, including any superclass methods, including Object equals and hashcode. Unfortunately enforcing security by enforcing invariants after calling a superclass constructor, is far more complicated. Using a serial constructor, is less evil. Regards, Peter. - Original message - On 14/01/15 12:58, Peter Firmstone wrote: Hi Chris, Sorry, no. Currently when an ObjectStreamClass is read in from the stream, the framework searches for the first zero arg constructor of a non serializable class and creates and instance of the class read and resolved from the stream, however it does so using a super class constructor. Then from the super class down, fields are read in and set in order for each class in the object's inheritance hierarchy. This is my understanding also. The alternative I propose, doesn't create the instance, instead it reads the fields from the stream, one by one and without instantiating them, if they are newly read objects, stores them temporarily into byte [] arrays in a Map with reference handle keys, otherwise it just holds the reference handle. GetField.get(...), or ReadSerial, will create an object before returning it. So the invariant checker methods will turn the stream field values into objects at some point. Also, from your previous examples, I though I seen the invariant checker method explicitly create superclass instance, so that it could be used in the validation, right? What I was trying to do was to reduce your examples, and description, into a sequence of steps, to try to better understand how this would work. What I came up with, I believe, is equivalent to what was done in this example [1]. If you intend to chain the serial constructors up with static check methods that create superclass instances, then I think is equates to top-down checking as I have described it. What it does next is wrap this information into a caller sensitive api, GetFields or ReadSerial instance, that is passed as a constructor parameter to the child class serial constructor. The child class checks invariants and reads each field it needs using a static method prior to calling a superclass constructor, each class in the inheritance hierarchy for the object then checks its invariants until it gets to the first non serializable superclass. The benefit of this order is that each class is present in the thread security context, so protection domain security and invariants are enforced before instantiating an object. Hope this helps illuminate it a little better, regards, -Chris. [1] http://mail.openjdk.java.net/pipermail/core-libs-dev/2015-January/030550.html Peter. - Original message - Peter F, I am still struggling with the basic concept of you proposal. Let me see if I understand it correctly. Does the following describe a similar scenario as you envisage: 1) For each Serializable type, T, in the deserialized types hierarchy, starting with the top most ( closest to j.l.Object ), 1a) Read T's fields from the stream, fields 1b) validate(t, fields) // t will be null first time 1c) allocate a new instance of T, and assign to t 1d) set fields in t 2) Return t; So for each level in the hierarchy, an instance of a type is created only after its invariants have been checked. This instance is then passed to the next level so it can participate in that levels invariants validation. If this scenario is along the same lines as yours, then I just don't see how 1c above will always be possible. If we could somehow make the object caller sensitive until after deserialization completes, then could avoid having to try to allocate multiple instance down the hierarchy. -Chris. On 13/01/15 10:24, Peter Firmstone wrote: Could we use a static validator method and generate bytecode for constructors dynamically? The developer can optionally implement the constructors. static GetField invariantCheck(GetField f); Create a caller sensitive GetField implementation and add a two new methods to GetField: abstract Object createSuper();
Re: Explicit Serialization API and Security
On 01/14/2015 12:37 PM, Chris Hegarty wrote: What do you think? I agree completely. An API at the level that you are proposing will provide the necessary functionality and flexibility that is required to do validation in readObject. As you clearly stated, and is already the case, validation in this way can depend on supertype state. Failure automicity of the whole type hierarchy is the only thing that is missing, but I think that could possibly be built on top, or solved in a different way. I don't see that as being a blocker for moving this forward. This proposal stands on its own merits. Peter, Is this something that you want to actively flesh out? If not, I can try to help move this forward. Hi Chris, I can prepare a prototype, yes. Just give me a couple of days. Regards, Peter -Chris.
Re: Explicit Serialization API and Security
The following is a summary of our exploration so far: We have narrowed down to two options: A. constructors with static invariant check methods. B. Continue to use readObject(), we have a proposed solution to the final field problem and could modify the jvm to register for finalization late to avoid finalizer attacks. Question: How can a programmer protect against reference stealing attacks? Once this is taken into consideration, B's complexity exceeds A. This is a good guideline for constructors, but deserialization offers more attack vectors: https://www.securecoding.cert.org/confluence/display/java/TSM01-J.+Do+not+let+the+this+reference+escape+during+object+construction Item A.: * Prevents reference stealing attacks. * A subclass checking its invariants can only create a superclass instance for intra invariant checking, if superclass invariants are satisfied. Logic Steps for item A: 1. Framework calls child class serial constructor 2. Child class constructor calls static invariant check. 3. For each class in hierarchy: a. If current class has no intra class invariants: * Check invariants and return result to super class constructor or throw Exception. b. Else if the child class has intra class invariants: * Create a superclass instance (invariants are checked before it can be created). * Check invariants and return result to super class constructor or throw Exception. 7. Each Superclass checks its invariants, repeating step 3, until Object's constructor is called, or an Exception is thrown. 8. Constructors return, each class descending down the object hierarchy sets fields. Process is complete and new object is published. Remember that when satisfying invariants, class types can be checked before deserializing an object. For flexibility this could be in the form of a permission check. Regards, Peter.
Re: Explicit Serialization API and Security
On 13/01/15 10:00, Peter Levart wrote:
Hi Chris,
I stepped back and asked myself what problem(s) we are searching
solution(s) for and tried to see how existing API solves or doesn't
solve them. I think that existing readObject() + GetFields API already
solves most of the problems discussed so far as it:
- provides encapsulation (using GetFields API we only have access to
deserialized values of the class where readObject() method being invoked
is declared)
- provides a means to check intra-object invariants in an encapsulated
way - the superclass state is already initialized/deserialized when
readObject() is called, so we can check GetFields provided values
against superclass state in a way that preserves encapsulation.
What readObject()/GetFields combo doesn't provide is a relatively nice
solution for final fields (we have to resort to clumsy reflection). So
why not adding just that to the API? For example, extend
ObjectInputStream with the following method:
ObjectInputStream {
...
FieldAccess fields();
...
}
where FieldAccess is something like:
public interface FieldAccess {
FieldAccess set(String fieldName, boolean value);
FieldAccess set(String fieldName, byte value);
FieldAccess set(String fieldName, char value);
FieldAccess set(String fieldName, short value);
FieldAccess set(String fieldName, int value);
FieldAccess set(String fieldName, long value);
FieldAccess set(String fieldName, float value);
FieldAccess set(String fieldName, double value);
T FieldAccess set(String fieldName, T value);
}
...which would confine access to just those fields declared in the class
where readObject() being invoked is declared and just for the instance
and time of readObject() call-back.
Simple, flexible and stright-forward. It solves the problem with final
fields. It doesn't provide any auto-magic, which, although it might seem
pretty at first, can sometimes just be in the way because of
inflexibility. Sugar can be added though (in the form of
GetFields.defaultReadFields() or even special check() methods), but
flexibility is a necessary ingredient, I think.
What do you think?
I agree completely.
An API at the level that you are proposing will provide the necessary
functionality and flexibility that is required to do validation in
readObject. As you clearly stated, and is already the case, validation
in this way can depend on supertype state.
Failure automicity of the whole type hierarchy is the only thing that is
missing, but I think that could possibly be built on top, or solved in a
different way. I don't see that as being a blocker for moving this
forward. This proposal stands on its own merits.
Peter,
Is this something that you want to actively flesh out? If not, I can
try to help move this forward.
-Chris.
Regards, Peter
On 01/12/2015 04:56 PM, Chris Hegarty wrote:
On 10/01/15 07:00, Peter Firmstone wrote:
Again, thank you all for engaging in discussion of this very difficult
topic.
While we can't presently check intra object dependencies during
deserialization with readObject(), the examples I provide can do this.
I have replied to Davids mail with a small change to GetField ( added
superTypeFields() ) to return the deserialized supertypes fields. This
gives subtypes the ability to check values of the supertypes
persistent state.
As with your original proposal, it is limited to the persistent state
as read off the stream, and not the transient state, but I think it
gets us most of the way there.
In your original proposal, it looks quite cumbersome to hook up the
static validator method in the constructor hierarchy ( as it is to do
this with standard constructors too ). It also relies on the fact that
the subtype has to create an instance of the supertype. I just wonder
if we can push on the alternative static validator proposal to come up
with something a but more attractive ( maybe not! ).
ObjectInputValidation is sufficient for enforcing business rules,
however it can't stop an attacker. By the time the validator runs, the
attackers object of choice has been instantiated and it's game over.
That's right, the attacker may choose any Serializable class from the
classpath, and may even extend non serializable classes, with a zero arg
constructor, such as ClassLoader.
To trust deserialization of an Object, as opposed to a String or
primitive, the ObjectInputStream (or overriding subclass) must limit the
classes allowed to be deserialized.
Agreed. I am working on fleshing out a more concrete proposal on
confinement, that I mentioned in previous mails. Hopefully, I will
have something in the next few days.
For a class to be trusted, it must be trusted to check its parameters
and enforce its invariants during object deserialization and to not
deserialize with priviliged context.
Validating an entire object graph's invariants, requires each
Re: Explicit Serialization API and Security
On 13/01/15 00:51, Peter Firmstone wrote: - Original message - On 10/01/15 07:00, Peter Firmstone wrote: Again, thank you all for engaging in discussion of this very difficult topic. While we can't presently check intra object dependencies during deserialization with readObject(), the examples I provide can do this. I have replied to Davids mail with a small change to GetField ( added superTypeFields() ) to return the deserialized supertypes fields. This gives subtypes the ability to check values of the supertypes persistent state. Unfortunately this breaks encapsulation, a class is then locked into using that serial form as public api forever. Yes, of course. Withdrawn. -Chris.
Re: Explicit Serialization API and Security
Hi Chris, Sorry, no. Currently when an ObjectStreamClass is read in from the stream, the framework searches for the first zero arg constructor of a non serializable class and creates and instance of the class read and resolved from the stream, however it does so using a super class constructor. Then from the super class down, fields are read in and set in order for each class in the object's inheritance hierarchy. The alternative I propose, doesn't create the instance, instead it reads the fields from the stream, one by one and without instantiating them, if they are newly read objects, stores them temporarily into byte [] arrays in a Map with reference handle keys, otherwise it just holds the reference handle. What it does next is wrap this information into a caller sensitive api, GetFields or ReadSerial instance, that is passed as a constructor parameter to the child class serial constructor. The child class checks invariants and reads each field it needs using a static method prior to calling a superclass constructor, each class in the inheritance hierarchy for the object then checks its invariants until it gets to the first non serializable superclass. The benefit of this order is that each class is present in the thread security context, so protection domain security and invariants are enforced before instantiating an object. Hope this helps illuminate it a little better, regards, Peter. - Original message - Peter F, I am still struggling with the basic concept of you proposal. Let me see if I understand it correctly. Does the following describe a similar scenario as you envisage: 1) For each Serializable type, T, in the deserialized types hierarchy, starting with the top most ( closest to j.l.Object ), 1a) Read T's fields from the stream, fields 1b) validate(t, fields) // t will be null first time 1c) allocate a new instance of T, and assign to t 1d) set fields in t 2) Return t; So for each level in the hierarchy, an instance of a type is created only after its invariants have been checked. This instance is then passed to the next level so it can participate in that levels invariants validation. If this scenario is along the same lines as yours, then I just don't see how 1c above will always be possible. If we could somehow make the object caller sensitive until after deserialization completes, then could avoid having to try to allocate multiple instance down the hierarchy. -Chris. On 13/01/15 10:24, Peter Firmstone wrote: Could we use a static validator method and generate bytecode for constructors dynamically? The developer can optionally implement the constructors. static GetField invariantCheck(GetField f); Create a caller sensitive GetField implementation and add a two new methods to GetField: abstract Object createSuper(); // to access superclass object methods for inavariant checking. abstract Class getType(String name); Set fields from within constructors. The generated constructors are straight forward: 1. Call static method. 2. Call super class constructor with result from static method. 3. Set final fields 4. How to set transient fields, implement a private method called from within the constructor? Require a permission to extend GetField?
Re: Explicit Serialization API and Security
On 14/01/15 12:58, Peter Firmstone wrote: Hi Chris, Sorry, no. Currently when an ObjectStreamClass is read in from the stream, the framework searches for the first zero arg constructor of a non serializable class and creates and instance of the class read and resolved from the stream, however it does so using a super class constructor. Then from the super class down, fields are read in and set in order for each class in the object's inheritance hierarchy. This is my understanding also. The alternative I propose, doesn't create the instance, instead it reads the fields from the stream, one by one and without instantiating them, if they are newly read objects, stores them temporarily into byte [] arrays in a Map with reference handle keys, otherwise it just holds the reference handle. GetField.get(...), or ReadSerial, will create an object before returning it. So the invariant checker methods will turn the stream field values into objects at some point. Also, from your previous examples, I though I seen the invariant checker method explicitly create superclass instance, so that it could be used in the validation, right? What I was trying to do was to reduce your examples, and description, into a sequence of steps, to try to better understand how this would work. What I came up with, I believe, is equivalent to what was done in this example [1]. If you intend to chain the serial constructors up with static check methods that create superclass instances, then I think is equates to top-down checking as I have described it. What it does next is wrap this information into a caller sensitive api, GetFields or ReadSerial instance, that is passed as a constructor parameter to the child class serial constructor. The child class checks invariants and reads each field it needs using a static method prior to calling a superclass constructor, each class in the inheritance hierarchy for the object then checks its invariants until it gets to the first non serializable superclass. The benefit of this order is that each class is present in the thread security context, so protection domain security and invariants are enforced before instantiating an object. Hope this helps illuminate it a little better, regards, -Chris. [1] http://mail.openjdk.java.net/pipermail/core-libs-dev/2015-January/030550.html Peter. - Original message - Peter F, I am still struggling with the basic concept of you proposal. Let me see if I understand it correctly. Does the following describe a similar scenario as you envisage: 1) For each Serializable type, T, in the deserialized types hierarchy, starting with the top most ( closest to j.l.Object ), 1a) Read T's fields from the stream, fields 1b) validate(t, fields) // t will be null first time 1c) allocate a new instance of T, and assign to t 1d) set fields in t 2) Return t; So for each level in the hierarchy, an instance of a type is created only after its invariants have been checked. This instance is then passed to the next level so it can participate in that levels invariants validation. If this scenario is along the same lines as yours, then I just don't see how 1c above will always be possible. If we could somehow make the object caller sensitive until after deserialization completes, then could avoid having to try to allocate multiple instance down the hierarchy. -Chris. On 13/01/15 10:24, Peter Firmstone wrote: Could we use a static validator method and generate bytecode for constructors dynamically? The developer can optionally implement the constructors. static GetField invariantCheck(GetField f); Create a caller sensitive GetField implementation and add a two new methods to GetField: abstract Object createSuper(); // to access superclass object methods for inavariant checking. abstract Class getType(String name); Set fields from within constructors. The generated constructors are straight forward: 1. Call static method. 2. Call super class constructor with result from static method. 3. Set final fields 4. How to set transient fields, implement a private method called from within the constructor? Require a permission to extend GetField?
Re: Explicit Serialization API and Security
Hi Chris,
I stepped back and asked myself what problem(s) we are searching
solution(s) for and tried to see how existing API solves or doesn't
solve them. I think that existing readObject() + GetFields API already
solves most of the problems discussed so far as it:
- provides encapsulation (using GetFields API we only have access to
deserialized values of the class where readObject() method being invoked
is declared)
- provides a means to check intra-object invariants in an encapsulated
way - the superclass state is already initialized/deserialized when
readObject() is called, so we can check GetFields provided values
against superclass state in a way that preserves encapsulation.
What readObject()/GetFields combo doesn't provide is a relatively nice
solution for final fields (we have to resort to clumsy reflection). So
why not adding just that to the API? For example, extend
ObjectInputStream with the following method:
ObjectInputStream {
...
FieldAccess fields();
...
}
where FieldAccess is something like:
public interface FieldAccess {
FieldAccess set(String fieldName, boolean value);
FieldAccess set(String fieldName, byte value);
FieldAccess set(String fieldName, char value);
FieldAccess set(String fieldName, short value);
FieldAccess set(String fieldName, int value);
FieldAccess set(String fieldName, long value);
FieldAccess set(String fieldName, float value);
FieldAccess set(String fieldName, double value);
T FieldAccess set(String fieldName, T value);
}
...which would confine access to just those fields declared in the class
where readObject() being invoked is declared and just for the instance
and time of readObject() call-back.
Simple, flexible and stright-forward. It solves the problem with final
fields. It doesn't provide any auto-magic, which, although it might seem
pretty at first, can sometimes just be in the way because of
inflexibility. Sugar can be added though (in the form of
GetFields.defaultReadFields() or even special check() methods), but
flexibility is a necessary ingredient, I think.
What do you think?
Regards, Peter
On 01/12/2015 04:56 PM, Chris Hegarty wrote:
On 10/01/15 07:00, Peter Firmstone wrote:
Again, thank you all for engaging in discussion of this very difficult
topic.
While we can't presently check intra object dependencies during
deserialization with readObject(), the examples I provide can do this.
I have replied to Davids mail with a small change to GetField ( added
superTypeFields() ) to return the deserialized supertypes fields. This
gives subtypes the ability to check values of the supertypes
persistent state.
As with your original proposal, it is limited to the persistent state
as read off the stream, and not the transient state, but I think it
gets us most of the way there.
In your original proposal, it looks quite cumbersome to hook up the
static validator method in the constructor hierarchy ( as it is to do
this with standard constructors too ). It also relies on the fact that
the subtype has to create an instance of the supertype. I just wonder
if we can push on the alternative static validator proposal to come up
with something a but more attractive ( maybe not! ).
ObjectInputValidation is sufficient for enforcing business rules,
however it can't stop an attacker. By the time the validator runs, the
attackers object of choice has been instantiated and it's game over.
That's right, the attacker may choose any Serializable class from the
classpath, and may even extend non serializable classes, with a zero arg
constructor, such as ClassLoader.
To trust deserialization of an Object, as opposed to a String or
primitive, the ObjectInputStream (or overriding subclass) must limit the
classes allowed to be deserialized.
Agreed. I am working on fleshing out a more concrete proposal on
confinement, that I mentioned in previous mails. Hopefully, I will
have something in the next few days.
For a class to be trusted, it must be trusted to check its parameters
and enforce its invariants during object deserialization and to not
deserialize with priviliged context.
Validating an entire object graph's invariants, requires each class in
the graph to take responsibility for validating and enforcing its own
invariants.
Agreed, and I think that the revised static validator method gives us
this.
As shown in my earlier example, intra class invariants can be enforced
using Serialization constructors, from which static methods are called
to check invariants prior to super class constructors being called.
Yes, but it is cumbersome and easy to make mistakes.
Presently, I override ObjectInputStream and use a Permission called
DeserializationPermission to limit classes that can be deserialized.
Untrusted connections are run from unprivileged context to limit classes
that can be instantiated, while those
Re: Explicit Serialization API and Security
Could we use a static validator method and generate bytecode for constructors dynamically? The developer can optionally implement the constructors. static GetField invariantCheck(GetField f); Create a caller sensitive GetField implementation and add a two new methods to GetField: abstract Object createSuper(); // to access superclass object methods for inavariant checking. abstract Class getType(String name); Set fields from within constructors. The generated constructors are straight forward: 1. Call static method. 2. Call super class constructor with result from static method. 3. Set final fields 4. How to set transient fields, implement a private method called from within the constructor? Require a permission to extend GetField?
Re: Explicit Serialization API and Security
On 08/01/15 20:10, Brian Goetz wrote:
1) Validate invariants
A clear and easy to understand mechanism that can validate the
deserialized
fields. Does not prevent the use of final fields, as the
serialization framework
will be responsible for setting them. Something along the lines
of what David
suggested:
private static void validate(GetField fields) {
if (fields.getInt(lo) fields.getInt(hi)) { ... }
}
This could be a “special” method, or annotation driven. TBD.
Note: the validate method is static, so the object instance is
not required to
be created before running the validation.
Sort of...
This is true if the fields participating in the invariant are
primitives. But if they're refs, what do you do? What if you want to
validate something like
count == list.size() // fields are int count, List list
? Then wouldn't GetField.getObject have to deserialize the object
referred to by that field?
Yes it would.
For clarity, I would like to describe how things currently work.
1) Allocate a new instance of the deserialized type.
2) Call the first non-Serializable types no-arg constructor
( may be j.l.Object ).
3) For each type in the deserialized types hierarchy, starting
with the top most ( closest to j.l.Object ),
3a) create objects representing all fields values for the type
[this step is recursive and will go to 1 until all
non-primitive types have been created ]
3b) [ holder for invariant validation ]
3c) assign objects to their respective members of the
containing instance
[ For simplicity, ignore cyclic references are readObjectXXX for now,
I will address them separately. ]
Without any user visible side-effects, no readObjectXXX methods, it
would appear that there is no reason why 1 2 must happen before 3a.
Since objects representing field values are created recursively, then
all the objects representing the field values are created, per class in
the hierarchy, before being assigned. If we have no reaObjectXXX
methods, then the objects being created in 3a could be stored locally,
repeating 3a as needed, and only assign after all types in the hierarchy
have been walked. Essentially the sequence of steps could be, 3, 3a+,
[3b], 1, 2, 3c+.
Given this, an the invariant could be validated at 3b, without the need
for the creation of the containing object.
Cyclic references: If we encounter a cyclic reference, then we can
de-optimize; stop, create the required instances reachable in the
graph, fill in whatever fields are currently known, then continue.
readObjectXXX: Since these are instance methods then they must have
visibility to any deserialized state in the super type. These can be
handled similarly to cyclic references, but can be determine up front,
at step 3, rather than in the flow.
-Chris.
Re: Explicit Serialization API and Security
On 08/01/15 22:03, David M. Lloyd wrote:
private static void validate(GetField fields) {
if (fields.getInt(lo) fields.getInt(hi)) { ... }
}
...
In fact you cannot validate invariants across multiple objects at all
using this method *or* readObject() (existing or enhanced) unless the
object in question is an enum, Class, or String (and a few other special
cases) because you can't rely on initialization order during
deserialization. That's the very reason why OIS#registerValidation()
even exists - inter-object validation is not possible until after the
root-most readObject has completed, which is the time when validations
are executed. Robust validation of a given object class may require two
stages - near validation and spanning validation - to fully
validate. However the readObject() approach and its proposed variations
(including the static validate() version) can still be useful for
fail-fast and non-complex validations; you just have to understand that
(just as today) any Object you examine might not be fully initialized yet.
If I may, I'd like to build a little on this proposal:
1) Specify that validate is called down the hierarchy, from
j.l.Object.
2) Provide access to persistent supertype's fields, so they can
participate in the validation.
public static abstract class GetField {
/**
* Returns the persistent fields of the supertype, read from
* the stream, or null if the the supertype is not Serializable.
*/
public abstract GetField superTypeFields();
}
-Chris.
Re: Explicit Serialization API and Security
Am 12.01.2015 um 17:15 schrieb Stephen Colebourne scolebou...@joda.org: On 12 January 2015 at 11:37, Chris Hegarty chris.hega...@oracle.com wrote: For clarity, I would like to describe how things currently work. 1) Allocate a new instance of the deserialized type. 2) Call the first non-Serializable types no-arg constructor ( may be j.l.Object ). 3) For each type in the deserialized types hierarchy, starting with the top most ( closest to j.l.Object ), 3a) create objects representing all fields values for the type [this step is recursive and will go to 1 until all non-primitive types have been created ] 3b) [ holder for invariant validation ] 3c) assign objects to their respective members of the containing instance Just to note that for me, the problem is that (1) happens at all. IMO, serialization should be separated entirely. Objects should only be created via standard constructors or factory methods. That is the direction that the static readObjectAndResolve() was in. Talk of dedicated serialization constructors or more use of unsafe doesn't really strike me as much of a way forward, except perhaps where compatibility is concerned. Stephen That’s the way we mostly implemented the serialization in our product in order to be more open for simpler conversion of enhancements being done on existing classes. In our case we serialize using a proxy class with the writeReplace() on the original and readObject()/readResolve() method on the serialize proxy class. One of the most annoying part of the existing Serialization for me that should also be looked into is the way that errors are being reported if something goes wrong while serializing/deserializing objects. In that part I spent the most of my time in the past. - Patrick
Re: Explicit Serialization API and Security
- Original message - On 10/01/15 07:00, Peter Firmstone wrote: Again, thank you all for engaging in discussion of this very difficult topic. While we can't presently check intra object dependencies during deserialization with readObject(), the examples I provide can do this. I have replied to Davids mail with a small change to GetField ( added superTypeFields() ) to return the deserialized supertypes fields. This gives subtypes the ability to check values of the supertypes persistent state. Unfortunately this breaks encapsulation, a class is then locked into using that serial form as public api forever. As with your original proposal, it is limited to the persistent state as read off the stream, and not the transient state, but I think it gets us most of the way there. In your original proposal, it looks quite cumbersome to hook up the static validator method in the constructor hierarchy ( as it is to do this with standard constructors too ). It also relies on the fact that the subtype has to create an instance of the supertype. I just wonder if we can push on the alternative static validator proposal to come up with something a but more attractive ( maybe not! ). Before proposing the constructor, I had considered a static validator method, with the following limitations: * A static method invariant check, while simple, cannot accommodate intra object invariants in class inheritance hierarchies without breaking encapsulation and allowing child classes to examine stream content. * When we break encapsulation, complexity of implementing Serializable safely increases; how to evolve serial form without breaking compatibility. * An Object under construction, usually copies mutable parameters before checking invariants, how can we guarantee that these objects are not shared and haven't mutated between the time the invariants are checked and when fields are set by reflection after construction? I'd also considered a static factory method: * It offers maximum flexibility for evolution; it can return objects of any class it likes. * Lacks the ability to recreate inherited private state, without breaking encapsulation. * Issues establishing intra object dependencies in inheritance hierarchies. Advantages that led me to propose the use of a constructor: * If existing constructors throw an exception when invariants haven't been satisfied, best practise dictates they must do so prior to calling Object's super class constructor to avoid finalizer attacks; static validation methods should already exist, if not the class should be refactored anyway, whether it implements Serializable or not. * Improved code reuse; invariant validation code is shared with other constructors. * Ability to satisfy complex intra object invariants in class inheritance heirarchies without breaking encapsulation. * Although it appears complex at first glance, once learnt, it's easily proven, preserves encapsulation and is easy to implement and test. * No serial form lock in, encapsulation has not been broken. Only the implementing class knows and uses the field names and types directly. It just seems like the best compromise, it's not perfect, is has less flaws. Have you seen the changes I proposing for failure atomicity, preliminary webrev http://cr.openjdk.java.net/~chegar/failureAtomicity/ .. and I think we can go further than this, creating the containing object lazily, if there are no readObjectXXX methods in the hierarchy. Yes, but need some more time to absorb it, before commenting. Regards, Peter.
Re: Explicit Serialization API and Security
On 10/01/15 07:00, Peter Firmstone wrote: Again, thank you all for engaging in discussion of this very difficult topic. While we can't presently check intra object dependencies during deserialization with readObject(), the examples I provide can do this. I have replied to Davids mail with a small change to GetField ( added superTypeFields() ) to return the deserialized supertypes fields. This gives subtypes the ability to check values of the supertypes persistent state. As with your original proposal, it is limited to the persistent state as read off the stream, and not the transient state, but I think it gets us most of the way there. In your original proposal, it looks quite cumbersome to hook up the static validator method in the constructor hierarchy ( as it is to do this with standard constructors too ). It also relies on the fact that the subtype has to create an instance of the supertype. I just wonder if we can push on the alternative static validator proposal to come up with something a but more attractive ( maybe not! ). ObjectInputValidation is sufficient for enforcing business rules, however it can't stop an attacker. By the time the validator runs, the attackers object of choice has been instantiated and it's game over. That's right, the attacker may choose any Serializable class from the classpath, and may even extend non serializable classes, with a zero arg constructor, such as ClassLoader. To trust deserialization of an Object, as opposed to a String or primitive, the ObjectInputStream (or overriding subclass) must limit the classes allowed to be deserialized. Agreed. I am working on fleshing out a more concrete proposal on confinement, that I mentioned in previous mails. Hopefully, I will have something in the next few days. For a class to be trusted, it must be trusted to check its parameters and enforce its invariants during object deserialization and to not deserialize with priviliged context. Validating an entire object graph's invariants, requires each class in the graph to take responsibility for validating and enforcing its own invariants. Agreed, and I think that the revised static validator method gives us this. As shown in my earlier example, intra class invariants can be enforced using Serialization constructors, from which static methods are called to check invariants prior to super class constructors being called. Yes, but it is cumbersome and easy to make mistakes. Presently, I override ObjectInputStream and use a Permission called DeserializationPermission to limit classes that can be deserialized. Untrusted connections are run from unprivileged context to limit classes that can be instantiated, while those with trusted connections are run with a Subject that allows any class to be deserialized. Interesting. I think there is overlap here with confinement. ReadSerial, could do this: Class c = rs.getType(foo); And Foo f = rs.getObject(foo, Foo.class); Which performs instanceof type check, (prior to Object deserialization, if first time deserialized, otherwise after) and generic complile time type checked method return. Thus we must restrict the classes that can be deserialized with ObjectInputStream. Have you seen the changes I proposing for failure atomicity, preliminary webrev http://cr.openjdk.java.net/~chegar/failureAtomicity/ .. and I think we can go further than this, creating the containing object lazily, if there are no readObjectXXX methods in the hierarchy. If Foo is mutable and we want a private copy, the caller needs to copy or clone it before checking invariants, as it would any mutable constructor parameter. Each level of validation is the responsibility of the component with the most knowledge. 1. Trusted class lists - administration, it might change. 2. Object invariants and intra object invariants - Classes, not objects. 3. Business rules, but not security - ObjectInputValidation. So summing up, in order to secure deserialization we must validate all data input, preferably before allowing object instantiation. Yes, where possible. If we can phase out readObjectXXX and replace it with a static validator, then I think this is possible. Once an object has been constructed an attacker can gain a reference, whether by a finalizer attack or some other cleverly crafted stream, the attacker cannot obtain a reference to an object that doesn't exist. Circular links can allow an attacker to obtain an object reference ,prior to its invariants being checked, when we rely on readObject() to throw an exception. Delaying finalizer registration won't save you. Yes, circular references are nasty. Classes with circular links should be final and use a transient boolean initialized field, that every method checks to prevent an attacker doing anything useful, should they gain a reference to an incorrectly constructed object. The real question is, do we continue to plaster over the
Re: Explicit Serialization API and Security
On 01/12/2015 05:51 AM, Chris Hegarty wrote:
On 08/01/15 22:03, David M. Lloyd wrote:
private static void validate(GetField fields) {
if (fields.getInt(lo) fields.getInt(hi)) { ... }
}
...
In fact you cannot validate invariants across multiple objects at all
using this method *or* readObject() (existing or enhanced) unless the
object in question is an enum, Class, or String (and a few other special
cases) because you can't rely on initialization order during
deserialization. That's the very reason why OIS#registerValidation()
even exists - inter-object validation is not possible until after the
root-most readObject has completed, which is the time when validations
are executed. Robust validation of a given object class may require two
stages - near validation and spanning validation - to fully
validate. However the readObject() approach and its proposed variations
(including the static validate() version) can still be useful for
fail-fast and non-complex validations; you just have to understand that
(just as today) any Object you examine might not be fully initialized
yet.
If I may, I'd like to build a little on this proposal:
1) Specify that validate is called down the hierarchy, from
j.l.Object.
2) Provide access to persistent supertype's fields, so they can
participate in the validation.
public static abstract class GetField {
/**
* Returns the persistent fields of the supertype, read from
* the stream, or null if the the supertype is not Serializable.
*/
public abstract GetField superTypeFields();
}
Maybe limited to accessible fields?
--
- DML
Re: Explicit Serialization API and Security
On 12 January 2015 at 11:37, Chris Hegarty chris.hega...@oracle.com wrote: For clarity, I would like to describe how things currently work. 1) Allocate a new instance of the deserialized type. 2) Call the first non-Serializable types no-arg constructor ( may be j.l.Object ). 3) For each type in the deserialized types hierarchy, starting with the top most ( closest to j.l.Object ), 3a) create objects representing all fields values for the type [this step is recursive and will go to 1 until all non-primitive types have been created ] 3b) [ holder for invariant validation ] 3c) assign objects to their respective members of the containing instance Just to note that for me, the problem is that (1) happens at all. IMO, serialization should be separated entirely. Objects should only be created via standard constructors or factory methods. That is the direction that the static readObjectAndResolve() was in. Talk of dedicated serialization constructors or more use of unsafe doesn't really strike me as much of a way forward, except perhaps where compatibility is concerned. Stephen
Re: Explicit Serialization API and Security
Again, thank you all for engaging in discussion of this very difficult
topic.
While we can't presently check intra object dependencies during
deserialization with readObject(), the examples I provide can do this.
ObjectInputValidation is sufficient for enforcing business rules,
however it can't stop an attacker. By the time the validator runs, the
attackers object of choice has been instantiated and it's game over.
That's right, the attacker may choose any Serializable class from the
classpath, and may even extend non serializable classes, with a zero arg
constructor, such as ClassLoader.
To trust deserialization of an Object, as opposed to a String or
primitive, the ObjectInputStream (or overriding subclass) must limit the
classes allowed to be deserialized.
For a class to be trusted, it must be trusted to check its parameters
and enforce its invariants during object deserialization and to not
deserialize with priviliged context.
Validating an entire object graph's invariants, requires each class in
the graph to take responsibility for validating and enforcing its own
invariants.
As shown in my earlier example, intra class invariants can be enforced
using Serialization constructors, from which static methods are called
to check invariants prior to super class constructors being called.
Presently, I override ObjectInputStream and use a Permission called
DeserializationPermission to limit classes that can be deserialized.
Untrusted connections are run from unprivileged context to limit classes
that can be instantiated, while those with trusted connections are run
with a Subject that allows any class to be deserialized.
ReadSerial, could do this:
Class c = rs.getType(foo);
And
Foo f = rs.getObject(foo, Foo.class);
Which performs instanceof type check, (prior to Object deserialization,
if first time deserialized, otherwise after) and generic complile time
type checked method return. Thus we must restrict the classes that can
be deserialized with ObjectInputStream.
If Foo is mutable and we want a private copy, the caller needs to copy
or clone it before checking invariants, as it would any mutable
constructor parameter.
Each level of validation is the responsibility of the component with the
most knowledge.
1. Trusted class lists - administration, it might change.
2. Object invariants and intra object invariants - Classes, not objects.
3. Business rules, but not security - ObjectInputValidation.
So summing up, in order to secure deserialization we must validate all
data input, preferably before allowing object instantiation.
Once an object has been constructed an attacker can gain a reference,
whether by a finalizer attack or some other cleverly crafted stream, the
attacker cannot obtain a reference to an object that doesn't exist.
Circular links can allow an attacker to obtain an object reference
,prior to its invariants being checked, when we rely on readObject() to
throw an exception. Delaying finalizer registration won't save you.
Classes with circular links should be final and use a transient boolean
initialized field, that every method checks to prevent an attacker
doing anything useful, should they gain a reference to an incorrectly
constructed object.
The real question is, do we continue to plaster over the issues with the
Serialization framework's api, and continue to create special cases such
as a second final field freeze after a constructor completes?To be
honest, I don't like this second final field freeze, because invariants
haven't been checked.
Don't get me wrong, Serialization is quite clever, but implementing
Serializable properly can consume a lot of time due to api complexity
and is presently a security problem.
In my examples all constructors share the same invariant checks and
because it's public API, the invariants can be tested easily with unit
tests.
It seems like we're trying to give constructor properties to a private
instance method at the expense of increasing complexity, wouldn't it be
simpler in the long term to provide a Serialization constructor?
P.S. David, I like your suggestion of using a protected method, for
limiting array size.
Thank you,
Peter.
On 01/08/2015 02:10 PM, Brian Goetz wrote:
1) Validate invariants
A clear and easy to understand mechanism that can validate the
deserialized
fields. Does not prevent the use of final fields, as the
serialization framework
will be responsible for setting them. Something along the lines
of what David
suggested:
private static void validate(GetField fields) {
if (fields.getInt(lo) fields.getInt(hi)) { ... }
}
This could be a ?special? method, or annotation driven. TBD.
Note: the validate method is static, so the object instance is
not required to
be created before running the validation.
Sort of...
This is true
Re: Explicit Serialization API and Security
On 01/08/2015 02:10 PM, Brian Goetz wrote:
1) Validate invariants
A clear and easy to understand mechanism that can validate the
deserialized
fields. Does not prevent the use of final fields, as the
serialization framework
will be responsible for setting them. Something along the lines
of what David
suggested:
private static void validate(GetField fields) {
if (fields.getInt(lo) fields.getInt(hi)) { ... }
}
This could be a “special” method, or annotation driven. TBD.
Note: the validate method is static, so the object instance is
not required to
be created before running the validation.
Sort of...
This is true if the fields participating in the invariant are
primitives. But if they're refs, what do you do? What if you want to
validate something like
count == list.size() // fields are int count, List list
? Then wouldn't GetField.getObject have to deserialize the object
referred to by that field?
In fact you cannot validate invariants across multiple objects at all
using this method *or* readObject() (existing or enhanced) unless the
object in question is an enum, Class, or String (and a few other special
cases) because you can't rely on initialization order during
deserialization. That's the very reason why OIS#registerValidation()
even exists - inter-object validation is not possible until after the
root-most readObject has completed, which is the time when validations
are executed. Robust validation of a given object class may require two
stages - near validation and spanning validation - to fully
validate. However the readObject() approach and its proposed variations
(including the static validate() version) can still be useful for
fail-fast and non-complex validations; you just have to understand that
(just as today) any Object you examine might not be fully initialized yet.
--
- DML
Re: Explicit Serialization API and Security
Thank you all for participating in this discussion.
Initially a constructor signature for deserialization was proposed to
enforce invariants and encapsulation, however there appears to be
interest in using alternative methods, although they appear to be
improvements over the status quo, I'm having trouble working out how to:
* Enforce intra class invariants with alternate proposals without
breaking encapsulation.
* How the static method proposal can be used to replace final fields
referents without needing to implement readObject().
* And how the alternative GetFields readObject() implementation can
enforce invariants and prevent finalizer attack, while also
allowing subclassing?
What started me down this path, is our project (Apache River) is heavily
dependant on Serialization, I wanted to create a secure
ObjectInputStream subclass that restricted deserialized classes to those
I had audited. My aim was to allow deserialization to enforce security
using verification, similar to how an http server verifies its input.
Unfortunately before I can achieve the goal of secure deserialization,
there's a denial of service issue in ObjectInputStream: I'd also like to
propose a system property, to allow limiting the size of arrays, created
during deserialization. Presently I can craft an inputstream to take
down the JVM, if I can do it, so can an attacker, I'd like to get that
fixed if possible.
I've attached a text file that contains three classes using the original
proposed serial constructor, A, B and C which have intra class
invariants that must be satiisfied. C also contains a circular link and
implements an interface called Circular, the Serialization framework
calls it after construction to provide access to fields with circular
links. Can the other proposals provide similar safety?
Notes:
1. The class with the circular link is final and every method checks
if invariants are satisfied.
2. The method with @SerialConstructor annotation, is the only
constructor called by the serialization frame work, other than the
Circular interface method, there are no other methods that need to
be implemented.
3. Encapsulation is preserved, classes have full control over their
invariants, their internal implementation and what's serialized.
Regards,
Peter.
// A simple complete example of safely constructed explicit Serialization.
//
// No special framework requirements, other than @CallerSensitive ReadSerial
// to preserve encapsulation.
// Object's serial form can add or remove fields without breaking encapsulation
or
// causing compatibility errors.
// When a field is not present in the stream, the default value is returned
// and the programmers invariant checking code does the rest.
// Classes can implement finalizer method if they want, no need to
// disallow it.
// Programmer only uses public api, IDE friendly.
// Class inheritance heirarchy can be changed and still all invariants can be
satisfied.
// Public api is unit test friendly.
// You can't do intra class invariant checks with readObject() without breaking
// encapsulation.
//
// It is not possible to construct an object that doesn't satisfy invariants.
//
// Easy to audit for security.
public class A implements Serializable {
private final int lower, upper;
static boolean check(int lower, int upper){
if (upper lower) throw new IllegalArgumentException(
upper limit cannot be less than lower);
return true;
}
public A(int lower, int upper){
this(check(lower,upper),lower, upper);
}
@SerialConstructor
public A(ReadSerial rs){
this(rs.getInt(lower), rs.getInt(upper));
}
private A(boolean checked, int lower, int upper){
this.lower = lower;
this.upper = upper;
}
public final int lowerLimit(){
return lower;
}
public final int upperLimit(){
return upper;
}
}
public class B extends A {
private final cur;
static boolean check(int lower, int cur, int upper){
if (cur lower || cur upper)
throw new IllegalArgumentException(cur out of bounds);
return true;
}
static boolean check(ReadSerial rs){
A a = new A(rs);
return check(a.lowerLimit(), rs.getInt(cur),
a.getUpperLimit());
}
public B(int lower, int cur, int upper){
this(check(lower, cur, upper), lower, cur, upper);
}
private B(boolean check, int lower, int cur, int upper){
super(lower, upper);
this.cur = cur;
}
@SerialConstructor
public B(ReadSerial rs){
this(check(rs), rs);
}
Re: Explicit Serialization API and Security
On 01/08/2015 02:32 AM, Peter Firmstone wrote:
Thank you all for participating in this discussion.
Initially a constructor signature for deserialization was proposed to
enforce invariants and encapsulation, however there appears to be
interest in using alternative methods, although they appear to be
improvements over the status quo, I'm having trouble working out how to:
* Enforce intra class invariants with alternate proposals without
breaking encapsulation.
* How the static method proposal can be used to replace final fields
referents without needing to implement readObject().
* And how the alternative GetFields readObject() implementation can
enforce invariants and prevent finalizer attack, while also
allowing subclassing?
What started me down this path, is our project (Apache River) is heavily
dependant on Serialization, I wanted to create a secure
ObjectInputStream subclass that restricted deserialized classes to those
I had audited. My aim was to allow deserialization to enforce security
using verification, similar to how an http server verifies its input.
Now we're getting down to brass tacks. :-)
Maybe I missed this part of the discussion, but can you elaborate on why
ObjectInputValidation is inadequate for validating intra-class
invariants? I was thinking this is really the only way to effectively
achieve intra-class validation, because (barring complex graph traversal
algorithms) there isn't really any better way to guarantee that any
given subgraph of the object graph is actually fully initialized.
As far as replacing final fields, I think the seed of the best idea came
from Chris Hegarty - the problem isn't the readObject() method, the
problem is that there's no way to initialize your final fields if you
use it. By adding a defaultReadFields() (name not important) method
like he proposes, you can validate your single class. But I don't see
any reason why this couldn't be taken another step farther, and allow
the readObject method to actually change, add, and remove fields from
its internal map before doing so. This would allow readObject to
arbitrarily transform its content, and assign to final fields, while
only minimally changing the API and implementation.
Unfortunately before I can achieve the goal of secure deserialization,
there's a denial of service issue in ObjectInputStream: I'd also like to
propose a system property, to allow limiting the size of arrays, created
during deserialization. Presently I can craft an inputstream to take
down the JVM, if I can do it, so can an attacker, I'd like to get that
fixed if possible.
I agree this would be a very good (and easy to implement) idea. You can
limit the underlying stream but that does you no good if the attacker
can cause the construction of any number of very large objects before
the end of stream is hit.
I wonder if this could be as simple as adding a protected method on
ObjectInputStream in the vein of:
protected boolean validateArrayCreation(Class? elementType, int
dimension) {
return true;
}
Or elementType could be arrayType; I don't think it matters too much.
Method returns false and InvalidObjectException is thrown.
Implementations could do a simple global max array size, different max
sizes by primitive vs ref, different by type, or even have a budget of
heap and use a combination of this method and overridden resovleClass()
to create a rough estimate of memory usage, failing at a heuristic
threshold.
I've attached a text file that contains three classes using the original
proposed serial constructor, A, B and C which have intra class
invariants that must be satiisfied. C also contains a circular link and
implements an interface called Circular, the Serialization framework
calls it after construction to provide access to fields with circular
links. Can the other proposals provide similar safety?
Notes:
1. The class with the circular link is final and every method checks
if invariants are satisfied.
2. The method with @SerialConstructor annotation, is the only
constructor called by the serialization frame work, other than the
Circular interface method, there are no other methods that need to
be implemented.
3. Encapsulation is preserved, classes have full control over their
invariants, their internal implementation and what's serialized.
I was previously firmly of the opinion that a serialization constructor
is the best way to accomplish this, but this discussion thread has at
least given me quite some doubt if not outright changed my mind, though
I think there's still an unanswered question regarding validation (above).
If nothing else, this discussion has yielded some good ideas for
enhancements to JBoss Marshalling. :-)
--
- DML
Re: Explicit Serialization API and Security
Peter, David
I would like to see how far we can push the existing Serialization mechanism,
before embarking on a road that may lead us to substantially different one.
Whether that be constructor based, to otherwise ( we know we will need a
replacement for Unsafe.allocateInstance, for third-party Serialization
libraries, in the future ).
Just to summarise the potential improvements that have been discussed so far (
I can create JIRA issues for these ):
1) Validate invariants
A clear and easy to understand mechanism that can validate the deserialized
fields. Does not prevent the use of final fields, as the serialization
framework
will be responsible for setting them. Something along the lines of what
David
suggested:
private static void validate(GetField fields) {
if (fields.getInt(lo) fields.getInt(hi)) { ... }
}
This could be a “special” method, or annotation driven. TBD.
Note: the validate method is static, so the object instance is not required
to
be created before running the validation.
2) Failure atomicity for defaultReadObject and readFields
In many cases the setting of field values can be deferred until after they
have
been validated, at a minimum that the non-primitive types are assignable.
Whole hierarchy failure atomicity in many cases ( where there are not
“special readObjectXXX methods ), or at a minimum per class failure
atomicity, can be achieved ( either all fields will be set, or contain
their default
values ).
Coupled with 1) above this could work well.
3) Finalization
It is clear that the finalization attack is an issue for deserialized
objects.
I think that a deserialized object should not be “finalizable” until after
a certain point in its construction. I would like to investigate further the
possibility of making the VM aware of the first no-args default constructor,
or j.l.Object, being called by the serialization mechanism, and possibly
treating it differently. Or maybe spin “field holder” classes for
finalizable
types being deserialized, and deserialze into them first. This doesn’t need
to be ultra-performant.
4) Confinement
A new mechanism to ensure that a deserialized field value is not shared,
and loaded by the same loader, or parent of, the object being
constructed’s loader.
This will eliminate the need to define a readObject to essentially copy
the deserialized field value, as it cannot always be trusted. And again
the issue with final fields. Something along the lines of:
class Foo {
@Confined
private final Bar bar = new BarImpl();
}
@Confined could be placed on fields, or possibly classes.
There may have been more, sorry if I missed anything.
-Chris.
On 8 Jan 2015, at 15:32, David M. Lloyd david.ll...@redhat.com wrote:
On 01/08/2015 02:32 AM, Peter Firmstone wrote:
Thank you all for participating in this discussion.
Initially a constructor signature for deserialization was proposed to
enforce invariants and encapsulation, however there appears to be
interest in using alternative methods, although they appear to be
improvements over the status quo, I'm having trouble working out how to:
* Enforce intra class invariants with alternate proposals without
breaking encapsulation.
* How the static method proposal can be used to replace final fields
referents without needing to implement readObject().
* And how the alternative GetFields readObject() implementation can
enforce invariants and prevent finalizer attack, while also
allowing subclassing?
What started me down this path, is our project (Apache River) is heavily
dependant on Serialization, I wanted to create a secure
ObjectInputStream subclass that restricted deserialized classes to those
I had audited. My aim was to allow deserialization to enforce security
using verification, similar to how an http server verifies its input.
Now we're getting down to brass tacks. :-)
Maybe I missed this part of the discussion, but can you elaborate on why
ObjectInputValidation is inadequate for validating intra-class invariants? I
was thinking this is really the only way to effectively achieve intra-class
validation, because (barring complex graph traversal algorithms) there isn't
really any better way to guarantee that any given subgraph of the object
graph is actually fully initialized.
As far as replacing final fields, I think the seed of the best idea came from
Chris Hegarty - the problem isn't the readObject() method, the problem is
that there's no way to initialize your final fields if you use it. By adding
a defaultReadFields() (name not important) method like he proposes, you can
validate your single class. But I don't see any reason why this couldn't be
taken another step farther, and allow the readObject method to actually
change, add, and
Re: Explicit Serialization API and Security
1) Validate invariants
A clear and easy to understand mechanism that can validate the deserialized
fields. Does not prevent the use of final fields, as the serialization
framework
will be responsible for setting them. Something along the lines of what
David
suggested:
private static void validate(GetField fields) {
if (fields.getInt(lo) fields.getInt(hi)) { ... }
}
This could be a “special” method, or annotation driven. TBD.
Note: the validate method is static, so the object instance is not
required to
be created before running the validation.
Sort of...
This is true if the fields participating in the invariant are
primitives. But if they're refs, what do you do? What if you want to
validate something like
count == list.size() // fields are int count, List list
? Then wouldn't GetField.getObject have to deserialize the object
referred to by that field?
Re: Explicit Serialization API and Security
On 06/01/15 17:49, Peter Levart wrote:
On 01/06/2015 06:21 PM, Chris Hegarty wrote:
On 6 Jan 2015, at 15:06, Peter Levart peter.lev...@gmail.com wrote:
On 01/06/2015 04:03 PM, Peter Levart wrote:
private void readObject(ObjectInputStream in) throws IOException,
ClassNotFoundException {
ObjectInputStream.GetField fields = in.readFields(); // this
already validates the types
Well, not true currently. But type validation could be added at this
point.
Right. I think I’ll file a bug to track this as it seems reasonable to
add type validation to readFields and defaultReadObject. So we can
probably assume/ignore it in this discussion.
I like the idea of a callback into the serialization framework to
handling the setting of final fields, after validation. I played a
little with your patch and added it to a branch in the sandbox**
So a simple example, without legacy fields, might looks as below (
without the need for writeObject or serialPersistentFields ). The
simple validating readObject is starting to look like boilerplate ?
Well, 1st and last line are always the same, yes. What's between them is
what you would have to write in a special check-only method too.
I guess what I'm getting at is, if you want just invariant checking,
then maybe something like this would be more readable:
@SerialInvariantChecker()
private static void validate(@SerialParam(name=lo,
type=Integer.class)int lo,
@SerialParam(name=hi,
type=Integer.class)int hi)
{
if (lo hi)
throw new IllegalArgumentException(lo: + lo + hi: + hi);
}
... and the serialization machinery would call this when appropriate.
-Chris
Regards, Peter
public class SimpleInterval implements Serializable {
private final int lo, hi;
private static void validate(int lo, int hi) {
// invariant
if (lo hi)
throw new IllegalArgumentException(lo: + lo + hi:
+ hi);
}
public SimpleInterval(int lo, int hi) {
validate(lo, hi);
this.lo = lo;
this.hi = hi;
}
public int getLo() { return lo; }
public int getHi() { return hi; }
private void readObject(ObjectInputStream in) throws IOException,
ClassNotFoundException {
ObjectInputStream.GetField fields = in.readFields();
// validate 'lo' and 'hi' fields invariant
int lo = fields.get(lo, 0);
int hi = fields.get(hi, 0);
validate(lo, hi);
// set current fields from read data
fields.defaultReadFields(); // this is new API!
}
}
-Chris.
** hg clone http://hg.openjdk.java.net/jdk9/sandbox sandbox
cd sandbox
sh get_source.sh
sh common/bin/hgforest.sh update -r serial-exp-branch
I also added your example, etc, under:
jdk/test/java/io/Serializable/invarientChecker
see http://cr.openjdk.java.net/~chegar/docs/sandbox.html
Re: Explicit Serialization API and Security
On 01/07/2015 03:54 PM, Chris Hegarty wrote:
On 06/01/15 17:49, Peter Levart wrote:
On 01/06/2015 06:21 PM, Chris Hegarty wrote:
On 6 Jan 2015, at 15:06, Peter Levart peter.lev...@gmail.com wrote:
On 01/06/2015 04:03 PM, Peter Levart wrote:
private void readObject(ObjectInputStream in) throws IOException,
ClassNotFoundException {
ObjectInputStream.GetField fields = in.readFields(); // this
already validates the types
Well, not true currently. But type validation could be added at this
point.
Right. I think I’ll file a bug to track this as it seems reasonable to
add type validation to readFields and defaultReadObject. So we can
probably assume/ignore it in this discussion.
I like the idea of a callback into the serialization framework to
handling the setting of final fields, after validation. I played a
little with your patch and added it to a branch in the sandbox**
So a simple example, without legacy fields, might looks as below (
without the need for writeObject or serialPersistentFields ). The
simple validating readObject is starting to look like boilerplate ?
Well, 1st and last line are always the same, yes. What's between them is
what you would have to write in a special check-only method too.
I guess what I'm getting at is, if you want just invariant checking,
then maybe something like this would be more readable:
@SerialInvariantChecker()
private static void validate(@SerialParam(name=lo,
type=Integer.class)int lo,
@SerialParam(name=hi,
type=Integer.class)int hi)
{
if (lo hi)
throw new IllegalArgumentException(lo: + lo + hi: +
hi);
}
... and the serialization machinery would call this when appropriate.
Nice.
We have method parameter names accessible via reflection since JDK8 and
types have always been, so no @SerialParam is necessary.
Peter
-Chris
Regards, Peter
public class SimpleInterval implements Serializable {
private final int lo, hi;
private static void validate(int lo, int hi) {
// invariant
if (lo hi)
throw new IllegalArgumentException(lo: + lo + hi:
+ hi);
}
public SimpleInterval(int lo, int hi) {
validate(lo, hi);
this.lo = lo;
this.hi = hi;
}
public int getLo() { return lo; }
public int getHi() { return hi; }
private void readObject(ObjectInputStream in) throws IOException,
ClassNotFoundException {
ObjectInputStream.GetField fields = in.readFields();
// validate 'lo' and 'hi' fields invariant
int lo = fields.get(lo, 0);
int hi = fields.get(hi, 0);
validate(lo, hi);
// set current fields from read data
fields.defaultReadFields(); // this is new API!
}
}
-Chris.
** hg clone http://hg.openjdk.java.net/jdk9/sandbox sandbox
cd sandbox
sh get_source.sh
sh common/bin/hgforest.sh update -r serial-exp-branch
I also added your example, etc, under:
jdk/test/java/io/Serializable/invarientChecker
see http://cr.openjdk.java.net/~chegar/docs/sandbox.html
Re: Explicit Serialization API and Security
On 01/07/2015 10:02 AM, Peter Levart wrote:
On 01/07/2015 03:54 PM, Chris Hegarty wrote:
On 06/01/15 17:49, Peter Levart wrote:
On 01/06/2015 06:21 PM, Chris Hegarty wrote:
On 6 Jan 2015, at 15:06, Peter Levart peter.lev...@gmail.com wrote:
On 01/06/2015 04:03 PM, Peter Levart wrote:
private void readObject(ObjectInputStream in) throws IOException,
ClassNotFoundException {
ObjectInputStream.GetField fields = in.readFields(); // this
already validates the types
Well, not true currently. But type validation could be added at this
point.
Right. I think I’ll file a bug to track this as it seems reasonable to
add type validation to readFields and defaultReadObject. So we can
probably assume/ignore it in this discussion.
I like the idea of a callback into the serialization framework to
handling the setting of final fields, after validation. I played a
little with your patch and added it to a branch in the sandbox**
So a simple example, without legacy fields, might looks as below (
without the need for writeObject or serialPersistentFields ). The
simple validating readObject is starting to look like boilerplate ?
Well, 1st and last line are always the same, yes. What's between them is
what you would have to write in a special check-only method too.
I guess what I'm getting at is, if you want just invariant checking,
then maybe something like this would be more readable:
@SerialInvariantChecker()
private static void validate(@SerialParam(name=lo,
type=Integer.class)int lo,
@SerialParam(name=hi,
type=Integer.class)int hi)
{
if (lo hi)
throw new IllegalArgumentException(lo: + lo + hi: +
hi);
}
... and the serialization machinery would call this when appropriate.
Nice.
We have method parameter names accessible via reflection since JDK8 and
types have always been, so no @SerialParam is necessary.
I tend to disagree, a bit. Most superficially, there's no reason to
bind serialization stream parameter names to method parameter names.
But more importantly, this implies a pretty complex implementation when
you consider we already have an API to represent the fields of a class
in a stream (GetFields) as well as a well-established convention of
using specifically-named private members for this kind of thing (in the
form of readObject/writeObject, objectStreamFields, etc.).
You could just as well support this approach, and probably with a
substantially simpler implementation:
private static void validate(GetField fields) {
if (fields.getInt(lo) fields.getInt(hi)) { ... }
}
But given these principles, I think this idea proposal was superior and
is a sort of obvious why wasn't it always this way kind of thing:
On 01/06/2015 06:21 PM, Chris Hegarty wrote:
[...]
private void readObject(ObjectInputStream in) throws IOException,
ClassNotFoundException {
ObjectInputStream.GetField fields = in.readFields();
// validate 'lo' and 'hi' fields invariant
int lo = fields.get(lo, 0);
int hi = fields.get(hi, 0);
validate(lo, hi);
// set current fields from read data
fields.defaultReadFields(); // this is new API!
}
--
- DML
Re: Explicit Serialization API and Security
On 6 Jan 2015, at 08:31, Stephen Colebourne scolebou...@joda.org wrote:
On 6 January 2015 at 07:27, Peter Levart peter.lev...@gmail.com wrote:
readObject() can be used on both ends of the spectrum or anywhere
in-between. It can be used for explicit deserialization or just for
validation of default deserialized state. Would separate validation method
give us anything more or simplify things? readObject() can be used just as:
private void readObject(ObjectInputStream in) throws IOException,
ClassNotFoundException {
in.defaultReadObject();
...
just validation
...
}
Explicit deserialization and final fields don't play nicely together
currently, yes. Sometimes data-racey-publication thread-safety is sacrificed
just because explicit deserialization would complicate code too much by
using reflection.
I've thought on a number of occasions that what I wanted from
serializable was a merger of readObject and readResolve
private Object readObjectAndResolve(ObjectInputStream in) throws IOException
This is an interesting idea.
Just so I understand, readObject is called down the inheritance hierarchy and
can read, into locals, its classes serializable fields ( of course if can
access its super types fields that are already set ), where as just a single
readResolve call is made, if it is defined by or accessible (via inheritance)
by the given class.
-Chris.
Using such a method, the input could be read into local variables, and
then a normal constructor used to create the object itself. It avoids
the final fields problem (as its just reading to local variables) and
it handles the validation (by calling a regular constructor/factory).
Its also more efficient in many cases, as a common pattern today is to
have one object instance created by readObject (or serialization
internals) and then another returned by readResolve. If
readObjectAndResolve() can't handle cyclic graphs, I can live with
that.
Stephen
Re: Explicit Serialization API and Security
On 6 January 2015 at 07:27, Peter Levart peter.lev...@gmail.com wrote:
readObject() can be used on both ends of the spectrum or anywhere
in-between. It can be used for explicit deserialization or just for
validation of default deserialized state. Would separate validation method
give us anything more or simplify things? readObject() can be used just as:
private void readObject(ObjectInputStream in) throws IOException,
ClassNotFoundException {
in.defaultReadObject();
...
just validation
...
}
Explicit deserialization and final fields don't play nicely together
currently, yes. Sometimes data-racey-publication thread-safety is sacrificed
just because explicit deserialization would complicate code too much by
using reflection.
I've thought on a number of occasions that what I wanted from
serializable was a merger of readObject and readResolve
private Object readObjectAndResolve(ObjectInputStream in) throws IOException
Using such a method, the input could be read into local variables, and
then a normal constructor used to create the object itself. It avoids
the final fields problem (as its just reading to local variables) and
it handles the validation (by calling a regular constructor/factory).
Its also more efficient in many cases, as a common pattern today is to
have one object instance created by readObject (or serialization
internals) and then another returned by readResolve. If
readObjectAndResolve() can't handle cyclic graphs, I can live with
that.
Stephen
Re: Explicit Serialization API and Security
On 6 Jan 2015, at 07:27, Peter Levart peter.lev...@gmail.com wrote:
Hi Brian,
On 01/05/2015 09:51 PM, Brian Goetz wrote:
Thanks Peter(s): I think it's just about time to re-sync on the goals, since
there are an infinitude of issues with serialization, only some of which we
can address.
To that end, let me toss out some of the results of our recent explorations
with improving serialization, where we started out on something not unlike
what Peter is suggesting, and dialed back.
One thing we can do to positively improve the serialization experience is to
reduce the set of situations where users have to write explicit readObject()
methods. In the JDK, we end up having to do so for all manner of
security-motivated reasons, and such code is hard to write correctly, hard
to validate, hard to audit, and hard to evolve.
After an audit of our codebase (thanks Chris!), we identified some of the
biggest offenders on this score:
- Validating invariants
- Ensuring confinement
The latter comes up in cases where there's a field:
class Foo {
private final Bar bar = new BarImpl();
}
which implicitly has an invariant: the Bar referenced by Foo.bar is a)
exactly of type BarImpl and not some other subclass and b) there is exactly
one reference to that BarImpl (assuming it doesn't escape Foo.)
Note that both of these still use the default deserialization; having to
override readObject() just to do some extra checking is error-prone and
messy. So I think the goal of a separate validation method which is called
automatically is a good one, since it means we can ditch some percentage of
otherwise-needless readObject implementations (which then have to jump
through hoops to set final fields.)
readObject() can be used on both ends of the spectrum or anywhere in-between.
It can be used for explicit deserialization or just for validation of default
deserialized state. Would separate validation method give us anything more or
simplify things? readObject() can be used just as:
private void readObject(ObjectInputStream in) throws IOException,
ClassNotFoundException {
in.defaultReadObject();
...
just validation
...
}
One issue with the above pattern is that defaultReadObject will read and set
the fields before the explicit validation code, ‘just validate’, is executed.
So if validation fails, throws an appropriate exception, the object may be left
in an inconsistent state.
One approach, with existing serialization, is to use the GetField API rather
than defaultReadObject. Read the field values into locals and validate before
setting. But this again falls foul for final fields.
As an aside: I have been looking at ObjectInputStream, and related classes,
recently. The current default implementation of readObject sets non-primitive
field values one at a time, without first checking that all their types are
assignable. In many cases the setting of field values can be deferred until
after they have been validated, at a minimum that the non-primitive types are
assignable. Whole hierarchy failure atomicity in many cases, or at a minimum
per class descriptor failure atomicity, can be achieved ( either all fields
will be set, or contain their default values ). Preliminary webrev [1].
From this exploration, it seems like the natural place for an 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 users 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.
To me, these benefits seems worthwhile, and justify a separate invariant
checker mechanism.
I agree that being able to play nicely with final fields is also an
important goal here.
Explicit deserialization and final fields don't play nicely together
currently, yes. Sometimes data-racey-publication thread-safety is sacrificed
just because explicit deserialization would complicate code too much by using
reflection.
Given my above comments, certainly in the case of invariant checking, we should
be able to make this experience a little better ( confining the use of
reflectively setting final fields and ensuring their safe publication in one
place, the Serialization implementation ).
-Chris.
[1] http://cr.openjdk.java.net/~chegar/failureAtomicity/
Regards, Peter
On 1/5/2015 9:11 AM, Peter Levart wrote:
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
Re: Explicit Serialization API and Security
On 6 January 2015 at 10:25, Chris Hegarty chris.hega...@oracle.com wrote: On 6 Jan 2015, at 08:31, Stephen Colebourne scolebou...@joda.org wrote: I've thought on a number of occasions that what I wanted from serializable was a merger of readObject and readResolve private Object readObjectAndResolve(ObjectInputStream in) throws IOException This is an interesting idea. Just so I understand, readObject is called down the inheritance hierarchy and can read, into locals, its classes serializable fields ( of course if can access its super types fields that are already set ), where as just a single readResolve call is made, if it is defined by or accessible (via inheritance) by the given class. I tend to work with shallow/no hierarchies so I've not thought too much about the detail. I'd imagine you'd want to have readObjectAndResolve() be a static method called only on the class being deserialized and not superclasses. The method itself would be responsible for any superclass deserialization. (Static because there is no instance to call it on and it should have no access to instance variables). It may be that the input should not be ObjectInputStream, but some simpler but related type. Stephen
Re: Explicit Serialization API and Security
- Original message -
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.
Not sure I follow. Currently you do new A(rs) which internally calls
A.check(rs) and I was suggesting you call A.check(rs) directly so
avoiding the need to create an A just for validation purposes, and which
is subsequently discarded.
David
A is also created to allow B to access its upper and lower limit fields, in
this case to ensure that the integer cur is between these limits. Intra class
invariants.
Cheers,
Peter.
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);
}
}
Re: Explicit Serialization API and Security
On 5 Jan 2015, at 20:51, Brian Goetz brian.go...@oracle.com wrote:
Thanks Peter(s): I think it's just about time to re-sync on the goals, since
there are an infinitude of issues with serialization, only some of which we
can address.
To that end, let me toss out some of the results of our recent explorations
with improving serialization, where we started out on something not unlike
what Peter is suggesting, and dialed back.
One thing we can do to positively improve the serialization experience is to
reduce the set of situations where users have to write explicit readObject()
methods. In the JDK, we end up having to do so for all manner of
security-motivated reasons, and such code is hard to write correctly, hard to
validate, hard to audit, and hard to evolve.
After an audit of our codebase (thanks Chris!), we identified some of the
biggest offenders on this score:
- Validating invariants
- Ensuring confinement
The latter comes up in cases where there's a field:
class Foo {
private final Bar bar = new BarImpl();
}
which implicitly has an invariant: the Bar referenced by Foo.bar is a)
exactly of type BarImpl and not some other subclass and b) there is exactly
one reference to that BarImpl (assuming it doesn't escape Foo.)
This type of pattern typically results in a readObject having to be defined to
copy/clone the deserialized BarImpl, and then assign Foo.bar to the new copy.
Again the issue is with setting the final field.
serialPersistentFields can help, in that it can ensure that the deserialized
BarImpl is not a back reference, and that no one else can refer to it. But then
you have to explicitly define the complete serial form of the class, and evolve
it explicitly over time. Cumbersome.
Life may be made a little easier, avoid readObject and serialPersistentFields,
if we could ensure that BarImpl was not shared, and loaded by the same class
loader as Foo ( or a parent of ). Maybe simply popping an annotation, or other
such, on the field declaration.
class Foo {
@Confined
private final Bar bar = new BarImpl();
}
Note that both of these still use the default deserialization; having to
override readObject() just to do some extra checking is error-prone and
messy. So I think the goal of a separate validation method which is called
automatically is a good one, since it means we can ditch some percentage of
otherwise-needless readObject implementations (which then have to jump
through hoops to set final fields.)
I agree that being able to play nicely with final fields is also an important
goal here.
Agreed. I don’t have fully formed ideas on how best to deal with the general
case of handling final fields, but I think that an invariant checker, and
confinement mechanism, may get us a long way. In so much as they allow us to
avoid having to explicitly set final fields in user code.
-Chris.
On 1/5/2015 9:11 AM, Peter Levart wrote:
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
Re: Explicit Serialization API and Security
On 6 January 2015 at 11:46, Chris Hegarty chris.hega...@oracle.com wrote:
With shallow/no hierarchies, and others, the serialization proxy pattern
works quite well. The overhead of course being the creation of the proxy
itself, but this is typically a minimal container with just the state needed
to create the “real” object, and no behaviour. Whatever the input would have
to be to the static factory” method, readObjectAndWriteReplace, then it
would probably have the same overhead, albeit minimal, as the proxy. Though
one less serializable type. This could work out nice, but it could not
support cyclic graphs ( which I think I could live with ), or private
superclass state ( which I think would be limiting ).
I used a shared proxy on 310. Small serialized form (short class name,
shared overhead if more than one type from same package, but no
sharing). Its fine, but quite verbose.
Oh, and to be clear, with readObjectAndResolve() you'd ban
readObject() and readResolve() from being in the same class.
On private superclass state, that state must be settable via the
constructor or setters of the superclass. Providing you control the
superclass and can change your class if the superclass changes, then I
don't see private superclass state as a problem.
private static Object readObjectAndResolve(in) {
String name = in.readStr(name)
int age = in.readInt(age)
Address addr = in.readObj(address)
return new Person(name, age, address);
}
works fine even if name is in the superclass AbstractPerson and
age/address is in Person.
Stephen
Re: Explicit Serialization API and Security
On 6 Jan 2015, at 11:13, Stephen Colebourne scolebou...@joda.org wrote: On 6 January 2015 at 10:25, Chris Hegarty chris.hega...@oracle.com wrote: On 6 Jan 2015, at 08:31, Stephen Colebourne scolebou...@joda.org wrote: I've thought on a number of occasions that what I wanted from serializable was a merger of readObject and readResolve private Object readObjectAndResolve(ObjectInputStream in) throws IOException This is an interesting idea. Just so I understand, readObject is called down the inheritance hierarchy and can read, into locals, its classes serializable fields ( of course if can access its super types fields that are already set ), where as just a single readResolve call is made, if it is defined by or accessible (via inheritance) by the given class. I tend to work with shallow/no hierarchies so I've not thought too much about the detail. I'd imagine you'd want to have readObjectAndResolve() be a static method called only on the class being deserialized and not superclasses. The method itself would be responsible for any superclass deserialization. (Static because there is no instance to call it on and it should have no access to instance variables). It may be that the input should not be ObjectInputStream, but some simpler but related type. With shallow/no hierarchies, and others, the serialization proxy pattern works quite well. The overhead of course being the creation of the proxy itself, but this is typically a minimal container with just the state needed to create the “real” object, and no behaviour. Whatever the input would have to be to the static factory” method, readObjectAndWriteReplace, then it would probably have the same overhead, albeit minimal, as the proxy. Though one less serializable type. This could work out nice, but it could not support cyclic graphs ( which I think I could live with ), or private superclass state ( which I think would be limiting ). -Chris. Stephen
Re: Explicit Serialization API and Security
Hi Chris,
On 01/06/2015 12:10 PM, Chris Hegarty wrote:
On 6 Jan 2015, at 07:27, Peter Levart peter.lev...@gmail.com wrote:
Hi Brian,
On 01/05/2015 09:51 PM, Brian Goetz wrote:
Thanks Peter(s): I think it's just about time to re-sync on the goals, since
there are an infinitude of issues with serialization, only some of which we can
address.
To that end, let me toss out some of the results of our recent explorations
with improving serialization, where we started out on something not unlike what
Peter is suggesting, and dialed back.
One thing we can do to positively improve the serialization experience is to
reduce the set of situations where users have to write explicit readObject()
methods. In the JDK, we end up having to do so for all manner of
security-motivated reasons, and such code is hard to write correctly, hard to
validate, hard to audit, and hard to evolve.
After an audit of our codebase (thanks Chris!), we identified some of the
biggest offenders on this score:
- Validating invariants
- Ensuring confinement
The latter comes up in cases where there's a field:
class Foo {
private final Bar bar = new BarImpl();
}
which implicitly has an invariant: the Bar referenced by Foo.bar is a) exactly
of type BarImpl and not some other subclass and b) there is exactly one
reference to that BarImpl (assuming it doesn't escape Foo.)
Note that both of these still use the default deserialization; having to
override readObject() just to do some extra checking is error-prone and messy.
So I think the goal of a separate validation method which is called
automatically is a good one, since it means we can ditch some percentage of
otherwise-needless readObject implementations (which then have to jump through
hoops to set final fields.)
readObject() can be used on both ends of the spectrum or anywhere in-between.
It can be used for explicit deserialization or just for validation of default
deserialized state.
Well, that in-between is actually not really available (unless
explicit (de)serialization part uses block data mode, which is not
very evolvable). But I think this could be enhanced (see below).
Would separate validation method give us anything more or simplify things?
readObject() can be used just as:
private void readObject(ObjectInputStream in) throws IOException,
ClassNotFoundException {
in.defaultReadObject();
...
just validation
...
}
One issue with the above pattern is that defaultReadObject will read and set
the fields before the explicit validation code, ‘just validate’, is executed.
So if validation fails, throws an appropriate exception, the object may be left
in an inconsistent state.
Right.
What about the following addition to
ObjectInputStream.GetField/ObjectOutputStream.PutField API. For example:
public class Interval implements Serializable {
// current fields
private final int lo, hi;
private static final ObjectStreamField[] serialPersistentFields = {
new ObjectStreamField(lo, Integer.TYPE),
new ObjectStreamField(hi, Integer.TYPE),
// legacy 'len' field
new ObjectStreamField(len, Integer.TYPE)
};
private static void validate(int lo, int hi) {
// invariant
if (lo hi)
throw new IllegalArgumentException(lo: + lo + hi: + hi);
}
public Interval(int lo, int hi) {
validate(lo, hi);
this.lo = lo;
this.hi = hi;
}
public int getLo() { return lo; }
public int getHi() { return hi; }
public int getLen() { return hi - lo; }
private void readObject(ObjectInputStream in) throws IOException,
ClassNotFoundException {
ObjectInputStream.GetField fields = in.readFields(); // this
already validates the types
// validate 'lo' and 'hi' fields invariant
int lo = fields.get(lo, 0);
int hi = fields.get(hi, 0);
validate(lo, hi);
// validate legacy 'len' field
int len = fields.get(len, hi - lo);
if (len != hi - lo) throw new IllegalArgumentException(len: +
len + is wrong);
// set current fields from read data
fields.defaultReadFields(); // this is new API!
}
private void writeObject(ObjectOutputStream out) throws IOException {
ObjectOutputStream.PutField fields = out.putFields();
// put current fields
fields.defaultPutFields(); // this is new API!
// put legacy len field
fields.put(len, hi - lo);
// write-out buffered puts
out.writeFields();
}
}
I've taken a quick look at what might be needed to do that, and it
appears very simple:
http://cr.openjdk.java.net/~plevart/misc/ObjectIOStream.GetPutFields/webrev/
One approach, with existing serialization, is to use the GetField API rather
than defaultReadObject. Read the field values into locals and validate before
setting. But this again falls foul for final fields.
The above addition to [Get|Put]Fields
Re: Explicit Serialization API and Security
On 01/06/2015 04:03 PM, Peter Levart wrote:
private void readObject(ObjectInputStream in) throws IOException,
ClassNotFoundException {
ObjectInputStream.GetField fields = in.readFields(); // this
already validates the types
Well, not true currently. But type validation could be added at this point.
Peter
Re: Explicit Serialization API and Security
On 01/06/2015 06:21 PM, Chris Hegarty wrote:
On 6 Jan 2015, at 15:06, Peter Levart peter.lev...@gmail.com wrote:
On 01/06/2015 04:03 PM, Peter Levart wrote:
private void readObject(ObjectInputStream in) throws IOException,
ClassNotFoundException {
ObjectInputStream.GetField fields = in.readFields(); // this already
validates the types
Well, not true currently. But type validation could be added at this point.
Right. I think I’ll file a bug to track this as it seems reasonable to add type
validation to readFields and defaultReadObject. So we can probably
assume/ignore it in this discussion.
I like the idea of a callback into the serialization framework to handling the
setting of final fields, after validation. I played a little with your patch
and added it to a branch in the sandbox**
So a simple example, without legacy fields, might looks as below ( without the
need for writeObject or serialPersistentFields ). The simple validating
readObject is starting to look like boilerplate ?
Well, 1st and last line are always the same, yes. What's between them is
what you would have to write in a special check-only method too.
Regards, Peter
public class SimpleInterval implements Serializable {
private final int lo, hi;
private static void validate(int lo, int hi) {
// invariant
if (lo hi)
throw new IllegalArgumentException(lo: + lo + hi: + hi);
}
public SimpleInterval(int lo, int hi) {
validate(lo, hi);
this.lo = lo;
this.hi = hi;
}
public int getLo() { return lo; }
public int getHi() { return hi; }
private void readObject(ObjectInputStream in) throws IOException,
ClassNotFoundException {
ObjectInputStream.GetField fields = in.readFields();
// validate 'lo' and 'hi' fields invariant
int lo = fields.get(lo, 0);
int hi = fields.get(hi, 0);
validate(lo, hi);
// set current fields from read data
fields.defaultReadFields(); // this is new API!
}
}
-Chris.
** hg clone http://hg.openjdk.java.net/jdk9/sandbox sandbox
cd sandbox
sh get_source.sh
sh common/bin/hgforest.sh update -r serial-exp-branch
I also added your example, etc, under:
jdk/test/java/io/Serializable/invarientChecker
see http://cr.openjdk.java.net/~chegar/docs/sandbox.html
Re: Explicit Serialization API and Security
On 6 Jan 2015, at 15:06, Peter Levart peter.lev...@gmail.com wrote:
On 01/06/2015 04:03 PM, Peter Levart wrote:
private void readObject(ObjectInputStream in) throws IOException,
ClassNotFoundException {
ObjectInputStream.GetField fields = in.readFields(); // this already
validates the types
Well, not true currently. But type validation could be added at this point.
Right. I think I’ll file a bug to track this as it seems reasonable to add type
validation to readFields and defaultReadObject. So we can probably
assume/ignore it in this discussion.
I like the idea of a callback into the serialization framework to handling the
setting of final fields, after validation. I played a little with your patch
and added it to a branch in the sandbox**
So a simple example, without legacy fields, might looks as below ( without the
need for writeObject or serialPersistentFields ). The simple validating
readObject is starting to look like boilerplate ?
public class SimpleInterval implements Serializable {
private final int lo, hi;
private static void validate(int lo, int hi) {
// invariant
if (lo hi)
throw new IllegalArgumentException(lo: + lo + hi: + hi);
}
public SimpleInterval(int lo, int hi) {
validate(lo, hi);
this.lo = lo;
this.hi = hi;
}
public int getLo() { return lo; }
public int getHi() { return hi; }
private void readObject(ObjectInputStream in) throws IOException,
ClassNotFoundException {
ObjectInputStream.GetField fields = in.readFields();
// validate 'lo' and 'hi' fields invariant
int lo = fields.get(lo, 0);
int hi = fields.get(hi, 0);
validate(lo, hi);
// set current fields from read data
fields.defaultReadFields(); // this is new API!
}
}
-Chris.
** hg clone http://hg.openjdk.java.net/jdk9/sandbox sandbox
cd sandbox
sh get_source.sh
sh common/bin/hgforest.sh update -r serial-exp-branch
I also added your example, etc, under:
jdk/test/java/io/Serializable/invarientChecker
see http://cr.openjdk.java.net/~chegar/docs/sandbox.html
Re: Explicit Serialization API and Security
Hi Brian,
On 01/05/2015 09:51 PM, Brian Goetz wrote:
Thanks Peter(s): I think it's just about time to re-sync on the goals,
since there are an infinitude of issues with serialization, only some
of which we can address.
To that end, let me toss out some of the results of our recent
explorations with improving serialization, where we started out on
something not unlike what Peter is suggesting, and dialed back.
One thing we can do to positively improve the serialization experience
is to reduce the set of situations where users have to write explicit
readObject() methods. In the JDK, we end up having to do so for all
manner of security-motivated reasons, and such code is hard to write
correctly, hard to validate, hard to audit, and hard to evolve.
After an audit of our codebase (thanks Chris!), we identified some of
the biggest offenders on this score:
- Validating invariants
- Ensuring confinement
The latter comes up in cases where there's a field:
class Foo {
private final Bar bar = new BarImpl();
}
which implicitly has an invariant: the Bar referenced by Foo.bar is a)
exactly of type BarImpl and not some other subclass and b) there is
exactly one reference to that BarImpl (assuming it doesn't escape Foo.)
Note that both of these still use the default deserialization; having
to override readObject() just to do some extra checking is error-prone
and messy. So I think the goal of a separate validation method which
is called automatically is a good one, since it means we can ditch
some percentage of otherwise-needless readObject implementations
(which then have to jump through hoops to set final fields.)
readObject() can be used on both ends of the spectrum or anywhere
in-between. It can be used for explicit deserialization or just for
validation of default deserialized state. Would separate validation
method give us anything more or simplify things? readObject() can be
used just as:
private void readObject(ObjectInputStream in) throws IOException,
ClassNotFoundException {
in.defaultReadObject();
...
just validation
...
}
I agree that being able to play nicely with final fields is also an
important goal here.
Explicit deserialization and final fields don't play nicely together
currently, yes. Sometimes data-racey-publication thread-safety is
sacrificed just because explicit deserialization would complicate code
too much by using reflection.
Regards, Peter
On 1/5/2015 9:11 AM, Peter Levart wrote:
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
Re: Explicit Serialization API and Security
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);
}
}
Re: Explicit Serialization API and Security
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
Re: Explicit Serialization API and Security
On 01/03/2015 09:36 AM, Peter Levart wrote: On 01/03/2015 01:38 PM, Peter Firmstone wrote: Hi, I would like to know what are the potential issues with simple constructor chaining where each constructor checks the invariant of its class state (with the already initialized state of superclass(es)). Finalizer attack; a subclass can override the finalize method and receive a thread of execution, even when it hasn't gotten hold of a reference during construction. It's best to prevent an object's construction, by throwing any exceptions before Object's default constructor is called. As Brian points out, this scheme can only validate intra-class invariants. It can't validate class-against-subclass state. And, the finalize() method will be called anyway - although this time with fully uninitialized instance (all fields default values). If this is safer, then alright. Is there anything in JLS that guarantees finalization for instances which fail construction or deserialization? Wouldn't it be better to register for finalization just those instances that complete their construction or deserialization normally? I'm just trying to understand why it is the way it is. Would something like this prevent Finalizer attacks? - leave finalization registration the way it is (at object allocation time). - provide internal API with which a previously registered object can be de-registered - deserialization infrastructure de-registers the instances that fail deserialization How about simply forbidding classes with finalizers from being serialized or deserialized with this mechanism? Finalizers never really work the way you want anyway. Seems a better option than essentially doubling (or more) the end-user complexity to me. -- - DML
Re: Explicit Serialization API and Security
On 01/03/2015 02:29 PM, Peter Firmstone wrote: - Original message - Wouldn't it be better to register for finalization just those instances that complete their construction or deserialization normally? I'm just trying to understand why it is the way it is. Perhaps that might be an option, someone who knows more about finalization might be able to help here. In the early days, the sandbox and bytecode verifier were intended to make java secure, additional private methods were created as vulnerabilities were better understood. I think a problem with Serialization is you have to establish trust before you can use it. It would be nice if there was an input validator like html servers use, to validate the stream before instantiating objects. Eg array size check before array creation, type check before object instantiation and restrict creation to permitted classes, to a subset of what's available on the class path. You can do this already, albeit to a more limited extent, by customizing class resolution in ObjectInputStream. Other approaches include using readResolve/writeReplace to create serialized representation objects, and the existing validation scheme where the validator is deferred until the deserialize operation is complete. -- - DML
Re: Explicit Serialization API and Security
On 01/05/2015 03:17 PM, David M. Lloyd wrote: Would something like this prevent Finalizer attacks? - leave finalization registration the way it is (at object allocation time). This was written incorrectly: after Object default constructor completes - provide internal API with which a previously registered object can be de-registered - deserialization infrastructure de-registers the instances that fail deserialization How about simply forbidding classes with finalizers from being serialized or deserialized with this mechanism? Finalizers never really work the way you want anyway. Seems a better option than essentially doubling (or more) the end-user complexity to me. This is invisible to end-user. Just internal mechanics. I thought about this for some more, which I explained in a followup post. Regards, peter
Re: Explicit Serialization API and Security
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.
Not sure I follow. Currently you do new A(rs) which internally calls
A.check(rs) and I was suggesting you call A.check(rs) directly so
avoiding the need to create an A just for validation purposes, and which
is subsequently discarded.
David
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);
}
}
Re: Explicit Serialization API and Security
Thanks Peter(s): I think it's just about time to re-sync on the goals,
since there are an infinitude of issues with serialization, only some of
which we can address.
To that end, let me toss out some of the results of our recent
explorations with improving serialization, where we started out on
something not unlike what Peter is suggesting, and dialed back.
One thing we can do to positively improve the serialization experience
is to reduce the set of situations where users have to write explicit
readObject() methods. In the JDK, we end up having to do so for all
manner of security-motivated reasons, and such code is hard to write
correctly, hard to validate, hard to audit, and hard to evolve.
After an audit of our codebase (thanks Chris!), we identified some of
the biggest offenders on this score:
- Validating invariants
- Ensuring confinement
The latter comes up in cases where there's a field:
class Foo {
private final Bar bar = new BarImpl();
}
which implicitly has an invariant: the Bar referenced by Foo.bar is a)
exactly of type BarImpl and not some other subclass and b) there is
exactly one reference to that BarImpl (assuming it doesn't escape Foo.)
Note that both of these still use the default deserialization; having to
override readObject() just to do some extra checking is error-prone and
messy. So I think the goal of a separate validation method which is
called automatically is a good one, since it means we can ditch some
percentage of otherwise-needless readObject implementations (which then
have to jump through hoops to set final fields.)
I agree that being able to play nicely with final fields is also an
important goal here.
On 1/5/2015 9:11 AM, Peter Levart wrote:
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
Re: Explicit Serialization API and Security
- Original message -
On 01/04/2015 02:48 AM, Peter Firmstone wrote:
class B extends A {
public final int cur;
private static ReadSerial check(ReadSerial rs) {
A a = new A(rs);
What to do if A is abstract?
:)
Create an anonymous instance, or create a static private unshared class that
overrides it, for the purpose of validation and discard it.
The important thing is A must retain control of access to its internal state.
Cheers,
Peter.
Regards, Peter
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);
}
}
Re: Explicit Serialization API and Security
On 01/04/2015 02:48 AM, Peter Firmstone wrote:
class B extends A {
public final int cur;
private static ReadSerial check(ReadSerial rs) {
A a = new A(rs);
What to do if A is abstract?
Regards, Peter
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);
}
}
Re: Explicit Serialization API and Security
On 3/01/2015 9:24 PM, Peter Firmstone wrote:
Thanks Brian,
Those are good questions, some thoughts and examples inline:
- Original message -
Overall the direction seems promising. Poking at it a bit...
- ReadSerial methods are caller-sensitive and only show a class a view
of its own fields.
- Invariant checking is separate from deserialization, and does not
seem entirely built-in -- subclass constructors seem responsible for
asking parents to do validity-checking?
My mistake, the last example wasn't quite right, BaseFoo, should call it's own
static check method from within it's constructor, rather than from SubFoo.
Each class in the heirarchy needs to check it's invarients using a static
method, before calling a superclass constructor, this prevents object
construction if an invarient isn't satisfied, because the object isn't created
an attacker cannot hold a reference to it.
By the time Object's default constructor is called, even though no fields have
been set, invarients have already been checked for every member class in the
Object.
An attacker may choose to call another constructor and not pass on ReadSerial,
so all constructors need to perform these checks.
- 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
}
To check such an invariant, the serialization library would have to
construct the object (in a potentially bad state), invoke the checker at
each layer, and then fail deserialization if any checker said no.
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);
}
}
But,
an evil checker could still squirrel away a reference under the carpet.
Not with the above example, if an invarient isn't satisfied, an object instance
of B cannot be created.
But circular links are still a challenge...
Another challenge in invariant checking is circular data structures. If
you have two objects:
class Brother {
final Brother brother;
}
that refer to each other, an invariant you might want to check after
deserialization is that
this.brother.brother == this
Obviously you have to patch one or the other instance after construction
to retain the circular references; at what point do you do invariant
checking?
There are two cases here I think:
One where there is a trust relationship between two classes and the circular
relationship is known at compilation time. At least one will provide a mutable
setter method, or a Constructor that accepts an argument with a matching
argument type, and the other invokes it during construction, I say trusted
because 'this' is allowed to escape.
In Brother's, because the brother field is final, this was constructed with a
trust relationship, because 'this' escapes from at least one Brother during
construction. I would argue that the object that allows this to escape should
also be responsible for rebuilding the circular relation during deserialisation.
However the Brother that didn't let this escape could be serialised first, in
any case both are instances of Brother, so either instance could be a subclass,
each can correctly create the other.
public class Brother {
private static Constructor getBroConstructor(ReadSerial rs){
// we can check our subclass has permission here if we like,
//
Re: Explicit Serialization API and Security
Hi,
I would like to know what are the potential issues with simple
constructor chaining where each constructor checks the invariant of its
class state (with the already initialized state of superclass(es)).
Finalizer attack; a subclass can override the finalize method and
receive a thread of execution, even when it hasn't gotten hold of a reference
during construction. It's best to prevent an object's construction, by
throwing any exceptions before Object's default constructor is called.
A class can override the finalize method and make it final, but than can cause
objects to hang around longer than necessary.
It's worth noting that all classes that implement Serializable have a default
no arg constructor, so presently don't have the option of preventing object
construction and typically have to clone mutable referents after
deserialization. Final fields only provide protection for immutable referent
objects.
So this offers an improvement, at least for objects that don't have circular
links and it would allow them to retain compatibility with their existing
serial form.
Cheers,
Peter.
For
example, the above would be written as:
class A {
final int lower, upper;
A(ReadSerial rs) {
int l = rs.getInt(lower);
int u = rs.getInt(upper);
if (l u) throw new IllegalArgumentException();
lower = l;
upper = u;
}
}
class B extends A {
final int cur;
B(ReadSerial rs) {
super(rs);
int c = rs.getInt(cur);
if (c lower || c upper) throw new
IllegalArgumentException(); cur = c;
}
}
Another challenge in invariant checking is circular data structures.
If you have two objects:
class Brother {
final Brother brother;
}
that refer to each other, an invariant you might want to check after
deserialization is that
this.brother.brother == this
This is tricky, yes. In general, the graph of objects has to be linked
somehow gradually where links are established to half-initialized
objects. So there would have to be a special post-deserialization
call-back over the deserialized objects just for checking the
invariants. But what to do with the instances that escape during the
process? This is why explicit (de)serialization is tricky - it allows
arbitrary user code to be executed during the construction of the object
graph.
Perhaps we just need some kind of declarative prescription of serialized
state and mapping to fields of objects in the form of annotations or
such (see what JPA is doing in this field). Combined with
post-deserialization invariant-checking call-back method perhaps.
Regards, Peter
Obviously you have to patch one or the other instance after
construction to retain the circular references; at what point do you
do invariant checking?
On 1/1/2015 7:43 AM, Peter Firmstone wrote:
Subclass example:
class SubFoo extends BaseFoo {
public static ReadSerial check(ReadSerial rs){
if (rs.getInt(y) 128) throw Exception(too big);
return rs;
}
private final int y;
public SubFoo( int x , int y) {
super(x);
this.y = y;
}
public SubFoo( ReadSerial rs ){
super(BaseFoo.check(check(rs)));
// SubFoo can't get at BaseFoo's rs.getInt(x),
// it's visible only to BaseFoo. Instead SubFoo would get
// the default int value of 0. Just in case both classes have
// private fields named x.
// ReadSerial is caller sensitive.
this.y = rs.getInt(y);
}
}
Classes in the heirarchy can provide a static method that throws an
exception to check invarients while preventing a finaliser attack.
We'd want to check invarients for other constructors also, but for
berevity...
Eg:
class BaseFoo implements Serializable{
public static ReadSerial check(ReadSerial rs) throws Exception
{
if (rs.getInt(x) 1)
throw IllegalArgumentException(message);
return rs;
}
Sent from my Nokia N900.
- Original message -
So, if I understand this correctly, the way this would get used is:
class BaseFoo implements Serializable {
private final int x;
public BaseFoo(ReadSerial rs) {
this(rs.getInt(x));
}
public BaseFoo(int x) {
this.x = x;
}
}
Right?
What happens with subclasses? I think then I need an extra RS arg
in my
constructors:
class SubFoo extends BaseFoo {
private final int y;
public SubFoo(ReadSerial rs) {
this(rs.getInt(y));
}
public BaseFoo(ReadSerial rs, int y) {
super(rs);
this.y = y;
}
}
Is this what you envision?
On 12/27/2014 8:03 PM, Peter Firmstone wrote:
Is there any interest in developing an explicit API for
Re: Explicit Serialization API and Security
On 01/02/2015 11:53 PM, Brian Goetz wrote:
Overall the direction seems promising. Poking at it a bit...
- ReadSerial methods are caller-sensitive and only show a class a
view of its own fields.
- Invariant checking is separate from deserialization, and does not
seem entirely built-in -- subclass constructors seem responsible for
asking parents to do validity-checking?
- 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
}
To check such an invariant, the serialization library would have to
construct the object (in a potentially bad state), invoke the checker
at each layer, and then fail deserialization if any checker said no.
But, an evil checker could still squirrel away a reference under the
carpet.
Hi,
I would like to know what are the potential issues with simple
constructor chaining where each constructor checks the invariant of its
class state (with the already initialized state of superclass(es)). For
example, the above would be written as:
class A {
final int lower, upper;
A(ReadSerial rs) {
int l = rs.getInt(lower);
int u = rs.getInt(upper);
if (l u) throw new IllegalArgumentException();
lower = l;
upper = u;
}
}
class B extends A {
final int cur;
B(ReadSerial rs) {
super(rs);
int c = rs.getInt(cur);
if (c lower || c upper) throw new IllegalArgumentException();
cur = c;
}
}
Another challenge in invariant checking is circular data structures.
If you have two objects:
class Brother {
final Brother brother;
}
that refer to each other, an invariant you might want to check after
deserialization is that
this.brother.brother == this
This is tricky, yes. In general, the graph of objects has to be linked
somehow gradually where links are established to half-initialized
objects. So there would have to be a special post-deserialization
call-back over the deserialized objects just for checking the
invariants. But what to do with the instances that escape during the
process? This is why explicit (de)serialization is tricky - it allows
arbitrary user code to be executed during the construction of the object
graph.
Perhaps we just need some kind of declarative prescription of serialized
state and mapping to fields of objects in the form of annotations or
such (see what JPA is doing in this field). Combined with
post-deserialization invariant-checking call-back method perhaps.
Regards, Peter
Obviously you have to patch one or the other instance after
construction to retain the circular references; at what point do you
do invariant checking?
On 1/1/2015 7:43 AM, Peter Firmstone wrote:
Subclass example:
class SubFoo extends BaseFoo {
public static ReadSerial check(ReadSerial rs){
if (rs.getInt(y) 128) throw Exception(too big);
return rs;
}
private final int y;
public SubFoo( int x , int y) {
super(x);
this.y = y;
}
public SubFoo( ReadSerial rs ){
super(BaseFoo.check(check(rs)));
// SubFoo can't get at BaseFoo's rs.getInt(x),
// it's visible only to BaseFoo. Instead SubFoo would get
// the default int value of 0. Just in case both classes have
// private fields named x.
// ReadSerial is caller sensitive.
this.y = rs.getInt(y);
}
}
Classes in the heirarchy can provide a static method that throws an
exception to check invarients while preventing a finaliser attack. We'd
want to check invarients for other constructors also, but for
berevity...
Eg:
class BaseFoo implements Serializable{
public static ReadSerial check(ReadSerial rs) throws Exception
{
if (rs.getInt(x) 1)
throw IllegalArgumentException(message);
return rs;
}
Sent from my Nokia N900.
- Original message -
So, if I understand this correctly, the way this would get used is:
class BaseFoo implements Serializable {
private final int x;
public BaseFoo(ReadSerial rs) {
this(rs.getInt(x));
}
public BaseFoo(int x) {
this.x = x;
}
}
Right?
What happens with subclasses? I think then I need an extra RS arg
in my
constructors:
class SubFoo extends BaseFoo {
private final int y;
public SubFoo(ReadSerial rs) {
this(rs.getInt(y));
}
public BaseFoo(ReadSerial rs, int y) {
super(rs);
this.y = y;
}
}
Is this what you envision?
On 12/27/2014 8:03 PM, Peter Firmstone wrote:
Is there any interest in developing an explicit API for
Serialization?:
1. Use a public constructor signature with a single argument,
ReadSerialParameters (read only, writable only by the
serialization framework) to recreate objects,
Re: Explicit Serialization API and Security
- Original message -
As Brian points out, this scheme can only validate intra-class
invariants. It can't validate class-against-subclass state.
Did he say that?
It's true that a superclass can't validate subclass state, it can't be expected
to know much about it, but it can validate its own and prevent construction.
It can prevent extension too, ie be final, or require a permission to be
extended.
And, the
finalize() method will be called anyway
The jvm makes no gaurantee a finalizer will run.
- although this time with fully
uninitialized instance (all fields default values). If this is safer,
then alright.
Is there anything in JLS that guarantees finalization for instances
which fail construction or deserialization?
Not that I'm aware of.
Wouldn't it be better to
register for finalization just those instances that complete their
construction or deserialization normally? I'm just trying to understand
why it is the way it is.
Perhaps that might be an option, someone who knows more about finalization
might be able to help here.
In the early days, the sandbox and bytecode verifier were intended to make java
secure, additional private methods were created as vulnerabilities were better
understood.
I think a problem with Serialization is you have to establish trust before you
can use it.
It would be nice if there was an input validator like html servers use, to
validate the stream before instantiating objects. Eg array size check before
array creation, type check before object instantiation and restrict creation to
permitted classes, to a subset of what's available on the class path.
Techniques that prevent invalid object creation, are the most secure.
Circular links between untrusted classes still appear to require at least one
object to be created before invarients can be checked and at least one mutated
after.
Another option might be to require a Permission for Circular links, such that a
thread may be run with a Subject that allows it for trusted connections and
disallows it for untrusted serial data.
Regards,
Peter.
Would something like this prevent Finalizer attacks?
- leave finalization registration the way it is (at object allocation
time). - provide internal API with which a previously registered object
can be de-registered
- deserialization infrastructure de-registers the instances that fail
deserialization
An alternative is to make an object unusable if it fails invarient checks, so
all methods throw an exception.
Regards, Peter
A class can override the finalize method and make it final, but than
can cause objects to hang around longer than necessary.
It's worth noting that all classes that implement Serializable have a
default no arg constructor, so presently don't have the option of
preventing object construction and typically have to clone mutable
referents after deserialization. Final fields only provide protection
for immutable referent objects.
So this offers an improvement, at least for objects that don't have
circular links and it would allow them to retain compatibility with
their existing serial form.
Cheers,
Peter.
For
example, the above would be written as:
class A {
final int lower, upper;
A(ReadSerial rs) {
int l = rs.getInt(lower);
int u = rs.getInt(upper);
if (l u) throw new IllegalArgumentException();
lower = l;
upper = u;
}
}
class B extends A {
final int cur;
B(ReadSerial rs) {
super(rs);
int c = rs.getInt(cur);
if (c lower || c upper) throw new
IllegalArgumentException(); cur = c;
}
}
Another challenge in invariant checking is circular data
structures. If you have two objects:
class Brother {
final Brother brother;
}
that refer to each other, an invariant you might want to check
after deserialization is that
this.brother.brother == this
This is tricky, yes. In general, the graph of objects has to be
linked somehow gradually where links are established to
half-initialized objects. So there would have to be a special
post-deserialization call-back over the deserialized objects just
for checking the invariants. But what to do with the instances that
escape during the process? This is why explicit (de)serialization
is tricky - it allows arbitrary user code to be executed during the
construction of the
object
graph.
Perhaps we just need some kind of declarative prescription of
serialized
state and mapping to fields of objects in the form of annotations or
such (see what JPA is doing in this field). Combined with
post-deserialization invariant-checking call-back method perhaps.
Regards, Peter.
Re: Explicit Serialization API and Security
Just a quick comment about the finalization aspect ( as I have been thinking about this too ). On 3 Jan 2015, at 15:36, Peter Levart peter.lev...@gmail.com wrote: On 01/03/2015 01:38 PM, Peter Firmstone wrote: Hi, I would like to know what are the potential issues with simple constructor chaining where each constructor checks the invariant of its class state (with the already initialized state of superclass(es)). Finalizer attack; a subclass can override the finalize method and receive a thread of execution, even when it hasn't gotten hold of a reference during construction. It's best to prevent an object's construction, by throwing any exceptions before Object's default constructor is called. As Brian points out, this scheme can only validate intra-class invariants. It can't validate class-against-subclass state. And, the finalize() method will be called anyway - although this time with fully uninitialized instance (all fields default values). If this is safer, then alright. In many cases default values will be safer, but not always, e.g. port number of ‘0’ implies ephemeral port. Is there anything in JLS that guarantees finalization for instances which fail construction or deserialisation? 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).” [1] It is unfortunate that the way default serialization constructs objects, the j.l.Object constructor is invoked before attempting to set any fields. Wouldn't it be better to register for finalization just those instances that complete their construction or deserialization normally? I'm just trying to understand why it is the way it is. Would something like this prevent Finalizer attacks? - leave finalization registration the way it is (at object allocation time). - provide internal API with which a previously registered object can be de-registered - deserialization infrastructure de-registers the instances that fail deserialisation I have given this some thought too, and also think that deserialized objects should not become finalizable until after the fields have been successfully set. Whether this be an internal API, JLS change, or an extension to the “magic” serialization mechanism to invoke the first non-serializable classes no-args constructor, I’m not sure. -Chris. [1] http://docs.oracle.com/javase/specs/jls/se7/html/jls-12.html#jls-12.6.1 Regards, Peter
Re: Explicit Serialization API and Security
Thanks Brian,
Those are good questions, some thoughts and examples inline:
- Original message -
Overall the direction seems promising. Poking at it a bit...
- ReadSerial methods are caller-sensitive and only show a class a view
of its own fields.
- Invariant checking is separate from deserialization, and does not
seem entirely built-in -- subclass constructors seem responsible for
asking parents to do validity-checking?
My mistake, the last example wasn't quite right, BaseFoo, should call it's own
static check method from within it's constructor, rather than from SubFoo.
Each class in the heirarchy needs to check it's invarients using a static
method, before calling a superclass constructor, this prevents object
construction if an invarient isn't satisfied, because the object isn't created
an attacker cannot hold a reference to it.
By the time Object's default constructor is called, even though no fields have
been set, invarients have already been checked for every member class in the
Object.
An attacker may choose to call another constructor and not pass on ReadSerial,
so all constructors need to perform these checks.
- 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
}
To check such an invariant, the serialization library would have to
construct the object (in a potentially bad state), invoke the checker at
each layer, and then fail deserialization if any checker said no.
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);
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);
}
}
But,
an evil checker could still squirrel away a reference under the carpet.
Not with the above example, if an invarient isn't satisfied, an object instance
of B cannot be created.
But circular links are still a challenge...
Another challenge in invariant checking is circular data structures. If
you have two objects:
class Brother {
final Brother brother;
}
that refer to each other, an invariant you might want to check after
deserialization is that
this.brother.brother == this
Obviously you have to patch one or the other instance after construction
to retain the circular references; at what point do you do invariant
checking?
There are two cases here I think:
One where there is a trust relationship between two classes and the circular
relationship is known at compilation time. At least one will provide a mutable
setter method, or a Constructor that accepts an argument with a matching
argument type, and the other invokes it during construction, I say trusted
because 'this' is allowed to escape.
In Brother's, because the brother field is final, this was constructed with a
trust relationship, because 'this' escapes from at least one Brother during
construction. I would argue that the object that allows this to escape should
also be responsible for rebuilding the circular relation during deserialisation.
However the Brother that didn't let this escape could be serialised first, in
any case both are instances of Brother, so either instance could be a subclass,
each can correctly create the other.
public class Brother {
private static Constructor getBroConstructor(ReadSerial rs){
// we can check our subclass has permission here if we like,
// even though an object hasn't been created, its class is
// in the current execution context.
Class type = rs.getType(brother);
if ( !Brother.class.isAssignableFrom(type))
throw new Exception(not my bro);
Re: Explicit Serialization API and Security
On 01/03/2015 01:38 PM, Peter Firmstone wrote:
Hi,
I would like to know what are the potential issues with simple
constructor chaining where each constructor checks the invariant of its
class state (with the already initialized state of superclass(es)).
Finalizer attack; a subclass can override the finalize method and
receive a thread of execution, even when it hasn't gotten hold of a
reference during construction. It's best to prevent an object's
construction, by throwing any exceptions before Object's default
constructor is called.
As Brian points out, this scheme can only validate intra-class
invariants. It can't validate class-against-subclass state. And, the
finalize() method will be called anyway - although this time with fully
uninitialized instance (all fields default values). If this is safer,
then alright.
Is there anything in JLS that guarantees finalization for instances
which fail construction or deserialization? Wouldn't it be better to
register for finalization just those instances that complete their
construction or deserialization normally? I'm just trying to understand
why it is the way it is.
Would something like this prevent Finalizer attacks?
- leave finalization registration the way it is (at object allocation time).
- provide internal API with which a previously registered object can be
de-registered
- deserialization infrastructure de-registers the instances that fail
deserialization
Regards, Peter
A class can override the finalize method and make it final, but than
can cause objects to hang around longer than necessary.
It's worth noting that all classes that implement Serializable have a
default no arg constructor, so presently don't have the option of
preventing object construction and typically have to clone mutable
referents after deserialization. Final fields only provide protection
for immutable referent objects.
So this offers an improvement, at least for objects that don't have
circular links and it would allow them to retain compatibility with
their existing serial form.
Cheers,
Peter.
For
example, the above would be written as:
class A {
final int lower, upper;
A(ReadSerial rs) {
int l = rs.getInt(lower);
int u = rs.getInt(upper);
if (l u) throw new IllegalArgumentException();
lower = l;
upper = u;
}
}
class B extends A {
final int cur;
B(ReadSerial rs) {
super(rs);
int c = rs.getInt(cur);
if (c lower || c upper) throw new
IllegalArgumentException();cur = c;
}
}
Another challenge in invariant checking is circular data structures.
If you have two objects:
class Brother {
final Brother brother;
}
that refer to each other, an invariant you might want to check after
deserialization is that
this.brother.brother == this
This is tricky, yes. In general, the graph of objects has to be linked
somehow gradually where links are established to half-initialized
objects. So there would have to be a special post-deserialization
call-back over the deserialized objects just for checking the
invariants. But what to do with the instances that escape during the
process? This is why explicit (de)serialization is tricky - it allows
arbitrary user code to be executed during the construction of the
object
graph.
Perhaps we just need some kind of declarative prescription of
serialized
state and mapping to fields of objects in the form of annotations or
such (see what JPA is doing in this field). Combined with
post-deserialization invariant-checking call-back method perhaps.
Regards, Peter
Obviously you have to patch one or the other instance after
construction to retain the circular references; at what point do you
do invariant checking?
On 1/1/2015 7:43 AM, Peter Firmstone wrote:
Subclass example:
class SubFoo extends BaseFoo {
public static ReadSerial check(ReadSerial rs){
if (rs.getInt(y) 128) throw Exception(too big);
return rs;
}
private final int y;
public SubFoo( int x , int y) {
super(x);
this.y = y;
}
public SubFoo( ReadSerial rs ){
super(BaseFoo.check(check(rs)));
// SubFoo can't get at BaseFoo's rs.getInt(x),
// it's visible only to BaseFoo. Instead SubFoo would get
// the default int value of 0. Just in case both classes have
// private fields named x.
// ReadSerial is caller sensitive.
this.y = rs.getInt(y);
}
}
Classes in the heirarchy can provide a static method that throws an
exception to check invarients while preventing a finaliser attack.
We'd want to check invarients for other constructors also, but for
berevity...
Eg:
class BaseFoo implements Serializable{
public static ReadSerial check(ReadSerial rs) throws Exception
{
if
Re: Explicit Serialization API and Security
P.S. Thanks for engaging this difficult subject. It's worth remembering the finalizer attack isn't the only issue, a subclass will have a reference after construction completes, it has a thread of execution and if the superclass hasn't checked invarients, because circular links haven't been wired up, it's game over. For explicit serialization, this is only a problem for circular relationships. A permission check for extension can be performed before Object's constructor is called, or the class can be final. That leaves the case of determining which of two objects is trusted to pass a reference before invarients have been checked. One of the objects in the circular relationship could be the attacker and we just gave it a reference before checking invarients. Once created, an object has little control over references to it. So it would seem that circular links would only be allowed on trusted serial data, for network connections, that means an authenticated, secure connection. So a class that implements Circular would require a permission check to be performed after the Circular object is created, but before passing a reference that wires up a circular link. So it becomes a case of: 1. Do I trust my subclass? 2. Do I trust the framework? 3 Am I trustworthy? 4. Can I trust classes loaded after my constructor completes, to receive my reference, even though I haven't checked invarients and to honour my wishes and delete my reference if I find invarients aren't satisfied? Peter. - Original message - Just a quick comment about the finalization aspect ( as I have been thinking about this too ). On 3 Jan 2015, at 15:36, Peter Levart peter.lev...@gmail.com wrote: On 01/03/2015 01:38 PM, Peter Firmstone wrote: Hi, I would like to know what are the potential issues with simple constructor chaining where each constructor checks the invariant of its class state (with the already initialized state of superclass(es)). Finalizer attack; a subclass can override the finalize method and receive a thread of execution, even when it hasn't gotten hold of a reference during construction. It's best to prevent an object's construction, by throwing any exceptions before Object's default constructor is called. As Brian points out, this scheme can only validate intra-class invariants. It can't validate class-against-subclass state. And, the finalize() method will be called anyway - although this time with fully uninitialized instance (all fields default values). If this is safer, then alright. In many cases default values will be safer, but not always, e.g. port number of ‘0’ implies ephemeral port. Is there anything in JLS that guarantees finalization for instances which fail construction or deserialisation? 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).” [1] It is unfortunate that the way default serialization constructs objects, the j.l.Object constructor is invoked before attempting to set any fields. Wouldn't it be better to register for finalization just those instances that complete their construction or deserialization normally? I'm just trying to understand why it is the way it is. Would something like this prevent Finalizer attacks? - leave finalization registration the way it is (at object allocation time). - provide internal API with which a previously registered object can be de-registered - deserialization infrastructure de-registers the instances that fail deserialisation I have given this some thought too, and also think that deserialized objects should not become finalizable until after the fields have been successfully set. Whether this be an internal API, JLS change, or an extension to the “magic” serialization mechanism to invoke the first non-serializable classes no-args constructor, I’m not sure. -Chris. [1] http://docs.oracle.com/javase/specs/jls/se7/html/jls-12.html#jls-12.6.1 Regards, Peter
Re: Explicit Serialization API and Security
On 01/03/2015 09:29 PM, Peter Firmstone wrote: - Original message - As Brian points out, this scheme can only validate intra-class invariants. It can't validate class-against-subclass state. Sorry, I meant it can't validate class -against-superclass state. Did he say that? Not directly - his example was pointing that out. Regards, Peter
Re: Explicit Serialization API and Security
On 4/01/2015 9:55 AM, Peter Levart wrote:
- Original message -
As Brian points out, this scheme can only validate intra-class
invariants. It can't validate class-against-subclass state.
Sorry, I meant it can't validate class -against-superclass state.
Did he say that?
Not directly - his example was pointing that out.
Ah, I think you missed my example :), Brian was more or less asking a
question, but
the answer is definitely yes.
In my example below, subclass B's constructor creates an unextended
instance of A, from within B's static check method.
It then uses this copy of A to check A's field against B's. At this
point, note an instance of B
hasn't been constructed, it can still throw an exception, without B ever
being constructed, also you'll note
that when an unextended copy of A was constructed, A checked its
invariants too.
B having satisfied all invariants can now be safely constructed. The
unextended copy of A is discarded without ever being shared.
- 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
}
To check such an invariant, the serialization library would have to
construct the object (in a potentially bad state), invoke the checker at
each layer, and then fail deserialization if any checker said no.?
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);
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);
}
}
Cheers,
Peter.
On 4/01/2015 9:55 AM, Peter Levart wrote:
Regards, Peter
Re: Explicit Serialization API and Security
Overall the direction seems promising. Poking at it a bit...
- ReadSerial methods are caller-sensitive and only show a class a view
of its own fields.
- Invariant checking is separate from deserialization, and does not
seem entirely built-in -- subclass constructors seem responsible for
asking parents to do validity-checking?
- 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
}
To check such an invariant, the serialization library would have to
construct the object (in a potentially bad state), invoke the checker at
each layer, and then fail deserialization if any checker said no. But,
an evil checker could still squirrel away a reference under the carpet.
Another challenge in invariant checking is circular data structures. If
you have two objects:
class Brother {
final Brother brother;
}
that refer to each other, an invariant you might want to check after
deserialization is that
this.brother.brother == this
Obviously you have to patch one or the other instance after construction
to retain the circular references; at what point do you do invariant
checking?
On 1/1/2015 7:43 AM, Peter Firmstone wrote:
Subclass example:
class SubFoo extends BaseFoo {
public static ReadSerial check(ReadSerial rs){
if (rs.getInt(y) 128) throw Exception(too big);
return rs;
}
private final int y;
public SubFoo( int x , int y) {
super(x);
this.y = y;
}
public SubFoo( ReadSerial rs ){
super(BaseFoo.check(check(rs)));
// SubFoo can't get at BaseFoo's rs.getInt(x),
// it's visible only to BaseFoo. Instead SubFoo would get
// the default int value of 0. Just in case both classes have
// private fields named x.
// ReadSerial is caller sensitive.
this.y = rs.getInt(y);
}
}
Classes in the heirarchy can provide a static method that throws an
exception to check invarients while preventing a finaliser attack. We'd
want to check invarients for other constructors also, but for berevity...
Eg:
class BaseFoo implements Serializable{
public static ReadSerial check(ReadSerial rs) throws Exception
{
if (rs.getInt(x) 1)
throw IllegalArgumentException(message);
return rs;
}
Sent from my Nokia N900.
- Original message -
So, if I understand this correctly, the way this would get used is:
class BaseFoo implements Serializable {
private final int x;
public BaseFoo(ReadSerial rs) {
this(rs.getInt(x));
}
public BaseFoo(int x) {
this.x = x;
}
}
Right?
What happens with subclasses? I think then I need an extra RS arg in my
constructors:
class SubFoo extends BaseFoo {
private final int y;
public SubFoo(ReadSerial rs) {
this(rs.getInt(y));
}
public BaseFoo(ReadSerial rs, int y) {
super(rs);
this.y = y;
}
}
Is this what you envision?
On 12/27/2014 8:03 PM, Peter Firmstone wrote:
Is there any interest in developing an explicit API for
Serialization?:
1. Use a public constructor signature with a single argument,
ReadSerialParameters (read only, writable only by the
serialization framework) to recreate objects, subclasses (when
permitted) call this first from their own constructor, they have
an identical constructor signature. ReadSerialParameters that are
null may contain a circular reference and will be available after
construction, see #3 below.
2. Use a factory method (defined by an interface) with one parameter,
WriteSerialParameters (write only, readable only by the
serialization framework), this method can be overridden by
subclasses (when permitted)
3. For circular links, a public method (defined by an interface) that
accepts one argument, ReadSerialParameters, this method is called
after the constructor completes, subclasses overriding this should
call the superclass method. If this method is not called, an
implementation, if known to possibly contain circular links,
should check it has been fully initialized in each object method
called.
4. Retains compatibility with current serialization stream format.
5. Each serial field has a name, calling class and object reference,
similar to explicitly declaring private static final
ObjectStreamField[] serialPersistentFields .
Benefits:
1. An object's internal form is not publicised.
2. Each class in an object's heirarchy can use a static method to
check invarients and throw an exception, prior to
java.lang.Object's constructor being called, preventing
construction and avoiding finalizer attacks.
3. Final field friendly.
4. Compatible with existing serial form.
5. Flexible serial
Re: Explicit Serialization API and Security
Subclass example:
class SubFoo extends BaseFoo {
public static ReadSerial check(ReadSerial rs){
if (rs.getInt(y) 128) throw Exception(too big);
return rs;
}
private final int y;
public SubFoo( int x , int y) {
super(x);
this.y = y;
}
public SubFoo( ReadSerial rs ){
super(BaseFoo.check(check(rs)));
// SubFoo can't get at BaseFoo's rs.getInt(x),
// it's visible only to BaseFoo. Instead SubFoo would get
// the default int value of 0. Just in case both classes have
// private fields named x.
// ReadSerial is caller sensitive.
this.y = rs.getInt(y);
}
}
Classes in the heirarchy can provide a static method that throws an exception
to check invarients while preventing a finaliser attack. We'd want to check
invarients for other constructors also, but for berevity...
Eg:
class BaseFoo implements Serializable{
public static ReadSerial check(ReadSerial rs) throws Exception
{
if (rs.getInt(x) 1)
throw IllegalArgumentException(message);
return rs;
}
Sent from my Nokia N900.
- Original message -
So, if I understand this correctly, the way this would get used is:
class BaseFoo implements Serializable {
private final int x;
public BaseFoo(ReadSerial rs) {
this(rs.getInt(x));
}
public BaseFoo(int x) {
this.x = x;
}
}
Right?
What happens with subclasses? I think then I need an extra RS arg in my
constructors:
class SubFoo extends BaseFoo {
private final int y;
public SubFoo(ReadSerial rs) {
this(rs.getInt(y));
}
public BaseFoo(ReadSerial rs, int y) {
super(rs);
this.y = y;
}
}
Is this what you envision?
On 12/27/2014 8:03 PM, Peter Firmstone wrote:
Is there any interest in developing an explicit API for Serialization?:
1. Use a public constructor signature with a single argument,
ReadSerialParameters (read only, writable only by the
serialization framework) to recreate objects, subclasses (when
permitted) call this first from their own constructor, they have
an identical constructor signature. ReadSerialParameters that are
null may contain a circular reference and will be available after
construction, see #3 below.
2. Use a factory method (defined by an interface) with one parameter,
WriteSerialParameters (write only, readable only by the
serialization framework), this method can be overridden by
subclasses (when permitted)
3. For circular links, a public method (defined by an interface) that
accepts one argument, ReadSerialParameters, this method is called
after the constructor completes, subclasses overriding this should
call the superclass method. If this method is not called, an
implementation, if known to possibly contain circular links,
should check it has been fully initialized in each object method
called.
4. Retains compatibility with current serialization stream format.
5. Each serial field has a name, calling class and object reference,
similar to explicitly declaring private static final
ObjectStreamField[] serialPersistentFields .
Benefits:
1. An object's internal form is not publicised.
2. Each class in an object's heirarchy can use a static method to
check invarients and throw an exception, prior to
java.lang.Object's constructor being called, preventing
construction and avoiding finalizer attacks.
3. Final field friendly.
4. Compatible with existing serial form.
5. Flexible serial form evolution.
6. All methods are public and explicitly defined.
7. All class ProtectionDomain's exist in the current execution
context, allowing an object to throw a SecurityException before
construction.
8. Less susceptible to deserialization attacks.
Problems:
1. Implementations cannot be package private or private. Implicit
serialization publicises internal form, any thoughts?
Recommendations:
1. Create a security check in the serialization framework for
implicit serialization, allowing administrators to reduce their
deserialization attack surface.
2. For improved security, disallow classes implementing explicit
serialization from having static state and static initializer
blocks, only allow static methods, this would require complier and
verifier changes.
3. Alternative to #2, allow final static fields, but don't allow
static initializer blocks or mutable static fields, similar to
interfaces.
Penny for your thoughts?
Regards,
Peter Firmstone.
Re: Explicit Serialization API and Security
Not quite, the constructor signature is the same for super and child classes.
ReadSerial is a container for each serialized Object in an ObjectInputStream
that provides and controlls access to serial fields, identified by name, type
and calling class, such that each class has it's own parameters contained
within ReadSerial.
Serial parameters, might be better terminology than serial fields. Serial
parameters, retrieved from ReadSerial are compatible with serial fields in the
serialization specification, providing a compatible upgrade path, and enabling
all classes belonging to an Object to obtain parameters during construction,
whilst allowing implementations to be independant of an object's serial form.
When ReadSerial doesn't contain a parameter, it will return null.
// pseudocode
public abstract class ReadSerial {
protected ReadSerial(){
// check permission
}
// perform caller class dependant permission and type check
public T T getParameter( ClassT type, String name );
}
Peter.
Sent from my Nokia N900.
- Original message -
So, if I understand this correctly, the way this would get used is:
class BaseFoo implements Serializable {
private final int x;
public BaseFoo(ReadSerial rs) {
this(rs.getInt(x));
}
public BaseFoo(int x) {
this.x = x;
}
}
Right?
What happens with subclasses? I think then I need an extra RS arg in my
constructors:
class SubFoo extends BaseFoo {
private final int y;
public SubFoo(ReadSerial rs) {
this(rs.getInt(y));
}
public BaseFoo(ReadSerial rs, int y) {
super(rs);
this.y = y;
}
}
Is this what you envision?
On 12/27/2014 8:03 PM, Peter Firmstone wrote:
Is there any interest in developing an explicit API for Serialization?:
1. Use a public constructor signature with a single argument,
ReadSerialParameters (read only, writable only by the
serialization framework) to recreate objects, subclasses (when
permitted) call this first from their own constructor, they have
an identical constructor signature. ReadSerialParameters that are
null may contain a circular reference and will be available after
construction, see #3 below.
2. Use a factory method (defined by an interface) with one parameter,
WriteSerialParameters (write only, readable only by the
serialization framework), this method can be overridden by
subclasses (when permitted)
3. For circular links, a public method (defined by an interface) that
accepts one argument, ReadSerialParameters, this method is called
after the constructor completes, subclasses overriding this should
call the superclass method. If this method is not called, an
implementation, if known to possibly contain circular links,
should check it has been fully initialized in each object method
called.
4. Retains compatibility with current serialization stream format.
5. Each serial field has a name, calling class and object reference,
similar to explicitly declaring private static final
ObjectStreamField[] serialPersistentFields .
Benefits:
1. An object's internal form is not publicised.
2. Each class in an object's heirarchy can use a static method to
check invarients and throw an exception, prior to
java.lang.Object's constructor being called, preventing
construction and avoiding finalizer attacks.
3. Final field friendly.
4. Compatible with existing serial form.
5. Flexible serial form evolution.
6. All methods are public and explicitly defined.
7. All class ProtectionDomain's exist in the current execution
context, allowing an object to throw a SecurityException before
construction.
8. Less susceptible to deserialization attacks.
Problems:
1. Implementations cannot be package private or private. Implicit
serialization publicises internal form, any thoughts?
Recommendations:
1. Create a security check in the serialization framework for
implicit serialization, allowing administrators to reduce their
deserialization attack surface.
2. For improved security, disallow classes implementing explicit
serialization from having static state and static initializer
blocks, only allow static methods, this would require complier and
verifier changes.
3. Alternative to #2, allow final static fields, but don't allow
static initializer blocks or mutable static fields, similar to
interfaces.
Penny for your thoughts?
Regards,
Peter Firmstone.
Re: Explicit Serialization API and Security
Just to note that there is some overlap between improving serialization and the meta-model for Java (Beans v2.0) work I'm looking at [1][2]. The key tool that a meta-model provides is to enable class authors to select which pieces of their state form the published properties. In most cases, the properties are the same data as that needed for effective serialization. The Joda-Beans project [3] does this and provides XML, JSON and binary serialization via properties. thanks Stephen [1] https://groups.google.com/forum/#!forum/beans-2-meta-model [2] https://github.com/jodastephen/property-alliance [3] http://www.joda.org/joda-beans/ On 28 December 2014 at 01:03, Peter Firmstone peter.firmst...@zeus.net.au wrote: Is there any interest in developing an explicit API for Serialization?: 1. Use a public constructor signature with a single argument, ReadSerialParameters (read only, writable only by the serialization framework) to recreate objects, subclasses (when permitted) call this first from their own constructor, they have an identical constructor signature. ReadSerialParameters that are null may contain a circular reference and will be available after construction, see #3 below. 2. Use a factory method (defined by an interface) with one parameter, WriteSerialParameters (write only, readable only by the serialization framework), this method can be overridden by subclasses (when permitted) 3. For circular links, a public method (defined by an interface) that accepts one argument, ReadSerialParameters, this method is called after the constructor completes, subclasses overriding this should call the superclass method. If this method is not called, an implementation, if known to possibly contain circular links, should check it has been fully initialized in each object method called. 4. Retains compatibility with current serialization stream format. 5. Each serial field has a name, calling class and object reference, similar to explicitly declaring private static final ObjectStreamField[] serialPersistentFields . Benefits: 1. An object's internal form is not publicised. 2. Each class in an object's heirarchy can use a static method to check invarients and throw an exception, prior to java.lang.Object's constructor being called, preventing construction and avoiding finalizer attacks. 3. Final field friendly. 4. Compatible with existing serial form. 5. Flexible serial form evolution. 6. All methods are public and explicitly defined. 7. All class ProtectionDomain's exist in the current execution context, allowing an object to throw a SecurityException before construction. 8. Less susceptible to deserialization attacks. Problems: 1. Implementations cannot be package private or private. Implicit serialization publicises internal form, any thoughts? Recommendations: 1. Create a security check in the serialization framework for implicit serialization, allowing administrators to reduce their deserialization attack surface. 2. For improved security, disallow classes implementing explicit serialization from having static state and static initializer blocks, only allow static methods, this would require complier and verifier changes. 3. Alternative to #2, allow final static fields, but don't allow static initializer blocks or mutable static fields, similar to interfaces. Penny for your thoughts? Regards, Peter Firmstone.
Re: Explicit Serialization API and Security
So, if I understand this correctly, the way this would get used is:
class BaseFoo implements Serializable {
private final int x;
public BaseFoo(ReadSerial rs) {
this(rs.getInt(x));
}
public BaseFoo(int x) {
this.x = x;
}
}
Right?
What happens with subclasses? I think then I need an extra RS arg in my
constructors:
class SubFoo extends BaseFoo {
private final int y;
public SubFoo(ReadSerial rs) {
this(rs.getInt(y));
}
public BaseFoo(ReadSerial rs, int y) {
super(rs);
this.y = y;
}
}
Is this what you envision?
On 12/27/2014 8:03 PM, Peter Firmstone wrote:
Is there any interest in developing an explicit API for Serialization?:
1. Use a public constructor signature with a single argument,
ReadSerialParameters (read only, writable only by the
serialization framework) to recreate objects, subclasses (when
permitted) call this first from their own constructor, they have
an identical constructor signature. ReadSerialParameters that are
null may contain a circular reference and will be available after
construction, see #3 below.
2. Use a factory method (defined by an interface) with one parameter,
WriteSerialParameters (write only, readable only by the
serialization framework), this method can be overridden by
subclasses (when permitted)
3. For circular links, a public method (defined by an interface) that
accepts one argument, ReadSerialParameters, this method is called
after the constructor completes, subclasses overriding this should
call the superclass method. If this method is not called, an
implementation, if known to possibly contain circular links,
should check it has been fully initialized in each object method
called.
4. Retains compatibility with current serialization stream format.
5. Each serial field has a name, calling class and object reference,
similar to explicitly declaring private static final
ObjectStreamField[] serialPersistentFields .
Benefits:
1. An object's internal form is not publicised.
2. Each class in an object's heirarchy can use a static method to
check invarients and throw an exception, prior to
java.lang.Object's constructor being called, preventing
construction and avoiding finalizer attacks.
3. Final field friendly.
4. Compatible with existing serial form.
5. Flexible serial form evolution.
6. All methods are public and explicitly defined.
7. All class ProtectionDomain's exist in the current execution
context, allowing an object to throw a SecurityException before
construction.
8. Less susceptible to deserialization attacks.
Problems:
1. Implementations cannot be package private or private. Implicit
serialization publicises internal form, any thoughts?
Recommendations:
1. Create a security check in the serialization framework for
implicit serialization, allowing administrators to reduce their
deserialization attack surface.
2. For improved security, disallow classes implementing explicit
serialization from having static state and static initializer
blocks, only allow static methods, this would require complier and
verifier changes.
3. Alternative to #2, allow final static fields, but don't allow
static initializer blocks or mutable static fields, similar to
interfaces.
Penny for your thoughts?
Regards,
Peter Firmstone.
