Skip to content

Commit

Permalink
Merge the fix for NUL byte handling in subjectAltName when rendering …
Browse files Browse the repository at this point in the history
…an X509Extension as a string.
  • Loading branch information
exarkun committed Aug 23, 2013
2 parents 54cc390 + 9af07b0 commit 6bbf44a
Show file tree
Hide file tree
Showing 4 changed files with 149 additions and 6 deletions.
7 changes: 7 additions & 0 deletions ChangeLog
Original file line number Diff line number Diff line change
@@ -1,3 +1,10 @@
2013-08-11 Christian Heimes <christian@python.org>

* OpenSSL/crypto/x509ext.c: Fix handling of NULL bytes inside
subjectAltName general names when formatting an X509 extension
as a string.
* OpenSSL/crypto/x509.c: Fix memory leak in get_extension().

2012-04-03 Jean-Paul Calderone <exarkun@twistedmatrix.com>

* OpenSSL/crypto/pkey.c: Release the GIL around RSA and DSA key
Expand Down
1 change: 1 addition & 0 deletions OpenSSL/crypto/x509.c
Original file line number Diff line number Diff line change
Expand Up @@ -758,6 +758,7 @@ crypto_X509_get_extension(crypto_X509Obj *self, PyObject *args) {

extobj = PyObject_New(crypto_X509ExtensionObj, &crypto_X509Extension_Type);
extobj->x509_extension = X509_EXTENSION_dup(ext);
extobj->dealloc = 1;

return (PyObject*)extobj;
}
Expand Down
83 changes: 78 additions & 5 deletions OpenSSL/crypto/x509ext.c
Original file line number Diff line number Diff line change
Expand Up @@ -236,19 +236,92 @@ crypto_X509Extension_dealloc(crypto_X509ExtensionObj *self)
PyObject_Del(self);
}


/* Special handling of subjectAltName. OpenSSL's builtin formatter,
* X509V3_EXT_print, mishandles NUL bytes allowing a truncated display that
* does not accurately reflect what's in the extension.
*/
int
crypto_X509Extension_str_subjectAltName(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.
*/
static PyObject *
crypto_X509Extension_str(crypto_X509ExtensionObj *self)
{
crypto_X509Extension_str(crypto_X509ExtensionObj *self) {
int str_len;
char *tmp_str;
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_subjectAltName(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);
return NULL;
Expand All @@ -267,7 +340,7 @@ PyTypeObject crypto_X509Extension_Type = {
"X509Extension",
sizeof(crypto_X509ExtensionObj),
0,
(destructor)crypto_X509Extension_dealloc,
(destructor)crypto_X509Extension_dealloc,
NULL, /* print */
NULL, /* getattr */
NULL, /* setattr (setattrfunc)crypto_X509Name_setattr, */
Expand Down
64 changes: 63 additions & 1 deletion OpenSSL/test/test_crypto.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

from unittest import main

import os, re, sys
import os, re
from subprocess import PIPE, Popen
from datetime import datetime, timedelta

Expand Down Expand Up @@ -265,6 +265,37 @@ def normalize_privatekey_pem(pem):
-----END RSA PRIVATE KEY-----
""")

# certificate with NULL bytes in subjectAltName and common name

nulbyteSubjectAltNamePEM = 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):
"""
Expand Down Expand Up @@ -871,6 +902,19 @@ def test_get_components(self):
[(b("CN"), b("foo")), (b("OU"), b("bar"))])


def test_load_nul_byte_attribute(self):
"""
An :py:class:`OpenSSL.crypto.X509Name` from an
:py:class:`OpenSSL.crypto.X509` instance loaded from a file can have a
NUL byte in the value of one of its attributes.
"""
cert = load_certificate(FILETYPE_PEM, nulbyteSubjectAltNamePEM)
subject = cert.get_subject()
self.assertEqual(
"null.python.org\x00example.org", subject.commonName)



class _PKeyInteractionTestsMixin:
"""
Tests which involve another thing and a PKey.
Expand Down Expand Up @@ -1399,6 +1443,24 @@ def test_get_extension(self):
self.assertRaises(TypeError, cert.get_extension, "hello")


def test_nullbyte_subjectAltName(self):
"""
The fields of a `subjectAltName` extension on an X509 may contain NUL
bytes and this value is reflected in the string representation of the
extension object.
"""
cert = load_certificate(FILETYPE_PEM, nulbyteSubjectAltNamePEM)

ext = cert.get_extension(3)
self.assertEqual(ext.get_short_name(), b('subjectAltName'))
self.assertEqual(
b("DNS:altnull.python.org\x00example.com, "
"email:null@python.org\x00user@example.org, "
"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"),
b(str(ext)))


def test_invalid_digest_algorithm(self):
"""
:py:obj:`X509.digest` raises :py:obj:`ValueError` if called with an unrecognized hash
Expand Down

0 comments on commit 6bbf44a

Please sign in to comment.