Return-Path: X-Original-To: apmail-river-dev-archive@www.apache.org Delivered-To: apmail-river-dev-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 871CF10DE0 for ; Sat, 21 Feb 2015 03:15:43 +0000 (UTC) Received: (qmail 29082 invoked by uid 500); 21 Feb 2015 03:15:43 -0000 Delivered-To: apmail-river-dev-archive@river.apache.org Received: (qmail 29058 invoked by uid 500); 21 Feb 2015 03:15:43 -0000 Mailing-List: contact dev-help@river.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@river.apache.org Delivered-To: mailing list dev@river.apache.org Received: (qmail 29047 invoked by uid 99); 21 Feb 2015 03:15:43 -0000 Received: from nike.apache.org (HELO nike.apache.org) (192.87.106.230) by apache.org (qpsmtpd/0.29) with ESMTP; Sat, 21 Feb 2015 03:15:43 +0000 X-ASF-Spam-Status: No, hits=2.2 required=5.0 tests=HTML_MESSAGE,SPF_PASS X-Spam-Check-By: apache.org Received-SPF: pass (nike.apache.org: local policy) Received: from [207.57.65.70] (HELO zeus.net.au) (207.57.65.70) by apache.org (qpsmtpd/0.29) with ESMTP; Sat, 21 Feb 2015 03:15:16 +0000 Received: (qmail 61601 invoked by uid 16710); 21 Feb 2015 03:15:10 -0000 Received: from unknown (HELO [10.67.208.54]) ([49.182.15.64]) (envelope-sender ) by 207.57.65.70 (qmail-ldap-1.03) with SMTP for ; 21 Feb 2015 03:15:10 -0000 From: Peter Reply-To: Peter To: dev@river.apache.org, =?UTF-8?Q?Micha=C5=82_K=C5=82eczek?= "(XPro)" Subject: Re: Security X-Mailer: Modest 3.2 References: <1423059065.78312.ezmlm@river.apache.org> <39C0176A-2005-4FBD-A0D6-F44878A53539@gmail.com> <54D71A45.5090104@zeus.net.au> <54D8AB4C.4010806@zeus.net.au> <54E481B5.90805@zeus.net.au> <54E65BB9.5020709@xpro.biz> <54E65DFA.4090400@xpro.biz> In-Reply-To: <54E65DFA.4090400@xpro.biz> Content-Type: multipart/alternative; boundary="=-xY9gC2UEL9ZzZXjKbdGk" Date: Sat, 21 Feb 2015 13:15:03 +1000 Message-Id: <1424488503.3755.5.camel@Nokia-N900> Mime-Version: 1.0 X-Virus-Checked: Checked by ClamAV on apache.org --=-xY9gC2UEL9ZzZXjKbdGk Content-Type: text/plain; charset=utf-8 Content-ID: <1424488502.3755.2.camel@Nokia-N900> Content-Transfer-Encoding: 8bit Thanks for your interest. It doesn't, but if there's no downloaded code, or there's no unprivileged code in the execution context, then the code is privileged, or is run with the privileges of the users Subject. The return argument to a remote call doesn't run with a domain representing the subject of the remote caller. Remote method invocation does so, only for parameter arguments. In any case ObjectInputStream is an easy target for dos, with or without downloaded code. I've decided not to investigate Serialization security any further, at least for the River project. While it is possible to secure Serialization, the obstacles aren't technical; there is little interest and already some objections. A suitable solution wouldn't inconvenience users of untrusted networks in any way whatsoever, but that's a moot point I have participated in some discussions on core-libs-dev, there are some changes in the pipeline to make minor improvements to Serialization's susceptibility to dos, but there's an enormous amount of legacy code the will continue to cause problems. I think River could potentially simplify it's service model, if this project were to only target trusted networks with some limited capacity for untrusted networks. Proxy trust adds significant complexity, a common complaint, it almost succeded, but it's not quite right. Secure Discovery V2 can be used for connections over untrusted networks, but only where the lookup service only allows trusted clients and services to connect and all parties in a djinn group trust each other. This makes me wonder if proxy trust can be dropped altogether and if secure discovery V2 and secure authenticated connections can be relied upon completely for security. When the participating members agree on defining some goals, we can look at how we can better align our security api's with those goals. My original interest in River was for untrusted networks. Regards, Peter. ----- Original message ----- > BTW - I'm really interested in the reasoning why deserialization code > does not call the non-serializable superclass constructor in the > security context of the subclass(es) - so that it really mimics the > normal constructor call chain. > > Michal > > Michał Kłeczek (XPro) wrote: > > Isn't the issue with non-serializable superclass constructor call this > > one? : > > http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2008-5353 > > > > If so - I don't really see how it relates to River - to be able to > > expoit this kind of vulnerability an attacker must have already > > downloaded and run his code - otherwise the exploiting subclass could > > not have been loaded hence no construction would take place. > > I think if we can make sure only trusted (whatever that means :) ) code > > is ever run - we don't have to do anything more with (de)serialization > > - except making DoS more difficult by restricting size of the stream. > > > > Thanks, > > Michal > > > > Peter Firmstone wrote: > > > Continuing on ... > > > > > > Lets say for example, we have a secure OS and we provide a service on > > > a public port and we have a determined attacker attempting to use > > > deserialization to take over our system or bring it to its knees with > > > denial of service. > > > > > > We know this is relatively easy with standard ObjectInputStream. > > > > > > When we invoke a remote method, the return value is what you call a > > > root object, that is it's the root of a serialized object tree, > > > originating through the root object, via its fields. > > > > > > Most Serializable objects have invariants. > > > > > > We can consider the java serialization stream format as a string of > > > commands that allows creation of any object, bypassing all levels of > > > visibility.  That is an attacker can create package private classes > > > or private internal classes, provided they implement Serializable. > > > So think of Serializable as a public constructor that lacks a context > > > representing the attacker.  That's right, there is no context > > > representing the remote endpoint in the thread call stack. > > > > > > Now we can place restrictions on the stream, such as a requirement > > > that the stream is reset before a limit on the number of objects > > > deserialized is reached.  We can also place a limit on the count of > > > all array elements read in from the stream, until the stream is > > > reset.  These measures ensure the stream cannot go on forever, they > > > also prevent stack overflow and out of memory errors.  At some point > > > the caller will regain control of the stream without running out of > > > resources. > > > > > > But getting back to the previous problem, the attacker has command > > > line access to create any Serializable object. > > > > > > Now most objects have invariants that should be satisfied, for > > > correct functional state, but a major problem with Serialization is > > > it creates an instance, using the first zero arg constructor of the > > > first non serializable superclass.  Yep that's right, that could be > > > ClassLoader, you clever attacker you!  The poor old object hasn't > > > even had a chance to check it's invariants and it's game over, > > > that's not fair! > > > > > > This will never do, I had to create a new contract for atomic object > > > construction: > > > > > > 1. The class must implement Serializable (can be inherited) AND one > > > of the following conditions must be met: > > > 2. The class must be annotated with @AtomicSerial OR > > > 3. The class must be stateless and can be created by calling > > > class.newInstance() OR > > > 4. The class must have DeSerializationPermission > > > > > > Now if the class is annotated with @AtomicSerial, it must also have a > > > constructor signature that has public or default visibility: > > > > > > SomeClass(GetArg arg) throws IOException{ > > > // The first thing I must do is to call a static method that > > > checks invariants, before calling a superclass constructor, the > > > static method should return the argument for another constructor. > > > } > > > > > > Some simple rules for our object input stream: > > > > > > 1. Though shalt not publish a reference to a partially constructed > > > object. > > > 2. Though shalt not publish a reference if an object fails > > > construction, where readObject, readObjectNoData and readResolve > > > methods are considered to be constructors for the purpose of > > > deserialization of conventional Serializable objects. > > > 3. Though shalt not attempt to construct a Serializable object that > > > doesn't have an @AtomicSerial annotation, or has serial fields > > > (state) and doesn't have DeSerializationPermission. > > > 4. Only call a protected constructor if the class doesn't implement > > > @AtomicSerial and has DeSerializationPermission. > > > 5. Do not honour the standard java Serialization construction > > > contract (not all Serializable classes can be constructed even if > > > they have DeSerializationPermission). > > > 6. If a standard Serializable class has DeSerializationPermission for > > > it to be constructed it must have a zero arg constructor or a > > > constructor that accepts null object arguments and default > > > primitive values. > > > 7. If an any object in a serialized object graph fails it's invariant > > > checks, deserialization of the object graph fails at that point > > > and control is returned to the caller, by way of an > > > InvalidObjectException (a child class of IOException). > > > 8. Honour the standard java serialization stream protocol. > > > 9. Count number of objects received and throw a > > > StreamCorruptedException if limit is exceeded. > > > 10. Count number of array elements received and throw > > > StreamCorruptedException if limit is exceeded. > > > 11. readResolve() can be called on @AtomicSerial instances but, > > > readObject() is never called and neither is readObjectNoData(). > > > > > > Obligations of our object output stream: > > > > > > 1. Reset the stream before the limit is reached. > > > 2. Replace java collections and maps with safe @AtomicSerial > > > implementations, it is the developers obligation to replace them > > > with their preferred implementations during construction, these > > > are functional but are only immutable containers for keys, values > > > and comparators. > > > 3. Honour java's serial stream protocol. > > > > > > Some simple rules for developers implementing @AtomicSerial: > > > > > > 1. Check all invariants from a static method called by your class > > > constructor prior to calling a superclass constructor. > > > 2. Check types of values in collections and maps and keys in maps. > > > 3. Check field types. > > > 4. Copy or clone arrays and collections. > > > 5. Do not call a super class constructor until all invariants have > > > been checked. > > > 6. Handle failed invariants by throwing an InvalidObjectException, > > > again make sure you did not call the super class constructor. > > > 7. Implement the interface ReadObject if you want to duplicate > > > existing readObject method functionality, this will allow you to > > > communicate with the stream prior to calling a super class > > > constructor, so you can check invariants.  Annotate a static > > > method that returns an instance of ReadObject with @ReadInput. > > > 8. Your ReadObject instance can be retrieved by from GetArg, your > > > ReadObject instance will have read the stream prior to your > > > constructor having been called. > > > 9. Don't call defaultReadObject() from your ReadObject > > > implementation! 10. Check all fields can be retrieved, prior to > > > calling a super class constructor. > > > 11. Beware of the implicit super class constructor. > > > 12. Enum, and Proxy instances are considered secure, don't bother > > > subclassing proxy, your class won't be deserialized. > > > 13. InvocationHandler's need to implement @AtomicSerial. > > > > > > Note an object instance is not created until the default constructor > > > in java.lang.Object is called. > > > > > > Golden rules: > > > > > > 1. Do not create arrays or collections blindly by reading in an > > > integer or long, length or size from the stream to pass as an > > > argument to an array or collection constructor. > > > 2. Let the stream be responsible for array creation. > > > 3. Clone or copy arrays and collections. > > > 4. Clone or copy mutable objects, these may be shared by other > > > objects in the stream. > > > 5. Don't grant DeSerializationPermission unless you're really really > > > sure you know what your doing, you must have access to the source > > > code of the object you want to deserialize, you must make sure it > > > is secure, if in doubt ask first. > > > > > > Now before anyone goes off thinking "Oh no I don't need this > > > functionality, do I have to use it?"  The answer is no, it will be > > > set via configuration constraints.  If an endpoint doesn't support > > > AtomicValidation, it will prevent ObjectInputStream creation.  If an > > > object in the stream doesn't support it, deserialization will fail > > > and control will be returned to the caller. > > > > > > Other than configuration, this is invisible to clients, only service > > > api parameters and return values and private service classes, such as > > > smart proxy's and proxy verifiers need implement it. > > > > > > Note: I have tested both Reggie and Outrigger, standard serial form > > > is preserved for all objects, all tests pass with > > > AtomicValidation.YES enabled. > > > > > > Clients will be able to be configured with InvocationConstraints; > > > AtomicValidation.YES, Integrity.YES and Confidentiality.YES. > > > > > > The client can also require DownloadPermission by specifying either > > > of the properties: > > > > > > net.jini.loader.ClassLoading.provider=net.jini.loader.pref.RequireDlPermProvider > > > > > > > > > OR > > > > > > java.rmi.server.RMIClassLoaderSpi=net.jini.loader.pref.RequireDlPermProvider > > > > > > > > > If the first property is specified, it isn't required to be loaded by > > > the system ClassLoader. > > > > > > The next step is to provide a service interface for a bootstrap proxy > > > to return a smart proxy and to define a suitable Entry for the > > > bootstrap proxy to declare the interfaces it's smart proxy > > > implements. > > > > > > Clients can then lookup bootstrap proxy, based on the Entry, > > > authenticate the bootstrap proxy and grant it DownloadPermission > > > dynamically.  The client will then need to prepare the smart proxy, > > > placing constraints on it. > > > > > > While the bootstrap proxy should have an AtomicVerification.YES > > > constraint, the endpoint for the smart proxy doesn't have to, so > > > after trust has been established, it is possible to run with standard > > > serialization.  In this case the smart proxy itself must be > > > serializable using @AtomicSerial, but objects serialized over the > > > smart proxy's endpoint's need not.  I've also have made it possible > > > for proxy codebases to declare the permissions they require so these > > > can be granted dynamically after trust is established. > > > > > > This would also provide the client with delayed unmarshalling > > > functionality, while preserving the standard ServiceRegistrar > > > interface, which can be managed using ServiceDiscoveryManager, and > > > ServiceItemFilter. > > > > > > In other words, this additional functionality requires minimal effort > > > on behalf of the developer, while those who don't need it can remain > > > blissfully ignorant and implement it later if they want to. > > > > > > I haven't uploaded to svn, would you like to see some patches? > > > > > > Regards, > > > > > > Peter. --=-xY9gC2UEL9ZzZXjKbdGk--