From 0fbd40fcb21747d49bdc33c1848bca54f45aa4e9 Mon Sep 17 00:00:00 2001 From: Reactor Scram Date: Thu, 15 Feb 2024 14:12:37 -0600 Subject: [PATCH] feat(linux): Notify systemd when we've started (#3628) 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 Co-authored-by: Thomas Eizinger --- .github/workflows/ci.yml | 1 + rust/Cargo.lock | 7 ++++ rust/connlib/tunnel/Cargo.toml | 1 + .../tunnel/src/device_channel/tun_linux.rs | 14 +++++++ scripts/tests/systemd/dns-systemd-resolved.sh | 39 +++++++++++++++++++ scripts/tests/systemd/firezone-client.service | 20 ++++++++++ 6 files changed, 82 insertions(+) create mode 100755 scripts/tests/systemd/dns-systemd-resolved.sh create mode 100644 scripts/tests/systemd/firezone-client.service diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7ae9eb07d..64fdaf367 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -192,6 +192,7 @@ jobs: direct-ping-portal-relay-down, dns-etc-resolvconf, dns-nm, + systemd/dns-systemd-resolved, ] steps: - uses: actions/checkout@v4 diff --git a/rust/Cargo.lock b/rust/Cargo.lock index 59e225b89..c421faca7 100644 --- a/rust/Cargo.lock +++ b/rust/Cargo.lock @@ -2126,6 +2126,7 @@ dependencies = [ "rand_core 0.6.4", "resolv-conf", "rtnetlink", + "sd-notify", "secrecy", "serde", "serde_json", @@ -5654,6 +5655,12 @@ dependencies = [ "thiserror", ] +[[package]] +name = "sd-notify" +version = "0.4.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "621e3680f3e07db4c9c2c3fb07c6223ab2fab2e54bd3c04c3ae037990f428c32" + [[package]] name = "sdp" version = "0.6.0" diff --git a/rust/connlib/tunnel/Cargo.toml b/rust/connlib/tunnel/Cargo.toml index ce956d737..1813c312f 100644 --- a/rust/connlib/tunnel/Cargo.toml +++ b/rust/connlib/tunnel/Cargo.toml @@ -48,6 +48,7 @@ tempfile = "3.10.0" netlink-packet-route = { version = "0.17", default-features = false } netlink-packet-core = { version = "0.7", default-features = false } rtnetlink = { version = "0.13" } +sd-notify = "0.4.1" # Android tunnel dependencies [target.'cfg(target_os = "android")'.dependencies] diff --git a/rust/connlib/tunnel/src/device_channel/tun_linux.rs b/rust/connlib/tunnel/src/device_channel/tun_linux.rs index dfe47d56b..58525a489 100644 --- a/rust/connlib/tunnel/src/device_channel/tun_linux.rs +++ b/rust/connlib/tunnel/src/device_channel/tun_linux.rs @@ -101,6 +101,7 @@ impl Tun { ) -> Result { // TODO: Tech debt: // TODO: Gateways shouldn't set up DNS, right? Only clients? + // TODO: Move this configuration up to the client let dns_control_method = connlib_shared::linux::get_dns_control_from_env(); tracing::info!(?dns_control_method); @@ -259,6 +260,18 @@ async fn set_iface_config( Some(DnsControlMethod::Systemd) => configure_systemd_resolved(&dns_config).await?, } + // TODO: Having this inside the library is definitely wrong. I think `set_iface_config` + // needs to return before `new` returns, so that the `on_tunnel_ready` callback + // happens after the IP address and DNS are set up. Then we can call `sd_notify` + // inside `on_tunnel_ready` in the client. + // + // `sd_notify::notify` is always safe to call, it silently returns `Ok(())` + // if we aren't running as a systemd service. + if let Err(error) = sd_notify::notify(true, &[sd_notify::NotifyState::Ready]) { + // Nothing we can do about it + tracing::warn!(?error, "Failed to notify systemd that we're ready"); + } + Ok(()) } @@ -343,6 +356,7 @@ async fn configure_network_manager(_dns_config: &[IpAddr]) -> Result<()> { } async fn configure_systemd_resolved(_dns_config: &[IpAddr]) -> Result<()> { + // TODO: Call `resolvectl` here Err(Error::Other( "DNS control with `systemd-resolved` is not implemented yet", )) diff --git a/scripts/tests/systemd/dns-systemd-resolved.sh b/scripts/tests/systemd/dns-systemd-resolved.sh new file mode 100755 index 000000000..6401eb02d --- /dev/null +++ b/scripts/tests/systemd/dns-systemd-resolved.sh @@ -0,0 +1,39 @@ +#!/usr/bin/env bash +# Test Linux DNS control using `systemd-resolved` directly inside the CI runner + +set -euo pipefail + +BINARY_NAME=firezone-linux-client + +docker compose exec client cat firezone-linux-client > "$BINARY_NAME" +chmod u+x "$BINARY_NAME" +sudo mv "$BINARY_NAME" "/usr/bin/$BINARY_NAME" +# TODO: Check whether this is redundant with the systemd service file +sudo setcap cap_net_admin+eip "/usr/bin/$BINARY_NAME" + +sudo cp scripts/tests/systemd/firezone-client.service /etc/systemd/system/ +systemd-analyze security firezone-client + +# TODO: Use DNS and not IP +# HTTPBIN_DNS=172.21.0.100 +HTTPBIN_IP=172.20.0.100 + +IFACE_NAME="tun-firezone" + +echo "# Accessing a resource should fail before the client is up" +# TODO: For now I'm cheating and forcing curl to try the tunnel iface. +# This doesn't test that Firezone is adding the routes. +# If I don't do this, curl just connects through the Docker bridge. +curl --interface "$IFACE_NAME" $HTTPBIN_IP/get && exit 1 + +echo "# Start Firezone" +resolvectl dns tun-firezone && exit 1 +if ! sudo systemctl start firezone-client +then + sudo systemctl status firezone-client + exit 1 +fi +resolvectl dns tun-firezone + +echo "# Accessing a resource should succeed after the client is up" +curl --interface "$IFACE_NAME" $HTTPBIN_IP/get diff --git a/scripts/tests/systemd/firezone-client.service b/scripts/tests/systemd/firezone-client.service new file mode 100644 index 000000000..42735ea68 --- /dev/null +++ b/scripts/tests/systemd/firezone-client.service @@ -0,0 +1,20 @@ +[Unit] +Description=Firezone Client + +[Service] +AmbientCapabilities=CAP_NET_ADMIN + +Environment="FIREZONE_API_URL=ws://localhost:8081" +# Will re-enable this once systemd DNS control is ready for CI +# Environment="FIREZONE_DNS_CONTROL=systemd-resolved" +Environment="FIREZONE_ID=D0455FDE-8F65-4960-A778-B934E4E85A5F" +Environment="FIREZONE_TOKEN=n.SFMyNTY.g2gDaANtAAAAJGM4OWJjYzhjLTkzOTItNGRhZS1hNDBkLTg4OGFlZjZkMjhlMG0AAAAkN2RhN2QxY2QtMTExYy00NGE3LWI1YWMtNDAyN2I5ZDIzMGU1bQAAACtBaUl5XzZwQmstV0xlUkFQenprQ0ZYTnFJWktXQnMyRGR3XzJ2Z0lRdkZnbgYAGUmu74wBYgABUYA.UN3vSLLcAMkHeEh5VHumPOutkuue8JA6wlxM9JxJEPE" +Environment="RUST_LOG=firezone_linux_client=trace,wire=trace,connlib_client_shared=trace,firezone_tunnel=trace,connlib_shared=trace,warn" + +ExecStart=firezone-linux-client +Type=notify +# TODO: Come back to this and cut down the permissions +User=root + +[Install] +WantedBy=default.target