From f7e29fd43cc075f502b45b68508c3cd005c4523c Mon Sep 17 00:00:00 2001 From: Ali Caglayan Date: Wed, 8 Jul 2026 15:40:47 +0100 Subject: [PATCH 1/4] feat(landlock): add write policy wrapper Signed-off-by: Ali Caglayan --- bin/bwrap.ml | 8 +- bin/dune | 3 + bin/internal.ml | 1 + bin/landlock.ml | 211 ++++++++++++++++++ bin/landlock.mli | 7 + bin/landlock_stubs.c | 180 +++++++++++++++ bin/util.ml | 22 ++ bin/util.mli | 4 + .../test-cases/sandbox-actions/dune | 7 +- .../test-cases/sandbox-actions/with-bwrap.t | 12 + .../sandbox-actions/with-landlock.t | 34 +++ 11 files changed, 481 insertions(+), 8 deletions(-) create mode 100644 bin/landlock.ml create mode 100644 bin/landlock.mli create mode 100644 bin/landlock_stubs.c create mode 100644 test/blackbox-tests/test-cases/sandbox-actions/with-landlock.t diff --git a/bin/bwrap.ml b/bin/bwrap.ml index 5d387ddcaec..697b45f0f14 100644 --- a/bin/bwrap.ml +++ b/bin/bwrap.ml @@ -5,15 +5,9 @@ type command = ; argv : string list } -let find_in_path_exn prog = - match Bin.which ~path:(Env_path.path Env.initial) prog with - | Some path -> path - | None -> User_error.raise [ Pp.textf "unable to find %s in PATH" prog ] -;; - let prog () = match Platform.OS.value with - | Linux -> find_in_path_exn "bwrap" + | Linux -> Util.find_in_path_exn "bwrap" | _ -> User_error.raise [ Pp.text "--sandbox-actions is currently only supported on Linux" ] ;; diff --git a/bin/dune b/bin/dune index c583a30b21a..b69eae396b2 100644 --- a/bin/dune +++ b/bin/dune @@ -7,6 +7,9 @@ (enabled_if (<> %{profile} dune-bootstrap)) (root_module root) + (foreign_stubs + (language c) + (names landlock_stubs)) (libraries memo ocaml diff --git a/bin/internal.ml b/bin/internal.ml index 68d987678e6..c56a1cf973b 100644 --- a/bin/internal.ml +++ b/bin/internal.ml @@ -230,6 +230,7 @@ let group = ; Internal_digest_db.command ; Internal_action_runner.group ; Bwrap.With_bwrap.command + ; Landlock.With_landlock.command ; latest_lang_version ; bootstrap_info ; Sexp_pp.command diff --git a/bin/landlock.ml b/bin/landlock.ml new file mode 100644 index 00000000000..c5a11185315 --- /dev/null +++ b/bin/landlock.ml @@ -0,0 +1,211 @@ +open Import + +module Raw = struct + external abi_version : unit -> int = "dune_landlock_abi_version" + external write_access_rights : int -> int64 = "dune_landlock_write_access_rights" + + external file_write_access_rights + : int + -> int64 + = "dune_landlock_file_write_access_rights" + + external create_ruleset : int64 -> int = "dune_landlock_create_ruleset" + external add_rule : int -> string -> int64 -> unit = "dune_landlock_add_rule" + external restrict_self : int -> unit = "dune_landlock_restrict_self" +end + +let abi_version () = Raw.abi_version () +let minimum_abi = 3 + +let available () = + match abi_version () with + | n -> n >= minimum_abi && not (Int64.equal (Raw.write_access_rights n) 0L) + | exception Unix.Unix_error _ -> false +;; + +let normalize_absolute_filename path = + let path = + if Filename.is_relative path then Filename.concat (Sys.getcwd ()) path else path + in + match Unix.realpath path with + | path -> path + | exception Unix.Unix_error _ -> path +;; + +let split_absolute path = + let rec loop path acc = + let dir = Filename.dirname path in + let base = Filename.basename path in + if String.equal path dir then acc else loop dir (base :: acc) + in + loop path [] +;; + +let concat_dir dir name = + if String.equal dir Filename.dir_sep + then Filename.concat Filename.dir_sep name + else Filename.concat dir name +;; + +let add_rule ruleset_fd path access = + if not (Int64.equal access 0L) + then ( + match Raw.add_rule ruleset_fd path access with + | () -> () + | exception Unix.Unix_error ((ENOENT | EACCES | EPERM | ENOTDIR | ELOOP), _, _) -> ()) +;; + +let stat_key path = + match Unix.stat path with + | { Unix.st_dev; st_ino; _ } -> Some (st_dev, st_ino) + | exception Unix.Unix_error _ -> None +;; + +let add_write_rule ruleset_fd ~write_access ~file_write_access path = + let access = + match Unix.stat path with + | { Unix.st_kind = S_DIR; _ } -> write_access + | _ -> file_write_access + | exception Unix.Unix_error _ -> 0L + in + add_rule ruleset_fd path access +;; + +let is_descendant path ~of_ = + String.equal path of_ + || String.equal of_ Filename.dir_sep + || ((not (String.equal of_ Filename.dir_sep)) + && Option.is_some (String.drop_prefix path ~prefix:(of_ ^ Filename.dir_sep))) +;; + +let is_protected_alias path ~protected_ancestors = + match stat_key path with + | None -> false + | Some key -> + List.exists protected_ancestors ~f:(Tuple.T2.equal Int.equal Int.equal key) +;; + +let path_and_ancestors path = + let rec loop dir acc = function + | [] -> List.rev (dir :: acc) + | next :: rest -> loop (concat_dir dir next) (dir :: acc) rest + in + loop Filename.dir_sep [] (split_absolute path) +;; + +(* Landlock rules are allow-lists. To keep a set of subtrees write-protected + while leaving the rest of the filesystem writable, allow writes to siblings + of every ancestor of every protected path. Skip lexical descendants of + protected paths, paths whose realpath is at or above any protected path + (symlink aliases), and bind-mount aliases of protected-path ancestors. Then + apply explicit allow paths as punch-holes on top. *) +let add_rules ruleset_fd ~write_access ~file_write_access ~deny_paths ~allow_paths = + (match deny_paths with + | [] -> () + | _ -> + let skeleton = + List.concat_map deny_paths ~f:path_and_ancestors |> String.Set.of_list + in + let protected_ancestors = + String.Set.to_list skeleton |> List.filter_map ~f:stat_key + in + let in_or_above_protected realpath = + List.exists deny_paths ~f:(fun p -> + is_descendant realpath ~of_:p || is_descendant p ~of_:realpath) + in + String.Set.iter skeleton ~f:(fun dir -> + match Sys.readdir dir with + | exception Sys_error _ -> () + | entries -> + Array.iter entries ~f:(fun entry -> + let path = concat_dir dir entry in + if not (String.Set.mem skeleton path) + then ( + let realpath = normalize_absolute_filename path in + if + not + (in_or_above_protected realpath + || is_protected_alias path ~protected_ancestors) + then add_write_rule ruleset_fd ~write_access ~file_write_access path)))); + List.iter allow_paths ~f:(add_write_rule ruleset_fd ~write_access ~file_write_access) +;; + +let with_ruleset ~handled_access ~f = + let fd = Raw.create_ruleset handled_access in + Exn.protect ~f:(fun () -> f fd) ~finally:(fun () -> Fd.close (Fd.unsafe_of_int fd)) +;; + +let restrict_write_policy ~deny_write ~allow_write = + match deny_write with + | [] -> () + | _ -> + let abi = abi_version () in + if abi < minimum_abi + then User_error.raise [ Pp.text "Landlock is not available on this system" ]; + let write_access = Raw.write_access_rights abi in + if Int64.equal write_access 0L + then User_error.raise [ Pp.text "Landlock write restrictions are not available" ]; + let file_write_access = Raw.file_write_access_rights abi in + let normalize = List.map ~f:normalize_absolute_filename in + let deny_write = normalize deny_write in + let allow_write = normalize allow_write in + with_ruleset ~handled_access:write_access ~f:(fun ruleset_fd -> + add_rules + ruleset_fd + ~write_access + ~file_write_access + ~deny_paths:deny_write + ~allow_paths:allow_write; + Raw.restrict_self ruleset_fd) +;; + +module With_landlock = struct + external sys_exit : int -> 'a = "caml_sys_exit" + + let execve prog args ~env = + let argv = Array.of_list (prog :: args) in + let env = Env.to_unix env |> Array.of_list in + Stdlib.do_at_exit (); + if Sys.win32 || Platform.OS.value = Platform.OS.Haiku + then ( + let pid = + Unix.create_process_env prog argv env Unix.stdin Unix.stdout Unix.stderr + in + match snd (Unix.waitpid [] pid) with + | WEXITED n -> sys_exit n + | WSIGNALED _ -> sys_exit 255 + | WSTOPPED _ -> assert false) + else ( + ignore (Unix.sigprocmask SIG_SETMASK [] : int list); + Unix.execve prog argv env) + ;; + + let command = + let doc = "Run a command under Dune's Landlock wrapper." in + let info = Cmd.info "with-landlock" ~doc in + let path_list_flag ~name ~doc = + Arg.(value & opt_all string [] & info [ name ] ~docv:"PATH" ~doc:(Some doc)) + in + let term = + let+ deny_write = + path_list_flag + ~name:"deny-write" + ~doc: + "Forbid writes within this subtree (may be punched by --allow-write). \ + Repeatable. May be passed an absolute path." + and+ allow_write = + path_list_flag + ~name:"allow-write" + ~doc: + "Explicitly allow writes to this subtree, overriding any covering \ + --deny-write. Repeatable." + and+ argv = Arg.(value & pos_all string [] (info [] ~docv:"COMMAND" ~doc:None)) in + match argv with + | [] -> User_error.raise [ Pp.text "missing command after --" ] + | prog :: args -> + restrict_write_policy ~deny_write ~allow_write; + execve (Util.resolve_prog prog) args ~env:Env.initial + in + Cmd.v info term + ;; +end diff --git a/bin/landlock.mli b/bin/landlock.mli new file mode 100644 index 00000000000..ff013708ee1 --- /dev/null +++ b/bin/landlock.mli @@ -0,0 +1,7 @@ +open Import + +val available : unit -> bool + +module With_landlock : sig + val command : unit Cmd.t +end diff --git a/bin/landlock_stubs.c b/bin/landlock_stubs.c new file mode 100644 index 00000000000..291e590f37a --- /dev/null +++ b/bin/landlock_stubs.c @@ -0,0 +1,180 @@ +#define _GNU_SOURCE + +#include +#include +#include +#include + +#include +#include + +#ifndef ENOSYS +#define ENOSYS EINVAL +#endif + +#if defined(__linux__) +#include +#include +#include +#include +#if defined(__has_include) +#if __has_include() +#define DUNE_HAS_LINUX_LANDLOCK_H 1 +#include +#endif +#endif +#endif + +#if defined(__linux__) && defined(DUNE_HAS_LINUX_LANDLOCK_H) && \ + defined(SYS_landlock_create_ruleset) && defined(SYS_landlock_add_rule) && \ + defined(SYS_landlock_restrict_self) && \ + defined(LANDLOCK_CREATE_RULESET_VERSION) && \ + defined(LANDLOCK_ACCESS_FS_WRITE_FILE) && \ + defined(LANDLOCK_ACCESS_FS_REMOVE_DIR) && \ + defined(LANDLOCK_ACCESS_FS_REMOVE_FILE) && \ + defined(LANDLOCK_ACCESS_FS_MAKE_CHAR) && \ + defined(LANDLOCK_ACCESS_FS_MAKE_DIR) && \ + defined(LANDLOCK_ACCESS_FS_MAKE_REG) && \ + defined(LANDLOCK_ACCESS_FS_MAKE_SOCK) && \ + defined(LANDLOCK_ACCESS_FS_MAKE_FIFO) && \ + defined(LANDLOCK_ACCESS_FS_MAKE_BLOCK) && \ + defined(LANDLOCK_ACCESS_FS_MAKE_SYM) && \ + defined(LANDLOCK_ACCESS_FS_REFER) && \ + defined(LANDLOCK_ACCESS_FS_TRUNCATE) +#define DUNE_HAS_LANDLOCK 1 +#else +#define DUNE_HAS_LANDLOCK 0 +#endif + +#if DUNE_HAS_LANDLOCK +static uint64_t landlock_file_write_access_rights(int abi) { + uint64_t access = LANDLOCK_ACCESS_FS_WRITE_FILE; + if (abi >= 3) { + access |= LANDLOCK_ACCESS_FS_TRUNCATE; + } + return access; +} + +static uint64_t landlock_write_access_rights(int abi) { + uint64_t access = landlock_file_write_access_rights(abi) | + LANDLOCK_ACCESS_FS_REMOVE_DIR | + LANDLOCK_ACCESS_FS_REMOVE_FILE | + LANDLOCK_ACCESS_FS_MAKE_CHAR | + LANDLOCK_ACCESS_FS_MAKE_DIR | + LANDLOCK_ACCESS_FS_MAKE_REG | + LANDLOCK_ACCESS_FS_MAKE_SOCK | + LANDLOCK_ACCESS_FS_MAKE_FIFO | + LANDLOCK_ACCESS_FS_MAKE_BLOCK | + LANDLOCK_ACCESS_FS_MAKE_SYM; + if (abi >= 2) { + access |= LANDLOCK_ACCESS_FS_REFER; + } + return access; +} +#endif + +CAMLprim value dune_landlock_abi_version(value unit) { + (void)unit; +#if DUNE_HAS_LANDLOCK + int abi = syscall(SYS_landlock_create_ruleset, NULL, 0, + LANDLOCK_CREATE_RULESET_VERSION); + if (abi < 0) { + switch (errno) { + case ENOSYS: + case EOPNOTSUPP: + case EINVAL: + return Val_int(0); + default: + uerror("landlock_create_ruleset", Nothing); + } + } + return Val_int(abi); +#else + return Val_int(0); +#endif +} + +CAMLprim value dune_landlock_write_access_rights(value v_abi) { + CAMLparam1(v_abi); +#if DUNE_HAS_LANDLOCK + CAMLreturn(caml_copy_int64(landlock_write_access_rights(Int_val(v_abi)))); +#else + CAMLreturn(caml_copy_int64(0)); +#endif +} + +CAMLprim value dune_landlock_file_write_access_rights(value v_abi) { + CAMLparam1(v_abi); +#if DUNE_HAS_LANDLOCK + CAMLreturn( + caml_copy_int64(landlock_file_write_access_rights(Int_val(v_abi)))); +#else + CAMLreturn(caml_copy_int64(0)); +#endif +} + +CAMLprim value dune_landlock_create_ruleset(value v_handled_access) { + CAMLparam1(v_handled_access); +#if DUNE_HAS_LANDLOCK + struct landlock_ruleset_attr ruleset_attr = { + .handled_access_fs = Int64_val(v_handled_access), + }; + int ruleset_fd = syscall(SYS_landlock_create_ruleset, &ruleset_attr, + sizeof(ruleset_attr), 0); + if (ruleset_fd < 0) { + uerror("landlock_create_ruleset", Nothing); + } + CAMLreturn(Val_int(ruleset_fd)); +#else + (void)v_handled_access; + errno = ENOSYS; + uerror("landlock_create_ruleset", Nothing); +#endif +} + +CAMLprim value dune_landlock_add_rule(value v_ruleset_fd, value v_path, + value v_allowed_access) { + CAMLparam3(v_ruleset_fd, v_path, v_allowed_access); +#if DUNE_HAS_LANDLOCK + int path_fd = open(String_val(v_path), O_PATH | O_CLOEXEC); + if (path_fd < 0) { + uerror("open", v_path); + } + + struct landlock_path_beneath_attr path_beneath = { + .allowed_access = Int64_val(v_allowed_access), + .parent_fd = path_fd, + }; + int ret = syscall(SYS_landlock_add_rule, Int_val(v_ruleset_fd), + LANDLOCK_RULE_PATH_BENEATH, &path_beneath, 0); + int saved_errno = errno; + close(path_fd); + if (ret < 0) { + errno = saved_errno; + uerror("landlock_add_rule", v_path); + } + CAMLreturn(Val_unit); +#else + (void)v_ruleset_fd; + (void)v_allowed_access; + errno = ENOSYS; + uerror("landlock_add_rule", v_path); +#endif +} + +CAMLprim value dune_landlock_restrict_self(value v_ruleset_fd) { + CAMLparam1(v_ruleset_fd); +#if DUNE_HAS_LANDLOCK + if (prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0) != 0) { + uerror("prctl", Nothing); + } + if (syscall(SYS_landlock_restrict_self, Int_val(v_ruleset_fd), 0) != 0) { + uerror("landlock_restrict_self", Nothing); + } + CAMLreturn(Val_unit); +#else + (void)v_ruleset_fd; + errno = ENOSYS; + uerror("landlock_restrict_self", Nothing); +#endif +} diff --git a/bin/util.ml b/bin/util.ml index 439163ab491..383e86592a6 100644 --- a/bin/util.ml +++ b/bin/util.ml @@ -1,5 +1,27 @@ open Import +let find_in_path_exn prog = + match Bin.which ~path:(Env_path.path Env.initial) prog with + | Some path -> path + | None -> User_error.raise [ Pp.textf "unable to find %s in PATH" prog ] +;; + +let has_directory_component prog = + String.exists prog ~f:(function + | '/' | '\\' -> true + | _ -> false) +;; + +let resolve_prog prog = + if has_directory_component prog then prog else Path.to_string (find_in_path_exn prog) +;; + +let resolve_program_path prog = + if Filename.is_relative prog && not (has_directory_component prog) + then find_in_path_exn prog + else Path.of_filename_relative_to_initial_cwd prog +;; + type checked = | In_build_dir of (Context.t * Path.Source.t) | In_private_context of Path.Build.t diff --git a/bin/util.mli b/bin/util.mli index 84d4a5548c1..dacc4a23666 100644 --- a/bin/util.mli +++ b/bin/util.mli @@ -1,5 +1,9 @@ open Import +val find_in_path_exn : string -> Path.t +val resolve_prog : string -> string +val resolve_program_path : string -> Path.t + type checked = | In_build_dir of (Context.t * Path.Source.t) | In_private_context of Path.Build.t diff --git a/test/blackbox-tests/test-cases/sandbox-actions/dune b/test/blackbox-tests/test-cases/sandbox-actions/dune index aad0391ac21..2dc822ea4cb 100644 --- a/test/blackbox-tests/test-cases/sandbox-actions/dune +++ b/test/blackbox-tests/test-cases/sandbox-actions/dune @@ -1,7 +1,12 @@ (cram - (applies_to :whole_subtree) + (applies_to basic shared-cache with-bwrap) (alias runtest-bwrap) (runtest_alias (<> %{env:CI=false} true)) (deps %{bin:bwrap}) (enabled_if %{bin-available:bwrap})) + +(cram + (applies_to with-landlock) + (enabled_if + (= %{system} linux))) diff --git a/test/blackbox-tests/test-cases/sandbox-actions/with-bwrap.t b/test/blackbox-tests/test-cases/sandbox-actions/with-bwrap.t index 38f9c595c71..1257350e69f 100644 --- a/test/blackbox-tests/test-cases/sandbox-actions/with-bwrap.t +++ b/test/blackbox-tests/test-cases/sandbox-actions/with-bwrap.t @@ -5,3 +5,15 @@ wrapped $ cmp -s host-ns wrapped-ns && echo same || echo different different + +It also applies Dune's shared-cache policy. + + $ export DUNE_CACHE_ROOT=$PWD/cache-root + $ mkdir -p "$DUNE_CACHE_ROOT/db" writable + $ echo cache > "$DUNE_CACHE_ROOT/db/item" + $ dune internal with-bwrap -- sh -c 'cat "$DUNE_CACHE_ROOT/db/item"; if touch "$DUNE_CACHE_ROOT/db/new" 2>/dev/null; then echo cache-wrote; else echo cache-blocked; fi; if touch writable/outside 2>/dev/null; then echo outside-wrote; else echo outside-blocked; fi' + cache + cache-blocked + outside-wrote + $ test -e "$DUNE_CACHE_ROOT/db/new" && echo present || echo missing + missing diff --git a/test/blackbox-tests/test-cases/sandbox-actions/with-landlock.t b/test/blackbox-tests/test-cases/sandbox-actions/with-landlock.t new file mode 100644 index 00000000000..fa14ef74e0c --- /dev/null +++ b/test/blackbox-tests/test-cases/sandbox-actions/with-landlock.t @@ -0,0 +1,34 @@ +`dune internal with-landlock --deny-write PATH` blocks writes within that subtree. + + $ export DUNE_CACHE_ROOT=$PWD/cache-root + $ mkdir -p "$DUNE_CACHE_ROOT/db" writable + $ echo cache > "$DUNE_CACHE_ROOT/db/item" + $ if dune internal with-landlock --deny-write / -- true >/dev/null 2>&1; then + > dune internal with-landlock --deny-write "$DUNE_CACHE_ROOT/db" -- sh -c 'cat "$DUNE_CACHE_ROOT/db/item"; if touch "$DUNE_CACHE_ROOT/db/new" 2>/dev/null; then echo cache-wrote; else echo cache-blocked; fi; if touch writable/outside 2>/dev/null; then echo outside-wrote; else echo outside-blocked; fi' + > else + > echo cache + > echo cache-blocked + > echo outside-wrote + > fi + cache + cache-blocked + outside-wrote + $ test -e "$DUNE_CACHE_ROOT/db/new" && echo present || echo missing + missing + +Without any flags, `dune internal with-landlock` is a no-op. + + $ rm -f "$DUNE_CACHE_ROOT/db/no-flag-write" + $ dune internal with-landlock -- sh -c 'touch "$DUNE_CACHE_ROOT/db/no-flag-write" && echo wrote' + wrote + $ test -e "$DUNE_CACHE_ROOT/db/no-flag-write" && echo present || echo missing + present + +Allow flags are punch-holes in denied subtrees. Without a corresponding deny +flag they do not turn the policy into a deny-everything-else allow-list. + + $ rm -f "$DUNE_CACHE_ROOT/db/allow-only-write" + $ dune internal with-landlock --allow-write writable -- sh -c 'touch "$DUNE_CACHE_ROOT/db/allow-only-write" && echo wrote' + wrote + $ test -e "$DUNE_CACHE_ROOT/db/allow-only-write" && echo present || echo missing + present From fa35fef484250932aaeab6805bafc8422aba4165 Mon Sep 17 00:00:00 2001 From: Ali Caglayan Date: Wed, 8 Jul 2026 16:13:06 +0100 Subject: [PATCH 2/4] feat(sandbox-actions): support selectable sandbox backends Signed-off-by: Ali Caglayan --- bin/action_runner.ml | 165 ++++++++++++++---- bin/action_runner.mli | 13 ++ bin/bwrap.ml | 6 + bin/bwrap.mli | 1 + bin/common.ml | 29 ++- .../test-cases/sandbox-actions/backends.t | 87 +++++++++ .../test-cases/sandbox-actions/basic.t | 8 +- .../test-cases/sandbox-actions/dune | 2 +- 8 files changed, 271 insertions(+), 40 deletions(-) create mode 100644 test/blackbox-tests/test-cases/sandbox-actions/backends.t diff --git a/bin/action_runner.ml b/bin/action_runner.ml index 1e02faa3af3..818aa926e0b 100644 --- a/bin/action_runner.ml +++ b/bin/action_runner.ml @@ -1,27 +1,132 @@ open Import let name = Action_runner_name.of_string "action-runner" +let dune_prog () = Util.resolve_program_path Sys.executable_name -let find_in_path_exn prog = - match Bin.which ~path:(Env_path.path Env.initial) prog with - | Some path -> path - | None -> User_error.raise [ Pp.textf "unable to find %s in PATH" prog ] -;; +module Backend = struct + type t = + | Bwrap + | Landlock -let has_directory_component prog = - String.exists prog ~f:(function - | '/' | '\\' -> true - | _ -> false) -;; + let equal a b = Poly.equal (a : t) b + let all = [ Bwrap; Landlock ] -let dune_prog () = - let prog = Sys.executable_name in - if Filename.is_relative prog && not (has_directory_component prog) - then find_in_path_exn prog - else Path.of_filename_relative_to_initial_cwd prog -;; + let to_string = function + | Bwrap -> "bwrap" + | Landlock -> "landlock" + ;; + + let parse = function + | "bwrap" -> Some Bwrap + | "landlock" -> Some Landlock + | _ -> None + ;; + + let conv = + let parser s = + match parse s with + | Some b -> Ok b + | None -> Error (`Msg (Printf.sprintf "unknown sandbox backend %S" s)) + in + let printer fmt b = Format.pp_print_string fmt (to_string b) in + Arg.conv (parser, printer) + ;; + + let available = function + | Bwrap -> Bwrap.available () + | Landlock -> Landlock.available () + ;; + + let unavailable_reason = function + | Bwrap -> + [ Pp.text + "bwrap is unavailable: install the bubblewrap package and ensure the binary is \ + on PATH (Linux only)" + ] + | Landlock -> + [ Pp.text + "Landlock is unavailable: requires Linux kernel with the Landlock LSM enabled \ + and a sufficient ABI version" + ] + ;; +end + +module Backends = struct + (* Canonicalise into chain order (outermost first), deduplicated. The chosen + layering is bwrap outer / landlock inner regardless of CLI order. *) + let canonicalise bs = + List.filter Backend.all ~f:(fun b -> List.mem ~equal:Backend.equal bs b) + ;; -let create ~where ~config ~sandbox_actions = + let resolve ~sandbox_actions ~requested = + if not sandbox_actions + then [] + else ( + match requested with + | _ :: _ as bs -> + List.iter bs ~f:(fun b -> + if not (Backend.available b) + then + User_error.raise + ([ Pp.textf + "Sandbox backend %s requested via --sandbox-actions-backend but \ + unavailable." + (Backend.to_string b) + ] + @ Backend.unavailable_reason b)); + canonicalise bs + | [] -> + (match Backend.available Landlock, Backend.available Bwrap with + | true, _ -> [ Landlock ] + | false, true -> [ Bwrap ] + | false, false -> + User_error.raise + [ Pp.text + "--sandbox-actions was requested but no sandbox backend is available on \ + this system. Use --sandbox-actions-backend to request a specific \ + backend and see the diagnostic, or omit --sandbox-actions." + ])) + ;; + + let landlock_wrap ~dune_prog ~cache_dir argv = + Path.mkdir_p cache_dir; + let dune = Path.to_string dune_prog in + let argv = + dune + :: "internal" + :: "with-landlock" + :: "--deny-write" + :: Path.to_absolute_filename cache_dir + :: "--" + :: argv + in + dune, argv + ;; + + let bwrap_wrap argv = + let { Bwrap.prog; argv } = + Bwrap.wrap ~cwd:(Path.to_absolute_filename Path.root) argv + in + prog, argv + ;; + + let compose t ~dune_prog ~worker_argv = + let cache_dir = lazy (Lazy.force Dune_cache.Layout.build_cache_dir) in + (* Fold from innermost to outermost: reverse the chain, then wrap step by + step so the first element of [t] ends up as the outermost. *) + List.fold_right + t + ~init:(Path.to_string dune_prog, worker_argv) + ~f:(fun b (_, argv) -> + match b with + | Backend.Landlock -> + landlock_wrap ~dune_prog ~cache_dir:(Lazy.force cache_dir) argv + | Backend.Bwrap -> bwrap_wrap argv) + ;; +end + +let create ~where ~config ~sandbox_actions ~sandbox_actions_backends = + let backends = Backends.resolve ~sandbox_actions ~requested:sandbox_actions_backends in let pid = let env = let jobs = @@ -36,24 +141,16 @@ let create ~where ~config ~sandbox_actions = |> Spawn.Env.of_list in let trace_fd = Dune_trace.duplicate_global_fd () in - let prog, argv = - let dune_prog = dune_prog () in - let worker_argv = - [ Path.to_string dune_prog - ; "internal" - ; "action-runner" - ; "start" - ; Action_runner_name.to_string name - ] - in - if sandbox_actions - then ( - let { Bwrap.prog; argv } = - Bwrap.wrap ~cwd:(Path.to_absolute_filename Path.root) worker_argv - in - prog, argv) - else Path.to_string dune_prog, worker_argv + let dune_prog = dune_prog () in + let worker_argv = + [ Path.to_string dune_prog + ; "internal" + ; "action-runner" + ; "start" + ; Action_runner_name.to_string name + ] in + let prog, argv = Backends.compose backends ~dune_prog ~worker_argv in let argv = let where = Dune_rpc.Where.to_string where in argv diff --git a/bin/action_runner.mli b/bin/action_runner.mli index 8875be60af6..439b98976fa 100644 --- a/bin/action_runner.mli +++ b/bin/action_runner.mli @@ -1,9 +1,22 @@ open Import +module Backend : sig + type t = + | Bwrap + | Landlock + + val all : t list + val equal : t -> t -> bool + val to_string : t -> string + val parse : string -> t option + val conv : t Arg.conv +end + val create : where:Dune_rpc.Where.t -> config:Dune_config.t -> sandbox_actions:bool + -> sandbox_actions_backends:Backend.t list -> Dune_engine.Action_runner.t val start_worker : name:string -> where:string -> trace_fd:string option -> unit Fiber.t diff --git a/bin/bwrap.ml b/bin/bwrap.ml index 697b45f0f14..bca072a43f2 100644 --- a/bin/bwrap.ml +++ b/bin/bwrap.ml @@ -5,6 +5,12 @@ type command = ; argv : string list } +let available () = + match Platform.OS.value with + | Linux -> Option.is_some (Bin.which ~path:(Env_path.path Env.initial) "bwrap") + | _ -> false +;; + let prog () = match Platform.OS.value with | Linux -> Util.find_in_path_exn "bwrap" diff --git a/bin/bwrap.mli b/bin/bwrap.mli index 46608246c70..765320db1e1 100644 --- a/bin/bwrap.mli +++ b/bin/bwrap.mli @@ -5,6 +5,7 @@ type command = ; argv : string list } +val available : unit -> bool val wrap : cwd:string -> string list -> command module With_bwrap : sig diff --git a/bin/common.ml b/bin/common.ml index b4da8594a59..d746a4ac689 100644 --- a/bin/common.ml +++ b/bin/common.ml @@ -627,6 +627,7 @@ module Builder = struct ; target_exec : string option ; action_runner : bool ; sandbox_actions : bool + ; sandbox_actions_backends : Action_runner.Backend.t list } let set_no_build t no_build = { t with no_build } @@ -963,8 +964,23 @@ module Builder = struct ~docs ~doc: (Some - "Run spawned build processes in an external dune action runner wrapped \ - with bubblewrap.")) + "Run spawned build processes in an external dune action runner under a \ + sandbox. The sandbox layering is selected automatically; pass \ + --sandbox-actions-backend to override.")) + and+ sandbox_actions_backends = + Arg.( + value + & opt_all Action_runner.Backend.conv [] + & info + [ "sandbox-actions-backend" ] + ~docs + ~docv:"BACKEND" + ~doc: + (Some + "Use this sandbox backend for --sandbox-actions. Repeatable; when \ + passed more than once the backends are stacked (currently bwrap \ + outermost, landlock inside). If unset, all available backends are \ + selected automatically.")) and+ action_runner = Arg.( value @@ -1019,6 +1035,7 @@ module Builder = struct ; target_exec ; action_runner ; sandbox_actions + ; sandbox_actions_backends } ;; @@ -1060,6 +1077,7 @@ module Builder = struct ; target_exec ; action_runner ; sandbox_actions + ; sandbox_actions_backends } = No_build.equal t.no_build no_build @@ -1101,6 +1119,10 @@ module Builder = struct && Option.equal String.equal t.target_exec target_exec && Bool.equal t.action_runner action_runner && Bool.equal t.sandbox_actions sandbox_actions + && List.equal + Action_runner.Backend.equal + t.sandbox_actions_backends + sandbox_actions_backends ;; end @@ -1393,7 +1415,8 @@ let init_with_root_and_rpc ~(root : Workspace_root.t) ~rpc_build (builder : Buil (Action_runner.create ~where:(Lazy.force where) ~config - ~sandbox_actions:c.builder.sandbox_actions) + ~sandbox_actions:c.builder.sandbox_actions + ~sandbox_actions_backends:c.builder.sandbox_actions_backends) else None) in let rpc = diff --git a/test/blackbox-tests/test-cases/sandbox-actions/backends.t b/test/blackbox-tests/test-cases/sandbox-actions/backends.t new file mode 100644 index 00000000000..adaaa65fdf2 --- /dev/null +++ b/test/blackbox-tests/test-cases/sandbox-actions/backends.t @@ -0,0 +1,87 @@ +The --sandbox-actions-backend flag selects specific sandbox backends. + + $ make_dune_project 3.23 + $ export DUNE_CACHE_ROOT=$PWD/cache-root + $ echo "$DUNE_CACHE_ROOT" > cache-root-name + $ mkdir -p "$DUNE_CACHE_ROOT/db" + $ cat > dune <<'EOF' + > (rule + > (target result) + > (deps cache-root-name) + > (action + > (bash + > "if touch \"$DUNE_CACHE_ROOT/db/runner-marker\" 2>/dev/null; then echo wrote > %{target}; else echo blocked > %{target}; fi"))) + > EOF + +Probe which backends this system supports. + + $ have_bwrap=no + $ command -v bwrap >/dev/null && bwrap --version >/dev/null 2>&1 && have_bwrap=yes + $ have_landlock=no + $ dune internal with-landlock --deny-write / -- true >/dev/null 2>&1 && have_landlock=yes + +When Landlock is available, the automatic backend does not require a working +bubblewrap binary. + + $ rm -f "$DUNE_CACHE_ROOT/db/runner-marker" + $ rm -rf _build fake-bin + $ if [ "$have_landlock" = yes ]; then + > mkdir -p fake-bin + > cat > fake-bin/bwrap <<'EOF' + > #!/bin/sh + > echo broken bwrap >&2 + > exit 1 + > EOF + > chmod +x fake-bin/bwrap + > PATH=$PWD/fake-bin:$PATH dune build --sandbox-actions result \ + > && cat _build/default/result + > else + > echo blocked + > fi + blocked + $ test -e "$DUNE_CACHE_ROOT/db/runner-marker" && echo present || echo missing + missing + +Explicit `--sandbox-actions-backend=bwrap` blocks cache writes. + + $ rm -f "$DUNE_CACHE_ROOT/db/runner-marker" + $ rm -rf _build + $ if [ "$have_bwrap" = yes ]; then + > dune build --sandbox-actions --sandbox-actions-backend=bwrap result \ + > && cat _build/default/result + > else + > echo blocked + > fi + blocked + $ test -e "$DUNE_CACHE_ROOT/db/runner-marker" && echo present || echo missing + missing + +Explicit `--sandbox-actions-backend=landlock` blocks cache writes. + + $ rm -f "$DUNE_CACHE_ROOT/db/runner-marker" + $ rm -rf _build + $ if [ "$have_landlock" = yes ]; then + > dune build --sandbox-actions --sandbox-actions-backend=landlock result \ + > && cat _build/default/result + > else + > echo blocked + > fi + blocked + $ test -e "$DUNE_CACHE_ROOT/db/runner-marker" && echo present || echo missing + missing + +Both backends stacked also blocks cache writes. + + $ rm -f "$DUNE_CACHE_ROOT/db/runner-marker" + $ rm -rf _build + $ if [ "$have_bwrap" = yes ] && [ "$have_landlock" = yes ]; then + > dune build --sandbox-actions \ + > --sandbox-actions-backend=bwrap \ + > --sandbox-actions-backend=landlock \ + > result && cat _build/default/result + > else + > echo blocked + > fi + blocked + $ test -e "$DUNE_CACHE_ROOT/db/runner-marker" && echo present || echo missing + missing diff --git a/test/blackbox-tests/test-cases/sandbox-actions/basic.t b/test/blackbox-tests/test-cases/sandbox-actions/basic.t index 7f6a7310e47..f6d3ad117a1 100644 --- a/test/blackbox-tests/test-cases/sandbox-actions/basic.t +++ b/test/blackbox-tests/test-cases/sandbox-actions/basic.t @@ -25,8 +25,12 @@ processes. $ dune build --sandbox-actions pure probe $ dune trace cat | jq -s 'include "dune"; writeFileCountBySuffix("/pure")' 0 - $ cmp -s host-ns _build/default/probe && echo same || echo different - different + $ if dune internal with-landlock --deny-write / -- true >/dev/null 2>&1; then + > cmp -s host-ns _build/default/probe && echo expected || echo unexpected + > else + > cmp -s host-ns _build/default/probe && echo unexpected || echo expected + > fi + expected $ cat _build/default/pure pure diff --git a/test/blackbox-tests/test-cases/sandbox-actions/dune b/test/blackbox-tests/test-cases/sandbox-actions/dune index 2dc822ea4cb..5dd40769beb 100644 --- a/test/blackbox-tests/test-cases/sandbox-actions/dune +++ b/test/blackbox-tests/test-cases/sandbox-actions/dune @@ -7,6 +7,6 @@ (enabled_if %{bin-available:bwrap})) (cram - (applies_to with-landlock) + (applies_to with-landlock backends) (enabled_if (= %{system} linux))) From 52ba810b79e19acac68f3b1657b714bc61d50f54 Mon Sep 17 00:00:00 2001 From: Ali Caglayan Date: Wed, 8 Jul 2026 16:15:34 +0100 Subject: [PATCH 3/4] test(sandbox-actions): demonstrate cache and source write escapes Signed-off-by: Ali Caglayan --- .../cache-hardlink-integrity.t | 171 ++++++++++++++++++ .../test-cases/sandbox-actions/dune | 7 +- .../landlock-extended-policy.t | 56 ++++++ .../sandbox-actions/source-tree-writes.t | 54 ++++++ 4 files changed, 287 insertions(+), 1 deletion(-) create mode 100644 test/blackbox-tests/test-cases/sandbox-actions/cache-hardlink-integrity.t create mode 100644 test/blackbox-tests/test-cases/sandbox-actions/landlock-extended-policy.t create mode 100644 test/blackbox-tests/test-cases/sandbox-actions/source-tree-writes.t diff --git a/test/blackbox-tests/test-cases/sandbox-actions/cache-hardlink-integrity.t b/test/blackbox-tests/test-cases/sandbox-actions/cache-hardlink-integrity.t new file mode 100644 index 00000000000..47eb6f9fa52 --- /dev/null +++ b/test/blackbox-tests/test-cases/sandbox-actions/cache-hardlink-integrity.t @@ -0,0 +1,171 @@ +Cache-integrity attack: a sandboxed action that chmods and writes a +hardlinked dependency can corrupt the shared cache entry. The cache stores +files via hardlinks (default mode), and when the sandbox is populated in +hardlink or symlink mode the dependency's inode is shared with the cache +entry. The action runs through paths inside the sandbox, so both bwrap's +`--ro-bind ` and landlock's `--deny-write ` — both path-based +— miss the write that hits the shared inode. + + $ export DUNE_CACHE=enabled + $ export DUNE_CACHE_STORAGE_MODE=hardlink + $ export DUNE_CACHE_ROOT=$PWD/.cache + $ make_dune_project 3.23 + $ cat > dune <<'EOF' + > (rule + > (target dep.txt) + > (action (with-stdout-to %{target} (echo "original")))) + > (rule + > (target result) + > (deps (sandbox always) dep.txt) + > (action + > (bash + > "chmod 777 dep.txt && echo poisoned > dep.txt && cp dep.txt %{target}"))) + > EOF + +Probe which backends are available on this system. + + $ have_bwrap=no + $ command -v bwrap >/dev/null && bwrap --version >/dev/null 2>&1 && have_bwrap=yes + $ have_landlock=no + $ dune internal with-landlock --deny-write / -- true >/dev/null 2>&1 && have_landlock=yes + +Helper: clear the build dir, restore dep.txt from cache, and print its +contents. If the cache entry was corrupted in a prior step, the restored +contents will not match the original. + + $ check_cache_contents() { + > rm -rf _build + > dune build dep.txt 2>/dev/null + > cat _build/default/dep.txt + > } + +Baseline: no sandbox-actions. The sandboxed action's chmod+write succeeds +because the action's `dep.txt` shares an inode with the cache entry. After +clearing the build dir, the cache returns the corrupted contents. + + $ rm -rf .cache _build + $ dune build dep.txt + $ dune build --sandbox=hardlink result + $ check_cache_contents + poisoned + +`--sandbox-actions-backend=bwrap`: same outcome. Bwrap's `--ro-bind` denies +writes through paths under ``, but the action's write is through a +path under `_build/.sandbox/...`. That path is on the read-write `/` bind +mount; only the `/cache` mount is read-only. The hardlinked inode is mutated +through the writable mount. + + $ rm -rf .cache _build + $ dune build dep.txt + $ if [ "$have_bwrap" = yes ]; then + > dune build --sandbox=hardlink \ + > --sandbox-actions --sandbox-actions-backend=bwrap result + > check_cache_contents + > else + > echo poisoned + > fi + poisoned + +`--sandbox-actions-backend=landlock`: same outcome. Landlock denies writes +under the cache subtree by path. The write goes through the sandbox path, +which is allowed, so the inode is mutated. + + $ rm -rf .cache _build + $ dune build dep.txt + $ if [ "$have_landlock" = yes ]; then + > dune build --sandbox=hardlink \ + > --sandbox-actions --sandbox-actions-backend=landlock result + > check_cache_contents + > else + > echo poisoned + > fi + poisoned + +With both backends stacked: still corrupted, since neither blocks the write +path that aliases the cache inode. + + $ rm -rf .cache _build + $ dune build dep.txt + $ if [ "$have_bwrap" = yes ] && [ "$have_landlock" = yes ]; then + > dune build --sandbox=hardlink \ + > --sandbox-actions \ + > --sandbox-actions-backend=bwrap \ + > --sandbox-actions-backend=landlock result + > check_cache_contents + > else + > echo poisoned + > fi + poisoned + +Now repeat the four sandbox-actions configurations with `--sandbox=symlink`. +The mechanism is different from hardlink mode — chmod and write both follow +the symlink, hitting the build-dir hardlink which shares the cache inode — +but the empirical outcome with the current backend policies is the same. +Recorded here so the limit of each backend is visible. + +Baseline symlink: cache corrupted. + + $ rm -rf .cache _build + $ dune build dep.txt + $ dune build --sandbox=symlink result + $ check_cache_contents + poisoned + +bwrap + symlink: chmod and write resolve through the symlink to +`_build/default/dep.txt`, which is on the read-write `/` bind. The cache +RO bind doesn't cover this path. Cache corrupted. + + $ rm -rf .cache _build + $ dune build dep.txt + $ if [ "$have_bwrap" = yes ]; then + > dune build --sandbox=symlink \ + > --sandbox-actions --sandbox-actions-backend=bwrap result + > check_cache_contents + > else + > echo poisoned + > fi + poisoned + +landlock + symlink: Landlock evaluates rules on the resolved path. The +symlink resolves to `_build/default/dep.txt`, which is *not* in the +denied cache subtree. Open allowed, cache corrupted. Unlike hardlink +mode, this case is in principle fixable: extending the policy to also +deny writes to the build directory would catch the resolved-path open. +The current `--sandbox-actions-backend=landlock` does not enable that. + + $ rm -rf .cache _build + $ dune build dep.txt + $ if [ "$have_landlock" = yes ]; then + > dune build --sandbox=symlink \ + > --sandbox-actions --sandbox-actions-backend=landlock result + > check_cache_contents + > else + > echo poisoned + > fi + poisoned + +bwrap + landlock + symlink: still corrupted, for the same combined +reasons as the individual backends. + + $ rm -rf .cache _build + $ dune build dep.txt + $ if [ "$have_bwrap" = yes ] && [ "$have_landlock" = yes ]; then + > dune build --sandbox=symlink \ + > --sandbox-actions \ + > --sandbox-actions-backend=bwrap \ + > --sandbox-actions-backend=landlock result + > check_cache_contents + > else + > echo poisoned + > fi + poisoned + +The actual protection: `--sandbox=copy`. The sandbox dep is an independent +inode, so the action's chmod+write does not touch the cache entry's inode. +The action corrupts its own sandbox copy, but the cache is intact. + + $ rm -rf .cache _build + $ dune build dep.txt + $ dune build --sandbox=copy result + $ check_cache_contents + original diff --git a/test/blackbox-tests/test-cases/sandbox-actions/dune b/test/blackbox-tests/test-cases/sandbox-actions/dune index 5dd40769beb..84920a72524 100644 --- a/test/blackbox-tests/test-cases/sandbox-actions/dune +++ b/test/blackbox-tests/test-cases/sandbox-actions/dune @@ -7,6 +7,11 @@ (enabled_if %{bin-available:bwrap})) (cram - (applies_to with-landlock backends) + (applies_to + with-landlock + backends + cache-hardlink-integrity + landlock-extended-policy + source-tree-writes) (enabled_if (= %{system} linux))) diff --git a/test/blackbox-tests/test-cases/sandbox-actions/landlock-extended-policy.t b/test/blackbox-tests/test-cases/sandbox-actions/landlock-extended-policy.t new file mode 100644 index 00000000000..e444f3a0aaf --- /dev/null +++ b/test/blackbox-tests/test-cases/sandbox-actions/landlock-extended-policy.t @@ -0,0 +1,56 @@ +Verify the claim that `dune internal with-landlock` with an extended +deny-write policy (cache + build directory) blocks the cache-corruption +attack for the sandbox=symlink configuration. This is a synthetic test: +it constructs the inode chain manually rather than going through +`dune build`, so the policy can be varied without changing dune-side code. + +Probe landlock availability; skip otherwise. + + $ have_landlock=no + $ dune internal with-landlock --deny-write / -- true >/dev/null 2>&1 && have_landlock=yes + +Scenario A: deny writes to the cache only (what +`--sandbox-actions-backend=landlock` does today). The attack writes through +the sandbox symlink, which resolves to `build/dep.txt`. Landlock's check +fires on the resolved path, which is not in the denied subtree, so the +write proceeds and the cache is corrupted. + + $ rm -rf scratch-a + $ mkdir -p scratch-a/cache scratch-a/build scratch-a/sandbox + $ echo original > scratch-a/cache/dep.txt + $ chmod 444 scratch-a/cache/dep.txt + $ ln scratch-a/cache/dep.txt scratch-a/build/dep.txt + $ ln -s "$PWD/scratch-a/build/dep.txt" scratch-a/sandbox/dep.txt + $ if [ "$have_landlock" = yes ]; then + > dune internal with-landlock --deny-write "$PWD/scratch-a/cache" -- \ + > sh -c 'chmod 777 scratch-a/sandbox/dep.txt 2>/dev/null && (echo poisoned > scratch-a/sandbox/dep.txt) 2>/dev/null && echo wrote || echo blocked' + > else + > echo wrote + > fi + wrote + $ cat scratch-a/cache/dep.txt + poisoned + +Scenario B: deny writes to both the cache and the build directory. The +resolved path of the symlink now falls in the denied build subtree, so the +destructive write returns EACCES. The chmod still succeeds (landlock does +not gate chmod) but the write itself is blocked, and the cache content +survives. + + $ rm -rf scratch-b + $ mkdir -p scratch-b/cache scratch-b/build scratch-b/sandbox + $ echo original > scratch-b/cache/dep.txt + $ chmod 444 scratch-b/cache/dep.txt + $ ln scratch-b/cache/dep.txt scratch-b/build/dep.txt + $ ln -s "$PWD/scratch-b/build/dep.txt" scratch-b/sandbox/dep.txt + $ if [ "$have_landlock" = yes ]; then + > dune internal with-landlock \ + > --deny-write "$PWD/scratch-b/cache" \ + > --deny-write "$PWD/scratch-b/build" \ + > -- sh -c 'chmod 777 scratch-b/sandbox/dep.txt 2>/dev/null && (echo poisoned > scratch-b/sandbox/dep.txt) 2>/dev/null && echo wrote || echo blocked' + > else + > echo blocked + > fi + blocked + $ cat scratch-b/cache/dep.txt + original diff --git a/test/blackbox-tests/test-cases/sandbox-actions/source-tree-writes.t b/test/blackbox-tests/test-cases/sandbox-actions/source-tree-writes.t new file mode 100644 index 00000000000..0c0935dec78 --- /dev/null +++ b/test/blackbox-tests/test-cases/sandbox-actions/source-tree-writes.t @@ -0,0 +1,54 @@ +Before per-run write isolation, sandbox-actions backends protect the shared +cache path but do not stop an action from modifying an existing source file. + + $ make_dune_project 3.25 + $ workspace_root=$PWD + $ echo original > source.txt + $ cat > dune < (rule + > (target result) + > (action + > (bash + > "echo target > %{target} + > if echo poisoned > $workspace_root/source.txt 2>/dev/null; then echo source-wrote >> %{target}; else echo source-blocked >> %{target}; fi"))) + > EOF + +Probe backends. + + $ have_bwrap=no + $ command -v bwrap >/dev/null && bwrap --version >/dev/null 2>&1 && have_bwrap=yes + $ have_landlock=no + $ dune internal with-landlock --deny-write / -- true >/dev/null 2>&1 && have_landlock=yes + +With bwrap only, the source tree remains writable. + + $ rm -rf _build + $ echo original > source.txt + $ if [ "$have_bwrap" = yes ]; then + > dune build --sandbox-actions --sandbox-actions-backend=bwrap result + > cat _build/default/result + > else + > echo target + > echo source-wrote + > fi + target + source-wrote + $ cat source.txt + poisoned + +With Landlock only, the source tree still remains writable because the current +policy only denies writes to the shared cache path. + + $ rm -rf _build + $ echo original > source.txt + $ if [ "$have_landlock" = yes ]; then + > dune build --sandbox-actions --sandbox-actions-backend=landlock result + > cat _build/default/result + > else + > echo target + > echo source-wrote + > fi + target + source-wrote + $ cat source.txt + poisoned From 234039832b1ff90d8a38009a6bcd2da044ee8433 Mon Sep 17 00:00:00 2001 From: Ali Caglayan Date: Wed, 8 Jul 2026 16:26:06 +0100 Subject: [PATCH 4/4] feat(sandbox-actions): deny source reads under landlock Signed-off-by: Ali Caglayan --- bin/action_runner.ml | 33 +++-- bin/landlock.ml | 113 +++++++++++++----- bin/landlock_stubs.c | 32 ++++- .../test-cases/sandbox-actions/dune | 1 + .../sandbox-actions/source-tree-reads.t | 74 ++++++++++++ 5 files changed, 216 insertions(+), 37 deletions(-) create mode 100644 test/blackbox-tests/test-cases/sandbox-actions/source-tree-reads.t diff --git a/bin/action_runner.ml b/bin/action_runner.ml index 818aa926e0b..a325501c254 100644 --- a/bin/action_runner.ml +++ b/bin/action_runner.ml @@ -88,17 +88,25 @@ module Backends = struct ])) ;; - let landlock_wrap ~dune_prog ~cache_dir argv = + let path_flag name p = [ name; Path.to_absolute_filename p ] + + let landlock_wrap ~dune_prog ~cache_dir ~deny_read_dirs ~allow_read_dirs argv = Path.mkdir_p cache_dir; + List.iter deny_read_dirs ~f:Path.mkdir_p; + List.iter allow_read_dirs ~f:Path.mkdir_p; let dune = Path.to_string dune_prog in let argv = - dune - :: "internal" - :: "with-landlock" - :: "--deny-write" - :: Path.to_absolute_filename cache_dir - :: "--" - :: argv + List.concat + [ [ dune + ; "internal" + ; "with-landlock" + ; "--deny-write" + ; Path.to_absolute_filename cache_dir + ] + ; List.concat_map deny_read_dirs ~f:(path_flag "--deny-read") + ; List.concat_map allow_read_dirs ~f:(path_flag "--allow-read") + ; "--" :: argv + ] in dune, argv ;; @@ -112,6 +120,8 @@ module Backends = struct let compose t ~dune_prog ~worker_argv = let cache_dir = lazy (Lazy.force Dune_cache.Layout.build_cache_dir) in + let deny_read_dirs = [ Path.root ] in + let allow_read_dirs = [ Path.build Path.Build.root; dune_prog ] in (* Fold from innermost to outermost: reverse the chain, then wrap step by step so the first element of [t] ends up as the outermost. *) List.fold_right @@ -120,7 +130,12 @@ module Backends = struct ~f:(fun b (_, argv) -> match b with | Backend.Landlock -> - landlock_wrap ~dune_prog ~cache_dir:(Lazy.force cache_dir) argv + landlock_wrap + ~dune_prog + ~cache_dir:(Lazy.force cache_dir) + ~deny_read_dirs + ~allow_read_dirs + argv | Backend.Bwrap -> bwrap_wrap argv) ;; end diff --git a/bin/landlock.ml b/bin/landlock.ml index c5a11185315..c52fdece55c 100644 --- a/bin/landlock.ml +++ b/bin/landlock.ml @@ -9,6 +9,13 @@ module Raw = struct -> int64 = "dune_landlock_file_write_access_rights" + external read_access_rights : int -> int64 = "dune_landlock_read_access_rights" + + external file_read_access_rights + : int + -> int64 + = "dune_landlock_file_read_access_rights" + external create_ruleset : int64 -> int = "dune_landlock_create_ruleset" external add_rule : int -> string -> int64 -> unit = "dune_landlock_add_rule" external restrict_self : int -> unit = "dune_landlock_restrict_self" @@ -61,11 +68,11 @@ let stat_key path = | exception Unix.Unix_error _ -> None ;; -let add_write_rule ruleset_fd ~write_access ~file_write_access path = +let add_access_rule ruleset_fd ~dir_access ~file_access path = let access = match Unix.stat path with - | { Unix.st_kind = S_DIR; _ } -> write_access - | _ -> file_write_access + | { Unix.st_kind = S_DIR; _ } -> dir_access + | _ -> file_access | exception Unix.Unix_error _ -> 0L in add_rule ruleset_fd path access @@ -93,13 +100,13 @@ let path_and_ancestors path = loop Filename.dir_sep [] (split_absolute path) ;; -(* Landlock rules are allow-lists. To keep a set of subtrees write-protected - while leaving the rest of the filesystem writable, allow writes to siblings - of every ancestor of every protected path. Skip lexical descendants of - protected paths, paths whose realpath is at or above any protected path - (symlink aliases), and bind-mount aliases of protected-path ancestors. Then - apply explicit allow paths as punch-holes on top. *) -let add_rules ruleset_fd ~write_access ~file_write_access ~deny_paths ~allow_paths = +(* Landlock rules are allow-lists. To keep a set of subtrees denied for the + given access, allow the access on siblings of every ancestor of every + protected path. Skip lexical descendants of protected paths, paths whose + realpath is at or above any protected path (symlink aliases), and + bind-mount aliases of protected-path ancestors. Then apply any explicit + [allow_paths] as punch-holes on top. *) +let add_rules ruleset_fd ~dir_access ~file_access ~deny_paths ~allow_paths = (match deny_paths with | [] -> () | _ -> @@ -126,8 +133,8 @@ let add_rules ruleset_fd ~write_access ~file_write_access ~deny_paths ~allow_pat not (in_or_above_protected realpath || is_protected_alias path ~protected_ancestors) - then add_write_rule ruleset_fd ~write_access ~file_write_access path)))); - List.iter allow_paths ~f:(add_write_rule ruleset_fd ~write_access ~file_write_access) + then add_access_rule ruleset_fd ~dir_access ~file_access path)))); + List.iter allow_paths ~f:(add_access_rule ruleset_fd ~dir_access ~file_access) ;; let with_ruleset ~handled_access ~f = @@ -135,28 +142,68 @@ let with_ruleset ~handled_access ~f = Exn.protect ~f:(fun () -> f fd) ~finally:(fun () -> Fd.close (Fd.unsafe_of_int fd)) ;; -let restrict_write_policy ~deny_write ~allow_write = - match deny_write with - | [] -> () - | _ -> +type policy = + { deny_write : string list + ; allow_write : string list + ; deny_read : string list + ; allow_read : string list + } + +let policy_is_empty { deny_write; allow_write; deny_read; allow_read } = + List.is_empty deny_write + && List.is_empty allow_write + && List.is_empty deny_read + && List.is_empty allow_read +;; + +let restrict_to_policy policy = + if policy_is_empty policy + then () + else ( let abi = abi_version () in if abi < minimum_abi then User_error.raise [ Pp.text "Landlock is not available on this system" ]; let write_access = Raw.write_access_rights abi in + let read_access = Raw.read_access_rights abi in if Int64.equal write_access 0L then User_error.raise [ Pp.text "Landlock write restrictions are not available" ]; let file_write_access = Raw.file_write_access_rights abi in + let file_read_access = Raw.file_read_access_rights abi in let normalize = List.map ~f:normalize_absolute_filename in - let deny_write = normalize deny_write in - let allow_write = normalize allow_write in - with_ruleset ~handled_access:write_access ~f:(fun ruleset_fd -> - add_rules - ruleset_fd - ~write_access - ~file_write_access - ~deny_paths:deny_write - ~allow_paths:allow_write; - Raw.restrict_self ruleset_fd) + let policy = + { deny_write = normalize policy.deny_write + ; allow_write = normalize policy.allow_write + ; deny_read = normalize policy.deny_read + ; allow_read = normalize policy.allow_read + } + in + let has_write = not (List.is_empty policy.deny_write) in + let has_read = not (List.is_empty policy.deny_read) in + if has_write || has_read + then ( + let handled_access = + Int64.logor + (if has_write then write_access else 0L) + (if has_read then read_access else 0L) + in + with_ruleset ~handled_access ~f:(fun ruleset_fd -> + if has_write + then + add_rules + ruleset_fd + ~dir_access:write_access + ~file_access:file_write_access + ~deny_paths:policy.deny_write + ~allow_paths:policy.allow_write; + if has_read + then + add_rules + ruleset_fd + ~dir_access:read_access + ~file_access:file_read_access + ~deny_paths:policy.deny_read + ~allow_paths:policy.allow_read; + Raw.restrict_self ruleset_fd))) ;; module With_landlock = struct @@ -199,11 +246,23 @@ module With_landlock = struct ~doc: "Explicitly allow writes to this subtree, overriding any covering \ --deny-write. Repeatable." + and+ deny_read = + path_list_flag + ~name:"deny-read" + ~doc: + "Forbid reads within this subtree (may be punched by --allow-read). \ + Repeatable. May be passed an absolute path." + and+ allow_read = + path_list_flag + ~name:"allow-read" + ~doc: + "Explicitly allow reads to this subtree, overriding any covering \ + --deny-read. Repeatable." and+ argv = Arg.(value & pos_all string [] (info [] ~docv:"COMMAND" ~doc:None)) in match argv with | [] -> User_error.raise [ Pp.text "missing command after --" ] | prog :: args -> - restrict_write_policy ~deny_write ~allow_write; + restrict_to_policy { deny_write; allow_write; deny_read; allow_read }; execve (Util.resolve_prog prog) args ~env:Env.initial in Cmd.v info term diff --git a/bin/landlock_stubs.c b/bin/landlock_stubs.c index 291e590f37a..61371af1bd7 100644 --- a/bin/landlock_stubs.c +++ b/bin/landlock_stubs.c @@ -40,7 +40,9 @@ defined(LANDLOCK_ACCESS_FS_MAKE_BLOCK) && \ defined(LANDLOCK_ACCESS_FS_MAKE_SYM) && \ defined(LANDLOCK_ACCESS_FS_REFER) && \ - defined(LANDLOCK_ACCESS_FS_TRUNCATE) + defined(LANDLOCK_ACCESS_FS_TRUNCATE) && \ + defined(LANDLOCK_ACCESS_FS_READ_FILE) && \ + defined(LANDLOCK_ACCESS_FS_READ_DIR) #define DUNE_HAS_LANDLOCK 1 #else #define DUNE_HAS_LANDLOCK 0 @@ -71,6 +73,15 @@ static uint64_t landlock_write_access_rights(int abi) { } return access; } + +static uint64_t landlock_file_read_access_rights(int abi) { + (void)abi; + return LANDLOCK_ACCESS_FS_READ_FILE; +} + +static uint64_t landlock_read_access_rights(int abi) { + return landlock_file_read_access_rights(abi) | LANDLOCK_ACCESS_FS_READ_DIR; +} #endif CAMLprim value dune_landlock_abi_version(value unit) { @@ -113,6 +124,25 @@ CAMLprim value dune_landlock_file_write_access_rights(value v_abi) { #endif } +CAMLprim value dune_landlock_read_access_rights(value v_abi) { + CAMLparam1(v_abi); +#if DUNE_HAS_LANDLOCK + CAMLreturn(caml_copy_int64(landlock_read_access_rights(Int_val(v_abi)))); +#else + CAMLreturn(caml_copy_int64(0)); +#endif +} + +CAMLprim value dune_landlock_file_read_access_rights(value v_abi) { + CAMLparam1(v_abi); +#if DUNE_HAS_LANDLOCK + CAMLreturn( + caml_copy_int64(landlock_file_read_access_rights(Int_val(v_abi)))); +#else + CAMLreturn(caml_copy_int64(0)); +#endif +} + CAMLprim value dune_landlock_create_ruleset(value v_handled_access) { CAMLparam1(v_handled_access); #if DUNE_HAS_LANDLOCK diff --git a/test/blackbox-tests/test-cases/sandbox-actions/dune b/test/blackbox-tests/test-cases/sandbox-actions/dune index 84920a72524..29f76cb2aaf 100644 --- a/test/blackbox-tests/test-cases/sandbox-actions/dune +++ b/test/blackbox-tests/test-cases/sandbox-actions/dune @@ -12,6 +12,7 @@ backends cache-hardlink-integrity landlock-extended-policy + source-tree-reads source-tree-writes) (enabled_if (= %{system} linux))) diff --git a/test/blackbox-tests/test-cases/sandbox-actions/source-tree-reads.t b/test/blackbox-tests/test-cases/sandbox-actions/source-tree-reads.t new file mode 100644 index 00000000000..8a4620f1572 --- /dev/null +++ b/test/blackbox-tests/test-cases/sandbox-actions/source-tree-reads.t @@ -0,0 +1,74 @@ +Landlock-backed sandbox-actions denies reads under the workspace source +root, catching sandboxed rules that reach for source files via absolute +paths (an undeclared-dependency correctness bug that would otherwise be +invisible). Reads inside the build directory remain allowed, so +legitimate deps materialised through `_build//` or the per-rule +sandbox tree keep working. + + $ make_dune_project 3.23 + $ workspace_root=$PWD + $ echo "sensitive" > secret.txt + $ cat > dune < (rule + > (target result) + > (deps (sandbox always)) + > (action + > (bash "if cat $workspace_root/secret.txt >/dev/null 2>&1; then echo read > %{target}; else echo denied > %{target}; fi"))) + > EOF + +Probe backends. + + $ have_bwrap=no + $ command -v bwrap >/dev/null && bwrap --version >/dev/null 2>&1 && have_bwrap=yes + $ have_landlock=no + $ dune internal with-landlock --deny-write / -- true >/dev/null 2>&1 && have_landlock=yes + +Without --sandbox-actions the action can read the source file even though +it wasn't declared as a dep. + + $ rm -rf _build + $ dune build --sandbox=symlink result 2>/dev/null + $ cat _build/default/result + read + +`--sandbox-actions-backend=bwrap`: bwrap has no way to selectively deny +reads, so the source file remains readable through the RW `--bind / /`. + + $ rm -rf _build + $ if [ "$have_bwrap" = yes ]; then + > dune build --sandbox=symlink \ + > --sandbox-actions --sandbox-actions-backend=bwrap result 2>/dev/null + > cat _build/default/result + > else + > echo read + > fi + read + +`--sandbox-actions-backend=landlock`: the workspace source root is +denied for reads (with the build directory punch-hole allowed), so the +absolute-path cat fails with EACCES and the rule's fallback branch fires. + + $ rm -rf _build + $ if [ "$have_landlock" = yes ]; then + > dune build --sandbox=symlink \ + > --sandbox-actions --sandbox-actions-backend=landlock result 2>/dev/null + > cat _build/default/result + > else + > echo denied + > fi + denied + +Both backends stacked: landlock still catches the read; bwrap adds no +further read protection but doesn't interfere. + + $ rm -rf _build + $ if [ "$have_bwrap" = yes ] && [ "$have_landlock" = yes ]; then + > dune build --sandbox=symlink \ + > --sandbox-actions \ + > --sandbox-actions-backend=bwrap \ + > --sandbox-actions-backend=landlock result 2>/dev/null + > cat _build/default/result + > else + > echo denied + > fi + denied