From 68845f0c33a0f278ef6a35cff767ab2d217d0fda Mon Sep 17 00:00:00 2001 From: Rodney Osodo Date: Mon, 3 Aug 2026 15:04:29 +0300 Subject: [PATCH 1/3] fix: require admin auth to access /metrics The /metrics endpoint was unauthenticated, leaking internal label values such as resource paths and TEE types. Require a valid admin JWT (via the configured admin authentication/authorization backend) before serving Prometheus metrics, and map failures to AdminAuthAccess like the other admin-protected endpoints. Relax the regex_acl anchor validation from '^/kbs' to '^/' so that /metrics can be granted to admin roles via allowed_endpoints; existing '^/kbs...$' rules remain valid. Add an integration test covering no token, valid token, DenyAll admin backend, and a restricted ACL that excludes /metrics. Signed-off-by: Rodney Osodo --- Cargo.lock | 1 + integration-tests/Cargo.toml | 1 + integration-tests/src/common.rs | 13 ++++- integration-tests/tests/admin.rs | 72 ++++++++++++++++++++++++ kbs/docs/admin.md | 2 +- kbs/docs/config.md | 2 +- kbs/docs/metrics.md | 5 ++ kbs/src/admin/authorization/regex_acl.rs | 3 +- kbs/src/api_server.rs | 10 +++- 9 files changed, 101 insertions(+), 8 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 314d17b2d7..e5ada1cffd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4004,6 +4004,7 @@ dependencies = [ "openssl", "policy-engine", "reference-value-provider-service", + "reqwest 0.13.4", "rstest", "serde_json", "serial_test", diff --git a/integration-tests/Cargo.toml b/integration-tests/Cargo.toml index 420303e6fa..4938a8c5ec 100644 --- a/integration-tests/Cargo.toml +++ b/integration-tests/Cargo.toml @@ -17,6 +17,7 @@ anyhow.workspace = true base64.workspace = true const_format.workspace = true openssl.workspace = true +reqwest.workspace = true rstest.workspace = true serde_json.workspace = true jsonwebtoken.workspace = true diff --git a/integration-tests/src/common.rs b/integration-tests/src/common.rs index 7166293bae..41b95e6d74 100644 --- a/integration-tests/src/common.rs +++ b/integration-tests/src/common.rs @@ -185,7 +185,7 @@ async fn wait_for_reference_value(url: &str, key: &str) -> Result<()> { } impl TestHarness { - fn sign_admin_token(&self) -> Result { + pub fn sign_admin_token(&self) -> Result { let encoding_key = EncodingKey::from_ed_pem(self.auth_privkey.as_bytes())?; let now = SystemTime::now() .duration_since(UNIX_EPOCH) @@ -288,7 +288,7 @@ impl TestHarness { "regex_acl": { "acls": [{ "role": ADMIN_ROLE, - "allowed_endpoints": "^/kbs/v0/.*$" + "allowed_endpoints": "^/(kbs/v0/.*|metrics)$" }] } }, @@ -504,6 +504,15 @@ impl TestHarness { Ok(payload) } + + pub async fn get_metrics(&self, admin_token: Option) -> Result { + info!("TEST: Getting metrics"); + let mut request = reqwest::Client::new().get(format!("{KBS_URL}/metrics")); + if let Some(token) = admin_token { + request = request.bearer_auth(token); + } + Ok(request.send().await?) + } } static LOGGING_INIT: Once = Once::new(); diff --git a/integration-tests/tests/admin.rs b/integration-tests/tests/admin.rs index 13d4efd366..fb5ea27b67 100644 --- a/integration-tests/tests/admin.rs +++ b/integration-tests/tests/admin.rs @@ -192,6 +192,78 @@ import rego.v1 default executables = 97 "; +// +// The /metrics endpoint must be protected by admin authentication: +// no token is denied, a valid token is allowed, and disabled/restricted +// admin backends deny even authenticated requests. +// +#[rstest] +#[case::metrics_no_token(KbsConfigType::EarTokenBuiltInRvps, false)] +#[case::metrics_with_valid_token(KbsConfigType::EarTokenBuiltInRvps, true)] +#[case::metrics_with_deny_admin_backend(KbsConfigType::EarTokenBuiltInRvpsDenyAllAdmin, true)] +#[case::metrics_with_restricted_simple_backend( + KbsConfigType::EarTokenBuiltInRvpsSimpleRestrictedAdmin, + true +)] +#[serial(integration_ports)] +#[tokio::test(flavor = "multi_thread", worker_threads = 4)] +async fn metrics_requires_admin_auth( + #[case] test_config: KbsConfigType, + #[case] provide_token: bool, +) -> Result<()> { + init_tracing(); + + let deny_all = test_config == KbsConfigType::EarTokenBuiltInRvpsDenyAllAdmin; + let restricted = test_config == KbsConfigType::EarTokenBuiltInRvpsSimpleRestrictedAdmin; + + let harness = TestHarness::new(test_config.into()).await?; + harness.wait().await?; + + let token = provide_token.then(|| harness.sign_admin_token().expect("admin token")); + let res = harness.get_metrics(token).await?; + + harness.cleanup().await?; + + if deny_all { + assert_eq!( + res.status(), + reqwest::StatusCode::UNAUTHORIZED, + "metrics must be denied when the admin backend is disabled" + ); + return Ok(()); + } + + if restricted { + assert_eq!( + res.status(), + reqwest::StatusCode::UNAUTHORIZED, + "metrics must be denied for roles not allowed by the restricted ACL" + ); + return Ok(()); + } + + if provide_token { + assert_eq!( + res.status(), + reqwest::StatusCode::OK, + "metrics must be reachable with a valid admin token" + ); + let body = res.text().await?; + assert!( + body.contains("kbs_build_info"), + "expected metrics body to contain kbs_build_info, got: {body}" + ); + } else { + assert_eq!( + res.status(), + reqwest::StatusCode::UNAUTHORIZED, + "metrics must be denied without an admin token" + ); + } + + Ok(()) +} + // // Set a secret with the a valid admin private key // and with the wrong admin private key. diff --git a/kbs/docs/admin.md b/kbs/docs/admin.md index 2ea77e751d..a4f0cd0eca 100644 --- a/kbs/docs/admin.md +++ b/kbs/docs/admin.md @@ -39,7 +39,7 @@ This mode enables real admin authentication and authorization. - `authentication = bearer_jwt` verifies `Authorization: Bearer ` - JWT **MUST** contain a `role` claim - `authorization = regex_acl` authorizes by `acl(role -> allowed_endpoints)` -- `allowed_endpoints` must start with `^/kbs` and end with `$` +- `allowed_endpoints` must start with `^/` and end with `$` Example: diff --git a/kbs/docs/config.md b/kbs/docs/config.md index 97f6dad042..069a9c65cd 100644 --- a/kbs/docs/config.md +++ b/kbs/docs/config.md @@ -314,7 +314,7 @@ Each ACL entry: | `role` | String | JWT `role` value to match | Yes | | `allowed_endpoints` | String | Regex of allowed request paths | Yes | -`allowed_endpoints` must start with `^/kbs` and end with `$`. +`allowed_endpoints` must start with `^/` and end with `$`. ### Storage Backend Configuration diff --git a/kbs/docs/metrics.md b/kbs/docs/metrics.md index df65cb1cc4..234a02e703 100644 --- a/kbs/docs/metrics.md +++ b/kbs/docs/metrics.md @@ -2,6 +2,7 @@ The Key Broker Service (KBS) exposes Prometheus metrics on the `/metrics` HTTP endpoint served at the same port as KBS itself (see the `sockets` item in the `http_server` section of your KBS configuration file, 8080 by default). +> **Access control**: The `/metrics` endpoint is protected by the [admin API](admin.md) authentication and authorization configuration. Scraping clients must present a valid admin JWT (see `admin.authentication`) that is allowed for the `/metrics` path by the configured ACL (e.g. an `allowed_endpoints` regex of `^/(kbs/v0/.*|metrics)$`). When the admin backend is `DenyAll`, `/metrics` is not accessible at all. This prevents unauthenticated disclosure of sensitive label values (resource paths, TEE types). The `/metrics` endpoint itself is excluded from request metrics collection to avoid skewing the data with monitoring traffic. @@ -69,6 +70,10 @@ scrape_configs: scheme: https # or http if using insecure_http tls_config: insecure_skip_verify: true # only if using self-signed certificates + # The /metrics endpoint is admin-protected; present a valid admin JWT. + authorization: + type: Bearer + credentials_file: /path/to/admin.jwt ``` ## Kubernetes Deployment diff --git a/kbs/src/admin/authorization/regex_acl.rs b/kbs/src/admin/authorization/regex_acl.rs index f53e4eb2e3..d0ec9fdceb 100644 --- a/kbs/src/admin/authorization/regex_acl.rs +++ b/kbs/src/admin/authorization/regex_acl.rs @@ -42,8 +42,7 @@ impl TryFrom for RegexAclAuthorizer { fn try_from(config: RegexAclConfig) -> Result { let mut acls = Vec::new(); for acl in config.acls { - if !acl.allowed_endpoints.starts_with("^/kbs") || !acl.allowed_endpoints.ends_with("$") - { + if !acl.allowed_endpoints.starts_with("^/") || !acl.allowed_endpoints.ends_with("$") { return Err(Error::UnanchoredRegex); } let regex = Regex::new(&acl.allowed_endpoints)?; diff --git a/kbs/src/api_server.rs b/kbs/src/api_server.rs index 231e2a0443..25510a3015 100644 --- a/kbs/src/api_server.rs +++ b/kbs/src/api_server.rs @@ -476,9 +476,15 @@ pub(crate) async fn api( } pub(crate) async fn prometheus_metrics_handler( - _request: HttpRequest, - _core: web::Data, + request: HttpRequest, + core: web::Data, ) -> Result { + core.admin + .check_admin_access(&request) + .map_err(|e| Error::AdminAuthAccess { + source: e, + endpoint: "metrics".to_string(), + })?; let report = crate::prometheus::export_metrics().map_err(|e| Error::PrometheusError { source: e })?; Ok(HttpResponse::Ok().body(report)) From 44b272d14833c49af9b11421b8a35c5c372fe137 Mon Sep 17 00:00:00 2001 From: Rodney Osodo Date: Mon, 3 Aug 2026 15:37:27 +0300 Subject: [PATCH 2/3] fix: make admin auth on /metrics optional via config Add http_server.require_admin_auth_metrics (default false) so operators can opt in to protecting /metrics with admin JWT auth instead of it being mandatory. The default preserves unauthenticated metric scraping for existing deployments; enabling it applies the check_admin_access guard added previously. Update the metrics integration test to opt in to the protection, add a test asserting /metrics stays accessible without a token by default, and document the new flag in config.md and metrics.md. Signed-off-by: Rodney Osodo --- integration-tests/src/common.rs | 6 +++++ integration-tests/tests/admin.rs | 43 ++++++++++++++++++++++++++++---- kbs/docs/config.md | 1 + kbs/docs/metrics.md | 12 +++++++-- kbs/src/api_server.rs | 16 +++++++----- kbs/src/config.rs | 13 ++++++++++ 6 files changed, 78 insertions(+), 13 deletions(-) diff --git a/integration-tests/src/common.rs b/integration-tests/src/common.rs index 41b95e6d74..bb25589bd3 100644 --- a/integration-tests/src/common.rs +++ b/integration-tests/src/common.rs @@ -102,18 +102,22 @@ impl From for TestParameters { KbsConfigType::EarTokenBuiltInRvps => TestParameters { rvps_type: RvpsType::Builtin, admin_type: AdminType::Simple, + require_admin_auth_metrics: false, }, KbsConfigType::EarTokenRemoteRvps => TestParameters { rvps_type: RvpsType::Remote, admin_type: AdminType::Simple, + require_admin_auth_metrics: false, }, KbsConfigType::EarTokenBuiltInRvpsDenyAllAdmin => TestParameters { rvps_type: RvpsType::Builtin, admin_type: AdminType::DenyAll, + require_admin_auth_metrics: false, }, KbsConfigType::EarTokenBuiltInRvpsSimpleRestrictedAdmin => TestParameters { rvps_type: RvpsType::Builtin, admin_type: AdminType::SimpleRestricted, + require_admin_auth_metrics: false, }, } } @@ -123,6 +127,7 @@ impl From for TestParameters { pub struct TestParameters { pub rvps_type: RvpsType, pub admin_type: AdminType, + pub require_admin_auth_metrics: bool, } /// Internal state of tests @@ -349,6 +354,7 @@ impl TestHarness { insecure_http: true, payload_request_size: 2, worker_count: Some(4), + require_admin_auth_metrics: test_parameters.require_admin_auth_metrics, tls: TlsConfig::default(), }, admin: admin_config, diff --git a/integration-tests/tests/admin.rs b/integration-tests/tests/admin.rs index fb5ea27b67..749040cf5d 100644 --- a/integration-tests/tests/admin.rs +++ b/integration-tests/tests/admin.rs @@ -10,7 +10,7 @@ use tracing::info; extern crate integration_tests; use crate::integration_tests::common::{ - init_tracing, KbsConfigType, PolicyType, TestHarness, ADMIN_ROLE, + init_tracing, KbsConfigType, PolicyType, TestHarness, TestParameters, ADMIN_ROLE, }; // @@ -193,9 +193,9 @@ default executables = 97 "; // -// The /metrics endpoint must be protected by admin authentication: -// no token is denied, a valid token is allowed, and disabled/restricted -// admin backends deny even authenticated requests. +// The /metrics endpoint can be protected by admin authentication (opt-in via +// http_server.require_admin_auth_metrics): no token is denied, a valid token is +// allowed, and disabled/restricted admin backends deny even authenticated requests. // #[rstest] #[case::metrics_no_token(KbsConfigType::EarTokenBuiltInRvps, false)] @@ -216,7 +216,9 @@ async fn metrics_requires_admin_auth( let deny_all = test_config == KbsConfigType::EarTokenBuiltInRvpsDenyAllAdmin; let restricted = test_config == KbsConfigType::EarTokenBuiltInRvpsSimpleRestrictedAdmin; - let harness = TestHarness::new(test_config.into()).await?; + let mut params = TestParameters::from(test_config); + params.require_admin_auth_metrics = true; + let harness = TestHarness::new(params).await?; harness.wait().await?; let token = provide_token.then(|| harness.sign_admin_token().expect("admin token")); @@ -264,6 +266,37 @@ async fn metrics_requires_admin_auth( Ok(()) } +// +// With http_server.require_admin_auth_metrics disabled (the default), /metrics is +// served without any authentication, preserving backward compatibility. +// +#[rstest] +#[serial(integration_ports)] +#[tokio::test(flavor = "multi_thread", worker_threads = 4)] +async fn metrics_unprotected_by_default() -> Result<()> { + init_tracing(); + + let harness = TestHarness::new(KbsConfigType::EarTokenBuiltInRvps.into()).await?; + harness.wait().await?; + + let res = harness.get_metrics(None).await?; + + harness.cleanup().await?; + + assert_eq!( + res.status(), + reqwest::StatusCode::OK, + "metrics must be reachable without an admin token when protection is off" + ); + let body = res.text().await?; + assert!( + body.contains("kbs_build_info"), + "expected metrics body to contain kbs_build_info, got: {body}" + ); + + Ok(()) +} + // // Set a secret with the a valid admin private key // and with the wrong admin private key. diff --git a/kbs/docs/config.md b/kbs/docs/config.md index 069a9c65cd..6c10424840 100644 --- a/kbs/docs/config.md +++ b/kbs/docs/config.md @@ -27,6 +27,7 @@ The following properties can be set under the `[http_server]` section. | `certificate` | String | Path to a certificate file to be used for HTTPS. | No | None | | `payload_request_size` | Integer | Request payload size in mega bytes. | No | 2 | | `worker_count` | Integer | Number of HTTP actix worker threads | No | Num of logical CPU cores | +| `require_admin_auth_metrics` | Boolean | Require a valid admin token to access the `/metrics` endpoint. | No | `false` | | `tls_profile` | String | TLS security profile (see [TLS Configuration](#tls-configuration)) | No | `intermediate` | | `tls_min_version` | String | Minimum TLS version: `1.2` or `1.3` | No | Profile-dependent | | `tls_max_version` | String | Maximum TLS version: `1.2` or `1.3` | No | Profile-dependent | diff --git a/kbs/docs/metrics.md b/kbs/docs/metrics.md index 234a02e703..e4383aa353 100644 --- a/kbs/docs/metrics.md +++ b/kbs/docs/metrics.md @@ -2,7 +2,14 @@ The Key Broker Service (KBS) exposes Prometheus metrics on the `/metrics` HTTP endpoint served at the same port as KBS itself (see the `sockets` item in the `http_server` section of your KBS configuration file, 8080 by default). -> **Access control**: The `/metrics` endpoint is protected by the [admin API](admin.md) authentication and authorization configuration. Scraping clients must present a valid admin JWT (see `admin.authentication`) that is allowed for the `/metrics` path by the configured ACL (e.g. an `allowed_endpoints` regex of `^/(kbs/v0/.*|metrics)$`). When the admin backend is `DenyAll`, `/metrics` is not accessible at all. This prevents unauthenticated disclosure of sensitive label values (resource paths, TEE types). +> **Access control**: By default `/metrics` is served without authentication. Set +> `require_admin_auth_metrics = true` under the `http_server` section of your KBS +> configuration to protect the endpoint with the [admin API](admin.md) authentication and +> authorization configuration. When enabled, scraping clients must present a valid admin JWT +> (see `admin.authentication`) that is allowed for the `/metrics` path by the configured ACL +> (e.g. an `allowed_endpoints` regex of `^/(kbs/v0/.*|metrics)$`). When the admin backend is +> `DenyAll`, `/metrics` is not accessible at all. This prevents unauthenticated disclosure of +> sensitive label values (resource paths, TEE types). The `/metrics` endpoint itself is excluded from request metrics collection to avoid skewing the data with monitoring traffic. @@ -70,7 +77,8 @@ scrape_configs: scheme: https # or http if using insecure_http tls_config: insecure_skip_verify: true # only if using self-signed certificates - # The /metrics endpoint is admin-protected; present a valid admin JWT. + # Only needed when http_server.require_admin_auth_metrics = true: + # present a valid admin JWT. authorization: type: Bearer credentials_file: /path/to/admin.jwt diff --git a/kbs/src/api_server.rs b/kbs/src/api_server.rs index 25510a3015..f0548ce31a 100644 --- a/kbs/src/api_server.rs +++ b/kbs/src/api_server.rs @@ -479,12 +479,16 @@ pub(crate) async fn prometheus_metrics_handler( request: HttpRequest, core: web::Data, ) -> Result { - core.admin - .check_admin_access(&request) - .map_err(|e| Error::AdminAuthAccess { - source: e, - endpoint: "metrics".to_string(), - })?; + // The `/metrics` endpoint may optionally be protected by admin auth to prevent + // unauthenticated disclosure of sensitive label values (resource paths, TEE types). + if core.config.http_server.require_admin_auth_metrics { + core.admin + .check_admin_access(&request) + .map_err(|e| Error::AdminAuthAccess { + source: e, + endpoint: "metrics".to_string(), + })?; + } let report = crate::prometheus::export_metrics().map_err(|e| Error::PrometheusError { source: e })?; Ok(HttpResponse::Ok().body(report)) diff --git a/kbs/src/config.rs b/kbs/src/config.rs index 3cdda79937..54744323b4 100644 --- a/kbs/src/config.rs +++ b/kbs/src/config.rs @@ -123,6 +123,12 @@ pub struct HttpServerConfig { /// If not specified, defaults to the number of logical CPU cores. pub worker_count: Option, + /// Require a valid admin token (via the configured admin authentication/authorization + /// backend) to access the `/metrics` endpoint. When enabled, Prometheus scraping must + /// present a bearer JWT allowed for the `/metrics` path. Defaults to `false` for + /// backward compatibility with unauthenticated metric scraping. + pub require_admin_auth_metrics: bool, + /// TLS/HTTPS configuration #[serde(flatten)] pub tls: TlsConfig, @@ -135,6 +141,7 @@ impl Default for HttpServerConfig { insecure_http: DEFAULT_INSECURE_HTTP, payload_request_size: DEFAULT_PAYLOAD_REQUEST_SIZE, worker_count: None, + require_admin_auth_metrics: false, tls: TlsConfig::default(), } } @@ -371,6 +378,7 @@ mod tests { insecure_http: false, payload_request_size: DEFAULT_PAYLOAD_REQUEST_SIZE, worker_count: None, + require_admin_auth_metrics: false, tls: TlsConfig { private_key: Some("/etc/kbs-private.key".into()), certificate: Some("/etc/kbs-cert.pem".into()), @@ -432,6 +440,7 @@ mod tests { insecure_http: DEFAULT_INSECURE_HTTP, payload_request_size: DEFAULT_PAYLOAD_REQUEST_SIZE, worker_count: None, + require_admin_auth_metrics: false, tls: TlsConfig::default(), }, admin: AdminConfig::DenyAll {}, @@ -476,6 +485,7 @@ mod tests { insecure_http: false, payload_request_size: DEFAULT_PAYLOAD_REQUEST_SIZE, worker_count: None, + require_admin_auth_metrics: false, tls: TlsConfig { private_key: Some("/etc/kbs-private.key".into()), certificate: Some("/etc/kbs-cert.pem".into()), @@ -525,6 +535,7 @@ mod tests { insecure_http: true, payload_request_size: DEFAULT_PAYLOAD_REQUEST_SIZE, worker_count: None, + require_admin_auth_metrics: false, tls: TlsConfig::default(), }, admin: make_token_authorization_admin_config(), @@ -571,6 +582,7 @@ mod tests { insecure_http: true, payload_request_size: DEFAULT_PAYLOAD_REQUEST_SIZE, worker_count: None, + require_admin_auth_metrics: false, tls: TlsConfig::default(), }, admin: AdminConfig::InsecureAllowAll {}, @@ -613,6 +625,7 @@ mod tests { insecure_http: true, payload_request_size: DEFAULT_PAYLOAD_REQUEST_SIZE, worker_count: None, + require_admin_auth_metrics: false, tls: TlsConfig::default(), }, admin: AdminConfig::DenyAll {}, From 660b8013d9d79fa09c746b3c77be017eca5b5894 Mon Sep 17 00:00:00 2001 From: Rodney Osodo Date: Fri, 18 Sep 2026 13:52:05 +0300 Subject: [PATCH 3/3] fix(auth): restrict admin ACL allowed_endpoints to ^/kbs and ^/metrics Signed-off-by: Rodney Osodo --- integration-tests/src/common.rs | 14 ++++-- kbs/docs/admin.md | 2 +- kbs/docs/config.md | 2 +- kbs/docs/metrics.md | 12 ++--- kbs/src/admin/authorization/regex_acl.rs | 57 +++++++++++++++++++++++- 5 files changed, 75 insertions(+), 12 deletions(-) diff --git a/integration-tests/src/common.rs b/integration-tests/src/common.rs index bb25589bd3..64380cf69d 100644 --- a/integration-tests/src/common.rs +++ b/integration-tests/src/common.rs @@ -291,10 +291,16 @@ impl TestHarness { "authorization_mode": "AuthenticatedAuthorization", "authorization": { "regex_acl": { - "acls": [{ - "role": ADMIN_ROLE, - "allowed_endpoints": "^/(kbs/v0/.*|metrics)$" - }] + "acls": [ + { + "role": ADMIN_ROLE, + "allowed_endpoints": "^/kbs/v0/.*$" + }, + { + "role": ADMIN_ROLE, + "allowed_endpoints": "^/metrics$" + } + ] } }, "authentication": { diff --git a/kbs/docs/admin.md b/kbs/docs/admin.md index a4f0cd0eca..5ac5c497bd 100644 --- a/kbs/docs/admin.md +++ b/kbs/docs/admin.md @@ -39,7 +39,7 @@ This mode enables real admin authentication and authorization. - `authentication = bearer_jwt` verifies `Authorization: Bearer ` - JWT **MUST** contain a `role` claim - `authorization = regex_acl` authorizes by `acl(role -> allowed_endpoints)` -- `allowed_endpoints` must start with `^/` and end with `$` +- `allowed_endpoints` must start with `^/kbs` or `^/metrics` and end with `$` Example: diff --git a/kbs/docs/config.md b/kbs/docs/config.md index 6c10424840..885c257f12 100644 --- a/kbs/docs/config.md +++ b/kbs/docs/config.md @@ -315,7 +315,7 @@ Each ACL entry: | `role` | String | JWT `role` value to match | Yes | | `allowed_endpoints` | String | Regex of allowed request paths | Yes | -`allowed_endpoints` must start with `^/` and end with `$`. +`allowed_endpoints` must start with `^/kbs` or `^/metrics` and end with `$`. Each rule targets a single top-level namespace; grant access to both by adding one rule per namespace. ### Storage Backend Configuration diff --git a/kbs/docs/metrics.md b/kbs/docs/metrics.md index e4383aa353..45a3b162e9 100644 --- a/kbs/docs/metrics.md +++ b/kbs/docs/metrics.md @@ -5,11 +5,13 @@ The Key Broker Service (KBS) exposes Prometheus metrics on the `/metrics` HTTP e > **Access control**: By default `/metrics` is served without authentication. Set > `require_admin_auth_metrics = true` under the `http_server` section of your KBS > configuration to protect the endpoint with the [admin API](admin.md) authentication and -> authorization configuration. When enabled, scraping clients must present a valid admin JWT -> (see `admin.authentication`) that is allowed for the `/metrics` path by the configured ACL -> (e.g. an `allowed_endpoints` regex of `^/(kbs/v0/.*|metrics)$`). When the admin backend is -> `DenyAll`, `/metrics` is not accessible at all. This prevents unauthenticated disclosure of -> sensitive label values (resource paths, TEE types). +> authorization configuration. This is defense-in-depth for deployments that do not place an +> authenticating gateway in front of KBS; it is not a replacement for restricting network +> access to the metrics port. When enabled, scraping clients must present a valid admin JWT +> (see `admin.authentication`) that is allowed for the `/metrics` path by the configured ACL. +> Because each ACL rule targets a single top-level namespace, grant metrics with its own rule, +> e.g. an `allowed_endpoints` regex of `^/metrics$` alongside the existing `^/kbs/v0/.*$` rule. +> When the admin backend is `DenyAll`, `/metrics` is not accessible at all. The `/metrics` endpoint itself is excluded from request metrics collection to avoid skewing the data with monitoring traffic. diff --git a/kbs/src/admin/authorization/regex_acl.rs b/kbs/src/admin/authorization/regex_acl.rs index d0ec9fdceb..a4e9b97a83 100644 --- a/kbs/src/admin/authorization/regex_acl.rs +++ b/kbs/src/admin/authorization/regex_acl.rs @@ -33,6 +33,13 @@ struct AdminAclRuleEntry { role: String, } +/// Top-level URI namespaces that an admin ACL may target. Admin authentication is +/// intentionally limited to the KBS API (`/kbs`) and the optional admin-protected +/// Prometheus endpoint (`/metrics`), so that relaxing the anchoring check cannot +/// silently extend it to arbitrary endpoints. A rule spanning both namespaces must +/// be split into one [`AdminAclRule`] per namespace. +const ALLOWED_ENDPOINT_PREFIXES: [&str; 2] = ["^/kbs", "^/metrics"]; + pub struct RegexAclAuthorizer { acls: Vec, } @@ -42,7 +49,11 @@ impl TryFrom for RegexAclAuthorizer { fn try_from(config: RegexAclConfig) -> Result { let mut acls = Vec::new(); for acl in config.acls { - if !acl.allowed_endpoints.starts_with("^/") || !acl.allowed_endpoints.ends_with("$") { + let anchored = acl.allowed_endpoints.ends_with('$'); + let known_namespace = ALLOWED_ENDPOINT_PREFIXES + .iter() + .any(|prefix| acl.allowed_endpoints.starts_with(prefix)); + if !known_namespace || !anchored { return Err(Error::UnanchoredRegex); } let regex = Regex::new(&acl.allowed_endpoints)?; @@ -78,3 +89,47 @@ impl AuthorizationTrait for RegexAclAuthorizer { }) } } + +#[cfg(test)] +mod tests { + use super::*; + + fn config(allowed_endpoints: &str) -> RegexAclConfig { + RegexAclConfig { + acls: vec![AdminAclRule { + role: "admin".to_string(), + allowed_endpoints: allowed_endpoints.to_string(), + }], + } + } + + #[test] + fn accepts_anchored_kbs_and_metrics_regexes() { + for allowed in ["^/kbs/.+$", "^/kbs/v0/resource/.+$", "^/metrics$"] { + assert!( + RegexAclAuthorizer::try_from(config(allowed)).is_ok(), + "expected {allowed} to be accepted" + ); + } + } + + #[test] + fn rejects_unanchored_or_unknown_namespace_regexes() { + for rejected in [ + "metrics$", // not anchored at the start + "^/metrics", // not anchored at the end + "^/kbs/v0/.+", // not anchored at the end + "^/resource/.+$", // unknown top-level namespace + "^/healthz$", // health endpoints are not admin-scoped + "^/.*$", // would grant every path + ] { + assert!( + matches!( + RegexAclAuthorizer::try_from(config(rejected)), + Err(Error::UnanchoredRegex) + ), + "expected {rejected} to be rejected" + ); + } + } +}