Address a TOCTTOU vulnerability in SGX read/map (#131)

* Fix a bug where configuration error ends up doing a huge allocation, rather than catching the error.  Add some documentation to the slabmgr code.

* Add a unit test and some documentation to answer the question in issue #107.  I can't see how offset and map_start would end up being different.

* Rewrite of SGX file_map to remove TOCTTTOU now passes all unit tests

* Apply a similar fix to file_read.  

* Factor complicated verification code into a common helper routine.

* Adjust the memory copying strategy so that all bits in the returned buffed are exactly the same bits as verified in the trusted, scratch buffer.

* Fixing the TOCTOU issue in file checking

* Adding comments for load_trusted_file() and copy_and_check_trusted_file(); Deprecate the old design

* Documenting the file checking mechanism

LibOS/shim/test/apps/ltp/TIMEOUTS

@@ -17,6 +17,7 @@ fchdir02,40
 fchmod07,40
 fchmodat01,40
 fcntl02_64,40
+fcntl08_64,40
 fcntl13_64,40
 fork02,40
 futex_wait_bitset02,40
@@ -34,7 +35,8 @@ poll01,40
 pread01_64,40
 pread01,40
 pread02,40
-preadv01,40
+preadv01,80
+pwrite01_64,40
 pwrite04_64,40
 pwritev01,40
 read01,40

Pal/lib/api.h

@@ -38,6 +38,13 @@ typedef ptrdiff_t ssize_t;
 # define unlikely(x)	__builtin_expect((!!(x)),0)
 #endif
 
+#ifndef MIN
+# define MIN(a, b) ((a) < (b) ? (a) : (b))
+#endif
+#ifndef MAX
+# define MAX(a, b) ((a) > (b) ? (a) : (b))
+#endif
+
 #define __alloca __builtin_alloca
 
 #define XSTRINGIFY(x) STRINGIFY(x)

Pal/lib/crypto/adapters/mbedtls_adapter.c

@@ -101,6 +101,58 @@ int lib_AESCMAC(const uint8_t *key, uint64_t key_len, const uint8_t *input,
                                input, input_len, mac);
 }
 
