Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 14 additions & 5 deletions src/util/key_obfuscator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ use crate::crypto::chacha20poly1305::ChaCha20Poly1305;
///
/// It provides client-side deterministic encryption of given keys using ChaCha20-Poly1305.
pub struct KeyObfuscator {
obfuscation_key: [u8; 32],
hashing_key: [u8; 32],
obfuscation_key: [u8; KEY_LENGTH],
hashing_key: [u8; KEY_LENGTH],
}

impl KeyObfuscator {
Expand All @@ -24,6 +24,7 @@ impl KeyObfuscator {
}
}

const KEY_LENGTH: usize = 32;
const TAG_LENGTH: usize = 16;
const NONCE_LENGTH: usize = 12;

Expand Down Expand Up @@ -137,20 +138,28 @@ impl KeyObfuscator {

/// Derives the obfuscation and hashing keys from the master key.
fn derive_obfuscation_and_hashing_keys(
obfuscation_master_key: &[u8; 32],
) -> ([u8; 32], [u8; 32]) {
obfuscation_master_key: &[u8; KEY_LENGTH],
) -> ([u8; KEY_LENGTH], [u8; KEY_LENGTH]) {
let prk = Self::hkdf(obfuscation_master_key, "pseudo_random_key".as_bytes());
let k1 = Self::hkdf(&prk, "obfuscation_key".as_bytes());
let k2 = Self::hkdf(&prk, &[&k1[..], "hashing_key".as_bytes()].concat());
(k1, k2)
}
fn hkdf(initial_key_material: &[u8], salt: &[u8]) -> [u8; 32] {
fn hkdf(initial_key_material: &[u8], salt: &[u8]) -> [u8; KEY_LENGTH] {
let mut engine = HmacEngine::<sha256::Hash>::new(salt);
engine.input(initial_key_material);
Hmac::from_engine(engine).to_byte_array()
}
}

impl Drop for KeyObfuscator {
fn drop(&mut self) {
// Zeroize the owned keys
self.obfuscation_key.copy_from_slice(&[0u8; KEY_LENGTH]);
self.hashing_key.copy_from_slice(&[0u8; KEY_LENGTH]);
}
}

#[cfg(test)]
mod tests {
use crate::util::key_obfuscator::KeyObfuscator;
Expand Down
63 changes: 48 additions & 15 deletions src/util/storable_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,16 @@ use std::io;
use std::io::{Error, ErrorKind};

/// [`StorableBuilder`] is a utility to build and deconstruct [`Storable`] objects.
///
/// It provides client-side Encrypt-then-MAC using ChaCha20-Poly1305.
pub struct StorableBuilder<T: EntropySource> {
data_encryption_key: [u8; 32],
entropy_source: T,
}

impl<T: EntropySource> StorableBuilder<T> {
/// Constructs a new instance.
pub fn new(data_encryption_key: [u8; 32], entropy_source: T) -> StorableBuilder<T> {
Self { data_encryption_key, entropy_source }
pub fn new(entropy_source: T) -> StorableBuilder<T> {
Self { entropy_source }
}
}

Expand All @@ -34,18 +34,21 @@ const CHACHA20_CIPHER_NAME: &'static str = "ChaCha20Poly1305";
impl<T: EntropySource> StorableBuilder<T> {
/// Creates a [`Storable`] that can be serialized and stored as `value` in [`PutObjectRequest`].
///
/// Uses ChaCha20 for encrypting `input` and Poly1305 for generating a mac/tag.
/// Uses ChaCha20 for encrypting `input` and Poly1305 for generating a mac/tag with associated
/// data `aad` (usually the storage key).
///
/// Refer to docs on [`Storable`] for more information.
///
/// [`PutObjectRequest`]: crate::types::PutObjectRequest
pub fn build(&self, input: Vec<u8>, version: i64) -> Storable {
pub fn build(
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i wonder if we could be opinionated here for future writes and have the AAD commit to:

  1. The key-value key (lol) [fixes the issue mentioned in pr]
  2. store_id [fixes potential multi-tenancy issues]

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re: 2.:

  1. The key-value key (lol) [fixes the issue mentioned in pr]

Hmm, that might be an option, but (especially for backwards compat) the current approach where the 'user' determines the aad (given that they also build the actual storable) might be more flexible?

  1. store_id [fixes potential multi-tenancy issues]

I guess we also could consider committing to the store id, but it would mean it can never change, e.g., that services can't ever move data between stores for the user.

Also, as noted below, I doubt the described replay attacks can be fully mitigated.

&self, input: Vec<u8>, version: i64, data_encryption_key: &[u8; 32], aad: &[u8],
) -> Storable {
let mut nonce = vec![0u8; 12];
self.entropy_source.fill_bytes(&mut nonce[4..]);

let mut data_blob = PlaintextBlob { value: input, version }.encode_to_vec();

let mut cipher = ChaCha20Poly1305::new(&self.data_encryption_key, &nonce, &[]);
let mut cipher = ChaCha20Poly1305::new(data_encryption_key, &nonce, aad);
let mut tag = vec![0u8; 16];
cipher.encrypt_inplace(&mut data_blob, &mut tag);
Storable {
Expand All @@ -62,10 +65,14 @@ impl<T: EntropySource> StorableBuilder<T> {
/// corresponding version as stored at the time of [`PutObjectRequest`].
///
/// [`PutObjectRequest`]: crate::types::PutObjectRequest
pub fn deconstruct(&self, mut storable: Storable) -> io::Result<(Vec<u8>, i64)> {
let encryption_metadata = storable.encryption_metadata.unwrap();
pub fn deconstruct(
&self, mut storable: Storable, data_encryption_key: &[u8; 32], aad: &[u8],
) -> io::Result<(Vec<u8>, i64)> {
let encryption_metadata = storable
.encryption_metadata
.ok_or_else(|| Error::new(ErrorKind::InvalidData, "Invalid Metadata"))?;
let mut cipher =
ChaCha20Poly1305::new(&self.data_encryption_key, &encryption_metadata.nonce, &[]);
ChaCha20Poly1305::new(data_encryption_key, &encryption_metadata.nonce, aad);

cipher
.decrypt_inplace(&mut storable.data, encryption_metadata.tag.borrow())
Expand Down Expand Up @@ -97,16 +104,42 @@ mod tests {
let test_entropy_provider = TestEntropyProvider;
let mut data_key = [0u8; 32];
test_entropy_provider.fill_bytes(&mut data_key);
let storable_builder = StorableBuilder {
data_encryption_key: data_key,
entropy_source: test_entropy_provider,
};
let storable_builder = StorableBuilder::new(test_entropy_provider);
let expected_data = b"secret".to_vec();
let expected_version = 8;
let storable = storable_builder.build(expected_data.clone(), expected_version);
let aad = b"A";
let storable =
storable_builder.build(expected_data.clone(), expected_version, &data_key, aad);

let (actual_data, actual_version) = storable_builder.deconstruct(storable).unwrap();
let (actual_data, actual_version) =
storable_builder.deconstruct(storable, &data_key, aad).unwrap();
assert_eq!(actual_data, expected_data);
assert_eq!(actual_version, expected_version);
}

#[test]
fn decrypt_key_mismatch_fails() {
let test_entropy_provider = TestEntropyProvider;
let mut data_key = [0u8; 32];
test_entropy_provider.fill_bytes(&mut data_key);
let storable_builder = StorableBuilder::new(test_entropy_provider);

let expected_data_a = b"secret_a".to_vec();
let expected_version_a = 8;
let aad_a = b"A";
let storable_a =
storable_builder.build(expected_data_a.clone(), expected_version_a, &data_key, aad_a);

let expected_data_b = b"secret_b".to_vec();
let expected_version_b = 8;
let aad_b = b"B";
let storable_b =
storable_builder.build(expected_data_b.clone(), expected_version_b, &data_key, aad_b);

let (actual_data, actual_version) =
storable_builder.deconstruct(storable_a, &data_key, aad_a).unwrap();
assert_eq!(actual_data, expected_data_a);
assert_eq!(actual_version, expected_version_a);
assert!(storable_builder.deconstruct(storable_b, &data_key, aad_a).is_err());
}
}