Skip to content

Rotating a device's public key silently breaks its tunnel — the gateway applies Modify as Delete #368

Description

@sashasimkin

What happens

Change a device's public key over the API and the tunnel stops carrying traffic, with no error anywhere. The API still shows the new key, because it reads the database. The gateway still runs and reports nothing wrong. But the WireGuard interface keeps the old key, so the client's handshakes keep succeeding while every data packet is silently dropped.

The cause is a single comparison. handle_updates in the gateway checks update.update_type against the literal 2 and treats that as Delete. In the shared proto, 2 is UPDATE_TYPE_MODIFY and 3 is UPDATE_TYPE_DELETE. So Modify and Delete are applied as each other's opposite: a Modify removes a peer, and a Delete writes one with no addresses.

Restarting the gateway fixes it, because the full configuration it receives on reconnect takes a different path and is rendered from the database.

Environment

  • defguard core 2.0.3
  • defguard-gateway 2.0.3, kernel WireGuard (userspace = false), started with --config /etc/defguard/gateway.toml
  • Ubuntu 24.04, x86_64
  • one location, one gateway, network devices managed over the REST API

Related evidence

Cases 1-4 below are live gateway logs at log_level = "debug", captured on a running instance. Public keys, addresses and identifiers are replaced with placeholders; everything else is verbatim. The proto and source excerpts are from the public repositories at the tags named. The suggested direction has not been compiled or tested.

Where it goes wrong

DefGuard/proto, v2/gateway.proto:

enum UpdateType {
  UPDATE_TYPE_UNSPECIFIED = 0;
  UPDATE_TYPE_CREATE = 1;
  UPDATE_TYPE_MODIFY = 2;
  UPDATE_TYPE_DELETE = 3;
}

DefGuard/gateway v2.0.3, src/gateway.rs:

Some(update::Update::Peer(peer_config)) => {
    debug!("Applying peer configuration: {peer_config:?}");
    // UpdateType::Delete
    if update.update_type == 2 {
        debug!("Deleting peer {peer_config:?}");
        self.peers.remove(&peer_config.pubkey);
        ... remove_peer ...
    }
    // UpdateType::Create, UpdateType::Modify
    else {
        debug!("Updating peer {peer_config:?}, update type: {}", update.update_type);
        self.peers.insert(...);
        ... configure_peer ...
    }
}

What that adds up to:

core sends wire value branch taken gateway does right?
Create 1 else configure_peer yes, by luck
Modify 2 == 2 remove_peer no
Delete 3 else configure_peer no

The two branches log different lines and cannot both run, so the logs below say which one was taken.

How to reproduce

One location, one gateway, two keypairs <PUBKEY_A> and <PUBKEY_B>.

1. Create a device — this one works

POST /api/v1/device/network with wireguard_pubkey: <PUBKEY_A>, assigned_ips: ["10.0.0.7"]

Received update: Update { update_type: Create, update: Some(Peer(Peer { pubkey: "<PUBKEY_A>", allowed_ips: ["10.0.0.7"], preshared_key: None, keepalive_interval: Some(25) })) }
Applying peer configuration: Peer { pubkey: "<PUBKEY_A>", allowed_ips: ["10.0.0.7"], preshared_key: None, keepalive_interval: Some(25) }

wg show <iface> allowed-ips:

<PUBKEY_A>   10.0.0.7/32

Correct, though only because Create happens to fall into the branch that writes.

2. Rotate its key — the peer is removed instead of updated

PUT /api/v1/device/{id} with wireguard_pubkey: <PUBKEY_B>. The database now holds <PUBKEY_B>, address unchanged.

Received update: Update { update_type: Modify, update: Some(Peer(Peer { pubkey: "<PUBKEY_B>", allowed_ips: ["10.0.0.7"], preshared_key: None, keepalive_interval: Some(25) })) }
Applying peer configuration: Peer { pubkey: "<PUBKEY_B>", allowed_ips: ["10.0.0.7"], preshared_key: None, keepalive_interval: Some(25) }
Deleting peer Peer { pubkey: "<PUBKEY_B>", allowed_ips: ["10.0.0.7"], preshared_key: None, keepalive_interval: Some(25) }

wg show <iface> allowed-ips:

<PUBKEY_A>   10.0.0.7/32

Deleting peer is there and Updating peer is not, so the == 2 branch ran. The gateway tried to remove a peer identified by the new key, which had never been added, so nothing happened — and the old peer was left alone, because the event carries no previous key to remove.

This is the case that is hard to spot in production: the client keeps handshaking, so it looks connected, but nothing gets through.

3. Change only the address — the working peer disappears

PUT /api/v1/device/network/{id} changing only assigned_ips, key untouched. Same branch, but this time the key does match a peer that exists, so the removal succeeds and the peer is gone.

4. Delete the device — the peer stays, with no addresses

DELETE /api/v1/device/{id}. The device is gone from the database.

Received update: Update { update_type: Delete, update: Some(Peer(Peer { pubkey: "<PUBKEY_B>", allowed_ips: [], preshared_key: None, keepalive_interval: None })) }
Applying peer configuration: Peer { pubkey: "<PUBKEY_B>", allowed_ips: [], preshared_key: None, keepalive_interval: None }
Updating peer Peer { pubkey: "<PUBKEY_B>", allowed_ips: [], preshared_key: None, keepalive_interval: None }, update type: 3

The gateway prints update type: 3 itself, and Updating peer only exists in the else branch, so 3 clearly takes the write path. The peer is left on the interface with an empty address list. Checking by address (wg show <iface> allowed-ips | grep <address>) finds nothing, which makes the delete look like it worked.

Getting back to a good state

systemctl restart defguard-gateway. On reconnect the core sends a full network configuration, which goes through Update::Network and configure_interface and is built from the database, so it never touches the branch above.

Why it matters

If key rotation is part of normal provisioning, this shows up as individual clients losing connectivity for no visible reason, at random times, with a healthy-looking control plane. Changing an address destroys a working peer outright. Deleting a device leaves an addressless peer behind, and those accumulate until someone restarts the gateway.

Suggested direction

Match on the generated enum instead of a literal, and log anything unexpected rather than letting it fall into a branch:

match UpdateType::try_from(update.update_type) {
    Ok(UpdateType::Delete) => { ... remove_peer ... }
    Ok(UpdateType::Create | UpdateType::Modify) => { ... configure_peer ... }
    other => warn!("unhandled update_type {}: {other:?}", update.update_type),
}

Worth deciding at the same time: a key rotation can't really be expressed as a peer modification. In WireGuard the public key is the peer's identity, and DeviceModified only carries the new one — so even with the comparison fixed, the gateway would add the new peer and leave the old one behind. That needs either the previous key in the event, or the core emitting a delete followed by a create.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions