Code Review Request:8019544: Need to run ProviderTest.java in othervm mode

[Rajan Halade]

Could you please review the small fix for 8019544:

http://cr.openjdk.java.net/~juh/rajan/8019544/webrev.00/ 
<http://cr.openjdk.java.net/%7Ejuh/rajan/8019544/webrev.00/>


This is a test only fix for test stabilization.

--
<http://www.oracle.com> Rajan Halade, CISSP | Senior Member of Technical 
Staff

Phone: +1 4082763359 
Oracle JAVA Platform Group
4220 Network Circle, Bldg 22, #2155 | Santa Clara, CA 94538
<http://www.oracle.com/commitment> Oracle is committed to developing 
practices and products that help protect the environment

Code Review Request: 8014266

[Rajan Halade]

May I request you to review this simple change.
http://cr.openjdk.java.net/~xuelei/8014266/webrev/

This is to update the test comment to assist in timeout analysis. The 
test may timeout on heavy loaded system as it involves multiple tls 
transactions.


--
<http://www.oracle.com> Rajan Halade, CISSP | Senior Member of Technical 
Staff

Phone: +1 4082763359 
Oracle JAVA Platform Group
4220 Network Circle, Bldg 22, #2155 | Santa Clara, CA 94538
<http://www.oracle.com/commitment> Oracle is committed to developing 
practices and products that help protect the environment

Thoughts on possible options to JDK-8027598

[Rajan Halade]

I am working towards fixing JDK-8027598 
, there are multiple 
options available here and would appreciate your thoughts on this. It 
was filed to address the issue at large reported inJDK-8027526 
.


Problem - When a regression test is run in agentvm mode and alters 
security providers, it can cause adverse effects on next tests executed 
in the batch. We have been batteling with few intermittent failures 
which are caused by scenario like this so I think it is important to 
have this fix.


Possible approaches -
1. Enhance JTREG - This option would require change in jtreg to 
store/restore security providers when run in agentvm mode.
2. Update impacting tests to run in othervm - simple change but may slow 
down batch execution slightly.
3. Update each test to use library 
java/security/KeyPairGenerator/Failover.java like done in 
java/security/Provider tests - another easy change and tests would 
continue to run agentvm but would have added overhead of restoring 
providers.


We will continue to pursue option 1 but many not be possible. Option 2 & 
3 above are equally good and are debatable so would like your thoughts 
on it.


Thanks,
Rajan

Re: Thoughts on possible options to JDK-8027598

[Rajan Halade]

Thanks for your comments. See inline -

- Rajan

On 11/20/2013 03:54, Chris Hegarty wrote:

On 20/11/13 01:07, Xuelei Fan wrote:

On 11/20/2013 8:19 AM, Rajan Halade wrote:

I am working towards fixing JDK-8027598
<https://bugs.openjdk.java.net/browse/JDK-8027598>, there are multiple
options available here and would appreciate your thoughts on this. It
was filed to address the issue at large reported inJDK-8027526
<https://bugs.openjdk.java.net/browse/JDK-8027526>.

Problem - When a regression test is run in agentvm mode and alters
security providers, it can cause adverse effects on next tests executed
in the batch. We have been batteling with few intermittent failures
which are caused by scenario like this so I think it is important to
have this fix.

Possible approaches -
1. Enhance JTREG - This option would require change in jtreg to
store/restore security providers when run in agentvm mode.

If I am correct, JTREG has support provider cleanup already. But the
question is even JTREG reset the providers, it still cannot ensure next
test won't be impacted because in some circumstances JDK may need to
cache something which depends on previous providers.  Still need to
analysis the test case by case.


Right, any static or cached data may be invalid, and this will require 
careful changes to the JRE itself.
Pardon my ignorance - if I gather correctly then ProvidersSnapshot 
library also doesn't sandbox effects completely.


If there are not too many of these tests, I don't think that 
explicitly marking them othervm will hurt too much. And making them 
reliable will reduce greatly the amount of time spent head scratching 
chasing intermittent failures.
I agree with you, we need to consider using othervm mode. There are ~66 
(1% of all reg tests) such tests which are run in agentvm, don't use 
ProvidersSnapshot library, and are modifying providers. I think its safe 
to go with othervm option, let me know.


- Rajan


-Chris.



Xuelei

2. Update impacting tests to run in othervm - simple change but may 
slow

down batch execution slightly.
3. Update each test to use library
java/security/KeyPairGenerator/Failover.java like done in
java/security/Provider tests - another easy change and tests would
continue to run agentvm but would have added overhead of restoring
providers.

We will continue to pursue option 1 but many not be possible. Option 
2 &

3 above are equally good and are debatable so would like your thoughts
on it.

Thanks,
Rajan

Re: Thoughts on possible options to JDK-8027598

[Rajan Halade]

On 11/22/2013 11:42, Sean Mullan wrote:

On 11/21/2013 04:53 AM, Chris Hegarty wrote:

If I am correct, JTREG has support provider cleanup already. But the
question is even JTREG reset the providers, it still cannot ensure 
next

test won't be impacted because in some circumstances JDK may need to
cache something which depends on previous providers. Still need to
analysis the test case by case.


Right, any static or cached data may be invalid, and this will require
careful changes to the JRE itself.

Pardon my ignorance - if I gather correctly then ProvidersSnapshot
library also doesn't sandbox effects completely.


FYI. I am not part of the security team, and someone from the security
group should ultimately have to agree/disagree, but we have similar
issues in other parts of the platform. I don't know the specifics of the
code here so it may, or may not, be an issue.

If any part of the security framework stores values in static fields,
that is dependent on the security providers, then when the providers
change this static value may be incorrect.

We encounter this from time to time with system properties. E.g. JDK
HTTP Client code reads system property and stores in static field, JDK
HTTP Client will never never re-read the property as it believes it will
not change. Having jtreg reset/clean certain system properties will not
help in this case.


Yes, the example with system properties is a good one.

However, providers are designed to be added and removed dynamically 
(see the java.security.Security API), so if there is some static 
information not being cleared when providers are reset, I would tend 
to think it may be a bug in the JDK.

yes. thanks!


My preference would be change jtreg to clear/restore providers, and 
more thoroughly analyze any subsequent test failures as it may be a 
bug in the JDK.
Ok, I will follow up with jtreg team to find out when they can commit to 
timeline.


- Rajan


--Sean

Code Review Request: 8021418

[Rajan Halade]

May I request you to review this simple change -

http://cr.openjdk.java.net/~juh/rajan/8021418/ 



The test is modified to set SO_REUSEADDR on ServerSocket to false for 
stabilization.


Thanks,
Rajan

Re: Code Review Request: 8021418

[Rajan Halade]

On 11/26/2013 18:09, Xuelei Fan wrote:

This change looks fine.

thanks!


JSSE regression tests use a lot of code as "new ServerSocket(0)", we may
want a cleanup for test stabilization.

I will look at regression tests and file separate bug for it.

- Rajan


Xuelei

On 11/27/2013 9:13 AM, Rajan Halade wrote:

May I request you to review this simple change -

http://cr.openjdk.java.net/~juh/rajan/8021418/
<http://cr.openjdk.java.net/%7Ejuh/rajan/8021418/>

The test is modified to set SO_REUSEADDR on ServerSocket to false for
stabilization.

Thanks,
Rajan

Re: Code Review Request: 8021418

[Rajan Halade]

On 11/27/2013 13:07, Bradford Wetmore wrote:

Where did this workaround suggestion come from?

Refer to JDK-6978415, it has possible workaround listed.


Please link these two issues (JDK-6978415) together.

the two bugs are linked now.


Have you talked to the network team about this?

No, since workaround suggested solved the issue.

Thanks,
Rajan


brad




On 11/26/2013 6:09 PM, Xuelei Fan wrote:

This change looks fine.

JSSE regression tests use a lot of code as "new ServerSocket(0)", we may
want a cleanup for test stabilization.

Xuelei

On 11/27/2013 9:13 AM, Rajan Halade wrote:

May I request you to review this simple change -

http://cr.openjdk.java.net/~juh/rajan/8021418/
<http://cr.openjdk.java.net/%7Ejuh/rajan/8021418/>

The test is modified to set SO_REUSEADDR on ServerSocket to false for
stabilization.

Thanks,
Rajan

Re: Thoughts on possible options to JDK-8027598

[Rajan Halade]

On 11/27/2013 12:47, Bradford Wetmore wrote:

> jtreg team? I am not aware of one. Jon Gibbons works on this in his
> spare time.

I would call him "the JTREG team."

Was there an ETA for this?  Balancing recent test stabilization 
efforts with this, you may consider consider adding othervm and then 
back it out when the JTREG fix is made.
I am discussing this with Jon and will soon update with summary of it. I 
am also working on adding /othervm to affected tests.


- Rajan


Brad



On 11/25/2013 2:29 AM, Chris Hegarty wrote:

On 22/11/13 21:11, Rajan Halade wrote:


On 11/22/2013 11:42, Sean Mullan wrote:

On 11/21/2013 04:53 AM, Chris Hegarty wrote:
If I am correct, JTREG has support provider cleanup already. 
But the

question is even JTREG reset the providers, it still cannot ensure
next
test won't be impacted because in some circumstances JDK may 
need to

cache something which depends on previous providers. Still need to
analysis the test case by case.


Right, any static or cached data may be invalid, and this will
require
careful changes to the JRE itself.

Pardon my ignorance - if I gather correctly then ProvidersSnapshot
library also doesn't sandbox effects completely.


FYI. I am not part of the security team, and someone from the 
security

group should ultimately have to agree/disagree, but we have similar
issues in other parts of the platform. I don't know the specifics of
the
code here so it may, or may not, be an issue.

If any part of the security framework stores values in static fields,
that is dependent on the security providers, then when the providers
change this static value may be incorrect.

We encounter this from time to time with system properties. E.g. JDK
HTTP Client code reads system property and stores in static field, 
JDK

HTTP Client will never never re-read the property as it believes it
will
not change. Having jtreg reset/clean certain system properties 
will not

help in this case.


Yes, the example with system properties is a good one.

However, providers are designed to be added and removed dynamically
(see the java.security.Security API), so if there is some static
information not being cleared when providers are reset, I would tend
to think it may be a bug in the JDK.


OK, if this is the case then that is fine.


yes. thanks!


My preference would be change jtreg to clear/restore providers, and
more thoroughly analyze any subsequent test failures as it may be a
bug in the JDK.
Ok, I will follow up with jtreg team to find out when they can 
commit to

timeline.


jtreg team? I am not aware of one. Jon Gibbons works on this in his
spare time.  You may want to take a look at jtreg [1] in the code tools
project.

-Chris.

[1] http://openjdk.java.net/projects/code-tools/jtreg/



- Rajan


--Sean

RFR 8033271: Manual security tests have @ignore rather than @run main/manual

[Rajan Halade]

Please review this fix:

http://cr.openjdk.java.net/~wetmore/8033271/webrev.01/

I removed @ignore from all these tests and marked them with manual for 
now. These tests will be considered for automation as a separate effort 
and will be executed manually for now.


Thanks,
Rajan

Re: RFR 8033271: Manual security tests have @ignore rather than @run main/manual

[Rajan Halade]

Thanks Xuelei for your comment. I updated all tests as per your suggestion -

http://cr.openjdk.java.net/~wetmore/8033271/webrev.02/

Please review again.

Thanks,
Rajan
On 3/14/2014 16:39, Xuelei Fan wrote:

I like the changes. I'm afraid the lines following the @summary tag will
be joined into one line during testing.  We had such bad experiences in
a lot JSSE tests.  Would you like put the manual testing comment into a
separated commend block?

For example, change the update

   * @summary Ensure authentication via GSS-API/Kerberos v5 works.
- * @ignore see runwjaas.csh for instructions for how to run this test
- */
-/*
- * Can set logging to FINEST to view exchange.
+ *  Set logging to FINEST to view exchange.
+ *  See runwjaas.csh for instructions for how to run this test
+ *
+ * @run main/manual AuthOnly

To

   * @summary Ensure authentication via GSS-API/Kerberos v5 works.
- * @ignore see runwjaas.csh for instructions for how to run this test
+ * @run main/manual AuthOnly
   */

  /*
- * Can set logging to FINEST to view exchange.
+ * Set logging to FINEST to view exchange.
+ * See runwjaas.csh for instructions for how to run this test
   */

Thanks,
Xuelei

On 3/15/2014 4:07 AM, Rajan Halade wrote:

Please review this fix:

http://cr.openjdk.java.net/~wetmore/8033271/webrev.01/

I removed @ignore from all these tests and marked them with manual for
now. These tests will be considered for automation as a separate effort
and will be executed manually for now.

Thanks,
Rajan

Re: RFR 8033271: Manual security tests have @ignore rather than @run main/manual

[Rajan Halade]

Thanks again! Updated review with corrections -

http://cr.openjdk.java.net/~wetmore/8033271/webrev.03/

- Rajan

On 3/14/2014 18:36, Xuelei Fan wrote:

Minimal comments:

test/sun/security/smartcardio/TestAll.java
==
Looks like there is no actual update.


