From d58d9723d0d1884681bec19240980444bcfceb5f Mon Sep 17 00:00:00 2001 From: Gianluca Scopelliti Date: Mon, 23 Aug 2021 17:08:38 +0200 Subject: [PATCH 01/27] linker.py: add KEEP to input_callbacks to preserve input symbols --- src/drivers/linker.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/drivers/linker.py b/src/drivers/linker.py index 4f3ec85..cf9bca7 100644 --- a/src/drivers/linker.py +++ b/src/drivers/linker.py @@ -829,7 +829,7 @@ def sort_key(entry): call_prog('msp430-gcc', ['-c', '-o', o_file, c_file]) - input_callbacks += ' {}(.sm.{}.callbacks)\n'.format(o_file, sm) + input_callbacks += ' KEEP({}(.sm.{}.callbacks))\n'.format(o_file, sm) input_callbacks += ' . = ALIGN(2);' # Table of connection keys From ec7dbeedc26069971b546a19cf2370ad60d432d6 Mon Sep 17 00:00:00 2001 From: Gianluca Scopelliti Date: Mon, 23 Aug 2021 17:12:42 +0200 Subject: [PATCH 02/27] changes in stubs and linker.py for Authentic Execution - support many-to-many relationships - linker.py: add "--num-connections" argument to set the maximum connections of a module (impacts on the allocated memory) - fix nonce use, prevent replay attacks - add attest() entry point - set_key: remove attestation - handle_input: fix bug when input data size is zero (use sancus_tag instead of sancus_unwrap) --- src/drivers/linker.py | 86 ++++++++++++++++++------------ src/sancus_support/reactive.h | 1 + src/stubs/CMakeLists.txt | 2 + src/stubs/reactive_stubs_support.h | 21 +++++--- src/stubs/sm_attest.c | 6 +++ src/stubs/sm_input.c | 64 +++++++++++++++++----- src/stubs/sm_output.c | 29 +++++++--- src/stubs/sm_set_key.c | 37 ++++++++----- 8 files changed, 170 insertions(+), 76 deletions(-) create mode 100644 src/stubs/sm_attest.c diff --git a/src/drivers/linker.py b/src/drivers/linker.py index cf9bca7..8b269d2 100644 --- a/src/drivers/linker.py +++ b/src/drivers/linker.py @@ -63,7 +63,7 @@ def add_sym(file, sym_map): args += [file, file] call_prog('msp430-elf-objcopy', args) - return file + return file def parse_size(val): @@ -92,16 +92,17 @@ def get_symbol(elf_file, name): def get_io_sym_map(sm_name): sym_map = { - '__sm_handle_input': '__sm_{}_handle_input'.format(sm_name), - '__sm_num_inputs': '__sm_{}_num_inputs'.format(sm_name), - '__sm_num_connections': '__sm_{}_num_connections'.format(sm_name), - '__sm_io_keys': '__sm_{}_io_keys'.format(sm_name), - '__sm_input_callbacks': '__sm_{}_input_callbacks'.format(sm_name), - '__sm_output_nonce': '__sm_{}_output_nonce'.format(sm_name), - '__sm_send_output': '__sm_{}_send_output'.format(sm_name), - '__sm_set_key': '__sm_{}_set_key'.format(sm_name), - '__sm_X_exit': '__sm_{}_exit'.format(sm_name), - '__sm_X_stub_malloc': '__sm_{}_stub_malloc'.format(sm_name), + '__sm_handle_input': '__sm_{}_handle_input'.format(sm_name), + '__sm_num_inputs': '__sm_{}_num_inputs'.format(sm_name), + '__sm_num_connections': '__sm_{}_num_connections'.format(sm_name), + '__sm_max_connections': '__sm_{}_max_connections'.format(sm_name), + '__sm_io_connections': '__sm_{}_io_connections'.format(sm_name), + '__sm_input_callbacks': '__sm_{}_input_callbacks'.format(sm_name), + '__sm_send_output': '__sm_{}_send_output'.format(sm_name), + '__sm_set_key': '__sm_{}_set_key'.format(sm_name), + '__sm_attest': '__sm_{}_attest'.format(sm_name), + '__sm_X_exit': '__sm_{}_exit'.format(sm_name), + '__sm_X_stub_malloc': '__sm_{}_stub_malloc'.format(sm_name), '__sm_X_stub_reactive_handle_output': '__sm_{}_stub_reactive_handle_output'.format(sm_name) } @@ -115,7 +116,7 @@ def get_io_sect_map(sm_name): '.rela.sm.X.text': '.rela.sm.{}.text'.format(sm_name), } - for entry in ('__sm{}_set_key', '__sm{}_handle_input'): + for entry in ('__sm{}_set_key', '__sm{}_attest', '__sm{}_handle_input'): map['.sm.X.{}.table'.format(entry.format(''))] = \ '.sm.{}.{}.table'.format(sm_name, entry.format('_' + sm_name)) map['.rela.sm.X.{}.table'.format(entry.format(''))] = \ @@ -136,15 +137,19 @@ def create_io_stub(sm, stub): def sort_entries(entries): - # If the set_key entry exists, it should have index 0 and if the - # handle_input entry exists, it should have index 1. This is accomplished by - # mapping those entries to __ and ___ respectively since those come + # If the set_key entry exists, it should have index 0, if the + # attest entry exists, it should have index 1 and if + # handle_input entry exists, it should have index 2. This is accomplished by + # mapping those entries to __, ___ and ___ respectively since those come # alphabetically before any valid entry name. def sort_key(entry): if re.match(r'__sm_\w+_set_key', entry.name): return '__' - if re.match(r'__sm_\w+_handle_input', entry.name): + if re.match(r'__sm_\w+_attest', entry.name): return '___' + if re.match(r'__sm_\w+_handle_input', entry.name): + return '____' + return entry.name entries.sort(key=sort_key) @@ -194,6 +199,12 @@ def sort_key(entry): '$PROJECT that is substituted for this Path in the linker. This allows projects ' 'to give the linker the correct path at compile time while still supporting a generic, ' 'project-dependent configuration file.') +parser.add_argument('--num-connections', + help='Parameter for Authentic Execution. Maximum number of ' + 'connections that the module can have. ' + 'This number impacts on the size of the module.', + type=int, + default=0) args, cli_ld_args = parser.parse_known_args() set_args(args) @@ -212,7 +223,7 @@ def sort_key(entry): for a in archive_files: debug("Unpacking archive for Sancus SM inspection: " + a) file_name = a - if ':' in a: + if ':' in a: # support calls such as -lib:/full/path file_name = file_name.split(':')[1] @@ -269,6 +280,7 @@ def sort_key(entry): elf_relocations = defaultdict(list) added_set_key_stub = False +added_attest_stub = False added_input_stub = False added_output_stub = False @@ -409,6 +421,14 @@ def sort_key(entry): input_files_to_scan.append(generated_file) added_set_key_stub = True + if not added_attest_stub: + # Generate the attest stub file + generated_file = create_io_stub(sm, 'sm_attest.o') + generated_object_files.append(generated_file) + # And register it to also be scanned by this loop later + input_files_to_scan.append(generated_file) + added_attest_stub = True + if which == 'input': dest = sms_inputs @@ -463,7 +483,7 @@ def sort_key(entry): The YAML file allows to set the following things: 1) warn whenever an ocall is performed and abort linking process 2) Swap out assembly stubs for custom project dependent values - 3) Set a peripheral offset for the first SM to + 3) Set a peripheral offset for the first SM to """ try: file_path = '' @@ -832,21 +852,17 @@ def sort_key(entry): input_callbacks += ' KEEP({}(.sm.{}.callbacks))\n'.format(o_file, sm) input_callbacks += ' . = ALIGN(2);' - # Table of connection keys - io_keys = '' + # Table of connections + num_connections = '' + io_connections = '' if len(ios) > 0: - io_keys += '__sm_{}_io_keys = .;\n'.format(sm) - io_keys += ' . += {};\n'.format(len(ios) * KEY_SIZE) - io_keys += ' . = ALIGN(2);' - - # Nonce used by outputs - outputs_nonce = '' - - if len(outputs) > 0: - outputs_nonce += '__sm_{}_output_nonce = .;\n'.format(sm) - outputs_nonce += ' . += 2;\n' - outputs_nonce += ' . = ALIGN(2);' + num_connections += '__sm_{}_num_connections = .;\n'.format(sm) + num_connections += ' . += 2;\n' + num_connections += ' . = ALIGN(2);' + io_connections += '__sm_{}_io_connections = .;\n'.format(sm) + io_connections += ' . += {};\n'.format(args.num_connections * (6 + KEY_SIZE)) + io_connections += ' . = ALIGN(2);' # Set data section offset if peripheral access is set # in that case, optionally provide first SM with exclusive access to last peripheral @@ -859,10 +875,10 @@ def sort_key(entry): exit_file, '\n '.join(tables), input_callbacks, '\n '.join(extra_labels))) - + data_sections.append(data_section.format(sm, '\n '.join(id_syms), - args.sm_stack_size, io_keys, - outputs_nonce, + args.sm_stack_size, num_connections, + io_connections, data_section_start)) if sm in sms_entries: @@ -880,7 +896,7 @@ def sort_key(entry): symbols.append('__sm_{}_io_{}_idx = {};'.format(sm, io, index)) # Add symbols for the number of connections/inputs - symbols.append('__sm_{}_num_connections = {};'.format(sm, len(ios))) + symbols.append('__sm_{}_max_connections = {};'.format(sm, args.num_connections)) symbols.append('__sm_{}_num_inputs = {};'.format(sm, len(inputs))) if args.prepare_for_sm_text_section_wrapping: diff --git a/src/sancus_support/reactive.h b/src/sancus_support/reactive.h index db60b61..d455113 100644 --- a/src/sancus_support/reactive.h +++ b/src/sancus_support/reactive.h @@ -6,6 +6,7 @@ #include typedef uint16_t io_index; +typedef uint16_t conn_index; typedef uint8_t io_data __attribute__((aligned(2))); // The ASM symbols are used for the linker to be able to detect inputs/outputs diff --git a/src/stubs/CMakeLists.txt b/src/stubs/CMakeLists.txt index 9427a17..9172a8e 100644 --- a/src/stubs/CMakeLists.txt +++ b/src/stubs/CMakeLists.txt @@ -23,6 +23,7 @@ set(EXTRA_FLAGS -I${CMAKE_SOURCE_DIR}/src/sancus_support) add_object(sm_output.o sm_output.c ${EXTRA_FLAGS}) add_object(sm_input.o sm_input.c ${EXTRA_FLAGS}) add_object(sm_set_key.o sm_set_key.c ${EXTRA_FLAGS}) +add_object(sm_attest.o sm_attest.c ${EXTRA_FLAGS}) set(STUBS ${CMAKE_CURRENT_BINARY_DIR}/sm_entry.o @@ -34,6 +35,7 @@ set(STUBS ${CMAKE_CURRENT_BINARY_DIR}/sm_output.o ${CMAKE_CURRENT_BINARY_DIR}/sm_input.o ${CMAKE_CURRENT_BINARY_DIR}/sm_set_key.o + ${CMAKE_CURRENT_BINARY_DIR}/sm_attest.o ${CMAKE_CURRENT_BINARY_DIR}/sm_mmio_entry.o ${CMAKE_CURRENT_BINARY_DIR}/sm_mmio_exclusive.o diff --git a/src/stubs/reactive_stubs_support.h b/src/stubs/reactive_stubs_support.h index c5aca41..70097a3 100644 --- a/src/stubs/reactive_stubs_support.h +++ b/src/stubs/reactive_stubs_support.h @@ -3,7 +3,7 @@ #include "reactive.h" -void reactive_handle_output(io_index output_id, void* data, size_t len); +void reactive_handle_output(conn_index conn_id, void* data, size_t len); typedef uint8_t IoKey[SANCUS_KEY_SIZE]; typedef void (*InputCallback)(const void*, size_t); @@ -12,16 +12,25 @@ typedef enum { Ok = 0x0, IllegalConnection = 0x1, - MalformedPayload = 0x2 + MalformedPayload = 0x2, + InternalError = 0x3 } ResultCode; +typedef struct Connection { + io_index io_id; + conn_index conn_id; + uint16_t nonce; + IoKey key; +} Connection; + // These will be allocated by the linker -extern IoKey __sm_io_keys[]; +extern Connection __sm_io_connections[]; extern InputCallback __sm_input_callbacks[]; -extern uint16_t __sm_output_nonce; -extern char __sm_num_connections; -#define SM_NUM_CONNECTIONS (size_t)&__sm_num_connections +extern char __sm_max_connections; +#define SM_MAX_CONNECTIONS (size_t)&__sm_max_connections + +extern uint16_t __sm_num_connections; extern char __sm_num_inputs; #define SM_NUM_INPUTS (size_t)&__sm_num_inputs diff --git a/src/stubs/sm_attest.c b/src/stubs/sm_attest.c new file mode 100644 index 0000000..a915dcb --- /dev/null +++ b/src/stubs/sm_attest.c @@ -0,0 +1,6 @@ +#include "reactive_stubs_support.h" + +void SM_ENTRY(SM_NAME) __sm_attest(const uint8_t* challenge, size_t len, uint8_t *result) +{ + sancus_tag(challenge, len, result); +} diff --git a/src/stubs/sm_input.c b/src/stubs/sm_input.c index 992d2e6..e582ac8 100644 --- a/src/stubs/sm_input.c +++ b/src/stubs/sm_input.c @@ -1,25 +1,61 @@ #include "reactive_stubs_support.h" #include +#include -#define AD_SIZE 2 - -void SM_ENTRY(SM_NAME) __sm_handle_input(uint16_t conn_id, +void SM_ENTRY(SM_NAME) __sm_handle_input(conn_index conn_id, const void* payload, size_t len) { - if (conn_id >= SM_NUM_INPUTS) - return; + // search for the connection. + // Unfortunately, this operation is O(n) with n = number of connections + int i; + Connection *conn = NULL; + for (i=0; i<__sm_num_connections; i++) { + conn = &__sm_io_connections[i]; + + if(conn->conn_id == conn_id) + break; + } - const size_t data_len = len - AD_SIZE - SANCUS_TAG_SIZE; - const uint8_t* cipher = (uint8_t*)payload + AD_SIZE; - const uint8_t* tag = cipher + data_len; + if (i == __sm_num_connections) + return; // connection not found - // TODO check for stack overflow! - uint8_t* input_buffer = alloca(data_len); + if (conn->io_id >= SM_NUM_INPUTS) + return; - if (sancus_unwrap_with_key(__sm_io_keys[conn_id], payload, AD_SIZE, - cipher, data_len, tag, input_buffer)) - { - __sm_input_callbacks[conn_id](input_buffer, data_len); + // associated data only contains the nonce, therefore we can use this + // this trick to build the array fastly (i.e. by swapping the bytes) + const uint16_t nonce_rev = conn->nonce << 8 | conn->nonce >> 8; + const size_t data_len = len - SANCUS_TAG_SIZE; + + if(data_len == 0) { + // In this case, sancus_unwrap would always fail due to some bug + // therefore we just check if the tag is correct. + const uint8_t *tag = payload; + uint8_t expected_tag[SANCUS_TAG_SIZE]; + sancus_tag_with_key(conn->key, &nonce_rev, sizeof(nonce_rev), expected_tag); + int success = 1, i; + for(i=0; inonce++; + __sm_input_callbacks[conn->io_id](NULL, 0); + } + } + else { + const uint8_t* cipher = payload; + const uint8_t* tag = cipher + data_len; + // TODO check for stack overflow! + uint8_t* input_buffer = alloca(data_len); + if (sancus_unwrap_with_key(conn->key, &nonce_rev, sizeof(nonce_rev), + cipher, data_len, tag, input_buffer)) { + conn->nonce++; + __sm_input_callbacks[conn->io_id](input_buffer, data_len); + } } } diff --git a/src/stubs/sm_output.c b/src/stubs/sm_output.c index 6febf29..2e59b2d 100644 --- a/src/stubs/sm_output.c +++ b/src/stubs/sm_output.c @@ -5,11 +5,26 @@ SM_FUNC(SM_NAME) void __sm_send_output(io_index index, const void* data, size_t len) { - const size_t nonce_len = sizeof(__sm_output_nonce); - const size_t payload_len = nonce_len + len + SANCUS_TAG_SIZE; - uint8_t* payload = malloc(payload_len); - *(uint16_t*)payload = __sm_output_nonce++; - sancus_wrap_with_key(__sm_io_keys[index], payload, nonce_len, data, len, - payload + nonce_len, payload + nonce_len + len); - reactive_handle_output(index, payload, payload_len); + const size_t payload_len = len + SANCUS_TAG_SIZE; + + // search for all the connections associated to the index. + // Unfortunately, this operation is O(n) with n = number of connections + int i; + for (i=0; i<__sm_num_connections; i++) { + Connection *conn = &__sm_io_connections[i]; + if(conn->io_id != index) + continue; + + uint8_t* payload = malloc(payload_len); + if (payload == NULL) + continue; + + // associated data only contains the nonce, therefore we can use this + // this trick to build the array fastly (i.e. by swapping the bytes) + uint16_t nonce_rev = conn->nonce << 8 | conn->nonce >> 8; + sancus_wrap_with_key(conn->key, &nonce_rev, sizeof(nonce_rev), data, + len, payload, payload + len); + conn->nonce++; + reactive_handle_output(conn->conn_id, payload, payload_len); + } } diff --git a/src/stubs/sm_set_key.c b/src/stubs/sm_set_key.c index e982d4c..9b211b7 100644 --- a/src/stubs/sm_set_key.c +++ b/src/stubs/sm_set_key.c @@ -1,21 +1,30 @@ #include "reactive_stubs_support.h" -void SM_ENTRY(SM_NAME) __sm_set_key(const uint8_t* ad, const uint8_t* cipher, - const uint8_t* tag, uint8_t* result) +uint16_t SM_ENTRY(SM_NAME) __sm_set_key(const uint8_t* ad, const uint8_t* cipher, + const uint8_t* tag) { - uint16_t conn_id = (ad[2] << 8) | ad[3]; - ResultCode code = Ok; + conn_index conn_id = (ad[0] << 8) | ad[1]; + io_index io_id = (ad[2] << 8) | ad[3]; + uint16_t nonce = (ad[4] << 8) | ad[5]; - if (conn_id >= SM_NUM_CONNECTIONS) - code = IllegalConnection; - else if (!sancus_unwrap(ad, 4, cipher, SANCUS_KEY_SIZE, tag, - __sm_io_keys[conn_id])) - { - code = MalformedPayload; + if (__sm_num_connections == SM_MAX_CONNECTIONS) { + return 1; } - result[0] = 0; - result[1] = code; - uint8_t result_ad[] = {ad[0], ad[1], result[0], result[1]}; - sancus_tag(result_ad, sizeof(result_ad), result + 2); + if(nonce != __sm_num_connections) { + return 2; + } + + Connection *conn = &__sm_io_connections[__sm_num_connections]; + + if (!sancus_unwrap(ad, 6, cipher, SANCUS_KEY_SIZE, tag, conn->key)) { + return 3; + } + + __sm_num_connections++; + conn->io_id = io_id; + conn->conn_id = conn_id; + conn->nonce = 0; + + return 0; } From c05544c13a1b7fb59d9fef7e4690f1928add221d Mon Sep 17 00:00:00 2001 From: Gianluca Scopelliti Date: Thu, 30 Sep 2021 14:37:22 +0200 Subject: [PATCH 03/27] add assertion for the Connection struct, add global var in linker.py --- src/drivers/linker.py | 3 ++- src/stubs/reactive_stubs_support.h | 8 ++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/src/drivers/linker.py b/src/drivers/linker.py index 8b269d2..5c9f2cc 100644 --- a/src/drivers/linker.py +++ b/src/drivers/linker.py @@ -17,6 +17,7 @@ MAC_SIZE = int(sancus.config.SECURITY / 8) KEY_SIZE = MAC_SIZE +CONNECTION_STRUCT_SIZE = 6 + KEY_SIZE class SmEntry: @@ -861,7 +862,7 @@ def sort_key(entry): num_connections += ' . += 2;\n' num_connections += ' . = ALIGN(2);' io_connections += '__sm_{}_io_connections = .;\n'.format(sm) - io_connections += ' . += {};\n'.format(args.num_connections * (6 + KEY_SIZE)) + io_connections += ' . += {};\n'.format(args.num_connections * CONNECTION_STRUCT_SIZE) io_connections += ' . = ALIGN(2);' # Set data section offset if peripheral access is set diff --git a/src/stubs/reactive_stubs_support.h b/src/stubs/reactive_stubs_support.h index 70097a3..84b548a 100644 --- a/src/stubs/reactive_stubs_support.h +++ b/src/stubs/reactive_stubs_support.h @@ -23,6 +23,14 @@ typedef struct Connection { IoKey key; } Connection; +// The size of the Connection struct is also hardcoded in linker.py. Hence, +// we need to make sure that it does not change at compile time (e.g. due to +// optimizations). +// Besides, if the struct changes, we need to adjust this value here and in +// linker.py (check the CONNECTION_STRUCT_SIZE global variable) as well. +_Static_assert (sizeof(Connection) == 6 + SANCUS_KEY_SIZE, + "Size of Connection struct differs from the expected value"); + // These will be allocated by the linker extern Connection __sm_io_connections[]; extern InputCallback __sm_input_callbacks[]; From 62d2bf3284d40ad59ad46815648d551c68abbb12 Mon Sep 17 00:00:00 2001 From: Gianluca Scopelliti Date: Thu, 30 Sep 2021 15:28:18 +0200 Subject: [PATCH 04/27] fix sancus_is_outside_sm --- src/sancus_support/sm_support.h | 36 +++++++++++++++++++++++++++------ 1 file changed, 30 insertions(+), 6 deletions(-) diff --git a/src/sancus_support/sm_support.h b/src/sancus_support/sm_support.h index 9881973..7687018 100644 --- a/src/sancus_support/sm_support.h +++ b/src/sancus_support/sm_support.h @@ -199,15 +199,12 @@ extern char __unprotected_sp; #endif -#define __OUTSIDE_SM( p, sm ) \ - ( ((void*) p < (void*) &__PS(sm)) || ((void*) p >= (void*) &__PE(sm)) ) && \ - ( ((void*) p < (void*) &__SS(sm)) || ((void*) p >= (void*) &__SE(sm)) ) - /* * Returns true iff whole buffer [p,p+len-1] is outside of the sm SancusModule */ -#define sancus_is_outside_sm( sm, p, len) \ - ( __OUTSIDE_SM(p, sm) && __OUTSIDE_SM((p+len-1), sm) ) +#define sancus_is_outside_sm(sm, p, len) \ + ( is_buffer_outside_region(&__PS(sm), &__PE(sm), p, len) && \ + is_buffer_outside_region(&__SS(sm), &__SE(sm), p, len) ) /** * Interrupt vector for the Sancus violation ISR. @@ -311,6 +308,33 @@ sm_id sancus_enable_wrapped(struct SancusModule* sm, unsigned nonce, void* tag); #undef always_inline #define always_inline static inline __attribute__((always_inline)) +/* + * Returns true if buf is outside the memory region [start, end) + * if start >= end, immediately return false + */ +always_inline int is_buffer_outside_region(void *start, void *end, + void *buf, size_t len) { + void *buf_end; + + // make sure start < end, otherwise return false + if (start >= end) { + return 0; + } + + if(len > 0) { + buf_end = buf + len - 1; + } + else { + buf_end = buf; + } + + if( (end <= buf) || (start > buf_end) ) { + return 1; + } + + return 0; +} + /** * Performs constant time comparison between to buffers * Returns 0 if the first `n` bytes of the two buffers are equal, -1 otherwise From 1b1f009c9383f249151b5469c1da200acb917f0b Mon Sep 17 00:00:00 2001 From: Gianluca Scopelliti Date: Thu, 30 Sep 2021 16:46:23 +0200 Subject: [PATCH 05/27] sm_input: O(1) lookup of conn using index returned by set_key --- src/stubs/sm_input.c | 17 ++++------------- src/stubs/sm_set_key.c | 3 ++- 2 files changed, 6 insertions(+), 14 deletions(-) diff --git a/src/stubs/sm_input.c b/src/stubs/sm_input.c index e582ac8..7bbedb5 100644 --- a/src/stubs/sm_input.c +++ b/src/stubs/sm_input.c @@ -3,22 +3,13 @@ #include #include -void SM_ENTRY(SM_NAME) __sm_handle_input(conn_index conn_id, +void SM_ENTRY(SM_NAME) __sm_handle_input(uint16_t conn_idx, const void* payload, size_t len) { - // search for the connection. - // Unfortunately, this operation is O(n) with n = number of connections - int i; - Connection *conn = NULL; - for (i=0; i<__sm_num_connections; i++) { - conn = &__sm_io_connections[i]; + if(conn_idx >= __sm_num_connections) + return; // bad ID given by caller - if(conn->conn_id == conn_id) - break; - } - - if (i == __sm_num_connections) - return; // connection not found + Connection *conn = &__sm_io_connections[conn_idx]; if (conn->io_id >= SM_NUM_INPUTS) return; diff --git a/src/stubs/sm_set_key.c b/src/stubs/sm_set_key.c index 9b211b7..7bbc078 100644 --- a/src/stubs/sm_set_key.c +++ b/src/stubs/sm_set_key.c @@ -1,7 +1,7 @@ #include "reactive_stubs_support.h" uint16_t SM_ENTRY(SM_NAME) __sm_set_key(const uint8_t* ad, const uint8_t* cipher, - const uint8_t* tag) + const uint8_t* tag, uint16_t *conn_idx) { conn_index conn_id = (ad[0] << 8) | ad[1]; io_index io_id = (ad[2] << 8) | ad[3]; @@ -16,6 +16,7 @@ uint16_t SM_ENTRY(SM_NAME) __sm_set_key(const uint8_t* ad, const uint8_t* cipher } Connection *conn = &__sm_io_connections[__sm_num_connections]; + *conn_idx = __sm_num_connections; if (!sancus_unwrap(ad, 6, cipher, SANCUS_KEY_SIZE, tag, conn->key)) { return 3; From 967f06a0442c8457666dc75212593b416db8605c Mon Sep 17 00:00:00 2001 From: Jo Van Bulck Date: Thu, 30 Sep 2021 18:18:31 +0200 Subject: [PATCH 06/27] sancus_is_outside_sm: check for int overflow closes #30 --- src/sancus_support/sm_support.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/sancus_support/sm_support.h b/src/sancus_support/sm_support.h index 7687018..8b368b6 100644 --- a/src/sancus_support/sm_support.h +++ b/src/sancus_support/sm_support.h @@ -328,7 +328,8 @@ always_inline int is_buffer_outside_region(void *start, void *end, buf_end = buf; } - if( (end <= buf) || (start > buf_end) ) { + /* check for int overflow and finally validate `buf` falls outside */ + if( (buf <= buf_end) && (end <= buf) || (start > buf_end) ) { return 1; } From 588605d198af008fdde05e0071bb77bdf559ff6b Mon Sep 17 00:00:00 2001 From: Jo Van Bulck Date: Fri, 1 Oct 2021 11:08:58 +0200 Subject: [PATCH 07/27] Fix brackets sancus_is_outside_sm --- src/sancus_support/sm_support.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sancus_support/sm_support.h b/src/sancus_support/sm_support.h index 8b368b6..c7d05e5 100644 --- a/src/sancus_support/sm_support.h +++ b/src/sancus_support/sm_support.h @@ -329,7 +329,7 @@ always_inline int is_buffer_outside_region(void *start, void *end, } /* check for int overflow and finally validate `buf` falls outside */ - if( (buf <= buf_end) && (end <= buf) || (start > buf_end) ) { + if( (buf <= buf_end) && ((end <= buf) || (start > buf_end))) { return 1; } From 2fc4ce62944231dfdefcb335e46040b236b3589a Mon Sep 17 00:00:00 2001 From: Gianluca Scopelliti Date: Fri, 1 Oct 2021 11:57:07 +0200 Subject: [PATCH 08/27] declare extern vars for the public/secret regions, sanitize sm_attest() --- src/drivers/linker.py | 6 +++++- src/stubs/reactive_stubs_support.h | 4 ++++ src/stubs/sm_attest.c | 5 +++++ 3 files changed, 14 insertions(+), 1 deletion(-) diff --git a/src/drivers/linker.py b/src/drivers/linker.py index 5c9f2cc..abebcca 100644 --- a/src/drivers/linker.py +++ b/src/drivers/linker.py @@ -105,7 +105,11 @@ def get_io_sym_map(sm_name): '__sm_X_exit': '__sm_{}_exit'.format(sm_name), '__sm_X_stub_malloc': '__sm_{}_stub_malloc'.format(sm_name), '__sm_X_stub_reactive_handle_output': - '__sm_{}_stub_reactive_handle_output'.format(sm_name) + '__sm_{}_stub_reactive_handle_output'.format(sm_name), + '__sm_X_public_start': '__sm_{}_public_start'.format(sm_name), + '__sm_X_public_end': '__sm_{}_public_end'.format(sm_name), + '__sm_X_secret_start': '__sm_{}_secret_start'.format(sm_name), + '__sm_X_secret_end': '__sm_{}_secret_end'.format(sm_name) } return sym_map diff --git a/src/stubs/reactive_stubs_support.h b/src/stubs/reactive_stubs_support.h index 84b548a..d21230a 100644 --- a/src/stubs/reactive_stubs_support.h +++ b/src/stubs/reactive_stubs_support.h @@ -45,4 +45,8 @@ extern char __sm_num_inputs; #define SM_NAME X +// declare symbols for the public/secret regions +extern void *__sm_X_public_start, *__sm_X_public_end, + *__sm_X_secret_start, *__sm_X_secret_end; + #endif diff --git a/src/stubs/sm_attest.c b/src/stubs/sm_attest.c index a915dcb..0fdd711 100644 --- a/src/stubs/sm_attest.c +++ b/src/stubs/sm_attest.c @@ -2,5 +2,10 @@ void SM_ENTRY(SM_NAME) __sm_attest(const uint8_t* challenge, size_t len, uint8_t *result) { + if( !sancus_is_outside_sm(SM_NAME, (void *) challenge, len) || + !sancus_is_outside_sm(SM_NAME, (void *) result, SANCUS_TAG_SIZE) ) { + return; + } + sancus_tag(challenge, len, result); } From 7d29fb20bb39ec6500d8aab668050ac3d0105480 Mon Sep 17 00:00:00 2001 From: Gianluca Scopelliti Date: Fri, 1 Oct 2021 12:24:48 +0200 Subject: [PATCH 09/27] sanitize inputs of __sm_set_key and __sm_handle_input --- src/stubs/sm_input.c | 4 ++++ src/stubs/sm_set_key.c | 18 ++++++++++++++---- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/src/stubs/sm_input.c b/src/stubs/sm_input.c index 7bbedb5..f7e14e7 100644 --- a/src/stubs/sm_input.c +++ b/src/stubs/sm_input.c @@ -6,6 +6,10 @@ void SM_ENTRY(SM_NAME) __sm_handle_input(uint16_t conn_idx, const void* payload, size_t len) { + if( !sancus_is_outside_sm(SM_NAME, (void *) payload, len)) { + return; + } + if(conn_idx >= __sm_num_connections) return; // bad ID given by caller diff --git a/src/stubs/sm_set_key.c b/src/stubs/sm_set_key.c index 7bbc078..2d61889 100644 --- a/src/stubs/sm_set_key.c +++ b/src/stubs/sm_set_key.c @@ -1,25 +1,35 @@ #include "reactive_stubs_support.h" +#define AD_SIZE 6 + uint16_t SM_ENTRY(SM_NAME) __sm_set_key(const uint8_t* ad, const uint8_t* cipher, const uint8_t* tag, uint16_t *conn_idx) { + if( !sancus_is_outside_sm(SM_NAME, (void *) ad, AD_SIZE) || + !sancus_is_outside_sm(SM_NAME, (void *) cipher, SANCUS_KEY_SIZE) || + !sancus_is_outside_sm(SM_NAME, (void *) tag, SANCUS_TAG_SIZE) || + !sancus_is_outside_sm(SM_NAME, (void *) conn_idx, sizeof(uint16_t)) ) { + return 1; + } + + // Note: make sure we only use AD_SIZE bytes of the buffer `ad` conn_index conn_id = (ad[0] << 8) | ad[1]; io_index io_id = (ad[2] << 8) | ad[3]; uint16_t nonce = (ad[4] << 8) | ad[5]; if (__sm_num_connections == SM_MAX_CONNECTIONS) { - return 1; + return 2; } if(nonce != __sm_num_connections) { - return 2; + return 3; } Connection *conn = &__sm_io_connections[__sm_num_connections]; *conn_idx = __sm_num_connections; - if (!sancus_unwrap(ad, 6, cipher, SANCUS_KEY_SIZE, tag, conn->key)) { - return 3; + if (!sancus_unwrap(ad, AD_SIZE, cipher, SANCUS_KEY_SIZE, tag, conn->key)) { + return 4; } __sm_num_connections++; From 05546931c8bcc35dc1fae0a3a5d537f39580aee0 Mon Sep 17 00:00:00 2001 From: Jo Van Bulck Date: Fri, 1 Oct 2021 13:07:27 +0200 Subject: [PATCH 10/27] reactive: declare symbols for the public/secret regions --- src/stubs/reactive_stubs_support.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/stubs/reactive_stubs_support.h b/src/stubs/reactive_stubs_support.h index d21230a..1ad5464 100644 --- a/src/stubs/reactive_stubs_support.h +++ b/src/stubs/reactive_stubs_support.h @@ -46,7 +46,6 @@ extern char __sm_num_inputs; #define SM_NAME X // declare symbols for the public/secret regions -extern void *__sm_X_public_start, *__sm_X_public_end, - *__sm_X_secret_start, *__sm_X_secret_end; +extern char __PS(SM_NAME), __PE(SM_NAME), __SS(SM_NAME), __SE(SM_NAME); #endif From bb6edf22c2f3be80d31b2c15cb04bb1087016a69 Mon Sep 17 00:00:00 2001 From: Gianluca Scopelliti Date: Tue, 5 Oct 2021 18:10:52 +0200 Subject: [PATCH 11/27] fix declaration of symbols for public/secret regions --- src/stubs/reactive_stubs_support.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/stubs/reactive_stubs_support.h b/src/stubs/reactive_stubs_support.h index 1ad5464..7a19db0 100644 --- a/src/stubs/reactive_stubs_support.h +++ b/src/stubs/reactive_stubs_support.h @@ -46,6 +46,8 @@ extern char __sm_num_inputs; #define SM_NAME X // declare symbols for the public/secret regions -extern char __PS(SM_NAME), __PE(SM_NAME), __SS(SM_NAME), __SE(SM_NAME); +#define __SECTION(sect, name) sect(name) +extern char __SECTION(__PS, SM_NAME), __SECTION(__PE, SM_NAME), + __SECTION(__SS, SM_NAME), __SECTION(__SE, SM_NAME); #endif From 09f2bfd2b21a0f48cbfcbfb7a292220b6d93415e Mon Sep 17 00:00:00 2001 From: Gianluca Scopelliti Date: Tue, 5 Oct 2021 19:29:25 +0200 Subject: [PATCH 12/27] __sm_handle_input: return a value to indicate success/error --- src/stubs/sm_input.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/stubs/sm_input.c b/src/stubs/sm_input.c index f7e14e7..1d62eeb 100644 --- a/src/stubs/sm_input.c +++ b/src/stubs/sm_input.c @@ -3,20 +3,20 @@ #include #include -void SM_ENTRY(SM_NAME) __sm_handle_input(uint16_t conn_idx, +uint16_t SM_ENTRY(SM_NAME) __sm_handle_input(uint16_t conn_idx, const void* payload, size_t len) { if( !sancus_is_outside_sm(SM_NAME, (void *) payload, len)) { - return; + return 1; } if(conn_idx >= __sm_num_connections) - return; // bad ID given by caller + return 2; // bad ID given by caller Connection *conn = &__sm_io_connections[conn_idx]; if (conn->io_id >= SM_NUM_INPUTS) - return; + return 3; // associated data only contains the nonce, therefore we can use this // this trick to build the array fastly (i.e. by swapping the bytes) @@ -53,4 +53,6 @@ void SM_ENTRY(SM_NAME) __sm_handle_input(uint16_t conn_idx, __sm_input_callbacks[conn->io_id](input_buffer, data_len); } } + + return 0; } From 6152f0e75fc1f5d282d51b90f038fcf0a58528ff Mon Sep 17 00:00:00 2001 From: Gianluca Scopelliti Date: Wed, 6 Oct 2021 11:45:33 +0200 Subject: [PATCH 13/27] __sm_attest: return a value to indicate success/error --- src/stubs/sm_attest.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/stubs/sm_attest.c b/src/stubs/sm_attest.c index 0fdd711..63314e7 100644 --- a/src/stubs/sm_attest.c +++ b/src/stubs/sm_attest.c @@ -1,11 +1,13 @@ #include "reactive_stubs_support.h" -void SM_ENTRY(SM_NAME) __sm_attest(const uint8_t* challenge, size_t len, uint8_t *result) +uint16_t SM_ENTRY(SM_NAME) __sm_attest(const uint8_t* challenge, size_t len, + uint8_t *result) { if( !sancus_is_outside_sm(SM_NAME, (void *) challenge, len) || !sancus_is_outside_sm(SM_NAME, (void *) result, SANCUS_TAG_SIZE) ) { - return; + return 1; } sancus_tag(challenge, len, result); + return 0; } From 6f8cb878c78b00c2d9ebcbc2413e2af73e98c186 Mon Sep 17 00:00:00 2001 From: Gianluca Scopelliti Date: Wed, 6 Oct 2021 18:06:56 +0200 Subject: [PATCH 14/27] simplify __sm_handle_input * moved the check on `io_id` to `__sm_set_key` * add check on `len`: it has to be >= `SANCUS_TAG_SIZE` * remove the condition on `data_len`: the workaround to handle length==0 should not be implemented here, but rather in the core or in sm_support.h --- src/stubs/sm_input.c | 55 ++++++++++++++---------------------------- src/stubs/sm_set_key.c | 4 ++- 2 files changed, 21 insertions(+), 38 deletions(-) diff --git a/src/stubs/sm_input.c b/src/stubs/sm_input.c index 1d62eeb..92eeda9 100644 --- a/src/stubs/sm_input.c +++ b/src/stubs/sm_input.c @@ -6,53 +6,34 @@ uint16_t SM_ENTRY(SM_NAME) __sm_handle_input(uint16_t conn_idx, const void* payload, size_t len) { - if( !sancus_is_outside_sm(SM_NAME, (void *) payload, len)) { + // sanitize input buffer + if(!sancus_is_outside_sm(SM_NAME, (void *) payload, len)) { return 1; } - if(conn_idx >= __sm_num_connections) - return 2; // bad ID given by caller + // check correctness of other parameters + if(len < SANCUS_TAG_SIZE || conn_idx >= __sm_num_connections) { + return 2; + } Connection *conn = &__sm_io_connections[conn_idx]; - if (conn->io_id >= SM_NUM_INPUTS) - return 3; - // associated data only contains the nonce, therefore we can use this // this trick to build the array fastly (i.e. by swapping the bytes) const uint16_t nonce_rev = conn->nonce << 8 | conn->nonce >> 8; const size_t data_len = len - SANCUS_TAG_SIZE; - - if(data_len == 0) { - // In this case, sancus_unwrap would always fail due to some bug - // therefore we just check if the tag is correct. - const uint8_t *tag = payload; - uint8_t expected_tag[SANCUS_TAG_SIZE]; - sancus_tag_with_key(conn->key, &nonce_rev, sizeof(nonce_rev), expected_tag); - int success = 1, i; - for(i=0; inonce++; - __sm_input_callbacks[conn->io_id](NULL, 0); - } - } - else { - const uint8_t* cipher = payload; - const uint8_t* tag = cipher + data_len; - // TODO check for stack overflow! - uint8_t* input_buffer = alloca(data_len); - if (sancus_unwrap_with_key(conn->key, &nonce_rev, sizeof(nonce_rev), - cipher, data_len, tag, input_buffer)) { - conn->nonce++; - __sm_input_callbacks[conn->io_id](input_buffer, data_len); - } + const uint8_t* cipher = payload; + const uint8_t* tag = cipher + data_len; + // TODO check for stack overflow! + uint8_t* input_buffer = alloca(data_len); + + if (sancus_unwrap_with_key(conn->key, &nonce_rev, sizeof(nonce_rev), + cipher, data_len, tag, input_buffer)) { + conn->nonce++; + __sm_input_callbacks[conn->io_id](input_buffer, data_len); + return 0; } - return 0; + // here only if decryption fails + return 4; } diff --git a/src/stubs/sm_set_key.c b/src/stubs/sm_set_key.c index 2d61889..b754c27 100644 --- a/src/stubs/sm_set_key.c +++ b/src/stubs/sm_set_key.c @@ -17,11 +17,13 @@ uint16_t SM_ENTRY(SM_NAME) __sm_set_key(const uint8_t* ad, const uint8_t* cipher io_index io_id = (ad[2] << 8) | ad[3]; uint16_t nonce = (ad[4] << 8) | ad[5]; + // check if there is still space left in the array if (__sm_num_connections == SM_MAX_CONNECTIONS) { return 2; } - if(nonce != __sm_num_connections) { + // check parameters + if(nonce != __sm_num_connections || io_id >= SM_NUM_INPUTS) { return 3; } From e8f5890c16de3cb8651477d63ee8361e6bfc80ea Mon Sep 17 00:00:00 2001 From: Gianluca Scopelliti Date: Wed, 6 Oct 2021 18:36:18 +0200 Subject: [PATCH 15/27] small fix return value in __sm_handle_input --- src/stubs/sm_input.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/stubs/sm_input.c b/src/stubs/sm_input.c index 92eeda9..55bd6da 100644 --- a/src/stubs/sm_input.c +++ b/src/stubs/sm_input.c @@ -35,5 +35,5 @@ uint16_t SM_ENTRY(SM_NAME) __sm_handle_input(uint16_t conn_idx, } // here only if decryption fails - return 4; + return 3; } From 3ae47745d3721792bbcf0cf7dfc9551823ceaf16 Mon Sep 17 00:00:00 2001 From: Gianluca Scopelliti Date: Fri, 15 Oct 2021 10:28:56 +0200 Subject: [PATCH 16/27] use config file for `num_connections` parameter --- src/drivers/linker.py | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/drivers/linker.py b/src/drivers/linker.py index abebcca..f124d1a 100644 --- a/src/drivers/linker.py +++ b/src/drivers/linker.py @@ -204,12 +204,6 @@ def sort_key(entry): '$PROJECT that is substituted for this Path in the linker. This allows projects ' 'to give the linker the correct path at compile time while still supporting a generic, ' 'project-dependent configuration file.') -parser.add_argument('--num-connections', - help='Parameter for Authentic Execution. Maximum number of ' - 'connections that the module can have. ' - 'This number impacts on the size of the module.', - type=int, - default=0) args, cli_ld_args = parser.parse_known_args() set_args(args) @@ -861,12 +855,17 @@ def sort_key(entry): num_connections = '' io_connections = '' + if hasattr(sm_config[sm], "num_connections"): + sm_num_connections = sm_config[sm].num_connections + else: + sm_num_connections = 0 + if len(ios) > 0: num_connections += '__sm_{}_num_connections = .;\n'.format(sm) num_connections += ' . += 2;\n' num_connections += ' . = ALIGN(2);' io_connections += '__sm_{}_io_connections = .;\n'.format(sm) - io_connections += ' . += {};\n'.format(args.num_connections * CONNECTION_STRUCT_SIZE) + io_connections += ' . += {};\n'.format(sm_num_connections * CONNECTION_STRUCT_SIZE) io_connections += ' . = ALIGN(2);' # Set data section offset if peripheral access is set @@ -901,7 +900,7 @@ def sort_key(entry): symbols.append('__sm_{}_io_{}_idx = {};'.format(sm, io, index)) # Add symbols for the number of connections/inputs - symbols.append('__sm_{}_max_connections = {};'.format(sm, args.num_connections)) + symbols.append('__sm_{}_max_connections = {};'.format(sm, sm_num_connections)) symbols.append('__sm_{}_num_inputs = {};'.format(sm, len(inputs))) if args.prepare_for_sm_text_section_wrapping: From ce713964b79be7cbd4082aafdd94da63e187646f Mon Sep 17 00:00:00 2001 From: Gianluca Scopelliti Date: Fri, 15 Oct 2021 12:27:49 +0200 Subject: [PATCH 17/27] add num_connections property in SmConfig --- src/drivers/sancus/sancus_config.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/drivers/sancus/sancus_config.py b/src/drivers/sancus/sancus_config.py index c9826e1..b29bf0c 100644 --- a/src/drivers/sancus/sancus_config.py +++ b/src/drivers/sancus/sancus_config.py @@ -12,6 +12,7 @@ SM_CONFIG_DEFAULT_SM_EXIT="" SM_CONFIG_DEFAULT_SM_ISR="" SM_CONFIG_DEFAULT_PERIPHERAL_OFFSET=0 +SM_CONFIG_DEFAULT_NUM_CONNECTIONS=None class SmParserError(Exception): """ @@ -125,6 +126,7 @@ def _safe_config_parse(name, default_val): self._sm_exit = _safe_config_parse('sm_exit', SM_CONFIG_DEFAULT_SM_EXIT) self._sm_isr = _safe_config_parse('sm_isr' , SM_CONFIG_DEFAULT_SM_ISR) self._peripheral_offset = _safe_config_parse('peripheral_offset', 0) + self._num_connections = _safe_config_parse('num_connections', SM_CONFIG_DEFAULT_NUM_CONNECTIONS) """ Getters as properties for all class variables. @@ -178,6 +180,13 @@ def peripheral_offset(self): else: raise AttributeError + @property + def num_connections(self): + if self._num_connections != SM_CONFIG_DEFAULT_NUM_CONNECTIONS: + return abs(self._num_connections) + else: + raise AttributeError + def __str__(self): if self._contains_config: s = '' + self._name + ':\n' @@ -187,6 +196,7 @@ def __str__(self): if hasattr(self, 'sm_exit'): s += '\t\t SM_EXIT: ' + self._sm_exit + '\n' if hasattr(self, 'sm_isr'): s += '\t\t SM_ISR: ' + self._sm_isr + '\n' if hasattr(self, 'peripheral_offset'): s += '\t\t Peripheral Offset: ' + str(self._peripheral_offset) + '\n' + if hasattr(self, 'num_connections'): s += '\t\t Number of connections: ' + str(self._num_connections) + '\n' else: s = 'No YAML config provided.' return s \ No newline at end of file From 5621e279e5447ce09e04404494ba886b806c7182 Mon Sep 17 00:00:00 2001 From: Gianluca Scopelliti Date: Fri, 15 Oct 2021 13:44:02 +0200 Subject: [PATCH 18/27] add comments in linker.py to explain the connection array --- src/drivers/linker.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/drivers/linker.py b/src/drivers/linker.py index f124d1a..ba52c07 100644 --- a/src/drivers/linker.py +++ b/src/drivers/linker.py @@ -851,7 +851,15 @@ def sort_key(entry): input_callbacks += ' KEEP({}(.sm.{}.callbacks))\n'.format(o_file, sm) input_callbacks += ' . = ALIGN(2);' - # Table of connections + """ + Table of connections: in a reactive application, a connection links the + output of a SM (defined using the macro `SM_OUTPUT`) to the input of another + (defined using the macro `SM_INPUT`). + These connections are stored in a `Connection` array on each SM (see + `reactive_stubs_support.h`). The array is allocated here with a fixed size, + according to the `num_connections` parameter in the SM config (default 0). + """ + num_connections = '' io_connections = '' From 16ae05c792f82b722af5fbb71def97bfe6859de4 Mon Sep 17 00:00:00 2001 From: Gianluca Scopelliti Date: Fri, 15 Oct 2021 16:56:38 +0200 Subject: [PATCH 19/27] add num_connections parameter in sm-config-example.yaml --- src/drivers/sm-config-example.yaml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/drivers/sm-config-example.yaml b/src/drivers/sm-config-example.yaml index 4c82eed..b2a8d13 100644 --- a/src/drivers/sm-config-example.yaml +++ b/src/drivers/sm-config-example.yaml @@ -23,6 +23,13 @@ my_sm_1: # amount of byte. This allows to map the data section over the # peripherals to grant exclusive and faster access to these peripherals - peripheral_offset: 159 + # Set the maximum number of connections allowed for this SM in a reactive + # application. Connections link the output of a SM (defined using the macro + # `SM_OUTPUT`) with the input of another (defined using the macro + # `SM_INPUT`). If this SM does not have any inputs nor outputs, there is no + # need to specify this parameter (default value is zero). In the Authentic + # Execution framework, this parameter is automatically configured. + - num_connections: 3 my_sm_2: - disallow_outcalls: True \ No newline at end of file From b50bb02a64c00c2c10d4546ffe699897f649c530 Mon Sep 17 00:00:00 2001 From: Gianluca Scopelliti Date: Wed, 20 Oct 2021 10:27:29 +0200 Subject: [PATCH 20/27] move io_id check from set_key to handle_input in set_key, io_id might as well be an _output_ identifier, so it was wrong to add that check here. --- src/stubs/sm_input.c | 7 ++++++- src/stubs/sm_set_key.c | 4 ++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/stubs/sm_input.c b/src/stubs/sm_input.c index 55bd6da..a9903c6 100644 --- a/src/stubs/sm_input.c +++ b/src/stubs/sm_input.c @@ -18,6 +18,11 @@ uint16_t SM_ENTRY(SM_NAME) __sm_handle_input(uint16_t conn_idx, Connection *conn = &__sm_io_connections[conn_idx]; + // check if io_id is a valid input ID + if (conn->io_id >= SM_NUM_INPUTS) { + return 3; + } + // associated data only contains the nonce, therefore we can use this // this trick to build the array fastly (i.e. by swapping the bytes) const uint16_t nonce_rev = conn->nonce << 8 | conn->nonce >> 8; @@ -35,5 +40,5 @@ uint16_t SM_ENTRY(SM_NAME) __sm_handle_input(uint16_t conn_idx, } // here only if decryption fails - return 3; + return 4; } diff --git a/src/stubs/sm_set_key.c b/src/stubs/sm_set_key.c index b754c27..ea0158b 100644 --- a/src/stubs/sm_set_key.c +++ b/src/stubs/sm_set_key.c @@ -22,8 +22,8 @@ uint16_t SM_ENTRY(SM_NAME) __sm_set_key(const uint8_t* ad, const uint8_t* cipher return 2; } - // check parameters - if(nonce != __sm_num_connections || io_id >= SM_NUM_INPUTS) { + // check nonce + if(nonce != __sm_num_connections) { return 3; } From 0a81ca0f43b57e69b22e20ecf3fa2d40b29fe9d5 Mon Sep 17 00:00:00 2001 From: Gianluca Scopelliti Date: Wed, 20 Oct 2021 14:44:12 +0200 Subject: [PATCH 21/27] fix sm_{}_io_connections buffer allocation if num_connections is zero --- src/drivers/linker.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/drivers/linker.py b/src/drivers/linker.py index ba52c07..c369e43 100644 --- a/src/drivers/linker.py +++ b/src/drivers/linker.py @@ -869,11 +869,14 @@ def sort_key(entry): sm_num_connections = 0 if len(ios) > 0: + # make sure we allocate space even if num_connections is zero + io_connections_size = max(sm_num_connections * CONNECTION_STRUCT_SIZE, 2) + num_connections += '__sm_{}_num_connections = .;\n'.format(sm) num_connections += ' . += 2;\n' num_connections += ' . = ALIGN(2);' io_connections += '__sm_{}_io_connections = .;\n'.format(sm) - io_connections += ' . += {};\n'.format(sm_num_connections * CONNECTION_STRUCT_SIZE) + io_connections += ' . += {};\n'.format(io_connections_size) io_connections += ' . = ALIGN(2);' # Set data section offset if peripheral access is set From 7ee6635c6a81679e72979d5394c900f962838c01 Mon Sep 17 00:00:00 2001 From: Gianluca Scopelliti Date: Thu, 21 Oct 2021 10:21:21 +0200 Subject: [PATCH 22/27] return values of stubs as ResultCode values --- src/stubs/reactive_stubs_support.h | 13 ++++++++----- src/stubs/sm_attest.c | 9 ++++++--- src/stubs/sm_input.c | 10 +++++----- src/stubs/sm_set_key.c | 10 +++++----- 4 files changed, 24 insertions(+), 18 deletions(-) diff --git a/src/stubs/reactive_stubs_support.h b/src/stubs/reactive_stubs_support.h index 7a19db0..3c7a251 100644 --- a/src/stubs/reactive_stubs_support.h +++ b/src/stubs/reactive_stubs_support.h @@ -10,10 +10,13 @@ typedef void (*InputCallback)(const void*, size_t); typedef enum { - Ok = 0x0, - IllegalConnection = 0x1, - MalformedPayload = 0x2, - InternalError = 0x3 + Ok = 0x0, + IllegalConnection = 0x1, + MalformedPayload = 0x2, + IllegalParameters = 0x3, + BufferInsideSM = 0x4, + CryptoError = 0x5, + InternalError = 0x6 } ResultCode; typedef struct Connection { @@ -47,7 +50,7 @@ extern char __sm_num_inputs; // declare symbols for the public/secret regions #define __SECTION(sect, name) sect(name) -extern char __SECTION(__PS, SM_NAME), __SECTION(__PE, SM_NAME), +extern char __SECTION(__PS, SM_NAME), __SECTION(__PE, SM_NAME), __SECTION(__SS, SM_NAME), __SECTION(__SE, SM_NAME); #endif diff --git a/src/stubs/sm_attest.c b/src/stubs/sm_attest.c index 63314e7..6bfec25 100644 --- a/src/stubs/sm_attest.c +++ b/src/stubs/sm_attest.c @@ -5,9 +5,12 @@ uint16_t SM_ENTRY(SM_NAME) __sm_attest(const uint8_t* challenge, size_t len, { if( !sancus_is_outside_sm(SM_NAME, (void *) challenge, len) || !sancus_is_outside_sm(SM_NAME, (void *) result, SANCUS_TAG_SIZE) ) { - return 1; + return BufferInsideSM; } - sancus_tag(challenge, len, result); - return 0; + if( !sancus_tag(challenge, len, result) ) { + return CryptoError; + } + + return Ok; } diff --git a/src/stubs/sm_input.c b/src/stubs/sm_input.c index a9903c6..417b807 100644 --- a/src/stubs/sm_input.c +++ b/src/stubs/sm_input.c @@ -8,19 +8,19 @@ uint16_t SM_ENTRY(SM_NAME) __sm_handle_input(uint16_t conn_idx, { // sanitize input buffer if(!sancus_is_outside_sm(SM_NAME, (void *) payload, len)) { - return 1; + return BufferInsideSM; } // check correctness of other parameters if(len < SANCUS_TAG_SIZE || conn_idx >= __sm_num_connections) { - return 2; + return IllegalParameters; } Connection *conn = &__sm_io_connections[conn_idx]; // check if io_id is a valid input ID if (conn->io_id >= SM_NUM_INPUTS) { - return 3; + return IllegalConnection; } // associated data only contains the nonce, therefore we can use this @@ -36,9 +36,9 @@ uint16_t SM_ENTRY(SM_NAME) __sm_handle_input(uint16_t conn_idx, cipher, data_len, tag, input_buffer)) { conn->nonce++; __sm_input_callbacks[conn->io_id](input_buffer, data_len); - return 0; + return Ok; } // here only if decryption fails - return 4; + return CryptoError; } diff --git a/src/stubs/sm_set_key.c b/src/stubs/sm_set_key.c index ea0158b..7694958 100644 --- a/src/stubs/sm_set_key.c +++ b/src/stubs/sm_set_key.c @@ -9,7 +9,7 @@ uint16_t SM_ENTRY(SM_NAME) __sm_set_key(const uint8_t* ad, const uint8_t* cipher !sancus_is_outside_sm(SM_NAME, (void *) cipher, SANCUS_KEY_SIZE) || !sancus_is_outside_sm(SM_NAME, (void *) tag, SANCUS_TAG_SIZE) || !sancus_is_outside_sm(SM_NAME, (void *) conn_idx, sizeof(uint16_t)) ) { - return 1; + return BufferInsideSM; } // Note: make sure we only use AD_SIZE bytes of the buffer `ad` @@ -19,19 +19,19 @@ uint16_t SM_ENTRY(SM_NAME) __sm_set_key(const uint8_t* ad, const uint8_t* cipher // check if there is still space left in the array if (__sm_num_connections == SM_MAX_CONNECTIONS) { - return 2; + return InternalError; } // check nonce if(nonce != __sm_num_connections) { - return 3; + return MalformedPayload; } Connection *conn = &__sm_io_connections[__sm_num_connections]; *conn_idx = __sm_num_connections; if (!sancus_unwrap(ad, AD_SIZE, cipher, SANCUS_KEY_SIZE, tag, conn->key)) { - return 4; + return CryptoError; } __sm_num_connections++; @@ -39,5 +39,5 @@ uint16_t SM_ENTRY(SM_NAME) __sm_set_key(const uint8_t* ad, const uint8_t* cipher conn->conn_id = conn_id; conn->nonce = 0; - return 0; + return Ok; } From e5cb9ced9d5417bdfb8ffcf99b64a754975c2e90 Mon Sep 17 00:00:00 2001 From: Gianluca Scopelliti Date: Thu, 21 Oct 2021 11:12:20 +0200 Subject: [PATCH 23/27] handle_output: check if allocated buffer is outside SM --- src/stubs/sm_output.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/stubs/sm_output.c b/src/stubs/sm_output.c index 2e59b2d..5cb53a4 100644 --- a/src/stubs/sm_output.c +++ b/src/stubs/sm_output.c @@ -16,7 +16,7 @@ SM_FUNC(SM_NAME) void __sm_send_output(io_index index, continue; uint8_t* payload = malloc(payload_len); - if (payload == NULL) + if (payload == NULL || !sancus_is_outside_sm(SM_NAME, (void *) payload, payload_len)) continue; // associated data only contains the nonce, therefore we can use this From e760cea9fc96373bc86d38039b54e75e045f83e2 Mon Sep 17 00:00:00 2001 From: Gianluca Scopelliti Date: Thu, 21 Oct 2021 11:13:20 +0200 Subject: [PATCH 24/27] remove unused include --- src/stubs/sm_input.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/stubs/sm_input.c b/src/stubs/sm_input.c index 417b807..05f3f55 100644 --- a/src/stubs/sm_input.c +++ b/src/stubs/sm_input.c @@ -1,7 +1,6 @@ #include "reactive_stubs_support.h" #include -#include uint16_t SM_ENTRY(SM_NAME) __sm_handle_input(uint16_t conn_idx, const void* payload, size_t len) From 537c3a058efe5f8865f5e0e2772167563ea94792 Mon Sep 17 00:00:00 2001 From: Gianluca Scopelliti Date: Thu, 21 Oct 2021 15:11:31 +0200 Subject: [PATCH 25/27] return a value from `output` functions --- src/sancus_support/reactive.h | 18 +++++++++--------- src/stubs/sm_output.c | 19 ++++++++++++++----- 2 files changed, 23 insertions(+), 14 deletions(-) diff --git a/src/sancus_support/reactive.h b/src/sancus_support/reactive.h index d455113..d3bebc5 100644 --- a/src/sancus_support/reactive.h +++ b/src/sancus_support/reactive.h @@ -11,15 +11,15 @@ typedef uint8_t io_data __attribute__((aligned(2))); // The ASM symbols are used for the linker to be able to detect inputs/outputs -#define SM_OUTPUT_AUX(sm, name) \ - asm("__sm_" #sm "_output_tag_" #name " = 0\n"); \ - SM_FUNC(sm) void name(const io_data* data, size_t len) \ - { \ - extern char __sm_##sm##_io_##name##_idx; \ - SM_FUNC(sm) void __sm_##sm##_send_output(unsigned int, \ - const void*, size_t); \ - __sm_##sm##_send_output((io_index)&__sm_##sm##_io_##name##_idx, \ - data, len); \ +#define SM_OUTPUT_AUX(sm, name) \ + asm("__sm_" #sm "_output_tag_" #name " = 0\n"); \ + SM_FUNC(sm) uint16_t name(const io_data* data, size_t len) \ + { \ + extern char __sm_##sm##_io_##name##_idx; \ + SM_FUNC(sm) uint16_t __sm_##sm##_send_output(unsigned int, \ + const void*, size_t); \ + return __sm_##sm##_send_output((io_index)&__sm_##sm##_io_##name##_idx, \ + data, len); \ } #define SM_OUTPUT(sm, name) SM_OUTPUT_AUX(sm, name) diff --git a/src/stubs/sm_output.c b/src/stubs/sm_output.c index 5cb53a4..cc71424 100644 --- a/src/stubs/sm_output.c +++ b/src/stubs/sm_output.c @@ -2,7 +2,7 @@ #include -SM_FUNC(SM_NAME) void __sm_send_output(io_index index, +SM_FUNC(SM_NAME) uint16_t __sm_send_output(io_index index, const void* data, size_t len) { const size_t payload_len = len + SANCUS_TAG_SIZE; @@ -12,12 +12,19 @@ SM_FUNC(SM_NAME) void __sm_send_output(io_index index, int i; for (i=0; i<__sm_num_connections; i++) { Connection *conn = &__sm_io_connections[i]; - if(conn->io_id != index) - continue; + if (conn->io_id != index) { + continue; + } uint8_t* payload = malloc(payload_len); - if (payload == NULL || !sancus_is_outside_sm(SM_NAME, (void *) payload, payload_len)) - continue; + + if (payload == NULL) { + return InternalError; + } + + if ( !sancus_is_outside_sm(SM_NAME, (void *) payload, payload_len) ) { + return BufferInsideSM; + } // associated data only contains the nonce, therefore we can use this // this trick to build the array fastly (i.e. by swapping the bytes) @@ -27,4 +34,6 @@ SM_FUNC(SM_NAME) void __sm_send_output(io_index index, conn->nonce++; reactive_handle_output(conn->conn_id, payload, payload_len); } + + return Ok; } From b27b130dd87a51dfc1d57f02ef3e0f79a8ad8256 Mon Sep 17 00:00:00 2001 From: Gianluca Scopelliti Date: Mon, 25 Oct 2021 14:46:39 +0200 Subject: [PATCH 26/27] use uintptr_t type instead of void* in is_buffer_outside_region --- src/sancus_support/sm_support.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/sancus_support/sm_support.h b/src/sancus_support/sm_support.h index c7d05e5..9f33c64 100644 --- a/src/sancus_support/sm_support.h +++ b/src/sancus_support/sm_support.h @@ -4,6 +4,7 @@ #include "config.h" #include +#include #if __GNUC__ >= 5 || __clang_major__ >= 5 @@ -312,10 +313,9 @@ sm_id sancus_enable_wrapped(struct SancusModule* sm, unsigned nonce, void* tag); * Returns true if buf is outside the memory region [start, end) * if start >= end, immediately return false */ -always_inline int is_buffer_outside_region(void *start, void *end, - void *buf, size_t len) { - void *buf_end; - +always_inline int is_buffer_outside_region(uintptr_t start, uintptr_t end, + uintptr_t buf, size_t len) { + uintptr_t buf_end; // make sure start < end, otherwise return false if (start >= end) { return 0; From d57b76dfd8ca21ac4c71d49edc16aa35570e838e Mon Sep 17 00:00:00 2001 From: Gianluca Scopelliti Date: Tue, 26 Oct 2021 15:45:16 +0200 Subject: [PATCH 27/27] fix: cast from void* to uintptr_t inside is_buffer_outside_region --- src/sancus_support/sm_support.h | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/sancus_support/sm_support.h b/src/sancus_support/sm_support.h index 9f33c64..12264ef 100644 --- a/src/sancus_support/sm_support.h +++ b/src/sancus_support/sm_support.h @@ -313,9 +313,13 @@ sm_id sancus_enable_wrapped(struct SancusModule* sm, unsigned nonce, void* tag); * Returns true if buf is outside the memory region [start, end) * if start >= end, immediately return false */ -always_inline int is_buffer_outside_region(uintptr_t start, uintptr_t end, - uintptr_t buf, size_t len) { +always_inline int is_buffer_outside_region(void *start_p, void *end_p, + void *buf_p, size_t len) { + uintptr_t start = (uintptr_t) start_p; + uintptr_t end = (uintptr_t) end_p; + uintptr_t buf = (uintptr_t) buf_p; uintptr_t buf_end; + // make sure start < end, otherwise return false if (start >= end) { return 0;