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

Security considerations #99

Open
nfeske opened this issue Jul 29, 2024 · 14 comments
Open

Security considerations #99

nfeske opened this issue Jul 29, 2024 · 14 comments

Comments

@nfeske
Copy link
Member

nfeske commented Jul 29, 2024

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:

On a related note, have there been consideration into how to limit the damage of potential threat actors as the Genode ecosystem grows? For instance, having the Goa tool use a chroot on Linux? And, how to limit the damage of a potential command injection vulnerability in the Goa script?

I know a lot of the security may be based on trust. As in, I trust this person that I add a depot download link to, thus their code will not perform malicious actions. And, to begin with, such a trust model works well, but with larger and deeper dependency chains, becomes more laborous to review.

I know that running the compiled artifact in a Genode system would limit what the component is given capabilities to perform. So, the scenario I'm considering is that of trying to take control of a developer machine, running Linux, by exploiting (a potential) vulnerability in Goa.

Just to give an example of the kind of exploit that would be used to target developers, consider the CVE-2018-6574 vulnerability in the go get command of the toolchain for the Go programming language. (A proof of concept exploit for the aforementioned vulnerability: https://github.com/j4k0m/CVE-2018-6574)

@nfeske
Copy link
Member Author

nfeske commented Jul 29, 2024

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.

@jschlatow
Copy link
Member

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.

jschlatow added a commit that referenced this issue Sep 6, 2024
This reduces the use of global variables and renders bin/goa easier to
grasp. Moreover, this change paves the way for eliminating config variables
from the global namespace without the need to adapt every line that uses
such a variable. Within procedures, we can easily import namespace
variables.

#99
@jschlatow
Copy link
Member

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.

jschlatow added a commit to jschlatow/goa that referenced this issue Sep 6, 2024
TCL's safe interpreter hides file operations, source, and exec commands.
Loading goarc files with a safe slave interpreter is a basic security
measure. This change also moves all config variables into a separate
namespace. Our interpreter additionally hides the set command in order to
make sure that goarc files are only able to set predefined variables in
the config namespace.

genodelabs#99
jschlatow added a commit to jschlatow/goa that referenced this issue Sep 9, 2024
TCL's safe interpreter hides file operations, source, and exec commands.
Loading goarc files with a safe slave interpreter is a basic security
measure. This change also moves all config variables into a separate
namespace. Our interpreter additionally hides the set command in order to
make sure that goarc files are only able to set predefined variables in
the config namespace.

genodelabs#99
jschlatow added a commit to jschlatow/goa that referenced this issue Sep 9, 2024
Restrict path variables to working directory and project directory. The
set of allowed paths can be extended by adding `lappend allowed_paths ...`
commands to _~/goarc_ or _/goarc_.

genodelabs#99
@jschlatow
Copy link
Member

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.

@nfeske
Copy link
Member Author

nfeske commented Sep 9, 2024

That's amazing!

@jschlatow
Copy link
Member

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:

  • 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.
  • Since items in the version array as well as the content from used_apis and archives files are used for path construction, special care must be taken that they do not refer to paths outside the allowed_paths.
  • The depot tool could also be affected by malformed entries in archives files. I think it'd be best to bubblewrap the depot and import tool execution in order to restrict their file-system accesses.
  • There are several places where symlinks could be used to trick Goa into malicious operations. For tcl-native file operations, we can validate that symlinks do not point to any location outside the allowed_paths or var_dir. For tar --dereference, it's probably best to use bubblewrap.

jschlatow added a commit to jschlatow/goa that referenced this issue Sep 10, 2024
jschlatow added a commit to jschlatow/goa that referenced this issue Sep 10, 2024
jschlatow added a commit to jschlatow/goa that referenced this issue Sep 10, 2024
jschlatow added a commit to jschlatow/goa that referenced this issue Sep 11, 2024
jschlatow added a commit to jschlatow/goa that referenced this issue Sep 11, 2024
Restrict tool paths to PATH, Goa-internal tools and the default Genode
tool-chain path. The set of allowed tool paths can be extended by adding
`lappend allowed_tools ...` commands to _~/goarc_ or _/goarc_.

This commit prevents malicious configuration of cross_dev_prefix.

genodelabs#99
jschlatow added a commit to jschlatow/goa that referenced this issue Sep 11, 2024
In order to prevent that the presence of malicious symlinks can be used
to escape the allowed_paths, we replace the file command with an alias.
This allows validation of paths during `file normalize` and `file link`
operations, which both resolve symlinks.

genodelabs#99
jschlatow added a commit to jschlatow/goa that referenced this issue Sep 12, 2024
TCL's safe interpreter hides file operations, source, and exec commands.
Loading goarc files with a safe slave interpreter is a basic security
measure. This change also moves all config variables into a separate
namespace. Our interpreter additionally hides the set command in order to
make sure that goarc files are only able to set predefined variables in
the config namespace.

genodelabs#99
jschlatow added a commit to jschlatow/goa that referenced this issue Sep 12, 2024
Restrict path variables to working directory and project directory. The
set of allowed paths can be extended by adding `lappend allowed_paths ...`
commands to _~/goarc_ or _/goarc_.

genodelabs#99
jschlatow added a commit to jschlatow/goa that referenced this issue Sep 12, 2024
jschlatow added a commit to jschlatow/goa that referenced this issue Sep 12, 2024
jschlatow added a commit to jschlatow/goa that referenced this issue Sep 12, 2024
Restrict tool paths to PATH, Goa-internal tools and the default Genode
tool-chain path. The set of allowed tool paths can be extended by adding
`lappend allowed_tools ...` commands to _~/goarc_ or _/goarc_.

This commit prevents malicious configuration of cross_dev_prefix.

genodelabs#99
jschlatow added a commit to jschlatow/goa that referenced this issue Sep 12, 2024
In order to prevent that the presence of malicious symlinks can be used
to escape the allowed_paths, we replace the file command with an alias.
This allows validation of paths during `file normalize` and `file link`
operations, which both resolve symlinks.

genodelabs#99
jschlatow added a commit to jschlatow/goa that referenced this issue Sep 12, 2024
In order to prevent untrusted goarc files to execute malicious commands
via target_opt(sculpt-cmd), users must confirm the command execution
interactively.

genodelabs#99
@jschlatow
Copy link
Member

  • 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

@jschlatow
Copy link
Member

  • Since items in the version array as well as the content from used_apis and archives files are used for path construction, special care must be taken that they do not refer to paths outside the allowed_paths.

Fixed by 94a73be

@jschlatow
Copy link
Member

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.

@jschlatow
Copy link
Member

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.

jschlatow added a commit to jschlatow/goa that referenced this issue Sep 26, 2024
jschlatow added a commit to jschlatow/goa that referenced this issue Sep 26, 2024
The command downloads and installs the current Genode toolchain (if unavailable)
and converts it into a squashfs. The squashfs is mounted via squashfuse
and bind mounted into the bubblewrapped build environment.

genodelabs#99
@jschlatow
Copy link
Member

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.

@jschlatow
Copy link
Member

@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.

jschlatow added a commit to jschlatow/goa that referenced this issue Sep 26, 2024
jschlatow added a commit to jschlatow/goa that referenced this issue Oct 1, 2024
jschlatow added a commit to jschlatow/goa that referenced this issue Oct 1, 2024
jschlatow added a commit to jschlatow/goa that referenced this issue Oct 1, 2024
jschlatow added a commit to jschlatow/goa that referenced this issue Oct 1, 2024
jschlatow added a commit to jschlatow/goa that referenced this issue Oct 1, 2024
jschlatow added a commit to jschlatow/goa that referenced this issue Oct 1, 2024
jschlatow added a commit to jschlatow/goa that referenced this issue Oct 1, 2024
This is in preparation of moving the tool chain into a squashfs image.

genodelabs#99
jschlatow added a commit to jschlatow/goa that referenced this issue Oct 1, 2024
jschlatow added a commit to jschlatow/goa that referenced this issue Oct 1, 2024
jschlatow added a commit to jschlatow/goa that referenced this issue Oct 1, 2024
jschlatow added a commit to jschlatow/goa that referenced this issue Oct 1, 2024
jschlatow added a commit to jschlatow/goa that referenced this issue Oct 1, 2024
jschlatow added a commit to jschlatow/goa that referenced this issue Oct 1, 2024
jschlatow added a commit to jschlatow/goa that referenced this issue Oct 1, 2024
jschlatow added a commit to jschlatow/goa that referenced this issue Oct 1, 2024
jschlatow added a commit to jschlatow/goa that referenced this issue Oct 1, 2024
jschlatow added a commit to jschlatow/goa that referenced this issue Oct 1, 2024
jschlatow added a commit to jschlatow/goa that referenced this issue Oct 1, 2024
The command downloads and installs the current Genode toolchain (if unavailable)
and converts it into a squashfs. The squashfs is mounted via squashfuse
and bind mounted into the bubblewrapped build environment.

genodelabs#99
jschlatow added a commit to jschlatow/goa that referenced this issue Oct 1, 2024
@jschlatow
Copy link
Member

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.

jschlatow added a commit to jschlatow/goa that referenced this issue Oct 2, 2024
Instead of trying to export all secret keys referenced by the depot
directory, the to-be-exported secret keys must be mentioned explicitly.

genodelabs#99
genodelabs#104
jschlatow added a commit that referenced this issue Oct 8, 2024
This reduces the use of global variables and renders bin/goa easier to
grasp. Moreover, this change paves the way for eliminating config variables
from the global namespace without the need to adapt every line that uses
such a variable. Within procedures, we can easily import namespace
variables.

#99
@atopia
Copy link
Contributor

atopia commented Oct 15, 2024

@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?

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

3 participants