From fe42f6c9727b47010b0f3e60e66a6b8134c9f0f7 Mon Sep 17 00:00:00 2001 From: Nils Pukropp Date: Sat, 31 Oct 2026 13:07:09 +0100 Subject: [PATCH 1/2] fix(account): open address tokens with inactive account keys, skip a broken address An address key's token was decrypted with the active account keys only. A token encrypted to an account key that is now inactive never opened, although that key still opens with the password, so the address failed with "has no primary key". proton-drive-linux issue #40 shows this as a mount that retries "missing key" forever. Tokens are now tried with every account key that opens, active ones first; signing still uses active keys only. The key report walks the same chain. addresses() also failed as a whole when one address did not open. That address is now left out with a warning and the rest work; the call fails only when none opens. address_private_keys and the new address(id) name the reason for a left-out address, and default_address fails with it when the left-out address is the default rather than signing with another one. --- CHANGELOG.md | 25 +++ crates/proton-drive-rs/src/client.rs | 17 +- crates/proton-sdk/src/account.rs | 279 ++++++++++++++++++++++-- crates/proton-sdk/src/account/report.rs | 14 +- 4 files changed, 303 insertions(+), 32 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5c84d5c..d855874 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,31 @@ commits a change ports are recorded in [UPSTREAM_SYNC.md](UPSTREAM_SYNC.md). This file starts after 0.9.2; earlier changes are in the git history. +## [Unreleased] + +### Fixed + +- **Address keys whose token is for an inactive account key.** An address key's + token was decrypted with the active account keys only. A token encrypted to + an account key since marked inactive never opened, although that key still + opens with the password, so the address failed with "has no primary key". + Tokens are now tried with every account key that opens, active ones first. + Signing still uses active keys only. `AccountClient::key_report` follows the + same chain. +- **One broken address no longer fails the account.** `AccountClient::addresses` + failed as a whole when any one address's keys did not open. That address is + now left out with a warning, and the others work. It fails only when no + address opens. `address_private_keys` names the reason for a left-out + address, and `default_address` fails with it when the left-out address is the + default, instead of handing out the next address. + +### Added + +- `AccountClient::address(id)`: one address by id, failing with the reason when + its keys did not open. `proton-drive-rs` uses it to resolve membership + addresses, which before said "membership address not found" for such an + address. + ## [0.10.0] — 2026-10-26 ### Added diff --git a/crates/proton-drive-rs/src/client.rs b/crates/proton-drive-rs/src/client.rs index 0ccc62e..9804265 100644 --- a/crates/proton-drive-rs/src/client.rs +++ b/crates/proton-drive-rs/src/client.rs @@ -4737,13 +4737,7 @@ impl ProtonDriveClient { /// The `AddressKeyID` of an address's primary key (write requests that bind /// material to an address key need it alongside the `AddressID`). async fn address_primary_key_id(&self, address_id: &AddressId) -> Result { - self.account - .addresses() - .await? - .into_iter() - .find(|address| &address.id == address_id) - .map(|address| address.primary_key_id) - .ok_or_else(|| ProtonError::invalid_operation("membership address not found")) + Ok(self.account.address(address_id).await?.primary_key_id) } /// Create a fresh-file draft: generate a node key + content key, encrypt the @@ -5473,13 +5467,8 @@ impl ProtonDriveClient { &self, address_id: AddressId, ) -> Result<(AddressId, String, PrivateKey)> { - let address = self - .account - .addresses() - .await? - .into_iter() - .find(|a| a.id == address_id) - .ok_or_else(|| ProtonError::invalid_operation("membership address not found"))?; + // `address` names why an address whose keys did not open is missing. + let address = self.account.address(&address_id).await?; let keys = self.account.address_private_keys(&address_id).await?; let signing_key = keys diff --git a/crates/proton-sdk/src/account.rs b/crates/proton-sdk/src/account.rs index 5dca44a..2375577 100644 --- a/crates/proton-sdk/src/account.rs +++ b/crates/proton-sdk/src/account.rs @@ -97,8 +97,16 @@ struct Cache { /// key id → passphrase derived from the mailbox password and key salts. key_passphrases: Option, user_keys: Option>, + /// Every user key that opens, active ones first: what an address key's + /// token may be encrypted to. See [`AccountClient::token_keys`]. + token_keys: Option>, addresses: Option>, address_keys: HashMap>, + /// Addresses whose keys did not open, with why. They are left out of + /// `addresses` so the others still work. + failed_addresses: HashMap, + /// Why the lowest-ordered address did not open, when it did not. + default_failed: Option, /// email → active (non-compromised) public keys, for authorship verification. public_keys: HashMap>, } @@ -193,8 +201,22 @@ impl AccountClient { let response: AddressListResponse = self.inner.http.get("core/v4/addresses").await?; let mut addresses = Vec::with_capacity(response.addresses.len()); + let mut failed = HashMap::new(); + let mut first_error = None; for dto in &response.addresses { - let (address, keys) = self.decrypt_address(dto).await?; + // An address whose keys do not open is left out rather than + // failing the account: the shares on every other address still + // work, and one broken address must not stop them all. + let (address, keys) = match self.decrypt_address(dto).await { + Ok(decrypted) => decrypted, + Err(ProtonError::InvalidOperation(reason)) => { + tracing::warn!(address_id = %dto.id, %reason, "address keys do not open; skipping the address"); + failed.insert(dto.id.clone(), reason.clone()); + first_error.get_or_insert(reason); + continue; + } + Err(e) => return Err(e), + }; self.inner .cache .lock() @@ -203,16 +225,50 @@ impl AccountClient { .insert(address.id.clone(), keys); addresses.push(address); } + if addresses.is_empty() + && let Some(reason) = first_error + { + return Err(ProtonError::invalid_operation(reason)); + } addresses.sort_by_key(|a| a.order); + let default_failed = response + .addresses + .iter() + .min_by_key(|dto| dto.order) + .and_then(|dto| failed.get(&dto.id).cloned()); - self.inner.cache.lock().await.addresses = Some(addresses.clone()); + let mut cache = self.inner.cache.lock().await; + cache.failed_addresses = failed; + cache.default_failed = default_failed; + cache.addresses = Some(addresses.clone()); Ok(addresses) } + /// One address by id. An address whose keys did not open fails with the + /// reason, not as unknown. + pub async fn address(&self, address_id: &AddressId) -> Result
{ + let addresses = self.addresses().await?; + if let Some(address) = addresses.into_iter().find(|a| &a.id == address_id) { + return Ok(address); + } + let cache = self.inner.cache.lock().await; + Err(match cache.failed_addresses.get(address_id) { + Some(reason) => ProtonError::invalid_operation(reason.clone()), + None => ProtonError::invalid_operation(format!("no address {address_id}")), + }) + } + /// The default (lowest-ordered) address. + /// + /// When that address's keys did not open, this fails with the reason + /// rather than handing out the next address: what it signs would then be + /// signed by an address the user did not choose. pub async fn default_address(&self) -> Result
{ - self.addresses() - .await? + let addresses = self.addresses().await?; + if let Some(reason) = self.inner.cache.lock().await.default_failed.clone() { + return Err(ProtonError::invalid_operation(reason)); + } + addresses .into_iter() .next() .ok_or_else(|| ProtonError::invalid_operation("user has no address")) @@ -231,8 +287,12 @@ impl AccountClient { self.addresses().await?; let cache = self.inner.cache.lock().await; - cache.address_keys.get(address_id).cloned().ok_or_else(|| { - ProtonError::invalid_operation(format!("no keys for address {address_id}")) + if let Some(keys) = cache.address_keys.get(address_id) { + return Ok(keys.clone()); + } + Err(match cache.failed_addresses.get(address_id) { + Some(reason) => ProtonError::invalid_operation(reason.clone()), + None => ProtonError::invalid_operation(format!("no keys for address {address_id}")), }) } @@ -316,7 +376,7 @@ impl AccountClient { self.user_keys().await.map(drop) } - /// Decrypted user (account) keys. + /// Decrypted active user (account) keys. async fn user_keys(&self) -> Result> { { let cache = self.inner.cache.lock().await; @@ -329,16 +389,22 @@ impl AccountClient { let response: UserResponse = self.inner.http.get("core/v4/users").await?; let mut keys = Vec::new(); + let mut inactive = Vec::new(); for key_dto in &response.user.keys { - if !key_dto.is_active() { - continue; - } let Some(passphrase) = passphrases.get(key_dto.id.as_str()) else { - tracing::warn!(key_id = %key_dto.id, "no passphrase for user key"); + if key_dto.is_active() { + tracing::warn!(key_id = %key_dto.id, "no passphrase for user key"); + } continue; }; match PrivateKey::from_armored(&key_dto.private_key, passphrase) { - Ok(key) => keys.push(key), + Ok(key) if key_dto.is_active() => keys.push(key), + Ok(key) => inactive.push(key), + // An inactive key is usually one whose passphrase went with an + // old password, so not opening is what it is expected to do. + Err(e) if !key_dto.is_active() => { + tracing::debug!(key_id = %key_dto.id, error = %e, "inactive user key does not open") + } Err(e) => { tracing::warn!(key_id = %key_dto.id, error = %e, "failed to unlock user key") } @@ -356,10 +422,32 @@ impl AccountClient { }); } - self.inner.cache.lock().await.user_keys = Some(keys.clone()); + let mut cache = self.inner.cache.lock().await; + cache.token_keys = Some(keys.iter().cloned().chain(inactive).collect()); + cache.user_keys = Some(keys.clone()); Ok(keys) } + /// Every user key that opens, active ones first, for decrypting address + /// key tokens. + /// + /// A token can be encrypted to an account key that has since been marked + /// inactive, for example after a key was replaced. That key still opens + /// with the password, and the address key behind the token is still the + /// one the address's shares are encrypted to, so the token must be tried + /// with it too. Only active keys are used for anything else. + async fn token_keys(&self) -> Result> { + self.user_keys().await?; + Ok(self + .inner + .cache + .lock() + .await + .token_keys + .clone() + .unwrap_or_default()) + } + /// The per-key passphrases, key id → passphrase, derived (and cached) from /// the mailbox password and the account's key salts. /// @@ -398,7 +486,7 @@ impl AccountClient { /// Build the public [`Address`] and unlock all of its active private keys. async fn decrypt_address(&self, dto: &AddressDto) -> Result<(Address, Vec)> { - let user_keys = self.user_keys().await?; + let user_keys = self.token_keys().await?; let passphrases = self.key_passphrases().await?; let mut keys = Vec::new(); @@ -672,8 +760,9 @@ mod tests { assert!(report.user_keys.iter().all(|key| key.unlocks)); let keys = &report.addresses[0].keys; assert!(keys[0].unlocks && keys[0].error.is_none()); - assert!(!keys[1].unlocks); - assert!(keys[1].error.as_deref().unwrap().contains("missing key")); + // The report tries the token with every key that opens, as the unlock + // path does. + assert!(keys[1].unlocks && keys[1].error.is_none()); let recipients = keys[1].token_recipients.as_ref().unwrap(); let meant_for = report .user_key_named(recipients[0].as_deref().unwrap()) @@ -682,6 +771,164 @@ mod tests { assert!(!meant_for.active); } + /// A body answering both `core/v4/users` and `core/v4/addresses`: one + /// active account key and one inactive one, and `addresses` as given. + fn account_with( + active: &crate::crypto::GeneratedNodeKey, + inactive: &crate::crypto::GeneratedNodeKey, + addresses: serde_json::Value, + ) -> String { + serde_json::json!({ + "Code": 1000, + "User": { + "ID": "user-id", "MaxSpace": 1, "UsedSpace": 0, + "Keys": [ + { "ID": "key-1", "PrivateKey": active.locked_armored, "Primary": 1, "Active": 1 }, + { "ID": "key-2", "PrivateKey": inactive.locked_armored, "Primary": 0, "Active": 0 } + ] + }, + "Addresses": addresses + }) + .to_string() + } + + /// An address key entry whose token, for `user_key`, carries the + /// passphrase of `address_key`. + fn address_key( + id: &str, + primary: bool, + user_key: &crate::crypto::GeneratedNodeKey, + address_key: &crate::crypto::GeneratedNodeKey, + ) -> serde_json::Value { + let token = PrivateKey::from_armored(&user_key.locked_armored, &user_key.passphrase) + .unwrap() + .encrypt(&address_key.passphrase) + .unwrap(); + serde_json::json!({ + "ID": id, "PrivateKey": address_key.locked_armored, "Token": token, + "Signature": "signature", "Primary": i32::from(primary), "Active": 1 + }) + } + + fn both_passphrases( + active: &crate::crypto::GeneratedNodeKey, + inactive: &crate::crypto::GeneratedNodeKey, + ) -> KeyPassphrases { + KeyPassphrases::from_iter([ + ("key-1", active.passphrase.clone()), + ("key-2", inactive.passphrase.clone()), + ]) + } + + #[tokio::test] + async fn an_address_key_whose_token_is_for_an_inactive_account_key_opens() { + use crate::crypto::generate_node_key; + + let (active, inactive) = (generate_node_key().unwrap(), generate_node_key().unwrap()); + let (first, second) = (generate_node_key().unwrap(), generate_node_key().unwrap()); + let body = account_with( + &active, + &inactive, + serde_json::json!([{ + "ID": "address-1", "Email": "user@proton.me", "Status": 1, "Order": 1, + "Keys": [ + address_key("address-key-1", true, &inactive, &first), + address_key("address-key-2", false, &active, &second) + ] + }]), + ); + let (session, _) = session_serving(body).await; + let client = + AccountClient::with_key_passphrases(&session, both_passphrases(&active, &inactive)); + + let address = client.default_address().await.expect("primary key opens"); + assert_eq!(address.primary_key_id.as_str(), "address-key-1"); + let keys = client.address_private_keys(&address.id).await.unwrap(); + assert_eq!(keys.len(), 2); + } + + #[tokio::test] + async fn an_address_whose_keys_do_not_open_leaves_the_others_working() { + use crate::crypto::generate_node_key; + + let (active, inactive) = (generate_node_key().unwrap(), generate_node_key().unwrap()); + let (good, broken, stranger) = ( + generate_node_key().unwrap(), + generate_node_key().unwrap(), + generate_node_key().unwrap(), + ); + // The broken address's only key has a token for a key not on the account. + let body = account_with( + &active, + &inactive, + serde_json::json!([ + { "ID": "address-broken", "Email": "old@proton.me", "Status": 1, "Order": 1, + "Keys": [address_key("address-key-b", true, &stranger, &broken)] }, + { "ID": "address-good", "Email": "user@proton.me", "Status": 1, "Order": 2, + "Keys": [address_key("address-key-g", true, &active, &good)] } + ]), + ); + let (session, _) = session_serving(body).await; + let client = + AccountClient::with_key_passphrases(&session, both_passphrases(&active, &inactive)); + + let addresses = client.addresses().await.expect("one address opens"); + assert_eq!(addresses.len(), 1); + assert_eq!(addresses[0].id.as_str(), "address-good"); + let good_id = addresses[0].id.clone(); + assert_eq!( + client.address_private_keys(&good_id).await.unwrap().len(), + 1 + ); + + let broken_id: AddressId = "address-broken".to_owned().into(); + for error in [ + client + .address(&broken_id) + .await + .expect_err("broken") + .to_string(), + match client.address_private_keys(&broken_id).await { + Ok(_) => panic!("broken address opened"), + Err(e) => e.to_string(), + }, + // The broken address is the default, so nothing is signed with + // another one in its place. + client + .default_address() + .await + .expect_err("default is broken") + .to_string(), + ] { + assert!(error.contains("address-broken"), "{error}"); + } + } + + #[tokio::test] + async fn an_account_whose_addresses_all_fail_to_open_is_an_error() { + use crate::crypto::generate_node_key; + + let (active, inactive) = (generate_node_key().unwrap(), generate_node_key().unwrap()); + let (broken, stranger) = (generate_node_key().unwrap(), generate_node_key().unwrap()); + let body = account_with( + &active, + &inactive, + serde_json::json!([{ + "ID": "address-broken", "Email": "old@proton.me", "Status": 1, "Order": 1, + "Keys": [address_key("address-key-b", true, &stranger, &broken)] + }]), + ); + let (session, _) = session_serving(body).await; + let client = + AccountClient::with_key_passphrases(&session, both_passphrases(&active, &inactive)); + + let error = client.addresses().await.expect_err("nothing opens"); + assert!( + matches!(error, ProtonError::InvalidOperation(_)), + "{error:?}" + ); + } + #[tokio::test] async fn an_account_without_an_active_key_is_not_a_password_problem() { let key = crate::crypto::generate_node_key().unwrap(); diff --git a/crates/proton-sdk/src/account/report.rs b/crates/proton-sdk/src/account/report.rs index 5fe52d2..e6c8bf5 100644 --- a/crates/proton-sdk/src/account/report.rs +++ b/crates/proton-sdk/src/account/report.rs @@ -26,7 +26,8 @@ pub struct KeyReport { pub struct UserKeyReport { pub id: UserKeyId, pub primary: bool, - /// `Active` from the server. The unlock path uses active keys only. + /// `Active` from the server. The unlock path signs with active keys only, + /// but tries an inactive key that opens for address key tokens too. pub active: bool, /// Whether the client's passphrase for this key opens it. Tried for /// inactive keys too, which the unlock path never does. @@ -98,7 +99,10 @@ impl AccountClient { let user: UserResponse = self.inner.http.get("core/v4/users").await?; let mut user_keys = Vec::with_capacity(user.user.keys.len()); + // Every key that opens, active first, as `AccountClient::token_keys` + // tries them for a token. let mut active_unlocked = Vec::new(); + let mut inactive_unlocked = Vec::new(); for dto in &user.user.keys { let unlocked = match passphrases.get(dto.id.as_str()) { Some(passphrase) => PrivateKey::from_armored(&dto.private_key, passphrase) @@ -109,6 +113,8 @@ impl AccountClient { Ok(key) => { if dto.is_active() { active_unlocked.push(key); + } else { + inactive_unlocked.push(key); } (true, None) } @@ -123,6 +129,10 @@ impl AccountClient { key_ids: crypto::key_ids(&dto.private_key).unwrap_or_default(), }); } + let token_keys: Vec = active_unlocked + .into_iter() + .chain(inactive_unlocked) + .collect(); let response: AddressListResponse = self.inner.http.get("core/v4/addresses").await?; let mut addresses = Vec::with_capacity(response.addresses.len()); @@ -131,7 +141,7 @@ impl AccountClient { for key_dto in &dto.keys { let passphrase = match (&key_dto.token, &key_dto.signature) { (Some(token), Some(_signature)) => { - crypto::decrypt_armored_with_keys(token, &active_unlocked) + crypto::decrypt_armored_with_keys(token, &token_keys) .map_err(|e| format!("token: {e}")) } _ => passphrases From 48dd410e416c6f8758c1fe534e6a265be5d15963 Mon Sep 17 00:00:00 2001 From: Nils Pukropp Date: Sat, 31 Oct 2026 13:27:43 +0100 Subject: [PATCH 2/2] chore(VER) bumped to 0.10.1 --- CHANGELOG.md | 2 +- Cargo.lock | 4 ++-- Cargo.toml | 2 +- crates/proton-drive-rs/Cargo.toml | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d855874..327fc81 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,7 +7,7 @@ commits a change ports are recorded in [UPSTREAM_SYNC.md](UPSTREAM_SYNC.md). This file starts after 0.9.2; earlier changes are in the git history. -## [Unreleased] +## [0.10.1] — 2026-10-31 ### Fixed diff --git a/Cargo.lock b/Cargo.lock index 2c42693..c60d262 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2060,7 +2060,7 @@ dependencies = [ [[package]] name = "proton-drive-rs" -version = "0.10.0" +version = "0.10.1" dependencies = [ "async-trait", "base64 0.23.1", @@ -2085,7 +2085,7 @@ dependencies = [ [[package]] name = "proton-sdk" -version = "0.10.0" +version = "0.10.1" dependencies = [ "aes-gcm 0.11.1", "async-trait", diff --git a/Cargo.toml b/Cargo.toml index c16277e..8da4bd2 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.10.0" +version = "0.10.1" edition = "2024" license = "MIT" repository = "https://github.com/narrrl/proton-sdk-rs" diff --git a/crates/proton-drive-rs/Cargo.toml b/crates/proton-drive-rs/Cargo.toml index 3e4f89c..056a86e 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.10.0" } +proton-sdk = { path = "../proton-sdk", version = "0.10.1" } tokio.workspace = true reqwest.workspace = true serde.workspace = true