RFR: 8360564: Implement JEP 524: PEM Encodings of Cryptographic Objects (Second Preview) [v6]

Sean Mullan mullan at openjdk.org
Wed Oct 15 14:04:18 UTC 2025


On Mon, 13 Oct 2025 17:22:25 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:
> 
>   updates

src/java.base/share/classes/java/security/PEMDecoder.java line 47:

> 45: /**
> 46:  * {@code PEMDecoder} implements a decoder for Privacy-Enhanced Mail (PEM) data.
> 47:  * PEM is a textual encoding used to store and transfer security

In `PEMEncoder` it says "cryptographic objects", here it says "security objects". Should be consistent.

src/java.base/share/classes/java/security/PEMDecoder.java line 63:

> 61:  *  <li>X509 CERTIFICATE : {@code X509Certificate}</li>
> 62:  *  <li>X.509 CERTIFICATE : {@code X509Certificate}</li>
> 63:  *  <li>CRL : {@code X509CRL}</li>

X509 CERTIFICATE, X.509 CERTIFICATE, and CRL are not standard types as defined in RFC 7468. Putting them in this list means all implementations must support them, which I don't think is appropriate. I think these types should be moved to the Implementation Note.

src/java.base/share/classes/java/security/PEMDecoder.java line 66:

> 64:  *  <li>X509 CRL : {@code X509CRL}</li>
> 65:  *  <li>PUBLIC KEY : {@code PublicKey}</li>
> 66:  *  <li>PUBLIC KEY : {@code X509EncodedKeySpec} (When passed as a {@code Class}

"When" should be lower-case. Same comment for following bullets that contain parentheses.

src/java.base/share/classes/java/security/PEMDecoder.java line 114:

> 112:  * The {@link #withFactory(Provider)} method uses the specified provider
> 113:  * to produce cryptographic objects from {@link KeyFactory} and
> 114:  * {@link CertificateFactory}. The {@link #withDecryption(char[])} configures the

The {@link #withDecryption(char[])} method ...

src/java.base/share/classes/java/security/PEMDecoder.java line 119:

> 117:  * If an encrypted private key PEM is processed by a decoder not configured
> 118:  * for decryption, an {@link EncryptedPrivateKeyInfo} object is returned.
> 119:  * Decryption configured instances will decode unencrypted PEM.

Reword this sentence so it is easier to understand, ex: "A PEMDecoder configured for decryption .."

src/java.base/share/classes/java/security/PEMDecoder.java line 136:

> 134:  * }
> 135:  *
> 136:  * @implNote This implementation decodes PEM type {@code RSA PRIVATE KEY} as

s/PEM/the PEM/

src/java.base/share/classes/java/security/PEMDecoder.java line 343:

> 341:      * leading data preceding the PEM header. For {@code DEREncodable} types
> 342:      * other than {@code PEM}, leading data is ignored and not returned as part
> 343:      * of the DEREncodable object.

put code font around DEREncodable.

src/java.base/share/classes/java/security/PEMDecoder.java line 387:

> 385:      *
> 386:      * <p> If no PEM data is found, an {@code IllegalArgumentException} is
> 387:      * thrown.

Below it says an `EOFException` is thrown if no PEM data is found. Same comment for method that takes a Class param.

src/java.base/share/classes/java/security/PEMDecoder.java line 399:

> 397:      *   end of the {@code InputStream}
> 398:      * @throws IllegalArgumentException on error in decoding
> 399:      * @throws ClassCastException if {@code tClass} does not represent the PEM type

It's a little odd this throws a `ClassCastException`. This seems more like an `IllegalArgumentException` to me because you are passing in the wrong type.

src/java.base/share/classes/java/security/PEMDecoder.java line 499:

> 497:      *
> 498:      * @param provider the factory provider
> 499:      * @return a new PEMEncoder instance configured with the {@code Provider}.

s/PEMEncoder/PEMDecoder/

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/27147#discussion_r2432340162
PR Review Comment: https://git.openjdk.org/jdk/pull/27147#discussion_r2432655321
PR Review Comment: https://git.openjdk.org/jdk/pull/27147#discussion_r2432346397
PR Review Comment: https://git.openjdk.org/jdk/pull/27147#discussion_r2432360574
PR Review Comment: https://git.openjdk.org/jdk/pull/27147#discussion_r2432366357
PR Review Comment: https://git.openjdk.org/jdk/pull/27147#discussion_r2432369853
PR Review Comment: https://git.openjdk.org/jdk/pull/27147#discussion_r2432399291
PR Review Comment: https://git.openjdk.org/jdk/pull/27147#discussion_r2432427373
PR Review Comment: https://git.openjdk.org/jdk/pull/27147#discussion_r2432459995
PR Review Comment: https://git.openjdk.org/jdk/pull/27147#discussion_r2432466783


More information about the security-dev mailing list