From 60c6d87b05602bf110b4c9b2d00cbae8ce607245 Mon Sep 17 00:00:00 2001 From: Felix Fontein Date: Sat, 19 Feb 2022 18:51:28 +0100 Subject: [PATCH] [stable-1] x509_certificate: regenerate certificate on CA's subject change (#406) * Regenerate certificate on CA's subject change. (#402) (cherry picked from commit 3ebc132c03cda63270c377974f4e013eb7a563d4) * Add fix for PyOpenSSL backend. * x509_certificate: check existing certificate's signature for selfsigned and ownca provider (#407) * Verify whether signature matches. * Add changelog fragment. * Forgot imports. * Fix wrong name. * Check whether the CA private key fits to the CA certificate. Use correct key in tests. * Refactor code. (cherry picked from commit 28729657ac077e06a75b9ac7ba1668712ce3ba1c) * There doesn't seem a way to do this with pyOpenSSL. --- .../402-x509_certificate-ownca-subject.yml | 2 + .../407-x509_certificate-signature.yml | 8 ++ .../crypto/cryptography_support.py | 65 +++++++++++++ .../module_backends/certificate_ownca.py | 28 +++++- .../module_backends/certificate_selfsigned.py | 13 +++ .../targets/x509_certificate/tasks/ownca.yml | 95 +++++++++++++++---- .../x509_certificate/tests/validate_ownca.yml | 8 ++ 7 files changed, 202 insertions(+), 17 deletions(-) create mode 100644 changelogs/fragments/402-x509_certificate-ownca-subject.yml create mode 100644 changelogs/fragments/407-x509_certificate-signature.yml diff --git a/changelogs/fragments/402-x509_certificate-ownca-subject.yml b/changelogs/fragments/402-x509_certificate-ownca-subject.yml new file mode 100644 index 00000000..8a07e261 --- /dev/null +++ b/changelogs/fragments/402-x509_certificate-ownca-subject.yml @@ -0,0 +1,2 @@ +bugfixes: + - "x509_certificate - regenerate certificate when the CA's subject changes for ``provider=ownca`` (https://github.com/ansible-collections/community.crypto/issues/400, https://github.com/ansible-collections/community.crypto/pull/402)." diff --git a/changelogs/fragments/407-x509_certificate-signature.yml b/changelogs/fragments/407-x509_certificate-signature.yml new file mode 100644 index 00000000..72aed287 --- /dev/null +++ b/changelogs/fragments/407-x509_certificate-signature.yml @@ -0,0 +1,8 @@ +bugfixes: + - "x509_certificate - for the ``ownca`` provider, check whether the CA private key actually belongs to the CA certificate. This fix only covers the ``cryptography`` backend, not the ``pyopenssl`` backend (https://github.com/ansible-collections/community.crypto/pull/407)." + - "x509_certificate - regenerate certificate when the CA's public key changes for ``provider=ownca``. This fix only covers the ``cryptography`` backend, not the ``pyopenssl`` backend (https://github.com/ansible-collections/community.crypto/pull/407)." + - "x509_certificate - regenerate certificate when the private key changes for ``provider=selfsigned``. This fix only covers the ``cryptography`` backend, not the ``pyopenssl`` backend (https://github.com/ansible-collections/community.crypto/pull/407)." +known_issues: + - "x509_certificate - when using the ``ownca`` provider with the ``pyopenssl`` backend, it is possible to specify a CA private key which is not related to the CA certificate (https://github.com/ansible-collections/community.crypto/pull/407)." + - "x509_certificate - when using the ``ownca`` provider with the ``pyopenssl`` backend, changing the CA's public key does not cause regeneration of the certificate (https://github.com/ansible-collections/community.crypto/pull/407)." + - "x509_certificate - when using the ``selfsigned`` provider with the ``pyopenssl`` backend, changing the private key does not cause regeneration of the certificate (https://github.com/ansible-collections/community.crypto/pull/407)." diff --git a/plugins/module_utils/crypto/cryptography_support.py b/plugins/module_utils/crypto/cryptography_support.py index 004d5f2f..2fb2da77 100644 --- a/plugins/module_utils/crypto/cryptography_support.py +++ b/plugins/module_utils/crypto/cryptography_support.py @@ -31,13 +31,36 @@ from ansible_collections.community.crypto.plugins.module_utils.version import Lo try: import cryptography from cryptography import x509 + from cryptography.exceptions import InvalidSignature from cryptography.hazmat.backends import default_backend from cryptography.hazmat.primitives import serialization + from cryptography.hazmat.primitives.asymmetric import padding import ipaddress except ImportError: # Error handled in the calling module. pass +try: + import cryptography.hazmat.primitives.asymmetric.rsa +except ImportError: + pass +try: + import cryptography.hazmat.primitives.asymmetric.ec +except ImportError: + pass +try: + import cryptography.hazmat.primitives.asymmetric.dsa +except ImportError: + pass +try: + import cryptography.hazmat.primitives.asymmetric.ed25519 +except ImportError: + pass +try: + import cryptography.hazmat.primitives.asymmetric.ed448 +except ImportError: + pass + try: # This is a separate try/except since this is only present in cryptography 2.5 or newer from cryptography.hazmat.primitives.serialization.pkcs12 import ( @@ -57,8 +80,13 @@ except ImportError: _load_pkcs12 = None from .basic import ( + CRYPTOGRAPHY_HAS_DSA_SIGN, + CRYPTOGRAPHY_HAS_EC_SIGN, CRYPTOGRAPHY_HAS_ED25519, + CRYPTOGRAPHY_HAS_ED25519_SIGN, CRYPTOGRAPHY_HAS_ED448, + CRYPTOGRAPHY_HAS_ED448_SIGN, + CRYPTOGRAPHY_HAS_RSA_SIGN, OpenSSLObjectError, ) @@ -579,3 +607,40 @@ def _parse_pkcs12_legacy(pkcs12_bytes, passphrase=None): if maybe_name != backend._ffi.NULL: friendly_name = backend._ffi.string(maybe_name) return private_key, certificate, additional_certificates, friendly_name + + +def cryptography_verify_signature(signature, data, hash_algorithm, signer_public_key): + ''' + Check whether the given signature of the given data was signed by the given public key object. + ''' + try: + if CRYPTOGRAPHY_HAS_RSA_SIGN and isinstance(signer_public_key, cryptography.hazmat.primitives.asymmetric.rsa.RSAPublicKey): + signer_public_key.verify(signature, data, padding.PKCS1v15(), hash_algorithm) + return True + if CRYPTOGRAPHY_HAS_EC_SIGN and isinstance(signer_public_key, cryptography.hazmat.primitives.asymmetric.ec.EllipticCurvePublicKey): + signer_public_key.verify(signature, data, cryptography.hazmat.primitives.asymmetric.ec.ECDSA(hash_algorithm)) + return True + if CRYPTOGRAPHY_HAS_DSA_SIGN and isinstance(signer_public_key, cryptography.hazmat.primitives.asymmetric.dsa.DSAPublicKey): + signer_public_key.verify(signature, data, hash_algorithm) + return True + if CRYPTOGRAPHY_HAS_ED25519_SIGN and isinstance(signer_public_key, cryptography.hazmat.primitives.asymmetric.ed25519.Ed25519PublicKey): + signer_public_key.verify(signature, data) + return True + if CRYPTOGRAPHY_HAS_ED448_SIGN and isinstance(signer_public_key, cryptography.hazmat.primitives.asymmetric.ed448.Ed448PublicKey): + signer_public_key.verify(signature, data) + return True + raise OpenSSLObjectError(u'Unsupported public key type {0}'.format(type(signer_public_key))) + except InvalidSignature: + return False + + +def cryptography_verify_certificate_signature(certificate, signer_public_key): + ''' + Check whether the given X509 certificate object was signed by the given public key object. + ''' + return cryptography_verify_signature( + certificate.signature, + certificate.tbs_certificate_bytes, + certificate.signature_hash_algorithm, + signer_public_key + ) diff --git a/plugins/module_utils/crypto/module_backends/certificate_ownca.py b/plugins/module_utils/crypto/module_backends/certificate_ownca.py index c0a44ea8..f348e00f 100644 --- a/plugins/module_utils/crypto/module_backends/certificate_ownca.py +++ b/plugins/module_utils/crypto/module_backends/certificate_ownca.py @@ -28,8 +28,10 @@ from ansible_collections.community.crypto.plugins.module_utils.crypto.support im ) from ansible_collections.community.crypto.plugins.module_utils.crypto.cryptography_support import ( + cryptography_compare_public_keys, cryptography_key_needs_digest_for_signing, cryptography_serial_number_of_cert, + cryptography_verify_certificate_signature, ) from ansible_collections.community.crypto.plugins.module_utils.crypto.module_backends.certificate import ( @@ -107,6 +109,9 @@ class OwnCACertificateBackendCryptography(CertificateBackend): except OpenSSLBadPassphraseError as exc: module.fail_json(msg=str(exc)) + if not cryptography_compare_public_keys(self.ca_cert.public_key(), self.ca_private_key.public_key()): + raise CertificateError('The CA private key does not belong to the CA certificate') + if cryptography_key_needs_digest_for_signing(self.ca_private_key): if self.digest is None: raise CertificateError( @@ -173,6 +178,16 @@ class OwnCACertificateBackendCryptography(CertificateBackend): if super(OwnCACertificateBackendCryptography, self).needs_regeneration(): return True + self._ensure_existing_certificate_loaded() + + # Check whether certificate is signed by CA certificate + if not cryptography_verify_certificate_signature(self.existing_certificate, self.ca_cert.public_key()): + return True + + # Check subject + if self.ca_cert.subject != self.existing_certificate.issuer: + return True + # Check AuthorityKeyIdentifier if self.create_authority_key_identifier: try: @@ -185,7 +200,6 @@ class OwnCACertificateBackendCryptography(CertificateBackend): except cryptography.x509.ExtensionNotFound: expected_ext = x509.AuthorityKeyIdentifier.from_issuer_public_key(self.ca_cert.public_key()) - self._ensure_existing_certificate_loaded() try: ext = self.existing_certificate.extensions.get_extension_for_class(x509.AuthorityKeyIdentifier) if ext.value != expected_ext: @@ -297,6 +311,18 @@ class OwnCACertificateBackendPyOpenSSL(CertificateBackend): """Return bytes for self.cert.""" return crypto.dump_certificate(crypto.FILETYPE_PEM, self.cert) + def needs_regeneration(self): + if super(OwnCACertificateBackendPyOpenSSL, self).needs_regeneration(): + return True + + self._ensure_existing_certificate_loaded() + + # Check subject + if self.ca_cert.get_subject() != self.existing_certificate.get_issuer(): + return True + + return False + def dump(self, include_certificate): result = super(OwnCACertificateBackendPyOpenSSL, self).dump(include_certificate) result.update({ diff --git a/plugins/module_utils/crypto/module_backends/certificate_selfsigned.py b/plugins/module_utils/crypto/module_backends/certificate_selfsigned.py index fb348fe3..141a6f74 100644 --- a/plugins/module_utils/crypto/module_backends/certificate_selfsigned.py +++ b/plugins/module_utils/crypto/module_backends/certificate_selfsigned.py @@ -22,6 +22,7 @@ from ansible_collections.community.crypto.plugins.module_utils.crypto.support im from ansible_collections.community.crypto.plugins.module_utils.crypto.cryptography_support import ( cryptography_key_needs_digest_for_signing, cryptography_serial_number_of_cert, + cryptography_verify_certificate_signature, ) from ansible_collections.community.crypto.plugins.module_utils.crypto.module_backends.certificate import ( @@ -134,6 +135,18 @@ class SelfSignedCertificateBackendCryptography(CertificateBackend): """Return bytes for self.cert.""" return self.cert.public_bytes(Encoding.PEM) + def needs_regeneration(self): + if super(SelfSignedCertificateBackendCryptography, self).needs_regeneration(): + return True + + self._ensure_existing_certificate_loaded() + + # Check whether certificate is signed by private key + if not cryptography_verify_certificate_signature(self.existing_certificate, self.privatekey.public_key()): + return True + + return False + def dump(self, include_certificate): result = super(SelfSignedCertificateBackendCryptography, self).dump(include_certificate) diff --git a/tests/integration/targets/x509_certificate/tasks/ownca.yml b/tests/integration/targets/x509_certificate/tasks/ownca.yml index 5776425e..1bed03c2 100644 --- a/tests/integration/targets/x509_certificate/tasks/ownca.yml +++ b/tests/integration/targets/x509_certificate/tasks/ownca.yml @@ -14,14 +14,20 @@ - name: (OwnCA, {{select_crypto_backend}}) Generate CA CSR openssl_csr: - path: '{{ remote_tmp_dir }}/ca_csr.csr' + path: '{{ item.path }}' privatekey_path: '{{ remote_tmp_dir }}/ca_privatekey.pem' - subject: - commonName: Example CA + subject: '{{ item.subject }}' useCommonNameForSAN: no basic_constraints: - 'CA:TRUE' basic_constraints_critical: yes + loop: + - path: '{{ remote_tmp_dir }}/ca_csr.csr' + subject: + commonName: Example CA + - path: '{{ remote_tmp_dir }}/ca_csr2.csr' + subject: + commonName: Example CA 2 - name: (OwnCA, {{select_crypto_backend}}) Generate CA CSR (privatekey passphrase) openssl_csr: @@ -62,6 +68,15 @@ - result_check_mode is changed - result is changed +- name: (OwnCA, {{select_crypto_backend}}) Generate selfsigned CA certificate with different commonName + x509_certificate: + path: '{{ remote_tmp_dir }}/ca_cert2.pem' + csr_path: '{{ remote_tmp_dir }}/ca_csr2.csr' + privatekey_path: '{{ remote_tmp_dir }}/ca_privatekey.pem' + provider: selfsigned + selfsigned_digest: sha256 + select_crypto_backend: '{{ select_crypto_backend }}' + - name: (OwnCA, {{select_crypto_backend}}) Generate selfsigned CA certificate (privatekey passphrase) x509_certificate: path: '{{ remote_tmp_dir }}/ca_cert_pw.pem' @@ -110,6 +125,54 @@ select_crypto_backend: '{{ select_crypto_backend }}' check_mode: yes +- name: (OwnCA, {{select_crypto_backend}}) Copy ownca certificate to new file to check regeneration + copy: + src: '{{ remote_tmp_dir }}/ownca_cert.pem' + dest: '{{ item }}' + remote_src: true + loop: + - '{{ remote_tmp_dir }}/ownca_cert_ca_cn.pem' + - '{{ remote_tmp_dir }}/ownca_cert_ca_key.pem' + +- name: (OwnCA, {{select_crypto_backend}}) Regenerate ownca certificate with different CA subject + x509_certificate: + path: '{{ remote_tmp_dir }}/ownca_cert_ca_cn.pem' + csr_path: '{{ remote_tmp_dir }}/csr.csr' + privatekey_path: '{{ remote_tmp_dir }}/privatekey.pem' + ownca_path: '{{ remote_tmp_dir }}/ca_cert2.pem' + ownca_privatekey_path: '{{ remote_tmp_dir }}/ca_privatekey.pem' + provider: ownca + ownca_digest: sha256 + select_crypto_backend: '{{ select_crypto_backend }}' + return_content: yes + register: ownca_certificate_ca_subject_changed + +- name: (OwnCA, {{select_crypto_backend}}) Regenerate ownca certificate with different CA key + x509_certificate: + path: '{{ remote_tmp_dir }}/ownca_cert_ca_key.pem' + csr_path: '{{ remote_tmp_dir }}/csr.csr' + privatekey_path: '{{ remote_tmp_dir }}/privatekey.pem' + ownca_path: '{{ remote_tmp_dir }}/ca_cert_pw.pem' + ownca_privatekey_path: '{{ remote_tmp_dir }}/ca_privatekey_pw.pem' + ownca_privatekey_passphrase: hunter2 + provider: ownca + ownca_digest: sha256 + select_crypto_backend: '{{ select_crypto_backend }}' + return_content: yes + register: ownca_certificate_ca_key_changed + +- name: (OwnCA, {{select_crypto_backend}}) Get certificate information + community.crypto.x509_certificate_info: + path: '{{ remote_tmp_dir }}/ownca_cert.pem' + select_crypto_backend: '{{ select_crypto_backend }}' + register: result + +- name: (OwnCA, {{select_crypto_backend}}) Get private key information + community.crypto.openssl_privatekey_info: + path: '{{ remote_tmp_dir }}/privatekey.pem' + select_crypto_backend: '{{ select_crypto_backend }}' + register: result_privatekey + - name: (OwnCA, {{select_crypto_backend}}) Check ownca certificate x509_certificate: path: '{{ remote_tmp_dir }}/ownca_cert.pem' @@ -285,7 +348,7 @@ path: '{{ remote_tmp_dir }}/ownca_cert_backup.pem' csr_path: '{{ remote_tmp_dir }}/csr_ecc.csr' ownca_path: '{{ remote_tmp_dir }}/ca_cert.pem' - ownca_privatekey_path: '{{ remote_tmp_dir }}/privatekey.pem' + ownca_privatekey_path: '{{ remote_tmp_dir }}/ca_privatekey.pem' provider: ownca ownca_digest: sha256 backup: yes @@ -296,7 +359,7 @@ path: '{{ remote_tmp_dir }}/ownca_cert_backup.pem' csr_path: '{{ remote_tmp_dir }}/csr_ecc.csr' ownca_path: '{{ remote_tmp_dir }}/ca_cert.pem' - ownca_privatekey_path: '{{ remote_tmp_dir }}/privatekey.pem' + ownca_privatekey_path: '{{ remote_tmp_dir }}/ca_privatekey.pem' provider: ownca ownca_digest: sha256 backup: yes @@ -307,7 +370,7 @@ path: '{{ remote_tmp_dir }}/ownca_cert_backup.pem' csr_path: '{{ remote_tmp_dir }}/csr.csr' ownca_path: '{{ remote_tmp_dir }}/ca_cert.pem' - ownca_privatekey_path: '{{ remote_tmp_dir }}/privatekey.pem' + ownca_privatekey_path: '{{ remote_tmp_dir }}/ca_privatekey.pem' provider: ownca ownca_digest: sha256 backup: yes @@ -335,7 +398,7 @@ path: '{{ remote_tmp_dir }}/ownca_cert_ski.pem' csr_path: '{{ remote_tmp_dir }}/csr_ecc.csr' ownca_path: '{{ remote_tmp_dir }}/ca_cert.pem' - ownca_privatekey_path: '{{ remote_tmp_dir }}/privatekey.pem' + ownca_privatekey_path: '{{ remote_tmp_dir }}/ca_privatekey.pem' provider: ownca ownca_digest: sha256 ownca_create_subject_key_identifier: always_create @@ -348,7 +411,7 @@ path: '{{ remote_tmp_dir }}/ownca_cert_ski.pem' csr_path: '{{ remote_tmp_dir }}/csr_ecc.csr' ownca_path: '{{ remote_tmp_dir }}/ca_cert.pem' - ownca_privatekey_path: '{{ remote_tmp_dir }}/privatekey.pem' + ownca_privatekey_path: '{{ remote_tmp_dir }}/ca_privatekey.pem' provider: ownca ownca_digest: sha256 ownca_create_subject_key_identifier: always_create @@ -361,7 +424,7 @@ path: '{{ remote_tmp_dir }}/ownca_cert_ski.pem' csr_path: '{{ remote_tmp_dir }}/csr_ecc.csr' ownca_path: '{{ remote_tmp_dir }}/ca_cert.pem' - ownca_privatekey_path: '{{ remote_tmp_dir }}/privatekey.pem' + ownca_privatekey_path: '{{ remote_tmp_dir }}/ca_privatekey.pem' provider: ownca ownca_digest: sha256 ownca_create_subject_key_identifier: never_create @@ -374,7 +437,7 @@ path: '{{ remote_tmp_dir }}/ownca_cert_ski.pem' csr_path: '{{ remote_tmp_dir }}/csr_ecc.csr' ownca_path: '{{ remote_tmp_dir }}/ca_cert.pem' - ownca_privatekey_path: '{{ remote_tmp_dir }}/privatekey.pem' + ownca_privatekey_path: '{{ remote_tmp_dir }}/ca_privatekey.pem' provider: ownca ownca_digest: sha256 ownca_create_subject_key_identifier: never_create @@ -387,7 +450,7 @@ path: '{{ remote_tmp_dir }}/ownca_cert_ski.pem' csr_path: '{{ remote_tmp_dir }}/csr_ecc.csr' ownca_path: '{{ remote_tmp_dir }}/ca_cert.pem' - ownca_privatekey_path: '{{ remote_tmp_dir }}/privatekey.pem' + ownca_privatekey_path: '{{ remote_tmp_dir }}/ca_privatekey.pem' provider: ownca ownca_digest: sha256 ownca_create_subject_key_identifier: always_create @@ -400,7 +463,7 @@ path: '{{ remote_tmp_dir }}/ownca_cert_aki.pem' csr_path: '{{ remote_tmp_dir }}/csr_ecc.csr' ownca_path: '{{ remote_tmp_dir }}/ca_cert.pem' - ownca_privatekey_path: '{{ remote_tmp_dir }}/privatekey.pem' + ownca_privatekey_path: '{{ remote_tmp_dir }}/ca_privatekey.pem' provider: ownca ownca_digest: sha256 ownca_create_authority_key_identifier: yes @@ -413,7 +476,7 @@ path: '{{ remote_tmp_dir }}/ownca_cert_aki.pem' csr_path: '{{ remote_tmp_dir }}/csr_ecc.csr' ownca_path: '{{ remote_tmp_dir }}/ca_cert.pem' - ownca_privatekey_path: '{{ remote_tmp_dir }}/privatekey.pem' + ownca_privatekey_path: '{{ remote_tmp_dir }}/ca_privatekey.pem' provider: ownca ownca_digest: sha256 ownca_create_authority_key_identifier: yes @@ -426,7 +489,7 @@ path: '{{ remote_tmp_dir }}/ownca_cert_aki.pem' csr_path: '{{ remote_tmp_dir }}/csr_ecc.csr' ownca_path: '{{ remote_tmp_dir }}/ca_cert.pem' - ownca_privatekey_path: '{{ remote_tmp_dir }}/privatekey.pem' + ownca_privatekey_path: '{{ remote_tmp_dir }}/ca_privatekey.pem' provider: ownca ownca_digest: sha256 ownca_create_authority_key_identifier: no @@ -439,7 +502,7 @@ path: '{{ remote_tmp_dir }}/ownca_cert_aki.pem' csr_path: '{{ remote_tmp_dir }}/csr_ecc.csr' ownca_path: '{{ remote_tmp_dir }}/ca_cert.pem' - ownca_privatekey_path: '{{ remote_tmp_dir }}/privatekey.pem' + ownca_privatekey_path: '{{ remote_tmp_dir }}/ca_privatekey.pem' provider: ownca ownca_digest: sha256 ownca_create_authority_key_identifier: no @@ -452,7 +515,7 @@ path: '{{ remote_tmp_dir }}/ownca_cert_aki.pem' csr_path: '{{ remote_tmp_dir }}/csr_ecc.csr' ownca_path: '{{ remote_tmp_dir }}/ca_cert.pem' - ownca_privatekey_path: '{{ remote_tmp_dir }}/privatekey.pem' + ownca_privatekey_path: '{{ remote_tmp_dir }}/ca_privatekey.pem' provider: ownca ownca_digest: sha256 ownca_create_authority_key_identifier: yes diff --git a/tests/integration/targets/x509_certificate/tests/validate_ownca.yml b/tests/integration/targets/x509_certificate/tests/validate_ownca.yml index 6cd3e069..f4aeb96d 100644 --- a/tests/integration/targets/x509_certificate/tests/validate_ownca.yml +++ b/tests/integration/targets/x509_certificate/tests/validate_ownca.yml @@ -31,6 +31,14 @@ - ownca_certificate.notBefore == ownca_certificate_idempotence.notBefore - ownca_certificate.notAfter == ownca_certificate_idempotence.notAfter +- name: (OwnCA validation, {{select_crypto_backend}}) Validate ownca certificate regeneration + assert: + that: + - ownca_certificate_ca_subject_changed is changed + # ownca_certificate_ca_key_changed is not changed for the pyopenssl backend, + # see https://github.com/ansible-collections/community.crypto/pull/406 + - ownca_certificate_ca_key_changed is changed or select_crypto_backend == 'pyopenssl' + - name: (OwnCA validation, {{select_crypto_backend}}) Read certificate slurp: src: '{{ remote_tmp_dir }}/ownca_cert.pem'