Skip to content

fix(dictyon): validate server-supplied key-hex and routing-data fields at netmap ingestion #97

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
forkwright merged 3 commits into from
Aug 16, 2026
Merged

fix(dictyon): validate server-supplied key-hex and routing-data fields at netmap ingestion #97

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
424cd4b
fix(dictyon): validate server-supplied key-hex and routing-data field…
Aug 16, 2026
c230008
chore(gate): carry the gate stamp for fix/55-audit-wave-1-batch
forkwright Aug 16, 2026
ead8187
test(dictyon): pin is_valid_cidr's prefix-length boundary per family
Aug 16, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
235 changes: 16 additions & 219 deletions crates/dictyon/src/control/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,34 +17,32 @@
//! - `control/controlclient/direct.go` in the Tailscale Go source
//! - `tailcfg/tailcfg.go` for type definitions

use std::collections::HashSet;

use mitos::capability::CAPABILITY_VERSION;
use mitos::keys::{DiscoPrivate, MachinePrivate, NodePrivate};
use mitos::types::{
AuthInfo, BackendLogId, DerpMap, DnsConfig, Hostinfo, MapRequest, MapResponse, Node,
PeerChange, PeerRemoval, RegisterRequest, RegisterResponse,
AuthInfo, BackendLogId, Hostinfo, MapRequest, MapResponse, Node, RegisterRequest,
RegisterResponse,
};
use snafu::Snafu;
use tracing::{debug, warn};

use crate::transport::ControlConnection;
use crate::wire::AsyncControlStream;

mod netmap;
mod register;
mod validate;

pub use netmap::Netmap;
// WHY(cfg(test)): the only consumer is `control::tests` (via `use
// super::*`), which is itself `#[cfg(test)]` -- a non-test `cargo check`
// never compiles that consumer, so an unconditional re-export here reads as
// unused outside test builds even though test builds do use it.
#[cfg(test)]
pub(crate) use netmap::MAX_PEERS;
use register::classify_register_response;
pub use register::{NonEmptyUrl, RegisterFault, RegisterOutcome};

/// Upper bound on the peers a netmap retains from a coordination server.
///
/// WHY: every peer-bearing field of a [`MapResponse`] is server-controlled and
/// unbounded on the wire, so a misbehaving or hostile server can grow
/// [`Netmap::peers`] without limit across a single connection. The cap sits far
/// above any realistic tailnet, so it is unreachable in normal operation and
/// only bounds the memory one connection can commit.
const MAX_PEERS: usize = 10_000;

/// Errors from control protocol operations.
#[derive(Debug, Snafu)]
#[non_exhaustive]
Expand Down Expand Up @@ -105,118 +103,6 @@ impl From<crate::wire::WireError> for ControlError {
}
}

/// The local view of the network map, maintained by applying
/// [`MapResponse`] updates.
///
/// Starts empty and is populated by the first full map response.
/// Subsequent delta responses update it incrementally.
#[derive(Debug)]
pub struct Netmap {
/// This node's own information.
pub self_node: Node,
/// Known peers in the tailnet.
pub peers: Vec<Node>,
/// Current DNS configuration.
pub dns_config: Option<DnsConfig>,
/// Current DERP relay topology.
pub derp_map: Option<DerpMap>,
}

impl Netmap {
/// Build a netmap from the first full [`MapResponse`].
///
/// A missing self node falls back to a zero-value [`Node`]; missing
/// peers default to an empty list.
fn from_full_response(resp: MapResponse) -> Self {
let self_node = resp.node.unwrap_or_else(|| Node {
id: 0,
stable_id: None,
key: String::new(), // kanon:ignore RUST/plain-string-secret -- public key hex, not a secret
machine: None,
name: String::new(),
cap: None,
tags: None,
addresses: Vec::new(),
allowed_ips: None,
endpoints: None,
derp: None,
disco_key: None,
key_expiry: None,
last_seen: None,
online: None,
});

let mut peers = resp.peers.unwrap_or_default();
cap_peers(&mut peers);

Self {
self_node,
peers,
dns_config: resp.dns_config,
derp_map: resp.derp_map,
}
}

/// Apply a delta [`MapResponse`] onto an already-initialized netmap.
///
/// See [`ControlClient::apply_map_response`] for the merge semantics.
fn apply_delta(&mut self, resp: MapResponse) {
if let Some(node) = resp.node {
self.self_node = node;
}

// Full peer replacement (if server sends full list again).
if let Some(mut peers) = resp.peers {
cap_peers(&mut peers);
self.peers = peers;
}

// Incremental peer additions/changes.
if let Some(changed) = resp.peers_changed {
// WHY: counted rather than warned per peer, so a server sending a
// large over-cap delta costs one log line instead of one per peer.
let mut refused = 0usize;
for changed_peer in changed {
if let Some(existing) = self.peers.iter_mut().find(|p| p.key == changed_peer.key) {
// WHY: an update to a peer already held is not growth, so it
// still applies once the netmap has reached the cap.
*existing = changed_peer;
} else if self.peers.len() < MAX_PEERS {
self.peers.push(changed_peer);
} else {
refused += 1;
}
}
if refused > 0 {
warn!(
cap = MAX_PEERS,
refused, "netmap is at the peer cap; refused new peers from a map response"
);
}
}

// Peer removals.
if let Some(removals) = resp.peers_removed {
let index = PeerRemovalIndex::build(&removals);
self.peers.retain(|peer| !index.removes(peer));
}

if let Some(changes) = resp.peers_changed_patch {
for change in changes {
apply_peer_change(&mut self.peers, change);
}
}

if let Some(dns) = resp.dns_config {
self.dns_config = Some(dns);
}

if let Some(derp) = resp.derp_map {
self.derp_map = Some(derp);
}
}
}

