Webrev updated: - http://cr.openjdk.java.net/~weijun/8166222/root/webrev.01
Move the change for JarUtils.java here. - http://cr.openjdk.java.net/~weijun/8166222/webrev.02/ Update multiple "is not validated" strings to "is invalid" in Resources.java. Several tests that grep these words are also updated. No functional change. Thanks Max > On Jul 14, 2017, at 11:22 PM, Weijun Wang <[email protected]> wrote: > > All comments accepted. I’ll update my webrev next month. :-) > > No comment on real code change? > > --Max > >> On Jul 14, 2017, at 11:20 PM, Sean Mullan <[email protected]> wrote: >> >> Finally getting back to reviewing this update. A few comments: >> >> SignatureFileVerifier.java: >> >> 729 debug.println("getTimeStamp caught: "+e); >> >> Can you add a more descriptive message here, like: "Exception processing >> timestamp, code will be treated as signed, but not timestamped: ". >> >> Resources.java: >> >> 209 {".CertPath.not.validated.", "[CertPath not validated: "}, >> 210 {".TSA.CertPath.not.validated.", "[TSA CertPath not validated: >> "}, >> >> I think it would be more clear to say "Invalid CertPath". "not validated" >> sounds like keytool didn't even try to validate the path. >> >> Also, I think you should say "certificate chain" instead of "CertPath" >> because that is the term that is used in all the other certificate chain >> messages emitted by keytool. >> >> 265 {"The.tsa.certificate.chain.is.not.validated.reason.1", >> 266 "The TSA certificate chain is not validated. Reason: >> %s"}, >> >> s/not validated/invalid/ >> >> 275 >> {"This.jar.contains.entries.whose.tsa.certificate.chain.is.not.validated.reason.1", >> 276 "This jar contains entries whose TSA certificate chain >> is not validated. Reason: %s"}, >> >> s/not/validated/invalid/ >> >> 281 {"bad.timestamp.verifying", >> 282 "This jar contains signatures that include an invalid >> timestamp. Without a valid timestamp, users may not be able to validate this >> jar after any of the signer certificates expire (as early as >> %1$tY-%1$tm-%1$td)."}, >> >> Can you also include information how to debug this, i.e. "Rerun jarsigner >> with -J-Djava.security.debug=jar for more information." >> >> --Sean >> >> On 5/22/17 11:43 AM, Weijun Wang wrote: >>> An updated webrev at >>> http://cr.openjdk.java.net/~weijun/8166222/webrev.01/ >>> Some more changes: >>> - TSA cert chain validation is performed in both verification and signing. >>> The exit code for validation failure when -strict is specified is 64. >>> - printCert() and validateCertChain() know about they are dealing with TSA >>> cert chain and do not touch any flag except for tsaChainNotValidated. Code >>> being a little ugly. >>> - TSA certs lines added to -verbose -certs. >>> BTW, the code change also fixed >>> https://bugs.openjdk.java.net/browse/JDK-8180289. >>> Here is an example: >>> $ jarsigner -verify -strict ts2.jar -verbose -certs >>> s k 75 Mon May 22 22:49:06 CST 2017 META-INF/MANIFEST.MF >>> [entry was signed on 5/22/17, 10:49 PM] >>>>>> Signer >>> X.509, CN=signer (signer) >>> [certificate is valid from 5/22/17, 10:49 PM to 12/8/17, 10:49 PM] >>> X.509, CN=CA (ca) >>> [certificate is valid from 4/12/17, 10:49 PM to 10/29/17, 10:49 PM] >>>>>> TSA >>> X.509, CN=ts >>> [certificate is valid from 5/22/17, 10:49 PM to 12/8/17, 10:49 PM] >>> [ExtendedKeyUsage extension does not support timestamping] >>> [TSA CertPath not validated: Extended key usage does not permit use >>> for TSA server] >>> 305 Mon May 22 22:49:08 CST 2017 META-INF/SIGNER.SF >>> 3096 Mon May 22 22:51:06 CST 2017 META-INF/SIGNER.RSA >>> smk 1 Mon May 22 22:48:56 CST 2017 A >>> [entry was signed on 5/22/17, 10:49 PM] >>>>>> Signer >>> X.509, CN=signer (signer) >>> [certificate is valid from 5/22/17, 10:49 PM to 12/8/17, 10:49 PM] >>> X.509, CN=CA (ca) >>> [certificate is valid from 4/12/17, 10:49 PM to 10/29/17, 10:49 PM] >>>>>> TSA >>> X.509, CN=ts >>> [certificate is valid from 5/22/17, 10:49 PM to 12/8/17, 10:49 PM] >>> [TSA CertPath not validated: Extended key usage does not permit use >>> for TSA server] >>> s = signature was verified >>> m = entry is listed in manifest >>> k = at least one certificate was found in keystore >>> - Signed by "CN=signer" >>> Digest algorithm: SHA-256 >>> Signature algorithm: SHA256withRSA, 2048-bit key >>> Timestamped by "CN=ts" on Mon May 22 14:49:08 UTC 2017 >>> Timestamp digest algorithm: SHA-256 >>> Timestamp signature algorithm: SHA1withRSA, 2048-bit key >>> jar verified, with signer errors. >>> Error: >>> This jar contains entries whose TSA certificate chain is not validated. >>> Reason: Extended key usage does not permit use for TSA server >>> Thanks >>> Max >>> On 04/12/2017 11:52 PM, Weijun Wang wrote: >>>> Please take a review at >>>> >>>> http://cr.openjdk.java.net/~weijun/8166222/webrev.00/ >>>> >>>> The major code change is inside SignatureFileVerifier.java. Now if the >>>> timestamp on a signed jar is invalid (For example, using a weak algorithm >>>> now disabled), the jar file will be treated as a signed jar without a >>>> timestamp. Before this change, it was treated unsigned. >>>> >>>> In jarsigner/Main.java, I also add a line to validate the TSA cert chain. >>>> If not validated, a warning will be shown which is similar to the one when >>>> signer cert chain is not validated. If -strict is on, exit code will >>>> change too. >>>> >>>> I also make a small change at >>>> >>>> http://cr.openjdk.java.net/~weijun/8166222/root/webrev.00/ >>>> >>>> The executeCommand() method shows more info (mainly stdout and stderr >>>> outputs) than executeProcess(). >>>> >>>> Because of the behavior change and new warnings, this change will need a >>>> Compatibility and Specification Review (CSR). At the moment, please review >>>> the code change first. >>>> >>>> Thanks >>>> Max >
