RFR: 8296244: Alternate implementation of user-based authorization Subject APIs that doesn’t depend on Security Manager APIs

Weijun Wang weijun at openjdk.org
Tue Jan 30 16:48:53 UTC 2024


On Tue, 30 Jan 2024 14:19:02 GMT, Daniel Fuchs <dfuchs at openjdk.org> wrote:

>> 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.

This is complicated.

The benefit of `current()` is that it is always equivalent to `getSubject(AccessController.getContext())`  *as long as the latter works*. However, in this case, when a security manager is not allowed, the latter DOES NOT work but `current()` still works.

I'll revert the change. Before finding an alternative solution for `JMXSubjectDomainCombiner`, I assume this code only works when a security manager is allowed. It's better to throw an UOE instead of returning null.

I'm not sure of the other `getSubject` call (below), but I'll revert the change as all.

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

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


More information about the core-libs-dev mailing list