Skip to content

docs(rpc): say what the HTTP servers trust their callers with - #778

Open
MegaRedHand wants to merge 4 commits into
feat/beacon-builder-registrationfrom
docs/beacon-api-trust
Open

MegaRedHand wants to merge 4 commits into
feat/beacon-builder-registrationfrom
docs/beacon-api-trust

Conversation

@MegaRedHand

Copy link
Copy Markdown
Collaborator

Neither HTTP server authenticates its callers, and nothing in docs/rpc.md says so. This adds a "What the servers trust" section under "Servers & Ports", which pablodeymo asked for in #696's security read. It is stacked on #696 because the note covers register_validator, which that PR adds.

The section says:

  • The default is loopback. That is why --http-address defaults to 127.0.0.1. The port should be exposed only behind a firewall rule or an authenticating proxy, never to the internet.
  • What the node checks for itself: everything it gossips on a caller's behalf, signatures included. That covers blocks, envelopes, attestations, aggregates, sync committee messages and contributions, operations, payload attestations, bids and proposer preferences. A failure is a 400, so a caller cannot make the node publish what its peers would refuse.
  • What it takes on the caller's word:
    • fee recipients from prepare_beacon_proposer and register_validator. The registration's signature is not verified here (Lighthouse doesn't verify it either), and the fee recipient is set whatever the builder answers;
    • subnet subscriptions;
    • block production requests;
    • the lean aggregator toggle;
    • heap profiles on the metrics server.

Docs only, no code changes.

https://claude.ai/code/session_011vghNjQ8uikd9KL5Jo5at7

Neither server authenticates its callers. List what the node checks for
itself (everything it gossips on a caller's behalf) and what it takes on
the caller's word: fee recipients from prepare_beacon_proposer and
register_validator, subnet subscriptions, block production, the lean
aggregator toggle and heap profiles. Hence the loopback default for
--http-address.

Claude-Session: https://claude.ai/code/session_011vghNjQ8uikd9KL5Jo5at7
@github-actions

github-actions Bot commented Nov 5, 2026

Copy link
Copy Markdown

🤖 Grok Code Review

I'll verify the new RPC trust documentation against the actual server and validation paths so the review isn't just stylistic.


Automated review by Grok (xAI) · grok-4.7 · custom prompt

@github-actions

github-actions Bot commented Nov 5, 2026

Copy link
Copy Markdown

🤖 Gemini Code Review

The documentation addition is clear, accurate, and clearly outlines the RPC trust model and security boundaries for operators. Looks good to merge.


Automated review by Gemini · gemini-4-argon · custom prompt

@github-actions

github-actions Bot commented Nov 5, 2026

Copy link
Copy Markdown

🤖 Claude Code Review

Review: PR 778 (docs/rpc.md, "What the servers trust")

This is a docs-only change, so the main risk is a security statement that doesn't match the code. Writing down the trust model is worthwhile, and the warning about the default bind address is good. A few claims need checking or softening.

Accuracy and over-claims

  1. "A caller cannot make this node publish what its peers would refuse" (around line 41). This is stronger than the code likely guarantees. Gossip validation has IGNORE conditions that depend on time and on what a peer has already seen, such as duplicates, future or past slot windows, and equivocations under broadcast_validation=gossip. A local check cannot fully match those.

    • Suggested wording: "…cannot make this node publish a message with an invalid signature or one that fails validation."
    • Also make sure the list of message types (sync committee, BLS changes, envelopes, data columns, payload attestations, builder bids, proposer preferences) matches handlers that exist and actually verify signatures. A lean node uses XMSS, not BLS, so state which signature scheme applies where.
    • If any listed endpoint skips verification, this paragraph becomes a false security guarantee. That is worse than having no docs.
  2. Default bind address (around line 30). Confirm that --http-address really defaults to 127.0.0.1 in the CLI definition. Also confirm it controls both the API and metrics listeners. The text implies one flag governs both servers.

  3. "(neither does Lighthouse)" (around line 51). Referencing another client's behavior is brittle and can go stale. Either drop it or link a source.

Security substance worth surfacing

  1. register_validator fee-recipient hijack (lines 47–55). The doc says the window lasts "until the validator client sends its own again." But an attacker can simply resend every slot and win the race indefinitely, so the window is effectively unbounded while the attacker has access.

    • This path is avoidable. The registration carries a signature, and verifying it locally costs one BLS verify per registration.
    • Please consider a follow-up issue to either verify it, or skip the fee-recipient update when the builder rejects the registration.
    • Neither fix is possible for prepare_beacon_proposer, which is unsigned by spec. That asymmetry is worth stating explicitly.
  2. Block production DoS (line 60). Every block production request makes the execution client build a payload, which lets any caller drive EL CPU load. Add the same note you give for subscriptions: it costs CPU and bandwidth with no rate limit.

  3. Missing expensive read endpoints. Debug and state endpoints (full BeaconState SSZ, /eth/v*/debug/*) are memory- and CPU-heavy. They belong under the same "costs resources" category even though they are read-only.

  4. pprof (lines 63–64). Heap profiles can leak internal layout and allocation-site details. Say whether profiling is on by default, which build or flag enables it, and recommend leaving it off in production.

  5. Aggregator toggle (lines 61–62). Mention the consequence: turning it off can silently stop this node's aggregation duty, which affects justification on networks with few aggregators.

Minor

  • The section is a ### heading under the preceding ##, but it governs both servers. Consider promoting it to ##, or moving it above the per-server sections, so it isn't read as part of the port-merging note.
  • "an envelope's signature and data columns" is ambiguous. Data columns don't carry a proposer signature; they are checked through inclusion proofs and KZG. Say "data columns (inclusion proof and KZG)".

Overall this is a useful addition. Items 1 and 4 should be addressed or confirmed before merge, because operators will rely on these guarantees.


Automated review by Claude · claude-opus-5-5 · custom prompt

@github-actions

github-actions Bot commented Nov 5, 2026

Copy link
Copy Markdown

🤖 Codex Code Review

  • [P2] Narrow the guarantee about peer acceptance — docs/rpc.md:35–41. Block publication verifies the proposer signature and blob proofs, but does not run all incoming gossip checks before broadcasting. For example, a correctly signed block for a future slot can pass post_block and be published directly by publish_beacon_block, while peers ignore it via gossip/block.rs’s future-slot check. Replace “A caller cannot make this node publish what its peers would refuse” with the specific validation guarantees actually enforced. This security documentation currently overstates the protection.

Automated review by OpenAI Codex · gpt-6.1-sol · custom prompt

@github-actions

github-actions Bot commented Nov 5, 2026

Copy link
Copy Markdown

🤖 Kimi Code Review

The security guidance is useful, but a few claims should be tightened:

  1. docs/rpc.md:36-42 — local validation does not guarantee peer acceptance.
    “A caller cannot make this node publish what its peers would refuse” is too strong. Peers may reject otherwise valid messages because of a different chain view, timing, duplicates/equivocations, or gossip-specific rules. Suggested replacement: “Requests that fail the node’s local validation are rejected and are not gossiped.” Also scope “a failure is a 400” specifically to request/validation failures; execution-client, builder, and internal failures may reasonably return other statuses.

  2. docs/rpc.md:53-55 — do not imply the epoch refresh limits the fee-recipient attack.
    Validator-client refresh cadence is not a security guarantee: clients may be offline or behave differently, and an unauthorized caller can repeat the request. Prefer “until another request overwrites it,” with the normal epoch refresh mentioned only as typical behavior, if retained.

  3. docs/rpc.md:56-65 — consider making the availability impact explicit.
    Subscriptions, repeated payload construction, aggregator changes, and heap profiling are unauthenticated resource-consumption or operational-control vectors. The pprof entry should also note that profiles expose internal diagnostic information and can be expensive to collect; this reinforces why loopback or authenticated access control is required.


Automated review by Kimi (Moonshot AI) · kimi-k3.1-pro · custom prompt

@github-actions

github-actions Bot commented Nov 5, 2026

Copy link
Copy Markdown

🤖 DeepSeek Code Review

This PR only touches docs/rpc.md, so there are no code-level correctness or memory-safety issues to review. The new trust-model section is a useful addition and the overall warning to bind only on 127.0.0.1 is appropriate.

A few documentation issues in the added What the servers trust section:

  • The first paragraph lists “proposer preferences” as one of the things checked “against the chain, signatures included,” but later POST /eth/v1/validator/prepare_beacon_proposer is correctly listed under what the node takes on the caller’s word, with no signature. Those are effectively the same proposer-preparation/preferences data, so the first list contradicts the later trust boundary. Remove “proposer preferences” from the checked list or clarify that it refers to a different object.

  • The register_validator explanation overstates the impact of not verifying the registration signature. The doc says the builder does verify, so an attacker without the validator key cannot redirect that validator’s builder-built rewards by submitting an invalid registration. The locally-built-block reward redirection is caused by prepare_beacon_proposer, not by unverified register_validator. The sentence “A caller can therefore redirect…” combines these two cases and is misleading.

  • The claim about Lighthouse behavior, “neither does Lighthouse,” is an external-behavior assertion that may become stale or be inaccurate. Better to document only ethlambda’s behavior unless it is tested and version-specific.

Minor wording/clarity:

  • /sync_committee_subscriptions is shorthand; use the full path /eth/v1/validator/sync_committee_subscriptions for consistency.
  • “A failure is a 400” is a broad generalization; some APIs may return 200, 202, or 500 depending on the validation path or internal failure. Consider saying validation failures use the endpoint’s documented error status.

Overall, the documentation direction is good; the main fixes are to remove the contradiction around proposer preparations and clarify the register_validator trust boundary.


Automated review by DeepSeek · deepseek-v4-pro · custom prompt

This branch has not been deployed

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant