RFR: 8333344: JMX attaching of Subject does not work when security manager not allowed [v3]

[Sean Mullan]

On Tue, 11 Jun 2024 16:51:11 GMT, Weijun Wang <weijun at openjdk.org> wrote:

>> Kevin Walls has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Sean comments
>
> src/java.management.rmi/share/classes/javax/management/remote/rmi/RMIConnectionImpl.java line 1436:
> 
>> 1434:             } else {
>> 1435:                 // ACC is present, we have a Subject and SM is permitted:
>> 1436:                 return AccessController.doPrivileged((PrivilegedExceptionAction<Object>) () -> Subject.doAs(subject, op), acc);
> 
> Why is it necessary to call both `doAs` and `doPrivileged`?

Can you just call `AccessController.doPrivileged(op, acc)` as in the original code? `Subject.doAs` requires a permission, which is why I assume you are wrapping it in a `doPrivileged` but you are asserting all of the permissions of the `java.management` module, which means you probably want to use limited doPriv and only assert the `getSubject` permission but that is getting complicated. I'd go back to the original call as it should work in the SM allow case.

> src/java.management/share/classes/com/sun/jmx/remote/internal/ServerNotifForwarder.java line 350:
> 
>> 348:     @SuppressWarnings("removal")
>> 349:     private Subject getSubject() {
>> 350:         Subject subject = null;
> 
> Just call `Subject.current()`. When SM is allowed, it is equivalent to `getSubject(AccessController.getContext())`.

For the same reason you should be able to just call `Subject.current` in the tests. I don't think you need the `SimpleStandard.useGetSubjectACC` property to toggle the testing of either old or replacement APIs as long as the test has runs in both SM allow and disallow (i.e., no SM prop set) modes. Would you agree Weijun?

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/19624#discussion_r1635475145
PR Review Comment: https://git.openjdk.org/jdk/pull/19624#discussion_r1635449088