diff --git a/docs/cedarling/reference/cedarling-properties.md b/docs/cedarling/reference/cedarling-properties.md index a89bb4f2e53..9e5c519a979 100644 --- a/docs/cedarling/reference/cedarling-properties.md +++ b/docs/cedarling/reference/cedarling-properties.md @@ -46,6 +46,12 @@ To load the policy store, one of the following properties must be set: - **`CEDARLING_POLICY_STORE_REFRESH_INTERVAL`** : Background refresh interval in seconds for URL-based policy store sources (`CEDARLING_POLICY_STORE_URI` pointing at a Cedar Archive URL). When set to a non-zero value, Cedarling spawns a worker that periodically re-fetches the policy store and atomically swaps the in-memory `Authz` instance when the upstream changes. A server-side `Cache-Control: max-age` / `Expires` hint may *shorten* the next interval but never extends it. Default is `0` (refresh disabled — load-once-at-startup behavior). Non-zero values below `5` seconds are clamped to `5`. Ignored for local sources (`CEDARLING_POLICY_STORE_LOCAL_FN`, `CEDARLING_POLICY_STORE_LOCAL`). See [Background refresh](./cedarling-policy-store.md#background-refresh) for the per-request consistency model, the strategy ladder, and the emitted metric keys. +### Limiting policy store size + +- **`CEDARLING_POLICY_STORE_MAX_FILE_SIZE`** : Maximum decompressed size, in bytes, of a single file inside a Cedar Archive (`.cjar`). Archives are ZIP files, so a small download can expand into a very large buffer in memory (a "zip bomb"); an archive whose entry exceeds this limit is rejected with an error rather than being decompressed. The whole-archive decompressed size is capped at ten times this value, and an archive may hold at most 10000 entries. Set to `0` to disable the size caps. Default is `10485760` (10 MB). + + This limit is independent of `CEDARLING_HTTP_MAX_RESPONSE_SIZE_BYTES`, which bounds the compressed archive while it is downloaded. Serving a `.cjar` over HTTP whose compressed size exceeds `CEDARLING_HTTP_MAX_RESPONSE_SIZE_BYTES` (10 MB by default) requires raising that property as well. + ### Optional properties Properties listed here are optional. If a property value is not set, diff --git a/jans-cedarling/cedarling/config/default_config.yaml b/jans-cedarling/cedarling/config/default_config.yaml index 3f28bc50c4c..bd56e2b6cff 100644 --- a/jans-cedarling/cedarling/config/default_config.yaml +++ b/jans-cedarling/cedarling/config/default_config.yaml @@ -6,6 +6,8 @@ CEDARLING_LOG_TTL: 60 CEDARLING_LOCAL_JWKS: null CEDARLING_POLICY_STORE_LOCAL: null CEDARLING_POLICY_STORE_LOCAL_FN: "../config/policy-store.cjar" +# Cap on the decompressed size of a single .cjar entry. +CEDARLING_POLICY_STORE_MAX_FILE_SIZE: 10485760 CEDARLING_JWT_SIG_VALIDATION: "enabled" CEDARLING_JWT_STATUS_VALIDATION: "enabled" CEDARLING_STRICT_SCHEMA_VALIDATION: "enabled" diff --git a/jans-cedarling/cedarling/src/bootstrap_config/decode.rs b/jans-cedarling/cedarling/src/bootstrap_config/decode.rs index 511fb2db3b5..8f438e2996a 100644 --- a/jans-cedarling/cedarling/src/bootstrap_config/decode.rs +++ b/jans-cedarling/cedarling/src/bootstrap_config/decode.rs @@ -145,17 +145,20 @@ fn build_policy_store_config( Ok(PolicyStoreConfig { source: PolicyStoreSource::Yaml(policy_store), refresh_interval_secs: raw.policy_store_refresh_interval_secs, + max_file_size: raw.policy_store_max_file_size, }) - } + }, // Case: get the policy store from a URI (None, Some(policy_store_uri), None, None) => Ok(PolicyStoreConfig { source: PolicyStoreSource::Uri(policy_store_uri), refresh_interval_secs: raw.policy_store_refresh_interval_secs, + max_file_size: raw.policy_store_max_file_size, }), // Case: get the policy store from a CjarUrl (None, None, None, Some(policy_store_cjar_url)) => Ok(PolicyStoreConfig { source: PolicyStoreSource::CjarUrl(policy_store_cjar_url), refresh_interval_secs: raw.policy_store_refresh_interval_secs, + max_file_size: raw.policy_store_max_file_size, }), // Case: get the policy store from a local file or directory (None, None, Some(raw_path), None) => { @@ -168,7 +171,9 @@ fn build_policy_store_config( .and_then(|ext| ext.to_str()) .map(str::to_lowercase); match file_ext.as_deref() { - Some("json") => return Err(BootstrapConfigLoadingError::LegacyJsonNotSupported), + Some("json") => { + return Err(BootstrapConfigLoadingError::LegacyJsonNotSupported); + }, Some("yaml" | "yml") => PolicyStoreSource::FileYaml(path.into()), Some("cjar") => PolicyStoreSource::CjarFile(path.into()), _ => { @@ -181,6 +186,7 @@ fn build_policy_store_config( Ok(PolicyStoreConfig { source, refresh_interval_secs: raw.policy_store_refresh_interval_secs, + max_file_size: raw.policy_store_max_file_size, }) }, // Case: multiple policy stores were set @@ -247,6 +253,130 @@ fn resolve_log_type( #[cfg(test)] mod tests { use super::*; + use crate::common::policy_store::archive_handler::ArchiveLimits; + + /// Minimal config body; a policy store source is required, and the inline + /// YAML source keeps these tests off the filesystem and network. Inline + /// JSON is rejected as a legacy policy store, so the value must be YAML. + fn raw_config_json(extra: &str) -> String { + format!( + r#"{{ + "CEDARLING_APPLICATION_NAME": "test", + "CEDARLING_POLICY_STORE_LOCAL": "cedar_version: v4.0.0\npolicy_stores: {{}}" + {extra} + }}"# + ) + } + + fn decode(extra: &str) -> BootstrapConfig { + let raw: BootstrapConfigRaw = serde_json::from_str(&raw_config_json(extra)) + .expect("raw bootstrap config should deserialize"); + BootstrapConfig::try_from(raw).expect("raw config should decode") + } + + #[test] + fn archive_cap_does_not_move_the_http_cap() { + // The two caps bound different memory (decompressed output vs. buffered + // response body), so tuning the archive cap must leave every other + // HTTP fetch on its own default. + let http_default = Some(HttpClientConfig::DEFAULT_MAX_RESPONSE_SIZE_BYTES); + + let neither = decode(""); + assert_eq!( + neither.http_client_config.max_response_size_bytes, http_default, + "With neither property set the HTTP cap must use its own default" + ); + assert_eq!( + neither.policy_store_config.max_file_size, + ArchiveLimits::DEFAULT_MAX_ENTRY_SIZE, + "With neither property set the archive cap must use its own default" + ); + + let small = decode(r#", "CEDARLING_POLICY_STORE_MAX_FILE_SIZE": 4096"#); + assert_eq!( + small.http_client_config.max_response_size_bytes, http_default, + "A small archive cap must not shrink unrelated HTTP responses" + ); + assert_eq!( + small.policy_store_config.max_file_size, 4096, + "The archive cap must carry the configured value" + ); + + let disabled = decode(r#", "CEDARLING_POLICY_STORE_MAX_FILE_SIZE": 0"#); + assert_eq!( + disabled.http_client_config.max_response_size_bytes, http_default, + "Disabling the archive cap must not remove the HTTP cap" + ); + } + + #[test] + fn http_cap_does_not_move_the_archive_cap() { + let explicit = decode( + r#", "CEDARLING_POLICY_STORE_MAX_FILE_SIZE": 4096, + "CEDARLING_HTTP_MAX_RESPONSE_SIZE_BYTES": 8192"#, + ); + assert_eq!( + explicit.http_client_config.max_response_size_bytes, + Some(8192), + "An explicit HTTP cap must be used as given" + ); + assert_eq!( + explicit.policy_store_config.max_file_size, 4096, + "An explicit HTTP cap must not disturb the archive cap" + ); + + let disabled = decode( + r#", "CEDARLING_POLICY_STORE_MAX_FILE_SIZE": 4096, + "CEDARLING_HTTP_MAX_RESPONSE_SIZE_BYTES": 0"#, + ); + assert_eq!( + disabled.http_client_config.max_response_size_bytes, None, + "An explicit 0 must disable the HTTP cap" + ); + assert_eq!( + disabled.policy_store_config.max_file_size, 4096, + "Disabling the HTTP cap must leave the archive cap enforced" + ); + } + + #[test] + fn max_file_size_is_honored_from_json_and_yaml() { + // Env is covered by the `raw_config` tests; these two are the remaining + // documented input formats. + let from_json = BootstrapConfig::load_from_json(&raw_config_json( + r#", "CEDARLING_POLICY_STORE_MAX_FILE_SIZE": 512"#, + )) + .expect("JSON bootstrap config should load"); + assert_eq!( + from_json.policy_store_config.max_file_size, 512, + "The policy store cap must be honored from JSON" + ); + + let yaml = concat!( + "CEDARLING_APPLICATION_NAME: test\n", + "CEDARLING_POLICY_STORE_LOCAL: 'cedar_version: v4.0.0'\n", + "CEDARLING_POLICY_STORE_MAX_FILE_SIZE: 512\n", + ); + let raw: BootstrapConfigRaw = + serde_yaml_ng::from_str(yaml).expect("YAML bootstrap config should deserialize"); + let from_yaml = BootstrapConfig::try_from(raw).expect("YAML config should decode"); + assert_eq!( + from_yaml.policy_store_config.max_file_size, 512, + "The policy store cap must be honored from YAML" + ); + } + + #[test] + fn max_file_size_is_honored_from_a_string_valued_env_var() { + // Env vars always arrive as strings, so the numeric properties go + // through `deserialize_or_parse_string_as_json`. + let config = decode(r#", "CEDARLING_POLICY_STORE_MAX_FILE_SIZE": "4096""#); + + assert_eq!( + config.policy_store_config.max_file_size, 4096, + "A string-valued cap must parse to the same number" + ); + } #[test] fn test_reject_legacy_json_inline() { @@ -261,8 +391,7 @@ mod tests { local_policy_store: Some(case.to_string()), ..Default::default() }; - let err = build_policy_store_config(&raw) - .expect_err("legacy JSON must be rejected"); + let err = build_policy_store_config(&raw).expect_err("legacy JSON must be rejected"); assert!( matches!(err, BootstrapConfigLoadingError::LegacyJsonNotSupported), "expected LegacyJsonNotSupported for input: {case}, got {err:?}" @@ -294,4 +423,3 @@ mod tests { } } } - diff --git a/jans-cedarling/cedarling/src/bootstrap_config/mod.rs b/jans-cedarling/cedarling/src/bootstrap_config/mod.rs index d253178f4ee..218d173b351 100644 --- a/jans-cedarling/cedarling/src/bootstrap_config/mod.rs +++ b/jans-cedarling/cedarling/src/bootstrap_config/mod.rs @@ -290,6 +290,22 @@ mod tests { use super::*; + #[test] + fn test_default_config_ships_independent_size_caps() { + let config = BootstrapConfig::load_default().unwrap(); + + assert_eq!( + config.policy_store_config.max_file_size, + crate::common::policy_store::archive_handler::ArchiveLimits::DEFAULT_MAX_ENTRY_SIZE, + "The shipped default config must carry the bundled archive cap" + ); + assert_eq!( + config.http_client_config.max_response_size_bytes, + Some(crate::HttpClientConfig::DEFAULT_MAX_RESPONSE_SIZE_BYTES), + "The shipped default config must use the HTTP cap's own default" + ); + } + #[test] fn test_load_default_config() { let config = BootstrapConfig::load_default().unwrap(); diff --git a/jans-cedarling/cedarling/src/bootstrap_config/policy_store_config.rs b/jans-cedarling/cedarling/src/bootstrap_config/policy_store_config.rs index 1582b64d56d..4ca3462c935 100644 --- a/jans-cedarling/cedarling/src/bootstrap_config/policy_store_config.rs +++ b/jans-cedarling/cedarling/src/bootstrap_config/policy_store_config.rs @@ -7,6 +7,7 @@ use serde::{Deserialize, Serialize}; use std::path::PathBuf; use crate::bootstrap_config::BootstrapConfigLoadingError; +use crate::common::policy_store::archive_handler::ArchiveLimits; /// `PolicyStoreConfig` - Configuration for the policy store. /// @@ -23,6 +24,17 @@ pub struct PolicyStoreConfig { /// interval but never lengthens it. #[serde(default)] pub refresh_interval_secs: u64, + + /// `CEDARLING_POLICY_STORE_MAX_FILE_SIZE` — cap on the decompressed size of + /// a single `.cjar` entry, in bytes. `0` disables the cap. + #[serde(default = "default_policy_store_max_file_size")] + pub max_file_size: u64, +} + +/// Serde default for [`PolicyStoreConfig::max_file_size`], so a config +/// deserialized without the field still gets a bounded archive loader. +pub(crate) fn default_policy_store_max_file_size() -> u64 { + ArchiveLimits::DEFAULT_MAX_ENTRY_SIZE } impl PolicyStoreConfig { @@ -59,6 +71,14 @@ impl PolicyStoreConfig { } } +impl PolicyStoreConfig { + /// Resource limits for `.cjar` loading, derived from [`Self::max_file_size`]. + #[must_use] + pub(crate) fn archive_limits(&self) -> ArchiveLimits { + ArchiveLimits::from_max_file_size(self.max_file_size) + } +} + impl Default for PolicyStoreConfig { fn default() -> Self { Self { @@ -66,6 +86,7 @@ impl Default for PolicyStoreConfig { "cedar_version: v4.0.0\npolicy_stores: {}\n".to_string(), ), refresh_interval_secs: 0, + max_file_size: default_policy_store_max_file_size(), } } } @@ -184,6 +205,7 @@ impl TryFrom for PolicyStoreConfig { Ok(Self { source, refresh_interval_secs: 0, + max_file_size: default_policy_store_max_file_size(), }) } } diff --git a/jans-cedarling/cedarling/src/bootstrap_config/raw_config/config.rs b/jans-cedarling/cedarling/src/bootstrap_config/raw_config/config.rs index ccb1e11231d..3b6c0e52407 100644 --- a/jans-cedarling/cedarling/src/bootstrap_config/raw_config/config.rs +++ b/jans-cedarling/cedarling/src/bootstrap_config/raw_config/config.rs @@ -10,8 +10,8 @@ use super::default_values::{ default_enabled_feature_toggle, default_http_client_max_response_size_bytes, default_http_client_max_retries, default_http_client_retry_delay_secs, default_jti, default_jwks_refresh_min_interval, default_log_channel_capacity, default_log_max_retries, - default_status_list_refresh_interval_max, default_token_cache_capacity, - default_token_cache_max_ttl, default_true, + default_policy_store_max_file_size, default_status_list_refresh_interval_max, + default_token_cache_capacity, default_token_cache_max_ttl, default_true, }; #[cfg(not(target_arch = "wasm32"))] use super::default_values::{ @@ -505,6 +505,16 @@ pub struct BootstrapConfigRaw { #[serde(rename = "CEDARLING_POLICY_STORE_REFRESH_INTERVAL", default)] #[serde(deserialize_with = "deserialize_or_parse_string_as_json")] pub policy_store_refresh_interval_secs: u64, + + /// Maximum decompressed size, in bytes, of a single entry inside a `.cjar` + /// policy store archive. Bounds zip-bomb expansion. `0` disables the cap. + /// Default: 10 MB (`10485760`). + #[serde( + rename = "CEDARLING_POLICY_STORE_MAX_FILE_SIZE", + default = "default_policy_store_max_file_size", + deserialize_with = "deserialize_or_parse_string_as_json" + )] + pub policy_store_max_file_size: u64, } impl Default for BootstrapConfigRaw { @@ -552,6 +562,7 @@ fn get_cedarling_env_vars() -> HashMap { #[cfg(test)] mod tests { use super::*; + use crate::common::policy_store::archive_handler::ArchiveLimits; use crate::jwt_config::{MIN_JWKS_REFRESH_SECS, MIN_STATUS_LIST_REFRESH_SECS}; use std::{ env, @@ -835,6 +846,62 @@ mod tests { ); } + #[test] + fn test_policy_store_max_file_size_and_http_cap_default_independently() { + with_env_vars(&[], || { + let config = BootstrapConfigRaw::from_raw_config_and_env(None).unwrap(); + + assert_eq!( + config.policy_store_max_file_size, + ArchiveLimits::DEFAULT_MAX_ENTRY_SIZE, + "Policy store max file size should default to 10 MB" + ); + assert_eq!( + config.http_client_max_response_size_bytes, + crate::HttpClientConfig::DEFAULT_MAX_RESPONSE_SIZE_BYTES, + "An unset HTTP cap must use its own default, not the archive cap" + ); + }); + } + + #[test] + fn test_policy_store_max_file_size_from_env_var() { + with_env_vars(&[("CEDARLING_POLICY_STORE_MAX_FILE_SIZE", "2048")], || { + let config = BootstrapConfigRaw::from_raw_config_and_env(None).unwrap(); + + assert_eq!( + config.policy_store_max_file_size, 2048, + "Policy store max file size should match environment value" + ); + }); + } + + #[test] + fn test_policy_store_max_file_size_rejects_invalid_values() { + // `0` is the documented disable sentinel and stays valid; anything that + // isn't a non-negative integer must fail the bootstrap rather than + // silently fall back to the default cap. + with_env_vars(&[("CEDARLING_POLICY_STORE_MAX_FILE_SIZE", "0")], || { + let config = BootstrapConfigRaw::from_raw_config_and_env(None) + .expect("0 is the disable sentinel and must be accepted"); + assert_eq!( + config.policy_store_max_file_size, 0, + "0 must be preserved rather than replaced by the default" + ); + }); + + for invalid in ["-1", "not-a-number"] { + with_env_vars(&[("CEDARLING_POLICY_STORE_MAX_FILE_SIZE", invalid)], || { + let err = BootstrapConfigRaw::from_raw_config_and_env(None) + .expect_err("a non-u64 max file size must be rejected"); + assert!( + matches!(err, BootstrapConfigLoadingError::DecodingJSON(_)), + "expected a decoding error for input {invalid:?}, got {err:?}" + ); + }); + } + } + #[test] fn test_jwks_refresh_interval_from_env_var() { with_env_vars( diff --git a/jans-cedarling/cedarling/src/bootstrap_config/raw_config/default_values.rs b/jans-cedarling/cedarling/src/bootstrap_config/raw_config/default_values.rs index 64b5dadd02e..35456e8dcf0 100644 --- a/jans-cedarling/cedarling/src/bootstrap_config/raw_config/default_values.rs +++ b/jans-cedarling/cedarling/src/bootstrap_config/raw_config/default_values.rs @@ -6,6 +6,7 @@ //! In this file we define functions for serde `default` macro. use super::feature_types::FeatureToggle; +use crate::common::policy_store::archive_handler::ArchiveLimits; #[cfg(not(target_arch = "wasm32"))] use crate::log::StdOutLoggerMode; use crate::{HttpClientConfig, JwtConfig, lock_config::LockServiceConfig}; @@ -74,3 +75,7 @@ pub(super) fn default_http_client_retry_delay_secs() -> u64 { pub(super) fn default_http_client_max_response_size_bytes() -> u64 { HttpClientConfig::DEFAULT_MAX_RESPONSE_SIZE_BYTES } + +pub(super) fn default_policy_store_max_file_size() -> u64 { + ArchiveLimits::DEFAULT_MAX_ENTRY_SIZE +} diff --git a/jans-cedarling/cedarling/src/common/policy_store/archive_handler.rs b/jans-cedarling/cedarling/src/common/policy_store/archive_handler.rs index caf553b2734..80e6f1ce5de 100644 --- a/jans-cedarling/cedarling/src/common/policy_store/archive_handler.rs +++ b/jans-cedarling/cedarling/src/common/policy_store/archive_handler.rs @@ -26,8 +26,53 @@ use std::io::{Cursor, Read, Seek}; #[cfg(not(target_arch = "wasm32"))] use std::path::Path; use std::sync::Mutex; +use std::sync::atomic::{AtomicU64, Ordering}; use zip::ZipArchive; +/// Resource limits bounding what [`ArchiveVfs`] will decompress into memory. +/// A compressed `.cjar` can expand arbitrarily; these turn an OOM into a typed +/// [`ArchiveError`]. A limit of `0` disables that check. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +// The shared `max_` prefix marks these as caps rather than measurements, which +// is worth more at the use sites than satisfying `struct_field_names`. +#[allow(clippy::struct_field_names)] +pub(crate) struct ArchiveLimits { + /// Maximum decompressed size of a single entry, in bytes. + pub max_entry_size: u64, + /// Maximum combined decompressed size of every entry, in bytes. + pub max_total_size: u64, + /// Maximum number of entries in the archive. + pub max_entries: usize, +} + +impl ArchiveLimits { + /// Matches the 10 MB cap `StatusList::parse` applies to status lists. + pub(crate) const DEFAULT_MAX_ENTRY_SIZE: u64 = 10 * 1024 * 1024; + + /// Derived rather than configured, so raising the per-entry cap scales the + /// total with it instead of tripping a fixed ceiling. + const TOTAL_SIZE_RATIO: u64 = 10; + + /// Also bounds the O(n) scans in `read_dir` / `is_directory_locked`. + const MAX_ENTRIES: usize = 10_000; + + /// Build limits from a `CEDARLING_POLICY_STORE_MAX_FILE_SIZE` value. `0` + /// disables both size caps; the entry-count cap always applies. + pub(crate) fn from_max_file_size(max_entry_size: u64) -> Self { + Self { + max_entry_size, + max_total_size: max_entry_size.saturating_mul(Self::TOTAL_SIZE_RATIO), + max_entries: Self::MAX_ENTRIES, + } + } +} + +impl Default for ArchiveLimits { + fn default() -> Self { + Self::from_max_file_size(Self::DEFAULT_MAX_ENTRY_SIZE) + } +} + /// VFS implementation backed by a ZIP archive. /// /// This implementation reads files on-demand from a ZIP archive without extraction, @@ -49,6 +94,13 @@ use zip::ZipArchive; pub(super) struct ArchiveVfs { /// The ZIP archive reader (wrapped in Mutex for thread safety) archive: Mutex>, + /// Resource limits enforced at construction and on every `read_file`. + limits: ArchiveLimits, + /// Bytes actually decompressed so far, charged against + /// `limits.max_total_size`. The construction-time total can only use + /// author-controlled central-directory sizes, so this is where the + /// whole-archive guarantee is actually enforced. + total_read: AtomicU64, } impl ArchiveVfs @@ -59,8 +111,9 @@ where /// /// This method: /// 1. Validates the reader contains a valid ZIP archive - /// 2. Checks for path traversal attempts - /// 3. Validates archive structure + /// 2. Enforces `limits` on entry count and decompressed size + /// 3. Checks for path traversal attempts + /// 4. Validates archive structure /// /// # Errors /// @@ -68,11 +121,23 @@ where /// - Reader does not contain a valid ZIP archive /// - Archive contains path traversal attempts /// - Archive is corrupted - pub(super) fn from_reader(reader: T) -> Result { + /// - Archive exceeds any of the `limits` + pub(super) fn from_reader(reader: T, limits: ArchiveLimits) -> Result { let mut archive = ZipArchive::new(reader).map_err(|e| ArchiveError::InvalidZipFormat { details: e.to_string(), })?; + // Checked before the loop: the count is a per-call cost multiplier for + // `read_dir` / `is_directory_locked`, not just a memory concern. + if limits.max_entries > 0 && archive.len() > limits.max_entries { + return Err(ArchiveError::TooManyEntries { + count: archive.len(), + limit: limits.max_entries, + }); + } + + let mut total_size: u64 = 0; + // Validate all file names for security for i in 0..archive.len() { let file = archive @@ -123,10 +188,31 @@ where path: file_name.to_string(), }); } + + // Central-directory sizes are author-controlled and can understate + // reality, so this is only a cheap fail-fast; `read_file` re-checks + // against the real decompressed byte count. + let declared_size = file.size(); + + if limits.max_entry_size > 0 && declared_size > limits.max_entry_size { + return Err(ArchiveError::EntrySizeExceeded { + path: file_name.to_string(), + limit: limits.max_entry_size, + }); + } + + total_size = total_size.saturating_add(declared_size); + if limits.max_total_size > 0 && total_size > limits.max_total_size { + return Err(ArchiveError::ArchiveSizeExceeded { + limit: limits.max_total_size, + }); + } } Ok(Self { archive: Mutex::new(archive), + limits, + total_read: AtomicU64::new(0), }) } } @@ -148,8 +234,12 @@ impl ArchiveVfs { /// - Archive is not a valid ZIP /// - Archive contains path traversal attempts /// - Archive is corrupted + /// - Archive exceeds any of the `limits` #[cfg(not(target_arch = "wasm32"))] - pub(super) fn from_file>(path: P) -> Result { + pub(super) fn from_file>( + path: P, + limits: ArchiveLimits, + ) -> Result { let path = path.as_ref(); // Validate extension @@ -169,7 +259,7 @@ impl ArchiveVfs { source: e, })?; - Self::from_reader(file) + Self::from_reader(file, limits) } } @@ -187,9 +277,13 @@ impl ArchiveVfs>> { /// - Bytes are not a valid ZIP archive /// - Archive contains path traversal attempts /// - Archive is corrupted - pub(super) fn from_buffer(buffer: Vec) -> Result { + /// - Archive exceeds any of the `limits` + pub(super) fn from_buffer( + buffer: Vec, + limits: ArchiveLimits, + ) -> Result { let cursor = Cursor::new(buffer); - Self::from_reader(cursor) + Self::from_reader(cursor, limits) } } @@ -215,6 +309,25 @@ where } } + /// Charge `len` decompressed bytes against `limits.max_total_size`. + /// Rejected reads still count: the bytes were decompressed either way. + fn charge_total(&self, len: usize) -> Result<(), std::io::Error> { + let max_total = self.limits.max_total_size; + if max_total == 0 { + return Ok(()); + } + + let len = len as u64; + let previous = self.total_read.fetch_add(len, Ordering::Relaxed); + if previous.saturating_add(len) > max_total { + return Err(std::io::Error::other(ArchiveError::ArchiveSizeExceeded { + limit: max_total, + })); + } + + Ok(()) + } + /// Check if a path exists in the archive (file or directory). fn path_exists(&self, path: &str) -> Result { let normalized = Self::normalize_path(path); @@ -320,8 +433,26 @@ where ) })?; + let max_entry_size = self.limits.max_entry_size; let mut contents = Vec::new(); - file.read_to_end(&mut contents)?; + + if max_entry_size == 0 { + file.read_to_end(&mut contents)?; + } else { + // One byte past the cap is enough to detect a central directory that + // understated this entry and slipped past the check in `from_reader`. + file.take(max_entry_size.saturating_add(1)) + .read_to_end(&mut contents)?; + + if contents.len() as u64 > max_entry_size { + return Err(std::io::Error::other(ArchiveError::EntrySizeExceeded { + path: path.to_string(), + limit: max_entry_size, + })); + } + } + + self.charge_total(contents.len())?; Ok(contents) } @@ -418,7 +549,7 @@ mod tests { use zip::write::{ExtendedFileOptions, FileOptions}; /// Helper to create a test .cjar archive in memory - fn create_test_archive(files: Vec<(&str, &str)>) -> Vec { + pub(super) fn create_test_archive(files: Vec<(&str, &str)>) -> Vec { let mut buffer = Vec::new(); { let cursor = Cursor::new(&mut buffer); @@ -436,17 +567,37 @@ mod tests { buffer } + /// Helper to create a test .cjar archive with zero-filled entries of the + /// given sizes, which deflate to almost nothing — the shape of a zip bomb. + pub(super) fn create_archive_with_sizes(files: Vec<(&str, usize)>) -> Vec { + let mut buffer = Vec::new(); + { + let cursor = Cursor::new(&mut buffer); + let mut zip = zip::ZipWriter::new(cursor); + + for (name, size) in files { + let options = FileOptions::::default() + .compression_method(CompressionMethod::Deflated); + zip.start_file(name, options).unwrap(); + zip.write_all(&vec![0u8; size]).unwrap(); + } + + zip.finish().unwrap(); + } + buffer + } + #[test] fn test_from_buffer_valid_archive() { let bytes = create_test_archive(vec![("metadata.json", "{}")]); - let _result = ArchiveVfs::from_buffer(bytes) + let _result = ArchiveVfs::from_buffer(bytes, ArchiveLimits::default()) .expect("expect ArchiveVfs initialized correctly from buffer"); } #[test] fn test_from_buffer_invalid_zip() { let bytes = b"This is not a ZIP file".to_vec(); - let result = ArchiveVfs::from_buffer(bytes); + let result = ArchiveVfs::from_buffer(bytes, ArchiveLimits::default()); let err = result.expect_err("Expected InvalidZipFormat error for non-ZIP data"); assert!( matches!(err, ArchiveError::InvalidZipFormat { .. }), @@ -457,7 +608,7 @@ mod tests { #[test] fn test_from_buffer_path_traversal() { let bytes = create_test_archive(vec![("../../../etc/passwd", "malicious")]); - let result = ArchiveVfs::from_buffer(bytes); + let result = ArchiveVfs::from_buffer(bytes, ArchiveLimits::default()); let err = result.expect_err("Expected PathTraversal error for malicious path"); assert!( matches!(err, ArchiveError::PathTraversal { .. }), @@ -471,7 +622,7 @@ mod tests { ("metadata.json", r#"{"version":"1.0"}"#), ("schema.cedarschema", "namespace Test;"), ]); - let vfs = ArchiveVfs::from_buffer(bytes).unwrap(); + let vfs = ArchiveVfs::from_buffer(bytes, ArchiveLimits::default()).unwrap(); let content = vfs.read_file("metadata.json").unwrap(); assert_eq!(String::from_utf8(content).unwrap(), r#"{"version":"1.0"}"#); @@ -483,7 +634,7 @@ mod tests { #[test] fn test_read_file_not_found() { let bytes = create_test_archive(vec![("metadata.json", "{}")]); - let vfs = ArchiveVfs::from_buffer(bytes).unwrap(); + let vfs = ArchiveVfs::from_buffer(bytes, ArchiveLimits::default()).unwrap(); let result = vfs.read_file("nonexistent.json"); let err = result.expect_err("Expected error for nonexistent file"); @@ -499,7 +650,7 @@ mod tests { ("metadata.json", "{}"), ("policies/policy1.cedar", "permit();"), ]); - let vfs = ArchiveVfs::from_buffer(bytes).unwrap(); + let vfs = ArchiveVfs::from_buffer(bytes, ArchiveLimits::default()).unwrap(); assert!(vfs.exists("metadata.json")); assert!(vfs.exists("policies/policy1.cedar")); @@ -513,7 +664,7 @@ mod tests { ("metadata.json", "{}"), ("policies/policy1.cedar", "permit();"), ]); - let vfs = ArchiveVfs::from_buffer(bytes).unwrap(); + let vfs = ArchiveVfs::from_buffer(bytes, ArchiveLimits::default()).unwrap(); assert!(vfs.is_file("metadata.json")); assert!(vfs.is_file("policies/policy1.cedar")); @@ -528,7 +679,7 @@ mod tests { ("policies/policy1.cedar", "permit();"), ("policies/policy2.cedar", "forbid();"), ]); - let vfs = ArchiveVfs::from_buffer(bytes).unwrap(); + let vfs = ArchiveVfs::from_buffer(bytes, ArchiveLimits::default()).unwrap(); assert!(vfs.is_dir(".")); assert!(vfs.is_dir("policies")); @@ -543,7 +694,7 @@ mod tests { ("schema.cedarschema", "namespace Test;"), ("policies/policy1.cedar", "permit();"), ]); - let vfs = ArchiveVfs::from_buffer(bytes).unwrap(); + let vfs = ArchiveVfs::from_buffer(bytes, ArchiveLimits::default()).unwrap(); let entries = vfs.read_dir(".").unwrap(); assert_eq!(entries.len(), 3); @@ -561,7 +712,7 @@ mod tests { ("policies/policy2.cedar", "forbid();"), ("policies/nested/policy3.cedar", "deny();"), ]); - let vfs = ArchiveVfs::from_buffer(bytes).unwrap(); + let vfs = ArchiveVfs::from_buffer(bytes, ArchiveLimits::default()).unwrap(); let entries = vfs.read_dir("policies").unwrap(); assert_eq!(entries.len(), 3); @@ -583,7 +734,7 @@ mod tests { let bytes = create_test_archive(vec![("metadata.json", "{}")]); std::fs::write(&archive_path, bytes).unwrap(); - let result = ArchiveVfs::from_file(&archive_path); + let result = ArchiveVfs::from_file(&archive_path, ArchiveLimits::default()); assert!(matches!( result.expect_err("should fail"), ArchiveError::InvalidExtension { .. } @@ -601,7 +752,8 @@ mod tests { let bytes = create_test_archive(vec![("metadata.json", "{}")]); std::fs::write(&archive_path, bytes).unwrap(); - ArchiveVfs::from_file(&archive_path).expect("should load valid .cjar file"); + ArchiveVfs::from_file(&archive_path, ArchiveLimits::default()) + .expect("should load valid .cjar file"); } #[test] @@ -615,7 +767,7 @@ mod tests { ("entities/users/regular.json", "{}"), ("entities/groups/admins.json", "{}"), ]); - let vfs = ArchiveVfs::from_buffer(bytes).unwrap(); + let vfs = ArchiveVfs::from_buffer(bytes, ArchiveLimits::default()).unwrap(); // Test root let root_entries = vfs.read_dir(".").unwrap(); @@ -630,3 +782,257 @@ mod tests { assert_eq!(allow_entries.len(), 2); // policy1.cedar, policy2.cedar } } + +#[cfg(test)] +mod limit_tests { + use super::tests::{create_archive_with_sizes, create_test_archive}; + use super::*; + + /// Keeps the boundary tests cheap while exercising the same code paths as + /// the 10 MB production default. + const SMALL_LIMIT: u64 = 4096; + + /// `usize` view of [`SMALL_LIMIT`], for the byte-count helpers. + fn small_limit_bytes() -> usize { + usize::try_from(SMALL_LIMIT).expect("SMALL_LIMIT fits in usize") + } + + fn small_limits() -> ArchiveLimits { + ArchiveLimits::from_max_file_size(SMALL_LIMIT) + } + + #[test] + fn test_zip_bomb_entry_rejected_at_default_limit() { + // ~11 MB of zeros deflates to a few KB: a tiny archive that was + // previously `read_to_end`'d straight into memory. + let oversized = usize::try_from(ArchiveLimits::DEFAULT_MAX_ENTRY_SIZE + 1) + .expect("default cap fits in usize"); + let bytes = create_archive_with_sizes(vec![("metadata.json", oversized)]); + assert!( + bytes.len() < 100 * 1024, + "test archive should be small on disk to model a zip bomb, was {} bytes", + bytes.len() + ); + + let err = ArchiveVfs::from_buffer(bytes, ArchiveLimits::default()) + .expect_err("Expected EntrySizeExceeded for a zip-bomb entry"); + assert!( + matches!(err, ArchiveError::EntrySizeExceeded { .. }), + "Expected EntrySizeExceeded, got: {err:?}" + ); + } + + #[test] + fn test_entry_at_exact_limit_is_accepted() { + let bytes = create_archive_with_sizes(vec![("metadata.json", small_limit_bytes())]); + + let vfs = ArchiveVfs::from_buffer(bytes, small_limits()) + .expect("An entry of exactly the limit must be accepted"); + + let contents = vfs + .read_file("metadata.json") + .expect("read_file must also accept an entry of exactly the limit"); + assert_eq!(contents.len(), small_limit_bytes()); + } + + #[test] + fn test_entry_one_byte_over_limit_is_rejected() { + let bytes = create_archive_with_sizes(vec![("metadata.json", small_limit_bytes() + 1)]); + + let err = ArchiveVfs::from_buffer(bytes, small_limits()) + .expect_err("Expected EntrySizeExceeded one byte past the limit"); + match err { + ArchiveError::EntrySizeExceeded { path, limit } => { + assert_eq!(path, "metadata.json"); + assert_eq!(limit, SMALL_LIMIT); + }, + other => panic!("Expected EntrySizeExceeded, got: {other:?}"), + } + } + + #[test] + fn test_total_archive_size_exceeded() { + // Every entry sits under the per-entry cap; only the sum trips the + // total, which `from_max_file_size` derives as 10x the per-entry cap. + let limits = small_limits(); + let entry_count = usize::try_from(limits.max_total_size / SMALL_LIMIT + 1) + .expect("entry count fits in usize"); + let names: Vec = (0..entry_count).map(|i| format!("file{i}.json")).collect(); + let bytes = create_archive_with_sizes( + names + .iter() + .map(|n| (n.as_str(), small_limit_bytes())) + .collect(), + ); + + let err = ArchiveVfs::from_buffer(bytes, limits) + .expect_err("Expected ArchiveSizeExceeded when the entries sum past the total"); + match err { + ArchiveError::ArchiveSizeExceeded { limit } => { + assert_eq!(limit, limits.max_total_size); + }, + other => panic!("Expected ArchiveSizeExceeded, got: {other:?}"), + } + } + + #[test] + fn test_total_archive_size_at_exact_limit_is_accepted() { + let limits = small_limits(); + let entry_count = usize::try_from(limits.max_total_size / SMALL_LIMIT) + .expect("entry count fits in usize"); + let names: Vec = (0..entry_count).map(|i| format!("file{i}.json")).collect(); + let bytes = create_archive_with_sizes( + names + .iter() + .map(|n| (n.as_str(), small_limit_bytes())) + .collect(), + ); + + ArchiveVfs::from_buffer(bytes, limits) + .expect("An archive totalling exactly the limit must be accepted"); + } + + #[test] + fn test_too_many_entries_is_rejected() { + let limits = ArchiveLimits { + max_entries: 4, + ..ArchiveLimits::default() + }; + let names: Vec = (0..5).map(|i| format!("file{i}.json")).collect(); + let bytes = create_test_archive(names.iter().map(|n| (n.as_str(), "{}")).collect()); + + let err = ArchiveVfs::from_buffer(bytes, limits) + .expect_err("Expected TooManyEntries past the entry-count cap"); + match err { + ArchiveError::TooManyEntries { count, limit } => { + assert_eq!(count, 5); + assert_eq!(limit, 4); + }, + other => panic!("Expected TooManyEntries, got: {other:?}"), + } + } + + #[test] + fn test_entry_count_at_exact_limit_is_accepted() { + let limits = ArchiveLimits { + max_entries: 5, + ..ArchiveLimits::default() + }; + let names: Vec = (0..5).map(|i| format!("file{i}.json")).collect(); + let bytes = create_test_archive(names.iter().map(|n| (n.as_str(), "{}")).collect()); + + ArchiveVfs::from_buffer(bytes, limits) + .expect("An archive with exactly the entry limit must be accepted"); + } + + #[test] + fn test_zero_disables_size_limits() { + // `0` is the "no cap" sentinel, matching the HTTP response cap. + let limits = ArchiveLimits::from_max_file_size(0); + assert_eq!(limits.max_total_size, 0); + + let bytes = create_archive_with_sizes(vec![("metadata.json", 64 * 1024)]); + let vfs = ArchiveVfs::from_buffer(bytes, limits) + .expect("Size caps must be disabled when the limit is 0"); + assert_eq!(vfs.read_file("metadata.json").unwrap().len(), 64 * 1024); + } + + /// Overwrite every little-endian `u32` occurrence of `from` with `to`, + /// which rewrites an entry's size in both the local header and the central + /// directory. Returns how many fields were patched. + fn patch_declared_size(bytes: &mut [u8], from: u32, to: u32) -> usize { + let (from, to) = (from.to_le_bytes(), to.to_le_bytes()); + let mut patched = 0; + for i in 0..bytes.len().saturating_sub(4) { + if bytes[i..i + 4] == from { + bytes[i..i + 4].copy_from_slice(&to); + patched += 1; + } + } + patched + } + + #[test] + fn test_total_size_is_enforced_against_bytes_actually_read() { + // Understating every entry lets the archive past the construction-time + // total, which can only see declared sizes. Each entry still fits under + // the per-entry cap, so only the running total of real decompressed + // bytes catches it. + let entry = small_limit_bytes(); + let mut bytes = create_archive_with_sizes(vec![("a.json", entry), ("b.json", entry)]); + let patched = patch_declared_size( + &mut bytes, + u32::try_from(entry).expect("entry size fits in u32"), + 1, + ); + assert!( + patched >= 4, + "expected to patch 4 size fields, patched {patched}" + ); + + let limits = ArchiveLimits { + max_entry_size: SMALL_LIMIT, + max_total_size: SMALL_LIMIT + 1000, + max_entries: 100, + }; + let vfs = ArchiveVfs::from_buffer(bytes, limits) + .expect("understated sizes must pass the construction-time total"); + + assert_eq!( + vfs.read_file("a.json").expect("first entry fits").len(), + entry + ); + + let err = vfs + .read_file("b.json") + .expect_err("the second entry must push the running total past the cap"); + let source = err + .get_ref() + .and_then(|e| e.downcast_ref::()) + .expect("io::Error must carry the typed ArchiveError"); + assert!( + matches!(source, ArchiveError::ArchiveSizeExceeded { .. }), + "Expected ArchiveSizeExceeded, got: {source:?}" + ); + } + + #[test] + fn test_read_file_rejects_entry_whose_declared_size_lies() { + // Rewrite the central directory's recorded size to 1 byte so the + // construction-time check passes, then confirm `read_file` still + // refuses to buffer the real payload. + let real_size = small_limit_bytes() + 1; + let mut bytes = create_archive_with_sizes(vec![("metadata.json", real_size)]); + let truthful = u32::try_from(real_size) + .expect("test entry size fits in u32") + .to_le_bytes(); + let lie = 1u32.to_le_bytes(); + + let mut patched = 0; + for i in 0..bytes.len().saturating_sub(4) { + if bytes[i..i + 4] == truthful { + bytes[i..i + 4].copy_from_slice(&lie); + patched += 1; + } + } + assert!( + patched >= 2, + "expected to patch the local header and central directory size fields, patched {patched}" + ); + + let vfs = ArchiveVfs::from_buffer(bytes, small_limits()) + .expect("A understated declared size must pass the cheap header check"); + + let err = vfs + .read_file("metadata.json") + .expect_err("read_file must reject an entry that decompresses past the cap"); + let source = err + .get_ref() + .and_then(|e| e.downcast_ref::()) + .expect("io::Error must carry the typed ArchiveError"); + assert!( + matches!(source, ArchiveError::EntrySizeExceeded { .. }), + "Expected EntrySizeExceeded, got: {source:?}" + ); + } +} diff --git a/jans-cedarling/cedarling/src/common/policy_store/archive_security_tests.rs b/jans-cedarling/cedarling/src/common/policy_store/archive_security_tests.rs index 8649357dbcf..9af228235a1 100644 --- a/jans-cedarling/cedarling/src/common/policy_store/archive_security_tests.rs +++ b/jans-cedarling/cedarling/src/common/policy_store/archive_security_tests.rs @@ -18,7 +18,7 @@ use std::io::{Cursor, Write}; use zip::write::{ExtendedFileOptions, FileOptions}; use zip::{CompressionMethod, ZipWriter}; -use super::archive_handler::ArchiveVfs; +use super::archive_handler::{ArchiveLimits, ArchiveVfs}; use super::entity_parser::{EntityParser, ParsedEntity}; use super::errors::{ArchiveError, PolicyStoreError, ValidationError}; use super::issuer_parser::IssuerParser; @@ -39,7 +39,7 @@ mod path_traversal { #[test] fn test_rejects_parent_directory_traversal_in_archive() { let archive = create_path_traversal_archive(); - let result = ArchiveVfs::from_buffer(archive); + let result = ArchiveVfs::from_buffer(archive, ArchiveLimits::default()); let err = result.expect_err("Expected PathTraversal error"); assert!( @@ -60,7 +60,7 @@ mod path_traversal { zip.write_all(b"root:x:0:0").unwrap(); let archive = zip.finish().unwrap().into_inner(); - let result = ArchiveVfs::from_buffer(archive); + let result = ArchiveVfs::from_buffer(archive, ArchiveLimits::default()); let err = result.expect_err("archive with path traversal should be rejected"); assert!( @@ -93,7 +93,7 @@ mod path_traversal { } let archive = zip.finish().unwrap().into_inner(); - let result = ArchiveVfs::from_buffer(archive); + let result = ArchiveVfs::from_buffer(archive, ArchiveLimits::default()); // Should reject due to path traversal let err = result.expect_err("Expected PathTraversal error for double-dot sequences"); @@ -117,7 +117,7 @@ mod path_traversal { zip.write_all(b"content").unwrap(); let archive = zip.finish().unwrap().into_inner(); - let result = ArchiveVfs::from_buffer(archive); + let result = ArchiveVfs::from_buffer(archive, ArchiveLimits::default()); // Should reject archives containing Windows-style path traversal let err = result.expect_err("expected PathTraversal error for Windows path separators"); @@ -138,7 +138,7 @@ mod malicious_archives { #[test] fn test_rejects_corrupted_zip() { let archive = create_corrupted_archive(); - let result = ArchiveVfs::from_buffer(archive); + let result = ArchiveVfs::from_buffer(archive, ArchiveLimits::default()); let err = result.expect_err("Expected InvalidZipFormat error"); assert!( @@ -150,7 +150,7 @@ mod malicious_archives { #[test] fn test_rejects_non_zip_file() { let not_a_zip = b"This is definitely not a ZIP file".to_vec(); - let result = ArchiveVfs::from_buffer(not_a_zip); + let result = ArchiveVfs::from_buffer(not_a_zip, ArchiveLimits::default()); let err = result.expect_err("Expected InvalidZipFormat error"); assert!( @@ -162,7 +162,7 @@ mod malicious_archives { #[test] fn test_rejects_empty_file() { let empty: Vec = Vec::new(); - let result = ArchiveVfs::from_buffer(empty); + let result = ArchiveVfs::from_buffer(empty, ArchiveLimits::default()); let err = result.expect_err("empty buffer should not be a valid archive"); assert!( matches!(err, ArchiveError::InvalidZipFormat { .. }), @@ -178,7 +178,7 @@ mod malicious_archives { let archive = zip.finish().unwrap().into_inner(); // Empty ZIP should be valid but have no files - let result = ArchiveVfs::from_buffer(archive); + let result = ArchiveVfs::from_buffer(archive, ArchiveLimits::default()); let vfs = result.expect("Empty ZIP archive should be accepted by ArchiveVfs"); assert!(!vfs.exists("metadata.json")); } @@ -186,7 +186,7 @@ mod malicious_archives { #[test] fn test_deeply_nested_paths() { let archive = create_deep_nested_archive(100); - let vfs = ArchiveVfs::from_buffer(archive) + let vfs = ArchiveVfs::from_buffer(archive, ArchiveLimits::default()) .expect("ArchiveVfs should handle deeply nested paths without error"); // Verify VFS is usable for a deeply nested archive @@ -220,7 +220,7 @@ mod malicious_archives { zip.write_all(b"{}").unwrap(); let archive = zip.finish().unwrap().into_inner(); - let vfs = ArchiveVfs::from_buffer(archive) + let vfs = ArchiveVfs::from_buffer(archive, ArchiveLimits::default()) .expect("ArchiveVfs should handle archives with very long filenames"); // If accepted, verify VFS is functional @@ -241,7 +241,7 @@ mod input_validation { let builder = fixtures::invalid_metadata_json(); let archive = builder.build_archive().unwrap(); - let vfs = ArchiveVfs::from_buffer(archive).unwrap(); + let vfs = ArchiveVfs::from_buffer(archive, ArchiveLimits::default()).unwrap(); let loader = DefaultPolicyStoreLoader::new(vfs); let result = loader.load_directory(".", true); @@ -260,7 +260,7 @@ mod input_validation { let builder = fixtures::invalid_policy_syntax(); let archive = builder.build_archive().unwrap(); - let vfs = ArchiveVfs::from_buffer(archive).unwrap(); + let vfs = ArchiveVfs::from_buffer(archive, ArchiveLimits::default()).unwrap(); let loader = DefaultPolicyStoreLoader::new(vfs); let result = loader.load_directory(".", true); @@ -279,7 +279,7 @@ mod input_validation { let builder = fixtures::minimal_valid().with_entity("invalid", "{ not valid json }"); let archive = builder.build_archive().unwrap(); - let vfs = ArchiveVfs::from_buffer(archive).unwrap(); + let vfs = ArchiveVfs::from_buffer(archive, ArchiveLimits::default()).unwrap(); let loader = DefaultPolicyStoreLoader::new(vfs); let loaded_directory = loader .load_directory(".", true) @@ -303,7 +303,7 @@ mod input_validation { let builder = fixtures::invalid_trusted_issuer(); let archive = builder.build_archive().unwrap(); - let vfs = ArchiveVfs::from_buffer(archive).unwrap(); + let vfs = ArchiveVfs::from_buffer(archive, ArchiveLimits::default()).unwrap(); let loader = DefaultPolicyStoreLoader::new(vfs); let loaded_directory = loader .load_directory(".", true) @@ -327,7 +327,7 @@ mod input_validation { let builder = fixtures::duplicate_entity_uids(); let archive = builder.build_archive().unwrap(); - let vfs = ArchiveVfs::from_buffer(archive).unwrap(); + let vfs = ArchiveVfs::from_buffer(archive, ArchiveLimits::default()).unwrap(); let loader = DefaultPolicyStoreLoader::new(vfs); let loaded_directory = loader .load_directory(".", true) @@ -381,7 +381,7 @@ mod input_validation { .unwrap(); let archive = zip.finish().unwrap().into_inner(); - let result = ArchiveVfs::from_buffer(archive); + let result = ArchiveVfs::from_buffer(archive, ArchiveLimits::default()); // Should handle unicode gracefully result.expect("ArchiveVfs should handle unicode filenames without error"); @@ -396,7 +396,7 @@ permit(principal, action, resource);"#, ); let archive = builder.build_archive().unwrap(); - let vfs = ArchiveVfs::from_buffer(archive).unwrap(); + let vfs = ArchiveVfs::from_buffer(archive, ArchiveLimits::default()).unwrap(); let loader = DefaultPolicyStoreLoader::new(vfs); // Cedar allows special characters in @id() annotations within the policy content. @@ -433,7 +433,7 @@ mod resource_exhaustion { } let archive = builder.build_archive().unwrap(); - let vfs = ArchiveVfs::from_buffer(archive).unwrap(); + let vfs = ArchiveVfs::from_buffer(archive, ArchiveLimits::default()).unwrap(); let loader = DefaultPolicyStoreLoader::new(vfs); let result = loader.load_directory(".", true); @@ -458,7 +458,7 @@ when {{ {large_condition} }};"# PolicyStoreTestBuilder::new("abc123def456").with_policy("large-policy", &policy); let archive = builder.build_archive().unwrap(); - let vfs = ArchiveVfs::from_buffer(archive).unwrap(); + let vfs = ArchiveVfs::from_buffer(archive, ArchiveLimits::default()).unwrap(); let loader = DefaultPolicyStoreLoader::new(vfs); // Large policies should be handled gracefully @@ -492,7 +492,7 @@ when {{ {large_condition} }};"# .with_entity("deep_roles", serde_json::to_string(&entities).unwrap()); let archive = builder.build_archive().unwrap(); - let vfs = ArchiveVfs::from_buffer(archive).unwrap(); + let vfs = ArchiveVfs::from_buffer(archive, ArchiveLimits::default()).unwrap(); let loader = DefaultPolicyStoreLoader::new(vfs); let result = loader.load_directory(".", true); @@ -520,7 +520,7 @@ mod file_extension_validation { let wrong_ext = temp_dir.path().join("store.zip"); std::fs::write(&wrong_ext, &archive_bytes).unwrap(); - let result = ArchiveVfs::from_file(&wrong_ext); + let result = ArchiveVfs::from_file(&wrong_ext, ArchiveLimits::default()); let err = result.expect_err("Expected InvalidExtension error"); assert!( matches!(err, ArchiveError::InvalidExtension { .. }), @@ -540,7 +540,7 @@ mod file_extension_validation { let correct_ext = temp_dir.path().join("store.cjar"); std::fs::write(&correct_ext, &archive_bytes).unwrap(); - let result = ArchiveVfs::from_file(&correct_ext); + let result = ArchiveVfs::from_file(&correct_ext, ArchiveLimits::default()); result.expect("ArchiveVfs should accept .cjar extension"); } } diff --git a/jans-cedarling/cedarling/src/common/policy_store/errors.rs b/jans-cedarling/cedarling/src/common/policy_store/errors.rs index 43392728617..d8ccdcfba0d 100644 --- a/jans-cedarling/cedarling/src/common/policy_store/errors.rs +++ b/jans-cedarling/cedarling/src/common/policy_store/errors.rs @@ -348,6 +348,21 @@ pub(crate) enum ArchiveError { #[error("Path traversal attempt detected in archive: '{path}'")] PathTraversal { path: String }, + /// A single entry decompresses past `CEDARLING_POLICY_STORE_MAX_FILE_SIZE`. + #[error( + "Archive entry '{path}' exceeds the maximum decompressed entry size of {limit} bytes \ + (CEDARLING_POLICY_STORE_MAX_FILE_SIZE)" + )] + EntrySizeExceeded { path: String, limit: u64 }, + + /// The archive's combined decompressed size exceeds the configured cap. + #[error("Archive exceeds the maximum total decompressed size of {limit} bytes")] + ArchiveSizeExceeded { limit: u64 }, + + /// The archive holds more entries than the configured cap. + #[error("Archive contains {count} entries, exceeding the maximum of {limit}")] + TooManyEntries { count: usize, limit: usize }, + /// Unsupported operation on this platform #[cfg(target_arch = "wasm32")] #[error("Archive operations are not supported on this platform")] diff --git a/jans-cedarling/cedarling/src/common/policy_store/loader.rs b/jans-cedarling/cedarling/src/common/policy_store/loader.rs index c411e38d38e..4f2e2f928b6 100644 --- a/jans-cedarling/cedarling/src/common/policy_store/loader.rs +++ b/jans-cedarling/cedarling/src/common/policy_store/loader.rs @@ -20,6 +20,7 @@ use std::path::Path; +use super::archive_handler::{ArchiveLimits, ArchiveVfs}; use super::errors::{PolicyStoreError, ValidationError}; use super::metadata::PolicyStoreMetadata; use super::schema_parser::{ParsedSchema, SchemaFile}; @@ -82,6 +83,7 @@ pub(crate) fn load_policy_store_directory( pub(crate) async fn load_policy_store_archive( path: &Path, strict: bool, + limits: ArchiveLimits, ) -> Result { let path = path.to_path_buf(); @@ -90,8 +92,7 @@ pub(crate) async fn load_policy_store_archive( // (reading from zip archive). Using `spawn_blocking` ensures these operations don't block // the async executor. tokio::task::spawn_blocking(move || { - use super::archive_handler::ArchiveVfs; - let archive_vfs = ArchiveVfs::from_file(&path)?; + let archive_vfs = ArchiveVfs::from_file(&path, limits)?; let loader = DefaultPolicyStoreLoader::new(archive_vfs); let loaded_directory = loader.load_directory(".", strict)?; @@ -115,6 +116,7 @@ pub(crate) async fn load_policy_store_archive( pub(crate) fn load_policy_store_archive( _path: &Path, _strict: bool, + _limits: ArchiveLimits, ) -> Result { Err(super::errors::ArchiveError::WasmUnsupported.into()) } @@ -128,10 +130,9 @@ pub(crate) fn load_policy_store_archive( pub(crate) fn load_policy_store_archive_bytes( bytes: &[u8], strict: bool, + limits: ArchiveLimits, ) -> Result { - use super::archive_handler::ArchiveVfs; - - let archive_vfs = ArchiveVfs::from_buffer(bytes.to_owned())?; + let archive_vfs = ArchiveVfs::from_buffer(bytes.to_owned(), limits)?; let loader = DefaultPolicyStoreLoader::new(archive_vfs); loader.load_directory(".", strict) } diff --git a/jans-cedarling/cedarling/src/common/policy_store/loader_tests.rs b/jans-cedarling/cedarling/src/common/policy_store/loader_tests.rs index 93b2c3d7857..659e1b8aa35 100644 --- a/jans-cedarling/cedarling/src/common/policy_store/loader_tests.rs +++ b/jans-cedarling/cedarling/src/common/policy_store/loader_tests.rs @@ -7,17 +7,17 @@ //! //! This module is extracted from `loader.rs` for maintainability. -use super::super::archive_handler::ArchiveVfs; +use super::super::archive_handler::{ArchiveLimits, ArchiveVfs}; use super::super::entity_parser::EntityParser; use super::super::errors::{CedarParseErrorDetail, PolicyStoreError, ValidationError}; use super::super::issuer_parser::IssuerParser; use super::super::manager::{ConversionError, PolicyStoreManager}; use super::super::vfs_adapter::{DirEntry, MemoryVfs, PhysicalVfs, VfsFileSystem}; use super::*; +use std::fmt::Write as FmtWrite; use std::fs::{self, File}; use std::io::{Cursor, Read, Write}; use std::path::{Path, PathBuf}; -use std::fmt::Write as FmtWrite; use tempfile::TempDir; use zip::CompressionMethod; use zip::write::{ExtendedFileOptions, FileOptions}; @@ -935,8 +935,8 @@ fn make_archive_with_custom_issuer(entries: &[(&str, &str)]) -> Vec { fn test_load_custom_issuers_archive_vfs_end_to_end() { let archive_bytes = make_archive_with_custom_issuer(&[("custom-issuers/acme.json", ACME_JSON)]); - let archive_vfs = - ArchiveVfs::from_buffer(archive_bytes.clone()).expect("ArchiveVfs from buffer"); + let archive_vfs = ArchiveVfs::from_buffer(archive_bytes.clone(), ArchiveLimits::default()) + .expect("ArchiveVfs from buffer"); let loader = DefaultPolicyStoreLoader::new(archive_vfs); let loaded_directory = loader .load_directory(".", true) @@ -961,7 +961,7 @@ fn test_load_custom_issuers_archive_vfs_end_to_end() { assert!(token.required); assert!(token.required_claims.contains("sub")); - let loaded2 = load_policy_store_archive_bytes(&archive_bytes, true) + let loaded2 = load_policy_store_archive_bytes(&archive_bytes, true, ArchiveLimits::default()) .expect("load_policy_store_archive_bytes should succeed"); assert_eq!( @@ -991,7 +991,7 @@ fn test_load_custom_issuers_archive_vfs_duplicate_id_errors() { ), ]); - let loaded = load_policy_store_archive_bytes(&archive_bytes, true) + let loaded = load_policy_store_archive_bytes(&archive_bytes, true, ArchiveLimits::default()) .expect("load should succeed — dedup is detected at convert time"); let err = PolicyStoreManager::convert_to_legacy(loaded, false) @@ -1444,8 +1444,8 @@ fn test_archive_vfs_end_to_end_from_file() { zip.finish().unwrap(); // Step 1: Create ArchiveVfs from file path - let archive_vfs = - ArchiveVfs::from_file(&archive_path).expect("Should create ArchiveVfs from .cjar file"); + let archive_vfs = ArchiveVfs::from_file(&archive_path, ArchiveLimits::default()) + .expect("Should create ArchiveVfs from .cjar file"); // Step 2: Create loader with ArchiveVfs let loader = DefaultPolicyStoreLoader::new(archive_vfs); @@ -1487,8 +1487,8 @@ fn test_archive_vfs_end_to_end_from_bytes() { let archive_bytes = create_test_archive("WASM Archive Store", "fedcba654321", &[], &[]); // Create ArchiveVfs from bytes (works in WASM!) - let archive_vfs = - ArchiveVfs::from_buffer(archive_bytes).expect("Should create ArchiveVfs from bytes"); + let archive_vfs = ArchiveVfs::from_buffer(archive_bytes, ArchiveLimits::default()) + .expect("Should create ArchiveVfs from bytes"); // Create loader and load policy store let loader = DefaultPolicyStoreLoader::new(archive_vfs); @@ -1561,7 +1561,8 @@ fn test_archive_vfs_with_multiple_policies() { zip.finish().unwrap(); } - let archive_vfs = ArchiveVfs::from_buffer(archive_bytes).expect("Should create ArchiveVfs"); + let archive_vfs = ArchiveVfs::from_buffer(archive_bytes, ArchiveLimits::default()) + .expect("Should create ArchiveVfs"); let loader = DefaultPolicyStoreLoader::new(archive_vfs); let loaded_directory = loader @@ -1621,8 +1622,8 @@ fn test_archive_vfs_vs_physical_vfs_equivalence() { zip.finish().expect("Should finalize archive"); } - let archive_vfs = - ArchiveVfs::from_buffer(archive_bytes).expect("Should create ArchiveVfs from bytes"); + let archive_vfs = ArchiveVfs::from_buffer(archive_bytes, ArchiveLimits::default()) + .expect("Should create ArchiveVfs from bytes"); let loader = DefaultPolicyStoreLoader::new(archive_vfs); let loaded_directory = loader .load_directory(".", true) @@ -2002,8 +2003,8 @@ fn test_load_schema_from_schemas_dir_in_archive() { zip.finish().unwrap(); } - let archive_vfs = - ArchiveVfs::from_buffer(archive_bytes).expect("Should create ArchiveVfs from bytes"); + let archive_vfs = ArchiveVfs::from_buffer(archive_bytes, ArchiveLimits::default()) + .expect("Should create ArchiveVfs from bytes"); let loader = DefaultPolicyStoreLoader::new(archive_vfs); let result = loader @@ -2342,8 +2343,8 @@ fn test_archive_shared_namespace_full_pipeline() { zip.finish().expect("Should finalize archive"); } - let archive_vfs = - ArchiveVfs::from_buffer(archive_bytes).expect("Should create ArchiveVfs from bytes"); + let archive_vfs = ArchiveVfs::from_buffer(archive_bytes, ArchiveLimits::default()) + .expect("Should create ArchiveVfs from bytes"); let loader = DefaultPolicyStoreLoader::new(archive_vfs); let result = loader @@ -2408,11 +2409,8 @@ fn test_max_recursion_depth_exceeded() { let _ = write!(path, "/level{i}"); } let file_path = format!("{path}/deep.cedar"); - vfs.create_file( - &file_path, - b"permit(principal, action, resource);", - ) - .unwrap(); + vfs.create_file(&file_path, b"permit(principal, action, resource);") + .unwrap(); let loader = DefaultPolicyStoreLoader::new(vfs); let result = loader.load_directory(".", true); diff --git a/jans-cedarling/cedarling/src/http_utils/mod.rs b/jans-cedarling/cedarling/src/http_utils/mod.rs index fd2e2849a3d..f0a11a8d30a 100644 --- a/jans-cedarling/cedarling/src/http_utils/mod.rs +++ b/jans-cedarling/cedarling/src/http_utils/mod.rs @@ -119,9 +119,14 @@ pub enum HttpRequestReasonError { InvalidUtf8(#[source] std::string::FromUtf8Error), #[error("failed to read response body bytes: {0}")] DecodeResponseBytes(#[source] reqwest::Error), + // Names the property so an operator can tell which knob to raise, whichever + // endpoint (JWKS, status list, policy store, ...) tripped it. + // `read_so_far` is the declared `Content-Length` on the fast path and the + // bytes received so far when streaming, so the message covers both. #[error( "response body exceeds the configured limit of {limit} bytes \ - (read {read_so_far} bytes before stopping)" + ({read_so_far} bytes declared or received); raise \ + CEDARLING_HTTP_MAX_RESPONSE_SIZE_BYTES to allow larger responses" )] ResponseTooLarge { limit: u64, read_so_far: u64 }, } @@ -396,6 +401,11 @@ mod tests { ), "body over the 1024-byte cap must surface as ResponseTooLarge {{ limit: 1024, .. }}, got {err:?}", ); + assert!( + err.to_string() + .contains("CEDARLING_HTTP_MAX_RESPONSE_SIZE_BYTES"), + "the error must name the property that controls the cap, got: {err}" + ); } #[tokio::test] diff --git a/jans-cedarling/cedarling/src/init/policy_store.rs b/jans-cedarling/cedarling/src/init/policy_store.rs index f7106c0ecbf..d6049f0375a 100644 --- a/jans-cedarling/cedarling/src/init/policy_store.rs +++ b/jans-cedarling/cedarling/src/init/policy_store.rs @@ -7,6 +7,7 @@ use std::path::Path; use std::{fs, io}; use crate::bootstrap_config::policy_store_config::{PolicyStoreConfig, PolicyStoreSource}; +use crate::common::policy_store::archive_handler::ArchiveLimits; use crate::common::policy_store::errors::{PolicyStoreError, ValidationError}; use crate::common::policy_store::legacy_store::LegacyAgamaPolicyStore; use crate::common::policy_store::manager::PolicyStoreManager; @@ -73,7 +74,7 @@ fn extract_first_policy_store( .take(1) .map(|(k, v)| { let store: PolicyStore = v.to_owned().into(); - + let metadata = crate::common::policy_store::metadata::PolicyStoreMetadata { cedar_version: agama_policy_store .cedar_version @@ -146,6 +147,8 @@ pub(crate) async fn load_policy_store( http_client: &HttpClient, strict_schema_validation: bool, ) -> Result { + let limits = config.archive_limits(); + let loaded = match &config.source { PolicyStoreSource::Yaml(policy_yaml) => { if crate::common::policy_store::is_json_content(policy_yaml) { @@ -160,6 +163,7 @@ pub(crate) async fn load_policy_store( policy_store_uri, http_client, strict_schema_validation, + limits, ) .await? }, @@ -176,18 +180,19 @@ pub(crate) async fn load_policy_store( }, #[cfg(not(target_arch = "wasm32"))] PolicyStoreSource::CjarFile(path) => LoadedPolicyStore { - store: load_policy_store_from_cjar_file(path, strict_schema_validation).await?, + store: load_policy_store_from_cjar_file(path, strict_schema_validation, limits).await?, body_hash: None, validators: CacheHeadersState::default(), }, #[cfg(target_arch = "wasm32")] PolicyStoreSource::CjarFile(path) => LoadedPolicyStore { - store: load_policy_store_from_cjar_file(path, strict_schema_validation)?, + store: load_policy_store_from_cjar_file(path, strict_schema_validation, limits)?, body_hash: None, validators: CacheHeadersState::default(), }, PolicyStoreSource::CjarUrl(url) => { - load_policy_store_from_cjar_url(url, http_client, strict_schema_validation).await? + load_policy_store_from_cjar_url(url, http_client, strict_schema_validation, limits) + .await? }, #[cfg(not(target_arch = "wasm32"))] PolicyStoreSource::Directory(path) => LoadedPolicyStore { @@ -202,12 +207,12 @@ pub(crate) async fn load_policy_store( validators: CacheHeadersState::default(), }, PolicyStoreSource::ArchiveBytes(bytes) => LoadedPolicyStore { - store: load_policy_store_from_archive_bytes(bytes, strict_schema_validation)?, + store: load_policy_store_from_archive_bytes(bytes, strict_schema_validation, limits)?, body_hash: None, validators: CacheHeadersState::default(), }, PolicyStoreSource::Uri(uri) => { - load_policy_store_from_uri(uri, http_client, strict_schema_validation).await? + load_policy_store_from_uri(uri, http_client, strict_schema_validation, limits).await? }, }; @@ -232,6 +237,7 @@ async fn load_policy_store_from_uri( uri: &str, http_client: &HttpClient, strict_schema_validation: bool, + limits: ArchiveLimits, ) -> Result { let response = http_client.get_with_retry(uri).await?; @@ -242,7 +248,7 @@ async fn load_policy_store_from_uri( if bytes.starts_with(&ZIP_MAGIC) { return Ok(LoadedPolicyStore { - store: parse_cjar_bytes(&bytes, strict_schema_validation).await?, + store: parse_cjar_bytes(&bytes, strict_schema_validation, limits).await?, body_hash: Some(body_hash), validators, }); @@ -265,11 +271,14 @@ async fn load_policy_store_from_uri( pub(crate) async fn parse_cjar_bytes( bytes: &[u8], strict_schema_validation: bool, + limits: ArchiveLimits, ) -> Result { use crate::common::policy_store::loader; - let loaded = loader::load_policy_store_archive_bytes(bytes, strict_schema_validation) - .map_err(|e| PolicyStoreLoadError::Archive(format!("Failed to load from archive: {e}")))?; + let loaded = loader::load_policy_store_archive_bytes(bytes, strict_schema_validation, limits) + .map_err(|e| { + PolicyStoreLoadError::Archive(format!("Failed to load from archive: {e}")) + })?; let store_id = loaded.metadata.policy_store.id.clone(); let store_metadata = loaded.metadata.clone(); @@ -310,10 +319,11 @@ fn convert_archive_to_legacy( async fn load_policy_store_from_cjar_file( path: &Path, strict_schema_validation: bool, + limits: ArchiveLimits, ) -> Result { use crate::common::policy_store::loader; - let loaded = loader::load_policy_store_archive(path, strict_schema_validation) + let loaded = loader::load_policy_store_archive(path, strict_schema_validation, limits) .await .map_err(|e| map_policy_store_err(e, true))?; @@ -341,11 +351,12 @@ async fn load_policy_store_from_cjar_file( fn load_policy_store_from_cjar_file( path: &Path, strict_schema_validation: bool, + limits: ArchiveLimits, ) -> Result { use crate::common::policy_store::loader; // Call the loader stub function to ensure it's used and the error variant is constructed - match loader::load_policy_store_archive(path, strict_schema_validation) { + match loader::load_policy_store_archive(path, strict_schema_validation, limits) { Err(e) => Err(PolicyStoreLoadError::Archive(format!( "Loading from file path is not supported in WASM. Use CjarUrl instead. Original error: {e}", ))), @@ -366,6 +377,7 @@ async fn load_policy_store_from_cjar_url( url: &str, http_client: &HttpClient, strict_schema_validation: bool, + limits: ArchiveLimits, ) -> Result { use crate::common::policy_store::loader; @@ -386,7 +398,7 @@ async fn load_policy_store_from_cjar_url( let body_hash = crate::init::policy_store_refresh::body_hash(&bytes); - let loaded = loader::load_policy_store_archive_bytes(&bytes, strict_schema_validation) + let loaded = loader::load_policy_store_archive_bytes(&bytes, strict_schema_validation, limits) .map_err(|e| map_policy_store_err(e, true))?; let store_id = loaded.metadata.policy_store.id.clone(); @@ -474,13 +486,13 @@ fn load_policy_store_from_directory( fn load_policy_store_from_archive_bytes( bytes: &[u8], strict_schema_validation: bool, + limits: ArchiveLimits, ) -> Result { use crate::common::policy_store::loader; // Load from bytes (works in both native and WASM) - let loaded = - loader::load_policy_store_archive_bytes(bytes, strict_schema_validation) - .map_err(|e| map_policy_store_err(e, true))?; + let loaded = loader::load_policy_store_archive_bytes(bytes, strict_schema_validation, limits) + .map_err(|e| map_policy_store_err(e, true))?; // Get the policy store ID and metadata let store_id = loaded.metadata.policy_store.id.clone(); @@ -650,10 +662,13 @@ mod test { policy_store_local_fn: Some("../test_files/policy-store_generated.json".to_string()), ..Default::default() }; - let err = BootstrapConfig::from_raw_config(&raw) - .expect_err("legacy JSON file must be rejected"); + let err = + BootstrapConfig::from_raw_config(&raw).expect_err("legacy JSON file must be rejected"); assert!( - matches!(err, crate::BootstrapConfigLoadingError::LegacyJsonNotSupported), + matches!( + err, + crate::BootstrapConfigLoadingError::LegacyJsonNotSupported + ), "expected LegacyJsonNotSupported, got {err:?}" ); } @@ -668,7 +683,10 @@ mod test { let err = BootstrapConfig::from_raw_config(&raw) .expect_err("legacy JSON inline must be rejected"); assert!( - matches!(err, crate::BootstrapConfigLoadingError::LegacyJsonNotSupported), + matches!( + err, + crate::BootstrapConfigLoadingError::LegacyJsonNotSupported + ), "expected LegacyJsonNotSupported, got {err:?}" ); } @@ -792,7 +810,7 @@ mod test { let err = load_policy_store( &PolicyStoreConfig { source: PolicyStoreSource::Uri(uri), - refresh_interval_secs: 0, + ..Default::default() }, &HTTP_CLIENT, false, @@ -830,7 +848,7 @@ mod test { let err = load_policy_store( &PolicyStoreConfig { source: PolicyStoreSource::Uri(uri), - refresh_interval_secs: 0, + ..Default::default() }, &HTTP_CLIENT, false, @@ -866,7 +884,7 @@ mod test { load_policy_store( &PolicyStoreConfig { source: PolicyStoreSource::Uri(uri), - refresh_interval_secs: 0, + ..Default::default() }, &HTTP_CLIENT, false, @@ -898,7 +916,7 @@ mod test { load_policy_store( &PolicyStoreConfig { source: PolicyStoreSource::Uri(uri), - refresh_interval_secs: 0, + ..Default::default() }, &HTTP_CLIENT, false, diff --git a/jans-cedarling/cedarling/src/init/policy_store_refresh.rs b/jans-cedarling/cedarling/src/init/policy_store_refresh.rs index 73cfead7731..ea4ff961541 100644 --- a/jans-cedarling/cedarling/src/init/policy_store_refresh.rs +++ b/jans-cedarling/cedarling/src/init/policy_store_refresh.rs @@ -32,6 +32,7 @@ use crate::async_sleep::sleep; use crate::authz::Authz; use crate::authz::metrics::MetricsCollector; use crate::bootstrap_config::{AuthorizationConfig, JwtConfig}; +use crate::common::policy_store::archive_handler::ArchiveLimits; use crate::common::policy_store::{PolicyStoreWithID, TrustedIssuer}; use crate::context_data_api::DataStore; use crate::http::cache_headers::CacheHeadersState; @@ -374,9 +375,10 @@ impl RefreshSource { &self, bytes: &[u8], strict_schema_validation: bool, + limits: ArchiveLimits, ) -> Result { if bytes.starts_with(&ZIP_MAGIC) { - return parse_cjar_bytes(bytes, strict_schema_validation).await; + return parse_cjar_bytes(bytes, strict_schema_validation, limits).await; } if crate::common::policy_store::is_json_bytes(bytes) { return Err(PolicyStoreLoadError::LegacyJsonNotSupported); @@ -441,6 +443,9 @@ pub(crate) struct WorkerContext { /// dropped its schema could install a configuration the startup path /// would have rejected. pub(crate) strict_schema_validation: bool, + /// Forwarded from `BootstrapConfig.policy_store_config` so a refreshed + /// `.cjar` is held to the same size limits as the bootstrap load. + pub(crate) archive_limits: ArchiveLimits, } /// Spawn the background refresh worker. Returns a [`PolicyStoreRefreshHandle`] @@ -658,7 +663,11 @@ async fn parse_swap_and_record( ) -> RefreshOutcome { let url = ctx.source.url(); let start = Utc::now(); - let parsed = match ctx.source.parse(&bytes, ctx.strict_schema_validation).await { + let parsed = match ctx + .source + .parse(&bytes, ctx.strict_schema_validation, ctx.archive_limits) + .await + { Ok(p) => p, Err(e) => { state.consecutive_failures = state.consecutive_failures.saturating_add(1); diff --git a/jans-cedarling/cedarling/src/lib.rs b/jans-cedarling/cedarling/src/lib.rs index db1b396efaa..0351b0fec29 100644 --- a/jans-cedarling/cedarling/src/lib.rs +++ b/jans-cedarling/cedarling/src/lib.rs @@ -590,6 +590,7 @@ fn maybe_spawn_refresh_worker( initial_body_hash: seed.initial_body_hash, initial_validators: seed.initial_validators, strict_schema_validation: config.authorization_config.strict_schema_validation, + archive_limits: config.policy_store_config.archive_limits(), }; Some(Arc::new(spawn_refresh_worker(ctx))) } diff --git a/jans-cedarling/cedarling/src/tests/policy_store_loader.rs b/jans-cedarling/cedarling/src/tests/policy_store_loader.rs index 4f5eca6b74e..4e2677b9a00 100644 --- a/jans-cedarling/cedarling/src/tests/policy_store_loader.rs +++ b/jans-cedarling/cedarling/src/tests/policy_store_loader.rs @@ -283,8 +283,8 @@ fn create_jwt_trusted_issuer_json_with_id( oidc_endpoint: &str, token_metadata: &str, ) -> String { - let token_metadata: serde_json::Value = serde_json::from_str(token_metadata) - .expect("token_metadata must be valid JSON"); + let token_metadata: serde_json::Value = + serde_json::from_str(token_metadata).expect("token_metadata must be valid JSON"); let value = json!({ "id": issuer_id, "name": "Jans", @@ -1222,8 +1222,53 @@ async fn test_cjar_url_handles_http_error() { /// /// This tests the `load_policy_store_archive_bytes` function which is the /// underlying mechanism used by `CjarUrl` and is WASM-compatible. +/// `CEDARLING_POLICY_STORE_MAX_FILE_SIZE` must reach the archive loader, not +/// just the `ArchiveVfs::from_buffer` call site. A store that loads fine at the +/// default cap must be rejected once the configured cap drops below it. +#[test] +async fn test_configured_max_file_size_reaches_archive_loader() { + let archive_bytes = create_authz_policy_store_builder() + .build_archive() + .expect("Failed to build test archive"); + + let http_client = crate::http::HttpClient::new(crate::HttpClientConfig::default()) + .expect("Should create HttpClient"); + + crate::init::policy_store::load_policy_store( + &crate::PolicyStoreConfig { + source: PolicyStoreSource::ArchiveBytes(archive_bytes.clone()), + ..Default::default() + }, + &http_client, + true, + ) + .await + .expect("The archive must load at the default 10 MB cap"); + + // `LoadedPolicyStore` isn't `Debug`, so match rather than `expect_err`. + let result = crate::init::policy_store::load_policy_store( + &crate::PolicyStoreConfig { + source: PolicyStoreSource::ArchiveBytes(archive_bytes), + max_file_size: 16, + ..Default::default() + }, + &http_client, + true, + ) + .await; + + match result { + Ok(_) => panic!("The same archive must be rejected once the cap drops to 16 bytes"), + Err(err) => assert!( + err.to_string().contains("maximum decompressed entry size"), + "error should name the entry-size cap, got: {err}" + ), + } +} + #[test] async fn test_load_policy_store_archive_bytes_directly() { + use crate::common::policy_store::archive_handler::ArchiveLimits; use crate::common::policy_store::loader::load_policy_store_archive_bytes; // Build archive bytes @@ -1233,7 +1278,7 @@ async fn test_load_policy_store_archive_bytes_directly() { .expect("Failed to build test archive"); // Load directly using the bytes loader - let loaded = load_policy_store_archive_bytes(&archive_bytes, true) + let loaded = load_policy_store_archive_bytes(&archive_bytes, true, ArchiveLimits::default()) .expect("Should load policy store from bytes"); // Verify the loaded policy store @@ -1266,11 +1311,12 @@ async fn test_load_policy_store_archive_bytes_directly() { /// Test that invalid archive bytes are rejected. #[test] async fn test_load_policy_store_archive_bytes_invalid() { + use crate::common::policy_store::archive_handler::ArchiveLimits; use crate::common::policy_store::loader::load_policy_store_archive_bytes; // Try to load invalid bytes let invalid_bytes = vec![0x00, 0x01, 0x02, 0x03]; - let err = load_policy_store_archive_bytes(&invalid_bytes, true) + let err = load_policy_store_archive_bytes(&invalid_bytes, true, ArchiveLimits::default()) .expect_err("Should fail to load invalid archive bytes"); // Verify the error is an Archive error (invalid zip format)