On Sun, 11 May 2025 19:02:55 GMT, Anthony Scarpino <ascarp...@openjdk.org> wrote:
>> Hi all, >> >> I need a code review of the PEM API. Privacy-Enhanced Mail (PEM) is a >> format for encoding and decoding cryptographic keys and certificates. It >> will be integrated into JDK24 as a Preview Feature. Preview features does >> not permanently define the API and it is subject to change in future >> releases until it is finalized. >> >> Details about this change can be seen at [PEM API >> JEP](https://bugs.openjdk.org/browse/JDK-8300911). >> >> Thanks >> >> Tony > > Anthony Scarpino has updated the pull request incrementally with one > additional commit since the last revision: > > comments on the 11th src/java.base/share/classes/sun/security/rsa/RSAPrivateCrtKeyImpl.java line 72: > 70: private BigInteger coeff; // CRT coefficient > 71: > 72: // RSA or RSS-PSS KeyType Typo: s/RSS-PSS/RSA-PSS/ src/java.base/share/classes/sun/security/x509/X509Key.java line 147: > 145: * X509Key. Useful for PKCS8v2. > 146: */ > 147: public static X509Key parse(byte[] encoded) throws IOException Isn't this the same as `X509Key.parse(byte[])`? src/java.base/share/classes/sun/security/x509/X509Key.java line 150: > 148: { > 149: DerValue in = new DerValue(encoded); > 150: AlgorithmId algorithm; Can move to line 155. src/java.base/share/conf/security/java.security line 1560: > 1558: # withEncryption method. > 1559: # > 1560: jdk.epkcs8.defaultAlgorithm=PBEWithHmacSHA256AndAES_128 What about naming this `jdk.pem.pbeAlgorithm`? The `withEncryption` method never mentions PKCS#8, so maybe don't need to expose this in the property name. Also having "pem" in the property name makes it more clear what API this property is used for. test/jdk/java/security/KeyFactory/KeyFactoryGetKeySpecForInvalidSpec.java line 3: > 1: /* > 2: * Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. > 3: * Copyright (c) 2025, Oracle and/or its affiliates. All rights reserved. Not necessary anymore since file is not changed. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/17543#discussion_r2085063839 PR Review Comment: https://git.openjdk.org/jdk/pull/17543#discussion_r2085071940 PR Review Comment: https://git.openjdk.org/jdk/pull/17543#discussion_r2085068341 PR Review Comment: https://git.openjdk.org/jdk/pull/17543#discussion_r2084832539 PR Review Comment: https://git.openjdk.org/jdk/pull/17543#discussion_r2084756079