Skip to content

Commit

Permalink
faces/totp: remove dynamic memory allocation
Browse files Browse the repository at this point in the history
Allocate an unlimited extent 128 byte buffer once during setup
instead of allocating and deallocating repeatedly. A static buffer
was not used because it fails to be reentrant and prevents multiple
instances of the watch face to be compiled by the user.

The advantage is the complete prevention of memory management errors,
improving the reliability of the watch. It also eliminates the overhead
of the memory allocator itself since malloc is not free.
The disadvantage is a worst case default size of 128 bytes was required,
meaning about 90 bytes will be wasted in the common case since most keys
are not that big. This can be overridden by the user via preprocessor.

The key lengths are checked on TOTP watch face initialization
and if any key is found to be too large to fit the buffer
it is turned off and the label and ERROR is displayed instead.

The base32 encoded secrets are decoded dynamically to the buffer
at the following times:

 - Face enters the foreground
 - User switches TOTP code

Therefore, there is still some extra runtime overhead
that can still be eliminated by code generation.
This will be addressed in future commits.

Tested-by: Matheus Afonso Martins Moreira <[email protected]>
Tested-on-hardware-by: madhogs <[email protected]>
Signed-off-by: Matheus Afonso Martins Moreira <[email protected]>
GitHub-Pull-Request: #385
  • Loading branch information
matheusmoreira committed Aug 26, 2024
1 parent 09f4618 commit d8b3be6
Showing 1 changed file with 58 additions and 39 deletions.
97 changes: 58 additions & 39 deletions movement/watch_faces/complication/totp_face.c
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,10 @@
#include "TOTP.h"
#include "base32.h"

#ifndef TOTP_FACE_MAX_KEY_LENGTH
#define TOTP_FACE_MAX_KEY_LENGTH 128
#endif