test/sun/security/smartcardio/*
===
- 32 //This test requires special hardware.
+ 32 // This test requires special hardware.

Looks nicer if there is leading space.


test/sun/security/smartcardio/TestConnectAgain.java
===
- 29  * @run main/manual TestTransmit
+ 29  * @run main/manual TestConnectAgain

Maybe a typo here.  Would you please make the update?


test/sun/security/ssl/X509TrustManagerImpl/ClientServer.java
===
- 35  * JSSE supports algorithm constraints with CR 6916074,
- 36  * need to update this test case in JDK 7 soon
+
+ 35  * JSSE supports algorithm constraints with CR 6916074, need to
+ 36  * update this test case in JDK 7 soon

Would you mind add a blank line and join the two line accordingly?

Otherwise, looks fine to me.

Thanks,
Xuelei

RFR 8037258: AIOB while parsing CRL for revoked certificate

[Rajan Halade]

Please review this small fix -

http://cr.openjdk.java.net/~mullan/webrevs/8037258/webrev.02/

I have also updated the source to remove unused variables and updated 
javadoc information.


Thanks,
Rajan

[JDK 9] RFR: 8041781: Need new regression tests for PBE keys

[Rajan Halade]

May I request you to review these 3 new tests to be added for PBE keys. 
New tests are added to address following:


- seal/unseal works correctly with PBE algorithms
- key wrapper works correctly with PBEKey
- SecretKeyFactory.translateKey() method works

JDK Issue: https://bugs.openjdk.java.net/browse/JDK-8041781
Webrev: http://cr.openjdk.java.net/~rhalade/8041781/webrev.00/

Thanks,
Rajan

Re: [JDK 9] RFR: 8041781: Need new regression tests for PBE keys

[Rajan Halade]

Ping...can someone help review this please.

Thanks,
Rajan
On 6/18/14, 3:36 PM, Rajan Halade wrote:
May I request you to review these 3 new tests to be added for PBE 
keys. New tests are added to address following:


- seal/unseal works correctly with PBE algorithms
- key wrapper works correctly with PBEKey
- SecretKeyFactory.translateKey() method works

JDK Issue: https://bugs.openjdk.java.net/browse/JDK-8041781
Webrev: http://cr.openjdk.java.net/~rhalade/8041781/webrev.00/

Thanks,
Rajan

[JDK-9] RFR: 8041787: Need new regressions tests for buffer handling for PBE algorithms

[Rajan Halade]

May I request you to review these 4 new tests to be added for PBE keys. 
New tests are added to address following:


- same buffer can be used for encrypt and decrypt with PBE
- Mac update works correctly with different size of ByteBuffer
- doFinal and update operation result in same PBMac
- for PBEWithMD5AndDES cipher, only CBC mode and PKCS#5Padding is allowed

JDK Issue: https://bugs.openjdk.java.net/browse/JDK-8041787
Webrev: http://cr.openjdk.java.net/~rhalade/8041787/webrev.00/

Thanks,
Rajan

RFR: 8060103: CheckBlacklistedCerts.java thinks its openjdk build

[Rajan Halade]

May I request you to review small fix to CheckBlacklistedCerts test to 
correctly determine if OpenJDK build is used.


Webrev: http://cr.openjdk.java.net/~rhalade/8060103/webrev.00/
Bug: https://bugs.openjdk.java.net/browse/JDK-8060103

Thanks,
Rajan

[9] RFR: 8048601: Tests for JCE crypto ciphers

[Rajan Halade]

May I request you to review new tests added to check Cipher operations 
with different algorithms and modes.


Webrev: http://cr.openjdk.java.net/~rhalade/8048601/webrev.00/
Bug: https://bugs.openjdk.java.net/browse/JDK-8048601

Thanks,
Rajan

[9] RFR: 8049237: Need new tests for X509V3 certificates

[Rajan Halade]

May I request you to review two new tests added. V1toV3Cert tries to 
covert a existing V1 certificate to V3 and V3Certificate test tries to 
generate V3 certificate with different extensions.


Webrev: http://cr.openjdk.java.net/~rhalade/8049237/webrev.00/
Bug: https://bugs.openjdk.java.net/browse/JDK-8049237

Thanks,
Rajan

[9] RFR: 8048624: Tests for SealedObject

[Rajan Halade]

May I request you to review two new tests added to check SealedObject 
with different ciphers.


Bug: https://bugs.openjdk.java.net/browse/JDK-8048624
Webrev: http://cr.openjdk.java.net/~rhalade/8048624/webrev/

Thanks,
Rajan

[9] RFR: 8130031: Remove the intermittent keyword for this test

[Rajan Halade]

Please help me with your review for this one line fix to remove 
intermittent keyword. The test is pretty stable now.


Bug: https://bugs.openjdk.java.net/browse/JDK-8130031
Webrev: http://cr.openjdk.java.net/~rhalade/8130031/webrev.00/

Thanks,
Rajan

Re: [9] RFR: 8048624: Tests for SealedObject

[Rajan Halade]

Hi Valerie,

I have updated the webrev after your comment and our discussion to only 
retain TestSealedObjectNull test.


http://cr.openjdk.java.net/~rhalade/8048624/webrev.01/

Thanks,
Rajan
On 7/23/15 2:20 PM, Valerie Peng wrote:



- I think it's more universal to call getParameters() instead of 
getIV(). Otherwise, if the test is enhanced with GCM mode, it will not 
work.
- Certain combination of mode and padding require certain input 
length. With SealedObject, the input to the Cipher object is the 
"serialized" bytes. Otherwise, IllegalBlockSizeException will be 
thrown. If the test coverage is about SealedObject code, we don't need 
these many different combinations. What is the aim for coverage here?
- DES, DESede and Blowfish and no AES? Note that AES block size is 16 
bytes, so the current input will need to be adjusted. Currently, the 
serialized form is 24 bytes. You need to bump it up to at least 32 
bytes to avoid IllegalBlockSizeException.



- This is not really testing NullCipher. If you replace NullCipher 
with any other cipher, this test would still pass.
- Well, in reality, no one will ever use SealedObject with NullCipher, 
I can't think of a reason to. I wonder if anyone actually uses 
NullCipher. What is the purpose of this test?


Thanks,
Valerie

On 7/17/2015 4:21 PM, Rajan Halade wrote:
May I request you to review two new tests added to check SealedObject 
with different ciphers.


Bug: https://bugs.openjdk.java.net/browse/JDK-8048624
Webrev: http://cr.openjdk.java.net/~rhalade/8048624/webrev/

Thanks,
Rajan

Re: [9] RFR: 8049237: Need new tests for X509V3 certificates

[Rajan Halade]

On 8/25/15 10:09 AM, Sean Mullan wrote:

On 08/18/2015 09:01 PM, Xuelei Fan wrote:

Hi Rajan,

Sorry for the delay.  The code looks fine to me.  I was just wondering,
what's the purpose for the V1toV3Cert.java test?  Is it practice 
behavior?


I had the same question. I would remove this test as it seems like a 
obscure case not supported by any of our tools/APIs.
This test tries to convert V1 to V3 certificate. Ok, I removed this test 
case.


http://cr.openjdk.java.net/~rhalade/8049237/webrev.01/

Thanks,
Rajan


--Sean


Thanks,
Xuelei

On 7/15/2015 9:37 AM, Rajan Halade wrote:

May I request you to review two new tests added. V1toV3Cert tries to
covert a existing V1 certificate to V3 and V3Certificate test tries to
generate V3 certificate with different extensions.

Webrev: http://cr.openjdk.java.net/~rhalade/8049237/webrev.00/
Bug: https://bugs.openjdk.java.net/browse/JDK-8049237

Thanks,
Rajan

Re: [9] RFR: 8048601: Tests for JCE crypto ciphers

[Rajan Halade]

Hi Valerie,

Thanks for comments! I have fixed all of them.

http://cr.openjdk.java.net/~rhalade/8048601/webrev.01/

- Rajan

On 8/20/15 4:19 PM, Valerie Peng wrote:

Rajan,

Here are some comments for the other half of the webrev:
TestCipherTextLength.java
1) line 82 and 85, why TEXT_LEN is needed? I don't find any usage of 
it inside runAll(..) method?

removed.
2) the names here are a bit too similar, e.g. PBEWrapper, 
PBECipherWrapper, AESPBEWrapper. The individual subclasses aren't that 
big, why not make them inner static classes of the parent class? No 
need to repeat the Wrapper and PBE in the subclass names. To match 
your other wrapper class DESCipherWrapper, perhaps you can rename the 
PBE counterpart to something like:


PBECipherWrapper (parent) with subclasses (defined statically 
inside PBECipherWrapper) named PBKDF2, AES, and Legacy (these are 
PKCS#5 v1.5 defined algorithms).

fixed, thanks!


PBKDF2Wrapper.java
1) typo on line 43 and 52 - should be T_R_ANSFORMATION

fixed
2) Instead of "PKDF2_DEFAULT_KEY_LEN" (typo? I think u meant PBKDF2), 
it should really be CIPHER_KEY_SIZE since the purpose here is to 
generate keys for the Cipher object.

renamed the variable.
3) the key that you generated on line 57 can be viewed as an 
intermediate key. You could have moved the processing in 
initCipher(...) method into the constructor and store the resulting 
cipher key into 'key'. For example:

SecretKey derivedKey = keyFactory.generateSecret(pbeKeySpec);
key = new SecretKeySpec(derivedKey.getEncoded(), KEY_ALGORITHM);

key is generated in consturctor.


Thanks,
Valerie

On 8/19/2015 3:38 PM, Valerie Peng wrote:

Rajan,

TestCipher.java
1) this class is used as the base for many algorithm, modes, and 
paddings. But then it hardcodes some values or parameter types which 
may not work against some algorithms, e.g. IvParameterSpec with 
8-byte value, padding length being 8 bytes. PAD_LEN=8 vs 
PAD_BYTES=16, is this difference intentional?
Seems to me that this TestCipher class only works against Ciphers w/ 
8-byte blocks and uses iv. I think it's clearer to have a constant 
named BLKSIZE instead of just 8.

Yes. I have changed iv to be final variable now initialized in constructor.
2) Also, u don't need to repeatedly generate random bytes for input 
data, IV, etc. The input data can be generated once outside of loop. 
As for the parameters, either u can just let the cipher object 
generates it or grab the IV values from whatever data that u have 
already, e.g. certain part of the input data.

fixed, no need for random anymore.
2) input length is always 800. Would be nice to have a comment 
stating the requirement, e.g. multiple of blocks in order to work 
against ciphers w/ NoPadding
3) line 54, Would be nice to clarify that the range of tested key 
size is from MINIMUM_KEY_SIZE to what returned by 
Cipher.getMaxAllowedKeyLength(ALGORITHM) with the increment of 
KEYCUTTER.

added comments.
4) line 66, keyStrength is better phrased as variousKeySize in my 
opinion.

renamed to variousKeySize.
5) MINIMUM_KEY_SIZE and KEYCUTTER should be algorithm specific. It'd 
be clearer to explicitly list out the algorithms which works against 
the the hardcoded values for the sake of maintenance.

added comments.
6) It looks to me that String[] modes and String[] paddings could be 
moved to runAll(). This seems more consistent with the signature of 
runTest() where mode and padding are again specified here.
I left it as is and changed runTest to private method. Either is ok with 
me.


Thanks,
Rajan


Will send u comments regarding the TestLength ones in a separate email.
Thanks,
Valerie

On 7/14/2015 1:27 PM, Rajan Halade wrote:
May I request you to review new tests added to check Cipher 
operations with different algorithms and modes.


Webrev: http://cr.openjdk.java.net/~rhalade/8048601/webrev.00/
Bug: https://bugs.openjdk.java.net/browse/JDK-8048601

Thanks,
Rajan

Re: [9] RFR: 8049237: Need new tests for X509V3 certificates

[Rajan Halade]

Hi Sean,

On 8/26/15 8:39 AM, Sean Mullan wrote:
This looks ok now, although I would probably eliminate the tests for 
SHA-1 and bump up the keysize to 2048, so that this test won't have to 
be updated when those algorithms/keysizes are constrained in the future.
I would take up other enhancements later. For now, I have updated test 
to reset jdk.tls.disabledAlgorithms to remove constraints.


http://cr.openjdk.java.net/~rhalade/8049237/webrev.02

Thanks,
Rajan


--Sean

On 08/25/2015 02:30 PM, Rajan Halade wrote:



On 8/25/15 10:09 AM, Sean Mullan wrote:

On 08/18/2015 09:01 PM, Xuelei Fan wrote:

Hi Rajan,

Sorry for the delay.  The code looks fine to me.  I was just 
wondering,

what's the purpose for the V1toV3Cert.java test?  Is it practice
behavior?


I had the same question. I would remove this test as it seems like a
obscure case not supported by any of our tools/APIs.

This test tries to convert V1 to V3 certificate. Ok, I removed this test
case.

http://cr.openjdk.java.net/~rhalade/8049237/webrev.01/

Thanks,
Rajan


--Sean


Thanks,
Xuelei

On 7/15/2015 9:37 AM, Rajan Halade wrote:

May I request you to review two new tests added. V1toV3Cert tries to
covert a existing V1 certificate to V3 and V3Certificate test 
tries to

generate V3 certificate with different extensions.

Webrev: http://cr.openjdk.java.net/~rhalade/8049237/webrev.00/
Bug: https://bugs.openjdk.java.net/browse/JDK-8049237

Thanks,
Rajan

Re: [9] RFR: 8049237: Need new tests for X509V3 certificates

[Rajan Halade]

On 8/27/15 4:43 AM, Sean Mullan wrote:
I don't think the different signature variants buy you much, since 
what you are mainly testing here is the ability to parse the created 
certificate without errors. I would simplify the test by just having a 
single test for each key type.


--Sean 

Test updated, see http://cr.openjdk.java.net/~rhalade/8049237/webrev.03/

Thanks,
Rajan

[9] RFR: 8048356: SecureRandom default provider tests

[Rajan Halade]

Please help with your review of this new test to check default provider 
used with SecureRandom.


Bug: https://bugs.openjdk.java.net/browse/JDK-8048356
Webrev: http://cr.openjdk.java.net/~rhalade/8048356/webrev.00/

Thanks,
Rajan

Re: security/infra/java/security/cert/CertPathValidator/certification/QuoVadisCA.java fails in jdk

[Rajan Halade]

We found an issue with revocation of intermedicate CA issued by QuoVadis 
which is fixed by CA now. I am waiting to get new test certificates to 
fix this bug.


Thanks,
Rajan

On 9/18/18 3:06 AM, Lindenmaier, Goetz wrote:

Hi Rajan,

are you working on this issue?  We have this test disabled, I would like to run 
it again.

Best regards,
   Goetz.


-Original Message-
From: security-dev  On Behalf Of
Rajan Halade
Sent: Mittwoch, 11. Juli 2018 17:42
To: Baesken, Matthias ; security-
d...@openjdk.java.net
Subject: Re:
security/infra/java/security/cert/CertPathValidator/certification/QuoVadisC
A.java fails in jdk

Thanks for the report, I have filed JDK-8207059. Yes, the test certificate used
here need to be updated.

- Rajan


On 7/11/18 7:43 AM, Baesken, Matthias wrote:




... and in stdout  there is  a better message that seems to show the
reason ,   a certificate is expected to  be "GOOD"   but it  has been  revoked
Fri Jan 19 15:39:57 CET 2018   .

Should  the  test   be updated  with  more   recent  certs   (seems they
are hardwired in the test java source) ?

Re: RFR [12]: 8195793: Remove GTE CyberTrust Global Root

[Rajan Halade]

Looks good to me!

Thanks,
Rajan

On 10/18/18 8:04 AM, Sean Mullan wrote:
Please review this change to remove the GTE CyberTrust Global Root 
from the cacerts keystore. This root is expired and all certificates 
that chain back to this root have expired.


Note that retaining roots past their expiration date may make sense in 
some cases. For example, if we removed a root it could break signed 
code that had been previously timestamped. It may make sense to allow 
for a transition period for those apps to be signed and re-deployed 
using new certificates.


However, this is much less of a risk going forward. Applets have been 
deprecated since JDK 9 and WebStart apps are not supported as of 
(Oracle) JDK 11. These were the primary use cases for signed and 
timestamped code that I am aware of.


webrev: http://cr.openjdk.java.net/~mullan/webrevs/8195793/webrev.00/

Thanks,
Sean

RFR: 8207059: Update test certificates in QuoVadisCA.java test

[Rajan Halade]

Please review this fix to update test certificates used in QuoVadis CA 
tests.


Webrev: http://cr.openjdk.java.net/~rhalade/8207059/webrev.00/

Thanks,
Rajan

Re: jdk11u and jdk/jdk : jtreg test error in security/infra/java/security/cert/CertPathValidator/certification/ActalisCA.java

[Rajan Halade]

I will address these after this winter break or during if I get time.

Thanks,
Rajan

On 12/19/18 2:28 AM, Lindenmaier, Goetz wrote:

Hi,

the amount of tests failing is increasing:
   
security/infra/java/security/cert/CertPathValidator/certification/ActalisCA.java
   
security/infra/java/security/cert/CertPathValidator/certification/ComodoCA.java
   
security/infra/java/security/cert/CertPathValidator/certification/BuypassCA.java
or did I miss a fix for ActalisCA?

ByupassCA.java says "Certificate has been revoked, reason: UNSPECIFIED, revocation 
date: Mon Dec 17"

We would appreciate a fix in jdk 11, 12 and jdk/jdk.

Maybe similar to "8207059: Update test certificates in QuoVadisCA.java test"?

And maybe you can check whether other tests need updates in the very near 
future, too?
E.g., before the jdk12 release date?

Best regards,
   Goetz.





-Original Message-
From: Sean Mullan 
Sent: Dienstag, 27. November 2018 13:48
To: Baesken, Matthias ; security-
d...@openjdk.java.net
Cc: Lindenmaier, Goetz ; Sibabrata Sahoo

Subject: Re: jdk11u and jdk/jdk : jtreg test error in
security/infra/java/security/cert/CertPathValidator/certification/ActalisCA.ja
va

On 11/27/18 7:36 AM, Baesken, Matthias wrote:

Hi Sean ,  maybe I was not clear enough   -   when I  said  " but we only see

the issue  since last week"I meant we  started to see the issue  in our 
local
jtreg  test runs

   for jdk11u  and for jdk/jdk  .

Oh, I see. I have not looked at this issue myself. Maybe Siba (the RE)
can offer some suggestions.


Do you think

security/infra/java/security/cert/CertPathValidator/certification/ActalisCA.ja
va   should be set on the exclude list  for now  (until  JDK-8202651 is 
resolved)
?

Sure, if Siba does not have any plans to fix this soon.

--Sean



Best regards, Matthias


-Original Message-
From: Sean Mullan 
Sent: Montag, 26. November 2018 15:57
To: Baesken, Matthias ; security-
d...@openjdk.java.net
Cc: Lindenmaier, Goetz 
Subject: Re: jdk11u and jdk/jdk : jtreg test error in


security/infra/java/security/cert/CertPathValidator/certification/ActalisCA.ja

va

On 11/26/18 8:44 AM, Baesken, Matthias wrote:

Hi Sean thanks for the info .

Strange that  the bug   https://bugs.openjdk.java.net/browse/JDK-

8202651

is from  May  ,  but we only see the issue  since last week ?!

It was marked Confidential for some reason, possibly by accident because
these tests used to be closed before we open sourced them in JDK 10. I
opened up the bug today.

--Sean

Re: RFR 8180573: Refactor sun/security/tools shell tests to plain java tests

[Rajan Halade]

Review took longer than I planned. Your fix looks good.

In addition to conversion to java, fix has good enhancements needed to 
tests. For instance - changing non-default digest algorithm to SHA-1 
from SHA-256 for AlgOptions.sh. Thanks for these changes.


Thanks,
Rajan

On 3/25/19 8:20 PM, Weijun Wang wrote:

Ding-dong.


On Mar 5, 2019, at 8:26 AM, Weijun Wang  wrote:

Webrev updated at

   https://cr.openjdk.java.net/~weijun/8180573/webrev.01

BTW, last time I mistakenly removed ExportPrivateKeyNoPwd.java which is used by 
ListKeychainStore.sh. It's now back.

Thanks,
Max


On Feb 15, 2019, at 9:31 PM, Weijun Wang  wrote:

Hi Philipp,

In most cases, it's just about creating a non-empty file; in some case, the content is 
relevant. For the former, I will change it to something like "new byte[10]"; 
for the latter, I'll use getBytes(UTF_8).

Thanks,
Max



On Feb 15, 2019, at 5:34 PM, Philipp Kunz  wrote:

Hi Max,

I don't know if it is important enough, certainly not a serious issue.
In your patch, for example in DiffEnd.java and a few other tests, Strings are 
encoded to byte streams with String.getBytes() which uses the default platform 
character set to encode the strings.
Manifests, however, always use UTF-8 as the character set to encode. In my 
opinion, getBytes(java.nio.charset.StandardCharsets.UTF_8) would be appropriate 
to specify the encoding in a platform-independent way.
Now the manifests used in the tests use so few different characters that this 
might not even make a real difference because the first around 100 characters 
or so of most character sets are the same in most encodings.
I suspect that the encoding might also have been platform-dependent in at least 
some of the previous shell tests and therefore this aspect might as well be 
addressed separately and is not strictly necessary to just properly convert 
shell tests to java.

Regards,
Philipp


On Wed, 2019-02-13 at 11:01 +0800, Weijun Wang wrote:

Please review the fix at


https://cr.openjdk.java.net/~weijun/8180573/webrev.00/


Notes:

- Most changes are just .sh -> .java

- StorePasswordsByShell.sh combined into StorePasswords.java

- In most cases, JarUtils is called to create a jar file. Sometimes the jar 
command is called because of delicate differences, for example, jar adds 
directory entries also.

- The i18n tests are completely manual described in i18n.html. Old i18n.java is 
useless, now is also empty.

- Copyright year updated to 2019, @bug unchanged.

Two files not converted yet:

- ./keytool/console.sh

This is a manual test calling old versions of JDK.

- ./keytool/ListKeychainStore.sh

I tried on this one but "security list-keychains -s ..." has no effect on mach5 
machines when calling by ProcessTools. No idea why, I've created a separate bug 
(JDK-8218886) for it.

Thanks,
Max

Re: RFR [backport to jdk and 11u]: 8216577: Add GlobalSign's R6 Root certificate

[Rajan Halade]

Thanks Christoph for this. I have pushed fix to jdk/jdk repository.

- Rajan

On 4/29/19 12:31 AM, Langer, Christoph wrote:


Hi,

the change to add GlobalSign's R6 Root certificate has only been 
pushed to jdk12 updates (12.0.1) so far. According to [0], it was an 
oversight to not have it added to jdk/jdk. I also want to bring it to 11u.


Taking the change to both, jdk/jdk and jdk-updates/jdk11u-dev does not 
apply cleanly. test/jdk/sun/security/lib/cacerts/VerifyCACerts.java 
fails to resolve because of changed license header and different bug 
ids in the @bug tag. So, please review the resolved change. It's the 
same for both repos.


Webrev jdk/jdk: 
http://cr.openjdk.java.net/~clanger/webrevs/8216577.jdk/ 
<http://cr.openjdk.java.net/~clanger/webrevs/8216577.jdk/>


Webrev jdk11u-dev: 
http://cr.openjdk.java.net/~clanger/webrevs/8216577.jdk11u-dev/


Bug: https://bugs.openjdk.java.net/browse/JDK-8216577 
<https://bugs.openjdk.java.net/browse/JDK-8216577>


Please review.

@Rajan Halade <mailto:rajan.hal...@oracle.com>: please let me know 
whether I shall push it to jdk/jdk or feel free to do it yourself.


Thanks & Best regards

Christoph

[0] 
https://mail.openjdk.java.net/pipermail/security-dev/2019-April/019766.html 

RFR: 8222137: Remove T-Systems root CA certificate

[Rajan Halade]

Please review this fix for removal of T-Systems Deutsche Telekom Root CA 
2 certificate.


Webrev: http://cr.openjdk.java.net/~rhalade/8222137/webrev.00/

Release note is at - https://bugs.openjdk.java.net/browse/JDK-8223161

Thanks,
Rajan

Re: RFR: 8222137: Remove T-Systems root CA certificate

[Rajan Halade]

I have created needed backports including 12-pool, 11-pool, 8-pool, and 
7-pool. These are assigned to Sean C. for now. Please co-ordinate with him.


Thanks,
Rajan

On 5/1/19 6:19 AM, Langer, Christoph wrote:


Hi Rajan,

I’ll take this to jdk11 updates. What about jdk12 updates? I can 
process both releases, if you want.


Best regards

Christoph

*From:*security-dev  *On Behalf 
Of *Rajan Halade

*Sent:* Dienstag, 30. April 2019 20:39
*To:* Sean Mullan ; security-dev 


*Subject:* RFR: 8222137: Remove T-Systems root CA certificate

Please review this fix for removal of T-Systems Deutsche Telekom Root 
CA 2 certificate.


Webrev: http://cr.openjdk.java.net/~rhalade/8222137/webrev.00/

Release note is at - https://bugs.openjdk.java.net/browse/JDK-8223161

Thanks,
Rajan

RFR: 8222136: Remove two Comodo root CA certificates that are expiring

[Rajan Halade]

Please review this fix for removal of two Comodo root CAcertificates.

Webrev: http://cr.openjdk.java.net/~rhalade/8222136/webrev.00/

Release note is at - https://bugs.openjdk.java.net/browse/JDK-8223976

Thanks,
Rajan

RFR: 8223499: Remove two DocuSign root certificates that are expiring

[Rajan Halade]

Please review this fix for removal of two DocuSign root CAcertificates.

Webrev: http://cr.openjdk.java.net/~rhalade/8223499/webrev.00/

Release note is at - https://bugs.openjdk.java.net/browse/JDK-8224004

Thanks,
Rajan

RFR: 8202651: Test ActalisCA.java and ComodoCA fails

[Rajan Halade]

Please review this fix to update test certificates used in Actalis and 
Comodo CA interop tests. The bug also mentioned QuoVadisCA test but I am 
not able to reproduce the failure. For Actalis CA, I couldn't get 
revoked test certificate but the test is updated for valid certificate 
and will pass now by skipping expired revoked chain.


Webrev: http://cr.openjdk.java.net/~rhalade/8202651/webrev.00/
Bug: https://bugs.openjdk.java.net/browse/JDK-8202651

Thanks,
Rajan

Re: RFR: 8202651: Test ActalisCA.java and ComodoCA fails

[Rajan Halade]

On 5/22/19 8:39 AM, Sean Mullan wrote:

On 5/21/19 5:31 PM, Rajan Halade wrote:
Please review this fix to update test certificates used in Actalis 
and Comodo CA interop tests. The bug also mentioned QuoVadisCA test 
but I am not able to reproduce the failure. For Actalis CA, I 
couldn't get revoked test certificate but the test is updated for 
valid certificate and will pass now by skipping expired revoked chain.


It looks like the test is still expecting a revoked status - is that 
still working because the IntCA is revoked?:

It is working because revoked certificate is expired, test is skipped then.


 232 // Validate Revoked
 233 pathValidator.validate(new String[]{REVOKED, INT_REVOKED},
 234 ValidatePathWithParams.Status.REVOKED,
 235 "Fri Jan 29 01:06:42 PST 2016", System.out);
 236

It should be ok if the revoked certificate is expired though as you 
can set the validation date to the past (within the interval where the 
certificate is still valid).
Or is it because the Actalis OCSP responder is no longer reporting 
that the certificate is revoked?
Earlier test had past validation with OCSP but for some time now OCSP is 
returning UNKNOWN status instead of REVOKED. This could be an issue 
depending on how implementation treats UNKNOWN status. We will have to 
follow up with CA to check on policy - Is this only happening because we 
are using test certificate or is it a policy?


Thanks,
Rajan


--Sean



Webrev: http://cr.openjdk.java.net/~rhalade/8202651/webrev.00/
Bug: https://bugs.openjdk.java.net/browse/JDK-8202651

Thanks,
Rajan

Re: RFR: 8202651: Test ActalisCA.java and ComodoCA fails

[Rajan Halade]

On 5/22/19 9:34 AM, Sean Mullan wrote:

On 5/22/19 12:04 PM, Rajan Halade wrote:

On 5/22/19 8:39 AM, Sean Mullan wrote:

On 5/21/19 5:31 PM, Rajan Halade wrote:
Please review this fix to update test certificates used in Actalis 
and Comodo CA interop tests. The bug also mentioned QuoVadisCA test 
but I am not able to reproduce the failure. For Actalis CA, I 
couldn't get revoked test certificate but the test is updated for 
valid certificate and will pass now by skipping expired revoked chain.


It looks like the test is still expecting a revoked status - is that 
still working because the IntCA is revoked?:
It is working because revoked certificate is expired, test is skipped 
then.


Have you asked Actalis for a new revoked test certificate? If you 
can't get one, I would remove the revoked certificates and the test 
for it then, since you are not testing that behavior anymore and that 
is not apparent from the test right now.

I will follow up with CA then and leave this bug open for now.


Also do you know why the revocation check for the intermediate CA is 
not working?
Revocation check on intermediate CA is working fine. INT_REVOKED is a 
good certificate, may name is misleading. INT_REVOKED here means that 
this is a intermediate CA for revoked EE certificate.


Thanks,
Rajan




 232 // Validate Revoked
 233 pathValidator.validate(new String[]{REVOKED, INT_REVOKED},
 234 ValidatePathWithParams.Status.REVOKED,
 235 "Fri Jan 29 01:06:42 PST 2016", System.out);
 236

It should be ok if the revoked certificate is expired though as you 
can set the validation date to the past (within the interval where 
the certificate is still valid).
Or is it because the Actalis OCSP responder is no longer reporting 
that the certificate is revoked?
Earlier test had past validation with OCSP but for some time now OCSP 
is returning UNKNOWN status instead of REVOKED. This could be an 
issue depending on how implementation treats UNKNOWN status. We will 
have to follow up with CA to check on policy - Is this only happening 
because we are using test certificate or is it a policy?


It depends, if it is a TLS certificate then it is usually acceptable 
to report the revoked certificate as UNKNOWN after it expires since 
you should not be trusting expired TLS certificates. For a code 
signing certificate, it is better to retain the REVOKED status for a 
longer time period after it expires since it may still be in use (for 
example, in a timestamped application).


--Sean



Thanks,
Rajan


--Sean



Webrev: http://cr.openjdk.java.net/~rhalade/8202651/webrev.00/
Bug: https://bugs.openjdk.java.net/browse/JDK-8202651

Thanks,
Rajan

Re: RFR(S): 8224727: Problem list 2 tests in security/infra/java/security/cert/CertPathValidator/certification

[Rajan Halade]

I have pushed fix for ComodoCA with JDK-8202651 and have filed 
JDK-8224768 to address ActalisCA issue. You can problem list ActalisCA 
for now with JDK-8224727.


Thanks,
Rajan

On 5/24/19 6:43 AM, Sean Mullan wrote:

On 5/24/19 4:00 AM, Langer, Christoph wrote:

Hi,

please review the problem listing of

security/infra/java/security/cert/CertPathValidator/certification/ActalisCA.java 
and


security/infra/java/security/cert/CertPathValidator/certification/ComodoCA.java 



until JDK-8202651 and JDK-8215546 are resolved.

Bug: https://bugs.openjdk.java.net/browse/JDK-8224727

Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8224727.0/


I would prefer to just problem list ActalisCA.java. Rajan has a fix 
for ComodoCA which he posted the other day as part of 
https://bugs.openjdk.java.net/browse/JDK-8202651, so if he just splits 
that off into a separate bug then he can push that fix. Can you 
coordinate with Rajan the best way to handle this?


There seems to be overlap in these various bugs for the failures in 
Actalis, Buypass and Comodo - I think we should clean this up so that 
there is one unique bug per failing test.


--Sean

Re: RFR(S): 8224727: Problem list 2 tests in security/infra/java/security/cert/CertPathValidator/certification

[Rajan Halade]

Looks good, thanks!

- Rajan

On 5/24/19 10:50 PM, Langer, Christoph wrote:

Hi Rajan,

thanks for this. Problem list update would be then:
http://cr.openjdk.java.net/~clanger/webrevs/8224727.1/

Please review.

Thanks
Christoph



-Original Message-
From: Rajan Halade 
Sent: Freitag, 24. Mai 2019 18:52
To: Langer, Christoph 
Cc: Sean Mullan ; security-dev 
Subject: Re: RFR(S): 8224727: Problem list 2 tests in
security/infra/java/security/cert/CertPathValidator/certification

I have pushed fix for ComodoCA with JDK-8202651 and have filed
JDK-8224768 to address ActalisCA issue. You can problem list ActalisCA
for now with JDK-8224727.

Thanks,
Rajan

On 5/24/19 6:43 AM, Sean Mullan wrote:

On 5/24/19 4:00 AM, Langer, Christoph wrote:

Hi,

please review the problem listing of



security/infra/java/security/cert/CertPathValidator/certification/ActalisCA.ja
va

and



security/infra/java/security/cert/CertPathValidator/certification/ComodoCA.
java


until JDK-8202651 and JDK-8215546 are resolved.

Bug: https://bugs.openjdk.java.net/browse/JDK-8224727

Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8224727.0/

I would prefer to just problem list ActalisCA.java. Rajan has a fix
for ComodoCA which he posted the other day as part of
https://bugs.openjdk.java.net/browse/JDK-8202651, so if he just splits
that off into a separate bug then he can push that fix. Can you
coordinate with Rajan the best way to handle this?

There seems to be overlap in these various bugs for the failures in
Actalis, Buypass and Comodo - I think we should clean this up so that
there is one unique bug per failing test.

--Sean

RFR: 8228337: problemList failing/ignored manual tests in security-libs

[Rajan Halade]

Please review this problemList update to add earlier ignored manual regression 
tests. These tests require special setup/hardware and few have outdated JDK and 
need update.

webrev: http://cr.openjdk.java.net/~rhalade/8228337/webrev.00/ 


Thanks,
Rajan

RFR: 8228755: ProblemList sun/security/tools/jarsigner/Warning.java

[Rajan Halade]

Please review this patch to problem list Warning.java. I am not able to 
reproduce the failure and will require more time to find root cause and fix.

diff -r f459f98aa30d test/jdk/ProblemList.txt
--- a/test/jdk/ProblemList.txt  Mon Jul 29 20:18:43 2019 +
+++ b/test/jdk/ProblemList.txt  Mon Jul 29 15:03:16 2019 -0700
@@ -661,6 +661,8 @@
 sun/security/tools/jarsigner/warnings/BadKeyUsageTest.java  8026393 
generic-all
 sun/security/tools/jarsigner/PreserveRawManifestEntryAndDigest.java  8228431 
solaris-all
 
+sun/security/tools/jarsigner/Warning.java   8228745 
generic-all
+
 javax/net/ssl/ServerName/SSLEngineExplorerMatchedSNI.java   8212096 
generic-all
 javax/net/ssl/DTLS/PacketLossRetransmission.java8169086 
macosx-x64
 javax/net/ssl/DTLS/RespondToRetransmit.java 8169086 
macosx-x64

Thanks,
Rajan

RFR: 8231887: ComodoCA.java fails because certificate was revoked

[Rajan Halade]

Please review this fix in ComodoCA.java test to update test certificates used. 
CA revoked earlier test artifacts so we need to update those.

Webrev: http://cr.openjdk.java.net/~rhalade/8231887/webrev.00/ 


Thanks,
Rajan

RFR: 8232019: Add LuxTrust certificate updates to the existing root program

[Rajan Halade]

May I request you to review this root certificate update to add “LuxTrust 
Global Root 2” CA to cacerts.

Webrev: http://cr.openjdk.java.net/~rhalade/8232019/webrev.00/ 


Thanks,
Rajan

RFR: 8233223: Add Amazon Root CA certificates

[Rajan Halade]

May I request you to review this fix to add 4 new Amazon root certificates to 
cacerts.

Webrev: http://cr.openjdk.java.net/~rhalade/8233223/webrev.00/ 


Thanks,
Rajan

RFR: 8190492: Remove SSLv2Hello and SSLv3 from default enabled TLS protocols

[Rajan Halade]

May I request you to review following fix which removes SSLv2Hello and SSLv3 
from default enabled protocols. 

SSLv3 has been deprecated with RFC 7568. We have already disabled it by default 
in 2015 by adding it to the jdk.tls.disabledAlgorithms property. This fix 
removes it from default enabled list as well. If client/server want to use this 
protocol they can still do so by enabling it with setEnabledProtocols() API.

Webrev: http://cr.openjdk.java.net/~rhalade/8190492/webrev.00/ 


Thanks,
Rajan

Re: RFR: 8190492: Remove SSLv2Hello and SSLv3 from default enabled TLS protocols

[Rajan Halade]

The method at line 596 is only called at line 855 from customizedProtocols 
class. It should really be read as getServerDefaultProtocols. Supported 
protocols are returned correctly on SSLEngine, SSLServerSocket, and SSLSocket.

I thought about cleaning/renaming it but then AbstractTLSContext already has 
serverDefaultProtocols and getProtocols() in customizedProtocols can also be 
moved out. I will file a separate bug to handle these cleanups.
 
Thanks,
Rajan

> On Dec 4, 2019, at 5:18 PM, Bradford Wetmore  
> wrote:
> 
> In line 601, doesn't this mean that SSL3/SSL20Hello are not longer available 
> as supported, and that you can't turn them back on?
> 
> Brad
> 
> 
> On 12/4/2019 1:19 PM, Rajan Halade wrote:
>> May I request you to review following fix which removes SSLv2Hello and SSLv3 
>> from default enabled protocols.
>> SSLv3 has been deprecated with RFC 7568. We have already disabled it by 
>> default in 2015 by adding it to the jdk.tls.disabledAlgorithms property. 
>> This fix removes it from default enabled list as well. If client/server want 
>> to use this protocol they can still do so by enabling it with 
>> setEnabledProtocols() API.
>> Webrev: http://cr.openjdk.java.net/~rhalade/8190492/webrev.00/
>> Thanks,
>> Rajan

RFR: 8225128: Add exception for expiring DocuSign root to VerifyCACerts test

[Rajan Halade]

May I request you to review this small update to add keynectisrootca to expiry 
check exempt list. JDK-8225068 will remove this certificate after expiry.

diff -r af8e77a59bd8 test/jdk/sun/security/lib/cacerts/VerifyCACerts.java
--- a/test/jdk/sun/security/lib/cacerts/VerifyCACerts.java  Fri Feb 14 
12:47:18 2020 -0800
+++ b/test/jdk/sun/security/lib/cacerts/VerifyCACerts.java  Fri Feb 14 
13:13:14 2020 -0800
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2017, 2019, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2017, 2020, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -259,6 +259,8 @@
 {
 // Valid until: Tue Jul 09 14:40:36 EDT 2019
 add("utnuserfirstobjectca [jdk]");
+// Valid until: Tue May 26 00:00:00 GMT 2020
+add("keynectisrootca [jdk]");
 }
 };


Thanks,
Rajan

Re: [JDK 15] RFR 8239979: sun/security/tools/keytool/ExtOptionCamelCase.java is not run

[Rajan Halade]

Thanks Amy for the fix! 

Any reason for using othervm to run this test? Otherwise your fix looks good to 
me.

Thanks,
Rajan

> On Feb 25, 2020, at 6:48 PM, Amy Lu  wrote:
> 
> sun/security/tools/keytool/ExtOptionCamelCase.java
> 
> This is not a compile-only test, but due to the missed @run tag, test is not 
> run.
> 
> Please review the patch to fix this issue.
> 
> The patch also fixed @modules. Test performs deep reflection 
> setAccessible(true) on a non-public member, `open` added to the module.
> 
> bug: https://bugs.openjdk.java.net/browse/JDK-8239979 
> 
> webrev: http://cr.openjdk.java.net/~amlu/8239979/webrev.00/index.html 
> 
> 
> Thanks,
> Amy
> 
> 
> --- old/test/jdk/sun/security/tools/keytool/ExtOptionCamelCase.java   
> 2020-02-26 10:36:19.0 +0800
> +++ new/test/jdk/sun/security/tools/keytool/ExtOptionCamelCase.java   
> 2020-02-26 10:36:18.0 +0800
> @@ -1,5 +1,5 @@
>  /*
> - * Copyright (c) 2019, Oracle and/or its affiliates. All rights reserved.
> + * Copyright (c) 2019, 2020, Oracle and/or its affiliates. All rights 
> reserved.
>   * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
>   *
>   * This code is free software; you can redistribute it and/or modify it
> @@ -24,11 +24,13 @@
>  /*
>   * @test
>   * @bug 8231950
> + * @summary keytool -ext camel-case shorthand not working
>   * @modules java.base/sun.security.tools.keytool
> + *  java.base/sun.security.tools.keytool:open
>   *  java.base/sun.security.util
>   *  java.base/sun.security.x509
>   * @compile -XDignore.symbol.file ExtOptionCamelCase.java
> - * @summary keytool -ext camel-case shorthand not working
> + * @run main/othervm ExtOptionCamelCase
>   */
>  
>  import sun.security.tools.keytool.Main;

