On Tue, 30 Jan 2024 13:53:37 GMT, Daniel Fuchs <dfu...@openjdk.org> wrote:

>> This code change adds an alternative implementation of user-based 
>> authorization `Subject` APIs that doesn't depend on Security Manager APIs. 
>> Depending on if the Security Manager is allowed, the methods store the 
>> current subject differently. See the spec change in the `Subject.java` file 
>> for details. When the Security Manager APIs are finally removed in a future 
>> release, this new implementation will be only implementation for these 
>> methods.
>> 
>> One major change in the new implementation is that `Subject.getSubject` 
>> always throws an `UnsupportedOperationException` since it has an 
>> `AccessControlContext` argument but the current subject is no longer 
>> associated with an `AccessControlContext` object.
>> 
>> Now it's the time to migrate from the `getSubject` and `doAs` methods to 
>> `current` and `callAs`. If the user application is simply calling 
>> `getSubject(AccessController.getContext())`, then switching to `current()` 
>> would work. If the `AccessControlContext` argument is retrieved from an 
>> earlier `getContext()` call and the associated subject might be different 
>> from that of the current `AccessControlContext`, then instead of storing the 
>> previous `AccessControlContext` object and passing it into `getSubject` to 
>> get the "previous" subject, the application should store the `current()` 
>> return value directly.
>
> src/java.management/share/classes/com/sun/jmx/remote/internal/ServerNotifForwarder.java
>  line 349:
> 
>> 347:     @SuppressWarnings("removal")
>> 348:     private Subject getSubject() {
>> 349:         return Subject.current();
> 
> Since `Subject::current` is not deprecated the annotation at line 347 above 
> should be removed.

OK - things seem to be a bit convoluted here and some pieces might be missing. 
I suspect that what needs to be done is more complicated:

`RMIConnectionImpl` sets up an ACC and calls doPrivileged with that ACC, on the 
assumption that the subject is tied to the ACC and it can be retrieved down the 
road from the ACC. `RMIConnectionImpl` has the subject, and the delegation 
subject too. 

So for `Subject::current` to work here, shouldn't 
`RMIConnectionImpl::doPrivilegedOperation` use `Subject::callAs` when the 
security manager is disallowed?

It seems that when `Subject::current` is used, some analysis should be done to 
verify where the Subject is supposed to come from - that is - how the caller is 
expecting the subject to reach the callee.

Typically, JMX doesn't use `Subject::doAs` but ties a `Subject` to an 
`AccessControlContext` and uses `doPrivileged` instead.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/17472#discussion_r1471308151

Reply via email to