Skip to content

Commit

Permalink
Local copy of APDU buffer for processing (#280)
Browse files Browse the repository at this point in the history
System module now keeps an enclave-only copy of the APDU buffer and
performs a copy from the host APDU buffer before processing and a
copy back to the host APDU buffer after processing is done.
In this way we prevent double buffer fetching.

Updated unit tests.
  • Loading branch information
amendelzon authored Jan 23, 2025
1 parent 799ea32 commit 551acd6
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 16 deletions.
36 changes: 21 additions & 15 deletions firmware/src/sgx/src/trusted/system.c
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,12 @@
#include "bc_err.h"

/**
* APDU buffer
* APDU buffer (host pointer and local enclave copy)
*/
#define EXPECTED_APDU_BUFFER_SIZE 85
static unsigned char* apdu_buffer;
static size_t apdu_buffer_size;
#define APDU_BUFFER_SIZE 85

static unsigned char* host_apdu_buffer;
static unsigned char apdu_buffer[APDU_BUFFER_SIZE];

static void wipe_system() {
seed_wipe();
Expand All @@ -50,7 +51,7 @@ static unsigned int do_onboard(unsigned int rx) {
}

// Onboarding
uint8_t tmp_buffer[apdu_buffer_size];
uint8_t tmp_buffer[sizeof(apdu_buffer)];
memcpy(tmp_buffer, APDU_DATA_PTR, SEED_LENGTH);
if (!seed_generate(tmp_buffer, SEED_LENGTH)) {
wipe_system();
Expand All @@ -75,7 +76,7 @@ static unsigned int do_change_password(unsigned int rx) {
}

// Password change
uint8_t tmp_buffer[apdu_buffer_size];
uint8_t tmp_buffer[sizeof(apdu_buffer)];
size_t password_length = APDU_DATA_SIZE(rx);
memcpy(tmp_buffer, APDU_DATA_PTR, password_length);
if (!access_set_password((char*)tmp_buffer, password_length)) {
Expand Down Expand Up @@ -160,27 +161,32 @@ static external_processor_result_t system_do_process_apdu(unsigned int rx) {
}

unsigned int system_process_apdu(unsigned int rx) {
return hsm_process_apdu(rx);
// Copy host APDU => enclave APDU
memcpy(apdu_buffer, host_apdu_buffer, sizeof(apdu_buffer));
unsigned int tx = hsm_process_apdu(rx);
// Copy enclave APDU => host APDU
memcpy(host_apdu_buffer, apdu_buffer, sizeof(apdu_buffer));
return tx;
}

bool system_init(unsigned char* msg_buffer, size_t msg_buffer_size) {
// Setup the shared APDU buffer
if (msg_buffer_size != EXPECTED_APDU_BUFFER_SIZE) {
LOG("Expected APDU buffer size to be %u but got %lu\n",
EXPECTED_APDU_BUFFER_SIZE,
// Validate that host and enclave APDU buffers have the same size
if (msg_buffer_size != sizeof(apdu_buffer)) {
LOG("Expected APDU buffer size to be %lu but got %lu\n",
sizeof(apdu_buffer),
msg_buffer_size);
return false;
}

// Validate that the APDU buffer is entirely outside the enclave
// Validate that the host APDU buffer is entirely outside the enclave
// memory space
if (!oe_is_outside_enclave(msg_buffer, msg_buffer_size)) {
LOG("APDU buffer memory area not outside the enclave\n");
return false;
}

apdu_buffer = msg_buffer;
apdu_buffer_size = msg_buffer_size;
// Set the pointer to the host APDU buffer
host_apdu_buffer = msg_buffer;

// Initialize modules
LOG("Initializing modules...\n");
Expand Down Expand Up @@ -209,7 +215,7 @@ bool system_init(unsigned char* msg_buffer, size_t msg_buffer_size) {
LOG("System wiped\n");
}

if (!communication_init(apdu_buffer, apdu_buffer_size)) {
if (!communication_init(apdu_buffer, sizeof(apdu_buffer))) {
LOG("Error initializing communication module\n");
return false;
}
Expand Down
5 changes: 4 additions & 1 deletion firmware/src/sgx/test/system/test_system.c
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,7 @@ bc_state_updating_backup_t N_bc_state_updating_backup_var;
static try_context_t G_try_last_open_context_var;
try_context_t* G_try_last_open_context = &G_try_last_open_context_var;
unsigned char G_io_apdu_buffer[IO_APDU_BUFFER_SIZE];
unsigned char* G_communication_msg_buffer;

// Mock implementation of dependencies
bool oe_is_outside_enclave(const void* ptr, size_t size) {
Expand Down Expand Up @@ -212,12 +213,14 @@ bool access_set_password(char* password, uint8_t password_length) {
}

bool communication_init(unsigned char* msg_buffer, size_t msg_buffer_size) {
G_communication_msg_buffer = msg_buffer;
assert(msg_buffer_size == sizeof(G_io_apdu_buffer));
MOCK_CALL(communication_init);
return true;
}

unsigned char* communication_get_msg_buffer() {
return G_io_apdu_buffer;
return G_communication_msg_buffer;
}

uint8_t access_get_retries() {
Expand Down

0 comments on commit 551acd6

Please sign in to comment.