github-personal/firezone
1
0
Fork 0
mirror of https://github.com/outbackdingo/firezone.git synced 2026-01-27 10:18:54 +00:00
Code Issues Packages Projects Releases Wiki Activity

chore(rust): ban the use of .unwrap except in tests (#7319)

Browse Source 
Using the clippy lint `unwrap_used`, we can automatically lint against
all uses of `.unwrap()` on `Result` and `Option`. This turns up quite a
few results actually. In most cases, they are invariants that can't
actually be hit. For these, we change them to `Option`. In other cases,
they can actually be hit. For example, if the user supplies an invalid
log-filter.

Activating this lint ensures the compiler will yell at us every time we
use `.unwrap` to double-check whether we do indeed want to panic here.

Resolves: #7292.
This commit is contained in:
Thomas Eizinger
2024-11-13 03:59:22 +00:00
committed by GitHub
parent b230cf79fd
commit 48ba2869a8
38 changed files with 318 additions and 155 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
rust/Cargo.lock generated
View File

@@ -2175,6 +2175,7 @@ dependencies = [
"socket2",
"stun_codec",
"test-strategy",
"thiserror",
"tokio",
"tracing",
"tracing-core",

1
rust/Cargo.toml
View File

@@ -71,6 +71,7 @@ wildcard_enum_match_arm = "warn" # Ensures we match on all combinations of `Poll
redundant_else = "warn"
redundant_clone = "warn"
unwrap_in_result = "warn"
unwrap_used = "warn"
[workspace.lints.rustdoc]
private-intra-doc-links = "allow" # We don't publish any of our docs but want to catch dead links.

2
rust/bin-shared/Cargo.toml
View File

@@ -11,6 +11,7 @@ axum = { version = "0.7.7", default-features = false, features = ["http1", "toki
clap = { version = "4.5.19", features = ["derive", "env"] }
firezone-logging = { workspace = true }
futures = "0.3"
hex-literal = "0.4.1"
ip_network = { version = "0.4", default-features = false, features = ["serde"] }
socket-factory = { workspace = true }
thiserror = "1.0.68"
@@ -19,7 +20,6 @@ tracing = { workspace = true }
tun = { workspace = true }
[dev-dependencies]
hex-literal = "0.4.1"
tokio = { workspace = true, features = ["macros", "rt-multi-thread"] }
[target.'cfg(target_os = "linux")'.dependencies]

2
rust/bin-shared/benches/tunnel.rs
View File

@@ -1,3 +1,5 @@
#![allow(clippy::unwrap_used)]
use anyhow::Result;
fn main() -> Result<()> {

2
rust/bin-shared/src/lib.rs
View File

@@ -1,3 +1,5 @@
#![cfg_attr(test, allow(clippy::unwrap_used))]
pub mod http_health_check;
mod network_changes;

8
rust/bin-shared/src/network_changes/windows.rs
View File

@@ -64,6 +64,7 @@
use crate::platform::DnsControlMethod;
use anyhow::{anyhow, Context as _, Result};
use firezone_logging::anyhow_dyn_err;
use std::thread;
use tokio::sync::{
mpsc::{self, error::TrySendError},
@@ -257,7 +258,12 @@ impl<'a> Drop for Listener<'a> {
// we crash the GUI process before we can get back to the main thread
// and drop the DNS listeners
fn drop(&mut self) {
self.close_dont_drop().unwrap();
if let Err(e) = self.close_dont_drop() {
tracing::error!(
error = anyhow_dyn_err(&e),
"Failed to close `Listener` gracefully"
);
}
}
}

35
rust/bin-shared/src/tun_device_manager/windows.rs
View File

@@ -1,7 +1,7 @@
use crate::windows::{CREATE_NO_WINDOW, TUNNEL_UUID};
use crate::TUNNEL_NAME;
use anyhow::{Context as _, Result};
use firezone_logging::std_dyn_err;
use firezone_logging::{anyhow_dyn_err, std_dyn_err};
use ip_network::{IpNetwork, Ipv4Network, Ipv6Network};
use ring::digest;
use std::{
@@ -426,7 +426,19 @@ fn dll_already_exists(path: &Path, dll_bytes: &DllBytes) -> bool {
Ok(x) => x,
};
let actual_len = usize::try_from(f.metadata().unwrap().len()).unwrap();
let actual_len = match file_length(&f) {
Err(e) => {
tracing::warn!(
error = anyhow_dyn_err(&e),
path = %path.display(),
"Failed to get file length"
);
return false;
}
Ok(l) => l,
};
let expected_len = dll_bytes.bytes.len();
// If the dll is 100 MB instead of 0.5 MB, this allows us to skip a 100 MB read
if actual_len != expected_len {
@@ -438,11 +450,18 @@ fn dll_already_exists(path: &Path, dll_bytes: &DllBytes) -> bool {
return false;
}
let expected = ring::test::from_hex(dll_bytes.expected_sha256).unwrap();
let expected = dll_bytes.expected_sha256;
let actual = digest::digest(&digest::SHA256, &buf);
expected == actual.as_ref()
}
fn file_length(f: &std::fs::File) -> Result<usize> {
let len = f.metadata().context("Failed to read metadata")?.len();
let len = usize::try_from(len).context("File length doesn't fit into usize")?;
Ok(len)
}
/// Returns the absolute path for installing and loading `wintun.dll`
///
/// e.g. `C:\Users\User\AppData\Local\dev.firezone.client\data\wintun.dll`
@@ -457,14 +476,16 @@ struct DllBytes {
/// Bytes embedded in the client with `include_bytes`
pub bytes: &'static [u8],
/// Expected SHA256 hash
pub expected_sha256: &'static str,
pub expected_sha256: [u8; 32],
}
#[cfg(target_arch = "x86_64")]
fn wintun_bytes() -> DllBytes {
DllBytes {
bytes: include_bytes!("../wintun/bin/amd64/wintun.dll"),
expected_sha256: "e5da8447dc2c320edc0fc52fa01885c103de8c118481f683643cacc3220dafce",
expected_sha256: hex_literal::hex!(
"e5da8447dc2c320edc0fc52fa01885c103de8c118481f683643cacc3220dafce"
),
}
}
@@ -472,6 +493,8 @@ fn wintun_bytes() -> DllBytes {
fn wintun_bytes() -> DllBytes {
DllBytes {
bytes: include_bytes!("../wintun/bin/arm64/wintun.dll"),
expected_sha256: "f7ba89005544be9d85231a9e0d5f23b2d15b3311667e2dad0debd344918a3f80",
expected_sha256: hex_literal::hex!(
"f7ba89005544be9d85231a9e0d5f23b2d15b3311667e2dad0debd344918a3f80"
),
}
}

2
rust/bin-shared/tests/network_notifiers.rs
View File

@@ -1,3 +1,5 @@
#![allow(clippy::unwrap_used)]
use firezone_bin_shared::{new_dns_notifier, new_network_notifier, platform::DnsControlMethod};
use futures::future::FutureExt as _;
use std::time::Duration;

9
rust/connlib/clients/android/src/lib.rs
View File

@@ -31,7 +31,7 @@ use std::{
use std::{sync::OnceLock, time::Duration};
use thiserror::Error;
use tokio::runtime::Runtime;
use tracing_subscriber::prelude::*;
use tracing_subscriber::{prelude::*, EnvFilter};
mod make_writer;
mod tun;
@@ -119,7 +119,7 @@ fn call_method(
.map_err(|source| CallbackError::CallMethodFailed { name, source })
}
fn init_logging(log_dir: &Path, log_filter: String) -> firezone_logging::file::Handle {
fn init_logging(log_dir: &Path, log_filter: EnvFilter) -> firezone_logging::file::Handle {
// On Android, logging state is persisted indefinitely after the System.loadLibrary
// call, which means that a disconnect and tunnel process restart will not
// reinitialize the guard. This is a problem because the guard remains tied to
@@ -151,7 +151,7 @@ fn init_logging(log_dir: &Path, log_filter: String) -> firezone_logging::file::H
)
.with_writer(make_writer::MakeWriter::new("connlib")),
)
.with(firezone_logging::filter(&log_filter))
.with(log_filter)
.try_init();
handle
@@ -324,6 +324,9 @@ fn connect(
let os_version = string_from_jstring!(env, os_version);
let log_dir = string_from_jstring!(env, log_dir);
let log_filter = string_from_jstring!(env, log_filter);
let log_filter =
firezone_logging::try_filter(&log_filter).context("Failed to parse log-filter")?;
let device_info = string_from_jstring!(env, device_info);
let device_info =
serde_json::from_str(&device_info).context("Failed to deserialize `DeviceInfo`")?;

49
rust/connlib/clients/apple/src/lib.rs
View File

@@ -10,6 +10,7 @@ use backoff::ExponentialBackoffBuilder;
use connlib_client_shared::{Callbacks, DisconnectError, Session, V4RouteList, V6RouteList};
use connlib_model::ResourceView;
use firezone_logging::anyhow_dyn_err;
use firezone_logging::std_dyn_err;
use firezone_telemetry::Telemetry;
use firezone_telemetry::APPLE_DSN;
use ip_network::{Ipv4Network, Ipv6Network};
@@ -24,7 +25,7 @@ use std::{
time::Duration,
};
use tokio::runtime::Runtime;
use tracing_subscriber::{prelude::*, util::TryInitError};
use tracing_subscriber::prelude::*;
use tun::Tun;
/// The Apple client implements reconnect logic in the upper layer using OS provided
@@ -125,26 +126,45 @@ impl Callbacks for CallbackHandler {
tunnel_address_v6: Ipv6Addr,
dns_addresses: Vec<IpAddr>,
) {
let dns_addresses = match serde_json::to_string(&dns_addresses) {
Ok(dns_addresses) => dns_addresses,
Err(e) => {
tracing::error!(error = std_dyn_err(&e), "Failed to serialize DNS addresses");
return;
}
};
self.inner.on_set_interface_config(
tunnel_address_v4.to_string(),
tunnel_address_v6.to_string(),
serde_json::to_string(&dns_addresses)
.expect("developer error: a list of ips should always be serializable"),
dns_addresses,
);
}
fn on_update_routes(&self, route_list_4: Vec<Ipv4Network>, route_list_6: Vec<Ipv6Network>) {
self.inner.on_update_routes(
serde_json::to_string(&V4RouteList::new(route_list_4)).unwrap(),
serde_json::to_string(&V6RouteList::new(route_list_6)).unwrap(),
);
match (
serde_json::to_string(&V4RouteList::new(route_list_4)),
serde_json::to_string(&V6RouteList::new(route_list_6)),
) {
(Ok(route_list_4), Ok(route_list_6)) => {
self.inner.on_update_routes(route_list_4, route_list_6);
}
(Err(e), _) | (_, Err(e)) => {
tracing::error!(error = std_dyn_err(&e), "Failed to serialize route list");
}
}
}
fn on_update_resources(&self, resource_list: Vec<ResourceView>) {
self.inner.on_update_resources(
serde_json::to_string(&resource_list)
.expect("developer error: failed to serialize resource list"),
);
let resource_list = match serde_json::to_string(&resource_list) {
Ok(resource_list) => resource_list,
Err(e) => {
tracing::error!(error = std_dyn_err(&e), "Failed to serialize resource list");
return;
}
};
self.inner.on_update_resources(resource_list);
}
fn on_disconnect(&self, error: &DisconnectError) {
@@ -152,10 +172,7 @@ impl Callbacks for CallbackHandler {
}
}
fn init_logging(
log_dir: PathBuf,
log_filter: String,
) -> Result<firezone_logging::file::Handle, TryInitError> {
fn init_logging(log_dir: PathBuf, log_filter: String) -> Result<firezone_logging::file::Handle> {
let (file_layer, handle) = firezone_logging::file::layer(&log_dir);
tracing_subscriber::registry()
@@ -173,7 +190,7 @@ fn init_logging(
)),
)
.with(file_layer)
.with(firezone_logging::filter(&log_filter))
.with(firezone_logging::try_filter(&log_filter).context("Failed to parse log-filter")?)
.try_init()?;
Ok(handle)

2
rust/connlib/model/src/lib.rs
View File

@@ -3,6 +3,8 @@
//! This includes types provided by external crates, i.e. [boringtun] to make sure that
//! we are using the same version across our own crates.
#![cfg_attr(test, allow(clippy::unwrap_used))]
mod view;
pub use boringtun::x25519::PublicKey;

5
rust/connlib/snownet/src/allocation.rs
View File

@@ -225,7 +225,8 @@ impl Allocation {
channel_bindings: Default::default(),
last_now: now,
buffered_channel_bindings: RingBuffer::new(100),
software: Software::new(format!("snownet; session={session_id}")).unwrap(),
software: Software::new(format!("snownet; session={session_id}"))
.expect("description has less then 128 chars"),
};
allocation.send_binding_requests();
@@ -1119,7 +1120,7 @@ fn make_channel_bind_request(
);
message.add_attribute(XorPeerAddress::new(target));
message.add_attribute(ChannelNumber::new(channel).unwrap());
message.add_attribute(ChannelNumber::new(channel).expect("channel number out of range")); // Panic is fine here, because we control the channel number within this module.
message.add_attribute(software);
message

1
rust/connlib/snownet/src/lib.rs
View File

@@ -1,6 +1,7 @@
//! A SANS-IO connectivity library for wireguard connections formed by ICE.
#![cfg_attr(test, allow(clippy::unwrap_in_result))]
#![cfg_attr(test, allow(clippy::unwrap_used))]
mod allocation;
mod backoff;

7
rust/connlib/tunnel/src/dns.rs
View File

@@ -34,8 +34,10 @@ pub(crate) const DNS_PORT: u16 = 53;
/// For Chrome and other Chrome-based browsers, this is not required as
/// Chrome will automatically disable DoH if your server(s) don't support
/// it. See <https://www.chromium.org/developers/dns-over-https/#faq>.
static DOH_CANARY_DOMAIN: LazyLock<DomainName> =
LazyLock::new(|| DomainName::vec_from_str("use-application-dns.net").unwrap());
static DOH_CANARY_DOMAIN: LazyLock<DomainName> = LazyLock::new(|| {
DomainName::vec_from_str("use-application-dns.net")
.expect("static domain name should always parse")
});
pub struct StubResolver {
fqdn_to_ips: HashMap<DomainName, Vec<IpAddr>>,
@@ -797,6 +799,7 @@ mod tests {
}
#[cfg(feature = "divan")]
#[allow(clippy::unwrap_used)]
mod benches {
use super::*;
use rand::{distributions::DistString, seq::IteratorRandom, Rng};

11
rust/connlib/tunnel/src/gateway.rs
View File

@@ -338,7 +338,7 @@ impl GatewayState {
dns_resource_nat::NatStatus::Inactive
});
let packet = dns_resource_nat::domain_status(req.resource, req.domain, nat_status);
let packet = dns_resource_nat::domain_status(req.resource, req.domain, nat_status)?;
let mut buffer = EncryptBuffer::new();
let Some(transmit) = encrypt_packet(packet, req.client, &mut self.node, &mut buffer, now)?
@@ -476,7 +476,14 @@ fn handle_p2p_control_packet(
req.resource,
req.domain,
dns_resource_nat::NatStatus::Inactive,
);
)
.inspect_err(|e| {
tracing::warn!(
error = anyhow_dyn_err(e),
"Failed to create `DomainStatus` packet"
)
})
.ok()?;
return Some(packet);
}

2
rust/connlib/tunnel/src/lib.rs
View File

@@ -3,6 +3,8 @@
//! This is both the wireguard and ICE implementation that should work in tandem.
//! [Tunnel] is the main entry-point for this crate.
#![cfg_attr(test, allow(clippy::unwrap_used))]
use crate::messages::{Offer, ResolveRequest, SecretKey};
use bimap::BiMap;
use chrono::Utc;

34
rust/connlib/tunnel/src/p2p_control.rs
View File

@@ -19,7 +19,7 @@ pub const DOMAIN_STATUS_EVENT: FzP2pEventType = FzP2pEventType::new(1);
#[cfg_attr(not(test), expect(dead_code, reason = "Will be used soon."))]
pub mod dns_resource_nat {
use super::*;
use anyhow::{Context, Result};
use anyhow::{Context as _, Result};
use connlib_model::{DomainName, ResourceId};
use ip_packet::{FzP2pControlSlice, IpPacket};
use std::net::IpAddr;
@@ -29,37 +29,45 @@ pub mod dns_resource_nat {
resource: ResourceId,
domain: DomainName,
proxy_ips: Vec<IpAddr>,
) -> IpPacket {
debug_assert_eq!(proxy_ips.len(), 8);
) -> Result<IpPacket> {
anyhow::ensure!(proxy_ips.len() == 8, "Expected 8 proxy IPs");
let payload = serde_json::to_vec(&AssignedIps {
resource,
domain,
proxy_ips,
})
.unwrap();
.context("Failed to serialize `AssignedIps` event")?;
ip_packet::make::fz_p2p_control(
let ip_packet = ip_packet::make::fz_p2p_control(
[ASSIGNED_IPS_EVENT.into_u8(), 0, 0, 0, 0, 0, 0, 0],
&payload,
)
.expect("with only 8 proxy IPs, payload should be less than max packet size")
.context("Failed to create p2p control protocol packet")?;
Ok(ip_packet)
}
/// Construct a new [`DomainStatus`] event.
pub fn domain_status(resource: ResourceId, domain: DomainName, status: NatStatus) -> IpPacket {
pub fn domain_status(
resource: ResourceId,
domain: DomainName,
status: NatStatus,
) -> Result<IpPacket> {
let payload = serde_json::to_vec(&DomainStatus {
status,
resource,
domain,
})
.unwrap();
.context("Failed to serialize `DomainStatus` event")?;
ip_packet::make::fz_p2p_control(
let ip_packet = ip_packet::make::fz_p2p_control(
[DOMAIN_STATUS_EVENT.into_u8(), 0, 0, 0, 0, 0, 0, 0],
&payload,
)
.expect("payload is less than max packet size")
.context("Failed to create p2p control protocol packet")?;
Ok(ip_packet)
}
pub fn decode_assigned_ips(packet: FzP2pControlSlice) -> Result<AssignedIps> {
@@ -132,7 +140,8 @@ pub mod dns_resource_nat {
ResourceId::from_u128(101),
domain("example.com"),
eight_proxy_ips(),
);
)
.unwrap();
let slice = packet.as_fz_p2p_control().unwrap();
let assigned_ips = decode_assigned_ips(slice).unwrap();
@@ -148,7 +157,8 @@ pub mod dns_resource_nat {
ResourceId::from_u128(101),
domain("example.com"),
NatStatus::Active,
);
)
.unwrap();
let slice = packet.as_fz_p2p_control().unwrap();
let domain_status = decode_domain_status(slice).unwrap();

4
rust/gateway/src/main.rs
View File

@@ -60,7 +60,7 @@ async fn main() {
}
async fn try_main(cli: Cli, telemetry: &mut Telemetry) -> Result<()> {
firezone_logging::setup_global_subscriber(layer::Identity::default());
firezone_logging::setup_global_subscriber(layer::Identity::default())?;
let firezone_id = get_firezone_id(cli.firezone_id).await
.context("Couldn't read FIREZONE_ID or write it to disk: Please provide it through the env variable or provide rw access to /var/lib/firezone/")?;
@@ -109,7 +109,7 @@ async fn get_firezone_id(env_id: Option<String>) -> Result<String> {
}
let id_path = Path::new(ID_PATH);
tokio::fs::create_dir_all(id_path.parent().unwrap()).await?;
tokio::fs::create_dir_all(id_path.parent().context("Missing parent")?).await?;
let mut id_file = tokio::fs::File::create(id_path).await?;
let id = Uuid::new_v4().to_string();
id_file.write_all(id.as_bytes()).await?;

2
rust/gui-client/src-common/src/lib.rs
View File

@@ -1,3 +1,5 @@
#![cfg_attr(test, allow(clippy::unwrap_used))]
pub mod auth;
pub mod compositor;
pub mod controller;

4
rust/gui-client/src-common/src/settings.rs
View File

@@ -38,8 +38,8 @@ mod defaults {
impl Default for AdvancedSettings {
fn default() -> Self {
Self {
auth_base_url: Url::parse(defaults::AUTH_BASE_URL).unwrap(),
api_url: Url::parse(defaults::API_URL).unwrap(),
auth_base_url: Url::parse(defaults::AUTH_BASE_URL).expect("static URL is a valid URL"),
api_url: Url::parse(defaults::API_URL).expect("static URL is a valid URL"),
favorite_resources: Default::default(),
internet_resource_enabled: Default::default(),
log_filter: defaults::LOG_FILTER.to_string(),

2
rust/gui-client/src-tauri/src/client/debug_commands.rs
View File

@@ -34,7 +34,7 @@ pub fn run(cmd: Cmd) -> Result<()> {
fn set_autostart(enabled: bool) -> Result<()> {
firezone_headless_client::setup_stdout_logging()?;
let rt = tokio::runtime::Runtime::new().unwrap();
let rt = tokio::runtime::Runtime::new()?;
rt.block_on(crate::client::gui::set_autostart(enabled))?;
Ok(())
}

10
rust/gui-client/src-tauri/src/client/gui.rs
View File

@@ -150,7 +150,9 @@ pub(crate) fn run(
// Per https://tauri.app/v1/guides/features/system-tray/#preventing-the-app-from-closing
// Closing the window fully seems to deallocate it or something.
window.hide().unwrap();
if let Err(e) = window.hide() {
tracing::warn!(error = std_dyn_err(&e), "Failed to hide window")
};
api.prevent_close();
}
})
@@ -260,10 +262,10 @@ pub(crate) fn run(
}
Ok(Err(error)) => {
tracing::error!(error = std_dyn_err(&error), "run_controller returned an error");
errors::show_error_dialog(&error).unwrap();
if let Err(e) = errors::show_error_dialog(&error) {
tracing::error!(error = anyhow_dyn_err(&e), "Failed to show error dialog");
}
telemetry.stop_on_crash().await;
1
}
Ok(Ok(_)) => {

24
rust/gui-client/src-tauri/src/client/logging.rs
View File

@@ -55,14 +55,22 @@ fn show_export_dialog(app: &tauri::AppHandle, ctlr_tx: CtlrTx) -> Result<()> {
tauri_plugin_dialog::FileDialogBuilder::new(app.dialog().clone())
.add_filter("Zip", &["zip"])
.set_file_name(filename)
.save_file(move |file_path| match file_path {
None => {}
Some(path) => {
let path = path.into_path().unwrap();
// blocking_send here because we're in a sync callback within Tauri somewhere
ctlr_tx
.blocking_send(ControllerRequest::ExportLogs { path, stem })
.unwrap()
.save_file(move |file_path| {
let Some(file_path) = file_path else {
return;
};
let path = match file_path.clone().into_path() {
Ok(path) => path,
Err(e) => {
tracing::warn!(error = std_dyn_err(&e), %file_path, "Invalid file path");
return;
}
};
// blocking_send here because we're in a sync callback within Tauri somewhere
if let Err(e) = ctlr_tx.blocking_send(ControllerRequest::ExportLogs { path, stem }) {
tracing::warn!("Failed to send `ExportLogs` command: {e}");
}
});
Ok(())

1
rust/gui-client/src-tauri/src/main.rs
View File

@@ -2,6 +2,7 @@
// Prevents additional console window on Windows in release, DO NOT REMOVE!!
#![cfg_attr(not(debug_assertions), windows_subsystem = "windows")]
#![cfg_attr(test, allow(clippy::unwrap_used))]
mod client;

6
rust/headless-client/src/ipc_service.rs
View File

@@ -381,10 +381,8 @@ impl<'a> Handler<'a> {
tracing::info!(
"Caught SIGINT / SIGTERM / Ctrl+C while an IPC client is connected"
);
self.ipc_tx
.send(&ServerMsg::TerminatingGracefully)
.await
.unwrap();
// Ignore the result here because we're terminating anyway.
let _ = self.ipc_tx.send(&ServerMsg::TerminatingGracefully).await;
break HandlerOk::ServiceTerminating;
}
}

2
rust/headless-client/src/lib.rs
View File

@@ -8,6 +8,8 @@
//! Tauri deb bundler to pick it up easily.
//! Otherwise we would just make it a normal binary crate.
#![cfg_attr(test, allow(clippy::unwrap_used))]
use anyhow::{Context as _, Result};
use connlib_client_shared::Callbacks;
use connlib_model::ResourceView;

4
rust/headless-client/src/main.rs
View File

@@ -1,5 +1,7 @@
//! The headless Client, AKA standalone Client
#![cfg_attr(test, allow(clippy::unwrap_used))]
use anyhow::{anyhow, Context as _, Result};
use backoff::ExponentialBackoffBuilder;
use clap::Parser;
@@ -152,7 +154,7 @@ fn main() -> Result<()> {
.as_deref()
.map(firezone_logging::file::layer)
.unzip();
firezone_logging::setup_global_subscriber(layer);
firezone_logging::setup_global_subscriber(layer).context("Failed to set up logging")?;
tracing::info!(arch = std::env::consts::ARCH, version = VERSION);

3
rust/ip-packet/src/lib.rs
View File

@@ -1,3 +1,5 @@
#![cfg_attr(test, allow(clippy::unwrap_used))]
pub mod make;
mod fz_p2p_control;
@@ -9,6 +11,7 @@ mod ipv6_header_slice_mut;
mod nat46;
mod nat64;
#[cfg(feature = "proptest")]
#[allow(clippy::unwrap_used)]
pub mod proptest;
mod slice_utils;
mod tcp_header_slice_mut;

23
rust/logging/src/lib.rs
View File

@@ -1,3 +1,5 @@
#![cfg_attr(test, allow(clippy::unwrap_used))]
mod dyn_err;
pub mod file;
mod format;
@@ -5,6 +7,7 @@ mod format;
mod unwrap_or;
mod err_with_sources;
use anyhow::{Context, Result};
use sentry_tracing::EventFilter;
use tracing::{subscriber::DefaultGuard, Subscriber};
use tracing_log::LogTracer;
@@ -18,27 +21,27 @@ pub use err_with_sources::{err_with_sources, ErrorWithSources};
pub use format::Format;
/// Registers a global subscriber with stdout logging and `additional_layer`
pub fn setup_global_subscriber<L>(additional_layer: L)
pub fn setup_global_subscriber<L>(additional_layer: L) -> Result<()>
where
L: Layer<Registry> + Send + Sync,
{
let directives = std::env::var("RUST_LOG").unwrap_or_default();
let subscriber = Registry::default()
.with(additional_layer.with_filter(filter(&directives)))
.with(
additional_layer
.with_filter(try_filter(&directives).context("Failed to parse directives")?),
)
.with(sentry_layer())
.with(
fmt::layer()
.event_format(Format::new())
.with_filter(filter(&directives)),
.with_filter(try_filter(&directives).context("Failed to parse directives")?),
);
tracing::subscriber::set_global_default(subscriber).expect("Could not set global default");
LogTracer::init().unwrap();
}
tracing::subscriber::set_global_default(subscriber).context("Could not set global default")?;
LogTracer::init().context("Failed to init LogTracer")?;
/// Constructs an opinionated [`EnvFilter`] with some crates already silenced.
pub fn filter(directives: &str) -> EnvFilter {
try_filter(directives).unwrap()
Ok(())
}
/// Constructs an opinionated [`EnvFilter`] with some crates already silenced.
@@ -124,7 +127,7 @@ where
})
.span_filter(|md| *md.level() == tracing::Level::TRACE && md.target() == TELEMETRY_TARGET)
.enable_span_attributes()
.with_filter(filter("trace")) // Filter out noisy crates but pass all events otherwise.
.with_filter(try_filter("trace").expect("static filter always parses")) // Filter out noisy crates but pass all events otherwise.
}
#[doc(hidden)]

24
rust/phoenix-channel/src/lib.rs
View File

@@ -1,3 +1,5 @@
#![cfg_attr(test, allow(clippy::unwrap_used))]
mod get_user_agent;
mod heartbeat;
mod login_url;
@@ -88,14 +90,14 @@ async fn create_and_connect_websocket(
user_agent: String,
socket_factory: Arc<dyn SocketFactory<TcpSocket>>,
) -> Result<WebSocketStream<MaybeTlsStream<TcpStream>>, InternalError> {
tracing::debug!(host = %url.host().unwrap(), %user_agent, "Connecting to portal");
tracing::debug!(host = url.host().map(tracing::field::display), %user_agent, "Connecting to portal");
let duration = Duration::from_secs(5);
let socket = tokio::time::timeout(duration, connect(addresses, &*socket_factory))
.await
.map_err(|_| InternalError::Timeout { duration })??;
let (stream, _) = client_async_tls(make_request(url, user_agent), socket)
let (stream, _) = client_async_tls(make_request(url, user_agent)?, socket)
.await
.map_err(InternalError::WebSocket)?;
@@ -158,6 +160,7 @@ enum InternalError {
CloseMessage,
StreamClosed,
InvalidUrl,
FailedToBuildRequest(tokio_tungstenite::tungstenite::http::Error),
SocketConnection(std::io::Error),
Timeout { duration: Duration },
}
@@ -185,6 +188,9 @@ impl fmt::Display for InternalError {
InternalError::Timeout { duration, .. } => {
write!(f, "operation timed out after {duration:?}")
}
InternalError::FailedToBuildRequest(_) => {
write!(f, "failed to build request")
}
}
}
}
@@ -196,6 +202,7 @@ impl std::error::Error for InternalError {
InternalError::WebSocket(e) => Some(e),
InternalError::Serde(e) => Some(e),
InternalError::SocketConnection(e) => Some(e),
InternalError::FailedToBuildRequest(e) => Some(e),
InternalError::MissedHeartbeat => None,
InternalError::CloseMessage => None,
InternalError::StreamClosed => None,
@@ -788,14 +795,17 @@ impl<T, R> PhoenixMessage<T, R> {
}
// This is basically the same as tungstenite does but we add some new headers (namely user-agent)
fn make_request(url: Url, user_agent: String) -> Request {
fn make_request(url: Url, user_agent: String) -> Result<Request, InternalError> {
let mut r = [0u8; 16];
OsRng.fill_bytes(&mut r);
let key = base64::engine::general_purpose::STANDARD.encode(r);
Request::builder()
let request = Request::builder()
.method("GET")
.header("Host", url.host().unwrap().to_string())
.header(
"Host",
url.host().ok_or(InternalError::InvalidUrl)?.to_string(),
)
.header("Connection", "Upgrade")
.header("Upgrade", "websocket")
.header("Sec-WebSocket-Version", "13")
@@ -803,7 +813,9 @@ fn make_request(url: Url, user_agent: String) -> Request {
.header("User-Agent", user_agent)
.uri(url.to_string())
.body(())
.expect("building static request always works")
.map_err(InternalError::FailedToBuildRequest)?;
Ok(request)
}
#[derive(Debug, Deserialize, Serialize, Clone)]

1
rust/relay/Cargo.toml
View File

@@ -32,6 +32,7 @@ smallvec = "1.13.2"
socket-factory = { workspace = true }
socket2 = { workspace = true }
stun_codec = "0.3.4"
thiserror = "1.0.68"
tokio = { workspace = true, features = ["macros", "rt-multi-thread", "net", "time", "signal"] }
tracing = { workspace = true, features = ["log"] }
tracing-core = "0.1.31"

9
rust/relay/src/auth.rs
View File

@@ -11,7 +11,8 @@ use stun_codec::rfc5389::attributes::{MessageIntegrity, Realm, Username};
use uuid::Uuid;
// TODO: Upstream a const constructor to `stun-codec`.
pub static FIREZONE: Lazy<Realm> = Lazy::new(|| Realm::new("firezone".to_owned()).unwrap());
pub static FIREZONE: Lazy<Realm> =
Lazy::new(|| Realm::new("firezone".to_owned()).expect("static realm is less than 128 chars"));
pub(crate) trait MessageIntegrityExt {
fn verify(
@@ -91,11 +92,15 @@ impl Nonces {
}
}
#[derive(Debug, PartialEq)]
#[derive(Debug, PartialEq, thiserror::Error)]
pub(crate) enum Error {
#[error("expired")]
Expired,
#[error("invalid password")]
InvalidPassword,
#[error("invalid username")]
InvalidUsername,
#[error("invalid nonce")]
InvalidNonce,
}

3
rust/relay/src/lib.rs
View File

@@ -1,9 +1,12 @@
#![cfg_attr(test, allow(clippy::unwrap_used))]
mod net_ext;
mod server;
mod sleep;
pub mod auth;
#[cfg(feature = "proptest")]
#[allow(clippy::unwrap_used)]
pub mod proptest;
pub mod sockets;

11
rust/relay/src/main.rs
View File

@@ -1,3 +1,5 @@
#![cfg_attr(test, allow(clippy::unwrap_used))]
use anyhow::{anyhow, bail, Context, Result};
use backoff::ExponentialBackoffBuilder;
use clap::Parser;
@@ -574,7 +576,10 @@ where
}
Event::HeartbeatSent => {
tracing::debug!(target: "relay", "Heartbeat sent to portal");
*self.last_heartbeat_sent.lock().unwrap() = Some(Instant::now());
*self
.last_heartbeat_sent
.lock()
.unwrap_or_else(|e| e.into_inner()) = Some(Instant::now());
}
Event::InboundMessage {
msg: IngressMessage::Init(Init {}),
@@ -609,7 +614,9 @@ fn make_is_healthy(
}
fn is_healthy(last_heartbeat_sent: Arc<Mutex<Option<Instant>>>) -> bool {
let guard = last_heartbeat_sent.lock().unwrap();
let guard = last_heartbeat_sent
.lock()
.unwrap_or_else(|e| e.into_inner());
let Some(last_hearbeat_sent) = *guard else {
return true; // If we are not connected to the portal, we are always healthy.

6
rust/relay/src/server.rs
View File

@@ -1025,7 +1025,11 @@ where
self.add_nonce(new_nonce);
message.add_attribute(Nonce::new(new_nonce.to_string()).unwrap());
message.add_attribute(
Nonce::new(new_nonce.to_string()).expect(
"UUIDs are valid nonces because they are less than 128 characters long",
),
);
message.add_attribute((*FIREZONE).clone());
}

61
rust/relay/src/server/client_message.rs
View File

@@ -2,6 +2,7 @@ use crate::auth::{generate_password, split_username, systemtime_from_unix, FIREZ
use crate::server::channel_data::ChannelData;
use crate::server::UDP_TRANSPORT;
use crate::Attribute;
use anyhow::{Context, Result};
use bytecodec::DecodeExt;
use secrecy::SecretString;
use std::io;
@@ -162,7 +163,7 @@ impl Allocate {
username: Username,
relay_secret: &SecretString,
nonce: Uuid,
) -> Self {
) -> Result<Self> {
let (requested_transport, nonce, message_integrity) = Self::make_attributes(
transaction_id,
&lifetime,
@@ -170,9 +171,9 @@ impl Allocate {
relay_secret,
nonce,
None,
);
)?;
Self {
Ok(Self {
transaction_id,
message_integrity: Some(message_integrity),
requested_transport,
@@ -182,7 +183,7 @@ impl Allocate {
requested_address_family: None, // IPv4 is the default.
additional_address_family: None,
software: None,
}
})
}
pub fn new_authenticated_udp_ip6(
@@ -191,7 +192,7 @@ impl Allocate {
username: Username,
relay_secret: &SecretString,
nonce: Uuid,
) -> Self {
) -> Result<Self> {
let requested_address_family = RequestedAddressFamily::new(AddressFamily::V6);
let (requested_transport, nonce, message_integrity) = Self::make_attributes(
@@ -201,9 +202,9 @@ impl Allocate {
relay_secret,
nonce,
Some(requested_address_family.clone()),
);
)?;
Self {
Ok(Self {
transaction_id,
message_integrity: Some(message_integrity),
requested_transport,
@@ -213,7 +214,7 @@ impl Allocate {
requested_address_family: Some(requested_address_family),
additional_address_family: None,
software: None,
}
})
}
pub fn new_unauthenticated_udp(
@@ -250,9 +251,9 @@ impl Allocate {
relay_secret: &SecretString,
nonce: Uuid,
requested_address_family: Option<RequestedAddressFamily>,
) -> (RequestedTransport, Nonce, MessageIntegrity) {
) -> Result<(RequestedTransport, Nonce, MessageIntegrity)> {
let requested_transport = RequestedTransport::new(UDP_TRANSPORT);
let nonce = Nonce::new(nonce.as_hyphenated().to_string()).expect("len(uuid) < 128");
let nonce = Nonce::new(nonce.as_hyphenated().to_string()).context("Invalid nonce")?;
let mut message =
Message::<Attribute>::new(MessageClass::Request, ALLOCATE, transaction_id);
@@ -268,15 +269,15 @@ impl Allocate {
message.add_attribute(lifetime.clone());
}
let (expiry, salt) = split_username(username.name()).expect("a valid username");
let (expiry, salt) = split_username(username.name())?;
let expiry_systemtime = systemtime_from_unix(expiry);
let password = generate_password(relay_secret, expiry_systemtime, salt);
let message_integrity =
MessageIntegrity::new_long_term_credential(&message, username, &FIREZONE, &password)
.unwrap();
(requested_transport, nonce, message_integrity)
MessageIntegrity::new_long_term_credential(&message, username, &FIREZONE, &password)?;
Ok((requested_transport, nonce, message_integrity))
}
pub fn parse(message: &Message<Attribute>) -> Result<Self, Message<Attribute>> {
@@ -359,8 +360,8 @@ impl Refresh {
username: Username,
relay_secret: &SecretString,
nonce: Uuid,
) -> Self {
let nonce = Nonce::new(nonce.as_hyphenated().to_string()).expect("len(uuid) < 128");
) -> Result<Self> {
let nonce = Nonce::new(nonce.as_hyphenated().to_string()).context("Invalid nonce")?;
let mut message = Message::<Attribute>::new(MessageClass::Request, REFRESH, transaction_id);
message.add_attribute(username.clone());
@@ -370,23 +371,22 @@ impl Refresh {
message.add_attribute(lifetime.clone());
}
let (expiry, salt) = split_username(username.name()).expect("a valid username");
let (expiry, salt) = split_username(username.name())?;
let expiry_systemtime = systemtime_from_unix(expiry);
let password = generate_password(relay_secret, expiry_systemtime, salt);
let message_integrity =
MessageIntegrity::new_long_term_credential(&message, &username, &FIREZONE, &password)
.unwrap();
MessageIntegrity::new_long_term_credential(&message, &username, &FIREZONE, &password)?;
Self {
Ok(Self {
transaction_id,
message_integrity: Some(message_integrity),
lifetime,
username: Some(username),
nonce: Some(nonce),
software: None,
}
})
}
pub fn parse(message: &Message<Attribute>) -> Self {
@@ -450,8 +450,8 @@ impl ChannelBind {
username: Username,
relay_secret: &SecretString,
nonce: Uuid,
) -> Self {
let nonce = Nonce::new(nonce.as_hyphenated().to_string()).expect("len(uuid) < 128");
) -> Result<Self> {
let nonce = Nonce::new(nonce.as_hyphenated().to_string()).context("Invalid nonce")?;
let mut message =
Message::<Attribute>::new(MessageClass::Request, CHANNEL_BIND, transaction_id);
@@ -460,16 +460,15 @@ impl ChannelBind {
message.add_attribute(xor_peer_address.clone());
message.add_attribute(nonce.clone());
let (expiry, salt) = split_username(username.name()).expect("a valid username");
let (expiry, salt) = split_username(username.name())?;
let expiry_systemtime = systemtime_from_unix(expiry);
let password = generate_password(relay_secret, expiry_systemtime, salt);
let message_integrity =
MessageIntegrity::new_long_term_credential(&message, &username, &FIREZONE, &password)
.unwrap();
MessageIntegrity::new_long_term_credential(&message, &username, &FIREZONE, &password)?;
Self {
Ok(Self {
transaction_id,
channel_number,
message_integrity: Some(message_integrity),
@@ -477,7 +476,7 @@ impl ChannelBind {
username: Some(username),
nonce: Some(nonce),
software: None,
}
})
}
pub fn parse(message: &Message<Attribute>) -> Result<Self, Message<Attribute>> {
@@ -584,12 +583,14 @@ impl CreatePermission {
/// Computes the effective lifetime of an allocation.
fn compute_effective_lifetime(requested_lifetime: Option<&Lifetime>) -> Lifetime {
let Some(requested) = requested_lifetime else {
return Lifetime::new(DEFAULT_ALLOCATION_LIFETIME).unwrap();
return Lifetime::new(DEFAULT_ALLOCATION_LIFETIME)
.expect("Default lifetime is less than 0xFFFF_FFFF");
};
let effective_lifetime = requested.lifetime().min(MAX_ALLOCATION_LIFETIME);
Lifetime::new(effective_lifetime).unwrap()
Lifetime::new(effective_lifetime)
.expect("lifetime is at most MAX_ALLOCATION_LIFETIME which is less than 0xFFFF_FFFF")
}
fn bad_request(message: &Message<Attribute>) -> Message<Attribute> {

94
rust/relay/tests/regression.rs
View File

@@ -1,3 +1,5 @@
#![allow(clippy::unwrap_used)]
use bytecodec::{DecodeExt, EncodeExt};
use firezone_relay::{
AddressFamily, Allocate, AllocationPort, Attribute, Binding, ChannelBind, ChannelData,
@@ -61,7 +63,8 @@ fn deallocate_once_time_expired(
valid_username(&username_salt),
secret,
nonce,
),
)
.unwrap(),
now,
),
[
@@ -121,7 +124,8 @@ fn unauthenticated_allocate_triggers_authentication(
valid_username(&username_salt),
&secret,
first_nonce,
),
)
.unwrap(),
now,
),
[
@@ -165,7 +169,8 @@ fn when_refreshed_in_time_allocation_does_not_expire(
valid_username(&username_salt),
&secret,
nonce,
),
)
.unwrap(),
now,
),
[
@@ -198,7 +203,8 @@ fn when_refreshed_in_time_allocation_does_not_expire(
valid_username(&username_salt),
&secret,
nonce,
),
)
.unwrap(),
now,
),
[send_message(
@@ -243,7 +249,8 @@ fn when_receiving_lifetime_0_for_existing_allocation_then_delete(
valid_username(&username_salt),
&secret,
nonce,
),
)
.unwrap(),
now,
),
[
@@ -275,7 +282,8 @@ fn when_receiving_lifetime_0_for_existing_allocation_then_delete(
valid_username(&username_salt),
&secret,
nonce,
),
)
.unwrap(),
now,
),
[
@@ -315,36 +323,45 @@ fn freeing_allocation_clears_all_channels(
let secret = server.auth_secret().to_owned();
let _ = server.server.handle_client_message(
ClientMessage::Allocate(Allocate::new_authenticated_udp_implicit_ip4(
allocate_transaction_id,
None,
valid_username(&username_salt),
&secret,
nonce,
)),
ClientMessage::Allocate(
Allocate::new_authenticated_udp_implicit_ip4(
allocate_transaction_id,
None,
valid_username(&username_salt),
&secret,
nonce,
)
.unwrap(),
),
ClientSocket::new(source),
now,
);
let _ = server.server.handle_client_message(
ClientMessage::ChannelBind(ChannelBind::new(
channel_bind_transaction_id,
channel,
XorPeerAddress::new(peer.into()),
valid_username(&username_salt),
&secret,
nonce,
)),
ClientMessage::ChannelBind(
ChannelBind::new(
channel_bind_transaction_id,
channel,
XorPeerAddress::new(peer.into()),
valid_username(&username_salt),
&secret,
nonce,
)
.unwrap(),
),
ClientSocket::new(source),
now,
);
let _ = server.server.handle_client_message(
ClientMessage::Refresh(Refresh::new(
refresh_transaction_id,
Some(Lifetime::new(Duration::ZERO).unwrap()),
valid_username(&username_salt),
&secret,
nonce,
)),
ClientMessage::Refresh(
Refresh::new(
refresh_transaction_id,
Some(Lifetime::new(Duration::ZERO).unwrap()),
valid_username(&username_salt),
&secret,
nonce,
)
.unwrap(),
),
ClientSocket::new(source),
now,
);
@@ -387,7 +404,8 @@ fn ping_pong_relay(
valid_username(&username_salt),
&secret,
nonce,
),
)
.unwrap(),
now,
),
[
@@ -422,7 +440,8 @@ fn ping_pong_relay(
valid_username(&username_salt),
&secret,
nonce,
),
)
.unwrap(),
now,
),
[send_message(
@@ -496,7 +515,8 @@ fn allows_rebind_channel_after_expiry(
valid_username(&username_salt),
&secret,
nonce,
),
)
.unwrap(),
now,
),
[
@@ -531,7 +551,8 @@ fn allows_rebind_channel_after_expiry(
valid_username(&username_salt),
&secret,
nonce,
),
)
.unwrap(),
now,
),
[send_message(
@@ -566,7 +587,8 @@ fn allows_rebind_channel_after_expiry(
valid_username(&username_salt),
&secret,
nonce,
),
)
.unwrap(),
now,
),
[send_message(
@@ -614,7 +636,8 @@ fn ping_pong_ip6_relay(
valid_username(&username_salt),
&secret,
nonce,
),
)
.unwrap(),
now,
),
[
@@ -649,7 +672,8 @@ fn ping_pong_ip6_relay(
valid_username(&username_salt),
&secret,
nonce,
),
)
.unwrap(),
now,
),
[send_message(

6
rust/telemetry/src/lib.rs
View File

@@ -83,10 +83,12 @@ impl Telemetry {
scope.set_tag("api_url", api_url);
let ctx = sentry::integrations::contexts::utils::device_context();
scope.set_context("device", ctx);
let ctx = sentry::integrations::contexts::utils::os_context().unwrap();
scope.set_context("os", ctx);
let ctx = sentry::integrations::contexts::utils::rust_context();
scope.set_context("rust", ctx);
if let Some(ctx) = sentry::integrations::contexts::utils::os_context() {
scope.set_context("os", ctx);
}
});
self.inner.replace(inner);
sentry::start_session();