Just bumping this one more time. I intend to start by opening a JIRA to add the two proposed methods to `ReflectionFactory`, and go from there. I guess that we might need a JEP for the proposed serialization restrictions, which is going to be considerably more involved, so I'm putting that off as a second step for now, pending further discussion.
On Wed, May 15, 2024 at 9:00 PM - <liangchenb...@gmail.com> wrote: > 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 >> > -- - DML • he/him
