Add untrusted environment variable override

Add "untrusted" sections for environment variables defined in Occlum.json. Environment
variable defined in "default" will be shown in libos directly. Environment variable
defined in "untrusted" can be passed from occlum run or PAL layer and can override
the value in "default" and thus is considered "untrusted".

README.md

@@ -81,11 +81,21 @@ Occlum can be configured easily via a config file named `Occlum.json`, which is
     },
     // Environment variables
     //
-    // This gives a list of trusted environment variables for the "root"
+    // This gives a list of environment variables for the "root"
-    // process started by `occlum run` command.
+    // process started by `occlum exec` command.
-    "env": [
+    "env": {
-        "OCCLUM=yes"
+        // The default env vars given to each "root" LibOS process. As these env vars
-    ],
+        // are specified in this config file, they are considered trusted.
+        "default": [
+            "OCCLUM=yes"
+        ],
+        // The untrusted env vars that are captured by Occlum from the host environment
+        // and passed to the "root" LibOS processes. These untrusted env vars can
+        // override the trusted, default envs specified above.
+        "untrusted": [
+            "EXAMPLE"
+        ]
+    },
     // Entry points
     //
     // Entry points specify all valid path prefixes for <path> in `occlum run

etc/template/Occlum.json

@@ -7,9 +7,14 @@
         "default_heap_size": "32MB",
         "default_mmap_size": "80MB"
     },
-    "env": [
+    "env": {
-        "OCCLUM=yes"
+        "default": [
-    ],
+            "OCCLUM=yes"
+        ],
+        "untrusted": [
+            "EXAMPLE"
+        ]
+    },
     "entry_points": [
         "/bin"
     ],

src/Enclave.edl

