From 77ea1b896561a3e7bf8515dc751c0e9b0388627e Mon Sep 17 00:00:00 2001 From: James Haggerty Date: Wed, 10 Apr 2024 12:34:43 +1000 Subject: [PATCH] Reduce totp_face_lfs memory usage --- movement/lib/TOTP/TOTP.h | 2 +- .../watch_faces/complication/totp_face_lfs.c | 85 ++++++++++++++----- .../watch_faces/complication/totp_face_lfs.h | 3 +- 3 files changed, 65 insertions(+), 25 deletions(-) diff --git a/movement/lib/TOTP/TOTP.h b/movement/lib/TOTP/TOTP.h index 4b5897e..adde66d 100644 --- a/movement/lib/TOTP/TOTP.h +++ b/movement/lib/TOTP/TOTP.h @@ -4,7 +4,7 @@ #include #include "time.h" -typedef enum { +typedef enum __attribute__ ((__packed__)) { SHA1, SHA224, SHA256, diff --git a/movement/watch_faces/complication/totp_face_lfs.c b/movement/watch_faces/complication/totp_face_lfs.c index 820ad52..6666862 100644 --- a/movement/watch_faces/complication/totp_face_lfs.c +++ b/movement/watch_faces/complication/totp_face_lfs.c @@ -35,29 +35,41 @@ #include "totp_face_lfs.h" -#define MAX_TOTP_RECORDS 20 -#define MAX_TOTP_SECRET_SIZE 48 +#define MAX_TOTP_RECORDS 30 +#define MAX_TOTP_SECRET_SIZE 128 #define TOTP_FILE "totp_uris.txt" const char* TOTP_URI_START = "otpauth://totp/"; struct totp_record { - uint8_t *secret; - size_t secret_size; char label[2]; - uint32_t period; hmac_alg algorithm; + uint8_t period; + uint8_t secret_size; + + union { + uint8_t *secret; + struct { + uint16_t file_secret_offset; + uint16_t file_secret_length; + }; + }; }; +/* This is used if we're not storing all the secrets but instead + * calculating them on demand. Avoids malloc in normal operation. + */ +static uint8_t current_secret[MAX_TOTP_SECRET_SIZE]; + static struct totp_record totp_records[MAX_TOTP_RECORDS]; -static int num_totp_records = 0; +static uint8_t num_totp_records = 0; static void init_totp_record(struct totp_record *totp_record) { - totp_record->secret_size = 0; totp_record->label[0] = 'A'; totp_record->label[1] = 'A'; - totp_record->period = 30; totp_record->algorithm = SHA1; + totp_record->period = 30; + totp_record->secret_size = 0; } static bool totp_face_lfs_read_param(struct totp_record *totp_record, char *param, char *value) { @@ -69,14 +81,13 @@ static bool totp_face_lfs_read_param(struct totp_record *totp_record, char *para totp_record->label[0] = value[0]; totp_record->label[1] = value[1]; } else if (!strcmp(param, "secret")) { - if (UNBASE32_LEN(strlen(value)) > MAX_TOTP_SECRET_SIZE) { + totp_record->file_secret_length = strlen(value); + if (UNBASE32_LEN(totp_record->file_secret_length) > MAX_TOTP_SECRET_SIZE) { printf("TOTP secret too long: %s\n", value); return false; } - totp_record->secret = malloc(UNBASE32_LEN(strlen(value))); - totp_record->secret_size = base32_decode((unsigned char *)value, totp_record->secret); + totp_record->secret_size = base32_decode((unsigned char *)value, current_secret); if (totp_record->secret_size == 0) { - free(totp_record->secret); printf("TOTP can't decode secret: %s\n", value); return false; } @@ -126,8 +137,8 @@ static void totp_face_lfs_read_file(char *filename) { } char line[256]; - int32_t offset = 0; - while (filesystem_read_line(filename, line, &offset, 255) && strlen(line)) { + int32_t offset = 0, old_offset = 0; + while (old_offset = offset, filesystem_read_line(filename, line, &offset, 255) && strlen(line)) { if (num_totp_records == MAX_TOTP_RECORDS) { printf("TOTP max records: %d\n", MAX_TOTP_RECORDS); break; @@ -155,7 +166,13 @@ static void totp_face_lfs_read_file(char *filename) { do { char *param_middle = strchr(param, '='); *param_middle = '\0'; - error = error || !totp_face_lfs_read_param(&totp_records[num_totp_records], param, param_middle + 1); + if (totp_face_lfs_read_param(&totp_records[num_totp_records], param, param_middle + 1)) { + if (!strcmp(param, "secret")) { + totp_records[num_totp_records].file_secret_offset = old_offset + (param_middle + 1 - line); + } + } else { + error = true; + } } while ((param = strtok_r(NULL, "&", ¶m_saveptr))); if (error) { @@ -186,15 +203,42 @@ void totp_face_lfs_setup(movement_settings_t *settings, uint8_t watch_face_index #endif } +static uint8_t *totp_face_lfs_get_file_secret(struct totp_record *record) { + char buffer[BASE32_LEN(MAX_TOTP_SECRET_SIZE) + 1]; + int32_t file_secret_offset = record->file_secret_offset; + + /* TODO filesystem_read_line is quite inefficient. Consider writing a new function, + * and keeping the file open? + */ + if (!filesystem_read_line(TOTP_FILE, buffer, &file_secret_offset, record->file_secret_length + 1)) { + /* Shouldn't happen at this point. Return current_secret, which is misleading but will not cause a crash. */ + printf("TOTP can't read expected secret from totp_uris.txt (failed readline)\n"); + return current_secret; + } + if (base32_decode((unsigned char *)buffer, current_secret) != record->secret_size) { + printf("TOTP can't properly decode secret from totp_uris.txt; failed at offset %d; read to %d\n", buffer, record->file_secret_offset, file_secret_offset); + } + return current_secret; +} + static void totp_face_set_record(totp_lfs_state_t *totp_state, int i) { + struct totp_record *record; + if (num_totp_records == 0 && i >= num_totp_records) { return; } totp_state->current_index = i; - TOTP(totp_records[i].secret, totp_records[i].secret_size, totp_records[i].period, totp_records[i].algorithm); + record = &totp_records[i]; + + TOTP( + totp_face_lfs_get_file_secret(record), + record->secret_size, + record->period, + record->algorithm + ); totp_state->current_code = getCodeFromTimestamp(totp_state->timestamp); - totp_state->steps = totp_state->timestamp / totp_records[i].period; + totp_state->steps = totp_state->timestamp / record->period; } void totp_face_lfs_activate(movement_settings_t *settings, void *context) { @@ -256,12 +300,7 @@ bool totp_face_lfs_loop(movement_event_t event, movement_settings_t *settings, v totp_face_display(totp_state); break; case EVENT_LIGHT_BUTTON_UP: - if (totp_state->current_index - 1 >= 0) { - totp_face_set_record(totp_state, totp_state->current_index - 1); - } else { - // Wrap around to the last record. - totp_face_set_record(totp_state, num_totp_records - 1); - } + totp_face_set_record(totp_state, (totp_state->current_index + num_totp_records - 1) % num_totp_records); totp_face_display(totp_state); break; case EVENT_ALARM_BUTTON_DOWN: diff --git a/movement/watch_faces/complication/totp_face_lfs.h b/movement/watch_faces/complication/totp_face_lfs.h index 72ae246..67dcd0b 100644 --- a/movement/watch_faces/complication/totp_face_lfs.h +++ b/movement/watch_faces/complication/totp_face_lfs.h @@ -35,7 +35,8 @@ * otpauth://totp/ACME%20Co:john.doe@email.com?secret=HXDMVJECJJWSRB3HWIZR4IFUGFTMXBOZ&issuer=ACME%20Co&algorithm=SHA1&digits=6&period=30 * * This is also the same as what Aegis exports in plain-text format. - * This face performs minimal sanitisation of input, however. + * This face performs minimal sanitisation of input, however, and you + * will only see errors if you use the simulator or view the serial console. * * At the moment, to get the records onto the filesystem, start a serial connection and do: * echo otpauth://totp/Example:alice@google.com?secret=JBSWY3DPEHPK3PXP&issuer=Example > totp_uris.txt