Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions integration-tests/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
31 changes: 26 additions & 5 deletions integration-tests/src/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,18 +102,22 @@ impl From<KbsConfigType> 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,
},
}
}
Expand All @@ -123,6 +127,7 @@ impl From<KbsConfigType> for TestParameters {
pub struct TestParameters {
pub rvps_type: RvpsType,
pub admin_type: AdminType,
pub require_admin_auth_metrics: bool,
}

/// Internal state of tests
Expand Down Expand Up @@ -185,7 +190,7 @@ async fn wait_for_reference_value(url: &str, key: &str) -> Result<()> {
}

impl TestHarness {
fn sign_admin_token(&self) -> Result<String> {
pub fn sign_admin_token(&self) -> Result<String> {
let encoding_key = EncodingKey::from_ed_pem(self.auth_privkey.as_bytes())?;
let now = SystemTime::now()
.duration_since(UNIX_EPOCH)
Expand Down Expand Up @@ -286,10 +291,16 @@ impl TestHarness {
"authorization_mode": "AuthenticatedAuthorization",
"authorization": {
"regex_acl": {
"acls": [{
"role": ADMIN_ROLE,
"allowed_endpoints": "^/kbs/v0/.*$"
}]
"acls": [
{
"role": ADMIN_ROLE,
"allowed_endpoints": "^/kbs/v0/.*$"
},
{
"role": ADMIN_ROLE,
"allowed_endpoints": "^/metrics$"
}
]
}
},
"authentication": {
Expand Down Expand Up @@ -349,6 +360,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,
Expand Down Expand Up @@ -504,6 +516,15 @@ impl TestHarness {

Ok(payload)
}

pub async fn get_metrics(&self, admin_token: Option<String>) -> Result<reqwest::Response> {
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();
Expand Down
107 changes: 106 additions & 1 deletion integration-tests/tests/admin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};

//
Expand Down Expand Up @@ -192,6 +192,111 @@ import rego.v1
default executables = 97
";

//
// 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)]
#[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 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"));
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(())
}

//
// 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.
Expand Down
2 changes: 1 addition & 1 deletion kbs/docs/admin.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ This mode enables real admin authentication and authorization.
- `authentication = bearer_jwt` verifies `Authorization: Bearer <JWT>`
- 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 `^/kbs` or `^/metrics` and end with `$`

Example:

Expand Down
3 changes: 2 additions & 1 deletion kbs/docs/config.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 |
Expand Down Expand Up @@ -314,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 `^/kbs` 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

Expand Down
15 changes: 15 additions & 0 deletions kbs/docs/metrics.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,16 @@

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**: 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. 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.
Expand Down Expand Up @@ -69,6 +79,11 @@ scrape_configs:
scheme: https # or http if using insecure_http
tls_config:
insecure_skip_verify: true # only if using self-signed certificates
# Only needed when http_server.require_admin_auth_metrics = true:
# present a valid admin JWT.
authorization:
type: Bearer
credentials_file: /path/to/admin.jwt
Comment on lines +82 to +86

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we still need this?

```

## Kubernetes Deployment
Expand Down
58 changes: 56 additions & 2 deletions kbs/src/admin/authorization/regex_acl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<AdminAclRuleEntry>,
}
Expand All @@ -42,8 +49,11 @@ impl TryFrom<RegexAclConfig> for RegexAclAuthorizer {
fn try_from(config: RegexAclConfig) -> Result<Self> {
let mut acls = Vec::new();
for acl in config.acls {
if !acl.allowed_endpoints.starts_with("^/kbs") || !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)?;
Expand Down Expand Up @@ -79,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"
);
}
}
}
14 changes: 12 additions & 2 deletions kbs/src/api_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -476,9 +476,19 @@ pub(crate) async fn api(
}

pub(crate) async fn prometheus_metrics_handler(
_request: HttpRequest,
_core: web::Data<ApiServer>,
request: HttpRequest,
core: web::Data<ApiServer>,
) -> Result<HttpResponse> {
// 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))
Expand Down
Loading
Loading