cloudflare
Peer indices should be unpredictable and unique per session
Peer indices (sender/receiver IDs) seem to start with zero and are incremented by one:
impl Device {
fn next_index(&mut self) -> u32 {
let next_index = self.next_index;
self.next_index += 1;
assert!(next_index < (1 << 24), "Too many peers created");
next_index
}
// ...
fn update_peer(...) {
// ...
let next_index = self.next_index();
// ...
let mut tunn = Tunn::new(
// ...
next_index,
)
.unwrap();
// ...
let peer = Peer::new(tunn, next_index, endpoint, &allowed_ips, preshared_key);
// ...
self.peers_by_idx.insert(next_index, Arc::clone(&peer));
Which are used right here:
fn register_udp_handler(&self, udp: Arc<UDPSocket>) -> Result<(), Error> {
// ...
while let Ok((addr, packet)) = udp.recvfrom(&mut t.src_buf[..]) {
// ...
let peer = match (packet[0], packet.len()) {
// ...
HANDSHAKE_RESPONSE => {
let peer_idx = u32::from_le_bytes(make_array(&packet[8..])) >> 8;
peers_by_idx.get(&peer_idx)
}
COOKIE_REPLY => {
let peer_idx = u32::from_le_bytes(make_array(&packet[4..])) >> 8;
peers_by_idx.get(&peer_idx)
}
(DATA, DATA_OVERHEAD_SZ...std::usize::MAX) => {
// A data packet, with at least a header
let peer_idx = u32::from_le_bytes(make_array(&packet[4..])) >> 8;
peers_by_idx.get(&peer_idx)
}
Problems:
- The receiver/sender ID should be random and unpredictable. Otherwise it could give an attacker an advantage in pulling off a denial of service attack.
- A monotonically increasing counter from 0 could leak activity and reveal whether a peer has actually connected or not. This kind of side-channel information could potentially be used in another attack.
- Two sessions (after key rotation) should not have the same sender/receiver ID. That would make them linkable.
I don't think there is a requirement for that in the wireguard specification. Unlinkability is not a property of wireguard
I can't find "unlinkability" in the WG whitepaper either. Based on the message interactions and IP/ports the protocol is already linkable, but this decision could make it worse by having another way in the protocol to link sessions.
My third "problem" was based on a misinterpretation, I overlooked the >> 8. So the top 24 bit is used for the peer ID, the last 8 bits are reserved for the "cyclic session index" (incremented via inc_index in handshake.rs). So actually the session IDs are kind of unique.
Being able to identify a peer based on the sender ID is undesirable though. The WG protocol has a weak form of identity hiding, including a peer identifier violates this property even more.