diff --git a/docs/src/sync-protocol.md b/docs/src/sync-protocol.md index 4d86f0af6..0bb588383 100644 --- a/docs/src/sync-protocol.md +++ b/docs/src/sync-protocol.md @@ -53,21 +53,29 @@ The salt is the SHA256 hash of the 16-byte form of the client key. #### Encryption The client uses [AEAD](https://commondatastorage.googleapis.com/chromium-boringssl-docs/aead.h.html), with algorithm CHACHA20_POLY1305. -Each encrypted payload has an associated version ID. -The 16-byte form of this UUID is used as the associated data (AAD) with the AEAD algorithm. The client should generate a random nonce, noting that AEAD is _not secure_ if a nonce is used repeatedly for the same key. +AEAD supports additional authenticated data (AAD) which must be provided for both open and seal operations. +In this protocol, the AAD is always 17 bytes of the form: + * `app_id` (byte) - always 1 + * `version_id` (16 bytes) - 16-byte form of the version ID associated with this data + * for versions (AddVersion, GetChildVersion), the _parent_ version_id + * for snapshots (AddSnapshot, GetSnapshot), the snapshot version_id + +The `app_id` field is for future expansion to handle other, non-task data using this protocol. +Including it in the AAD ensures that such data cannot be confused with task data. + Although the AEAD specification distinguishes ciphertext and tags, for purposes of this specification they are considered concatenated into a single bytestring as in BoringSSL's `EVP_AEAD_CTX_seal`. #### Representation -The final byte-stream is comprised of the following structure, with integers represented in network-endian format. +The final byte-stream is comprised of the following structure: -* `version` (32-bit int) - format version (always 1) +* `version` (byte) - format version (always 1) * `nonce` (12 bytes) - encryption nonce * `ciphertext` (remaining bytes) - ciphertext from sealing operation -Future versions may have a completely different format. +The `version` field identifies this data format, and future formats will have a value other than 1 in this position. ### Version diff --git a/taskchampion/src/server/crypto.rs b/taskchampion/src/server/crypto.rs index fe47f65d5..b2728acad 100644 --- a/taskchampion/src/server/crypto.rs +++ b/taskchampion/src/server/crypto.rs @@ -1,12 +1,13 @@ /// This module implements the encryption specified in the sync-protocol /// document. -use byteorder::{NetworkEndian, ReadBytesExt, WriteBytesExt}; use ring::{aead, digest, pbkdf2, rand, rand::SecureRandom}; -use std::io::{Cursor, Read}; +use std::io::Read; use uuid::Uuid; const PBKDF2_ITERATIONS: u32 = 100000; -const ENVELOPE_VERSION: u32 = 1; +const ENVELOPE_VERSION: u8 = 1; +const AAD_LEN: usize = 17; +const TASK_APP_ID: u8 = 1; /// An Cryptor stores a secret and allows sealing and unsealing. It derives a key from the secret, /// which takes a nontrivial amount of time, so it should be created once and re-used for the given @@ -55,7 +56,7 @@ impl Cryptor { .map_err(|_| anyhow::anyhow!("error generating random nonce"))?; let nonce = aead::Nonce::assume_unique_for_key(nonce_buf); - let aad = aead::Aad::from(version_id.as_bytes()); + let aad = self.make_aad(version_id); let tag = self .key @@ -86,7 +87,7 @@ impl Cryptor { let mut nonce = [0u8; aead::NONCE_LEN]; nonce.copy_from_slice(env.nonce); let nonce = aead::Nonce::assume_unique_for_key(nonce); - let aad = aead::Aad::from(version_id.as_bytes()); + let aad = self.make_aad(version_id); let mut payload = env.payload.to_vec(); let plaintext = self @@ -99,6 +100,13 @@ impl Cryptor { payload: plaintext.to_vec(), }) } + + fn make_aad(&self, version_id: Uuid) -> aead::Aad<[u8; AAD_LEN]> { + let mut aad = [0u8; AAD_LEN]; + aad[0] = TASK_APP_ID; + aad[1..].copy_from_slice(version_id.as_bytes()); + aead::Aad::from(aad) + } } /// Secret represents a secret key as used for encryption and decryption. @@ -126,26 +134,25 @@ struct Envelope<'a> { impl<'a> Envelope<'a> { fn from_bytes(buf: &'a [u8]) -> anyhow::Result> { - if buf.len() <= 4 + aead::NONCE_LEN { + if buf.len() <= 1 + aead::NONCE_LEN { anyhow::bail!("envelope is too small"); } - let mut rdr = Cursor::new(buf); - let version = rdr.read_u32::().unwrap(); - if version != 1 { - anyhow::bail!("unrecognized envelope version {}", version); + let version = buf[0]; + if version != ENVELOPE_VERSION { + anyhow::bail!("unrecognized encryption envelope version {}", version); } Ok(Envelope { - nonce: &buf[4..4 + aead::NONCE_LEN], - payload: &buf[4 + aead::NONCE_LEN..], + nonce: &buf[1..1 + aead::NONCE_LEN], + payload: &buf[1 + aead::NONCE_LEN..], }) } fn to_bytes(&self) -> Vec { - let mut buf = Vec::with_capacity(4 + self.nonce.len() + self.payload.len()); + let mut buf = Vec::with_capacity(1 + self.nonce.len() + self.payload.len()); - buf.write_u32::(ENVELOPE_VERSION).unwrap(); + buf.push(ENVELOPE_VERSION); buf.extend_from_slice(self.nonce); buf.extend_from_slice(self.payload); buf @@ -210,6 +217,30 @@ mod test { assert_eq!(env, env2); } + #[test] + fn envelope_bad_version() { + let env = Envelope { + nonce: &[2; 12], + payload: b"HELLO", + }; + + let mut bytes = env.to_bytes(); + bytes[0] = 99; + assert!(Envelope::from_bytes(&bytes).is_err()); + } + + #[test] + fn envelope_too_short() { + let env = Envelope { + nonce: &[2; 12], + payload: b"HELLO", + }; + + let bytes = env.to_bytes(); + let bytes = &bytes[..10]; + assert!(Envelope::from_bytes(bytes).is_err()); + } + #[test] fn round_trip() { let version_id = Uuid::new_v4();