On Wed, 17 Sep 2025 01:00:21 GMT, Anthony Scarpino <[email protected]> 
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:
> 
>   rework test & commented out code.

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

> 48:  * objects, such as asymmetric keys, certificates, and certificate 
> revocation
> 49:  * lists (CRLs).  It is defined in RFC 1421 and RFC 7468.  A PEM structure
> 50:  * consists of Base64-encoded binary data enclosed by a type-identifying 
> header

Make the last sentence to be consistent in `PEMEncoder`.

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

> 87:  *  <li>{@code EncryptedPrivateKeyInfo} : ENCRYPTED PRIVATE KEY</li>
> 88:  *  <li>{@code KeyPair} : PRIVATE KEY</li>
> 89:  *  <li>{@code KeyPair} (if configured with Encryption) : ENCRYPTED 
> PRIVATE KEY

Encryption should be all lower case.

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

> 96:  *  </ul>
> 97:  *
> 98:  * <p> This class is immutable and thread-safe.

In the examples, use either "private key" or `PrivateKey`, not both.

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

> 119:  * @see EncryptedPrivateKeyInfo
> 120:  *
> 121:  * @spec https://www.rfc-editor.org/info/rfc1421

Add a spec link to PKCS#8 2.0 since you mention it above.

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

> 263:      * uses the default encryption parameters of the provider that is 
> selected.
> 264:      * For greater flexibility with encryption options and parameters, 
> use
> 265:      * {@link EncryptedPrivateKeyInfo#encryptKey(DEREncodable, Key,

Line 256, did you mean to say "encoded" instead of "encrypted"? i.e.: "Only 
{@link PrivateKey} objects can be encoded with this newly ..."

It makes more sense when you read the following sentence. Also the result is an 
encoding, the encryption is an intermediate step.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/27147#discussion_r2360942020
PR Review Comment: https://git.openjdk.org/jdk/pull/27147#discussion_r2360986687
PR Review Comment: https://git.openjdk.org/jdk/pull/27147#discussion_r2360993760
PR Review Comment: https://git.openjdk.org/jdk/pull/27147#discussion_r2360960709
PR Review Comment: https://git.openjdk.org/jdk/pull/27147#discussion_r2361016203

Reply via email to