Re: [JDK 15] RFR 8239979: sun/security/tools/keytool/ExtOptionCamelCase.java is not run

[Rajan Halade]

Thanks! looks good.

Thanks,
Rajan

> On Feb 25, 2020, at 10:24 PM, Amy Lu  wrote:
> 
> Removed othervm: 
> http://cr.openjdk.java.net/~amlu/8239979/webrev.01/index.html 
> <http://cr.openjdk.java.net/~amlu/8239979/webrev.01/index.html>
> 
> Thanks,
> Amy
> 
> On 2/26/20 11:02 AM, Rajan Halade wrote:
>> Thanks Amy for the fix! 
>> 
>> Any reason for using othervm to run this test? Otherwise your fix looks good 
>> to me.
>> 
>> Thanks,
>> Rajan
>> 
>>> On Feb 25, 2020, at 6:48 PM, Amy Lu >> <mailto:amy...@oracle.com>> wrote:
>>> 
>>> sun/security/tools/keytool/ExtOptionCamelCase.java
>>> 
>>> This is not a compile-only test, but due to the missed @run tag, test is 
>>> not run.
>>> 
>>> Please review the patch to fix this issue.
>>> 
>>> The patch also fixed @modules. Test performs deep reflection 
>>> setAccessible(true) on a non-public member, `open` added to the module.
>>> 
>>> bug: https://bugs.openjdk.java.net/browse/JDK-8239979 
>>> <https://bugs.openjdk.java.net/browse/JDK-8239979>
>>> webrev: http://cr.openjdk.java.net/~amlu/8239979/webrev.00/index.html 
>>> <http://cr.openjdk.java.net/~amlu/8239979/webrev.00/index.html>
>>> 
>>> Thanks,
>>> Amy
>>> 
>>> 
>>>  --- old/test/jdk/sun/security/tools/keytool/ExtOptionCamelCase.java
>>> 2020-02-26 10:36:19.0 +0800
>>> +++ new/test/jdk/sun/security/tools/keytool/ExtOptionCamelCase.java 
>>> 2020-02-26 10:36:18.0 +0800
>>> @@ -1,5 +1,5 @@
>>>  /*
>>> - * Copyright (c) 2019, Oracle and/or its affiliates. All rights reserved.
>>> + * Copyright (c) 2019, 2020, Oracle and/or its affiliates. All rights 
>>> reserved.
>>>   * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
>>>   *
>>>   * This code is free software; you can redistribute it and/or modify it
>>> @@ -24,11 +24,13 @@
>>>  /*
>>>   * @test
>>>   * @bug 8231950
>>> + * @summary keytool -ext camel-case shorthand not working
>>>   * @modules java.base/sun.security.tools.keytool
>>> + *  java.base/sun.security.tools.keytool:open
>>>   *  java.base/sun.security.util
>>>   *  java.base/sun.security.x509
>>>   * @compile -XDignore.symbol.file ExtOptionCamelCase.java
>>> - * @summary keytool -ext camel-case shorthand not working
>>> + * @run main/othervm ExtOptionCamelCase
>>>   */
>>>  
>>>  import sun.security.tools.keytool.Main;
>> 
> 

