Regardless of `FIREZONE_DNS_CONTROL`, always try to notify systemd that
we've started.
I had accidentally conflated the idea of running as a systemd service
with the idea of using systemd to control DNS. They're separate, but
I'll keep the service unit in here and always use `sd-notify` since it
should be harmless to use even in Alpine.
~~If `FIREZONE_DNS_CONTROL` is `systemd-resolved`, try to notify systemd
that we've finished startup and the tunnel is ready.~~
Also adds a CI test, including a systemd service file that is **not**
ready for general use.
Ready for review once it's green.
---------
Signed-off-by: Reactor Scram <ReactorScram@users.noreply.github.com>
Co-authored-by: Thomas Eizinger <thomas@eizinger.io>
Reverts #3622 I don't know why, but that change seemed to cause the
`/etc/resolv.conf` test to fail in CI and I was thinking of the "roll
back first" principle
https://cloud.google.com/blog/products/gcp/reliable-releases-and-rollbacks-cre-life-lessons
~~I also change one `ping` in CI to `until ping`. This was an earlier
attempt before I did the revert, and it seems safe to leave it in.~~
Initially, we thought that we need to replace the entire `Allocation` if
the credentials to the relay change. However, during testing it turned
out that the credentials will change every time the portal sends us new
credentials. Likely, the portal hashes some kind of nonce into the
password as well.
Consequently, throwing away the entire state of the `Allocation` is
wrong. Instead, we will simply try to refresh the allocation using the
new credentials. If the refresh fails, we will try to make a new
allocation. If that also fails unrecoverably, then we "suspend" the
allocation, i.e. the `Allocation` will not perform any further action by
itself.
In case we get a new `refresh` call (which happens every time we want to
use the `Allocation` for a connection), we restart things and try to
make a new one.
With #3391, constructing a new tunnel will no longer be `async` which
makes DNS resolution the only `async` component of
`set_peer_connection_request`. In general, adding resources as part of
setting up a connection is a duplicated of the logic within
`allow_access`.
We solve both of these problems at once by moving the DNS resolution out
of `connlib` into the `gateway` binary and perform it as part of the
eventloop during a connection setup.
Only user-facing if users are using the Docker image for the Linux
client.
I split off a module for `/etc/resolv.conf` since the code and unit
tests are about 300 lines and aren't related to the rest of the
`tun_linux.rs` code.
---------
Signed-off-by: Reactor Scram <ReactorScram@users.noreply.github.com>
Without this alignment, accessing the `name` field reliably produces a
segfault:
```
Fatal signal 11 (SIGSEGV), code 1 (SEGV_MAPERR), fault addr 0x1d in tid 13835 (Thread-7), pid 13757 (irezone.android)
```
Interestingly, this only happens in release builds on 32-bit platforms.
Logging the returned name fixes it too which hints at some kind of
optimisation issue. Adding a padding is the most reliable fix.
Fixes: #3637.
---------
Signed-off-by: Jamil <jamilbk@users.noreply.github.com>
Co-authored-by: Thomas Eizinger <thomas@eizinger.io>
The Docker image for the client is opted in to this new feature. The
bare `linux-client-x64` exe is not. I don't know if users are using the
Docker images?
I wanted to use CLI args, but the DNS control code ("config" or
"control"? Or "SplitDNS"?) has to run at the end of `set_iface_config`,
which on Linux runs in a worker, so I couldn't figure out how to move it
into `on_set_interface_config` in the callbacks. Maybe there is a way,
but the env var results in a small diff.
`str0m` sends its own STUN keep-alives and @conectado has already
removed the logic that uses the wireguard keep-alives to detect stale
connections in
8234529cdf
as part of the integration of `snownet`.
We don't need two keep-alive mechanisms at once.
Fixes#3578Fixes#3551
The issue turned out to be a bunk Repository. Upon unraveling that ball
of yarn, I decided to clean up the Tunnel implementation altogether. It
uses the existing tunnel in-memory store for pushing updates to a
connected SessionActivity.
This PR includes many bug fixes as well.
Bumps [crash-handler](https://github.com/EmbarkStudios/crash-handling)
from 0.6.0 to 0.6.1.
<details>
<summary>Release notes</summary>
<p><em>Sourced from <a
href="https://github.com/EmbarkStudios/crash-handling/releases">crash-handler's
releases</a>.</em></p>
<blockquote>
<h2>crash-handler-0.6.1</h2>
<h3>Added</h3>
<ul>
<li><a
href="https://redirect.github.com/EmbarkStudios/crash-handling/pull/81">PR#81</a>
resolved <a
href="https://redirect.github.com/EmbarkStudios/crash-handling/issues/79">#79</a>
by adding <code>make_single_crash_event</code>.</li>
</ul>
</blockquote>
</details>
<details>
<summary>Commits</summary>
<ul>
<li><a
href="d34d00bc51"><code>d34d00b</code></a>
chore: Release</li>
<li><a
href="789c99498c"><code>789c994</code></a>
Update CHANGELOGs</li>
<li><a
href="addf1486f8"><code>addf148</code></a>
Update (<a
href="https://redirect.github.com/EmbarkStudios/crash-handling/issues/81">#81</a>)</li>
<li><a
href="16c2545f2a"><code>16c2545</code></a>
chore: Release</li>
<li><a
href="955629bab9"><code>955629b</code></a>
Update CHANGELOG</li>
<li><a
href="5e907ff389"><code>5e907ff</code></a>
Add Android support for the i686 and x86-64 targets (<a
href="https://redirect.github.com/EmbarkStudios/crash-handling/issues/76">#76</a>)</li>
<li><a
href="14bba1b81e"><code>14bba1b</code></a>
Fix using <code>crash-handler</code> under Miri (<a
href="https://redirect.github.com/EmbarkStudios/crash-handling/issues/75">#75</a>)</li>
<li><a
href="1db8fca031"><code>1db8fca</code></a>
chore: Release</li>
<li>See full diff in <a
href="https://github.com/EmbarkStudios/crash-handling/compare/crash-handler-0.6.0...crash-handler-0.6.1">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 <dependency name> major version` will close this
group update PR and stop Dependabot creating any more for the specific
dependency's major version (unless you unignore this specific
dependency's major version or upgrade to it yourself)
- `@dependabot ignore <dependency name> minor version` will close this
group update PR and stop Dependabot creating any more for the specific
dependency's minor version (unless you unignore this specific
dependency's minor version or upgrade to it yourself)
- `@dependabot ignore <dependency name>` will close this group update PR
and stop Dependabot creating any more for the specific dependency
(unless you unignore this specific dependency or upgrade to it yourself)
- `@dependabot unignore <dependency name>` will remove all of the ignore
conditions of the specified dependency
- `@dependabot unignore <dependency name> <ignore condition>` will
remove the ignore condition of the specified dependency and ignore
conditions
</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>
This splits off the easy parts from #3605.
- Add quotes around `PHOENIX_SECURE_COOKIES` because my local
`docker-compose` considers unquoted 'false' to be a schema error - Env
vars are strings or numbers, not bools, it says
- Create `test.httpbin.docker.local` container in a new subnet so it can
be used as a DNS resource without the existing CIDR resource picking it
up
- Add resources and policies to `seeds.exs` per #3342
- Fix warning about `CONNLIB_LOG_UPLOAD_INTERVAL_SECS` not being set
- Add `resolv-conf` dep and unit tests to `firezone-tunnel` and
`firezone-linux-client`
- Impl `on_disconnect` in the Linux client with `tracing::error!`
- Add comments
```[tasklist]
- [x] (failed) Confirm that the client container actually does stop faster this way
- [x] Wait for tests to pass
- [x] Mark as ready for review
```
Bumps [time](https://github.com/time-rs/time) from 0.3.32 to 0.3.34.
<details>
<summary>Release notes</summary>
<p><em>Sourced from <a
href="https://github.com/time-rs/time/releases">time's
releases</a>.</em></p>
<blockquote>
<h2>v0.3.34</h2>
<p>See the <a
href="https://github.com/time-rs/time/blob/main/CHANGELOG.md">changelog</a>
for details.</p>
<h2>v0.3.33</h2>
<p>See the <a
href="https://github.com/time-rs/time/blob/main/CHANGELOG.md">changelog</a>
for details.</p>
</blockquote>
</details>
<details>
<summary>Changelog</summary>
<p><em>Sourced from <a
href="https://github.com/time-rs/time/blob/main/CHANGELOG.md">time's
changelog</a>.</em></p>
<blockquote>
<h2>0.3.34 [2024-12-03]</h2>
<h3>Fixed</h3>
<p>Computing the local offset on Windows works again. It was broken in
some cases in v0.3.32 and
v0.3.33.</p>
<h2>0.3.33 [2024-02-03]</h2>
<h3>Fixed</h3>
<p>Builds targeting <code>wasm32-unknown-unknown</code> now work
again.</p>
</blockquote>
</details>
<details>
<summary>Commits</summary>
<ul>
<li><a
href="f11f9b2a0c"><code>f11f9b2</code></a>
v0.3.34 release</li>
<li><a
href="ef7bfbd638"><code>ef7bfbd</code></a>
fix unsigned to signed conversion bug (<a
href="https://redirect.github.com/time-rs/time/issues/656">#656</a>)</li>
<li><a
href="76468cb651"><code>76468cb</code></a>
v0.3.33 release</li>
<li><a
href="6c2b602a41"><code>6c2b602</code></a>
Fix wasm32-unknown-unknown build (<a
href="https://redirect.github.com/time-rs/time/issues/655">#655</a>)</li>
<li>See full diff in <a
href="https://github.com/time-rs/time/compare/v0.3.32...v0.3.34">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 <dependency name> major version` will close this
group update PR and stop Dependabot creating any more for the specific
dependency's major version (unless you unignore this specific
dependency's major version or upgrade to it yourself)
- `@dependabot ignore <dependency name> minor version` will close this
group update PR and stop Dependabot creating any more for the specific
dependency's minor version (unless you unignore this specific
dependency's minor version or upgrade to it yourself)
- `@dependabot ignore <dependency name>` will close this group update PR
and stop Dependabot creating any more for the specific dependency
(unless you unignore this specific dependency or upgrade to it yourself)
- `@dependabot unignore <dependency name>` will remove all of the ignore
conditions of the specified dependency
- `@dependabot unignore <dependency name> <ignore condition>` will
remove the ignore condition of the specified dependency and ignore
conditions
</details>
Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Currently, we log messages from the portal several times via different
ways. For one, the message is included in the span via
`tracing::instrument`. Then it is also logged on `trace!` level twice
(this PR removes one of them). Plus, the fields of the
`request_connection` span are not printed in a very human-readable way
(bytes arrays). This makes the logs super noisy, see here for the latest
run on `main`:
https://github.com/firezone/firezone/actions/runs/7808301643/job/21298585685#step:13:13
Compare this with the logs from the run in this PR:
https://github.com/firezone/firezone/actions/runs/7813774334/job/21313812863?pr=3596#step:13:13
Some of these improvements were made as part of debugging #3391.
Extracting them here in a separate PR to reduce the diff of #3391.
Currently, a `REFRESH` for an allocation is only triggered after half
its lifetime (which defaults to 10 minutes). A refresh is the only way
for us to check whether an allocation is still active. If the relay
restarted or the allocation was somehow invalidated otherwise,
attempting to refresh it will fail.
By triggering a refresh for each allocation every time we get a new
connection, we immediately check whether that allocation (and thus its
candidates) are still valid. What to do with invalidated candidates is
left to a future iteration.
When a relay restarts, its credentials change but the socket we use to
connect to it might not. Because we upsert `Allocation`s within a
`snownet::Node` based on the socket, such a change is currently not
picked up.
Instead, we now check whether an existing allocation uses the same
credentials and if it doesn't we throw the old one away and use the new
one instead.
Candidates, especially relay candidates can become invalid. For example,
a relay might be shut down, change its credentials or now be unreachable
due to network changes. In order to signal these changes in network
connections to a `snownet::Node`, we introduce a `CandidateEvent`.
Right now, `CandidateEvent` does not yet have an `Invalid` variant
because it would be dead code (it is not yet emitted by anything). This
PR is just the scaffolding to make that easier to introduce later.
If we receive a 401 or a 438, we should re-authenticate the request with
the new nonce sent from the server. Except if we had sent a nonce and
receive a 401, then it means our credentials are completely invalid and
retrying to authenticate will not solve it.
Mangling the source IP is not correct, we actually need to search for
the allocated socket within our list of allocations, otherwise we cannot
translate between IPv4 and IPv6 as we e.g. wouldn't find an IPv6
allocation on an IPv4 relay.
This was causing some problems in my tests with connlib, when the client
re-created the connection and recieved the candidates before the offer
response, this *seems* like it made the client hang waiting for the
connection to happen, without failing it.
I couldn't find out why exactly the hang happens since it seems like the
remote candidates are added to the new connection and stun packets are
directed to the new connection, so it may be unrelated but I think it's
still right to clean this hashmap up.
---------
Co-authored-by: Thomas Eizinger <thomas@eizinger.io>
Closes#3567
This doesn't feel complete, but there isn't much more I can do without
changing lots of code and doing more research, and I don't want to hold
up the Windows beta for error handling that may not be used.
I'm only handling crashes, errors, and panics in the `Controller` task
since it's hard to simulate them elsewhere. Everything else like the
wintun worker thread will have to be best-effort or tested by modifying
the code temporarily.
Crashes go to stderr and and the log file, and we can get a line number
from the crash dump:
Errors go to stderr and the log file, but we don't get a line number,
because we aren't capturing the backtrace:
Panics go to stderr and the log file, but we don't even get the panic
message. I don't think I can do much about that. Tokio doesn't give us
the full `PanicInfo`, which contains the message and location (line
number). If we end up debugging a panic in prod we could bisect by
putting tighter bounds until we narrow it down.
The panic hook ends up not triggered because:
- If I don't catch the panic, the app won't close, because Tokio will
catch the panic
- If I do catch the panic, I can't call the panic hook manually, because
Tokio doesn't give me a `PanicInfo` struct. And I can't resume the
panic, because I'm still inside a Tokio task.
I don't think there's any point capturing a crash dump on panic, because
Tokio will have already stopped the panicked thread, and the backtrace
will only show the catch location.
Maybe the message would print if I changed the controller task to a
controller thread, so that panicking that thread will trigger
`tracing_panic` and then abort the process? But that's a bigger change.
---------
Signed-off-by: Reactor Scram <ReactorScram@users.noreply.github.com>
Co-authored-by: Gabi <gabrielalejandro7@gmail.com>
Closes#3521 because now almost all of the `?` in the main loop will log
an error and not bail. This is nice for trivial things like copying a
resource when we're not signed in. (e.g. User opens the menu, the token
expires, user clicks something, it should just do nothing instead of
crashing)
Closes a long-standing TODO for moving more of the request handling code
inside `impl Controller`
In order to bind a channel to an IPv4 peer, the client must have
previously made an IPv4 allocation. The same applies to IPv6. The relay
will enforce this and reject the channel binding if that isn't the case
but we shouldn't rely on this in the client code. Instead, we should not
attempt to bind a channel if we haven't previously made an allocation
for that IP family.
We always try to make an allocation for both IPv4 and IPv6 but not every
relay may operate in a dual-socket mode. Thus, it may only return an
IPv4 or an IPv6 address.
Initially, I thought it could be useful to keep the connection around in
case we want to do an ICE restart. We don't do that (yet) so for now,
the safer option is to just clean up all the state and let the upper
layers deal with reestablishing it.
This should hopefully fix the repeated emitting of the
`ConnectionFailed` event.
Bumps [minidumper](https://github.com/EmbarkStudios/crash-handling) from
0.8.0 to 0.8.1.
<details>
<summary>Release notes</summary>
<p><em>Sourced from <a
href="https://github.com/EmbarkStudios/crash-handling/releases">minidumper's
releases</a>.</em></p>
<blockquote>
<h2>minidumper-0.8.1</h2>
<h3>Changed</h3>
<ul>
<li><a
href="https://redirect.github.com/EmbarkStudios/crash-handling/pull/81">PR#81</a>
resolved <a
href="https://redirect.github.com/EmbarkStudios/crash-handling/issues/80">#80</a>
by updating <code>polling</code> to 0.3.</li>
</ul>
</blockquote>
</details>
<details>
<summary>Commits</summary>
<ul>
<li><a
href="3b77c9b00d"><code>3b77c9b</code></a>
chore: Release</li>
<li><a
href="d34d00bc51"><code>d34d00b</code></a>
chore: Release</li>
<li><a
href="789c99498c"><code>789c994</code></a>
Update CHANGELOGs</li>
<li><a
href="addf1486f8"><code>addf148</code></a>
Update (<a
href="https://redirect.github.com/EmbarkStudios/crash-handling/issues/81">#81</a>)</li>
<li><a
href="16c2545f2a"><code>16c2545</code></a>
chore: Release</li>
<li><a
href="955629bab9"><code>955629b</code></a>
Update CHANGELOG</li>
<li><a
href="5e907ff389"><code>5e907ff</code></a>
Add Android support for the i686 and x86-64 targets (<a
href="https://redirect.github.com/EmbarkStudios/crash-handling/issues/76">#76</a>)</li>
<li><a
href="14bba1b81e"><code>14bba1b</code></a>
Fix using <code>crash-handler</code> under Miri (<a
href="https://redirect.github.com/EmbarkStudios/crash-handling/issues/75">#75</a>)</li>
<li>See full diff in <a
href="https://github.com/EmbarkStudios/crash-handling/compare/minidumper-0.8.0...minidumper-0.8.1">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 <dependency name> major version` will close this
group update PR and stop Dependabot creating any more for the specific
dependency's major version (unless you unignore this specific
dependency's major version or upgrade to it yourself)
- `@dependabot ignore <dependency name> minor version` will close this
group update PR and stop Dependabot creating any more for the specific
dependency's minor version (unless you unignore this specific
dependency's minor version or upgrade to it yourself)
- `@dependabot ignore <dependency name>` will close this group update PR
and stop Dependabot creating any more for the specific dependency
(unless you unignore this specific dependency or upgrade to it yourself)
- `@dependabot unignore <dependency name>` will remove all of the ignore
conditions of the specified dependency
- `@dependabot unignore <dependency name> <ignore condition>` will
remove the ignore condition of the specified dependency and ignore
conditions
</details>
Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Closes#3519
The actor name is saved in plain text in our AppData folder.
I did simple manual tests on my dev laptop:
- Restart the app, it reloads the actor name from disk
- Sign out of the app, the file is deleted from the disk
Caveats:
- If you un-install the app while signed in, the actor name will still
be there until something cleans up AppData. I don't think the MSI cleans
it up.
- If the token is present but the actor name is missing, I just treat it
as if the token was missing and make the app be signed-out, since this
will happen on dev systems until this PR has been merged for some time.
- If we try to delete the file and the file is NotFound, I just ignore
it.
The user is already passing us a mutable buffer so we might as well give
them a `MutableIpPacket` to allow them to further mutate it.
Extracted out of #3391.
Currently, we retry STUN bindings at a fixed interval if we don't
receive a response. This results in very noise logs if we attempt to
contact an IPv6 STUN server but don't have an IPv6 interface.
