Re: RFR: 8322767: TLS full handshake is slow with PKCS12KeyStore and X509KeyManagerImpl [v3]

2024-03-27 Thread Xue-Lei Andrew Fan
On Wed, 27 Mar 2024 08:15:08 GMT, Hai-May Chao wrote: > I ran the benchmark to measure the time needed to build a TLS context using > PKIX KeyManager (with protocols "TLSv1.2" and "TLS”) before and after the > changes to X509KeyManagerImpl.java. Here are the results: > > Without changes:

Re: RFR: 8322767: TLS full handshake is slow with PKCS12KeyStore and X509KeyManagerImpl [v3]

2024-03-26 Thread Xue-Lei Andrew Fan
On Tue, 26 Mar 2024 06:00:33 GMT, Hai-May Chao wrote: >> For the PKIX KeyManager and PKCS12 Keystore, when the TLS server sends the >> ServerHello message and ultimately calls the >> X509KeyManagerImpl.chooseEngineServerAlias() method, it retrieves the >> private key from the keystore,

Re: RFR: 8322767: TLS full handshake is slow with PKCS12KeyStore and X509KeyManagerImpl [v2]

2024-03-25 Thread Xue-Lei Andrew Fan
On Fri, 22 Mar 2024 06:56:33 GMT, Hai-May Chao wrote: >> For the PKIX KeyManager and PKCS12 Keystore, when the TLS server sends the >> ServerHello message and ultimately calls the >> X509KeyManagerImpl.chooseEngineServerAlias() method, it retrieves the >> private key from the keystore,

Re: RFR: 8311596: Add separate system properties for TLS server and client for maximum chain length [v8]

2023-11-09 Thread Xue-Lei Andrew Fan
On Thu, 9 Nov 2023 19:03:27 GMT, Sean Mullan wrote: >> I'm not sure if the number 8 or 10 really make a good difference in >> practice. No matter 8 or 10, if customers need lower value, they can >> always consider adjusting it. My concern is mainly about compatibility >> issues. If you

Re: RFR: 8311596: Add separate system properties for TLS server and client for maximum chain length [v8]

2023-11-07 Thread Xue-Lei Andrew Fan
On Tue, 7 Nov 2023 20:27:06 GMT, Sean Mullan wrote: >> I'm not sure if there is a clear reason to change the default value from 10 >> to 8. I'm fine if you want to keep to use value 10 for less compatibility >> issues. Otherwise, I have no more comment. Thanks! >> >>> Yes, I can place the

Re: RFR: 8311596: Add separate system properties for TLS server and client for maximum chain length [v8]

2023-11-06 Thread Xue-Lei Andrew Fan
On Mon, 6 Nov 2023 20:48:59 GMT, Hai-May Chao wrote: >> I think the wording of the comment is somewhat confusing because it is >> trying to explain the behavior of both properties together and the words >> "either" and "neither" may be hard to parse. I recommend separate comment >> blocks for

Re: RFR: 8311596: Add separate system properties for TLS server and client for maximum chain length [v8]

2023-11-01 Thread Xue-Lei Andrew Fan
On Mon, 30 Oct 2023 21:53:44 GMT, Hai-May Chao wrote: >> I agree that wording is more clear. We should also update the RN with that >> wording. > > This section of comments was taken from the CSR. I updated the comments as > follows. If it looks fine, I will update the related doc. Thanks! >

Re: RFR: 8311596: Add separate system properties for TLS server and client for maximum chain length [v6]

2023-10-25 Thread Xue-Lei Andrew Fan
On Fri, 20 Oct 2023 17:19:52 GMT, Xue-Lei Andrew Fan wrote: >> Hai-May Chao has updated the pull request with a new target base due to a >> merge or a rebase. The incremental webrev excludes the unrelated changes >> brought in by the merge/rebase. The pull request conta

Re: RFR: 8311596: Add separate system properties for TLS server and client for maximum chain length [v6]

2023-10-20 Thread Xue-Lei Andrew Fan
On Wed, 18 Oct 2023 00:25:02 GMT, Hai-May Chao wrote: >> Please review the enhancement for JDK-8311596 and its CSR JDK-8313236. Thank >> you. > > Hai-May Chao has updated the pull request with a new target base due to a > merge or a rebase. The incremental webrev excludes the unrelated changes

Integrated: 8312259: StatusResponseManager unused code clean up

2023-08-10 Thread Xue-Lei Andrew Fan
On Tue, 18 Jul 2023 17:43:42 GMT, Xue-Lei Andrew Fan wrote: > Hi, > > May I have the code cleanup update reviewed? With this update, the unused > code in StatusResponseManager.java will be removed. > > Thanks, > Xuelei This pull request has now been integrated. Chang

Re: RFR: 8312259: StatusResponseManager unused code clean up [v3]

2023-08-01 Thread Xue-Lei Andrew Fan
On Mon, 31 Jul 2023 18:03:22 GMT, Sean Mullan wrote: > I think @jnimeh should review this, as I think these methods were added when > implementing OCSP Stapling, and it would be good for him to make sure they > are no longer needed.. @jnimeh Did you have a chance to have a look at this

Re: RFR: 8312259: StatusResponseManager unused code clean up [v4]

2023-07-31 Thread Xue-Lei Andrew Fan
> Hi, > > May I have the code cleanup update reviewed? With this update, the unused > code in StatusResponseManager.java will be removed. > > Thanks, > Xuelei Xue-Lei Andrew Fan has updated the pull request incrementally with one additional commit since the las

Re: RFR: 8312259: StatusResponseManager unused code clean up [v3]

2023-07-31 Thread Xue-Lei Andrew Fan
On Mon, 31 Jul 2023 17:46:22 GMT, Mark Powers wrote: >> Xue-Lei Andrew Fan has updated the pull request incrementally with one >> additional commit since the last revision: >> >> revise test case > > test/jdk/sun/security/ssl/Stapl

Re: RFR: 8312259: StatusResponseManager unused code clean up [v3]

2023-07-31 Thread Xue-Lei Andrew Fan
On Wed, 19 Jul 2023 04:35:56 GMT, Xue-Lei Andrew Fan wrote: >> Hi, >> >> May I have the code cleanup update reviewed? With this update, the unused >> code in StatusResponseManager.java will be removed. >> >> Thanks, >> Xuelei > > Xue-Lei Andre

Re: RFR: 8302017: Allocate BadPaddingException only if it will be thrown [v3]

2023-07-27 Thread Xue-Lei Andrew Fan
On Thu, 27 Jul 2023 20:35:42 GMT, Valerie Peng wrote: >> I checked back the specification back to RFC 2437, released on October 1998, >> which requires to encode NULL parameters as well. As the update to keep the >> consistency is not trivial, I may just remove it and see if it could be a >>

Re: RFR: 8302017: Allocate BadPaddingException only if it will be thrown [v4]

2023-07-27 Thread Xue-Lei Andrew Fan
On Thu, 27 Jul 2023 20:42:37 GMT, Valerie Peng wrote: >> This change refactors the RSAPadding class to return an output record >> containing the status instead of relying on exception object to indicate a >> failure. >> >> Thanks in advance for review~ >> Valerie > > Valerie Peng has updated

Re: RFR: 8312630: java/security should not create unmodifiable collections with redundant wrapping

2023-07-26 Thread Xue-Lei Andrew Fan
On Wed, 26 Jul 2023 06:11:56 GMT, Xue-Lei Andrew Fan wrote: >> Some java/security classes apply the below coding style, >> >> Set set = ...; >> Set unmodifiableSet = Collections.unmodifiableSet(new HashSet<>(set)); >> >> It may be unnecessary to wr

Re: RFR: 8312630: java/security should not create unmodifiable collections with redundant wrapping

2023-07-26 Thread Xue-Lei Andrew Fan
On Tue, 25 Jul 2023 06:27:25 GMT, John Jiang wrote: > Some java/security classes apply the below coding style, > > Set set = ...; > Set unmodifiableSet = Collections.unmodifiableSet(new HashSet<>(set)); > > It may be unnecessary to wrap that `set` with HashSet before creating >

Re: RFR: 8302017: Allocate BadPaddingException only if it will be thrown [v3]

2023-07-24 Thread Xue-Lei Andrew Fan
On Mon, 24 Jul 2023 19:58:34 GMT, Valerie Peng wrote: >> src/java.base/share/classes/sun/security/rsa/RSASignature.java line 227: >> >>> 225: byte[] padded2 = padding.pad(encoded2); >>> 226: return MessageDigest.isEqual(padded2, decrypted); >>> 227: }

Re: RFR: 8312578: Redundant javadoc in X400Address

2023-07-24 Thread Xue-Lei Andrew Fan
On Mon, 24 Jul 2023 08:04:53 GMT, John Jiang wrote: > [JDK-8296741] removed the constructor `X400Address(byte[] value)`, but it > didn't remove the javadoc for this constructor. > This simple patch just removes this javadoc. > > [JDK-8296741]: >

Re: RFR: 8312443: sun.security should use toLowerCase(Locale.ROOT)

2023-07-20 Thread Xue-Lei Andrew Fan
On Thu, 20 Jul 2023 09:29:50 GMT, John Jiang wrote: > sun.security codes should use `toLowerCase(Locale.ROOT)` instead of > `toLowerCase()`. > In addition, no `toUpperCase()` was found in this code scope. Looks good to me. - Marked as reviewed by xuelei (Reviewer). PR Review:

Re: RFR: 8302017: Allocate BadPaddingException only if it will be thrown [v3]

2023-07-19 Thread Xue-Lei Andrew Fan
On Thu, 20 Jul 2023 00:39:08 GMT, Valerie Peng wrote: >> This change refactors the RSAPadding class to return an output record >> containing the status instead of relying on exception object to indicate a >> failure. >> >> Thanks in advance for review~ >> Valerie > > Valerie Peng has updated

Re: [jdk21] RFR: 8311902: Concurrency regression in the PBKDF2 key impl of SunJCE provider

2023-07-19 Thread Xue-Lei Andrew Fan
On Wed, 19 Jul 2023 17:51:20 GMT, Valerie Peng wrote: > 8311902: Concurrency regression in the PBKDF2 key impl of SunJCE provider Marked as reviewed by xuelei (Reviewer). src/java.base/share/classes/com/sun/crypto/provider/PBKDF2KeyImpl.java line 274: > 272: } > 273: > 274:

Re: RFR: 8312259: StatusResponseManager unused code clean up [v3]

2023-07-18 Thread Xue-Lei Andrew Fan
> Hi, > > May I have the code cleanup update reviewed? With this update, the unused > code in StatusResponseManager.java will be removed. > > Thanks, > Xuelei Xue-Lei Andrew Fan has updated the pull request incrementally with one additional commit since the last revision

Re: RFR: 8312259: StatusResponseManager unused code clean up [v2]

2023-07-18 Thread Xue-Lei Andrew Fan
On Tue, 18 Jul 2023 21:54:07 GMT, Mark Powers wrote: >> Xue-Lei Andrew Fan has updated the pull request incrementally with one >> additional commit since the last revision: >> >> add final keyword back > > src/java.base/share/classes/sun/security/ssl/StatusRe

