RFR: 8360564: Implement JEP 524: PEM Encodings of Cryptographic Objects (Second Preview) [v4]
Weijun Wang
weijun at openjdk.org
Wed Oct 1 21:20:26 UTC 2025
On Thu, 25 Sep 2025 23:03:11 GMT, Anthony Scarpino <ascarpino at openjdk.org> wrote:
>> Hi
>>
>> Please review the [Second Preview](https://openjdk.org/jeps/8360563) for the PEM API. The most significant changes from [JEP 470](https://openjdk.org/jeps/470) are:
>>
>> - Renamed the name of `PEMRecord` class to `PEM`.
>> - Revised the new `encryptKey` methods of the `EncryptedPrivateKeyInfo` class to accept `DEREncodable` objects rather than just `PrivateKey` objects so that cryptographic objects with public keys, i.e., `KeyPair` and `PKCS8EncodedKeySpec`, can also be encrypted.
>> - Enhanced the `PEMEncoder` and `PEMDecoder` classes to support the encryption and decryption of `KeyPair` and `PKCS8EncodedKeySpec` objects.
>>
>> thanks
>>
>> Tony
>
> Anthony Scarpino has updated the pull request incrementally with one additional commit since the last revision:
>
> missed some decoder comments
Comments on the APIs.
src/java.base/share/classes/java/security/PEM.java line 98:
> 96: * formatted.
> 97: * @throws NullPointerException if {@code type} and/or {@code content} are
> 98: * {@code null}.
No need to add a period at the end if it's not a full sentence. There are several places in this file.
src/java.base/share/classes/java/security/PEM.java line 117:
> 115: * {@code content} data in String form. {@code leadingData} is set to null.
> 116: *
> 117: * @param type the PEM type identifier
In the other constructor, the words are "the type identifier". Please be consistent.
src/java.base/share/classes/java/security/PEMDecoder.java line 76:
> 74: * decryption)</li>
> 75: * <li>ENCRYPTED PRIVATE KEY : {@code PKCS8EncodedKeySpec} (if configured with
> 76: * decryption)</li>
Plus "and passed as a Class parameter".
src/java.base/share/classes/java/security/PEMDecoder.java line 119:
> 117: * {@snippet lang = java:
> 118: * PEMDecoder pd = PEMDecoder.of();
> 119: * PrivateKey priKey = pd.decode(priKeyPEM, PrivateKey.class);
In the 2nd example below, indent `withFactory(provider);` with 8 spaces.
src/java.base/share/classes/java/security/PEMDecoder.java line 333:
> 331: *
> 332: * <p> This method reads the {@code String} until PEM data is found
> 333: * or the end of the {@code String} is reached. If no PEM data is found,
Three lines above. Do we need to say "tClass must extend DEREncodable"? The `<S extends DEREncodable>` style means it has to be.
src/java.base/share/classes/java/security/PEMDecoder.java line 374:
> 372: * the PEM footer or the end of the stream. If an I/O error occurs,
> 373: * the read position in the stream may become inconsistent.
> 374: * It is recommended to perform no further decoding operations
Three lines above. Is it better to say "until the end of the first PEM footer"? Same with the other decode-from-stream method.
src/java.base/share/classes/java/security/PEMEncoder.java line 43:
> 41: import java.util.Objects;
> 42: import java.util.concurrent.locks.ReentrantLock;
> 43:
In the 2nd line below, you used "store and transfer security objects". Let's all use "cryptographic objects". Same in `PEMDecoder`.
src/java.base/share/classes/java/security/PEMEncoder.java line 61:
> 59: * which takes a password and returns a new {@code PEMEncoder} instance
> 60: * configured to encrypt the key with that password. Alternatively, a
> 61: * private key encrypted as an {@code EncryptedKeyInfo} object can be encoded
Typo, it's `EncryptedPrivateKeyInfo`. Maybe add a link.
src/java.base/share/classes/java/security/PEMEncoder.java line 68:
> 66: * contain both private and public keys.
> 67: * {@link KeyPair} objects passed to the {@code encode} or
> 68: * {@code encodeToString} methods are encoded as a
Three lines above. Should "OneAsymmetricKey" be fixed-width `OneAsymmetricKey`?
src/java.base/share/classes/java/security/PEMEncoder.java line 69:
> 67: * {@link KeyPair} objects passed to the {@code encode} or
> 68: * {@code encodeToString} methods are encoded as a
> 69: * OneAsymmetricKey structure using the "PRIVATE KEY" type.
Should "OneAsymmetricKey" be fixed-width `OneAsymmetricKey`? Also, 4 lines above here.
src/java.base/share/classes/java/security/PEMEncoder.java line 95:
> 93: * <li>{@code PKCS8EncodedKeySpec} (if configured with encryption) :
> 94: * ENCRYPTED PRIVATE KEY</li>
> 95: * <li>{@code PEM} : {@code PEM.type()}</li>
`X509EncodedKeySpec` missing.
src/java.base/share/classes/javax/crypto/EncryptedPrivateKeyInfo.java line 330:
> 328:
> 329: /**
> 330: * Creates and encrypts an {@code EncryptedPrivateKeyInfo} from a given
I know the line above is not new, but it sounds strange to me. You cannot encrypt something that is already named "encrypted".
src/java.base/share/classes/javax/crypto/EncryptedPrivateKeyInfo.java line 368:
> 366: */
> 367: @PreviewFeature(feature = PreviewFeature.Feature.PEM_API)
> 368: public static EncryptedPrivateKeyInfo encryptKey(DEREncodable de,
Shall we name it `encryptKey` or simply `encrypt`? I'm asking because it can be something other than a key. The decrypt side has `getKey`, `getKeySpec`, and `getKayPair`. Since we have only one on the encrypt side, it needn't use the noun of one of them.
src/java.base/share/classes/javax/crypto/EncryptedPrivateKeyInfo.java line 459:
> 457: public static EncryptedPrivateKeyInfo encryptKey(DEREncodable de,
> 458: Key encryptKey, String algorithm, AlgorithmParameterSpec params,
> 459: Provider provider, SecureRandom random) {
Technically, must `encryptKey` be a PBE key? Can it be any key? I notice that PBE is not mentioned in `getKey`.
-------------
PR Review: https://git.openjdk.org/jdk/pull/27147#pullrequestreview-3290505282
PR Review Comment: https://git.openjdk.org/jdk/pull/27147#discussion_r2395706082
PR Review Comment: https://git.openjdk.org/jdk/pull/27147#discussion_r2395700263
PR Review Comment: https://git.openjdk.org/jdk/pull/27147#discussion_r2395742704
PR Review Comment: https://git.openjdk.org/jdk/pull/27147#discussion_r2395761939
PR Review Comment: https://git.openjdk.org/jdk/pull/27147#discussion_r2395797969
PR Review Comment: https://git.openjdk.org/jdk/pull/27147#discussion_r2395782621
PR Review Comment: https://git.openjdk.org/jdk/pull/27147#discussion_r2395714807
PR Review Comment: https://git.openjdk.org/jdk/pull/27147#discussion_r2395720590
PR Review Comment: https://git.openjdk.org/jdk/pull/27147#discussion_r2395722689
PR Review Comment: https://git.openjdk.org/jdk/pull/27147#discussion_r2395724602
PR Review Comment: https://git.openjdk.org/jdk/pull/27147#discussion_r2395729138
PR Review Comment: https://git.openjdk.org/jdk/pull/27147#discussion_r2395505870
PR Review Comment: https://git.openjdk.org/jdk/pull/27147#discussion_r2395494777
PR Review Comment: https://git.openjdk.org/jdk/pull/27147#discussion_r2395575014
More information about the security-dev
mailing list