RFR: 8225130: Add exception for expiring Comodo roots to VerifyCACerts test

[Rajan Halade]

May I request you to review this fix to test VerifyCACerts.

diff -r 4a5a7dc9d05c test/jdk/sun/security/lib/cacerts/VerifyCACerts.java
--- a/test/jdk/sun/security/lib/cacerts/VerifyCACerts.java  Sun Mar 01 
17:36:03 2020 -0800
+++ b/test/jdk/sun/security/lib/cacerts/VerifyCACerts.java  Sun Mar 01 
22:42:59 2020 -0800
@@ -261,6 +261,12 @@
 add("utnuserfirstobjectca [jdk]");
 // Valid until: Tue May 26 00:00:00 GMT 2020
 add("keynectisrootca [jdk]");
+// Valid until: Sat May 30 10:38:31 GMT 2020
+add("addtrustexternalca [jdk]");
+// Valid until: Sat May 30 10:48:38 GMT 2020
+add("addtrustclass1ca [jdk]");
+// Valid until: Sat May 30 10:44:50 GMT 2020
+add("addtrustqualifiedca [jdk]");
 }
 };

Thanks,
Rajan

RFR: 8240686: 70 security tests are failing on Windows due to "Fetch artifact failed"

[Rajan Halade]

The fetch artifact issue is fixed within infra so these tests can be removed 
from ProblemList now. Please review this changeset:

diff -r 7af6364e1792 test/jdk/ProblemList.txt
--- a/test/jdk/ProblemList.txt  Fri Mar 06 18:03:09 2020 -0800
+++ b/test/jdk/ProblemList.txt  Fri Mar 06 19:56:26 2020 -0800
@@ -722,77 +722,6 @@
 sun/security/provider/PolicyParser/ExtDirsChange.java   8039280 
generic-all
 sun/security/provider/PolicyParser/PrincipalExpansionError.java 8039280 
generic-all
 
-sun/security/tools/keytool/NssTest.java 8240686 
windows-all
-sun/security/pkcs11/SampleTest.java 8240686 
windows-all
-sun/security/pkcs11/tls/TestPRF.java8240686 
windows-all
-sun/security/pkcs11/tls/TestPremaster.java  8240686 
windows-all
-sun/security/pkcs11/tls/TestLeadingZeroesP11.java   8240686 
windows-all
-sun/security/pkcs11/tls/TestMasterSecret.java   8240686 
windows-all
-sun/security/pkcs11/tls/TestKeyMaterial.java8240686 
windows-all
-sun/security/pkcs11/Signature/TestDSAKeyLength.java 8240686 
windows-all
-sun/security/pkcs11/Signature/TestRSAKeyLength.java 8240686 
windows-all
-sun/security/pkcs11/Signature/TestDSA2.java 8240686 
windows-all
-sun/security/pkcs11/Signature/TestDSA.java  8240686 
windows-all
-sun/security/pkcs11/Signature/SignatureTestPSS.java 8240686 
windows-all
-sun/security/pkcs11/Signature/SigInteropPSS.java8240686 
windows-all
-sun/security/pkcs11/Signature/ReinitSignature.java  8240686 
windows-all
-sun/security/pkcs11/Signature/KeyAndParamCheckForPSS.java   8240686 
windows-all
-sun/security/pkcs11/Signature/InitAgainPSS.java 8240686 
windows-all
-sun/security/pkcs11/Signature/ByteBuffers.java  8240686 
windows-all
-sun/security/pkcs11/Serialize/SerializeProvider.java8240686 
windows-all
-sun/security/pkcs11/SecureRandom/TestDeserialization.java   8240686 
windows-all
-sun/security/pkcs11/SecureRandom/Basic.java 8240686 
windows-all
-sun/security/pkcs11/Secmod/TrustAnchors.java8240686 
windows-all
-sun/security/pkcs11/Secmod/GetPrivateKey.java   8240686 
windows-all
-sun/security/pkcs11/Secmod/LoadKeystore.java8240686 
windows-all
-sun/security/pkcs11/Secmod/TestNssDbSqlite.java 8240686 
windows-all
-sun/security/pkcs11/Secmod/JksSetPrivateKey.java8240686 
windows-all
-sun/security/pkcs11/Secmod/Crypto.java  8240686 
windows-all
-sun/security/pkcs11/Secmod/AddTrustedCert.java  8240686 
windows-all
-sun/security/pkcs11/Secmod/AddPrivateKey.java   8240686 
windows-all
-sun/security/pkcs11/rsa/TestSignatures.java 8240686 
windows-all
-sun/security/pkcs11/rsa/TestKeyPairGenerator.java   8240686 
windows-all
-sun/security/pkcs11/rsa/TestKeyFactory.java 8240686 
windows-all
-sun/security/pkcs11/rsa/TestCACerts.java8240686 
windows-all
-sun/security/pkcs11/rsa/KeyWrap.java8240686 
windows-all
-sun/security/pkcs11/Provider/Login.sh   8240686 
windows-all
-sun/security/pkcs11/Provider/ConfigQuotedString.sh  8240686 
windows-all
-sun/security/pkcs11/MessageDigest/TestCloning.java  8240686 
windows-all
-sun/security/pkcs11/MessageDigest/ReinitDigest.java 8240686 
windows-all
-sun/security/pkcs11/MessageDigest/DigestKAT.java8240686 
windows-all
-sun/security/pkcs11/Mac/MacKAT.java 8240686 
windows-all
-sun/security/pkcs11/Mac/MacSameTest.java8240686 
windows-all
-sun/security/pkcs11/Mac/ReinitMac.java  8240686 
windows-all
-sun/security/pkcs11/MessageDigest/ByteBuffers.java  8240686 
windows-all
-sun/security/pkcs11/KeyPairGenerator/TestDH2048.java8240686 
windows-all
-sun/security/pkcs11/KeyGenerator/TestKeyGenerator.java  8240686 
windows-all
-sun/security/pkcs11/KeyGenerator/DESParity.java 8240686 
windows-all
-sun/security/pkcs11/KeyAgreement/UnsupportedDHKeys.java 8240686 
windows-all
-sun/security/pkcs11/KeyAgreement/TestInterop.java   8240686 
windows-all
-sun/security/pkcs11/KeyAgreement/TestDH.java8240686 
windows-all
-sun/security/pkcs11/KeyAgreement/TestShort.java 8240686 
windows-all
-sun/security/pkcs11/KeyAgreement/SupportedDHKeys.java   8240686 
windows-all
-sun/security/pkcs11/ec/TestECGenSpec.java   8240686 
windows-all
-sun/security/pkcs11/ec/TestECDSA2.java  8240686 
windows-all
-sun/security/pkcs11/ec/TestE

Re: security/infra/java/security/cert/CertPathValidator/certification/GlobalSignR6CA.java jtreg test errors

[Rajan Halade]

Hi Matthias,

I tried several runs of this test but am not able to reproduce the issue. May 
be requests from my tests are routed to different OCSP instance. OCSP responder 
instance can return internalError for inconsistent internal state.

How frequent is the failure for you if you are still seeing it?
 
Thanks,
Rajan

> On Mar 19, 2020, at 4:23 AM, Baesken, Matthias  
> wrote:
> 
> Hello, for a few days we see the test 
> security/infra/java/security/cert/CertPathValidator/certification/GlobalSignR6CA.java
> failing sometimes. The failures are seen not only in jdk/jdk but also in 
> jdk11, that's why we suppose it might be
> some issue with the infrastructure and/or certificate authority ?
>  
> The errors  are like this one  (shows up on different OS platforms) :
> ...
>  
>   Issuer: CN=VeriSign Class 3 Public Primary Certification Authority - G3, 
> OU="(c) 1999 VeriSign, Inc. - For authorized use only", OU=VeriSign Trust 
> Network, O="VeriSign, Inc.", C=US
>   Subject: CN=VeriSign Class 3 Public Primary Certification Authority - G3, 
> OU="(c) 1999 VeriSign, Inc. - For authorized use only", OU=VeriSign Trust 
> Network, O="VeriSign, Inc.", C=US)
> certpath: X509CertSelector.match: subject DNs don't match
> java.lang.RuntimeException: TEST FAILED: couldn't determine EE certificate 
> status
>at 
> ValidatePathWithParams.validate(ValidatePathWithParams.java:177)
>at GlobalSignR6CA.main(GlobalSignR6CA.java:192)
>at 
> java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
>at 
> java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
>at 
> java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
>at java.base/java.lang.reflect.Method.invoke(Method.java:566)
>at 
> com.sun.javatest.regtest.agent.MainWrapper$MainThread.run(MainWrapper.java:127)
>at java.base/java.lang.Thread.run(Thread.java:834)
> Caused by: java.security.cert.CertPathValidatorException: OCSP response 
> error: INTERNAL_ERROR
>at 
> java.base/sun.security.provider.certpath.PKIXMasterCertPathValidator.validate(PKIXMasterCertPathValidator.java:135)
>at 
> java.base/sun.security.provider.certpath.PKIXCertPathValidator.validate(PKIXCertPathValidator.java:237)
>at 
> java.base/sun.security.provider.certpath.PKIXCertPathValidator.validate(PKIXCertPathValidator.java:145)
>at 
> java.base/sun.security.provider.certpath.PKIXCertPathValidator.engineValidate(PKIXCertPathValidator.java:84)
>at 
> java.base/java.security.cert.CertPathValidator.validate(CertPathValidator.java:309)
>at 
> ValidatePathWithParams.doCertPathValidate(ValidatePathWithParams.java:288)
>at 
> ValidatePathWithParams.validate(ValidatePathWithParams.java:142)
>... 7 more
> Caused by: java.security.cert.CertPathValidatorException: OCSP response 
> error: INTERNAL_ERROR
>at 
> java.base/sun.security.provider.certpath.OCSPResponse.verify(OCSPResponse.java:386)
>at 
> java.base/sun.security.provider.certpath.OCSP.check(OCSP.java:195)
>at 
> java.base/sun.security.provider.certpath.RevocationChecker.checkOCSP(RevocationChecker.java:742)
>at 
> java.base/sun.security.provider.certpath.RevocationChecker.check(RevocationChecker.java:362)
>at 
> java.base/sun.security.provider.certpath.RevocationChecker.check(RevocationChecker.java:336)
>at 
> java.base/sun.security.provider.certpath.PKIXMasterCertPathValidator.validate(PKIXMasterCertPathValidator.java:125)
>... 13 more
>  
> Do you notice the issue in your jtreg tests as well ?
>  
> Any hints about this ?
>  
> Thanks, Matthias

Re: RFR[15] JDK-8237977: Further update javax/net/ssl/compatibility/Compatibility.java

[Rajan Halade]

Hi John,

Thanks for taking care of this fix. Your changes look good me. 

I have couple of suggestions:

- consider changing header for last column from “Why negative case” to “Reason”
- for a failed test case (a testcase that succeeds when expected to fail or a 
testcase that fails when expected to succeed) code will not have any reason 
listed and we will need to click hyperlink in the report to find out root 
cause. May I suggest to pass in status to TestCase.negativeCaseReason() method 
call and use reason as “Refer to log at test hyperlink for details…” for such 
cases.

Thanks,
Rajan

> On Mar 24, 2020, at 6:50 PM, sha.ji...@oracle.com wrote:
> 
> Hi,
> This patch updates the manual test 
> javax/net/ssl/compatibility/Compatibility.java on the following points:
> 1. Covers SSLv3
> 2. The server side doesn't limit/specify protocols and cipher suites anymore. 
> Only the client side specifies these parameters.
> 3. Only focus on the testing JDK specified by jtreg option "-jdk", and not 
> run the cases between the JDKs builds in jdkList. This would save much 
> execution time.
> 4. The report has a new column that clarifying why a case is a negative case.
> 
> Issue: https://bugs.openjdk.java.net/browse/JDK-8237977
> Webrev: http://cr.openjdk.java.net/~jjiang/8237977/webrev.00/
> 
> Best regards,
> John Jiang
> 

Re: RFR[15] JDK-8237977: Further update javax/net/ssl/compatibility/Compatibility.java

