Re: RFR: 8315487: Security Providers Filter [v21]

2025-04-05 Thread Xue-Lei Andrew Fan
On Thu, 20 Feb 2025 20:31:40 GMT, Martin Balao wrote: >> In addition to the goals, scope, motivation, specification and requirement >> notes in [JDK-8315487](https://bugs.openjdk.org/browse/JDK-8315487), we >> would like to describe the most relevant decisions taken during the >> implementatio

Re: RFR: 8315487: Security Providers Filter [v21]

2025-03-20 Thread Xue-Lei Andrew Fan
On Thu, 20 Feb 2025 20:31:40 GMT, Martin Balao wrote: >> In addition to the goals, scope, motivation, specification and requirement >> notes in [JDK-8315487](https://bugs.openjdk.org/browse/JDK-8315487), we >> would like to describe the most relevant decisions taken during the >> implementatio

Re: RFR: 8315487: Security Providers Filter [v17]

2024-12-17 Thread Xue-Lei Andrew Fan
On Tue, 17 Dec 2024 17:57:02 GMT, Martin Balao wrote: >> In addition to the goals, scope, motivation, specification and requirement >> notes in [JDK-8315487](https://bugs.openjdk.org/browse/JDK-8315487), we >> would like to describe the most relevant decisions taken during the >> implementatio

Re: RFR: 8315487: Security Providers Filter [v17]

2024-12-17 Thread Xue-Lei Andrew Fan
On Tue, 17 Dec 2024 17:57:02 GMT, Martin Balao wrote: >> In addition to the goals, scope, motivation, specification and requirement >> notes in [JDK-8315487](https://bugs.openjdk.org/browse/JDK-8315487), we >> would like to describe the most relevant decisions taken during the >> implementatio

Re: RFR: 8315487: Security Providers Filter [v17]

2024-12-17 Thread Xue-Lei Andrew Fan
On Tue, 17 Dec 2024 17:57:02 GMT, Martin Balao wrote: >> In addition to the goals, scope, motivation, specification and requirement >> notes in [JDK-8315487](https://bugs.openjdk.org/browse/JDK-8315487), we >> would like to describe the most relevant decisions taken during the >> implementatio

Re: RFR: 8315487: Security Providers Filter [v13]

2024-12-16 Thread Xue-Lei Andrew Fan
On Mon, 16 Dec 2024 20:41:04 GMT, Anthony Scarpino wrote: > > > It's only the combination of a Provider that overrides > > > getService/getServices + does not call putService/put + overrides > > > newInstance without calling its parent + uses a non-Java SE service type > > > that would be unf

Re: RFR: 8315487: Security Providers Filter [v13]

2024-12-15 Thread Xue-Lei Andrew Fan
On Sun, 15 Dec 2024 07:18:02 GMT, Xue-Lei Andrew Fan wrote: > It's only the combination of a Provider that overrides getService/getServices > + does not call putService/put + overrides newInstance without calling its > parent + uses a non-Java SE service type that would be un

Re: RFR: 8315487: Security Providers Filter [v13]

2024-12-14 Thread Xue-Lei Andrew Fan
On Fri, 13 Dec 2024 05:14:07 GMT, Xue-Lei Andrew Fan wrote: > It's only the combination of a Provider that overrides getService/getServices > + does not call putService/put + overrides newInstance without calling its > parent + uses a non-Java SE service type that would be unfilt

Re: RFR: 8315487: Security Providers Filter [v13]

2024-12-12 Thread Xue-Lei Andrew Fan
On Thu, 12 Dec 2024 22:54:59 GMT, Martin Balao wrote: >>> In rare situations, a third-party provider can override >>> java.security.Provider.Service::newInstance and return an unvetted service >>> implementation (SPI). >> >> >> Well, there is a concern of mine. I don't agree the case is rare.

Re: RFR: 8315487: Security Providers Filter [v13]

2024-12-12 Thread Xue-Lei Andrew Fan
On Mon, 2 Dec 2024 21:05:53 GMT, Martin Balao wrote: > getService/getServices API overrides are supported since the initial PR. > Please check the JEP and implementation, and let us know if you see any flaw. I guess you refer to the following section in the JEP. Otherwise, please let me know

Re: RFR: 8315487: Security Providers Filter [v13]

2024-12-01 Thread Xue-Lei Andrew Fan
On Fri, 29 Nov 2024 14:28:27 GMT, Martin Balao wrote: >> In addition to the goals, scope, motivation, specification and requirement >> notes in [JDK-8315487](https://bugs.openjdk.org/browse/JDK-8315487), we >> would like to describe the most relevant decisions taken during the >> implementatio

Re: RFR: 8315487: Security Providers Filter [v9]

2024-11-30 Thread Xue-Lei Andrew Fan
On Fri, 29 Nov 2024 19:22:57 GMT, Francisco Ferrari Bihurriet wrote: > ... We agree that this pull request is too large to review ... Thank you! > ... Is not a goal of this proposal to allow different filter implementations Got it. Thank you for the clarification. But, does it sound reason

Re: RFR: 8315487: Security Providers Filter [v9]

2024-11-04 Thread Xue-Lei Andrew Fan
On Mon, 21 Oct 2024 18:30:50 GMT, Martin Balao wrote: >> In addition to the goals, scope, motivation, specification and requirement >> notes in [JDK-8315487](https://bugs.openjdk.org/browse/JDK-8315487), we >> would like to describe the most relevant decisions taken during the >> implementatio

Re: RFR: 8341964: Add mechanism to disable different parts of TLS cipher suite [v4]

2024-11-02 Thread Xue-Lei Andrew Fan
On Wed, 30 Oct 2024 15:44:58 GMT, Artur Barashev wrote: >> The current syntax of the jdk.tls.disabledAlgorithms makes it difficult to >> disable algorithms that affect both the key exchange and authentication >> parts of a TLS cipher suite. For example, if you add "RSA" to the >> jdk.tls.disab

Re: RFR: 8341964: Add mechanism to disable different parts of TLS cipher suite [v4]

2024-11-01 Thread Xue-Lei Andrew Fan
On Wed, 30 Oct 2024 15:44:58 GMT, Artur Barashev wrote: >> The current syntax of the jdk.tls.disabledAlgorithms makes it difficult to >> disable algorithms that affect both the key exchange and authentication >> parts of a TLS cipher suite. For example, if you add "RSA" to the >> jdk.tls.disab

Re: RFR: 8331682: Slow networks/Impatient clients can potentially send unencrypted TLSv1.3 alerts that won't parse on the server [v26]

2024-10-29 Thread Xue-Lei Andrew Fan
On Tue, 29 Oct 2024 20:32:50 GMT, Artur Barashev wrote: > * I'm not sure what you mean by `plaintext`, what is your definition of > plaintext? Per TLS1.3 RFC it's a plaintext unless contentType == 23 With "plaintext", I meant it is not TLSCiphertext. You have a good point. Thank you!

Re: RFR: 8331682: Slow networks/Impatient clients can potentially send unencrypted TLSv1.3 alerts that won't parse on the server [v26]

2024-10-29 Thread Xue-Lei Andrew Fan
On Tue, 29 Oct 2024 19:35:54 GMT, Artur Barashev wrote: >> Check for unexpected plaintext alert message during TLSv1.3 handshake. This >> can happen if client doesn't receive ServerHello due to network timeout and >> tries to close the connection by sending an alert message. > > Artur Barashev

Re: RFR: 8331682: Slow networks/Impatient clients can potentially send unencrypted TLSv1.3 alerts that won't parse on the server [v24]

2024-10-29 Thread Xue-Lei Andrew Fan
On Tue, 29 Oct 2024 17:16:35 GMT, Xue-Lei Andrew Fan wrote: >> But it may not be in the same log, depending on where the `SSLLogger` is >> directed vs. the Exceptions. I'd say keep it in. > > It may be sufficient to have it in exception only. We normally don't lo

Re: RFR: 8331682: Slow networks/Impatient clients can potentially send unencrypted TLSv1.3 alerts that won't parse on the server [v24]

2024-10-29 Thread Xue-Lei Andrew Fan
On Fri, 25 Oct 2024 21:25:10 GMT, Bradford Wetmore wrote: >> src/java.base/share/classes/sun/security/ssl/SSLCipher.java line 1870: >> >>> 1868: if (SSLLogger.isOn && SSLLogger.isOn("ssl")) { >>> 1869: SSLLogger.info(msg); >>> 1870:

Re: RFR: 8331682: Slow networks/Impatient clients can potentially send unencrypted TLSv1.3 alerts that won't parse on the server [v24]

2024-10-26 Thread Xue-Lei Andrew Fan
On Fri, 25 Oct 2024 23:44:50 GMT, Artur Barashev wrote: >> 1. If we got through the `bb.remaining() <= tagSize` arm, we're not going to >> be encrypted. I think we are safe here. >> 2. I would dump the alert level (fatal/warning) + reason. >> 3. `bb` size definitely needs to be checked. e.g. i

Re: RFR: 8331682: Slow networks/Impatient clients can potentially send unencrypted TLSv1.3 alerts that won't parse on the server [v24]

2024-10-25 Thread Xue-Lei Andrew Fan
On Fri, 25 Oct 2024 13:30:43 GMT, Artur Barashev wrote: >> Check for unexpected plaintext alert message during TLSv1.3 handshake. This >> can happen if client doesn't receive ServerHello due to network timeout and >> tries to close the connection by sending an alert message. > > Artur Barashev

Re: RFR: 8331682: Slow networks/Impatient clients can potentially send unencrypted TLSv1.3 alerts that won't parse on the server [v20]

2024-10-24 Thread Xue-Lei Andrew Fan
On Thu, 24 Oct 2024 15:38:59 GMT, Artur Barashev wrote: > The goal is to help users to debug this situation so we provide the only > possible cause we know about. Then you introduce more confusing for your unknown parts. You may never know why it happens but it just happens. What we want to

Re: RFR: 8331682: Slow networks/Impatient clients can potentially send unencrypted TLSv1.3 alerts that won't parse on the server [v20]

2024-10-24 Thread Xue-Lei Andrew Fan
On Thu, 17 Oct 2024 17:17:40 GMT, Xue-Lei Andrew Fan wrote: > > > Does it happen in server side (server send plaintext) as well? I found > > > some cases that the client decryption failed. > > > > > > Current reports indicate it happens on the serv

Re: RFR: 8331682: Slow networks/Impatient clients can potentially send unencrypted TLSv1.3 alerts that won't parse on the server [v22]

2024-10-24 Thread Xue-Lei Andrew Fan
On Mon, 21 Oct 2024 20:18:24 GMT, Artur Barashev wrote: >> Check for unexpected plaintext alert message during TLSv1.3 handshake. This >> can happen if client doesn't receive ServerHello due to network timeout and >> tries to close the connection by sending an alert message. > > Artur Barashev

Re: RFR: 8331682: Slow networks/Impatient clients can potentially send unencrypted TLSv1.3 alerts that won't parse on the server [v20]

2024-10-17 Thread Xue-Lei Andrew Fan
On Thu, 17 Oct 2024 20:11:06 GMT, Artur Barashev wrote: > We want to address just this particular issue and not to cause any collateral > damage by simplifying things. For example: if plaintext alert comes after > handshake we want to throw an exception. I think I have a better sense of your i

Re: RFR: 8331682: Slow networks/Impatient clients can potentially send unencrypted TLSv1.3 alerts that won't parse on the server [v20]

2024-10-17 Thread Xue-Lei Andrew Fan
On Thu, 17 Oct 2024 18:48:29 GMT, Xue-Lei Andrew Fan wrote: > SSLCipher has contentType. Here is the idea in my mind. @artur-oracle Please check if I missed something. Thank you! @Override public Plaintext decrypt(byte contentType, ByteBuffer

Re: RFR: 8331682: Slow networks/Impatient clients can potentially send unencrypted TLSv1.3 alerts that won't parse on the server [v20]

2024-10-17 Thread Xue-Lei Andrew Fan
On Fri, 11 Oct 2024 18:36:50 GMT, Artur Barashev wrote: >> Check for unexpected plaintext alert message during TLSv1.3 handshake. This >> can happen if client doesn't receive ServerHello due to network timeout and >> tries to close the connection by sending an alert message. > > Artur Barashev

Re: RFR: 8331682: Slow networks/Impatient clients can potentially send unencrypted TLSv1.3 alerts that won't parse on the server [v20]

2024-10-17 Thread Xue-Lei Andrew Fan
On Thu, 17 Oct 2024 17:46:39 GMT, Artur Barashev wrote: > > Currently, if an alter message cannot be decrypted, exception thrown. When > > alert is received, if it is not decryptable, treat it as an alert and close > > the connection accordingly, without sending more message to peers. > > * Fo

Re: RFR: 8331682: Slow networks/Impatient clients can potentially send unencrypted TLSv1.3 alerts that won't parse on the server [v20]

2024-10-17 Thread Xue-Lei Andrew Fan
On Thu, 17 Oct 2024 15:55:29 GMT, Artur Barashev wrote: >> Does it happen in server side (server send plaintext) as well? I found some >> cases that the client decryption failed. > >> Does it happen in server side (server send plaintext) as well? I found some >> cases that the client decryptio

Re: RFR: 8331682: Slow networks/Impatient clients can potentially send unencrypted TLSv1.3 alerts that won't parse on the server [v20]

2024-10-17 Thread Xue-Lei Andrew Fan
On Thu, 17 Oct 2024 16:13:58 GMT, Artur Barashev wrote: >> src/java.base/share/classes/sun/security/ssl/SSLSocketInputRecord.java line >> 266: >> >>> 264: recordBody.flip(); >>> 265: // Record is ready to be decoded, save it. >>> 266: saveLastDecodeRecord(); >> >> It mi

Re: RFR: 8331682: Slow networks/Impatient clients can potentially send unencrypted TLSv1.3 alerts that won't parse on the server [v20]

2024-10-17 Thread Xue-Lei Andrew Fan
On Thu, 17 Oct 2024 15:55:29 GMT, Artur Barashev wrote: > > Does it happen in server side (server send plaintext) as well? I found some > > cases that the client decryption failed. > > Current reports indicate it happens on the server side only (server throws > the exception). Please share any

Re: RFR: 8331682: Slow networks/Impatient clients can potentially send unencrypted TLSv1.3 alerts that won't parse on the server [v7]

2024-10-16 Thread Xue-Lei Andrew Fan
On Tue, 24 Sep 2024 20:19:58 GMT, Artur Barashev wrote: >> Can’t review it, still don’t understand how the error condition happens. >> (But I do know massive problems with extra messages sent when a broken >> connection is wound down - it might want to use aggressive timeouts for >> those grat

Re: RFR: 8331682: Slow networks/Impatient clients can potentially send unencrypted TLSv1.3 alerts that won't parse on the server [v20]

2024-10-16 Thread Xue-Lei Andrew Fan
On Fri, 11 Oct 2024 18:36:50 GMT, Artur Barashev wrote: >> Check for unexpected plaintext alert message during TLSv1.3 handshake. This >> can happen if client doesn't receive ServerHello due to network timeout and >> tries to close the connection by sending an alert message. > > Artur Barashev

Re: RFR: 8331682: Slow networks/Impatient clients can potentially send unencrypted TLSv1.3 alerts that won't parse on the server [v20]

2024-10-16 Thread Xue-Lei Andrew Fan
On Fri, 11 Oct 2024 18:36:50 GMT, Artur Barashev wrote: >> Check for unexpected plaintext alert message during TLSv1.3 handshake. This >> can happen if client doesn't receive ServerHello due to network timeout and >> tries to close the connection by sending an alert message. > > Artur Barashev

Re: RFR: 8342211: Insufficient buffer remaining for AEAD cipher fragment [v2]

2024-10-16 Thread Xue-Lei Andrew Fan
On Wed, 16 Oct 2024 05:31:23 GMT, Xue-Lei Andrew Fan wrote: >> Hi, >> >> Please review this simple update, which is trying to expose more information >> about the failed decryption. >> >> Here is the JBS: https://bugs.openjdk.org/browse/JDK-8342211 >&

Re: RFR: 8331682: Slow networks/Impatient clients can potentially send unencrypted TLSv1.3 alerts that won't parse on the server [v20]

2024-10-16 Thread Xue-Lei Andrew Fan
On Fri, 11 Oct 2024 18:36:50 GMT, Artur Barashev wrote: >> Check for unexpected plaintext alert message during TLSv1.3 handshake. This >> can happen if client doesn't receive ServerHello due to network timeout and >> tries to close the connection by sending an alert message. > > Artur Barashev

Re: RFR: 8342211: Insufficient buffer remaining for AEAD cipher fragment [v2]

2024-10-15 Thread Xue-Lei Andrew Fan
> Hi, > > Please review this simple update, which is trying to expose more information > about the failed decryption. > > Here is the JBS: https://bugs.openjdk.org/browse/JDK-8342211 > > Best, > Xuelei Xue-Lei Andrew Fan has updated the pull request incrementally

RFR: 8342211: Insufficient buffer remaining for AEAD cipher fragment

2024-10-15 Thread Xue-Lei Andrew Fan
Hi, Please review this simple update, which is trying to expose more information about the failed decryption. Here is the JBS: https://bugs.openjdk.org/browse/JDK-8342211 Best, Xuelei - Commit messages: - 8342211: Insufficient buffer remaining for AEAD cipher fragment Changes: h

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: Bench

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, decrypt

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, decrypt

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 wan

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 co

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 update?

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 t

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-25 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 > `unmodifiableSe

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]: > Ma

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: htt

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 t

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 > > src/java.base/share/classes/sun/security/ssl/StatusResponseManager.jav

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 Webrev

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 wh

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 inc

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 >> synchronize

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 update

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 t

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 behaviors.

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 (Review

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 pul

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 poss

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: https://git.openjdk.org/jdk/pull/14086#pullrequestreview-14

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 Review:

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 vario

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 N

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: >> SSLEngineImpl.DelegatedTask.run()->TransportContext.dispatch()->SSLHandshake().consume()->HandshakeConsumer.consume()->SSLExten

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 r

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

2023-06-15 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 >> synchronized

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 > u

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 cat

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: https://git.openjdk.org/jdk/p

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

2023-06-06 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 c

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 adv

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 the

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 field.

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 > won'

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 x

  1   2   3   4   >