RFR: 8298420: PEM API: Implementation (Preview) [v15]

Tue May 6 21:29:29 UTC 2025

On Fri, 2 May 2025 06:09:52 GMT, Anthony Scarpino <ascarpino at openjdk.org> wrote:

>> Hi all,
>> 
>> I need a code review of the PEM API.  Privacy-Enhanced Mail (PEM) is a format for encoding and decoding cryptographic keys and certificates.  It will be integrated into JDK24 as a Preview Feature.  Preview features does not permanently define the API and it is subject to change in future releases until it is finalized.
>> 
>> Details about this change can be seen at [PEM API JEP](https://bugs.openjdk.org/browse/JDK-8300911).
>> 
>> Thanks
>> 
>> Tony
>
> Anthony Scarpino has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 66 commits:
> 
>  - major code review comments update
>  - Merge branch 'master' into pem
>  - Merge branch 'master' into pem
>  - javadoc updates
>  - code review comments
>  - merge with master
>  - better comment and remove commented out code
>  - Merge branch 'master' into pem
>  - Merge branch 'pem-merge' into pem
>  - merge
>  - ... and 56 more: https://git.openjdk.org/jdk/compare/e2ae50d8...0c540327

Comments on the `PEMEncoder` API.

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

> 46: /**
> 47:  * PEMEncoder is an immutable class for Privacy-Enhanced Mail (PEM)
> 48:  * data.  PEM is a textual encoding used to store and transfer security

"An immutable class .. for ... data" sounds like it is the data itself. What about "for encoding data into...".

The first word `PEMEncoder` should be in `<code>`.

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

> 52:  * and footer.
> 53:  *
> 54:  * <p> Encoding may be performed on Java Cryptographic Extension (JCE) objects

Is "JCE objects" a formal term? We used to say "JCA and JCE". How do we call them now?

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

> 57:  * {@linkplain X509EncodedKeySpec X509} formats.
> 58:  *
> 59:  * <p> Encrypted private key PEM data can be built by calling the encode methods

I understand "encode methods" here mean `encode` and `encodeToString`, but at this early place in the specification no one knows about those methods yet. Does it make sense to append several sentences at the end of the previous paragraph saying something similar to "call encode to encode, call encodeToString to encode to string".

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

> 61:  * by passing an {@link EncryptedPrivateKeyInfo} object into the encode methods.
> 62:  *
> 63:  * <p> PKCS8 2.0 allows OneAsymmetricKey encoding, which may contain both private

It's "PKCS #8". Enclose `OneAsymmetricKey` in `<code>`.

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

> 68:  * {@linkplain PEMRecord#pem()} with a generated the PEM header and footer
> 69:  * from {@linkplain PEMRecord#type()}.  It will not check the validity of
> 70:  * the data.

Since you mention `PEMRecord` specifically, I'd see the clarification that the `leadingData` there will not be encoded. Otherwise, you cannot guarantee on the encoding.

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

> 73:  * {@link java.nio.charset.StandardCharsets#ISO_8859_1 ISO-8859-1}.
> 74:  *
> 75:  * @apiNote

Why is this an `apiNote`. Can't we put an example directly into the spec. Also, please add an example on encrypting a private key.

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

> 81:  *
> 82:  * @see PKCS8EncodedKeySpec
> 83:  * @see X509EncodedKeySpec

I cannot see how these 2 deserve this place. I'd rather link to `PEMRecord` and `PEMDecoder`.

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

> 86:  *       RFC 1421: Privacy Enhancement for Internet Electronic Mail
> 87:  * @spec https://www.rfc-editor.org/info/rfc7468
> 88:  *       RFC 7468: Textual Encodings of PKIX, PKCS, and CMS Structures

You mentioned PKCS #8 2.0. Is it worth adding it here?

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

> 125: 
> 126:     /**
> 127:      * Encoded a given {@code DEREncodable} and return the PEM encoding in a

Typo: `Encodes`.

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

> 126:     /**
> 127:      * Encoded a given {@code DEREncodable} and return the PEM encoding in a
> 128:      * String

Either `<code>String</code>` or `string`.

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

> 128:      * String
> 129:      *
> 130:      * @param de a cryptographic object to be PEM encoded that implements

`@param de` for the 2 methods should be the same.

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

PR Review: https://git.openjdk.org/jdk/pull/17543#pullrequestreview-2819604929
PR Review Comment: https://git.openjdk.org/jdk/pull/17543#discussion_r2076258346
PR Review Comment: https://git.openjdk.org/jdk/pull/17543#discussion_r2076262106
PR Review Comment: https://git.openjdk.org/jdk/pull/17543#discussion_r2076267314
PR Review Comment: https://git.openjdk.org/jdk/pull/17543#discussion_r2076270426
PR Review Comment: https://git.openjdk.org/jdk/pull/17543#discussion_r2076273132
PR Review Comment: https://git.openjdk.org/jdk/pull/17543#discussion_r2076281319
PR Review Comment: https://git.openjdk.org/jdk/pull/17543#discussion_r2076282886
PR Review Comment: https://git.openjdk.org/jdk/pull/17543#discussion_r2076284962
PR Review Comment: https://git.openjdk.org/jdk/pull/17543#discussion_r2076288529
PR Review Comment: https://git.openjdk.org/jdk/pull/17543#discussion_r2076291290
PR Review Comment: https://git.openjdk.org/jdk/pull/17543#discussion_r2076299229