JDK-8293412 Remove unnecessary java.security.egd overrides #10716

mcpowers · 2022-10-14T21:49:07Z

https://bugs.openjdk.org/browse/JDK-8293412

Progress

Change must be properly reviewed (1 review required, with at least 1 Reviewer)
Change must not contain extraneous whitespace
Commit message must refer to an issue

Issue

JDK-8293412: Remove unnecessary java.security.egd overrides

Reviewers

Xue-Lei Andrew Fan (@XueleiFan - Reviewer) ⚠️ Review applies to 1375cc9b
Daniel Jeliński (@djelinski - Committer) ⚠️ Review applies to 2dc18e5c

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 10716

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

Using diff file

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

bridgekeeper · 2022-10-14T21:50:19Z

👋 Welcome back mpowers! 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 · 2022-10-14T21:53:47Z

@mcpowers The following labels will be automatically applied to this pull request:

core-libs
security

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

mcpowers · 2022-10-28T22:39:32Z

No failures were noticed during mach5 and jtreg testing.

mlbridge · 2022-10-29T02:11:05Z

Webrevs

XueleiFan

I think it is a good clean up so that the default one get used in testing.

openjdk · 2022-10-29T04:22:50Z

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

8293412: Remove unnecessary java.security.egd overrides

Reviewed-by: xuelei, djelinski

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

ea83cb9: 8297450: ScaledTextFieldBorderTest.java fails when run with -show parameter
336d230: 8297958: NMT: Display peak values
0d2a9ee: 8298142: Update internal comment on language features in SourceVersion
203251f: 8297379: Enable the ByteBuffer path of Poly1305 optimizations
1e46832: 8297602: Compiler crash with type annotation and generic record during pattern matching
b0e5432: 8297687: new URI(S,S,S,S) throws exception with incorrect index position reported in the error message
2243646: 8298145: Remove ContiguousSpace::capacity
84b927a: 8296024: Usage of DirectBuffer::address should be guarded
a9e6c62: 8297186: G1 triggers unnecessary full GCs when heap utilization is low
4458de9: 8297172: Fix some issues of auto-vectorization of Long.bitCount/numberOfTrailingZeros/numberOfLeadingZeros()
... and 48 more: https://git.openjdk.org/jdk/compare/2821fa9883cc9687b53e3bb5655732a614cff8fc...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.

As you do not have Committer status in this project an existing Committer must agree to sponsor your change. Possible candidates are the reviewers of this PR (@XueleiFan, @djelinski) but any other Committer may sponsor as well.

➡️ To flag this PR as ready for integration with the above commit message, type /integrate in a new comment. (Afterwards, your sponsor types /sponsor in a new comment to perform the integration).

AlanBateman · 2022-10-29T06:03:13Z

/label remove core-libs

openjdk · 2022-10-29T06:04:44Z

@AlanBateman
The core-libs label was successfully removed.

djelinski

I'm still verifying the jarsigner and the SecurityTools changes, but all of the other files actually need the EGD override.

djelinski · 2022-10-31T07:41:11Z

test/jdk/java/security/SecureRandom/EnoughSeedTest.java

@@ -40,8 +40,6 @@ public class EnoughSeedTest {
            = Security.getProperty(DRBG_CONFIG);

    public static void main(String[] args) {
-        System.setProperty("java.security.egd", "file:/dev/urandom");


This test uses generateSeed and needs egd to avoid reading from /dev/random

Backing out change. I hadn't considered the fact that this test might be run on slower hardware.

djelinski · 2022-10-31T07:42:23Z

test/jdk/java/security/SecureRandom/MacNativePRNGSetSeed.java

@@ -27,7 +27,7 @@
 * @bug 8156709
 * @summary Cannot call setSeed on NativePRNG on Mac if EGD is /dev/urandom
 * @requires os.name == "Mac OS X"
- * @run main/othervm -Djava.security.egd=file:/dev/urandom MacNativePRNGSetSeed


this change makes this test pointless, see summary 2 lines above

Backing out change.

djelinski · 2022-10-31T07:47:19Z

test/jdk/java/security/SecureRandom/MultiThreadTest.java

@@ -41,7 +41,7 @@
 * @library /lib/testlibrary
 * @summary Test behavior of a shared SecureRandom object when it is operated
 *          by multiple threads concurrently.
- * @run main/othervm -Djava.security.egd=file:/dev/urandom MultiThreadTest


This uses generateSeed and needs egd to avoid using dev/random

Backing out change.

djelinski · 2022-10-31T07:48:19Z

test/jdk/java/security/SecureRandom/ApiTest.java

@@ -53,8 +53,6 @@ public class ApiTest {
            = Security.getProperty(DRBG_CONFIG);

    public static void main(String[] args) throws Exception {
-        System.setProperty("java.security.egd", "file:/dev/urandom");


This test uses generateSeed and needs egd.

Backing out change.

djelinski · 2022-10-31T07:50:06Z

test/jdk/java/security/SecureRandom/SerializedSeedTest.java

@@ -29,7 +29,7 @@
 *          as well from its serialized instance in the same time then the
 *          generated random numbers should be different when one or both are
 *          reseeded.
- * @run main/othervm -Djava.security.egd=file:/dev/urandom SerializedSeedTest


This uses generateSeed and needs egd to avoid using dev/random

Backing out change.

djelinski · 2022-10-31T07:50:33Z

test/jdk/java/security/SecureRandom/Serialize.java

@@ -33,8 +33,6 @@
 public class Serialize {

    public static void main(String args[]) throws Exception {
-        System.setProperty("java.security.egd", "file:/dev/urandom");


This uses generateSeed and needs egd to avoid using dev/random

Backing out change.

djelinski · 2022-10-31T09:24:28Z

test/jdk/sun/security/provider/SeedGenerator/SeedGeneratorChoice.java

@@ -26,7 +26,7 @@
 * @bug 6998583 8141039
 * @summary NativeSeedGenerator is making 8192 byte read requests from
 *             entropy pool on each init.
- * @run main/othervm -Djava.security.egd=file:/dev/random SeedGeneratorChoice


This test uses generateSeed and should be run with egd pointing to /dev/urandom to avoid reading from /dev/random

Backing out change.

Changed to /dev/urandom.

djelinski · 2022-10-31T09:34:03Z

test/jdk/sun/security/provider/SecureRandom/AutoReseed.java

@@ -28,7 +28,7 @@
 * @test
 * @bug 8051408
 * @summary make sure nextBytes etc can be called before setSeed
- * @run main/othervm -Djava.security.egd=file:/dev/urandom AutoReseed


This test uses generateSeed and needs egd to avoid reading from /dev/random

Backing out change.

djelinski

Thanks! LGTM.

mcpowers · 2022-11-22T22:20:07Z

Verified with testing that /dev/urandom is used when overrides are removed from Compatibility.java and SecurityTools.java.

mcpowers · 2022-12-06T20:23:51Z

/integrate

openjdk · 2022-12-06T20:25:23Z

@mcpowers
Your change (at version 8076b61) is now ready to be sponsored by a Committer.

wangweij · 2022-12-06T20:35:02Z

/sponsor

openjdk · 2022-12-06T20:36:55Z

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

ea83cb9: 8297450: ScaledTextFieldBorderTest.java fails when run with -show parameter
336d230: 8297958: NMT: Display peak values
0d2a9ee: 8298142: Update internal comment on language features in SourceVersion
203251f: 8297379: Enable the ByteBuffer path of Poly1305 optimizations
1e46832: 8297602: Compiler crash with type annotation and generic record during pattern matching
b0e5432: 8297687: new URI(S,S,S,S) throws exception with incorrect index position reported in the error message
2243646: 8298145: Remove ContiguousSpace::capacity
84b927a: 8296024: Usage of DirectBuffer::address should be guarded
a9e6c62: 8297186: G1 triggers unnecessary full GCs when heap utilization is low
4458de9: 8297172: Fix some issues of auto-vectorization of Long.bitCount/numberOfTrailingZeros/numberOfLeadingZeros()
... and 48 more: https://git.openjdk.org/jdk/compare/2821fa9883cc9687b53e3bb5655732a614cff8fc...master

Your commit was automatically rebased without conflicts.

openjdk · 2022-12-06T20:37:28Z

@wangweij @mcpowers Pushed as commit 79d163d.

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

mcpowers added 3 commits September 23, 2022 11:16

first iteration

016a9cd

Merge

e80e36e

second iteration

6a710c7

openjdk bot added security security-dev@openjdk.org core-libs core-libs-dev@openjdk.org labels Oct 14, 2022

Merge

1375cc9

mcpowers marked this pull request as ready for review October 29, 2022 02:04

openjdk bot added the rfr Pull request is ready for review label Oct 29, 2022

XueleiFan approved these changes Oct 29, 2022

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Oct 29, 2022

openjdk bot removed the core-libs core-libs-dev@openjdk.org label Oct 29, 2022

djelinski suggested changes Oct 31, 2022

View reviewed changes

mcpowers added 2 commits November 9, 2022 07:44

Merge

cc8c952

yes we need /dev/urandom

2dc18e5

djelinski approved these changes Nov 15, 2022

View reviewed changes

javadoc update

80be7e6

Merge

8076b61

openjdk bot added the sponsor Pull request is ready to be sponsored label Dec 6, 2022

openjdk bot added the integrated Pull request has been integrated label Dec 6, 2022

openjdk bot closed this Dec 6, 2022

openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review sponsor Pull request is ready to be sponsored labels Dec 6, 2022

mcpowers deleted the JDK-8293412 branch December 7, 2022 13:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

JDK-8293412 Remove unnecessary java.security.egd overrides #10716

JDK-8293412 Remove unnecessary java.security.egd overrides #10716

mcpowers commented Oct 14, 2022 •

edited by openjdk bot

Loading

bridgekeeper bot commented Oct 14, 2022

openjdk bot commented Oct 14, 2022

mcpowers commented Oct 28, 2022 •

edited

Loading

mlbridge bot commented Oct 29, 2022 •

edited

Loading

XueleiFan left a comment

openjdk bot commented Oct 29, 2022 •

edited

Loading

AlanBateman commented Oct 29, 2022

openjdk bot commented Oct 29, 2022

djelinski left a comment

djelinski Oct 31, 2022

mcpowers Nov 11, 2022

djelinski Oct 31, 2022

mcpowers Nov 11, 2022

djelinski Oct 31, 2022

mcpowers Nov 11, 2022

djelinski Oct 31, 2022

mcpowers Nov 11, 2022

djelinski Oct 31, 2022

mcpowers Nov 11, 2022

djelinski Oct 31, 2022

mcpowers Nov 11, 2022

djelinski Oct 31, 2022

mcpowers Nov 11, 2022

mcpowers Nov 11, 2022

djelinski Oct 31, 2022

mcpowers Nov 11, 2022

djelinski left a comment

mcpowers commented Nov 22, 2022

mcpowers commented Dec 6, 2022

openjdk bot commented Dec 6, 2022

wangweij commented Dec 6, 2022

openjdk bot commented Dec 6, 2022

openjdk bot commented Dec 6, 2022

JDK-8293412 Remove unnecessary java.security.egd overrides #10716

JDK-8293412 Remove unnecessary java.security.egd overrides #10716

Conversation

mcpowers commented Oct 14, 2022 • edited by openjdk bot Loading

Progress

Issue

Reviewers

Reviewing

bridgekeeper bot commented Oct 14, 2022

openjdk bot commented Oct 14, 2022

mcpowers commented Oct 28, 2022 • edited Loading

mlbridge bot commented Oct 29, 2022 • edited Loading

Webrevs

XueleiFan left a comment

Choose a reason for hiding this comment

openjdk bot commented Oct 29, 2022 • edited Loading

AlanBateman commented Oct 29, 2022

openjdk bot commented Oct 29, 2022

djelinski left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

djelinski left a comment

Choose a reason for hiding this comment

mcpowers commented Nov 22, 2022

mcpowers commented Dec 6, 2022

openjdk bot commented Dec 6, 2022

wangweij commented Dec 6, 2022

openjdk bot commented Dec 6, 2022

openjdk bot commented Dec 6, 2022

mcpowers commented Oct 14, 2022 •

edited by openjdk bot

Loading

mcpowers commented Oct 28, 2022 •

edited

Loading

mlbridge bot commented Oct 29, 2022 •

edited

Loading

openjdk bot commented Oct 29, 2022 •

edited

Loading