From 89849c47536a990663c3124a07ab884dd8ed94db Mon Sep 17 00:00:00 2001 From: MacNite Date: Mon, 14 Sep 2026 17:18:20 +0200 Subject: [PATCH] Fix sensor fault handling and secure OTA releases --- .github/workflows/release.yml | 31 +++++++++++++++--- docs/flashing.md | 17 ++++++---- docs/ota-over-ble.md | 39 ++++++++++++++++------- firmware-nrf54lm20a/src/accel.c | 5 ++- firmware-nrf54lm20a/src/audio.c | 14 ++++++-- firmware-nrf54lm20a/src/hive_config.h | 4 +-- firmware-nrf54lm20a/src/mic.c | 14 +++++--- firmware-nrf54lm20a/src/ota.c | 14 +++++++- firmware-nrf54lm20a/sysbuild/mcuboot.conf | 10 +++--- 9 files changed, 112 insertions(+), 36 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 93f9392..1794be8 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -77,6 +77,22 @@ jobs: app-path: hiveinside toolchains: arm-zephyr-eabi + - name: Require the deployment signing key + env: + SIGNING_KEY_PEM: ${{ secrets.HIVEINSIDE_SIGNING_KEY_PEM }} + run: | + set -eu + # The SDK fallback is a public key: allowing it here would make a + # release image deployable by anybody who possesses that known key. + test -n "$SIGNING_KEY_PEM" || { + echo "HIVEINSIDE_SIGNING_KEY_PEM must contain the deployment private key" >&2 + exit 1 + } + key_file="$RUNNER_TEMP/hiveinside-signing.pem" + umask 077 + printf '%s\n' "$SIGNING_KEY_PEM" > "$key_file" + echo "SIGNING_KEY_FILE=$key_file" >> "$GITHUB_ENV" + - name: Read the firmware version working-directory: hiveinside run: | @@ -99,7 +115,14 @@ jobs: -b xiao_nrf54lm20a/nrf54lm20a/cpuapp \ -d "${{ matrix.config }}" \ firmware-nrf54lm20a \ - ${EXTRA:+-- $EXTRA} + -- -DSB_CONFIG_BOOT_SIGNATURE_KEY_FILE="$SIGNING_KEY_FILE" $EXTRA + + # Refuse artifacts unless MCUboot actually embedded the provisioned + # key; merely writing the secret without selecting it would silently + # fall back to the SDK's publicly known development key. + grep -Fqx \ + "CONFIG_BOOT_SIGNATURE_KEY_FILE=\"$SIGNING_KEY_FILE\"" \ + "${{ matrix.config }}/mcuboot/zephyr/.config" - name: Collect the release images working-directory: hiveinside @@ -230,9 +253,9 @@ jobs: echo "zephyr revision : $(cat staging/openocd/ZEPHYR_REVISION)" echo "built by : ${GITHUB_SERVER_URL}/${GITHUB_REPOSITORY}/actions/runs/${GITHUB_RUN_ID}" echo - echo "Signing key: MCUboot's RSA-2048 *development* key, which is public." - echo "That provides image formatting, not production authenticity — any" - echo "image signed with the same public key is accepted by these builds." + echo "Signing key: project-owned deployment key supplied to release CI." + echo "The private key is not included; archive its matching public key and" + echo "the SWD recovery procedure described in docs/ota-over-ble.md." echo echo "OTA payloads (upload to HiveHub; size and CRC-32 are what the OTA" echo "BEGIN frame carries):" diff --git a/docs/flashing.md b/docs/flashing.md index a88cd8f..b097442 100644 --- a/docs/flashing.md +++ b/docs/flashing.md @@ -87,12 +87,17 @@ the OTA `BEGIN` frame carries), the commit and the Zephyr revision behind the build, and `west-manifest-frozen.yml` plus `build-info-.zip` are there to reproduce it. -Release images are signed with **MCUboot's development key, which is public** — -that is image formatting, not proof of origin. A device running these images -accepts any image signed with the same public key. For a deployment you control, -provision your own key (see the checklist in -[`ota-over-ble.md`](ota-over-ble.md#production-release-and-recovery-checklist)) -and build your own images. +The release workflow requires a project-owned MCUboot signing key and refuses to +produce these artifacts when its `HIVEINSIDE_SIGNING_KEY_PEM` secret is absent. +This is a required deployment step: the SDK's default development key is public, +so a node built with it accepts firmware from anyone using that known key. For a +local deployment build, generate and select the key with +`SB_CONFIG_BOOT_SIGNATURE_KEY_FILE` exactly as shown in the +[`ota-over-ble.md` production checklist](ota-over-ble.md#production-release-and-recovery-checklist). +Keep the private key outside the repository and preserve the matching public key +plus an SWD recovery path. Signing is currently the trust boundary because the +unencrypted OTA GATT writes have no HMAC; the filter-accept-list TODO remains in +`ota_init()` pending coordinated HiveHub bonding support. ### If the board does not come up diff --git a/docs/ota-over-ble.md b/docs/ota-over-ble.md index acc7375..58d6fa3 100644 --- a/docs/ota-over-ble.md +++ b/docs/ota-over-ble.md @@ -194,8 +194,12 @@ reset (up to a full `MEASURE_INTERVAL_MS` away). The deadline is armed only when MCUboot reports a real test swap (`BOOT_SWAP_TYPE_REVERT`); a directly SWD-flashed image has no rollback target, is never armed, and cannot boot loop. -Only the three OTA characteristics are exposed. A filter accept list would be a -stronger connection guardrail, but requires a future HiveHub bonding change. +Only the three OTA characteristics are exposed. They require neither link +encryption nor the audio service's HMAC, and the node advertises connectably at +all times. Image-signature trust is therefore the deployment security boundary: +any nearby central can upload bytes, but only a project-signed image may boot. +The filter-accept-list TODO in `ota_init()` is intentionally still in place; it +requires a future, coordinated HiveHub bonding change. ## Build @@ -340,11 +344,24 @@ over SWD — a device that cannot complete an OTA cannot receive the fix for it. The SDK's default MCUboot development key is suitable only for bring-up. It is publicly known and therefore provides image formatting, **not production -authenticity**. Before deploying devices, provision a project-owned signing key, -configure both MCUboot and sysbuild signing to use it, keep the private key out of -the repository and build logs, and archive the matching public key and recovery -procedure. Changing the key later requires an SWD recovery image whose MCUboot -contains the new public key. +authenticity**. Using a project-owned key is a required deployment step, not an +optional hardening measure. Generate it with MCUboot's `imgtool keygen`, store +the private PEM outside the repository, and select it at the sysbuild level so +MCUboot and application signing cannot diverge: + +```bash +python bootloader/mcuboot/scripts/imgtool.py keygen \ + -k /secure/hiveinside-signing.pem -t rsa-2048 +west build --pristine=always --sysbuild \ + -b xiao_nrf54lm20a/nrf54lm20a/cpuapp -d build firmware-nrf54lm20a -- \ + -DSB_CONFIG_BOOT_SIGNATURE_KEY_FILE=/secure/hiveinside-signing.pem +``` + +Keep the key out of build logs, archive its matching public key and the recovery +procedure, and provision the `HIVEINSIDE_SIGNING_KEY_PEM` GitHub Actions secret +before running `.github/workflows/release.yml`; that workflow now refuses to +produce artifacts without it and verifies MCUboot selected it. Changing the key +later requires an SWD recovery image whose MCUboot contains the new public key. For every release: @@ -365,13 +382,13 @@ For every release: whose bootloader, signing key, partition layout, or radio/application startup is broken. -Steps 1 and 3 are mechanised: `.github/workflows/release.yml` builds both +Steps 1 and 3 are mechanised: `.github/workflows/release.yml` requires the +project-owned signing-key secret, builds both variants pristine on a tag, publishes the version-stamped payload together with a `manifest.txt` carrying each payload's byte size and CRC-32, and attaches the frozen west manifest and both images' Kconfig so the build can be reproduced. -Steps 2, 4 and 5 still need a person and a board. Note that those published -images use the **development** signing key — a deployment with its own key has -to build and publish its own artifacts. +Steps 2, 4 and 5 still need a person and a board. The workflow fails before the +build rather than publishing an artifact made with the development key. ## Implementation diff --git a/firmware-nrf54lm20a/src/accel.c b/firmware-nrf54lm20a/src/accel.c index 15ef4a9..2ea50c1 100644 --- a/firmware-nrf54lm20a/src/accel.c +++ b/firmware-nrf54lm20a/src/accel.c @@ -164,7 +164,10 @@ static bool sample_xyz(uint32_t period_us, float *x_mg, float *y_mg, break; } if (k_uptime_ticks() > deadline) { - break; + /* Reading after data-ready timed out returned the previous sample, + * making a stuck IMU look like valid zero vibration. Fail this + * sample so accel_read() rejects a short capture instead. */ + return false; } k_usleep(200); } diff --git a/firmware-nrf54lm20a/src/audio.c b/firmware-nrf54lm20a/src/audio.c index fda058d..12ac834 100644 --- a/firmware-nrf54lm20a/src/audio.c +++ b/firmware-nrf54lm20a/src/audio.c @@ -42,6 +42,7 @@ enum { }; static uint8_t state, error, nonce[8], format; +static bool nonce_usable; static uint32_t bytes_sent, dropped_bytes, running_crc, clipped, samples; static uint16_t seq, duration_ds; static int8_t gain_db; @@ -90,9 +91,14 @@ BT_GATT_SERVICE_DEFINE(audio_service, static void rotate_nonce(void) { - if (bt_rand(nonce, sizeof(nonce)) != 0) { - /* A predictable challenge must never make a microphone available. */ + nonce_usable = false; + if (bt_rand(nonce, sizeof(nonce)) == 0) { + nonce_usable = true; + } else { + /* Substituting a constant after RNG failure made an observed START + * HMAC replayable. Keep the advertised value inert and fail closed. */ memset(nonce, 0, sizeof(nonce)); + printk("[AUDIO] nonce generation failed; START disabled\n"); } } @@ -132,6 +138,10 @@ static bool key_usable(void) static bool authenticate(const uint8_t *request) { + if (!nonce_usable) { + printk("[AUDIO] START refused: no unpredictable nonce\n"); + return false; + } if (!key_usable()) { printk("[AUDIO] START refused: audio_secret.h is absent or all zero\n"); return false; diff --git a/firmware-nrf54lm20a/src/hive_config.h b/firmware-nrf54lm20a/src/hive_config.h index 8ea621e..bc1567e 100644 --- a/firmware-nrf54lm20a/src/hive_config.h +++ b/firmware-nrf54lm20a/src/hive_config.h @@ -43,7 +43,7 @@ */ #define HIVEINSIDE_FW_VERSION_MAJOR 0 #define HIVEINSIDE_FW_VERSION_MINOR 6 -#define HIVEINSIDE_FW_VERSION_PATCH 2 +#define HIVEINSIDE_FW_VERSION_PATCH 3 #define HIVEINSIDE_STRINGIFY_(value) #value #define HIVEINSIDE_STRINGIFY(value) HIVEINSIDE_STRINGIFY_(value) @@ -89,7 +89,7 @@ #define HIVEINSIDE_COMPANY_ID 0x02E5 #endif -/* A one-second non-connectable advertising interval is short enough to be +/* A one-second connectable advertising interval is short enough to be * heard reliably in HiveHub's scan window while allowing the controller to * sleep between three brief advertising-channel transmissions. BLE intervals * use 0.625 ms units. */ diff --git a/firmware-nrf54lm20a/src/mic.c b/firmware-nrf54lm20a/src/mic.c index 6ce8292..b7bd415 100644 --- a/firmware-nrf54lm20a/src/mic.c +++ b/firmware-nrf54lm20a/src/mic.c @@ -215,11 +215,17 @@ void mic_read(struct measurement *m) ? 20.0f * log10f(peak_dev / FULL_SCALE) : SILENCE_DBFS; m->mic_frames = count; - m->mic_ok = true; - - if (fft_count >= 64) { - compute_bands(fft_count, m); + if (fft_count < 64) { + /* The MIC flag covers RMS, peak and all five bands. Setting it for a + * short capture left the uncomputed zero-initialized bands looking + * like valid full-scale audio, so reject the whole measurement. */ + printk("[MIC] only %u FFT samples; skipping\n", + (unsigned)fft_count); + return; } + + compute_bands(fft_count, m); + m->mic_ok = true; } #else /* !ENABLE_MIC or no nordic,nrf-pdm node enabled in the devicetree */ diff --git a/firmware-nrf54lm20a/src/ota.c b/firmware-nrf54lm20a/src/ota.c index ad64c28..289e93e 100644 --- a/firmware-nrf54lm20a/src/ota.c +++ b/firmware-nrf54lm20a/src/ota.c @@ -119,7 +119,7 @@ static ssize_t ctrl_write(struct bt_conn *conn, const struct bt_gatt_attr *attr, const uint8_t *p = buf; int rc; - ARG_UNUSED(conn); ARG_UNUSED(attr); ARG_UNUSED(flags); + ARG_UNUSED(attr); ARG_UNUSED(flags); if (offset || !len) { return BT_GATT_ERR(BT_ATT_ERR_INVALID_OFFSET); } @@ -152,6 +152,18 @@ static ssize_t ctrl_write(struct bt_conn *conn, const struct bt_gatt_attr *attr, if (!link_claim(LINK_OWNER_OTA)) { fail(OTA_ERR_BEGIN); return len; } + /* Leaving OTA at the generic 30--50 ms link interval made every + * write-with-response wait needlessly: its ATT response is the flash + * flow-control boundary. Match audio's faster link setup after claim. */ + struct bt_le_conn_param cp = { + .interval_min = HIVE_AUDIO_CONN_INTERVAL_UNITS, + .interval_max = HIVE_AUDIO_CONN_INTERVAL_UNITS, + .latency = 0, + .timeout = OTA_SUPERVISION_TIMEOUT, + }; + (void)bt_conn_le_param_update(conn, &cp); + (void)bt_conn_le_data_len_update(conn, BT_LE_DATA_LEN_PARAM_MAX); + (void)bt_conn_le_phy_update(conn, BT_CONN_LE_PHY_PARAM_2M); received = 0; /* crc32_ieee_update uses zlib's pre/post complement internally. */ running_crc = 0; diff --git a/firmware-nrf54lm20a/sysbuild/mcuboot.conf b/firmware-nrf54lm20a/sysbuild/mcuboot.conf index e28fd3b..ff7f48e 100644 --- a/firmware-nrf54lm20a/sysbuild/mcuboot.conf +++ b/firmware-nrf54lm20a/sysbuild/mcuboot.conf @@ -47,8 +47,8 @@ CONFIG_UART_CONSOLE=n CONFIG_REGULATOR=n CONFIG_MFD=n -# Image signing uses the SDK's default development key, and sysbuild signs the -# application with the same key so test-swap works out of the box. For a real -# deployment, generate a private key and point MCUboot at it with -# CONFIG_BOOT_SIGNATURE_KEY_FILE (and keep the public key with the release), so -# the device only accepts images you signed. +# Local builds use the SDK's public default development key unless sysbuild's +# SB_CONFIG_BOOT_SIGNATURE_KEY_FILE selects a project-owned key for both images. +# That default makes bring-up convenient but is not deployable security; the +# release workflow refuses to build without its provisioned production key. +# See docs/ota-over-ble.md and docs/flashing.md for the required key procedure.