.

Author: CBenoit

devolutions-gateway/src/api/webapp.rs

@@ -10,6 +10,7 @@ use axum::{Json, Router, http};
 use axum_extra::TypedHeader;
 use axum_extra::headers::{self, HeaderMapExt as _};
 use picky::key::PrivateKey;
+use secrecy::ExposeSecret as _;
 use tap::prelude::*;
 use tower_http::services::ServeFile;
 use uuid::Uuid;

devolutions-gateway/src/config.rs

@@ -9,7 +9,7 @@ use camino::{Utf8Path, Utf8PathBuf};
 use cfg_if::cfg_if;
 use picky::key::{PrivateKey, PublicKey};
 use picky::pem::Pem;
-use secrecy::SecretString;
+use secrecy::{ExposeSecret as _, SecretString};
 use tap::prelude::*;
 use tokio::sync::Notify;
 use tokio_rustls::rustls::pki_types;
@@ -197,7 +197,7 @@ pub struct Conf {
     pub debug: dto::DebugConf,
 }
 
-#[derive(PartialEq, Debug, Clone)]
+#[derive(Debug, Clone)]
 pub struct WebAppConf {
     pub enabled: bool,
     pub authentication: WebAppAuth,
@@ -206,13 +206,13 @@ pub struct WebAppConf {
     pub static_root_path: std::path::PathBuf,
 }
 
-#[derive(PartialEq, Eq, Debug, Clone)]
+#[derive(Debug, Clone)]
 pub enum WebAppAuth {
     Custom(HashMap<String, WebAppUser>),
     None,
 }
 
-#[derive(PartialEq, Eq, Debug, Clone)]
+#[derive(Debug, Clone)]
 pub struct WebAppUser {
     pub name: String,
     /// Hash of the password, in the PHC string format
@@ -1256,7 +1256,8 @@ fn read_pfx_file(
     use picky::x509::certificate::CertType;
 
     let crypto_context = password
-        .map(|pwd| Pkcs12CryptoContext::new_with_password(pwd.expose_secret()))
+        .map(|secret| secret.expose_secret())
+        .map(Pkcs12CryptoContext::new_with_password)
         .unwrap_or_else(Pkcs12CryptoContext::new_without_password);
     let parsing_params = Pkcs12ParsingParams::default();
 
@@ -1577,6 +1578,8 @@ fn to_listener_urls(conf: &dto::ListenerConf, hostname: &str, auto_ipv6: bool) -
 pub mod dto {
     use std::collections::HashMap;
 
+    use secrecy::ExposeSecret as _;
+
     use super::*;
 
     /// Source of truth for Gateway configuration
@@ -1585,7 +1588,7 @@ pub mod dto {
     /// and is not trying to be too smart.
     ///
     /// Unstable options are subject to change
-    #[derive(PartialEq, Debug, Clone, Serialize, Deserialize)]
+    #[derive(Debug, Clone, Serialize, Deserialize)]
     #[serde(rename_all = "PascalCase")]
     pub struct ConfFile {
         /// This Gateway unique ID (e.g.: 123e4567-e89b-12d3-a456-426614174000)
@@ -1626,7 +1629,10 @@ pub mod dto {
         #[serde(alias = "PrivateKeyFile", skip_serializing_if = "Option::is_none")]
         pub tls_private_key_file: Option<Utf8PathBuf>,
         /// Password to use for decrypting the TLS private key
-        #[serde(skip_serializing_if = "Option::is_none")]
+        #[serde(
+            skip_serializing_if = "Option::is_none",
+            serialize_with = "serialize_opt_secret_string"
+        )]
         pub tls_private_key_password: Option<SecretString>,
         /// Subject name of the certificate to use for TLS
         #[serde(skip_serializing_if = "Option::is_none")]
@@ -1661,8 +1667,11 @@ pub mod dto {
         pub credssp_private_key_file: Option<Utf8PathBuf>,
 
         /// Password to use for decrypting the CredSSP private key
-        #[serde(skip_serializing_if = "Option::is_none")]
-        pub credssp_private_key_password: Option<Password>,
+        #[serde(
+            skip_serializing_if = "Option::is_none",
+            serialize_with = "serialize_opt_secret_string"
+        )]
+        pub credssp_private_key_password: Option<SecretString>,
 
         /// Listeners to launch at startup
         #[serde(default, skip_serializing_if = "Vec::is_empty")]
@@ -1811,15 +1820,16 @@ pub mod dto {
     }
 
     /// Domain user credentials.
-    #[derive(PartialEq, Eq, Debug, Clone, Serialize, Deserialize)]
+    #[derive(Debug, Clone, Serialize, Deserialize)]
     pub struct DomainUser {
         /// Username in FQDN format (e.g. "pw13@example.com").
         ///
         /// **Note**: the user's domain part must match the internal KDC realm.
         /// The KDC realm is derived from the gateway ID using the [KerberosServer::realm] method.
         pub fqdn: String,
         /// User password.
-        pub password: String,
+        #[serde(serialize_with = "serialize_secret_string")]
+        pub password: SecretString,
         /// Salt for generating the user's key.
         ///
         /// Usually, it is equal to `{REALM}{username}` (e.g. "EXAMPLEpw13").
@@ -1832,7 +1842,7 @@ pub mod dto {
 
             Self {
                 username: fqdn,
-                password,
+                password: password.expose_secret().to_owned(),
                 salt,
             }
         }
@@ -1841,7 +1851,7 @@ pub mod dto {
     /// Kerberos server config
     ///
     /// This config is used to configure the Kerberos server during RDP proxying.
-    #[derive(PartialEq, Eq, Debug, Clone, Serialize, Deserialize)]
+    #[derive(Debug, Clone, Serialize, Deserialize)]
     pub struct KerberosServer {
         /// Users credentials inside fake KDC.
         pub users: Vec<DomainUser>,
@@ -1896,7 +1906,7 @@ pub mod dto {
     }
 
     /// The Kerberos credentials-injection configuration.
-    #[derive(PartialEq, Eq, Debug, Clone, Serialize, Deserialize)]
+    #[derive(Debug, Clone, Serialize, Deserialize)]
     pub struct KerberosConfig {
         /// Kerberos server and KDC configuration.
         pub kerberos_server: KerberosServer,
@@ -1910,7 +1920,7 @@ pub mod dto {
     ///
     /// Note to developers: all options should be safe by default, never add an option
     /// that needs to be overridden manually in order to be safe.
-    #[derive(PartialEq, Eq, Debug, Clone, Serialize, Deserialize)]
+    #[derive(Debug, Clone, Serialize, Deserialize)]
     pub struct DebugConf {
         /// Dump received tokens using a `debug` statement
         #[serde(default)]
@@ -1974,7 +1984,15 @@ pub mod dto {
 
     impl DebugConf {
         pub fn is_default(&self) -> bool {
-            Self::default().eq(self)
+            !self.dump_tokens
+                && !self.disable_token_validation
+                && self.override_kdc.is_none()
+                && self.log_directives.is_none()
+                && self.capture_path.is_none()
+                && self.lib_xmf_path.is_none()
+                && !self.enable_unstable
+                && self.kerberos.is_none()
+                && self.ws_keep_alive_interval == ws_keep_alive_interval_default_value()
         }
     }
 
@@ -2355,6 +2373,23 @@ pub mod dto {
         }
     }
 
+    fn serialize_secret_string<S>(value: &SecretString, serializer: S) -> Result<S::Ok, S::Error>
+    where
+        S: serde::Serializer,
+    {
+        serializer.serialize_str(value.expose_secret())
+    }
+
+    fn serialize_opt_secret_string<S>(value: &Option<SecretString>, serializer: S) -> Result<S::Ok, S::Error>
+    where
+        S: serde::Serializer,
+    {
+        match value {
+            Some(s) => serializer.serialize_str(s.expose_secret()),
+            None => serializer.serialize_none(),
+        }
+    }
+
     impl ProxyConf {
         /// Convert this DTO to the http-client-proxy ProxyConfig.
         pub fn to_proxy_config(&self) -> http_client_proxy::ProxyConfig {

devolutions-gateway/src/credential/crypto.rs

@@ -16,8 +16,8 @@ use core::fmt;
 use std::sync::LazyLock;
 
 use anyhow::Context as _;
-use chacha20poly1305::aead::{Aead, KeyInit, OsRng};
-use chacha20poly1305::{ChaCha20Poly1305, Key, Nonce};
+use chacha20poly1305::aead::{Aead, AeadCore, KeyInit, OsRng};
+use chacha20poly1305::{ChaCha20Poly1305, Nonce};
 use parking_lot::Mutex;
 use rand::RngCore as _;
 use secrets::SecretBox;
@@ -28,7 +28,7 @@ use zeroize::{Zeroize, ZeroizeOnDrop};
 /// Initialized lazily on first access. The key material is stored in memory
 /// protected by mlock/mprotect via libsodium's SecretBox, wrapped in a Mutex
 /// for thread-safe access.
-pub static MASTER_KEY: LazyLock<Mutex<MasterKeyManager>> = LazyLock::new(|| {
+pub(super) static MASTER_KEY: LazyLock<Mutex<MasterKeyManager>> = LazyLock::new(|| {
     Mutex::new(MasterKeyManager::new().expect("failed to initialize credential encryption master key"))
 });
 
@@ -39,7 +39,7 @@ pub static MASTER_KEY: LazyLock<Mutex<MasterKeyManager>> = LazyLock::new(|| {
 /// - Protected (mprotect) with appropriate access controls
 /// - Excluded from core dumps
 /// - Zeroized on drop
-pub struct MasterKeyManager {
+pub(super) struct MasterKeyManager {
     // SecretBox provides mlock/mprotect for the key material.
     key_material: SecretBox<[u8; 32]>,
 }
@@ -64,37 +64,31 @@ impl MasterKeyManager {
     /// Encrypt a password using ChaCha20-Poly1305.
     ///
     /// Returns the nonce and ciphertext (which includes the Poly1305 auth tag).
-    pub fn encrypt(&self, plaintext: &str) -> anyhow::Result<EncryptedPassword> {
+    pub(super) fn encrypt(&self, plaintext: &str) -> anyhow::Result<EncryptedPassword> {
         let key_ref = self.key_material.borrow();
-        let cipher = ChaCha20Poly1305::new(Key::from_slice(key_ref.as_ref()));
+        let cipher = ChaCha20Poly1305::new_from_slice(key_ref.as_ref()).expect("key is exactly 32 bytes");
 
         // Generate random 96-bit nonce (12 bytes for ChaCha20-Poly1305).
-        let mut nonce_bytes = [0u8; 12];
-        OsRng.fill_bytes(&mut nonce_bytes);
-        let nonce = Nonce::from_slice(&nonce_bytes);
+        let nonce = ChaCha20Poly1305::generate_nonce(OsRng);
 
         // Encrypt (ciphertext includes 16-byte Poly1305 tag).
         let ciphertext = cipher
-            .encrypt(nonce, plaintext.as_bytes())
+            .encrypt(&nonce, plaintext.as_bytes())
             .map_err(|e| anyhow::anyhow!("encryption failed: {e}"))?;
 
-        Ok(EncryptedPassword {
-            nonce: nonce_bytes,
-            ciphertext,
-        })
+        Ok(EncryptedPassword { nonce, ciphertext })
     }
 
     /// Decrypt a password, returning a zeroize-on-drop wrapper.
     ///
     /// The returned `DecryptedPassword` should have a short lifetime.
     /// Use it immediately and let it drop to zeroize the plaintext.
-    pub fn decrypt(&self, encrypted: &EncryptedPassword) -> anyhow::Result<DecryptedPassword> {
+    pub(super) fn decrypt(&self, encrypted: &EncryptedPassword) -> anyhow::Result<DecryptedPassword> {
         let key_ref = self.key_material.borrow();
-        let cipher = ChaCha20Poly1305::new(Key::from_slice(key_ref.as_ref()));
-        let nonce = Nonce::from_slice(&encrypted.nonce);
+        let cipher = ChaCha20Poly1305::new_from_slice(key_ref.as_ref()).expect("key is exactly 32 bytes");
 
         let plaintext_bytes = cipher
-            .decrypt(nonce, encrypted.ciphertext.as_ref())
+            .decrypt(&encrypted.nonce, encrypted.ciphertext.as_ref())
             .map_err(|e| anyhow::anyhow!("decryption failed: {e}"))?;
 
         // Convert bytes to String.
@@ -113,7 +107,7 @@ impl MasterKeyManager {
 #[derive(Clone)]
 pub struct EncryptedPassword {
     /// 96-bit nonce (12 bytes) for ChaCha20-Poly1305.
-    nonce: [u8; 12],
+    nonce: Nonce,
 
     /// Ciphertext + 128-bit auth tag (plaintext_len + 16 bytes).
     ciphertext: Vec<u8>,
@@ -151,6 +145,7 @@ impl fmt::Debug for DecryptedPassword {
 }
 
 #[cfg(test)]
+#[expect(clippy::unwrap_used, reason = "test code, panics are expected")]
 mod tests {
     use super::*;
 

devolutions-gateway/src/credential/mod.rs

@@ -1,19 +1,20 @@
 mod crypto;
 
-pub use crypto::{DecryptedPassword, EncryptedPassword, MASTER_KEY};
-use secrecy::ExposeSecret;
+#[rustfmt::skip]
+pub use crypto::{DecryptedPassword, EncryptedPassword};
 
-use core::fmt;
 use std::collections::HashMap;
 use std::sync::Arc;
 
 use anyhow::Context;
 use async_trait::async_trait;
 use devolutions_gateway_task::{ShutdownSignal, Task};
 use parking_lot::Mutex;
-use serde::{de, ser};
+use secrecy::ExposeSecret as _;
 use uuid::Uuid;
 
+use self::crypto::MASTER_KEY;
+
 /// Credential at the application protocol level
 #[derive(Debug, Clone)]
 pub enum AppCredential {

devolutions-gateway/src/rd_clean_path.rs

@@ -4,6 +4,7 @@ use std::sync::Arc;
 
 use anyhow::Context as _;
 use ironrdp_connector::sspi;
+use secrecy::ExposeSecret as _;
 use ironrdp_pdu::nego;
 use ironrdp_rdcleanpath::RDCleanPathPdu;
 use tap::prelude::*;
@@ -404,7 +405,7 @@ async fn handle_with_credential_injection(
             } = user;
 
             // The username is in the FQDN format. Thus, the domain field can be empty.
-            sspi::CredentialsBuffers::AuthIdentity(sspi::AuthIdentityBuffers::from_utf8(fqdn, "", password))
+            sspi::CredentialsBuffers::AuthIdentity(sspi::AuthIdentityBuffers::from_utf8(fqdn, "", password.expose_secret()))
         });
 
         Some(sspi::KerberosServerConfig {

devolutions-gateway/src/rdp_proxy.rs

@@ -3,6 +3,7 @@ use std::sync::Arc;
 
 use anyhow::Context as _;
 use ironrdp_acceptor::credssp::CredsspProcessGenerator as CredsspServerProcessGenerator;
+use secrecy::ExposeSecret as _;
 use ironrdp_connector::credssp::CredsspProcessGenerator as CredsspClientProcessGenerator;
 use ironrdp_connector::sspi;
 use ironrdp_connector::sspi::generator::{GeneratorState, NetworkRequest};
@@ -130,8 +131,8 @@ where
                 salt: _,
             } = user;
 
-            // The username is in the FQDN format. Thus, the domain field can be empty.
-            sspi::CredentialsBuffers::AuthIdentity(sspi::AuthIdentityBuffers::from_utf8(fqdn, "", password))
+            // The username is an the FQDN format. Thus, the domain field can be empty.
+            sspi::CredentialsBuffers::AuthIdentity(sspi::AuthIdentityBuffers::from_utf8(fqdn, "", password.expose_secret()))
         });
 
         Some(sspi::KerberosServerConfig {