[RFC][icedtea-web] PR742: Fix checking multiple levels of JAR certs for trust

[Danesh Dadachanji]

On 10/08/11 06:01 PM, Deepak Bhole wrote:
> * Danesh Dadachanji<ddadacha at redhat.com>  [2011-08-10 17:20]:
>> Hello,
>>
>> Here's an update for this bug. It took so long because of the (at
>> the time) mysterious PR771. This will now check all the certificates
>> along the certPath for trust in the store. It also displays a new
>> icon[1] and automatically selects the "Always Trust" checkbox when
>> an applet is verified. Along the way I found a miscalculation in the
>> window size of the dialog. It was too small to display the entire
>> icon so I increased the height.
>>
>> I've tested it on all of the certificate holding JNLPs on the test
>> wiki page.
>>
>> The original reporter's applet is signed by an older version of a
>> Thawte CA which I was unable to find online. The newer version is
>> technically considered a different certificate (public keys are
>> different) so this patch still won't verify their applet.
>>
>
> Hi Danesh,
>
> Patch looks good. Couple of minor things:
>

Hi Deepak,

Thanks for the review!

I pulled the icon stuff out and into another patch because it isn't 
related to PR742. I will email a patch in a few moments. The subject 
will be: Update UI for SecurityDialog

> When a cert has been verified and we show a prompt with the new question
> icon, the title still says "Warning - Security" ... we should probably
> update this to something that doesn't look as alarming ... maybe
> "Security approval needed" or something?
>
> Also, one code related fix below:
>
>> -                rootInCacerts = CertificateUtils.inKeyStores(root, caKeyStores);
>> +                // Check entire cert path for a trusted CA
>> +                List<? extends Certificate>  certList = certPath.getCertificates();
>> +                for (int i = 0; i != certList.size(); i++) {
>> +                	if ((rootInCacerts = CertificateUtils.inKeyStores(
>> +                            (X509Certificate) certList.get(i), caKeyStores))) {
>> +                        break;
>> +                    }
>> +                }
>
> It would be cleaner to use the Java foreach syntax with
> certPath.getCertificates() directly... e.g.:
>
> for (Certificate c: certPath.getCertificates()) {
> 	...
> }

I changed it to this =)

ChangeLog
+2011-08-11  Danesh Dadachanji <ddadacha at redhat.com>
+
+ PR742: IcedTea-Web checks certs only upto 1 level deep before declaring
+ them untrusted.
+ * NEWS: Updated.
+ * netx/net/sourceforge/jnlp/tools/JarSigner.java:
+ (checkTrustedCerts): All certs along certPath are now checked for trust.
+

>
> Cheers,
> Deepak

-------------- next part --------------
A non-text attachment was scrubbed...
Name: PR742.patch
Type: text/x-patch
Size: 1585 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20110811/4f5fbaf9/PR742.patch