Skip to content

Require server-issued challenges for attestation submit#1756

Open
Mavline wants to merge 3 commits intoScottcjn:mainfrom
Mavline:codex/attestation-replay-timestamp-bypass
Open

Require server-issued challenges for attestation submit#1756
Mavline wants to merge 3 commits intoScottcjn:mainfrom
Mavline:codex/attestation-replay-timestamp-bypass

Conversation

@Mavline
Copy link
Contributor

@Mavline Mavline commented Mar 22, 2026

Follow-up to #1746.

This tightens the attestation nonce contract so /attest/submit only accepts live server-issued challenges from /attest/challenge. The previous merged path could still be bypassed by supplying client-controlled timestamp fields, and in-tree pico_bridge still had an insecure local-nonce fallback.

What changed:

  • require a live challenge nonce for every attestation submit
  • stop consulting client-provided timestamp fields during nonce validation
  • keep replay tracking in used_nonces after successful challenge consumption
  • fail pico_bridge closed when challenge fetch fails instead of synthesizing a local nonce
  • add regression coverage for arbitrary nonce rejection, timestamp-bypass rejection, and pico fail-closed behavior

Validation:

  • /tmp/rustchain-test-venv/bin/python -m unittest node/tests/test_attest_nonce_replay.py node/tests/test_attest_submit_challenge_binding.py
  • /tmp/rustchain-test-venv/bin/python miners/pico_bridge/tests/test_pico_bridge_miner.py

Important deployment note:

  • this PR keeps the existing node-local challenge store model; multi-backend deployments still need sticky routing or a shared nonce store for first-submit success across nodes.

@Mavline Mavline requested a review from Scottcjn as a code owner March 22, 2026 04:36
@github-actions github-actions bot added BCOS-L1 Beacon Certified Open Source tier BCOS-L1 (required for non-doc PRs) node Node server related tests Test suite changes labels Mar 22, 2026
@github-actions
Copy link

Welcome to RustChain! Thanks for your first pull request.

Before we review, please make sure:

  • Your PR has a BCOS-L1 or BCOS-L2 label
  • New code files include an SPDX license header
  • You've tested your changes against the live node

Bounty tiers: Micro (1-10 RTC) | Standard (20-50) | Major (75-100) | Critical (100-150)

A maintainer will review your PR soon. Thanks for contributing!

@Mavline
Copy link
Contributor Author

Mavline commented Mar 23, 2026

Review-ready. This PR is a focused security hardening follow-up to merged PR #1746.

It closes the remaining client-controlled timestamp / arbitrary-nonce path in /attest/submit and makes pico_bridge fail closed
when a live server-issued challenge cannot be fetched. The targeted regression coverage listed in the PR body is green.

Happy to answer any review questions or adjust the patch if needed.

@Scottcjn
Copy link
Owner

Review: Approve

Excellent security hardening. Removing the local nonce fallback and requiring server-issued challenges closes a real replay attack surface. Test coverage is thorough.

One minor: expires_at = int(challenge_expires_at) could crash if None — add a guard. Also a breaking change for multi-node setups (documented in the docstring, but worth release notes).

💰 75 RTC — solid security contribution @Mavline.

Please share your RTC wallet address.

@Scottcjn
Copy link
Owner

Excellent security hardening — removing the client-supplied timestamp attack surface is the right call. Has a merge conflict with main though. Can you rebase? Will merge immediately after.

@Scottcjn
Copy link
Owner

Update: CI fix just landed on main (removed broken blake2 pip dep). The BCOS scan should pass now. Just need a rebase to resolve merge conflicts, then I'll merge immediately. git fetch origin && git rebase origin/main && git push --force-with-lease

@Scottcjn
Copy link
Owner

Still approved — excellent security hardening. Just needs rebase against current main. The BCOS scan CI is being fixed separately.

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) node Node server related size/L PR: 201-500 lines tests Test suite changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants