diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2bd38c72c..03fd3982e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -617,16 +617,17 @@ jobs: cargo_env_scripts: true - name: Configure Linux runner - if: ${{ matrix.os == 'linux' }} + if: ${{ matrix.os == 'linux' && matrix.arch != 'arm64' }} run: | sudo apt-get update - sudo apt-get -o Acquire::Retries=3 install python3-wget python3-setuptools libsystemd-dev dh-make + sudo apt-get -o Acquire::Retries=3 install python3-wget python3-setuptools libsystemd-dev dh-make libsodium-dev - name: Configure Linux (arm) runner if: ${{ matrix.os == 'linux' && matrix.arch == 'arm64' }} run: | sudo dpkg --add-architecture arm64 - sudo apt-get -o Acquire::Retries=3 install -qy binutils-aarch64-linux-gnu gcc-aarch64-linux-gnu g++-aarch64-linux-gnu qemu-user + sudo apt-get update + sudo apt-get -o Acquire::Retries=3 install -qy binutils-aarch64-linux-gnu gcc-aarch64-linux-gnu g++-aarch64-linux-gnu qemu-user libsodium-dev:arm64 rustup target add aarch64-unknown-linux-gnu echo "STRIP_EXECUTABLE=aarch64-linux-gnu-strip" >> $GITHUB_ENV @@ -663,6 +664,20 @@ jobs: Write-Output "windows_sdk_ver_bin_path=$path" | Out-File -FilePath $env:GITHUB_OUTPUT -Append -Encoding utf8 shell: pwsh + + - name: Enable mlock for production + # On Linux, libsodium-dev is installed in the configure steps above (apt-get). + # On Windows, libsodium is installed here via vcpkg (deferred to production to avoid slow builds on PRs). + if: ${{ needs.preflight.outputs.rust-profile == 'production' }} + run: | + if ($Env:RUNNER_OS -eq "Windows") { + # Install libsodium via vcpkg for the mlock feature (requires static library) + vcpkg install libsodium:x64-windows-static + echo "VCPKG_ROOT=$Env:VCPKG_INSTALLATION_ROOT" >> $Env:GITHUB_ENV + } + echo "CARGO_FEATURES=mlock" >> $Env:GITHUB_ENV + shell: pwsh + - name: Build run: | if ($Env:RUNNER_OS -eq "Linux") { diff --git a/Cargo.lock b/Cargo.lock index 760a0302d..41aff5206 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8,6 +8,16 @@ version = "2.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "320119579fcad9c21884f5c4861d16174d0e06250625266f50fe6898340abefa" +[[package]] +name = "aead" +version = "0.5.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d122413f284cf2d62fb1b7db97e02edb8cda96d769b16e443a4f6195e35662b0" +dependencies = [ + "crypto-common 0.1.6", + "generic-array", +] + [[package]] name = "aead" version = "0.6.0-rc.2" @@ -46,7 +56,7 @@ version = "0.11.0-rc.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0686ba04dc80c816104c96cd7782b748f6ad58c5dd4ee619ff3258cf68e83d54" dependencies = [ - "aead", + "aead 0.6.0-rc.2", "aes 0.9.0-rc.1", "cipher 0.5.0-rc.1", "ctr", @@ -867,6 +877,30 @@ version = "0.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "613afe47fcd5fac7ccf1db93babcb082c5994d996f20b8b159f2ad1658eb5724" +[[package]] +name = "chacha20" +version = "0.9.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c3613f74bd2eac03dad61bd53dbe620703d4371614fe0bc3b9f04dd36fe4e818" +dependencies = [ + "cfg-if", + "cipher 0.4.4", + "cpufeatures", +] + +[[package]] +name = "chacha20poly1305" +version = "0.10.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "10cd79432192d1c0f4e1a0fef9527696cc039165d729fb41b3f4f4f354c2dc35" +dependencies = [ + "aead 0.5.2", + "chacha20", + "cipher 0.4.4", + "poly1305", + "zeroize", +] + [[package]] name = "chrono" version = "0.4.43" @@ -916,6 +950,7 @@ checksum = "773f3b9af64447d2ce9850330c473515014aa235e6a783b02db81ff39e4a3dad" dependencies = [ "crypto-common 0.1.6", "inout 0.1.4", + "zeroize", ] [[package]] @@ -1170,6 +1205,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1bfb12502f3fc46cca1bb51ac28df9d618d813cdc3d2f25b9fe775a34af26bb3" dependencies = [ "generic-array", + "rand_core 0.6.4", "typenum", ] @@ -1215,7 +1251,7 @@ dependencies = [ "libloading", "log", "paste", - "secrecy", + "secrecy 0.8.0", ] [[package]] @@ -1448,6 +1484,7 @@ dependencies = [ "camino", "ceviche", "cfg-if", + "chacha20poly1305", "devolutions-agent-shared", "devolutions-gateway-generators", "devolutions-gateway-task", @@ -1487,10 +1524,13 @@ dependencies = [ "pin-project-lite 0.2.16", "portpicker", "proptest", + "rand 0.8.5", "reqwest", "rstest", "rustls-cng", "rustls-native-certs", + "secrecy 0.10.3", + "secrets", "serde", "serde-querystring", "serde_json", @@ -4486,6 +4526,12 @@ version = "11.1.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d6790f58c7ff633d8771f42965289203411a5e5c68388703c06e14f24770b41e" +[[package]] +name = "opaque-debug" +version = "0.3.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c08d65885ee38876c4f86fa503fb49d7b507c2b62552df7c70b2fce627e06381" + [[package]] name = "openssl" version = "0.10.75" @@ -4742,7 +4788,7 @@ version = "7.0.0-rc.20" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "4cdc52be663aebd70d7006ae305c87eb32a2b836d6c2f26f7e384f845d80b621" dependencies = [ - "aead", + "aead 0.6.0-rc.2", "aes 0.9.0-rc.1", "aes-gcm", "aes-kw", @@ -4801,7 +4847,7 @@ dependencies = [ "spki 0.8.0-rc.4", "thiserror 2.0.18", "time", - "universal-hash", + "universal-hash 0.6.0-rc.2", "x25519-dalek", "zeroize", ] @@ -5028,6 +5074,17 @@ dependencies = [ "windows-sys 0.61.2", ] +[[package]] +name = "poly1305" +version = "0.8.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8159bd90725d2df49889a078b54f4f79e87f1f8a8444194cdca81d38f5393abf" +dependencies = [ + "cpufeatures", + "opaque-debug", + "universal-hash 0.5.1", +] + [[package]] name = "polyval" version = "0.7.0-rc.2" @@ -5036,7 +5093,7 @@ checksum = "1ffd40cc99d0fbb02b4b3771346b811df94194bc103983efa0203c8893755085" dependencies = [ "cfg-if", "cpufeatures", - "universal-hash", + "universal-hash 0.6.0-rc.2", ] [[package]] @@ -6035,6 +6092,27 @@ dependencies = [ "zeroize", ] +[[package]] +name = "secrecy" +version = "0.10.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e891af845473308773346dc847b2c23ee78fe442e0472ac50e22a18a93d3ae5a" +dependencies = [ + "serde", + "zeroize", +] + +[[package]] +name = "secrets" +version = "1.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f51745a213c4a2acabad80cd511e40376996bc83db6ceb4ebc7853d41c597988" +dependencies = [ + "libc", + "pkg-config", + "vcpkg", +] + [[package]] name = "security-framework" version = "2.11.1" @@ -7650,6 +7728,16 @@ version = "0.2.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ebc1c04c71510c7f702b52b7c350734c9ff1295c464a03335b00bb84fc54f853" +[[package]] +name = "universal-hash" +version = "0.5.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fc1de2c688dc15305988b563c3854064043356019f97a4b46276fe734c4f07ea" +dependencies = [ + "crypto-common 0.1.6", + "subtle", +] + [[package]] name = "universal-hash" version = "0.6.0-rc.2" diff --git a/README.md b/README.md index f61fefe19..31709e049 100644 --- a/README.md +++ b/README.md @@ -20,12 +20,24 @@ immediately, without going through the acceptance testing process of our quality ### From sources -Ensure that you have [the Rust toolchain installed][install_rust], then clone this repository and run: +Ensure that you have [the Rust toolchain installed][install_rust] and then clone this repository and run: ```shell cargo install --path ./devolutions-gateway ``` +To enable enhanced in-memory credential protection (mlock via libsodium), build with the `mlock` feature: + +```shell +cargo install --path ./devolutions-gateway --features mlock +``` + +> **Note:** The `mlock` feature requires [libsodium][libsodium] to be installed. +> On Windows, it is found automatically via vcpkg. +> On Linux and macOS, install it using your system package manager (e.g., `apt install libsodium-dev` or `brew install libsodium`). +> Production builds should always include the `mlock` feature. +> Without it, a startup warning is emitted in release builds. + ## Configuration Devolutions Gateway is configured using a JSON document. @@ -339,6 +351,7 @@ See the dedicated [README.md file](./.github/workflows/README.md) in the `workfl [official_website]: https://devolutions.net/gateway/download/ [github_release]: https://github.com/Devolutions/devolutions-gateway/releases [install_rust]: https://www.rust-lang.org/tools/install +[libsodium]: https://libsodium.org/ [psmodule]: https://www.powershellgallery.com/packages/DevolutionsGateway/ [rustls]: https://crates.io/crates/rustls [microsoft_tls]: https://learn.microsoft.com/en-us/windows-server/security/tls/tls-registry-settings diff --git a/devolutions-gateway/Cargo.toml b/devolutions-gateway/Cargo.toml index b8bcf4cb4..3ec30064f 100644 --- a/devolutions-gateway/Cargo.toml +++ b/devolutions-gateway/Cargo.toml @@ -14,6 +14,7 @@ workspace = true [features] default = [] +mlock = ["dep:secrets"] openapi = ["dep:utoipa"] [dependencies] @@ -74,6 +75,10 @@ bitflags = "2.9" # Security, crypto… picky = { version = "7.0.0-rc.15", default-features = false, features = ["jose", "x509", "pkcs12", "time_conversion"] } zeroize = { version = "1.8", features = ["derive"] } +chacha20poly1305 = "0.10" +secrets = { version = "1.2", optional = true } +secrecy = { version = "0.10", features = ["serde"] } +rand = "0.8" multibase = "0.9" argon2 = { version = "0.5", features = ["std"] } x509-cert = { version = "0.2", default-features = false, features = ["std"] } diff --git a/devolutions-gateway/src/api/preflight.rs b/devolutions-gateway/src/api/preflight.rs index 256fb80c1..24929b5f1 100644 --- a/devolutions-gateway/src/api/preflight.rs +++ b/devolutions-gateway/src/api/preflight.rs @@ -11,7 +11,7 @@ use uuid::Uuid; use crate::DgwState; use crate::config::Conf; -use crate::credential::{AppCredentialMapping, CredentialStoreHandle}; +use crate::credential::{CredentialStoreHandle, InsertError}; use crate::extract::PreflightScope; use crate::http::HttpError; use crate::session::SessionMessageSender; @@ -45,7 +45,7 @@ struct ProvisionTokenParams { struct ProvisionCredentialsParams { token: String, #[serde(flatten)] - mapping: AppCredentialMapping, + mapping: crate::credential::CleartextAppCredentialMapping, time_to_live: Option, } @@ -340,7 +340,14 @@ async fn handle_operation( let previous_entry = credential_store .insert(token, mapping, time_to_live) .inspect_err(|error| warn!(%operation.id, error = format!("{error:#}"), "Failed to insert credentials")) - .map_err(|e| PreflightError::new(PreflightAlertStatus::InvalidParams, format!("{e:#}")))?; + .map_err(|e| match e { + InsertError::InvalidToken(_) => { + PreflightError::new(PreflightAlertStatus::InvalidParams, format!("{e:#}")) + } + InsertError::Internal(_) => { + PreflightError::new(PreflightAlertStatus::InternalServerError, format!("{e:#}")) + } + })?; if previous_entry.is_some() { outputs.push(PreflightOutput { diff --git a/devolutions-gateway/src/api/webapp.rs b/devolutions-gateway/src/api/webapp.rs index 86ed8db60..2c6b99f89 100644 --- a/devolutions-gateway/src/api/webapp.rs +++ b/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; diff --git a/devolutions-gateway/src/config.rs b/devolutions-gateway/src/config.rs index 37f8eaef6..5ca9fc49f 100644 --- a/devolutions-gateway/src/config.rs +++ b/devolutions-gateway/src/config.rs @@ -9,6 +9,7 @@ use camino::{Utf8Path, Utf8PathBuf}; use cfg_if::cfg_if; use picky::key::{PrivateKey, PublicKey}; use picky::pem::Pem; +use secrecy::{ExposeSecret as _, SecretString}; use tap::prelude::*; use tokio::sync::Notify; use tokio_rustls::rustls::pki_types; @@ -17,7 +18,6 @@ use url::Url; use uuid::Uuid; use crate::SYSTEM_LOGGER; -use crate::credential::Password; use crate::listener::ListenerUrls; use crate::target_addr::TargetAddr; use crate::token::Subkey; @@ -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,17 +206,17 @@ pub struct WebAppConf { pub static_root_path: std::path::PathBuf, } -#[derive(PartialEq, Eq, Debug, Clone)] +#[derive(Debug, Clone)] pub enum WebAppAuth { Custom(HashMap), None, } -#[derive(PartialEq, Eq, Debug, Clone)] +#[derive(Debug, Clone)] pub struct WebAppUser { pub name: String, /// Hash of the password, in the PHC string format - pub password_hash: Password, + pub password_hash: SecretString, } /// AI Router configuration (experimental) @@ -1243,7 +1243,7 @@ fn generate_self_signed_certificate( fn read_pfx_file( path: &Utf8Path, - password: Option<&Password>, + password: Option<&SecretString>, ) -> anyhow::Result<( Vec>, pki_types::PrivateKeyDer<'static>, @@ -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,8 +1629,11 @@ pub mod dto { #[serde(alias = "PrivateKeyFile", skip_serializing_if = "Option::is_none")] pub tls_private_key_file: Option, /// Password to use for decrypting the TLS private key - #[serde(skip_serializing_if = "Option::is_none")] - pub tls_private_key_password: Option, + #[serde( + skip_serializing_if = "Option::is_none", + serialize_with = "serialize_opt_secret_string" + )] + pub tls_private_key_password: Option, /// Subject name of the certificate to use for TLS #[serde(skip_serializing_if = "Option::is_none")] pub tls_certificate_subject_name: Option, @@ -1661,8 +1667,11 @@ pub mod dto { pub credssp_private_key_file: Option, /// Password to use for decrypting the CredSSP private key - #[serde(skip_serializing_if = "Option::is_none")] - pub credssp_private_key_password: Option, + #[serde( + skip_serializing_if = "Option::is_none", + serialize_with = "serialize_opt_secret_string" + )] + pub credssp_private_key_password: Option, /// Listeners to launch at startup #[serde(default, skip_serializing_if = "Vec::is_empty")] @@ -1811,7 +1820,7 @@ 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"). /// @@ -1819,7 +1828,8 @@ pub mod dto { /// 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, @@ -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(value: &SecretString, serializer: S) -> Result + where + S: serde::Serializer, + { + serializer.serialize_str(value.expose_secret()) + } + + fn serialize_opt_secret_string(value: &Option, serializer: S) -> Result + 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 { diff --git a/devolutions-gateway/src/credential/crypto.rs b/devolutions-gateway/src/credential/crypto.rs new file mode 100644 index 000000000..c74c9f542 --- /dev/null +++ b/devolutions-gateway/src/credential/crypto.rs @@ -0,0 +1,235 @@ +//! In-memory credential encryption using ChaCha20-Poly1305. +//! +//! This module provides encryption-at-rest for passwords stored in the credential store. +//! A randomly generated 256-bit master key is stored in a zeroize-on-drop wrapper. +//! When the `mlock` feature is enabled, libsodium's memory locking facilities +//! (mlock/mprotect) are additionally used to prevent the key from being swapped to +//! disk or appearing in core dumps. +//! +//! ## Security Properties +//! +//! - Passwords encrypted at rest in regular heap memory +//! - Decryption on-demand into short-lived zeroized buffers +//! - ChaCha20-Poly1305 provides authenticated encryption +//! - Random 96-bit nonces prevent nonce reuse +//! - Master key zeroized on drop +//! - With `mlock` feature: Master key stored in mlock'd memory (excluded from core dumps) + +use core::fmt; +use std::sync::LazyLock; + +use anyhow::Context as _; +use chacha20poly1305::aead::{Aead, AeadCore, KeyInit, OsRng}; +use chacha20poly1305::{ChaCha20Poly1305, Nonce}; +use parking_lot::Mutex; +use rand::RngCore as _; +use secrecy::SecretString; +#[cfg(feature = "mlock")] +use secrets::SecretBox; +#[cfg(not(feature = "mlock"))] +use zeroize::Zeroizing; + +/// Global master key for credential encryption. +/// +/// Initialized lazily on first access. The key material is wrapped in a Mutex +/// for thread-safe access. With the `mlock` feature, key memory is additionally +/// protected by mlock/mprotect via libsodium's SecretBox. +pub(super) static MASTER_KEY: LazyLock> = LazyLock::new(|| { + Mutex::new(MasterKeyManager::new().expect("failed to initialize credential encryption master key")) +}); + +/// Manages the master encryption key. +/// +/// The key is zeroized on drop. When the `mlock` feature is enabled, the key +/// memory is additionally: +/// - Locked (mlock) to prevent swapping to disk +/// - Protected (mprotect) with appropriate access controls +/// - Excluded from core dumps +pub(super) struct MasterKeyManager { + #[cfg(feature = "mlock")] + key_material: SecretBox<[u8; 32]>, + #[cfg(not(feature = "mlock"))] + key_material: Zeroizing<[u8; 32]>, +} + +impl MasterKeyManager { + /// Generate a new random 256-bit master key. + /// + /// # Errors + /// + /// Returns error if secure memory allocation fails or RNG fails. + fn new() -> anyhow::Result { + #[cfg(feature = "mlock")] + let key_material = SecretBox::try_new(|key_bytes: &mut [u8; 32]| { + OsRng.fill_bytes(key_bytes); + Ok::<_, anyhow::Error>(()) + }) + .context("failed to allocate secure memory for master key")?; + + #[cfg(not(feature = "mlock"))] + let key_material = { + let mut key = Zeroizing::new([0u8; 32]); + OsRng.fill_bytes(key.as_mut()); + key + }; + + Ok(Self { key_material }) + } + + /// Encrypt a password using ChaCha20-Poly1305. + /// + /// Returns the nonce and ciphertext (which includes the Poly1305 auth tag). + pub(super) fn encrypt(&self, plaintext: &str) -> anyhow::Result { + #[cfg(feature = "mlock")] + let key_ref = self.key_material.borrow(); + #[cfg(feature = "mlock")] + let key_bytes: &[u8] = key_ref.as_ref(); + + #[cfg(not(feature = "mlock"))] + let key_bytes: &[u8] = self.key_material.as_ref(); + + let cipher = ChaCha20Poly1305::new_from_slice(key_bytes).expect("key is exactly 32 bytes"); + + // Generate random 96-bit nonce (12 bytes for ChaCha20-Poly1305). + let nonce = ChaCha20Poly1305::generate_nonce(OsRng); + + // Encrypt (ciphertext includes 16-byte Poly1305 tag). + let ciphertext = cipher + .encrypt(&nonce, plaintext.as_bytes()) + .ok() + .context("AEAD encryption failed")?; + + Ok(EncryptedPassword { nonce, ciphertext }) + } + + /// Decrypt a password, returning a `SecretString` that zeroizes on drop. + /// + /// The returned `SecretString` should have a short lifetime. + /// Use it immediately and let it drop to zeroize the plaintext. + pub(super) fn decrypt(&self, encrypted: &EncryptedPassword) -> anyhow::Result { + #[cfg(feature = "mlock")] + let key_ref = self.key_material.borrow(); + #[cfg(feature = "mlock")] + let key_bytes: &[u8] = key_ref.as_ref(); + + #[cfg(not(feature = "mlock"))] + let key_bytes: &[u8] = self.key_material.as_ref(); + + let cipher = ChaCha20Poly1305::new_from_slice(key_bytes).expect("key is exactly 32 bytes"); + + let plaintext_bytes = cipher + .decrypt(&encrypted.nonce, encrypted.ciphertext.as_ref()) + .ok() + .context("AEAD decryption failed")?; + + // Convert bytes to String. + let plaintext = String::from_utf8(plaintext_bytes).context("decrypted password is not valid UTF-8")?; + + Ok(SecretString::from(plaintext)) + } +} + +// Note: With `mlock` feature, SecretBox handles secure zeroization and munlock automatically on drop. +// Without `mlock` feature, Zeroizing handles secure zeroization on drop (no mlock). + +/// Encrypted password stored in heap memory. +/// +/// Contains the nonce and ciphertext (including Poly1305 authentication tag). +/// This can be safely stored in regular memory as it's encrypted. +#[derive(Clone)] +pub struct EncryptedPassword { + /// 96-bit nonce (12 bytes) for ChaCha20-Poly1305. + nonce: Nonce, + + /// Ciphertext + 128-bit auth tag (plaintext_len + 16 bytes). + ciphertext: Vec, +} + +impl fmt::Debug for EncryptedPassword { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("EncryptedPassword") + .field("ciphertext_len", &self.ciphertext.len()) + .finish_non_exhaustive() + } +} + +#[cfg(test)] +#[expect(clippy::unwrap_used, reason = "test code, panics are expected")] +mod tests { + use secrecy::ExposeSecret as _; + + use super::*; + + #[test] + fn test_encrypt_decrypt_roundtrip() { + let key_manager = MasterKeyManager::new().unwrap(); + let plaintext = "my-secret-password"; + + let encrypted = key_manager.encrypt(plaintext).unwrap(); + let decrypted = key_manager.decrypt(&encrypted).unwrap(); + + assert_eq!(decrypted.expose_secret(), plaintext); + } + + #[test] + fn test_different_nonces() { + let key_manager = MasterKeyManager::new().unwrap(); + let plaintext = "password"; + + let encrypted1 = key_manager.encrypt(plaintext).unwrap(); + let encrypted2 = key_manager.encrypt(plaintext).unwrap(); + + // Same plaintext should produce different ciphertexts (different nonces). + assert_ne!(encrypted1.nonce, encrypted2.nonce); + assert_ne!(encrypted1.ciphertext, encrypted2.ciphertext); + } + + #[test] + fn test_wrong_key_fails_decryption() { + let key_manager1 = MasterKeyManager::new().unwrap(); + let key_manager2 = MasterKeyManager::new().unwrap(); + + let encrypted = key_manager1.encrypt("secret").unwrap(); + + // Decryption with different key should fail. + assert!(key_manager2.decrypt(&encrypted).is_err()); + } + + #[test] + fn test_corrupted_ciphertext_fails() { + let key_manager = MasterKeyManager::new().unwrap(); + let mut encrypted = key_manager.encrypt("secret").unwrap(); + + // Corrupt the ciphertext. + encrypted.ciphertext[0] ^= 0xFF; + + // Should fail authentication. + assert!(key_manager.decrypt(&encrypted).is_err()); + } + + #[test] + fn test_empty_password() { + let key_manager = MasterKeyManager::new().unwrap(); + let encrypted = key_manager.encrypt("").unwrap(); + let decrypted = key_manager.decrypt(&encrypted).unwrap(); + assert_eq!(decrypted.expose_secret(), ""); + } + + #[test] + fn test_unicode_password() { + let key_manager = MasterKeyManager::new().unwrap(); + let plaintext = "пароль-密码-كلمة السر"; + let encrypted = key_manager.encrypt(plaintext).unwrap(); + let decrypted = key_manager.decrypt(&encrypted).unwrap(); + assert_eq!(decrypted.expose_secret(), plaintext); + } + + #[test] + fn test_global_master_key() { + // Test that the global MASTER_KEY works. + let plaintext = "test-password"; + let encrypted = MASTER_KEY.lock().encrypt(plaintext).unwrap(); + let decrypted = MASTER_KEY.lock().decrypt(&encrypted).unwrap(); + assert_eq!(decrypted.expose_secret(), plaintext); + } +} diff --git a/devolutions-gateway/src/credential.rs b/devolutions-gateway/src/credential/mod.rs similarity index 50% rename from devolutions-gateway/src/credential.rs rename to devolutions-gateway/src/credential/mod.rs index 9779f33bb..166be9412 100644 --- a/devolutions-gateway/src/credential.rs +++ b/devolutions-gateway/src/credential/mod.rs @@ -1,29 +1,119 @@ -use core::fmt; +mod crypto; + +#[rustfmt::skip] +pub use crypto::EncryptedPassword; + use std::collections::HashMap; +use std::fmt; 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; + +/// Error returned by [`CredentialStoreHandle::insert`]. +#[derive(Debug)] +pub enum InsertError { + /// The provided token is invalid (e.g., missing or malformed JTI). + /// + /// This is a client-side error: the caller supplied bad input. + InvalidToken(anyhow::Error), + /// An internal error occurred (e.g., encryption failure). + Internal(anyhow::Error), +} + +impl fmt::Display for InsertError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::InvalidToken(e) => e.fmt(f), + Self::Internal(e) => e.fmt(f), + } + } +} + +impl std::error::Error for InsertError {} + /// Credential at the application protocol level +#[derive(Debug, Clone)] +pub enum AppCredential { + UsernamePassword { + username: String, + password: EncryptedPassword, + }, +} + +impl AppCredential { + /// Decrypt the password using the global master key. + /// + /// Returns the username and a short-lived decrypted password that zeroizes on drop. + pub fn decrypt_password(&self) -> anyhow::Result<(String, secrecy::SecretString)> { + match self { + AppCredential::UsernamePassword { username, password } => { + let decrypted = MASTER_KEY.lock().decrypt(password)?; + Ok((username.clone(), decrypted)) + } + } + } +} + +/// Application protocol level credential mapping +#[derive(Debug, Clone)] +pub struct AppCredentialMapping { + pub proxy: AppCredential, + pub target: AppCredential, +} + +/// Cleartext credential received from the API, used for deserialization only. +/// +/// Passwords are encrypted and stored as [`AppCredential`] inside the credential store. +/// This type is never stored directly — hand it to [`CredentialStoreHandle::insert`]. #[derive(Debug, Deserialize)] #[serde(tag = "kind")] -pub enum AppCredential { +pub enum CleartextAppCredential { #[serde(rename = "username-password")] - UsernamePassword { username: String, password: Password }, + UsernamePassword { + username: String, + password: secrecy::SecretString, + }, +} + +impl CleartextAppCredential { + fn encrypt(self) -> anyhow::Result { + match self { + CleartextAppCredential::UsernamePassword { username, password } => { + let encrypted = MASTER_KEY.lock().encrypt(password.expose_secret())?; + Ok(AppCredential::UsernamePassword { + username, + password: encrypted, + }) + } + } + } } -/// Application protocol level credential mapping +/// Cleartext credential mapping received from the API, used for deserialization only. +/// +/// Passwords are encrypted on write. Hand this directly to [`CredentialStoreHandle::insert`]. #[derive(Debug, Deserialize)] -pub struct AppCredentialMapping { +pub struct CleartextAppCredentialMapping { #[serde(rename = "proxy_credential")] - pub proxy: AppCredential, + pub proxy: CleartextAppCredential, #[serde(rename = "target_credential")] - pub target: AppCredential, + pub target: CleartextAppCredential, +} + +impl CleartextAppCredentialMapping { + fn encrypt(self) -> anyhow::Result { + Ok(AppCredentialMapping { + proxy: self.proxy.encrypt()?, + target: self.target.encrypt()?, + }) + } } #[derive(Debug, Clone)] @@ -43,9 +133,13 @@ impl CredentialStoreHandle { pub fn insert( &self, token: String, - mapping: Option, + mapping: Option, time_to_live: time::Duration, - ) -> anyhow::Result> { + ) -> Result, InsertError> { + let mapping = mapping + .map(CleartextAppCredentialMapping::encrypt) + .transpose() + .map_err(InsertError::Internal)?; self.0.lock().insert(token, mapping, time_to_live) } @@ -80,8 +174,10 @@ impl CredentialStore { token: String, mapping: Option, time_to_live: time::Duration, - ) -> anyhow::Result> { - let jti = crate::token::extract_jti(&token).context("failed to extract token ID")?; + ) -> Result, InsertError> { + let jti = crate::token::extract_jti(&token) + .context("failed to extract token ID") + .map_err(InsertError::InvalidToken)?; let entry = CredentialEntry { token, @@ -99,78 +195,6 @@ impl CredentialStore { } } -#[derive(PartialEq, Eq, Clone, zeroize::Zeroize)] -pub struct Password(String); - -impl Password { - /// Do not copy the return value without wrapping into some "Zeroize"able structure - pub fn expose_secret(&self) -> &str { - &self.0 - } -} - -impl From<&str> for Password { - fn from(value: &str) -> Self { - Self(value.to_owned()) - } -} - -impl From for Password { - fn from(value: String) -> Self { - Self(value) - } -} - -impl fmt::Debug for Password { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.debug_struct("Password").finish_non_exhaustive() - } -} - -impl<'de> de::Deserialize<'de> for Password { - fn deserialize(deserializer: D) -> Result - where - D: serde::Deserializer<'de>, - { - struct V; - - impl de::Visitor<'_> for V { - type Value = Password; - - fn expecting(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { - formatter.write_str("a string") - } - - fn visit_string(self, v: String) -> Result - where - E: de::Error, - { - Ok(Password(v)) - } - - fn visit_str(self, v: &str) -> Result - where - E: de::Error, - { - Ok(Password(v.to_owned())) - } - } - - let password = deserializer.deserialize_string(V)?; - - Ok(password) - } -} - -impl ser::Serialize for Password { - fn serialize(&self, serializer: S) -> Result - where - S: serde::Serializer, - { - serializer.serialize_str(&self.0) - } -} - pub struct CleanupTask { pub handle: CredentialStoreHandle, } diff --git a/devolutions-gateway/src/rd_clean_path.rs b/devolutions-gateway/src/rd_clean_path.rs index 436b690c0..6d4614b5e 100644 --- a/devolutions-gateway/src/rd_clean_path.rs +++ b/devolutions-gateway/src/rd_clean_path.rs @@ -6,6 +6,7 @@ use anyhow::Context as _; use ironrdp_connector::sspi; use ironrdp_pdu::nego; use ironrdp_rdcleanpath::RDCleanPathPdu; +use secrecy::ExposeSecret as _; use tap::prelude::*; use thiserror::Error; use tokio::io::{AsyncRead, AsyncReadExt as _, AsyncWrite, AsyncWriteExt as _}; @@ -404,7 +405,11 @@ 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 { diff --git a/devolutions-gateway/src/rdp_proxy.rs b/devolutions-gateway/src/rdp_proxy.rs index 0da9569dd..b3dc466a7 100644 --- a/devolutions-gateway/src/rdp_proxy.rs +++ b/devolutions-gateway/src/rdp_proxy.rs @@ -7,6 +7,7 @@ use ironrdp_connector::credssp::CredsspProcessGenerator as CredsspClientProcessG use ironrdp_connector::sspi; use ironrdp_connector::sspi::generator::{GeneratorState, NetworkRequest}; use ironrdp_pdu::{mcs, nego, x224}; +use secrecy::ExposeSecret as _; use tokio::io::{AsyncRead, AsyncWrite, AsyncWriteExt}; use typed_builder::TypedBuilder; @@ -131,7 +132,11 @@ where } = 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 { @@ -402,14 +407,17 @@ where { use ironrdp_tokio::FramedWrite as _; - let credentials = match credentials { - crate::credential::AppCredential::UsernamePassword { username, password } => { - ironrdp_connector::Credentials::UsernamePassword { - username: username.clone(), - password: password.expose_secret().to_owned(), - } - } + // Decrypt password into short-lived buffer. + let (username, decrypted_password) = credentials + .decrypt_password() + .context("failed to decrypt credentials")?; + + let credentials = ironrdp_connector::Credentials::UsernamePassword { + username, + password: decrypted_password.expose_secret().to_owned(), }; + // decrypted_password drops here, zeroizing its buffer; note: a copy of the plaintext + // remains in `credentials` above, which is a regular String (downstream API limitation). let (mut sequence, mut ts_request) = ironrdp_connector::credssp::CredsspSequence::init( credentials, @@ -558,14 +566,19 @@ where where S: ironrdp_tokio::FramedRead + ironrdp_tokio::FramedWrite, { - let crate::credential::AppCredential::UsernamePassword { username, password } = credentials; + // Decrypt password into short-lived buffer. + let (username, decrypted_password) = credentials + .decrypt_password() + .context("failed to decrypt credentials")?; - let username = sspi::Username::parse(username).context("invalid username")?; + let username = sspi::Username::parse(&username).context("invalid username")?; let identity = sspi::AuthIdentity { username, - password: password.expose_secret().to_owned().into(), + password: decrypted_password.expose_secret().to_owned().into(), }; + // decrypted_password drops here, zeroizing its buffer; note: a copy of the plaintext + // remains in `identity` above (downstream API limitation). let mut sequence = ironrdp_acceptor::credssp::CredsspSequence::init( &identity, diff --git a/devolutions-gateway/src/service.rs b/devolutions-gateway/src/service.rs index 64dde91c4..4b28cbf42 100644 --- a/devolutions-gateway/src/service.rs +++ b/devolutions-gateway/src/service.rs @@ -49,6 +49,14 @@ impl GatewayService { info!(version = env!("CARGO_PKG_VERSION")); + // Warn in release builds if the mlock security feature is not compiled in. + #[cfg(all(not(feature = "mlock"), not(debug_assertions)))] + warn!( + "Credential encryption master key does not have mlock memory protection. \ + Rebuild with the `mlock` feature (requires libsodium) to prevent key exposure \ + in core dumps and swap." + ); + let conf_file = conf_handle.get_conf_file(); trace!(?conf_file);