From a5bf71ac6adeaf36e905950ab54c1c390419baea Mon Sep 17 00:00:00 2001 From: Felix Fontein Date: Thu, 3 Jan 2019 12:34:24 +0100 Subject: [PATCH] openssl_csr: idempotency doesn't work correctly for keyUsage (#50361) * Fix key usage idempotency bug. * Extend tests. * Add changelog. --- .../50361-openssl_csr-idempotency.yml | 2 + lib/ansible/modules/crypto/openssl_csr.py | 13 ++++- .../targets/openssl_csr/tasks/main.yml | 58 ++++++++++++++++++- .../targets/openssl_csr/tests/validate.yml | 12 +++- 4 files changed, 82 insertions(+), 3 deletions(-) create mode 100644 changelogs/fragments/50361-openssl_csr-idempotency.yml diff --git a/changelogs/fragments/50361-openssl_csr-idempotency.yml b/changelogs/fragments/50361-openssl_csr-idempotency.yml new file mode 100644 index 00000000000..e5c59cbb4f9 --- /dev/null +++ b/changelogs/fragments/50361-openssl_csr-idempotency.yml @@ -0,0 +1,2 @@ +bugfixes: +- "openssl_csr - fix problem with idempotency of keyUsage option." diff --git a/lib/ansible/modules/crypto/openssl_csr.py b/lib/ansible/modules/crypto/openssl_csr.py index 77631ce16ab..23bdbe340f8 100644 --- a/lib/ansible/modules/crypto/openssl_csr.py +++ b/lib/ansible/modules/crypto/openssl_csr.py @@ -456,7 +456,18 @@ class CertificateSigningRequest(crypto_utils.OpenSSLObject): return set(current) == set(expected) and usages_ext[0].get_critical() == critical def _check_keyUsage(extensions): - return _check_keyUsage_(extensions, b'keyUsage', self.keyUsage, self.keyUsage_critical) + usages_ext = [ext for ext in extensions if ext.get_short_name() == b'keyUsage'] + if (not usages_ext and self.keyUsage) or (usages_ext and not self.keyUsage): + return False + elif not usages_ext and not self.keyUsage: + return True + else: + # OpenSSL._util.lib.OBJ_txt2nid() always returns 0 for all keyUsage values + # (since keyUsage has a fixed bitfield for these values and is not extensible). + # Therefore, we create an extension for the wanted values, and compare the + # data of the extensions (which is the serialized bitfield). + expected_ext = crypto.X509Extension(b"keyUsage", False, ', '.join(self.keyUsage).encode('ascii')) + return usages_ext[0].get_data() == expected_ext.get_data() and usages_ext[0].get_critical() == self.keyUsage_critical def _check_extenededKeyUsage(extensions): return _check_keyUsage_(extensions, b'extendedKeyUsage', self.extendedKeyUsage, self.extendedKeyUsage_critical) diff --git a/test/integration/targets/openssl_csr/tasks/main.yml b/test/integration/targets/openssl_csr/tasks/main.yml index 2347ab1eb8d..2c7806dd190 100644 --- a/test/integration/targets/openssl_csr/tasks/main.yml +++ b/test/integration/targets/openssl_csr/tasks/main.yml @@ -4,12 +4,39 @@ openssl_privatekey: path: '{{ output_dir }}/privatekey.pem' + - name: Generate CSR (check mode) + openssl_csr: + path: '{{ output_dir }}/csr.csr' + privatekey_path: '{{ output_dir }}/privatekey.pem' + subject: + commonName: www.ansible.com + check_mode: yes + register: generate_csr_check + - name: Generate CSR openssl_csr: path: '{{ output_dir }}/csr.csr' privatekey_path: '{{ output_dir }}/privatekey.pem' subject: commonName: www.ansible.com + register: generate_csr + + - name: Generate CSR (idempotent) + openssl_csr: + path: '{{ output_dir }}/csr.csr' + privatekey_path: '{{ output_dir }}/privatekey.pem' + subject: + commonName: www.ansible.com + register: generate_csr_check_idempotent + + - name: Generate CSR (idempotent, check mode) + openssl_csr: + path: '{{ output_dir }}/csr.csr' + privatekey_path: '{{ output_dir }}/privatekey.pem' + subject: + commonName: www.ansible.com + check_mode: yes + register: generate_csr_check_idempotent_check # keyUsage longname and shortname should be able to be used # interchangeably. Hence the long name is specified here @@ -37,8 +64,8 @@ subject: commonName: 'www.ansible.com' keyUsage: + - Key Agreement - digitalSignature - - keyAgreement extendedKeyUsage: - ipsecUser - qcStatements @@ -46,6 +73,35 @@ - Biometric Info register: csr_ku_xku + - name: Generate CSR with KU and XKU (test XKU change) + openssl_csr: + path: '{{ output_dir }}/csr_ku_xku.csr' + privatekey_path: '{{ output_dir }}/privatekey.pem' + subject: + commonName: 'www.ansible.com' + keyUsage: + - digitalSignature + - keyAgreement + extendedKeyUsage: + - ipsecUser + - qcStatements + - Biometric Info + register: csr_ku_xku_change + + - name: Generate CSR with KU and XKU (test KU change) + openssl_csr: + path: '{{ output_dir }}/csr_ku_xku.csr' + privatekey_path: '{{ output_dir }}/privatekey.pem' + subject: + commonName: 'www.ansible.com' + keyUsage: + - digitalSignature + extendedKeyUsage: + - ipsecUser + - qcStatements + - Biometric Info + register: csr_ku_xku_change_2 + - name: Generate CSR with old API openssl_csr: path: '{{ output_dir }}/csr_oldapi.csr' diff --git a/test/integration/targets/openssl_csr/tests/validate.yml b/test/integration/targets/openssl_csr/tests/validate.yml index dc77b0ff6c1..23e3a5b2129 100644 --- a/test/integration/targets/openssl_csr/tests/validate.yml +++ b/test/integration/targets/openssl_csr/tests/validate.yml @@ -17,10 +17,20 @@ - csr_cn.stdout.split('=')[-1] == 'www.ansible.com' - csr_modulus.stdout == privatekey_modulus.stdout -- name: Validate CSR_KU_XKU (assert idempotency) +- name: Validate CSR (check mode, idempotency) + assert: + that: + - generate_csr_check is changed + - generate_csr is changed + - generate_csr_check_idempotent is not changed + - generate_csr_check_idempotent_check is not changed + +- name: Validate CSR_KU_XKU (assert idempotency, change) assert: that: - csr_ku_xku is not changed + - csr_ku_xku_change is changed + - csr_ku_xku_change_2 is changed - name: Validate old_API CSR (test - Common Name) shell: "openssl req -noout -subject -in {{ output_dir }}/csr_oldapi.csr -nameopt oneline,-space_eq"