On Wed, 12 Jun 2024 16:11:28 GMT, Kevin Walls <kev...@openjdk.org> wrote:
>> JMX uses APIs related to the Security Mananger which are deprecated. Use of
>> AccessControlContext will be removed when Security Manager is removed.
>>
>> Until then, updates are needed to not require setting
>> -Djava.security.manager=allow to use JMX authentication.
>
> Kevin Walls has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Undo test policy updates
src/java.management.rmi/share/classes/javax/management/remote/rmi/RMIConnectionImpl.java
line 1304:
> 1302: // No ACC, therefore no SM. May have a Subject:
> 1303: if (subject != null) {
> 1304: return Subject.doAs(subject, action);
Is it ever possible for subject to be `null` (passed in as `null` to the
constructor) and an SM to be enabled? If so, then the call above to
`Subject.doAs` would trigger a permission for an `AuthPermission("doAs")`
permission. (This also may have been the case you were seeing in tests).
I think following Weijun's advice above is cleaner and safer, so you do one or
the other depending on the allowSM setting, and not whether certain variables
are null or not.
src/java.management/share/classes/com/sun/jmx/remote/security/MBeanServerFileAccessController.java
line 304:
> 302: }
> 303:
> 304: @SuppressWarnings("removal")
Is this annotation needed? I see you have one below.
src/java.management/share/classes/com/sun/jmx/remote/security/MBeanServerFileAccessController.java
line 312:
> 310: @SuppressWarnings("removal")
> 311: final AccessControlContext acc =
> AccessController.getContext();
> 312: //@SuppressWarnings("removal")
Remove this line?
src/java.management/share/classes/javax/management/monitor/Monitor.java line
1543:
> 1541: // No SecurityManager:
> 1542: Subject.doAs(s, action); // s is permitted to be null
> 1543: } else {
Even though ac should never be null, I would keep the original check for ac ==
null inside the SM code path to be safe and consistent with the original code,
and use only call doAs if allowSM is false, so like:
if (!SharedSecrets.getJavaLangAccess().allowSecurityManager()) {
// No SecurityManager:
Subject.doAs(s, action); // s is permitted to be null
} else {
if (ac == null) {
throw new SecurityException("AccessControlContext cannot be null");
}
// ACC means SM is permitted.
AccessController.doPrivileged(action, ac);
}
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/19624#discussion_r1637086418
PR Review Comment: https://git.openjdk.org/jdk/pull/19624#discussion_r1637060238
PR Review Comment: https://git.openjdk.org/jdk/pull/19624#discussion_r1637060517
PR Review Comment: https://git.openjdk.org/jdk/pull/19624#discussion_r1637073809
