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

Proposed by Christian Heimes on 2013-08-12
Status: Needs review
Proposed branch: lp:~heimes/pyopenssl/pyopenssl
Merge into: lp:~exarkun/pyopenssl/trunk
Diff against target: 207 lines (+144/-2)
4 files modified
ChangeLog (+6/-0)
OpenSSL/crypto/x509.c (+1/-0)
OpenSSL/crypto/x509ext.c (+78/-2)
OpenSSL/test/test_crypto.py (+59/-0)
To merge this branch: bzr merge lp:~heimes/pyopenssl/pyopenssl
Reviewer Review Type Date Requested Status
Jean-Paul Calderone 2013-08-12 Pending
Review via email: mp+179673@code.launchpad.net

Description of the change

To post a comment you must log in.

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().

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'ChangeLog'
2--- ChangeLog 2012-04-03 19:25:07 +0000
3+++ ChangeLog 2013-08-12 11:40:45 +0000
4@@ -1,3 +1,9 @@
5+2013-08-11 Christian Heimes <christian@python.org>
6+
7+ * OpenSSL/crypto/x509ext.c: Fix handling of NULL bytes inside
8+ subjectAltName general names, CVE-2013-4073.
9+ * OpenSSL/crypto/x509.c: Fix memory leak in get_extension().
10+
11 2012-04-03 Jean-Paul Calderone <exarkun@twistedmatrix.com>
12
13 * OpenSSL/crypto/pkey.c: Release the GIL around RSA and DSA key
14
15=== modified file 'OpenSSL/crypto/x509.c'
16--- OpenSSL/crypto/x509.c 2011-07-16 05:22:14 +0000
17+++ OpenSSL/crypto/x509.c 2013-08-12 11:40:45 +0000
18@@ -758,6 +758,7 @@
19
20 extobj = PyObject_New(crypto_X509ExtensionObj, &crypto_X509Extension_Type);
21 extobj->x509_extension = X509_EXTENSION_dup(ext);
22+ extobj->dealloc = 1;
23
24 return (PyObject*)extobj;
25 }
26
27=== modified file 'OpenSSL/crypto/x509ext.c'
28--- OpenSSL/crypto/x509ext.c 2011-07-16 05:14:58 +0000
29+++ OpenSSL/crypto/x509ext.c 2013-08-12 11:40:45 +0000
30@@ -16,7 +16,7 @@
31 static char crypto_X509Extension_get_critical_doc[] = "\n\
32 Returns the critical field of the X509Extension\n\
33 \n\
34-:return: The critical field.\n\
35+// :return: The critical field.\n\
36 ";
37
38 static PyObject *
39@@ -236,6 +236,75 @@
40 PyObject_Del(self);
41 }
42
43+
44+/* Special handling of subjectAltName, see CVE-2013-4073 */
45+
46+int
47+crypto_X509Extension_str_san(crypto_X509ExtensionObj *self, BIO *bio)
48+{
49+ GENERAL_NAMES *names;
50+ const X509V3_EXT_METHOD *method = NULL;
51+ long i, length, num;
52+ const unsigned char *p;
53+
54+ method = X509V3_EXT_get(self->x509_extension);
55+ if (method == NULL) {
56+ return -1;
57+ }
58+
59+ p = self->x509_extension->value->data;
60+ length = self->x509_extension->value->length;
61+ if (method->it) {
62+ names = (GENERAL_NAMES*)(ASN1_item_d2i(NULL, &p, length,
63+ ASN1_ITEM_ptr(method->it)));
64+ }
65+ else {
66+ names = (GENERAL_NAMES*)(method->d2i(NULL, &p, length));
67+ }
68+ if (names == NULL) {
69+ return -1;
70+ }
71+
72+ num = sk_GENERAL_NAME_num(names);
73+ for (i = 0; i < num; i++) {
74+ GENERAL_NAME *name;
75+ ASN1_STRING *as;
76+ name = sk_GENERAL_NAME_value(names, i);
77+ switch (name->type) {
78+ case GEN_EMAIL:
79+ BIO_puts(bio, "email:");
80+ as = name->d.rfc822Name;
81+ BIO_write(bio, ASN1_STRING_data(as),
82+ ASN1_STRING_length(as));
83+ break;
84+ case GEN_DNS:
85+ BIO_puts(bio, "DNS:");
86+ as = name->d.dNSName;
87+ BIO_write(bio, ASN1_STRING_data(as),
88+ ASN1_STRING_length(as));
89+ break;
90+ case GEN_URI:
91+ BIO_puts(bio, "URI:");
92+ as = name->d.uniformResourceIdentifier;
93+ BIO_write(bio, ASN1_STRING_data(as),
94+ ASN1_STRING_length(as));
95+ break;
96+ default:
97+ /* use builtin print for GEN_OTHERNAME, GEN_X400,
98+ * GEN_EDIPARTY, GEN_DIRNAME, GEN_IPADD and GEN_RID
99+ */
100+ GENERAL_NAME_print(bio, name);
101+ }
102+ /* trailing ', ' except for last element */
103+ if (i < (num - 1)) {
104+ BIO_puts(bio, ", ");
105+ }
106+ }
107+ sk_GENERAL_NAME_pop_free(names, GENERAL_NAME_free);
108+
109+ return 0;
110+}
111+
112 /*
113 * Print a nice text representation of the certificate request.
114 */
115@@ -247,7 +316,14 @@
116 PyObject *str;
117 BIO *bio = BIO_new(BIO_s_mem());
118
119- if (!X509V3_EXT_print(bio, self->x509_extension, 0, 0))
120+ if (OBJ_obj2nid(self->x509_extension->object) == NID_subject_alt_name) {
121+ if (crypto_X509Extension_str_san(self, bio) == -1) {
122+ BIO_free(bio);
123+ exception_from_error_queue(crypto_Error);
124+ return NULL;
125+ }
126+ }
127+ else if (!X509V3_EXT_print(bio, self->x509_extension, 0, 0))
128 {
129 BIO_free(bio);
130 exception_from_error_queue(crypto_Error);
131
132=== modified file 'OpenSSL/test/test_crypto.py'
133--- OpenSSL/test/test_crypto.py 2012-03-09 22:58:00 +0000
134+++ OpenSSL/test/test_crypto.py 2013-08-12 11:40:45 +0000
135@@ -265,6 +265,37 @@
136 -----END RSA PRIVATE KEY-----
137 """)
138
139+# certificate with NULL bytes in subjectAltName and common name
140+
141+nullbyte_san_PEM = b("""-----BEGIN CERTIFICATE-----
142+MIIE2DCCA8CgAwIBAgIBADANBgkqhkiG9w0BAQUFADCBxTELMAkGA1UEBhMCVVMx
143+DzANBgNVBAgMBk9yZWdvbjESMBAGA1UEBwwJQmVhdmVydG9uMSMwIQYDVQQKDBpQ
144+eXRob24gU29mdHdhcmUgRm91bmRhdGlvbjEgMB4GA1UECwwXUHl0aG9uIENvcmUg
145+RGV2ZWxvcG1lbnQxJDAiBgNVBAMMG251bGwucHl0aG9uLm9yZwBleGFtcGxlLm9y
146+ZzEkMCIGCSqGSIb3DQEJARYVcHl0aG9uLWRldkBweXRob24ub3JnMB4XDTEzMDgw
147+NzEzMTE1MloXDTEzMDgwNzEzMTI1MlowgcUxCzAJBgNVBAYTAlVTMQ8wDQYDVQQI
148+DAZPcmVnb24xEjAQBgNVBAcMCUJlYXZlcnRvbjEjMCEGA1UECgwaUHl0aG9uIFNv
149+ZnR3YXJlIEZvdW5kYXRpb24xIDAeBgNVBAsMF1B5dGhvbiBDb3JlIERldmVsb3Bt
150+ZW50MSQwIgYDVQQDDBtudWxsLnB5dGhvbi5vcmcAZXhhbXBsZS5vcmcxJDAiBgkq
151+hkiG9w0BCQEWFXB5dGhvbi1kZXZAcHl0aG9uLm9yZzCCASIwDQYJKoZIhvcNAQEB
152+BQADggEPADCCAQoCggEBALXq7cn7Rn1vO3aA3TrzA5QLp6bb7B3f/yN0CJ2XFj+j
153+pHs+Gw6WWSUDpybiiKnPec33BFawq3kyblnBMjBU61ioy5HwQqVkJ8vUVjGIUq3P
154+vX/wBmQfzCe4o4uM89gpHyUL9UYGG8oCRa17dgqcv7u5rg0Wq2B1rgY+nHwx3JIv
155+KRrgSwyRkGzpN8WQ1yrXlxWjgI9de0mPVDDUlywcWze1q2kwaEPTM3hLAmD1PESA
156+oY/n8A/RXoeeRs9i/Pm/DGUS8ZPINXk/yOzsR/XvvkTVroIeLZqfmFpnZeF0cHzL
157+08LODkVJJ9zjLdT7SA4vnne4FEbAxDbKAq5qkYzaL4UCAwEAAaOB0DCBzTAMBgNV
158+HRMBAf8EAjAAMB0GA1UdDgQWBBSIWlXAUv9hzVKjNQ/qWpwkOCL3XDALBgNVHQ8E
159+BAMCBeAwgZAGA1UdEQSBiDCBhYIeYWx0bnVsbC5weXRob24ub3JnAGV4YW1wbGUu
160+Y29tgSBudWxsQHB5dGhvbi5vcmcAdXNlckBleGFtcGxlLm9yZ4YpaHR0cDovL251
161+bGwucHl0aG9uLm9yZwBodHRwOi8vZXhhbXBsZS5vcmeHBMAAAgGHECABDbgAAAAA
162+AAAAAAAAAAEwDQYJKoZIhvcNAQEFBQADggEBAKxPRe99SaghcI6IWT7UNkJw9aO9
163+i9eo0Fj2MUqxpKbdb9noRDy2CnHWf7EIYZ1gznXPdwzSN4YCjV5d+Q9xtBaowT0j
164+HPERs1ZuytCNNJTmhyqZ8q6uzMLoht4IqH/FBfpvgaeC5tBTnTT0rD5A/olXeimk
165+kX4LxlEx5RAvpGB2zZVRGr6LobD9rVK91xuHYNIxxxfEGE8tCCWjp0+3ksri9SXx
166+VHWBnbM9YaL32u3hxm8sYB/Yb8WSBavJCWJJqRStVRHM1koZlJmXNx2BX4vPo6iW
167+RFEIPQsFZRLrtnCAiEhyT8bC2s/Njlu6ly9gtJZWSV46Q3ZjBL4q9sHKqZQ=
168+-----END CERTIFICATE-----""")
169+
170
171 class X509ExtTests(TestCase):
172 """
173@@ -1398,6 +1429,34 @@
174 self.assertRaises(IndexError, cert.get_extension, 4)
175 self.assertRaises(TypeError, cert.get_extension, "hello")
176
177+ def test_nullbyte_san(self):
178+ """
179+ Test correct handling of CN and SAN with NULL bytes
180+
181+ see CVE-2013-4073
182+ """
183+ cert = load_certificate(FILETYPE_PEM, nullbyte_san_PEM)
184+ subject = cert.get_subject()
185+ self.assertEqual(subject.CN, 'null.python.org\x00example.org')
186+ issuer = cert.get_issuer()
187+ self.assertEqual(issuer.CN, 'null.python.org\x00example.org')
188+
189+ ext = cert.get_extension(0)
190+ self.assertEqual(ext.get_short_name(), b('basicConstraints'))
191+
192+ ext = cert.get_extension(1)
193+ self.assertEqual(ext.get_short_name(), b('subjectKeyIdentifier'))
194+
195+ ext = cert.get_extension(2)
196+ self.assertEqual(ext.get_short_name(), b('keyUsage'))
197+
198+ ext = cert.get_extension(3)
199+ self.assertEqual(ext.get_short_name(), b('subjectAltName'))
200+ self.assertEqual(str(ext),
201+ 'DNS:altnull.python.org\x00example.com, '
202+ 'email:null@python.org\x00user@example.org, '
203+ 'URI:http://null.python.org\x00http://example.org, '
204+ 'IP Address:192.0.2.1, IP Address:2001:DB8:0:0:0:0:0:1\n')
205
206 def test_invalid_digest_algorithm(self):
207 """

Subscribers

People subscribed via source and target branches

to status/vote changes: