Re: RFR: 8212136: Remove BaseSSLSocketImpl finalizer method [v3]

[Bradford Wetmore]

On Tue, 3 May 2022 02:02:58 GMT, Xue-Lei Andrew Fan  wrote:

>>> Thanks for the rewording. Updated.
>> 
>> I made one more tweak that reads better.
>
> Yes, it looks better.  Updated.  Thanks!

Looks good, thanks.

-

PR: https://git.openjdk.java.net/jdk/pull/8065

Re: RFR: 8284490: Remove finalizer method in java.security.jgss [v14]

[Xue-Lei Andrew Fan]

On Mon, 2 May 2022 22:35:08 GMT, Brent Christian  wrote:

> Hi. Sorry, I should have brought this up earlier, but there is a jtreg test 
> library to help with ensuring the GC runs, 
> `test/lib/jdk/test/lib/util/ForceGC.java`. You might consider replacing the 
> test code that runs/checks the GC with `ForceGC.await()`.

It looks like a graceful GC utility.  There are also some other test cases I 
committed that do no use ForceGC.  I filed a [new 
bug](https://bugs.openjdk.java.net/browse/JDK-8286045), and will make the 
update all together.  Thank you.

-

PR: https://git.openjdk.java.net/jdk/pull/8136

Re: RFR: 8212136: Remove BaseSSLSocketImpl finalizer method [v3]

[Xue-Lei Andrew Fan]

On Mon, 2 May 2022 17:55:56 GMT, Bradford Wetmore  wrote:

>> Thanks for the rewording.  Updated.
>
>> Thanks for the rewording. Updated.
> 
> I made one more tweak that reads better.

Yes, it looks better.  Updated.  Thanks!

-

PR: https://git.openjdk.java.net/jdk/pull/8065

Re: RFR: 8212136: Remove BaseSSLSocketImpl finalizer method [v5]

[Xue-Lei Andrew Fan]

> Please review the update to remove finalizer method in the SunJSSE provider 
> implementation.  It is one of the efforts to clean up the use of finalizer 
> method in JDK.

Xue-Lei Andrew Fan has updated the pull request incrementally with one 
additional commit since the last revision:

  More note update

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/8065/files
  - new: https://git.openjdk.java.net/jdk/pull/8065/files/6130f5e6..8caf85af

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=8065=04
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8065=03-04

  Stats: 3 lines in 1 file changed: 1 ins; 0 del; 2 mod
  Patch: https://git.openjdk.java.net/jdk/pull/8065.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/8065/head:pull/8065

PR: https://git.openjdk.java.net/jdk/pull/8065

Re: RFR: JDK-8284688 Minor cleanup could be done in java.security.jgss [v4]

[Weijun Wang]

On Mon, 2 May 2022 22:39:09 GMT, Mark Powers  wrote:

>> 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 11 commits:
> 
>  - sixth iteration
>  - Merge
>  - fifth iteration
>  - Merge
>  - fourth iteration
>  - third iteration
>  - Merge
>  - Merge
>  - Merge
>  - second iteration
>  - ... and 1 more: 
> https://git.openjdk.java.net/jdk/compare/64225e19...5ce46ffb

Many many thanks for this huge code change. No problem found. I have several 
comments and you're free to make your own decision.

src/java.security.jgss/share/classes/org/ietf/jgss/GSSException.java line 333:

> 331: public String getMajorString() {
> 332: 
> 333: return Objects.requireNonNullElseGet(majorString, () -> 
> messages[major - 1]);

Isn't this too fancy?

src/java.security.jgss/share/classes/sun/security/jgss/GSSHeader.java line 267:

> 265: throw new IOException("DerInputStream.getLength(): 
> lengthTag="
> 266:   + tmp + ", "
> 267:   + "too big.");

Combine the 2 lines above.

src/java.security.jgss/share/classes/sun/security/jgss/krb5/SubjectComber.java 
line 154:

> 152: while (iterator.hasNext()) {
> 153: Object obj = iterator.next();
> 154: if (!(obj instanceof 
> @SuppressWarnings("unchecked")KerberosTicket ticket)) {

Does not look fluent to me.

src/java.security.jgss/share/classes/sun/security/jgss/spnego/SpNegoContext.java
 line 876:

> 874: 
> 875: // pass token
> 876: tok = Objects.requireNonNullElseGet(token, () -> new byte[0]);

Is this how `requireNonNullElseGet` is meant to be used?

-

Marked as reviewed by weijun (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/7746

Re: zlib before 1.2.12 allows memory corruption (CVE-2018-25032)

[Vitaly Provodin]

Volker, Bernd,

thanks for the replies - they were really useful

Vitaly


> On 27 Apr 2022, at 14:59, Volker Simonis  wrote:
> 
> Hi Bernd, Vitaly,
> 
> Amazon Corretto [1] also includes the fixes for CVE-2018-25032. This
> is our statement:
> 
> "Based upon our analysis, OpenJDK/Corretto is not affected by
> CVE-2018-25032, because the zlib "memLevel" parameter is not settable
> and is fixed at 8, and the usage of the Z_FIXED strategy is prevented.
> With these settings there is no way to invoke the issue described in
> the CVE and we only include this fix out of an abundance of caution."
> 
> You're right that the vulnerability can also be exploited without the
> Z_FIXED strategy, but in that case only with memLevel set to "1" (see
> [2] for more details).
> 
> Given all the currently available information, I don't think there's a
> reason to worry because of CVE-2018-25032 in the context of Java.
> 
> Best regards,
> Volker
> 
> [1] 
> https://github.com/corretto/corretto-8/blob/release-8.332.08.1/CHANGELOG.md
> [2] https://www.openwall.com/lists/oss-security/2022/03/28/1
> 
> On Wed, Apr 27, 2022 at 1:21 AM Bernd Eckenfels  
> wrote:
>> 
>> Hello Vitaly,
>> 
>> (Personal answer not affiliated with OpenJDK members)
>> 
>> I had also asked about this before, but there was no answer (which is 
>> however not surprising, since it is the policy of OpenJDK and Oracle to not 
>> comment on unfixed security issues).
>> 
>> My hope was, that by reporting it before the April update, the (trivial?) 
>> zlib update would be merged, but it is still the old version according to 
>> the source files. So it depends on build parameters and exploitability of 
>> the weakness if you are still in danger (I guess:).
>> 
>> BTW while I can understand to not publish unfixed problems, it does really 
>> not do the java users a favor to not comment on generally known/published 
>> problems, especially not for 2 quarters.
>> 
>> There is however a ray of light on the horizon, I see CVE-2018-25032 fixed 
>> in the Azul April Release  Notes and asume they provide the update out of 
>> band. (Probably only for Windows binaries, haven’t analysed them yet)
>> 
>> They state:
>>> Our analysis shows that Azul Zulu and OpenJDK are not affected by 
>>> CVE-2018-25032.
>>> In OpenJDK, the Zlib "memLevel" parameter is always set to 8 and can not be 
>>> changed by a
>>> Java code, and the Z_FIXED strategy is permanently disabled. The CVE does 
>>> not apply to Azul
>>> Zulu and OpenJDK with these settings. However, Azul decided to include the 
>>> corresponding
>>> patch to the Zlib library in Azul products just in case someone chooses to 
>>> use Zlib from Azul
>>> Zulu outside of Java applications.
>> 
>> (I am not sure of the analysis is complete I think the z_fixed was not a 
>> strict requirement, but I could be wrong.)
>> 
>> Hopefully the vulnerability group will share their finding in a few month.
>> 
>> Gruss
>> Bernd
>> --
>> http://bernd.eckenfels.net
>> 
>> Von: security-dev  im Auftrag von Vitaly 
>> Provodin 
>> Gesendet: Thursday, April 21, 2022 2:06:57 AM
>> An: security-dev@openjdk.java.net ; 
>> build-...@openjdk.java.net 
>> Cc: Vitaly Provodin 
>> Betreff: zlib before 1.2.12 allows memory corruption (CVE-2018-25032)
>> 
>> Hi all,
>> 
>> Recently we (at JetBrains) were faced with the vulnerability issue 
>> CVE-2018-25032 (zlib before 1.2.12 allows memory corruption…)
>> It is known that Linux, macOS builds uses system’s zlib but Windows - 
>> bundled one (by default).
>> On Linux and macOS users can work around the issue by installing proper zlib 
>> on their systems.
>> Are there any ideas for Windows? - the way building (under Cygwin!) with 
>> system zlib looks unworkable in case if Cygwin is not installed on user's 
>> machines.
>> 
>> It looks like after implementing 
>> https://bugs.openjdk.java.net/browse/JDK-8249963 (which also discussed here 
>> https://mail.openjdk.java.net/pipermail/core-libs-dev/2020-July/067868.html) 
>> the resolution of such issues can be shifted to users but what can be done 
>> now?
>> 
>> Thanks,
>> Vitaly

Re: RFR: 8253176: Signature.getParameters should specify that it can throw UnsupportedOperationException [v2]

[Weijun Wang]

On Mon, 2 May 2022 21:14:21 GMT, Valerie Peng  wrote:

>> Then what does "cannot generate parameter values" mean? Any example?
>
> An example is RSASSA-PSS, i.e. it requires the caller to explicitly state 
> which message digest to use, etc.

You listed 2 cases when null is returned: 1) not supplied. 2) cannot generate. 
My understanding is that the RSASSA-PSS case is 1), and the EdDSA case is 2). 
This is why I suggested an "or" relation between them.

-

PR: https://git.openjdk.java.net/jdk/pull/8396

Re: RFR: 8209038: Clarify the javadoc of Cipher.getParameters() [v4]

[Valerie Peng]

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

Valerie Peng has updated the pull request incrementally with one additional 
commit since the last revision:

  update to address review feedback and minor javadoc cleanup

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/8117/files
  - new: https://git.openjdk.java.net/jdk/pull/8117/files/baae26fe..77e8fa0d

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=8117=03
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8117=02-03

  Stats: 171 lines in 2 files changed: 1 ins; 0 del; 170 mod
  Patch: https://git.openjdk.java.net/jdk/pull/8117.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/8117/head:pull/8117

PR: https://git.openjdk.java.net/jdk/pull/8117

Re: RFR: 8255552: Add DES/3DES/MD5 to jdk.security.legacyAlgorithms [v5]

[Hai-May Chao]

On Mon, 2 May 2022 22:38:18 GMT, Weijun Wang  wrote:

>> Hai-May Chao has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Updated spec in java.security
>
> test/jdk/sun/security/tools/keytool/WeakSecretKeyTest.java line 66:
> 
>> 64: .shouldContain("Warning")
>> 65: .shouldMatch(" uses the DESede 
>> algorithm.*considered a security risk")
>> 66: .shouldMatch(" uses the DES/CBC 
>> algorithm.*considered a security risk")
> 
> Please update "DES/CBC" to "DES". I've just fixed it with 
> https://github.com/openjdk/jdk/commit/50a4df87c87febdf5fa8561b7d0d21b8d6623943.

Ok, thanks for the heads-up.

-

PR: https://git.openjdk.java.net/jdk/pull/8300

Integrated: 8286024: PKCS12 keystore shows "DES/CBC" as the algorithm of a DES SecretKeyEntry

[Weijun Wang]

On Mon, 2 May 2022 17:41:52 GMT, Weijun Wang  wrote:

> PKCS12 stores the object identifier of a SecretKey along with it, and when 
> retrieved, translate the object identifier to an algorithm name. 
> Unfortunately, inside `KnownOIDs.java`, "DES" is [only registered 
> as](https://github.com/wangweij/jdk/blob/7a6cbef157b67bb4fb877617f2a23228aade9a5d/src/java.base/share/classes/sun/security/util/KnownOIDs.java#L368-L368)
>  an alias of another name "DES/CBC". We should modify it to "DES" before 
> returning the secret key.

This pull request has now been integrated.

Changeset: 50a4df87
Author:Weijun Wang 
URL:   
https://git.openjdk.java.net/jdk/commit/50a4df87c87febdf5fa8561b7d0d21b8d6623943
Stats: 53 lines in 2 files changed: 52 ins; 0 del; 1 mod

8286024: PKCS12 keystore shows "DES/CBC" as the algorithm of a DES 
SecretKeyEntry

Reviewed-by: valeriep

-

PR: https://git.openjdk.java.net/jdk/pull/8505

Re: RFR: 8255552: Add DES/3DES/MD5 to jdk.security.legacyAlgorithms [v5]

[Weijun Wang]

On Fri, 29 Apr 2022 19:42:27 GMT, Hai-May Chao  wrote:

>> Please review these changes to add DES/3DES/MD5 to 
>> `jdk.security.legacyAlgorithms` security property, and to add the legacy 
>> algorithm constraint checking to `keytool` commands that are associated with 
>> secret key entries stored in the keystore. These `keytool` commands are 
>> -genseckey, -importpass, -list, and -importkeystore. As a result, `keytool` 
>> will be able to generate warnings when it detects that the secret key based 
>> algorithms and PBE based Mac and cipher algorithms are weak. Also removes 
>> the "This algorithm will be disabled in a future update.” from the existing 
>> warnings for the asymmetric keys/certificates.
>> Will also file a CSR.
>
> Hai-May Chao has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Updated spec in java.security

test/jdk/sun/security/tools/keytool/WeakSecretKeyTest.java line 66:

> 64: .shouldContain("Warning")
> 65: .shouldMatch(" uses the DESede 
> algorithm.*considered a security risk")
> 66: .shouldMatch(" uses the DES/CBC 
> algorithm.*considered a security risk")

Please update "DES/CBC" to "DES". I've just fixed it with 
https://github.com/openjdk/jdk/commit/50a4df87c87febdf5fa8561b7d0d21b8d6623943.

-

PR: https://git.openjdk.java.net/jdk/pull/8300

Re: RFR: JDK-8284688 Minor cleanup could be done in java.security.jgss [v4]

[Mark Powers]

> 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 11 commits:

 - sixth iteration
 - Merge
 - fifth iteration
 - Merge
 - fourth iteration
 - third iteration
 - Merge
 - Merge
 - Merge
 - second iteration
 - ... and 1 more: https://git.openjdk.java.net/jdk/compare/64225e19...5ce46ffb

-

Changes: https://git.openjdk.java.net/jdk/pull/7746/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=7746=03
  Stats: 927 lines in 73 files changed: 88 ins; 193 del; 646 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: 8284490: Remove finalizer method in java.security.jgss [v14]

[Brent Christian]

On Mon, 2 May 2022 15:27:39 GMT, Xue-Lei Andrew Fan  wrote:

>> Please review the update to remove finalizer method in the 
>> java.security.jgss module. It is one of the efforts to clean up the use of 
>> finalizer method in JDK.
>
> Xue-Lei Andrew Fan has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   add intermittent test keyword

Hi. Sorry, I should have brought this up earlier, but there is a jtreg test 
library to help with ensuring the GC runs, 
`test/lib/jdk/test/lib/util/ForceGC.java`. You might consider replacing the 
test code that runs/checks the GC with `ForceGC.await()`.

-

PR: https://git.openjdk.java.net/jdk/pull/8136

Re: RFR: 8286024: PKCS12 keystore shows "DES/CBC" as the algorithm of a DES SecretKeyEntry

[Valerie Peng]

On Mon, 2 May 2022 17:41:52 GMT, Weijun Wang  wrote:

> PKCS12 stores the object identifier of a SecretKey along with it, and when 
> retrieved, translate the object identifier to an algorithm name. 
> Unfortunately, inside `KnownOIDs.java`, "DES" is [only registered 
> as](https://github.com/wangweij/jdk/blob/7a6cbef157b67bb4fb877617f2a23228aade9a5d/src/java.base/share/classes/sun/security/util/KnownOIDs.java#L368-L368)
>  an alias of another name "DES/CBC". We should modify it to "DES" before 
> returning the secret key.

Looks fine. Thanks.

-

Marked as reviewed by valeriep (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/8505

Re: RFR: 8253176: Signature.getParameters should specify that it can throw UnsupportedOperationException [v2]

[Valerie Peng]

On Fri, 29 Apr 2022 04:27:36 GMT, Xue-Lei Andrew Fan  wrote:

>> What kind of additional sentence do you have in mind?
>
>> What kind of additional sentence do you have in mind?
> 
> It may be fine to put it into the state for 'null" returned value.  For 
> example:
> 
> 
> The returned parameters may be the same that were used to initialize
> this signature, or may contain additional default or random parameter
> values used by the underlying signature implementation, or null if the
> underlying signature implementation does not support returning the
> parameters as {@code AlgorithmParameters}.
> 
> 
> 
> The null return conditional in the following sentence may be able to combine 
> together.
> 
> 
> The returned parameters may be the same that were used to initialize
> this signature, or may contain additional default or random parameter
> values used by the underlying signature implementation.  {@code null}
> may be returned if the underlying signature implementation does not
> support returning the parameters as {@code AlgorithmParameters}, or  conditions>

How about the case when no parameters are given? Say A is the user-supplied 
values, B is the provider specific default or random values, your suggestion 
has A, A+B, and null. Isn't the sentence about B needed (no A and provider can 
generate the parameters)?

-

PR: https://git.openjdk.java.net/jdk/pull/8396

Re: RFR: 8253176: Signature.getParameters should specify that it can throw UnsupportedOperationException [v2]

[Valerie Peng]

On Thu, 28 Apr 2022 23:28:39 GMT, Weijun Wang  wrote:

>> The impl does not need to generate parameter values, but rather cannot 
>> convert the supplied parameter values into AlgorithmParameter objects. By 
>> parameter values, I mean the components of the parameters.
>
> Then what does "cannot generate parameter values" mean? Any example?

An example is RSASSA-PSS, i.e. it requires the caller to explicitly state which 
message digest to use, etc.

-

PR: https://git.openjdk.java.net/jdk/pull/8396

RFR: 8286024: PKCS12 keystore shows "DES/CBC" as the algorithm of a DES SecretKeyEntry

[Weijun Wang]

PKCS12 stores the object identifier of a SecretKey along with it, and when 
retrieved, translate the object identifier to an algorithm name. Unfortunately, 
inside `KnownOIDs.java`, "DES" is [only registered 
as](https://github.com/wangweij/jdk/blob/7a6cbef157b67bb4fb877617f2a23228aade9a5d/src/java.base/share/classes/sun/security/util/KnownOIDs.java#L368-L368)
 an alias of another name "DES/CBC". We should modify it to "DES" before 
returning the secret key.

-

Commit messages:
 - remove exe bits
 - fix

Changes: https://git.openjdk.java.net/jdk/pull/8505/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8505=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8286024
  Stats: 53 lines in 2 files changed: 52 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/8505.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/8505/head:pull/8505

PR: https://git.openjdk.java.net/jdk/pull/8505

Re: RFR: 8209038: Clarify the javadoc of Cipher.getParameters() [v3]

[Valerie Peng]

On Fri, 29 Apr 2022 15:23:47 GMT, Sean Mullan  wrote:

>> Valerie Peng has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Update for getParameters()
>
> src/java.base/share/classes/javax/crypto/Cipher.java line 1053:
> 
>> 1051:  * The returned parameters may be the same that were used to 
>> initialize
>> 1052:  * this cipher, or may contain additional default or random 
>> parameter
>> 1053:  * values used by the underlying cipher implementation. If the 
>> required
> 
> The PR for https://github.com/openjdk/jdk/pull/8396/ has one difference in 
> this sentence in that it says "... values used by the underlying signature 
> implementation _if the underlying signature implementation supports returning 
> the parameters as {@code AlgorithmParameters}_." Should this also be 
> consistent?
> 
> Also, I don't think you need to repeat "underlying signature " again above, 
> you could just say: ... values used by the underlying signature 
> implementation _if the implementation supports returning the parameters as 
> {@code AlgorithmParameters}_."

Signature class has that sentence is due to the peculiar case of EdDSA 
signature impl where its parameter spec does not correlate to an ASN.1 
structure. Since there is no such case for current cipher impls (as far as I 
know), that's why I have not added it to Cipher class. 

Ok, how about `If the required parameters were not supplied and can be 
generated by the cipher, the generated parameters will be returned.`?

-

PR: https://git.openjdk.java.net/jdk/pull/8117

Re: RFR: 8209038: Clarify the javadoc of Cipher.getParameters() [v3]

[Valerie Peng]

On Fri, 29 Apr 2022 15:18:34 GMT, Sean Mullan  wrote:

>> Valerie Peng has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Update for getParameters()
>
> src/java.base/share/classes/javax/crypto/Cipher.java line 1055:
> 
>> 1053:  * values used by the underlying cipher implementation. If the 
>> required
>> 1054:  * parameters were not supplied and the underlying cipher 
>> implementation
>> 1055:  * can generate the parameter values, it will be returned. 
>> Otherwise,
> 
> "it will be returned" sounds a bit awkward here. I would change this to "the 
> generated parameter values will be returned".

Ok.

-

PR: https://git.openjdk.java.net/jdk/pull/8117

Re: RFR: 8209038: Clarify the javadoc of Cipher.getParameters() [v3]

[Valerie Peng]

On Thu, 28 Apr 2022 19:23:18 GMT, Weijun Wang  wrote:

>> Valerie Peng has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Update for getParameters()
>
> src/java.base/share/classes/javax/crypto/CipherSpi.java line 449:
> 
>> 447:  *
>> 448:  * Note that when a Cipher object is initialized, it loses all
>> 449:  * previously-acquired state. In other words, initializing a Cipher 
>> is
> 
> Two `{@code Cipher}` above.

Yes, I will look through the whole class (and CipherSpi.java too) regarding the 
Cipher vs cipher thing.

-

PR: https://git.openjdk.java.net/jdk/pull/8117

Re: RFR: 8209038: Clarify the javadoc of Cipher.getParameters() [v3]

[Valerie Peng]

On Thu, 28 Apr 2022 19:17:08 GMT, Weijun Wang  wrote:

>> Valerie Peng has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Update for getParameters()
>
> src/java.base/share/classes/javax/crypto/Cipher.java line 1056:
> 
>> 1054:  * parameters were not supplied and the underlying cipher 
>> implementation
>> 1055:  * can generate the parameter values, it will be returned. 
>> Otherwise,
>> 1056:  * {@code null} returned.
> 
> Should this be "null is returned"?

Yes, will fix.

> src/java.base/share/classes/javax/crypto/Cipher.java line 1787:
> 
>> 1785:  * Ensures that Cipher is in a valid state for update() and 
>> doFinal()
>> 1786:  * calls - should be initialized and in ENCRYPT_MODE or 
>> DECRYPT_MODE.
>> 1787:  * @throws IllegalStateException if Cipher object is not in valid 
>> state
> 
> "Cipher" in `{@code}`? Or make it lowercase.

Ok, I will use lowercase.

-

PR: https://git.openjdk.java.net/jdk/pull/8117

Re: RFR: 8212136: Remove BaseSSLSocketImpl finalizer method [v3]

[Bradford Wetmore]

On Mon, 2 May 2022 17:45:44 GMT, Xue-Lei Andrew Fan  wrote:

> Thanks for the rewording. Updated.

I made one more tweak that reads better.

-

PR: https://git.openjdk.java.net/jdk/pull/8065

Re: RFR: 8212136: Remove BaseSSLSocketImpl finalizer method [v3]

[Xue-Lei Andrew Fan]

On Mon, 2 May 2022 16:46:17 GMT, Bradford Wetmore  wrote:

>> Xue-Lei Andrew Fan has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   comment about remove finalize() method
>
> src/java.base/share/classes/sun/security/ssl/BaseSSLSocketImpl.java line 44:
> 
>> 42:  * overridden in SSLSocketImpl.
>> 43:  *
>> 44:  * There used to be a finalize() implementation, but decided that was
> 
> Since you haven't pushed yet, maybe:
> 
> There used to be a finalize() implementation that sent close_notify's, 
> but decided that was not needed.  Not closing properly is more properly an 
> error condition that should be avoided.  Applications should close sockets 
> and not rely on garbage collection.  
> 
> The underlying native resources are handled by the Socket Cleaner.

Thanks for the rewording.  Updated.

-

PR: https://git.openjdk.java.net/jdk/pull/8065

Re: RFR: 8212136: Remove BaseSSLSocketImpl finalizer method [v4]

[Xue-Lei Andrew Fan]

> Please review the update to remove finalizer method in the SunJSSE provider 
> implementation.  It is one of the efforts to clean up the use of finalizer 
> method in JDK.

Xue-Lei Andrew Fan has updated the pull request incrementally with one 
additional commit since the last revision:

  update the note

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/8065/files
  - new: https://git.openjdk.java.net/jdk/pull/8065/files/c90e25a1..6130f5e6

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=8065=03
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8065=02-03

  Stats: 5 lines in 1 file changed: 1 ins; 0 del; 4 mod
  Patch: https://git.openjdk.java.net/jdk/pull/8065.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/8065/head:pull/8065

PR: https://git.openjdk.java.net/jdk/pull/8065

Re: RFR: 8255552: Add DES/3DES/MD5 to jdk.security.legacyAlgorithms [v5]

[Weijun Wang]

On Fri, 29 Apr 2022 19:42:27 GMT, Hai-May Chao  wrote:

>> Please review these changes to add DES/3DES/MD5 to 
>> `jdk.security.legacyAlgorithms` security property, and to add the legacy 
>> algorithm constraint checking to `keytool` commands that are associated with 
>> secret key entries stored in the keystore. These `keytool` commands are 
>> -genseckey, -importpass, -list, and -importkeystore. As a result, `keytool` 
>> will be able to generate warnings when it detects that the secret key based 
>> algorithms and PBE based Mac and cipher algorithms are weak. Also removes 
>> the "This algorithm will be disabled in a future update.” from the existing 
>> warnings for the asymmetric keys/certificates.
>> Will also file a CSR.
>
> Hai-May Chao has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Updated spec in java.security

If one generates a secret key with `-keyalg des`, then the warning message also 
uses lowercase `des`. It probably should be standardized.

-

PR: https://git.openjdk.java.net/jdk/pull/8300

Re: RFR: 8212136: Remove BaseSSLSocketImpl finalizer method [v3]

[Bradford Wetmore]

On Mon, 2 May 2022 05:01:21 GMT, Xue-Lei Andrew Fan  wrote:

>> Please review the update to remove finalizer method in the SunJSSE provider 
>> implementation.  It is one of the efforts to clean up the use of finalizer 
>> method in JDK.
>
> Xue-Lei Andrew Fan has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   comment about remove finalize() method

src/java.base/share/classes/sun/security/ssl/BaseSSLSocketImpl.java line 44:

> 42:  * overridden in SSLSocketImpl.
> 43:  *
> 44:  * There used to be a finalize() implementation, but decided that was

Since you haven't pushed yet, maybe:

There used to be a finalize() implementation that sent close_notify's, but 
decided that was not needed.  An application should close its sockets and not 
let them get garbage collected without closing them.  

The underlying native resources are handled by the Socket Cleaner.

-

PR: https://git.openjdk.java.net/jdk/pull/8065

Re: RFR: 8255552: Add DES/3DES/MD5 to jdk.security.legacyAlgorithms [v5]

[Sean Mullan]

On Fri, 29 Apr 2022 19:42:27 GMT, Hai-May Chao  wrote:

>> Please review these changes to add DES/3DES/MD5 to 
>> `jdk.security.legacyAlgorithms` security property, and to add the legacy 
>> algorithm constraint checking to `keytool` commands that are associated with 
>> secret key entries stored in the keystore. These `keytool` commands are 
>> -genseckey, -importpass, -list, and -importkeystore. As a result, `keytool` 
>> will be able to generate warnings when it detects that the secret key based 
>> algorithms and PBE based Mac and cipher algorithms are weak. Also removes 
>> the "This algorithm will be disabled in a future update.” from the existing 
>> warnings for the asymmetric keys/certificates.
>> Will also file a CSR.
>
> Hai-May Chao has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Updated spec in java.security

Changes requested by mullan (Reviewer).

src/java.base/share/classes/sun/security/tools/keytool/Main.java line 2196:

> 2194: 
> 2195: try {
> 2196: SecretKey secKey = (SecretKey) keyStore.getKey(alias, 
> storePass);

This means any secret key entries that are protected by a different password 
than `storePass` will throw an `UnrecoverableKeyException` and we will not be 
able to check the constraints. For PKCS12 this is not an issue since 
`storePass` and `keyPass` have to be the same. But for other keystores like 
JCEKS it might be a problem. However, I note this is not really a new issue as 
details about secret key entries other than the fact they exist as entries are 
not listed, presumably because we may not have the right password. 

I would recommend adding a comment explaining this.

For a future RFE, it might be useful to enhance `keytool -list -alias` to have 
a `-keypass` option so that additional information for entries protected by a 
different password than `storePass` could be listed, such as their algorithm 
and key size.

src/java.base/share/classes/sun/security/tools/keytool/Main.java line 5039:

> 5037: private void checkWeakConstraint(String label, String algName,
> 5038: ConstraintsParameters cp) {
> 5039: // Do not check disabled algorithms for symmetric key based 
> algorithms for now

If this method is intended only to be called for symmetric keys, you should 
change the type of `cp` to `SecretKeyConstraintsParameters`.

src/java.base/share/classes/sun/security/tools/keytool/Main.java line 5248:

> 5246: private static class SecretKeyConstraintsParameters implements 
> ConstraintsParameters {
> 5247: private final Key key;
> 5248: public SecretKeyConstraintsParameters(Key key) {

This ctor could just be private.

src/java.base/share/classes/sun/security/tools/keytool/Main.java line 5259:

> 5257: @Override
> 5258: public Set getKeys() {
> 5259: return null;

You should return `Set.of(key)` here. This allows the constraints to also check 
the key size, if that is also restricted. I would suggest adding a test where 
`jdk.security.legacyAlgorithms` includes a constraint for "AES < 256" to see if 
`keytool` warns about it when generating an AES key of 128 bits.

-

PR: https://git.openjdk.java.net/jdk/pull/8300

Interesting in TLS Ticket Requests

[xueleifan(XueleiFan)]

Hi,


A new standard, RFC 9149 TLS Ticket Requests, was published on April 2022. Is 
anyone interested in have it implemented in JDK?

As described in RFC 8446/TLS 1.3, TLS servers vend clients an arbitrary number 
of session tickets for session resumption.  However, the number may be not what 
clients desired.  For security reason, session ticket can be used only one 
time.  If the client desired number and server supplied number does not match, 
the performance impact could be significant.

Currently, in JDK, the server vends only one session ticket for each 
handshaking.  However, clients can open parallel TLS connections to the same 
server for HTTP, or they can race TLS connections across different network 
interfaces.  In such circumstances, one session ticket is hardly to be good for 
TLS connections performance.  But, without an explicit desired number requested 
from the client, it is hard to know what is the best number of session tickets 
that the server should vend to clients.  The issues could be addressed with RFC 
9149, TLS Ticket Requests.

Are you interested to have the TLS Ticket Requests feature in JDK?  Please let 
me know your ideas and concerns.

Thanks,
Xuelei

Re: RFR: 8284490: Remove finalizer method in java.security.jgss [v14]

[Xue-Lei Andrew Fan]

> Please review the update to remove finalizer method in the java.security.jgss 
> module. It is one of the efforts to clean up the use of finalizer method in 
> JDK.

Xue-Lei Andrew Fan has updated the pull request incrementally with one 
additional commit since the last revision:

  add intermittent test keyword

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/8136/files
  - new: https://git.openjdk.java.net/jdk/pull/8136/files/d554c724..e933d404

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=8136=13
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8136=12-13

  Stats: 3 lines in 3 files changed: 3 ins; 0 del; 0 mod
  Patch: https://git.openjdk.java.net/jdk/pull/8136.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/8136/head:pull/8136

PR: https://git.openjdk.java.net/jdk/pull/8136

Re: RFR: 8284490: Remove finalizer method in java.security.jgss [v13]

[Weijun Wang]

On Thu, 28 Apr 2022 18:32:31 GMT, Xue-Lei Andrew Fan  wrote:

>> Please review the update to remove finalizer method in the 
>> java.security.jgss module. It is one of the efforts to clean up the use of 
>> finalizer method in JDK.
>
> Xue-Lei Andrew Fan has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   add sleep back

LGTM

-

Marked as reviewed by weijun (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/8136

Interesting in DTLS 1.3

[xueleifan(XueleiFan)]

Hi,

The Datagram Transport Layer Security (DTLS) Protocol Version 1.3 (DTLS 1.3) 
has been published on April 2022. 
The specification describes the most current version of the DTLS protocol as a 
delta from TLS 1.3 and obsoletes DTLS 1.2.

In JDK, the Java specifications for DTLS 1.0 and DTLS 1.2 are 
defined
 and 
implemented.
 For DTLS 1.3, there may be three different choices for JDK.

The 1st one is doing nothing, and JDK will not support DTLS 1.3 in the future.

The 2nd one is define the specification in JDK, but without the implementation. 
 Third party’s provider could have the implementation if this feature is 
required in some circumstances.

The 3rd one, as DTLS 1.0 and DTLS 1.2, is to have an implementation in JDK.

Are you using DTLS protocols in your applications?  Is anyone interested in 
have DTLS 1.3 in JDK? Which option is best for you?

Thanks,
Xuelei

Integrated: 8285827: Describe the keystore.pkcs12.legacy system property in the java.security file

[Weijun Wang]

On Thu, 28 Apr 2022 14:35:54 GMT, Weijun Wang  wrote:

> We added a new system property back in 
> https://bugs.openjdk.java.net/browse/JDK-8153005 but it's better to describe 
> it in the `java.security` file as well.
> 
> Please review the text. I especially added the last sentence so that people 
> won't set `-Dkeystore.pkcs12.legacy=false`.

This pull request has now been integrated.

Changeset: cfcba1fc
Author:Weijun Wang 
URL:   
https://git.openjdk.java.net/jdk/commit/cfcba1fccc8e3e6a68e1cb1826b70e076d5d83c4
Stats: 24 lines in 1 file changed: 20 ins; 0 del; 4 mod

8285827: Describe the keystore.pkcs12.legacy system property in the 
java.security file

Reviewed-by: mullan

-

PR: https://git.openjdk.java.net/jdk/pull/8452

Re: RFR: 8285827: Describe the keystore.pkcs12.legacy system property in the java.security file [v2]

[Sean Mullan]

On Fri, 29 Apr 2022 21:49:32 GMT, Weijun Wang  wrote:

>> We added a new system property back in 
>> https://bugs.openjdk.java.net/browse/JDK-8153005 but it's better to describe 
>> it in the `java.security` file as well.
>> 
>> Please review the text. I especially added the last sentence so that people 
>> won't set `-Dkeystore.pkcs12.legacy=false`.
>
> Weijun Wang has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   clearer text

Marked as reviewed by mullan (Reviewer).

-

PR: https://git.openjdk.java.net/jdk/pull/8452

Re: Timeframe for JEP-411 completely removing SecurityManager APIs

[Peter Firmstone]

Hi Arjan,

Java 8 is supported until 2030, and 17 to 2029, we would be unable to 
continue testing against new Java releases.


https://bugs.openjdk.java.net/browse/JDK-8272340

With this choice, there will be incompatible Java versions we must 
prevent our software from running on, until it becomes possible to 
support a later version.


It also means that someone who wants to use new Java API's on a Java 
version we don't support may be forced to migrate away from using our 
software.   It also means that we will be unable to take advantage of 
new Java API's which may put us at a significant disadvantage.


Regards,

Peter.

On 2/05/2022 5:24 pm, arjan tijms wrote:

Hi,

On Monday, May 2, 2022, Peter Firmstone  
wrote:


I guess I'm just trying to say we need more time, the process of
extricating SM for security will take years, if we can leave SM as
it is in deprecated form for a number of years, that would be
greatly appreciated.


Just wondering, but would it not be an option to just keep using the 
last LTS that has the SM? That may well enable you to keep using the 
SM for something like 10 years?


Kind regards,
Arjan Tijms



Regards,

Peter.

On 27/04/2022 3:38 am, Sean Mullan wrote:



On 4/26/22 1:06 PM, Scott Stark wrote:

By "migration feature" I'm talking about being able to
retain the type of library code where one has a
conditional call to an AccessController::doPrivileged(...)
method that is only done when System.getSecurityManager()
is not null. Not having to remove this code in all
dependent libraries for a given Jakarta EE application
server product in order to run on Java SE 21 is seen as
necessary to navigate supporting application servers over
a range of Java SE versions. The general consensus was
that having to deal with Java SE 11, 17 and 21 would only
be possible if this SecurityManager related code could
remain as is, even if the only executed path would be
for System.getSecurityManager() == null. We can deal with
a gradual degradation of the SecurityManager behavior, but
it was unclear if Java SE 21 was looking for a complete
removal of the APIs the libraries use.


Yes, we understand these concerns. We recognize the
compatibility issues and the importance for code using the SM
APIs to continue to work as if an SM has not been enabled.
This is the motivation behind the language in the JEP that
discusses a gradual degradation and phasing out of the SM APIs
until the compatibility risk is low enough that removal is
acceptable.

Also, you mention SE 21, but as of yet there is not yet a
targeted release for the SM removal. There will likely be a
JEP for the removal of the SM and this will need to go through
several phases of the JEP process before it can be targeted to
a specific release.

I'm sure many of the Jakarta EE platform dev members have
code repositories to offer for scanning to aide in
determining when the SecurityManager dependencies have
been removed. If there is a avenue for that information,
please let me know.


Thanks for that offer. I don't have an avenue for that
information yet, but I will see if we can start creating a
list of significant SM-enabled libraries and other projects
that we can monitor over time.

--Sean

Thanks,
Scott

On Apr 26, 2022 at 11:09:22 AM, Sean Mullan
mailto:sean.mul...@oracle.com>>
wrote:

Hello Scott,

On 4/25/22 2:25 PM, Scott Stark wrote:

Hello,

I'm Scott Stark of Red Hat, and a member of the
Jakarta EE platform dev
group (EEPD). I'm currently coordinating the
Jakarta EE 10 release that
is targeting June of this year (2022). The removal
of the
SecurityManager as described in JEP-411 has been a
topic for the EEPD on
may calls this year. Recent discussions make it
clear that any
SecurityManager alternative would need to be taken
up by the EEPD, and
such an effort is going to be a non-trivial
undertaking, and may not be
addressed at all.

A general concern among vendors in the EEPD is the
timeframe for the
code that bridges between the JVM running with and
without a
SecurityManager instance needing to be updated.
 

Re: Timeframe for JEP-411 completely removing SecurityManager APIs

[arjan tijms]

Hi,

On Monday, May 2, 2022, Peter Firmstone  wrote:

I guess I'm just trying to say we need more time, the process of
> extricating SM for security will take years, if we can leave SM as it is in
> deprecated form for a number of years, that would be greatly appreciated.


Just wondering, but would it not be an option to just keep using the last
LTS that has the SM? That may well enable you to keep using the SM for
something like 10 years?

Kind regards,
Arjan Tijms




>
> Regards,
>
> Peter.
>
> On 27/04/2022 3:38 am, Sean Mullan wrote:
>
>>
>>
>> On 4/26/22 1:06 PM, Scott Stark wrote:
>>
>>> By "migration feature" I'm talking about being able to retain the type
>>> of library code where one has a conditional call to an
>>> AccessController::doPrivileged(...) method that is only done when
>>> System.getSecurityManager() is not null. Not having to remove this code in
>>> all dependent libraries for a given Jakarta EE application server product
>>> in order to run on Java SE 21 is seen as necessary to navigate supporting
>>> application servers over a range of Java SE versions. The general consensus
>>> was that having to deal with Java SE 11, 17 and 21 would only be possible
>>> if this SecurityManager related code could remain as is, even if the only
>>> executed path would be for System.getSecurityManager() == null. We can
>>> deal with a gradual degradation of the SecurityManager behavior, but it was
>>> unclear if Java SE 21 was looking for a complete removal of the APIs the
>>> libraries use.
>>>
>>
>> Yes, we understand these concerns. We recognize the compatibility issues
>> and the importance for code using the SM APIs to continue to work as if an
>> SM has not been enabled. This is the motivation behind the language in the
>> JEP that discusses a gradual degradation and phasing out of the SM APIs
>> until the compatibility risk is low enough that removal is acceptable.
>>
>> Also, you mention SE 21, but as of yet there is not yet a targeted
>> release for the SM removal. There will likely be a JEP for the removal of
>> the SM and this will need to go through several phases of the JEP process
>> before it can be targeted to a specific release.
>>
>> I'm sure many of the Jakarta EE platform dev members have code
>>> repositories to offer for scanning to aide in determining when the
>>> SecurityManager dependencies have been removed. If there is a avenue for
>>> that information, please let me know.
>>>
>>
>> Thanks for that offer. I don't have an avenue for that information yet,
>> but I will see if we can start creating a list of significant SM-enabled
>> libraries and other projects that we can monitor over time.
>>
>> --Sean
>>
>> Thanks,
>>> Scott
>>>
>>> On Apr 26, 2022 at 11:09:22 AM, Sean Mullan >> > wrote:
>>>
 Hello Scott,

 On 4/25/22 2:25 PM, Scott Stark wrote:

> Hello,
>
> I'm Scott Stark of Red Hat, and a member of the Jakarta EE platform dev
> group (EEPD). I'm currently coordinating the Jakarta EE 10 release that
> is targeting June of this year (2022). The removal of the
> SecurityManager as described in JEP-411 has been a topic for the EEPD
> on
> may calls this year. Recent discussions make it clear that any
> SecurityManager alternative would need to be taken up by the EEPD, and
> such an effort is going to be a non-trivial undertaking, and may not be
> addressed at all.
>
> A general concern among vendors in the EEPD is the timeframe for the
> code that bridges between the JVM running with and without a
> SecurityManager instance needing to be updated. Such code is the
> subject
> of this JEP-411 paragraph:
>
> "In feature releases after Java 18, we will degrade other Security
> Manager APIs so that they remain in place but with limited or no
> functionality. For example, we may revise
> AccessController::doPrivileged
> simply to run the given action, or revise System::getSecurityManager
> always to return null. This will allow libraries that support the
> Security Manager and were compiled against previous Java releases to
> continue to work without change or even recompilation. We expect to
> remove the APIs once the compatibility risk of doing so declines to an
> acceptable level."
>
> Of particular interest is the timeframe for "remove the APIs once the
> compatibility risk of doing so declines to an acceptable level".
>
> Vendors in EEPD would like to see Java SE 21 ship with a migration
> feature along the lines of the proposed "AccessController::doPrivilege
> d
> simply to run the given action, or revise System::getSecurityManager
> always to return null" behaviors.
>

 Can you clarify what you mean by "a migration feature" and also provide
 some background as to why vendors in EEPD would like to see this? Do you
 mean something like a system property that