move keys outside of sync encryptionother-record-stores

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<XSalsa20Poly1305>,
 }
 
-pub fn new_key(settings: &Settings) -> Result<Key> {
-    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<Key> {
-    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<String> {
-    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<Key> {
-    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<EncryptedHistory> {
     // 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<V4, Local>,
 }
 
 impl KeyRecord {
     pub fn serialize(&self) -> Result<DecryptedData> {
-        Ok(DecryptedData(self.id.as_bytes().to_owned()))
+        Ok(DecryptedData(self.id.to_string().into_bytes()))
     }
 
     pub fn deserialize(data: &DecryptedData, version: &str) -> Result<Self> {
         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::<V4, Local>::from(Key::from(encryption_key)).encode_id();
+        let key_id = Key::<V4, Local>::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<EncryptionKey> {
-        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::<V4, Local>::from(Key::from(encryption_key)).encode_id()
-            }
+            KEY_VERSION => Key::<V4, Local>::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<V4, Local>,
         /// 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<AtuinKey> {
+    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<AtuinKey> {
+    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<String> {
+    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<AtuinKey> {
+    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<EncryptedData> {
         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<DecryptedData> {
+    fn decrypt(data: EncryptedData, _ad: AdditionalData, _key: &[u8; 32]) -> Result<DecryptedData> {
         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<String> = 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::<bip39::ErrorKind>() {
                         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 {