Skip to content

Commit

Permalink
Fix a regression on exec in trying to repro gramineproject#59. Remove…
Browse files Browse the repository at this point in the history
… an assertion that doesn't seem to be doing much good, given that the size is always statically determined in hex2str.

Fixing the bound of a debug buffer in SGX PAL

Rewrite bytes2hexstr; Add two new macros, alloca_bytes2hexstr() and malloc_bytes2hexstr().

Remove HASHBUF_SIZE

Add comments for the bytes2hexstr() macros.

Add a compile-time assertion
  • Loading branch information
donporter committed Nov 13, 2018
1 parent 1dcef5f commit 203a392
Show file tree
Hide file tree
Showing 11 changed files with 107 additions and 73 deletions.
24 changes: 0 additions & 24 deletions LibOS/shim/test/native/exec_victim.c

This file was deleted.

8 changes: 8 additions & 0 deletions LibOS/shim/test/regression/00_bootstrap.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,14 @@

regression.run_checks()

# Running Exec
regression = Regression(loader, "exec")

regression.add_check(name="2 page child binary",
check=lambda res: "00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000 000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000 000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000 000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000 000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000 000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000 000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000 000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000 000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000 000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000 000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000 000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000 000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000 000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000 000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000 000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000 " in res[0].out)

regression.run_checks()

# Shared Object Test
regression = Regression(loader, "shared_object")

Expand Down
File renamed without changes.
41 changes: 41 additions & 0 deletions LibOS/shim/test/regression/exec_victim.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
/* -*- mode:c; c-file-style:"k&r"; c-basic-offset: 4; tab-width:4; indent-tabs-mode:nil; mode:auto-fill; fill-column:78; -*- */
/* vim: set ts=4 sw=4 et tw=78 fo=cqt wm=0: */

#include <stdio.h>
#include <stdlib.h>

int main(int argc, char ** argv, const char ** envp)
{
FILE * out = stdout;

if (argc > 1) {
int fd = atoi(argv[argc - 1]);
printf("inherited file descriptor %d\n", fd);
out = fdopen(fd, "a");
if (!out) {
perror("fdopen");
exit(1);
}
}

fprintf(out, "Hello World (%s)!\n", argv[0]);
fprintf(out, "envp[\'IN_EXECVE\'] = %s\n", getenv("IN_EXECVE"));
fprintf(out, "00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000 \
000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000 \
000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000 \
000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000 \
000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000 \
000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000 \
000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000 \
000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000 \
000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000 \
000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000 \
000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000 \
000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000 \
000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000 \
000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000 \
000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000 \
000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000 \
\n");
return 0;
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
loader.preload = file:$(SHIMPATH)
loader.env.LD_LIBRARY_PATH = /lib
loader.debug_type = inline
loader.debug_type = none
loader.syscall_symbol = syscalldb

fs.mount.lib.type = chroot
Expand Down
2 changes: 2 additions & 0 deletions LibOS/shim/test/regression/manifest.template
Original file line number Diff line number Diff line change
Expand Up @@ -18,3 +18,5 @@ net.rules.2 = 0.0.0.0:0-65535:127.0.0.1:8000

sgx.trusted_files.ld = file:../../../../Runtime/ld-linux-x86-64.so.2
sgx.trusted_files.libc = file:../../../../Runtime/libc.so.6
sgx.trusted_files.victim = file:exec_victim
sgx.trusted_children.victim = file:exec_victim.sig
10 changes: 6 additions & 4 deletions Pal/lib/assert.h
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,19 @@

/*
* assert.h
*
* Define a common interface for assertions that builds for both the PAL
*
* Define a common interface for assertions that builds for both the PAL
* and libOS.
*
*
*/

#ifndef ASSERT_H
#define ASSERT_H

#define COMPILE_TIME_ASSERT(pred) switch(0){case 0:case pred:;}

/* All environments should implement warn, which prints a non-optional debug
* message. All environments should also implement __abort, which
* message. All environments should also implement __abort, which
* terminates the process.
*/

Expand Down
26 changes: 20 additions & 6 deletions Pal/lib/hex.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,13 @@
#ifndef HEX_H
#define HEX_H

/* This function is a helper for debug printing.
* It accepts a pointer to a numerical value, and
/* This function is a helper for debug printing.
* It accepts a pointer to a numerical value, and
* formats it as a hex string, for printing.
* size is the number of bytes pointed to by hex.
* str is the caller-provided buffer, len is the length of the buffer.
* The len must be at least (size * 2)+1.
*
*
* Note that it does not normalize for endianness, and pads to the
* size the compiler things the string is.
*/
Expand All @@ -51,7 +51,21 @@ char * __bytes2hexstr(void * hex, size_t size, char *str, size_t len)
#define IS_INDEXABLE(arg) (sizeof((arg)[0]))
#define IS_ARRAY(arg) (IS_INDEXABLE(arg) > 0 && (((void *) &(arg)) == ((void *) (arg))))

#define bytes2hexstr(array, str, len) (IS_ARRAY(array) ? \
__bytes2hexstr((array), sizeof(array), str, len) \
: NULL)

