push: Verify site CA on registration

metrics-cache previously accepted any server certificate (pure Trust On
First Use) when registering to the push-agent receiver. This meant the
one-time token was vulnerable to MITM who could register as the cluster.

The registration/bootstrap now verifies the receiver against a site CA
certificate supplied via push.siteCaCertificate before sending the
token.

This only affects the registration step; the receiver then returns the
site CA again which is stored for doing actual pushes.

There is an explicit opt-out of verification via
push.insecureSkipSiteCAVerification, which defaults to false but allows
for the dev environment to spin up without someone having to pass in a
certificate (only a token).

Author: relrod

.gitignore

@@ -17,4 +17,7 @@ target
 
 # rejected/unreviewed cargo-insta test snapshots
 *.snap.new
-*.pending-snap
+*.pending-snap
+
+# Custom dev env Helm overrides
+devel/custom_values.yaml

charts/cmk-rustik/templates/metrics-cache/deployment.yaml

@@ -38,6 +38,9 @@ spec:
             {{- end }}
             {{- if .Values.push.enabled }}
             - --push-receiver={{ required "push.url is required when push mode is enabled" .Values.push.url }}
+            {{- if .Values.push.insecureSkipSiteCaVerification }}
+            - --push-registration-insecure-skip-site-ca-verification
+            {{- end }}
             {{- end }}
             {{- range $k, $v := .Values.emitAll }}
             {{- if $v }}
@@ -55,6 +58,14 @@ spec:
                   name: {{ include "rustik.fullname" . }}-push-registration
                   key: token
                   optional: true
+            {{- if .Values.push.siteCaCertificate }}
+            - name: CMK_PUSH_AGENT_RECEIVER_SITE_CA_PEM
+              valueFrom:
+                secretKeyRef:
+                  name: {{ include "rustik.fullname" . }}-push-registration
+                  key: site-ca-pem
+                  optional: true
+            {{- end }}
             {{- end }}
             - name: NODE_NAME
               valueFrom:

charts/cmk-rustik/templates/metrics-cache/secret-push-one-time-token.yaml

@@ -5,4 +5,7 @@ metadata:
   name: {{ include "rustik.fullname" . }}-push-registration
 stringData:
   token: {{ .Values.push.registrationToken }}
+  {{- with .Values.push.siteCaCertificate }}
+  site-ca-pem: {{ . | quote }}
+  {{- end }}
 {{- end }}

charts/cmk-rustik/values.yaml

@@ -108,6 +108,21 @@ push:
   # url is the url of the push-agent receiver for your site. It *includes* the
   # site name. Example: https://example.com:8000/production
   url: ""
+  # siteCaCertificate is the PEM-encoded certificate of the Checkmk site CA.
+  # Used for registering to the Checkmk site's push-agent receiver service.
+  # It can be found in Setup > Certificate overview, with description
+  # "Signing the site certificate" and path ending "/ssl/ca.pem". If using helm
+  # directly from the command line, you can pass this in with:
+  # --set-file push.siteCaCertificate
+  siteCaCertificate: ""
+  # insecureSkipSiteCaVerification *should NOT* ever be set to `true` in
+  # production. It provides an escape hatch primarily for development of the
+  # agent itself or for environments that do not care about potential
+  # man-in-the-middle attacks (which could expose your one-time registration
+  # token to an attacker who could then register as your cluster). You have
+  # been warned. When set to true, will trust whatever push-agent receiver
+  # answers at the configured URL, without veryifying its identity.
+  insecureSkipSiteCaVerification: false
 
 # hostLabels defines how annotations get imported as labels in Checkmk. By
 # default annotations do not become host labels.

devel/values.yaml

@@ -36,3 +36,7 @@ metricsCache:
 push:
   enabled: false
   url: https://172.18.0.1:8000/heute
+  # In devel we default this to true so that it's easier to set up against a dev
+  # site. If you want to test validation, override this in custom_values.yaml
+  # or set the `just` variable site_ca to a site CA file path.
+  insecureSkipSiteCaVerification: true

justfile

@@ -7,6 +7,7 @@ cache_target := "metrics-cache-dev"
 push := ""
 push_ott := ""
 push_url := ""
+site_ca := ""
 cluster_host_name := ""
 
 # Build an image for Kubernetes using Docker
@@ -28,9 +29,15 @@ kind-load: dockerize kind-create
 kind-helm-install:
     helm upgrade --install cmk-rustik ./charts/cmk-rustik \
       -n checkmk-monitoring --create-namespace -f devel/values.yaml \
+      {{ if path_exists("devel/custom_values.yaml") == "true" { "-f devel/custom_values.yaml" } else { "" } }} \
       {{ if push != "" { "--set push.enabled=" + push } else { "" } }} \
       {{ if push_ott != "" { "--set push.registrationToken=" + push_ott } else { "" } }} \
       {{ if push_url != "" { "--set push.url=" + push_url } else { "" } }} \
