From 9b1cbcf3a41ca2c76d4f65b54bcf7ab381bbb4b5 Mon Sep 17 00:00:00 2001
From: Felix Fontein <felix@fontein.de>
Date: Mon, 11 Feb 2019 11:30:56 +0100
Subject: [PATCH] openssl_csr: ignore empty strings in altnames (#51473)

* Ignore empty strings in altnames.

* Add changelog.

* Add idempotence check without SAN.

* Fix bug in cryptography backend.
---
 .../51473-openssl_csr-idempotence.yaml        |  2 +
 lib/ansible/modules/crypto/openssl_csr.py     |  4 +-
 .../targets/openssl_csr/tasks/impl.yml        | 42 +++++++++++++++++++
 .../targets/openssl_csr/tests/validate.yml    |  8 ++++
 4 files changed, 54 insertions(+), 2 deletions(-)
 create mode 100644 changelogs/fragments/51473-openssl_csr-idempotence.yaml

diff --git a/changelogs/fragments/51473-openssl_csr-idempotence.yaml b/changelogs/fragments/51473-openssl_csr-idempotence.yaml
new file mode 100644
index 00000000000..ed636b4a8f6
--- /dev/null
+++ b/changelogs/fragments/51473-openssl_csr-idempotence.yaml
@@ -0,0 +1,2 @@
+bugfixes:
+- "openssl_csr - fixes idempotence problem with PyOpenSSL backend when no Subject Alternative Names were specified."
diff --git a/lib/ansible/modules/crypto/openssl_csr.py b/lib/ansible/modules/crypto/openssl_csr.py
index 1ed86269eab..07e7eef639c 100644
--- a/lib/ansible/modules/crypto/openssl_csr.py
+++ b/lib/ansible/modules/crypto/openssl_csr.py
@@ -534,7 +534,7 @@ 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(',')]
+            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]
@@ -840,7 +840,7 @@ class CertificateSigningRequestCryptography(CertificateSigningRequestBase):
         def _check_subjectAltName(extensions):
             current_altnames_ext = _find_extension(extensions, cryptography.x509.SubjectAlternativeName)
             current_altnames = [str(altname) for altname in current_altnames_ext.value] if current_altnames_ext else []
-            altnames = [str(self._get_san(altname)) for altname in self.subjectAltName]
+            altnames = [str(self._get_san(altname)) for altname in self.subjectAltName] if self.subjectAltName else []
             if set(altnames) != set(current_altnames):
                 return False
             if altnames:
diff --git a/test/integration/targets/openssl_csr/tasks/impl.yml b/test/integration/targets/openssl_csr/tasks/impl.yml
index 79c2cd43b8e..6d7461270f9 100644
--- a/test/integration/targets/openssl_csr/tasks/impl.yml
+++ b/test/integration/targets/openssl_csr/tasks/impl.yml
@@ -41,6 +41,48 @@
   check_mode: yes
   register: generate_csr_check_idempotent_check
 
+- name: Generate CSR without SAN (check mode)
+  openssl_csr:
+    path: '{{ output_dir }}/csr-nosan.csr'
+    privatekey_path: '{{ output_dir }}/privatekey.pem'
+    subject:
+      commonName: www.ansible.com
+    useCommonNameForSAN: no
+    select_crypto_backend: '{{ select_crypto_backend }}'
+  check_mode: yes
+  register: generate_csr_nosan_check
+
+- name: Generate CSR without SAN
+  openssl_csr:
+    path: '{{ output_dir }}/csr-nosan.csr'
+    privatekey_path: '{{ output_dir }}/privatekey.pem'
+    subject:
+      commonName: www.ansible.com
+    useCommonNameForSAN: no
+    select_crypto_backend: '{{ select_crypto_backend }}'
+  register: generate_csr_nosan
+
+- name: Generate CSR without SAN (idempotent)
+  openssl_csr:
+    path: '{{ output_dir }}/csr-nosan.csr'
+    privatekey_path: '{{ output_dir }}/privatekey.pem'
+    subject:
+      commonName: www.ansible.com
+    useCommonNameForSAN: no
+    select_crypto_backend: '{{ select_crypto_backend }}'
+  register: generate_csr_nosan_check_idempotent
+
+- name: Generate CSR without SAN (idempotent, check mode)
+  openssl_csr:
+    path: '{{ output_dir }}/csr-nosan.csr'
+    privatekey_path: '{{ output_dir }}/privatekey.pem'
+    subject:
+      commonName: www.ansible.com
+    useCommonNameForSAN: no
+    select_crypto_backend: '{{ select_crypto_backend }}'
+  check_mode: yes
+  register: generate_csr_nosan_check_idempotent_check
+
 # keyUsage longname and shortname should be able to be used
 # interchangeably. Hence the long name is specified here
 # but the short name is used to test idempotency for ipsecuser
diff --git a/test/integration/targets/openssl_csr/tests/validate.yml b/test/integration/targets/openssl_csr/tests/validate.yml
index 818b988d08d..755b2d301ef 100644
--- a/test/integration/targets/openssl_csr/tests/validate.yml
+++ b/test/integration/targets/openssl_csr/tests/validate.yml
@@ -25,6 +25,14 @@
       - generate_csr_check_idempotent is not changed
       - generate_csr_check_idempotent_check is not changed
 
+- name: Validate CSR without SAN (check mode, idempotency)
+  assert:
+    that:
+      - generate_csr_nosan_check is changed
+      - generate_csr_nosan is changed
+      - generate_csr_nosan_check_idempotent is not changed
+      - generate_csr_nosan_check_idempotent_check is not changed
+
 - name: Validate CSR_KU_XKU (assert idempotency, change)
   assert:
     that: