From c14ee62678342520e743de26e2f9fd6b70975528 Mon Sep 17 00:00:00 2001 From: "Hui, Chunyang" Date: Wed, 6 May 2020 08:53:53 +0000 Subject: [PATCH] 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 | 20 +++++-- etc/template/Occlum.json | 11 ++-- src/Enclave.edl | 1 + src/exec/occlum_exec.proto | 1 + src/exec/src/bin/occlum_exec_client.rs | 13 ++++- src/exec/src/server.rs | 32 +++++++----- src/libos/src/config.rs | 44 ++++++++++++---- src/libos/src/entry.rs | 72 +++++++++++++++++++++----- src/pal/include/occlum_pal_api.h | 5 +- src/pal/src/pal_api.c | 3 +- src/run/main.c | 3 +- test/Occlum.json | 15 ++++-- test/env/Makefile | 1 + test/env/main.c | 16 ++++++ test/test_common.mk | 3 +- 15 files changed, 189 insertions(+), 51 deletions(-) diff --git a/README.md b/README.md index ec5952cb..675d3563 100644 --- a/README.md +++ b/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" - // process started by `occlum run` command. - "env": [ - "OCCLUM=yes" - ], + // This gives a list of environment variables for the "root" + // process started by `occlum exec` command. + "env": { + // 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 in `occlum run diff --git a/etc/template/Occlum.json b/etc/template/Occlum.json index c263eeee..f551925e 100644 --- a/etc/template/Occlum.json +++ b/etc/template/Occlum.json @@ -7,9 +7,14 @@ "default_heap_size": "32MB", "default_mmap_size": "80MB" }, - "env": [ - "OCCLUM=yes" - ], + "env": { + "default": [ + "OCCLUM=yes" + ], + "untrusted": [ + "EXAMPLE" + ] + }, "entry_points": [ "/bin" ], diff --git a/src/Enclave.edl b/src/Enclave.edl index b1a1c485..0bcba69f 100644 --- a/src/Enclave.edl +++ b/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); /* diff --git a/src/exec/occlum_exec.proto b/src/exec/occlum_exec.proto index 3029b740..79d15e67 100644 --- a/src/exec/occlum_exec.proto +++ b/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 { diff --git a/src/exec/src/bin/occlum_exec_client.rs b/src/exec/src/bin/occlum_exec_client.rs index 2dc3c7a9..19da3b5b 100644 --- a/src/exec/src/bin/occlum_exec_client.rs +++ b/src/exec/src/bin/occlum_exec_client.rs @@ -49,14 +49,20 @@ fn exec_command( client: &OcclumExecClient, command: &str, parameters: &[&str], + envs: &[&str], ) -> Result { - 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 = env::args().collect(); + let env: Vec = 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 || { diff --git a/src/exec/src/server.rs b/src/exec/src/server.rs index 5676b087..7040656d 100644 --- a/src/exec/src/server.rs +++ b/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) -> Result<(Vec<*const libc::c_char>,Vec), i32> { + let mut strings_content = Vec::::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, + envs: &Vec, 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::::new(); - let mut cmd_args_array = Vec::<*const libc::c_char>::new(); - 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 (cmd_args_array, _cmd_args) = vec_strings_to_cchars(args)?; + let (cmd_envs_array, _cmd_envs) = vec_strings_to_cchars(envs)?; 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, ) diff --git a/src/libos/src/config.rs b/src/libos/src/config.rs index 52cde5d2..56ba103b 100644 --- a/src/libos/src/config.rs +++ b/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 { pub struct Config { pub vm: ConfigVM, pub process: ConfigProcess, - pub env: Vec, + pub env: ConfigEnv, pub entry_points: Vec, pub mount: Vec, } @@ -95,6 +96,12 @@ pub struct ConfigProcess { pub default_mmap_size: usize, } +#[derive(Debug)] +pub struct ConfigEnv { + pub default: Vec, + pub untrusted: HashSet, +} + #[derive(Debug)] pub struct ConfigMount { pub type_: ConfigMountFsType, @@ -121,13 +128,7 @@ impl Config { fn from_input(input: &InputConfig) -> Result { let vm = ConfigVM::from_input(&input.vm)?; let process = ConfigProcess::from_input(&input.process)?; - let env = { - let mut env = Vec::new(); - for input_env in &input.env { - env.push(CString::new(input_env.clone())?); - } - env - }; + let env = ConfigEnv::from_input(&input.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 { + Ok(ConfigEnv { + default: input.default.clone(), + untrusted: input.untrusted.clone(), + }) + } +} + impl ConfigMount { fn from_input(input: &InputConfigMount) -> Result { 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, + pub env: InputConfigEnv, #[serde(default)] pub entry_points: Vec, #[serde(default)] @@ -318,6 +328,22 @@ impl Default for InputConfigProcess { } } +#[derive(Deserialize, Debug)] +#[serde(deny_unknown_fields)] +struct InputConfigEnv { + pub default: Vec, + pub untrusted: HashSet, +} + +impl Default for InputConfigEnv { + fn default() -> InputConfigEnv { + InputConfigEnv { + default: Vec::new(), + untrusted: HashSet::new(), + } + } +} + #[derive(Deserialize, Debug)] #[serde(deny_unknown_fields)] struct InputConfigMount { diff --git a/src/libos/src/entry.rs b/src/libos/src/entry.rs index db48cf1c..29ba3f4d 100644 --- a/src/libos/src/entry.rs +++ b/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) { - Ok(path_and_args_and_host_stdio_fds) => path_and_args_and_host_stdio_fds, - Err(e) => { - eprintln!("invalid arguments for LibOS: {}", e.backtrace()); - return ecall_errno!(e.errno()); - } - }; + let (path, args, env, host_stdio_fds) = + match parse_arguments(path_buf, argv, env, host_stdio_fds) { + Ok(all_parsed_args) => all_parsed_args, + Err(e) => { + 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 { 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, HostStdioFds)> { +) -> Result<(PathBuf, Vec, Vec, 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, + env_concat: Vec, host_stdio_fds: &HostStdioFds, ) -> Result { 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> { + #[derive(Debug)] + struct EnvDefaultInner { + content: Vec, + helper: HashMap, // Env key: index of content + } + + let env_listed = &config::LIBOS_CONFIG.env.untrusted; + let mut env_checked: Vec = 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()) +} diff --git a/src/pal/include/occlum_pal_api.h b/src/pal/include/occlum_pal_api.h index cc4882d5..f23d299f 100644 --- a/src/pal/include/occlum_pal_api.h +++ b/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); diff --git a/src/pal/src/pal_api.c b/src/pal/src/pal_api.c index b3893244..72b16e44 100644 --- a/src/pal/src/pal_api.c +++ b/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); diff --git a/src/run/main.c b/src/run/main.c index 2176b99e..11622425 100644 --- a/src/run/main.c +++ b/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; } diff --git a/test/Occlum.json b/test/Occlum.json index a38a6299..66b7ec89 100644 --- a/test/Occlum.json +++ b/test/Occlum.json @@ -7,10 +7,17 @@ "default_heap_size": "8MB", "default_mmap_size": "32MB" }, - "env": [ - "OCCLUM=yes", - "TEST=true" - ], + "env": { + "default": [ + "OCCLUM=yes", + "STABLE=yes", + "OVERRIDE=N" + ], + "untrusted": [ + "TEST", + "OVERRIDE" + ] + }, "entry_points": [ "/bin" ], diff --git a/test/env/Makefile b/test/env/Makefile index 221a1765..e5090b18 100644 --- a/test/env/Makefile +++ b/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)" diff --git a/test/env/main.c b/test/env/main.c index 93126186..5a9638ce 100644 --- a/test/env/main.c +++ b/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; } diff --git a/test/test_common.mk b/test/test_common.mk index a78c3dc9..b57a54af 100644 --- a/test/test_common.mk +++ b/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)