diff --git a/Cargo.lock b/Cargo.lock index e0164af..3b3b861 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5062,9 +5062,9 @@ checksum = "3d595e54a326bc53c1c197b32d295e14b169e3cfeaa8dc82b529f947fba6bcf5" [[package]] name = "proton-drive-rs" -version = "0.7.2" +version = "0.7.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d6ce113d234474821d34787436ee80e1458e3d8a8f97212219615ed315eb70be" +checksum = "1868294071187f6198aad10e01bd1ccaa02e70cd6b92f729f9cf7215671aa20f" dependencies = [ "async-trait", "base64 0.23.0", @@ -5089,9 +5089,9 @@ dependencies = [ [[package]] name = "proton-sdk" -version = "0.7.2" +version = "0.7.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2a33c67df9694f90ce60e5b34984275956244138511b4640ca0d100e1dac8b3d" +checksum = "118e33440fcf26a05bd6cc19d544d1270ab183ece0fe535904b4767b16d5b3e0" dependencies = [ "aes-gcm 0.11.0", "async-trait", diff --git a/Cargo.toml b/Cargo.toml index 698b21b..591c805 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -28,8 +28,8 @@ authors = ["Nils Pukropp "] opt-level = 2 [workspace.dependencies] -proton-sdk = { version = "0.7.2" } -proton-drive-rs = { version = "0.7.2" } +proton-sdk = { version = "0.7.3" } +proton-drive-rs = { version = "0.7.3" } # Internal crates. pstr-core = { path = "crates/pstr-core" } diff --git a/android/app/src/main/java/io/narl/protonstream/ui/AccountUi.kt b/android/app/src/main/java/io/narl/protonstream/ui/AccountUi.kt index 94b1e5c..1eef576 100644 --- a/android/app/src/main/java/io/narl/protonstream/ui/AccountUi.kt +++ b/android/app/src/main/java/io/narl/protonstream/ui/AccountUi.kt @@ -332,10 +332,12 @@ private fun VerificationDialog(url: String, onToken: (String) -> Unit, onDismiss } } -/** Security keys need WebAuthn, which the sign-in here does not speak. */ +/** + * Security keys need WebAuthn, which the sign-in here does not speak. An account + * with nothing else is refused before this form (`AccountStore::sign_in`). + */ private const val SECURITY_KEY_NOTE = - "Security keys are not supported. If your account has only a security key, add an " + - "authenticator app in your Proton account settings to sign in here." + "Security keys are not supported here — use the code from your authenticator app." /** Installed once per page, at start and again at finish in case the first ran too early. */ private const val VERIFICATION_BRIDGE = """ diff --git a/crates/pstr-app/src/ui/account.rs b/crates/pstr-app/src/ui/account.rs index 839ba64..f45da17 100644 --- a/crates/pstr-app/src/ui/account.rs +++ b/crates/pstr-app/src/ui/account.rs @@ -208,10 +208,10 @@ fn code_form(ui: &mut egui::Ui, panel: &mut AccountPanel, actions: &mut Vec, /// `1` single password, `2` separate mailbox password — Proton's values. password_mode: u8, - mailbox_password: String, - key_salts: Vec, + /// Key id → the passphrase that unlocks it. ASCII: they are bcrypt output. + key_passphrases: HashMap, } impl StoredSession { fn capture( session: &ProtonApiSession, tokens: proton_sdk::http::Tokens, - mailbox_password: &str, - key_salts: Vec, + key_passphrases: HashMap, ) -> Self { Self { session_id: session.session_id().as_str().to_owned(), @@ -80,11 +87,17 @@ impl StoredSession { PasswordMode::Single => 1, PasswordMode::Dual => 2, }, - mailbox_password: mailbox_password.to_owned(), - key_salts, + key_passphrases, } } + fn unlock(&self) -> HashMap> { + self.key_passphrases + .iter() + .map(|(id, passphrase)| (id.clone(), passphrase.as_bytes().to_vec())) + .collect() + } + fn resume_parameters(&self) -> ResumeParameters { ResumeParameters { session_id: self.session_id.clone().into(), @@ -354,6 +367,13 @@ impl AccountStore { password: password.to_owned(), }; if pending.session.is_waiting_for_second_factor() { + let methods = pending.session.second_factor_methods(); + if methods.fido2 && !methods.totp { + // A security key needs WebAuthn, which this sign-in does not + // speak; asking for a code would only fail as a wrong one. + pending.session.end().await.ok(); + return Err(Error::SignIn(SECURITY_KEY_ONLY.to_owned())); + } return Ok(SignIn::SecondFactor(pending)); } pending.after_second_factor().await @@ -368,17 +388,8 @@ impl AccountStore { return Ok(None); }; let session = ProtonApiSession::resume(client_configuration(), stored.resume_parameters())?; - persist_refreshed_tokens( - &self.secrets, - &session, - stored.mailbox_password.clone(), - stored.key_salts.clone(), - ); - let drive = ProtonDriveClient::with_key_salts( - &session, - stored.mailbox_password.into_bytes(), - stored.key_salts, - ); + persist_refreshed_tokens(&self.secrets, &session, stored.key_passphrases.clone()); + let drive = ProtonDriveClient::with_key_passphrases(&session, stored.unlock()); Ok(Some(account(session, drive))) } @@ -393,29 +404,35 @@ impl AccountStore { self.secrets.delete(SESSION_KEY) } + /// The stored session, or `None` for none — or for one this build cannot + /// read, such as the shape development builds before 1.2.0 stored, which + /// kept the mailbox password. Signing in again is the only fix for either, + /// and it replaces what is there. fn load(&self) -> Result> { let Some(encoded) = self.secrets.get(SESSION_KEY)? else { return Ok(None); }; - serde_json::from_str(&encoded) - .map(Some) - .map_err(|e| Error::Config(format!("the stored Proton session is unreadable: {e}"))) + Ok(serde_json::from_str(&encoded) + .inspect_err(|error| { + tracing::warn!(%error, "the stored Proton session is unreadable; sign in again"); + }) + .ok()) } } /// Unlock the key chain with `mailbox_password`, store the session, and hand /// back the account. /// -/// The key salts are read here, while the token still has the scope that -/// endpoint needs. Loading the addresses is what proves the mailbox password: -/// it unlocks the user keys, and a wrong password unlocks none of them. +/// The key passphrases are derived here, while the token still has the scope +/// the key salts need. Loading the addresses is what proves the mailbox +/// password: it unlocks the user keys, and a wrong password unlocks none of +/// them. Only the passphrases are stored; the password goes no further. async fn finish( secrets: Arc, session: ProtonApiSession, mailbox_password: &str, ) -> Result { let probe = ProtonDriveClient::new(&session, mailbox_password.as_bytes().to_vec()); - let key_salts = probe.account().key_salts().await?; probe .account() .addresses() @@ -426,28 +443,29 @@ async fn finish( } other => other.into(), })?; + let key_passphrases = probe + .account() + .key_passphrases() + .await? + .into_iter() + .map(|(id, passphrase)| { + String::from_utf8(passphrase) + .map(|passphrase| (id, passphrase)) + .map_err(|_| Error::Config("a derived key passphrase is not text".to_owned())) + }) + .collect::>>()?; let stored = StoredSession::capture( &session, session.current_tokens().await, - mailbox_password, - key_salts.clone(), + key_passphrases.clone(), ); let encoded = serde_json::to_string(&stored) .map_err(|e| Error::Config(format!("serialize the Proton session: {e}")))?; secrets.set(SESSION_KEY, &encoded)?; - persist_refreshed_tokens( - &secrets, - &session, - mailbox_password.to_owned(), - key_salts.clone(), - ); + persist_refreshed_tokens(&secrets, &session, key_passphrases); - let drive = ProtonDriveClient::with_key_salts( - &session, - mailbox_password.as_bytes().to_vec(), - key_salts, - ); + let drive = ProtonDriveClient::with_key_passphrases(&session, stored.unlock()); Ok(account(session, drive)) } @@ -467,14 +485,12 @@ fn account(session: ProtonApiSession, drive: ProtonDriveClient) -> Account { fn persist_refreshed_tokens( secrets: &Arc, session: &ProtonApiSession, - mailbox_password: String, - key_salts: Vec, + key_passphrases: HashMap, ) { let secrets = Arc::clone(secrets); let template = session.clone(); session.http().set_on_tokens_refreshed(move |tokens| { - let stored = - StoredSession::capture(&template, tokens, &mailbox_password, key_salts.clone()); + let stored = StoredSession::capture(&template, tokens, key_passphrases.clone()); let written = serde_json::to_string(&stored) .map_err(|e| Error::Config(e.to_string())) .and_then(|encoded| secrets.set(SESSION_KEY, &encoded)); @@ -566,8 +582,7 @@ mod tests { refresh_token: "refresh".to_owned(), scopes: vec!["drive".to_owned()], password_mode: 2, - mailbox_password: "mailbox".to_owned(), - key_salts: Vec::new(), + key_passphrases: HashMap::from([("key-1".to_owned(), "derived".to_owned())]), } } @@ -604,6 +619,29 @@ mod tests { assert_eq!(store.username().expect("read"), None); } + #[test] + fn the_stored_session_holds_no_password() { + let encoded = serde_json::to_string(&stored()).unwrap(); + assert!(!encoded.contains("password\""), "{encoded}"); + assert!(!encoded.contains("key_salts"), "{encoded}"); + } + + #[test] + fn a_session_stored_by_an_older_build_reads_as_signed_out() { + let secrets = Arc::new(MemorySecrets::default()); + secrets + .set( + SESSION_KEY, + r#"{"session_id":"s","username":"u","user_id":"i","access_token":"a", + "refresh_token":"r","scopes":[],"password_mode":1, + "mailbox_password":"hunter2","key_salts":[]}"#, + ) + .unwrap(); + let store = AccountStore::new(secrets); + assert_eq!(store.username().expect("read"), None); + assert!(store.resume().expect("resume").is_none()); + } + #[test] fn a_stored_session_keeps_its_password_mode() { let params = stored().resume_parameters(); diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index f59c1c9..8b12ce9 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -105,9 +105,11 @@ an edge case. - Nothing this app holds can write to the share: a viewer link grants `MemberRole::Viewer`, and the SDK's public-link write side is unported anyway. - Signing in to a Proton account is optional (`pstr-core::account`). The - session — tokens, the mailbox password that unlocks the key chain, and the - key salts a refreshed token can no longer fetch — is one blob in the same - credential store, under `proton-account`. Refresh tokens are single-use, so + session — tokens, and per account key the passphrase that unlocks it, + derived at sign-in — is one blob in the same credential store, under + `proton-account`. The mailbox password itself is never stored. Only TOTP + second factors are spoken; an account whose only one is a security key is + refused with a reason before the code prompt. Refresh tokens are single-use, so every refresh is written back. On the desktop the blob can outgrow one credential entry — Windows caps one at 1280 UTF-16 units — so `KeyringSecretStore` splits a long value across `key#0`, `key#1`, … under a