Re: RFR: 8312259: StatusResponseManager unused code clean up [v2]

2023-07-18 Thread Xue-Lei Andrew Fan
> Hi, > > May I have the code cleanup update reviewed? With this update, the unused > code in StatusResponseManager.java will be removed. > > Thanks, > Xuelei Xue-Lei Andrew Fan has updated the pull request incrementally with one additional commit since the last r

Re: RFR: 8312259: StatusResponseManager unused code clean up

2023-07-18 Thread Xue-Lei Andrew Fan
On Tue, 18 Jul 2023 21:54:07 GMT, Mark Powers wrote: >> Hi, >> >> May I have the code cleanup update reviewed? With this update, the unused >> code in StatusResponseManager.java will be removed. >> >> Thanks, >> Xuelei > >

RFR: 8312259: StatusResponseManager unused code clean up

2023-07-18 Thread Xue-Lei Andrew Fan
Hi, May I have the code cleanup update reviewed? With this update, the unused code in StatusResponseManager.java will be removed. Thanks, Xuelei - Commit messages: - 8312259: StatusResponseManager unused code clean up Changes: https://git.openjdk.org/jdk/pull/14924/files

Re: RFR: 8302017: Allocate BadPaddingException only if it will be thrown [v2]

2023-07-18 Thread Xue-Lei Andrew Fan
On Thu, 13 Jul 2023 09:58:34 GMT, Ferenc Rakoczi wrote: >> src/java.base/share/classes/sun/security/rsa/RSASignature.java line 231: >> >>> 229: RSAUtil.decodeSignature(digestOID, >>> unpadded)); >>> 230: } >>> 231: } >> >> I understand

Re: RFR: 8302017: Allocate BadPaddingException only if it will be thrown [v2]

2023-07-18 Thread Xue-Lei Andrew Fan
On Tue, 18 Jul 2023 10:46:52 GMT, Ferenc Rakoczi wrote: >> Yes, BadPaddingException is still thrown by RSACore class. > > That exception is thrown by RSACore.parseMsg() if the message is larger than > the modulus. I think it is at least debatable whether it should be a > BadPaddingException. I

Re: RFR: 8311902: Concurrency regression in the PBKDF2 key impl of SunJCE provider [v2]

2023-07-14 Thread Xue-Lei Andrew Fan
On Fri, 14 Jul 2023 21:57:32 GMT, Valerie Peng wrote: >> This change adds back the Reference.ReachabilityFence(Object) call removed >> by [JDK-8301553](https://bugs.openjdk.org/browse/JDK-8301553). >> >> Please help review. >> Thanks! >> Valerie > > Valerie Peng has updated the pull request

Re: RFR: 8290005: com/sun/jndi/ldap/LdapCBPropertiesTest.java failling with NullPointerException [v6]

2023-07-14 Thread Xue-Lei Andrew Fan
On Fri, 14 Jul 2023 20:00:55 GMT, Matthew Donovan wrote: >> In this PR, I added methods to the TransportContext class to synchronize >> access to the handshakeContext field. I also updated locations in the code >> that rely on the handshakeContext field to not be null to use the >>

Re: RFR: 8290005: com/sun/jndi/ldap/LdapCBPropertiesTest.java failling with NullPointerException [v2]

2023-07-14 Thread Xue-Lei Andrew Fan
On Fri, 14 Jul 2023 14:16:00 GMT, Matthew Donovan wrote: > TransportContext also has a `protocolVersion` field. Is it possible to just > use that instead? Did you mean a change in duplexCloseOutput() like the following? - // The protocol version may have been negotiated. -

Re: RFR: 8311902: Concurrency regression in the PBKDF2 key impl of SunJCE provider

2023-07-13 Thread Xue-Lei Andrew Fan
On Fri, 14 Jul 2023 02:37:09 GMT, Xue-Lei Andrew Fan wrote: > > > It looks good to me to rollback to previous behaviors. I was just > > > wondering, if the use of key in other methods, like hashCode()/equals(), > > > has the similar issue? Thanks! > > &

Re: RFR: 8311902: Concurrency regression in the PBKDF2 key impl of SunJCE provider

2023-07-13 Thread Xue-Lei Andrew Fan
On Thu, 13 Jul 2023 21:01:06 GMT, Valerie Peng wrote: > > It looks good to me to rollback to previous behaviors. I was just > > wondering, if the use of key in other methods, like hashCode()/equals(), > > has the similar issue? Thanks! > > For the usage of hashCode()/equals(), there should be

Re: RFR: 8290005: com/sun/jndi/ldap/LdapCBPropertiesTest.java failling with NullPointerException [v2]

2023-07-13 Thread Xue-Lei Andrew Fan
On Thu, 13 Jul 2023 12:11:09 GMT, Matthew Donovan wrote: >> I reverted the changes to SSLEngineImpl and looked at the history of >> SSLSocketImpl and TransportContext but didn't see anything that I would be >> undoing with this change. > > Sorry for the delay on any updates here. > > I

Re: RFR: 8302017: Allocate BadPaddingException only if it will be thrown [v2]

2023-07-12 Thread Xue-Lei Andrew Fan
On Wed, 12 Jul 2023 23:12:18 GMT, Valerie Peng wrote: >> This change refactors the RSAPadding class to return an output record >> containing the status instead of relying on exception object to indicate a >> failure. >> >> Thanks in advance for review~ >> Valerie > > Valerie Peng has updated

Re: RFR: 8311902: Concurrency regression in the PBKDF2 key impl of SunJCE provider

2023-07-12 Thread Xue-Lei Andrew Fan
On Thu, 13 Jul 2023 02:46:10 GMT, Valerie Peng wrote: > This change adds back the Reference.ReachabilityFence(Object) call removed by > [JDK-8301553](https://bugs.openjdk.org/browse/JDK-8301553). > > Please help review. > Thanks! > Valerie It looks good to me to rollback to previous

Re: RFR: 8302017: Allocate BadPaddingException only if it will be thrown

2023-07-12 Thread Xue-Lei Andrew Fan
On Tue, 11 Jul 2023 23:19:40 GMT, Valerie Peng wrote: > This change refactors the RSAPadding class to return an output record > containing the status instead of relying on exception object to indicate a > failure. > > Thanks in advance for review~ > Valerie Changes requested by xuelei

Re: RFR: 8295894: Remove SECOM certificate that is expiring in September 2023

2023-07-11 Thread Xue-Lei Andrew Fan
On Tue, 11 Jul 2023 20:42:14 GMT, Rajan Halade wrote: > The fix is to remove the expiring SECOM root certificate after approval from > root CA to remove it. > > Release note is at - https://bugs.openjdk.org/browse/JDK-8311884 Marked as reviewed by xuelei (Reviewer). - PR Review:

Re: RFR: 8295068: SSLEngine throws NPE parsing CertificateRequests [v2]

2023-07-06 Thread Xue-Lei Andrew Fan
On Thu, 6 Jul 2023 15:52:00 GMT, Kevin Driver wrote: >> JDK-8295068: an NPE is thrown when an invalid `id` is found to match up a >> `ClientCertificateType`; rather than throwing the `NPE`, we now ignore an >> `id` which does match a `ClientCertificateType`. > > Kevin Driver has updated the

Re: RFR: 8295068: SSLEngine throws NPE parsing CertificateRequests

2023-07-06 Thread Xue-Lei Andrew Fan
On Wed, 5 Jul 2023 20:25:26 GMT, Kevin Driver wrote: > JDK-8295068: an NPE is thrown when an invalid `id` is found to match up a > `ClientCertificateType`; rather than throwing the `NPE`, we now throw an > `IllegalArgumentException`. This does not seem to be a scenario where > recovery is

Re: RFR: 8308540: On Kerberos TGT referral, if krb5.conf is missing realm, bad exception message

2023-06-26 Thread Xue-Lei Andrew Fan
On Mon, 22 May 2023 15:03:15 GMT, Weijun Wang wrote: > Add realm name to the exception message, and make it the primary exception > (retry exception added suppressed). Marked as reviewed by xuelei (Reviewer). - PR Review:

Re: [jdk21] RFR: 8309740: Expand timeout windows for tests in JDK-8179502

2023-06-23 Thread Xue-Lei Andrew Fan
On Fri, 23 Jun 2023 14:55:45 GMT, Jamil Nimeh wrote: > This is a backport of the test fixes comprising JDK-8309740. Looks good to me. I was just wondering, if the /backport tag could work from the original PR for backporting. - Marked as reviewed by xuelei (Reviewer). PR

Re: RFR: 8309740: Expand timeout windows for tests in JDK-8179502

2023-06-16 Thread Xue-Lei Andrew Fan
On Fri, 16 Jun 2023 18:19:45 GMT, Jamil Nimeh wrote: > This PR is for tests that were modified/added in JDK-8179502. The timeout > windows for those tests were a little too short on some test systems, > especially when the system is under heavy load. After a few iterations > trying out

Re: RFR: 8290005: com/sun/jndi/ldap/LdapCBPropertiesTest.java failling with NullPointerException [v4]

2023-06-16 Thread Xue-Lei Andrew Fan
On Fri, 16 Jun 2023 14:45:52 GMT, Matthew Donovan wrote: >> I may update both SSLSocketImpl and SSLEngineImpl, as SSLSocketImpl uses the >> locks similar to SSLEngineImpl. > > There is a difference, though. The close() method in SSLSocketImpl is not > synchronized and there is the chance of a

Re: RFR: 8290005: com/sun/jndi/ldap/LdapCBPropertiesTest.java failling with NullPointerException [v4]

2023-06-16 Thread Xue-Lei Andrew Fan
On Fri, 16 Jun 2023 14:24:17 GMT, Matthew Donovan wrote: >> I had a search of the value assignment of handshakeSession. Here is one >> example of the calling stack: >>

Re: RFR: 8290005: com/sun/jndi/ldap/LdapCBPropertiesTest.java failling with NullPointerException [v4]

2023-06-16 Thread Xue-Lei Andrew Fan
On Fri, 16 Jun 2023 11:59:01 GMT, Matthew Donovan wrote: >> src/java.base/share/classes/sun/security/ssl/SSLEngineImpl.java line 914: >> >>> 912: @Override >>> 913: public SSLSession getHandshakeSession() { >>> 914: engineLock.lock(); >> >> I'm not very sure of this update. By

Re: RFR: 8290005: com/sun/jndi/ldap/LdapCBPropertiesTest.java failling with NullPointerException [v3]

2023-06-16 Thread Xue-Lei Andrew Fan
On Wed, 3 May 2023 11:26:32 GMT, Matthew Donovan wrote: >> In this PR, I added methods to the TransportContext class to synchronize >> access to the handshakeContext field. I also updated locations in the code >> that rely on the handshakeContext field to not be null to use the >>

Re: RFR: 8310106: sun.security.ssl.SSLHandshake.getHandshakeProducer() incorrectly checks handshakeConsumers

2023-06-15 Thread Xue-Lei Andrew Fan
On Thu, 15 Jun 2023 06:02:13 GMT, Jaikiran Pai wrote: > Can I please get a review of this change which fixes the issue noted in > https://bugs.openjdk.org/browse/JDK-8310106? > > The commit here fixes the typo in > `sun.security.ssl.SSLHandshake.getHandshakeProducer()` method to correctly >

Re: RFR: 8309667: TLS handshake fails because of ConcurrentModificationException in PKCS12KeyStore.engineGetEntry

2023-06-15 Thread Xue-Lei Andrew Fan
On Fri, 16 Jun 2023 01:21:57 GMT, Weijun Wang wrote: > The `attributes` field inside the `PKCS12KeyStore.Entry` class might be > modified and retrieved at the same time. Make it concurrent. > > The test uses some reflection to get this field and try updating it in > multiple threads. Nice

Integrated: 8309867: redundant class field RSAPadding.md

2023-06-12 Thread Xue-Lei Andrew Fan
On Mon, 12 Jun 2023 16:39:33 GMT, Xue-Lei Andrew Fan wrote: > Hi, > > May I get this simple update reviewed? > > The class field RSAPadding.md can be converted to a local variable of the > constructor, and save the class footprint. > > Thanks, > Xuelei Thi

Re: RFR: 8309867: redundant class field RSAPadding.md

2023-06-12 Thread Xue-Lei Andrew Fan
On Mon, 12 Jun 2023 17:53:42 GMT, Hai-May Chao wrote: >> Hi, >> >> May I get this simple update reviewed? >> >> The class field RSAPadding.md can be converted to a local variable of the >> constructor, and save the class footprint. >> >> Thanks, >> Xuelei > > LGTM Thank you @haimaychao and

RFR: 8309867: redundant class field RSAPadding.md

2023-06-12 Thread Xue-Lei Andrew Fan
Hi, May I get this simple update reviewed? The class field RSAPadding.md can be converted to a local variable of the constructor, and save the class footprint. Thanks, Xuelei - Commit messages: - 8309867: redundant class field RSAPadding.md Changes:

Re: RFR: 8307144: namedParams in XECParameters and EdDSAParameters can be private final

2023-06-07 Thread Xue-Lei Andrew Fan
On Thu, 25 May 2023 21:17:40 GMT, Ben Perez wrote: > Changed `namedParams` in XECParameters and EdDSAParameters to be `private > final` Marked as reviewed by xuelei (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/14162#pullrequestreview-1466696530

Re: RFR: 8294985: SSLEngine throws IAE during parsing of X500Principal [v25]

2023-05-30 Thread Xue-Lei Andrew Fan
On Tue, 30 May 2023 19:24:09 GMT, Kevin Driver wrote: >> Fixes: [JDK-8294985](https://bugs.openjdk.org/browse/JDK-8294985) > > Kevin Driver has updated the pull request incrementally with two additional > commits since the last revision: > > - undo import changes > - undo import changes My

Re: RFR: 8297885: misc sun/security/pkcs11 tests timed out

2023-05-26 Thread Xue-Lei Andrew Fan
On Sat, 27 May 2023 01:49:22 GMT, Valerie Peng wrote: > Increase the timeout value of the "LargeDSAKey,java" test to address the > intermittent test failure(s). As for the other mentioned test failures, the > execution time fluctuates even less, so leaving them alone for now. > > Thanks in

Re: RFR: 8294985: SSLEngine throws IAE during parsing of X500Principal [v19]

2023-05-23 Thread Xue-Lei Andrew Fan
On Mon, 22 May 2023 19:41:25 GMT, Kevin Driver wrote: >> Fixes: [JDK-8294985](https://bugs.openjdk.org/browse/JDK-8294985) > > Kevin Driver has updated the pull request with a new target base due to a > merge or a rebase. The incremental webrev excludes the unrelated changes > brought in by

Re: RFR: 8294985: SSLEngine throws IAE during parsing of X500Principal [v19]

2023-05-23 Thread Xue-Lei Andrew Fan
On Tue, 23 May 2023 16:48:52 GMT, Kevin Driver wrote: >> test/jdk/sun/security/ssl/SSLEngineImpl/TestBadDNForPeerCA.java line 27: >> >>> 25: * @test >>> 26: * @library /test/lib >>> 27: * @summary verify correct exception handling in the event of an >>> unparseable >> >> Missing @bug

Re: RFR: 8301381: Verify DTLS 1.0 cannot be negotiated

2023-05-19 Thread Xue-Lei Andrew Fan
On Fri, 19 May 2023 12:03:54 GMT, Matthew Donovan wrote: > This PR implements a test to verify that a DTLS server running "out of the > box" (i.e., DTLSv1.0 disabled in java.security) will not handshake with a > client requesting DTLSv1.0. The test also implements the opposite: a client >

Re: RFR: 8305091: Change ChaCha20 cipher init behavior to match AES-GCM

2023-05-18 Thread Xue-Lei Andrew Fan
On Thu, 18 May 2023 18:16:49 GMT, Daniel Jeliński wrote: > Here you go: @djelinski Thank you! - PR Comment: https://git.openjdk.org/jdk/pull/13428#issuecomment-1553459091

Re: RFR: 8294985: SSLEngine throws IAE during parsing of X500Principal [v13]

2023-05-18 Thread Xue-Lei Andrew Fan
On Thu, 18 May 2023 17:49:07 GMT, Kevin Driver wrote: >> Fixes: [JDK-8294985](https://bugs.openjdk.org/browse/JDK-8294985) > > Kevin Driver has updated the pull request incrementally with one additional > commit since the last revision: > > all review comments applied Marked as reviewed by

Re: RFR: 8305091: Change ChaCha20 cipher init behavior to match AES-GCM

2023-05-18 Thread Xue-Lei Andrew Fan
On Thu, 18 May 2023 16:55:05 GMT, Daniel Jeliński wrote: > the QUIC specification permits dropping duplicate packets only after fully > decrypting them. May I have a reference, for example the section number, of the specification? - PR Comment:

Re: RFR: 8294985: SSLEngine throws IAE during parsing of X500Principal [v11]

2023-05-18 Thread Xue-Lei Andrew Fan
On Thu, 18 May 2023 16:15:39 GMT, Kevin Driver wrote: >> Fixes: [JDK-8294985](https://bugs.openjdk.org/browse/JDK-8294985) > > Kevin Driver has updated the pull request incrementally with one additional > commit since the last revision: > > review comments addressed

Re: RFR: 8294985: SSLEngine throws IAE during parsing of X500Principal [v11]

2023-05-18 Thread Xue-Lei Andrew Fan
On Thu, 18 May 2023 16:15:39 GMT, Kevin Driver wrote: >> Fixes: [JDK-8294985](https://bugs.openjdk.org/browse/JDK-8294985) > > Kevin Driver has updated the pull request incrementally with one additional > commit since the last revision: > > review comments addressed

Re: RFR: 8308118: Avoid multiarray allocations in AESCrypt.makeSessionKey [v2]

2023-05-18 Thread Xue-Lei Andrew Fan
On Thu, 18 May 2023 07:18:04 GMT, Aleksey Shipilev wrote: > > jdk_security or tier2. > > Gotcha, I already tested both, see "Additional Testing" section in PR. Thanks! - PR Comment: https://git.openjdk.org/jdk/pull/13996#issuecomment-1553126729

Re: RFR: 8294985: SSLEngine throws IAE during parsing of X500Principal [v10]

2023-05-18 Thread Xue-Lei Andrew Fan
On Thu, 18 May 2023 04:04:17 GMT, Xue-Lei Andrew Fan wrote: >> Kevin Driver has updated the pull request incrementally with one additional >> commit since the last revision: >> >> rework based upon code review comments > > src/java.base/

Re: RFR: 8308118: Avoid multiarray allocations in AESCrypt.makeSessionKey [v2]

2023-05-18 Thread Xue-Lei Andrew Fan
On Thu, 18 May 2023 06:44:10 GMT, Aleksey Shipilev wrote: > > Looks good to me. Please make sure the security regression testing passed. > > Thanks! By "security regression testing" that you mean `jdk_security`, or > something else? jdk_security or tier2. - PR Comment:

Re: RFR: 8294985: SSLEngine throws IAE during parsing of X500Principal [v10]

2023-05-17 Thread Xue-Lei Andrew Fan
On Wed, 17 May 2023 21:54:20 GMT, Kevin Driver wrote: >> Fixes: [JDK-8294985](https://bugs.openjdk.org/browse/JDK-8294985) > > Kevin Driver has updated the pull request incrementally with one additional > commit since the last revision: > > rework based upon code review comments Similar

Re: RFR: 8308118: Avoid multiarray allocations in AESCrypt.makeSessionKey [v2]

2023-05-17 Thread Xue-Lei Andrew Fan
On Tue, 16 May 2023 09:18:57 GMT, Aleksey Shipilev wrote: >> One of our services has a hot path with AES/GCM cipher reuse. The JDK code >> reinitializes the session key on that path, and >> [JDK-8308105](https://bugs.openjdk.org/browse/JDK-8308105) shows up >> prominently there. >> >> Fixing

Re: RFR: 8308118: Avoid multiarray allocations in AESCrypt.makeSessionKey [v2]

2023-05-17 Thread Xue-Lei Andrew Fan
On Wed, 17 May 2023 12:57:15 GMT, Aleksey Shipilev wrote: > @XueleiFan, or anyone else, please take a look? I will have a look, but I may need more time. - PR Comment: https://git.openjdk.org/jdk/pull/13996#issuecomment-1551895053

Re: RFR: 8294985: SSLEngine throws IAE during parsing of X500Principal [v6]

2023-05-17 Thread Xue-Lei Andrew Fan
On Fri, 12 May 2023 20:29:47 GMT, Kevin Driver wrote: >> Do you have any plans to write a test? If not, the bug needs a `noreg` label. > >> Do you have any plans to write a test? If not, the bug needs a `noreg` label. > > As discussed internally, the test that surfaced this issue will be >

Re: RFR: 8308118: Avoid multiarray allocations in AESCrypt.makeSessionKey

2023-05-15 Thread Xue-Lei Andrew Fan
On Mon, 15 May 2023 19:59:13 GMT, Aleksey Shipilev wrote: > One of our services has a hot path with AES/GCM cipher reuse. The JDK code > reinitializes the session key on that path, and > [JDK-8308105](https://bugs.openjdk.org/browse/JDK-8308105) shows up > prominently there. While >

Re: RFR: 8294985: SSLEngine throws IAE during parsing of X500Principal [v6]

2023-05-15 Thread Xue-Lei Andrew Fan
On Fri, 12 May 2023 20:30:04 GMT, Kevin Driver wrote: >> src/java.base/share/classes/sun/security/ssl/CertificateAuthoritiesExtension.java >> line 136: >> >>> 134: } catch (IllegalArgumentException iae) { >>> 135: throw new SSLException("X500Principal could not be

Re: RFR: 8298127: HSS/LMS Signature Verification [v9]

2023-05-11 Thread Xue-Lei Andrew Fan
On Thu, 11 May 2023 06:27:39 GMT, Ferenc Rakoczi wrote: > I had considered that and decided not to use it. In my opinion, Java Enum is > much more complicated than it should be for this case. OK. > Efficiency is not a concern here OK. > but I also don't see how enum could be more

Re: RFR: 8298127: HSS/LMS Signature Verification [v9]

2023-05-11 Thread Xue-Lei Andrew Fan
On Wed, 10 May 2023 15:20:50 GMT, Ferenc Rakoczi wrote: >> Implement support for Leighton-Micali Signatures (LMS) as described in RFC >> 8554. LMS is an approved software signing algorithm for CNSA 2.0, with >> SHA-256/192 parameters recommended. > > Ferenc Rakoczi has updated the pull request

Re: RFR: 8294985: SSLEngine throws IAE during parsing of X500Principal [v3]

2023-05-09 Thread Xue-Lei Andrew Fan
On Wed, 3 May 2023 22:11:20 GMT, Kevin Driver wrote: > I will look at making related changes in these spots as well. > OK. > @XueleiFan wrt your other question about updating the `getAuthorities` > method, I considered this, but it looks like it would involve changing a > method signature

Re: RFR: 8306015: Update sun.security.ssl TLS tests to use SSLContextTemplate or SSLEngineTemplate [v2]

2023-05-09 Thread Xue-Lei Andrew Fan
On Mon, 8 May 2023 17:56:41 GMT, Matthew Donovan wrote: >> I refactored tests in the test/jdk/sun/security/ssl directories to use the >> test template classes. > > Matthew Donovan has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains

Re: RFR: 8294983: SSLEngine throws ClassCastException during handshake [v3]

2023-05-03 Thread Xue-Lei Andrew Fan
On Wed, 3 May 2023 20:51:30 GMT, Kevin Driver wrote: >> Fixes a scenario where a `ServerHandshakeContext` might be cast as a >> `ClientHandshakeContext`. > > Kevin Driver has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains three

Re: RFR: 8306014: Update javax.net.ssl TLS tests to use SSLContextTemplate or SSLEngineTemplate

2023-05-02 Thread Xue-Lei Andrew Fan
On Mon, 17 Apr 2023 13:25:53 GMT, Matthew Donovan wrote: > I refactored tests in the test/jdk/javax/net/ssl directories to use the test > template classes. Looks good to me. The checks are not fully passed. Please double check if the failures are related to this update. -

Re: RFR: 8294985: SSLEngine throws IAE during parsing of X500Principal [v3]

2023-05-01 Thread Xue-Lei Andrew Fan
On Fri, 28 Apr 2023 19:15:59 GMT, Kevin Driver wrote: >> Fixes: [JDK-8294985](https://bugs.openjdk.org/browse/JDK-8294985) > > Kevin Driver has updated the pull request incrementally with one additional > commit since the last revision: > > Update >

Re: RFR: 8297878: KEM: Implementation [v2]

2023-04-14 Thread Xue-Lei Andrew Fan
On Fri, 14 Apr 2023 16:23:03 GMT, Weijun Wang wrote: >>> 2. Do validate parameters on your own (because no one else does) >>> >> I would say: validate the parameters on your own, because not everyone else >> does. > > I added another proposal in you PR at >

Re: RFR: 8297878: KEM: Implementation [v2]

2023-04-14 Thread Xue-Lei Andrew Fan
On Fri, 14 Apr 2023 15:00:51 GMT, Weijun Wang wrote: > 2. Do validate parameters on your own (because no one else does) > I would say: validate the parameters on your own, because not everyone else does. - PR Review Comment:

Re: RFR: 8294985: SSLEngine throws IAE during parsing of X500Principal

2023-04-13 Thread Xue-Lei Andrew Fan
On Thu, 13 Apr 2023 18:49:48 GMT, Kevin Driver wrote: > Fixes: [JDK-8294985](https://bugs.openjdk.org/browse/JDK-8294985) Is [JDK-8294985](https://bugs.openjdk.org/browse/JDK-8294985) in JBS a closed bug? - PR Comment:

Re: RFR: 8297878: KEM: Implementation [v2]

2023-04-13 Thread Xue-Lei Andrew Fan
On Thu, 13 Apr 2023 22:36:04 GMT, Weijun Wang wrote: >> src/java.base/share/classes/javax/crypto/KEMSpi.java line 119: >> >>> 117: * of {@code from} and {@code to} are within the correct range. >>> 118: * Therefore an implementation of this method does not need to >>> 119:

Re: RFR: 8297878: KEM: Implementation

2023-04-13 Thread Xue-Lei Andrew Fan
On Fri, 31 Mar 2023 02:25:04 GMT, Weijun Wang wrote: > The KEM API and DHKEM impl. Note that this PR uses new methods in > https://github.com/openjdk/jdk/pull/13250. Changes requested by xuelei (Reviewer). src/java.base/share/classes/javax/crypto/KEMSpi.java line 119: > 117: * of

Re: RFR: 8297878: KEM: Implementation

2023-04-13 Thread Xue-Lei Andrew Fan
On Thu, 13 Apr 2023 19:01:24 GMT, Xue-Lei Andrew Fan wrote: >> Currently, `provider()` is a method of `KEM.Encapsulator`. If `KEMSpi. >> newEncapsulator` also returns this interface, then what value should its >> `provider()` method return? This is what I meant

Re: RFR: 8297878: KEM: Implementation

2023-04-13 Thread Xue-Lei Andrew Fan
On Thu, 13 Apr 2023 17:54:22 GMT, Weijun Wang wrote: > Currently, `provider()` is a method of `KEM.Encapsulator`. If `KEMSpi. > newEncapsulator` also returns this interface, then what value should its > `provider()` method return? This is what I meant registering itself to a > provider. > >

Re: RFR: 8297878: KEM: Implementation

2023-04-13 Thread Xue-Lei Andrew Fan
On Thu, 13 Apr 2023 17:10:01 GMT, Weijun Wang wrote: > `DHKEM.java` is the implementation, and it does not know which provider it > will be put into. It's inside the provider that calls `putService` or `put` > to add an implementation there, not that the implementation registered itself > in

Re: RFR: 8297878: KEM: Implementation

2023-04-12 Thread Xue-Lei Andrew Fan
On Wed, 12 Apr 2023 23:23:02 GMT, Weijun Wang wrote: > If the interface is only in `KEM`, then it needs a `provider()` method, but > an implementation actually does not know what the provider is. With "implementation", do you mean the javax/crypto/KEPSpi.java or

Re: RFR: 8297878: KEM: Implementation

2023-04-12 Thread Xue-Lei Andrew Fan
On Wed, 12 Apr 2023 22:17:00 GMT, Weijun Wang wrote: > So you think the name is misleading? Yes. > But then if you want to talk about one you have to say which class it is in, > and this could be confusing and sometimes wrong. I did not get the idea yet to define it in KEPSpi. It looks

Re: RFR: 8297878: KEM: Implementation

2023-04-12 Thread Xue-Lei Andrew Fan
On Wed, 12 Apr 2023 18:32:30 GMT, Weijun Wang wrote: >> src/java.base/share/classes/javax/crypto/KEMSpi.java line 107: >> >>> 105: * @see KEM.Encapsulator >>> 106: */ >>> 107: interface EncapsulatorSpi { >> >> Is it really necessary to define a EncapsulatorSpi? It looks like an

Re: RFR: 8297878: KEM: Implementation

2023-04-12 Thread Xue-Lei Andrew Fan
On Fri, 31 Mar 2023 02:25:04 GMT, Weijun Wang wrote: > The KEM API and DHKEM impl. Note that this PR uses new methods in > https://github.com/openjdk/jdk/pull/13250. src/java.base/share/classes/javax/crypto/KEMSpi.java line 107: > 105: * @see KEM.Encapsulator > 106: */ > 107:

Re: RFR: 8305091: Change ChaCha20 cipher init behavior to match AES-GCM

2023-04-11 Thread Xue-Lei Andrew Fan
On Tue, 11 Apr 2023 17:26:25 GMT, Jamil Nimeh wrote: > This fixes an issue where the key/nonce reuse policy for SunJCE ChaCha20 and > ChaCha20-Poly1305 was overly strict in enforcing no-reuse when the Cipher was > in DECRYPT_MODE. For decryption, this should be allowed and be consistent >

Re: RFR: 8182621: JSSE should reject empty TLS plaintexts [v2]

2023-04-10 Thread Xue-Lei Andrew Fan
On Fri, 7 Apr 2023 13:56:39 GMT, Matthew Donovan wrote: >> Added code similar to the suggested patches for empty Handshake messages. I >> also implemented tests to verify empty Handshake, Alert, and >> ChangeCipherSpec messages result in expected behavior: for SSLEngineImpl, >> exceptions are

Re: RFR: 8182621: JSSE should reject empty TLS plaintexts [v2]

2023-04-09 Thread Xue-Lei Andrew Fan
On Fri, 7 Apr 2023 13:56:39 GMT, Matthew Donovan wrote: >> Added code similar to the suggested patches for empty Handshake messages. I >> also implemented tests to verify empty Handshake, Alert, and >> ChangeCipherSpec messages result in expected behavior: for SSLEngineImpl, >> exceptions are

Re: RFR: 8182621: JSSE should reject empty TLS plaintexts

2023-04-06 Thread Xue-Lei Andrew Fan
On Mon, 3 Apr 2023 18:13:19 GMT, Matthew Donovan wrote: > Added code similar to the suggested patches for empty Handshake messages. I > also implemented tests to verify empty Handshake, Alert, and ChangeCipherSpec > messages result in expected behavior: for SSLEngineImpl, exceptions are >

Re: RFR: 8302017: Allocate BadPaddingException only if it will be thrown

2023-03-27 Thread Xue-Lei Andrew Fan
On Mon, 27 Mar 2023 08:44:31 GMT, Aleksey Shipilev wrote: > If the exception creation is the problem, maybe we should turn it stackless This is one direction of the update, I think. Alternatively, it may be doable by not using exception in the unpad() implementation any longer. -

Re: RFR: 8302017: Allocate BadPaddingException only if it will be thrown

2023-03-24 Thread Xue-Lei Andrew Fan
On Tue, 14 Mar 2023 21:58:46 GMT, Xue-Lei Andrew Fan wrote: >> May I get a chance to review it before the integration? I may need more >> time to dig into time-constant issue. > >> May I get a chance to review it before the integration? I may need more time >> to d

Re: RFR: 8302017: Allocate BadPaddingException only if it will be thrown

2023-03-14 Thread Xue-Lei Andrew Fan
On Tue, 14 Mar 2023 21:23:02 GMT, Xue-Lei Andrew Fan wrote: > May I get a chance to review it before the integration? I may need more time > to dig into time-constant issue. If I read the Bleichenbacher's Attack[1][2] right, the attack works if it can tell the difference betwee

  1   2   3   4   >