On Tue, 23 Nov 2021 21:44:17 GMT, Roger Riggs <rri...@openjdk.org> wrote:

>> src/java.base/share/classes/java/io/ObjectInputFilter.java line 529:
>> 
>>> 527:      * if the filter string is invalid, an {@link 
>>> ExceptionInInitializerError} is thrown
>>> 528:      * and the initialization fails; subsequent attempts to use the 
>>> configuration or
>>> 529:      * serialization will fail with an implementation specific 
>>> exception.
>> 
>> I'm confused about exactly what happens after `ExceptionInInitializerError`.
>> 
>>> Subsequent attempts to use the configuration or serialization will fail....
>> 
>> Which configuration? I thought OIF.Config is a utility class and thus has no 
>> instances. If its class initialization fails, then other code cannot use 
>> `Config.setSerialFilter` to set a global filter (which might be desirable, 
>> but throws NCDFE instead of `IllegalStateException`) and other code can't 
>> use `Config.createFilter` to create individual filters. Is that right? It 
>> seems like there ought to be a better arrangement than to have the system 
>> come up in some dysfunctional way, where any subsequent reference to 
>> `OIF.Config` results in NCDFE.
>> 
>> And surely this affects deserialization, not serialization?
>
> Most configurations of `jdk.serialFilter` and` jdk.serialFilterFactory` will 
> be valid.
> If they are not valid, the cause must be clear and useful suggestion made to 
> correct the command line
> or security properties.
> 
> It must not be possible to do any deserialization if the command line (or 
> security) properties are invalid.
> Since the states are held in static fields of `ObjectInputFilter.Config`, the 
> initialization is done in a static initializer.
> Exceptions/errors in the static initializer cause 
> `ExceptionInInitializerError` to be thrown.
> In most cases, that will be sufficient to report the invalid properties and 
> the program will be terminated.
> The message on the error should be sufficient correct the filter
> 
> However, there is a chance that `ExceptionInInitializerError` will be caught 
> and ignored.
> Ignoring the exception should not allow deserialization.
> At the moment, this comes for free because when the initialization of 
> `ObjectInputFilter.Config`
> fails any subsequent reference to the class causes `NoClassDefFoundError`.
> Neither calls to `ObjectInputFilter.Config` get or set methods will succeed.
> Deserialization is prevented because `ObjectInputStream` constructors call 
> `Config.getSerialFilterFactorySingleton` which will fail with 
> `NoClassDefFoundError`.
> 
> The question that has been raised is how much of that should be included in 
> the specification
> of `ObjectInputFilter.Config`.  The CSR proposes that after 
> `ExceptionInInitializerError` is
> thrown, it is specified that an implementation specific exception/error is 
> thrown when attempting to use `Config`.
> There is no additional value in being specific about the error that is thrown.

If the intent is to disable serialization entirely, then this state should be 
represented explicitly. Having things throw `NoClassDefFoundError` looks like a 
mistake and a bug that needs to be fixed. In addition, it requires that all 
code paths to deserialization use `OIF.Config` in order to provoke the NCDFE. 
This might be true today, but under maintenance a different code path might be 
introduced that happens not to use that class, allowing deserialization to 
proceed.

An alternative policy might be to disallow any deserialization that would use 
the default filter. This could be accomplished by having a "fail-safe" or 
"fallback" filter that always returns REJECT. This would be at least as 
restrictive and thus no less safe than any valid filter that could be 
installed. In addition, it would allow things that don't use the global filter 
to proceed, such as,

    var ois = new ObjectInputStream(...);
    ois.setObjectInputFilter(new ObjectInputFilter() { ... });

or

    var ois = new ObjectInputStream(...);
    ois.setObjectInputFilter(ObjectInputFilter.Config.createFilter(...));

There could be a reasonable policy discussion about whether it's preferable to 
disable all deserialization or just deserialization that depends on the 
(invalidly specified) global filter.

-------------

PR: https://git.openjdk.java.net/jdk/pull/6508

Reply via email to