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

classic Classic list List threaded Threaded
11 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Weijun Wang
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
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Weijun Wang
Ping again.

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
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Anthony Scarpino
I think your code looks good.

You should check the CertPathValidator tests, I think one of them might
fail after this change.

Tony


On 05/10/2017 04:36 PM, Weijun Wang wrote:

> Ping again.
>
> 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

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Weijun Wang
I assume we will backport this to jdk9u.

So I'd like to fix the newly found regression JDK-8180289 along with
this bug, since both are about CertPath validation warning messages.

I'll need some time thinking about a proper fix. The current warning
message on cert expiration is only on "signer certificate", and CertPath
validation will report expirations on intermediate CA certs as well as
TSA certs. I don't think it's a good idea to print different warnings
for them (and their -strict exit codes are all 4) and might show the
info in -verbose -certs outputs.

Thanks
Max


On 05/12/2017 11:19 AM, Anthony Scarpino wrote:

> I think your code looks good.
>
> You should check the CertPathValidator tests, I think one of them might
> fail after this change.
>
> Tony
>
>
> On 05/10/2017 04:36 PM, Weijun Wang wrote:
>> Ping again.
>>
>> 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
>
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Sean Mullan
In reply to this post by Weijun Wang
* Resources.java

[278] s/includes/include/

* SignatureFileVerifier

[728] setting ts to null in the catch block seems a bit ugly. Maybe it
would be better to refactor that code into a static method that returns
a valid Timestamp or null if not.

* Main.java

[271-2] Perhaps you should use a different exit code to distinguish it
from an invalid signer chain?

[1064-6] Not sure I understand this block of code. How can a jar have an
invalid timestamp if -tsa was not specified?

[1546-7] TODO item - Why can't we do this? We need to warn the user if
they sign a JAR and the TSA chain is invalid so they know what to expect
and are not surprised when it stops working the day after the signer's
cert expires.

--Sean

On 5/10/17 7:36 PM, Weijun Wang wrote:

> Ping again.
>
> 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
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Weijun Wang


On 05/17/2017 04:01 AM, Sean Mullan wrote:
> * Resources.java
>
> [278] s/includes/include/
>
> * SignatureFileVerifier
>
> [728] setting ts to null in the catch block seems a bit ugly. Maybe it
> would be better to refactor that code into a static method that returns
> a valid Timestamp or null if not.

Or I can initialize ts to be null.

>
> * Main.java
>
> [271-2] Perhaps you should use a different exit code to distinguish it
> from an invalid signer chain?

I can use 64. Although I am not quite sure of the usefulness of
difference exit codes now.

>
> [1064-6] Not sure I understand this block of code. How can a jar have an
> invalid timestamp if -tsa was not specified?

This is in verification. Because of weak algorithms (and maybe more),
it's possible that the API does not show there is a timestamp
(noTimestamp) but the PKCS7 block file has a tsToken
(hasTimestampBlock). I'd like to show different warnings here.

>
> [1546-7] TODO item - Why can't we do this? We need to warn the user if
> they sign a JAR and the TSA chain is invalid so they know what to expect
> and are not surprised when it stops working the day after the signer's
> cert expires.

Yes, we can.

BTW, I am thinking of more changes. jarsigner had so many different
warnings (cert expired, cert usage...) because it was not using
sun.security.validator.Validator before and checked these all by its
own. It should rely on Validator but keep showing the warnings for
compatibility. The lines on 1992-2012 was added to deliberately avoid
showing both these "legacy" warnings and validation warnings at the same
time. I'd prefer to remove these lines. As long as the exit code does
not change (they are all the same for signer certs, 4), showing an extra
warning line is not harmful.

Thanks
Max

>
> --Sean
>
> On 5/10/17 7:36 PM, Weijun Wang wrote:
>> Ping again.
>>
>> 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
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Weijun Wang
Thinking about this again.

Currently we have these warnings and their exit codes (when -strict):

- signer cert validity problem - 4
- signer cert keyUsage problem - 8
- other signer cert validation problems - 4
- disabled alg specified while signing - 4

We are now adding

- TSA cert validation problems

We also intend to add one later

- weak (but not yet disabled) alg specified while signing or detected in
verification

If we want to be compatible with before, I'd like to reuse 8 for
extendedKeyUsage issue when a TSA cert does not allow timestamping, and
4 for other TSA validation errors and weak/disabled algs.

If we can be imcompatible, I am thinking of

2 - disabled alg used in signing or weak alg used in verification.
4 - any problem with signer certs
8 - any problem with TSA certs

Note: 2 was used by "expired soon" some time ago but we stopped using it
because this is not a real error. It's now an always-warning (like no
timestamp) and no exit code.

Thanks
Max

On 05/18/2017 09:50 AM, Weijun Wang wrote:
>> [271-2] Perhaps you should use a different exit code to distinguish it
>> from an invalid signer chain?
>
> I can use 64. Although I am not quite sure of the usefulness of
> difference exit codes now.
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Weijun Wang
In reply to this post by Weijun Wang
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
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Sean Mullan
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
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Weijun Wang
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 <[hidden email]> 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

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [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 <[hidden email]> 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 <[hidden email]> 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
>

Loading...