diff --git a/CHANGELOG.md b/CHANGELOG.md index 2d55d9c..1e4036f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,8 @@ ## [Unreleased] ### Security +- **UART recovery issues no challenge on a board without an entropy source.** When `eos_hal_rng_get()` failed, `recovery_handle_auth()` filled the 32-byte challenge from a linear congruential generator seeded with `eos_hal_get_tick_ms()`. The challenge is the only thing that stops a captured `(challenge, response)` pair from being replayed, and with that fallback it was a function of the millisecond at which the AUTH command was handled -- a few thousand values on a freshly reset board -- which the client sees before it has to answer and can retry after a reset at no cost in failure count. The AUTH is now refused outright (NACK, event `EOS_LOG_AUTH_NO_ENTROPY` = `0x23`, which `tools/uart_recovery.py` decodes; counted like any other failure so the backoff applies). No board port provides `rng_get` (0 of the 83 ops tables under `boards/`), so this was the challenge on every board; none provides `otp_read` either, so no board authenticates anyone today and the defect was latent. Pinned by `test_auth_refuses_when_the_board_has_no_entropy_source` in `tests/unit/test_recovery.c`. +- **UART recovery refuses an unprovisioned secret, and a write into an unmapped slot.** `recovery_handle_auth()` compared the client's response against `SHA-256(challenge || secret)` for whatever the OTP held at the secret offset -- and unprogrammed fuses read back as all zeros or all ones, both public, so a device whose recovery secret was never provisioned authenticated any client that knew that. Both patterns are now refused before the comparison (logged as event `0x22`), the way the keystore already refuses an all-zero key. `recovery_handle_write()` had stopped calling `eos_recovery_write_in_range()` -- the rule the unit tests exercise -- for an inline check that never looked at the slot base, so a slot the board leaves unmapped (base 0) turned "write at offset" into "write at flash address". The handler uses the shared rule again -- which also refuses a `WRITE` with `len == 0`, where the inline check had ACKed it and written nothing; a recovery client sees a NACK there now. No shipped board port provides `otp_read` today, so the first defect was latent (authentication always failed); both are pinned by `tests/unit/test_recovery.c`. The authentication outcomes are logged as `EOS_LOG_AUTH_SUCCESS` / `EOS_LOG_AUTH_FAIL` / `EOS_LOG_AUTH_UNPROVISIONED` (`0x20`-`0x22`, now defined in `include/eos_types.h` beside the other `EOS_LOG_*` codes instead of as bare hex), `tools/uart_recovery.py` decodes all three, and `tests/unit/test_boot_log_event_names.py` keeps the firmware, the header and the client's table in step. - **The production-key gate covers every release-shaped build, and the key it accepts is checked against the curve.** The gate matched the literal string `Release` and nothing else, so `MinSizeRel`, `RelWithDebInfo`, `release`, `RELEASE` and an unset build type all configured a real board on the development anchor. It now exempts only `Debug`. Separately, `EBLDR_PRODUCTION_KEY` was checked for length and hex-ness but never for being a point on edwards25519 — the defect the development key itself had — so one mistyped hex digit in the release secret would have shipped a fleet that refuses every image, with a green build. `tools/check_production_key.py` applies the verifier's own rule (`[L]P == identity`, `P != identity`, not the development key) in pure Python; `cmake/ProductionKey.cmake` runs it at configure when python3 is present and warns in so many words when it is not, and `release.yml` runs it on the secret before any board is configured. - **A release can no longer be built on the development trust anchor.** `EBLDR_PRODUCTION_KEY` had no CMake option, nothing set it, and its `#else` branch declared an `extern` nothing defined -- so every firmware job in `release.yml` compiled the `#ifndef` branch and shipped the RFC 8032 test key as the anchor for every board without OTP, which is every board under `boards/`. The only guard was a `#warning`. Now: `-DEBLDR_PRODUCTION_KEY=<64 hex characters>` generates `ebldr_production_key[]` (`cmake/ProductionKey.cmake`, declared in `include/eos_production_key.h`) and selects the production branch of `core/keystore.c`; a Release build of a real board refuses to configure without it unless it says `-DEBLDR_ALLOW_DEV_KEY=ON`; the development key and malformed values are refused as production keys; `release.yml` passes the `EBLDR_PRODUCTION_KEY_HEX` secret on every board configure (empty until a maintainer provides it, so a release fails closed) and scans every built `.elf`/`.bin`/`.a`/`.o` for the development key's bytes. `tests/unit/test_keystore_production.c` compiles the production branch with a fixture key on every host build; `tests/unit/test_production_key_gate.py` and `test_release_workflow_production_key.py` pin the gate and the workflow. - **The compiled-in development trust anchor was not a point on the curve.** `core/keystore.c` `default_dev_key` was described as the RFC 8032 section 7.1 TEST 1 public key, but it diverged from that key at byte 21 and the bytes it held did not decode to a point on the curve, so no signature could ever verify against it. It is now the RFC key, and `tests/unit/test_keystore.c` checks that the anchor verifies the RFC's own signature. Behaviour change: a board with no OTP and no `EBLDR_PRODUCTION_KEY` goes from refusing every image (since #104 made signature verification unconditional at install) to accepting images signed with the public RFC test key -- the documented development intent of the `#warning` in `core/keystore.c`, and a key that must never reach a device. #120 tracks making that structural. diff --git a/core/recovery.c b/core/recovery.c index 1e5c816..fafdc95 100644 --- a/core/recovery.c +++ b/core/recovery.c @@ -25,6 +25,11 @@ /* Recovery protocol commands */ #define RCVR_CMD_PING 0x01 #define RCVR_CMD_INFO 0x02 +/* Bits of the caps byte in the INFO response. */ +#define RCVR_CAP_RNG 0x01 /* board has an entropy source: AUTH can issue a challenge */ +#define RCVR_CAP_OTP 0x02 /* board has OTP: a shared secret can be provisioned */ +/* INFO response: ack(1) + five little-endian uint32 (20) + caps(1). */ +#define RCVR_INFO_LEN 22 #define RCVR_CMD_ERASE 0x03 #define RCVR_CMD_WRITE 0x04 #define RCVR_CMD_VERIFY 0x05 @@ -137,15 +142,19 @@ static int recovery_handle_auth(void) } if (auth_state == RCVR_AUTH_NONE) { - /* Generate challenge using RNG */ + /* The challenge is the only thing that stops a captured response + * from being replayed, so it has to come from an entropy source. + * A board without one gets no challenge at all: a fallback seeded + * from the millisecond tick lets a client reset the board and + * retry, at no cost, until a challenge it already holds an answer + * for comes back. Refusing here is fail-closed in the same way as + * an unreadable or unprovisioned secret below. */ int rc = eos_hal_rng_get(challenge, RCVR_CHALLENGE_SIZE); if (rc != EOS_OK) { - /* Fallback: use tick-based pseudo-random */ - uint32_t seed = eos_hal_get_tick_ms(); - for (int i = 0; i < RCVR_CHALLENGE_SIZE; i++) { - seed = seed * 1103515245 + 12345; - challenge[i] = (uint8_t)(seed >> 16); - } + auth_fail_count++; + auth_state = RCVR_AUTH_NONE; + eos_boot_log_append(EOS_LOG_AUTH_NO_ENTROPY, EOS_SLOT_NONE, auth_fail_count); + return recovery_send_nack(); } /* Send challenge to client */ @@ -182,7 +191,24 @@ static int recovery_handle_auth(void) /* Fail authentication if OTP secret is unreadable */ auth_fail_count++; auth_state = RCVR_AUTH_NONE; - eos_boot_log_append(0x21, EOS_SLOT_NONE, auth_fail_count); /* AUTH_FAIL */ + eos_boot_log_append(EOS_LOG_AUTH_FAIL, EOS_SLOT_NONE, auth_fail_count); + return recovery_send_nack(); + } + + /* Unprogrammed fuses read back as all zeros or all ones, and both + * are public. A board whose recovery secret was never provisioned + * must not authenticate anyone: the keystore already refuses an + * all-zero key for the same reason. Branch-free so the check does + * not leak which value the fuses hold. */ + uint8_t all_zero = 0, all_ones = 0xFF; + for (size_t i = 0; i < sizeof(shared_secret); i++) { + all_zero |= shared_secret[i]; + all_ones &= shared_secret[i]; + } + if (all_zero == 0 || all_ones == 0xFF) { + auth_fail_count++; + auth_state = RCVR_AUTH_NONE; + eos_boot_log_append(EOS_LOG_AUTH_UNPROVISIONED, EOS_SLOT_NONE, auth_fail_count); return recovery_send_nack(); } @@ -199,12 +225,12 @@ static int recovery_handle_auth(void) EOS_SHA256_DIGEST_SIZE) == 0) { auth_state = RCVR_AUTH_AUTHENTICATED; auth_fail_count = 0; - eos_boot_log_append(0x20, EOS_SLOT_NONE, 0); /* AUTH_SUCCESS */ + eos_boot_log_append(EOS_LOG_AUTH_SUCCESS, EOS_SLOT_NONE, 0); return recovery_send_ack(); } else { auth_fail_count++; auth_state = RCVR_AUTH_NONE; - eos_boot_log_append(0x21, EOS_SLOT_NONE, auth_fail_count); /* AUTH_FAIL */ + eos_boot_log_append(EOS_LOG_AUTH_FAIL, EOS_SLOT_NONE, auth_fail_count); return recovery_send_nack(); } } @@ -218,29 +244,58 @@ static int recovery_handle_ping(void) return eos_hal_uart_send(response, sizeof(response)); } +/* Little-endian store: the recovery protocol's byte order, independent of + * the target's. Returns the next offset. */ +static size_t rcvr_put_le32(uint8_t *buf, size_t at, uint32_t v) +{ + buf[at + 0] = (uint8_t)(v & 0xFF); + buf[at + 1] = (uint8_t)((v >> 8) & 0xFF); + buf[at + 2] = (uint8_t)((v >> 16) & 0xFF); + buf[at + 3] = (uint8_t)((v >> 24) & 0xFF); + return at + 4; +} + static int recovery_handle_info(void) { const eos_board_ops_t *ops = eos_hal_get_ops(); if (!ops) return recovery_send_nack(); - struct { - uint8_t ack; - uint32_t flash_size; - uint32_t slot_a_addr; - uint32_t slot_a_size; - uint32_t slot_b_addr; - uint32_t slot_b_size; - } info; - - info.ack = RCVR_ACK; - info.flash_size = ops->flash_size; - info.slot_a_addr = ops->slot_a_addr; - info.slot_a_size = ops->slot_a_size; - info.slot_b_addr = ops->slot_b_addr; - info.slot_b_size = ops->slot_b_size; - - return eos_hal_uart_send(&info, sizeof(info)); + /* The INFO response is a wire format, so it is built as bytes, not as a + * struct. Before this it was an unpacked struct -- 24 bytes, three of + * padding after ack that nothing wrote -- and sizeof(info) sent all 24: + * an unauthenticated caller (INFO needs no auth) received three bytes of + * whatever the previous call had left on the stack, and the repo's own + * client, which has always parsed the packed layout, printed them as the + * flash size. A byte buffer cannot have padding on any toolchain, needs + * no packing attribute or pragma to stay 22 bytes, and puts the five + * words on the wire little-endian regardless of the target's byte order + * -- which is what tools/uart_recovery.py has always assumed ('flash_size); + i = rcvr_put_le32(info, i, ops->slot_a_addr); + i = rcvr_put_le32(info, i, ops->slot_a_size); + i = rcvr_put_le32(info, i, ops->slot_b_addr); + i = rcvr_put_le32(info, i, ops->slot_b_size); + info[i++] = (uint8_t)((ops->rng_get ? RCVR_CAP_RNG : 0) | + (ops->otp_read ? RCVR_CAP_OTP : 0)); + /* Every byte accounted for: the length is the contract, not a sizeof. */ + if (i != RCVR_INFO_LEN) + return recovery_send_nack(); + + return eos_hal_uart_send(info, sizeof(info)); } static int recovery_handle_erase(eos_slot_t slot) @@ -286,8 +341,11 @@ static int recovery_handle_write(eos_slot_t slot, uint32_t offset, uint16_t len) /* offset/len come straight from the wire; without this check a * recovery client can write past the slot boundary into the other - * slot, boot-control blocks, or the boot log. */ - if (slot_size == 0 || (uint64_t)offset + len > (uint64_t)slot_size) + * slot, boot-control blocks, or the boot log. The rule lives in + * eos_recovery_write_in_range() -- the function the unit tests drive -- + * and it also refuses a slot the board leaves unmapped (base 0), which + * would otherwise turn "write at offset" into "write at address". */ + if (eos_recovery_write_in_range(base, slot_size, offset, len) != EOS_OK) return recovery_send_nack(); recovery_send_ack(); diff --git a/docs/adr/ADR-021-recovery-info-stays-unauthenticated.md b/docs/adr/ADR-021-recovery-info-stays-unauthenticated.md new file mode 100644 index 0000000..983aa8b --- /dev/null +++ b/docs/adr/ADR-021-recovery-info-stays-unauthenticated.md @@ -0,0 +1,70 @@ +--- +adr: 21 +title: Recovery INFO stays unauthenticated and carries a capability byte; LOG does not +status: Proposed +date: 2026-09-15 +deciders: Architecture Council, eBoot maintainers +source: docs/threat_model.md T-404; the automated review of #131 (finding 2) and #139 (finding 4); .ai/security.md on documented weaker postures +--- + +# ADR-021 — Recovery INFO stays unauthenticated and carries a capability byte; LOG does not + +## Context + +`docs/threat_model.md` T-404 records the recovery `INFO` and `LOG` commands +as an information-disclosure surface, with the planned mitigation "Restrict +INFO/LOG to authenticated sessions (Phase 3)". + +#131 makes authenticated recovery unreachable *by construction* on a board +with no entropy source: `AUTH` refuses to issue a challenge it cannot make +random. The event it logs, `EOS_LOG_AUTH_NO_ENTROPY`, is readable only over +`LOG`, which requires the authentication the missing entropy prevents. What an +integrator on such a board observed was a bare `NACK` — indistinguishable from +a wrong secret, an unreadable OTP, or an unprovisioned one — after +`0+1+2+4+8 = 15 s` of backoff across five attempts. + +#139 answers that by adding one byte, `caps`, to the `INFO` response: +`RCVR_CAP_RNG` and `RCVR_CAP_OTP`, derived from the board's ops table. The +question "why does AUTH always NACK on this board?" is then answered before +the first `AUTH` is sent. That design depends on `INFO` being reachable +*before* authentication, which is exactly what T-404's planned mitigation +would remove. The two cannot both stand. + +## Decision + +1. **`INFO` remains unauthenticated, deliberately.** Its response is flash + geometry (five `uint32`) and the capability byte. Every one of those + values is something an attacker with UART access learns anyway — geometry + from the board's public documentation or from the first `ERASE` that + NACKs out of range, capabilities by attempting `AUTH` once. Disclosing + them costs nothing the attacker did not already have and buys the + integrator a diagnosis that otherwise costs 15 s and a wrong conclusion. + +2. **`LOG` remains authenticated.** Boot history — which slot booted, why the + previous boot failed, recovery entry and exit reasons — is the material + T-404 is right to protect, and nothing in #131 or #139 needs it before + authentication. + +3. **The `INFO` response is a wire contract:** 22 bytes, packed — `ack`, + five little-endian `uint32`, `caps` — defined in `core/recovery.c` with a + `_Static_assert` on its size, documented in `docs/architecture.md`, and + parsed by `tools/uart_recovery.py`, which refuses the 24-byte unpacked + response of pre-fix firmware rather than decoding it. Adding a field means + changing all three and the assert. + +## Consequences + +- T-404's mitigation narrows from "INFO/LOG" to "LOG", and its disclosure + column records `caps`. The `INFO` half is closed by this decision rather + than by Phase 3. +- A future field in `INFO` must pass the same test as `caps`: would an + attacker learn it by trying anyway? If not, it belongs behind `AUTH`, or in + `LOG`. +- The three bytes of uninitialised stack that `INFO` used to send (#140) are + a separate defect — a leak of *unintended* data — and are fixed regardless + of this decision. + +## Note on numbering + +ADR-020 is the highest number in use; this is 021. It stands alone rather +than extending ADR-020, which concerns the install path. diff --git a/docs/adr/README.md b/docs/adr/README.md index bc72656..d889841 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -14,6 +14,7 @@ superseded by a later record that names it. | ADR | Title | Status | |---|---|---| | 020 | [Install path verifies the signature before anti-rollback](ADR-020-install-path-verifies-signature-before-anti-rollback.md) | Proposed | +| 021 | [Recovery INFO stays unauthenticated and carries a capability byte; LOG does not](ADR-021-recovery-info-stays-unauthenticated.md) | Proposed | ## Note on numbering diff --git a/docs/architecture.md b/docs/architecture.md index fcd7efe..e67396f 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -96,7 +96,7 @@ Reset_Handler() | Command | Code | Description | |-----------|------|---------------------------------| | PING | 0x01 | Identity check — returns "EOS" | -| INFO | 0x02 | Flash layout and slot addresses | +| INFO | 0x02 | Flash layout, slot addresses and capabilities. Unauthenticated (ADR-021). Response: 22 bytes packed -- `ack`, `flash_size`, `slot_a_addr`, `slot_a_size`, `slot_b_addr`, `slot_b_size` (LE `uint32`), `caps` (bit 0 RNG, bit 1 OTP) | | ERASE | 0x03 | Erase target slot | | WRITE | 0x04 | Write chunk to slot | | VERIFY | 0x05 | Verify slot image integrity | diff --git a/docs/threat_model.md b/docs/threat_model.md index ad4a0bb..0010ac6 100644 --- a/docs/threat_model.md +++ b/docs/threat_model.md @@ -139,7 +139,7 @@ External boundaries: | T-401 | Spoofing | Unauthorized party connects to UART and replaces firmware | Medium | Critical | High | Recovery authentication — challenge-response or shared secret (Phase 3) | Planned | | T-402 | Tampering | Malformed UART packets corrupt flash or boot state | Medium | High | High | Packet validation; length bounds checking; CRC on write data | Partial | | T-403 | Denial of Service | Flood UART with garbage to prevent legitimate recovery | Low | Medium | Low | Command timeout; watchdog feed during recovery; rate limiting | Partial | -| T-404 | Information Disclosure | INFO and LOG commands leak device configuration and boot history | Low | Medium | Low | Restrict INFO/LOG to authenticated sessions (Phase 3) | Planned | +| T-404 | Information Disclosure | INFO discloses flash geometry and, since #139, a capability byte (`caps`: has RNG, has OTP); LOG discloses boot history | Low | Medium | Low | LOG restricted to authenticated sessions (Phase 3). INFO is deliberately unauthenticated per ADR-021: everything it discloses an attacker learns by trying, and the integrator needs it before AUTH to diagnose a board with no entropy source | Partial (INFO decided; LOG planned) | | T-405 | Repudiation | Recovery operations not logged | Medium | Medium | Medium | Boot log records recovery entry, commands executed, exit reason | Planned | | T-406 | Elevation of Privilege | FACTORY command erases all data without authorization | Medium | Critical | High | Require authentication before FACTORY; confirmation sequence | Planned | diff --git a/include/eos_types.h b/include/eos_types.h index 1807d7f..21a9354 100644 --- a/include/eos_types.h +++ b/include/eos_types.h @@ -143,6 +143,13 @@ typedef struct { #define EOS_LOG_FACTORY_RESET 0x0A #define EOS_LOG_WATCHDOG_RESET 0x0B #define EOS_LOG_BOOT_FAIL 0x0C +/* Recovery-session authentication. Emitted by core/recovery.c; decoded by + * tools/uart_recovery.py, whose table tests/unit/test_boot_log_event_names.py + * keeps in step with this list. */ +#define EOS_LOG_AUTH_SUCCESS 0x20 +#define EOS_LOG_AUTH_FAIL 0x21 +#define EOS_LOG_AUTH_UNPROVISIONED 0x22 +#define EOS_LOG_AUTH_NO_ENTROPY 0x23 /* Maximum constants */ #define EOS_MAX_BOOT_ATTEMPTS 3 diff --git a/tests/unit/test_boot_log_event_names.py b/tests/unit/test_boot_log_event_names.py new file mode 100644 index 0000000..5b488fa --- /dev/null +++ b/tests/unit/test_boot_log_event_names.py @@ -0,0 +1,143 @@ +# SPDX-License-Identifier: MIT +# Copyright (c) 2026 EoS Project + +"""Every boot-log event the firmware emits has a name the recovery client prints. + +core/recovery.c logged its authentication outcomes as bare hex -- 0x20, 0x21 +and then 0x22 -- with a trailing comment for a name, while include/eos_types.h +defined EOS_LOG_* for every other event and tools/uart_recovery.py mapped +codes to names for the `log` command. The table stopped at 0x21, so the one +event that says "this device was never provisioned" printed as UNKNOWN(0x22) +on the client that exists to read it. Nothing tied the three lists together. + +Three rules, all source-level, all cheap: + +1. Every event passed to eos_boot_log_append() in core/, stage0/ and stage1/ + is an EOS_LOG_* name, never a literal. +2. Every EOS_LOG_* code defined in include/eos_types.h is in the client's + BOOT_LOG_EVENT_NAMES, under the same name minus the prefix. +3. The client names no code the header does not define. +""" + +import ast +import re +from pathlib import Path + +REPO_ROOT = Path(__file__).resolve().parents[2] +TYPES_H = REPO_ROOT / "include" / "eos_types.h" +CLIENT = REPO_ROOT / "tools" / "uart_recovery.py" +FIRMWARE_DIRS = ("core", "stage0", "stage1") + + +def _strip_c_comments(text): + text = re.sub(r"/\*.*?\*/", " ", text, flags=re.DOTALL) + return re.sub(r"//[^\n]*", " ", text) + + +EVENT_DEFINE = re.compile(r"^\s*#define\s+EOS_LOG_([A-Z0-9_]+)\s+(0x[0-9A-Fa-f]+)\b", re.M) + + +def header_events(text=None): + """{name: code} for every #define EOS_LOG_ in eos_types.h. + + The match stops at the value rather than at end of line: this header + writes trailing comments on its defines (EOS_LOG_MAGIC has one), and a + parser that anchors on end of line silently drops any code written that + way -- under-collection, which no downstream rule can notice. + """ + if text is None: + text = TYPES_H.read_text(encoding="utf-8") + found = EVENT_DEFINE.findall(text) + events = {name: int(code, 16) for name, code in found if name != "MAGIC"} + assert events, "no EOS_LOG_* event codes found in include/eos_types.h" + return events + + +def client_events(): + """{code: name} from BOOT_LOG_EVENT_NAMES, read from the source without importing + the client (it imports pyserial at module level).""" + tree = ast.parse(CLIENT.read_text(encoding="utf-8")) + for node in ast.walk(tree): + if isinstance(node, ast.Assign) and any( + isinstance(t, ast.Name) and t.id == "BOOT_LOG_EVENT_NAMES" for t in node.targets + ): + table = ast.literal_eval(node.value) + assert table, "BOOT_LOG_EVENT_NAMES is empty" + return table + raise AssertionError("BOOT_LOG_EVENT_NAMES not found in tools/uart_recovery.py") + + +def firmware_append_sites(): + """[(file:line, event expression)] for every eos_boot_log_append() call.""" + sites = [] + for d in FIRMWARE_DIRS: + for path in sorted((REPO_ROOT / d).glob("*.c")): + text = _strip_c_comments(path.read_text(encoding="utf-8")) + for m in re.finditer(r"\beos_boot_log_append\s*\(\s*([^,)]+?)\s*[,)]", text): + expr = m.group(1).strip() + if expr.startswith("uint32_t"): + continue # the definition / extern declaration, not a call + line = text.count("\n", 0, m.start()) + 1 + sites.append((f"{path.relative_to(REPO_ROOT)}:{line}", expr)) + assert sites, "no eos_boot_log_append() call sites found" + return sites + + +def test_firmware_logs_events_by_name_only(): + events = header_events() + literal = [(where, expr) for where, expr in firmware_append_sites() + if not re.fullmatch(r"EOS_LOG_[A-Z0-9_]+", expr)] + assert not literal, "eos_boot_log_append() called with a bare event value: %s" % literal + unknown = [(where, expr) for where, expr in firmware_append_sites() + if expr[len("EOS_LOG_"):] not in events] + assert not unknown, "event names with no #define in include/eos_types.h: %s" % unknown + + +def test_client_names_every_event_the_header_defines(): + events = header_events() + table = client_events() + missing = sorted("EOS_LOG_%s = 0x%02X" % (name, code) + for name, code in events.items() if code not in table) + assert not missing, "tools/uart_recovery.py BOOT_LOG_EVENT_NAMES lacks: %s" % missing + renamed = sorted("0x%02X: header %s, client %s" % (code, name, table[code]) + for name, code in events.items() if table.get(code) not in (None, name)) + assert not renamed, "client and header disagree on a name: %s" % renamed + + +def test_client_names_no_event_the_header_does_not_define(): + codes = set(header_events().values()) + extra = sorted("0x%02X: %s" % (code, name) for code, name in client_events().items() + if code not in codes) + assert not extra, "tools/uart_recovery.py names codes the firmware never emits: %s" % extra + + +def test_the_authentication_events_are_named_and_decodable(): + events = header_events() + assert events["AUTH_SUCCESS"] == 0x20 + assert events["AUTH_FAIL"] == 0x21 + assert events["AUTH_UNPROVISIONED"] == 0x22 + assert client_events()[0x22] == "AUTH_UNPROVISIONED" + + +def test_header_parser_sees_a_define_with_a_trailing_comment(): + """The failure mode of the parser is collecting fewer rows than the file + has, which nothing downstream can detect; pin the one style this header + already uses that an end-of-line anchor would miss.""" + sample = ( + "#define EOS_LOG_BOOT_START 0x01\n" + "#define EOS_LOG_AUTH_NO_ENTROPY 0x23 /* board provides no rng_get */\n" + "#define EOS_LOG_WITH_CPP_COMMENT 0x24 // trailing\n" + "#define EOS_LOG_MAGIC 0x454C4F47 /* \"ELOG\" */\n" + ) + events = header_events(sample) + assert events == {"BOOT_START": 0x01, "AUTH_NO_ENTROPY": 0x23, "WITH_CPP_COMMENT": 0x24} + + +def test_header_parser_collects_every_event_define_in_the_real_header(): + """Cross-check the parser against a looser count of the same lines, so a + future style the strict pattern misses shows up as a mismatch.""" + text = TYPES_H.read_text(encoding="utf-8") + loose = [m.group(1) for m in re.finditer(r"^\s*#define\s+EOS_LOG_([A-Z0-9_]+)\b", text, re.M) + if m.group(1) != "MAGIC"] + assert sorted(header_events()) == sorted(loose) + diff --git a/tests/unit/test_recovery.c b/tests/unit/test_recovery.c index e99133d..cf86f75 100644 --- a/tests/unit/test_recovery.c +++ b/tests/unit/test_recovery.c @@ -111,11 +111,22 @@ static const uint8_t SIM_CHALLENGE[32] = { 0x18, 0x19, 0x1A, 0x1B, 0x1C, 0x1D, 0x1E, 0x1F, }; +/* What the simulated OTP holds at the recovery-secret offset. Tests point it + * at an unprogrammed pattern to show that such a device authenticates nobody. */ +static const uint8_t *sim_secret = SIM_SHARED_SECRET; +static const uint8_t SIM_SECRET_ALL_ZERO[32] = { 0 }; +static const uint8_t SIM_SECRET_ALL_ONES[32] = { + 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, + 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, + 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, + 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, +}; + static int sim_otp_read(uint32_t offset, void *buf, size_t len) { if (offset != 0x180 || len > sizeof(SIM_SHARED_SECRET)) return EOS_ERR_GENERIC; - memcpy(buf, SIM_SHARED_SECRET, len); + memcpy(buf, sim_secret, len); return EOS_OK; } @@ -236,9 +247,125 @@ static void setup(void) script_pos = 0; out_len = 0; verify_payload_bytes_read = 0; + sim_secret = SIM_SHARED_SECRET; eos_hal_init(&sim_ops); } +/* Script the two-packet authentication exchange, computing the response the + * way a client that knows `secret` would. */ +static void script_auth(const uint8_t *secret) +{ + eos_sha256_ctx_t ctx; + uint8_t auth_response[32]; + eos_sha256_init(&ctx); + eos_sha256_update(&ctx, SIM_CHALLENGE, sizeof(SIM_CHALLENGE)); + eos_sha256_update(&ctx, secret, 32); + eos_sha256_final(&ctx, auth_response); + + uint8_t pkt[8]; + put_pkt(pkt, RCVR_CMD_AUTH, 0, 0, 0); + script_append(pkt, sizeof(pkt)); /* -> server sends challenge */ + put_pkt(pkt, RCVR_CMD_AUTH, 0, 0, 0); + script_append(pkt, sizeof(pkt)); /* -> server awaits response */ + script_append(auth_response, sizeof(auth_response)); +} + +/* A device whose recovery secret was never programmed reads its fuses back + * as all zeros or all ones. Both are public, so a client that knows nothing + * can compute the "right" response. The server must refuse it -- and, having + * refused, must still refuse the write that follows. */ +static void check_unprovisioned_secret_is_refused(const uint8_t *pattern) +{ + setup(); + sim_secret = pattern; + + script_auth(pattern); + + uint8_t pkt[8]; + put_pkt(pkt, RCVR_CMD_WRITE, EOS_SLOT_A, 4, 0); + script_append(pkt, sizeof(pkt)); + uint8_t payload[4] = { 0xDE, 0xAD, 0xBE, 0xEF }; + script_append(payload, sizeof(payload)); + + eos_bootctl_t bctl; + eos_bootctl_init_defaults(&bctl); + if (setjmp(exit_jmp) == 0) { + eos_recovery_enter(&bctl); + } + + /* [0]=ACK, [1..32]=challenge, [33]=auth verdict, [34]=write verdict */ + ASSERT(out_len >= 35); + ASSERT(out_buf[0] == RCVR_ACK); + ASSERT(out_buf[33] == RCVR_NACK); + ASSERT(out_buf[34] == RCVR_NACK); + ASSERT(sim_flash[SIM_SLOT_A_ADDR] == 0xFF); +} + +TEST(test_auth_refuses_an_unprovisioned_secret) +{ + check_unprovisioned_secret_is_refused(SIM_SECRET_ALL_ZERO); + check_unprovisioned_secret_is_refused(SIM_SECRET_ALL_ONES); +} + +/* The control for the test above: the same exchange with a provisioned + * secret authenticates and the write goes through, so the refusal is about + * the secret's value and not about the exchange. */ +TEST(test_auth_accepts_a_provisioned_secret) +{ + script_auth(SIM_SHARED_SECRET); + + uint8_t pkt[8]; + put_pkt(pkt, RCVR_CMD_WRITE, EOS_SLOT_A, 4, 0); + script_append(pkt, sizeof(pkt)); + uint8_t payload[4] = { 0xDE, 0xAD, 0xBE, 0xEF }; + script_append(payload, sizeof(payload)); + + eos_bootctl_t bctl; + eos_bootctl_init_defaults(&bctl); + if (setjmp(exit_jmp) == 0) { + eos_recovery_enter(&bctl); + } + + ASSERT(out_len >= 36); + ASSERT(out_buf[33] == RCVR_ACK); + ASSERT(out_buf[34] == RCVR_ACK); + ASSERT(out_buf[35] == RCVR_ACK); + ASSERT(memcmp(&sim_flash[SIM_SLOT_A_ADDR], payload, sizeof(payload)) == 0); +} + +/* A board that leaves slot B unmapped reports base 0 for it. The write + * handler used to check only offset + len against the slot size, so a + * write "at offset 0x800 of slot B" landed at flash address 0x800 -- in + * this layout, between the boot-control block and its backup. The shared + * range rule refuses an unmapped slot; the handler has to use it. */ +TEST(test_write_refuses_a_slot_the_board_leaves_unmapped) +{ + eos_board_ops_t unmapped_b = sim_ops; + unmapped_b.slot_b_addr = 0; + unmapped_b.slot_b_size = SIM_SLOT_B_SIZE; + eos_hal_init(&unmapped_b); + + script_auth(SIM_SHARED_SECRET); + + uint8_t pkt[8]; + put_pkt(pkt, RCVR_CMD_WRITE, EOS_SLOT_B, 4, 0x800); + script_append(pkt, sizeof(pkt)); + uint8_t payload[4] = { 0xDE, 0xAD, 0xBE, 0xEF }; + script_append(payload, sizeof(payload)); /* only consumed if accepted */ + + eos_bootctl_t bctl; + eos_bootctl_init_defaults(&bctl); + if (setjmp(exit_jmp) == 0) { + eos_recovery_enter(&bctl); + } + + ASSERT(out_len >= 35); + ASSERT(out_buf[33] == RCVR_ACK); /* authenticated */ + ASSERT(out_buf[34] == RCVR_NACK); /* write refused */ + ASSERT(sim_flash[0x800] == 0xFF); + ASSERT(sim_flash[0x801] == 0xFF); +} + TEST(test_write_range_helper_rejects_invalid_bounds) { ASSERT(eos_recovery_write_in_range(SIM_SLOT_A_ADDR, SIM_SLOT_A_SIZE, @@ -322,6 +449,52 @@ TEST(test_write_rejects_offset_past_slot_end) ASSERT(memcmp(&sim_flash[SIM_SLOT_A_ADDR], payload, sizeof(payload)) == 0); } +/* Without an entropy source there is no challenge worth sending. The old + * fallback derived all 32 bytes from the millisecond tick at the moment the + * AUTH command was handled, so a client that had captured one (challenge, + * response) pair could reset the board and retry -- seeing each challenge + * before having to answer it, at no cost in failure count -- until the + * captured one came back. No shipped board port provides rng_get, so this is + * every board in the tree. The server must refuse the AUTH outright, log why, + * and keep refusing the write that follows. */ +TEST(test_auth_refuses_when_the_board_has_no_entropy_source) +{ + eos_board_ops_t no_rng = sim_ops; + no_rng.rng_get = NULL; + eos_hal_init(&no_rng); + eos_boot_log_init(0); + + uint8_t pkt[8]; + put_pkt(pkt, RCVR_CMD_AUTH, 0, 0, 0); + script_append(pkt, sizeof(pkt)); /* -> must be refused, not answered */ + put_pkt(pkt, RCVR_CMD_WRITE, EOS_SLOT_A, 4, 0); + script_append(pkt, sizeof(pkt)); + uint8_t payload[4] = { 0xDE, 0xAD, 0xBE, 0xEF }; + script_append(payload, sizeof(payload)); /* only consumed if accepted */ + + eos_bootctl_t bctl; + eos_bootctl_init_defaults(&bctl); + if (setjmp(exit_jmp) == 0) { + eos_recovery_enter(&bctl); + } + + /* [0]=auth verdict, [1]=write verdict: no 32-byte challenge in between. */ + ASSERT(out_len >= 2); + ASSERT(out_buf[0] == RCVR_NACK); + ASSERT(out_buf[1] == RCVR_NACK); + ASSERT(sim_flash[SIM_SLOT_A_ADDR] == 0xFF); + + /* The refusal is recorded as EOS_LOG_AUTH_NO_ENTROPY, not as a client + * failure, so a field log tells the integrator what is missing. */ + bool logged = false; + for (uint32_t i = 0; i < eos_boot_log_get_head(); i++) { + eos_boot_log_entry_t entry; + ASSERT(eos_boot_log_read(i, &entry) == EOS_OK); + if (entry.event == EOS_LOG_AUTH_NO_ENTROPY) logged = true; + } + ASSERT(logged); +} + static void fill_header(eos_image_header_t *hdr, uint32_t image_size) { memset(hdr, 0, sizeof(*hdr)); @@ -372,12 +545,82 @@ TEST(test_verify_rejects_oversized_image_before_reading_payload) ASSERT(verify_payload_bytes_read == 0); } +#define RCVR_CMD_INFO 0x02 +#define RCVR_CAP_RNG 0x01 +#define RCVR_CAP_OTP 0x02 +#define INFO_WIRE_LEN 22 /* ack(1) + five uint32 + caps(1), packed */ + +static uint32_t le32(const uint8_t *p) +{ + return (uint32_t)p[0] | ((uint32_t)p[1] << 8) | ((uint32_t)p[2] << 16) | ((uint32_t)p[3] << 24); +} + +/* Drive RCVR_CMD_INFO -- which needs no authentication -- and decode the + * response exactly as tools/uart_recovery.py does. Before the fix the struct was + * unpacked (24 bytes, three of padding after ack that nothing wrote) and + * sizeof(info) sent all of it, so an unauthenticated caller received three + * bytes of whatever the previous call left on the stack, and the client + * decoded them as the flash size. out_buf is poisoned first so any byte the + * firmware does not write shows as 0xA5 rather than as a lucky zero. */ +TEST(test_info_sends_the_packed_layout_the_client_parses_and_nothing_else) +{ + memset(out_buf, 0xA5, sizeof(out_buf)); + uint8_t pkt[8]; + put_pkt(pkt, RCVR_CMD_INFO, 0, 0, 0); + script_append(pkt, sizeof(pkt)); + + eos_bootctl_t bctl; + eos_bootctl_init_defaults(&bctl); + if (setjmp(exit_jmp) == 0) { + eos_recovery_enter(&bctl); + } + + ASSERT(out_len == INFO_WIRE_LEN); /* not 24: no padding on the wire */ + ASSERT(out_buf[0] == RCVR_ACK); + ASSERT(le32(&out_buf[1]) == SIM_FLASH_SIZE); /* was 0x00A5A5A5 -- the leak */ + ASSERT(le32(&out_buf[5]) == SIM_SLOT_A_ADDR); + ASSERT(le32(&out_buf[9]) == SIM_SLOT_A_SIZE); + ASSERT(le32(&out_buf[13]) == SIM_SLOT_B_ADDR); + ASSERT(le32(&out_buf[17]) == SIM_SLOT_B_SIZE); + ASSERT(out_buf[21] == (RCVR_CAP_RNG | RCVR_CAP_OTP)); /* sim_ops has both */ + ASSERT(out_buf[22] == 0xA5); /* the byte after: untouched */ +} + +/* The capability byte is the answer to "why does AUTH always NACK?" on a board + * with no entropy source, delivered before the 15 s of backoff it used to cost + * to find out. It is derived from the ops table, not configured. */ +TEST(test_info_reports_a_board_without_an_entropy_source) +{ + eos_board_ops_t no_rng = sim_ops; + no_rng.rng_get = NULL; + eos_hal_init(&no_rng); + + uint8_t pkt[8]; + put_pkt(pkt, RCVR_CMD_INFO, 0, 0, 0); + script_append(pkt, sizeof(pkt)); + eos_bootctl_t bctl; + eos_bootctl_init_defaults(&bctl); + if (setjmp(exit_jmp) == 0) { + eos_recovery_enter(&bctl); + } + + ASSERT(out_len == INFO_WIRE_LEN); + ASSERT((out_buf[21] & RCVR_CAP_RNG) == 0); + ASSERT((out_buf[21] & RCVR_CAP_OTP) != 0); /* OTP is still there */ +} + int main(void) { printf("=== test_recovery ===\n"); run_test_write_range_helper_rejects_invalid_bounds(); run_test_write_rejects_offset_past_slot_end(); + run_test_auth_refuses_an_unprovisioned_secret(); + run_test_auth_accepts_a_provisioned_secret(); + run_test_write_refuses_a_slot_the_board_leaves_unmapped(); + run_test_auth_refuses_when_the_board_has_no_entropy_source(); run_test_verify_rejects_oversized_image_before_reading_payload(); + run_test_info_sends_the_packed_layout_the_client_parses_and_nothing_else(); + run_test_info_reports_a_board_without_an_entropy_source(); printf("%d/%d tests passed\n", tests_passed, tests_run); return (tests_passed == tests_run) ? 0 : 1; } diff --git a/tests/unit/test_uart_recovery.py b/tests/unit/test_uart_recovery.py index 17d4017..a57f237 100644 --- a/tests/unit/test_uart_recovery.py +++ b/tests/unit/test_uart_recovery.py @@ -15,6 +15,7 @@ def __init__(self, incoming=b""): self.incoming = bytearray(incoming) self.writes = [] self.closed = False + self.timeout = 5.0 # settable, like serial.Serial.timeout def read(self, size): chunk = bytes(self.incoming[:size]) @@ -100,3 +101,82 @@ def test_log_rejects_incomplete_payload(monkeypatch, capsys): assert client.read_boot_log(0, 2) is False assert "Incomplete log response" in capsys.readouterr().out + + +def test_info_decodes_the_packed_22_byte_response(monkeypatch, capsys): + """The firmware sends ack, five little-endian uint32 and a caps byte, packed: + 22 bytes, and nothing after. The client reads those, probes for surplus + bytes (which would mean pre-fix firmware -- see the 24-byte test), finds + none, and decodes the geometry from the same offsets test_recovery.c + asserts the firmware writes them at.""" + payload = (bytes([0xAA]) + + struct.pack(" bool: print(f"Device responded: {ident} v{version}") return True + # ack(1) + five uint32 (20) + caps(1): the packed layout core/recovery.c sends. + INFO_LEN = 22 + + # What firmware before the packed-struct fix sent: ack, three bytes of + # padding, then the five fields -- 24 bytes, never the 21 the client read. + LEGACY_INFO_LEN = 24 + LEGACY_PROBE_TIMEOUT = 0.25 # seconds; generous for two bytes on any baud + def info(self): self._send_packet(CMD_INFO) - response = self.ser.read(1 + 4 * 5) - if len(response) < 21 or response[0] != ACK: + response = self.ser.read(self.INFO_LEN) + if len(response) < self.INFO_LEN or response[0] != ACK: print("Failed to get device info") return False + # A device running pre-fix firmware sends 24 bytes. read(22) of those + # succeeds, and decoding it prints the leaked padding as the flash size + # and invents a capability byte from a slot-size byte -- the exact + # defect this client is meant to have stopped showing. So: probe for + # more, with a short timeout of its own -- two bytes at 115200 baud + # take well under a millisecond, and the port's 5 s TIMEOUT is for + # flash operations, not this. A short read rather than in_waiting, + # because on a real UART those two bytes may still be on the wire + # when read(22) returns and in_waiting would report 0. + saved_timeout = self.ser.timeout + self.ser.timeout = self.LEGACY_PROBE_TIMEOUT + try: + extra = self.ser.read(self.LEGACY_INFO_LEN - self.INFO_LEN) + finally: + self.ser.timeout = saved_timeout + if extra: + print("Device is running pre-fix firmware: INFO responded with " + f"{self.INFO_LEN + len(extra)} bytes (unpacked layout). " + "Its geometry cannot be decoded reliably and its first three " + "bytes after ACK are uninitialised stack. Update the " + "bootloader before using this tool.") + # drain whatever else is queued so the next command starts clean + while self.ser.read(64): + pass + return False flash_size, slot_a_addr, slot_a_size, slot_b_addr, slot_b_size = \ struct.unpack(' bool: