Skip to content

fix: harden attestation signing fallback#6868

Open
vicentsmith470-web wants to merge 1 commit into
Scottcjn:mainfrom
vicentsmith470-web:vicentsmith/attestation-signing-hardening
Open

fix: harden attestation signing fallback#6868
vicentsmith470-web wants to merge 1 commit into
Scottcjn:mainfrom
vicentsmith470-web:vicentsmith/attestation-signing-hardening

Conversation

@vicentsmith470-web
Copy link
Copy Markdown
Contributor

Summary

Fixes #6859 by hardening the attestation-signing follow-ups from the #6839 review.

What changed:

  • setup_miner.py now downloads and SHA-256 verifies miners/signing_helpers.py next to the installed miner, so installed Linux/Windows miners can use the shared pipe-message builder instead of always taking the inline fallback.
  • The shell installers also download and verify signing_helpers.py via miners/checksums.sha256.
  • Linux and Windows miners now log a WARNING if Ed25519 attestation signing fails instead of silently swallowing the exception.
  • Added regression coverage for the installed-miner inline fallback path by driving Linux LocalMiner.attest() with _SIGNING_HELPERS=False and asserting the signed bytes match the node verifier reconstruction (miner_id|miner|nonce|commitment).
  • Added regression coverage that signing failures emit a warning and fall through unsigned.

Scope notes

Validation

python -m py_compile miners\linux\rustchain_linux_miner.py miners\windows\rustchain_windows_miner.py setup_miner.py tests\test_attestation_signing_6798.py tests\test_setup_miner_downloads.py tests\test_install_miner_checksums.py
python -m pytest -q tests\test_attestation_signing_6798.py tests\test_linux_miner_identity.py tests\test_linux_miner_network_retry.py tests\test_setup_miner_downloads.py tests\test_install_miner_checksums.py
20 passed
git diff --check

Bounty / payout

Wallet: RTCf69dd944558d4e843a4a676495a97638055caea2

@github-actions github-actions Bot added BCOS-L1 Beacon Certified Open Source tier BCOS-L1 (required for non-doc PRs) tests Test suite changes size/L PR: 201-500 lines labels Jun 5, 2026
@vicentsmith470-web vicentsmith470-web force-pushed the vicentsmith/attestation-signing-hardening branch 2 times, most recently from 1b04231 to 504cf09 Compare June 5, 2026 02:58
@vicentsmith470-web
Copy link
Copy Markdown
Contributor Author

Validation update after rebasing on current main and syncing the fetchall baseline line drift:

  • python -m pytest -q tests\test_windows_miner_setup_checksum.py tests\test_setup_miner_downloads.py tests\test_install_miner_checksums.py tests\test_attestation_signing_6798.py tests\test_linux_miner_identity.py tests\test_linux_miner_network_retry.py -> 23 passed
  • python -m py_compile miners\linux\rustchain_linux_miner.py miners\windows\rustchain_windows_miner.py setup_miner.py tests\test_attestation_signing_6798.py tests\test_setup_miner_downloads.py tests\test_install_miner_checksums.py tests\test_windows_miner_setup_checksum.py -> passed
  • git diff --check origin/main -> passed

CI is green on the updated head.

@Scottcjn
Copy link
Copy Markdown
Owner

Scottcjn commented Jun 5, 2026

This is a clean, complete follow-up to #6859 — thank you. I verified it addresses all three items:

  1. Distributes signing_helpers.py — both installers download it and SHA-256 verify it against miners/checksums.sha256 (new entry added), and setup_miner.py pins it with a test (test_setup_miner_pins_current_signing_helper_artifact). Installed miners now use the shared pipe-message builder instead of always falling to the inline copy.
  2. Logs on signing failure — the silent except: pass is now logging.warning(...) on both Linux and Windows, so a broken keypair/helper is diagnosable. Bonus: you re-enabled TLS certificate verification (dropping the old verify=False path) — a real security improvement.
  3. Tests the inline fallbacktest_linux_miner_inline_fallback_matches_node_verifier exercises the path real installs hit and asserts byte-equality with the node verifier's reconstruction, plus test_linux_miner_warns_when_attestation_signing_fails.

Also verified: the checksum manifest matches the actual miner files (no stale-manifest issue), and it's only 2 commits behind main. The signing algorithm is unchanged from the verified #6839 fix — this is purely the hardening/distribution/logging/test layer — so no fresh crypto review needed.

One thing holding the green check: main itself currently has a red test gate (a stale fetchall-guard baseline — line-number drift, fix in flight at #6870). Once #6870 lands, rebase onto main and your CI should go green, then I'll merge. Solid work. 🦞

@vicentsmith470-web vicentsmith470-web force-pushed the vicentsmith/attestation-signing-hardening branch from 504cf09 to 224578b Compare June 5, 2026 03:11
Copy link
Copy Markdown
Contributor

@jaxint jaxint left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution! 🎉

Copy link
Copy Markdown
Contributor

@jaxint jaxint left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work! 👍

Copy link
Copy Markdown
Contributor

@JesusMP22 JesusMP22 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review: Harden attestation signing fallback

Summary: Improves the robustness of attestation signing by hardening the fallback mechanism, ensuring miners can still attest even when primary signing fails.

What I like:

  • Fallback mechanisms are critical for system reliability
  • Attestation is core to RustChain's trust model

Suggestions:

  1. Document the fallback signing path and its security properties
  2. Add tests that simulate primary signing failure and verify fallback works
  3. Consider whether fallback signatures should be distinguishable from primary ones
  4. Ensure the fallback doesn't introduce a weaker security guarantee

Security considerations:

  • Fallback paths are often where security weaknesses hide
  • Ensure the fallback signing key has the same protection as the primary
  • Consider whether fallback could be exploited to submit fraudulent attestations

Verdict: ✅ Good reliability improvement. Fallback hardening is important for production systems.

Copy link
Copy Markdown
Contributor

@jaxint jaxint left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work! Thanks for contributing.

Copy link
Copy Markdown
Contributor

@jaxint jaxint left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution! 🎉

Copy link
Copy Markdown
Contributor

@jaxint jaxint left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this contribution! Great work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

BCOS-L1 Beacon Certified Open Source tier BCOS-L1 (required for non-doc PRs) size/L PR: 201-500 lines tests Test suite changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Follow-up to #6839: harden attestation-signing fix (test the fallback, log swallowed errors, Mac coverage)

4 participants