@@ -38,6 +38,7 @@ enclave {
         public int occlum_ecall_new_process(
             [in, string] const char* executable_path,
             [user_check] const char** argv,
+            [user_check] const char** env,
             [in] const struct occlum_stdio_fds* io_fds);
 
         /*

src/exec/occlum_exec.proto

@@ -39,6 +39,7 @@ message ExecComm {
   string sockpath = 2;
   string command = 3;
   repeated string parameters = 4;
+  repeated string enviroments = 5;
 }
 
 message ExecCommResponse {

src/exec/src/bin/occlum_exec_client.rs

@@ -49,14 +49,20 @@ fn exec_command(
     client: &OcclumExecClient,
     command: &str,
     parameters: &[&str],
+    envs: &[&str],
 ) -> Result<u32, String> {
-    debug!("exec_command {:?} {:?}", command, parameters);
+    debug!("exec_command {:?} {:?} {:?}", command, parameters, envs);
 
     let mut parameter_list = RepeatedField::default();
     for p in parameters {
         parameter_list.push(p.to_string());
     }
 
+    let mut enviroments_list = RepeatedField::default();
+    for env in envs {
+        enviroments_list.push(env.to_string());
+    }
+
     let tmp_dir = TempDir::new("occlum_tmp").expect("create temp dir");
     let sockpath = tmp_dir.path().join("occlum.sock");
 
@@ -87,6 +93,7 @@ fn exec_command(
                     process_id: process::id(),
                     command: command.to_string(),
                     parameters: parameter_list,
+                    enviroments: enviroments_list,
                     sockpath: String::from(sockpath.as_path().to_str().unwrap()),
                     ..Default::default()
                 },
@@ -296,6 +303,7 @@ fn main() -> Result<(), i32> {
         .get_matches();
 
     let args: Vec<String> = env::args().collect();
+    let env: Vec<String> = env::vars().into_iter().map(|(key, val)| format!("{}={}", key, val)).collect();
 
     let mut sock_file = String::from(args[0].as_str());
     let sock_file = str::replace(
@@ -334,8 +342,9 @@ fn main() -> Result<(), i32> {
         };
 
         let (cmd, args) = cmd_args.split_first().unwrap();
+        let env: Vec<&str> = env.iter().map(|string| string.as_str()).collect();
 
-        match exec_command(&client, cmd, args) {
+        match exec_command(&client, cmd, args, &env) {
             Ok(process_id) => {
                 let signals = Signals::new(&[SIGUSR1]).unwrap();
                 let signal_thread = thread::spawn(move || {

src/exec/src/server.rs

@@ -231,12 +231,13 @@ impl OcclumExec for OcclumExecImpl {
 
         let cmd = req.command.clone();
         let args = req.parameters.into_vec().clone();
+        let envs = req.enviroments.into_vec().clone();
         let client_process_id = req.process_id;
 
         //Run the command in a thread
         thread::spawn(move || {
             let mut exit_status = Box::new(0);
-            rust_occlum_pal_exec(&cmd, &args, &stdio_fds, &mut exit_status)
+            rust_occlum_pal_exec(&cmd, &args, &envs, &stdio_fds, &mut exit_status)
                 .expect("failed to execute the command");
 
             reset_stop_timer(_execution_lock, _stop_timer, crate::DEFAULT_SERVER_TIMER);
@@ -336,6 +337,7 @@ extern "C" {
     fn occlum_pal_exec(
         cmd_path: *const libc::c_char,
         cmd_args: *const *const libc::c_char,
+        cmd_env: *const *const libc::c_char,
         io_fds: *const occlum_stdio_fds,
         exit_status: *mut i32,
     ) -> i32;
@@ -353,32 +355,38 @@ extern "C" {
     fn occlum_pal_kill(pid: i32, sig: i32) -> i32;
 }
 
+fn vec_strings_to_cchars(strings: &Vec<String>) -> Result<(Vec<*const libc::c_char>,Vec<CString>), i32> {
+    let mut strings_content = Vec::<CString>::new();
+    let mut cchar_strings = Vec::<*const libc::c_char>::new();
+    for string in strings {
+        let string = CString::new(string.as_str()).expect("arg: new failed");
+        cchar_strings.push(string.as_ptr());
+        strings_content.push(string);
+    }
+
+    cchar_strings.push(0 as *const libc::c_char);
+    Ok((cchar_strings, strings_content))
+}
+
 /// Executes the command inside Occlum enclave
 fn rust_occlum_pal_exec(
     cmd: &str,
     args: &Vec<String>,
+    envs: &Vec<String>,
     stdio: &occlum_stdio_fds,
     exit_status: &mut i32,
 ) -> Result<(), i32> {
     let cmd_path = CString::new(cmd).expect("cmd_path: new failed");
-    let mut cmd_args = Vec::<CString>::new();
+    let (cmd_args_array, _cmd_args) = vec_strings_to_cchars(args)?;
-    let mut cmd_args_array = Vec::<*const libc::c_char>::new();
+    let (cmd_envs_array, _cmd_envs) = vec_strings_to_cchars(envs)?;
-    for arg in args {
-        let arg = CString::new(arg.as_str()).expect("arg: new failed");
-        &cmd_args_array.push(arg.as_ptr());
-        &cmd_args.push(arg);
-    }
-
-    cmd_args_array.push(0 as *const libc::c_char);
 
     let stdio_raw = Box::new(stdio);
 
-    info!("{:?} {:?}", cmd_path, cmd_args);
-
     let ret = unsafe {
         occlum_pal_exec(
             cmd_path.as_ptr() as *const libc::c_char,
             Box::into_raw(cmd_args_array.into_boxed_slice()) as *const *const libc::c_char,
+            Box::into_raw(cmd_envs_array.into_boxed_slice()) as *const *const libc::c_char,
             *stdio_raw,
             exit_status as *mut i32,
         )

src/libos/src/config.rs

@@ -1,5 +1,6 @@
 use super::*;
 use serde::{Deserialize, Serialize};
+use std::collections::HashSet;
 use std::ffi::CString;
 use std::io::Read;
 use std::path::{Path, PathBuf};
@@ -78,7 +79,7 @@ fn parse_mac(mac_str: &str) -> Result<sgx_aes_gcm_128bit_tag_t> {
 pub struct Config {
     pub vm: ConfigVM,
     pub process: ConfigProcess,
-    pub env: Vec<CString>,
+    pub env: ConfigEnv,
     pub entry_points: Vec<PathBuf>,
     pub mount: Vec<ConfigMount>,
 }
@@ -95,6 +96,12 @@ pub struct ConfigProcess {
     pub default_mmap_size: usize,
 }
 
+#[derive(Debug)]
+pub struct ConfigEnv {
+    pub default: Vec<CString>,
+    pub untrusted: HashSet<String>,
+}
+
 #[derive(Debug)]
 pub struct ConfigMount {
     pub type_: ConfigMountFsType,
@@ -121,13 +128,7 @@ impl Config {
     fn from_input(input: &InputConfig) -> Result<Config> {
         let vm = ConfigVM::from_input(&input.vm)?;
         let process = ConfigProcess::from_input(&input.process)?;
-        let env = {
+        let env = ConfigEnv::from_input(&input.env)?;
-            let mut env = Vec::new();
-            for input_env in &input.env {
-                env.push(CString::new(input_env.clone())?);
-            }
-            env
-        };
         let entry_points = {
             let mut entry_points = Vec::new();
             for ep in &input.entry_points {
@@ -176,6 +177,15 @@ impl ConfigProcess {
     }
 }
 
+impl ConfigEnv {
+    fn from_input(input: &InputConfigEnv) -> Result<ConfigEnv> {
+        Ok(ConfigEnv {
+            default: input.default.clone(),
+            untrusted: input.untrusted.clone(),
+        })
+    }
+}
+
 impl ConfigMount {
     fn from_input(input: &InputConfigMount) -> Result<ConfigMount> {
         const ALL_FS_TYPES: [&str; 3] = ["sefs", "hostfs", "ramfs"];
@@ -256,7 +266,7 @@ struct InputConfig {
     #[serde(default)]
     pub process: InputConfigProcess,
     #[serde(default)]
-    pub env: Vec<String>,
+    pub env: InputConfigEnv,
     #[serde(default)]
     pub entry_points: Vec<String>,
     #[serde(default)]
@@ -318,6 +328,22 @@ impl Default for InputConfigProcess {
     }
 }
 
+#[derive(Deserialize, Debug)]
+#[serde(deny_unknown_fields)]
+struct InputConfigEnv {
+    pub default: Vec<CString>,
+    pub untrusted: HashSet<String>,
+}
+
+impl Default for InputConfigEnv {
+    fn default() -> InputConfigEnv {
+        InputConfigEnv {
+            default: Vec::new(),
+            untrusted: HashSet::new(),
+        }
+    }
+}
+
 #[derive(Deserialize, Debug)]
 #[serde(deny_unknown_fields)]
 struct InputConfigMount {

src/libos/src/entry.rs

@@ -81,24 +81,26 @@ pub extern "C" fn occlum_ecall_init(log_level: *const c_char, instance_dir: *con
 pub extern "C" fn occlum_ecall_new_process(
     path_buf: *const c_char,
     argv: *const *const c_char,
+    env: *const *const c_char,
     host_stdio_fds: *const HostStdioFds,
 ) -> i32 {
     if HAS_INIT.load(Ordering::SeqCst) == false {
         return ecall_errno!(EAGAIN);
     }
 
-    let (path, args, host_stdio_fds) = match parse_arguments(path_buf, argv, host_stdio_fds) {
+    let (path, args, env, host_stdio_fds) =
-        Ok(path_and_args_and_host_stdio_fds) => path_and_args_and_host_stdio_fds,
+        match parse_arguments(path_buf, argv, env, host_stdio_fds) {
-        Err(e) => {
+            Ok(all_parsed_args) => all_parsed_args,
-            eprintln!("invalid arguments for LibOS: {}", e.backtrace());
+            Err(e) => {
-            return ecall_errno!(e.errno());
+                eprintln!("invalid arguments for LibOS: {}", e.backtrace());
-        }
+                return ecall_errno!(e.errno());
-    };
+            }
+        };
 
     let _ = unsafe { backtrace::enable_backtrace(&ENCLAVE_PATH, PrintFormat::Short) };
     panic::catch_unwind(|| {
         backtrace::__rust_begin_short_backtrace(|| {
-            match do_new_process(&path, &args, &host_stdio_fds) {
+            match do_new_process(&path, &args, env, &host_stdio_fds) {
                 Ok(pid_t) => pid_t as i32,
                 Err(e) => {
                     eprintln!("failed to boot up LibOS: {}", e.backtrace());
@@ -180,8 +182,9 @@ fn parse_log_level(level_chars: *const c_char) -> Result<LevelFilter> {
 fn parse_arguments(
     path_ptr: *const c_char,
     argv: *const *const c_char,
+    env: *const *const c_char,
     host_stdio_fds: *const HostStdioFds,
-) -> Result<(PathBuf, Vec<CString>, HostStdioFds)> {
+) -> Result<(PathBuf, Vec<CString>, Vec<CString>, HostStdioFds)> {
     let path_buf = {
         if path_ptr.is_null() {
             return_errno!(EINVAL, "empty path");
@@ -208,26 +211,32 @@ fn parse_arguments(
     let mut args = clone_cstrings_safely(argv)?;
     args.insert(0, program_cstring);
 
+    let env_merged = merge_env(env)?;
+    trace!(
+        "env_merged = {:?}  (default env and untrusted env)",
+        env_merged
+    );
+
     let host_stdio_fds = HostStdioFds::from_user(host_stdio_fds)?;
 
-    Ok((path_buf, args, host_stdio_fds))
+    Ok((path_buf, args, env_merged, host_stdio_fds))
 }
 
 fn do_new_process(
     program_path: &PathBuf,
     argv: &Vec<CString>,
+    env_concat: Vec<CString>,
     host_stdio_fds: &HostStdioFds,
 ) -> Result<pid_t> {
     validate_program_path(program_path)?;
 
-    let envp = &config::LIBOS_CONFIG.env;
     let file_actions = Vec::new();
     let current = &process::IDLE;
     let program_path_str = program_path.to_str().unwrap();
     let new_tid = process::do_spawn_without_exec(
         &program_path_str,
         argv,
-        envp,
+        &env_concat,
         &file_actions,
         host_stdio_fds,
         current,
@@ -296,3 +305,42 @@ fn do_kill(pid: i32, sig: i32) -> Result<()> {
     };
     crate::signal::do_kill_from_outside_enclave(filter, signum)
 }
+
+fn merge_env(env: *const *const c_char) -> Result<Vec<CString>> {
+    #[derive(Debug)]
+    struct EnvDefaultInner {
+        content: Vec<CString>,
+        helper: HashMap<String, usize>, // Env key: index of content
+    }
+
+    let env_listed = &config::LIBOS_CONFIG.env.untrusted;
+    let mut env_checked: Vec<CString> = Vec::new();
+    let mut env_default = EnvDefaultInner {
+        content: Vec::new(),
+        helper: HashMap::new(),
+    };
+
+    // Use inner struct to parse env default
+    for (idx, val) in config::LIBOS_CONFIG.env.default.iter().enumerate() {
+        env_default.content.push(CString::new(val.clone())?);
+        let kv: Vec<&str> = val.to_str().unwrap().splitn(2, '=').collect(); // only split the first "="
+        env_default.helper.insert(kv[0].to_string(), idx);
+    }
+
+    // Filter out env which are not listed in Occlum.json env untrusted section
+    // and remove env default element if it is overrided
+    if (!env.is_null()) {
+        let env_untrusted = clone_cstrings_safely(env)?;
+        for iter in env_untrusted.iter() {
+            let env_kv: Vec<&str> = iter.to_str().unwrap().splitn(2, '=').collect();
+            if env_listed.contains(env_kv[0]) {
+                env_checked.push(iter.clone());
+                if let Some(idx) = env_default.helper.get(env_kv[0]) {
+                    env_default.content.remove(*idx);
+                }
+            }
+        }
+    }
+    trace!("env_checked from env untrusted: {:?}", env_checked);
+    Ok([env_default.content, env_checked].concat())
+}

src/pal/include/occlum_pal_api.h

@@ -8,7 +8,7 @@ extern "C" {
 /*
  * Occlum PAL API version number
  */
-#define OCCLUM_PAL_VERSION 1
+#define OCCLUM_PAL_VERSION 2
 
 /*
  * @brief Get version of Occlum PAL API
@@ -69,6 +69,8 @@ int occlum_pal_init(const struct occlum_pal_attr* attr);
  * @param cmd_path      The path of the command to be executed
  * @param cmd_args      The arguments to the command. The array must be NULL
  *                      terminated.
+ * @param cmd_env       The untrusted env vars to the command. The array must
+ *                      be NULL terminated.
  * @param io_fds        The file descriptors of the redirected standard I/O
  *                      (i.e., stdin, stdout, stderr), If set to NULL, will
  *                      use the original standard I/O file descriptors.
@@ -82,6 +84,7 @@ int occlum_pal_init(const struct occlum_pal_attr* attr);
  */
 int occlum_pal_exec(const char* cmd_path,
                     const char** cmd_args,
+                    const char** cmd_env,
                     const struct occlum_stdio_fds* io_fds,
                     int* exit_status);
 

src/pal/src/pal_api.c

@@ -51,6 +51,7 @@ int occlum_pal_init(const struct occlum_pal_attr* attr) {
 
 int occlum_pal_exec(const char* cmd_path,
                     const char** cmd_args,
+                    const char** cmd_env,
                     const struct occlum_stdio_fds* io_fds,
                     int* exit_status) {
     errno = 0;
@@ -68,7 +69,7 @@ int occlum_pal_exec(const char* cmd_path,
     }
 
     int ecall_ret = 0; // libos_tid
-    sgx_status_t ecall_status = occlum_ecall_new_process(eid, &ecall_ret, cmd_path, cmd_args, io_fds);
+    sgx_status_t ecall_status = occlum_ecall_new_process(eid, &ecall_ret, cmd_path, cmd_args, cmd_env, io_fds);
     if (ecall_status != SGX_SUCCESS) {
         const char* sgx_err = pal_get_sgx_error_msg(ecall_status);
         PAL_ERROR("Failed to do ECall: %s", sgx_err);

src/run/main.c

@@ -25,6 +25,7 @@ int main(int argc, char* argv[]) {
     }
     const char* cmd_path = (const char*) argv[1];
     const char** cmd_args = (const char**) &argv[2];
+    extern const char **environ;
 
     // Check Occlum PAL version
     int pal_version = occlum_pal_get_version();
@@ -47,7 +48,7 @@ int main(int argc, char* argv[]) {
         .stderr_fd = STDERR_FILENO,
     };
     int exit_status = 0;
-    if (occlum_pal_exec(cmd_path, cmd_args, &io_fds, &exit_status) < 0) {
+    if (occlum_pal_exec(cmd_path, cmd_args, environ, &io_fds, &exit_status) < 0) {
         // Command not found or other internal errors
         return 127;
     }

test/Occlum.json

@@ -7,10 +7,17 @@
         "default_heap_size": "8MB",
         "default_mmap_size": "32MB"
     },
-    "env": [
+    "env": {
-        "OCCLUM=yes",
+        "default": [
-        "TEST=true"
+            "OCCLUM=yes",
-    ],
+            "STABLE=yes",
+            "OVERRIDE=N"
+        ],
+        "untrusted": [
+            "TEST",
+            "OVERRIDE"
+        ]
+    },
     "entry_points": [
         "/bin"
     ],

test/env/Makefile

@@ -11,4 +11,5 @@ EXTRA_C_FLAGS := \
 	-DEXPECT_ARG2="\"$(ARG2)\"" \
 	-DEXPECT_ARG3="\"$(ARG3)\""
 EXTRA_LINK_FLAGS :=
+EXTRA_ENV := TEST=true STABLE=no OVERRIDE=Y
 BIN_ARGS := "$(ARG1)" "$(ARG2)" "$(ARG3)"

test/env/main.c

@@ -108,9 +108,25 @@ static int test_env_getenv() {
 
     // Here we call getenv() again to make sure that
     // LibOS can handle several environment variables in Occlum.json correctly
+    // TEST is set as untrusted in Occlum.json thus can be changed
     if (test_env_val("TEST", "true") < 0) {
         THROW_ERROR("get environment variable failed");
     }
+
+    // STABLE is set to "yes" as default in Occlum.json and is given a value of
+    // "no" from outside. Because it is not set to untrusted thus can't be modified
+    // and should have the value defined in Occlum.json
+    if (test_env_val("STABLE", "yes") < 0) {
+        THROW_ERROR("get environment variable failed");
+    }
+
+    // OVERRIDE is set to "N" as default in Occlum.json and is given a value of
+    // "Y" from outside. As it is also set to untrusted thus it should be modified
+    // and have the value passed from outside
+    if (test_env_val("OVERRIDE", "Y") < 0) {
+        THROW_ERROR("untrusted env override failed");
+    }
+
     return 0;
 }
 

test/test_common.mk

@@ -3,6 +3,7 @@ INCLUDE_MAKEFILE := $(lastword $(MAKEFILE_LIST))
 CUR_DIR := $(shell dirname $(realpath $(MAIN_MAKEFILE)))
 PROJECT_DIR := $(realpath $(CUR_DIR)/../../)
 SGX_MODE ?= HW
+EXTRA_ENV :=
 
 ifneq ($(SGX_MODE), HW)
 	BUILD_DIR := $(PROJECT_DIR)/build_sim
@@ -71,7 +72,7 @@ $(BUILD_DIR)/test/obj/$(TEST_NAME)/%.o: %.cc
 
 test:
 	@cd $(BUILD_DIR)/test && \
-		$(BUILD_DIR)/bin/occlum exec /bin/$(TEST_NAME) $(BIN_ARGS)
+		$(EXTRA_ENV) $(BUILD_DIR)/bin/occlum exec /bin/$(TEST_NAME) $(BIN_ARGS)
 
 test-native:
 	@LD_LIBRARY_PATH=/usr/local/occlum/lib cd $(IMAGE_DIR) && ./bin/$(TEST_NAME) $(BIN_ARGS)