From b2e992cecd93fbedc260d86fcb25bc39191e0b5b Mon Sep 17 00:00:00 2001
From: Felix Fontein <felix@fontein.de>
Date: Thu, 7 Mar 2019 16:29:35 +0100
Subject: [PATCH] openssl_csr: improve subject validation (#53198)

* Improve subject field validation.

* Add country name idempotency test.

* Add failed country name test.

* Add changelog.
---
 .../53198-openssl_csr-subject-validation.yml  |  2 ++
 lib/ansible/modules/crypto/openssl_csr.py     | 15 +++++---
 .../targets/openssl_csr/tasks/impl.yml        | 35 +++++++++++++++++++
 .../targets/openssl_csr/tests/validate.yml    |  8 +++++
 4 files changed, 56 insertions(+), 4 deletions(-)
 create mode 100644 changelogs/fragments/53198-openssl_csr-subject-validation.yml

diff --git a/changelogs/fragments/53198-openssl_csr-subject-validation.yml b/changelogs/fragments/53198-openssl_csr-subject-validation.yml
new file mode 100644
index 00000000000..b5f92e7517a
--- /dev/null
+++ b/changelogs/fragments/53198-openssl_csr-subject-validation.yml
@@ -0,0 +1,2 @@
+bugfixes:
+- "openssl_csr - improve ``subject`` validation."
diff --git a/lib/ansible/modules/crypto/openssl_csr.py b/lib/ansible/modules/crypto/openssl_csr.py
index 7e4835b13df..7e0f9acbd0e 100644
--- a/lib/ansible/modules/crypto/openssl_csr.py
+++ b/lib/ansible/modules/crypto/openssl_csr.py
@@ -484,7 +484,11 @@ class CertificateSigningRequestPyOpenSSL(CertificateSigningRequestBase):
             if entry[1] is not None:
                 # Workaround for https://github.com/pyca/pyopenssl/issues/165
                 nid = OpenSSL._util.lib.OBJ_txt2nid(to_bytes(entry[0]))
-                OpenSSL._util.lib.X509_NAME_add_entry_by_NID(subject._name, nid, OpenSSL._util.lib.MBSTRING_UTF8, to_bytes(entry[1]), -1, -1, 0)
+                if nid == 0:
+                    raise CertificateSigningRequestError('Unknown subject field identifier "{0}"'.format(entry[0]))
+                res = OpenSSL._util.lib.X509_NAME_add_entry_by_NID(subject._name, nid, OpenSSL._util.lib.MBSTRING_UTF8, to_bytes(entry[1]), -1, -1, 0)
+                if res == 0:
+                    raise CertificateSigningRequestError('Invalid value for subject field identifier "{0}": {1}'.format(entry[0], entry[1]))
 
         extensions = []
         if self.subjectAltName:
@@ -766,9 +770,12 @@ class CertificateSigningRequestCryptography(CertificateSigningRequestBase):
 
     def _generate_csr(self):
         csr = cryptography.x509.CertificateSigningRequestBuilder()
-        csr = csr.subject_name(cryptography.x509.Name([
-            cryptography.x509.NameAttribute(self._get_name_oid(entry[0]), to_text(entry[1])) for entry in self.subject
-        ]))
+        try:
+            csr = csr.subject_name(cryptography.x509.Name([
+                cryptography.x509.NameAttribute(self._get_name_oid(entry[0]), to_text(entry[1])) for entry in self.subject
+            ]))
+        except ValueError as e:
+            raise CertificateSigningRequestError(str(e))
 
         if self.subjectAltName:
             csr = csr.add_extension(cryptography.x509.SubjectAlternativeName([
diff --git a/test/integration/targets/openssl_csr/tasks/impl.yml b/test/integration/targets/openssl_csr/tasks/impl.yml
index 9c701a9a5b4..1d63076306d 100644
--- a/test/integration/targets/openssl_csr/tasks/impl.yml
+++ b/test/integration/targets/openssl_csr/tasks/impl.yml
@@ -206,3 +206,38 @@
       commonName: This is for Ansible
     useCommonNameForSAN: no
     select_crypto_backend: '{{ select_crypto_backend }}'
+
+- name: Generate CSR with country name
+  openssl_csr:
+    path: '{{ output_dir }}/csr4.csr'
+    privatekey_path: '{{ output_dir }}/privatekey2.pem'
+    country_name: de
+    select_crypto_backend: '{{ select_crypto_backend }}'
+  register: country_idempotent_1
+
+- name: Generate CSR with country name (idempotent)
+  openssl_csr:
+    path: '{{ output_dir }}/csr4.csr'
+    privatekey_path: '{{ output_dir }}/privatekey2.pem'
+    country_name: de
+    select_crypto_backend: '{{ select_crypto_backend }}'
+  register: country_idempotent_2
+
+- name: Generate CSR with country name (idempotent 2)
+  openssl_csr:
+    path: '{{ output_dir }}/csr4.csr'
+    privatekey_path: '{{ output_dir }}/privatekey2.pem'
+    subject:
+      C: de
+    select_crypto_backend: '{{ select_crypto_backend }}'
+  register: country_idempotent_3
+
+- name: Generate CSR with country name (bad country name)
+  openssl_csr:
+    path: '{{ output_dir }}/csr4.csr'
+    privatekey_path: '{{ output_dir }}/privatekey2.pem'
+    subject:
+      C: dex
+    select_crypto_backend: '{{ select_crypto_backend }}'
+  register: country_fail_4
+  ignore_errors: yes
diff --git a/test/integration/targets/openssl_csr/tests/validate.yml b/test/integration/targets/openssl_csr/tests/validate.yml
index ab2bf358d7d..e8e46aedaaf 100644
--- a/test/integration/targets/openssl_csr/tests/validate.yml
+++ b/test/integration/targets/openssl_csr/tests/validate.yml
@@ -101,3 +101,11 @@
   assert:
     that:
       - csr3_cn.stdout.split('=')[-1] == 'This is for Ansible'
+
+- name: Validate country name idempotency and validation
+  assert:
+    that:
+      - country_idempotent_1 is changed
+      - country_idempotent_2 is not changed
+      - country_idempotent_3 is not changed
+      - country_fail_4 is failed