Re: RFR 8244148: keytool -printcert and -printcrl should support the -trustcacerts and -keystore options

[sha . jiang]

Hi Hai-May,

On 2020/6/13 06:34, Hai-May Chao wrote:

Hi John,

Updated Webrev -
https://cr.openjdk.java.net/~hchao/8244148/webrev.03/

Thanks for this updated webrev!
I have no more comment.

Best regards,
John Jiang



On Jun 11, 2020, at 1:45 AM, sha.ji...@oracle.com 
 wrote:


Hi Hai-May,

On 2020/6/8 04:01, Hai-May Chao wrote:

Updated webrev -

https://cr.openjdk.java.net/~hchao/8244148/webrev.02/

-- src/java.base/share/classes/sun/security/util/FilePaths.java
Would it be better to use String.join() or even java.nio.file.Path to
build the file path?



The current code is based on the original code such as in 
KeyStoreUtil.java and is consistent with others.



-- src/java.base/share/classes/sun/security/util/AnchorCertificates.java
55 File f = new File(FilePaths.cacerts());
...
59 try (FileInputStream fis = new 
FileInputStream(f)) {

f is used only once, so it may be unnecessary.


This is the existing code struct which is working fine.



56 KeyStore cacerts;
Though it's not in your change, would you mind to declare this variable
in the try block? I just want to narrow the variable scope.


Done.



-- test/lib/jdk/test/lib/SecurityTools.java
Could move method createCacerts() to 
test/lib/jdk/test/lib/security/KeyStoreUtils.java?

This class contains a set of methods for creating trust/key stores.
And getCertFromFile() in test/lib/jdk/test/lib/security/CertUtils.java
can load certificate from a file.


Moved the method createCacerts() to the suggested location under test/.
Tried to use getCertFromFile() and it did not work as it read from 
test.src. So we need to read the cert directly.




289 int pos = 0;
...
294 for (String crt : crts) {
If not using for-each style, pos can be declared in the for statement.


Ok, changed to not using for-each style.



--
test/jdk/sun/security/tools/keytool/fakecacerts/java.base/sun/security/util/FilePaths.java
This patch can be used by other tests, so could you please move it to a
common path? Like 
test/jdk/sun/security/util/module_patch/java.base/sun/security/util/FilePaths.java


Moved the patch to the suggested location under test/.



32 public static String cacerts() {
33 return "mycacerts";
34 }
Could it be flexible for returning an alternative path?
For example, accept a system property, say test.cacerts, for specifying
an alternative path. mycacerts is the default value of this property.


Done.

Thanks,
Hai-May




Best regards,
John Jiang



Thanks,
Hai-May


On Jun 5, 2020, at 11:04 PM, Weijun Wang > wrote:


I still think duplicated commands in TrustedCert.java are useless. 
Line 104 and line 133 are exactly the same, line 109 and line 138 
are exactly the same, and you haven't made any change to these 2 
files in between.


Same for line 80 and line 96 of TrustedCRL.java.

Everything else is fine.

Thanks,
Max


On Jun 6, 2020, at 2:25 AM, Hai-May Chao > wrote:


Updated webrev -

https://cr.openjdk.java.net/~hchao/8244148/webrev.01/

Added one command line -importcert in TrustCert.java.
Added createCacerts() in test/lib SecurityTools.java.

Thanks,
Hai-May



On Jun 4, 2020, at 5:57 AM, Weijun Wang  
wrote:




On Jun 4, 2020, at 7:29 PM, Hai-May Chao 
 wrote:


Hi Max,

On Jun 3, 2020, at 12:59 AM, Weijun Wang 
 wrote:


The source change looks fine to me.

In TrustedCert.java:

- You can use FileOutputStream and 
Files.copy(Path,OutputStream) in cat().


This cat() is taken from WealAlg.java.



- There is no need to recreate root.jks and root.pem.


The sequences of the commands used in this test scenario allows 
me to test -printcert for the -trustcacerts and -keytsore 
options. We had discussion offline about it. The test uses 
trusted certificates and checks no warnings on the weak 
algorithms to address the requirement described in the bug. I 
believe it does serve that purpose, and looks legitimate to me. 
There could be different ways of testing a functionality, and 
please let me know if there is a problem with the current approach.


I just meant that the keytool commands generating root.jks and 
root.pem are exactly the same and there is no need to recreate it.




Please also elaborate your comment about no need to recreate 
root.jks and root.pem.




- Why not use -trustcacerts below?

160 kt("-importcert -file server.pem -noprompt", 
"server.jks”);



Because here is to import the server (end-entity) cert, and it 
will not make a difference for the test result whether to use 
the -trustcacerts or not. It’s the ca (intermediate) cert needs 
to have it in this test scenario. I intended to leave it out in 
#160 to distinguish between server and ca certs.


OK.

Then how about we add a new command before line 155?

  kt("-importcert -file ca.pem", "ca.jks").shouldNotHaveExitValue(0);

This would prove the "-trustcacerts" on line 155 is really 

Re: RFR 8244148: keytool -printcert and -printcrl should support the -trustcacerts and -keystore options

[Hai-May Chao]

> On Jun 12, 2020, at 5:37 AM, Weijun Wang  wrote:
> 
> I re-read the CSR.
> 
> The precise meaning of the 2 options for -printcert is: "If the cert is a 
> trusted certificate in either keystore or cacerts, we will not warn if it 
> uses a weak signature algorithm". This is so subtle and I wonder it's worth 
> describing it. Or we just say "This command does not check for the weakness 
> of a certificate's signature algorithm if it's a trusted certificate in the 
> user keystore (specified by -keystore) or the `cacerts` keystore (if 
> -trustcacerts is specified)”.

Updated CSR with the second mentioned.

> 
> For -printcrl, the "issuer CA" is a little confusing. I would say "This 
> commands attempts to verify the CRL using a certificate from the user 
> keystore (specified by -keystore) or the `cacerts` keystore (if -trustcacerts 
> is specified), and print out a warning if it cannot be verified”.

CRL issuer is usually the CA, and the implementation of -printcrl tries to get 
its issuer from the user or cacerts keystore. Updated CSR as suggested as if 
there may be confusing.

Thanks,
Hai-May

> 
> Thanks,
> Max
> 
> p.s. A new kind of check comes to my mind. Suppose you are printing a 
> certificate whose own key is 2048-bit RSA, signature is using SHA256withRSA, 
> but the signer key is only 512 bits (we can notice this from the size of the 
> signature). Shall we print out a warning? 

Your thought?

> 
>> On Jun 11, 2020, at 11:21 AM, Weijun Wang  wrote:
>> 
>> "to a self-signed certificate in the keystore". This is not correct. What we 
>> look for is a TrustedCertificateEntry.
>> 
>> "It emits warning when a certificate is not trusted and uses weak 
>> algorithms". Precisely, it's "uses a weak signature algorithm".
>> 
>> --Max
>> 
>> 
>>> On Jun 10, 2020, at 5:31 PM, Hai-May Chao  wrote:
>>> 
>>> 
>>> 
 On Jun 9, 2020, at 5:58 PM, Weijun Wang  wrote:
 
 Good to see both -keystore and -trustcacerts mentioned. Some comments:
 
 1. I think there is no need to say "from -file cert_file". Or, do you mean 
 the new function does not apply to those from -sslserver and -jarfile? If 
 so, that might be a problem.
>>> 
>>> You’re right. Removed it.
>>> 
 
 2. While you said "attempts to construct a chain of trust", do you also 
 want to describe what happens if it succeeds or fails?
>>> 
>>> Updated manpage.
>>> 
 
 3. It will be nice if you can include the exact diff of the man page files 
 either inside the CSR itself or as a comment.
 
>>> 
>>> Included the diff of the manpage in the CSR.
>>> 
>>> Thanks,
>>> Hai-May
>>> 
>>> 
 Thanks,
 Max
 
> On Jun 9, 2020, at 10:51 PM, Hai-May Chao  wrote:
> 
> 
> 
>> On Jun 7, 2020, at 6:08 PM, Weijun Wang  wrote:
>> 
>> Looks fine to me.
>> 
>> For CSR, since there is already a "Note" there for these 2 options, you 
>> can add a few words about what -keystore and -trustcacerts can do.
> 
> Updated CSR as suggested.
> 
> Thanks,
> Hai-May
> 
> 
>> 
>> Thanks,
>> Max
>> 
>>> On Jun 8, 2020, at 4:01 AM, Hai-May Chao  
>>> wrote:
>>> 
>>> Updated webrev -
>>> 
>>> https://cr.openjdk.java.net/~hchao/8244148/webrev.02/
>>> 
>>> Thanks,
>>> Hai-May
>>> 
>>> 
 On Jun 5, 2020, at 11:04 PM, Weijun Wang  
 wrote:
 
 I still think duplicated commands in TrustedCert.java are useless. 
 Line 104 and line 133 are exactly the same, line 109 and line 138 are 
 exactly the same, and you haven't made any change to these 2 files in 
 between.
 
 Same for line 80 and line 96 of TrustedCRL.java.
 
 Everything else is fine.
 
 Thanks,
 Max
 
 
> On Jun 6, 2020, at 2:25 AM, Hai-May Chao  
> wrote:
> 
> Updated webrev - 
> 
> https://cr.openjdk.java.net/~hchao/8244148/webrev.01/
> 
> Added one command line -importcert in TrustCert.java.
> Added createCacerts() in test/lib SecurityTools.java.
> 
> Thanks,
> Hai-May
> 
> 
> 
>> On Jun 4, 2020, at 5:57 AM, Weijun Wang  
>> wrote:
>> 
>> 
>> 
>>> On Jun 4, 2020, at 7:29 PM, Hai-May Chao  
>>> wrote:
>>> 
>>> Hi Max,
>>> 
 On Jun 3, 2020, at 12:59 AM, Weijun Wang  
 wrote:
 
 The source change looks fine to me.
 
 In TrustedCert.java:
 
 - You can use FileOutputStream and Files.copy(Path,OutputStream) 
 in cat().
>>> 
>>> This cat() is taken from WealAlg.java.
>>> 
 
 - There is no need to recreate root.jks and root.pem.
>>> 
>>> The sequences of the 

Re: RFR 8244148: keytool -printcert and -printcrl should support the -trustcacerts and -keystore options

[Hai-May Chao]

Hi John,

Updated Webrev -
https://cr.openjdk.java.net/~hchao/8244148/webrev.03/

> On Jun 11, 2020, at 1:45 AM, sha.ji...@oracle.com wrote:
> 
> Hi Hai-May,
> 
> On 2020/6/8 04:01, Hai-May Chao wrote:
>> Updated webrev -
>> 
>> https://cr.openjdk.java.net/~hchao/8244148/webrev.02/ 
>> -- 
>> src/java.base/share/classes/sun/security/util/FilePaths.java
> Would it be better to use String.join() or even java.nio.file.Path to
> build the file path?
> 

The current code is based on the original code such as in KeyStoreUtil.java and 
is consistent with others.

> -- src/java.base/share/classes/sun/security/util/AnchorCertificates.java
> 55 File f = new File(FilePaths.cacerts());
> ...
> 59 try (FileInputStream fis = new FileInputStream(f)) {
> f is used only once, so it may be unnecessary.

This is the existing code struct which is working fine.

> 
> 56 KeyStore cacerts;
> Though it's not in your change, would you mind to declare this variable
> in the try block? I just want to narrow the variable scope.

Done.

> 
> -- test/lib/jdk/test/lib/SecurityTools.java
> Could move method createCacerts() to 
> test/lib/jdk/test/lib/security/KeyStoreUtils.java?
> This class contains a set of methods for creating trust/key stores.
> And getCertFromFile() in test/lib/jdk/test/lib/security/CertUtils.java
> can load certificate from a file.

Moved the method createCacerts() to the suggested location under test/.
Tried to use getCertFromFile() and it did not work as it read from test.src. So 
we need to read the cert directly.

> 
> 289 int pos = 0;
> ...
> 294 for (String crt : crts) {
> If not using for-each style, pos can be declared in the for statement.

Ok, changed to not using for-each style.

> 
> --  
> test/jdk/sun/security/tools/keytool/fakecacerts/java.base/sun/security/util/FilePaths.java
> This patch can be used by other tests, so could you please move it to a
> common path? Like 
> test/jdk/sun/security/util/module_patch/java.base/sun/security/util/FilePaths.java

Moved the patch to the suggested location under test/.

> 
> 32 public static String cacerts() {
> 33 return "mycacerts";
> 34 }
> Could it be flexible for returning an alternative path?
> For example, accept a system property, say test.cacerts, for specifying
> an alternative path. mycacerts is the default value of this property.

Done.

Thanks,
Hai-May


> 
> Best regards,
> John Jiang
> 
>> 
>> Thanks,
>> Hai-May
>> 
>> 
>>> On Jun 5, 2020, at 11:04 PM, Weijun Wang >> > wrote:
>>> 
>>> I still think duplicated commands in TrustedCert.java are useless. Line 104 
>>> and line 133 are exactly the same, line 109 and line 138 are exactly the 
>>> same, and you haven't made any change to these 2 files in between.
>>> 
>>> Same for line 80 and line 96 of TrustedCRL.java.
>>> 
>>> Everything else is fine.
>>> 
>>> Thanks,
>>> Max
>>> 
>>> 
 On Jun 6, 2020, at 2:25 AM, Hai-May Chao >>> > wrote:
 
 Updated webrev - 
 
 https://cr.openjdk.java.net/~hchao/8244148/webrev.01/ 
 
 
 Added one command line -importcert in TrustCert.java.
 Added createCacerts() in test/lib SecurityTools.java.
 
 Thanks,
 Hai-May
 
 
 
> On Jun 4, 2020, at 5:57 AM, Weijun Wang  
>  wrote:
> 
> 
> 
>> On Jun 4, 2020, at 7:29 PM, Hai-May Chao  
>>  wrote:
>> 
>> Hi Max,
>> 
>>> On Jun 3, 2020, at 12:59 AM, Weijun Wang  
>>>  wrote:
>>> 
>>> The source change looks fine to me.
>>> 
>>> In TrustedCert.java:
>>> 
>>> - You can use FileOutputStream and Files.copy(Path,OutputStream) in 
>>> cat().
>> 
>> This cat() is taken from WealAlg.java.
>> 
>>> 
>>> - There is no need to recreate root.jks and root.pem.
>> 
>> The sequences of the commands used in this test scenario allows me to 
>> test -printcert for the -trustcacerts and -keytsore options. We had 
>> discussion offline about it. The test uses trusted certificates and 
>> checks no warnings on the weak algorithms to address the requirement 
>> described in the bug. I believe it does serve that purpose, and looks 
>> legitimate to me. There could be different ways of testing a 
>> functionality, and please let me know if there is a problem with the 
>> current approach.
> 
> I just meant that the keytool commands generating root.jks and root.pem 
> are exactly the same and there is no need to recreate it.
> 
>> 
>> Please also elaborate your comment about no need to recreate root.jks 
>> and root.pem.
>> 
>>> 
>>> - Why not use -trustcacerts below?
>>> 

Re: RFR 8244148: keytool -printcert and -printcrl should support the -trustcacerts and -keystore options

[Weijun Wang]

I re-read the CSR.

The precise meaning of the 2 options for -printcert is: "If the cert is a 
trusted certificate in either keystore or cacerts, we will not warn if it uses 
a weak signature algorithm". This is so subtle and I wonder it's worth 
describing it. Or we just say "This command does not check for the weakness of 
a certificate's signature algorithm if it's a trusted certificate in the user 
keystore (specified by -keystore) or the `cacerts` keystore (if -trustcacerts 
is specified)".

For -printcrl, the "issuer CA" is a little confusing. I would say "This 
commands attempts to verify the CRL using a certificate from the user keystore 
(specified by -keystore) or the `cacerts` keystore (if -trustcacerts is 
specified), and print out a warning if it cannot be verified".

Thanks,
Max

p.s. A new kind of check comes to my mind. Suppose you are printing a 
certificate whose own key is 2048-bit RSA, signature is using SHA256withRSA, 
but the signer key is only 512 bits (we can notice this from the size of the 
signature). Shall we print out a warning? 

> On Jun 11, 2020, at 11:21 AM, Weijun Wang  wrote:
> 
> "to a self-signed certificate in the keystore". This is not correct. What we 
> look for is a TrustedCertificateEntry.
> 
> "It emits warning when a certificate is not trusted and uses weak 
> algorithms". Precisely, it's "uses a weak signature algorithm".
> 
> --Max
> 
> 
>> On Jun 10, 2020, at 5:31 PM, Hai-May Chao  wrote:
>> 
>> 
>> 
>>> On Jun 9, 2020, at 5:58 PM, Weijun Wang  wrote:
>>> 
>>> Good to see both -keystore and -trustcacerts mentioned. Some comments:
>>> 
>>> 1. I think there is no need to say "from -file cert_file". Or, do you mean 
>>> the new function does not apply to those from -sslserver and -jarfile? If 
>>> so, that might be a problem.
>> 
>> You’re right. Removed it.
>> 
>>> 
>>> 2. While you said "attempts to construct a chain of trust", do you also 
>>> want to describe what happens if it succeeds or fails?
>> 
>> Updated manpage.
>> 
>>> 
>>> 3. It will be nice if you can include the exact diff of the man page files 
>>> either inside the CSR itself or as a comment.
>>> 
>> 
>> Included the diff of the manpage in the CSR.
>> 
>> Thanks,
>> Hai-May
>> 
>> 
>>> Thanks,
>>> Max
>>> 
 On Jun 9, 2020, at 10:51 PM, Hai-May Chao  wrote:
 
 
 
> On Jun 7, 2020, at 6:08 PM, Weijun Wang  wrote:
> 
> Looks fine to me.
> 
> For CSR, since there is already a "Note" there for these 2 options, you 
> can add a few words about what -keystore and -trustcacerts can do.
 
 Updated CSR as suggested.
 
 Thanks,
 Hai-May
 
 
> 
> Thanks,
> Max
> 
>> On Jun 8, 2020, at 4:01 AM, Hai-May Chao  wrote:
>> 
>> Updated webrev -
>> 
>> https://cr.openjdk.java.net/~hchao/8244148/webrev.02/
>> 
>> Thanks,
>> Hai-May
>> 
>> 
>>> On Jun 5, 2020, at 11:04 PM, Weijun Wang  wrote:
>>> 
>>> I still think duplicated commands in TrustedCert.java are useless. Line 
>>> 104 and line 133 are exactly the same, line 109 and line 138 are 
>>> exactly the same, and you haven't made any change to these 2 files in 
>>> between.
>>> 
>>> Same for line 80 and line 96 of TrustedCRL.java.
>>> 
>>> Everything else is fine.
>>> 
>>> Thanks,
>>> Max
>>> 
>>> 
 On Jun 6, 2020, at 2:25 AM, Hai-May Chao  
 wrote:
 
 Updated webrev - 
 
 https://cr.openjdk.java.net/~hchao/8244148/webrev.01/
 
 Added one command line -importcert in TrustCert.java.
 Added createCacerts() in test/lib SecurityTools.java.
 
 Thanks,
 Hai-May
 
 
 
> On Jun 4, 2020, at 5:57 AM, Weijun Wang  
> wrote:
> 
> 
> 
>> On Jun 4, 2020, at 7:29 PM, Hai-May Chao  
>> wrote:
>> 
>> Hi Max,
>> 
>>> On Jun 3, 2020, at 12:59 AM, Weijun Wang  
>>> wrote:
>>> 
>>> The source change looks fine to me.
>>> 
>>> In TrustedCert.java:
>>> 
>>> - You can use FileOutputStream and Files.copy(Path,OutputStream) in 
>>> cat().
>> 
>> This cat() is taken from WealAlg.java.
>> 
>>> 
>>> - There is no need to recreate root.jks and root.pem.
>> 
>> The sequences of the commands used in this test scenario allows me 
>> to test -printcert for the -trustcacerts and -keytsore options. We 
>> had discussion offline about it. The test uses trusted certificates 
>> and checks no warnings on the weak algorithms to address the 
>> requirement described in the bug. I believe it does serve that 
>> purpose, and looks legitimate to me. There could be different ways 
>> of testing a functionality, and please let me know if there is a 

Re: RFR 8244148: keytool -printcert and -printcrl should support the -trustcacerts and -keystore options

[sha . jiang]

Hi Hai-May,

On 2020/6/8 04:01, Hai-May Chao wrote:

Updated webrev -

https://cr.openjdk.java.net/~hchao/8244148/webrev.02/

-- src/java.base/share/classes/sun/security/util/FilePaths.java
Would it be better to use String.join() or even java.nio.file.Path to
build the file path?

-- src/java.base/share/classes/sun/security/util/AnchorCertificates.java
55 File f = new File(FilePaths.cacerts());
...
59 try (FileInputStream fis = new FileInputStream(f)) {
f is used only once, so it may be unnecessary.

56 KeyStore cacerts;
Though it's not in your change, would you mind to declare this variable
in the try block? I just want to narrow the variable scope.

-- test/lib/jdk/test/lib/SecurityTools.java
Could move method createCacerts() to 
test/lib/jdk/test/lib/security/KeyStoreUtils.java?

This class contains a set of methods for creating trust/key stores.
And getCertFromFile() in test/lib/jdk/test/lib/security/CertUtils.java
can load certificate from a file.

289 int pos = 0;
...
294 for (String crt : crts) {
If not using for-each style, pos can be declared in the for statement.

--
test/jdk/sun/security/tools/keytool/fakecacerts/java.base/sun/security/util/FilePaths.java
This patch can be used by other tests, so could you please move it to a
common path? Like 
test/jdk/sun/security/util/module_patch/java.base/sun/security/util/FilePaths.java


32 public static String cacerts() {
33 return "mycacerts";
34 }
Could it be flexible for returning an alternative path?
For example, accept a system property, say test.cacerts, for specifying
an alternative path. mycacerts is the default value of this property.

Best regards,
John Jiang



Thanks,
Hai-May


On Jun 5, 2020, at 11:04 PM, Weijun Wang > wrote:


I still think duplicated commands in TrustedCert.java are useless. 
Line 104 and line 133 are exactly the same, line 109 and line 138 are 
exactly the same, and you haven't made any change to these 2 files in 
between.


Same for line 80 and line 96 of TrustedCRL.java.

Everything else is fine.

Thanks,
Max


On Jun 6, 2020, at 2:25 AM, Hai-May Chao > wrote:


Updated webrev -

https://cr.openjdk.java.net/~hchao/8244148/webrev.01/

Added one command line -importcert in TrustCert.java.
Added createCacerts() in test/lib SecurityTools.java.

Thanks,
Hai-May




On Jun 4, 2020, at 5:57 AM, Weijun Wang  wrote:



On Jun 4, 2020, at 7:29 PM, Hai-May Chao  
wrote:


Hi Max,

On Jun 3, 2020, at 12:59 AM, Weijun Wang  
wrote:


The source change looks fine to me.

In TrustedCert.java:

- You can use FileOutputStream and Files.copy(Path,OutputStream) 
in cat().


This cat() is taken from WealAlg.java.



- There is no need to recreate root.jks and root.pem.


The sequences of the commands used in this test scenario allows me 
to test -printcert for the -trustcacerts and -keytsore options. We 
had discussion offline about it. The test uses trusted 
certificates and checks no warnings on the weak algorithms to 
address the requirement described in the bug. I believe it does 
serve that purpose, and looks legitimate to me. There could be 
different ways of testing a functionality, and please let me know 
if there is a problem with the current approach.


I just meant that the keytool commands generating root.jks and 
root.pem are exactly the same and there is no need to recreate it.




Please also elaborate your comment about no need to recreate 
root.jks and root.pem.




- Why not use -trustcacerts below?

160 kt("-importcert -file server.pem -noprompt", 
"server.jks”);



Because here is to import the server (end-entity) cert, and it 
will not make a difference for the test result whether to use the 
-trustcacerts or not. It’s the ca (intermediate) cert needs to 
have it in this test scenario. I intended to leave it out in #160 
to distinguish between server and ca certs.


OK.

Then how about we add a new command before line 155?

  kt("-importcert -file ca.pem", "ca.jks").shouldNotHaveExitValue(0);

This would prove the "-trustcacerts" on line 155 is really useful.





- It's probably better to add a " " between cmd and options in 
patchcmd(). Same in TrustedCRL.java.


Ok, will change it.



In TrustedCRL.java:

- No need to recreate ks and ca.crl. Just call "-printcrl" with 
different options.


Same reply as above.


Same question as above.





- Why create using MD5withRSA? Do you meant to warn about the 
weak algorithm?


Yes, exactly, and it differentiates from the weak algorithm 
SHA1withRSA used in root CA where no warning will be emitted. 
There is another -gencrl in #119 without using MD5withRSA so I’d 
have two test cases.




Also I would suggest you create a dedicate method (maybe in 
SecurityTools.java) to create your own cacerts. There is no need 
to copy over the system cacerts, just make sure the file is 
created with the JKS storetype. We are 

Re: RFR 8244148: keytool -printcert and -printcrl should support the -trustcacerts and -keystore options

[Weijun Wang]

"to a self-signed certificate in the keystore". This is not correct. What we 
look for is a TrustedCertificateEntry.

"It emits warning when a certificate is not trusted and uses weak algorithms". 
Precisely, it's "uses a weak signature algorithm".

--Max


> On Jun 10, 2020, at 5:31 PM, Hai-May Chao  wrote:
> 
> 
> 
>> On Jun 9, 2020, at 5:58 PM, Weijun Wang  wrote:
>> 
>> Good to see both -keystore and -trustcacerts mentioned. Some comments:
>> 
>> 1. I think there is no need to say "from -file cert_file". Or, do you mean 
>> the new function does not apply to those from -sslserver and -jarfile? If 
>> so, that might be a problem.
> 
> You’re right. Removed it.
> 
>> 
>> 2. While you said "attempts to construct a chain of trust", do you also want 
>> to describe what happens if it succeeds or fails?
> 
> Updated manpage.
> 
>> 
>> 3. It will be nice if you can include the exact diff of the man page files 
>> either inside the CSR itself or as a comment.
>> 
> 
> Included the diff of the manpage in the CSR.
> 
> Thanks,
> Hai-May
> 
> 
>> Thanks,
>> Max
>> 
>>> On Jun 9, 2020, at 10:51 PM, Hai-May Chao  wrote:
>>> 
>>> 
>>> 
 On Jun 7, 2020, at 6:08 PM, Weijun Wang  wrote:
 
 Looks fine to me.
 
 For CSR, since there is already a "Note" there for these 2 options, you 
 can add a few words about what -keystore and -trustcacerts can do.
>>> 
>>> Updated CSR as suggested.
>>> 
>>> Thanks,
>>> Hai-May
>>> 
>>> 
 
 Thanks,
 Max
 
> On Jun 8, 2020, at 4:01 AM, Hai-May Chao  wrote:
> 
> Updated webrev -
> 
> https://cr.openjdk.java.net/~hchao/8244148/webrev.02/
> 
> Thanks,
> Hai-May
> 
> 
>> On Jun 5, 2020, at 11:04 PM, Weijun Wang  wrote:
>> 
>> I still think duplicated commands in TrustedCert.java are useless. Line 
>> 104 and line 133 are exactly the same, line 109 and line 138 are exactly 
>> the same, and you haven't made any change to these 2 files in between.
>> 
>> Same for line 80 and line 96 of TrustedCRL.java.
>> 
>> Everything else is fine.
>> 
>> Thanks,
>> Max
>> 
>> 
>>> On Jun 6, 2020, at 2:25 AM, Hai-May Chao  
>>> wrote:
>>> 
>>> Updated webrev - 
>>> 
>>> https://cr.openjdk.java.net/~hchao/8244148/webrev.01/
>>> 
>>> Added one command line -importcert in TrustCert.java.
>>> Added createCacerts() in test/lib SecurityTools.java.
>>> 
>>> Thanks,
>>> Hai-May
>>> 
>>> 
>>> 
 On Jun 4, 2020, at 5:57 AM, Weijun Wang  wrote:
 
 
 
> On Jun 4, 2020, at 7:29 PM, Hai-May Chao  
> wrote:
> 
> Hi Max,
> 
>> On Jun 3, 2020, at 12:59 AM, Weijun Wang  
>> wrote:
>> 
>> The source change looks fine to me.
>> 
>> In TrustedCert.java:
>> 
>> - You can use FileOutputStream and Files.copy(Path,OutputStream) in 
>> cat().
> 
> This cat() is taken from WealAlg.java.
> 
>> 
>> - There is no need to recreate root.jks and root.pem.
> 
> The sequences of the commands used in this test scenario allows me to 
> test -printcert for the -trustcacerts and -keytsore options. We had 
> discussion offline about it. The test uses trusted certificates and 
> checks no warnings on the weak algorithms to address the requirement 
> described in the bug. I believe it does serve that purpose, and looks 
> legitimate to me. There could be different ways of testing a 
> functionality, and please let me know if there is a problem with the 
> current approach.
 
 I just meant that the keytool commands generating root.jks and 
 root.pem are exactly the same and there is no need to recreate it.
 
> 
> Please also elaborate your comment about no need to recreate root.jks 
> and root.pem.
> 
>> 
>> - Why not use -trustcacerts below?
>> 
>> 160 kt("-importcert -file server.pem -noprompt", 
>> "server.jks”);
> 
> 
> Because here is to import the server (end-entity) cert, and it will 
> not make a difference for the test result whether to use the 
> -trustcacerts or not. It’s the ca (intermediate) cert needs to have 
> it in this test scenario. I intended to leave it out in #160 to 
> distinguish between server and ca certs.
 
 OK.
 
 Then how about we add a new command before line 155?
 
 kt("-importcert -file ca.pem", "ca.jks").shouldNotHaveExitValue(0);
 
 This would prove the "-trustcacerts" on line 155 is really useful.
 
> 
>> 
>> - It's probably better to add a " " between cmd and options in 
>> patchcmd(). Same in 

Re: RFR 8244148: keytool -printcert and -printcrl should support the -trustcacerts and -keystore options

[Hai-May Chao]

> On Jun 9, 2020, at 5:58 PM, Weijun Wang  wrote:
> 
> Good to see both -keystore and -trustcacerts mentioned. Some comments:
> 
> 1. I think there is no need to say "from -file cert_file". Or, do you mean 
> the new function does not apply to those from -sslserver and -jarfile? If so, 
> that might be a problem.

You’re right. Removed it.

> 
> 2. While you said "attempts to construct a chain of trust", do you also want 
> to describe what happens if it succeeds or fails?

Updated manpage.

> 
> 3. It will be nice if you can include the exact diff of the man page files 
> either inside the CSR itself or as a comment.
> 

Included the diff of the manpage in the CSR.

Thanks,
Hai-May


> Thanks,
> Max
> 
>> On Jun 9, 2020, at 10:51 PM, Hai-May Chao  wrote:
>> 
>> 
>> 
>>> On Jun 7, 2020, at 6:08 PM, Weijun Wang  wrote:
>>> 
>>> Looks fine to me.
>>> 
>>> For CSR, since there is already a "Note" there for these 2 options, you can 
>>> add a few words about what -keystore and -trustcacerts can do.
>> 
>> Updated CSR as suggested.
>> 
>> Thanks,
>> Hai-May
>> 
>> 
>>> 
>>> Thanks,
>>> Max
>>> 
 On Jun 8, 2020, at 4:01 AM, Hai-May Chao  wrote:
 
 Updated webrev -
 
 https://cr.openjdk.java.net/~hchao/8244148/webrev.02/
 
 Thanks,
 Hai-May
 
 
> On Jun 5, 2020, at 11:04 PM, Weijun Wang  wrote:
> 
> I still think duplicated commands in TrustedCert.java are useless. Line 
> 104 and line 133 are exactly the same, line 109 and line 138 are exactly 
> the same, and you haven't made any change to these 2 files in between.
> 
> Same for line 80 and line 96 of TrustedCRL.java.
> 
> Everything else is fine.
> 
> Thanks,
> Max
> 
> 
>> On Jun 6, 2020, at 2:25 AM, Hai-May Chao  wrote:
>> 
>> Updated webrev - 
>> 
>> https://cr.openjdk.java.net/~hchao/8244148/webrev.01/
>> 
>> Added one command line -importcert in TrustCert.java.
>> Added createCacerts() in test/lib SecurityTools.java.
>> 
>> Thanks,
>> Hai-May
>> 
>> 
>> 
>>> On Jun 4, 2020, at 5:57 AM, Weijun Wang  wrote:
>>> 
>>> 
>>> 
 On Jun 4, 2020, at 7:29 PM, Hai-May Chao  
 wrote:
 
 Hi Max,
 
> On Jun 3, 2020, at 12:59 AM, Weijun Wang  
> wrote:
> 
> The source change looks fine to me.
> 
> In TrustedCert.java:
> 
> - You can use FileOutputStream and Files.copy(Path,OutputStream) in 
> cat().
 
 This cat() is taken from WealAlg.java.
 
> 
> - There is no need to recreate root.jks and root.pem.
 
 The sequences of the commands used in this test scenario allows me to 
 test -printcert for the -trustcacerts and -keytsore options. We had 
 discussion offline about it. The test uses trusted certificates and 
 checks no warnings on the weak algorithms to address the requirement 
 described in the bug. I believe it does serve that purpose, and looks 
 legitimate to me. There could be different ways of testing a 
 functionality, and please let me know if there is a problem with the 
 current approach.
>>> 
>>> I just meant that the keytool commands generating root.jks and root.pem 
>>> are exactly the same and there is no need to recreate it.
>>> 
 
 Please also elaborate your comment about no need to recreate root.jks 
 and root.pem.
 
> 
> - Why not use -trustcacerts below?
> 
> 160 kt("-importcert -file server.pem -noprompt", 
> "server.jks”);
 
 
 Because here is to import the server (end-entity) cert, and it will 
 not make a difference for the test result whether to use the 
 -trustcacerts or not. It’s the ca (intermediate) cert needs to have it 
 in this test scenario. I intended to leave it out in #160 to 
 distinguish between server and ca certs.
>>> 
>>> OK.
>>> 
>>> Then how about we add a new command before line 155?
>>> 
>>> kt("-importcert -file ca.pem", "ca.jks").shouldNotHaveExitValue(0);
>>> 
>>> This would prove the "-trustcacerts" on line 155 is really useful.
>>> 
 
> 
> - It's probably better to add a " " between cmd and options in 
> patchcmd(). Same in TrustedCRL.java.
 
 Ok, will change it.
 
> 
> In TrustedCRL.java:
> 
> - No need to recreate ks and ca.crl. Just call "-printcrl" with 
> different options.
 
 Same reply as above.
>>> 
>>> Same question as above.
>>> 
 
> 
> - Why create using MD5withRSA? Do you meant to warn about the weak 
> algorithm?
 
 Yes, exactly, and it differentiates from the weak 

Re: RFR 8244148: keytool -printcert and -printcrl should support the -trustcacerts and -keystore options

[Weijun Wang]

Good to see both -keystore and -trustcacerts mentioned. Some comments:

1. I think there is no need to say "from -file cert_file". Or, do you mean the 
new function does not apply to those from -sslserver and -jarfile? If so, that 
might be a problem.

2. While you said "attempts to construct a chain of trust", do you also want to 
describe what happens if it succeeds or fails?

3. It will be nice if you can include the exact diff of the man page files 
either inside the CSR itself or as a comment.

Thanks,
Max

> On Jun 9, 2020, at 10:51 PM, Hai-May Chao  wrote:
> 
> 
> 
>> On Jun 7, 2020, at 6:08 PM, Weijun Wang  wrote:
>> 
>> Looks fine to me.
>> 
>> For CSR, since there is already a "Note" there for these 2 options, you can 
>> add a few words about what -keystore and -trustcacerts can do.
> 
> Updated CSR as suggested.
> 
> Thanks,
> Hai-May
> 
> 
>> 
>> Thanks,
>> Max
>> 
>>> On Jun 8, 2020, at 4:01 AM, Hai-May Chao  wrote:
>>> 
>>> Updated webrev -
>>> 
>>> https://cr.openjdk.java.net/~hchao/8244148/webrev.02/
>>> 
>>> Thanks,
>>> Hai-May
>>> 
>>> 
 On Jun 5, 2020, at 11:04 PM, Weijun Wang  wrote:
 
 I still think duplicated commands in TrustedCert.java are useless. Line 
 104 and line 133 are exactly the same, line 109 and line 138 are exactly 
 the same, and you haven't made any change to these 2 files in between.
 
 Same for line 80 and line 96 of TrustedCRL.java.
 
 Everything else is fine.
 
 Thanks,
 Max
 
 
> On Jun 6, 2020, at 2:25 AM, Hai-May Chao  wrote:
> 
> Updated webrev - 
> 
> https://cr.openjdk.java.net/~hchao/8244148/webrev.01/
> 
> Added one command line -importcert in TrustCert.java.
> Added createCacerts() in test/lib SecurityTools.java.
> 
> Thanks,
> Hai-May
> 
> 
> 
>> On Jun 4, 2020, at 5:57 AM, Weijun Wang  wrote:
>> 
>> 
>> 
>>> On Jun 4, 2020, at 7:29 PM, Hai-May Chao  
>>> wrote:
>>> 
>>> Hi Max,
>>> 
 On Jun 3, 2020, at 12:59 AM, Weijun Wang  
 wrote:
 
 The source change looks fine to me.
 
 In TrustedCert.java:
 
 - You can use FileOutputStream and Files.copy(Path,OutputStream) in 
 cat().
>>> 
>>> This cat() is taken from WealAlg.java.
>>> 
 
 - There is no need to recreate root.jks and root.pem.
>>> 
>>> The sequences of the commands used in this test scenario allows me to 
>>> test -printcert for the -trustcacerts and -keytsore options. We had 
>>> discussion offline about it. The test uses trusted certificates and 
>>> checks no warnings on the weak algorithms to address the requirement 
>>> described in the bug. I believe it does serve that purpose, and looks 
>>> legitimate to me. There could be different ways of testing a 
>>> functionality, and please let me know if there is a problem with the 
>>> current approach.
>> 
>> I just meant that the keytool commands generating root.jks and root.pem 
>> are exactly the same and there is no need to recreate it.
>> 
>>> 
>>> Please also elaborate your comment about no need to recreate root.jks 
>>> and root.pem.
>>> 
 
 - Why not use -trustcacerts below?
 
 160 kt("-importcert -file server.pem -noprompt", "server.jks”);
>>> 
>>> 
>>> Because here is to import the server (end-entity) cert, and it will not 
>>> make a difference for the test result whether to use the -trustcacerts 
>>> or not. It’s the ca (intermediate) cert needs to have it in this test 
>>> scenario. I intended to leave it out in #160 to distinguish between 
>>> server and ca certs.
>> 
>> OK.
>> 
>> Then how about we add a new command before line 155?
>> 
>> kt("-importcert -file ca.pem", "ca.jks").shouldNotHaveExitValue(0);
>> 
>> This would prove the "-trustcacerts" on line 155 is really useful.
>> 
>>> 
 
 - It's probably better to add a " " between cmd and options in 
 patchcmd(). Same in TrustedCRL.java.
>>> 
>>> Ok, will change it.
>>> 
 
 In TrustedCRL.java:
 
 - No need to recreate ks and ca.crl. Just call "-printcrl" with 
 different options.
>>> 
>>> Same reply as above.
>> 
>> Same question as above.
>> 
>>> 
 
 - Why create using MD5withRSA? Do you meant to warn about the weak 
 algorithm?
>>> 
>>> Yes, exactly, and it differentiates from the weak algorithm SHA1withRSA 
>>> used in root CA where no warning will be emitted. There is another 
>>> -gencrl in #119 without using MD5withRSA so I’d have two test cases.
>>> 
 
 Also I would suggest you create a dedicate method (maybe in 
 SecurityTools.java) to create your own cacerts. There is no need to 
 

Re: RFR 8244148: keytool -printcert and -printcrl should support the -trustcacerts and -keystore options

[Hai-May Chao]

> On Jun 7, 2020, at 6:08 PM, Weijun Wang  wrote:
> 
> Looks fine to me.
> 
> For CSR, since there is already a "Note" there for these 2 options, you can 
> add a few words about what -keystore and -trustcacerts can do.

Updated CSR as suggested.

Thanks,
Hai-May


> 
> Thanks,
> Max
> 
>> On Jun 8, 2020, at 4:01 AM, Hai-May Chao  wrote:
>> 
>> Updated webrev -
>> 
>> https://cr.openjdk.java.net/~hchao/8244148/webrev.02/
>> 
>> Thanks,
>> Hai-May
>> 
>> 
>>> On Jun 5, 2020, at 11:04 PM, Weijun Wang  wrote:
>>> 
>>> I still think duplicated commands in TrustedCert.java are useless. Line 104 
>>> and line 133 are exactly the same, line 109 and line 138 are exactly the 
>>> same, and you haven't made any change to these 2 files in between.
>>> 
>>> Same for line 80 and line 96 of TrustedCRL.java.
>>> 
>>> Everything else is fine.
>>> 
>>> Thanks,
>>> Max
>>> 
>>> 
 On Jun 6, 2020, at 2:25 AM, Hai-May Chao  wrote:
 
 Updated webrev - 
 
 https://cr.openjdk.java.net/~hchao/8244148/webrev.01/
 
 Added one command line -importcert in TrustCert.java.
 Added createCacerts() in test/lib SecurityTools.java.
 
 Thanks,
 Hai-May
 
 
 
> On Jun 4, 2020, at 5:57 AM, Weijun Wang  wrote:
> 
> 
> 
>> On Jun 4, 2020, at 7:29 PM, Hai-May Chao  wrote:
>> 
>> Hi Max,
>> 
>>> On Jun 3, 2020, at 12:59 AM, Weijun Wang  wrote:
>>> 
>>> The source change looks fine to me.
>>> 
>>> In TrustedCert.java:
>>> 
>>> - You can use FileOutputStream and Files.copy(Path,OutputStream) in 
>>> cat().
>> 
>> This cat() is taken from WealAlg.java.
>> 
>>> 
>>> - There is no need to recreate root.jks and root.pem.
>> 
>> The sequences of the commands used in this test scenario allows me to 
>> test -printcert for the -trustcacerts and -keytsore options. We had 
>> discussion offline about it. The test uses trusted certificates and 
>> checks no warnings on the weak algorithms to address the requirement 
>> described in the bug. I believe it does serve that purpose, and looks 
>> legitimate to me. There could be different ways of testing a 
>> functionality, and please let me know if there is a problem with the 
>> current approach.
> 
> I just meant that the keytool commands generating root.jks and root.pem 
> are exactly the same and there is no need to recreate it.
> 
>> 
>> Please also elaborate your comment about no need to recreate root.jks 
>> and root.pem.
>> 
>>> 
>>> - Why not use -trustcacerts below?
>>> 
>>> 160 kt("-importcert -file server.pem -noprompt", "server.jks”);
>> 
>> 
>> Because here is to import the server (end-entity) cert, and it will not 
>> make a difference for the test result whether to use the -trustcacerts 
>> or not. It’s the ca (intermediate) cert needs to have it in this test 
>> scenario. I intended to leave it out in #160 to distinguish between 
>> server and ca certs.
> 
> OK.
> 
> Then how about we add a new command before line 155?
> 
>  kt("-importcert -file ca.pem", "ca.jks").shouldNotHaveExitValue(0);
> 
> This would prove the "-trustcacerts" on line 155 is really useful.
> 
>> 
>>> 
>>> - It's probably better to add a " " between cmd and options in 
>>> patchcmd(). Same in TrustedCRL.java.
>> 
>> Ok, will change it.
>> 
>>> 
>>> In TrustedCRL.java:
>>> 
>>> - No need to recreate ks and ca.crl. Just call "-printcrl" with 
>>> different options.
>> 
>> Same reply as above.
> 
> Same question as above.
> 
>> 
>>> 
>>> - Why create using MD5withRSA? Do you meant to warn about the weak 
>>> algorithm?
>> 
>> Yes, exactly, and it differentiates from the weak algorithm SHA1withRSA 
>> used in root CA where no warning will be emitted. There is another 
>> -gencrl in #119 without using MD5withRSA so I’d have two test cases.
>> 
>>> 
>>> Also I would suggest you create a dedicate method (maybe in 
>>> SecurityTools.java) to create your own cacerts. There is no need to 
>>> copy over the system cacerts, just make sure the file is created with 
>>> the JKS storetype. We are thinking of upgrading the storetype of 
>>> cacerts and it's nice to do this at a single place so we can modify it 
>>> easily later.
>> 
>> I created a method in SecurityTools.java to create the own cacerts. With 
>> this keystore, the subsequent importing a certificate reply would not 
>> work. It turns out that its caks.size() is zero detected at 
>> establishCertChain() in keytool/Main.java after root cert has been 
>> imported to that cacerts. At this point I’d like to suggest a separate 
>> bug be filed to cover the cacerts enhancement that you suggested.
> 
> I meant creating the 

Re: RFR 8244148: keytool -printcert and -printcrl should support the -trustcacerts and -keystore options

[Weijun Wang]

Looks fine to me.

For CSR, since there is already a "Note" there for these 2 options, you can add 
a few words about what -keystore and -trustcacerts can do.

Thanks,
Max

> On Jun 8, 2020, at 4:01 AM, Hai-May Chao  wrote:
> 
> Updated webrev -
> 
> https://cr.openjdk.java.net/~hchao/8244148/webrev.02/
> 
> Thanks,
> Hai-May
> 
> 
>> On Jun 5, 2020, at 11:04 PM, Weijun Wang  wrote:
>> 
>> I still think duplicated commands in TrustedCert.java are useless. Line 104 
>> and line 133 are exactly the same, line 109 and line 138 are exactly the 
>> same, and you haven't made any change to these 2 files in between.
>> 
>> Same for line 80 and line 96 of TrustedCRL.java.
>> 
>> Everything else is fine.
>> 
>> Thanks,
>> Max
>> 
>> 
>>> On Jun 6, 2020, at 2:25 AM, Hai-May Chao  wrote:
>>> 
>>> Updated webrev - 
>>> 
>>> https://cr.openjdk.java.net/~hchao/8244148/webrev.01/
>>> 
>>> Added one command line -importcert in TrustCert.java.
>>> Added createCacerts() in test/lib SecurityTools.java.
>>> 
>>> Thanks,
>>> Hai-May
>>> 
>>> 
>>> 
 On Jun 4, 2020, at 5:57 AM, Weijun Wang  wrote:
 
 
 
> On Jun 4, 2020, at 7:29 PM, Hai-May Chao  wrote:
> 
> Hi Max,
> 
>> On Jun 3, 2020, at 12:59 AM, Weijun Wang  wrote:
>> 
>> The source change looks fine to me.
>> 
>> In TrustedCert.java:
>> 
>> - You can use FileOutputStream and Files.copy(Path,OutputStream) in 
>> cat().
> 
> This cat() is taken from WealAlg.java.
> 
>> 
>> - There is no need to recreate root.jks and root.pem.
> 
> The sequences of the commands used in this test scenario allows me to 
> test -printcert for the -trustcacerts and -keytsore options. We had 
> discussion offline about it. The test uses trusted certificates and 
> checks no warnings on the weak algorithms to address the requirement 
> described in the bug. I believe it does serve that purpose, and looks 
> legitimate to me. There could be different ways of testing a 
> functionality, and please let me know if there is a problem with the 
> current approach.
 
 I just meant that the keytool commands generating root.jks and root.pem 
 are exactly the same and there is no need to recreate it.
 
> 
> Please also elaborate your comment about no need to recreate root.jks and 
> root.pem.
> 
>> 
>> - Why not use -trustcacerts below?
>> 
>> 160 kt("-importcert -file server.pem -noprompt", "server.jks”);
> 
> 
> Because here is to import the server (end-entity) cert, and it will not 
> make a difference for the test result whether to use the -trustcacerts or 
> not. It’s the ca (intermediate) cert needs to have it in this test 
> scenario. I intended to leave it out in #160 to distinguish between 
> server and ca certs.
 
 OK.
 
 Then how about we add a new command before line 155?
 
   kt("-importcert -file ca.pem", "ca.jks").shouldNotHaveExitValue(0);
 
 This would prove the "-trustcacerts" on line 155 is really useful.
 
> 
>> 
>> - It's probably better to add a " " between cmd and options in 
>> patchcmd(). Same in TrustedCRL.java.
> 
> Ok, will change it.
> 
>> 
>> In TrustedCRL.java:
>> 
>> - No need to recreate ks and ca.crl. Just call "-printcrl" with 
>> different options.
> 
> Same reply as above.
 
 Same question as above.
 
> 
>> 
>> - Why create using MD5withRSA? Do you meant to warn about the weak 
>> algorithm?
> 
> Yes, exactly, and it differentiates from the weak algorithm SHA1withRSA 
> used in root CA where no warning will be emitted. There is another 
> -gencrl in #119 without using MD5withRSA so I’d have two test cases.
> 
>> 
>> Also I would suggest you create a dedicate method (maybe in 
>> SecurityTools.java) to create your own cacerts. There is no need to copy 
>> over the system cacerts, just make sure the file is created with the JKS 
>> storetype. We are thinking of upgrading the storetype of cacerts and 
>> it's nice to do this at a single place so we can modify it easily later.
> 
> I created a method in SecurityTools.java to create the own cacerts. With 
> this keystore, the subsequent importing a certificate reply would not 
> work. It turns out that its caks.size() is zero detected at 
> establishCertChain() in keytool/Main.java after root cert has been 
> imported to that cacerts. At this point I’d like to suggest a separate 
> bug be filed to cover the cacerts enhancement that you suggested.
 
 I meant creating the cacerts in one method, something like
 
  void createCacerts(String ks, String... crt);
 
 and you can call createCacerts("mycacerts", "root.crt") to create it. The 
 method can call KeyStore APIs and not keytool commands.
 
 BTW, what 

Re: RFR 8244148: keytool -printcert and -printcrl should support the -trustcacerts and -keystore options

[Hai-May Chao]

Updated webrev -

https://cr.openjdk.java.net/~hchao/8244148/webrev.02/ 


Thanks,
Hai-May


> On Jun 5, 2020, at 11:04 PM, Weijun Wang  wrote:
> 
> I still think duplicated commands in TrustedCert.java are useless. Line 104 
> and line 133 are exactly the same, line 109 and line 138 are exactly the 
> same, and you haven't made any change to these 2 files in between.
> 
> Same for line 80 and line 96 of TrustedCRL.java.
> 
> Everything else is fine.
> 
> Thanks,
> Max
> 
> 
>> On Jun 6, 2020, at 2:25 AM, Hai-May Chao  wrote:
>> 
>> Updated webrev - 
>> 
>> https://cr.openjdk.java.net/~hchao/8244148/webrev.01/
>> 
>> Added one command line -importcert in TrustCert.java.
>> Added createCacerts() in test/lib SecurityTools.java.
>> 
>> Thanks,
>> Hai-May
>> 
>> 
>> 
>>> On Jun 4, 2020, at 5:57 AM, Weijun Wang  wrote:
>>> 
>>> 
>>> 
 On Jun 4, 2020, at 7:29 PM, Hai-May Chao  wrote:
 
 Hi Max,
 
> On Jun 3, 2020, at 12:59 AM, Weijun Wang  wrote:
> 
> The source change looks fine to me.
> 
> In TrustedCert.java:
> 
> - You can use FileOutputStream and Files.copy(Path,OutputStream) in cat().
 
 This cat() is taken from WealAlg.java.
 
> 
> - There is no need to recreate root.jks and root.pem.
 
 The sequences of the commands used in this test scenario allows me to test 
 -printcert for the -trustcacerts and -keytsore options. We had discussion 
 offline about it. The test uses trusted certificates and checks no 
 warnings on the weak algorithms to address the requirement described in 
 the bug. I believe it does serve that purpose, and looks legitimate to me. 
 There could be different ways of testing a functionality, and please let 
 me know if there is a problem with the current approach.
>>> 
>>> I just meant that the keytool commands generating root.jks and root.pem are 
>>> exactly the same and there is no need to recreate it.
>>> 
 
 Please also elaborate your comment about no need to recreate root.jks and 
 root.pem.
 
> 
> - Why not use -trustcacerts below?
> 
> 160 kt("-importcert -file server.pem -noprompt", "server.jks”);
 
 
 Because here is to import the server (end-entity) cert, and it will not 
 make a difference for the test result whether to use the -trustcacerts or 
 not. It’s the ca (intermediate) cert needs to have it in this test 
 scenario. I intended to leave it out in #160 to distinguish between server 
 and ca certs.
>>> 
>>> OK.
>>> 
>>> Then how about we add a new command before line 155?
>>> 
>>>   kt("-importcert -file ca.pem", "ca.jks").shouldNotHaveExitValue(0);
>>> 
>>> This would prove the "-trustcacerts" on line 155 is really useful.
>>> 
 
> 
> - It's probably better to add a " " between cmd and options in 
> patchcmd(). Same in TrustedCRL.java.
 
 Ok, will change it.
 
> 
> In TrustedCRL.java:
> 
> - No need to recreate ks and ca.crl. Just call "-printcrl" with different 
> options.
 
 Same reply as above.
>>> 
>>> Same question as above.
>>> 
 
> 
> - Why create using MD5withRSA? Do you meant to warn about the weak 
> algorithm?
 
 Yes, exactly, and it differentiates from the weak algorithm SHA1withRSA 
 used in root CA where no warning will be emitted. There is another -gencrl 
 in #119 without using MD5withRSA so I’d have two test cases.
 
> 
> Also I would suggest you create a dedicate method (maybe in 
> SecurityTools.java) to create your own cacerts. There is no need to copy 
> over the system cacerts, just make sure the file is created with the JKS 
> storetype. We are thinking of upgrading the storetype of cacerts and it's 
> nice to do this at a single place so we can modify it easily later.
 
 I created a method in SecurityTools.java to create the own cacerts. With 
 this keystore, the subsequent importing a certificate reply would not 
 work. It turns out that its caks.size() is zero detected at 
 establishCertChain() in keytool/Main.java after root cert has been 
 imported to that cacerts. At this point I’d like to suggest a separate bug 
 be filed to cover the cacerts enhancement that you suggested.
>>> 
>>> I meant creating the cacerts in one method, something like
>>> 
>>>  void createCacerts(String ks, String... crt);
>>> 
>>> and you can call createCacerts("mycacerts", "root.crt") to create it. The 
>>> method can call KeyStore APIs and not keytool commands.
>>> 
>>> BTW, what does caks.size() == 0 matter here?
>>> 
>>> Thanks,
>>> Max
>>> 
>>> 
 
 Thanks,
 Hai-May
 
 
> Thanks,
> Max
> 
> 
>> On Jun 2, 2020, at 2:37 AM, Hai-May Chao  wrote:
>> 
>> Hi,
>> 
>> I’d like to request a review for:
>> 
>> JBS: 

Re: RFR 8244148: keytool -printcert and -printcrl should support the -trustcacerts and -keystore options

[Weijun Wang]

I still think duplicated commands in TrustedCert.java are useless. Line 104 and 
line 133 are exactly the same, line 109 and line 138 are exactly the same, and 
you haven't made any change to these 2 files in between.

Same for line 80 and line 96 of TrustedCRL.java.

Everything else is fine.

Thanks,
Max


> On Jun 6, 2020, at 2:25 AM, Hai-May Chao  wrote:
> 
> Updated webrev - 
> 
> https://cr.openjdk.java.net/~hchao/8244148/webrev.01/
> 
> Added one command line -importcert in TrustCert.java.
> Added createCacerts() in test/lib SecurityTools.java.
> 
> Thanks,
> Hai-May
> 
> 
> 
>> On Jun 4, 2020, at 5:57 AM, Weijun Wang  wrote:
>> 
>> 
>> 
>>> On Jun 4, 2020, at 7:29 PM, Hai-May Chao  wrote:
>>> 
>>> Hi Max,
>>> 
 On Jun 3, 2020, at 12:59 AM, Weijun Wang  wrote:
 
 The source change looks fine to me.
 
 In TrustedCert.java:
 
 - You can use FileOutputStream and Files.copy(Path,OutputStream) in cat().
>>> 
>>> This cat() is taken from WealAlg.java.
>>> 
 
 - There is no need to recreate root.jks and root.pem.
>>> 
>>> The sequences of the commands used in this test scenario allows me to test 
>>> -printcert for the -trustcacerts and -keytsore options. We had discussion 
>>> offline about it. The test uses trusted certificates and checks no warnings 
>>> on the weak algorithms to address the requirement described in the bug. I 
>>> believe it does serve that purpose, and looks legitimate to me. There could 
>>> be different ways of testing a functionality, and please let me know if 
>>> there is a problem with the current approach.
>> 
>> I just meant that the keytool commands generating root.jks and root.pem are 
>> exactly the same and there is no need to recreate it.
>> 
>>> 
>>> Please also elaborate your comment about no need to recreate root.jks and 
>>> root.pem.
>>> 
 
 - Why not use -trustcacerts below?
 
 160 kt("-importcert -file server.pem -noprompt", "server.jks”);
>>> 
>>> 
>>> Because here is to import the server (end-entity) cert, and it will not 
>>> make a difference for the test result whether to use the -trustcacerts or 
>>> not. It’s the ca (intermediate) cert needs to have it in this test 
>>> scenario. I intended to leave it out in #160 to distinguish between server 
>>> and ca certs.
>> 
>> OK.
>> 
>> Then how about we add a new command before line 155?
>> 
>>kt("-importcert -file ca.pem", "ca.jks").shouldNotHaveExitValue(0);
>> 
>> This would prove the "-trustcacerts" on line 155 is really useful.
>> 
>>> 
 
 - It's probably better to add a " " between cmd and options in patchcmd(). 
 Same in TrustedCRL.java.
>>> 
>>> Ok, will change it.
>>> 
 
 In TrustedCRL.java:
 
 - No need to recreate ks and ca.crl. Just call "-printcrl" with different 
 options.
>>> 
>>> Same reply as above.
>> 
>> Same question as above.
>> 
>>> 
 
 - Why create using MD5withRSA? Do you meant to warn about the weak 
 algorithm?
>>> 
>>> Yes, exactly, and it differentiates from the weak algorithm SHA1withRSA 
>>> used in root CA where no warning will be emitted. There is another -gencrl 
>>> in #119 without using MD5withRSA so I’d have two test cases.
>>> 
 
 Also I would suggest you create a dedicate method (maybe in 
 SecurityTools.java) to create your own cacerts. There is no need to copy 
 over the system cacerts, just make sure the file is created with the JKS 
 storetype. We are thinking of upgrading the storetype of cacerts and it's 
 nice to do this at a single place so we can modify it easily later.
>>> 
>>> I created a method in SecurityTools.java to create the own cacerts. With 
>>> this keystore, the subsequent importing a certificate reply would not work. 
>>> It turns out that its caks.size() is zero detected at establishCertChain() 
>>> in keytool/Main.java after root cert has been imported to that cacerts. At 
>>> this point I’d like to suggest a separate bug be filed to cover the cacerts 
>>> enhancement that you suggested.
>> 
>> I meant creating the cacerts in one method, something like
>> 
>>   void createCacerts(String ks, String... crt);
>> 
>> and you can call createCacerts("mycacerts", "root.crt") to create it. The 
>> method can call KeyStore APIs and not keytool commands.
>> 
>> BTW, what does caks.size() == 0 matter here?
>> 
>> Thanks,
>> Max
>> 
>> 
>>> 
>>> Thanks,
>>> Hai-May
>>> 
>>> 
 Thanks,
 Max
 
 
> On Jun 2, 2020, at 2:37 AM, Hai-May Chao  wrote:
> 
> Hi,
> 
> I’d like to request a review for:
> 
> JBS: https://bugs.openjdk.java.net/browse/JDK-8244148
> CSR: https://bugs.openjdk.java.net/browse/JDK-8246269
> Webrev: http://cr.openjdk.java.net/~hchao/8244148/webrev.00/
> 
> The change is to add the support of -trustcacerts and -keystore options 
> to -printcert and -princrl command for keytool. This enables keytool to 
> use the trusted certificates when verifying untrusted 

Re: RFR 8244148: keytool -printcert and -printcrl should support the -trustcacerts and -keystore options

[Hai-May Chao]

Updated webrev - 

https://cr.openjdk.java.net/~hchao/8244148/webrev.01/

Added one command line -importcert in TrustCert.java.
Added createCacerts() in test/lib SecurityTools.java.

Thanks,
Hai-May



> On Jun 4, 2020, at 5:57 AM, Weijun Wang  wrote:
> 
> 
> 
>> On Jun 4, 2020, at 7:29 PM, Hai-May Chao  wrote:
>> 
>> Hi Max,
>> 
>>> On Jun 3, 2020, at 12:59 AM, Weijun Wang  wrote:
>>> 
>>> The source change looks fine to me.
>>> 
>>> In TrustedCert.java:
>>> 
>>> - You can use FileOutputStream and Files.copy(Path,OutputStream) in cat().
>> 
>> This cat() is taken from WealAlg.java.
>> 
>>> 
>>> - There is no need to recreate root.jks and root.pem.
>> 
>> The sequences of the commands used in this test scenario allows me to test 
>> -printcert for the -trustcacerts and -keytsore options. We had discussion 
>> offline about it. The test uses trusted certificates and checks no warnings 
>> on the weak algorithms to address the requirement described in the bug. I 
>> believe it does serve that purpose, and looks legitimate to me. There could 
>> be different ways of testing a functionality, and please let me know if 
>> there is a problem with the current approach.
> 
> I just meant that the keytool commands generating root.jks and root.pem are 
> exactly the same and there is no need to recreate it.
> 
>> 
>> Please also elaborate your comment about no need to recreate root.jks and 
>> root.pem.
>> 
>>> 
>>> - Why not use -trustcacerts below?
>>> 
>>> 160 kt("-importcert -file server.pem -noprompt", "server.jks”);
>> 
>> 
>> Because here is to import the server (end-entity) cert, and it will not make 
>> a difference for the test result whether to use the -trustcacerts or not. 
>> It’s the ca (intermediate) cert needs to have it in this test scenario. I 
>> intended to leave it out in #160 to distinguish between server and ca certs.
> 
> OK.
> 
> Then how about we add a new command before line 155?
> 
>kt("-importcert -file ca.pem", "ca.jks").shouldNotHaveExitValue(0);
> 
> This would prove the "-trustcacerts" on line 155 is really useful.
> 
>> 
>>> 
>>> - It's probably better to add a " " between cmd and options in patchcmd(). 
>>> Same in TrustedCRL.java.
>> 
>> Ok, will change it.
>> 
>>> 
>>> In TrustedCRL.java:
>>> 
>>> - No need to recreate ks and ca.crl. Just call "-printcrl" with different 
>>> options.
>> 
>> Same reply as above.
> 
> Same question as above.
> 
>> 
>>> 
>>> - Why create using MD5withRSA? Do you meant to warn about the weak 
>>> algorithm?
>> 
>> Yes, exactly, and it differentiates from the weak algorithm SHA1withRSA used 
>> in root CA where no warning will be emitted. There is another -gencrl in 
>> #119 without using MD5withRSA so I’d have two test cases.
>> 
>>> 
>>> Also I would suggest you create a dedicate method (maybe in 
>>> SecurityTools.java) to create your own cacerts. There is no need to copy 
>>> over the system cacerts, just make sure the file is created with the JKS 
>>> storetype. We are thinking of upgrading the storetype of cacerts and it's 
>>> nice to do this at a single place so we can modify it easily later.
>> 
>> I created a method in SecurityTools.java to create the own cacerts. With 
>> this keystore, the subsequent importing a certificate reply would not work. 
>> It turns out that its caks.size() is zero detected at establishCertChain() 
>> in keytool/Main.java after root cert has been imported to that cacerts. At 
>> this point I’d like to suggest a separate bug be filed to cover the cacerts 
>> enhancement that you suggested.
> 
> I meant creating the cacerts in one method, something like
> 
>   void createCacerts(String ks, String... crt);
> 
> and you can call createCacerts("mycacerts", "root.crt") to create it. The 
> method can call KeyStore APIs and not keytool commands.
> 
> BTW, what does caks.size() == 0 matter here?
> 
> Thanks,
> Max
> 
> 
>> 
>> Thanks,
>> Hai-May
>> 
>> 
>>> Thanks,
>>> Max
>>> 
>>> 
 On Jun 2, 2020, at 2:37 AM, Hai-May Chao  wrote:
 
 Hi,
 
 I’d like to request a review for:
 
 JBS: https://bugs.openjdk.java.net/browse/JDK-8244148
 CSR: https://bugs.openjdk.java.net/browse/JDK-8246269
 Webrev: http://cr.openjdk.java.net/~hchao/8244148/webrev.00/
 
 The change is to add the support of -trustcacerts and -keystore options to 
 -printcert and -princrl command for keytool. This enables keytool to use 
 the trusted certificates when verifying untrusted artifacts that are 
 signed by CAs. It also incorporates Max’s change that consolidates the 
 code to get the default location of cacerts keystore.
 
 Thanks,
 Hai-May

Re: RFR 8244148: keytool -printcert and -printcrl should support the -trustcacerts and -keystore options

[Weijun Wang]

> On Jun 4, 2020, at 7:29 PM, Hai-May Chao  wrote:
> 
> Hi Max,
> 
>> On Jun 3, 2020, at 12:59 AM, Weijun Wang  wrote:
>> 
>> The source change looks fine to me.
>> 
>> In TrustedCert.java:
>> 
>> - You can use FileOutputStream and Files.copy(Path,OutputStream) in cat().
> 
> This cat() is taken from WealAlg.java.
> 
>> 
>> - There is no need to recreate root.jks and root.pem.
> 
> The sequences of the commands used in this test scenario allows me to test 
> -printcert for the -trustcacerts and -keytsore options. We had discussion 
> offline about it. The test uses trusted certificates and checks no warnings 
> on the weak algorithms to address the requirement described in the bug. I 
> believe it does serve that purpose, and looks legitimate to me. There could 
> be different ways of testing a functionality, and please let me know if there 
> is a problem with the current approach.

I just meant that the keytool commands generating root.jks and root.pem are 
exactly the same and there is no need to recreate it.

> 
> Please also elaborate your comment about no need to recreate root.jks and 
> root.pem.
> 
>> 
>> - Why not use -trustcacerts below?
>> 
>> 160 kt("-importcert -file server.pem -noprompt", "server.jks”);
> 
> 
> Because here is to import the server (end-entity) cert, and it will not make 
> a difference for the test result whether to use the -trustcacerts or not. 
> It’s the ca (intermediate) cert needs to have it in this test scenario. I 
> intended to leave it out in #160 to distinguish between server and ca certs.

OK.

Then how about we add a new command before line 155?

kt("-importcert -file ca.pem", "ca.jks").shouldNotHaveExitValue(0);

This would prove the "-trustcacerts" on line 155 is really useful.

> 
>> 
>> - It's probably better to add a " " between cmd and options in patchcmd(). 
>> Same in TrustedCRL.java.
> 
> Ok, will change it.
> 
>> 
>> In TrustedCRL.java:
>> 
>> - No need to recreate ks and ca.crl. Just call "-printcrl" with different 
>> options.
> 
> Same reply as above.

Same question as above.

> 
>> 
>> - Why create using MD5withRSA? Do you meant to warn about the weak algorithm?
> 
> Yes, exactly, and it differentiates from the weak algorithm SHA1withRSA used 
> in root CA where no warning will be emitted. There is another -gencrl in #119 
> without using MD5withRSA so I’d have two test cases.
> 
>> 
>> Also I would suggest you create a dedicate method (maybe in 
>> SecurityTools.java) to create your own cacerts. There is no need to copy 
>> over the system cacerts, just make sure the file is created with the JKS 
>> storetype. We are thinking of upgrading the storetype of cacerts and it's 
>> nice to do this at a single place so we can modify it easily later.
> 
> I created a method in SecurityTools.java to create the own cacerts. With this 
> keystore, the subsequent importing a certificate reply would not work. It 
> turns out that its caks.size() is zero detected at establishCertChain() in 
> keytool/Main.java after root cert has been imported to that cacerts. At this 
> point I’d like to suggest a separate bug be filed to cover the cacerts 
> enhancement that you suggested.

I meant creating the cacerts in one method, something like

   void createCacerts(String ks, String... crt);

and you can call createCacerts("mycacerts", "root.crt") to create it. The 
method can call KeyStore APIs and not keytool commands.

BTW, what does caks.size() == 0 matter here?

Thanks,
Max


> 
> Thanks,
> Hai-May
> 
> 
>> Thanks,
>> Max
>> 
>> 
>>> On Jun 2, 2020, at 2:37 AM, Hai-May Chao  wrote:
>>> 
>>> Hi,
>>> 
>>> I’d like to request a review for:
>>> 
>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8244148
>>> CSR: https://bugs.openjdk.java.net/browse/JDK-8246269
>>> Webrev: http://cr.openjdk.java.net/~hchao/8244148/webrev.00/
>>> 
>>> The change is to add the support of -trustcacerts and -keystore options to 
>>> -printcert and -princrl command for keytool. This enables keytool to use 
>>> the trusted certificates when verifying untrusted artifacts that are signed 
>>> by CAs. It also incorporates Max’s change that consolidates the code to get 
>>> the default location of cacerts keystore.
>>> 
>>> Thanks,
>>> Hai-May
>>> 
>> 
> 

Re: RFR 8244148: keytool -printcert and -printcrl should support the -trustcacerts and -keystore options

[Hai-May Chao]

Hi Max,

> On Jun 3, 2020, at 12:59 AM, Weijun Wang  wrote:
> 
> The source change looks fine to me.
> 
> In TrustedCert.java:
> 
> - You can use FileOutputStream and Files.copy(Path,OutputStream) in cat().

This cat() is taken from WealAlg.java.

> 
> - There is no need to recreate root.jks and root.pem.

The sequences of the commands used in this test scenario allows me to test 
-printcert for the -trustcacerts and -keytsore options. We had discussion 
offline about it. The test uses trusted certificates and checks no warnings on 
the weak algorithms to address the requirement described in the bug. I believe 
it does serve that purpose, and looks legitimate to me. There could be 
different ways of testing a functionality, and please let me know if there is a 
problem with the current approach.

Please also elaborate your comment about no need to recreate root.jks and 
root.pem.

> 
> - Why not use -trustcacerts below?
> 
> 160 kt("-importcert -file server.pem -noprompt", "server.jks”);


Because here is to import the server (end-entity) cert, and it will not make a 
difference for the test result whether to use the -trustcacerts or not. It’s 
the ca (intermediate) cert needs to have it in this test scenario. I intended 
to leave it out in #160 to distinguish between server and ca certs.

> 
> - It's probably better to add a " " between cmd and options in patchcmd(). 
> Same in TrustedCRL.java.

Ok, will change it.

> 
> In TrustedCRL.java:
> 
> - No need to recreate ks and ca.crl. Just call "-printcrl" with different 
> options.

Same reply as above.

> 
> - Why create using MD5withRSA? Do you meant to warn about the weak algorithm?

Yes, exactly, and it differentiates from the weak algorithm SHA1withRSA used in 
root CA where no warning will be emitted. There is another -gencrl in #119 
without using MD5withRSA so I’d have two test cases.

> 
> Also I would suggest you create a dedicate method (maybe in 
> SecurityTools.java) to create your own cacerts. There is no need to copy over 
> the system cacerts, just make sure the file is created with the JKS 
> storetype. We are thinking of upgrading the storetype of cacerts and it's 
> nice to do this at a single place so we can modify it easily later.

I created a method in SecurityTools.java to create the own cacerts. With this 
keystore, the subsequent importing a certificate reply would not work. It turns 
out that its caks.size() is zero detected at establishCertChain() in 
keytool/Main.java after root cert has been imported to that cacerts. At this 
point I’d like to suggest a separate bug be filed to cover the cacerts 
enhancement that you suggested.

Thanks,
Hai-May


> Thanks,
> Max
> 
> 
>> On Jun 2, 2020, at 2:37 AM, Hai-May Chao  wrote:
>> 
>> Hi,
>> 
>> I’d like to request a review for:
>> 
>> JBS: https://bugs.openjdk.java.net/browse/JDK-8244148
>> CSR: https://bugs.openjdk.java.net/browse/JDK-8246269
>> Webrev: http://cr.openjdk.java.net/~hchao/8244148/webrev.00/
>> 
>> The change is to add the support of -trustcacerts and -keystore options to 
>> -printcert and -princrl command for keytool. This enables keytool to use the 
>> trusted certificates when verifying untrusted artifacts that are signed by 
>> CAs. It also incorporates Max’s change that consolidates the code to get the 
>> default location of cacerts keystore.
>> 
>> Thanks,
>> Hai-May
>> 
> 

Re: RFR 8244148: keytool -printcert and -printcrl should support the -trustcacerts and -keystore options

[Weijun Wang]

The source change looks fine to me.

In TrustedCert.java:

- You can use FileOutputStream and Files.copy(Path,OutputStream) in cat().

- There is no need to recreate root.jks and root.pem.

- Why not use -trustcacerts below?

 160 kt("-importcert -file server.pem -noprompt", "server.jks");

- It's probably better to add a " " between cmd and options in patchcmd(). Same 
in TrustedCRL.java.

In TrustedCRL.java:

- No need to recreate ks and ca.crl. Just call "-printcrl" with different 
options.

- Why create using MD5withRSA? Do you meant to warn about the weak algorithm?

Also I would suggest you create a dedicate method (maybe in SecurityTools.java) 
to create your own cacerts. There is no need to copy over the system cacerts, 
just make sure the file is created with the JKS storetype. We are thinking of 
upgrading the storetype of cacerts and it's nice to do this at a single place 
so we can modify it easily later.

Thanks,
Max


> On Jun 2, 2020, at 2:37 AM, Hai-May Chao  wrote:
> 
> Hi,
> 
> I’d like to request a review for:
> 
> JBS: https://bugs.openjdk.java.net/browse/JDK-8244148
> CSR: https://bugs.openjdk.java.net/browse/JDK-8246269
> Webrev: http://cr.openjdk.java.net/~hchao/8244148/webrev.00/
> 
> The change is to add the support of -trustcacerts and -keystore options to 
> -printcert and -princrl command for keytool. This enables keytool to use the 
> trusted certificates when verifying untrusted artifacts that are signed by 
> CAs. It also incorporates Max’s change that consolidates the code to get the 
> default location of cacerts keystore.
> 
> Thanks,
> Hai-May
> 

RFR 8244148: keytool -printcert and -printcrl should support the -trustcacerts and -keystore options

[Hai-May Chao]

Hi,

I’d like to request a review for:

JBS: https://bugs.openjdk.java.net/browse/JDK-8244148
CSR: https://bugs.openjdk.java.net/browse/JDK-8246269
Webrev: http://cr.openjdk.java.net/~hchao/8244148/webrev.00/

The change is to add the support of -trustcacerts and -keystore options to 
-printcert and -princrl command for keytool. This enables keytool to use the 
trusted certificates when verifying untrusted artifacts that are signed by CAs. 
It also incorporates Max’s change that consolidates the code to get the default 
location of cacerts keystore.

Thanks,
Hai-May