From fc2fc81ed589fc16be21d79bf5cea9923acf1497 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ksawery=20Dzieko=C5=84ski?= Date: Wed, 6 Apr 2022 12:35:22 +0200 Subject: [PATCH] Validate X509 certificate inputs Related: LP#1772674 Change-Id: Ia6f44eb7aa0b37b0efa961d1ae48ac69c688b592 --- charmhelpers/contrib/hahelpers/apache.py | 17 ++++ charmhelpers/contrib/openstack/cert_utils.py | 99 ++++++++++++++++++-- charmhelpers/contrib/openstack/context.py | 21 +++-- 3 files changed, 123 insertions(+), 14 deletions(-) diff --git a/charmhelpers/contrib/hahelpers/apache.py b/charmhelpers/contrib/hahelpers/apache.py index a54702bc..aea18eca 100644 --- a/charmhelpers/contrib/hahelpers/apache.py +++ b/charmhelpers/contrib/hahelpers/apache.py @@ -24,6 +24,8 @@ import os +from base64 import b64decode + from charmhelpers.core import host from charmhelpers.core.hookenv import ( config as config_get, @@ -31,8 +33,13 @@ from charmhelpers.core.hookenv import ( relation_ids, related_units as relation_list, log, + ERROR, INFO, ) +from charmhelpers.contrib.openstack.cert_utils import ( + x509_get_pubkey, + x509_validate_cert, +) # This file contains the CA cert from the charms ssl_ca configuration # option, in future the file name should be updated reflect that. @@ -61,6 +68,11 @@ def get_cert(cn=None): if not key: key = relation_get(ssl_key_attr, rid=r_id, unit=unit) + + # this likely fails too quietly, raise? + if not x509_validate_cert(b64decode(cert), ssl_key=b64decode(key)): + return (None, None) + return (cert, key) @@ -75,6 +87,11 @@ def get_ca_cert(): if ca_cert is None: ca_cert = relation_get('ca_cert', rid=r_id, unit=unit) + + # this likely fails too quietly, raise? + if not x509_get_pubkey(b64decode(ca_cert)): + return None + return ca_cert diff --git a/charmhelpers/contrib/openstack/cert_utils.py b/charmhelpers/contrib/openstack/cert_utils.py index 5c961c58..29bb44a3 100644 --- a/charmhelpers/contrib/openstack/cert_utils.py +++ b/charmhelpers/contrib/openstack/cert_utils.py @@ -16,8 +16,12 @@ import os import json +import subprocess +import tempfile from base64 import b64decode +import six + from charmhelpers.contrib.network.ip import ( get_hostname, resolve_network_cidr, @@ -34,6 +38,7 @@ from charmhelpers.core.hookenv import ( log, WARNING, INFO, + ERROR, ) from charmhelpers.contrib.openstack.ip import ( resolve_address, @@ -53,9 +58,9 @@ from charmhelpers.core.host import ( write_file, ) -from charmhelpers.contrib.hahelpers.apache import ( - CONFIG_CA_CERT_FILE, -) +# This file contains the CA cert from the charms ssl_ca configuration +# option, in future the file name should be updated reflect that. +CONFIG_CA_CERT_FILE = 'keystone_juju_ca_cert' class CertRequest(object): @@ -337,7 +342,7 @@ def _manage_ca_certs(ca, cert_relation_id): """ config_ssl_ca = config('ssl_ca') config_cert_file = ca_cert_absolute_path(CONFIG_CA_CERT_FILE) - if config_ssl_ca: + if config_ssl_ca and x509_get_pubkey(b64decode(config_ssl_ca)): log("Installing CA certificate from charm ssl_ca config to {}".format( config_cert_file), INFO) install_ca_cert( @@ -346,10 +351,12 @@ def _manage_ca_certs(ca, cert_relation_id): elif os.path.exists(config_cert_file): log("Removing CA certificate {}".format(config_cert_file), INFO) os.remove(config_cert_file) - log("Installing CA certificate from certificate relation", INFO) - install_ca_cert( - ca.encode(), - name=get_cert_relation_ca_name(cert_relation_id)) + + if x509_get_pubkey(ca.encode()): + log("Installing CA certificate from certificate relation", INFO) + install_ca_cert( + ca.encode(), + name=get_cert_relation_ca_name(cert_relation_id)) def process_certificates(service_name, relation_id, unit, @@ -441,3 +448,79 @@ def get_bundle_for_cn(cn, relation_name=None): if cert_bundle: break return cert_bundle + + +def _x509_normalize_input(input_): + if isinstance(input_, six.text_type): + return input_.encode() + if isinstance(input_, bytes): + return input_ + return bytes() + + +def x509_validate_cert_chain(ssl_cert, ssl_ca=None): + cmd = ['openssl', 'verify'] + + with tempfile.TemporaryFile() as cert_fd, tempfile.NamedTemporaryFile() as ca_fd: + cert_fd.write(ssl_cert); cert_fd.seek(0) # noqa: E702 + if ssl_ca: + ca_fd.write(ssl_ca); ca_fd.seek(0) # noqa: E702 + cmd.extend(['-CAfile', ca_fd.name]) + + try: + subprocess.check_output(cmd, stdin=cert_fd) + return True + except subprocess.CalledProcessError as e: + log('Chain verification exited with code {} and output:\n{}'.format( + e.returncode, e.output.decode()), level=ERROR) + return False + raise Exception('Error during certificate chain validation') + + +def x509_get_pubkey(input_, private=False): + # since we don't want to leak private key material and generally wouldn't + # want to loiter on the filesystem more than it's necessary, we can't use + # `ssl.SSLContext.load_cert_chain` to confirm cert-key parity without + # dropping `tempfile.NamedTemporaryFile`s and passing their paths in + # + # issue with this approach is that any sort of exception condition may let + # these files linger, posing a leakage risk, so the best we can afford is + # `tempfile.TemporaryFile` which immediately removes filesystem-mappable + # paths, while keeping a file descriptor, sufficient for passing as stdin + # to subprocess calls + + input_ = _x509_normalize_input(input_) + if not input_: + return bytes() + + cmd = ['openssl'] + cmd += ['pkey', '-pubout'] if private else ['x509', '-pubkey', '-noout'] + cmd += ['-in', '/dev/stdin'] + + with tempfile.TemporaryFile() as fd: + fd.write(input_); fd.seek(0) # noqa: E702 + try: + return subprocess.check_output(cmd, stdin=fd).strip() + except subprocess.CalledProcessError as e: + log('Getting public key exited with code {} and output:\n{}'.format( + e.returncode, e.output.decode()), level=ERROR) + return bytes() + + +def x509_validate_cert_parity(ssl_cert, ssl_key): + priv = x509_get_pubkey(ssl_key, private=True) + pub = x509_get_pubkey(ssl_cert) + return all([priv, pub, priv == pub]) + + +def x509_validate_cert(ssl_cert, ssl_key=None, ssl_ca=None, validate_chain=False): + # normalize string input types + ssl_cert = _x509_normalize_input(ssl_cert) + ssl_key = _x509_normalize_input(ssl_key) + ssl_ca = _x509_normalize_input(ssl_ca) + + return all([ + x509_validate_cert_parity( + ssl_cert, ssl_key) if ssl_cert and ssl_key else True, + x509_validate_cert_chain(ssl_cert, ssl_ca) if validate_chain else True + ]) diff --git a/charmhelpers/contrib/openstack/context.py b/charmhelpers/contrib/openstack/context.py index 54081f0c..e8c5dc8a 100644 --- a/charmhelpers/contrib/openstack/context.py +++ b/charmhelpers/contrib/openstack/context.py @@ -89,6 +89,10 @@ from charmhelpers.contrib.hahelpers.apache import ( get_ca_cert, install_ca_cert, ) +from charmhelpers.contrib.openstack.cert_utils import ( + x509_get_pubkey, + x509_validate_cert, +) from charmhelpers.contrib.openstack.neutron import ( neutron_plugin_attribute, parse_data_port_mappings, @@ -326,17 +330,22 @@ class PostgresqlDBContext(OSContextGenerator): def db_ssl(rdata, ctxt, ssl_dir): - if 'ssl_ca' in rdata and ssl_dir: + ssl_ca = b64decode(rdata.get('ssl_ca', bytes())) + if 'ssl_ca' in rdata and x509_get_pubkey(ssl_ca) and ssl_dir: ca_path = os.path.join(ssl_dir, 'db-client.ca') with open(ca_path, 'wb') as fh: - fh.write(b64decode(rdata['ssl_ca'])) + fh.write(ssl_ca) ctxt['database_ssl_ca'] = ca_path elif 'ssl_ca' in rdata: log("Charm not setup for ssl support but ssl ca found", level=INFO) return ctxt - if 'ssl_cert' in rdata: + ssl_cert = b64decode(rdata.get('ssl_cert', bytes())) + ssl_key = b64decode(rdata.get('ssl_key', bytes())) + if 'ssl_cert' in rdata and x509_validate_cert( + ssl_cert, ssl_key, ssl_ca.decode() if ssl_ca else None + ): cert_path = os.path.join( ssl_dir, 'db-client.cert') if not os.path.exists(cert_path): @@ -344,12 +353,12 @@ def db_ssl(rdata, ctxt, ssl_dir): time.sleep(60) with open(cert_path, 'wb') as fh: - fh.write(b64decode(rdata['ssl_cert'])) + fh.write(ssl_cert) ctxt['database_ssl_cert'] = cert_path key_path = os.path.join(ssl_dir, 'db-client.key') with open(key_path, 'wb') as fh: - fh.write(b64decode(rdata['ssl_key'])) + fh.write(ssl_key) ctxt['database_ssl_key'] = key_path @@ -701,7 +710,7 @@ class AMQPContext(OSContextGenerator): rabbitmq_port = ssl_port ssl_ca = relation_get('ssl_ca', rid=rid, unit=unit) - if ssl_ca: + if ssl_ca and x509_get_pubkey(b64decode(ssl_ca)): ctxt['rabbit_ssl_ca'] = ssl_ca if relation_get('ha_queues', rid=rid, unit=unit) is not None: