[icedtea-web] RFC: Fix to check cert equality in addition to inheritance

[Deepak Bhole]

* Deepak Bhole <dbhole at redhat.com> [2012-02-28 15:40]:
> Hi,
> 
> http://icedtea.classpath.org/hg/icedtea-web/rev/9b7eca03a9ea fixed the
> API usage (and bugs related to verification failure in OpenJDK7), but
> introduced another bug whereby certs were not being checked for
> equality, only for proper inheritance.
> 
> Attached patch fixes the issue.
> 
> ChangeLog:
> 2012-02-28  Deepak Bhole <dbhole at redhat.com>
>     * netx/net/sourceforge/jnlp/security/CertificateUtils.java
>     (inKeyStores): Added check for certificate equality.
> 
> OK for 1.2 and HEAD?
> 

Omair pointed out a problem in the new patch -- the new code continues
to allow certs in keystore to act as root certs when it should not be
allowed. Attached patch removed that and only checks for full equality.

OK for push?

Cheers,
Deepak

> Thanks,
> Deepak

> diff -r b1984bb670f0 netx/net/sourceforge/jnlp/security/CertificateUtils.java
> --- a/netx/net/sourceforge/jnlp/security/CertificateUtils.java	Tue Feb 28 11:35:41 2012 -0500
> +++ b/netx/net/sourceforge/jnlp/security/CertificateUtils.java	Tue Feb 28 15:21:34 2012 -0500
> @@ -167,33 +167,48 @@
>                  // Check against all certs
>                  Enumeration<String> aliases = keyStores[i].aliases();
>                  while (aliases.hasMoreElements()) {
> +
> +                    // Verify against this entry
>                      String alias = aliases.nextElement();
> -                    try {
> -                        // Verify against this entry
> -                        c.verify(keyStores[i].getCertificate(alias).getPublicKey());
> +                    Certificate verifyAgainst = keyStores[i].getCertificate(alias);
> +                    boolean matched = false;
> +
> +                    if (c.equals(verifyAgainst)) {
> +                        matched = true; // If they are equal, they match
> +                    } else {
> +
> +                        try {
> +                            // Not equal. Check if cert in keystore signed c
> +                            c.verify(verifyAgainst.getPublicKey());
> +
> +                            // If we got here, it means verification succeeded
> +                            matched = true;
> +                        } catch (NoSuchAlgorithmException nsae) {
> +                            // Unsupported signature algorithm
> +                            // Consider non-match and keep going
> +                        } catch (InvalidKeyException ike) {
> +                            // Incorrect/corrupt key
> +                            // Consider non-match and keep going
> +                        } catch (NoSuchProviderException nspe) {
> +                            // No default provider
> +                            // Consider non-match and keep going
> +                        } catch (SignatureException se) {
> +                            // Signature error
> +                            // Consider non-match and keep going
> +                        } catch (CertificateException ce) {
> +                            // Encoding error
> +                            // Consider non-match and keep going
> +                        }
> +                    }
> +
> +                    if (matched) {
>  
>                          if (JNLPRuntime.isDebug()) {
>                              System.out.println(c.getSubjectX500Principal().getName() + " found in cacerts");
>                          }
> -                        
> -                        // If we got here, it means verification succeeded. Return true.
> +
>                          return true;
> -                    } catch (NoSuchAlgorithmException nsae) {
> -                        // Unsupported signature algorithm 
> -                        // Consider non-match and keep going
> -                    } catch (InvalidKeyException ike) {
> -                        // Incorrect/corrupt key
> -                        // Consider non-match and keep going                     
> -                    } catch (NoSuchProviderException nspe) {
> -                        // No default provider 
> -                        // Consider non-match and keep going
> -                    } catch (SignatureException se) {
> -                        // Signature error
> -                        // Consider non-match and keep going
> -                    } catch (CertificateException ce) {
> -                        // Encoding error
> -                        // Consider non-match and keep going
> -                    }
> +                    } // else continue
>                  }
>              } catch (KeyStoreException e) {
>                  e.printStackTrace();

-------------- next part --------------
diff -r b1984bb670f0 netx/net/sourceforge/jnlp/security/CertificateUtils.java
--- a/netx/net/sourceforge/jnlp/security/CertificateUtils.java	Tue Feb 28 11:35:41 2012 -0500
+++ b/netx/net/sourceforge/jnlp/security/CertificateUtils.java	Wed Feb 29 10:35:21 2012 -0500
@@ -167,34 +167,19 @@
                 // Check against all certs
                 Enumeration<String> aliases = keyStores[i].aliases();
                 while (aliases.hasMoreElements()) {
+
+                    // Verify against this entry
                     String alias = aliases.nextElement();
-                    try {
-                        // Verify against this entry
-                        c.verify(keyStores[i].getCertificate(alias).getPublicKey());
 
+                    if (c.equals(keyStores[i].getCertificate(alias))) {
                         if (JNLPRuntime.isDebug()) {
                             System.out.println(c.getSubjectX500Principal().getName() + " found in cacerts");
                         }
-                        
-                        // If we got here, it means verification succeeded. Return true.
+
                         return true;
-                    } catch (NoSuchAlgorithmException nsae) {
-                        // Unsupported signature algorithm 
-                        // Consider non-match and keep going
-                    } catch (InvalidKeyException ike) {
-                        // Incorrect/corrupt key
-                        // Consider non-match and keep going                     
-                    } catch (NoSuchProviderException nspe) {
-                        // No default provider 
-                        // Consider non-match and keep going
-                    } catch (SignatureException se) {
-                        // Signature error
-                        // Consider non-match and keep going
-                    } catch (CertificateException ce) {
-                        // Encoding error
-                        // Consider non-match and keep going
-                    }
+                    } // else continue
                 }
+
             } catch (KeyStoreException e) {
                 e.printStackTrace();
                 // continue