Open nfeske opened 3 months ago
As observed by @mewmew, right now, Goa is used by a small circle of mutually trusting people. But even today, I already ran into the situation where the goarc file that comes with @jschlatow's https://github.com/jschlatow/goa-projects contains settings (like depot_dir
) that assume a certain directory structure outside the scope of the Goa project. This case was merely an inconvenience for me but it brings @mewmew's point home. As goarc files can contain arbitrary Tcl code that is executed by Goa, anything bad could happen when using a Goa project authored by an untrusted party.
This is of course not a Goa-specific issue. The same attack vector exists for any Makefile originating from a potentially malicious 3rd part. But Goa is in a good position to take measures against such risks.
We could consider limiting the Tcl commands allowed in goarc files. E.g., disallowing any form of I/O or exec calls. It may be worth investigating if Tcl provides some sort of sandboxing mechanism for Tcl sub programs used as DSL (guessing that others had such needs before). Or alternatively, if the Tcl subset supported by goarc is small enough (e.g., only using set
), we could parse goarc files manually.
When spawning 3rd-party tools during the import step and - most importantly - when invoking 3rd-party build systems, Goa could implicitly create a (chroot-like) environment using appropriate Linux kernel mechanisms. So the reach of the (generally overly complex and hence not completely trustworthy) 3rd-party tooling would be limited to the Goa project's var
subdirectories.
Until these measures are in place, it is probably best to use a "disposable" development VM when dealing Goa projects of untrusted origin.
Thanks @mewmew and @nfeske for raising the discussion.
We could consider limiting the Tcl commands allowed in goarc files.
Tcl is actually able to create safe child interpreters. There is also some example code that we could use for reference.
Using a safe interpreter would prevent any exec and file operations in goarc files. However, it might still be possible to trick Goa into performing harmful file system operations. For instance, any goarc file may set bin_dir
to an arbitrary directory, which gets force-deleted upon goa build
. I therefore think it's best to restrict Goa's file operations to a user-defined list of allowed paths. It might also be useful to make Goa's path variables immutable, such that once defined by the user (e.g. in ~/goarc) they won't be changed by any goarc file lower in the file-system hierarchy.
In preparation of using a child interpreter to load goarc files, I had a longer cleanup session. Commit bdee6c6 moves the main program logic into a goa namespace and separate procedures, and paves the way for moving config variables into a config namespace.
Commit c271490 implements the idea of using a safe TCL interpreter for goarc loading. Moreover, commit 00a2559 adds the policy that path variables must refer to paths inside the working directory or project directory. The policy can be extended by appending to the list allowed_paths
. The latter is only allowed in ~/goarc and /goarc.
That's amazing!
I further reviewed Goa w.r.t. sensitive commands (exec, spawn, file, glob). There are a few obvious and less obvious issues that we need to address:
target_opt(sculpt-cmd)
variable allows arbitrary command execution. I added this for starting a VNC client when using Sculpt as a remote test target. Mitigation: When set in a goarc file, Goa should ask the user for confirmation before executing the command.version
array as well as the content from _usedapis and archives files are used for path construction, special care must be taken that they do not refer to paths outside the allowed_paths
.allowed_paths
or var_dir
. For tar --dereference
, it's probably best to use bubblewrap.
- The
target_opt(sculpt-cmd)
variable allows arbitrary command execution. I added this for starting a VNC client when using Sculpt as a remote test target. Mitigation: When set in a goarc file, Goa should ask the user for confirmation before executing the command.
This is addressed by 838c084
- Since items in the
version
array as well as the content from _usedapis and archives files are used for path construction, special care must be taken that they do not refer to paths outside theallowed_paths
.
Fixed by 94a73be
Commit b6f2b11 further adds an allowed_tools
variable similar to the allowed_paths
variable. This variable is used for validating that cross_dev_prefix
does not point to an arbitrary executable.
With commit 9675bdc, Goa now intercepts Tcl's file operations in order to check arguments against the allowed_paths
and allowed_tools
whenever a file normalize
and file link
operations is executed. The rationale is that both operations resolve symlinks, hence even if the path(s) passed as arguments appear valid (i.e. inside the scope of allowed_paths
), the path after resolving symlinks might not.
I pushed another commit series to my issue099_security branch. The series implement the sandboxing of build-tool executions using bubblewrap. For better re-usability, I added a wrapper script called gaol (pronounced "jail") around bwrap. Moreover, I added a download mechanism for the Genode toolchain in case a system-wide installation was not found. In this case, the toolchain archive is downloaded and converted into a squashfs archive. Goa then mounts the archive using squashfuse and bind mounts it into the sandbox environment. There is a new "install-toolchain" command that takes care of this.
I would have liked to implement the mounting mechanism in the gaol tool as well. However, since I want to preserve stderr output of the bwrap command (and its children), I cannot use expect. tclsh, on the other hand, has no awareness of signals so that I'm unable to perform cleanup tasks such as unmounting the squashfs when the user hits ctrl-c during the build procedure.
@atopia I'd be interested in your thoughts on providing the rust toolchain as squashfs as well. This would relieve the user from any manual installation tasks when building rust components.
I force pushed my issue branch with a few fixups for cmake and qmake. I also changed the gaol tool to execute the bwrap command within bash, which enables the use of arbitrary file descriptors (e.g. required for the --ro-bind-data
argument).
The latter turned out to be useful for adding support for signing archives via sq, which does not cache passwords of secret keys. The --ro-bind-data
allows piping multiple unencrypted keys to the bwrap command. Experimental support for sq can be found on my sequoia branch.
@atopia I'd be interested in your thoughts on providing the rust toolchain as squashfs as well. This would relieve the user from any manual installation tasks when building rust components.
My understanding is that we provide a custom Genode C/C++ toolchain because we need to control code generation w.r.t. features like Thread Local Storage and because we want to control the update cycle of the gcc suite for -Werror
and the like. As we saw with our recent struggle with TLS in the Rust standard library, in principle Rust support could benefit from a similar control of the toolchain. At a quick glance this can be done reasonably well as long as we still require rustup
to be installed on the system. In this case we could check if we have a nightly toolchain with the rust-src
component installed or otherwise install the toolchain to a local directory, and if needed pin the toolchain to a specific version that can be checked against in the same way.
The alternatives would be to a) actually distribute a version of the toolchain ourselves (as we have done in the past, but also by utilizing rustup) and/or b) to implement rustup
's functionality of downloading and unpacking a toolchain within Goa. Seeing that the other build methods rely on system-wide installations of make, cmake, meson and the like, I don't think there is a good reason not to rely on rustup
for Rust toolchain management.
I've written down the result of my quick exploration of the topic in #107. Seeing that we currently don't have a pressing need to require a specific Rust version on the system, I would put the feature on the back burner, unless people feel there is an urgent need to automate toolchain installation?
In issue #98, @mewmew raised a fairly important topic that deserves a dedicated issue. It is not urgent but we should keep the topic in eyesight and eventually address it. Here is @mewmew's original text: