Thanks Seán,
A good explanation. :)
Solaris was a very good platform for exposing and debugging race
conditions, of course we have very good static analysis now.
Regards,
Peter.
On 23/06/2021 5:10 pm, Seán Coffey wrote:
Thank for the feedback Peter. Comments inline.
On 22/06/2021 22:40, Peter Firmstone wrote:
Was ever to run with SecurityManager?
I found the issue while porting to jdk8u where Solaris uses a
configuration file with the SunPKCS11 Provider by default - We have
tests to register Providers while SecurityManager is in place. This
failed for SunPKCS11.
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.
Yes - that was one of my first actions. [1]. The jdk.crypto.cryptoki
ProtectionDomain lacks the permission and rightly so IMO. The default
policy doesn't grant "setContextClassLoader" permission to any JDK
module. It's not required when we use InnocuousThread.
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?
Use of InnocuousThread is made in various JDK classes for similar
purpose where daemon threads need to be run with limited privilege.
Similar use seen in networking and ldap classes.
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?
InnocuousThread sets this to null. The assert is just a belt and
braces approach which is a useful check during test runs. Again,
similar approach done in other JDK libraries.
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.
see above. We shouldn't have an issue. A non-null classloader would
lead to classloader memory leak in some environments.
regards,
Sean.
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.
[1]
access: domain that failed ProtectionDomain (jrt:/jdk.crypto.cryptoki
<no signer certificates>)
jdk.internal.loader.ClassLoaders$PlatformClassLoader@5433274e
<no principals>
java.security.Permissions@7006c658 (
("java.io.FilePermission" "<<ALL FILES>>" "read")
("java.net.SocketPermission" "localhost:0" "listen,resolve")
("java.security.SecurityPermission" "clearProviderProperties.*")
("java.security.SecurityPermission"
"getProperty.auth.login.defaultCallbackHandler")
("java.security.SecurityPermission" "putProviderProperty.*")
("java.security.SecurityPermission" "authProvider.*")
("java.security.SecurityPermission" "removeProviderProperty.*")
("java.util.PropertyPermission" "java.specification.version" "read")
("java.util.PropertyPermission" "java.vm.vendor" "read")
("java.util.PropertyPermission" "path.separator" "read")
("java.util.PropertyPermission" "os.version" "read")
("java.util.PropertyPermission" "java.vendor.url" "read")
("java.util.PropertyPermission" "java.vm.name" "read")
("java.util.PropertyPermission" "java.vm.specification.version" "read")
("java.util.PropertyPermission" "os.name" "read")
("java.util.PropertyPermission"
"sun.security.pkcs11.allowSingleThreadedModules" "read")
("java.util.PropertyPermission"
"sun.security.pkcs11.disableKeyExtraction" "read")
("java.util.PropertyPermission" "java.version" "read")
("java.util.PropertyPermission" "os.arch" "read")
("java.util.PropertyPermission" "java.specification.vendor" "read")
("java.util.PropertyPermission" "java.vm.specification.name" "read")
("java.util.PropertyPermission" "file.separator" "read")
("java.util.PropertyPermission" "line.separator" "read")
("java.util.PropertyPermission" "java.vm.specification.vendor" "read")
("java.util.PropertyPermission" "java.specification.name" "read")
("java.util.PropertyPermission" "java.vendor" "read")
("java.util.PropertyPermission" "java.vm.version" "read")
("java.util.PropertyPermission" "jdk.crypto.KeyAgreement.legacyKDF"
"read")
("java.util.PropertyPermission" "java.class.version" "read")
("java.lang.RuntimePermission" "accessClassInPackage.com.sun.beans.*")
("java.lang.RuntimePermission" "accessClassInPackage.sun.security.*")
("java.lang.RuntimePermission"
"accessClassInPackage.com.sun.crypto.provider")
("java.lang.RuntimePermission" "accessClassInPackage.com.apple.*")
("java.lang.RuntimePermission"
"accessClassInPackage.com.sun.java.swing.plaf.*")
("java.lang.RuntimePermission" "accessSystemModules")
("java.lang.RuntimePermission" "accessClassInPackage.sun.nio.ch")
("java.lang.RuntimePermission" "accessClassInPackage.com.sun.beans")
("java.lang.RuntimePermission" "loadLibrary.j2pkcs11")
)
Exception in thread "main" java.security.ProviderException:
Initialization failed
at
jdk.crypto.cryptoki/sun.security.pkcs11.SunPKCS11.<init>(SunPKCS11.java:386)
at
jdk.crypto.cryptoki/sun.security.pkcs11.SunPKCS11$1.run(SunPKCS11.java:117)
at
jdk.crypto.cryptoki/sun.security.pkcs11.SunPKCS11$1.run(SunPKCS11.java:114)
at
java.base/java.security.AccessController.doPrivileged(AccessController.java:569)
at
jdk.crypto.cryptoki/sun.security.pkcs11.SunPKCS11.configure(SunPKCS11.java:114)
at
java.base/sun.security.jca.ProviderConfig$3.run(ProviderConfig.java:257)
at
java.base/sun.security.jca.ProviderConfig$3.run(ProviderConfig.java:248)
at
java.base/java.security.AccessController.doPrivileged(AccessController.java:318)
at
java.base/sun.security.jca.ProviderConfig.doLoadProvider(ProviderConfig.java:248)
at
java.base/sun.security.jca.ProviderConfig.getProvider(ProviderConfig.java:226)
at
java.base/sun.security.jca.ProviderList.loadAll(ProviderList.java:317)
at
java.base/sun.security.jca.ProviderList.removeInvalid(ProviderList.java:334)
at
java.base/sun.security.jca.Providers.getFullProviderList(Providers.java:175)
at
java.base/java.security.Security.getProviders(Security.java:458)
at DefaultPKCS11.main(DefaultPKCS11.java:13)
Caused by: java.security.AccessControlException: access denied
("java.lang.RuntimePermission" "setContextClassLoader")
at
java.base/java.security.AccessControlContext.checkPermission(AccessControlContext.java:485)
at
java.base/java.security.AccessController.checkPermission(AccessController.java:1068)
at
java.base/java.lang.SecurityManager.checkPermission(SecurityManager.java:416)
at
java.base/java.lang.Thread.setContextClassLoader(Thread.java:1525)
at
jdk.crypto.cryptoki/sun.security.pkcs11.SunPKCS11$NativeResourceCleaner.<init>(SunPKCS11.java:982)
at
jdk.crypto.cryptoki/sun.security.pkcs11.SunPKCS11.initToken(SunPKCS11.java:1193)
at
jdk.crypto.cryptoki/sun.security.pkcs11.SunPKCS11.<init>(SunPKCS11.java:377)
... 14 more
