Skip to content

fix: require ownership proof for /epoch/enroll#2117

Merged
Scottcjn merged 1 commit intoScottcjn:mainfrom
createkr:feat/issue-enroll-signature-verification
Apr 6, 2026
Merged

fix: require ownership proof for /epoch/enroll#2117
Scottcjn merged 1 commit intoScottcjn:mainfrom
createkr:feat/issue-enroll-signature-verification

Conversation

@createkr
Copy link
Copy Markdown
Contributor

@createkr createkr commented Apr 6, 2026

Fix: Add Ed25519 signature verification to /epoch/enroll

Problem

POST /epoch/enroll accepted arbitrary miner_pubkey without cryptographic proof of ownership. Any caller who knew a pubkey with a recent attestation could enroll it, enabling:

  • Miner ID hijacking via INSERT OR REPLACE INTO miner_header_keys
  • Enrollment race attacks (attacker enrolls victim first with low weight)

Changes

Server-side (node/rustchain_v2_integrated_v2.2.1_rip200.py)

  1. record_attestation_success(): Added signing_pubkey parameter; stores the Ed25519 public key from attestation in miner_attest_recent.signing_pubkey (idempotent ALTER TABLE migration).
  2. enroll_epoch(): When signature + public_key are provided:
    • Looks up the stored signing pubkey from the miner's attestation
    • Verifies the enrollment pubkey matches the attestation pubkey (PUBKEY_MISMATCH → 400)
    • Verifies the Ed25519 signature over miner_pubkey|miner_id|epoch (INVALID_ENROLLMENT_SIGNATURE → 400)
    • If pynacl is unavailable but signature provided → fail-closed (ED25519_UNAVAILABLE → 503)
    • Unsigned requests still accepted (warn-only) for backward compatibility

Client-side (rustchain-miner/src/miner.rs, rustchain-miner/src/attestation.rs)

  1. Miner struct: Added signing_key: SigningKey and public_key_hex: String fields.
  2. Miner::new(): Generates Ed25519 keypair at startup (reused for attestation + enrollment).
  3. do_attestation(): Uses new attest_with_key() function with the stored keypair.
  4. enroll(): Signs miner_pubkey|miner_id|epoch with the same keypair used in attestation.
  5. attestation.rs: Added attest_with_key() function (accepts pre-generated keypair); original attest() retained for backward compatibility.

Tests (node/tests/test_enroll_signature_verification.py)

7 test cases:

  • ✅ Signed enrollment accepted after attestation
  • ✅ Wrong keypair rejected (PUBKEY_MISMATCH)
  • ✅ Invalid/bogus signature rejected (INVALID_ENROLLMENT_SIGNATURE)
  • ✅ Tampered miner_id rejected (INVALID_ENROLLMENT_SIGNATURE)
  • ✅ Unsigned enrollment accepted (backward compat)
  • ✅ Incomplete signature (sig-only or pubkey-only) rejected
  • ✅ Attacker's keypair can't enroll victim's pubkey

Compatibility

  • Backward compatible: Unsigned enrollment requests are still accepted (warn-only log). Legacy miners continue working.
  • Schema migration: Idempotent ALTER TABLE miner_attest_recent ADD COLUMN signing_pubkey TEXT — safe to run on existing databases.
  • No breaking changes: The attest() function is retained alongside the new attest_with_key().

Verification

  • Rust: cargo check passes (no warnings)
  • Python: All 7 tests pass (run individually due to pre-existing Prometheus metrics registration issue in test infrastructure)
  • Existing tests: test_attest_signature_verification.py and test_attestation_overwrite_reward_loss.py unaffected

@createkr createkr requested a review from Scottcjn as a code owner April 6, 2026 04:30
@github-actions github-actions bot added BCOS-L1 Beacon Certified Open Source tier BCOS-L1 (required for non-doc PRs) miner Miner client related node Node server related tests Test suite changes labels Apr 6, 2026
@createkr
Copy link
Copy Markdown
Contributor Author

createkr commented Apr 6, 2026

For bounty payout, please use RTC wallet: RTC1d48d848a5aa5ecf2c5f01aa5fb64837daaf2f35.

@Scottcjn
Copy link
Copy Markdown
Owner

Scottcjn commented Apr 6, 2026

Merged. Payment: 50 RTC. Addresses #2116.

Adds Ed25519 signature verification on /epoch/enroll — stores signing_pubkey from attestation, verifies enrollment caller is the same entity. Backward-compatible (unsigned requests accepted with warning for legacy miners). This closes the miner_id hijack vector properly.

@Scottcjn Scottcjn merged commit c2b049d into Scottcjn:main Apr 6, 2026
8 of 9 checks passed
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) miner Miner client related node Node server related size/XL PR: 500+ lines tests Test suite changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants