diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 335bd032..ae474ca0 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -85,12 +85,6 @@ jobs: run: | make generate - - name: Set Authentication Key - env: - AUTH_KEY: ${{ secrets.AUTH_KEY }} - run: | - echo "#define AUTH_DEFAULT_KEY \"$AUTH_KEY\"" > PROVESFlightControllerReference/Components/TcSecurityDeframer/AuthDefaultKey.h - - name: Build MCUBoot run: | make build-mcuboot @@ -154,12 +148,6 @@ jobs: # here to keep the YAMCS instance config / adapter in sync. make make-ci-spacecraft-id - - name: Set Authentication Key - env: - AUTH_KEY: ${{ secrets.AUTH_KEY }} - run: | - echo "#define AUTH_DEFAULT_KEY \"$AUTH_KEY\"" > PROVESFlightControllerReference/Components/TcSecurityDeframer/AuthDefaultKey.h - - name: Install Framer Plugin run: | make framer-plugin @@ -177,10 +165,18 @@ jobs: pkill -9 python || true - name: Start GDS + env: + PROVES_AUTH_KEY: ${{ secrets.AUTH_KEY }} run: | nohup make gds-integration UART_DEVICE="$UART_DEVICE" > gds-bootstrap.log 2>&1 & echo $! > server.pid + - name: Provision Key + env: + PROVES_AUTH_KEY: ${{ secrets.AUTH_KEY }} + run: | + make test-integration FILTER=provision_key + - name: Sync Sequence Number run: | make test-integration FILTER=sync_sequence_number @@ -205,6 +201,8 @@ jobs: echo "UART_DEVICE=$ZEPHYR_TTY" >> $GITHUB_ENV - name: Start GDS + env: + PROVES_AUTH_KEY: ${{ secrets.AUTH_KEY }} run: | nohup make gds-integration UART_DEVICE="$UART_DEVICE" > gds.log 2>&1 & echo $! > server.pid @@ -284,6 +282,12 @@ jobs: - name: Start YAMCS Stack env: PYTHONUNBUFFERED: "1" + # proves_adapter.py builds the same authenticated framing the GDS + # plugin does, so it needs the key at construction. Now that the key + # is provisioned onto the satellite instead of compiled into the + # image, this step has to be given it explicitly like every other + # step that starts a framing process. + PROVES_AUTH_KEY: ${{ secrets.AUTH_KEY }} run: | # SPACECRAFT_ID=67 matches the override applied by `make # make-ci-spacecraft-id` in the build job so the adapter syncs on @@ -413,12 +417,6 @@ jobs: run: | make submodules fprime-venv - - name: Set Authentication Key - env: - AUTH_KEY: ${{ secrets.AUTH_KEY }} - run: | - echo "#define AUTH_DEFAULT_KEY \"$AUTH_KEY\"" > PROVESFlightControllerReference/Components/TcSecurityDeframer/AuthDefaultKey.h - - name: Install Framer Plugin run: | make framer-plugin @@ -432,6 +430,8 @@ jobs: echo "UART_DEVICE=$ZEPHYR_TTY" >> $GITHUB_ENV - name: Bootstrap Sequence Number over UART + env: + PROVES_AUTH_KEY: ${{ secrets.AUTH_KEY }} run: | # DIVIDER_PRM_SET and lora.TRANSMIT both pass through the authentication # router and require the GDS-side sequence number to match FSW. On a @@ -446,6 +446,11 @@ jobs: BOOTSTRAP_GDS_PID=$! echo $BOOTSTRAP_GDS_PID > server-bootstrap.pid sleep 5 + # Provision the HMAC key on the UART deframer instance; the key store + # is shared board-wide (see TcSecurityDeframer key-store propagation), + # so the LoRa instance picks it up on its next frame without a + # separate provision over radio. + make test-integration FILTER=provision_key # Sync against the LoRa deframer instance: the radio traffic that # follows is validated by it, not by the UART instance this # bootstrap link talks through. @@ -469,6 +474,8 @@ jobs: echo "UART_DEVICE=$ZEPHYR_TTY" >> $GITHUB_ENV - name: Sync Sequence Number over UART + env: + PROVES_AUTH_KEY: ${{ secrets.AUTH_KEY }} run: | nohup make gds-integration UART_DEVICE="$UART_DEVICE" > gds-sync.log 2>&1 & BOOTSTRAP_GDS_PID=$! @@ -522,6 +529,8 @@ jobs: pkill -9 python || true - name: Start GDS on LoRa Passthrough + env: + PROVES_AUTH_KEY: ${{ secrets.AUTH_KEY }} run: | # Capture GDS's own stdout/stderr for the whole session (not just what # pytest captures on failure) so a crash mid-run has a full record of @@ -544,7 +553,7 @@ jobs: env: TLM_SAMPLE_LOG: tlm_sample_radio.csv run: | - make test-integration FILTER="not sync_sequence_number and not format_filesystem and not uart_only" PYTEST_ARGS=--with-radio + make test-integration FILTER="not sync_sequence_number and not format_filesystem and not provision_key and not uart_only" PYTEST_ARGS=--with-radio - name: Format Filesystem if: always() diff --git a/AGENTS.md b/AGENTS.md index a6e88670..dd02b3f8 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -270,7 +270,6 @@ make generate # Generate F Prime build cache (force) make generate-if-needed # Generate only if build directory missing make build # Build firmware (runs generate-if-needed) make build-mcuboot # Build firmware with MCUBoot bootloader signing -make generate-auth-key # Generate AuthDefaultKey.h with a random HMAC key make fmt # Run linters and formatters (pre-commit) make data-budget # Analyze telemetry data budget (use VERBOSE=1 for details) make docs-sync # Sync component SDD files to docs-site/components/ @@ -581,13 +580,22 @@ After compiling, upload the sequence through GDS for execution on the board. ### Authentication & Security -Commands can be HMAC-authenticated using the `TcSecurityDeframer` component. The authentication key is stored in `PROVESFlightControllerReference/Components/TcSecurityDeframer/AuthDefaultKey.h`. +Commands can be HMAC-authenticated using the `TcSecurityDeframer` component. The authentication key is **never compiled into the firmware image** (see issue #220); it lives in a dedicated `keystore_partition` on internal flash, mounted as littlefs at `/keys` (`/keys/authkeys.bin`), separate from the SD-card FAT filesystem used for other storage. A keyless board still boots, so it can be provisioned in the field or on the bench. + +The key store holds up to 2 slots (`{valid, spi, key}`), keyed by the CCSDS Security Parameter Index (SPI) already present in every frame. Three commands manage it: + +- `PROVISION_KEY(spi, key)` — bootstrap. Bypass-allowlisted so it works on a keyless satellite, but the handler only honors it while the store is empty. +- `ADD_KEY(spi, key)` — rotation. Requires an authenticated frame; fails once 2 keys already exist. +- `REMOVE_KEY(spi)` — rotation. Requires an authenticated frame; fails if it would drop the active key count below 1. + +The store is the single source of truth shared by all `TcSecurityDeframer` instances (UART/LoRa/Sband): on an unknown SPI, a deframer reloads the store from disk once and retries before rejecting the frame, so a rotation issued over any one link propagates to the others automatically. The ground plugin (`Framing/src/authenticate_plugin.py`) sources its key from the `--authentication-key` CLI arg or the `PROVES_AUTH_KEY` env var — it no longer reads a compiled-in default. ```bash -make generate-auth-key # Generate a new random HMAC key (only if file doesn't exist) -make copy-secrets SECRETS_DIR= # Copy production keys and auth key from a secure directory +make copy-secrets SECRETS_DIR= # Copy production keys from a secure directory ``` +To provision a keyless board, uplink `PROVISION_KEY(spi=0, key=)` over a trusted bench/CI link (see `PROVESFlightControllerReference/test/int/provision_key_test.py`), then give ground the same key via `PROVES_AUTH_KEY`. + Firmware is signed with MCUBoot for secure boot. The signing key is at `keys/proves.pem` (copied from the MCUBoot test key or a production key). Build signed firmware with: ```bash diff --git a/Framing/src/authenticate_plugin.py b/Framing/src/authenticate_plugin.py index 0b99fc9a..a09c2cbb 100644 --- a/Framing/src/authenticate_plugin.py +++ b/Framing/src/authenticate_plugin.py @@ -19,54 +19,67 @@ SEQUENCE_NUMBER_FILE = os.path.join(_SEQUENCE_NUMBER_DIR, _SEQUENCE_NUMBER_FILENAME) -def get_default_auth_key_from_header() -> str: - """ - Read the authentication key from AuthDefaultKey.h file. +# The flight side (Authenticator.cpp parseHexKey) accepts exactly a 128-bit key as 32 hex +# characters. Anything else either blows up later in bytes.fromhex() during framing or produces +# frames the board silently rejects, so both key sources are normalized and checked here instead. +AUTH_KEY_HEX_LENGTH = 32 + + +def normalize_auth_key(key: str, source: str) -> str: + """Strip any 0x prefix and validate the key is exactly 128 bits of hex. + + Args: + key: The key as supplied by the operator + source: Where it came from, for the error message Returns: - Default authentication key (without 0x prefix) from AuthDefaultKey.h + The key as 32 hex characters, without 0x prefix Raises: - FileNotFoundError: If AuthDefaultKey.h file is not found - ValueError: If AuthDefaultKey.h does not contain a valid key - IOError: If there is an error reading the file + ValueError: If the key is not exactly 32 hexadecimal characters """ - path = ( - "PROVESFlightControllerReference/Components/TcSecurityDeframer/AuthDefaultKey.h" - ) - - if not os.path.exists(path): - raise FileNotFoundError( - f"AuthDefaultKey.h not found at {path}. " - "Authentication plugin requires AuthDefaultKey.h to be present. " - "Ensure the file exists or run 'make generate-auth-key' to create it." + normalized = key.strip() + if normalized[:2].lower() == "0x": + normalized = normalized[2:] + + if len(normalized) != AUTH_KEY_HEX_LENGTH: + raise ValueError( + f"Authentication key from {source} is {len(normalized)} hex characters; " + f"expected exactly {AUTH_KEY_HEX_LENGTH} (a 128-bit key)." ) - try: - with open(path, "r") as f: - for line in f: - # Look for line like: #define AUTH_DEFAULT_KEY "4916d208d40612daad6edbc7333c4c13" - if "AUTH_DEFAULT_KEY" in line and '"' in line: - # Extract key from between quotes - start = line.find('"') + 1 - end = line.find('"', start) - if start > 0 and end > start: - key = line[start:end] - # Remove 0x prefix if present (shouldn't be, but handle it) - if key.startswith("0x") or key.startswith("0X"): - key = key[2:] - return key - except (IOError, OSError) as e: - raise IOError( - f"Error reading AuthDefaultKey.h from {path}: {e}. " - "Authentication plugin cannot proceed without a valid AuthDefaultKey.h file." - ) from e - - # If we get here, file exists but contains no valid key - raise ValueError( - f"No valid key found in {path}. " - 'AuthDefaultKey.h must contain a line with: #define AUTH_DEFAULT_KEY ""' - ) + bytes.fromhex(normalized) + except ValueError as exc: + raise ValueError( + f"Authentication key from {source} is not valid hexadecimal: {exc}" + ) from exc + + return normalized + + +def get_auth_key_from_env() -> str: + """ + Read the authentication key from the PROVES_AUTH_KEY environment variable. + + The key is never compiled into the flight image (see issue #220), so ground + tooling must be told the key out-of-band: via --authentication-key or this + environment variable. + + Returns: + Authentication key as a hex string (without 0x prefix) from PROVES_AUTH_KEY + + Raises: + ValueError: If PROVES_AUTH_KEY is unset or not a 128-bit hex key + """ + key = os.environ.get("PROVES_AUTH_KEY") + if not key: + raise ValueError( + "No authentication key available: pass --authentication-key or set " + "the PROVES_AUTH_KEY environment variable. The key is provisioned " + "onto the satellite with the PROVISION_KEY command and is never " + "compiled into the flight image." + ) + return normalize_auth_key(key, "the PROVES_AUTH_KEY environment variable") # pragma: no cover @@ -87,7 +100,7 @@ def __init__( spi: Security Parameter Index (default: 0) window_size: Window size for authentication (default: 50) authentication_type: Type of authentication (default: "HMAC") - authentication_key: Authentication key as hex string without 0x prefix (default: reads from spi_dict.txt) + authentication_key: Authentication key as hex string without 0x prefix (default: reads from PROVES_AUTH_KEY env var) **kwargs: Additional keyword arguments (ignored for now) """ super().__init__() @@ -101,9 +114,15 @@ def __init__( self.spi = spi self.window_size = window_size self.authentication_type = authentication_type - # Use provided key or read from AuthDefaultKey.h + # Use provided key or read from the PROVES_AUTH_KEY environment variable. Either way the + # key is validated up front, so a malformed key fails at startup with a clear message + # rather than mid-run inside frame(). if authentication_key is None: - authentication_key = get_default_auth_key_from_header() + authentication_key = get_auth_key_from_env() + else: + authentication_key = normalize_auth_key( + authentication_key, "--authentication-key" + ) self.authentication_key = authentication_key def get_sequence_number_from_file(self, filename: str, addition: bool) -> int: @@ -153,12 +172,9 @@ def frame(self, data: bytes) -> bytes: # Security Trailer of 16 octets in length (TM Baseline) # the output MAC is 2*128 bits in total length. (32 bytes) - # Convert hex string to bytes (16 bytes) - # Keys are stored without 0x prefix, but handle it if present for backward compatibility - key_hex = self.authentication_key - if key_hex.startswith("0x") or key_hex.startswith("0X"): - key_hex = key_hex[2:] - key = bytes.fromhex(key_hex) + # Convert hex string to bytes (16 bytes). The key was normalized and validated in + # __init__, so this cannot fail here. + key = bytes.fromhex(self.authentication_key) hmac_object = hmac.new(key, data, hashlib.sha256) @@ -177,11 +193,6 @@ def deframe(self, data: bytes, no_copy=False) -> tuple[bytes, bytes, bytes]: @classmethod def get_arguments(cls) -> dict: """Return CLI argument definitions for this plugin""" - # Get default key from AuthDefaultKey.h for help text - try: - default_key = get_default_auth_key_from_header() - except (FileNotFoundError, ValueError, IOError): - default_key = "" return { ("--spi",): { "type": int, @@ -200,8 +211,8 @@ def get_arguments(cls) -> dict: }, ("--authentication-key",): { "type": str, - "help": f"Authentication key as hex string without 0x prefix (default: {default_key} from AuthDefaultKey.h)", - "default": None, # Will be set to key from AuthDefaultKey.h in __init__ + "help": "Authentication key as hex string without 0x prefix (default: reads from PROVES_AUTH_KEY env var)", + "default": None, # Will be set from PROVES_AUTH_KEY in __init__ }, } diff --git a/Makefile b/Makefile index bb3e819f..d78000ab 100644 --- a/Makefile +++ b/Makefile @@ -132,7 +132,7 @@ docs-build: uv ## Build MkDocs documentation site @$(UVX) --from mkdocs-material mkdocs build .PHONY: generate -generate: submodules fprime-venv zephyr generate-auth-key keys/proves.pem ## Generate FPrime-Zephyr Proves Core Reference +generate: submodules fprime-venv zephyr keys/proves.pem ## Generate FPrime-Zephyr Proves Core Reference @$(UV_RUN) fprime-util generate --force .PHONY: generate-if-needed @@ -153,21 +153,6 @@ ZEPHYR_CONFIG ?= $(BUILD_DIR)/zephyr/.config check-console-disabled: uv ## Fail if the Zephyr UART console is enabled (it corrupts the F' downlink); run after 'make build' @$(UV_RUN) python3 scripts/check_console_disabled.py "$(ZEPHYR_CONFIG)" -##@ Authentication Keys - -AUTH_DEFAULT_KEY_HEADER ?= PROVESFlightControllerReference/Components/TcSecurityDeframer/AuthDefaultKey.h -AUTH_KEY_TEMPLATE ?= scripts/generate_auth_default_key.h - -.PHONY: generate-auth-key -generate-auth-key: ## Generate AuthDefaultKey.h with a random HMAC key - @if [ -f "$(AUTH_DEFAULT_KEY_HEADER)" ]; then \ - echo "$(AUTH_DEFAULT_KEY_HEADER) already exists. Skipping generation."; \ - else \ - echo "Generating $(AUTH_DEFAULT_KEY_HEADER) with random key..."; \ - $(UV_RUN) python3 scripts/generate_auth_key_header.py --output $(AUTH_DEFAULT_KEY_HEADER) --template $(AUTH_KEY_TEMPLATE); \ - fi - @echo "Generated $(AUTH_DEFAULT_KEY_HEADER)" - keys/proves.pem: @mkdir -p keys @cp lib/zephyr-workspace/bootloader/mcuboot/root-rsa-2048.pem keys/proves.pem @@ -224,7 +209,7 @@ test-unit: ## Run unit tests cmake --build build-gtest ctest --test-dir build-gtest -FILTER ?= not sync_sequence_number and not format_filesystem +FILTER ?= not sync_sequence_number and not format_filesystem and not provision_key .PHONY: test-integration test-integration: uv ## Run integration tests (set TEST= or pass test targets) @@ -423,13 +408,13 @@ sequence: fprime-venv ## Compile a sequence file (usage: make sequence SEQ=start @$(UV_RUN) fprime-seqgen sequences/$(SEQ).seq -d $(ARTIFACT_DIR)/zephyr/fprime-zephyr-deployment .PHONY: gds -gds: ## Run FPrime GDS +gds: ## Run FPrime GDS (set SPI= to authenticate with a non-default key slot) @echo "Running FPrime GDS..." @if [ -n "$(UART_DEVICE)" ]; then \ echo "Using UART_DEVICE=$(UART_DEVICE)"; \ - $(GDS_COMMAND) --uart-device $(UART_DEVICE); \ + $(GDS_COMMAND) --uart-device $(UART_DEVICE) $(if $(SPI),--spi $(SPI)); \ fi - $(GDS_COMMAND) + $(GDS_COMMAND) $(if $(SPI),--spi $(SPI)) .PHONY: delete-shadow-gds delete-shadow-gds: @@ -465,7 +450,6 @@ copy-secrets: @mkdir -p ./keys/ @cp $(SECRETS_DIR)/proves.pem ./keys/ @cp $(SECRETS_DIR)/proves.pub.pem ./keys/ - @cp $(SECRETS_DIR)/AuthDefaultKey.h ./PROVESFlightControllerReference/Components/TcSecurityDeframer/ @echo "Copied secret files 🤫" .PHONY: make-ci-spacecraft-id diff --git a/PROVESFlightControllerReference/Components/ProvesRouter/Bypasser.cpp b/PROVESFlightControllerReference/Components/ProvesRouter/Bypasser.cpp index df10645f..7fe1a4d3 100644 --- a/PROVESFlightControllerReference/Components/ProvesRouter/Bypasser.cpp +++ b/PROVESFlightControllerReference/Components/ProvesRouter/Bypasser.cpp @@ -44,6 +44,9 @@ static constexpr uint32_t kBypassOpCodes[] = { 0x2100B000, //!< ComCcsdsUart.tcSecurityDeframer.GET_SEQ_NUM 0x2200B000, //!< ComCcsdsLora.tcSecurityDeframer.GET_SEQ_NUM 0x2300B000, //!< ComCcsdsSband.tcSecurityDeframer.GET_SEQ_NUM + 0x2100B002, //!< ComCcsdsUart.tcSecurityDeframer.PROVISION_KEY + 0x2200B002, //!< ComCcsdsLora.tcSecurityDeframer.PROVISION_KEY + 0x2300B002, //!< ComCcsdsSband.tcSecurityDeframer.PROVISION_KEY 0x10065000, //!< ReferenceDeployment.amateurRadio.TELL_JOKE }; diff --git a/PROVESFlightControllerReference/Components/TcSecurityDeframer/Authenticator.cpp b/PROVESFlightControllerReference/Components/TcSecurityDeframer/Authenticator.cpp index 5e760442..5bbcbdc4 100644 --- a/PROVESFlightControllerReference/Components/TcSecurityDeframer/Authenticator.cpp +++ b/PROVESFlightControllerReference/Components/TcSecurityDeframer/Authenticator.cpp @@ -5,12 +5,14 @@ #include "Authenticator.hpp" -#include #include #include namespace Components { + +static_assert(PacketAuthenticator::kPsaSuccess == PSA_SUCCESS, "kPsaSuccess must mirror PSA_SUCCESS"); + namespace { constexpr size_t kKeyHexLength = @@ -35,8 +37,9 @@ bool hexToNibble(char ch, uint8_t& nibble) { return false; } +} // namespace + // Parse a 32-character hex string (16 bytes) into a byte array. -// Returns true on success and fills `keyBytes` with the parsed bytes. bool parseHexKey(const char* key, uint8_t (&keyBytes)[Ccsds355_0_B_2::kTCSecurityTrailer]) { if (key == nullptr) { return false; @@ -58,22 +61,15 @@ bool parseHexKey(const char* key, uint8_t (&keyBytes)[Ccsds355_0_B_2::kTCSecurit return true; } -} // namespace - -// Import an HMAC key into PSA for message verification. -PacketAuthenticator::KeyImportResult importHmacKey(const char* key, uint32_t& keyId) { +// Import a raw 128-bit HMAC key into PSA for message verification. +PacketAuthenticator::KeyImportResult importHmacKeyBytes(const uint8_t (&keyBytes)[Ccsds355_0_B_2::kTCSecurityTrailer], + uint32_t& keyId) { // Initialize PSA crypto library const psa_status_t initStatus = psa_crypto_init(); if (initStatus != PSA_SUCCESS) { return {PacketAuthenticator::KeyImportStatus::InitError, initStatus}; } - // Parse the hex-encoded default key into raw bytes - uint8_t keyBytes[Ccsds355_0_B_2::kTCSecurityTrailer]; - if (!parseHexKey(key, keyBytes)) { - return {PacketAuthenticator::KeyImportStatus::ParseKeyError, PSA_ERROR_INVALID_ARGUMENT}; - } - // Set up the key attributes psa_key_attributes_t attributes = PSA_KEY_ATTRIBUTES_INIT; psa_set_key_type(&attributes, PSA_KEY_TYPE_HMAC); @@ -84,11 +80,10 @@ PacketAuthenticator::KeyImportResult importHmacKey(const char* key, uint32_t& ke psa_set_key_lifetime(&attributes, PSA_KEY_LIFETIME_VOLATILE); // Import the key into PSA key store - const psa_status_t status = psa_import_key(&attributes, keyBytes, sizeof(keyBytes), &keyId); + const psa_status_t status = psa_import_key(&attributes, keyBytes, Ccsds355_0_B_2::kTCSecurityTrailer, &keyId); // Clean up sensitive data regardless of import outcome psa_reset_key_attributes(&attributes); - mbedtls_platform_zeroize(keyBytes, sizeof keyBytes); if (status != PSA_SUCCESS) { return {PacketAuthenticator::KeyImportStatus::ImportKeyError, status}; @@ -97,10 +92,14 @@ PacketAuthenticator::KeyImportResult importHmacKey(const char* key, uint32_t& ke return {PacketAuthenticator::KeyImportStatus::Success, PSA_SUCCESS}; } +int32_t destroyHmacKey(uint32_t keyId) { + return psa_destroy_key(keyId); +} + PacketAuthenticator::AuthenticationResult authenticatePacket(const uint8_t* dataBuffer, size_t dataSize, const Mac& hmac, - uint32_t& keyId) { + uint32_t keyId) { // Basic input validation: buffer present and at least trailer-sized if (!dataBuffer || dataSize < Ccsds355_0_B_2::kTCSecurityTrailer) { return {PacketAuthenticator::AuthenticationStatus::VerifyError, PSA_ERROR_INVALID_ARGUMENT}; diff --git a/PROVESFlightControllerReference/Components/TcSecurityDeframer/Authenticator.hpp b/PROVESFlightControllerReference/Components/TcSecurityDeframer/Authenticator.hpp index 296fd038..2d1859e1 100644 --- a/PROVESFlightControllerReference/Components/TcSecurityDeframer/Authenticator.hpp +++ b/PROVESFlightControllerReference/Components/TcSecurityDeframer/Authenticator.hpp @@ -17,10 +17,14 @@ namespace PacketAuthenticator { enum class KeyImportStatus { Success, //!< Key was successfully imported InitError, //!< There was an error initializing the authentication process - ParseKeyError, //!< There was an error parsing the key from storage ImportKeyError, //!< There was an error importing the authentication key }; +//! Mirror of PSA_SUCCESS, so callers can check a psaStatus without including +//! (this header is part of the pure-C++ layer covered by the gtest unit tests). Authenticator.cpp +//! static_asserts that it still matches. +constexpr int32_t kPsaSuccess = 0; + struct KeyImportResult { KeyImportStatus status; //!< The status of the key import attempt int32_t psaStatus; //!< The status code returned by the PSA crypto functions @@ -41,9 +45,22 @@ struct AuthenticationResult { } // namespace PacketAuthenticator -//! Import an HMAC key into PSA for message verification. -PacketAuthenticator::KeyImportResult importHmacKey(const char* key, //!< The hex-encoded authentication key to import - uint32_t& keyId //!< The key ID to use for the imported key +//! Parse a 32-character hex string (16 bytes) into a byte array. +//! Returns true on success and fills `keyBytes` with the parsed bytes. +bool parseHexKey(const char* key, //!< The hex-encoded key string to parse + uint8_t (&keyBytes)[Ccsds355_0_B_2::kTCSecurityTrailer] //!< The parsed key bytes +); + +//! Import a raw 128-bit HMAC key into PSA for message verification. +PacketAuthenticator::KeyImportResult importHmacKeyBytes( + const uint8_t (&keyBytes)[Ccsds355_0_B_2::kTCSecurityTrailer], //!< The raw key bytes to import + uint32_t& keyId //!< The key ID to use for the imported key +); + +//! Destroy a previously-imported PSA key. Used to release the old key on rotation. +//! Returns the PSA status: a failed destroy leaves the old key usable in PSA, which the caller +//! must not silently treat as a released slot. +int32_t destroyHmacKey(uint32_t keyId //!< The PSA key ID to destroy ); //! Check the validity of the packet HMAC @@ -51,7 +68,7 @@ PacketAuthenticator::AuthenticationResult authenticatePacket( const uint8_t* buffer, //!< The packet data buffer size_t size, //!< The size of the data buffer const Mac& hmac, //!< The HMAC extracted from the packet to validate against - uint32_t& keyId //!< The hex-encoded authentication key to use for validation + uint32_t keyId //!< The PSA key ID to use for validation ); } // namespace Components diff --git a/PROVESFlightControllerReference/Components/TcSecurityDeframer/TcSecurityDeframer.cpp b/PROVESFlightControllerReference/Components/TcSecurityDeframer/TcSecurityDeframer.cpp index 11a5a036..a75f6d64 100644 --- a/PROVESFlightControllerReference/Components/TcSecurityDeframer/TcSecurityDeframer.cpp +++ b/PROVESFlightControllerReference/Components/TcSecurityDeframer/TcSecurityDeframer.cpp @@ -5,19 +5,81 @@ #include "PROVESFlightControllerReference/Components/TcSecurityDeframer/TcSecurityDeframer.hpp" +#include + #include #include +#include +#include +#include #include #include "Authenticator.hpp" #include "TcSecurityDeframer.hpp" #include "Types.hpp" -// Include generated header with default key (generated at build time) -#include "AuthDefaultKey.h" - namespace Components { +namespace { + +//! Mutex protecting the key store, shared by every TcSecurityDeframer instance. +//! +//! All three instances (UART/LoRa/Sband) back onto a single key store file, so this cannot be a +//! per-instance member: without a shared lock, a PROVISION_KEY/ADD_KEY on one instance's command +//! thread can rewrite the file while another instance's dataIn_handler is reading it on the com +//! thread, which at worst yields a half-old/half-new fixed-layout record whose valid byte is set +//! but whose key bytes are mixed. It also guards each instance's in-memory m_keyStore/m_keyIds, +//! which is per-instance state; one lock covering both is sufficient and keeps the lock order +//! simple, since contention here is limited to key rotation and SPI misses. +//! +//! Function-local static so initialization order relative to component construction is defined. +Os::Mutex& keyStoreLock() { + static Os::Mutex lock; + return lock; +} + +//! Counter bumped by every successful writeKeyStore(), read under keyStoreLock(). +//! +//! An instance whose in-memory copy is at the current generation knows no one has rewritten the +//! store since it last read, so an unknown SPI really is unknown and needs no flash read. Without +//! this, any unauthenticated frame carrying a bogus SPI - SPI validation runs before MAC +//! verification - would drive a littlefs read plus a full PSA destroy/re-import of every slot, on +//! the com thread, holding the lock shared by all three uplinks. +U32& keyStoreGeneration() { + static U32 generation = 0; + return generation; +} + +//! Asks the filesystem holding `filePath` whether it is actually mounted. +//! +//! fs_statvfs resolves the mount point and queries the underlying FS, so a success is direct +//! evidence that /keys is up. This is the signal that makes "stat says the file is missing" safe to +//! act on: on Zephyr, fs_stat returns -ENOENT for an unmounted mount point just as it does for a +//! missing file (subsys/fs/fs.c: fs_get_mnt_point), so absence alone proves nothing. +KeyStore::MountProbe probeMount(const char* filePath) { + char directory[64]; + const char* const lastSlash = std::strrchr(filePath, '/'); + if (lastSlash == nullptr || lastSlash == filePath) { + directory[0] = '/'; + directory[1] = '\0'; + } else { + const size_t length = static_cast(lastSlash - filePath); + if (length >= sizeof directory) { + return KeyStore::MountProbe::Unknown; + } + std::memcpy(directory, filePath, length); + directory[length] = '\0'; + } + + FwSizeType totalBytes = 0; + FwSizeType freeBytes = 0; + return (Os::FileSystem::getFreeSpace(directory, totalBytes, freeBytes) == Os::FileSystem::OP_OK) + ? KeyStore::MountProbe::Live + : KeyStore::MountProbe::Unknown; +} + +} // namespace + // ---------------------------------------------------------------------- // Component construction and destruction // ---------------------------------------------------------------------- @@ -26,7 +88,13 @@ TcSecurityDeframer ::TcSecurityDeframer(const char* const compName) : TcSecurityDeframerComponentBase(compName), m_sequenceNumberFilePath(), m_sequenceNumber(0), - m_sequenceNumberWindow(0) {} + m_sequenceNumberWindow(0), + m_keyStoreFilePath(), + m_keyStore(), + m_keyIds{0}, + // Sentinel: no generation ever takes this value, so the first unknown-SPI check reloads even + // if configure() has not run. + m_keyStoreGeneration(std::numeric_limits::max()) {} TcSecurityDeframer ::~TcSecurityDeframer() {} @@ -54,11 +122,30 @@ void TcSecurityDeframer ::dataIn_handler(FwIndexType portNum, Fw::Buffer& data, this->log_WARNING_HI_ParsingFailed_ThrottleClear(); { - Os::ScopeLock lock(this->m_sequenceNumberLock); + // Lock order: key store lock, then sequence number lock. Every other handler that takes + // both locks (none currently do) must follow the same order to avoid deadlock. + Os::ScopeLock keyLock(keyStoreLock()); + Os::ScopeLock seqLock(this->m_sequenceNumberLock); // --- Validate SPI and anti-replay sequence number --- - const PacketValidator::Status validationStatus = - validatePacket(parseResult.securityHeader, this->m_sequenceNumber, this->m_sequenceNumberWindow); + PacketValidator::Status validationStatus = validatePacket(parseResult.securityHeader, this->m_sequenceNumber, + this->m_sequenceNumberWindow, this->activeSpiSlots()); + + if (validationStatus == PacketValidator::Status::SpiInvalid && + this->m_keyStoreGeneration != keyStoreGeneration()) { + // The key store is shared across all TcSecurityDeframer instances (UART/LoRa/Sband). + // A rotation issued over one link is picked up here so the others don't need their + // own commands re-run: reload from disk once and retry before giving up. + // + // Gated on the generation counter because this path is reachable by an unauthenticated + // frame: SPI validation runs before MAC verification, so without the gate a stream of + // bogus-SPI frames would serialize all three uplinks behind a flash read and a full PSA + // re-import per frame. Every writer of the store is in this process and bumps the + // counter under the same lock, so the reload is exact and free in the common case. + (void)this->loadKeyStore(); + validationStatus = validatePacket(parseResult.securityHeader, this->m_sequenceNumber, + this->m_sequenceNumberWindow, this->activeSpiSlots()); + } if (validationStatus == PacketValidator::Status::SpiInvalid) { this->log_WARNING_HI_SpiInvalid(parseResult.securityHeader.spi); @@ -69,9 +156,15 @@ void TcSecurityDeframer ::dataIn_handler(FwIndexType portNum, Fw::Buffer& data, this->log_WARNING_HI_SpiInvalid_ThrottleClear(); this->log_WARNING_HI_SequenceNumberInvalid_ThrottleClear(); + uint32_t hmacKeyId = 0; + const bool keyFound = this->findKeyIdForSpi(parseResult.securityHeader.spi, hmacKeyId); + // --- Authenticate: HMAC over Security Header + Data Field --- const PacketAuthenticator::AuthenticationResult authResult = - authenticatePacket(data.getData(), data.getSize(), parseResult.securityTrailer.mac, this->m_hmacKeyId); + keyFound + ? authenticatePacket(data.getData(), data.getSize(), parseResult.securityTrailer.mac, hmacKeyId) + : PacketAuthenticator::AuthenticationResult{PacketAuthenticator::AuthenticationStatus::VerifyError, + 0}; if (authResult.status != PacketAuthenticator::AuthenticationStatus::Authenticated) { this->log_WARNING_HI_AuthenticationFailed(static_cast(authResult.status), @@ -150,35 +243,288 @@ void TcSecurityDeframer ::SET_SEQ_NUM_cmdHandler(FwOpcodeType opCode, U32 cmdSeq this->cmdResponse_out(opCode, cmdSeq, Fw::CmdResponse::OK); } +void TcSecurityDeframer ::PROVISION_KEY_cmdHandler(FwOpcodeType opCode, + U32 cmdSeq, + U16 spi, + const Fw::CmdStringArg& key) { + Os::ScopeLock lock(keyStoreLock()); + + // Refresh from disk before inspecting the store: this instance's in-memory copy may predate a + // rotation issued over another link, and the trust-on-first-use check below is only meaningful + // against the store that is actually on flash. + const Os::File::Status loadStatus = this->loadKeyStore(); + + // Trust-on-first-use must be gated on proof that the store is empty, not merely on the absence + // of a key we managed to read. PROVISION_KEY is bypass-allowlisted so it works on a keyless + // board, so if an unreadable store (a truncated record reading back as BAD_SIZE, a littlefs + // error, a mount not ready yet) counted as "keyless", anyone in radio range could induce a read + // failure and install their own key while a valid one still sits on flash. + // + // The proof cannot come from loadStatus alone: on the Zephyr target an absent file reports + // OTHER_ERROR, not DOESNT_EXIST, so gating on the status made a factory-fresh (or /keys-erased) + // board refuse its own bootstrap forever - keyless and unprovisionable, i.e. total command + // loss. probeKeyStore() asks the filesystem instead. See Components::KeyStore in Types.hpp. + KeyStore::MountProbe mountProbe = KeyStore::MountProbe::Unknown; + KeyStore::StoreProbe storeProbe = KeyStore::StoreProbe::Unreadable; + this->probeKeyStore(loadStatus, mountProbe, storeProbe); + + if (!KeyStore::storeStateIsKnown(mountProbe, storeProbe)) { + this->log_WARNING_HI_KeyProvisionFailed(KeyStoreProvisionStatus::StoreUnreadable); + this->cmdResponse_out(opCode, cmdSeq, Fw::CmdResponse::EXECUTION_ERROR); + return; + } + + // PROVISION_KEY is trust-on-first-use bootstrap: only honored while the store is empty. + // Once any key exists, rotation must go through ADD_KEY/REMOVE_KEY (which require auth). + if (!KeyStore::storeIsProvisionable(mountProbe, storeProbe, this->activeKeyCount())) { + this->log_WARNING_HI_KeyProvisionFailed(KeyStoreProvisionStatus::NotEmpty); + this->cmdResponse_out(opCode, cmdSeq, Fw::CmdResponse::EXECUTION_ERROR); + return; + } + + uint8_t keyBytes[Ccsds355_0_B_2::kTCSecurityTrailer]; + if (!parseHexKey(key.toChar(), keyBytes)) { + // parseHexKey fills the buffer as it scans, so a key rejected part-way through leaves a + // prefix of real key bytes behind on this stack frame. + mbedtls_platform_zeroize(keyBytes, sizeof keyBytes); + this->log_WARNING_HI_KeyProvisionFailed(KeyStoreProvisionStatus::ParseKeyError); + this->cmdResponse_out(opCode, cmdSeq, Fw::CmdResponse::EXECUTION_ERROR); + return; + } + + this->m_keyStore[0].set_valid(true); + this->m_keyStore[0].set_spi(spi); + this->m_keyStore[0].set_key(keyBytes); + // The key now lives in m_keyStore; drop the plaintext copy from this stack frame. + mbedtls_platform_zeroize(keyBytes, sizeof keyBytes); + + if (this->writeKeyStore() != Os::File::OP_OK) { + this->m_keyStore[0].set_valid(false); + // Don't leave the rejected key's bytes in an invalid slot: a later successful write would + // persist them to flash. + mbedtls_platform_zeroize(this->m_keyStore[0].get_key(), sizeof(AuthKeySlot::Type_of_key)); + this->log_WARNING_HI_KeyProvisionFailed(KeyStoreProvisionStatus::WriteError); + this->cmdResponse_out(opCode, cmdSeq, Fw::CmdResponse::EXECUTION_ERROR); + return; + } + + // The store is durable at this point. A PSA import failure is reported but not rolled back: + // the key stays on flash so a reboot or a later reload can retry the import, and discarding a + // key the operator may not be able to re-send would be the worse outcome. + const bool imported = this->importKeyStore(); + this->tlmWrite_ActiveKeyCount(this->activeKeyCount()); + if (!imported) { + this->log_WARNING_HI_KeyProvisionFailed(KeyStoreProvisionStatus::ImportError); + this->cmdResponse_out(opCode, cmdSeq, Fw::CmdResponse::EXECUTION_ERROR); + return; + } + + this->log_ACTIVITY_HI_KeyProvisioned(spi); + this->cmdResponse_out(opCode, cmdSeq, Fw::CmdResponse::OK); +} + +void TcSecurityDeframer ::ADD_KEY_cmdHandler(FwOpcodeType opCode, U32 cmdSeq, U16 spi, const Fw::CmdStringArg& key) { + Os::ScopeLock lock(keyStoreLock()); + + // Refresh from disk before the read-modify-write below. Without this, an instance whose + // in-memory store predates a rotation issued over another link would write its stale copy + // back, resurrecting a revoked key on flash and dropping the current one. + const Os::File::Status loadStatus = this->loadKeyStore(); + + // If the store could not be read back, its contents are unknown, and the read-modify-write + // below would persist a guess: writing the last in-memory copy over whatever is actually on + // flash. Refuse rather than risk dropping a live key. As in PROVISION_KEY, "could not be read" + // has to be established by probing the filesystem, not by reading loadStatus. + KeyStore::MountProbe mountProbe = KeyStore::MountProbe::Unknown; + KeyStore::StoreProbe storeProbe = KeyStore::StoreProbe::Unreadable; + this->probeKeyStore(loadStatus, mountProbe, storeProbe); + + if (!KeyStore::storeStateIsKnown(mountProbe, storeProbe)) { + this->log_WARNING_HI_KeyAddFailed(KeyStoreProvisionStatus::StoreUnreadable); + this->cmdResponse_out(opCode, cmdSeq, Fw::CmdResponse::EXECUTION_ERROR); + return; + } + + const U8 count = this->activeKeyCount(); + if (count >= AuthKeyStore::SIZE) { + this->log_WARNING_HI_KeyAddFailed(KeyStoreProvisionStatus::StoreFull); + this->cmdResponse_out(opCode, cmdSeq, Fw::CmdResponse::EXECUTION_ERROR); + return; + } + + // Two slots with the same SPI are unusable: findKeyIdForSpi only ever returns the first match, + // so the new key would authenticate nothing while REMOVE_KEY would clear only one of the two. + if (this->hasSpi(spi)) { + this->log_WARNING_HI_KeyAddFailed(KeyStoreProvisionStatus::DuplicateSpi); + this->cmdResponse_out(opCode, cmdSeq, Fw::CmdResponse::EXECUTION_ERROR); + return; + } + + uint8_t keyBytes[Ccsds355_0_B_2::kTCSecurityTrailer]; + if (!parseHexKey(key.toChar(), keyBytes)) { + // See PROVISION_KEY: a partially-parsed key leaves real key bytes on the stack. + mbedtls_platform_zeroize(keyBytes, sizeof keyBytes); + this->log_WARNING_HI_KeyAddFailed(KeyStoreProvisionStatus::ParseKeyError); + this->cmdResponse_out(opCode, cmdSeq, Fw::CmdResponse::EXECUTION_ERROR); + return; + } + + // Find the first empty slot for the new key + U32 emptySlot = AuthKeyStore::SIZE; + for (U32 i = 0; i < AuthKeyStore::SIZE; i++) { + if (!this->m_keyStore[i].get_valid()) { + emptySlot = i; + break; + } + } + FW_ASSERT(emptySlot < AuthKeyStore::SIZE); + + this->m_keyStore[emptySlot].set_valid(true); + this->m_keyStore[emptySlot].set_spi(spi); + this->m_keyStore[emptySlot].set_key(keyBytes); + // The key now lives in m_keyStore; drop the plaintext copy from this stack frame. + mbedtls_platform_zeroize(keyBytes, sizeof keyBytes); + + if (this->writeKeyStore() != Os::File::OP_OK) { + this->m_keyStore[emptySlot].set_valid(false); + // See PROVISION_KEY: an invalid slot must not carry key bytes into a later write. + mbedtls_platform_zeroize(this->m_keyStore[emptySlot].get_key(), sizeof(AuthKeySlot::Type_of_key)); + this->log_WARNING_HI_KeyAddFailed(KeyStoreProvisionStatus::WriteError); + this->cmdResponse_out(opCode, cmdSeq, Fw::CmdResponse::EXECUTION_ERROR); + return; + } + + // See PROVISION_KEY: a durable store with a failed import is reported, not rolled back. + const bool imported = this->importKeyStore(); + this->tlmWrite_ActiveKeyCount(this->activeKeyCount()); + if (!imported) { + this->log_WARNING_HI_KeyAddFailed(KeyStoreProvisionStatus::ImportError); + this->cmdResponse_out(opCode, cmdSeq, Fw::CmdResponse::EXECUTION_ERROR); + return; + } + + this->log_ACTIVITY_HI_KeyAdded(spi); + this->cmdResponse_out(opCode, cmdSeq, Fw::CmdResponse::OK); +} + +void TcSecurityDeframer ::REMOVE_KEY_cmdHandler(FwOpcodeType opCode, U32 cmdSeq, U16 spi) { + Os::ScopeLock lock(keyStoreLock()); + + // Refresh from disk before the read-modify-write below, for the same reason as ADD_KEY. This + // also keeps the LastKey and SpiNotFound rejections below truthful against the current store + // rather than a stale copy. + const Os::File::Status loadStatus = this->loadKeyStore(); + + // See ADD_KEY: an unreadable store makes the read-modify-write a guess. + KeyStore::MountProbe mountProbe = KeyStore::MountProbe::Unknown; + KeyStore::StoreProbe storeProbe = KeyStore::StoreProbe::Unreadable; + this->probeKeyStore(loadStatus, mountProbe, storeProbe); + + if (!KeyStore::storeStateIsKnown(mountProbe, storeProbe)) { + this->log_WARNING_HI_KeyRemoveFailed(KeyStoreProvisionStatus::StoreUnreadable); + this->cmdResponse_out(opCode, cmdSeq, Fw::CmdResponse::EXECUTION_ERROR); + return; + } + + if (this->activeKeyCount() <= 1) { + this->log_WARNING_HI_KeyRemoveFailed(KeyStoreProvisionStatus::LastKey); + this->cmdResponse_out(opCode, cmdSeq, Fw::CmdResponse::EXECUTION_ERROR); + return; + } + + U32 targetSlot = AuthKeyStore::SIZE; + for (U32 i = 0; i < AuthKeyStore::SIZE; i++) { + if (this->m_keyStore[i].get_valid() && this->m_keyStore[i].get_spi() == spi) { + targetSlot = i; + break; + } + } + + if (targetSlot >= AuthKeyStore::SIZE) { + this->log_WARNING_HI_KeyRemoveFailed(KeyStoreProvisionStatus::SpiNotFound); + this->cmdResponse_out(opCode, cmdSeq, Fw::CmdResponse::EXECUTION_ERROR); + return; + } + + // Clearing `valid` alone would leave the revoked key's raw bytes in the fixed-layout record on + // flash, recoverable by anyone who can read the partition. Wipe the slot's key material too, + // keeping a copy only long enough to restore the slot if the durable write fails. + uint8_t revokedKey[Ccsds355_0_B_2::kTCSecurityTrailer]; + static_assert(sizeof(revokedKey) == sizeof(AuthKeySlot::Type_of_key), "key slot size mismatch"); + std::memcpy(revokedKey, this->m_keyStore[targetSlot].get_key(), sizeof revokedKey); + + this->m_keyStore[targetSlot].set_valid(false); + mbedtls_platform_zeroize(this->m_keyStore[targetSlot].get_key(), sizeof(AuthKeySlot::Type_of_key)); + + const Os::File::Status writeStatus = this->writeKeyStore(); + if (writeStatus != Os::File::OP_OK) { + this->m_keyStore[targetSlot].set_key(revokedKey); + this->m_keyStore[targetSlot].set_valid(true); + } + mbedtls_platform_zeroize(revokedKey, sizeof revokedKey); + + if (writeStatus != Os::File::OP_OK) { + this->log_WARNING_HI_KeyRemoveFailed(KeyStoreProvisionStatus::WriteError); + this->cmdResponse_out(opCode, cmdSeq, Fw::CmdResponse::EXECUTION_ERROR); + return; + } + + // See PROVISION_KEY: a durable store with a failed import is reported, not rolled back. On + // removal the failure can only concern the surviving slot(s), which are re-imported here. + const bool imported = this->importKeyStore(); + this->tlmWrite_ActiveKeyCount(this->activeKeyCount()); + if (!imported) { + this->log_WARNING_HI_KeyRemoveFailed(KeyStoreProvisionStatus::ImportError); + this->cmdResponse_out(opCode, cmdSeq, Fw::CmdResponse::EXECUTION_ERROR); + return; + } + + this->log_ACTIVITY_HI_KeyRemoved(spi); + this->cmdResponse_out(opCode, cmdSeq, Fw::CmdResponse::OK); +} + // ---------------------------------------------------------------------- // Public helper methods // ---------------------------------------------------------------------- void TcSecurityDeframer ::configure() { - Os::ScopeLock lock(this->m_sequenceNumberLock); Fw::ParamValid is_valid; - // Get the sequence number window size from the parameter - this->m_sequenceNumberWindow = this->paramGet_SEQ_NUM_WINDOW(is_valid); - FW_ASSERT(is_valid == Fw::ParamValid::VALID || is_valid == Fw::ParamValid::DEFAULT); + { + Os::ScopeLock lock(this->m_sequenceNumberLock); - // Get the file path from the parameter - this->m_sequenceNumberFilePath = this->paramGet_SEQ_NUM_FILE_PATH(is_valid); - FW_ASSERT(is_valid == Fw::ParamValid::VALID || is_valid == Fw::ParamValid::DEFAULT); + // Get the sequence number window size from the parameter + this->m_sequenceNumberWindow = this->paramGet_SEQ_NUM_WINDOW(is_valid); + FW_ASSERT(is_valid == Fw::ParamValid::VALID || is_valid == Fw::ParamValid::DEFAULT); - // Get the sequence number from the file system. On a read failure (already evented - // by readSequenceNumber) fall back to 0 rather than refusing to boot; the operator - // can correct the counter with SET_SEQ_NUM. - U32 sequenceNumber = 0; - (void)this->readSequenceNumber(sequenceNumber); - this->m_sequenceNumber = sequenceNumber; + // Get the file path from the parameter + this->m_sequenceNumberFilePath = this->paramGet_SEQ_NUM_FILE_PATH(is_valid); + FW_ASSERT(is_valid == Fw::ParamValid::VALID || is_valid == Fw::ParamValid::DEFAULT); - // Telemeter the current sequence number - this->tlmWrite_CurrentSequenceNumber(this->m_sequenceNumber); + // Get the sequence number from the file system. On a read failure (already evented + // by readSequenceNumber) fall back to 0 rather than refusing to boot; the operator + // can correct the counter with SET_SEQ_NUM. + U32 sequenceNumber = 0; + (void)this->readSequenceNumber(sequenceNumber); + this->m_sequenceNumber = sequenceNumber; + + // Telemeter the current sequence number + this->tlmWrite_CurrentSequenceNumber(this->m_sequenceNumber); + } + + { + Os::ScopeLock lock(keyStoreLock()); - // Import the HMAC key - PacketAuthenticator::KeyImportResult result = importHmacKey(AUTH_DEFAULT_KEY, this->m_hmacKeyId); - FW_ASSERT(result.status == PacketAuthenticator::KeyImportStatus::Success); + // Get the key store file path from the parameter + this->m_keyStoreFilePath = this->paramGet_KEY_STORE_FILE_PATH(is_valid); + FW_ASSERT(is_valid == Fw::ParamValid::VALID || is_valid == Fw::ParamValid::DEFAULT); + + // Load the key store from the file system and import any valid keys into PSA. On a + // missing/unreadable store (already evented by loadKeyStore), m_keyStore is left with + // no valid slots: a keyless board must still boot so it can be provisioned. + (void)this->loadKeyStore(); + this->tlmWrite_ActiveKeyCount(this->activeKeyCount()); + } } // ---------------------------------------------------------------------- @@ -217,4 +563,179 @@ Os::File::Status TcSecurityDeframer ::writeSequenceNumber(const U32 value) { return status; } +Os::File::Status TcSecurityDeframer ::loadKeyStore() { + AuthKeyStore store; + Os::File::Status status = Utilities::FileHelper::readFromFile(this->m_keyStoreFilePath.toChar(), store); + if (status == Os::File::OP_OK) { + this->m_keyStore = store; + this->log_WARNING_HI_KeyStoreReadFailed_ThrottleClear(); + } else if (status == Os::File::DOESNT_EXIST) { + // No store file yet: keyless state. A keyless board must still boot so it can be + // provisioned; leave m_keyStore at its default (no valid slots). + this->m_keyStore = AuthKeyStore(); + this->log_WARNING_HI_KeyStoreReadFailed_ThrottleClear(); + } else { + this->log_WARNING_HI_KeyStoreReadFailed(static_cast(status)); + } + + // m_keyStore now reflects the store as of this generation, whether or not the read succeeded: + // on a failure a retry would read the same bytes, so re-reading per frame buys nothing. The + // next writer bumps the generation and forces a fresh attempt. + this->m_keyStoreGeneration = keyStoreGeneration(); + + // Import failures are surfaced to the operator by the command handlers, which call + // importKeyStore() directly; a reload has no command context to report into. + (void)this->importKeyStore(); + return status; +} + +void TcSecurityDeframer ::probeKeyStore(const Os::File::Status loadStatus, + KeyStore::MountProbe& mount, + KeyStore::StoreProbe& store) const { + if (loadStatus == Os::File::OP_OK) { + // A full, successful read is self-evidently proof the filesystem served the file. + mount = KeyStore::MountProbe::Live; + store = KeyStore::StoreProbe::Present; + return; + } + + // The read failed. Do not infer why from its status: ZephyrFile::open collapses every fs_open + // errno into OTHER_ERROR, so on flight hardware a missing file and a corrupt filesystem are + // indistinguishable here. Ask the filesystem directly instead. + // + // Os::FileSystem::getPathType() is deliberately not used: it folds every error into NOT_EXIST, + // which would turn an I/O error into a false "absent" and re-open the very hole this gate + // exists to close. Go through the interface to keep the real status. + Os::FileSystem::PathType pathType = Os::FileSystem::PathType::NOT_EXIST; + const Os::FileSystem::Status statStatus = + Os::FileSystem::getSingleton()._getPathType(this->m_keyStoreFilePath.toChar(), pathType); + + // DOESNT_EXIST from stat is a positive answer ("this path is not there"), but on Zephyr it is + // also what an unmounted /keys reports, so it only counts once probeMount() corroborates it. + store = + (statStatus == Os::FileSystem::DOESNT_EXIST) ? KeyStore::StoreProbe::Absent : KeyStore::StoreProbe::Unreadable; + mount = probeMount(this->m_keyStoreFilePath.toChar()); +} + +Os::File::Status TcSecurityDeframer ::writeKeyStore() { + // Write to a temp file, flush, then rename over the target - the same pattern as + // StartupManager::persist_boot_count, and for a more serious failure mode. Overwriting the + // live store in place meant a reset mid-write (e.g. the watchdog power cycle used for + // command-loss recovery) could leave a truncated file, which reads back as BAD_SIZE and boots + // the board keyless with no authenticated way back in. The rename is not guaranteed power-cut + // atomic on the flight FS, but the new store is fully written and flushed before it replaces + // the old one, so the worst case shrinks to a missing file during the rename window. + Fw::String tempPath(this->m_keyStoreFilePath); + tempPath += ".tmp"; + + Os::File file; + Os::File::Status status = + file.open(tempPath.toChar(), Os::File::Mode::OPEN_CREATE, Os::File::OverwriteType::OVERWRITE); + if (status == Os::File::OP_OK) { + status = Utilities::FileHelper::writeToFile(file, this->m_keyStore); + if (status == Os::File::OP_OK) { + // close() returns void and cannot report a flush failure, so flush explicitly before + // the rename makes the new store authoritative. + status = file.flush(); + } + (void)file.close(); + } + + if (status == Os::File::OP_OK && + Os::FileSystem::rename(tempPath.toChar(), this->m_keyStoreFilePath.toChar()) != Os::FileSystem::OP_OK) { + status = Os::File::OTHER_ERROR; + } + + if (status != Os::File::OP_OK) { + this->log_WARNING_HI_KeyStoreWriteFailed(static_cast(status)); + } else { + this->log_WARNING_HI_KeyStoreWriteFailed_ThrottleClear(); + // Tell the other instances their copy is stale, and record that ours is not: m_keyStore is + // exactly what was just written. + keyStoreGeneration()++; + this->m_keyStoreGeneration = keyStoreGeneration(); + } + + return status; +} + +bool TcSecurityDeframer ::importKeyStore() { + bool allImported = true; + + // Release any previously-imported keys before re-importing, so rotation (and reloads that + // pick up another link's rotation) never leaves a stale key importable in PSA. + for (U32 i = 0; i < AuthKeyStore::SIZE; i++) { + if (this->m_keyIds[i] != 0) { + // A failed destroy leaves the old key live in PSA, so the slot is not actually + // recycled. Nothing more can be done here, but the caller must not report success: + // on REMOVE_KEY in particular that would tell the operator a key is revoked when it + // can still authenticate frames. + if (destroyHmacKey(this->m_keyIds[i]) != PacketAuthenticator::kPsaSuccess) { + allImported = false; + } + this->m_keyIds[i] = 0; + } + } + + for (U32 i = 0; i < AuthKeyStore::SIZE; i++) { + if (!this->m_keyStore[i].get_valid()) { + continue; + } + + uint32_t keyId = 0; + const PacketAuthenticator::KeyImportResult result = importHmacKeyBytes(this->m_keyStore[i].get_key(), keyId); + if (result.status == PacketAuthenticator::KeyImportStatus::Success) { + this->m_keyIds[i] = keyId; + } else { + // The slot stays without a usable PSA key id, so findKeyIdForSpi will miss it and every + // frame for that SPI fails authentication. The store on disk is unaffected, so a + // subsequent reload/rotation can recover once the underlying PSA issue clears - but the + // caller must not report success, or the operator would believe a key is live that the + // board cannot actually use. + allImported = false; + } + } + + return allImported; +} + +bool TcSecurityDeframer ::findKeyIdForSpi(uint32_t spi, uint32_t& keyId) const { + for (U32 i = 0; i < AuthKeyStore::SIZE; i++) { + if (this->m_keyStore[i].get_valid() && this->m_keyStore[i].get_spi() == spi && this->m_keyIds[i] != 0) { + keyId = this->m_keyIds[i]; + return true; + } + } + return false; +} + +bool TcSecurityDeframer ::hasSpi(uint16_t spi) const { + for (U32 i = 0; i < AuthKeyStore::SIZE; i++) { + if (this->m_keyStore[i].get_valid() && this->m_keyStore[i].get_spi() == spi) { + return true; + } + } + return false; +} + +U8 TcSecurityDeframer ::activeKeyCount() const { + U8 count = 0; + for (U32 i = 0; i < AuthKeyStore::SIZE; i++) { + if (this->m_keyStore[i].get_valid()) { + count++; + } + } + return count; +} + +ActiveSpiSlots TcSecurityDeframer ::activeSpiSlots() const { + static_assert(AuthKeyStore::SIZE == kMaxActiveKeys, "ActiveSpiSlots must match AuthKeyStore::SIZE"); + ActiveSpiSlots slots{}; + for (U32 i = 0; i < AuthKeyStore::SIZE; i++) { + slots[i].valid = this->m_keyStore[i].get_valid(); + slots[i].spi = this->m_keyStore[i].get_spi(); + } + return slots; +} + } // namespace Components diff --git a/PROVESFlightControllerReference/Components/TcSecurityDeframer/TcSecurityDeframer.fpp b/PROVESFlightControllerReference/Components/TcSecurityDeframer/TcSecurityDeframer.fpp index f4b0d7c7..5d4fe39b 100644 --- a/PROVESFlightControllerReference/Components/TcSecurityDeframer/TcSecurityDeframer.fpp +++ b/PROVESFlightControllerReference/Components/TcSecurityDeframer/TcSecurityDeframer.fpp @@ -13,6 +13,31 @@ module Components { MacParseError, @< MAC could not be parsed from packet } + @ A single HMAC authentication key slot + struct AuthKeySlot { + valid: bool @< Whether this slot holds an active key + spi: U16 @< Security Parameter Index selecting this key (CCSDS 355.0-B-2 key selector) + key: [16] U8 @< Raw 128-bit HMAC key bytes + } + + @ On-flash store of active authentication keys. Holds at most 2 keys so rotation can add a new + @ key before removing the old one (see issue #220: keys must never be compiled into the image) + array AuthKeyStore = [2] AuthKeySlot + + @ FPP shadow-enum representing Components::PacketKeyStore::ProvisionStatus + enum KeyStoreProvisionStatus { + Ok, @< Command succeeded + NotEmpty, @< PROVISION_KEY was rejected because the store already has a key + StoreFull, @< ADD_KEY was rejected because the store already has 2 keys + LastKey, @< REMOVE_KEY was rejected because it would remove the last remaining key + SpiNotFound, @< REMOVE_KEY was rejected because no slot has the given SPI + ParseKeyError, @< The supplied key was not a valid 32-character hex string + WriteError, @< The updated store could not be written to the file system + ImportError, @< The updated key could not be imported into PSA + DuplicateSpi, @< ADD_KEY was rejected because a slot already holds the given SPI + StoreUnreadable, @< The store could not be read back, so its contents are unknown + } + @ Component placed between the TcDeframer and SpacePacketDeframer components. It @ implements the TC ProcessSecurity flow of CCSDS 355.0-B-2: parse the Security @ Header and Trailer, validate the SPI and anti-replay sequence number, and verify @@ -29,11 +54,26 @@ module Components { @ Command to set the current sequence number sync command SET_SEQ_NUM(seq_num: U32) + @ Command to bootstrap a key onto a keyless satellite. Bypass-allowlisted so it can run + @ before any key exists; rejected once the store already holds a key (use ADD_KEY to rotate) + sync command PROVISION_KEY(spi: U16, key: string size 33) + + @ Command to add a second active key for rotation. Requires an authenticated frame. Fails + @ if the store already has 2 keys + sync command ADD_KEY(spi: U16, key: string size 33) + + @ Command to remove an active key by SPI. Requires an authenticated frame. Fails if it + @ would drop the key count below 1 + sync command REMOVE_KEY(spi: U16) + ### Telemetry ### @ Telemetry for the current sequence number, updated on each successfully authenticated packet telemetry CurrentSequenceNumber : U32 + @ Telemetry for the number of active keys in the key store, updated on configure() and every key store mutation + telemetry ActiveKeyCount : U8 + ### Events ### @ SequenceNumberGet returns the current sequence number from the file system in response to a command @@ -60,13 +100,40 @@ module Components { @ SpiInvalid indicates that a received packet had an invalid SPI value event SpiInvalid(packet_spi: U32) severity warning high id 4 format "SPI invalid: Received={}" throttle 2 + @ KeyStoreReadFailed indicates that there was an error reading the key store from the file system + event KeyStoreReadFailed(status: Os.FileStatus) severity warning high id 16 format "Failed to read key store, error: {}" throttle 2 + + @ KeyStoreWriteFailed indicates that there was an error writing the key store to the file system + event KeyStoreWriteFailed(status: Os.FileStatus) severity warning high id 17 format "Failed to write key store, error: {}" throttle 2 + + @ KeyProvisioned indicates PROVISION_KEY succeeded + event KeyProvisioned(spi: U16) severity activity high id 9 format "Key provisioned for SPI={}" + + @ KeyProvisionFailed indicates PROVISION_KEY was rejected or failed + event KeyProvisionFailed(status: KeyStoreProvisionStatus) severity warning high id 10 format "Key provisioning failed: {}" throttle 2 + + @ KeyAdded indicates ADD_KEY succeeded + event KeyAdded(spi: U16) severity activity high id 11 format "Key added for SPI={}" + + @ KeyAddFailed indicates ADD_KEY was rejected or failed + event KeyAddFailed(status: KeyStoreProvisionStatus) severity warning high id 12 format "Key add failed: {}" throttle 2 + + @ KeyRemoved indicates REMOVE_KEY succeeded + event KeyRemoved(spi: U16) severity activity high id 13 format "Key removed for SPI={}" + + @ KeyRemoveFailed indicates REMOVE_KEY was rejected or failed + event KeyRemoveFailed(status: KeyStoreProvisionStatus) severity warning high id 15 format "Key remove failed: {}" throttle 2 + ### Parameters ### @ Parameter for the sequence numbers window size, used to prevent replay attacks. The window allows no reuse of previous sequence numbers but allows for new sequence numbers to be accepted within the window size param SEQ_NUM_WINDOW : U32 default 50000 @ Parameter for the file path where the current sequence number is stored - param SEQ_NUM_FILE_PATH : string default "//sequence_number.txt" + param SEQ_NUM_FILE_PATH : string default "/keys/sequence_number.bin" + + @ Parameter for the file path where the HMAC authentication key store is stored + param KEY_STORE_FILE_PATH : string default "/keys/authkeys.bin" ### Ports ### diff --git a/PROVESFlightControllerReference/Components/TcSecurityDeframer/TcSecurityDeframer.hpp b/PROVESFlightControllerReference/Components/TcSecurityDeframer/TcSecurityDeframer.hpp index 08040a52..0c09263d 100644 --- a/PROVESFlightControllerReference/Components/TcSecurityDeframer/TcSecurityDeframer.hpp +++ b/PROVESFlightControllerReference/Components/TcSecurityDeframer/TcSecurityDeframer.hpp @@ -13,6 +13,7 @@ #include #include +#include "PROVESFlightControllerReference/Components/TcSecurityDeframer/AuthKeyStoreArrayAc.hpp" #include "PROVESFlightControllerReference/Components/TcSecurityDeframer/Authenticator.hpp" #include "PROVESFlightControllerReference/Components/TcSecurityDeframer/Parser.hpp" #include "PROVESFlightControllerReference/Components/TcSecurityDeframer/TcSecurityDeframerComponentAc.hpp" @@ -74,6 +75,26 @@ class TcSecurityDeframer final : public TcSecurityDeframerComponentBase { U32 seqNum //!< The sequence number to set ) override; + //! Handler implementation for command PROVISION_KEY + void PROVISION_KEY_cmdHandler(FwOpcodeType opCode, //!< The opcode + U32 cmdSeq, //!< The command sequence number + U16 spi, //!< The SPI to provision + const Fw::CmdStringArg& key //!< The hex-encoded key to provision + ) override; + + //! Handler implementation for command ADD_KEY + void ADD_KEY_cmdHandler(FwOpcodeType opCode, //!< The opcode + U32 cmdSeq, //!< The command sequence number + U16 spi, //!< The SPI to add + const Fw::CmdStringArg& key //!< The hex-encoded key to add + ) override; + + //! Handler implementation for command REMOVE_KEY + void REMOVE_KEY_cmdHandler(FwOpcodeType opCode, //!< The opcode + U32 cmdSeq, //!< The command sequence number + U16 spi //!< The SPI to remove + ) override; + public: // ---------------------------------------------------------------------- // Public helper methods @@ -81,7 +102,7 @@ class TcSecurityDeframer final : public TcSecurityDeframerComponentBase { //! Initialize component //! - //! Loads the sequence number from persistent storage + //! Loads the sequence number and key store from persistent storage void configure(); private: @@ -97,6 +118,43 @@ class TcSecurityDeframer final : public TcSecurityDeframerComponentBase { Os::File::Status writeSequenceNumber(const U32 value //!< The sequence number to write ); + //! Loads the key store from the file system into m_keyStore and (re)imports every valid slot + //! into PSA, destroying any previously-imported keys first. Must be called with the key store + //! lock held. On a missing/unreadable file, m_keyStore is left with no valid slots (keyless state). + Os::File::Status loadKeyStore(); + + //! Determines what is actually known about the on-flash key store after a loadKeyStore() + //! attempt, by interrogating the filesystem rather than trusting the read status (which cannot + //! distinguish "missing" from "broken" on the Zephyr target). Feeds the pure + //! Components::KeyStore policy predicates. Must be called with the key store lock held. + void probeKeyStore(const Os::File::Status loadStatus, //!< Status returned by loadKeyStore() + KeyStore::MountProbe& mount, //!< Set to whether /keys is demonstrably mounted + KeyStore::StoreProbe& store //!< Set to what is known about the store file + ) const; + + //! Writes m_keyStore to the file system. Must be called with the key store lock held. + Os::File::Status writeKeyStore(); + + //! (Re)imports every valid slot in m_keyStore into PSA, destroying any previously-imported + //! keys first, and updates m_keyIds. Returns false if any valid slot failed to import, leaving + //! that slot's m_keyIds entry at 0. Must be called with the key store lock held. + bool importKeyStore(); + + //! Finds the PSA key id for the given SPI among currently-imported keys. + //! Must be called with the key store lock held. + bool findKeyIdForSpi(uint32_t spi, uint32_t& keyId) const; + + //! Returns true if any valid slot already holds the given SPI. + //! Must be called with the key store lock held. + bool hasSpi(uint16_t spi) const; + + //! Returns the number of valid slots in m_keyStore. Must be called with the key store lock held. + U8 activeKeyCount() const; + + //! Projects m_keyStore's valid/spi fields into the plain-C++ ActiveSpiSlots type consumed by + //! the pure-C++ Validator. Must be called with the key store lock held. + ActiveSpiSlots activeSpiSlots() const; + private: // ---------------------------------------------------------------------- // Private member variables @@ -109,7 +167,18 @@ class TcSecurityDeframer final : public TcSecurityDeframerComponentBase { U32 m_sequenceNumber; //!< The current sequence number U32 m_sequenceNumberWindow; //!< The allowed window for sequence number validation - uint32_t m_hmacKeyId; //!< The HMAC key ID used for authentication + // Key store state is coupled between in-memory runtime state (m_keyStore/m_keyIds) and on-disk + // persistent storage; both are protected by keyStoreLock() in the .cpp. That mutex is + // deliberately NOT a member: the store file is shared across all TcSecurityDeframer instances + // (UART/LoRa/Sband), so a per-instance lock would let one instance rewrite the file while + // another is reading it. See keyStoreLock() for the full rationale. + Fw::String m_keyStoreFilePath; //!< File path where the key store is stored + AuthKeyStore m_keyStore; //!< The active key store, up to 2 slots + uint32_t m_keyIds[AuthKeyStore::SIZE]; //!< PSA key ids parallel to m_keyStore, valid iff the slot is valid + //! Value of keyStoreGeneration() when m_keyStore was last read from flash. Lets an unknown-SPI + //! frame tell "another instance rotated the store" from "this SPI is simply not ours" without + //! re-reading flash. Initialized to a sentinel no generation takes, so the first check reloads. + U32 m_keyStoreGeneration; }; } // namespace Components diff --git a/PROVESFlightControllerReference/Components/TcSecurityDeframer/Types.hpp b/PROVESFlightControllerReference/Components/TcSecurityDeframer/Types.hpp index 3d4b5937..28c797da 100644 --- a/PROVESFlightControllerReference/Components/TcSecurityDeframer/Types.hpp +++ b/PROVESFlightControllerReference/Components/TcSecurityDeframer/Types.hpp @@ -14,6 +14,67 @@ namespace Components { using Mac = std::array; //!< The MAC field 16 octets in length +constexpr size_t kMaxActiveKeys = 2; //!< Max simultaneously active auth keys (mirrors AuthKeyStore::SIZE) + +//! A single active-key SPI slot. Mirrors the FPP-generated AuthKeySlot's `valid`/`spi` fields +//! without depending on the FPP/F Prime type, so Validator.cpp can stay pure C++. +struct ActiveSpiSlot { + bool valid; //!< Whether this slot holds an active key + uint32_t spi; //!< The SPI associated with the active key +}; + +using ActiveSpiSlots = std::array; //!< The set of currently active SPIs + +//! Key-store admission policy. +//! +//! The decision "may this board be provisioned / may its key store be rewritten" is expressed here +//! as pure C++ over probe results, deliberately away from any Os:: type, so it can be exercised +//! directly by the host gtest suite across the full status matrix. +//! +//! Why it cannot be written directly against Os::File::Status: on the Zephyr target a *genuinely +//! missing* file does not report Os::File::DOESNT_EXIST. ZephyrFile::open discards fs_open's errno +//! and returns OTHER_ERROR for every failure, so DOESNT_EXIST is unreachable on flight hardware +//! while it is the normal result on the POSIX host. Any gate written as +//! `status != OP_OK && status != DOESNT_EXIST -> refuse` therefore passes on the host and refuses +//! on the target, which is exactly how a brand-new board became unprovisionable. The read status +//! alone is not a usable signal; the caller must probe the filesystem for the answer. +namespace KeyStore { + +//! Whether the filesystem holding the key store is demonstrably mounted. +enum class MountProbe { + Live, //!< The mount point answered a statvfs, so the filesystem is up + Unknown, //!< The mount point could not be interrogated: not mounted, not ready, or erroring +}; + +//! What is demonstrably true about the key store file itself. +enum class StoreProbe { + Present, //!< The store was read back in full; its contents are known + Absent, //!< The store was positively confirmed not to exist (stat says so) + Unreadable, //!< The store may or may not exist, and its contents are unknown +}; + +//! True when the on-flash key store's contents are known with certainty. +//! +//! Note the asymmetry: `Present` needs no mount probe, because a successful full read is itself +//! proof the filesystem served the file. `Absent` does need one, because on Zephyr fs_stat reports +//! -ENOENT both for "file missing from a mounted filesystem" and for "no such mount point" +//! (subsys/fs/fs.c fs_get_mnt_point). Treating the latter as emptiness would let an attacker who +//! can keep /keys from mounting provision their own key over a board that still holds a valid one. +constexpr bool storeStateIsKnown(MountProbe mount, StoreProbe store) { + return (store == StoreProbe::Present) || (store == StoreProbe::Absent && mount == MountProbe::Live); +} + +//! True when trust-on-first-use provisioning (PROVISION_KEY) may proceed. +//! +//! PROVISION_KEY is bypass-allowlisted, i.e. reachable unauthenticated over RF, so it is only safe +//! while the store is *proven* empty. Proven means: we know what is on flash (storeStateIsKnown) +//! and it holds no active key. An unreadable store is never provisionable. +constexpr bool storeIsProvisionable(MountProbe mount, StoreProbe store, uint8_t activeKeyCount) { + return storeStateIsKnown(mount, store) && activeKeyCount == 0; +} + +} // namespace KeyStore + //! CCSDS 355.0-B-2 //! https://ccsds.org/Pubs/355x0b2.pdf namespace Ccsds355_0_B_2 { diff --git a/PROVESFlightControllerReference/Components/TcSecurityDeframer/Validator.cpp b/PROVESFlightControllerReference/Components/TcSecurityDeframer/Validator.cpp index 0e494f34..0fc771b1 100644 --- a/PROVESFlightControllerReference/Components/TcSecurityDeframer/Validator.cpp +++ b/PROVESFlightControllerReference/Components/TcSecurityDeframer/Validator.cpp @@ -8,10 +8,14 @@ namespace Components { namespace { -//! Validate the SPI field of the packet -bool spiValid(uint32_t spi) { - // For now we only support SPI 0, which indicates no additional security processing beyond HMAC - return spi == 0; +//! Validate the SPI field of the packet against the active SPI slots: it must match a valid slot +bool spiValid(uint32_t spi, const ActiveSpiSlots& activeSpis) { + for (const ActiveSpiSlot& slot : activeSpis) { + if (slot.valid && slot.spi == spi) { + return true; + } + } + return false; } //! Validate packet sequence number must be greater than the last accepted sequence number and within the window @@ -35,8 +39,9 @@ bool sequenceNumberValid(uint32_t packetSequenceNumber, uint32_t sequenceNumber, PacketValidator::Status validatePacket(const Ccsds355_0_B_2::TCSecurityHeader& secHeader, uint32_t sequenceNumber, - uint32_t sequenceNumberWindow) { - if (!spiValid(secHeader.spi)) { + uint32_t sequenceNumberWindow, + const ActiveSpiSlots& activeSpis) { + if (!spiValid(secHeader.spi, activeSpis)) { return PacketValidator::Status::SpiInvalid; } diff --git a/PROVESFlightControllerReference/Components/TcSecurityDeframer/Validator.hpp b/PROVESFlightControllerReference/Components/TcSecurityDeframer/Validator.hpp index ee16cda7..9689d6ad 100644 --- a/PROVESFlightControllerReference/Components/TcSecurityDeframer/Validator.hpp +++ b/PROVESFlightControllerReference/Components/TcSecurityDeframer/Validator.hpp @@ -26,7 +26,8 @@ enum class Status { PacketValidator::Status validatePacket( const Ccsds355_0_B_2::TCSecurityHeader& secHeader, //!< The parsed security header uint32_t sequenceNumber, //!< The current sequence number - uint32_t sequenceNumberWindow //!< The acceptable sequence number window + uint32_t sequenceNumberWindow, //!< The acceptable sequence number window + const ActiveSpiSlots& activeSpis //!< The active SPI slots, used to validate the SPI ); } // namespace Components diff --git a/PROVESFlightControllerReference/Components/TcSecurityDeframer/docs/sdd.md b/PROVESFlightControllerReference/Components/TcSecurityDeframer/docs/sdd.md index cbdc9687..5343ec95 100644 --- a/PROVESFlightControllerReference/Components/TcSecurityDeframer/docs/sdd.md +++ b/PROVESFlightControllerReference/Components/TcSecurityDeframer/docs/sdd.md @@ -9,10 +9,26 @@ The component does not enforce policy. Frames that fail verification are still f The component is a thin stateful shell over pure-function namespaces: - `Ccsds355_0_B_2::parse` (Parser) — Security Header (SPI, sequence number) and Trailer (MAC) extraction -- `Components::validatePacket` (Validator) — SPI validation and anti-replay sequence-number window validation -- `Components::authenticatePacket` / `importHmacKey` (Authenticator) — HMAC-SHA-256 (truncated to 16 bytes) verification via PSA crypto +- `Components::validatePacket` (Validator) — SPI validation against the active key store and anti-replay sequence-number window validation +- `Components::authenticatePacket` / `parseHexKey` / `importHmacKeyBytes` (Authenticator) — HMAC-SHA-256 (truncated to 16 bytes) verification via PSA crypto -The only component state is the last accepted sequence number (mutex-guarded, persisted to file) and the imported HMAC key id. +`Validator` takes the active SPI set as a plain `ActiveSpiSlots` array (`Types.hpp`) rather than the FPP-generated key store type directly, so it — and its unit tests — stay pure C++ with no F Prime dependency; `TcSecurityDeframer::activeSpiSlots()` projects the real key store into that shape before calling `validatePacket`. + +Component state is the last accepted sequence number and the on-flash key store (each mutex-guarded and persisted to file), plus the PSA key ids imported from the store's valid slots. + +### Key Storage + +The HMAC authentication key is **never compiled into the firmware image** (issue #220). It is persisted at `KEY_STORE_FILE_PATH` (default `/keys/authkeys.bin`) on a dedicated littlefs `keystore_partition` on internal flash, holding up to 2 slots (`{valid, spi, key}`). `configure()` loads the store and imports every valid slot into PSA; a missing/empty store is not an error — a keyless board still boots so it can be provisioned. The sequence-number file lives on the same partition (`SEQ_NUM_FILE_PATH`, default `/keys/sequence_number.bin`). + +The store is shared across all `TcSecurityDeframer` instances (UART/LoRa/Sband): on an unknown SPI, `dataIn_handler` reloads the store from disk once and retries validation before rejecting the frame, so a rotation issued over one link is picked up by the others without a separate command per link. That reload is gated on a process-wide generation counter bumped by every successful store write: SPI validation runs before MAC verification, so the path is reachable by unauthenticated frames, and without the gate a stream of bogus-SPI frames would drive a flash read plus a full PSA re-import per frame on the com thread while holding the lock shared by all three uplinks. + +Three commands manage the store (see [Commands](#commands)): `PROVISION_KEY` (bootstrap, only while the store is empty), `ADD_KEY` (rotation, fails at 2 active keys), and `REMOVE_KEY` (rotation, fails at 1 remaining key). All three re-read the store from flash first, then re-import it into PSA and update `ActiveKeyCount` telemetry on success. + +All three also refuse to act (`StoreUnreadable`) unless the store's contents are *known*, because otherwise they would be acting on a guess. + +"Known" cannot be decided from the read status. On the Zephyr target `ZephyrFile::open` discards `fs_open`'s errno and reports `OTHER_ERROR` for every failure, so `Os::File::DOESNT_EXIST` is unreachable on flight hardware even though it is the normal result for a missing file on the POSIX host. Gating on the status therefore made a factory-fresh (or `/keys`-erased) board refuse `PROVISION_KEY` forever: keyless *and* unprovisionable, i.e. total command loss. Instead `probeKeyStore()` interrogates the filesystem — `fs_stat` on the store file for a positive absence answer, plus `fs_statvfs` on the mount point to prove `/keys` is actually mounted — and feeds the pure predicates in `Components::KeyStore` (`Types.hpp`). Both signals are required: Zephyr returns `-ENOENT` for an unmounted mount point exactly as it does for a missing file, so absence alone proves nothing. For `ADD_KEY`/`REMOVE_KEY` that prevents a read-modify-write from persisting a stale guess over the real store. For `PROVISION_KEY` it is a security property: the command is bypass-allowlisted so it works on a keyless board, so trust-on-first-use must be gated on *proof* that the store is empty. Treating an unreadable store as keyless would let anyone in radio range induce a read failure and install their own key while a valid one still sits on flash. + +`REMOVE_KEY` zeroizes the revoked slot's key bytes before persisting, so a removed key is not recoverable from the on-flash record; the slot is restored if the durable write fails. Primary data path connections: @@ -33,11 +49,21 @@ class TcSecurityDeframer { -dataReturnIn_handler(portNum, data, context) -GET_SEQ_NUM_cmdHandler(opCode, cmdSeq) -SET_SEQ_NUM_cmdHandler(opCode, cmdSeq, seqNum) + -PROVISION_KEY_cmdHandler(opCode, cmdSeq, spi, key) + -ADD_KEY_cmdHandler(opCode, cmdSeq, spi, key) + -REMOVE_KEY_cmdHandler(opCode, cmdSeq, spi) -readSequenceNumber(value) -writeSequenceNumber(value) + -loadKeyStore() + -writeKeyStore() + -importKeyStore() + -findKeyIdForSpi(spi, keyId) + -activeKeyCount() + -activeSpiSlots() -m_sequenceNumber : U32 -m_sequenceNumberWindow : U32 - -m_hmacKeyId : uint32_t + -m_keyStore : AuthKeyStore + -m_keyIds : uint32_t[2] } class Ccsds355_0_B_2 { @@ -47,12 +73,14 @@ class Ccsds355_0_B_2 { class PacketValidator { <> - +validatePacket(secHeader, sequenceNumber, window) Status + +validatePacket(secHeader, sequenceNumber, window, activeSpis) Status } class PacketAuthenticator { <> - +importHmacKey(key, keyId) KeyImportResult + +parseHexKey(key, keyBytes) bool + +importHmacKeyBytes(keyBytes, keyId) KeyImportResult + +destroyHmacKey(keyId) int32_t +authenticatePacket(buffer, size, mac, keyId) AuthenticationResult } @@ -101,19 +129,20 @@ The MAC is HMAC-SHA-256 truncated to 16 bytes, computed over the Security Header ## Behavior 1. Parse the Security Header and Trailer. If the frame is too short to contain them it cannot be stripped for downstream deframing: log ParsingFailed and return the buffer upstream (drop). -2. Validate the SPI (only SPI 0 is currently supported) and the anti-replay sequence number (must be strictly ahead of the last accepted value, within SEQ_NUM_WINDOW, with U32 wraparound handled). -3. If validation passes, verify the MAC. +2. Validate the SPI (must match a valid slot in the active key store — see [Key Storage](#key-storage)) and the anti-replay sequence number (must be strictly ahead of the last accepted value, within SEQ_NUM_WINDOW, with U32 wraparound handled). On an unknown SPI, the key store is reloaded from disk once and validation retried — but only if another instance has written the store since this one last read it — so a rotation issued over another link is picked up here without letting unauthenticated frames drive repeated flash reads. +3. If validation passes, look up the PSA key id for the packet's SPI and verify the MAC with it. 4. Only when all checks pass: store and persist the received sequence number, telemeter it, and set `authenticated = true` in the frame context. Frames failing any check never advance the sequence number (issue #426). 5. Strip the Security Header and Trailer and forward on dataOut with the resulting `authenticated` flag. ProvesRouter rejects unauthenticated packets unless their opcode is on the bypass allowlist. -At startup, `configure()` loads the persisted sequence number and telemeters it so the first downlinked value is correct before any command is accepted (issue #427). +At startup, `configure()` loads the persisted sequence number and key store, telemetering both so the first downlinked values are correct before any command is accepted (issue #427). A missing or empty key store leaves the board keyless (bootable, awaiting `PROVISION_KEY`) rather than failing to boot. ## Parameters | Name | Type | Default | Description | |---|---|---|---| | SEQ_NUM_WINDOW | U32 | 50000 | Maximum allowed forward sequence-number distance before rejecting a packet as out-of-window. | -| SEQ_NUM_FILE_PATH | string | "//sequence_number.txt" | File path used to persist and restore the sequence number across restarts. | +| SEQ_NUM_FILE_PATH | string | "/keys/sequence_number.bin" | File path used to persist and restore the sequence number across restarts. | +| KEY_STORE_FILE_PATH | string | "/keys/authkeys.bin" | File path used to persist and restore the authentication key store across restarts. | ## Port Descriptions @@ -131,6 +160,7 @@ Standard AC ports are also present for command handling, events, telemetry, para | Name | Type | Description | |---|---|---| | CurrentSequenceNumber | U32 | Current accepted sequence number tracked by the component. Emitted at startup and on each accepted packet. | +| ActiveKeyCount | U8 | Number of valid slots (0-2) in the key store. Emitted at startup and after PROVISION_KEY/ADD_KEY/REMOVE_KEY. | Routed/bypassed/rejected packet counts are telemetered by ProvesRouter, which owns the accept/reject policy. @@ -146,6 +176,14 @@ Routed/bypassed/rejected packet counts are telemetered by ProvesRouter, which ow | AuthenticationFailed | Warning High (throttle 2) | auth_status: PacketAuthenticatorStatus, rc: I32 | Logged when MAC verification fails. Format: "Authentication failed: Status={}, PSA Return Code={}" | | ParsingFailed | Warning High (throttle 2) | parse_status: PacketParserStatus | Logged when frame parsing fails. Format: "Parsing failed: {}" | | SpiInvalid | Warning High (throttle 2) | packet_spi: U32 | Logged when SPI validation fails. Format: "SPI invalid: Received={}" | +| KeyStoreReadFailed | Warning High (throttle 2) | status: Os.FileStatus | Logged when the key store read fails (not thrown for a missing file — that's the keyless state). Format: "Failed to read key store, error: {}" | +| KeyStoreWriteFailed | Warning High (throttle 2) | status: Os.FileStatus | Logged when the key store write fails. Format: "Failed to write key store, error: {}" | +| KeyProvisioned | Activity High | spi: U16 | Logged by PROVISION_KEY on success. Format: "Key provisioned for SPI={}" | +| KeyProvisionFailed | Warning High (throttle 2) | status: KeyStoreProvisionStatus | Logged when PROVISION_KEY fails (store not empty, unreadable store, bad hex key, write failure, or PSA import failure). Format: "Key provisioning failed: {}" | +| KeyAdded | Activity High | spi: U16 | Logged by ADD_KEY on success. Format: "Key added for SPI={}" | +| KeyAddFailed | Warning High (throttle 2) | status: KeyStoreProvisionStatus | Logged when ADD_KEY fails (store full, duplicate SPI, unreadable store, bad hex key, write failure, or PSA import failure). Format: "Key add failed: {}" | +| KeyRemoved | Activity High | spi: U16 | Logged by REMOVE_KEY on success. Format: "Key removed for SPI={}" | +| KeyRemoveFailed | Warning High (throttle 2) | status: KeyStoreProvisionStatus | Logged when REMOVE_KEY fails (last remaining key, SPI not found, unreadable store, write failure, or PSA import failure). Format: "Key remove failed: {}" | ## Commands @@ -153,6 +191,9 @@ Routed/bypassed/rejected packet counts are telemetered by ProvesRouter, which ow |---|---|---|---| | GET_SEQ_NUM | Sync | None | Reads and reports the current sequence number (SequenceNumberGet event). | | SET_SEQ_NUM | Sync | seq_num: U32 | Sets and persists a new sequence number (SequenceNumberSet event). | +| PROVISION_KEY | Sync | spi: U16, key: string | Bootstrap: writes the first key slot and imports it into PSA. Only succeeds while the store is empty; bypass-allowlisted in ProvesRouter so it works on a keyless board (KeyProvisioned/KeyProvisionFailed events). | +| ADD_KEY | Sync | spi: U16, key: string | Rotation: adds a key to an empty slot. Requires an authenticated frame (not bypass-allowlisted); fails if 2 keys are already active (KeyAdded/KeyAddFailed events). | +| REMOVE_KEY | Sync | spi: U16 | Rotation: invalidates the slot matching `spi`. Requires an authenticated frame (not bypass-allowlisted); fails if it would drop the active key count below 1 (KeyRemoved/KeyRemoveFailed events). | ## Unit Tests @@ -161,8 +202,11 @@ TcSecurityDeframer helper functionality is covered by unit tests in PROVESFlight | Test File | Coverage | |---|---| | test_TcSecurityDeframer_Parser.cpp | Valid parse path plus parse failures for SPI, sequence number, and MAC size checks. | -| test_TcSecurityDeframer_Validator.cpp | SPI validation, out-of-window and replayed sequence numbers, window boundary, and wraparound handling. | -| test_TcSecurityDeframer_Authenticator.cpp | Key import failures, successful MAC verification, and failed verification with corrupted MAC or data. | +| test_TcSecurityDeframer_Validator.cpp | SPI validation against the active `ActiveSpiSlots` set (single slot, second slot, no valid slots), out-of-window and replayed sequence numbers, window boundary, and wraparound handling. | +| test_TcSecurityDeframer_KeyStorePolicy.cpp | The key store admission predicates (`Components::KeyStore::storeStateIsKnown` / `storeIsProvisionable`) over the exhaustive mount x store-probe x active-key-count matrix: a cold board on a live mount is provisionable; an unreadable store never is; an absent store on an unproven mount never is; and a regression test documenting why a gate written over `Os::File::Status` diverges between host and target. | +| test_TcSecurityDeframer_Authenticator.cpp | `parseHexKey` (valid upper/lowercase, null, wrong length, non-hex characters), key import via hex and raw bytes, `destroyHmacKey`, successful MAC verification, and failed verification with corrupted MAC, corrupted data, or a destroyed key. | + +These cover only the pure-function layer (Parser/Validator/Authenticator; no F Prime or Zephyr dependency). The key store mutation rules enforced in the command handlers (`PROVISION_KEY`/`ADD_KEY`/`REMOVE_KEY` — provision-only-when-empty, add fails at 2, remove fails at 1) are F-Prime-component-dependent and are not covered here; see the commented-out `register_fprime_ut` block in `CMakeLists.txt` for a future on-target/component test pass. Run unit tests with: @@ -180,16 +224,16 @@ make framer-plugin Then run GDS with the framing plugin enabled as configured by the project tooling. -## Generating Keys +## Provisioning Keys -The default authentication key header (AuthDefaultKey.h) is generated at build time from project key material via `make generate-auth-key` or `make copy-secrets`. This generated file is machine-local and not committed. +The authentication key is no longer compiled into the image, so there is nothing to generate at build time. Instead, a keyless board is provisioned after flashing by uplinking `PROVISION_KEY(spi, key)` over a bypass-allowlisted link (the flight-side integration test `PROVESFlightControllerReference/test/int/provision_key_test.py` does this in CI). Ground must be given the same key, via the `--authentication-key` CLI arg or `PROVES_AUTH_KEY` env var to the framing plugin (`Framing/src/authenticate_plugin.py`). `make copy-secrets` still copies production key material from a secure directory for use in provisioning. ## Requirements | Name | Description | Validation | |---|---|---| | AUTH001 | The component shall parse incoming frames to extract the SPI, sequence number, and MAC fields. | Unit Test | -| AUTH003 | The component shall validate that the SPI value corresponds to a configured Security Association. | Unit Test | +| AUTH003 | The component shall validate that the SPI value corresponds to a configured Security Association (an active slot in the on-flash key store). | Unit Test | | AUTH004 | The component shall validate the received sequence number against the stored sequence number. | Unit Test | | AUTH004-A | The component shall not authenticate packets with sequence numbers that are outside the acceptable window and shall log an event. | Unit Test, Inspection | | AUTH004-B | The component shall set the stored sequence number to the sequence number transmitted in the packet only when a packet is fully validated and authenticated. | Inspection | @@ -207,3 +251,4 @@ Opcode-based bypass policy (formerly AUTH002) is owned by ProvesRouter; see its | --- | --- | | 2025-11-26 | Initial design. | | 2026-07-17 | Renamed to TcSecurityDeframer, refactor to discrete responsibilities: Authenticator, Parser, Validator. Pass-through interface between TcDeframer and SpacePacketDeframer; verification result carried in frame context; policy enforcement moved to ProvesRouter. | +| 2026-07-23 | Moved the authentication key off the compiled-in image onto a littlefs key store on internal flash, alongside the sequence-number file (issue #220). Added PROVISION_KEY/ADD_KEY/REMOVE_KEY commands supporting up to 2 active keys; SPI validation now checks the active key store instead of a hard-coded SPI 0. | diff --git a/PROVESFlightControllerReference/ReferenceDeployment/Top/ReferenceDeploymentPackets.fppi b/PROVESFlightControllerReference/ReferenceDeployment/Top/ReferenceDeploymentPackets.fppi index 5ca221ec..f2ad7ef1 100644 --- a/PROVESFlightControllerReference/ReferenceDeployment/Top/ReferenceDeploymentPackets.fppi +++ b/PROVESFlightControllerReference/ReferenceDeployment/Top/ReferenceDeploymentPackets.fppi @@ -171,6 +171,10 @@ telemetry packets ReferenceDeploymentPackets { amateurRadio.count_names + #ComCcsdsSband.tcSecurityDeframer.ActiveKeyCount + ComCcsdsLora.tcSecurityDeframer.ActiveKeyCount + ComCcsdsUart.tcSecurityDeframer.ActiveKeyCount + } packet DetumblePerformance id 16 group 5 { diff --git a/PROVESFlightControllerReference/project/config/CommandDispatcherImplCfg.hpp b/PROVESFlightControllerReference/project/config/CommandDispatcherImplCfg.hpp index b28b21a5..5b2c69b6 100644 --- a/PROVESFlightControllerReference/project/config/CommandDispatcherImplCfg.hpp +++ b/PROVESFlightControllerReference/project/config/CommandDispatcherImplCfg.hpp @@ -11,7 +11,14 @@ // Define configuration values for dispatcher enum { - CMD_DISPATCHER_DISPATCH_TABLE_SIZE = 350, // !< The size of the table holding opcodes to dispatch + // Must be >= the deployment's total command count (see the dictionary's + // `commands` array). CommandDispatcher inserts every opcode into a + // fixed-capacity RedBlackTreeMap and FW_ASSERTs when the insert fails + // (CommandDispatcherImpl.cpp:35), so exceeding this bricks the boot rather + // than degrading. Adding PROVISION_KEY/ADD_KEY/REMOVE_KEY to the + // TcSecurityDeframer instances pushed the count from 348 to 354; headroom + // raised here so the next few commands do not have to repeat this. + CMD_DISPATCHER_DISPATCH_TABLE_SIZE = 512, // !< The size of the table holding opcodes to dispatch CMD_DISPATCHER_SEQUENCER_TABLE_SIZE = 10, // !< The size of the table holding commands in progress }; diff --git a/PROVESFlightControllerReference/test/int/antenna_deployer_test.py b/PROVESFlightControllerReference/test/int/antenna_deployer_test.py index cf724b51..e3ee9469 100644 --- a/PROVESFlightControllerReference/test/int/antenna_deployer_test.py +++ b/PROVESFlightControllerReference/test/int/antenna_deployer_test.py @@ -5,7 +5,7 @@ """ import pytest -from common import proves_send_and_assert_command +from common import exit_safe_mode, proves_send_and_assert_command from fprime_gds.common.data_types.event_data import EventData from fprime_gds.common.testing_fw.api import IntegrationTestAPI @@ -30,6 +30,10 @@ def configure_antenna_deployer(fprime_test_api: IntegrationTestAPI, start_gds): ("MAX_DEPLOY_ATTEMPTS", 3), ] + # Deployment is inhibited while FSW is in SAFE_MODE, which a bench with no + # power at the battery terminals auto-enters on a LOW_BATTERY reading. + exit_safe_mode(fprime_test_api) + proves_send_and_assert_command( fprime_test_api, f"{antenna_deployer}.SET_DEPLOYMENT_STATE", [False] ) diff --git a/PROVESFlightControllerReference/test/int/common.py b/PROVESFlightControllerReference/test/int/common.py index 88ae3f59..627c4274 100644 --- a/PROVESFlightControllerReference/test/int/common.py +++ b/PROVESFlightControllerReference/test/int/common.py @@ -40,6 +40,28 @@ def set_default_retries(n: int) -> None: _DEFAULT_RETRIES = n +def exit_safe_mode(fprime_test_api: IntegrationTestAPI) -> None: + """Command FSW out of SAFE_MODE so that face hardware stays powered. + + On a bench with no power at the battery terminals the power monitor reads + ~0 V, so modeManager auto-enters SAFE_MODE with reason=LOW_BATTERY and the + safe-mode sequence switches the face load switches off. Any subsequent + sensor read on that face then fails with EXECUTION_ERROR. Sending + EXIT_SAFE_MODE first clears the condition long enough for a short test to + run (auto-entry re-arms only after SafeModeDebounceSeconds). + + EXIT_SAFE_MODE is a no-op that still responds OK when the board is already + in NORMAL, so the command completing is a meaningful precondition rather + than something to swallow: if it never lands, deployment stays inhibited and + the face load switches stay off, and the caller's real assertion fails for a + reason that has nothing to do with what it was testing. Asserting here (with + the usual retries) points at the actual cause. + """ + proves_send_and_assert_command( + fprime_test_api, "ReferenceDeployment.modeManager.EXIT_SAFE_MODE" + ) + + def set_radio_recover_fn(fn: Callable[[], None] | None) -> None: """Register a callable to re-establish the radio link. diff --git a/PROVESFlightControllerReference/test/int/conftest.py b/PROVESFlightControllerReference/test/int/conftest.py index 933ebcee..9dec6bac 100644 --- a/PROVESFlightControllerReference/test/int/conftest.py +++ b/PROVESFlightControllerReference/test/int/conftest.py @@ -89,6 +89,17 @@ def pytest_addoption(parser: pytest.Parser) -> None: default=None, help="Override retry count for proves_send_and_assert_command (default: 3 UART, 5 radio).", ) + parser.addoption( + "--no-battery", + action="store_true", + default=False, + help="Bench is running on USB power with nothing at the battery terminals. " + "The power monitor then reads ~0 V, so modeManager auto-enters SAFE_MODE " + "with reason=LOW_BATTERY every SafeModeDebounceSeconds and its safe-mode " + "sequence switches the face load switches off; enough of those cycles " + "wedges the face I2C bus for the rest of the session. This option drops " + "SafeModeEntryVoltage to 0 before each test so the auto-entry never fires.", + ) parser.addoption( "--bare-flight-controler-board", action="store_true", @@ -119,8 +130,11 @@ def start_gds( GDS is used to send commands and receive telemetry/events. """ gds_working = False + attempts = 0 + last_error: Exception | None = None timeout_time = time.time() + 30 while time.time() < timeout_time: + attempts += 1 try: if request.config.getoption("--with-radio"): _enable_radio(fprime_test_api_session) @@ -129,9 +143,16 @@ def start_gds( ) gds_working = True break - except Exception: + except Exception as exc: # noqa: BLE001 - retried until the 30s budget runs out + last_error = exc time.sleep(1) - assert gds_working + # This fixture gates every test in the directory, so a bare assert here reports + # as an unexplained setup error on all of them. Say what actually failed. + assert gds_working, ( + f"No response to {cmdDispatch}.CMD_NO_OP after {attempts} attempts over 30s " + "- the GDS<->board link is not working, so no test in this directory can " + f"run. Last error: {last_error!r}" + ) if request.config.getoption("--with-radio"): # Allow the boot-time event backlog to drain before any test commands @@ -160,6 +181,30 @@ def _enable_radio(fprime_test_api: IntegrationTestAPI) -> None: ) +@pytest.fixture(autouse=True) +def suppress_low_battery_safe_mode( + request: pytest.FixtureRequest, fprime_test_api: IntegrationTestAPI +): + """With --no-battery, keep FSW out of LOW_BATTERY safe mode for each test. + + Applied per test rather than once per session because SAFEMODEENTRYVOLTAGE is + set with PRM_SET (not PRM_SAVE), so it reverts to its 6.7 V default on every + reboot - and the reset_manager tests deliberately reboot mid-suite. + + Fire-and-forget: this is bench scaffolding, and a failure to send it should + show up as the real test failing rather than as a setup error. + """ + if not request.config.getoption("--no-battery", default=False): + return + try: + fprime_test_api.send_command( + "ReferenceDeployment.modeManager.SAFEMODEENTRYVOLTAGE_PRM_SET", [0.0] + ) + fprime_test_api.send_command("ReferenceDeployment.modeManager.EXIT_SAFE_MODE") + except Exception: # noqa: BLE001 - advisory only + pass + + @pytest.fixture(autouse=True) def start_radio(request: pytest.FixtureRequest, fprime_test_api: IntegrationTestAPI): """Fixture to start the radio before tests""" @@ -178,7 +223,6 @@ def start_radio(request: pytest.FixtureRequest, fprime_test_api: IntegrationTest def recover_from_safe_mode( request: pytest.FixtureRequest, fprime_test_api: IntegrationTestAPI, - start_gds, ): """Best-effort: after each test, if FSW slipped into SAFE_MODE (e.g. low voltage brownout from a burnwire-heavy test, or a partial file upload diff --git a/PROVESFlightControllerReference/test/int/drv2605_test.py b/PROVESFlightControllerReference/test/int/drv2605_test.py index 49963330..29011271 100644 --- a/PROVESFlightControllerReference/test/int/drv2605_test.py +++ b/PROVESFlightControllerReference/test/int/drv2605_test.py @@ -12,7 +12,10 @@ from fprime_gds.common.models.serialize.time_type import TimeType from fprime_gds.common.testing_fw.api import IntegrationTestAPI -pytestmark = [pytest.mark.requires_face] +# requires_battery as well as requires_face: the assertion compares INA219 system +# power before and during the magnetorquer pulse, and on an unpowered bench that +# rail reads 0.0 W in both samples. +pytestmark = [pytest.mark.requires_face, pytest.mark.requires_battery] drv2605Manager = "ReferenceDeployment.drv2605Face0Manager" ina219SysManager = "ReferenceDeployment.ina219SysManager" diff --git a/PROVESFlightControllerReference/test/int/mode_manager_test.py b/PROVESFlightControllerReference/test/int/mode_manager_test.py index cb00ae73..2abb1738 100644 --- a/PROVESFlightControllerReference/test/int/mode_manager_test.py +++ b/PROVESFlightControllerReference/test/int/mode_manager_test.py @@ -543,6 +543,10 @@ def test_safe_08_clean_reboot_no_safe_mode( @pytest.mark.slow @pytest.mark.uart_only(reason="Requires reboot and GDS reconnect") +# The reboot half of this test is a hardware power cycle driven by the watchdog, +# so with JP6 open the boot count never increments even though the command-loss +# detection and SAFE_MODE entry it also asserts both work. +@pytest.mark.requires_watchdog_jumper def test_safe_09_command_loss_triggers_safe_mode_and_reboot( fprime_test_api: IntegrationTestAPI, start_gds ): diff --git a/PROVESFlightControllerReference/test/int/provision_key_test.py b/PROVESFlightControllerReference/test/int/provision_key_test.py new file mode 100644 index 00000000..0b42d87f --- /dev/null +++ b/PROVESFlightControllerReference/test/int/provision_key_test.py @@ -0,0 +1,122 @@ +""" +provision_key_test.py: + +This module provisions the HMAC authentication key onto a keyless satellite +so ground and flight share a key. Must run before any auth-required command +is sent, on the bypass-allowlisted link (PROVISION_KEY is refused once the +on-flash key store already holds a key). Idempotent: if the board was +already provisioned by a previous run (the key store lives on internal +flash and survives reflashing), a NotEmpty rejection is treated as success +rather than a failure, since the store already holds the CI secret key. +""" + +import os +import random +import time + +import pytest +from common import FIB_BACKOFF +from fprime_gds.common.data_types.event_data import EventData +from fprime_gds.common.testing_fw.api import IntegrationTestAPI +from fprime_gds.common.testing_fw.predicates import is_a_member_of + + +@pytest.mark.provision_key +def test_provision_key( + fprime_test_api: IntegrationTestAPI, start_gds, request: pytest.FixtureRequest +): + """Provision the HMAC key (spi=0) on a keyless board, tolerating a prior provision""" + link = request.config.getoption("--sync-deframer", default=None) + if link is None: + link = ( + "lora" + if request.config.getoption("--with-radio", default=False) + else "uart" + ) + deframer = { + "uart": "ComCcsdsUart.tcSecurityDeframer", + "lora": "ComCcsdsLora.tcSecurityDeframer", + }[link] + + key = os.environ.get("PROVES_AUTH_KEY") + if not key: + pytest.fail( + "PROVES_AUTH_KEY environment variable not set; cannot provision key" + ) + + # await_event coerces any non-event_predicate into an *id* predicate, so the + # "either outcome" match has to be expressed over event IDs. Passing a list of + # event_predicates here would silently never match (they'd be evaluated against + # an int). + outcome_ids = [ + fprime_test_api.translate_event_name(f"{deframer}.KeyProvisioned"), + fprime_test_api.translate_event_name(f"{deframer}.KeyProvisionFailed"), + ] + + # Every authenticated test downstream depends on this one, so a single dropped uplink must not + # fail the whole provisioning gate. proves_send_and_assert_command can't be used here (the + # outcome is an either/or event pair, and a command response alone doesn't tell us which), so + # retry by hand on the same Fibonacci backoff it uses to absorb LoRa's half-duplex collisions. + evt: EventData | None = None + for attempt in range(len(FIB_BACKOFF)): + fprime_test_api.clear_histories() + fprime_test_api.send_command(f"{deframer}.PROVISION_KEY", ["0", key]) + + evt = fprime_test_api.await_event( + is_a_member_of(outcome_ids), + timeout=10, + ) + if evt is not None: + break + + time.sleep(FIB_BACKOFF[attempt] * random.uniform(0.5, 1.5)) + + assert evt is not None, ( + f"No KeyProvisioned/KeyProvisionFailed event from {deframer} after " + f"{len(FIB_BACKOFF)} PROVISION_KEY attempts; the command may not be reaching the board" + ) + + if evt.template.get_full_name().endswith("KeyProvisionFailed"): + status = evt.args[0].val + # NotEmpty is the ONLY tolerated failure, and only because CI boards keep the key store on + # a littlefs partition that survives reflashing, so every run after the first re-provisions + # a board that is already provisioned. + # + # StoreUnreadable in particular must never be tolerated here. On a board that already holds + # a key it means the store went unreadable (a real fault); on a keyless board it is the + # signature of the cold-provisioning lockout - the board cannot be commanded at all, and + # letting this pass is precisely how that shipped. See test_cold_provision_gap below. + assert status == "NotEmpty", ( + f"PROVISION_KEY failed with unexpected status {status!r}; " + "board should either be keyless or already hold the CI key" + ) + + +@pytest.mark.provision_key +def test_cold_provision_gap(): + """Placeholder for the cold-provision path, which is not yet reachable from CI. + + The bug this file's assertions now guard against (PROVISION_KEY refused with StoreUnreadable on + a board whose key store file does not exist) can only be exercised against a *genuinely blank* + key store. Nothing in the test fleet can produce that state: + + * every bench/CI board was provisioned under an earlier firmware revision, and the littlefs + keystore_partition survives reflashing, so the store is never absent; + * the existing ``fsFormat.FORMAT`` command formats the FatFS root ``/``, NOT the littlefs + ``/keys`` partition, so it cannot clear the store either. + + Automating this therefore needs new firmware capability - either a command that erases the + keystore partition (which must itself be authenticated, since it is a remote-bricking primitive + if it is not) or a CI step that flashes a blank keystore partition image over SWD. Rather than + fake a cold board with a mock that would re-hide the host/target divergence, the invariant is + covered exhaustively at the unit level in + ``test/unit-tests/test_TcSecurityDeframer_KeyStorePolicy.cpp``, and this test is left as an + explicit, visible gap. + + TODO(#472): implement once /keys can be erased or pre-flashed blank from CI. + """ + pytest.skip( + "Cold-provision path needs a blank /keys partition; no mechanism exists yet to produce " + "one from CI (fsFormat.FORMAT targets FatFS /, not littlefs /keys). Covered at unit level " + "by test_TcSecurityDeframer_KeyStorePolicy.cpp." + ) diff --git a/PROVESFlightControllerReference/test/int/rtc_test.py b/PROVESFlightControllerReference/test/int/rtc_test.py index 19ff7178..5820a5cc 100644 --- a/PROVESFlightControllerReference/test/int/rtc_test.py +++ b/PROVESFlightControllerReference/test/int/rtc_test.py @@ -18,7 +18,7 @@ from fprime_gds.common.models.serialize.numerical_types import U32Type from fprime_gds.common.models.serialize.time_type import TimeType from fprime_gds.common.testing_fw.api import IntegrationTestAPI -from fprime_gds.common.testing_fw.predicates import event_predicate +from fprime_gds.common.testing_fw.predicates import event_predicate, is_a_member_of from fprime_gds.common.tools.seqgen import SeqGenException, generateSequence rtcManager = "ReferenceDeployment.rtcManager" @@ -105,6 +105,30 @@ def uplink_sequence_and_await_completion( fprime_test_api.__log(msg, TestLogger.RED) raise fprime_test_api.send_command(f"{fileManager}.CreateDirectory", ["/seq"]) + # CreateDirectory is dispatched asynchronously. Uplinking straight away + # races it, and when /seq does not already exist the receiving end fails + # with FileOpenError before the mkdir lands. Wait for the command to + # resolve either way first - "already exists" comes back as + # DirectoryCreateError, which is just as good for our purposes. + create_evt = fprime_test_api.await_event( + is_a_member_of( + [ + fprime_test_api.translate_event_name( + f"{fileManager}.CreateDirectorySucceeded" + ), + fprime_test_api.translate_event_name( + f"{fileManager}.DirectoryCreateError" + ), + ] + ), + timeout=timeout, + ) + # Falling through on a timeout would put us back in the race this wait exists to close, + # and report it as a confusing FileOpenError/FileReceived timeout further downstream. + assert create_evt is not None, ( + "No CreateDirectorySucceeded/DirectoryCreateError from " + f"{fileManager} within {timeout}s of CreateDirectory" + ) fprime_test_api.uplink_file(temp_bin_path, destination) fprime_test_api.await_event("FileReceived", timeout=timeout) diff --git a/PROVESFlightControllerReference/test/int/tmp112_test.py b/PROVESFlightControllerReference/test/int/tmp112_test.py index be73d011..bf6b6e51 100644 --- a/PROVESFlightControllerReference/test/int/tmp112_test.py +++ b/PROVESFlightControllerReference/test/int/tmp112_test.py @@ -9,7 +9,7 @@ from datetime import datetime import pytest -from common import FIB_BACKOFF, proves_send_and_assert_command +from common import FIB_BACKOFF, exit_safe_mode, proves_send_and_assert_command from fprime_gds.common.data_types.event_data import EventData from fprime_gds.common.models.serialize.numerical_types import F32Type from fprime_gds.common.models.serialize.time_type import TimeType @@ -23,6 +23,9 @@ @pytest.fixture(autouse=True) def setup_test(fprime_test_api: IntegrationTestAPI, start_gds): """Fixture to turn on face 0 before each test""" + # Must precede TURN_ON: entering safe mode switches the face load switch + # back off, so powering the face first would just be undone. + exit_safe_mode(fprime_test_api) proves_send_and_assert_command( fprime_test_api, "ReferenceDeployment.face0LoadSwitch.TURN_ON", diff --git a/PROVESFlightControllerReference/test/int/veml6031_test.py b/PROVESFlightControllerReference/test/int/veml6031_test.py index 7b729b56..fb3f862c 100644 --- a/PROVESFlightControllerReference/test/int/veml6031_test.py +++ b/PROVESFlightControllerReference/test/int/veml6031_test.py @@ -7,7 +7,7 @@ from datetime import datetime import pytest -from common import proves_send_and_assert_command +from common import exit_safe_mode, proves_send_and_assert_command from fprime_gds.common.data_types.event_data import EventData from fprime_gds.common.models.serialize.numerical_types import F32Type from fprime_gds.common.models.serialize.time_type import TimeType @@ -21,6 +21,9 @@ @pytest.fixture(autouse=True) def setup_test(fprime_test_api: IntegrationTestAPI, start_gds): """Fixture to turn on face 0 before each test""" + # Must precede TURN_ON: entering safe mode switches the face load switch + # back off, so powering the face first would just be undone. + exit_safe_mode(fprime_test_api) proves_send_and_assert_command( fprime_test_api, "ReferenceDeployment.face0LoadSwitch.TURN_ON", diff --git a/PROVESFlightControllerReference/test/unit-tests/CMakeLists.txt b/PROVESFlightControllerReference/test/unit-tests/CMakeLists.txt index 389ccaf6..18ec6d5d 100644 --- a/PROVESFlightControllerReference/test/unit-tests/CMakeLists.txt +++ b/PROVESFlightControllerReference/test/unit-tests/CMakeLists.txt @@ -53,7 +53,6 @@ add_library(security_deframer_authenticator STATIC ) target_include_directories(security_deframer_authenticator PUBLIC ${CMAKE_CURRENT_SOURCE_DIR}/../../.. - ${CMAKE_CURRENT_SOURCE_DIR} # for AuthDefaultKey.h ) # RtcManager RtcHelper diff --git a/PROVESFlightControllerReference/test/unit-tests/test_TcSecurityDeframer_Authenticator.cpp b/PROVESFlightControllerReference/test/unit-tests/test_TcSecurityDeframer_Authenticator.cpp index dbcb5e8c..598aff24 100644 --- a/PROVESFlightControllerReference/test/unit-tests/test_TcSecurityDeframer_Authenticator.cpp +++ b/PROVESFlightControllerReference/test/unit-tests/test_TcSecurityDeframer_Authenticator.cpp @@ -17,8 +17,11 @@ static const std::vector kTestPacket = {1, 2, 3, 4, 5, 6 //! Import the test key, asserting success, and return the PSA key id static uint32_t importTestKey() { + uint8_t keyBytes[Ccsds355_0_B_2::kTCSecurityTrailer]; + EXPECT_TRUE(parseHexKey(kTestKeyHex, keyBytes)); + uint32_t keyId = 0; - auto res = importHmacKey(kTestKeyHex, keyId); + auto res = importHmacKeyBytes(keyBytes, keyId); EXPECT_EQ(res.status, PacketAuthenticator::KeyImportStatus::Success); EXPECT_EQ(res.psaStatus, PSA_SUCCESS); return keyId; @@ -31,18 +34,54 @@ static Mac macOf(const std::vector& packet) { return mac; } -TEST(PacketAuthenticatorTest, ImportInvalidHexKey) { - uint32_t keyId = 0; - auto res = importHmacKey("invalidkey", keyId); - EXPECT_EQ(res.status, PacketAuthenticator::KeyImportStatus::ParseKeyError); - EXPECT_EQ(res.psaStatus, PSA_ERROR_INVALID_ARGUMENT); +TEST(ParseHexKeyTest, ValidLowercaseKey) { + uint8_t keyBytes[Ccsds355_0_B_2::kTCSecurityTrailer]; + EXPECT_TRUE(parseHexKey(kTestKeyHex, keyBytes)); + EXPECT_EQ(keyBytes[0], 0x14); + EXPECT_EQ(keyBytes[1], 0x40); + EXPECT_EQ(keyBytes[Ccsds355_0_B_2::kTCSecurityTrailer - 1], 0xfa); +} + +TEST(ParseHexKeyTest, ValidUppercaseKey) { + uint8_t keyBytes[Ccsds355_0_B_2::kTCSecurityTrailer]; + EXPECT_TRUE(parseHexKey("14408C2711281F4D70452CE3730BB4FA", keyBytes)); +} + +TEST(ParseHexKeyTest, NullKeyRejected) { + uint8_t keyBytes[Ccsds355_0_B_2::kTCSecurityTrailer]; + EXPECT_FALSE(parseHexKey(nullptr, keyBytes)); +} + +TEST(ParseHexKeyTest, WrongLengthRejected) { + uint8_t keyBytes[Ccsds355_0_B_2::kTCSecurityTrailer]; + EXPECT_FALSE(parseHexKey("1234", keyBytes)); } -TEST(PacketAuthenticatorTest, ImportNullKey) { +TEST(ParseHexKeyTest, NonHexCharacterRejected) { + uint8_t keyBytes[Ccsds355_0_B_2::kTCSecurityTrailer]; + EXPECT_FALSE(parseHexKey("zz408c2711281f4d70452ce3730bb4fa", keyBytes)); +} + +TEST(PacketAuthenticatorTest, ImportKeyBytesDirectly) { + uint8_t keyBytes[Ccsds355_0_B_2::kTCSecurityTrailer]; + ASSERT_TRUE(parseHexKey(kTestKeyHex, keyBytes)); + uint32_t keyId = 0; - auto res = importHmacKey(nullptr, keyId); - EXPECT_EQ(res.status, PacketAuthenticator::KeyImportStatus::ParseKeyError); - EXPECT_EQ(res.psaStatus, PSA_ERROR_INVALID_ARGUMENT); + auto res = importHmacKeyBytes(keyBytes, keyId); + EXPECT_EQ(res.status, PacketAuthenticator::KeyImportStatus::Success); + EXPECT_EQ(res.psaStatus, PSA_SUCCESS); + EXPECT_NE(keyId, 0u); + + destroyHmacKey(keyId); +} + +TEST(PacketAuthenticatorTest, DestroyedKeyFailsToVerify) { + uint32_t keyId = importTestKey(); + destroyHmacKey(keyId); + + auto res = authenticatePacket(kTestPacket.data(), kTestPacket.size(), macOf(kTestPacket), keyId); + EXPECT_EQ(res.status, PacketAuthenticator::AuthenticationStatus::VerifyError); + EXPECT_NE(res.psaStatus, PSA_SUCCESS); } TEST(PacketAuthenticatorTest, NullBuffer) { diff --git a/PROVESFlightControllerReference/test/unit-tests/test_TcSecurityDeframer_KeyStorePolicy.cpp b/PROVESFlightControllerReference/test/unit-tests/test_TcSecurityDeframer_KeyStorePolicy.cpp new file mode 100644 index 00000000..79c1c40c --- /dev/null +++ b/PROVESFlightControllerReference/test/unit-tests/test_TcSecurityDeframer_KeyStorePolicy.cpp @@ -0,0 +1,142 @@ +// ====================================================================== +// \title test_TcSecurityDeframer_KeyStorePolicy.cpp +// \brief Unit tests for the pure key-store admission policy (Components::KeyStore) +// +// These tests guard a bug class that a component-level host test structurally cannot catch: the +// original defect only manifested on the Zephyr target, because ZephyrFile::open collapses every +// fs_open errno into Os::File::OTHER_ERROR, while the POSIX host maps ENOENT to +// Os::File::DOESNT_EXIST. A gate written as "refuse unless OP_OK or DOESNT_EXIST" therefore passed +// every host test and bricked a factory-fresh board. +// +// The fix pushes the decision behind a pure predicate over *probe results* rather than over a read +// status, so the divergence has nowhere to hide: there is no "missing file" status input to get +// wrong, and the matrix below is exhaustive over the predicate's entire domain. +// ====================================================================== + +#include + +#include "PROVESFlightControllerReference/Components/TcSecurityDeframer/Types.hpp" + +using namespace Components; +using KeyStore::MountProbe; +using KeyStore::storeIsProvisionable; +using KeyStore::StoreProbe; +using KeyStore::storeStateIsKnown; + +namespace { + +constexpr MountProbe kMounts[] = {MountProbe::Live, MountProbe::Unknown}; +constexpr StoreProbe kStores[] = {StoreProbe::Present, StoreProbe::Absent, StoreProbe::Unreadable}; + +} // namespace + +// ---------------------------------------------------------------------- +// The bootstrap path: a keyless board must be able to accept its first key +// ---------------------------------------------------------------------- + +//! The C1 regression itself: a brand-new board (/keys freshly formatted, store file never written) +//! must be provisionable, or it is permanently keyless and unreachable by command. +TEST(KeyStorePolicy, ColdBoardOnLiveMountIsProvisionable) { + EXPECT_TRUE(storeIsProvisionable(MountProbe::Live, StoreProbe::Absent, 0)); +} + +//! A store file that exists and reads back cleanly but holds no valid slot is equally proven empty. +TEST(KeyStorePolicy, ReadableButEmptyStoreIsProvisionable) { + EXPECT_TRUE(storeIsProvisionable(MountProbe::Live, StoreProbe::Present, 0)); + // A successful read is its own proof the filesystem is up, so it does not need a mount probe. + EXPECT_TRUE(storeIsProvisionable(MountProbe::Unknown, StoreProbe::Present, 0)); +} + +// ---------------------------------------------------------------------- +// The security property: trust-on-first-use may never overwrite a live key +// ---------------------------------------------------------------------- + +//! PROVISION_KEY is bypass-allowlisted (unauthenticated, reachable over RF), so once any key is on +//! flash it must be refused; rotation has to go through the authenticated ADD_KEY/REMOVE_KEY. +TEST(KeyStorePolicy, ProvisionedBoardIsNotProvisionable) { + for (uint8_t count = 1; count <= kMaxActiveKeys; count++) { + EXPECT_FALSE(storeIsProvisionable(MountProbe::Live, StoreProbe::Present, count)) + << "count=" << static_cast(count); + } +} + +//! The core hardening property: if we cannot tell what is on flash, we must not write to it. An +//! attacker who can induce a read failure (glitching, a corrupt record, a wedged littlefs) must not +//! thereby be able to install their own key over a valid one. +TEST(KeyStorePolicy, UnreadableStoreIsNeverProvisionable) { + for (MountProbe mount : kMounts) { + for (uint8_t count = 0; count <= kMaxActiveKeys; count++) { + EXPECT_FALSE(storeIsProvisionable(mount, StoreProbe::Unreadable, count)); + EXPECT_FALSE(storeStateIsKnown(mount, StoreProbe::Unreadable)); + } + } +} + +//! The trap in the naive fix. On Zephyr, fs_open/fs_stat return -ENOENT both for "file missing from +//! a mounted filesystem" and for "that mount point does not exist" (subsys/fs/fs.c +//! fs_get_mnt_point). So mapping ENOENT straight to "absent, therefore empty, therefore +//! provisionable" would let anyone who can keep /keys from mounting take over a board that still +//! holds a valid key. Absence only counts when the mount independently answered. +TEST(KeyStorePolicy, AbsentFileWithoutLiveMountIsNeverProvisionable) { + EXPECT_FALSE(storeIsProvisionable(MountProbe::Unknown, StoreProbe::Absent, 0)); + EXPECT_FALSE(storeStateIsKnown(MountProbe::Unknown, StoreProbe::Absent)); +} + +// ---------------------------------------------------------------------- +// Exhaustive matrix +// ---------------------------------------------------------------------- + +//! Pin the entire domain, so any future widening of the predicate has to be a deliberate edit here. +TEST(KeyStorePolicy, ExhaustiveMatrix) { + for (MountProbe mount : kMounts) { + for (StoreProbe store : kStores) { + const bool expectKnown = + (store == StoreProbe::Present) || (store == StoreProbe::Absent && mount == MountProbe::Live); + EXPECT_EQ(expectKnown, storeStateIsKnown(mount, store)); + + for (uint8_t count = 0; count <= kMaxActiveKeys; count++) { + EXPECT_EQ(expectKnown && count == 0, storeIsProvisionable(mount, store, count)); + } + } + } +} + +// ---------------------------------------------------------------------- +// Host/target divergence documentation +// ---------------------------------------------------------------------- + +//! Mirrors of the Os::File::Status values a key-store read can produce, kept as plain ints so this +//! pure test needs no Os:: dependency. The point of this test is the *classification* step that +//! TcSecurityDeframer::probeKeyStore performs, and specifically that the classification is not +//! allowed to depend on the read status. +enum class ReadStatus { OpOk, DoesntExist, OtherError, BadSize }; + +//! What the old (buggy) gate concluded, purely from the read status. +bool legacyGateAllowed(ReadStatus status) { + return status == ReadStatus::OpOk || status == ReadStatus::DoesntExist; +} + +//! This is the whole bug in one assertion. On the POSIX host a missing file reads back as +//! DOESNT_EXIST and the legacy gate lets provisioning through; on the Zephyr target the identical +//! situation reads back as OTHER_ERROR and the legacy gate refuses. Same board state, opposite +//! answer, decided by which Os layer you happened to compile against. Any future gate that consumes +//! a raw read status reintroduces this. +TEST(KeyStorePolicy, LegacyStatusGateDivergesBetweenHostAndTarget) { + // The same physical condition - key store file genuinely absent: + const ReadStatus onHost = ReadStatus::DoesntExist; // POSIX open() sets ENOENT + const ReadStatus onTarget = ReadStatus::OtherError; // ZephyrFile::open discards fs_open's errno + + EXPECT_TRUE(legacyGateAllowed(onHost)); + EXPECT_FALSE(legacyGateAllowed(onTarget)); + EXPECT_NE(legacyGateAllowed(onHost), legacyGateAllowed(onTarget)) + << "A missing key store must not produce different decisions on host and target"; + + // The replacement predicate takes probe results, not a status, so both platforms feed it the + // same StoreProbe::Absent + MountProbe::Live and get the same answer. + EXPECT_TRUE(storeIsProvisionable(MountProbe::Live, StoreProbe::Absent, 0)); + + // And OTHER_ERROR that is *not* a missing file (a truncated record, a wedged FS) still lands on + // Unreadable, which is refused - the hardening property survives. + EXPECT_FALSE(storeIsProvisionable(MountProbe::Live, StoreProbe::Unreadable, 0)); + EXPECT_FALSE(storeIsProvisionable(MountProbe::Live, StoreProbe::Unreadable, 1)); +} diff --git a/PROVESFlightControllerReference/test/unit-tests/test_TcSecurityDeframer_Validator.cpp b/PROVESFlightControllerReference/test/unit-tests/test_TcSecurityDeframer_Validator.cpp index 998276d3..54f4afbd 100644 --- a/PROVESFlightControllerReference/test/unit-tests/test_TcSecurityDeframer_Validator.cpp +++ b/PROVESFlightControllerReference/test/unit-tests/test_TcSecurityDeframer_Validator.cpp @@ -5,27 +5,58 @@ using namespace Components; using Header = Ccsds355_0_B_2::TCSecurityHeader; +namespace { + +//! Build an ActiveSpiSlots with a single valid slot at the given SPI +ActiveSpiSlots singleActiveSpi(uint32_t spi) { + return ActiveSpiSlots{{{true, spi}, {false, 0u}}}; +} + +//! Build an ActiveSpiSlots with no valid slots (keyless state) +ActiveSpiSlots noActiveSpis() { + return ActiveSpiSlots{{{false, 0u}, {false, 0u}}}; +} + +//! Build an ActiveSpiSlots with two valid slots (rotation state) +ActiveSpiSlots twoActiveSpis(uint32_t spiA, uint32_t spiB) { + return ActiveSpiSlots{{{true, spiA}, {true, spiB}}}; +} + +} // namespace + TEST(PacketValidatorTest, ValidPacket) { Header h{0u, 11u}; // spi 0 valid, seq 11 within window of expected 10 - auto res = validatePacket(h, 10u, 5u); + auto res = validatePacket(h, 10u, 5u, singleActiveSpi(0u)); EXPECT_EQ(res, PacketValidator::Status::Valid); } TEST(PacketValidatorTest, SpiInvalid) { - Header h{1u, 11u}; // non-zero SPI invalid - auto res = validatePacket(h, 10u, 5u); + Header h{1u, 11u}; // spi 1 is not among the active slots + auto res = validatePacket(h, 10u, 5u, singleActiveSpi(0u)); + EXPECT_EQ(res, PacketValidator::Status::SpiInvalid); +} + +TEST(PacketValidatorTest, SpiInvalidWhenStoreEmpty) { + Header h{0u, 11u}; // keyless board: no slot is valid, so no SPI matches + auto res = validatePacket(h, 10u, 5u, noActiveSpis()); EXPECT_EQ(res, PacketValidator::Status::SpiInvalid); } +TEST(PacketValidatorTest, SpiValidInSecondSlot) { + Header h{7u, 11u}; // rotation state: spi 7 lives in the second active slot + auto res = validatePacket(h, 10u, 5u, twoActiveSpis(0u, 7u)); + EXPECT_EQ(res, PacketValidator::Status::Valid); +} + TEST(PacketValidatorTest, SequenceNumberInvalid) { Header h{0u, 20u}; // sequence 20, expected 10, window 5 -> out - auto res = validatePacket(h, 10u, 5u); + auto res = validatePacket(h, 10u, 5u, singleActiveSpi(0u)); EXPECT_EQ(res, PacketValidator::Status::SequenceNumberInvalid); } TEST(PacketValidatorTest, SequenceNumberReplayRejected) { Header h{0u, 9u}; // below last accepted -> replay - auto res = validatePacket(h, 10u, 5u); + auto res = validatePacket(h, 10u, 5u, singleActiveSpi(0u)); EXPECT_EQ(res, PacketValidator::Status::SequenceNumberInvalid); } @@ -36,7 +67,7 @@ TEST(PacketValidatorTest, SequenceNumberWrapWithinWindow) { // distance = seq - expected = 1 - 0xFFFFFFFE = 3 (mod 2^32) Header h{0u, seq}; - auto res = validatePacket(h, expected, 5u); + auto res = validatePacket(h, expected, 5u, singleActiveSpi(0u)); EXPECT_EQ(res, PacketValidator::Status::Valid); } @@ -47,18 +78,18 @@ TEST(PacketValidatorTest, SequenceNumberWrapOutOfWindow) { // distance = 10 - 0xFFFFFFF0 = 26 (mod 2^32) Header h{0u, seq}; - auto res = validatePacket(h, expected, 5u); + auto res = validatePacket(h, expected, 5u, singleActiveSpi(0u)); EXPECT_EQ(res, PacketValidator::Status::SequenceNumberInvalid); } TEST(PacketValidatorTest, SequenceNumberEqualToExpected) { Header h{0u, 10u}; // sequence equal to last accepted must be rejected (no reuse) - auto res = validatePacket(h, 10u, 5u); + auto res = validatePacket(h, 10u, 5u, singleActiveSpi(0u)); EXPECT_EQ(res, PacketValidator::Status::SequenceNumberInvalid); } TEST(PacketValidatorTest, SequenceNumberAtWindowBoundary) { Header h{0u, 15u}; // expected 10, window 5 -> distance = 5 = window -> valid - auto res = validatePacket(h, 10u, 5u); + auto res = validatePacket(h, 10u, 5u, singleActiveSpi(0u)); EXPECT_EQ(res, PacketValidator::Status::Valid); } diff --git a/README.md b/README.md index 1ad4c26e..d3338976 100644 --- a/README.md +++ b/README.md @@ -152,7 +152,23 @@ MCUBoot only boots images that are **signed with the same key** the bootloader i If you regenerate/replace the bootloader (or switch computers and flash a bootloader built elsewhere), make sure you also update `keys/proves.pem` to the matching signing key, or your built images will not boot. -You also want to make sure the authentication key the gds runs with is the same as the authentication key on the board. For that, you want to make sure the authentication key in PROVESFlightControllerReference/Components/TcSecurityDeframer/AuthDefaultKey.h matches. +You also want to make sure the authentication key the gds runs with is the same as the authentication key provisioned on the board. The board's key lives in its on-flash key store (never in the image); ground reads its key from the `--authentication-key` CLI arg or the `PROVES_AUTH_KEY` env var. Make sure these match the key you provisioned with `PROVISION_KEY`/`ADD_KEY`. + +##### Provisioning your first key + +A freshly flashed board boots with an empty on-flash key store — no authentication key ever ships in the image. Because the store is empty, the board allows exactly one unauthenticated command: `PROVISION_KEY`. Once any key is provisioned, `PROVISION_KEY` is refused, so this only works the first time (or again after every key has been removed). + +1. Start GDS as normal (`make gds`). +2. From the GDS command view, send `PROVISION_KEY(spi=, key=<32 hex chars>)`, e.g. `PROVISION_KEY(0, 00112233445566778899aabbccddeeff)`. +3. Tell ground to use the same key for every command after that: set `PROVES_AUTH_KEY` to the same hex string (or pass `--authentication-key`). If you provisioned a non-zero SPI, also run gds with `make gds SPI=` (or pass `--spi ` directly) so ground's outgoing frames carry the matching SPI. + +The board holds up to two active keys so a rotation never leaves you locked out. Ground uses exactly one key at a time (whichever `--authentication-key`/`PROVES_AUTH_KEY` it was started with), so rotate in this order: + +1. Keep running GDS with the **old** key and send `ADD_KEY(new_spi, new_key)` — the command itself has to authenticate under the old key. +2. Restart GDS with the **new** key (and `--spi new_spi`), and confirm commands are accepted. +3. Only then send `REMOVE_KEY(old_spi)`, authenticated under the new key. + +Doing step 3 before step 2 works too, but leaves nothing to fall back on if the new key turns out to be wrong. `REMOVE_KEY` refuses to remove the last remaining key. ## Running Integration Tests diff --git a/boards/bronco_space/proves_flight_control_board_v5/proves_flight_control_board_v5.dtsi b/boards/bronco_space/proves_flight_control_board_v5/proves_flight_control_board_v5.dtsi index 5b9a1132..96813f80 100644 --- a/boards/bronco_space/proves_flight_control_board_v5/proves_flight_control_board_v5.dtsi +++ b/boards/bronco_space/proves_flight_control_board_v5/proves_flight_control_board_v5.dtsi @@ -23,6 +23,17 @@ disk-access; mount-point = "/"; }; + lfs1: lfs1 { + compatible = "zephyr,fstab,littlefs"; + mount-point = "/keys"; + partition = <&keystore_partition>; + automount; + read-size = <16>; + prog-size = <16>; + cache-size = <64>; + lookahead-size = <32>; + block-cycles = <512>; + }; }; @@ -63,7 +74,7 @@ zephyr_udc0: &usbd { }; &flash0 { - reg = <0x10000000 DT_SIZE_M(4)>; + reg = <0x10000000 DT_SIZE_M(16)>; partitions { compatible = "fixed-partitions"; #address-cells = <0x1>; @@ -89,9 +100,14 @@ zephyr_udc0: &usbd { reg = <0x300000 0x100000>; }; - storage_partition: partition@400000 { + keystore_partition: partition@400000 { + label = "keystore_partition"; + reg = <0x400000 0x40000>; + }; + + storage_partition: partition@440000 { label = "storage_partition"; - reg = <0x400000 0xC00000>; + reg = <0x440000 0xBC0000>; }; }; }; diff --git a/docs-site/components/TcSecurityDeframer.md b/docs-site/components/TcSecurityDeframer.md index cbdc9687..5343ec95 100644 --- a/docs-site/components/TcSecurityDeframer.md +++ b/docs-site/components/TcSecurityDeframer.md @@ -9,10 +9,26 @@ The component does not enforce policy. Frames that fail verification are still f The component is a thin stateful shell over pure-function namespaces: - `Ccsds355_0_B_2::parse` (Parser) — Security Header (SPI, sequence number) and Trailer (MAC) extraction -- `Components::validatePacket` (Validator) — SPI validation and anti-replay sequence-number window validation -- `Components::authenticatePacket` / `importHmacKey` (Authenticator) — HMAC-SHA-256 (truncated to 16 bytes) verification via PSA crypto +- `Components::validatePacket` (Validator) — SPI validation against the active key store and anti-replay sequence-number window validation +- `Components::authenticatePacket` / `parseHexKey` / `importHmacKeyBytes` (Authenticator) — HMAC-SHA-256 (truncated to 16 bytes) verification via PSA crypto -The only component state is the last accepted sequence number (mutex-guarded, persisted to file) and the imported HMAC key id. +`Validator` takes the active SPI set as a plain `ActiveSpiSlots` array (`Types.hpp`) rather than the FPP-generated key store type directly, so it — and its unit tests — stay pure C++ with no F Prime dependency; `TcSecurityDeframer::activeSpiSlots()` projects the real key store into that shape before calling `validatePacket`. + +Component state is the last accepted sequence number and the on-flash key store (each mutex-guarded and persisted to file), plus the PSA key ids imported from the store's valid slots. + +### Key Storage + +The HMAC authentication key is **never compiled into the firmware image** (issue #220). It is persisted at `KEY_STORE_FILE_PATH` (default `/keys/authkeys.bin`) on a dedicated littlefs `keystore_partition` on internal flash, holding up to 2 slots (`{valid, spi, key}`). `configure()` loads the store and imports every valid slot into PSA; a missing/empty store is not an error — a keyless board still boots so it can be provisioned. The sequence-number file lives on the same partition (`SEQ_NUM_FILE_PATH`, default `/keys/sequence_number.bin`). + +The store is shared across all `TcSecurityDeframer` instances (UART/LoRa/Sband): on an unknown SPI, `dataIn_handler` reloads the store from disk once and retries validation before rejecting the frame, so a rotation issued over one link is picked up by the others without a separate command per link. That reload is gated on a process-wide generation counter bumped by every successful store write: SPI validation runs before MAC verification, so the path is reachable by unauthenticated frames, and without the gate a stream of bogus-SPI frames would drive a flash read plus a full PSA re-import per frame on the com thread while holding the lock shared by all three uplinks. + +Three commands manage the store (see [Commands](#commands)): `PROVISION_KEY` (bootstrap, only while the store is empty), `ADD_KEY` (rotation, fails at 2 active keys), and `REMOVE_KEY` (rotation, fails at 1 remaining key). All three re-read the store from flash first, then re-import it into PSA and update `ActiveKeyCount` telemetry on success. + +All three also refuse to act (`StoreUnreadable`) unless the store's contents are *known*, because otherwise they would be acting on a guess. + +"Known" cannot be decided from the read status. On the Zephyr target `ZephyrFile::open` discards `fs_open`'s errno and reports `OTHER_ERROR` for every failure, so `Os::File::DOESNT_EXIST` is unreachable on flight hardware even though it is the normal result for a missing file on the POSIX host. Gating on the status therefore made a factory-fresh (or `/keys`-erased) board refuse `PROVISION_KEY` forever: keyless *and* unprovisionable, i.e. total command loss. Instead `probeKeyStore()` interrogates the filesystem — `fs_stat` on the store file for a positive absence answer, plus `fs_statvfs` on the mount point to prove `/keys` is actually mounted — and feeds the pure predicates in `Components::KeyStore` (`Types.hpp`). Both signals are required: Zephyr returns `-ENOENT` for an unmounted mount point exactly as it does for a missing file, so absence alone proves nothing. For `ADD_KEY`/`REMOVE_KEY` that prevents a read-modify-write from persisting a stale guess over the real store. For `PROVISION_KEY` it is a security property: the command is bypass-allowlisted so it works on a keyless board, so trust-on-first-use must be gated on *proof* that the store is empty. Treating an unreadable store as keyless would let anyone in radio range induce a read failure and install their own key while a valid one still sits on flash. + +`REMOVE_KEY` zeroizes the revoked slot's key bytes before persisting, so a removed key is not recoverable from the on-flash record; the slot is restored if the durable write fails. Primary data path connections: @@ -33,11 +49,21 @@ class TcSecurityDeframer { -dataReturnIn_handler(portNum, data, context) -GET_SEQ_NUM_cmdHandler(opCode, cmdSeq) -SET_SEQ_NUM_cmdHandler(opCode, cmdSeq, seqNum) + -PROVISION_KEY_cmdHandler(opCode, cmdSeq, spi, key) + -ADD_KEY_cmdHandler(opCode, cmdSeq, spi, key) + -REMOVE_KEY_cmdHandler(opCode, cmdSeq, spi) -readSequenceNumber(value) -writeSequenceNumber(value) + -loadKeyStore() + -writeKeyStore() + -importKeyStore() + -findKeyIdForSpi(spi, keyId) + -activeKeyCount() + -activeSpiSlots() -m_sequenceNumber : U32 -m_sequenceNumberWindow : U32 - -m_hmacKeyId : uint32_t + -m_keyStore : AuthKeyStore + -m_keyIds : uint32_t[2] } class Ccsds355_0_B_2 { @@ -47,12 +73,14 @@ class Ccsds355_0_B_2 { class PacketValidator { <> - +validatePacket(secHeader, sequenceNumber, window) Status + +validatePacket(secHeader, sequenceNumber, window, activeSpis) Status } class PacketAuthenticator { <> - +importHmacKey(key, keyId) KeyImportResult + +parseHexKey(key, keyBytes) bool + +importHmacKeyBytes(keyBytes, keyId) KeyImportResult + +destroyHmacKey(keyId) int32_t +authenticatePacket(buffer, size, mac, keyId) AuthenticationResult } @@ -101,19 +129,20 @@ The MAC is HMAC-SHA-256 truncated to 16 bytes, computed over the Security Header ## Behavior 1. Parse the Security Header and Trailer. If the frame is too short to contain them it cannot be stripped for downstream deframing: log ParsingFailed and return the buffer upstream (drop). -2. Validate the SPI (only SPI 0 is currently supported) and the anti-replay sequence number (must be strictly ahead of the last accepted value, within SEQ_NUM_WINDOW, with U32 wraparound handled). -3. If validation passes, verify the MAC. +2. Validate the SPI (must match a valid slot in the active key store — see [Key Storage](#key-storage)) and the anti-replay sequence number (must be strictly ahead of the last accepted value, within SEQ_NUM_WINDOW, with U32 wraparound handled). On an unknown SPI, the key store is reloaded from disk once and validation retried — but only if another instance has written the store since this one last read it — so a rotation issued over another link is picked up here without letting unauthenticated frames drive repeated flash reads. +3. If validation passes, look up the PSA key id for the packet's SPI and verify the MAC with it. 4. Only when all checks pass: store and persist the received sequence number, telemeter it, and set `authenticated = true` in the frame context. Frames failing any check never advance the sequence number (issue #426). 5. Strip the Security Header and Trailer and forward on dataOut with the resulting `authenticated` flag. ProvesRouter rejects unauthenticated packets unless their opcode is on the bypass allowlist. -At startup, `configure()` loads the persisted sequence number and telemeters it so the first downlinked value is correct before any command is accepted (issue #427). +At startup, `configure()` loads the persisted sequence number and key store, telemetering both so the first downlinked values are correct before any command is accepted (issue #427). A missing or empty key store leaves the board keyless (bootable, awaiting `PROVISION_KEY`) rather than failing to boot. ## Parameters | Name | Type | Default | Description | |---|---|---|---| | SEQ_NUM_WINDOW | U32 | 50000 | Maximum allowed forward sequence-number distance before rejecting a packet as out-of-window. | -| SEQ_NUM_FILE_PATH | string | "//sequence_number.txt" | File path used to persist and restore the sequence number across restarts. | +| SEQ_NUM_FILE_PATH | string | "/keys/sequence_number.bin" | File path used to persist and restore the sequence number across restarts. | +| KEY_STORE_FILE_PATH | string | "/keys/authkeys.bin" | File path used to persist and restore the authentication key store across restarts. | ## Port Descriptions @@ -131,6 +160,7 @@ Standard AC ports are also present for command handling, events, telemetry, para | Name | Type | Description | |---|---|---| | CurrentSequenceNumber | U32 | Current accepted sequence number tracked by the component. Emitted at startup and on each accepted packet. | +| ActiveKeyCount | U8 | Number of valid slots (0-2) in the key store. Emitted at startup and after PROVISION_KEY/ADD_KEY/REMOVE_KEY. | Routed/bypassed/rejected packet counts are telemetered by ProvesRouter, which owns the accept/reject policy. @@ -146,6 +176,14 @@ Routed/bypassed/rejected packet counts are telemetered by ProvesRouter, which ow | AuthenticationFailed | Warning High (throttle 2) | auth_status: PacketAuthenticatorStatus, rc: I32 | Logged when MAC verification fails. Format: "Authentication failed: Status={}, PSA Return Code={}" | | ParsingFailed | Warning High (throttle 2) | parse_status: PacketParserStatus | Logged when frame parsing fails. Format: "Parsing failed: {}" | | SpiInvalid | Warning High (throttle 2) | packet_spi: U32 | Logged when SPI validation fails. Format: "SPI invalid: Received={}" | +| KeyStoreReadFailed | Warning High (throttle 2) | status: Os.FileStatus | Logged when the key store read fails (not thrown for a missing file — that's the keyless state). Format: "Failed to read key store, error: {}" | +| KeyStoreWriteFailed | Warning High (throttle 2) | status: Os.FileStatus | Logged when the key store write fails. Format: "Failed to write key store, error: {}" | +| KeyProvisioned | Activity High | spi: U16 | Logged by PROVISION_KEY on success. Format: "Key provisioned for SPI={}" | +| KeyProvisionFailed | Warning High (throttle 2) | status: KeyStoreProvisionStatus | Logged when PROVISION_KEY fails (store not empty, unreadable store, bad hex key, write failure, or PSA import failure). Format: "Key provisioning failed: {}" | +| KeyAdded | Activity High | spi: U16 | Logged by ADD_KEY on success. Format: "Key added for SPI={}" | +| KeyAddFailed | Warning High (throttle 2) | status: KeyStoreProvisionStatus | Logged when ADD_KEY fails (store full, duplicate SPI, unreadable store, bad hex key, write failure, or PSA import failure). Format: "Key add failed: {}" | +| KeyRemoved | Activity High | spi: U16 | Logged by REMOVE_KEY on success. Format: "Key removed for SPI={}" | +| KeyRemoveFailed | Warning High (throttle 2) | status: KeyStoreProvisionStatus | Logged when REMOVE_KEY fails (last remaining key, SPI not found, unreadable store, write failure, or PSA import failure). Format: "Key remove failed: {}" | ## Commands @@ -153,6 +191,9 @@ Routed/bypassed/rejected packet counts are telemetered by ProvesRouter, which ow |---|---|---|---| | GET_SEQ_NUM | Sync | None | Reads and reports the current sequence number (SequenceNumberGet event). | | SET_SEQ_NUM | Sync | seq_num: U32 | Sets and persists a new sequence number (SequenceNumberSet event). | +| PROVISION_KEY | Sync | spi: U16, key: string | Bootstrap: writes the first key slot and imports it into PSA. Only succeeds while the store is empty; bypass-allowlisted in ProvesRouter so it works on a keyless board (KeyProvisioned/KeyProvisionFailed events). | +| ADD_KEY | Sync | spi: U16, key: string | Rotation: adds a key to an empty slot. Requires an authenticated frame (not bypass-allowlisted); fails if 2 keys are already active (KeyAdded/KeyAddFailed events). | +| REMOVE_KEY | Sync | spi: U16 | Rotation: invalidates the slot matching `spi`. Requires an authenticated frame (not bypass-allowlisted); fails if it would drop the active key count below 1 (KeyRemoved/KeyRemoveFailed events). | ## Unit Tests @@ -161,8 +202,11 @@ TcSecurityDeframer helper functionality is covered by unit tests in PROVESFlight | Test File | Coverage | |---|---| | test_TcSecurityDeframer_Parser.cpp | Valid parse path plus parse failures for SPI, sequence number, and MAC size checks. | -| test_TcSecurityDeframer_Validator.cpp | SPI validation, out-of-window and replayed sequence numbers, window boundary, and wraparound handling. | -| test_TcSecurityDeframer_Authenticator.cpp | Key import failures, successful MAC verification, and failed verification with corrupted MAC or data. | +| test_TcSecurityDeframer_Validator.cpp | SPI validation against the active `ActiveSpiSlots` set (single slot, second slot, no valid slots), out-of-window and replayed sequence numbers, window boundary, and wraparound handling. | +| test_TcSecurityDeframer_KeyStorePolicy.cpp | The key store admission predicates (`Components::KeyStore::storeStateIsKnown` / `storeIsProvisionable`) over the exhaustive mount x store-probe x active-key-count matrix: a cold board on a live mount is provisionable; an unreadable store never is; an absent store on an unproven mount never is; and a regression test documenting why a gate written over `Os::File::Status` diverges between host and target. | +| test_TcSecurityDeframer_Authenticator.cpp | `parseHexKey` (valid upper/lowercase, null, wrong length, non-hex characters), key import via hex and raw bytes, `destroyHmacKey`, successful MAC verification, and failed verification with corrupted MAC, corrupted data, or a destroyed key. | + +These cover only the pure-function layer (Parser/Validator/Authenticator; no F Prime or Zephyr dependency). The key store mutation rules enforced in the command handlers (`PROVISION_KEY`/`ADD_KEY`/`REMOVE_KEY` — provision-only-when-empty, add fails at 2, remove fails at 1) are F-Prime-component-dependent and are not covered here; see the commented-out `register_fprime_ut` block in `CMakeLists.txt` for a future on-target/component test pass. Run unit tests with: @@ -180,16 +224,16 @@ make framer-plugin Then run GDS with the framing plugin enabled as configured by the project tooling. -## Generating Keys +## Provisioning Keys -The default authentication key header (AuthDefaultKey.h) is generated at build time from project key material via `make generate-auth-key` or `make copy-secrets`. This generated file is machine-local and not committed. +The authentication key is no longer compiled into the image, so there is nothing to generate at build time. Instead, a keyless board is provisioned after flashing by uplinking `PROVISION_KEY(spi, key)` over a bypass-allowlisted link (the flight-side integration test `PROVESFlightControllerReference/test/int/provision_key_test.py` does this in CI). Ground must be given the same key, via the `--authentication-key` CLI arg or `PROVES_AUTH_KEY` env var to the framing plugin (`Framing/src/authenticate_plugin.py`). `make copy-secrets` still copies production key material from a secure directory for use in provisioning. ## Requirements | Name | Description | Validation | |---|---|---| | AUTH001 | The component shall parse incoming frames to extract the SPI, sequence number, and MAC fields. | Unit Test | -| AUTH003 | The component shall validate that the SPI value corresponds to a configured Security Association. | Unit Test | +| AUTH003 | The component shall validate that the SPI value corresponds to a configured Security Association (an active slot in the on-flash key store). | Unit Test | | AUTH004 | The component shall validate the received sequence number against the stored sequence number. | Unit Test | | AUTH004-A | The component shall not authenticate packets with sequence numbers that are outside the acceptable window and shall log an event. | Unit Test, Inspection | | AUTH004-B | The component shall set the stored sequence number to the sequence number transmitted in the packet only when a packet is fully validated and authenticated. | Inspection | @@ -207,3 +251,4 @@ Opcode-based bypass policy (formerly AUTH002) is owned by ProvesRouter; see its | --- | --- | | 2025-11-26 | Initial design. | | 2026-07-17 | Renamed to TcSecurityDeframer, refactor to discrete responsibilities: Authenticator, Parser, Validator. Pass-through interface between TcDeframer and SpacePacketDeframer; verification result carried in frame context; policy enforcement moved to ProvesRouter. | +| 2026-07-23 | Moved the authentication key off the compiled-in image onto a littlefs key store on internal flash, alongside the sequence-number file (issue #220). Added PROVISION_KEY/ADD_KEY/REMOVE_KEY commands supporting up to 2 active keys; SPI validation now checks the active key store instead of a hard-coded SPI 0. | diff --git a/prj.conf b/prj.conf index 946a59e3..d5788b49 100644 --- a/prj.conf +++ b/prj.conf @@ -74,6 +74,7 @@ CONFIG_FS_FATFS_EXFAT=y CONFIG_FS_FATFS_MOUNT_MKFS=y CONFIG_FS_FATFS_FSTAB_AUTOMOUNT=y CONFIG_FILE_SYSTEM_MKFS=y +CONFIG_FILE_SYSTEM_LITTLEFS=y CONFIG_MCUBOOT_SIGNATURE_KEY_FILE="keys/proves.pem" CONFIG_HAPTICS=y diff --git a/pytest.ini b/pytest.ini index 07ca9309..97b1fc54 100644 --- a/pytest.ini +++ b/pytest.ini @@ -2,11 +2,13 @@ markers = uart_only: marks tests that sever the RF link (resets, TRANSMIT toggle) and should only be run when connected via UART sync_sequence_number: marks the test that synchronizes the sequence number between GDS and flight software; should be run before any other tests to avoid sequence number mismatches + provision_key: marks the test that provisions the HMAC authentication key onto a keyless board; should be run before any auth-required command is sent format_filesystem: marks the test that formats the filesystem; should be run before any other tests to ensure a clean state requires_face: marks tests that require a face board (TMP112 / VEML6031 / DRV2605 sensors) to be plugged in; skip on a bare flight controller requires_antenna: marks tests that require the antenna board to be plugged in and the burnwire capacitor installed; skip on a bare flight controller requires_battery: marks tests that require the battery board connected with power flowing from the battery terminals; skip on a bare flight controller requires_watchdog_jumper: marks tests that require the JP6 watchdog jumper to be bridged so the watchdog can reset the MCU; skip when JP6 is open + slow: marks tests that take tens of seconds because they wait on a reboot or a timeout expiring filterwarnings = ignore::DeprecationWarning:yamcs\..* ignore::DeprecationWarning:google\.protobuf\..* diff --git a/scripts/generate_auth_default_key.h b/scripts/generate_auth_default_key.h deleted file mode 100644 index d344f741..00000000 --- a/scripts/generate_auth_default_key.h +++ /dev/null @@ -1,11 +0,0 @@ -// This file is auto-generated at build time. -// DO NOT EDIT MANUALLY - it will be overwritten during build. - -#ifndef AUTH_DEFAULT_KEY_H -#define AUTH_DEFAULT_KEY_H - -// Default authentication key generated at build time -// This ensures the code works even if external key files are not available on the satellite -#define AUTH_DEFAULT_KEY "@AUTH_DEFAULT_KEY@" - -#endif // AUTH_DEFAULT_KEY_H diff --git a/scripts/generate_auth_key_header.py b/scripts/generate_auth_key_header.py deleted file mode 100755 index 17636423..00000000 --- a/scripts/generate_auth_key_header.py +++ /dev/null @@ -1,138 +0,0 @@ -#!/usr/bin/env python3 -""" -Generate AuthDefaultKey.h file with a random HMAC key. - -This script generates AuthDefaultKey.h directly without needing spi_dict.txt. -""" - -import argparse -import os -import secrets -import sys - - -def generate_random_key() -> str: - """Generate a random 32-character hex key (16 bytes).""" - return secrets.token_hex(16) - - -def generate_auth_key_header(key: str, output_path: str, template_path: str) -> None: - """ - Generate AuthDefaultKey.h file from template. - - Args: - key: The authentication key (hex string without 0x prefix) - output_path: Path to output header file - template_path: Path to template file - """ - # Ensure output directory exists - os.makedirs(os.path.dirname(output_path), exist_ok=True) - - # Read template - with open(template_path, "r") as f: - template = f.read() - - # Replace placeholder with actual key - content = template.replace("@AUTH_DEFAULT_KEY@", key) - - # Write output file - with open(output_path, "w") as f: - f.write(content) - - -def extract_key_from_header(header_path: str) -> str: - """ - Extract the key from an existing AuthDefaultKey.h file. - - Args: - header_path: Path to AuthDefaultKey.h file - - Returns: - The authentication key (hex string without 0x prefix) - """ - if not os.path.exists(header_path): - raise FileNotFoundError(f"AuthDefaultKey.h not found at {header_path}") - - with open(header_path, "r") as f: - for line in f: - if "AUTH_DEFAULT_KEY" in line and '"' in line: - # Extract key from line like: #define AUTH_DEFAULT_KEY "4916d208d40612daad6edbc7333c4c13" - start = line.find('"') + 1 - end = line.find('"', start) - if start > 0 and end > start: - return line[start:end] - - raise ValueError("No valid key found in AuthDefaultKey.h") - - -def main(): - """Main function to parse arguments and generate/extract AuthDefaultKey.h.""" - parser = argparse.ArgumentParser( - description="Generate AuthDefaultKey.h with a random HMAC key" - ) - parser.add_argument( - "--output", - type=str, - default="PROVESFlightControllerReference/Components/TcSecurityDeframer/AuthDefaultKey.h", - help="Output path for AuthDefaultKey.h", - ) - parser.add_argument( - "--template", - type=str, - default="scripts/generate_auth_default_key.h", - help="Path to template file", - ) - parser.add_argument( - "--key", - type=str, - default=None, - help="Use this specific key instead of generating a random one", - ) - parser.add_argument( - "--print-key", - action="store_true", - help="Print the key to stdout (for use in Makefile or debugging)", - ) - parser.add_argument( - "--extract", - action="store_true", - help="Extract key from existing header file instead of generating", - ) - - args = parser.parse_args() - - if args.extract: - # Extract from existing file - try: - key = extract_key_from_header(args.output) - if args.print_key: - print(key) - else: - print(f"Extracted key from {args.output}") - except (FileNotFoundError, ValueError) as e: - print(f"Error: {e}", file=sys.stderr) - sys.exit(1) - else: - # Generate new file - try: - if args.key: - key = args.key - # Remove 0x prefix if present - if key.startswith("0x") or key.startswith("0X"): - key = key[2:] - else: - key = generate_random_key() - - generate_auth_key_header(key, args.output, args.template) - - if args.print_key: - print(key) - else: - print(f"Generated {args.output}") - except (FileNotFoundError, ValueError) as e: - print(f"Error: {e}", file=sys.stderr) - sys.exit(1) - - -if __name__ == "__main__": - main() diff --git a/tools/yamcs/proves_adapter.py b/tools/yamcs/proves_adapter.py index 1fd1ba55..ffa6d6b4 100644 --- a/tools/yamcs/proves_adapter.py +++ b/tools/yamcs/proves_adapter.py @@ -44,7 +44,7 @@ def _crc16_ccitt(data: bytes) -> int: from authenticate_plugin import ( # noqa: E402 AuthenticateFramer, - get_default_auth_key_from_header, + get_auth_key_from_env, ) from fprime_gds.common.communication.ccsds.space_data_link import ( # noqa: E402 SpaceDataLinkFramerDeframer, @@ -404,7 +404,7 @@ def _parse_scid_list(val: str) -> list[int]: p.add_argument( "--auth-key", default=None, - help="HMAC key as hex string (no 0x prefix). Defaults to key from AuthDefaultKey.h.", + help="HMAC key as hex string (no 0x prefix). Defaults to the PROVES_AUTH_KEY env var.", ) # Frame size and CCSDS identifiers @@ -448,8 +448,8 @@ def main(): # Resolve auth key auth_key = args.auth_key if auth_key is None: - auth_key = get_default_auth_key_from_header() - print("[auth] Loaded key from AuthDefaultKey.h") + auth_key = get_auth_key_from_env() + print("[auth] Loaded key from PROVES_AUTH_KEY env var") auth_framer = AuthenticateFramer(authentication_key=auth_key) diff --git a/west.yml b/west.yml index 73f68164..6e2d67ac 100644 --- a/west.yml +++ b/west.yml @@ -29,7 +29,8 @@ manifest: - picolibc # C library - mbedtls # Crypto library - mcuboot # Bootloader support - - fatfs # FatFS (file system) support + - fatfs # FatFS (file system) support (SD card, mounted at /) + - littlefs # LittleFS - internal-flash key store, mounted at /keys. - hal_st # Required for certain sensors - name: loramac-node @@ -87,6 +88,10 @@ manifest: revision: f4ead3bf4a6dab3a07d7b5f5315795c073db568d path: lib/zephyr-workspace/modules/fatfs + - name: littlefs + revision: 8f5ca347843363882619d8f96c00d8dbd88a8e79 + path: lib/zephyr-workspace/modules/fs/littlefs + self: path: . west-commands: west-commands.yml