diff --git a/rust/Cargo.lock b/rust/Cargo.lock index 07c46d775..f65f28dc8 100644 --- a/rust/Cargo.lock +++ b/rust/Cargo.lock @@ -6490,9 +6490,9 @@ dependencies = [ [[package]] name = "secrecy" -version = "0.8.0" +version = "0.10.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9bd1c54ea06cfd2f6b63219704de0b9b4f72dcc2b8fdef820be6cd799780e91e" +checksum = "e891af845473308773346dc847b2c23ee78fe442e0472ac50e22a18a93d3ae5a" dependencies = [ "serde", "zeroize", diff --git a/rust/Cargo.toml b/rust/Cargo.toml index b4b179187..b0a7c10e4 100644 --- a/rust/Cargo.toml +++ b/rust/Cargo.toml @@ -144,7 +144,7 @@ rtnetlink = { version = "0.17.0", default-features = false, features = ["tokio_s rustls = { version = "0.23.31", default-features = false, features = ["ring"] } sadness-generator = "0.6.0" sd-notify = "0.4.5" # This is a pure Rust re-implementation, so it isn't vulnerable to CVE-2024-3094 -secrecy = "0.8" +secrecy = "0.10" semver = "1.0.27" sentry = { version = "0.42.0", default-features = false } sentry-tracing = "0.42.0" diff --git a/rust/cli/src/main.rs b/rust/cli/src/main.rs index 0e80a2278..81668808f 100644 --- a/rust/cli/src/main.rs +++ b/rust/cli/src/main.rs @@ -53,7 +53,7 @@ fn main() -> Result<()> { continue; } - break SecretString::new(token); + break SecretString::new(token.into_boxed_str()); }; write_to_file(ETC_FIREZONE_GATEWAY_TOKEN, token)?; diff --git a/rust/client-ffi/src/lib.rs b/rust/client-ffi/src/lib.rs index 07eee9217..4472e3658 100644 --- a/rust/client-ffi/src/lib.rs +++ b/rust/client-ffi/src/lib.rs @@ -14,7 +14,7 @@ use firezone_logging::sentry_layer; use firezone_telemetry::{Telemetry, analytics}; use phoenix_channel::{LoginUrl, PhoenixChannel, get_user_agent}; use platform::RELEASE; -use secrecy::{Secret, SecretString}; +use secrecy::{SecretBox, SecretString}; use socket_factory::{SocketFactory, TcpSocket, UdpSocket}; use tokio::sync::Mutex; use tracing_subscriber::{Layer, layer::SubscriberExt as _}; @@ -495,7 +495,7 @@ fn connect( let _guard = runtime.enter(); // Constructing `PhoenixChannel` requires a runtime context. let portal = PhoenixChannel::disconnected( - Secret::new(url), + SecretBox::init_with(|| url), get_user_agent(os_version, platform::COMPONENT, platform::VERSION), "client", (), diff --git a/rust/connlib/phoenix-channel/src/lib.rs b/rust/connlib/phoenix-channel/src/lib.rs index d357da683..6fa34f399 100644 --- a/rust/connlib/phoenix-channel/src/lib.rs +++ b/rust/connlib/phoenix-channel/src/lib.rs @@ -19,7 +19,7 @@ use futures::future::BoxFuture; use futures::{FutureExt, SinkExt, StreamExt}; use itertools::Itertools as _; use rand_core::{OsRng, RngCore}; -use secrecy::{ExposeSecret, Secret}; +use secrecy::{ExposeSecret, SecretBox}; use serde::{Deserialize, Serialize, de::DeserializeOwned}; use socket_factory::{SocketFactory, TcpSocket, TcpStream}; use std::task::{Context, Poll, Waker}; @@ -51,7 +51,7 @@ pub struct PhoenixChannel { pending_join_requests: BTreeMap, // Stored here to allow re-connecting. - url_prototype: Secret>, + url_prototype: SecretBox>, last_url: Option, user_agent: String, make_reconnect_backoff: Box ExponentialBackoff + Send>, @@ -259,7 +259,7 @@ where /// The provided URL must contain a host. /// Additionally, you must already provide any query parameters required for authentication. pub fn disconnected( - url: Secret>, + url: SecretBox>, user_agent: String, login: &'static str, init_req: TInitReq, diff --git a/rust/connlib/phoenix-channel/src/login_url.rs b/rust/connlib/phoenix-channel/src/login_url.rs index 6256721f4..f75eda422 100644 --- a/rust/connlib/phoenix-channel/src/login_url.rs +++ b/rust/connlib/phoenix-channel/src/login_url.rs @@ -1,5 +1,5 @@ use base64::{Engine, engine::general_purpose::STANDARD}; -use secrecy::{CloneableSecret, ExposeSecret as _, SecretString, Zeroize}; +use secrecy::{CloneableSecret, ExposeSecret as _, SecretString, zeroize::Zeroize}; use serde::Deserialize; use sha2::Digest as _; use std::{ @@ -52,6 +52,7 @@ impl Zeroize for LoginUrl { } } +#[derive(Debug, Clone)] pub struct PublicKeyParam(pub [u8; 32]); impl IntoIterator for PublicKeyParam { @@ -63,6 +64,7 @@ impl IntoIterator for PublicKeyParam { } } +#[derive(Debug, Clone)] pub struct NoParams; impl IntoIterator for NoParams { @@ -332,7 +334,7 @@ mod tests { fn base_url_removes_params_and_path() { let login_url = LoginUrl::client( "wss://api.firez.one", - &SecretString::new("foobar".to_owned()), + &SecretString::from("foobar"), "some-id".to_owned(), None, DeviceInfo::default(), diff --git a/rust/connlib/phoenix-channel/tests/lib.rs b/rust/connlib/phoenix-channel/tests/lib.rs index 846903ba0..5a0790019 100644 --- a/rust/connlib/phoenix-channel/tests/lib.rs +++ b/rust/connlib/phoenix-channel/tests/lib.rs @@ -1,6 +1,8 @@ #![cfg(not(windows))] // For some reason, Windows doesn't like this test. #![allow(clippy::unwrap_used)] +use secrecy::SecretBox; + #[tokio::test] async fn client_does_not_pipeline_messages() { use std::{str::FromStr, sync::Arc, time::Duration}; @@ -8,7 +10,7 @@ async fn client_does_not_pipeline_messages() { use backoff::exponential::ExponentialBackoff; use futures::{SinkExt, StreamExt}; use phoenix_channel::{DeviceInfo, LoginUrl, PhoenixChannel, PublicKeyParam}; - use secrecy::{Secret, SecretString}; + use secrecy::SecretString; use tokio::net::TcpListener; use tokio_tungstenite::tungstenite::Message; use url::Url; @@ -59,16 +61,16 @@ async fn client_does_not_pipeline_messages() { } }); - let login_url = Secret::new( + let login_url = SecretBox::init_with(|| { LoginUrl::client( Url::from_str(&format!("ws://localhost:{}", server_addr.port())).unwrap(), - &SecretString::new("secret".to_owned()), + &SecretString::from("secret"), String::new(), None, DeviceInfo::default(), ) - .unwrap(), - ); + .unwrap() + }); let mut channel = PhoenixChannel::<(), InboundMsg, _>::disconnected( login_url, diff --git a/rust/connlib/tunnel/src/messages.rs b/rust/connlib/tunnel/src/messages.rs index 5ddf9f7a9..04a243313 100644 --- a/rust/connlib/tunnel/src/messages.rs +++ b/rust/connlib/tunnel/src/messages.rs @@ -6,7 +6,7 @@ use chrono::{DateTime, Utc, serde::ts_seconds}; use connlib_model::RelayId; use dns_types::DomainName; use ip_network::IpNetwork; -use secrecy::{ExposeSecret as _, Secret}; +use secrecy::ExposeSecret as _; use serde::{Deserialize, Serialize}; use std::fmt; @@ -79,7 +79,7 @@ pub struct Offer { #[expect(deprecated)] impl Offer { // Not a very clean API but it is deprecated anyway. - pub fn into_snownet_offer(self, key: Secret) -> snownet::Offer { + pub fn into_snownet_offer(self, key: SecretKey) -> snownet::Offer { snownet::Offer { session_key: x25519::StaticSecret::from(key.expose_secret().0), credentials: snownet::Credentials { diff --git a/rust/connlib/tunnel/src/messages/key.rs b/rust/connlib/tunnel/src/messages/key.rs index 8dd3acd25..c3477c31f 100644 --- a/rust/connlib/tunnel/src/messages/key.rs +++ b/rust/connlib/tunnel/src/messages/key.rs @@ -1,6 +1,6 @@ use base64::{Engine, display::Base64Display, engine::general_purpose::STANDARD}; use boringtun::x25519::PublicKey; -use secrecy::{CloneableSecret, DebugSecret, Secret, SerializableSecret, Zeroize}; +use secrecy::{CloneableSecret, SecretBox, SerializableSecret, zeroize::Zeroize}; use serde::{Deserialize, Deserializer, Serialize, Serializer, de}; use std::{fmt, str::FromStr}; @@ -17,8 +17,6 @@ const KEY_SIZE: usize = 32; #[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] pub struct Key(pub [u8; KEY_SIZE]); -impl DebugSecret for Key {} - impl FromStr for Key { type Err = base64::DecodeError; @@ -83,7 +81,7 @@ impl Serialize for Key { impl CloneableSecret for Key {} impl SerializableSecret for Key {} -pub type SecretKey = Secret; +pub type SecretKey = SecretBox; #[cfg(test)] mod test { diff --git a/rust/connlib/tunnel/src/tests/sut.rs b/rust/connlib/tunnel/src/tests/sut.rs index a7d682aab..f045802ec 100644 --- a/rust/connlib/tunnel/src/tests/sut.rs +++ b/rust/connlib/tunnel/src/tests/sut.rs @@ -992,7 +992,8 @@ fn make_preshared_key_and_ice( client_key: PublicKey, gateway_key: PublicKey, ) -> (SecretKey, IceCredentials, IceCredentials) { - let secret_key = SecretKey::new(Key(hkdf("SECRET_KEY_DOMAIN_SEP", client_key, gateway_key))); + let secret_key = + SecretKey::init_with(|| Key(hkdf("SECRET_KEY_DOMAIN_SEP", client_key, gateway_key))); let client_ice = ice_creds("CLIENT_ICE_DOMAIN_SEP", client_key, gateway_key); let gateway_ice = ice_creds("GATEWAY_ICE_DOMAIN_SEP", client_key, gateway_key); diff --git a/rust/gateway/src/main.rs b/rust/gateway/src/main.rs index 30cfd1fcb..68e15fd70 100644 --- a/rust/gateway/src/main.rs +++ b/rust/gateway/src/main.rs @@ -21,7 +21,7 @@ use phoenix_channel::LoginUrl; use phoenix_channel::get_user_agent; use phoenix_channel::PhoenixChannel; -use secrecy::{Secret, SecretString}; +use secrecy::{ExposeSecret, SecretBox, SecretString}; use std::{collections::BTreeSet, fmt, path::Path}; use std::{path::PathBuf, process::ExitCode}; use std::{sync::Arc, time::Duration}; @@ -196,7 +196,7 @@ async fn try_main(cli: Cli, telemetry: &mut Telemetry) -> Result<()> { nameservers, ); let portal = PhoenixChannel::disconnected( - Secret::new(login), + SecretBox::init_with(|| login), get_user_agent(None, "gateway", env!("CARGO_PKG_VERSION")), PHOENIX_TOPIC, (), @@ -276,7 +276,13 @@ async fn read_systemd_credential(name: &str) -> Result { let path = PathBuf::from(creds_dir).join(name); let content = tokio::fs::read_to_string(&path).await?; - Ok(SecretString::new(content.trim().to_owned())) + // Immediately wrap in `SecretString` to ensure it will be zeroized. + let untrimmed_token = SecretString::from(content); + + // Create a 2nd `SecretString` that is trimmed. + let trimmed_token = SecretString::from(untrimmed_token.expose_secret().trim()); + + Ok(trimmed_token) } #[derive(Parser, Debug)] @@ -447,7 +453,6 @@ impl ValidateChecksumAdapter { #[cfg(test)] mod tests { use super::*; - use secrecy::ExposeSecret as _; use tempfile::TempDir; #[tokio::test] diff --git a/rust/gui-client/src-tauri/Cargo.toml b/rust/gui-client/src-tauri/Cargo.toml index 34423a1e1..7ad67b1a9 100644 --- a/rust/gui-client/src-tauri/Cargo.toml +++ b/rust/gui-client/src-tauri/Cargo.toml @@ -44,7 +44,7 @@ rand = { workspace = true } reqwest = { workspace = true, features = ["stream", "rustls-tls"] } rustls = { workspace = true } sadness-generator = { workspace = true } -secrecy = { workspace = true } +secrecy = { workspace = true, features = ["serde"] } semver = { workspace = true, features = ["serde"] } serde = { workspace = true, features = ["derive"] } serde_json = { workspace = true } diff --git a/rust/gui-client/src-tauri/src/auth.rs b/rust/gui-client/src-tauri/src/auth.rs index 4b82c58f1..306432419 100644 --- a/rust/gui-client/src-tauri/src/auth.rs +++ b/rust/gui-client/src-tauri/src/auth.rs @@ -66,12 +66,14 @@ impl Request { url.set_path(account_slug); } - url.query_pairs_mut() - .append_pair("as", "client") - .append_pair("nonce", self.nonce.expose_secret()) - .append_pair("state", self.state.expose_secret()); + // Avoid further usage of `Url` here so we don't need to zeroize it. + let base = url.to_string(); - SecretString::new(url.to_string()) + SecretString::from(format!( + "{base}?as=client&nonce={}&state={}", + self.nonce.expose_secret(), + self.state.expose_secret() + )) } } diff --git a/rust/gui-client/src-tauri/src/controller.rs b/rust/gui-client/src-tauri/src/controller.rs index e057131c4..ea71149af 100644 --- a/rust/gui-client/src-tauri/src/controller.rs +++ b/rust/gui-client/src-tauri/src/controller.rs @@ -331,7 +331,7 @@ impl Controller { self.send_ipc(&service::ClientMsg::Connect { api_url: api_url.to_string(), - token: token.expose_secret().clone(), + token, is_internet_resource_active: self.general_settings.internet_resource_enabled(), }) .await?; diff --git a/rust/gui-client/src-tauri/src/deep_link.rs b/rust/gui-client/src-tauri/src/deep_link.rs index ff5f9e158..0ab375951 100644 --- a/rust/gui-client/src-tauri/src/deep_link.rs +++ b/rust/gui-client/src-tauri/src/deep_link.rs @@ -97,13 +97,13 @@ pub(crate) fn parse_auth_callback(url: &Url) -> Result { if fragment.is_some() { bail!("`fragment` should appear exactly once"); } - fragment = Some(SecretString::new(value.to_string())); + fragment = Some(SecretString::from(value.as_ref())); } "state" => { if state.is_some() { bail!("`state` should appear exactly once"); } - state = Some(SecretString::new(value.to_string())); + state = Some(SecretString::from(value.as_ref())); } _ => {} } diff --git a/rust/gui-client/src-tauri/src/service.rs b/rust/gui-client/src-tauri/src/service.rs index 9fb2118e1..c3eb2e881 100644 --- a/rust/gui-client/src-tauri/src/service.rs +++ b/rust/gui-client/src-tauri/src/service.rs @@ -22,7 +22,7 @@ use futures::{ task::{Context, Poll}, }; use phoenix_channel::{DeviceInfo, LoginUrl, PhoenixChannel, get_user_agent}; -use secrecy::{Secret, SecretString}; +use secrecy::{ExposeSecret, SecretBox, SecretString}; use std::{ io::{self, Write}, mem, @@ -47,12 +47,13 @@ mod platform; pub use platform::{elevation_check, install, run}; -#[derive(Debug, PartialEq, serde::Deserialize, serde::Serialize)] +#[derive(Debug, serde::Deserialize, serde::Serialize)] pub enum ClientMsg { ClearLogs, Connect { api_url: String, - token: String, + #[serde(serialize_with = "serialize_token")] + token: SecretString, is_internet_resource_active: bool, }, Disconnect, @@ -67,6 +68,13 @@ pub enum ClientMsg { }, } +fn serialize_token(token: &SecretString, serializer: S) -> Result +where + S: serde::Serializer, +{ + serializer.serialize_str(token.expose_secret()) +} + /// Messages that end up in the GUI, either forwarded from connlib or from the Tunnel service. #[derive(Debug, serde::Deserialize, serde::Serialize, strum::Display)] pub enum ServerMsg { @@ -516,8 +524,6 @@ impl<'a> Handler<'a> { token, is_internet_resource_active, } => { - let token = SecretString::new(token); - if !self.session.is_none() { tracing::debug!(session = ?self.session, "Connecting despite existing session"); } @@ -641,7 +647,7 @@ impl<'a> Handler<'a> { // Synchronous DNS resolution here let portal = PhoenixChannel::disconnected( - Secret::new(url), + SecretBox::init_with(|| url), get_user_agent(None, "gui-client", env!("CARGO_PKG_VERSION")), "client", (), diff --git a/rust/headless-client/src/main.rs b/rust/headless-client/src/main.rs index f355441eb..75cbd3d7b 100644 --- a/rust/headless-client/src/main.rs +++ b/rust/headless-client/src/main.rs @@ -19,7 +19,7 @@ use opentelemetry_sdk::metrics::SdkMeterProvider; use phoenix_channel::PhoenixChannel; use phoenix_channel::get_user_agent; use phoenix_channel::{DeviceInfo, LoginUrl}; -use secrecy::{Secret, SecretString}; +use secrecy::{SecretBox, SecretString}; use std::{ path::{Path, PathBuf}, sync::Arc, @@ -336,7 +336,7 @@ fn try_main() -> Result<()> { // for an Internet connection if it launches us at startup. // When running interactively, it is useful for the user to see that we can't reach the portal. let portal = PhoenixChannel::disconnected( - Secret::new(url), + SecretBox::init_with(|| url), get_user_agent(None, "headless-client", env!("CARGO_PKG_VERSION")), "client", (), diff --git a/rust/relay/server/src/auth.rs b/rust/relay/server/src/auth.rs index 1a7308f37..4d63d1ea4 100644 --- a/rust/relay/server/src/auth.rs +++ b/rust/relay/server/src/auth.rs @@ -228,7 +228,7 @@ pub fn generate_password(relay_secret: &SecretString, expiry: u64, username_salt hasher.update(format!("{expiry}")); hasher.update(":"); - hasher.update(relay_secret.expose_secret().as_str()); + hasher.update(relay_secret.expose_secret()); hasher.update(":"); hasher.update(username_salt); @@ -256,7 +256,7 @@ mod tests { fn generate_password_test_vector() { let expiry = 60 * 60 * 24 * 365 * 60; - let password = generate_password(&RELAY_SECRET_1.parse().unwrap(), expiry, SAMPLE_USERNAME); + let password = generate_password(&RELAY_SECRET_1.into(), expiry, SAMPLE_USERNAME); assert_eq!(password, "00hqldgk5xLeKKOB+xls9mHMVtgqzie9DulfgQwMv68") } @@ -265,7 +265,7 @@ mod tests { fn generate_password_test_vector_elixir() { let expiry = 1685984278; let password = generate_password( - &"1cab293a-4032-46f4-862a-40e5d174b0d2".parse().unwrap(), + &"1cab293a-4032-46f4-862a-40e5d174b0d2".into(), expiry, "uvdgKvS9GXYZ_vmv", ); @@ -274,14 +274,11 @@ mod tests { #[test] fn smoke() { - let message_integrity = message_integrity( - &RELAY_SECRET_1.parse().unwrap(), - 1685200000, - "n23JJ2wKKtt30oXi", - ); + let message_integrity = + message_integrity(&RELAY_SECRET_1.into(), 1685200000, "n23JJ2wKKtt30oXi"); let result = message_integrity.verify( - &RELAY_SECRET_1.parse().unwrap(), + &RELAY_SECRET_1.into(), "1685200000:n23JJ2wKKtt30oXi", systemtime_from_unix(1685200000 - 1000), ); @@ -292,13 +289,13 @@ mod tests { #[test] fn expired_is_not_valid() { let message_integrity = message_integrity( - &RELAY_SECRET_1.parse().unwrap(), + &RELAY_SECRET_1.into(), 1685200000 - 1000, "n23JJ2wKKtt30oXi", ); let result = message_integrity.verify( - &RELAY_SECRET_1.parse().unwrap(), + &RELAY_SECRET_1.into(), "1685199000:n23JJ2wKKtt30oXi", systemtime_from_unix(1685200000), ); @@ -308,14 +305,11 @@ mod tests { #[test] fn different_relay_secret_makes_password_invalid() { - let message_integrity = message_integrity( - &RELAY_SECRET_2.parse().unwrap(), - 1685200000, - "n23JJ2wKKtt30oXi", - ); + let message_integrity = + message_integrity(&RELAY_SECRET_2.into(), 1685200000, "n23JJ2wKKtt30oXi"); let result = message_integrity.verify( - &RELAY_SECRET_1.parse().unwrap(), + &RELAY_SECRET_1.into(), "1685200000:n23JJ2wKKtt30oXi", systemtime_from_unix(168520000 + 1000), ); @@ -325,14 +319,11 @@ mod tests { #[test] fn invalid_username_format_fails() { - let message_integrity = message_integrity( - &RELAY_SECRET_2.parse().unwrap(), - 1685200000, - "n23JJ2wKKtt30oXi", - ); + let message_integrity = + message_integrity(&RELAY_SECRET_2.into(), 1685200000, "n23JJ2wKKtt30oXi"); let result = message_integrity.verify( - &RELAY_SECRET_1.parse().unwrap(), + &RELAY_SECRET_1.into(), "foobar", systemtime_from_unix(168520000 + 1000), ); diff --git a/rust/relay/server/src/main.rs b/rust/relay/server/src/main.rs index 2a98f3f8c..bc66b94b9 100644 --- a/rust/relay/server/src/main.rs +++ b/rust/relay/server/src/main.rs @@ -15,7 +15,7 @@ use futures::{FutureExt, future}; use phoenix_channel::{Event, LoginUrl, NoParams, PhoenixChannel, get_user_agent}; use rand::rngs::StdRng; use rand::{Rng, SeedableRng}; -use secrecy::{ExposeSecret, Secret, SecretString}; +use secrecy::{ExposeSecret, SecretBox, SecretString}; use std::borrow::Cow; use std::net::{Ipv4Addr, Ipv6Addr, SocketAddr}; use std::pin::Pin; @@ -239,7 +239,7 @@ async fn try_main(args: Args) -> Result<()> { )?; let mut channel = PhoenixChannel::disconnected( - Secret::new(login), + SecretBox::init_with(|| login), get_user_agent(None, "relay", env!("CARGO_PKG_VERSION")), "relay", JoinMessage {