mirror of
https://github.com/jmcorgan/fips.git
synced 2026-07-30 19:46:15 +00:00
Correct what the address-to-link map claims to be for
The doc said the map "enables dispatching incoming packets to the right connection before authentication completes." It does not: find_link_by_addr has no callers outside its own tests on any branch, and encrypted frames dispatch by session index. Its live readers are the msg1 admission fast path and the duplicate-inbound-handshake check. The comment mattered because the map has a trap that invites exactly the misuse it was advertising. An outbound dial registers the literal configured address string, which may be a hostname, while an inbound packet carries the resolved form; TransportAddr compares byte-wise, so the two never match and a lookup keyed on an inbound address returns "no such link" for every hostname-configured peer rather than failing. The entry is also single-valued per key, so an inbound handshake overwrites an outbound dial's entry for the same address. Both current readers survive this because each compares a key written in the same form it reads. That is a property of those two call sites and not of the map, so the comment now says so, and says not to key a peer-identity question on it. Documentation only; no behaviour changes.
This commit is contained in:
+17
-2
@@ -318,8 +318,23 @@ struct PendingConnect {
|
||||
/// 1. **Connection phase** (`connections`): Handshake in progress, indexed by LinkId
|
||||
/// 2. **Active phase** (`peers`): Authenticated, indexed by NodeAddr
|
||||
///
|
||||
/// The `addr_to_link` map enables dispatching incoming packets to the right
|
||||
/// connection before authentication completes.
|
||||
/// The `addr_to_link` map is a reverse lookup from `(transport, address)` to
|
||||
/// link. It is **not** a packet-dispatch path, despite what this comment used
|
||||
/// to say: `find_link_by_addr` has no callers outside its own tests, and
|
||||
/// encrypted frames are dispatched by session index. Its live readers are the
|
||||
/// `should_admit_msg1` fast path and the duplicate-inbound-handshake check in
|
||||
/// `handle_msg1`.
|
||||
///
|
||||
/// **Do not key a peer-identity question on it.** The address form is not
|
||||
/// canonical: an outbound dial registers the literal configured string, which
|
||||
/// may be a hostname (`"node-b:2121"`), while an inbound packet carries the
|
||||
/// resolved form (`"10.128.2.4:2121"`). `TransportAddr` compares byte-wise, so
|
||||
/// those never match, and a lookup keyed on an inbound address silently returns
|
||||
/// "no such link" for every hostname-configured peer rather than failing. The
|
||||
/// entry is also single-valued per key, so an inbound handshake overwrites an
|
||||
/// outbound dial's entry for the same address. The readers above tolerate this
|
||||
/// because each compares a key written in the same form it reads; a new reader
|
||||
/// that does not will be quietly wrong.
|
||||
// Discovery lookup constants moved to config: node.discovery.attempt_timeouts_secs, node.discovery.ttl
|
||||
pub struct Node {
|
||||
// === Immutable Context ===
|
||||
|
||||
Reference in New Issue
Block a user