chore: uuhhhhhh crypto lol (#805)

* chore: uuhhhhhh crypto lol * remove dead code * fix key decoding * use inplace encryption
2024-11-08 09:25:33 +01:00 · 2023-04-17 21:12:02 +01:00 · 2023-04-17 21:12:02 +01:00 · c7d89c1703
commit c7d89c1703
parent 678323b543
8 changed files with 196 additions and 74 deletions
--- 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"
--- 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"] }
--- 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());
--- 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<u8>,
-    pub nonce: secretbox::Nonce,
+    pub nonce: Nonce<XSalsa20Poly1305>,
 }

-pub fn new_key(settings: &Settings) -> Result<secretbox::Key> {
+pub fn new_key(settings: &Settings) -> Result<Key> {
    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<secretbox::Key> {
 }

 // Loads the secret key, will create + save if it doesn't exist
-pub fn load_key(settings: &Settings) -> Result<secretbox::Key> {
+pub fn load_key(settings: &Settings) -> Result<Key> {
    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<String> {
        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<String> {
    }
 }

-pub type Key = secretbox::Key;
-pub fn encode_key(key: secretbox::Key) -> Result<String> {
-    let buf = rmp_serde::to_vec(&key).wrap_err("could not encode key to message pack")?;
+pub fn encode_key(key: &Key) -> Result<String> {
+    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<secretbox::Key> {
+pub fn decode_key(key: String) -> Result<Key> {
    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<EncryptedHistory> {
+pub fn encrypt(history: &History, key: &Key) -> Result<EncryptedHistory> {
    // 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<History> {
-    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<History> {
+    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");
    }
 }
--- 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"
--- 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<DB: Database>(
    }))
 }

-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()
 }
--- 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}");
                }
--- 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::<bip39::ErrorKind>() {
                        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");
    }
 }