Re: RFR: 8268349: Provide more detail in JEP 411 warning messages
I can re-license some code that decorates Concurrent collections with references, so you can do this without blocking. https://pfirmstone.github.io/JGDMS/jgdms-collections/apidocs/index.html On 9/06/2021 4:31 am, Alan Bateman wrote: On Tue, 8 Jun 2021 18:22:55 GMT, Weijun Wang wrote: I thought about that but not sure of performance impact. Is the worst problem that more than one warnings will be printed for a single caller? It's not really harmless. As for the frame, if the warning message only contain the caller class name and its code source, why is it worth using a key of multiple frames? The message will look the same. WeakHashMap access needs synchronization. Whether we need to cache to avoid excessive warnings isn't clear. If the SM is enabled once and never disabled/re-enabled then caching isn't interesting. On the other hand if there are programs that are enabling/disabling to execute subsets of code then maybe it is. Maybe we should just drop this and see if there is any feedback on the repeated warning? Not sure what you meant by "WeakHashMap access synchronization", it's just a noun without any other parts. Do you think synchronization is necessary? For the cache, I'm OK to drop it at the moment. I think it would be simpler to start out without the caller cache. Sorry the sentence got garbled, I was trying to repeat what I said above that WeakHashMap is not synchronized so you would need to add synchronization to use it. - PR: https://git.openjdk.java.net/jdk/pull/4400 -- Regards, Peter Firmstone 0498 286 363 Zeus Project Services Pty Ltd.
Integrated: 8240997: Remove more "hack" word in security codes
On Fri, 21 May 2021 18:37:12 GMT, Jack Hartstein wrote: > This change is part of a larger code cleanup effort and removes the term > "hack" out of several files in the jdk. > JBS: [](https://bugs.openjdk.java.net/browse/JDK-8240997 ) This pull request has now been integrated. Changeset: 58a59e3d Author:Jack Hartstein Committer: Jamil Nimeh URL: https://git.openjdk.java.net/jdk/commit/58a59e3dcb830211e1eef8122c9f7113c00ded4c Stats: 8 lines in 4 files changed: 0 ins; 0 del; 8 mod 8240997: Remove more "hack" word in security codes Reviewed-by: xuelei - PR: https://git.openjdk.java.net/jdk/pull/4148
Low level hooks in JDK for instrumentation of permission checks.
Apologies in advance if this seems like paranoid security.
As you are likely now aware, we have been using SecurityManager a little
differently than recommended as we adapted it to our requirements.
Sometimes it's not always easy to explain or obvious why something is
done in a certain way.
It's clear we can use StackWalker to implement AccessController
functionality. And it's also clear we can use ThreadLocal or Scope
Local's to preserve the user Subject across threads.
Going forward we will need low level hooks in the JDK for our
authentication layer, clearly this is an opportunity to further simplify
and improve our authentication layer.
Because we use a remote service architecture, with proxy's, the proxy's
are dynamically granted permission's after Service Authentication, these
permissions also require the user principal to be logged in, we may have
a number of services on the stack, for example to participate in a
transaction.
We have a tool to generate least privilege policy files.
There are two reasons we do this:
1. Simplicity of administration and auditing of policy files.
2. Limit the permissions of code, and grant certain permissions to
users to ensure users are authenticated before allowing data parsing.
An example of item two, is our services require users to be logged in to
ensure that any data provided by the user is a trusted data source (we
still check the data). We re-implemented a subset of Java
Serialization and have a DeSerializationPermission, which is granted to
Principals of users.
If a user is not logged in, data cannot be de-serialized, because the
code alone doesn't have permission to do so.
Hopefully modules and packages will have strong encapsulation in future
so we don't need permission's like java.lang.RuntimePermission
"accessClassInPackage.*" No doubt we will need to create our own
Permission's.
We would like to be able to limit data parsing, like XML or Java
de-serialization, to logged in users only.
We don't break encapsulation, in future we will only use reflection to
call public methods and constructors (we are currently in the process of
doing so). Our build systems use Maven, our build is modular.
I would also like to request that all JDK modules be given
ProtectionDomain's following SecurityManager deprecation. Currently some
modules have null ProtectionDomain's to show they have AllPermission.
However we don't grant AllPermission to code in practise, we like to
grant certain Permission's to Principal's, not code, where the Principal
is the source of data, indicating the user has been authenticated and we
only grant what's necessary and no more.
Examples of permission's granted to JDK modules in POLP policy files,
taken from a test harness:
grant codebase "jrt:/jdk.security.jgss"
{
permission java.lang.RuntimePermission
"accessClassInPackage.sun.security.util";
permission java.security.SecurityPermission
"putProviderProperty.JdkSASL";
};
grant codebase "jrt:/jdk.crypto.mscapi"
{
permission java.lang.RuntimePermission
"accessClassInPackage.sun.security.util";
permission java.lang.RuntimePermission "loadLibrary.sunmscapi";
permission java.security.SecurityPermission
"putProviderProperty.SunMSCAPI";
};
grant codebase "jrt:/jdk.localedata"
{
permission java.lang.RuntimePermission
"accessClassInPackage.sun.util.locale.provider";
permission java.lang.RuntimePermission
"accessClassInPackage.sun.util.resources";
};
grant codebase "jrt:/jdk.security.auth"
{
permission java.io.FilePermission
"C:\\Users\\peter\\Documents\\NetBeansProjects\\JGDMS\\qa\\lib\\jiniharness.jar",
"read";
permission javax.security.auth.AuthPermission "modifyPrincipals";
permission javax.security.auth.AuthPermission
"modifyPrivateCredentials";
permission javax.security.auth.AuthPermission
"modifyPublicCredentials";
permission java.lang.RuntimePermission
"accessClassInPackage.sun.security.krb5";
permission java.lang.RuntimePermission
"accessClassInPackage.sun.security.util";
permission java.lang.RuntimePermission "getProtectionDomain";
};
Example of POLP grant to code with principal, code alone cannot access
these, in case you are wondering, we use this to secure the RMI Registry
using stateless TLSv1.3, it's used by our Service Watchdog, or Service
Activation framework called Phoenix, it's the sole use we have of the
Java RMI JRMP protocol, in cases where this isn't used we can disable
Java Serialization completely:
grant codebase
"file:/C:/Users/peter/Documents/NetBeansProjects/JGDMS/JGDMS/dist/target/JGDMS-3.1.1-SNAPSHOT/lib/jgdms-rmi-tls-3.1.1-SNAPSHOT.jar",
principal javax.security.auth.x500.X500Principal "CN=Phoenix"
{
permission java.util.PropertyPermission "javax.net.ssl.trustStore",
"read";
permission java.util.PropertyPermission
"javax.net.ssl.trustStorePassword", "read";
permission java.util.PropertyPermission
Re: RFR: 8240997: Remove more "hack" word in security codes
On Fri, 21 May 2021 18:37:12 GMT, Jack Hartstein wrote: > This change is part of a larger code cleanup effort and removes the term > "hack" out of several files in the jdk. > JBS: [](https://bugs.openjdk.java.net/browse/JDK-8240997 ) Marked as reviewed by xuelei (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/4148
Re: RFR: 8240997: Remove more "hack" word in security codes
On Fri, 21 May 2021 18:37:12 GMT, Jack Hartstein wrote: > This change is part of a larger code cleanup effort and removes the term > "hack" out of several files in the jdk. > JBS: [](https://bugs.openjdk.java.net/browse/JDK-8240997 ) Hi, Please send me an e-mail at dalibor.to...@oracle.com so that I can mark your account as verified. - PR: https://git.openjdk.java.net/jdk/pull/4148
RFR: 8240997: Remove more "hack" word in security codes
This change is part of a larger code cleanup effort and removes the term "hack" out of several files in the jdk. JBS: [](https://bugs.openjdk.java.net/browse/JDK-8240997 ) - Commit messages: - 8240997: Remove more hack word in security codes Changes: https://git.openjdk.java.net/jdk/pull/4148/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=4148=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8240997 Stats: 8 lines in 4 files changed: 0 ins; 0 del; 8 mod Patch: https://git.openjdk.java.net/jdk/pull/4148.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4148/head:pull/4148 PR: https://git.openjdk.java.net/jdk/pull/4148
Re: RFR: 8268349: Provide more detail in JEP 411 warning messages
On Tue, 8 Jun 2021 18:22:55 GMT, Weijun Wang wrote: >>> I thought about that but not sure of performance impact. Is the worst >>> problem that more than one warnings will be printed for a single caller? >>> It's not really harmless. >>> >>> As for the frame, if the warning message only contain the caller class name >>> and its code source, why is it worth using a key of multiple frames? The >>> message will look the same. >> >> WeakHashMap access needs synchronization. Whether we need to cache to avoid >> excessive warnings isn't clear. If the SM is enabled once and never >> disabled/re-enabled then caching isn't interesting. On the other hand if >> there are programs that are enabling/disabling to execute subsets of code >> then maybe it is. Maybe we should just drop this and see if there is any >> feedback on the repeated warning? > > Not sure what you meant by "WeakHashMap access synchronization", it's just a > noun without any other parts. Do you think synchronization is necessary? > > For the cache, I'm OK to drop it at the moment. I think it would be simpler to start out without the caller cache. Sorry the sentence got garbled, I was trying to repeat what I said above that WeakHashMap is not synchronized so you would need to add synchronization to use it. - PR: https://git.openjdk.java.net/jdk/pull/4400
Re: RFR: 8268349: Provide more detail in JEP 411 warning messages
On Tue, 8 Jun 2021 17:15:29 GMT, Alan Bateman wrote: >> I thought about that but not sure of performance impact. Is the worst >> problem that more than one warnings will be printed for a single caller? >> It's not really harmless. >> >> As for the frame, if the warning message only contain the caller class name >> and its code source, why is it worth using a key of multiple frames? The >> message will look the same. > >> I thought about that but not sure of performance impact. Is the worst >> problem that more than one warnings will be printed for a single caller? >> It's not really harmless. >> >> As for the frame, if the warning message only contain the caller class name >> and its code source, why is it worth using a key of multiple frames? The >> message will look the same. > > WeakHashMap access needs synchronization. Whether we need to cache to avoid > excessive warnings isn't clear. If the SM is enabled once and never > disabled/re-enabled then caching isn't interesting. On the other hand if > there are programs that are enabling/disabling to execute subsets of code > then maybe it is. Maybe we should just drop this and see if there is any > feedback on the repeated warning? Not sure what you meant by "WeakHashMap access synchronization", it's just a noun without any other parts. Do you think synchronization is necessary? For the cache, I'm OK to drop it at the moment. - PR: https://git.openjdk.java.net/jdk/pull/4400
Re: RFR: 8268349: Provide more detail in JEP 411 warning messages
On Tue, 8 Jun 2021 12:22:52 GMT, Weijun Wang wrote: > I thought about that but not sure of performance impact. Is the worst problem > that more than one warnings will be printed for a single caller? It's not > really harmless. > > As for the frame, if the warning message only contain the caller class name > and its code source, why is it worth using a key of multiple frames? The > message will look the same. WeakHashMap access synchronization. Whether we need to cache to avoid excessive warnings isn't clear. If the SM is enabled once and never disabled/re-enabled then caching isn't interesting. On the other hand if there are programs that are enabling/disabling to execute subsets of code then maybe it is. Maybe we should just drop this and see if there is any feedback on the repeated warning? - PR: https://git.openjdk.java.net/jdk/pull/4400
RFR: 8266182: Create a manual test for jdk/sun/security/pkcs12/ParamsTest.java
ParamsTest is an interop test between keytool <-> openssl. There are some manual steps listed in jdk/sun/security/pkcs12/params/README to perform after the execution of jtreg execution. So this test is to perform that manual steps. - Commit messages: - 8266182: Create a manual test for jdk/sun/security/pkcs12/ParamsTest.java - Revert "8266182: Create a manual test for jdk/sun/security/pkcs12/ParamsTest.java" - 8266182: Create a manual test for jdk/sun/security/pkcs12/ParamsTest.java Changes: https://git.openjdk.java.net/jdk/pull/4413/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=4413=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8266182 Stats: 37 lines in 1 file changed: 37 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/4413.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4413/head:pull/4413 PR: https://git.openjdk.java.net/jdk/pull/4413
Re: RFR: 8268349: Provide more detail in JEP 411 warning messages
On Tue, 8 Jun 2021 06:11:17 GMT, Alan Bateman wrote: >> More loudly and precise warning messages when a security manager is either >> enabled at startup or installed at runtime. > > src/java.base/share/classes/java/lang/System.java line 331: > >> 329: >> 330: // Remember original System.err. setSecurityManager() warning goes >> here >> 331: private static PrintStream oldErrStream = null; > > I assume this should needs to be volatile and @Stable. I think we need a > better name for it too. Will add the modifiers. How about "originalErr"? > src/java.base/share/classes/java/lang/System.java line 336: > >> 334: // Remember callers of setSecurityManager() here so that warning >> 335: // is only printed once for each different caller >> 336: final static Map callersOfSSM = new >> WeakHashMap<>(); > > You can't use a WeakHashMap without synchronization but a big question here > is whether a single caller frame is sufficient. If I were doing this then I > think I would capture the hash of a number of stack frames to create a better > filter. I thought about that but not sure of performance impact. Is the worst problem that more than one warnings will be printed for a single caller? It's not really harmless. As for the frame, if the warning message only contain the caller class name and its code source, why is it worth using a key of multiple frames? The message will look the same. > src/java.base/share/classes/java/lang/System.java line 2219: > >> 2217: WARNING: java.lang.SecurityManager is >> deprecated and will be removed in a future release >> 2218: WARNING: -Djava.security.manager=%s >> will have no effect when java.lang.SecurityManager is removed >> 2219: """, smProp); > > Raw strings may be useful here but means the lines length are inconsistent > and makes it too hard to look at side by side diffs now. I understand what you mean when I switch to Split View. While I can extract the lines to a method, I somehow think it's not worth doing because for each type of warning the method is only called once. - PR: https://git.openjdk.java.net/jdk/pull/4400
Re: RFR: 8268349: Provide more detail in JEP 411 warning messages
On Tue, 8 Jun 2021 06:41:11 GMT, Alan Bateman wrote: > > You might want to make your "WARNING" consistent with the VM's "Warning" so > > that OutputAnalyzer's logic to ignore warnings will automatically ignore > > these too. > > The uppercase "WARNING" is intentional here, it was the same with illegal > reflective access warnings. I'm sure Max has or will run all tests to see if > there are any issues. Will definitely run all from tier1-tier9. I ran them multiple times while implementing JEP 411. I've seen warnings with "VM" word in the prefix and test methods that filter them out, but feel the warnings here are not related to VM. The new warnings do have impacts on some tests and I'll be very carefully not break them. - PR: https://git.openjdk.java.net/jdk/pull/4400
Re: RFR: 8268349: Provide more detail in JEP 411 warning messages
On Mon, 7 Jun 2021 20:42:53 GMT, Weijun Wang wrote: > More loudly and precise warning messages when a security manager is either > enabled at startup or installed at runtime. Changes to LoggerFinderLoaderTest look reasonable to me. - PR: https://git.openjdk.java.net/jdk/pull/4400
Re: RFR: 8268349: Provide more detail in JEP 411 warning messages
On Tue, 8 Jun 2021 06:30:00 GMT, David Holmes wrote: > You might want to make your "WARNING" consistent with the VM's "Warning" so > that OutputAnalyzer's logic to ignore warnings will automatically ignore > these too. The uppercase "WARNING" is intentional here, it was the same with illegal reflective access warnings. I'm sure Max has or will run all tests to see if there are any issues. - PR: https://git.openjdk.java.net/jdk/pull/4400
Re: RFR: 8268349: Provide more detail in JEP 411 warning messages
On Mon, 7 Jun 2021 20:42:53 GMT, Weijun Wang wrote: > More loudly and precise warning messages when a security manager is either > enabled at startup or installed at runtime. There are a number of hotspot tests that will trigger this warning, so please ensure they work correctly with the extra output. You might want to make your "WARNING" consistent with the VM's "Warning" so that OutputAnalyzer's logic to ignore warnings will automatically ignore these too. Thanks, David - PR: https://git.openjdk.java.net/jdk/pull/4400
Re: RFR: 8268349: Provide more detail in JEP 411 warning messages
On Mon, 7 Jun 2021 20:42:53 GMT, Weijun Wang wrote: > More loudly and precise warning messages when a security manager is either > enabled at startup or installed at runtime. Changes requested by alanb (Reviewer). src/java.base/share/classes/java/lang/System.java line 331: > 329: > 330: // Remember original System.err. setSecurityManager() warning goes > here > 331: private static PrintStream oldErrStream = null; I assume this should needs to be volatile and @Stable. I think we need a better name for it too. src/java.base/share/classes/java/lang/System.java line 336: > 334: // Remember callers of setSecurityManager() here so that warning > 335: // is only printed once for each different caller > 336: final static Map callersOfSSM = new > WeakHashMap<>(); You can't use a WeakHashMap without synchronization but a big question here is whether a single caller frame is sufficient. If I were doing this then I think I would capture the hash of a number of stack frames to create a better filter. src/java.base/share/classes/java/lang/System.java line 2219: > 2217: WARNING: java.lang.SecurityManager is > deprecated and will be removed in a future release > 2218: WARNING: -Djava.security.manager=%s > will have no effect when java.lang.SecurityManager is removed > 2219: """, smProp); Raw strings may be useful here but means the lines length are inconsistent and makes it too hard to look at side by side diffs now. - PR: https://git.openjdk.java.net/jdk/pull/4400
