fix(freedesktop-rs#508): correct passphrase handling and secret serialization

Author: cachebag

Cargo.toml

@@ -33,3 +33,4 @@ nmrs = { path = "nmrs", version = "3.0" }
 async-trait = "0.1.89"
 bitflags = "2.13.0"
 tokio = { version = "1.52.3", features = ["rt-multi-thread", "macros", "sync", "time", "process"] }
+zeroize = { version = "1.9.0", features = ["aarch64", "derive", "std"] }

nmrs/CHANGELOG.md

@@ -4,6 +4,16 @@ All notable changes to the `nmrs` crate will be documented in this file.
 
 ## [Unreleased]
 
+### Added
+
+- `Passphrase` keeps Wi-Fi, OpenVPN, and WireGuard secrets redacted from debug
+  output and zeroizes their owned buffers when dropped. ([#508](https://github.com/freedesktop-rs/nmrs/pull/508))
+
+### Changed
+
+- **Breaking:** Wi-Fi, OpenVPN, and WireGuard secret fields now use
+  `Passphrase` instead of `String`. ([#508](https://github.com/freedesktop-rs/nmrs/pull/508))
+
 ### Fixed
 
 - Resolve Bluetooth devices through the BlueZ adapter that owns their address
@@ -12,11 +22,6 @@ All notable changes to the `nmrs` crate will be documented in this file.
 ## [3.4.1] - 2026-07-19
 ### Added
 
-- `AccessPoint::is_hidden()` and `NetworkSnapshot::hidden_access_points()`
-  expose access points for which NetworkManager did not report an SSID. ([#496](https://github.com/freedesktop-rs/nmrs/pull/496))
-- An isolated Docker-based NetworkManager integration harness, including
-  virtual WPA Wi-Fi coverage with `mac80211_hwsim`, keeps integration tests
-  separate from developer network profiles. ([#504](https://github.com/freedesktop-rs/nmrs/pull/504))
 - Isolated NetworkManager integration contracts now cover saved-profile events,
   secret-agent registration, wired DHCP activation, and virtual WPA Wi-Fi
   discovery/authentication/reconnection without touching developer profiles. ([#505](https://github.com/freedesktop-rs/nmrs/pull/505))
@@ -26,8 +31,6 @@ All notable changes to the `nmrs` crate will be documented in this file.
 
 ### Changed
 
-- `NetworkSnapshot::wifi_groups()` now omits hidden access points; use
-  `hidden_access_points()` when those individually reported APs are needed. ([#494](https://github.com/freedesktop-rs/nmrs/pull/494), [#496](https://github.com/freedesktop-rs/nmrs/pull/496))
 - Network, device, and settings monitors now return only after their initial
   D-Bus subscriptions are installed, so a mutation immediately after startup
   cannot race the subscription task. ([#505](https://github.com/freedesktop-rs/nmrs/pull/505))
@@ -37,8 +40,6 @@ All notable changes to the `nmrs` crate will be documented in this file.
 
 ### Fixed
 
-- Automatic Wi-Fi scans and readiness checks now skip unmanaged or unavailable
-  radios, selecting a usable managed device when one is present. ([#504](https://github.com/freedesktop-rs/nmrs/pull/504))
 - Preserve complete OpenVPN, VLAN, WireGuard, Bluetooth, Wi-Fi, and access-point
   settings when constructing or decoding NetworkManager payloads. ([#505](https://github.com/freedesktop-rs/nmrs/pull/505))
 - Preserve saved Wi-Fi profiles when stored-secret activation fails, while

nmrs/Cargo.toml

@@ -25,7 +25,7 @@ base64.workspace = true
 tokio.workspace = true
 async-trait.workspace = true
 bitflags.workspace = true
-zeroize = { version = "1.9.0", features = ["aarch64", "derive", "std"] }
+zeroize.workspace = true
 
 [package.metadata.docs.rs]
 all-features = true

nmrs/src/api/builders/vpn.rs

@@ -71,7 +71,7 @@ use zvariant::{Dict, Value, signature};
 use super::wireguard_builder::WireGuardBuilder;
 use crate::api::models::{
     ConnectionError, ConnectionOptions, OpenVpnAuthType, OpenVpnCompression, OpenVpnConfig,
-    OpenVpnProxy, VpnCredentials,
+    OpenVpnProxy, Passphrase, VpnCredentials,
 };
 
 /// Builds WireGuard VPN connection settings.
@@ -141,6 +141,16 @@ fn push_opt_str(out: &mut Vec<(String, String)>, key: &str, value: Option<&Strin
     }
 }
 
+/// Pushes a plaintext copy of a secret onto `out` if `value` is `Some`.
+///
+/// NetworkManager's VPN settings dictionary owns its strings, so the copy
+/// cannot retain the zeroization behavior of [`Passphrase`].
+fn push_opt_secret(out: &mut Vec<(String, String)>, key: &str, value: Option<&Passphrase>) {
+    if let Some(value) = value {
+        out.push((key.to_string(), value.expose_secret().to_owned()));
+    }
+}
+
 /// Pushes `(key, value.to_string())` onto `out` if `value` is `Some`.
 ///
 /// Used for numeric/boolean OpenVPN options that NetworkManager stores as
@@ -301,7 +311,10 @@ pub fn build_openvpn_connection(
                     vpn_data.push(("http-proxy-username".into(), u.clone()));
                 }
                 if let Some(p) = password {
-                    vpn_data.push(("http-proxy-password".into(), p.clone()));
+                    vpn_data.push((
+                        "http-proxy-password".into(),
+                        p.expose_secret().to_owned(),
+                    ));
                 }
             }
             OpenVpnProxy::Socks {
@@ -328,15 +341,11 @@ pub fn build_openvpn_connection(
     let data_dict = string_pairs_to_dict(vpn_data)?;
 
     let mut vpn_secrets: Vec<(String, String)> = Vec::new();
-    push_opt_display(
-        &mut vpn_secrets,
-        "password",
-        config.password.clone().map(|p| p.reveal()),
-    );
-    push_opt_display(
+    push_opt_secret(&mut vpn_secrets, "password", config.password.as_ref());
+    push_opt_secret(
         &mut vpn_secrets,
         "cert-pass",
-        config.key_password.clone().map(|p| p.reveal()),
+        config.key_password.as_ref(),
     );
 
     let mut vpn: HashMap<&'static str, Value<'static>> = HashMap::new();

nmrs/src/api/builders/wifi_builder.rs

@@ -162,9 +162,10 @@ impl WifiConnectionBuilder {
     /// routers that advertise both WPA and WPA2.
     #[must_use]
     pub fn wpa_psk(mut self, psk: impl Into<Passphrase>) -> Self {
+        let psk = psk.into();
         let mut security = HashMap::new();
         security.insert("key-mgmt", Value::from("wpa-psk"));
-        security.insert("psk", Value::from(psk.into().reveal()));
+        security.insert("psk", Value::from(psk.expose_secret().to_owned()));
         security.insert("psk-flags", Value::from(0u32));
         security.insert("auth-alg", Value::from("open"));
 
@@ -215,7 +216,10 @@ impl WifiConnectionBuilder {
 
         match opts.method {
             EapMethod::Peap | EapMethod::Ttls => {
-                e1x.insert("password", Value::from(opts.password.reveal()));
+                e1x.insert(
+                    "password",
+                    Value::from(opts.password.expose_secret().to_owned()),
+                );
 
                 if let Some(ai) = opts.anonymous_identity {
                     e1x.insert("anonymous-identity", Value::from(ai));
@@ -235,7 +239,10 @@ impl WifiConnectionBuilder {
                 }
 
                 if let Some(password) = opts.private_key_password {
-                    e1x.insert("private-key-password", Value::from(password.reveal()));
+                    e1x.insert(
+                        "private-key-password",
+                        Value::from(password.expose_secret().to_owned()),
+                    );
                 }
 
                 if let Some(cert) =

nmrs/src/api/builders/wireguard_builder.rs

@@ -205,7 +205,7 @@ impl WireGuardBuilder {
         }
 
         // Validate private key
-        validate_wireguard_key(private_key.reveal_ref(), "Private key")?;
+        validate_wireguard_key(private_key.expose_secret(), "Private key")?;
 
         // Validate address
         let (ip, prefix) = validate_address(&address)?;
@@ -246,7 +246,10 @@ impl WireGuardBuilder {
 
         // Build wireguard section
         let mut wireguard = HashMap::new();
-        wireguard.insert("private-key", Value::from(private_key.reveal()));
+        wireguard.insert(
+            "private-key",
+            Value::from(private_key.expose_secret().to_owned()),
+        );
 
         // Build peers array
         let mut peers_array: Vec<HashMap<String, zvariant::Value<'static>>> = Vec::new();
@@ -259,7 +262,10 @@ impl WireGuardBuilder {
             peer_dict.insert("allowed-ips".into(), Value::from(peer.allowed_ips));
 
             if let Some(psk) = peer.preshared_key {
-                peer_dict.insert("preshared-key".into(), Value::from(psk.reveal()));
+                peer_dict.insert(
+                    "preshared-key".into(),
+                    Value::from(psk.expose_secret().to_owned()),
+                );
             }
 
             if let Some(ka) = peer.persistent_keepalive {

nmrs/src/api/models/mod.rs

@@ -9,6 +9,7 @@ mod error;
 mod monitor;
 mod network_event;
 mod openvpn;
+mod passphrase;
 mod radio;
 mod saved_connection;
 pub(crate) mod snapshot;
@@ -33,6 +34,7 @@ pub use error::*;
 pub use monitor::*;
 pub use network_event::*;
 pub use openvpn::*;
+pub use passphrase::*;
 pub use radio::*;
 pub use saved_connection::*;
 pub use snapshot::{AppletNetworkSummary, NetworkSnapshot};

nmrs/src/api/models/passphrase.rs

@@ -0,0 +1,83 @@
+use std::fmt;
+
+use zeroize::ZeroizeOnDrop;
+
+/// An owned secret string whose buffer is zeroized when dropped.
+///
+/// `Passphrase` redacts its [`Debug`](fmt::Debug) output and keeps the secret
+/// wrapper attached when values are moved or destructured. Use
+/// [`expose_secret`](Self::expose_secret) only where plaintext access is
+/// required.
+///
+/// Zeroization reduces the lifetime of stale secret data in memory, but it
+/// cannot protect a live value from a debugger, core dump, or copies made for
+/// transport to NetworkManager. Cloning creates another independently
+/// zeroized secret allocation. Equality comparisons are not constant-time.
+#[non_exhaustive]
+#[derive(Clone, Default, Eq, PartialEq, ZeroizeOnDrop)]
+pub struct Passphrase(String);
+
+impl Passphrase {
+    /// Wraps an owned secret string.
+    #[must_use]
+    pub fn new(passphrase: String) -> Self {
+        Self(passphrase)
+    }
+
+    /// Returns the secret length in bytes.
+    #[must_use]
+    pub fn len(&self) -> usize {
+        self.0.len()
+    }
+
+    /// Returns `true` when the secret is empty.
+    #[must_use]
+    pub fn is_empty(&self) -> bool {
+        self.0.is_empty()
+    }
+
+    /// Borrows the plaintext secret.
+    ///
+    /// Keep the borrow short-lived and do not log or persist it. Any copy made
+    /// from this value must be cleared separately.
+    #[must_use]
+    pub fn expose_secret(&self) -> &str {
+        &self.0
+    }
+}
+
+impl fmt::Debug for Passphrase {
+    fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result {
+        formatter.write_str("Passphrase([REDACTED])")
+    }
+}
+
+impl From<String> for Passphrase {
+    fn from(passphrase: String) -> Self {
+        Self::new(passphrase)
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn debug_output_redacts_secret() {
+        let passphrase = Passphrase::new("correct horse battery staple".to_string());
+
+        let output = format!("{passphrase:?}");
+
+        assert_eq!(output, "Passphrase([REDACTED])");
+        assert!(!output.contains("correct horse battery staple"));
+    }
+
+    #[test]
+    fn exposes_secret_only_when_requested() {
+        let passphrase = Passphrase::new("secret".to_string());
+
+        assert_eq!(passphrase.expose_secret(), "secret");
+        assert_eq!(passphrase.len(), 6);
+        assert!(!passphrase.is_empty());
+    }
+}

nmrs/src/api/models/wifi.rs

@@ -1,10 +1,9 @@
 use std::fmt::Debug;
 
 use serde::{Deserialize, Serialize};
-use zeroize::ZeroizeOnDrop;
-
 use super::access_point::{AccessPoint, SecurityFeatures};
 use super::error::ConnectionError;
+use super::passphrase::Passphrase;
 use super::saved_connection::SavedConnectionBrief;
 
 /// Visible Wi-Fi access points grouped by interface and SSID for applet UIs.
@@ -860,97 +859,6 @@ impl EapOptionsBuilder {
     }
 }
 
-/// A memory-safe wrapper around [`String`] to protect secret passphrases.
-///
-/// Guarantees that the underlying memory is zeroized on [`Drop`], preventing the passphrase from
-/// leaking. Also hides the passphrase from [`Debug`].
-///
-/// # Usage
-/// Passphrase data should always be held within a [`Passphrase`] for as long as possible within
-/// its lifetime.
-///
-/// [`Passphrase::reveal`] exists for flexibility and returns the inner [`String`], but it forfeits
-/// the protection which this type provides - use with care.
-///
-/// # Examples
-/// ```
-/// use nmrs::Passphrase;
-/// use zeroize::Zeroize;
-///
-/// fn main() {
-///     let s: String = "password".to_string();
-///     let mut pass = Passphrase::from(s);
-///
-///     // Get the String back if needed.
-///     let mut revealed = pass.reveal();
-///
-///     // ...
-///
-///     // Revealed passphrases must be zeroized manually.
-///     revealed.zeroize();
-/// }
-/// ```
-#[derive(Clone, Default, Eq, PartialEq, ZeroizeOnDrop)]
-pub struct Passphrase(String);
-
-impl Passphrase {
-    pub fn new(passphrase: String) -> Self {
-        Passphrase(passphrase)
-    }
-
-    pub fn len(&self) -> usize {
-        self.0.len()
-    }
-
-    pub fn is_empty(&self) -> bool {
-        self.0.is_empty()
-    }
-
-    /// Moves the inner [`String`] outside of [`Passphrase`].
-    ///
-    /// # Security
-    /// * [`Debug`] is no longer protected.
-    /// * [`ZeroizeOnDrop`] will no longer apply since the inner [`String`] is returned so
-    ///   `zeroize()` *must* be called manually before [`Drop`] occurs:
-    /// ```
-    /// use nmrs::Passphrase;
-    /// use zeroize::Zeroize;
-    /// {
-    ///     let mut passphrase: Passphrase = Passphrase::new("password".to_string());
-    ///     let mut revealed = passphrase.reveal();
-    ///
-    ///     // ...
-    ///
-    ///     revealed.zeroize();
-    /// } // Dropped here  
-    /// ```
-    pub fn reveal(mut self) -> String {
-        std::mem::take(&mut self.0)
-    }
-
-    /// Returns a borrowed reference to the inner [`String`]. See [`Passphrase::reveal`] for moving
-    /// the inner value.
-    ///
-    /// # Security
-    /// The returned reference is **not** protected by zeroization or from being logged and should not be
-    /// cloned.
-    pub fn reveal_ref(&self) -> &str {
-        &self.0
-    }
-}
-
-impl Debug for Passphrase {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        f.debug_tuple("Passphrase").field(&"[REDACTED]").finish()
-    }
-}
-
-impl From<String> for Passphrase {
-    fn from(s: String) -> Self {
-        Passphrase(s)
-    }
-}
-
 /// Wi-Fi connection security types.
 ///
 /// Represents the authentication method for connecting to a WiFi network.