From c7d89c1703c6dc580b2ef2cbb66b0df0b1e72b50 Mon Sep 17 00:00:00 2001 From: Conrad Ludgate Date: Mon, 17 Apr 2023 21:12:02 +0100 Subject: [PATCH] chore: uuhhhhhh crypto lol (#805) * chore: uuhhhhhh crypto lol * remove dead code * fix key decoding * use inplace encryption --- Cargo.lock | 125 ++++++++++++++++++++++++- atuin-client/Cargo.toml | 5 +- atuin-client/src/api_client.rs | 6 +- atuin-client/src/encryption.rs | 69 ++++++++------ atuin-server/Cargo.toml | 1 + atuin-server/src/handlers/user.rs | 39 +++----- atuin/src/command/client/sync.rs | 4 +- atuin/src/command/client/sync/login.rs | 21 ++--- 8 files changed, 196 insertions(+), 74 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 9838dad..bc262bd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2,6 +2,16 @@ # It is not intended for manual editing. version = 3 +[[package]] +name = "aead" +version = "0.5.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d122413f284cf2d62fb1b7db97e02edb8cda96d769b16e443a4f6195e35662b0" +dependencies = [ + "crypto-common", + "generic-array", +] + [[package]] name = "ahash" version = "0.7.6" @@ -37,6 +47,17 @@ version = "1.0.64" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b9a8f622bcf6ff3df478e9deba3e03e4e04b300f8e6a139e192c05fa3490afc7" +[[package]] +name = "argon2" +version = "0.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "95c2fcf79ad1932ac6269a738109997a83c227c09b75842ae564dc8ede6a861c" +dependencies = [ + "base64ct", + "blake2", + "password-hash", +] + [[package]] name = "async-trait" version = "0.1.58" @@ -121,6 +142,7 @@ dependencies = [ "directories", "eyre", "fs-err", + "generic-array", "hex", "interim", "itertools", @@ -145,6 +167,7 @@ dependencies = [ "urlencoding", "uuid", "whoami", + "xsalsa20poly1305", ] [[package]] @@ -161,6 +184,7 @@ dependencies = [ name = "atuin-server" version = "14.0.1" dependencies = [ + "argon2", "async-trait", "atuin-common", "axum", @@ -253,6 +277,12 @@ version = "0.21.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a4a4ddaa51a5bc52a6948f74c06d20aaaddb71924eab79b8c97a8c556e942d6a" +[[package]] +name = "base64ct" +version = "1.6.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8c3c1a368f70d6cf7302d78f8f7093da241fb8e8807c05cc9e51a125895a6d5b" + [[package]] name = "beef" version = "0.5.2" @@ -265,6 +295,15 @@ version = "1.3.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "bef38d45163c2f1dde094a7dfd33ccf595c92905c8f8f4fdc18d06fb1037718a" +[[package]] +name = "blake2" +version = "0.10.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "46502ad458c9a52b69d4d4d32775c788b7a1b85e8bc9d482d92250fc0e3f8efe" +dependencies = [ + "digest", +] + [[package]] name = "block-buffer" version = "0.10.3" @@ -335,6 +374,17 @@ dependencies = [ "chrono", ] +[[package]] +name = "cipher" +version = "0.4.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "773f3b9af64447d2ce9850330c473515014aa235e6a783b02db81ff39e4a3dad" +dependencies = [ + "crypto-common", + "inout", + "zeroize", +] + [[package]] name = "clap" version = "4.1.14" @@ -527,6 +577,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1bfb12502f3fc46cca1bb51ac28df9d618d813cdc3d2f25b9fe775a34af26bb3" dependencies = [ "generic-array", + "rand_core", "typenum", ] @@ -794,6 +845,7 @@ version = "0.14.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "bff49e947297f3312447abdca79f45f4738097cc82b06e72054d2223f601f1b9" dependencies = [ + "serde", "typenum", "version_check", ] @@ -1029,6 +1081,15 @@ dependencies = [ "unicode-width", ] +[[package]] +name = "inout" +version = "0.1.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a0c10553d664a4d0bcff9f4215d0aac67a639cc68ef660840afe309b807bc9f5" +dependencies = [ + "generic-array", +] + [[package]] name = "instant" version = "0.1.12" @@ -1363,6 +1424,12 @@ version = "1.17.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b7e5500299e16ebb147ae15a00a942af264cf3688f47923b8fc2cd5858f23ad3" +[[package]] +name = "opaque-debug" +version = "0.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "624a8340c38c1b80fd549087862da4ba43e08858af025b236e509b6649fc13d5" + [[package]] name = "openssl-probe" version = "0.1.5" @@ -1434,6 +1501,17 @@ dependencies = [ "regex", ] +[[package]] +name = "password-hash" +version = "0.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "346f04948ba92c43e8469c1ee6736c7563d71012b17d40745260fe106aac2166" +dependencies = [ + "base64ct", + "rand_core", + "subtle", +] + [[package]] name = "paste" version = "1.0.9" @@ -1499,6 +1577,17 @@ version = "0.3.25" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1df8c4ec4b0627e53bdf214615ad287367e482558cf84b109250b37464dc03ae" +[[package]] +name = "poly1305" +version = "0.8.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8159bd90725d2df49889a078b54f4f79e87f1f8a8444194cdca81d38f5393abf" +dependencies = [ + "cpufeatures", + "opaque-debug", + "universal-hash", +] + [[package]] name = "portable-atomic" version = "0.3.19" @@ -1552,9 +1641,9 @@ dependencies = [ [[package]] name = "rand_core" -version = "0.6.3" +version = "0.6.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d34f1408f55294453790c48b2f1ebbb1c5b4b7563eb1f418bcfcfdbb06ebb4e7" +checksum = "ec0be4795e2f6a28069bec0b5ff3e2ac9bafc99e6a9a7dc3547996c5c816922c" dependencies = [ "getrandom", ] @@ -1776,6 +1865,15 @@ version = "1.0.11" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "4501abdff3ae82a1c1b477a17252eb69cee9e66eb915c1abaa4f44d873df9f09" +[[package]] +name = "salsa20" +version = "0.10.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "97a22f5af31f73a954c10289c93e8a50cc23d971e80ee446f1f6f7137a088213" +dependencies = [ + "cipher", +] + [[package]] name = "same-file" version = "1.0.6" @@ -2506,6 +2604,16 @@ version = "0.1.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "39ec24b3121d976906ece63c9daad25b85969647682eee313cb5779fdd69e14e" +[[package]] +name = "universal-hash" +version = "0.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7d3160b73c9a19f7e2939a2fdad446c57c1bbbbf4d919d3213ff1267a580d8b5" +dependencies = [ + "crypto-common", + "subtle", +] + [[package]] name = "untrusted" version = "0.7.1" @@ -2895,6 +3003,19 @@ dependencies = [ "winapi", ] +[[package]] +name = "xsalsa20poly1305" +version = "0.9.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "472c385ee974833d7e59979eeb74175d56774be3768b5bcc581337e21396bda3" +dependencies = [ + "aead", + "poly1305", + "salsa20", + "subtle", + "zeroize", +] + [[package]] name = "zeroize" version = "1.6.0" diff --git a/atuin-client/Cargo.toml b/atuin-client/Cargo.toml index 6c5d04b..3542510 100644 --- a/atuin-client/Cargo.toml +++ b/atuin-client/Cargo.toml @@ -15,12 +15,13 @@ repository = { workspace = true } default = ["sync"] sync = [ "urlencoding", - "sodiumoxide", "reqwest", "sha2", "hex", "rmp-serde", "base64", + "generic-array", + "xsalsa20poly1305", ] [dependencies] @@ -60,6 +61,8 @@ rmp-serde = { version = "1.1.1", optional = true } base64 = { workspace = true, optional = true } tokio = { workspace = true } semver = { workspace = true } +xsalsa20poly1305 = { version = "0.9.0", optional = true } +generic-array = { version = "0.14", optional = true, features = ["serde"] } [dev-dependencies] tokio = { version = "1", features = ["full"] } diff --git a/atuin-client/src/api_client.rs b/atuin-client/src/api_client.rs index dee9861..80051ba 100644 --- a/atuin-client/src/api_client.rs +++ b/atuin-client/src/api_client.rs @@ -7,13 +7,13 @@ use reqwest::{ header::{HeaderMap, AUTHORIZATION, USER_AGENT}, StatusCode, Url, }; -use sodiumoxide::crypto::secretbox; use atuin_common::api::{ AddHistoryRequest, CountResponse, DeleteHistoryRequest, ErrorResponse, IndexResponse, LoginRequest, LoginResponse, RegisterResponse, StatusResponse, SyncHistoryResponse, }; use semver::Version; +use xsalsa20poly1305::Key; use crate::{ encryption::{decode_key, decrypt}, @@ -28,7 +28,7 @@ static APP_USER_AGENT: &str = concat!("atuin/", env!("CARGO_PKG_VERSION"),); pub struct Client<'a> { sync_addr: &'a str, - key: secretbox::Key, + key: Key, client: reqwest::Client, } @@ -182,7 +182,7 @@ impl<'a> Client<'a> { .iter() // TODO: handle deletion earlier in this chain .map(|h| serde_json::from_str(h).expect("invalid base64")) - .map(|h| decrypt(&h, &self.key).expect("failed to decrypt history! check your key")) + .map(|h| decrypt(h, &self.key).expect("failed to decrypt history! check your key")) .map(|mut h| { if deleted.contains(&h.id) { h.deleted_at = Some(chrono::Utc::now()); diff --git a/atuin-client/src/encryption.rs b/atuin-client/src/encryption.rs index fe19ce9..9145a11 100644 --- a/atuin-client/src/encryption.rs +++ b/atuin-client/src/encryption.rs @@ -14,7 +14,11 @@ use base64::prelude::{Engine, BASE64_STANDARD}; use eyre::{eyre, Context, Result}; use fs_err as fs; use serde::{Deserialize, Serialize}; -use sodiumoxide::crypto::secretbox; +pub use xsalsa20poly1305::Key; +use xsalsa20poly1305::{ + aead::{Nonce, OsRng}, + AeadInPlace, KeyInit, XSalsa20Poly1305, +}; use crate::{ history::{History, HistoryWithoutDelete}, @@ -24,14 +28,14 @@ use crate::{ #[derive(Debug, Serialize, Deserialize)] pub struct EncryptedHistory { pub ciphertext: Vec, - pub nonce: secretbox::Nonce, + pub nonce: Nonce, } -pub fn new_key(settings: &Settings) -> Result { +pub fn new_key(settings: &Settings) -> Result { let path = settings.key_path.as_str(); - let key = secretbox::gen_key(); - let encoded = encode_key(key.clone())?; + let key = XSalsa20Poly1305::generate_key(&mut OsRng); + let encoded = encode_key(&key)?; let mut file = fs::File::create(path)?; file.write_all(encoded.as_bytes())?; @@ -40,7 +44,7 @@ pub fn new_key(settings: &Settings) -> Result { } // Loads the secret key, will create + save if it doesn't exist -pub fn load_key(settings: &Settings) -> Result { +pub fn load_key(settings: &Settings) -> Result { let path = settings.key_path.as_str(); let key = if PathBuf::from(path).exists() { @@ -60,8 +64,8 @@ pub fn load_encoded_key(settings: &Settings) -> Result { let key = fs::read_to_string(path)?; Ok(key) } else { - let key = secretbox::gen_key(); - let encoded = encode_key(key)?; + let key = XSalsa20Poly1305::generate_key(&mut OsRng); + let encoded = encode_key(&key)?; let mut file = fs::File::create(path)?; file.write_all(encoded.as_bytes())?; @@ -70,38 +74,47 @@ pub fn load_encoded_key(settings: &Settings) -> Result { } } -pub type Key = secretbox::Key; -pub fn encode_key(key: secretbox::Key) -> Result { - let buf = rmp_serde::to_vec(&key).wrap_err("could not encode key to message pack")?; +pub fn encode_key(key: &Key) -> Result { + let buf = rmp_serde::to_vec(key.as_slice()).wrap_err("could not encode key to message pack")?; let buf = BASE64_STANDARD.encode(buf); Ok(buf) } -pub fn decode_key(key: String) -> Result { +pub fn decode_key(key: String) -> Result { let buf = BASE64_STANDARD .decode(key.trim_end()) .wrap_err("encryption key is not a valid base64 encoding")?; - let buf: secretbox::Key = rmp_serde::from_slice(&buf) + let buf: &[u8] = rmp_serde::from_slice(&buf) .wrap_err("encryption key is not a valid message pack encoding")?; - Ok(buf) + Ok(*Key::from_slice(buf)) } -pub fn encrypt(history: &History, key: &secretbox::Key) -> Result { +pub fn encrypt(history: &History, key: &Key) -> Result { // serialize with msgpack - let buf = rmp_serde::to_vec(history)?; + let mut buf = rmp_serde::to_vec(history)?; - let nonce = secretbox::gen_nonce(); + let nonce = XSalsa20Poly1305::generate_nonce(&mut OsRng); + XSalsa20Poly1305::new(key) + .encrypt_in_place(&nonce, &[], &mut buf) + .map_err(|_| eyre!("could not encrypt"))?; - let ciphertext = secretbox::seal(&buf, &nonce, key); - - Ok(EncryptedHistory { ciphertext, nonce }) + Ok(EncryptedHistory { + ciphertext: buf, + nonce, + }) } -pub fn decrypt(encrypted_history: &EncryptedHistory, key: &secretbox::Key) -> Result { - let plaintext = secretbox::open(&encrypted_history.ciphertext, &encrypted_history.nonce, key) - .map_err(|_| eyre!("failed to open secretbox - invalid key?"))?; +pub fn decrypt(mut encrypted_history: EncryptedHistory, key: &Key) -> Result { + XSalsa20Poly1305::new(key) + .decrypt_in_place( + &encrypted_history.nonce, + &[], + &mut encrypted_history.ciphertext, + ) + .map_err(|_| eyre!("could not encrypt"))?; + let plaintext = encrypted_history.ciphertext; let history = rmp_serde::from_slice(&plaintext); @@ -126,7 +139,7 @@ pub fn decrypt(encrypted_history: &EncryptedHistory, key: &secretbox::Key) -> Re #[cfg(test)] mod test { - use sodiumoxide::crypto::secretbox; + use xsalsa20poly1305::{aead::OsRng, KeyInit, XSalsa20Poly1305}; use crate::history::History; @@ -134,8 +147,8 @@ mod test { #[test] fn test_encrypt_decrypt() { - let key1 = secretbox::gen_key(); - let key2 = secretbox::gen_key(); + let key1 = XSalsa20Poly1305::generate_key(&mut OsRng); + let key2 = XSalsa20Poly1305::generate_key(&mut OsRng); let history = History::new( chrono::Utc::now(), @@ -156,12 +169,12 @@ mod test { // test decryption works // this should pass - match decrypt(&e1, &key1) { + match decrypt(e1, &key1) { Err(e) => panic!("failed to decrypt, got {}", e), Ok(h) => assert_eq!(h, history), }; // this should err - let _ = decrypt(&e2, &key1).expect_err("expected an error decrypting with invalid key"); + let _ = decrypt(e2, &key1).expect_err("expected an error decrypting with invalid key"); } } diff --git a/atuin-server/Cargo.toml b/atuin-server/Cargo.toml index a60bc25..773f3eb 100644 --- a/atuin-server/Cargo.toml +++ b/atuin-server/Cargo.toml @@ -33,3 +33,4 @@ chronoutil = "0.2.3" tower = "0.4" tower-http = { version = "0.3", features = ["trace"] } reqwest = { workspace = true } +argon2 = "0.5.0" diff --git a/atuin-server/src/handlers/user.rs b/atuin-server/src/handlers/user.rs index 61af989..89aa060 100644 --- a/atuin-server/src/handlers/user.rs +++ b/atuin-server/src/handlers/user.rs @@ -2,12 +2,16 @@ use std::borrow::Borrow; use std::collections::HashMap; use std::time::Duration; +use argon2::{ + password_hash::SaltString, Algorithm, Argon2, Params, PasswordHash, PasswordHasher, + PasswordVerifier, Version, +}; use axum::{ extract::{Path, State}, Json, }; use http::StatusCode; -use sodiumoxide::crypto::pwhash::argon2id13; +use rand::rngs::OsRng; use tracing::{debug, error, info, instrument}; use uuid::Uuid; @@ -22,18 +26,10 @@ use reqwest::header::CONTENT_TYPE; use atuin_common::api::*; -pub fn verify_str(secret: &str, verify: &str) -> bool { - sodiumoxide::init().unwrap(); - - let mut padded = [0_u8; 128]; - secret.as_bytes().iter().enumerate().for_each(|(i, val)| { - padded[i] = *val; - }); - - match argon2id13::HashedPassword::from_slice(&padded) { - Some(hp) => argon2id13::pwhash_verify(&hp, verify.as_bytes()), - None => false, - } +pub fn verify_str(hash: &str, password: &str) -> bool { + let arg2 = Argon2::new(Algorithm::Argon2id, Version::V0x13, Params::default()); + let Ok(hash) = PasswordHash::new(hash) else { return false }; + arg2.verify_password(password.as_bytes(), &hash).is_ok() } // Try to send a Discord webhook once - if it fails, we don't retry. "At most once", and best effort. @@ -185,16 +181,9 @@ pub async fn login( })) } -fn hash_secret(secret: &str) -> String { - sodiumoxide::init().unwrap(); - let hash = argon2id13::pwhash( - secret.as_bytes(), - argon2id13::OPSLIMIT_INTERACTIVE, - argon2id13::MEMLIMIT_INTERACTIVE, - ) - .unwrap(); - let texthash = std::str::from_utf8(&hash.0).unwrap().to_string(); - - // postgres hates null chars. don't do that to postgres - texthash.trim_end_matches('\u{0}').to_string() +fn hash_secret(password: &str) -> String { + let arg2 = Argon2::new(Algorithm::Argon2id, Version::V0x13, Params::default()); + let salt = SaltString::generate(&mut OsRng); + let hash = arg2.hash_password(password.as_bytes(), &salt).unwrap(); + hash.to_string() } diff --git a/atuin/src/command/client/sync.rs b/atuin/src/command/client/sync.rs index 419177a..3980e13 100644 --- a/atuin/src/command/client/sync.rs +++ b/atuin/src/command/client/sync.rs @@ -50,10 +50,10 @@ impl Cmd { let key = load_key(&settings).wrap_err("could not load encryption key")?; if base64 { - let encode = encode_key(key).wrap_err("could not encode encryption key")?; + let encode = encode_key(&key).wrap_err("could not encode encryption key")?; println!("{encode}"); } else { - let mnemonic = bip39::Mnemonic::from_entropy(&key.0, bip39::Language::English) + let mnemonic = bip39::Mnemonic::from_entropy(&key, bip39::Language::English) .map_err(|_| eyre::eyre!("invalid key"))?; println!("{mnemonic}"); } diff --git a/atuin/src/command/client/sync/login.rs b/atuin/src/command/client/sync/login.rs index 6aa2d84..9bfe0b4 100644 --- a/atuin/src/command/client/sync/login.rs +++ b/atuin/src/command/client/sync/login.rs @@ -1,7 +1,7 @@ use std::{io, path::PathBuf}; use clap::Parser; -use eyre::{bail, Context, ContextCompat, Result}; +use eyre::{bail, Context, Result}; use tokio::{fs::File, io::AsyncWriteExt}; use atuin_client::{ @@ -62,10 +62,7 @@ impl Cmd { } else { // try parse the key as a mnemonic... let key = match bip39::Mnemonic::from_phrase(&key, bip39::Language::English) { - Ok(mnemonic) => encode_key( - Key::from_slice(mnemonic.entropy()) - .context("key was not the correct length")?, - )?, + Ok(mnemonic) => encode_key(Key::from_slice(mnemonic.entropy()))?, Err(err) => { if let Some(err) = err.downcast_ref::() { match err { @@ -131,17 +128,15 @@ mod tests { #[test] fn mnemonic_round_trip() { - let key = Key { - 0: [ - 3, 1, 4, 1, 5, 9, 2, 6, 5, 3, 5, 8, 9, 7, 9, 3, 2, 3, 8, 4, 6, 2, 6, 4, 3, 3, 8, 3, - 2, 7, 9, 5, - ], - }; - let phrase = bip39::Mnemonic::from_entropy(&key.0, bip39::Language::English) + let key = Key::from([ + 3, 1, 4, 1, 5, 9, 2, 6, 5, 3, 5, 8, 9, 7, 9, 3, 2, 3, 8, 4, 6, 2, 6, 4, 3, 3, 8, 3, 2, + 7, 9, 5, + ]); + let phrase = bip39::Mnemonic::from_entropy(&key, bip39::Language::English) .unwrap() .into_phrase(); let mnemonic = bip39::Mnemonic::from_phrase(&phrase, bip39::Language::English).unwrap(); - assert_eq!(mnemonic.entropy(), &key.0); + assert_eq!(mnemonic.entropy(), key.as_slice()); assert_eq!(phrase, "adapt amused able anxiety mother adapt beef gaze amount else seat alcohol cage lottery avoid scare alcohol cactus school avoid coral adjust catch pink"); } }