mirror of
https://github.com/jmcorgan/fips.git
synced 2026-10-05 19:18:25 +00:00
Confirm the peer identity after the handshake, not just the address
An inbound setup message from the address of an established peer is admitted even when the node is configured to refuse inbound connections. That carve-out exists so a peer that re-handshakes is not locked out, but it decided purely on the address, so an off-path party sourcing from that address was admitted too. The waiver now classifies into three outcomes rather than two: no waiver was needed, a waiver was used and an owning identity is known, or a waiver was used and no identity owns the link. The third rejects after the key exchange. An earlier shape returned nothing for that case, which silently skipped the check for an ordinary population, which is the defect this change exists to close. The address lookup deliberately falls through rather than returning when it finds no owning identity. The reverse lookup can name a link that no longer exists, because removal clears it only under the key rebuilt from the link's own address, while the cross-connection path inserts a second key from the observed source address. Returning there would refuse a peer the address scan can still attribute, and refuse it permanently: the confirmation returns above the insert that repairs the map, so nothing downstream would ever fix it. Six tests, each broken to prove it can fail. The refusing transport is a real bound socket rather than the loopback handle, which has no refuse override, so "no response was sent" is an observation on a wire rather than a property of a transport that was never started.
This commit is contained in:
@@ -1,5 +1,6 @@
|
|||||||
//! Handshake handlers and connection promotion.
|
//! Handshake handlers and connection promotion.
|
||||||
|
|
||||||
|
use crate::NodeAddr;
|
||||||
use crate::PeerIdentity;
|
use crate::PeerIdentity;
|
||||||
use crate::node::acl::PeerAclContext;
|
use crate::node::acl::PeerAclContext;
|
||||||
use crate::node::rate_limit::Msg1Class;
|
use crate::node::rate_limit::Msg1Class;
|
||||||
@@ -11,6 +12,28 @@ use crate::transport::{Link, LinkDirection, LinkId, ReceivedPacket};
|
|||||||
use std::time::Duration;
|
use std::time::Duration;
|
||||||
use tracing::{debug, info, warn};
|
use tracing::{debug, info, warn};
|
||||||
|
|
||||||
|
/// Why an inbound msg1 got past the `accept_connections` gate, and against
|
||||||
|
/// what identity the post-DH confirmation must check it.
|
||||||
|
///
|
||||||
|
/// Three outcomes, not two: an `Option` would conflate "no waiver was needed"
|
||||||
|
/// with "the waiver was used and nobody owns the matched address", and the
|
||||||
|
/// second of those is the case that must reject.
|
||||||
|
#[derive(Debug, PartialEq, Eq)]
|
||||||
|
pub(in crate::node) enum Msg1Waiver {
|
||||||
|
/// The transport accepts fresh inbound handshakes (or no transport is
|
||||||
|
/// registered), so the address carve-out did not admit this msg1 and
|
||||||
|
/// there is nothing to confirm.
|
||||||
|
NotNeeded,
|
||||||
|
/// The carve-out is what admitted this msg1, and the matched address
|
||||||
|
/// belongs to this identity: either a promoted peer, or a handshake
|
||||||
|
/// already in flight on the matched link whose identity is expected
|
||||||
|
/// (outbound dial) or already learned (inbound msg1).
|
||||||
|
Expect(NodeAddr),
|
||||||
|
/// The carve-out is what admitted this msg1, and no identity can be
|
||||||
|
/// attributed to the matched address. Fail closed: reject after the DH.
|
||||||
|
Unattributed,
|
||||||
|
}
|
||||||
|
|
||||||
impl Node {
|
impl Node {
|
||||||
/// Returns true if an inbound msg1's source matches an established
|
/// Returns true if an inbound msg1's source matches an established
|
||||||
/// link, i.e. it is rekey/restart maintenance traffic rather than a
|
/// link, i.e. it is rekey/restart maintenance traffic rather than a
|
||||||
@@ -63,6 +86,80 @@ impl Node {
|
|||||||
false
|
false
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Classify the msg1 waiver for a source that `should_admit_msg1`
|
||||||
|
/// admitted, so the post-DH confirmation knows whether it has an
|
||||||
|
/// identity to check against and what to do when it has none.
|
||||||
|
///
|
||||||
|
/// `established` is the caller's already-computed
|
||||||
|
/// `is_established_link_msg1(...)`, so the O(peers) scan is not repeated
|
||||||
|
/// on the refusal path.
|
||||||
|
///
|
||||||
|
/// The two attribution limbs are composed the same way
|
||||||
|
/// `is_established_link_msg1` composes its own: as an OR, not as an
|
||||||
|
/// if/else. An `addr_to_link` entry that yields no identity must not
|
||||||
|
/// short-circuit the address scan, because the two keys can be different
|
||||||
|
/// forms of the same peer's address (the hostname-vs-numeric case that
|
||||||
|
/// predicate 2 exists for) and the entry can outlive the link it named.
|
||||||
|
pub(in crate::node) fn msg1_waiver(
|
||||||
|
&self,
|
||||||
|
established: bool,
|
||||||
|
transport_id: crate::transport::TransportId,
|
||||||
|
remote_addr: &crate::transport::TransportAddr,
|
||||||
|
) -> Msg1Waiver {
|
||||||
|
// The carve-out only admits anything when the gate would otherwise
|
||||||
|
// refuse, so an accepting transport has nothing to confirm.
|
||||||
|
if self
|
||||||
|
.transports
|
||||||
|
.get(&transport_id)
|
||||||
|
.is_none_or(|t| t.accept_connections())
|
||||||
|
{
|
||||||
|
return Msg1Waiver::NotNeeded;
|
||||||
|
}
|
||||||
|
if !established {
|
||||||
|
// `should_admit_msg1` refused this msg1 and the caller returned,
|
||||||
|
// so this arm is unreachable from the one call site. Fail closed
|
||||||
|
// rather than skipping the check, so a second caller cannot
|
||||||
|
// reintroduce the hole this classifier exists to close.
|
||||||
|
return Msg1Waiver::Unattributed;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Predicate 1: the reverse-address lookup.
|
||||||
|
if let Some(&link_id) = self.addr_to_link.get(&(transport_id, remote_addr.clone())) {
|
||||||
|
if let Some(peer) = self.peers.values().find(|p| p.link_id() == link_id) {
|
||||||
|
return Msg1Waiver::Expect(*peer.node_addr());
|
||||||
|
}
|
||||||
|
// A link with no promoted peer: a dial in progress or an inbound
|
||||||
|
// handshake in flight. Both register a connection carrying the
|
||||||
|
// expected (outbound) or learned (inbound) identity.
|
||||||
|
if let Some(id) = self
|
||||||
|
.connections
|
||||||
|
.get(&link_id)
|
||||||
|
.and_then(|c| c.expected_identity())
|
||||||
|
{
|
||||||
|
return Msg1Waiver::Expect(*id.node_addr());
|
||||||
|
}
|
||||||
|
// Deliberately fall through instead of returning. The entry can
|
||||||
|
// name a link that no longer exists — `remove_link` clears the
|
||||||
|
// reverse lookup only under the key it rebuilds from the link's
|
||||||
|
// own remote address, so an entry inserted under a second
|
||||||
|
// address form for that link survives its removal. Rejecting
|
||||||
|
// here would refuse a peer predicate 2 can still attribute, and
|
||||||
|
// would refuse it permanently: this classifier's caller returns
|
||||||
|
// above the insert that overwrites the stale entry, so nothing
|
||||||
|
// downstream would ever repair the map.
|
||||||
|
}
|
||||||
|
|
||||||
|
// Predicate 2: the address scan over promoted peers, which always
|
||||||
|
// yields an identity when it matches.
|
||||||
|
self.peers
|
||||||
|
.values()
|
||||||
|
.find(|p| {
|
||||||
|
p.transport_id() == Some(transport_id) && p.current_addr() == Some(remote_addr)
|
||||||
|
})
|
||||||
|
.map(|p| Msg1Waiver::Expect(*p.node_addr()))
|
||||||
|
.unwrap_or(Msg1Waiver::Unattributed)
|
||||||
|
}
|
||||||
|
|
||||||
/// Returns true if an inbound msg1 should be admitted past the
|
/// Returns true if an inbound msg1 should be admitted past the
|
||||||
/// `accept_connections` gate.
|
/// `accept_connections` gate.
|
||||||
///
|
///
|
||||||
@@ -135,6 +232,12 @@ impl Node {
|
|||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Snapshot which identity, if any, the address carve-out attributed
|
||||||
|
// this source to. Taken here rather than after the DH so the answer
|
||||||
|
// is the one the gate acted on. On an accepting transport this is one
|
||||||
|
// map lookup and a return.
|
||||||
|
let waiver = self.msg1_waiver(established, packet.transport_id, &packet.remote_addr);
|
||||||
|
|
||||||
// Parse header
|
// Parse header
|
||||||
let header = match Msg1Header::parse(&packet.data) {
|
let header = match Msg1Header::parse(&packet.data) {
|
||||||
Some(h) => h,
|
Some(h) => h,
|
||||||
@@ -263,6 +366,39 @@ impl Node {
|
|||||||
|
|
||||||
let peer_node_addr = *peer_identity.node_addr();
|
let peer_node_addr = *peer_identity.node_addr();
|
||||||
|
|
||||||
|
// The address carve-out admitted this msg1 past a refusing gate on
|
||||||
|
// the strength of the source address alone. Now that the DH has
|
||||||
|
// revealed the initiator's static, confirm it belongs to the party
|
||||||
|
// that address is attributed to; an off-path party sourcing from an
|
||||||
|
// established peer's address gets no further than here. Cheap
|
||||||
|
// rejection is unchanged: a stranger under accept_connections=false
|
||||||
|
// is still refused above, having paid nothing.
|
||||||
|
match waiver {
|
||||||
|
Msg1Waiver::NotNeeded => {}
|
||||||
|
Msg1Waiver::Expect(expected) if expected == peer_node_addr => {}
|
||||||
|
Msg1Waiver::Expect(expected) => {
|
||||||
|
warn!(
|
||||||
|
expected = %self.peer_display_name(&expected),
|
||||||
|
actual = %self.peer_display_name(&peer_node_addr),
|
||||||
|
transport_id = %packet.transport_id,
|
||||||
|
"Msg1 admitted by the established-address waiver carries a different identity, dropping"
|
||||||
|
);
|
||||||
|
self.stats_mut()
|
||||||
|
.record_reject(RejectReason::Handshake(HandshakeReject::BadState));
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
Msg1Waiver::Unattributed => {
|
||||||
|
warn!(
|
||||||
|
actual = %self.peer_display_name(&peer_node_addr),
|
||||||
|
transport_id = %packet.transport_id,
|
||||||
|
"Msg1 admitted by the established-address waiver, but no identity owns that address, dropping"
|
||||||
|
);
|
||||||
|
self.stats_mut()
|
||||||
|
.record_reject(RejectReason::Handshake(HandshakeReject::BadState));
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// Identity-based restart/rekey detection: if the peer is already
|
// Identity-based restart/rekey detection: if the peer is already
|
||||||
// active but addr_to_link didn't match (different source address, e.g.,
|
// active but addr_to_link didn't match (different source address, e.g.,
|
||||||
// TCP from a different port), we still need to check for restart/rekey.
|
// TCP from a different port), we still need to check for restart/rekey.
|
||||||
|
|||||||
@@ -6,7 +6,7 @@ pub(crate) mod discovery;
|
|||||||
mod dispatch;
|
mod dispatch;
|
||||||
mod encrypted;
|
mod encrypted;
|
||||||
mod forwarding;
|
mod forwarding;
|
||||||
mod handshake;
|
pub(in crate::node) mod handshake;
|
||||||
mod mmp;
|
mod mmp;
|
||||||
mod rekey;
|
mod rekey;
|
||||||
mod rx_loop;
|
mod rx_loop;
|
||||||
|
|||||||
@@ -1285,3 +1285,529 @@ async fn a_msg1_with_a_correct_payload_len_is_not_dropped() {
|
|||||||
payload at the DH"
|
payload at the DH"
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// ============================================================================
|
||||||
|
// Identity confirmation after the DH (the address-keyed msg1 carve-out)
|
||||||
|
// ============================================================================
|
||||||
|
|
||||||
|
/// A node whose only transport refuses fresh inbound handshakes, paired with
|
||||||
|
/// a second started UDP socket standing in for the far end.
|
||||||
|
///
|
||||||
|
/// Returns the node, the far end's address, the far end's receive channel,
|
||||||
|
/// and the far end's transport, which the caller must keep alive for its
|
||||||
|
/// receive loop to go on running.
|
||||||
|
///
|
||||||
|
/// Both transports are started on purpose. A msg2 the node decides to send
|
||||||
|
/// then really leaves it and really arrives on the returned channel, which is
|
||||||
|
/// what makes "no msg2 was sent" an observation about the confirmation rather
|
||||||
|
/// than a property of the fixture: on an unstarted transport every send
|
||||||
|
/// fails, and the send-failure arm records the same reject the confirmation
|
||||||
|
/// records, so the two worlds would be indistinguishable.
|
||||||
|
async fn node_refusing_inbound(
|
||||||
|
transport_id: TransportId,
|
||||||
|
) -> (
|
||||||
|
Node,
|
||||||
|
TransportAddr,
|
||||||
|
crate::transport::PacketRx,
|
||||||
|
crate::transport::udp::UdpTransport,
|
||||||
|
) {
|
||||||
|
use crate::config::UdpConfig;
|
||||||
|
use crate::transport::udp::UdpTransport;
|
||||||
|
|
||||||
|
let mut node = make_node();
|
||||||
|
|
||||||
|
let refusing = UdpConfig {
|
||||||
|
bind_addr: Some("127.0.0.1:0".to_string()),
|
||||||
|
accept_connections: Some(false),
|
||||||
|
..Default::default()
|
||||||
|
};
|
||||||
|
let (tx, _rx) = packet_channel(64);
|
||||||
|
let mut udp = UdpTransport::new(transport_id, None, refusing, tx);
|
||||||
|
udp.start_async().await.expect("node transport must bind");
|
||||||
|
node.transports
|
||||||
|
.insert(transport_id, TransportHandle::Udp(udp));
|
||||||
|
|
||||||
|
let far_end_config = UdpConfig {
|
||||||
|
bind_addr: Some("127.0.0.1:0".to_string()),
|
||||||
|
..Default::default()
|
||||||
|
};
|
||||||
|
let (far_tx, far_rx) = packet_channel(64);
|
||||||
|
let mut far_end = UdpTransport::new(TransportId::new(200), None, far_end_config, far_tx);
|
||||||
|
far_end.start_async().await.expect("far end must bind");
|
||||||
|
let far_addr = TransportAddr::from_string(
|
||||||
|
&far_end
|
||||||
|
.local_addr()
|
||||||
|
.expect("a started transport has a local address")
|
||||||
|
.to_string(),
|
||||||
|
);
|
||||||
|
|
||||||
|
(node, far_addr, far_rx, far_end)
|
||||||
|
}
|
||||||
|
|
||||||
|
/// A real, cryptographically valid msg1 from `initiator` addressed to
|
||||||
|
/// `responder`'s static key.
|
||||||
|
///
|
||||||
|
/// It has to be valid under our static, or `receive_handshake_init` refuses
|
||||||
|
/// it for the wrong reason and the test passes without ever reaching the
|
||||||
|
/// confirmation.
|
||||||
|
fn genuine_msg1(initiator: &Node, responder: &Node) -> Vec<u8> {
|
||||||
|
use crate::node::wire::build_msg1;
|
||||||
|
|
||||||
|
let responder_identity = PeerIdentity::from_pubkey_full(responder.identity().pubkey_full());
|
||||||
|
let mut conn = PeerConnection::outbound(LinkId::new(9_999), responder_identity, 1_000);
|
||||||
|
let noise_msg1 = conn
|
||||||
|
.start_handshake(
|
||||||
|
initiator.identity().keypair(),
|
||||||
|
initiator.startup_epoch(),
|
||||||
|
1_000,
|
||||||
|
)
|
||||||
|
.expect("the initiator side of a real msg1 must build");
|
||||||
|
build_msg1(SessionIndex::new(7), &noise_msg1)
|
||||||
|
}
|
||||||
|
|
||||||
|
/// The node address a `Node`'s own identity presents to its peers.
|
||||||
|
fn node_addr_of(node: &Node) -> NodeAddr {
|
||||||
|
*PeerIdentity::from_pubkey_full(node.identity().pubkey_full()).node_addr()
|
||||||
|
}
|
||||||
|
|
||||||
|
/// The FMP phase byte of a packet, for telling a msg1 from a msg2 on the wire.
|
||||||
|
fn wire_phase(data: &[u8]) -> Option<u8> {
|
||||||
|
crate::node::wire::CommonPrefix::parse(data).map(|p| p.phase)
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Assert that nothing arrives on `rx` within a window long enough for a
|
||||||
|
/// localhost datagram to have been delivered had one been sent.
|
||||||
|
async fn assert_nothing_sent(rx: &mut crate::transport::PacketRx, why: &str) {
|
||||||
|
let arrival = tokio::time::timeout(std::time::Duration::from_millis(250), rx.recv()).await;
|
||||||
|
assert!(arrival.is_err(), "{}", why);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[tokio::test]
|
||||||
|
async fn a_msg1_spoofed_from_an_established_peers_address_is_dropped_after_the_dh_reveals_a_different_identity()
|
||||||
|
{
|
||||||
|
use crate::peer::ActivePeer;
|
||||||
|
|
||||||
|
let transport_id = TransportId::new(1);
|
||||||
|
let (mut node, victim_addr, mut far_rx, _far_end) = node_refusing_inbound(transport_id).await;
|
||||||
|
|
||||||
|
// The victim: a promoted peer at the address the spoofed msg1 will be
|
||||||
|
// sourced from, so the carve-out admits the msg1 past the refusing gate.
|
||||||
|
let victim = make_node();
|
||||||
|
let victim_identity = PeerIdentity::from_pubkey_full(victim.identity().pubkey_full());
|
||||||
|
let victim_node_addr = *victim_identity.node_addr();
|
||||||
|
let victim_link = node.allocate_link_id();
|
||||||
|
let mut victim_peer = ActivePeer::new(victim_identity, victim_link, 1_000);
|
||||||
|
victim_peer.set_current_addr(transport_id, victim_addr.clone());
|
||||||
|
node.peers.insert(victim_node_addr, victim_peer);
|
||||||
|
node.addr_to_link
|
||||||
|
.insert((transport_id, victim_addr.clone()), victim_link);
|
||||||
|
|
||||||
|
// The off-path party: a genuine msg1 under our static, built with a
|
||||||
|
// different identity's keypair, sourced from the victim's address.
|
||||||
|
let attacker = make_node();
|
||||||
|
let attacker_node_addr = node_addr_of(&attacker);
|
||||||
|
let wire_msg1 = genuine_msg1(&attacker, &node);
|
||||||
|
|
||||||
|
let bad_state_before = node.stats().handshake.bad_state;
|
||||||
|
node.handle_msg1(ReceivedPacket::with_timestamp(
|
||||||
|
transport_id,
|
||||||
|
victim_addr.clone(),
|
||||||
|
wire_msg1,
|
||||||
|
2_000,
|
||||||
|
))
|
||||||
|
.await;
|
||||||
|
|
||||||
|
assert!(
|
||||||
|
node.get_peer(&attacker_node_addr).is_none(),
|
||||||
|
"the identity the DH revealed does not own the address the waiver \
|
||||||
|
matched, so it must not become a peer"
|
||||||
|
);
|
||||||
|
assert_eq!(
|
||||||
|
node.peer_count(),
|
||||||
|
1,
|
||||||
|
"only the victim may remain a peer after the spoofed msg1"
|
||||||
|
);
|
||||||
|
assert_eq!(
|
||||||
|
node.connection_count(),
|
||||||
|
0,
|
||||||
|
"the rejected msg1 must leave no connection behind"
|
||||||
|
);
|
||||||
|
assert_eq!(
|
||||||
|
node.link_count(),
|
||||||
|
0,
|
||||||
|
"the rejected msg1 must leave no link behind"
|
||||||
|
);
|
||||||
|
assert_eq!(
|
||||||
|
node.stats().handshake.bad_state - bad_state_before,
|
||||||
|
1,
|
||||||
|
"the drop must be counted"
|
||||||
|
);
|
||||||
|
assert_nothing_sent(
|
||||||
|
&mut far_rx,
|
||||||
|
"no msg2 may reach the victim's address: the responder answers only \
|
||||||
|
after the confirmation, and this msg1 must not get that far",
|
||||||
|
)
|
||||||
|
.await;
|
||||||
|
}
|
||||||
|
|
||||||
|
#[tokio::test]
|
||||||
|
async fn a_msg1_admitted_by_a_link_no_identity_owns_is_dropped_after_the_dh() {
|
||||||
|
let transport_id = TransportId::new(1);
|
||||||
|
let (mut node, source_addr, mut far_rx, _far_end) = node_refusing_inbound(transport_id).await;
|
||||||
|
|
||||||
|
// The fixture `test_should_admit_msg1_admits_rekey_when_accept_off` uses:
|
||||||
|
// a reverse-address entry with no peer and no connection behind it. It is
|
||||||
|
// enough to waive the refusing gate, and it attributes the address to
|
||||||
|
// nobody.
|
||||||
|
let link_id = node.allocate_link_id();
|
||||||
|
node.addr_to_link
|
||||||
|
.insert((transport_id, source_addr.clone()), link_id);
|
||||||
|
|
||||||
|
assert!(
|
||||||
|
node.should_admit_msg1(transport_id, &source_addr),
|
||||||
|
"the fixture must exercise the carve-out, not the gate"
|
||||||
|
);
|
||||||
|
|
||||||
|
let initiator = make_node();
|
||||||
|
let initiator_node_addr = node_addr_of(&initiator);
|
||||||
|
let wire_msg1 = genuine_msg1(&initiator, &node);
|
||||||
|
|
||||||
|
let bad_state_before = node.stats().handshake.bad_state;
|
||||||
|
node.handle_msg1(ReceivedPacket::with_timestamp(
|
||||||
|
transport_id,
|
||||||
|
source_addr.clone(),
|
||||||
|
wire_msg1,
|
||||||
|
2_000,
|
||||||
|
))
|
||||||
|
.await;
|
||||||
|
|
||||||
|
assert!(
|
||||||
|
node.get_peer(&initiator_node_addr).is_none(),
|
||||||
|
"a msg1 the waiver could attribute to no identity must not promote \
|
||||||
|
the identity the DH revealed"
|
||||||
|
);
|
||||||
|
assert_eq!(node.peer_count(), 0, "no peer may be created");
|
||||||
|
assert_eq!(
|
||||||
|
node.connection_count(),
|
||||||
|
0,
|
||||||
|
"the rejected msg1 must leave no connection behind"
|
||||||
|
);
|
||||||
|
assert_eq!(
|
||||||
|
node.link_count(),
|
||||||
|
0,
|
||||||
|
"the rejected msg1 must leave no link behind"
|
||||||
|
);
|
||||||
|
assert_eq!(
|
||||||
|
node.stats().handshake.bad_state - bad_state_before,
|
||||||
|
1,
|
||||||
|
"the drop must be counted"
|
||||||
|
);
|
||||||
|
assert_nothing_sent(
|
||||||
|
&mut far_rx,
|
||||||
|
"no msg2 may leave the node for an address no identity owns",
|
||||||
|
)
|
||||||
|
.await;
|
||||||
|
}
|
||||||
|
|
||||||
|
#[tokio::test]
|
||||||
|
async fn a_msg1_from_a_link_whose_dial_expects_this_identity_is_admitted() {
|
||||||
|
let transport_id = TransportId::new(1);
|
||||||
|
let (mut node, peer_addr, mut far_rx, _far_end) = node_refusing_inbound(transport_id).await;
|
||||||
|
|
||||||
|
// UDP is connectionless, so `initiate_connection` runs `start_handshake`
|
||||||
|
// in the same synchronous stretch: the reverse-address entry and the
|
||||||
|
// connection carrying the dialled identity land together, and the
|
||||||
|
// classifier can attribute the address. Do not substitute a
|
||||||
|
// connection-oriented transport here; that arm defers `start_handshake`
|
||||||
|
// and is the window the next test is about.
|
||||||
|
let peer = make_node();
|
||||||
|
let peer_identity = PeerIdentity::from_pubkey_full(peer.identity().pubkey_full());
|
||||||
|
node.initiate_connection(transport_id, peer_addr.clone(), peer_identity)
|
||||||
|
.await
|
||||||
|
.expect("the dial must register the link and the connection");
|
||||||
|
|
||||||
|
// Our own dial's msg1 went out first; drain it so the assertion below is
|
||||||
|
// about the answer to the crossing msg1.
|
||||||
|
let ours = tokio::time::timeout(std::time::Duration::from_secs(1), far_rx.recv())
|
||||||
|
.await
|
||||||
|
.expect("our dial's msg1 must arrive")
|
||||||
|
.expect("the far end's channel must be open");
|
||||||
|
assert_eq!(
|
||||||
|
wire_phase(&ours.data),
|
||||||
|
Some(crate::node::wire::PHASE_MSG1),
|
||||||
|
"the dial's own packet is a msg1"
|
||||||
|
);
|
||||||
|
|
||||||
|
let bad_state_before = node.stats().handshake.bad_state;
|
||||||
|
let wire_msg1 = genuine_msg1(&peer, &node);
|
||||||
|
node.handle_msg1(ReceivedPacket::with_timestamp(
|
||||||
|
transport_id,
|
||||||
|
peer_addr.clone(),
|
||||||
|
wire_msg1,
|
||||||
|
2_000,
|
||||||
|
))
|
||||||
|
.await;
|
||||||
|
|
||||||
|
let answer = tokio::time::timeout(std::time::Duration::from_secs(1), far_rx.recv())
|
||||||
|
.await
|
||||||
|
.expect("the crossing msg1 must be answered")
|
||||||
|
.expect("the far end's channel must be open");
|
||||||
|
assert_eq!(
|
||||||
|
wire_phase(&answer.data),
|
||||||
|
Some(crate::node::wire::PHASE_MSG2),
|
||||||
|
"a simultaneous open with the peer we dialled must still be answered"
|
||||||
|
);
|
||||||
|
assert_eq!(
|
||||||
|
node.stats().handshake.bad_state - bad_state_before,
|
||||||
|
0,
|
||||||
|
"a crossing msg1 from the identity the dial expects must not be \
|
||||||
|
rejected"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[tokio::test]
|
||||||
|
async fn a_crossing_msg1_in_the_connection_oriented_dial_window_is_rejected() {
|
||||||
|
use crate::config::TcpConfig;
|
||||||
|
use crate::transport::tcp::TcpTransport;
|
||||||
|
|
||||||
|
let mut node = make_node();
|
||||||
|
let transport_id = TransportId::new(1);
|
||||||
|
|
||||||
|
// bind_addr=None makes accept_connections() false, the idiom
|
||||||
|
// `test_should_admit_msg1_rejects_fresh_when_accept_off` uses.
|
||||||
|
let cfg = TcpConfig {
|
||||||
|
bind_addr: None,
|
||||||
|
..Default::default()
|
||||||
|
};
|
||||||
|
let (tx, _rx) = packet_channel(64);
|
||||||
|
let tcp = TcpTransport::new(transport_id, None, cfg, tx);
|
||||||
|
node.transports
|
||||||
|
.insert(transport_id, TransportHandle::Tcp(tcp));
|
||||||
|
|
||||||
|
let peer = make_node();
|
||||||
|
let peer_identity = PeerIdentity::from_pubkey_full(peer.identity().pubkey_full());
|
||||||
|
let addr = TransportAddr::from_string("10.0.0.2:2121");
|
||||||
|
|
||||||
|
// The state `initiate_connection`'s connection-oriented arm leaves behind
|
||||||
|
// while the transport connect is outstanding: a link, a reverse-address
|
||||||
|
// entry, a pending connect, and no connection yet. Built directly rather
|
||||||
|
// than by driving a connect, which would need a reachable peer.
|
||||||
|
let link_id = node.allocate_link_id();
|
||||||
|
let link = Link::new(
|
||||||
|
link_id,
|
||||||
|
transport_id,
|
||||||
|
addr.clone(),
|
||||||
|
LinkDirection::Outbound,
|
||||||
|
Duration::from_millis(100),
|
||||||
|
);
|
||||||
|
node.links.insert(link_id, link);
|
||||||
|
node.addr_to_link
|
||||||
|
.insert((transport_id, addr.clone()), link_id);
|
||||||
|
node.pending_connects.push(PendingConnect {
|
||||||
|
link_id,
|
||||||
|
transport_id,
|
||||||
|
remote_addr: addr.clone(),
|
||||||
|
peer_identity,
|
||||||
|
});
|
||||||
|
|
||||||
|
let bad_state_before = node.stats().handshake.bad_state;
|
||||||
|
let wire_msg1 = genuine_msg1(&peer, &node);
|
||||||
|
node.handle_msg1(ReceivedPacket::with_timestamp(
|
||||||
|
transport_id,
|
||||||
|
addr.clone(),
|
||||||
|
wire_msg1,
|
||||||
|
2_000,
|
||||||
|
))
|
||||||
|
.await;
|
||||||
|
|
||||||
|
// This is the assertion that separates the two worlds, and it is the one
|
||||||
|
// to read first when the test reds. After the change the reject returns
|
||||||
|
// above every registry mutation, so the dial's own entry is untouched.
|
||||||
|
// Without it, `handle_msg1` allocates a fresh link id, writes it over
|
||||||
|
// this entry, and then removes the entry outright when the msg2 send
|
||||||
|
// fails on the unstarted transport.
|
||||||
|
assert_eq!(
|
||||||
|
node.addr_to_link.get(&(transport_id, addr.clone())),
|
||||||
|
Some(&link_id),
|
||||||
|
"the dial's reverse-address entry must still name the dial's own link"
|
||||||
|
);
|
||||||
|
// The remaining three state the shape of the outcome. They hold either
|
||||||
|
// way for this fixture, whose unstarted TCP transport cannot send, so
|
||||||
|
// they are not what makes this test able to fail.
|
||||||
|
assert_eq!(node.peer_count(), 0, "no peer may be created");
|
||||||
|
assert_eq!(node.connection_count(), 0, "no connection may be created");
|
||||||
|
assert_eq!(
|
||||||
|
node.stats().handshake.bad_state - bad_state_before,
|
||||||
|
1,
|
||||||
|
"the drop must be counted"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[tokio::test]
|
||||||
|
async fn msg1_waiver_classifies_all_three_outcomes() {
|
||||||
|
use crate::config::UdpConfig;
|
||||||
|
use crate::node::handlers::handshake::Msg1Waiver;
|
||||||
|
use crate::peer::ActivePeer;
|
||||||
|
use crate::transport::udp::UdpTransport;
|
||||||
|
|
||||||
|
let mut node = make_node();
|
||||||
|
|
||||||
|
// A registered accepting transport, not an unregistered id, so the
|
||||||
|
// NotNeeded assertion exercises the `accept_connections()` limb of the
|
||||||
|
// guard rather than its absent-transport fallback.
|
||||||
|
let accepting_id = TransportId::new(1);
|
||||||
|
let (accept_tx, _accept_rx) = packet_channel(64);
|
||||||
|
let accepting = UdpTransport::new(
|
||||||
|
accepting_id,
|
||||||
|
None,
|
||||||
|
UdpConfig {
|
||||||
|
bind_addr: Some("127.0.0.1:0".to_string()),
|
||||||
|
accept_connections: Some(true),
|
||||||
|
..Default::default()
|
||||||
|
},
|
||||||
|
accept_tx,
|
||||||
|
);
|
||||||
|
node.transports
|
||||||
|
.insert(accepting_id, TransportHandle::Udp(accepting));
|
||||||
|
|
||||||
|
let refusing_id = TransportId::new(2);
|
||||||
|
let (refuse_tx, _refuse_rx) = packet_channel(64);
|
||||||
|
let refusing = UdpTransport::new(
|
||||||
|
refusing_id,
|
||||||
|
None,
|
||||||
|
UdpConfig {
|
||||||
|
bind_addr: Some("127.0.0.1:0".to_string()),
|
||||||
|
accept_connections: Some(false),
|
||||||
|
..Default::default()
|
||||||
|
},
|
||||||
|
refuse_tx,
|
||||||
|
);
|
||||||
|
node.transports
|
||||||
|
.insert(refusing_id, TransportHandle::Udp(refusing));
|
||||||
|
|
||||||
|
let classify = |node: &Node, transport_id: TransportId, addr: &TransportAddr| {
|
||||||
|
node.msg1_waiver(
|
||||||
|
node.is_established_link_msg1(transport_id, addr),
|
||||||
|
transport_id,
|
||||||
|
addr,
|
||||||
|
)
|
||||||
|
};
|
||||||
|
|
||||||
|
// NotNeeded: the gate would have admitted this msg1 anyway, so nothing
|
||||||
|
// was waived and there is nothing to confirm.
|
||||||
|
let fresh = TransportAddr::from_string("10.0.0.2:2121");
|
||||||
|
assert_eq!(
|
||||||
|
classify(&node, accepting_id, &fresh),
|
||||||
|
Msg1Waiver::NotNeeded,
|
||||||
|
"an accepting transport waives nothing"
|
||||||
|
);
|
||||||
|
|
||||||
|
// Unattributed: the carve-out admits on a bare reverse-address entry
|
||||||
|
// that names neither a promoted peer nor a carrier.
|
||||||
|
let bare = TransportAddr::from_string("10.0.0.3:2121");
|
||||||
|
let bare_link = node.allocate_link_id();
|
||||||
|
node.addr_to_link
|
||||||
|
.insert((refusing_id, bare.clone()), bare_link);
|
||||||
|
assert_eq!(
|
||||||
|
classify(&node, refusing_id, &bare),
|
||||||
|
Msg1Waiver::Unattributed,
|
||||||
|
"a link no identity owns must fail closed"
|
||||||
|
);
|
||||||
|
|
||||||
|
// Expect, by promoted peer. `ActivePeer::new` sets neither transport_id
|
||||||
|
// nor current_addr, so this peer is reached through the reverse-address
|
||||||
|
// entry that names its link.
|
||||||
|
let promoted_addr = TransportAddr::from_string("10.0.0.4:2121");
|
||||||
|
let promoted_link = node.allocate_link_id();
|
||||||
|
let promoted = make_peer_identity();
|
||||||
|
let promoted_node_addr = *promoted.node_addr();
|
||||||
|
node.peers.insert(
|
||||||
|
promoted_node_addr,
|
||||||
|
ActivePeer::new(promoted, promoted_link, 1_000),
|
||||||
|
);
|
||||||
|
node.addr_to_link
|
||||||
|
.insert((refusing_id, promoted_addr.clone()), promoted_link);
|
||||||
|
assert_eq!(
|
||||||
|
classify(&node, refusing_id, &promoted_addr),
|
||||||
|
Msg1Waiver::Expect(promoted_node_addr),
|
||||||
|
"an address whose link a promoted peer owns is attributed to that peer"
|
||||||
|
);
|
||||||
|
|
||||||
|
// Expect, by carrier: a link with no promoted peer yet, whose connection
|
||||||
|
// carries the identity the dial expects.
|
||||||
|
let carrier_addr = TransportAddr::from_string("10.0.0.5:2121");
|
||||||
|
let carrier_link = node.allocate_link_id();
|
||||||
|
let dialled = make_peer_identity();
|
||||||
|
let dialled_node_addr = *dialled.node_addr();
|
||||||
|
node.connections.insert(
|
||||||
|
carrier_link,
|
||||||
|
PeerConnection::outbound(carrier_link, dialled, 1_000),
|
||||||
|
);
|
||||||
|
node.addr_to_link
|
||||||
|
.insert((refusing_id, carrier_addr.clone()), carrier_link);
|
||||||
|
assert_eq!(
|
||||||
|
classify(&node, refusing_id, &carrier_addr),
|
||||||
|
Msg1Waiver::Expect(dialled_node_addr),
|
||||||
|
"an address whose link carries a dialled identity is attributed to it"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[tokio::test]
|
||||||
|
async fn a_stale_reverse_address_entry_does_not_hide_a_peer_reachable_by_address() {
|
||||||
|
use crate::config::UdpConfig;
|
||||||
|
use crate::node::handlers::handshake::Msg1Waiver;
|
||||||
|
use crate::peer::ActivePeer;
|
||||||
|
use crate::transport::udp::UdpTransport;
|
||||||
|
|
||||||
|
let mut node = make_node();
|
||||||
|
let transport_id = TransportId::new(1);
|
||||||
|
let (tx, _rx) = packet_channel(64);
|
||||||
|
let udp = UdpTransport::new(
|
||||||
|
transport_id,
|
||||||
|
None,
|
||||||
|
UdpConfig {
|
||||||
|
bind_addr: Some("127.0.0.1:0".to_string()),
|
||||||
|
accept_connections: Some(false),
|
||||||
|
..Default::default()
|
||||||
|
},
|
||||||
|
tx,
|
||||||
|
);
|
||||||
|
node.transports
|
||||||
|
.insert(transport_id, TransportHandle::Udp(udp));
|
||||||
|
|
||||||
|
// A live peer whose current_addr is the numeric form inbound packets
|
||||||
|
// carry, which is the second predicate's whole reason for existing.
|
||||||
|
let numeric = TransportAddr::from_string("100.64.0.5:2121");
|
||||||
|
let peer_link = node.allocate_link_id();
|
||||||
|
let peer_identity = make_peer_identity();
|
||||||
|
let peer_node_addr = *peer_identity.node_addr();
|
||||||
|
let mut peer = ActivePeer::new(peer_identity, peer_link, 1_000);
|
||||||
|
peer.set_current_addr(transport_id, numeric.clone());
|
||||||
|
node.peers.insert(peer_node_addr, peer);
|
||||||
|
|
||||||
|
// A reverse-address entry at that same numeric address naming a link
|
||||||
|
// that no longer exists. `remove_link` clears the reverse lookup only
|
||||||
|
// under the key it rebuilds from the removed link's own remote address,
|
||||||
|
// so an entry inserted for that link under a second address form outlives
|
||||||
|
// it, and link ids are never reused.
|
||||||
|
let dead_link = node.allocate_link_id();
|
||||||
|
assert_ne!(
|
||||||
|
dead_link, peer_link,
|
||||||
|
"the stale entry names a different link"
|
||||||
|
);
|
||||||
|
node.addr_to_link
|
||||||
|
.insert((transport_id, numeric.clone()), dead_link);
|
||||||
|
|
||||||
|
assert_eq!(
|
||||||
|
node.msg1_waiver(
|
||||||
|
node.is_established_link_msg1(transport_id, &numeric),
|
||||||
|
transport_id,
|
||||||
|
&numeric
|
||||||
|
),
|
||||||
|
Msg1Waiver::Expect(peer_node_addr),
|
||||||
|
"the stale entry must not hide the peer the address scan finds: \
|
||||||
|
classifying this Unattributed would reject the peer's rekey msg1 \
|
||||||
|
after the DH, and would go on rejecting it, because the reject \
|
||||||
|
returns above the insert that would overwrite the stale entry"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user