Re: RFR: 8290669: Fix wording in sun.security.ec

2022-07-26 Thread John Jiang
On Fri, 22 Jul 2022 02:15:44 GMT, longxu0509 wrote: > this patch fixes several wording in sun.security.ec Just changed the label in this JBS. Thanks! - PR: https://git.openjdk.org/jdk/pull/9606

Re: RFR: 8295011: EC point multiplication improvement for secp256r1 [v3]

2022-10-31 Thread John Jiang
On Fri, 28 Oct 2022 18:05:30 GMT, Xue-Lei Andrew Fan wrote: >> Hi, >> >> May I have this update reviewed? >> >> The EC point multiplication for secp256r1 could be improved for better >> performance, by using more efficient algorithm and pre-computation. >> Improvement for other curves are si

Re: RFR: 8294731: Improve multiplicative inverse for secp256r1 implementation [v3]

2022-10-31 Thread John Jiang
On Sat, 8 Oct 2022 15:34:57 GMT, Xue-Lei Andrew Fan wrote: >> Hi, >> >> May I have this patch reviewed? >> >> This is one of a few steps to improve the EC performance. The multiplicative >> inverse implementation could be improved for better performance. >> >> For secp256r1 prime p, the curr

Re: RFR: 8294073: Performance improvement for message digest implementations

2022-11-20 Thread John Jiang
On Tue, 20 Sep 2022 22:12:04 GMT, Xue-Lei Andrew Fan wrote: > Hi, > > In the message digest implementation, for example SHA256, in JDK, two bitwise > operations could be improved with equivalent arithmetic, and then the number > bitwise operations could be reduced accordingly. Specifically >

Re: RFR: 8295010: Reduce if required in EC limbs operations [v6]

2022-11-27 Thread John Jiang
On Wed, 23 Nov 2022 06:21:05 GMT, Xue-Lei Andrew Fan wrote: >> Hi, >> >> May I have this update reviewed? With this update, the result will be >> reduced if required in EC limbs operations in the JDK implementation. >> >> In the current implementation, the EC limbs addition and subtraction re

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

2023-07-20 Thread John Jiang
sun.security codes should use `toLowerCase(Locale.ROOT)` instead of `toLowerCase()`. In addition, no `toUpperCase()` was found in this code scope. - Commit messages: - 8312443: sun.security should use toLowerCase(Locale.ROOT) Changes: https://git.openjdk.org/jdk/pull/14948/files W

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

2023-07-20 Thread John Jiang
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. This pull request has now been integrated. Changeset: 4e8f331a Author:J

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

2023-07-20 Thread John Jiang
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. @XueleiFan Thanks for your review! - PR Comment: https://git.op

RFR: 8312578: Redundant javadoc in X400Address

2023-07-24 Thread John Jiang
[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]: - Commit messages: - 8312578: Redundant javadoc in X40

Integrated: 8312578: Redundant javadoc in X400Address

2023-07-24 Thread John Jiang
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]: > <https://bugs.open

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

2023-07-24 Thread John Jiang
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 `unmodifiableSet`. Some usages on `Collections.unmodifiableList` and `Collections.u

RFR: 8313087: DerValue::toString should output the values in byte array

2023-07-25 Thread John Jiang
DerValue::toString may be better to output the hex view of the byte array variable `buffer`. - Commit messages: - 8313087: DerValue::toString should output the values in byte array Changes: https://git.openjdk.org/jdk/pull/15029/files Webrev: https://webrevs.openjdk.org/?repo=jdk&

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

2023-07-26 Thread John Jiang
`Collections.unmodifiableList` and > `Collections.unmodifiableMap` have the same issue. John Jiang has updated the pull request incrementally with one additional commit since the last revision: use copyOf instead of modifiableXXX - Changes: - all: https://git.openjdk.org/jd

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

2023-07-26 Thread John Jiang
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 wrap that `set` with HashSet before creating >>

Re: RFR: 8313087: DerValue::toString should output a hex view of the values in byte array

2023-07-26 Thread John Jiang
On Wed, 26 Jul 2023 13:49:49 GMT, Sean Mullan wrote: >> DerValue::toString may be better to output the hex view of the byte array >> variable `buffer`. > > Please change the title of the bug to be "DerValue::toString should output a > hex view of the values in byte array". > > Otherwise, LGTM.

Integrated: 8313087: DerValue::toString should output a hex view of the values in byte array

2023-07-26 Thread John Jiang
On Wed, 26 Jul 2023 04:02:04 GMT, John Jiang wrote: > DerValue::toString may be better to output the hex view of the byte array > variable `buffer`. This pull request has now been integrated. Changeset: 830413f1 Author: John Jiang URL: https://git.openjdk.org/jdk/

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

2023-07-26 Thread John Jiang
On Wed, 26 Jul 2023 14:41:16 GMT, Sean Mullan wrote: > it says "It is cloned to prevent subsequent modification." > Now, if the set passed in is unmodifiable, that is no longer true. My understanding: The parameter `protectionParams` is cloned as `this.protectionParams`, and `this.protectionPar

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

