Skip to content

Commit ebb7fec

Browse files
committed
release: 7.14.2 security line — the device shows what it signs
Every defect closed here is the same shape: the device signed bytes it never displayed, or displayed text it could not vouch for. EVM omitted chain_id no longer yields a pre-EIP-155 signature; it is refused before any screen is drawn arbitrary contract data requires AdvancedMode, and the check gates rather than warns 0x clear-signing is bound to complete calldata and to both tokens resolving calldata beyond the initial chunk is no longer described from hardcoded ABI offsets EIP-712 the structured JSON path is withdrawn: it could not guarantee that a displayed value was the value being hashed, and one screen took its title from the attacker-supplied domain name EthereumSignTypedHash requires AdvancedMode and shows an explicit blind-sign screen first Display message signing renders every byte, escaping non-printables, instead of stopping at the first NUL bodies too long for the screen are paged with n/m titles rather than silently clipped the fit measurement is draw_string()'s own walk with pixel writes off, so measuring and drawing cannot disagree Altcoins THORChain and Maya memos keep their last character and refuse a declared length that misdescribes its own content; trailing memo fields are disclosed Osmosis prints the exact signed integer beside the exact denom Cosmos IBC shows the receiver instead of printing the sender twice Solana SPL amounts are scaled by the decimals in the signed instruction, and never render a nonzero transfer as zero Binance denom formatting no longer writes past its buffer Setup and consent an injected RecoveryDevice cannot write settings mid-ceremony a cancelled passphrase confirmation is not cached cancelled authenticator and Uniswap operations do not complete Reproduced on signed v7.14.1 before the fixes and verified on 7.14.2 silicon after them. CI now captures the OLED for these paths; it previously captured none of them, because the harness read the firmware version with grep -oP, which BusyBox lacks, and silently fell back to 7.14.0. Not claimed: independent re-exploitation and a final adversarial round are incomplete, and the fit measurement covers layout_standard_notification only, so custom-layout address and xpub bodies still clip without warning. deps/python-keepkey pins keepkey/python-keepkey#216, which merges once this is green.
1 parent 1af2ffe commit ebb7fec

100 files changed

Lines changed: 5362 additions & 803 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.github/workflows/ci.yml

Lines changed: 89 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ on:
3535
default: false
3636

3737
env:
38-
BASE_IMAGE: kktech/firmware:v15
38+
BASE_IMAGE: kktech/firmware@sha256:7438e53933d47d53157ed6d96d864cb208597e62dce26235ace09d1063427fa2
3939
EMU_IMAGE: kkemu-ci
4040

4141
jobs:
@@ -88,8 +88,13 @@ jobs:
8888

8989
- name: Install gitleaks
9090
run: |
91-
GITLEAKS_VERSION=$(curl -sSf https://api.github.com/repos/gitleaks/gitleaks/releases/latest \
92-
| grep -oP '"tag_name":\s*"v\K[^"]+')
91+
# PINNED. Tracking releases/latest means a new upstream ruleset can turn
92+
# this gate red with no change to this repository -- which is exactly what
93+
# happened: a newer generic-api-key rule began flagging published BIP32 test
94+
# vectors in 2014/2018 history, and because every build job declares
95+
# `needs: [.., secret-scan]`, the whole build and test graph was SKIPPED
96+
# rather than failed. Bump deliberately, with the scan re-verified. See #424.
97+
GITLEAKS_VERSION=8.30.0
9398
curl -sSfL "https://github.com/gitleaks/gitleaks/releases/download/v${GITLEAKS_VERSION}/gitleaks_${GITLEAKS_VERSION}_linux_x64.tar.gz" \
9499
| tar -xz -C /usr/local/bin gitleaks
95100
gitleaks version
@@ -207,6 +212,20 @@ jobs:
207212
done
208213
[ "$FAILED" = "0" ] || exit 1
209214
215+
- name: Enforce RNG source invariants
216+
run: |
217+
if grep -q 'RAND_PLATFORM_INDEPENDENT=0' CMakeLists.txt; then
218+
echo "::error::RAND_PLATFORM_INDEPENDENT is a definedness switch, not a value toggle"
219+
exit 1
220+
fi
221+
grep -q 'add_definitions(-DRAND_PLATFORM_INDEPENDENT)' CMakeLists.txt
222+
grep -q '#ifndef RAND_PLATFORM_INDEPENDENT' lib/rand/rng.c
223+
grep -q 'defined(EMULATOR) && defined(__arm__)' lib/rand/rng.c
224+
if grep -En 'return[[:space:]]+random\(\)' lib/rand/rng.c; then
225+
echo "::error::Emulator cryptography must not use libc random()"
226+
exit 1
227+
fi
228+
210229
# ═══════════════════════════════════════════════════════════
211230
# STAGE 2: BUILD — compile only after gate passes
212231
# ═══════════════════════════════════════════════════════════
@@ -490,7 +509,12 @@ jobs:
490509
# pb2 files expect; newer protoc generates Python that requires
491510
# newer protobuf runtime, which breaks the python-keepkey suite).
492511
# - protobuf 3.20.3 (Python runtime — strict pin).
493-
# - nanopb 0.3.9.4.post3 (the proto generator the firmware build uses).
512+
# - nanopb 0.3.9.4.post3 — NOTE: this is NOT the version the firmware
513+
# builder image uses. The pinned image builds nanopb-0.3.9.8 from source
514+
# (see Dockerfile). This job installs a different generator via pip, so
515+
# headers produced here are not byte-comparable with the firmware build's.
516+
# Tracked in GH #425; do not treat this job's output as provenance for
517+
# the firmware artifacts.
494518
# - KK_DEBUG_LINK=ON (default OFF; without it,
495519
# fsm_msgDebugLinkGetState is excluded from the build and any
496520
# read_layout() call hangs the test).
@@ -726,6 +750,67 @@ jobs:
726750

