Skip to content

Commit bada7f5

Browse files
authored
refactor(dgw): use secrecy::SecretString for decrypted passwords (#1692)
1 parent 8bd9835 commit bada7f5

2 files changed

Lines changed: 9 additions & 30 deletions

File tree

devolutions-gateway/src/credential/crypto.rs

Lines changed: 7 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -23,11 +23,11 @@ use chacha20poly1305::aead::{Aead, AeadCore, KeyInit, OsRng};
2323
use chacha20poly1305::{ChaCha20Poly1305, Nonce};
2424
use parking_lot::Mutex;
2525
use rand::RngCore as _;
26+
use secrecy::SecretString;
2627
#[cfg(feature = "mlock")]
2728
use secrets::SecretBox;
2829
#[cfg(not(feature = "mlock"))]
2930
use zeroize::Zeroizing;
30-
use zeroize::{Zeroize, ZeroizeOnDrop};
3131

3232
/// Global master key for credential encryption.
3333
///
@@ -101,11 +101,11 @@ impl MasterKeyManager {
101101
Ok(EncryptedPassword { nonce, ciphertext })
102102
}
103103

104-
/// Decrypt a password, returning a zeroize-on-drop wrapper.
104+
/// Decrypt a password, returning a `SecretString` that zeroizes on drop.
105105
///
106-
/// The returned `DecryptedPassword` should have a short lifetime.
106+
/// The returned `SecretString` should have a short lifetime.
107107
/// Use it immediately and let it drop to zeroize the plaintext.
108-
pub(super) fn decrypt(&self, encrypted: &EncryptedPassword) -> anyhow::Result<DecryptedPassword> {
108+
pub(super) fn decrypt(&self, encrypted: &EncryptedPassword) -> anyhow::Result<SecretString> {
109109
#[cfg(feature = "mlock")]
110110
let key_ref = self.key_material.borrow();
111111
#[cfg(feature = "mlock")]
@@ -123,7 +123,7 @@ impl MasterKeyManager {
123123
// Convert bytes to String.
124124
let plaintext = String::from_utf8(plaintext_bytes).context("decrypted password is not valid UTF-8")?;
125125

126-
Ok(DecryptedPassword(plaintext))
126+
Ok(SecretString::from(plaintext))
127127
}
128128
}
129129

@@ -151,32 +151,11 @@ impl fmt::Debug for EncryptedPassword {
151151
}
152152
}
153153

154-
/// Temporarily decrypted password, zeroized on drop.
155-
///
156-
/// IMPORTANT: This should have a SHORT lifetime. Decrypt immediately before use,
157-
/// and let it drop as soon as possible to zeroize the plaintext.
158-
#[derive(Zeroize, ZeroizeOnDrop)]
159-
pub struct DecryptedPassword(String);
160-
161-
impl DecryptedPassword {
162-
/// Expose the plaintext password.
163-
///
164-
/// IMPORTANT: Do not store the returned reference beyond the lifetime
165-
/// of this `DecryptedPassword`. It will be zeroized when dropped.
166-
pub fn expose_secret(&self) -> &str {
167-
&self.0
168-
}
169-
}
170-
171-
impl fmt::Debug for DecryptedPassword {
172-
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
173-
f.debug_struct("DecryptedPassword").finish_non_exhaustive()
174-
}
175-
}
176-
177154
#[cfg(test)]
178155
#[expect(clippy::unwrap_used, reason = "test code, panics are expected")]
179156
mod tests {
157+
use secrecy::ExposeSecret as _;
158+
180159
use super::*;
181160

182161
#[test]

devolutions-gateway/src/credential/mod.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
mod crypto;
22

33
#[rustfmt::skip]
4-
pub use crypto::{DecryptedPassword, EncryptedPassword};
4+
pub use crypto::EncryptedPassword;
55

66
use std::collections::HashMap;
77
use std::fmt;
@@ -51,7 +51,7 @@ impl AppCredential {
5151
/// Decrypt the password using the global master key.
5252
///
5353
/// Returns the username and a short-lived decrypted password that zeroizes on drop.
54-
pub fn decrypt_password(&self) -> anyhow::Result<(String, DecryptedPassword)> {
54+
pub fn decrypt_password(&self) -> anyhow::Result<(String, secrecy::SecretString)> {
5555
match self {
5656
AppCredential::UsernamePassword { username, password } => {
5757
let decrypted = MASTER_KEY.lock().decrypt(password)?;

0 commit comments

Comments
 (0)