RFR: 8277322: Document that setting an invalid property `jdk.serialFilter` disables deserialization

[Roger Riggs]

On Tue, 23 Nov 2021 23:07:08 GMT, Stuart Marks <smarks at openjdk.org> wrote:

>> 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.

This is about the second line of defense; what happens when the developer deliberately ignores the first error.
If the command line parameters are invalid it might be an option to call `System.exit(1)` but there
is no precedent for that and it seems undesirable.

Any and all deserialization is only possible after the command line or security properties, if any, are successfully applied.
In the examples above, the constructors for `ObjectInputStream` do not succeed if either the serial filter or filter factory are not valid.  The builtin filter factory applies the default filter regardless of the setting of an `ObjectInputFilter` set on the stream.  The only way to completely control the filter combinations is to provide
a valid filter factory on the command line; but that is not the case raising the issue here.

The initialization of both could be re-specified and re-implemented to allow the initialization of `Config` to
complete successfully but defer throwing an exception (or Error) until either filter or filter factory
was requested from `Config.getSerialFilter` or `Config.getSerialFilterFactory`.
Both of them are called from the `ObjectInputStream` constructors. 
At present, it is incompletely specified and implemented to throw `IllegalStateException` for `getSerialFilterFactory`.

The `NCDFE` is a more reliable backstop against misuse from any source, including reflection, than the alternative.

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

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