727751
# ═══════════════════════════════════════════════════════════
728752
# STAGE 4: PUBLISH — manual trigger only, all tests must pass
753+
754+
# ═══════════════════════════════════════════════════════════
755+
# GATE: one authoritative answer for the whole graph
756+
# ═══════════════════════════════════════════════════════════
757+
#
758+
# Every build and test job declares `needs: [lint-format, static-analysis,
759+
# check-submodules, secret-scan]`. When one of those gate jobs fails, GitHub
760+
# marks the whole downstream graph SKIPPED rather than failed -- and a skipped
761+
# job is not a red check. The run summary then shows green ticks on whatever
762+
# finished, which reads as healthy unless someone opens the job list.
763+
#
764+
# That has now happened three times on the 7.14.2 line: gitleaks failing on
765+
# develop, lint-format timing out inside its apt.llvm.org install (#471), and
766+
# gitleaks again after an unpinned upstream bump. Each time the ARM build,
767+
# the unit tests and both python suites produced NOTHING while the run looked
768+
# partially green.
769+
#
770+
# This job exists so that cannot happen quietly. It needs every required job,
771+
# runs with `if: always()` so it executes even when they skip, and fails
772+
# unless each one reports exactly `success`. failure, cancelled and skipped
773+
# are all treated as not-success, because for a required job they are.
774+
#
775+
# publish-emulator is deliberately absent: it is workflow_dispatch-only and
776+
# is legitimately skipped on every push and pull_request.
777+
#
778+
# Point branch protection at THIS job rather than the individual ones. It is
779+
# the only check whose green means "the entire graph ran and passed".
780+
ci-gate:
781+
name: CI gate
782+
if: always()
783+
needs:
784+
- lint-format
785+
- secret-scan
786+
- static-analysis
787+
- check-submodules
788+
- build-emulator
789+
- build-arm-firmware
790+
- unit-tests
791+
- python-integration-tests
792+
- python-dylib-tests
793+
- generate-test-report
794+
runs-on: ubuntu-latest
795+
timeout-minutes: 5
796+
steps:
797+
- name: Assert every required job succeeded
798+
env:
799+
NEEDS_JSON: ${{ toJSON(needs) }}
800+
run: |
801+
set -euo pipefail
802+
echo "$NEEDS_JSON" | jq -r 'to_entries[] | "\(.value.result)\t\(.key)"' | sort
803+
echo
804+
NOT_SUCCESS=$(echo "$NEEDS_JSON" \
805+
| jq -r 'to_entries[] | select(.value.result != "success") | .key')
806+
if [ -n "$NOT_SUCCESS" ]; then
807+
echo "::error::Required jobs did not succeed: $(echo $NOT_SUCCESS | tr '\n' ' ')"
808+
echo "A skipped or cancelled required job is NOT a pass. If a gate-stage"
809+
echo "job failed, everything downstream was skipped and produced no signal."
810+
exit 1
811+
fi
812+
echo "All required jobs reported success."
813+
729814
# ═══════════════════════════════════════════════════════════
730815

