8231107: Allow store password to be null when saving a PKCS12 KeyStore #5950

wangweij · 2021-10-14T14:43:32Z

You can create a password-less PKCS12 KeyStore file now by calling ks.store(outStream, null) no matter what the default cert protection algorithm and Mac algorithm are defined in java.security.

Note: the system properties set in ToolsJDK.gmk to generate cacerts must be retained (at the moment) because the tool is launched with BOOT_JDK.

Progress

Change must not contain extraneous whitespace
Commit message must refer to an issue
Change must be properly reviewed

Issues

JDK-8231107: Allow store password to be null when saving a PKCS12 KeyStore
JDK-8274862: Allow store password to be null when saving a PKCS12 KeyStore (CSR) ⚠️ Issue is not open.

Reviewers

Sean Mullan (@seanjmullan - Reviewer) ⚠️ Review applies to 7d78977

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.java.net/jdk pull/5950/head:pull/5950
$ git checkout pull/5950

Update a local copy of the PR:
$ git checkout pull/5950
$ git pull https://git.openjdk.java.net/jdk pull/5950/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 5950

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

Using diff file

Download this PR as a diff file:
https://git.openjdk.java.net/jdk/pull/5950.diff

…enc certs not easy 8231107: Storing PKCS12 keystore without integrity protection and no enc certs not easy

bridgekeeper · 2021-10-14T14:43:49Z

👋 Welcome back weijun! 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.

wangweij · 2021-10-14T14:43:50Z

/csr

openjdk · 2021-10-14T14:45:30Z

@wangweij this pull request will not be integrated until the CSR request JDK-8274862 for issue JDK-8231107 has been approved.

openjdk · 2021-10-14T14:47:12Z

@wangweij 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.

mlbridge · 2021-10-14T14:53:10Z

Webrevs

seanjmullan · 2021-10-25T17:02:10Z

test/jdk/sun/security/pkcs12/EmptyPassword.java

-            ks.store(fos, new char[1]);
-        }
+
+        // 8202299: interop before new char[0] and new char[1]


Can you make this comment more descriptive? Not sure what "before" means. Is this just making sure you can store a keystore and key entry with "\0" as the password and load it back with ""? Should you also try to load it back with "\0" too?

Typo: s/before/between/.

I'll add one with "\0". That should always work since it's the same password used in load() and store().

seanjmullan · 2021-10-25T17:05:58Z

test/jdk/sun/security/pkcs12/EmptyPassword.java