[Rajan Halade]

Thanks for the update. Changes look good to me.

- Rajan

> On Mar 25, 2020, at 1:48 AM, sha.ji...@oracle.com wrote:
> 
> Hi Rajan,
> Thanks for your suggestions.
> Please review the updated webrev: 
> http://cr.openjdk.java.net/~jjiang/8237977/webrev.01/ 
> <http://cr.openjdk.java.net/~jjiang/8237977/webrev.01/>
> On 2020/3/25 11:08, Rajan Halade wrote:
>> Hi John,
>> 
>> Thanks for taking care of this fix. Your changes look good me. 
>> 
>> I have couple of suggestions:
>> 
>> - consider changing header for last column from “Why negative case” to 
>> “Reason”
>> - for a failed test case (a testcase that succeeds when expected to fail or 
>> a testcase that fails when expected to succeed) code will not have any 
>> reason listed and we will need to click hyperlink in the report to find out 
>> root cause. May I suggest to pass in status to TestCase.negativeCaseReason() 
>> method call and use reason as “Refer to log at test hyperlink for details…” 
>> for such cases.
> A new method TestCase::reason is introduced for the reasons.
> 
> Best regards,
> John Jiang
>> 
>> 
>> Thanks,
>> Rajan
>> 
>>> On Mar 24, 2020, at 6:50 PM, sha.ji...@oracle.com 
>>> <mailto:sha.ji...@oracle.com> wrote:
>>> 
>>> Hi,
>>> This patch updates the manual test 
>>> javax/net/ssl/compatibility/Compatibility.java on the following points:
>>> 1. Covers SSLv3
>>> 2. The server side doesn't limit/specify protocols and cipher suites 
>>> anymore. Only the client side specifies these parameters.
>>> 3. Only focus on the testing JDK specified by jtreg option "-jdk", and not 
>>> run the cases between the JDKs builds in jdkList. This would save much 
>>> execution time.
>>> 4. The report has a new column that clarifying why a case is a negative 
>>> case.
>>> 
>>> Issue: https://bugs.openjdk.java.net/browse/JDK-8237977 
>>> <https://bugs.openjdk.java.net/browse/JDK-8237977>
>>> Webrev: http://cr.openjdk.java.net/~jjiang/8237977/webrev.00/ 
>>> <http://cr.openjdk.java.net/~jjiang/8237977/webrev.00/>
>>> 
>>> Best regards,
>>> John Jiang
>>> 
>> 

RFR: 8225068: Remove DocuSign root certificate that is expiring in May 2020

[Rajan Halade]

Please review this patch to remove Keynectis CA certificate from cacerts store.

Webrev: http://cr.openjdk.java.net/~rhalade/8225068/webrev.00/ 


Thanks,
Rajan

RFR: 8225069: Remove Comodo root certificate that is expiring in May 2020

[Rajan Halade]

Please review this fix to remove expiring Comodo root CA “AddTrust Class 1 CA 
Root”. Other two, AddTrust Qualified CA Root and AddTrust External CA Root, 
will remain since there are code signing certificates issued from those.

Webrev: http://cr.openjdk.java.net/~rhalade/8225069/webrev.00/ 


Thanks,
Rajan

Re: RFR:8246330:Add TLS Tests for Legacy ECDSA curves

[Rajan Halade]

One minor comment -

Can you please put try-catch around "(new DisabledCurve()).run();” instead of 
over for loop?

Otherwise test looks good.

Thanks,
Rajan

> On Jun 4, 2020, at 8:41 PM, shivangi.g.gu...@oracle.com wrote:
> 
> Corrected the links.
> 
> On 04/06/20 10:59 pm, shivangi.g.gu...@oracle.com 
>  wrote:
>> Hi, 
>> 
>> May I please find a sponsor for this patch? 
>> 
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8246330 
>>  
>> 
>> Description:Many of EC named curves has been disabled with security property 
>> "jdk.disabled.namedCurves". The purpose of this Test is to verify the 
>> behavior of any EC named curve from the disabled list. 
>> This Test will address a single name from the disabled list as sect283r1.
>> 
>> 
>> 
>> Webrev: http://cr.openjdk.java.net/~sshivang/reviews/8246330/webrev.00/ 
>> 
>> The patch has been tested on mach5, and the individual test was passed. 
>> 
>> Thanks 
>> 
>> Shivangi
>> 

Re: RFR:8246330:Add TLS Tests for Legacy ECDSA curves

[Rajan Halade]

Looks good to me.

Thanks,
Rajan

> On Jun 8, 2020, at 4:34 AM, shivangi.g.gu...@oracle.com wrote:
> 
> Thanks Rajan. I changed the webrev and ran Mach5 too. Here is the new webrev 
> link: -
> 
> http://cr.openjdk.java.net/~sshivang/reviews/8246330/webrev.01/ 
> <http://cr.openjdk.java.net/~sshivang/reviews/8246330/webrev.01/>
> 
> Thanks
> 
> Shivangi
> 
> 
> 
> On 06/06/20 7:37 am, Rajan Halade wrote:
>> One minor comment -
>> 
>> Can you please put try-catch around "(new DisabledCurve()).run();” instead 
>> of over for loop?
>> 
>> Otherwise test looks good.
>> 
>> Thanks,
>> Rajan
>> 
>>> On Jun 4, 2020, at 8:41 PM, shivangi.g.gu...@oracle.com 
>>> <mailto:shivangi.g.gu...@oracle.com> wrote:
>>> 
>>> Corrected the links.
>>> 
>>> On 04/06/20 10:59 pm, shivangi.g.gu...@oracle.com 
>>> <mailto:shivangi.g.gu...@oracle.com> wrote:
>>>> Hi, 
>>>> 
>>>> May I please find a sponsor for this patch? 
>>>> 
>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8246330 
>>>> <https://bugs.openjdk.java.net/browse/JDK-8246330> 
>>>> 
>>>> Description:Many of EC named curves has been disabled with security 
>>>> property "jdk.disabled.namedCurves". The purpose of this Test is to verify 
>>>> the behavior of any EC named curve from the disabled list. 
>>>> This Test will address a single name from the disabled list as sect283r1.  
>>>>   
>>>> 
>>>> 
>>>> Webrev: http://cr.openjdk.java.net/~sshivang/reviews/8246330/webrev.00/ 
>>>> <http://cr.openjdk.java.net/~sshivang/reviews/8246330/webrev.00/>
>>>> The patch has been tested on mach5, and the individual test was passed. 
>>>> 
>>>> Thanks 
>>>> 
>>>> Shivangi
>>>> 
>> 

RFR: 8243321: Add Entrust root CA - G4 to Oracle Root CA program

[Rajan Halade]

Please review this patch to add “Entrust Root Certification Authority - G4” 
root certificate to cacerts store.

Webrev: http://cr.openjdk.java.net/~rhalade/8243321/webrev.00/ 

Release note: https://bugs.openjdk.java.net/browse/JDK-8250756 


Thanks,
Rajan

RFR: 8243320: Add SSL root certificates to Oracle Root CA program

[Rajan Halade]

Please review this patch to add SSL Corporation issued root CA certificates to 
cacerts.

Webrev: http://cr.openjdk.java.net/~rhalade/8243320/webrev.00/ 

Release Note: https://bugs.openjdk.java.net/browse/JDK-8250860 


Thanks,
Rajan

RFR: 8251859: sun/security/validator/PKIXValAndRevCheckTests.java fails

[Rajan Halade]

Please review this patch to perform backdated certpath check as test 
certificate has expired.

diff -r cb8450f00ee9 
test/jdk/sun/security/validator/PKIXValAndRevCheckTests.java
--- a/test/jdk/sun/security/validator/PKIXValAndRevCheckTests.java  Tue Aug 
18 01:08:18 2020 +0200
+++ b/test/jdk/sun/security/validator/PKIXValAndRevCheckTests.java  Mon Aug 
17 17:09:47 2020 -0700
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2019, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2019, 2020, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -374,6 +374,7 @@
 // Test 1: Path validation with revocation explicitly turned
 // off, expected to pass.
 pkixParams = new PKIXBuilderParameters(trustAnchors, null);
+pkixParams.setDate(VALID_DATE);
 pkixParams.setRevocationEnabled(false);
 validatePath(goodPath, Collections.emptyList(), pkixParams, null);

Thanks,
Rajan

RFR: 8238157: Remove intermittent key from AmazonCA.java

[Rajan Halade]

Please review this update to remove key intermittent from AmazonCA test. This 
test no longer fails intermittently.

@@ -24,7 +24,6 @@
 /*
  * @test
  * @bug 8233223
- * @key intermittent
  * @summary Interoperability tests with Amazon's CA1, CA2, CA3, and CA4
  * @build ValidatePathWithParams
  * @run main/othervm -Djava.security.debug=certpath AmazonCA OCSP

Thanks,
Rajan

RFR: 8249176: Update GlobalSignR6CA test certificates

[Rajan Halade]

Please review this change to update test artifacts used for GlobalSign R6 Root 
CA.

Webrev: http://cr.openjdk.java.net/~rhalade/8249176/webrev.00/

Thanks,
Rajan

RFR: 8239105: Add exception for expiring Digicert root certificates to Ver…

[Rajan Halade]

8239105: Add exception for expiring Digicert root certificates to Ver…

-

Commit messages:
 - 8239105: Add exception for expiring Digicert root certificates to 
VerifyCACerts test

Changes: https://git.openjdk.java.net/jdk/pull/484/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=484&range=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8239105
  Stats: 4 lines in 1 file changed: 4 ins; 0 del; 0 mod
  Patch: https://git.openjdk.java.net/jdk/pull/484.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/484/head:pull/484

PR: https://git.openjdk.java.net/jdk/pull/484

Re: RFR: 8239105 : Add exception for expiring Digicert root certificates to VerifyCACerts test

[Rajan Halade]

On Fri, 2 Oct 2020 17:09:37 GMT, Sean Mullan  wrote:

> Looks good. Shouldn't we be seeing mach5 test failures from this though? I 
> didn't see any failures.

We are still not beyond 90 days. It will start failing over the weekend.

-

PR: https://git.openjdk.java.net/jdk/pull/484

Integrated: 8239105 : Add exception for expiring Digicert root certificates to VerifyCACerts test

[Rajan Halade]

On Fri, 2 Oct 2020 16:21:47 GMT, Rajan Halade  wrote:

> 8239105 : Add exception for expiring Digicert root certificates to 
> VerifyCACerts test

This pull request has now been integrated.

Changeset: 123e786d
Author:    Rajan Halade 
URL:   https://git.openjdk.java.net/jdk/commit/123e786d
Stats: 4 lines in 1 file changed: 4 ins; 0 del; 0 mod

8239105: Add exception for expiring Digicert root certificates to VerifyCACerts 
test

"8239105: added verisigntsaca and thawtepremiumserverca to EXPIRY_EXC_ENTRIES 
list"

Reviewed-by: mullan

-

PR: https://git.openjdk.java.net/jdk/pull/484

RFR: 8254081: java/security/cert/PolicyNode/GetPolicyQualifiers.java fails due to an expired certificate

[Rajan Halade]

8254081: java/security/cert/PolicyNode/GetPolicyQualifiers.java fails due to an 
expired certificate

-

Commit messages:
 - 8254081: java/security/cert/PolicyNode/GetPolicyQualifiers.java fails due to 
an expired certificate

Changes: https://git.openjdk.java.net/jdk/pull/528/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=528&range=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8254081
  Stats: 4 lines in 1 file changed: 3 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/528.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/528/head:pull/528

PR: https://git.openjdk.java.net/jdk/pull/528

Re: RFR: 8254081: java/security/cert/PolicyNode/GetPolicyQualifiers.java fails due to an expired certificate [v2]

[Rajan Halade]

> 8254081: java/security/cert/PolicyNode/GetPolicyQualifiers.java fails due to 
> an expired certificate

Rajan Halade has updated the pull request incrementally with one additional 
commit since the last revision:

  8254081: Use DateFormat instead of Date

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/528/files
  - new: https://git.openjdk.java.net/jdk/pull/528/files/f8ff3bbc..0de3b2a4

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=528&range=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=528&range=00-01

  Stats: 5 lines in 1 file changed: 3 ins; 1 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/528.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/528/head:pull/528

PR: https://git.openjdk.java.net/jdk/pull/528

Re: RFR: 8254081: java/security/cert/PolicyNode/GetPolicyQualifiers.java fails due to an expired certificate [v2]

[Rajan Halade]

On Tue, 6 Oct 2020 16:21:34 GMT, Xue-Lei Andrew Fan  wrote:

>> Rajan Halade has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   8254081: Use DateFormat instead of Date
>
> test/jdk/java/security/cert/PolicyNode/GetPolicyQualifiers.java line 57:
> 
>> 55: params.setRevocationEnabled(false);
>> 56: // Certificates expired on Oct 6th, 2020
>> 57: params.setDate(new Date("July 01, 2020"));
> 
> The Date() constructor is deprecated, would you like to use the replacement 
> method DateFormat.parse()?

I have fixed it, thanks!

-

PR: https://git.openjdk.java.net/jdk/pull/528

Integrated: 8254081: java/security/cert/PolicyNode/GetPolicyQualifiers.java fails due to an expired certificate

[Rajan Halade]

On Tue, 6 Oct 2020 16:00:12 GMT, Rajan Halade  wrote:

> 8254081: java/security/cert/PolicyNode/GetPolicyQualifiers.java fails due to 
> an expired certificate

This pull request has now been integrated.

Changeset: 54b340b4
Author:    Rajan Halade 
URL:   https://git.openjdk.java.net/jdk/commit/54b340b4
Stats: 6 lines in 1 file changed: 5 ins; 0 del; 1 mod

8254081: java/security/cert/PolicyNode/GetPolicyQualifiers.java fails due to an 
expired certificate

Perform backdated validation of test certificate.

Reviewed-by: mullan, xuelei

-

PR: https://git.openjdk.java.net/jdk/pull/528

Re: [9] RFR: 8048356: SecureRandom default provider tests

[Rajan Halade]

Thanks for the comments, Brad. I have addressed those here -

http://cr.openjdk.java.net/~rhalade/8048356/webrev.01/

I plan to have this test to check for default provider only so 
SecureRandom.getInstance(algorithm, provider) API is not good fit here. 
I think there are other tests which use this API and provide sufficient 
coverage.


Thanks,
Rajan
On 11/9/15 1:54 PM, Bradford Wetmore wrote:



On 9/4/2015 1:43 PM, Rajan Halade wrote:

Please help with your review of this new test to check default provider
used with SecureRandom.

Bug: https://bugs.openjdk.java.net/browse/JDK-8048356
Webrev: http://cr.openjdk.java.net/~rhalade/8048356/webrev.00/


51:  Minor nit, you might reverse the order to match line 47.

... if (!provider.equals("SUN"))...

I would suggest also doing a test with the specified provider.

SecureRandom instance =
SecureRandom.getInstance(algorithm, provider);

70:  For readability, you might also reset provider here, or at least 
provide a comment.


Otherwise, looks ok.

Brad

Re: RFR 8143377: Test PKCS8Test.java fails

[Rajan Halade]

Hi Amanda,

If you want skip test on solaris, please use -

@requires (os.family != "solaris")

http://openjdk.java.net/jtreg/tag-spec.html#requires_names

Note: I am not a official OpenJDK reviewer.

Thanks,
Rajan

On 11/24/15 2:05 PM, Amanda Jiang wrote:

Hi Max,

Could you please review the fixes below for a test bug?

Bug: https://bugs.openjdk.java.net/browse/JDK-8143377
Webrev: http://cr.openjdk.java.net/~amjiang/8143377/webrev/

Thanks,
Amanda

Re: RFR 8143377: Test PKCS8Test.java fails

[Rajan Halade]

This test was added to ProblemList.txt, please update it to remove the test.

- sun/security/pkcs/pkcs8/PKCS8Test.java solaris-all

Thanks,
Rajan

On 11/24/15 3:13 PM, Rajan Halade wrote:

Hi Amanda,

If you want skip test on solaris, please use -

@requires (os.family != "solaris")

http://openjdk.java.net/jtreg/tag-spec.html#requires_names

Note: I am not a official OpenJDK reviewer.

Thanks,
Rajan

On 11/24/15 2:05 PM, Amanda Jiang wrote:

Hi Max,

Could you please review the fixes below for a test bug?

Bug: https://bugs.openjdk.java.net/browse/JDK-8143377
Webrev: http://cr.openjdk.java.net/~amjiang/8143377/webrev/

Thanks,
Amanda

Re: RFR 8143377: Test PKCS8Test.java fails

[Rajan Halade]

Looks good to me, thanks for the updates!

Please wait for one official reviewer before you commit.

Thanks,
Rajan
On 11/24/15 3:36 PM, Amanda Jiang wrote:

Hi Rajan,

Thanks for reviewing, please check the updated changeset:

http://cr.openjdk.java.net/~amjiang/8143377/webrev.01/


Thanks,
Amanda

On 15/11/24 下午3:26, Rajan Halade wrote:
This test was added to ProblemList.txt, please update it to remove 
the test.


- sun/security/pkcs/pkcs8/PKCS8Test.java solaris-all

Thanks,
Rajan

On 11/24/15 3:13 PM, Rajan Halade wrote:

Hi Amanda,

If you want skip test on solaris, please use -

@requires (os.family != "solaris")

http://openjdk.java.net/jtreg/tag-spec.html#requires_names

Note: I am not a official OpenJDK reviewer.

Thanks,
Rajan

On 11/24/15 2:05 PM, Amanda Jiang wrote:

Hi Max,

Could you please review the fixes below for a test bug?

Bug: https://bugs.openjdk.java.net/browse/JDK-8143377
Webrev: http://cr.openjdk.java.net/~amjiang/8143377/webrev/

Thanks,
Amanda

RFR: [9]: 8055351: sun/security/provider/DSA/TestAlgParameterGenerator.java failed with interrupted! (timed out?)

[Rajan Halade]

Please help with your review of following patch to 
TestAlgParameterGenerator test. It allows test more time to run, not run 
in othervm, and few more trivial updates.


Bug: https://bugs.openjdk.java.net/browse/JDK-8055351
Webrev: http://cr.openjdk.java.net/~rhalade/8055351/webrev.00/

Thanks,
Rajan

Re: RFR: [9]: 8055351: sun/security/provider/DSA/TestAlgParameterGenerator.java failed with interrupted! (timed out?)

[Rajan Halade]

On 4/8/16 3:08 PM, Valerie Peng wrote:


Maybe you can just remove the comments on 103 and 104 for now.
I think this comment has good information. If okay, I will like to 
retain it.

How did u come to the 600 timeout value? Just curious.

Nothing specific, it is hard to decide acceptable timeout value here.

Thanks,
Rajan

Rest looks fine.
Thanks,
Valerie

On 4/8/2016 2:27 PM, Rajan Halade wrote:
Please help with your review of following patch to 
TestAlgParameterGenerator test. It allows test more time to run, not 
run in othervm, and few more trivial updates.


Bug: https://bugs.openjdk.java.net/browse/JDK-8055351
Webrev: http://cr.openjdk.java.net/~rhalade/8055351/webrev.00/

Thanks,
Rajan

RFR: [9]: 8153829: javax/net/ssl/Stapling/HttpsUrlConnClient.java fails intermittently with NullPointerException

[Rajan Halade]

Please help with your review of following patch to OCSP stapling tests. 
HttpsUrlConnClient test failed intermittently with NPE because OCSP 
server was not ready. With this patch, SimpleOCSPServer indicates server 
ready to accept connections. Similar fix is applied to other tests in 
this area.


Bug: https://bugs.openjdk.java.net/browse/JDK-8153829
Webrev: http://cr.openjdk.java.net/~rhalade/8153829/webrev.00/

Thanks,
Rajan

Re: RFR: [9]: 8153829: javax/net/ssl/Stapling/HttpsUrlConnClient.java fails intermittently with NullPointerException

[Rajan Halade]

On 4/12/16 4:08 AM, Xuelei Fan wrote:

Looks fine to me except two minor comments:

test/java/security/testlibrary/SimpleOCSPServer.java
-
  254 serverReady = false;
For safe, I may put line 215-254 into a try-final block.
Fixed this and uploaded correct webrev 
http://cr.openjdk.java.net/~rhalade/8153829/webrev.01/, thanks!


- Rajan


test/javax/net/ssl/Stapling/HttpsUrlConnClient.java
-
  301 // Wait 5 seconds for server ready
The comment says "5 seconds", but the actual implementation is 1 second.
  5 seconds may reduce the frequency of intermittent failure. I would
like to use 5 second wait and update the implementation (line 305-307).

Similar to other test cases.

Thanks,
Xuelei


On 4/12/2016 7:44 AM, Rajan Halade wrote:

Please help with your review of following patch to OCSP stapling tests.
HttpsUrlConnClient test failed intermittently with NPE because OCSP
server was not ready. With this patch, SimpleOCSPServer indicates server
ready to accept connections. Similar fix is applied to other tests in
this area.

Bug: https://bugs.openjdk.java.net/browse/JDK-8153829
Webrev: http://cr.openjdk.java.net/~rhalade/8153829/webrev.00/

Thanks,
Rajan

RFR: [9]: 8154196: Mark javax/net/ssl/DTLS/CipherSuite.java as intermittently failing

[Rajan Halade]

The test javax/net/ssl/DTLS/CipherSuite.java has been seen to fail 
intermittently, see bug 8132320, 8130460.


The test should be marked accordingly.

diff -r 32bb1700f683 test/javax/net/ssl/DTLS/CipherSuite.java
--- a/test/javax/net/ssl/DTLS/CipherSuite.javaTue Apr 05 21:18:01 
2016 +0100
+++ b/test/javax/net/ssl/DTLS/CipherSuite.javaWed Apr 13 12:21:48 
2016 -0700

@@ -27,6 +27,7 @@
 /*
  * @test
  * @bug 8043758
+ * @key intermittent
  * @summary Datagram Transport Layer Security (DTLS)
  * @modules java.base/sun.security.util
  * @build DTLSOverDatagram

Thanks,
Rajan

RFR: [9]: 8151834: Test SmallPrimeExponentP.java times out intermittently

[Rajan Halade]

Hi Max,

Please review this patch to fix SmallPrimeExponentP test. It now uses 
fixed seed value to generate keypair. I did several rounds of testing on 
different machines to make sure that primes 63/65 are generated in fixed 
number of iterations. Other updates to test are done to include detailed 
summary and to log debug information.


Bug: https://bugs.openjdk.java.net/browse/JDK-8151834
Webrev: http://cr.openjdk.java.net/~rhalade/8151834/webrev.00/

Thank you for your help with the fix to have constant seed.

Thanks,
Rajan

Re: RFR: [9]: 8151834: Test SmallPrimeExponentP.java times out intermittently

[Rajan Halade]

Thanks for the comment, I have removed extra check.

http://cr.openjdk.java.net/~rhalade/8151834/webrev.01/

Thanks,
Rajan

On 4/14/16 12:51 PM, Wang Weijun wrote:

Looks good.

Btw, since there is already the @requires tag, I think lines 52-56 are no 
longer needed.

Thanks
Max


在 2016年4月15日，03:44，Rajan Halade  写道：

Hi Max,

Please review this patch to fix SmallPrimeExponentP test. It now uses fixed 
seed value to generate keypair. I did several rounds of testing on different 
machines to make sure that primes 63/65 are generated in fixed number of 
iterations. Other updates to test are done to include detailed summary and to 
log debug information.

Bug: https://bugs.openjdk.java.net/browse/JDK-8151834
Webrev: http://cr.openjdk.java.net/~rhalade/8151834/webrev.00/

Thank you for your help with the fix to have constant seed.

Thanks,
Rajan

RFR: [9]: 8137231: sun/security/rsa/SpecTest.java timeout with Agent error: java.lang.Exception

[Rajan Halade]

Please provide your review of following patch to SpecTest.java test. 
This test purpose shouldn't require generating extra key pair with 
different public exponent value.


Bug: https://bugs.openjdk.java.net/browse/JDK-8137231
Webrev: http://cr.openjdk.java.net/~rhalade/8137231/webrev.00/

Thanks,
Rajan

RFR: [9]: 8154382: Remove intermittent keyword from SupportedDSAParamGen.java

[Rajan Halade]

Please review following patch to remove intermittent keyword from this 
test. Test has never seen to fail intermittently.


diff -r 8606d027b2c2 
test/sun/security/provider/DSA/SupportedDSAParamGen.java
--- a/test/sun/security/provider/DSA/SupportedDSAParamGen.java Fri Apr 
15 16:19:15 2016 +0100
+++ b/test/sun/security/provider/DSA/SupportedDSAParamGen.java Fri Apr 
15 16:58:04 2016 -0700

@@ -24,7 +24,6 @@
 /*
  * @test
  * @bug 8072452
- * @key intermittent
  * @summary Support DHE sizes up to 8192-bits and DSA sizes up to 
3072-bits

  * @run main/timeout=300 SupportedDSAParamGen 1024 160
  * @run main/timeout=300 SupportedDSAParamGen 2048 224

Thanks,
Rajan

RFR: [9]: 8129329: sun/security/ssl/SSLContextImpl/MD2InTrustAnchor.java failed intermittently

[Rajan Halade]

Please provide your review of following patch to this test to start 
server in a separate thread, have additional logging and few other nits. 
Old test didn't have enough logging at the time of failure so it is not 
clear why client didn't connect which resulted in timeout on server accept.


Bug: https://bugs.openjdk.java.net/browse/JDK-8129329
Webrev: http://cr.openjdk.java.net/~rhalade/8129329/webrev.00/

Thanks,
Rajan

Re: RFR: [9]: 8129329: sun/security/ssl/SSLContextImpl/MD2InTrustAnchor.java failed intermittently

[Rajan Halade]

Thanks for your comments!

Updated webrev: http://cr.openjdk.java.net/~rhalade/8129329/webrev.01/

On 4/20/16 5:23 PM, Xuelei Fan wrote:

You made a few update of the coding style, for example line 60-62, which
is different from the traditional coding.  Are you following some coding
conventions?
I use netbeans auto-format which coding conventions [1] implemented. Per 
4.2, general principal is to have line break before operator.


[1] 
http://www.oracle.com/technetwork/java/javase/documentation/codeconventions-136091.html#248


As if you are already there, maybe you can do more revise.
1. although it is a test, most of the methods and fields can be private.
2. would you mind use try-with-resource for sslServerSocket, too.

These are addressed in new webrev.

Thanks,
Rajan


Thanks,
Xuelei

On 4/21/2016 5:55 AM, Rajan Halade wrote:

Please provide your review of following patch to this test to start
server in a separate thread, have additional logging and few other nits.
Old test didn't have enough logging at the time of failure so it is not
clear why client didn't connect which resulted in timeout on server accept.

Bug: https://bugs.openjdk.java.net/browse/JDK-8129329
Webrev: http://cr.openjdk.java.net/~rhalade/8129329/webrev.00/

Thanks,
Rajan

RFR: 9: 8156671: Refactor sun/security/rsa/SignatureTest.java

[Rajan Halade]

Please provide your review of patch to refactor SignatureTest. This test 
now runs all signature algorithms with key generated once resulting in 
saving 75% of execution time.


Bug: https://bugs.openjdk.java.net/browse/JDK-8156671
Webrev: http://cr.openjdk.java.net/~rhalade/8156671/webrev.00/

Thanks,
Rajan

RFR: 9: 8155049: New tests from 8144566 fail with "No expected Server Name Indication"

[Rajan Halade]

Please provide your review of this one line fix.

These tests fail on machine where non-FQDN hostname is returned. As per 
RFC for SNI extension, hostname needs to be fully qualified DNS hostname 
and same is asserted in JDK when it adds hostname to SNI extension. This 
fix skips test execution if hostname returned is not fully qualified.


Bug: https://bugs.openjdk.java.net/browse/JDK-8155049
Webrev: http://cr.openjdk.java.net/~rhalade/8155049/webrev.00/

Thanks,
Rajan

RFR: 9: 8156035: Remove intermittent key from sun/security/rsa/SpecTest.java

[Rajan Halade]

Please review this small change to remove intermittent key from this 
test. JDK-8137231 addressed intermittent failure and we haven't seen 
test failure for a long time and with frequent runs.


Bug: https://bugs.openjdk.java.net/browse/JDK-8156035

diff -r 3675fb8573d4 test/sun/security/rsa/SpecTest.java
--- a/test/sun/security/rsa/SpecTest.javaWed May 18 18:46:14 2016 +0200
+++ b/test/sun/security/rsa/SpecTest.javaWed May 18 10:37:42 2016 -0700
@@ -24,7 +24,6 @@
 /**
  * @test
  * @bug 8044199 8137231
- * @key intermittent
  * @summary Check same KeyPair's private key and public key have same 
modulus.

  * also check public key's public exponent equals to given spec's public
  * exponent. Only key size 1024 is tested with 
RSAKeyGenParameterSpec.F0 (3).


Thanks,
Rajan

Re: RFR 8156059: Update/Develop new tests for JEP 287: SHA-3 Hash Algorithms

[Rajan Halade]

Hi Amanda,

I have following comments:
- UnsupportedProvider.java line 55: this assumes that it is okay to have 
NoSuchAlgorithmException for all providers.
- Update MessageDigest tests to use RandomFactory from 
http://hg.openjdk.java.net/jdk9/jdk9/jdk/file/f8a3c1510f95/test/lib/testlibrary/jdk/testlibrary. 
It logs seed information which will be helpful to reproduce failure.

- Also, MessageDigest tests have no provider checks.

Rest of tests look fine to me. Note, I am not a official reviewer so 
please wait for official reviewer to integrate changes.


Thanks,
Rajan

On 5/19/16 11:34 AM, Amanda Jiang wrote:

Hi All,

Please help to review tests for JEP287: SHA-3 Hash Alogrithms.

BugID: https://bugs.openjdk.java.net/browse/JDK-8156059
Webrev: http://cr.openjdk.java.net/~amjiang/8156059/webrev.01/
Test Plan: https://bugs.openjdk.java.net/browse/JDK-8068855

Thanks,
Amanda

RFR: 9: 8157469: DefaultProviderList.java fails with no provider class apple.security.AppleProvider found

[Rajan Halade]

Please review following fix.

AppleProvider is in java.base module so will not be loaded by 
ServiceLoader. 8157489 is filed to fix 
java.base/macosx/classes/module-info.java.extra.


Bug: https://bugs.openjdk.java.net/browse/JDK-8157469
Webrev: http://cr.openjdk.java.net/~rhalade/8157469/webrev.00/

Thanks,
Rajan

Re: RFR 8156059: Update/Develop new tests for JEP 287: SHA-3 Hash Algorithms

[Rajan Halade]

Hi Amanda,

Thanks for the update. Few more comments after review:

- TestSameLength.java: line 93-103, check here should use provider used 
with test instance of MessageDigest, call md.getProvider(). You will 
need to reorganize test code for this. Security.getProvider() returns 
provider installed with specified name.
- It will be good if you have a function isSHA3Supported() to check SHA3 
supported or not


Same comments applies from other MessageDigest tests in your review.

Thanks,
Rajan

On 5/23/16 3:24 PM, Amanda Jiang wrote:

Hi Valerie , Rajan,

Thanks for reviewing the webrev.  I have updaed 
UnsupportedProvider.java to check NSAE for SUN and OracleUcrypto 
providers.


Other tests has  been updated by Rajan's comments as well.

Please check the latest webrev and let me know if there are any other 
problems.


http://cr.openjdk.java.net/~amjiang/8156059/webrev.02/


Thanks,
Amanda


On 16/5/19 下午3:31, Valerie Peng wrote:


True, I think the UnsupportedProvider.java should assume SUN provider 
to support SHA-3 and not allow NSAE.
OracleUcrypto provider only supports SHA-3 on Solaris 12 or later, so 
what you have is fine.


As for other tests, as long as SHA-3 algos are covered, it's up to 
you to make the enhancements that Rajan suggested.

Thanks,
Valerie

On 5/19/2016 3:18 PM, Rajan Halade wrote:

Hi Amanda,

I have following comments:
- UnsupportedProvider.java line 55: this assumes that it is okay to 
have NoSuchAlgorithmException for all providers.
- Update MessageDigest tests to use RandomFactory from 
http://hg.openjdk.java.net/jdk9/jdk9/jdk/file/f8a3c1510f95/test/lib/testlibrary/jdk/testlibrary. 
It logs seed information which will be helpful to reproduce failure.

- Also, MessageDigest tests have no provider checks.

Rest of tests look fine to me. Note, I am not a official reviewer so 
please wait for official reviewer to integrate changes.


Thanks,
Rajan

On 5/19/16 11:34 AM, Amanda Jiang wrote:

Hi All,

Please help to review tests for JEP287: SHA-3 Hash Alogrithms.

BugID: https://bugs.openjdk.java.net/browse/JDK-8156059
Webrev: http://cr.openjdk.java.net/~amjiang/8156059/webrev.01/
Test Plan: https://bugs.openjdk.java.net/browse/JDK-8068855

Thanks,
Amanda

Re: RFR 8156059: Update/Develop new tests for JEP 287: SHA-3 Hash Algorithms

[Rajan Halade]

Thanks, I have no more comments.

- Rajan

On 5/26/2016 16:58, Amanda Jiang wrote:

Hi Rajan,

Thanks for your suggestions, I have updated tests with your comments. 
For TestSampleLength.java, please see my reply inline.


webrev: http://cr.openjdk.java.net/~amjiang/8156059/webrev.03/

Thanks,
Amanda

On 16/5/25 下午12:31, Rajan Halade wrote:

Hi Amanda,

Thanks for the update. Few more comments after review:

- TestSameLength.java: line 93-103, check here should use provider 
used with test instance of MessageDigest, call md.getProvider(). You 
will need to reorganize test code for this. Security.getProvider() 
returns provider installed with specified name.
the block in line93-103 is for checking if system supports SHA-3, not 
related to a specific provider. md.getProvider() won't work since if 
NSAE thrown, md = null.
- It will be good if you have a function isSHA3Supported() to check 
SHA3 supported or not


Same comments applies from other MessageDigest tests in your review.

Thanks,
Rajan

On 5/23/16 3:24 PM, Amanda Jiang wrote:

Hi Valerie , Rajan,

Thanks for reviewing the webrev.  I have updaed 
UnsupportedProvider.java to check NSAE for SUN and OracleUcrypto 
providers.


Other tests has  been updated by Rajan's comments as well.

Please check the latest webrev and let me know if there are any 
other problems.


http://cr.openjdk.java.net/~amjiang/8156059/webrev.02/


Thanks,
Amanda


On 16/5/19 下午3:31, Valerie Peng wrote:


True, I think the UnsupportedProvider.java should assume SUN 
provider to support SHA-3 and not allow NSAE.
OracleUcrypto provider only supports SHA-3 on Solaris 12 or later, 
so what you have is fine.


As for other tests, as long as SHA-3 algos are covered, it's up to 
you to make the enhancements that Rajan suggested.

Thanks,
Valerie

On 5/19/2016 3:18 PM, Rajan Halade wrote:

Hi Amanda,

I have following comments:
- UnsupportedProvider.java line 55: this assumes that it is okay 
to have NoSuchAlgorithmException for all providers.
- Update MessageDigest tests to use RandomFactory from 
http://hg.openjdk.java.net/jdk9/jdk9/jdk/file/f8a3c1510f95/test/lib/testlibrary/jdk/testlibrary. 
It logs seed information which will be helpful to reproduce failure.

- Also, MessageDigest tests have no provider checks.

Rest of tests look fine to me. Note, I am not a official reviewer 
so please wait for official reviewer to integrate changes.


Thanks,
Rajan

On 5/19/16 11:34 AM, Amanda Jiang wrote:

Hi All,

Please help to review tests for JEP287: SHA-3 Hash Alogrithms.

BugID: https://bugs.openjdk.java.net/browse/JDK-8156059
Webrev: http://cr.openjdk.java.net/~amjiang/8156059/webrev.01/
Test Plan: https://bugs.openjdk.java.net/browse/JDK-8068855

Thanks,
Amanda

Re: [9] RFR: 8157896: TestDSAGenParameterSpec.java test fails with timeout

[Rajan Halade]

Thanks for the patch, Siba!

This fix looks confusing to me. TestDSAGenParameterSpec has timeout 
specified for 2048 but not for 3072/224 and 4096 case. Why would these 
run faster than smaller lengths?


May be there is some other reason, area experts can help here.

Thanks,
Rajan

On 5/31/2016 08:04, Sibabrata Sahoo wrote:


Hi,

Please review the fix for the following JBS bugs,

JBS: https://bugs.openjdk.java.net/browse/JDK-8157896

https://bugs.openjdk.java.net/browse/JDK-8157898

webrev: http://cr.openjdk.java.net/~ssahoo/8157896/webrev.00/ 



Description:

These Tests takes a lots of time to generate DSA parameter when the 
prime and subprime values are greater in some machine with some 
platform. The reason is being the operations are CPU intensive, which 
causes the Tests to timeout in rare occasion. But the Test performance 
can be improved by, instead of generating DSA parameter with all 
supported input in a single run, it can be divided for each input as a 
separate run, so that each run can complete much before compared to 
all input at once. It also provides the flexibility to associate test 
options like “timeout” for specific input type where the execution 
time is expected to exceed the default.


Note: The fix won’t give a guarantee that the timeout will never 
happen but it can surely help reduce the timeout in greater extent. 
Please provide your suggestion, in case any.


Thanks,

Siba

RFR: 9: 8158569: ImpactOnSNI.java test failed with timeout

[Rajan Halade]

Please help with your review of this patch. I was not able to reproduce 
the timeout but thread dump indicates that test timed out on waiting for 
server thread to join. Server thread is kept waiting on accept if client 
doesn't connect.


This patch allows test to throw exception, if any, on client or server 
side eliminating this timeout scenario.


Webrev: http://cr.openjdk.java.net/~rhalade/8158569/webrev.00/

Thanks,
Rajan

Re: RFR: 9: 8158569: ImpactOnSNI.java test failed with timeout

[Rajan Halade]

You are right, I missed the fact that startClient has exception catch 
which will allow test to call to serverThread.join(). Updated webrev 
fixed this along with some more debug output.


Webrev: http://cr.openjdk.java.net/~rhalade/8158569/webrev.01/

Thanks,
Rajan

On 6/8/16 9:08 PM, Xuelei Fan wrote:


I'm not sure this patch would work.  What if throwing an exception
immediately after the server ready in client side with/without this patch?

There might be something wrong in the url connections of the testing.
It might be worthy to add more defbug info for further evaluation.

Xuelei

On 6/9/2016 6:19 AM, Rajan Halade wrote:

Please help with your review of this patch. I was not able to reproduce
the timeout but thread dump indicates that test timed out on waiting for
server thread to join. Server thread is kept waiting on accept if client
doesn't connect.

This patch allows test to throw exception, if any, on client or server
side eliminating this timeout scenario.

Webrev: http://cr.openjdk.java.net/~rhalade/8158569/webrev.00/

Thanks,
Rajan

Re: [9] RFR:8160337 Remove intermittent key from sun/security/pkcs11/rsa/TestKeyPairGenerator.java

[Rajan Halade]

Hi Tim,

Thanks for taking care of this one. Since the test is still using random 
data key "randomness" should be retained.


Thanks,
Rajan

On 7/6/16 1:04 AM, Tim Du wrote:

Hi All,

Would you help to review the path for "8160337:Remove intermittent key 
from sun/security/pkcs11/rsa/TestKeyPairGenerator.java"? JDK-8074580 
has been fixed, @key intermittent can be removed.Thanks.


JBS: https://bugs.openjdk.java.net/browse/JDK-8160337
webrev: http://cr.openjdk.java.net/~tidu/8160337/webrev.00/

Regards
Tim

Re: [9] RFR: JDK-7052815 Change tests that remove or add providers to run in othervm mode

[Rajan Halade]

Hi Tim,

Please also update the copyright year to 2016 wherever applicable.

Thanks,
Rajan

On 7/7/16 2:11 AM, Tim Du wrote:

Hi Sean:

Thank you for reviewing the codes.
I updated the title for this bug and added othervm for the tests that 
add but not remove their own provider. Also corrected the typo. New 
webrev is available here: 
http://cr.openjdk.java.net/~tidu/7052815/webrev.01/ .

Please help to review it again.Thanks.

Regards
Tim
On 2016/7/6 23:05, Sean Mullan wrote:
Could you add a more descriptive title for this bug report than 
"Finish checking of 7052537"? How about "Change tests that remove 
providers to run in othervm mode".


Also, have you also checked if there are tests that just add their 
own provider but don't remove it? This could also potentially cause 
strange behavior if another test run in parallel accidentally uses 
that provider (perhaps if it was inserted as the first provider and 
implemented an algorithm that the test was testing).


Typo in RemoveProviders.java:

28  * @run main/othrevm RemoveProviders

s/othrevm/othervm/

--Sean

On 07/05/2016 08:11 AM, Tim Du wrote:

Hi ,

Please help to review the path for "JDK-7052815 :Finish checking of
7052537" .

A few test are using removedProvider() to removed the security
provider,like JDK-7052537, If the tests not in othervm,it will affect
other tests. Checking tests which are using removedProvider() for
security area and update them run in othervm mode to avoid the same 
issue.


JBS: https://bugs.openjdk.java.net/browse/JDK-7052815
webrev: http://cr.openjdk.java.net/~tidu/7052815/webrev.00/

Regards
Tim