731816
publish-emulator:

.github/workflows/release.yml

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ on:
1818
- 'v*'
1919

2020
env:
21-
BASE_IMAGE: kktech/firmware:v15
21+
BASE_IMAGE: kktech/firmware@sha256:7438e53933d47d53157ed6d96d864cb208597e62dce26235ace09d1063427fa2
2222

2323
permissions:
2424
contents: write
@@ -92,6 +92,14 @@ jobs:
9292
run: |
9393
echo "# KeepKey Firmware v${{ needs.validate.outputs.fw_version }} — Hash Manifest" > HASHES.txt
9494
echo "" >> HASHES.txt
95+
# Provenance: name the exact toolchain these bytes came out of. BASE_IMAGE
96+
# is a sha256 manifest digest, not a tag, so this identifies one immutable
97+
# image rather than whatever the tag pointed at on the day. Without it a
98+
# green CI build and a locally reproduced binary cannot be shown to be the
99+
# same toolchain product. See GH #425.
100+
echo "builder image ${BASE_IMAGE}" >> HASHES.txt
101+
echo "source commit ${GITHUB_SHA}" >> HASHES.txt
102+
echo "" >> HASHES.txt
95103
for f in *.bin; do
96104
[ -f "$f" ] || continue
97105
FULL_HASH=$(sha256sum "$f" | awk '{print $1}')

.gitleaks.toml

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
title = "KeepKey firmware gitleaks configuration"
2+
3+
# Start from the upstream ruleset and only narrow it, never widen it.
4+
[extend]
5+
useDefault = true
6+
7+
[allowlist]
8+
description = """
9+
Two U2F attestation artifacts are public by design and must not fail the scan.
10+
11+
1. include/keepkey/firmware/u2f/u2f_keys.h — U2F_ATT_PRIV_KEY[]
12+
13+
This is the FIDO U2F batch attestation key. The standard deliberately shares one
14+
attestation key across a whole batch of devices, so that an attestation
15+
certificate identifies a model rather than an individual unit. The key therefore
16+
has to be present in every device: it is compiled into every firmware image and
17+
is extractable from any public release binary. It authenticates nothing, protects
18+
no user funds or data, and is not a wallet key or a code-signing key.
19+
20+
The certificate it accompanies is self-signed as CN=KeepKey U2F and expires
21+
2028-10-01.
22+
23+
2. include/keepkey/firmware/u2f/trezordevkey.pem
24+
25+
An orphaned key file that corresponds to nothing the firmware ships. Its private
26+
scalar does not match U2F_ATT_PRIV_KEY[], and its public key does not match the
27+
public key in the compiled attestation certificate — verified by decoding both.
28+
It is neither the attestation key nor the key that signed the attestation cert.
29+
30+
It is allowlisted rather than merely deleted because CI scans with fetch-depth 0,
31+
so gitleaks reads full history and this file has been present since commit
32+
fe3e5e001 (2019-04-27). Deleting it from the working tree would not clear the
33+
historical finding, and rewriting published history to remove a key that secures
34+
nothing is not a trade worth making.
35+
36+
Removing the dead file and correcting genkeys.sh — which regenerates the
37+
certificate as CN="Trezor U2F" and would also swap in this non-shipping key — is
38+
tracked separately.
39+
40+
3. deps/ — third-party and submodule content
41+
42+
CI checks out without submodules, but it scans with fetch-depth 0, and crypto
43+
dependencies were vendored directly into this repository before they became
44+
submodules. Every finding under deps/ is the generic-api-key rule firing on
45+
high-entropy constants and test vectors in that vendored crypto — for example
46+
the ed25519-donna headers and the AES self-test tables. None is a credential,
47+
and none of it is KeepKey-authored code.
48+
49+
4. docs/ — prose and recorded submodule pins
50+
51+
The security documentation records exact submodule pins, and gitleaks scores a
52+
40-character git commit SHA at entropy ~3.8 and reports it as a generic API key
53+
(for example the python-keepkey pin in
54+
docs/security/7.15.0-rc21-clearsign-release-control.md).
55+
56+
This is scoped by PATH rather than by a regex on the secret. An earlier version
57+
of this file allowlisted the pattern \\b[0-9a-f]{40}\\b globally, which was
58+
rightly flagged in review: that suppressed every 40-character lowercase hex
59+
finding anywhere in the tree, including in source, not merely documented commit
60+
hashes. Path scoping keeps the exemption where the false positives actually are.
61+
62+
Anything under docs/ is prose intended for publication. A credential must never
63+
be committed there in the first place, and this exemption is not a licence to do
64+
so — it is an acknowledgement that git SHAs in prose are not secrets.
65+
66+
5. tests/ — historical, pre-submodule test vectors
67+
68+
This path does not exist in the working tree. The python test suite lived at the
69+
repository root before it became the deps/python-keepkey submodule, and CI scans
70+
with fetch-depth 0, so gitleaks still reads those commits.
71+
72+
Every finding here is the generic-api-key rule firing on published BIP32 test
73+
vectors — the flagged lines are WIF private keys printed next to their own address
74+
and public key as documentation, e.g. tests/test_ecies.py (2014, inherited from
75+
Trezor) and tests/test_msg_eos_signtx.py (2018). They are upstream test fixtures,
76+
published for a decade, and secure nothing.
77+
78+
Scoped by path for the same reason as deps/: the secrets are real key material by
79+
format, so a regex exemption would be dangerous, whereas the path is provably dead.
80+
81+
The cost of a path exemption, stated plainly: a genuine secret committed under
82+
tests/ would NOT be caught. That is acceptable only while the directory does not
83+
exist. If a tests/ tree is ever reintroduced at the repository root, remove this
84+
exemption in the same commit — verified by negative control, a real EC private key
85+
planted under tests/ is ignored, while the same key under lib/firmware/ is still
86+
detected.
87+
88+
Scope note: every exemption is scoped by path and the default ruleset is otherwise
89+
intact, so a genuine secret committed in source still fails the scan. Verified by
90+
negative control: a planted EC private key under lib/firmware/ is still detected.
91+
"""
92+
paths = [
93+
'''include/keepkey/firmware/u2f/trezordevkey\.pem''',
94+
'''include/keepkey/firmware/u2f/u2f_keys\.h''',
95+
'''^deps/''',
96+
'''^docs/''',
97+
'''^tests/''',
98+
]

CMakeLists.txt

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ endif()
99

1010
project(
1111
KeepKeyFirmware
12-
VERSION 7.14.1
12+
VERSION 7.14.2
1313
LANGUAGES C CXX ASM)
1414

1515
set(BOOTLOADER_MAJOR_VERSION 2)
@@ -105,7 +105,11 @@ add_definitions(-DUSE_MONERO=0)
105105
add_definitions(-DUSE_NEM=0)
106106
add_definitions(-DUSE_NANO=1)
107107

108-
add_definitions(-DRAND_PLATFORM_INDEPENDENT=0)
108+
# trezor-crypto's rand.c tests only whether this macro is defined. A value of
109+
# zero therefore did not disable anything; it excluded the library's insecure
110+
# test LCG by definedness. Use the upstream spelling so that intent is clear,
111+
# and let lib/rand/rng.c fail the build if this provider contract is removed.
112+
add_definitions(-DRAND_PLATFORM_INDEPENDENT)
109113

110114
add_definitions(-DPB_FIELD_16BIT=1)
111115
add_definitions(-DPB_NO_ERRMSG=1)

deps/device-protocol

docs/Build.md

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,14 @@
11
Prerequisites
22
-------------
33

4-
Install nanopb-0.3.9.4 from:
4+
Install nanopb-0.3.9.8 from:
55

6-
`https://github.com/nanopb/nanopb/releases/tag/nanopb-0.3.9.4`
6+
`https://github.com/nanopb/nanopb/releases/tag/nanopb-0.3.9.8`
7+
8+
This must match the version baked into the pinned builder image
9+
(`Dockerfile`, `git clone --branch nanopb-0.3.9.8`). Generated headers
10+
differ between nanopb majors, so a mismatch means a local build and a CI
11+
build are not the same product even from the same source. See GH #425.
712

813
Install the python-protobuf dependency:
914

0 commit comments

Comments
 (0)