+int lib_AESCMACInit(LIB_AESCMAC_CONTEXT * context,
+                    const uint8_t *key, uint64_t key_len)
+{
+    int ret;
+
+    switch (key_len) {
+    case 16:
+        context->cipher = MBEDTLS_CIPHER_AES_128_ECB;
+        break;
+    case 24:
+        context->cipher = MBEDTLS_CIPHER_AES_192_ECB;
+        break;
+    case 32:
+        context->cipher = MBEDTLS_CIPHER_AES_256_ECB;
+        break;
+    default:
+        return -PAL_ERROR_INVAL;
+    }
+
+    const mbedtls_cipher_info_t *cipher_info =
+        mbedtls_cipher_info_from_type(context->cipher);
+
+    if ( ( ret = mbedtls_cipher_setup( &context->ctx, cipher_info ) ) != 0 )
+        return ret;
+
+    return mbedtls_cipher_cmac_starts( &context->ctx, key,
+                                       key_len * BITS_PER_BYTE );
+}
+
+int lib_AESCMACUpdate(LIB_AESCMAC_CONTEXT * context, const uint8_t * input,
+                      uint64_t input_len)
+{
+    return mbedtls_cipher_cmac_update( &context->ctx, input, input_len );
+}
+
+int lib_AESCMACFinish(LIB_AESCMAC_CONTEXT * context, uint8_t * mac,
+                      uint64_t mac_len)
+{
+    const mbedtls_cipher_info_t *cipher_info =
+        mbedtls_cipher_info_from_type(context->cipher);
+
+    int ret = -PAL_ERROR_INVAL;
+    if (mac_len < cipher_info->block_size)
+        goto exit;
+
+    ret = mbedtls_cipher_cmac_finish( &context->ctx, mac );
+
+exit:
+    mbedtls_cipher_free( &context->ctx );
+    return( ret );
+}
+
 int lib_RSAInitKey(LIB_RSA_KEY *key)
 {
     /* For now, we only need PKCS_V15 type padding. If we need to support

Pal/lib/pal_crypto.h

@@ -68,6 +68,10 @@ typedef mbedtls_sha256_context LIB_SHA256_CONTEXT;
 #include "crypto/mbedtls/mbedtls/dhm.h"
 typedef mbedtls_dhm_context LIB_DH_CONTEXT;
 typedef mbedtls_rsa_context LIB_RSA_KEY;
+typedef struct {
+    mbedtls_cipher_type_t cipher;
+    mbedtls_cipher_context_t ctx;
+} LIB_AESCMAC_CONTEXT;
 #endif /* CRYPTO_USE_MBEDTLS */
 
 #ifndef CRYPTO_PROVIDER_SPECIFIED
@@ -92,6 +96,15 @@ void lib_DhFinal(LIB_DH_CONTEXT *context);
 int lib_AESCMAC(const uint8_t *key, uint64_t key_len, const uint8_t *input,
                 uint64_t input_len, uint8_t *mac, uint64_t mac_len);
 
+/* note: 'lib_AESCMAC' is the combination of 'lib_AESCMACInit',
+ * 'lib_AESCMACUpdate', and 'lib_AESCMACFinish'. */
+int lib_AESCMACInit(LIB_AESCMAC_CONTEXT * context,
+                    const uint8_t *key, uint64_t key_len);
+int lib_AESCMACUpdate(LIB_AESCMAC_CONTEXT * context, const uint8_t * input,
+                      uint64_t input_len);
+int lib_AESCMACFinish(LIB_AESCMAC_CONTEXT * context, uint8_t * mac,
+                      uint64_t mac_len);
+
 /* RSA. Limited functionality. */
 // Initializes the key structure
 int lib_RSAInitKey(LIB_RSA_KEY *key);

Pal/regression/02_File.py

@@ -1,16 +1,13 @@
 #!/usr/bin/python
 
-import os, sys, mmap, random, string
+import os, sys, mmap, random, string, binascii
 from regression import Regression
 
 loader = os.environ['PAL_LOADER']
 
 def prepare_files(args):
     global file_exist
-    file_exist = ''.join([random.choice(string.ascii_letters) for i in range(mmap.PAGESIZE)])
-
-    with open("file_exist.tmp", "w") as f:
-        f.write(file_exist)
+    file_exist = open('File', 'rb').read()
 
     if os.path.exists("file_nonexist.tmp"):
         os.remove("file_nonexist.tmp")
@@ -32,9 +29,9 @@ regression.add_check(name="Basic File Creation",
                       "File Creation Test 3 OK" in res[0].log)
 
 regression.add_check(name="File Reading",
-    check=lambda res: ("Read Test 1 (0th - 40th): " + file_exist[0:40]) in res[0].log and
-                      ("Read Test 2 (0th - 40th): " + file_exist[0:40]) in res[0].log and
-                      ("Read Test 3 (200th - 240th): " + file_exist[200:240]) in res[0].log)
+    check=lambda res: ("Read Test 1 (0th - 40th): " + binascii.hexlify(file_exist[0:40])) in res[0].log and
+                      ("Read Test 2 (0th - 40th): " + binascii.hexlify(file_exist[0:40])) in res[0].log and
+                      ("Read Test 3 (200th - 240th): " + binascii.hexlify(file_exist[200:240])) in res[0].log)
 
 def check_write(res):
     global file_exist
@@ -46,14 +43,17 @@ def check_write(res):
 regression.add_check(name="File Writing", check=check_write)
 
 regression.add_check(name="File Attribute Query",
-    check=lambda res: ("Query: type = 1, size = %d" % (mmap.PAGESIZE)) in res[0].log)
+    check=lambda res: ("Query: type = 1, size = %d" % (len(file_exist))) in res[0].log)
 
 regression.add_check(name="File Attribute Query by Handle",
-    check=lambda res: ("Query by Handle: type = 1, size = %d" % (mmap.PAGESIZE)) in res[0].log)
+    check=lambda res: ("Query by Handle: type = 1, size = %d" % (len(file_exist))) in res[0].log)
 
 regression.add_check(name="File Mapping",
-    check=lambda res: ("Map Test 1 (0th - 40th): " + file_exist[0:40]) in res[0].log and
-                      ("Map Test 2 (200th - 240th): " + file_exist[200:240]) in res[0].log)
+    check=lambda res: ("Map Test 1 (0th - 40th): " + binascii.hexlify(file_exist[0:40])) in res[0].log and
+                      ("Map Test 2 (200th - 240th): " + binascii.hexlify(file_exist[200:240])) in res[0].log and
+                      ("Map Test 3 (4096th - 4136th): " + binascii.hexlify(file_exist[4096:4136])) in res[0].log and
+                      ("Map Test 4 (4296th - 4336th): " +
+ binascii.hexlify(file_exist[4296:4336])) in res[0].log)
 
 regression.add_check(name="Set File Length",
     check=lambda res: os.stat("file_nonexist.tmp").st_size == mmap.ALLOCATIONGRANULARITY)

