RFR: 8338411: Implement JEP 486: Permanently Disable the Security Manager [v2]

Sean Mullan mullan at openjdk.org
Fri Oct 18 19:56:17 UTC 2024


On Tue, 15 Oct 2024 14:50:54 GMT, Daniel Fuchs <dfuchs at openjdk.org> wrote:

>> Sean Mullan has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 97 commits:
>> 
>>  - Merge remote-tracking branch 'jdk-sandbox/jep486' into JDK-8338411
>>  - Change apiNote to deprecated annotation on checkAccess methods. Change method dedescription to "Does nothing".
>>  - Sanitize the class descriptions of DelegationPermission and ServicePermission
>>    by removing text that refers to granting permissions, but avoid changes that
>>    affect the API specification, such as the description and format of input
>>    parameters.
>>  - Restored methods in RMIConnection to throw SecurityExceptions again but
>>    with adjusted text that avoids the word "permission".
>>  - Add text to class description of MBeanServer stating that implementations
>>    may throw SecurityException if authorization doesn't allow access to resource.
>>  - Restore text about needing permissions from the desktop environment in the
>>    getPixelColor and createScreenCapture methods.
>>  - Add api note to getClassContext to use StackWalker instead and
>>    add DROP_METHOD_INFO option to StackWalker.
>>  - Change checkAccess() methods to be no-ops, rather than throwing
>>    SecurityException.
>>  - Merge
>>  - Merge
>>  - ... and 87 more: https://git.openjdk.org/jdk/compare/f50bd0d9...f89d9d09
>
> src/java.management.rmi/share/classes/javax/management/remote/rmi/RMIConnection.java line 159:
> 
>> 157:      * is specified for the MBean.
>> 158:      * @throws IOException if a general communication exception occurred.
>> 159:      * @throws UnsupportedOperationException if {@code delegationSubject} is non-null.
> 
> Maybe we should revert those changes, or word them differently. AFAIU, is is still possible for a JMXConnectorServer to implement coarse grained authorization by setting up an `MBeanServerAccessController`, and in fact, the default JMX Agent does that. The JDK has a built-in implementation of `MBeanServerAccessController`,  `MBeanServerFileAccessController`, which will throw `SecurityException` if access is denied by the `MBeanServerFileAccessController`. I believe this will result in the `RMIConnection` throwing `SecurityException` if the operation is denied by the `MBeanServerAccessController`.
> 
> So I believe - in all methods here and in `RMIConnectionImpl`, we should leave the door open for `SecurityException` to get thrown.
> 
> An alternative could be to cover that use case with a blanket statement, here, in `RMIConnectionImpl`, in `MBeanServer`, and in `MBeanServerConnection`.

I restored the changes to `RMIConnection` to throw `SecurityException` but adjusted the text to say "is not authorized" instead of "does not have permission". See https://github.com/openjdk/jdk/pull/21498/commits/86ff71461ef1d695c02497626facda63c496a287.

As we discussed offline, I also added a sentence to the `MBeanServer` class description to state that it and its subclasses may throw `SecurityException`s if the implementation doesn't authorize access to the underlying resource: https://github.com/openjdk/jdk/pull/21498/commits/44432e56a91a992150ee873e81282d1fe21e69ea.

> src/java.management/share/classes/javax/management/remote/JMXAuthenticator.java line 67:
> 
>> 65:      */
>> 66:     public Subject authenticate(Object credentials);
>> 67: }
> 
> Should this be reverted? Authentication has not been removed.

Yes. See the fix in https://github.com/openjdk/jdk/pull/21498/commits/23a43e0d90aff8754909785f582ba0666046cf6c.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21498#discussion_r1806952922
PR Review Comment: https://git.openjdk.org/jdk/pull/21498#discussion_r1806953524


More information about the nio-dev mailing list