On Wed, 24 Nov 2021 15:39:13 GMT, Roger Riggs <rri...@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
