Merge lp:~heimes/pyopenssl/pyopenssl into lp:~exarkun/pyopenssl/trunk

[Christian Heimes]

See http://bugs.python.org/issue18709
http://www.ruby-lang.org/en/news/2013/06/27/hostname-check-bypassing-vulnerability-in-openssl-client-cve-2013-4073/

Additionally to my original patch from last week I have also fixed a memory leak.

Unmerged revisions

170. By Christian Heimes on 2013-08-11

Fix handling of NULL bytes inside subjectAltName general names, CVE-2013-4073.

169. By Christian Heimes on 2013-08-11

Fix memory leak in get_extension().

=== modified file 'ChangeLog'
--- ChangeLog	2012-04-03 19:25:07 +0000
+++ ChangeLog	2013-08-12 11:40:45 +0000
@@ -1,3 +1,9 @@
[phone]  Christian Heimes  <[e-mail]>
+
+	* OpenSSL/crypto/x509ext.c: Fix handling of NULL bytes inside
+	  subjectAltName general names, CVE-2013-4073.
+	* OpenSSL/crypto/x509.c: Fix memory leak in get_extension().
+
 2012-04-03  Jean-Paul Calderone  <[e-mail]>
 
 	* OpenSSL/crypto/pkey.c: Release the GIL around RSA and DSA key

=== modified file 'OpenSSL/crypto/x509.c'
--- OpenSSL/crypto/x509.c	2011-07-16 05:22:14 +0000
+++ OpenSSL/crypto/x509.c	2013-08-12 11:40:45 +0000
@@ -758,6 +758,7 @@
 
     extobj = PyObject_New(crypto_X509ExtensionObj, &crypto_X509Extension_Type);
     extobj->x509_extension = X509_EXTENSION_dup(ext);
+    extobj->dealloc = 1;
 
     return (PyObject*)extobj;
 }

=== modified file 'OpenSSL/crypto/x509ext.c'
--- OpenSSL/crypto/x509ext.c	2011-07-16 05:14:58 +0000
+++ OpenSSL/crypto/x509ext.c	2013-08-12 11:40:45 +0000
@@ -16,7 +16,7 @@
 static char crypto_X509Extension_get_critical_doc[] = "\n\
 Returns the critical field of the X509Extension\n\
 \n\
-:return: The critical field.\n\
+// :return: The critical field.\n\
 ";
 
 static PyObject *
@@ -236,6 +236,75 @@
     PyObject_Del(self);
 }
 
+
+/* Special handling of subjectAltName, see CVE-2013-4073 */
+
+int
+crypto_X509Extension_str_san(crypto_X509ExtensionObj *self, BIO *bio)
+{
+    GENERAL_NAMES *names;
+    const X509V3_EXT_METHOD *method = NULL;
+    long i, length, num;
+    const unsigned char *p;
+
+    method = X509V3_EXT_get(self->x509_extension);
+    if (method == NULL) {
+        return -1;
+    }
+
+    p = self->x509_extension->value->data;
+    length = self->x509_extension->value->length;
+    if (method->it) {
+        names = (GENERAL_NAMES*)(ASN1_item_d2i(NULL, &p, length,
+                                               ASN1_ITEM_ptr(method->it)));
+    }
+    else {
+        names = (GENERAL_NAMES*)(method->d2i(NULL, &p, length));
+    }
+    if (names == NULL) {
+        return -1;
+    }
+
+    num = sk_GENERAL_NAME_num(names);
+    for (i = 0; i < num; i++) {
+            GENERAL_NAME *name;
+            ASN1_STRING *as;
+            name = sk_GENERAL_NAME_value(names, i);
+            switch (name->type) {
+                case GEN_EMAIL:
+                    BIO_puts(bio, "email:");
+                    as = name->d.rfc822Name;
+                    BIO_write(bio, ASN1_STRING_data(as),
+                              ASN1_STRING_length(as));
+                    break;
+                case GEN_DNS:
+                    BIO_puts(bio, "DNS:");
+                    as = name->d.dNSName;
+                    BIO_write(bio, ASN1_STRING_data(as),
+                              ASN1_STRING_length(as));
+                    break;
+                case GEN_URI:
+                    BIO_puts(bio, "URI:");
+                    as = name->d.uniformResourceIdentifier;
+                    BIO_write(bio, ASN1_STRING_data(as),
+                              ASN1_STRING_length(as));
+                    break;
+                default:
+                    /* use builtin print for GEN_OTHERNAME, GEN_X400,
+                     * GEN_EDIPARTY, GEN_DIRNAME, GEN_IPADD and GEN_RID
+                     */
+                    GENERAL_NAME_print(bio, name);
+            }
+            /* trailing ', ' except for last element */
+            if (i < (num - 1)) {
+                BIO_puts(bio, ", ");
+            }
+    }
+    sk_GENERAL_NAME_pop_free(names, GENERAL_NAME_free);
+
+    return 0;
+}
+
 /*
  * Print a nice text representation of the certificate request.
  */
