With this patch, we sample a list of DNS resources on each test run and
create a "TCP service" for each of their addresses. Using this list of
resources, we then change the `SendTcpPayload` transition to
`ConnectTcp` and establish TCP connections using `smoltcp` to these
services.
For now, we don't send any data on these connections but we do set the
keep-alive interval to 5s, meaning `smoltcp` itself will keep these
connections alive. We also set the timeout to 30s and after each
transition in a test-run, we assert that all TCP sockets are still in
their expected state:
- `ESTABLISHED` for most of them.
- `CLOSED` for all sockets where we ended up sampling an IPv4 address
but the DNS resource only supports IPv6 addresses (or vice-versa). In
these cases, we use the ICMP error to sent by the Gateway to assert that
the socket is `CLOSED`. Unfortunately, `smoltcp` currently does not
handle ICMP messages for its sockets, so we have to call `abort`
ourselves.
Overall, this should assert that regardless of whether we roam networks,
switch relays or do other kind of stuff with the underlying connection,
the tunneled TCP connection stays alive.
In order to make this work, I had to tweak the timeouts when we are
on-demand refreshing allocations. This only happens in one particular
case: When we are being given new relays by the portal, we refresh all
_other_ relays to make sure they are still present. In other words, all
relays that we didn't remove and didn't just add but still had in-memory
are refreshed. This is important for cases where we are
network-partitioned from the portal whilst relays are deployed or reset
their state otherwise. Instead of the previous 8s max elapsed time of
the exponential backoff like we have it for other requests, we now only
use a single message with a 1s timeout there. With the increased ICE
timeout of 15s, a TCP connection with a 30s timeout would otherwise not
survive such an event. This is because it takes the above mentioned 8s
for us to remove a non-functioning relay, all whilst trying to establish
a new connection (which also incurs its own ICE timeout then).
With the reduced timeout on the on-demand refresh of 1s, we detect the
disappeared relay much quicker and can immediately establish a new
connection via one of the new ones. As always with reduced timeouts,
this can create false-positives if the relay doesn't reply within 1s for
some reason.
Resolves: #9531
The Postgres logical decoding protocol is lacking documentation and
unclear about keepalive behavior when `wal_sender_timeout` is set to 0
(disabled). We have it disabled so that Postgres doesn't terminate our
connection for falling too far behind.
What we failed to take into account is that on some installations,
Postgres _never_ requests an immediate reply (keepalive with the reply
now bit set) if wal_sender_timeout is disabled. This means we would
always reply with the empty message, failing to advance the position of
the LSN.
In this PR, we fix that to always respond to every keepalive message
with a standby status update to advance the LSN position.
Relevant documentation:
https://www.postgresql.org/docs/current/protocol-replication.html#PROTOCOL-REPLICATION-STANDBY-STATUS-UPDATE
When defining a resource, a Firezone admin can define traffic filters to
only allow traffic on certain TCP and/or UDP ports and/or restrict
traffic on the ICMP protocol.
Presently, when a packet is filtered out on the Gateway, we simply drop
it. Dropping packets means the sending application can only react to
timeouts and has no other means on error handling. ICMP was conceived to
deal with these kind of situations. In particular, the "destination
unreachable" type has a dedicated code for filtered packets:
"Communication administratively prohibited".
Instead of just dropping the not-allowed packet, we now send back an
ICMP error with this particular code set, thus informing the sending
application that the packet did not get lost but was in fact not routed
for policy reasons.
When setting a traffic filter that does not allow TCP traffic,
attempting to `curl` such a resource now results in the following:
```
❯ sudo docker compose exec --env RUST_LOG=info -it client /bin/sh -c 'curl -v example.com'
* Host example.com:80 was resolved.
* IPv6: fd00:2021:1111:8000::, fd00:2021:1111:8000::1, fd00:2021:1111:8000::2, fd00:2021:1111:8000::3
* IPv4: 100.96.0.1, 100.96.0.2, 100.96.0.3, 100.96.0.4
* Trying [fd00:2021:1111:8000::]:80...
* connect to fd00:2021:1111:8000:: port 80 from fd00:2021:1111::1e:7658 port 34560 failed: Permission denied
* Trying [fd00:2021:1111:8000::1]:80...
* connect to fd00:2021:1111:8000::1 port 80 from fd00:2021:1111::1e:7658 port 34828 failed: Permission denied
* Trying [fd00:2021:1111:8000::2]:80...
* connect to fd00:2021:1111:8000::2 port 80 from fd00:2021:1111::1e:7658 port 44314 failed: Permission denied
* Trying [fd00:2021:1111:8000::3]:80...
* connect to fd00:2021:1111:8000::3 port 80 from fd00:2021:1111::1e:7658 port 37628 failed: Permission denied
* Trying 100.96.0.1:80...
* connect to 100.96.0.1 port 80 from 100.66.110.26 port 53780 failed: Host is unreachable
* Trying 100.96.0.2:80...
* connect to 100.96.0.2 port 80 from 100.66.110.26 port 60748 failed: Host is unreachable
* Trying 100.96.0.3:80...
* connect to 100.96.0.3 port 80 from 100.66.110.26 port 38378 failed: Host is unreachable
* Trying 100.96.0.4:80...
* connect to 100.96.0.4 port 80 from 100.66.110.26 port 49866 failed: Host is unreachable
* Failed to connect to example.com port 80 after 9 ms: Could not connect to server
* closing connection #0
curl: (7) Failed to connect to example.com port 80 after 9 ms: Could not connect to server
```
In order to better track, how well our `ENOBUFS` mitigation is working,
we should log the use of our feature flag to PostHog. This will give us
some stats how often this is happening. That combined with the lack of
error reports should give us good confidence in permanently enabling
this behaviour.
When a packet gets filtered because we are unable to evaluate the source
protocol (i.e. TCP/UDP/ICMP), then the current error message currently
misleadingly says that the packet got filtered because the protocol is
not supported.
The truth however is that we were never able to apply the filter in the
first place. This is a subtle difference that is quite important when
debugging filtered packets. To improve this, we add an error message to
the stack here.
Firezone uses ICMP errors to signal to client applications that e.g. a
certain IP is not reachable. This happens for example if a DNS resource
only resolves to IPv4 addresses yet the client application attempted to
use an IPv6 proxy address to connect to it.
In the presence of traffic filters for such a resource that does _not_
allow ICMP, we currently filter out these ICMP errors because - well -
ICMP traffic is not allowed! However, even in the presence of ICMP
traffic being allowed, we would fail to evaluate this filter because the
ICMP error packet is not an ICMP echo reply and therefore doesn't have
an ICMP identifier. We require this in the DNS resource NAT to identify
"connections" and NAT them correctly. The same L4 component is used to
evaluate the traffic filters.
ICMP errors are critical to many usage scenarios and algorithms like
happy-eyeballs. Dropping them usually results in weird behaviour as
client applications can then only react to timeouts.
We aren't sending the OTEL metrics anywhere yet but it still makes sense
to also use the "newer" hex-representation of the Firezone ID here as
the service ID.
At present, our primary indicator as to whether telemetry is active is
whether we have a Sentry session. For our analytics events however, we
currently require passing in the Firezone ID and API url again. This
makes it difficult to send analytics events from areas of the code that
don't have this information available.
To still allow for that, we integrate the `analytics` module more
tightly with the Sentry session. This allows us to drop two parameters
from the `$identify` event and also means we now respect the
`NO_TELEMETRY` setting for these events except for `new_session`. This
event is sent regardless because it allows us to track, how many on-prem
installations of Firezone are out there.
In #9733, we changed the replies of the handle_data messages which seems
to have caused Postgres to not respect our acknowledgements sent in the
keepalive.
To fix this, we revert to sending an empty message in response to write
messages.
Socket APIs across operating systems vary in how they handle
back-pressure. In most cases, a non-blocking socket should return
`EWOULDBLOCK` when it cannot send a given datagram and would have to
block to wait for resources to free up.
It appears that macOS doesn't always behave like that. In particular, we
are seeing error logs from a few users where sending a datagram fails
with
> No buffer space available (os error 55)
Digging through `libc`, I've found that this error is known as `ENOBUFS`
[0].
There are reports on the Apple developer forum [1] that recommend
retrying when this error happens. It is however unclear as to whether it
is entirely safe to map this error to `EWOULDBLOCK`. Other non-blocking
event-loop implementations [2] appear to do that but we don't know
whether it is fully correct.
At present, Firezone's behaviour here is to drop the packet. This means
the host networking stack has to fall-back to running into a timeout and
re-send the packet. This very likely negatively impacts the UX for the
users hitting this.
In order to validate this assumption, we implement a feature-flag. This
allows us to ship this code but switch back to the old behaviour, should
it negatively impact how Firezone behaves. In particular, if the
assumption that mapping `ENOBUFS` to `EWOULDBLOCK` is safe turns out
wrong and `kqueue` does in fact not signal readiness when more buffers
are available, then we may have missing wake-ups which would lead a
further delay in datagrams being sent.
[0]:
8e6f36c6ba/src/unix/bsd/apple/mod.rs (L2998)
[1]: https://developer.apple.com/forums/thread/42334
[2]:
aac866f399/src/unix/stream.c (L820)
When receiving UDP packets that we cannot decode we log an error. In
order to identify, whether we might have bugs in our decoding logic, we
now also print the hex-encoding of the packet for further analysis on
DEBUG.
A recent release of `tslink` now supports configuration via the
`package.metadata` table which resolved a warning about "unknown key"
that we have seeing for a while.
At present, and as a result of how `connlib` evolved, we still implement
a `Poll`-based function for receiving data on our UDP socket. Ever since
we moved to dedicated threads for the UDP socket, we can directly block
on "block" on receiving datagrams and don't have to poll the socket.
This simplifies the implementation a fair bit. Additionally, it made me
reailise that we currently don't expose any errors on the UDP socket.
Likely, those will be ephemeral but it is still better than completely
silencing them.
With a real AD (and not Intune), it seems the `valueName` attribute is
required for text elements.
Supersedes: #9419
Co-authored-by: Antoine Labarussias <antoinelabarussias@gmail.com>
This log is misplaced within the current `try_connect` function because
that will also be called when we didn't have Internet while we tried to
first connect and then witnessed a network change (in which case the
token is cached in the `WaitingForNetwork` session state).
Thus, we move the log to the `Connect` msg handler where we shouldn't
have any existing session.
Bumps [tslink](https://github.com/icsmw/tslink) from 0.3.0 to 0.4.2.
<details>
<summary>Changelog</summary>
<p><em>Sourced from <a
href="https://github.com/icsmw/tslink/blob/master/changelog.md">tslink's
changelog</a>.</em></p>
<blockquote>
<h1>0.4.2 (08.06.2025)</h1>
<h2>Changes</h2>
<ul>
<li>Migrate settings to <code>package.metadata.tslink</code></li>
</ul>
<h1>0.4.1 (08.06.2025)</h1>
<h2>Changes</h2>
<ul>
<li>Add support arrays in the context of <code>const</code></li>
</ul>
<h1>0.4.0 (08.06.2025)</h1>
<h2>Features</h2>
<ul>
<li>Add support of <code>const</code> for primitive types</li>
</ul>
</blockquote>
</details>
<details>
<summary>Commits</summary>
<ul>
<li>See full diff in <a
href="https://github.com/icsmw/tslink/commits">compare view</a></li>
</ul>
</details>
<br />
[](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores)
You can trigger a rebase of this PR by commenting `@dependabot rebase`.
[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)
---
<details>
<summary>Dependabot commands and options</summary>
<br />
You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits
that have been made to it
- `@dependabot merge` will merge this PR after your CI passes on it
- `@dependabot squash and merge` will squash and merge this PR after
your CI passes on it
- `@dependabot cancel merge` will cancel a previously requested merge
and block automerging
- `@dependabot reopen` will reopen this PR if it is closed
- `@dependabot close` will close this PR and stop Dependabot recreating
it. You can achieve the same result by closing it manually
- `@dependabot show <dependency name> ignore conditions` will show all
of the ignore conditions of the specified dependency
- `@dependabot ignore this major version` will close this PR and stop
Dependabot creating any more for this major version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this minor version` will close this PR and stop
Dependabot creating any more for this minor version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop
Dependabot creating any more for this dependency (unless you reopen the
PR or upgrade to it yourself)
</details>
> **Note**
> Automatic rebases have been disabled on this pull request as it has
been open for over 30 days.
Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
We've not added changelog entries for several of the PRs that went in
recently. This adds some for the more user-facing changes and fixes that
we are shipping.
Certain UNIX systems such as macOS also use the EHOSTDOWN error to
signal that a packet cannot be sent to a certain IP. There is nothing we
can do about this error so we downgrade it from a WARN to a DEBUG like
we do for other kinds of "unreachable" errors.
When we shipped the feature of optimistc server-reflexive candidates, we
failed to add a check to only combine address and base such that they
are the same IP version. This is not harmful but unnecessary noise.
When we create a new connection, we seed the local ICE agent with all
known local candidates, i.e. host addresses and allocations on relays.
Server-reflexive candidates are never added to the local agent because
you cannot send directly from a server-reflexive addresses. Instead, an
agent sends from the _base_ of a server-reflexive candidate which in
turn is known as a host candidate.
The server-reflexive candidate is however signaled to the remote so it
can try and send packets to it. Those will then be mapped by the NAT to
our host candidate.
In case we have just performed a network reset, our own server-reflexive
candidate may not be known yet and therefore the seeding doesn't add an
candidates. With no candidates being seeded, we also can't signal them
to the remote.
For candidates discovered later in this process, the signalling happens
as part of adding them to the local agent. Because server-reflexive
candidates are not added to the local agent, we currently miss out on
signaling those to the remote IF they weren't already present when the
ICE agent got created.
This scenario can happen right after a network reset. In practice, it
shouldn't be much of an issue though. As soon as we start sending from
our host candidate, the remote will create a peer-reflexive candidate
for it. It is however cleaner to directly send the server-reflexive
candidate once we discover it.
Inserting a change log incurs some minor overhead for sending query over
the network and reacting to its response. In many cases, this makes up
the bulk of the actual time it takes to run the change log insert.
To reduce this overhead and avoid any kind of processing delay in the
WAL consumers, we introduce batch insert functionality with size `500`
and timeout `30` seconds. If either of those two are hit, we flush the
batch using `insert_all`.
`insert_all` does not use `Ecto.Changeset`, so we need to be a bit more
careful about the data we insert, and check the inserted LSNs to
determine what to update the acknowledged LSN pointer to.
The functionality to determine when to call the new `on_flush/1`
callback lives in the replication_connection module, but the actual
behavior of `on_flush/1` is left to the child modules to implement. The
`Events.ReplicationConnection` module does not use flush behavior, and
so does not override the defaults, which is not to use a flush
mechanism.
Related: #949
When Firezone detects that the user is switching networks, we perform an
internal reset where we clear all connections and also all local
candidates. As part of the reset, we then send STUN requests to our
relays to re-discover our host and server-reflexive candidates. In this
scenario, the Gateway is still connected to its network and is therefore
able to send its candidates as soon as it receives the connection intent
from the portal.
This opens us up to the following race condition which leads to a
false-positive "ICE timeout":
1. Client roams network and clears all local state.
2. Client sends STUN binding requests to relays.
3. Client initiates a new connection.
4. Gateway acknowledges connection.
5. Client creates new ICE agent and attempts to seed it with local
candidates. We don't have a response from the relays yet and therefore
don't have any local candidates.
6. Client receives remote candidates and adds them to the agent.
7. ICE agent is unable to form pairs and therefore concludes that it is
disconnected.
8. We treat the disconnected event as a connection failure and clear the
connection.
9. Relays respond to STUN binding requests but we cannot add the new
candidates to the connection because it is already cleared.
The ICE spec states that after an agent transitions into the
"disconnected" state, it may transition back to "connected" if e.g. new
candidates are added as those allow the forming of new pairs. In
general, it is recommended to not treat "disconnected" as a permanent
state. To honor this recommendation, we introduce a 2s grace-period in
which we can recover from such a "disconnected" state.
At present, it appears that `actions/toolkit` has a bug where it isn't
always able to correctly fetch an ID token. See
https://github.com/actions/toolkit/issues/2098 for the upstream issue.
As a result, our CI often fails relatively often. A simple restart
usually fixes the issue. This however is annoying because it means PRs
get de-queued from the merge-queue or don't queue in the first place and
therefore require baby-sitting.
To fix this, we attempt to build a retry-mechanism from within the
action. Using `continue-on-error`, we tell the "auth" step to continue,
even if it fails. Following that, we try to authenticate again but only
if the previous one failed. We do this up to 3 times before actually
giving up.
---------
Co-authored-by: Jamil Bou Kheir <jamilbk@users.noreply.github.com>
We are currently in the process of transitioning the Firezone Clients
away from always hashing the ID before sending it to the portal. This
will make lookups and correlation of data between our systems much
easier.
The way we are performing this migration is that new installations of
Firezone will directly generate a 64 char hex-string as the Firezone ID.
If the ID looks like a UUID (which is the old format), we still hash it
and send it to the portal, otherwise we send it as-is.
Presently, the telemetry integration with Sentry and PostHog do the
opposite. They always sets the Firezone ID as-is and includes an
`external_id` that is the hashed form if it detects that it is a UUID
(or in the case of PostHog, create an alias). It is much better to flip
this around and always set the hex-string as the user id. That way, we
can simply always filter by the `user.id` attribute in Sentry and always
refer to the ID that we are seeing in the portal.
Feature flags may be accessed _very_ often such as on every log
statement with #9780. To make sure this is as performant as possible, we
move from an `RwLock` to atomic booleans with relaxed ordering.
Instead of conditionally enabling the `logs` feature in the Sentry
client, we always enable it and control via the `tracing` integration,
which events should get forwarded to Sentry. The feature-flag check
accesses only shared-memory and is therefore really fast.
We already re-evaluate feature flags on a timer which means this boolean
will flip over automatically and logs will be streamed to Sentry.
Customer hit what seems to be a rare race condition where we try to
connect whilst we already have a session. I don't know which state it is
in so I am replacing it with a WARN log to learn more about this in
Sentry in case it gets hit again.
Bumps [phoenix_ecto](https://github.com/phoenixframework/phoenix_ecto)
from 4.6.4 to 4.6.5.
<details>
<summary>Changelog</summary>
<p><em>Sourced from <a
href="https://github.com/phoenixframework/phoenix_ecto/blob/main/CHANGELOG.md">phoenix_ecto's
changelog</a>.</em></p>
<blockquote>
<h2>v4.6.5</h2>
<ul>
<li>Bug fixes
<ul>
<li>Unallow existing allowances when attempting to allow a Plug to
access a connection</li>
</ul>
</li>
</ul>
</blockquote>
</details>
<details>
<summary>Commits</summary>
<ul>
<li>See full diff in <a
href="https://github.com/phoenixframework/phoenix_ecto/commits/v4.6.5">compare
view</a></li>
</ul>
</details>
<br />
[](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores)
Dependabot will resolve any conflicts with this PR as long as you don't
alter it yourself. You can also trigger a rebase manually by commenting
`@dependabot rebase`.
[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)
---
<details>
<summary>Dependabot commands and options</summary>
<br />
You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits
that have been made to it
- `@dependabot merge` will merge this PR after your CI passes on it
- `@dependabot squash and merge` will squash and merge this PR after
your CI passes on it
- `@dependabot cancel merge` will cancel a previously requested merge
and block automerging
- `@dependabot reopen` will reopen this PR if it is closed
- `@dependabot close` will close this PR and stop Dependabot recreating
it. You can achieve the same result by closing it manually
- `@dependabot show <dependency name> ignore conditions` will show all
of the ignore conditions of the specified dependency
- `@dependabot ignore this major version` will close this PR and stop
Dependabot creating any more for this major version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this minor version` will close this PR and stop
Dependabot creating any more for this minor version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop
Dependabot creating any more for this dependency (unless you reopen the
PR or upgrade to it yourself)
</details>
Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
On Windows - in order to prevent routing loops - we resolve the best
"non-tunnel" route to a particular host for each IP address. The
resulting source IP is then used as source for packets leaving our
interface. In case the system doesn't have IPv6 connectivity or are
simply no routes available, we fail this "source IP resolver" with an IO
error.
Presently, this uses the "other" IO error type which causes this to be
logged on a WARN level in the event-loop. The IO error types
`HostUnreachable` and `NetworkUnreachable` are expected during normal
operation of Firezone and are therefore only logged on DEBUG.
By changing this IO error type, we fix the WARN log spam on Windows for
machines without IPv6 connectivity.
