From 532d155b1cd01392f53b2278e5d8163b90e94979 Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Sun, 10 May 2015 09:22:19 +0000 Subject: [PATCH] get_sans_from_csr using pyOpenSSL --- letsencrypt/client/crypto_util.py | 68 +++++++++----------- letsencrypt/client/tests/crypto_util_test.py | 14 +--- setup.py | 3 +- 3 files changed, 37 insertions(+), 48 deletions(-) diff --git a/letsencrypt/client/crypto_util.py b/letsencrypt/client/crypto_util.py index 6419aeebe..8a654c77b 100644 --- a/letsencrypt/client/crypto_util.py +++ b/letsencrypt/client/crypto_util.py @@ -4,6 +4,7 @@ is capable of handling the signatures. """ +import logging import time import Crypto.Hash.SHA256 @@ -11,6 +12,7 @@ import Crypto.PublicKey.RSA import Crypto.Signature.PKCS1_v1_5 import M2Crypto +import OpenSSL def make_csr(key_str, domains): @@ -163,7 +165,29 @@ def make_ss_cert(key_str, domains, not_before=None, return cert.as_pem() -def get_sans_from_csr(csr): +def _request_san(req): # TODO: implement directly in PyOpenSSL! + # constants based on implementation of + # OpenSSL.crypto.X509Error._subjectAltNameString + parts_separator = ", " + part_separator = ":" + extension_short_name = "subjectAltName" + + # pylint: disable=protected-access,no-member + label = OpenSSL.crypto.X509Extension._prefixes[OpenSSL.crypto._lib.GEN_DNS] + assert parts_separator not in label + prefix = label + part_separator + + extensions = [ext._subjectAltNameString().split(parts_separator) + for ext in req.get_extensions() + if ext.get_short_name() == extension_short_name] + # WARNING: this function assumes that no SAN can include + # parts_separator, hence the split! + + return [part.split(part_separator)[1] for parts in extensions + for part in parts if part.startswith(prefix)] + + +def get_sans_from_csr(csr, typ=OpenSSL.crypto.FILETYPE_PEM): """Get list of Subject Alternative Names from signing request. :param str csr: Certificate Signing Request in PEM format (must contain @@ -172,39 +196,11 @@ def get_sans_from_csr(csr): :returns: List of referenced subject alternative names :rtype: list - """ - # TODO: This is a temporary solution involving parsing the .as_text() - # output because there doesn't seem to be a built-in feature in - # any Python cryptography module that performs this function. - # In the future we should try to replace this with a more direct - # use of relevant OpenSSL or other X509-parsing APIs. - req = M2Crypto.X509.load_request_string(csr) - text = req.as_text().split("\n") - if (len(text) < 2 or text[0] != "Certificate Request:" or - text[1] != " Data:"): - raise ValueError("Unable to parse CSR") - text = text[2:] - while text and text[0] != " Attributes:": - text = text[1:] - while text and text[0] != " Requested Extensions:": - text = text[1:] - while text and text[0] != " X509v3 Subject Alternative Name: ": - text = text[1:] - text = text[1:] - if not text: - raise ValueError("Unable to parse CSR") - # XXX: This might break for non-ASCII hostnames and for non-DNS - # names in SANs. There is also a parser safety concern about - # whether the CSR's contents are interpreted in the same way - # by this code and by any other code that might interpret the - # CSR for a different purpose. Also, if there is a non-DNS - # name in a SAN that contains ", DNS:example.com, " as part - # of the name (for example, in the comment field of an e-mail - # SAN), this code will be fooled into returning that name as - # if it were an additional DNS SAN. The severity of this is - # unclear, because the client currently presents the results of - # this list to the user for confirmation before requesting the - # cert from the server. - return [san.split(":")[1] for san in text[0].strip().split(", ") - if san.startswith("DNS:")] + """ + try: + request = OpenSSL.crypto.load_certificate_request(typ, csr) + except OpenSSL.crypto.Error as error: + logging.exception(error) + raise + return _request_san(request) diff --git a/letsencrypt/client/tests/crypto_util_test.py b/letsencrypt/client/tests/crypto_util_test.py index 1e02a1296..5d7c4a7ba 100644 --- a/letsencrypt/client/tests/crypto_util_test.py +++ b/letsencrypt/client/tests/crypto_util_test.py @@ -5,6 +5,7 @@ import pkg_resources import unittest import M2Crypto +import OpenSSL RSA256_KEY = pkg_resources.resource_string(__name__, 'testdata/rsa256_key.pem') @@ -120,24 +121,15 @@ class GetSansFromCsrTest(unittest.TestCase): def test_parse_non_csr(self): from letsencrypt.client.crypto_util import get_sans_from_csr - self.assertRaises(M2Crypto.X509.X509Error, get_sans_from_csr, + self.assertRaises(OpenSSL.crypto.Error, get_sans_from_csr, "hello there") def test_parse_no_sans(self): from letsencrypt.client.crypto_util import get_sans_from_csr csr = pkg_resources.resource_string( __name__, os.path.join('testdata', 'csr-nosans.pem')) - self.assertRaises(ValueError, get_sans_from_csr, csr) + self.assertEqual([], get_sans_from_csr(csr)) - @mock.patch("M2Crypto.X509.load_request_string") - def test_parse_weird_m2crypto_output(self, mock_lrs): - # It's not clear how to reach this exception with invalid input, - # because M2Crypto is likely to raise X509Error rather than - # returning invalid output, but we can test the possibility with - # mock. - mock_lrs.as_text.return_value = "Something other than OpenSSL output" - from letsencrypt.client.crypto_util import get_sans_from_csr - self.assertRaises(ValueError, get_sans_from_csr, "input") class MakeCSRTest(unittest.TestCase): # pylint: disable=too-few-public-methods """Tests for letsencrypt.client.crypto_util.make_csr.""" diff --git a/setup.py b/setup.py index 1fc643304..79ff892b7 100755 --- a/setup.py +++ b/setup.py @@ -28,7 +28,8 @@ install_requires = [ 'mock', 'psutil>=2.1.0', # net_connections introduced in 2.1.0 'pycrypto', - 'PyOpenSSL', + # https://pyopenssl.readthedocs.org/en/latest/api/crypto.html#OpenSSL.crypto.X509Req.get_extensions + 'PyOpenSSL>=0.15', 'python-augeas', 'python2-pythondialog', 'requests',