[10] RFR 8166222: Don't treat signed jars with invalid timestamps as unsigned

[Weijun Wang]

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 <weijun.wang at oracle.com> 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 <sean.mullan at 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
>