From 325c326c499c3df82489ff5d7986349b66682669 Mon Sep 17 00:00:00 2001 From: Nils Pukropp Date: Wed, 7 Oct 2026 02:24:51 +0200 Subject: [PATCH] feat(account): hold key passphrases as wiped secrets and report a locked account as KeysLocked A wrong mailbox password surfaced as InvalidOperation("no active user key could be unlocked"), which a caller could only recognise by its text. It is now ProtonError::KeysLocked, and AccountClient::unlock() checks a secret with a single core/v4/users request instead of loading every address. Derived passphrases, the mailbox password and the cache encryption key were plain Vecs, cloned freely and never wiped. They are Zeroizing now, and the passphrase map is KeyPassphrases: wiped on drop, redacted in Debug, with derive_secret() for a client that wants a key only an unlocked account can reproduce. with_key_passphrases takes it. Breaking: ProtonError gains a variant, key_passphrases() and with_key_passphrases() change type, and derive_key_passphrase returns Zeroizing>. --- Cargo.lock | 5 +- Cargo.toml | 3 +- README.md | 6 + crates/proton-drive-rs/Cargo.toml | 2 +- crates/proton-drive-rs/src/client.rs | 4 +- crates/proton-sdk/Cargo.toml | 1 + crates/proton-sdk/src/account.rs | 171 +++++++++++++++++-- crates/proton-sdk/src/account/passphrases.rs | 154 +++++++++++++++++ crates/proton-sdk/src/cache.rs | 5 +- crates/proton-sdk/src/crypto/derive.rs | 10 +- crates/proton-sdk/src/error.rs | 18 +- 11 files changed, 351 insertions(+), 28 deletions(-) create mode 100644 crates/proton-sdk/src/account/passphrases.rs diff --git a/Cargo.lock b/Cargo.lock index b6be7bc..c18118c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2060,7 +2060,7 @@ dependencies = [ [[package]] name = "proton-drive-rs" -version = "0.7.4" +version = "0.8.0" dependencies = [ "async-trait", "base64 0.23.1", @@ -2085,7 +2085,7 @@ dependencies = [ [[package]] name = "proton-sdk" -version = "0.7.4" +version = "0.8.0" dependencies = [ "aes-gcm 0.11.1", "async-trait", @@ -2112,6 +2112,7 @@ dependencies = [ "thiserror 2.0.21", "tokio", "tracing", + "zeroize", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index e226a2a..ddfd5e7 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -3,7 +3,7 @@ resolver = "2" members = ["crates/proton-sdk", "crates/proton-drive-rs"] [workspace.package] -version = "0.7.4" +version = "0.8.0" edition = "2024" license = "MIT" repository = "https://github.com/narrrl/proton-sdk-rs" @@ -43,6 +43,7 @@ sha1 = "0.11" sha2 = "0.11" aes-gcm = "0.11" hkdf = "0.13" +zeroize = "1" bcrypt = "0.19" pgp = "0.21" # Match the chrono line rPGP pulls in; used to stamp detached-signature subpackets. diff --git a/README.md b/README.md index 6b204c7..f650ef3 100644 --- a/README.md +++ b/README.md @@ -137,6 +137,12 @@ graph TD 3. **Share & Node Keys**: Nodes (files and folders) belong to shares. A share's passphrase is decrypted by the address key. The decrypted share passphrase is used to unlock the node's private key (usually X25519/Ed25519 or legacy RSA). 4. **File Content Keys**: Files contain metadata and data blocks. The file's name and passphrase are encrypted. The symmetric **Content Key** (AES-256) is encapsulated in a PGP Public-Key Encrypted Session-Key (PKESK) packet addressed to the node key. See [content.rs](file:///home/narl/dev/private/proton-sdk-rs/crates/proton-sdk/src/crypto/content.rs). +#### Resuming without the password + +A client that persists a session does not have to keep the mailbox password (on a single-password account, the login password). Capture `AccountClient::key_passphrases()` right after login and resume with `ProtonDriveClient::with_key_passphrases()`. A `KeyPassphrases` is wiped when dropped and prints as `KeyPassphrases(n redacted)`, but each passphrase unlocks its key, so store it as carefully as the password. `KeyPassphrases::derive_secret(purpose)` gives a 32-byte secret only an unlocked account can reproduce, for encrypting local state. + +`AccountClient::unlock()` checks a password or a set of passphrases with one request. A secret that does not fit fails with `ProtonError::KeysLocked`, which a login prompt should answer by asking again, never by retrying. + ### 2. Upload Pipeline When uploading a file: diff --git a/crates/proton-drive-rs/Cargo.toml b/crates/proton-drive-rs/Cargo.toml index b92258e..f022a38 100644 --- a/crates/proton-drive-rs/Cargo.toml +++ b/crates/proton-drive-rs/Cargo.toml @@ -14,7 +14,7 @@ readme = "../../README.md" documentation = "https://docs.rs/proton-drive-rs" [dependencies] -proton-sdk = { path = "../proton-sdk", version = "0.7.4" } +proton-sdk = { path = "../proton-sdk", version = "0.8.0" } tokio.workspace = true reqwest.workspace = true serde.workspace = true diff --git a/crates/proton-drive-rs/src/client.rs b/crates/proton-drive-rs/src/client.rs index 2bf155d..1617cdb 100644 --- a/crates/proton-drive-rs/src/client.rs +++ b/crates/proton-drive-rs/src/client.rs @@ -15,7 +15,7 @@ use lru::LruCache; use sha2::{Digest, Sha256}; use tokio::sync::{Mutex, OwnedSemaphorePermit, RwLock, Semaphore, TryAcquireError, mpsc}; -use proton_sdk::account::{AccountClient, KeySalt}; +use proton_sdk::account::{AccountClient, KeyPassphrases, KeySalt}; use proton_sdk::api::ResponseCode; use proton_sdk::cache::{CacheRepository, InMemoryCacheRepository}; use proton_sdk::crypto::PrivateKey; @@ -525,7 +525,7 @@ impl ProtonDriveClient { /// password. They unlock the keys all the same, so keep them as secret. pub fn with_key_passphrases( session: &ProtonApiSession, - key_passphrases: HashMap>, + key_passphrases: KeyPassphrases, ) -> Self { Self::with_account( session, diff --git a/crates/proton-sdk/Cargo.toml b/crates/proton-sdk/Cargo.toml index c8f53f7..8338cd4 100644 --- a/crates/proton-sdk/Cargo.toml +++ b/crates/proton-sdk/Cargo.toml @@ -31,6 +31,7 @@ sha2.workspace = true aes-gcm.workspace = true hkdf.workspace = true bcrypt.workspace = true +zeroize.workspace = true pgp.workspace = true chrono.workspace = true tracing.workspace = true diff --git a/crates/proton-sdk/src/account.rs b/crates/proton-sdk/src/account.rs index 60d06ca..969809d 100644 --- a/crates/proton-sdk/src/account.rs +++ b/crates/proton-sdk/src/account.rs @@ -11,11 +11,15 @@ //! derived in step 1.) Decrypted keys are cached for the lifetime of the client. mod dtos; +mod passphrases; + +pub use passphrases::KeyPassphrases; use std::collections::HashMap; use std::sync::Arc; use tokio::sync::Mutex; +use zeroize::Zeroizing; use crate::crypto::{self, PrivateKey, PublicKey}; use crate::error::{ProtonError, Result}; @@ -79,7 +83,7 @@ pub struct AccountClient { struct Inner { http: ApiHttpClient, - mailbox_password: Vec, + mailbox_password: Zeroizing>, cache: Mutex, } @@ -89,7 +93,7 @@ struct Cache { /// caller (see [`AccountClient::with_key_salts`]). key_salts: Option>, /// key id → passphrase derived from the mailbox password and key salts. - key_passphrases: Option>>, + key_passphrases: Option, user_keys: Option>, addresses: Option>, address_keys: HashMap>, @@ -102,7 +106,7 @@ impl AccountClient { Self { inner: Arc::new(Inner { http: session.http().clone(), - mailbox_password: mailbox_password.into(), + mailbox_password: Zeroizing::new(mailbox_password.into()), cache: Mutex::new(Cache::default()), }), } @@ -125,7 +129,7 @@ impl AccountClient { Self { inner: Arc::new(Inner { http: session.http().clone(), - mailbox_password: mailbox_password.into(), + mailbox_password: Zeroizing::new(mailbox_password.into()), cache: Mutex::new(Cache { key_salts: Some(key_salts), ..Cache::default() @@ -142,12 +146,12 @@ impl AccountClient { /// missing from the map does not unlock, exactly as a key without a salt. pub fn with_key_passphrases( session: &ProtonApiSession, - key_passphrases: HashMap>, + key_passphrases: KeyPassphrases, ) -> Self { Self { inner: Arc::new(Inner { http: session.http().clone(), - mailbox_password: Vec::new(), + mailbox_password: Zeroizing::new(Vec::new()), cache: Mutex::new(Cache { key_passphrases: Some(key_passphrases), ..Cache::default() @@ -297,6 +301,19 @@ impl AccountClient { }) } + /// Check that this client's password (or passphrases) unlocks the account. + /// + /// Fetches the user's keys once (`core/v4/users`) and unlocks them, nothing + /// more: no address is loaded or decrypted. Fails with + /// [`ProtonError::KeysLocked`] when the secret is wrong, which is the + /// answer a login prompt wants for "try again", and with a transport error + /// when Proton could not be reached, which says nothing about the secret. + /// The unlocked keys are cached, so a client that passes this has done the + /// work its first real call would have. + pub async fn unlock(&self) -> Result<()> { + self.user_keys().await.map(drop) + } + /// Decrypted user (account) keys. async fn user_keys(&self) -> Result> { { @@ -327,9 +344,14 @@ impl AccountClient { } if keys.is_empty() { - return Err(ProtonError::invalid_operation( - "no active user key could be unlocked", - )); + // Keys the account has, none of which this client's secret opens — + // as opposed to an account with no active key at all, which no + // password fixes. + return Err(if response.user.keys.iter().any(|k| k.is_active()) { + ProtonError::KeysLocked + } else { + ProtonError::invalid_operation("the account has no active user key") + }); } self.inner.cache.lock().await.user_keys = Some(keys.clone()); @@ -345,7 +367,7 @@ impl AccountClient { /// itself out of storage. Deriving needs the salts, so on a session /// resumed without them call this right after login, as for /// [`key_salts`](Self::key_salts). - pub async fn key_passphrases(&self) -> Result>> { + pub async fn key_passphrases(&self) -> Result { { let cache = self.inner.cache.lock().await; if let Some(passphrases) = &cache.key_passphrases { @@ -354,7 +376,7 @@ impl AccountClient { } let salts = self.key_salts().await?; - let mut passphrases = HashMap::new(); + let mut passphrases = KeyPassphrases::new(); for salt in &salts { let Some(salt_b64) = &salt.value else { continue; @@ -365,7 +387,7 @@ impl AccountClient { let salt_bytes = decode_base64(salt_b64)?; let passphrase = crypto::derive_key_passphrase(&self.inner.mailbox_password, &salt_bytes)?; - passphrases.insert(salt.key_id.clone(), passphrase); + passphrases.insert(salt.key_id.clone(), passphrase.to_vec()); } self.inner.cache.lock().await.key_passphrases = Some(passphrases.clone()); @@ -392,7 +414,7 @@ impl AccountClient { let passphrase = match (&key_dto.token, &key_dto.signature) { (Some(token), Some(_signature)) => { match crypto::decrypt_armored_with_keys(token, &user_keys) { - Ok(passphrase) => passphrase, + Ok(passphrase) => Zeroizing::new(passphrase), Err(e) => { tracing::warn!(key_id = %key_dto.id, error = %e, "failed to decrypt address key token"); continue; @@ -400,7 +422,7 @@ impl AccountClient { } } _ => match passphrases.get(key_dto.id.as_str()) { - Some(passphrase) => passphrase.clone(), + Some(passphrase) => Zeroizing::new(passphrase.to_vec()), None => { tracing::warn!(key_id = %key_dto.id, "no passphrase for address key"); continue; @@ -489,10 +511,129 @@ mod tests { #[tokio::test] async fn seeded_key_passphrases_are_used_without_deriving_or_a_request() { - let passphrases = HashMap::from([("key-1".to_owned(), b"derived".to_vec())]); + let passphrases = KeyPassphrases::from_iter([("key-1", b"derived".to_vec())]); let client = AccountClient::with_key_passphrases(&session(), passphrases.clone()); // No salts were given and the base URL is unreachable from a test, so // anything but the seeded map would fail here. assert_eq!(client.key_passphrases().await.unwrap(), passphrases); } + + /// A session pointed at a loopback server that answers every request with + /// `body`, and the number of requests it has served. + async fn session_serving( + body: String, + ) -> ( + ProtonApiSession, + std::sync::Arc, + ) { + use std::sync::atomic::{AtomicUsize, Ordering}; + use tokio::io::{AsyncReadExt, AsyncWriteExt}; + + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let addr = listener.local_addr().unwrap(); + let served = std::sync::Arc::new(AtomicUsize::new(0)); + let counter = served.clone(); + tokio::spawn(async move { + loop { + let Ok((mut sock, _)) = listener.accept().await else { + return; + }; + let mut buf = [0u8; 4096]; + let _ = sock.read(&mut buf).await; + counter.fetch_add(1, Ordering::SeqCst); + let head = format!( + "HTTP/1.1 200 OK\r\nContent-Type: application/json\r\nContent-Length: {}\r\nConnection: close\r\n\r\n", + body.len() + ); + let _ = sock.write_all(head.as_bytes()).await; + let _ = sock.write_all(body.as_bytes()).await; + let _ = sock.flush().await; + } + }); + + let session = ProtonApiSession::resume( + ProtonClientConfiguration::new("test@1.0.0") + .with_base_url(format!("http://{addr}/")) + .with_retry_policy(crate::config::RetryPolicy::disabled()), + ResumeParameters { + session_id: "session".to_owned().into(), + username: "user".to_owned(), + user_id: "user-id".to_owned().into(), + access_token: "access".to_owned(), + refresh_token: "refresh".to_owned(), + scopes: Vec::new(), + is_waiting_for_second_factor_code: false, + password_mode: PasswordMode::Dual, + }, + ) + .expect("resume"); + (session, served) + } + + /// The `core/v4/users` answer for one account key locked with `passphrase`. + fn user_with_key(armored: &str, active: i32) -> String { + serde_json::json!({ + "Code": 1000, + "User": { + "ID": "user-id", "MaxSpace": 1, "UsedSpace": 0, + "Keys": [{ "ID": "key-1", "PrivateKey": armored, "Primary": 1, "Active": active }] + } + }) + .to_string() + } + + #[tokio::test] + async fn unlock_accepts_the_passphrase_that_locks_the_key() { + let key = crate::crypto::generate_node_key().unwrap(); + let (session, served) = session_serving(user_with_key(&key.locked_armored, 1)).await; + let passphrases = KeyPassphrases::from_iter([("key-1", key.passphrase.clone())]); + + AccountClient::with_key_passphrases(&session, passphrases) + .unlock() + .await + .expect("right passphrase"); + assert_eq!(served.load(std::sync::atomic::Ordering::SeqCst), 1); + } + + #[tokio::test] + async fn unlock_reports_a_wrong_passphrase_as_keys_locked() { + let key = crate::crypto::generate_node_key().unwrap(); + let (session, _) = session_serving(user_with_key(&key.locked_armored, 1)).await; + let passphrases = KeyPassphrases::from_iter([("key-1", b"not it".to_vec())]); + + let error = AccountClient::with_key_passphrases(&session, passphrases) + .unlock() + .await + .expect_err("wrong passphrase"); + assert!(matches!(error, ProtonError::KeysLocked), "{error:?}"); + assert!(!error.is_retriable()); + } + + #[tokio::test] + async fn unlock_treats_a_missing_passphrase_as_keys_locked() { + let key = crate::crypto::generate_node_key().unwrap(); + let (session, _) = session_serving(user_with_key(&key.locked_armored, 1)).await; + + let error = AccountClient::with_key_passphrases(&session, KeyPassphrases::new()) + .unlock() + .await + .expect_err("no passphrase"); + assert!(matches!(error, ProtonError::KeysLocked), "{error:?}"); + } + + #[tokio::test] + async fn an_account_without_an_active_key_is_not_a_password_problem() { + let key = crate::crypto::generate_node_key().unwrap(); + let (session, _) = session_serving(user_with_key(&key.locked_armored, 0)).await; + let passphrases = KeyPassphrases::from_iter([("key-1", key.passphrase.clone())]); + + let error = AccountClient::with_key_passphrases(&session, passphrases) + .unlock() + .await + .expect_err("no active key"); + assert!( + matches!(error, ProtonError::InvalidOperation(_)), + "{error:?}" + ); + } } diff --git a/crates/proton-sdk/src/account/passphrases.rs b/crates/proton-sdk/src/account/passphrases.rs new file mode 100644 index 0000000..c65d44b --- /dev/null +++ b/crates/proton-sdk/src/account/passphrases.rs @@ -0,0 +1,154 @@ +//! The derived key passphrases, held as the secrets they are. + +use std::collections::HashMap; +use std::fmt; + +use hkdf::Hkdf; +use sha2::Sha256; +use zeroize::Zeroizing; + +/// Key id → the passphrase that unlocks that key. +/// +/// Each passphrase is `bcrypt(mailbox password, key salt)`, so it unlocks its +/// key exactly as the password would, and is as secret. It is also all a client +/// needs to *resume*: store these instead of the mailbox password and the +/// password itself (which on a single-password account is the login password) +/// never has to be kept. +/// +/// The bytes are wiped when the value drops, and [`Debug`](fmt::Debug) prints +/// only how many there are, so a stray `{:?}` cannot put them in a log. Cloning +/// copies the secrets, and each copy is wiped on its own. +#[derive(Clone, Default)] +pub struct KeyPassphrases(HashMap>>); + +impl KeyPassphrases { + pub fn new() -> Self { + Self::default() + } + + /// Add (or replace) the passphrase for `key_id`. + pub fn insert(&mut self, key_id: impl Into, passphrase: impl Into>) { + self.0 + .insert(key_id.into(), Zeroizing::new(passphrase.into())); + } + + /// The passphrase for `key_id`, if there is one. + pub fn get(&self, key_id: &str) -> Option<&[u8]> { + self.0.get(key_id).map(|p| p.as_slice()) + } + + pub fn len(&self) -> usize { + self.0.len() + } + + pub fn is_empty(&self) -> bool { + self.0.is_empty() + } + + /// Every `(key id, passphrase)`, in no particular order. + pub fn iter(&self) -> impl Iterator { + self.0.iter().map(|(id, p)| (id.as_str(), p.as_slice())) + } + + /// A 32-byte secret for `purpose`, derived from these passphrases and nothing + /// else, for a client that needs a key of its own that only an unlocked + /// account can produce (for instance to encrypt a local cache). + /// + /// The same passphrases and `purpose` always give the same secret, and a + /// different `purpose` gives an unrelated one, so one secret never doubles as + /// another. The result does not depend on insertion order. + pub fn derive_secret(&self, purpose: &[u8]) -> Zeroizing<[u8; 32]> { + let mut entries: Vec<(&str, &[u8])> = self.iter().collect(); + entries.sort_unstable_by_key(|(id, _)| *id); + + // Length-prefixed, so ("ab", "c") and ("a", "bc") cannot collide. + let mut material = Zeroizing::new(Vec::new()); + for (id, passphrase) in entries { + material.extend_from_slice(&(id.len() as u64).to_be_bytes()); + material.extend_from_slice(id.as_bytes()); + material.extend_from_slice(&(passphrase.len() as u64).to_be_bytes()); + material.extend_from_slice(passphrase); + } + + let mut out = Zeroizing::new([0u8; 32]); + Hkdf::::new(Some(b"proton-sdk.KeyPassphrases.derive_secret"), &material) + .expand(purpose, out.as_mut()) + .expect("32 bytes is a valid HKDF-SHA256 output length"); + out + } +} + +impl, P: Into>> FromIterator<(K, P)> for KeyPassphrases { + fn from_iter>(iter: I) -> Self { + let mut out = Self::new(); + for (id, passphrase) in iter { + out.insert(id, passphrase); + } + out + } +} + +/// Equal when they hold the same passphrases. For tests and change detection, +/// not for authenticating anyone, so the comparison is not constant-time. +impl PartialEq for KeyPassphrases { + fn eq(&self, other: &Self) -> bool { + self.0.len() == other.0.len() + && self.0.iter().all(|(id, p)| { + other + .0 + .get(id) + .is_some_and(|q| p.as_slice() == q.as_slice()) + }) + } +} + +impl Eq for KeyPassphrases {} + +impl fmt::Debug for KeyPassphrases { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "KeyPassphrases({} redacted)", self.0.len()) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn debug_never_prints_a_passphrase() { + let passphrases = KeyPassphrases::from_iter([("key-1", b"hunter2".to_vec())]); + let shown = format!("{passphrases:?}"); + assert!(!shown.contains("hunter2")); + assert!(!shown.contains("key-1")); + assert_eq!(shown, "KeyPassphrases(1 redacted)"); + } + + #[test] + fn a_secret_ignores_insertion_order() { + let a = KeyPassphrases::from_iter([("k1", b"one".to_vec()), ("k2", b"two".to_vec())]); + let b = KeyPassphrases::from_iter([("k2", b"two".to_vec()), ("k1", b"one".to_vec())]); + assert_eq!(*a.derive_secret(b"cache"), *b.derive_secret(b"cache")); + } + + #[test] + fn a_secret_depends_on_purpose_and_passphrases() { + let a = KeyPassphrases::from_iter([("k1", b"one".to_vec())]); + let other = KeyPassphrases::from_iter([("k1", b"two".to_vec())]); + assert_ne!(*a.derive_secret(b"cache"), *a.derive_secret(b"other")); + assert_ne!(*a.derive_secret(b"cache"), *other.derive_secret(b"cache")); + } + + #[test] + fn a_secret_cannot_be_forged_by_moving_the_boundary() { + let a = KeyPassphrases::from_iter([("ab", b"c".to_vec())]); + let b = KeyPassphrases::from_iter([("a", b"bc".to_vec())]); + assert_ne!(*a.derive_secret(b"cache"), *b.derive_secret(b"cache")); + } + + #[test] + fn equal_when_they_hold_the_same_passphrases() { + let a = KeyPassphrases::from_iter([("k1", b"one".to_vec())]); + assert_eq!(a, a.clone()); + assert_ne!(a, KeyPassphrases::new()); + } +} diff --git a/crates/proton-sdk/src/cache.rs b/crates/proton-sdk/src/cache.rs index 58b7d16..053052f 100644 --- a/crates/proton-sdk/src/cache.rs +++ b/crates/proton-sdk/src/cache.rs @@ -23,6 +23,7 @@ use async_trait::async_trait; use base64::Engine; use hkdf::Hkdf; use sha2::Sha256; +use zeroize::Zeroizing; use crate::error::{ProtonError, Result}; @@ -289,7 +290,7 @@ impl CacheRepository for InMemoryCacheRepository { /// (matching the C# behavior). pub struct EncryptedCacheRepository { inner: Arc, - encryption_key: Vec, + encryption_key: Zeroizing>, } const SALT_LEN: usize = 16; @@ -303,7 +304,7 @@ impl EncryptedCacheRepository { pub fn new(inner: Arc, encryption_key: impl Into>) -> Self { Self { inner, - encryption_key: encryption_key.into(), + encryption_key: Zeroizing::new(encryption_key.into()), } } diff --git a/crates/proton-sdk/src/crypto/derive.rs b/crates/proton-sdk/src/crypto/derive.rs index 8da33f6..881eef0 100644 --- a/crates/proton-sdk/src/crypto/derive.rs +++ b/crates/proton-sdk/src/crypto/derive.rs @@ -7,6 +7,7 @@ //! in the C# SDK (which calls `SrpClient.HashPassword`). use bcrypt::HashParts; +use zeroize::Zeroizing; use super::errors::CryptoError; @@ -15,7 +16,10 @@ const BCRYPT_PREFIX_LEN: usize = 29; /// Derive the unlocking passphrase for a key from the mailbox password and the /// key's 16-byte salt. -pub fn derive_key_passphrase(password: &[u8], salt: &[u8]) -> Result, CryptoError> { +pub fn derive_key_passphrase( + password: &[u8], + salt: &[u8], +) -> Result>, CryptoError> { if salt.len() != 16 { return Err(CryptoError::Unlock(format!( "key salt must be 16 bytes, got {}", @@ -31,12 +35,12 @@ pub fn derive_key_passphrase(password: &[u8], salt: &[u8]) -> Result, Cr // `$2b$10$<22 char salt><31 char hash>` — drop everything up to and // including the encoded salt to obtain the passphrase bytes. - let hash = parts.to_string(); + let hash = Zeroizing::new(parts.to_string()); let passphrase = hash .get(BCRYPT_PREFIX_LEN..) .ok_or_else(|| CryptoError::Unlock("bcrypt hash shorter than expected".into()))?; - Ok(passphrase.as_bytes().to_vec()) + Ok(Zeroizing::new(passphrase.as_bytes().to_vec())) } #[cfg(test)] diff --git a/crates/proton-sdk/src/error.rs b/crates/proton-sdk/src/error.rs index 5d1fca5..494f7e7 100644 --- a/crates/proton-sdk/src/error.rs +++ b/crates/proton-sdk/src/error.rs @@ -23,6 +23,16 @@ pub enum ProtonError { #[error("cryptography error: {0}")] Crypto(#[from] crate::crypto::CryptoError), + /// The account's keys exist but none could be unlocked with the passphrases + /// this client holds: the mailbox password (or the stored passphrases) is + /// wrong or stale. + /// + /// Typed apart from [`InvalidOperation`](Self::InvalidOperation) because a + /// caller can act on it — ask for the password again — and must not retry + /// it: the same secret fails the same way every time. + #[error("none of the account's keys could be unlocked with this password")] + KeysLocked, + /// The SDK was used in a way that violates an invariant. #[error("invalid operation: {0}")] InvalidOperation(String), @@ -44,10 +54,14 @@ impl ProtonError { pub fn is_retriable(&self) -> bool { match self { Self::Api(error) => error.is_retriable(), - // A serialization or crypto failure is deterministic; an invariant + // A serialization or crypto failure is deterministic, a wrong + // passphrase fails the same way every time, and an invariant // violation is a bug. Only a transport failure is worth replaying. Self::Transport(_) => true, - Self::Serialization(_) | Self::Crypto(_) | Self::InvalidOperation(_) => false, + Self::Serialization(_) + | Self::Crypto(_) + | Self::KeysLocked + | Self::InvalidOperation(_) => false, } } }