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.
2019-06-24 06:34:12 +02:00 · 2019-06-24 06:34:12 +02:00 · 75ca8eb42f
commit 75ca8eb42f
parent 73dc4d7e97
5 changed files with 98 additions and 4 deletions
--- a/changelogs/fragments/58256-openssl_certificate-san-comparison.yml
+++ 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)."
--- 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
--- 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
--- 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:'):]
--- 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