+      {{ if site_ca != "" { \
+             "--set push.insecureSkipSiteCaVerification=false " + \
+             "--set-file push.siteCaCertificate=" + site_ca \
+         } else { "" } \
+      }} \
       {{ if cluster_host_name != "" { "--set clusterHostName=" + cluster_host_name } else { "" } }}
 
 # Delete the helm deployment from the kind cluster

metrics-cache/src/cli_args.rs

@@ -120,6 +120,20 @@ pub struct CliArgs {
     #[arg(long, env = "CMK_PUSH_AGENT_RECEIVER_OTT", hide_env_values = true)]
     pub push_registration_ott: Option<String>,
 
+    /// CA certificate (PEM) of the Checkmk site, used to verify we are about to
+    /// register with the correct push-agent receiver.
+    #[arg(
+        long,
+        env = "CMK_PUSH_AGENT_RECEIVER_SITE_CA_PEM",
+        hide_env_values = true
+    )]
+    pub push_registration_pem: Option<String>,
+
+    /// Avoid verifying the identity of the configured push-agent receiver
+    /// during initial registration. Do NOT use in production.
+    #[arg(long, default_value_t = false)]
+    pub push_registration_insecure_skip_site_ca_verification: bool,
+
     /// Excluded node role (infix) patterns for cluster-level aggregations,
     /// comma-separated
     #[arg(long = "excluded-node-role-patterns", value_delimiter = ',')]

metrics-cache/src/push/register.rs

@@ -28,7 +28,7 @@ use kube::api::Api;
 use rcgen::{CertificateParams, DistinguishedName, DnType, KeyPair};
 use serde::{Deserialize, Serialize};
 use std::collections::BTreeMap;
-use tracing::{error, info, trace};
+use tracing::{error, info, trace, warn};
 use uuid::Uuid;
 
 use crate::cli_args::CliArgs;
@@ -138,10 +138,64 @@ impl<'a> CheckmkPushRegistration<'a> {
             )
             .into());
         };
-        let client = reqwest::ClientBuilder::new()
-            .danger_accept_invalid_certs(true) // TOFU for register
-            .build()?;
 
+        // Check whether someone has activated the escape hatch.
+        let accept_invalid_certs = self
+            .cli_args
+            .push_registration_insecure_skip_site_ca_verification;
+
+        // Registration must never send the token over an unverified connection
+        // unless the user explicitly opted out, which they should never do in
+        // production.
+        let builder = match (accept_invalid_certs, &self.cli_args.push_registration_pem) {
+            (false, None) => {
+                // Case 1: Not accepting invalid certs, but not given a cert
+                // Intentionally do not reference the unsafe option.
+                return Err(push::Error::PushMode(
+                    "Push mode was enabled but the agent is not yet registered and \
+                     no Checkmk site CA certificate was provided. The agent needs \
+                     this to know that it is registering to the correct server and \
+                     to prevent man-in-the-middle attacks. If you are trying to \
+                     configure push mode, set the site CA certificate in your helm \
+                     push.siteCaCertificate (on the CLI, --set-file might prove \
+                     useful). The certificate can be downloaded from your Checkmk \
+                     instance under Setup > Certificate overview with description \
+                     \"Signing the site certificate\" and path ending \
+                     \"/ssl/ca.pem\"."
+                        .to_string(),
+                )
+                .into());
+            }
+            (true, Some(_)) => {
+                // Case 2: Accepting invalid certs, and also given a cert
+                return Err(push::Error::PushMode(
+                    "Push mode was enabled with the INSECURE option \
+                     push.insecureSkipSiteCaVerification in your helm values \
+                     but a Checkmk site CA certificate was also supplied with \
+                     push.siteCaCertificate. Exiting because we do not know \
+                     which configuration is intended."
+                        .to_string(),
+                )
+                .into());
+            }
+            (true, None) => {
+                // Case 3: Accepting invalid certs
+                warn!(
+                    "INSECURE option push.insecureSkipSiteCaVerification is enabled, not \
+                     validating push-agent receiver identity while registering. This \
+                     configuration is NOT RECOMMENDED in production."
+                );
+                reqwest::ClientBuilder::new().danger_accept_invalid_certs(true)
+            }
+            (false, Some(pem)) => {
+                // Case 4: Pinning the cert
+                reqwest::ClientBuilder::new()
+                    .tls_certs_only([reqwest::Certificate::from_pem(pem.as_bytes())?])
+                    .danger_accept_invalid_hostnames(true)
+            }
+        };
+
+        let client = builder.build()?;
         let response = client
             .post(&url)
             .header("Authorization", format!("CMK-TOKEN {}", ott))