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 <sean.mul...@oracle.com> 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
