From 4ebad2401ed7b809607e180fea2f1bb228306cd1 Mon Sep 17 00:00:00 2001 From: Don Porter Date: Thu, 5 Jul 2018 12:28:05 -0400 Subject: [PATCH] PAL should give a clearer error when the executable cannot be found. Add it, and make a unit test for this behavior. Fix loading by manifest on the SGX PAL, and fix the unit test that should be testing this. --- Pal/regression/00_Bootstrap.py | 15 +++++++- Pal/src/host/Linux-SGX/sgx_main.c | 64 +++++++++++++++++++++++-------- Pal/src/host/Linux/db_main.c | 1 + 3 files changed, 63 insertions(+), 17 deletions(-) diff --git a/Pal/regression/00_Bootstrap.py b/Pal/regression/00_Bootstrap.py index 8c7020ef..925bcfa3 100644 --- a/Pal/regression/00_Bootstrap.py +++ b/Pal/regression/00_Bootstrap.py @@ -100,10 +100,11 @@ if rv: sys.exit(rv) # Running Bootstrap2 -regression = Regression(loader, "Bootstrap2") +regression = Regression(loader, manifest_file("Bootstrap2")) regression.add_check(name="Control Block: Manifest as Executable Name", - check=lambda res: "Loaded Manifest: file:" + manifest_file("Bootstrap2") in res[0].log) + check=lambda res: "Loaded Manifest: file:" + manifest_file("Bootstrap2") in res[0].log + and "User Program Started" in res[0].log) rv = regression.run_checks() if rv: sys.exit(rv) @@ -178,3 +179,13 @@ regression.add_check(name="Load Large Number of Items in Manifest", rv = regression.run_checks() if rv: sys.exit(rv) + +# Running an executable that doesn't exist, should at least warn you +regression = Regression(loader, "fakenews") + +regression.add_check(name="Error on missing executable and manifest", + check=lambda res: "Executable not found" in res[0].log and + any([line.startswith("USAGE: ") for line in res[0].log])) + +rv = regression.run_checks() +if rv: sys.exit(rv) diff --git a/Pal/src/host/Linux-SGX/sgx_main.c b/Pal/src/host/Linux-SGX/sgx_main.c index b5bcbcae..c598bda6 100644 --- a/Pal/src/host/Linux-SGX/sgx_main.c +++ b/Pal/src/host/Linux-SGX/sgx_main.c @@ -667,7 +667,8 @@ finalize: static int load_enclave (struct pal_enclave * enclave, const char * manifest_uri, const char * exec_uri, - const char ** arguments, const char ** environments) + const char ** arguments, const char ** environments, + bool exec_uri_inferred) { struct pal_sec * pal_sec = &enclave->pal_sec; int ret; @@ -726,13 +727,15 @@ static int load_enclave (struct pal_enclave * enclave, return -EINVAL; } - if (exec_uri == NULL) { - if (get_config(enclave->config, "loader.exec", cfgbuf, CONFIG_MAX) > 0) { - exec_uri = resolve_uri(cfgbuf, &errstring); - if (!exec_uri) { - SGX_DBG(DBG_E, "%s: %s\n", errstring, cfgbuf); - return -EINVAL; - } + // A manifest can specify an executable with a different base name + // than the manifest itself. Always give the exec field of the manifest + // precedence if specified. + if (get_config(enclave->config, "loader.exec", cfgbuf, CONFIG_MAX) > 0) { + exec_uri = resolve_uri(cfgbuf, &errstring); + exec_uri_inferred = false; + if (!exec_uri) { + SGX_DBG(DBG_E, "%s: %s\n", errstring, cfgbuf); + return -EINVAL; } } @@ -741,8 +744,18 @@ static int load_enclave (struct pal_enclave * enclave, exec_uri + static_strlen("file:"), O_RDONLY|O_CLOEXEC, 0); if (IS_ERR(enclave->exec)) { - SGX_DBG(DBG_E, "cannot open executable %s\n", exec_uri); - return -EINVAL; + if (exec_uri_inferred) { + // It is valid for an enclave not to have an executable. + // We need to catch the case where we inferred the executable + // from the manifest file name, but it doesn't exist, and let + // the enclave go a bit further. Go ahead and warn the user, + // though. + SGX_DBG(DBG_I, "Inferred executable cannot be opened: %s. This may be ok, or may represent a manifest misconfiguration. This typically represents advanced usage, and if it is not what you intended, try setting the loader.exec field in the manifest.\n", exec_uri); + enclave->exec = -1; + } else { + SGX_DBG(DBG_E, "cannot open executable %s\n", exec_uri); + return -EINVAL; + } } } else { enclave->exec = -1; @@ -843,9 +856,13 @@ static int load_enclave (struct pal_enclave * enclave, int main (int argc, const char ** argv, const char ** envp) { - const char * manifest_uri = NULL, * exec_uri = NULL; + const char * manifest_uri = NULL; + char * exec_uri = NULL; const char * pal_loader = argv[0]; int retval = -EINVAL; + bool exec_uri_inferred = false; // Handle the case where the exec uri is + // inferred from the manifest name somewhat + // differently argc--; argv++; @@ -879,6 +896,8 @@ int main (int argc, const char ** argv, const char ** envp) int fd = INLINE_SYSCALL(open, 3, exec_uri + 5, O_RDONLY|O_CLOEXEC, 0); if (IS_ERR(fd)) { + SGX_DBG(DBG_E, "Executable not found\n"); + SGX_DBG(DBG_E, "USAGE: [executable|manifest] args ...\n"); retval = -ERRNO(fd); goto finalize; } @@ -905,8 +924,19 @@ int main (int argc, const char ** argv, const char ** envp) } if (memcmp(filebuf, "\177ELF", 4)) { - manifest_uri = exec_uri; - exec_uri = NULL; + // In this case the manifest is given as the executable. Set + // manifest_uri to sgx_manifest (should be the same), and + // and drop the .manifest* from exec_uri, so that the program + // loads properly. + manifest_uri = sgx_manifest; + size_t exec_len = strlen(exec_uri); + if (strcmp_static(exec_uri + exec_len - strlen(".manifest"), ".manifest")) { + exec_uri[exec_len - strlen(".manifest")] = '\0'; + exec_uri_inferred = true; + } else if (strcmp_static(exec_uri + exec_len - strlen(".manifest.sgx"), ".manifest.sgx")) { + exec_uri[exec_len - strlen(".manifest.sgx")] = '\0'; + exec_uri_inferred = true; + } } fd = INLINE_SYSCALL(open, 3, sgx_manifest, O_RDONLY|O_CLOEXEC, 0); @@ -920,8 +950,12 @@ int main (int argc, const char ** argv, const char ** envp) } SGX_DBG(DBG_I, "manifest file: %s\n", manifest_uri); + if (exec_uri) + SGX_DBG(DBG_I, "executable file: %s\n", exec_uri); + else + SGX_DBG(DBG_I, "executable file not found\n"); - return load_enclave(enclave, manifest_uri, exec_uri, argv, envp); + return load_enclave(enclave, manifest_uri, exec_uri, argv, envp, exec_uri_inferred); usage: SGX_DBG(DBG_E, "USAGE: %s [executable|manifest] args ...\n", pal_loader); @@ -949,5 +983,5 @@ int pal_init_enclave (const char * manifest_uri, memset(enclave, 0, sizeof(struct pal_enclave)); return load_enclave(enclave, manifest_uri, exec_uri, - arguments, environments); + arguments, environments, 0); } diff --git a/Pal/src/host/Linux/db_main.c b/Pal/src/host/Linux/db_main.c index b815bc5e..2259bbdf 100644 --- a/Pal/src/host/Linux/db_main.c +++ b/Pal/src/host/Linux/db_main.c @@ -281,6 +281,7 @@ void pal_linux_main (void * args) done_init: if (!parent && !exec && !manifest) { + printf("Executable not found\n"); printf("USAGE: %s [executable|manifest] args ...\n", pal_name); _DkProcessExit(0); return;