mirror of
https://github.com/narrrl/proton-stream.git
synced 2026-11-03 16:41:26 +01:00
fix(core,app,android): keep the password out of storage and refuse key-only 2FA
Moves to proton-sdk / proton-drive-rs 0.7.3. The stored Proton session now holds, per account key, the passphrase derived from the mailbox password at sign-in, and resumes through `ProtonDriveClient::with_key_passphrases`. The mailbox password — on a single-password account, the login password — and the key salts are no longer stored. A session in the earlier shape, which only development builds of 1.2.0 wrote, reads as signed out: signing in again replaces it. An account whose only second factor is a security key is refused at sign-in with a reason, rather than asked for a code that can only fail as a wrong one. The code prompts now say plainly that security keys are not supported.
This commit is contained in:
1 parent
d9ca8f8d60
commit
da16ab0575
6 files changed
+108
-66
No files matched your search
Generated
+4
-4
@@ -5062,9 +5062,9 @@ checksum = "3d595e54a326bc53c1c197b32d295e14b169e3cfeaa8dc82b529f947fba6bcf5"
|
|||||||
|
|
||||||
[[package]]
|
[[package]]
|
||||||
name = "proton-drive-rs"
|
name = "proton-drive-rs"
|
||||||
version = "0.7.2"
|
version = "0.7.3"
|
||||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||||
checksum = "d6ce113d234474821d34787436ee80e1458e3d8a8f97212219615ed315eb70be"
|
checksum = "1868294071187f6198aad10e01bd1ccaa02e70cd6b92f729f9cf7215671aa20f"
|
||||||
dependencies = [
|
dependencies = [
|
||||||
"async-trait",
|
"async-trait",
|
||||||
"base64 0.23.0",
|
"base64 0.23.0",
|
||||||
@@ -5089,9 +5089,9 @@ dependencies = [
|
|||||||
|
|
||||||
[[package]]
|
[[package]]
|
||||||
name = "proton-sdk"
|
name = "proton-sdk"
|
||||||
version = "0.7.2"
|
version = "0.7.3"
|
||||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||||
checksum = "2a33c67df9694f90ce60e5b34984275956244138511b4640ca0d100e1dac8b3d"
|
checksum = "118e33440fcf26a05bd6cc19d544d1270ab183ece0fe535904b4767b16d5b3e0"
|
||||||
dependencies = [
|
dependencies = [
|
||||||
"aes-gcm 0.11.0",
|
"aes-gcm 0.11.0",
|
||||||
"async-trait",
|
"async-trait",
|
||||||
|
|||||||
+2
-2
@@ -28,8 +28,8 @@ authors = ["Nils Pukropp <contact@narl.io>"]
|
|||||||
opt-level = 2
|
opt-level = 2
|
||||||
|
|
||||||
[workspace.dependencies]
|
[workspace.dependencies]
|
||||||
proton-sdk = { version = "0.7.2" }
|
proton-sdk = { version = "0.7.3" }
|
||||||
proton-drive-rs = { version = "0.7.2" }
|
proton-drive-rs = { version = "0.7.3" }
|
||||||
|
|
||||||
# Internal crates.
|
# Internal crates.
|
||||||
pstr-core = { path = "crates/pstr-core" }
|
pstr-core = { path = "crates/pstr-core" }
|
||||||
|
|||||||
@@ -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 =
|
private const val SECURITY_KEY_NOTE =
|
||||||
"Security keys are not supported. If your account has only a security key, add an " +
|
"Security keys are not supported here — use the code from your authenticator app."
|
||||||
"authenticator app in your Proton account settings to sign in here."
|
|
||||||
|
|
||||||
/** Installed once per page, at start and again at finish in case the first ran too early. */
|
/** Installed once per page, at start and again at finish in case the first ran too early. */
|
||||||
private const val VERIFICATION_BRIDGE = """
|
private const val VERIFICATION_BRIDGE = """
|
||||||
|
|||||||
@@ -208,10 +208,10 @@ fn code_form(ui: &mut egui::Ui, panel: &mut AccountPanel, actions: &mut Vec<Acti
|
|||||||
);
|
);
|
||||||
field.request_focus();
|
field.request_focus();
|
||||||
ui.add_space(theme::space::S);
|
ui.add_space(theme::space::S);
|
||||||
// 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`).
|
||||||
ui.label(ui::muted(
|
ui.label(ui::muted(
|
||||||
"Security keys are not supported. If your account has only a security key, add an \
|
"Security keys are not supported here — use the code from your authenticator app.",
|
||||||
authenticator app in your Proton account settings to sign in here.",
|
|
||||||
));
|
));
|
||||||
ui.add_space(theme::space::L);
|
ui.add_space(theme::space::L);
|
||||||
error_line(ui, panel);
|
error_line(ui, panel);
|
||||||
|
|||||||
@@ -8,13 +8,15 @@
|
|||||||
//!
|
//!
|
||||||
//! ## What is kept, and where
|
//! ## What is kept, and where
|
||||||
//!
|
//!
|
||||||
//! Following `proton-drive-linux`: one JSON blob in the [`SecretStore`] holding
|
//! One JSON blob in the [`SecretStore`] holding the session tokens and, per
|
||||||
//! the session tokens, the mailbox password and the account's key salts. The
|
//! account key, the passphrase that unlocks it. Those passphrases are derived
|
||||||
//! mailbox password is what unlocks the key chain, so without it a resumed
|
//! at sign-in from the mailbox password and the key salts, and kept *instead*
|
||||||
//! session could list nothing. The key salts are kept because
|
//! of the password: they unlock the key chain just the same, but a store that
|
||||||
//! `core/v4/keys/salts` needs a scope only a password sign-in grants: after the
|
//! leaks gives away this app's access to the account rather than the password
|
||||||
//! first token refresh it answers 403, and a session resumed without them
|
//! — which, on a single-password account, is the login password itself.
|
||||||
//! could never unlock its keys again.
|
//! Deriving them later is not an option either way: `core/v4/keys/salts`
|
||||||
|
//! needs a scope only a password sign-in grants, and answers 403 after the
|
||||||
|
//! first token refresh.
|
||||||
//!
|
//!
|
||||||
//! Proton refresh tokens are single-use, so every refresh is written back
|
//! Proton refresh tokens are single-use, so every refresh is written back
|
||||||
//! (see [`persist_refreshed_tokens`]). A refresh that is not leaves the store
|
//! (see [`persist_refreshed_tokens`]). A refresh that is not leaves the store
|
||||||
@@ -27,9 +29,10 @@
|
|||||||
//! way, so [`AccountStore::sign_in`] returns a [`SignIn`] saying what it needs
|
//! way, so [`AccountStore::sign_in`] returns a [`SignIn`] saying what it needs
|
||||||
//! next and holding the half-made session until it is given.
|
//! next and holding the half-made session until it is given.
|
||||||
|
|
||||||
|
use std::collections::HashMap;
|
||||||
use std::sync::Arc;
|
use std::sync::Arc;
|
||||||
|
|
||||||
use proton_drive_rs::{KeySalt, NodeKind, ProtonDriveClient};
|
use proton_drive_rs::{NodeKind, ProtonDriveClient};
|
||||||
use proton_sdk::api::HumanVerificationCredential;
|
use proton_sdk::api::HumanVerificationCredential;
|
||||||
use proton_sdk::error::ProtonError;
|
use proton_sdk::error::ProtonError;
|
||||||
use proton_sdk::ids::NodeUid;
|
use proton_sdk::ids::NodeUid;
|
||||||
@@ -43,6 +46,11 @@ use crate::shares::{SecretStore, client_configuration};
|
|||||||
/// `share-…`, so the two cannot collide.
|
/// `share-…`, so the two cannot collide.
|
||||||
const SESSION_KEY: &str = "proton-account";
|
const SESSION_KEY: &str = "proton-account";
|
||||||
|
|
||||||
|
/// Why an account whose only second factor is a security key cannot sign in.
|
||||||
|
const SECURITY_KEY_ONLY: &str = "This account's only second factor is a security key, which this \
|
||||||
|
app cannot use yet. Add an authenticator app in your Proton account settings, then sign in \
|
||||||
|
with its code.";
|
||||||
|
|
||||||
/// Blocks in flight for streaming from the account, as for a public link —
|
/// Blocks in flight for streaming from the account, as for a public link —
|
||||||
/// see `ShareStore::open`.
|
/// see `ShareStore::open`.
|
||||||
const MAX_INFLIGHT_BLOCKS: usize = 48;
|
const MAX_INFLIGHT_BLOCKS: usize = 48;
|
||||||
@@ -58,16 +66,15 @@ struct StoredSession {
|
|||||||
scopes: Vec<String>,
|
scopes: Vec<String>,
|
||||||
/// `1` single password, `2` separate mailbox password — Proton's values.
|
/// `1` single password, `2` separate mailbox password — Proton's values.
|
||||||
password_mode: u8,
|
password_mode: u8,
|
||||||
mailbox_password: String,
|
/// Key id → the passphrase that unlocks it. ASCII: they are bcrypt output.
|
||||||
key_salts: Vec<KeySalt>,
|
key_passphrases: HashMap<String, String>,
|
||||||
}
|
}
|
||||||
|
|
||||||
impl StoredSession {
|
impl StoredSession {
|
||||||
fn capture(
|
fn capture(
|
||||||
session: &ProtonApiSession,
|
session: &ProtonApiSession,
|
||||||
tokens: proton_sdk::http::Tokens,
|
tokens: proton_sdk::http::Tokens,
|
||||||
mailbox_password: &str,
|
key_passphrases: HashMap<String, String>,
|
||||||
key_salts: Vec<KeySalt>,
|
|
||||||
) -> Self {
|
) -> Self {
|
||||||
Self {
|
Self {
|
||||||
session_id: session.session_id().as_str().to_owned(),
|
session_id: session.session_id().as_str().to_owned(),
|
||||||
@@ -80,11 +87,17 @@ impl StoredSession {
|
|||||||
PasswordMode::Single => 1,
|
PasswordMode::Single => 1,
|
||||||
PasswordMode::Dual => 2,
|
PasswordMode::Dual => 2,
|
||||||
},
|
},
|
||||||
mailbox_password: mailbox_password.to_owned(),
|
key_passphrases,
|
||||||
key_salts,
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn unlock(&self) -> HashMap<String, Vec<u8>> {
|
||||||
|
self.key_passphrases
|
||||||
|
.iter()
|
||||||
|
.map(|(id, passphrase)| (id.clone(), passphrase.as_bytes().to_vec()))
|
||||||
|
.collect()
|
||||||
|
}
|
||||||
|
|
||||||
fn resume_parameters(&self) -> ResumeParameters {
|
fn resume_parameters(&self) -> ResumeParameters {
|
||||||
ResumeParameters {
|
ResumeParameters {
|
||||||
session_id: self.session_id.clone().into(),
|
session_id: self.session_id.clone().into(),
|
||||||
@@ -354,6 +367,13 @@ impl AccountStore {
|
|||||||
password: password.to_owned(),
|
password: password.to_owned(),
|
||||||
};
|
};
|
||||||
if pending.session.is_waiting_for_second_factor() {
|
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));
|
return Ok(SignIn::SecondFactor(pending));
|
||||||
}
|
}
|
||||||
pending.after_second_factor().await
|
pending.after_second_factor().await
|
||||||
@@ -368,17 +388,8 @@ impl AccountStore {
|
|||||||
return Ok(None);
|
return Ok(None);
|
||||||
};
|
};
|
||||||
let session = ProtonApiSession::resume(client_configuration(), stored.resume_parameters())?;
|
let session = ProtonApiSession::resume(client_configuration(), stored.resume_parameters())?;
|
||||||
persist_refreshed_tokens(
|
persist_refreshed_tokens(&self.secrets, &session, stored.key_passphrases.clone());
|
||||||
&self.secrets,
|
let drive = ProtonDriveClient::with_key_passphrases(&session, stored.unlock());
|
||||||
&session,
|
|
||||||
stored.mailbox_password.clone(),
|
|
||||||
stored.key_salts.clone(),
|
|
||||||
);
|
|
||||||
let drive = ProtonDriveClient::with_key_salts(
|
|
||||||
&session,
|
|
||||||
stored.mailbox_password.into_bytes(),
|
|
||||||
stored.key_salts,
|
|
||||||
);
|
|
||||||
Ok(Some(account(session, drive)))
|
Ok(Some(account(session, drive)))
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -393,29 +404,35 @@ impl AccountStore {
|
|||||||
self.secrets.delete(SESSION_KEY)
|
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<Option<StoredSession>> {
|
fn load(&self) -> Result<Option<StoredSession>> {
|
||||||
let Some(encoded) = self.secrets.get(SESSION_KEY)? else {
|
let Some(encoded) = self.secrets.get(SESSION_KEY)? else {
|
||||||
return Ok(None);
|
return Ok(None);
|
||||||
};
|
};
|
||||||
serde_json::from_str(&encoded)
|
Ok(serde_json::from_str(&encoded)
|
||||||
.map(Some)
|
.inspect_err(|error| {
|
||||||
.map_err(|e| Error::Config(format!("the stored Proton session is unreadable: {e}")))
|
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
|
/// Unlock the key chain with `mailbox_password`, store the session, and hand
|
||||||
/// back the account.
|
/// back the account.
|
||||||
///
|
///
|
||||||
/// The key salts are read here, while the token still has the scope that
|
/// The key passphrases are derived here, while the token still has the scope
|
||||||
/// endpoint needs. Loading the addresses is what proves the mailbox password:
|
/// the key salts need. Loading the addresses is what proves the mailbox
|
||||||
/// it unlocks the user keys, and a wrong password unlocks none of them.
|
/// 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(
|
async fn finish(
|
||||||
secrets: Arc<dyn SecretStore>,
|
secrets: Arc<dyn SecretStore>,
|
||||||
session: ProtonApiSession,
|
session: ProtonApiSession,
|
||||||
mailbox_password: &str,
|
mailbox_password: &str,
|
||||||
) -> Result<Account> {
|
) -> Result<Account> {
|
||||||
let probe = ProtonDriveClient::new(&session, mailbox_password.as_bytes().to_vec());
|
let probe = ProtonDriveClient::new(&session, mailbox_password.as_bytes().to_vec());
|
||||||
let key_salts = probe.account().key_salts().await?;
|
|
||||||
probe
|
probe
|
||||||
.account()
|
.account()
|
||||||
.addresses()
|
.addresses()
|
||||||
@@ -426,28 +443,29 @@ async fn finish(
|
|||||||
}
|
}
|
||||||
other => other.into(),
|
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::<Result<HashMap<_, _>>>()?;
|
||||||
|
|
||||||
let stored = StoredSession::capture(
|
let stored = StoredSession::capture(
|
||||||
&session,
|
&session,
|
||||||
session.current_tokens().await,
|
session.current_tokens().await,
|
||||||
mailbox_password,
|
key_passphrases.clone(),
|
||||||
key_salts.clone(),
|
|
||||||
);
|
);
|
||||||
let encoded = serde_json::to_string(&stored)
|
let encoded = serde_json::to_string(&stored)
|
||||||
.map_err(|e| Error::Config(format!("serialize the Proton session: {e}")))?;
|
.map_err(|e| Error::Config(format!("serialize the Proton session: {e}")))?;
|
||||||
secrets.set(SESSION_KEY, &encoded)?;
|
secrets.set(SESSION_KEY, &encoded)?;
|
||||||
persist_refreshed_tokens(
|
persist_refreshed_tokens(&secrets, &session, key_passphrases);
|
||||||
&secrets,
|
|
||||||
&session,
|
|
||||||
mailbox_password.to_owned(),
|
|
||||||
key_salts.clone(),
|
|
||||||
);
|
|
||||||
|
|
||||||
let drive = ProtonDriveClient::with_key_salts(
|
let drive = ProtonDriveClient::with_key_passphrases(&session, stored.unlock());
|
||||||
&session,
|
|
||||||
mailbox_password.as_bytes().to_vec(),
|
|
||||||
key_salts,
|
|
||||||
);
|
|
||||||
Ok(account(session, drive))
|
Ok(account(session, drive))
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -467,14 +485,12 @@ fn account(session: ProtonApiSession, drive: ProtonDriveClient) -> Account {
|
|||||||
fn persist_refreshed_tokens(
|
fn persist_refreshed_tokens(
|
||||||
secrets: &Arc<dyn SecretStore>,
|
secrets: &Arc<dyn SecretStore>,
|
||||||
session: &ProtonApiSession,
|
session: &ProtonApiSession,
|
||||||
mailbox_password: String,
|
key_passphrases: HashMap<String, String>,
|
||||||
key_salts: Vec<KeySalt>,
|
|
||||||
) {
|
) {
|
||||||
let secrets = Arc::clone(secrets);
|
let secrets = Arc::clone(secrets);
|
||||||
let template = session.clone();
|
let template = session.clone();
|
||||||
session.http().set_on_tokens_refreshed(move |tokens| {
|
session.http().set_on_tokens_refreshed(move |tokens| {
|
||||||
let stored =
|
let stored = StoredSession::capture(&template, tokens, key_passphrases.clone());
|
||||||
StoredSession::capture(&template, tokens, &mailbox_password, key_salts.clone());
|
|
||||||
let written = serde_json::to_string(&stored)
|
let written = serde_json::to_string(&stored)
|
||||||
.map_err(|e| Error::Config(e.to_string()))
|
.map_err(|e| Error::Config(e.to_string()))
|
||||||
.and_then(|encoded| secrets.set(SESSION_KEY, &encoded));
|
.and_then(|encoded| secrets.set(SESSION_KEY, &encoded));
|
||||||
@@ -566,8 +582,7 @@ mod tests {
|
|||||||
refresh_token: "refresh".to_owned(),
|
refresh_token: "refresh".to_owned(),
|
||||||
scopes: vec!["drive".to_owned()],
|
scopes: vec!["drive".to_owned()],
|
||||||
password_mode: 2,
|
password_mode: 2,
|
||||||
mailbox_password: "mailbox".to_owned(),
|
key_passphrases: HashMap::from([("key-1".to_owned(), "derived".to_owned())]),
|
||||||
key_salts: Vec::new(),
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -604,6 +619,29 @@ mod tests {
|
|||||||
assert_eq!(store.username().expect("read"), None);
|
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]
|
#[test]
|
||||||
fn a_stored_session_keeps_its_password_mode() {
|
fn a_stored_session_keeps_its_password_mode() {
|
||||||
let params = stored().resume_parameters();
|
let params = stored().resume_parameters();
|
||||||
|
|||||||
@@ -105,9 +105,11 @@ an edge case.
|
|||||||
- Nothing this app holds can write to the share: a viewer link grants
|
- 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.
|
`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
|
- Signing in to a Proton account is optional (`pstr-core::account`). The
|
||||||
session — tokens, the mailbox password that unlocks the key chain, and the
|
session — tokens, and per account key the passphrase that unlocks it,
|
||||||
key salts a refreshed token can no longer fetch — is one blob in the same
|
derived at sign-in — is one blob in the same credential store, under
|
||||||
credential store, under `proton-account`. Refresh tokens are single-use, so
|
`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
|
every refresh is written back. On the desktop the blob can outgrow one
|
||||||
credential entry — Windows caps one at 1280 UTF-16 units — so
|
credential entry — Windows caps one at 1280 UTF-16 units — so
|
||||||
`KeyringSecretStore` splits a long value across `key#0`, `key#1`, … under a
|
`KeyringSecretStore` splits a long value across `key#0`, `key#1`, … under a
|
||||||
|
|||||||
Reference in new issue
Block a user