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

Sean Mullan mullan at openjdk.org
Fri Oct 18 19:46:49 UTC 2024


On Tue, 15 Oct 2024 17:01:59 GMT, Sean Mullan <mullan at openjdk.org> wrote:

>>> While making `LogManager.checkAccess` be a no-op might be more convenient, it could unconditionally permit operations that formerly required a permission check: clearly a bad idea. Always throwing a `SecurityException` is the safest option.
>> 
>> It's not about convenience _or_ safety; this part of the change has a provably flawed logical basis.
>> 
>> These methods would no longer called from within the JDK after this change. All three of these methods were already previously defined to be a no-op when no security manager was installed (specifically when `System.getSecurityManager() == null`). Since no security manager may be installed after this change, this method will always return `null`. Thus, a no-op is still the most correct behavior and does not permit any operation that previously required a permission check (since it was already a no-op any time no security manager was installed, which will now be the only possible scenario). Therefore it is provably no safer to throw `SecurityException` here, since this will only prompt library developers to introduce the workaround I posted above when their tests fail, yielding the exact same result (except with a minor inconvenience to library developers).
>> 
>> Either way is fine (as I said, the workaround is trivial), but IMO it's best to be conscious of the correct reasoning lest flawed assumptions _do_ end up enabling the introduction of unsafe changes elsewhere in the code. We don't have to make any assumptions about safety or previous behavior because it's all statically provable.
>
> I see your point now. We have strived to preserve compatibility with libraries that follow well known code patterns as described in the JEP, so I will discuss this with others who are more familiar with the compatibility risk of making this change.

I have changed `LogManager.checkAccess`, `Thread.checkAccess`, `ThreadGroup.checkAccess` to always do nothing. See the fixes in https://github.com/openjdk/jdk/pull/21498/commits/2ebb6de50194770658462507cee28b785fb30dbd and https://github.com/openjdk/jdk/pull/21498/commits/16e17b89b3dbce4f49706c032c315977dd54c315.

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

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


More information about the nio-dev mailing list