TLS v1.3 extensions in TLS v1.2 handshake

2021-05-24 Thread Fridrich Strba
Hello, good people, The java 11 implementation of TLS v1.3 was backported into java 8 since some CPUs and it results sometimes in new handshake failures with hard-to-updage-firmware devices whose shell life might be still long. We somehow debugged those failures and some devices bomb because

Re: RFR: 8255674: SSLEngine class description is missing "case" in switch statement

2021-05-24 Thread Xue-Lei Andrew Fan
On Tue, 25 May 2021 04:36:53 GMT, Jaikiran Pai wrote: > Can I please get a review for this trivial fix in the code sample in javadoc > comment of `javax.net.ssl.SSLEngine` class? > > I've run `make docs-image` locally and the generated javadoc after this > change looks fine. Marked as

RFR: 8255674: SSLEngine class description is missing "case" in switch statement

2021-05-24 Thread Jaikiran Pai
Can I please get a review for this trivial fix in the code sample in javadoc comment of `javax.net.ssl.SSLEngine` class? I've run `make docs-image` locally and the generated javadoc after this change looks fine. - Commit messages: - 8255674 SSLEngine class description is missing

Re: RFR: 8267587: Update java.util to use enhanced switch [v2]

2021-05-24 Thread Tagir F . Valeev
> Inspired by PR#4088. Most of the changes are done automatically using > IntelliJ IDEA refactoring. Some manual adjustments are also performed, > including indentations, moving comments, extracting common cast out of switch > expression branches, etc. > > I also noticed that there are some

Re: RFR: 8267587: Update java.util to use enhanced switch

2021-05-24 Thread Tagir F . Valeev
On Mon, 24 May 2021 13:46:58 GMT, Daniel Fuchs wrote: >> Inspired by PR#4088. Most of the changes are done automatically using >> IntelliJ IDEA refactoring. Some manual adjustments are also performed, >> including indentations, moving comments, extracting common cast out of >> switch

Re: RFR: 8267587: Update java.util to use enhanced switch

2021-05-24 Thread Tagir F . Valeev
On Mon, 24 May 2021 13:44:36 GMT, Daniel Fuchs wrote: >> Inspired by PR#4088. Most of the changes are done automatically using >> IntelliJ IDEA refactoring. Some manual adjustments are also performed, >> including indentations, moving comments, extracting common cast out of >> switch

Re: RFR: 8248268: Support KWP in addition to KW [v7]

2021-05-24 Thread Valerie Peng
On Sat, 22 May 2021 01:02:50 GMT, Xue-Lei Andrew Fan wrote: >> Valerie Peng has updated the pull request with a new target base due to a >> merge or a rebase. The pull request now contains seven commits: >> >> - Merge master into JDK-8248268 >> - Minor update to address review comments. >>

Re: RFR: 8248268: Support KWP in addition to KW [v7]

2021-05-24 Thread Michael StJohns
Some more general comments - related to the restructuring. In AESKeyWrap at 152-155 - that check probably should be moved to W().   KWP should do the formatting prior to passing the data to W().  Also at 185-187 - move that to W_INV(). AESKeyWrap at 158 - shouldn't you be returning the

Re: RFR: 8248268: Support KWP in addition to KW [v7]

2021-05-24 Thread Valerie Peng
On Sat, 22 May 2021 00:45:27 GMT, Xue-Lei Andrew Fan wrote: >> Valerie Peng has updated the pull request with a new target base due to a >> merge or a rebase. The pull request now contains seven commits: >> >> - Merge master into JDK-8248268 >> - Minor update to address review comments. >>

Re: RFR: 8248268: Support KWP in addition to KW [v7]

2021-05-24 Thread Valerie Peng
On Fri, 21 May 2021 20:44:57 GMT, Xue-Lei Andrew Fan wrote: >> Valerie Peng has updated the pull request with a new target base due to a >> merge or a rebase. The pull request now contains seven commits: >> >> - Merge master into JDK-8248268 >> - Minor update to address review comments. >>

Re: RFR: 8255557: Decouple GCM from CipherCore [v5]

2021-05-24 Thread Anthony Scarpino
On Mon, 24 May 2021 20:09:06 GMT, Valerie Peng wrote: >> Anthony Scarpino has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Review comments update > > test/jdk/com/sun/crypto/provider/Cipher/AES/TestAESCipher.java line 90: > >> 88:

Re: RFR: 8248268: Support KWP in addition to KW [v7]

2021-05-24 Thread Valerie Peng
On Fri, 21 May 2021 19:15:49 GMT, Xue-Lei Andrew Fan wrote: >> Valerie Peng has updated the pull request with a new target base due to a >> merge or a rebase. The pull request now contains seven commits: >> >> - Merge master into JDK-8248268 >> - Minor update to address review comments. >>

Re: RFR: 8255557: Decouple GCM from CipherCore [v5]

2021-05-24 Thread Anthony Scarpino
On Mon, 24 May 2021 20:38:29 GMT, Valerie Peng wrote: >> Anthony Scarpino has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Review comments update > > test/jdk/com/sun/crypto/provider/Cipher/AEAD/GCMShortBuffer.java line 34: > >> 32: /*

RFR: 8267543: Post JEP 411 refactoring: security

2021-05-24 Thread Weijun Wang
For all modified files in #4073 having "security", "crypto", or "ssl" in their names. Codes are refactored to bring a `@SuppressWarning` annotation nearer to the deprecated API usage and also shrink the size of its target. - Depends on: https://git.openjdk.java.net/jdk/pull/4073

Re: RFR: 8255557: Decouple GCM from CipherCore [v5]

2021-05-24 Thread Valerie Peng
On Fri, 21 May 2021 21:18:34 GMT, Anthony Scarpino wrote: >> Hi, >> >> I need a review of this rather large change to GCM. GCM will no longer use >> CipherCore, and AESCrypt to handle it's buffers and other objects. It is >> also a major code redesign limits the amount of data copies and

Re: RFR: 8255557: Decouple GCM from CipherCore [v2]

2021-05-24 Thread Anthony Scarpino
On Mon, 24 May 2021 18:15:17 GMT, Valerie Peng wrote: >> src.array() throws an exception if it's read only > > Other files seem to use src.hasArray() call which also checked for read only. > Maybe that's what you meant to use? Yes, I could use hasArray(). I hadn't started using that until

Re: RFR: 8267521: Post JEP 411 refactoring: maximum covering > 50K [v3]

2021-05-24 Thread Weijun Wang
On Mon, 24 May 2021 09:00:07 GMT, Daniel Fuchs wrote: > Thanks for taking in my suggestions for FtpClient. I have also reviewed the > changes to java.util.logging and JMX. I wish there was a way to handle > doPrivileged returning void more nicely. Maybe component maintainers will be > able to

Re: RFR: 8255557: Decouple GCM from CipherCore [v4]

2021-05-24 Thread Valerie Peng
On Fri, 21 May 2021 04:07:05 GMT, Anthony Scarpino wrote: >> src/java.base/share/classes/com/sun/crypto/provider/GaloisCounterMode.java >> line 640: >> >>> 638: * @return number of bytes used from 'in' >>> 639: */ >>> 640: int mergeBlock(byte[] buffer, int bufOfs,

Re: RFR: 8255557: Decouple GCM from CipherCore [v3]

2021-05-24 Thread Valerie Peng
On Mon, 24 May 2021 16:54:39 GMT, Anthony Scarpino wrote: >> src/java.base/share/classes/com/sun/crypto/provider/GHASH.java line 338: >> >>> 336: public int doFinal(ByteBuffer src, ByteBuffer dst) { >>> 337: return doFinal(src, src.remaining()); >>> 338: } >> >> Have you

Re: RFR: 8255557: Decouple GCM from CipherCore [v2]

2021-05-24 Thread Valerie Peng
On Mon, 24 May 2021 16:34:51 GMT, Anthony Scarpino wrote: >> src/java.base/share/classes/com/sun/crypto/provider/GCTR.java line 203: >> >>> 201: // allocating and copying for direct bytebuffers >>> 202: if (!src.isDirect() && !dst.isDirect() && >>> 203:

Re: RFR: 8236671: NullPointerException in JKS keystore [v2]

2021-05-24 Thread Will Sargent
I have tried to sign up to the bug tracking system (through reset password I think?) but I'm not getting an email out, so I can't add to the bug. I have created a test case in Github: https://github.com/wsargent/jca-key-failure/ The stack trace shows the invalid key store entry after saving and

Integrated: 8267184: Add -Djava.security.manager=allow to tests calling System.setSecurityManager

2021-05-24 Thread Weijun Wang
On Mon, 17 May 2021 17:51:36 GMT, Weijun Wang wrote: > Please review the test changes for [JEP > 411](https://openjdk.java.net/jeps/411). > > With JEP 411 and the default value of `-Djava.security.manager` becoming > `disallow`, tests calling `System.setSecurityManager()` need >

Re: RFR: 8255557: Decouple GCM from CipherCore [v3]

2021-05-24 Thread Anthony Scarpino
On Tue, 18 May 2021 18:38:40 GMT, Valerie Peng wrote: >> Anthony Scarpino has updated the pull request incrementally with one >> additional commit since the last revision: >> >> cleanup > > src/java.base/share/classes/com/sun/crypto/provider/GHASH.java line 338: > >> 336: public int

Re: RFR: 8255557: Decouple GCM from CipherCore [v2]

2021-05-24 Thread Anthony Scarpino
On Mon, 17 May 2021 22:42:40 GMT, Valerie Peng wrote: >> Anthony Scarpino has updated the pull request incrementally with two >> additional commits since the last revision: >> >> - review comment updates >> - Fixed the lack of overlap detection with GCMEncrypt.update() > >

Re: RFR: 8255557: Decouple GCM from CipherCore [v3]

2021-05-24 Thread Anthony Scarpino
On Tue, 18 May 2021 17:24:13 GMT, Valerie Peng wrote: >> Anthony Scarpino has updated the pull request incrementally with one >> additional commit since the last revision: >> >> cleanup > > src/java.base/share/classes/com/sun/crypto/provider/GHASH.java line 189: > >> 187:

Integrated: 8267584: The java.security.krb5.realm system property only needs to be defined once

2021-05-24 Thread Weijun Wang
On Sun, 23 May 2021 20:42:24 GMT, Weijun Wang wrote: > A system property only needs to be put in a `{@systemProperty}` tag where > it's first defined. For this one, it's >

Integrated: 8266400: importkeystore fails to a password less pkcs12 keystore

2021-05-24 Thread Hai-May Chao
On Wed, 19 May 2021 19:01:21 GMT, Hai-May Chao wrote: > Please review the fix to address keytool -importkeystore failure when > importing to a password-less PKCS12 keystore. This pull request has now been integrated. Changeset: f2d880c1 Author:Hai-May Chao URL:

Re: RFR: 8266400: importkeystore fails to a password less pkcs12 keystore

2021-05-24 Thread Weijun Wang
On Wed, 19 May 2021 19:01:21 GMT, Hai-May Chao wrote: > Please review the fix to address keytool -importkeystore failure when > importing to a password-less PKCS12 keystore. Marked as reviewed by weijun (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/4119

Re: RFR: 8267184: Add -Djava.security.manager=allow to tests calling System.setSecurityManager [v4]

2021-05-24 Thread Weijun Wang
> Please review the test changes for [JEP > 411](https://openjdk.java.net/jeps/411). > > With JEP 411 and the default value of `-Djava.security.manager` becoming > `disallow`, tests calling `System.setSecurityManager()` need > `-Djava.security.manager=allow` when launched. This PR covers such

Re: RFR: 8267584: The java.security.krb5.realm system property only needs to be defined once

2021-05-24 Thread Sean Mullan
On Sun, 23 May 2021 20:42:24 GMT, Weijun Wang wrote: > A system property only needs to be put in a `{@systemProperty}` tag where > it's first defined. For this one, it's >

Re: RFR: 8266459: Implement JEP 411: Deprecate the Security Manager for Removal [v4]

2021-05-24 Thread Weijun Wang
On Sun, 23 May 2021 16:35:43 GMT, Sean Mullan wrote: >> src/java.base/share/classes/java/lang/SecurityManager.java line 104: >> >>> 102: * method will throw an {@code UnsupportedOperationException}). If the >>> 103: * {@systemProperty java.security.manager} system property is set to >>> the

Re: RFR: 8266459: Implement JEP 411: Deprecate the Security Manager for Removal [v4]

2021-05-24 Thread Weijun Wang
> Please review this implementation of [JEP > 411](https://openjdk.java.net/jeps/411). > > The code change is divided into 3 commits. Please review them one by one. > > 1. > https://github.com/openjdk/jdk/commit/576161d15423f58281e384174d28c9f9be7941a1 > The essential change for this JEP,

Re: RFR: 8267587: Update java.util to use enhanced switch

2021-05-24 Thread Daniel Fuchs
On Mon, 24 May 2021 04:20:23 GMT, Tagir F. Valeev wrote: > Inspired by PR#4088. Most of the changes are done automatically using > IntelliJ IDEA refactoring. Some manual adjustments are also performed, > including indentations, moving comments, extracting common cast out of switch >

Re: RFR: 8267110: Update java.util to use instanceof pattern variable [v5]

2021-05-24 Thread Patrick Concannon
> Hi, > > Could someone please review my code for updating the code in the `java.util` > package to make use of the `instanceof` pattern variable? > > Kind regards, > Patrick Patrick Concannon has updated the pull request with a new target base due to a merge or a rebase. The incremental

Re: RFR: 8267521: Post JEP 411 refactoring: maximum covering > 50K [v3]

2021-05-24 Thread Daniel Fuchs
On Fri, 21 May 2021 20:37:30 GMT, Weijun Wang wrote: >> The code change refactors classes that have a `SuppressWarnings("removal")` >> annotation that covers more than 50KB of code. The big annotation is often >> quite faraway from the actual deprecated API usage it is suppressing, and >>