From b9ae604a7ef2d2006f08e41b76b3edb5987817f5 Mon Sep 17 00:00:00 2001 From: Italo Sampaio Date: Tue, 1 Oct 2024 15:44:44 -0300 Subject: [PATCH] Changes after code review - reverted block size type back to size_t - moved all mocks to the `mock` dir - reworked Makefile to get rid of uneeded targets - added all warnings to common.mk --- firmware/src/hal/include/hal/nvmem.h | 2 +- firmware/src/hal/sgx/src/trusted/nvmem.c | 4 +-- firmware/src/hal/sgx/test/common/common.mk | 14 +++++---- firmware/src/hal/sgx/test/mock/mock.h | 30 +++++++++++++++++++ .../src/hal/sgx/test/mock/mock_secret_store.c | 12 ++++---- .../mock.h => mock/mock_secret_store.h} | 8 +++-- firmware/src/hal/sgx/test/nvmem/Makefile | 10 ------- firmware/src/hal/sgx/test/nvmem/test_nvmem.c | 6 ++-- 8 files changed, 58 insertions(+), 28 deletions(-) create mode 100644 firmware/src/hal/sgx/test/mock/mock.h rename firmware/src/hal/sgx/test/{common/mock.h => mock/mock_secret_store.h} (96%) diff --git a/firmware/src/hal/include/hal/nvmem.h b/firmware/src/hal/include/hal/nvmem.h index c3e7c15b..2237d26b 100644 --- a/firmware/src/hal/include/hal/nvmem.h +++ b/firmware/src/hal/include/hal/nvmem.h @@ -75,7 +75,7 @@ void nvmem_init(); * * @return whether the block was successfully registered */ -bool nvmem_register_block(char* key, void* addr, uint8_t size); +bool nvmem_register_block(char* key, void* addr, size_t size); /** * @brief Loads registered blocks into memory diff --git a/firmware/src/hal/sgx/src/trusted/nvmem.c b/firmware/src/hal/sgx/src/trusted/nvmem.c index 98fd84d2..f37344ea 100644 --- a/firmware/src/hal/sgx/src/trusted/nvmem.c +++ b/firmware/src/hal/sgx/src/trusted/nvmem.c @@ -36,7 +36,7 @@ typedef struct { char* key; void* addr; - uint8_t size; + size_t size; } nvm_block_t; static nvm_block_t nvm_blocks[MAX_NVM_BLOCKS]; @@ -58,7 +58,7 @@ void nvmem_init() { nvm_blocks_count = 0; } -bool nvmem_register_block(char* key, void* addr, uint8_t size) { +bool nvmem_register_block(char* key, void* addr, size_t size) { if (nvm_blocks_count >= MAX_NVM_BLOCKS) { LOG("Error registering NVM block <%s>: too many blocks\n", key); return false; diff --git a/firmware/src/hal/sgx/test/common/common.mk b/firmware/src/hal/sgx/test/common/common.mk index 3291190f..71522536 100644 --- a/firmware/src/hal/sgx/test/common/common.mk +++ b/firmware/src/hal/sgx/test/common/common.mk @@ -21,13 +21,17 @@ # SOFTWARE. SRCDIR = ../../src/trusted -COMMONDIR = ../../../../common/src MOCKDIR = ../mock HALINCDIR = ../../../../hal/include -THISCOMMONDIR = ../common -CFLAGS = -iquote $(COMMONDIR) +TESTCOMMONDIR = ../common +CFLAGS = -Wall -Wextra -Werror -Wno-unused-parameter -Wno-unused-function CFLAGS += -iquote $(SRCDIR) -iquote $(HALINCDIR) -CFLAGS += -iquote $(THISCOMMONDIR) +CFLAGS += -iquote $(TESTCOMMONDIR) +CFLAGS += -iquote $(MOCKDIR) CFLAGS += -DHSM_PLATFORM_SGX -include ../../../../../coverage/coverage.mk \ No newline at end of file +VPATH += $(MOCKDIR):$(SRCDIR) + +include ../../../../../coverage/coverage.mk + +CFLAGS += $(COVFLAGS) diff --git a/firmware/src/hal/sgx/test/mock/mock.h b/firmware/src/hal/sgx/test/mock/mock.h new file mode 100644 index 00000000..c263ce1b --- /dev/null +++ b/firmware/src/hal/sgx/test/mock/mock.h @@ -0,0 +1,30 @@ +/** + * 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. + */ + +#ifndef __MOCK_H +#define __MOCK_H + +#include "mock_secret_store.h" + +#endif // #ifndef __MOCK_H diff --git a/firmware/src/hal/sgx/test/mock/mock_secret_store.c b/firmware/src/hal/sgx/test/mock/mock_secret_store.c index 6d4852fc..6fa2fd7d 100644 --- a/firmware/src/hal/sgx/test/mock/mock_secret_store.c +++ b/firmware/src/hal/sgx/test/mock/mock_secret_store.c @@ -4,12 +4,12 @@ #include #include -#include "mock.h" +#include "mock_secret_store.h" typedef struct mock_sest_register { char *key; uint8_t *secret; - uint8_t secret_length; + size_t secret_length; } mock_sest_register_t; #define MOCK_SEST_MAX_REGISTERS 10 @@ -23,7 +23,7 @@ typedef struct mock_secret_store { static mock_secret_store_t g_mock_secret_store; bool mock_sest_exists(char *key) { - for (int i = 0; i < g_mock_secret_store.num_registers; i++) { + for (size_t i = 0; i < g_mock_secret_store.num_registers; i++) { if (strcmp(g_mock_secret_store.registers[i].key, key) == 0) { return true; } @@ -37,7 +37,7 @@ bool mock_sest_write(char *key, uint8_t *secret, size_t secret_length) { return false; } int register_index = -1; - for (int i = 0; i < g_mock_secret_store.num_registers; i++) { + for (size_t i = 0; i < g_mock_secret_store.num_registers; i++) { if (strcmp(g_mock_secret_store.registers[i].key, key) == 0) { register_index = i; break; @@ -65,7 +65,7 @@ uint8_t mock_sest_read(char *key, uint8_t *dest, size_t dest_length) { g_mock_secret_store.fail_next_read = false; return 0; } - for (int i = 0; i < g_mock_secret_store.num_registers; i++) { + for (size_t i = 0; i < g_mock_secret_store.num_registers; i++) { if (strcmp(g_mock_secret_store.registers[i].key, key) == 0) { assert(dest_length >= g_mock_secret_store.registers[i].secret_length); @@ -83,7 +83,7 @@ void mock_sest_init() { } void mock_sest_reset() { - for (int i = 0; i < g_mock_secret_store.num_registers; i++) { + for (size_t i = 0; i < g_mock_secret_store.num_registers; i++) { free(g_mock_secret_store.registers[i].key); free(g_mock_secret_store.registers[i].secret); } diff --git a/firmware/src/hal/sgx/test/common/mock.h b/firmware/src/hal/sgx/test/mock/mock_secret_store.h similarity index 96% rename from firmware/src/hal/sgx/test/common/mock.h rename to firmware/src/hal/sgx/test/mock/mock_secret_store.h index 23ebc27f..39e58510 100644 --- a/firmware/src/hal/sgx/test/common/mock.h +++ b/firmware/src/hal/sgx/test/mock/mock_secret_store.h @@ -21,12 +21,14 @@ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS * IN THE SOFTWARE. */ + +#ifndef __MOCK_SECRET_STORE_H +#define __MOCK_SECRET_STORE_H + #include #include #include -// Mock functions for the secret store - /** * @brief Initializes the mock secret store */ @@ -87,3 +89,5 @@ void mock_sest_fail_next_read(bool fail); * @param fail whether the next call to sest_write should fail */ void mock_sest_fail_next_write(bool fail); + +#endif // #ifndef __MOCK_SECRET_STORE_H diff --git a/firmware/src/hal/sgx/test/nvmem/Makefile b/firmware/src/hal/sgx/test/nvmem/Makefile index f8b07b97..010ce60d 100644 --- a/firmware/src/hal/sgx/test/nvmem/Makefile +++ b/firmware/src/hal/sgx/test/nvmem/Makefile @@ -30,17 +30,7 @@ all: $(PROG) $(PROG): $(OBJS) $(CC) $(COVFLAGS) -o $@ $^ -test_nvmem.o: test_nvmem.c - $(CC) $(CFLAGS) -c -o $@ $^ - -nvmem.o: $(SRCDIR)/nvmem.c - $(CC) $(CFLAGS) $(COVFLAGS) -c -o $@ $^ - -mock_secret_store.o: $(MOCKDIR)/mock_secret_store.c - $(CC) $(CFLAGS) -c -o $@ $^ - .PHONY: clean test - clean: rm -f $(PROG) *.o $(COVFILES) diff --git a/firmware/src/hal/sgx/test/nvmem/test_nvmem.c b/firmware/src/hal/sgx/test/nvmem/test_nvmem.c index fd1e175d..b2bc1993 100644 --- a/firmware/src/hal/sgx/test/nvmem/test_nvmem.c +++ b/firmware/src/hal/sgx/test/nvmem/test_nvmem.c @@ -33,6 +33,8 @@ #include "mock.h" #define TEST_NVMEM_NUM_BLOCKS 5 +// FIXME: sest_read will not work with blocks larger than 255 bytes. For this +// reason we have to keep the block size below this limit for the nvmem tests. #define TEST_NVMEM_BLOCK_SIZE 255 #define TEST_NVMEM_KEY_SIZE 32 @@ -73,7 +75,7 @@ void test_register_multiple_blocks() { setup(); printf("Test nvmem register multiple blocks...\n"); - size_t num_blocks = TEST_NVMEM_NUM_BLOCKS; + int num_blocks = TEST_NVMEM_NUM_BLOCKS; char blocks[num_blocks][TEST_NVMEM_BLOCK_SIZE]; for (int i = 0; i < num_blocks; i++) { char key[TEST_NVMEM_KEY_SIZE]; @@ -86,7 +88,7 @@ void test_register_blocks_over_limit() { setup(); printf("Test nvmem register blocks over the allowed limit...\n"); - size_t num_blocks = TEST_NVMEM_NUM_BLOCKS + 1; + int num_blocks = TEST_NVMEM_NUM_BLOCKS + 1; char blocks[num_blocks][TEST_NVMEM_BLOCK_SIZE]; for (int i = 0; i < num_blocks; i++) { char key[TEST_NVMEM_KEY_SIZE];