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

Implement a sandbox for environment variables and files #49387

Closed
wants to merge 6 commits into from

Conversation

jsgf
Copy link
Contributor

@jsgf jsgf commented Mar 26, 2018

This PR introduces some simple sandboxing for process environment variables and for include files (collectively "system environment").

This is primarily to allow a build system to more precisely control the inputs to rustc which may affect the generated output. Rust has two mechanisms by which an input source can access ambient properties of rustc's system environment: env!() and option_env!() for reading environment variables, and include!()/include_str!()/include_bytes!() for reading arbitrary files.

(This PR specifically does not intend to address any actual security concerns, since there are many other avenues that it does not attempt to control, such as compiler plugins/proc macros. However, it does help with unintentional problems which could result in later security problems if unaddressed.)

Environment Variables

rustc allows source code to directly access its process environment variables via the env!() and optional_env!() (pseudo-)macros. This poses a few of problems:

  • the build system has no idea what variables the code is using for the purposes of tracking its inputs
  • code can read arbitrary contents from arbitrary variables, which may pose unwanted information leakage (from build environment to final deployment environment, for example)
  • it combines the environment rustc needs for its own operation (PATH and LD_LIBRARY_PATH, for example) with environment the compiled code might want, and doesn't allow them to be set independently

We introduce the following new command-line options to allow the apparent environment to be controlled at a fine-grain level:

  • --env-clear - completely empty the logical environment visible to env!()/optional_env!(), causing them to all fail/return None. Without any other options, this will completely disable environment variable access.
  • --env-allow NAME - allow a specific environment variable to be read from the process environment
  • --env-define NAME=VAL - define a logical environment variable. This does not need to be present in the actual process environment, or if it is, its value is overridden

By default, the environment is completely open, leaving the existing behaviour unchanged. Once one of the options above is specified, accesses to environment variable become controlled accordingly.

Paths

Rust allows arbitrary other files to be directly included, either as more Rust source code (include!()), a text string (include_str!()) or arbitrary binary data (include_bytes!()). These macros take a raw string which is used as a path which may be absolute - they therefore allow any file that rustc has permission to access to be used in the compiled output.

(This differs from separating a crate into multiple source files, as those files are always relative to the top-level lib.rs/main.rs source.)

This causes a couple of problems:

  • the build system can't know or constrain what files are actually inputs to the compiler
  • the code can't unintentionally leak state from the build environment to the deployment environment

To implement this, we introduce a couple of command-line options:

  • --clear-include-prefixes - clear all allowable prefixes, effectively disabling all include*!() macros
  • --include-prefix PATH - add PATH to the set of valid prefixes. All included paths must match one of the valid path prefixes before it can be opened.

All paths are canonicalized before matching, so they must exist at the time they're specified.

By default, all path prefixes are valid, leaving the current behaviour unchanged. They are only constrained once one of the options above are specified.

Note that a "path prefix" can be an entire pathname, allowing these options to explicitly specify which individual files may be included.

Rustfmt

Rustfmt uses the ParseSess::with_span_handler() constructor, so I've updated it to use a default (unlimited) sandbox in rust-lang/rustfmt#2566.

