At the moment, `connlib`'s UDP thread spawns a single task for reading
and writing to the UDP socket. It will always first try to write data
before reading new data. To avoid scheduling issues, we split this into
two dedicated tasks and insert
```rust
tokio::task::yield_now().await;
```
into each loop. This allows the `tokio` runtime to schedule each of the
tasks fairly even if one of them is very busy.
For example, if we are very busy writing data (because we are receiving
a lot of IP traffic), this ensures that we will occasionally also read
from our socket to receive STUN control messages from our peers.
Previously, boringtun's sender/receiver index of a session would just be
rendered as a full u32. In reality, this u32 contains two pieces of
information: The higher 24 bits identify the peer and the lower 8 bits
identify the session with that peer. With the update to boringtun in
https://github.com/firezone/boringtun/pull/112, we encode this logic in
a dedicated type that has prints this information separately. Here is
what the logs now look like:
```
2025-08-05T07:38:37.742Z DEBUG boringtun::noise: Received handshake_response local_idx=(3428714|1) remote_idx=(1937676|1)
2025-08-05T07:38:37.743Z DEBUG boringtun::noise: New session idx=(3428714|1)
2025-08-05T07:38:37.743Z DEBUG boringtun::noise: Sending keepalive local_idx=(3428714|1)
```
We can run into this when multiple DNS queries all need to be sent to
the same Gateway and we don't have a connection yet. Hence, downgrade
this error to a debug log.
Right now, `snownet` de-multiplexes WireGuard packets based on their
source tuple (IP + port) to the _first_ connection that would like to
handle this traffic. What appears to be happening based on observation
from customer logs is that we sometimes dispatch the traffic to the
wrong connection.
The WireGuard packet format uses session indices to declare, which
session a packet is for. The local session index is selected during the
handshake for a particular session.
By associating the different session indices (we can have up to 8 in
parallel per peer) with our Firezone-specific connection ID, we can
change our de-multiplexing scheme to uses these indices instead of the
source tuple. This is especially important for Gateways as those talk to
multiple different clients.
The session index is a 32-bit integer where the top 24 bits identify the
connection and the bottom 8 bits are used in a round-robin fashion to
identify individual sessions within the connection. Thus, to find the
correct connection, we right-shift the session index of an incoming
packet to arrive back at the 24-bit connection identifier.
In environments with a limited number of ports outside the NAT, a
connection from a new Client may come from a source tuple of a previous
Client. In such a case, we'd dispatch the packets to the wrong
connection, causing the Client to not be able to handshake a tunnel.
When a Client upserts a connection to a Gateway, we currently assume
that the connection is still intact. After all, it hasn't hit an ICE
timeout, otherwise the connection would not be present in memory. If
however the Gateway restarted or somehow lost its connection state and
the Client hasn't noticed yet, then the upsert will be an _insert_ for
the Gateway and ICE will create a new connection for us.
In order to ensure that the WireGuard tunnel state and ICE are
synchronized at all times, we also need to handshake a new session.
`boringtun` maintains up to 8 concurrent sessions for us. This allows
for a smooth roll-over where packets encrypted with the keys from
previous sessions can still be decrypted. Thus, we can easily roll-over
the session on every connection upsert without any trouble.
To ensure that this doesn't happen _very_ rapidly, we debounce these
proactive session roll-overs to happen at most every 20s.
This follows the idea of MADR-0017.
---------
Signed-off-by: Thomas Eizinger <thomas@eizinger.io>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Jamil <jamilbk@users.noreply.github.com>
When the connection to a Client disappears, the Gateway currently clears
all state related to this peer. Whilst eagerly cleaning up memory can be
good, in this case, it may lead to the Client thinking it has access to
a resource when in reality it doesn't.
Just because the connection to a Client failed doesn't mean their access
authorizations are invalid. In case the Client reconnects, it should be
able to just continue sending traffic.
At the moment, this only works if the connection also failed on the
Client and therefore, its view of the world in regards to "which
resources do I have access to" was also reset.
What we are seeing in Sentry reports though is that Clients are
attempting to access these resources, thinking they have access but the
Gateway denies it because it has lost the access authorization state.
To make things easier to debug, we enforce the order that candidates are
processed in. We want candidates to be processed in the order of their
inverse priority as higher priorities are better. For example, a host
candidate has a higher priority than a relay candidate.
This will make our logs more consistent because a `0-0` candidate pair
is always a `host-host` pair.
We enforce this with our own `IceCandidate` type which implements
`PartialOrd` and `Ord`. This now moves the deserialisation for the
portal messages to a `Deserialise` impl on this type. In order to ensure
that a single faulty candidate doesn't invalidate the entire list, we
use `serde_with` to skip over those elements that cannot be
deserialised.
In #7548, we added a feature to Firezone where TURN channels get bound
on-demand as they are needed. To ensure many communication paths work,
we also proactively bind them as soon as we receive a candidate from a
remote.
When a new remote candidate gets added, str0m forms pairs with all the
existing local candidates and starts testing these candidate pairs. For
local relay candidates, this means sending a channel data message from
the allocation.
At the moment, this results in the following pattern in the logs:
```
Received candidate from remote cid=20af9d29-c973-4d77-909a-abed5d7a0234 candidate=Candidate(relay=[3231E680683CFC98E69A12A60F426AA5E5F110CB]:62759/udp raddr=[59A533B0D4D3CB3717FD3D655E1D419E1C9C0772]:0 prio=37492735)
No channel to peer, binding new one active_socket=462A7A508E3C99875E69C2519CA020330A6004EC:3478 peer=[3231E680683CFC98E69A12A60F426AA5E5F110CB]:62759
Already binding a channel to peer active_socket=Some(462A7A508E3C99875E69C2519CA020330A6004EC:3478) peer=[3231E680683CFC98E69A12A60F426AA5E5F110CB]:62759
class=success response from=462A7A508E3C99875E69C2519CA020330A6004EC:3478 method=channel bind rtt=9.928424ms tid=042F52145848D6C1574BB997
```
What happens here is:
1. We receive a new candidate and proactively bind a channel (this is a
silent operation and therefore not visible in the logs).
2. str0m formed new pairs for these candidates and starts testing them,
triggering a new channel binding because the previous one isn't
completed yet.
3. We refuse to make another channel binding because we see that we
already have one in-flight.
4. The channel binding succeeds.
What we do now is:
If we want to send data to a peer through a channel, we check whether we
have a connected OR an in-flight channel and send it in both cases. If
the channel binding is still in-flight, we therefore just pipeline the
channel data message just after it. Chances are that - assuming no
packet re-orderings on the network - by the time our channel data
message arrives at the relay that binding is active and can be relayed.
This allows the very first binding attempt from str0m to already succeed
instead of waiting for the timeout and sending another binding request.
In addition, it makes these logs less confusing.
Our TURN traffic is fairly minimal for this to be okay on DEBUG (instead
of TRACE). However, it can be quite noisy when one is just scanning
through the logs. Putting it on another target allows us to filter those
out later.
Note that these only concern the TURN control protocol. Channel data
messages are separate from this and **not** logged.
Spans only attach to logs of lower severity, i.e. a DEBUG span is only
visible for DEBUG and TRACE statements. In order to see the connection
ID here with our INFO statements, we need to make it an INFO span.
Also, a span does nothing unless it is entered 🤦♂️
With the removal of the NAT64/46 modules, we can now simplify the
internals of our `IpPacket` struct. The requirements for our `IpPacket`
struct are somewhat delicate.
On the one hand, we don't want to be overly restrictive in our parsing /
validation code because there is a lot of broken software out there that
doesn't necessarily follow RFCs. Hence, we want to be as lenient as
possible in what we accept.
On the other hand, we do need to verify certain aspects of the packet,
like the payload lengths. At the moment, we are somewhat too lenient
there which causes errors on the Gateway where we have to NAT or
otherwise manipulate the packets. See #9567 or #9552 for example.
To fix this, we make the parsing in the `IpPacket` constructor more
restrictive. If it is a UDP, TCP or ICMP packet, we attempt to fully
parse its headers and validate the payload lengths.
This parsing allows us to then rely on the integrity of the packet as
part of the implementation. This does create several code paths that can
in theory panic but in practice, should be impossible to hit. To ensure
that this does in fact not happen, we also tackle an issue that is long
overdue: Fuzzing.
Resolves: #6667Resolves: #9567Resolves: #9552
Rust 1.88 shipped a new std-function on `HashMap` to conditionally
extract elements from a `HashMap`. This is handy for time-based expiry
of resources on the Gateway.
When filtering through logs in Sentry, it is useful to narrow them down
by context of a client, gateway or resource. Currently, these fields are
sometimes called `client`, `cid`, `client_id` etc and the same for the
Gateway and Resources.
To make this filtering easier, name all of them `cid` for Client IDs,
`gid` for Gateway IDs and `rid` for Resource IDs.
We use several buffer pools across `connlib` that are all backed by the
same buffer-pool library. Within that library, we currently use another
object-pool library to provide the actual pooling functionality.
Benchmarking has shown that spend quite a bit of time (a few % of total
CPU time), fighting for the lock to either add or remote a buffer from
the pool. This is unnecessary. By using a queue, we can remove buffers
from the front and add buffers at the back, both of which can be
implemented in a lock-free way such that they don't contend.
Using the well-known `crossbeam-queue` library, we have such a queue
directly available.
I wasn't able to directly measure a performance gain in terms of
throughput. What we can measure though, is how much time we spend
dealing with our buffer pool vs everything else. If we compare the
`perf` outputs that were recorded during an `iperf` run each, we can see
that we spend about 60% less time dealing with the buffer pool than we
did before.
|Before|After|
|---|---|
|<img width="1982" height="553" alt="Screenshot From 2025-07-24
20-27-50"
src="https://github.com/user-attachments/assets/1698f28b-5821-456f-95fa-d6f85d901920"
/>|<img width="1982" height="553" alt="Screenshot From 2025-07-24
20-27-53"
src="https://github.com/user-attachments/assets/4f26a2d1-03e3-4c0d-84da-82c53b9761dd"
/>|
The number in the thousands on the left is how often the respective
function was the currently executing function during the profiling run.
Resolves: #9972
Presently, for each UDP packet that we process in `snownet`, we check if
we have already seen this local address of ours and if not, add it to
our list of host candidates. This is a safe way for ensuring that we
consider all addresses that we receive data on as ones that we tell our
peers that they should try and contact us on.
Performance profiling has shown that hashing the socket address of each
packet that is coming in is quite wasteful. We spend about 4-5% of our
main thread time doing this. For comparison, decrypting packets is only
about 30%.
Most of the time, we will already know about this address and therefore,
spending all this CPU time is completely pointless. At the same time
though, we need to be sure that we do discover our local address
correctly.
Inspired by STUN, we therefore move this responsibility to the
`allocation` module. The `allocation` module is responsible for
interacting with our TURN servers and will yield server-reflexive and
relay candidates as a result. It also knows, what the local address is
that it received traffic on so we simply extend that to yield host
candidates as well in addition to server-reflexive and relay candidates.
On my local machine, this bumps us across the 3.5 Gbits/sec mark:
```
Connecting to host 172.20.0.110, port 5201
[ 5] local 100.93.174.92 port 57890 connected to 172.20.0.110 port 5201
[ ID] Interval Transfer Bitrate Retr Cwnd
[ 5] 0.00-1.00 sec 319 MBytes 2.67 Gbits/sec 18 548 KBytes
[ 5] 1.00-2.00 sec 413 MBytes 3.46 Gbits/sec 4 884 KBytes
[ 5] 2.00-3.00 sec 417 MBytes 3.50 Gbits/sec 4 1.10 MBytes
[ 5] 3.00-4.00 sec 425 MBytes 3.56 Gbits/sec 415 785 KBytes
[ 5] 4.00-5.00 sec 430 MBytes 3.60 Gbits/sec 154 820 KBytes
[ 5] 5.00-6.00 sec 434 MBytes 3.64 Gbits/sec 251 793 KBytes
[ 5] 6.00-7.00 sec 436 MBytes 3.66 Gbits/sec 123 811 KBytes
[ 5] 7.00-8.00 sec 435 MBytes 3.65 Gbits/sec 2 788 KBytes
[ 5] 8.00-9.00 sec 423 MBytes 3.55 Gbits/sec 0 1.06 MBytes
[ 5] 9.00-10.00 sec 433 MBytes 3.63 Gbits/sec 8 1017 KBytes
- - - - - - - - - - - - - - - - - - - - - - - - -
[ ID] Interval Transfer Bitrate Retr
[ 5] 0.00-20.00 sec 8.21 GBytes 3.53 Gbits/sec 1728 sender
[ 5] 0.00-20.00 sec 8.21 GBytes 3.53 Gbits/sec receiver
iperf Done.
```
These are otherwise hit pretty often in the hot-path and slow packet
routing down because tracing needs to evaluate whether it should log the
statement.
Whilst entering and leaving a span for every packet is very expensive,
doing the same whenever we make timeout related changes is just fine.
Thus, we re-introduce a span removed in #9949 but only for the
`handle_timeout` function.
This gives us the context of the connection ID for not just our own
logs, but also the ones from `boringtun`.
By chance, I've discovered in a CI failure that we won't be able to
handshake a new session if the `preshared_key` changes. This makes a lot
of sense. The `preshared_key` needs to be the same on both ends as it is
a shared secret that gets mixed into the Noise handshake.
In following sequence of events, we would thus previously run into a
"failed to decrypt handshake packet" scenario:
1. Client requests a connection.
2. Gateway authorizes the connection.
3. Portal restarts / gets deployed. To my knowledge, this will rotate
the `preshared_key` to a new secret. Restarting the portal also cuts all
WebSockets and therefore, the Gateways response never arrives.
4. Client reconnects to the WebSocket, requests a new connection.
5. Gateway reuses the local connection but this connection still uses
the old `preshared_key`!
6. Client needs to wait for the Gateway's ICE timeout before it can
establish a new connection.
How exactly (3) happens doesn't matter. There are probably other
conditions as to where the WebSocket connections get cut and we cannot
complete our connection handshake.
Previously, our idle timer was only driven by incoming and outgoing
packets. To detect whether the tunnel is idle, we checked whether either
the last incoming or last outgoing packet was more than 20s ago.
For one, having two timestamps here is unnecessarily complex. We can
simply combine them and always update this timestamp as `last_activity`.
Two, recently, we have started to also take into account not only
packets but other changes to the tunnel, such as an upsert of the
connection or adding new candidate. What we failed to do though, is
update these timestamps because their variable name was related to
packets and not to any activity.
The problem with not updating these timestamps however is that we will
very quickly move out of "connected" back to "idle" because the old
timestamps are still more than 20s ago. Hence, the previous fixes of
moving out of idle on new candidates and connection upsert were
ineffective.
By combining and renaming the timestamps, it is now much more obvious
that we need to update this timestamp in the respective handler
functions which then grants us another 20s of non-idling. This is
important for e.g. connection upserts to ensure the Gateway runs into an
ICE timeout within a short amount of time, should there be something
wrong with the connection that the Client just upserted.
As profiling shows, even if the log target isn't enabled, simply
checking whether or not it is enabled is a significant performance hit.
By guarding these behind `debug_assertions`, I was able to almost
achieve 3.75 Gbits/s locally (when rebased onto #9998). Obviously, this
doesn't quite translate into real-world improvements but it is
nonetheless a welcome improvement.
```
Connecting to host 172.20.0.110, port 5201
[ 5] local 100.93.174.92 port 34678 connected to 172.20.0.110 port 5201
[ ID] Interval Transfer Bitrate Retr Cwnd
[ 5] 0.00-1.00 sec 401 MBytes 3.37 Gbits/sec 14 644 KBytes
[ 5] 1.00-2.00 sec 448 MBytes 3.76 Gbits/sec 3 976 KBytes
[ 5] 2.00-3.00 sec 453 MBytes 3.80 Gbits/sec 43 979 KBytes
[ 5] 3.00-4.00 sec 449 MBytes 3.77 Gbits/sec 21 911 KBytes
[ 5] 4.00-5.00 sec 452 MBytes 3.79 Gbits/sec 4 1.15 MBytes
[ 5] 5.00-6.00 sec 451 MBytes 3.78 Gbits/sec 81 1.01 MBytes
[ 5] 6.00-7.00 sec 445 MBytes 3.73 Gbits/sec 39 705 KBytes
[ 5] 7.00-8.00 sec 436 MBytes 3.66 Gbits/sec 3 1016 KBytes
[ 5] 8.00-9.00 sec 460 MBytes 3.85 Gbits/sec 1 956 KBytes
[ 5] 9.00-10.00 sec 453 MBytes 3.80 Gbits/sec 0 1.19 MBytes
- - - - - - - - - - - - - - - - - - - - - - - - -
[ ID] Interval Transfer Bitrate Retr
[ 5] 0.00-10.00 sec 4.34 GBytes 3.73 Gbits/sec 209 sender
[ 5] 0.00-10.00 sec 4.34 GBytes 3.73 Gbits/sec receiver
```
I didn't want to remove the `wire` logs entirely because they are quite
useful for debugging. However, they are also exactly this: A debugging
tool. In a production build, we are very unlikely to turn these on which
makes `debug_assertions` a good tool for keeping these around without
interfering with performance.
Our `ThreadedUdpSocket` uses a background thread for the actual socket
operation. It merely represents a handle to send and receive from these
sockets but not the socket itself. Dropping the handle will shutdown the
background thread but that is an asynchronous operation.
In order to be sure that we can rebind the same port, we need to wait
for the background thread to stop.
We thus add a `Drop` implementation for the `ThreadedUdpSocket` that
waits for its background thread to disappear before it continues.
Resolves: #9992
Currently, packets for allocations, i.e. from relays are parsed inside
the `Allocation` struct. We have one of those structs for each relay
that `snownet` is talking to. When we disconnect from a relay because it
is e.g. not responding, then we deallocate this struct. As a result,
message that arrive from this relay can no longer be handled. This can
happen when the response time is longer than our timeout.
These packets then fall-through and end up being logged as "packet has
unknown format".
To prevent this, we make the signature on `Allocation` strongly-typed
and expect a fully parsed `Message` to be given to us. This allows us to
parse the message early and discard it with a DEBUG log in case we don't
have the necessary local state to handle it.
The functionality here is essentially the same, we just change at what
level this is being logged at from WARN to DEBUG.
We have to make one additional adjustment to make this work: Guard all
messages to be parsed by any `Allocation` to come from port 3478. This
is the assigned port that all relays are expected to listen on. If we
don't have any local state for a given address, we cannot decide whether
it is a STUN message for an agent or a STUN message for a relay that we
have disconnected from. Therefore, we need to de-multiplex based on the
source port.
Due to network partitions between the Client and the Portal, it is
possible that a Client requests a new connection, then disconnects from
the portal and re-requests the connection once it is reconnected.
On the Gateway, we would have already authorized the first request and
initialise our ICE agents with our local candidates. The second time
around, the connection would be reused. The Client however has lost its
state and therefore, we need to tell it our candidates again.
---------
Signed-off-by: Thomas Eizinger <thomas@eizinger.io>
Room join requests on the portal are only valid whilst we have a
WebSocket connection. To make sure the portal processes all our requests
correctly, we need to hold all other messages back while we are waiting
to join the room.
If the connection flaps while we are waiting to join a room, we may have
a lingering join request that never gets fulfilled and thus blocks the
sending of messages forever.
---------
Co-authored-by: Jamil Bou Kheir <jamilbk@users.noreply.github.com>
Now that we are capable of migrating a connection to another relay with
#9979, our test suite exposed an edge-case: If we are in the middle of
migrating a connection, it could be that the idle timer triggers because
we have not seen any application traffic in the last 20s.
Moving to idle mode drastically reduces the number of STUN bindings we
send and if this happens whilst we are still checking candidates, the
nomination doesn't happen in time for our boringtun handshake to
succeed.
Thus, we add a condition to our idle timer to not trigger unless ICE has
completed and reports us as `connected`.
When looking through customer logs, we see a lot of "Resolved best route
outside of tunnel" messages. Those get logged every time we need to
rerun our re-implementation of Windows' weighting algorithm as to which
source interface / IP a packet should be sent from.
Currently, this gets cached in every socket instance so for the
peer-to-peer socket, this is only computed once per destination IP.
However, for DNS queries, we make a new socket for every query. Using a
new source port DNS queries is recommended to avoid fingerprinting of
DNS queries. Using a new socket also means that we need to re-run this
algorithm every time we make a DNS query which is why we see this log so
often.
To fix this, we need to share this cache across all UDP sockets. Cache
invalidation is one of the hardest problems in computer science and this
instance is no different. This cache needs to be reset every time we
roam as that changes the weighting of which source interface to use.
To achieve this, we extend the `SocketFactory` trait with a `reset`
method. This method is called whenever we roam and can then reset a
shared cache inside the `UdpSocketFactory`. The "source IP resolver"
function that is passed to the UDP socket now simply accesses this
shared cache and inserts a new entry when it needs to resolve the IP.
As an added benefit, this may speed up DNS queries on Windows a bit
(although I haven't benchmarked it). It should certainly drastically
reduce the amount of syscalls we make on Windows.
In #6876, we added functionality that would only make use of new remote
candidates whilst we haven't nominated a socket yet with the remote. The
reason for that was because in the described edge-case where relays
reboot or get replaced whilst the client is partitioned from the portal
(or we experience a connection hiccup), only one of the two peers, i.e.
Client or Gateway would migrate to the new relay, leaving the other one
in an inconsistent state.
Looking at recent customer logs, I've been seeing a lot of these
messages:
> Unknown connection or socket has already been nominated
For this particular customer, these are then very quickly followed by
ICE timeouts, leaving the connection unusable.
Considering that, I no longer think that the above change was a good
idea and we should instead always make use of all candidates that we are
given. What we are seeing is that in deployment scenarios where the
latency link between Client and Gateway is very short (5-10ms) yet the
latency to the portal is longer (~30-50ms), we trigger a race condition
where we are temporarily nominating a _peer-reflexive_ candidate pair
instead of a regular one. This happens because with such a short latency
link, Client and Gateway are _faster_ in sending back and forth several
STUN bindings than the control plane is in delivering all the
candidates.
Due to the functionality added in #6876, this then results in us not
accepting the candidates. It further appears that a nominated
peer-reflexive candidate does not provide a stable connection which is
why we then run into an ICE timeout, requiring Firezone to establish a
new connection only to have the same thing happen again.
This is very disruptive for the user experience as the connection only
works for a few moments at a time.
With #9793, we have actually added a feature that is also at play here.
Now that we don't immediately act on an ICE timeout, it is actually
possible for both Client and Gateway to migrate a connection to a
different relay, should the one that they are using get disconnected. In
#9793, we added a timeout of 2s for this.
To make this fully work, we need to patch str0m to transition to
`Checking` early. Presently, str0m would directly transition from
`Disconnected` to `Connected` in this case which in some of the
high-latency scenarios that we are testing in CI is not enough to
recover the connection within 2s. By transitioning to `Checking` early,
we abort this timer.
Related: https://github.com/algesten/str0m/pull/676
In case we received a newly nominated socket from `str0m` whilst our
connection was in idle mode, we mistakenly did not apply that and kept
using the old one. ICE would still be functioning in this case because
`str0m` would have updated its internal state but we would be sending
packets into Nirvana.
I don't think that this is likely to be hit in production though as it
would be quite unusual to receive a new nomination whilst the connection
was completely idle.
When encrypting IP packets, `snownet` needs to prepare a buffer where
the encrypted packet is going to end up. Depending on whether we are
sending data via a relayed connection or direct, this buffer needs to be
offset by 4 bytes to allow for the 4-byte channel-data header of the
TURN protocol.
At present, we always first encrypt the packet and then on-demand move
the packet by 4-bytes to the left if we **don't** need to send it via a
relay. Internally, this translates to a `memmove` instruction which
actually turns out to be very cheap (I couldn't measure a speed
difference between this and `main`).
All of this code has grown historically though so I figured, it is
better to clean it up a bit to first evaluate, whether we have a direct
or relayed connection and based on that, write the encrypted packet
directly to the front of the buffer or offset it by 4 bytes.
Profiling has shown that using a spinlock-based buffer pool is
marginally (~1%) faster than the mutex-based one because it resolves
contention quicker.
Profiling has shown that checking whether the level is enabled is
actually more expensive than checking whether the packet is a DNS
packet. This improves performance by about 3%.
When being presented an invalid peer certificate, there is no reason why
we should retry the connection, it is unlikely to fix itself. Plus, the
certificate may get / be cached and a restart of the application is
necessary.
Resolves: #9944
This was exposed by #9846. It is being added here as a dedicated PR
because the compatibility tests would fail or at least be flaky for the
latest client release so we cannot add the integration test right away.
When receiving an `init` message from the portal, we will now revoke all
authorizations not listed in the `authorizations` list of the `init`
message.
We (partly) test this by introducing a new transition in our proptests
that de-authorizes a certain resource whilst the Gateway is simulated to
be partitioned. It is difficult to test that we cannot make a connection
once that has happened because we would have to simulate a malicious
client that knows about resources / connections or ignores the "remove
resource" message.
Testing this is deferred to a dedicated task. We do test that we hit the
code path of revoking the resource authorization and because the other
resources keep working, we also test that we are at least not revoking
the wrong ones.
Resolves: #9892
From Sentry reports and user-submitted logs, we know that it is possible
for Client and Gateway to de-sync in regards to what each other's public
key is. In such a scenario, ICE will succeed to make a connection but
`boringtun` will fail to handshake a tunnel. By default, `boringtun`
tries for 90s to handshake a session before it gives up and expires it.
In Firezone, the ICE agent takes care of establishing connectivity
whereas `boringtun` itself just encrypts and decrypts packets. As such,
if ICE is working, we know that packets aren't getting lost but instead,
there must be some other issue as to why we cannot establish a session.
To improve the UX in these error cases, we reduce the rekey-attempt-time
to 15s. This roughly matches our ICE timeout. Those 15s count from the
moment we send the first handshake which is just after ICE completes.
Thus we can be sure that after at most 15s, we either have a working
WireGuard session or the connection gets cleaned up.
Related: #9890
Related: #9850
When we invalidate or discard an allocation, it may happen that a relay
still sends channel-data messages to us. We don't recognize those and
will therefore attempt to parse them as WireGuard packets, ultimately
ending in an "Packet has unknown format" error.
To avoid this, we check if the packet is a valid channel-data message
even if we presently don't have an allocation on the relay that is
sending us the packet. In those cases, we can stop processing the
packet, thus avoiding these errors from being logged.
When a connection is in idle-mode, it only sends a STUN request every 25
seconds. If the Client disconnects e.g. due to a network partition, it
may send a new connection intent later. If the Gateway's connection is
still around then because it was in idle mode, it won't send any
candidates to the remote, making the Client's connection fail with "no
candidates received".
To alleviate this, we wake a connection out of idle mode every time it
is being upserted. This ensures that the connection will fail within 15s
IF the above scenario happens, allowing the Client to reconnect within a
much shorter time-frame.
Note that attempting to repair such a connection is likely pointless. It
is much safer to discard it and let them both establish a new
connection.
Related: #9862
Whilst looking through the auth module of the relay, I noticed that we
unnecessarily convert back and forth between expiry timestamps and
username formats when we could just be using the already parsed version.
