Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 0 additions & 75 deletions .github/dependabot.yml

This file was deleted.

6 changes: 4 additions & 2 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -36,15 +36,17 @@ jobs:
-DCMAKE_C_COMPILER=gcc-12 \
-DCMAKE_CXX_COMPILER=g++-12 \
-DENABLE_COVERAGE=ON \
-DBUILD_TESTS=ON
-DEBLDR_BUILD_TESTS=ON

- name: Build (host)
run: cmake --build build/host --parallel $(nproc)

- name: Run C unit tests
run: |
cd build/host
ctest --output-on-failure --parallel $(nproc)
# --no-tests=error: ctest exits 0 when it finds no tests at all, so
# without this a misnamed -D flag makes this job pass having run none.
ctest --output-on-failure --no-tests=error --parallel $(nproc)

- name: Run Python tests
run: |
Expand Down
132 changes: 132 additions & 0 deletions .github/workflows/claude-code-review.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,132 @@
# Claude code review.
#
# This complements — it does not replace — the automatic Copilot review that
# runs on every PR via the "Code Quality Copilot review for default branch"
# ruleset. Copilot does the fast first pass; Claude does the deeper one, and
# only when asked, so routine PRs cost nothing.
#
# Two ways in:
# 1. Mention @claude in an issue, a PR comment, or a review comment.
# 2. Add the `deep-review` label to a PR for a full review pass.
#
# Requires an ANTHROPIC_API_KEY secret (org-level is easiest — one secret
# covers every repo). Without it both jobs fail fast with a clear message
# rather than reviewing silently with no credentials.
name: Claude Code Review

on:
issue_comment:
types: [created]
pull_request_review_comment:
types: [created]
pull_request_review:
types: [submitted]
issues:
types: [opened, assigned, labeled]
pull_request:
types: [labeled]

# Never run two reviews on the same PR at once; a new trigger supersedes the
# one in flight.
concurrency:
group: claude-review-${{ github.event.pull_request.number || github.event.issue.number || github.ref }}
cancel-in-progress: true

jobs:
# ── 1. On-demand: someone wrote @claude ──────────────────────────────────
mention:
if: |
github.event_name != 'pull_request' && (
(github.event_name == 'issue_comment' && contains(github.event.comment.body, '@claude')) ||
(github.event_name == 'pull_request_review_comment' && contains(github.event.comment.body, '@claude')) ||
(github.event_name == 'pull_request_review' && contains(github.event.review.body, '@claude')) ||
(github.event_name == 'issues' && (contains(github.event.issue.body, '@claude') || contains(github.event.issue.title, '@claude')))
)
runs-on: ubuntu-latest
timeout-minutes: 20
permissions:
contents: read
pull-requests: write
issues: write
id-token: write
steps:
- name: Check credentials
env:
KEY: ${{ secrets.ANTHROPIC_API_KEY }}
run: |
if [ -z "$KEY" ]; then
echo "::error::ANTHROPIC_API_KEY is not set. Add it as an organisation or repository secret."
exit 1
fi

- uses: actions/checkout@v6
with:
fetch-depth: 1

- uses: anthropics/claude-code-action@v1
with:
anthropic_api_key: ${{ secrets.ANTHROPIC_API_KEY }}
use_sticky_comment: true

# ── 2. Full pass: PR labelled `deep-review` ──────────────────────────────
deep-review:
if: github.event_name == 'pull_request' && github.event.label.name == 'deep-review'
runs-on: ubuntu-latest
timeout-minutes: 30
permissions:
contents: read
pull-requests: write
id-token: write
steps:
- name: Check credentials
env:
KEY: ${{ secrets.ANTHROPIC_API_KEY }}
run: |
if [ -z "$KEY" ]; then
echo "::error::ANTHROPIC_API_KEY is not set. Add it as an organisation or repository secret."
exit 1
fi

- uses: actions/checkout@v6
with:
fetch-depth: 1

- uses: anthropics/claude-code-action@v1
with:
anthropic_api_key: ${{ secrets.ANTHROPIC_API_KEY }}
use_sticky_comment: true
prompt: |
REPO: ${{ github.repository }}
PR NUMBER: ${{ github.event.pull_request.number }}

Review this pull request. The PR branch is already checked out in
the working directory.

This repository follows the standards in its own CLAUDE.md,
QUALITY.md, TESTING.md, SECURITY.md and VERIFY.md — read the ones
that apply to the changed files and review against them, not
against generic style preferences.

Prioritise, in this order:
1. Correctness — logic errors, unhandled failure paths, integer
overflow/underflow, memory safety, race conditions.
2. Security — unvalidated input crossing a boundary, injection,
authorization that checks only authentication, secrets in
source or logs.
3. Tests — does a new behaviour have a test that would actually
fail without the fix? Flag assertions that cannot fail, and
tests that only check the negative case.
4. Maintainability — only where it genuinely impedes a reader.

Report what you verified and what you could not. Say plainly when
a concern is unverified rather than implying you checked it. If
nothing needs changing, say so in one line — do not manufacture
findings.

Use `gh pr comment` for top-level feedback.
Use `mcp__github_inline_comment__create_inline_comment` (with
`confirmed: true`) for specific lines.
Only post GitHub comments — do not return review text as a message.

claude_args: |
--allowedTools "mcp__github_inline_comment__create_inline_comment,Bash(gh pr comment:*),Bash(gh pr diff:*),Bash(gh pr view:*),Bash(gh pr checks:*)"
9 changes: 6 additions & 3 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,9 @@ stm32f4 | stm32h7 | nrf52 | rpi4 | riscv64_virt | esp32 | x86_64_efi | \
imx8m | am64x | samd51 | sifive_u | cortex_r5 | none")

option(EBLDR_BUILD_TESTS "Build unit tests (native only)" OFF)
option(EBLDR_REQUIRE_SIGNATURES "Require Ed25519 signatures for boot" ON)
option(EBLDR_RECOVERY_AUTH "Require authentication for recovery commands" ON)
option(EBLDR_VERIFY_STAGE1 "Verify Stage-1 hash before jump" ON)
option(EBLDR_REQUIRE_SIGNATURES "Require Ed25519 signatures for boot" ON)
option(EBLDR_RECOVERY_AUTH "Require authentication for recovery commands" ON)
option(EBLDR_VERIFY_STAGE1 "Verify Stage-1 hash before jump" ON)
option(EBLDR_HARDENING "Enable compiler hardening flags" ON)
option(EBLDR_SANITIZE "Enable ASAN/UBSAN for host builds" OFF)
option(EBLDR_BUILD_FUZZ "Build libFuzzer fuzz targets" OFF)
Expand Down Expand Up @@ -104,7 +104,10 @@ add_library(eboot_core STATIC
core/ecc_scrub.c
core/bmc_handoff.c
core/os_adapter.c
core/sha512.c
core/ed25519_verify.c
core/secure_boot.c
core/fdt_loader.c
core/keystore.c
core/debug_lock.c
core/fw_decrypt.c
Expand Down
5 changes: 3 additions & 2 deletions SECURITY.md
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,9 @@ eBootloader follows a **90-day coordinated disclosure** policy:
| Version comparison | Phase 1 | ✅ Implemented | `eos_image_check_version()` API |
| Redundant metadata | Phase 1 | ✅ Implemented | Dual boot control block with CRC |
| Anti-tearing writes | Phase 1 | ✅ Implemented | Power-safe metadata updates |
| SHA-256 image hash | Phase 2 | 🔲 Planned | Full cryptographic hash for image verification |
| Ed25519 signatures | Phase 2 | 🔲 Planned | Digital signature verification of firmware |
| SHA-256 image hash | Phase 2 | ✅ Implemented | Full cryptographic hash for image verification |
| SHA-512 | Phase 2 | ✅ Implemented | FIPS 180-4; required by Ed25519, `core/sha512.c` |
| Ed25519 signatures | Phase 2 | ✅ Implemented | RFC 8032 verification of firmware signatures, `core/ed25519_verify.c` |
| Anti-rollback counters | Phase 2 | 🔲 Planned | Monotonic counter prevents version downgrade |
| Dual key slots | Phase 2 | 🔲 Planned | Primary + backup key for seamless rotation |
| AES-256-GCM encryption | Phase 3 | 🔲 Planned | Encrypted firmware payloads |
Expand Down
7 changes: 6 additions & 1 deletion TASKS.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,12 @@ Status is one of: `todo`, `in-progress`, `blocked`, `review`, `done`.

| ID | Task | Owner | Verified by | Evidence |
|----|------|-------|-------------|----------|
| — | None yet. | — | — | — |
| T-001 | Make Ed25519 verification actually work (it rejected every valid signature) | security | reviewer | `eos_ed25519_verify()` rejected all three RFC 8032 §7.1 vectors and rejected a signature made by `python-cryptography` — the same library `tools/sign_image.py` signs with — so **no image signed by this repo's own tool could ever boot**. Two independent causes: (a) the challenge used SHA-256 zero-padded to 64 bytes instead of SHA-512, and (b) the curve arithmetic was wrong at the base case — `[1]B` did not return the base point, and `ge_frombytes()` failed to decode valid RFC 8032 points. Replaced `core/ed25519_verify.c` with a correct RFC 8032 verifier and added `core/sha512.c`. Now: 3/3 RFC vectors accepted; 512/512 single-bit signature flips rejected; non-canonical S (S+L) rejected. |
| T-002 | Add SHA-512 (FIPS 180-4), which Ed25519 requires | backend | reviewer | New `core/sha512.c`. Digests for empty, `abc`, 1,000,000×`a`, and a 128-byte block match `sha512sum` exactly; streaming digests match the one-shot result across chunk sizes 1/63/111/112/113/127/128/129/255. |
| T-003 | Replace Ed25519 tests that could not fail | testing | reviewer | Every test in `tests/unit/test_ed25519.c` asserted only that bad signatures are *rejected*, which a verifier that rejects everything passes trivially — that is precisely the defect it missed. Rewritten to assert both directions, RFC 8032 vectors first. 10/10 pass; the positive-vector test fails against the old implementation. |
| T-004 | CI ran zero tests and reported success | testing | reviewer | `ci.yml` configured with `-DBUILD_TESTS=ON`, but this project's option is `EBLDR_BUILD_TESTS`; the flag set an unrelated cache variable and no test was ever built. `ctest` exits 0 when it finds no tests (verified: exit code 0), so the job passed green. This is how T-001 shipped. Fixed the flag and added `--no-tests=error`; with the old flag the job now exits 8. CI runs 11 tests. |
| T-005 | Fix a mismatched `extern` that no compiler could see | backend | reviewer | `core/secure_boot.c` declared `eos_ed25519_verify(msg, msg_len, sig, pubkey)` while the definition is `(signature, public_key, message, msg_len)` — a caller would have passed a `size_t` length where a key pointer was expected. Both ad-hoc `extern`s removed; the single prototype now lives in `include/eos_crypto_boot.h`. |
| T-006 | Build the two source files that were never compiled | backend | reviewer | `core/secure_boot.c` and `core/fdt_loader.c` were absent from `CMakeLists.txt`. `secure_boot.c` also called `eos_sha256()`, which was defined nowhere in the tree, so it could not have linked. Added the one-shot `eos_sha256()` to `core/crypto_boot.c` and both files to the build; clean under `-Wall -Wextra`. |

---

Expand Down
14 changes: 9 additions & 5 deletions core/crypto_boot.c
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,15 @@ int eos_crypto_safe_compare(const uint8_t *a, const uint8_t *b, size_t len)
* SHA-256 Hashing API
* ================================================================ */

void eos_sha256(const void *data, size_t len,
uint8_t digest[EOS_SHA256_DIGEST_SIZE])
{
eos_sha256_ctx_t ctx;
eos_sha256_init(&ctx);
eos_sha256_update(&ctx, (const uint8_t *)data, len);
eos_sha256_final(&ctx, digest);
}

int eos_crypto_hash(const uint8_t *data, size_t len,
uint8_t digest[EOS_SHA256_DIGEST_SIZE])
{
Expand Down Expand Up @@ -209,11 +218,6 @@ int eos_crypto_verify_image(uint32_t image_addr, uint32_t image_size,
* underlying curve arithmetic is in core/ed25519_verify.c.
* ================================================================ */

/* Forward declaration — implemented in ed25519_verify.c */
extern int eos_ed25519_verify(const uint8_t signature[64],
const uint8_t public_key[32],
const uint8_t *message, size_t msg_len);

int eos_crypto_verify_signature(const uint8_t *data, size_t data_len,
const uint8_t *signature, size_t sig_len,
const uint8_t *public_key, size_t key_len)
Expand Down
Loading
Loading