/*
* bytes2hexstr converts an array into a hexadecimal string and fills into a
* given buffer. The buffer size is given as an extra argument.
*/
#define bytes2hexstr(array, str, len) ({ \
COMPILE_TIME_ASSERT(IS_ARRAY(array)); \
__bytes2hexstr((array), sizeof(array), str, len);})

/*
* alloca_bytes2hexstr uses __alloca to allocate a buffer on the current frame
* and then fills the hexadecimal string into the buffer.
* This buffer can only be used within the caller frame (function).
*/
#define alloca_bytes2hexstr(array) \
(bytes2hexstr((array), __alloca(sizeof(array) * 2 + 1), sizeof(array) * 2 + 1))

#endif // HEX_H
5 changes: 2 additions & 3 deletions Pal/regression/Hex.c
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,7 @@
int main() {
char x[] = {0xde, 0xad, 0xbe, 0xef};
char y[] = {0xcd, 0xcd, 0xcd, 0xcd, 0xcd, 0xcd, 0xcd, 0xcd};
char buf[(sizeof(y) * 2) + 1];
pal_printf("Hex test 1 is %s\n", bytes2hexstr(x, buf, 17));
pal_printf("Hex test 2 is %s\n", bytes2hexstr(y, buf, 17));
pal_printf("Hex test 1 is %s\n", alloca_bytes2hexstr(x));
pal_printf("Hex test 2 is %s\n", alloca_bytes2hexstr(y));
return 0;
}
8 changes: 4 additions & 4 deletions Pal/src/host/Linux-SGX/db_process.c
Original file line number Diff line number Diff line change
Expand Up @@ -236,8 +236,8 @@ int _DkProcessCreate (PAL_HANDLE * handle, const char * uri,
sizeof(pal_enclave_state.enclave_keyhash),
data.keyhash_mac, sizeof data.keyhash_mac);

char mac_buf[MACBUF_SIZE];
SGX_DBG(DBG_P|DBG_S, "Attestation data: %s\n", bytes2hexstr(data.keyhash_mac, mac_buf, MACBUF_SIZE));
SGX_DBG(DBG_P|DBG_S, "Attestation data: %s\n",
alloca_bytes2hexstr(data.keyhash_mac));

ret = _DkStreamAttestationRequest(proc, &data,
&check_child_mrenclave, &param);
Expand Down Expand Up @@ -306,8 +306,8 @@ int init_child_process (PAL_HANDLE * parent_handle)
sizeof(pal_enclave_state.enclave_keyhash),
data.keyhash_mac, sizeof data.keyhash_mac);

char mac_buf[MACBUF_SIZE];
SGX_DBG(DBG_P|DBG_S, "Attestation data: %s\n", bytes2hexstr(data.keyhash_mac, mac_buf, MACBUF_SIZE));
SGX_DBG(DBG_P|DBG_S, "Attestation data: %s\n",
alloca_bytes2hexstr(data.keyhash_mac));

ret = _DkStreamAttestationRespond(parent, &data,
&check_parent_mrenclave,
Expand Down
54 changes: 23 additions & 31 deletions Pal/src/host/Linux-SGX/enclave_framework.c
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ void sgx_ocfree (void)
SET_ENCLAVE_TLS(ustack, GET_ENCLAVE_TLS(ustack_top));
}

