From d4ce3e999dfb1174029ecd6739971ae0bbf6beed Mon Sep 17 00:00:00 2001 From: Noctua Date: Fri, 29 Sep 2023 07:20:15 +0200 Subject: [PATCH] chore: update charm libraries (#213) Co-authored-by: Github Actions --- .../grafana_k8s/v0/grafana_dashboard.py | 3 +- .../observability_libs/v0/cert_handler.py | 27 ++++- .../prometheus_k8s/v0/prometheus_scrape.py | 20 +--- .../v1/prometheus_remote_write.py | 4 +- .../v2/tls_certificates.py | 111 ++++++++++++++---- 5 files changed, 118 insertions(+), 47 deletions(-) diff --git a/lib/charms/grafana_k8s/v0/grafana_dashboard.py b/lib/charms/grafana_k8s/v0/grafana_dashboard.py index de8715bf..1f1bc4f0 100644 --- a/lib/charms/grafana_k8s/v0/grafana_dashboard.py +++ b/lib/charms/grafana_k8s/v0/grafana_dashboard.py @@ -219,7 +219,7 @@ def __init__(self, *args): # Increment this PATCH version before using `charmcraft publish-lib` or reset # to 0 if you are raising the major API version -LIBPATCH = 34 +LIBPATCH = 35 logger = logging.getLogger(__name__) @@ -1195,6 +1195,7 @@ def _on_grafana_dashboard_relation_created(self, event: RelationCreatedEvent) -> `grafana_dashboaard` relationship is joined """ if self._charm.unit.is_leader(): + self._update_all_dashboards_from_dir() self._upset_dashboards_on_relation(event.relation) def _on_grafana_dashboard_relation_changed(self, event: RelationChangedEvent) -> None: diff --git a/lib/charms/observability_libs/v0/cert_handler.py b/lib/charms/observability_libs/v0/cert_handler.py index 15087be5..88a8374e 100644 --- a/lib/charms/observability_libs/v0/cert_handler.py +++ b/lib/charms/observability_libs/v0/cert_handler.py @@ -33,8 +33,10 @@ This library requires a peer relation to be declared in the requirer's metadata. Peer relation data is used for "persistent storage" of the private key and certs. """ +import ipaddress import json import socket +from itertools import filterfalse from typing import List, Optional, Union try: @@ -62,7 +64,16 @@ LIBID = "b5cd5cd580f3428fa5f59a8876dcbe6a" LIBAPI = 0 -LIBPATCH = 7 +LIBPATCH = 8 + + +def is_ip_address(value: str) -> bool: + """Return True if the input value is a valid IPv4 address; False otherwise.""" + try: + ipaddress.IPv4Address(value) + return True + except ipaddress.AddressValueError: + return False class CertChanged(EventBase): @@ -88,7 +99,7 @@ def __init__( peer_relation_name: str, certificates_relation_name: str = "certificates", cert_subject: Optional[str] = None, - extra_sans_dns: Optional[List[str]] = None, + extra_sans_dns: Optional[List[str]] = None, # TODO: in v1, rename arg to `sans` ): """CertHandler is used to wrap TLS Certificates management operations for charms. @@ -111,7 +122,9 @@ def __init__( self.cert_subject = charm.unit.name.replace("/", "-") if not cert_subject else cert_subject # Use fqdn only if no SANs were given, and drop empty/duplicate SANs - self.sans_dns = list(set(filter(None, (extra_sans_dns or [socket.getfqdn()])))) + sans = list(set(filter(None, (extra_sans_dns or [socket.getfqdn()])))) + self.sans_ip = list(filter(is_ip_address, sans)) + self.sans_dns = list(filterfalse(is_ip_address, sans)) self.peer_relation_name = peer_relation_name self.certificates_relation_name = certificates_relation_name @@ -229,6 +242,7 @@ def _generate_csr( private_key=private_key.encode(), subject=self.cert_subject, sans_dns=self.sans_dns, + sans_ip=self.sans_ip, ) if renew and self._csr: @@ -237,7 +251,12 @@ def _generate_csr( new_certificate_signing_request=csr, ) else: - logger.info("Creating CSR for %s with DNS %s", self.cert_subject, self.sans_dns) + logger.info( + "Creating CSR for %s with DNS %s and IPs %s", + self.cert_subject, + self.sans_dns, + self.sans_ip, + ) self.certificates.request_certificate_creation(certificate_signing_request=csr) # Note: CSR is being replaced with a new one, so until we get the new cert, we'd have diff --git a/lib/charms/prometheus_k8s/v0/prometheus_scrape.py b/lib/charms/prometheus_k8s/v0/prometheus_scrape.py index 4ea89a1c..e4297aa1 100644 --- a/lib/charms/prometheus_k8s/v0/prometheus_scrape.py +++ b/lib/charms/prometheus_k8s/v0/prometheus_scrape.py @@ -362,7 +362,7 @@ def _on_scrape_targets_changed(self, event): # Increment this PATCH version before using `charmcraft publish-lib` or reset # to 0 if you are raising the major API version -LIBPATCH = 41 +LIBPATCH = 42 PYDEPS = ["cosl"] @@ -604,12 +604,12 @@ def render_alertmanager_static_configs(alertmanagers: List[str]): return { "alertmanagers": [ { + # For https we still do not render a `tls_config` section because + # certs are expected to be made available by the charm via the + # `update-ca-certificates` mechanism. "scheme": scheme, "path_prefix": path_prefix, "static_configs": [{"targets": netlocs}], - # FIXME figure out how to get alertmanager's ca_file into here - # Without this, prom errors: "x509: certificate signed by unknown authority" - "tls_config": {"insecure_skip_verify": True}, } for (scheme, path_prefix), netlocs in paths.items() ] @@ -1176,16 +1176,8 @@ def _static_scrape_config(self, relation) -> list: scrape_configs, hosts, topology ) - # If scheme is https but no ca section present, then auto add "insecure_skip_verify", - # otherwise scraping errors out with "x509: certificate signed by unknown authority". - # https://prometheus.io/docs/prometheus/latest/configuration/configuration/#tls_config - for scrape_config in scrape_configs: - tls_config = scrape_config.get("tls_config", {}) - ca_present = "ca" in tls_config or "ca_file" in tls_config - if scrape_config.get("scheme") == "https" and not ca_present: - tls_config["insecure_skip_verify"] = True - scrape_config["tls_config"] = tls_config - + # For https scrape targets we still do not render a `tls_config` section because certs + # are expected to be made available by the charm via the `update-ca-certificates` mechanism. return scrape_configs def _relation_hosts(self, relation: Relation) -> Dict[str, Tuple[str, str]]: diff --git a/lib/charms/prometheus_k8s/v1/prometheus_remote_write.py b/lib/charms/prometheus_k8s/v1/prometheus_remote_write.py index de1be49f..89658165 100644 --- a/lib/charms/prometheus_k8s/v1/prometheus_remote_write.py +++ b/lib/charms/prometheus_k8s/v1/prometheus_remote_write.py @@ -46,7 +46,7 @@ # Increment this PATCH version before using `charmcraft publish-lib` or reset # to 0 if you are raising the major API version -LIBPATCH = 1 +LIBPATCH = 2 PYDEPS = ["cosl"] @@ -838,7 +838,7 @@ def _inject_alert_expr_labels(self, rules: Dict[str, Any]) -> Dict[str, Any]: # Inject topology and put it back in the list rule["expr"] = self._tool.inject_label_matchers( re.sub(r"%%juju_topology%%,?", "", rule["expr"]), - topology.label_matcher_dict, + topology.alert_expression_dict, ) except KeyError: # Some required JujuTopology key is missing. Just move on. diff --git a/lib/charms/tls_certificates_interface/v2/tls_certificates.py b/lib/charms/tls_certificates_interface/v2/tls_certificates.py index fc2f450b..f4a08366 100644 --- a/lib/charms/tls_certificates_interface/v2/tls_certificates.py +++ b/lib/charms/tls_certificates_interface/v2/tls_certificates.py @@ -276,7 +276,6 @@ def _on_all_certificates_invalidated(self, event: AllCertificatesInvalidatedEven import json import logging import uuid -from collections import defaultdict from contextlib import suppress from datetime import datetime, timedelta from ipaddress import IPv4Address @@ -299,7 +298,7 @@ def _on_all_certificates_invalidated(self, event: AllCertificatesInvalidatedEven ) from ops.framework import EventBase, EventSource, Handle, Object from ops.jujuversion import JujuVersion -from ops.model import SecretNotFoundError +from ops.model import Relation, SecretNotFoundError # The unique Charmhub library identifier, never change it LIBID = "afd8c2bccf834997afce12c2706d2ede" @@ -309,7 +308,7 @@ def _on_all_certificates_invalidated(self, event: AllCertificatesInvalidatedEven # Increment this PATCH version before using `charmcraft publish-lib` or reset # to 0 if you are raising the major API version -LIBPATCH = 10 +LIBPATCH = 16 PYDEPS = ["cryptography", "jsonschema"] @@ -641,6 +640,17 @@ def generate_ca( private_key_object.public_key() # type: ignore[arg-type] ) subject_identifier = key_identifier = subject_identifier_object.public_bytes() + key_usage = x509.KeyUsage( + digital_signature=True, + key_encipherment=True, + key_cert_sign=True, + key_agreement=False, + content_commitment=False, + data_encipherment=False, + crl_sign=False, + encipher_only=False, + decipher_only=False, + ) cert = ( x509.CertificateBuilder() .subject_name(subject) @@ -658,6 +668,7 @@ def generate_ca( ), critical=False, ) + .add_extension(key_usage, critical=True) .add_extension( x509.BasicConstraints(ca=True, path_length=None), critical=True, @@ -690,7 +701,8 @@ def generate_certificate( """ csr_object = x509.load_pem_x509_csr(csr) subject = csr_object.subject - issuer = x509.load_pem_x509_certificate(ca).issuer + ca_pem = x509.load_pem_x509_certificate(ca) + issuer = ca_pem.issuer private_key = serialization.load_pem_private_key(ca_key, password=ca_key_password) certificate_builder = ( @@ -701,6 +713,20 @@ def generate_certificate( .serial_number(x509.random_serial_number()) .not_valid_before(datetime.utcnow()) .not_valid_after(datetime.utcnow() + timedelta(days=validity)) + .add_extension( + x509.AuthorityKeyIdentifier( + key_identifier=ca_pem.extensions.get_extension_for_class( + x509.SubjectKeyIdentifier + ).value.key_identifier, + authority_cert_issuer=None, + authority_cert_serial_number=None, + ), + critical=False, + ) + .add_extension( + x509.SubjectKeyIdentifier.from_public_key(csr_object.public_key()), critical=False + ) + .add_extension(x509.BasicConstraints(ca=False, path_length=None), critical=False) ) extensions_list = csr_object.extensions @@ -731,6 +757,7 @@ def generate_certificate( extension.value, critical=extension.critical, ) + certificate_builder._version = x509.Version.v3 cert = certificate_builder.sign(private_key, hashes.SHA256()) # type: ignore[arg-type] return cert.public_bytes(serialization.Encoding.PEM) @@ -828,7 +855,7 @@ def generate_csr( sans_oid (list): List of registered ID SANs sans_dns (list): List of DNS subject alternative names (similar to the arg: sans) sans_ip (list): List of IP subject alternative names - additional_critical_extensions (list): List if critical additional extension objects. + additional_critical_extensions (list): List of critical additional extension objects. Object must be a x509 ExtensionType. Returns: @@ -898,6 +925,22 @@ def __init__(self, charm: CharmBase, relationship_name: str): self.charm = charm self.relationship_name = relationship_name + def _load_app_relation_data(self, relation: Relation) -> dict: + """Loads relation data from the application relation data bag. + + Json loads all data. + + Args: + relation_object: Relation data from the application databag + + Returns: + dict: Relation data in dict format. + """ + # If unit is not leader, it does not try to reach relation data. + if not self.model.unit.is_leader(): + return {} + return _load_relation_data(relation.data[self.charm.app]) + def _add_certificate( self, relation_id: int, @@ -932,7 +975,7 @@ def _add_certificate( "ca": ca, "chain": chain, } - provider_relation_data = _load_relation_data(relation.data[self.charm.app]) + provider_relation_data = self._load_app_relation_data(relation) provider_certificates = provider_relation_data.get("certificates", []) certificates = copy.deepcopy(provider_certificates) if new_certificate in certificates: @@ -965,7 +1008,7 @@ def _remove_certificate( raise RuntimeError( f"Relation {self.relationship_name} with relation id {relation_id} does not exist" ) - provider_relation_data = _load_relation_data(relation.data[self.charm.app]) + provider_relation_data = self._load_app_relation_data(relation) provider_certificates = provider_relation_data.get("certificates", []) certificates = copy.deepcopy(provider_certificates) for certificate_dict in certificates: @@ -1000,7 +1043,7 @@ def revoke_all_certificates(self) -> None: This method is meant to be used when the Root CA has changed. """ for relation in self.model.relations[self.relationship_name]: - provider_relation_data = _load_relation_data(relation.data[self.charm.app]) + provider_relation_data = self._load_app_relation_data(relation) provider_certificates = copy.deepcopy(provider_relation_data.get("certificates", [])) for certificate in provider_certificates: certificate["revoked"] = True @@ -1062,7 +1105,7 @@ def remove_certificate(self, certificate: str) -> None: def get_issued_certificates( self, relation_id: Optional[int] = None - ) -> Dict[str, Dict[str, str]]: + ) -> Dict[str, List[Dict[str, str]]]: """Returns a dictionary of issued certificates. It returns certificates from all relations if relation_id is not specified. @@ -1071,7 +1114,7 @@ def get_issued_certificates( Returns: dict: Certificates per application name. """ - certificates: Dict[str, Dict[str, str]] = defaultdict(dict) + certificates: Dict[str, List[Dict[str, str]]] = {} relations = ( [ relation @@ -1082,13 +1125,19 @@ def get_issued_certificates( else self.model.relations.get(self.relationship_name, []) ) for relation in relations: - provider_relation_data = _load_relation_data(relation.data[self.charm.app]) + provider_relation_data = self._load_app_relation_data(relation) provider_certificates = provider_relation_data.get("certificates", []) + + certificates[relation.app.name] = [] # type: ignore[union-attr] for certificate in provider_certificates: if not certificate.get("revoked", False): - certificates[relation.app.name].update( # type: ignore[union-attr] - {certificate["certificate_signing_request"]: certificate["certificate"]} + certificates[relation.app.name].append( # type: ignore[union-attr] + { + "csr": certificate["certificate_signing_request"], + "certificate": certificate["certificate"], + } ) + return certificates def _on_relation_changed(self, event: RelationChangedEvent) -> None: @@ -1106,9 +1155,13 @@ def _on_relation_changed(self, event: RelationChangedEvent) -> None: Returns: None """ - assert event.unit is not None + if event.unit is None: + logger.error("Relation_changed event does not have a unit.") + return + if not self.model.unit.is_leader(): + return requirer_relation_data = _load_relation_data(event.relation.data[event.unit]) - provider_relation_data = _load_relation_data(event.relation.data[self.charm.app]) + provider_relation_data = self._load_app_relation_data(event.relation) if not self._relation_data_is_valid(requirer_relation_data): logger.debug("Relation data did not pass JSON Schema validation") return @@ -1147,7 +1200,7 @@ def _revoke_certificates_for_which_no_csr_exists(self, relation_id: int) -> None ) if not certificates_relation: raise RuntimeError(f"Relation {self.relationship_name} does not exist") - provider_relation_data = _load_relation_data(certificates_relation.data[self.charm.app]) + provider_relation_data = self._load_app_relation_data(certificates_relation) list_of_csrs: List[str] = [] for unit in certificates_relation.units: requirer_relation_data = _load_relation_data(certificates_relation.data[unit]) @@ -1165,27 +1218,33 @@ def _revoke_certificates_for_which_no_csr_exists(self, relation_id: int) -> None self.remove_certificate(certificate=certificate["certificate"]) def get_requirer_csrs_with_no_certs( - self, + self, relation_id: Optional[int] = None ) -> List[Dict[str, Union[int, str, List[Dict[str, str]]]]]: """Filters the requirer's units csrs. Keeps the ones for which no certificate was provided. + Args: + relation_id (int): Relation id + Returns: list: List of dictionaries that contain the unit's csrs that don't have a certificate issued. """ - all_unit_csr_mappings = copy.deepcopy(self.get_requirer_csrs()) + all_unit_csr_mappings = copy.deepcopy(self.get_requirer_csrs(relation_id=relation_id)) + filtered_all_unit_csr_mappings: List[Dict[str, Union[int, str, List[Dict[str, str]]]]] = [] for unit_csr_mapping in all_unit_csr_mappings: + csrs_without_certs = [] for csr in unit_csr_mapping["unit_csrs"]: # type: ignore[union-attr] - if self.certificate_issued_for_csr( + if not self.certificate_issued_for_csr( app_name=unit_csr_mapping["application_name"], # type: ignore[arg-type] csr=csr["certificate_signing_request"], # type: ignore[index] ): - unit_csr_mapping["unit_csrs"].remove(csr) # type: ignore[union-attr, arg-type] - if len(unit_csr_mapping["unit_csrs"]) == 0: # type: ignore[arg-type] - all_unit_csr_mappings.remove(unit_csr_mapping) - return all_unit_csr_mappings + csrs_without_certs.append(csr) + if csrs_without_certs: + unit_csr_mapping["unit_csrs"] = csrs_without_certs # type: ignore[assignment] + filtered_all_unit_csr_mappings.append(unit_csr_mapping) + return filtered_all_unit_csr_mappings def get_requirer_csrs( self, relation_id: Optional[int] = None @@ -1237,9 +1296,9 @@ def certificate_issued_for_csr(self, app_name: str, csr: str) -> bool: bool: True/False depending on whether a certificate has been issued for the given CSR. """ issued_certificates_per_csr = self.get_issued_certificates()[app_name] - for request, cert in issued_certificates_per_csr.items(): - if request == csr: - return csr_matches_certificate(csr, cert) + for issued_pair in issued_certificates_per_csr: + if "csr" in issued_pair and issued_pair["csr"] == csr: + return csr_matches_certificate(csr, issued_pair["certificate"]) return False