Skip to content

fix(p2p): require auth on /p2p/gossip POST + correct state root endianness - #8198

Open
rebel117 wants to merge 1 commit into
Scottcjn:mainfrom
rebel117:fix-8177-p2p-gossip-auth
Open

fix(p2p): require auth on /p2p/gossip POST + correct state root endianness#8198
rebel117 wants to merge 1 commit into
Scottcjn:mainfrom
rebel117:fix-8177-p2p-gossip-auth

Conversation

@rebel117

@rebel117 rebel117 commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes #8177 — two findings addressed.

Finding 1: /p2p/gossip POST has no auth

Every P2P read endpoint (/p2p/state, /p2p/attestation_state, /p2p/peers) calls _require_p2p_read_auth() to validate the X-P2P-Key header. The gossip POST — the write endpoint that feeds CRDT merges via p2p_node.handle_gossip() — had only per-IP rate limiting, no auth.

That meant any network-accessible attacker could POST gossip messages without knowing the P2P secret, potentially injecting fake attestation records or corrupting epoch state.

Fix: Added _require_p2p_read_auth() at the top of the receive_gossip handler, before the rate limit check.

Finding 2: State root endianness mismatch

compute_box_id() uses to_bytes(8, "big") and to_bytes(2, "big") for all integer encoding. The state root merkle tree leaf computation used len(rows).to_bytes(8, "little") for the count prefix — inconsistent with the rest of the hashing code.

Fix: Changed to to_bytes(8, "big") so the state root is deterministically reproducible across implementations assuming uniform big-endian encoding.

Testing

  • Added two tests: test_p2p_gossip_requires_auth_header (verifies 401 without key) and test_p2p_gossip_accepts_valid_auth (verifies 200 with valid key)
  • Updated existing gossip tests to include the X-P2P-Key header
  • pytest node/tests/test_p2p_gossip_routes.py — 6/6 pass
test_p2p_gossip_requires_json_object PASSED
test_p2p_gossip_forwards_valid_object_body PASSED
test_p2p_gossip_rejects_oversized_payload_before_handler PASSED
test_gossip_message_rejects_payload_that_exceeds_serialized_cap PASSED
test_p2p_gossip_requires_auth_header PASSED
test_p2p_gossip_accepts_valid_auth PASSED

The /p2p/gossip POST endpoint — the write path that feeds CRDT merges
and can inject attestation records or epoch state — had no auth gate,
while every P2P GET endpoint required X-P2P-Key. Added the same
_require_p2p_read_auth() check to the gossip handler so unauthenticated
callers can't write to the CRDT.

Also fixed the state root merkle tree count prefix from little-endian
to big-endian to match the convention used by compute_box_id and the
rest of the UTXO hashing code.
@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Welcome to RustChain! Thanks for your first pull request.

Before we review, please make sure:

  • Non-doc PRs have a BCOS-L1 or BCOS-L2 label
  • Doc-only PRs are exempt from BCOS tier labels when they only touch docs/**, *.md, or common image/PDF files
  • 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!

@github-actions github-actions Bot added BCOS-L1 Beacon Certified Open Source tier BCOS-L1 (required for non-doc PRs) BCOS-L2 Beacon Certified Open Source tier BCOS-L2 (required for non-doc PRs) node Node server related tests Test suite changes size/S PR: 11-50 lines labels Aug 7, 2026

@FlintLeng FlintLeng left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

PR Review: P2P Gossip POST Auth + State Root Endianness Fix

Reviewed on: 2026-08-13

Summary

Two fixes in one PR:

  1. Auth fix: /p2p/gossip POST now requires X-P2P-Key header, matching all other P2P endpoints
  2. Endianness fix: compute_state_root() uses big-endian for count bytes, matching the Rust implementation

Auth Fix ✅

The vulnerability: Every other P2P endpoint (/p2p/sync, /p2p/peers) required X-P2P-Key auth, but /p2p/gossip POST did not. Anyone could feed CRDT merges into the node without authentication.

The fix is correct: Reuses the existing _require_p2p_read_auth() function at the top of receive_gossip(). The auth check runs before the rate limit and the expensive verify+CRDT work, so unauthenticated requests are rejected early.

Test updates: All existing tests now include the X-P2P-Key header. Two new tests:

  • test_p2p_gossip_requires_auth_header — rejects POST without header (401)
  • test_p2p_gossip_accepts_valid_auth — accepts POST with correct header (200)

Endianness Fix ✅

The bug: compute_state_root() used to_bytes(8, 'little') for the row count. The Rust implementation uses big-endian. The two nodes would compute different state roots for identical state, breaking cross-language consensus.

The fix is correct: 'little''big'. Big-endian is the conventional choice for network protocols and cryptographic hashes (network byte order).

Minor Note

This PR combines two unrelated fixes. Normally I'd prefer separate PRs, but both are small and one-line each. Acceptable in this case.

Wallet: RTC019e78d600fb3131c29d7ba80aba8fe644be426e

✅ LGTM — two correct fixes that close an auth gap and fix cross-language consensus.

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) BCOS-L2 Beacon Certified Open Source tier BCOS-L2 (required for non-doc PRs) node Node server related size/S PR: 11-50 lines tests Test suite changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[SECURITY] /p2p/gossip POST has no auth + state root endianness inconsistency

2 participants