Re: RFR: JDK-8284688 Minor cleanup could be done in java.security.jgss [v7]
> https://bugs.openjdk.java.net/browse/JDK-8284688 > > [JDK-8273046](https://bugs.openjdk.java.net/browse/JDK-8273046) is the > umbrella bug for this bug. The changes were too large for a single code > review, so it was decided to split into smaller chunks. This is one such > chunk: > > open/src/java.security.jgss/share/classes/javax/security > open/src/java.security.jgss/share/classes/org/ietf > open/src/java.security.jgss/share/classes/sun/security Mark Powers has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 18 commits: - Merge - reverse IntelliJ unboxing suggestion - Merge - Merge - Merge - Brad comments - Max comments - sixth iteration - Merge - fifth iteration - ... and 8 more: https://git.openjdk.java.net/jdk/compare/2ed75be6...30ca27a2 - Changes: https://git.openjdk.java.net/jdk/pull/7746/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=7746=06 Stats: 894 lines in 72 files changed: 96 ins; 193 del; 605 mod Patch: https://git.openjdk.java.net/jdk/pull/7746.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7746/head:pull/7746 PR: https://git.openjdk.java.net/jdk/pull/7746
Re: RFR: 8253176: Signature.getParameters should specify that it can throw UnsupportedOperationException [v9]
On Tue, 17 May 2022 22:22:36 GMT, Valerie Peng wrote: >> This is to update the method javadoc of >> java.security.Signature.getParameters() with the missing `@throws >> UnsupportedOperationException`. In addition, the wording on the returned >> parameters are updated to match those in Cipher and CipherSpi classes. >> >> CSR will be filed later. >> >> Thanks, >> Valerie > > Valerie Peng has updated the pull request incrementally with one additional > commit since the last revision: > > more minor cleanups for consistencies. src/java.base/share/classes/java/security/SignatureSpi.java line 377: > 375: * > 376: * This method is overridden by providers to initialize > 377: * this signature object with the specified parameter set. 1st paragraph: values, 2nd: set. Make them the same. src/java.base/share/classes/java/security/SignatureSpi.java line 397: > 395: * > 396: * This method is overridden by providers to return the parameters > 397: * used with this signature object. The 2 paragraphs above look the same. Of course, if you believe the 1st paragraph should always be a simple description, that's OK. - PR: https://git.openjdk.java.net/jdk/pull/8396
Re: RFR: 8286908: ECDSA signature should not return parameters
On Tue, 17 May 2022 19:56:22 GMT, Weijun Wang wrote: > Let ECDSA's `engineGetParameters()` always return null. At the same time, > remove the remembered `sigParams` field. One behavior change is that after > calling `setParameter()`, one can call `init()` again with a key using > different parameters. I think this should be allowed since we are reusing the > signature object with a brand new key. > > `setParameter` is kept unchanged to be able to deal with certificates still > having parameters after the signature algorithm object identifier. See > https://bugs.openjdk.java.net/browse/JDK-8225745. > > Also added SHA1withECDSA to the no-NULL list in `KnownOIDs`. > > All security-related tests passed. Marked as reviewed by hchao (Committer). - PR: https://git.openjdk.java.net/jdk/pull/8758
Re: RFR: 8253176: Signature.getParameters should specify that it can throw UnsupportedOperationException [v9]
> This is to update the method javadoc of > java.security.Signature.getParameters() with the missing `@throws > UnsupportedOperationException`. In addition, the wording on the returned > parameters are updated to match those in Cipher and CipherSpi classes. > > CSR will be filed later. > > Thanks, > Valerie Valerie Peng has updated the pull request incrementally with one additional commit since the last revision: more minor cleanups for consistencies. - Changes: - all: https://git.openjdk.java.net/jdk/pull/8396/files - new: https://git.openjdk.java.net/jdk/pull/8396/files/94a584bc..681a5bc4 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8396=08 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8396=07-08 Stats: 43 lines in 2 files changed: 4 ins; 0 del; 39 mod Patch: https://git.openjdk.java.net/jdk/pull/8396.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8396/head:pull/8396 PR: https://git.openjdk.java.net/jdk/pull/8396
Integrated: 8286090: Add RC2/RC4 to jdk.security.legacyAlgorithms
On Sat, 14 May 2022 01:51:34 GMT, Hai-May Chao wrote: > Please review the small change to add RC2 and ARCFOUR to > jdk.security.legacyAlgorithms. So it enables keytool -genseckey, -list, and > -importkeystore commands to warn users when RC2 or ARCFOUR algorithm is used. This pull request has now been integrated. Changeset: 2ed75be6 Author:Hai-May Chao URL: https://git.openjdk.java.net/jdk/commit/2ed75be659503da584cfec9ead5e27665ae900ef Stats: 20 lines in 2 files changed: 18 ins; 0 del; 2 mod 8286090: Add RC2/RC4 to jdk.security.legacyAlgorithms Reviewed-by: mullan - PR: https://git.openjdk.java.net/jdk/pull/8712
Re: RFR: 8286090: Add RC2/RC4 to jdk.security.legacyAlgorithms
On Mon, 16 May 2022 17:17:24 GMT, Sean Mullan wrote: >> Please review the small change to add RC2 and ARCFOUR to >> jdk.security.legacyAlgorithms. So it enables keytool -genseckey, -list, and >> -importkeystore commands to warn users when RC2 or ARCFOUR algorithm is used. > > Marked as reviewed by mullan (Reviewer). @seanjmullan Thanks for the review. - PR: https://git.openjdk.java.net/jdk/pull/8712
Re: RFR: 8283577: SSLEngine.unwrap on read-only input ByteBuffer [v2]
On Sat, 14 May 2022 03:29:14 GMT, Anthony Scarpino wrote: >> Hi, >> >> I need a review of this fix to allow a read-only 'src' buffer to be used >> with SSLEngine.unwrap(). A temporary read-write buffer is created in the >> SSLCipher operation when a read-only buffer is passed. If the 'src' is >> read-write, there is no effect on the current operation >> >> The PR also includes a CSR for an API implementation note to the >> SSLEngine.unwrap. The 'src' buffer may be modified during the decryption >> operation. 'unwrap()' has had this behavior forever, so there is no >> compatibility issue with this note. Using the 'src' buffer for in-place >> decryption was a performance decision. >> >> Tony > > Anthony Scarpino 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 contains four additional > commits since the last revision: > > - review update > - update some nits > - PR ready > - Initial I think read-only has been allowed by the spec from the start, we just never allowed it to work properly. I don't think this opens up possible complications in the code. - PR: https://git.openjdk.java.net/jdk/pull/8462
Integrated: 8283577: SSLEngine.unwrap on read-only input ByteBuffer
On Fri, 29 Apr 2022 03:58:57 GMT, Anthony Scarpino wrote: > Hi, > > I need a review of this fix to allow a read-only 'src' buffer to be used with > SSLEngine.unwrap(). A temporary read-write buffer is created in the SSLCipher > operation when a read-only buffer is passed. If the 'src' is read-write, > there is no effect on the current operation > > The PR also includes a CSR for an API implementation note to the > SSLEngine.unwrap. The 'src' buffer may be modified during the decryption > operation. 'unwrap()' has had this behavior forever, so there is no > compatibility issue with this note. Using the 'src' buffer for in-place > decryption was a performance decision. > > Tony This pull request has now been integrated. Changeset: f17c68ce Author:Anthony Scarpino URL: https://git.openjdk.java.net/jdk/commit/f17c68ce4a0b4f5c3131f4e4626a5a55b7f2f61f Stats: 393 lines in 3 files changed: 291 ins; 20 del; 82 mod 8283577: SSLEngine.unwrap on read-only input ByteBuffer Reviewed-by: wetmore - PR: https://git.openjdk.java.net/jdk/pull/8462
Re: RFR: 8286908: ECDSA signature should not return parameters
On Tue, 17 May 2022 19:56:22 GMT, Weijun Wang wrote: > Let ECDSA's `engineGetParameters()` always return null. At the same time, > remove the remembered `sigParams` field. One behavior change is that after > calling `setParameter()`, one can call `init()` again with a key using > different parameters. I think this should be allowed since we are reusing the > signature object with a brand new key. > > `setParameter` is kept unchanged to be able to deal with certificates still > having parameters after the signature algorithm object identifier. See > https://bugs.openjdk.java.net/browse/JDK-8225745. > > Also added SHA1withECDSA to the no-NULL list in `KnownOIDs`. > > All security-related tests passed. Marked as reviewed by ascarpino (Reviewer). Looks good to me. The changes seem within the spec to me. - PR: https://git.openjdk.java.net/jdk/pull/8758
Integrated: 8002277: Refactor two PBE classes to simplify maintenance
On Tue, 3 May 2022 19:30:40 GMT, Valerie Peng wrote: > This change refactors the PBES2Core and PKCS12PBECipherCore classes in SunJCE > provider as requested in the bug record. Functionality should remain the same > with a clearer and simplified code/control flow with less lines of code. > This should improve readability and maintenance. I enhanced one existing > regression test to test more scenarios. This test would pass before the > proposed change and continues to pass with the proposed changes. This pull request has now been integrated. Changeset: 61ddbef3 Author:Valerie Peng URL: https://git.openjdk.java.net/jdk/commit/61ddbef3681770b7a1f56456f686fcb176063329 Stats: 733 lines in 6 files changed: 185 ins; 402 del; 146 mod 8002277: Refactor two PBE classes to simplify maintenance Reviewed-by: weijun - PR: https://git.openjdk.java.net/jdk/pull/8521
Re: RFR: 8286908: ECDSA signature should not return parameters
On Tue, 17 May 2022 20:27:41 GMT, Jamil Nimeh wrote: > Do the behavioral changes you've cited in the PR description warrant a CSR, > or do you feel this behavioral change is still consistent with the current > Signature API documentation? I think so. In fact, after this change, there's simply no parameters for the signature, so calling `setParameter` should not change any internal state and thus has no effect on other calls. While this is a behavior change, I don't think it has any negative impact to users. - PR: https://git.openjdk.java.net/jdk/pull/8758
Re: RFR: 8286908: ECDSA signature should not return parameters
On Tue, 17 May 2022 19:56:22 GMT, Weijun Wang wrote: > Let ECDSA's `engineGetParameters()` always return null. At the same time, > remove the remembered `sigParams` field. One behavior change is that after > calling `setParameter()`, one can call `init()` again with a key using > different parameters. I think this should be allowed since we are reusing the > signature object with a brand new key. > > `setParameter` is kept unchanged to be able to deal with certificates still > having parameters after the signature algorithm object identifier. See > https://bugs.openjdk.java.net/browse/JDK-8225745. > > Also added SHA1withECDSA to the no-NULL list in `KnownOIDs`. Do the behavioral changes you've cited in the PR description warrant a CSR, or do you feel this behavioral change is still consistent with the current Signature API documentation? - PR: https://git.openjdk.java.net/jdk/pull/8758
RFR: 8286908: ECDSA signature should not return parameters
Let ECDSA's `engineGetParameters()` always return null. At the same time, remove the remembered `sigParams` field. One behavior change is that after calling `setParameter()`, one can call `init()` again with a key using different parameters. I think this should be allowed since we are reusing the signature object with a brand new key. `setParameter` is kept unchanged to be able to deal with certificates still having parameters after the signature algorithm object identifier. See https://bugs.openjdk.java.net/browse/JDK-8225745. Also added SHA1withECDSA to the no-NULL list in `KnownOIDs`. - Commit messages: - the fix Changes: https://git.openjdk.java.net/jdk/pull/8758/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8758=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8286908 Stats: 96 lines in 3 files changed: 59 ins; 29 del; 8 mod Patch: https://git.openjdk.java.net/jdk/pull/8758.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8758/head:pull/8758 PR: https://git.openjdk.java.net/jdk/pull/8758
Re: RFR: 8002277: Refactor two PBE classes to simplify maintenance [v8]
On Thu, 12 May 2022 22:52:59 GMT, Valerie Peng wrote: >> This change refactors the PBES2Core and PKCS12PBECipherCore classes in >> SunJCE provider as requested in the bug record. Functionality should remain >> the same with a clearer and simplified code/control flow with less lines of >> code. This should improve readability and maintenance. I enhanced one >> existing regression test to test more scenarios. This test would pass before >> the proposed change and continues to pass with the proposed changes. > > Valerie Peng has updated the pull request incrementally with one additional > commit since the last revision: > > reset ivSpec and minor nit fix. Marked as reviewed by weijun (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8521
Integrated: 8209038: Clarify the javadoc of Cipher.getParameters()
On Wed, 6 Apr 2022 00:14:04 GMT, Valerie Peng wrote:
> Anyone can help review this javadoc update? The main change is the wording
> for the method javadoc of
> Cipher.getParameters()/CipherSpi.engineGetParameters(). The original wording
> is somewhat restrictive and request is to broaden this to accommodate more
> scenarios such as when null can be returned.
> The rest are minor things like add {@code } to class name and null, and
> remove redundant ".".
>
> Will file CSR after the review is close to being wrapped up.
> Thanks~
This pull request has now been integrated.
Changeset: 0c5ab6da
Author:Valerie Peng
URL:
https://git.openjdk.java.net/jdk/commit/0c5ab6daa93cd063d8fa54880f7b1aa981c27c5f
Stats: 417 lines in 2 files changed: 3 ins; 5 del; 409 mod
8209038: Clarify the javadoc of Cipher.getParameters()
Reviewed-by: xuelei, mullan, weijun
-
PR: https://git.openjdk.java.net/jdk/pull/8117
Re: RFR: 8284926: Share the certificate NamedGroup in SignatureScheme::getSignerOfPreferableAlgorithm [v3]
> It would not to generate the certificate's ECParameterSpec and NamedGroup > multiple times in method `SignatureScheme::getSignerOfPreferableAlgorithm`. John Jiang has updated the pull request incrementally with one additional commit since the last revision: add some comments - Changes: - all: https://git.openjdk.java.net/jdk/pull/8271/files - new: https://git.openjdk.java.net/jdk/pull/8271/files/eb706ae8..cd6e6dba Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8271=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8271=01-02 Stats: 36 lines in 3 files changed: 9 ins; 12 del; 15 mod Patch: https://git.openjdk.java.net/jdk/pull/8271.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8271/head:pull/8271 PR: https://git.openjdk.java.net/jdk/pull/8271
Re: RFR: 8284926: Share the certificate NamedGroup in SignatureScheme::getSignerOfPreferableAlgorithm [v2]
On Mon, 18 Apr 2022 12:37:15 GMT, John Jiang wrote: >> It would not to generate the certificate's ECParameterSpec and NamedGroup >> multiple times in method `SignatureScheme::getSignerOfPreferableAlgorithm`. > > John Jiang has updated the pull request incrementally with one additional > commit since the last revision: > > cache ParamSpec and NamedGroup in X509Possession @XueleiFan Just submitted a new commit and addressed your comments. - PR: https://git.openjdk.java.net/jdk/pull/8271
