Repository navigation
fix(l2ps): bind the history signature to the host and the page parameters - #1050
Conversation
The history read signs only the subnet, the address and a timestamp. The version 2 message also names the node the caller means and the page parameters, so a relayed request cannot be presented to another node or repointed at a different window. This adds the message builder and the check that a host names this node (its ed25519 address, or the hostname of a non-loopback advertised URL); the handler is wired in the next commit.
…ters
A request with signatureVersion 2 must sign
getL2PSHistory/2:{l2psUid}:{address}:{timestamp}:{host}:{limit}:{offset}:{since}
where host is this node's address or advertised hostname and the page
values are the ones applied to the query. A signature made for another
host, or with different limit, offset or since, no longer verifies.
The unversioned message is still accepted for existing clients and keeps
its behaviour, including the unsigned page parameters, until it is retired.
…tory signature Off by default so existing clients keep working. When on, the history read refuses the legacy signature, which names neither the node nor the page.
Page values are read without coercion, so a hostile object can no longer throw before the signature is checked. signatureVersion is compared strictly and anything but 2 or "2" is refused rather than treated as the legacy form. The version 2 string is joined with ':', so its timestamp must be digits only and the subnet uid and address must not contain ':'. A bare advertised hostname identifies a node only when the advertised URL uses the default port; otherwise host:port is required, so two nodes behind one address no longer share a binding. When L2PS_HISTORY_REQUIRE_V2 is on, legacy requests get a 401 whose hint lists the version 2 form first.
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.
Tip: disable this comment in your organization's Code Review settings.
|
ⓘ Qodo reviews are paused because the subscription is no longer active. Ask your workspace admin to reactivate the subscription to resume reviews. Manage billing |
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configuration
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
A loopback advertised URL is not an identity, but only 127.0.0.1 and localhost were recognised, so two nodes advertising 127.0.0.2 shared a host binding; the whole 127/8 block, bracketed ::1 and the IPv4-mapped form now count as loopback everywhere the check is used. A supplied limit of zero fell through to the default page because the check was truthiness, so the signed clamped value and the applied value disagreed; only a missing value takes the default now, and a non-finite number is treated as missing. An explicit null signatureVersion selected the legacy form instead of being refused as malformed.
The URL parser rewrites ::ffff:127.0.0.1 to ::ffff:7f00:1 before the loopback check sees it, so the mapped form slipped through as an identity. The handler test now names the host exactly as the parser normalises it, so only the loopback rule can refuse the binding.
…tory-signature-binding
Closes #1039.
The authenticated history read verified a signature over
getL2PSHistory:{l2psUid}:{address}:{timestamp}. The host and the page parameters were outside the signed string, so anything able to relay the request could present the signature to another node serving the same subnet, or change which page the caller got back.What changes
signatureVersion: 2in the request:hostmust be this node: its ed25519 address as/inforeports it underidentity(case and0xignored), or the hostname of its advertised URL (host:portwhen the URL has a non-default port; a loopback or empty advertised URL never matches).limit,offset,sinceare the values the node applies, as plain integers: defaults 100, 0, 0, limit clamped to 1..1000. A mismatch fails verification.:, andsignatureVersionmust be exactly 2 or "2" (anything else is a 400, never a silent fallback).L2PS_HISTORY_REQUIRE_V2(default off) lets an operator retire it; the 401 hint now lists the v2 form first and says the node address is always accepted as host.limitcan no longer throw before the signature check.Tests
bun test src/libs/network/handlers/: 44 pass (42 new). They drive the real handler with the real signature verification; only the executor, mempool, logger and shared state are mocked. Cases: v2 happy path with the address in different case and prefix and with the advertised hostname with and without port; wrong and swapped host; loopback advertised URL; each page value differing from what was signed; missing fields; no host; non-integer limit; a legacy signature presented as v2 and the reverse; stale timestamps for both forms; strictsignatureVersion; non-canonical timestamp and uid; the require-v2 switch.SDK side
l2psHistoryAuthMessagein the SDK must build the same string, sendsignatureVersion: 2and the samehost, sign the applied page values, and prefer the node's/infoidentity as host. Tracked on the SDK side.