Cap node-auth JWT certificate chains

crates/api-core/src/node_auth.rs

@@ -70,6 +70,8 @@ enum RejectReason {
     Algorithm(Algorithm),
     #[error("no x5c certificate chain in the JWT header")]
     NoChain,
+    #[error("x5c chain has {actual} certificates; maximum is {maximum}")]
+    ChainTooLong { actual: usize, maximum: usize },
     #[error("x5c chain did not verify against the trusted roots: {0}")]
     Chain(rustls::Error),
     #[error("leaf certificate could not be parsed as X.509")]
@@ -135,23 +137,9 @@ impl NodeJwtValidator {
         })
     }
 
-    /// Re-reads the root CA bundle from disk and returns a verifier built from
-    /// it, without installing it.
+    /// Test-only convenience that reads the root CA bundle from disk.
     ///
-    /// The TLS listener reloads the same file every five minutes to pick up
-    /// cert-manager rotations; without this the validator would keep its
-    /// startup snapshot and start rejecting tokens whose `x5c` chains to the
-    /// new CA — which, with `mtls_enabled = false`, locks nodes out until the
-    /// API restarts.
-    ///
-    /// The fallible read is split from [`install_roots`](Self::install_roots)
-    /// so the listener can build this and its new TLS acceptor before
-    /// committing either. Both derive from this same bundle, and the listener
-    /// must never end up trusting one generation of it on the TLS path and
-    /// another on the token path — nor drop the acceptor and serve plaintext
-    /// while bearer auth stays armed. A failed build leaves the previous
-    /// verifier in place: a half-written bundle must not disarm node auth.
-    /// Test-only convenience. Production goes through
+    /// Production goes through
     /// [`build_roots_from_pem`](Self::build_roots_from_pem) so the listener can
     /// share one read of the bundle with the TLS acceptor.
     #[cfg(test)]
@@ -164,21 +152,33 @@ impl NodeJwtValidator {
         self.build_roots_from_pem(&pem)
     }
 
-    /// Builds anchors from a bundle the caller already read.
+    /// Builds anchors from a bundle the caller already read, without installing
+    /// them.
     ///
-    /// The listener uses this so the TLS acceptor and this validator are built
-    /// from the *same* bytes: reading the file twice lets a rotation land
-    /// between the two reads, leaving each path trusting a different generation
-    /// of the client CA.
+    /// The listener reloads the same file every five minutes to pick up
+    /// cert-manager rotations. Building both the TLS acceptor and this
+    /// validator from the *same* bytes prevents a rotation between two reads
+    /// from leaving each path trusting a different client-CA generation. A
+    /// stale validator would reject tokens chaining to the new CA and, with
+    /// mTLS disabled, lock nodes out until the API restarts.
+    ///
+    /// The fallible build is separate from [`install_roots`](Self::install_roots)
+    /// so the listener can build this verifier and its new TLS acceptor before
+    /// committing either. It must never drop the acceptor and serve plaintext
+    /// while bearer auth remains armed. A failed build leaves the previous
+    /// verifier in place: a half-written bundle must not disarm node auth.
     pub(crate) fn build_roots_from_pem(
         &self,
         pem: &[u8],
     ) -> Result<Arc<dyn ClientCertVerifier>, NodeAuthError> {
         Self::verifier_from_pem(&self.root_cafile_path, pem)
     }
 
