Re: RFR: 8268698: Use Objects.check{Index, FromToIndex, FromIndexSize} where possible [v8]
> After JDK-8265518(#3615), it's possible to replace all variants of checkIndex > by Objects.checkIndex/Objects.checkFromToIndex/Objects.checkFromIndexSize in > the whole JDK codebase. Yi Yang has updated the pull request incrementally with one additional commit since the last revision: tests rely on IOOBE exception message - Changes: - all: https://git.openjdk.java.net/jdk/pull/4507/files - new: https://git.openjdk.java.net/jdk/pull/4507/files/2330ee38..ab1b509d Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=4507=07 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4507=06-07 Stats: 104 lines in 2 files changed: 18 ins; 2 del; 84 mod Patch: https://git.openjdk.java.net/jdk/pull/4507.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4507/head:pull/4507 PR: https://git.openjdk.java.net/jdk/pull/4507
Authorization layer API and low level access checks.
Java developers such as myself need a light weight API that allows
developers to continue to support authorization and access controls,
without dictating how that should be implemented or whether these
controls are fine grained, course grained, based solely on user
authorization or also includes code authorization.
SecurityManager has been deprecated, we need to commence removal of
dependencies on deprecated Java API's, however we are unable to make a
decision on how to proceed without understanding the level of support
that OpenJDK will provide for an authorization layer in future. (If this
is zero, we at least need to know).
Currently there is no such API that allows developers who require an
authorization layer to continue to supporting Java 8 as well future
versions Java with one development codebase. It's a non goal of this to
debate the need for cross version support. I simply wish to open
discussions on alternatives and whether OpenJDK is considering them.
SecurityManager API low level functionality replacements:
1. StackWalker - Can stack walker be back ported to Java 8?
2. Permission checks - Can we have low level Guard service hooks to
replace existing permission checks?
Note: I'm not sure how to replace an inherited AccessControlContext
(with a new implementation based on StackWalker functionality) at thread
creation time, as it must be created when threads are created, possibly
by using ThreadFactory everywhere, but this doesn't cover all threads.
How to cater for virtual threads?
For replacement of permission checks, I propose using a Guard service
authorization API (feel free to propose alternatives).
The proposed authorization layer API would utilize the existing Provider
Service mechanism to register authorization layer hooks, for use in
permission checks by JDK code, and library code that implements their
own Permission's:
GuardFactory runtimeGuardFactory = GuardFactory.getInstance("RUNTIME");
Guard createClassLoader =
runtimeGuardFactory.orders("createClassLoader", null);
// Permission check
createClassLoader.check();
Guard exitVM = runtimeGuardFactory.orders("exitVM", null);
exitVM.check();
GuardFactory socketGuardFactory = GuardFactory.getInstance("SOCKET");
// Permission check
socketGuardFactory.orders(host, action).check();
GuardFactory fileGuardFactory = GuardFactory.getInstance("FILE");
fileGuardFactory.orders(path, actions).check();
Guard service hooks, are based on existing Permission types (independent
instances to avoid circular deadlocks), developers only need implement
those relevant to them and may only use checks for users if they wish:
"AWT"
"FILE"
"SERIALIZABLE"
"MANAGEMENT"
"REFLECT"
"RUNTIME"
"NET"
"SOCKET"
"URL"
"FILE-LINK"
"SECURITY"
"SQL"
"LOGGING"
"PROPERTY"
"MBEAN"
"MBEAN-SERVER"
"MBEAN-TRUST"
"SUBJECT-DELEGATION"
"TLS"
"AUTH"
"KERBEROS-DELEGATION"
"KERBEROS-SERVICE"
"PRIVATE-CREDENTIAL"
"AUDIO"
"JAXB"
"WEB-SERVICE"
I would like to suggest adding a new provider type:
"PARSE-DATA" - To be called by any code about to parse data, eg
deserialization, XML, JSON, SQL, etc. The use case for this is for
servers to grant it to authenticated users (user supplied input data),
so that it can only be performed following user authentication.
Existing Permission implementations
--
Regards,
Peter Firmstone
Re: RFR: 8268427: Improve AlgorithmConstraints:checkAlgorithm performance [v5]
On Tue, 22 Jun 2021 06:41:02 GMT, Dongbo He wrote: >> Now AlgorithmConstraints:checkAlgorithm uses List to check if an algorithm >> has been disabled. It is less efficient when there are more disabled >> elements in the list, we can use Set instead of List to speed up the search. >> >> Patch contains a benchmark that can be run with `make test >> TEST="micro:java.security.AlgorithmConstraintsPermits"`. >> Baseline results before patch: >> >> Benchmark(algorithm) Mode CntScore >> Error Units >> AlgorithmConstraintsPermits.permitsSSLv3 avgt5 21.687 ? >> 1.118 ns/op >> AlgorithmConstraintsPermits.permits DES avgt5 324.216 ? >> 6.233 ns/op >> AlgorithmConstraintsPermits.permits NULL avgt5 709.462 ? >> 51.259 ns/op >> AlgorithmConstraintsPermits.permits TLS1.3 avgt5 687.497 ? >> 170.181 ns/op >> >> Benchmark results after patch: >> >> Benchmark(algorithm) Mode CntScore >> Error Units >> AlgorithmConstraintsPermits.permitsSSLv3 avgt5 46.407 ? >> 1.057 ns/op >> AlgorithmConstraintsPermits.permits DES avgt5 65.722 ? >> 0.578 ns/op >> AlgorithmConstraintsPermits.permits NULL avgt5 43.988 ? >> 1.264 ns/op >> AlgorithmConstraintsPermits.permits TLS1.3 avgt5 399.546 ? >> 11.194 ns/op >> >> SSLv3, DES, NULL are the first, middle and last element in >> `jdk.tls.disabledAlgorithms` from `conf/security/java.security`. >> >> Tomcat(maxKeepAliveRequests=1, which will disable HTTP/1.0 >> keep-alive)+Jmeter: >> Before patch: >> >> summary + 50349 in 00:00:30 = 1678.4/s Avg: 238 Min: 188 Max: 298 >> Err: 0 (0.00%) Active: 400 Started: 400 Finished: 0 >> summary = 135183 in 00:01:22 = 1654.5/s Avg: 226 Min:16 Max: 1281 >> Err: 0 (0.00%) >> summary + 50240 in 00:00:30 = 1674.1/s Avg: 238 Min: 200 Max: 308 >> Err: 0 (0.00%) Active: 400 Started: 400 Finished: 0 >> summary = 185423 in 00:01:52 = 1659.7/s Avg: 229 Min:16 Max: 1281 >> Err: 0 (0.00%) >> summary + 50351 in 00:00:30 = 1678.4/s Avg: 238 Min: 191 Max: 306 >> Err: 0 (0.00%) Active: 400 Started: 400 Finished: 0 >> summary = 235774 in 00:02:22 = 1663.7/s Avg: 231 Min:16 Max: 1281 >> Err: 0 (0.00%) >> summary + 50461 in 00:00:30 = 1681.9/s Avg: 237 Min: 174 Max: 303 >> Err: 0 (0.00%) Active: 400 Started: 400 Finished: 0 >> >> After patch: >> >> summary + 59003 in 00:00:30 = 1966.6/s Avg: 203 Min: 158 Max: 272 >> Err: 0 (0.00%) Active: 400 Started: 400 Finished: 0 >> summary = 146675 in 00:01:18 = 1884.6/s Avg: 198 Min:26 Max: 697 >> Err: 0 (0.00%) >> summary + 58965 in 00:00:30 = 1965.9/s Avg: 203 Min: 166 Max: 257 >> Err: 0 (0.00%) Active: 400 Started: 400 Finished: 0 >> summary = 205640 in 00:01:48 = 1907.2/s Avg: 199 Min:26 Max: 697 >> Err: 0 (0.00%) >> summary + 59104 in 00:00:30 = 1969.1/s Avg: 203 Min: 157 Max: 266 >> Err: 0 (0.00%) Active: 400 Started: 400 Finished: 0 >> summary = 264744 in 00:02:18 = 1920.7/s Avg: 200 Min:26 Max: 697 >> Err: 0 (0.00%) >> summary + 59323 in 00:00:30 = 1977.6/s Avg: 202 Min: 158 Max: 256 >> Err: 0 (0.00%) Active: 400 Started: 400 Finished: 0 >> >> >> Testing: tier1, tier2 > > Dongbo He has updated the pull request incrementally with one additional > commit since the last revision: > > fix the code style Thank you for your review, Xuelei. Hi, mullan, ascarpino. Can I get some comments from you? @seanjmullan @ascarpino - PR: https://git.openjdk.java.net/jdk/pull/4424
Integrated: Merge jdk17
On Wed, 23 Jun 2021 00:21:57 GMT, Jesper Wilhelmsson wrote: > Forwardport JDK 17 -> JDK 18 This pull request has now been integrated. Changeset: b6cfca8a Author:Jesper Wilhelmsson URL: https://git.openjdk.java.net/jdk/commit/b6cfca8a89810c7ed63ebc34ed9855b66ebcb5d9 Stats: 1931 lines in 60 files changed: 653 ins; 1061 del; 217 mod Merge - PR: https://git.openjdk.java.net/jdk/pull/4562
Re: RFR: Merge jdk17 [v2]
> Forwardport JDK 17 -> JDK 18 Jesper Wilhelmsson has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 59 commits: - Merge - 8268290: Improve LockFreeQueue<> utility Reviewed-by: iwalulya, tschatzl - 8264941: Remove CodeCache::mark_for_evol_deoptimization() method Reviewed-by: kvn, vlivanov, sspitsyn - 8269031: linux x86_64 check for binutils 2.25 or higher after 8265783 Reviewed-by: ihse, erikj - 8267657: Add missing PrintC1Statistics before incrementing counters Reviewed-by: iveresov - 8268857: Merge VM_PrintJNI and VM_PrintThreads and remove the unused field 'is_deadlock' of DeadlockCycle Reviewed-by: dholmes - 8269077: TestSystemGC uses "require vm.gc.G1" for large pages subtest Reviewed-by: tschatzl, kbarrett - Merge - 8268458: Add verification type for evacuation failures Reviewed-by: kbarrett, iwalulya - 8268952: Automatically update heap sizes in G1MonitoringScope Reviewed-by: kbarrett, iwalulya - ... and 49 more: https://git.openjdk.java.net/jdk/compare/35e4c272...7bf4b35f - Changes: https://git.openjdk.java.net/jdk/pull/4562/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=4562=01 Stats: 16267 lines in 261 files changed: 13210 ins; 2433 del; 624 mod Patch: https://git.openjdk.java.net/jdk/pull/4562.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4562/head:pull/4562 PR: https://git.openjdk.java.net/jdk/pull/4562
RFR: Merge jdk17
Forwardport JDK 17 -> JDK 18 - Commit messages: - Merge - 8268404: [TESTBUG] tools/jpackage/windows/WinInstallerIconTest.java failed "AssertionError: Failed: Check icon" - 8267652: c2 loop unrolling by 8 results in reading memory past array - 8267399: C2: java/text/Normalizer/ConformanceTest.java test failed with assertion - 826: Upstream 8268230: Foreign Linker API & Windows user32/kernel32: String conversion seems broken - 8268524: nmethod::post_compiled_method_load_event racingly called on zombie - 8266631: StandardJavaFileManager: getJavaFileObjects() impl violates the spec - 8267421: j.l.constant.DirectMethodHandleDesc.Kind.valueOf(int) implementation doesn't conform to the spec regarding REF_invokeInterface handling - 8268349: Provide clear run-time warnings about Security Manager deprecation - 8268293: VectorAPI cast operation on mask and shuffle is broken - ... and 1 more: https://git.openjdk.java.net/jdk/compare/0c693e2f...7bf4b35f The merge commit only contains trivial merges, so no merge-specific webrevs have been generated. Changes: https://git.openjdk.java.net/jdk/pull/4562/files Stats: 1931 lines in 60 files changed: 653 ins; 1061 del; 217 mod Patch: https://git.openjdk.java.net/jdk/pull/4562.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4562/head:pull/4562 PR: https://git.openjdk.java.net/jdk/pull/4562
Re: [jdk17] RFR: 8269034: AccessControlException for SunPKCS11 daemon threads
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=117=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
Re: [jdk17] RFR: 8269034: AccessControlException for SunPKCS11 daemon threads [v2]
> 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 Sean Coffey has updated the pull request incrementally with one additional commit since the last revision: Move TokenPoller to Runnable - Changes: - all: https://git.openjdk.java.net/jdk17/pull/117/files - new: https://git.openjdk.java.net/jdk17/pull/117/files/2a168946..03af6494 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk17=117=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk17=117=00-01 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 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
Re: RFR: 8268698: Use Objects.check{Index, FromToIndex, FromIndexSize} where possible [v7]
On Tue, 22 Jun 2021 02:58:28 GMT, Yi Yang wrote: > I found that after solving the problem that Preconditions cannot be used > during the VM startup, a series of functions such as > String.checkIndex/checkOffset/.. can also be harmlessly replaced, but this > changeset is somewhat large and may overwrite the previous review progress. > If it will confuse the current review progress for reviewers involving in > this PR, I'd like to file a new PR to do this change later. Yes, I think that helps to review. It's often easier, review-wise, to have separate PRs for specific areas, rather than keep expanding an existing PR. - PR: https://git.openjdk.java.net/jdk/pull/4507
Re: RFR: 8266182: Create a manual test for jdk/sun/security/pkcs12/ParamsTest.java [v3]
On Fri, 18 Jun 2021 13:24:17 GMT, Abdul Kolarkunnu wrote: >> 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. > > Abdul Kolarkunnu has updated the pull request incrementally with one > additional commit since the last revision: > > 8266182: Create a manual test for jdk/sun/security/pkcs12/ParamsTest.java Marked as reviewed by xuelei (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/4413
Re: RFR: 8268967: Update java.security to use switch expressions
On Tue, 22 Jun 2021 10:56:00 GMT, Patrick Concannon wrote: > Hi, > > Could someone please review my code for updating the code in the > `java.security` packages to make use of the switch expressions? > > Kind regards, > Patrick Looks good to me. Thanks for the nice update. - Marked as reviewed by xuelei (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/4553
Re: RFR: 8268427: Improve AlgorithmConstraints:checkAlgorithm performance [v5]
On Tue, 22 Jun 2021 06:41:02 GMT, Dongbo He wrote: >> Now AlgorithmConstraints:checkAlgorithm uses List to check if an algorithm >> has been disabled. It is less efficient when there are more disabled >> elements in the list, we can use Set instead of List to speed up the search. >> >> Patch contains a benchmark that can be run with `make test >> TEST="micro:java.security.AlgorithmConstraintsPermits"`. >> Baseline results before patch: >> >> Benchmark(algorithm) Mode CntScore >> Error Units >> AlgorithmConstraintsPermits.permitsSSLv3 avgt5 21.687 ? >> 1.118 ns/op >> AlgorithmConstraintsPermits.permits DES avgt5 324.216 ? >> 6.233 ns/op >> AlgorithmConstraintsPermits.permits NULL avgt5 709.462 ? >> 51.259 ns/op >> AlgorithmConstraintsPermits.permits TLS1.3 avgt5 687.497 ? >> 170.181 ns/op >> >> Benchmark results after patch: >> >> Benchmark(algorithm) Mode CntScore >> Error Units >> AlgorithmConstraintsPermits.permitsSSLv3 avgt5 46.407 ? >> 1.057 ns/op >> AlgorithmConstraintsPermits.permits DES avgt5 65.722 ? >> 0.578 ns/op >> AlgorithmConstraintsPermits.permits NULL avgt5 43.988 ? >> 1.264 ns/op >> AlgorithmConstraintsPermits.permits TLS1.3 avgt5 399.546 ? >> 11.194 ns/op >> >> SSLv3, DES, NULL are the first, middle and last element in >> `jdk.tls.disabledAlgorithms` from `conf/security/java.security`. >> >> Tomcat(maxKeepAliveRequests=1, which will disable HTTP/1.0 >> keep-alive)+Jmeter: >> Before patch: >> >> summary + 50349 in 00:00:30 = 1678.4/s Avg: 238 Min: 188 Max: 298 >> Err: 0 (0.00%) Active: 400 Started: 400 Finished: 0 >> summary = 135183 in 00:01:22 = 1654.5/s Avg: 226 Min:16 Max: 1281 >> Err: 0 (0.00%) >> summary + 50240 in 00:00:30 = 1674.1/s Avg: 238 Min: 200 Max: 308 >> Err: 0 (0.00%) Active: 400 Started: 400 Finished: 0 >> summary = 185423 in 00:01:52 = 1659.7/s Avg: 229 Min:16 Max: 1281 >> Err: 0 (0.00%) >> summary + 50351 in 00:00:30 = 1678.4/s Avg: 238 Min: 191 Max: 306 >> Err: 0 (0.00%) Active: 400 Started: 400 Finished: 0 >> summary = 235774 in 00:02:22 = 1663.7/s Avg: 231 Min:16 Max: 1281 >> Err: 0 (0.00%) >> summary + 50461 in 00:00:30 = 1681.9/s Avg: 237 Min: 174 Max: 303 >> Err: 0 (0.00%) Active: 400 Started: 400 Finished: 0 >> >> After patch: >> >> summary + 59003 in 00:00:30 = 1966.6/s Avg: 203 Min: 158 Max: 272 >> Err: 0 (0.00%) Active: 400 Started: 400 Finished: 0 >> summary = 146675 in 00:01:18 = 1884.6/s Avg: 198 Min:26 Max: 697 >> Err: 0 (0.00%) >> summary + 58965 in 00:00:30 = 1965.9/s Avg: 203 Min: 166 Max: 257 >> Err: 0 (0.00%) Active: 400 Started: 400 Finished: 0 >> summary = 205640 in 00:01:48 = 1907.2/s Avg: 199 Min:26 Max: 697 >> Err: 0 (0.00%) >> summary + 59104 in 00:00:30 = 1969.1/s Avg: 203 Min: 157 Max: 266 >> Err: 0 (0.00%) Active: 400 Started: 400 Finished: 0 >> summary = 264744 in 00:02:18 = 1920.7/s Avg: 200 Min:26 Max: 697 >> Err: 0 (0.00%) >> summary + 59323 in 00:00:30 = 1977.6/s Avg: 202 Min: 158 Max: 256 >> Err: 0 (0.00%) Active: 400 Started: 400 Finished: 0 >> >> >> Testing: tier1, tier2 > > Dongbo He has updated the pull request incrementally with one additional > commit since the last revision: > > fix the code style Looks good to me. Thank you! - Marked as reviewed by xuelei (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/4424
[jdk17] RFR: 8269034: AccessControlException for SunPKCS11 daemon threads
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=117=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
Re: RFR: 8268873: Unnecessary Vector usage in java.base
On Mon, 14 Jun 2021 11:34:50 GMT, Andrey Turbanov wrote: > Usage of thread-safe collection `Vector` is unnecessary. It's recommended to > use `ArrayList` if a thread-safe implementation is not needed. > I checked only places where `Vector` was used as local variable. Marked as reviewed by stsypa...@github.com (no known OpenJDK username). - PR: https://git.openjdk.java.net/jdk/pull/4482
Re: RFR: 8269034: AccessControlException for SunPKCS11 daemon threads
On Tue, 22 Jun 2021 12:01:07 GMT, 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 Please ignore - I'm going to open a PR against jdk17 for this - PR: https://git.openjdk.java.net/jdk/pull/4555
Withdrawn: 8269034: AccessControlException for SunPKCS11 daemon threads
On Tue, 22 Jun 2021 12:01:07 GMT, 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 This pull request has been closed without being integrated. - PR: https://git.openjdk.java.net/jdk/pull/4555
RFR: 8269034: AccessControlException for SunPKCS11 daemon threads
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/jdk/pull/4555/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=4555=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/jdk/pull/4555.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4555/head:pull/4555 PR: https://git.openjdk.java.net/jdk/pull/4555
RFR: 8268967: Update java.security to use switch expressions
Hi, Could someone please review my code for updating the code in the `java.security` packages to make use of the switch expressions? Kind regards, Patrick - Commit messages: - Merge remote-tracking branch 'origin/master' into JDK-8268967 - 8268967: Update java.security to use switch expressions Changes: https://git.openjdk.java.net/jdk/pull/4553/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=4553=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8268967 Stats: 107 lines in 3 files changed: 0 ins; 61 del; 46 mod Patch: https://git.openjdk.java.net/jdk/pull/4553.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4553/head:pull/4553 PR: https://git.openjdk.java.net/jdk/pull/4553
Re: RFR: 8268427: Improve AlgorithmConstraints:checkAlgorithm performance [v4]
On Tue, 22 Jun 2021 03:45:32 GMT, Xue-Lei Andrew Fan wrote: >> Dongbo He has updated the pull request incrementally with one additional >> commit since the last revision: >> >> refactor the code && fix some errors > > src/java.base/share/classes/sun/security/util/DisabledAlgorithmConstraints.java > line 133: > >> 131: >> 132: // Check for alias >> 133: Pattern INCLUDE_PATTERN = Pattern.compile( > > You may miss my previous comment. This variable is not static, hence all > capital letters naming does not apply here. > > As you are already here, it may be nice if you'd like to have this variable > as a static final class field. Then, the compile() will be called one time > at most for this class. As a static class field, you could use the > capitalized name. Sorry, I misunderstood what you meant before, I will fix it. Thank you! - PR: https://git.openjdk.java.net/jdk/pull/4424
Re: RFR: 8268427: Improve AlgorithmConstraints:checkAlgorithm performance [v5]
> Now AlgorithmConstraints:checkAlgorithm uses List to check if an algorithm > has been disabled. It is less efficient when there are more disabled elements > in the list, we can use Set instead of List to speed up the search. > > Patch contains a benchmark that can be run with `make test > TEST="micro:java.security.AlgorithmConstraintsPermits"`. > Baseline results before patch: > > Benchmark(algorithm) Mode CntScore > Error Units > AlgorithmConstraintsPermits.permitsSSLv3 avgt5 21.687 ? > 1.118 ns/op > AlgorithmConstraintsPermits.permits DES avgt5 324.216 ? > 6.233 ns/op > AlgorithmConstraintsPermits.permits NULL avgt5 709.462 ? > 51.259 ns/op > AlgorithmConstraintsPermits.permits TLS1.3 avgt5 687.497 ? > 170.181 ns/op > > Benchmark results after patch: > > Benchmark(algorithm) Mode CntScoreError > Units > AlgorithmConstraintsPermits.permitsSSLv3 avgt5 46.407 ? 1.057 > ns/op > AlgorithmConstraintsPermits.permits DES avgt5 65.722 ? 0.578 > ns/op > AlgorithmConstraintsPermits.permits NULL avgt5 43.988 ? 1.264 > ns/op > AlgorithmConstraintsPermits.permits TLS1.3 avgt5 399.546 ? 11.194 > ns/op > > SSLv3, DES, NULL are the first, middle and last element in > `jdk.tls.disabledAlgorithms` from `conf/security/java.security`. > > Tomcat(maxKeepAliveRequests=1, which will disable HTTP/1.0 keep-alive)+Jmeter: > Before patch: > > summary + 50349 in 00:00:30 = 1678.4/s Avg: 238 Min: 188 Max: 298 Err: > 0 (0.00%) Active: 400 Started: 400 Finished: 0 > summary = 135183 in 00:01:22 = 1654.5/s Avg: 226 Min:16 Max: 1281 Err: > 0 (0.00%) > summary + 50240 in 00:00:30 = 1674.1/s Avg: 238 Min: 200 Max: 308 Err: > 0 (0.00%) Active: 400 Started: 400 Finished: 0 > summary = 185423 in 00:01:52 = 1659.7/s Avg: 229 Min:16 Max: 1281 Err: > 0 (0.00%) > summary + 50351 in 00:00:30 = 1678.4/s Avg: 238 Min: 191 Max: 306 Err: > 0 (0.00%) Active: 400 Started: 400 Finished: 0 > summary = 235774 in 00:02:22 = 1663.7/s Avg: 231 Min:16 Max: 1281 Err: > 0 (0.00%) > summary + 50461 in 00:00:30 = 1681.9/s Avg: 237 Min: 174 Max: 303 Err: > 0 (0.00%) Active: 400 Started: 400 Finished: 0 > > After patch: > > summary + 59003 in 00:00:30 = 1966.6/s Avg: 203 Min: 158 Max: 272 Err: > 0 (0.00%) Active: 400 Started: 400 Finished: 0 > summary = 146675 in 00:01:18 = 1884.6/s Avg: 198 Min:26 Max: 697 Err: > 0 (0.00%) > summary + 58965 in 00:00:30 = 1965.9/s Avg: 203 Min: 166 Max: 257 Err: > 0 (0.00%) Active: 400 Started: 400 Finished: 0 > summary = 205640 in 00:01:48 = 1907.2/s Avg: 199 Min:26 Max: 697 Err: > 0 (0.00%) > summary + 59104 in 00:00:30 = 1969.1/s Avg: 203 Min: 157 Max: 266 Err: > 0 (0.00%) Active: 400 Started: 400 Finished: 0 > summary = 264744 in 00:02:18 = 1920.7/s Avg: 200 Min:26 Max: 697 Err: > 0 (0.00%) > summary + 59323 in 00:00:30 = 1977.6/s Avg: 202 Min: 158 Max: 256 Err: > 0 (0.00%) Active: 400 Started: 400 Finished: 0 > > > Testing: tier1, tier2 Dongbo He has updated the pull request incrementally with one additional commit since the last revision: fix the code style - Changes: - all: https://git.openjdk.java.net/jdk/pull/4424/files - new: https://git.openjdk.java.net/jdk/pull/4424/files/b863e2b3..02b09719 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=4424=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4424=03-04 Stats: 9 lines in 1 file changed: 4 ins; 2 del; 3 mod Patch: https://git.openjdk.java.net/jdk/pull/4424.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4424/head:pull/4424 PR: https://git.openjdk.java.net/jdk/pull/4424
