Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Syscall filtering PoC #267

Merged
merged 16 commits into from
Sep 12, 2023
Merged

Syscall filtering PoC #267

merged 16 commits into from
Sep 12, 2023

Conversation

fw-immunant
Copy link
Contributor

This adds some demos and infrastructure used by both those demos and eventually the final runtime. We still need to weld together the pieces in the manner described best in our slides, and the memory-map tracking is still WIP. I would plausibly prefer rewriting part of the memory-map tracking in Rust (memory_map.c) to have access to fancier data structures and more assurances.

@fw-immunant fw-immunant force-pushed the fw/syscall-filtering-poc branch 6 times, most recently from 89aafe8 to 247ae37 Compare August 11, 2023 00:22
@fw-immunant fw-immunant marked this pull request as ready for review August 11, 2023 01:20
@fw-immunant fw-immunant force-pushed the fw/syscall-filtering-poc branch from 247ae37 to 5147254 Compare August 11, 2023 01:20
@fw-immunant
Copy link
Contributor Author

There are demos of four runtime components here:

  • read-pkru demonstrates reading an inferior's current pkru value with ptrace, which is needed for memory-map tracking.
  • seccomp-filter demonstrates the basic structure of the seccomp syscall policy we need; to make this into the MVP for IA2 itself we'll just have to add a clause for each of the 24ish syscalls from Syscall prioritization #233.
  • landlock demonstrates the path exclusion we need for /proc/self/mem; try running DENY_PATH=/proc/self/mem ./landlock cat /proc/self/mem and compare to the same with a different DENY_PATH or programs that interact with a specified file in different ways.
  • track-memory-map demonstrates memory-map tracking; try running ../rewriter/tests/mmap_loop/mmap_loop_main_wrapped & followed by sudo ./track-memory-map $(pgrep mmap_loop).

@fw-immunant fw-immunant requested review from rinon and ahomescu August 11, 2023 01:29
@fw-immunant
Copy link
Contributor Author

memory_map.c currently relies on linear scans of a vector of memory mappings; I would much rather this used a tree so we get sustainable (logarithmic) scaling. We could open-code a simple binary tree in C, but I would be happier using Rust's BTreeMap, something along these lines.

Copy link
Collaborator

@rinon rinon left a comment

Choose a reason for hiding this comment

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

Mostly just minor comments.

Could you add a bit of doc comments (e.g. what you put in the PR comments) for each of the POC main files? Just so someone can tell what a thing should do when they open it.

I like the idea of doing the memory map tracker in Rust, let's do it.

rewriter/tests/mmap_loop/main.c Outdated Show resolved Hide resolved
rewriter/tests/mmap_loop/main.c Outdated Show resolved Hide resolved
runtime/CMakeLists.txt Outdated Show resolved Hide resolved
runtime/memory_map.c Outdated Show resolved Hide resolved
runtime/get_inferior_pkru.c Show resolved Hide resolved
Comment on lines +42 to +43
// this would compare syscall number to write() and allow if it matches
/*BPF_JUMP(BPF_JMP | BPF_JEQ | BPF_K, __NR_write, 0, 1),
BPF_STMT(BPF_RET | BPF_K, SECCOMP_RET_ALLOW),*/
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think these comment are necessary, the macros is pretty straightforward.

// a user notification fd to pass to the supervisor, but we also want to pass
// FLAG_TSYNC, and these two cannot be combined in one call because they
// impose conflicting interpretations on the syscall return value.
int sc_unotify_fd = syscall(SYS_seccomp, SECCOMP_SET_MODE_FILTER,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Might be worth adding a comment after this saying what we would do with this fd?

runtime/landlock.h Outdated Show resolved Hide resolved
runtime/track_memory_map_demo.c Outdated Show resolved Hide resolved
runtime/memory_map.c Outdated Show resolved Hide resolved
@fw-immunant fw-immunant force-pushed the fw/syscall-filtering-poc branch 4 times, most recently from 6d03c28 to 8758dd1 Compare September 1, 2023 21:40
Copy link
Collaborator

@rinon rinon left a comment

Choose a reason for hiding this comment

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

Have a few comments from last time that still seem relevant. New stuff looks ok, but I'll take another look tomorrow.

runtime/mem_region.h Outdated Show resolved Hide resolved
runtime/memory-map/src/disjoint_interval_tree.rs Outdated Show resolved Hide resolved
@fw-immunant fw-immunant force-pushed the fw/syscall-filtering-poc branch 4 times, most recently from 0ce7e62 to 2c232c9 Compare September 6, 2023 17:19
@fw-immunant fw-immunant force-pushed the fw/syscall-filtering-poc branch from 2c232c9 to e17a907 Compare September 11, 2023 23:55
use no_std as this reduces binary size from several megs to 36kB
also rename remove -> unmap because unmap can actually split regions but definitely does unmap exactly the region specified
this is not comprehensive, but is enough for tests to pass
…rtment at most once

we'll want to revisit this when #165 is solved and we can enforce stricter ownership over allocations
unfortunately, even when tracing a program from exec(), the initial mappings of stack and executable are pre-existing and not known to the tracer. we could read them from /proc/<pid>/maps, but for now we simply allow to protect these unknown regions of memory. TODO: file a bug on this
@fw-immunant fw-immunant force-pushed the fw/syscall-filtering-poc branch from e17a907 to dcc2167 Compare September 12, 2023 00:04
@fw-immunant
Copy link
Contributor Author

I said I wasn't going to clean up this history but I ended up doing it just to make sure the code was all in good shape. As such, merging via rebase rather than squash. Some of the intermediate commits are worth noticing individually as they're changes to our memory-management policy needed to run real programs, which we may want to revisit later.

@fw-immunant fw-immunant merged commit 988d6ea into main Sep 12, 2023
32 checks passed
@fw-immunant fw-immunant deleted the fw/syscall-filtering-poc branch September 12, 2023 00:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants