diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 93c7a54f1..c3dd8ca5e 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 }} + shell: pwsh - name: Install fpm if: ${{ matrix.os == 'Linux' }} @@ -871,17 +862,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' }} @@ -1180,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/Cargo.lock b/Cargo.lock index a6b2c1567..e7166ca78 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", @@ -1491,6 +1528,8 @@ dependencies = [ "rstest", "rustls-cng", "rustls-native-certs", + "secrecy 0.10.3", + "secure-memory", "serde", "serde-querystring", "serde_json", @@ -4495,6 +4534,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 +4796,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 +4855,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 +5082,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 +5101,7 @@ checksum = "1ffd40cc99d0fbb02b4b3771346b811df94194bc103983efa0203c8893755085" dependencies = [ "cfg-if", "cpufeatures", - "universal-hash", + "universal-hash 0.6.0-rc.2", ] [[package]] @@ -6044,6 +6100,34 @@ 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 = "secure-memory" +version = "0.0.0" +dependencies = [ + "libc", + "tracing", + "windows 0.61.3", + "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" @@ -7647,6 +7731,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..563a6b834 100644 --- a/README.md +++ b/README.md @@ -20,7 +20,7 @@ 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 diff --git a/ci/setup-linux-build-deps.ps1 b/ci/setup-linux-build-deps.ps1 new file mode 100644 index 000000000..49ce40211 --- /dev/null +++ b/ci/setup-linux-build-deps.ps1 @@ -0,0 +1,33 @@ +#!/usr/bin/env pwsh + +param( + [Parameter(Mandatory = $true)] + [ValidateSet('x86_64', 'arm64')] + [string] $Architecture +) + +$ErrorActionPreference = 'Stop' + +$packages = @( + 'python3-wget', + 'python3-setuptools', + 'libsystemd-dev', + 'dh-make' +) + +if ($Architecture -eq 'arm64') { + $packages += @( + 'binutils-aarch64-linux-gnu', + 'gcc-aarch64-linux-gnu', + 'g++-aarch64-linux-gnu', + 'qemu-user' + ) +} + +& sudo apt-get update +& sudo apt-get '-o' 'Acquire::Retries=3' 'install' '-qy' @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/crates/secure-memory-verifier/Cargo.toml b/crates/secure-memory-verifier/Cargo.toml new file mode 100644 index 000000000..f930168f4 --- /dev/null +++ b/crates/secure-memory-verifier/Cargo.toml @@ -0,0 +1,24 @@ +[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_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..37c2163aa --- /dev/null +++ b/crates/secure-memory-verifier/README.md @@ -0,0 +1,95 @@ +# secure-memory-verifier + +A Windows-only standalone binary that verifies the runtime behaviour of the +`secure-memory` crate's protection tracks. + +Run it manually on a Windows machine to confirm the OS hardening is active. + +## What it checks + +| 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 | + +> **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 + +- 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` + +## 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 + +# 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). + +## 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`). +- **Full-memory dumps:** `WerRegisterExcludedMemoryBlock` does not exclude the + page from `MiniDumpWithFullMemory` or any locally-captured full dump. + +## 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 | 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..b4431633e --- /dev/null +++ b/crates/secure-memory-verifier/src/check_lock.rs @@ -0,0 +1,98 @@ +//! 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/main.rs b/crates/secure-memory-verifier/src/main.rs new file mode 100644 index 000000000..18c5616eb --- /dev/null +++ b/crates/secure-memory-verifier/src/main.rs @@ -0,0 +1,125 @@ +//! 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 | +//! +//! 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)] +pub(crate) fn print_check(name: &str) { + println!("[CHECK] {name}"); +} + +#[cfg(windows)] +pub(crate) fn print_pass(msg: &str) { + println!("[PASS] {msg}"); +} + +#[cfg(windows)] +pub(crate) fn print_fail(msg: &str) { + eprintln!("[FAIL] {msg}"); +} + +#[cfg(windows)] +pub(crate) fn print_info(msg: &str) { + println!("[INFO] {msg}"); +} + +#[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. + 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), + other => { + eprintln!("unknown --child mode: {other:?}"); + process::exit(2); + } + } + } + + let cmd = args.get(1).map(String::as_str).unwrap_or("--help"); + + 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); + } + }; + + let exit_code = if ok { 0 } else { 1 }; + + process::exit(exit_code); + + 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"); + } +} + +#[cfg(not(windows))] +fn main() { + eprintln!("secure-memory-verifier is only supported on Windows"); + std::process::exit(2); +} diff --git a/crates/secure-memory/Cargo.toml b/crates/secure-memory/Cargo.toml new file mode 100644 index 000000000..149698828 --- /dev/null +++ b/crates/secure-memory/Cargo.toml @@ -0,0 +1,27 @@ +[package] +name = "secure-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_ErrorReporting", + "Win32_System_Memory", + "Win32_System_SystemInformation", +] diff --git a/crates/secure-memory/README.md b/crates/secure-memory/README.md new file mode 100644 index 000000000..a328f026c --- /dev/null +++ b/crates/secure-memory/README.md @@ -0,0 +1,75 @@ +# secure-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 (`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). + +**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` | ✗ | +| Write protect | `mprotect(PROT_READ)` | `VirtualProtect(PAGE_READONLY)` | ✗ | +| Dump exclusion | `MADV_DONTDUMP` | `WerRegisterExcludedMemoryBlock` (see note) | ✗ | +| Zeroize on drop | ✓ | ✓ | ✓ | + +**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. +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 debug message is +logged once at construction time. No feature flag is required; the crate always compiles +and runs. + +## Usage + +```rust +use secure_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/secure-memory/src/fallback.rs b/crates/secure-memory/src/fallback.rs new file mode 100644 index 000000000..bb2e83556 --- /dev/null +++ b/crates/secure-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 debug 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!( + "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/secure-memory/src/lib.rs b/crates/secure-memory/src/lib.rs new file mode 100644 index 000000000..0b7c6e400 --- /dev/null +++ b/crates/secure-memory/src/lib.rs @@ -0,0 +1,189 @@ +#![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; + +use core::fmt; + +#[cfg(not(any(unix, windows)))] +use fallback as platform; +#[cfg(unix)] +use unix as platform; +#[cfg(windows)] +use windows as platform; + +/// 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 page is registered for exclusion from crash dumps. + /// + /// - **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 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, 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. +/// +/// - `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, +} + +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)] +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/secure-memory/src/unix.rs b/crates/secure-memory/src/unix.rs new file mode 100644 index 000000000..eebd0b2e1 --- /dev/null +++ b/crates/secure-memory/src/unix.rs @@ -0,0 +1,215 @@ +//! Unix (Linux / macOS) secure-allocation backend. +//! +//! ## Memory layout +//! +//! ```text +//! ┌──────────────┬──────────────┬──────────────┐ +//! │ guard page │ data page │ guard page │ +//! │ PROT_NONE │ PROT_READ │ PROT_NONE │ +//! └──────────────┴──────────────┴──────────────┘ +//! ^base ^data ^data + page_size +//! ``` +//! +//! 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 four 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, + "secure-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!("secure-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 r_guard_before != 0 { + tracing::debug!( + "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() + ); + } + + // ── 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!( + "secure-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!( + "secure-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) }; + + // ── 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, + 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. + // 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) }; + + // 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) }; + usize::try_from(ps).unwrap_or(4096) + }) +} diff --git a/crates/secure-memory/src/windows.rs b/crates/secure-memory/src/windows.rs new file mode 100644 index 000000000..436a44524 --- /dev/null +++ b/crates/secure-memory/src/windows.rs @@ -0,0 +1,252 @@ +//! Windows secure-allocation backend. +//! +//! ## Memory layout +//! +//! ```text +//! ┌──────────────┬──────────────┬──────────────┐ +//! │ guard page │ data page │ guard page │ +//! │ 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. +//! 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`. +//! 3. WER dump exclusion via `WerRegisterExcludedMemoryBlock`. +//! 4. Data page demoted to `PAGE_READONLY` after the secret is written. +//! +//! ## Dump exclusion on Windows +//! +//! `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 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 paged to disk 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_READONLY, 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, + /// Whether `WerRegisterExcludedMemoryBlock` succeeded for the data page. + wer_excluded: 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, + "secure-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!( + "secure-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 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 trailing guard page failed ({})", + 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!( + "secure-memory: VirtualLock failed ({}); \ + secret may be paged to disk", + std::io::Error::last_os_error() + ); + } + + // ── Register the data page for WER dump exclusion ──────────────────── + // 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::(), u32::try_from(ps).expect("page size fits in 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" + ); + } + + // ── 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, + locked, + wer_excluded, + _marker: std::marker::PhantomData, + }; + let status = ProtectionStatus { + locked, + guard_pages, + // On Windows, dump_excluded reflects WER exclusion only. + // See module-level documentation for scope and limitations. + dump_excluded: wer_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. + // 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) }; + + // 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) }; + } + + // 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). + 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 b8bcf4cb4..bf1a2077f 100644 --- a/devolutions-gateway/Cargo.toml +++ b/devolutions-gateway/Cargo.toml @@ -19,6 +19,7 @@ openapi = ["dep:utoipa"] [dependencies] # In-house +secure-memory.path = "../crates/secure-memory" transport.path = "../crates/transport" jmux-proxy.path = "../crates/jmux-proxy" devolutions-agent-shared.path = "../crates/devolutions-agent-shared" @@ -74,6 +75,8 @@ 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" +secrecy = { version = "0.10", features = ["serde"] } 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..a144314bf 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,15 @@ 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, + "an internal error occurred".to_owned(), + ), + })?; 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..82a258787 --- /dev/null +++ b/devolutions-gateway/src/credential/crypto.rs @@ -0,0 +1,219 @@ +//! 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 `secure-memory`, which applies +//! the best available OS hardening (mlock, guard pages, core-dump exclusion) and always zeroizes on drop. +//! +//! ## 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 regardless of platform. +//! - Master key held in mlock'd / guard-paged memory where the OS permits it. + +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 secrecy::SecretString; +use secure_memory::ProtectedBytes; + +/// Global master key for credential encryption. +/// +/// 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 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())); + +/// Manages the master encryption key. +/// +/// 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 { + key_material: ProtectedBytes<32>, +} + +impl MasterKeyManager { + /// Generate a new random 256-bit master key and place it in protected memory. + /// + /// 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::warn!( + "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 { + let cipher = + ChaCha20Poly1305::new_from_slice(self.key_material.expose_secret()).expect("key is exactly 32 bytes"); + + // Generate a random 96-bit nonce (12 bytes for ChaCha20-Poly1305). + let nonce = ChaCha20Poly1305::generate_nonce(OsRng); + + // Encrypt; ciphertext includes 16-byte Poly1305 authentication 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 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 { + 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")?; + + let plaintext = String::from_utf8(plaintext_bytes).context("decrypted password is not valid UTF-8")?; + + Ok(SecretString::from(plaintext)) + } +} + +/// Encrypted password stored in heap memory. +/// +/// 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 authentication 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(); + 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(); + let plaintext = "password"; + + let encrypted1 = key_manager.encrypt(plaintext).unwrap(); + let encrypted2 = key_manager.encrypt(plaintext).unwrap(); + + // 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(); + let key_manager2 = MasterKeyManager::new(); + + let encrypted = key_manager1.encrypt("secret").unwrap(); + + // 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(); + let mut encrypted = key_manager.encrypt("secret").unwrap(); + + // Corrupt the ciphertext. + encrypted.ciphertext[0] ^= 0xFF; + + // Authentication must fail. + assert!(key_manager.decrypt(&encrypted).is_err()); + } + + #[test] + fn test_empty_password() { + let key_manager = MasterKeyManager::new(); + 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(); + 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() { + 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,