On Fri, 12 Sep 2025 17:54:49 GMT, Weijun Wang <[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 > > src/java.base/share/classes/sun/security/pkcs/PKCS8Key.java line 159: > >> 157: privKeyMaterial = val.data.getOctetString(); >> 158: >> 159: // Special check and parsing for ECDSA's SEC1v2 format > > So there are 2 ways to extract public key from PKCS8. Are you going to check > whether they match? > > Also, can we add a test case for this format? In this method, there is only one path for SEC1v2 requires the version to be 1, PKCS8 requires the version to be 2. The two should not intersect in this method. It is true that a version 2 OneAsymmetricKey could contain a SEC1v2 private key encoding. At that point a KeyFactory needs to generate both keys to verify. I think that goes beyond what is necessary for what amounts to a "user error" in putting the wrong public and private key pair together. I would like to explore redoing PKCS8Key.java as it has taken on many more tasks with encoding & decoding different algorithms. But I don't want that to be tied to a JEP, rather a bug or RFE. > src/java.base/share/classes/sun/security/util/Pem.java line 355: > >> 353: return pemEncoded(pem.type(), p); >> 354: } >> 355: > > Is it correct to put the 2 new methods here? Seems not closely related to PEM. They are needed by PEM and EKPI, so this seems like the right place. > src/java.base/share/classes/sun/security/util/Pem.java line 387: > >> 385: * return a PrivateKey >> 386: * @param provider KeyFactory provider >> 387: */ > > This method is very similar to `PKCS8Key::parseKey`. Maybe we should combine > them. Before the code review, I intentionally split these apart. I didn't like how the code looked and that returning a `DEREncodable` complicated `parseKey()`. It made PKCS8Key much more complicated with typecasting and passing in `pair`. If I redo `PKCS8Key.java`, this would be on the list to find a more elegant way to merge them. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/27147#discussion_r2349810230 PR Review Comment: https://git.openjdk.org/jdk/pull/27147#discussion_r2349813496 PR Review Comment: https://git.openjdk.org/jdk/pull/27147#discussion_r2349830806
