[RFC][icedtea-web] Fix JarSigner to check that cert start dates have passed

[Danesh Dadachanji]

On 30/03/12 05:22 PM, Deepak Bhole wrote:
> * Danesh Dadachanji<ddadacha at redhat.com>  [2012-03-30 17:04]:
>> On 30/03/12 04:20 PM, Deepak Bhole wrote:
>>> * Danesh Dadachanji<ddadacha at redhat.com>   [2012-03-30 16:02]:
>>>> Hi,
>>>>
>>>> Currently, JarSigner never sets notYetValidCert to true, the
>>>> notBefore date is never checked when sorting out the certificates.
>>>> If it were true, the certificate would be considered as having
>>>> signing issues and all the unverified prompts would start
>>>> triggering. Attached is a patch to fix this, everything else is
>>>> already taken care of WRT notYetValidCert being checked in other
>>>> places.
>>>>
>>>> ChangeLog
>>>> +2012-03-30  Danesh Dadachanji<ddadacha at redhat.com>
>>>> +
>>>> +	Certificate start dates are not being checked, they are still verified
>>>> +	even if the date has yet not been reached.
>>>> +	* netx/net/sourceforge/jnlp/tools/JarSigner.java (verifyJar): If the start
>>>> +	date is in the future, set notYetValidCert to true.
>>>> +
>>>>
>>>>
>>>> Okay for HEAD? Thoughts on backporting? I don't think this should
>>>> wait to be backported since currently it is verifying certificates
>>>> it should not be letting through, misleading users when dialogs
>>>> prompt.
>>>>
>>>
>>> I think this one is fine for 1.1 and 1.2 in addition to HEAD.
>>
>> Bah noticed a bug in the patch, if the cert expires in 6 months or
>> less, that flag is set and the notYetValidCert isn't. I tested it
>> with a 365 day valid cert the first time around. :S
>>
>
> This looks fine. Btw, do we really need a warning for something about to
> expire? Until it expires, the cert is completely valid. Perhaps we
> should remove it?

I don't see the warning being too troublesome if it's not directly shown 
to the user. It's only shown if you click on More Information. It also 
doesn't affect whether or not the cert is verified so I would guess most 
users don't even notice it if the signer is fine. =) Perhaps we should 
change the icon associated with it to be more friendly (currently the 
warning one with the yellow triangle and "!"). What do you think?

The only thing this flag really does is add "SRunWithoutRestrictions" to 
the warnings list (apart from the "SHasExpiringCert" warning). I'm not 
entirely sure it should be doing this though..

>> Updated patch in attachment, ChangeLog is the same. Thanks to
>> Omair's comment, I did a slightly more extensive search on where
>> notBefore() is called as well as my original search for
>> "notYetValidCert", everything looks logical now.
>>
>> I've now tested this with:
>>   - a cert that has notBefore=$YESTERDAY, notAfter=$TOMORROW: Both
>> "not yet valid" and "expiring in 6 months" warnings show in More
>> Info dialog.
>>   - a cert that has notBefore=$YESTERDAY, notAfter=$NEXT_YEAR: Just
>> "not yet valid" warning shows in More Info dialog.
>>
>> Is there anything else I should test? /me can't think of anything
>> off the bat.
>>
>> Cheers,
>> Danesh
>
>> diff --git a/netx/net/sourceforge/jnlp/tools/JarSigner.java b/netx/net/sourceforge/jnlp/tools/JarSigner.java
>> --- a/netx/net/sourceforge/jnlp/tools/JarSigner.java
>> +++ b/netx/net/sourceforge/jnlp/tools/JarSigner.java
>> @@ -297,9 +297,15 @@ public class JarSigner implements CertVe
>>                               if (cert instanceof X509Certificate) {
>>                                   checkCertUsage((X509Certificate) cert, null);
>>                                   if (!showcerts) {
>> +                                    long notBefore = ((X509Certificate) cert)
>> +                                                     .getNotBefore().getTime();
>>                                       long notAfter = ((X509Certificate) cert)
>>                                                       .getNotAfter().getTime();
>>
>> +                                    if (now<  notBefore) {
>> +                                        notYetValidCert = true;
>> +                                    }
>> +
>>                                       if (notAfter<  now) {
>>                                           hasExpiredCert = true;
>>                                       } else if (notAfter<  now + SIX_MONTHS) {
>