diff --git a/mbedtls_config.h b/mbedtls_config.h index c8638af..30b616f 100644 --- a/mbedtls_config.h +++ b/mbedtls_config.h @@ -11,4 +11,14 @@ #define MBEDTLS_SHA1_C #define MBEDTLS_MD_C +// H6: authenticated-encrypted backups (AES-256-GCM payload, PBKDF2-HMAC-SHA256 +// key derivation from an operator passphrase). GCM_C pulls in the cipher layer +// (mbedtls_gcm_setkey -> mbedtls_cipher_setup), which needs CIPHER_C + AES_C; +// PKCS5_C provides mbedtls_pkcs5_pbkdf2_hmac_ext and needs MD_C (above) and the +// already-enabled SHA-256. Verify the resulting firmware image size on device. +#define MBEDTLS_AES_C +#define MBEDTLS_CIPHER_C +#define MBEDTLS_GCM_C +#define MBEDTLS_PKCS5_C + #endif \ No newline at end of file diff --git a/serial/commands_backup.c b/serial/commands_backup.c index 31adaa9..9027a0b 100644 --- a/serial/commands_backup.c +++ b/serial/commands_backup.c @@ -7,14 +7,54 @@ #include #include -void cmd_export_keys(int argc, char **argv) { +#define BACKUP_PASSPHRASE_MAX 64 + +// --------------------------------------------------------------------------- +// Read one line from the console into buf (NUL-terminated, CR/LF stripped). +// Returns the length, or -1 on timeout with nothing entered. +// --------------------------------------------------------------------------- + +static int read_line(char *buf, size_t cap, uint32_t timeout_ms) { + size_t len = 0; + absolute_time_t deadline = make_timeout_time_ms(timeout_ms); + while (!time_reached(deadline)) { + int c = getchar_timeout_us(10000); + if (c == PICO_ERROR_TIMEOUT) + continue; + if (c == '\r') + continue; + if (c == '\n') + break; + if (len < cap - 1) + buf[len++] = (char)c; + } + buf[len] = '\0'; + return (len == 0 && time_reached(deadline)) ? -1 : (int)len; +} + +// --------------------------------------------------------------------------- +// Per-key admin confirmation prompt (backup_admin_confirm_fn). +// --------------------------------------------------------------------------- + +static bool confirm_admin_prompt(uint16_t id, const char *name, void *ctx) { + (void)ctx; + printf("[import] key %u \"%s\" grants ADMIN. import as admin? type yes:\r\n", id, name); + char answer[8]; + read_line(answer, sizeof(answer), 30000); + return strcmp(answer, "yes") == 0; +} + +// --------------------------------------------------------------------------- +// Emit an encrypted backup of the current key set under `passphrase`. +// --------------------------------------------------------------------------- + +static void emit_backup(const char *passphrase) { static uint8_t export_buf[sizeof(backup_header_t) + BACKUP_MAX_KEYS * sizeof(backup_key_t)]; static char b64_buf[BASE64_ENCODED_LEN(sizeof(export_buf))]; - int len = backup_export(export_buf, sizeof(export_buf)); + int len = backup_export(export_buf, sizeof(export_buf), passphrase); if (len < 0) { printf("error: export failed\r\n"); - buzzer_play_command_ack(); return; } @@ -22,18 +62,45 @@ void cmd_export_keys(int argc, char **argv) { printf("--- BEGIN HSLOCK BACKUP ---\r\n"); printf("%s\r\n", b64_buf); printf("--- END HSLOCK BACKUP ---\r\n"); +} + +void cmd_export_keys(int argc, char **argv) { + (void)argc; + (void)argv; + + printf("enter backup passphrase (encrypts the blob):\r\n"); + char passphrase[BACKUP_PASSPHRASE_MAX]; + if (read_line(passphrase, sizeof(passphrase), 60000) <= 0 || passphrase[0] == '\0') { + printf("error: no passphrase entered\r\n"); + buzzer_play_command_ack(); + return; + } + + emit_backup(passphrase); + memset(passphrase, 0, sizeof(passphrase)); buzzer_play_command_ack(); } void cmd_import_keys(int argc, char **argv) { - // Backup first + (void)argc; + (void)argv; + + printf("enter backup passphrase (decrypts the blob):\r\n"); + char passphrase[BACKUP_PASSPHRASE_MAX]; + if (read_line(passphrase, sizeof(passphrase), 60000) <= 0 || passphrase[0] == '\0') { + printf("error: no passphrase entered\r\n"); + buzzer_play_command_ack(); + return; + } + + // Safety backup of the current key set (encrypted under the same passphrase) + // before anything is overwritten. printf("backing up current keys...\r\n"); - cmd_export_keys(0, NULL); + emit_backup(passphrase); printf("paste import data, then send empty line:\r\n"); - // Read base64 directly from serial into a large static buffer static char b64_buf[BASE64_ENCODED_LEN(sizeof(backup_header_t) + BACKUP_MAX_KEYS * sizeof(backup_key_t))]; int b64_len = 0; @@ -66,6 +133,7 @@ void cmd_import_keys(int argc, char **argv) { if (b64_len == 0) { printf("error: no data received\r\n"); + memset(passphrase, 0, sizeof(passphrase)); buzzer_play_command_ack(); return; } @@ -75,20 +143,18 @@ void cmd_import_keys(int argc, char **argv) { int len = base64_decode(b64_buf, b64_len, import_buf); if (len < 0) { printf("error: invalid base64\r\n"); + memset(passphrase, 0, sizeof(passphrase)); buzzer_play_command_ack(); return; } - // Export current keys as backup before overwriting - printf("backing up current keys...\r\n"); - cmd_export_keys(0, NULL); - printf("importing...\r\n"); - if (backup_import(import_buf, (size_t)len)) { + if (backup_import(import_buf, (size_t)len, passphrase, confirm_admin_prompt, NULL)) { printf("import ok\r\n"); } else { printf("error: import failed\r\n"); } + memset(passphrase, 0, sizeof(passphrase)); buzzer_play_command_ack(); } diff --git a/storage/backup.c b/storage/backup.c index ef42a3d..5411f12 100644 --- a/storage/backup.c +++ b/storage/backup.c @@ -1,22 +1,83 @@ #include "backup.h" #include "storage.h" #include "lfs_util.h" +#include "pico/rand.h" +#include +#include +#include +#include + +#include #include #include // --------------------------------------------------------------------------- -// Checksum helper +// Scratch plaintext buffer (the serialised records, in the clear). Kept in BSS +// rather than on the stack: BACKUP_MAX_KEYS records is several KiB and this is +// core-0-only console work. +// --------------------------------------------------------------------------- + +#define BACKUP_PLAIN_MAX (BACKUP_MAX_KEYS * sizeof(backup_key_t)) + +static uint8_t plain_scratch[BACKUP_PLAIN_MAX]; + +// --------------------------------------------------------------------------- +// Checksum helper (paste-corruption hint over the ciphertext only) +// --------------------------------------------------------------------------- + +static uint32_t backup_checksum(const uint8_t *ciphertext, size_t len) { + return lfs_crc(0xFFFFFFFF, ciphertext, len); +} + +// --------------------------------------------------------------------------- +// Fill n random bytes from the platform CSPRNG. // --------------------------------------------------------------------------- -static uint32_t backup_checksum(const backup_key_t *keys, uint32_t count) { - uint32_t crc = 0xFFFFFFFF; - crc = lfs_crc(crc, keys, count * sizeof(backup_key_t)); - return crc; +static void fill_random(uint8_t *out, size_t n) { + size_t i = 0; + while (i < n) { + uint64_t r = get_rand_64(); + size_t chunk = (n - i < sizeof(r)) ? (n - i) : sizeof(r); + memcpy(out + i, &r, chunk); + i += chunk; + } } // --------------------------------------------------------------------------- -// logic model → backup key (fresh checksum) +// Derive the 32-byte AEAD key from passphrase + salt (PBKDF2-HMAC-SHA256). +// Returns 0 on success. +// --------------------------------------------------------------------------- + +static int derive_key(const char *passphrase, const uint8_t *salt, uint8_t key[BACKUP_KEY_LEN]) { + if (passphrase == NULL || passphrase[0] == '\0') + return -1; + // PBKDF2-HMAC-SHA256. mbedtls_pkcs5_pbkdf2_hmac_ext exists only in 3.6+ (the + // context-based form it replaces is deprecated there); pre-3.6 (some CI hosts + // / pico-sdk snapshots) has only the context form. Pick per version so the + // build is warning-clean on both. +#if defined(MBEDTLS_VERSION_NUMBER) && MBEDTLS_VERSION_NUMBER >= 0x03060000 + return mbedtls_pkcs5_pbkdf2_hmac_ext(MBEDTLS_MD_SHA256, (const unsigned char *)passphrase, + strlen(passphrase), salt, BACKUP_SALT_LEN, + BACKUP_PBKDF2_ITERS, BACKUP_KEY_LEN, key); +#else + const mbedtls_md_info_t *md = mbedtls_md_info_from_type(MBEDTLS_MD_SHA256); + if (md == NULL) + return -1; + mbedtls_md_context_t ctx; + mbedtls_md_init(&ctx); + int rc = mbedtls_md_setup(&ctx, md, 1 /* HMAC */); + if (rc == 0) + rc = mbedtls_pkcs5_pbkdf2_hmac(&ctx, (const unsigned char *)passphrase, strlen(passphrase), + salt, BACKUP_SALT_LEN, BACKUP_PBKDF2_ITERS, BACKUP_KEY_LEN, + key); + mbedtls_md_free(&ctx); + return rc; +#endif +} + +// --------------------------------------------------------------------------- +// logic model <-> backup key // --------------------------------------------------------------------------- static backup_key_t to_backup_key(const key_record_t *k) { @@ -30,16 +91,12 @@ static backup_key_t to_backup_key(const key_record_t *k) { return b; } -// --------------------------------------------------------------------------- -// backup key → logic model -// --------------------------------------------------------------------------- - static key_record_t to_key_record(const backup_key_t *b) { key_record_t k; k.id = b->id; - // The backup blob is untrusted input: its is_enabled/is_admin bytes may hold - // any value, so read them as raw bytes and canonicalise. Loading a `bool` - // whose object representation is not 0/1 is undefined behaviour. + // The decrypted payload is still untrusted structurally (a passphrase holder + // may craft arbitrary bytes): read the flag bytes raw and canonicalise. + // Loading a `bool` whose object representation is not 0/1 is UB. uint8_t enabled_raw, admin_raw; memcpy(&enabled_raw, &b->is_enabled, sizeof(enabled_raw)); memcpy(&admin_raw, &b->is_admin, sizeof(admin_raw)); @@ -54,121 +111,195 @@ static key_record_t to_key_record(const backup_key_t *b) { } // --------------------------------------------------------------------------- -// Export +// Export: serialise, encrypt-then-MAC (AES-256-GCM), emit v2 blob. // --------------------------------------------------------------------------- -int backup_export(uint8_t *buf, size_t buf_size) { +int backup_export(uint8_t *buf, size_t buf_size, const char *passphrase) { static key_record_t records[BACKUP_MAX_KEYS]; int count = storage_key_list(records, BACKUP_MAX_KEYS); if (count < 0) return -1; - size_t needed = sizeof(backup_header_t) + count * sizeof(backup_key_t); - if (buf_size < needed) - return -1; - - // Populate backup keys - backup_key_t *keys = (backup_key_t *)(buf + sizeof(backup_header_t)); + // Serialise the valid records into the plaintext scratch. + backup_key_t *plain = (backup_key_t *)plain_scratch; int exported_key_count = 0; for (int i = 0; i < count; i++) { if (!records[i].is_checksum_valid) { printf("[backup] export: key %u has invalid checksum, skipping\r\n", records[i].id); continue; } - keys[exported_key_count++] = to_backup_key(&records[i]); + plain[exported_key_count++] = to_backup_key(&records[i]); } + size_t cipher_len = (size_t)exported_key_count * sizeof(backup_key_t); + size_t needed = sizeof(backup_header_t) + cipher_len; + if (buf_size < needed) + return -1; + backup_header_t hdr = { .magic = BACKUP_MAGIC, .version = BACKUP_VERSION, .key_count = (uint32_t)exported_key_count, - .checksum = backup_checksum(keys, (uint32_t)exported_key_count), }; + fill_random(hdr.salt, BACKUP_SALT_LEN); + fill_random(hdr.iv, BACKUP_IV_LEN); + + uint8_t key[BACKUP_KEY_LEN]; + if (derive_key(passphrase, hdr.salt, key) != 0) + return -1; + + uint8_t *ciphertext = buf + sizeof(backup_header_t); + + mbedtls_gcm_context gcm; + mbedtls_gcm_init(&gcm); + int rc = mbedtls_gcm_setkey(&gcm, MBEDTLS_CIPHER_ID_AES, key, BACKUP_KEY_LEN * 8); + if (rc == 0) { + // AAD = header prefix up to (excluding) the tag; authenticates + // magic/version/key_count/salt/iv so none can be altered undetected. + rc = mbedtls_gcm_crypt_and_tag(&gcm, MBEDTLS_GCM_ENCRYPT, cipher_len, hdr.iv, BACKUP_IV_LEN, + (const unsigned char *)&hdr, BACKUP_AAD_LEN, plain_scratch, + ciphertext, BACKUP_TAG_LEN, hdr.tag); + } + mbedtls_gcm_free(&gcm); + + // Scrub key material and the cleartext scratch. + memset(key, 0, sizeof(key)); + memset(plain_scratch, 0, cipher_len); + + if (rc != 0) { + printf("[backup] export: encryption failed (%d)\r\n", rc); + return -1; + } + + hdr.checksum = backup_checksum(ciphertext, cipher_len); memcpy(buf, &hdr, sizeof(hdr)); - return (int)(sizeof(backup_header_t) + exported_key_count * sizeof(backup_key_t)); + return (int)needed; } // --------------------------------------------------------------------------- -// Import +// Import: authenticate, decrypt, THEN parse. Never touch storage until the tag +// verifies and every admin record is operator-confirmed. // --------------------------------------------------------------------------- -bool backup_import(const uint8_t *buf, size_t size) { +bool backup_import(const uint8_t *buf, size_t size, const char *passphrase, + backup_admin_confirm_fn confirm_admin, void *confirm_ctx) { if (size < sizeof(backup_header_t)) { printf("[backup] import: buffer too small\r\n"); return false; } - const backup_header_t *hdr = (const backup_header_t *)buf; + backup_header_t hdr; + memcpy(&hdr, buf, sizeof(hdr)); - if (hdr->magic != BACKUP_MAGIC) { + if (hdr.magic != BACKUP_MAGIC) { printf("[backup] import: bad magic\r\n"); return false; } - if (hdr->version != BACKUP_VERSION) { - printf("[backup] import: unsupported version %u\r\n", hdr->version); + if (hdr.version != BACKUP_VERSION) { + printf("[backup] import: unsupported version %u\r\n", hdr.version); return false; } - - if (hdr->key_count > KEY_MAX_COUNT) { - printf("[backup] import: too many keys (%u > %d)\r\n", hdr->key_count, KEY_MAX_COUNT); + if (hdr.key_count > BACKUP_MAX_KEYS) { + printf("[backup] import: too many keys (%u > %d)\r\n", hdr.key_count, BACKUP_MAX_KEYS); return false; } - size_t expected = sizeof(backup_header_t) + hdr->key_count * sizeof(backup_key_t); - if (size < expected) { + size_t cipher_len = (size_t)hdr.key_count * sizeof(backup_key_t); + if (size < sizeof(backup_header_t) + cipher_len) { printf("[backup] import: truncated data\r\n"); return false; } - const backup_key_t *keys = (const backup_key_t *)(buf + sizeof(backup_header_t)); + const uint8_t *ciphertext = buf + sizeof(backup_header_t); + + // CRC is only a paste-corruption hint; the GCM tag is the trust boundary. + if (backup_checksum(ciphertext, cipher_len) != hdr.checksum) + printf("[backup] import: checksum mismatch (possible paste corruption)\r\n"); - // Verify whole-backup checksum - uint32_t expected_crc = backup_checksum(keys, hdr->key_count); - if (hdr->checksum != expected_crc) { - printf("[backup] import: backup checksum mismatch\r\n"); + uint8_t key[BACKUP_KEY_LEN]; + if (derive_key(passphrase, hdr.salt, key) != 0) { + printf("[backup] import: missing/invalid passphrase\r\n"); return false; } - // Validate all names are NUL-terminated before touching storage - for (uint32_t i = 0; i < hdr->key_count; i++) { + backup_key_t *plain = (backup_key_t *)plain_scratch; + + mbedtls_gcm_context gcm; + mbedtls_gcm_init(&gcm); + int rc = mbedtls_gcm_setkey(&gcm, MBEDTLS_CIPHER_ID_AES, key, BACKUP_KEY_LEN * 8); + if (rc == 0) { + rc = mbedtls_gcm_auth_decrypt(&gcm, cipher_len, hdr.iv, BACKUP_IV_LEN, + (const unsigned char *)&hdr, BACKUP_AAD_LEN, hdr.tag, + BACKUP_TAG_LEN, ciphertext, plain_scratch); + } + mbedtls_gcm_free(&gcm); + memset(key, 0, sizeof(key)); + + // MAC failure (tamper / wrong passphrase) -> reject BEFORE any parsing. + if (rc != 0) { + memset(plain_scratch, 0, cipher_len); + printf("[backup] import: authentication failed - rejected\r\n"); + return false; + } + + // Payload is now authentic. Validate structure before touching storage. + for (uint32_t i = 0; i < hdr.key_count; i++) { bool terminated = false; for (int j = 0; j < KEY_NAME_MAX; j++) { - if (keys[i].name[j] == '\0') { + if (plain[i].name[j] == '\0') { terminated = true; break; } } if (!terminated) { - printf("[backup] import: key %u has unterminated name\r\n", keys[i].id); + printf("[backup] import: key %u has unterminated name\r\n", plain[i].id); + memset(plain_scratch, 0, cipher_len); + return false; + } + if (plain[i].id > KEY_ID_MAX) { + printf("[backup] import: key %u has invalid id (max %u)\r\n", plain[i].id, KEY_ID_MAX); + memset(plain_scratch, 0, cipher_len); return false; } } - // Validate all key IDs are in range - for (uint32_t i = 0; i < hdr->key_count; i++) { - if (keys[i].id > KEY_ID_MAX) { - printf("[backup] import: key %u has invalid id (max %u)\r\n", keys[i].id, KEY_ID_MAX); - return false; + // Defense-in-depth: even authenticated, an is_admin record is confirmed + // per-key before any destructive write. A denied record aborts the whole + // import (existing keys untouched). A NULL callback denies all admin keys. + for (uint32_t i = 0; i < hdr.key_count; i++) { + uint8_t admin_raw; + memcpy(&admin_raw, &plain[i].is_admin, sizeof(admin_raw)); + if (admin_raw != 0) { + plain[i].name[KEY_NAME_MAX - 1] = '\0'; + bool ok = confirm_admin && confirm_admin(plain[i].id, plain[i].name, confirm_ctx); + if (!ok) { + printf("[backup] import: admin key %u not confirmed - aborting\r\n", plain[i].id); + memset(plain_scratch, 0, cipher_len); + return false; + } } } - // Checksum valid - delete existing keys + // Authenticated + validated + confirmed: replace the key set. static key_record_t existing[BACKUP_MAX_KEYS]; int existing_count = storage_key_list(existing, BACKUP_MAX_KEYS); - for (int i = 0; i < existing_count; i++) { + for (int i = 0; i < existing_count; i++) storage_key_delete(existing[i].id); - } - // Write new keys - for (uint32_t i = 0; i < hdr->key_count; i++) { - key_record_t rec = to_key_record(&keys[i]); + bool ok = true; + for (uint32_t i = 0; i < hdr.key_count; i++) { + key_record_t rec = to_key_record(&plain[i]); if (!storage_key_save(&rec)) { - printf("[backup] import: failed to write key %u\r\n", keys[i].id); - return false; + printf("[backup] import: failed to write key %u\r\n", plain[i].id); + ok = false; + break; } } - printf("[backup] import: wrote %u keys\r\n", hdr->key_count); - return true; -} \ No newline at end of file + memset(plain_scratch, 0, cipher_len); + + if (ok) + printf("[backup] import: wrote %u keys\r\n", hdr.key_count); + return ok; +} diff --git a/storage/backup.h b/storage/backup.h index afae954..9a008c7 100644 --- a/storage/backup.h +++ b/storage/backup.h @@ -9,18 +9,52 @@ // --------------------------------------------------------------------------- // Binary format (base64-encoded for serial transport) // --------------------------------------------------------------------------- +// +// A backup's whole purpose is to live off-device (an operator's laptop, a repo, +// a chat log), so the raw HMAC seeds it carries must be useless to anyone who +// obtains the blob, and a tampered blob must never be trusted. The payload is +// therefore encrypted-then-MAC'd under a key derived from an operator +// passphrase (a backup is portable, so the key cannot be device-bound): +// +// PBKDF2-HMAC-SHA256(passphrase, salt) -> 32-byte key +// AES-256-GCM(key, iv) over the serialised key records +// +// With GCM the authentication tag IS the MAC of ciphertext + associated data, +// so this is encrypt-then-MAC by construction. Import derives the same key from +// the blob's salt, GCM-verifies the tag over the header (as AAD) and the +// ciphertext, and REJECTS before any record is parsed if verification fails. +// A wrong passphrase and any tamper (of header, ciphertext or tag) all fail the +// tag check. The CRC-32 is retained only as a paste-corruption hint; it is NOT +// a trust boundary. #define BACKUP_MAGIC 0x4C4C5348U // "HSLL" -#define BACKUP_VERSION 1 +#define BACKUP_VERSION 2 #define BACKUP_MAX_KEYS KEY_MAX_COUNT +#define BACKUP_SALT_LEN 16 +#define BACKUP_IV_LEN 12 +#define BACKUP_TAG_LEN 16 +#define BACKUP_KEY_LEN 32 +#define BACKUP_PBKDF2_ITERS 100000u + +// Header is cleartext; `salt`, `iv`, `key_count`, `magic` and `version` are fed +// to GCM as associated data so they are authenticated by `tag`. `checksum` is a +// non-cryptographic paste-corruption hint over the ciphertext and is NOT part +// of the AAD. The ciphertext (key_count * sizeof(backup_key_t) bytes) follows. typedef struct __attribute__((packed)) { uint32_t magic; uint32_t version; uint32_t key_count; - uint32_t checksum; // covers all backup_key_t records + uint8_t salt[BACKUP_SALT_LEN]; + uint8_t iv[BACKUP_IV_LEN]; + uint8_t tag[BACKUP_TAG_LEN]; + uint32_t checksum; // CRC-32 of ciphertext: paste-corruption hint ONLY } backup_header_t; +// Bytes of the header that are authenticated as GCM associated data: everything +// up to (not including) the tag itself. +#define BACKUP_AAD_LEN offsetof(backup_header_t, tag) + typedef struct __attribute__((packed)) { uint16_t id; char name[KEY_NAME_MAX]; @@ -34,10 +68,23 @@ typedef struct __attribute__((packed)) { // API // --------------------------------------------------------------------------- -// Serialise all keys into buf. Returns byte count written, -1 on error. -int backup_export(uint8_t *buf, size_t buf_size); +// Per-key operator confirmation for admin records. Even with a valid MAC, an +// imported record that would grant admin is confirmed one-by-one (import is the +// only path that can set is_admin without set-key-admin). Return true to import +// the record as admin. Returning NULL for the callback denies all admin records. +typedef bool (*backup_admin_confirm_fn)(uint16_t id, const char *name, void *ctx); + +// Encrypt-then-MAC all keys into buf under `passphrase`. Returns byte count +// written, -1 on error (buffer too small, no passphrase, crypto/storage error). +int backup_export(uint8_t *buf, size_t buf_size, const char *passphrase); -// Overwrite all keys from buf. Returns false on error. -bool backup_import(const uint8_t *buf, size_t size); +// Decrypt + authenticate buf under `passphrase`, then overwrite all keys. +// Verifies the GCM tag BEFORE parsing any record; returns false (touching +// nothing) on any authentication failure, wrong passphrase, or malformed blob. +// Each is_admin record is gated through `confirm_admin`; a denied admin record +// aborts the whole import before existing keys are touched. Returns false on +// error. +bool backup_import(const uint8_t *buf, size_t size, const char *passphrase, + backup_admin_confirm_fn confirm_admin, void *confirm_ctx); #endif diff --git a/test/Makefile b/test/Makefile index 0c35e66..786dda0 100644 --- a/test/Makefile +++ b/test/Makefile @@ -61,6 +61,15 @@ TOTP_LIBS := -lmbedcrypto STORAGE_SRCS := harness_storage.c $(ROOT)/storage/storage.c $(ROOT)/storage/backup.c \ $(ROOT)/libs/littlefs/lfs.c $(ROOT)/libs/littlefs/lfs_util.c STORAGE_DEFS := -DLFS_NO_MALLOC -DLFS_NO_DEBUG +# backup.c now encrypt-then-MACs the blob (AES-256-GCM + PBKDF2) via mbedtls, so +# the storage harness links the REAL system mbedtls (libmbedtls-dev). `-idirafter +# stub` keeps the pico/hardware/lwip shims reachable while letting the real +# (system dirs) win over the compile-only +# stub/mbedtls/md.h (which lacks GCM/PKCS5 and SHA-256). +STORAGE_INCLUDES := -I$(ROOT) -I$(ROOT)/hardware -I$(ROOT)/network -I$(ROOT)/serial \ + -I$(ROOT)/storage -I$(SHARED) -I$(BASE32) -I$(BASE64) \ + -I$(QRCODEGEN) -I$(ROOT)/libs/littlefs -idirafter stub +STORAGE_LIBS := -lmbedcrypto # Commands harness: links serial/commands.c (the dispatcher) against SPY handler # stubs + a stub buzzer, all inside harness_commands.c. No real key/system/ @@ -88,6 +97,14 @@ WHOLE_SRCS := $(ROOT)/hardware/buzzer.c $(ROOT)/hardware/clock.c \ # backup.c see the same lfs.h / lfs_util.h configuration the firmware does. WHOLE_DEFS := -DLFS_NO_MALLOC -DLFS_NO_DEBUG +# Same include roots as INCLUDES but with stub as `-idirafter` so the REAL system +# mbedtls headers win: backup.c (AES-GCM/PKCS5) and totp.c (HMAC-SHA1) are +# compiled compile-only here and the stub/mbedtls/md.h lacks GCM/PKCS5/SHA-256. +# Everything else (pico/hardware/lwip) has no system header, so stub still wins. +WHOLE_INCLUDES := -I$(ROOT) -I$(ROOT)/hardware -I$(ROOT)/network -I$(ROOT)/serial \ + -I$(ROOT)/storage -I$(SHARED) -I$(BASE32) -I$(BASE64) \ + -I$(QRCODEGEN) -I$(ROOT)/libs/littlefs -idirafter stub + CC := gcc CSTD := -std=c11 WARN := -Wall -Wextra @@ -143,7 +160,7 @@ $(BUILD)/asan_totp: $(TOTP_SRCS) | $(BUILD) $(CC) $(CSTD) $(WARN) $(ASAN_FLAGS) $(TOTP_INCLUDES) $(TOTP_SRCS) $(TOTP_LIBS) -o $@ $(BUILD)/asan_storage: $(STORAGE_SRCS) | $(BUILD) - $(CC) $(CSTD) $(WARN) $(ASAN_FLAGS) $(STORAGE_DEFS) $(INCLUDES) $(STORAGE_SRCS) -o $@ + $(CC) $(CSTD) $(WARN) $(ASAN_FLAGS) $(STORAGE_DEFS) $(STORAGE_INCLUDES) $(STORAGE_SRCS) $(STORAGE_LIBS) -o $@ $(BUILD)/asan_commands: $(COMMANDS_SRCS) | $(BUILD) $(CC) $(CSTD) $(WARN) $(ASAN_FLAGS) $(INCLUDES) $(COMMANDS_SRCS) -o $@ @@ -169,7 +186,7 @@ $(BUILD)/vg_totp: $(TOTP_SRCS) | $(BUILD) $(CC) $(CSTD) $(WARN) $(VG_FLAGS) $(TOTP_INCLUDES) $(TOTP_SRCS) $(TOTP_LIBS) -o $@ $(BUILD)/vg_storage: $(STORAGE_SRCS) | $(BUILD) - $(CC) $(CSTD) $(WARN) $(VG_FLAGS) $(STORAGE_DEFS) $(INCLUDES) $(STORAGE_SRCS) -o $@ + $(CC) $(CSTD) $(WARN) $(VG_FLAGS) $(STORAGE_DEFS) $(STORAGE_INCLUDES) $(STORAGE_SRCS) $(STORAGE_LIBS) -o $@ $(BUILD)/vg_commands: $(COMMANDS_SRCS) | $(BUILD) $(CC) $(CSTD) $(WARN) $(VG_FLAGS) $(INCLUDES) $(COMMANDS_SRCS) -o $@ @@ -191,8 +208,8 @@ coverage: check-submodules | $(COV_DIR)/obj -o $(COV_DIR)/obj/cov_base64 $(CC) $(CSTD) $(WARN) $(COV_FLAGS) $(TOTP_INCLUDES) $(TOTP_SRCS) $(TOTP_LIBS) \ -o $(COV_DIR)/obj/cov_totp - $(CC) $(CSTD) $(WARN) $(COV_FLAGS) $(STORAGE_DEFS) $(INCLUDES) $(STORAGE_SRCS) \ - -o $(COV_DIR)/obj/cov_storage + $(CC) $(CSTD) $(WARN) $(COV_FLAGS) $(STORAGE_DEFS) $(STORAGE_INCLUDES) $(STORAGE_SRCS) \ + $(STORAGE_LIBS) -o $(COV_DIR)/obj/cov_storage $(CC) $(CSTD) $(WARN) $(COV_FLAGS) $(INCLUDES) $(COMMANDS_SRCS) \ -o $(COV_DIR)/obj/cov_commands @echo "== coverage: running instrumented harnesses ==" @@ -205,7 +222,7 @@ coverage: check-submodules | $(COV_DIR)/obj @for src in $(WHOLE_SRCS); do \ obj=$(COV_DIR)/obj/whole_$$(echo $$src | tr '/.' '__').o; \ echo " CC $$src"; \ - $(CC) $(CSTD) $(WARN) $(COV_FLAGS) $(WHOLE_DEFS) $(INCLUDES) \ + $(CC) $(CSTD) $(WARN) $(COV_FLAGS) $(WHOLE_DEFS) $(WHOLE_INCLUDES) \ -c $$src -o $$obj || exit 1; \ done @echo "== coverage: capturing baseline (all .gcno at 0%) + run data ==" diff --git a/test/harness_storage.c b/test/harness_storage.c index cc6071d..4573908 100644 --- a/test/harness_storage.c +++ b/test/harness_storage.c @@ -24,13 +24,95 @@ #include #include +#include +#include +#include +#include + #include "hardware/flash.h" /* XIP_BASE, FLASH_SECTOR_SIZE */ #include "pico/stdlib.h" /* PICO_FLASH_SIZE_BYTES, PICO_OK */ #include "backup.h" -#include "lfs_util.h" /* lfs_crc: matches backup.c's whole-backup checksum */ +#include "lfs_util.h" /* lfs_crc: matches backup.c's ciphertext checksum hint */ #include "storage.h" +/* backup.c pulls its salt/iv from get_rand_64(); the harness supplies a + * deterministic definition (real hardware uses the ROSC RNG). Values need not + * be cryptographically strong here - only distinct enough for GCM correctness. */ +uint64_t get_rand_64(void) { + static uint64_t s = 0x9E3779B97F4A7C15ull; + s ^= s << 13; + s ^= s >> 7; + s ^= s << 17; + return s; +} + +/* Test passphrase used for every export/import roundtrip below. */ +static const char *const PASS = "correct horse battery staple"; + +/* Per-key admin-confirm callbacks driving backup_import's is_admin backstop. */ +static bool confirm_yes(uint16_t id, const char *name, void *ctx) { + (void)id; + (void)name; + (void)ctx; + return true; +} +static bool confirm_no(uint16_t id, const char *name, void *ctx) { + (void)id; + (void)name; + (void)ctx; + return false; +} + +/* Forge a VALID v2 blob (correct PBKDF2 key + GCM tag) from arbitrary record + * bytes - the model of a passphrase holder crafting a malicious payload. Mirrors + * backup_export's crypto so backup_import accepts it, letting the tests inject a + * chosen-secret admin record / a non-bool flag byte. Returns total blob length. */ +static int forge_blob(const char *passphrase, const backup_key_t *recs, uint32_t count, + uint8_t *out) { + backup_header_t hdr = {0}; + hdr.magic = BACKUP_MAGIC; + hdr.version = BACKUP_VERSION; + hdr.key_count = count; + for (int i = 0; i < BACKUP_SALT_LEN; i++) + hdr.salt[i] = (uint8_t)(0xA0 + i); + for (int i = 0; i < BACKUP_IV_LEN; i++) + hdr.iv[i] = (uint8_t)(0x50 + i); + + uint8_t key[BACKUP_KEY_LEN]; +#if defined(MBEDTLS_VERSION_NUMBER) && MBEDTLS_VERSION_NUMBER >= 0x03060000 + assert(mbedtls_pkcs5_pbkdf2_hmac_ext(MBEDTLS_MD_SHA256, (const unsigned char *)passphrase, + strlen(passphrase), hdr.salt, BACKUP_SALT_LEN, + BACKUP_PBKDF2_ITERS, BACKUP_KEY_LEN, key) == 0); +#else + const mbedtls_md_info_t *md = mbedtls_md_info_from_type(MBEDTLS_MD_SHA256); + assert(md != NULL); + mbedtls_md_context_t md_ctx; + mbedtls_md_init(&md_ctx); + assert(mbedtls_md_setup(&md_ctx, md, 1 /* HMAC */) == 0); + assert(mbedtls_pkcs5_pbkdf2_hmac(&md_ctx, (const unsigned char *)passphrase, strlen(passphrase), + hdr.salt, BACKUP_SALT_LEN, BACKUP_PBKDF2_ITERS, BACKUP_KEY_LEN, + key) == 0); + mbedtls_md_free(&md_ctx); +#endif + + size_t cipher_len = (size_t)count * sizeof(backup_key_t); + uint8_t *ct = out + sizeof(backup_header_t); + + mbedtls_gcm_context gcm; + mbedtls_gcm_init(&gcm); + assert(mbedtls_gcm_setkey(&gcm, MBEDTLS_CIPHER_ID_AES, key, BACKUP_KEY_LEN * 8) == 0); + assert(mbedtls_gcm_crypt_and_tag(&gcm, MBEDTLS_GCM_ENCRYPT, cipher_len, hdr.iv, BACKUP_IV_LEN, + (const unsigned char *)&hdr, BACKUP_AAD_LEN, + (const unsigned char *)recs, ct, BACKUP_TAG_LEN, + hdr.tag) == 0); + mbedtls_gcm_free(&gcm); + + hdr.checksum = lfs_crc(0xFFFFFFFF, ct, cipher_len); + memcpy(out, &hdr, sizeof(hdr)); + return (int)(sizeof(backup_header_t) + cipher_len); +} + /* Must match storage.c's private layout constants. */ #define STORAGE_SIZE_BYTES (256 * 1024) #define STORAGE_FLASH_OFFSET (PICO_FLASH_SIZE_BYTES - STORAGE_SIZE_BYTES) @@ -175,15 +257,37 @@ int main(void) { assert(storage_wifi_get(&wgot) == false); /* gone */ assert(storage_wifi_clear() == false); /* already gone */ - /* --- backup export -> import roundtrip -------------------------------- */ - /* Current store holds keys id=1 (updated) and id=3. */ + /* --- H6: encrypt-then-MAC backup export -> import roundtrip ----------- */ + /* Current store holds id=1 (non-admin) and id=3 (ADMIN). */ static uint8_t backup[sizeof(backup_header_t) + BACKUP_MAX_KEYS * sizeof(backup_key_t)]; - int blen = backup_export(backup, sizeof backup); + int blen = backup_export(backup, sizeof backup, PASS); assert(blen > 0); assert((size_t)blen == sizeof(backup_header_t) + 2 * sizeof(backup_key_t)); - /* buffer too small -> -1 */ - assert(backup_export(backup, sizeof(backup_header_t)) == -1); + /* the header is cleartext + well-formed; the payload is ciphertext, so it + * must NOT contain the raw seeds in the clear. */ + { + const backup_header_t *h = (const backup_header_t *)backup; + assert(h->magic == BACKUP_MAGIC && h->version == BACKUP_VERSION && h->key_count == 2); + /* k3's secret starts at byte 0x40 (make_key seed); it must not appear + * verbatim anywhere in the ciphertext. */ + uint8_t needle[KEY_SECRET_LEN]; + for (int i = 0; i < KEY_SECRET_LEN; i++) + needle[i] = (uint8_t)(0x40 + i); + bool leaked = false; + for (size_t off = sizeof(backup_header_t); off + KEY_SECRET_LEN <= (size_t)blen; off++) + if (memcmp(backup + off, needle, KEY_SECRET_LEN) == 0) + leaked = true; + assert(!leaked); + } + + /* buffer too small -> -1; missing passphrase -> -1 */ + assert(backup_export(backup, sizeof(backup_header_t), PASS) == -1); + assert(backup_export(backup, sizeof backup, NULL) == -1); + assert(backup_export(backup, sizeof backup, "") == -1); + /* re-export a good blob for the tests below */ + blen = backup_export(backup, sizeof backup, PASS); + assert(blen > 0); /* Snapshot expected keys, then wipe the store. */ key_record_t expect[8]; @@ -193,8 +297,9 @@ int main(void) { assert(storage_key_delete(expect[i].id) == true); assert(storage_key_list(list, 16) == 0); - /* Import restores them. */ - assert(backup_import(backup, (size_t)blen) == true); + /* Import with the right passphrase restores everything (the admin key id=3 + * is confirmed via confirm_yes). */ + assert(backup_import(backup, (size_t)blen, PASS, confirm_yes, NULL) == true); int restored_n = storage_key_list(list, 16); assert(restored_n == expect_n); for (int i = 0; i < expect_n; i++) { @@ -203,48 +308,62 @@ int main(void) { assert(keys_equal(&expect[i], &r)); } - /* --- corrupt / invalid backups all rejected --------------------------- */ - /* too small */ - assert(backup_import(backup, sizeof(backup_header_t) - 1) == false); + /* --- wrong passphrase -> rejected, store untouched -------------------- */ + assert(backup_import(backup, (size_t)blen, "wrong passphrase", confirm_yes, NULL) == false); + assert(backup_import(backup, (size_t)blen, NULL, confirm_yes, NULL) == false); + assert(storage_key_list(list, 16) == expect_n); - /* bad magic */ + /* --- malformed / tampered blobs all rejected before any parse --------- */ + /* too small */ + assert(backup_import(backup, sizeof(backup_header_t) - 1, PASS, confirm_yes, NULL) == false); + /* bad magic (pre-decrypt header check) */ { uint8_t bad[sizeof backup]; memcpy(bad, backup, (size_t)blen); - backup_header_t *h = (backup_header_t *)bad; - h->magic = 0xDEADBEEFu; - assert(backup_import(bad, (size_t)blen) == false); + ((backup_header_t *)bad)->magic = 0xDEADBEEFu; + assert(backup_import(bad, (size_t)blen, PASS, confirm_yes, NULL) == false); } /* bad version */ { uint8_t bad[sizeof backup]; memcpy(bad, backup, (size_t)blen); - backup_header_t *h = (backup_header_t *)bad; - h->version = BACKUP_VERSION + 1; - assert(backup_import(bad, (size_t)blen) == false); + ((backup_header_t *)bad)->version = BACKUP_VERSION + 1; + assert(backup_import(bad, (size_t)blen, PASS, confirm_yes, NULL) == false); } /* key_count too large */ { uint8_t bad[sizeof backup]; memcpy(bad, backup, (size_t)blen); - backup_header_t *h = (backup_header_t *)bad; - h->key_count = KEY_MAX_COUNT + 1; - assert(backup_import(bad, (size_t)blen) == false); + ((backup_header_t *)bad)->key_count = KEY_MAX_COUNT + 1; + assert(backup_import(bad, (size_t)blen, PASS, confirm_yes, NULL) == false); } /* truncated body (header claims more keys than bytes provided) */ { uint8_t bad[sizeof backup]; memcpy(bad, backup, (size_t)blen); - backup_header_t *h = (backup_header_t *)bad; - h->key_count = 200; /* body far shorter than 200 records */ - assert(backup_import(bad, (size_t)blen) == false); + ((backup_header_t *)bad)->key_count = 200; + assert(backup_import(bad, (size_t)blen, PASS, confirm_yes, NULL) == false); } - /* checksum mismatch (flip a payload byte, keep header intact) */ + /* GCM tag tampered (CRC still valid) -> MAC failure */ + { + uint8_t bad[sizeof backup]; + memcpy(bad, backup, (size_t)blen); + ((backup_header_t *)bad)->tag[0] ^= 0xFF; + assert(backup_import(bad, (size_t)blen, PASS, confirm_yes, NULL) == false); + } + /* ciphertext byte flipped -> MAC failure */ { uint8_t bad[sizeof backup]; memcpy(bad, backup, (size_t)blen); bad[sizeof(backup_header_t)] ^= 0xFF; - assert(backup_import(bad, (size_t)blen) == false); + assert(backup_import(bad, (size_t)blen, PASS, confirm_yes, NULL) == false); + } + /* AAD tampered: key_count altered within range (2 -> 1) -> MAC failure */ + { + uint8_t bad[sizeof backup]; + memcpy(bad, backup, (size_t)blen); + ((backup_header_t *)bad)->key_count = 1; + assert(backup_import(bad, (size_t)blen, PASS, confirm_yes, NULL) == false); } /* The store still holds the good import after all rejected attempts. */ @@ -254,32 +373,61 @@ int main(void) { for (int i = 0; i < expect_n; i++) assert(storage_key_delete(expect[i].id) == true); assert(storage_key_list(list, 16) == 0); - int elen = backup_export(backup, sizeof backup); + int elen = backup_export(backup, sizeof backup, PASS); assert((size_t)elen == sizeof(backup_header_t)); - assert(backup_import(backup, (size_t)elen) == true); + assert(backup_import(backup, (size_t)elen, PASS, confirm_yes, NULL) == true); assert(storage_key_list(list, 16) == 0); - /* --- UBSan regression: non-bool flag byte in an otherwise-valid blob --- */ - /* to_key_record() must not load is_enabled/is_admin straight into a `bool`: - * a crafted import blob can carry any byte there, and loading a bool whose - * object representation is not 0/1 is undefined behaviour (caught by - * -fsanitize=undefined). Build a valid 1-key blob, poke a non-bool flag - * byte, refresh the header CRC so the blob still validates, and import it. - * The import must succeed and the flag must read back canonicalised. */ + /* --- is_admin backstop: an authenticated admin record still needs per-key + * operator confirmation. Forge a VALID blob (correct passphrase, real GCM + * tag) carrying a chosen-secret admin record - the exact H6 escalation. --- */ { - key_record_t seed = make_key(9, "ub", true, false, 1234, 0x40); - assert(storage_key_save(&seed) == true); + backup_key_t evil = {0}; + evil.id = 7; + snprintf(evil.name, sizeof evil.name, "pwned"); + for (int i = 0; i < KEY_SECRET_LEN; i++) + evil.secret[i] = (uint8_t)i; + evil.is_enabled = true; + evil.is_admin = true; + evil.created_at = 42; + uint8_t craft[sizeof backup]; - int clen = backup_export(craft, sizeof craft); - assert((size_t)clen == sizeof(backup_header_t) + sizeof(backup_key_t)); - assert(storage_key_delete(9) == true); + int clen = forge_blob(PASS, &evil, 1, craft); + + /* denied by the operator -> rejected, store untouched */ + assert(backup_import(craft, (size_t)clen, PASS, confirm_no, NULL) == false); + assert(storage_key_list(list, 16) == 0); + /* NULL callback denies all admin records */ + assert(backup_import(craft, (size_t)clen, PASS, NULL, NULL) == false); + assert(storage_key_list(list, 16) == 0); + /* confirmed -> imported as admin */ + assert(backup_import(craft, (size_t)clen, PASS, confirm_yes, NULL) == true); + key_record_t g; + assert(storage_key_get(7, &g) == true); + assert(g.is_admin == true); + assert(storage_key_delete(7) == true); + } - backup_key_t *bk = (backup_key_t *)(craft + sizeof(backup_header_t)); - *(uint8_t *)&bk->is_admin = 67; /* non-bool byte in an otherwise-valid record */ - backup_header_t *bh = (backup_header_t *)craft; - bh->checksum = lfs_crc(0xFFFFFFFF, bk, sizeof(backup_key_t)); + /* --- UBSan regression: non-bool flag byte in an authenticated payload --- */ + /* to_key_record() must not load is_enabled/is_admin straight into a `bool`: + * a passphrase holder can forge a validly-encrypted record with any byte in + * those fields, and loading a bool whose representation is not 0/1 is UB + * (caught by -fsanitize=undefined). Forge a valid 1-key blob with a non-bool + * admin byte; import (confirmed) must succeed and read back canonicalised. */ + { + backup_key_t bk = {0}; + bk.id = 9; + snprintf(bk.name, sizeof bk.name, "ub"); + for (int i = 0; i < KEY_SECRET_LEN; i++) + bk.secret[i] = (uint8_t)(0x40 + i); + *(uint8_t *)&bk.is_enabled = 1; + *(uint8_t *)&bk.is_admin = 67; /* non-bool byte */ + bk.created_at = 1234; + + uint8_t craft[sizeof backup]; + int clen = forge_blob(PASS, &bk, 1, craft); - assert(backup_import(craft, (size_t)clen) == true); + assert(backup_import(craft, (size_t)clen, PASS, confirm_yes, NULL) == true); key_record_t g; assert(storage_key_get(9, &g) == true); assert(g.is_admin == true); /* canonicalised: exactly 1, not 67 */