diff --git a/crates/openshell-core/src/policy.rs b/crates/openshell-core/src/policy.rs index 1645b9da44..20212fd2e3 100644 --- a/crates/openshell-core/src/policy.rs +++ b/crates/openshell-core/src/policy.rs @@ -92,6 +92,19 @@ pub enum LandlockCompatibility { HardRequirement, } +/// Accepted `landlock.compatibility` values in their proto string form. +/// +/// Single source of truth shared by YAML parsing, proto→runtime conversion, +/// and gateway policy validation so the accepted set cannot drift. +pub const LANDLOCK_COMPATIBILITY_VALUES: [&str; 2] = ["best_effort", "hard_requirement"]; + +/// Returns `true` if `value` is an accepted `landlock.compatibility` string. +/// +/// The empty string is accepted and defaults to `best_effort`. +pub fn is_valid_landlock_compatibility(value: &str) -> bool { + value.is_empty() || LANDLOCK_COMPATIBILITY_VALUES.contains(&value) +} + // ============================================================================ // Proto to Rust type conversions // ============================================================================ @@ -114,7 +127,11 @@ impl TryFrom for SandboxPolicy { .map(FilesystemPolicy::from) .unwrap_or_default(), network, - landlock: proto.landlock.map(LandlockPolicy::from).unwrap_or_default(), + landlock: proto + .landlock + .map(LandlockPolicy::try_from) + .transpose()? + .unwrap_or_default(), process: proto.process.map(ProcessPolicy::from).unwrap_or_default(), }) } @@ -138,14 +155,20 @@ impl From for FilesystemPolicy { } } -impl From for LandlockPolicy { - fn from(proto: ProtoLandlockPolicy) -> Self { - let compatibility = if proto.compatibility == "hard_requirement" { - LandlockCompatibility::HardRequirement - } else { - LandlockCompatibility::BestEffort +impl TryFrom for LandlockPolicy { + type Error = miette::Error; + + fn try_from(proto: ProtoLandlockPolicy) -> Result { + let compatibility = match proto.compatibility.as_str() { + "best_effort" | "" => LandlockCompatibility::BestEffort, + "hard_requirement" => LandlockCompatibility::HardRequirement, + otherwise => miette::bail!( + "invalid landlock.compatibility {:?}; accepted: {}", + otherwise, + LANDLOCK_COMPATIBILITY_VALUES.join(", ") + ), }; - Self { compatibility } + Ok(Self { compatibility }) } } @@ -165,3 +188,49 @@ impl From for ProcessPolicy { } } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn try_from_maps_known_compatibility_values() { + for (input, expected) in [ + ("", LandlockCompatibility::BestEffort), + ("best_effort", LandlockCompatibility::BestEffort), + ("hard_requirement", LandlockCompatibility::HardRequirement), + ] { + let proto = ProtoLandlockPolicy { + compatibility: input.into(), + }; + let policy = LandlockPolicy::try_from(proto).expect("should convert"); + assert_eq!( + std::mem::discriminant(&policy.compatibility), + std::mem::discriminant(&expected), + "input {input:?} mapped to unexpected variant", + ); + } + } + + #[test] + fn try_from_rejects_invalid_compatibility() { + let proto = ProtoLandlockPolicy { + compatibility: "hard-requirement".into(), + }; + let err = LandlockPolicy::try_from(proto).expect_err("should reject"); + let msg = format!("{err:?}"); + assert!( + msg.contains("best_effort") && msg.contains("hard_requirement"), + "error should list accepted values, got: {msg}", + ); + } + + #[test] + fn is_valid_landlock_compatibility_accepts_empty_and_known() { + assert!(is_valid_landlock_compatibility("")); + assert!(is_valid_landlock_compatibility("best_effort")); + assert!(is_valid_landlock_compatibility("hard_requirement")); + assert!(!is_valid_landlock_compatibility("nope")); + assert!(!is_valid_landlock_compatibility("BestEffort")); + } +} diff --git a/crates/openshell-policy/src/lib.rs b/crates/openshell-policy/src/lib.rs index 30584c89df..42c28ae3be 100644 --- a/crates/openshell-policy/src/lib.rs +++ b/crates/openshell-policy/src/lib.rs @@ -74,11 +74,19 @@ struct FilesystemDef { read_write: Vec, } +#[derive(Debug, Default, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +enum LandlockCompatibilityDef { + #[default] + BestEffort, + HardRequirement, +} + #[derive(Debug, Serialize, Deserialize)] #[serde(deny_unknown_fields)] struct LandlockDef { - #[serde(default, skip_serializing_if = "String::is_empty")] - compatibility: String, + #[serde(default)] + compatibility: LandlockCompatibilityDef, } #[derive(Debug, Serialize, Deserialize)] @@ -810,7 +818,10 @@ fn to_proto(raw: PolicyFile) -> Result { read_write: fs.read_write, }), landlock: raw.landlock.map(|ll| LandlockPolicy { - compatibility: ll.compatibility, + compatibility: match ll.compatibility { + LandlockCompatibilityDef::BestEffort => "best_effort".to_string(), + LandlockCompatibilityDef::HardRequirement => "hard_requirement".to_string(), + }, }), process: raw.process.map(|p| ProcessPolicy { run_as_user: p.run_as_user, @@ -825,16 +836,28 @@ fn to_proto(raw: PolicyFile) -> Result { // Proto → YAML conversion // --------------------------------------------------------------------------- -fn from_proto(policy: &SandboxPolicy) -> PolicyFile { +fn from_proto(policy: &SandboxPolicy) -> Result { let filesystem_policy = policy.filesystem.as_ref().map(|fs| FilesystemDef { include_workdir: fs.include_workdir, read_only: fs.read_only.clone(), read_write: fs.read_write.clone(), }); - let landlock = policy.landlock.as_ref().map(|ll| LandlockDef { - compatibility: ll.compatibility.clone(), - }); + let landlock = match policy.landlock.as_ref() { + Some(ll) => { + let compatibility = match ll.compatibility.as_str() { + "hard_requirement" => LandlockCompatibilityDef::HardRequirement, + "best_effort" | "" => LandlockCompatibilityDef::BestEffort, + otherwise => miette::bail!( + "invalid landlock.compatibility {:?}; accepted: {}", + otherwise, + openshell_core::policy::LANDLOCK_COMPATIBILITY_VALUES.join(", ") + ), + }; + Some(LandlockDef { compatibility }) + } + _ => None, + }; let process = policy.process.as_ref().and_then(|p| { if p.run_as_user.is_empty() && p.run_as_group.is_empty() { @@ -958,14 +981,14 @@ fn from_proto(policy: &SandboxPolicy) -> PolicyFile { let network_middlewares = middleware::from_proto(&policy.network_middlewares); - PolicyFile { + Ok(PolicyFile { version: policy.version, filesystem_policy, landlock, process, network_policies, network_middlewares, - } + }) } // --------------------------------------------------------------------------- @@ -1020,7 +1043,7 @@ pub fn parse_sandbox_policy(yaml: &str) -> Result { /// canonical YAML field names (e.g. `filesystem_policy`, not `filesystem`) /// and is round-trippable through `parse_sandbox_policy`. pub fn serialize_sandbox_policy(policy: &SandboxPolicy) -> Result { - let yaml_repr = from_proto(policy); + let yaml_repr = from_proto(policy)?; serde_yml::to_string(&yaml_repr) .into_diagnostic() .wrap_err("failed to serialize policy to YAML") @@ -1031,7 +1054,7 @@ pub fn serialize_sandbox_policy(policy: &SandboxPolicy) -> Result { /// The shape mirrors the YAML schema used by [`serialize_sandbox_policy`], so /// automation can use the same documented field names in either format. pub fn sandbox_policy_to_json_value(policy: &SandboxPolicy) -> Result { - let json_repr = from_proto(policy); + let json_repr = from_proto(policy)?; serde_json::to_value(&json_repr) .into_diagnostic() .wrap_err("failed to serialize policy to JSON") @@ -1188,6 +1211,8 @@ pub enum PolicyViolation { policy_name: String, host: String, }, + /// `landlock.compatibility` has an unrecognized value. + InvalidLandlockCompatibility { value: String }, } impl fmt::Display for PolicyViolation { @@ -1298,6 +1323,13 @@ impl fmt::Display for PolicyViolation { '{policy_name}' tls: skip endpoint '{host}'" ) } + Self::InvalidLandlockCompatibility { value } => { + write!( + f, + "invalid landlock.compatibility '{value}'; accepted: {}", + openshell_core::policy::LANDLOCK_COMPATIBILITY_VALUES.join(", ") + ) + } } } } @@ -1342,6 +1374,17 @@ pub fn validate_sandbox_policy( } } + // Check landlock compatibility mode is a recognized value. Direct gRPC/SDK + // clients bypass YAML serde validation, so reject invalid values here at the + // gateway create path rather than deferring rejection to sandbox startup. + if let Some(ref landlock) = policy.landlock + && !openshell_core::policy::is_valid_landlock_compatibility(&landlock.compatibility) + { + violations.push(PolicyViolation::InvalidLandlockCompatibility { + value: landlock.compatibility.clone(), + }); + } + // Check filesystem paths if let Some(ref fs) = policy.filesystem { let total_paths = fs.read_only.len() + fs.read_write.len(); @@ -2023,6 +2066,80 @@ network_policies: assert_eq!(violations.len(), 2); } + #[test] + fn parse_rejects_invalid_landlock_compatibility() { + let err = parse_sandbox_policy("version: 1\nlandlock:\n compatibility: bogus\n") + .expect_err("should reject invalid YAML enum value"); + let msg = format!("{err:?}"); + assert!( + msg.contains("best_effort") && msg.contains("hard_requirement"), + "error should list accepted values, got: {msg}", + ); + } + + #[test] + fn parse_accepts_known_landlock_compatibility() { + for value in ["best_effort", "hard_requirement"] { + let yaml = format!("version: 1\nlandlock:\n compatibility: {value}\n"); + let policy = parse_sandbox_policy(&yaml).expect("should parse"); + assert_eq!( + policy.landlock.as_ref().expect("landlock").compatibility, + value, + ); + } + } + + #[test] + fn validate_rejects_invalid_landlock_compatibility_proto() { + let mut policy = restrictive_default_policy(); + policy.landlock = Some(LandlockPolicy { + compatibility: "nope".into(), + }); + let violations = validate_sandbox_policy(&policy).unwrap_err(); + assert!( + violations + .iter() + .any(|v| matches!(v, PolicyViolation::InvalidLandlockCompatibility { .. })), + "expected InvalidLandlockCompatibility, got: {violations:?}", + ); + } + + #[test] + fn validate_accepts_empty_landlock_compatibility() { + // Empty string is the proto default and maps to best_effort. + let mut policy = restrictive_default_policy(); + policy.landlock = Some(LandlockPolicy { + compatibility: String::new(), + }); + assert!(validate_sandbox_policy(&policy).is_ok()); + } + + #[test] + fn serialize_rejects_invalid_landlock_compatibility() { + // Old policies persisted before gateway validation can hold invalid + // values; serialization must error rather than normalize to best_effort. + let mut policy = restrictive_default_policy(); + policy.landlock = Some(LandlockPolicy { + compatibility: "hard-requirement".to_string(), + }); + let err = serialize_sandbox_policy(&policy).expect_err("should reject"); + let msg = format!("{err:?}"); + assert!( + msg.contains("best_effort") && msg.contains("hard_requirement"), + "error should list accepted values, got: {msg}", + ); + } + + #[test] + fn serialize_accepts_empty_landlock_compatibility() { + // Empty string is the proto default; serialize must not error on it. + let mut policy = restrictive_default_policy(); + policy.landlock = Some(LandlockPolicy { + compatibility: String::new(), + }); + assert!(serialize_sandbox_policy(&policy).is_ok()); + } + #[test] fn validate_rejects_invalid_middleware_control_fields() { let cases = [ diff --git a/crates/openshell-supervisor-process/src/sandbox/linux/landlock.rs b/crates/openshell-supervisor-process/src/sandbox/linux/landlock.rs index bf42faede8..0a5bbfdc79 100644 --- a/crates/openshell-supervisor-process/src/sandbox/linux/landlock.rs +++ b/crates/openshell-supervisor-process/src/sandbox/linux/landlock.rs @@ -144,6 +144,14 @@ fn prepare_with_path_open_mode( } if read_only.is_empty() && read_write.is_empty() { + if matches!( + policy.landlock.compatibility, + LandlockCompatibility::HardRequirement + ) { + miette::bail!( + "landlock.compatibility is hard_requirement but no filesystem paths are configured" + ); + } return Ok(None); } @@ -522,6 +530,68 @@ mod tests { panic!("hard_requirement should accept mixed directory and device paths: {err}"); } } + #[test] + fn prepare_hard_requirement_no_paths_aborts() { + // Zero configured paths under hard_requirement must fail startup rather + // than silently running without filesystem restrictions. + let policy = hard_requirement_policy(vec![], vec![]); + let Err(err) = prepare(&policy, None) else { + panic!("should abort with no paths"); + }; + let msg = err.to_string(); + assert!( + msg.contains("hard_requirement") && msg.contains("no filesystem paths"), + "error should explain the empty hard_requirement policy: {msg}" + ); + } + + #[test] + fn prepare_best_effort_no_paths_is_noop() { + let policy = SandboxPolicy { + version: 1, + filesystem: FilesystemPolicy { + read_only: vec![], + read_write: vec![], + include_workdir: false, + }, + network: NetworkPolicy::default(), + landlock: LandlockPolicy { + compatibility: LandlockCompatibility::BestEffort, + }, + process: ProcessPolicy::default(), + }; + let prepared = prepare(&policy, None).expect("best_effort no-op should succeed"); + assert!(prepared.is_none(), "no paths should produce no ruleset"); + } + + #[test] + fn prepare_include_workdir_counts_as_configured_path() { + // With no explicit paths but include_workdir set, the workdir must be + // treated as a configured path — so the zero-path abort must NOT fire. + let policy = SandboxPolicy { + version: 1, + filesystem: FilesystemPolicy { + read_only: vec![], + read_write: vec![], + include_workdir: true, + }, + network: NetworkPolicy::default(), + landlock: LandlockPolicy { + compatibility: LandlockCompatibility::HardRequirement, + }, + process: ProcessPolicy::default(), + }; + // Any error (e.g. Landlock unavailable on this host) is acceptable, but + // it must not be the "no filesystem paths" abort. + if let Err(err) = prepare(&policy, Some("/tmp")) { + let msg = err.to_string(); + assert!( + !msg.contains("no filesystem paths"), + "workdir should count as a configured path: {msg}" + ); + } + } + fn tailored_access(path: &Path, requested_access: BitFlags) -> BitFlags { let path_fd = PathFd::new(path).unwrap(); access_for_path_fd(&path_fd, requested_access, ABI::V2).unwrap() diff --git a/crates/openshell-supervisor-process/src/sandbox/linux/mod.rs b/crates/openshell-supervisor-process/src/sandbox/linux/mod.rs index 107a50e370..8afb3caef2 100644 --- a/crates/openshell-supervisor-process/src/sandbox/linux/mod.rs +++ b/crates/openshell-supervisor-process/src/sandbox/linux/mod.rs @@ -99,14 +99,60 @@ pub fn log_sandbox_readiness(policy: &SandboxPolicy, workdir: Option<&str>) { let total_paths = read_only.len() + read_write.len(); if total_paths == 0 { - openshell_ocsf::ocsf_emit!( - openshell_ocsf::ConfigStateChangeBuilder::new(openshell_ocsf::ctx::ctx()) - .severity(openshell_ocsf::SeverityId::Informational) - .status(openshell_ocsf::StatusId::Success) - .state(openshell_ocsf::StateId::Other, "skipped") - .message("Landlock filesystem sandbox skipped: no paths configured".to_string()) - .build() - ); + if matches!( + policy.landlock.compatibility, + openshell_core::policy::LandlockCompatibility::HardRequirement + ) { + // hard_requirement with no paths is fatal (see `landlock::prepare`). + // Emit a failure state so operators don't see a misleading "skipped" + // success event immediately before startup aborts. + openshell_ocsf::ocsf_emit!( + openshell_ocsf::ConfigStateChangeBuilder::new(openshell_ocsf::ctx::ctx()) + .severity(openshell_ocsf::SeverityId::High) + .status(openshell_ocsf::StatusId::Failure) + .state(openshell_ocsf::StateId::Other, "invalid") + .message( + "Landlock hard_requirement but no filesystem paths configured; \ + sandbox startup will abort" + .to_string(), + ) + .build() + ); + // Dual-emit a security finding for the unsafe policy (per OCSF + // guidance: pair the domain event with a DetectionFinding). + openshell_ocsf::ocsf_emit!( + openshell_ocsf::DetectionFindingBuilder::new(openshell_ocsf::ctx::ctx()) + .activity(openshell_ocsf::ActivityId::Open) + .severity(openshell_ocsf::SeverityId::High) + .confidence(openshell_ocsf::ConfidenceId::High) + .is_alert(true) + .finding_info( + openshell_ocsf::FindingInfo::new( + "landlock-hard-requirement-no-paths", + "Landlock Hard Requirement Without Paths", + ) + .with_desc( + "landlock.compatibility is hard_requirement but no filesystem \ + paths are configured; sandbox startup will abort.", + ), + ) + .message( + "Landlock hard_requirement with no filesystem paths configured".to_string(), + ) + .build() + ); + } else { + openshell_ocsf::ocsf_emit!( + openshell_ocsf::ConfigStateChangeBuilder::new(openshell_ocsf::ctx::ctx()) + .severity(openshell_ocsf::SeverityId::Informational) + .status(openshell_ocsf::StatusId::Success) + .state(openshell_ocsf::StateId::Other, "skipped") + .message( + "Landlock filesystem sandbox skipped: no paths configured".to_string(), + ) + .build() + ); + } return; } diff --git a/docs/reference/policy-schema.mdx b/docs/reference/policy-schema.mdx index 574ce29993..f1aad9d517 100644 --- a/docs/reference/policy-schema.mdx +++ b/docs/reference/policy-schema.mdx @@ -92,14 +92,16 @@ Configures [Landlock LSM](https://docs.kernel.org/security/landlock.html) enforc **Compatibility modes:** -| Value | Kernel ABI unavailable | Individual path inaccessible | All paths inaccessible | -|---|---|---|---| -| `best_effort` | Warns and continues without Landlock. | Skips the path, applies remaining rules. | Warns and continues without Landlock (refuses to apply an empty ruleset). | -| `hard_requirement` | Aborts sandbox startup. | Aborts sandbox startup. | Aborts sandbox startup. | +| Value | No paths configured | Kernel ABI unavailable | Individual path inaccessible | All paths inaccessible | +|---|---|---|---|---| +| `best_effort` | Landlock skipped (no-op). | Warns and continues without Landlock. | Skips the path, applies remaining rules. | Warns and continues without Landlock (refuses to apply an empty ruleset). | +| `hard_requirement` | Aborts sandbox startup. | Aborts sandbox startup. | Aborts sandbox startup, except in Kubernetes sidecar (current-user) mode where the inaccessible path is skipped. | Aborts sandbox startup. | `best_effort` (the default) is appropriate for most deployments. It handles missing paths gracefully. For example, `/app` might not exist in every container image but is included in the baseline path set for containers that do have it. Individual missing paths are skipped while the remaining filesystem rules are still enforced. -`hard_requirement` is for environments where any gap in filesystem isolation is unacceptable. If a listed path cannot be opened for any reason (missing, permission denied, symlink loop), sandbox startup fails immediately rather than running with reduced protection. +`hard_requirement` is for environments where any gap in filesystem isolation is unacceptable. If a listed path cannot be opened for any reason (missing, permission denied, symlink loop), sandbox startup fails immediately rather than running with reduced protection. Configuring `hard_requirement` with no filesystem paths is also a startup error. + +In Kubernetes sidecar (current-user) mode the sandbox cannot distinguish an intentionally denied path from a misconfigured one, so an individual inaccessible path is skipped and the remaining rules are applied instead of aborting. The other `hard_requirement` failures (kernel ABI unavailable, no paths configured, all paths inaccessible) still abort startup. When a path is skipped under `best_effort`, the sandbox logs a warning that includes the path, the specific error, and a human-readable reason (for example, "path does not exist" or "permission denied").