Skip to content

Refuse a loose RPC cookie, and land the 1062 hygiene follow-ups - #1074

Merged
reardencode merged 18 commits into
masterfrom
harden/cookie-and-audit
Oct 19, 2026
Merged

reardencode merged 18 commits into
masterfrom
harden/cookie-and-audit

Conversation

@rearden-grok

@rearden-grok rearden-grok Bot commented Oct 18, 2026 •

Copy link
Copy Markdown
Contributor

Closes #1060. Closes #1062.

Runtime

--rpc-cookie-perms owner|group|all (conf rpc_cookie_perms=) is the widest mode an existing cookie file may have. The default is owner: group or world bits refuse startup. group allows a 0640 file and still refuses world bits. all allows at most 0644 (world read). Group and world write still refuse startup. The node still does not create or chmod the cookie. A loose cookie used to warn once and start.

NixOS services.rbitcoin.rpc.cookiePerms passes that flag only when set. Unset leaves the unit argv on the node default, so a default service command does not grow a flag. It requires cookieFile.

DNS and fixed-seed inject, and peer addr gossip, drop unroutable clearnet addresses (loopback, private, link-local, documentation, and the other std bogons). addpeeraddress and --connect still accept them. Onion, I2P, and CJDNS are not treated as IP bogons.

Remembered fork tips are capped at 64, oldest dropped. Invalidated stays uncapped.

Not a behavior change

P2P admission still has no absurd-fee cap. Mempool fee math still checks u64 overflow, not MAX_MONEY. Peer AEAD stays the chacha20-poly1305 0.1 that bip324 0.11 links. Fuzz stays nightly-only. Duplicate crates that noise_sv2 still requires are skipped in deny.toml; a new duplicate name fails qc. The mutants attribute crate is optional and compiles only with --features mutants. FUZZ_SANITIZER overrides the nightly sanitizer choice. A Sunday (or manual) job runs asmap and block_wire for 120s under UBSan. That job is not a required check.

P2P accept has no absurd-fee cap and mempool fee math checks u64 overflow, not MAX_MONEY. BIP324 stays on upstream chacha20-poly1305 0.1. Fuzz stays nightly-only; silent-payment tweaks are not a missing bin.
Default owner rejects group and world bits at startup. group allows a 0640 file and all allows any mode. The node still does not create or chmod the cookie. NixOS services.rbitcoin.rpc.cookiePerms passes the flag only when set.
cargo deny now fails a new duplicate. The copies that noise_sv2 and the hashes lines still require are skipped with a reason. workspace = true stays allowed: that is the wildcards lint, and the tree has no version star.
The attribute crate is optional. Default node builds do not run the proc-macro. Nightly cargo-mutants passes --features mutants, and every workspace member has that feature so the workspace flag resolves.
DNS and fixed-seed inject, and peer addr messages, skip loopback, private, link-local, documentation, and other non-routable IPs. addpeeraddress and --connect still accept them. Onion, I2P, and CJDNS stay.
The oldest hint drops when a heavier branch replaces the tip. Invalidated stays uncapped: forgetting one of those hashes would accept the block again.
Unset keeps the current sanitizer for each target. A weekly job, not a pull request, runs asmap and block_wire for 120 seconds with undefined. The nightly AddressSanitizer matrix stays the long run.
Windows has no permission bits, so the declared perms are unused there. The argument stays on the signature so every caller still passes it.

@rearden-grok rearden-grok Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Verdict

The cookie flag, NixOS wiring, mutants feature gate, and pinned cargo deny skips match the commits. I would not treat the addr change as finished: the new bogon check does not run on the addrv2 path that stores, dials, persists, and relays for the life of the process.

1. Steady-state addrv2 still stores and relays unroutable addresses

clearnet_unroutable / netaddr_unroutable_gossip run in AddrMan::inject (DNS and fixed seeds) and in IBD inject_learned_addrs (PeerEvent::Addrs).

The session that actually handles peer traffic is peer.rs on_addrv2 → PeerHub::learn_addrv2 → add_learned_addr (peers.rs), with no routability check. That is the book installed by set_addrman, the book tip-follow dials, and the book merge_from writes to peers on shutdown. A peer can still fill all 8192 slots with loopback, RFC1918, link-local, and documentation addresses, including the IPv4-mapped forms.

The same function then relays the original list (for addr in list.iter().take(allow) in on_addrv2). Those addresses are forwarded to neighbors even if a later store filter would have dropped them.

