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

Koushik Muthukrishnan Thirupattur duke at openjdk.org
Tue Oct 14 03:59:08 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/DEREncodable.java line 58:

> 56: public sealed interface DEREncodable permits AsymmetricKey, KeyPair,
> 57:     PKCS8EncodedKeySpec, X509EncodedKeySpec, EncryptedPrivateKeyInfo,
> 58:     X509Certificate, X509CRL, PEM {

Do we need to add any javadoc note here stating why PEM replaces PEMRecord to help understand intent?

src/java.base/share/classes/java/security/PEM.java line 68:

> 66:  * metadata that accompanies the PEM data. This value was not defensively
> 67:  * copied by the constructor, and the {@link #leadingData()} method does not
> 68:  * return a clone.

I see the Javadoc already notes that leadingData isn’t defensively copied or cloned. Given that PEM is a record (and callers might assume immutability), should we clarify ownership semantics a bit more — e.g., that the caller must not modify the array after passing it in?

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

> 230:                     }
> 231:                     byte[] p8 = Pem.decryptEncoding(
> 232:                         decoder.decode(pem.content()), password.getPassword());

Should we need a null check for pem and a short defensive check for pem.content() being empty/blank before attempting decode? Also do we explicitly specify why explicit zeroing is used ?

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

> 231:                     byte[] p8 = Pem.decryptEncoding(
> 232:                         decoder.decode(pem.content()), password.getPassword());
> 233:                     DEREncodable d = Pem.toDEREncodable(p8, true, factory);

If toDEREncodable may throw, do we need to guarantee zeroing on all paths in case of success or exception?

src/java.base/share/classes/java/security/PEMEncoder.java line 286:

> 284:      * privateKeyEncoding will be zeroed when the method returns
> 285:      */
> 286:     private String buildKey(byte[] publicEncoding, byte[] privateEncoding) {

Do we need to explicitly document argument contracts: which combination corresponds to which PEM types here ?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/27147#discussion_r2427844413
PR Review Comment: https://git.openjdk.org/jdk/pull/27147#discussion_r2427845013
PR Review Comment: https://git.openjdk.org/jdk/pull/27147#discussion_r2427845632
PR Review Comment: https://git.openjdk.org/jdk/pull/27147#discussion_r2427845911
PR Review Comment: https://git.openjdk.org/jdk/pull/27147#discussion_r2427846490


More information about the security-dev mailing list