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

[Anthony Scarpino]

On Wed, 1 Oct 2025 20:13:45 GMT, Weijun Wang <weijun at openjdk.org> wrote:

>> Anthony Scarpino has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   missed some decoder comments
>
> 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.

"first" sounds awkward to me, I can change it to "a PEM footer".

> 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.

I think it would be confusing to have @code around it or other monospaced font when there is no class with that name.  If that were the case, then PKCS #8 and ASN.1 would need it to and I think it would become harder to read with all the font changes.

> 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.

line 91?

> src/java.base/share/classes/javax/crypto/EncryptedPrivateKeyInfo.java line 1:
> 
>> 1: /*
> 
> Since you added `getKey` and `getKeyPair` with a password argument, can we also add a `getKeySpec` with the same argument to be consistent?
> 
> Also, if you add this method, can it be used instead of `Pem.decryptEncoding`? That method is called inside EPKI and it creates another EPKI which looks wasteful and dangerously recursive.

Hmm..  I had not planned to add any to getKeySpec as I view that as legacy.  We can consider that in a future update.

I can probably change the internal method to take the `this` instead.

> 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`.

No, I should clean that up.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/27147#discussion_r2411999544
PR Review Comment: https://git.openjdk.org/jdk/pull/27147#discussion_r2412463817
PR Review Comment: https://git.openjdk.org/jdk/pull/27147#discussion_r2396287482
PR Review Comment: https://git.openjdk.org/jdk/pull/27147#discussion_r2414636177
PR Review Comment: https://git.openjdk.org/jdk/pull/27147#discussion_r2417915297