Pal/regression/File.c

@@ -5,14 +5,30 @@
 #include "pal_debug.h"
 #include "api.h"
 
+#define NUM_TO_HEX(num) \
+    ((num) >= 10 ? 'a' + ((num) - 10) : '0' + (num))
+
+static __attribute__((noinline))
+void print_hex (char * fmt, const void * data, int len)
+{
+    char * buf = __alloca(len * 2 + 1);
+    buf[len * 2] = '\0';
+    for (int i = 0; i < len; i++) {
+        unsigned char b = ((unsigned char *)data)[i];
+        buf[i * 2] = NUM_TO_HEX(b >> 4);
+        buf[i * 2 + 1] = NUM_TO_HEX(b & 0xf);
+    }
+    pal_printf(fmt, buf);
+}
+
 int main (int argc, char ** argv, char ** envp)
 {
-    char buffer1[41], buffer2[41];
+    char buffer1[40], buffer2[40], buffer3[40];
     int ret;
 
     /* test regular file opening */
 
-    PAL_HANDLE file1 = DkStreamOpen("file:file_exist.tmp",
+    PAL_HANDLE file1 = DkStreamOpen("file:File",
                                     PAL_ACCESS_RDWR, 0, 0, 0);
     if (file1) {
         pal_printf("File Open Test 1 OK\n");
@@ -22,19 +38,19 @@ int main (int argc, char ** argv, char ** envp)
         ret = DkStreamRead(file1, 0, 40, buffer1, NULL, 0);
         if (ret > 0) {
             buffer1[ret] = 0;
-            pal_printf("Read Test 1 (0th - 40th): %s\n", buffer1);
+            print_hex("Read Test 1 (0th - 40th): %s\n", buffer1, 40);
         }
 
         ret = DkStreamRead(file1, 0, 40, buffer1, NULL, 0);
         if (ret > 0) {
             buffer1[ret] = 0;
-            pal_printf("Read Test 2 (0th - 40th): %s\n", buffer1);
+            print_hex("Read Test 2 (0th - 40th): %s\n", buffer1, 40);
         }
 
         ret = DkStreamRead(file1, 200, 40, buffer2, NULL, 0);
         if (ret > 0) {
             buffer2[ret] = 0;
-            pal_printf("Read Test 3 (200th - 240th): %s\n", buffer2);
+            print_hex("Read Test 3 (200th - 240th): %s\n", buffer2, 40);
         }
 
         /* test file attribute query */
@@ -46,31 +62,44 @@ int main (int argc, char ** argv, char ** envp)
 
         /* test file map */
 
-        void * mem1 = (void *) DkStreamMap(file1, NULL, PAL_PROT_READ, 0,
-                                           attr1.pending_size);
+        void * mem1 = (void *) DkStreamMap(file1, NULL,
+                                PAL_PROT_READ | PAL_PROT_WRITECOPY, 0, 4096);
         if (mem1) {
             memcpy(buffer1, mem1, 40);
-            buffer1[40] = 0;
-            pal_printf("Map Test 1 (0th - 40th): %s\n", buffer1);
+            print_hex("Map Test 1 (0th - 40th): %s\n", buffer1, 40);
 
             memcpy(buffer2, mem1 + 200, 40);
-            buffer2[40] = 0;
-            pal_printf("Map Test 2 (200th - 240th): %s\n", buffer2);
+            print_hex("Map Test 2 (200th - 240th): %s\n", buffer2, 40);
+
+            DkStreamUnmap(mem1, 4096);
+        } else {
+            pal_printf("Map Test 1 & 2: Failed to map buffer\n");
+        }
+
+        /* DEP 11/24/17: For SGX writecopy exercises a different path in the PAL */
+        void * mem2 = (void *) DkStreamMap(file1, NULL,
+                                PAL_PROT_READ | PAL_PROT_WRITECOPY, 4096, 4096);
+        if (mem2) {
+            memcpy(buffer3, mem2, 40);
+            print_hex("Map Test 3 (4096th - 4136th): %s\n", buffer3, 40);
+
+            memcpy(buffer3, mem2 + 200, 40);
+            print_hex("Map Test 4 (4296th - 4336th): %s\n", buffer3, 40);
 
-            DkStreamUnmap(mem1, attr1.pending_size);
+            DkStreamUnmap(mem2, 4096);
         }
 
         DkObjectClose(file1);
     }
 
-    PAL_HANDLE file2 = DkStreamOpen("file:./file_exist.tmp",
+    PAL_HANDLE file2 = DkStreamOpen("file:File",
                                     PAL_ACCESS_RDWR, 0, 0, 0);
     if (file2) {
         pal_printf("File Open Test 2 OK\n");
         DkObjectClose(file2);
     }
 
-    PAL_HANDLE file3 = DkStreamOpen("file:../regression/file_exist.tmp",
+    PAL_HANDLE file3 = DkStreamOpen("file:../regression/File",
                                     PAL_ACCESS_RDWR, 0, 0, 0);
     if (file3) {
         pal_printf("File Open Test 3 OK\n");
@@ -78,7 +107,7 @@ int main (int argc, char ** argv, char ** envp)
     }
 
     PAL_STREAM_ATTR attr2;
-    if (DkStreamAttributesQuery("file:file_exist.tmp", &attr2))
+    if (DkStreamAttributesQuery("file:File", &attr2))
         pal_printf("Query: type = %d, size = %d\n",
                    attr2.handle_type, attr2.pending_size);
 

Pal/regression/File.manifest.template

@@ -14,7 +14,7 @@ net.allow_bind.1 = 127.0.0.1:8000
 # allow to connect to port 8000
 net.allow_peer.1 = 127.0.0.1:8000
 
-sgx.allowed_files.tmp1 = file:file_exist.tmp
-sgx.allowed_files.tmp2 = file:../regression/file_exist.tmp
+sgx.trusted_files.tmp1 = file:File
+sgx.trusted_files.tmp2 = file:../regression/File
 sgx.allowed_files.tmp3 = file:file_nonexist.tmp
 sgx.allowed_files.tmp4 = file:file_delete.tmp

Pal/src/db_main.c

@@ -94,11 +94,13 @@ static void read_environments (const char *** envpp)
         int len, idx;
     } * setenvs = NULL;
     int nsetenvs = 0;
+    int size;
 
     if (!pal_state.root_config)
         return;
 
     ssize_t cfgsize = get_config_entries_size(store, "loader.env");
+    /* XXX Propagate this error? */
     if (cfgsize < 0)
         return;
 

Pal/src/db_streams.c

@@ -571,6 +571,7 @@ DkStreamMap (PAL_HANDLE handle, PAL_PTR addr, PAL_FLG prot, PAL_NUM offset,
         LEAVE_PAL_CALL_RETURN((PAL_PTR) NULL);
     }
 
+    /* Check that all addresses and sizes are aligned */
     if ((addr && !ALLOC_ALIGNED(addr)) || !size || !ALLOC_ALIGNED(size) ||
         !ALLOC_ALIGNED(offset)) {
         _DkRaiseFailure(PAL_ERROR_INVAL);

Pal/src/do-rel.h

@@ -41,10 +41,6 @@
                               (void *) (l->l_addr + relative->r_offset))
 #endif
 
-#ifndef MIN
-# define MIN(a, b) (((a) < (b)) ? (a) : (b))
-#endif
-
 static void __attribute_unused
 elf_dynamic_do_rel (struct link_map *l, ElfW(Addr) reladdr, int relsize)
 {

Pal/src/host/Linux-SGX/db_files.c

@@ -99,6 +99,9 @@ static int64_t file_read (PAL_HANDLE handle, uint64_t offset, uint64_t count,
     if (stubs) {
         map_start = offset & ~(TRUSTED_STUB_SIZE - 1);
         map_end = (end + TRUSTED_STUB_SIZE - 1) & ~(TRUSTED_STUB_SIZE - 1);
+        /* Don't go past the end of file with the stub map either */
+        if (map_end > total)
+            map_end = ALLOC_ALIGNUP(total);
     } else {
         map_start = ALLOC_ALIGNDOWN(offset);
         map_end = ALLOC_ALIGNUP(end);
@@ -111,17 +114,18 @@ static int64_t file_read (PAL_HANDLE handle, uint64_t offset, uint64_t count,
         return -PAL_ERROR_DENIED;
 
     if (stubs) {
-        ret = verify_trusted_file(handle->file.realpath, umem,
-                                  map_start, map_end - map_start,
-                                  stubs, total);
-
+        ret = copy_and_verify_trusted_file(handle->file.realpath, umem,
+                                           map_start, map_end,
+                                           buffer, offset, end - offset,
+                                           stubs, total);
         if (ret < 0) {
-            ocall_unmap_untrusted(umem, map_start - map_end);
+            ocall_unmap_untrusted(umem, map_end - map_start);
             return ret;
         }
+    } else {
+        memcpy(buffer, umem + (offset - map_start), end - offset);
     }
 
-    memcpy(buffer, umem + offset - map_start, end - offset);
     ocall_unmap_untrusted(umem, map_end - map_start);
     return end - offset;
 }
@@ -204,6 +208,10 @@ static int file_map (PAL_HANDLE handle, void ** addr, int prot,
         return -PAL_ERROR_DENIED;
     }
 
+    mem = get_reserved_pages(mem, size);
+    if (!mem)
+        return -PAL_ERROR_NOMEM;
+
     uint64_t end = (offset + size > total) ? total : offset + size;
     uint64_t map_start, map_end;
 
@@ -223,29 +231,23 @@ static int file_map (PAL_HANDLE handle, void ** addr, int prot,
     }
 
     if (stubs) {
-        ret = verify_trusted_file(handle->file.realpath, umem,
-                                  map_start, map_end - map_start,
-                                  stubs, total);
+        ret = copy_and_verify_trusted_file(handle->file.realpath, umem,
+                                           map_start, map_end,
+                                           mem, offset, end - offset,
+                                           stubs, total);
 
         if (ret < 0) {
             SGX_DBG(DBG_E, "file_map - verify trusted returned %d\n", ret);
             ocall_unmap_untrusted(umem, map_end - map_start);
             return ret;
         }
-    }
-
-    /* The memory will always allocated with flag MAP_PRIVATE
-       and MAP_FILE */
-
-    mem = get_reserved_pages(mem, size);
-
-    if (mem) {
-        memcpy(mem, umem + offset - map_start, end - offset);
-        *addr = mem;
+    } else {
+        memcpy(mem, umem + (offset - map_start), end - offset);
     }
 
     ocall_unmap_untrusted(umem, map_end - map_start);
-    return mem ? 0 : -PAL_ERROR_NOMEM;
+    *addr = mem;
+    return 0;
 }
 
 /* 'setlength' operation for file stream. */

Pal/src/host/Linux-SGX/enclave_framework.c

@@ -114,6 +114,27 @@ int init_enclave_key (void)
     return 0;
 }
 
+/*
+ * The file integrity check is designed as follow:
+ *
+ * For each file that requires authentication (specified in the manifest
+ * as "sgx.trusted_files.xxx"), a SHA256 checksum is generated and stored
+ * in the manifest, signed and verified as part of the enclave's crypto
+ * measurement. When user requests for opening the file, Graphene loads
+ * the whole file, generate the SHA256 checksum, and check with the known
+ * checksums listed in the manifest. If the checksum does not match, and
+ * neither does the file is allowed for unauthenticated access, the file
+ * access will be rejected.
+ *
+ * During the generation of the SHA256 checksum, a 128-bit hash is also
+ * generated for each chunk in the file. The per-chunk hashes are used
+ * for partial verification in future reads, to avoid re-verifying the
+ * whole file again or the need of caching file contents. The per-chunk
+ * hashes are stored as "stubs" for each file. For a performance reason,
+ * each per-chunk hash is a 128-bit AES-CMAC hash value, using a secret
+ * key generated at the beginning of the enclave.
+ */
+
 DEFINE_LIST(trusted_file);
 struct trusted_file {
     LIST_TYPE(trusted_file) list;
@@ -132,9 +153,9 @@ static int trusted_file_indexes = 0;
 static int allow_file_creation = 0;
 
 
-/* Function: load_trusted_file
- * checks if the file to be opened is trusted or allowed,
- * according to the setting in manifest
+/*
+ * 'load_trusted_file' checks if the file to be opened is trusted
+ * or allowed for unauthenticated access, according to the manifest.
  *
  * file:     file handle to be opened
  * stubptr:  buffer for catching matched file stub.
@@ -143,7 +164,6 @@ static int allow_file_creation = 0;
  *
  * return:  0 succeed
  */
-
 int load_trusted_file (PAL_HANDLE file, sgx_stub_t ** stubptr,
                        uint64_t * sizeptr, int create)
 {
@@ -234,7 +254,7 @@ int load_trusted_file (PAL_HANDLE file, sgx_stub_t ** stubptr,
     if (!stubs)
         return -PAL_ERROR_NOMEM;
 
-    sgx_stub_t * s = stubs;
+    sgx_stub_t * s = stubs; /* stubs is an array of 128bit values */
     uint64_t offset = 0;
     LIB_SHA256_CONTEXT sha;
     void * umem;
@@ -244,20 +264,51 @@ int load_trusted_file (PAL_HANDLE file, sgx_stub_t ** stubptr,
         goto failed;
 
     for (; offset < tf->size ; offset += TRUSTED_STUB_SIZE, s++) {
-        uint64_t mapping_size = tf->size - offset;
-        if (mapping_size > TRUSTED_STUB_SIZE)
-            mapping_size = TRUSTED_STUB_SIZE;
+        /* For each stub, generate a 128bit hash of a file chunk with
+         * AES-CMAC, and then update the SHA256 digest. */
+        uint64_t mapping_size = MIN(tf->size - offset, TRUSTED_STUB_SIZE);
+        LIB_AESCMAC_CONTEXT aes_cmac;
+        ret = lib_AESCMACInit(&aes_cmac, (uint8_t *) &enclave_key,
+                              AES_CMAC_KEY_LEN);
+        if (ret < 0)
+            goto failed;
 
         ret = ocall_map_untrusted(fd, offset, mapping_size, PROT_READ, &umem);
         if (ret < 0)
             goto unmap;
 
-        lib_AESCMAC((void *) &enclave_key, AES_CMAC_KEY_LEN, umem,
-                    mapping_size, (uint8_t *) s, sizeof *s);
+        /*
+         * To prevent TOCTOU attack when generating the file checksum, we
+         * need to copy the file content into the enclave before hashing.
+         * For optimization, we use a relatively small buffer (1024 byte) to
+         * store the data for checksum generation.
+         */
+
+#define FILE_CHUNK_SIZE 1024
+
+        uint8_t small_chunk[FILE_CHUNK_SIZE]; /* Buffer for hashing */
+        int chunk_offset = 0;
+
+        for (; chunk_offset < mapping_size; chunk_offset += FILE_CHUNK_SIZE) {
+            uint64_t chunk_size = MIN(mapping_size - chunk_offset, FILE_CHUNK_SIZE);
 
-        /* update the file checksum */
-        ret = lib_SHA256Update(&sha, umem, mapping_size);
+            /* Any file content needs to be copied into the enclave before
+             * checking and re-hashing */
+            memcpy(small_chunk, umem + chunk_offset, chunk_size);
 
+            /* Update the file checksum */
+            ret = lib_SHA256Update(&sha, small_chunk, chunk_size);
+            if (ret < 0)
+                goto unmap;
+
+            /* Update the checksum for the file chunk */
+            ret = lib_AESCMACUpdate(&aes_cmac, small_chunk, chunk_size);
+            if (ret < 0)
+                goto unmap;
+        }
+
+        /* Store the checksum for one file chunk for checking */
+        ret = lib_AESCMACFinish(&aes_cmac, (uint8_t *) s, sizeof *s);
 unmap:
         ocall_unmap_untrusted(umem, mapping_size);
         if (ret < 0)
@@ -266,6 +317,9 @@ unmap:
 
     sgx_checksum_t hash;
 
+    /* Finalize and checking if the checksum of the whole file matches
+     * with record given in the manifest. */
+
     ret = lib_SHA256Final(&sha, (uint8_t *) hash.bytes);
     if (ret < 0)
         goto failed;
@@ -310,33 +364,145 @@ failed:
     return ret;
 }
 
-int verify_trusted_file (const char * uri, void * mem,
-                         uint64_t offset, uint64_t size,
-                         sgx_stub_t * stubs,
-                         uint64_t total_size)
+/*
+ * A common helper function for copying and checking the file contents
+ * from a buffer mapped outside the enclaves into an in-enclave buffer.
+ * If needed, regions at either the beginning or the end of the copied regions
+ * are copied into a scratch buffer to avoid a TOCTTOU race.
+ *
+ * * Note that it must be done this way to avoid the following TOCTTOU race
+ * * condition with the untrusted host as an adversary:
+ *       *  Adversary: put good contents in buffer
+ *       *  Enclave: buffer check passes
+ *       *  Adversary: put bad contents in buffer
+ *       *  Enclave: copies in bad buffer contents
+ *
+ * * For optimization, we verify the memory in place, as the application code
+ *   should not use the memory before return.  There can be subtle interactions
+ *   at the edges of a region with ELF loading.  Namely, the ELF loader will
+ *   want to map several file chunks that are not aligned to TRUSTED_STUB_SIZE
+ *   next to each other, sometimes overlapping.  There is probably room to
+ *   improve load time with more smarts around ELF loading, but for now, just
+ *   make things work.
+ *
+ * 'umem' is the untrusted file memory mapped outside the enclave (should
+ * already be mapped up by the caller). 'umem_start' and 'umem_end' are
+ * the offset _within the file_ of 'umem'.  'umem_start' should be aligned
+ * to the file checking chunk size (TRUSTED_STUB_SIZE). 'umem_end' can be
+ * either aligned, or equal to 'total_size'. 'buffer' is the in-enclave
+ * buffer for copying the file content. 'offset' is the offset within the file
+ * for copying into the buffer. 'size' is the size of the in-enclave buffer.
+ * 'stubs' contain the checksums of all the chunks in a file.
+ */
+int copy_and_verify_trusted_file (const char * path, const void * umem,
+                    uint64_t umem_start, uint64_t umem_end,
+                    void * buffer, uint64_t offset, uint64_t size,
+                    sgx_stub_t * stubs, uint64_t total_size)
 {
-    uint64_t checking = offset;
+    /* Check that the untrusted mapping is aligned to TRUSTED_STUB_SIZE
+     * and includes the range for copying into the buffer */
+    assert(umem_start % TRUSTED_STUB_SIZE == 0);
+    assert(offset >= umem_start && offset + size <= umem_end);
+
+    /* Start copying and checking at umem_start. The checked content may or
+     * may not be copied into the file content, depending on the offset of
+     * the content within the file. */
+    uint64_t checking = umem_start;
+    /* The stubs is an array of 128-bit hash values of the file chunks.
+     * from the beginning of the file. 's' points to the stub that needs to
+     * be checked for the current offset. */
     sgx_stub_t * s = stubs + checking / TRUSTED_STUB_SIZE;
+    int ret = 0;
 
-    for (; checking < offset + size ; checking += TRUSTED_STUB_SIZE, s++) {
-        uint64_t checking_size = TRUSTED_STUB_SIZE;
-        if (checking_size > total_size - checking)
-            checking_size = total_size - checking;
-
+    for (; checking < umem_end ; checking += TRUSTED_STUB_SIZE, s++) {
+        /* Check one chunk at a time. */
+        uint64_t checking_size = MIN(total_size - checking, TRUSTED_STUB_SIZE);
+        uint64_t checking_end = checking + checking_size;
         uint8_t hash[AES_CMAC_DIGEST_LEN];
-        lib_AESCMAC((void *) &enclave_key,
-                    AES_CMAC_KEY_LEN,
-                    mem + checking - offset, checking_size,
-                    hash, sizeof(hash));
 
+        if (checking >= offset && checking_end <= offset + size) {
+            /* If the checking chunk completely overlaps with the region
+             * needed for copying into the buffer, simplying use the buffer
+             * for checking */
+            memcpy(buffer + checking - offset, umem + checking - umem_start,
+                   checking_size);
+
+            /* Storing the checksum (using AES-CMAC) inside hash. */
+            ret = lib_AESCMAC((uint8_t *) &enclave_key,
+                              AES_CMAC_KEY_LEN,
+                              buffer + checking - offset, checking_size,
+                              hash, sizeof(hash));
+        } else {
+            /* If the checking chunk only partially overlaps with the region,
+             * read the file content in smaller chunks and only copy the part
+             * needed by the caller. */
+            LIB_AESCMAC_CONTEXT aes_cmac;
+            ret = lib_AESCMACInit(&aes_cmac, (uint8_t *) &enclave_key,
+                                  AES_CMAC_KEY_LEN);
+            if (ret < 0)
+                goto failed;
+
+            uint8_t small_chunk[FILE_CHUNK_SIZE]; /* A small buffer */
+            uint64_t chunk_offset = checking;
+
+            for (; chunk_offset < checking_end
+                 ; chunk_offset += FILE_CHUNK_SIZE) {
+                uint64_t chunk_size = MIN(checking_end - chunk_offset,
+                                          FILE_CHUNK_SIZE);
+
+                /* Copy into the small buffer before hashing the content */
+                memcpy(small_chunk, umem + (chunk_offset - umem_start),
+                       chunk_size);
+
+                /* Update the hash for the current chunk */
+                ret = lib_AESCMACUpdate(&aes_cmac, small_chunk, chunk_size);
+                if (ret < 0)
+                    goto failed;
+
+                /* Determine if the part just copied and checked is needed
+                 * by the caller. If so, copy it into the user buffer. */
+                uint64_t copy_start = chunk_offset;
+                uint64_t copy_end = copy_start + chunk_size;
+
+                if (copy_start < offset)
+                    copy_start = offset;
+                if (copy_end > offset + size)
+                    copy_end = offset + size;
+
+                if (copy_end > copy_start)
+                    memcpy(buffer + (copy_start - offset),
+                           small_chunk + (copy_start - chunk_offset),
+                           copy_end - copy_start);
+            }
+
+            /* Storing the checksum (using AES-CMAC) inside hash. */
+            ret = lib_AESCMACFinish(&aes_cmac, hash, sizeof(hash));
+        }
+
+        if (ret < 0)
+            goto failed;
+
+        /*
+         * Check if the hash matches with the checksum of current chunk.
+         * If not, return with access denied. Note: some file content may
+         * still be in the buffer (including the corrupted part).
+         * We assume the user won't use the content if this function
+         * returns with failures.
+         *
+         * XXX: Maybe we should zero the buffer after denying the access?
+         */
         if (memcmp(s, hash, sizeof(sgx_stub_t))) {
-            SGX_DBG(DBG_E, "Accesing file:%s is denied. "
-                    "Does not match with its MAC.\n", uri);
+            SGX_DBG(DBG_E, "Accesing file:%s is denied. Does not match with MAC"
+                    " at chunk starting at %llu-%llu.\n",
+                    path, checking, checking_end);
             return -PAL_ERROR_DENIED;
         }
     }
 
     return 0;
+
+failed:
+    return -PAL_ERROR_DENIED;
 }
 
 static int register_trusted_file (const char * uri, const char * checksum_str)

Pal/src/host/Linux-SGX/enclave_pages.c

@@ -77,8 +77,9 @@ void * get_reserved_pages(void * addr, uint64_t size)
     if (!size)
         return NULL;
 
+    SGX_DBG(DBG_M, "*** get_reserved_pages: heap_base %p, heap_size %llu, limit %p ***\n", heap_base, heap_size, heap_base + heap_size);
     if (addr >= heap_base + heap_size) {
-        SGX_DBG(DBG_M, "*** allocating out of heap: %p ***\n", addr);
+        SGX_DBG(DBG_E, "*** allocating out of heap: %p ***\n", addr);
         return NULL;
     }
 
@@ -255,6 +256,8 @@ void free_pages(void * addr, uint64_t size)
 {
     void * addr_top = addr + size;
 
+    SGX_DBG(DBG_M, "free_pages: trying to free %p %llu\n", addr, size);
+    
     if (!addr || !size)
         return;
 

Pal/src/host/Linux-SGX/pal_linux.h

@@ -120,9 +120,10 @@ int init_trusted_files (void);
 int load_trusted_file
     (PAL_HANDLE file, sgx_stub_t ** stubptr, uint64_t * sizeptr, int create);
 
-int verify_trusted_file
-    (const char * uri, void * mem, uint64_t offset, uint64_t size,
-     sgx_stub_t * stubs, uint64_t total_size);
+int copy_and_verify_trusted_file (const char * path, const void * umem,
+                    uint64_t umem_start, uint64_t umem_end,
+                    void * buffer, uint64_t offset, uint64_t size,
+                    sgx_stub_t * stubs, uint64_t total_size);
 
 int init_trusted_children (void);
 int register_trusted_child (const char * uri, const char * mrenclave_str);

Pal/src/pal_internal.h

@@ -75,8 +75,12 @@ struct handle_ops {
     int (*delete) (PAL_HANDLE handle, int access);
 
     /* 'map' and 'unmap' will map or unmap the handle into memory space,
-       it's not necessary mapped by mmap, so unmap also needs 'handle'
-       to deal with special cases */
+     * it's not necessary mapped by mmap, so unmap also needs 'handle'
+     * to deal with special cases.
+     * 
+     * Common PAL code will ensure that *address, offset, and size are 
+     * page-aligned. 'address' should not be NULL.
+     */
     int (*map) (PAL_HANDLE handle, void ** address, int prot, uint64_t offset,
                 uint64_t size);