Leaving add_learned open is the right exception for addpeeraddress and --connect. The predicate belongs in learn_addrv2 and on that relay loop, same as inject_learned_addrs. addr_response_net will also keep advertising any bogon already in the book, including rows loaded from an older peers file. Nothing scrubs those on load.

2. The predicate still accepts several non-routable ranges

On the paths that do call it, ipv4_unroutable uses is_unspecified, is_loopback, is_private, is_link_local, is_broadcast, is_multicast, and is_documentation. Rust 1.95 also has stable classifiers this function does not call:

  • is_shared: 100.64.0.0/10 (CGNAT, RFC 6598)
  • is_benchmarking: 198.18.0.0/15 (RFC 2544)
  • is_reserved: 240.0.0.0/4

Core treats those as unroutable. A DNS seed response or an IBD addr batch can still put them in the dial set. IPv6 special-cases 2001:db8::/32 because Ipv6Addr::is_documentation is unstable, and still accepts 2001:10::/28, 2001:20::/28, and 100::/64.

CJDNS fc00::/8 is correctly excluded before the unique-local check, and IPv4-mapped addresses are folded into the v4 check. Onion, I2P, and CJDNS staying out of the bogon set is right.

3. group and all are wider than the 0640 / 0644 story

Group is mode & 0o007 == 0. Modes 0660, 0670, and 0770 all start. A group-writable cookie lets any member of that group replace the RPC secret. The mempool recipe and the new docs describe 0640 (group read). Core's group is owner-read plus group-read, not every group bit.

All is true, so 0666 and 0777 start. That turns the check off. Core's all is world-read (0644).

The tests cover 0600, 0640, and 0644, so 0660 is currently specified as success.

The check also fail-opens. read_to_string runs first, then if let Ok(meta) = fs::metadata(path) stats the path again. A metadata error, or a replace between the read and the stat, skips the refusal and starts with the bytes already read. Stat the file that was read (File::metadata on that handle).

Windows compiles the parameter out (let _ = perms). The operator docs describe the refusal without saying it is Unix mode bits only.

Default owner refusing a 0640 file at startup is what #1060 asked for, and it is applied both in validate and again when RPC binds. An existing 0640 deployment will refuse to start until --rpc-cookie-perms group (or NixOS cookiePerms) is set. The changelog says so. The NixOS module correctly omits the flag when cookiePerms is null and rejects cookiePerms without cookieFile.

Also looked at

  • Fork tips: a FIFO of 64 hints, duplicates not refreshed, Invalidated left uncapped so forgetting a hash cannot make that block acceptable again. Dropping a hint means invalidateblock will not reconsider that archive tip unless it is still held or precious. The new test covers the deque only.
  • mutants is optional and the skip attributes are behind cfg_attr. The empty mutants feature on the other crates is what makes --workspace --features mutants resolve. A default build does not link the proc-macro.
  • Deny skips are exact versions, so a bump fails qc. wildcards stays allow. The comment says workspace = true is that lint. cargo-deny 0.20 flags * / 1.* requirements. If a deny run really flags workspace = true, the comment should quote that diagnostic. Otherwise a new star requirement still passes qc, which is the other half of audit S1.
  • FUZZ_SANITIZER overrides the per-bin choice and is passed into cargo fuzz run. The Sunday UBSan job is not a required check. Every nightly cron still schedules the two ubsan runners; they checkout and exit unless date -u +%u is 7 or the run is workflow_dispatch.
  • The fee-cap, MAX_MONEY, and chacha20-poly1305 0.1 notes are documentation of current behavior. They do not change admission or the AEAD crate.

Static review of bf006613. I did not run the suite.

Comment thread crates/rbitcoin-net/src/netaddr.rs Outdated
}
}

/// Seed inject and peer addr gossip. Onion, I2P, and CJDNS are not IP bogons.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This comment says the predicate covers peer addr gossip. It runs for AddrMan::inject and IBD inject_learned_addrs only.

The process book is updated from PeerHub::learn_addrv2 (peers.rs), which on_addrv2 calls and then relays the unfiltered list to neighbors. That path still stores loopback, RFC1918, link-local, and documentation addresses, persists them through merge_from into peers, and serves them from addr_response_net. add_learned has to stay open for addpeeraddress and --connect; learn_addrv2 and the relay loop need this predicate.

o[0] == 0x20 && o[1] == 0x01 && o[2] == 0x0d && o[3] == 0xb8
}

