openssl_csr: fix idempotency problems (#55142)

* Add test for generating a CSR with everything, and testing idempotency.

* Proper SAN normalization before comparison.

* Fix check in cryptography backend.

* Convert SANs to text. Update comments.

* Add changelog.

5 files changed, 209 insertions, 7 deletions

diff --git a/changelogs/fragments/55142-openssl_csr-crypto-backend-san.yml b/changelogs/fragments/55142-openssl_csr-crypto-backend-san.yml
new file mode 100644
index 0000000000..707e0cbef5
--- /dev/null
+++ b/changelogs/fragments/55142-openssl_csr-crypto-backend-san.yml
@@ -0,0 +1,3 @@
+bugfixes:
+- "openssl_csr - the cryptography backend's idempotency checking for basic constraints was broken."
+- "openssl_csr - SAN normalization for IP addresses for the pyOpenSSL backend was broken."
diff --git a/lib/ansible/modules/crypto/openssl_csr.py b/lib/ansible/modules/crypto/openssl_csr.py
index 6584d636f0..ed9022a829 100644
--- a/lib/ansible/modules/crypto/openssl_csr.py
+++ b/lib/ansible/modules/crypto/openssl_csr.py
@@ -554,6 +554,16 @@ class CertificateSigningRequestPyOpenSSL(CertificateSigningRequestBase):
         except crypto_utils.OpenSSLBadPassphraseError as exc:
             raise CertificateSigningRequestError(exc)
 
+    def _normalize_san(self, san):
+        # apperently 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 = ipaddress.ip_address(san[3:])
+            san = 'IP:{0}'.format(ip.compressed)
+        return san
+
     def _check_csr(self):
         def _check_subject(csr):
             subject = [(OpenSSL._util.lib.OBJ_txt2nid(to_bytes(sub[0])), to_bytes(sub[1])) for sub in self.subject]
@@ -565,12 +575,11 @@ class CertificateSigningRequestPyOpenSSL(CertificateSigningRequestBase):
 
         def _check_subjectAltName(extensions):
             altnames_ext = next((ext for ext in extensions if ext.get_short_name() == b'subjectAltName'), '')
-            altnames = [altname.strip() for altname in str(altnames_ext).split(',') if altname.strip()]
-            # apperently 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)
-            altnames = [name if not name.startswith('IP Address:') else "IP:" + name.split(':', 1)[1] for name in altnames]
+            altnames = [self._normalize_san(altname.strip()) for altname in
+                        to_text(altnames_ext, errors='surrogate_or_strict').split(',') if altname.strip()]
             if self.subjectAltName:
-                if set(altnames) != set(self.subjectAltName) or altnames_ext.get_critical() != self.subjectAltName_critical:
+                if (set(altnames) != set([self._normalize_san(to_text(name)) for name in self.subjectAltName]) or
+                        altnames_ext.get_critical() != self.subjectAltName_critical):
                     return False
             else:
                 if altnames:
@@ -761,8 +770,8 @@ class CertificateSigningRequestCryptography(CertificateSigningRequestBase):
 
         def _check_basicConstraints(extensions):
             bc_ext = _find_extension(extensions, cryptography.x509.BasicConstraints)
-            current_ca = bc_ext.ca if bc_ext else False
-            current_path_length = bc_ext.path_length if bc_ext else None
+            current_ca = bc_ext.value.ca if bc_ext else False
+            current_path_length = bc_ext.value.path_length if bc_ext else None
             ca, path_length = crypto_utils.cryptography_get_basic_constraints(self.basicConstraints)
             # Check CA flag
             if ca != current_ca:
diff --git a/lib/ansible/modules/crypto/openssl_csr_info.py b/lib/ansible/modules/crypto/openssl_csr_info.py
index 8edb165e0e..c1c4ee237b 100644
--- a/lib/ansible/modules/crypto/openssl_csr_info.py
+++ b/lib/ansible/modules/crypto/openssl_csr_info.py
@@ -439,6 +439,8 @@ class CertificateSigningRequestInfoPyOpenSSL(CertificateSigningRequestInfo):
             return None, False
 
     def _normalize_san(self, san):
+        # apperently 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:'):
diff --git a/test/integration/targets/openssl_csr/tasks/impl.yml b/test/integration/targets/openssl_csr/tasks/impl.yml
index f5af14cfa7..43393111c3 100644
--- a/test/integration/targets/openssl_csr/tasks/impl.yml
+++ b/test/integration/targets/openssl_csr/tasks/impl.yml
@@ -330,3 +330,184 @@
     backup: yes
     select_crypto_backend: '{{ select_crypto_backend }}'
   register: csr_backup_5
+
+- name: Generate CSR with everything
+  openssl_csr:
+    path: '{{ output_dir }}/csr_everything.csr'
+    privatekey_path: '{{ output_dir }}/privatekey.pem'
+    subject:
+      commonName: www.example.com
+      C: de
+      L: Somewhere
+      ST: Zurich
+      streetAddress: Welcome Street
+      O: Ansible
+      organizationalUnitName: Crypto Department
+      serialNumber: "1234"
+      SN: Last Name
+      GN: First Name
+      title: Chief
+      pseudonym: test
+      UID: asdf
+      emailAddress: test@example.com
+      postalAddress: 1234 Somewhere
+      postalCode: "1234"
+    useCommonNameForSAN: no
+    key_usage:
+      - digitalSignature
+      - keyAgreement
+      - Non Repudiation
+      - Key Encipherment
+      - dataEncipherment
+      - Certificate Sign
+      - cRLSign
+      - Encipher Only
+      - decipherOnly
+    key_usage_critical: yes
+    extended_key_usage:
+      - serverAuth  # the same as "TLS Web Server Authentication"
+      - TLS Web Server Authentication
+      - TLS Web Client Authentication
+      - Code Signing
+      - E-mail Protection
+      - timeStamping
+      - OCSPSigning
+      - Any Extended Key Usage
+      - qcStatements
+      - DVCS
+      - IPSec User
+      - biometricInfo
+    subject_alt_name:
+      - "DNS:www.ansible.com"
+      - "IP:1.2.3.4"
+      - "IP:::1"
+      - "email:test@example.org"
+      - "URI:https://example.org/test/index.html"
+    basic_constraints:
+      - "CA:TRUE"
+      - "pathlen:23"
+    basic_constraints_critical: yes
+    ocsp_must_staple: yes
+    select_crypto_backend: '{{ select_crypto_backend }}'
+  register: everything_1
+
+- name: Generate CSR with everything (idempotent, check mode)
+  openssl_csr:
+    path: '{{ output_dir }}/csr_everything.csr'
+    privatekey_path: '{{ output_dir }}/privatekey.pem'
+    subject:
+      commonName: www.example.com
+      C: de
+      L: Somewhere
+      ST: Zurich
+      streetAddress: Welcome Street
+      O: Ansible
+      organizationalUnitName: Crypto Department
+      serialNumber: "1234"
+      SN: Last Name
+      GN: First Name
+      title: Chief
+      pseudonym: test
+      UID: asdf
+      emailAddress: test@example.com
+      postalAddress: 1234 Somewhere
+      postalCode: "1234"
+    useCommonNameForSAN: no
+    key_usage:
+      - digitalSignature
+      - keyAgreement
+      - Non Repudiation
+      - Key Encipherment
+      - dataEncipherment
+      - Certificate Sign
+      - cRLSign
+      - Encipher Only
+      - decipherOnly
+    key_usage_critical: yes
+    extended_key_usage:
+      - serverAuth  # the same as "TLS Web Server Authentication"
+      - TLS Web Server Authentication
+      - TLS Web Client Authentication
+      - Code Signing
+      - E-mail Protection
+      - timeStamping
+      - OCSPSigning
+      - Any Extended Key Usage
+      - qcStatements
+      - DVCS
+      - IPSec User
+      - biometricInfo
+    subject_alt_name:
+      - "DNS:www.ansible.com"
+      - "IP:1.2.3.4"
+      - "IP:::1"
+      - "email:test@example.org"
+      - "URI:https://example.org/test/index.html"
+    basic_constraints:
+      - "CA:TRUE"
+      - "pathlen:23"
+    basic_constraints_critical: yes
+    ocsp_must_staple: yes
+    select_crypto_backend: '{{ select_crypto_backend }}'
+  check_mode: yes
+  register: everything_2
+
+- name: Generate CSR with everything (idempotent)
+  openssl_csr:
+    path: '{{ output_dir }}/csr_everything.csr'
+    privatekey_path: '{{ output_dir }}/privatekey.pem'
+    subject:
+      commonName: www.example.com
+      C: de
+      L: Somewhere
+      ST: Zurich
+      streetAddress: Welcome Street
+      O: Ansible
+      organizationalUnitName: Crypto Department
+      serialNumber: "1234"
+      SN: Last Name
+      GN: First Name
+      title: Chief
+      pseudonym: test
+      UID: asdf
+      emailAddress: test@example.com
+      postalAddress: 1234 Somewhere
+      postalCode: "1234"
+    useCommonNameForSAN: no
+    key_usage:
+      - digitalSignature
+      - keyAgreement
+      - Non Repudiation
+      - Key Encipherment
+      - dataEncipherment
+      - Certificate Sign
+      - cRLSign
+      - Encipher Only
+      - decipherOnly
+    key_usage_critical: yes
+    extended_key_usage:
+      - serverAuth  # the same as "TLS Web Server Authentication"
+      - TLS Web Server Authentication
+      - TLS Web Client Authentication
+      - Code Signing
+      - E-mail Protection
+      - timeStamping
+      - OCSPSigning
+      - Any Extended Key Usage
+      - qcStatements
+      - DVCS
+      - IPSec User
+      - biometricInfo
+    subject_alt_name:
+      - "DNS:www.ansible.com"
+      - "IP:1.2.3.4"
+      - "IP:::1"
+      - "email:test@example.org"
+      - "URI:https://example.org/test/index.html"
+    basic_constraints:
+      - "CA:TRUE"
+      - "pathlen:23"
+    basic_constraints_critical: yes
+    ocsp_must_staple: yes
+    select_crypto_backend: '{{ select_crypto_backend }}'
+  register: everything_3
diff --git a/test/integration/targets/openssl_csr/tests/validate.yml b/test/integration/targets/openssl_csr/tests/validate.yml
index 585cf64c0a..43e3b3336b 100644
--- a/test/integration/targets/openssl_csr/tests/validate.yml
+++ b/test/integration/targets/openssl_csr/tests/validate.yml
@@ -138,3 +138,10 @@
       - csr_backup_4.backup_file is string
       - csr_backup_5 is not changed
       - csr_backup_5.backup_file is undefined
+
+- name: Check CSR with everything
+  assert:
+    that:
+      - everything_1 is changed
+      - everything_2 is not changed
+      - everything_3 is not changed