refactor(keystore): rework item-level encryption traits

The existing `trait EntityEncryptionExt` was confusing: it depended
on mutating the internal state, and never calling the appropriate
methods too many or too few times. It wasn't obvious when or whether
it was ever appropriate for an implementing item to override one of
the default trait methods.

In fairness, it's a complicated problem.

This commit contains my approach, which breaks it up substantially.
Entities no longer implement `Serialize` or `Deserialize`, and in fact
must not implement either of those. (If it were possible, I'd have
added negative trait bounds to that effect.) Doing this with separate
types gives us some static type safety, which is always nice!

Instead, entities which want to serialize must implement `Encrypting`,
which requires them to copy all non-sensitive data and encrypt all sensitive data
into an associated type. This associated type implements `Serialize`.

Likewise, entities which want to deserialize must implement `Decrypting`,
which requires them to copy all non-sensitive data and decrypt all sensitive data
from an associated type. This associated type implements `Deserialize`.

The benefit is that required methods have no defaults, and it is
impossible to improperly override default methods. There are now
helper traits `EncryptData` and `DecryptData`, which are automatically
implemented for all entities, and handle all the details of the
symmetrical encryption and decryption.

Author: coriolinus

keystore/src/lib.rs

@@ -11,20 +11,21 @@ pub(crate) mod proteus;
 mod traits;
 pub mod transaction;
 
-pub use self::connection::{ConnectionType, Database, DatabaseKey};
+#[cfg(not(target_family = "wasm"))]
+use sha2::{Digest, Sha256};
+
 #[cfg(feature = "dummy-entity")]
 pub use self::entities::{DummyStoreValue, DummyValue};