@rust-highfive
Copy link
Collaborator

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @nikomatsakis (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 26, 2018
@jsgf
Copy link
Contributor Author

jsgf commented Mar 26, 2018

OK, I'll fix up the Cargo.locks

@jsgf
Copy link
Contributor Author

jsgf commented Mar 26, 2018

Corresponding rustfmt PR rust-lang/rustfmt#2566

@jsgf jsgf force-pushed the rustc-env-sb branch 10 times, most recently from 611b892 to 0e98404 Compare March 27, 2018 23:01
@jsgf
Copy link
Contributor Author

jsgf commented Mar 28, 2018

cc @michaelwoerister since this is slightly related to path remapping.

@nikomatsakis
Copy link
Contributor

Is there any kind of RFC etc?

We do make various changes to rustc flags without them on occasion, but this seems like a series enough piece of design that it would be important to coordinate, no?

@jsgf
Copy link
Contributor Author

jsgf commented Apr 2, 2018

There's no RFC, but I could write one up if needed - the pull request is basically what it would say though. Should I post it to internals to see what people think?

@jsgf
Copy link
Contributor Author

jsgf commented Apr 2, 2018

@bors
Copy link
Contributor

bors commented Apr 3, 2018

☔ The latest upstream changes (presumably #49590) made this pull request unmergeable. Please resolve the merge conflicts.

@jsgf
Copy link
Contributor Author

jsgf commented Apr 5, 2018

rust-lang/cargo#5282

@bors
Copy link
Contributor

bors commented Apr 6, 2018

☔ The latest upstream changes (presumably #49154) made this pull request unmergeable. Please resolve the merge conflicts.

@bors
Copy link
Contributor

bors commented Apr 21, 2018

☔ The latest upstream changes (presumably #50080) made this pull request unmergeable. Please resolve the merge conflicts.

The sandbox can control which environment variables are readable
and can set their values.  It also maintains a set of path prefixes
which can be used for file access; accessing a path without one of
the defined prefixes is disallowed.
The environment sandbox controls access to environment variables and
which path prefixes are available for opening from source (ie, with
the env!() and include!() class of macros).

By default, it does nothing. However, if you configure some sandbox options
then they will override the default-open mode of operation.

The options are:
--env-clear - clear entire environment, making all env!() operations fail
--env-allow VAR - allow a specific environment variable to be used
--env-define VAR=VAL - define the value of an environment variable for env!()
--clear-include-prefixes - clear all valid prefixes, making all include!() operations fail
--include-prefix PATH - define a path prefix that all include files must start with

These options are cumulative.

The environment sandboxing is different from controlling the environment that's
present when rustc is invoked, say with the "env" command. Sandboxing allows the
environment used by rustc - such as PATH or LD_LIBRARY_PATH (or equiv) -
versus the environment that's available to the compiled Rust source itself.

(This change just collects the options and sets up the EnvSandbox, but does not
implement any constraints.)
@bors
Copy link
Contributor

bors commented Apr 26, 2018

☔ The latest upstream changes (presumably #50236) made this pull request unmergeable. Please resolve the merge conflicts.

@shepmaster shepmaster added S-blocked Status: Marked as blocked ❌ on something else such as an RFC or other implementation work. and removed S-blocked Status: Marked as blocked ❌ on something else such as an RFC or other implementation work. labels May 7, 2018
@pietroalbini pietroalbini added S-blocked Status: Marked as blocked ❌ on something else such as an RFC or other implementation work. and removed S-blocked Status: Marked as blocked ❌ on something else such as an RFC or other implementation work. labels May 14, 2018
@kennytm kennytm added S-blocked Status: Marked as blocked ❌ on something else such as an RFC or other implementation work. and removed S-blocked Status: Marked as blocked ❌ on something else such as an RFC or other implementation work. labels May 21, 2018
@pietroalbini pietroalbini added S-blocked Status: Marked as blocked ❌ on something else such as an RFC or other implementation work. and removed S-blocked Status: Marked as blocked ❌ on something else such as an RFC or other implementation work. labels May 28, 2018
@TimNN TimNN added A-allocators Area: Custom and system allocators and removed A-allocators Area: Custom and system allocators labels Jun 5, 2018
@pietroalbini pietroalbini added S-blocked Status: Marked as blocked ❌ on something else such as an RFC or other implementation work. and removed S-blocked Status: Marked as blocked ❌ on something else such as an RFC or other implementation work. labels Jun 25, 2018
@TimNN TimNN added A-allocators Area: Custom and system allocators and removed A-allocators Area: Custom and system allocators labels Jul 3, 2018
@TimNN
Copy link
Contributor

TimNN commented Jul 10, 2018

Ping from triage! Thanks for your PR @jsgf. It looks like this PR is blocked on an RFC which is going to take some time to resolve, so we're closing this PR for now. Feel free to reopen in the future.

@TimNN TimNN closed this Jul 10, 2018
@TimNN TimNN added S-blocked-closed and removed S-blocked Status: Marked as blocked ❌ on something else such as an RFC or other implementation work. labels Jul 10, 2018
@jyn514 jyn514 added S-blocked Status: Marked as blocked ❌ on something else such as an RFC or other implementation work. and removed S-blocked-closed labels Mar 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-blocked Status: Marked as blocked ❌ on something else such as an RFC or other implementation work.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants