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

Sandboxing on MacOS is too strict #3659

Closed
silene opened this issue Nov 6, 2018 · 7 comments
Closed

Sandboxing on MacOS is too strict #3659

silene opened this issue Nov 6, 2018 · 7 comments
Milestone

Comments

@silene
Copy link
Contributor

silene commented Nov 6, 2018

On MacOS, sandbox-exec is called with the policy (deny network*), which breaks programs that rely on unix sockets for inter-process communications. Here is an example of the needed permissions:

(allow network-bind (path "/private/var/tmp/rmk-GjXRh"))
(allow network-outbound (path "/private/var/tmp/rmk-GjXRh"))

Adding (allow network* (remote unix)) after (deny network*) in src/state/shellscripts/sandbox_exec.sh seems to be sufficient and it should still forbid applications from actually accessing the network.

# opam config report
# opam-version      2.0.1 
# self-upgrade      no
# system            arch=x86_64 os=macos os-distribution=homebrew os-version=10.12.6
# solver            builtin-mccs+glpk
# install-criteria  -removed,-count[version-lag,request],-count[version-lag,changed],-changed
# upgrade-criteria  -removed,-count[version-lag,solution],-new
# jobs              1
# repositories      2 (http) (default repo at e5263e9b)
# pinned            0
# current-switch    default
@avsm
Copy link
Member

avsm commented Nov 6, 2018

Do you need domain sockets for builds, or just for running tests?

@silene
Copy link
Contributor Author

silene commented Nov 7, 2018

It is the build system itself that uses unix sockets, so I need it for potentially everything.

@gasche
Copy link
Member

gasche commented Nov 7, 2018

For context: the build system is remake, created by @silene, as a mix of make and (dbj's/apenwarr's) redo, and it uses unix sockets for local IPC (on top of multi-process parallelism, I suppose).

@silene: my current understanding is that your proposal is fairly reasonable: I see no strong security need to prevent local sockets during build. Would you send an actual Pull Requet with your proposed sandboxing-script changes?

silene added a commit to silene/opam that referenced this issue Nov 7, 2018
This commit allows applications to use unix socket.

It also allows them to write to /dev/dtracehelper. This permission is
not strictly needed, as no program is broken by a denied access. However,
MacOS' dynamic loader (hence almost all the applications) wants to access
it, so the system logs are flooded by messages such as "SandboxViolation:
sh(49331) deny(1) file-write-data /dev/dtracehelper" when using opam.
@rjbou rjbou added this to the 2.0.2 milestone Nov 8, 2018
@rjbou rjbou closed this as completed in 8b31550 Nov 8, 2018
@rjbou
Copy link
Collaborator

rjbou commented Nov 8, 2018

Thanks for the report and contribution!

@avsm
Copy link
Member

avsm commented Nov 8, 2018

This does look like it'll allow access to any domain sockets even outside of the build root. From a security perspective, it might be nice to refine this to only allow the sockets to be dropped in the opamroot. remake appears to support setting REMAKE_SOCKET to a suitable value.

My general worry is that we're widening the attack surface on osx a lot with this patch. There are an awful lot of services listening on domain sockets throughout the system...

@rjbou rjbou reopened this Nov 8, 2018
@silene
Copy link
Contributor Author

silene commented Nov 8, 2018

This does look like it'll allow access to any domain sockets even outside of the build root.

How so? The file corresponding to the socket still needs to be accessible.

remake appears to support setting REMAKE_SOCKET to a suitable value.

No, the environment variable REMAKE_SOCKET is set by the socket creator to indicate to other programs what the name of the socket is. What controls the location of the socket is TMPDIR. I suppose that opam sets up a private temporary directory rather than the global one. (Or at least it should.)

There are an awful lot of services listening on domain sockets throughout the system...

Sure, but how many of them are in directories accessible from the sandbox? The issue is not that an application inside the sandbox can use unix sockets. The issue is that an application inside the sandbox can use unix sockets that should have been outside the sandbox.

@avsm
Copy link
Member

avsm commented Nov 14, 2018

Sure, but how many of them are in directories accessible from the sandbox? The issue is not that an application inside the sandbox can use unix sockets. The issue is that an application inside the sandbox can use unix sockets that should have been outside the sandbox.

I believe the current sandbox offers read-only access to the whole filesystem; it's only file writes that are restricted. So with this change, socket connects anywhere in the filesystem will probably just work. Previously this wasn't the case since the whole class of connectivity was denied.

No, the environment variable REMAKE_SOCKET is set by the socket creator to indicate to other programs what the name of the socket is. What controls the location of the socket is TMPDIR. I suppose that opam sets up a private temporary directory rather than the global one. (Or at least it should.)

Indeed, that's even better -- opam could just let sockets be connected to from the opam sandbox and tmpdir to let remake work without modifications.

rjbou pushed a commit to rjbou/opam that referenced this issue Dec 6, 2018
This commit allows applications to use unix socket.

It also allows them to write to /dev/dtracehelper. This permission is
not strictly needed, as no program is broken by a denied access. However,
MacOS' dynamic loader (hence almost all the applications) wants to access
it, so the system logs are flooded by messages such as "SandboxViolation:
sh(49331) deny(1) file-write-data /dev/dtracehelper" when using opam.
@rjbou rjbou closed this as completed Jun 27, 2019
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

No branches or pull requests

4 participants