Skip to content

fix(firmware): use base MAC for onboarding device digest - #1984

Open
Aditya-9-6 wants to merge 2 commits into
ruvnet:mainfrom
Aditya-9-6:fix-c6-onboarding-device-digest
Open

Aditya-9-6 wants to merge 2 commits into
ruvnet:mainfrom
Aditya-9-6:fix-c6-onboarding-device-digest

Conversation

@Aditya-9-6

Copy link
Copy Markdown

Fixes #1942.

Root cause

In \serial_onboarding.c:
\\c
const esp_err_t mac_result = esp_efuse_mac_get_default(base_mac);
\
On chips with IEEE 802.15.4 support (\SOC_IEEE802154_SUPPORTED=1, e.g. ESP32-C6 and ESP32-H2), \esp_efuse_mac_get_default\ does not return a 6-byte MAC-48. Instead, it calls \insert_mac_ext_into_mac(mac)\ which:

  1. Inserts the 2-byte \ESP_EFUSE_MAC_EXT\ at \mac[3..4].
  2. Shifts the original \mac[3..5]\ bytes out to \mac[5..7], producing an 8-byte EUI-64.

Passing \uint8_t base_mac[6]\ caused:

  1. A 2-byte stack write overflow.
  2. \sizeof(base_mac)\ (6 bytes) copied to \digest_input\ contained only [OUI0, OUI1, OUI2, EXT0, EXT1, MAC3].
  3. Bytes 4 and 5 of the factory MAC (the unique board-level increments within a manufacturing lot) were truncated into indices 6 and 7 and omitted from the SHA-256 computation.

All boards in the same batch therefore hashed the exact same prefix, emitting colliding \device_digest\ values (such as \

esp_efuse_mac_get_default expands to an 8-byte EUI-64 on chips with
SOC_IEEE802154_SUPPORTED (such as ESP32-C6), inserting the 2-byte
MAC_EXT at bytes 3..4 and pushing the lower MAC bytes out to bytes 5..7.
Because base_mac was declared as uint8_t[6], this overflowed the buffer
by 2 bytes and caused device_digest to only hash bytes 0..5 (OUI, EXT,
and byte 3). Boards from the same batch therefore shared the identical
prefix, resulting in identical device digests.

Use esp_base_mac_addr_get to retrieve the standard 6-byte base MAC
address across all targets without IEEE 802.15.4 extension, ensuring all
bytes of the MAC are hashed and the digest is unique per board.

Fixes ruvnet#1942.

@ruvnet ruvnet left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Evidence-gate review at exact head 1bb586308b92e903a6c1b9a63e6e8581adc34605: REJECT.

The one-line API correction is source-level sound across ESP-IDF 5.4 and 6.0.2, and issue #1942 confirms reachable ESP32-C6 provisioning impact. The delivery and qualification boundary is incomplete:

  • no test exercises device_digest;
  • all six exact-head workflows required authorization and ran zero jobs;
  • no S3/C6 target builds, sanitizer checks, distinct-device vectors, migration/retry test, or real-hardware replay ran; and
  • the tracked affected 0.8.12 C6 binary and checksum are unchanged, so the repository-provided firmware does not contain this repair.

Please add production-code host seams/fixtures, S3/C6/H2 width and digest vectors, failure/fallback and onboarding-retry coverage, authorize the exact-head gates, rebuild both supported targets, and publish a replacement C6 artifact/checksum with multi-board replay before merge.

Add a self-contained host test that verifies the device_digest()
function in serial_onboarding.c without requiring the ESP-IDF SDK.

Changes:
- test/test_device_digest.c: test harness with two MAC vectors + fallback
- test/device_digest_shim.c/h: exposes device_digest() as non-static
  symbol, decoupled from the heavy serial_onboarding.c dependencies
- test/stubs/esp_mac.h: stub header for esp_base_mac_addr_get()
- test/stubs/esp_stubs.c: add MAC stub implementations (esp_stub_set_mac,
  esp_stub_set_base_mac_result, esp_base_mac_addr_get); guard wasm/mmwave
  stubs with #ifndef TEST_DEVICE_DIGEST to allow slim compilation
- test/stubs/esp_stubs.h: fix include ordering so esp_err_t typedef
  precedes esp_mac.h which depends on it
- test/stubs/psa/crypto.h: pure-C SHA-256 + PSA Crypto stub (no external
  library dependency)

Build and run:
  gcc -std=c99 -DTEST_DEVICE_DIGEST -Itest -Itest/stubs ^
      -o test_device_digest ^
      test/test_device_digest.c test/device_digest_shim.c ^
      test/stubs/esp_stubs.c -lm
  test_device_digest.exe
  # PASS: device_digest() - all vectors verified

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

device_digest is not unique across boards, weakening the USB provisioning anti-swap guard

2 participants