Currently, we have two structs for representing IP packets: `IpPacket`
and `MutableIpPacket`. As the name suggests, they mostly differ in
mutability. This design was originally inspired by the `pnet_packet`
crate which we based our `IpPacket` on. With subsequent iterations, we
added more and more functionality onto our `IpPacket`, like NAT64 &
NAT46 translation. As a result of that, the `MutableIpPacket` is no
longer directly based on `pnet_packet` but instead just keeps an
internal buffer.
This duplication can be resolved by merging the two structs into a
single `IpPacket`. We do this by first replacing all usages of
`IpPacket` with `MutableIpPacket`, deleting `IpPacket` and renaming
`MutableIpPacket` to `IpPacket`. The final design now has different
`self`-receivers: Some functions take `&self`, some `&mut self` and some
consume the packet using `self`.
This results in a more ergonomic usage of `IpPacket` across the codebase
and deletes a fair bit of code. It also takes us one step closer towards
using `etherparse` for all our IP packet interaction-needs. Lastly, I am
currently exploring a performance-optimisation idea that stack-allocates
all IP packets and for that, the current split between `IpPacket` and
`MutableIpPacket` does not really work.
Related: #6366.
This PR introduces the `etherparse` dependency for parsing and
generating IP packets.
Using `etherparse`, we can implement the NAT46 & NAT64 implementations
for the gateway more elegantly because it allows us to parse the IP and
protocol headers into a static and much richer representation. The
conversion to the IPv4/IPv6 equivalent is then just a question of
transforming one data structure into another and writing it to the
correct place in the buffer.
We extract this functionality into dedicated `nat64` and `nat46`
modules.
Furthermore, we implement the various functions in `ip_packet::make`
using `etherparse` too. Following that, we also overhaul the NAT
translation tests that we have in `ip_packet::proptests`. Those now use
the more low-level `consume_to_ipX` APIs which makes the tests more
ergonomic to write.
In the future, we should upstream `Ipv4HeaderSliceMut` and
`Ipv6HeaderSliceMut` to `etherparse`.
Moving all of this functionality to `etherparse` will make it easier to
write tests that involve more IP packets as well as customise the
behaviour of our NAT.
Related: #5614.
Related: #6371.
Related: #6353.
Bumps [gradle/actions](https://github.com/gradle/actions) from 3 to 4.
<details>
<summary>Release notes</summary>
<p><em>Sourced from <a
href="https://github.com/gradle/actions/releases">gradle/actions's
releases</a>.</em></p>
<blockquote>
<h2>v4.0.0</h2>
<p>Final release of <code>v4.0.0</code> of the
<code>setup-gradle</code>, <code>dependency-submission</code> and
<code>wrapper-validation</code> actions provided under
<code>gradle/actions</code>.
This release is available under the <code>v4</code> tag.</p>
<h2>Major changes from the <code>v3</code> release</h2>
<h3>The <code>arguments</code> parameter has been removed</h3>
<p>Using the action to execute Gradle via the <code>arguments
</code>parameter was deprecated in <code>v3</code> and this parameter
has been removed.
<a
href="https://github.com/gradle/actions/blob/v4.0.0-rc.1/docs/deprecation-upgrade-guide.md#using-the-action-to-execute-gradle-via-the-arguments-parameter-is-deprecated">See
here for more details</a>.</p>
<h3>Cache cleanup enabled by default</h3>
<p>After a number of fixes and improvements, this release enables <a
href="https://github.com/gradle/actions/blob/v4.0.0-rc.1/docs/setup-gradle.md#configuring-cache-cleanup">cache-cleanup</a>
by default for all Jobs using the <code>setup-gradle</code> and
<code>dependency-submission</code> actions.</p>
<p>Improvements and bugfixes related cache cleanup:</p>
<ul>
<li>By default, cache cleanup is not run if any Gradle build fails (<a
href="https://redirect.github.com/gradle/actions/issues/71">#71</a>)</li>
<li>Cache cleanup is not run after configuration-cache reuse (<a
href="https://redirect.github.com/gradle/actions/issues/19">#19</a>)</li>
</ul>
<p>This feature should help to minimize the size of entries written to
the GitHub Actions cache, speeding up builds and reducing cache
usage.</p>
<h3>Wrapper validation enabled by default</h3>
<p>In <code>v3</code>, the <code>setup-gradle</code> action was enhanced
to support Gradle wrapper validation, removing the need to use a
separate workflow
file with the <code>gradle/actions/wrapper-validation</code> action.</p>
<p>With this release, wrapper validation has been significantly
improved, and is now enabled by default (<a
href="https://redirect.github.com/gradle/actions/issues/12">#12</a>):</p>
<ul>
<li>The <code>allow-snapshot-wrappers</code> makes it possible to
validate snapshot wrapper jars using <code>setup-gradle</code>.</li>
<li>Checksums for <a
href="https://services.gradle.org/distributions-snapshots/">nightly and
snapshot Gradle versions</a> are now validated (<a
href="https://redirect.github.com/gradle/actions/issues/281">#281</a>).</li>
<li>Valid wrapper checksums are cached in Gradle User Home, reducing the
need to retrieve checksum values remotely (<a
href="https://redirect.github.com/gradle/actions/issues/172">#172</a>).</li>
<li>Reduce network calls in <code>wrapper-validation</code> for new
Gradle versions: By only fetching wrapper checksums for Gradle versions
that were not known when this action was released, this release reduces
the likelihood that a network failure could cause failure in wrapper
validation (<a
href="https://redirect.github.com/gradle/actions/issues/171">#171</a>)</li>
<li>Improved error message when <code>wrapper-validation</code> finds no
wrapper jars (<a
href="https://redirect.github.com/gradle/actions/issues/284">#284</a>)</li>
</ul>
<p>Wrapper validation is important for supply-chain integrity. Enabling
this feature by default will increase the coverage of wrapper
validation on projects using GitHub Actions.</p>
<h3>New input parameters for Dependency Graph generation</h3>
<p>Some dependency-graph inputs that could previously only be configured
via environment variables now have dedicated action inputs:</p>
<ul>
<li><code>dependency-graph-report-dir</code>: sets the location where
dependency-graph reports will be generated</li>
<li><code>dependency-graph-exclude-projects</code> and
<code>dependency-graph-include-projects</code>: <a
href="https://github.com/gradle/actions/blob/v4.0.0-rc.1/docs/dependency-submission.md#selecting-gradle-projects-that-will-contribute-to-the-dependency-graph">select
which Gradle projects will contribute to the generated dependency
graph</a>.</li>
<li><code>dependency-graph-exclude-configurations</code> and
<code>dependency-graph-include-configurations</code>: <a
href="https://github.com/gradle/actions/blob/v4.0.0-rc.1/docs/dependency-submission.md#selecting-gradle-configurations-that-will-contribute-to-the-dependency-graph">select
which Gradle configurations will contribute to the generated dependency
graph</a>.</li>
</ul>
<h3>Other improvements</h3>
<ul>
<li>In Job summary, the action now provides an explanation when cache is
set to <code>read-only</code> or <code>disabled</code> (<a
href="https://redirect.github.com/gradle/actions/issues/255">#255</a>)</li>
<li>When <code>setup-gradle</code> requests a specific Gradle version,
the action will no longer download and install that version if it is
already available on the <code>PATH</code> of the runner (<a
href="https://redirect.github.com/gradle/actions/issues/270">#270</a>)</li>
<li>To attempt to speed up builds, the <code>setup-gradle</code> and
<code>dependency-submission</code> actions now attempt to use the
<code>D:</code> drive for Gradle User Home if it is available (<a
href="https://redirect.github.com/gradle/actions/issues/290">#290</a>)</li>
</ul>
<h2>Deprecations and breaking changes</h2>
<!-- raw HTML omitted -->
</blockquote>
<p>... (truncated)</p>
</details>
<details>
<summary>Commits</summary>
<ul>
<li><a
href="16bf8bc8fe"><code>16bf8bc</code></a>
Rework docs for Develocity support</li>
<li><a
href="faf4eeacd5"><code>faf4eea</code></a>
[bot] Update dist directory</li>
<li><a
href="4b7cc6e174"><code>4b7cc6e</code></a>
Differentiate Gradle 8.1 from 8.10 when checking version (<a
href="https://redirect.github.com/gradle/actions/issues/358">#358</a>)</li>
<li><a
href="0873530e60"><code>0873530</code></a>
Increase Gradle version coverage for init-scripts</li>
<li><a
href="f67327f0c8"><code>f67327f</code></a>
[bot] Update dist directory</li>
<li><a
href="d32a10b3ae"><code>d32a10b</code></a>
Dependency updates (<a
href="https://redirect.github.com/gradle/actions/issues/356">#356</a>)</li>
<li><a
href="e598a32529"><code>e598a32</code></a>
Quote version 8.10 in integ test</li>
<li><a
href="d6c8cf816c"><code>d6c8cf8</code></a>
Bump unzip-stream from 0.3.1 to 0.3.4 in /sources</li>
<li><a
href="79ea5b8f3e"><code>79ea5b8</code></a>
Bump org.junit.jupiter:junit-jupiter</li>
<li><a
href="d77a030aaf"><code>d77a030</code></a>
Bump com.google.guava:guava in /.github/workflow-samples/kotlin-dsl</li>
<li>Additional commits viewable in <a
href="https://github.com/gradle/actions/compare/v3...v4">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>
This publishes the 1.3.0 clients and gateways so that Internet Resources
will work.
The feature is still disabled for the Stripe plans until we publish the
launch post. Select customers have the feature enabled.
Closes#2667
- No known issues from the knowledge base were fixed
- I confirmed on the Windows laptop that the fix for #6469 is in this
MSI.
- The changelog looks good in the Vercel preview
---------
Signed-off-by: Reactor Scram <ReactorScram@users.noreply.github.com>
This will always build images we can use for last-minute compatibility
tests, even if the merge group was bypassed.
---------
Signed-off-by: Jamil <jamilbk@users.noreply.github.com>
Currently, the gateway requires a strict ordering of first receiving a
`request_connection` message, following by multiple `allow_access`
messages. Additionally, access can be granted as part of the initial
`request_connection` message too.
This isn't an ideal design. Setting up a new connection is infallible,
all we need to do is send our ICE credentials back to the client.
However, untangling that will require a bit more effort.
Starting with #6335, following this strict order on the client is a more
difficult. Whilst we can send them in order, it is harder to maintain
those ordering guarantees across all our systems.
To avoid this, we change the gateway to perform an upsert for its local
ACLs for a client. In case that an `allow_access` call would somehow get
to the gateway earlier, we can simply already create the `Peer` and only
set up the actual connection later.
---------
Signed-off-by: Jamil <jamilbk@users.noreply.github.com>
Co-authored-by: Jamil <jamilbk@users.noreply.github.com>
`install-action` uses `cargo-binstall` as a fallback. That binary
contacts GitHub which may run into rate-limit without being
authenticated. In that case, we will install manually which takes very
long.
Resolves: #6374.
When `snownet` was first being developed, these tests ensured that
hole-punching as well as connectivity via a relayed works correctly. We
have since added extensive tests that ensure connectivity works in many
scenarios via `tunnel_test`. `tunnel_test` does not (yet) have a
simulated NAT so hole-punching itself is not covered by that.
UDP hole-punching is shockingly trivial though because all you need to
do is send UDP packets to the same socket that the other party is
sending from. This isn't done by our own code but rather by str0m's
implement of ICE (as long as we add the correct candidates).
The `snownet-tests` themselves are quite fragile because they need to
set up their own event loop and manually construct an IP packet. They
haven't caught a single bug to my knowledge so I am proposing to delete
them for ease of maintenance.
For example, in
https://github.com/firezone/firezone/actions/runs/10449965474/job/28948590058?pr=6335
the tests fail because we no longer directly force a handshake when the
connection is established. This is unnecessary now because the buffered
intent packet will directly force a handshake from the client to the
gateway. Yet, `snownet-tests` event loop would need adjusting to also do
that.
It happened in the past that we screwed up the `preconditions` of the
state machine test such that no more transitions were sampled that
actually send packets. To protect against this, we use the newly
introduced logs and grep for certain transitions.
In the future, we can consider emitting a more structured output, like
writing all testcases to a DB and run more complex queries against it to
ensure that certain cases are covered.
For quite a while now, we have been making extensive use of
property-based testing to ensure `connlib` works as intended. The idea
of proptests is that - given a certain seed - we deterministically
sample test inputs and assert properties on a given function.
If the test fails, `proptest` prints the seed which can then be added to
a regressions file to iterate on the test case and fix it. It is quite
obvious that non-determinism in how the test input gets generated is no
bueno and reduces the value we get out of these tests a fair bit.
The `HashMap` and `HashSet` data structures are known to be
non-deterministic in their iteration order. This causes non-determinism
during the input generation because we make use of a lot of maps and
sets to gradually build up the test input. We fix all uses of `HashMap`
and `HashSet` by replacing them with `BTreeMap` and `BTreeSet`.
To ensure this doesn't regress, we refactor `tunnel_test` to not make
use of proptest's macros and instead, we initialise and run the test
ourselves. This allows us to dump the sampled state and transitions into
a file per test run. In CI, we then run a 2nd iteration of all
regression tests and compare the sampled state and transitions with the
previous run. They must match byte-for-byte.
Finally, to discourage use of non-deterministic iteration, we ban the
use of the iteration functions on `HashMap` and `HashSet` across the
codebase. This doesn't catch iteration in a `for`-loop but it is better
than not linting against it at all.
---------
Signed-off-by: Thomas Eizinger <thomas@eizinger.io>
Co-authored-by: Reactor Scram <ReactorScram@users.noreply.github.com>
Currently, `connlib` can only handle "simple" DNS wildcards where `*`
matches any number of subdomains, including zero and `?` matches a
single subdomain.
With this PR, we expand `connlib'`s capabilities to allow for a much
more complex matching of domains that more closely resembles glob
patterns:
- `**` matches any number of subdomains. This supersedes the previous
`*` operator.
- `*` matches a single subdomain. This supersedes the previous `?`
operator.
- `?` matches a single character. This wasn't possible before.
- Additionally, any of these can be combined. Previously, only `*` or
`?` was allowed and they were only accepted at the front of the domain
name pattern.
Resolves: #5056.
---------
Signed-off-by: Thomas Eizinger <thomas@eizinger.io>
- Adds `http_test_server_image` to inputs so that it gets set properly
for CI (`debug`) and CD (`perf`)
- Updates `dev` -> `debug` in docker-compose.yml to fix pulls
- Fixes issue with seeds and relevant docs from #6205
The compose service I defined is called `otel` not `otlp`. With this fix
in place, the relay successfully connects to the OTLP exporter.
it is worthwhile noting that the connection to the OTLP exporter itself
is not critical for relay operation. Even if it fails, it won't affect
the actual data plane. I do think it makes sense to still have a working
OTLP exporter in the compose definition. As it makes it easier to test
whether the ingestion of metrics and traces works as expected.
It appears that the configuration via env variables doesn't work as
expected. This PR changes bencher's config to use commandline arguments.
With that, the `--branch-start-point` actually takes effect and copies
over the thresholds configured on bencher for the `main` branch.
With the thresholds in place, we can configure bencher to only alert us
if a threshold is exceeded and otherwise be quiet and not post a
comment.
Currently, `tunnel_test` executes all actions within the same `Instant`,
i.e. time is never advanced by itself. The difficulty with advancing
time compared to other actions like sending packets is that all
time-related actions "overlap". In other words, all timers within
connlib advance at the same time. This makes it difficult to model the
expected behaviour after a certain amount of time has passed as we'd
effectively need to model all timers and their relation to particular
actions (like resending of connection intents or STUN requests).
Instead of only advancing time by itself, we can model some aspect of it
by introducing latency on network messages. This allows us to define a
range of an "acceptable" network latency within everything is expected
to work.
Whilst this doesn't cover all failure cases, it gives us a solid
foundation of parameters within which we should not expect any
operational problems.
Currently, we have a homegrown benchmark suite that reports results of
the iperf runs within CI by comparing a run on `main` with the current
branch.
These comments are noisy because they happen on every PR, regardless of
the performance results. As a result, they tend to be skimmed over by
devs and not actually considered. To properly track performance, we need
to record benchmark results over time and use statistics to detect
regressions.
https://bencher.dev does exactly that. it supports various benchmark
harnesses to automatically collect benchmarks. For our case, we simply
use the generic JSON adapter to extract the relevant metrics from the
iperf results and report them to the bencher backend.
With these metrics in place, bencher can plot the results over time, and
alert us in the case of regressions using thresholds based on
statistical tests.
Resolves: #5818.
---------
Signed-off-by: Thomas Eizinger <thomas@eizinger.io>
Co-authored-by: Reactor Scram <ReactorScram@users.noreply.github.com>
This version was a few months old and started throwing errors about
features that stabilized since then.
e.g.
https://github.com/firezone/firezone/actions/runs/10011089436/job/27673759249
```
error[E0658]: use of unstable library feature 'proc_macro_byte_character'
--> /home/runner/.cargo/registry/src/index.crates.io-6f17d22bba15001f/proc-macro2-1.0.86/src/wrapper.rs:871:21
|
871 | proc_macro::Literal::byte_character(byte)
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: see issue #115268 <https://github.com/rust-lang/rust/issues/115268> for more information
= help: add `#![feature(proc_macro_byte_character)]` to the crate attributes to enable
= note: this compiler was built on 2024-03-25; consider upgrading it if it is out of date
error[E0658]: use of unstable library feature 'proc_macro_c_str_literals'
--> /home/runner/.cargo/registry/src/index.crates.io-6f17d22bba15001f/proc-macro2-1.0.86/src/wrapper.rs:898:21
|
898 | proc_macro::Literal::c_string(string)
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: see issue #119750 <https://github.com/rust-lang/rust/issues/119750> for more information
= help: add `#![feature(proc_macro_c_str_literals)]` to the crate attributes to enable
= note: this compiler was built on 2024-03-25; consider upgrading it if it is out of date
For more information about this error, try `rustc --explain E0658`.
error: could not compile `proc-macro2` (lib) due to 2 previous errors
```
Our Rust CI runs various jobs in different configurations of packages
and / or features. Currently, only the clippy job denies warnings which
makes it possible that some code still generates warnings under
particular configurations.
To ensure we always fail on warnings, we set a global env var to deny
warnings for all Rust CI jobs.
---------
Signed-off-by: Thomas Eizinger <thomas@eizinger.io>
Co-authored-by: Reactor Scram <ReactorScram@users.noreply.github.com>
I started a playbook for publishing GUI releases, I didn't see any other
one around.
I think there's a middle step I'm not clear on:
1. Open this PR and get it approved
2. Do something? Publish the draft release maybe? Run a special CI
workflow?
3. Merge this PR to update the changelog and bump the versions in Git
```[tasklist]
### Tasks
```
Closes#5810
```[tasklist]
### Tasks
- [x] Try not to set the icon every time we change Resources
- [x] Get production icons
- [x] Add changelog comment
- [x] Add CI stress test that sets the icon 10,000 times
- [x] Open for review
- [x] Repair changelog
- [ ] Merge
```
---------
Signed-off-by: Reactor Scram <ReactorScram@users.noreply.github.com>
