On Tue, May 14, 2024 at 10:01 AM David Lloyd <david.ll...@redhat.com> wrote:
> (Moving to core-libs-dev) > > On Tue, May 14, 2024 at 9:40 AM Alan Bateman <alan.bate...@oracle.com> > wrote: > >> On 14/05/2024 14:42, David Lloyd wrote: >> >> ReflectionFactory allows access to serialization facilities without any >> access checking (other than the defunct SecurityManager checks). Perhaps >> this class could gain some more methods, like this: >> >> * `newGetterForSerialization(Field field)` - includes ability to access >> `objectStreamFields` and `serialVersionUID`, or these could be separate >> methods >> * `newSetterForSerialziation(Field field)` >> >> Does this seem workable? >> >> The intention with ReflectionFactory is that serialization libraries go >> through the readObject/writeObject and other magic methods, to avoid field >> access. >> >> Probably best to being this to core-libs-dev for further discussion. >> > > This doesn't match my recollection. The `readObject` and `writeObject` > methods are optional private methods which serializable classes *may* > provide when they want a bespoke serialization strategy (and the other > methods that are accessed via this special class are similar in this > regard). They are not in any way magical in that they do not provide the > means to perform this part of the serialization process, and thus they are > not a substitute for field access in serialization. According to > JDK-8164908, these methods were added because at the time, Unsafe was still > state of the art for custom serialization and IIOP to access fields (with > libraries using Field actively moving to Unsafe instead). However Unsafe > did not and does not provide access to methods, so in order to avoid the > aforementioned `add-opens` explosion, these methods were added as a > compromise. Now that Unsafe is being deprecated, it is my opinion that this > does need to be revisited because at the time, Unsafe was the recommended > approach. > > [1] https://bugs.openjdk.org/browse/JDK-8164908 > It seems to me that it might be a good idea (as part of 8305968 (Integrity by default)) to put the `ReflectionFactory` API behind a restricted method call somehow, even considered separately from the changes suggested above. Maybe in conjunction with a non-standard switch that works similarly to `--enable-native-access`, e.g. `--enable-serialization=org.serial.framework`? That would also somewhat mitigate the security risk of adding the aforementioned methods or something like them to this class. Should I open a bug for either (or both) of these suggestions? Or perhaps there is a better way to proceed? -- - DML • he/him