-    /// Installs anchors from [`build_roots`](Self::build_roots). Infallible, so
-    /// it is safe to call in a commit phase alongside other swaps.
+    /// Installs anchors from [`build_roots_from_pem`](Self::build_roots_from_pem).
+    ///
+    /// The listener calls this only after it has also built a TLS acceptor from
+    /// the same bundle. Because this method is infallible, it is safe in the
+    /// commit phase that keeps TLS and bearer token trust anchors in sync.
     pub(crate) fn install_roots(&self, cert_verifier: Arc<dyn ClientCertVerifier>) {
         *self
             .cert_verifier
@@ -229,10 +229,20 @@ impl NodeJwtValidator {
         }
 
         // 1. The certificate chain must verify against the trusted roots.
+        let encoded_chain = header
+            .x5c
+            .as_ref()
+            .filter(|chain| !chain.is_empty())
+            .ok_or(RejectReason::NoChain)?;
+        if encoded_chain.len() > ::rpc::node_jwt::NODE_JWT_MAX_X5C_CERTIFICATES {
+            return Err(RejectReason::ChainTooLong {
+                actual: encoded_chain.len(),
+                maximum: ::rpc::node_jwt::NODE_JWT_MAX_X5C_CERTIFICATES,
+            });
+        }
         let chain = header
             .x5c_der()
             .map_err(RejectReason::Malformed)?
-            .filter(|chain| !chain.is_empty())
             .ok_or(RejectReason::NoChain)?;
         let leaf = CertificateDer::from(chain[0].clone());
         let intermediates: Vec<CertificateDer> = chain[1..]
@@ -511,6 +521,57 @@ mod tests {
         assert!(validator.spiffe_id_from_bearer(&no_chain).is_none());
     }
 
+    /// The encoded list is checked before Base64 decoding so an untrusted
+    /// header cannot make certificate-path validation scale with its length.
+    #[test]
+    fn x5c_certificate_count_is_capped_before_decoding() {
+        #[derive(Debug, Eq, PartialEq)]
+        enum Rejection {
+            Malformed,
+            ChainTooLong { actual: usize, maximum: usize },
+        }
+
+        let dir = tempfile::tempdir().expect("tempdir");
+        let pki = test_pki(MACHINE_PATH);
+        let validator = validator_for(&dir, &pki.ca_pem);
+        let encoding_key =
+            jsonwebtoken::EncodingKey::from_ec_pem(pki.key_pem.as_bytes()).expect("encoding key");
+
+        check_values(
+            [
+                Check {
+                    scenario: "the maximum certificate count still reaches decoding",
+                    input: ::rpc::node_jwt::NODE_JWT_MAX_X5C_CERTIFICATES,
+                    expect: Rejection::Malformed,
+                },
+                Check {
+                    scenario: "one certificate above the limit is rejected before decoding",
+                    input: ::rpc::node_jwt::NODE_JWT_MAX_X5C_CERTIFICATES + 1,
+                    expect: Rejection::ChainTooLong {
+                        actual: ::rpc::node_jwt::NODE_JWT_MAX_X5C_CERTIFICATES + 1,
+                        maximum: ::rpc::node_jwt::NODE_JWT_MAX_X5C_CERTIFICATES,
+                    },
+                },
+            ],
+            |certificate_count| {
+                let mut header = jsonwebtoken::Header::new(Algorithm::ES256);
+                // This string is deliberately invalid Base64: only the overlong
+                // case may reach `ChainTooLong` without trying to decode it.
+                header.x5c = Some(vec!["@".to_string(); certificate_count]);
+                let token = jsonwebtoken::encode(&header, &serde_json::json!({}), &encoding_key)
+                    .expect("token encodes");
+
+                match validator.validate(&token) {
+                    Err(RejectReason::Malformed(_)) => Rejection::Malformed,
+                    Err(RejectReason::ChainTooLong { actual, maximum }) => {
+                        Rejection::ChainTooLong { actual, maximum }
+                    }
+                    result => panic!("unexpected node-token validation result: {result:?}"),
+                }
+            },
+        );
+    }
+
     /// Both the identity cross-check and algorithm pin are independent of the
     /// x5c-chain verification. Keep explicit coverage so an otherwise-valid
     /// certificate cannot accidentally make either attacker-controlled header

crates/rpc/src/node_jwt.rs

@@ -32,6 +32,7 @@
 //! mTLS client cert remains the only credential.
 
 use std::io::Cursor;
+use std::sync::atomic::{AtomicU64, Ordering};
 use std::sync::{Arc, RwLock};
 use std::task::{Context, Poll};
 use std::time::{SystemTime, UNIX_EPOCH};
@@ -50,6 +51,19 @@ pub const NODE_JWT_AUDIENCE: &str = "nico-api";
 /// re-mint locally, so a leaked one ages out in minutes.
 pub const NODE_JWT_TTL_SECS: u64 = 300;
 
+/// Maximum certificates carried in a node-auth JWT's `x5c` header.
+///
+/// Node certificates normally include a leaf and issuing CA. This leaves room
+/// for additional intermediates while bounding the verifier's work on an
+/// untrusted request.
+pub const NODE_JWT_MAX_X5C_CERTIFICATES: usize = 4;
+
+/// Avoid repeatedly warning from the per-request token-minting path for the
+/// same persistent certificate-bundle configuration error.
+const OVER_LIMIT_WARNING_INTERVAL_SECS: u64 = NODE_JWT_TTL_SECS;
+
+static LAST_OVER_LIMIT_WARNING_AT: AtomicU64 = AtomicU64::new(0);
+
 /// A cached token is reused until it has less than this long left, then
 /// re-minted. Comfortably above per-request latency, comfortably below TTL.
 const REMINT_MARGIN_SECS: u64 = 60;
@@ -66,6 +80,9 @@ pub enum NodeJwtError {
     BadKey(String),
     #[error("client private key does not match the certificate's public key")]
     KeyCertMismatch,
+    /// The client certificate bundle exceeds the node-auth JWT header limit.
+    #[error("client certificate file has {actual} certificates; maximum is {maximum}")]
+    TooManyCertificates { actual: usize, maximum: usize },
     #[error("JWT signing failed: {0}")]
     Sign(#[from] jsonwebtoken::errors::Error),
     #[error("system clock is before the UNIX epoch")]
@@ -124,9 +141,11 @@ impl NodeJwtMinter {
     }
 
     /// Returns a currently-valid token, re-minting if the cached one is
-    /// missing or close to expiry. Returns `None` (and logs at debug) when
-    /// minting is impossible — e.g. the cert/key files don't exist yet — so
-    /// callers degrade gracefully to mTLS-only.
+    /// missing or close to expiry. Returns `None` when minting is impossible
+    /// — e.g. the cert/key files don't exist yet — so callers degrade
+    /// gracefully to mTLS-only. Transient failures log at debug; an
+    /// over-limit certificate bundle logs at most once every five minutes per
+    /// process because it requires an operator configuration change.
     pub fn current(&self) -> Option<String> {
         self.current_with_expiry().map(|(token, _)| token)
     }
@@ -158,6 +177,17 @@ impl NodeJwtMinter {
                     .unwrap_or_else(|poisoned| poisoned.into_inner()) = Some(minted);
                 Some(result)
             }
+            Err(error @ NodeJwtError::TooManyCertificates { .. }) => {
+                if should_log_over_limit_warning(now, &LAST_OVER_LIMIT_WARNING_AT) {
+                    tracing::warn!(
+                        target: "node_auth",
+                        cert_path = %self.cert_path,
+                        %error,
+                        "node-auth: client certificate bundle cannot mint node JWT; continuing with mTLS only"
+                    );
+                }
+                None
+            }
             Err(error) => {
                 tracing::debug!(
                     target: "node_auth",
@@ -178,6 +208,12 @@ impl NodeJwtMinter {
             .collect::<Result<Vec<_>, _>>()
             .map_err(|e| NodeJwtError::BadCertificate(e.to_string()))?;
         let leaf = chain.first().ok_or(NodeJwtError::NoCertificate)?;
+        if chain.len() > NODE_JWT_MAX_X5C_CERTIFICATES {
+            return Err(NodeJwtError::TooManyCertificates {
+                actual: chain.len(),
+                maximum: NODE_JWT_MAX_X5C_CERTIFICATES,
+            });
+        }
         let sub = spiffe_uri_from_cert(leaf.as_ref())?;
         let secret = parse_secret_key(&key_pem)?;
 
@@ -214,6 +250,20 @@ fn unix_now() -> Result<u64, NodeJwtError> {
         .map_err(|_| NodeJwtError::Clock)
 }
 
+fn should_log_over_limit_warning(now: u64, last_warning_at: &AtomicU64) -> bool {
+    let mut last = last_warning_at.load(Ordering::Relaxed);
+    loop {
+        if last != 0 && now.saturating_sub(last) < OVER_LIMIT_WARNING_INTERVAL_SECS {
+            return false;
+        }
+        match last_warning_at.compare_exchange_weak(last, now, Ordering::Relaxed, Ordering::Relaxed)
+        {
+            Ok(_) => return true,
+            Err(current) => last = current,
+        }
+    }
+}
+
 /// Extracts the single SPIFFE URI SAN from the certificate — the same field
 /// the server's authn layer maps to a machine principal for mTLS.
 fn spiffe_uri_from_cert(der: &[u8]) -> Result<String, NodeJwtError> {
@@ -356,6 +406,9 @@ where
 
 #[cfg(test)]
 mod tests {
+    use std::sync::atomic::AtomicU64;
+
+    use carbide_test_support::{Check, check_values};
     use jsonwebtoken::{DecodingKey, Validation};
     use serde::Deserialize;
 
@@ -450,6 +503,81 @@ mod tests {
         assert_eq!(claims.exp - claims.iat, NODE_JWT_TTL_SECS);
     }
 
+    #[test]
+    fn certificate_chain_length_must_fit_the_node_jwt_header() {
+        #[derive(Debug, Eq, PartialEq)]
+        enum Outcome {
+            Mints,
+            TooManyCertificates { actual: usize, maximum: usize },
+        }
+
+        let (cert_pem, key_pem) = cert_and_key();
+        let dir = tempfile::tempdir().expect("tempdir");
+
+        check_values(
+            [
+                Check {
+                    scenario: "the maximum certificate count is minted",
+                    input: NODE_JWT_MAX_X5C_CERTIFICATES,
+                    expect: Outcome::Mints,
+                },
+                Check {
+                    scenario: "one certificate above the limit is rejected",
+                    input: NODE_JWT_MAX_X5C_CERTIFICATES + 1,
+                    expect: Outcome::TooManyCertificates {
+                        actual: NODE_JWT_MAX_X5C_CERTIFICATES + 1,
+                        maximum: NODE_JWT_MAX_X5C_CERTIFICATES,
+                    },
+                },
+            ],
+            |certificate_count| {
+                let minter = NodeJwtMinter::new(
+                    write_temp(&dir, "cert.pem", &cert_pem.repeat(certificate_count)),
+                    write_temp(&dir, "key.pem", &key_pem),
+                );
+
+                match minter.mint(unix_now().expect("system clock")) {
+                    Ok(_) => Outcome::Mints,
+                    Err(NodeJwtError::TooManyCertificates { actual, maximum }) => {
+                        Outcome::TooManyCertificates { actual, maximum }
+                    }
+                    Err(error) => panic!("unexpected node-token mint error: {error:?}"),
+                }
+            },
+        );
+    }
+
+    #[test]
+    fn over_limit_bundle_warning_is_rate_limited() {
+        check_values(
+            [
+                Check {
+                    scenario: "the first process-wide failure is reported",
+                    input: (0, 1_000),
+                    expect: true,
+                },
+                Check {
+                    scenario: "a failure one second later is suppressed",
+                    input: (1_000, 1_001),
+                    expect: false,
+                },
+                Check {
+                    scenario: "a failure just before the interval ends is suppressed",
+                    input: (1_000, 1_000 + OVER_LIMIT_WARNING_INTERVAL_SECS - 1),
+                    expect: false,
+                },
+                Check {
+                    scenario: "a failure at the interval boundary is reported",
+                    input: (1_000, 1_000 + OVER_LIMIT_WARNING_INTERVAL_SECS),
+                    expect: true,
+                },
+            ],
+            |(last_warning_at, now)| {
+                should_log_over_limit_warning(now, &AtomicU64::new(last_warning_at))
+            },
+        );
+    }
+
     #[test]
     fn sec1_key_pem_is_accepted() {
         // Vault PKI hands out SEC1-encoded EC keys; re-encode the test key the

docs/design/machine-identity/node-auth-jwt.md

@@ -276,6 +276,14 @@ against the root CA bundle it already holds (`[tls] root_cafile_path`). There
 is no JWKS endpoint, no key registry, and no key distribution problem — CA
 rotation is handled wherever the root bundle is handled today.
 
+An `x5c` chain contains at most four certificates, including the leaf. The
+node refuses to mint a token when its client-certificate bundle exceeds that
+limit, emits a `node_auth` warning at most once every five minutes per process,
+and continues with mTLS only. The API rejects an over-limit token before Base64
+decoding its certificates or building a verification path. Keep the node's
+bundle within the limit; otherwise, a deployment with `mtls_enabled = false`
+has no node authentication path.
+
 **CA rotation** moves both consumers of that bundle together. The TLS
 listener already re-reads the file every five minutes for cert-manager
 rotations; the validator's trust anchors are held behind a lock and refreshed