Bumps [tokio-stream](https://github.com/tokio-rs/tokio) from 0.1.16 to
0.1.17.
<details>
<summary>Commits</summary>
<ul>
<li><a
href="67355c6d23"><code>67355c6</code></a>
chore: prepare tokio-stream v0.1.17 (<a
href="https://redirect.github.com/tokio-rs/tokio/issues/7020">#7020</a>)</li>
<li><a
href="405d746d38"><code>405d746</code></a>
signal: remove oneshot channels from tests (<a
href="https://redirect.github.com/tokio-rs/tokio/issues/7015">#7015</a>)</li>
<li><a
href="e0d1293fac"><code>e0d1293</code></a>
ci: add instructions that explain how to fix spellcheck errors (<a
href="https://redirect.github.com/tokio-rs/tokio/issues/7016">#7016</a>)</li>
<li><a
href="480c010b01"><code>480c010</code></a>
signal: add <code>SignalKind::info</code> on illumos (<a
href="https://redirect.github.com/tokio-rs/tokio/issues/6995">#6995</a>)</li>
<li><a
href="c032ea0203"><code>c032ea0</code></a>
ci: detect trailing whitespace (<a
href="https://redirect.github.com/tokio-rs/tokio/issues/7013">#7013</a>)</li>
<li><a
href="0b31c2f73d"><code>0b31c2f</code></a>
chore: prepare tokio-util v0.7.13 (<a
href="https://redirect.github.com/tokio-rs/tokio/issues/7012">#7012</a>)</li>
<li><a
href="129f9fc0c8"><code>129f9fc</code></a>
codec: fix incorrect handling of invalid utf-8 in
<code>LinesCodec::decode_eof</code> (#...</li>
<li><a
href="b5c227d51f"><code>b5c227d</code></a>
tracing: move tracing instrumentation tests into tokio tests (<a
href="https://redirect.github.com/tokio-rs/tokio/issues/7007">#7007</a>)</li>
<li><a
href="dcae2b9eb8"><code>dcae2b9</code></a>
ci: unfreeze FreeBSD from rustc 1.81 (<a
href="https://redirect.github.com/tokio-rs/tokio/issues/7009">#7009</a>)</li>
<li><a
href="bb9d57017e"><code>bb9d570</code></a>
chore: prepare Tokio v1.42.0 (<a
href="https://redirect.github.com/tokio-rs/tokio/issues/7005">#7005</a>)</li>
<li>Additional commits viewable in <a
href="https://github.com/tokio-rs/tokio/compare/tokio-stream-0.1.16...tokio-stream-0.1.17">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>
In case we fail to update the TUN device with its IPs or set the routes,
we currently just log a warning and continue operation. That isn't
ideal, setting the correct IPs and routes is crucial for the Gateway to
operate correctly.
Additionally, the design of sending the `Interface` through a separately
spawned task is kind of clunky. Instead of having a channel, we
introduce another `FuturesSet` that allows us to process these `async`
tasks inline within the event loop.
Exporting logs on macOS in >= 1.4.0 presents a challenge because the app
process and tunnel process now write to different directories. They do
this because the network extension is now packaged as a system
extension, which means it runs as root.
Since the Apple App Store requires apps to be sandboxed, and we don't
wish to break App Store compatibility just yet, the tunnel process and
app process can't read or write to a common shared directory anymore.
This leaves IPC as the only reliable means for app <-> tunnel
communication. We already use the provided `sendProviderMessage`
function exposed by Network Extension for other operations, so we add
another message type to handle log export.
The log archive from the tunnel needs to be sent in chunks to reduce the
amount of total memory the export process consumes. To facilitate this,
a custom TunnelLogArchive class is used which handles the business logic
of keeping a pointer into the file we're chunking over.
As each chunk is read from the tunnel log archive, we send it using the
`sendProviderMessage`'s completionHandler callback. If the number of
sent bytes is less than our maximum chunk size, we know we're done.
On the app side, we read each chunk of data from the tunnel process
until a `done` boolean is signaled, at which point we close the archive
file we're writing to and complete the export.
Lastly, we need to compress the app and tunnel archives separately, then
compress the final result into a final archive path set by the user.
This is because it's impossible to compress two decoupled directories at
once.
Very little to any noticeable delay is added when performing this over
the previous approach. Exporting multiple GB of log data from the tunnel
took only a second or two on my M1. Presumably, having to read
uncompressed chunks and doing the compression on the app-side would
result in a couple orders of magnitude more IPC calls.
Refs #7071
Since we need to react to state on this instance changing, we need an
instance of TunnelManager to last the lifetime of the app process.
Thus, it makes more sense to use the `.shared` public static pattern to
maintain an instance of it we can use from anywhere instead of having to
store it in various other classes.
In #7477, we introduced a regression in our test suite for DNS queries
that are forwarded through the tunnel.
In order to be deterministic when users configure overlapping CIDR
resources, we use the sort order of all CIDR resource IDs to pick, which
one "wins". To make sure existing connections are not interrupted, this
rule does not apply when we already have a connection to a gateway for a
resource. In other words, if a new CIDR resource (e.g. resource `A`) is
added to connlib that has an overlapping route with another resource
(e.g. resource `B`) but we already have a connection to resource `B`, we
will continue routing traffic for this CIDR range to resource `B`,
despite `A` sorting "before" `B`.
The regression that we introduced was that we did not account for
resources being "connected" after forwarding a query through the tunnel
to it. As a result, in the found failure case, the test suite was
expecting to route the packet to resource `A` because it did not know
that we are connected to resource `B` at the time of processing the ICMP
packet.
Previously, we needed to track our own user state in order to set the
whole thing in Sentry. That was necessary because Sentry didn't allow us
to _retrieve_ the current user of the scope but always required the full
user to be set. This was changed
https://github.com/getsentry/sentry-rust/pull/715, which allows us to
remove some of that code and hopefully mitigating any sort of lingering
state when it comes to telemetry sessions.
Due to how we currently initialise telemetry in the IPC service, I think
we are missing out on events when it _exits_ due to an error because we
don't explicitly stop the telemetry session. We have alerts from a fair
few users in Sentry where the IPC service appears to stop / disappear
but there are no corresponding events for the IPC service.
In case an upstream DNS server responds with a payload that exceeds the
available buffer space of an IP packet, we need to truncate the
response. Currently, this truncation uses the **wrong** constant to
check for the maximum allowed length. Instead of the
`MAX_DATAGRAM_PAYLOAD`, we actually need to check against a limit that
is less than the MTU as the IP layer and the UDP layer both add an
overhead.
To fix this, we introduce such a constant and provide additional
documentation on the remaining ones to hopefully avoid future errors.
The macOS client starting in 1.4.0 uses a system extension for its
network extension package type. This process runs as root and does not
have access to the app's Group Container folder for reading / writing
log files directly, and vice-versa. This means the tunnel now writes its
logs to a separate directory as the GUI app process.
Since the logging functions of clearing logs, calculating their size,
and exporting them assume all the logs are in the same directory, we
need to introduce IPC handlers to ensure the GUI app can conveniently
still perform these functions when initiated by the user.
We already use the Network Extension API `sendProviderMessage` as our
IPC mechanism for adhoc, bi-directional communication through the
tunnel, so we add more handlers to this mechanism to support the logging
functions summarized above.
In this PR we only fix the log size calculation and clear log
functionality. Exporting logs is more involved and will be implemented
in another dedicated PR.
When a Firezone Client roams, we reset all network connections and
rebind our local sockets. Doing that enables us to start from a clean
state and establish new connections to Gateways. What we are currently
not clearing are in-flight DNS queries. Those are all very likely to
fail because our network connection is changing. There is no point in us
keeping those around. Additionally, as part of roaming, it may also be
that our upstream DNS server changes and thus, we may suddenly receive a
response from a DNS server that we no longer know about.
Clear all in-flight DNS queries on reset solves this.
Certain packets cannot be translated as part of NAT64/46. The RFC says
to "Silently drop" those. Currently, we log all errors that happens
during the translation and don't follow this guideline.
Most of these "silently drop" errors are related to ICMP types that
cannot be represented in the other version such as ICMPv6 Neighbor
Solicitation.
To fix this, we introduce a new error type in the `ip_packet` module:
`ImpossibleTranslation`. For convenience reasons, we carry that one
through all layers as an `anyhow::Error` and test at the very top of the
event-loop, whether the root-cause of the error is such a failed
translation. If so, we ignore the error and move on. This isn't as
type-safe as it could be but it is much easier to implement.
Additionally, the risk of a bug here (i.e. if we stop emitting this
error within the IP packet translation layer) is merely that the log
will pop up again.
Resolves: #7516.
When the portal connection in a relay fails, we currently stringify the
error early. This is unnecessary and we should instead retain the full
error chain for as long as possible.
Initially, when we receive a new candidate from a remote peer, we bind a
channel for each remote address on the relay that we sampled. This
ensures that every possible communication path is actually functioning.
In ICE, all candidates are tried against each other, meaning the remote
will attempt to send from each of their candidates to every one of ours,
including our relay candidates. To allow this traffic, a channel needs
to be bound first.
For various reasons, an allocation might become stale or needs to be
otherwise invalidated. In that case, all the channel bindings are lost
but there might still be an active connection that wants to utilise
them. In that case, we will see "No channel" warnings like
https://firezone-inc.sentry.io/issues/6036662614/events/f8375883fd3243a4afbb27c36f253e23/.
To fix this, we use the attempt to encode a message for a channel as an
intent to bind a new one. This is deemed safe because wanting to encode
a message to a peer as a channel data message means we want such a
channel to exist. The first message here is still dropped but that is
better than not establishing the channel at all.
At present, the relay's event-loop simply drops a UDP packet in case the
socket is not ready for writing. This is terrible for throughput because
it means the encapsulated packet within the WG payload needs to be
retransmitted by the source after a timeout. To avoid this, we instead
buffer the packet and suspend the event loop until it has been correctly
flushed out. This may still cause packet loss because the receive buffer
may overflow in the meantime. However, there is nothing we can do about
that because UDP itself doesn't have any backpressure.
The relay listens on many sockets at once via a separate worker thread
and an `mio` event-loop. In addition to the current subscription to
readable event, we now also subscribe to writable events.
At the very top of the relay's event-loop, we insert a `flush` function
that ensures all buffered packets have been written out and - in case
writing a packet fails - suspends the event-loop with a waker. If we
receive a new event for write-readiness, we wake the waker which will
trigger a new call to `Eventloop::poll` where we again try to flush the
pending packet. We don't bother with tracking exactly, which socket sent
the write-readiness and which socket we have still pending packets in.
Instead, we suspend the entire event-loop until all pending packets have
been flushed.
Resolves: #7519.
When deciding what to do with a certain DNS query, we check whether the
domain name in question corresponds to any of the (wildcard) DNS
resource addresses. If yes, we resolve it to the resource ID of that
resource. The source of those resource IDs is the `dns_resources` map.
If we have looked up a `ResourceId` in that map, it is impossible for it
to not be "known" which means the branch deleted in this PR is
completely redundant and already covered by the catch-all branch where
`maybe_resource` is `None`.
When a Gateway or Client is running in an environment without IPv4 or
IPv6 connectivity, our initial probes for sending packets to the relays
will fail with network unreachable. That isn't a very big concern and
happens a lot in the wild. There is no need to report these as telemetry
events.
Resolves: #7514.
For persistent applications like the IPC service, it is possible that
telemetry gets initialised with different parameters depending on what
the user logs in with. Currently, only the first one is persisted and
all consecutive ones are ignored, leading to events that may be wrongly
tagged for a certain user / environment.
To fix this, we only skip the init if we are still in the same
environment. Otherwise, the close the previous session and initialise a
new one.
Fixes: #7525.
In order to achieve concurrency within `connlib`, we needed to create a
way for IP packets to own the piece of memory they are sitting in. This
allows us to concurrently read IP packets and them batch-process them
(as opposed to have a dedicated buffer and reference it). At the moment,
those IP packets are defined on the stack. With a size of ~1300 bytes
that isn't very large but still causes _some_ amount of copying.
We can avoid this copying by relying on a buffer pool:
1. When reading a new IP packet, we request a new buffer from the pool.
2. When the IP packet gets dropped, the buffer gets returned to the
pool.
This allows us to reuse an allocation for a packet once it finished
processing, resulting in less CPU time spent on copying around memory.
This causes us to make more _individual_ heap-allocations in the
beginning: Each packet is being processed by `connlib` is allocated on
the heap somewhere. At some point during the lifetime of the tunnel,
this will settle in an ideal state where we have allocated enough slots
to cover new packets whilst also reusing memory from packets that
finished processing already.
The actual `IpPacket` data type is now just a pointer. As a result, the
channels to and from the TUN thread (where we were holding multiple of
these packets) are now significantly smaller, leading to roughly the
same memory usage overall.
In my local testing on Linux, the client still only uses about ~15MB of
RAM even with multiple concurrent speedtests running.
Similar to #7497, when we receive a `ConnectResult`, we can simply
silently bail out of the function and not change our state instead of
printing a loud warning.
#7522 won't successfully complete on production because of the migration
in this PR. So, instead, we need to modify this migration, and then
manually apply the same operation to staging.
Normally, there always be exactly on pending flow per resource. It
appears though that it can sometimes happen that we first request a flow
for a resource but by the time it is authorised, we've already cleared
its local state.
Regardless, this isn't a concerning error and not worth logging on WARN
(which happens one layer up).
Windows appears to randomly fail to update the tray menu. There is
nothing we can do about that. Hence, we downgrade these errors to debug
and make the functions infallible, reducing the complexity for the
caller.
There is nothing we can do if the user doesn't have any DNS servers
defined. The default log level is INFO so a user reading the logs will
still come across this message in case they are trying to debug what is
happening.
Long term, problems like these would probably warrant some kind of
notification channel from `connlib` to the GUI where we can display
messages to the user.
There are several reasons why we can disconnect from a relay at runtime:
- STUN is blocked
- We have invalid credentials
- The TURN server is not protocol-conform
The first two are very much possible in production and there is nothing
we can do about them. When relays reboot, their credentials change and
if the Internet connection of a user / gateway gets cut, we may
disconnect from the relay because the messages get lost.
The last one should never happen if we are connected to our own relays.
Firezone can be self-hosted so ultimately, we don't have control over
what we are talking to. That error however is more of a safe-guard for
`connlib` itself to disconnect from the server as soon as it detects
that it is behaving weirdly.
None of these reasons are worth reporting to Sentry as a problem because
they aren't really fixable as such. It is more important that the user
sees them in the logs if they decide to dig into them which they will
still do on INFO level.
Why:
* Currently, when using the API, a user has no way of easily identifying
what identities they are pulling back as the response only includes the
`provider_identifier` which for most of our AuthProviders is an ID for
the IdP and not an email address. Along with that, when adding users to
an OIDC provider within Firezone, there is no check for whether or not
an identity has already been added with a given email address. By
creating a separate email column on the `auth_identities` table, it will
be very straight forward to know whether an email address exists for a
given identity, return it in an API response and allow the admin of a
Firezone account to track users (Identities) by email rather than IdP
identifier.
Fixes#7392
The communication between the GUI client, the IPC service and `connlib`
are asynchronous. As such, it may happen that the state machines run out
of sync. Receiving a `TunnelReady` despite not being in the right state
for that is no concern and can be handled gracefully.
In most cases, the caller of this function already handled the case of
it failing gracefully by logging. From Sentry alerts, we can see that if
this fails, there isn't much we can do about it and most likely, the
next refresh will work again (this has only happened a single time).
Logging this on `debug` is good enough in case something doesn't work
and we need to reproduce it or something really bad happens we need see
it in the breadcrumbs of another Sentry event.
When a client disconnects, we clear up the connection on the gateway.
There might still be packets arriving from resources that we then cannot
route. This isn't worth returning an error.
We are already handling one case where we are trying to remove a route
that doesn't exist. `ESRCH` is another variant of this error that
manifests as "No such process". According to the Internet, this just
means the route doesn't exist so we can bail out early here.
Currently, the Gateway logs all errors that happen when the event-loop
exits on ERROR level. This creates Sentry alerts for things like
"Unauthorized" errors or "404 Not found".
That isn't useful to us. To mitigate this, we polish the code a bit to
only log an ERROR when we actually fail to setup something during
startup (like the TUN device). In all other cases, we now log a more
user-friendly message on INFO but still exit with the appropriate exit
code (0 on CTRL+C, 1 on any other error).
