8298420: Implement JEP 470: PEM Encodings of Cryptographic Objects (Preview) #17543

ascarpino · 2024-01-24T00:01:06Z

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

Thanks

Tony

Progress

Change must be properly reviewed (1 review required, with at least 1 Reviewer)
Change requires CSR request JDK-8329419 to be approved
Change must not contain extraneous whitespace
Commit message must refer to an issue

Issues

JDK-8298420: Implement JEP 470: PEM Encodings of Cryptographic Objects (Preview) (Enhancement - P2)
JDK-8329419: Implement JEP 470: PEM Encodings of Cryptographic Objects (Preview) (CSR)

Reviewers

Weijun Wang (@wangweij - Reviewer)
Mark Reinhold (@mbreinhold - Reviewer)
Sean Mullan (@seanjmullan - Reviewer)
Jamil Nimeh (@jnimeh - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/17543/head:pull/17543
$ git checkout pull/17543

Update a local copy of the PR:
$ git checkout pull/17543
$ git pull https://git.openjdk.org/jdk.git pull/17543/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 17543

View PR using the GUI difftool:
$ git pr show -t 17543

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/17543.diff

Using Webrev

Link to Webrev Comment

bridgekeeper · 2024-01-24T00:02:21Z

👋 Welcome back ascarpino! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2024-01-24T00:03:36Z

@ascarpino The following label will be automatically applied to this pull request:

security

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

Fix CRL.

openjdk · 2024-03-13T20:15:01Z

@ascarpino This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8298420: Implement JEP 470: PEM Encodings of Cryptographic Objects (Preview)

Reviewed-by: weijun, mr, mullan, jnimeh

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 231 new commits pushed to the master branch:

28f5093: 8356233: NMT: tty->print_cr should not be used in VirtualMemoryTracker::add_reserved_region()
cd052c7: 8345431: Improve jar --validate to detect duplicate or invalid entries
b2a61a9: 8356985: Use "stdin.encoding" in Console's read*() methods
... and 228 more: https://git.openjdk.org/jdk/compare/5e50a584744d316dd881c9404f75e65f31bb0e75...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

openjdk · 2024-03-28T20:08:32Z

@ascarpino this pull request can not be integrated into master due to one or more merge conflicts. To resolve these merge conflicts and update this pull request you can run the following commands in the local repository for your personal fork:

git checkout pem
git fetch https://git.openjdk.org/jdk.git master
git merge FETCH_HEAD
# resolve conflicts and follow the instructions given by git merge
git commit -m "Merge master"
git push

Fixed P8v2 0xA0 Test: Use EKPI with PBEParameters to match EPK8 base64 cleanup

seanjmullan

A few remaining very minor comments. This is looking really good now.

src/java.base/share/classes/java/security/PEMEncoder.java

src/java.base/share/classes/java/security/PEMDecoder.java

src/java.base/share/classes/javax/crypto/EncryptedPrivateKeyInfo.java

src/java.base/share/classes/java/security/PEMDecoder.java

Krushna948 · 2025-05-21T07:55:37Z

src/java.base/share/classes/java/security/PEMRecord.java

+ *
+ * @param type the type identifier in the PEM header without PEM syntax labels.
+ *           For a public key, {@code type} would be "PUBLIC KEY".
+ * @param pem any data between the PEM header and footer.


Here 'pem' - any data between the PEM header and footer.
But the constructor description below for both the constructors
"pem the Base64-encoded data encapsulated by the PEM header and footer"

Observation
If I pass the data between PEM header and footer, the PEMRecord created successfully.
If I include Header and Footer
For eg as pem string,

"-----BEGIN PUBLIC KEY-----\n" +
"MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQDGRGrflwdiorIrC02pmr0jAKXI\n" +
"qxbBHxMUslLg8bjJiJCbanW7g7j7fR3RwGVU0cWh8rsQ/y4U7Yy0gBAsWCrr/TDS\n" +
"Xf3RWiZbQiQo6brZFiFZ5WgWgTpuzxDKpzLjyXCe17FXgbgEYscRPB/Rff6q2OS4\n" +
"H6stY3fHctzmU1HmUQIDAQAB\n" +
"-----END PUBLIC KEY-----";

PEMRecord creation throws java.lang.IllegalArgumentException: Illegal footer:

Looks some inconsistency in documentation/ behavior

The code requires a end of line character. Perhaps the documentation can be clarified later

src/java.base/share/classes/java/security/PEMRecord.java

Krushna948 · 2025-05-22T12:02:28Z

src/java.base/share/classes/java/security/PEMDecoder.java

+ * public key will be returned in that format.  Any type of PEM data can be
+ * decoded into a {@code PEMRecord} by specifying {@code PEMRecord.class}.
+ * If the Class parameter doesn't match the PEM content, an
+ * {@code IllegalArgumentException} will be thrown.


Looks a Typo, it supposed to be ClassCastException?

Thanks.. I can follow up on this with post integration.

src/java.base/share/classes/java/security/PEMDecoder.java

Krushna948 · 2025-05-22T12:49:32Z

src/java.base/share/classes/java/security/PEMDecoder.java

+     * from the specified {@link Provider} to produce cryptographic objects.
+     * Any errors using the {@code Provider} will occur during decoding.
+     *
+     * <p>If {@code provider} is {@code null}, a new instance is returned with


May this needs to be updated as below @throws NullPointerException if {@code provider} is null

yes, this could use some clarification post-integration

test/jdk/java/security/PEM/PEMDecoderTest.java

ascarpino · 2025-05-28T19:51:05Z

/integrate

openjdk · 2025-05-28T19:52:24Z

Going to push as commit bb2c80c.
Since your change was applied there have been 231 commits pushed to the master branch:

28f5093: 8356233: NMT: tty->print_cr should not be used in VirtualMemoryTracker::add_reserved_region()
cd052c7: 8345431: Improve jar --validate to detect duplicate or invalid entries
b2a61a9: 8356985: Use "stdin.encoding" in Console's read*() methods
... and 228 more: https://git.openjdk.org/jdk/compare/5e50a584744d316dd881c9404f75e65f31bb0e75...master

Your commit was automatically rebased without conflicts.

openjdk · 2025-05-28T19:52:31Z

@ascarpino Pushed as commit bb2c80c.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

ExE-Boss · 2025-08-18T20:02:46Z

src/java.base/share/classes/javax/crypto/EncryptedPrivateKeyInfo.java

 */

-public class EncryptedPrivateKeyInfo {
+public non-sealed class EncryptedPrivateKeyInfo implements DEREncodable {


Note that the JEPs¹² list EncryptedPrivateKeyInfo as:

public final class EncryptedPrivateKeyInfo { ... }

Footnotes

JEP 470: PEM Encodings of Cryptographic Objects (Preview) § DER‑encodable cryptographic objects ↩

JEP draft: PEM Encodings of Cryptographic Objects (Second Preview) § DER‑encodable cryptographic objects ↩

This has been fixed in the latest versions of the JEPs.

ascarpino added 3 commits January 23, 2024 09:28

publish pem

03e2088

Merge branch 'master' into pem

ceb233f

cleanup of copyrights and other commented out code

4b8bff1

openjdk bot added the security security-dev@openjdk.org label Jan 24, 2024

ascarpino added 8 commits January 25, 2024 13:04

try MIMEEncoder again

2f4e8cf

Decodes PKCS1

8fcc466

Decodes PKCS1

659fbb4

KeySpec

fca32e5

cleanup, added @SInCE, added KeyPair pub/private key decode (untested)

faaea5b

implement of()

25eb842

Add KeySpec check for KeyPair during decode.

c5eb599

Fix CRL.

Remove default constructor

5670c9e

openjdk bot added the merge-conflict Pull request has merge conflict with target branch label Mar 28, 2024

ascarpino added 4 commits April 1, 2024 16:04

Update javadoc

10c2918

Update javadoc. Sealed SO.

7bcab97

Merge branch 'master' into pem

4f64a0e

clean up for this-escape lint change

b220432

openjdk bot changed the title ~~JDK-8300911: PEM API (Preview)~~ 8300911: PEM API (Preview) Apr 10, 2024

ascarpino added 4 commits April 18, 2024 10:06

Results of API discussion and cleanup

e76d310

Switch from EPS to P8EPS & X509EPS

a215874

comment changes

3f8881f

Merge branch 'master' into pem

954ad9d

openjdk bot removed the merge-conflict Pull request has merge conflict with target branch label Apr 22, 2024

ascarpino added 3 commits April 24, 2024 11:47

comment/exception updates

6f3fd2d

Doc changes

24eed10

Fixed P8v2 0xA0 Test: Use EKPI with PBEParameters to match EPK8 base64 cleanup

update

05729e6

seanjmullan reviewed May 14, 2025

View reviewed changes

ascarpino added 3 commits May 14, 2025 18:55

comments

78d5bf6

Merge branch 'master' into pem

572841f

merge in JEP 513

8bf36d6

openjdk bot removed the merge-conflict Pull request has merge conflict with target branch label May 15, 2025

fix EKS error after JEP 513

80139d4

wangweij reviewed May 15, 2025

View reviewed changes

ascarpino added 3 commits May 15, 2025 21:27

comments

7402183

comments

397d0cb

comments

f05de12

openjdk bot removed the csr Pull request needs approved CSR before integration label May 21, 2025

test updates

8a07c3c

ascarpino changed the title ~~8298420: PEM API: Implementation (Preview)~~ 8298420: Implement JEP 470: PEM Encodings of Cryptographic Objects (Preview) May 27, 2025

PEM reading tweak

fdeaa93

ascarpino requested a review from mbreinhold May 27, 2025 23:13

Krushna948 reviewed May 28, 2025

View reviewed changes

mbreinhold approved these changes May 28, 2025

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label May 28, 2025

artur-oracle reviewed May 28, 2025

View reviewed changes

test/jdk/java/security/PEM/PEMDecoderTest.java Show resolved Hide resolved

seanjmullan approved these changes May 28, 2025

View reviewed changes

wangweij approved these changes May 28, 2025

View reviewed changes

jnimeh approved these changes May 28, 2025

View reviewed changes

openjdk bot added the integrated Pull request has been integrated label May 28, 2025

openjdk bot closed this May 28, 2025

openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels May 28, 2025

ExE-Boss reviewed Aug 18, 2025

View reviewed changes

8298420: Implement JEP 470: PEM Encodings of Cryptographic Objects (Preview) #17543

8298420: Implement JEP 470: PEM Encodings of Cryptographic Objects (Preview) #17543

Uh oh!

Conversation

ascarpino commented Jan 24, 2024 • edited by openjdk bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Issues

Reviewers

Reviewing

Uh oh!

bridgekeeper bot commented Jan 24, 2024

Uh oh!

openjdk bot commented Jan 24, 2024

Uh oh!

openjdk bot commented Mar 13, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openjdk bot commented Mar 28, 2024

Uh oh!

seanjmullan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ascarpino commented May 28, 2025

Uh oh!

openjdk bot commented May 28, 2025

Uh oh!

openjdk bot commented May 28, 2025

Uh oh!

Choose a reason for hiding this comment

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

11 participants

ascarpino commented Jan 24, 2024 •

edited by openjdk bot

Loading

openjdk bot commented Mar 13, 2024 •

edited

Loading