Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Changes in stubs and linker.py for Authentic Execution #36

Merged
merged 27 commits into from
Oct 26, 2021
Merged
Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
d58d972
linker.py: add KEEP to input_callbacks to preserve input symbols
gianlu33 Aug 23, 2021
ec7dbee
changes in stubs and linker.py for Authentic Execution
gianlu33 Aug 23, 2021
c05544c
add assertion for the Connection struct, add global var in linker.py
gianlu33 Sep 30, 2021
62d2bf3
fix sancus_is_outside_sm
gianlu33 Sep 30, 2021
1b1f009
sm_input: O(1) lookup of conn using index returned by set_key
gianlu33 Sep 30, 2021
967f06a
sancus_is_outside_sm: check for int overflow
jovanbulck Sep 30, 2021
588605d
Fix brackets sancus_is_outside_sm
jovanbulck Oct 1, 2021
2fc4ce6
declare extern vars for the public/secret regions, sanitize sm_attest()
gianlu33 Oct 1, 2021
7d29fb2
sanitize inputs of __sm_set_key and __sm_handle_input
gianlu33 Oct 1, 2021
0554693
reactive: declare symbols for the public/secret regions
jovanbulck Oct 1, 2021
bb6edf2
fix declaration of symbols for public/secret regions
gianlu33 Oct 5, 2021
09f2bfd
__sm_handle_input: return a value to indicate success/error
gianlu33 Oct 5, 2021
6152f0e
__sm_attest: return a value to indicate success/error
gianlu33 Oct 6, 2021
6f8cb87
simplify __sm_handle_input
gianlu33 Oct 6, 2021
e8f5890
small fix return value in __sm_handle_input
gianlu33 Oct 6, 2021
3ae4774
use config file for `num_connections` parameter
gianlu33 Oct 15, 2021
ce71396
add num_connections property in SmConfig
gianlu33 Oct 15, 2021
5621e27
add comments in linker.py to explain the connection array
gianlu33 Oct 15, 2021
16ae05c
add num_connections parameter in sm-config-example.yaml
gianlu33 Oct 15, 2021
b50bb02
move io_id check from set_key to handle_input
gianlu33 Oct 20, 2021
0a81ca0
fix sm_{}_io_connections buffer allocation if num_connections is zero
gianlu33 Oct 20, 2021
7ee6635
return values of stubs as ResultCode values
gianlu33 Oct 21, 2021
e5cb9ce
handle_output: check if allocated buffer is outside SM
gianlu33 Oct 21, 2021
e760cea
remove unused include
gianlu33 Oct 21, 2021
537c3a0
return a value from `output` functions
gianlu33 Oct 21, 2021
b27b130
use uintptr_t type instead of void* in is_buffer_outside_region
gianlu33 Oct 25, 2021
d57b76d
fix: cast from void* to uintptr_t inside is_buffer_outside_region
gianlu33 Oct 26, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
103 changes: 67 additions & 36 deletions src/drivers/linker.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

MAC_SIZE = int(sancus.config.SECURITY / 8)
KEY_SIZE = MAC_SIZE
CONNECTION_STRUCT_SIZE = 6 + KEY_SIZE


class SmEntry:
Expand Down Expand Up @@ -63,7 +64,7 @@ def add_sym(file, sym_map):

args += [file, file]
call_prog('msp430-elf-objcopy', args)
return file
return file


def parse_size(val):
Expand Down Expand Up @@ -92,18 +93,23 @@ 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)
'__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
Expand All @@ -115,7 +121,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(''))] = \
Expand All @@ -136,15 +142,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)
Expand Down Expand Up @@ -212,7 +222,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]

Expand Down Expand Up @@ -269,6 +279,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

Expand Down Expand Up @@ -409,6 +420,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

Expand Down Expand Up @@ -463,7 +482,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 = ''
Expand Down Expand Up @@ -829,24 +848,36 @@ 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
io_keys = ''
"""
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).
"""

