Re: RFR: 8264859: Implement Context-Specific Deserialization Filters [v7]
On Tue, 25 May 2021 11:18:15 GMT, Chris Hegarty wrote:
>> Roger Riggs has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Move merge and rejectUndecidedClass methods to OIF.Config
>> As default methods on OIF, their implementations were not concrete and not
>> trustable
>
> src/java.base/share/classes/java/io/ObjectInputFilter.java line 400:
>
>> 398: * {@link BinaryOperator {@literal
>> BinaryOperator}} interface, provide its implementation and
>> 399: * be accessible via the {@linkplain
>> ClassLoader#getSystemClassLoader() application class loader}.
>> 400: * The filter factory configured using the system or security
>> property during initialization
>
> What is the expected behaviour if the factory property is to set to a
> non-class or non-accessible class? The current implementation does (it
> probably should be more graceful) :
>
> $ java -Djdk.serialFilterFactory=allow T
> Exception in thread "main" java.lang.ExceptionInInitializerError
> at
> java.base/java.io.ObjectInputFilter$Config.(ObjectInputFilter.java:537)
> at
> java.base/java.io.ObjectInputStream.(ObjectInputStream.java:394)
> at T.main(T.java:5)
> Caused by: java.lang.ClassNotFoundException: allow
> at
> java.base/jdk.internal.loader.BuiltinClassLoader.loadClass(BuiltinClassLoader.java:636)
> at
> java.base/jdk.internal.loader.ClassLoaders$AppClassLoader.loadClass(ClassLoaders.java:182)
> at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:519)
> at java.base/java.lang.Class.forName0(Native Method)
> at java.base/java.lang.Class.forName(Class.java:466)
> at
> java.base/java.io.ObjectInputFilter$Config.(ObjectInputFilter.java:519)
> ... 2 more
If the factory class can not be found, the exception must be fatal;
continuing to run without the filter would be a security risk.
ExceptionInInitializerError was the closest I could find.
I'll improve the message; Oddly, ExceptionInInitializer does not allow both a
message and initCause().
And the stacktrace for the ClassNotFoundException is not going to be very
interesting.
-
PR: https://git.openjdk.java.net/jdk/pull/3996
Re: RFR: 8264859: Implement Context-Specific Deserialization Filters [v7]
On Mon, 24 May 2021 21:57:50 GMT, Roger Riggs wrote:
>> JEP 415: Context-specific Deserialization Filters extends the
>> deserialization filtering mechanisms with more flexible and customizable
>> protections against malicious deserialization. See JEP 415:
>> https://openjdk.java.net/jeps/415.
>> The `java.io.ObjectInputFilter` and `java.io.ObjectInputStream` classes are
>> extended with additional
>> configuration mechanisms and filter utilities.
>>
>> javadoc for `ObjectInputFilter`, `ObjectInputFilter.Config`, and
>> `ObjectInputStream`:
>>
>> http://cr.openjdk.java.net/~rriggs/filter-factory/java.base/java/io/ObjectInputFilter.html
>
> Roger Riggs has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Move merge and rejectUndecidedClass methods to OIF.Config
> As default methods on OIF, their implementations were not concrete and not
> trustable
src/java.base/share/classes/java/io/ObjectInputFilter.java line 73:
> 71: * var filter =
> ObjectInputFilter.Config.createFilter("example.*;java.base/*;!*")
> 72: * ObjectInputFilter.Config.setSerialFilter(filter);
> 73: * }
It's good to have a straightforward example, but it has an implicit assumption
- that the built-in filter factory is in operation ( and will not change ). I
wonder if there is a way to update the example (without too much fuss), or
otherwise add a textual qualification. Though, I'm not sure what this would
look like.
src/java.base/share/classes/java/io/ObjectInputFilter.java line 400:
> 398: * {@link BinaryOperator {@literal
> BinaryOperator}} interface, provide its implementation and
> 399: * be accessible via the {@linkplain
> ClassLoader#getSystemClassLoader() application class loader}.
> 400: * The filter factory configured using the system or security
> property during initialization
What is the expected behaviour if the factory property is to set to a non-class
or non-accessible class? The current implementation does (it probably should be
more graceful) :
$ java -Djdk.serialFilterFactory=allow T
Exception in thread "main" java.lang.ExceptionInInitializerError
at
java.base/java.io.ObjectInputFilter$Config.(ObjectInputFilter.java:537)
at
java.base/java.io.ObjectInputStream.(ObjectInputStream.java:394)
at T.main(T.java:5)
Caused by: java.lang.ClassNotFoundException: allow
at
java.base/jdk.internal.loader.BuiltinClassLoader.loadClass(BuiltinClassLoader.java:636)
at
java.base/jdk.internal.loader.ClassLoaders$AppClassLoader.loadClass(ClassLoaders.java:182)
at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:519)
at java.base/java.lang.Class.forName0(Native Method)
at java.base/java.lang.Class.forName(Class.java:466)
at
java.base/java.io.ObjectInputFilter$Config.(ObjectInputFilter.java:519)
... 2 more
-
PR: https://git.openjdk.java.net/jdk/pull/3996
Re: RFR: 8264859: Implement Context-Specific Deserialization Filters [v7]
On Mon, 24 May 2021 21:57:50 GMT, Roger Riggs wrote: >> JEP 415: Context-specific Deserialization Filters extends the >> deserialization filtering mechanisms with more flexible and customizable >> protections against malicious deserialization. See JEP 415: >> https://openjdk.java.net/jeps/415. >> The `java.io.ObjectInputFilter` and `java.io.ObjectInputStream` classes are >> extended with additional >> configuration mechanisms and filter utilities. >> >> javadoc for `ObjectInputFilter`, `ObjectInputFilter.Config`, and >> `ObjectInputStream`: >> >> http://cr.openjdk.java.net/~rriggs/filter-factory/java.base/java/io/ObjectInputFilter.html > > Roger Riggs has updated the pull request incrementally with one additional > commit since the last revision: > > Move merge and rejectUndecidedClass methods to OIF.Config > As default methods on OIF, their implementations were not concrete and not > trustable The conf/security/java.security file will need to be updated as part of this change. It does not have an entry for the factory property, and also its description of jdk.serialFilter will be no longer accurate - since filter set by jdk.serialFilter may no longer have any impact on OIS, if a filter factory is specified as either a system property or security property. - PR: https://git.openjdk.java.net/jdk/pull/3996
Re: RFR: 8264859: Implement Context-Specific Deserialization Filters [v7]
On Mon, 24 May 2021 21:57:50 GMT, Roger Riggs wrote:
>> JEP 415: Context-specific Deserialization Filters extends the
>> deserialization filtering mechanisms with more flexible and customizable
>> protections against malicious deserialization. See JEP 415:
>> https://openjdk.java.net/jeps/415.
>> The `java.io.ObjectInputFilter` and `java.io.ObjectInputStream` classes are
>> extended with additional
>> configuration mechanisms and filter utilities.
>>
>> javadoc for `ObjectInputFilter`, `ObjectInputFilter.Config`, and
>> `ObjectInputStream`:
>>
>> http://cr.openjdk.java.net/~rriggs/filter-factory/java.base/java/io/ObjectInputFilter.html
>
> Roger Riggs has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Move merge and rejectUndecidedClass methods to OIF.Config
> As default methods on OIF, their implementations were not concrete and not
> trustable
src/java.base/share/classes/java/io/ObjectInputFilter.java line 177:
> 175: * // Initially this would be the static JVM-wide filter
> passed from the OIS constructor
> 176: * // Append the filter to reject all UNDECIDED results
> 177: * filter = next.merge(filter).rejectUndecidedClass();
Update for merge() now being class method
src/java.base/share/classes/java/io/ObjectInputFilter.java line 866:
> 864: /**
> 865: * Returns a filter that merges the status of a filter and
> another filter.
> 866: * If the other filter is {@code null}, the filter is returned.
Now that this method is static, this sentence could be further clarified with
some markup, IMO:
"If `{@code anotherFilter}` is `{@code null}`, `{@code filter}` is returned."
src/java.base/share/classes/java/io/ObjectInputFilter.java line 874:
> 872: * Invoke {@code filter} on the {@code FilterInfo} to
> get its {@code status};
> 873: * Return {@code REJECTED} if the {@code status} is
> {@code REJECTED};
> 874: * Invoke the {@code otherFilter} to get the {@code
> otherStatus};
"the `otherFilter`" -> "`anotherFilter`"
src/java.base/share/classes/java/io/ObjectInputFilter.java line 892:
> 890:
> 891: /**
> 892: * Returns a filter that invokes a filter and maps {@code
> UNDECIDED} to {@code REJECTED}
"...that invokes _the given_ filter..." ?
src/java.base/share/classes/java/io/ObjectInputFilter.java line 895:
> 893: * for classes, with some exceptions, and otherwise returns the
> status.
> 894: * The filter returned checks that classes not {@code ALLOWED}
> and not {@code REJECTED} by the filter
> 895: * are {@code REJECTED}, if the class is an array and the base
> component type is not allowed,
Could/should this be simplified to, "...checks that classes not ALLOWED by the
filter are REJECTED."?
Also, I would add something like, "...,_including_ if the class is..." or
"...,_even_ if the class is..."; otherwise it sounds a bit like this _only_
applies to arrays.
src/java.base/share/classes/java/io/ObjectInputFilter.java line 1422:
> 1420: * {@linkplain ObjectInputStream#ObjectInputStream(InputStream)
> ObjectInputStream constructors}.
> 1421: * When invoked from {@link
> ObjectInputStream#setObjectInputFilter(ObjectInputFilter)
> 1422: * to set the stream-specific filter} the requested filter
> replaces the static serial filter,
"When invoked _from to_ set the..."
src/java.base/share/classes/java/io/ObjectInputFilter.java line 1477:
> 1475:
> 1476: /**
> 1477: * Returns the class name name of this builtin
> deserialization filter factory.
name name
-
PR: https://git.openjdk.java.net/jdk/pull/3996
Re: RFR: 8264859: Implement Context-Specific Deserialization Filters [v7]
> JEP 415: Context-specific Deserialization Filters extends the deserialization > filtering mechanisms with more flexible and customizable protections against > malicious deserialization. See JEP 415: https://openjdk.java.net/jeps/415. > The `java.io.ObjectInputFilter` and `java.io.ObjectInputStream` classes are > extended with additional > configuration mechanisms and filter utilities. > > javadoc for `ObjectInputFilter`, `ObjectInputFilter.Config`, and > `ObjectInputStream`: > > http://cr.openjdk.java.net/~rriggs/filter-factory/java.base/java/io/ObjectInputFilter.html Roger Riggs has updated the pull request incrementally with one additional commit since the last revision: Move merge and rejectUndecidedClass methods to OIF.Config As default methods on OIF, their implementations were not concrete and not trustable - Changes: - all: https://git.openjdk.java.net/jdk/pull/3996/files - new: https://git.openjdk.java.net/jdk/pull/3996/files/f1a797a5..141bf720 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=3996&range=06 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=3996&range=05-06 Stats: 459 lines in 3 files changed: 232 ins; 141 del; 86 mod Patch: https://git.openjdk.java.net/jdk/pull/3996.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3996/head:pull/3996 PR: https://git.openjdk.java.net/jdk/pull/3996
