Re: RFR: 8340327: A common framework to support public key algorithms with standard parameter sets [v10]
On Tue, 8 Oct 2024 18:46:20 GMT, Sean Mullan wrote: >> Weijun Wang has updated the pull request incrementally with one additional >> commit since the last revision: >> >> null check as asserts, and better exception messages > > src/java.base/share/classes/sun/security/provider/NamedSignature.java line 63: > >> 61: >> 62: private Object sk2 = null; >> 63: private Object pk2 = null; > > Nit - don't need to set these to null. OK. - PR Review Comment: https://git.openjdk.org/jdk/pull/21167#discussion_r1792446518
Re: RFR: 8340327: A common framework to support public key algorithms with standard parameter sets [v10]
On Tue, 8 Oct 2024 02:28:35 GMT, Weijun Wang wrote:
>> src/java.base/share/classes/sun/security/pkcs/NamedPKCS8Key.java line 81:
>>
>>> 79: } finally {
>>> 80: val.clear();
>>> 81: }
>>
>> The `this.key` is from the `PKCS8Key` class, right? However, looking at the
>> impl of the `PKCS8Key` class, it looks to me that `key` should be equivalent
>> to the `rawBytes` here instead of a DER bytes with OctetString tag.
>
> Yes, `this.key` is the one inside `PKCS8Key`.
>
> Since EdDSA and XDH, the private key has taken this OCTET in OCTET approach.
> My code is identical to the EdDSA code at
> https://github.com/openjdk/jdk/blob/adca97b659d725b0dd320322297dcbd1b443a047/src/java.base/share/classes/sun/security/ec/ed/EdDSAPrivateKeyImpl.java#L50-L64.
>
> In https://datatracker.ietf.org/doc/html/rfc8410#autoid-7 and
> https://www.ietf.org/archive/id/draft-ietf-lamps-kyber-certificates-04.html#name-private-key-format,
> you can see the definitions:
>
>
>OneAsymmetricKey ::= SEQUENCE {
> version Version,
> privateKeyAlgorithm PrivateKeyAlgorithmIdentifier,
> privateKey PrivateKey,
> ...
>}
>
>PrivateKey ::= OCTET STRING
>
>... CurvePrivateKey object and wrapped by the OCTET STRING of the
>"privateKey" field.
>
>CurvePrivateKey ::= OCTET STRING
For what it's worth, my [PEM](https://github.com/openjdk/jdk/pull/17543)
changes to
[PKCS8Key.java](https://github.com/openjdk/jdk/pull/17543/files#diff-d4d775f071342d20e524e55883168e018a15f32e0d607518ef3d5f0f76dcdd29)
change `key` to `privKeyMaterial` because `key` doesn't refer to the Key
interface, but binary data
-
PR Review Comment: https://git.openjdk.org/jdk/pull/21167#discussion_r1792422384
Re: RFR: 8340327: A common framework to support public key algorithms with standard parameter sets [v10]
On Tue, 8 Oct 2024 19:11:45 GMT, Sean Mullan wrote:
>> Weijun Wang has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> null check as asserts, and better exception messages
>
> src/java.base/share/classes/sun/security/provider/NamedSignature.java line
> 121:
>
>> 119: return implSign(name, secKey, sk2, msg, appRandom);
>> 120: } else {
>> 121: throw new IllegalStateException("No private key");
>
> `Signature.engineSign` is not specified to throw `IllegalStateException`,
> even though it seems like the more correct exception to throw. So this should
> probably be a `SignatureException`.
>
> Same comment for `engineVerify`.
In fact, this should never happen since `engineVerify` should only be called
when there is a public key (`Signature::verify` checks if `state == VERIFY`).
But I'll use `SignatureException`.
> src/java.base/share/classes/sun/security/provider/NamedSignature.java line
> 196:
>
>> 194: /// This object will be passed into the [#implVerify] method along
>> with the raw key.
>> 195: ///
>> 196: /// The default implementation returns `null`.
>
> If the majority of implementations are supposed to implement this method,
> then it may be better to make it abstract (so that it won't be accidentally
> missed) and add a note that subclasses should return null if they don't have
> any additional checks.
I'll discuss my customers.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/21167#discussion_r1792455162
PR Review Comment: https://git.openjdk.org/jdk/pull/21167#discussion_r1792452057
Re: RFR: 8331959: Update PKCS#11 Cryptographic Token Interface to v3.1
On Mon, 7 Oct 2024 23:31:49 GMT, Weijun Wang wrote: >> Could someone please help review this PR? It updates the PKCS#11 headers and >> the relevant files to v3.1. >> >> Thanks! >> Valerie > > Looks good to me. Thanks. Thanks @wangweij @ascarpino for the review~Will integrate after management approves. - PR Comment: https://git.openjdk.org/jdk/pull/21396#issuecomment-2400917761
Re: RFR: 8298387: Implementing ML-DSA signature algorithm [v2]
> Java implementation of ML-DSA, the FIPS 204 post-quantum signature scheme > https://nvlpubs.nist.gov/nistpubs/FIPS/NIST.FIPS.204.pdf. Depends on > https://github.com/openjdk/jdk/pull/21167 Ben Perez has updated the pull request incrementally with one additional commit since the last revision: Updated hintBitUnpack to reflect FIPS 204. Moved key/sig decoding to sign/verify - Changes: - all: https://git.openjdk.org/jdk/pull/21364/files - new: https://git.openjdk.org/jdk/pull/21364/files/1b9c113e..5b449124 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=21364&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=21364&range=00-01 Stats: 124 lines in 2 files changed: 51 ins; 48 del; 25 mod Patch: https://git.openjdk.org/jdk/pull/21364.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/21364/head:pull/21364 PR: https://git.openjdk.org/jdk/pull/21364
Re: RFR: 8340327: A common framework to support public key algorithms with standard parameter sets [v12]
> To prepare for new PQC algorithms like ML-KEM and ML-DSA where there are only > named standardized parameter sets, a common framework is introduced. > > A example of EdDSA implementation using this framework is included as a test. Weijun Wang has updated the pull request incrementally with one additional commit since the last revision: same spec change in NamedKEM - Changes: - all: https://git.openjdk.org/jdk/pull/21167/files - new: https://git.openjdk.org/jdk/pull/21167/files/87216d7e..6c84d200 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=21167&range=11 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=21167&range=10-11 Stats: 4 lines in 1 file changed: 0 ins; 0 del; 4 mod Patch: https://git.openjdk.org/jdk/pull/21167.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/21167/head:pull/21167 PR: https://git.openjdk.org/jdk/pull/21167
Re: RFR: 8298387: Implementing ML-DSA signature algorithm
On Fri, 4 Oct 2024 20:59:45 GMT, Ben Perez wrote: > Java implementation of ML-DSA, the FIPS 204 post-quantum signature scheme > https://nvlpubs.nist.gov/nistpubs/FIPS/NIST.FIPS.204.pdf. Depends on > https://github.com/openjdk/jdk/pull/21167 src/java.base/share/classes/sun/security/provider/ML_DSA.java line 545: > 543: int[][] s1 = > Arrays.stream(sk.s1()).map(int[]::clone).toArray(int[][]::new); > 544: int[][] s2 = > Arrays.stream(sk.s2()).map(int[]::clone).toArray(int[][]::new); > 545: int[][] t0 = > Arrays.stream(sk.t0()).map(int[]::clone).toArray(int[][]::new); Instead of calling `mlDsa.skDecode(skBytes)` in `ML_DSA_Provider`, can we move the call here? Then `sk` becomes a local variable and you probably don't need to make the deep clones above. src/java.base/share/classes/sun/security/provider/ML_DSA.java line 563: > 561: hash.update(rnd); > 562: hash.update(mu); > 563: byte[] rhoPrime = hash.squeeze(mlDsaMaskSeedLength); The name is `rhoDoublePrime`. src/java.base/share/classes/sun/security/provider/ML_DSA.java line 660: > 658: //Check verify conditions > 659: boolean hashEq = Arrays.equals(sig.commitmentHash(), > cTildePrime); > 660: boolean weight = hammingWeight(sig.hint()) <= omega; This is no longer required in the final FIPS 204. On the other hand, `hintBitUnpack` is modified to add more checks. src/java.base/share/classes/sun/security/provider/ML_DSA.java line 665: > 663: > 664: /* > 665: Data conversion functions in Section 8.1 of specification It's Section 8 now. src/java.base/share/classes/sun/security/provider/ML_DSA_Provider.java line 109: > 107: } > 108: > 109: // TODO: check key in initSign and initVerify? Maybe you can at least check the size of the keys? src/java.base/share/classes/sun/security/provider/ML_DSA_Provider.java line 135: > 133: var mlDsa = new ML_DSA(size); > 134: var pk = mlDsa.pkDecode(pkBytes); > 135: var sig = mlDsa.sigDecode(sigBytes); Check the size of `sigBytes` and throw a `SignatureException` if invalid. - PR Review Comment: https://git.openjdk.org/jdk/pull/21364#discussion_r1791986347 PR Review Comment: https://git.openjdk.org/jdk/pull/21364#discussion_r1791999822 PR Review Comment: https://git.openjdk.org/jdk/pull/21364#discussion_r1792033938 PR Review Comment: https://git.openjdk.org/jdk/pull/21364#discussion_r1792048448 PR Review Comment: https://git.openjdk.org/jdk/pull/21364#discussion_r1791975633 PR Review Comment: https://git.openjdk.org/jdk/pull/21364#discussion_r1791978136
Re: RFR: 8298387: Implementing ML-DSA signature algorithm
On Fri, 4 Oct 2024 20:59:45 GMT, Ben Perez wrote: > Java implementation of ML-DSA, the FIPS 204 post-quantum signature scheme > https://nvlpubs.nist.gov/nistpubs/FIPS/NIST.FIPS.204.pdf. Depends on > https://github.com/openjdk/jdk/pull/21167 src/java.base/share/classes/sun/security/provider/ML_DSA.java line 659: > 657: > 658: //Check verify conditions > 659: boolean hashEq = Arrays.equals(sig.commitmentHash(), > cTildePrime); Do we want a constant-time array check here and use `MessageDigest.isEqual()` instead? - PR Review Comment: https://git.openjdk.org/jdk/pull/21364#discussion_r1792184664
Re: 8179502: Enhance OCSP, CRL and Certificate Fetch Timeouts.
Hi Hempushpa, I'm not actively involved in the OpenJDK 17u Project, so I'm not the best person to review this. You might want to ask one of the JDK 17u Project Maintainers [1] who would be a suitable person to review this backport. Thanks, Sean [1] https://wiki.openjdk.org/display/JDKUpdates/JDK+17u On 10/8/24 2:49 AM, Hempushpa Sahu wrote: Hi Sean, There is an update from @Jamil Nimeh on the PR (https://github.com/openjdk/jdk17u-dev/pull/2747), and he doesn't have reviewer rights. So, could you please help us here to review the PR. We are really looking forward to getting this backport done, as our customer is waiting for the fix. Thanks, Hempushpa Sahu -Original Message- From: Sean Mullan Sent: Thursday, September 19, 2024 6:05 PM To: Hempushpa Sahu ; security-dev@openjdk.org Cc: Syed Moinudeen1 Subject: [EXTERNAL] Re: 8179502: Enhance OCSP, CRL and Certificate Fetch Timeouts. Hi, This list is primarily for discussing and reviewing security related topics (bugs, enhancements, etc) for the JDK mainline (https://github.com/openjdk/jdk/ ). Please use the jdk-updates-dev list for issues that relate to previous JDK releases. I see that the assignee has already asked for a review a couple of days ago, so hopefully this will move forward soon. --Sean On 9/19/24 3:23 AM, Hempushpa Sahu wrote: Hi Team, We are working on a case, and the customer is awaiting the fix. To deliver the fix, this PR INVALID URI REMOVED k_jdk17u-2Ddev_pull_2747&d=DwIDaQ&c=BSDicqBQBDjDI9RkVyTcHQ&r=E-NNzCMrZ leBploUPg81ekUNFNM-G7U74D9ne2u5_vQ&m=Oe3i8D9aSd1OQ4nDnX83NxHGWSbJunmxq b0NJdJ4TbgNlpS-vJGSFQ1L1720BMQD&s=-aY2XSsvktwv-osX2Kd2GcuD2XpQXRupS3Yl tTSqNuY&e= needs to be merged. We requested an update on the review progress on August 20, 2024, but no progress has been made since then. It has been over a month, and we are still waiting for the PR to be reviewed and merged. Can anyone please assist us in moving forward with the review process by letting us know who can review the code? Thanks.
Re: RFR: 8331682: Slow networks/Impatient clients can potentially send unencrypted TLSv1.3 alerts that won't parse on the server [v17]
> Check for unexpected plaintext alert message during TLSv1.3 handshake. This > can happen if client doesn't receive ServerHello due to network timeout and > tries to close the connection by sending an alert message. Artur Barashev has updated the pull request incrementally with one additional commit since the last revision: Return null if there is no record we attempted to decode - Changes: - all: https://git.openjdk.org/jdk/pull/21043/files - new: https://git.openjdk.org/jdk/pull/21043/files/b6f79f36..f2f463fd Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=21043&range=16 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=21043&range=15-16 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/21043.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/21043/head:pull/21043 PR: https://git.openjdk.org/jdk/pull/21043
Re: RFR: 8298387: Implementing ML-DSA signature algorithm
On Fri, 4 Oct 2024 20:59:45 GMT, Ben Perez wrote:
> Java implementation of ML-DSA, the FIPS 204 post-quantum signature scheme
> https://nvlpubs.nist.gov/nistpubs/FIPS/NIST.FIPS.204.pdf. Depends on
> https://github.com/openjdk/jdk/pull/21167
src/java.base/share/classes/sun/security/provider/ML_DSA.java line 45:
> 43: private static final int montRModQ = 4193792;
> 44: private static final int montDimInverse = 16382; //
> toMont((mlDsa_n)^-1 (mod mlDsa_q))
> 45: private static final int[] montZetasForNtt = new int[]{
It would be nice to link to a source for these magic values in the comments,
unless we are generating some of these values ourselves.
src/java.base/share/classes/sun/security/provider/ML_DSA.java line 418:
> 416:
> 417: public ML_DSA(int security_level) {
> 418: switch (security_level) {
Might want a comment about why there are only `security_level` values of 2, 3,
& 5.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/21364#discussion_r1792109302
PR Review Comment: https://git.openjdk.org/jdk/pull/21364#discussion_r1792111251
Re: RFR: 8340133: Investigate if the java launcher could give hints about JShell
On Tue, 8 Oct 2024 15:28:17 GMT, Jan Lahoda wrote: > Currently, running `java` without any parameters will lead to an output that > is a full `--help`, which is over 100 lines (on my computer at least), and it > feels overwhelming. And many people might actually want to run JShell/REPL, > not the `java` launcher, but it is difficult find out about JShell. > > The proposal herein is to print a much shorter help, together with a pointer > to JShell, when the launcher does not know what to do. I.e. there is nothing > specified to start, and no option like `--help` is specified. In particular, > on my machine, it prints: > > $ java > openjdk 24-internal 2025-03-18 > > Usage: java [options...] [arguments to main method...] > > Where is one of: > to execute the main method of a compiled class > -jar to execute the main class in a JAR archive > -m [/] to execute the main class of a module > to compile and execute a single-file program > > Where key options include: > --class-path > a : separated list of directories and JAR archives to search for class > files > --module-path > a : separated list of directories and JAR archives to search for modules > > For more details about this launcher: java --help > For an interactive Java environment:jshell > > > Hopefully, this may be easier both for people trying to run something, and > for people that are really looking for JShell. > > What do you think? > > Thanks! Yay! This looks so much better than the default blob. I think you are on the right track, but the actual message can do with some fine tuning. For instance, `` seems a bit ... untypically non-formal. What about just ``? - PR Comment: https://git.openjdk.org/jdk/pull/21411#issuecomment-2400192734
Re: RFR: 8298387: Implementing ML-DSA signature algorithm
On Fri, 4 Oct 2024 20:59:45 GMT, Ben Perez wrote: > Java implementation of ML-DSA, the FIPS 204 post-quantum signature scheme > https://nvlpubs.nist.gov/nistpubs/FIPS/NIST.FIPS.204.pdf. Depends on > https://github.com/openjdk/jdk/pull/21167 src/java.base/share/classes/sun/security/provider/ML_DSA.java line 437: > 435: s2PackedLength = 384; > 436: s1s2CoeffSize = 3; > 437: wCoeffSize = 6; Would it make sense to create an object or `record` to group all these values together? - PR Review Comment: https://git.openjdk.org/jdk/pull/21364#discussion_r1792116257
Re: RFR: 8298387: Implementing ML-DSA signature algorithm
On Tue, 8 Oct 2024 15:38:18 GMT, Kevin Driver wrote:
>> Java implementation of ML-DSA, the FIPS 204 post-quantum signature scheme
>> https://nvlpubs.nist.gov/nistpubs/FIPS/NIST.FIPS.204.pdf. Depends on
>> https://github.com/openjdk/jdk/pull/21167
>
> src/java.base/share/classes/sun/security/provider/ML_DSA.java line 418:
>
>> 416:
>> 417: public ML_DSA(int security_level) {
>> 418: switch (security_level) {
>
> Might want a comment about why there are only `security_level` values of 2,
> 3, & 5.
I See elsewhere this corresponds to the three "strength" values for the
algorithm. Perhaps then, some constants such as `static final int ML-DSA-44 =
2;`. Just a suggestion.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/21364#discussion_r1792142142
Re: RFR: 8335288: SunPKCS11 initialization will call C_GetMechanismInfo on unsupported mechanisms [v2]
On Mon, 7 Oct 2024 06:31:13 GMT, Kåre Fiedler Christiansen wrote: > Hi. I'm the original reporter of this issue. Thanks for the work done on this > bug. I see the the fix has been targeted at only Java 24. Are there any plans > to back port this to Java 21, which is the current LTS version, and where the > regression was introduced? I have just opened a request to backport this to JDK 21 update. Someone from the sustaining team should handle the backport request. Thank~ - PR Comment: https://git.openjdk.org/jdk/pull/20207#issuecomment-2400934375
Re: RFR: 8340327: A common framework to support public key algorithms with standard parameter sets [v10]
On Mon, 7 Oct 2024 18:54:03 GMT, Weijun Wang wrote:
>> To prepare for new PQC algorithms like ML-KEM and ML-DSA where there are
>> only named standardized parameter sets, a common framework is introduced.
>>
>> A example of EdDSA implementation using this framework is included as a test.
>
> Weijun Wang has updated the pull request incrementally with one additional
> commit since the last revision:
>
> null check as asserts, and better exception messages
src/java.base/share/classes/sun/security/provider/NamedSignature.java line 63:
> 61:
> 62: private Object sk2 = null;
> 63: private Object pk2 = null;
Nit - don't need to set these to null.
src/java.base/share/classes/sun/security/provider/NamedSignature.java line 121:
> 119: return implSign(name, secKey, sk2, msg, appRandom);
> 120: } else {
> 121: throw new IllegalStateException("No private key");
`Signature.engineSign` is not specified to throw `IllegalStateException`, even
though it seems like the more correct exception to throw. So this should
probably be a `SignatureException`.
Same comment for `engineVerify`.
src/java.base/share/classes/sun/security/provider/NamedSignature.java line 146:
> 144: @SuppressWarnings("deprecation")
> 145: protected Object engineGetParameter(String param) throws
> InvalidParameterException {
> 146: throw new UnsupportedOperationException("getParameter() not
> supported");
`engineGetParameter` is not specified to throw UOE, so suggest throwing
`InvalidParameterException` instead. Same comment for `engineSetParameter`.
src/java.base/share/classes/sun/security/provider/NamedSignature.java line 196:
> 194: /// This object will be passed into the [#implVerify] method along
> with the raw key.
> 195: ///
> 196: /// The default implementation returns `null`.
If the majority of implementations are supposed to implement this method, then
it may be better to make it abstract (so that it won't be accidentally missed)
and add a note that subclasses should return null if they don't have any
additional checks.
src/java.base/share/classes/sun/security/provider/NamedSignature.java line 208:
> 206: /// User-defined function to validate a private key.
> 207: ///
> 208: /// This method will be called in `initSign`. This gives provider a
> chance to
s/provider/the provider/
src/java.base/share/classes/sun/security/provider/NamedSignature.java line 210:
> 208: /// This method will be called in `initSign`. This gives provider a
> chance to
> 209: /// reject the key so an `InvalidKeyException` can be thrown earlier.
> 210: /// An implementation can optional return a "parsed key" as an
> `Object` value.
s/optional/optionally/
src/java.base/share/classes/sun/security/provider/NamedSignature.java line 213:
> 211: /// This object will be passed into the [#implSign] method along
> with the raw key.
> 212: ///
> 213: /// The default implementation returns `null`.
Same comment as for implCheckPublicKey.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/21167#discussion_r1792352928
PR Review Comment: https://git.openjdk.org/jdk/pull/21167#discussion_r1792381631
PR Review Comment: https://git.openjdk.org/jdk/pull/21167#discussion_r1792386559
PR Review Comment: https://git.openjdk.org/jdk/pull/21167#discussion_r1792370232
PR Review Comment: https://git.openjdk.org/jdk/pull/21167#discussion_r1792354924
PR Review Comment: https://git.openjdk.org/jdk/pull/21167#discussion_r1792355642
PR Review Comment: https://git.openjdk.org/jdk/pull/21167#discussion_r1792369393
Re: RFR: 8335288: SunPKCS11 initialization will call C_GetMechanismInfo on unsupported mechanisms [v2]
On Tue, 8 Oct 2024 22:33:40 GMT, Valerie Peng wrote: > > Hi. I'm the original reporter of this issue. Thanks for the work done on > > this bug. I see the the fix has been targeted at only Java 24. Are there > > any plans to back port this to Java 21, which is the current LTS version, > > and where the regression was introduced? > > I have just opened a request to backport this to JDK 21 update. Someone from > the sustaining team should handle the backport request. Thank~ Correction: It'll be the OpenJDK21 maintainers to handle the backport request (not the sustaining team). - PR Comment: https://git.openjdk.org/jdk/pull/20207#issuecomment-2400971765
Re: RFR: 8340327: A common framework to support public key algorithms with standard parameter sets [v10]
On Tue, 8 Oct 2024 19:16:10 GMT, Sean Mullan wrote:
>> Weijun Wang has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> null check as asserts, and better exception messages
>
> src/java.base/share/classes/sun/security/provider/NamedSignature.java line
> 146:
>
>> 144: @SuppressWarnings("deprecation")
>> 145: protected Object engineGetParameter(String param) throws
>> InvalidParameterException {
>> 146: throw new UnsupportedOperationException("getParameter() not
>> supported");
>
> `engineGetParameter` is not specified to throw UOE, so suggest throwing
> `InvalidParameterException` instead. Same comment for `engineSetParameter`.
Yes I will. I noticed that the newest implementation `HSS` does throw
`InvalidParameterException`, and a little older one `ECDSASignature` and
`EdDSASignature` were throwing `UnsupportedOperationException`.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/21167#discussion_r1792463680
Re: RFR: 8340327: A common framework to support public key algorithms with standard parameter sets [v11]
> To prepare for new PQC algorithms like ML-KEM and ML-DSA where there are only > named standardized parameter sets, a common framework is introduced. > > A example of EdDSA implementation using this framework is included as a test. Weijun Wang has updated the pull request incrementally with one additional commit since the last revision: resolve Sean's comments; some small utility method updates - Changes: - all: https://git.openjdk.org/jdk/pull/21167/files - new: https://git.openjdk.org/jdk/pull/21167/files/986db839..87216d7e Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=21167&range=10 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=21167&range=09-10 Stats: 24 lines in 3 files changed: 4 ins; 2 del; 18 mod Patch: https://git.openjdk.org/jdk/pull/21167.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/21167/head:pull/21167 PR: https://git.openjdk.org/jdk/pull/21167
Re: RFR: 8340327: A common framework to support public key algorithms with standard parameter sets [v10]
On Tue, 8 Oct 2024 19:49:53 GMT, Anthony Scarpino wrote:
>> Yes, `this.key` is the one inside `PKCS8Key`.
>>
>> Since EdDSA and XDH, the private key has taken this OCTET in OCTET approach.
>> My code is identical to the EdDSA code at
>> https://github.com/openjdk/jdk/blob/adca97b659d725b0dd320322297dcbd1b443a047/src/java.base/share/classes/sun/security/ec/ed/EdDSAPrivateKeyImpl.java#L50-L64.
>>
>> In https://datatracker.ietf.org/doc/html/rfc8410#autoid-7 and
>> https://www.ietf.org/archive/id/draft-ietf-lamps-kyber-certificates-04.html#name-private-key-format,
>> you can see the definitions:
>>
>>
>>OneAsymmetricKey ::= SEQUENCE {
>> version Version,
>> privateKeyAlgorithm PrivateKeyAlgorithmIdentifier,
>> privateKey PrivateKey,
>> ...
>>}
>>
>>PrivateKey ::= OCTET STRING
>>
>>... CurvePrivateKey object and wrapped by the OCTET STRING of the
>>"privateKey" field.
>>
>>CurvePrivateKey ::= OCTET STRING
>
> For what it's worth, my [PEM](https://github.com/openjdk/jdk/pull/17543)
> changes to
> [PKCS8Key.java](https://github.com/openjdk/jdk/pull/17543/files#diff-d4d775f071342d20e524e55883168e018a15f32e0d607518ef3d5f0f76dcdd29)
> change `key` to `privKeyMaterial` because `key` doesn't refer to the Key
> interface, but binary data
@ascarpino Yes I noticed it when I was trying to apply your patch onto mine to
see if I can use PEM to encode and decode my new keys. Let's see who gets
integrated first. :-)
-
PR Review Comment: https://git.openjdk.org/jdk/pull/21167#discussion_r1792482081
Re: RFR: 8298387: Implementing ML-DSA signature algorithm [v3]
> Java implementation of ML-DSA, the FIPS 204 post-quantum signature scheme > https://nvlpubs.nist.gov/nistpubs/FIPS/NIST.FIPS.204.pdf. Depends on > https://github.com/openjdk/jdk/pull/21167 Ben Perez has updated the pull request incrementally with one additional commit since the last revision: renamed internal keyGen/sign/verify functions to be same as spec - Changes: - all: https://git.openjdk.org/jdk/pull/21364/files - new: https://git.openjdk.org/jdk/pull/21364/files/5b449124..127accad Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=21364&range=02 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=21364&range=01-02 Stats: 7 lines in 2 files changed: 1 ins; 0 del; 6 mod Patch: https://git.openjdk.org/jdk/pull/21364.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/21364/head:pull/21364 PR: https://git.openjdk.org/jdk/pull/21364
Re: RFR: 8336665: CCE in X509CRLImpl$TBSCertList.getCertIssuer [v4]
On Tue, 24 Sep 2024 12:33:54 GMT, Sean Mullan wrote:
>> Mark Powers has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> another comment from Sean
>
> src/java.base/share/classes/sun/security/x509/X509CRLImpl.java line 292:
>
>> 290: throw new CRLException("Parsing error: "
>> 291: + "issuer is not an X.500 DN");
>> 292: }
>
> I checked RFC 5280 and you can have more than one name in the
> `CertificateIssuer` field of the `CertificateIssuerExtension`, see
> https://www.rfc-editor.org/rfc/rfc5280#section-5.3.3
>
> But for this code, we are only interested in the `X500Name`, as we
> subsequently use that to associate the CRL entry with its issuer. So instead,
> what you should do is loop thru the names until we find an `X500Name`, and
> only throw a `CRLException` if we don't find an `X500Name`. Let me know if
> this makes sense.
Fixed.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/20528#discussion_r1792096204
Re: RFR: 8336665: CCE in X509CRLImpl$TBSCertList.getCertIssuer [v5]
> https://bugs.openjdk.org/browse/JDK-8336665 Mark Powers has updated the pull request incrementally with one additional commit since the last revision: allow more than one name - Changes: - all: https://git.openjdk.org/jdk/pull/20528/files - new: https://git.openjdk.org/jdk/pull/20528/files/6907ae46..336f81a7 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=20528&range=04 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=20528&range=03-04 Stats: 7 lines in 1 file changed: 3 ins; 0 del; 4 mod Patch: https://git.openjdk.org/jdk/pull/20528.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/20528/head:pull/20528 PR: https://git.openjdk.org/jdk/pull/20528
RFR: 8340133: Investigate if the java launcher could give hints about JShell
Currently, running `java` without any parameters will lead to an output that is a full `--help`, which is over 100 lines (on my computer at least), and it feels overwhelming. And many people might actually want to run JShell/REPL, not the `java` launcher, but it is difficult find out about JShell. The proposal herein is to print a much shorter help, together with a pointer to JShell, when the launcher does not know what to do. I.e. there is nothing specified to start, and no option like `--help` is specified. In particular, on my machine, it prints: $ java openjdk 24-internal 2025-03-18 Usage: java [options...] [arguments to main method...] Where is one of: to execute the main method of a compiled class -jar to execute the main class in a JAR archive -m [/] to execute the main class of a module to compile and execute a single-file program Where key options include: --class-path a : separated list of directories and JAR archives to search for class files --module-path a : separated list of directories and JAR archives to search for modules For more details about this launcher: java --help For an interactive Java environment:jshell Hopefully, this may be easier both for people trying to run something, and for people that are really looking for JShell. What do you think? Thanks! - Commit messages: - Cleanup. - Adjusting/improving the concise help. - Attempting to make the java launcher message consice, and let it refer to JShell. Changes: https://git.openjdk.org/jdk/pull/21411/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=21411&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8340133 Stats: 54 lines in 6 files changed: 49 ins; 0 del; 5 mod Patch: https://git.openjdk.org/jdk/pull/21411.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/21411/head:pull/21411 PR: https://git.openjdk.org/jdk/pull/21411
Re: RFR: 8336665: CCE in X509CRLImpl$TBSCertList.getCertIssuer [v4]
On Tue, 8 Oct 2024 15:28:58 GMT, Mark Powers wrote:
>> src/java.base/share/classes/sun/security/x509/X509CRLImpl.java line 292:
>>
>>> 290: throw new CRLException("Parsing error: "
>>> 291: + "issuer is not an X.500 DN");
>>> 292: }
>>
>> I checked RFC 5280 and you can have more than one name in the
>> `CertificateIssuer` field of the `CertificateIssuerExtension`, see
>> https://www.rfc-editor.org/rfc/rfc5280#section-5.3.3
>>
>> But for this code, we are only interested in the `X500Name`, as we
>> subsequently use that to associate the CRL entry with its issuer. So
>> instead, what you should do is loop thru the names until we find an
>> `X500Name`, and only throw a `CRLException` if we don't find an `X500Name`.
>> Let me know if this makes sense.
>
> Fixed.
Does the test need to be modified to test for more than one name? I could go
either way.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/20528#discussion_r1792102768
