From 754d4e022b68b9604b404cda1eeb4e7d8157c8c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Kowalczyk?= Date: Fri, 2 Jul 2021 03:27:56 +0200 Subject: [PATCH] Strip confidential information from logs on "error" level MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Michał Kowalczyk --- Pal/src/host/Linux-SGX/db_exception.c | 17 ++------- Pal/src/host/Linux-SGX/db_files.c | 54 +++++++++++++-------------- Pal/src/host/Linux-SGX/enclave_pf.c | 36 +++++++++--------- 3 files changed, 48 insertions(+), 59 deletions(-) diff --git a/Pal/src/host/Linux-SGX/db_exception.c b/Pal/src/host/Linux-SGX/db_exception.c index 412b9cd146..837b5eb285 100644 --- a/Pal/src/host/Linux-SGX/db_exception.c +++ b/Pal/src/host/Linux-SGX/db_exception.c @@ -166,7 +166,7 @@ static bool handle_ud(sgx_cpu_context_t* uc) { /* syscall: LibOS may know how to handle this */ return false; } - log_error("Unknown or illegal instruction at RIP 0x%016lx", uc->rip); + log_error("Unknown or illegal instruction executed"); return false; } @@ -226,23 +226,12 @@ void _DkExceptionHandler(unsigned int exit_info, sgx_cpu_context_t* uc, if (ei.info.valid) { /* EXITINFO field: vector = exception number, exit_type = 0x3 for HW / 0x6 for SW */ - log_error("(SGX HW reported AEX vector 0x%x with exit_type = 0x%x)", ei.info.vector, + log_debug("(SGX HW reported AEX vector 0x%x with exit_type = 0x%x)", ei.info.vector, ei.info.exit_type); } else { - log_error("(untrusted PAL sent PAL event 0x%x)", ei.intval); + log_debug("(untrusted PAL sent PAL event 0x%x)", ei.intval); } - log_error("rax: 0x%08lx rcx: 0x%08lx rdx: 0x%08lx rbx: 0x%08lx\n" - "rsp: 0x%08lx rbp: 0x%08lx rsi: 0x%08lx rdi: 0x%08lx\n" - "r8 : 0x%08lx r9 : 0x%08lx r10: 0x%08lx r11: 0x%08lx\n" - "r12: 0x%08lx r13: 0x%08lx r14: 0x%08lx r15: 0x%08lx\n" - "rflags: 0x%08lx rip: 0x%08lx", - uc->rax, uc->rcx, uc->rdx, uc->rbx, - uc->rsp, uc->rbp, uc->rsi, uc->rdi, - uc->r8, uc->r9, uc->r10, uc->r11, - uc->r12, uc->r13, uc->r14, uc->r15, - uc->rflags, uc->rip); - _DkProcessExit(1); } diff --git a/Pal/src/host/Linux-SGX/db_files.c b/Pal/src/host/Linux-SGX/db_files.c index 03bc023821..7b639d0d19 100644 --- a/Pal/src/host/Linux-SGX/db_files.c +++ b/Pal/src/host/Linux-SGX/db_files.c @@ -45,7 +45,7 @@ static int file_open(PAL_HANDLE* handle, const char* type, const char* uri, int char* path = (void*)hdl + HANDLE_SIZE(file); int ret; if ((ret = get_norm_path(uri, path, &uri_size)) < 0) { - log_error("Could not normalize path (%s): %s", uri, pal_strerror(ret)); + log_warning("Could not normalize path (%s): %s", uri, pal_strerror(ret)); free(hdl); return ret; } @@ -71,7 +71,7 @@ static int file_open(PAL_HANDLE* handle, const char* type, const char* uri, int /* check if the file is seekable and get real file size */ ret = ocall_fstat(fd, &st); if (ret < 0) { - log_error("file_open(%s): fstat failed: %d", path, ret); + log_warning("file_open(%s): fstat failed: %d", path, ret); ret = unix_to_pal_error(ret); goto out; } @@ -90,7 +90,7 @@ static int file_open(PAL_HANDLE* handle, const char* type, const char* uri, int /* disallow opening more than one writable handle to a PF */ if (pf_mode & PF_FILE_MODE_WRITE) { if (pf->writable_fd >= 0) { - log_error("file_open(%s): disallowing concurrent writable handle", path); + log_warning("file_open(%s): disallowing concurrent writable handle", path); ret = -PAL_ERROR_DENIED; goto out; } @@ -100,7 +100,7 @@ static int file_open(PAL_HANDLE* handle, const char* type, const char* uri, int /* the protected files should be regular files (seekable) */ if (!hdl->file.seekable) { - log_error("file_open(%s): disallowing non-seekable file handle", path); + log_warning("file_open(%s): disallowing non-seekable file handle", path); goto out; } @@ -111,7 +111,7 @@ static int file_open(PAL_HANDLE* handle, const char* type, const char* uri, int pf->writable_fd = fd; } } else { - log_error("load_protected_file(%s, %d) failed", path, hdl->file.fd); + log_warning("load_protected_file(%s, %d) failed", path, hdl->file.fd); goto out; } } else { @@ -120,9 +120,9 @@ static int file_open(PAL_HANDLE* handle, const char* type, const char* uri, int void* umem; ret = load_trusted_file(hdl, &chunk_hashes, &total, create, &umem); if (ret < 0) { - log_error("Accessing file:%s is denied (%s). This file is not trusted or allowed." - " Trusted files should be regular files (seekable).", hdl->file.realpath, - pal_strerror(ret)); + log_warning("Accessing file:%s is denied (%s). This file is not trusted or allowed." + " Trusted files should be regular files (seekable).", hdl->file.realpath, + pal_strerror(ret)); goto out; } @@ -155,7 +155,7 @@ static int64_t pf_file_read(struct protected_file* pf, PAL_HANDLE handle, uint64 int fd = handle->file.fd; if (!pf->context) { - log_error("pf_file_read(PF fd %d): PF not initialized", fd); + log_warning("pf_file_read(PF fd %d): PF not initialized", fd); return -PAL_ERROR_BADHANDLE; } @@ -163,7 +163,7 @@ static int64_t pf_file_read(struct protected_file* pf, PAL_HANDLE handle, uint64 pf_status_t pfs = pf_read(pf->context, offset, count, buffer, &bytes_read); if (PF_FAILURE(pfs)) { - log_error("pf_file_read(PF fd %d): pf_read failed: %s", fd, pf_strerror(pfs)); + log_warning("pf_file_read(PF fd %d): pf_read failed: %s", fd, pf_strerror(pfs)); return -PAL_ERROR_DENIED; } @@ -216,14 +216,14 @@ static int64_t pf_file_write(struct protected_file* pf, PAL_HANDLE handle, uint6 int fd = handle->file.fd; if (!pf->context) { - log_error("pf_file_write(PF fd %d): PF not initialized", fd); + log_warning("pf_file_write(PF fd %d): PF not initialized", fd); return -PAL_ERROR_BADHANDLE; } pf_status_t pf_ret = pf_write(pf->context, offset, count, buffer); if (PF_FAILURE(pf_ret)) { - log_error("pf_file_write(PF fd %d): pf_write failed: %s", fd, pf_strerror(pf_ret)); + log_warning("pf_file_write(PF fd %d): pf_write failed: %s", fd, pf_strerror(pf_ret)); return -PAL_ERROR_DENIED; } @@ -254,7 +254,7 @@ static int64_t file_write(PAL_HANDLE handle, uint64_t offset, uint64_t count, co } /* case of trusted file: disallow writing completely */ - log_error("Writing to a trusted file (%s) is disallowed!", handle->file.realpath); + log_warning("Writing to a trusted file (%s) is disallowed!", handle->file.realpath); return -PAL_ERROR_DENIED; } @@ -324,12 +324,12 @@ static int pf_file_map(struct protected_file* pf, PAL_HANDLE handle, void** addr assert(WITHIN_MASK(prot, PAL_PROT_MASK)); if ((prot & PAL_PROT_READ) && (prot & PAL_PROT_WRITE)) { - log_error("pf_file_map(PF fd %d): trying to map with R+W access", fd); + log_warning("pf_file_map(PF fd %d): trying to map with R+W access", fd); return -PAL_ERROR_NOTSUPPORT; } if (!pf->context) { - log_error("pf_file_map(PF fd %d): PF not initialized", fd); + log_warning("pf_file_map(PF fd %d): PF not initialized", fd); return -PAL_ERROR_BADHANDLE; } @@ -370,8 +370,8 @@ static int pf_file_map(struct protected_file* pf, PAL_HANDLE handle, void** addr if (prot & PAL_PROT_READ) { /* we don't check this on writes since file size may be extended then */ if (offset >= pf_size) { - log_error("pf_file_map(PF fd %d): offset (%lu) >= file size (%lu)", fd, offset, - pf_size); + log_warning("pf_file_map(PF fd %d): offset (%lu) >= file size (%lu)", fd, offset, + pf_size); ret = -PAL_ERROR_INVAL; goto out; } @@ -385,7 +385,7 @@ static int pf_file_map(struct protected_file* pf, PAL_HANDLE handle, void** addr pf_ret = PF_STATUS_CORRUPTED; } if (PF_FAILURE(pf_ret)) { - log_error("pf_file_map(PF fd %d): pf_read failed: %s", fd, pf_strerror(pf_ret)); + log_warning("pf_file_map(PF fd %d): pf_read failed: %s", fd, pf_strerror(pf_ret)); ret = -PAL_ERROR_DENIED; goto out; } @@ -435,7 +435,7 @@ static int file_map(PAL_HANDLE handle, void** addr, int prot, uint64_t offset, u } if (!(prot & PAL_PROT_WRITECOPY) && (prot & PAL_PROT_WRITE)) { - log_error( + log_warning( "file_map does not currently support writable pass-through mappings on SGX. You " "may add the PAL_PROT_WRITECOPY (MAP_PRIVATE) flag to your file mapping to keep " "the writes inside the enclave but they won't be reflected outside of the " @@ -482,7 +482,7 @@ static int file_map(PAL_HANDLE handle, void** addr, int prot, uint64_t offset, u } else if (bytes == -EINTR || bytes == -EAGAIN) { continue; } else { - log_error("file_map - ocall_pread on allowed file returned %ld", bytes); + log_warning("file_map - ocall_pread on allowed file returned %ld", bytes); ret = unix_to_pal_error(bytes); goto out; } @@ -510,8 +510,8 @@ static int64_t pf_file_setlength(struct protected_file* pf, PAL_HANDLE handle, u pf_status_t pfs = pf_set_size(pf->context, length); if (PF_FAILURE(pfs)) { - log_error("pf_file_setlength(PF fd %d, %lu): pf_set_size returned %s", fd, length, - pf_strerror(pfs)); + log_warning("pf_file_setlength(PF fd %d, %lu): pf_set_size returned %s", fd, length, + pf_strerror(pfs)); return -PAL_ERROR_DENIED; } return length; @@ -538,12 +538,12 @@ static int file_flush(PAL_HANDLE handle) { if (pf) { int ret = flush_pf_maps(pf, /*buffer=*/NULL, /*remove=*/false); if (ret < 0) { - log_error("file_flush(PF fd %d): flush_pf_maps returned %s", fd, pal_strerror(ret)); + log_warning("file_flush(PF fd %d): flush_pf_maps returned %s", fd, pal_strerror(ret)); return ret; } pf_status_t pfs = pf_flush(pf->context); if (PF_FAILURE(pfs)) { - log_error("file_flush(PF fd %d): pf_flush returned %s", fd, pf_strerror(pfs)); + log_warning("file_flush(PF fd %d): pf_flush returned %s", fd, pf_strerror(pfs)); return -PAL_ERROR_DENIED; } } else { @@ -584,8 +584,8 @@ static int pf_file_attrquery(struct protected_file* pf, int fd_from_attrquery, c pf = load_protected_file(path, &fd_from_attrquery, real_size, PF_FILE_MODE_READ, /*create=*/false, pf); if (!pf) { - log_error("pf_file_attrquery: load_protected_file(%s, %d) failed", path, - fd_from_attrquery); + log_warning("pf_file_attrquery: load_protected_file(%s, %d) failed", path, + fd_from_attrquery); /* The call above will fail for PFs that were tampered with or have a wrong path. * glibc kills the process if this fails during directory enumeration, but that * should be fine given the scenario. @@ -643,7 +643,7 @@ static int file_attrquery(const char* type, const char* uri, PAL_STREAM_ATTR* at } ret = get_norm_path(uri, path, &len); if (ret < 0) { - log_error("Could not normalize path (%s): %s", uri, pal_strerror(ret)); + log_warning("Could not normalize path (%s): %s", uri, pal_strerror(ret)); goto out; } diff --git a/Pal/src/host/Linux-SGX/enclave_pf.c b/Pal/src/host/Linux-SGX/enclave_pf.c index cf7f4e6bf4..0dca373e2e 100644 --- a/Pal/src/host/Linux-SGX/enclave_pf.c +++ b/Pal/src/host/Linux-SGX/enclave_pf.c @@ -42,14 +42,14 @@ static pf_status_t cb_read(pf_handle_t handle, void* buffer, uint64_t offset, si continue; if (read < 0) { - log_error("cb_read(%d, %p, %lu, %lu): read failed: %ld", fd, buffer, offset, - size, read); + log_warning("cb_read(%d, %p, %lu, %lu): read failed: %ld", fd, buffer, offset, + size, read); return PF_STATUS_CALLBACK_FAILED; } /* EOF is an error condition, we want to read exactly `size` bytes */ if (read == 0) { - log_error("cb_read(%d, %p, %lu, %lu): EOF", fd, buffer, offset, size); + log_warning("cb_read(%d, %p, %lu, %lu): EOF", fd, buffer, offset, size); return PF_STATUS_CALLBACK_FAILED; } @@ -70,14 +70,14 @@ static pf_status_t cb_write(pf_handle_t handle, const void* buffer, uint64_t off continue; if (written < 0) { - log_error("cb_write(%d, %p, %lu, %lu): write failed: %ld", fd, buffer, offset, - size, written); + log_warning("cb_write(%d, %p, %lu, %lu): write failed: %ld", fd, buffer, offset, + size, written); return PF_STATUS_CALLBACK_FAILED; } /* EOF is an error condition, we want to write exactly `size` bytes */ if (written == 0) { - log_error("cb_write(%d, %p, %lu, %lu): EOF", fd, buffer, offset, size); + log_warning("cb_write(%d, %p, %lu, %lu): EOF", fd, buffer, offset, size); return PF_STATUS_CALLBACK_FAILED; } @@ -91,7 +91,7 @@ static pf_status_t cb_truncate(pf_handle_t handle, uint64_t size) { int fd = *(int*)handle; int ret = ocall_ftruncate(fd, size); if (ret < 0) { - log_error("cb_truncate(%d, %lu): ocall failed: %d", fd, size, ret); + log_warning("cb_truncate(%d, %lu): ocall failed: %d", fd, size, ret); return PF_STATUS_CALLBACK_FAILED; } return PF_STATUS_SUCCESS; @@ -108,7 +108,7 @@ static pf_status_t cb_aes_cmac(const pf_key_t* key, const void* input, size_t in int ret = lib_AESCMAC((const uint8_t*)key, sizeof(*key), input, input_size, (uint8_t*)mac, sizeof(*mac)); if (ret != 0) { - log_error("lib_AESCMAC failed: %d", ret); + log_warning("lib_AESCMAC failed: %d", ret); return PF_STATUS_CALLBACK_FAILED; } return PF_STATUS_SUCCESS; @@ -120,7 +120,7 @@ static pf_status_t cb_aes_gcm_encrypt(const pf_key_t* key, const pf_iv_t* iv, co int ret = lib_AESGCMEncrypt((const uint8_t*)key, sizeof(*key), (const uint8_t*)iv, input, input_size, aad, aad_size, output, (uint8_t*)mac, sizeof(*mac)); if (ret != 0) { - log_error("lib_AESGCMEncrypt failed: %d", ret); + log_warning("lib_AESGCMEncrypt failed: %d", ret); return PF_STATUS_CALLBACK_FAILED; } return PF_STATUS_SUCCESS; @@ -133,7 +133,7 @@ static pf_status_t cb_aes_gcm_decrypt(const pf_key_t* key, const pf_iv_t* iv, co input_size, aad, aad_size, output, (const uint8_t*)mac, sizeof(*mac)); if (ret != 0) { - log_error("lib_AESGCMDecrypt failed: %d", ret); + log_warning("lib_AESGCMDecrypt failed: %d", ret); return PF_STATUS_CALLBACK_FAILED; } return PF_STATUS_SUCCESS; @@ -142,7 +142,7 @@ static pf_status_t cb_aes_gcm_decrypt(const pf_key_t* key, const pf_iv_t* iv, co static pf_status_t cb_random(uint8_t* buffer, size_t size) { int ret = _DkRandomBitsRead(buffer, size); if (ret < 0) { - log_error("_DkRandomBitsRead failed: %d", ret); + log_warning("_DkRandomBitsRead failed: %d", ret); return PF_STATUS_CALLBACK_FAILED; } return PF_STATUS_SUCCESS; @@ -261,7 +261,7 @@ static int is_directory(const char* path, bool* is_dir) { fd = ret; ret = ocall_fstat(fd, &st); if (ret < 0) { - log_error("is_directory(%s): fstat failed: %d", path, ret); + log_warning("is_directory(%s): fstat failed: %d", path, ret); goto out; } @@ -272,7 +272,7 @@ static int is_directory(const char* path, bool* is_dir) { if (fd >= 0) { int rv = ocall_close(fd); if (rv < 0) { - log_error("is_directory(%s): close failed: %d", path, rv); + log_warning("is_directory(%s): close failed: %d", path, rv); } } @@ -291,7 +291,7 @@ static int register_protected_dir(const char* path) { ret = ocall_open(path, O_RDONLY | O_DIRECTORY, 0); if (ret < 0) { - log_error("register_protected_dir: opening %s failed: %d", path, ret); + log_warning("register_protected_dir: opening %s failed: %d", path, ret); ret = unix_to_pal_error(ret); goto out; } @@ -303,7 +303,7 @@ static int register_protected_dir(const char* path) { returned = ocall_getdents(fd, buf, bufsize); if (returned < 0) { ret = unix_to_pal_error(returned); - log_error("register_protected_dir: reading %s failed: %d", path, ret); + log_warning("register_protected_dir: reading %s failed: %d", path, ret); goto out; } @@ -355,7 +355,7 @@ static int register_protected_path(const char* path, struct protected_file** new size_t len = URI_MAX; ret = get_norm_path(path, normpath, &len); if (ret < 0) { - log_error("Couldn't normalize path (%s): %s", path, pal_strerror(ret)); + log_warning("Couldn't normalize path (%s): %s", path, pal_strerror(ret)); goto out; } @@ -533,7 +533,7 @@ static int open_protected_file(const char* path, struct protected_file* pf, pf_h pf_status_t pfs; pfs = pf_open(handle, path, size, mode, create, &g_pf_wrap_key, &pf->context); if (PF_FAILURE(pfs)) { - log_error("pf_open(%d, %s) failed: %s", *(int*)handle, path, pf_strerror(pfs)); + log_warning("pf_open(%d, %s) failed: %s", *(int*)handle, path, pf_strerror(pfs)); return -PAL_ERROR_DENIED; } return 0; @@ -621,7 +621,7 @@ int unload_protected_file(struct protected_file* pf) { return ret; pf_status_t pfs = pf_close(pf->context); if (PF_FAILURE(pfs)) { - log_error("unload_protected_file(%p) failed: %s", pf, pf_strerror(pfs)); + log_warning("unload_protected_file(%p) failed: %s", pf, pf_strerror(pfs)); } pf->context = NULL;