#define HASHBUF_SIZE ((sizeof(sgx_arch_hash_t)*2)+1)
int sgx_get_report (sgx_arch_hash_t * mrenclave,
sgx_arch_attributes_t * attributes,
void * enclave_data,
Expand All @@ -56,21 +55,18 @@ int sgx_get_report (sgx_arch_hash_t * mrenclave,
if (ret)
return -PAL_ERROR_DENIED;

char hash_buf[HASHBUF_SIZE];
char mac_buf[MACBUF_SIZE];

SGX_DBG(DBG_S, "Generated report:\n");
SGX_DBG(DBG_S, " cpusvn: %08x %08x\n", report->cpusvn[0],
report->cpusvn[1]);
SGX_DBG(DBG_S, " mrenclave: %s\n", bytes2hexstr(report->mrenclave, hash_buf, HASHBUF_SIZE));
SGX_DBG(DBG_S, " mrsigner: %s\n", bytes2hexstr(report->mrsigner, hash_buf, HASHBUF_SIZE));
SGX_DBG(DBG_S, " mrenclave: %s\n", alloca_bytes2hexstr(report->mrenclave));
SGX_DBG(DBG_S, " mrsigner: %s\n", alloca_bytes2hexstr(report->mrsigner));
SGX_DBG(DBG_S, " attributes.flags: %016lx\n", report->attributes.flags);
SGX_DBG(DBG_S, " sttributes.xfrm: %016lx\n", report->attributes.xfrm);

SGX_DBG(DBG_S, " isvprodid: %02x\n", report->isvprodid);
SGX_DBG(DBG_S, " isvsvn: %02x\n", report->isvsvn);
SGX_DBG(DBG_S, " keyid: %s\n", bytes2hexstr(report->keyid, hash_buf, HASHBUF_SIZE));
SGX_DBG(DBG_S, " mac: %s\n", bytes2hexstr(report->mac, mac_buf, MACBUF_SIZE));
SGX_DBG(DBG_S, " keyid: %s\n", alloca_bytes2hexstr(report->keyid));
SGX_DBG(DBG_S, " mac: %s\n", alloca_bytes2hexstr(report->mac));

return 0;
}
Expand All @@ -91,9 +87,9 @@ int sgx_verify_report (sgx_arch_report_t * report)
return -PAL_ERROR_DENIED;
}

char key_buf[KEYBUF_SIZE];
SGX_DBG(DBG_S, "Get report key for verification: %s\n",
alloca_bytes2hexstr(enclave_key));

SGX_DBG(DBG_S, "Get report key for verification: %s\n", bytes2hexstr(enclave_key, key_buf, KEYBUF_SIZE));
return 0;
}

Expand All @@ -109,8 +105,7 @@ int init_enclave_key (void)
return -PAL_ERROR_DENIED;
}

char key_buf[KEYBUF_SIZE];
SGX_DBG(DBG_S, "Get sealing key: %s\n", bytes2hexstr(enclave_key, key_buf, KEYBUF_SIZE));
SGX_DBG(DBG_S, "Get sealing key: %s\n", alloca_bytes2hexstr(enclave_key));
return 0;
}

Expand Down Expand Up @@ -840,10 +835,10 @@ void test_dh (void)
FreeDhKey(&key1);
FreeDhKey(&key2);

SGX_DBG(DBG_S, "key exchange(side A): %s (%d)\n", __bytes2hexstr(agree1, agreesz1, scratch, (agreesz1 * 2) + 1),
agreesz1);
SGX_DBG(DBG_S, "key exchange(side B): %s (%d)\n", __bytes2hexstr(agree2, agreesz2, scratch, (agreesz2 * 2) + 1),
agreesz2);
SGX_DBG(DBG_S, "key exchange(side A): %s\n",
__bytes2hexstr(agree1, agreesz1, scratch, agreesz1 * 2 + 1));
SGX_DBG(DBG_S, "key exchange(side B): %s\n",
__bytes2hexstr(agree2, agreesz2, scratch, agreesz2 * 2 + 1));
}
#endif

Expand Down Expand Up @@ -887,9 +882,8 @@ int init_enclave (void)

pal_enclave_config.enclave_key = rsa;

char hash_buf[HASHBUF_SIZE];
SGX_DBG(DBG_S, "enclave (software) key hash: %s\n",
bytes2hexstr(pal_enclave_state.enclave_keyhash, hash_buf, HASHBUF_SIZE));
alloca_bytes2hexstr(pal_enclave_state.enclave_keyhash));

return 0;

Expand All @@ -901,7 +895,8 @@ int init_enclave (void)

int _DkStreamKeyExchange (PAL_HANDLE stream, PAL_SESSION_KEY * keyptr)
{
unsigned char session_key[32] __attribute__((aligned(32)));
uint8_t session_key[sizeof(PAL_SESSION_KEY)]
__attribute__((aligned(sizeof(PAL_SESSION_KEY))));
uint8_t pub[DH_SIZE] __attribute__((aligned(DH_SIZE)));
uint8_t agree[DH_SIZE] __attribute__((aligned(DH_SIZE)));
PAL_NUM pubsz, agreesz;
Expand Down Expand Up @@ -956,9 +951,8 @@ int _DkStreamKeyExchange (PAL_HANDLE stream, PAL_SESSION_KEY * keyptr)
for (int i = 0 ; i < agreesz ; i++)
session_key[i % sizeof(session_key)] ^= agree[i];

char key_buf[KEYBUF_SIZE];
SGX_DBG(DBG_S, "key exchange: (%p) %s\n", session_key,
bytes2hexstr(session_key, key_buf, KEYBUF_SIZE));
alloca_bytes2hexstr(session_key));

