Re: [jdk17] RFR: 8270556: Exclude security/infra/java/security/cert/CertPathValidator/certification/LetsEncryptCA
On Thu, 15 Jul 2021 15:04:04 GMT, Christoph Langer wrote: > The test is failing since 8th of July. Let's exclude it for now. Marked as reviewed by mbaesken (Reviewer). - PR: https://git.openjdk.java.net/jdk17/pull/251
Re: RFR: 8270317: Large Allocation in CipherSuite
On Wed, 14 Jul 2021 21:18:23 GMT, Xin Liu wrote:
>> ### Benchmark results
>>
>> I have benchmarked 3 cases.
>>
>> 1. The current situation.
>>
>> Benchmark
>> (cipherSuite) Mode CntScore Error Units
>> CipherSuiteBench.benchmarkCipherSuite
>> TLS_AES_256_GCM_SHA384 avgt 25 124.783 ? 2.050 ns/op
>> CipherSuiteBench.benchmarkCipherSuite
>> TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA384 avgt 25 125.403 ? 0.554 ns/op
>> CipherSuiteBench.benchmarkCipherSuite
>> TLS_DHE_DSS_WITH_AES_128_CBC_SHA256 avgt 25 127.117 ? 0.789 ns/op
>> CipherSuiteBench.benchmarkCipherSuite
>> TLS_DHE_RSA_WITH_AES_256_CBC_SHA avgt 25 127.869 ? 1.112 ns/op
>>
>>
>> 2. Use `static final array` instead of calling `CipherSuite.values` each
>> time.
>>
>> Benchmark
>> (cipherSuite) Mode Cnt Score Error Units
>> CipherSuiteBench.benchmarkCipherSuite
>> TLS_AES_256_GCM_SHA384 avgt 25 10.146 ? 0.252 ns/op
>> CipherSuiteBench.benchmarkCipherSuite
>> TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA384 avgt 25 30.501 ? 0.207 ns/op
>> CipherSuiteBench.benchmarkCipherSuite
>> TLS_DHE_DSS_WITH_AES_128_CBC_SHA256 avgt 25 47.375 ? 0.150 ns/op
>> CipherSuiteBench.benchmarkCipherSuite
>> TLS_DHE_RSA_WITH_AES_256_CBC_SHA avgt 25 55.887 ? 3.786 ns/op
>>
>>
>> 3. Using Hashmap for lookup instead of iterating through the array each
>> time. (Method in this PR)
>>
>> Benchmark
>> (cipherSuite) Mode Cnt Score Error Units
>> CipherSuiteBench.benchmarkCipherSuite
>> TLS_AES_256_GCM_SHA384 avgt 25 13.533 ? 0.148 ns/op
>> CipherSuiteBench.benchmarkCipherSuite
>> TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA384 avgt 25 11.269 ? 0.147 ns/op
>> CipherSuiteBench.benchmarkCipherSuite
>> TLS_DHE_DSS_WITH_AES_128_CBC_SHA256 avgt 25 11.507 ? 0.107 ns/op
>> CipherSuiteBench.benchmarkCipherSuite
>> TLS_DHE_RSA_WITH_AES_256_CBC_SHA avgt 25 10.932 ? 0.146 ns/op
>>
>>
>> I have picked 4 cipher suite from the start of the list and are roughly 10
>> positions apart. I have opted to go with HashMap for name and id lookup as
>> they provide a more consistent times and benchmarks are similar for the
>> first few cipher suits in the enum as well.
>
> src/java.base/share/classes/sun/security/ssl/CipherSuite.java line 865:
>
>> 863: maps_name.put(cs.name, cs);
>> 864: for (String alias : cs.aliases) {
>> 865: maps_name.put(alias, cs);
>
> A minor concern here. HashMap can't have duplicate keys. what if there are
> duplicated names/aliases?
>
> Even it's not the case now, CipherSuite is subject to change in the future
> and I think it is a good opportunity to detect key duplication here.
> Currently, it's silently overwritten. This may introduce inconsistent
> behavior for nameOf.
this could be potentially stored in immutable collections which might be
slightly more compact + they throw when they encounter duplicate keys
1) change base type to Map
2) copy ciphers array into Map.Entry array
3) maps_id = Map.ofEntries(entries) // + handle IAE
similar for the name map but with a list in between.
there might be a JDK internal shortcut to get to new
ImmutableCollections.MapN<>(flatArray) right away without the Map.Entry step -
I am sure a reviewer will chime in if there is.
-
PR: https://git.openjdk.java.net/jdk/pull/4783
Authorization layer - threads and privileged calls
I'm currently experimenting with a new authorization layer for java, post JEP 411. I would like your thoughts around threads. This is intended to be simpler than Java's existing authorization layer, support user Subjects and code based authorization. Concepts: 1. Application code has no privileges, unless a privileged call is made (implements Callable), the privileges are only in force during execution of the Callable and are not transferable to other threads. 2. A Thread with a stack that only contains code visible to the platform ClassLoader is considered privileged. 3. Privileged means it has defined privileges, it doesn't mean AllPermission. Agents will be used to instrument the Java API for guard checks (would be nice if OpenJDK can annotate these methods or do something to help us identify these locations). Clearly, this will break a lot of existing code, many applications simply won't run, because they don't utilise the API. It would work fine for new applications. In Java's existing authorization layer implementation (designed prior to the introduction of Executor frameworks), a thread inherits the stack context of the thread which created it, with executors, tasks don't inherit the context of the thread which places the task. The new framework isn't able to capture the creating threads context, so it makes more sense to treat anything outside a privileged call, or system thread as unprivileged, it does however capture the caller when creating a privileged task, this is a Task that has privileged access, so it's important that it is not allowed to escape. I am thinking about allowing privileged domains, such that if a library (which doesn't implement privileged calls), may be thought of as a system domain, should it create threads, then provided those threads only have privileged domains on the stack, guard checks may proceed. For unprivileged application code, all guard checks fail. Any thoughts or questions? -- Regards, Peter Firmstone 0498 286 363 Zeus Project Services Pty Ltd.
Integrated: Merge jdk17
On Thu, 15 Jul 2021 23:42:37 GMT, Jesper Wilhelmsson wrote: > Forwardport JDK 17 -> JDK 18 This pull request has now been integrated. Changeset: 7240d678 Author:Jesper Wilhelmsson URL: https://git.openjdk.java.net/jdk/commit/7240d67868add94c97ca1d7ba372548cd76b8ffc Stats: 29 lines in 1 file changed: 14 ins; 2 del; 13 mod Merge - PR: https://git.openjdk.java.net/jdk/pull/4802
Re: RFR: 8267125: AES Galois CounterMode (GCM) interleaved implementation using AVX512 + VAES instructions [v4]
On Wed, 14 Jul 2021 21:02:01 GMT, Smita Kamath wrote: >> I would like to submit AES-GCM optimization for x86_64 architectures >> supporting AVX3+VAES (Evex encoded AES). This optimization interleaves AES >> and GHASH operations. >> Performance gain of ~1.5x - 2x for message sizes 8k and above. > > Smita Kamath has updated the pull request incrementally with one additional > commit since the last revision: > > Updated AES-GCM intrinsic to match latest Java Code src/java.base/share/classes/com/sun/crypto/provider/GaloisCounterMode.java line 611: > 609: outOfs + len); > 610: ghash.update(ct, ctOfs, segments); > 611: ctOfs = len; This does not look right when the initial value of ctOfs != 0. - PR: https://git.openjdk.java.net/jdk/pull/4019
Re: RFR: 8267125: AES Galois CounterMode (GCM) interleaved implementation using AVX512 + VAES instructions [v4]
On Thu, 15 Jul 2021 22:44:05 GMT, Valerie Peng wrote:
>> Smita Kamath has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Updated AES-GCM intrinsic to match latest Java Code
>
> src/java.base/share/classes/com/sun/crypto/provider/GaloisCounterMode.java
> line 170:
>
>> 168:
>> 169: // always encrypt mode for embedded cipher
>> 170: blockCipher.init(false, key.getAlgorithm(), keyValue);
>
> Is this change intentional? Looks like we are reverting to older version of
> source and undo newer changes.
Nope.. unintentional
> src/java.base/share/classes/com/sun/crypto/provider/GaloisCounterMode.java
> line 472:
>
>> 470: engine = null;
>> 471: if (encodedKey != null) {
>> 472: Arrays.fill(encodedKey, (byte)0);
>
> Looks like another unintentional newer->older change.
I don't remember an old comment about that, dunno if that was reverted
> src/java.base/share/classes/com/sun/crypto/provider/GaloisCounterMode.java
> line 992:
>
>> 990: */
>> 991: byte[] overlapDetection(byte[] in, int inOfs, byte[] out, int
>> outOfs) {
>> 992: if (in == out && (!encryption || inOfs < outOfs)) {
>
> So, we will always allocate an output buffer for decryption if in==out? Why
> just decryption? Update the javadoc for this method with the reason?
If the crypto is decryption in-place, an internal output buffer is needed in
case the auth tag fails, otherwise the input buffer would be zero'ed.
-
PR: https://git.openjdk.java.net/jdk/pull/4019
Re: RFR: 8267125: AES Galois CounterMode (GCM) interleaved implementation using AVX512 + VAES instructions [v4]
On Wed, 14 Jul 2021 21:02:01 GMT, Smita Kamath wrote: >> I would like to submit AES-GCM optimization for x86_64 architectures >> supporting AVX3+VAES (Evex encoded AES). This optimization interleaves AES >> and GHASH operations. >> Performance gain of ~1.5x - 2x for message sizes 8k and above. > > Smita Kamath has updated the pull request incrementally with one additional > commit since the last revision: > > Updated AES-GCM intrinsic to match latest Java Code src/java.base/share/classes/com/sun/crypto/provider/GaloisCounterMode.java line 589: > 587: * Requires 768 bytes (48 AES blocks) to efficiently use the > intrinsic > 588: * @param in input buffer > 589: * @param inOfs input offset missed @param inLen src/java.base/share/classes/com/sun/crypto/provider/GaloisCounterMode.java line 594: > 592: * @param out output buffer > 593: * @param outOfs output offset > 594: * @param gctr object for the CTR operation typo: CTR->GCTR? - PR: https://git.openjdk.java.net/jdk/pull/4019
RFR: Merge jdk17
Forwardport JDK 17 -> JDK 18 - Commit messages: - Merge - 8268635: Corrupt oop in ClassLoaderData - 8269276: Additional tests for MessageDigest with different providers The merge commit only contains trivial merges, so no merge-specific webrevs have been generated. Changes: https://git.openjdk.java.net/jdk/pull/4802/files Stats: 29 lines in 1 file changed: 14 ins; 2 del; 13 mod Patch: https://git.openjdk.java.net/jdk/pull/4802.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4802/head:pull/4802 PR: https://git.openjdk.java.net/jdk/pull/4802
Re: RFR: 8267125: AES Galois CounterMode (GCM) interleaved implementation using AVX512 + VAES instructions [v4]
On Wed, 14 Jul 2021 21:02:01 GMT, Smita Kamath wrote:
>> I would like to submit AES-GCM optimization for x86_64 architectures
>> supporting AVX3+VAES (Evex encoded AES). This optimization interleaves AES
>> and GHASH operations.
>> Performance gain of ~1.5x - 2x for message sizes 8k and above.
>
> Smita Kamath has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Updated AES-GCM intrinsic to match latest Java Code
src/java.base/share/classes/com/sun/crypto/provider/GaloisCounterMode.java line
992:
> 990: */
> 991: byte[] overlapDetection(byte[] in, int inOfs, byte[] out, int
> outOfs) {
> 992: if (in == out && (!encryption || inOfs < outOfs)) {
So, we will always allocate an output buffer for decryption if in==out? Why
just decryption? Update the javadoc for this method with the reason?
-
PR: https://git.openjdk.java.net/jdk/pull/4019
Re: RFR: 8267125: AES Galois CounterMode (GCM) interleaved implementation using AVX512 + VAES instructions [v4]
On Wed, 14 Jul 2021 21:02:01 GMT, Smita Kamath wrote: >> I would like to submit AES-GCM optimization for x86_64 architectures >> supporting AVX3+VAES (Evex encoded AES). This optimization interleaves AES >> and GHASH operations. >> Performance gain of ~1.5x - 2x for message sizes 8k and above. > > Smita Kamath has updated the pull request incrementally with one additional > commit since the last revision: > > Updated AES-GCM intrinsic to match latest Java Code src/java.base/share/classes/com/sun/crypto/provider/GaloisCounterMode.java line 170: > 168: > 169: // always encrypt mode for embedded cipher > 170: blockCipher.init(false, key.getAlgorithm(), keyValue); Is this change intentional? Looks like we are reverting to older version of source and undo newer changes. - PR: https://git.openjdk.java.net/jdk/pull/4019
Re: RFR: 8267125: AES Galois CounterMode (GCM) interleaved implementation using AVX512 + VAES instructions [v4]
On Wed, 14 Jul 2021 21:02:01 GMT, Smita Kamath wrote:
>> I would like to submit AES-GCM optimization for x86_64 architectures
>> supporting AVX3+VAES (Evex encoded AES). This optimization interleaves AES
>> and GHASH operations.
>> Performance gain of ~1.5x - 2x for message sizes 8k and above.
>
> Smita Kamath has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Updated AES-GCM intrinsic to match latest Java Code
src/java.base/share/classes/com/sun/crypto/provider/GaloisCounterMode.java line
472:
> 470: engine = null;
> 471: if (encodedKey != null) {
> 472: Arrays.fill(encodedKey, (byte)0);
Looks like another unintentional newer->older change.
-
PR: https://git.openjdk.java.net/jdk/pull/4019
Re: [jdk17] RFR: 8267100: [BACKOUT] JDK-8196415 Disable SHA-1 Signed JARs
On Fri, 18 Jun 2021 15:21:06 GMT, Sean Mullan wrote: > Please review this fix to backout the change to Disable SHA-1 Signed JARs > from JDK 17 due to a startup performance regression (see > https://bugs.openjdk.java.net/browse/JDK-8266971). The plan is to now address > the performance issue in JDK 18, which will also allow for more bake time. @seanjmullan Since the fix for this bug was integrated via PR #113 can you close this PR to withdraw it? - PR: https://git.openjdk.java.net/jdk17/pull/100
Re: RFR: 8270317: Large Allocation in CipherSuite
On Thu, 15 Jul 2021 17:51:36 GMT, djelinski wrote: >> That's what containsKey() does, returning false of the value is null. > > Right. But calling `containsKey` and then `get` results in two hash map > lookups. If we cache the result of `get`, we only need one lookup, which > should be a tiny bit faster. See https://stackoverflow.com/q/14601016 I feel that by checking for null, we can reduce one lookup at the Hashtable. Will update the pull request. - PR: https://git.openjdk.java.net/jdk/pull/4783
Re: RFR: 8270317: Large Allocation in CipherSuite
On Thu, 15 Jul 2021 16:39:14 GMT, Anthony Scarpino
wrote:
>> src/java.base/share/classes/sun/security/ssl/CipherSuite.java line 916:
>>
>>> 914: static String nameOf(int id) {
>>> 915: if (maps_id.containsKey(id)) {
>>> 916: return maps_id.get(id).name;
>>
>> Would it make sense to skip `containsKey` and null-check the value returned
>> by `get` instead?
>
> That's what containsKey() does, returning false of the value is null.
Right. But calling `containsKey` and then `get` results in two hash map
lookups. If we cache the result of `get`, we only need one lookup, which should
be a tiny bit faster. See https://stackoverflow.com/q/14601016
-
PR: https://git.openjdk.java.net/jdk/pull/4783
Re: RFR: 8270317: Large Allocation in CipherSuite
On Wed, 14 Jul 2021 19:30:23 GMT, Xue-Lei Andrew Fan wrote: > Hi Clive Verghese, > > The performance improve is impressive to me. Would you mind have an > additional benchmark for the throughput (@BenchmarkMode(Mode.Throughput))? I > guess the throughput should be good as well. > > Thanks! Sure, I'll add the Throughput mode and run the benchmarks. > The benchmark you provided looks a bit odd... In variant 1 best and worst > cases differ by 3 ns, and in variant 2 they differ by 45 ns. The algorithm is > supposed to be the same, so... Where does the difference come from? I'll rerun the benchmarks with larger iterations and investigate a bit further to understand this difference. Thank you for point it out. - PR: https://git.openjdk.java.net/jdk/pull/4783
Re: RFR: 8270317: Large Allocation in CipherSuite
On Wed, 14 Jul 2021 20:21:58 GMT, djelinski
wrote:
>> ### Benchmark results
>>
>> I have benchmarked 3 cases.
>>
>> 1. The current situation.
>>
>> Benchmark
>> (cipherSuite) Mode CntScore Error Units
>> CipherSuiteBench.benchmarkCipherSuite
>> TLS_AES_256_GCM_SHA384 avgt 25 124.783 ? 2.050 ns/op
>> CipherSuiteBench.benchmarkCipherSuite
>> TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA384 avgt 25 125.403 ? 0.554 ns/op
>> CipherSuiteBench.benchmarkCipherSuite
>> TLS_DHE_DSS_WITH_AES_128_CBC_SHA256 avgt 25 127.117 ? 0.789 ns/op
>> CipherSuiteBench.benchmarkCipherSuite
>> TLS_DHE_RSA_WITH_AES_256_CBC_SHA avgt 25 127.869 ? 1.112 ns/op
>>
>>
>> 2. Use `static final array` instead of calling `CipherSuite.values` each
>> time.
>>
>> Benchmark
>> (cipherSuite) Mode Cnt Score Error Units
>> CipherSuiteBench.benchmarkCipherSuite
>> TLS_AES_256_GCM_SHA384 avgt 25 10.146 ? 0.252 ns/op
>> CipherSuiteBench.benchmarkCipherSuite
>> TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA384 avgt 25 30.501 ? 0.207 ns/op
>> CipherSuiteBench.benchmarkCipherSuite
>> TLS_DHE_DSS_WITH_AES_128_CBC_SHA256 avgt 25 47.375 ? 0.150 ns/op
>> CipherSuiteBench.benchmarkCipherSuite
>> TLS_DHE_RSA_WITH_AES_256_CBC_SHA avgt 25 55.887 ? 3.786 ns/op
>>
>>
>> 3. Using Hashmap for lookup instead of iterating through the array each
>> time. (Method in this PR)
>>
>> Benchmark
>> (cipherSuite) Mode Cnt Score Error Units
>> CipherSuiteBench.benchmarkCipherSuite
>> TLS_AES_256_GCM_SHA384 avgt 25 13.533 ? 0.148 ns/op
>> CipherSuiteBench.benchmarkCipherSuite
>> TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA384 avgt 25 11.269 ? 0.147 ns/op
>> CipherSuiteBench.benchmarkCipherSuite
>> TLS_DHE_DSS_WITH_AES_128_CBC_SHA256 avgt 25 11.507 ? 0.107 ns/op
>> CipherSuiteBench.benchmarkCipherSuite
>> TLS_DHE_RSA_WITH_AES_256_CBC_SHA avgt 25 10.932 ? 0.146 ns/op
>>
>>
>> I have picked 4 cipher suite from the start of the list and are roughly 10
>> positions apart. I have opted to go with HashMap for name and id lookup as
>> they provide a more consistent times and benchmarks are similar for the
>> first few cipher suits in the enum as well.
>
> src/java.base/share/classes/sun/security/ssl/CipherSuite.java line 916:
>
>> 914: static String nameOf(int id) {
>> 915: if (maps_id.containsKey(id)) {
>> 916: return maps_id.get(id).name;
>
> Would it make sense to skip `containsKey` and null-check the value returned
> by `get` instead?
That's what containsKey() does, returning false of the value is null.
-
PR: https://git.openjdk.java.net/jdk/pull/4783
[jdk17] RFR: 8270556: Exclude security/infra/java/security/cert/CertPathValidator/certification/LetsEncryptCA
The test is failing since 8th of July. Let's exclude it for now. - Commit messages: - 8270556: Exclude security/infra/java/security/cert/CertPathValidator/certification/LetsEncryptCA Changes: https://git.openjdk.java.net/jdk17/pull/251/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk17=251=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8270556 Stats: 1 line in 1 file changed: 1 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk17/pull/251.diff Fetch: git fetch https://git.openjdk.java.net/jdk17 pull/251/head:pull/251 PR: https://git.openjdk.java.net/jdk17/pull/251
RFR: 8209776: Refactor jdk/security/JavaDotSecurity/ifdefs.sh to plain java test
This change converts security/JavaDotSecurity/ifdefs.sh to java equivalent - Commit messages: - refactored to java Changes: https://git.openjdk.java.net/jdk/pull/4791/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=4791=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8209776 Stats: 159 lines in 2 files changed: 101 ins; 58 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/4791.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4791/head:pull/4791 PR: https://git.openjdk.java.net/jdk/pull/4791
