From fb48cc8e120900b838f43601648213585740f9fd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20CORTIER?= Date: Wed, 25 Feb 2026 12:41:53 +0900 Subject: [PATCH 01/17] feat(dgw): encrypt in-memory credentials with optional mlock protection Add ChaCha20-Poly1305 encryption for credentials stored in the credential store. Passwords are encrypted at rest with a randomly generated 256-bit master key that is zeroized on drop. When built with the `mlock` cargo feature, the master key is additionally protected via libsodium's mlock/mprotect facilities, preventing exposure in memory dumps or swap. A startup warning is emitted in release builds when mlock is not enabled. Issue: DGW-326 --- .github/workflows/ci.yml | 39 ++- Cargo.lock | 98 +++++++- README.md | 15 +- ci/setup-linux-build-deps.ps1 | 91 +++++++ devolutions-gateway/Cargo.toml | 5 + devolutions-gateway/src/api/preflight.rs | 13 +- devolutions-gateway/src/api/webapp.rs | 1 + devolutions-gateway/src/config.rs | 73 ++++-- devolutions-gateway/src/credential/crypto.rs | 235 ++++++++++++++++++ .../src/{credential.rs => credential/mod.rs} | 192 +++++++------- devolutions-gateway/src/rd_clean_path.rs | 7 +- devolutions-gateway/src/rdp_proxy.rs | 35 ++- devolutions-gateway/src/service.rs | 8 + 13 files changed, 666 insertions(+), 146 deletions(-) create mode 100644 ci/setup-linux-build-deps.ps1 create mode 100644 devolutions-gateway/src/credential/crypto.rs rename devolutions-gateway/src/{credential.rs => credential/mod.rs} (50%) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 93c7a54f1..000649248 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -618,17 +618,8 @@ jobs: - name: Configure Linux runner if: ${{ matrix.os == 'linux' }} - run: | - sudo apt-get update - sudo apt-get -o Acquire::Retries=3 install python3-wget python3-setuptools libsystemd-dev dh-make - - - 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 - rustup target add aarch64-unknown-linux-gnu - echo "STRIP_EXECUTABLE=aarch64-linux-gnu-strip" >> $GITHUB_ENV + run: ./ci/setup-linux-build-deps.ps1 -Architecture ${{ matrix.arch }} -InstallLibsodium + shell: pwsh - name: Install fpm if: ${{ matrix.os == 'Linux' }} @@ -663,6 +654,19 @@ 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 (in the script). + # 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") { @@ -871,17 +875,8 @@ jobs: - name: Configure Linux runner if: ${{ matrix.os == 'linux' }} - run: | - sudo apt-get update - sudo apt-get -o Acquire::Retries=3 install python3-wget python3-setuptools libsystemd-dev dh-make - - - 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 - rustup target add aarch64-unknown-linux-gnu - echo "STRIP_EXECUTABLE=aarch64-linux-gnu-strip" >> $GITHUB_ENV + run: ./ci/setup-linux-build-deps.ps1 -Architecture ${{ matrix.arch }} + shell: pwsh - name: Install fpm if: ${{ matrix.os == 'Linux' }} diff --git a/Cargo.lock b/Cargo.lock index a6b2c1567..04802c462 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.44" @@ -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.17", "portpicker", "proptest", + "rand 0.8.5", "reqwest", "rstest", "rustls-cng", "rustls-native-certs", + "secrecy 0.10.3", + "secrets", "serde", "serde-querystring", "serde_json", @@ -4495,6 +4535,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.76" @@ -4751,7 +4797,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", @@ -4810,7 +4856,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", ] @@ -5037,6 +5083,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" @@ -5045,7 +5102,7 @@ checksum = "1ffd40cc99d0fbb02b4b3771346b811df94194bc103983efa0203c8893755085" dependencies = [ "cfg-if", "cpufeatures", - "universal-hash", + "universal-hash 0.6.0-rc.2", ] [[package]] @@ -6044,6 +6101,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 = "3.5.1" @@ -7647,6 +7725,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/ci/setup-linux-build-deps.ps1 b/ci/setup-linux-build-deps.ps1 new file mode 100644 index 000000000..19ef27f1b --- /dev/null +++ b/ci/setup-linux-build-deps.ps1 @@ -0,0 +1,91 @@ +#!/usr/bin/env pwsh + +param( + [Parameter(Mandatory = $true)] + [ValidateSet('x86_64', 'arm64')] + [string] $Architecture, + + [switch] $InstallLibsodium +) + +$ErrorActionPreference = 'Stop' + +function Get-OsReleaseValue { + param( + [Parameter(Mandatory = $true)] + [string] $Name + ) + + $match = Get-Content '/etc/os-release' | Select-String -Pattern "^${Name}=(.*)$" | Select-Object -First 1 + if (-not $match) { + throw "missing ${Name} in /etc/os-release" + } + + return $match.Matches[0].Groups[1].Value.Trim('"') +} + +function Set-UbuntuMultiarchSources { + $versionCodename = Get-OsReleaseValue -Name 'VERSION_CODENAME' + $sourcesFile = '/etc/apt/sources.list.d/ubuntu-multiarch.sources' + $tempFile = [System.IO.Path]::GetTempFileName() + + @" +Types: deb +Architectures: amd64 +URIs: http://archive.ubuntu.com/ubuntu/ +Suites: $versionCodename $($versionCodename)-updates $($versionCodename)-backports +Components: main restricted universe multiverse +Signed-By: /usr/share/keyrings/ubuntu-archive-keyring.gpg + +Types: deb +Architectures: amd64 +URIs: http://security.ubuntu.com/ubuntu/ +Suites: $($versionCodename)-security +Components: main restricted universe multiverse +Signed-By: /usr/share/keyrings/ubuntu-archive-keyring.gpg + +Types: deb +Architectures: arm64 +URIs: http://ports.ubuntu.com/ubuntu-ports/ +Suites: $versionCodename $($versionCodename)-updates $($versionCodename)-backports $($versionCodename)-security +Components: main restricted universe multiverse +Signed-By: /usr/share/keyrings/ubuntu-archive-keyring.gpg +"@ | Set-Content -Path $tempFile -NoNewline + + try { + & sudo rm -f '/etc/apt/sources.list.d/ubuntu.sources' '/etc/apt/sources.list' + & sudo install -m 644 $tempFile $sourcesFile + } finally { + Remove-Item -Path $tempFile -Force -ErrorAction SilentlyContinue + } +} + +$packages = @( + 'python3-wget', + 'python3-setuptools', + 'libsystemd-dev', + 'dh-make' +) + +if ($Architecture -eq 'arm64') { + & sudo dpkg --add-architecture arm64 + Set-UbuntuMultiarchSources + $packages += @( + 'binutils-aarch64-linux-gnu', + 'gcc-aarch64-linux-gnu', + 'g++-aarch64-linux-gnu', + 'qemu-user' + ) +} + +if ($InstallLibsodium) { + $packages += if ($Architecture -eq 'arm64') { 'libsodium-dev:arm64' } else { 'libsodium-dev' } +} + +& sudo apt-get update +& sudo apt-get '-o' 'Acquire::Retries=3' 'install' '-qy' @packages + +if ($Architecture -eq 'arm64') { + & rustup target add aarch64-unknown-linux-gnu + Add-Content -Path $Env:GITHUB_ENV -Value 'STRIP_EXECUTABLE=aarch64-linux-gnu-strip' +} 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); From 5e577a527b6eb32c427ec13589403de15c026d01 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20CORTIER?= Date: Wed, 25 Mar 2026 22:52:28 +0900 Subject: [PATCH 02/17] Vendor mlock logic --- .github/workflows/ci.yml | 15 +- Cargo.lock | 13 +- ci/setup-linux-build-deps.ps1 | 60 +----- crates/secret-memory/Cargo.toml | 26 +++ crates/secret-memory/README.md | 70 +++++++ crates/secret-memory/src/fallback.rs | 52 +++++ crates/secret-memory/src/lib.rs | 187 +++++++++++++++++ crates/secret-memory/src/unix.rs | 199 +++++++++++++++++++ crates/secret-memory/src/windows.rs | 183 +++++++++++++++++ devolutions-gateway/Cargo.toml | 3 +- devolutions-gateway/src/credential/crypto.rs | 166 +++++++--------- devolutions-gateway/src/service.rs | 8 - 12 files changed, 801 insertions(+), 181 deletions(-) create mode 100644 crates/secret-memory/Cargo.toml create mode 100644 crates/secret-memory/README.md create mode 100644 crates/secret-memory/src/fallback.rs create mode 100644 crates/secret-memory/src/lib.rs create mode 100644 crates/secret-memory/src/unix.rs create mode 100644 crates/secret-memory/src/windows.rs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 000649248..8a16b6f0c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -618,7 +618,7 @@ jobs: - name: Configure Linux runner if: ${{ matrix.os == 'linux' }} - run: ./ci/setup-linux-build-deps.ps1 -Architecture ${{ matrix.arch }} -InstallLibsodium + run: ./ci/setup-linux-build-deps.ps1 -Architecture ${{ matrix.arch }} shell: pwsh - name: Install fpm @@ -654,19 +654,6 @@ 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 (in the script). - # 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 04802c462..b5107801f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1530,7 +1530,7 @@ dependencies = [ "rustls-cng", "rustls-native-certs", "secrecy 0.10.3", - "secrets", + "secret-memory", "serde", "serde-querystring", "serde_json", @@ -6112,14 +6112,13 @@ dependencies = [ ] [[package]] -name = "secrets" -version = "1.2.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f51745a213c4a2acabad80cd511e40376996bc83db6ceb4ebc7853d41c597988" +name = "secret-memory" +version = "0.0.0" dependencies = [ "libc", - "pkg-config", - "vcpkg", + "tracing", + "windows 0.61.3", + "zeroize", ] [[package]] diff --git a/ci/setup-linux-build-deps.ps1 b/ci/setup-linux-build-deps.ps1 index 19ef27f1b..49ce40211 100644 --- a/ci/setup-linux-build-deps.ps1 +++ b/ci/setup-linux-build-deps.ps1 @@ -3,63 +3,11 @@ param( [Parameter(Mandatory = $true)] [ValidateSet('x86_64', 'arm64')] - [string] $Architecture, - - [switch] $InstallLibsodium + [string] $Architecture ) $ErrorActionPreference = 'Stop' -function Get-OsReleaseValue { - param( - [Parameter(Mandatory = $true)] - [string] $Name - ) - - $match = Get-Content '/etc/os-release' | Select-String -Pattern "^${Name}=(.*)$" | Select-Object -First 1 - if (-not $match) { - throw "missing ${Name} in /etc/os-release" - } - - return $match.Matches[0].Groups[1].Value.Trim('"') -} - -function Set-UbuntuMultiarchSources { - $versionCodename = Get-OsReleaseValue -Name 'VERSION_CODENAME' - $sourcesFile = '/etc/apt/sources.list.d/ubuntu-multiarch.sources' - $tempFile = [System.IO.Path]::GetTempFileName() - - @" -Types: deb -Architectures: amd64 -URIs: http://archive.ubuntu.com/ubuntu/ -Suites: $versionCodename $($versionCodename)-updates $($versionCodename)-backports -Components: main restricted universe multiverse -Signed-By: /usr/share/keyrings/ubuntu-archive-keyring.gpg - -Types: deb -Architectures: amd64 -URIs: http://security.ubuntu.com/ubuntu/ -Suites: $($versionCodename)-security -Components: main restricted universe multiverse -Signed-By: /usr/share/keyrings/ubuntu-archive-keyring.gpg - -Types: deb -Architectures: arm64 -URIs: http://ports.ubuntu.com/ubuntu-ports/ -Suites: $versionCodename $($versionCodename)-updates $($versionCodename)-backports $($versionCodename)-security -Components: main restricted universe multiverse -Signed-By: /usr/share/keyrings/ubuntu-archive-keyring.gpg -"@ | Set-Content -Path $tempFile -NoNewline - - try { - & sudo rm -f '/etc/apt/sources.list.d/ubuntu.sources' '/etc/apt/sources.list' - & sudo install -m 644 $tempFile $sourcesFile - } finally { - Remove-Item -Path $tempFile -Force -ErrorAction SilentlyContinue - } -} - $packages = @( 'python3-wget', 'python3-setuptools', @@ -68,8 +16,6 @@ $packages = @( ) if ($Architecture -eq 'arm64') { - & sudo dpkg --add-architecture arm64 - Set-UbuntuMultiarchSources $packages += @( 'binutils-aarch64-linux-gnu', 'gcc-aarch64-linux-gnu', @@ -78,10 +24,6 @@ if ($Architecture -eq 'arm64') { ) } -if ($InstallLibsodium) { - $packages += if ($Architecture -eq 'arm64') { 'libsodium-dev:arm64' } else { 'libsodium-dev' } -} - & sudo apt-get update & sudo apt-get '-o' 'Acquire::Retries=3' 'install' '-qy' @packages diff --git a/crates/secret-memory/Cargo.toml b/crates/secret-memory/Cargo.toml new file mode 100644 index 000000000..860d53ef3 --- /dev/null +++ b/crates/secret-memory/Cargo.toml @@ -0,0 +1,26 @@ +[package] +name = "secret-memory" +version = "0.0.0" +edition = "2024" +license = "MIT/Apache-2.0" +authors = ["Devolutions Inc. "] +description = "Minimal protected in-memory storage for a single master key" +publish = false + +[lints] +workspace = true + +[dependencies] +tracing = "0.1" +zeroize = "1.8" + +[target.'cfg(unix)'.dependencies] +libc = "0.2" + +[target.'cfg(windows)'.dependencies.windows] +version = "0.61" +features = [ + "Win32_Foundation", + "Win32_System_Memory", + "Win32_System_SystemInformation", +] diff --git a/crates/secret-memory/README.md b/crates/secret-memory/README.md new file mode 100644 index 000000000..ed66b7b8a --- /dev/null +++ b/crates/secret-memory/README.md @@ -0,0 +1,70 @@ +# secret-memory + +A minimal, auditable in-memory secret store for a single fixed-size master key. + +## Purpose + +This crate provides exactly one type — `ProtectedBytes` — whose sole job is +to hold a `[u8; N]` (e.g.: a master key) with the best available OS memory-hardening applied at runtime. + +It is intentionally **not** a general-purpose secret library. + +## Threat model + +**Protected against:** +- Swapping the secret to disk (via `mlock` / `VirtualLock`). +- The secret appearing in Linux core dumps (via `madvise(MADV_DONTDUMP)`). +- Adjacent heap corruption reaching the secret (via guard pages). +- Accidental logging (redacted `Debug`, no `Display`). +- Residual bytes after the secret is dropped (zeroize-before-free). + +**Not protected against:** +- A privileged process reading `/proc//mem` or `ReadProcessMemory`. +- The OS itself (kernel, hypervisor). +- CPU microarchitectural side channels (Spectre, Meltdown, …). +- Transient register / stack copies during `expose_secret` calls — memory + locking does **not** prevent the CPU from holding secret bytes in registers + or on the call stack while the caller uses them. +- Attackers with `ptrace` or equivalent capability. +- SGX / TPM / hardware-backed enclaves. + +## Platform guarantees + +| Feature | Linux | Windows | Other | +|-----------------|---------------------|---------------------|--------------------| +| Page allocation | `mmap(MAP_ANON)` | `VirtualAlloc` | `Box` heap | +| Guard pages | `mprotect(PROT_NONE)` | `VirtualProtect(PAGE_NOACCESS)` | ✗ | +| RAM lock | `mlock` | `VirtualLock` | ✗ | +| Dump exclusion | `MADV_DONTDUMP` | ✗ (see note) | ✗ | +| Zeroize on drop | ✓ | ✓ | ✓ | + +**Windows dump exclusion note:** Windows does not expose a per-region public API +equivalent to `MADV_DONTDUMP`. `VirtualLock` prevents paging, which avoids +pagefile-based exposure, but crash dumps (WER, procdump, …) will include the +locked pages. `dump_excluded` is always `false` on Windows. + +**macOS note:** The Unix backend compiles for macOS (mmap + guard pages + mlock), +but `MADV_DONTDUMP` is Linux-only, so `dump_excluded` is always `false` on macOS. +macOS support is not tested in CI. + +## Fallback behavior + +On platforms where neither the Unix nor Windows backend compiles, the crate falls +back to a plain `Box<[u8; N]>` with `zeroize`-on-drop. A warning is logged +once at construction time. No feature flag is required; the crate always compiles +and runs. + +## Usage + +```rust +use secret_memory::ProtectedBytes; + +let key = ProtectedBytes::new([0u8; 32]); +let status = key.protection_status(); +if !status.locked { + tracing::warn!("master key is not mlock'd"); +} + +// Short-lived borrow: +let bytes: &[u8; 32] = key.expose_secret(); +``` diff --git a/crates/secret-memory/src/fallback.rs b/crates/secret-memory/src/fallback.rs new file mode 100644 index 000000000..2b388add7 --- /dev/null +++ b/crates/secret-memory/src/fallback.rs @@ -0,0 +1,52 @@ +//! Fallback backend: plain heap allocation with zeroize-on-drop. +//! +//! Used on platforms where neither the Unix nor the Windows backend is +//! available. All hardening features are absent; a message is logged once +//! at construction time. + +use std::sync::Once; + +use crate::ProtectionStatus; + +/// Heap-backed allocation with zeroize-on-drop. No OS hardening. +pub(crate) struct SecureAlloc { + inner: Box<[u8; N]>, +} + +impl SecureAlloc { + pub(crate) fn new(src: &[u8; N]) -> (Self, ProtectionStatus) { + warn_once(); + + let mut b = Box::new([0u8; N]); + b.copy_from_slice(src); + + let status = ProtectionStatus { + locked: false, + guard_pages: false, + dump_excluded: false, + fallback_backend: true, + }; + (Self { inner: b }, status) + } + + pub(crate) fn expose(&self) -> &[u8; N] { + &self.inner + } +} + +impl Drop for SecureAlloc { + fn drop(&mut self) { + zeroize::Zeroize::zeroize(self.inner.as_mut()); + } +} + +fn warn_once() { + static WARNED: Once = Once::new(); + WARNED.call_once(|| { + tracing::debug!( + "secret-memory: advanced memory protection (mlock, guard pages, \ + dump exclusion) is not available on this platform; \ + falling back to plain heap allocation with zeroize-on-drop only" + ); + }); +} diff --git a/crates/secret-memory/src/lib.rs b/crates/secret-memory/src/lib.rs new file mode 100644 index 000000000..f06a39f52 --- /dev/null +++ b/crates/secret-memory/src/lib.rs @@ -0,0 +1,187 @@ +#![cfg_attr(doc, doc = include_str!("../README.md"))] + +#[cfg(any(test, not(any(unix, windows))))] +mod fallback; + +#[cfg(unix)] +mod unix; +#[cfg(windows)] +mod windows; + +#[cfg(not(any(unix, windows)))] +use fallback as platform; +#[cfg(unix)] +use unix as platform; +#[cfg(windows)] +use windows as platform; + +use core::fmt; + +/// The memory-protection features that were successfully activated for a +/// [`ProtectedBytes`] allocation. +/// +/// All fields are `false` when the platform backend is not available +/// (`fallback_backend == true`). +#[derive(Debug, Clone, Copy)] +pub struct ProtectionStatus { + /// The pages are locked in RAM (`mlock` / `VirtualLock`). + /// + /// When `false` the OS may page the secret to disk under memory pressure. + pub locked: bool, + + /// Guard pages are installed immediately before and after the data page. + /// + /// Any out-of-bounds access into adjacent memory will fault at the OS level. + pub guard_pages: bool, + + /// The region is excluded from core / crash dumps. + /// + /// Currently only achievable on Linux via `madvise(MADV_DONTDUMP)`. + /// Always `false` on Windows and macOS — see the crate README. + pub dump_excluded: bool, + + /// No OS-level hardening is available; using plain heap allocation. + /// + /// The secret is still zeroized on drop but none of the other protections + /// are active. A warning is logged once at construction time. + pub fallback_backend: bool, +} + +/// A fixed-size, protected in-memory secret. +/// +/// On supported platforms the backing storage is a dedicated page-based +/// allocation with guard pages, memory locking, and (where available) +/// exclusion from core dumps. On all platforms the bytes are zeroized +/// before the backing allocation is released. +/// +/// # Safety properties +/// +/// - Never printed: `Debug` emits `[REDACTED]`; `Display` is absent. +/// - Not clonable, not copyable. +/// - One unavoidable stack copy in `new`: the `[u8; N]` argument lives on the +/// caller's stack until it is zeroized immediately after being transferred +/// into secure storage. +/// - `mlock` / `VirtualLock` prevent the secret from being paged to disk but +/// do **not** prevent transient exposure in CPU registers or on the call stack +/// while `expose_secret` is in use. +pub struct ProtectedBytes { + inner: platform::SecureAlloc, + status: ProtectionStatus, +} + +impl ProtectedBytes { + /// Move `bytes` into a new protected allocation. + /// + /// The local copy of `bytes` is zeroized immediately after it has been + /// transferred into secure storage. + /// + /// # Panics + /// + /// Panics if the underlying OS page allocation fails (equivalent to + /// out-of-memory). Hardening steps that fail at runtime (mlock limits, + /// unavailable `madvise` flags, …) do **not** panic; they are downgraded + /// and reported via [`ProtectedBytes::protection_status`] and a + /// `tracing::debug!`. + pub fn new(mut bytes: [u8; N]) -> Self { + let (inner, status) = platform::SecureAlloc::new(&bytes); + // Zeroize the stack copy now that the secret lives in secure storage. + zeroize::Zeroize::zeroize(&mut bytes); + Self { inner, status } + } + + /// Borrow the secret bytes. + /// + /// Keep the returned reference as short-lived as possible. + /// The CPU may hold the value in registers or on the stack during use. + #[must_use] + pub fn expose_secret(&self) -> &[u8; N] { + self.inner.expose() + } + + /// Return the protection status achieved at construction time. + #[must_use] + pub fn protection_status(&self) -> &ProtectionStatus { + &self.status + } +} + +impl fmt::Debug for ProtectedBytes { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "ProtectedBytes<{N}>([REDACTED])") + } +} + +// Intentionally absent: Display, Clone, Copy, Serialize, Deserialize. + +#[cfg(test)] +#[expect(clippy::unwrap_used, reason = "test code")] +mod tests { + use super::*; + + #[test] + fn construction_and_expose() { + let secret = ProtectedBytes::new([0x42u8; 32]); + assert_eq!(secret.expose_secret(), &[0x42u8; 32]); + } + + #[test] + fn redacted_debug_does_not_leak_bytes() { + let secret = ProtectedBytes::new([0xFFu8; 32]); + let s = format!("{secret:?}"); + assert!(s.contains("REDACTED"), "debug must say REDACTED, got: {s}"); + // Must not contain the byte value in decimal or hex. + assert!(!s.contains("255"), "debug must not leak decimal value"); + assert!(!s.contains("ff"), "debug must not leak hex value"); + assert!(!s.contains("FF"), "debug must not leak hex value (upper)"); + } + + /// Verify that construction does not panic and that the status makes sense + /// for the current platform. + #[test] + fn protection_status_coherent() { + let secret = ProtectedBytes::new([1u8; 32]); + let st = secret.protection_status(); + + // fallback_backend is mutually exclusive with OS hardening. + if st.fallback_backend { + assert!(!st.locked); + assert!(!st.guard_pages); + assert!(!st.dump_excluded); + } + + // dump_excluded without locked would be unusual; at minimum, if + // dump_excluded is true then a real OS backend must be active. + if st.dump_excluded { + assert!(!st.fallback_backend); + } + } + + #[cfg(any(unix, windows))] + #[test] + fn os_backend_is_not_fallback() { + let secret = ProtectedBytes::new([2u8; 32]); + assert!( + !secret.protection_status().fallback_backend, + "expected OS backend on this platform" + ); + } + + #[cfg(any(unix, windows))] + #[test] + fn guard_pages_active() { + let secret = ProtectedBytes::new([3u8; 32]); + let st = secret.protection_status(); + assert!(st.guard_pages, "guard pages should be active"); + } + + // Test the fallback backend directly on all platforms. + #[test] + fn fallback_backend_constructs_correctly() { + let (alloc, status) = fallback::SecureAlloc::<32>::new(&[0x99u8; 32]); + assert_eq!(alloc.expose(), &[0x99u8; 32]); + assert!(status.fallback_backend); + assert!(!status.locked); + assert!(!status.guard_pages); + assert!(!status.dump_excluded); + } +} diff --git a/crates/secret-memory/src/unix.rs b/crates/secret-memory/src/unix.rs new file mode 100644 index 000000000..4e2cfca0f --- /dev/null +++ b/crates/secret-memory/src/unix.rs @@ -0,0 +1,199 @@ +//! Unix (Linux / macOS) secure-allocation backend. +//! +//! ## Memory layout +//! +//! ```text +//! ┌──────────────┬──────────────┬──────────────┐ +//! │ guard page │ data page │ guard page │ +//! │ PROT_NONE │ PROT_R|W │ PROT_NONE │ +//! └──────────────┴──────────────┴──────────────┘ +//! ^base ^data ^data + page_size +//! ``` +//! +//! The secret occupies the first `N` bytes of the data page. +//! The rest of the page is unused. +//! Guard pages are set to `PROT_NONE` so any out-of-bounds access faults immediately at the OS level. +//! +//! ## Hardening steps (best-effort) +//! +//! 1. Guard pages via `mprotect(PROT_NONE)`. +//! 2. RAM lock via `mlock` — may fail under tight `ulimit -l` limits. +//! 3. Core-dump exclusion via `madvise(MADV_DONTDUMP)` — Linux only. +//! +//! All three are best-effort: failure is logged and reflected in [`ProtectionStatus`] but does **not** abort the process. + +use std::ptr; +use std::sync::OnceLock; + +use crate::ProtectionStatus; + +/// Page-based secure allocation for Unix. +pub(crate) struct SecureAlloc { + /// Start of the entire 3-page `mmap` region (the first guard page). + base: *mut u8, + /// Start of the data page (`base + page_size`). + data: *mut u8, + /// Whether `mlock` succeeded. + locked: bool, + /// Marker: `SecureAlloc` logically owns a `[u8; N]`. + _marker: std::marker::PhantomData<[u8; N]>, +} + +// SAFETY: `SecureAlloc` has exclusive ownership of its `mmap` allocation. +// There is no shared mutable state and no aliasing. +unsafe impl Send for SecureAlloc {} + +// SAFETY: `expose()` returns a shared reference to immutable bytes, which +// is safe to hand out to multiple threads simultaneously. +unsafe impl Sync for SecureAlloc {} + +impl SecureAlloc { + pub(crate) fn new(src: &[u8; N]) -> (Self, ProtectionStatus) { + let ps = page_size(); + assert!( + N <= ps, + "secret-memory: N ({N}) exceeds page size ({ps}); not supported" + ); + + let total = 3 * ps; + + // Allocate three contiguous anonymous private pages. + // SAFETY: `MAP_ANON | MAP_PRIVATE` with `fd = -1`, `offset = 0` is the + // standard idiom for anonymous memory; no file backing, no aliases. + let base_raw = unsafe { + libc::mmap( + ptr::null_mut(), + total, + libc::PROT_READ | libc::PROT_WRITE, + libc::MAP_ANON | libc::MAP_PRIVATE, + -1, + 0, + ) + }; + + if base_raw == libc::MAP_FAILED { + panic!("secret-memory: mmap({total}) failed; process is out of address space"); + } + + let base = base_raw as *mut u8; + + // data page starts at base + page_size. + // SAFETY: `base` is a valid allocation of `total = 3 * ps` bytes; + // `base + ps` is within that range. + let data = unsafe { base.add(ps) }; + + // ── Guard page before the data (page 0) ───────────────────────────── + // SAFETY: `base` is page-aligned and points to `ps` valid bytes. + let r_guard_before = unsafe { libc::mprotect(base as *mut libc::c_void, ps, libc::PROT_NONE) }; + + // ── Guard page after the data (page 2) ────────────────────────────── + // SAFETY: `base + 2 * ps` is within the allocation; page-aligned. + let guard_after = unsafe { base.add(2 * ps) }; + // SAFETY: `guard_after` is page-aligned, `ps` bytes within the allocation. + let r_guard_after = unsafe { libc::mprotect(guard_after as *mut libc::c_void, ps, libc::PROT_NONE) }; + + let guard_pages = r_guard_before == 0 && r_guard_after == 0; + if !guard_pages { + tracing::debug!( + "secret-memory: mprotect for guard pages failed ({}); \ + guard pages are not active", + std::io::Error::last_os_error() + ); + } + + // ── Lock the data page in RAM ──────────────────────────────────────── + // SAFETY: `data` is page-aligned; `ps` bytes are within the allocation. + let r_lock = unsafe { libc::mlock(data as *const libc::c_void, ps) }; + let locked = r_lock == 0; + if !locked { + tracing::debug!( + "secret-memory: mlock failed ({}); \ + secret may be paged to disk — consider raising `ulimit -l`", + std::io::Error::last_os_error() + ); + } + + // ── Exclude from core dumps (Linux ≥ 3.4 only) ────────────────────── + #[cfg(target_os = "linux")] + let dump_excluded = { + // SAFETY: `data` is page-aligned; `ps` bytes are a valid mapping. + let r = unsafe { libc::madvise(data as *mut libc::c_void, ps, libc::MADV_DONTDUMP) }; + if r != 0 { + tracing::debug!( + "secret-memory: madvise(MADV_DONTDUMP) failed ({}); \ + region may appear in core dumps", + std::io::Error::last_os_error() + ); + } + r == 0 + }; + // macOS and other Unixes: no equivalent to MADV_DONTDUMP. + #[cfg(not(target_os = "linux"))] + let dump_excluded = false; + + // ── Copy secret into the data page ────────────────────────────────── + // SAFETY: `src` (caller stack) and `data` (mmap region) are + // non-overlapping; both are valid for `N` bytes. + unsafe { ptr::copy_nonoverlapping(src.as_ptr(), data, N) }; + + let alloc = SecureAlloc { + base, + data, + locked, + _marker: std::marker::PhantomData, + }; + let status = ProtectionStatus { + locked, + guard_pages, + dump_excluded, + fallback_backend: false, + }; + + (alloc, status) + } + + pub(crate) fn expose(&self) -> &[u8; N] { + // SAFETY: `self.data` is valid for `N` initialised bytes and is live + // for at least as long as `self`. + unsafe { &*(self.data as *const [u8; N]) } + } +} + +impl Drop for SecureAlloc { + fn drop(&mut self) { + let ps = page_size(); + + // Restore read+write on the data page so we can zeroize it. + // (It should already be RW; this is defensive.) + // SAFETY: `self.data` is page-aligned; `ps` bytes are within our mapping. + let _ = unsafe { libc::mprotect(self.data as *mut libc::c_void, ps, libc::PROT_READ | libc::PROT_WRITE) }; + + // Zeroize secret bytes using `zeroize` to defeat compiler optimisations. + // SAFETY: `self.data` is valid for `N` bytes; align of `u8` is 1. + let secret = unsafe { std::slice::from_raw_parts_mut(self.data, N) }; + zeroize::Zeroize::zeroize(secret); + + if self.locked { + // SAFETY: `self.data` and `ps` are the values used in the matching `mlock`. + let _ = unsafe { libc::munlock(self.data as *const libc::c_void, ps) }; + } + + // Unmap the full three-page region. + // SAFETY: `self.base` is the start of the mapping of size `3 * ps`. + let _ = unsafe { libc::munmap(self.base as *mut libc::c_void, 3 * ps) }; + } +} + +/// Return the system page size, cached after the first call. +fn page_size() -> usize { + static PAGE_SIZE: OnceLock = OnceLock::new(); + *PAGE_SIZE.get_or_init(|| { + // SAFETY: `sysconf` with `_SC_PAGESIZE` is always safe to call. + let ps = unsafe { libc::sysconf(libc::_SC_PAGESIZE) }; + if ps > 0 { + ps as usize + } else { + 4096 // conservative fallback + } + }) +} diff --git a/crates/secret-memory/src/windows.rs b/crates/secret-memory/src/windows.rs new file mode 100644 index 000000000..f6577f1f3 --- /dev/null +++ b/crates/secret-memory/src/windows.rs @@ -0,0 +1,183 @@ +//! Windows secure-allocation backend. +//! +//! ## Memory layout +//! +//! ```text +//! ┌──────────────┬──────────────┬──────────────┐ +//! │ guard page │ data page │ guard page │ +//! │ PAGE_NOACCESS│PAGE_READWRITE│ PAGE_NOACCESS│ +//! └──────────────┴──────────────┴──────────────┘ +//! ^base ^data ^data + page_size +//! ``` +//! +//! The three pages are a single `VirtualAlloc` region. +//! Guard pages are set to `PAGE_NOACCESS` via `VirtualProtect`. +//! +//! ## Hardening steps (best-effort) +//! +//! 1. Guard pages via `VirtualProtect(PAGE_NOACCESS)`. +//! 2. RAM lock via `VirtualLock`. +//! +//! ## Dump-exclusion limitation +//! +//! Windows does not expose a per-region public API equivalent to Linux's `MADV_DONTDUMP`. +//! `VirtualLock` prevents the pages from being written to the pagefile but crash dumps (WER, procdump, …) will still include them. +//! `dump_excluded` is always `false` on Windows. + +use std::ffi::c_void; +use std::ptr; +use std::sync::OnceLock; + +use windows::Win32::System::Memory::{ + MEM_COMMIT, MEM_RELEASE, MEM_RESERVE, PAGE_NOACCESS, PAGE_PROTECTION_FLAGS, PAGE_READWRITE, VirtualAlloc, + VirtualFree, VirtualLock, VirtualProtect, VirtualUnlock, +}; +use windows::Win32::System::SystemInformation::{GetSystemInfo, SYSTEM_INFO}; + +use crate::ProtectionStatus; + +/// Page-based secure allocation for Windows. +pub(crate) struct SecureAlloc { + /// Start of the entire 3-page `VirtualAlloc` region (the first guard page). + base: *mut u8, + /// Start of the data page (`base + page_size`). + data: *mut u8, + /// Whether `VirtualLock` succeeded. + locked: bool, + /// Marker: `SecureAlloc` logically owns a `[u8; N]`. + _marker: std::marker::PhantomData<[u8; N]>, +} + +// SAFETY: `SecureAlloc` has exclusive ownership of its `VirtualAlloc` region. +// There is no shared mutable state and no aliasing. +unsafe impl Send for SecureAlloc {} + +// SAFETY: `expose()` returns a shared reference to immutable bytes, which +// is safe to hand out to multiple threads simultaneously. +unsafe impl Sync for SecureAlloc {} + +impl SecureAlloc { + pub(crate) fn new(src: &[u8; N]) -> (Self, ProtectionStatus) { + let ps = page_size(); + assert!( + N <= ps, + "secret-memory: N ({N}) exceeds page size ({ps}); not supported" + ); + + let total = 3 * ps; + + // Allocate three contiguous committed pages (read/write). + // SAFETY: `VirtualAlloc` with `None` address, `MEM_COMMIT | MEM_RESERVE`, + // and `PAGE_READWRITE` is the standard anonymous allocation idiom. + let base_raw = unsafe { VirtualAlloc(None, total, MEM_COMMIT | MEM_RESERVE, PAGE_READWRITE) }; + + if base_raw.is_null() { + panic!( + "secret-memory: VirtualAlloc({total}) failed ({})", + std::io::Error::last_os_error() + ); + } + + let base = base_raw as *mut u8; + + // Data page starts at base + page_size. + // SAFETY: `base` is a valid allocation of `total = 3 * ps` bytes; + // `base + ps` is within that range. + let data = unsafe { base.add(ps) }; + + // ── Guard page before the data (page 0) ───────────────────────────── + let mut old_prot = PAGE_PROTECTION_FLAGS::default(); + // SAFETY: `base` is page-aligned and points to `ps` valid committed bytes. + let r_guard_before = unsafe { VirtualProtect(base as *const c_void, ps, PAGE_NOACCESS, &mut old_prot) }; + + // ── Guard page after the data (page 2) ────────────────────────────── + // SAFETY: `base + 2 * ps` is within the allocation; page-aligned. + let guard_after = unsafe { base.add(2 * ps) }; + // SAFETY: `guard_after` is page-aligned, `ps` committed bytes. + let r_guard_after = unsafe { VirtualProtect(guard_after as *const c_void, ps, PAGE_NOACCESS, &mut old_prot) }; + + let guard_pages = r_guard_before.is_ok() && r_guard_after.is_ok(); + if !guard_pages { + tracing::debug!( + "secret-memory: VirtualProtect for guard pages failed ({}); \ + guard pages are not active", + std::io::Error::last_os_error() + ); + } + + // ── Lock the data page in RAM ──────────────────────────────────────── + // SAFETY: `data` is page-aligned; `ps` committed bytes within the allocation. + let r_lock = unsafe { VirtualLock(data as *const c_void, ps) }; + let locked = r_lock.is_ok(); + if !locked { + tracing::debug!( + "secret-memory: VirtualLock failed ({}); \ + secret may be paged to disk", + std::io::Error::last_os_error() + ); + } + + // ── Copy secret into the data page ────────────────────────────────── + // SAFETY: `src` (caller stack) and `data` (VirtualAlloc region) are + // non-overlapping; both are valid for `N` bytes. + unsafe { ptr::copy_nonoverlapping(src.as_ptr(), data, N) }; + + let alloc = SecureAlloc { + base, + data, + locked, + _marker: std::marker::PhantomData, + }; + let status = ProtectionStatus { + locked, + guard_pages, + dump_excluded: false, // no per-region dump exclusion on Windows + fallback_backend: false, + }; + + (alloc, status) + } + + pub(crate) fn expose(&self) -> &[u8; N] { + // SAFETY: `self.data` is valid for `N` initialised bytes and is live + // for at least as long as `self`. + unsafe { &*(self.data as *const [u8; N]) } + } +} + +impl Drop for SecureAlloc { + fn drop(&mut self) { + let ps = page_size(); + + // Restore read+write on the data page so we can zeroize it. + let mut old_prot = PAGE_PROTECTION_FLAGS::default(); + // SAFETY: `self.data` is page-aligned; `ps` committed bytes in our region. + let _ = unsafe { VirtualProtect(self.data as *const c_void, ps, PAGE_READWRITE, &mut old_prot) }; + + // Zeroize secret bytes using `zeroize` to defeat compiler optimisations. + // SAFETY: `self.data` is valid for `N` bytes; align of `u8` is 1. + let secret = unsafe { std::slice::from_raw_parts_mut(self.data, N) }; + zeroize::Zeroize::zeroize(secret); + + if self.locked { + // SAFETY: `self.data` and `ps` are the values used in `VirtualLock`. + let _ = unsafe { VirtualUnlock(self.data as *const c_void, ps) }; + } + + // Release the entire three-page region. + // SAFETY: `self.base` is the base of the allocation; `dwSize` must be 0 + // when `dwFreeType` is `MEM_RELEASE` (Windows requirement). + let _ = unsafe { VirtualFree(self.base as *mut c_void, 0, MEM_RELEASE) }; + } +} + +/// Return the system page size, cached after the first call. +fn page_size() -> usize { + static PAGE_SIZE: OnceLock = OnceLock::new(); + *PAGE_SIZE.get_or_init(|| { + let mut info = SYSTEM_INFO::default(); + // SAFETY: `GetSystemInfo` fills the provided struct; always safe to call. + unsafe { GetSystemInfo(&mut info) }; + info.dwPageSize as usize + }) +} diff --git a/devolutions-gateway/Cargo.toml b/devolutions-gateway/Cargo.toml index 3ec30064f..27f018be5 100644 --- a/devolutions-gateway/Cargo.toml +++ b/devolutions-gateway/Cargo.toml @@ -14,12 +14,12 @@ workspace = true [features] default = [] -mlock = ["dep:secrets"] openapi = ["dep:utoipa"] [dependencies] # In-house +secret-memory.path = "../crates/secret-memory" transport.path = "../crates/transport" jmux-proxy.path = "../crates/jmux-proxy" devolutions-agent-shared.path = "../crates/devolutions-agent-shared" @@ -76,7 +76,6 @@ bitflags = "2.9" 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" diff --git a/devolutions-gateway/src/credential/crypto.rs b/devolutions-gateway/src/credential/crypto.rs index c74c9f542..3edcfab25 100644 --- a/devolutions-gateway/src/credential/crypto.rs +++ b/devolutions-gateway/src/credential/crypto.rs @@ -1,19 +1,17 @@ //! 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. +//! A randomly generated 256-bit master key is held in a [`ProtectedBytes<32>`] allocation backed by `secret-memory`, which applies +//! the best available OS hardening (mlock, guard pages, core-dump exclusion) and always zeroizes on drop. //! -//! ## Security Properties +//! ## 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) +//! - 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 regardless of platform. +//! - Master key held in mlock'd / guard-paged memory where the OS permits it. use core::fmt; use std::sync::LazyLock; @@ -24,76 +22,74 @@ 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; +use secret_memory::ProtectedBytes; /// 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")) -}); +/// Initialized lazily on first access. +/// The key is stored in a [`ProtectedBytes<32>`] allocation. +/// A [`Mutex`] provides thread-safe interior mutability. +/// +/// A warning is logged at startup if full memory hardening is unavailable (see [`ProtectionStatus`]). +/// +/// [`ProtectionStatus`]: secret_memory::ProtectionStatus +pub(super) static MASTER_KEY: LazyLock> = LazyLock::new(|| Mutex::new(MasterKeyManager::new())); /// 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 +/// The key is held in a [`ProtectedBytes<32>`] allocation: +/// - Locked in RAM (`mlock` / `VirtualLock`) where available. +/// - Surrounded by guard pages where available. +/// - Excluded from core dumps on Linux (`MADV_DONTDUMP`). +/// - Always zeroized on drop. pub(super) struct MasterKeyManager { - #[cfg(feature = "mlock")] - key_material: SecretBox<[u8; 32]>, - #[cfg(not(feature = "mlock"))] - key_material: Zeroizing<[u8; 32]>, + key_material: ProtectedBytes<32>, } impl MasterKeyManager { - /// Generate a new random 256-bit master key. + /// Generate a new random 256-bit master key and place it in protected memory. /// - /// # 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 }) + /// Logs a warning if any hardening step is unavailable. + fn new() -> Self { + let mut raw = [0u8; 32]; + OsRng.fill_bytes(&mut raw); + let key_material = ProtectedBytes::new(raw); + + let st = key_material.protection_status(); + if st.fallback_backend { + tracing::warn!( + "master key: advanced memory protection is unavailable on this platform; \ + the key is protected only by zeroize-on-drop" + ); + } else { + if !st.locked { + tracing::warn!( + "master key: mlock/VirtualLock failed; \ + the key may be paged to disk under memory pressure" + ); + } + if !st.dump_excluded { + tracing::debug!( + "master key: core-dump exclusion is not active \ + (unavailable on this platform or kernel)" + ); + } + } + + 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(self.key_material.expose_secret()).expect("key is exactly 32 bytes"); - let cipher = ChaCha20Poly1305::new_from_slice(key_bytes).expect("key is exactly 32 bytes"); - - // Generate random 96-bit nonce (12 bytes for ChaCha20-Poly1305). + // Generate a random 96-bit nonce (12 bytes for ChaCha20-Poly1305). let nonce = ChaCha20Poly1305::generate_nonce(OsRng); - // Encrypt (ciphertext includes 16-byte Poly1305 tag). + // Encrypt; ciphertext includes 16-byte Poly1305 authentication tag. let ciphertext = cipher .encrypt(&nonce, plaintext.as_bytes()) .ok() @@ -102,46 +98,35 @@ impl MasterKeyManager { Ok(EncryptedPassword { nonce, ciphertext }) } - /// Decrypt a password, returning a `SecretString` that zeroizes on drop. + /// 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. + /// The returned value should be used immediately and dropped promptly to + /// minimize the plaintext lifetime in heap memory. 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 cipher = + ChaCha20Poly1305::new_from_slice(self.key_material.expose_secret()).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. +/// Contains the nonce and ciphertext (including the Poly1305 authentication +/// tag). Safe to store in regular memory because it is 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 + 128-bit authentication tag (plaintext_len + 16 bytes). ciphertext: Vec, } @@ -162,7 +147,7 @@ mod tests { #[test] fn test_encrypt_decrypt_roundtrip() { - let key_manager = MasterKeyManager::new().unwrap(); + let key_manager = MasterKeyManager::new(); let plaintext = "my-secret-password"; let encrypted = key_manager.encrypt(plaintext).unwrap(); @@ -173,43 +158,43 @@ mod tests { #[test] fn test_different_nonces() { - let key_manager = MasterKeyManager::new().unwrap(); + let key_manager = MasterKeyManager::new(); 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). + // Same plaintext must 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 key_manager1 = MasterKeyManager::new(); + let key_manager2 = MasterKeyManager::new(); let encrypted = key_manager1.encrypt("secret").unwrap(); - // Decryption with different key should fail. + // Decryption with a different key must fail. assert!(key_manager2.decrypt(&encrypted).is_err()); } #[test] fn test_corrupted_ciphertext_fails() { - let key_manager = MasterKeyManager::new().unwrap(); + let key_manager = MasterKeyManager::new(); let mut encrypted = key_manager.encrypt("secret").unwrap(); // Corrupt the ciphertext. encrypted.ciphertext[0] ^= 0xFF; - // Should fail authentication. + // Authentication must fail. assert!(key_manager.decrypt(&encrypted).is_err()); } #[test] fn test_empty_password() { - let key_manager = MasterKeyManager::new().unwrap(); + let key_manager = MasterKeyManager::new(); let encrypted = key_manager.encrypt("").unwrap(); let decrypted = key_manager.decrypt(&encrypted).unwrap(); assert_eq!(decrypted.expose_secret(), ""); @@ -217,7 +202,7 @@ mod tests { #[test] fn test_unicode_password() { - let key_manager = MasterKeyManager::new().unwrap(); + let key_manager = MasterKeyManager::new(); let plaintext = "пароль-密码-كلمة السر"; let encrypted = key_manager.encrypt(plaintext).unwrap(); let decrypted = key_manager.decrypt(&encrypted).unwrap(); @@ -226,7 +211,6 @@ mod tests { #[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(); diff --git a/devolutions-gateway/src/service.rs b/devolutions-gateway/src/service.rs index 4b28cbf42..64dde91c4 100644 --- a/devolutions-gateway/src/service.rs +++ b/devolutions-gateway/src/service.rs @@ -49,14 +49,6 @@ 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); From e50a0d82c9845b2e03bf1d2a448a2437cb332add Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20CORTIER?= Date: Wed, 25 Mar 2026 22:57:55 +0900 Subject: [PATCH 03/17] . --- crates/secret-memory/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/secret-memory/src/lib.rs b/crates/secret-memory/src/lib.rs index f06a39f52..9fd68f560 100644 --- a/crates/secret-memory/src/lib.rs +++ b/crates/secret-memory/src/lib.rs @@ -8,6 +8,8 @@ mod unix; #[cfg(windows)] mod windows; +use core::fmt; + #[cfg(not(any(unix, windows)))] use fallback as platform; #[cfg(unix)] @@ -15,8 +17,6 @@ use unix as platform; #[cfg(windows)] use windows as platform; -use core::fmt; - /// The memory-protection features that were successfully activated for a /// [`ProtectedBytes`] allocation. /// From 0e34a988586c84842668706d184ecbf4e9658bf4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20CORTIER?= Date: Wed, 25 Mar 2026 23:04:04 +0900 Subject: [PATCH 04/17] . --- crates/secret-memory/src/unix.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/crates/secret-memory/src/unix.rs b/crates/secret-memory/src/unix.rs index 4e2cfca0f..67f694ef4 100644 --- a/crates/secret-memory/src/unix.rs +++ b/crates/secret-memory/src/unix.rs @@ -190,10 +190,6 @@ fn page_size() -> usize { *PAGE_SIZE.get_or_init(|| { // SAFETY: `sysconf` with `_SC_PAGESIZE` is always safe to call. let ps = unsafe { libc::sysconf(libc::_SC_PAGESIZE) }; - if ps > 0 { - ps as usize - } else { - 4096 // conservative fallback - } + usize::try_from(ps).unwrap_or(4096) }) } From 0fb5612569911874bd3cf1b99e15c0e8a5e4b500 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20CORTIER?= Date: Thu, 26 Mar 2026 15:09:53 +0900 Subject: [PATCH 05/17] . --- .github/workflows/ci.yml | 36 ++- Cargo.lock | 12 +- crates/secure-memory-verifier/Cargo.toml | 27 ++ crates/secure-memory-verifier/README.md | 119 +++++++++ .../secure-memory-verifier/src/check_guard.rs | 182 +++++++++++++ .../secure-memory-verifier/src/check_lock.rs | 97 +++++++ .../secure-memory-verifier/src/check_wer.rs | 249 ++++++++++++++++++ crates/secure-memory-verifier/src/main.rs | 138 ++++++++++ .../Cargo.toml | 3 +- .../README.md | 4 +- .../src/fallback.rs | 2 +- .../src/lib.rs | 12 +- .../src/unix.rs | 10 +- .../src/windows.rs | 66 ++++- devolutions-gateway/Cargo.toml | 2 +- devolutions-gateway/src/credential/crypto.rs | 8 +- 16 files changed, 938 insertions(+), 29 deletions(-) create mode 100644 crates/secure-memory-verifier/Cargo.toml create mode 100644 crates/secure-memory-verifier/README.md create mode 100644 crates/secure-memory-verifier/src/check_guard.rs create mode 100644 crates/secure-memory-verifier/src/check_lock.rs create mode 100644 crates/secure-memory-verifier/src/check_wer.rs create mode 100644 crates/secure-memory-verifier/src/main.rs rename crates/{secret-memory => secure-memory}/Cargo.toml (89%) rename crates/{secret-memory => secure-memory}/README.md (98%) rename crates/{secret-memory => secure-memory}/src/fallback.rs (95%) rename crates/{secret-memory => secure-memory}/src/lib.rs (91%) rename crates/{secret-memory => secure-memory}/src/unix.rs (96%) rename crates/{secret-memory => secure-memory}/src/windows.rs (69%) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8a16b6f0c..87505f587 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1162,10 +1162,44 @@ jobs: psexec -accepteula -s pwsh.exe $scriptPath Get-Content -Path ./crates/pedm-simulator/pedm-simulator_run-expect-elevation.out + secure-memory-verifier: + name: Secure memory verifier + runs-on: windows-2022 + needs: [preflight] + + steps: + - name: Checkout ${{ github.repository }} + uses: actions/checkout@v4 + with: + ref: ${{ needs.preflight.outputs.ref }} + + - name: Setup Rust cache + uses: ./.github/actions/setup-rust-cache + with: + sccache-enabled: ${{ needs.preflight.outputs.sccache }} + + - name: Configure WER LocalDumps + shell: pwsh + run: | + $key = "HKLM:\SOFTWARE\Microsoft\Windows\Windows Error Reporting\LocalDumps\secure-memory-verifier.exe" + New-Item $key -Force | Out-Null + Set-ItemProperty $key DumpType 2 + Set-ItemProperty $key DumpCount 5 + Set-ItemProperty $key DumpFolder $env:TEMP + + - name: Run secure-memory verifier + shell: pwsh + run: cargo run --release -p secure-memory-verifier -- all + + - name: Show sccache stats + if: ${{ needs.preflight.outputs.sccache == 'true' && !cancelled() }} + shell: pwsh + run: sccache --show-stats + success: name: Success if: ${{ always() }} - needs: [tests, lints, check-dependencies, jetsocat-lipo, devolutions-gateway-powershell, devolutions-gateway, devolutions-gateway-merge, devolutions-pedm-desktop, devolutions-agent, devolutions-agent-merge, devolutions-pedm-client, dotnet-utils-tests, winapi-sanitizer-tests, winapi-miri, pedm-simulator] + needs: [tests, lints, check-dependencies, jetsocat-lipo, devolutions-gateway-powershell, devolutions-gateway, devolutions-gateway-merge, devolutions-pedm-desktop, devolutions-agent, devolutions-agent-merge, devolutions-pedm-client, dotnet-utils-tests, winapi-sanitizer-tests, winapi-miri, pedm-simulator, secure-memory-verifier] runs-on: ubuntu-latest steps: diff --git a/Cargo.lock b/Cargo.lock index b5107801f..45ae4bbe8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1530,7 +1530,7 @@ dependencies = [ "rustls-cng", "rustls-native-certs", "secrecy 0.10.3", - "secret-memory", + "secure-memory", "serde", "serde-querystring", "serde_json", @@ -6112,7 +6112,7 @@ dependencies = [ ] [[package]] -name = "secret-memory" +name = "secure-memory" version = "0.0.0" dependencies = [ "libc", @@ -6121,6 +6121,14 @@ dependencies = [ "zeroize", ] +[[package]] +name = "secure-memory-verifier" +version = "0.0.0" +dependencies = [ + "secure-memory", + "windows 0.61.3", +] + [[package]] name = "security-framework" version = "3.5.1" diff --git a/crates/secure-memory-verifier/Cargo.toml b/crates/secure-memory-verifier/Cargo.toml new file mode 100644 index 000000000..741f79365 --- /dev/null +++ b/crates/secure-memory-verifier/Cargo.toml @@ -0,0 +1,27 @@ +[package] +name = "secure-memory-verifier" +version = "0.0.0" +authors = ["Devolutions Inc. "] +edition = "2024" +publish = false +description = "Windows runtime verifier for the secure-memory crate" + +[lints] +workspace = true + +[dependencies] +secure-memory = { path = "../secure-memory" } + +[target.'cfg(windows)'.dependencies.windows] +version = "0.61" +features = [ + "Win32_Foundation", + "Win32_Storage_FileSystem", + "Win32_System_Diagnostics_Debug", + "Win32_System_ErrorReporting", + "Win32_System_Kernel", + "Win32_System_Memory", + "Win32_System_ProcessStatus", + "Win32_System_SystemInformation", + "Win32_System_Threading", +] diff --git a/crates/secure-memory-verifier/README.md b/crates/secure-memory-verifier/README.md new file mode 100644 index 000000000..b637977b7 --- /dev/null +++ b/crates/secure-memory-verifier/README.md @@ -0,0 +1,119 @@ +# secure-memory-verifier + +A Windows-only standalone binary that verifies the runtime behaviour of the +`secret-memory` crate's four protection tracks independently. + +## What it checks + +| Subcommand | Track | Verification method | Automated? | +|---|---|---|---| +| `lock` | RAM locking | `QueryWorkingSetEx` Locked bit | Fully automated | +| `guard-underflow` | Guard pages (leading) | Child process crashes on access before data | Fully automated | +| `guard-overflow` | Guard pages (trailing) | Child process crashes on access after data | Fully automated | +| `wer-dump` | WER dump exclusion | `WerRegisterExcludedMemoryBlock` + crash child + scan dump | Requires WER pre-config (see below) | + +## Prerequisites + +- Windows 10 or later (Windows 11 recommended) +- Rust toolchain with `x86_64-pc-windows-msvc` or `aarch64-pc-windows-msvc` target +- `cargo build -p secure-memory-verifier` or `cargo run -p secure-memory-verifier` + +### WER dump-exclusion check prerequisites + +The `wer-dump` subcommand requires WER LocalDumps to be configured for the +verifier executable. This requires administrator rights. + +```powershell +$key = "HKLM:\SOFTWARE\Microsoft\Windows\Windows Error Reporting\LocalDumps\secure-memory-verifier.exe" +New-Item $key -Force | Out-Null +Set-ItemProperty $key DumpType 2 # 2 = full dump +Set-ItemProperty $key DumpCount 5 +Set-ItemProperty $key DumpFolder $env:TEMP # or any writable folder +``` + +If this key is absent, `wer-dump` prints `[FAIL]` and exits 1. + +## Running locally + +```powershell +# Build first +cargo build -p secure-memory-verifier + +# Individual checks +cargo run -p secure-memory-verifier -- lock +cargo run -p secure-memory-verifier -- guard-underflow +cargo run -p secure-memory-verifier -- guard-overflow +cargo run -p secure-memory-verifier -- wer-dump # requires LocalDumps pre-config + +# All checks +cargo run -p secure-memory-verifier -- all +``` + +Exit code 0 = all checks passed. Exit code 1 = at least one check failed. +Exit code 2 = bad arguments. + +## Exact guarantees proven by each check + +### `lock` — RAM locking via QueryWorkingSetEx + +**Proves:** The kernel has pinned the secret's data page to physical RAM. +The `Locked` bit in the working-set entry is set, meaning `VirtualLock` +succeeded and the OS has honoured the lock in the working-set database. + +**Does not prove:** +- The secret was never transiently in registers or on the call stack while `expose_secret` was active. +- A kernel-mode driver cannot read the page. +- The lock holds under extreme memory pressure on all Windows editions. + +### `guard-underflow` / `guard-overflow` — Guard pages via child-process crashes + +**Proves:** Accessing one byte before or one byte after the secret's data page +immediately raises `STATUS_ACCESS_VIOLATION` (0xC0000005). The guard pages are +`PAGE_NOACCESS` — not `PAGE_GUARD` — so they are permanent (not one-shot). + +**Method:** A child process is spawned that deliberately dereferences the +guard-page address. The parent asserts the child exited with exception code +0xC0000005. + +**Does not prove:** +- Accesses that stay within the data page are detected. +- Protection is enforced in kernel mode or via DMA. +- The guard prevents attacks that skip the boundary (e.g. format-string bugs targeting arbitrary addresses). + +### `wer-dump` — WER dump exclusion + +**Proves:** When the secret's data page is registered with +`WerRegisterExcludedMemoryBlock` and a crash subsequently occurs, the +WER-generated full-memory dump does not contain the secret's canary pattern. +`ProtectedBytes::new` performs this registration automatically; the verifier +confirms it took effect end-to-end. + +**Does not prove:** +- Third-party dump tools (ProcDump, WinDbg, Task Manager minidump, …) honour + `WerRegisterExcludedMemoryBlock`. They typically do not. +- Every WER dump format or WER version behaves identically. +- Exclusion covers the full 3-page `VirtualAlloc` region (only the data page is registered). +- Full-memory dumps produced by `MiniDumpWithFullMemory` or kernel tools are excluded + (no public Windows API reliably excludes a page from those). + +## Non-guarantees (applies to all checks) + +- **Transient exposure:** The secret briefly exists on the call stack and in CPU + registers while `expose_secret()` is in use. No check here can prevent that. +- **Kernel-mode access:** Any kernel-mode component can read any user-mode page + regardless of `PAGE_NOACCESS` or `VirtualLock`. +- **Suspend-and-read attacks:** Another process with `PROCESS_VM_READ` access + can read the page between `VirtualProtect` calls during drop. +- **Hibernation / sleep:** `VirtualLock` prevents pagefile writes but does not + prevent the RAM contents from being written to the hibernation file + (`hiberfil.sys`). + +## Common failure modes + +| Symptom | Likely cause | +|---|---| +| `lock` FAIL — Locked bit not set | Process lacks `SeLockMemoryPrivilege`; increase the working-set limit | +| `guard-*` FAIL — child exits 0 | `VirtualProtect(PAGE_NOACCESS)` failed; guard pages not established | +| `guard-*` FAIL — unexpected exit code | Structured exception handler (SEH) in a DLL caught the AV; check for injected DLLs | +| `wer-dump` FAIL — not configured | WER LocalDumps registry key absent; run the setup PowerShell above | +| `wer-dump` FAIL — canary found | `WerRegisterExcludedMemoryBlock` not honoured; check WER service status | diff --git a/crates/secure-memory-verifier/src/check_guard.rs b/crates/secure-memory-verifier/src/check_guard.rs new file mode 100644 index 000000000..1846329ff --- /dev/null +++ b/crates/secure-memory-verifier/src/check_guard.rs @@ -0,0 +1,182 @@ +//! Guard-page verification via child-process crash tests. +//! +//! ## Memory layout reminder +//! +//! ```text +//! ┌──────────────┬──────────────┬──────────────┐ +//! │ guard page │ data page │ guard page │ +//! │ PAGE_NOACCESS│PAGE_READWRITE│ PAGE_NOACCESS│ +//! └──────────────┴──────────────┴──────────────┘ +//! ^base ^data ^data + ps +//! ``` +//! +//! `data` is page-aligned. Therefore: +//! - `data - 1` is the last byte of the leading guard page. +//! - `data + page_size` is the first byte of the trailing guard page. +//! +//! ## Why child processes? +//! +//! Accessing a `PAGE_NOACCESS` page raises `STATUS_ACCESS_VIOLATION` +//! (0xC0000005). There is no in-process recovery path that would allow the +//! verifier itself to continue, so each probe runs in a fresh child process. +//! The parent asserts the child exited with the expected exception code. +//! +//! ## What this proves +//! +//! Any out-of-bounds byte store/load that crosses the page boundary adjacent +//! to the data page will fault at the OS level immediately, before the value +//! can be observed by attacker-controlled code in the same process. +//! +//! ## What this does NOT prove +//! +//! - Accesses that stay within the data page are caught (they are not). +//! - Protection holds in kernel mode or via DMA. + +use std::mem; + +use secure_memory::ProtectedBytes; +use windows::Win32::System::SystemInformation::{GetSystemInfo, SYSTEM_INFO}; + +use crate::{print_check, print_fail, print_info, print_pass}; + +/// Which guard page to probe. +#[derive(Clone, Copy)] +pub(crate) enum Side { + /// Byte immediately before the data page (leading guard). + Under, + /// First byte of the trailing guard page. + Over, +} + +impl Side { + fn name(self) -> &'static str { + match self { + Side::Under => "guard-underflow", + Side::Over => "guard-overflow", + } + } + + fn child_arg(self) -> &'static str { + match self { + Side::Under => "guard-underflow", + Side::Over => "guard-overflow", + } + } +} + +// ── Parent (verifier) side ──────────────────────────────────────────────────── + +/// Run the guard-page check for the given side by spawning a child process. +pub(crate) fn run(side: Side) -> bool { + let name = side.name(); + print_check(&format!("{name}: spawning child to probe {name}")); + + let exe = match std::env::current_exe() { + Ok(p) => p, + Err(e) => { + print_fail(&format!("{name}: could not determine current executable: {e}")); + return false; + } + }; + + { + // Check the protection status in order for information. + + let probe = ProtectedBytes::<32>::new([0xAAu8; 32]); + let status = probe.protection_status(); + + if !status.guard_pages { + print_info(&format!( + "{name}: guard pages were not established (protection_status.guard_pages == false); child crash may not occur" + )); + } + } + + let child_result = std::process::Command::new(&exe) + .args(["--child", side.child_arg()]) + .status(); + + let exit_status = match child_result { + Ok(s) => s, + Err(e) => { + print_fail(&format!("{name}: failed to spawn child process: {e}")); + return false; + } + }; + + // On Windows, a process killed by an unhandled exception exits with the + // exception code as its exit code. + // STATUS_ACCESS_VIOLATION = 0xC0000005 = -1073741819i32 + const ACCESS_VIOLATION: i32 = -1073741819i32; + + match exit_status.code() { + Some(code) if code == ACCESS_VIOLATION => { + print_pass(&format!( + "{name}: child exited with STATUS_ACCESS_VIOLATION (0xC0000005) — guard page fired" + )); + true + } + Some(0) => { + print_fail(&format!( + "{name}: child exited cleanly (code 0) — guard page did NOT fire" + )); + false + } + Some(code) => { + print_fail(&format!( + "{name}: child exited with unexpected code {code:#010x} (expected STATUS_ACCESS_VIOLATION 0xC0000005)" + )); + false + } + None => { + // On Unix this would mean signal-kill; on Windows code() is always Some. + print_fail(&format!("{name}: child had no exit code")); + false + } + } +} + +// ── Child side ──────────────────────────────────────────────────────────────── + +/// Run inside the child process. Intentionally accesses a guard page, which +/// crashes with `STATUS_ACCESS_VIOLATION`. Never returns. +pub(crate) fn child(side: Side) -> ! { + let secret = ProtectedBytes::<32>::new([0x42u8; 32]); + let data_ptr = secret.expose_secret().as_ptr(); + + let access_ptr: *const u8 = match side { + Side::Under => { + // `data_ptr` is page-aligned (= base + page_size). + // Therefore `data_ptr - 1` is the last byte of the leading guard page. + // SAFETY: arithmetic only; we do not dereference here. + unsafe { data_ptr.sub(1) } + } + Side::Over => { + let ps = system_page_size(); + // `data_ptr + page_size` is the first byte of the trailing guard page. + // SAFETY: arithmetic only; we do not dereference here. + unsafe { data_ptr.add(ps) } + } + }; + + // Intentional access violation: read from a PAGE_NOACCESS guard page. + // The OS raises STATUS_ACCESS_VIOLATION immediately, terminating this process. + // This is the expected outcome; the parent process checks the exit code. + // + // SAFETY: this is deliberately unsafe — we are probing the guard page. + // The parent verifier spawns this child precisely to observe the crash. + unsafe { access_ptr.read_volatile() }; + + // Reaching here means the guard page did not fire, which is a failure. + eprintln!("child: guard page did not cause access violation — unexpected"); + std::process::exit(99) +} + +fn system_page_size() -> usize { + // SAFETY: `mem::zeroed()` is valid for `SYSTEM_INFO` — it is a struct of + // plain integer and pointer fields with no invalid bit patterns. + let mut info: SYSTEM_INFO = unsafe { mem::zeroed() }; + // SAFETY: `GetSystemInfo` writes to the provided struct; it is always safe to call. + unsafe { GetSystemInfo(&mut info) }; + info.dwPageSize as usize +} diff --git a/crates/secure-memory-verifier/src/check_lock.rs b/crates/secure-memory-verifier/src/check_lock.rs new file mode 100644 index 000000000..2894f5a51 --- /dev/null +++ b/crates/secure-memory-verifier/src/check_lock.rs @@ -0,0 +1,97 @@ +//! RAM-locking verification via `QueryWorkingSetEx`. +//! +//! ## What this proves +//! +//! `VirtualLock` returning success only means the OS *accepted* the locking +//! request. The working-set attribute is the authoritative runtime signal: the +//! kernel sets the `Locked` bit in the working-set entry only when the page is +//! genuinely pinned to physical RAM. Querying that bit via `QueryWorkingSetEx` +//! is the correct runtime verification primitive. +//! +//! ## What this does NOT prove +//! +//! - The secret value never existed in registers or on the call stack. +//! - The page cannot be read by a privileged process (e.g. a kernel-mode driver). +//! - The lock will hold under extreme memory pressure on all Windows editions. + +use std::mem::{self, size_of}; + +use secure_memory::ProtectedBytes; +use windows::Win32::System::ProcessStatus::{PSAPI_WORKING_SET_EX_INFORMATION, QueryWorkingSetEx}; +use windows::Win32::System::Threading::GetCurrentProcess; + +use crate::{print_check, print_fail, print_info, print_pass}; + +/// Bit layout of `PSAPI_WORKING_SET_EX_BLOCK.Flags` (from Windows SDK): +/// +/// ```text +/// bit 0 Valid (page is in the working set) +/// bits 1–3 ShareCount +/// bits 4–14 Win32Protection +/// bit 15 Shared +/// bits 16–21 Node +/// bit 22 Locked ← what we check +/// bit 23 LargePage +/// ``` +const LOCKED_BIT: usize = 22; + +pub(crate) fn run() -> bool { + print_check("lock: verifying data page is locked in RAM via QueryWorkingSetEx"); + + let secret = ProtectedBytes::<32>::new([0x5Au8; 32]); + let status = secret.protection_status(); + + if !status.locked { + print_info("VirtualLock was not achieved (protection_status.locked == false); QueryWorkingSetEx may still reflect this"); + } + + let data_ptr = secret.expose_secret().as_ptr(); + + // SAFETY: `mem::zeroed()` is valid for `PSAPI_WORKING_SET_EX_INFORMATION` + // because every bit pattern is a valid (if meaningless) representation + // of a struct of integers and pointers. + let mut info: PSAPI_WORKING_SET_EX_INFORMATION = unsafe { mem::zeroed() }; + info.VirtualAddress = data_ptr as *mut _; + + // SAFETY: GetCurrentProcess() always returns the pseudo-handle (-1) for the + // current process; it is always valid for the current process's lifetime. + let process = unsafe { GetCurrentProcess() }; + + let struct_size = + u32::try_from(size_of::()).expect("struct fits in u32"); + + // SAFETY: `info.VirtualAddress` is set to a page-aligned address inside our + // 3-page VirtualAlloc region; `process` is the current-process pseudo-handle; + // `struct_size` exactly covers one `PSAPI_WORKING_SET_EX_INFORMATION` entry. + let ok = unsafe { QueryWorkingSetEx(process, std::ptr::addr_of_mut!(info).cast(), struct_size) }; + + if ok.is_err() { + print_fail(&format!( + "lock: QueryWorkingSetEx failed: {}", + std::io::Error::last_os_error() + )); + return false; + } + + // SAFETY: `Flags` is a plain `usize`-sized integer field of the union. + // Reading it as an integer is always defined behaviour. + let flags: usize = unsafe { info.VirtualAttributes.Flags }; + let valid = (flags & 1) != 0; + let locked = ((flags >> LOCKED_BIT) & 1) != 0; + + if !valid { + print_fail("lock: QueryWorkingSetEx reports Valid==0; the page is not in the working set"); + return false; + } + + if locked { + print_pass("lock: Locked bit is set — page is pinned in physical RAM"); + } else { + print_fail("lock: Locked bit is NOT set — page may be swapped to disk"); + if status.locked { + print_info("lock: VirtualLock reported success but the kernel working-set entry disagrees"); + } + } + + locked +} diff --git a/crates/secure-memory-verifier/src/check_wer.rs b/crates/secure-memory-verifier/src/check_wer.rs new file mode 100644 index 000000000..2e7867822 --- /dev/null +++ b/crates/secure-memory-verifier/src/check_wer.rs @@ -0,0 +1,249 @@ +//! WER (Windows Error Reporting) dump-exclusion verification. +//! +//! ## What this proves +//! +//! `WerRegisterExcludedMemoryBlock` asks the WER subsystem to omit a specific +//! memory range from automatically-generated crash reports. This check: +//! +//! 1. Creates a `ProtectedBytes<32>` containing a known canary pattern. +//! 2. Registers the data page with `WerRegisterExcludedMemoryBlock`. +//! 3. Crashes the process intentionally so WER generates a dump. +//! 4. The parent process finds the dump and searches it for the canary. +//! 5. Absence of the canary confirms the exclusion worked. +//! +//! ## Prerequisites (WER LocalDumps must be pre-configured) +//! +//! WER only writes local dumps when the registry key is present. Configure it +//! before running this check: +//! +//! ```powershell +//! $key = "HKLM:\SOFTWARE\Microsoft\Windows\Windows Error Reporting\LocalDumps\secure-memory-verifier.exe" +//! New-Item $key -Force | Out-Null +//! Set-ItemProperty $key DumpType 2 # 2 = full dump +//! Set-ItemProperty $key DumpCount 5 +//! Set-ItemProperty $key DumpFolder $env:TEMP +//! ``` +//! +//! Administrator rights are required to create that key. +//! +//! ## What this does NOT prove +//! +//! - Third-party dump tools (ProcDump, WinDbg, procdump, …) honour +//! `WerRegisterExcludedMemoryBlock`. They typically do not (e.g.: minidump) +//! - WER exclusion covers every possible dump format or WER version. + +use std::path::{Path, PathBuf}; +use std::time::{Duration, Instant}; + +use secure_memory::ProtectedBytes; + +use crate::{print_check, print_fail, print_info, print_pass}; + +/// Unique canary placed in the secret when testing WER exclusion. +/// 32 bytes, distinctive enough to be unlikely to collide with other data. +const WER_CANARY: [u8; 32] = [ + 0xDE, 0xC0, 0xAD, 0xDE, 0xEF, 0xBE, 0xAD, 0xDE, 0x57, 0xE8, 0x44, 0x20, 0xC1, 0x0C, 0xDB, 0x20, 0xFE, 0xDC, 0xBA, + 0x98, 0x76, 0x54, 0x32, 0x10, 0xC0, 0xFF, 0xEE, 0xC0, 0xFF, 0xEE, 0x57, 0xE8, +]; + +// ── Parent side ─────────────────────────────────────────────────────────────── + +pub(crate) fn run() -> bool { + print_check("wer-dump: verifying WerRegisterExcludedMemoryBlock excludes the secret from WER crash reports"); + + let dump_folder = match find_wer_dump_folder() { + Some(f) => f, + None => { + print_fail( + "wer-dump: WER LocalDumps is not configured for this executable. \ + Run the setup commands in the README and re-run this check.", + ); + return false; + } + }; + + print_info(&format!("wer-dump: WER dump folder: {}", dump_folder.display())); + + // Record the highest existing dump mtime before spawning, so we can identify the new dump. + let baseline_time = newest_dump_mtime(&dump_folder); + + let exe = match std::env::current_exe() { + Ok(p) => p, + Err(e) => { + print_fail(&format!("wer-dump: could not determine current executable: {e}")); + return false; + } + }; + + print_info("wer-dump: spawning child process to crash with WER exclusion registered..."); + let child_result = std::process::Command::new(&exe).args(["--child", "wer-crash"]).status(); + + match child_result { + Ok(s) if s.code() == Some(0) => { + print_fail("wer-dump: child exited cleanly — it should have crashed"); + return false; + } + Err(e) => { + print_fail(&format!("wer-dump: failed to spawn child: {e}")); + return false; + } + Ok(_) => {} // non-zero exit code = expected crash + } + + // Wait up to 30 s for WER to generate the dump. + print_info("wer-dump: waiting for WER to write the crash dump (up to 30 s)..."); + let dump_path = match wait_for_new_dump(&dump_folder, baseline_time, Duration::from_secs(20)) { + Some(p) => p, + None => { + print_fail( + "wer-dump: no new dump appeared in the WER folder within 30 s. \ + Ensure WER LocalDumps is configured and the service is running.", + ); + return false; + } + }; + + print_info(&format!("wer-dump: found dump: {}", dump_path.display())); + + let dump_bytes = match std::fs::read(&dump_path) { + Ok(b) => b, + Err(e) => { + print_fail(&format!("wer-dump: could not read dump file: {e}")); + return false; + } + }; + + let canary_found = find_pattern(&dump_bytes, &WER_CANARY); + + if canary_found { + print_fail("wer-dump: canary found in WER dump — WerRegisterExcludedMemoryBlock did NOT exclude the region"); + false + } else { + print_pass("wer-dump: canary absent from WER dump — the secret was excluded from the crash report"); + true + } +} + +// ── Child side ──────────────────────────────────────────────────────────────── + +/// Run in the child process. Creates a protected secret — `ProtectedBytes::new` +/// registers WER exclusion automatically — then crashes intentionally so WER +/// generates a dump for the parent to inspect. +pub(crate) fn child_crash() -> ! { + let secret = ProtectedBytes::<32>::new(WER_CANARY); + + let status = secret.protection_status(); + if !status.dump_excluded { + eprintln!( + "child/wer-crash: WerRegisterExcludedMemoryBlock was not registered \ + (dump_excluded == false); dump may contain the canary" + ); + // Continue anyway — the crash is still useful for the dump-existence check. + } + + // Keep `secret` alive until the crash so its data page is in the dump. + let _ = secret.expose_secret().as_ptr(); + + // Crash the process. WER will generate a dump for the parent to inspect. + // SAFETY: intentional null-pointer dereference to trigger STATUS_ACCESS_VIOLATION, + // causing WER to produce a crash dump that the parent verifier inspects. + unsafe { + let null: *const u8 = std::ptr::null(); + let _ = null.read_volatile(); + } + + unreachable!("process should have crashed") +} + +// ── Helpers ─────────────────────────────────────────────────────────────────── + +/// Check the registry (read-only) for the WER LocalDumps folder configured for +/// this executable. Returns `None` if not configured. +fn find_wer_dump_folder() -> Option { + let exe_name = std::env::current_exe().ok()?; + let exe_file = exe_name.file_name()?.to_string_lossy().into_owned(); + + // Use `reg query` to avoid importing the full Win32 registry API. + let key = format!(r"HKLM\SOFTWARE\Microsoft\Windows\Windows Error Reporting\LocalDumps\{exe_file}"); + + let output = std::process::Command::new("reg") + .args(["query", &key, "/v", "DumpFolder"]) + .output() + .ok()?; + + if !output.status.success() { + // Also try the global LocalDumps key (no per-app key). + let output2 = std::process::Command::new("reg") + .args([ + "query", + r"HKLM\SOFTWARE\Microsoft\Windows\Windows Error Reporting\LocalDumps", + "/v", + "DumpFolder", + ]) + .output() + .ok()?; + + if !output2.status.success() { + return None; + } + return parse_reg_sz_value(&output2.stdout); + } + + parse_reg_sz_value(&output.stdout) +} + +fn parse_reg_sz_value(reg_output: &[u8]) -> Option { + let text = str::from_utf8(reg_output).ok()?; + // `reg query` output lines look like: + // DumpFolder REG_EXPAND_SZ C:\CrashDumps + for line in text.lines() { + if line.trim_start().starts_with("DumpFolder") { + let parts: Vec<&str> = line.split_whitespace().collect(); + if parts.len() >= 3 { + return Some(PathBuf::from(parts[2..].join(" "))); + } + } + } + None +} + +fn newest_dump_mtime(folder: &Path) -> Option { + std::fs::read_dir(folder) + .ok()? + .flatten() + .filter(|e| { + e.path() + .extension() + .map(|x| x.eq_ignore_ascii_case("dmp")) + .unwrap_or(false) + }) + .filter_map(|e| e.metadata().ok()?.modified().ok()) + .max() +} + +fn wait_for_new_dump(folder: &Path, baseline: Option, timeout: Duration) -> Option { + let start = Instant::now(); + loop { + if let Ok(entries) = std::fs::read_dir(folder) { + for entry in entries.flatten() { + let path = entry.path(); + if !path.extension().map(|x| x.eq_ignore_ascii_case("dmp")).unwrap_or(false) { + continue; + } + if let Ok(mtime) = entry.metadata().and_then(|m| m.modified()) + && baseline.map(|b| mtime > b).unwrap_or(true) + { + return Some(path); + } + } + } + if start.elapsed() >= timeout { + return None; + } + std::thread::sleep(Duration::from_millis(250)); + } +} + +fn find_pattern(haystack: &[u8], needle: &[u8]) -> bool { + haystack.windows(needle.len()).any(|w| w == needle) +} diff --git a/crates/secure-memory-verifier/src/main.rs b/crates/secure-memory-verifier/src/main.rs new file mode 100644 index 000000000..b9f6fd956 --- /dev/null +++ b/crates/secure-memory-verifier/src/main.rs @@ -0,0 +1,138 @@ +//! Windows runtime verifier for the `secure-memory` crate. +//! +//! Exercises three distinct protection tracks: +//! +//! | Track | Subcommand | How it works | +//! |---|---|---| +//! | RAM locking | `lock` | `QueryWorkingSetEx` inspects the Locked bit | +//! | Guard underflow | `guard-underflow` | child crashes on `PAGE_NOACCESS` byte before data | +//! | Guard overflow | `guard-overflow` | child crashes on `PAGE_NOACCESS` byte after data | +//! | WER exclusion | `wer-dump` | `WerRegisterExcludedMemoryBlock` + crash child + scan dump | +//! +//! Run `secure-memory-verifier all` to execute every track in sequence. + +#![allow( + clippy::print_stdout, + clippy::print_stderr, + reason = "this is a CLI tool; printing to stdout/stderr is intentional" +)] + +#[cfg(windows)] +mod check_guard; +#[cfg(windows)] +mod check_lock; +#[cfg(windows)] +mod check_wer; + +use std::process; + +// ── Output helpers ──────────────────────────────────────────────────────────── + +pub fn print_check(name: &str) { + println!("[CHECK] {name}"); +} + +pub fn print_pass(msg: &str) { + println!("[PASS] {msg}"); +} + +pub fn print_fail(msg: &str) { + eprintln!("[FAIL] {msg}"); +} + +pub fn print_info(msg: &str) { + println!("[INFO] {msg}"); +} + +pub fn print_skip(msg: &str) { + println!("[SKIP] {msg}"); +} + +// ── Entry point ─────────────────────────────────────────────────────────────── + +fn main() { + let args: Vec = std::env::args().collect(); + + // Hidden child-process modes: `--child `. + // These spawn a process that intentionally crashes or exercises a specific path. + #[cfg(windows)] + if args.get(1).map(String::as_str) == Some("--child") { + match args.get(2).map(String::as_str) { + Some("guard-underflow") => check_guard::child(check_guard::Side::Under), + Some("guard-overflow") => check_guard::child(check_guard::Side::Over), + Some("wer-crash") => check_wer::child_crash(), + other => { + eprintln!("unknown --child mode: {other:?}"); + process::exit(2); + } + } + } + + let cmd = args.get(1).map(String::as_str).unwrap_or("--help"); + + #[cfg(windows)] + { + let ok = match cmd { + "lock" => check_lock::run(), + "guard-underflow" => check_guard::run(check_guard::Side::Under), + "guard-overflow" => check_guard::run(check_guard::Side::Over), + "wer-dump" => check_wer::run(), + "all" => run_all(), + "--help" | "-h" => { + print_usage(); + return; + } + other => { + eprintln!("unknown subcommand: {other}"); + print_usage(); + process::exit(2); + } + }; + process::exit(if ok { 0 } else { 1 }); + } + + #[cfg(not(windows))] + { + eprintln!("secure-memory-verifier only runs on Windows"); + process::exit(2); + } +} + +#[cfg(windows)] +type Check = (&'static str, fn() -> bool); + +#[cfg(windows)] +fn run_all() -> bool { + let checks: &[Check] = &[ + ("lock", check_lock::run), + ("guard-underflow", || check_guard::run(check_guard::Side::Under)), + ("guard-overflow", || check_guard::run(check_guard::Side::Over)), + ("wer-dump", check_wer::run), + ]; + + let results: Vec<(&str, bool)> = checks.iter().map(|(name, f)| (*name, f())).collect(); + + println!(); + println!("=== Summary ==="); + let mut all_ok = true; + for (name, ok) in &results { + if *ok { + println!(" PASS {name}"); + } else { + println!(" FAIL {name}"); + all_ok = false; + } + } + all_ok +} + +fn print_usage() { + println!("Usage: secure-memory-verifier "); + println!(); + println!("Subcommands:"); + println!(" lock Verify data page is locked in RAM (QueryWorkingSetEx)"); + println!(" guard-underflow Verify guard page fires on access before data (child crash)"); + println!(" guard-overflow Verify guard page fires on access after data (child crash)"); + println!(" wer-dump Verify WER excludes the secret from crash dumps"); + println!(" all Run every check in sequence"); +} diff --git a/crates/secret-memory/Cargo.toml b/crates/secure-memory/Cargo.toml similarity index 89% rename from crates/secret-memory/Cargo.toml rename to crates/secure-memory/Cargo.toml index 860d53ef3..149698828 100644 --- a/crates/secret-memory/Cargo.toml +++ b/crates/secure-memory/Cargo.toml @@ -1,5 +1,5 @@ [package] -name = "secret-memory" +name = "secure-memory" version = "0.0.0" edition = "2024" license = "MIT/Apache-2.0" @@ -21,6 +21,7 @@ libc = "0.2" version = "0.61" features = [ "Win32_Foundation", + "Win32_System_ErrorReporting", "Win32_System_Memory", "Win32_System_SystemInformation", ] diff --git a/crates/secret-memory/README.md b/crates/secure-memory/README.md similarity index 98% rename from crates/secret-memory/README.md rename to crates/secure-memory/README.md index ed66b7b8a..c738f9b86 100644 --- a/crates/secret-memory/README.md +++ b/crates/secure-memory/README.md @@ -1,4 +1,4 @@ -# secret-memory +# secure-memory A minimal, auditable in-memory secret store for a single fixed-size master key. @@ -57,7 +57,7 @@ and runs. ## Usage ```rust -use secret_memory::ProtectedBytes; +use secure_memory::ProtectedBytes; let key = ProtectedBytes::new([0u8; 32]); let status = key.protection_status(); diff --git a/crates/secret-memory/src/fallback.rs b/crates/secure-memory/src/fallback.rs similarity index 95% rename from crates/secret-memory/src/fallback.rs rename to crates/secure-memory/src/fallback.rs index 2b388add7..2d8277d8b 100644 --- a/crates/secret-memory/src/fallback.rs +++ b/crates/secure-memory/src/fallback.rs @@ -44,7 +44,7 @@ fn warn_once() { static WARNED: Once = Once::new(); WARNED.call_once(|| { tracing::debug!( - "secret-memory: advanced memory protection (mlock, guard pages, \ + "secure-memory: advanced memory protection (mlock, guard pages, \ dump exclusion) is not available on this platform; \ falling back to plain heap allocation with zeroize-on-drop only" ); diff --git a/crates/secret-memory/src/lib.rs b/crates/secure-memory/src/lib.rs similarity index 91% rename from crates/secret-memory/src/lib.rs rename to crates/secure-memory/src/lib.rs index 9fd68f560..ef87ac71c 100644 --- a/crates/secret-memory/src/lib.rs +++ b/crates/secure-memory/src/lib.rs @@ -34,10 +34,16 @@ pub struct ProtectionStatus { /// Any out-of-bounds access into adjacent memory will fault at the OS level. pub guard_pages: bool, - /// The region is excluded from core / crash dumps. + /// The region is excluded from crash / core dumps. /// - /// Currently only achievable on Linux via `madvise(MADV_DONTDUMP)`. - /// Always `false` on Windows and macOS — see the crate README. + /// Platform semantics differ: + /// + /// - **Linux**: `madvise(MADV_DONTDUMP)` excludes the page from kernel + /// core dumps and most user-space dump tools that respect the VMA flags. + /// - **Windows**: `WerRegisterExcludedMemoryBlock` excludes the page from + /// WER-generated crash reports. Full-memory dumps produced by + /// `MiniDumpWithFullMemory`, ProcDump `-ma`, or kernel tools are **not** + /// affected — no public API reliably excludes a page from those. pub dump_excluded: bool, /// No OS-level hardening is available; using plain heap allocation. diff --git a/crates/secret-memory/src/unix.rs b/crates/secure-memory/src/unix.rs similarity index 96% rename from crates/secret-memory/src/unix.rs rename to crates/secure-memory/src/unix.rs index 67f694ef4..049832511 100644 --- a/crates/secret-memory/src/unix.rs +++ b/crates/secure-memory/src/unix.rs @@ -52,7 +52,7 @@ impl SecureAlloc { let ps = page_size(); assert!( N <= ps, - "secret-memory: N ({N}) exceeds page size ({ps}); not supported" + "secure-memory: N ({N}) exceeds page size ({ps}); not supported" ); let total = 3 * ps; @@ -72,7 +72,7 @@ impl SecureAlloc { }; if base_raw == libc::MAP_FAILED { - panic!("secret-memory: mmap({total}) failed; process is out of address space"); + panic!("secure-memory: mmap({total}) failed; process is out of address space"); } let base = base_raw as *mut u8; @@ -95,7 +95,7 @@ impl SecureAlloc { let guard_pages = r_guard_before == 0 && r_guard_after == 0; if !guard_pages { tracing::debug!( - "secret-memory: mprotect for guard pages failed ({}); \ + "secure-memory: mprotect for guard pages failed ({}); \ guard pages are not active", std::io::Error::last_os_error() ); @@ -107,7 +107,7 @@ impl SecureAlloc { let locked = r_lock == 0; if !locked { tracing::debug!( - "secret-memory: mlock failed ({}); \ + "secure-memory: mlock failed ({}); \ secret may be paged to disk — consider raising `ulimit -l`", std::io::Error::last_os_error() ); @@ -120,7 +120,7 @@ impl SecureAlloc { let r = unsafe { libc::madvise(data as *mut libc::c_void, ps, libc::MADV_DONTDUMP) }; if r != 0 { tracing::debug!( - "secret-memory: madvise(MADV_DONTDUMP) failed ({}); \ + "secure-memory: madvise(MADV_DONTDUMP) failed ({}); \ region may appear in core dumps", std::io::Error::last_os_error() ); diff --git a/crates/secret-memory/src/windows.rs b/crates/secure-memory/src/windows.rs similarity index 69% rename from crates/secret-memory/src/windows.rs rename to crates/secure-memory/src/windows.rs index f6577f1f3..da1d1e5c8 100644 --- a/crates/secret-memory/src/windows.rs +++ b/crates/secure-memory/src/windows.rs @@ -17,17 +17,35 @@ //! //! 1. Guard pages via `VirtualProtect(PAGE_NOACCESS)`. //! 2. RAM lock via `VirtualLock`. +//! 3. WER dump exclusion via `WerRegisterExcludedMemoryBlock`. //! -//! ## Dump-exclusion limitation +//! ## Dump-exclusion on Windows //! -//! Windows does not expose a per-region public API equivalent to Linux's `MADV_DONTDUMP`. -//! `VirtualLock` prevents the pages from being written to the pagefile but crash dumps (WER, procdump, …) will still include them. -//! `dump_excluded` is always `false` on Windows. +//! Windows does not have a single universal per-region dump-exclusion API +//! equivalent to Linux's `madvise(MADV_DONTDUMP)`. The protections that exist +//! are scoped: +//! +//! - **WER crash reports**: `WerRegisterExcludedMemoryBlock` tells the Windows +//! Error Reporting subsystem to omit the registered range from automatically- +//! generated crash dumps. This is the mechanism used here. +//! `ProtectionStatus::dump_excluded` reflects whether this registration +//! succeeded. +//! +//! - **Full-memory / forensic dumps** (`MiniDumpWithFullMemory`, kernel dumps, +//! ProcDump `-ma`, …): no public callback API reliably excludes a page from +//! these on current Windows versions. Applications that write their own +//! dumps using `MiniDumpNormal` (the typical crash-reporter default) will +//! not capture non-stack heap pages, but `MiniDumpWithFullMemory` captures +//! everything regardless of WER registration or `IncludeVmRegionCallback`. +//! +//! `VirtualLock` prevents the secret from being written to the pagefile but +//! does **not** affect dump capture. use std::ffi::c_void; use std::ptr; use std::sync::OnceLock; +use windows::Win32::System::ErrorReporting::{WerRegisterExcludedMemoryBlock, WerUnregisterExcludedMemoryBlock}; use windows::Win32::System::Memory::{ MEM_COMMIT, MEM_RELEASE, MEM_RESERVE, PAGE_NOACCESS, PAGE_PROTECTION_FLAGS, PAGE_READWRITE, VirtualAlloc, VirtualFree, VirtualLock, VirtualProtect, VirtualUnlock, @@ -44,6 +62,8 @@ pub(crate) struct SecureAlloc { data: *mut u8, /// Whether `VirtualLock` succeeded. locked: bool, + /// Whether `WerRegisterExcludedMemoryBlock` succeeded for the data page. + wer_excluded: bool, /// Marker: `SecureAlloc` logically owns a `[u8; N]`. _marker: std::marker::PhantomData<[u8; N]>, } @@ -61,7 +81,7 @@ impl SecureAlloc { let ps = page_size(); assert!( N <= ps, - "secret-memory: N ({N}) exceeds page size ({ps}); not supported" + "secure-memory: N ({N}) exceeds page size ({ps}); not supported" ); let total = 3 * ps; @@ -73,7 +93,7 @@ impl SecureAlloc { if base_raw.is_null() { panic!( - "secret-memory: VirtualAlloc({total}) failed ({})", + "secure-memory: VirtualAlloc({total}) failed ({})", std::io::Error::last_os_error() ); } @@ -99,7 +119,7 @@ impl SecureAlloc { let guard_pages = r_guard_before.is_ok() && r_guard_after.is_ok(); if !guard_pages { tracing::debug!( - "secret-memory: VirtualProtect for guard pages failed ({}); \ + "secure-memory: VirtualProtect for guard pages failed ({}); \ guard pages are not active", std::io::Error::last_os_error() ); @@ -111,7 +131,7 @@ impl SecureAlloc { let locked = r_lock.is_ok(); if !locked { tracing::debug!( - "secret-memory: VirtualLock failed ({}); \ + "secure-memory: VirtualLock failed ({}); \ secret may be paged to disk", std::io::Error::last_os_error() ); @@ -122,16 +142,36 @@ impl SecureAlloc { // non-overlapping; both are valid for `N` bytes. unsafe { ptr::copy_nonoverlapping(src.as_ptr(), data, N) }; + // ── Register the data page for WER dump exclusion ──────────────────── + // `WerRegisterExcludedMemoryBlock` asks the Windows Error Reporting + // subsystem to omit this page from automatically-generated crash dumps. + // Registration covers the full page, not just N bytes, because the + // allocation model is page-based. + + // SAFETY: `data` is a valid, committed, page-aligned pointer; `ps` is + // exactly one page — the size passed to `VirtualAlloc`. + let wer_hr = unsafe { WerRegisterExcludedMemoryBlock(data.cast::(), ps as u32) }; + let wer_excluded = wer_hr.is_ok(); + if !wer_excluded { + tracing::debug!( + "secure-memory: WerRegisterExcludedMemoryBlock failed ({wer_hr:?}); \ + the data page will not be excluded from WER crash reports" + ); + } + let alloc = SecureAlloc { base, data, locked, + wer_excluded, _marker: std::marker::PhantomData, }; let status = ProtectionStatus { locked, guard_pages, - dump_excluded: false, // no per-region dump exclusion on Windows + // On Windows, dump_excluded reflects WER exclusion only. + // See module-level documentation for scope and limitations. + dump_excluded: wer_excluded, fallback_backend: false, }; @@ -164,6 +204,14 @@ impl Drop for SecureAlloc { let _ = unsafe { VirtualUnlock(self.data as *const c_void, ps) }; } + // Unregister WER exclusion before freeing the page. + // Must happen before `VirtualFree` to avoid a dangling registration. + if self.wer_excluded { + // SAFETY: `self.data` is the same pointer passed to + // `WerRegisterExcludedMemoryBlock`; still valid here. + let _ = unsafe { WerUnregisterExcludedMemoryBlock(self.data.cast::()) }; + } + // Release the entire three-page region. // SAFETY: `self.base` is the base of the allocation; `dwSize` must be 0 // when `dwFreeType` is `MEM_RELEASE` (Windows requirement). diff --git a/devolutions-gateway/Cargo.toml b/devolutions-gateway/Cargo.toml index 27f018be5..6f5df3ce3 100644 --- a/devolutions-gateway/Cargo.toml +++ b/devolutions-gateway/Cargo.toml @@ -19,7 +19,7 @@ openapi = ["dep:utoipa"] [dependencies] # In-house -secret-memory.path = "../crates/secret-memory" +secure-memory.path = "../crates/secure-memory" transport.path = "../crates/transport" jmux-proxy.path = "../crates/jmux-proxy" devolutions-agent-shared.path = "../crates/devolutions-agent-shared" diff --git a/devolutions-gateway/src/credential/crypto.rs b/devolutions-gateway/src/credential/crypto.rs index 3edcfab25..269cdd2ca 100644 --- a/devolutions-gateway/src/credential/crypto.rs +++ b/devolutions-gateway/src/credential/crypto.rs @@ -1,7 +1,7 @@ //! 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 held in a [`ProtectedBytes<32>`] allocation backed by `secret-memory`, which applies +//! A randomly generated 256-bit master key is held in a [`ProtectedBytes<32>`] allocation backed by `secure-memory`, which applies //! the best available OS hardening (mlock, guard pages, core-dump exclusion) and always zeroizes on drop. //! //! ## Security properties @@ -22,7 +22,7 @@ use chacha20poly1305::{ChaCha20Poly1305, Nonce}; use parking_lot::Mutex; use rand::RngCore as _; use secrecy::SecretString; -use secret_memory::ProtectedBytes; +use secure_memory::ProtectedBytes; /// Global master key for credential encryption. /// @@ -32,7 +32,7 @@ use secret_memory::ProtectedBytes; /// /// A warning is logged at startup if full memory hardening is unavailable (see [`ProtectionStatus`]). /// -/// [`ProtectionStatus`]: secret_memory::ProtectionStatus +/// [`ProtectionStatus`]: secure_memory::ProtectionStatus pub(super) static MASTER_KEY: LazyLock> = LazyLock::new(|| Mutex::new(MasterKeyManager::new())); /// Manages the master encryption key. @@ -69,7 +69,7 @@ impl MasterKeyManager { ); } if !st.dump_excluded { - tracing::debug!( + tracing::warn!( "master key: core-dump exclusion is not active \ (unavailable on this platform or kernel)" ); From d36f45cd12fe48c17d2df0cba93dcabf8db26beb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20CORTIER?= Date: Thu, 26 Mar 2026 15:12:43 +0900 Subject: [PATCH 06/17] . --- crates/secure-memory-verifier/src/check_lock.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/crates/secure-memory-verifier/src/check_lock.rs b/crates/secure-memory-verifier/src/check_lock.rs index 2894f5a51..b4431633e 100644 --- a/crates/secure-memory-verifier/src/check_lock.rs +++ b/crates/secure-memory-verifier/src/check_lock.rs @@ -42,7 +42,9 @@ pub(crate) fn run() -> bool { let status = secret.protection_status(); if !status.locked { - print_info("VirtualLock was not achieved (protection_status.locked == false); QueryWorkingSetEx may still reflect this"); + print_info( + "VirtualLock was not achieved (protection_status.locked == false); QueryWorkingSetEx may still reflect this", + ); } let data_ptr = secret.expose_secret().as_ptr(); @@ -57,8 +59,7 @@ pub(crate) fn run() -> bool { // current process; it is always valid for the current process's lifetime. let process = unsafe { GetCurrentProcess() }; - let struct_size = - u32::try_from(size_of::()).expect("struct fits in u32"); + let struct_size = u32::try_from(size_of::()).expect("struct fits in u32"); // SAFETY: `info.VirtualAddress` is set to a page-aligned address inside our // 3-page VirtualAlloc region; `process` is the current-process pseudo-handle; From f1165a7435a414592e93d157fc03b8ff5f7600f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20CORTIER?= Date: Thu, 26 Mar 2026 15:27:23 +0900 Subject: [PATCH 07/17] . --- .github/workflows/ci.yml | 10 ++++++++++ crates/secure-memory-verifier/README.md | 10 +++++++++- crates/secure-memory-verifier/src/check_wer.rs | 10 +++++----- 3 files changed, 24 insertions(+), 6 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 87505f587..d653b5ecc 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1181,6 +1181,16 @@ jobs: - name: Configure WER LocalDumps shell: pwsh run: | + # Enable WER globally (CI runners often have it disabled). + $wer = "HKLM:\SOFTWARE\Microsoft\Windows\Windows Error Reporting" + Set-ItemProperty $wer -Name Disabled -Value 0 -Type DWord -Force + Set-ItemProperty $wer -Name DontShowUI -Value 1 -Type DWord -Force + Set-ItemProperty $wer -Name ForceQueue -Value 0 -Type DWord -Force + + # Start the WER service if it is not already running. + Start-Service -Name WerSvc -ErrorAction SilentlyContinue + + # Per-application LocalDumps configuration. $key = "HKLM:\SOFTWARE\Microsoft\Windows\Windows Error Reporting\LocalDumps\secure-memory-verifier.exe" New-Item $key -Force | Out-Null Set-ItemProperty $key DumpType 2 diff --git a/crates/secure-memory-verifier/README.md b/crates/secure-memory-verifier/README.md index b637977b7..a945b6c96 100644 --- a/crates/secure-memory-verifier/README.md +++ b/crates/secure-memory-verifier/README.md @@ -24,6 +24,14 @@ The `wer-dump` subcommand requires WER LocalDumps to be configured for the verifier executable. This requires administrator rights. ```powershell +# Ensure WER is enabled and writes dumps immediately (required on CI runners). +$wer = "HKLM:\SOFTWARE\Microsoft\Windows\Windows Error Reporting" +Set-ItemProperty $wer -Name Disabled -Value 0 -Type DWord -Force +Set-ItemProperty $wer -Name DontShowUI -Value 1 -Type DWord -Force +Set-ItemProperty $wer -Name ForceQueue -Value 0 -Type DWord -Force +Start-Service -Name WerSvc -ErrorAction SilentlyContinue + +# Per-application LocalDumps configuration. $key = "HKLM:\SOFTWARE\Microsoft\Windows\Windows Error Reporting\LocalDumps\secure-memory-verifier.exe" New-Item $key -Force | Out-Null Set-ItemProperty $key DumpType 2 # 2 = full dump @@ -31,7 +39,7 @@ Set-ItemProperty $key DumpCount 5 Set-ItemProperty $key DumpFolder $env:TEMP # or any writable folder ``` -If this key is absent, `wer-dump` prints `[FAIL]` and exits 1. +If the `LocalDumps` key is absent, `wer-dump` prints `[FAIL]` and exits 1. ## Running locally diff --git a/crates/secure-memory-verifier/src/check_wer.rs b/crates/secure-memory-verifier/src/check_wer.rs index 2e7867822..20ae3b0b4 100644 --- a/crates/secure-memory-verifier/src/check_wer.rs +++ b/crates/secure-memory-verifier/src/check_wer.rs @@ -90,13 +90,13 @@ pub(crate) fn run() -> bool { Ok(_) => {} // non-zero exit code = expected crash } - // Wait up to 30 s for WER to generate the dump. - print_info("wer-dump: waiting for WER to write the crash dump (up to 30 s)..."); - let dump_path = match wait_for_new_dump(&dump_folder, baseline_time, Duration::from_secs(20)) { + // Wait up to 30s for WER to generate the dump. + print_info("wer-dump: waiting for WER to write the crash dump (up to 30s)..."); + let dump_path = match wait_for_new_dump(&dump_folder, baseline_time, Duration::from_secs(30)) { Some(p) => p, None => { print_fail( - "wer-dump: no new dump appeared in the WER folder within 30 s. \ + "wer-dump: no new dump appeared in the WER folder within 30s. \ Ensure WER LocalDumps is configured and the service is running.", ); return false; @@ -240,7 +240,7 @@ fn wait_for_new_dump(folder: &Path, baseline: Option, tim if start.elapsed() >= timeout { return None; } - std::thread::sleep(Duration::from_millis(250)); + std::thread::sleep(Duration::from_millis(300)); } } From b04efe8e35cc2d2a0a471539edd858872c92f18b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20CORTIER?= Date: Thu, 26 Mar 2026 15:34:43 +0900 Subject: [PATCH 08/17] . --- .github/workflows/ci.yml | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d653b5ecc..a8e332673 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1181,12 +1181,27 @@ jobs: - name: Configure WER LocalDumps shell: pwsh run: | + # Remove any JIT debugger registration (Visual Studio / .NET register one by + # default on Windows runners). WER only handles a crash when no JIT debugger + # claims it first; with AeDebug Auto=1 WER never fires. + $aeDebug = "HKLM:\SOFTWARE\Microsoft\Windows NT\CurrentVersion\AeDebug" + if (Test-Path $aeDebug) { + Set-ItemProperty $aeDebug -Name Auto -Value 0 -Type String -Force + Set-ItemProperty $aeDebug -Name Debugger -Value "" -Type String -Force + } + # Enable WER globally (CI runners often have it disabled). $wer = "HKLM:\SOFTWARE\Microsoft\Windows\Windows Error Reporting" Set-ItemProperty $wer -Name Disabled -Value 0 -Type DWord -Force Set-ItemProperty $wer -Name DontShowUI -Value 1 -Type DWord -Force Set-ItemProperty $wer -Name ForceQueue -Value 0 -Type DWord -Force + # Override any group-policy-level WER disable. + $werPolicy = "HKLM:\SOFTWARE\Policies\Microsoft\Windows\Windows Error Reporting" + if (Test-Path $werPolicy) { + Set-ItemProperty $werPolicy -Name Disabled -Value 0 -Type DWord -Force + } + # Start the WER service if it is not already running. Start-Service -Name WerSvc -ErrorAction SilentlyContinue From 2b33232f670ebaed36866148ce39ab95f40dd0e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20CORTIER?= Date: Thu, 26 Mar 2026 16:50:31 +0900 Subject: [PATCH 09/17] . --- .github/workflows/ci.yml | 61 +------------------ crates/secure-memory-verifier/README.md | 32 ++++------ .../secure-memory-verifier/src/check_wer.rs | 18 ++---- crates/secure-memory-verifier/src/main.rs | 4 -- 4 files changed, 19 insertions(+), 96 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a8e332673..8a16b6f0c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1162,69 +1162,10 @@ jobs: psexec -accepteula -s pwsh.exe $scriptPath Get-Content -Path ./crates/pedm-simulator/pedm-simulator_run-expect-elevation.out - secure-memory-verifier: - name: Secure memory verifier - runs-on: windows-2022 - needs: [preflight] - - steps: - - name: Checkout ${{ github.repository }} - uses: actions/checkout@v4 - with: - ref: ${{ needs.preflight.outputs.ref }} - - - name: Setup Rust cache - uses: ./.github/actions/setup-rust-cache - with: - sccache-enabled: ${{ needs.preflight.outputs.sccache }} - - - name: Configure WER LocalDumps - shell: pwsh - run: | - # Remove any JIT debugger registration (Visual Studio / .NET register one by - # default on Windows runners). WER only handles a crash when no JIT debugger - # claims it first; with AeDebug Auto=1 WER never fires. - $aeDebug = "HKLM:\SOFTWARE\Microsoft\Windows NT\CurrentVersion\AeDebug" - if (Test-Path $aeDebug) { - Set-ItemProperty $aeDebug -Name Auto -Value 0 -Type String -Force - Set-ItemProperty $aeDebug -Name Debugger -Value "" -Type String -Force - } - - # Enable WER globally (CI runners often have it disabled). - $wer = "HKLM:\SOFTWARE\Microsoft\Windows\Windows Error Reporting" - Set-ItemProperty $wer -Name Disabled -Value 0 -Type DWord -Force - Set-ItemProperty $wer -Name DontShowUI -Value 1 -Type DWord -Force - Set-ItemProperty $wer -Name ForceQueue -Value 0 -Type DWord -Force - - # Override any group-policy-level WER disable. - $werPolicy = "HKLM:\SOFTWARE\Policies\Microsoft\Windows\Windows Error Reporting" - if (Test-Path $werPolicy) { - Set-ItemProperty $werPolicy -Name Disabled -Value 0 -Type DWord -Force - } - - # Start the WER service if it is not already running. - Start-Service -Name WerSvc -ErrorAction SilentlyContinue - - # Per-application LocalDumps configuration. - $key = "HKLM:\SOFTWARE\Microsoft\Windows\Windows Error Reporting\LocalDumps\secure-memory-verifier.exe" - New-Item $key -Force | Out-Null - Set-ItemProperty $key DumpType 2 - Set-ItemProperty $key DumpCount 5 - Set-ItemProperty $key DumpFolder $env:TEMP - - - name: Run secure-memory verifier - shell: pwsh - run: cargo run --release -p secure-memory-verifier -- all - - - name: Show sccache stats - if: ${{ needs.preflight.outputs.sccache == 'true' && !cancelled() }} - shell: pwsh - run: sccache --show-stats - success: name: Success if: ${{ always() }} - needs: [tests, lints, check-dependencies, jetsocat-lipo, devolutions-gateway-powershell, devolutions-gateway, devolutions-gateway-merge, devolutions-pedm-desktop, devolutions-agent, devolutions-agent-merge, devolutions-pedm-client, dotnet-utils-tests, winapi-sanitizer-tests, winapi-miri, pedm-simulator, secure-memory-verifier] + needs: [tests, lints, check-dependencies, jetsocat-lipo, devolutions-gateway-powershell, devolutions-gateway, devolutions-gateway-merge, devolutions-pedm-desktop, devolutions-agent, devolutions-agent-merge, devolutions-pedm-client, dotnet-utils-tests, winapi-sanitizer-tests, winapi-miri, pedm-simulator] runs-on: ubuntu-latest steps: diff --git a/crates/secure-memory-verifier/README.md b/crates/secure-memory-verifier/README.md index a945b6c96..331732bfe 100644 --- a/crates/secure-memory-verifier/README.md +++ b/crates/secure-memory-verifier/README.md @@ -1,16 +1,18 @@ # secure-memory-verifier A Windows-only standalone binary that verifies the runtime behaviour of the -`secret-memory` crate's four protection tracks independently. +`secure-memory` crate's four protection tracks independently. + +Run it manually on a Windows machine to confirm the OS hardening is active. ## What it checks -| Subcommand | Track | Verification method | Automated? | -|---|---|---|---| -| `lock` | RAM locking | `QueryWorkingSetEx` Locked bit | Fully automated | -| `guard-underflow` | Guard pages (leading) | Child process crashes on access before data | Fully automated | -| `guard-overflow` | Guard pages (trailing) | Child process crashes on access after data | Fully automated | -| `wer-dump` | WER dump exclusion | `WerRegisterExcludedMemoryBlock` + crash child + scan dump | Requires WER pre-config (see below) | +| Subcommand | Track | Verification method | +|---|---|---| +| `lock` | RAM locking | `QueryWorkingSetEx` Locked bit | +| `guard-underflow` | Guard pages (leading) | Child process crashes on access before data | +| `guard-overflow` | Guard pages (trailing) | Child process crashes on access after data | +| `wer-dump` | WER dump exclusion | `WerRegisterExcludedMemoryBlock` + crash child + scan dump | ## Prerequisites @@ -24,14 +26,6 @@ The `wer-dump` subcommand requires WER LocalDumps to be configured for the verifier executable. This requires administrator rights. ```powershell -# Ensure WER is enabled and writes dumps immediately (required on CI runners). -$wer = "HKLM:\SOFTWARE\Microsoft\Windows\Windows Error Reporting" -Set-ItemProperty $wer -Name Disabled -Value 0 -Type DWord -Force -Set-ItemProperty $wer -Name DontShowUI -Value 1 -Type DWord -Force -Set-ItemProperty $wer -Name ForceQueue -Value 0 -Type DWord -Force -Start-Service -Name WerSvc -ErrorAction SilentlyContinue - -# Per-application LocalDumps configuration. $key = "HKLM:\SOFTWARE\Microsoft\Windows\Windows Error Reporting\LocalDumps\secure-memory-verifier.exe" New-Item $key -Force | Out-Null Set-ItemProperty $key DumpType 2 # 2 = full dump @@ -39,7 +33,7 @@ Set-ItemProperty $key DumpCount 5 Set-ItemProperty $key DumpFolder $env:TEMP # or any writable folder ``` -If the `LocalDumps` key is absent, `wer-dump` prints `[FAIL]` and exits 1. +If the key is absent, `wer-dump` prints `[FAIL]` and exits 1. ## Running locally @@ -93,14 +87,11 @@ guard-page address. The parent asserts the child exited with exception code **Proves:** When the secret's data page is registered with `WerRegisterExcludedMemoryBlock` and a crash subsequently occurs, the WER-generated full-memory dump does not contain the secret's canary pattern. -`ProtectedBytes::new` performs this registration automatically; the verifier -confirms it took effect end-to-end. **Does not prove:** - Third-party dump tools (ProcDump, WinDbg, Task Manager minidump, …) honour `WerRegisterExcludedMemoryBlock`. They typically do not. - Every WER dump format or WER version behaves identically. -- Exclusion covers the full 3-page `VirtualAlloc` region (only the data page is registered). - Full-memory dumps produced by `MiniDumpWithFullMemory` or kernel tools are excluded (no public Windows API reliably excludes a page from those). @@ -124,4 +115,5 @@ confirms it took effect end-to-end. | `guard-*` FAIL — child exits 0 | `VirtualProtect(PAGE_NOACCESS)` failed; guard pages not established | | `guard-*` FAIL — unexpected exit code | Structured exception handler (SEH) in a DLL caught the AV; check for injected DLLs | | `wer-dump` FAIL — not configured | WER LocalDumps registry key absent; run the setup PowerShell above | -| `wer-dump` FAIL — canary found | `WerRegisterExcludedMemoryBlock` not honoured; check WER service status | +| `wer-dump` FAIL — no dump within 30s | WER service not running, or a JIT debugger is intercepting the crash | +| `wer-dump` FAIL — canary found | `WerRegisterExcludedMemoryBlock` not honoured by WER | diff --git a/crates/secure-memory-verifier/src/check_wer.rs b/crates/secure-memory-verifier/src/check_wer.rs index 20ae3b0b4..1bdacf335 100644 --- a/crates/secure-memory-verifier/src/check_wer.rs +++ b/crates/secure-memory-verifier/src/check_wer.rs @@ -6,10 +6,9 @@ //! memory range from automatically-generated crash reports. This check: //! //! 1. Creates a `ProtectedBytes<32>` containing a known canary pattern. -//! 2. Registers the data page with `WerRegisterExcludedMemoryBlock`. -//! 3. Crashes the process intentionally so WER generates a dump. -//! 4. The parent process finds the dump and searches it for the canary. -//! 5. Absence of the canary confirms the exclusion worked. +//! 2. Crashes a child process so WER generates a full-memory dump. +//! 3. The parent process finds the dump and searches it for the canary. +//! 4. Absence of the canary confirms WER honoured the exclusion end-to-end. //! //! ## Prerequisites (WER LocalDumps must be pre-configured) //! @@ -28,8 +27,8 @@ //! //! ## What this does NOT prove //! -//! - Third-party dump tools (ProcDump, WinDbg, procdump, …) honour -//! `WerRegisterExcludedMemoryBlock`. They typically do not (e.g.: minidump) +//! - Third-party dump tools (ProcDump, WinDbg, …) honour +//! `WerRegisterExcludedMemoryBlock`. They typically do not. //! - WER exclusion covers every possible dump format or WER version. use std::path::{Path, PathBuf}; @@ -46,8 +45,6 @@ const WER_CANARY: [u8; 32] = [ 0x98, 0x76, 0x54, 0x32, 0x10, 0xC0, 0xFF, 0xEE, 0xC0, 0xFF, 0xEE, 0x57, 0xE8, ]; -// ── Parent side ─────────────────────────────────────────────────────────────── - pub(crate) fn run() -> bool { print_check("wer-dump: verifying WerRegisterExcludedMemoryBlock excludes the secret from WER crash reports"); @@ -90,7 +87,6 @@ pub(crate) fn run() -> bool { Ok(_) => {} // non-zero exit code = expected crash } - // Wait up to 30s for WER to generate the dump. print_info("wer-dump: waiting for WER to write the crash dump (up to 30s)..."); let dump_path = match wait_for_new_dump(&dump_folder, baseline_time, Duration::from_secs(30)) { Some(p) => p, @@ -113,9 +109,7 @@ pub(crate) fn run() -> bool { } }; - let canary_found = find_pattern(&dump_bytes, &WER_CANARY); - - if canary_found { + if find_pattern(&dump_bytes, &WER_CANARY) { print_fail("wer-dump: canary found in WER dump — WerRegisterExcludedMemoryBlock did NOT exclude the region"); false } else { diff --git a/crates/secure-memory-verifier/src/main.rs b/crates/secure-memory-verifier/src/main.rs index b9f6fd956..65ee9852d 100644 --- a/crates/secure-memory-verifier/src/main.rs +++ b/crates/secure-memory-verifier/src/main.rs @@ -44,10 +44,6 @@ pub fn print_info(msg: &str) { println!("[INFO] {msg}"); } -pub fn print_skip(msg: &str) { - println!("[SKIP] {msg}"); -} - // ── Entry point ─────────────────────────────────────────────────────────────── fn main() { From d6d039883f8d228f0a857e3199322e5b974cd15a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20CORTIER?= Date: Thu, 26 Mar 2026 16:54:45 +0900 Subject: [PATCH 10/17] . --- README.md | 13 ------------- devolutions-gateway/src/api/preflight.rs | 2 +- devolutions-gateway/src/credential/crypto.rs | 2 +- 3 files changed, 2 insertions(+), 15 deletions(-) diff --git a/README.md b/README.md index 31709e049..563a6b834 100644 --- a/README.md +++ b/README.md @@ -26,18 +26,6 @@ Ensure that you have [the Rust toolchain installed][install_rust] and then clone 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. @@ -351,7 +339,6 @@ 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/src/api/preflight.rs b/devolutions-gateway/src/api/preflight.rs index 24929b5f1..e63d3a046 100644 --- a/devolutions-gateway/src/api/preflight.rs +++ b/devolutions-gateway/src/api/preflight.rs @@ -345,7 +345,7 @@ async fn handle_operation( PreflightError::new(PreflightAlertStatus::InvalidParams, format!("{e:#}")) } InsertError::Internal(_) => { - PreflightError::new(PreflightAlertStatus::InternalServerError, format!("{e:#}")) + PreflightError::new(PreflightAlertStatus::InternalServerError, "an internal error occurred".to_owned()) } })?; diff --git a/devolutions-gateway/src/credential/crypto.rs b/devolutions-gateway/src/credential/crypto.rs index 269cdd2ca..c7124c0a4 100644 --- a/devolutions-gateway/src/credential/crypto.rs +++ b/devolutions-gateway/src/credential/crypto.rs @@ -30,7 +30,7 @@ use secure_memory::ProtectedBytes; /// The key is stored in a [`ProtectedBytes<32>`] allocation. /// A [`Mutex`] provides thread-safe interior mutability. /// -/// A warning is logged at startup if full memory hardening is unavailable (see [`ProtectionStatus`]). +/// A warning is logged when the master key is first initialized if full memory hardening is unavailable (see [`ProtectionStatus`]). /// /// [`ProtectionStatus`]: secure_memory::ProtectionStatus pub(super) static MASTER_KEY: LazyLock> = LazyLock::new(|| Mutex::new(MasterKeyManager::new())); From b7733d27443a2b54d51a7f91079d682988631982 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20CORTIER?= Date: Thu, 26 Mar 2026 16:55:40 +0900 Subject: [PATCH 11/17] . --- devolutions-gateway/src/api/preflight.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/devolutions-gateway/src/api/preflight.rs b/devolutions-gateway/src/api/preflight.rs index e63d3a046..a144314bf 100644 --- a/devolutions-gateway/src/api/preflight.rs +++ b/devolutions-gateway/src/api/preflight.rs @@ -344,9 +344,10 @@ async fn handle_operation( InsertError::InvalidToken(_) => { PreflightError::new(PreflightAlertStatus::InvalidParams, format!("{e:#}")) } - InsertError::Internal(_) => { - PreflightError::new(PreflightAlertStatus::InternalServerError, "an internal error occurred".to_owned()) - } + InsertError::Internal(_) => PreflightError::new( + PreflightAlertStatus::InternalServerError, + "an internal error occurred".to_owned(), + ), })?; if previous_entry.is_some() { From 9067fc2fe97dcad2f04af5b1e869a204d205e2ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20CORTIER?= Date: Fri, 27 Mar 2026 11:04:27 +0900 Subject: [PATCH 12/17] . --- .github/workflows/ci.yml | 27 +- crates/secure-memory-verifier/Cargo.toml | 3 - crates/secure-memory-verifier/README.md | 44 +--- .../secure-memory-verifier/src/check_wer.rs | 243 ------------------ crates/secure-memory-verifier/src/main.rs | 7 - crates/secure-memory/README.md | 21 +- crates/secure-memory/src/fallback.rs | 4 +- crates/secure-memory/src/lib.rs | 43 ++-- crates/secure-memory/src/unix.rs | 38 ++- crates/secure-memory/src/windows.rs | 83 +++--- 10 files changed, 151 insertions(+), 362 deletions(-) delete mode 100644 crates/secure-memory-verifier/src/check_wer.rs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8a16b6f0c..c3dd8ca5e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1162,10 +1162,35 @@ jobs: psexec -accepteula -s pwsh.exe $scriptPath Get-Content -Path ./crates/pedm-simulator/pedm-simulator_run-expect-elevation.out + secure-memory-verifier: + name: secure-memory-verifier + runs-on: windows-2022 + needs: [preflight] + + steps: + - name: Checkout ${{ github.repository }} + uses: actions/checkout@v4 + with: + ref: ${{ needs.preflight.outputs.ref }} + + - name: Setup Rust cache + uses: ./.github/actions/setup-rust-cache + with: + sccache-enabled: ${{ needs.preflight.outputs.sccache }} + + - name: Run secure-memory-verifier + run: cargo run -p secure-memory-verifier -- all + shell: pwsh + + - name: Show sccache stats + if: ${{ needs.preflight.outputs.sccache == 'true' && !cancelled() }} + shell: pwsh + run: sccache --show-stats + success: name: Success if: ${{ always() }} - needs: [tests, lints, check-dependencies, jetsocat-lipo, devolutions-gateway-powershell, devolutions-gateway, devolutions-gateway-merge, devolutions-pedm-desktop, devolutions-agent, devolutions-agent-merge, devolutions-pedm-client, dotnet-utils-tests, winapi-sanitizer-tests, winapi-miri, pedm-simulator] + needs: [tests, lints, check-dependencies, jetsocat-lipo, devolutions-gateway-powershell, devolutions-gateway, devolutions-gateway-merge, devolutions-pedm-desktop, devolutions-agent, devolutions-agent-merge, devolutions-pedm-client, dotnet-utils-tests, winapi-sanitizer-tests, winapi-miri, pedm-simulator, secure-memory-verifier] runs-on: ubuntu-latest steps: diff --git a/crates/secure-memory-verifier/Cargo.toml b/crates/secure-memory-verifier/Cargo.toml index 741f79365..f930168f4 100644 --- a/crates/secure-memory-verifier/Cargo.toml +++ b/crates/secure-memory-verifier/Cargo.toml @@ -16,9 +16,6 @@ secure-memory = { path = "../secure-memory" } version = "0.61" features = [ "Win32_Foundation", - "Win32_Storage_FileSystem", - "Win32_System_Diagnostics_Debug", - "Win32_System_ErrorReporting", "Win32_System_Kernel", "Win32_System_Memory", "Win32_System_ProcessStatus", diff --git a/crates/secure-memory-verifier/README.md b/crates/secure-memory-verifier/README.md index 331732bfe..37c2163aa 100644 --- a/crates/secure-memory-verifier/README.md +++ b/crates/secure-memory-verifier/README.md @@ -1,7 +1,7 @@ # secure-memory-verifier A Windows-only standalone binary that verifies the runtime behaviour of the -`secure-memory` crate's four protection tracks independently. +`secure-memory` crate's protection tracks. Run it manually on a Windows machine to confirm the OS hardening is active. @@ -12,7 +12,13 @@ Run it manually on a Windows machine to confirm the OS hardening is active. | `lock` | RAM locking | `QueryWorkingSetEx` Locked bit | | `guard-underflow` | Guard pages (leading) | Child process crashes on access before data | | `guard-overflow` | Guard pages (trailing) | Child process crashes on access after data | -| `wer-dump` | WER dump exclusion | `WerRegisterExcludedMemoryBlock` + crash child + scan dump | + +> **Note on WER dump exclusion:** `WerRegisterExcludedMemoryBlock` is called by +> `ProtectedBytes::new` but is not verified here. It registers the data page for +> exclusion from WER crash reports sent to Microsoft Watson only. Full-memory +> dumps (`MiniDumpWithFullMemory`, ProcDump `-ma`, LocalDumps `DumpType=2`, +> kernel dumps) capture all committed read/write pages regardless. No public +> Windows API reliably excludes a page from those. ## Prerequisites @@ -20,21 +26,6 @@ Run it manually on a Windows machine to confirm the OS hardening is active. - Rust toolchain with `x86_64-pc-windows-msvc` or `aarch64-pc-windows-msvc` target - `cargo build -p secure-memory-verifier` or `cargo run -p secure-memory-verifier` -### WER dump-exclusion check prerequisites - -The `wer-dump` subcommand requires WER LocalDumps to be configured for the -verifier executable. This requires administrator rights. - -```powershell -$key = "HKLM:\SOFTWARE\Microsoft\Windows\Windows Error Reporting\LocalDumps\secure-memory-verifier.exe" -New-Item $key -Force | Out-Null -Set-ItemProperty $key DumpType 2 # 2 = full dump -Set-ItemProperty $key DumpCount 5 -Set-ItemProperty $key DumpFolder $env:TEMP # or any writable folder -``` - -If the key is absent, `wer-dump` prints `[FAIL]` and exits 1. - ## Running locally ```powershell @@ -45,7 +36,6 @@ cargo build -p secure-memory-verifier cargo run -p secure-memory-verifier -- lock cargo run -p secure-memory-verifier -- guard-underflow cargo run -p secure-memory-verifier -- guard-overflow -cargo run -p secure-memory-verifier -- wer-dump # requires LocalDumps pre-config # All checks cargo run -p secure-memory-verifier -- all @@ -82,19 +72,6 @@ guard-page address. The parent asserts the child exited with exception code - Protection is enforced in kernel mode or via DMA. - The guard prevents attacks that skip the boundary (e.g. format-string bugs targeting arbitrary addresses). -### `wer-dump` — WER dump exclusion - -**Proves:** When the secret's data page is registered with -`WerRegisterExcludedMemoryBlock` and a crash subsequently occurs, the -WER-generated full-memory dump does not contain the secret's canary pattern. - -**Does not prove:** -- Third-party dump tools (ProcDump, WinDbg, Task Manager minidump, …) honour - `WerRegisterExcludedMemoryBlock`. They typically do not. -- Every WER dump format or WER version behaves identically. -- Full-memory dumps produced by `MiniDumpWithFullMemory` or kernel tools are excluded - (no public Windows API reliably excludes a page from those). - ## Non-guarantees (applies to all checks) - **Transient exposure:** The secret briefly exists on the call stack and in CPU @@ -106,6 +83,8 @@ WER-generated full-memory dump does not contain the secret's canary pattern. - **Hibernation / sleep:** `VirtualLock` prevents pagefile writes but does not prevent the RAM contents from being written to the hibernation file (`hiberfil.sys`). +- **Full-memory dumps:** `WerRegisterExcludedMemoryBlock` does not exclude the + page from `MiniDumpWithFullMemory` or any locally-captured full dump. ## Common failure modes @@ -114,6 +93,3 @@ WER-generated full-memory dump does not contain the secret's canary pattern. | `lock` FAIL — Locked bit not set | Process lacks `SeLockMemoryPrivilege`; increase the working-set limit | | `guard-*` FAIL — child exits 0 | `VirtualProtect(PAGE_NOACCESS)` failed; guard pages not established | | `guard-*` FAIL — unexpected exit code | Structured exception handler (SEH) in a DLL caught the AV; check for injected DLLs | -| `wer-dump` FAIL — not configured | WER LocalDumps registry key absent; run the setup PowerShell above | -| `wer-dump` FAIL — no dump within 30s | WER service not running, or a JIT debugger is intercepting the crash | -| `wer-dump` FAIL — canary found | `WerRegisterExcludedMemoryBlock` not honoured by WER | diff --git a/crates/secure-memory-verifier/src/check_wer.rs b/crates/secure-memory-verifier/src/check_wer.rs deleted file mode 100644 index 1bdacf335..000000000 --- a/crates/secure-memory-verifier/src/check_wer.rs +++ /dev/null @@ -1,243 +0,0 @@ -//! WER (Windows Error Reporting) dump-exclusion verification. -//! -//! ## What this proves -//! -//! `WerRegisterExcludedMemoryBlock` asks the WER subsystem to omit a specific -//! memory range from automatically-generated crash reports. This check: -//! -//! 1. Creates a `ProtectedBytes<32>` containing a known canary pattern. -//! 2. Crashes a child process so WER generates a full-memory dump. -//! 3. The parent process finds the dump and searches it for the canary. -//! 4. Absence of the canary confirms WER honoured the exclusion end-to-end. -//! -//! ## Prerequisites (WER LocalDumps must be pre-configured) -//! -//! WER only writes local dumps when the registry key is present. Configure it -//! before running this check: -//! -//! ```powershell -//! $key = "HKLM:\SOFTWARE\Microsoft\Windows\Windows Error Reporting\LocalDumps\secure-memory-verifier.exe" -//! New-Item $key -Force | Out-Null -//! Set-ItemProperty $key DumpType 2 # 2 = full dump -//! Set-ItemProperty $key DumpCount 5 -//! Set-ItemProperty $key DumpFolder $env:TEMP -//! ``` -//! -//! Administrator rights are required to create that key. -//! -//! ## What this does NOT prove -//! -//! - Third-party dump tools (ProcDump, WinDbg, …) honour -//! `WerRegisterExcludedMemoryBlock`. They typically do not. -//! - WER exclusion covers every possible dump format or WER version. - -use std::path::{Path, PathBuf}; -use std::time::{Duration, Instant}; - -use secure_memory::ProtectedBytes; - -use crate::{print_check, print_fail, print_info, print_pass}; - -/// Unique canary placed in the secret when testing WER exclusion. -/// 32 bytes, distinctive enough to be unlikely to collide with other data. -const WER_CANARY: [u8; 32] = [ - 0xDE, 0xC0, 0xAD, 0xDE, 0xEF, 0xBE, 0xAD, 0xDE, 0x57, 0xE8, 0x44, 0x20, 0xC1, 0x0C, 0xDB, 0x20, 0xFE, 0xDC, 0xBA, - 0x98, 0x76, 0x54, 0x32, 0x10, 0xC0, 0xFF, 0xEE, 0xC0, 0xFF, 0xEE, 0x57, 0xE8, -]; - -pub(crate) fn run() -> bool { - print_check("wer-dump: verifying WerRegisterExcludedMemoryBlock excludes the secret from WER crash reports"); - - let dump_folder = match find_wer_dump_folder() { - Some(f) => f, - None => { - print_fail( - "wer-dump: WER LocalDumps is not configured for this executable. \ - Run the setup commands in the README and re-run this check.", - ); - return false; - } - }; - - print_info(&format!("wer-dump: WER dump folder: {}", dump_folder.display())); - - // Record the highest existing dump mtime before spawning, so we can identify the new dump. - let baseline_time = newest_dump_mtime(&dump_folder); - - let exe = match std::env::current_exe() { - Ok(p) => p, - Err(e) => { - print_fail(&format!("wer-dump: could not determine current executable: {e}")); - return false; - } - }; - - print_info("wer-dump: spawning child process to crash with WER exclusion registered..."); - let child_result = std::process::Command::new(&exe).args(["--child", "wer-crash"]).status(); - - match child_result { - Ok(s) if s.code() == Some(0) => { - print_fail("wer-dump: child exited cleanly — it should have crashed"); - return false; - } - Err(e) => { - print_fail(&format!("wer-dump: failed to spawn child: {e}")); - return false; - } - Ok(_) => {} // non-zero exit code = expected crash - } - - print_info("wer-dump: waiting for WER to write the crash dump (up to 30s)..."); - let dump_path = match wait_for_new_dump(&dump_folder, baseline_time, Duration::from_secs(30)) { - Some(p) => p, - None => { - print_fail( - "wer-dump: no new dump appeared in the WER folder within 30s. \ - Ensure WER LocalDumps is configured and the service is running.", - ); - return false; - } - }; - - print_info(&format!("wer-dump: found dump: {}", dump_path.display())); - - let dump_bytes = match std::fs::read(&dump_path) { - Ok(b) => b, - Err(e) => { - print_fail(&format!("wer-dump: could not read dump file: {e}")); - return false; - } - }; - - if find_pattern(&dump_bytes, &WER_CANARY) { - print_fail("wer-dump: canary found in WER dump — WerRegisterExcludedMemoryBlock did NOT exclude the region"); - false - } else { - print_pass("wer-dump: canary absent from WER dump — the secret was excluded from the crash report"); - true - } -} - -// ── Child side ──────────────────────────────────────────────────────────────── - -/// Run in the child process. Creates a protected secret — `ProtectedBytes::new` -/// registers WER exclusion automatically — then crashes intentionally so WER -/// generates a dump for the parent to inspect. -pub(crate) fn child_crash() -> ! { - let secret = ProtectedBytes::<32>::new(WER_CANARY); - - let status = secret.protection_status(); - if !status.dump_excluded { - eprintln!( - "child/wer-crash: WerRegisterExcludedMemoryBlock was not registered \ - (dump_excluded == false); dump may contain the canary" - ); - // Continue anyway — the crash is still useful for the dump-existence check. - } - - // Keep `secret` alive until the crash so its data page is in the dump. - let _ = secret.expose_secret().as_ptr(); - - // Crash the process. WER will generate a dump for the parent to inspect. - // SAFETY: intentional null-pointer dereference to trigger STATUS_ACCESS_VIOLATION, - // causing WER to produce a crash dump that the parent verifier inspects. - unsafe { - let null: *const u8 = std::ptr::null(); - let _ = null.read_volatile(); - } - - unreachable!("process should have crashed") -} - -// ── Helpers ─────────────────────────────────────────────────────────────────── - -/// Check the registry (read-only) for the WER LocalDumps folder configured for -/// this executable. Returns `None` if not configured. -fn find_wer_dump_folder() -> Option { - let exe_name = std::env::current_exe().ok()?; - let exe_file = exe_name.file_name()?.to_string_lossy().into_owned(); - - // Use `reg query` to avoid importing the full Win32 registry API. - let key = format!(r"HKLM\SOFTWARE\Microsoft\Windows\Windows Error Reporting\LocalDumps\{exe_file}"); - - let output = std::process::Command::new("reg") - .args(["query", &key, "/v", "DumpFolder"]) - .output() - .ok()?; - - if !output.status.success() { - // Also try the global LocalDumps key (no per-app key). - let output2 = std::process::Command::new("reg") - .args([ - "query", - r"HKLM\SOFTWARE\Microsoft\Windows\Windows Error Reporting\LocalDumps", - "/v", - "DumpFolder", - ]) - .output() - .ok()?; - - if !output2.status.success() { - return None; - } - return parse_reg_sz_value(&output2.stdout); - } - - parse_reg_sz_value(&output.stdout) -} - -fn parse_reg_sz_value(reg_output: &[u8]) -> Option { - let text = str::from_utf8(reg_output).ok()?; - // `reg query` output lines look like: - // DumpFolder REG_EXPAND_SZ C:\CrashDumps - for line in text.lines() { - if line.trim_start().starts_with("DumpFolder") { - let parts: Vec<&str> = line.split_whitespace().collect(); - if parts.len() >= 3 { - return Some(PathBuf::from(parts[2..].join(" "))); - } - } - } - None -} - -fn newest_dump_mtime(folder: &Path) -> Option { - std::fs::read_dir(folder) - .ok()? - .flatten() - .filter(|e| { - e.path() - .extension() - .map(|x| x.eq_ignore_ascii_case("dmp")) - .unwrap_or(false) - }) - .filter_map(|e| e.metadata().ok()?.modified().ok()) - .max() -} - -fn wait_for_new_dump(folder: &Path, baseline: Option, timeout: Duration) -> Option { - let start = Instant::now(); - loop { - if let Ok(entries) = std::fs::read_dir(folder) { - for entry in entries.flatten() { - let path = entry.path(); - if !path.extension().map(|x| x.eq_ignore_ascii_case("dmp")).unwrap_or(false) { - continue; - } - if let Ok(mtime) = entry.metadata().and_then(|m| m.modified()) - && baseline.map(|b| mtime > b).unwrap_or(true) - { - return Some(path); - } - } - } - if start.elapsed() >= timeout { - return None; - } - std::thread::sleep(Duration::from_millis(300)); - } -} - -fn find_pattern(haystack: &[u8], needle: &[u8]) -> bool { - haystack.windows(needle.len()).any(|w| w == needle) -} diff --git a/crates/secure-memory-verifier/src/main.rs b/crates/secure-memory-verifier/src/main.rs index 65ee9852d..574d3f764 100644 --- a/crates/secure-memory-verifier/src/main.rs +++ b/crates/secure-memory-verifier/src/main.rs @@ -7,7 +7,6 @@ //! | RAM locking | `lock` | `QueryWorkingSetEx` inspects the Locked bit | //! | Guard underflow | `guard-underflow` | child crashes on `PAGE_NOACCESS` byte before data | //! | Guard overflow | `guard-overflow` | child crashes on `PAGE_NOACCESS` byte after data | -//! | WER exclusion | `wer-dump` | `WerRegisterExcludedMemoryBlock` + crash child + scan dump | //! //! Run `secure-memory-verifier all` to execute every track in sequence. @@ -21,8 +20,6 @@ mod check_guard; #[cfg(windows)] mod check_lock; -#[cfg(windows)] -mod check_wer; use std::process; @@ -56,7 +53,6 @@ fn main() { match args.get(2).map(String::as_str) { Some("guard-underflow") => check_guard::child(check_guard::Side::Under), Some("guard-overflow") => check_guard::child(check_guard::Side::Over), - Some("wer-crash") => check_wer::child_crash(), other => { eprintln!("unknown --child mode: {other:?}"); process::exit(2); @@ -72,7 +68,6 @@ fn main() { "lock" => check_lock::run(), "guard-underflow" => check_guard::run(check_guard::Side::Under), "guard-overflow" => check_guard::run(check_guard::Side::Over), - "wer-dump" => check_wer::run(), "all" => run_all(), "--help" | "-h" => { print_usage(); @@ -103,7 +98,6 @@ fn run_all() -> bool { ("lock", check_lock::run), ("guard-underflow", || check_guard::run(check_guard::Side::Under)), ("guard-overflow", || check_guard::run(check_guard::Side::Over)), - ("wer-dump", check_wer::run), ]; let results: Vec<(&str, bool)> = checks.iter().map(|(name, f)| (*name, f())).collect(); @@ -129,6 +123,5 @@ fn print_usage() { println!(" lock Verify data page is locked in RAM (QueryWorkingSetEx)"); println!(" guard-underflow Verify guard page fires on access before data (child crash)"); println!(" guard-overflow Verify guard page fires on access after data (child crash)"); - println!(" wer-dump Verify WER excludes the secret from crash dumps"); println!(" all Run every check in sequence"); } diff --git a/crates/secure-memory/README.md b/crates/secure-memory/README.md index c738f9b86..a328f026c 100644 --- a/crates/secure-memory/README.md +++ b/crates/secure-memory/README.md @@ -13,7 +13,8 @@ It is intentionally **not** a general-purpose secret library. **Protected against:** - Swapping the secret to disk (via `mlock` / `VirtualLock`). -- The secret appearing in Linux core dumps (via `madvise(MADV_DONTDUMP)`). +- The secret appearing in Linux core dumps (`madvise(MADV_DONTDUMP)`) and + Windows Error Reporting (WER) crash reports (`WerRegisterExcludedMemoryBlock`). - Adjacent heap corruption reaching the secret (via guard pages). - Accidental logging (redacted `Debug`, no `Display`). - Residual bytes after the secret is dropped (zeroize-before-free). @@ -35,13 +36,17 @@ It is intentionally **not** a general-purpose secret library. | Page allocation | `mmap(MAP_ANON)` | `VirtualAlloc` | `Box` heap | | Guard pages | `mprotect(PROT_NONE)` | `VirtualProtect(PAGE_NOACCESS)` | ✗ | | RAM lock | `mlock` | `VirtualLock` | ✗ | -| Dump exclusion | `MADV_DONTDUMP` | ✗ (see note) | ✗ | +| Write protect | `mprotect(PROT_READ)` | `VirtualProtect(PAGE_READONLY)` | ✗ | +| Dump exclusion | `MADV_DONTDUMP` | `WerRegisterExcludedMemoryBlock` (see note) | ✗ | | Zeroize on drop | ✓ | ✓ | ✓ | -**Windows dump exclusion note:** Windows does not expose a per-region public API -equivalent to `MADV_DONTDUMP`. `VirtualLock` prevents paging, which avoids -pagefile-based exposure, but crash dumps (WER, procdump, …) will include the -locked pages. `dump_excluded` is always `false` on Windows. +**Windows dump exclusion note:** `WerRegisterExcludedMemoryBlock` registers the +data page for exclusion from WER crash reports sent to Microsoft Watson. +`dump_excluded = true` means this registration succeeded. It does **not** imply +universal protection: full-memory dumps (`MiniDumpWithFullMemory`, ProcDump +`-ma`, LocalDumps `DumpType=2`, kernel dumps) capture all committed read/write +pages regardless. `MiniDumpWriteDump` callbacks can filter regions but only for +cooperating dump writers, not externally triggered dumps. **macOS note:** The Unix backend compiles for macOS (mmap + guard pages + mlock), but `MADV_DONTDUMP` is Linux-only, so `dump_excluded` is always `false` on macOS. @@ -50,8 +55,8 @@ macOS support is not tested in CI. ## Fallback behavior On platforms where neither the Unix nor Windows backend compiles, the crate falls -back to a plain `Box<[u8; N]>` with `zeroize`-on-drop. A warning is logged -once at construction time. No feature flag is required; the crate always compiles +back to a plain `Box<[u8; N]>` with `zeroize`-on-drop. A debug message is +logged once at construction time. No feature flag is required; the crate always compiles and runs. ## Usage diff --git a/crates/secure-memory/src/fallback.rs b/crates/secure-memory/src/fallback.rs index 2d8277d8b..bb2e83556 100644 --- a/crates/secure-memory/src/fallback.rs +++ b/crates/secure-memory/src/fallback.rs @@ -1,8 +1,8 @@ //! Fallback backend: plain heap allocation with zeroize-on-drop. //! //! Used on platforms where neither the Unix nor the Windows backend is -//! available. All hardening features are absent; a message is logged once -//! at construction time. +//! available. All hardening features are absent; a debug message is logged +//! once at construction time. use std::sync::Once; diff --git a/crates/secure-memory/src/lib.rs b/crates/secure-memory/src/lib.rs index ef87ac71c..8da5c6d14 100644 --- a/crates/secure-memory/src/lib.rs +++ b/crates/secure-memory/src/lib.rs @@ -34,42 +34,39 @@ pub struct ProtectionStatus { /// Any out-of-bounds access into adjacent memory will fault at the OS level. pub guard_pages: bool, - /// The region is excluded from crash / core dumps. + /// The page is registered for exclusion from crash dumps. /// - /// Platform semantics differ: - /// - /// - **Linux**: `madvise(MADV_DONTDUMP)` excludes the page from kernel - /// core dumps and most user-space dump tools that respect the VMA flags. - /// - **Windows**: `WerRegisterExcludedMemoryBlock` excludes the page from - /// WER-generated crash reports. Full-memory dumps produced by - /// `MiniDumpWithFullMemory`, ProcDump `-ma`, or kernel tools are **not** - /// affected — no public API reliably excludes a page from those. + /// - **Linux**: `madvise(MADV_DONTDUMP)` — excluded from kernel core dumps + /// and user-space tools that respect VMA flags. + /// - **Windows**: `WerRegisterExcludedMemoryBlock` — excluded from the mini + /// dump embedded in WER crash reports sent to Microsoft Watson only. + /// Full-memory dumps (`MiniDumpWithFullMemory`, ProcDump `-ma`, kernel live + /// dumps) capture all committed pages regardless. `MiniDumpWriteDump` + /// callbacks (`RemoveMemoryCallback` / `IncludeVmRegionCallback`) can filter + /// regions but only for cooperating dump writers, not externally triggered + /// dumps. + /// - **macOS**: always `false`; no equivalent API exists. pub dump_excluded: bool, /// No OS-level hardening is available; using plain heap allocation. /// /// The secret is still zeroized on drop but none of the other protections - /// are active. A warning is logged once at construction time. + /// are active. A debug message is logged once at construction time. pub fallback_backend: bool, } /// A fixed-size, protected in-memory secret. /// /// On supported platforms the backing storage is a dedicated page-based -/// allocation with guard pages, memory locking, and (where available) -/// exclusion from core dumps. On all platforms the bytes are zeroized -/// before the backing allocation is released. -/// -/// # Safety properties +/// allocation with guard pages, memory locking, read-only page protection after +/// construction, and (where available) exclusion from core dumps. On all +/// platforms the bytes are zeroized before the backing allocation is released. /// -/// - Never printed: `Debug` emits `[REDACTED]`; `Display` is absent. -/// - Not clonable, not copyable. -/// - One unavoidable stack copy in `new`: the `[u8; N]` argument lives on the -/// caller's stack until it is zeroized immediately after being transferred -/// into secure storage. -/// - `mlock` / `VirtualLock` prevent the secret from being paged to disk but -/// do **not** prevent transient exposure in CPU registers or on the call stack -/// while `expose_secret` is in use. +/// - `Debug` emits `[REDACTED]`; `Display` is absent; not `Clone` or `Copy`. +/// - One unavoidable stack copy in `new`: the `[u8; N]` argument is zeroized +/// immediately after being transferred into secure storage. +/// - `mlock` / `VirtualLock` prevent paging to disk but do not prevent transient +/// exposure in registers or on the call stack during `expose_secret`. pub struct ProtectedBytes { inner: platform::SecureAlloc, status: ProtectionStatus, diff --git a/crates/secure-memory/src/unix.rs b/crates/secure-memory/src/unix.rs index 049832511..eebd0b2e1 100644 --- a/crates/secure-memory/src/unix.rs +++ b/crates/secure-memory/src/unix.rs @@ -5,22 +5,24 @@ //! ```text //! ┌──────────────┬──────────────┬──────────────┐ //! │ guard page │ data page │ guard page │ -//! │ PROT_NONE │ PROT_R|W │ PROT_NONE │ +//! │ PROT_NONE │ PROT_READ │ PROT_NONE │ //! └──────────────┴──────────────┴──────────────┘ //! ^base ^data ^data + page_size //! ``` //! -//! The secret occupies the first `N` bytes of the data page. -//! The rest of the page is unused. -//! Guard pages are set to `PROT_NONE` so any out-of-bounds access faults immediately at the OS level. +//! The secret occupies the first `N` bytes of the data page; the rest is unused. +//! Guard pages are `PROT_NONE` so any out-of-bounds access faults immediately. +//! The data page is `PROT_READ|WRITE` only during construction (while the secret +//! is being written). It is demoted to `PROT_READ` before `new` returns. //! //! ## Hardening steps (best-effort) //! //! 1. Guard pages via `mprotect(PROT_NONE)`. //! 2. RAM lock via `mlock` — may fail under tight `ulimit -l` limits. //! 3. Core-dump exclusion via `madvise(MADV_DONTDUMP)` — Linux only. +//! 4. Data page demoted to `PROT_READ` after the secret is written. //! -//! All three are best-effort: failure is logged and reflected in [`ProtectionStatus`] but does **not** abort the process. +//! All four are best-effort: failure is logged and reflected in [`ProtectionStatus`] but does **not** abort the process. use std::ptr; use std::sync::OnceLock; @@ -93,10 +95,15 @@ impl SecureAlloc { let r_guard_after = unsafe { libc::mprotect(guard_after as *mut libc::c_void, ps, libc::PROT_NONE) }; let guard_pages = r_guard_before == 0 && r_guard_after == 0; - if !guard_pages { + if r_guard_before != 0 { tracing::debug!( - "secure-memory: mprotect for guard pages failed ({}); \ - guard pages are not active", + "secure-memory: mprotect for leading guard page failed ({})", + std::io::Error::last_os_error() + ); + } + if r_guard_after != 0 { + tracing::debug!( + "secure-memory: mprotect for trailing guard page failed ({})", std::io::Error::last_os_error() ); } @@ -136,6 +143,19 @@ impl SecureAlloc { // non-overlapping; both are valid for `N` bytes. unsafe { ptr::copy_nonoverlapping(src.as_ptr(), data, N) }; + // ── Demote data page to read-only ──────────────────────────────────── + // The secret is now in place and never needs to be modified in-place. + // Removing write access prevents accidental overwrites. + // SAFETY: `data` is page-aligned; `ps` bytes are within the allocation. + let r_readonly = unsafe { libc::mprotect(data as *mut libc::c_void, ps, libc::PROT_READ) }; + if r_readonly != 0 { + tracing::debug!( + "secure-memory: mprotect(PROT_READ) for data page failed ({}); \ + data page remains writable", + std::io::Error::last_os_error() + ); + } + let alloc = SecureAlloc { base, data, @@ -164,7 +184,7 @@ impl Drop for SecureAlloc { let ps = page_size(); // Restore read+write on the data page so we can zeroize it. - // (It should already be RW; this is defensive.) + // The page was demoted to PROT_READ in new(); this re-enables writes. // SAFETY: `self.data` is page-aligned; `ps` bytes are within our mapping. let _ = unsafe { libc::mprotect(self.data as *mut libc::c_void, ps, libc::PROT_READ | libc::PROT_WRITE) }; diff --git a/crates/secure-memory/src/windows.rs b/crates/secure-memory/src/windows.rs index da1d1e5c8..0ea95474b 100644 --- a/crates/secure-memory/src/windows.rs +++ b/crates/secure-memory/src/windows.rs @@ -5,9 +5,13 @@ //! ```text //! ┌──────────────┬──────────────┬──────────────┐ //! │ guard page │ data page │ guard page │ -//! │ PAGE_NOACCESS│PAGE_READWRITE│ PAGE_NOACCESS│ +//! │ PAGE_NOACCESS│ PAGE_READONLY│ PAGE_NOACCESS│ //! └──────────────┴──────────────┴──────────────┘ //! ^base ^data ^data + page_size +//! +//! The data page is `PAGE_READWRITE` only during construction (while the +//! secret is being written). It is immediately demoted to `PAGE_READONLY` +//! before `new` returns. //! ``` //! //! The three pages are a single `VirtualAlloc` region. @@ -18,28 +22,24 @@ //! 1. Guard pages via `VirtualProtect(PAGE_NOACCESS)`. //! 2. RAM lock via `VirtualLock`. //! 3. WER dump exclusion via `WerRegisterExcludedMemoryBlock`. +//! 4. Data page demoted to `PAGE_READONLY` after the secret is written. //! -//! ## Dump-exclusion on Windows -//! -//! Windows does not have a single universal per-region dump-exclusion API -//! equivalent to Linux's `madvise(MADV_DONTDUMP)`. The protections that exist -//! are scoped: +//! ## Dump exclusion on Windows //! -//! - **WER crash reports**: `WerRegisterExcludedMemoryBlock` tells the Windows -//! Error Reporting subsystem to omit the registered range from automatically- -//! generated crash dumps. This is the mechanism used here. -//! `ProtectionStatus::dump_excluded` reflects whether this registration -//! succeeded. +//! `WerRegisterExcludedMemoryBlock` registers the data page for exclusion from +//! the mini dump embedded in WER crash reports sent to Microsoft Watson. This +//! is the only public per-region dump-exclusion API on Windows; its scope is +//! narrower than Linux's `madvise(MADV_DONTDUMP)`. //! -//! - **Full-memory / forensic dumps** (`MiniDumpWithFullMemory`, kernel dumps, -//! ProcDump `-ma`, …): no public callback API reliably excludes a page from -//! these on current Windows versions. Applications that write their own -//! dumps using `MiniDumpNormal` (the typical crash-reporter default) will -//! not capture non-stack heap pages, but `MiniDumpWithFullMemory` captures -//! everything regardless of WER registration or `IncludeVmRegionCallback`. +//! Full-memory dumps (`MiniDumpWithFullMemory`, ProcDump `-ma`, LocalDumps with +//! `DumpType=2`, kernel live dumps) capture all committed read/write pages +//! unconditionally. `MiniDumpWriteDump` callbacks (`RemoveMemoryCallback` / +//! `IncludeVmRegionCallback`) can filter regions, but only for dump writers that +//! explicitly pass a callback; they do not provide a standing process-level +//! exclusion for externally triggered dumps. //! -//! `VirtualLock` prevents the secret from being written to the pagefile but -//! does **not** affect dump capture. +//! `VirtualLock` prevents the secret from being paged to disk but does not +//! affect dump capture. use std::ffi::c_void; use std::ptr; @@ -47,8 +47,8 @@ use std::sync::OnceLock; use windows::Win32::System::ErrorReporting::{WerRegisterExcludedMemoryBlock, WerUnregisterExcludedMemoryBlock}; use windows::Win32::System::Memory::{ - MEM_COMMIT, MEM_RELEASE, MEM_RESERVE, PAGE_NOACCESS, PAGE_PROTECTION_FLAGS, PAGE_READWRITE, VirtualAlloc, - VirtualFree, VirtualLock, VirtualProtect, VirtualUnlock, + VirtualAlloc, VirtualAlloc, VirtualAlloc, VirtualFree, VirtualFree, VirtualFree, VirtualLoc, VirtualLock, + VirtualLock, VirtualProtect, VirtualProtect, VirtualUnlock, VirtualUnlock, }; use windows::Win32::System::SystemInformation::{GetSystemInfo, SYSTEM_INFO}; @@ -117,10 +117,15 @@ impl SecureAlloc { let r_guard_after = unsafe { VirtualProtect(guard_after as *const c_void, ps, PAGE_NOACCESS, &mut old_prot) }; let guard_pages = r_guard_before.is_ok() && r_guard_after.is_ok(); - if !guard_pages { + if r_guard_before.is_err() { + tracing::debug!( + "secure-memory: VirtualProtect for leading guard page failed ({})", + std::io::Error::last_os_error() + ); + } + if r_guard_after.is_err() { tracing::debug!( - "secure-memory: VirtualProtect for guard pages failed ({}); \ - guard pages are not active", + "secure-memory: VirtualProtect for trailing guard page failed ({})", std::io::Error::last_os_error() ); } @@ -137,17 +142,12 @@ impl SecureAlloc { ); } - // ── Copy secret into the data page ────────────────────────────────── - // SAFETY: `src` (caller stack) and `data` (VirtualAlloc region) are - // non-overlapping; both are valid for `N` bytes. - unsafe { ptr::copy_nonoverlapping(src.as_ptr(), data, N) }; - // ── Register the data page for WER dump exclusion ──────────────────── - // `WerRegisterExcludedMemoryBlock` asks the Windows Error Reporting - // subsystem to omit this page from automatically-generated crash dumps. + // Done before writing the secret so the page is excluded from WER crash + // reports from the moment the secret lands on it. // Registration covers the full page, not just N bytes, because the // allocation model is page-based. - + // // SAFETY: `data` is a valid, committed, page-aligned pointer; `ps` is // exactly one page — the size passed to `VirtualAlloc`. let wer_hr = unsafe { WerRegisterExcludedMemoryBlock(data.cast::(), ps as u32) }; @@ -159,6 +159,24 @@ impl SecureAlloc { ); } + // ── Copy secret into the data page ────────────────────────────────── + // SAFETY: `src` (caller stack) and `data` (VirtualAlloc region) are + // non-overlapping; both are valid for `N` bytes. + unsafe { ptr::copy_nonoverlapping(src.as_ptr(), data, N) }; + + // ── Demote data page to read-only ──────────────────────────────────── + // The secret is now in place and never needs to be modified in-place. + // Removing write access prevents accidental overwrites. + // SAFETY: `data` is page-aligned; `ps` committed bytes in our region. + let r_readonly = unsafe { VirtualProtect(data as *const c_void, ps, PAGE_READONLY, &mut old_prot) }; + if r_readonly.is_err() { + tracing::debug!( + "secure-memory: VirtualProtect(PAGE_READONLY) for data page failed ({}); \ + data page remains writable", + std::io::Error::last_os_error() + ); + } + let alloc = SecureAlloc { base, data, @@ -190,6 +208,7 @@ impl Drop for SecureAlloc { let ps = page_size(); // Restore read+write on the data page so we can zeroize it. + // The page was demoted to PAGE_READONLY in new(); this re-enables writes. let mut old_prot = PAGE_PROTECTION_FLAGS::default(); // SAFETY: `self.data` is page-aligned; `ps` committed bytes in our region. let _ = unsafe { VirtualProtect(self.data as *const c_void, ps, PAGE_READWRITE, &mut old_prot) }; From 59f8fb5e45523aad9449641b1c336678ac8c8e3b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20CORTIER?= Date: Fri, 27 Mar 2026 11:06:05 +0900 Subject: [PATCH 13/17] . --- crates/secure-memory/src/windows.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/secure-memory/src/windows.rs b/crates/secure-memory/src/windows.rs index 0ea95474b..4466ccf92 100644 --- a/crates/secure-memory/src/windows.rs +++ b/crates/secure-memory/src/windows.rs @@ -47,8 +47,8 @@ use std::sync::OnceLock; use windows::Win32::System::ErrorReporting::{WerRegisterExcludedMemoryBlock, WerUnregisterExcludedMemoryBlock}; use windows::Win32::System::Memory::{ - VirtualAlloc, VirtualAlloc, VirtualAlloc, VirtualFree, VirtualFree, VirtualFree, VirtualLoc, VirtualLock, - VirtualLock, VirtualProtect, VirtualProtect, VirtualUnlock, VirtualUnlock, + VirtualAlloc, VirtualFree, VirtualFree, VirtualFree, VirtualLoc, VirtualLock, VirtualLock, VirtualProtect, + VirtualProtect, VirtualUnlock, VirtualUnlock, }; use windows::Win32::System::SystemInformation::{GetSystemInfo, SYSTEM_INFO}; From 94f715e8806ab90e16dfdebd234e705a9fdb989d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20CORTIER?= Date: Fri, 27 Mar 2026 11:10:24 +0900 Subject: [PATCH 14/17] . --- crates/secure-memory/src/windows.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/crates/secure-memory/src/windows.rs b/crates/secure-memory/src/windows.rs index 4466ccf92..5cc717d50 100644 --- a/crates/secure-memory/src/windows.rs +++ b/crates/secure-memory/src/windows.rs @@ -46,10 +46,7 @@ use std::ptr; use std::sync::OnceLock; use windows::Win32::System::ErrorReporting::{WerRegisterExcludedMemoryBlock, WerUnregisterExcludedMemoryBlock}; -use windows::Win32::System::Memory::{ - VirtualAlloc, VirtualFree, VirtualFree, VirtualFree, VirtualLoc, VirtualLock, VirtualLock, VirtualProtect, - VirtualProtect, VirtualUnlock, VirtualUnlock, -}; +use windows::Win32::System::Memory::{VirtualAlloc, VirtualFree, VirtualLock, VirtualProtect, VirtualUnlock}; use windows::Win32::System::SystemInformation::{GetSystemInfo, SYSTEM_INFO}; use crate::ProtectionStatus; From ef2ceb99ec803040a07ce6d46818d962292574ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20CORTIER?= Date: Fri, 27 Mar 2026 11:25:06 +0900 Subject: [PATCH 15/17] . --- Cargo.lock | 1 - crates/secure-memory-verifier/src/main.rs | 130 +++++++++---------- crates/secure-memory/src/windows.rs | 5 +- devolutions-gateway/Cargo.toml | 1 - devolutions-gateway/src/credential/crypto.rs | 2 +- 5 files changed, 69 insertions(+), 70 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 45ae4bbe8..e7166ca78 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1524,7 +1524,6 @@ dependencies = [ "pin-project-lite 0.2.17", "portpicker", "proptest", - "rand 0.8.5", "reqwest", "rstest", "rustls-cng", diff --git a/crates/secure-memory-verifier/src/main.rs b/crates/secure-memory-verifier/src/main.rs index 574d3f764..18c5616eb 100644 --- a/crates/secure-memory-verifier/src/main.rs +++ b/crates/secure-memory-verifier/src/main.rs @@ -21,34 +21,34 @@ mod check_guard; #[cfg(windows)] mod check_lock; -use std::process; - -// ── Output helpers ──────────────────────────────────────────────────────────── - -pub fn print_check(name: &str) { +#[cfg(windows)] +pub(crate) fn print_check(name: &str) { println!("[CHECK] {name}"); } -pub fn print_pass(msg: &str) { +#[cfg(windows)] +pub(crate) fn print_pass(msg: &str) { println!("[PASS] {msg}"); } -pub fn print_fail(msg: &str) { +#[cfg(windows)] +pub(crate) fn print_fail(msg: &str) { eprintln!("[FAIL] {msg}"); } -pub fn print_info(msg: &str) { +#[cfg(windows)] +pub(crate) fn print_info(msg: &str) { println!("[INFO] {msg}"); } -// ── Entry point ─────────────────────────────────────────────────────────────── - +#[cfg(windows)] fn main() { + use std::process; + let args: Vec = std::env::args().collect(); // Hidden child-process modes: `--child `. // These spawn a process that intentionally crashes or exercises a specific path. - #[cfg(windows)] if args.get(1).map(String::as_str) == Some("--child") { match args.get(2).map(String::as_str) { Some("guard-underflow") => check_guard::child(check_guard::Side::Under), @@ -62,66 +62,64 @@ fn main() { let cmd = args.get(1).map(String::as_str).unwrap_or("--help"); - #[cfg(windows)] - { - let ok = match cmd { - "lock" => check_lock::run(), - "guard-underflow" => check_guard::run(check_guard::Side::Under), - "guard-overflow" => check_guard::run(check_guard::Side::Over), - "all" => run_all(), - "--help" | "-h" => { - print_usage(); - return; - } - other => { - eprintln!("unknown subcommand: {other}"); - print_usage(); - process::exit(2); - } - }; - process::exit(if ok { 0 } else { 1 }); - } + let ok = match cmd { + "lock" => check_lock::run(), + "guard-underflow" => check_guard::run(check_guard::Side::Under), + "guard-overflow" => check_guard::run(check_guard::Side::Over), + "all" => run_all(), + "--help" | "-h" => { + print_usage(); + return; + } + other => { + eprintln!("unknown subcommand: {other}"); + print_usage(); + process::exit(2); + } + }; - #[cfg(not(windows))] - { - eprintln!("secure-memory-verifier only runs on Windows"); - process::exit(2); - } -} + let exit_code = if ok { 0 } else { 1 }; -#[cfg(windows)] -type Check = (&'static str, fn() -> bool); + process::exit(exit_code); -#[cfg(windows)] -fn run_all() -> bool { - let checks: &[Check] = &[ - ("lock", check_lock::run), - ("guard-underflow", || check_guard::run(check_guard::Side::Under)), - ("guard-overflow", || check_guard::run(check_guard::Side::Over)), - ]; - - let results: Vec<(&str, bool)> = checks.iter().map(|(name, f)| (*name, f())).collect(); - - println!(); - println!("=== Summary ==="); - let mut all_ok = true; - for (name, ok) in &results { - if *ok { - println!(" PASS {name}"); - } else { - println!(" FAIL {name}"); - all_ok = false; + type Check = (&'static str, fn() -> bool); + + fn run_all() -> bool { + let checks: &[Check] = &[ + ("lock", check_lock::run), + ("guard-underflow", || check_guard::run(check_guard::Side::Under)), + ("guard-overflow", || check_guard::run(check_guard::Side::Over)), + ]; + + let results: Vec<(&str, bool)> = checks.iter().map(|(name, f)| (*name, f())).collect(); + + println!(); + println!("=== Summary ==="); + let mut all_ok = true; + for (name, ok) in &results { + if *ok { + println!(" PASS {name}"); + } else { + println!(" FAIL {name}"); + all_ok = false; + } } + all_ok + } + + fn print_usage() { + println!("Usage: secure-memory-verifier "); + println!(); + println!("Subcommands:"); + println!(" lock Verify data page is locked in RAM (QueryWorkingSetEx)"); + println!(" guard-underflow Verify guard page fires on access before data (child crash)"); + println!(" guard-overflow Verify guard page fires on access after data (child crash)"); + println!(" all Run every check in sequence"); } - all_ok } -fn print_usage() { - println!("Usage: secure-memory-verifier "); - println!(); - println!("Subcommands:"); - println!(" lock Verify data page is locked in RAM (QueryWorkingSetEx)"); - println!(" guard-underflow Verify guard page fires on access before data (child crash)"); - println!(" guard-overflow Verify guard page fires on access after data (child crash)"); - println!(" all Run every check in sequence"); +#[cfg(not(windows))] +fn main() { + eprintln!("secure-memory-verifier is only supported on Windows"); + std::process::exit(2); } diff --git a/crates/secure-memory/src/windows.rs b/crates/secure-memory/src/windows.rs index 5cc717d50..597471006 100644 --- a/crates/secure-memory/src/windows.rs +++ b/crates/secure-memory/src/windows.rs @@ -46,7 +46,10 @@ use std::ptr; use std::sync::OnceLock; use windows::Win32::System::ErrorReporting::{WerRegisterExcludedMemoryBlock, WerUnregisterExcludedMemoryBlock}; -use windows::Win32::System::Memory::{VirtualAlloc, VirtualFree, VirtualLock, VirtualProtect, VirtualUnlock}; +use windows::Win32::System::Memory::{ + MEM_COMMIT, MEM_RELEASE, MEM_RESERVE, PAGE_NOACCESS, PAGE_PROTECTION_FLAGS, PAGE_READONLY, PAGE_READWRITE, + VirtualAlloc, VirtualFree, VirtualLock, VirtualProtect, VirtualUnlock, +}; use windows::Win32::System::SystemInformation::{GetSystemInfo, SYSTEM_INFO}; use crate::ProtectionStatus; diff --git a/devolutions-gateway/Cargo.toml b/devolutions-gateway/Cargo.toml index 6f5df3ce3..bf1a2077f 100644 --- a/devolutions-gateway/Cargo.toml +++ b/devolutions-gateway/Cargo.toml @@ -77,7 +77,6 @@ picky = { version = "7.0.0-rc.15", default-features = false, features = ["jose", zeroize = { version = "1.8", features = ["derive"] } chacha20poly1305 = "0.10" 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/credential/crypto.rs b/devolutions-gateway/src/credential/crypto.rs index c7124c0a4..82a258787 100644 --- a/devolutions-gateway/src/credential/crypto.rs +++ b/devolutions-gateway/src/credential/crypto.rs @@ -17,10 +17,10 @@ use core::fmt; use std::sync::LazyLock; use anyhow::Context as _; +use chacha20poly1305::aead::rand_core::RngCore as _; use chacha20poly1305::aead::{Aead, AeadCore, KeyInit, OsRng}; use chacha20poly1305::{ChaCha20Poly1305, Nonce}; use parking_lot::Mutex; -use rand::RngCore as _; use secrecy::SecretString; use secure_memory::ProtectedBytes; From 56b0e472e6318a989d58a66cdd24bbf6399b64f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20CORTIER?= Date: Fri, 27 Mar 2026 11:30:19 +0900 Subject: [PATCH 16/17] . --- crates/secure-memory/src/lib.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/crates/secure-memory/src/lib.rs b/crates/secure-memory/src/lib.rs index 8da5c6d14..0b7c6e400 100644 --- a/crates/secure-memory/src/lib.rs +++ b/crates/secure-memory/src/lib.rs @@ -117,7 +117,6 @@ impl fmt::Debug for ProtectedBytes { // Intentionally absent: Display, Clone, Copy, Serialize, Deserialize. #[cfg(test)] -#[expect(clippy::unwrap_used, reason = "test code")] mod tests { use super::*; From e4dcc632997f76b57a972a43dd055e9cc3c65ed4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20CORTIER?= Date: Fri, 27 Mar 2026 11:41:57 +0900 Subject: [PATCH 17/17] . --- crates/secure-memory/src/windows.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/crates/secure-memory/src/windows.rs b/crates/secure-memory/src/windows.rs index 597471006..436a44524 100644 --- a/crates/secure-memory/src/windows.rs +++ b/crates/secure-memory/src/windows.rs @@ -150,7 +150,9 @@ impl SecureAlloc { // // SAFETY: `data` is a valid, committed, page-aligned pointer; `ps` is // exactly one page — the size passed to `VirtualAlloc`. - let wer_hr = unsafe { WerRegisterExcludedMemoryBlock(data.cast::(), ps as u32) }; + let wer_hr = unsafe { + WerRegisterExcludedMemoryBlock(data.cast::(), u32::try_from(ps).expect("page size fits in u32")) + }; let wer_excluded = wer_hr.is_ok(); if !wer_excluded { tracing::debug!(