Skip to content

Commit

Permalink
[Pal/Linux-SGX] Add uniform message on using insecure manifest options
Browse files Browse the repository at this point in the history
Previously, all insecure-manifest-option messages were scattered. Some
insecure manifest options were not warned about at all (e.g.
`sgx.debug`). This commit adds a uniform message with all insecure
manifest options detected, and only prints this message for the SGX PAL.

Signed-off-by: Dmitrii Kuvaiskii <dmitrii.kuvaiskii@intel.com>
  • Loading branch information
dimakuv committed Oct 1, 2021
1 parent 9a4fd15 commit 19fbf37
Show file tree
Hide file tree
Showing 4 changed files with 119 additions and 24 deletions.
3 changes: 2 additions & 1 deletion CONTRIBUTING.rst
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,8 @@ https://groups.google.com/g/gramine-users
Please verify that your change doesn't introduce any insecure-by-default
functionality. If an option allows users to introduce a security risk, the
option should have a name prefixed with ``insecure__`` and be disabled by
default.
default. All new insecure options must be added to the Linux-SGX PAL function
``print_warnings_on_insecure_configs()``.

Simple bugfixes need not have advance discussion, but we welcome queries from
newcomers.
Expand Down
17 changes: 1 addition & 16 deletions Pal/src/db_main.c
Original file line number Diff line number Diff line change
Expand Up @@ -467,13 +467,7 @@ noreturn void pal_main(uint64_t instance_id, /* current instance id */
"(the value must be `true` or `false`)");
}