2023-07-26 Thread John Jiang
On Wed, 26 Jul 2023 16:52:20 GMT, Sean Mullan wrote: > The latter part is true (prevented from subsequent modification) but, unless > I am mistaken, the former (making a clone/copy) is not. For example, before > your change, this assert would pass: > > ``` > Map m = Collections.unmodifiableMap

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

2023-07-26 Thread John Jiang
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

RFR: 8313226: Redundant condition test in X509CRLImpl

2023-07-26 Thread John Jiang
if ((nextByte == DerValue.tag_SequenceOf) && (! ((nextByte & 0x0c0) == 0x080))) { ... ... } If `nextByte` is `DerValue.tag_SequenceOf`, exactly `0x30`, then the test after `&&` should always be true. - Commit messages: - 8313226: Redundant condition test in X509CRLI

Re: RFR: 8313226: Redundant condition test in X509CRLImpl

2023-08-01 Thread John Jiang
On Thu, 27 Jul 2023 04:00:21 GMT, John Jiang wrote: > if ((nextByte == DerValue.tag_SequenceOf) > && (! ((nextByte & 0x0c0) == 0x080))) { > ... > ... > } > > If `nextByte` is `DerValue.tag_SequenceOf`, exactly `0x30`, then the test > afte

Re: RFR: 8313226: Redundant condition test in X509CRLImpl

2023-08-01 Thread John Jiang
On Tue, 1 Aug 2023 15:22:48 GMT, Jamil Nimeh wrote: >> if ((nextByte == DerValue.tag_SequenceOf) >> && (! ((nextByte & 0x0c0) == 0x080))) { >> ... >> ... >> } >> >> If `nextByte` is `DerValue.tag_SequenceOf`, exactly `0x30`, then the test >> after `&&` should always be true. > >

Integrated: 8313226: Redundant condition test in X509CRLImpl

2023-08-01 Thread John Jiang
On Thu, 27 Jul 2023 04:00:21 GMT, John Jiang wrote: > if ((nextByte == DerValue.tag_SequenceOf) > && (! ((nextByte & 0x0c0) == 0x080))) { > ... > ... > } > > If `nextByte` is `DerValue.tag_SequenceOf`, exactly `0x30`, then the test > after `&

RFR: JDK-8315422: getSoTimeout() would be in try block in SSLSocketImpl

2023-08-30 Thread John Jiang
The method `SSLSocketImpl::closeSocket` has the below code snippet, if (appInput.readLock.tryLock()) { int soTimeout = getSoTimeout(); try { // deplete could hang on the skip operation // in case of infinite socket read timeout. // Change read timeout to avoid dead

Integrated: JDK-8315422: getSoTimeout() would be in try block in SSLSocketImpl

2023-08-31 Thread John Jiang
On Thu, 31 Aug 2023 02:34:58 GMT, John Jiang wrote: > The method `SSLSocketImpl::closeSocket` has the below code snippet, > > > if (appInput.readLock.tryLock()) { > int soTimeout = getSoTimeout(); > try { > // deplete could hang on the skip operation &g

RFR: 8320049: PKCS10 would not discard the cause when throw SignatureException on invalid key

2023-11-14 Thread John Jiang
When throw SignatureException on invalid key, it may be better to contain the cause exception. - Commit messages: - 8320049: PKCS10 would not discard the cause when throw SignatureException on invalid key Changes: https://git.openjdk.org/jdk/pull/16649/files Webrev: https://webre

Re: RFR: 8320049: PKCS10 would not discard the cause when throw SignatureException on invalid key

2023-11-23 Thread John Jiang
On Tue, 14 Nov 2023 08:59:30 GMT, John Jiang wrote: > When throw SignatureException on invalid key, it may be better to contain the > cause exception. Could this patch be reviewed? - PR Comment: https://git.openjdk.org/jdk/pull/16649#issuecomment-1824025129

Integrated: 8320049: PKCS10 would not discard the cause when throw SignatureException on invalid key

2023-11-28 Thread John Jiang
On Tue, 14 Nov 2023 08:59:30 GMT, John Jiang wrote: > When throw SignatureException on invalid key, it may be better to contain the > cause exception. This pull request has now been integrated. Changeset: 2c4c6c9b Author: John Jiang URL: https://git.openjdk.org/jdk/

Re: RFR: 8320049: PKCS10 would not discard the cause when throw SignatureException on invalid key

2023-11-28 Thread John Jiang
On Tue, 28 Nov 2023 21:04:49 GMT, Sean Mullan wrote: >> When throw SignatureException on invalid key, it may be better to contain >> the cause exception. > > Looks fine to me, although I view this more as an Enhancement than a Bug. @seanjmullan Thanks for your review! I just switched the JBS

RFR: 8322734: A redundant return in PKCS5Padding

2023-12-25 Thread John Jiang
It looks the `return` in method `PKCS5Padding::padWithLen` is redundant. - Commit messages: - 8322734: A redundant return in PKCS5Padding Changes: https://git.openjdk.org/jdk/pull/17189/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=17189&range=00 Issue: https://bugs.open

Re: RFR: 8322734: A redundant return in method padWithLen [v2]

2023-12-25 Thread John Jiang
> It looks the `return`s in the methods `PKCS5Padding::padWithLen` and > `ISO10126Padding::padWithLen` are redundant. John Jiang has updated the pull request incrementally with one additional commit since the last revision: Remove return from ISO10126Padding::padW

Integrated: 8322734: A redundant return in method padWithLen

2023-12-26 Thread John Jiang
On Mon, 25 Dec 2023 09:30:02 GMT, John Jiang wrote: > It looks the `return`s in the methods `PKCS5Padding::padWithLen` and > `ISO10126Padding::padWithLen` are redundant. This pull request has now been integrated. Changeset: 2a59243c Author: John Jiang URL: https://git.openj

Re: RFR: 8322734: A redundant return in method padWithLen

2023-12-26 Thread John Jiang
On Mon, 25 Dec 2023 09:42:36 GMT, Jie Fu wrote: >> It looks the `return`s in the methods `PKCS5Padding::padWithLen` and >> `ISO10126Padding::padWithLen` are redundant. > > I see the same case in ISO10126Padding.java. > Shall we also remove it? @DamonFool Thanks for your review! -

RFR: 8322766: Micro bench SSLHandshake would use modern algorithms

2023-12-28 Thread John Jiang
test/micro/org/openjdk/bench/java/security/SSLHandshake.java is using keystore type `JKS` and TrustManagerFactory/KeyManagerFactory algorithm `SunX509`. It may be better to use `PKCS12` and `PKIX` respectively. - Commit messages: - 8322766: Micro bench SSLHandshake would use modern

Re: RFR: 8322766: Micro bench SSLHandshake would use modern algorithms [v2]

2023-12-29 Thread John Jiang
> test/micro/org/openjdk/bench/java/security/SSLHandshake.java is using > keystore type `JKS` and TrustManagerFactory/KeyManagerFactory algorithm > `SunX509`. > It may be better to use `PKCS12` and `PKIX` respectively. John Jiang has updated the pull request incrementally with on

Re: RFR: 8322766: Micro bench SSLHandshake would use modern algorithms

2023-12-29 Thread John Jiang
On Fri, 29 Dec 2023 19:13:57 GMT, Daniel Jeliński wrote: >> test/micro/org/openjdk/bench/java/security/SSLHandshake.java is using >> keystore type `JKS` and TrustManagerFactory/KeyManagerFactory algorithm >> `SunX509`. >> It may be better to use `PKCS12` and `PKIX` respectively. > > Could you c

Re: RFR: 8322766: Micro bench SSLHandshake would use modern algorithms [v2]

2023-12-30 Thread John Jiang
On Sat, 30 Dec 2023 09:20:59 GMT, Daniel Jeliński wrote: > Users usually use the default algorithms, so benchmarking them is probably > more useful than measuring the non-default ones. I see. > There's a [ticket to update the default keyManager > algorithm](https://bugs.openjdk.org/browse/JDK

Re: RFR: 8322766: Micro bench SSLHandshake should use default algorithms [v2]

2024-01-02 Thread John Jiang
On Tue, 2 Jan 2024 13:43:27 GMT, Sean Mullan wrote: >> John Jiang has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Use default algorithms > > Can you change the bug summary to "Micro bench SSLHandsh

Re: RFR: 8322766: Micro bench SSLHandshake should use default algorithms [v2]

2024-01-03 Thread John Jiang
On Sat, 30 Dec 2023 09:20:59 GMT, Daniel Jeliński wrote: >> John Jiang has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Use default algorithms > > Thanks. Users usually use the default algorithms, so benchma

Integrated: 8322766: Micro bench SSLHandshake should use default algorithms

2024-01-03 Thread John Jiang
On Fri, 29 Dec 2023 06:49:26 GMT, John Jiang wrote: > test/micro/org/openjdk/bench/java/security/SSLHandshake.java is using > keystore type `JKS` and TrustManagerFactory/KeyManagerFactory algorithm > `SunX509`. > It may be better to use `PKCS12` and `PKIX` respectively. This pull

RFR: 8320449: ECDHKeyAgreement should validate parameters before using them

2024-01-11 Thread John Jiang
ECDHKeyAgreement should validate the parameters before assigning them to the fields. - Commit messages: - 8320449: ECDHKeyAgreement should validate parameters before using them Changes: https://git.openjdk.org/jdk/pull/17373/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=1

Re: RFR: 8320449: ECDHKeyAgreement should validate parameters before using them

2024-01-12 Thread John Jiang
On Fri, 12 Jan 2024 13:46:43 GMT, Sean Mullan wrote: >> ECDHKeyAgreement should validate the parameters before assigning them to the >> fields. > > src/java.base/share/classes/sun/security/ec/ECDHKeyAgreement.java line 83: > >> 81: privateKey = null; >> 82: privateKeyOps = null;

Re: RFR: 8320449: ECDHKeyAgreement should validate parameters before using them

2024-01-12 Thread John Jiang
On Fri, 12 Jan 2024 20:51:03 GMT, Sean Mullan wrote: >> ECDHKeyAgreement should validate the parameters before assigning them to the >> fields. > > test/jdk/sun/security/ec/ECDHKeyAgreementParamValidation.java line 28: > >> 26: * @bug 8320449 >> 27: * @summary ECDHKeyAgreement should validate

Re: RFR: 8320449: ECDHKeyAgreement should validate parameters before using them [v2]

2024-01-12 Thread John Jiang
> ECDHKeyAgreement should validate the parameters before assigning them to the > fields. John Jiang has updated the pull request incrementally with one additional commit since the last revision: more assertThrows on doPhase - Changes: - all: https://git.openjdk.org/jd

Re: RFR: 8320449: ECDHKeyAgreement should validate parameters before using them [v2]

2024-01-12 Thread John Jiang
On Sat, 13 Jan 2024 01:07:20 GMT, Bernd wrote: >> John Jiang has updated the pull request incrementally with one additional >> commit since the last revision: >> >> more assertThrows on doPhase > > test/jdk/sun/security/ec/ECDHKeyAgreementParamVal

Re: RFR: 8320449: ECDHKeyAgreement should validate parameters before using them [v3]

2024-01-14 Thread John Jiang
> ECDHKeyAgreement should validate the parameters before assigning them to the > fields. John Jiang has updated the pull request incrementally with one additional commit since the last revision: Not use JUnit - Changes: - all: https://git.openjdk.org/jdk/pull/17373

Re: RFR: 8320449: ECDHKeyAgreement should validate parameters before using them [v3]

2024-01-14 Thread John Jiang
On Sun, 14 Jan 2024 19:09:57 GMT, Sean Mullan wrote: >> I originally didn't depend on JUnit. But this tool can easily execute >> multiple test cases independently. >> A single failed case doesn't make the whole test fail fast, and all cases >> always be executed. > > Fair point, although there

Re: RFR: 8320449: ECDHKeyAgreement should validate parameters before using them [v4]

2024-01-16 Thread John Jiang
> ECDHKeyAgreement should validate the parameters before assigning them to the > fields. John Jiang has updated the pull request incrementally with one additional commit since the last revision: more check on generateSecret - Changes: - all: https://git.openjdk.org/jd

Integrated: 8320449: ECDHKeyAgreement should validate parameters before using them

2024-01-16 Thread John Jiang
On Thu, 11 Jan 2024 13:33:54 GMT, John Jiang wrote: > ECDHKeyAgreement should validate the parameters before assigning them to the > fields. This pull request has now been integrated. Changeset: 43d2d68d Author: John Jiang URL: https://git.openjdk.org/jdk/

Re: RFR: 8320449: ECDHKeyAgreement should validate parameters before using them [v4]

2024-01-16 Thread John Jiang
On Tue, 16 Jan 2024 18:44:36 GMT, Sean Mullan wrote: >> John Jiang has updated the pull request incrementally with one additional >> commit since the last revision: >> >> more check on generateSecret > > Marked as reviewed by mullan (Reviewer). @seanjmu

RFR: 8325022: Incorrect error message on TLS 1.2 client authentication

2024-01-30 Thread John Jiang
If the server doesn't receive the client certificate for required client authentication, it should raise error `Empty client certificate chain`. - Commit messages: - 8325022: Incorrect error message on TLS 1.2 client authentication Changes: https://git.openjdk.org/jdk/pull/17645/fi

Re: RFR: 8325022: Incorrect error message on client authentication [v2]

2024-01-31 Thread John Jiang
> If the server doesn't receive the client certificate for required client > authentication, it should raise error `Empty client certificate chain`. John Jiang has updated the pull request incrementally with one additional commit since the last revision: fix more err

Re: RFR: 8325022: Incorrect error message on client authentication [v2]

2024-01-31 Thread John Jiang
On Wed, 31 Jan 2024 20:43:31 GMT, Bernd wrote: >> John Jiang has updated the pull request incrementally with one additional >> commit since the last revision: >> >> fix more error messages > > src/java.base/share/classes/sun/security/ssl/CertificateMe

Integrated: 8325022: Incorrect error message on client authentication

2024-01-31 Thread John Jiang
On Wed, 31 Jan 2024 07:42:32 GMT, John Jiang wrote: > If the server doesn't receive the client certificate for required client > authentication, it should raise error `Empty client certificate chain`. This pull request has now been integrated. Changeset: fe78c0f1 Author: John

Re: RFR: JDK-8311644 Server should not send bad_certificate alert when the client does not send any certificates

2024-02-13 Thread John Jiang
On Tue, 6 Feb 2024 01:23:00 GMT, Anthony Scarpino wrote: > Hi, > > I need a code review of what is really a text change. This changes the alert > type returned during some TLS CertificateMessage failures to what is > recommended in [RFC 8446](https://tools.ietf.org/html/rfc8446). An > addit

Re: RFR: JDK-8311644 Server should not send bad_certificate alert when the client does not send any certificates

2024-02-13 Thread John Jiang
On Tue, 6 Feb 2024 01:23:00 GMT, Anthony Scarpino wrote: > Hi, > > I need a code review of what is really a text change. This changes the alert > type returned during some TLS CertificateMessage failures to what is > recommended in [RFC 8446](https://tools.ietf.org/html/rfc8446). An > addit

Re: RFR: JDK-8311644 Server should not send bad_certificate alert when the client does not send any certificates

2024-02-13 Thread John Jiang
On Tue, 6 Feb 2024 01:23:00 GMT, Anthony Scarpino wrote: > Hi, > > I need a code review of what is really a text change. This changes the alert > type returned during some TLS CertificateMessage failures to what is > recommended in [RFC 8446](https://tools.ietf.org/html/rfc8446). An > addit

Re: RFR: JDK-8311644 Server should not send bad_certificate alert when the client does not send any certificates

2024-02-13 Thread John Jiang
On Tue, 6 Feb 2024 01:23:00 GMT, Anthony Scarpino wrote: > Hi, > > I need a code review of what is really a text change. This changes the alert > type returned during some TLS CertificateMessage failures to what is > recommended in [RFC 8446](https://tools.ietf.org/html/rfc8446). An > addit

Re: RFR: JDK-8311644 Server should not send bad_certificate alert when the client does not send any certificates

2024-02-13 Thread John Jiang
On Tue, 6 Feb 2024 01:23:00 GMT, Anthony Scarpino wrote: > Hi, > > I need a code review of what is really a text change. This changes the alert > type returned during some TLS CertificateMessage failures to what is > recommended in [RFC 8446](https://tools.ietf.org/html/rfc8446). An > addit

Re: RFR: JDK-8311644 Server should not send bad_certificate alert when the client does not send any certificates

2024-02-13 Thread John Jiang
On Tue, 6 Feb 2024 01:23:00 GMT, Anthony Scarpino wrote: > Hi, > > I need a code review of what is really a text change. This changes the alert > type returned during some TLS CertificateMessage failures to what is > recommended in [RFC 8446](https://tools.ietf.org/html/rfc8446). An > addit

Re: RFR: 8325162: Remove duplicate GCMParameters class [v2]

2024-02-13 Thread John Jiang
On Tue, 13 Feb 2024 20:58:17 GMT, Daniel Jeliński wrote: >> Currently we have 2 GCMParameters classes in java.base, one in >> sun.security.util package, the other in com.sun.crypto.provider. >> >> This PR merges the changes from the sun.security.util copy into >> com.sun.crypto.provider, and r

Re: RFR: JDK-8311644 Server should not send bad_certificate alert when the client does not send any certificates [v2]

2024-02-22 Thread John Jiang
On Thu, 22 Feb 2024 22:41:10 GMT, Anthony Scarpino wrote: >> Hi, >> >> I need a code review of what is really a text change. This changes the >> alert type returned during some TLS CertificateMessage failures to what is >> recommended in [RFC 8446](https://tools.ietf.org/html/rfc8446). An

RFR: 8327182: Move serverAlias into the loop

2024-03-03 Thread John Jiang
In method `X509Authentication::createServerPossession`, it looks unnecessary to define variable `serverAlias` out of the for-loop. It may be better to move `serverAlias` into that loop to narrow down the scope. - Commit messages: - 8327182: Move serverAlias into the loop Changes: h

Re: RFR: 8327182: Move serverAlias into the loop

2024-03-03 Thread John Jiang
On Mon, 4 Mar 2024 05:04:13 GMT, Guoxiong Li wrote: >> In method `X509Authentication::createServerPossession`, it looks unnecessary >> to define variable `serverAlias` out of the for-loop. >> It may be better to move `serverAlias` into that loop to narrow down the >> scope. > > src/java.base/sh

Re: RFR: 8327182: Move serverAlias into the loop

2024-03-04 Thread John Jiang
On Mon, 4 Mar 2024 08:26:28 GMT, Guoxiong Li wrote: >> With my understanding, in each iteration, firstly choose an alias from key >> manager with a key type, and then try to get the keys and certificates >> associated with this alias. >> If an alias or its associated keys and certificates have

Re: RFR: 8327461: KeyStore getEntry is not thread-safe [v3]

2024-03-07 Thread John Jiang
On Thu, 7 Mar 2024 23:13:14 GMT, Hai-May Chao wrote: >> Change was made to engineGetEntry() in PKCS12KeyStore to extract the key and >> certificate chain from Entry only once. This is because the entry may get >> updated between engineGetKey() and engineGetCertificateChain() which causes >> in

Re: RFR: 8327182: Move serverAlias into the loop [v2]

2024-03-13 Thread John Jiang
> In method `X509Authentication::createServerPossession`, it looks unnecessary > to define variable `serverAlias` out of the for-loop. > It may be better to move `serverAlias` into that loop to narrow down the > scope. John Jiang has updated the pull request with a new target b

Re: RFR: 8327182: Move serverAlias into the loop [v2]

2024-03-18 Thread John Jiang
On Fri, 15 Mar 2024 13:34:23 GMT, Sean Mullan wrote: >> John Jiang has updated the pull request with a new target base due to a >> merge or a rebase. The pull request now contains two commits: >> >> - Merge branch 'master' into JDK-8327182 >> -

Integrated: 8327182: Move serverAlias into the loop

2024-03-18 Thread John Jiang
On Mon, 4 Mar 2024 03:58:18 GMT, John Jiang wrote: > In method `X509Authentication::createServerPossession`, it looks unnecessary > to define variable `serverAlias` out of the for-loop. > It may be better to move `serverAlias` into that loop to narrow down the > scope. This pull

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

2024-03-18 Thread John Jiang
On Thu, 22 Feb 2024 01:14:24 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, decrypts it

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

2024-03-18 Thread John Jiang
On Thu, 22 Feb 2024 01:14:24 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, decrypts it

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

2024-03-19 Thread John Jiang
On Thu, 22 Feb 2024 01:14:24 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, decrypts it

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

2024-03-19 Thread John Jiang
On Thu, 22 Feb 2024 01:14:24 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, decrypts it

Re: RFR: 8326643: JDK server does not send a dummy change_cipher_spec record after HelloRetryRequest message [v5]

2024-03-20 Thread John Jiang
On Wed, 20 Mar 2024 09:55:34 GMT, Prasadrao Koppula wrote: >> JDK server does not send a dummy change_cipher_spec record after >> HelloRetryRequest message. >> >> According to RFC 8446 (Middlebox Compatibility Mode), if the client sends a >> non-empty session ID in the ClientHello message, th

Re: RFR: 8326643: JDK server does not send a dummy change_cipher_spec record after HelloRetryRequest message [v5]

2024-03-20 Thread John Jiang
On Wed, 20 Mar 2024 09:55:34 GMT, Prasadrao Koppula wrote: >> JDK server does not send a dummy change_cipher_spec record after >> HelloRetryRequest message. >> >> According to RFC 8446 (Middlebox Compatibility Mode), if the client sends a >> non-empty session ID in the ClientHello message, th

Re: RFR: 8326643: JDK server does not send a dummy change_cipher_spec record after HelloRetryRequest message [v5]

2024-03-20 Thread John Jiang
On Thu, 21 Mar 2024 02:23:20 GMT, Prasadrao Koppula wrote: >> test/jdk/javax/net/ssl/TLSv13/EngineOutOfSeqCCS.java line 253: >> >>> 251: } >>> 252: >>> 253: private static void log(String str) { >> >> I suggest method `log` accepts `String...` instead of `String`. >> This method can c

Re: RFR: 8326643: JDK server does not send a dummy change_cipher_spec record after HelloRetryRequest message [v7]

2024-03-20 Thread John Jiang
On Thu, 21 Mar 2024 02:03:39 GMT, Prasadrao Koppula wrote: >> JDK server does not send a dummy change_cipher_spec record after >> HelloRetryRequest message. >> >> According to RFC 8446 (Middlebox Compatibility Mode), if the client sends a >> non-empty session ID in the ClientHello message, th

Re: RFR: 8326643: JDK server does not send a dummy change_cipher_spec record after HelloRetryRequest message [v7]

2024-03-21 Thread John Jiang
On Thu, 21 Mar 2024 02:03:39 GMT, Prasadrao Koppula wrote: >> JDK server does not send a dummy change_cipher_spec record after >> HelloRetryRequest message. >> >> According to RFC 8446 (Middlebox Compatibility Mode), if the client sends a >> non-empty session ID in the ClientHello message, th

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

2024-03-25 Thread John Jiang
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

RFR: 8333046: Clean codes in sun.security.util.math

2024-05-28 Thread John Jiang
A simple cleanup on the changes introduced by JDK-8329538. - Commit messages: - 8333046: Clean codes in sun.security.util.math Changes: https://git.openjdk.org/jdk/pull/19429/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=19429&range=00 Issue: https://bugs.openjdk.org/br

Integrated: 8333046: Clean codes in sun.security.util.math

2024-05-31 Thread John Jiang
On Tue, 28 May 2024 14:42:13 GMT, John Jiang wrote: > A simple cleanup on the changes introduced by JDK-8329538. This pull request has now been integrated. Changeset: c0ce7d87 Author: John Jiang URL: https://git.openjdk.org/jdk/commit/c0ce7d871f09df6bf4a21be3579f3f39a49a77bd St

Re: RFR: 8333046: Clean codes in sun.security.util.math

2024-05-31 Thread John Jiang
On Fri, 31 May 2024 13:53:18 GMT, Weijun Wang wrote: >> A simple cleanup on the changes introduced by JDK-8329538. > > Looks good. Thanks for the cleanup. @wangweij Thanks for your review! - PR Comment: https://git.openjdk.org/jdk/pull/19429#issuecomment-2143311471

Re: RFR: 8328608: Multiple NewSessionTicket support for TLS

2024-06-18 Thread John Jiang
On Wed, 29 May 2024 18:53:55 GMT, Anthony Scarpino wrote: > Hi > > This change is to improve TLS 1.3 session resumption by allowing a TLS server > to send more than one resumption ticket per connection and clients to store > more. Resumption is a quick way to use an existing TLS session to e

Re: RFR: 8328608: Multiple NewSessionTicket support for TLS

2024-06-18 Thread John Jiang
On Wed, 19 Jun 2024 00:04:54 GMT, Anthony Scarpino wrote: >> src/java.base/share/classes/sun/security/ssl/NewSessionTicket.java line 369: >> >>> 367: if (SSLLogger.isOn && SSLLogger.isOn("ssl,handshake")) >>> { >>> 368: SSLLogger.fine("No session ticket prod

Re: RFR: 8328608: Multiple NewSessionTicket support for TLS

2024-06-19 Thread John Jiang
On Wed, 29 May 2024 18:53:55 GMT, Anthony Scarpino wrote: > Hi > > This change is to improve TLS 1.3 session resumption by allowing a TLS server > to send more than one resumption ticket per connection and clients to store > more. Resumption is a quick way to use an existing TLS session to e

Re: RFR: 8328608: Multiple NewSessionTicket support for TLS

2024-06-19 Thread John Jiang
On Wed, 5 Jun 2024 17:33:02 GMT, Anthony Scarpino wrote: >> src/java.base/share/classes/sun/security/util/Cache.java line 683: >> >>> 681: >>> 682: // Limit the number of queue entries. >>> 683: private static final int MAXQUEUESIZE = 10; >> >> What do you think about making th

Re: RFR: 8328608: Multiple NewSessionTicket support for TLS

2024-06-19 Thread John Jiang
On Wed, 29 May 2024 18:53:55 GMT, Anthony Scarpino wrote: > Hi > > This change is to improve TLS 1.3 session resumption by allowing a TLS server > to send more than one resumption ticket per connection and clients to store > more. Resumption is a quick way to use an existing TLS session to e

Re: RFR: 8328608: Multiple NewSessionTicket support for TLS

2024-06-21 Thread John Jiang
On Wed, 29 May 2024 18:53:55 GMT, Anthony Scarpino wrote: > Hi > > This change is to improve TLS 1.3 session resumption by allowing a TLS server > to send more than one resumption ticket per connection and clients to store > more. Resumption is a quick way to use an existing TLS session to e

Re: RFR: 8328608: Multiple NewSessionTicket support for TLS

2024-06-21 Thread John Jiang
On Fri, 21 Jun 2024 15:35:46 GMT, Anthony Scarpino wrote: > Let me start from the beginning and hopefully it will clear things up. > > Today, each Finished connection gets one resumption ticket. That session's > SSLContext contains the cache. When someone wants to resume, they use the > same

RFR: 8364597: Replace THL A29 Limited with Tencent

2025-08-04 Thread John Jiang
`THL A29 Limited` was a `Tencent` company but was dissolved. So, the copyright notes just use `Tencent` directly. - Commit messages: - 8364597: Replace THL A29 Limited with Tencent Changes: https://git.openjdk.org/jdk/pull/26614/files Webrev: https://webrevs.openjdk.org/?repo=jdk&

Integrated: 8364597: Replace THL A29 Limited with Tencent

2025-08-07 Thread John Jiang
On Mon, 4 Aug 2025 07:38:01 GMT, John Jiang wrote: > `THL A29 Limited` was a `Tencent` company but was dissolved. > So, the copyright notes just use `Tencent` directly. This pull request has now been integrated. Changeset: 4c9eadda Author: John Jiang URL: https://git.openjdk.o