blobs: attempt to verify on decode when possible

regular chunks are only decoded when their contents are accessed, in
which case we need to have the key anyway and want to verify the digest.

for blobs we need to verify beforehand, since their checksums are always
calculated based on their raw content, and stored in the manifest.

manifests are also stored as blobs, but don't have a digest in the
traditional sense (they might have a signature covering parts of their
contents, but that is verified already when loading the manifest).

this commit does not cover pull/sync code which copies blobs and chunks
as-is without decoding them.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>

diff --git a/src/backup/data_blob.rs b/src/backup/data_blob.rs
index af9ebf8acd08813c96e596c705e8e52ec11b1ad3..59336b802bd237d57d1b8cec306fe8521fb12f4a 100644 (file)
--- a/src/backup/data_blob.rs
+++ b/src/backup/data_blob.rs
@@ -185,16 +185,23 @@ impl DataBlob {
     }
 
     /// Decode blob data
-    pub fn decode(&self, config: Option<&CryptConfig>) -> Result<Vec<u8>, Error> {
+    pub fn decode(&self, config: Option<&CryptConfig>, digest: Option<&[u8; 32]>) -> Result<Vec<u8>, Error> {
 
         let magic = self.magic();
 
         if magic == &UNCOMPRESSED_BLOB_MAGIC_1_0 {
             let data_start = std::mem::size_of::<DataBlobHeader>();
-            Ok(self.raw_data[data_start..].to_vec())
+            let data = self.raw_data[data_start..].to_vec();
+            if let Some(digest) = digest {
+                Self::verify_digest(&data, None, digest)?;
+            }
+            Ok(data)
         } else if magic == &COMPRESSED_BLOB_MAGIC_1_0 {
             let data_start = std::mem::size_of::<DataBlobHeader>();
             let data = zstd::block::decompress(&self.raw_data[data_start..], MAX_BLOB_SIZE)?;
+            if let Some(digest) = digest {
+                Self::verify_digest(&data, None, digest)?;
+            }
             Ok(data)
         } else if magic == &ENCR_COMPR_BLOB_MAGIC_1_0 || magic == &ENCRYPTED_BLOB_MAGIC_1_0 {
             let header_len = std::mem::size_of::<EncryptedDataBlobHeader>();
@@ -208,6 +215,9 @@ impl DataBlob {
                 } else {
                     config.decode_uncompressed_chunk(&self.raw_data[header_len..], &head.iv, &head.tag)?
                 };
+                if let Some(digest) = digest {
+                    Self::verify_digest(&data, Some(config), digest)?;
+                }
                 Ok(data)
             } else {
                 bail!("unable to decrypt blob - missing CryptConfig");
@@ -276,12 +286,26 @@ impl DataBlob {
             return Ok(());
         }
 
-        let data = self.decode(None)?;
+        // verifies digest!
+        let data = self.decode(None, Some(expected_digest))?;
 
         if expected_chunk_size != data.len() {
             bail!("detected chunk with wrong length ({} != {})", expected_chunk_size, data.len());
         }
-        let digest = openssl::sha::sha256(&data);
+
+        Ok(())
+    }
+
+    fn verify_digest(
+        data: &[u8],
+        config: Option<&CryptConfig>,
+        expected_digest: &[u8; 32],
+    ) -> Result<(), Error> {
+
+        let digest = match config {
+            Some(config) => config.compute_digest(data),
+            None => openssl::sha::sha256(&data),
+        };
         if &digest != expected_digest {
             bail!("detected chunk with wrong digest.");
         }

diff --git a/src/backup/datastore.rs b/src/backup/datastore.rs
index 8d882d298af818e44ed08c98ce06367d898977d7..a8bb282bd767ab9790eee76c6a49df48104eecac 100644 (file)
--- a/src/backup/datastore.rs
+++ b/src/backup/datastore.rs
@@ -591,7 +591,8 @@ impl DataStore {
         backup_dir: &BackupDir,
     ) -> Result<Value, Error> {
         let blob = self.load_blob(backup_dir, MANIFEST_BLOB_NAME)?;
-        let manifest_data = blob.decode(None)?;
+        // no expected digest available
+        let manifest_data = blob.decode(None, None)?;
         let manifest: Value = serde_json::from_slice(&manifest_data[..])?;
         Ok(manifest)
     }

diff --git a/src/backup/manifest.rs b/src/backup/manifest.rs
index 44df61588ddb040f3ab50c35f4bea01214200298..a42cdeb70865d941c5af25754d4e9156022060ea 100644 (file)
--- a/src/backup/manifest.rs
+++ b/src/backup/manifest.rs
@@ -238,7 +238,8 @@ impl TryFrom<super::DataBlob> for BackupManifest {
     type Error = Error;
 
     fn try_from(blob: super::DataBlob) -> Result<Self, Error> {
-        let data = blob.decode(None)
+        // no expected digest available
+        let data = blob.decode(None, None)
             .map_err(|err| format_err!("decode backup manifest blob failed - {}", err))?;
         let json: Value = serde_json::from_slice(&data[..])
             .map_err(|err| format_err!("unable to parse backup manifest json - {}", err))?;

diff --git a/src/backup/read_chunk.rs b/src/backup/read_chunk.rs
index fb8296fd60b0c5220058414979a586af0b65924c..200f53ea6ce395d421025c80434832ed537b1f37 100644 (file)
--- a/src/backup/read_chunk.rs
+++ b/src/backup/read_chunk.rs
@@ -40,9 +40,7 @@ impl ReadChunk for LocalChunkReader {
     fn read_chunk(&self, digest: &[u8; 32]) -> Result<Vec<u8>, Error> {
         let chunk = ReadChunk::read_raw_chunk(self, digest)?;
 
-        let raw_data = chunk.decode(self.crypt_config.as_ref().map(Arc::as_ref))?;
-
-        // fixme: verify digest?
+        let raw_data = chunk.decode(self.crypt_config.as_ref().map(Arc::as_ref), Some(digest))?;
 
         Ok(raw_data)
     }
@@ -85,7 +83,7 @@ impl AsyncReadChunk for LocalChunkReader {
         Box::pin(async move {
             let chunk = AsyncReadChunk::read_raw_chunk(self, digest).await?;
 
-            let raw_data = chunk.decode(self.crypt_config.as_ref().map(Arc::as_ref))?;
+            let raw_data = chunk.decode(self.crypt_config.as_ref().map(Arc::as_ref), Some(digest))?;
 
             // fixme: verify digest?
 

diff --git a/src/backup/verify.rs b/src/backup/verify.rs
index 0617fbf6cbba7285dbae78a9dd2e3bf9738d5de4..ec47534c12cbd1c2bfc8935b3821a0b5dd9997fe 100644 (file)
--- a/src/backup/verify.rs
+++ b/src/backup/verify.rs
@@ -6,7 +6,7 @@ use crate::server::WorkerTask;
 
 use super::{
     DataStore, BackupGroup, BackupDir, BackupInfo, IndexFile,
-    ENCR_COMPR_BLOB_MAGIC_1_0, ENCRYPTED_BLOB_MAGIC_1_0,
+    CryptMode,
     FileInfo, ArchiveType, archive_type,
 };
 
@@ -24,15 +24,15 @@ fn verify_blob(datastore: &DataStore, backup_dir: &BackupDir, info: &FileInfo) -
         bail!("wrong index checksum");
     }
 
-    let magic = blob.magic();
-
-    if magic == &ENCR_COMPR_BLOB_MAGIC_1_0 || magic == &ENCRYPTED_BLOB_MAGIC_1_0 {
-        return Ok(());
+    match blob.crypt_mode()? {
+        CryptMode::Encrypt => Ok(()),
+        CryptMode::None => {
+            // digest already verified above
+            blob.decode(None, None)?;
+            Ok(())
+        },
+        CryptMode::SignOnly => bail!("Invalid CryptMode for blob"),
     }
-
-    blob.decode(None)?;
-
-    Ok(())
 }
 
 fn verify_index_chunks(

diff --git a/src/client/backup_reader.rs b/src/client/backup_reader.rs
index c60b6524bc38896637b5816555a309882b36c72f..d41857165b3b865bf857a8b5e8418949cf527750 100644 (file)
--- a/src/client/backup_reader.rs
+++ b/src/client/backup_reader.rs
@@ -130,7 +130,8 @@ impl BackupReader {
         let mut raw_data = Vec::with_capacity(64 * 1024);
         self.download(MANIFEST_BLOB_NAME, &mut raw_data).await?;
         let blob = DataBlob::load_from_reader(&mut &raw_data[..])?;
-        let data = blob.decode(None)?;
+        // no expected digest available
+        let data = blob.decode(None, None)?;
 
         let manifest = BackupManifest::from_data(&data[..], self.crypt_config.as_ref().map(Arc::as_ref))?;
 

diff --git a/src/client/backup_writer.rs b/src/client/backup_writer.rs
index 0b0ef93bb29da21413c57601eb3fe43ce7d2b82b..38686f67bac190cd2dbc8242c38b436ad1994641 100644 (file)
--- a/src/client/backup_writer.rs
+++ b/src/client/backup_writer.rs
@@ -480,7 +480,8 @@ impl BackupWriter {
         self.h2.download("previous", Some(param), &mut raw_data).await?;
 
         let blob = DataBlob::load_from_reader(&mut &raw_data[..])?;
-        let data = blob.decode(self.crypt_config.as_ref().map(Arc::as_ref))?;
+        // no expected digest available
+        let data = blob.decode(self.crypt_config.as_ref().map(Arc::as_ref), None)?;
 
         let manifest = BackupManifest::from_data(&data[..], self.crypt_config.as_ref().map(Arc::as_ref))?;
 

diff --git a/src/client/remote_chunk_reader.rs b/src/client/remote_chunk_reader.rs
index bf195d66f02fc41e73a34b9d2b6b8edb45406b15..4db11477fa79787742a9022db83a000bd15a8e58 100644 (file)
--- a/src/client/remote_chunk_reader.rs
+++ b/src/client/remote_chunk_reader.rs
@@ -62,9 +62,7 @@ impl ReadChunk for RemoteChunkReader {
 
         let chunk = ReadChunk::read_raw_chunk(self, digest)?;
 
-        let raw_data = chunk.decode(self.crypt_config.as_ref().map(Arc::as_ref))?;
-
-        // fixme: verify digest?
+        let raw_data = chunk.decode(self.crypt_config.as_ref().map(Arc::as_ref), Some(digest))?;
 
         let use_cache = self.cache_hint.contains_key(digest);
         if use_cache {
@@ -94,9 +92,7 @@ impl AsyncReadChunk for RemoteChunkReader {
 
             let chunk = Self::read_raw_chunk(self, digest).await?;
 
-            let raw_data = chunk.decode(self.crypt_config.as_ref().map(Arc::as_ref))?;
-
-            // fixme: verify digest?
+            let raw_data = chunk.decode(self.crypt_config.as_ref().map(Arc::as_ref), Some(digest))?;
 
             let use_cache = self.cache_hint.contains_key(digest);
             if use_cache {

diff --git a/tests/blob_writer.rs b/tests/blob_writer.rs
index 3d17ebd6f0c032d6f4105d60203a5662e26b7718..7ea25bb8065e4f544be074f94180c78357ffa14f 100644 (file)
--- a/tests/blob_writer.rs
+++ b/tests/blob_writer.rs
@@ -21,9 +21,13 @@ lazy_static! {
         let key = [1u8; 32];
         Arc::new(CryptConfig::new(key).unwrap())
     };
+
+    static ref TEST_DIGEST_PLAIN: [u8; 32] = [83, 154, 96, 195, 167, 204, 38, 142, 204, 224, 130, 201, 24, 71, 2, 188, 130, 155, 177, 6, 162, 100, 61, 238, 38, 219, 63, 240, 191, 132, 87, 238];
+
+    static ref TEST_DIGEST_ENC: [u8; 32] = [50, 162, 191, 93, 255, 132, 9, 14, 127, 23, 92, 39, 246, 102, 245, 204, 130, 104, 4, 106, 182, 239, 218, 14, 80, 17, 150, 188, 239, 253, 198, 117];
 }
 
-fn verify_test_blob(mut cursor: Cursor<Vec<u8>>) -> Result<(), Error> {
+fn verify_test_blob(mut cursor: Cursor<Vec<u8>>, digest: &[u8; 32]) -> Result<(), Error> {
 
     // run read tests with different buffer sizes
     for size in [1, 3, 64*1024].iter() {
@@ -52,7 +56,7 @@ fn verify_test_blob(mut cursor: Cursor<Vec<u8>>) -> Result<(), Error> {
 
     let blob = DataBlob::load_from_reader(&mut &raw_data[..])?;
 
-    let data = blob.decode(Some(&CRYPT_CONFIG))?;
+    let data = blob.decode(Some(&CRYPT_CONFIG), Some(digest))?;
     if data != *TEST_DATA {
         bail!("blob data is wrong (decode)");
     }
@@ -65,7 +69,7 @@ fn test_uncompressed_blob_writer() -> Result<(), Error> {
     let mut blob_writer = DataBlobWriter::new_uncompressed(tmp)?;
     blob_writer.write_all(&TEST_DATA)?;
 
-    verify_test_blob(blob_writer.finish()?)
+    verify_test_blob(blob_writer.finish()?, &*TEST_DIGEST_PLAIN)
 }
 
 #[test]
@@ -74,7 +78,7 @@ fn test_compressed_blob_writer() -> Result<(), Error> {
     let mut blob_writer = DataBlobWriter::new_compressed(tmp)?;
     blob_writer.write_all(&TEST_DATA)?;
 
-    verify_test_blob(blob_writer.finish()?)
+    verify_test_blob(blob_writer.finish()?, &*TEST_DIGEST_PLAIN)
 }
 
 #[test]
@@ -83,7 +87,7 @@ fn test_encrypted_blob_writer() -> Result<(), Error> {
     let mut blob_writer = DataBlobWriter::new_encrypted(tmp, CRYPT_CONFIG.clone())?;
     blob_writer.write_all(&TEST_DATA)?;
 
-    verify_test_blob(blob_writer.finish()?)
+    verify_test_blob(blob_writer.finish()?, &*TEST_DIGEST_ENC)
 }
 
 #[test]
@@ -92,5 +96,5 @@ fn test_encrypted_compressed_blob_writer() -> Result<(), Error> {
     let mut blob_writer = DataBlobWriter::new_encrypted_compressed(tmp, CRYPT_CONFIG.clone())?;
     blob_writer.write_all(&TEST_DATA)?;
 
-    verify_test_blob(blob_writer.finish()?)
+    verify_test_blob(blob_writer.finish()?, &*TEST_DIGEST_ENC)
 }