Skip to content

Commit

Permalink
faces/totp: improve memory usage
Browse files Browse the repository at this point in the history 
The TOTP face is working in the simulator but fails on the real hardware
when loaded with lots of codes, just like the LFS version.
This is likely caused by the recent refactoring of the TOTP face
which introduced a declarative credential interface for ease of use.
That's accomplished by decoding the secrets at runtime which increases
the RAM requirements. Users are likely hitting memory limits.

In order to mitigate this, the algorithm is changed from decoding
all of the secrets only once during initialization to on the fly
decoding of the secret for the current TOTP credential only.
This converts this face's dynamic memory usage from O(N) to O(1)
at the cost of memory management when switching faces and credentials
which could impact power consumption.

Due to variable key sizes, the memory cannot be statically allocated.
Perhaps there's a maximum key size that can serve as worst case?

Also took this opportunity to restructure the code a bit.

Reported-by: madhogs <[email protected]>
Fixed-by: Matheus Afonso Martins Moreira <[email protected]>
Tested-by: Matheus Afonso Martins Moreira <[email protected]>
Tested-on-hardware-by: madhogs <[email protected]>
Signed-off-by: Matheus Afonso Martins Moreira <[email protected]>
Fixes: joeycastillo#384
  • Loading branch information
@matheusmoreira
matheusmoreira committed Mar 17, 2024
1 parent 955ac94 commit d82b670
Show file tree
Hide file tree
Showing 2 changed files with 73 additions and 34 deletions.
105 changes: 71 additions & 34 deletions movement/watch_faces/complication/totp_face.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,14 +43,14 @@ typedef struct {
unsigned char labels[2];
hmac_alg algorithm;
uint32_t period;
size_t key_length;
unsigned char *key;
size_t encoded_key_length;
unsigned char *encoded_key;
} totp_t;

#define CREDENTIAL(label, key_array, algo, timestep) \
(const totp_t) { \
.key = ((unsigned char *) key_array), \
.key_length = sizeof(key_array) - 1, \
.encoded_key = ((unsigned char *) key_array), \
.encoded_key_length = sizeof(key_array) - 1, \
.period = (timestep), \
.labels = (#label), \
.algorithm = (algo), \
Expand All @@ -75,6 +75,46 @@ 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_face_decode_current_secret(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);
}
}

static bool totp_generate(totp_state_t *totp_state) {
if (!totp_state->current_decoded_key) {
totp_face_decode_current_secret(totp_state);
}

if (!totp_state->current_decoded_key) {
watch_display_string("ERROR", 4);
return false;
}

totp_t *totp = totp_current(totp_state);

TOTP(
totp_state->current_decoded_key,
totp_state->current_decoded_key_length,
totp->period,
totp->algorithm
);

return true;
}

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

static void totp_face_decode_secrets(void) {
for (size_t n = totp_total(), i = 0; i < n; ++i) {
totp_t *totp = &credentials[i];
unsigned char *key = totp->key;

totp->key = malloc(UNBASE32_LEN(totp->key_length));
totp->key_length = base32_decode(key, totp->key);

if (totp->key_length == 0) {
free(totp->key);
continue;
}
}
}

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

if (*context_ptr == NULL) {
totp_state_t *totp = malloc(sizeof(totp_state_t));
totp_face_decode_secrets();
*context_ptr = totp;
}
}

void totp_face_activate(movement_settings_t *settings, void *context) {
(void) settings;
memset(context, 0, sizeof(totp_state_t));
totp_state_t *totp_state = (totp_state_t *)context;
totp_t *totp = totp_current(totp_state);
TOTP(totp->key, totp->key_length, totp->period, totp->algorithm);

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

totp_state->timestamp = watch_utility_date_time_to_unix_time(watch_rtc_get_date_time(), movement_timezone_offsets[settings->bit.time_zone] * 60);
totp_state->current_code = getCodeFromTimestamp(totp_state->timestamp);

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

bool totp_face_loop(movement_event_t event, movement_settings_t *settings, void *context) {
(void) settings;
totp_state_t *totp_state = (totp_state_t *)context;
totp_t *totp;

totp_state_t *totp_state = (totp_state_t *) context;

switch (event.event_type) {
case EVENT_TICK:
Expand All @@ -144,26 +171,34 @@ 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;
}
totp = totp_current(totp_state);
TOTP(totp->key, totp->key_length, totp->period, totp->algorithm);
totp_display(totp_state);

if (totp_generate(totp_state)) {
totp_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--;
}
totp = totp_current(totp_state);
TOTP(totp->key, totp->key_length, totp->period, totp->algorithm);
totp_display(totp_state);

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

break;
case EVENT_ALARM_BUTTON_DOWN:
case EVENT_ALARM_LONG_PRESS:
Expand All @@ -182,5 +217,7 @@ 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;
(void) context;

totp_state_t *totp_state = (totp_state_t *) context;
totp_face_free_current_secret(totp_state);
}
2 changes: 2 additions & 0 deletions movement/watch_faces/complication/totp_face.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,8 @@ typedef struct {
uint8_t steps;
uint32_t current_code;
uint8_t current_index;
uint8_t *current_decoded_key;
size_t current_decoded_key_length;
} totp_state_t;

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

0 comments on commit d82b670

Please sign in to comment.