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

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@ sandlock run --no-supervisor -r /proc -r /usr -r /lib -r /lib64 -r /bin -r /etc
### Python API

```python
from sandlock import Sandbox, confine
from sandlock import Mount, Sandbox, confine

sandbox = Sandbox(
fs_writable=["/tmp/sandbox"],
Expand All @@ -272,7 +272,7 @@ result = agent.run(["python3", "agent.py"])
# Chroot with per-sandbox mount (Docker-style -v, no root needed)
chrooted = Sandbox(
chroot="/opt/rootfs",
fs_mount={"/work": "/tmp/sandbox-1/work"}, # maps /work inside chroot
fs_mount=[Mount("/work", "/tmp/sandbox-1/work")], # maps /work inside chroot
fs_readable=["/usr", "/bin", "/lib", "/etc"],
cwd="/work",
)
Expand Down
39 changes: 37 additions & 2 deletions crates/sandlock-core/src/http.rs
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,12 @@ pub fn http_acl_check(
/// allowed to reach the original destination on the intercepted ports. Concrete
/// HTTP rule hosts tighten the IP allowlist to those hosts; wildcard hosts or
/// explicit HTTP ports with no rules allow any IP on the HTTP ports.
///
/// Derived entries a caller already carries are not added twice. A policy can
/// be taken apart and rebuilt (`sandlock run --profile-file` rebuilds a builder
/// from the parsed profile, then applies flag overrides on top), and the
/// rebuilt net allowlist arrives here already holding the entries this

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[CONFIRMED] A deny-only profile with any [http] rules still fails the CLI rebuild with a spurious "mutually exclusive" error; this comment claims the round-trip is safe, but the failure fires before this function runs.

Profile with [network] deny = ["1.2.3.4"] plus [http] ports/rules: build() runs the allow/deny exclusivity check (builder.rs:976) before extend_net_allow_for_http (builder.rs:992) derives at least one NetAllow entry, so the first build passes and the resulting Sandbox has both lists non-empty. The CLI rebuild (main.rs:502-506) feeds the derived entry back as a user spec; on the second build() the exclusivity check sees allow+deny both non-empty and errors with zero override flags. push_unique cannot help because the error fires before extend runs, yet this doc comment and the new idempotency test assert the taken-apart-and-rebuilt flow is handled.

/// function added on the first build.
pub(crate) fn extend_net_allow_for_http(
net_allow: &mut Vec<NetAllow>,
http_allow: &[HttpRule],
Expand All @@ -189,6 +195,12 @@ pub(crate) fn extend_net_allow_for_http(
return;
}

fn push_unique(net_allow: &mut Vec<NetAllow>, rule: NetAllow) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[CONFIRMED] Exact-equality dedup only handles the unchanged-inputs rebuild; when derivation inputs differ between passes, pass-1 residue survives as grants nothing requested, wider than a single-pass build.

Profile with bare http.ports (pass 1 derives AnyIp:[80] via the empty-rules branch, lines 217-224) run with --http-allow "GET api.example.com/*": pass 2 derives only the concrete host, but the carried AnyIp:[80] entry survives as a user spec (NetAllow has no provenance field), defeating the "concrete hosts tighten the allowlist" contract stated at lines 180-181. The residue predates this PR, but the new comment and idempotency test only cover the identical-inputs case; tagging derived entries or re-deriving from the raw profile instead of the effective policy fixes it fully.

if !net_allow.contains(&rule) {
net_allow.push(rule);
}
}

let mut wildcard_seen = false;
let mut concrete_hosts: Vec<String> = Vec::new();
for rule in http_allow.iter().chain(http_deny.iter()) {
Expand All @@ -203,7 +215,7 @@ pub(crate) fn extend_net_allow_for_http(
}

if wildcard_seen || (http_allow.is_empty() && http_deny.is_empty()) {
net_allow.push(NetAllow {
push_unique(net_allow, NetAllow {
protocol: Protocol::Tcp,
target: NetTarget::AnyIp,
ports: http_ports.to_vec(),
Expand All @@ -212,7 +224,7 @@ pub(crate) fn extend_net_allow_for_http(
}

for host in concrete_hosts {
net_allow.push(NetAllow {
push_unique(net_allow, NetAllow {
protocol: Protocol::Tcp,
target: NetTarget::Host(host),
ports: http_ports.to_vec(),
Expand Down Expand Up @@ -481,6 +493,29 @@ mod tests {
assert_eq!(net_allow[1].ports, vec![80, 443]);
}

#[test]
fn extend_net_allow_for_http_is_idempotent() {
// A policy that is taken apart and rebuilt feeds the already derived
// entries back in as plain net-allow specs (that is what
// `sandlock run --profile-file` does before applying flag overrides),
// so a second pass must not grow the allowlist.
let allow = vec![HttpRule::parse("GET api.example.com/v1/*").unwrap()];
let mut net_allow = Vec::new();

extend_net_allow_for_http(&mut net_allow, &allow, &[], &[80]);
let first = net_allow.clone();
extend_net_allow_for_http(&mut net_allow, &allow, &[], &[80]);

assert_eq!(net_allow, first);

// Same for the any-IP entry, which comes from a different branch.
let mut wide = Vec::new();
extend_net_allow_for_http(&mut wide, &[], &[], &[8080]);
let first_wide = wide.clone();
extend_net_allow_for_http(&mut wide, &[], &[], &[8080]);
assert_eq!(wide, first_wide);
}

#[test]
fn extend_net_allow_for_http_adds_any_ip_for_wildcard_or_bare_port() {
let mut net_allow = Vec::new();
Expand Down
33 changes: 25 additions & 8 deletions crates/sandlock-core/src/profile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ use std::path::PathBuf;
use std::collections::HashMap;
use std::time::SystemTime;

pub mod canonical;

/// Program identity supplied by a profile alongside the policy.
/// Not a `Sandbox` field — passed separately to the sandbox runner.
#[derive(Debug, Clone, Default, PartialEq)]
Expand Down Expand Up @@ -337,13 +339,21 @@ pub fn parse_mount_spec(s: &str) -> Result<(PathBuf, PathBuf, bool), SandlockErr

/// Parses an RFC3339 timestamp string into `SystemTime`.
fn parse_time_start(s: &str) -> Result<SystemTime, SandlockError> {
Ok(parse_timestamp(s)?.into())
}

/// Parses an RFC3339 timestamp string, keeping the `jiff::Timestamp`.
///
/// `SystemTime` cannot represent a pre-epoch instant as a plain second count,
/// so the canonical form resolves the string through this instead and does its
/// own epoch split.
fn parse_timestamp(s: &str) -> Result<jiff::Timestamp, SandlockError> {
use crate::error::SandboxError;
let ts: jiff::Timestamp = s.parse().map_err(|e| {
s.parse().map_err(|e| {
SandlockError::Sandbox(SandboxError::Invalid(
format!("invalid [determinism].time_start {s:?}: {e}"),
))
})?;
Ok(ts.into())
})
}

// ============================================================
Expand Down Expand Up @@ -578,13 +588,20 @@ fn dirs_or_fallback() -> PathBuf {
.join("sandlock")
}

/// Parse a TOML profile string into a Sandbox + ProgramSpec.
pub fn parse_profile(content: &str) -> Result<(Sandbox, ProgramSpec), SandlockError> {
let input: ProfileInput = toml::from_str(content)
/// Deserialize a TOML profile string into the raw schema.
///
/// Shared by `parse_profile` and `canonical::parse` so both report a syntax or
/// unknown-key problem with the same wording.
fn deserialize_profile(content: &str) -> Result<ProfileInput, SandlockError> {
toml::from_str(content)
.map_err(|e| SandlockError::Sandbox(crate::error::SandboxError::Invalid(
format!("TOML parse error: {e}"),
)))?;
parse_input(input)
)))
}

/// Parse a TOML profile string into a Sandbox + ProgramSpec.
pub fn parse_profile(content: &str) -> Result<(Sandbox, ProgramSpec), SandlockError> {
parse_input(deserialize_profile(content)?)
}

/// Load a profile by name.
Expand Down
Loading
Loading