Closes#6873
The issue seems to be a race between flushing Sentry in the GUI process
and shutting down Firezone in the tunnel daemon (IPC service).
With this change, the GUI waits to hear `DisconnectedGracefully` from
the tunnel daemon before flushing Sentry, and the issue is prevented.
Adding the new state and new IPC message required small changes in
several places
---------
Signed-off-by: Reactor Scram <ReactorScram@users.noreply.github.com>
Co-authored-by: Thomas Eizinger <thomas@eizinger.io>
Bumps [derive_more](https://github.com/JelteF/derive_more) from 0.99.18
to 1.0.0.
<details>
<summary>Release notes</summary>
<p><em>Sourced from <a
href="https://github.com/JelteF/derive_more/releases">derive_more's
releases</a>.</em></p>
<blockquote>
<h2>v1.0.0 - Finally a stable release</h2>
<p><code>derive_more</code> is a library that adds derives for many of
the standard library traits. By using this library the following code
just works:</p>
<pre lang="rust"><code>use derive_more::{Add, Display, From, Into};
<p>#[derive(PartialEq, From, Add)]
struct MyInt(i32);</p>
<p>#[derive(PartialEq, From, Into)]
struct Point2D {
x: i32,
y: i32,
}</p>
<p>#[derive(PartialEq, From, Add, Display)]
enum MyEnum {
#[display("int: {_0}")]
Int(i32),
Uint(u32),
#[display("nothing")]
Nothing,
}</p>
<p>assert!(MyInt(11) == MyInt(5) + 6.into());
assert!((5, 6) == Point2D { x: 5, y: 6 }.into());
assert!(MyEnum::Int(15) == (MyEnum::Int(8) + 7.into()).unwrap());
assert!(MyEnum::Int(15).to_string() == "int: 15");
assert!(MyEnum::Uint(42).to_string() == "42");
assert!(MyEnum::Nothing.to_string() == "nothing");
</code></pre></p>
<p>Now, more than 8 years after the first commit and almost 5 years
after the 0.99.0 release, <code>derive_more</code> has finally reached
its 1.0.0 release. This release contains a lot of changes (including
some breaking ones) to make it easier to use the derives and make it
possible to extend them without having to break backwards compatibility
again. There are five major changes that I would like to call out, but
there are many more changes that are documented below:</p>
<ol>
<li>There is a new <code>Debug</code> derive that can be used to easily
customize <code>Debug</code> formatting.</li>
<li>A greatly improved <code>Display</code> derive, which allows you to
do anything that <a
href="https://github.com/dtolnay/thiserror"><code>thiserror</code></a>
provides, but it works for any type not just errors. And by combining
the <code>Display</code> derive with the <code>Error</code> and
<code>From</code> derives, there shouldn't really be any need to use
<code>thiserror</code> anymore (if you are missing a feature/behaviour
from <code>thiserror</code> please report an issue).</li>
<li>Traits that can return errors now return a type that implements
<code>Error</code> when an error occurs instead of a <code>&'static
str</code>.</li>
<li>When using <code>use derive_more::SomeTrait</code> the actual trait
is also imported not just the derive macro. This is especially useful
for <code>Error</code> and
<code>Display</code></li>
<li>The docs are now rendered on docs.rs and are much better
overall.</li>
</ol>
<h3>Breaking changes</h3>
<ul>
<li>The minimum supported Rust version (MSRV) is now Rust 1.75.</li>
<li>Add the <code>std</code> feature which should be disabled in
<code>no_std</code> environments.</li>
<li>All Cargo features, except <code>std</code>, are now disabled by
default. The <code>full</code> feature can be used to get the old
behavior of supporting all possible derives.</li>
<li>The <code>TryFrom</code>, <code>Add</code>, <code>Sub</code>,
<code>BitAnd</code>, <code>BitOr</code>, <code>BitXor</code>,
<code>Not</code> and <code>Neg</code> derives now return a dedicated
error type instead of a <code>&'static str</code> on error.</li>
<li>The <code>FromStr</code> derive now uses a dedicated
<code>FromStrError</code> error type instead of generating unique one
each time.</li>
<li>The <code>Display</code> derive (and other <code>fmt</code>-like
ones) now uses <code>#[display("...",
(<expr>),*)]</code> syntax instead of <code>#[display(fmt =
"...", ("<expr>"),*)]</code>, and
<code>#[display(bound(<bound>))]</code> instead of
<code>#[display(bound = "<bound>")]</code>. So without
the double quotes around the expressions and bounds.</li>
<li>The <code>Debug</code> and <code>Display</code> derives (and other
<code>fmt</code>-like ones) now transparently delegate to the inner type
when <code>#[display("...", (<expr>),*)]</code>
attribute is trivially substitutable with a transparent call. (<a
href="https://redirect.github.com/JelteF/derive_more/pull/322">#322</a>)</li>
</ul>
<!-- raw HTML omitted -->
</blockquote>
<p>... (truncated)</p>
</details>
<details>
<summary>Changelog</summary>
<p><em>Sourced from <a
href="https://github.com/JelteF/derive_more/blob/master/CHANGELOG.md">derive_more's
changelog</a>.</em></p>
<blockquote>
<h2>1.0.0 - 2024-08-07</h2>
<p>More than 8 years after the first commit and almost 5 years after the
0.99.0
release, <code>derive_more</code> has finally reached its 1.0.0 release.
This release
contains a lot of changes (including some breaking ones) to make it
easier to
use the derives and make it possible to extend them without having to
break
backwards compatibility again. There are five major changes that I would
like
to call out, but there are many more changes that are documented
below:</p>
<ol>
<li>There is a new <code>Debug</code> derive that can be used to easily
customize <code>Debug</code>
formatting.</li>
<li>A greatly improved <code>Display</code> derive, which allows you to
do anything that
<a
href="https://github.com/dtolnay/thiserror"><code>thiserror</code></a>
provides, but it works
for any type not just errors. And by combining the <code>Display</code>
derive with the
<code>Error</code> and <code>From</code> derives, there shouldn't really
be any need to use
<code>thiserror</code> anymore (if you are missing a feature/behaviour
from <code>thiserror</code>
please report an issue).</li>
<li>Traits that can return errors now return a type that implements
<code>Error</code>
when an error occurs instead of a <code>&'static str</code>.</li>
<li>When using <code>use derive_more::SomeTrait</code> the actual trait
is also imported
not just the derive macro. This is especially useful for
<code>Error</code> and
<code>Display</code></li>
<li>The docs are now rendered on docs.rs and are much better
overall.</li>
</ol>
<h3>Breaking changes</h3>
<ul>
<li>The minimum supported Rust version (MSRV) is now Rust 1.75.</li>
<li>Add the <code>std</code> feature which should be disabled in
<code>no_std</code> environments.</li>
<li>All Cargo features, except <code>std</code>, are now disabled by
default. The <code>full</code>
feature can be used to get the old behavior of supporting all possible
derives.</li>
<li>The <code>TryFrom</code>, <code>Add</code>, <code>Sub</code>,
<code>BitAnd</code>, <code>BitOr</code>, <code>BitXor</code>,
<code>Not</code> and <code>Neg</code>
derives now return a dedicated error type instead of a
<code>&'static str</code> on
error.</li>
<li>The <code>FromStr</code> derive now uses a dedicated
<code>FromStrError</code> error type instead
of generating unique one each time.</li>
<li>The <code>Display</code> derive (and other <code>fmt</code>-like
ones) now uses
<code>#[display("...", (<expr>),*)]</code> syntax
instead of
<code>#[display(fmt = "...",
("<expr>"),*)]</code>, and
<code>#[display(bound(<bound>))]</code>
instead of <code>#[display(bound = "<bound>")]</code>.
So without the double quotes
around the expressions and bounds.</li>
<li>The <code>Debug</code> and <code>Display</code> derives (and other
<code>fmt</code>-like ones) now transparently
delegate to the inner type when <code>#[display("...",
(<expr>),*)]</code> attribute is
trivially substitutable with a transparent call.
(<a
href="https://redirect.github.com/JelteF/derive_more/pull/322">#322</a>)</li>
<li>The <code>DebugCustom</code> derive is renamed to just
<code>Debug</code> (gated now under a separate
<code>debug</code> feature), and its semantics were changed to be a
superset of <code>std</code> variant
of <code>Debug</code>.</li>
<li>The <code>From</code> derive doesn't derive
<code>From<()></code> for enum variants without any
fields anymore. This feature was removed because it was considered
useless in</li>
</ul>
<!-- raw HTML omitted -->
</blockquote>
<p>... (truncated)</p>
</details>
<details>
<summary>Commits</summary>
<ul>
<li><a
href="d7f5b9e94d"><code>d7f5b9e</code></a>
chore: Release</li>
<li><a
href="40201b1e2e"><code>40201b1</code></a>
Update release date to be correct</li>
<li><a
href="88863ca4e6"><code>88863ca</code></a>
Update changelog wording</li>
<li><a
href="b713835894"><code>b713835</code></a>
Improve error when not enabling any features</li>
<li><a
href="330e4252fb"><code>330e425</code></a>
Order features in Cargo.toml alphabetically</li>
<li><a
href="84f2cbb05f"><code>84f2cbb</code></a>
Update README and CHANGELOG in preparation of 1.0.0</li>
<li><a
href="e8d60cf0e5"><code>e8d60cf</code></a>
Add compile_fail test for on purpose limited bounds (<a
href="https://redirect.github.com/JelteF/derive_more/issues/393">#393</a>,
<a
href="https://redirect.github.com/JelteF/derive_more/issues/392">#392</a>)</li>
<li><a
href="f665d18125"><code>f665d18</code></a>
Make anyhow reference a bit less strong</li>
<li><a
href="6d632b2db2"><code>6d632b2</code></a>
Add release announcement (<a
href="https://redirect.github.com/JelteF/derive_more/issues/390">#390</a>)</li>
<li><a
href="e87ab1315f"><code>e87ab13</code></a>
Don't create git tags for derive_more-impl (<a
href="https://redirect.github.com/JelteF/derive_more/issues/391">#391</a>)</li>
<li>Additional commits viewable in <a
href="https://github.com/JelteF/derive_more/compare/v0.99.18...v1.0.0">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>
Co-authored-by: Reactor Scram <ReactorScram@users.noreply.github.com>
Bumps
[sadness-generator](https://github.com/EmbarkStudios/crash-handling)
from 0.5.0 to 0.6.0.
<details>
<summary>Release notes</summary>
<p><em>Sourced from <a
href="https://github.com/EmbarkStudios/crash-handling/releases">sadness-generator's
releases</a>.</em></p>
<blockquote>
<h2>sadness-generator-0.6.0</h2>
<h3>Changed</h3>
<ul>
<li>Update MSRV to 1.62.0</li>
</ul>
</blockquote>
</details>
<details>
<summary>Commits</summary>
<ul>
<li><a
href="c61c72e0cf"><code>c61c72e</code></a>
chore: Release</li>
<li><a
href="a5553466de"><code>a555346</code></a>
chore: Release</li>
<li><a
href="45a469c86e"><code>45a469c</code></a>
chore: Release</li>
<li><a
href="d4d6f25cce"><code>d4d6f25</code></a>
chore: Release</li>
<li><a
href="7818928239"><code>7818928</code></a>
Update CHANGELOGs</li>
<li><a
href="e524a897c2"><code>e524a89</code></a>
Add heap corruption exception handling (<a
href="https://redirect.github.com/EmbarkStudios/crash-handling/issues/86">#86</a>)</li>
<li><a
href="065f3dd9c1"><code>065f3dd</code></a>
chore: Release</li>
<li><a
href="37e56acd3f"><code>37e56ac</code></a>
Update (<a
href="https://redirect.github.com/EmbarkStudios/crash-handling/issues/83">#83</a>)</li>
<li><a
href="3b77c9b00d"><code>3b77c9b</code></a>
chore: Release</li>
<li><a
href="d34d00bc51"><code>d34d00b</code></a>
chore: Release</li>
<li>Additional commits viewable in <a
href="https://github.com/EmbarkStudios/crash-handling/compare/sadness-generator-0.5.0...sadness-generator-0.6.0">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>
Co-authored-by: Reactor Scram <ReactorScram@users.noreply.github.com>
Closes#6854
- Sets release version from the GUI Client / Headless Client version
instead of the `firezone-telemetry` version
- Set environment to "production" and "staging" for well-known API URLs,
and "self-hosted" for others, since environments in Sentry can't have
slashes in them
- Sets API URL as a tag
- Sets release to `unit test` for unit testing `firezone-telemetry`
itself, since it has no good version number
<img width="398" alt="image"
src="https://github.com/user-attachments/assets/86f71193-2511-45c1-8304-413db8e5ef90">
When constructing a span, any currently set span will automatically be
set as the parent. In the case of the `connection` span, this was the
`accept_answer` or `new_connection` span from the client / gateway.
Those are not meant to be re-activated every time we enter the
`connection` span.
By setting an explicit parent, we avoid that.
Unfortunately, this means that this span will never have a parent, even
if other spans are active whilst we enter this one. We enter this one in
the hot-path, which is why it is being constructed ahead of time.
---------
Signed-off-by: Thomas Eizinger <thomas@eizinger.io>
This crate contains a useful macro `dbg!` which acts mostly like
`std::dbg!` but logs to `tracing` instead. Having it easily available as
a dependency during development makes debugging easier, especially with
`tunnel_test` which produces log-files per test-run.
Within `snownet` - `connlib`'s connectivity library - we use ICE to set
up a UDP "connection" between a client and a gateway. UDP is an
unreliable transport, meaning the only way how can detect that the
connection is broken is for both parties to constantly send messages and
acknowledgements back and forth. ICE uses STUN binding requests for
this.
In the default configuration of `str0m`, a STUN binding is sent every
3s, and we tolerate at most 9 missing responses before we consider the
connection broken. As these responses go missing, `str0m` halves this
interval, which results in a total ICE timeout of around 17 seconds. We
already tweak these values by reducing the number of requests to 8 and
setting the interval to 1.5s. This results in a total ICE timeout of
~10s which effectively means that there is at most a 10s lag between the
connection breaking and us considering it broken at which point new
packets arriving at the TUN interface can trigger the setup of a new
connection with the gateway.
Lowering these timeouts improves the user experience in case of a broken
connection because the user doesn't have to wait as long before they can
access their resources again. The downside of lowering these timeouts is
that we generate a lot of background noise. Especially on mobile
devices, this is bad because it prevents the CPU from going to sleep and
thus simply being signed into Firezone will drain your battery, even if
you don't use it.
Note that this doesn't apply at all if the client application on top
detects a network change. In that case, we hard-reset all connections
and instantly create new ones.
We attempted to fix this in #5576 by closing idle connections after 5
minutes. This however created new problems such as #6778.
The original problem here is that we send too many STUN messages as soon
as a connection is established. Simply increasing the timeout is not an
option because it would make the user experience really bad in case the
connection actually drops for reasons that the client app can't detect.
In this patch, we attempt to solve this in a different way: Detecting a
broken connection is only critical if the user is actively using the
tunnel (i.e. sending traffic). If there is no traffic, it doesn't matter
if we need longer to detect a broken connection. The user won't notice
because their phone is probably in their pocket or something.
With this patch, we now implement the following behaviour:
- A connection is considered idle after 10s of no application traffic.
- On idle connections, we send a STUN requests every 60s
- On idle connections, we wait for at most 4 missing responses before
considering the connection broken.
- Every connection will perform a client-initiated WireGuard keep-alive
every 25s, unless there is application traffic.
These values have been chosen while considering the following sources:
1. [RFC4787,
REQ-5](https://www.rfc-editor.org/rfc/rfc4787.html#section-12) requires
NATs to keep UDP NAT mappings alive for at least 2 minutes.
2.
[`conntrack`](https://www.kernel.org/doc/Documentation/networking/nf_conntrack-sysctl.rst)
adopts this requirement via the `nf_conntrack_udp_timeout_stream`
configuration.
3. 25s is the default keep-alive of the WireGuard kernel module.
In theory the WireGuard keep-alive itself should be good enough to keep
all NAT bindings alive. In practice, missed keep-alives are not exposed
by boringtun (the WireGuard implementation we rely on) and thus we need
the additional STUN keep-alives to detect broken connections. We set
those somewhat conservatively to 60s.
As soon as the user triggers new application traffic, these values are
reverted back to their defaults, meaning even if the connection died
just before the user is starting to use it again, we will know within
the usual 10s because we are triggering new STUN requests more often.
Note that existing gateways still implement the "close idle connections
after 5 minutes" behaviour. Customers will need to upgrade to a new
gateway version to fully benefit from these new always-on, low-power
connections.
Resolves: #6778.
---------
Signed-off-by: Thomas Eizinger <thomas@eizinger.io>
Refs #6138
Sentry is always enabled for now. In the near future we'll make it
opt-out per device and opt-in per org (see #6138 for details)
- Replaces the `crash_handling` module
- Catches panics in GUI process, tunnel daemon, and Headless Client
- Added a couple "breadcrumbs" to play with that feature
- User ID is not set yet
- Environment is set to the API URL, e.g. `wss://api.firezone.dev`
- Reports panics from the connlib async task
- Release should be automatically pulled from the Cargo version which we
automatically set in the version Makefile
Example screenshot of sentry.io with a caught panic:
<img width="861" alt="image"
src="https://github.com/user-attachments/assets/c5188d86-10d0-4d94-b503-3fba51a21a90">
In the past, we struggled a lot of the reproducibility of `tunnel_test`
failures because our input state and transition strategies were not
deterministic. In the end, we found out that it was due to the iteration
order of `HashMap`s.
To make sure this doesn't regress, we added a check to CI at the time
that compares the debug output of all regression seeds against a 2nd run
and ensures they are the same. That is overall a bit wonky.
We can do better by simple sampling a value from the strategy twice from
a test runner with the same seed. If the strategy is deterministic,
those need to be the same. We still rely on the debug output being
identical because:
a. Deriving `PartialEq` on everything is somewhat cumbersome
b. We actually care about the iteration order which a fancy `PartialEq`
implementation might ignore
Extracted from #6782
This moves more of `main` inside the async block, which makes it easier
to set up telemetry in the future.
We also log errors for the DNS notifier, which was overlooked before.
At the moment, the mapping of proxy IPs to the resolved IPs of a DNS
resource happens at the same time as the "authorisation" that the client
is allowed to talk to that resource. This is somewhat convoluted
because:
- Mapping proxy IPs to resolved IPs only needs to happen for DNS
resources, yet it is called for all resources (and internally skipped).
- Wildcard DNS resources only need to be authorised once, after which
the client is allowed to communicate with any domain matching the
wildcard address.
- The code that models resources within `ClientOnGateway` doesn't
differentiate between resource types at all.
With #6461, the authorisation of a resource will be completely decoupled
from the domain resolution for a particular domain of a DNS resource. To
make that easier to implement, we re-model the internals of
`ClientOnGateway` to differentiate the various resource types. Instead
of holding a single vec of addresses, the IPs are now indexed by the
respective domain. For CIDR resources, we only hold a single address
anyway and for the Internet Resource, the IP networks are static.
This new model now implies that allowing a resource that has already
been allowed essentially implies an update and the filters get
re-calculated.
Firefox uses this so-called canary domain `use-application-dns.net` to
detect, whether it should use DoH for its DNS queries. If answered with
a server error or without records, Firefox disables DoH as long as it
only its "Default protection" is enabled. If a user forces DoH, this
hint from the network is ignored.
See
https://support.mozilla.org/en-US/kb/canary-domain-use-application-dnsnet
for details.
I tested this on MacOS and Firefox does indeed instantly disable DoH. A
default installation of Chrome doesn't use DoH for me.
Related: #6375.
---------
Signed-off-by: Thomas Eizinger <thomas@eizinger.io>
Co-authored-by: Jamil <jamilbk@users.noreply.github.com>
Currently, `connlib` is entirely single-threaded. This allows us to
reuse a single buffer for processing IP packets and makes reasoning of
the packet processing code very simple. Being single-threaded also means
we can only make use of a single CPU core and all operations have to be
sequential.
Analyzing `connlib` using `perf` shows that we spend 26% of our CPU time
writing packets to the TUN interface [0]. Because we are
single-threaded, `connlib` cannot do anything else during this time. If
we could offload the writing of these packets to a different thread,
`connlib` could already process the next packet while the current one is
writing.
Packets that we send to the TUN interface arrived as an encrypted WG
packet over UDP and get decrypted into a - currently - shared buffer.
Moving the writing to a different thread implies that we have to have
more of these buffer that the next packet(s) can be decrypted into.
To avoid IP fragmentation, we set the maximum IP MTU to 1280 bytes on
the TUN interface. That actually isn't very big and easily fits into a
stackframe. The default stack size for threads is 2MB [1].
Instead of creating more buffers and cycling through them, we can also
simply stack-allocate our IP packets. This incurs some overhead from
copying packets but it is only ~3.5% [2] (This was measured without a
separate thread). With stack-allocated packets, almost all
lifetime-annotations go away which in itself is already a welcome
ergonomics boost. Stack-allocated packets also means we can simply spawn
a new thread for the packet processing. This thread is connected with
two channel to connlib's main thread. The capacity of 1000 packets will
at most consume an additional 3.5 MB of memory which is fine even on our
most-constrained devices such as iOS.
[0]: https://share.firefox.dev/3z78CzD
[1]: https://doc.rust-lang.org/std/thread/#stack-size
[2]: https://share.firefox.dev/3Bf4zlaResolves: #6653.
Resolves: #5541.
Currently, the order in which connlib matches against the patterns of
DNS resources is not specified. We simply iterate over all patterns and
take the first one that matches. Due to the iteration order of
`HashMap`s, this also isn't deterministic.
With this patch, we introduce a defined order in which we attempt to
match a particular domain against the defined DNS resources:
- Resources without wildcards are always prioritised over wildcard
domains
- Single-char wildcards (`?`) take priority over label wildcards (`*`)
- Label wildcards (`*`) take priority over catch-all wildcards (`**`)
By matching against the DNS resources in a defined order, we ensure that
DNS resources that overlap always resolve to the most specific resource.
---------
Signed-off-by: Thomas Eizinger <thomas@eizinger.io>
Co-authored-by: Reactor Scram <ReactorScram@users.noreply.github.com>
This log is currently printed after we receive the `init` message from
the client. It is a left-over from early days of connlib where receiving
`init` itself already triggered all kinds of actions.
These days, we are mostly just updating state. In addition, `init` can
be received multiple times during a client's session which is somewhat
confusing when you see multiple "Firezone started" logs.
When deciding which interface we are going to use for connecting to the
portal API, we need to filter through all adapters on Windows and
exclude our own TUN adapter to avoid routing loops. In addition, we also
need to filter for only online adapters, otherwise we might pick one
that is not actually routable.
Resolves: #6802.
Bumps [tauri-runtime](https://github.com/tauri-apps/tauri) from 0.14.4
to 0.14.5.
<details>
<summary>Release notes</summary>
<p><em>Sourced from <a
href="https://github.com/tauri-apps/tauri/releases">tauri-runtime's
releases</a>.</em></p>
<blockquote>
<h2>tauri-runtime v0.14.5</h2>
<p>Updating crates.io index
Locking 595 packages to latest compatible versions
Adding addr2line v0.22.0 (latest: v0.24.1)
Adding atk v0.15.1 (latest: v0.18.0)
Adding atk-sys v0.15.1 (latest: v0.18.0)
Adding base64 v0.13.1 (latest: v0.22.1)
Adding base64 v0.21.7 (latest: v0.22.1)
Adding bit-set v0.5.3 (latest: v0.8.0)
Adding bit-vec v0.6.3 (latest: v0.8.0)
Adding bitflags v1.3.2 (latest: v2.6.0)
Adding cairo-rs v0.15.12 (latest: v0.20.1)
Adding cairo-sys-rs v0.15.1 (latest: v0.20.0)
Adding cargo_toml v0.15.3 (latest: v0.20.4)
Adding cfb v0.7.3 (latest: v0.10.0)
Adding cfg-expr v0.9.1 (latest: v0.16.0)
Adding cfg-expr v0.15.8 (latest: v0.16.0)
Adding cfg_aliases v0.1.1 (latest: v0.2.1)
Adding clap v3.2.25 (latest: v4.5.16)
Adding clap_lex v0.2.4 (latest: v0.7.2)
Adding cocoa v0.24.1 (latest: v0.26.0)
Adding cocoa-foundation v0.1.2 (latest: v0.2.0)
Adding convert_case v0.4.0 (latest: v0.6.0)
Adding core-foundation v0.9.4 (latest: v0.10.0)
Adding core-graphics v0.22.3 (latest: v0.24.0)
Adding core-graphics v0.23.2 (latest: v0.24.0)
Adding core-graphics-types v0.1.3 (latest: v0.2.0)
Adding cssparser v0.27.2 (latest: v0.34.0)
Adding derive_more v0.99.18 (latest: v1.0.0)
Adding env_logger v0.8.4 (latest: v0.11.5)
Adding fixedbitset v0.4.2 (latest: v0.5.7)
Adding fluent-uri v0.1.4 (latest: v0.2.0)
Adding foreign-types v0.3.2 (latest: v0.5.0)
Adding foreign-types-shared v0.1.1 (latest: v0.3.1)
Adding freetype-sys v0.20.1 (latest: v0.22.0)
Adding gdk v0.15.4 (latest: v0.18.0)
Adding gdk-pixbuf v0.15.11 (latest: v0.20.1)
Adding gdk-pixbuf-sys v0.15.10 (latest: v0.20.1)
Adding gdk-sys v0.15.1 (latest: v0.18.0)
Adding gdkwayland-sys v0.15.3 (latest: v0.18.0)
Adding gdkx11-sys v0.15.1 (latest: v0.18.0)
Adding generator v0.7.5 (latest: v0.8.2)
Adding generic-array v0.14.7 (latest: v1.1.0)
Adding gethostname v0.4.3 (latest: v0.5.0)
Adding getrandom v0.1.16 (latest: v0.2.15)
Adding gimli v0.29.0 (latest: v0.31.0)
Adding gio v0.15.12 (latest: v0.20.1)
Adding gio-sys v0.15.10 (latest: v0.20.1)
Adding glib v0.15.12 (latest: v0.20.1)
Adding glib-macros v0.15.13 (latest: v0.20.1)</p>
<!-- raw HTML omitted -->
</blockquote>
<p>... (truncated)</p>
</details>
<details>
<summary>Commits</summary>
<ul>
<li><a
href="1f45808ba0"><code>1f45808</code></a>
Apply Version Updates From Current Changes (v1) (<a
href="https://redirect.github.com/tauri-apps/tauri/issues/10220">#10220</a>)</li>
<li><a
href="9082a39604"><code>9082a39</code></a>
fix(cli): env_logger usage</li>
<li><a
href="d84ae448e5"><code>d84ae44</code></a>
fix(cli): downgrade env_logger to 0.10 to satisfy MSRV, fix CI</li>
<li><a
href="3e1c28b90c"><code>3e1c28b</code></a>
chore(deps) Update dependency <code>@types/node</code> to v20.16.1 (<a
href="https://redirect.github.com/tauri-apps/tauri/issues/10722">#10722</a>)</li>
<li><a
href="2b61447dfc"><code>2b61447</code></a>
chore(deps) Update Rust crate proc-macro2 to v1.0.86 (<a
href="https://redirect.github.com/tauri-apps/tauri/issues/10621">#10621</a>)</li>
<li><a
href="0469b6b078"><code>0469b6b</code></a>
chore(deps) Update Tauri API Definitions (1.x) (<a
href="https://redirect.github.com/tauri-apps/tauri/issues/10622">#10622</a>)</li>
<li><a
href="3b841f28bb"><code>3b841f2</code></a>
chore(deps) Update Tauri CLI (1.x) (<a
href="https://redirect.github.com/tauri-apps/tauri/issues/10628">#10628</a>)</li>
<li><a
href="c3a90e5c27"><code>c3a90e5</code></a>
fix(tauri-runtime-wry): ensure tray is created when event loop ready (<a
href="https://redirect.github.com/tauri-apps/tauri/issues/10611">#10611</a>)</li>
<li><a
href="937849f28c"><code>937849f</code></a>
refactor(tauri-runtime-wry): Arc instead of Rc, closes <a
href="https://redirect.github.com/tauri-apps/tauri/issues/9775">#9775</a>
(<a
href="https://redirect.github.com/tauri-apps/tauri/issues/10587">#10587</a>)</li>
<li><a
href="48a7415c21"><code>48a7415</code></a>
chore(deps): update serialize-to-javascript to 0.1.2 (<a
href="https://redirect.github.com/tauri-apps/tauri/issues/10594">#10594</a>)</li>
<li>Additional commits viewable in <a
href="https://github.com/tauri-apps/tauri/compare/tauri-runtime-v0.14.4...tauri-runtime-v0.14.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>
Co-authored-by: Reactor Scram <ReactorScram@users.noreply.github.com>
Bumps [clap](https://github.com/clap-rs/clap) from 4.5.4 to 4.5.13.
<details>
<summary>Release notes</summary>
<p><em>Sourced from <a
href="https://github.com/clap-rs/clap/releases">clap's
releases</a>.</em></p>
<blockquote>
<h2>v4.5.13</h2>
<h2>[4.5.13] - 2024-07-31</h2>
<h3>Fixes</h3>
<ul>
<li><em>(derive)</em> Improve error message when
<code>#[flatten]</code>ing an optional <code>#[group(skip)]</code></li>
<li><em>(help)</em> Properly wrap long subcommand descriptions in
help</li>
</ul>
<h2>v4.5.12</h2>
<h2>[4.5.12] - 2024-07-31</h2>
<h2>v4.5.10</h2>
<h2>[4.5.10] - 2024-07-23</h2>
<h2>v4.5.9</h2>
<h2>[4.5.9] - 2024-07-09</h2>
<h3>Fixes</h3>
<ul>
<li><em>(error)</em> When defining a custom help flag, be sure to
suggest it like we do the built-in one</li>
</ul>
<h2>v4.5.8</h2>
<h2>[4.5.8] - 2024-06-28</h2>
<h3>Fixes</h3>
<ul>
<li>Reduce extra flushes</li>
</ul>
<h2>v4.5.7</h2>
<h2>[4.5.7] - 2024-06-10</h2>
<h3>Fixes</h3>
<ul>
<li>Clean up error message when too few arguments for
<code>num_args</code></li>
</ul>
<h2>v4.5.6</h2>
<h2>[4.5.6] - 2024-06-06</h2>
</blockquote>
</details>
<details>
<summary>Changelog</summary>
<p><em>Sourced from <a
href="https://github.com/clap-rs/clap/blob/master/CHANGELOG.md">clap's
changelog</a>.</em></p>
<blockquote>
<h2>[4.5.13] - 2024-07-31</h2>
<h3>Fixes</h3>
<ul>
<li><em>(derive)</em> Improve error message when
<code>#[flatten]</code>ing an optional <code>#[group(skip)]</code></li>
<li><em>(help)</em> Properly wrap long subcommand descriptions in
help</li>
</ul>
<h2>[4.5.12] - 2024-07-31</h2>
<h2>[4.5.11] - 2024-07-25</h2>
<h2>[4.5.10] - 2024-07-23</h2>
<h2>[4.5.9] - 2024-07-09</h2>
<h3>Fixes</h3>
<ul>
<li><em>(error)</em> When defining a custom help flag, be sure to
suggest it like we do the built-in one</li>
</ul>
<h2>[4.5.8] - 2024-06-28</h2>
<h3>Fixes</h3>
<ul>
<li>Reduce extra flushes</li>
</ul>
<h2>[4.5.7] - 2024-06-10</h2>
<h3>Fixes</h3>
<ul>
<li>Clean up error message when too few arguments for
<code>num_args</code></li>
</ul>
<h2>[4.5.6] - 2024-06-06</h2>
<h2>[4.5.5] - 2024-06-06</h2>
<h3>Fixes</h3>
<ul>
<li>Allow <code>exclusive</code> to override
<code>required_unless_present</code>,
<code>required_unless_present_any</code>,
<code>required_unless_present_all</code></li>
</ul>
</blockquote>
</details>
<details>
<summary>Commits</summary>
<ul>
<li><a
href="d222ae4cb6"><code>d222ae4</code></a>
chore: Release</li>
<li><a
href="a8abcb40c5"><code>a8abcb4</code></a>
docs: Update changelog</li>
<li><a
href="2690e1bdb1"><code>2690e1b</code></a>
Merge pull request <a
href="https://redirect.github.com/clap-rs/clap/issues/5621">#5621</a>
from shannmu/dynamic_valuehint</li>
<li><a
href="7fd7b3e40b"><code>7fd7b3e</code></a>
feat(clap_complete): Support to complete custom value of argument</li>
<li><a
href="fc6aaca52b"><code>fc6aaca</code></a>
Merge pull request <a
href="https://redirect.github.com/clap-rs/clap/issues/5638">#5638</a>
from epage/cargo</li>
<li><a
href="631e54bc71"><code>631e54b</code></a>
docs(cookbook): Style cargo plugin</li>
<li><a
href="6fb49d08bb"><code>6fb49d0</code></a>
Merge pull request <a
href="https://redirect.github.com/clap-rs/clap/issues/5636">#5636</a>
from gibfahn/styles_const</li>
<li><a
href="6f215eee98"><code>6f215ee</code></a>
refactor(styles): make styles example use a const</li>
<li><a
href="bbb2e6fdde"><code>bbb2e6f</code></a>
test: Add test case for completing custom value of argument</li>
<li><a
href="999071c46d"><code>999071c</code></a>
fix: Change <code>visible</code> to <code>hidden</code></li>
<li>Additional commits viewable in <a
href="https://github.com/clap-rs/clap/compare/clap_complete-v4.5.4...clap_complete-v4.5.13">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>
Co-authored-by: Reactor Scram <ReactorScram@users.noreply.github.com>
Closes#6791
We weren't closing the connlib session immediately when we get
`on_disconnect`, this patch fixes that.
This passes the manual test established in #6792. I also cycled through
sign-in, close, open, sign-out, and it looks fine.
---------
Signed-off-by: Reactor Scram <ReactorScram@users.noreply.github.com>
Synthetic replication for #6791.
The diff for the fix will probably be short, so I wanted this diff for
the test to be reviewed separately.
In your normal terminal: `cargo build -p firezone-gui-client -p
gui-smoke-test`
With sudo / admin powers: `./target/debug/gui-smoke-test.exe
--manual-tests`
Some customers _must_ have hit this, it's so easy to trigger.
I can't add it to the CI smoke test because there's no portal in CI
during the smoke test, unless we use Staging.
Fix#6781Fix#6375
The problem was that browsers in iOS(and possible other OSes) queries
for A, AAAA and HTTPS, and we correctly intercept A and AAAA.
Correctly intercepting HTTPS queries is more tricky since we need the
server's alpn, before this PR we were just forwarding those and then the
response back but the problem with that is that it'd return the real IP
for the service instead of our proxy IP.
So to quickly fix this we simply blackhole the query so the browser
never use that response.
In the future an improvement over this would be to intercept the
response instead of the query and mangle the ips there.
---------
Signed-off-by: Gabi <gabrielalejandro7@gmail.com>
Co-authored-by: Reactor Scram <ReactorScram@users.noreply.github.com>
Previously, the type signatures around parsing DNS queries were kind of
weird and hard to follow. Instead of nesting `Option` and `Result`, we
now use the `ControlFlow` type which is more expressive in what we
expect the caller to do with the return value.
We reserve an IP range _within_ the CG-NAT range for the sentinel DNS
servers. It is unnecessary to explicitly add that one as a route because
it is already covered by the routing entry of the entire CG-NAT range.
When encountering a PTR query, `connlib` checks if the query is for a
Firezone-managed resource and resolve it to the correct IP. If it isn't
for a DNS resource, we should forward the query to the upstream
resolver.
This isn't what is currently happening though. Instead of forwarding the
query, we bail early from `StubResolver::handle` and thus attempt to
route the packet through the tunnel. This however fails because the DNS
query was targeted at `connlib`'s stub resolver address which never
corresponds to a resource IP.
When TRACE logs where activated, this resulted in several entries such
as
> Unknown resource dst=100.100.111.1
To ensure this doesn't regress, we now generate PTR and MX record
queries in `tunnel_test`. We don't assert the response of those but we
do assert that we always get a response. The inclusion of MX records
asserts that unknown query types get correctly forwarded.
Resolves: #6749.
There were 3 problems currently on main, one on the tests and the actual
bug.
## Test problem
The routes were kept in a `BTreeSet` that when a new route was added it
was `insert`ed into and when it was removed it was `remove`d from using
the address of the route.
The problem is if there were overlapping route added twice in a row and
then a single one of those resources is removed the test would believe
the route no longer exists.
## Test solution
Keep the routes in a `BTreeMap` which maps the id to the ip and then we
calculate the routes based on that combined with the default routes,
that way we just remove the ID and the routes are kept in the correct
expected state.
## Real bug
So fixing this revealed a similar bug in connlib, since we kept things
in a similar struct, `active_cidr_resources` using `IpNetworkTable`.
To fix this I re-calculate the whole table each time we add/remove a
resource.
Note that this really doesn't properly fixes overlapping routes, this is
just helpful to fix the test, to fix them we need #4789
Furthermore, fixing these issues revealed an additional problem,
whenever we add an overlapping CIDR resource the old resource might be
overridden, causing the connection to be lost, furthermore this happened
in a non-deterministic(it's deterministic really but not explicit) way
causing the tests to fail.
To fix this we always sort resources by ID(it's an arbitrary order to
keep consistency with the proptests) and then we don't replace the
routing for resources that already had a connection.
Sadly, to model this in the test I had to almost copy exactly how we
calculate resources in connlib.
Fixes#6721
---------
Signed-off-by: Gabi <gabrielalejandro7@gmail.com>
Co-authored-by: Thomas Eizinger <thomas@eizinger.io>
Merging #6708 had an unintended side-effect that we are seeing a lot of
WARN logs from phoenix-channel because we can no longer parse the
response from gateways. We didn't do anything with these responses but
gateways are sending them for backwards-compatibility reasons.
To not confuse ourselves while debugging, we revert the client-side bit
of #6708 to remove these warnings.
Currently, checking whether a packet is a DNS query has multiple silent
exit paths. This makes it DNS problems difficult to debug because the
packets will be treated as if they have to get routed through the
tunnel.
This is also something we should fix but that isn't done in this PR: If
we know that a packet is for connlib's DNS stub resolver, we should
never route it through the tunnel. Currently, this isn't possible to
express with the type signature of our DNS module and requires more
refactoring.
---------
Signed-off-by: Thomas Eizinger <thomas@eizinger.io>
Co-authored-by: Jamil <jamilbk@users.noreply.github.com>
DNS queries for non-resources get forwarded by connlib byte-for-byte and
only the IP & UDP headers gets rewritten. The responses for some DNS
queries don't fit into UDP packets. To signal this, DNS servers set the
TC (truncated) bit on the response.
If set, most clients will discard the response and requery over TCP. DNS
over TCP is not yet supported in connlib, meaning any query response
that has the TC bit set will most likely lead to DNS issues downstream.
To make these cases easier to debug, we log a warning whenever we
encounter a DNS response from an upstream server that that has the TC
bit set.
Prior to version 1.1.0, clients did not have an embedded DNS resolver
and relied on the gateway for DNS resolution. In that design, the
gateway responded with the IPs that the domain resolved to.
Our next iteration of the control protocol (#6461) will decouple the
details of how DNS works from the flow-authorization. As a result, we
will need to be able to establish a flow for a DNS resource without
knowing which concrete domain the client is going to access.
Without a concrete domain, we cannot send anything back to these old
clients, meaning we unfortunately have to break compatibility with <
1.1.0 clients as part of implementing the new control protocol.
This only happens once for each domain, so logging this on DEBUG should
be fine. It is disabled by default and shouldn't be too spammy during
development.
This happens because the smoke test is stubbed out for release builds,
so any `use` statements that are only used in the smoke tests will cause
a warning in `--release` builds, including when we make release bundles.
