From f0ced77194cf5570fb3f522f38a8b1e40f5b5d51 Mon Sep 17 00:00:00 2001 From: Erick Bourgeois Date: Sun, 5 Jul 2026 07:07:58 -0400 Subject: [PATCH] Add --features k8s-token-review to build Signed-off-by: Erick Bourgeois --- .claude/CHANGELOG.md | 83 +++++++++++++++++++++++++++ .github/workflows/build.yaml | 7 ++- Cargo.toml | 6 +- Makefile | 24 ++++---- docs/src/operations/authentication.md | 36 +++++++----- src/auth.rs | 45 ++++++++++----- src/auth_test.rs | 43 ++++++++++++++ src/main.rs | 67 +++++++++++---------- 8 files changed, 240 insertions(+), 71 deletions(-) diff --git a/.claude/CHANGELOG.md b/.claude/CHANGELOG.md index 5aee3f4..c203ac8 100644 --- a/.claude/CHANGELOG.md +++ b/.claude/CHANGELOG.md @@ -1,5 +1,88 @@ # Changelog +## [2026-07-05 11:45] - Runtime-select auth mode: shared-secret and TokenReview mutually exclusive + +**Author:** Erick Bourgeois + +### Changed +- `src/auth.rs`: added `shared_secret_configured()` (BIND_API_TOKEN set → true). + `authenticate()` now consults the Kubernetes TokenReview API **only when + shared-secret mode is not selected** — the two modes are mutually exclusive. + Refactored `has_real_auth()` to reuse the helper. +- `src/main.rs`: the A2 fail-closed TokenReview authorization guard runs **only + in TokenReview mode**; shared-secret mode logs that TokenReview is not active. +- `src/auth_test.rs`: added `auth_mode_selection_tests`. +- `docs/src/operations/authentication.md`: rewrote "Authentication Modes" — the + mode is now runtime-selected by `BIND_API_TOKEN` (the published image always + has the feature compiled in). + +### Why +Shipping the `k8s-token-review` feature in the published image made TokenReview +AND'd onto every request whenever the feature was compiled, and the A2 guard +required an allowlist — which broke shared-secret/drone deployments: the kind e2e +crashed at startup, and even past it, every shared-secret request would have +401'd via TokenReview (a single Bearer token cannot be both the shared secret and +a valid ServiceAccount token). Making the modes mutually exclusive lets one image +serve both bindy (TokenReview) and drone/shared-secret deployments. + +### Verified +Feature-enabled binary in shared-secret mode, no cluster: starts (guard skipped); +no token / wrong token → 401; **right token → 200** (TokenReview skipped — would +have 401'd if it had run). `make regression` green (351 tests incl. the new +mode-selection tests). TokenReview mode (no `BIND_API_TOKEN`) is unchanged — the +fail-closed guard is intact. + +### Impact +- [x] Fixes shared-secret/drone auth in the feature-enabled image (kind e2e) +- [x] TokenReview mode unchanged; A2 fail-closed guarantee preserved +- [ ] Breaking change (a shared-secret deployment that also expected TokenReview + enforcement — impossible with one Bearer token — now cleanly uses shared + secret only) + +## [2026-07-05 09:30] - Ship k8s-token-review in published binaries; pin k8s-openapi version via feature + +**Author:** Erick Bourgeois + +### Changed +- `.github/workflows/build.yaml`: the `Build binary` step now passes + `extra-args: "--features k8s-token-review"`. CI already lint/tested both feature + sets, but the **shipped** release binary was built feature-less, so the + published image could not perform Kubernetes TokenReview and tripped the + startup guard under bindy's config (the "Mode B" blocker). +- `Cargo.toml`: the `k8s-token-review` feature now also enables + `k8s-openapi/v1_32`, pinning the API version. Previously the version came only + from a dev-dependency (tests) plus the `K8S_OPENAPI_ENABLED_VERSION` env var + (release builds), so `cargo build --release --features k8s-token-review` + without that env panicked ("None of the v1_* features are enabled"). +- Removed `K8S_OPENAPI_ENABLED_VERSION` from `Makefile`, the `build.yaml` env, + and the `Cross.toml` aarch64 passthrough — the feature is now the single source + of truth. (The env acts as a feature toggle: env `1.33` alongside the pinned + `v1_32` hard-errors "Both v1_32 and v1_33 features enabled", so keeping both was + a latent footgun.) + +### Why +bindy runs bindcar with TokenReview enabled; the default (`default = []`) binary +lacks the `kube`/`k8s-openapi` path. Pinning the version in the feature makes the +release build work on host cargo AND in the aarch64 `cross` container with no env +var to forget or mismatch. + +### Verified +`cargo build --release --features k8s-token-review` (env unset) compiles (pulls +`kube v4.0.0`, `k8s-openapi 0.28`); `make regression` (env unset) green — clippy ++ unit/doctests for both feature sets. + +### Follow-up (separate repo) +The `firestoned/github-actions` `rust/generate-sbom` action has no `features` +input, so the SBOM is generated feature-less and omits `kube`/`k8s-openapi`. Add +a `features`/`extra-args` input there and pass `--features k8s-token-review` so +the SBOM matches the shipped binary. Not fixed inline (composite-action rule). + +### Impact +- [x] Published images can now do TokenReview (fixes bindy Mode B) +- [x] `cargo build --release --features k8s-token-review` needs no env var +- [ ] Breaking change +- [x] CI/CD + build config + ## [2026-07-04 13:10] - Fix nsupdate builder COPY: remap /lib to /usr/lib (usrmerge) **Author:** Erick Bourgeois diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml index 01292ec..3fac8f3 100644 --- a/.github/workflows/build.yaml +++ b/.github/workflows/build.yaml @@ -36,7 +36,6 @@ permissions: env: CARGO_TERM_COLOR: always RUST_BACKTRACE: 1 - K8S_OPENAPI_ENABLED_VERSION: "1.32" jobs: @@ -200,6 +199,12 @@ jobs: uses: firestoned/github-actions/rust/build-binary@d0d51c638a90bffc2a1567fe7af112b37fe8854c # v1.3.7 with: target: ${{ matrix.platform.target }} + # Ship the k8s TokenReview auth path — bindy runs bindcar with + # TokenReview enabled, and the default (feature-less) binary trips the + # startup guard. The feature pins k8s-openapi's API version (v1_32) in + # Cargo.toml, so no K8S_OPENAPI_ENABLED_VERSION env is needed on host + # cargo or in the aarch64 cross container. + extra-args: "--features k8s-token-review" - name: Generate SBOM uses: firestoned/github-actions/rust/generate-sbom@d0d51c638a90bffc2a1567fe7af112b37fe8854c # v1.3.7 diff --git a/Cargo.toml b/Cargo.toml index 6e00a04..3aaa055 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -78,7 +78,11 @@ sha2 = "0.10" [features] default = [] -k8s-token-review = ["kube", "k8s-openapi"] +# Pin the k8s-openapi API version via the feature so a release binary build +# (`cargo build --release --features k8s-token-review`) needs no +# K8S_OPENAPI_ENABLED_VERSION env var — the version travels with the feature, +# on host cargo and in the aarch64 cross container alike. +k8s-token-review = ["kube", "k8s-openapi", "k8s-openapi/v1_32"] [dev-dependencies] # Testing diff --git a/Makefile b/Makefile index 1bc79a0..82a3b05 100644 --- a/Makefile +++ b/Makefile @@ -4,7 +4,9 @@ # Makefile for bindcar # Configuration -K8S_OPENAPI_ENABLED_VERSION ?= 1.32 +# Note: the k8s-openapi API version is pinned via the `k8s-token-review` feature +# in Cargo.toml (k8s-openapi/v1_32), so no K8S_OPENAPI_ENABLED_VERSION env is +# needed — setting it to a different version would conflict with the feature. IMAGE_NAME ?= bindcar IMAGE_TAG ?= latest REGISTRY ?= ghcr.io/firestoned @@ -24,11 +26,11 @@ help: ## Show this help .PHONY: build build: ## Build the binary in release mode - K8S_OPENAPI_ENABLED_VERSION=$(K8S_OPENAPI_ENABLED_VERSION) cargo build --release + cargo build --release .PHONY: test test: ## Run tests - K8S_OPENAPI_ENABLED_VERSION=$(K8S_OPENAPI_ENABLED_VERSION) cargo test + cargo test .PHONY: fmt fmt: ## Format code @@ -36,7 +38,7 @@ fmt: ## Format code .PHONY: clippy clippy: ## Run clippy - K8S_OPENAPI_ENABLED_VERSION=$(K8S_OPENAPI_ENABLED_VERSION) cargo clippy -- -D warnings + cargo clippy -- -D warnings .PHONY: check check: fmt clippy test ## Run all checks @@ -53,16 +55,16 @@ fmt-check: ## Verify formatting without modifying files .PHONY: clippy-all clippy-all: ## Clippy for default AND k8s-token-review features (deny warnings) @echo "==> clippy (default features)" - K8S_OPENAPI_ENABLED_VERSION=$(K8S_OPENAPI_ENABLED_VERSION) cargo clippy --all-targets -- -D warnings + cargo clippy --all-targets -- -D warnings @echo "==> clippy (k8s-token-review feature)" - K8S_OPENAPI_ENABLED_VERSION=$(K8S_OPENAPI_ENABLED_VERSION) cargo clippy --all-targets --features k8s-token-review -- -D warnings + cargo clippy --all-targets --features k8s-token-review -- -D warnings .PHONY: unit-tests unit-tests: ## Unit + doctests for BOTH feature sets — no cluster required @echo "==> unit tests (default features)" - K8S_OPENAPI_ENABLED_VERSION=$(K8S_OPENAPI_ENABLED_VERSION) cargo test + cargo test @echo "==> unit tests (k8s-token-review feature)" - K8S_OPENAPI_ENABLED_VERSION=$(K8S_OPENAPI_ENABLED_VERSION) cargo test --features k8s-token-review + cargo test --features k8s-token-review # Back-compat alias. .PHONY: test-all @@ -147,7 +149,7 @@ docker-buildx: ## Build multi-arch Docker image .PHONY: run run: ## Run the API server locally @mkdir -p .tmp/zones - K8S_OPENAPI_ENABLED_VERSION=$(K8S_OPENAPI_ENABLED_VERSION) RUST_LOG=debug BIND_ZONE_DIR=.tmp/zones cargo run + RUST_LOG=debug BIND_ZONE_DIR=.tmp/zones cargo run .PHONY: clean clean: ## Clean build artifacts @@ -164,7 +166,7 @@ docs: ## Build all documentation (MkDocs + rustdoc + OpenAPI) @echo "Ensuring documentation dependencies are installed..." @cd docs && poetry install --no-interaction --quiet @echo "Building rustdoc API documentation..." - @K8S_OPENAPI_ENABLED_VERSION=$(K8S_OPENAPI_ENABLED_VERSION) cargo doc --no-deps --all-features + @cargo doc --no-deps --all-features @echo "Building MkDocs documentation..." @cd docs && poetry run mkdocs build @echo "Copying rustdoc into documentation..." @@ -228,7 +230,7 @@ docs-serve: ## Serve documentation locally with live reload (MkDocs) .PHONY: docs-rustdoc docs-rustdoc: ## Build and open rustdoc API documentation only @echo "Building rustdoc API documentation..." - @K8S_OPENAPI_ENABLED_VERSION=$(K8S_OPENAPI_ENABLED_VERSION) cargo doc --no-deps --all-features --open + @cargo doc --no-deps --all-features --open .PHONY: docs-clean docs-clean: ## Clean documentation build artifacts diff --git a/docs/src/operations/authentication.md b/docs/src/operations/authentication.md index e2d3ef2..757ceb8 100644 --- a/docs/src/operations/authentication.md +++ b/docs/src/operations/authentication.md @@ -13,25 +13,33 @@ All other endpoints require a valid Bearer token in the Authorization header. ## Authentication Modes -bindcar supports two authentication modes: +The published bindcar images are built with the `k8s-token-review` feature +compiled in, and the active mode is selected **at runtime**. Shared-secret and +TokenReview auth are **mutually exclusive** — a single Bearer token cannot be +both a shared secret and a valid ServiceAccount token — so `BIND_API_TOKEN` +selects the mode: -### Basic Mode (Default) +### Shared-secret Mode (`BIND_API_TOKEN` set) -- Validates token presence and format only -- Does NOT verify token signatures -- Does NOT check expiration -- Suitable for trusted environments or when using external auth (API gateway, Linkerd service mesh) +- The presented Bearer token must equal `BIND_API_TOKEN` (constant-time compare) +- The Kubernetes TokenReview API is **not** consulted, and the fail-closed + TokenReview allowlist guard (below) is **not** enforced at startup +- Suitable for drone/standalone deployments and trusted environments -### TokenReview Mode (Optional) +### TokenReview Mode (`BIND_API_TOKEN` unset, feature compiled) -- Full token validation with Kubernetes TokenReview API -- Verifies token signatures -- Checks token expiration -- Validates token audience +- Full token validation with the Kubernetes TokenReview API +- Verifies token signatures, checks expiration, validates token audience - Restricts to allowed namespaces and ServiceAccounts -- **Recommended for production Kubernetes deployments** - -Enable TokenReview mode by building with the `k8s-token-review` feature. See [Kubernetes TokenReview Validation](../developer-guide/k8s-token-validation.md) for detailed configuration. +- **Fail-closed**: bindcar refuses to start unless an allowlist + (`BIND_ALLOWED_NAMESPACES` / `BIND_ALLOWED_SERVICE_ACCOUNTS`) is set or + `BIND_ALLOW_ANY_SERVICEACCOUNT=true` is explicitly configured +- **Recommended for production Kubernetes deployments** (this is how bindy runs bindcar) + +> A build **without** the `k8s-token-review` feature only validates token +> presence/format (no signature check) — token verification must then be handled +> by infrastructure (API gateway, Linkerd service mesh). See +> [Kubernetes TokenReview Validation](../developer-guide/k8s-token-validation.md). ## Bearer Token Authentication diff --git a/src/auth.rs b/src/auth.rs index 719a7da..23206a2 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -210,9 +210,19 @@ pub fn check_authorization_posture( /// Returns `true` if a real authenticator is configured at runtime: either the /// Kubernetes TokenReview feature is compiled in, or a shared secret is set. pub fn has_real_auth() -> bool { - if cfg!(feature = "k8s-token-review") { - return true; - } + cfg!(feature = "k8s-token-review") || shared_secret_configured() +} + +/// Returns `true` when shared-secret authentication is the selected mode — i.e. +/// [`BIND_API_TOKEN_ENV`] is set to a non-empty value. +/// +/// Shared-secret and Kubernetes TokenReview are **mutually exclusive**: a single +/// Bearer token cannot be both the shared secret and a valid ServiceAccount +/// token. When a shared secret is configured it is the selected mode, so the +/// TokenReview path is not consulted at request time and the fail-closed +/// TokenReview authorization posture (A2) is not enforced at startup. TokenReview +/// is used only when no shared secret is set (and the feature is compiled in). +pub fn shared_secret_configured() -> bool { std::env::var(BIND_API_TOKEN_ENV) .map(|value| !value.is_empty()) .unwrap_or(false) @@ -563,25 +573,30 @@ pub async fn authenticate( return Err((StatusCode::UNAUTHORIZED, Json(AuthError { error: e }))); } - // Validate token with Kubernetes TokenReview API if feature is enabled. + // Validate token with Kubernetes TokenReview API — but only when TokenReview + // is the active mode. A configured shared secret (BIND_API_TOKEN) selects + // shared-secret auth, which is mutually exclusive with TokenReview (see + // `shared_secret_configured`), so we do not also require the Bearer token to + // be a valid ServiceAccount token. + // // The detailed reason (namespace/SA/api error) is logged server-side only; the // client gets a single generic "Unauthorized" so it cannot distinguish // "valid token, wrong namespace" from "invalid token" from "apiserver // unreachable" — an authorization/identity-enumeration oracle (A7). #[cfg(feature = "k8s-token-review")] - if let Err(e) = validate_token_with_k8s(token).await { - warn!("Token validation failed: {}", e); - return Err(( - StatusCode::UNAUTHORIZED, - Json(AuthError { - error: "Unauthorized".to_string(), - }), - )); + if !shared_secret_configured() { + if let Err(e) = validate_token_with_k8s(token).await { + warn!("Token validation failed: {}", e); + return Err(( + StatusCode::UNAUTHORIZED, + Json(AuthError { + error: "Unauthorized".to_string(), + }), + )); + } + debug!("Token validated with Kubernetes TokenReview API"); } - #[cfg(feature = "k8s-token-review")] - debug!("Token validated with Kubernetes TokenReview API"); - #[cfg(not(feature = "k8s-token-review"))] debug!("Token validation: basic mode (presence check only)"); diff --git a/src/auth_test.rs b/src/auth_test.rs index 04eb681..16dce28 100644 --- a/src/auth_test.rs +++ b/src/auth_test.rs @@ -887,3 +887,46 @@ mod b4_auth_posture_tests { assert!(check_startup_auth_posture(true, false, "0.0.0.0", true).is_ok()); } } + +/// Auth-mode selection (Option 1): shared-secret and TokenReview are mutually +/// exclusive. A configured `BIND_API_TOKEN` selects shared-secret mode, in which +/// TokenReview is not consulted and the A2 fail-closed guard is not enforced. +#[cfg(test)] +mod auth_mode_selection_tests { + use crate::auth::{has_real_auth, shared_secret_configured, BIND_API_TOKEN_ENV}; + use serial_test::serial; + use std::env; + + #[test] + #[serial] + fn test_shared_secret_configured_true_when_token_set() { + env::set_var(BIND_API_TOKEN_ENV, "s3cret-token"); + assert!(shared_secret_configured()); + env::remove_var(BIND_API_TOKEN_ENV); + } + + #[test] + #[serial] + fn test_shared_secret_configured_false_when_unset() { + env::remove_var(BIND_API_TOKEN_ENV); + assert!(!shared_secret_configured()); + } + + #[test] + #[serial] + fn test_shared_secret_configured_false_when_empty() { + // An empty value is not a usable secret and must not select the mode. + env::set_var(BIND_API_TOKEN_ENV, ""); + assert!(!shared_secret_configured()); + env::remove_var(BIND_API_TOKEN_ENV); + } + + #[test] + #[serial] + fn test_has_real_auth_true_with_shared_secret() { + env::remove_var(BIND_API_TOKEN_ENV); + env::set_var(BIND_API_TOKEN_ENV, "tok"); + assert!(has_real_auth()); + env::remove_var(BIND_API_TOKEN_ENV); + } +} diff --git a/src/main.rs b/src/main.rs index 42949f2..88ddeba 100644 --- a/src/main.rs +++ b/src/main.rs @@ -297,38 +297,47 @@ async fn start_server(command: &Commands, insecure_override: bool) -> anyhow::Re #[cfg(feature = "k8s-token-review")] { use bindcar::auth::{ - check_authorization_posture, detect_kube_auth_mode, KubeAuthMode, - TokenReviewConfig, ALLOW_ANY_SERVICE_ACCOUNT_ENV, + check_authorization_posture, detect_kube_auth_mode, shared_secret_configured, + KubeAuthMode, TokenReviewConfig, ALLOW_ANY_SERVICE_ACCOUNT_ENV, }; - // Fail-closed authorization posture (A2): with TokenReview active but - // no namespace/service-account allowlist, any authenticated - // ServiceAccount in the cluster would be authorized. Refuse to start - // unless the operator configured an allowlist or explicitly opted in. - let tr_config = TokenReviewConfig::from_env(); - let allow_any = std::env::var(ALLOW_ANY_SERVICE_ACCOUNT_ENV) - .ok() - .and_then(|v| v.parse::().ok()) - .unwrap_or(false); - if let Err(e) = - check_authorization_posture(tr_config.is_authorization_restricted(), allow_any) - { - error!("{}", e); - return Err(anyhow::anyhow!(e)); - } - if allow_any && !tr_config.is_authorization_restricted() { - warn!("⚠️ BIND_ALLOW_ANY_SERVICEACCOUNT is set: every authenticated ServiceAccount in the cluster is authorized"); - } - - match detect_kube_auth_mode() { - KubeAuthMode::Explicit { ref server, .. } => { - info!( - "kubernetes auth mode: explicit (KUBE_API_SERVER={})", - server - ); + // TokenReview and shared-secret auth are mutually exclusive: a + // configured shared secret (BIND_API_TOKEN) selects shared-secret + // mode, in which TokenReview is never consulted at request time + // (see auth::authenticate). Only enforce the fail-closed TokenReview + // authorization posture (A2) when TokenReview is the active mode. + if shared_secret_configured() { + info!("auth mode: shared-secret (BIND_API_TOKEN) — Kubernetes TokenReview is not active"); + } else { + // Fail-closed authorization posture (A2): with TokenReview active + // but no namespace/service-account allowlist, any authenticated + // ServiceAccount in the cluster would be authorized. Refuse to + // start unless the operator configured an allowlist or opted in. + let tr_config = TokenReviewConfig::from_env(); + let allow_any = std::env::var(ALLOW_ANY_SERVICE_ACCOUNT_ENV) + .ok() + .and_then(|v| v.parse::().ok()) + .unwrap_or(false); + if let Err(e) = + check_authorization_posture(tr_config.is_authorization_restricted(), allow_any) + { + error!("{}", e); + return Err(anyhow::anyhow!(e)); } - KubeAuthMode::Default => { - info!("kubernetes auth mode: try_default (KUBECONFIG / ~/.kube/config / in-cluster)"); + if allow_any && !tr_config.is_authorization_restricted() { + warn!("⚠️ BIND_ALLOW_ANY_SERVICEACCOUNT is set: every authenticated ServiceAccount in the cluster is authorized"); + } + + match detect_kube_auth_mode() { + KubeAuthMode::Explicit { ref server, .. } => { + info!( + "kubernetes auth mode: explicit (KUBE_API_SERVER={})", + server + ); + } + KubeAuthMode::Default => { + info!("kubernetes auth mode: try_default (KUBECONFIG / ~/.kube/config / in-cluster)"); + } } } }