On Mon, 13 Oct 2025 17:22:25 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: > > 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