fn ipv4_unroutable(v: Ipv4Addr) -> bool {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

On 1.95 these stable checks are still accepted as routable: is_shared (100.64.0.0/10), is_benchmarking (198.18.0.0/15), and is_reserved (240.0.0.0/4). Core rejects all three. DNS inject and IBD addr batches can still fill the dial set with them.

IPv6 in clearnet_unroutable has the same shape of hole past 2001:db8::/32: 2001:10::/28, 2001:20::/28, and 100::/64.

Comment thread crates/rbitcoin-rpc/src/auth.rs Outdated
let mode = mode & 0o777;
match self {
Self::Owner => mode & 0o077 == 0,
Self::Group => mode & 0o007 == 0,

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

mode & 0o007 == 0 accepts every group bit. 0660, 0670, and 0770 all start, so a member of the cookie group can replace the RPC secret. The docs and the mempool recipe are 0640 (group read). Core's group is owner-read|group-read.

All below accepts 0666 and 0777, which turns the check off. Core's all is 0644. The tests never pass 0660, so this stays specified as success.

The caller also fail-opens: read_to_string then if let Ok(meta) = fs::metadata(path) stats the path again. Metadata failure, or a replace between the read and the stat, skips the refusal. Use File::metadata on the handle that was read.

reardencode and others added 3 commits October 18, 2026 14:02
Those ranges are not globally routable, and the std helpers that name them are still unstable on 1.95. DNS and IBD inject now skip them the same way as RFC1918.

Co-authored-by: Cursor <cursoragent@cursor.com>
learn_addrv2 and addr relay used the book without the bogon check, so a peer could fill it and forward those addresses. getaddr and peers-file load now drop them too. addpeeraddress and --connect still can.

Co-authored-by: Cursor <cursoragent@cursor.com>
group used to allow a group-writable cookie, and all allowed any mode. The mode is now read from the same file handle that supplies the secret, so a failed stat refuses startup.

Co-authored-by: Cursor <cursoragent@cursor.com>
@rearden-grok

rearden-grok Bot commented Oct 18, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the review. Three commits on this branch address it.

Steady-state gossip was the real hole. learn_addrv2 now drops unroutable clearnet before it stores an address, and the addr relay loop drops those same rows instead of forwarding them. getaddr will not advertise them, and loading the peers file skips them so an old file does not keep dialing them. addpeeraddress and --connect still accept a private address on purpose.

The missing ranges are in that same check: CGNAT (100.64.0.0/10), benchmarking (198.18.0.0/15), class E (240.0.0.0/4), plus IPv6 2001:10::/28, 2001:20::/28, and 100::/64. I tried the std helpers you named. On the 1.95 pin, is_shared, is_benchmarking, and is_reserved are still unstable, so those prefixes are written out by hand, same as 2001:db8::/32. CJDNS fc00::/8 is still kept.

The cookie flag is tighter. group is at most 0640, so a group-writable file (0660) refuses to start. all is at most 0644, so 0666 refuses to start. The mode comes from the same open file as the secret, and a failed stat refuses startup instead of continuing. The docs now say this check is Unix mode bits and that Windows ignores the flag.

I left wildcards = "allow" as it was. I did not re-run a deny experiment that turns that lint on.

@rearden-grok rearden-grok Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Verdict

The three findings from the last review are fixed in 784c53660. I would not re-open them. One gossip hole is still large enough to fill the address book.

Fixed

Steady-state learn_addrv2 and the addrv2 relay loop both call netaddr_unroutable_gossip. addr_response_net does too, so a private address kept for addpeeraddress or --connect is not advertised. AddrMan::load skips the same rows, so a peers file full of them is not dialed again after restart. --connect is applied after load, so a configured LAN peer still comes back.

The new prefixes match the ranges named last time. 100.64.0.0/10 is (second & 0xc0) == 64. 198.18.0.0/15 is 198 and (second & 0xfe) == 18. 240.0.0.0/4 is the high nibble. 2001:10::/28 and 2001:20::/28 are the low nibble of the second hextet. 100::/64 is the first eight bytes. Hand-rolling these is required: on rustc 1.95, is_shared, is_benchmarking, and is_reserved are still unstable. CJDNS fc00::/8 is still excluded before unique-local.

Cookie group is mode & 0o037 == 0 (at most 0640). all is mode & 0o033 == 0 (at most 0644). 0660 and 0666 fail. The mode is File::metadata on the handle just opened, and a stat error refuses startup. Docs, the changelog fragment, and the NixOS option text match that ceiling, including the Unix-only note.

Fork tips are still a FIFO of 64 hints, duplicates stay put, and Invalidated is still uncapped. That matches docs/ibd-memory.md. The deny skips are still exact versions. wildcards is still allow. There is no version = "*" in the tree. I did not re-run cargo-deny with that lint set to deny, so I am not treating the workspace = true comment as disproved.

Still open

ipv4_unroutable treats only 0.0.0.0 as unspecified. 0.0.0.0/8 otherwise passes, and so does a 6to4 address whose embedded IPv4 is a bogon (2002:0a00:0001:: is 10.0.0.1). Both are stored by learn_addrv2, relayed, and dialed. 8192 of the 0/8 addresses fill the book the same way RFC1918 used to. 64:ff9b::/96 (NAT64) has the same embedded-IPv4 shape and is not inspected.

Port 0 clearnet never becomes a NetAddr (from_addrv2 returns None), so the new relay guard does not see it. Those rows are still forwarded. They are not stored.

POSIX ACLs are invisible to the mode mask. A 0600 cookie can still be group-readable by ACL. Windows still ignores the flag, which the docs now say.

Static read of 784c53660. I did not re-run the suite.

}

fn ipv4_unroutable(v: Ipv4Addr) -> bool {
v.is_unspecified()

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

is_unspecified is only 0.0.0.0. The rest of 0.0.0.0/8 (0.1.2.3 and so on) is not private, link-local, shared, benchmarking, documentation, or reserved, so it is stored, relayed, and dialed. That prefix alone can fill all 8192 addrman slots.

The IPv6 arm has the same class of bypass for an embedded bogon: 2002:0a00:0001:: is 6to4 of 10.0.0.1, and 64:ff9b::a00:1 is NAT64 of 10.0.0.1. Neither is mapped (::ffff:0:0/96), unique-local, or one of the special prefixes, so both pass clearnet_unroutable.

Comment thread crates/rbitcoin-net/src/peer.rs Outdated
let mut batches: Vec<Vec<bitcoin::p2p::address::AddrV2Message>> =
vec![Vec::new(); neighbors.len()];
for addr in list.iter().take(allow) {
if crate::NetAddr::from_addrv2(addr)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This guard runs only when from_addrv2 returns Some. Port 0 clearnet returns None (I2P is the exception), so a port-0 row is still copied into the neighbor batch. It is not stored, because learn_addrv2 also requires Some.

reardencode and others added 2 commits October 18, 2026 15:56
0.0.0.0/8 was only caught at 0.0.0.0, and a private IPv4 wrapped in 2002::/16 or 64:ff9b::/96 still entered the dial set. A public embedded address stays.

Co-authored-by: Cursor <cursoragent@cursor.com>
from_addrv2 returns None for port 0, so the unroutable gossip guard never saw the row and it was forwarded anyway. I2P stays, because Core advertises it with port 0.

Co-authored-by: Cursor <cursoragent@cursor.com>
@rearden-grok

rearden-grok Bot commented Oct 18, 2026

Copy link
Copy Markdown
Contributor Author

The three leftovers from the last pass are in now.

0.0.0.0/8 is dropped as a whole. The old check only caught the single address 0.0.0.0, so 0.1.2.3 could still fill the address book, get stored, and be dialed.

6to4 (2002::/16) and well-known NAT64 (64:ff9b::/96) are dropped only when the IPv4 inside them is unroutable. 2002:0a00:0001:: is 10.0.0.1 and 64:ff9b::a00:1 is the same private address, so both are ignored. A public address wrapped the same way, such as 6to4 of 1.2.3.4, is still kept and can be relayed.

Port 0 on clearnet (and on Tor or CJDNS) is no longer relayed. Those rows were already not stored, because turning them into an address returns nothing, and that meant the gossip filter never looked at them. I2P with port 0 is still relayed. Core advertises I2P that way, and it is a real destination.

The other notes from that review are unchanged: POSIX ACLs that the mode bits cannot see, Windows ignoring the cookie permission flag, the deny.toml wildcard allow, and the fork-tip list of 64.

@rearden-grok rearden-grok Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Verdict

The three leftovers from the last review are fixed in 2940ca743. I would not re-open them. The new embedding check still lets one public IPv4 occupy every addrman slot, and a documentation prefix Core rejects as invalid still enters the book.

Fixed

0.0.0.0/8 is dropped, not only 0.0.0.0. Canonical 6to4 (2002:0a00:0001::, the all-zero suffix) and well-known NAT64 (64:ff9b::a00:1) are dropped when the embedded IPv4 is unroutable. A public embedding with an all-zero suffix is kept, which is what the new test pins.

Port 0 is no longer relayed, except I2P. Those clearnet rows were already absent from the book, because from_addrv2 returns None, and that None is why the unroutable guard never saw them. I2P port 0 still relays. Core advertises I2P that way.

Cookie group is still at most 0640 (mode & 0o037 == 0) and all at most 0644 (mode & 0o033 == 0). The mode comes from File::metadata on the handle just opened, and a stat error still refuses startup. Fork tips are still a FIFO of 64, and Invalidated is still uncapped. The deny skips are still exact versions.

Still open

ipv6_embedded_bogon only inspects 6to4 bytes 2–5 and the 32-bit payload of 64:ff9b::/96. A 6to4 address whose embedded IPv4 is public returns false for every value of the other 80 bits. 2002:0102:0304::1 and 2002:0102:0304::2 are different NetAddr::Ip keys, both dialable, both relayed. The addrman cap is a flat 8192 with no per-group limit, so those suffixes fill the book the same way 0/8 did. A peer with the addr permission skips the token bucket (take_addr_tokens returns want).

The same function misses other wrappers of a private IPv4:

  • IPv4-compatible ::a00:1 is 10.0.0.1. to_ipv4_mapped only unwraps ::ffff:0:0/96, so this is not the mapped form the v4 check already sees.
  • SIIT ::ffff:0:a00:1 (::ffff:0:0:0/96, RFC 6145) is 10.0.0.1 in the last four bytes. That prefix is not ::ffff:0:0/96.
  • Local NAT64 64:ff9b:1::/48 fails the well-known check because bytes 4–11 are not all zero, so 64:ff9b:1::a00:1 is stored, and the 48 bits inside the prefix are free.
  • Teredo 2001::/32 stores the client IPv4 bit-flipped in the last four bytes. A flipped 10.0.0.1 never reaches ipv4_unroutable. Server, flags, and port are free. Core's IsRoutable is true for Teredo; the flat map is what makes the free bits a full-book fill here.

ipv6_special_unroutable still has no 3fff::/20. That is the RFC 9637 documentation range. Core's IsRFC9637 makes IsValid false, so Core will not store or relay it. 3fff::1 passes every check in clearnet_unroutable and is dialable.

Port 0 was the case where from_addrv2 returns None and the relay loop forwarded the row anyway. Tor v2 and a CJDNS address outside fc00::/8 are the same shape when the port is non-zero: not stored, still forwarded. Unknown BIP155 network ids are in that bucket too; relaying those is what the spec asks for. Tor v2 is not.

POSIX ACLs, the Windows cookie flag, deny.toml wildcards = "allow", and the fork-tip cap of 64 are unchanged from the last review.

Static read of 2940ca743. I did not re-run the suite.

Comment thread crates/rbitcoin-net/src/netaddr.rs Outdated
fn ipv6_embedded_bogon(v: Ipv6Addr) -> bool {
let o = v.octets();
if o[0] == 0x20 && o[1] == 0x02 {
return ipv4_unroutable(Ipv4Addr::new(o[2], o[3], o[4], o[5]));

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This returns false for every suffix once bytes 2–5 are a public IPv4. 2002:0102:0304:: and 2002:0102:0304::1 are different map keys, and NetAddr::Ip is dialable, so the 80 free bits fill all 8192 slots. The inject test only keeps the all-zero suffix.

The same helper never looks at IPv4-compatible ::/96 (::a00:1), SIIT ::ffff:0:0:0/96 (::ffff:0:a00:1), local NAT64 64:ff9b:1::/48, or Teredo 2001::/32 (client IPv4 is bit-flipped). to_ipv4_mapped only unwraps ::ffff:0:0/96, so those private embeddings are stored and relayed. Local NAT64 and Teredo also have free bits outside the embedded IPv4.

/// ORCHIDv2, and `100::/64` discard. `is_documentation` is still unstable.
fn ipv6_special_unroutable(v: Ipv6Addr) -> bool {
let o = v.octets();
if o[0] == 0x20 && o[1] == 0x01 && o[2] == 0x0d && o[3] == 0xb8 {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

2001:db8::/32 is here, and 3fff::/20 is not. RFC 9637 is the expanded documentation range. Core treats it as invalid (IsRFC9637 in IsValid), so it is not stored or relayed there. 3fff::1 fails none of the checks below and is a dialable NetAddr::Ip, which is enough to fill the book.

Comment thread crates/rbitcoin-net/src/peer.rs Outdated
// None for it (so it is not stored), except I2P, which Core
// advertises with port 0. That None skips the unroutable
// guard, so drop the row here before it is forwarded.
if addr.port == 0 && !matches!(addr.addr, bitcoin::p2p::address::AddrV2::I2p(_)) {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Port 0 is covered. from_addrv2 still returns None for Tor v2 and for CJDNS outside fc00::/8 when the port is non-zero, and is_some_and then leaves those rows in the forwarded batch. They are not stored. Unknown BIP155 network ids take this path too, and relaying those is intentional. Tor v2 is a dead encoding and should not be forwarded.

reardencode and others added 2 commits October 18, 2026 16:29
A public 6to4 or local NAT64 suffix was a distinct dialable key, so one IPv4 could fill the address book. Teredo's server field is the same kind of free bits. 3fff::/20 is documentation.

Co-authored-by: Cursor <cursoragent@cursor.com>
Those rows never become a NetAddr, so the unroutable check did not see them and the relay loop forwarded them. Unknown BIP155 network ids are still forwarded.

Co-authored-by: Cursor <cursoragent@cursor.com>
@rearden-grok

rearden-grok Bot commented Oct 18, 2026

Copy link
Copy Markdown
Contributor Author

The leftovers from the last review are in now.

A public IPv4 wrapped in 6to4 or local NAT64 (64:ff9b:1::/48) is kept only when the extra bits are zero. 2002:0102:0304:: still stays. 2002:0102:0304::1 does not, so one IPv4 can no longer fill the address book by varying the suffix. The same rule applies to well-known NAT64, old IPv4-compatible addresses (::a00:1 is 10.0.0.1), and SIIT (::ffff:0:a00:1). A public address in those forms still stays. IPv4-mapped addresses were already unwrapped.

Teredo (2001::/32) is dropped outright. The server address alone is 32 free bits, so keeping a "canonical" Teredo address would still fill the book. 3fff::/20, the newer documentation range, is dropped too.

Tor v2 is no longer relayed, and neither is a CJDNS address that is not in fc00::/8. Those never became a stored address, which is why the filter missed them. I2P with port 0 is still relayed. An unknown BIP155 network id is still relayed, which is how a new network is supposed to spread.

The cookie ceilings, Windows behavior, POSIX ACLs, the deny-toml wildcard allow, and the 64 fork-tip cap are unchanged.

@rearden-grok rearden-grok Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Verdict

The findings from the last review are fixed in dda62123. I would not re-open them. One prefix can still fill the flat 8192 book. The rest of this pull request is in the shape the earlier rounds asked for.

Disposition

Fixed

0.0.0.0/8 is the whole prefix. Canonical 6to4 keeps a public embedded IPv4 only when bytes 6–15 are zero, so 2002:0102:0304:: stays and 2002:0102:0304::1 does not. Well-known NAT64 64:ff9b::/96, local NAT64 64:ff9b:1::/48 (padding bytes 6–11 must be zero), IPv4-compatible ::/96, and SIIT ::ffff:0:0:0/96 drop a private embedded IPv4 and keep a public one. IPv4-mapped ::ffff:0:0/96 is still unwrapped before that. Teredo 2001::/32 is dropped entirely. 3fff::/20 is dropped. Those checks run on inject, IBD learn, steady-state learn_addrv2, getaddr, and peers-file load.

Port 0 is not relayed, except I2P. Tor v2 is not relayed. CJDNS outside fc00::/8 is not relayed. Unknown BIP155 network ids are still relayed. addrv2_skip_relay is what the relay loop calls, so a None from from_addrv2 no longer means "forward it."

Cookie group is still at most 0640 and all at most 0644, from File::metadata on the opened handle. A stat error refuses startup. Fork tips are still a FIFO of 64. Invalidated is still uncapped. Deny skips are still exact versions.

Still open

2001:2::/48 (RFC 5180, the IPv6 benchmarking range) is not classified. The IPv4 twin 198.18.0.0/15 is. 2001:2::1 fails every check in clearnet_unroutable: it is not documentation, not ORCHID, not Teredo (2001:0000::/32), and not a tunnel wrapper. The last 80 bits are free, NetAddr::Ip is dialable, and both learn_addrv2 and addrv2_skip_relay keep the row. A peer with the addr permission skips the token bucket, so one session can replace the book with these addresses. Core's IsRoutable does not exclude this prefix either; the reason to drop it here is the same flat map that made Teredo a full-book fill, and it is the range that matches the benchmarking check this pull request already has.

Not blocking

Several canonical forms of one public IPv4 can all stay (1.2.3.4, ::1.2.3.4, ::ffff:1.2.3.4, ::ffff:0:1.2.3.4, 64:ff9b::1.2.3.4, 64:ff9b:1::1.2.3.4, 2002:0102:0304::). That is a handful of slots per address, not a suffix that fills 8192. The 6to4 form we keep is the all-zero interface id.

fec0::/10 and 3ffe::/16 are historical and can also fill a flat map. Core's IsRoutable still accepts them. I am not treating "every prefix that is not globally routed" as in scope.

POSIX ACLs are still invisible to the mode mask. Windows still ignores --rpc-cookie-perms. deny.toml wildcards is still allow, and there is still no version = "*" in the tree. The fork-tip cap of 64 is unchanged. The RPC token reader still does not check mode; this pull request is the cookie file. The pull request text still says all allows any mode; the code does not.

Steady-state v1 addr is ignored (not stored and not relayed). That cannot fill the book.

Static read of dda62123. I did not re-run the suite.

if o[0] == 0x20 && o[1] == 0x01 && o[2] == 0x00 && (o[3] & 0xf0) == 0x20 {
return true;
}
o[0] == 0x01 && o[1] == 0x00 && o[2..8].iter().all(|&b| b == 0)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

100::/64 is here, and 2001:2::/48 is not. That is RFC 5180, the IPv6 benchmarking range, the pair of 198.18.0.0/15 which ipv4_benchmarking already drops. First six bytes are 20 01 00 02 00 00; the other ten bytes are free. 2001:2::1 is not documentation, not ORCHID, not Teredo, and not one of the tunnel wrappers below, so learn_addrv2 stores it and addrv2_skip_relay forwards it.

198.18.0.0/15 was already ignored. The IPv6 range was not, and its last 80 bits are free, so one peer could fill the address book with addresses that are not globally routed.

Co-authored-by: Cursor <cursoragent@cursor.com>
@rearden-grok

rearden-grok Bot commented Oct 18, 2026

Copy link
Copy Markdown
Contributor Author

The open item from the last review is fixed.

2001:2::/48 is now dropped in full, same as the IPv4 benchmarking range 198.18.0.0/15. That includes 2001:2::1 and the far end of the prefix (2001:2:0:ffff:ffff:ffff:ffff:ffff). An address just outside it, 2001:2:1::1, is still kept. Those rows are not stored and not relayed.

The items that review left as not blocking are unchanged: several canonical forms of one public IPv4, fec0::/10 and 3ffe::/16, POSIX ACLs, the Windows cookie flag, the deny-toml wildcard allow, the 64 fork-tip cap, and the RPC token file. I did correct the pull request text, which still said all allows any mode. The code caps that at 0644.

@rearden-grok rearden-grok Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Verdict

Nothing from the earlier rounds is still open. 75daca8ca drops 2001:2::/48, and I do not see another way for one unroutable clearnet prefix to fill the 8192-slot book.

Disposition

Fixed

2001:2::/48 is the six bytes 20 01 00 02 00 00. 2001:2::1 and 2001:2:0:ffff:ffff:ffff:ffff:ffff both match. 2001:2:1::1 is 2001:0002:0001::1, so byte 5 is 0x01 and it stays. That prefix does not collide with Teredo (2001:0000::/32), ORCHID (2001:10::/28), or ORCHIDv2 (2001:20::/28).

The same clearnet_unroutable predicate is what seed inject, IBD inject_learned_addrs, learn_addrv2, getaddr, and peers-file load call. addrv2_skip_relay calls it for every address that becomes a NetAddr, and it drops port 0 (except I2P), Tor v2, and CJDNS outside fc00::/8 before that. A None from from_addrv2 is no longer forwarded.

Checked against the prefixes this series has been holding:

  • 0.0.0.0/8, loopback, RFC1918, link-local, multicast, broadcast, documentation, CGNAT 100.64.0.0/10 (second & 0xc0 == 64), benchmarking 198.18.0.0/15 (198 and second & 0xfe == 18), class E (first & 0xf0 == 240).
  • IPv6 documentation 2001:db8::/32 and 3fff::/20 (high nibble of byte 2 is 0, which is /20), ORCHID, ORCHIDv2, discard 100::/64, unique-local fd00::/8. fc00::/8 is still CJDNS and is excluded before unique-local.
  • IPv4-mapped ::ffff:0:0/96 is unwrapped with to_ipv4_mapped before the tunnel checks. 6to4 and local NAT64 keep a public embedded IPv4 only when the non-IPv4 bits are zero. Well-known NAT64, IPv4-compatible ::/96, and SIIT ::ffff:0:0:0/96 keep a public embedded IPv4 and drop a private one. Teredo is the whole 2001::/32.

Cookie group is mode & 0o037 == 0 (at most 0640). all is mode & 0o033 == 0 (at most 0644: group write is 0o020, which is inside 0o030). The mode is File::metadata on the handle just opened. Startup (config.rs) and the RPC bind (server.rs) both go through read_cookie_file. NixOS cookiePerms is an enum of owner, group, and all, and the unit omits the flag when it is null. The pull request text now says all is at most 0644.

Fork tips are still a FIFO of 64, written only when this node reorgs, and Invalidated is still uncapped. Deny skips are still exact versions.

Still open

None.

Not blocking

One public IPv4 can still occupy a handful of slots: the IPv4 itself, IPv4-mapped, IPv4-compatible, SIIT, well-known NAT64, canonical local NAT64, and canonical 6to4. That is a small constant, not a free suffix. The 6to4 form we keep is the all-zero interface id.

fec0::/10 and 3ffe::/16 can still fill a flat map. Core's IsRoutable accepts them too. I am not treating every historical range as part of this change.

POSIX ACLs are invisible to the mode mask. Windows ignores the flag. A cookie directory an attacker can rename into is outside the file-mode check. The RPC token reader still does not check mode. deny.toml wildcards is still allow. The fork-tip cap of 64 is unchanged. Steady-state v1 addr is ignored, so it cannot fill the book.

Static read of 75daca8ca. I did not re-run the suite.

Load drops 127.0.0.1, so the refused-connect test no longer sees the fail bit after AddrMan::load. The flush is still in the file.

Co-authored-by: Cursor <cursoragent@cursor.com>

@rearden-grok rearden-grok Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Verdict

Nothing is still open. a2a3dd202 only changes the test that CI failed. The filter and the cookie check are the same as 75daca8ca.

Disposition

Fixed

run_p2p_refused_connect_flushes_book_and_dials_proxy dials 127.0.0.1. That address is written to the peers file with the fail bit, and AddrMan::load now drops it, which is why the old assertion panicked. The test reads the file row ({addr} 0x..) and checks bit 4 (FAILED_LAST_CONNECT). It also checks that load does not bring the loopback address back. --connect is applied again from config after load, so the next start still dials that peer. The fail bit on a loopback row does not survive load, so it does not throttle that next dial. That matches the rule that a peers file full of loopback is not a dial set.

The row match is {connect} with a trailing space, so 127.0.0.1:9 does not match 127.0.0.1:90. The flag parse keeps any extra bits and still requires the fail bit.

The address filter is unchanged from the last review: 2001:2::/48 is in, tunnel suffixes are canonical, Teredo and 3fff::/20 are out, port 0 is not relayed except I2P, Tor v2 and non-fc00::/8 CJDNS are not relayed. Cookie group is at most 0640 and all at most 0644, from the opened handle.

Still open

None.

Not blocking

One public IPv4 can still take a handful of canonical slots (mapped, compatible, SIIT, NAT64, zero-suffix 6to4). fec0::/10 and 3ffe::/16 can still fill a flat map; Core's IsRoutable accepts them too. POSIX ACLs, Windows mode bits, a writable cookie directory, and the RPC token file are outside this check. deny.toml wildcards stay allowed. Fork tips stay a FIFO of 64. Steady-state v1 addr is ignored.

Static read of a2a3dd202. CI on this commit was still running.

node_listen_and_exit loaded 127.0.0.1:1 and expected the fail bit. Load drops loopback, so the flush is checked on the file row instead.

Co-authored-by: Cursor <cursoragent@cursor.com>
@reardencode
reardencode merged commit 4c6bc2e into master Oct 19, 2026
18 checks passed
@rearden-grok rearden-grok Bot mentioned this pull request Oct 19, 2026
2 of 3 tasks
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.

Muse audit public findings 10/17/2026 RPC cookie: a deliberately group-readable cookie warns on every start; flag like Core's -rpccookieperms?

1 participant