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

2020-06-12 Thread 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

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

2020-06-12 Thread 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

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

2020-06-12 Thread 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/ >>

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

2020-06-12 Thread 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

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

2020-06-11 Thread 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? --

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

2020-06-10 Thread 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

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

2020-06-10 Thread 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

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

2020-06-09 Thread 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

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

2020-06-09 Thread 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

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

2020-06-07 Thread 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 - > >

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

2020-06-07 Thread 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

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

2020-06-06 Thread 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.

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

2020-06-05 Thread 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,

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

2020-06-04 Thread 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

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

2020-06-04 Thread 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

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

2020-06-03 Thread 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");

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

2020-06-01 Thread 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