add an app_id to the encryption AAD

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
 

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<Envelope<'a>> {
-        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 = buf[0];
-        let version = rdr.read_u32::<NetworkEndian>().unwrap();
+        if version != ENVELOPE_VERSION {
-        if version != 1 {
+            anyhow::bail!("unrecognized encryption envelope version {}", version);
-            anyhow::bail!("unrecognized envelope version {}", version);
         }
 
         Ok(Envelope {
-            nonce: &buf[4..4 + aead::NONCE_LEN],
+            nonce: &buf[1..1 + aead::NONCE_LEN],
-            payload: &buf[4 + aead::NONCE_LEN..],
+            payload: &buf[1 + aead::NONCE_LEN..],
         })
     }
 
     fn to_bytes(&self) -> Vec<u8> {
-        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::<NetworkEndian>(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();