Integrated: 8209935: Test to cover CodeSource.getCodeSigners()
On Mon, 18 Apr 2022 09:46:06 GMT, Sibabrata Sahoo wrote: > A Test updated to cover the getCodeSigners. This pull request has now been integrated. Changeset: 0cb0ecf4 Author: Sibabrata Sahoo URL: https://git.openjdk.org/jdk/commit/0cb0ecf4433f1054ba2f0fbdabee01323893e0fe Stats: 157 lines in 3 files changed: 149 ins; 1 del; 7 mod 8209935: Test to cover CodeSource.getCodeSigners() Reviewed-by: mullan - PR: https://git.openjdk.org/jdk/pull/8286
Re: RFR: 8209935: Test to cover CodeSource.getCodeSigners() [v5]
> A Test updated to cover the getCodeSigners. Sibabrata Sahoo has updated the pull request incrementally with two additional commits since the last revision: - 8209935: Test to cover CodeSource.getCodeSigners() - 8209935: Test to cover CodeSource.getCodeSigners() - Changes: - all: https://git.openjdk.org/jdk/pull/8286/files - new: https://git.openjdk.org/jdk/pull/8286/files/512cb615..a0c1d070 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=8286&range=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=8286&range=03-04 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/8286.diff Fetch: git fetch https://git.openjdk.org/jdk pull/8286/head:pull/8286 PR: https://git.openjdk.org/jdk/pull/8286
Re: RFR: 8209935: Test to cover CodeSource.getCodeSigners() [v4]
On Thu, 9 Jun 2022 21:12:26 GMT, Sean Mullan wrote: >> Sibabrata Sahoo has updated the pull request incrementally with one >> additional commit since the last revision: >> >> 8209935: Test to cover CodeSource.getCodeSigners() > > test/jdk/java/security/CodeSource/CertsMatch.java line 27: > >> 25: * @test >> 26: * @bug 6299163 >> 27: * @summary REGRESSION: java.security.CodeSource#equals not symmetric > > I think you should remove this line about "REGRESSION" as the summary is > supposed to be about what the test is testing. Suggest changing this to > summarize what methods are being tested. Updated the summary. - PR: https://git.openjdk.org/jdk/pull/8286
Re: RFR: 8209935: Test to cover CodeSource.getCodeSigners() [v4]
> A Test updated to cover the getCodeSigners. Sibabrata Sahoo has updated the pull request incrementally with one additional commit since the last revision: 8209935: Test to cover CodeSource.getCodeSigners() - Changes: - all: https://git.openjdk.java.net/jdk/pull/8286/files - new: https://git.openjdk.java.net/jdk/pull/8286/files/c6318862..512cb615 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=8286&range=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=8286&range=02-03 Stats: 144 lines in 2 files changed: 144 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/8286.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8286/head:pull/8286 PR: https://git.openjdk.java.net/jdk/pull/8286
Re: RFR: 8209935: Test to cover CodeSource.getCodeSigners() [v3]
> A Test updated to cover the getCodeSigners. Sibabrata Sahoo has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains four additional commits since the last revision: - Merge remote-tracking branch 'upstream/master' into JDK-8209935 - Update Implies.java - Update Implies.java - 8209935: Test to cover CodeSource.getCodeSigners() - Changes: - all: https://git.openjdk.java.net/jdk/pull/8286/files - new: https://git.openjdk.java.net/jdk/pull/8286/files/70618a17..c6318862 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=8286&range=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=8286&range=01-02 Stats: 359585 lines in 5089 files changed: 236066 ins; 93423 del; 30096 mod Patch: https://git.openjdk.java.net/jdk/pull/8286.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8286/head:pull/8286 PR: https://git.openjdk.java.net/jdk/pull/8286
Integrated: 8286969: Add a new test library API to execute kinit in SecurityTools.java
On Wed, 18 May 2022 16:19:40 GMT, Sibabrata Sahoo wrote: > A new API to execute kinit. This pull request has now been integrated. Changeset: dbda0e2b Author: Sibabrata Sahoo URL: https://git.openjdk.java.net/jdk/commit/dbda0e2bda5d8ba86f068684941a05387947d993 Stats: 12 lines in 1 file changed: 12 ins; 0 del; 0 mod 8286969: Add a new test library API to execute kinit in SecurityTools.java Reviewed-by: rhalade, weijun - PR: https://git.openjdk.java.net/jdk/pull/8775
RFR: 8286969: Add a new test library API to execute kinit in SecurityTools.java
A new API to execute kinit. - Commit messages: - 8286969: Add a new test library API to execute kinit in SecurityTools.java Changes: https://git.openjdk.java.net/jdk/pull/8775/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=8775&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8286969 Stats: 12 lines in 1 file changed: 12 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/8775.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8775/head:pull/8775 PR: https://git.openjdk.java.net/jdk/pull/8775
Re: RFR: 8209935: Test to cover CodeSource.getCodeSigners() [v2]
> A Test updated to cover the getCodeSigners. Sibabrata Sahoo has updated the pull request incrementally with one additional commit since the last revision: Update Implies.java - Changes: - all: https://git.openjdk.java.net/jdk/pull/8286/files - new: https://git.openjdk.java.net/jdk/pull/8286/files/7db24d99..70618a17 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=8286&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=8286&range=00-01 Stats: 4 lines in 1 file changed: 2 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/8286.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8286/head:pull/8286 PR: https://git.openjdk.java.net/jdk/pull/8286
Re: RFR: 8209935: Test to cover CodeSource.getCodeSigners() [v2]
On Mon, 18 Apr 2022 13:22:35 GMT, Sibabrata Sahoo wrote: >> A Test updated to cover the getCodeSigners. > > Sibabrata Sahoo has updated the pull request incrementally with one > additional commit since the last revision: > > Update Implies.java All comments addressed. - PR: https://git.openjdk.java.net/jdk/pull/8286
RFR: 8209935: Test to cover CodeSource.getCodeSigners()
A Test updated to cover the getCodeSigners. - Commit messages: - Update Implies.java - 8209935: Test to cover CodeSource.getCodeSigners() Changes: https://git.openjdk.java.net/jdk/pull/8286/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=8286&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8209935 Stats: 12 lines in 1 file changed: 4 ins; 2 del; 6 mod Patch: https://git.openjdk.java.net/jdk/pull/8286.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8286/head:pull/8286 PR: https://git.openjdk.java.net/jdk/pull/8286
Re: RFR: 8284641: Doc errors in sun.security.ssl.SSLSessionContextImpl
On Mon, 11 Apr 2022 04:02:44 GMT, John Jiang wrote: > JDK-8228396 turned stateless resumption on by default, but the JavaDoc was > not modified accordingly. > And a "{" is missing for @systemProperty > jdk.tls.server.enableSessionTicketExtension. Marked as reviewed by ssahoo (Committer). - PR: https://git.openjdk.java.net/jdk/pull/8173
Re: RFR: 8281717: Cover logout method for several LoginModule [v3]
> Added coverage to logout method. Sibabrata Sahoo has updated the pull request incrementally with one additional commit since the last revision: Update AllPlatforms.java Updated exception handling to ignore the case where the running platform doesn't match the test case. In that case required native library loading will fail. - Changes: - all: https://git.openjdk.java.net/jdk/pull/7940/files - new: https://git.openjdk.java.net/jdk/pull/7940/files/13a999b0..b7e3bc04 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=7940&range=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=7940&range=01-02 Stats: 5 lines in 1 file changed: 2 ins; 0 del; 3 mod Patch: https://git.openjdk.java.net/jdk/pull/7940.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7940/head:pull/7940 PR: https://git.openjdk.java.net/jdk/pull/7940
Re: RFR: 8281717: Cover logout method for several LoginModule [v2]
On Thu, 24 Mar 2022 23:02:47 GMT, Rajan Halade wrote: >> Sibabrata Sahoo has updated the pull request incrementally with two >> additional commits since the last revision: >> >> - Update AllPlatforms.java >> - Update AllPlatforms.java > > test/jdk/com/sun/security/auth/module/AllPlatforms.java line 26: > >> 24: /* >> 25: * @test >> 26: * @bug 8039951 8281717 > > No need to add bug id here as this is not a product change. Done. > test/jdk/com/sun/security/auth/module/AllPlatforms.java line 44: > >> 42: try { >> 43: login("windows", "NTLoginModule", "required"); >> 44: login("unix", "UnixLoginModule", "required"); > > thanks for fixing this code. Test earlier skipped UnixLoginModule early on > "nix" platform as windows login would fail. It will run on all platform even the unsuccessful ones appear before. > test/jdk/com/sun/security/auth/module/AllPlatforms.java line 45: > >> 43: UNIX_MODULE, "optional", >> 44: NT_MODULE, "optional"); >> 45: login(OS.replaceAll("[^a-zA-Z0-9]", ""), >> getPlatformLoginModule(), "required"); > > I think test should still be run on all platforms even though login would > fail. On other platforms than the test being run on, test shouldn't fail with > "UnsatisfiedLinkError" as per JDK-8039951. Now it will run in all platform other than the running one and the expected failure is LoginException. So any other types of Exception will be treated as failure including UnsatisfiedLinkError. - PR: https://git.openjdk.java.net/jdk/pull/7940
Re: RFR: 8281717: Cover logout method for several LoginModule [v2]
> Added coverage to logout method. Sibabrata Sahoo has updated the pull request incrementally with two additional commits since the last revision: - Update AllPlatforms.java - Update AllPlatforms.java - Changes: - all: https://git.openjdk.java.net/jdk/pull/7940/files - new: https://git.openjdk.java.net/jdk/pull/7940/files/7c43e30f..13a999b0 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=7940&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=7940&range=00-01 Stats: 31 lines in 1 file changed: 2 ins; 17 del; 12 mod Patch: https://git.openjdk.java.net/jdk/pull/7940.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7940/head:pull/7940 PR: https://git.openjdk.java.net/jdk/pull/7940
RFR: 8281717: Cover logout method for several LoginModule
Added coverage to logout method. - Commit messages: - 8281717: Cover logout method for several LoginModule Changes: https://git.openjdk.java.net/jdk/pull/7940/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=7940&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8281717 Stats: 49 lines in 1 file changed: 29 ins; 11 del; 9 mod Patch: https://git.openjdk.java.net/jdk/pull/7940.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7940/head:pull/7940 PR: https://git.openjdk.java.net/jdk/pull/7940
Integrated: 8282293: Domain value for system property jdk.https.negotiate.cbt should be case-insensitive
On Wed, 9 Mar 2022 15:03:31 GMT, Sibabrata Sahoo wrote: > Domain value for system property jdk.https.negotiate.cbt is case-insensitive > now. Included Test has been updated to address the change. This pull request has now been integrated. Changeset: 86015e15 Author:Sibabrata Sahoo URL: https://git.openjdk.java.net/jdk/commit/86015e15a5105a779ee065cca64479c8d4fbc074 Stats: 11 lines in 2 files changed: 8 ins; 0 del; 3 mod 8282293: Domain value for system property jdk.https.negotiate.cbt should be case-insensitive Reviewed-by: weijun, rhalade - PR: https://git.openjdk.java.net/jdk/pull/7759
Re: RFR: 8282293: Domain value for system property jdk.https.negotiate.cbt should be case-insensitive [v2]
On Tue, 15 Mar 2022 19:46:09 GMT, Weijun Wang wrote: >> Sibabrata Sahoo has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Update HttpsCB.java > > src/java.base/share/classes/sun/net/www/protocol/https/AbstractDelegateHttpsURLConnection.java > line 338: > >> 336: return true; >> 337: } >> 338: String afterWildCard = domain.substring(1); > > `domain` could be an empty string if the property value is "domain:a,,b". I > know it's invalid but at least let's try our best to avoid a runtime > exception. In fact, why is this variable necessary? It looks like > `regionMatches` allows you to compare ...er... regions. Done. - PR: https://git.openjdk.java.net/jdk/pull/7759
Re: RFR: 8282293: Domain value for system property jdk.https.negotiate.cbt should be case-insensitive [v3]
> Domain value for system property jdk.https.negotiate.cbt is case-insensitive > now. Included Test has been updated to address the change. Sibabrata Sahoo has updated the pull request incrementally with one additional commit since the last revision: Update AbstractDelegateHttpsURLConnection.java - Changes: - all: https://git.openjdk.java.net/jdk/pull/7759/files - new: https://git.openjdk.java.net/jdk/pull/7759/files/6d276479..9f8a2081 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=7759&range=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=7759&range=01-02 Stats: 2 lines in 1 file changed: 0 ins; 1 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/7759.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7759/head:pull/7759 PR: https://git.openjdk.java.net/jdk/pull/7759
Re: RFR: 8282293: Domain value for system property jdk.https.negotiate.cbt should be case-insensitive [v2]
> Domain value for system property jdk.https.negotiate.cbt is case-insensitive > now. Included Test has been updated to address the change. Sibabrata Sahoo has updated the pull request incrementally with one additional commit since the last revision: Update HttpsCB.java - Changes: - all: https://git.openjdk.java.net/jdk/pull/7759/files - new: https://git.openjdk.java.net/jdk/pull/7759/files/6899fe36..6d276479 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=7759&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=7759&range=00-01 Stats: 2 lines in 1 file changed: 2 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/7759.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7759/head:pull/7759 PR: https://git.openjdk.java.net/jdk/pull/7759
RFR: 8282293: Domain value for system property jdk.https.negotiate.cbt should be case-insensitive
Domain value for system property jdk.https.negotiate.cbt is case-insensitive now. Included Test has been updated to address the change. - Commit messages: - 8282293: Domain value for system property jdk.https.negotiate.cbt should be case-insensitive Changes: https://git.openjdk.java.net/jdk/pull/7759/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=7759&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8282293 Stats: 10 lines in 2 files changed: 7 ins; 0 del; 3 mod Patch: https://git.openjdk.java.net/jdk/pull/7759.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7759/head:pull/7759 PR: https://git.openjdk.java.net/jdk/pull/7759
Integrated: 8277353: java/security/MessageDigest/ThreadSafetyTest.java test times out
On Wed, 1 Dec 2021 06:26:58 GMT, Sibabrata Sahoo wrote: > This Test gets timeout during low cpu availability. It is modified to support > extended timeout period during JTREG execution. This pull request has now been integrated. Changeset: f22d157e Author:Sibabrata Sahoo URL: https://git.openjdk.java.net/jdk/commit/f22d157e551fb28991e7713a45e63a0a8d9d2c4c Stats: 4 lines in 1 file changed: 0 ins; 0 del; 4 mod 8277353: java/security/MessageDigest/ThreadSafetyTest.java test times out Reviewed-by: weijun - PR: https://git.openjdk.java.net/jdk/pull/6626
Re: RFR: 8277353: java/security/MessageDigest/ThreadSafetyTest.java test times out [v2]
On Fri, 3 Dec 2021 06:14:49 GMT, Sibabrata Sahoo wrote: >> This Test gets timeout during low cpu availability. It is modified to >> support extended timeout period during JTREG execution. > > Sibabrata Sahoo has updated the pull request incrementally with one > additional commit since the last revision: > > 8277353: java/security/MessageDigest/ThreadSafetyTest.java test times out Thanks Alexey, I have updated the threadsFactor=4 and duration=2. With this change the Test is completing ~50 seconds in my laptop which is 50% improvement over original. - PR: https://git.openjdk.java.net/jdk/pull/6626
Re: RFR: 8277353: java/security/MessageDigest/ThreadSafetyTest.java test times out [v2]
> This Test gets timeout during low cpu availability. It is modified to support > extended timeout period during JTREG execution. Sibabrata Sahoo has updated the pull request incrementally with one additional commit since the last revision: 8277353: java/security/MessageDigest/ThreadSafetyTest.java test times out - Changes: - all: https://git.openjdk.java.net/jdk/pull/6626/files - new: https://git.openjdk.java.net/jdk/pull/6626/files/325e351d..9a197651 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=6626&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=6626&range=00-01 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/6626.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6626/head:pull/6626 PR: https://git.openjdk.java.net/jdk/pull/6626
Re: RFR: 8277353: java/security/MessageDigest/ThreadSafetyTest.java test times out
On Wed, 1 Dec 2021 15:12:16 GMT, Weijun Wang wrote: > Can you lower the `threadsFactor` or `duration`? Or set an upper limit for > `nTasks`? I can reduce the threadFactor and duration to close to half(threadsFactor=2 and duration=2 Or hardcode nTasks=20) and i think there still will be enough threads to verify threadsafety. In that case default JTREG timeout period should be enough and no need for any additional timeout with @run tag. - PR: https://git.openjdk.java.net/jdk/pull/6626
RFR: 8277353: java/security/MessageDigest/ThreadSafetyTest.java test times out
This Test gets timeout during low cpu availability. It is modified to support extended timeout period during JTREG execution. - Commit messages: - 8277353: java/security/MessageDigest/ThreadSafetyTest.java test times out - 8277353: java/security/MessageDigest/ThreadSafetyTest.java test times out Changes: https://git.openjdk.java.net/jdk/pull/6626/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=6626&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8277353 Stats: 3 lines in 1 file changed: 0 ins; 0 del; 3 mod Patch: https://git.openjdk.java.net/jdk/pull/6626.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6626/head:pull/6626 PR: https://git.openjdk.java.net/jdk/pull/6626
Re: [jdk17] RFR: 8269276: Additional tests for MessageDigest with different providers [v2]
On Wed, 14 Jul 2021 17:15:14 GMT, Valerie Peng wrote: >> Sibabrata Sahoo has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Create ReinitDigest.java >> >> Provider name verification required. > > test/jdk/sun/security/pkcs11/MessageDigest/ReinitDigest.java line 105: > >> 103: throw new RuntimeException("Algorithm name should equal"); >> 104: } >> 105: if (md1.getProvider().equals(md2.getProvider())) { > > nit: calls getName() since the error message says that the comparison is > based on provider name. Updated. It was meant to be provider name check. Thanks.. - PR: https://git.openjdk.java.net/jdk17/pull/250
[jdk17] Integrated: 8269276: Additional tests for MessageDigest with different providers
On Wed, 14 Jul 2021 10:06:59 GMT, Sibabrata Sahoo wrote: > Few more Test cases added to verify MessageDigest instance generated through > different providers. This pull request has now been integrated. Changeset: a32d2eef Author:Sibabrata Sahoo URL: https://git.openjdk.java.net/jdk17/commit/a32d2eefea12771522b942b32985df0fe50119e8 Stats: 29 lines in 1 file changed: 14 ins; 2 del; 13 mod 8269276: Additional tests for MessageDigest with different providers Reviewed-by: valeriep, wetmore - PR: https://git.openjdk.java.net/jdk17/pull/250
Re: [jdk17] RFR: 8269276: Additional tests for MessageDigest with different providers [v2]
> Few more Test cases added to verify MessageDigest instance generated through > different providers. Sibabrata Sahoo has updated the pull request incrementally with one additional commit since the last revision: Create ReinitDigest.java Provider name verification required. - Changes: - all: https://git.openjdk.java.net/jdk17/pull/250/files - new: https://git.openjdk.java.net/jdk17/pull/250/files/fb37a6fd..427bb38f Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk17&pr=250&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk17&pr=250&range=00-01 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk17/pull/250.diff Fetch: git fetch https://git.openjdk.java.net/jdk17 pull/250/head:pull/250 PR: https://git.openjdk.java.net/jdk17/pull/250
[jdk17] RFR: 8269276: Additional tests for MessageDigest with different providers
Few more Test cases added to verify MessageDigest instance generated through different providers. - Commit messages: - 8269276: Additional tests for MessageDigest with different providers Changes: https://git.openjdk.java.net/jdk17/pull/250/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk17&pr=250&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8269276 Stats: 29 lines in 1 file changed: 14 ins; 2 del; 13 mod Patch: https://git.openjdk.java.net/jdk17/pull/250.diff Fetch: git fetch https://git.openjdk.java.net/jdk17 pull/250/head:pull/250 PR: https://git.openjdk.java.net/jdk17/pull/250
Re: RFR: 8266182: Create a manual test for jdk/sun/security/pkcs12/ParamsTest.java [v3]
On Fri, 18 Jun 2021 13:24:17 GMT, Abdul Kolarkunnu wrote: >> ParamsTest is an interop test between keytool <-> openssl. There are some >> manual steps listed in jdk/sun/security/pkcs12/params/README to perform >> after the execution of jtreg execution. So this test is to perform that >> manual steps. > > Abdul Kolarkunnu has updated the pull request incrementally with one > additional commit since the last revision: > > 8266182: Create a manual test for jdk/sun/security/pkcs12/ParamsTest.java Marked as reviewed by ssahoo (Committer). - PR: https://git.openjdk.java.net/jdk/pull/4413
Re: RFR: 8266182: Create a manual test for jdk/sun/security/pkcs12/ParamsTest.java [v2]
On Fri, 18 Jun 2021 05:43:15 GMT, Abdul Kolarkunnu wrote: >> ParamsTest is an interop test between keytool <-> openssl. There are some >> manual steps listed in jdk/sun/security/pkcs12/params/README to perform >> after the execution of jtreg execution. So this test is to perform that >> manual steps. > > Abdul Kolarkunnu has updated the pull request incrementally with one > additional commit since the last revision: > > 8266182: Create a manual test for jdk/sun/security/pkcs12/ParamsTest.java test/jdk/sun/security/pkcs12/KeytoolOpensslInterop.sh line 25: > 23: > 24: # Use OpenSSL 1.1.0i or above versions, earlier versions may generate > different > 25: # info and test fail. It will be more helpful, if some instruction given about finding openssl, any pre-post requirement and any specific way to validate the result after execution for this manual Test execution. test/jdk/sun/security/pkcs12/KeytoolOpensslInteropTest.java line 43: > 41: */ > 42: > 43: public class KeytoolOpensslInteropTest { This file is not necessary and the shell script can have @run tag instead. - PR: https://git.openjdk.java.net/jdk/pull/4413
Integrated: 8179880: Refactor javax/security shell tests to plain java tests
On Thu, 27 May 2021 08:52:20 GMT, Sibabrata Sahoo wrote: > This change converts JTREG shell Test scripts to Java equivalent. This pull request has now been integrated. Changeset: 7f55dc15 Author: Sibabrata Sahoo URL: https://git.openjdk.java.net/jdk/commit/7f55dc15769bbab59024aa49671bced633de40ed Stats: 99 lines in 2 files changed: 11 ins; 80 del; 8 mod 8179880: Refactor javax/security shell tests to plain java tests Reviewed-by: weijun - PR: https://git.openjdk.java.net/jdk/pull/4220
Re: RFR: 8180571: Refactor sun/security/pkcs11 shell tests to plain java tests and fix failures
On Tue, 18 May 2021 13:19:53 GMT, Fernando Guallini wrote: > Refactor the following shell tests to Java: > - security/pkcs11/KeyStore/Basic.sh > - security/pkcs11/KeyStore/ClientAuth.sh > - security/pkcs11/KeyStore/SecretKeysBasic.sh > - security/pkcs11/Provider/ConfigQuotedString.sh > - security/pkcs11/Provider/Login.sh > - security/pkcs11/Config/ReadConfInUTF16Env.sh > > Currently, most of the shell tests in the list may be ignored during > execution time in most platforms since they are incorrectly filtered out by > the OS name they are run on. For example, ClientAuth.sh is only run if the OS > name is equal to ‘Linux’, but OS name may also include the architecture such > as ‘Linux x86_64’. Those platform constraints are removed in this PR. > > Additionally, further changes are introduced in the following test: > > - ClientAuth: it was failing intermittently because the server side was > binding to the wildcard address. The issue is fixed by binding to loopback > address instead. Also, Thread.sleep is replaced with CountdownLatch to > facilitate synchronization between client and server. Finally, a new ‘user1’ > certificate was generated since the current one has expired. > > - Basic: Remove redundant X509Certificate casting > > - SecretKeysBasic: it was already in the problem list since it reproduces the > open bug JDK-8209398 and fails. Test is refactored to java and still > reproduces the issue. > > All the mentioned tests were run many times in multiple platforms to ensure > stability test/jdk/sun/security/pkcs11/Config/ReadConfInUTF16Env.java line 25: > 23: > 24: /* @test > 25: * @bug 8187023 Should it have the enhancement bug id too. - PR: https://git.openjdk.java.net/jdk/pull/4092
RFR: 8179880: Refactor javax/security shell tests to plain java tests
This change converts JTREG shell Test scripts to Java equivalent. - Commit messages: - 8179880: Refactor javax/security shell tests to plain java tests Changes: https://git.openjdk.java.net/jdk/pull/4220/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=4220&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8179880 Stats: 99 lines in 2 files changed: 11 ins; 80 del; 8 mod Patch: https://git.openjdk.java.net/jdk/pull/4220.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4220/head:pull/4220 PR: https://git.openjdk.java.net/jdk/pull/4220
Integrated: 8180568: Refactor javax/crypto shell tests to plain java tests
On Wed, 5 May 2021 10:00:22 GMT, Sibabrata Sahoo wrote: > This change is to remove the shell Test and convert to it's java equivalent. > That is the reason the shell Tests are deleted and the equivalent Java > implementation provided. > The purpose of the Test TestExemption.java is with limited crypto policy a > JCE provider can supplemented additional crypto permissions through > "cryptoPerms" file bundled inside the JCE signed jar. > It also need to be noticed that TestExemption.java has '@requires > java.runtime.name ~= "OpenJDK.*"'. It looks the intension of this Test > availability in open GIT is to run by any other JDK providers. This pull request has now been integrated. Changeset: 20ad4289 Author:Sibabrata Sahoo URL: https://git.openjdk.java.net/jdk/commit/20ad42897427483a4a62e1de7e78d4620eb9e240 Stats: 339 lines in 4 files changed: 138 ins; 183 del; 18 mod 8180568: Refactor javax/crypto shell tests to plain java tests Reviewed-by: wetmore - PR: https://git.openjdk.java.net/jdk/pull/3876
RFR: 8180568: Refactor javax/crypto shell tests to plain java tests
This change is to remove the shell Test and convert to it's java equivalent. That is the reason the shell Tests are deleted and the equivalent Java implementation provided. The purpose of the Test TestExemption.java is with limited crypto policy a JCE provider can supplemented additional crypto permissions through "cryptoPerms" file bundled inside the JCE signed jar. It also need to be noticed that TestExemption.java has '@requires java.runtime.name ~= "OpenJDK.*"'. It looks the intension of this Test availability in open GIT is to run by any other JDK providers. - Commit messages: - 8180568: Refactor javax/crypto shell tests to plain java tests Changes: https://git.openjdk.java.net/jdk/pull/3876/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=3876&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8180568 Stats: 339 lines in 4 files changed: 138 ins; 183 del; 18 mod Patch: https://git.openjdk.java.net/jdk/pull/3876.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3876/head:pull/3876 PR: https://git.openjdk.java.net/jdk/pull/3876
Integrated: 8185127: Add tests to cover hashCode() method for java supported crypto key types
On Wed, 14 Apr 2021 13:38:06 GMT, Sibabrata Sahoo wrote: > This is a simple Test to add few additional API coverage for all java > supported key types. The objective of this Test is to cover equals() and > hashcode() methods for each key types. This pull request has now been integrated. Changeset: 343a4a76 Author:Sibabrata Sahoo URL: https://git.openjdk.java.net/jdk/commit/343a4a76f273078f78897e8fb7e695bc2c111536 Stats: 149 lines in 1 file changed: 149 ins; 0 del; 0 mod 8185127: Add tests to cover hashCode() method for java supported crypto key types Reviewed-by: valeriep - PR: https://git.openjdk.java.net/jdk/pull/3490
Re: RFR: 8185127: Add tests to cover hashCode() method for java supported crypto key types [v9]
On Wed, 28 Apr 2021 06:56:21 GMT, Sibabrata Sahoo wrote: >> This is a simple Test to add few additional API coverage for all java >> supported key types. The objective of this Test is to cover equals() and >> hashcode() methods for each key types. > > Sibabrata Sahoo has updated the pull request incrementally with one > additional commit since the last revision: > > Typo error in comment section updated > > Typo error in comment section updated Pushing the change after addressing minor typo issue in the review comment. - PR: https://git.openjdk.java.net/jdk/pull/3490
Re: RFR: 8185127: Add tests to cover hashCode() method for java supported crypto key types [v9]
> This is a simple Test to add few additional API coverage for all java > supported key types. The objective of this Test is to cover equals() and > hashcode() methods for each key types. Sibabrata Sahoo has updated the pull request incrementally with one additional commit since the last revision: Typo error in comment section updated Typo error in comment section updated - Changes: - all: https://git.openjdk.java.net/jdk/pull/3490/files - new: https://git.openjdk.java.net/jdk/pull/3490/files/b990d100..09fe0dc5 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=3490&range=08 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=3490&range=07-08 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/3490.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3490/head:pull/3490 PR: https://git.openjdk.java.net/jdk/pull/3490
Re: RFR: 8185127: Add tests to cover hashCode() method for java supported crypto key types [v6]
On Wed, 21 Apr 2021 00:16:59 GMT, Valerie Peng wrote: >> Sibabrata Sahoo has updated the pull request incrementally with two >> additional commits since the last revision: >> >> - Update CompareKeys.java >> - Update CompareKeys.java > > test/jdk/javax/crypto/KeyGenerator/CompareKeys.java line 85: > >> 83: || origKey.hashCode() == copyKey.hashCode()) >> 84: && !Arrays.equals(origKey.getEncoded(), >> copyKey.getEncoded()) >> 85: && !origKey.getFormat().equals(copyKey.getFormat())) { > > So, all of these 3 checks have to fail in order to be considered key > inequality? Could you spell out clearly what is expected here? I am not sure > if this compound condition is correct. As it is now, the copy must have > different format(2nd condition) AND different encoding(3rd condition) AND > (not equals AND not same hash code)(1st condition), in order to trigger the > RuntimeException. I have updated the condition to check for equality, hashCode, Format, Encoded bytes of original key against another copy of same. As the keys are same and the copy of key is a replica of original key, i am expecting all condition to match in same time. If there is any condition check fails then it will be treated as a mismatch. Please correct me if i am wrong here. - PR: https://git.openjdk.java.net/jdk/pull/3490
Re: RFR: 8185127: Add tests to cover hashCode() method for java supported crypto key types [v8]
> This is a simple Test to add few additional API coverage for all java > supported key types. The objective of this Test is to cover equals() and > hashcode() methods for each key types. Sibabrata Sahoo has updated the pull request incrementally with one additional commit since the last revision: Update CompareKeys.java - Changes: - all: https://git.openjdk.java.net/jdk/pull/3490/files - new: https://git.openjdk.java.net/jdk/pull/3490/files/7e4033e1..b990d100 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=3490&range=07 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=3490&range=06-07 Stats: 4 lines in 1 file changed: 0 ins; 0 del; 4 mod Patch: https://git.openjdk.java.net/jdk/pull/3490.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3490/head:pull/3490 PR: https://git.openjdk.java.net/jdk/pull/3490
Re: RFR: 8185127: Add tests to cover hashCode() method for java supported crypto key types [v7]
> This is a simple Test to add few additional API coverage for all java > supported key types. The objective of this Test is to cover equals() and > hashcode() methods for each key types. Sibabrata Sahoo has updated the pull request incrementally with one additional commit since the last revision: Updated condition for check key equality Updated condition for check key equality - Changes: - all: https://git.openjdk.java.net/jdk/pull/3490/files - new: https://git.openjdk.java.net/jdk/pull/3490/files/cce15c0b..7e4033e1 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=3490&range=06 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=3490&range=05-06 Stats: 15 lines in 1 file changed: 4 ins; 2 del; 9 mod Patch: https://git.openjdk.java.net/jdk/pull/3490.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3490/head:pull/3490 PR: https://git.openjdk.java.net/jdk/pull/3490
Re: RFR: 8185127: Add tests to cover hashCode() method for java supported crypto key types [v6]
> This is a simple Test to add few additional API coverage for all java > supported key types. The objective of this Test is to cover equals() and > hashcode() methods for each key types. Sibabrata Sahoo has updated the pull request incrementally with two additional commits since the last revision: - Update CompareKeys.java - Update CompareKeys.java - Changes: - all: https://git.openjdk.java.net/jdk/pull/3490/files - new: https://git.openjdk.java.net/jdk/pull/3490/files/cb219824..cce15c0b Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=3490&range=05 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=3490&range=04-05 Stats: 5 lines in 1 file changed: 3 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/3490.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3490/head:pull/3490 PR: https://git.openjdk.java.net/jdk/pull/3490
Re: RFR: 8185127: Add tests to cover hashCode() method for java supported crypto key types [v5]
> This is a simple Test to add few additional API coverage for all java > supported key types. The objective of this Test is to cover equals() and > hashcode() methods for each key types. Sibabrata Sahoo has updated the pull request incrementally with one additional commit since the last revision: Updated Test to remove SunMSCAPI provider SunMSCAPI provider excluded from the supported algorithm provider list because neither the native generated keys are serializable nor PKCS8EncodedKeySpec compatible for certain algorithms like RSA. - Changes: - all: https://git.openjdk.java.net/jdk/pull/3490/files - new: https://git.openjdk.java.net/jdk/pull/3490/files/be5d9a62..cb219824 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=3490&range=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=3490&range=03-04 Stats: 41 lines in 1 file changed: 10 ins; 15 del; 16 mod Patch: https://git.openjdk.java.net/jdk/pull/3490.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3490/head:pull/3490 PR: https://git.openjdk.java.net/jdk/pull/3490
Re: RFR: 8185127: Add tests to cover hashCode() method for java supported crypto key types [v4]
On Fri, 16 Apr 2021 08:34:11 GMT, Sibabrata Sahoo wrote: >> This is a simple Test to add few additional API coverage for all java >> supported key types. The objective of this Test is to cover equals() and >> hashcode() methods for each key types. > > Sibabrata Sahoo has updated the pull request incrementally with one > additional commit since the last revision: > > Update CompareKeys.java I have removed processing any KeyGeneration algorithm starts with "SunTls" along with SunMSCAPI. I am getting issues with KeyGenerated through SunMSCAPI provider which are neither serializable nor compatible with PKCS8EncodedKeySpec for certain algorithms like RSA. - PR: https://git.openjdk.java.net/jdk/pull/3490
Withdrawn: 8185127: Add tests to cover hashCode() method for java supported crypto key types
On Wed, 14 Apr 2021 13:38:06 GMT, Sibabrata Sahoo wrote: > This is a simple Test to add few additional API coverage for all java > supported key types. The objective of this Test is to cover equals() and > hashcode() methods for each key types. This pull request has been closed without being integrated. - PR: https://git.openjdk.java.net/jdk/pull/3490
Re: RFR: 8185127: Add tests to cover hashCode() method for java supported crypto key types [v4]
On Fri, 16 Apr 2021 08:34:11 GMT, Sibabrata Sahoo wrote: >> This is a simple Test to add few additional API coverage for all java >> supported key types. The objective of this Test is to cover equals() and >> hashcode() methods for each key types. > > Sibabrata Sahoo has updated the pull request incrementally with one > additional commit since the last revision: > > Update CompareKeys.java Getting java.io.NotSerializableException: sun.security.mscapi.CKey$NativeHandles for MSCAPI RSA algorithm. Need to change the Test with alternate implementation. For now Withdrawing the pull request. - PR: https://git.openjdk.java.net/jdk/pull/3490
Re: RFR: 8185127: Add Tests to cover hashCode() method for java supported crypto key types. [v4]
> This is a simple Test to add few additional API coverage for all java > supported key types. The objective of this Test is to cover equals() and > hashcode() methods for each key types. Sibabrata Sahoo has updated the pull request incrementally with one additional commit since the last revision: Update CompareKeys.java - Changes: - all: https://git.openjdk.java.net/jdk/pull/3490/files - new: https://git.openjdk.java.net/jdk/pull/3490/files/1fc51e07..be5d9a62 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=3490&range=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=3490&range=02-03 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/3490.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3490/head:pull/3490 PR: https://git.openjdk.java.net/jdk/pull/3490
Re: RFR: 8185127: Add Tests to cover hashCode() method for java supported crypto key types. [v3]
> This is a simple Test to add few additional API coverage for all java > supported key types. The objective of this Test is to cover equals() and > hashcode() methods for each key types. Sibabrata Sahoo has updated the pull request incrementally with one additional commit since the last revision: Addressed Initial comments. Addressed Initial comments. - Changes: - all: https://git.openjdk.java.net/jdk/pull/3490/files - new: https://git.openjdk.java.net/jdk/pull/3490/files/ca459d0c..1fc51e07 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=3490&range=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=3490&range=01-02 Stats: 85 lines in 1 file changed: 30 ins; 34 del; 21 mod Patch: https://git.openjdk.java.net/jdk/pull/3490.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3490/head:pull/3490 PR: https://git.openjdk.java.net/jdk/pull/3490
Re: RFR: 8185127: Add Tests to cover hashCode() method for java supported crypto key types. [v2]
On Thu, 15 Apr 2021 18:32:26 GMT, Valerie Peng wrote: >> test/jdk/javax/crypto/KeyGenerator/CompareKeys.java line 73: >> >>> 71: && !Arrays.equals(origKey.getEncoded(), >>> copyKey.getEncoded()) >>> 72: && origKey.hashCode() != copyKey.hashCode()) { >>> 73: throw new RuntimeException("Key inequality found"); >> >> Check the format equality as well? > > Should be || instead of &&? Would also be nice to know which one(s) failed. Considering equality and hashCode check may not be same for all KeyTypes, i have relaxed the verification with || operator. Also added print statement for possible failure cause. Added verification for format too. - PR: https://git.openjdk.java.net/jdk/pull/3490
Re: RFR: 8185127: Add Tests to cover hashCode() method for java supported crypto key types. [v2]
On Thu, 15 Apr 2021 18:23:09 GMT, Valerie Peng wrote: >> Sibabrata Sahoo has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Comment changed. >> >> Comment changed. > > test/jdk/javax/crypto/KeyGenerator/CompareKeys.java line 114: > >> 112: HmacSHA384("HmacSHA384"), >> 113: HmacSHA512("HmacSHA512"), >> 114: RC2("RC2"); > > Just curious, how are these decided? Should this be an exhaustive list or > just enough sampling for code coverage? If this is meant to be a general > test, have you tried to not hardcoding the algorithm names to be enum? > Otherwise, new algorithms will not be tested if not updating this test. Done. Key(pair)Generator list will be collected dynamically. In fact it will now test even the same algorithm names supported by different providers too instead finding the 1st one. > test/jdk/javax/crypto/KeyGenerator/CompareKeys.java line 124: > >> 122: public SecretKey genSecretKey() throws Exception { >> 123: KeyGenerator kg = KeyGenerator.getInstance(this.algoName); >> 124: return kg.generateKey(); > > Would be informative to print out which provider is tested, i.e. where this > kg is from. Same goes for KeyPairGenerator. Added print statement for KeyGenerator and corresponding Provider list. - PR: https://git.openjdk.java.net/jdk/pull/3490
Re: RFR: 8255410: Add ChaCha20 and Poly1305 support to SunPKCS11 provider [v3]
On Wed, 14 Apr 2021 03:58:33 GMT, Valerie Peng wrote: >> Could someone (perhaps Jamil?) please help review this change? This enhances >> SunPKCS11 provider with ChaCha20-Poly1305 cipher and ChaCha20 key generation >> support. Majority of the regression tests are adapted from the existing ones >> for SunJCE provider's ChaCha20-Poly1305 cipher impl. When testing against >> NSS v3.57, it does not have support for ChaCha20 cipher, thus I did not add >> support for ChaCha20 cipher and the corresponding parameter. >> >> Thanks! >> Valerie > > Valerie Peng has updated the pull request incrementally with one additional > commit since the last revision: > > Fixed an tagLen issue, no key+iv reuse check for decryption, and add > regression test for ChaCha20 SKF. test/jdk/sun/security/pkcs11/Cipher/TestChaChaPoly.java line 1: > 1: /* There is no compatibility Test exist between SunJCE and SunPKCS11 providers. Do we need one here. - PR: https://git.openjdk.java.net/jdk/pull/3420
Re: RFR: 8265227: Move Proc.java from security/testlibrary to test/lib [v3]
On Wed, 14 Apr 2021 22:57:55 GMT, Weijun Wang wrote: >> I'd like to move this tool to test/lib inside a proper named package. > > Weijun Wang has updated the pull request incrementally with one additional > commit since the last revision: > > do not call internal method Marked as reviewed by ssahoo (Committer). - PR: https://git.openjdk.java.net/jdk/pull/3496
Re: RFR: 8185127: Add Tests to cover hashCode() method for java supported crypto key types. [v2]
> This is a simple Test to add few additional API coverage for all java > supported key types. The objective of this Test is to cover equals() and > hashcode() methods for each key types. Sibabrata Sahoo has updated the pull request incrementally with one additional commit since the last revision: Comment changed. Comment changed. - Changes: - all: https://git.openjdk.java.net/jdk/pull/3490/files - new: https://git.openjdk.java.net/jdk/pull/3490/files/ff6490d6..ca459d0c Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=3490&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=3490&range=00-01 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/3490.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3490/head:pull/3490 PR: https://git.openjdk.java.net/jdk/pull/3490
RFR: 8185127: Add Tests to cover hashCode() method for java supported crypto key types.
This is a simple Test to add few additional API coverage for all java supported key types. The objective of this Test is to cover equals() and hashcode() methods for each key types. - Commit messages: - 8185127: Add Tests to cover hashCode() method for java supported crypto key types. Changes: https://git.openjdk.java.net/jdk/pull/3490/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=3490&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8185127 Stats: 153 lines in 1 file changed: 153 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/3490.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3490/head:pull/3490 PR: https://git.openjdk.java.net/jdk/pull/3490
Integrated: 8225438: javax/net/ssl/TLSCommon/TestSessionLocalPrincipal.java failed with Read timed out
On Mon, 22 Mar 2021 10:45:34 GMT, Sibabrata Sahoo wrote: > The Test getting timeout intermittently because the SO_TIMEOUT of 5 seconds > set on sslServerSocket. This time interval could be inadequate when the > machine is too busy. Also it looks setting SO_TIMEOUT is unnecessary here. So > the statement has been removed. This pull request has now been integrated. Changeset: 036ae0ea Author: Sibabrata Sahoo URL: https://git.openjdk.java.net/jdk/commit/036ae0ea Stats: 3 lines in 1 file changed: 0 ins; 1 del; 2 mod 8225438: javax/net/ssl/TLSCommon/TestSessionLocalPrincipal.java failed with Read timed out Reviewed-by: xuelei, rhalade, hchao - PR: https://git.openjdk.java.net/jdk/pull/3116
Integrated: 8247895: SHA1PRNGReseed.java is calling setSeed(0)
On Mon, 22 Mar 2021 10:30:07 GMT, Sibabrata Sahoo wrote: > The Test is updated to use positive integer as seed for SecureRandom. This pull request has now been integrated. Changeset: 5a51d709 Author: Sibabrata Sahoo URL: https://git.openjdk.java.net/jdk/commit/5a51d709 Stats: 4 lines in 1 file changed: 0 ins; 0 del; 4 mod 8247895: SHA1PRNGReseed.java is calling setSeed(0) Reviewed-by: weijun, rhalade - PR: https://git.openjdk.java.net/jdk/pull/3114
RFR: 8225438: javax/net/ssl/TLSCommon/TestSessionLocalPrincipal.java failed with Read timed out
The Test getting timeout intermittently because the SO_TIMEOUT of 5 seconds set on sslServerSocket. This time interval could be inadequate when the machine is too busy. Also it looks setting SO_TIMEOUT is unnecessary here. So the statement has been removed. - Commit messages: - 8225438: javax/net/ssl/TLSCommon/TestSessionLocalPrincipal.java failed with Read timed out Changes: https://git.openjdk.java.net/jdk/pull/3116/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=3116&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8225438 Stats: 3 lines in 1 file changed: 0 ins; 1 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/3116.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3116/head:pull/3116 PR: https://git.openjdk.java.net/jdk/pull/3116
RFR: 8247895: SHA1PRNGReseed.java is calling setSeed(0)
The Test is updated to use positive integer as seed for SecureRandom. - Commit messages: - 8247895: SHA1PRNGReseed.java is calling setSeed(0) Changes: https://git.openjdk.java.net/jdk/pull/3114/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=3114&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8247895 Stats: 4 lines in 1 file changed: 0 ins; 0 del; 4 mod Patch: https://git.openjdk.java.net/jdk/pull/3114.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3114/head:pull/3114 PR: https://git.openjdk.java.net/jdk/pull/3114
[13]RFR:8224650:Add tests to support X25519 and X448 in TLS
Hi Xuelei/Brad, Please review the patch for, JBS: https://bugs.openjdk.java.net/browse/JDK-8224650 Webrev: http://cr.openjdk.java.net/~ssahoo/8224650/webrev.00/ This is a small Test inherited from "SSLSocketTemplate" and reuse most part of it. The only difference is, it uses supported named groups along with a fixed set of ciphers supported with different TLS protocols. Though there are large number of supported ciphers but I have selected few to ensure the Test does not take much time to complete the execution. Please let me know if you have any suggestion for improvement. Thanks, Siba
[12] RFR: 8211787: javax/net/ssl/TLSCommon/TLSTest.java throws java.net.SocketTimeoutException: Read timed out
Hi Xuelei, Please review a minor fix for, JBS: https://bugs.openjdk.java.net/browse/JDK-8211787 Webrev: http://cr.openjdk.java.net/~ssahoo/8211787/webrev.00/ The original intention to "setSoTimeout()" is on serverSocket where the server will fail with timeout if it exceed certain amount of time before accepting a client connection. But by mistake it was set on wrong socket object which is created from accept() and causing a timeout if the InputStream exceed certain amount of time. I have corrected to setSoTimeout() to be set through serverSocket only. Thanks, Siba
RE: [11] RFR: 8208496: New Test to verify concurrent behavior for TLS.
serverSocket.close() and serverSocket.accept() are not declared synchronized in java/net/ServerSocket.java. Thanks, Siba -Original Message- From: Xuelei Fan Sent: Friday, August 03, 2018 10:38 PM To: Sibabrata Sahoo ; John Jiang Cc: security-dev@openjdk.java.net Subject: Re: [11] RFR: 8208496: New Test to verify concurrent behavior for TLS. Hi Siba, The use of SO_TIMEOUT may cause intermittent failure in JDK test environment. I'm not very sure if serverSocket.close() and serverSocket.accept() are synchronized or not. If they did, there may be a dead waiting condition. Xuelei On 8/3/2018 2:32 AM, Sibabrata Sahoo wrote: > Hi Xuelei/John, > > I did a minor change to handle graceful shutdown of server thread > along with handling timeout for long waiting accept(). > > Webrev: http://cr.openjdk.java.net/~ssahoo/8208496/webrev.01/ > <http://cr.openjdk.java.net/%7Essahoo/8208496/webrev.01/> > > Thanks, > > Siba > > *From:*John Jiang > *Sent:* Friday, August 03, 2018 7:21 AM > *To:* Sibabrata Sahoo > *Cc:* Xue-Lei Fan ; > security-dev@openjdk.java.net > *Subject:* Re: [11] RFR: 8208496: New Test to verify concurrent > behavior for TLS. > > Hi Siba, > Would it be better to check how many connections the server accepts? > In your case, the server must accept 50 (no more no less) connections; > otherwise, some problem may raise. > > And I suppose, when the server thread is interrupted, the server > socket may not be closed. > The server should exit immediately and gracefully when it has accepted > all the connections. > If the server can be closed gracefully, it may be no need to set the > server thread as daemon. > > Some minors: > -- 28 import java.net.SocketException; This import statement looks > unused. > > -- 131 sslSocket.setNeedClientAuth(false); > Now that client auth is not requested by default, so it may be > unnecessary to set false value explicitly. > > Best regards, > John Jiang > > On 2018/8/2 18:41, Sibabrata Sahoo wrote: > > Hi Xuelei, > > Please review the patch for, > > JBS: https://bugs.openjdk.java.net/browse/JDK-8208496 > > Webrev: http://cr.openjdk.java.net/~ssahoo/8208496/webrev.00/ > <http://cr.openjdk.java.net/%7Essahoo/8208496/webrev.00/> > > This is a new Test which test concurrent behavior of TLS. It uses 50 > client thread to access a single server port concurrently and repeat > this process for each protocol supported. > > Thanks, > > Siba >
RE: [11] RFR: 8208496: New Test to verify concurrent behavior for TLS.
Hi Xuelei/John, I did a minor change to handle graceful shutdown of server thread along with handling timeout for long waiting accept(). Webrev: http://cr.openjdk.java.net/~ssahoo/8208496/webrev.01/ Thanks, Siba From: John Jiang Sent: Friday, August 03, 2018 7:21 AM To: Sibabrata Sahoo Cc: Xue-Lei Fan ; security-dev@openjdk.java.net Subject: Re: [11] RFR: 8208496: New Test to verify concurrent behavior for TLS. Hi Siba, Would it be better to check how many connections the server accepts? In your case, the server must accept 50 (no more no less) connections; otherwise, some problem may raise. And I suppose, when the server thread is interrupted, the server socket may not be closed. The server should exit immediately and gracefully when it has accepted all the connections. If the server can be closed gracefully, it may be no need to set the server thread as daemon. Some minors: -- 28 import java.net.SocketException; This import statement looks unused. -- 131 sslSocket.setNeedClientAuth(false); Now that client auth is not requested by default, so it may be unnecessary to set false value explicitly. Best regards, John Jiang On 2018/8/2 18:41, Sibabrata Sahoo wrote: Hi Xuelei, Please review the patch for, JBS: https://bugs.openjdk.java.net/browse/JDK-8208496 Webrev: HYPERLINK "http://cr.openjdk.java.net/%7Essahoo/8208496/webrev.00/"http://cr.openjdk.java.net/~ssahoo/8208496/webrev.00/ This is a new Test which test concurrent behavior of TLS. It uses 50 client thread to access a single server port concurrently and repeat this process for each protocol supported. Thanks, Siba
[11] RFR: 8208496: New Test to verify concurrent behavior for TLS.
Hi Xuelei, Please review the patch for, JBS: https://bugs.openjdk.java.net/browse/JDK-8208496 Webrev: http://cr.openjdk.java.net/~ssahoo/8208496/webrev.00/ This is a new Test which test concurrent behavior of TLS. It uses 50 client thread to access a single server port concurrently and repeat this process for each protocol supported. Thanks, Siba
[11] RFR: 8206355: SSLSessionImpl.getLocalPrincipal() throws NPE
Hi, Please review the change for the following patch, JBS: https://bugs.openjdk.java.net/browse/JDK-8206355 Webrev: http://cr.openjdk.java.net/~ssahoo/8206355/webrev.00/ It fixes SSLSessionImpl.getLocalPrincipal() implementation when client side authentication is not enabled. Thanks, Siba.
RE: [11] RFR: 8205653: test/jdk/sun/management/jmxremote/bootstrap/RmiRegistrySslTest.java and RmiSslBootstrapTest.sh fail with handshake_failure
May I get the approval from serviceability-...@openjdk.java.net. Thanks, Siba -Original Message- From: Xuelei Fan Sent: Thursday, June 28, 2018 9:27 PM To: Daniel Fuchs ; Sibabrata Sahoo ; jmx-...@openjdk.java.net; security-dev@openjdk.java.net; serviceability-...@openjdk.java.net serviceability-...@openjdk.java.net Subject: Re: [11] RFR: 8205653: test/jdk/sun/management/jmxremote/bootstrap/RmiRegistrySslTest.java and RmiSslBootstrapTest.sh fail with handshake_failure Hi Siba, The change looks fine to me. I would like Serviceability review this change as well. Thanks, Xuelei On 6/28/2018 8:46 AM, Daniel Fuchs wrote: > [ccing serviceability-...@openjdk.java.net] > > Hi Siba, > > This looks good to me - but I'm not a SSL expert. > It would be good to get someone from the security team eyeball those > changes (Xuelei? Brad?) > > I added serviceability-...@openjdk.java.net in cc as this is where > reviews for JMX/Monitoring changes happen these days... > > best regards, > > -- daniel > > On 28/06/2018 17:10, Sibabrata Sahoo wrote: >> Hi, >> >> Please review the patch for, >> >> JBS: https://bugs.openjdk.java.net/browse/JDK-8205653 >> >> Webrev: http://cr.openjdk.java.net/~ssahoo/8205653/webrev.00/ >> >> Change: >> >> The Test has been upgraded to address the following 2 cases, >> >> 1. Add protocol support for TLSv1.3. The change is done in the >> config >> file named "management_ssltest11_ok.properties.in". >> 2. Add support for legacy TLS. Now a new config file >> "management_ssltest15_ok.properties.in" hold TLS protocol >> "TLS_RSA_WITH_AES_128_CBC_SHA,TLS_RSA_WITH_AES_256_CBC_SHA" >> instead >> of "SSL_RSA_WITH_RC4_128_SHA,SSL_RSA_WITH_RC4_128_MD5". >> >> Previously the Test was using DSA keys which is not compatible with >> TLSv1.3. So the keys has been upgraded to use RSA(2048 bit). Hence >> the instruction in "Readme.txt" changed which generates RSA(2048 bit) keys. >> >> NOTE: Few Test was problem listed which are removed from the list now. >> Mach 5 result PASS with multiple try for all 14 Test belongs to >> "open/test/jdk/sun/management/jmxremote" folder. >> >> Thanks, >> >> Siba >> >
RE: [11] RFR: 8205653: test/jdk/sun/management/jmxremote/bootstrap/RmiRegistrySslTest.java and RmiSslBootstrapTest.sh fail with handshake_failure
Resending the email after subscribing to HYPERLINK "mailto:jmx-...@openjdk.java.net"jmx-...@openjdk.java.net Thanks, Siba From: Sibabrata Sahoo Sent: Thursday, June 28, 2018 8:40 PM To: HYPERLINK "mailto:jmx-...@openjdk.java.net"jmx-...@openjdk.java.net; HYPERLINK "mailto:security-dev@openjdk.java.net"security-dev@openjdk.java.net Subject: [11] RFR: 8205653: test/jdk/sun/management/jmxremote/bootstrap/RmiRegistrySslTest.java and RmiSslBootstrapTest.sh fail with handshake_failure Hi, Please review the patch for, JBS: https://bugs.openjdk.java.net/browse/JDK-8205653 Webrev: http://cr.openjdk.java.net/~ssahoo/8205653/webrev.00/ Change: The Test has been upgraded to address the following 2 cases, Add protocol support for TLSv1.3. The change is done in the config file named "management_ssltest11_ok.properties.in". Add support for legacy TLS. Now a new config file "management_ssltest15_ok.properties.in" hold TLS protocol "TLS_RSA_WITH_AES_128_CBC_SHA,TLS_RSA_WITH_AES_256_CBC_SHA" instead of "SSL_RSA_WITH_RC4_128_SHA,SSL_RSA_WITH_RC4_128_MD5". Previously the Test was using DSA keys which is not compatible with TLSv1.3. So the keys has been upgraded to use RSA(2048 bit). Hence the instruction in "Readme.txt" changed which generates RSA(2048 bit) keys. NOTE: Few Test was problem listed which are removed from the list now. Mach 5 result PASS with multiple try for all 14 Test belongs to "open/test/jdk/sun/management/jmxremote" folder. Thanks, Siba
RE: [11] RFR: 8205653: test/jdk/sun/management/jmxremote/bootstrap/RmiRegistrySslTest.java and RmiSslBootstrapTest.sh fail with handshake_failure
Hi Daniel, I am not a member of HYPERLINK "mailto:jmx-...@openjdk.java.net"jmx-...@openjdk.java.net. So the review posted is waiting for moderator's approval. Thanks, Siba From: Sibabrata Sahoo Sent: Thursday, June 28, 2018 8:40 PM To: jmx-...@openjdk.java.net; security-dev@openjdk.java.net Subject: [11] RFR: 8205653: test/jdk/sun/management/jmxremote/bootstrap/RmiRegistrySslTest.java and RmiSslBootstrapTest.sh fail with handshake_failure Hi, Please review the patch for, JBS: https://bugs.openjdk.java.net/browse/JDK-8205653 Webrev: http://cr.openjdk.java.net/~ssahoo/8205653/webrev.00/ Change: The Test has been upgraded to address the following 2 cases, Add protocol support for TLSv1.3. The change is done in the config file named "management_ssltest11_ok.properties.in". Add support for legacy TLS. Now a new config file "management_ssltest15_ok.properties.in" hold TLS protocol "TLS_RSA_WITH_AES_128_CBC_SHA,TLS_RSA_WITH_AES_256_CBC_SHA" instead of "SSL_RSA_WITH_RC4_128_SHA,SSL_RSA_WITH_RC4_128_MD5". Previously the Test was using DSA keys which is not compatible with TLSv1.3. So the keys has been upgraded to use RSA(2048 bit). Hence the instruction in "Readme.txt" changed which generates RSA(2048 bit) keys. NOTE: Few Test was problem listed which are removed from the list now. Mach 5 result PASS with multiple try for all 14 Test belongs to "open/test/jdk/sun/management/jmxremote" folder. Thanks, Siba
[11] RFR: 8205653: test/jdk/sun/management/jmxremote/bootstrap/RmiRegistrySslTest.java and RmiSslBootstrapTest.sh fail with handshake_failure
Hi, Please review the patch for, JBS: https://bugs.openjdk.java.net/browse/JDK-8205653 Webrev: http://cr.openjdk.java.net/~ssahoo/8205653/webrev.00/ Change: The Test has been upgraded to address the following 2 cases, Add protocol support for TLSv1.3. The change is done in the config file named "management_ssltest11_ok.properties.in". Add support for legacy TLS. Now a new config file "management_ssltest15_ok.properties.in" hold TLS protocol "TLS_RSA_WITH_AES_128_CBC_SHA,TLS_RSA_WITH_AES_256_CBC_SHA" instead of "SSL_RSA_WITH_RC4_128_SHA,SSL_RSA_WITH_RC4_128_MD5". Previously the Test was using DSA keys which is not compatible with TLSv1.3. So the keys has been upgraded to use RSA(2048 bit). Hence the instruction in "Readme.txt" changed which generates RSA(2048 bit) keys. NOTE: Few Test was problem listed which are removed from the list now. Mach 5 result PASS with multiple try for all 14 Test belongs to "open/test/jdk/sun/management/jmxremote" folder. Thanks, Siba
RE: [11] RFR: JDK-8205111: Develop new Test to verify different key types for supported TLS protocols.
Hi Xuelei, Please review the updated webrev: http://cr.openjdk.java.net/~ssahoo/8205111/webrev.01/ - Now rsa_pss_pss* uses " DHE or ECDHE_RSA " ciphers for TLSv1.2 which is working fine now. - Additional code added for " read/write " after re-handshake. John, - PKCS12 used instead of JKS. - Comment section for private key updated. - try with resource used for socket. - "clientRenegoReady" variable is actually used and updated. Please check the Client section too. It is used for re-handshake completion. Yes it Is working as expected. - Multiple @run added to have the flexibility to change the parameter(Cipher) which are not in order(shuffled). Thanks, Siba -Original Message- From: Xuelei Fan Sent: Thursday, June 21, 2018 7:28 PM To: Sibabrata Sahoo ; security-dev@openjdk.java.net Subject: Re: [11] RFR: JDK-8205111: Develop new Test to verify different key types for supported TLS protocols. Note that rsa_pss_pss cannot work with TLS_RSA_WITH cipher suites, as this algorithm is limited to signature whiel TLS_RSA cipher suites need key encipherment. In lines 135-156, you can replace the TLS_RSA cipher suite with DHE or ECDHE_RSA. For the re-handshake part, please read/write something after the call to startHandshake() in each side. Otherwise, the key-update and session resumption may not complete before socket close. Otherwise, looks fine to me. Thanks, Xuelei On 6/20/2018 11:58 PM, Sibabrata Sahoo wrote: > Hi Xuelei, > > Please review the patch for, > > JBS: https://bugs.openjdk.java.net/browse/JDK-8205111 > > Webrev: http://cr.openjdk.java.net/~ssahoo/8205111/webrev.00/ > > Change: > > This Test file verifies all TLS protocols with the supported keytypes. > > Thanks, > > Siba >
[11] RFR: JDK-8205111: Develop new Test to verify different key types for supported TLS protocols.
Hi Xuelei, Please review the patch for, JBS: https://bugs.openjdk.java.net/browse/JDK-8205111 Webrev: http://cr.openjdk.java.net/~ssahoo/8205111/webrev.00/ Change: This Test file verifies all TLS protocols with the supported keytypes. Thanks, Siba
RE: RFR: 8200219: Develop new tests for using new elliptic curves: curve25519 and curve448
Hi Sean/Adam, Please review the updated patch, Webrev: http://cr.openjdk.java.net/~ssahoo/8184359/webrev.02/ Now there is only 1 Test file in the deleted list which has duplicate code. Due to that I have pointed the older JBS bug ID JDK-4936763 in KeyAgreementTest.java and linked the same to JDK-8184359 too. Reverted the duplicate code merge in KeySizeTest.java due to performance issue observed for higher key sizes. That ensured SupportedDHKeys.java & UnsupportedDHKeys.java will not be removed as it was provided in previous patch. Thanks, Siba -Original Message- From: Sean Mullan Sent: Wednesday, April 11, 2018 1:42 AM To: Sibabrata Sahoo ; Adam Petcher ; security-dev@openjdk.java.net Subject: Re: RFR: 8200219: Develop new tests for using new elliptic curves: curve25519 and curve448 On 4/9/18 10:13 AM, Sibabrata Sahoo wrote: > Here is the new patch addressing all comments. > Webrev:http://cr.openjdk.java.net/~ssahoo/8184359/webrev.01/ > > Changes includes: > 1) > KeyAgreementTest.java > KeySizeTest.java >I have ensured no duplicate Test cases exist for the functionality > provided in these 2 files. Due to that I have remove 3 files, > DHGenSecretKey.java, SupportedDHKeys.java, UnsupportedDHKeys.java from > "open/test/jdk/com/sun/crypto/provider/KeyAgreement" folder. For DHGenSecretKey, can you add the bugid of 4936763 to the test file in this webrev where the changes were merged and also link the bugs together in JBS? That way we know that these tests are now covering the fixes for that bug. For the others, see below. > Statements which affected due to merging the missing code from these deleted > files are KeyAgreementTest.java[29, 141], KeySizeTest.java[91-100, 112-162]. > Also I have observed that KeySizeTest.java Test takes around 2 minute to > complete in SOME PLATFORM to complete all 14 @run and approximately 20 > seconds for higher keys > 4096 for "DiffieHellman" only after merge. Please > suggest if removing the higher Keys from the Test cases will help or 20-30 > seconds for these higher keys is accepted here? It's a bit concerning. One way to make the tests run faster is to split them up into separate tests so that they can be run concurrently by jtreg. So we might be making things worse by merging them :( So I would suggest just merging DHGenSecretKey and leaving the other tests as-is. > 2) I have updated the comment in KeyAgreementTest.java[129] to indicate > provider search order. Ok. --Sean > As per Adam's comments the following changes, > 3) KeySizeTest.java & NegativeTest.java, which need utility methods from > Convert.java are using the library instead of defining it's own. > 4) MultiThreadTest.java generates two key pairs and do two key agreement > operations.
RE: RFR: 8200219: Develop new tests for using new elliptic curves: curve25519 and curve448
Hi Sean/Adam, Here is the new patch addressing all comments. Webrev: http://cr.openjdk.java.net/~ssahoo/8184359/webrev.01/ Changes includes: 1) KeyAgreementTest.java KeySizeTest.java I have ensured no duplicate Test cases exist for the functionality provided in these 2 files. Due to that I have remove 3 files, DHGenSecretKey.java, SupportedDHKeys.java, UnsupportedDHKeys.java from "open/test/jdk/com/sun/crypto/provider/KeyAgreement" folder. Statements which affected due to merging the missing code from these deleted files are KeyAgreementTest.java[29, 141], KeySizeTest.java[91-100, 112-162]. Also I have observed that KeySizeTest.java Test takes around 2 minute to complete in SOME PLATFORM to complete all 14 @run and approximately 20 seconds for higher keys > 4096 for "DiffieHellman" only after merge. Please suggest if removing the higher Keys from the Test cases will help or 20-30 seconds for these higher keys is accepted here? 2) I have updated the comment in KeyAgreementTest.java[129] to indicate provider search order. As per Adam's comments the following changes, 3) KeySizeTest.java & NegativeTest.java, which need utility methods from Convert.java are using the library instead of defining it's own. 4) MultiThreadTest.java generates two key pairs and do two key agreement operations. Thanks, Siba -Original Message- From: Sean Mullan Sent: Thursday, April 05, 2018 8:46 PM To: Sibabrata Sahoo ; Adam Petcher ; security-dev@openjdk.java.net Subject: Re: RFR: 8200219: Develop new tests for using new elliptic curves: curve25519 and curve448 Comments below ... On 4/2/18 4:07 AM, Sibabrata Sahoo wrote: > Hi Sean, > > My comments In-lined.. > > Thanks, > Siba > > -Original Message- > From: Sean Mullan > Sent: Saturday, March 31, 2018 12:13 AM > To: Sibabrata Sahoo ; Adam Petcher > ; security-dev@openjdk.java.net > Subject: Re: RFR: 8200219: Develop new tests for using new elliptic > curves: curve25519 and curve448 > > A few comments so far; have not finished my review yet. > > General comment: > > Many of these tests test more than XDH. That is fine and good for increasing > coverage, but have you looked through existing tests to see if you are > duplicating anything we are already testing and maybe those tests could be > removed or you could share the same code. One of the things we should be > looking at is to figure out how to reduce the overall time the security tests > take. > > There are few Lines of code related to " DiffieHellman " are duplicate in > KeyAgreementTest.java and KeySizeTest.java which are available in 2 existing > Test folders. i.e. "open\test\jdk\sun\security\pkcs11\KeyAgreement" and > "open\test\jdk\com\sun\crypto\provider\KeyAgreement". While there is no > equivalent Tests for the same for "ECDH" and "XDH". The remaining files > available in the webrev are mostly new. Our initial thinking was to have Test > files covering all similar algorithms in one place. In that case I may remove > 2-3 existing Test files inside these folders with next patch. Ok. > * KeyAgreementTest.java > > 128 // Uses platform supported provider to test interoperability. > > What do you mean by "platform supported provider"? Isn't this based on the > provider search order? So in some cases, you might be testing against the > same provider and not really doing interop testing? > > Yes my thinking here is Provider search order. I may need to change the > comment appropriately. Here the intention is not really interop based Test > unless a different provider found other than SunJCE. Ok. > > * KeySizeTest.java > > You are generating some large keys - any issues with test timeouts? Do we > need to test the generation of the keypairs? Could we use cached keypairs > instead? > > Yes here my intention is to test generation of keypairs with different key > sizes too. I have ran these Test files several times. I have not seen these > test files taking more times than few couple of seconds. Ok. I looked at the other tests and have no further comments. Thanks, Sean > On 3/26/18 12:38 PM, Sibabrata Sahoo wrote: >> Hi, >> >> Please review the patch for, >> >> JBS: https://bugs.openjdk.java.net/browse/JDK-8184359 >> >> Webrev: http://cr.openjdk.java.net/~ssahoo/8184359/webrev.00/ >> >> All the Test files uses KeyAgreement, KeyPairGenerator, Several >> KeySpecs from SunJCE library to Test DiffieHellman, ECDH and XDH with >> curve25519 and curve448 algorithms. Each Test files try to address >> several cases and the purpose of each has been commented in their own files. >> >> Thanks, >> >> Siba >>
RE: RFR: 8200219: Develop new tests for using new elliptic curves: curve25519 and curve448
Hi Sean, My comments In-lined.. Thanks, Siba -Original Message- From: Sean Mullan Sent: Saturday, March 31, 2018 12:13 AM To: Sibabrata Sahoo ; Adam Petcher ; security-dev@openjdk.java.net Subject: Re: RFR: 8200219: Develop new tests for using new elliptic curves: curve25519 and curve448 A few comments so far; have not finished my review yet. General comment: Many of these tests test more than XDH. That is fine and good for increasing coverage, but have you looked through existing tests to see if you are duplicating anything we are already testing and maybe those tests could be removed or you could share the same code. One of the things we should be looking at is to figure out how to reduce the overall time the security tests take. There are few Lines of code related to " DiffieHellman " are duplicate in KeyAgreementTest.java and KeySizeTest.java which are available in 2 existing Test folders. i.e. "open\test\jdk\sun\security\pkcs11\KeyAgreement" and "open\test\jdk\com\sun\crypto\provider\KeyAgreement". While there is no equivalent Tests for the same for "ECDH" and "XDH". The remaining files available in the webrev are mostly new. Our initial thinking was to have Test files covering all similar algorithms in one place. In that case I may remove 2-3 existing Test files inside these folders with next patch. * KeyAgreementTest.java 128 // Uses platform supported provider to test interoperability. What do you mean by "platform supported provider"? Isn't this based on the provider search order? So in some cases, you might be testing against the same provider and not really doing interop testing? Yes my thinking here is Provider search order. I may need to change the comment appropriately. Here the intention is not really interop based Test unless a different provider found other than SunJCE. * KeySizeTest.java You are generating some large keys - any issues with test timeouts? Do we need to test the generation of the keypairs? Could we use cached keypairs instead? Yes here my intention is to test generation of keypairs with different key sizes too. I have ran these Test files several times. I have not seen these test files taking more times than few couple of seconds. --Sean On 3/26/18 12:38 PM, Sibabrata Sahoo wrote: > Hi, > > Please review the patch for, > > JBS: https://bugs.openjdk.java.net/browse/JDK-8184359 > > Webrev: http://cr.openjdk.java.net/~ssahoo/8184359/webrev.00/ > > All the Test files uses KeyAgreement, KeyPairGenerator, Several > KeySpecs from SunJCE library to Test DiffieHellman, ECDH and XDH with > curve25519 and curve448 algorithms. Each Test files try to address > several cases and the purpose of each has been commented in their own files. > > Thanks, > > Siba >
RFR: 8200219: Develop new tests for using new elliptic curves: curve25519 and curve448
Hi, Please review the patch for, JBS: https://bugs.openjdk.java.net/browse/JDK-8184359 Webrev: http://cr.openjdk.java.net/~ssahoo/8184359/webrev.00/ All the Test files uses KeyAgreement, KeyPairGenerator, Several KeySpecs from SunJCE library to Test DiffieHellman, ECDH and XDH with curve25519 and curve448 algorithms. Each Test files try to address several cases and the purpose of each has been commented in their own files. Thanks, Siba
RE: [11] RFR: 8194486: Several krb5 tests failed in Mac.
Hi Max, Here is an updated patch: http://cr.openjdk.java.net/~ssahoo/8194486/webrev.01/ I found there are few more Test files which need the similar change as applicable to all other Test files in "sun/security/krb5/auto" folder. The change is applied to all Test files except the following 2 Test files inside the same folder. ReplayCacheExpunge.java ReplayCachePrecise.java Thanks, Siba -Original Message- From: Weijun Wang Sent: Thursday, January 11, 2018 11:33 AM To: Sibabrata Sahoo Cc: security-dev@openjdk.java.net Subject: Re: [11] RFR: 8194486: Several krb5 tests failed in Mac. > On Jan 11, 2018, at 1:30 PM, Sibabrata Sahoo > wrote: > > I will remove the commented lines. I think, it should be fine, if I do not > submit the webrev again. Not necessary at all. --Max > > Thanks, > Siba > > -Original Message- > From: Weijun Wang > Sent: Wednesday, January 10, 2018 9:07 PM > To: Sibabrata Sahoo > Cc: security-dev@openjdk.java.net > Subject: Re: [11] RFR: 8194486: Several krb5 tests failed in Mac. > > Code change looks fine. I think you can safely remove the lines commented > out. If you meant to keep the history, the Mercurial repo already had it. > > Thanks > Max > >> On Jan 10, 2018, at 8:58 PM, Sibabrata Sahoo >> wrote: >> >> Hi, >> >> Please review the change for the following patch, >> >> JBS: https://bugs.openjdk.java.net/browse/JDK-8194486 >> Webrev: http://cr.openjdk.java.net/~ssahoo/8194486/webrev.00/ >> >> Description: >> There are several failure occurred recently because the name service used by >> these Tests were not determinable. This patch fixes those issues. >> >> >> Thanks, >> Siba >
RE: [11] RFR: 8194486: Several krb5 tests failed in Mac.
I will remove the commented lines. I think, it should be fine, if I do not submit the webrev again. Thanks, Siba -Original Message- From: Weijun Wang Sent: Wednesday, January 10, 2018 9:07 PM To: Sibabrata Sahoo Cc: security-dev@openjdk.java.net Subject: Re: [11] RFR: 8194486: Several krb5 tests failed in Mac. Code change looks fine. I think you can safely remove the lines commented out. If you meant to keep the history, the Mercurial repo already had it. Thanks Max > On Jan 10, 2018, at 8:58 PM, Sibabrata Sahoo > wrote: > > Hi, > > Please review the change for the following patch, > > JBS: https://bugs.openjdk.java.net/browse/JDK-8194486 > Webrev: http://cr.openjdk.java.net/~ssahoo/8194486/webrev.00/ > > Description: > There are several failure occurred recently because the name service used by > these Tests were not determinable. This patch fixes those issues. > > > Thanks, > Siba
[11] RFR: 8194486: Several krb5 tests failed in Mac.
Hi, Please review the change for the following patch, JBS: https://bugs.openjdk.java.net/browse/JDK-8194486 Webrev: http://cr.openjdk.java.net/~ssahoo/8194486/webrev.00/ Description: There are several failure occurred recently because the name service used by these Tests were not determinable. This patch fixes those issues. Thanks, Siba
RE: [10] RFR: JDK-8183310: java/security/modules/ModularTest.java should clean up better
Hi Max, Please find the updated webrev: http://cr.openjdk.java.net/~ssahoo/8183310/webrev.02/ The setup will be called only once irrespective number of @run available inside the Test file. Thanks, Siba -Original Message- From: Weijun Wang Sent: Wednesday, August 23, 2017 3:04 PM To: Sibabrata Sahoo Cc: Valerie Peng; security-dev@openjdk.java.net Subject: Re: [10] RFR: JDK-8183310: java/security/modules/ModularTest.java should clean up better Then maybe you can also make setup() a separate @run. --Max > On Aug 23, 2017, at 4:17 PM, Sibabrata Sahoo > wrote: > > Hi Max, > > I have splitted the @run to convert each run to consume minimum time to avoid > any timeout for rare cases. > > Thanks, > Siba > > -Original Message- > From: Weijun Wang > Sent: Wednesday, August 23, 2017 1:43 PM > To: Sibabrata Sahoo > Cc: Valerie Peng; security-dev@openjdk.java.net > Subject: Re: [10] RFR: JDK-8183310: java/security/modules/ModularTest.java > should clean up better > > Is there any reason why service being "true" and "false" are in 2 @run? This > means setup() will run twice. > > --Max > >> On Aug 22, 2017, at 9:36 PM, Sibabrata Sahoo >> wrote: >> >> Hi Max, >> >> Please find the updated webrev addressing all comments in applicable test >> files. >> >> Webrev: http://cr.openjdk.java.net/~ssahoo/8183310/webrev.01/ >> >> Regarding "172-180: This sounds like you can use legacy jars as module jars >> and vice versa. Is this something new?" >> This is just additional Test case for regular jars in modulepath and modular >> jars in classpath, which verifies how the type get resolved. >> >> Thanks, >> Siba >> >> -Original Message- >> From: Weijun Wang >> Sent: Wednesday, August 02, 2017 3:53 PM >> To: Sibabrata Sahoo >> Cc: Valerie Peng; security-dev@openjdk.java.net >> Subject: Re: [10] RFR: JDK-8183310: java/security/modules/ModularTest.java >> should clean up better >> >> I am reading JaasModularClientTest.java now. It’s much simpler than the >> original one. Some comments: >> >> 38: This ProcessTools is deprecated. Is it possible to use the one in root >> repo? >> >> 53: Why make it public? >> >> 98-99: What localized strings are you trying to avoid? >> >> 103-104: Why do these 2 variables contain the option name? I mean in the >> following expression >> >> String.format("-cp %s --module-path %s %s %s", unnC, modL, addNMArg, >> C_TYPE) >> >> you have put some option names (-cp, --module-path) in the format string but >> another (--add-modules) in an argument (addNMArg). It will be more clear to >> put option names in format string and option values in arguments, say >> >> String.format("-cp %s --module-path %s --add-modules %s %s", unnC, modL, >> "ml", C_TYPE) >> >> Or just simply >> >> String.format("-cp %s --module-path %s --add-modules ml %s", unnC, modL, >> C_TYPE) >> >> >> 128: This loop looks a little strange. I'd rather just write the content >> twice. >> >> 136: This is the only usage of service parameter? If so, why not just move >> the line into constructor? >> >> 140: Why is there a "-" after "Case:"? >> >> 141: In every case you are expecting EXP_RES. Why bother include it as a >> parameter? In fact, what else do you expect? If EXP_RES is not seen, >> shouldn't the client just fails with an exception? If so, why check >> contains() on line 197 and not look at the exit value? >> >> 172-180: This sounds like you can use legacy jars as module jars and vice >> versa. Is this something new? >> >> 212-245: Please add an empty line before every new mBuilder assignment. >> >> 221: So you need jdk.security.auth for UnixPrincipal? It looks like this >> class is also available on Windows but I feel it a little strange. Maybe use >> UserPrincipal or create your new one? >> >> I'll read the other two tests and hopefully they have similar structures. >> >> Thanks >> Max >> >>> On Jul 15, 2017, at 6:30 PM, Sibabrata Sahoo >>> wrote: >>> >>> Hi, >>> >>> Please review the patch for the following, >>> >>> JBS: https://bugs.openjdk.java.net/browse/JDK-8183310 >>> Webrev: http://cr.openjdk.java.net/~ssahoo/8183310/webrev.00/ >>> >>> Change description: >>> The code has been changed significantly for cleaning up existing code and >>> to simplify it. During clean up, I have also removed the unwanted file >>> “java/security/modules/ModularTest.java”. >>> >>> SecurityProviderModularTest.java - Verify security provider in all possible >>> modular combination. There are 2 related files “TestClient.java” and >>> “TestProvider.java” renamed and changed a little during clean up. >>> JaasModularClientTest.java – Verify JAAS login module in all possible >>> modular combination. >>> JaasModularDefaultHandlerTest.java - Verify JAAS default handler in all >>> possible modular combination. As part of clean I have also removed >>> “TEST.properties” file. >>> >>> Thanks, >>> Siba >> >
RE: [10] RFR: JDK-8183310: java/security/modules/ModularTest.java should clean up better
Hi Max, I have splitted the @run to convert each run to consume minimum time to avoid any timeout for rare cases. Thanks, Siba -Original Message- From: Weijun Wang Sent: Wednesday, August 23, 2017 1:43 PM To: Sibabrata Sahoo Cc: Valerie Peng; security-dev@openjdk.java.net Subject: Re: [10] RFR: JDK-8183310: java/security/modules/ModularTest.java should clean up better Is there any reason why service being "true" and "false" are in 2 @run? This means setup() will run twice. --Max > On Aug 22, 2017, at 9:36 PM, Sibabrata Sahoo > wrote: > > Hi Max, > > Please find the updated webrev addressing all comments in applicable test > files. > > Webrev: http://cr.openjdk.java.net/~ssahoo/8183310/webrev.01/ > > Regarding "172-180: This sounds like you can use legacy jars as module jars > and vice versa. Is this something new?" > This is just additional Test case for regular jars in modulepath and modular > jars in classpath, which verifies how the type get resolved. > > Thanks, > Siba > > -Original Message- > From: Weijun Wang > Sent: Wednesday, August 02, 2017 3:53 PM > To: Sibabrata Sahoo > Cc: Valerie Peng; security-dev@openjdk.java.net > Subject: Re: [10] RFR: JDK-8183310: java/security/modules/ModularTest.java > should clean up better > > I am reading JaasModularClientTest.java now. It’s much simpler than the > original one. Some comments: > > 38: This ProcessTools is deprecated. Is it possible to use the one in root > repo? > > 53: Why make it public? > > 98-99: What localized strings are you trying to avoid? > > 103-104: Why do these 2 variables contain the option name? I mean in the > following expression > >String.format("-cp %s --module-path %s %s %s", unnC, modL, addNMArg, > C_TYPE) > > you have put some option names (-cp, --module-path) in the format string but > another (--add-modules) in an argument (addNMArg). It will be more clear to > put option names in format string and option values in arguments, say > >String.format("-cp %s --module-path %s --add-modules %s %s", unnC, modL, > "ml", C_TYPE) > > Or just simply > >String.format("-cp %s --module-path %s --add-modules ml %s", unnC, modL, > C_TYPE) > > > 128: This loop looks a little strange. I'd rather just write the content > twice. > > 136: This is the only usage of service parameter? If so, why not just move > the line into constructor? > > 140: Why is there a "-" after "Case:"? > > 141: In every case you are expecting EXP_RES. Why bother include it as a > parameter? In fact, what else do you expect? If EXP_RES is not seen, > shouldn't the client just fails with an exception? If so, why check > contains() on line 197 and not look at the exit value? > > 172-180: This sounds like you can use legacy jars as module jars and vice > versa. Is this something new? > > 212-245: Please add an empty line before every new mBuilder assignment. > > 221: So you need jdk.security.auth for UnixPrincipal? It looks like this > class is also available on Windows but I feel it a little strange. Maybe use > UserPrincipal or create your new one? > > I'll read the other two tests and hopefully they have similar structures. > > Thanks > Max > >> On Jul 15, 2017, at 6:30 PM, Sibabrata Sahoo >> wrote: >> >> Hi, >> >> Please review the patch for the following, >> >> JBS: https://bugs.openjdk.java.net/browse/JDK-8183310 >> Webrev: http://cr.openjdk.java.net/~ssahoo/8183310/webrev.00/ >> >> Change description: >> The code has been changed significantly for cleaning up existing code and to >> simplify it. During clean up, I have also removed the unwanted file >> “java/security/modules/ModularTest.java”. >> >> SecurityProviderModularTest.java - Verify security provider in all possible >> modular combination. There are 2 related files “TestClient.java” and >> “TestProvider.java” renamed and changed a little during clean up. >> JaasModularClientTest.java – Verify JAAS login module in all possible >> modular combination. >> JaasModularDefaultHandlerTest.java - Verify JAAS default handler in all >> possible modular combination. As part of clean I have also removed >> “TEST.properties” file. >> >> Thanks, >> Siba >
RE: [10] RFR: JDK-8183310: java/security/modules/ModularTest.java should clean up better
Hi Max, Please find the updated webrev addressing all comments in applicable test files. Webrev: http://cr.openjdk.java.net/~ssahoo/8183310/webrev.01/ Regarding "172-180: This sounds like you can use legacy jars as module jars and vice versa. Is this something new?" This is just additional Test case for regular jars in modulepath and modular jars in classpath, which verifies how the type get resolved. Thanks, Siba -Original Message- From: Weijun Wang Sent: Wednesday, August 02, 2017 3:53 PM To: Sibabrata Sahoo Cc: Valerie Peng; security-dev@openjdk.java.net Subject: Re: [10] RFR: JDK-8183310: java/security/modules/ModularTest.java should clean up better I am reading JaasModularClientTest.java now. It’s much simpler than the original one. Some comments: 38: This ProcessTools is deprecated. Is it possible to use the one in root repo? 53: Why make it public? 98-99: What localized strings are you trying to avoid? 103-104: Why do these 2 variables contain the option name? I mean in the following expression String.format("-cp %s --module-path %s %s %s", unnC, modL, addNMArg, C_TYPE) you have put some option names (-cp, --module-path) in the format string but another (--add-modules) in an argument (addNMArg). It will be more clear to put option names in format string and option values in arguments, say String.format("-cp %s --module-path %s --add-modules %s %s", unnC, modL, "ml", C_TYPE) Or just simply String.format("-cp %s --module-path %s --add-modules ml %s", unnC, modL, C_TYPE) 128: This loop looks a little strange. I'd rather just write the content twice. 136: This is the only usage of service parameter? If so, why not just move the line into constructor? 140: Why is there a "-" after "Case:"? 141: In every case you are expecting EXP_RES. Why bother include it as a parameter? In fact, what else do you expect? If EXP_RES is not seen, shouldn't the client just fails with an exception? If so, why check contains() on line 197 and not look at the exit value? 172-180: This sounds like you can use legacy jars as module jars and vice versa. Is this something new? 212-245: Please add an empty line before every new mBuilder assignment. 221: So you need jdk.security.auth for UnixPrincipal? It looks like this class is also available on Windows but I feel it a little strange. Maybe use UserPrincipal or create your new one? I'll read the other two tests and hopefully they have similar structures. Thanks Max > On Jul 15, 2017, at 6:30 PM, Sibabrata Sahoo > wrote: > > Hi, > > Please review the patch for the following, > > JBS: https://bugs.openjdk.java.net/browse/JDK-8183310 > Webrev: http://cr.openjdk.java.net/~ssahoo/8183310/webrev.00/ > > Change description: > The code has been changed significantly for cleaning up existing code and to > simplify it. During clean up, I have also removed the unwanted file > “java/security/modules/ModularTest.java”. > > SecurityProviderModularTest.java - Verify security provider in all possible > modular combination. There are 2 related files “TestClient.java” and > “TestProvider.java” renamed and changed a little during clean up. > JaasModularClientTest.java – Verify JAAS login module in all possible modular > combination. > JaasModularDefaultHandlerTest.java - Verify JAAS default handler in all > possible modular combination. As part of clean I have also removed > “TEST.properties” file. > > Thanks, > Siba
RE: [10] RFR: JDK-8183310: java/security/modules/ModularTest.java should clean up better
Hi, This patch is pending for your review. Thanks, Siba From: Sibabrata Sahoo Sent: Saturday, July 15, 2017 4:00 PM To: Weijun Wang; Valerie Peng; security-dev@openjdk.java.net Subject: [10] RFR: JDK-8183310: java/security/modules/ModularTest.java should clean up better Hi, Please review the patch for the following, JBS: https://bugs.openjdk.java.net/browse/JDK-8183310 Webrev: http://cr.openjdk.java.net/~ssahoo/8183310/webrev.00/ Change description: The code has been changed significantly for cleaning up existing code and to simplify it. During clean up, I have also removed the unwanted file "java/security/modules/ModularTest.java". SecurityProviderModularTest.java - Verify security provider in all possible modular combination. There are 2 related files "TestClient.java" and "TestProvider.java" renamed and changed a little during clean up. JaasModularClientTest.java - Verify JAAS login module in all possible modular combination. JaasModularDefaultHandlerTest.java - Verify JAAS default handler in all possible modular combination. As part of clean I have also removed "TEST.properties" file. Thanks, Siba
[10] RFR: JDK-8183310: java/security/modules/ModularTest.java should clean up better
Hi, Please review the patch for the following, JBS: https://bugs.openjdk.java.net/browse/JDK-8183310 Webrev: http://cr.openjdk.java.net/~ssahoo/8183310/webrev.00/ Change description: The code has been changed significantly for cleaning up existing code and to simplify it. During clean up, I have also removed the unwanted file "java/security/modules/ModularTest.java". SecurityProviderModularTest.java - Verify security provider in all possible modular combination. There are 2 related files "TestClient.java" and "TestProvider.java" renamed and changed a little during clean up. JaasModularClientTest.java - Verify JAAS login module in all possible modular combination. JaasModularDefaultHandlerTest.java - Verify JAAS default handler in all possible modular combination. As part of clean I have also removed "TEST.properties" file. Thanks, Siba
RE: RFR 8181461: sun/security/krb5/auto/KdcPolicy.java fails with java.lang.Exception: Does not match
Change looks fine to me. But I am not the reviewer yet. Thanks, Siba -Original Message- From: Weijun Wang Sent: Tuesday, June 06, 2017 11:23 AM To: Security Dev OpenJDK Cc: Gustavo Galimberti; Sibabrata Sahoo Subject: RFR 8181461: sun/security/krb5/auto/KdcPolicy.java fails with java.lang.Exception: Does not match Please take a review on this change: http://cr.openjdk.java.net/~weijun/8181461/webrev.00/ This is a test bug and the fix is simply: // 1. Default policy is tryLast writeConf(1, 3000, p1, p3); -test("a3000c3000c3000|a3000c3000-|a3000c3000c3000-"); +test("a3000c3000c3000|a3000c3000-|a3000c3000c3000a3000-"); Here, max_retries is 1 and timeout is 3000ms. A is a KDC that never replies, and C is one that usually replies in time. Here the test client might send out 2 AS_REQs, the initial one and the one with preauth. We should observe these possible results: (1). C always replies in time: 1. Initial AS_REQ sent to A, timeout (a3000) 2. Initial AS_REQ sent to C, succeed (c3000) 3. AS_REQ with preauth sent to C (try last good), succeed (c3000) (2). C fails the 1st time: 1. Initial AS_REQ sent to A, timeout (a3000) 2. Initial AS_REQ sent to C, timeout (c3000) 3. Final result is failure (-) (3). C succeeds for the 1st time but fails later: 1. Initial AS_REQ sent to A, timeout (a3000) 2. Initial AS_REQ sent to C, succeed (c3000) 3. AS_REQ with preauth sent to C (try last good), timeout (c3000) 4. AS_REQ with preauth sent to A, timeout (a3000) 5. Final result is failure (-) The original test code has a bug with case (3), where it assumes #4 above is not sent, this is wrong. AS_REQ with preauth is a new request different from the initial AS_REQ. The order of preference is changed according to the policy (set to tryLast) but all KDCs will still be tried. Thanks Max
RE: [9] RFR: 8168423: Test Task: Custom system class loader + security manager + malformed policy file = recursive initialization
Hi Sean, Here is the updated webrev: http://cr.openjdk.java.net/~ssahoo/8168423/webrev.02/ The only change between the previous is, The bugid is reverted back from 8168075 to 8168423. The reason is it fails jcheck with the following message, remote: Bugid 8168075 already used in this repository, in revision 16548 Regarding the following comment on " grant codeBase "file:./jars/*" ", we have already discussed and we are fine here to not make any change. Thanks, Siba -Original Message- From: Sean Mullan Sent: Wednesday, February 08, 2017 10:00 PM To: Sibabrata Sahoo; Adam Petcher; security-dev@openjdk.java.net Subject: Re: [9] RFR: 8168423: Test Task: Custom system class loader + security manager + malformed policy file = recursive initialization On 2/7/17 4:26 AM, Sibabrata Sahoo wrote: > Hi Sean, > > Please find the updated webrev at: > http://cr.openjdk.java.net/~ssahoo/8168075/webrev.01/ > > It includes the following changes, > 1) valid.policy, uses 'grant codebase "executable jar path"'. Hmm, the use of '.' in the codebase URL is probably not good practice here and I'm a little concerned it may not always work. Try this instead: grant codeBase "file:${test.classes}/-" A trailing "/-" matches all files (both class and JAR files) in the directory and recursively all files in subdirectories contained in that directory. --Sean > 2) In ClassLoaderTest.java, @bug renamed from 8168423 to 8168075. > 3) In ClassLoaderTest.java, the code comments has been removed from @summary > section. But it retains the same at line: 91-102. > > Thanks, > Siba > > -Original Message- > From: Sean Mullan > Sent: Friday, January 27, 2017 12:07 AM > To: Sibabrata Sahoo; Adam Petcher; security-dev@openjdk.java.net > Subject: Re: [9] RFR: 8168423: Test Task: Custom system class loader + > security manager + malformed policy file = recursive initialization > > Hi Siba, > > In valid.policy, use 'grant codeBase "file:${test.classes}/*"' so that only > the tests are granted the needed permissions. > > In ClassLoaderTest.java, the @bug should be 8168075. Also, the @summary > contains a bunch of lines (29-39) that should probably just be code comments. > > Seems fine otherwise. > > --Sean > > > On 1/11/17 10:33 AM, Sibabrata Sahoo wrote: >> Hi Adam/Sean, >> >> >> >> This patch is waiting for your review. >> >> >> >> Thanks, >> >> Siba >> >> >> >> *From:*Sibabrata Sahoo >> *Sent:* Friday, December 02, 2016 6:56 PM >> *To:* Sean Mullan; security-dev@openjdk.java.net >> *Subject:* [9] RFR: 8168423: Test Task: Custom system class loader + >> security manager + malformed policy file = recursive initialization >> >> >> >> Hi, >> >> >> >> Please review the patch for, >> >> >> >> JBS: https://bugs.openjdk.java.net/browse/JDK-8168423 >> >> Webrev: http://cr.openjdk.java.net/~ssahoo/8168423/webrev.00/ >> >> >> >> Description: >> >> This webrev address all possible cases for Classloader with >> SecurityManager having combination of valid/malformed policy file. >> This Test is going to fail until JDK-8168075 get fixed. In the mean >> time, it can be used to verify the fix for JDK-8168075. >> >> >> >> Here is the generic Logic behind generating all possible Test cases >> with different combination of policy file, class loader and module types. >> >> for(policyFile : {"NO_POLICY", "VALID", "MALFORMED"}) { >> >> for(classLoader : {"SystemClassLoader", "CustomClassLoader"}){ >> >>// It uses possible set of regular/modular jars to generate >> all possible Test cases in -cp and -module-path. >> >> for(clientModuletype : {"STRICT", "AUTO", "UNKNOWN"}) { >> >> for(classLoaderModuleType : {"STRICT", "AUTO", >> "UNKNOWN"}) { >> >> Create and run java command line for each possible >> Test cases and verify result. >> >> } >> >> } >> >> } >> >> } >> >> >> >> Thanks, >> >> Siba >> >> >>
RE: [9] RFR: 8168423: Test Task: Custom system class loader + security manager + malformed policy file = recursive initialization
Hi Sean, Please find the updated webrev at: http://cr.openjdk.java.net/~ssahoo/8168075/webrev.01/ It includes the following changes, 1) valid.policy, uses 'grant codebase "executable jar path"'. 2) In ClassLoaderTest.java, @bug renamed from 8168423 to 8168075. 3) In ClassLoaderTest.java, the code comments has been removed from @summary section. But it retains the same at line: 91-102. Thanks, Siba -Original Message- From: Sean Mullan Sent: Friday, January 27, 2017 12:07 AM To: Sibabrata Sahoo; Adam Petcher; security-dev@openjdk.java.net Subject: Re: [9] RFR: 8168423: Test Task: Custom system class loader + security manager + malformed policy file = recursive initialization Hi Siba, In valid.policy, use 'grant codeBase "file:${test.classes}/*"' so that only the tests are granted the needed permissions. In ClassLoaderTest.java, the @bug should be 8168075. Also, the @summary contains a bunch of lines (29-39) that should probably just be code comments. Seems fine otherwise. --Sean On 1/11/17 10:33 AM, Sibabrata Sahoo wrote: > Hi Adam/Sean, > > > > This patch is waiting for your review. > > > > Thanks, > > Siba > > > > *From:*Sibabrata Sahoo > *Sent:* Friday, December 02, 2016 6:56 PM > *To:* Sean Mullan; security-dev@openjdk.java.net > *Subject:* [9] RFR: 8168423: Test Task: Custom system class loader + > security manager + malformed policy file = recursive initialization > > > > Hi, > > > > Please review the patch for, > > > > JBS: https://bugs.openjdk.java.net/browse/JDK-8168423 > > Webrev: http://cr.openjdk.java.net/~ssahoo/8168423/webrev.00/ > > > > Description: > > This webrev address all possible cases for Classloader with > SecurityManager having combination of valid/malformed policy file. > This Test is going to fail until JDK-8168075 get fixed. In the mean > time, it can be used to verify the fix for JDK-8168075. > > > > Here is the generic Logic behind generating all possible Test cases > with different combination of policy file, class loader and module types. > > for(policyFile : {"NO_POLICY", "VALID", "MALFORMED"}) { > > for(classLoader : {"SystemClassLoader", "CustomClassLoader"}){ > >// It uses possible set of regular/modular jars to generate all > possible Test cases in -cp and -module-path. > > for(clientModuletype : {"STRICT", "AUTO", "UNKNOWN"}) { > > for(classLoaderModuleType : {"STRICT", "AUTO", "UNKNOWN"}) > { > > Create and run java command line for each possible > Test cases and verify result. > > } > > } > > } > > } > > > > Thanks, > > Siba > > >
RE: [9] RFR: 8168423: Test Task: Custom system class loader + security manager + malformed policy file = recursive initialization
Hi Adam/Sean, This patch is waiting for your review. Thanks, Siba From: Sibabrata Sahoo Sent: Friday, December 02, 2016 6:56 PM To: Sean Mullan; security-dev@openjdk.java.net Subject: [9] RFR: 8168423: Test Task: Custom system class loader + security manager + malformed policy file = recursive initialization Hi, Please review the patch for, JBS: https://bugs.openjdk.java.net/browse/JDK-8168423 Webrev: http://cr.openjdk.java.net/~ssahoo/8168423/webrev.00/ Description: This webrev address all possible cases for Classloader with SecurityManager having combination of valid/malformed policy file. This Test is going to fail until JDK-8168075 get fixed. In the mean time, it can be used to verify the fix for JDK-8168075. Here is the generic Logic behind generating all possible Test cases with different combination of policy file, class loader and module types. for(policyFile : {"NO_POLICY", "VALID", "MALFORMED"}) { for(classLoader : {"SystemClassLoader", "CustomClassLoader"}){ // It uses possible set of regular/modular jars to generate all possible Test cases in -cp and -module-path. for(clientModuletype : {"STRICT", "AUTO", "UNKNOWN"}) { for(classLoaderModuleType : {"STRICT", "AUTO", "UNKNOWN"}) { Create and run java command line for each possible Test cases and verify result. } } } } Thanks, Siba
[9] RFR 8161232: AsyncSSLSocketClose.java test fails timeout.
Hi, Please review the following patch, JBS: https://bugs.openjdk.java.net/browse/JDK-8161232 Webrev: http://cr.openjdk.java.net/~ssahoo/8161232/webrev.00/ Description: It was reported the Test was failing with timeout consistently on macOS. There is no log available anymore and the issue is not reproducible after several trial. It also seems that the actual issue has been fixed by JDK-8075484. I believe it will be appropriate for now to remove the Test from problem list and re-open if the issue persist again. Thanks, Siba
[9] RFR: 8168423: Test Task: Custom system class loader + security manager + malformed policy file = recursive initialization
Hi, Please review the patch for, JBS: https://bugs.openjdk.java.net/browse/JDK-8168423 Webrev: http://cr.openjdk.java.net/~ssahoo/8168423/webrev.00/ Description: This webrev address all possible cases for Classloader with SecurityManager having combination of valid/malformed policy file. This Test is going to fail until JDK-8168075 get fixed. In the mean time, it can be used to verify the fix for JDK-8168075. Here is the generic Logic behind generating all possible Test cases with different combination of policy file, class loader and module types. for(policyFile : {"NO_POLICY", "VALID", "MALFORMED"}) { for(classLoader : {"SystemClassLoader", "CustomClassLoader"}){ // It uses possible set of regular/modular jars to generate all possible Test cases in -cp and -module-path. for(clientModuletype : {"STRICT", "AUTO", "UNKNOWN"}) { for(classLoaderModuleType : {"STRICT", "AUTO", "UNKNOWN"}) { Create and run java command line for each possible Test cases and verify result. } } } } Thanks, Siba
[9] RFR 8170247: java/security/SecureRandom/ApiTest fails when run with unlimited policy.
Hi, Please review the patch for, JBS: https://bugs.openjdk.java.net/browse/JDK-8170247 Webrev: http://cr.openjdk.java.net/~ssahoo/8170247/webrev.00/ Description: The Test was failing to handle the expected failure for invalid parameters, when the SecureRandom parameter size is > algorithm running with unlimited policy. I have modified the condition to handle the expected failures correctly. The change is only applicable to the "if" condition(Line:200). The other 2 lines are minor changes related to statement performance and renaming a variable. Thanks, Siba
RE: [9] RFR: 8156054: Test Task: Develop new tests for JEP C155: Remove FilePermission Pathname Canonicalization
Hi Artem, I think building a String value from the actual result and displaying it against expected in the Log output will be more convenient here. Thanks, Siba From: Artem Smotrakov Sent: Tuesday, September 13, 2016 10:54 PM To: Sibabrata Sahoo; Weijun Wang; security-dev@openjdk.java.net Subject: Re: [9] RFR: 8156054: Test Task: Develop new tests for JEP C155: Remove FilePermission Pathname Canonicalization Hi Siba, I see that the test expects only true or false. You can just pass these boolean values, and check() can make sure that everything returns the expected boolean value without building a string. That would be clearer to me. Not an issue, it's up to you to change it or not. Artem On 09/13/2016 03:14 AM, Sibabrata Sahoo wrote: Hi, Please review the patch for, JBS: https://bugs.openjdk.java.net/browse/JDK-8156054 Webrev: HYPERLINK "http://cr.openjdk.java.net/%7Essahoo/8156054/webrev.00/"http://cr.openjdk.java.net/~ssahoo/8156054/webrev.00/ Description: It checks equals(), implies() and hashCode () of FilePermission when "jdk.io.permissionsUseCanonicalPath" set and un-set. Along with, it also verify compatibility with previous JDK version. Thanks, Siba
RE: [9] RFR: 8156054: Test Task: Develop new tests for JEP C155: Remove FilePermission Pathname Canonicalization
Hi Max, Here is the updated webrev: http://cr.openjdk.java.net/~ssahoo/8156054/webrev.01/ Thanks, Siba -Original Message- From: Weijun Wang Sent: Tuesday, September 13, 2016 4:25 PM To: Sibabrata Sahoo; security-dev@openjdk.java.net Subject: Re: [9] RFR: 8156054: Test Task: Develop new tests for JEP C155: Remove FilePermission Pathname Canonicalization 44 final String current = "."; 45 final File realFile = new File(current, "exist.file"); I suggest we just use new File("exist.file"). No other comment. Thanks Max On 9/13/2016 18:14, Sibabrata Sahoo wrote: > Hi, > > > > Please review the patch for, > > > > JBS: https://bugs.openjdk.java.net/browse/JDK-8156054 > > Webrev: http://cr.openjdk.java.net/~ssahoo/8156054/webrev.00/ > > > > Description: > > It checks equals(), implies() and hashCode () of FilePermission when > "jdk.io.permissionsUseCanonicalPath" set and un-set. Along with, it > also verify compatibility with previous JDK version. > > > > > > Thanks, > > Siba > > >
[9] RFR: 8165660: Remove the intermittent keyword from sun/security/krb5/auto/MaxRetries.java
Hi, Please review the patch for, JBS: https://bugs.openjdk.java.net/browse/JDK-8165660 Webrev: http://cr.openjdk.java.net/~ssahoo/8165660/webrev.00/ Description: This is a simple fix for removing intermittent keyword from sun/security/krb5/auto/MaxRetries.java file. Thanks, Siba
[9] RFR: 8165825: Remove the intermittent keyword from sun/security/krb5/auto/Unreachable.java
Hi, Please review the patch for, JBS: https://bugs.openjdk.java.net/browse/JDK-8165825 Webrev: http://cr.openjdk.java.net/~ssahoo/8165825/webrev.00/ Description: This is a simple fix for removing intermittent keyword from sun/security/krb5/auto/Unreachable.java file. Thanks, Siba
[9] RFR: 8156054: Test Task: Develop new tests for JEP C155: Remove FilePermission Pathname Canonicalization
Hi, Please review the patch for, JBS: https://bugs.openjdk.java.net/browse/JDK-8156054 Webrev: http://cr.openjdk.java.net/~ssahoo/8156054/webrev.00/ Description: It checks equals(), implies() and hashCode () of FilePermission when "jdk.io.permissionsUseCanonicalPath" set and un-set. Along with, it also verify compatibility with previous JDK version. Thanks, Siba
RE: [9] RFR: 8164922: sun/security/provider/SecureRandom/AutoReseed.java failed with timeout in Ubuntu Linux.
Hi Max, Here is the updated webrev: http://cr.openjdk.java.net/~ssahoo/8164922/webrev.01/ I did a minor change other than addressing the following comment. I allowed all the test case to execute irrespective exception thrown. Thanks, Siba -Original Message- From: Weijun Wang Sent: Tuesday, August 30, 2016 1:40 PM To: Sibabrata Sahoo; security-dev@openjdk.java.net Subject: Re: [9] RFR: 8164922: sun/security/provider/SecureRandom/AutoReseed.java failed with timeout in Ubuntu Linux. Please just use othervm, and remove the finally block. If oldegd is null the code change will change the environment permanently. --Max On 8/29/2016 23:20, Sibabrata Sahoo wrote: > Hi, > > > > Please review the patch for > "sun/security/provider/SecureRandom/AutoReseed.java failed with timeout" > > > > JBS: https://bugs.openjdk.java.net/browse/JDK-8164922 > > Webrev: http://cr.openjdk.java.net/~ssahoo/8164922/webrev.00/ > > > > Description: > > The Test was blocked while generating seed. I have providing a fix by > choosing a non-blocking mechanism while seeding by setting > "java.security.egd" System property to "file:/dev/urandom". > > > > Thanks, > > Siba > > >
[9] RFR: 8164922: sun/security/provider/SecureRandom/AutoReseed.java failed with timeout in Ubuntu Linux.
Hi, Please review the patch for "sun/security/provider/SecureRandom/AutoReseed.java failed with timeout" JBS: https://bugs.openjdk.java.net/browse/JDK-8164922 Webrev: http://cr.openjdk.java.net/~ssahoo/8164922/webrev.00/ Description: The Test was blocked while generating seed. I have providing a fix by choosing a non-blocking mechanism while seeding by setting "java.security.egd" System property to "file:/dev/urandom". Thanks, Siba
RE: [9] RFR: 8015595: Test sun/security/krb5/auto/Unreachable.java fails with Timeout error
Hi Max, Here is updated webrev: http://cr.openjdk.java.net/~ssahoo/8015595/webrev.02/ Addressed all the following comments. Thanks, Siba -Original Message- From: Weijun Wang Sent: Monday, August 29, 2016 6:14 PM To: Sibabrata Sahoo; security-dev@openjdk.java.net Subject: Re: [9] RFR: 8015595: Test sun/security/krb5/auto/Unreachable.java fails with Timeout error Looks fine mostly. Some nits: 58 // - SocketTimeoutException may occur in MAC because it will not throw s/in MAC/on Mac/. 66 + "occured or PortUnreachableException not thrown by the " s/or/which means/. 76 File f = new File("unreachable.krb5.conf"); 77 System.setProperty("java.security.krb5.conf", f.getPath()); How is this better than just set the property value to "unreachable.krb5.conf"? Thanks Max On 8/29/2016 20:32, Sibabrata Sahoo wrote: > Hi Max, > > Please find the updated webrev addressing all comments bellow: > http://cr.openjdk.java.net/~ssahoo/8015595/webrev.01/ > > Thanks, > Siba > > -Original Message- > From: Weijun Wang > Sent: Monday, August 29, 2016 6:36 AM > To: Sibabrata Sahoo; security-dev@openjdk.java.net > Subject: Re: [9] RFR: 8015595: Test > sun/security/krb5/auto/Unreachable.java fails with Timeout error > > Several comments: > > 1. findPortUnreachableExc() can be put outside of the Future and called > directly. > > 2. When a TimeoutException is caught from future.get(), I think this > should be treated as a failure. We have already used > findPortUnreachableExc() to confirm PUE is available on this system and that > port is not used by another process. If we still see timeout, it should be > investigated. In fact, if the test succeeds in this case, it will never fail. > Right? > > 3. Why not write 2 catch blocks here for each exception type? > > 120 } catch (SocketTimeoutException | PortUnreachableException > e) { > 121 if (e instanceof PortUnreachableException) { > 122 return true; > 123 } > 124 } > > Thanks > Max > > On 8/26/2016 18:39, Sibabrata Sahoo wrote: >> Hi, >> >> >> >> Here is the latest webrev: >> http://cr.openjdk.java.net/~ssahoo/8015595/webrev.00/ >> >> >> >> I have updated the test with these additional support, >> >> 1) Verifying, if the port is reachable or PortUnreachableException >> is supported by the platform, otherwise the Test will terminate >> itself with warning. >> >> 2) Removed the test from ProblemList.txt for MAC OS because of #1. >> >> 3) Uses only one KDC port in the configuration file. >> >> 4) Removed static "unreachable.krb5.conf" as the Test creates the >> file during runtime. >> >> >> >> Thanks, >> >> Siba >> >> >> >> *From:*Sibabrata Sahoo >> *Sent:* Wednesday, August 24, 2016 9:46 PM >> *To:* Weijun Wang; security-dev@openjdk.java.net >> *Subject:* [9] RFR: 8015595: Test >> sun/security/krb5/auto/Unreachable.java fails with Timeout error >> >> >> >> Hi, >> >> >> >> Please review the patch for "sun/security/krb5/auto/Unreachable.java >> fails with Timeout error" >> >> >> >> JBS: https://bugs.openjdk.java.net/browse/JDK-8015595 >> >> Webrev: http://cr.openjdk.java.net/~ssahoo/8015595/webrev.00/ >> >> >> >> Description: >> >> When a KDC port is unreachable, Kerberos login module depends on >> PortUnreachableException to exit immediately. But as per JavaDoc for >> receive() in "java.net.DatagramSocket", the PortUnreachableException >> is not guaranteed. In such case the Login module waits for 90 second >> by default. But the JTREG Test timeout has been set for 10 second and >> because of that the Test gets timeout. >> >> >> >> Since the "intermittent" failure is unavoidable due to >> "PortUnreachableException is not guaranteed", I have provided a fix >> to print a warning message for such cases instead of making the Test timeout. >> >> >> >> Thanks, >> >> Siba >> >> >>
RE: [9] RFR: 8015595: Test sun/security/krb5/auto/Unreachable.java fails with Timeout error
Hi Max, Please find the updated webrev addressing all comments bellow: http://cr.openjdk.java.net/~ssahoo/8015595/webrev.01/ Thanks, Siba -Original Message- From: Weijun Wang Sent: Monday, August 29, 2016 6:36 AM To: Sibabrata Sahoo; security-dev@openjdk.java.net Subject: Re: [9] RFR: 8015595: Test sun/security/krb5/auto/Unreachable.java fails with Timeout error Several comments: 1. findPortUnreachableExc() can be put outside of the Future and called directly. 2. When a TimeoutException is caught from future.get(), I think this should be treated as a failure. We have already used findPortUnreachableExc() to confirm PUE is available on this system and that port is not used by another process. If we still see timeout, it should be investigated. In fact, if the test succeeds in this case, it will never fail. Right? 3. Why not write 2 catch blocks here for each exception type? 120 } catch (SocketTimeoutException | PortUnreachableException e) { 121 if (e instanceof PortUnreachableException) { 122 return true; 123 } 124 } Thanks Max On 8/26/2016 18:39, Sibabrata Sahoo wrote: > Hi, > > > > Here is the latest webrev: > http://cr.openjdk.java.net/~ssahoo/8015595/webrev.00/ > > > > I have updated the test with these additional support, > > 1) Verifying, if the port is reachable or PortUnreachableException > is supported by the platform, otherwise the Test will terminate itself > with warning. > > 2) Removed the test from ProblemList.txt for MAC OS because of #1. > > 3) Uses only one KDC port in the configuration file. > > 4) Removed static "unreachable.krb5.conf" as the Test creates the > file during runtime. > > > > Thanks, > > Siba > > > > *From:*Sibabrata Sahoo > *Sent:* Wednesday, August 24, 2016 9:46 PM > *To:* Weijun Wang; security-dev@openjdk.java.net > *Subject:* [9] RFR: 8015595: Test > sun/security/krb5/auto/Unreachable.java fails with Timeout error > > > > Hi, > > > > Please review the patch for "sun/security/krb5/auto/Unreachable.java > fails with Timeout error" > > > > JBS: https://bugs.openjdk.java.net/browse/JDK-8015595 > > Webrev: http://cr.openjdk.java.net/~ssahoo/8015595/webrev.00/ > > > > Description: > > When a KDC port is unreachable, Kerberos login module depends on > PortUnreachableException to exit immediately. But as per JavaDoc for > receive() in "java.net.DatagramSocket", the PortUnreachableException > is not guaranteed. In such case the Login module waits for 90 second > by default. But the JTREG Test timeout has been set for 10 second and > because of that the Test gets timeout. > > > > Since the "intermittent" failure is unavoidable due to > "PortUnreachableException is not guaranteed", I have provided a fix to > print a warning message for such cases instead of making the Test timeout. > > > > Thanks, > > Siba > > >
RE: [9] RFR: 8015595: Test sun/security/krb5/auto/Unreachable.java fails with Timeout error
Hi, Here is the latest webrev: http://cr.openjdk.java.net/~ssahoo/8015595/webrev.00/ I have updated the test with these additional support, 1) Verifying, if the port is reachable or PortUnreachableException is supported by the platform, otherwise the Test will terminate itself with warning. 2) Removed the test from ProblemList.txt for MAC OS because of #1. 3) Uses only one KDC port in the configuration file. 4) Removed static "unreachable.krb5.conf" as the Test creates the file during runtime. Thanks, Siba From: Sibabrata Sahoo Sent: Wednesday, August 24, 2016 9:46 PM To: Weijun Wang; security-dev@openjdk.java.net Subject: [9] RFR: 8015595: Test sun/security/krb5/auto/Unreachable.java fails with Timeout error Hi, Please review the patch for "sun/security/krb5/auto/Unreachable.java fails with Timeout error" JBS: https://bugs.openjdk.java.net/browse/JDK-8015595 Webrev: http://cr.openjdk.java.net/~ssahoo/8015595/webrev.00/ Description: When a KDC port is unreachable, Kerberos login module depends on PortUnreachableException to exit immediately. But as per JavaDoc for receive() in "java.net.DatagramSocket", the PortUnreachableException is not guaranteed. In such case the Login module waits for 90 second by default. But the JTREG Test timeout has been set for 10 second and because of that the Test gets timeout. Since the "intermittent" failure is unavoidable due to "PortUnreachableException is not guaranteed", I have provided a fix to print a warning message for such cases instead of making the Test timeout. Thanks, Siba