if (keyptr)
memcpy(keyptr, session_key, sizeof(PAL_SESSION_KEY));
Expand Down Expand Up @@ -994,9 +988,8 @@ int _DkStreamAttestationRequest (PAL_HANDLE stream, void * data,
memcpy(&req.attributes, &pal_sec.enclave_attributes,
sizeof(sgx_arch_attributes_t));

char hash_buf[HASHBUF_SIZE];
SGX_DBG(DBG_S, "Sending attestation request ... (mrenclave = %s)\n",\
bytes2hexstr(req.mrenclave, hash_buf, HASHBUF_SIZE));
alloca_bytes2hexstr(req.mrenclave));

for (bytes = 0, ret = 0 ; bytes < sizeof(req) ; bytes += ret) {
ret = _DkStreamWrite(stream, 0, sizeof(req) - bytes,
Expand All @@ -1017,7 +1010,7 @@ int _DkStreamAttestationRequest (PAL_HANDLE stream, void * data,
}

SGX_DBG(DBG_S, "Received attestation (mrenclave = %s)\n",
bytes2hexstr(att.mrenclave, hash_buf, HASHBUF_SIZE));
alloca_bytes2hexstr(att.mrenclave));

ret = sgx_verify_report(&att.report);
if (ret < 0) {
Expand All @@ -1040,7 +1033,7 @@ int _DkStreamAttestationRequest (PAL_HANDLE stream, void * data,

if (ret == 1) {
SGX_DBG(DBG_S, "Not an allowed encalve (mrenclave = %s)\n",
bytes2hexstr(att.mrenclave, hash_buf, HASHBUF_SIZE));
alloca_bytes2hexstr(att.mrenclave));
ret = -PAL_ERROR_DENIED;
goto out;
}
Expand All @@ -1058,7 +1051,7 @@ int _DkStreamAttestationRequest (PAL_HANDLE stream, void * data,
sizeof(sgx_arch_attributes_t));

SGX_DBG(DBG_S, "Sending attestation ... (mrenclave = %s)\n",
bytes2hexstr(att.mrenclave, hash_buf, HASHBUF_SIZE));
alloca_bytes2hexstr(att.mrenclave));

for (bytes = 0, ret = 0 ; bytes < sizeof(att) ; bytes += ret) {
ret = _DkStreamWrite(stream, 0, sizeof(att) - bytes,
Expand Down Expand Up @@ -1094,9 +1087,8 @@ int _DkStreamAttestationRespond (PAL_HANDLE stream, void * data,
}
}

char hash_buf[HASHBUF_SIZE];
SGX_DBG(DBG_S, "Received attestation request ... (mrenclave = %s)\n",
bytes2hexstr(req.mrenclave, hash_buf, HASHBUF_SIZE));
alloca_bytes2hexstr(req.mrenclave));

ret = sgx_get_report(&req.mrenclave, &req.attributes, data, &att.report);
if (ret < 0) {
Expand All @@ -1109,7 +1101,7 @@ int _DkStreamAttestationRespond (PAL_HANDLE stream, void * data,
sizeof(sgx_arch_attributes_t));

SGX_DBG(DBG_S, "Sending attestation ... (mrenclave = %s)\n",
bytes2hexstr(att.mrenclave, hash_buf, HASHBUF_SIZE));
alloca_bytes2hexstr(att.mrenclave));

for (bytes = 0, ret = 0 ; bytes < sizeof(att) ; bytes += ret) {
ret = _DkStreamWrite(stream, 0, sizeof(att) - bytes,
Expand All @@ -1130,7 +1122,7 @@ int _DkStreamAttestationRespond (PAL_HANDLE stream, void * data,
}

SGX_DBG(DBG_S, "Received attestation (mrenclave = %s)\n",
bytes2hexstr(att.mrenclave, hash_buf, HASHBUF_SIZE));
alloca_bytes2hexstr(att.mrenclave));

ret = sgx_verify_report(&att.report);
if (ret < 0) {
Expand All @@ -1152,7 +1144,7 @@ int _DkStreamAttestationRespond (PAL_HANDLE stream, void * data,

if (ret == 1) {
SGX_DBG(DBG_S, "Not an allowed enclave (mrenclave = %s)\n",
bytes2hexstr(att.mrenclave, hash_buf, HASHBUF_SIZE));
alloca_bytes2hexstr(att.mrenclave));
ret = -PAL_ERROR_DENIED;
goto out;
}
Expand Down

0 comments on commit 203a392

Please sign in to comment.