diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ee030a808..abc240589 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -5,27 +5,37 @@ # Stage 1: GATE (seconds, no Docker) # ├─ lint-format clang-format diff check # ├─ static-analysis cppcheck static analysis +# ├─ crypto-tests pinned fork crypto vectors + ASan/UBSan # ├─ secret-scan gitleaks credential detection # └─ check-submodules verify all deps present # # Stage 2: BUILD (parallel, gated by Stage 1) -# ├─ build-emulator Docker image → artifact -# └─ build-arm-firmware cross-compile → .bin/.elf (downloadable) +# ├─ build-emulator Docker image → artifact [matrix: full / bitcoin-only] +# └─ build-arm-firmware cross-compile → .bin/.elf (downloadable) [same matrix] # # Stage 3: TEST (parallel, gated by Stage 2) -# ├─ unit-tests GoogleTest (make xunit) -# └─ python-integration full test suite +# ├─ unit-tests GoogleTest (make xunit) [same matrix — proves each +# │ variant's coin/token gating actually compiles+passes] +# └─ python-integration full test suite (full/default variant only) # -# Stage 4: PUBLISH (manual trigger, all tests must pass) -# └─ publish-emulator DockerHub push (workflow_dispatch only) +# Stage 4: PUBLISH (all tests must pass) +# ├─ publish-emulator DockerHub push, full/default variant only (workflow_dispatch only) +# └─ publish-emulator-libs macOS dylib + Windows DLL → rolling prerelease +# 'emulator-dylib-latest' (push to develop or alpha). +# Both platforms ship together or the job fails. +# Tagged releases get the same pair via release.yml. name: CI on: push: - branches: [master, develop, 'feature/**', 'fix/**', 'release/**', 'hotfix/**'] + branches: [master, develop, alpha, 'feature/**', 'fix/**', 'release/**', 'hotfix/**'] + # 'release/**' is here so a stacked release train is actually gated. A PR whose + # base is another release branch does not match master/develop/alpha, so it + # reported "no checks" in the UI while the push-triggered run on the same + # branch was red -- a reviewer saw a clean PR over a failing build. pull_request: - branches: [master, develop] + branches: [master, develop, alpha, 'release/**'] workflow_dispatch: inputs: publish_emulator: @@ -54,7 +64,7 @@ jobs: timeout-minutes: 3 steps: - name: Checkout - uses: actions/checkout@v6 + uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6 with: ref: ${{ github.event.pull_request.head.sha || github.sha }} @@ -87,7 +97,7 @@ jobs: timeout-minutes: 2 steps: - name: Checkout - uses: actions/checkout@v6 + uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6 with: ref: ${{ github.event.pull_request.head.sha || github.sha }} fetch-depth: 0 @@ -96,17 +106,34 @@ jobs: # Pin and verify the only scanner binary that is installed/executed. # Bumps require an independently recorded digest and ruleset review. run: | - GITLEAKS_VERSION=8.30.1 - GITLEAKS_SHA256=551f6fc83ea457d62a0d98237cbad105af8d557003051f41f3e7ca7b3f2470eb - GITLEAKS_ARCHIVE=/tmp/gitleaks.tar.gz - curl --fail --show-error --location \ + # PINNED to 8.30.0, deliberately, and VERIFIED. + # + # Two things this block has to get right at once, and a clean merge of + # two parents previously got exactly one of them: + # + # - the VERSION is pinned because tracking releases/latest lets a new + # upstream ruleset turn this gate red with no change to this + # repository. That happened: a newer generic-api-key rule began + # flagging published BIP32 test vectors in 2014/2018 history, and + # because every build job declares `needs: [.., secret-scan]`, the + # whole build and test graph was SKIPPED rather than failed (#424). + # - the BINARY is checksummed, because a scan is only worth what the + # executable running it is. + # + # The merged version downloaded 8.30.1 with a verified checksum and then + # overwrote it with an unverified 8.30.0 through `curl | tar`, so the + # checksum proved nothing about the binary that actually ran. One + # version, one archive, one verified digest. Bump deliberately, with the + # scan re-verified. + GITLEAKS_VERSION=8.30.0 + GITLEAKS_SHA256=79a3ab579b53f71efd634f3aaf7e04a0fa0cf206b7ed434638d1547a2470a66e + GITLEAKS_ARCHIVE="${RUNNER_TEMP}/gitleaks_${GITLEAKS_VERSION}_linux_x64.tar.gz" + curl -sSfL --retry 3 --retry-all-errors \ "https://github.com/gitleaks/gitleaks/releases/download/v${GITLEAKS_VERSION}/gitleaks_${GITLEAKS_VERSION}_linux_x64.tar.gz" \ -o "${GITLEAKS_ARCHIVE}" echo "${GITLEAKS_SHA256} ${GITLEAKS_ARCHIVE}" | sha256sum --check --strict tar -xzf "${GITLEAKS_ARCHIVE}" -C /usr/local/bin gitleaks - INSTALLED_VERSION=$(gitleaks version) - echo "gitleaks ${INSTALLED_VERSION}" - test "${INSTALLED_VERSION}" = "${GITLEAKS_VERSION}" + gitleaks version - name: Run gitleaks env: @@ -138,14 +165,17 @@ jobs: static-analysis: runs-on: ubuntu-latest - timeout-minutes: 5 + timeout-minutes: 15 steps: - name: Checkout - uses: actions/checkout@v6 + uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6 with: ref: ${{ github.event.pull_request.head.sha || github.sha }} submodules: false + - name: Init crypto submodule + run: git submodule update --init deps/crypto/trezor-firmware + - name: Install cppcheck run: sudo apt-get update && sudo apt-get install -y cppcheck @@ -182,15 +212,17 @@ jobs: : "${ERRORS:=0}" "${WARNINGS:=0}" "${STYLE:=0}" "${PERF:=0}" "${PORT:=0}" TOTAL=$((ERRORS + WARNINGS + STYLE + PERF + PORT)) - echo "## cppcheck summary" >> "$GITHUB_STEP_SUMMARY" - echo "| Severity | Count |" >> "$GITHUB_STEP_SUMMARY" - echo "|----------|-------|" >> "$GITHUB_STEP_SUMMARY" - echo "| error | $ERRORS |" >> "$GITHUB_STEP_SUMMARY" - echo "| warning | $WARNINGS |" >> "$GITHUB_STEP_SUMMARY" - echo "| style | $STYLE |" >> "$GITHUB_STEP_SUMMARY" - echo "| performance | $PERF |" >> "$GITHUB_STEP_SUMMARY" - echo "| portability | $PORT |" >> "$GITHUB_STEP_SUMMARY" - echo "| **total** | **$TOTAL** |" >> "$GITHUB_STEP_SUMMARY" + { + echo "## cppcheck summary" + echo "| Severity | Count |" + echo "|----------|-------|" + echo "| error | $ERRORS |" + echo "| warning | $WARNINGS |" + echo "| style | $STYLE |" + echo "| performance | $PERF |" + echo "| portability | $PORT |" + echo "| **total** | **$TOTAL** |" + } >> "$GITHUB_STEP_SUMMARY" # Print findings as GitHub annotations cat cppcheck_report.txt @@ -208,7 +240,7 @@ jobs: echo "cppcheck: clean — zero findings" - name: Upload cppcheck report - uses: actions/upload-artifact@v7 + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7 if: always() with: name: cppcheck-report @@ -220,10 +252,13 @@ jobs: timeout-minutes: 2 steps: - name: Checkout - uses: actions/checkout@v6 + uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6 with: ref: ${{ github.event.pull_request.head.sha || github.sha }} + - name: Init crypto submodule + run: git submodule update --init deps/crypto/trezor-firmware + - name: Verify submodules are declared run: | echo "Checking required submodule declarations..." @@ -246,6 +281,84 @@ jobs: done [ "$FAILED" = "0" ] || exit 1 + - name: Enforce RC18 security invariants + run: | + python3 tools/check_pallas_api_boundary.py + if git grep -n -E \ + 'uses:[[:space:]]+[^#[:space:]]+@(v[0-9]+|main|master)([[:space:]#]|$)' \ + -- .github/workflows; then + echo "::error::Every GitHub Action must be pinned to a full commit SHA" + exit 1 + fi + if git grep -n -E 'kktech/firmware:v[0-9]+' -- \ + .github/workflows scripts/build/docker scripts/emulator; then + echo "::error::The firmware builder must be pinned by manifest digest" + exit 1 + fi + if git grep -n -E \ + 'storage_(get|upsert)ClearsignIdentity|persistent_identity_for' \ + -- lib include; then + echo "::error::Unauthenticated persistent clearsign trust is retired" + exit 1 + fi + if git grep -n -E 'return[[:space:]]+random\(\)' -- \ + lib/rand/rng.c; then + echo "::error::Emulator cryptography must not use libc random()" + exit 1 + fi + if git grep -n -F 'option(KK_ZCASH_PRIVACY' -- CMakeLists.txt; then + echo "::error::Zcash privacy must not become a third release choice" + exit 1 + fi + grep -q 'set(KK_ZCASH_PRIVACY ON)' CMakeLists.txt + grep -q 'set(KK_ZCASH_PRIVACY OFF)' CMakeLists.txt + + crypto-tests: + runs-on: ubuntu-latest + timeout-minutes: 10 + steps: + - name: Checkout + uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6 + with: + ref: ${{ github.event.pull_request.head.sha || github.sha }} + + - name: Init crypto submodule + run: git submodule update --init deps/crypto/trezor-firmware + + - name: Install crypto test dependencies + run: sudo apt-get update && sudo apt-get install -y check libssl-dev pkg-config valgrind + + - name: Run optimized crypto suite + working-directory: deps/crypto/trezor-firmware/crypto + run: | + make clean + make VALGRIND=0 tests/test_check tests/test_pallas_ct + ./tests/test_check + ./tests/test_pallas_ct + + - name: Verify Pallas secret flow with Valgrind + working-directory: deps/crypto/trezor-firmware/crypto + run: | + make clean + make VALGRIND=1 OPTFLAGS='-O2 -g' tests/test_pallas_ct + valgrind --quiet --error-exitcode=1 --track-origins=yes \ + ./tests/test_pallas_ct + + - name: Run crypto suite with ASan and UBSan + working-directory: deps/crypto/trezor-firmware/crypto + run: | + make clean + make CC='gcc -fsanitize=address,undefined' VALGRIND=0 \ + OPTFLAGS='-O1 -g -fno-omit-frame-pointer' \ + tests/test_check tests/test_pallas_ct + # Check's default per-test timeout is too short for the two exhaustive + # codepoint tests under sanitizer instrumentation on shared runners. + # Keep the job-level timeout as the hard upper bound. + CK_DEFAULT_TIMEOUT=30 \ + ASAN_OPTIONS=detect_leaks=0:halt_on_error=1 \ + UBSAN_OPTIONS=halt_on_error=1 ./tests/test_check + ASAN_OPTIONS=detect_leaks=0:halt_on_error=1 \ + UBSAN_OPTIONS=halt_on_error=1 ./tests/test_pallas_ct - name: Enforce RNG source invariants run: | if grep -q 'RAND_PLATFORM_INDEPENDENT=0' CMakeLists.txt; then @@ -265,12 +378,25 @@ jobs: # ═══════════════════════════════════════════════════════════ build-emulator: - needs: [lint-format, static-analysis, check-submodules, secret-scan] + name: build-emulator${{ matrix.label }} + needs: [lint-format, static-analysis, check-submodules, secret-scan, crypto-tests] runs-on: ubuntu-latest timeout-minutes: 15 + strategy: + fail-fast: false + matrix: + include: + # Regular/full includes every supported chain, including Zcash + # shielded/Orchard. Bitcoin-only is the sole reduced build. + - variant: full + label: "" + cmake_flags: "" + - variant: bitcoin-only + label: " (bitcoin-only)" + cmake_flags: "-DKK_BITCOIN_ONLY=ON" steps: - name: Checkout - uses: actions/checkout@v6 + uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6 with: ref: ${{ github.event.pull_request.head.sha || github.sha }} @@ -284,11 +410,11 @@ jobs: git submodule update --init deps/sca-hardening/SecAESSTM32 - name: Set up Docker Buildx - uses: docker/setup-buildx-action@v4 + uses: docker/setup-buildx-action@bb05f3f5519dd87d3ba754cc423b652a5edd6d2c # v4 - name: Cache base image id: cache-base - uses: actions/cache@v5 + uses: actions/cache@caa296126883cff596d87d8935842f9db880ef25 # v5 with: path: /tmp/base-image.tar key: base-image-${{ env.BASE_IMAGE }} @@ -303,41 +429,47 @@ jobs: if: steps.cache-base.outputs.cache-hit == 'true' run: docker load -i /tmp/base-image.tar - - name: Build emulator image + - name: Build emulator image (${{ matrix.variant }}) run: | docker build \ - -t ${{ env.EMU_IMAGE }} \ + -t ${{ env.EMU_IMAGE }}-${{ matrix.variant }} \ + --build-arg coinsupport="${{ matrix.cmake_flags }}" \ -f scripts/emulator/Dockerfile \ . - name: Save emulator image - run: docker save ${{ env.EMU_IMAGE }} -o /tmp/emu-image.tar + run: docker save ${{ env.EMU_IMAGE }}-${{ matrix.variant }} -o /tmp/emu-image.tar - name: Upload emulator image artifact - uses: actions/upload-artifact@v7 + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7 with: - name: emu-image + name: emu-image-${{ matrix.variant }} path: /tmp/emu-image.tar retention-days: 1 build-arm-firmware: - needs: [lint-format, static-analysis, check-submodules, secret-scan] + name: build-arm-firmware${{ matrix.label }} + needs: [lint-format, static-analysis, check-submodules, secret-scan, crypto-tests] runs-on: ubuntu-latest timeout-minutes: 15 strategy: fail-fast: false - # Both release variants must compile on every PR. Without the - # bitcoin-only leg, a change that only breaks the KK_BITCOIN_ONLY image - # goes green here and fails for the first time in the release build. matrix: include: + # 'label' names the job, 'suffix' names the built files, 'variant' selects + # the build. The default variant is empty in both so the flashable artifact + # matches the historical name instead of gaining a "-full" token. - variant: full + label: "" + suffix: "" cmake_flags: "" - variant: bitcoin-only + label: " (bitcoin-only)" + suffix: "-bitcoin-only" cmake_flags: "-DKK_BITCOIN_ONLY=ON" steps: - name: Checkout - uses: actions/checkout@v6 + uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6 with: ref: ${{ github.event.pull_request.head.sha || github.sha }} @@ -352,7 +484,7 @@ jobs: - name: Cache base image id: cache-base - uses: actions/cache@v5 + uses: actions/cache@caa296126883cff596d87d8935842f9db880ef25 # v5 with: path: /tmp/base-image.tar key: base-image-${{ env.BASE_IMAGE }} @@ -376,45 +508,83 @@ jobs: echo "git_short=${GIT_SHORT}" >> "$GITHUB_OUTPUT" echo "Firmware version: ${FW_VERSION} (${GIT_SHORT})" - - name: Cross-compile firmware for ARM + - name: Cross-compile firmware for ARM (${{ matrix.variant }}) run: | docker run --rm \ -v ${{ github.workspace }}:/root/keepkey-firmware:z \ ${{ env.BASE_IMAGE }} /bin/sh -c "\ mkdir /root/build && cd /root/build && \ cmake -C /root/keepkey-firmware/cmake/caches/device.cmake /root/keepkey-firmware \ - ${{ matrix.cmake_flags }} \ -DCMAKE_BUILD_TYPE=MinSizeRel \ - -DCMAKE_COLOR_MAKEFILE=ON && \ + -DCMAKE_COLOR_MAKEFILE=ON \ + ${{ matrix.cmake_flags }} && \ make && \ + python3 /root/keepkey-firmware/tools/check_pallas_ct_disassembly.py \ + --elf bin/firmware.keepkey.elf \ + --variant '${{ matrix.variant }}' && \ mkdir -p /root/keepkey-firmware/bin && \ cp bin/*.bin /root/keepkey-firmware/bin/ && \ cp bin/*.elf /root/keepkey-firmware/bin/ && \ + cp bin/*.map /root/keepkey-firmware/bin/ 2>/dev/null || true && \ + arm-none-eabi-size -A bin/firmware.keepkey.elf > /root/keepkey-firmware/bin/firmware.keepkey.size.txt 2>/dev/null || true && \ + find . -name '*.su' -print0 | tar czf /root/keepkey-firmware/bin/stack-usage.tgz --null -T - && \ chmod -R a+rw /root/keepkey-firmware/bin" + # SRAM budget gate — RC7's privacy-enabled build hard-faulted on boot + # because static SRAM left an 11.2 KB gap while msg_write() carried a + # 12.4 KB stack frame. keepkey.ld now ASSERTs a 16 KiB reserve at link + # time; this step reports the numbers and enforces the frame margin + # (tools/sram-budgets.json). + - name: SRAM budget gate (${{ matrix.variant }}) + run: | + pip install --quiet pyelftools + # Mirror the report into the run summary. The gate has always + # enforced correctly, but its numbers only ever existed in a job + # log nobody opens, so the frame-arena fix had no reviewable + # evidence anywhere -- an RC audit recorded it as "zero coverage". + # pipefail so tee cannot mask a budget breach, and capture the status + # instead of letting `set -e` abort here -- a FAILING gate is exactly + # when the numbers need to reach the summary. Re-exit with it below. + set -o pipefail + rc=0 + python3 tools/check_sram_budget.py \ + --elf bin/firmware.keepkey.elf \ + --su-tar bin/stack-usage.tgz \ + --budgets tools/sram-budgets.json \ + --variant "${{ matrix.variant }}" 2>&1 \ + | tee "/tmp/sram-${{ matrix.variant }}.txt" || rc=$? + { + echo "### SRAM budget — ${{ matrix.variant }}" + echo '```' + cat "/tmp/sram-${{ matrix.variant }}.txt" + echo '```' + } >> "$GITHUB_STEP_SUMMARY" + exit $rc + - name: Rename firmware artifacts run: | cd bin for f in *.bin; do [ -f "$f" ] || continue - mv "$f" "firmware.keepkey.v${{ steps.version.outputs.fw_version }}-${{ steps.version.outputs.git_short }}-${f}" + mv "$f" "firmware.keepkey.v${{ steps.version.outputs.fw_version }}-${{ steps.version.outputs.git_short }}${{ matrix.suffix }}-${f}" done for f in *.elf; do [ -f "$f" ] || continue - mv "$f" "firmware.keepkey.v${{ steps.version.outputs.fw_version }}-${{ steps.version.outputs.git_short }}-${f}" + mv "$f" "firmware.keepkey.v${{ steps.version.outputs.fw_version }}-${{ steps.version.outputs.git_short }}${{ matrix.suffix }}-${f}" done ls -lh - echo "::notice::Firmware v${{ steps.version.outputs.fw_version }} built successfully" + echo "::notice::Firmware v${{ steps.version.outputs.fw_version }} (${{ matrix.variant }}) built successfully" - name: Upload firmware artifacts - uses: actions/upload-artifact@v7 + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7 with: - # matrix.variant in the name: two legs uploading the same artifact - # name is a hard failure on upload-artifact@v4+ (see release.yml). - name: firmware-v${{ steps.version.outputs.fw_version }}-${{ steps.version.outputs.git_short }}-${{ matrix.variant }} + name: firmware-v${{ steps.version.outputs.fw_version }}-${{ steps.version.outputs.git_short }}${{ matrix.suffix }} path: | bin/*.bin bin/*.elf + bin/*.map + bin/*.size.txt + bin/stack-usage.tgz retention-days: 90 # ═══════════════════════════════════════════════════════════ @@ -422,42 +592,53 @@ jobs: # ═══════════════════════════════════════════════════════════ unit-tests: + name: unit-tests${{ matrix.label }} needs: build-emulator runs-on: ubuntu-latest timeout-minutes: 10 + strategy: + fail-fast: false + matrix: + include: + - variant: full + label: "" + cmake_flags: "" + - variant: bitcoin-only + label: " (bitcoin-only)" + cmake_flags: "-DKK_BITCOIN_ONLY=ON" steps: - name: Download emulator image - uses: actions/download-artifact@v8 + uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8 with: - name: emu-image + name: emu-image-${{ matrix.variant }} path: /tmp - name: Load emulator image run: docker load -i /tmp/emu-image.tar - - name: Run unit tests + - name: Run unit tests (${{ matrix.variant }}) run: | # make xunit returns non-zero if any test fails — capture # exit code so JUnit XML still gets copied for reporting docker run --rm \ -v ${{ github.workspace }}/test-reports:/kkemu/test-reports \ --entrypoint /bin/sh \ - ${{ env.EMU_IMAGE }} \ + ${{ env.EMU_IMAGE }}-${{ matrix.variant }} \ -c "mkdir -p /kkemu/test-reports/firmware-unit && \ make xunit; RC=\$?; \ cp -r unittests/*.xml /kkemu/test-reports/firmware-unit/ 2>/dev/null; \ exit \$RC" - name: Upload unit test results - uses: actions/upload-artifact@v7 + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7 if: always() with: - name: unit-test-results + name: unit-test-results-${{ matrix.variant }} path: test-reports/firmware-unit/ retention-days: 30 python-integration-tests: - needs: [lint-format, static-analysis, check-submodules, secret-scan] + needs: [lint-format, static-analysis, check-submodules, secret-scan, crypto-tests] runs-on: ubuntu-latest timeout-minutes: 30 strategy: @@ -474,7 +655,7 @@ jobs: oled_artifact: oled-screenshots-bitcoin-only steps: - name: Checkout - uses: actions/checkout@v6 + uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6 with: ref: ${{ github.event.pull_request.head.sha || github.sha }} @@ -490,16 +671,25 @@ jobs: - name: Build and run tests (docker compose) working-directory: scripts/emulator run: | + # Run each test container — capture exit codes, always extract reports + # + # `|| RC=$?` rather than `; RC=$?`: this step runs under the default + # `bash -e`, where a bare failing command aborts immediately. With the + # semicolon form the abort happened BEFORE the assignment and before + # the extraction below, so a failing test run uploaded nothing at all + # — no JUnit, no report, no OLED frames — precisely when the evidence + # was needed. The `if: always()` on the upload steps could not help, + # because nothing had been copied out of the container yet. + FW_RC=0 + PY_RC=0 COMPOSE_ARGS=(-f docker-compose.yml) if [ "${{ matrix.variant }}" = "bitcoin-only" ]; then COMPOSE_ARGS+=(-f docker-compose.bitcoin-only.yml) fi - - # Run each test container — capture exit codes, always extract reports docker compose "${COMPOSE_ARGS[@]}" -p ${{ matrix.project }} \ - up --build --exit-code-from firmware-unit firmware-unit; FW_RC=$? + up --build --exit-code-from firmware-unit firmware-unit || FW_RC=$? docker compose "${COMPOSE_ARGS[@]}" -p ${{ matrix.project }} \ - up --build --exit-code-from python-keepkey python-keepkey; PY_RC=$? + up --build --exit-code-from python-keepkey python-keepkey || PY_RC=$? REPORT_ROOT=${{ github.workspace }}/test-reports/${{ matrix.variant }} mkdir -p "$REPORT_ROOT" @@ -529,7 +719,7 @@ jobs: [ "$FW_RC" -eq 0 ] && [ "$PY_RC" -eq 0 ] || exit 1 - name: Upload Python test results - uses: actions/upload-artifact@v7 + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7 if: always() with: name: ${{ matrix.python_artifact }} @@ -537,7 +727,7 @@ jobs: retention-days: 30 - name: Upload native test results - uses: actions/upload-artifact@v7 + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7 if: always() with: name: firmware-unit-results-${{ matrix.variant }} @@ -545,7 +735,7 @@ jobs: retention-days: 30 - name: Upload OLED screenshots - uses: actions/upload-artifact@v4 + uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4 if: always() with: name: ${{ matrix.oled_artifact }} @@ -600,12 +790,12 @@ jobs: # fsm_msgDebugLinkGetState is excluded from the build and any # read_layout() call hangs the test). python-dylib-tests: - needs: [lint-format, static-analysis, check-submodules, secret-scan] + needs: [lint-format, static-analysis, check-submodules, secret-scan, crypto-tests] runs-on: macos-latest timeout-minutes: 25 steps: - name: Checkout - uses: actions/checkout@v6 + uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6 with: ref: ${{ github.event.pull_request.head.sha || github.sha }} @@ -622,7 +812,7 @@ jobs: git submodule update --init deps/googletest - name: Setup Python 3.10 - uses: actions/setup-python@v5 + uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5 with: python-version: '3.10' @@ -701,7 +891,8 @@ jobs: # it fsm_msgDebugLinkGetState is excluded from the build. # CMAKE_POLICY_VERSION_MINIMUM works around vendored # googletest's pre-3.5 policy declaration. - export PATH="$PATH:$(python -c 'import os, nanopb; print(os.path.dirname(nanopb.__file__))')/generator" + NANOPB_GENERATOR_DIR="$(python -c 'import os, nanopb; print(os.path.dirname(nanopb.__file__))')/generator" + export PATH="$PATH:$NANOPB_GENERATOR_DIR" which protoc-gen-nanopb which nanopb_generator.py cmake \ @@ -713,26 +904,76 @@ jobs: - name: Build kkemulator_dylib run: | - export PATH="$PATH:$(python -c 'import os, nanopb; print(os.path.dirname(nanopb.__file__))')/generator" - cmake --build build-emu --target kkemulator_dylib -j$(sysctl -n hw.ncpu) + NANOPB_GENERATOR_DIR="$(python -c 'import os, nanopb; print(os.path.dirname(nanopb.__file__))')/generator" + export PATH="$PATH:$NANOPB_GENERATOR_DIR" + CPU_COUNT="$(sysctl -n hw.ncpu)" + cmake --build build-emu --target kkemulator_dylib -j"$CPU_COUNT" ls -la build-emu/lib/libkkemu* || ls -la build-emu/lib/emulator/libkkemu* || true # Surface the resolved binary path for the run step. macOS # produces .dylib; .so is preserved as a fallback for when this # job goes cross-platform. - DYLIB=$(find build-emu -name 'libkkemu.dylib' -o -name 'libkkemu.so' | head -1) + DYLIB="$(find build-emu \( -name 'libkkemu.dylib' -o -name 'libkkemu.so' \) -print -quit)" test -f "$DYLIB" || (echo "::error::libkkemu artifact not found" && exit 1) - echo "DYLIB_PATH=$(pwd)/$DYLIB" >> $GITHUB_ENV + echo "DYLIB_PATH=$(pwd)/$DYLIB" >> "$GITHUB_ENV" + + # ── Windows emulator DLL (cross-compiled from this macOS runner) ── + # Uses cmake/toolchains/mingw-w64-x86_64.cmake, so no Windows runner + # is needed. BLOCKING on purpose: the DLL and the dylib are the same + # deliverable on two platforms, and this job is the only place either + # is built. When the cross-build was non-blocking, a regression here + # produced a macOS-only set and stayed green. + - name: Install MinGW-w64 + run: brew install mingw-w64 + + - name: Cross-build Windows emulator DLL + run: | + # Same nanopb PATH handling as the macOS configure step above; the + # toolchain file cannot see the pyenv wrappers on its own. + NANOPB_DIR="$(python -c 'import os, nanopb; print(os.path.dirname(nanopb.__file__))')" + export PATH="$PATH:$NANOPB_DIR/generator" + CPU_COUNT="$(sysctl -n hw.ncpu)" + NANOPB_PLUGIN="$(command -v protoc-gen-nanopb)" + cmake \ + -DCMAKE_TOOLCHAIN_FILE=cmake/toolchains/mingw-w64-x86_64.cmake \ + -DKK_EMULATOR=1 \ + -DKK_BUILD_DYLIB=1 \ + -DKK_DEBUG_LINK=ON \ + -DNANOPB_DIR="$NANOPB_DIR" \ + -DNANOPB_PLUGIN="$NANOPB_PLUGIN" \ + -DCMAKE_POLICY_VERSION_MINIMUM=3.5 \ + -B build-emu-win . + cmake --build build-emu-win --target kkemulator_dylib -j"$CPU_COUNT" + DLL="$(find build-emu-win -name 'libkkemu.dll' -print -quit)" + test -f "$DLL" || (echo "::error::libkkemu.dll not produced by cross-build" && exit 1) + file "$DLL" + echo "WIN_DLL_PATH=$(pwd)/$DLL" >> "$GITHUB_ENV" + + - name: Stage emulator libraries + # One directory, published names applied here, so every consumer + # (this artifact, the rolling prerelease, a tagged firmware + # release) ships the identical pair of files. + if: always() && env.DYLIB_PATH != '' && env.WIN_DLL_PATH != '' + run: | + mkdir -p emulator-libs + cp "$DYLIB_PATH" emulator-libs/libkkemu-macos-arm64.dylib + cp "$WIN_DLL_PATH" emulator-libs/libkkemu-win-x64.dll + ls -lh emulator-libs/ - - name: Upload libkkemu.dylib - # Always upload, even on later test failure — the binary is + - name: Upload emulator libraries + # Always upload, even on later test failure — the binaries are # what vault and external auditors actually consume from this # PR. Tagged with the short commit SHA so multiple PR pushes # don't overwrite each other when a reviewer downloads them. - if: always() && env.DYLIB_PATH != '' - uses: actions/upload-artifact@v4 + # + # Gated on BOTH paths: a partial artifact carrying only the dylib + # is exactly the silent drop this coupling exists to prevent, and + # release.yml treats this artifact as the source of truth for a + # tagged release's emulator assets. + if: always() && env.DYLIB_PATH != '' && env.WIN_DLL_PATH != '' + uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4 with: - name: libkkemu-${{ github.sha }} - path: ${{ env.DYLIB_PATH }} + name: libkkemu-${{ github.event.pull_request.head.sha || github.sha }} + path: emulator-libs/ retention-days: 30 if-no-files-found: error @@ -749,7 +990,7 @@ jobs: # skip rationale), it's a transitive dep some tests use. pip install pytest pytest-timeout - - name: Run dylib screenshot tests + - name: Run dylib transport tests working-directory: deps/python-keepkey/tests env: KK_TRANSPORT: dylib @@ -757,12 +998,26 @@ jobs: run: | # `keepkeylib/` on PYTHONPATH so the package's relative-style # imports inside generated *_pb2.py files resolve. + # + # test_dylib_confirm_flow contributes exactly ONE test here: + # test_features_round_trip, the pure Initialize -> Features path + # that never enters confirm_helper. It is named explicitly rather + # than adding the whole file, because the file's other test + # (test_load_device_with_auto_confirm) is @unittest.skip'd on an + # OPEN firmware bug -- confirm_helper busy-loops on a ButtonAck + # the dylib consumed but never delivered, and no pytest-timeout + # method can interrupt a C-level kkemu_poll() loop. Adding the + # file wholesale would trade a silent coverage gap for a runner + # that burns the job's 25-minute timeout. + # + # When that firmware fix lands, run the whole file instead. PYTHONPATH=../keepkeylib:.. python -m pytest \ test_dylib_screenshot.py \ + "test_dylib_confirm_flow.py::TestDylibConfirmFlow::test_features_round_trip" \ -v --tb=short --junit-xml=../../../test-reports/dylib-junit.xml - name: Upload dylib test results - uses: actions/upload-artifact@v4 + uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4 if: always() with: name: python-dylib-test-results @@ -781,26 +1036,28 @@ jobs: timeout-minutes: 5 steps: - name: Checkout - uses: actions/checkout@v4 + uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4 with: ref: ${{ github.event.pull_request.head.sha || github.sha }} - name: Download unit test results - uses: actions/download-artifact@v4 + uses: actions/download-artifact@d3f86a106a0bac45b974a628896c90dbdf5c8093 # v4 continue-on-error: true with: - name: unit-test-results + # Report covers the regular/full build. Bitcoin-only is built and + # unit-tested in its own matrix leg but does not get a PDF. + name: unit-test-results-full path: test-reports/firmware-unit/ - name: Download python test results - uses: actions/download-artifact@v4 + uses: actions/download-artifact@d3f86a106a0bac45b974a628896c90dbdf5c8093 # v4 continue-on-error: true with: name: python-test-results path: test-reports/python-keepkey/ - name: Download OLED screenshots - uses: actions/download-artifact@v4 + uses: actions/download-artifact@d3f86a106a0bac45b974a628896c90dbdf5c8093 # v4 continue-on-error: true with: name: oled-screenshots @@ -822,7 +1079,7 @@ jobs: run: python3 scripts/generate-test-report.py - name: Upload test report - uses: actions/upload-artifact@v4 + uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4 if: always() with: name: test-report @@ -903,14 +1160,15 @@ jobs: timeout-minutes: 10 steps: - name: Checkout - uses: actions/checkout@v6 + uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6 with: ref: ${{ github.event.pull_request.head.sha || github.sha }} - name: Download emulator image - uses: actions/download-artifact@v8 + uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8 with: - name: emu-image + # Publish the regular/full image, including Zcash privacy support. + name: emu-image-full path: /tmp - name: Load emulator image @@ -925,11 +1183,11 @@ jobs: - name: Tag images for publish run: | - docker tag ${{ env.EMU_IMAGE }} kktech/kkemu:latest - docker tag ${{ env.EMU_IMAGE }} kktech/kkemu:v${{ steps.version.outputs.fw_version }} + docker tag ${{ env.EMU_IMAGE }}-full kktech/kkemu:latest + docker tag ${{ env.EMU_IMAGE }}-full kktech/kkemu:v${{ steps.version.outputs.fw_version }} - name: Login to DockerHub - uses: docker/login-action@v4 + uses: docker/login-action@abd2ef45e78c5afb21d64d4ca52ee8550d9572c7 # v4 with: username: ${{ secrets.KK_DOCKERHUB_USER }} password: ${{ secrets.KK_DOCKERHUB_PASS }} @@ -938,3 +1196,82 @@ jobs: run: | docker push kktech/kkemu:latest docker push kktech/kkemu:v${{ steps.version.outputs.fw_version }} + + # ═══════════════════════════════════════════════════════════ + # STAGE 4b: PUBLISH EMULATOR LIBS — rolling release of the + # emulator native libraries so downstream consumers (vault + # native-emulator / OLED preview) have a stable download URL + # instead of scraping per-run, 30-day-retention CI artifacts. + # + # Runs for every integration branch that ships emulator work — + # develop is the release line, alpha is the fork-pinned + # integration branch, and both are consumed. Tagged releases + # get the same two files from release.yml, which reuses the + # artifact this job's dependency produced. + # + # macOS dylib and Windows DLL ship together, always. They are + # one deliverable on two platforms; a release carrying only + # one is a broken release, not a partial success, so every + # check here is fatal. + # ═══════════════════════════════════════════════════════════ + publish-emulator-libs: + needs: [python-dylib-tests] + if: >- + github.event_name == 'push' && + (github.ref == 'refs/heads/develop' || github.ref == 'refs/heads/alpha') + runs-on: ubuntu-latest + timeout-minutes: 10 + permissions: + contents: write + steps: + - name: Checkout + uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6 + + - name: Download emulator libraries + uses: actions/download-artifact@d3f86a106a0bac45b974a628896c90dbdf5c8093 # v4 + with: + name: libkkemu-${{ github.sha }} + path: release-dylib + + - name: Stage release assets + run: | + FW_VERSION=$(sed -n '/^project/,/)/p' CMakeLists.txt | grep -oP '\d+\.\d+\.\d+') + # Traceability — which firmware/commit these binaries correspond to. + { + echo "firmware_version=${FW_VERSION}" + echo "commit=${GITHUB_SHA}" + echo "branch=${GITHUB_REF_NAME}" + echo "built_at=$(date -u +%Y-%m-%dT%H:%M:%SZ)" + } > release-dylib/VERSION.txt + echo "=== Assets ===" && ls -lh release-dylib/ + # Both binaries or no publish. The upload side already enforces + # this; asserting again here means a hand-crafted or partially + # expired artifact cannot slip a one-platform release through. + for asset in libkkemu-macos-arm64.dylib libkkemu-win-x64.dll; do + test -f "release-dylib/$asset" || \ + (echo "::error::$asset missing — refusing to publish a partial emulator release" && exit 1) + done + + - name: Publish to rolling prerelease + env: + GH_TOKEN: ${{ github.token }} + run: | + # Idempotent: create the rolling tag on first use, then replace + # its assets in place. `gh release upload --clobber` is used + # rather than an action that rewrites the release body, so the + # description stays stable across every update. + if ! gh release view emulator-dylib-latest --repo "$GITHUB_REPOSITORY" >/dev/null 2>&1; then + gh release create emulator-dylib-latest --repo "$GITHUB_REPOSITORY" \ + --prerelease --title "Emulator libraries (rolling)" \ + --notes "Rolling build of the KeepKey emulator native libraries. + + - \`libkkemu-macos-arm64.dylib\` — macOS Apple Silicon + - \`libkkemu-win-x64.dll\` — Windows x86_64 (MinGW cross-build) + - \`VERSION.txt\` — firmware version + commit these correspond to + + Both platforms are published together; CI fails rather than + shipping one without the other. Updated on every push to + \`develop\` and \`alpha\`. Not a signed firmware release." + fi + gh release upload emulator-dylib-latest release-dylib/* \ + --repo "$GITHUB_REPOSITORY" --clobber diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 7022c5f41..78570b00b 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -1,14 +1,14 @@ # KeepKey Firmware Release Pipeline # -# Triggered by version tags (v*) on master. +# Triggered by version tags (v*). # Builds firmware, computes reproducible hashes, # and creates a draft GitHub Release with all artifacts. # -# Git-flow: tag master after merging a release/* or hotfix/* branch. +# Tag a commit only after its CI workflow has completed successfully. # # Usage: -# git tag v7.11.0 -# git push origin v7.11.0 +# git tag -a v7.15.0-rc17 -m "KeepKey firmware 7.15.0 RC17 test candidate" +# git push origin v7.15.0-rc17 name: Release @@ -29,19 +29,53 @@ jobs: timeout-minutes: 3 outputs: fw_version: ${{ steps.version.outputs.fw_version }} + tag_name: ${{ steps.version.outputs.tag_name }} + is_prerelease: ${{ steps.version.outputs.is_prerelease }} steps: - - uses: actions/checkout@v6 - with: - submodules: recursive + - uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6 - name: Extract and verify version id: version run: | TAG_VERSION="${GITHUB_REF_NAME#v}" FW_VERSION=$(sed -n '/^project/,/)/p' CMakeLists.txt | grep -oP '\d+\.\d+\.\d+') - echo "fw_version=${FW_VERSION}" >> "$GITHUB_OUTPUT" + IS_PRERELEASE=false + RC_PREFIX="${FW_VERSION}-rc" + if [ "$TAG_VERSION" != "$FW_VERSION" ]; then - echo "::error::Tag (${TAG_VERSION}) != CMakeLists.txt (${FW_VERSION})" + if [[ "$TAG_VERSION" != "${RC_PREFIX}"* ]]; then + echo "::error::Tag (${TAG_VERSION}) must be ${FW_VERSION} or ${RC_PREFIX}" + exit 1 + fi + + RC_NUMBER="${TAG_VERSION#"${RC_PREFIX}"}" + if ! [[ "$RC_NUMBER" =~ ^[0-9]+$ ]]; then + echo "::error::Invalid release-candidate tag (${TAG_VERSION}); expected ${RC_PREFIX}" + exit 1 + fi + IS_PRERELEASE=true + fi + + { + echo "fw_version=${FW_VERSION}" + echo "tag_name=${GITHUB_REF_NAME}" + echo "is_prerelease=${IS_PRERELEASE}" + } >> "$GITHUB_OUTPUT" + + - name: Require green CI on the tagged commit + env: + GH_TOKEN: ${{ github.token }} + run: | + # A tag on a red (or untested) commit must not produce release + # artifacts. The tagged SHA already ran the CI workflow on its + # branch push; require that run to exist and have succeeded. + CONCLUSION=$(gh run list --repo "$GITHUB_REPOSITORY" \ + --workflow CI --commit "$GITHUB_SHA" \ + --json status,conclusion \ + --jq '[.[] | select(.status == "completed")] | map(.conclusion) | first') + echo "CI conclusion for $GITHUB_SHA: ${CONCLUSION:-none}" + if [ "$CONCLUSION" != "success" ]; then + echo "::error::No successful CI run found for ${GITHUB_SHA} — refusing to release." exit 1 fi @@ -65,13 +99,20 @@ jobs: cmake_flags: "-DKK_BITCOIN_ONLY=ON" timeout-minutes: 20 steps: - - uses: actions/checkout@v6 - with: - submodules: recursive + - uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6 + + - name: Init required submodules + run: | + git submodule update --init deps/crypto/trezor-firmware + git submodule update --init deps/device-protocol + git submodule update --init --recursive deps/python-keepkey + git submodule update --init deps/googletest + git submodule update --init deps/qrenc/QR-Code-generator + git submodule update --init deps/sca-hardening/SecAESSTM32 - name: Cache base image id: cache-base - uses: actions/cache@v5 + uses: actions/cache@caa296126883cff596d87d8935842f9db880ef25 # v5 with: path: /tmp/base-image.tar key: base-image-${{ env.BASE_IMAGE }} @@ -86,50 +127,46 @@ jobs: if: steps.cache-base.outputs.cache-hit == 'true' run: docker load -i /tmp/base-image.tar - - name: Cross-compile firmware + - name: Cross-compile firmware (${{ matrix.variant }}) run: | docker run --rm \ -v ${{ github.workspace }}:/root/keepkey-firmware:z \ ${{ env.BASE_IMAGE }} /bin/sh -c "\ mkdir /root/build && cd /root/build && \ cmake -C /root/keepkey-firmware/cmake/caches/device.cmake /root/keepkey-firmware \ - ${{ matrix.cmake_flags }} \ -DCMAKE_BUILD_TYPE=MinSizeRel \ - -DCMAKE_COLOR_MAKEFILE=ON && \ + -DCMAKE_COLOR_MAKEFILE=ON \ + ${{ matrix.cmake_flags }} && \ make && \ mkdir -p /root/keepkey-firmware/release && \ cp bin/firmware.keepkey.bin /root/keepkey-firmware/release/ && \ cp bin/firmware.keepkey.elf /root/keepkey-firmware/release/ && \ - cp bin/bootloader.bin /root/keepkey-firmware/release/ 2>/dev/null || true && \ + find . -name '*.su' -print0 | tar czf /root/keepkey-firmware/release/stack-usage.tgz --null -T - && \ chmod -R a+rw /root/keepkey-firmware/release" - - name: Compute hashes - working-directory: release + # Same SRAM budget gate CI enforces (rc8 boot-fault class): release + # artifacts must clear it too, not just the 16 KiB linker ASSERT. + - name: SRAM budget gate (${{ matrix.variant }}) run: | - SUFFIX="${{ matrix.suffix }}" - echo "# KeepKey Firmware v${{ needs.validate.outputs.fw_version }} — Hash Manifest" > "HASHES${SUFFIX}.txt" - echo "" >> "HASHES${SUFFIX}.txt" - # Provenance: name the exact toolchain these bytes came out of. BASE_IMAGE - # is a sha256 manifest digest, not a tag, so this identifies one immutable - # image rather than whatever the tag pointed at on the day. Without it a - # green CI build and a locally reproduced binary cannot be shown to be the - # same toolchain product. See GH #425. - echo "builder image ${BASE_IMAGE}" >> "HASHES${SUFFIX}.txt" - echo "source commit ${GITHUB_SHA}" >> "HASHES${SUFFIX}.txt" - echo "" >> "HASHES${SUFFIX}.txt" - for f in *.bin; do - [ -f "$f" ] || continue - FULL_HASH=$(sha256sum "$f" | awk '{print $1}') - echo "sha256 (full) $f $FULL_HASH" >> "HASHES${SUFFIX}.txt" - FILE_SIZE=$(stat -c%s "$f") - if [ "$FILE_SIZE" -gt 256 ]; then - PAYLOAD_HASH=$(tail -c +257 "$f" | sha256sum | awk '{print $1}') - echo "sha256 (payload) $f $PAYLOAD_HASH" >> "HASHES${SUFFIX}.txt" - fi - echo "" >> "HASHES${SUFFIX}.txt" - done - cat "HASHES${SUFFIX}.txt" + pip install --quiet pyelftools + python3 tools/check_sram_budget.py \ + --elf release/firmware.keepkey.elf \ + --su-tar release/stack-usage.tgz \ + --budgets tools/sram-budgets.json \ + --variant "${{ matrix.variant }}" + # A RELEASE PUBLISHES FIRMWARE ONLY -- never a bootloader. + # + # The build produces bin/bootloader.bin, and this step used to copy it + # into release/, where the rename turned it into bootloader.v.bin and + # release-assets/* published it. A bootloader is a separately signed + # artifact with its own rollout, and shipping one as a side effect of + # tagging firmware is how a device gets bricked by an image nobody + # reviewed as a bootloader release. It is no longer copied, so it cannot + # be renamed, hashed or attached. + # + # Renaming has to happen BEFORE hashing, or the manifest names files that + # are never published. - name: Rename artifacts working-directory: release run: | @@ -137,11 +174,40 @@ jobs: SUFFIX="${{ matrix.suffix }}" [ -f firmware.keepkey.bin ] && mv firmware.keepkey.bin "firmware.keepkey.v${VER}${SUFFIX}.bin" [ -f firmware.keepkey.elf ] && mv firmware.keepkey.elf "firmware.keepkey.v${VER}${SUFFIX}.elf" - [ -f bootloader.bin ] && mv bootloader.bin "bootloader.v${VER}.bin" ls -lh + # The manifest CI can produce describes the UNSIGNED build, because the + # signatures do not exist yet: hash-manifest.sh reads that state off the + # artifacts and says so in the file. Key holders re-run the same script + # over the signed binaries before publishing -- see the release body's + # signing checklist -- so the published hashes describe the published + # bytes rather than a draft nobody installs. + - name: Compute hashes + run: | + scripts/release/hash-manifest.sh release \ + "${{ needs.validate.outputs.fw_version }}" \ + "${{ matrix.variant }}" \ + "${{ matrix.suffix }}" + + - name: Record build provenance + working-directory: release + run: | + VARIANT="${{ matrix.variant }}" + SUFFIX="${{ matrix.suffix }}" + { + echo "firmware_commit=${GITHUB_SHA}" + echo "source_repository=${GITHUB_REPOSITORY}" + echo "source_ref=${GITHUB_REF}" + echo "workflow_ref=${GITHUB_WORKFLOW_REF}" + echo "builder_image=${BASE_IMAGE}" + echo "runner_image=${ImageOS:-unknown}" + echo "variant=${VARIANT}" + echo "cmake_flags=${{ matrix.cmake_flags }}" + } > "PROVENANCE${SUFFIX}.txt" + cat "PROVENANCE${SUFFIX}.txt" + - name: Upload release artifacts - uses: actions/upload-artifact@v7 + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7 with: # Per-variant, because upload-artifact v4+ makes names immutable: two # matrix legs writing one name is a hard failure, not a merge. @@ -153,14 +219,29 @@ jobs: needs: validate runs-on: ubuntu-latest timeout-minutes: 20 + strategy: + fail-fast: false + matrix: + include: + - variant: full + cmake_flags: "" + - variant: bitcoin-only + cmake_flags: "-DKK_BITCOIN_ONLY=ON" steps: - - uses: actions/checkout@v6 - with: - submodules: recursive + - uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6 + + - name: Init required submodules + run: | + git submodule update --init deps/crypto/trezor-firmware + git submodule update --init deps/device-protocol + git submodule update --init --recursive deps/python-keepkey + git submodule update --init deps/googletest + git submodule update --init deps/qrenc/QR-Code-generator + git submodule update --init deps/sca-hardening/SecAESSTM32 - name: Cache base image id: cache-base - uses: actions/cache@v5 + uses: actions/cache@caa296126883cff596d87d8935842f9db880ef25 # v5 with: path: /tmp/base-image.tar key: base-image-${{ env.BASE_IMAGE }} @@ -175,21 +256,29 @@ jobs: if: steps.cache-base.outputs.cache-hit == 'true' run: docker load -i /tmp/base-image.tar - - name: Build and test emulator + - name: Build and test emulator (${{ matrix.variant }}) run: | - docker build -t kkemu-release -f scripts/emulator/Dockerfile . - docker run --rm --entrypoint /bin/sh kkemu-release \ + docker build -t kkemu-release-${{ matrix.variant }} \ + --build-arg coinsupport="${{ matrix.cmake_flags }}" \ + -f scripts/emulator/Dockerfile . + docker run --rm --entrypoint /bin/sh kkemu-release-${{ matrix.variant }} \ -c "make xunit; RC=\$?; exit \$RC" create-release: needs: [validate, build-firmware, test] runs-on: ubuntu-latest timeout-minutes: 5 + permissions: + contents: write + # Reading the CI run's artifacts (the emulator libraries) needs + # actions:read. The top-level block grants contents only, and a + # declared block zeroes every scope it omits. + actions: read steps: - - uses: actions/checkout@v6 + - uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6 - name: Download firmware artifacts - uses: actions/download-artifact@v8 + uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8 with: pattern: release-firmware-* path: artifacts @@ -198,36 +287,118 @@ jobs: - name: Prepare release assets run: | mkdir -p release-assets - cp artifacts/*.bin artifacts/*.elf artifacts/HASHES*.txt release-assets/ + # No dash in the glob: the default variant's files are HASHES.txt and + # PROVENANCE.txt, and a 'HASHES-*' pattern would silently drop them. + cp artifacts/*.bin artifacts/*.elf artifacts/HASHES*.txt \ + artifacts/PROVENANCE*.txt release-assets/ + ls -lh release-assets/ + + # Firmware only. If a bootloader ever reaches this directory again, + # stop rather than publish it. + if ls release-assets/ | grep -i bootloader; then + echo "::error::A bootloader artifact reached the release assets." + exit 1 + fi + + - name: Attach emulator libraries + env: + GH_TOKEN: ${{ github.token }} + run: | + # Every release ships the emulator native libs alongside the + # device firmware. They are NOT rebuilt here: CI already built + # and tested them on the branch push this tag points at, and + # validate/ already required that run to be green. Reusing that + # artifact means the published binaries are the ones that were + # tested, not a lookalike from a second build. + RUN_ID=$(gh run list --repo "$GITHUB_REPOSITORY" \ + --workflow CI --commit "$GITHUB_SHA" \ + --json status,conclusion,databaseId \ + --jq '[.[] | select(.status == "completed" and .conclusion == "success")] | map(.databaseId) | first') + if [ -z "$RUN_ID" ] || [ "$RUN_ID" = "null" ]; then + echo "::error::No successful CI run for ${GITHUB_SHA} — cannot source emulator libraries." + exit 1 + fi + echo "Sourcing emulator libraries from CI run ${RUN_ID}" + # Artifacts expire after 30 days. Tagging a commit older than + # that means re-running CI on it to regenerate them. + gh run download "$RUN_ID" --repo "$GITHUB_REPOSITORY" \ + -n "libkkemu-${GITHUB_SHA}" -D emulator-libs + + # Both platforms or no release. A firmware release that ships a + # macOS emulator lib and silently omits the Windows one is the + # exact failure this coupling exists to prevent. + for asset in libkkemu-macos-arm64.dylib libkkemu-win-x64.dll; do + test -f "emulator-libs/$asset" || \ + (echo "::error::$asset missing from CI artifact — refusing to publish a partial emulator set." && exit 1) + done + cp emulator-libs/libkkemu-macos-arm64.dylib \ + emulator-libs/libkkemu-win-x64.dll release-assets/ ls -lh release-assets/ - name: Generate release body run: | VER="${{ needs.validate.outputs.fw_version }}" cat > release-body.md < **DRAFT** — firmware must be signed by 3/5 key holders before publishing. + > **DRAFT TEST CANDIDATE** — RC artifacts are unsigned and intended for + > release-candidate testing. Firmware must be signed by 3/5 key holders + > before publishing a production release. + > + > The attached \`HASHES*.txt\` describes the **unsigned** build. Signing + > rewrites the 256-byte metadata descriptor, so the device-image hash + > changes; only the payload hash survives it. Do not pin a hash from a + > draft manifest. - ### Signing Checklist - - [ ] Built on multiple machines, hashes match + ### Signing Checklist (per variant) + - [ ] Built on multiple machines, payload hashes match - [ ] Signed on air-gapped machine (3/5 signers) - [ ] Storage upgrade tested on production device + - [ ] **Signatures VERIFIED on every signed variant, BEFORE upload:** + \`scripts/release/verify-signatures.py \` + The real 3-of-5 ECDSA check -- the same one the device does -- + parsing the keys from include/keepkey/board/pubkeys.h so a + rotation cannot leave it checking a stale set. + The \`--require-signed\` manifest step below is STRUCTURAL ONLY: + a signature region holding a single non-zero byte passes it, so + it cannot tell a signed image from an unsigned one. Run both, + and run this one first. - [ ] Signed .bin uploaded, replacing unsigned + - [ ] \`HASHES*.txt\` regenerated from the signed binaries and re-uploaded: + \`scripts/release/hash-manifest.sh --require-signed ${VER} \` + (fails if any image is missing its quorum; the device-image hash it + prints is what Vault should pin) - [ ] Release notes finalized EOF - name: Create draft release - uses: softprops/action-gh-release@v2 + uses: softprops/action-gh-release@3bb12739c298aeb8a4eeaf626c5b8d85266b0e65 # v2 with: draft: true - name: "Firmware v${{ needs.validate.outputs.fw_version }}" + prerelease: ${{ needs.validate.outputs.is_prerelease }} + name: "Firmware ${{ needs.validate.outputs.tag_name }}" body_path: release-body.md files: release-assets/* fail_on_unmatched_files: true diff --git a/.gitignore b/.gitignore index 24e7efbbd..98b1e9ff2 100644 --- a/.gitignore +++ b/.gitignore @@ -3,3 +3,4 @@ build .DS_Store .vscode/ +build-btconly-check/ diff --git a/.gitleaks.toml b/.gitleaks.toml index 6d6bfbcb3..319adca94 100644 --- a/.gitleaks.toml +++ b/.gitleaks.toml @@ -1,10 +1,47 @@ -title = "KeepKey firmware gitleaks configuration" +title = "KeepKey firmware Gitleaks configuration" -# Start from the upstream ruleset and only narrow it, never widen it. [extend] useDefault = true -[allowlist] +[[allowlists]] +description = "Published AES test vectors in current and historical trezor-crypto layouts" +targetRules = ["generic-api-key"] +condition = "AND" +regexTarget = "line" +paths = [ + '''^deps/crypto/trezor-firmware/crypto/aes/aestst\.c$''', + '''^deps/crypto/trezor-crypto/aes/aestst\.c$''', +] +regexes = ['''(?i)^[[:space:]]*//[[:space:]]*key[[:space:]]*=[[:space:]]*[0-9a-f]+[[:space:]]*$'''] + +[[allowlists]] +description = "Ed25519 C type names in current and historical trezor-crypto layouts" +targetRules = ["generic-api-key"] +condition = "AND" +regexTarget = "line" +paths = [ + '''^deps/crypto/trezor-firmware/crypto/ed25519-donna/ed25519-(blake2b|keccak|sha3)\.h$''', + '''^deps/crypto/trezor-firmware/crypto/ed25519-donna/ed25519\.[ch]$''', + '''^deps/crypto/trezor-crypto/ed25519-donna/ed25519-(blake2b|keccak|sha3)\.h$''', + '''^deps/crypto/trezor-crypto/ed25519-donna/ed25519\.[ch]$''', +] +regexes = ['''ed25519.*secret_key.*signature'''] + +[[allowlists]] +description = "RC21 release provenance names an exact public python-keepkey git commit" +targetRules = ["generic-api-key"] +condition = "AND" +regexTarget = "line" +paths = ['''^docs/security/7\.15\.0-rc21-clearsign-release-control\.md$'''] +regexes = ['''^[[:space:]]*-[[:space:]]*python-keepkey:[[:space:]]*`c406a1ba9120da410c356dbff7f4d4bd1e1758fa`\.[[:space:]]*$'''] + +# Converted from the deprecated singular [allowlist] to a fourth [[allowlists]] +# entry. gitleaks refuses to load a config containing both forms ("[allowlist] is +# deprecated, it cannot be used alongside [[allowlists]]"), which is what the +# alpha<-develop merge produced: alpha carried three [[allowlists]] and develop +# carried this one [allowlist]. The merge kept both and the scan died at config +# load, taking the whole build graph with it. Content is unchanged. +[[allowlists]] description = """ Two U2F attestation artifacts are public by design and must not fail the scan. diff --git a/.gitmodules b/.gitmodules index 2d6c4446a..a228ea85c 100644 --- a/.gitmodules +++ b/.gitmodules @@ -1,10 +1,10 @@ [submodule "deps/device-protocol"] path = deps/device-protocol -url = https://github.com/keepkey/device-protocol.git -branch = master + url = https://github.com/keepkey/device-protocol.git +branch = up/release-protocol [submodule "deps/trezor-firmware"] path = deps/crypto/trezor-firmware -url = https://github.com/keepkey/trezor-firmware.git +url = https://github.com/BitHighlander/trezor-firmware.git [submodule "googletest"] path = deps/googletest url = https://github.com/google/googletest.git @@ -14,7 +14,7 @@ url = https://github.com/keepkey/code-signing-keys.git [submodule "deps/python-keepkey"] path = deps/python-keepkey url = https://github.com/keepkey/python-keepkey.git -branch = master +branch = reconcile/upstream-sync [submodule "deps/qrenc/QR-Code-generator"] path = deps/qrenc/QR-Code-generator url = https://github.com/keepkey/QR-Code-generator.git diff --git a/CMakeLists.txt b/CMakeLists.txt index 413dae82e..3ceee2a60 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -9,7 +9,7 @@ endif() project( KeepKeyFirmware - VERSION 7.14.3 + VERSION 7.15.0 LANGUAGES C CXX ASM) set(BOOTLOADER_MAJOR_VERSION 2) @@ -22,6 +22,16 @@ option(KK_DEBUG_LINK "Build with debug-link enabled" OFF) option(KK_BUILD_FUZZERS "Build the fuzzers?" OFF) option(KK_BITCOIN_ONLY "Build Bitcoin-only firmware (strip all non-BTC coins)" OFF) +# Zcash shielded/Orchard support is part of the regular firmware. It is an +# internal compile selection, not a third release variant: bitcoin-only strips +# the Zcash coin and privacy engine; every regular device/emulator build ships +# both. The open constant-time Pallas audit finding remains a release gate for +# RC18, but it must not silently change the product being audited. +if(KK_BITCOIN_ONLY) + set(KK_ZCASH_PRIVACY OFF) +else() + set(KK_ZCASH_PRIVACY ON) +endif() # When building the dylib, every static lib it links (kkfirmware, kkboard, # trezorcrypto, kkrand, kktransport, qrcodegenerator, SecAESSTM32, ...) must @@ -64,7 +74,7 @@ endif() if(NOT EXISTS ${CMAKE_SOURCE_DIR}/deps/crypto/trezor-firmware/crypto/Makefile) message( FATAL_ERROR - " trezor-crypto missing. Need to 'git submodule update --init --recursive" + "trezor-crypto fork missing. Run 'git submodule update --init deps/crypto/trezor-firmware'" ) endif() @@ -86,6 +96,13 @@ endif() if(${KK_EMULATOR}) add_definitions(-DEMULATOR) add_definitions(-DCONFIDENTIAL=) + # macOS/BSD declare strlcpy/strlcat in ; glibc (Linux) and MinGW + # (Windows) do not. Force-include the prototypes so the ~20 call sites build + # without -Werror=implicit-function-declaration (definitions come from + # lib/board/strlcpy.c + strlcat.c). Apple already has them in . + if(NOT APPLE) + add_compile_options(-include ${CMAKE_SOURCE_DIR}/include/keepkey/board/bsd_compat.h) + endif() else() add_definitions(-DCONFIDENTIAL=__attribute__\(\(section\("confidential"\)\)\)) endif() @@ -115,6 +132,13 @@ add_definitions(-DUSE_CARDANO=0) add_definitions(-DUSE_MONERO=0) add_definitions(-DUSE_NEM=0) +# NOT a USE_* style on/off toggle despite sitting next to them: trezor-crypto's +# rand.c tests this macro with #ifndef, so only its *definedness* matters. The +# old -D...=0 spelling read as "off" while actually meaning "on", and the +# insecure LCG random32() stayed out of the build purely by that double +# negation. Define it bare, matching upstream trezor-core's SConscript.firmware, +# so a future cleanup of an apparent "=0 means unused" define cannot silently +# compile in the LCG. lib/rand/rng.c #errors if this ever goes missing. # trezor-crypto's rand.c tests only whether this macro is defined. A value of # zero therefore did not disable anything; it excluded the library's insecure # test LCG by definedness. Use the upstream spelling so that intent is clear, @@ -137,22 +161,37 @@ add_definitions(-DBIP39_WORDLIST_PADDED=1) add_definitions(-DAES_128=1) +# NOTE: AES table size is selected per release product below. The regular +# image includes Zcash and its Pallas curve arithmetic, so it uses the smaller +# AES tables to preserve flash headroom. Bitcoin-only keeps FOUR_TABLES AES. + if(${KK_DEBUG_LINK}) add_definitions(-DDEBUG_LINK=1) else() add_definitions(-DDEBUG_LINK=0) endif() -# Always defined, 0 or 1, and always tested with `#if BITCOIN_ONLY`. Device -# builds compile with -Wundef -Werror, so an undefined identifier inside `#if` -# is a hard error rather than a silent zero -- which is what we want, because -# a silently-zero guard would ship the coin engines into the stripped image. +# Value macros: always defined, 0 or 1, and always tested with `#if FLAG`. +# Device builds compile with -Wundef -Werror, so an undefined identifier inside +# `#if` is a hard error rather than a silent zero -- which is what we want, +# because a silently-zero guard would ship the coin engines into the stripped +# image. if(${KK_BITCOIN_ONLY}) add_definitions(-DBITCOIN_ONLY=1) else() add_definitions(-DBITCOIN_ONLY=0) endif() +if(${KK_ZCASH_PRIVACY}) + add_definitions(-DZCASH_PRIVACY=1) + # The Orchard engine leaves the regular image tightest on flash; shrink the + # Gladman AES lookup tables from 4KB to 1KB each (-15,360 bytes ROM, + # slightly slower AES). Bitcoin-only keeps the fast FOUR_TABLES. + add_definitions(-DAES_SMALL_TABLES) +else() + add_definitions(-DZCASH_PRIVACY=0) +endif() + if("${CMAKE_BUILD_TYPE}" STREQUAL "Debug") add_definitions(-DDEBUG_ON) add_definitions(-DMEMORY_PROTECT=0) @@ -185,6 +224,12 @@ if(NOT ${KK_EMULATOR}) link_directories(${LIBOPENCM3_PATH}/lib) include_directories(${LIBOPENCM3_PATH}/include) + # Emit per-function stack-frame sizes (.su files) on device builds. CI's + # SRAM budget gate (tools/check_sram_budget.py) reports the largest frames + # and fails when the linker-asserted stack reserve minus the largest frame + # leaves less than the configured margin. See tools/firmware/keepkey.ld. + add_compile_options(-fstack-usage) + # Dummy empty libraries for stack smashing protection support, since we # implement __stack_chk_guard and __stack_chk_fail ourselves. file(WRITE ${CMAKE_BINARY_DIR}/ssp.c "") @@ -213,6 +258,10 @@ if(${KK_EMULATOR}) add_test(test-firmware ${CMAKE_BINARY_DIR}/bin/firmware-unit) add_test(test-board ${CMAKE_BINARY_DIR}/bin/board-unit) add_test(test-crypto ${CMAKE_BINARY_DIR}/bin/crypto-unit) + if(${KK_ZCASH_PRIVACY}) + add_test(test-pallas-ct ${CMAKE_BINARY_DIR}/bin/pallas-ct-unit) + add_test(test-zcash-crypto ${CMAKE_BINARY_DIR}/bin/zcash-crypto-unit) + endif() add_custom_target( xunit @@ -223,4 +272,10 @@ if(${KK_EMULATOR}) COMMAND ${CMAKE_BINARY_DIR}/bin/crypto-unit --gtest_output=xml:${CMAKE_BINARY_DIR}/unittests/crypto.xml) + if(${KK_ZCASH_PRIVACY}) + add_custom_command(TARGET xunit POST_BUILD + COMMAND ${CMAKE_BINARY_DIR}/bin/pallas-ct-unit + --gtest_output=xml:${CMAKE_BINARY_DIR}/unittests/pallas-ct.xml) + endif() + endif() diff --git a/Makefile b/Makefile new file mode 100644 index 000000000..a5a41ad99 --- /dev/null +++ b/Makefile @@ -0,0 +1,42 @@ +# Convenience targets — mirrors CI jobs so failures are caught locally. +# +# CI pins clang-format-20. Use that version if available, otherwise fall back. +# To install: brew install llvm@20 or apt-get install clang-format-20 +CLANG_FORMAT ?= $(shell command -v clang-format-20 2>/dev/null || echo clang-format) + +# Directories and exclusions must match .github/workflows/ci.yml lint-format job. +LINT_DIRS := include/keepkey lib/firmware lib/board lib/transport/src +LINT_SOURCES := $(shell find $(LINT_DIRS) -name '*.c' -o -name '*.h' 2>/dev/null \ + | grep -v generated | grep -v '\.pb\.') + +.PHONY: lint format help + +## lint: Check formatting (same rules as CI). Exits non-zero on any violation. +lint: + @echo "clang-format version: $$($(CLANG_FORMAT) --version)" + @FAILED=0; \ + for f in $(LINT_SOURCES); do \ + if ! $(CLANG_FORMAT) --style=file --dry-run --Werror "$$f" 2>/dev/null; then \ + echo " NEEDS FORMAT: $$f"; \ + FAILED=1; \ + fi; \ + done; \ + if [ "$$FAILED" = "1" ]; then \ + echo ""; \ + echo "Run 'make format' to fix all files."; \ + exit 1; \ + else \ + echo "All files pass clang-format check."; \ + fi + +## format: Auto-fix formatting in-place for all source files. +format: + @echo "Formatting $(LINT_DIRS)..." + @for f in $(LINT_SOURCES); do \ + $(CLANG_FORMAT) --style=file -i "$$f"; \ + done + @echo "Done. Review changes with: git diff" + +## help: List available targets. +help: + @grep -E '^## ' $(MAKEFILE_LIST) | sed 's/^## / make /' diff --git a/cmake/caches/device.cmake b/cmake/caches/device.cmake index 436a23bce..8265191dc 100644 --- a/cmake/caches/device.cmake +++ b/cmake/caches/device.cmake @@ -45,7 +45,14 @@ set(WARN_FLAGS -Werror") -set(KK_C_FLAGS "${ARCH_FLAGS} -std=gnu99 ${WARN_FLAGS}" CACHE STRING "") +# Newlib's snprintf unconditionally links the float engine (_svfprintf_r, +# _dtoa_r, soft-double libgcc, malloc) — ~22 KB of ROM with zero %f users in +# the firmware. Route all callers to the integer-only siprintf family instead. +# %lld/%llu still work (this toolchain's libc.a compiles the integer engine +# with long-long support). Device builds only; host/emulator keep libc printf. +set(PRINTF_FLAGS "-Dsnprintf=sniprintf -Dvsnprintf=vsniprintf") + +set(KK_C_FLAGS "${ARCH_FLAGS} -std=gnu99 ${WARN_FLAGS} ${PRINTF_FLAGS}" CACHE STRING "") set(KK_CXX_FLAGS "${ARCH_FLAGS} -std=gnu++11 ${WARN_FLAGS} \ -fno-exceptions \ -fno-rtti \ diff --git a/cmake/toolchains/mingw-w64-x86_64.cmake b/cmake/toolchains/mingw-w64-x86_64.cmake new file mode 100644 index 000000000..974f1a49c --- /dev/null +++ b/cmake/toolchains/mingw-w64-x86_64.cmake @@ -0,0 +1,40 @@ +# MinGW-w64 cross-compile toolchain for the Windows emulator DLL (libkkemu.dll, +# x86_64). Lets us cross-build the Windows DLL from the existing macOS/Linux +# emulator build host — no Windows runner required. +# +# Usage: +# cmake -DCMAKE_TOOLCHAIN_FILE=cmake/toolchains/mingw-w64-x86_64.cmake \ +# -DKK_EMULATOR=ON -DKK_BUILD_DYLIB=ON -DKK_DEBUG_LINK=ON ... +# cmake --build --target kkemulator_dylib +# +# Install MinGW: `brew install mingw-w64` (macOS) / `apt-get install mingw-w64`. +# +# Only the kkemulator_dylib target is meant to cross-compile. The standalone +# UDP `kkemu` binary is gated out on Windows (tools/emulator/CMakeLists.txt). + +set(CMAKE_SYSTEM_NAME Windows) +set(CMAKE_SYSTEM_PROCESSOR x86_64) + +set(TOOLCHAIN_PREFIX x86_64-w64-mingw32) +find_program(CMAKE_C_COMPILER NAMES ${TOOLCHAIN_PREFIX}-gcc) +find_program(CMAKE_CXX_COMPILER NAMES ${TOOLCHAIN_PREFIX}-g++) +find_program(CMAKE_RC_COMPILER NAMES ${TOOLCHAIN_PREFIX}-windres) + +if(NOT CMAKE_C_COMPILER) + message(FATAL_ERROR + "${TOOLCHAIN_PREFIX}-gcc not found. Install MinGW-w64 " + "(brew install mingw-w64 / apt-get install mingw-w64).") +endif() + +# Derive the target sysroot from the compiler location so this works across +# Homebrew versions and Linux package layouts. +get_filename_component(_kk_cc "${CMAKE_C_COMPILER}" REALPATH) +get_filename_component(_kk_bin "${_kk_cc}" DIRECTORY) +get_filename_component(_kk_root "${_kk_bin}/.." ABSOLUTE) +set(CMAKE_FIND_ROOT_PATH "${_kk_root}/${TOOLCHAIN_PREFIX}") + +# Find host programs on the host; libraries/headers in the target sysroot. +set(CMAKE_FIND_ROOT_PATH_MODE_PROGRAM NEVER) +set(CMAKE_FIND_ROOT_PATH_MODE_LIBRARY ONLY) +set(CMAKE_FIND_ROOT_PATH_MODE_INCLUDE ONLY) +set(CMAKE_FIND_ROOT_PATH_MODE_PACKAGE ONLY) diff --git a/deps/crypto/CMakeLists.txt b/deps/crypto/CMakeLists.txt index 2a9fbff97..4a1d9d880 100644 --- a/deps/crypto/CMakeLists.txt +++ b/deps/crypto/CMakeLists.txt @@ -58,6 +58,18 @@ set(sources #trezor-firmware/crypto/aes/aestst.c trezor-firmware/crypto/aes/aestab.c) +# Pallas/Orchard curve arithmetic (~2.4k LOC) -- only the Zcash shielded engine +# uses it. Excluded from the default and bitcoin-only images. +if(${KK_ZCASH_PRIVACY}) + list(APPEND sources + trezor-firmware/crypto/pallas.c + trezor-firmware/crypto/pallas_ct.c + trezor-firmware/crypto/pallas_sinsemilla.c + trezor-firmware/crypto/pallas_swu.c + trezor-firmware/crypto/redpallas.c + trezor-firmware/crypto/zcash_zip316.c) +endif() + # Clang 5.0 in the docker image (kktech/firmware:v7) is missing # , which breaks these. Until they're needed, we'll just elide # them. @@ -69,7 +81,6 @@ set(sources include_directories( ${CMAKE_CURRENT_SOURCE_DIR}/trezor-firmware/crypto - ${CMAKE_CURRENT_SOURCE_DIR}/trezor-firmware/ ${CMAKE_CURRENT_SOURCE_DIR}/trezor-firmware/crypto/ed25519-donna ${OPENSSL_INCLUDE_DIR}) diff --git a/deps/crypto/trezor-firmware b/deps/crypto/trezor-firmware index cdc05bebe..8a392f70a 160000 --- a/deps/crypto/trezor-firmware +++ b/deps/crypto/trezor-firmware @@ -1 +1 @@ -Subproject commit cdc05bebe9e6989cf711e1b5bea6324fd09f848e +Subproject commit 8a392f70a5d5575ece3dfb35f115d4a4b27f497c diff --git a/deps/python-keepkey b/deps/python-keepkey index 758f20c2c..c697a2511 160000 --- a/deps/python-keepkey +++ b/deps/python-keepkey @@ -1 +1 @@ -Subproject commit 758f20c2c2288fe30cbf192f927fc966de44adc1 +Subproject commit c697a25115ea859ab5b0a89f77dd2c77e61ab889 diff --git a/docs/Build.md b/docs/Build.md index 5ffdbe604..c5f120283 100644 --- a/docs/Build.md +++ b/docs/Build.md @@ -36,3 +36,20 @@ Running the tests $ cd build $ make all test ``` + +Release products +----------------- + +Two release products, no separate `zcash-privacy` artifact: + +| Product | Contents | +| --- | --- | +| Regular (`full`) | Every supported chain, including Zcash shielded/Orchard | +| Bitcoin-only | Bitcoin only; all non-Bitcoin coins and Zcash privacy code removed | + +An unflagged CMake build is the regular product (`BITCOIN_ONLY=0`, +`ZCASH_PRIVACY=1`). `-DKK_BITCOIN_ONLY=ON` sets `BITCOIN_ONLY=1` and +`ZCASH_PRIVACY=0`. Zcash privacy is part of the regular firmware and cannot be +disabled as a release choice; the internal `ZCASH_PRIVACY` value exists only so +bitcoin-only can compile the privacy sources out. Device, emulator, unit-test, +SRAM, and tagged-release CI matrices cover only these two products. diff --git a/docs/DiceEntropy.md b/docs/DiceEntropy.md index c3394591b..4c52b337c 100644 --- a/docs/DiceEntropy.md +++ b/docs/DiceEntropy.md @@ -1,14 +1,7 @@ # Dice Entropy On-device dice rolls, folded into the seed at creation time. Available from -firmware v7.14.3 (bitcoin-only line) and v7.15.0 (`ResetDevice.dice_entropy`). - -One difference from 7.15 in this line: the legacy `display_random` entropy -screen still exists here, because already-shipped 7.14 hosts request it. The -two are mutually exclusive — `ResetDevice` with both `display_random` and -`dice_entropy` set is refused with a SyntaxError, since the screen shows the -POST-mix internal entropy and honoring both would hand a host the seed -pre-image and make the dice fold-in worthless. +firmware v7.15.0 (`ResetDevice.dice_entropy`). ## What happens @@ -53,8 +46,7 @@ described it as a verifiable commitment; that was strictly worse. A host that supplies `ext_entropy` and reads that screen once computes `SHA256(shown || ext_entropy)` — the seed pre-image. Dice change nothing about that attack, because the displayed value is already post-mix. Unverifiable -mixing beats a verifiable seed pre-image. See the comment above the -`dice_entropy` block in `reset.c:reset_init()`. +mixing beats a verifiable seed pre-image. See the comment at `reset.c:136`. The roll digest is safe by contrast because it hashes the user's own input, not seed material. diff --git a/docs/README.md b/docs/README.md index e456f6ad5..ccdbe8ac7 100644 --- a/docs/README.md +++ b/docs/README.md @@ -2,6 +2,8 @@ * [How to Build](Build.md) * [Storage Layout](Storage.md) +* [Storage Version Gate — audit SOP](StorageVersionGate.md) * [Supported Coins](Coins.md) * [Host Communications](Host.md) * [Release Process](Release.md) +* [Dice Entropy](DiceEntropy.md) diff --git a/docs/Release.md b/docs/Release.md index dc753f30b..9eb498ab3 100644 --- a/docs/Release.md +++ b/docs/Release.md @@ -11,6 +11,12 @@ Release Process 1. We stay in compliance with the GPL license. 1. Build a release build of the firmware on multiple different machines, and compare firmware hashes. 1. Sign it on the airgapped machine with 3/5 signers. +1. Verify the signatures before publishing: `scripts/release/verify-signatures.py `. + * `hash-manifest.sh --require-signed` is structural only — a signature region holding one non-zero byte passes it. This does the real 3-of-5 ECDSA check the device does. + * It parses the keys from `include/keepkey/board/pubkeys.h`, so a rotation cannot leave it checking a stale set. 1. Double check that storage upgrade preserves keys on a production device. + * A signed upgrade must NEVER wipe; a downgrade wiping is correct and expected. + * If `STORAGE_VERSION` was bumped, set `STORAGE_VERSION_LAST_SHIPPED` to match in this release commit. See [Storage Version Gate](StorageVersionGate.md). + * Unsigned RC/dev builds cannot prove this: the bootloader wipes storage for unsigned images by design. Use a signed build. 1. Upload the signed firmware to github. 1. Publish release notes on github. diff --git a/docs/StorageVersionGate.md b/docs/StorageVersionGate.md new file mode 100644 index 000000000..ffa3dc750 --- /dev/null +++ b/docs/StorageVersionGate.md @@ -0,0 +1,107 @@ +# Storage version gate — audit SOP + +**A signed upgrade must never wipe. A downgrade wipes, and that is correct.** + +Those two sentences are the whole policy. Everything below exists to make the +first one impossible to break by accident. + +## Why an upgrade can wipe + +`storage_init()` calls `storage_fromFlash()` on whatever blob is in flash. If +`version_from_int()` does not recognise the version it returns +`StorageVersion_NONE`, `storage_fromFlash()` returns `SUS_Invalid`, and +`storage_init()` runs `storage_reset()` + `storage_commit()`. No prompt, no +warning — the wallet is gone at boot. + +An upgrading device always arrives carrying a blob written by the release it is +leaving. So incoming firmware must recognise every version any shipped firmware +ever wrote. There are exactly two ways to break that: + +1. **Lower `STORAGE_VERSION`** below a version that has shipped. +2. **Remove, reorder, or skip an entry** in `storage_versions.inc`, so a version + that used to be recognised no longer is. + +Both compile cleanly without the gate. Both silently wipe every field device on +upgrade. Neither shows up in any functional test, because tests create storage +with the firmware under test and never cross a release boundary. + +The reverse direction is not a defect: older firmware cannot read a newer blob, +so a **downgrade** legitimately lands on `SUS_Invalid` and resets. Do not +"fix" that. Do not add a compatibility shim for it. Downgrades wipe. + +Signing is a separate wipe path with its own rule — see below. + +## The hard checks + +Two `_Static_assert`s in `lib/firmware/storage.c`, both compile-time: + +| Check | Fires when | +|---|---| +| `STORAGE_VERSION >= STORAGE_VERSION_LAST_SHIPPED` | Someone lowers the version below a shipped release | +| `StorageVersion_##N == N`, on every entry | `storage_versions.inc` stops being contiguous from 1 | + +The second works because the enum is emitted in `.inc` order after +`StorageVersion_NONE = 0`, so a contiguous `1..N` list makes +`StorageVersion_N == N`. Delete entry 5 and `StorageVersion_17` becomes 16, and +the build stops. + +It is asserted on **every** entry, not just the last. Checking only the last +entry pins the entry *count*, which is weaker than it looks: renumbering +`ENTRY(16)` to `ENTRY(99)` leaves `StorageVersion_17` at 17 and compiles clean, +while `version_from_int` quietly loses `case 16` and every device carrying +version 16 is wiped on upgrade. That gap was found by mutation-testing the +assert rather than by reading it. + +`STORAGE_VERSION_LAST_SHIPPED` lives in `include/keepkey/firmware/storage.h`. + +## Auditing a change that touches storage + +1. **Did `STORAGE_VERSION` change?** If it went up, the release checklist below + applies. If it went **down**, stop — this wipes every upgrading device. There + is no valid reason to lower it on a branch; a revert of unshipped work should + restore the *reader* while leaving the version number alone. +2. **Did `storage_versions.inc` change?** Only ever by appending. Any deletion or + renumbering is a wipe, and the build will say so. +3. **Did `STORAGE_VERSION_LAST_SHIPPED` change?** Only legitimate in a release + commit, and only upward. A bump appearing in a feature branch, or any + decrease, is the single highest-severity review item in this file — it is + precisely the edit that disarms the gate to make a build compile. +4. **Is there a new `case` in `storage_fromFlash` for the new version, with the + fallthrough chain intact from the oldest version forward?** The chain is how + an old blob is migrated step by step; a missing link loads garbage rather + than wiping, which is worse. +5. **Run the reboot regression.** Create/reset, set PIN, serialize, reload as + after reboot, unlock, compare the recovered key. The ordinary storage tests + never cross the serialize/reboot boundary, and that boundary is where this + class of defect lives. + +## Release checklist addition + +When bumping `STORAGE_VERSION` for a release: + +- Add the new entry to `storage_versions.inc` — append only. +- Add the `case` and fallthrough in `storage_fromFlash`. +- Set `STORAGE_VERSION_LAST_SHIPPED` to the new value **in the release commit**, + not before. Until the build actually ships, the previous value is the truth. +- Verify on a production device that upgrade preserves keys. `docs/Release.md` + has always said this; the static asserts do not replace it, they only stop the + two failure modes that reach a device unnoticed. + +## The other wipe path: signatures + +Independent of versions, the bootloader erases storage unless +`should_restore()` (`tools/bootloader/usb_flash.c`) is satisfied. It requires +all three: + +- `SIG_FLAG != 0` — the incoming image's metadata does not request a wipe +- `!old_firmware_was_unsigned` — the firmware being replaced was officially signed +- `signatures_ok() == SIG_OK` on the new image + +Signed → signed preserves storage. Anything involving an unsigned image on +either side wipes, deliberately: it stops custom firmware from dumping storage +sectors written by official firmware. + +Consequence for testing: an unsigned development or RC build **cannot** validate +the preserve path, because it fails the second and third conditions by +construction. "Upgrade did not wipe" is only ever proven with a signed build on +a production device. diff --git a/docs/coin-integration/README.md b/docs/coin-integration/README.md new file mode 100644 index 000000000..5110255dc --- /dev/null +++ b/docs/coin-integration/README.md @@ -0,0 +1,305 @@ +# Coin Integration Guide for KeepKey Firmware + +This guide explains the submodule-first workflow required when adding new coin +support to the firmware. If you skip or reorder these steps, CI will fail +because the firmware's submodule commits won't resolve. + +## Repository Map + +The firmware repo has 7 submodules. Three matter for coin integration: + +| Submodule | Path | Upstream URL | Fork URL | Purpose | +|-----------|------|-------------|----------|---------| +| device-protocol | `deps/device-protocol` | `keepkey/device-protocol` | `BitHighlander/device-protocol` | Protobuf message definitions | +| python-keepkey | `deps/python-keepkey` | `keepkey/python-keepkey` | `BitHighlander/python-keepkey` | Python client + integration tests | +| trezor-firmware | `deps/crypto/trezor-firmware` | `keepkey/trezor-firmware` | (keepkey org) | Crypto primitives (curves, hashing) | + +**Critical**: `.gitmodules` points to upstream `keepkey/*` URLs. For development +branches with commits that don't exist upstream yet, you must temporarily change +the URLs to point to your fork. + +## The Submodule-First Workflow + +### Why Order Matters + +When CI clones the firmware, it runs `git submodule update --init`. The submodule +commits referenced in the firmware tree must exist in the repos that `.gitmodules` +URLs point to. If you add a new proto file to device-protocol and update the +firmware's submodule pointer, but that commit only exists on your fork and +`.gitmodules` still points to upstream — CI fails. + +### Dependency Order + +``` +1. device-protocol fork → proto messages +2. trezor-firmware fork → crypto primitives (if new curve/algo needed) +3. python-keepkey fork → client methods + wire ID registration + tests +4. firmware → FSM handlers, derivation, signing + unit tests +``` + +Each step must be pushed to the correct fork and the commit hash noted before +the next step can reference it. + +### Step-by-Step + +#### Phase 0: Verify Fork Remotes + +For each submodule that needs changes, ensure the fork remote exists: + +```bash +cd modules/keepkey-firmware + +# device-protocol: check if your fork remote exists +cd deps/device-protocol +git remote -v +# If only 'origin' pointing to keepkey/, add your fork: +git remote add fork https://github.com/BitHighlander/device-protocol.git +git fetch fork +cd ../.. + +# python-keepkey: same pattern +cd deps/python-keepkey +git remote add fork https://github.com/BitHighlander/python-keepkey.git +git fetch fork +cd ../.. + +# trezor-firmware (crypto): usually keepkey org has push access +cd deps/crypto/trezor-firmware +git remote -v +cd ../../.. +``` + +#### Phase 1: Device Protocol (Proto Messages) + +```bash +cd deps/device-protocol + +# Create branch from upstream master +git fetch origin +git checkout -b feature/-proto origin/master + +# Add your proto file +# Example: messages-zcash.proto, messages-solana.proto, etc. +# Also update messages.proto with new MessageType enum values + +git add messages-.proto +git commit -m "feat: add protocol messages (IDs XXXX-XXXX)" + +# Push to YOUR FORK (not upstream!) +git push fork feature/-proto + +# Note the commit hash — you'll need it for firmware +git rev-parse HEAD +# → abc1234... +``` + +**Wire ID conventions**: Check existing ranges in `messages.proto` to avoid collisions. +Current allocations: +- Zcash: 1300-1307 +- Tron: 1400-1403 +- TON: 1500-1503 +- Solana: 1200-1205 + +#### Phase 2: Crypto Primitives (If Needed) + +Only needed if the coin requires a new elliptic curve or hash function not +already in trezor-firmware/crypto. + +```bash +cd deps/crypto/trezor-firmware + +# Check current state +git status + +# Create branch from current HEAD +git checkout -b feature/-crypto + +# Add new crypto files +git add crypto/.c crypto/.h +git commit -m "feat: add primitives for " + +# Push (usually to keepkey/trezor-firmware directly) +git push origin feature/-crypto +``` + +**Examples of coin-specific crypto**: +- Zcash: `pallas.c/h`, `redpallas.c/h` (Pallas curve + RedPallas signatures) +- Solana: `ed25519` (already in repo) + +#### Phase 3: Python-KeepKey (Client + Tests) + +```bash +cd deps/python-keepkey + +git fetch origin +git checkout -b feature/-tests origin/master + +# Add/update: +# 1. keepkeylib/messages__pb2.py — protobuf bindings (or generate from proto) +# 2. keepkeylib/client.py — add client method(s) +# 3. keepkeylib/mapping.py — register wire IDs +# 4. tests/test_msg_.py — integration tests + +git add -A +git commit -m "feat: add client methods and tests" + +# Push to YOUR FORK +git push fork feature/-tests +``` + +**Important**: python-keepkey uses hand-written `_pb2.py` files for protobuf 3.x +compatibility. Do NOT regenerate all proto bindings — only add the new coin's +`_pb2.py` and register wire IDs in `mapping.py`. + +#### Phase 4: Firmware (Core Implementation) + +Now create the firmware branch with updated submodule pointers: + +```bash +cd modules/keepkey-firmware + +# Branch from develop +git checkout -b feature/ origin/develop + +# UPDATE .gitmodules TO POINT TO YOUR FORKS +# This is the critical step most people miss! +git config -f .gitmodules submodule.deps/device-protocol.url \ + https://github.com/BitHighlander/device-protocol.git +git config -f .gitmodules submodule.deps/python-keepkey.url \ + https://github.com/BitHighlander/python-keepkey.git + +# Update submodule pointers to your fork branch commits +cd deps/device-protocol +git fetch fork +git checkout +cd ../.. + +cd deps/python-keepkey +git fetch fork +git checkout +cd ../.. + +# If crypto was changed: +cd deps/crypto/trezor-firmware +git checkout +cd ../../.. + +# Stage submodule pointer updates + .gitmodules +git add .gitmodules deps/device-protocol deps/python-keepkey deps/crypto/trezor-firmware + +# Now add firmware code: +# - include/keepkey/firmware/.h +# - lib/firmware/.c +# - lib/firmware/fsm_msg_.h +# - include/keepkey/transport/messages-.options +# - lib/firmware/messagemap.def (add message registrations) +# - lib/firmware/fsm.c (add #include and declarations) +# - lib/firmware/CMakeLists.txt (add source files) +# - lib/transport/CMakeLists.txt (add proto build steps) +# - unittests/firmware/.cpp (add unit tests) +# - unittests/firmware/CMakeLists.txt (register test file) + +git add -A +git commit -m "feat: add support" +git push origin feature/ +``` + +#### Phase 5: Before Merging Upstream + +When your firmware PR is ready to merge into the main `keepkey/keepkey-firmware`: + +1. First merge device-protocol changes into upstream `keepkey/device-protocol` +2. First merge python-keepkey changes into upstream `keepkey/python-keepkey` +3. First merge crypto changes into upstream `keepkey/trezor-firmware` +4. **Then** update `.gitmodules` URLs back to `keepkey/*` upstream +5. Update submodule pointers to the upstream merge commits +6. Push the firmware PR + +## CI Pipeline + +The firmware uses CircleCI with docker-compose: + +- **emulator-build-test**: Builds emulator, runs `firmware-unit` and `python-keepkey` tests +- Tests must produce a status file with "0" to pass +- Both unit tests (GoogleTest C++) and integration tests (Python) run + +### Making CI Pass + +For each PR, ensure: +1. All submodule URLs in `.gitmodules` resolve (fork URLs during development) +2. All submodule commits exist in the repos the URLs point to +3. Firmware builds clean with `cmake` + `make` +4. `firmware-unit` tests pass (GoogleTest, `unittests/firmware/`) +5. `python-keepkey` tests pass (pytest, `deps/python-keepkey/tests/`) + +## Common Mistakes + +### 1. Forgetting to update .gitmodules URLs +**Symptom**: CI fails with "Could not find remote branch" or "reference is not a tree" +**Fix**: Change `.gitmodules` URLs to point to your fork before pushing + +### 2. Pushing submodule changes to upstream instead of fork +**Symptom**: Unauthorized push failure, or accidentally landing unreviewed proto changes +**Fix**: Always add your fork as a separate remote named `fork`, push there + +### 3. Not initializing the device-protocol submodule +**Symptom**: `deps/device-protocol` shows as `-` prefix in `git submodule status` +**Fix**: `git submodule init deps/device-protocol && git submodule update deps/device-protocol` + +### 4. Regenerating all python-keepkey proto bindings +**Symptom**: Massive diff touching files you didn't mean to change +**Fix**: Only add the new coin's `_pb2.py` file and register wire IDs in `mapping.py` + +### 5. Working in detached HEAD without realizing it +**Symptom**: Commits exist locally but can't be pushed, "branch not found" +**Fix**: Always create a named branch before committing in submodules + +## File Layout for New Coin + +``` +keepkey-firmware/ +├── deps/ +│ ├── device-protocol/ +│ │ └── messages-.proto ← Proto messages +│ ├── crypto/trezor-firmware/ +│ │ └── crypto/.{c,h} ← Crypto primitives (if needed) +│ └── python-keepkey/ +│ ├── keepkeylib/messages__pb2.py +│ ├── keepkeylib/client.py ← Add client method(s) +│ ├── keepkeylib/mapping.py ← Register wire IDs +│ └── tests/test_msg_.py ← Integration tests +├── include/keepkey/ +│ ├── firmware/.h ← Public API +│ └── transport/messages-.options ← Nanopb options +├── lib/firmware/ +│ ├── .c ← Core implementation +│ ├── fsm_msg_.h ← FSM message handlers +│ ├── fsm.c ← #include + declarations +│ ├── messagemap.def ← Message registrations +│ └── CMakeLists.txt ← Add source file +├── lib/transport/ +│ └── CMakeLists.txt ← Add proto build steps +└── unittests/firmware/ + ├── .cpp ← Unit tests (GoogleTest) + └── CMakeLists.txt ← Register test file +``` + +## Existing Coin References + +When implementing a new coin, study these existing implementations: + +| Coin | Proto | Firmware | Tests | Complexity | +|------|-------|----------|-------|------------| +| Mayachain | messages-mayachain.proto | mayachain.c | mayachain.cpp | Simple (address + sign) | +| Cosmos | messages-cosmos.proto | cosmos.c | cosmos.cpp | Moderate (amino encoding) | +| Ethereum | messages-ethereum.proto | ethereum.c | ethereum.cpp | Complex (EIP-155, tokens) | +| Zcash | messages-zcash.proto | zcash.c | (in progress) | Complex (ZIP-32, Orchard, RedPallas) | + +## Branch Naming Conventions + +| Repo | Branch Pattern | Example | +|------|---------------|---------| +| device-protocol | `feature/-proto` | `feature/zcash-proto` | +| trezor-firmware | `feature/-crypto` | `feature/zcash-crypto` | +| python-keepkey | `feature/-tests` | `feature/zcash-orchard-tests` | +| keepkey-firmware | `feature/` | `feature/zcash` | diff --git a/docs/coin-integration/zcash-on-device-ua.md b/docs/coin-integration/zcash-on-device-ua.md new file mode 100644 index 000000000..12cf02c46 --- /dev/null +++ b/docs/coin-integration/zcash-on-device-ua.md @@ -0,0 +1,425 @@ +# Zcash on-device unified address derivation (Phase 2) + +**Status:** design — not yet implemented. +**Owner:** firmware (this repo) +**Companions:** `hdwallet`, `keepkey-vault`, `device-protocol`. + +## 1. Why this exists + +The flow shipped in PR #142 / `feature-zcash` (PR #220) verifies that a host-supplied +`(ak, nk, rivk)` matches the device's seed-derived FVK, then displays the +**host-supplied** `u1...` string on the OLED. That is **not** an attestation +that the displayed unified address is spendable by the device. A malicious +host that knows the correct FVK can submit any UA string and the device will +faithfully render it. + +The only way the device can promise "this address is spendable by my seed at +this account" is to **derive the unified address itself** from material it +controls (the seed-rooted FVK + a diversifier index it accepts as input) and +display the device-derived bytes. The user then compares the device-shown +`u1...` to the wallet's claim before publishing. + +Half measures (FVK match only, fingerprint binding, "trust the wallet to +display the same UA we sent") are not interchangeable with on-device +derivation. They catch a different, smaller set of attacks. This document +specifies what's actually required to make the strong claim. + +## 2. Threat model + +The device is asked to attest: + +> The unified address rendered on the OLED is a valid encoding of an Orchard +> receiver `(d_j, pk_d_j)` where `pk_d_j = [ivk] · g_d_j`, `g_d_j` is the +> Pallas group element derived from a diversifier `d_j = FF1.Decrypt(dk, j)`, +> and `(ak, nk, rivk, dk)` is the FVK derived from the device's seed at the +> requested ZIP-32 Orchard account. + +Adversaries: + +1. **Compromised host process** — bun, hdwallet, sidecar, browser, USB driver. May fabricate or substitute UAs, FVKs, indexes, addresses. Cannot tamper with the device's OLED. +2. **Compromised wallet UI** — same observation surface as the host. User's only honest read is the OLED + the wallet's claim, side by side. +3. **Bit-flip / glitch** — out of scope for this document; addressed by the existing fault-injection hardening. + +Properties we want against (1) and (2): + +- The bytes rendered on the OLED must be a function of `(seed, account, j)` and nothing else. No host input flows into the displayed string. +- A user who reads the OLED and sees the same string in their wallet has cryptographic assurance that the UA's Orchard receiver is spendable by the device. + +Properties we explicitly do **not** claim: + +- Anything about non-Orchard receivers (transparent, Sapling) bundled into a multi-receiver UA. A UA can carry arbitrary other receivers; only the Orchard one is bound to this device. Display copy must scope the claim accordingly. +- Privacy / linkability of the displayed UA. ZIP-32 §6.1 fingerprinting is a separate (already-shipped) concern. + +## 3. Why this is large + +KeepKey firmware is C on Cortex-M3. Embedded Rust is not in this repo's +toolchain (Trezor Model T and Keystone3 both run Rust embedded; we don't). +Every primitive needed for Orchard UA derivation has to be implemented or +ported in C. The crates `orchard`, `pasta_curves`, `sinsemilla`, `f4jumble`, +`fpe`, and `zcash_keys` are the reference implementations; they don't run +here. + +What we already have, verified against the active Trezor firmware crypto +submodule at `deps/crypto/trezor-firmware` commit `376c64bcf`: + +- BLAKE2b (`deps/crypto/trezor-firmware/crypto/blake2b.{c,h}`) +- AES block cipher (`deps/crypto/trezor-firmware/crypto/aes/`) +- Pallas curve arithmetic (`pallas.{c,h}`) — point ops, scalar mult, modular ops +- RedPallas signatures (`redpallas.{c,h}`) +- A hard-coded RedPallas SpendAuth basepoint, useful for `ak = [ask]G_spendauth` + but not a general `GroupHash^Pallas` implementation +- Generic Bech32/Bech32m checksum support in + `deps/crypto/trezor-firmware/crypto/segwit_addr.{c,h}`. This still needs a + ZIP-316-specific wrapper and may need its BIP-173 90-character output guard + relaxed for UA strings. + +What we **don't** have and must build or adapt: + +| Primitive | Source / spec | Approx C LOC | Risk | +|---|---|---|---| +| FF1-AES256 (NIST SP 800-38G) for diversifier derivation | NIST SP 800-38G; `fpe` Rust crate | ~300 | Medium — AES exists, but FF1/FPE does not | +| `expand_message_xmd_blake2b` | RFC 9380 §5.3.1 | ~80 | Low | +| Pallas Simplified SWU map (`map_to_curve_simple_swu`) + isogeny | Pasta paper / Halo 2 reference | ~500 | High — algebraic, easy to get wrong, needs cross-vectors | +| `GroupHash^Pallas` + `DiversifyHash^Orchard` | ZIP-32 §5.4.2.1 / Orchard book §5.4 | ~80 | Low (composition of above) | +| Sinsemilla hash + commitment | Halo 2 spec §5.4.1.9; Orchard book §5.4 | ~500 | High — chunked commitment, easy off-by-one | +| `Commit^ivk` (specifically `ivk = SinsemillaShortCommit("z.cash:Orchard-CommitIvk", ak ‖ nk; rivk)`) | Orchard book §5.4 | ~100 | Med | +| F4Jumble (4-round Feistel-like permutation) | ZIP-316 §4.2 | ~150 | Low — well-specified | +| ZIP-316 Bech32m adapter | BIP-350 / ZIP-316 | ~50 | Low — checksum exists, but UA sizing/padding glue does not | +| ZIP-316 UA encoding (single-receiver, Orchard) | ZIP-316 §4 | ~150 | Low | +| Cross-language test harness | — | ~500 | — | +| **Subtotal — production primitives** | | **~2,500 LOC** | | + +This is conservative and excludes the FSM handler, layout code, proto changes, +and unit tests for everything (probably another 1,500 LOC). + +First-pass feasibility conclusion: the Trezor firmware dependency gives us the +generic base layer (BLAKE2b, AES, Pallas point/scalar arithmetic, RedPallas, and +Bech32m), but it does not give us the Orchard unified-address derivation stack. +There is no checked-in Trezor C implementation of FF1, Sinsemilla, +`expand_message_xmd_blake2b`, Pallas SWU/isogeny, Orchard `GroupHash`, F4Jumble, +or ZIP-316 UA assembly. Phase 2 therefore remains a real crypto port, not just +wiring existing Trezor libraries together. + +### 3.1. Online library survey + +Surveyed 2026-04-30. The pattern is clear: only FF1 has a plausible C source +to adapt. The rest should be treated as spec-driven C implementations with +Rust upstreams used for goldens. + +| Needed primitive | Best upstream candidate | Language / license | Fit for KeepKey firmware | Estimated firmware LOC | Effect on plan | +|---|---|---|---|---:|---| +| FF1-AES256 for `d_j = FF1.Decrypt(dk, j)` | [`0NG/Format-Preserving-Encryption`](https://github.com/0NG/Format-Preserving-Encryption) | C / MIT | Useful algorithm reference, not drop-in. It depends on OpenSSL BIGNUM/AES; firmware should replace this with existing Trezor AES and a fixed-size radix-256 path for the 11-byte diversifier. | 300-500 | Reduces spec ambiguity, but does not avoid a port. Still needs NIST vectors and Orchard vectors. | +| FF1 specification and test source | [NIST SP 800-38G Rev. 1 draft](https://csrc.nist.gov/pubs/sp/800/38/g/r1/2pd) | Spec | Normative source. NIST's 2025 draft keeps FF1, removes FF3, and disallows floating point. | 0 production / 50-100 tests | Hard requirement for validation. Also tells us not to port code that uses floating-point `log2` or general decimal FPE assumptions blindly. | +| `expand_message_xmd_blake2b` | [RFC 9380](https://www.rfc-editor.org/rfc/rfc9380.html) | Spec | Directly implement against existing BLAKE2b. No C library needed. | 80-120 | Low-risk self-contained helper. | +| Pallas Simplified SWU + isogeny | [`zcash/pasta_curves`](https://github.com/zcash/pasta_curves) / [docs.rs source](https://docs.rs/pasta_curves/latest/src/pasta_curves/pallas.rs.html) | Rust / MIT or Apache-2.0 | Authoritative reference and test-vector source only. No usable C port found. Existing KeepKey `pallas.{c,h}` gives field and point ops, but not SWU/isogeny. | 500-800 | Highest algebraic risk. Rust vectors are mandatory before using in address derivation. | +| Orchard `GroupHash^Pallas` / `DiversifyHash` | [`zcash/orchard`](https://github.com/zcash/orchard) plus `pasta_curves` | Rust / MIT or Apache-2.0 | Composition layer over `expand_message_xmd_blake2b` and SWU/isogeny. No standalone C implementation found. | 80-150 | Small LOC, but correctness depends entirely on SWU/isogeny. | +| Sinsemilla hash and commitment | [`zcash/sinsemilla`](https://github.com/zcash/sinsemilla) and Halo 2 Sinsemilla docs | Rust / MIT or Apache-2.0 | Reference only. No C implementation found. Needs fixed generators/constants and chunking rules ported carefully. | 700-1,200 | Main risk after SWU. Larger than the first estimate if constants/tables are checked in rather than generated. | +| `Commit^ivk` / Orchard IVK | [`zcash/orchard`](https://github.com/zcash/orchard) | Rust / MIT or Apache-2.0 | Thin wrapper over Sinsemilla plus scalar handling. Not useful until Sinsemilla exists. | 100-180 | Medium risk; mostly test-vector coverage. | +| F4Jumble | [`f4jumble` crate](https://docs.rs/f4jumble) / [ZIP-316](https://zips.z.cash/zip-0316) | Rust / MIT or Apache-2.0, spec MIT | Implement directly from ZIP-316 using existing BLAKE2b. No C library found; Rust crate is good for goldens. | 150-250 | Low-risk. Does not block crypto receiver derivation; needed for final `u1...` string. | +| Bech32m for ZIP-316 strings | Existing Trezor `segwit_addr.{c,h}`; optional reference [`whitslack/libbech32`](https://github.com/whitslack/libbech32) | C / existing vendored license; libbech32 MIT-style | We already have Bech32m checksum support. Need ZIP-316 use that ignores BIP-173's 90-character cap, per ZIP-316. `libbech32` is useful only if we want a comparison implementation. | 50-120 | Reduces previous estimate: no new checksum implementation, only wrapper/length-policy work. | +| ZIP-316 UA item assembly | [`zcash_address::unified`](https://docs.rs/zcash_address/latest/zcash_address/unified/index.html) / [ZIP-316](https://zips.z.cash/zip-0316) | Rust / MIT or Apache-2.0, spec MIT | Reference only. Implement compactSize item encoding for a single Orchard receiver, padding, F4Jumble, Bech32m. | 150-250 | Low-to-medium risk; mostly parser/encoding edge cases and exact HRP behavior. | + +Net effect: expected new production C stays roughly **2,100-3,500 LOC** before +FSM/UI/proto/test glue. The best-case reduction from online libraries is mostly +around Bech32m and FF1; there is no library discovery that changes the hard +parts: SWU/isogeny and Sinsemilla still need first-party C ports verified +byte-for-byte against upstream Rust. + +## 4. Cryptographic recipe (the actual algorithm) + +References: + +- Zcash Protocol Spec (NU6) §4.2.3, §5.4.1.6, §5.4.8.5 +- ZIP-32 §5.4 (Orchard key derivation) +- ZIP-316 (Unified Addresses) +- Orchard book — `https://zcash.github.io/orchard/` +- Pasta paper — `https://github.com/zcash/pasta_curves` + +Given device seed `S`, account index `a`: + +``` +# Already implemented in zcash.c +sk = ZIP-32 Orchard derivation (S, a) +ask = ToScalar(PRF^expand(sk, [0x06])) +nk = ToBase (PRF^expand(sk, [0x07])) +rivk = ToScalar(PRF^expand(sk, [0x08])) +ak = [ask]·G_spendauth # already serialised by current code + +# NEW — must be implemented +dk = PRF^expand(sk, [0x09])[0..32] # diversifier key (uses existing PRF^expand) +ivk = SinsemillaShortCommit( + "z.cash:Orchard-CommitIvk", + I2LEBSP_l_ivk(ak) ‖ I2LEBSP_l_ivk(nk), + rivk + ) + +# Per-address (single diversifier index j; default j = 0) +d_j = FF1-AES256.Decrypt(dk, /* tweak = */ "", I2LEBSP_88(j)) # 11 bytes +g_d_j = DiversifyHash^Orchard(d_j) = GroupHash^Pallas("z.cash:Orchard-gd", d_j) +pk_d_j = [ivk] · g_d_j # uses existing Pallas scalar mult + +raw_addr = bytes(d_j) ‖ bytes(pk_d_j) # 11 + 32 = 43 bytes + +# UA encoding (single receiver, Orchard) +receiver = uint8(0x03) ‖ uint8(43) ‖ raw_addr # ZIP-316 typecode 0x03 = Orchard +hrp = "u" +encoded = bech32m(hrp, F4Jumble(receiver ‖ padding_for_hrp("u"))) +``` + +The displayed string is `encoded`. Every byte that goes into it is a +deterministic function of `(S, a, j)` — the host contributes only the choice +of `j`. + +## 5. API design + +### 5.1 New proto messages (`device-protocol`) + +```proto +// Request: device derives the canonical Orchard UA for (account, j) from its +// own seed and renders it on the OLED for user confirmation. The host does +// NOT supply any address bytes. +// +// @next ZcashUnifiedAddress +// @next Failure +message ZcashGetUnifiedAddress { + repeated uint32 address_n = 1; // m/32'/133'/account' + optional uint32 account = 2; // alternative to address_n + optional uint64 diversifier_index = 3; // 88-bit; default 0 + optional bool show_display = 4; // when true, render OLED + require confirm + optional bytes expected_seed_fingerprint = 5; // optional ZIP-32 §6.1 binding +} + +// Response. +// +// @prev ZcashGetUnifiedAddress +message ZcashUnifiedAddress { + optional string address = 1; // device-derived "u1..." (max_size: 256) + optional bytes raw_receiver = 2; // d || pk_d, 43 bytes — for cross-check + optional bytes seed_fingerprint = 3; // ZIP-32 §6.1 fingerprint of attesting device + optional uint64 diversifier_index = 4; // echoed for clarity +} +``` + +The existing `ZcashDisplayAddress` (host supplies the string) **stays** but +is renamed in copy to "Display address (FVK match)" to reflect what it +actually proves. The new message is the strong-attestation path. + +### 5.2 hdwallet wrapper + +```ts +wallet.zcashGetUnifiedAddress({ + addressNList, + account?: number, + diversifierIndex?: bigint, // default 0n + showDisplay?: boolean, // default true + expectedSeedFingerprint?: Uint8Array, +}): Promise<{ + address: string, // device-derived UA — trustable + rawReceiver: Uint8Array, // 43 bytes + seedFingerprint?: Uint8Array, + diversifierIndex: bigint, +}> +``` + +### 5.3 Vault privacy tab + +Replace the current "Verify on device" button (which calls +`ZcashDisplayAddress`) with a **two-step** flow: + +1. "Show device address" → calls `ZcashGetUnifiedAddress`. UI displays the + returned UA prominently next to the device-rendered version. User reads + both side by side. +2. Optional "Verify host address (FVK match only)" button retained for the + weaker check, with copy that names the limitation. This is the existing + PR #141 button with the wording fixed. + +## 6. Implementation phases + +Each phase is mergeable on its own. Each closes with a cross-language test +harness verifying outputs against the upstream `orchard` Rust crate via +test vectors checked into the firmware repo. + +### Phase 2.1 — Foundation primitives + +**Goal:** ship FF1-AES256 + Pallas SWU + GroupHash + their tests. No new +firmware behavior — these compile into the binary and have unit tests. + +**Files (estimate):** +- `deps/crypto/trezor-firmware/crypto/ff1.{c,h}` — FF1-AES256 +- `deps/crypto/trezor-firmware/crypto/pallas_swu.{c,h}` — SWU map + isogeny +- Extend `pallas.{c,h}` with `expand_message_xmd_blake2b` +- `unittests/firmware/zcash_phase2.cpp` + +**Test vectors needed:** +- FF1-AES256: NIST CAVP vectors + `fpe` Rust crate vectors +- SWU: `pasta_curves::pallas::map_to_curve_simple_swu` golden outputs +- GroupHash: `orchard::keys` test fixtures + +**Effort:** 3–5 days dev, 2 days vectors + tests. **High-risk** on SWU isogeny. + +### Phase 2.2 — Sinsemilla + Orchard receiver + +**Goal:** derive `(d_j, pk_d_j)` on device. New FSM internal helper, no proto +changes yet. + +**Files:** +- `deps/crypto/trezor-firmware/crypto/sinsemilla.{c,h}` +- Extend `lib/firmware/zcash.{c,h}`: + - Add `dk` to `ZcashOrchardKeys` + - `zcash_orchard_ivk(rivk, ak, nk, ivk_out)` — uses Sinsemilla + - `zcash_orchard_diversifier(dk, j, d_out)` — uses FF1 + - `zcash_orchard_receiver(account, j, raw_out_43)` — composes everything + +**Test vectors:** `orchard::keys::FullViewingKey::default_address` for many +`(seed, account, j)` tuples. We must agree byte-for-byte. + +**Effort:** 5–7 days. **Highest-risk** phase — Sinsemilla is involved and +under-documented relative to its complexity. + +### Phase 2.3 — F4Jumble + bech32m + UA encoding + +**Goal:** turn 43 raw receiver bytes into a `u1...` string. + +**Files:** +- `lib/firmware/zip316.{c,h}` — F4Jumble + UA assembly +- Extend bech32 helper to support the `m` constant (BIP-350) + +**Test vectors:** ZIP-316 has explicit test vectors. Plus +`zcash_address::unified::Address::encode` outputs. + +**Effort:** 2–3 days. Low risk. + +### Phase 2.4 — Proto + FSM handler + UI + +**Goal:** wire the new message end-to-end through firmware → hdwallet → vault. + +**Files:** +- `device-protocol/messages-zcash.proto` — `ZcashGetUnifiedAddress` / + `ZcashUnifiedAddress` +- `lib/firmware/fsm_msg_zcash.h` — `fsm_msgZcashGetUnifiedAddress` +- `lib/firmware/messagemap.def` + `fsm.h` — message routing +- `hdwallet-keepkey` — wrapper +- `keepkey-vault` — UI flow update + bun handler + +**Effort:** 2–3 days. Mostly mechanical given primitives are tested. + +### Phase 2.5 — Hardening, fault injection, hardware test + +**Goal:** confirm on real hardware, scrub side channels, add fault-injection +mitigations consistent with the rest of the firmware. + +**Effort:** 2–3 days + hardware time. + +**Total: 14–21 working days** for a single contributor focused on this. Add +~50% for unknowns and review feedback → call it 4–5 weeks of calendar time +including review, hardware verification, and the cross-language test +infrastructure. + +## 7. Testing strategy + +### 7.1 Cross-language goldens + +Generate goldens from the `orchard` Rust crate, check them into +`unittests/firmware/zcash_orchard_vectors.h` as C arrays: + +```rust +// Generator (host-side, Rust) — run once, commit output +let seed = hex!("000102...1f"); +for account in [0, 1, 7, 100, 0x7fffffff] { + for j in [0u128, 1, 256, 0xdeadbeef, (1u128 << 88) - 1] { + let sk = ExtendedSpendingKey::master(&seed); + let osk = sk.derive_internal(account); + let fvk = FullViewingKey::from(&osk); + let addr = fvk.address_at(j, Scope::External); + let ua = unified::Address::try_from_items(vec![Receiver::Orchard(addr.to_raw_address_bytes())]); + emit_c_vector(seed, account, j, addr, ua); + } +} +``` + +The firmware unit test imports the header and asserts byte equality at every +intermediate stage (ivk, d, g_d, pk_d, raw_receiver, encoded UA). + +### 7.2 Phase boundaries + +Each phase ships with its own `unittests/firmware/zcash_phaseN.cpp` that +runs in CI via the existing `firmware-unit` target. A phase can't merge +without its goldens. + +### 7.3 On-device sanity + +Once Phase 2.4 lands, manual hardware test: +- Initialize a known seed (the all-allallall mnemonic) +- Call `ZcashGetUnifiedAddress(account=0, j=0)` +- Confirm OLED renders the same `u1...` string the upstream Rust crate + computes for that mnemonic + j=0 +- Repeat for j={1, 100, 2^32, 2^88-1} +- User-cancel test: long-press Cancel → device returns `Failure_ActionCancelled` +- Wrong-FVK injection: skip — there is no host-supplied FVK in this flow + +## 8. Open design questions + +1. **Diversifier index bounds.** Specced as 88-bit. Practical wallets use + small indexes (≤ 2^31). Do we accept full 88-bit on the wire and reject + ranges with no real users, or cap at 2^32 to fit a `uint32`? **Tentative:** + accept full `uint64` on the wire (caller passes 0..2^64 range; spec + technically allows up to 2^88 but no wallet uses that), reject any value + > 2^64-1 with `Failure_SyntaxError`. +2. **Multi-receiver UAs.** This design only ever displays a single-receiver + Orchard UA. Wallets that present users with `u1...` containing additional + transparent or Sapling receivers will have a UA the device won't match. + **Position:** the device-attested string is the *Orchard-only* UA derived + from this device. Wallets that bundle more receivers should present + *both* — "your full UA" and "the device-attested Orchard receiver" — and + the user verifies the latter against the OLED. +3. **Diversifier index display.** Show `j` on the OLED alongside the UA, or + omit? **Tentative:** show; at minimum show `j == 0` vs `j > 0` and the + account number, so users can spot if they ask for default but the host + slipped in a non-default index. +4. **bech32m vs raw-receiver display.** OLED is 256x64. A `u1...` Orchard + UA fits across multiple lines (already works in PR #142). Raw receiver + would be 86 hex chars — also displayable, less natural. **Tentative:** + show `u1...` (consistent with what wallets show); QR encodes the full UA. +5. **Should `dk` ship in `ZcashOrchardFVK`?** Currently FVK is `(ak, nk, rivk)`. + `dk` is part of the FVK in ZIP-32 (FVK = `(ak, nk, rivk, dk)`). Wallets + that want to derive their own diversifiers without round-tripping the + device need it. **Tentative:** add `dk` to `ZcashOrchardFVK` as + `optional bytes dk = 5;`. It's not secret beyond what FVK already exposes. + +## 9. Migration path + +The existing `ZcashDisplayAddress` flow stays; its UI copy is downgraded to +"Display address (FVK match) — proves the FVK belongs to this device, not +that the displayed UA is spendable." The new +`ZcashGetUnifiedAddress` flow is the recommended path going forward. Wallets +that want strong attestation use the new message; older code paths continue +to work. + +Once Phase 2 ships: + +- Vault: replace the "Verify on device" button's primary action to call + `ZcashGetUnifiedAddress`, drop the host-supplied address from the FSM call. +- Documentation: record the strong-attestation flow as the canonical "verify + address" procedure. + +## 10. References + +- Zcash Protocol Specification, NU6 — https://zips.z.cash/protocol/protocol.pdf +- ZIP-32 — https://zips.z.cash/zip-0032 +- ZIP-316 — https://zips.z.cash/zip-0316 +- Orchard book — https://zcash.github.io/orchard/ +- Pasta paper / curves — https://github.com/zcash/pasta_curves +- Halo 2 spec (Sinsemilla) — https://zcash.github.io/halo2/design/gadgets/sinsemilla.html +- BIP-350 (bech32m) — https://github.com/bitcoin/bips/blob/master/bip-0350.mediawiki +- NIST SP 800-38G (FF1) — https://nvlpubs.nist.gov/nistpubs/SpecialPublications/NIST.SP.800-38G.pdf +- RFC 9380 (hash-to-curve) — https://datatracker.ietf.org/doc/rfc9380/ + +## 11. What this document is not + +- A schedule. Effort estimates are working-days for a focused contributor; + calendar time depends on review velocity and hardware availability. +- Final spec for diversifier-index UX. Section 8 calls out tentative + positions; final UI copy and OLED layout are part of Phase 2.4 review. +- A claim that anything in Phase 2.1–2.3 is small. Sinsemilla and the SWU + isogeny are the high-risk pieces and may take longer than the estimates. + The estimates assume one contributor familiar with finite-field crypto. diff --git a/docs/coin-integration/zcash-pczt-clearsign.md b/docs/coin-integration/zcash-pczt-clearsign.md new file mode 100644 index 000000000..e03b3b612 --- /dev/null +++ b/docs/coin-integration/zcash-pczt-clearsign.md @@ -0,0 +1,322 @@ +# Zcash PCZT Clear Signing + +**Status:** implemented on `feature/clearsign-txs`. +**Base:** `release/7.15.0`. +**Scope:** Orchard PCZT signing. + +## Threat Model + +The firmware must not sign a host-provided Orchard sighash by itself. If the +companion app can choose the sighash directly, a compromised companion can turn +the hardware wallet into a blank-check signer: the user confirms one summary, +but the signature authorizes a different transaction digest. + +The firmware therefore has to build the signing digest from transaction data it +can validate. The current PCZT protocol does this in layers: + +1. The host sends ZIP-244 component digests and Orchard bundle metadata. +2. The device assembles the final ZIP-244 sighash from those component digests. +3. The device recomputes the Orchard digest from streamed action fields. +4. Signatures are returned only after the recomputed Orchard digest matches the + Orchard digest used in the device-computed sighash. + +## Firmware Policy + +`ZcashSignPCZT` is rejected before user confirmation unless it includes: + +- `header_digest`, exactly 32 bytes. +- Plaintext header fields: `tx_version`, `version_group_id`, `branch_id`, + `lock_time`, and `expiry_height`. Firmware recomputes ZIP-244 + `header_digest` from these fields and rejects on mismatch. +- `orchard_digest`, exactly 32 bytes. +- Orchard flags, value balance, and 32-byte anchor. +- A 32-byte transparent digest when transparent inputs or outputs are present. +- Optional transparent digest, if present, must be exactly 32 bytes. + +Sapling is out of scope for this signing path. Any host-provided +`sapling_digest` is rejected; firmware uses the ZIP-244 empty Sapling digest +internally. + +`ZcashPCZTAction` is rejected unless each action includes the fields needed to +recompute the Orchard digest: + +- `nullifier`, `cmx`, `epk`, `cv_net`, and `rk`, each 32 bytes. +- `enc_compact`, 52 bytes. +- `enc_memo`, 512 bytes. +- non-empty `enc_noncompact`. +- `out_ciphertext`, 80 bytes. +- `value`, 43-byte `recipient` (`d || pk_d`), and 32-byte `rseed` for trusted + Orchard output display. + +The legacy path where `ZcashPCZTAction.sighash` was accepted as the signing +digest is intentionally rejected. + +## What Is Verified + +The device now verifies `header_digest` from plaintext header fields and +recomputes `transparent_digest` from streamed transparent inputs/outputs before +emitting any transparent or Orchard signature. Sapling is not accepted in this +path. For shielded-only Orchard transactions, the transparent digest defaults to +the ZIP-244 empty transparent digest, so there is no host-provided transparent +component. + +The device verifies the Orchard action digest from the action plaintext fields +available in PCZT. Each action must also carry the plaintext Orchard output +metadata needed for trusted display: raw receiver `recipient = d || pk_d`, +`value`, and `rseed`. Firmware recomputes the action `cmx` from that metadata +and the action nullifier before displaying the ZIP-316 Orchard-only Unified +Address and amount. + +The device also computes the fee as: + +```text +fee = transparent_input_total - transparent_output_total + orchard_value_balance +``` + +The computed fee must match the requested fee and must be confirmed on-device +before any final signatures are returned. + +## UI Behavior + +Current signing screens show: + +- Shielded-only: total amount, fee, and Orchard action count. +- Transparent shielding: total amount, fee, transparent input count, and + transparent output count, and Orchard action count. +- Transparent input signing: per-input amount and BIP-44 path validation. +- Transparent outputs: each standard P2PKH/P2SH t-address and amount. +- Orchard outputs: each ZIP-316 Orchard-only Unified Address and amount after + note commitment binding. +- Final fee confirmation: computed fee after digest/output verification. + +## Outputs + +If we can derive a digest from plaintext transaction fields, we should do so for +outputs too. + +Transparent outputs are streamed as recipient scripts and values. Firmware +computes the transparent digest, displays transparent destination/address and +amount, and rejects non-standard scripts until an explicit raw-script review +policy exists. + +Orchard outputs are displayed from the supplied raw receiver/value/rseed +metadata only after firmware recomputes `cmx = NoteCommit(...)` and verifies it +matches the action `cmx`. This binds the displayed privacy recipient and amount +to the signed Orchard action. + +## libzcash-orchard-c Review + +Reviewed: https://github.com/wh00hw/libzcash-orchard-c + +This repo is applicable as implementation guidance, not as a wholesale firmware +dependency. It is a pure C11 static library under MIT, but it overlaps heavily +with primitives already present in this firmware tree: BLAKE2b, Pallas, +Sinsemilla, RedPallas, secp256k1, BIP32/BIP39, Base58, and ZIP-316 helpers. The +useful part for KeepKey is its transaction-signing shape: + +- Separate ZIP-244 `T.2 transparent_digest` from ZIP-244 `S.2` per-input + transparent signature digest. These are different commitments and must not be + collapsed into a single helper. +- Stream transparent inputs and outputs into independent BLAKE2b component + hashers: prevouts, sequences, outputs, amounts, scripts, and per-input + `txin_sig_digest`. +- Track transparent input/output value totals while hashing so the device can + compute `fee = transparent_in - transparent_out + orchard_value_balance` + locally and show that fee on-device. +- Treat Sapling as unsupported. A Sapling component would be a hidden value sink + until the firmware has Sapling parsing and display, so this firmware path + rejects host-provided Sapling data and uses the ZIP-244 empty Sapling digest. +- Capture every transparent output `(value, script_pubkey)` and render standard + P2PKH/P2SH scripts as Zcash t-addresses for user review. +- For Orchard outputs, `cmx` binding is the first target: require plaintext + `(d, pk_d, value, rseed)` metadata and recompute the note commitment against + the action `cmx`. The stronger follow-up is memo binding: recompute + `enc_ciphertext` and `epk` from `(recipient, value, rseed, memo)` using + Orchard KDF + ChaCha20-Poly1305. + +Reference role: KeepKey uses the transaction-signing structure and test-vector +shape while keeping the existing firmware primitives and protocol surfaces. + +Key files reviewed: + +- https://github.com/wh00hw/libzcash-orchard-c/blob/main/include/zip244.h +- https://github.com/wh00hw/libzcash-orchard-c/blob/main/src/zip244.c +- https://github.com/wh00hw/libzcash-orchard-c/blob/main/include/orchard_signer.h +- https://github.com/wh00hw/libzcash-orchard-c/blob/main/src/orchard_signer.c +- https://github.com/wh00hw/libzcash-orchard-c/blob/main/include/base58.h +- https://github.com/wh00hw/libzcash-orchard-c/blob/main/SECURITY.md + +## Updated Implementation Plan + +### Phase 1: digest helpers and policy + +Status: implemented for header/transparent helpers and clear-signing policy. + +- Require component digests and Orchard metadata before signing. +- Reject the legacy host-only action `sighash` path. +- Compute the ZIP-244 root sighash on-device from component digests. +- Verify the Orchard digest from streamed action fields. +- Add pure helpers for: + - `header_digest` from plaintext header fields. + - `transparent_digest` from plaintext transparent inputs/outputs. + - per-input transparent `SIGHASH_ALL` digest. + +### Phase 2: plaintext header and transparent streaming + +Status: implemented. + +- Extend the protocol with plaintext header fields and verify + `header_digest` locally. Implemented. +- Extend transparent input messages with raw ZIP-244 digest fields: + `prevout_txid`, `prevout_index`, `sequence`, `amount`, and `script_pubkey`. +- Add transparent output streaming: + `index`, `amount`, and `script_pubkey`. +- Compute `transparent_digest` locally and compare it to the companion-provided + digest during the migration period. +- Compute per-input transparent sighashes locally before ECDSA signing, instead + of signing `ZcashTransparentInput.sighash`. +- Track transparent input and output totals, compute the fee, compare it to the + requested fee, and display the computed fee. +- Reject transactions with transparent outputs that cannot be rendered on-device + until the UI has an explicit "unknown script" review policy. + +### Phase 3: transparent output UI + +Status: implemented for standard P2PKH/P2SH scripts. + +- Add a Zcash transparent script renderer for standard P2PKH and P2SH: + - mainnet P2PKH: `t1` + - mainnet P2SH: `t3` + - testnet P2PKH: `tm` + - testnet P2SH: `t2` +- Display each transparent output address and amount on the trusted screen. +- Require every displayed transparent output and the computed fee to be + confirmed before any signature is produced. + +### Phase 4: stronger Orchard output metadata binding + +Status: implemented for recipient/value display via `cmx` binding. + +- Extend `ZcashPCZTAction` with raw Orchard output metadata: + `recipient` (`d || pk_d`), `rseed`, and explicit output value. +- Recompute Orchard note commitment `cmx` from `(d, pk_d, value, rho, rseed)`, + where `rho` is the action nullifier, and reject on mismatch. +- Display the recipient as a ZIP-316 Orchard-only UA and display the output + value. Require per-output confirmation. +- Memo display remains a separate hardening step: add ChaCha20-Poly1305 support + if it is not linked into the firmware image, then recompute `enc_ciphertext` + and `epk` for memo binding. +- Render empty/text/opaque memos on-device once memo binding is available. + +## Crypto Library Inventory + +Already available in this firmware tree: + +- BLAKE2b with personalization. +- AES-256. +- Pallas field and point arithmetic. +- Pallas SWU / group hash support. +- Sinsemilla / Orchard IVK support. +- RedPallas signing. +- ZIP-316 Orchard-only unified-address helpers. +- Zcash transparent Base58Check plus standard P2PKH/P2SH script-to-address + rendering. +- ChaCha20-Poly1305 source exists under trezor-crypto, but it is currently not + linked into the firmware crypto target. Memo binding will need that target + wiring plus Orchard note-encryption KDF glue. + +No new primitive is required for the implemented PCZT clear-signing policy. The +remaining optional hardening work is parsing and digest construction for more +transaction components: + +- Sapling parsing only if Sapling is ever intentionally added to this firmware + path; current policy is to reject it. +- Orchard memo binding/display by recomputing note encryption from + recipient/value/rseed/memo. + +## Keystone Comparison + +Keystone is a useful architecture comparison, but the audit alone is not enough +evidence. I inspected the local Keystone firmware repo: + +- Path: `/Users/highlander/keepkey/keystone3-firmware` +- Branch: `master` +- Commit: `2a48ba022ac24d3b343fa4b9e59251a5de3e1160` + +The actual Keystone signing path does derive the signing digest from PCZT data: + +- `rust/rust_c/src/zcash/mod.rs::sign_zcash_tx` extracts a `ZcashPczt` UR and + calls `app_zcash::sign_pczt`. +- `rust/apps/zcash/src/pczt/sign.rs::sign_pczt` builds a low-level PCZT signer, + then calls `pczt_ext::sign_transparent` and/or `pczt_ext::sign_orchard`. +- `rust/zcash_vendor/src/pczt_ext.rs::shielded_sig_commitment` constructs the + ZIP-244-style commitment from locally computed component digests: + `digest_header`, `transparent_sig_digest`, `digest_sapling`, and + `digest_orchard`. +- `digest_orchard` recomputes the Orchard digest from action fields: + nullifier, cmx, ephemeral key, encrypted memo/ciphertext slices, cv_net, rk, + out ciphertext, bundle flags, value balance, and anchor. +- `transparent_sig_digest` computes transparent prevouts, amounts, scripts, + sequence, outputs, and per-input data for `SIGHASH_ALL`. + +Keystone still has broader in-firmware PCZT parsing and Orchard output +decryption support. KeepKey now covers the same no-bare-sighash signing rule for +the implemented scope: header digest, transparent digest, Orchard digest, +displayed transparent outputs, displayed Orchard receiver/value metadata, and +the miner fee are all verified before signatures are released. + +Keystone also does more UI-side output parsing than our current firmware: + +- `rust/apps/zcash/src/pczt/parse.rs::parse_orchard_output` tries to decrypt + Orchard outputs with external/internal OVKs, validates decoded recipient data, + verifies a supplied `user_address` matches the decoded Orchard receiver, and + treats undecryptable non-zero Orchard outputs as invalid. +- `src/ui/gui_chain/multi/gui_zcash.c::GuiZcashOverviewTo` displays parsed + output value, address, change tag, and memo. +- The checker validates Orchard `cv_net`, value balance, nullifier/rk for owned + spends, and note commitment consistency before the UI/sign flow. + +This gives us a concrete target, not just an FYI: + +1. KeepKey branch: reject bare host sighashes, compute the final sighash from + required component digests, verify the Orchard digest from streamed action + fields, and compute header/transparent digests from streamed plaintext. +2. Output UI: transparent outputs are shown directly from verified script/value. + Orchard outputs are shown from directly supplied PCZT receiver/value/rseed + metadata that is cryptographically checked against the action `cmx`. +3. Remaining parity step: memo binding/display by recomputing Orchard note + encryption from recipient/value/rseed/memo. + +Sources: + +- Local Keystone firmware at the commit above. +- Public audit context: + https://leastauthority.com/wp-content/uploads/2025/03/Least-Authority-ZCG-Keystone-Hardware-Wallet-Final-Audit-Report.pdf + +## Tests + +The TDD coverage for this policy lives in: + +- `unittests/firmware/zcash.cpp` +- `deps/python-keepkey/tests/test_msg_zcash_sign_pczt.py` + +The firmware unit tests cover the pure policy helper, ZIP-244 digest helpers, +transparent digest/sighash helpers, Orchard receiver encoding, and Orchard note +commitment recomputation. The Python protocol tests cover the emulator-facing +behavior: legacy host-sighash requests are rejected, verified PCZT requests +sign, missing Orchard action digest fields abort signing, transparent digests +must match streamed plaintext, host transparent sighashes are rejected, and +Orchard recipient/value tampering is rejected. + +Current local verification: + +- `build/bin/zcash-crypto-unit` passes 56 tests, including: + - `ComputeHeaderDigest_FromPlaintextFields` + - `ComputeTransparentDigest_DistinctFromPerInputSighash` + - `ComputeTransparentDigest_EmptyBundle` + - `ComputeTransparentSighash_RejectsUnsupportedRequest` + - `OrchardNoteCommitment_KnownVector` + - `OrchardReceiverToUnifiedAddress_KnownVector` +- `cmake --build build --target kkfirmware` passes. +- `cmake --build build --target kkfirmware.keepkey` passes. +- `git diff --check` is clean. diff --git a/docs/release/7.14.2-rc30-hardware-plan.md b/docs/release/7.14.2-rc30-hardware-plan.md deleted file mode 100644 index 68d403aa3..000000000 --- a/docs/release/7.14.2-rc30-hardware-plan.md +++ /dev/null @@ -1,589 +0,0 @@ -# rc30 hardware verification plan — KeepKey firmware 7.14.2 - -**Branch** `release/7.14.2` · **candidate** `862e2610f633ba4de03558483271fa4f8c5e6339` (rc30) · **release range** `1af2ffe7de..862e2610f` (65 commits) · **repo** `BitHighlander/keepkey-firmware` - ---- - -## 0. What rc30 is, and what it is not - -**There is no rc30 tag and there cannot be one.** `.github/workflows/release.yml:40-46` derives `TAG_VERSION="${GITHUB_REF_NAME#v}"` and hard-fails unless it equals the `project(... VERSION ...)` string in `CMakeLists.txt`, which is `7.14.2`. A tag `v7.14.2-rc30` yields `TAG_VERSION=7.14.2-rc30 != 7.14.2` and the `validate` job exits 1 before anything builds. Only `v7.14.2` FINAL can ever pass that gate. - -**So the artifact under test comes from the branch CI run, not a release.** `.github/workflows/ci.yml:363-369` uploads: - -``` -firmware-v7.14.2-862e2610 <- artifact (name = firmware-v${fw_version}-${git_short}) - firmware.keepkey.v7.14.2-862e2610-firmware.keepkey.bin - firmware.keepkey.v7.14.2-862e2610-*.elf -``` - -Two properties of that artifact the tester must internalise: - -1. **It is unsigned.** `ci.yml` has no signing step; signing only happens in the release workflow, which cannot run (above). Expect the bootloader to show the unofficial-firmware warning and to **wipe storage on install**. Confirm this at flash time and write it down — if the device comes up *already initialised* after flashing, that contradicts the storage-wipe model and is itself a finding. Nothing in this plan can support any claim about signed-upgrade storage preservation. -2. **The version string in the CI *test-report PDF* is not trustworthy** — `ci.yml:731` runs the same `grep -oP` inside a BusyBox container where `-P` does not exist, with `|| echo "unknown"` (#467). The *artifact* name is produced on `ubuntu-latest` with GNU grep and is correct. Identify the build by the device's own `Features` banner and the `.bin` sha256, not by any PDF header. - -**Known-red CI state: 22 integration failures, zero firmware defects among them.** Per `docs/release/7.14.2.md:340-353` (`22 failed, 362 passed, 47 skipped`, 134 s with the #477 per-test timeout): - -| count | message | why it is red | -|---|---|---| -| 9 | `Transaction signing disabled by policy` (TON) | documented BREAKING gate; suite must opt in | -| 5 | `Chain Id out of bounds` | #445 working as designed; fixed by python-keepkey #215 | -| 3 | `Enable AdvancedMode to blind-sign` (TRON) | documented BREAKING gate | -| 2 | `Timeout >60.0s` | extra disclosure screens the pinned suite never acks (#466) | -| 1 | `Arbitrary contract data signing disabled by policy` | 0x `transformERC20` via #468 | -| 1 | `Enable AdvancedMode to blind-sign typed hashes` | documented BREAKING gate | -| 1 | `Structured EIP-712 disabled pending canonical display hardening` | deliberately disabled | - -**Do not "fix" firmware for any of these.** Every one of the 22 is a stale pinned expectation. This plan produces hardware evidence for all seven rows — that mapping is in the final checklist — which is precisely what the release notes currently record as resting on diff-reading alone. - -`deps/python-keepkey` is pinned at `81e581f`, a commit on an unmerged topic branch, and the release notes say the pin must be updated before tag. **Every script below runs against that pinned client.** The repin is expected to change 6 of the 22 test results; it must not change *device* behaviour. If it does, re-run the affected tests. - -**Evidence standard.** A photograph proves what was drawn. It does not prove what was not drawn, it does not prove memory safety, and it cannot show a pre-fix screen on a post-fix build. Sections 3 and 4 say exactly where those limits bite. - ---- - -## 1. Bench setup — do this once, before test 1 - -### 1.1 Flash and identify - -Device into bootloader mode, then: - -``` -cd /Users/highlander/WebstormProjects/keepkey-stack/projects/keepkey-firmware-7142/deps/python-keepkey -./keepkeyctl firmware_update -f /firmware.keepkey.v7.14.2-862e2610-firmware.keepkey.bin -``` - -Record as artifact #0, before any test: - -- `sha256` of the `.bin` you flashed -- the device banner: `major.minor.patch` must read **7.14.2**, and `firmware_variant` must **not** be `KeepKeyBTC` (tests 3, 4, 6, 8, 9, 10, 11, 12 all need handlers absent from the btc-only build) -- `device_id` -- whether storage survived the flash - -### 1.2 Host environment — three traps, all reproduced live on this Mac - -1. **Quit KeepKey Vault** (and anything else on port 1646) first. It holds USB interface 0 and libusb then returns `LIBUSB_ERROR_ACCESS [-3]`. -2. **`export PROTOCOL_BUFFERS_PYTHON_IMPLEMENTATION=python`** once per shell. The vendored `_pb2` files predate protoc 3.19 and raise *"Descriptors cannot be created directly"* under the C++ backend. Three of the twelve scripts set it internally; the rest do not. Set it globally and stop thinking about it. -3. **`import config` picks HID and dies on this machine.** `tests/config.py:70-80` branches on `hid_devices[0][1] != None`. hidapi here reports this KeepKey's single interface as `interface_number=1 / usage_page=0xFF01` (`'KeepKey - main'`, path `b'DevSrvsID:4295145257'`), so `HidTransport.enumerate()` files it in the *debug* slot, `config` takes the HID branch, and opening path `None` raises `TypeError: expected bytes, NoneType found`. On a non-DEBUG_LINK build it can also fall through to `SocketTransportClient('trezor.bo:2000')`. WebUSB enumerates the same device correctly. - -**Fix once, for all scripts.** Create `deps/python-keepkey/tests/hwconn.py`: - -```python -import os, sys -sys.path = ['../'] + sys.path -from keepkeylib.transport_webusb import WebUsbTransport -_d = WebUsbTransport.enumerate() -if not _d: - raise SystemExit("no KeepKey on WebUSB -- is Vault still running?") -TRANSPORT = WebUsbTransport -TRANSPORT_ARGS = (_d[0],) -TRANSPORT_KWARGS = {'debug_link': False} -``` - -Then patch every script that says `import config`: - -``` -sed -i '' 's/^import config$/import hwconn as config/'