-pub use self::error::*;
-#[cfg(target_family = "wasm")]
-pub use self::traits::EntityEncryptionExt;
-pub use self::traits::{
-    Entity, EntityBase, EntityGetBorrowed, EntityTransactionDeleteBorrowed, EntityTransactionExt, FetchFromDatabase,
-    KeyType, UniqueEntity, UniqueEntityExt,
+pub use self::{
+    connection::{ConnectionType, Database, DatabaseKey},
+    error::*,
+    traits::{
+        DecryptData, Decryptable, Decrypting, EncryptData, Encrypting, Entity, EntityBase, EntityGetBorrowed,
+        EntityTransactionDeleteBorrowed, EntityTransactionExt, FetchFromDatabase, KeyType, UniqueEntity,
+        UniqueEntityExt,
+    },
 };
 
-#[cfg(not(target_family = "wasm"))]
-use sha2::{Digest, Sha256};
-
 /// Used to calculate ID hashes for some MlsEntities' SQLite tables (not used on wasm).
 /// We only use sha256 on platforms where we use SQLite.
 /// On wasm, we use IndexedDB, a key-value store, via the idb crate.

keystore/src/traits/entity.rs

@@ -1,7 +1,6 @@
 use std::borrow::Borrow;
 
 use async_trait::async_trait;
-use serde::{Serialize, de::DeserializeOwned};
 
 use crate::{CryptoKeystoreResult, EntityBase, KeyType};
 
@@ -10,7 +9,7 @@ use crate::{CryptoKeystoreResult, EntityBase, KeyType};
 /// It has a primary key, which uniquely identifies it.
 #[cfg_attr(target_family = "wasm", async_trait(?Send))]
 #[cfg_attr(not(target_family = "wasm"), async_trait)]
-pub trait Entity: EntityBase + Serialize + DeserializeOwned {
+pub trait Entity: EntityBase {
     /// Each distinct [`PrimaryKey`] uniquely identifies either 0 or 1 instance.
     ///
     /// This constraint should be enforced at the DB level.

keystore/src/traits/entity_encryption_ext.rs

@@ -0,0 +1,32 @@
+use crate::{CryptoKeystoreResult, Entity, KeyType as _};
+
+pub(super) const AES_GCM_256_NONCE_SIZE: usize = 12;
+
+#[derive(core_crypto_macros::Debug, Clone, PartialEq, Eq, serde::Serialize, serde::Deserialize)]
+pub(super) struct Aad {
+    type_name: Vec<u8>,
+    id: Vec<u8>,
+}
+
+impl<E> From<&'_ E> for Aad
+where
+    E: Entity,
+{
+    fn from(value: &E) -> Self {
+        let type_name = E::COLLECTION_NAME.as_bytes().to_vec();
+        let id = value.primary_key().bytes().into_owned();
+        Self { type_name, id }
+    }
+}
+
+impl Aad {
+    pub(super) fn serialize(&self) -> CryptoKeystoreResult<Vec<u8>> {
+        serde_json::to_vec(self).map_err(Into::into)
+    }
+
+    pub(super) fn from_primary_key<E: Entity>(primary_key: &E::PrimaryKey) -> Self {
+        let type_name = E::COLLECTION_NAME.as_bytes().to_vec();
+        let id = primary_key.bytes().into_owned();
+        Self { type_name, id }
+    }
+}

keystore/src/traits/item_encryption/aad.rs

@@ -0,0 +1,46 @@
+use super::aad::{AES_GCM_256_NONCE_SIZE, Aad};
+use crate::{CryptoKeystoreError, CryptoKeystoreResult, Entity, EntityTransactionDeleteBorrowed, KeyType as _};
+
+fn decrypt_with_nonce_and_aad(
+    cipher: &aes_gcm::Aes256Gcm,
+    msg: &[u8],
+    nonce: &[u8],
+    aad: &[u8],
+) -> CryptoKeystoreResult<Vec<u8>> {
+    use aes_gcm::aead::Aead as _;
+
+    let nonce = aes_gcm::Nonce::from_slice(nonce);
+    let payload = aes_gcm::aead::Payload { msg, aad };
+
+    let cleartext = cipher
+        .decrypt(nonce, payload)
+        .map_err(|_| CryptoKeystoreError::AesGcmError)?;
+
+    Ok(cleartext)
+}
+
+/// This trait is intended to provide a convenient way to decrypt data.
+///
+/// There is a blanket implementation covering all [`Entity`]s.
+pub trait DecryptData: Entity {
+    /// Decrypt some data, symmetrically to the process [`encrypt_data`][super::EncryptData::encrypt_data] uses.
+    fn decrypt_data(
+        cipher: &aes_gcm::Aes256Gcm,
+        primary_key: &<Self as Entity>::PrimaryKey,
+        data: &[u8],
+    ) -> CryptoKeystoreResult<Vec<u8>>;
+}
+
+impl<E: Entity> DecryptData for E {
+    fn decrypt_data(
+        cipher: &aes_gcm::Aes256Gcm,
+        primary_key: &E::PrimaryKey,
+        data: &[u8],
+    ) -> CryptoKeystoreResult<Vec<u8>> {
+        let aad = Aad::from_primary_key(primary_key).serialize()?;
+        let (nonce, msg) = data
+            .split_at_checked(AES_GCM_256_NONCE_SIZE)
+            .ok_or(CryptoKeystoreError::AesGcmError)?;
+        decrypt_with_nonce_and_aad(cipher, msg, &nonce, &aad)
+    }
+}

keystore/src/traits/item_encryption/decrypt_data.rs

@@ -0,0 +1,84 @@
+use serde::Deserialize;
+
+use crate::{CryptoKeystoreResult, Entity};
+
+/// This trait restores to a plaintext form of this struct, where all sensitive fields have been
+/// decrypted.
+///
+/// This is quite likely to be handled automatically by a macro, depending on how annoying it is to implement everywhere.
+///
+/// ## Example
+///
+/// ```rust,ignore
+/// // Foo is an Entity
+/// struct Foo {
+///     id: Vec<u8>,
+///     sensitive_data: Vec<u8>, // sensitive!
+/// }
+///
+/// #[derive(serde::Serialize)]
+/// struct EncryptedFoo<'de> {
+///     id: Vec<u8>,
+///     sensitive_data: &'de [u8],
+/// }
+///
+/// impl<'de> Decrypting<'de> for EncryptedFoo<'de> {
+///     type DecryptedForm = Foo;
+///
+///     fn decrypt(self, cipher: &aes_gcm::Aes256Gcm) -> CryptoKeystoreResult<Foo> {
+///         let id = self.id;
+///         let sensitive_data = E::decrypt_data(cipher, &id, self.sensitive_data)?;
+///         Ok(Foo {
+///             id,
+///             sensitive_data,
+///         })
+///     }
+/// }
+/// ```
+///
+/// This can then be used like:
+///
+/// ```rust,ignore
+/// let foo = serde_json::from_str::<EncryptedFoo>(json)?.decrypt(cipher)?;
+/// ```
+pub trait Decrypting<'de>: 'de + Deserialize<'de> {
+    type DecryptedForm: Entity;
+
+    fn decrypt(self, cipher: &aes_gcm::Aes256Gcm) -> CryptoKeystoreResult<Self::DecryptedForm>;
+}
+
+/// Helper trait for restoring from an encrypted form of this struct.
+///
+/// This is mainly useful so that the encrypted form does not need to be named, or even nameable.
+///
+/// This is quite likely to be handled automatically by a macro, depending on how annoying it is to implement everywhere.
+///
+/// ## Example
+///
+/// Extending the example from [`Decrypting`]:
+///
+/// ```rust,ignore
+/// // Foo is an Entity
+/// struct Foo { ... }
+///
+/// #[derive(serde::Serialize)]
+/// struct EncryptedFoo<'de> { ... }
+///
+/// impl<'de> Decrypting<'de> for EncryptedFoo<'de> {
+///     type DecryptedForm = Foo;
+///     fn decrypt(self, cipher: &aes_gcm::Aes256Gcm) -> CryptoKeystoreResult<Foo> { ... }
+/// }
+///
+/// impl<'de> Decryptable<'de> for Foo {
+///     type DecryptableFrom = EncryptedFoo<'de>;
+/// }
+/// ```
+///
+/// `EncryptedFoo` now no longer needs to appear in external code:
+///
+/// ```rust,ignore
+/// let foo = serde_json::from_str::<Foo::DecryptableFrom>(json)?.decrypt(cipher)?;
+/// ```
+pub trait Decryptable<'de>: Entity {
+    type DecryptableFrom: 'de + Decrypting<'de>;
+}

keystore/src/traits/item_encryption/decrypting.rs

@@ -0,0 +1,49 @@
+use crate::{CryptoKeystoreError, CryptoKeystoreResult, Entity};
+
+use super::aad::{AES_GCM_256_NONCE_SIZE, Aad};
+
+// About WASM Encryption:
+// The store key (i.e. passphrase) is hashed using SHA256 to obtain 32 bytes
+// The AES256-GCM cipher is then initialized and is used to encrypt individual values
+// Entities shall decide which fields need to be encrypted
+// Internal layout:
+// - Cleartext: [u8] bytes
+// - Ciphertext: [12 bytes of nonce..., ...encrypted data]
+fn encrypt_with_nonce_and_aad(
+    cipher: &aes_gcm::Aes256Gcm,
+    msg: &[u8],
+    nonce: &[u8],
+    aad: &[u8],
+) -> CryptoKeystoreResult<Vec<u8>> {
+    use aes_gcm::aead::Aead as _;
+
+    let nonce = aes_gcm::Nonce::from_slice(nonce);
+    let payload = aes_gcm::aead::Payload { msg, aad };
+
+    let mut encrypted = cipher
+        .encrypt(nonce, payload)
+        .map_err(|_| CryptoKeystoreError::AesGcmError)?;
+    let mut message = Vec::with_capacity(nonce.len() + encrypted.len());
+    message.extend_from_slice(nonce);
+    message.append(&mut encrypted);
+    Ok(message)
+}
+
+/// This trait is intended to provide a convenient way to encrypt data.
+///
+/// The encryption process embeds both a nonce and an AAD: an identity comprising
+/// both the entity's type and a unique identifier.
+///
+/// There is a blanket implementation covering all [`Entity`]s.
+pub trait EncryptData {
+    /// Encrypt some data, using a random nonce and an AAD.
+    fn encrypt_data(&self, cipher: &aes_gcm::Aes256Gcm, data: &[u8]) -> CryptoKeystoreResult<Vec<u8>>;
+}
+
+impl<E: Entity> EncryptData for E {
+    fn encrypt_data(&self, cipher: &aes_gcm::Aes256Gcm, data: &[u8]) -> CryptoKeystoreResult<Vec<u8>> {
+        let aad = Aad::from(self).serialize()?;
+        let nonce_bytes: [u8; AES_GCM_256_NONCE_SIZE] = rand::random();
+        encrypt_with_nonce_and_aad(cipher, data, &nonce_bytes, &aad)
+    }
+}

keystore/src/traits/item_encryption/encrypt_data.rs

@@ -0,0 +1,48 @@
+use serde::Serialize;
+
+use crate::CryptoKeystoreResult;
+
+/// This trait produces an encrypted form of this struct, where all sensitive fields have been
+/// encrypted using the [`EncryptData`][super::EncryptData] helper.
+///
+/// This is quite likely to be handled automatically by a macro, depending on how annoying it is to implement everywhere.
+///
+/// ## Example
+///
+/// ```rust,ignore
+/// // Foo is an Entity
+/// struct Foo {
+///     id: Vec<u8>,
+///     sensitive_data: Vec<u8>, // sensitive!
+/// }
+///
+/// #[derive(serde::Serialize)]
+/// struct EncryptedFoo<'a> {
+///     id: &'a Vec<u8>,
+///     sensitive_data: Vec<u8>,
+/// }
+///
+/// impl<'a> Encrypting<'a> for Foo {
+///     type EncryptedForm: EncryptedFoo<'a>;
+///
+///     fn encrypt(&'a self, cipher: &aes_gcm::Aes256Gcm) -> CryptoKeystoreResult<EncryptedFoo<'a>> {
+///         Ok(EncryptedFoo {
+///             id: &self.id,
+///             sensitive_data: self.encrypt_data(cipher, &self.sensitive_data)?,
+///         })
+///     }
+/// }
+/// ```
+///
+/// This can then be used like:
+///
+/// ```rust,ignore
+/// let json = serde_json::to_string(&foo.encrypt(cipher)?)?;
+/// ```
+pub trait Encrypting<'a> {
+    /// This type must be serializable, but can depend on the lifetime of `self` to reduce copying.
+    type EncryptedForm: 'a + Serialize;
+
+    /// Make an instance of the encrypted form of this struct, for which all sensitive fields have been encrypted.
+    fn encrypt(&'a self, cipher: &aes_gcm::Aes256Gcm) -> CryptoKeystoreResult<Self::EncryptedForm>;
+}