From dd796128319c5c4e0f7db5c52d2c17b337398a77 Mon Sep 17 00:00:00 2001 From: Conrad Ludgate Date: Tue, 20 Jun 2023 13:04:28 +0100 Subject: [PATCH] move keys outside of sync encryption --- atuin-client/Cargo.toml | 2 +- atuin-client/src/encryption.rs | 62 +------------ atuin-client/src/record/encodings/key.rs | 91 ++++++++++++++++---- atuin-client/src/record/encodings/mod.rs | 2 +- atuin-client/src/record/encryption/none.rs | 10 +-- atuin-client/src/record/mod.rs | 2 +- atuin-client/src/sync.rs | 10 ++- atuin/src/command/client/account/login.rs | 9 +- atuin/src/command/client/account/register.rs | 2 +- atuin/src/command/client/sync.rs | 2 +- 10 files changed, 101 insertions(+), 91 deletions(-) diff --git a/atuin-client/Cargo.toml b/atuin-client/Cargo.toml index 6492bd15..6da679f8 100644 --- a/atuin-client/Cargo.toml +++ b/atuin-client/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "atuin-client" -edition = "2018" +edition = "2021" description = "client library for atuin" version = { workspace = true } diff --git a/atuin-client/src/encryption.rs b/atuin-client/src/encryption.rs index 86125159..9ebb6cdf 100644 --- a/atuin-client/src/encryption.rs +++ b/atuin-client/src/encryption.rs @@ -8,12 +8,8 @@ // clients must share the secret in order to be able to sync, as it is needed // to decrypt -use std::{io::prelude::*, path::PathBuf}; - -use base64::prelude::{Engine, BASE64_STANDARD}; use chrono::{DateTime, Utc}; -use eyre::{bail, eyre, Context, Result}; -use fs_err as fs; +use eyre::{bail, eyre, Result}; use rmp::{decode::Bytes, Marker}; use serde::{Deserialize, Serialize}; pub use xsalsa20poly1305::Key; @@ -22,7 +18,7 @@ use xsalsa20poly1305::{ AeadInPlace, KeyInit, XSalsa20Poly1305, }; -use crate::{history::History, settings::Settings}; +use crate::history::History; #[derive(Debug, Serialize, Deserialize)] pub struct EncryptedHistory { @@ -30,60 +26,6 @@ pub struct EncryptedHistory { pub nonce: Nonce, } -pub fn new_key(settings: &Settings) -> Result { - let path = settings.key_path.as_str(); - - 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())?; - - Ok(key) -} - -// Loads the secret key, will create + save if it doesn't exist -pub fn load_key(settings: &Settings) -> Result { - let path = settings.key_path.as_str(); - - let key = if PathBuf::from(path).exists() { - let key = fs_err::read_to_string(path)?; - decode_key(key)? - } else { - new_key(settings)? - }; - - Ok(key) -} - -pub fn encode_key(key: &Key) -> Result { - let mut buf = vec![]; - rmp::encode::write_bin(&mut buf, 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 { - let buf = BASE64_STANDARD - .decode(key.trim_end()) - .wrap_err("encryption key is not a valid base64 encoding")?; - - // old code wrote the key as a fixed length array of 32 bytes - // new code writes the key with a length prefix - if buf.len() == 32 { - Ok(*Key::from_slice(&buf)) - } else { - let mut bytes = Bytes::new(&buf); - let key_len = rmp::decode::read_bin_len(&mut bytes).map_err(error_report)?; - if key_len != 32 || bytes.remaining_slice().len() != key_len as usize { - bail!("encryption key is not the correct size") - } - Ok(*Key::from_slice(bytes.remaining_slice())) - } -} - pub fn encrypt(history: &History, key: &Key) -> Result { // serialize with msgpack let mut buf = encode(history)?; diff --git a/atuin-client/src/record/encodings/key.rs b/atuin-client/src/record/encodings/key.rs index 6a381883..7565f03a 100644 --- a/atuin-client/src/record/encodings/key.rs +++ b/atuin-client/src/record/encodings/key.rs @@ -16,10 +16,16 @@ //! //! UTF8 encoding of the key ID, using the PASERK V4 `lid` (local-id) format. +use std::io::Write; +use std::path::PathBuf; + use atuin_common::record::DecryptedData; -use eyre::{bail, Context, Result}; -use rusty_paserk::id::EncodeId; -use rusty_paseto::core::{Key, Local, PasetoSymmetricKey, V4}; +use base64::prelude::BASE64_STANDARD; +use base64::Engine; +use eyre::{bail, ensure, eyre, Context, Result}; +use rand::rngs::OsRng; +use rand::RngCore; +use rusty_paserk::{Key, KeyId, Local, V4}; use crate::record::encryption::none::UnsafeNoEncryption; use crate::record::store::Store; @@ -29,19 +35,21 @@ const KEY_VERSION: &str = "v0"; const KEY_TAG: &str = "key"; struct KeyRecord { - id: String, + id: KeyId, } impl KeyRecord { pub fn serialize(&self) -> Result { - Ok(DecryptedData(self.id.as_bytes().to_owned())) + Ok(DecryptedData(self.id.to_string().into_bytes())) } pub fn deserialize(data: &DecryptedData, version: &str) -> Result { match version { KEY_VERSION => { let lid = std::str::from_utf8(&data.0).context("key id was not utf8 encoded")?; - Ok(Self { id: lid.to_owned() }) + Ok(Self { + id: lid.parse().context("invalid key id")?, + }) } _ => { bail!("unknown version {version:?}") @@ -72,7 +80,7 @@ impl KeyStore { let host_id = Settings::host_id().expect("failed to get host_id"); // the local_id is a hashed version of the encryption key. safe to store unencrypted - let key_id = PasetoSymmetricKey::::from(Key::from(encryption_key)).encode_id(); + let key_id = Key::::from_bytes(*encryption_key).to_id(); let record = KeyRecord { id: key_id }; let bytes = record.serialize()?; @@ -107,9 +115,8 @@ impl KeyStore { store: &mut (impl Store + Send + Sync), settings: &Settings, ) -> Result { - let encryption_key: [u8; 32] = crate::encryption::load_key(settings) - .context("could not load encryption key")? - .into(); + let encryption_key: [u8; 32] = + load_key(settings).context("could not load encryption key")?; // TODO: don't load this from disk so much let host_id = Settings::host_id().expect("failed to get host_id"); @@ -128,9 +135,7 @@ impl KeyStore { // encode the current key to match the registered version let current_key_id = match decrypted.version.as_str() { - KEY_VERSION => { - PasetoSymmetricKey::::from(Key::from(encryption_key)).encode_id() - } + KEY_VERSION => Key::::from_bytes(encryption_key).to_id(), version => bail!("unknown version {version:?}"), }; @@ -150,11 +155,67 @@ pub enum EncryptionKey { /// The current key is invalid Invalid { /// the id of the key - kid: String, + kid: KeyId, /// the id of the host that registered the key host_id: String, }, Valid { - encryption_key: [u8; 32], + encryption_key: AtuinKey, }, } +pub type AtuinKey = [u8; 32]; + +pub fn new_key(settings: &Settings) -> Result { + let path = settings.key_path.as_str(); + + let mut key = [0; 32]; + OsRng.fill_bytes(&mut key); + let encoded = encode_key(&key)?; + + let mut file = fs_err::File::create(path)?; + file.write_all(encoded.as_bytes())?; + + Ok(key) +} + +// Loads the secret key, will create + save if it doesn't exist +pub fn load_key(settings: &Settings) -> Result { + let path = settings.key_path.as_str(); + + let key = if PathBuf::from(path).exists() { + let key = fs_err::read_to_string(path)?; + decode_key(key)? + } else { + new_key(settings)? + }; + + Ok(key) +} + +pub fn encode_key(key: &AtuinKey) -> Result { + let mut buf = vec![]; + rmp::encode::write_bin(&mut buf, 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 { + let buf = BASE64_STANDARD + .decode(key.trim_end()) + .wrap_err("encryption key is not a valid base64 encoding")?; + + // old code wrote the key as a fixed length array of 32 bytes + // new code writes the key with a length prefix + match <[u8; 32]>::try_from(&*buf) { + Ok(key) => Ok(key), + Err(_) => { + let mut bytes = rmp::decode::Bytes::new(&buf); + let key_len = rmp::decode::read_bin_len(&mut bytes).map_err(|err| eyre!("{err:?}"))?; + ensure!(key_len == 32, "encryption key is not the correct size"); + <[u8; 32]>::try_from(bytes.remaining_slice()) + .context("encryption key is not the correct size") + } + } +} diff --git a/atuin-client/src/record/encodings/mod.rs b/atuin-client/src/record/encodings/mod.rs index 7d2ffef5..6c578029 100644 --- a/atuin-client/src/record/encodings/mod.rs +++ b/atuin-client/src/record/encodings/mod.rs @@ -1,4 +1,4 @@ //! A collect of all well known tag+version pairs. -pub mod kv; pub mod key; +pub mod kv; diff --git a/atuin-client/src/record/encryption/none.rs b/atuin-client/src/record/encryption/none.rs index ab090345..a5bbd4f3 100644 --- a/atuin-client/src/record/encryption/none.rs +++ b/atuin-client/src/record/encryption/none.rs @@ -1,6 +1,6 @@ use std::io::Write; -use atuin_common::record::{AdditonalData, DecryptedData, EncryptedData, Encryption}; +use atuin_common::record::{AdditionalData, DecryptedData, EncryptedData, Encryption}; use base64::{engine::general_purpose, write::EncoderStringWriter, Engine}; use eyre::{ensure, Context, ContextCompat, Result}; @@ -14,14 +14,14 @@ static CEK_HEADER: &str = "k0.none."; impl Encryption for UnsafeNoEncryption { fn re_encrypt( data: EncryptedData, - _ad: AdditonalData, + _ad: AdditionalData, _old_key: &[u8; 32], _new_key: &[u8; 32], ) -> Result { Ok(data) } - fn encrypt(data: DecryptedData, _ad: AdditonalData, _key: &[u8; 32]) -> EncryptedData { + fn encrypt(data: DecryptedData, _ad: AdditionalData, _key: &[u8; 32]) -> EncryptedData { let mut token = EncoderStringWriter::from_consumer( CONTENT_HEADER.to_owned(), &general_purpose::URL_SAFE_NO_PAD, @@ -35,7 +35,7 @@ impl Encryption for UnsafeNoEncryption { } } - fn decrypt(data: EncryptedData, _ad: AdditonalData, _key: &[u8; 32]) -> Result { + fn decrypt(data: EncryptedData, _ad: AdditionalData, _key: &[u8; 32]) -> Result { ensure!( data.content_encryption_key == CEK_HEADER, "exected unencrypted data, found a content encryption key" @@ -55,7 +55,7 @@ impl Encryption for UnsafeNoEncryption { mod tests { use super::*; - const AD: AdditonalData<'static> = AdditonalData { + const AD: AdditionalData<'static> = AdditionalData { id: "foo", version: "v0", tag: "kv", diff --git a/atuin-client/src/record/mod.rs b/atuin-client/src/record/mod.rs index f67bbf8a..f5d94308 100644 --- a/atuin-client/src/record/mod.rs +++ b/atuin-client/src/record/mod.rs @@ -1,8 +1,8 @@ pub mod store; pub mod encryption { - pub mod paseto_v4; pub mod none; + pub mod paseto_v4; } pub mod encodings; diff --git a/atuin-client/src/sync.rs b/atuin-client/src/sync.rs index f62dca33..a8e7cd1c 100644 --- a/atuin-client/src/sync.rs +++ b/atuin-client/src/sync.rs @@ -6,12 +6,12 @@ use chrono::prelude::*; use eyre::Result; use atuin_common::api::AddHistoryRequest; -use xsalsa20poly1305::Key; use crate::{ api_client, database::Database, - encryption::{decrypt, encrypt, load_key}, + encryption::{decrypt, encrypt}, + record::encodings::key::{load_key, AtuinKey}, settings::Settings, }; @@ -34,12 +34,13 @@ pub fn hash_str(string: &str) -> String { // Check if remote has things we don't, and if so, download them. // Returns (num downloaded, total local) async fn sync_download( - key: &Key, + key: &AtuinKey, force: bool, client: &api_client::Client<'_>, db: &mut (impl Database + Send), ) -> Result<(i64, i64)> { debug!("starting sync download"); + let key = key.into(); let remote_status = client.status().await?; let remote_count = remote_status.count; @@ -124,12 +125,13 @@ async fn sync_download( // Check if we have things remote doesn't, and if so, upload them async fn sync_upload( - key: &Key, + key: &AtuinKey, _force: bool, client: &api_client::Client<'_>, db: &mut (impl Database + Send), ) -> Result<()> { debug!("starting sync upload"); + let key = key.into(); let remote_status = client.status().await?; let remote_deleted: HashSet = HashSet::from_iter(remote_status.deleted.clone()); diff --git a/atuin/src/command/client/account/login.rs b/atuin/src/command/client/account/login.rs index 9bfe0b40..96e747d4 100644 --- a/atuin/src/command/client/account/login.rs +++ b/atuin/src/command/client/account/login.rs @@ -6,7 +6,7 @@ use tokio::{fs::File, io::AsyncWriteExt}; use atuin_client::{ api_client, - encryption::{decode_key, encode_key, new_key, Key}, + record::encodings::key::{decode_key, encode_key, new_key}, settings::Settings, }; use atuin_common::api::LoginRequest; @@ -62,7 +62,12 @@ 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()))?, + Ok(mnemonic) if mnemonic.entropy().len() == 32 => { + encode_key(mnemonic.entropy().try_into().unwrap())? + } + Ok(_) => { + bail!("key was not the correct length") + } Err(err) => { if let Some(err) = err.downcast_ref::() { match err { diff --git a/atuin/src/command/client/account/register.rs b/atuin/src/command/client/account/register.rs index d306a141..834b58df 100644 --- a/atuin/src/command/client/account/register.rs +++ b/atuin/src/command/client/account/register.rs @@ -47,7 +47,7 @@ pub async fn run( file.write_all(session.session.as_bytes()).await?; // Create a new key, and save it to disk - let _key = atuin_client::encryption::new_key(settings)?; + let _key = atuin_client::record::encodings::key::new_key(settings)?; Ok(()) } diff --git a/atuin/src/command/client/sync.rs b/atuin/src/command/client/sync.rs index 061b7376..5bbcf71f 100644 --- a/atuin/src/command/client/sync.rs +++ b/atuin/src/command/client/sync.rs @@ -45,7 +45,7 @@ impl Cmd { Self::Register(r) => r.run(&settings).await, Self::Status => status::run(&settings, db).await, Self::Key { base64 } => { - use atuin_client::encryption::{encode_key, load_key}; + use atuin_client::record::encodings::key::{encode_key, load_key}; let key = load_key(&settings).wrap_err("could not load encryption key")?; if base64 {