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.

Sign up for GitHub

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

darwin sandbox #3429

Merged
merged 4 commits into from
Mar 23, 2020
Merged

darwin sandbox #3429

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
afb78eb
libstore: disable resolve-system-dependencies hook
LnL7 Mar 20, 2020
7f2df90
libstore: relax default sandbox-paths on darwin
LnL7 Mar 20, 2020
f6c122a
sandbox: allow pty devices
LnL7 Mar 20, 2020
2e9bc12
sandbox: fix /bin/sh on catalina
LnL7 Mar 20, 2020
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
14 changes: 6 additions & 8 deletions src/libstore/globals.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,6 @@ namespace nix {
must be deleted and recreated on startup.) */
#define DEFAULT_SOCKET_PATH "/daemon-socket/socket"

/* chroot-like behavior from Apple's sandbox */
#if __APPLE__
#define DEFAULT_ALLOWED_IMPURE_PREFIXES "/System/Library /usr/lib /dev /bin/sh"
#else
#define DEFAULT_ALLOWED_IMPURE_PREFIXES ""
#endif

Settings settings;

static GlobalConfig::Register r1(&settings);
Expand Down Expand Up @@ -68,7 +61,12 @@ Settings::Settings()
sandboxPaths = tokenizeString<StringSet>("/bin/sh=" SANDBOX_SHELL);
#endif

allowedImpureHostPrefixes = tokenizeString<StringSet>(DEFAULT_ALLOWED_IMPURE_PREFIXES);

/* chroot-like behavior from Apple's sandbox */
#if __APPLE__
sandboxPaths = tokenizeString<StringSet>("/System/Library/Frameworks /System/Library/PrivateFrameworks /bin/sh /bin/bash /private/tmp /private/var/tmp /usr/lib");
Copy link
Member Author

Choose a reason for hiding this comment

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

I would really prefer not to need bash here (#3223) but other than either disallowing /bin/sh completely or having nix depend on something like fuse bindfs I don't think we can work around it. 😕

Copy link
Member

Choose a reason for hiding this comment

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

I really wish there was some way to say "allow /bin/bash but only when executed by /bin/sh" 🤷‍♀️

allowedImpureHostPrefixes = tokenizeString<StringSet>("/System/Library /usr/lib /dev /bin/sh");
#endif
}

void loadConfFile()
Expand Down
7 changes: 1 addition & 6 deletions src/libstore/globals.hh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -311,12 +311,7 @@ public:
Setting<bool> printMissing{this, true, "print-missing",
"Whether to print what paths need to be built or downloaded."};

Setting<std::string> preBuildHook{this,
#if __APPLE__
nixLibexecDir + "/nix/resolve-system-dependencies",
#else
"",
#endif
Setting<std::string> preBuildHook{this, "",
"pre-build-hook",
"A program to run just before a build to set derivation-specific build settings."};

Expand Down
10 changes: 10 additions & 0 deletions src/libstore/sandbox-defaults.sb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,12 @@
(literal "/dev/zero")
(subpath "/dev/fd"))

; Allow pseudo-terminals.
(allow file*
(literal "/dev/ptmx")
(regex #"^/dev/pty[a-z]+")
(regex #"^/dev/ttys[0-9]+"))

; Does nothing, but reduces build noise.
(allow file* (literal "/dev/dtracehelper"))

Expand All @@ -85,3 +91,7 @@
(literal "/etc")
(literal "/var")
(literal "/private/var/tmp"))

; This is used by /bin/sh on macOS 10.15 and later.
(allow file*
(literal "/private/var/select/sh"))
Copy link
Member

Choose a reason for hiding this comment

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

We actually want to explicitly disallow access to this, because this file is what’s used to control whether /bin/sh is bash or zsh. If we disallow access it will always resolve to bash. If we allow access then the user can configure it to be zsh instead.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thought I tested that and it failed, but looks like it's indeed only an error message.

$ sandbox-exec -f sandbox.sb /bin/sh -c "echo ok"
Error opening /private/var/select/sh: Operation not permitted
ok

Luckily it doesn't really matter and not having to spew out errors out by default is nice.

Failed to exec /bin/zsh as variant for /bin/sh (1: Operation not permitted). Falling back to /bin/bash.
ok