diff --git a/firmware/src/sgx/src/untrusted/keyvalue_store.c b/firmware/src/sgx/src/untrusted/keyvalue_store.c index a1058253..b68d6ce2 100644 --- a/firmware/src/sgx/src/untrusted/keyvalue_store.c +++ b/firmware/src/sgx/src/untrusted/keyvalue_store.c @@ -23,19 +23,52 @@ */ #include -#include "hsm_u.h" +#include +#include +#include #include "log.h" +#include "keyvalue_store.h" #define KVSTORE_PREFIX "./kvstore-" #define KVSTORE_SUFFIX ".dat" +#define KVSTORE_MAX_KEY_LEN 150 + +// Sanitizes a key by allowing only [a-zA-Z0-9]. If one or more invalid +// characters are found, Replace them with a single hyphen. +static void sanitize_key(char* key, char* sanitized_key) { + if (!key || !sanitized_key) + return; + + size_t key_len = strlen(key); + + // Truncate key if it's too long + if (key_len > KVSTORE_MAX_KEY_LEN) { + key_len = KVSTORE_MAX_KEY_LEN; + } + + bool prev_char_valid = false; + size_t sanitized_key_len = 0; + for (size_t i = 0; i < key_len; i++) { + if (isalnum(key[i])) { + sanitized_key[sanitized_key_len++] = key[i]; + prev_char_valid = true; + } else if (prev_char_valid) { + sanitized_key[sanitized_key_len++] = '-'; + prev_char_valid = false; + } + } + sanitized_key[sanitized_key_len] = '\0'; +} static char* filename_for(char* key) { + char sanitized_key[KVSTORE_MAX_KEY_LEN + 1]; + sanitize_key(key, sanitized_key); size_t filename_size = - strlen(KVSTORE_PREFIX) + strlen(KVSTORE_SUFFIX) + strlen(key); + strlen(KVSTORE_PREFIX) + strlen(KVSTORE_SUFFIX) + strlen(sanitized_key); char* filename = malloc(filename_size + 1); strcpy(filename, ""); strcat(filename, KVSTORE_PREFIX); - strcat(filename, key); + strcat(filename, sanitized_key); strcat(filename, KVSTORE_SUFFIX); return filename; } diff --git a/firmware/src/sgx/src/untrusted/keyvalue_store.h b/firmware/src/sgx/src/untrusted/keyvalue_store.h index 872a60a5..f54de20b 100644 --- a/firmware/src/sgx/src/untrusted/keyvalue_store.h +++ b/firmware/src/sgx/src/untrusted/keyvalue_store.h @@ -25,6 +25,9 @@ #ifndef __KEYVALUE_STORE_H #define __KEYVALUE_STORE_H +#include +#include + /** * @brief Tell whether a given key currently exists * diff --git a/firmware/src/sgx/test/common/common.mk b/firmware/src/sgx/test/common/common.mk index fbf7a6b1..7d43cd52 100644 --- a/firmware/src/sgx/test/common/common.mk +++ b/firmware/src/sgx/test/common/common.mk @@ -1,5 +1,6 @@ TESTCOMMONDIR = ../common SGXTRUSTEDDIR = ../../src/trusted +SGXUNTRUSTEDDIR = ../../src/untrusted HALINCDIR = ../../../hal/include HALSGXSRCDIR = ../../../hal/sgx/src/trusted POWHSMSRCDIR = ../../../powhsm/src @@ -7,13 +8,14 @@ COMMONDIR = ../../../common/src CFLAGS = -iquote $(TESTCOMMONDIR) CFLAGS += -iquote $(SGXTRUSTEDDIR) +CFLAGS += -iquote $(SGXUNTRUSTEDDIR) CFLAGS += -iquote $(HALINCDIR) CFLAGS += -iquote $(HALSGXSRCDIR) CFLAGS += -iquote $(POWHSMSRCDIR) CFLAGS += -iquote $(COMMONDIR) CFLAGS += -DHSM_PLATFORM_SGX -VPATH += $(SGXTRUSTEDDIR):$(COMMONDIR) +VPATH += $(SGXTRUSTEDDIR):$(SGXUNTRUSTEDDIR):$(COMMONDIR) include ../../../../coverage/coverage.mk diff --git a/firmware/src/sgx/test/keyvalue_store/Makefile b/firmware/src/sgx/test/keyvalue_store/Makefile new file mode 100644 index 00000000..705a9078 --- /dev/null +++ b/firmware/src/sgx/test/keyvalue_store/Makefile @@ -0,0 +1,38 @@ +# The MIT License (MIT) +# +# Copyright (c) 2021 RSK Labs Ltd +# +# Permission is hereby granted, free of charge, to any person obtaining a copy of +# this software and associated documentation files (the "Software"), to deal in +# the Software without restriction, including without limitation the rights to +# use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies +# of the Software, and to permit persons to whom the Software is furnished to do +# so, subject to the following conditions: +# +# The above copyright notice and this permission notice shall be included in all +# copies or substantial portions of the Software. +# +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +# SOFTWARE. + +include ../common/common.mk + +PROG = test.out +OBJS = keyvalue_store.o test_keyvalue_store.o log.o + +all: $(PROG) + +$(PROG): $(OBJS) + $(CC) $(COVFLAGS) -o $@ $^ + +.PHONY: clean test +clean: + rm -f $(PROG) *.o *.dat $(COVFILES) + +test: all + ./$(PROG) diff --git a/firmware/src/sgx/test/keyvalue_store/test_keyvalue_store.c b/firmware/src/sgx/test/keyvalue_store/test_keyvalue_store.c new file mode 100644 index 00000000..483d5ed3 --- /dev/null +++ b/firmware/src/sgx/test/keyvalue_store/test_keyvalue_store.c @@ -0,0 +1,282 @@ +/** + * The MIT License (MIT) + * + * Copyright (c) 2021 RSK Labs Ltd + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to + * deal in the Software without restriction, including without limitation the + * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or + * sell copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS + * IN THE SOFTWARE. + */ + +#include +#include +#include +#include +#include +#include +#include "keyvalue_store.h" + +// Test helpers +void setup() { + system("rm -f ./kvstore-*.dat"); +} + +void assert_key_exists(char* key, bool exists) { + assert(kvstore_exists(key) == exists); +} + +void assert_key_value(char* key, uint8_t* data, size_t data_size) { + uint8_t retrieved_data[BUFSIZ]; + size_t retrieved_size = + kvstore_get(key, retrieved_data, sizeof(retrieved_data)); + assert(retrieved_size == data_size); + assert(memcmp(retrieved_data, data, retrieved_size) == 0); +} + +void save_and_assert_success(char* key, uint8_t* data, size_t data_size) { + assert(kvstore_save(key, data, data_size)); + assert_key_exists(key, true); +} + +void remove_and_assert_success(char* key) { + assert(kvstore_remove(key)); + assert_key_exists(key, false); +} + +void assert_file_exists(char* filename, bool exists) { + FILE* file = fopen(filename, "rb"); + if (exists) { + assert(file != NULL); + } else { + assert(file == NULL); + } + if (file) { + fclose(file); + } +} + +void assert_file_contents(char* filename, uint8_t* data, size_t data_size) { + FILE* file = fopen(filename, "rb"); + assert(file != NULL); + + uint8_t file_data[BUFSIZ]; + size_t file_size = + fread(file_data, sizeof(file_data[0]), sizeof(file_data), file); + assert(file_size == data_size); + assert(memcmp(file_data, data, data_size) == 0); + + fclose(file); +} + +// Test cases +void test_save_retrieve() { + printf("Test save and retrieve...\n"); + setup(); + + struct { + char* key; + uint8_t* data; + } input_data[] = { + {"a-key", (uint8_t*)"some piece of data"}, + {"another-key", (uint8_t*)"another piece of data"}, + {"yet-another-key", (uint8_t*)"yet another piece of data"}, + {"the-last-key", (uint8_t*)"the last piece of data"}}; + size_t num_inputs = sizeof(input_data) / sizeof(input_data[0]); + + for (size_t i = 0; i < num_inputs; i++) { + save_and_assert_success(input_data[i].key, + input_data[i].data, + strlen((char*)input_data[i].data)); + } + + for (size_t i = 0; i < num_inputs; i++) { + assert_key_value(input_data[i].key, + input_data[i].data, + strlen((char*)input_data[i].data)); + } +} + +void test_kvstore_exists() { + printf("Test kvstore_exists...\n"); + setup(); + + struct { + char* key; + uint8_t* data; + } existing_keys[] = { + {"first-key", (uint8_t*)"some piece of data"}, + {"second-key", (uint8_t*)"another piece of data"}, + {"third-key", (uint8_t*)"yet another piece of data"}, + }; + size_t num_existing_keys = sizeof(existing_keys) / sizeof(existing_keys[0]); + + char* non_existing_keys[] = { + "non-existing-key-1", + "non-existing-key-2", + "non-existing-key-3", + }; + size_t num_non_existing_keys = + sizeof(non_existing_keys) / sizeof(non_existing_keys[0]); + + for (size_t i = 0; i < num_existing_keys; i++) { + save_and_assert_success(existing_keys[i].key, + existing_keys[i].data, + strlen((char*)existing_keys[i].data)); + } + + for (size_t i = 0; i < num_existing_keys; i++) { + assert_key_exists(existing_keys[i].key, true); + } + + for (size_t i = 0; i < num_non_existing_keys; i++) { + assert_key_exists(non_existing_keys[i], false); + } +} + +void test_save_remove() { + printf("Test save and remove...\n"); + setup(); + + struct { + char* key; + uint8_t* data; + bool remove; + } input_data[] = { + {"first-key", (uint8_t*)"some piece of data", false}, + {"second-key", (uint8_t*)"another piece of data", true}, + {"third-key", (uint8_t*)"yet another piece of data", true}, + {"fourth-key", (uint8_t*)"the last piece of data", false}, + }; + size_t num_inputs = sizeof(input_data) / sizeof(input_data[0]); + + for (size_t i = 0; i < num_inputs; i++) { + save_and_assert_success(input_data[i].key, + input_data[i].data, + strlen((char*)input_data[i].data)); + assert_key_value(input_data[i].key, + input_data[i].data, + strlen((char*)input_data[i].data)); + } + + // Remove selected keys + for (size_t i = 0; i < num_inputs; i++) { + if (input_data[i].remove) { + remove_and_assert_success(input_data[i].key); + } + } + + // Assert that the selected keys were removed and the others still exist + for (size_t i = 0; i < num_inputs; i++) { + if (input_data[i].remove) { + assert_key_exists(input_data[i].key, false); + } else { + assert_key_value(input_data[i].key, + input_data[i].data, + strlen((char*)input_data[i].data)); + } + } +} + +void test_filename() { + printf("Test filename for key...\n"); + setup(); + + struct { + char* key; + uint8_t* data; + char* filename; + } input_data[] = { + {"first-key", "data for the first key", "kvstore-first-key.dat"}, + {"second-key", "data for the second key", "kvstore-second-key.dat"}, + {"third-key", "data for the third key", "kvstore-third-key.dat"}, + {"fourth-key", "data for the fourth key", "kvstore-fourth-key.dat"}, + }; + size_t num_inputs = sizeof(input_data) / sizeof(input_data[0]); + + // Make sure none of the files exist + for (size_t i = 0; i < num_inputs; i++) { + assert_file_exists(input_data[i].filename, false); + } + + // Save data to each key and assert that the file name and contents are + // correct + for (size_t i = 0; i < num_inputs; i++) { + save_and_assert_success(input_data[i].key, + (uint8_t*)input_data[i].data, + strlen(input_data[i].data)); + assert_file_exists(input_data[i].filename, true); + assert_file_contents(input_data[i].filename, + (uint8_t*)input_data[i].data, + strlen(input_data[i].data)); + } +} + +void test_sanitize_key() { + printf("Test sanitize key...\n"); + setup(); + + struct { + char* key; + char* filename; + uint8_t* data; + } input_data[] = { + {"onlyletters", "kvstore-onlyletters.dat", "data1"}, + {"123456", "kvstore-123456.dat", "data2"}, + {"lettersandnumbers123", "kvstore-lettersandnumbers123.dat", "data3"}, + {"letters-and-numbers-with-hyphen-123", + "kvstore-letters-and-numbers-with-hyphen-123.dat", + "data4"}, + {"key containing spaces", "kvstore-key-containing-spaces.dat", "data5"}, + {"key containing special characters!@#$%^&*()", + "kvstore-key-containing-special-characters-.dat", + "data6"}, + {"../../../../../etc/passwd", "kvstore-etc-passwd.dat", "data7"}, + {"some@#£_&-(_./file#£+-:;name", "kvstore-some-file-name.dat", "data8"}, + }; + size_t num_inputs = sizeof(input_data) / sizeof(input_data[0]); + + // Make sure none of the files exist + for (size_t i = 0; i < num_inputs; i++) { + assert_file_exists(input_data[i].filename, false); + } + + // Save data to each key and assert that the file name and contents are + // correct + for (size_t i = 0; i < num_inputs; i++) { + save_and_assert_success( + input_data[i].key, input_data[i].data, strlen(input_data[i].data)); + assert_file_exists(input_data[i].filename, true); + assert_file_contents(input_data[i].filename, + input_data[i].data, + strlen(input_data[i].data)); + } + + // Ensure data can be retrieved with the original key + for (size_t i = 0; i < num_inputs; i++) { + assert_key_value( + input_data[i].key, input_data[i].data, strlen(input_data[i].data)); + } +} + +int main() { + test_save_retrieve(); + test_kvstore_exists(); + test_save_remove(); + test_filename(); + test_sanitize_key(); + return 0; +} \ No newline at end of file diff --git a/firmware/src/sgx/test/run-all.sh b/firmware/src/sgx/test/run-all.sh index e69b07d7..e54a9360 100755 --- a/firmware/src/sgx/test/run-all.sh +++ b/firmware/src/sgx/test/run-all.sh @@ -2,7 +2,7 @@ if [[ $1 == "exec" ]]; then BASEDIR=$(realpath $(dirname $0)) - TESTDIRS="system" + TESTDIRS="system keyvalue_store" for d in $TESTDIRS; do echo "******************************" echo "Testing $d..."