From a2f36f426a5654a5e9fb5d1eadb1356be52651af Mon Sep 17 00:00:00 2001 From: Felix Fontein Date: Tue, 8 Sep 2020 06:24:30 +0200 Subject: [PATCH] openssl_csr: catch errors on bad SANs (#106) * Catch errors on bad SANs. * Add changelog fragment. * Adjust cryptography version and error message. --- .../fragments/106-openssl_csr-idna-errors.yml | 2 ++ plugins/modules/openssl_csr.py | 18 ++++++++++++++++++ .../targets/openssl_csr/tasks/impl.yml | 11 ++++++++++- .../targets/openssl_csr/tests/validate.yml | 11 ++++++++++- 4 files changed, 40 insertions(+), 2 deletions(-) create mode 100644 changelogs/fragments/106-openssl_csr-idna-errors.yml diff --git a/changelogs/fragments/106-openssl_csr-idna-errors.yml b/changelogs/fragments/106-openssl_csr-idna-errors.yml new file mode 100644 index 00000000..f522eca9 --- /dev/null +++ b/changelogs/fragments/106-openssl_csr-idna-errors.yml @@ -0,0 +1,2 @@ +bugfixes: +- "openssl_csr - improve handling of IDNA errors (https://github.com/ansible-collections/community.crypto/issues/105)." diff --git a/plugins/modules/openssl_csr.py b/plugins/modules/openssl_csr.py index 5560db99..dc16b3d8 100644 --- a/plugins/modules/openssl_csr.py +++ b/plugins/modules/openssl_csr.py @@ -605,10 +605,12 @@ class CertificateSigningRequestBase(OpenSSLObject): self.subject = self.subject + parse_name_field(module.params['subject']) self.subject = [(entry[0], entry[1]) for entry in self.subject if entry[1]] + self.using_common_name_for_san = False if not self.subjectAltName and module.params['use_common_name_for_san']: for sub in self.subject: if sub[0] in ('commonName', 'CN'): self.subjectAltName = ['DNS:%s' % sub[1]] + self.using_common_name_for_san = True break if self.subject_key_identifier is not None: @@ -962,6 +964,22 @@ class CertificateSigningRequestCryptography(CertificateSigningRequestBase): if str(e) == 'Algorithm must be a registered hash algorithm.' and digest is None: self.module.fail_json(msg='Signing with Ed25519 and Ed448 keys requires cryptography 2.8 or newer.') raise + except UnicodeError as e: + # This catches IDNAErrors, which happens when a bad name is passed as a SAN + # (https://github.com/ansible-collections/community.crypto/issues/105). + # For older cryptography versions, this is handled by idna, which raises + # an idna.core.IDNAError. Later versions of cryptography deprecated and stopped + # requiring idna, whence we cannot easily handle this error. Fortunately, in + # most versions of idna, IDNAError extends UnicodeError. There is only version + # 2.3 where it extends Exception instead (see + # https://github.com/kjd/idna/commit/ebefacd3134d0f5da4745878620a6a1cba86d130 + # and then + # https://github.com/kjd/idna/commit/ea03c7b5db7d2a99af082e0239da2b68aeea702a). + msg = 'Error while creating CSR: {0}\n'.format(e) + if self.using_common_name_for_san: + self.module.fail_json(msg=msg + 'This is probably caused because the Common Name is used as a SAN.' + ' Specifying use_common_name_for_san=false might fix this.') + self.module.fail_json(msg=msg + 'This is probably caused by an invalid Subject Alternative DNS Name.') return self.request.public_bytes(cryptography.hazmat.primitives.serialization.Encoding.PEM) diff --git a/tests/integration/targets/openssl_csr/tasks/impl.yml b/tests/integration/targets/openssl_csr/tasks/impl.yml index 6a12f9e9..61b8d5e2 100644 --- a/tests/integration/targets/openssl_csr/tasks/impl.yml +++ b/tests/integration/targets/openssl_csr/tasks/impl.yml @@ -162,7 +162,7 @@ commonName: www.ansible.com select_crypto_backend: '{{ select_crypto_backend }}' -- name: Generate CSR with invalid SAN +- name: Generate CSR with invalid SAN (1/2) openssl_csr: path: '{{ output_dir }}/csrinvsan.csr' privatekey_path: '{{ output_dir }}/privatekey.pem' @@ -171,6 +171,15 @@ register: generate_csr_invalid_san ignore_errors: yes +- name: Generate CSR with invalid SAN (2/2) + openssl_csr: + path: '{{ output_dir }}/csrinvsan2.csr' + privatekey_path: '{{ output_dir }}/privatekey.pem' + subject_alt_name: "DNS:system:kube-controller-manager" + select_crypto_backend: '{{ select_crypto_backend }}' + register: generate_csr_invalid_san_2 + ignore_errors: yes + - name: Generate CSR with OCSP Must Staple openssl_csr: path: '{{ output_dir }}/csr_ocsp.csr' diff --git a/tests/integration/targets/openssl_csr/tests/validate.yml b/tests/integration/targets/openssl_csr/tests/validate.yml index bfbb9d87..2936ca9a 100644 --- a/tests/integration/targets/openssl_csr/tests/validate.yml +++ b/tests/integration/targets/openssl_csr/tests/validate.yml @@ -62,12 +62,21 @@ - csr_oldapi_cn.stdout.split('=')[-1] == 'www.ansible.com' - csr_oldapi_modulus.stdout == privatekey_modulus.stdout -- name: Validate invalid SAN +- name: Validate invalid SAN (1/2) assert: that: - generate_csr_invalid_san is failed - "'Subject Alternative Name' in generate_csr_invalid_san.msg" +- name: Validate invalid SAN (2/2) + # Note that pyOpenSSL simply accepts this name, and modern cryptography versions do so as well. + # The error has been observed with cryptography 1.7.2 and 1.9, but not with 2.3 and newer. + assert: + that: + - generate_csr_invalid_san_2 is failed + - "'The label system:kube-controller-manager is not a valid A-label' in generate_csr_invalid_san_2.msg" + when: select_crypto_backend == 'cryptography' and cryptography_version.stdout is version('2.0', '<') + - name: Validate OCSP Must Staple CSR (test - everything) shell: "openssl req -noout -in {{ output_dir }}/csr_ocsp.csr -text" register: csr_ocsp