Was ever to run with SecurityManager?
When you see an AccessControlException, I'd recommend setting the
following security debug property, so you can capture the
ProtectionDomain that failed the access check:
-Djava.security.debug=access:failure Clearly there's a ProtectionDomain
on the calling threads stack that doesn't have the necessary
permission. If you knew which one it was, you could just grant it
java.lang.RuntimePermission "setContextClassLoader" permission in the
policy file.
In NativeResourceCleaner the original constructor is setting the Context
ClassLoader of the calling thread to null, prior to calling the Thread
superclass constructor, so that both the calling thread and new thread
will nave a null context ClassLoader. In your new implementation, you
are asserting the context class loader of the created thread is null,
without setting the context ClassLoader of the original calling thread,
is that the intended behaviour?
Alternatively you could set the context ClassLoader of the calling
thread to null using a PrivilegedAction, prior to creating the new
thread and restore it?
If the original intent was to set the context ClassLoader of the new
thread to null, then why not just do that, rather than use an assertion?
If assertions are not enabled it may run with a non null context
ClassLoader? What are the consequences? It appears to me, the fix has
created a bigger problem, rather than fixed it. Just my 2 cents.
We use SecurityManager by default following principles of least
privilege (only the code paths we need to execute), and the original
reported bug is a non problem for us, we would capture the missing
permission and grant it, these are permission grants for Java 16:
grant codebase "jrt:/jdk.crypto.cryptoki"
{
permission java.lang.RuntimePermission
"accessClassInPackage.sun.security.util";
};
grant codebase "jrt:/jdk.crypto.ec"
{
permission java.security.SecurityPermission
"putProviderProperty.SunEC";
permission java.lang.RuntimePermission
"accessClassInPackage.sun.security.jca";
permission java.lang.RuntimePermission
"accessClassInPackage.sun.security.pkcs";
permission java.lang.RuntimePermission
"accessClassInPackage.sun.security.util";
permission java.lang.RuntimePermission
"accessClassInPackage.sun.security.util.math";
permission java.lang.RuntimePermission
"accessClassInPackage.sun.security.util.math.intpoly";
permission java.lang.RuntimePermission
"accessClassInPackage.sun.security.x509";
};
Good call making NativeResourceCleaner implement Runnable instead of extending
Thread though.
--
Regards,
Peter Firmstone
0498 286 363
Zeus Project Services Pty Ltd.
On 22/06/2021 11:32 pm, Sean Coffey wrote:
Sufficient permissions missing if this code was ever to run with
SecurityManager.
Cleanest approach appears to be use of InnocuousThread to create the
cleaner/poller threads.
Test case coverage extended to cover the SecurityManager scenario.
Reviewer request: @valeriepeng
-------------
Commit messages:
- 8269034: AccessControlException for SunPKCS11 daemon threads
Changes: https://git.openjdk.java.net/jdk17/pull/117/files
Webrev: https://webrevs.openjdk.java.net/?repo=jdk17&pr=117&range=00
Issue: https://bugs.openjdk.java.net/browse/JDK-8269034
Stats: 112 lines in 5 files changed: 73 ins; 17 del; 22 mod
Patch: https://git.openjdk.java.net/jdk17/pull/117.diff
Fetch: git fetch https://git.openjdk.java.net/jdk17 pull/117/head:pull/117
PR: https://git.openjdk.java.net/jdk17/pull/117
