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

[Roger Riggs]

On Wed, 24 Nov 2021 15:39:13 GMT, Roger Riggs <rriggs at openjdk.org> wrote:

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

The use of `ExceptionInInitializerError` can be completely replaced for  invalid values of `jdk.serialFilter` and `jdk.serialFilterFactory` with:

- If either property is supplied and is invalid; deserialization is disabled by:
- `OIF.Config.getSerialFilter()` and `OIF.Config.setSerialFilter(...)` throw IllegalStateException with the exception message thrown from `Config.createFilter(pattern)`
- `OIF.Config.getSerialFilterFactory()` and `OIF.Config.setSerialFilterFactory(...)` throw IllegalStateException with the exception message from the attempt to construct the filter factory.
- Both `new ObjectInputStream(...)` constructors call both `OIF.Config.getSerialFilter()` and `OIF.Config.getSerialFilterFactory()` and so will throw the appropriate `IllegalStateException` for invalid values of the properties.
- The static initialization of `OIF.Config` does not throw any exceptions (so no `ExceptionInInitializerError`) but hold the state so that the method above can throw `IllegalStateException` with the informative message.
- The `IllegalStateException`s will be thrown just slightly later than previously, now after the `Config` class is initialized instead of during initialization.
- The javadoc of the `Config` class will replace the descriptions of the previous error with descriptions of `ISE` and each of the methods mentioned above will have an added `IllegalStateException` documented referring to the property values.

Its not strictly compatible with the previous behavior but occurs only in the case of badly formed parameters on the command line.

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

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