From 0648e339a708cdd2e34a820b57cffaa528c52295 Mon Sep 17 00:00:00 2001 From: Yanis Guenane Date: Thu, 14 Sep 2017 18:03:00 +0200 Subject: [PATCH] openssl: remove static dict for keyUsage (#30339) keyUsage and extendedKeyUsage are currently statically limited via a static dict defined in modules_utils/crypto.py. If one specify a value that isn't in there, idempotency won't work. Instead of having static dict, we uses keyUsage and extendedKyeUsage values OpenSSL NID and compare those rather than comparing strings. Fixes: https://github.com/ansible/ansible/issues/30316 --- lib/ansible/module_utils/crypto.py | 28 ---------------- .../modules/crypto/openssl_certificate.py | 14 ++++---- lib/ansible/modules/crypto/openssl_csr.py | 13 ++++---- .../openssl_certificate/tasks/main.yml | 10 ++++++ .../targets/openssl_csr/tasks/main.yml | 33 +++++++++++++++++++ .../targets/openssl_csr/tests/validate.yml | 5 +++ 6 files changed, 63 insertions(+), 40 deletions(-) diff --git a/lib/ansible/module_utils/crypto.py b/lib/ansible/module_utils/crypto.py index d2724196c8..e83711fa5c 100644 --- a/lib/ansible/module_utils/crypto.py +++ b/lib/ansible/module_utils/crypto.py @@ -96,34 +96,6 @@ def load_certificate_request(path): raise OpenSSLObjectError(exc) -keyUsageLong = { - "digitalSignature": "Digital Signature", - "nonRepudiation": "Non Repudiation", - "keyEncipherment": "Key Encipherment", - "dataEncipherment": "Data Encipherment", - "keyAgreement": "Key Agreement", - "keyCertSign": "Certificate Sign", - "cRLSign": "CRL Sign", - "encipherOnly": "Encipher Only", - "decipherOnly": "Decipher Only", -} - -extendedKeyUsageLong = { - "anyExtendedKeyUsage": "Any Extended Key Usage", - "ipsecEndSystem": "IPSec End System", - "ipsecTunnel": "IPSec Tunnel", - "ipsecUser": "IPSec User", - "msSGC": "Microsoft Server Gated Crypto", - "nsSGC": "Netscape Server Gated Crypto", - "serverAuth": "TLS Web Server Authentication", - "clientAuth": "TLS Web Client Authentication", - "codeSigning": "Code Signing", - "emailProtection": "E-mail Protection", - "timeStamping": "Time Stamping", - "OCSPSigning": "OCSP Signing", -} - - @six.add_metaclass(abc.ABCMeta) class OpenSSLObject(object): diff --git a/lib/ansible/modules/crypto/openssl_certificate.py b/lib/ansible/modules/crypto/openssl_certificate.py index 6cf5b619cc..92a34e0514 100644 --- a/lib/ansible/modules/crypto/openssl_certificate.py +++ b/lib/ansible/modules/crypto/openssl_certificate.py @@ -538,9 +538,10 @@ class AssertOnlyCertificate(Certificate): for extension_idx in range(0, self.cert.get_extension_count()): extension = self.cert.get_extension(extension_idx) if extension.get_short_name() == 'keyUsage': - keyUsage = [crypto_utils.keyUsageLong.get(keyUsage, keyUsage) for keyUsage in self.keyUsage] - if (not self.keyUsage_strict and not all(x in str(extension).split(', ') for x in keyUsage)) or \ - (self.keyUsage_strict and not set(keyUsage) == set(str(extension).split(', '))): + keyUsage = [OpenSSL._util.lib.OBJ_txt2nid(keyUsage) for keyUsage in self.keyUsage] + current_ku = [OpenSSL._util.lib.OBJ_txt2nid(usage.strip()) for usage in str(extension).split(',')] + if (not self.keyUsage_strict and not all(x in current_ku for x in keyUsage)) or \ + (self.keyUsage_strict and not set(keyUsage) == set(current_ku)): self.message.append( 'Invalid keyUsage component (got %s, expected all of %s to be present)' % (str(extension).split(', '), keyUsage) ) @@ -550,9 +551,10 @@ class AssertOnlyCertificate(Certificate): for extension_idx in range(0, self.cert.get_extension_count()): extension = self.cert.get_extension(extension_idx) if extension.get_short_name() == 'extendedKeyUsage': - extKeyUsage = [crypto_utils.extendedKeyUsageLong.get(keyUsage, keyUsage) for keyUsage in self.extendedKeyUsage] - if (not self.extendedKeyUsage_strict and not all(x in str(extension).split(', ') for x in extKeyUsage)) or \ - (self.extendedKeyUsage_strict and not set(extKeyUsage) == set(str(extension).split(', '))): + extKeyUsage = [OpenSSL._util.lib.OBJ_txt2nid(keyUsage) for keyUsage in self.extendedKeyUsage] + current_xku = [OpenSSL._util.lib.OBJ_txt2nid(usage.strip()) for usage in str(extension).split(',')] + if (not self.extendedKeyUsage_strict and not all(x in current_xku for x in extKeyUsage)) or \ + (self.extendedKeyUsage_strict and not set(extKeyUsage) == set(current_xku)): self.message.append( 'Invalid extendedKeyUsage component (got %s, expected all of %s to be present)' % (str(extension).split(', '), extKeyUsage) ) diff --git a/lib/ansible/modules/crypto/openssl_csr.py b/lib/ansible/modules/crypto/openssl_csr.py index f54974807e..45494647e4 100644 --- a/lib/ansible/modules/crypto/openssl_csr.py +++ b/lib/ansible/modules/crypto/openssl_csr.py @@ -227,9 +227,10 @@ import os from ansible.module_utils import crypto as crypto_utils from ansible.module_utils.basic import AnsibleModule -from ansible.module_utils._text import to_native +from ansible.module_utils._text import to_native, to_bytes try: + import OpenSSL from OpenSSL import crypto except ImportError: pyopenssl_found = False @@ -348,22 +349,22 @@ class CertificateSigningRequest(crypto_utils.OpenSSLObject): return True - def _check_keyUsage_(extensions, extName, expected, critical, long): + def _check_keyUsage_(extensions, extName, expected, critical): usages_ext = [ext for ext in extensions if ext.get_short_name() == extName] if (not usages_ext and expected) or (usages_ext and not expected): return False elif not usages_ext and not expected: return True else: - current = [usage.strip() for usage in str(usages_ext[0]).split(',')] - expected = [long[usage] if usage in long else usage for usage in expected] + current = [OpenSSL._util.lib.OBJ_txt2nid(to_bytes(usage.strip())) for usage in str(usages_ext[0]).split(',')] + expected = [OpenSSL._util.lib.OBJ_txt2nid(to_bytes(usage)) for usage in expected] 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, crypto_utils.keyUsageLong) + return _check_keyUsage_(extensions, b'keyUsage', self.keyUsage, self.keyUsage_critical) def _check_extenededKeyUsage(extensions): - return _check_keyUsage_(extensions, b'extendedKeyUsage', self.extendedKeyUsage, self.extendedKeyUsage_critical, crypto_utils.extendedKeyUsageLong) + return _check_keyUsage_(extensions, b'extendedKeyUsage', self.extendedKeyUsage, self.extendedKeyUsage_critical) def _check_extensions(csr): extensions = csr.get_extensions() diff --git a/test/integration/targets/openssl_certificate/tasks/main.yml b/test/integration/targets/openssl_certificate/tasks/main.yml index 81c2b0f262..f35c90654d 100644 --- a/test/integration/targets/openssl_certificate/tasks/main.yml +++ b/test/integration/targets/openssl_certificate/tasks/main.yml @@ -51,6 +51,11 @@ path: '{{ output_dir }}/csr2.csr' privatekey_path: '{{ output_dir }}/privatekey2.pem' CN: 'www.example.com' + keyUsage: + - digitalSignature + extendedKeyUsage: + - ipsecUser + - biometricInfo - name: Generate selfsigned certificate2 openssl_certificate: @@ -77,6 +82,11 @@ L: Los Angeles O: ACME Inc. OU: Roadrunner pest control + keyUsage: + - digitalSignature + extendedKeyUsage: + - ipsecUser + - biometricInfo - import_tasks: ../tests/validate.yml diff --git a/test/integration/targets/openssl_csr/tasks/main.yml b/test/integration/targets/openssl_csr/tasks/main.yml index e06eadf393..4405d2b2de 100644 --- a/test/integration/targets/openssl_csr/tasks/main.yml +++ b/test/integration/targets/openssl_csr/tasks/main.yml @@ -9,6 +9,39 @@ privatekey_path: '{{ output_dir }}/privatekey.pem' commonName: 'www.ansible.com' + # 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 + # and vice-versa for biometricInfo + - name: Generate CSR with KU and XKU + openssl_csr: + path: '{{ output_dir }}/csr_ku_xku.csr' + privatekey_path: '{{ output_dir }}/privatekey.pem' + commonName: 'www.ansible.com' + keyUsage: + - digitalSignature + - keyAgreement + extendedKeyUsage: + - qcStatements + - DVCS + - IPSec User + - biometricInfo + + - name: Generate CSR with KU and XKU (test idempotency) + openssl_csr: + path: '{{ output_dir }}/csr_ku_xku.csr' + privatekey_path: '{{ output_dir }}/privatekey.pem' + commonName: 'www.ansible.com' + keyUsage: + - digitalSignature + - keyAgreement + extendedKeyUsage: + - ipsecUser + - qcStatements + - DVCS + - Biometric Info + register: csr_ku_xku + - import_tasks: ../tests/validate.yml when: pyopenssl_version.stdout|version_compare('0.15', '>=') diff --git a/test/integration/targets/openssl_csr/tests/validate.yml b/test/integration/targets/openssl_csr/tests/validate.yml index b24f61a2bb..0e41b6c7e8 100644 --- a/test/integration/targets/openssl_csr/tests/validate.yml +++ b/test/integration/targets/openssl_csr/tests/validate.yml @@ -15,3 +15,8 @@ that: - csr_cn.stdout.split('=')[-1] == 'www.ansible.com' - csr_modulus.stdout == privatekey_modulus.stdout + +- name: Validate CSR_KU_XKU (assert idempotency) + assert: + that: + - csr_ku_xku.changed == False