From cb5c57bcd5d0fa0cb21ba7523baef1880e7ce24b Mon Sep 17 00:00:00 2001 From: Felix Fontein Date: Mon, 15 Apr 2019 09:15:08 +0200 Subject: [PATCH] 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. --- .../55142-openssl_csr-crypto-backend-san.yml | 3 + lib/ansible/modules/crypto/openssl_csr.py | 23 ++- .../modules/crypto/openssl_csr_info.py | 2 + .../targets/openssl_csr/tasks/impl.yml | 181 ++++++++++++++++++ .../targets/openssl_csr/tests/validate.yml | 7 + 5 files changed, 209 insertions(+), 7 deletions(-) create mode 100644 changelogs/fragments/55142-openssl_csr-crypto-backend-san.yml 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 00000000000..707e0cbef58 --- /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 6584d636f07..ed9022a8296 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 8edb165e0e8..c1c4ee237b0 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 f5af14cfa77..43393111c3c 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 585cf64c0a2..43e3b3336b1 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