From 75ca8eb42f49e2814fbe80cb429690537122ef65 Mon Sep 17 00:00:00 2001 From: Felix Fontein Date: Mon, 24 Jun 2019 06:34:12 +0200 Subject: [PATCH] openssl_certificate: fix failing SAN comparisons (#58256) * Fix failing SAN comparison for older cryptography versions due to not implemented __hashh__ functions. * Fix SAN comparison: IPv6 addresses need to be normalized before comparing strings. * Add changelog. * Fix comment. --- ...256-openssl_certificate-san-comparison.yml | 2 + lib/ansible/module_utils/crypto.py | 34 ++++++++++++++ .../modules/crypto/openssl_certificate.py | 18 ++++++-- lib/ansible/modules/crypto/openssl_csr.py | 2 +- .../openssl_certificate/tasks/assertonly.yml | 46 +++++++++++++++++++ 5 files changed, 98 insertions(+), 4 deletions(-) create mode 100644 changelogs/fragments/58256-openssl_certificate-san-comparison.yml diff --git a/changelogs/fragments/58256-openssl_certificate-san-comparison.yml b/changelogs/fragments/58256-openssl_certificate-san-comparison.yml new file mode 100644 index 00000000000..8eb48d7aed5 --- /dev/null +++ b/changelogs/fragments/58256-openssl_certificate-san-comparison.yml @@ -0,0 +1,2 @@ +bugfixes: +- "openssl_certificate - fix Subject Alternate Name comparison, which was broken for IPv6 addresses with PyOpenSSL, or with older cryptography versions (before 2.1)." diff --git a/lib/ansible/module_utils/crypto.py b/lib/ansible/module_utils/crypto.py index 57956e8c267..9b579a89335 100644 --- a/lib/ansible/module_utils/crypto.py +++ b/lib/ansible/module_utils/crypto.py @@ -27,6 +27,8 @@ # For more details, search for the function _OID_MAP. +from distutils.version import LooseVersion + try: import OpenSSL from OpenSSL import crypto @@ -36,11 +38,43 @@ except ImportError: pass try: + import cryptography from cryptography import x509 from cryptography.hazmat.backends import default_backend as cryptography_backend from cryptography.hazmat.primitives.serialization import load_pem_private_key from cryptography.hazmat.primitives import hashes import ipaddress + + # Older versions of cryptography (< 2.1) do not have __hash__ functions for + # general name objects (DNSName, IPAddress, ...), while providing overloaded + # equality and string representation operations. This makes it impossible to + # use them in hash-based data structures such as set or dict. Since we are + # actually doing that in openssl_certificate, and potentially in other code, + # we need to monkey-patch __hash__ for these classes to make sure our code + # works fine. + if LooseVersion(cryptography.__version__) < LooseVersion('2.1'): + # A very simply hash function which relies on the representation + # of an object to be implemented. This is the case since at least + # cryptography 1.0, see + # https://github.com/pyca/cryptography/commit/7a9abce4bff36c05d26d8d2680303a6f64a0e84f + def simple_hash(self): + return hash(repr(self)) + + # The hash functions for the following types were added for cryptography 2.1: + # https://github.com/pyca/cryptography/commit/fbfc36da2a4769045f2373b004ddf0aff906cf38 + x509.DNSName.__hash__ = simple_hash + x509.DirectoryName.__hash__ = simple_hash + x509.GeneralName.__hash__ = simple_hash + x509.IPAddress.__hash__ = simple_hash + x509.OtherName.__hash__ = simple_hash + x509.RegisteredID.__hash__ = simple_hash + + if LooseVersion(cryptography.__version__) < LooseVersion('1.2'): + # The hash functions for the following types were added for cryptography 1.2: + # https://github.com/pyca/cryptography/commit/b642deed88a8696e5f01ce6855ccf89985fc35d0 + # https://github.com/pyca/cryptography/commit/d1b5681f6db2bde7a14625538bd7907b08dfb486 + x509.RFC822Name.__hash__ = simple_hash + x509.UniformResourceIdentifier.__hash__ = simple_hash except ImportError: # Error handled in the calling module. pass diff --git a/lib/ansible/modules/crypto/openssl_certificate.py b/lib/ansible/modules/crypto/openssl_certificate.py index 9e868a8239f..81e5f8c5234 100644 --- a/lib/ansible/modules/crypto/openssl_certificate.py +++ b/lib/ansible/modules/crypto/openssl_certificate.py @@ -542,6 +542,7 @@ from distutils.version import LooseVersion from ansible.module_utils import crypto as crypto_utils from ansible.module_utils.basic import AnsibleModule, missing_required_lib from ansible.module_utils._text import to_native, to_bytes, to_text +from ansible.module_utils.compat import ipaddress as compat_ipaddress MINIMAL_CRYPTOGRAPHY_VERSION = '1.6' MINIMAL_PYOPENSSL_VERSION = '0.15' @@ -1654,15 +1655,26 @@ class AssertOnlyCertificate(AssertOnlyCertificateBase): if self.extended_key_usage: return NO_EXTENSION + def _normalize_san(self, san): + # Apparently OpenSSL returns 'IP address' not 'IP' as specifier when converting the subjectAltName to string + # although it won't accept this specifier when generating the CSR. (https://github.com/openssl/openssl/issues/4004) + if san.startswith('IP Address:'): + san = 'IP:' + san[len('IP Address:'):] + if san.startswith('IP:'): + ip = compat_ipaddress.ip_address(san[3:]) + san = 'IP:{0}'.format(ip.compressed) + return san + def _validate_subject_alt_name(self): found = False for extension_idx in range(0, self.cert.get_extension_count()): extension = self.cert.get_extension(extension_idx) if extension.get_short_name() == b'subjectAltName': found = True - l_altnames = [altname.replace(b'IP Address', b'IP') for altname in - to_bytes(extension, errors='surrogate_or_strict').split(b', ')] - if not compare_sets(self.subject_alt_name, l_altnames, self.subject_alt_name_strict): + l_altnames = [self._normalize_san(altname.strip()) for altname in + to_text(extension, errors='surrogate_or_strict').split(', ')] + sans = [self._normalize_san(to_text(san, errors='surrogate_or_strict')) for san in self.subject_alt_name] + if not compare_sets(sans, l_altnames, self.subject_alt_name_strict): return self.subject_alt_name, l_altnames if not found: # This is only bad if the user specified a non-empty list diff --git a/lib/ansible/modules/crypto/openssl_csr.py b/lib/ansible/modules/crypto/openssl_csr.py index 65a3fe7f60a..c54416576d1 100644 --- a/lib/ansible/modules/crypto/openssl_csr.py +++ b/lib/ansible/modules/crypto/openssl_csr.py @@ -555,7 +555,7 @@ class CertificateSigningRequestPyOpenSSL(CertificateSigningRequestBase): raise CertificateSigningRequestError(exc) def _normalize_san(self, san): - # apperently openssl returns 'IP address' not 'IP' as specifier when converting the subjectAltName to string + # Apparently OpenSSL returns 'IP address' not 'IP' as specifier when converting the subjectAltName to string # although it won't accept this specifier when generating the CSR. (https://github.com/openssl/openssl/issues/4004) if san.startswith('IP Address:'): san = 'IP:' + san[len('IP Address:'):] diff --git a/test/integration/targets/openssl_certificate/tasks/assertonly.yml b/test/integration/targets/openssl_certificate/tasks/assertonly.yml index 0bee98fbbc6..994e00df817 100644 --- a/test/integration/targets/openssl_certificate/tasks/assertonly.yml +++ b/test/integration/targets/openssl_certificate/tasks/assertonly.yml @@ -18,6 +18,18 @@ commonName: www.example.com useCommonNameForSAN: no +- name: (Assertonly, {{select_crypto_backend}}) - Generate CSR (with SANs) + openssl_csr: + path: '{{ output_dir }}/csr_sans.csr' + privatekey_path: '{{ output_dir }}/privatekey.pem' + subject: + commonName: www.example.com + subject_alt_name: + - "DNS:ansible.com" + - "IP:127.0.0.1" + - "IP:::1" + useCommonNameForSAN: no + - name: (Assertonly, {{select_crypto_backend}}) - Generate selfsigned certificate (no extensions) openssl_certificate: path: '{{ output_dir }}/cert_noext.pem' @@ -27,6 +39,15 @@ selfsigned_digest: sha256 select_crypto_backend: '{{ select_crypto_backend }}' +- name: (Assertonly, {{select_crypto_backend}}) - Generate selfsigned certificate (with SANs) + openssl_certificate: + path: '{{ output_dir }}/cert_sans.pem' + csr_path: '{{ output_dir }}/csr_sans.csr' + privatekey_path: '{{ output_dir }}/privatekey.pem' + provider: selfsigned + selfsigned_digest: sha256 + select_crypto_backend: '{{ select_crypto_backend }}' + - name: (Assertonly, {{select_crypto_backend}}) - Assert that subject_alt_name is there (should fail) openssl_certificate: path: '{{ output_dir }}/cert_noext.pem' @@ -37,6 +58,29 @@ ignore_errors: yes register: extension_missing_san +- name: (Assertonly, {{select_crypto_backend}}) - Assert that subject_alt_name is there + openssl_certificate: + path: '{{ output_dir }}/cert_sans.pem' + provider: assertonly + subject_alt_name: + - "DNS:ansible.com" + - "IP:127.0.0.1" + - "IP:::1" + select_crypto_backend: '{{ select_crypto_backend }}' + register: extension_san + +- name: (Assertonly, {{select_crypto_backend}}) - Assert that subject_alt_name is there (strict) + openssl_certificate: + path: '{{ output_dir }}/cert_sans.pem' + provider: assertonly + subject_alt_name: + - "DNS:ansible.com" + - "IP:127.0.0.1" + - "IP:::1" + subject_alt_name_strict: yes + select_crypto_backend: '{{ select_crypto_backend }}' + register: extension_san_strict + - name: (Assertonly, {{select_crypto_backend}}) - Assert that key_usage is there (should fail) openssl_certificate: path: '{{ output_dir }}/cert_noext.pem' @@ -61,6 +105,8 @@ that: - extension_missing_san is failed - "'Found no subjectAltName extension' in extension_missing_san.msg" + - extension_san is succeeded + - extension_san_strict is succeeded - extension_missing_ku is failed - "'Found no keyUsage extension' in extension_missing_ku.msg" - extension_missing_eku is failed