authenticexecution/tz-code-generator

TrustZone code generator

Trustzone module code generator for the Authentic Execution framework

Installation

# from the root of this repository
pip install .

Run

#TODO

General rules

TODO

tz-code-generator's Issues

Bug: crash in `handle_output` for AES connections when `data_len` is zero

For some reason, calling an output of an AES connection passing empty data (i.e., setting data_len to zero) makes the TA crash.

What I would like to do is something like this:

SM_INPUT(button_pressed, data, data_len) {
    DMSG("Remote button has been pressed\n");

    // this is an AES connection
    OUTPUT(increment_presses, NULL, 0);
}

But this does not work. A workaround to this bug would be to pass fake data to the output like this:

SM_INPUT(button_pressed, data, data_len) {
    DMSG("Remote button has been pressed\n");

    // temporary: get around a bug
    unsigned char fake_data[2] = {1,2};

    // toggle LED
    OUTPUT(increment_presses, fake_data, 2);
}

However this is not really optimal. I think you should review the AES encryption part in the case where data is NULL and data_len is zero.

Also, you should add a check in handle_output to verify that if data is NULL then data_len must be zero. Otherwise you might have some other crashes in other parts of the code.

Wrong type for `entry_funcs`

Type of entry_funcs should be entry_t and not input_t

TZ-Code-Generator/tzcodegen/stubs/authentic_execution.c

Line 23 in 598eca5

input_t entry_funcs[{num_entrys}] = { {fill_entrys} };

Security: check index before access `entry_funcs` and `input_funcs`

We should check if the index entry_id - 3 is a valid index (is within the bounds of the array). Same also applies input_funcs

TZ-Code-Generator/tzcodegen/stubs/authentic_execution.c

Line 686 in 598eca5

entry_funcs[entry_id - 3](session, param_types, params, data_input, size);

Possible solution: define the size of both arrays using a #define. I would also define the starting index of entry points

// somewhere at the beginning
#define NUM_INPUTS {num_inputs}
#define NUM_ENTRIES {num_entrys}
#define ENTRY_START_INDEX 3

// the arrays become as follows:
input_t input_funcs[NUM_INPUTS] = { {fill_inputs} };
entry_t entry_funcs[NUM_ENTRIES] = { {fill_entrys} };

// checks in handle_entry (for handle_input, it's analogous)
if(index - ENTRY_START_INDEX >= 0 && index - ENTRY_START_INDEX < NUM_ENTRIES) {
   // access the array
}

Fix warnings

When compiling a TZ module there are some warnings that we should fix. Most warnings are about unused parameters and type casting.

For example, by compiling the gateway module of this example, the CI got the following warnings:

spongent.c:41:12: warning: no previous prototype for 'min' [-Wmissing-prototypes]
   41 | DataLength min(DataLength a, DataLength b){
      |            ^~~
spongent.c:51:6: warning: no previous prototype for 'copy_n' [-Wmissing-prototypes]
   51 | void copy_n(BitSequence *first, int num, BitSequence *result){
      |      ^~~~~~
spongent.c:59:6: warning: no previous prototype for 'PrintState' [-Wmissing-prototypes]
   59 | void PrintState(hashState *state)
      |      ^~~~~~~~~~
spongent.c: In function 'Permute':
spongent.c:173:2: warning: switch missing default case [-Wswitch-default]
  173 |  switch(version)
      |  ^~~~~~
spongent.c: At top level:
spongent.c:354:12: warning: no previous prototype for 'Duplexing' [-Wmissing-prototypes]
  354 | HashReturn Duplexing(hashState* state,
      |            ^~~~~~~~~
spongent.c: In function 'SpongentWrap':
spongent.c:403:16: warning: passing argument 1 of 'copy_n' discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
  403 |         copy_n(key, SW_RATE_BYTES, block);
      |                ^~~
spongent.c:51:26: note: expected 'BitSequence *' {aka 'unsigned char *'} but argument is of type 'const BitSequence *' {aka 'const unsigned char *'}
   51 | void copy_n(BitSequence *first, int num, BitSequence *result){
      |             ~~~~~~~~~~~~~^~~~~
spongent.c:415:12: warning: passing argument 1 of 'copy_n' discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
  415 |     copy_n(key, bitsLeft / 8, block);
      |            ^~~
spongent.c:51:26: note: expected 'BitSequence *' {aka 'unsigned char *'} but argument is of type 'const BitSequence *' {aka 'const unsigned char *'}
   51 | void copy_n(BitSequence *first, int num, BitSequence *result){
      |             ~~~~~~~~~~~~~^~~~~
spongent.c:426:16: warning: passing argument 1 of 'copy_n' discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
  426 |         copy_n(ad, SW_RATE_BYTES, block);
      |                ^~
spongent.c:51:26: note: expected 'BitSequence *' {aka 'unsigned char *'} but argument is of type 'const BitSequence *' {aka 'const unsigned char *'}
   51 | void copy_n(BitSequence *first, int num, BitSequence *result){
      |             ~~~~~~~~~~~~~^~~~~
spongent.c:437:12: warning: passing argument 1 of 'copy_n' discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
  437 |     copy_n(ad, bitsLeft / 8, block);
      |            ^~
spongent.c:51:26: note: expected 'BitSequence *' {aka 'unsigned char *'} but argument is of type 'const BitSequence *' {aka 'const unsigned char *'}
   51 | void copy_n(BitSequence *first, int num, BitSequence *result){
      |             ~~~~~~~~~~~~~^~~~~
spongent.c:453:32: warning: passing argument 1 of 'copy_n' discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
  453 |         copy_n(unwrap ? output : input, SW_RATE_BYTES, block);
      |                ~~~~~~~~~~~~~~~~^~~~~~~
spongent.c:51:26: note: expected 'BitSequence *' {aka 'unsigned char *'} but argument is of type 'const unsigned char *'
   51 | void copy_n(BitSequence *first, int num, BitSequence *result){
      |             ~~~~~~~~~~~~~^~~~~
spongent.c:455:20: warning: declaration of 'ret' shadows a previous local [-Wshadow]
  455 |         HashReturn ret = Duplexing(&state, block, SW_RATE + 1, duplexOut, 16);
      |                    ^~~
spongent.c:392:16: note: shadowed declaration is here
  392 |     HashReturn ret = Init(&state, NULL);
      |                ^~~
spongent.c:470:28: warning: passing argument 1 of 'copy_n' discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
  470 |     copy_n(unwrap ? output : input, bitsLeft / 8, block);
      |            ~~~~~~~~~~~~~~~~^~~~~~~
spongent.c:51:26: note: expected 'BitSequence *' {aka 'unsigned char *'} but argument is of type 'const unsigned char *'
   51 | void copy_n(BitSequence *first, int num, BitSequence *result){
      |             ~~~~~~~~~~~~~^~~~~
authentic_execution.c:30:7: warning: no previous prototype for 'malloc_aligned' [-Wmissing-prototypes]
   30 | void *malloc_aligned(size_t size) {
      |       ^~~~~~~~~~~~~~
authentic_execution.c:55:5: warning: no previous prototype for 'connections_add' [-Wmissing-prototypes]
   55 | int connections_add(Connection* connection)
      |     ^~~~~~~~~~~~~~~
authentic_execution.c:68:13: warning: no previous prototype for 'connections_get' [-Wmissing-prototypes]
   68 | Connection* connections_get(uint16_t conn_id)
      |             ^~~~~~~~~~~~~~~
authentic_execution.c:85:6: warning: no previous prototype for 'find_connections' [-Wmissing-prototypes]
   85 | void find_connections(uint16_t io_id, int *arr, uint8_t *num)
      |      ^~~~~~~~~~~~~~~~
authentic_execution.c: In function 'set_key':
authentic_execution.c:297:17: warning: unused variable 'exp_param_types' [-Wunused-variable]
  297 |  const uint32_t exp_param_types = TEE_PARAM_TYPES(TEE_PARAM_TYPE_MEMREF_INPUT,
      |                 ^~~~~~~~~~~~~~~
authentic_execution.c:293:51: warning: unused parameter 'param_types' [-Wunused-parameter]
  293 | static TEE_Result set_key(void *session, uint32_t param_types,
      |                                          ~~~~~~~~~^~~~~~~~~~~
authentic_execution.c: In function 'attest':
authentic_execution.c:369:17: warning: unused variable 'exp_param_types' [-Wunused-variable]
  369 |  const uint32_t exp_param_types = TEE_PARAM_TYPES(TEE_PARAM_TYPE_MEMREF_INPUT,
      |                 ^~~~~~~~~~~~~~~
authentic_execution.c:365:50: warning: unused parameter 'param_types' [-Wunused-parameter]
  365 | static TEE_Result attest(void *session, uint32_t param_types,
      |                                         ~~~~~~~~~^~~~~~~~~~~
authentic_execution.c: In function 'handle_output':
authentic_execution.c:493:35: warning: pointer targets in passing argument 2 of 'set_aes_key' differ in signedness [-Wpointer-sign]
  493 |       set_aes_key(sess, connection->connection_key, 16);
      |                         ~~~~~~~~~~^~~~~~~~~~~~~~~~
      |                                   |
      |                                   unsigned char *
authentic_execution.c:242:52: note: expected 'char *' but argument is of type 'unsigned char *'
  242 | static TEE_Result set_aes_key(void *session, char *key, uint32_t key_sz){
      |                                              ~~~~~~^~~
authentic_execution.c:494:26: warning: pointer targets in passing argument 2 of 'reset_aes_iv' differ in signedness [-Wpointer-sign]
  494 |       reset_aes_iv(sess, aad, 2, nonce, nonce_sz, data_len);
      |                          ^~~
      |                          |
      |                          unsigned char *
authentic_execution.c:277:53: note: expected 'char *' but argument is of type 'unsigned char *'
  277 | static TEE_Result reset_aes_iv(void *session, char *aad, size_t aad_sz,
      |                                               ~~~~~~^~~
authentic_execution.c:529:38: warning: pointer of type 'void *' used in arithmetic [-Wpointer-arith]
  529 |  TEE_MemMove(params[1].memref.buffer + (totalOutput * 2), conn_id, (2 * numberOfOutput));
      |                                      ^
authentic_execution.c:530:38: warning: pointer of type 'void *' used in arithmetic [-Wpointer-arith]
  530 |  TEE_MemMove(params[2].memref.buffer + indexOfData, data, ((data_len * numberOfOutput) + numberOfOutput));
      |                                      ^
authentic_execution.c:531:38: warning: pointer of type 'void *' used in arithmetic [-Wpointer-arith]
  531 |  TEE_MemMove(params[3].memref.buffer + (totalOutput * 16), tag, (16 * numberOfOutput));
      |                                      ^
authentic_execution.c:437:64: warning: unused parameter 'param_types' [-Wunused-parameter]
  437 | void handle_output(void *session, uint32_t output_id, uint32_t param_types,
      |                                                       ~~~~~~~~~^~~~~~~~~~~
authentic_execution.c: In function 'handle_input':
authentic_execution.c:575:34: warning: pointer targets in passing argument 2 of 'set_aes_key' differ in signedness [-Wpointer-sign]
  575 |      set_aes_key(sess, connection->connection_key, 16); //#######
      |                        ~~~~~~~~~~^~~~~~~~~~~~~~~~
      |                                  |
      |                                  unsigned char *
authentic_execution.c:242:52: note: expected 'char *' but argument is of type 'unsigned char *'
  242 | static TEE_Result set_aes_key(void *session, char *key, uint32_t key_sz){
      |                                              ~~~~~~^~~
authentic_execution.c:576:25: warning: pointer targets in passing argument 2 of 'reset_aes_iv' differ in signedness [-Wpointer-sign]
  576 |      reset_aes_iv(sess, aad, 2, nonce, nonce_sz, size);
      |                         ^~~
      |                         |
      |                         unsigned char *
authentic_execution.c:277:53: note: expected 'char *' but argument is of type 'unsigned char *'
  277 | static TEE_Result reset_aes_iv(void *session, char *aad, size_t aad_sz,
      |                                               ~~~~~~^~~
authentic_execution.c:602:21: warning: comparison of integer expressions of different signedness: 'int' and 'uint32_t' {aka 'unsigned int'} [-Wsign-compare]
  602 |   for (int n = 0; n < size; n++){
      |                     ^
authentic_execution.c:620:23: warning: comparison is always true due to limited range of data type [-Wtype-limits]
  620 |  if(connection->io_id >= 0 && connection->io_id < NUM_INPUTS) {
      |                       ^~
authentic_execution.c:546:17: warning: unused variable 'exp_param_types' [-Wunused-variable]
  546 |  const uint32_t exp_param_types = TEE_PARAM_TYPES(TEE_PARAM_TYPE_VALUE_INOUT,
      |                 ^~~~~~~~~~~~~~~
authentic_execution.c: At top level:
authentic_execution.c:631:12: warning: no previous prototype for 'handle_entry' [-Wmissing-prototypes]
  631 | TEE_Result handle_entry(void *session, uint32_t param_types, TEE_Param params[4]){
      |            ^~~~~~~~~~~~
authentic_execution.c: In function 'handle_entry':
authentic_execution.c:633:17: warning: unused variable 'exp_param_types' [-Wunused-variable]
  633 |  const uint32_t exp_param_types = TEE_PARAM_TYPES(TEE_PARAM_TYPE_VALUE_INOUT,
      |                 ^~~~~~~~~~~~~~~

`malloc`: check result and allocate only what is really needed

This happens in many places on your authentic_execution.c file. Example:

TZ-Code-Generator/tzcodegen/stubs/authentic_execution.c

Line 676 in 598eca5

data_input = malloc(16 * 16);

We should always check if the return value of a malloc is NULL. Also, we should always allocate the right number of bytes. It does not make sense to allocate a fixed amount of bytes like 16*16 in the example, because in this case you can just allocate a static buffer instead. The amount of bytes to allocate should depend on some variable.

Remove debug prints

Debug prints should not be part of the public code :)

if you use logging.debug(<string>) that's fine, you can keep those instructions around without any problems. But any other debug instructions (e.g., print) should be removed or replaced instead.

put some info on the README

...when you have time, not urgent

Bad UUID string injected to `user_ta_header_defines.h`

I just tried the script for testing and I noticed a problem in the UUID injected to user_ta_header_defines.h. This is the output:

#define TA_UUID		{ 0xa0f7260d, 0x91d5, 0x4751, {0xa3, 0xeb, 0x 6, 0x03, 0xc3, 0x9b, 0x28, 0x4e} }

As you can see, there is one byte, namely 0x 6, with a space between 0x and 6. Of course this would generate an error during compilation, so we should fix this issue.

I think the problem is in the following line:

TZ-Code-Generator/tzcodegen/generator.py

Line 34 in 598eca5

n[::2] = ['{:12x}'.format(u.node)[i:i + 2] for i in range(0, 12, 2)]

I also attach the output of the script printed to the terminal:

(env) ubuntu@gianluca-private:~/authexec/TZ-Code-Generator$ TZ-Code-Gen -i example/whatever/ -o out
a0f7260d-91d5-4751-a3eb-0603c39b284e
213959748456308857955459706705327892558
a0f7260d-91d5-4751-a3eb-0603c39b284e

#define TA_UUID
        { 0xa0f7260d, 0x91d5, 0x4751, \ 

                { 0xa3, 0xeb, 0x 6, 0x03, 0xc3, 0x9b, 0x28, 0x4e} }
<class 'dict'> {}
{'entry1': 3}
Found outputs: ['button_pressed']
<class 'list'>
{'button_pressed': 16384}