From 3a49f3ba1d7bd8398457177cc48a05536eb6179f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= Date: Thu, 22 Oct 2026 21:44:57 +0200 Subject: [PATCH] crypto: give every encrypted entry its own salt MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit One salt served the whole database, so a single argon2 evaluation per candidate password could be tested against every encrypted row at once — cracking many cost the same as cracking one — and two pastes sharing a password shared a key. The shipped default for it is the literal `somesalt`, so one precomputation works against every instance that never set the variable. Mint 16 random bytes per entry and store them beside the ciphertext in a new `salt` column, deriving with Argon2id, which RFC 9106 recommends over the Argon2i used here. Entries already stored keep the derivation they were sealed under, selected by whether they carry a salt; the process-wide salt stays for exactly those. A ciphertext captured from the old scheme guards that path, since nothing else would notice it breaking. Checked end to end as well: a database written by the previous release, with an encrypted paste in it, migrates and still opens with the right password. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 3 + README.md | 2 +- crates/wastebin_core/src/crypto.rs | 146 ++++++++++++++++-- crates/wastebin_core/src/db.rs | 87 +++++++++-- .../src/migrations/0008-add-salt-column.sql | 1 + 5 files changed, 216 insertions(+), 23 deletions(-) create mode 100644 crates/wastebin_core/src/migrations/0008-add-salt-column.sql diff --git a/CHANGELOG.md b/CHANGELOG.md index 9d352978..b1d2dd3e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,9 @@ ### Fixed +- Derive the key of every newly encrypted paste from a random salt of its own, + using Argon2id, instead of one salt shared by the whole database. Pastes + encrypted earlier keep opening with `WASTEBIN_PASSWORD_SALT`. - Reject off-origin and scheme-relative `Referer` values when redirecting back after a theme change, closing an open-redirect bypass via `//`-prefixed paths. - Unlink Unix domain socket after shutdown. diff --git a/README.md b/README.md index 4a2d058e..3884f37d 100644 --- a/README.md +++ b/README.md @@ -204,7 +204,7 @@ run-time behavior: | `WASTEBIN_DATABASE_PATH` | Path to the sqlite3 database file. | `:memory:` | | `WASTEBIN_HTTP_TIMEOUT` | Maximum number of seconds a request is processed until wastebin responds with 408. | `5` | | `WASTEBIN_MAX_BODY_SIZE` | Number of bytes to accept for POST requests. | `1048576`, i.e. 1 MB | -| `WASTEBIN_PASSWORD_SALT` | Salt used to hash user passwords used for encrypting pastes. | `somesalt` | +| `WASTEBIN_PASSWORD_SALT` | Salt used to hash the passwords of pastes encrypted before each paste got a random salt of its own; only needed to keep reading those. | `somesalt` | | `WASTEBIN_PASTE_EXPIRATIONS` | Possible paste expirations as a comma-separated list of seconds or values with duration magnitudes (`s`, `m`, `h`, `d`, `M`, `y` for seconds, minutes, hours, days, months and years respectively). Appending `=d` to one of the value makes it the default selection. | see [here](https://github.com/matze/wastebin/blob/eb61c78506a165605f145e8374ed64822405eda0/crates/wastebin_server/src/env.rs#L166) | | `WASTEBIN_SIGNING_KEY` | Key to sign cookies. Must be at least 64 bytes long. | Random key generated at startup, i.e. cookies will become invalid after restarts and paste creators will not be able to delete their pastes. | | `WASTEBIN_THEME` | Theme colors, one of `ayu`, `base16ocean`, `catppuccin`, `coldark`, `gruvbox`, `monokai`, `onehalf`, `rosepine`, `solarized`. See [this page](https://matze.github.io/wastebin/) for a preview. | `ayu` | diff --git a/crates/wastebin_core/src/crypto.rs b/crates/wastebin_core/src/crypto.rs index 33baa8b7..83182ccc 100644 --- a/crates/wastebin_core/src/crypto.rs +++ b/crates/wastebin_core/src/crypto.rs @@ -7,7 +7,11 @@ use tokio::task::spawn_blocking; use crate::env; -static CONFIG: LazyLock = LazyLock::new(|| argon2::Config { +/// Derivation used for entries sealed before each carried a salt of its own. +/// +/// Those entries are already in databases, and their key depends on every value here, so this +/// cannot change without making them unreadable. +static LEGACY_CONFIG: LazyLock = LazyLock::new(|| argon2::Config { variant: argon2::Variant::Argon2i, version: argon2::Version::Version13, mem_cost: 65536, @@ -19,8 +23,28 @@ static CONFIG: LazyLock = LazyLock::new(|| argon2::Config { hash_length: 32, }); +/// Derivation for entries that carry their own salt. +/// +/// Argon2id rather than Argon2i: RFC 9106 §4 recommends it, and its hybrid addressing is what +/// resists the tradeoff attacks that Argon2i's data-independent addressing invites. +static CONFIG: LazyLock = LazyLock::new(|| argon2::Config { + variant: argon2::Variant::Argon2id, + version: argon2::Version::Version13, + mem_cost: 65536, + time_cost: 10, + lanes: 4, + thread_mode: argon2::ThreadMode::Parallel, + secret: &[], + ad: &[], + hash_length: 32, +}); + +/// Process-wide salt, used only for entries sealed before each carried its own. static SALT: LazyLock = LazyLock::new(env::password_hash_salt); +/// Bytes of salt minted for a new entry. +const ENTRY_SALT_LEN: usize = 16; + /// Encryption or decryption errors. #[derive(thiserror::Error, Debug)] pub enum Error { @@ -44,6 +68,40 @@ pub(crate) struct Encrypted { pub struct Password(Vec); +/// How an entry's key is derived. +/// +/// One salt shared by every entry made a single argon2 evaluation per candidate password testable +/// against the whole database at once, and gave two pastes with the same password the same key. +/// New entries carry their own; the ones already stored keep the derivation they were sealed with. +#[derive(Clone, Debug)] +pub(crate) enum Derivation { + /// An entry from before per-entry salts: the process-wide salt and Argon2i. + Legacy, + /// An entry carrying a salt of its own, derived with Argon2id. + PerEntry(Vec), +} + +/// Mint a fresh salt for a new entry. +pub(crate) fn new_salt() -> Vec { + rand::rng().random::<[u8; ENTRY_SALT_LEN]>().to_vec() +} + +impl Derivation { + fn config(&self) -> &'static argon2::Config<'static> { + match self { + Self::Legacy => &LEGACY_CONFIG, + Self::PerEntry(_) => &CONFIG, + } + } + + fn salt(&self) -> &[u8] { + match self { + Self::Legacy => SALT.as_bytes(), + Self::PerEntry(salt) => salt, + } + } +} + /// Plaintext bytes to be encrypted. pub(crate) struct Plaintext(Vec); @@ -59,17 +117,25 @@ impl From> for Plaintext { } } -fn cipher_from(password: &[u8]) -> Result { - let key = argon2::hash_raw(password, SALT.as_bytes(), &CONFIG)?; +fn cipher_from( + password: &[u8], + salt: &[u8], + config: &argon2::Config, +) -> Result { + let key = argon2::hash_raw(password, salt, config)?; let key = Key::try_from(key.as_slice()).map_err(|_| Error::ChaCha20Poly1305Encrypt)?; Ok(XChaCha20Poly1305::new(&key)) } impl Plaintext { /// Consume and encrypt plaintext into [`Encrypted`] using `password`. - pub async fn encrypt(self, password: Password) -> Result { + pub async fn encrypt( + self, + password: Password, + derivation: Derivation, + ) -> Result { spawn_blocking(move || { - let cipher = cipher_from(&password.0)?; + let cipher = cipher_from(&password.0, derivation.salt(), derivation.config())?; let nonce = XNonce::from(rand::rng().random::<[u8; 24]>()); let ciphertext = cipher .encrypt(&nonce, self.0.as_ref()) @@ -88,9 +154,13 @@ impl Encrypted { } /// Decrypt into bytes using `password`. - pub async fn decrypt(self, password: Password) -> Result, Error> { + pub async fn decrypt( + self, + password: Password, + derivation: Derivation, + ) -> Result, Error> { spawn_blocking(move || { - let cipher = cipher_from(&password.0)?; + let cipher = cipher_from(&password.0, derivation.salt(), derivation.config())?; let nonce = XNonce::try_from(self.nonce.as_slice()) .map_err(|_| Error::ChaCha20Poly1305Decrypt)?; let plaintext = cipher @@ -111,13 +181,71 @@ mod tests { let password = "secret".to_string(); let plaintext = "encrypt me".to_string(); let encrypted = Plaintext::from(plaintext.as_bytes().to_vec()) - .encrypt(Password::from(password.as_bytes().to_vec())) + .encrypt( + Password::from(password.as_bytes().to_vec()), + Derivation::Legacy, + ) .await .unwrap(); let decrypted = encrypted - .decrypt(Password::from(password.as_bytes().to_vec())) + .decrypt( + Password::from(password.as_bytes().to_vec()), + Derivation::Legacy, + ) .await .unwrap(); assert_eq!(decrypted, plaintext.as_bytes()); } + + /// Captured from the scheme that shipped before per-entry salts: the default process-wide + /// salt and Argon2i. Every paste encrypted by a running instance is sealed exactly like this, + /// so the legacy derivation has to keep reproducing the same key byte for byte. + #[test] + fn a_paste_sealed_before_per_entry_salts_still_opens() { + fn unhex(s: &str) -> Vec { + (0..s.len()) + .step_by(2) + .map(|i| u8::from_str_radix(&s[i..i + 2], 16).unwrap()) + .collect() + } + + let ciphertext = unhex("099efe0be8d9b175835af41b5fe908b3ebc34efdebd11047299413bf40"); + let nonce = unhex("2514a266bf4d6ddf4ac3d02edb795b493a9756b43e81aabf"); + let nonce = XNonce::try_from(nonce.as_slice()).unwrap(); + + // The salt is spelled out rather than read through `SALT`, which follows the environment. + let cipher = cipher_from(b"hunter2", b"somesalt", Derivation::Legacy.config()).unwrap(); + let decrypted = cipher.decrypt(&nonce, ciphertext.as_ref()).unwrap(); + + assert_eq!(decrypted, b"legacy secret"); + } + + /// The salt exists to make one derivation testable against one entry. Sharing it across every + /// paste meant a single argon2 evaluation per candidate password could be checked against all + /// of them at once, and two pastes with the same password shared a key. + #[tokio::test] + async fn two_pastes_with_one_password_do_not_share_a_key() { + let first_salt = new_salt(); + let second_salt = new_salt(); + assert_ne!(first_salt, second_salt); + + let first = Plaintext::from(b"encrypt me".to_vec()) + .encrypt( + Password::from(b"secret".to_vec()), + Derivation::PerEntry(first_salt), + ) + .await + .unwrap(); + + // The key derived for one entry must not open another, even with the right password. + assert!( + first + .decrypt( + Password::from(b"secret".to_vec()), + Derivation::PerEntry(second_salt), + ) + .await + .is_err() + ); + } } diff --git a/crates/wastebin_core/src/db.rs b/crates/wastebin_core/src/db.rs index dee81566..9791ae23 100644 --- a/crates/wastebin_core/src/db.rs +++ b/crates/wastebin_core/src/db.rs @@ -105,7 +105,7 @@ pub enum Open { /// Module with types for insertion. pub mod write { - use crate::crypto::{Encrypted, Password, Plaintext}; + use crate::crypto::{self, Derivation, Encrypted, Password, Plaintext}; use crate::db::Error; use async_compression::tokio::bufread::ZstdEncoder; use chacha20poly1305::XNonce; @@ -149,6 +149,8 @@ pub mod write { pub data: Vec, /// Nonce for this entry pub nonce: Option, + /// Salt this entry's key was derived under, if it is encrypted + pub salt: Option>, } impl Entry { @@ -167,21 +169,25 @@ pub mod write { } impl CompressedEntry { - /// Encrypt if password is set. + /// Encrypt if password is set, under a salt minted for this entry alone. pub async fn encrypt(self) -> Result { - let (data, nonce) = if let Some(password) = &self.entry.password { + let (data, nonce, salt) = if let Some(password) = &self.entry.password { let password = Password::from(password.as_bytes().to_vec()); let plaintext = Plaintext::from(self.data); - let Encrypted { ciphertext, nonce } = plaintext.encrypt(password).await?; - (ciphertext, Some(nonce)) + let salt = crypto::new_salt(); + let Encrypted { ciphertext, nonce } = plaintext + .encrypt(password, Derivation::PerEntry(salt.clone())) + .await?; + (ciphertext, Some(nonce), Some(salt)) } else { - (self.data, None) + (self.data, None, None) }; Ok(DatabaseEntry { entry: self.entry, data, nonce, + salt, }) } } @@ -189,7 +195,7 @@ pub mod write { /// Module with types for reading from the database. pub mod read { - use crate::crypto::{Encrypted, Password}; + use crate::crypto::{Derivation, Encrypted, Password}; use crate::db::Error; use crate::expiration::Expiration; use crate::id::Id; @@ -211,6 +217,9 @@ pub mod read { pub must_be_deleted: bool, /// Nonce for this entry pub nonce: Option, + /// Salt this entry was sealed under. `None` for entries stored before each carried one, + /// which are still opened with the process-wide salt they were sealed with. + pub salt: Option>, } /// Potentially decrypted but still compressed entry @@ -284,10 +293,13 @@ pub mod read { } impl DatabaseEntry { + /// Decrypt with the derivation this entry was sealed under. pub async fn decrypt( self, password: Option, ) -> Result { + let derivation = self.salt.map_or(Derivation::Legacy, Derivation::PerEntry); + match (self.nonce, password) { (Some(_), None) => Err(Error::NoPassword), (None, None | Some(_)) => Ok(CompressedReadEntry { @@ -297,7 +309,7 @@ pub mod read { }), (Some(nonce), Some(password)) => { let encrypted = Encrypted::new(self.data, nonce); - let decrypted = encrypted.decrypt(password).await?; + let decrypted = encrypted.decrypt(password, derivation).await?; Ok(CompressedReadEntry { data: decrypted, metadata: self.metadata, @@ -381,6 +393,7 @@ impl Handler { M::up(include_str!("migrations/0005-drop-text-column.sql")), M::up(include_str!("migrations/0006-add-nonce-column.sql")), M::up(include_str!("migrations/0007-add-title-column.sql")), + M::up(include_str!("migrations/0008-add-salt-column.sql")), ]); migrations.to_latest(&mut conn)?; @@ -434,7 +447,12 @@ impl Handler { fn insert( &self, - write::DatabaseEntry { entry, data, nonce }: write::DatabaseEntry, + write::DatabaseEntry { + entry, + data, + nonce, + salt, + }: write::DatabaseEntry, ) -> Result<(Id, write::Entry), Error> { let mut counter = 0; let title = entry.title.clone(); @@ -445,11 +463,11 @@ impl Handler { let result = match entry.expires { None => self.conn.execute( - "INSERT INTO entries (id, uid, data, burn_after_reading, nonce, title) VALUES (?1, ?2, ?3, ?4, ?5, ?6)", - params![id.to_i64(), entry.uid, data, entry.burn_after_reading, nonce, title], + "INSERT INTO entries (id, uid, data, burn_after_reading, nonce, title, salt) VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7)", + params![id.to_i64(), entry.uid, data, entry.burn_after_reading, nonce, title, salt], ), Some(expires) => self.conn.execute( - "INSERT INTO entries (id, uid, data, burn_after_reading, nonce, expires, title) VALUES (?1, ?2, ?3, ?4, ?5, datetime('now', ?6), ?7)", + "INSERT INTO entries (id, uid, data, burn_after_reading, nonce, expires, title, salt) VALUES (?1, ?2, ?3, ?4, ?5, datetime('now', ?6), ?7, ?8)", params![ id.to_i64(), entry.uid, @@ -458,6 +476,7 @@ impl Handler { nonce, format!("{expires} seconds"), title, + salt, ], ), }; @@ -511,7 +530,7 @@ impl Handler { fn get(&self, id: Id) -> Result { let metadata = self.get_metadata(id)?; let entry = self.conn.query_row( - "SELECT data, burn_after_reading, nonce, expires < datetime('now') FROM entries WHERE id=?1", + "SELECT data, burn_after_reading, nonce, expires < datetime('now'), salt FROM entries WHERE id=?1", params![id.to_i64()], |row| { let nonce = row @@ -532,6 +551,7 @@ impl Handler { must_be_deleted: row.get::<_, Option>(1)?.unwrap_or(false), nonce, expired: row.get::<_, Option>(3)?.unwrap_or(false), + salt: row.get(4)?, }) }, )?; @@ -788,6 +808,47 @@ mod tests { Ok(db) } + /// Every encrypted entry is sealed under a salt of its own; without one stored beside it, the + /// entry silently falls back to the process-wide salt shared by the whole database. + #[tokio::test] + async fn encrypted_entries_carry_their_own_salt() -> Result<(), Box> { + let (_sender, receiver) = kanal::bounded(1); + let handler = Handler::new(Open::Memory, receiver)?; + + let mut salts = Vec::new(); + + for _ in 0..2 { + let entry = write::Entry { + text: "secret".to_string(), + password: Some("same password".to_string()), + ..Default::default() + }; + let (id, _) = handler.insert(entry.compress().await?.encrypt().await?)?; + let salt = handler + .get(id)? + .salt + .expect("encrypted entry without a salt"); + assert_eq!(salt.len(), 16); + salts.push(salt); + } + + assert_ne!(salts[0], salts[1]); + + let (id, _) = handler.insert( + write::Entry { + text: "plain".to_string(), + ..Default::default() + } + .compress() + .await? + .encrypt() + .await?, + )?; + assert!(handler.get(id)?.salt.is_none()); + + Ok(()) + } + #[tokio::test] async fn insert() -> Result<(), Box> { let db = new_db()?; diff --git a/crates/wastebin_core/src/migrations/0008-add-salt-column.sql b/crates/wastebin_core/src/migrations/0008-add-salt-column.sql new file mode 100644 index 00000000..cd271a6e --- /dev/null +++ b/crates/wastebin_core/src/migrations/0008-add-salt-column.sql @@ -0,0 +1 @@ +ALTER TABLE entries ADD COLUMN salt BLOB;