From c7826791d4d93b712759d34a24c2bad4dd4c14de Mon Sep 17 00:00:00 2001 From: Jonathan Bennett Date: Sat, 3 Oct 2026 03:19:01 +0000 Subject: [PATCH] feat(portduino): admin keys in config.yaml, beyond the protobuf's three (#12056) * feat(portduino): admin keys in config.yaml, beyond the protobuf's three meshtasticd can now list the public keys authorized to administer the node in its own config, as Security.AdminKeys, and list more than the three config.security.admin_key holds. The host's file is the authority: it is root-owned, so unlike the persisted protobuf an authorized remote cannot rewrite it, and applyHostKeys() re-asserts it at boot and after every remote write of the security config. A key removed from the file loses access on the next boot instead of lingering in flash. The protobuf array stays at three. It is not an arbitrary cap: a security set_config already encodes to ~207 bytes against the 221 a PKC admin packet can carry (233 payload bytes less 12 of PKC overhead), so a fourth key would not fit in one admin message on any platform. Four sites decided who may administer a node, each unrolling admin_key[0..2] by hand - the PKC gate in AdminModule, the same gate in DMShell, the is_managed guard, and the admin-key trial decrypt in Router::perhapsDecode. They now share AdminKeys::isAuthorized(), so a key source reaches all four at once, and enumeration skips empty slots rather than trusting admin_key_count. Each configured key costs one trial decryption per PKI packet from an unknown sender, so the host list is capped at 16 and duplicates are dropped. Files in config.d append to the list, one per operator. A key that is not exactly 32 canonical base64 bytes stops meshtasticd rather than silently authorizing something else, and --check reports it with its line instead. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01SP5hpFwdTBC2K9ggGFLpcs * test(portduino): config-check fixtures for Security.AdminKeys bin/test-config-check.sh drives a built meshtasticd against the fixtures, which is the only place the two process-level behaviours here are reachable: what --check reports, and that a normal boot refuses the file. A clean two-key list, a key three characters short of 32 bytes, and one key written as a scalar instead of a list - the last two as errors under --check and as a non-zero exit on a normal boot, since a truncated admin key would authorize something other than what was written and a file whose only key is unreadable would otherwise boot a node nobody can administer. adminkeys-append/ pins the append-across-config.d rule by value through --output-yaml, with the first key in config.yaml so the effective list does not depend on the filesystem's order. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01SP5hpFwdTBC2K9ggGFLpcs --------- Co-authored-by: Claude Opus 5 --- .github/copilot-instructions.md | 4 +- AGENTS.md | 2 +- bin/config-dist.yaml | 8 + bin/test-config-check.sh | 21 ++ src/mesh/AdminKeys.cpp | 86 ++++++ src/mesh/AdminKeys.h | 38 +++ src/mesh/NodeDB.cpp | 5 + src/mesh/Router.cpp | 19 +- src/modules/AdminModule.cpp | 13 +- src/modules/DMShell.cpp | 8 +- src/platform/portduino/ConfigCheck.cpp | 29 ++ src/platform/portduino/PortduinoGlue.cpp | 95 ++++++ src/platform/portduino/PortduinoGlue.h | 22 ++ test/fixtures/portduino-config/README.md | 26 +- .../config.d/more-admins.yaml | 5 + .../adminkeys-append/config.yaml | 14 + .../portduino-config/adminkeys-bad.yaml | 13 + .../adminkeys-not-a-list.yaml | 11 + test/fixtures/portduino-config/adminkeys.yaml | 12 + test/test_admin_keys/test_main.cpp | 273 ++++++++++++++++++ 20 files changed, 674 insertions(+), 30 deletions(-) create mode 100644 src/mesh/AdminKeys.cpp create mode 100644 src/mesh/AdminKeys.h create mode 100644 test/fixtures/portduino-config/adminkeys-append/config.d/more-admins.yaml create mode 100644 test/fixtures/portduino-config/adminkeys-append/config.yaml create mode 100644 test/fixtures/portduino-config/adminkeys-bad.yaml create mode 100644 test/fixtures/portduino-config/adminkeys-not-a-list.yaml create mode 100644 test/fixtures/portduino-config/adminkeys.yaml create mode 100644 test/test_admin_keys/test_main.cpp diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 5f5fe04484..a2855e59bc 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -98,7 +98,7 @@ Meshtastic packets on the air are typically encrypted one of two ways: the **per - **Channels** are symmetric rooms: anyone with the PSK can read any message on the channel. Channel 0 is the "primary" channel and ships with the short-form default PSK on factory devices, forming the public mesh most users join. (The LoRa modem preset `LONG_FAST` lives on `config.lora.modem_preset` and is an independent field - don't conflate "channel 0 default PSK" with the modem preset name.) - **DMs** addressed to a single node require PKI so that other holders of the channel PSK can't read them. Outside Ham mode, Meshtastic does not fall back to channel-symmetric encryption when the destination public key is unknown. -- **Remote admin** is a DM carrying an `AdminMessage`. The receiver only acts on it if the sender's public key is on its allowlist (`config.security.admin_key[0..2]`). +- **Remote admin** is a DM carrying an `AdminMessage`. The receiver only acts on it if the sender's public key is on its allowlist (`AdminKeys::isAuthorized()` in `src/mesh/AdminKeys.h`: `config.security.admin_key[0..2]`, plus any further keys meshtasticd's `Security.AdminKeys` supplied). - **Ham mode** (`owner.is_licensed=true`, where `owner` is the local `meshtastic_User` record) disables PKI entirely and sends cleartext - FCC Part 97 prohibits encryption on amateur bands. - **No ratchet, no session.** Every packet is encrypted from scratch - a stateless design that matches the high-loss, store-and-forward nature of LoRa. @@ -138,7 +138,7 @@ Implemented in `src/modules/AdminModule.cpp` → `handleReceivedProtobuf`. The a 1. **Response messages** - if `messageIsResponse(r)` is true (the payload is a response to one of our earlier admin requests), it's accepted without any further check. The in-file comment flags this as a known-untightened gap: a stricter implementation would remember which `public_key` we last queried and reject responses that don't match. 2. **Local admin** - `mp.from == 0` (phone app over BLE, serial CLI, internal module); never travels over the air. **Rejected** if `config.security.is_managed` is true, because managed devices expect admin to arrive over the air through an authorized remote path. 3. **Legacy admin channel (deprecated)** - the packet arrived on a channel named literally `"admin"`. Gated by `config.security.admin_channel_enabled`; returns `NOT_AUTHORIZED` if the flag is false. Kept for backward compatibility; new deployments should use PKI admin. -4. **PKI admin (preferred for remote)** - `mp.pki_encrypted == true` AND `mp.public_key` matches one of `config.security.admin_key[0..2]` (up to three authorized 32-byte Curve25519 public keys, typically copied from the admin node's own `user.public_key`). +4. **PKI admin (preferred for remote)** - `mp.pki_encrypted == true` AND `AdminKeys::isAuthorized(mp.public_key.bytes)`. That is `config.security.admin_key[0..2]` (up to three authorized 32-byte Curve25519 public keys, typically copied from the admin node's own `user.public_key`), plus, on meshtasticd, the keys `Security.AdminKeys` in `config.yaml` carried past those three. The same list backs `DMShell::isAuthorizedPacket`, the `is_managed` guard, and the admin-key trial decrypt in `Router::perhapsDecode` - add a key source there, not at a call site. On meshtasticd the file wins: `AdminKeys::applyHostKeys()` re-asserts it at boot and after every remote write of the security config. 5. **Fallthrough** → `NOT_AUTHORIZED`. On top of authorization, any remote admin message that **mutates** state (not a request, not a response) also has to pass a session-key check (`checkPassKey`): the client must first pull a fresh 8-byte `session_passkey` via `get_admin_session_key_request`, then echo that passkey back in the mutating message. The device rotates the passkey after 150 s and rejects values older than 300 s - a narrow anti-replay window on top of the PKI layer. diff --git a/AGENTS.md b/AGENTS.md index a8fed6b5ae..61052fd8ab 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -67,7 +67,7 @@ The test-and-diagnose workflows (`/test`, `/diagnose`, `/repro`, `/leakhunt`) no Two layers, both in `src/mesh/CryptoEngine.cpp`: - **Channel (symmetric)** - **AES-CTR** with a channel-wide PSK (AES-128 or AES-256). Nonce = packet_id ‖ from_node ‖ block_counter. No AEAD; integrity is soft (channel-hash filter). The well-known default PSK lives in `src/mesh/Channels.h`; a 1-byte PSK is a short-form index into it. -- **Per-peer PKI** - **X25519 ECDH** (Curve25519, 32-byte keys) → SHA-256 → **AES-256-CCM** with an 8-byte MAC. Fresh 32-bit `extraNonce` per packet, sent in the clear alongside the MAC. 12-byte wire overhead (`MESHTASTIC_PKC_OVERHEAD`). Used for DMs. Also used for remote admin (`src/modules/AdminModule.cpp`), where AdminMessage authorization is gated by `config.security.admin_key[0..2]`. Disabled entirely in Ham mode (`user.is_licensed=true`). +- **Per-peer PKI** - **X25519 ECDH** (Curve25519, 32-byte keys) → SHA-256 → **AES-256-CCM** with an 8-byte MAC. Fresh 32-bit `extraNonce` per packet, sent in the clear alongside the MAC. 12-byte wire overhead (`MESHTASTIC_PKC_OVERHEAD`). Used for DMs. Also used for remote admin (`src/modules/AdminModule.cpp`), where AdminMessage authorization is gated by `AdminKeys::isAuthorized()` (`config.security.admin_key[0..2]`, plus meshtasticd's `Security.AdminKeys`). Disabled entirely in Ham mode (`user.is_licensed=true`). Key rotation to never trigger casually: only the **full** factory reset (`factory_reset_device`, `eraseBleBonds=true`) wipes `security.private_key` and regenerates the keypair - every peer holds the old public key, so DMs silently fail PKI decrypt until NodeInfo re-exchanges. The **partial** config reset (`factory_reset_config`) preserves the private key and doesn't invalidate peer relationships. Explicitly blanking `security.private_key` via admin also triggers regen. See the **Encryption & Key Management** section of `.github/copilot-instructions.md` for the full spec (nonce layout, send/receive selection logic including infrastructure-portnum exceptions, admin-key + session-passkey authorization, `is_managed` scope, key-rotation hazards). diff --git a/bin/config-dist.yaml b/bin/config-dist.yaml index a591c1ba2c..8e57d3bbd6 100644 --- a/bin/config-dist.yaml +++ b/bin/config-dist.yaml @@ -259,6 +259,14 @@ HostMetrics: # Channel: 0 # channel to send Host Metrics over. Defaults to the primary channel. # UserStringCommand: cat /sys/firmware/devicetree/base/serial-number # Command to execute, to send the results as the userString +Security: +# Public keys authorized to send admin messages to this node, as base64, the same text the apps and +# the CLI show for a node's public key. Up to 16, and up to 3 of them are reflected into the node's +# own security config so clients and managed mode see them. Files in config.d add to this list, and +# this file is the authority: a key removed from here loses access, and remote admin cannot drop it. +# AdminKeys: +# - rvzaPCZmlh1nWnqNKUFHj0dLBJJIRJhRrkZmaFLHCVY= + Config: # DisplayMode: TWOCOLOR # uncomment to force BaseUI # DisplayMode: COLOR # uncomment to force MUI diff --git a/bin/test-config-check.sh b/bin/test-config-check.sh index 96c4531b4c..7a86968423 100755 --- a/bin/test-config-check.sh +++ b/bin/test-config-check.sh @@ -158,6 +158,23 @@ assert "bluetooth AdapterId must name a real adapter" 0 bluetooth-bad-adapter.ya "is not a BlueZ adapter name" \ "Result: 0 errors, 1 warning" +echo +echo "admin keys in the host config:" +assert "a list of admin keys is clean" 0 adminkeys.yaml check \ + "Result: 0 errors, 0 warnings" +assert "a malformed admin key is refused" 1 adminkeys-bad.yaml check \ + "is not a 32-byte base64 public key" \ + "Result: 1 error, 0 warnings" +assert "admin keys written as a scalar are refused" 1 adminkeys-not-a-list.yaml check \ + "Security.AdminKeys must be a list of base64 public keys" \ + "Result: 1 error, 0 warnings" +# Admin keys append rather than the last file winning, so one file per admin works. config.yaml is +# always loaded first, so the effective list is deterministic; --output-yaml is what reports it. +assert "admin keys append across config.d" 0 adminkeys-append/config.yaml yaml \ + "rvzaPCZmlh1nWnqNKUFHj0dLBJJIRJhRrkZmaFLHCVY=" \ + "NGC+MSAeaf7aoO7ouZl/XHwpmf2v5ZMlPNZUr0361xQ=" \ + "P2jeuoXA1sOGSPH9Vm3eHFlITu1EVnohWGKlGF3t8GU=" + echo echo "module names are matched exactly:" assert "unknown module names the valid set" 1 module-unknown.yaml check \ @@ -497,6 +514,10 @@ assert "non-mapping section is refused" 1 nonmap-section.yaml normal \ "as config file" assert "unknown module is refused" 1 module-unknown.yaml normal \ "Unknown Lora.Module: sx1263" +assert "a malformed admin key is refused on a normal boot" 1 adminkeys-bad.yaml normal \ + "is not a 32-byte base64 public key" +assert "admin keys as a scalar are refused on a normal boot" 1 adminkeys-not-a-list.yaml normal \ + "Security.AdminKeys must be a list of base64 public keys" assert "MACAddress conflict is refused" 1 mac-conflict.yaml normal \ "Cannot set both MACAddress and MACAddressSource!" # Only meaningful on a build without rgbmatrix. A build that supports HUB75 accepts diff --git a/src/mesh/AdminKeys.cpp b/src/mesh/AdminKeys.cpp new file mode 100644 index 0000000000..7c01c22f27 --- /dev/null +++ b/src/mesh/AdminKeys.cpp @@ -0,0 +1,86 @@ +#include "AdminKeys.h" +#include "NodeDB.h" +#include "configuration.h" +#include + +#ifdef ARCH_PORTDUINO +#include "platform/portduino/PortduinoGlue.h" +#endif + +namespace AdminKeys +{ + +#ifdef ARCH_PORTDUINO +// applyHostKeys() mirrors the first PROTOBUF_SLOTS host keys into config.security, so only the +// ones past that are counted here. +static size_t hostExtraCount() +{ + const size_t configured = portduino_config.admin_keys.size(); + return configured > PROTOBUF_SLOTS ? configured - PROTOBUF_SLOTS : 0; +} +#endif + +size_t count() +{ + size_t n = 0; + for (size_t slot = 0; slot < PROTOBUF_SLOTS; slot++) { + if (config.security.admin_key[slot].size == 32) + n++; + } +#ifdef ARCH_PORTDUINO + n += hostExtraCount(); +#endif + return n; +} + +const uint8_t *keyAt(size_t i) +{ + // Skips empty slots, so callers see a dense list whatever admin_key_count says. + for (size_t slot = 0; slot < PROTOBUF_SLOTS; slot++) { + if (config.security.admin_key[slot].size != 32) + continue; + if (i == 0) + return config.security.admin_key[slot].bytes; + i--; + } +#ifdef ARCH_PORTDUINO + if (i < hostExtraCount()) + return portduino_config.admin_keys[PROTOBUF_SLOTS + i].data(); +#endif + return nullptr; +} + +void applyHostKeys() +{ +#ifdef ARCH_PORTDUINO + if (portduino_config.admin_keys.empty()) + return; + + size_t slot = 0; + for (; slot < portduino_config.admin_keys.size() && slot < PROTOBUF_SLOTS; slot++) { + memcpy(config.security.admin_key[slot].bytes, portduino_config.admin_keys[slot].data(), 32); + config.security.admin_key[slot].size = 32; + } + // Clear any slot the host file no longer fills, so removing a key removes its access. + for (size_t i = slot; i < PROTOBUF_SLOTS; i++) { + memset(config.security.admin_key[i].bytes, 0, sizeof(config.security.admin_key[i].bytes)); + config.security.admin_key[i].size = 0; + } + config.security.admin_key_count = (pb_size_t)slot; + LOG_INFO("Admin keys from host config: %u authorized", (unsigned)count()); +#endif +} + +bool isAuthorized(const uint8_t *publicKey) +{ + if (!publicKey) + return false; + for (size_t i = 0, n = count(); i < n; i++) { + const uint8_t *key = keyAt(i); + if (key && memcmp(publicKey, key, 32) == 0) + return true; + } + return false; +} + +} // namespace AdminKeys diff --git a/src/mesh/AdminKeys.h b/src/mesh/AdminKeys.h new file mode 100644 index 0000000000..7663fc6e19 --- /dev/null +++ b/src/mesh/AdminKeys.h @@ -0,0 +1,38 @@ +#pragma once + +#include "mesh/generated/meshtastic/config.pb.h" +#include +#include + +/** + * The public keys authorized to administer this node. + * + * These are the up-to-three keys carried in config.security.admin_key plus, on meshtasticd, any + * further keys the host supplied in config.yaml that did not fit that array. Every authorization + * site goes through here so a new key source reaches all of them at once. + */ +namespace AdminKeys +{ +/// Slots in config.security.admin_key. Keys past this many live only in the host's config. +constexpr size_t PROTOBUF_SLOTS = + sizeof(meshtastic_Config_SecurityConfig::admin_key) / sizeof(meshtastic_Config_SecurityConfig_admin_key_t); + +/// How many authorized keys are configured. +size_t count(); + +/// The i-th authorized key, 32 bytes, or nullptr when i is past the end. +const uint8_t *keyAt(size_t i); + +/// True when publicKey, read as 32 bytes, is authorized. +bool isAuthorized(const uint8_t *publicKey); + +/// Mirrors the host's admin keys into config.security.admin_key, replacing what was there; a no-op +/// when the host configured none. Called at boot and after a remote admin writes security config. +void applyHostKeys(); + +/// True when any key is configured, i.e. remote admin is possible at all. +inline bool any() +{ + return count() > 0; +} +} // namespace AdminKeys diff --git a/src/mesh/NodeDB.cpp b/src/mesh/NodeDB.cpp index d90f88fa29..f473027411 100644 --- a/src/mesh/NodeDB.cpp +++ b/src/mesh/NodeDB.cpp @@ -3,6 +3,7 @@ #include "GPS.h" #endif #include "../detect/ScanI2C.h" +#include "AdminKeys.h" #include "Channels.h" #include "CryptoEngine.h" #include "Default.h" @@ -2972,6 +2973,10 @@ void NodeDB::loadFromDisk() saveToDisk(SEGMENT_CHANNELS); } #if ARCH_PORTDUINO + // The host's config.yaml is authoritative for admin keys: it is root-owned and cannot be + // rewritten by an authorized remote, so it decides who may administer this node. + AdminKeys::applyHostKeys(); + // set any config overrides if (portduino_config.has_configDisplayMode) { config.display.displaymode = (_meshtastic_Config_DisplayConfig_DisplayMode)portduino_config.configDisplayMode; diff --git a/src/mesh/Router.cpp b/src/mesh/Router.cpp index 8fe7ab9e4d..3a6b33d18b 100644 --- a/src/mesh/Router.cpp +++ b/src/mesh/Router.cpp @@ -1,4 +1,5 @@ #include "Router.h" +#include "AdminKeys.h" #include "Channels.h" #include "CryptoEngine.h" #include "MeshRadio.h" @@ -887,14 +888,7 @@ void resetAdminKeyFallbackBudget() static bool adminKeyFallbackAllowed() { - bool haveAdminKey = false; - for (int i = 0; i < 3; i++) { - if (config.security.admin_key[i].size == 32) { - haveAdminKey = true; - break; - } - } - if (!haveAdminKey) + if (!AdminKeys::any()) return false; // nothing to try, so do not spend a token // Injectable clock so the budget can be tested without sleeping, and without racing a slow host. @@ -985,11 +979,12 @@ DecodeState perhapsDecode(meshtastic_MeshPacket *p) viaPendingKey = havePendingKey; } if (!decrypted && adminKeyFallbackAllowed()) { - for (int i = 0; i < 3 && !decrypted; i++) { - if (config.security.admin_key[i].size != 32) - continue; + for (size_t i = 0, n = AdminKeys::count(); i < n && !decrypted; i++) { + const uint8_t *adminKey = AdminKeys::keyAt(i); + if (!adminKey) + break; remotePublic.size = 32; - memcpy(remotePublic.bytes, config.security.admin_key[i].bytes, 32); + memcpy(remotePublic.bytes, adminKey, 32); if (crypto->decryptCurve25519(p->from, remotePublic, p->id, rawSize, p->encrypted.bytes, bytes)) { decrypted = true; diff --git a/src/modules/AdminModule.cpp b/src/modules/AdminModule.cpp index 6aff797fcb..397a0ecdc6 100644 --- a/src/modules/AdminModule.cpp +++ b/src/modules/AdminModule.cpp @@ -35,6 +35,7 @@ #include "MessageStore.h" #include "RadioInterface.h" #include "TypeConversions.h" +#include "mesh/AdminKeys.h" #include "mesh/RadioLibInterface.h" #ifdef MESHTASTIC_PHONEAPI_ACCESS_CONTROL #include "mesh/PhoneAPI.h" @@ -187,12 +188,7 @@ bool AdminModule::handleReceivedProtobuf(const meshtastic_MeshPacket &mp, meshta return handled; } } else if (mp.pki_encrypted) { - if ((config.security.admin_key[0].size == 32 && - memcmp(mp.public_key.bytes, config.security.admin_key[0].bytes, 32) == 0) || - (config.security.admin_key[1].size == 32 && - memcmp(mp.public_key.bytes, config.security.admin_key[1].bytes, 32) == 0) || - (config.security.admin_key[2].size == 32 && - memcmp(mp.public_key.bytes, config.security.admin_key[2].bytes, 32) == 0)) { + if (AdminKeys::isAuthorized(mp.public_key.bytes)) { LOG_INFO("PKC admin payload with authorized sender key"); // Note: PKC admin does NOT automatically authorize the @@ -1204,6 +1200,8 @@ void AdminModule::handleSetConfig(const meshtastic_Config &c, bool fromOthers) } #endif config.security = incoming; + // A remote admin does not get to drop the host's own admin keys. + AdminKeys::applyHostKeys(); #if !(MESHTASTIC_EXCLUDE_PKI_KEYGEN) && !(MESHTASTIC_EXCLUDE_PKI) // First provisioning (no key) generates one; a private key supplied without its public key derives it. // A supplied public key that is itself blacklisted is re-derived too, so a restore carrying a whole @@ -1222,8 +1220,7 @@ void AdminModule::handleSetConfig(const meshtastic_Config &c, bool fromOthers) } } #endif - if (config.security.is_managed && !(config.security.admin_key[0].size == 32 || config.security.admin_key[1].size == 32 || - config.security.admin_key[2].size == 32)) { + if (config.security.is_managed && !AdminKeys::any()) { config.security.is_managed = false; const char *warning = "You must provide at least one admin public key to enable managed mode"; LOG_WARN(warning); diff --git a/src/modules/DMShell.cpp b/src/modules/DMShell.cpp index 36f45465a6..c4b96237b2 100644 --- a/src/modules/DMShell.cpp +++ b/src/modules/DMShell.cpp @@ -7,6 +7,7 @@ #include "NodeDB.h" #include "Throttle.h" #include "configuration.h" +#include "mesh/AdminKeys.h" #include "mesh/generated/meshtastic/mesh.pb.h" #include "mesh/mesh-pb-constants.h" #include "meshUtils.h" @@ -484,12 +485,7 @@ bool DMShellModule::isAuthorizedPacket(const meshtastic_MeshPacket &mp) const } if (mp.pki_encrypted) { - for (uint8_t i = 0; i < 3; ++i) { - if (config.security.admin_key[i].size == 32 && - memcmp(mp.public_key.bytes, config.security.admin_key[i].bytes, 32) == 0) { - return true; - } - } + return AdminKeys::isAuthorized(mp.public_key.bytes); } return false; diff --git a/src/platform/portduino/ConfigCheck.cpp b/src/platform/portduino/ConfigCheck.cpp index 7cd5fc5148..2f28dcd0cb 100644 --- a/src/platform/portduino/ConfigCheck.cpp +++ b/src/platform/portduino/ConfigCheck.cpp @@ -9,6 +9,7 @@ #include "yaml-cpp/eventhandler.h" #include +#include #include #include #include @@ -77,6 +78,7 @@ const std::map> &schema() {"General", {"MACAddress", "MACAddressSource", "MaxNodes", "MaxMessageQueue", "APIPort", "ConfigDirectory", "AvailableDirectory"}}, {"Config", {"DisplayMode", "EnableUDP", "StatusMessage"}}, + {"Security", {"AdminKeys"}}, {"Display", {"Panel", "spidev", "BusFrequency", "Width", "Height", "Invert", "Rotate", "OffsetX", "OffsetY", "OffsetRotate", "RGBOrder", "HUB75", "DC", "CS", "Backlight", "BacklightInvert", "BacklightPWMChannel", "Reset"}}, @@ -769,6 +771,31 @@ void checkTxGain(const std::string &file, const YAML::Node &node, std::vector &findings) +{ + if (!node.IsSequence()) { + findings.push_back({kError, file, lineOf(node), + "Security.AdminKeys must be a list of base64 public keys, so meshtasticd refuses to start on " + "this file"}); + return; + } + + if (node.size() > PORTDUINO_MAX_ADMIN_KEYS) + findings.push_back({kWarn, file, lineOf(node), + "Security.AdminKeys lists " + std::to_string(node.size()) + " keys but only the first " + + std::to_string(PORTDUINO_MAX_ADMIN_KEYS) + " are read; the rest are dropped"}); + + for (const auto &entry : node) { + std::array key; + if (!adminKeyFromBase64(entry.as(""), key)) + findings.push_back({kError, file, lineOf(entry), + "Security.AdminKeys entry '" + entry.as("") + + "' is not a 32-byte base64 public key, so meshtasticd refuses to start on this file"}); + } +} + // Module names match exactly and are inconsistently cased (RF95 upper, sx1262 lower), // and loadConfig() exits on an unknown name without printing the valid set, so name it here. void checkLoraModule(const std::string &file, const YAML::Node &module, std::vector &findings) @@ -897,6 +924,8 @@ void checkSection(const std::string &file, const std::string §ion, const YAM if (value.IsSequence()) for (const auto &pin : value) checkPinNode(file, section + "." + key, pin, findings); + } else if (section == "Security" && key == "AdminKeys") { + checkAdminKeys(file, value, findings); } else if (section == "Bluetooth" && key == "AdapterId") { // LinuxBluetooth uses this verbatim as the BlueZ object path (/org/bluez/), while the // MAC fallback only reads the leading hciN. A value like "hci1junk" therefore looks diff --git a/src/platform/portduino/PortduinoGlue.cpp b/src/platform/portduino/PortduinoGlue.cpp index cff57965fa..1bd7f5ece5 100644 --- a/src/platform/portduino/PortduinoGlue.cpp +++ b/src/platform/portduino/PortduinoGlue.cpp @@ -15,6 +15,8 @@ #include "meshUtils.h" #include #include +#include +#include #include #include #include @@ -1441,6 +1443,38 @@ bool loadConfig(const char *configPath) portduino_config.hostMetrics_user_command = (yamlConfig["HostMetrics"]["UserStringCommand"]).as(""); } + if (yamlConfig["Security"] && yamlConfig["Security"]["AdminKeys"]) { + const YAML::Node keys = yamlConfig["Security"]["AdminKeys"]; + if (!keys.IsSequence()) { + std::cout << "Security.AdminKeys must be a list of base64 public keys!" << std::endl; + if (!configCheck) + exit(EXIT_FAILURE); + } else { + // Appended, not assigned: loadConfig() runs once per file in config.d, so an + // operator can drop in a file per admin. Duplicates are dropped, since each key + // costs a trial decryption in Router's admin-key fallback. + for (const auto &entry : keys) { + const std::string text = entry.as(""); + std::array key; + if (!adminKeyFromBase64(text, key)) { + std::cout << "Security.AdminKeys: '" << text << "' is not a 32-byte base64 public key!" << std::endl; + if (!configCheck) + exit(EXIT_FAILURE); + continue; + } + if (std::find(portduino_config.admin_keys.begin(), portduino_config.admin_keys.end(), key) != + portduino_config.admin_keys.end()) + continue; + if (portduino_config.admin_keys.size() >= PORTDUINO_MAX_ADMIN_KEYS) { + std::cout << "Security.AdminKeys: at most " << PORTDUINO_MAX_ADMIN_KEYS + << " admin keys are supported, ignoring the rest!" << std::endl; + break; + } + portduino_config.admin_keys.push_back(key); + } + } + } + if (yamlConfig["Config"]) { portduino_config.has_config_overrides = true; if (yamlConfig["Config"]["DisplayMode"]) { @@ -1516,6 +1550,67 @@ static bool ends_with(std::string_view str, std::string_view suffix) return str.size() >= suffix.size() && str.compare(str.size() - suffix.size(), suffix.size(), suffix) == 0; } +static int base64Value(char c) +{ + if (c >= 'A' && c <= 'Z') + return c - 'A'; + if (c >= 'a' && c <= 'z') + return c - 'a' + 26; + if (c >= '0' && c <= '9') + return c - '0' + 52; + if (c == '+' || c == '-') // '-' and '_' so a base64url-encoded key is accepted too + return 62; + if (c == '/' || c == '_') + return 63; + return -1; +} + +// Decodes a 32-byte public key as the apps and the CLI print it. Anything that is not exactly 32 +// bytes of canonical base64 is refused rather than zero-padded: a truncated admin key would +// silently authorize the wrong peer. +bool adminKeyFromBase64(const std::string &text, std::array &out) +{ + uint32_t accum = 0; + int bits = 0; + size_t written = 0; + for (char c : text) { + if (isspace(static_cast(c)) || c == '=') + continue; + const int value = base64Value(c); + if (value < 0) + return false; + accum = (accum << 6) | static_cast(value); + bits += 6; + if (bits >= 8) { + bits -= 8; + if (written >= out.size()) + return false; + out[written++] = static_cast((accum >> bits) & 0xFF); + } + } + return written == out.size() && (accum & ((1u << bits) - 1)) == 0; +} + +std::string adminKeyToBase64(const std::array &key) +{ + static const char alphabet[] = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/"; + std::string out; + out.reserve(44); + for (size_t i = 0; i < key.size(); i += 3) { + const size_t remaining = key.size() - i; + uint32_t chunk = static_cast(key[i]) << 16; + if (remaining > 1) + chunk |= static_cast(key[i + 1]) << 8; + if (remaining > 2) + chunk |= key[i + 2]; + out += alphabet[(chunk >> 18) & 0x3F]; + out += alphabet[(chunk >> 12) & 0x3F]; + out += remaining > 1 ? alphabet[(chunk >> 6) & 0x3F] : '='; + out += remaining > 2 ? alphabet[chunk & 0x3F] : '='; + } + return out; +} + bool MAC_from_string(std::string mac_str, uint8_t *dmac) { mac_str.erase(std::remove(mac_str.begin(), mac_str.end(), ':'), mac_str.end()); diff --git a/src/platform/portduino/PortduinoGlue.h b/src/platform/portduino/PortduinoGlue.h index 8b59c6c02e..22fa3f6abb 100644 --- a/src/platform/portduino/PortduinoGlue.h +++ b/src/platform/portduino/PortduinoGlue.h @@ -1,7 +1,9 @@ #pragma once +#include #include #include #include +#include #include #include #include @@ -106,6 +108,13 @@ extern std::ofstream JSONFile; extern std::unique_ptr ch341Hal; int initGPIOPin(int pinNum, const std::string &gpioChipname, int line); bool loadConfig(const char *configPath); + +// Admin keys past the three that fit config.security are also tried by Router's admin-key decrypt +// fallback, so the list is capped to keep that per-packet cost bounded. +constexpr size_t PORTDUINO_MAX_ADMIN_KEYS = 16; +bool adminKeyFromBase64(const std::string &text, std::array &out); +std::string adminKeyToBase64(const std::array &key); + static bool ends_with(std::string_view str, std::string_view suffix); void getMacAddr(uint8_t *dmac); bool MAC_from_string(std::string mac_str, uint8_t *dmac); @@ -275,6 +284,9 @@ extern struct portduino_config_struct { int hostMetrics_interval = 0; int hostMetrics_channel = 0; + // Security + std::vector> admin_keys; + // config bool has_config_overrides = false; int configDisplayMode = 0; @@ -664,6 +676,16 @@ extern struct portduino_config_struct { out << YAML::EndMap; // HostMetrics } + // Security + if (!admin_keys.empty()) { + out << YAML::Key << "Security" << YAML::Value << YAML::BeginMap; + out << YAML::Key << "AdminKeys" << YAML::Value << YAML::BeginSeq; + for (const auto &key : admin_keys) + out << adminKeyToBase64(key); + out << YAML::EndSeq; + out << YAML::EndMap; // Security + } + // config if (has_config_overrides) { out << YAML::Key << "Config" << YAML::Value << YAML::BeginMap; diff --git a/test/fixtures/portduino-config/README.md b/test/fixtures/portduino-config/README.md index 4a73ed0a0b..b70dec451e 100644 --- a/test/fixtures/portduino-config/README.md +++ b/test/fixtures/portduino-config/README.md @@ -161,6 +161,29 @@ non-BLE build. The assertions only test what holds either way. | `bluetooth-bad-enabled.yaml` | `Enabled: maybe` is read with a fallback: silently replaced by `false`, so BLE never turns on. | | `bluetooth-bad-adapter.yaml` | `hci1junk` passes the MAC fallback but is not a BlueZ object path, so BLE silently never starts. | +## Admin keys (`Security.AdminKeys`) + +The host's config is the authority on who may administer the node: it is root-owned, +so unlike the persisted protobuf an already-authorized remote cannot rewrite it. Only +the first three keys fit `config.security.admin_key`; the rest are authorized from +this list alone. + +| File | Expected | +| --------------------------- | --------------------------------------------------------------------------------------------------- | +| `adminkeys.yaml` | Two keys, clean. | +| `adminkeys-bad.yaml` | A key three characters short of 32 bytes. Read without a fallback, so meshtasticd refuses the file. | +| `adminkeys-not-a-list.yaml` | One key written as a scalar: nothing in it would be read, so the file is refused. | + +A short or mistyped key is an error rather than a dropped entry because it would +authorize something other than what was written - and a file whose only admin key is +unreadable would otherwise boot a node nobody can administer. + +`adminkeys-append/` is a whole tree: a `config.yaml` with one key pointing at a +`config.d/` holding two more. Admin keys are the one list that appends across files +rather than the last file winning, so an operator can drop in one file per admin. +`config.yaml` is always loaded first, so the effective list is deterministic and +`--output-yaml` can assert it by value. + ## MAC address The MAC no longer determines NodeNum - that comes from the public key - but a MAC @@ -237,7 +260,8 @@ appears as a surviving pin, a surviving mode row, or a `HIGH` that should be `LO ## Running these as a normal boot `malformed-indent.yaml`, `nonmap-section.yaml`, `module-unknown.yaml`, -`mac-conflict.yaml` and `hub75-unknown-key.yaml` are also run _without_ `--check`, +`mac-conflict.yaml`, `hub75-unknown-key.yaml`, `adminkeys-bad.yaml` and +`adminkeys-not-a-list.yaml` are also run _without_ `--check`, where each must be rejected with a non-zero exit. That is the guard on `--check` mode not having quietly made the normal path permissive. No other fixture is run that way: a config meshtasticd accepts makes it boot a node and block. diff --git a/test/fixtures/portduino-config/adminkeys-append/config.d/more-admins.yaml b/test/fixtures/portduino-config/adminkeys-append/config.d/more-admins.yaml new file mode 100644 index 0000000000..70cc1e0660 --- /dev/null +++ b/test/fixtures/portduino-config/adminkeys-append/config.d/more-admins.yaml @@ -0,0 +1,5 @@ +# Loaded after config.yaml. These are added to its key, not substituted for it. +Security: + AdminKeys: + - NGC+MSAeaf7aoO7ouZl/XHwpmf2v5ZMlPNZUr0361xQ= + - P2jeuoXA1sOGSPH9Vm3eHFlITu1EVnohWGKlGF3t8GU= diff --git a/test/fixtures/portduino-config/adminkeys-append/config.yaml b/test/fixtures/portduino-config/adminkeys-append/config.yaml new file mode 100644 index 0000000000..4d91c53f05 --- /dev/null +++ b/test/fixtures/portduino-config/adminkeys-append/config.yaml @@ -0,0 +1,14 @@ +# Admin keys append across files instead of the last file winning, so an operator can drop one +# file per admin into config.d/. config.yaml is always loaded first, so the effective list is +# deterministic here: this key plus the two in config.d/. +General: + ConfigDirectory: config.d/ +Lora: + Module: sx1262 + CS: 21 + IRQ: 16 + Busy: 20 + Reset: 18 +Security: + AdminKeys: + - rvzaPCZmlh1nWnqNKUFHj0dLBJJIRJhRrkZmaFLHCVY= diff --git a/test/fixtures/portduino-config/adminkeys-bad.yaml b/test/fixtures/portduino-config/adminkeys-bad.yaml new file mode 100644 index 0000000000..e615475a2b --- /dev/null +++ b/test/fixtures/portduino-config/adminkeys-bad.yaml @@ -0,0 +1,13 @@ +# FAULT: the second key is three characters short of 32 bytes. Each entry is read without a +# fallback, because a truncated admin key would authorize something other than what was written, +# so meshtasticd refuses to start on this file. Expect an ERROR and exit 1. +Lora: + Module: sx1262 + CS: 21 + IRQ: 16 + Busy: 20 + Reset: 18 +Security: + AdminKeys: + - rvzaPCZmlh1nWnqNKUFHj0dLBJJIRJhRrkZmaFLHCVY= + - NGC+MSAeaf7aoO7ouZl/XHwpmf2v5ZMlPNZUr036 diff --git a/test/fixtures/portduino-config/adminkeys-not-a-list.yaml b/test/fixtures/portduino-config/adminkeys-not-a-list.yaml new file mode 100644 index 0000000000..43c895c93a --- /dev/null +++ b/test/fixtures/portduino-config/adminkeys-not-a-list.yaml @@ -0,0 +1,11 @@ +# FAULT: one key written as a scalar rather than a list, which is the obvious way to get it wrong +# with a single admin. Nothing in it would be read, so meshtasticd refuses the file rather than +# starting with no admin key. Expect an ERROR and exit 1. +Lora: + Module: sx1262 + CS: 21 + IRQ: 16 + Busy: 20 + Reset: 18 +Security: + AdminKeys: rvzaPCZmlh1nWnqNKUFHj0dLBJJIRJhRrkZmaFLHCVY= diff --git a/test/fixtures/portduino-config/adminkeys.yaml b/test/fixtures/portduino-config/adminkeys.yaml new file mode 100644 index 0000000000..ef6e13d23e --- /dev/null +++ b/test/fixtures/portduino-config/adminkeys.yaml @@ -0,0 +1,12 @@ +# CLEAN. Two admin keys in the host's own config: the first three (all the protobuf holds) are +# mirrored into config.security at boot, and any beyond that are authorized from this list alone. +Lora: + Module: sx1262 + CS: 21 + IRQ: 16 + Busy: 20 + Reset: 18 +Security: + AdminKeys: + - rvzaPCZmlh1nWnqNKUFHj0dLBJJIRJhRrkZmaFLHCVY= + - NGC+MSAeaf7aoO7ouZl/XHwpmf2v5ZMlPNZUr0361xQ= diff --git a/test/test_admin_keys/test_main.cpp b/test/test_admin_keys/test_main.cpp new file mode 100644 index 0000000000..9fd2f20ec8 --- /dev/null +++ b/test/test_admin_keys/test_main.cpp @@ -0,0 +1,273 @@ +// Unit tests for AdminKeys::count/keyAt/isAuthorized in src/mesh/AdminKeys.cpp, and for the +// admin-key base64 codec (adminKeyFromBase64 / adminKeyToBase64) in +// src/platform/portduino/PortduinoGlue.cpp. +// +// Why this behavior is required: four sites decide who may administer a node - the PKC gate in +// AdminModule::handleReceivedProtobuf, the same gate in DMShell::isAuthorized, the is_managed +// guard in AdminModule::handleSetConfig, and the admin-key trial-decrypt fallback in +// Router::perhapsDecode. Each one used to unroll config.security.admin_key[0..2] by hand, so a key +// source other than that array - meshtasticd's Security.AdminKeys, which may carry more keys than +// the three the protobuf holds - would have authorized at some sites and not others. AdminKeys is +// the single list all four read, so it has to answer for both sources at once. +// +// Regressions guarded if these assertions are deleted or relaxed: +// - keyAt() enumerating sparsely: config.security.admin_key can have a populated slot after an +// empty one (a client clearing the middle key), and admin_key_count is not maintained at every +// write site. An enumeration that stops at the first empty slot, or trusts admin_key_count, +// silently drops a configured key - Router's fallback would never try it, and an authorized +// admin could not reach the node. +// - host keys past the protobuf array going unauthorized: applyHostKeys() mirrors only the first +// PROTOBUF_SLOTS of them into config.security, so the rest exist nowhere else. +// - applyHostKeys() either not clearing the slots the host file no longer fills (a key removed +// from the file would keep working from the persisted config) or clearing slots when the host +// configured no keys at all (every non-meshtasticd build, and every host that sets its admin +// keys from a client, would lose them). +// - adminKeyFromBase64 accepting a key that is not exactly 32 canonical bytes. Padding a short +// key with zeros, or truncating a long one, installs an admin key nobody holds, or one that +// collides with another operator's. +#include "Arduino.h" +#include "TestUtil.h" +#include "mesh/AdminKeys.h" +#include "mesh/NodeDB.h" +#include "platform/portduino/PortduinoGlue.h" +#include +#include +#include +#include +#include + +// A 32-byte key whose every byte is fill, so a key is identifiable from keyAt()[0] alone. +static std::array keyOf(uint8_t fill) +{ + std::array key; + key.fill(fill); + return key; +} + +static void setProtobufSlot(size_t slot, uint8_t fill) +{ + memset(config.security.admin_key[slot].bytes, fill, 32); + config.security.admin_key[slot].size = 32; +} + +static void clearProtobufSlot(size_t slot) +{ + memset(config.security.admin_key[slot].bytes, 0, 32); + config.security.admin_key[slot].size = 0; +} + +void setUp(void) +{ + for (size_t slot = 0; slot < AdminKeys::PROTOBUF_SLOTS; slot++) + clearProtobufSlot(slot); + config.security.admin_key_count = 0; + portduino_config.admin_keys.clear(); +} + +void tearDown(void) +{ + setUp(); +} + +// --- No keys configured --- + +void test_no_keys_authorizes_nothing() +{ + TEST_ASSERT_EQUAL_UINT32(0, (uint32_t)AdminKeys::count()); + TEST_ASSERT_FALSE(AdminKeys::any()); + TEST_ASSERT_NULL(AdminKeys::keyAt(0)); + TEST_ASSERT_FALSE(AdminKeys::isAuthorized(keyOf(0x11).data())); +} + +void test_null_public_key_is_not_authorized() +{ + setProtobufSlot(0, 0x11); + TEST_ASSERT_FALSE(AdminKeys::isAuthorized(nullptr)); +} + +// --- Keys in config.security.admin_key --- + +void test_protobuf_key_is_authorized() +{ + setProtobufSlot(0, 0xA1); + TEST_ASSERT_EQUAL_UINT32(1, (uint32_t)AdminKeys::count()); + TEST_ASSERT_TRUE(AdminKeys::any()); + TEST_ASSERT_TRUE(AdminKeys::isAuthorized(keyOf(0xA1).data())); + TEST_ASSERT_FALSE(AdminKeys::isAuthorized(keyOf(0xA2).data())); +} + +void test_sparse_slots_enumerate_densely() +{ + // Slot 1 left empty: keyAt() must not stop there, and must not report it as a key. + setProtobufSlot(0, 0xA1); + setProtobufSlot(2, 0xA3); + TEST_ASSERT_EQUAL_UINT32(2, (uint32_t)AdminKeys::count()); + TEST_ASSERT_NOT_NULL(AdminKeys::keyAt(0)); + TEST_ASSERT_EQUAL_HEX8(0xA1, AdminKeys::keyAt(0)[0]); + TEST_ASSERT_NOT_NULL(AdminKeys::keyAt(1)); + TEST_ASSERT_EQUAL_HEX8(0xA3, AdminKeys::keyAt(1)[0]); + TEST_ASSERT_NULL(AdminKeys::keyAt(2)); + TEST_ASSERT_TRUE(AdminKeys::isAuthorized(keyOf(0xA3).data())); +} + +void test_stale_admin_key_count_is_not_trusted() +{ + // A count of zero against a populated slot is what several write sites leave behind. + setProtobufSlot(0, 0xA1); + config.security.admin_key_count = 0; + TEST_ASSERT_EQUAL_UINT32(1, (uint32_t)AdminKeys::count()); + TEST_ASSERT_TRUE(AdminKeys::isAuthorized(keyOf(0xA1).data())); +} + +void test_short_slot_is_not_a_key() +{ + // Only a full 32-byte key counts; a partially written slot must not authorize anything. + memset(config.security.admin_key[0].bytes, 0xA1, 32); + config.security.admin_key[0].size = 31; + TEST_ASSERT_EQUAL_UINT32(0, (uint32_t)AdminKeys::count()); + TEST_ASSERT_FALSE(AdminKeys::isAuthorized(keyOf(0xA1).data())); +} + +// --- Host keys from config.yaml, past the protobuf array --- + +void test_host_keys_past_protobuf_slots_are_authorized() +{ + // Five host keys, the first three of which NodeDB::loadFromDisk() mirrored into the protobuf. + for (uint8_t i = 0; i < 5; i++) + portduino_config.admin_keys.push_back(keyOf(0xB0 + i)); + for (size_t slot = 0; slot < AdminKeys::PROTOBUF_SLOTS; slot++) + setProtobufSlot(slot, 0xB0 + (uint8_t)slot); + + TEST_ASSERT_EQUAL_UINT32(5, (uint32_t)AdminKeys::count()); + for (uint8_t i = 0; i < 5; i++) + TEST_ASSERT_TRUE(AdminKeys::isAuthorized(keyOf(0xB0 + i).data())); + TEST_ASSERT_FALSE(AdminKeys::isAuthorized(keyOf(0xB5).data())); + + TEST_ASSERT_NOT_NULL(AdminKeys::keyAt(4)); + TEST_ASSERT_EQUAL_HEX8(0xB4, AdminKeys::keyAt(4)[0]); + TEST_ASSERT_NULL(AdminKeys::keyAt(5)); +} + +void test_host_keys_within_protobuf_slots_are_not_counted_twice() +{ + // Two host keys both fit the protobuf array, so they are the same two keys, not four. + portduino_config.admin_keys.push_back(keyOf(0xC1)); + portduino_config.admin_keys.push_back(keyOf(0xC2)); + setProtobufSlot(0, 0xC1); + setProtobufSlot(1, 0xC2); + TEST_ASSERT_EQUAL_UINT32(2, (uint32_t)AdminKeys::count()); +} + +void test_apply_host_keys_mirrors_into_protobuf_slots() +{ + // What NodeDB::loadFromDisk() does at boot: the host's keys take the protobuf slots, whatever a + // client or a remote admin left in them. + for (uint8_t i = 0; i < 5; i++) + portduino_config.admin_keys.push_back(keyOf(0xD0 + i)); + for (size_t slot = 0; slot < AdminKeys::PROTOBUF_SLOTS; slot++) + setProtobufSlot(slot, 0xEE); + + AdminKeys::applyHostKeys(); + TEST_ASSERT_EQUAL_UINT32(AdminKeys::PROTOBUF_SLOTS, (uint32_t)config.security.admin_key_count); + TEST_ASSERT_EQUAL_HEX8(0xD0, config.security.admin_key[0].bytes[0]); + TEST_ASSERT_EQUAL_HEX8(0xD2, config.security.admin_key[2].bytes[0]); + TEST_ASSERT_EQUAL_UINT32(5, (uint32_t)AdminKeys::count()); + TEST_ASSERT_FALSE(AdminKeys::isAuthorized(keyOf(0xEE).data())); +} + +void test_apply_host_keys_clears_slots_the_host_no_longer_fills() +{ + // Removing a key from the host file has to remove its access, not leave it in the persisted + // config where it keeps working. + portduino_config.admin_keys.push_back(keyOf(0xD0)); + for (size_t slot = 0; slot < AdminKeys::PROTOBUF_SLOTS; slot++) + setProtobufSlot(slot, 0xD0 + (uint8_t)slot); + + AdminKeys::applyHostKeys(); + TEST_ASSERT_EQUAL_UINT32(1, (uint32_t)config.security.admin_key_count); + TEST_ASSERT_EQUAL_UINT32(0, (uint32_t)config.security.admin_key[1].size); + TEST_ASSERT_EQUAL_UINT32(0, (uint32_t)config.security.admin_key[2].size); + TEST_ASSERT_EQUAL_UINT32(1, (uint32_t)AdminKeys::count()); + TEST_ASSERT_FALSE(AdminKeys::isAuthorized(keyOf(0xD1).data())); +} + +void test_apply_host_keys_leaves_config_alone_when_host_has_none() +{ + // The common case: no Security.AdminKeys in config.yaml, so keys set by a client must survive. + setProtobufSlot(1, 0xEE); + AdminKeys::applyHostKeys(); + TEST_ASSERT_EQUAL_UINT32(32, (uint32_t)config.security.admin_key[1].size); + TEST_ASSERT_TRUE(AdminKeys::isAuthorized(keyOf(0xEE).data())); +} + +// --- base64 codec --- + +void test_base64_round_trip() +{ + const std::string text = "rvzaPCZmlh1nWnqNKUFHj0dLBJJIRJhRrkZmaFLHCVY="; + std::array key{}; + TEST_ASSERT_TRUE(adminKeyFromBase64(text, key)); + TEST_ASSERT_EQUAL_STRING(text.c_str(), adminKeyToBase64(key).c_str()); +} + +void test_base64_accepts_unpadded_whitespace_and_urlsafe() +{ + std::array padded{}; + std::array other{}; + TEST_ASSERT_TRUE(adminKeyFromBase64("rvzaPCZmlh1nWnqNKUFHj0dLBJJIRJhRrkZmaFLHCVY=", padded)); + TEST_ASSERT_TRUE(adminKeyFromBase64("rvzaPCZmlh1nWnqNKUFHj0dLBJJIRJhRrkZmaFLHCVY", other)); + TEST_ASSERT_EQUAL_HEX8_ARRAY(padded.data(), other.data(), 32); + TEST_ASSERT_TRUE(adminKeyFromBase64(" rvzaPCZmlh1nWnqNKUFHj0dLBJJIRJhRrkZmaFLHCVY=\n", other)); + TEST_ASSERT_EQUAL_HEX8_ARRAY(padded.data(), other.data(), 32); + // The same 32 bytes in the base64url alphabet: '+' and '/' become '-' and '_'. + std::array urlsafe{}; + TEST_ASSERT_TRUE(adminKeyFromBase64("NGC-MSAeaf7aoO7ouZl_XHwpmf2v5ZMlPNZUr0361xQ=", urlsafe)); + std::array standard{}; + TEST_ASSERT_TRUE(adminKeyFromBase64("NGC+MSAeaf7aoO7ouZl/XHwpmf2v5ZMlPNZUr0361xQ=", standard)); + TEST_ASSERT_EQUAL_HEX8_ARRAY(standard.data(), urlsafe.data(), 32); +} + +void test_base64_rejects_wrong_length() +{ + std::array key{}; + TEST_ASSERT_FALSE(adminKeyFromBase64("", key)); + TEST_ASSERT_FALSE(adminKeyFromBase64("AAAA", key)); + // 40 characters, so 30 bytes: short of a key, and not short by a whole base64 group. + TEST_ASSERT_FALSE(adminKeyFromBase64("AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA", key)); + // 47 characters, so 35 bytes: one base64 group too many. + TEST_ASSERT_FALSE(adminKeyFromBase64("rvzaPCZmlh1nWnqNKUFHj0dLBJJIRJhRrkZmaFLHCVYAAAA=", key)); +} + +void test_base64_rejects_bad_characters_and_noncanonical_tail() +{ + std::array key{}; + TEST_ASSERT_FALSE(adminKeyFromBase64("rvzaPCZmlh1nWnqNKUFHj0dLBJJIRJhRrkZmaFLHCV*=", key)); + // 44 characters, but the last one carries bits past the 32nd byte. + TEST_ASSERT_FALSE(adminKeyFromBase64("AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAB=", key)); +} + +// --- Unity lifecycle --- + +void setup() +{ + initializeTestEnvironment(); + UNITY_BEGIN(); + RUN_TEST(test_no_keys_authorizes_nothing); + RUN_TEST(test_null_public_key_is_not_authorized); + RUN_TEST(test_protobuf_key_is_authorized); + RUN_TEST(test_sparse_slots_enumerate_densely); + RUN_TEST(test_stale_admin_key_count_is_not_trusted); + RUN_TEST(test_short_slot_is_not_a_key); + RUN_TEST(test_host_keys_past_protobuf_slots_are_authorized); + RUN_TEST(test_host_keys_within_protobuf_slots_are_not_counted_twice); + RUN_TEST(test_apply_host_keys_mirrors_into_protobuf_slots); + RUN_TEST(test_apply_host_keys_clears_slots_the_host_no_longer_fills); + RUN_TEST(test_apply_host_keys_leaves_config_alone_when_host_has_none); + RUN_TEST(test_base64_round_trip); + RUN_TEST(test_base64_accepts_unpadded_whitespace_and_urlsafe); + RUN_TEST(test_base64_rejects_wrong_length); + RUN_TEST(test_base64_rejects_bad_characters_and_noncanonical_tail); + exit(UNITY_END()); +} + +void loop() {}