if len(ios) > 0:
io_keys += '__sm_{}_io_keys = .;\n'.format(sm)
io_keys += ' . += {};\n'.format(len(ios) * KEY_SIZE)
io_keys += ' . = ALIGN(2);'
num_connections = ''
io_connections = ''

if hasattr(sm_config[sm], "num_connections"):
sm_num_connections = sm_config[sm].num_connections
else:
sm_num_connections = 0

# Nonce used by outputs
outputs_nonce = ''
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)

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(io_connections_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
Expand All @@ -859,10 +890,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:
Expand All @@ -880,7 +911,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, sm_num_connections))
symbols.append('__sm_{}_num_inputs = {};'.format(sm, len(inputs)))

if args.prepare_for_sm_text_section_wrapping:
Expand Down
10 changes: 10 additions & 0 deletions src/drivers/sancus/sancus_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
"""
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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'
Expand All @@ -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
7 changes: 7 additions & 0 deletions src/drivers/sm-config-example.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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

1 change: 1 addition & 0 deletions src/sancus_support/reactive.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#include <stdint.h>

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
Expand Down
37 changes: 31 additions & 6 deletions src/sancus_support/sm_support.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -311,6 +308,34 @@ 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,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we still have an issue here with overflow(!)

  • pointer arithmetic for void* pointers is illegal in C (while silently allowed in gcc)
  • overflow is only guaranteed to wrap around for unsigned data types in the C standard (even gcc might simply compile/optimize away the check we added as it's undefined behavior)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what we should probably do here is to cast all the void* inputs to uintptr_t first

thanks @mtvec !

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok done, by changing the signature of the function the cast should be implicit right?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks! I think it is fine now, but we might get compiler warnings if every argument is not explicitly casted in the function call. Not sure how that is for the sancus_is_outside_sm macro.

Maybe cleaner to keep the void* pointers in the function arguments and cast them as local variables instead if that's possible easily?

sorry for the overhead!

Copy link
Member Author

@gianlu33 gianlu33 Oct 26, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had the same thought but actually I did not get any warnings when compiling the library

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure, will existing applications not get a warning? eg

https://github.com/sancus-tee/sancus-examples/blob/master/sensor-reader/reader.c#L15

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, you are totally right, sorry! I did not make clean before rebuilding the library, so this is why I was not getting any warnings.

I fixed it, now everything should be fine!

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;
}

jovanbulck marked this conversation as resolved.
Show resolved Hide resolved
/* check for int overflow and finally validate `buf` falls outside */
if( (buf <= buf_end) && ((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
Expand Down
2 changes: 2 additions & 0 deletions src/stubs/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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

Expand Down
41 changes: 33 additions & 8 deletions src/stubs/reactive_stubs_support.h
Original file line number Diff line number Diff line change
Expand Up @@ -3,29 +3,54 @@

#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);
jovanbulck marked this conversation as resolved.
Show resolved Hide resolved

typedef uint8_t IoKey[SANCUS_KEY_SIZE];
typedef void (*InputCallback)(const void*, size_t);

typedef enum
{
Ok = 0x0,
IllegalConnection = 0x1,
MalformedPayload = 0x2
Ok = 0x0,
IllegalConnection = 0x1,
MalformedPayload = 0x2,
IllegalParameters = 0x3,
BufferInsideSM = 0x4,
CryptoError = 0x5,
InternalError = 0x6
} ResultCode;

typedef struct Connection {
io_index io_id;
conn_index conn_id;
uint16_t nonce;
IoKey key;
} Connection;
gianlu33 marked this conversation as resolved.
Show resolved Hide resolved

// 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 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
jovanbulck marked this conversation as resolved.
Show resolved Hide resolved

extern uint16_t __sm_num_connections;

extern char __sm_num_inputs;
#define SM_NUM_INPUTS (size_t)&__sm_num_inputs

#define SM_NAME X

// declare symbols for the public/secret regions
#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
Loading