SSL certificate chain validation issues

[Mark Doliner]

I can confirm that our GnuTLS certificate code is indeed vulnerable.
And our libnss cert code, too. I tested both by creating fake certs.

Specifically I made a fake CA cert, then used that to sign a fake
intermediate, then had the fake intermediate sign a fake cert for
gmail.com. I added the fake CA cert to Pidgin's ca-certs directory and
I altered Pidgin's code to use my fake gmail.com cert for all SSL
connections.

I started trying to implement datallah's new x509_verify_cert function
for GnuTLS. It didn't go well.

I wanted to use gnutls_certificate_verify_peers3(). I think this is
the recommended function. I think it does the most amount of work for
us. Unfortunately it wants a gnutls_session_t session parameter. We do
have a session when making the ssl connection, but I believe that it's
not available in purple_certificate_verify(). We could work around
that (hash table of gnutls_session_t in ssl-gnutls.c, but that's a
little messy).

We could also use lower-level cert verification functions
(gnutls_x509_trust_list_verify_named_crt() and
gnutls_x509_trust_list_verify_crt()?) but they're more complicated and
more error-prone.

I'm leaning toward just adding the basic constraints check for gnutls
in 2.x.y, then maybe doing a more complete fix in 3.0.0. Here's what I
have so far:

diff -r be8209a19937 libpurple/plugins/ssl/ssl-gnutls.c
--- a/libpurple/plugins/ssl/ssl-gnutls.c        Sun May 04 14:33:20 2014 -0700
+++ b/libpurple/plugins/ssl/ssl-gnutls.c        Thu May 08 00:41:38 2014 -0700
@@ -905,6 +905,27 @@ x509_certificate_signed_by(PurpleCertifi
                return FALSE;
        }

+       /* Make sure issuer has the CA flag set to true in the basic
+          constraints extension. */
+       ret = gnutls_x509_crt_get_basic_constraints(issuer_dat, NULL,
NULL, NULL);
+       if (ret == GNUTLS_E_REQUESTED_DATA_NOT_AVAILABLE) {
+               /* Issuing certificate is missing the basic
constraints extension.
+                  Allow it to act as a CA. */
+       } else if (ret <= 0) {
+               /* Issuing certificate is either missing the basic constraints
+                  extension, or it exists and the CA flag is set to false. In
+                  either case, reject the certificate. */
+               gchar *issuer_id;
+
+               issuer_id = purple_certificate_get_unique_id(issuer);
+               purple_debug_error("gnutls/x509", "Issuing cert %s
does not have the "
+                               "CA flag set to true in the basic
constraints extension. "
+                               "ret=%d\n", issuer_id ? issuer_id :
"(null)", ret);
+               g_free(issuer_id);
+
+               return FALSE;
+       }
+
        /* Now, check the signature */
        /* The second argument is a ptr to an array of "trusted" issuer certs,
           but we're only using one trusted one */


I need to check if it's really ok to let verification continue if the
cert is missing the basic constraints extension. It seemed to be
required for one of our prpls (maybe AIM or Yahoo!).