@@ -247,7 +316,14 @@
     PyObject *str;
     BIO *bio = BIO_new(BIO_s_mem());
 
-    if (!X509V3_EXT_print(bio, self->x509_extension, 0, 0))
+    if (OBJ_obj2nid(self->x509_extension->object) == NID_subject_alt_name) {
+        if (crypto_X509Extension_str_san(self, bio) == -1) {
+            BIO_free(bio);
+            exception_from_error_queue(crypto_Error);
+            return NULL;
+        }
+    }
+    else if (!X509V3_EXT_print(bio, self->x509_extension, 0, 0))
     {
         BIO_free(bio);
         exception_from_error_queue(crypto_Error);

=== modified file 'OpenSSL/test/test_crypto.py'
--- OpenSSL/test/test_crypto.py	2012-03-09 22:58:00 +0000
+++ OpenSSL/test/test_crypto.py	2013-08-12 11:40:45 +0000
@@ -265,6 +265,37 @@
 -----END RSA PRIVATE KEY-----
 """)
 
+# certificate with NULL bytes in subjectAltName and common name
+
+nullbyte_san_PEM = b("""-----BEGIN CERTIFICATE-----
+MIIE2DCCA8CgAwIBAgIBADANBgkqhkiG9w0BAQUFADCBxTELMAkGA1UEBhMCVVMx
+DzANBgNVBAgMBk9yZWdvbjESMBAGA1UEBwwJQmVhdmVydG9uMSMwIQYDVQQKDBpQ
+eXRob24gU29mdHdhcmUgRm91bmRhdGlvbjEgMB4GA1UECwwXUHl0aG9uIENvcmUg
+RGV2ZWxvcG1lbnQxJDAiBgNVBAMMG251bGwucHl0aG9uLm9yZwBleGFtcGxlLm9y
+ZzEkMCIGCSqGSIb3DQEJARYVcHl0aG9uLWRldkBweXRob24ub3JnMB4XDTEzMDgw
+NzEzMTE1MloXDTEzMDgwNzEzMTI1MlowgcUxCzAJBgNVBAYTAlVTMQ8wDQYDVQQI
+DAZPcmVnb24xEjAQBgNVBAcMCUJlYXZlcnRvbjEjMCEGA1UECgwaUHl0aG9uIFNv
+ZnR3YXJlIEZvdW5kYXRpb24xIDAeBgNVBAsMF1B5dGhvbiBDb3JlIERldmVsb3Bt
+ZW50MSQwIgYDVQQDDBtudWxsLnB5dGhvbi5vcmcAZXhhbXBsZS5vcmcxJDAiBgkq
+hkiG9w0BCQEWFXB5dGhvbi1kZXZAcHl0aG9uLm9yZzCCASIwDQYJKoZIhvcNAQEB
+BQADggEPADCCAQoCggEBALXq7cn7Rn1vO3aA3TrzA5QLp6bb7B3f/yN0CJ2XFj+j
+pHs+Gw6WWSUDpybiiKnPec33BFawq3kyblnBMjBU61ioy5HwQqVkJ8vUVjGIUq3P
+vX/wBmQfzCe4o4uM89gpHyUL9UYGG8oCRa17dgqcv7u5rg0Wq2B1rgY+nHwx3JIv
+KRrgSwyRkGzpN8WQ1yrXlxWjgI9de0mPVDDUlywcWze1q2kwaEPTM3hLAmD1PESA
+oY/n8A/RXoeeRs9i/Pm/DGUS8ZPINXk/yOzsR/XvvkTVroIeLZqfmFpnZeF0cHzL
+08LODkVJJ9zjLdT7SA4vnne4FEbAxDbKAq5qkYzaL4UCAwEAAaOB0DCBzTAMBgNV
+HRMBAf8EAjAAMB0GA1UdDgQWBBSIWlXAUv9hzVKjNQ/qWpwkOCL3XDALBgNVHQ8E
+BAMCBeAwgZAGA1UdEQSBiDCBhYIeYWx0bnVsbC5weXRob24ub3JnAGV4YW1wbGUu
+Y29tgSBudWxsQHB5dGhvbi5vcmcAdXNlckBleGFtcGxlLm9yZ4YpaHR0cDovL251
+bGwucHl0aG9uLm9yZwBodHRwOi8vZXhhbXBsZS5vcmeHBMAAAgGHECABDbgAAAAA
+AAAAAAAAAAEwDQYJKoZIhvcNAQEFBQADggEBAKxPRe99SaghcI6IWT7UNkJw9aO9
+i9eo0Fj2MUqxpKbdb9noRDy2CnHWf7EIYZ1gznXPdwzSN4YCjV5d+Q9xtBaowT0j
+HPERs1ZuytCNNJTmhyqZ8q6uzMLoht4IqH/FBfpvgaeC5tBTnTT0rD5A/olXeimk
+kX4LxlEx5RAvpGB2zZVRGr6LobD9rVK91xuHYNIxxxfEGE8tCCWjp0+3ksri9SXx
+VHWBnbM9YaL32u3hxm8sYB/Yb8WSBavJCWJJqRStVRHM1koZlJmXNx2BX4vPo6iW
+RFEIPQsFZRLrtnCAiEhyT8bC2s/Njlu6ly9gtJZWSV46Q3ZjBL4q9sHKqZQ=
+-----END CERTIFICATE-----""")
+
 
 class X509ExtTests(TestCase):
     """
@@ -1398,6 +1429,34 @@
         self.assertRaises(IndexError, cert.get_extension, 4)
         self.assertRaises(TypeError, cert.get_extension, "hello")
 
+    def test_nullbyte_san(self):
+        """
+        Test correct handling of CN and SAN with NULL bytes
+
+        see CVE-2013-4073
+        """
+        cert = load_certificate(FILETYPE_PEM, nullbyte_san_PEM)
+        subject = cert.get_subject()
+        self.assertEqual(subject.CN, 'null.python.org\x00example.org')
+        issuer = cert.get_issuer()
+        self.assertEqual(issuer.CN, 'null.python.org\x00example.org')
+
+        ext = cert.get_extension(0)
+        self.assertEqual(ext.get_short_name(), b('basicConstraints'))
+
+        ext = cert.get_extension(1)
+        self.assertEqual(ext.get_short_name(), b('subjectKeyIdentifier'))
+
+        ext = cert.get_extension(2)
+        self.assertEqual(ext.get_short_name(), b('keyUsage'))
+
+        ext = cert.get_extension(3)
+        self.assertEqual(ext.get_short_name(), b('subjectAltName'))
+        self.assertEqual(str(ext), 
+            'DNS:altnull.python.org\x00example.com, '
+            'email:[e-mail]\[e-mail], '
+            'URI:http://null.python.org\x00http://example.org, '
+            'IP Address:192.0.2.1, IP Address:2001:DB8:0:0:0:0:0:1\n')
 
     def test_invalid_digest_algorithm(self):
         """