Re: RFR: 8267125: AES Galois CounterMode (GCM) interleaved implementation using AVX512 + VAES instructions [v12]
On Fri, 20 Aug 2021 22:43:55 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: > > changes to make sure that ghash_long_swap_mask and counter_mask_addr calls > are not duplicated Good. I will test it. - Marked as reviewed by kvn (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/4019
Re: RFR: 8270380: Change the default value of the java.security.manager system property to disallow
On Fri, 20 Aug 2021 22:44:34 GMT, Weijun Wang wrote:
> This change modifies the default value of the `java.security.manager` system
> property from "allow" to "disallow". This means unless it's explicitly set to
> "allow", any call to `System.setSecurityManager()` would throw an UOE.
>
> The `AllowSecurityManager.java` and `SecurityManagerWarnings.java` tests are
> updated to confirm this behavior change. Two other tests are updated because
> they were added after JDK-8267184 and do not have
> `-Djava.security.manager=allow` on its `@run` line even it they need to
> install one at runtime.
Looks good Max.
One minor suggestion below
src/java.base/share/classes/java/lang/SecurityManager.java line 128:
> 126: * null
> 127: * None
> 128: * Always throws {@code UnsupportedOperationException}
Not sure "Always" is needed, could just be "Throws UOE"
-
Marked as reviewed by lancea (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/5204
RFR: 8270380: Change the default value of the java.security.manager system property to disallow
This change modifies the default value of the `java.security.manager` system property from "allow" to "disallow". This means unless it's explicitly set to "allow", any call to `System.setSecurityManager()` would throw an UOE. The `AllowSecurityManager.java` and `SecurityManagerWarnings.java` tests are updated to confirm this behavior change. Two other tests are updated because they were added after JDK-8267184 and do not have `-Djava.security.manager=allow` on its `@run` line even it they need to install one at runtime. - Commit messages: - 8270380: Change the default value of the java.security.manager system property to disallow Changes: https://git.openjdk.java.net/jdk/pull/5204/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=5204&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8270380 Stats: 24 lines in 6 files changed: 3 ins; 8 del; 13 mod Patch: https://git.openjdk.java.net/jdk/pull/5204.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5204/head:pull/5204 PR: https://git.openjdk.java.net/jdk/pull/5204
Re: RFR: 8267125: AES Galois CounterMode (GCM) interleaved implementation using AVX512 + VAES instructions [v12]
> 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: changes to make sure that ghash_long_swap_mask and counter_mask_addr calls are not duplicated - Changes: - all: https://git.openjdk.java.net/jdk/pull/4019/files - new: https://git.openjdk.java.net/jdk/pull/4019/files/94e46aae..c31bfe9a Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=4019&range=11 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=4019&range=10-11 Stats: 6 lines in 1 file changed: 4 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/4019.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4019/head:pull/4019 PR: https://git.openjdk.java.net/jdk/pull/4019
Integrated: 8270344: Session resumption errors
On Fri, 13 Aug 2021 14:00:45 GMT, Sean Coffey wrote: > Corner case where a session resumption can fail if the TLS server changes > supported protocol versions in relation to a cached SSLSession. This is > primarily an issue where the legacy TLS version is used in place of the newer > "supported_versions" TLS extension. This pull request has now been integrated. Changeset: 04a806ec Author:Sean Coffey URL: https://git.openjdk.java.net/jdk/commit/04a806ec86a388b8de31d42f904c4321beb69e14 Stats: 185 lines in 4 files changed: 162 ins; 21 del; 2 mod 8270344: Session resumption errors Reviewed-by: xuelei - PR: https://git.openjdk.java.net/jdk/pull/5110
Re: RFR: 8270344: Session resumption errors [v5]
On Fri, 20 Aug 2021 16:31:11 GMT, Sean Coffey wrote: >> src/java.base/share/classes/sun/security/ssl/ClientHello.java line 547: >> >>> 545: // handshake output stream, so that the output >>> records >>> 546: // (at the record layer) have the correct version >>> 547: chc.setVersion(sessionVersion); >> >> The removing of the call to "setVersion()" has an impact, I think. I think >> the declaration of this method could be removed in HandshakeContext class, >> and set the HandshakeContext.conContext.protocolVersion to >> HandshakeContext.maximumActiveProtocol in the HandshakeContext.initialize() >> method. > > I've removed the (now) redundant setVersion(..) method. > > With respect to initialize() method we already set > HandshakeContext.conContext.protocolVersion to > HandshakeContext.maximumActiveProtocol in the case of a new session. Is that > what you were aiming at ? > https://github.com/openjdk/jdk/blob/1ea437a4b87381b558cf8157ac52fc03e37ac507/src/java.base/share/classes/sun/security/ssl/HandshakeContext.java#L261 Yes. Thank you! - PR: https://git.openjdk.java.net/jdk/pull/5110
Re: RFR: 8270344: Session resumption errors [v5]
On Fri, 20 Aug 2021 16:34:51 GMT, Sean Coffey wrote: >> Corner case where a session resumption can fail if the TLS server changes >> supported protocol versions in relation to a cached SSLSession. This is >> primarily an issue where the legacy TLS version is used in place of the >> newer "supported_versions" TLS extension. > > Sean Coffey has updated the pull request incrementally with one additional > commit since the last revision: > > Remove redundant method and testcase cleanup Marked as reviewed by xuelei (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/5110
Re: RFR: 8270344: Session resumption errors [v4]
On Thu, 19 Aug 2021 19:51:36 GMT, Xue-Lei Andrew Fan wrote:
>> Sean Coffey has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> maxProtocolVersion refactoring
>
> test/jdk/sun/security/ssl/SSLSessionImpl/InvalidateSession.java line 60:
>
>> 58: System.setProperty("javax.net.ssl.keyStorePassword", passwd);
>> 59: System.setProperty("javax.net.ssl.trustStore", trustFilename);
>> 60: System.setProperty("javax.net.ssl.trustStorePassword", passwd);
>
> It is not recommended to use the binary key store files for JSSE test cases.
> Please refer to test/jdk/javax/net/ssl/templates/SSLContextTemplate.java for
> a replacement.
Good suggestion - done.
> test/jdk/sun/security/ssl/SSLSessionImpl/InvalidateSession.java line 173:
>
>> 171: }
>> 172: }
>> 173: }
>
> Is a new line required in the end of file? I see red symbol in the review
> board, I think the symbol may be generated by the GitHub.
not sure it matters, but added a new line
-
PR: https://git.openjdk.java.net/jdk/pull/5110
Re: RFR: 8270344: Session resumption errors [v5]
On Thu, 19 Aug 2021 19:48:15 GMT, Xue-Lei Andrew Fan wrote: >> Sean Coffey has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Remove redundant method and testcase cleanup > > src/java.base/share/classes/sun/security/ssl/ClientHello.java line 547: > >> 545: // handshake output stream, so that the output >> records >> 546: // (at the record layer) have the correct version >> 547: chc.setVersion(sessionVersion); > > The removing of the call to "setVersion()" has an impact, I think. I think > the declaration of this method could be removed in HandshakeContext class, > and set the HandshakeContext.conContext.protocolVersion to > HandshakeContext.maximumActiveProtocol in the HandshakeContext.initialize() > method. I've removed the (now) redundant setVersion(..) method. With respect to initialize() method we already set HandshakeContext.conContext.protocolVersion to HandshakeContext.maximumActiveProtocol in the case of a new session. Is that what you were aiming at ? https://github.com/openjdk/jdk/blob/1ea437a4b87381b558cf8157ac52fc03e37ac507/src/java.base/share/classes/sun/security/ssl/HandshakeContext.java#L261 - PR: https://git.openjdk.java.net/jdk/pull/5110
Re: RFR: 8270344: Session resumption errors [v5]
> Corner case where a session resumption can fail if the TLS server changes > supported protocol versions in relation to a cached SSLSession. This is > primarily an issue where the legacy TLS version is used in place of the newer > "supported_versions" TLS extension. Sean Coffey has updated the pull request incrementally with one additional commit since the last revision: Remove redundant method and testcase cleanup - Changes: - all: https://git.openjdk.java.net/jdk/pull/5110/files - new: https://git.openjdk.java.net/jdk/pull/5110/files/c12551ad..b32a69cd Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=5110&range=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=5110&range=03-04 Stats: 52 lines in 3 files changed: 10 ins; 31 del; 11 mod Patch: https://git.openjdk.java.net/jdk/pull/5110.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5110/head:pull/5110 PR: https://git.openjdk.java.net/jdk/pull/5110
Re: RFR: 8271199: Mutual TLS handshake fails signing client certificate with custom sensitive PKCS11 key
On Fri, 23 Jul 2021 10:33:14 GMT, Alexey Bakhtin wrote: > Hello, > > Could you please review the small patch for the issue described in > JDK-8271199: Mutual TLS handshake fails signing client certificate with > custom sensitive PKCS11 key > > I suggest updating the RSAPSSSignature.isValid() method to verify if provided > key components can be applied to SunRSASign implementation. > If not applied, implementation can try to select signer from other providers > > Regards > Alexey Gentle ping - PR: https://git.openjdk.java.net/jdk/pull/4887
