Skip to content

Commit e583208

Browse files
committed
anti-replay: use uint32_t for the interface, document endiannes
The counter is a uint32_t so update the interface to reflect that. As it is returned in pin data and is expected to be little-endian when parsed by the pin server, document this and add FIXMEs where the bytes currently aren't swapped for big-endian builds.
1 parent c96d7bd commit e583208

File tree

4 files changed

+22
-11
lines changed

4 files changed

+22
-11
lines changed

main/process/debug_handshake.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ void debug_handshake(void* process_ptr)
9898
JADE_ASSERT(storage_get_counter() == 3);
9999
uint32_t initial_replay_counter = UINT32_MAX;
100100

101-
JADE_ASSERT(storage_get_replay_counter((uint8_t*)&initial_replay_counter));
101+
JADE_ASSERT(storage_get_replay_counter(&initial_replay_counter));
102102
JADE_ASSERT(initial_replay_counter < UINT32_MAX);
103103

104104
jade_process_reply_to_message_ok(process);
@@ -121,7 +121,7 @@ void debug_handshake(void* process_ptr)
121121
}
122122

123123
uint32_t replay_counter = UINT32_MAX;
124-
JADE_ASSERT(storage_get_replay_counter((uint8_t*)&replay_counter));
124+
JADE_ASSERT(storage_get_replay_counter(&replay_counter));
125125
JADE_ASSERT(replay_counter == initial_replay_counter + 2);
126126

127127
if (!keychain_load(aeskey2, sizeof(aeskey2))) {

main/process/pinclient.c

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ typedef struct {
5959
uint8_t cke[EC_PUBLIC_KEY_LEN];
6060
uint8_t privkey[EC_PRIVATE_KEY_LEN];
6161
// Monotonic Forward Replay counter required for v2
62+
// (32 bit unsigned little-endian integer)
6263
uint8_t replay_counter[REPLAY_COUNTER_LEN];
6364
} pin_keys_t;
6465

@@ -207,8 +208,11 @@ static bool generate_ske(pin_keys_t* pinkeys)
207208
return false;
208209
}
209210

210-
if (wally_hmac_sha256(
211-
pinkeys->cke, EC_PUBLIC_KEY_LEN, pinkeys->replay_counter, REPLAY_COUNTER_LEN, hmac_tweak, HMAC_SHA256_LEN)
211+
uint8_t counter[sizeof(pinkeys->replay_counter)];
212+
JADE_STATIC_ASSERT(sizeof(counter) == REPLAY_COUNTER_LEN);
213+
// FIXME: counter needs to be byte-swapped if we are big-endian
214+
memcpy(counter, pinkeys->replay_counter, sizeof(counter));
215+
if (wally_hmac_sha256(pinkeys->cke, EC_PUBLIC_KEY_LEN, counter, sizeof(counter), hmac_tweak, HMAC_SHA256_LEN)
212216
!= WALLY_OK) {
213217
return false;
214218
}
@@ -308,7 +312,12 @@ static pinserver_result_t generate_ephemeral_pinkeys(pin_keys_t* pinkeys)
308312
}
309313

310314
// Load the replay counter and deduce the server key via tweak
311-
const bool res = storage_get_replay_counter(pinkeys->replay_counter);
315+
uint32_t counter;
316+
const bool res = storage_get_replay_counter(&counter);
317+
if (res) {
318+
// FIXME: counter needs to be byte-swapped if we are big-endian
319+
memcpy(pinkeys->replay_counter, &counter, sizeof(counter));
320+
}
312321
if (!res || !generate_ske(pinkeys)) {
313322
JADE_LOGE("Failed to deduce ephemeral server key");
314323
RETURN_RESULT(

main/storage.c

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -494,10 +494,11 @@ uint8_t storage_get_counter(void)
494494
return read_blob_fixed(DEFAULT_NAMESPACE, PIN_COUNTER_FIELD, &counter, sizeof(counter)) ? counter : 0;
495495
}
496496

497-
bool storage_get_replay_counter(uint8_t* replay_counter)
497+
bool storage_get_replay_counter(uint32_t* replay_counter)
498498
{
499-
// returns the latest counter and increments the one on flash for next use
500-
// if a counter is not set we create one set to 0
499+
// returns the latest counter and increments the one on flash for next use.
500+
// Note that the replay counter is never reset, only incremented.
501+
// if no counter is set (i.e. brand new device), we create one set to 0.
501502
JADE_ASSERT(replay_counter);
502503

503504
nvs_handle handle;
@@ -514,8 +515,9 @@ bool storage_get_replay_counter(uint8_t* replay_counter)
514515
j = 0;
515516
}
516517
JADE_ASSERT(j < UINT32_MAX);
517-
memcpy(replay_counter, &j, sizeof(j));
518-
err = nvs_set_u32(handle, REPLAY_COUNTER_FIELD, ++j);
518+
519+
*replay_counter = j;
520+
err = nvs_set_u32(handle, REPLAY_COUNTER_FIELD, j + 1);
519521
if (err != ESP_OK) {
520522
JADE_LOGE("nvs_set_u32() for %s failed: %u", REPLAY_COUNTER_FIELD, err);
521523
nvs_close(handle);

main/storage.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ bool storage_get_encrypted_blob(uint8_t* encrypted, size_t encrypted_len, size_t
4747
bool storage_decrement_counter(void);
4848
bool storage_restore_counter(void);
4949
uint8_t storage_get_counter(void);
50-
bool storage_get_replay_counter(uint8_t* replay_counter);
50+
bool storage_get_replay_counter(uint32_t* replay_counter);
5151
bool storage_erase_encrypted_blob(void);
5252

5353
bool storage_set_key_flags(uint8_t flags);

0 commit comments

Comments
 (0)