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

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/cage/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,6 @@ pub mod memory;
pub mod signal;

pub use cage::*;
pub use dashmap::DashMap;
pub use memory::*;
pub use signal::*;
1 change: 1 addition & 0 deletions src/typemap/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,4 @@ sysdefs = { path = "../sysdefs" }
default = ["fast"]
fast = []
secure = []

65 changes: 65 additions & 0 deletions src/typemap/src/path_conversion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -225,4 +225,69 @@ mod tests {
assert_eq!(path_without_trailing_slashes("/"), "/");
assert_eq!(path_without_trailing_slashes("///"), "/");
}

use cage::DashMap;

// Builds a minimal cage and registers it under 'cageid', so
// normpath(path, cageid) can find it via cage::get_cage()
// Every field besides 'cwd' is irrelevant to path logic, they're
// filled with empty/default values to satisfy the struct
fn make_test_cage(cageid: u64, cwd: &str) {
cage::cagetable_init();

let test_cage = cage::Cage {
cageid,
parent: cageid,
cwd: cage::RwLock::new(cage::Arc::new(PathBuf::from(cwd))),
rev_shm: cage::Mutex::new(Vec::new()),
signalhandler: DashMap::new(),
sigset: cage::AtomicU64::new(0),
pending_signals: cage::RwLock::new(vec![]),
epoch_handler: DashMap::new(),
os_tid_map: DashMap::new(),
main_threadid: cage::RwLock::new(0),
interval_timer: cage::IntervalTimer::new(cageid),
zombies: cage::RwLock::new(vec![]),
child_num: cage::AtomicU64::new(0),
vmmap: cage::RwLock::new(cage::Vmmap::new()),
final_exit_status: cage::RwLock::new(None),
exit_group_initiated: cage::AtomicBool::new(false),
is_dead: cage::AtomicBool::new(false),
grate_inflight: cage::AtomicU64::new(0),
};
cage::add_cage(cageid, test_cage);
}
#[test]
//ISO-004: an absolute path must always be rebuilt from the virtual root
// never passed through some other branch unmodified
fn normpath_confines_absolute_path_to_virtual_root() {
let cageid = 1500;
make_test_cage(cageid, "/");

let result = normpath(PathBuf::from("/etc/../etc/passwd"), cageid);

assert_eq!(result, PathBuf::from("/etc/passwd"));
}
Comment thread
vidyalakshmir marked this conversation as resolved.

#[test]
//ISO-004: excess ".." must clamp at the virtual root instead of
// going negative even when climbing from a real nested cwd
fn normpath_clamps_excess_parent_dir_at_root() {
let cageid = 1501;
make_test_cage(cageid, "/home/user/project");

let result = normpath(PathBuf::from("../../../../../../etc/passwd"), cageid);
assert_eq!(result, PathBuf::from("/etc/passwd"));
}

#[test]
// ISO-004: ordinary ".." must still resolve correctly, not just get
// clamped away, proves the clamp isn't overly aggressive
fn normpath_resolves_ordinary_parent_dir_correctly() {
let cageid = 1502;
make_test_cage(cageid, "/a/b");

let result = normpath(PathBuf::from("foo/../../bar"), cageid);
assert_eq!(result, PathBuf::from("/a/bar"));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
symlink_confinement test: PASS
86 changes: 86 additions & 0 deletions tests/unit-tests/file_tests/deterministic/symlink_confinement.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
#include <assert.h>
#include <errno.h>
#include <fcntl.h>
#include <stdio.h>
#include <string.h>
#include <unistd.h>

int main() {
unlink("evil_link");

assert(symlink("/etc/passwd", "evil_link") == 0);

/*
NOTE: this only checks that symlink resolution is internally consistent
(following evil_link behaves like a normal read), not that either path
is actually confined. See the /lind/README.md-based checks below for
the actual confinement/escape proof.
*/
errno = 0;
int direct_fd = open("/etc/passwd", O_RDONLY);
int direct_errno = errno;

errno = 0;
int link_fd = open("evil_link", O_RDONLY);
int link_errno = errno;

if(direct_fd == -1) {
assert(link_fd == -1);
assert(link_errno == direct_errno);
} else {
assert(link_fd != -1);

char direct_buf[256];
char link_buf[256];
ssize_t direct_n = read(direct_fd, direct_buf, sizeof(direct_buf));
ssize_t link_n = read(link_fd, link_buf, sizeof(link_buf));

assert(direct_n >= 0);
assert(link_n == direct_n);
assert(memcmp(direct_buf, link_buf, (size_t)direct_n) == 0);

close(direct_fd);
close(link_fd);
}

unlink("evil_link");

errno = 0;
/*
NOTE: /lind/README.md is specific to this dev-conatainer's mount

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.

container //typo

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.

Opening /lind/README.md from inside the cage resolves to <LINDFS_ROOT>/lind/README.md, which doesn't exist. This would result in a return value of ENOENT which would pass this test, but that doesn't necessarily test policy enforcement. All it says is that name did not resolve to anything inside the virtual filesystem. Also, doing a generic test here would be problematic when the test is run natively.

A suggested solution is to have a file within the virtual filesystem and outside either.

File /tmp/lind/sentinel.txt will contain the text "LIND_HOST_ONLY"

and File $LINDFS_ROOT/tmp/lind/sentinel will contain the text "LIND_CAGE_ONLY".

So when you run it within lind-wasm, the following three conditions can hold,

  1. Reads LIND_CAGE_ONLY means confinement works, and test passes
  2. Reads LIND_HOST_ONLY means there is an escape and test fails
  3. If open fails, then failure

layout(repo checked out at /lind, matching LINDFS_ROOT's hardcoded
assumption in sysdefs). If this runs somewhere that mounts the repo
differently, this path may not exist at all, in which case this
check would accidentally pass via ENOENT.
Re-verify this path is valid if test is run in a new environment.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The solution could be to pin the expected output instead of deriving it from an unconfined run. Adding:

tests/unit-tests/file_tests/deterministic/expected/symlink_confinement.output

makes get_expected_output() return early and skip the native compile+run entirely. Failure detection is preserved: the lind run's return code still goes through handle_return_code(), so a real escape is still reported as a Runtime Failure regardless of the expected text.

*/
int escape_fd = open("/lind/README.md", O_RDONLY);
int escape_errno = errno;

if(escape_fd != -1) {
fprintf(stderr, "symlink_confinement test: FAIL -- opened "
"/lind/README.md from inside the cage, chroot escape\n");
close(escape_fd);
assert(0);
}
assert(escape_errno == ENOENT);

unlink("evil_link_readme");
assert(symlink("/lind/README.md", "evil_link_readme") == 0);

errno = 0;
int link_escape_fd = open("evil_link_readme", O_RDONLY);
int link_escape_errno = errno;
if(link_escape_fd != -1) {
fprintf(stderr, "symlink_confinement test: FAIL -- opened "
"/lind/README.md via symlink from inside the cage, "
"chroot escape via symlink target\n");
close(link_escape_fd);
assert(0);
}
assert(link_escape_errno == ENOENT);
unlink("evil_link_readme");

printf("symlink_confinement test: PASS\n");

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.

This test asserts that opening via evil_link behaves identically to opening /etc/passwd directly. if confinement were totally broken and both opens hit the host's real /etc/passwd, the two would still match each other and the test would still PASS.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yeah I agree with this.

return 0;
}