diff --git a/filter/src/builtins/http/security/basic_auth/config.rs b/filter/src/builtins/http/security/basic_auth/config.rs index a3b766cd8..bf3912a52 100644 --- a/filter/src/builtins/http/security/basic_auth/config.rs +++ b/filter/src/builtins/http/security/basic_auth/config.rs @@ -160,6 +160,9 @@ impl TryFrom for InlineCredential { if raw.username.trim().is_empty() { return Err("username must not be empty".to_owned()); } + if raw.username.len() > 256 { + return Err("username must not exceed 256 bytes".to_owned()); + } Ok(Self { username: raw.username, @@ -229,6 +232,21 @@ credentials: ); } + #[test] + fn accepts_username_at_metadata_limit() { + let username = "u".repeat(256); + let config = format!("credentials:\n - username: {username}\n password: secret\n"); + serde_yaml::from_str::(&config).expect("256-byte username should be accepted"); + } + + #[test] + fn rejects_username_over_metadata_limit() { + let username = "u".repeat(257); + let config = format!("credentials:\n - username: {username}\n password: secret\n"); + let error = serde_yaml::from_str::(&config).expect_err("257-byte username should be rejected"); + assert!(error.to_string().contains("256 bytes")); + } + #[test] fn default_realm_is_restricted() { let cfg: BasicAuthConfig = serde_yaml::from_str( diff --git a/filter/src/builtins/http/security/basic_auth/filter.rs b/filter/src/builtins/http/security/basic_auth/filter.rs index 08b6c45af..b84405662 100644 --- a/filter/src/builtins/http/security/basic_auth/filter.rs +++ b/filter/src/builtins/http/security/basic_auth/filter.rs @@ -15,7 +15,7 @@ use subtle::ConstantTimeEq as _; use super::config::{BasicAuthConfig, CredentialSourceConfig, InlineCredential}; use crate::{ - FilterAction, FilterError, Rejection, + FilterAction, FilterError, IDENTITY_USER_ID_METADATA, Rejection, factory::parse_filter_config, filter::{HttpFilter, HttpFilterContext}, }; @@ -169,6 +169,18 @@ impl HttpFilter for BasicAuthFilter { tracing::debug!(username = %username, "authentication successful"); + // The authenticated principal is trusted only after verification. + // Keep it bounded because filter metadata is request-scoped and can be + // consumed by downstream admission and routing filters. + if username.len() > 256 { + tracing::debug!("authenticated username exceeds metadata bounds"); + return Ok(challenge_rejection(&self.challenge)); + } + if let Err(error) = ctx.try_set_metadata(IDENTITY_USER_ID_METADATA, username) { + tracing::debug!(?error, "authenticated principal could not be published"); + return Ok(challenge_rejection(&self.challenge)); + } + if self.strip_authorization { ctx.request_headers_to_remove.push(http::header::AUTHORIZATION); } diff --git a/filter/src/builtins/http/security/basic_auth/tests.rs b/filter/src/builtins/http/security/basic_auth/tests.rs index 67e4995f7..59ba50098 100644 --- a/filter/src/builtins/http/security/basic_auth/tests.rs +++ b/filter/src/builtins/http/security/basic_auth/tests.rs @@ -214,6 +214,44 @@ async fn authenticates_valid_credentials() { matches!(action, FilterAction::Continue), "valid credentials should continue" ); + assert_eq!( + ctx.get_metadata(crate::IDENTITY_USER_ID_METADATA), + Some("admin"), + "successful authentication should publish the generic trusted identity" + ); +} + +#[tokio::test] +async fn authenticates_username_at_metadata_limit() { + let username = "u".repeat(256); + let f = make_filter(&[(&username, "fakecreds")], "Restricted"); + let mut req = crate::test_utils::make_request(http::Method::GET, "/"); + req.headers + .insert(http::header::AUTHORIZATION, basic_header(&username, "fakecreds")); + let mut ctx = crate::test_utils::make_filter_context(&req); + + let action = f.on_request(&mut ctx).await.unwrap(); + assert!(matches!(action, FilterAction::Continue)); + assert_eq!( + ctx.get_metadata(crate::IDENTITY_USER_ID_METADATA), + Some(username.as_str()) + ); +} + +#[tokio::test] +async fn rejects_authenticated_principal_when_metadata_capacity_is_exhausted() { + let f = make_filter(&[("admin", "fakecreds")], "Restricted"); + let mut req = crate::test_utils::make_request(http::Method::GET, "/"); + req.headers + .insert(http::header::AUTHORIZATION, basic_header("admin", "fakecreds")); + let mut ctx = crate::test_utils::make_filter_context(&req); + for i in 0..128 { + ctx.set_metadata(format!("test.key.{i}"), "value"); + } + + let action = f.on_request(&mut ctx).await.unwrap(); + assert_rejection_with_challenge(&action, "Restricted"); + assert!(ctx.get_metadata(crate::IDENTITY_USER_ID_METADATA).is_none()); } #[tokio::test] @@ -224,6 +262,10 @@ async fn rejects_missing_authorization_header() { let action = f.on_request(&mut ctx).await.unwrap(); assert_rejection_with_challenge(&action, "TestRealm"); + assert!( + ctx.get_metadata(crate::IDENTITY_USER_ID_METADATA).is_none(), + "missing credentials must not publish identity" + ); } #[tokio::test] @@ -291,6 +333,10 @@ async fn rejects_wrong_password() { matches!(&action, FilterAction::Reject(r) if r.status == 401), "wrong password should return 401" ); + assert!( + ctx.get_metadata(crate::IDENTITY_USER_ID_METADATA).is_none(), + "invalid credentials must not publish identity" + ); } #[tokio::test] @@ -434,6 +480,33 @@ async fn kv_store_lookup_valid_credentials() { ); } +#[tokio::test] +async fn kv_store_rejects_username_over_metadata_limit_without_publishing_identity() { + let username = "u".repeat(257); + let yaml = yaml("kv_store: auth_users"); + let f = BasicAuthFilter::from_config(&yaml).unwrap(); + + let registry = KvStoreRegistry::new(); + let store = registry.get_or_create("auth_users"); + store.set(&username, Arc::from("fakecreds")); + + let mut req = crate::test_utils::make_request(http::Method::GET, "/"); + req.headers + .insert(http::header::AUTHORIZATION, basic_header(&username, "fakecreds")); + let mut ctx = crate::test_utils::make_filter_context(&req); + ctx.kv_stores = Some(®istry); + + let action = f.on_request(&mut ctx).await.unwrap(); + assert!( + matches!(&action, FilterAction::Reject(rejection) if rejection.status == 401), + "an oversized KV-backed username must be rejected" + ); + assert!( + ctx.get_metadata(crate::IDENTITY_USER_ID_METADATA).is_none(), + "an oversized username must not publish trusted identity metadata" + ); +} + #[tokio::test] async fn kv_store_missing_store_rejects() { let yaml = yaml("kv_store: nonexistent_store"); diff --git a/filter/src/context.rs b/filter/src/context.rs index fd5bcf367..598cebd46 100644 --- a/filter/src/context.rs +++ b/filter/src/context.rs @@ -31,6 +31,20 @@ const MAX_STRUCTURED_METADATA_KEYS: usize = 64; /// insert thousands of unique keys per request. const MAX_METADATA_ENTRIES: usize = 128; +/// Failure returned when a request metadata value cannot be stored. +#[derive(Clone, Copy, Debug, Eq, PartialEq, thiserror::Error)] +pub enum MetadataError { + /// The metadata key is empty or exceeds the key-size limit. + #[error("metadata key is empty or exceeds the 64-byte limit")] + InvalidKey, + /// The metadata value exceeds the value-size limit. + #[error("metadata value exceeds the 256-byte limit")] + ValueTooLong, + /// The request has reached its metadata-entry limit. + #[error("request metadata entry limit reached")] + Capacity, +} + /// Trusted header mutation recorded during pre-read body processing. /// /// Pre-read filters run *before* the request-phase pipeline. Mutations @@ -375,15 +389,28 @@ impl HttpFilterContext<'_> { /// 64 bytes and values to 256 bytes to bound per-request /// memory growth. pub fn set_metadata(&mut self, key: impl Into, value: impl Into) { + self.try_set_metadata(key, value).unwrap_or(()); + } + + /// Write metadata and report whether it was stored. + /// + /// This is the fallible form for filters whose security or routing + /// contract requires the metadata to be present before continuing. + /// + /// # Errors + /// + /// Returns [`MetadataError`] when the key or value exceeds its size limit, + /// or when the request metadata entry limit has been reached. + pub fn try_set_metadata(&mut self, key: impl Into, value: impl Into) -> Result<(), MetadataError> { let key = key.into(); let value = value.into(); if key.is_empty() || key.len() > 64 { tracing::warn!(key_len = key.len(), "metadata key rejected (must be 1-64 bytes)"); - return; + return Err(MetadataError::InvalidKey); } if value.len() > 256 { tracing::warn!(key = %key, value_len = value.len(), "metadata value rejected (max 256 bytes)"); - return; + return Err(MetadataError::ValueTooLong); } if !self.filter_metadata.contains_key(&key) && self.filter_metadata.len() >= MAX_METADATA_ENTRIES { tracing::warn!( @@ -391,9 +418,10 @@ impl HttpFilterContext<'_> { entries = self.filter_metadata.len(), "metadata entry rejected (max {MAX_METADATA_ENTRIES} entries)" ); - return; + return Err(MetadataError::Capacity); } self.filter_metadata.insert(key, value); + Ok(()) } /// Upgrade the request body delivery mode for this request. @@ -1066,7 +1094,11 @@ mod tests { "should accept exactly {MAX_METADATA_ENTRIES} entries" ); - ctx.set_metadata("overflow", "value"); + assert_eq!( + ctx.try_set_metadata("overflow", "value"), + Err(MetadataError::Capacity), + "entry beyond limit should report capacity failure" + ); assert!( ctx.get_metadata("overflow").is_none(), "entry beyond limit should be rejected" diff --git a/filter/src/lib.rs b/filter/src/lib.rs index ebfd11cbe..3cb574cac 100644 --- a/filter/src/lib.rs +++ b/filter/src/lib.rs @@ -52,7 +52,8 @@ pub use builtins::{ pub use builtins::{PolicyFilter, PolicyPluginFactoryFn, register_policy_plugin_factory}; pub use condition::{should_execute, should_execute_response, should_execute_response_ref}; pub use context::{ - HttpFilterContext, PendingHeaderResult, Request, Response, SubRequestResponseMode, TrustedHeaderMutation, + HttpFilterContext, MetadataError, PendingHeaderResult, Request, Response, SubRequestResponseMode, + TrustedHeaderMutation, }; pub use error_response::{ ErrorResponseContext, ErrorResponseFormatter, ErrorResponseFormatterHandle, FormattedErrorResponse, @@ -77,6 +78,13 @@ pub use registry::{FilterRegistry, SecurityClass}; pub use results::{FilterResultSet, matches_filter_result}; pub use tcp_filter::{TcpFilter, TcpFilterContext}; +/// Trusted metadata key for the authenticated principal identifier. +/// +/// Authentication filters write this key only after successfully verifying +/// credentials. Downstream filters may use it as a generic identity contract; +/// it is intentionally independent of the authentication mechanism. +pub const IDENTITY_USER_ID_METADATA: &str = "identity.user_id"; + // ----------------------------------------------------------------------------- // Custom Filter Registration // -----------------------------------------------------------------------------