if (use_cmdline_argv) {
/* Warn only in the first process. */
if (!parent_process) {
log_error("Using insecure argv source. Gramine will continue application execution, "
"but this configuration must not be used in production!");
}
} else {
if (!use_cmdline_argv) {
char* argv_src_file = NULL;

ret = toml_string_in(g_pal_state.manifest_root, "loader.argv_src_file", &argv_src_file);
Expand Down Expand Up @@ -509,15 +503,6 @@ noreturn void pal_main(uint64_t instance_id, /* current instance id */
"(the value must be `true` or `false`)");
}

if (use_host_env) {
/* Warn only in the first process. */
if (!parent_process) {
log_error("Forwarding host environment variables to the app is enabled. Gramine will "
"continue application execution, but this configuration must not be used in "
"production!");
}
}

char* env_src_file = NULL;
ret = toml_string_in(g_pal_state.manifest_root, "loader.env_src_file", &env_src_file);
if (ret < 0)
Expand Down
113 changes: 113 additions & 0 deletions Pal/src/host/Linux-SGX/db_main.c
Original file line number Diff line number Diff line change
Expand Up @@ -511,6 +511,113 @@ static int parse_host_topo_info(struct pal_sec* sec_info) {

extern void* g_enclave_base;
extern void* g_enclave_top;
extern bool g_allowed_files_warn;

static int print_warnings_on_insecure_configs(PAL_HANDLE parent_process) {
int ret;

if (parent_process) {
/* Warn only in the first process. */
return 0;
}

bool verbose_log_level = false;
bool sgx_debug = false;
bool use_cmdline_argv = false;
bool use_host_env = false;
bool disable_aslr = false;
bool allow_eventfd = false;
bool allow_all_files = false;
bool use_allowed_files = g_allowed_files_warn;

char* log_level_str = NULL;
ret = toml_string_in(g_pal_state.manifest_root, "loader.log_level", &log_level_str);
if (ret < 0)
goto out;
if (log_level_str && strcmp(log_level_str, "none") && strcmp(log_level_str, "error"))
verbose_log_level = true;

ret = toml_bool_in(g_pal_state.manifest_root, "sgx.debug",
/*defaultval=*/false, &sgx_debug);
if (ret < 0)
goto out;

ret = toml_bool_in(g_pal_state.manifest_root, "loader.insecure__use_cmdline_argv",
/*defaultval=*/false, &use_cmdline_argv);
if (ret < 0)
goto out;

ret = toml_bool_in(g_pal_state.manifest_root, "loader.insecure__use_host_env",
/*defaultval=*/false, &use_host_env);
if (ret < 0)
goto out;

ret = toml_bool_in(g_pal_state.manifest_root, "loader.insecure__disable_aslr",
/*defaultval=*/false, &disable_aslr);
if (ret < 0)
goto out;

ret = toml_bool_in(g_pal_state.manifest_root, "sys.insecure__allow_eventfd",
/*defaultval=*/false, &allow_eventfd);
if (ret < 0)
goto out;

if (get_file_check_policy() == FILE_CHECK_POLICY_ALLOW_ALL_BUT_LOG)
allow_all_files = true;

if (!verbose_log_level && !sgx_debug && !use_cmdline_argv && !use_host_env && !disable_aslr &&
!allow_eventfd && !allow_all_files && !use_allowed_files) {
/* there are no insecure configurations, skip printing */
ret = 0;
goto out;
}

log_always("-------------------------------------------------------------------------------"
"----------------------------------------");
log_always("Gramine detected the following insecure configurations:\n");

if (sgx_debug)
log_always(" - sgx.debug = true "
"(this is a debug enclave)");

if (verbose_log_level)
log_always(" - loader.log_level = warning|debug|trace|all "
"(verbose log level, may leak information)");

if (use_cmdline_argv)
log_always(" - loader.insecure__use_cmdline_argv = true "
"(forwarding command-line args from untrusted host to the app)");

if (use_host_env)
log_always(" - loader.insecure__use_host_env = true "
"(forwarding environment vars from untrusted host to the app)");

if (disable_aslr)
log_always(" - loader.insecure__disable_aslr = true "
"(Address Space Layout Randomization is disabled)");

if (allow_eventfd)
log_always(" - sys.insecure__allow_eventfd = true "
"(host-based eventfd is enabled)");

if (allow_all_files)
log_always(" - sgx.file_check_policy = allow_all_but_log "
"(all files are passed through from untrusted host without verification)");

if (use_allowed_files)
log_always(" - sgx.allowed_files = [ ... ] "
"(some files are passed through from untrusted host without verification)");

log_always("\nGramine will continue application execution, but this configuration must not be "
"used in production!");
log_always("-------------------------------------------------------------------------------"
"----------------------------------------\n");

ret = 0;
out:
free(log_level_str);
return ret;
}

/* Gramine uses GCC's stack protector that looks for a canary at gs:[0x8], but this function starts
* with a default canary and then updates it to a random one, so we disable stack protector here */
Expand Down Expand Up @@ -747,6 +854,12 @@ noreturn void pal_linux_main(char* uptr_libpal_uri, size_t libpal_uri_len, char*
ocall_exit(1, true);
}

/* this should be placed *after all* initialize-from-manifest routines */
if ((ret = print_warnings_on_insecure_configs(parent)) < 0) {
log_error("Cannot parse the manifest (while checking for insecure configurations)");
ocall_exit(1, true);
}

/* set up thread handle */
PAL_HANDLE first_thread = malloc(HANDLE_SIZE(thread));
if (!first_thread) {
Expand Down
10 changes: 3 additions & 7 deletions Pal/src/host/Linux-SGX/enclave_framework.c
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@

void* g_enclave_base;
void* g_enclave_top;
bool g_allowed_files_warn = false;

static int register_file(const char* uri, const char* checksum_str, bool check_duplicates);

Expand Down Expand Up @@ -760,13 +761,8 @@ static int init_trusted_files_from_toml_array(void) {
}

static void maybe_warn_about_allowed_files_usage(void) {
static bool g_allowed_files_warned = false;

if (!g_pal_state.parent_process && !g_allowed_files_warned) {
log_always("WARNING! \"allowed_files\" is an insecure feature designed for debugging and "
"prototyping, it must never be used in production!");
g_allowed_files_warned = true;
}
if (!g_pal_state.parent_process)
g_allowed_files_warn = true;
}

static int init_allowed_files_from_toml_table(void) {
Expand Down

0 comments on commit 19fbf37

Please sign in to comment.