Indeed, ReflectionFactory. newConstructorForSerialization can be used to access otherwise-private constructors. Before JDK-8315810, it could even allocate one class and call the constructor of another class.
I strongly support your proposal to restrict ReflectionFactory. Regards, Chen On Wed, May 15, 2024 at 6:23 PM David Lloyd <david.ll...@redhat.com> wrote: > > > 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 >