typedef struct {
unsigned char labels[2];
hmac_alg algorithm;
Expand Down Expand Up @@ -67,44 +71,45 @@ static totp_t credentials[] = {
// END OF KEY DATA.
////////////////////////////////////////////////////////////////////////////////

static inline totp_t *totp_at(size_t i) {
return &credentials[i];
}

static inline totp_t *totp_current(totp_state_t *totp_state) {
return &credentials[totp_state->current_index];
return totp_at(totp_state->current_index);
}

static inline size_t totp_total(void) {
return sizeof(credentials) / sizeof(*credentials);
}

static void totp_face_free_current_secret(totp_state_t *totp_state) {
if (totp_state->current_decoded_key) {
free(totp_state->current_decoded_key);
totp_state->current_decoded_key = 0;
static void totp_validate_key_lengths(void) {
for (size_t n = totp_total(), i = 0; i < n; ++i) {
totp_t *totp = totp_at(i);

if (UNBASE32_LEN(totp->encoded_key_length) > TOTP_FACE_MAX_KEY_LENGTH) {
// Key exceeds static limits, turn it off by zeroing the length
totp->encoded_key_length = 0;
}
}
}

static void totp_face_decode_current_secret(totp_state_t *totp_state) {
static bool totp_generate(totp_state_t *totp_state) {
totp_t *totp = totp_current(totp_state);

totp_state->current_decoded_key = malloc(UNBASE32_LEN(totp->encoded_key_length));
totp_state->current_decoded_key_length = base32_decode(totp->encoded_key, totp_state->current_decoded_key);

if (totp_state->current_decoded_key_length == 0) {
totp_face_free_current_secret(totp_state);
if (totp->encoded_key_length <= 0) {
// Key exceeded static limits and was turned off
return false;
}
}

static bool totp_generate(totp_state_t *totp_state) {
if (!totp_state->current_decoded_key) {
totp_face_decode_current_secret(totp_state);
}
totp_state->current_decoded_key_length = base32_decode(totp->encoded_key, totp_state->current_decoded_key);

if (!totp_state->current_decoded_key) {
watch_display_string("ERROR", 4);
if (totp_state->current_decoded_key_length == 0) {
// Decoding failed for some reason
// Not a base 32 string?
return false;
}

totp_t *totp = totp_current(totp_state);

TOTP(
totp_state->current_decoded_key,
totp_state->current_decoded_key_length,
Expand All @@ -115,6 +120,13 @@ static bool totp_generate(totp_state_t *totp_state) {
return true;
}

static void totp_display_error(totp_state_t *totp_state) {
char buf[9 + 1];
totp_t *totp = totp_current(totp_state);

snprintf(buf, sizeof(buf), "%c%c ERROR", totp->labels[0], totp->labels[1]);
}

static void totp_display(totp_state_t *totp_state) {
char buf[14];
div_t result;
Expand All @@ -132,27 +144,44 @@ static void totp_display(totp_state_t *totp_state) {
watch_display_string(buf, 0);
}

static void totp_generate_and_display(totp_state_t *totp_state) {
if (totp_generate(totp_state)) {
totp_display(totp_state);
} else {
totp_display_error(totp_state);
}
}

static inline uint32_t totp_compute_base_timestamp(movement_settings_t *settings) {
return watch_utility_date_time_to_unix_time(watch_rtc_get_date_time(), movement_timezone_offsets[settings->bit.time_zone] * 60);
}

void totp_face_setup(movement_settings_t *settings, uint8_t watch_face_index, void ** context_ptr) {
(void) settings;
(void) watch_face_index;

totp_validate_key_lengths();

if (*context_ptr == NULL) {
totp_state_t *totp = malloc(sizeof(totp_state_t));
totp->current_decoded_key = malloc(TOTP_FACE_MAX_KEY_LENGTH);
*context_ptr = totp;
}
}

void totp_face_activate(movement_settings_t *settings, void *context) {
(void) settings;

totp_state_t *totp_state = (totp_state_t *) context;
memset(totp_state, 0, sizeof(*totp_state));
totp_state_t *totp = (totp_state_t *) context;

totp_state->timestamp = watch_utility_date_time_to_unix_time(watch_rtc_get_date_time(), movement_timezone_offsets[settings->bit.time_zone] * 60);
totp->timestamp = totp_compute_base_timestamp(settings);
totp->steps = 0;
totp->current_code = 0;
totp->current_index = 0;
totp->current_decoded_key_length = 0;
// totp->current_decoded_key is already initialized in setup

if (totp_generate(totp_state)) {
totp_display(totp_state);
}
totp_generate_and_display(totp);
}

bool totp_face_loop(movement_event_t event, movement_settings_t *settings, void *context) {
Expand All @@ -171,33 +200,25 @@ bool totp_face_loop(movement_event_t event, movement_settings_t *settings, void
movement_move_to_face(0);
break;
case EVENT_ALARM_BUTTON_UP:
totp_face_free_current_secret(totp_state);

if (totp_state->current_index + 1 < totp_total()) {
totp_state->current_index++;
} else {
// wrap around to first key
totp_state->current_index = 0;
}

if (totp_generate(totp_state)) {
totp_display(totp_state);
}
totp_generate_and_display(totp_state);

break;
case EVENT_LIGHT_BUTTON_UP:
totp_face_free_current_secret(totp_state);

if (totp_state->current_index == 0) {
// Wrap around to the last credential.
totp_state->current_index = totp_total() - 1;
} else {
totp_state->current_index--;
}

if (totp_generate(totp_state)) {
totp_display(totp_state);
}
totp_generate_and_display(totp_state);

break;
case EVENT_ALARM_BUTTON_DOWN:
Expand All @@ -217,7 +238,5 @@ bool totp_face_loop(movement_event_t event, movement_settings_t *settings, void

void totp_face_resign(movement_settings_t *settings, void *context) {
(void) settings;

totp_state_t *totp_state = (totp_state_t *) context;
totp_face_free_current_secret(totp_state);
(void) context;
}

0 comments on commit d8b3be6

Please sign in to comment.