@@ -23,7 +23,7 @@

 /*
 * @test
- * @bug 8202299
+ * @bug 8202299 8231107
 * @modules java.base/sun.security.tools.keytool


Can you add an @summary?

I'll update the existing summary to @summary Testing empty (null, "", "\0") password behaviors.

seanjmullan · 2021-10-26T15:44:03Z

test/jdk/sun/security/pkcs12/EmptyPassword.java

@@ -23,7 +23,7 @@

 /*
 * @test
- * @bug 8202299
+ * @bug 8202299 8231107
 * @modules java.base/sun.security.tools.keytool


seanjmullan · 2021-10-26T15:44:28Z

test/jdk/sun/security/pkcs12/EmptyPassword.java

-            ks.store(fos, new char[1]);
-        }
+
+        // 8202299: interop before new char[0] and new char[1]


only in patch2: unchanged:

openjdk · 2021-11-30T22:41:14Z

@wangweij 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:

8231107: Allow store password to be null when saving a PKCS12 KeyStore

Reviewed-by: mullan

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 630 new commits pushed to the master branch:

2942646: 8276683: Malformed Javadoc inline tags in JDK source in com/sun/tools/javac/util/RawDiagnosticFormatter.java
e30e676: 8277606: String(String) constructor could copy hashIsZero
5a4a9bb: 8278019: ProblemList java/awt/dnd/BadSerializationTest/BadSerializationTest.java on linux and windows
15a6806: 8277434: tests fail with "assert(is_forwarded()) failed: only decode when actually forwarded"
21d9ca6: 8274983: C1 optimizes the invocation of private interface methods
98a9f03: 8277602: Deopt code does not extend the stack enough if the caller is an optimize entry blob
9150840: 8277899: Parallel: Simplify PSVirtualSpace::initialize logic
01cefc9: 8277977: Incorrect references to --enable-reproducible-builds in docs
69f56a0: 8264485: build.tools.depend.Depend.toString(byte[]) creates malformed hex strings
fecf906: 8267928: Loop predicate gets inexact loop limit before PhaseIdealLoop::rc_predicate
... and 620 more: https://git.openjdk.java.net/jdk/compare/07b1f1c282ee0a7df6a6b0f240962a032ea3a413...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.

wangweij · 2021-12-01T01:00:22Z

/integrate

openjdk · 2021-12-01T01:02:09Z

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

ab867f6: 8272162: S4U2Self ticket without forwardable flag
dd73e3c: 8277814: ConcurrentRefineThread should report rate when deactivating
65251f7: 8151594: Move concurrent refinement thread activation logging out of GC pause
f1c20e9: 8190748: java/text/Format/DateFormat/DateFormatTest.java and NonGregorianFormatTest fail intermittently
2942646: 8276683: Malformed Javadoc inline tags in JDK source in com/sun/tools/javac/util/RawDiagnosticFormatter.java
e30e676: 8277606: String(String) constructor could copy hashIsZero
5a4a9bb: 8278019: ProblemList java/awt/dnd/BadSerializationTest/BadSerializationTest.java on linux and windows
15a6806: 8277434: tests fail with "assert(is_forwarded()) failed: only decode when actually forwarded"
21d9ca6: 8274983: C1 optimizes the invocation of private interface methods
98a9f03: 8277602: Deopt code does not extend the stack enough if the caller is an optimize entry blob
... and 624 more: https://git.openjdk.java.net/jdk/compare/07b1f1c282ee0a7df6a6b0f240962a032ea3a413...master

Your commit was automatically rebased without conflicts.

openjdk · 2021-12-01T01:02:24Z

@wangweij Pushed as commit 7049c13.

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

8231107: Storing PKCS12 keystore without integrity protection and no …

7d78977

…enc certs not easy 8231107: Storing PKCS12 keystore without integrity protection and no enc certs not easy

openjdk bot added rfr Pull request is ready for review csr Pull request needs approved CSR before integration labels Oct 14, 2021

openjdk bot added the security security-dev@openjdk.org label Oct 14, 2021

wangweij changed the title ~~8231107: Storing PKCS12 keystore without integrity protection and no enc certs not easy~~ 8231107: Allow store password to be null when saving a PKCS12 KeyStore Oct 14, 2021

seanjmullan reviewed Oct 25, 2021

View reviewed changes

seanjmullan approved these changes Oct 26, 2021

View reviewed changes

wangweij added 3 commits October 26, 2021 13:46

test update

aae1b0c

spec change

3670f99

only in patch2: unchanged:

same spec in SPI

266cf6d

openjdk bot removed the csr Pull request needs approved CSR before integration label Nov 30, 2021

openjdk bot added the ready Pull request is ready to be integrated label Nov 30, 2021

openjdk bot closed this Dec 1, 2021

openjdk bot added integrated Pull request has been integrated and removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Dec 1, 2021

wangweij deleted the 8231107 branch December 1, 2021 01:12

8231107: Allow store password to be null when saving a PKCS12 KeyStore #5950

8231107: Allow store password to be null when saving a PKCS12 KeyStore #5950

Uh oh!

Conversation

wangweij commented Oct 14, 2021 • 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 Oct 14, 2021

Uh oh!

wangweij commented Oct 14, 2021

Uh oh!

openjdk bot commented Oct 14, 2021

Uh oh!

openjdk bot commented Oct 14, 2021

Uh oh!

mlbridge bot commented Oct 14, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Webrevs

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

openjdk bot commented Nov 30, 2021

Uh oh!

wangweij commented Dec 1, 2021

Uh oh!

openjdk bot commented Dec 1, 2021

Uh oh!

openjdk bot commented Dec 1, 2021

Uh oh!

Uh oh!

wangweij commented Oct 14, 2021 •

edited by openjdk bot

Loading

mlbridge bot commented Oct 14, 2021 •

edited

Loading