/// Client for the Tailscale control protocol.
///
/// Wraps a [`ControlConnection`] and the node's identity keys. Provides
Expand Down Expand Up @@ -500,6 +386,11 @@ impl ControlClient {
/// The retained peer list is bounded by [`MAX_PEERS`]: an over-cap list is
/// truncated and additions past the cap are refused. Updates to peers
/// already held still apply, since they do not grow the list.
///
/// Every `Node`'s key-hex and routing-data fields, and every DNS
/// resolver address, are validated before entering the netmap; a peer,
/// self-node update, or resolver that fails is dropped rather than
/// admitted with unparseable data.
pub fn apply_map_response(&mut self, resp: MapResponse) {
if resp.keep_alive == Some(true) {
return;
Expand Down Expand Up @@ -545,100 +436,6 @@ impl ControlClient {
}
}

fn apply_peer_change(peers: &mut [Node], change: PeerChange) {
let Some(peer) = peers.iter_mut().find(|peer| peer.id == change.node_id) else {
return;
};

if let Some(region) = change.derp_region.filter(|region| *region > 0) {
peer.derp = Some(format!("127.3.3.40:{region}"));
}

if let Some(cap) = change.cap.filter(|cap| *cap > 0) {
peer.cap = Some(cap);
}

if let Some(endpoints) = change.endpoints.filter(|endpoints| !endpoints.is_empty()) {
peer.endpoints = Some(endpoints);
}

if let Some(key) = change.key {
peer.key = key;
}

if let Some(disco_key) = change.disco_key {
peer.disco_key = Some(disco_key);
}

if let Some(online) = change.online {
peer.online = Some(online);
}

if let Some(last_seen) = change.last_seen {
peer.last_seen = Some(last_seen);
}

if let Some(key_expiry) = change.key_expiry {
peer.key_expiry = Some(key_expiry);
}
}

/// Truncate a server-supplied peer list to [`MAX_PEERS`].
///
/// WHY: shared by both paths that accept a whole list from the server, so the
/// cap cannot be enforced on one and forgotten on the other.
fn cap_peers(peers: &mut Vec<Node>) {
if peers.len() > MAX_PEERS {
let dropped = peers.len() - MAX_PEERS;
warn!(
cap = MAX_PEERS,
dropped, "map response exceeded the peer cap; dropped the excess peers"
);
peers.truncate(MAX_PEERS);
}
}

/// O(1) membership test over a server-supplied peer-removal list.
///
/// WHY: both the peer list and the removal list arrive from the coordination
/// server, so scanning the removals once per peer is O(n*m) work a server can
/// demand at will by sending a large delta. Indexing the removals first makes
/// the sweep linear in the peer count regardless of how many removals arrive.
struct PeerRemovalIndex<'a> {
node_ids: HashSet<i64>,
node_keys: HashSet<&'a str>,
}

impl<'a> PeerRemovalIndex<'a> {
/// Index a removal list by the two identifiers it can name a peer with.
fn build(removals: &'a [PeerRemoval]) -> Self {
let mut node_ids = HashSet::new();
let mut node_keys = HashSet::new();
for removal in removals {
match removal {
PeerRemoval::NodeId(node_id) => {
node_ids.insert(*node_id);
}
PeerRemoval::NodeKey(key) => {
node_keys.insert(key.as_str());
}
// WHY: PeerRemoval is #[non_exhaustive]; an unrecognized
// variant names a peer this client cannot index by ID or key.
_ => {} // skipped rather than treated as a match failure
}
}
Self {
node_ids,
node_keys,
}
}

/// Whether this removal set names `peer`, by node ID or by node key.
fn removes(&self, peer: &Node) -> bool {
self.node_ids.contains(&peer.id) || self.node_keys.contains(peer.key.as_str())
}
}

/// Returns the system hostname, falling back to `"unknown"`.
fn gethostname() -> String {
hostname::get()
Expand Down
Loading