From e95e963ad62735a0cd23a46e27a7cbef3790afb2 Mon Sep 17 00:00:00 2001 From: ohemorange Date: Fri, 16 Feb 2018 16:05:16 -0800 Subject: [PATCH] Get common name from CSR in new_order in ClientV2 (#5587) * switch new_order to use crypto_util._pyopenssl_cert_or_req_san * move certbot.crypto_util._get_names_from_loaded_cert_or_req functionality to acme.crypto_util._pyopenssl_cert_or_req_all_names --- acme/acme/client.py | 10 ++++------ acme/acme/crypto_util.py | 9 +++++++++ acme/acme/crypto_util_test.py | 24 ++++++++++++++++++++++++ acme/acme/testdata/cert-nocn.der | Bin 0 -> 1397 bytes certbot/crypto_util.py | 8 +------- 5 files changed, 38 insertions(+), 13 deletions(-) create mode 100644 acme/acme/testdata/cert-nocn.der diff --git a/acme/acme/client.py b/acme/acme/client.py index bc93ca06f..1f4ae4fad 100644 --- a/acme/acme/client.py +++ b/acme/acme/client.py @@ -1,7 +1,6 @@ """ACME client API.""" import base64 import collections -import cryptography import datetime from email.utils import parsedate_tz import heapq @@ -17,6 +16,7 @@ import re import requests import sys +from acme import crypto_util from acme import errors from acme import jws from acme import messages @@ -568,11 +568,9 @@ class ClientV2(ClientBase): :returns: The newly created order. :rtype: OrderResource """ - csr = cryptography.x509.load_pem_x509_csr(csr_pem, - cryptography.hazmat.backends.default_backend()) - san_extension = next(ext for ext in csr.extensions - if ext.oid == cryptography.x509.oid.ExtensionOID.SUBJECT_ALTERNATIVE_NAME) - dnsNames = san_extension.value.get_values_for_type(cryptography.x509.DNSName) + csr = OpenSSL.crypto.load_certificate_request(OpenSSL.crypto.FILETYPE_PEM, csr_pem) + # pylint: disable=protected-access + dnsNames = crypto_util._pyopenssl_cert_or_req_all_names(csr) identifiers = [] for name in dnsNames: diff --git a/acme/acme/crypto_util.py b/acme/acme/crypto_util.py index b8fba0348..a986721f0 100644 --- a/acme/acme/crypto_util.py +++ b/acme/acme/crypto_util.py @@ -186,6 +186,15 @@ def make_csr(private_key_pem, domains, must_staple=False): return OpenSSL.crypto.dump_certificate_request( OpenSSL.crypto.FILETYPE_PEM, csr) +def _pyopenssl_cert_or_req_all_names(loaded_cert_or_req): + common_name = loaded_cert_or_req.get_subject().CN + sans = _pyopenssl_cert_or_req_san(loaded_cert_or_req) + + if common_name is None: + return sans + else: + return [common_name] + [d for d in sans if d != common_name] + def _pyopenssl_cert_or_req_san(cert_or_req): """Get Subject Alternative Names from certificate or CSR using pyOpenSSL. diff --git a/acme/acme/crypto_util_test.py b/acme/acme/crypto_util_test.py index 1d7f83ccf..14aaac8b5 100644 --- a/acme/acme/crypto_util_test.py +++ b/acme/acme/crypto_util_test.py @@ -65,6 +65,30 @@ class SSLSocketAndProbeSNITest(unittest.TestCase): # self.assertRaises(errors.Error, self._probe, b'bar') +class PyOpenSSLCertOrReqAllNamesTest(unittest.TestCase): + """Test for acme.crypto_util._pyopenssl_cert_or_req_all_names.""" + + @classmethod + def _call(cls, loader, name): + # pylint: disable=protected-access + from acme.crypto_util import _pyopenssl_cert_or_req_all_names + return _pyopenssl_cert_or_req_all_names(loader(name)) + + def _call_cert(self, name): + return self._call(test_util.load_cert, name) + + def test_cert_one_san_no_common(self): + self.assertEqual(self._call_cert('cert-nocn.der'), + ['no-common-name.badssl.com']) + + def test_cert_no_sans_yes_common(self): + self.assertEqual(self._call_cert('cert.pem'), ['example.com']) + + def test_cert_two_sans_yes_common(self): + self.assertEqual(self._call_cert('cert-san.pem'), + ['example.com', 'www.example.com']) + + class PyOpenSSLCertOrReqSANTest(unittest.TestCase): """Test for acme.crypto_util._pyopenssl_cert_or_req_san.""" diff --git a/acme/acme/testdata/cert-nocn.der b/acme/acme/testdata/cert-nocn.der new file mode 100644 index 0000000000000000000000000000000000000000..59da83ccc6dc3f2a05fe762d6d3372cb446d88ac GIT binary patch literal 1397 zcmXqLVl6aiVu@V9%*4pVB#@u9Us#IesM(YUabM(~3ac9MvT?DFX?R2$!&+v%jyui@$=iqk>OnZe~epilL2x6-bU**cd7o6zr(rUzDDhmsyoq zl9`{U5SEyenF3)3rzV#cr78rc7L@_*baph56X!KFH!wCbHUNVtab6<>12ZEdBV$ub zQ_CoWY$SJw1{(?+2twS=Ed4`e(;vm)B!UDdfIVG98F8R5MnRyDq!9E5}j7rD>$H>aS+{DPw02Jq9 zYGPz$IK=YiN6$N!Lz`~|TrG$=Gg;zw%!&-p4N6;oPMx<=Tc^w}=YV&DkI$=3TMqCy zY}@d&=wuty<y8VeR_DBi?&nPSFc6UzRSD*z?WN>8RUfv3B+OC9k>W3I?eu7U#71 zRE9penEisy)$G-j&A)YDMK2k9 zJXb0_en_M$N8^>b*Zo&HTZ^{TMJ*D$lesb6U)Rz-AiH|l^)LeDLwhwS|v73 zy^Xs(Wq8!*E&kp8-JytK%_NSSJ>f0EjjX4i{mTxS;h^4nTwr%`!TU@JM|RiHnx855 zwmOGb*1Vo9<z44-7_GWflnou?CS@pS5qDU+ce3f47#0 zhnCQGX{UoK2C^Upd@N!tB6HsztZwACa?dQ3s1-Dyyl1Jj{cmt0ljUb*{LjL|%*49D zfCr>p7{q5XV1Q^=Wf3zFVdKzdV`ODzXJ&-6m<$3yiWOKq4crZ^*?52oSQs}MGBPnT zvlwU_Xu=dSF^b7%l#~<{Tj}c;gOi?Ka(-@pN?+{ z-k{bX6vj7TLb9iXGJ8r4RN%%kwn;#ppPyV@fMPCC70{=GEV2gDO_F)}y1<~w&Ck=# zOUzBxOG->BE(UuSIU@i|5MV}NWXSxZ>ULmW+UMOr#Oj|(nYf(rnDb6zkyW|Xz_ z$9{(K%Jus_l6IO3g=HA){+awKO)O|fmTT1Z+#7Ryp6LWmIpcR=gMC-d`J0n;Teo@| z6zW||Pht60z4=n>rj4&JFaGB1)RMV*x5}HhJC+@K>piVA&uZReo2JynpBFM}&U!55 z^j+hWs<_`F(Vy8@W!9tBm(mqy|NpbeP?kwtdY#U4)jc=2tozb#`ZwppYdhvmr}>U7 l*UbLhr)aUs$L-SL{YJ0C_h)*Gh(7yrDof|BxSMr#8URDu=Gy=O literal 0 HcmV?d00001 diff --git a/certbot/crypto_util.py b/certbot/crypto_util.py index 3ae16529d..8368855cd 100644 --- a/certbot/crypto_util.py +++ b/certbot/crypto_util.py @@ -340,14 +340,8 @@ def _get_names_from_cert_or_req(cert_or_req, load_func, typ): def _get_names_from_loaded_cert_or_req(loaded_cert_or_req): - common_name = loaded_cert_or_req.get_subject().CN # pylint: disable=protected-access - sans = acme_crypto_util._pyopenssl_cert_or_req_san(loaded_cert_or_req) - - if common_name is None: - return sans - else: - return [common_name] + [d for d in sans if d != common_name] + return acme_crypto_util._pyopenssl_cert_or_req_all_names(loaded_cert_or_req) def get_names_from_cert(csr, typ=OpenSSL.crypto.FILETYPE_PEM):