-
Notifications
You must be signed in to change notification settings - Fork 0
feat: sign-firmware action (Ed25519, Phase 1) #7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
6f4c6ff
918ef53
368f561
fbcf929
deb7e63
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| 6f4c6ffaf316f20ecc539f8e437ea9ce629e5cda:actions/sign-firmware/tests/dummy_private_key.pem:private-key:1 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
| name: "Extract Memory Map" | ||
| description: | | ||
| Compiles a small C extractor against the calling repo's | ||
| app_bootloader_interface.h and exports the slot addresses to | ||
| $GITHUB_ENV. The header is the single source of truth. | ||
|
|
||
| Exports: MAIN_APP_START, MAIN_APP_END, MAIN_SIG_START, MAIN_SIG_END, | ||
| MAIN_CRC_ADDR, BL_APP_START, BL_APP_END, BL_SIG_START, | ||
| BL_SIG_END, BL_CRC_ADDR. | ||
|
|
||
| The header must define EXEC_APP_START_ADDR, APP_FLASH_SIZE, | ||
| APP_SIG_START_ADDR, APP_SIG_END_ADDR, APP_CRC_ADDR, and the BOOT_* | ||
| equivalents. | ||
|
|
||
| inputs: | ||
| include-dir: | ||
| description: "Directory containing app_bootloader_interface.h" | ||
| required: true | ||
|
|
||
| runs: | ||
| using: "composite" | ||
| steps: | ||
| - name: Compile and run extractor | ||
| shell: bash | ||
| run: | | ||
| set -euo pipefail | ||
| gcc -I "${{ inputs.include-dir }}" \ | ||
| "$GITHUB_ACTION_PATH/extract.c" \ | ||
| -o /tmp/_extract_memmap | ||
| /tmp/_extract_memmap | tee -a "$GITHUB_ENV" | ||
|
Comment on lines
+27
to
+30
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use a unique temp binary path instead of a fixed Line 29 uses a predictable executable path ( Suggested patch set -euo pipefail
+ tmp_bin="$(mktemp /tmp/extract-memmap.XXXXXX)"
+ trap 'rm -f "$tmp_bin"' EXIT
gcc -I "${{ inputs.include-dir }}" \
"$GITHUB_ACTION_PATH/extract.c" \
- -o /tmp/_extract_memmap
- /tmp/_extract_memmap | tee -a "$GITHUB_ENV"
+ -o "$tmp_bin"
+ "$tmp_bin" | tee -a "$GITHUB_ENV"🤖 Prompt for AI Agents |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| #include <stdio.h> | ||
| #include "app_bootloader_interface.h" | ||
|
|
||
| int main(void) { | ||
| printf("MAIN_APP_START=0x%08X\n", EXEC_APP_START_ADDR); | ||
| printf("MAIN_APP_END=0x%08X\n", EXEC_APP_START_ADDR + APP_FLASH_SIZE); | ||
| printf("MAIN_SIG_START=0x%08X\n", APP_SIG_START_ADDR); | ||
| printf("MAIN_SIG_END=0x%08X\n", APP_SIG_END_ADDR); | ||
| printf("MAIN_CRC_ADDR=0x%08X\n", APP_CRC_ADDR); | ||
| printf("BL_APP_START=0x%08X\n", BOOTLOADER_START_ADDR); | ||
| printf("BL_APP_END=0x%08X\n", BOOTLOADER_START_ADDR + BOOT_FLASH_SIZE); | ||
| printf("BL_SIG_START=0x%08X\n", BOOT_SIG_START_ADDR); | ||
| printf("BL_SIG_END=0x%08X\n", BOOT_SIG_END_ADDR); | ||
| printf("BL_CRC_ADDR=0x%08X\n", BOOT_CRC_ADDR); | ||
| return 0; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| .venv/ | ||
| __pycache__/ |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,47 @@ | ||
| name: "Sign Firmware (Ed25519)" | ||
| description: | | ||
| Sign an Intel HEX firmware image with Ed25519 and patch the 64-byte | ||
| signature into a fixed region at app_end - 68. Does NOT compute CRC -- | ||
| run the existing CRC step AFTER this action. | ||
|
|
||
| inputs: | ||
| hex-path: | ||
| description: "Path to the input Intel HEX file" | ||
| required: true | ||
| app-start: | ||
| description: "App slot start address (e.g. 0x08020000)" | ||
| required: true | ||
| app-end: | ||
| description: "App slot end address, exclusive (e.g. 0x08040000)" | ||
| required: true | ||
| output-path: | ||
| description: "Output HEX path (defaults to in-place modification of hex-path)" | ||
| required: false | ||
| default: "" | ||
| private-key: | ||
| description: "PEM-encoded Ed25519 private key contents (pass secrets.FIRMWARE_SIGNING_PRIVATE_KEY)" | ||
| required: true | ||
|
|
||
| runs: | ||
| using: "composite" | ||
| steps: | ||
| - name: Set up Python | ||
| uses: actions/setup-python@v5 | ||
| with: | ||
| python-version: "3.x" | ||
|
|
||
| - name: Sign firmware | ||
| shell: bash | ||
| env: | ||
| FIRMWARE_SIGNING_PRIVATE_KEY: ${{ inputs.private-key }} | ||
| run: | | ||
| set -euo pipefail | ||
| args=( | ||
| --hex "${{ inputs.hex-path }}" | ||
| --app-start "${{ inputs.app-start }}" | ||
| --app-end "${{ inputs.app-end }}" | ||
| ) | ||
| if [ -n "${{ inputs.output-path }}" ]; then | ||
| args+=(--output "${{ inputs.output-path }}") | ||
|
Comment on lines
+40
to
+45
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Avoid direct expression interpolation inside shell commands. Line 40–Line 45 interpolate Suggested hardening - name: Sign firmware
shell: bash
env:
FIRMWARE_SIGNING_PRIVATE_KEY: ${{ inputs.private-key }}
+ HEX_PATH: ${{ inputs.hex-path }}
+ APP_START: ${{ inputs.app-start }}
+ APP_END: ${{ inputs.app-end }}
+ OUTPUT_PATH: ${{ inputs.output-path }}
run: |
set -euo pipefail
args=(
- --hex "${{ inputs.hex-path }}"
- --app-start "${{ inputs.app-start }}"
- --app-end "${{ inputs.app-end }}"
+ --hex "$HEX_PATH"
+ --app-start "$APP_START"
+ --app-end "$APP_END"
)
- if [ -n "${{ inputs.output-path }}" ]; then
- args+=(--output "${{ inputs.output-path }}")
+ if [ -n "$OUTPUT_PATH" ]; then
+ args+=(--output "$OUTPUT_PATH")
fi
sh "$GITHUB_ACTION_PATH/run.sh" "${args[@]}"🤖 Prompt for AI Agents |
||
| fi | ||
| sh "$GITHUB_ACTION_PATH/run.sh" "${args[@]}" | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| #!/bin/sh | ||
| # Entry point for sign_firmware.py that works in three environments: | ||
| # 1. CI containers / dev machines where cryptography + intelhex are already | ||
| # importable from system python3 -> runs sign_firmware.py directly. | ||
| # 2. Dev machines without those modules -> bootstraps a venv at | ||
| # $(dirname "$0")/.venv on first run, installs the deps, then runs from | ||
| # the venv. Subsequent runs reuse the venv. | ||
| # 3. CI runners with no Python at all -> caller must install python3 first; | ||
| # this script does not try to install it. | ||
| set -eu | ||
|
|
||
| SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" | ||
|
|
||
| if python3 -c "import cryptography, intelhex" 2>/dev/null; then | ||
| exec python3 "$SCRIPT_DIR/sign_firmware.py" "$@" | ||
| fi | ||
|
|
||
| VENV="$SCRIPT_DIR/.venv" | ||
| if [ ! -x "$VENV/bin/python" ]; then | ||
| echo "sign-firmware: bootstrapping venv at $VENV (one-time)" >&2 | ||
| python3 -m venv "$VENV" | ||
| "$VENV/bin/pip" install --quiet --upgrade pip | ||
| "$VENV/bin/pip" install --quiet "cryptography>=41" "intelhex>=2.3" | ||
| fi | ||
|
|
||
| exec "$VENV/bin/python" "$SCRIPT_DIR/sign_firmware.py" "$@" |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,184 @@ | ||
| #!/usr/bin/env python3 | ||
| """ | ||
| Sign an Intel HEX firmware image with Ed25519. | ||
|
|
||
| Layout (fixed): | ||
| sig region: 64 bytes at app_end - 68 | ||
| crc region: 4 bytes at app_end - 4 (NOT touched here; computed by a later step) | ||
| sig covers: bytes [app_start, app_end - 68), gaps padded with 0xFF | ||
|
|
||
| Key resolution (in order): | ||
| 1. FIRMWARE_SIGNING_PRIVATE_KEY env var (PEM contents) -- CI path | ||
| 2. --private-key-file FILE -- local explicit path | ||
| 3. --allow-dummy-key -- uses tests/dummy_private_key.pem next to this script | ||
| (committed, non-secret, for local pipeline smoke tests only) | ||
|
|
||
| Usage: | ||
| python sign_firmware.py \\ | ||
| --hex firmware.hex \\ | ||
| --app-start 0x08020000 \\ | ||
| --app-end 0x08040000 \\ | ||
| [--output firmware_signed.hex] \\ | ||
| [--private-key-file key.pem | --allow-dummy-key] | ||
| """ | ||
| from __future__ import annotations | ||
|
|
||
| import argparse | ||
| import os | ||
| import sys | ||
| from pathlib import Path | ||
|
|
||
| from cryptography.exceptions import InvalidSignature | ||
| from cryptography.hazmat.primitives import serialization | ||
| from cryptography.hazmat.primitives.asymmetric.ed25519 import ( | ||
| Ed25519PrivateKey, | ||
| Ed25519PublicKey, | ||
| ) | ||
| from intelhex import IntelHex | ||
|
|
||
| SIG_LEN = 64 | ||
| CRC_LEN = 4 | ||
| TRAILER_LEN = SIG_LEN + CRC_LEN # 68 bytes reserved at end of app slot | ||
| PAD_BYTE = 0xFF # erased flash state on STM32 | ||
|
|
||
|
|
||
| class SigningError(RuntimeError): | ||
| pass | ||
|
|
||
|
|
||
| def parse_address(value: str) -> int: | ||
| text = value.strip().replace("_", "") | ||
| try: | ||
| return int(text, 16) if text.lower().startswith("0x") else int(text, 0) | ||
| except ValueError as exc: | ||
| raise argparse.ArgumentTypeError(f"Invalid address {value!r}") from exc | ||
|
|
||
|
|
||
| def load_private_key(args: argparse.Namespace) -> Ed25519PrivateKey: | ||
| pem = os.environ.get("FIRMWARE_SIGNING_PRIVATE_KEY") | ||
| source = "FIRMWARE_SIGNING_PRIVATE_KEY env" | ||
|
|
||
| if not pem and args.private_key_file: | ||
| pem = Path(args.private_key_file).read_text() | ||
| source = f"file {args.private_key_file}" | ||
|
Comment on lines
+61
to
+63
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: cd actions/sign-firmware && wc -l sign_firmware.pyRepository: minimalx/.github Length of output: 81 🏁 Script executed: cd actions/sign-firmware && head -80 sign_firmware.py | tail -30 && echo "---" && sed -n '60,85p' sign_firmware.py && echo "---" && sed -n '170,185p' sign_firmware.pyRepository: minimalx/.github Length of output: 2672 🏁 Script executed: cd actions/sign-firmware && cat -n sign_firmware.py | grep -A5 -B5 "def main\|SigningError\|load_pem_private_key" | head -80Repository: minimalx/.github Length of output: 3286 🏁 Script executed: cd actions/sign-firmware && cat -n sign_firmware.pyRepository: minimalx/.github Length of output: 7548 🏁 Script executed: python3 << 'EOF'
# Check what exceptions load_pem_private_key can raise
from cryptography.hazmat.primitives import serialization
# Test invalid PEM
test_cases = [
("invalid_pem", "not a valid PEM"),
("empty_pem", ""),
("wrong_format", "-----BEGIN RSA PRIVATE KEY-----\ninvalid\n-----END RSA PRIVATE KEY-----"),
]
exception_types = set()
for name, pem in test_cases:
try:
serialization.load_pem_private_key(pem.encode(), password=None)
except Exception as e:
exception_types.add(type(e).__name__)
print(f"{name}: {type(e).__module__}.{type(e).__name__}: {e}")
print(f"\nException types raised: {exception_types}")
EOFRepository: minimalx/.github Length of output: 614 🏁 Script executed: python3 << 'EOF'
# Check what Path.read_text can raise
from pathlib import Path
import tempfile
import os
# Test 1: Non-existent file
try:
Path("/nonexistent/file.txt").read_text()
except Exception as e:
print(f"Non-existent: {type(e).__module__}.{type(e).__name__}")
# Test 2: Permission denied
with tempfile.TemporaryDirectory() as tmpdir:
test_file = Path(tmpdir) / "test.txt"
test_file.write_text("test")
os.chmod(test_file, 0o000)
try:
test_file.read_text()
except Exception as e:
print(f"Permission denied: {type(e).__module__}.{type(e).__name__}")
finally:
os.chmod(test_file, 0o644)
# Test 3: Invalid UTF-8
with tempfile.TemporaryDirectory() as tmpdir:
test_file = Path(tmpdir) / "invalid.txt"
test_file.write_bytes(b'\x80\x81\x82')
try:
test_file.read_text()
except Exception as e:
print(f"Invalid UTF-8: {type(e).__module__}.{type(e).__name__}")
print("\nAll above inherit from OSError or ValueError")
EOFRepository: minimalx/.github Length of output: 234 🏁 Script executed: rg "UnsupportedAlgorithm" --type pyRepository: minimalx/.github Length of output: 42 🏁 Script executed: grep -n "read_text\|load_pem_private_key" actions/sign-firmware/sign_firmware.pyRepository: minimalx/.github Length of output: 229 Wrap key-loading failures in Currently, Proposed fix from cryptography.exceptions import InvalidSignature
+from cryptography.exceptions import UnsupportedAlgorithm
@@
if not pem and args.private_key_file:
- pem = Path(args.private_key_file).read_text()
+ try:
+ pem = Path(args.private_key_file).read_text(encoding="utf-8")
+ except OSError as exc:
+ raise SigningError(
+ f"Unable to read private key file {args.private_key_file}: {exc}"
+ ) from exc
source = f"file {args.private_key_file}"
@@
elif not pem and args.allow_dummy_key:
dummy = Path(__file__).parent / "tests" / "dummy_private_key.pem"
if not dummy.exists():
raise SigningError(f"Dummy key not found at {dummy}")
- pem = dummy.read_text()
+ try:
+ pem = dummy.read_text(encoding="utf-8")
+ except OSError as exc:
+ raise SigningError(
+ f"Unable to read dummy private key: {exc}"
+ ) from exc
@@
- key = serialization.load_pem_private_key(pem.encode(), password=None)
+ try:
+ key = serialization.load_pem_private_key(pem.encode(), password=None)
+ except (ValueError, TypeError, UnsupportedAlgorithm) as exc:
+ raise SigningError(
+ "Invalid private key PEM. Provide an unencrypted Ed25519 private key."
+ ) from excAlso applies to: line 68 (dummy key read). 🤖 Prompt for AI Agents |
||
| elif not pem and args.allow_dummy_key: | ||
| dummy = Path(__file__).parent / "tests" / "dummy_private_key.pem" | ||
| if not dummy.exists(): | ||
| raise SigningError(f"Dummy key not found at {dummy}") | ||
| pem = dummy.read_text() | ||
| source = "DUMMY KEY (--allow-dummy-key)" | ||
| print( | ||
| "WARNING: signing with the committed dummy key. " | ||
| "DO NOT USE IN PRODUCTION.", | ||
| file=sys.stderr, | ||
| ) | ||
|
|
||
| if not pem: | ||
| raise SigningError( | ||
| "No private key provided. Set FIRMWARE_SIGNING_PRIVATE_KEY env, " | ||
| "pass --private-key-file, or use --allow-dummy-key for local testing." | ||
| ) | ||
|
|
||
| key = serialization.load_pem_private_key(pem.encode(), password=None) | ||
| if not isinstance(key, Ed25519PrivateKey): | ||
| raise SigningError( | ||
| f"Expected an Ed25519 private key, got {type(key).__name__}" | ||
| ) | ||
| print(f"Using private key from: {source}", file=sys.stderr) | ||
| return key | ||
|
|
||
|
|
||
| def extract_payload(ih: IntelHex, app_start: int, sig_offset: int) -> bytes: | ||
| if app_start >= sig_offset: | ||
| raise SigningError( | ||
| f"app_start (0x{app_start:08X}) must be below sig_offset (0x{sig_offset:08X})" | ||
| ) | ||
| ih.padding = PAD_BYTE | ||
| return bytes(ih.tobinarray(start=app_start, size=sig_offset - app_start)) | ||
|
|
||
|
|
||
| def patch_signature(ih: IntelHex, sig_offset: int, signature: bytes) -> None: | ||
| if len(signature) != SIG_LEN: | ||
| raise SigningError(f"Signature must be {SIG_LEN} bytes, got {len(signature)}") | ||
| for i, byte in enumerate(signature): | ||
| ih[sig_offset + i] = byte | ||
|
|
||
|
|
||
| def sign_hex( | ||
| *, | ||
| hex_path: Path, | ||
| app_start: int, | ||
| app_end: int, | ||
| private_key: Ed25519PrivateKey, | ||
| output_path: Path | None = None, | ||
| ) -> Path: | ||
| if app_end - app_start <= TRAILER_LEN: | ||
| raise SigningError( | ||
| f"App slot too small: end-start = {app_end - app_start} bytes, " | ||
| f"need > {TRAILER_LEN}" | ||
| ) | ||
| sig_offset = app_end - TRAILER_LEN | ||
|
|
||
| ih = IntelHex(str(hex_path)) | ||
| payload = extract_payload(ih, app_start, sig_offset) | ||
| signature = private_key.sign(payload) | ||
| patch_signature(ih, sig_offset, signature) | ||
|
|
||
| out = output_path or hex_path | ||
| ih.write_hex_file(str(out)) | ||
| print( | ||
| f"Signed {len(payload)} bytes from 0x{app_start:08X} to 0x{sig_offset:08X}; " | ||
| f"signature written at 0x{sig_offset:08X} in {out}", | ||
| file=sys.stderr, | ||
| ) | ||
| return out | ||
|
|
||
|
|
||
| def verify_signed_hex( | ||
| *, | ||
| hex_path: Path, | ||
| app_start: int, | ||
| app_end: int, | ||
| public_key: Ed25519PublicKey, | ||
| ) -> bool: | ||
| sig_offset = app_end - TRAILER_LEN | ||
| ih = IntelHex(str(hex_path)) | ||
| payload = extract_payload(ih, app_start, sig_offset) | ||
| signature = bytes(ih.tobinarray(start=sig_offset, size=SIG_LEN)) | ||
| try: | ||
| public_key.verify(signature, payload) | ||
| return True | ||
| except InvalidSignature: | ||
| return False | ||
|
|
||
|
|
||
| def parse_args(argv: list[str]) -> argparse.Namespace: | ||
| parser = argparse.ArgumentParser(description=__doc__, formatter_class=argparse.RawDescriptionHelpFormatter) | ||
| parser.add_argument("--hex", required=True, type=Path, help="Input Intel HEX file") | ||
| parser.add_argument("--app-start", required=True, type=parse_address, help="App slot start address") | ||
| parser.add_argument("--app-end", required=True, type=parse_address, help="App slot end address (exclusive)") | ||
| parser.add_argument("--output", type=Path, default=None, help="Output hex (default: in-place)") | ||
| parser.add_argument("--private-key-file", type=Path, default=None, help="Path to PEM private key") | ||
| parser.add_argument("--allow-dummy-key", action="store_true", help="Use committed dummy key for local testing") | ||
| return parser.parse_args(argv) | ||
|
|
||
|
|
||
| def main(argv: list[str]) -> int: | ||
| args = parse_args(argv) | ||
| try: | ||
| priv = load_private_key(args) | ||
| sign_hex( | ||
| hex_path=args.hex, | ||
| app_start=args.app_start, | ||
| app_end=args.app_end, | ||
| private_key=priv, | ||
| output_path=args.output, | ||
| ) | ||
| except SigningError as exc: | ||
| print(f"sign-firmware: {exc}", file=sys.stderr) | ||
| return 1 | ||
| return 0 | ||
|
|
||
|
|
||
| if __name__ == "__main__": | ||
| sys.exit(main(sys.argv[1:])) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| -----BEGIN PRIVATE KEY----- | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛑 Gitleaks has detected a secret with rule-id If this secret is a false positive, you can add the fingerprint below to your |
||
| MC4CAQAwBQYDK2VwBCIEIONneOHFBbq8a5/pmSWd0Ol3A5kOOHz8jW/5T5RkR+Sr | ||
| -----END PRIVATE KEY----- | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| -----BEGIN PUBLIC KEY----- | ||
| MCowBQYDK2VwAyEAulpl/okOwDXiz5hkCyp1fXSY2vPMOiF0dxjhFMSKIT0= | ||
| -----END PUBLIC KEY----- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fail closed when the signing key is missing.
The current default (
private-key: '') plus documented dummy fallback makes the signing path fail-open in CI. If the secret is absent/miswired, the pipeline can still produce “signed” artifacts with a known test key. Secure CI coding standards usually require explicit opt-in for dummy keys and otherwise hard-fail.Suggested hardening diff
Also applies to: 51-54
🤖 Prompt for AI Agents