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

[Sean Mullan]

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

src/java.base/share/classes/sun/security/pkcs/PKCS8Key.java line 79:

> 77: 
> 78:     /* PKCS8 version of the PEM */
> 79:     protected int version;

Do these need to be protected? It doesn't seem like any subclasses need them.

src/java.base/share/classes/sun/security/pkcs/PKCS8Key.java line 193:

> 191:      *
> 192:      * @param encoded the DER-encoded SubjectPublicKeyInfo value
> 193:      * @exception IOException on data format errors

Change to `InvalidKeyException`.

src/java.base/share/classes/sun/security/pkcs/PKCS8Key.java line 243:

> 241:     }
> 242: 
> 243:     public byte[] getPrivKeyMaterial() {

Do we really need this method, esp since it is the private key? I can't find any code that calls this.

src/java.base/share/classes/sun/security/pkcs/PKCS8Key.java line 305:

> 303:             } catch (IOException e) {
> 304:                 // encodedKey is still null
> 305:                 throw new SecurityException(e);

Should return null instead since that is what `getEncoded()` specifies.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/17543#discussion_r2075978714
PR Review Comment: https://git.openjdk.org/jdk/pull/17543#discussion_r2075985424
PR Review Comment: https://git.openjdk.org/jdk/pull/17543#discussion_r2075990253
PR Review Comment: https://git.openjdk.org/jdk/pull/17543#discussion_r2075998393