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>

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.");
         }

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)
     }

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))?;

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))?;
+        let raw_data = chunk.decode(self.crypt_config.as_ref().map(Arc::as_ref), Some(digest))?;
-
-        // fixme: verify 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?
 

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();
+    match blob.crypt_mode()? {
-
+        CryptMode::Encrypt => Ok(()),
-    if magic == &ENCR_COMPR_BLOB_MAGIC_1_0 || magic == &ENCRYPTED_BLOB_MAGIC_1_0 {
+        CryptMode::None => {
-        return Ok(());
+            // digest already verified above
+            blob.decode(None, None)?;
+            Ok(())
+        },
+        CryptMode::SignOnly => bail!("Invalid CryptMode for blob"),
     }
-
-    blob.decode(None)?;
-
-    Ok(())
 }
 
 fn verify_index_chunks(

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))?;
 

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))?;
 

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))?;
+        let raw_data = chunk.decode(self.crypt_config.as_ref().map(Arc::as_ref), Some(digest))?;
-
-        // fixme: verify 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))?;
+            let raw_data = chunk.decode(self.crypt_config.as_ref().map(Arc::as_ref), Some(digest))?;
-
-            // fixme: verify digest?
 
             let use_cache = self.cache_hint.contains_key(digest);
             if use_cache {

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)
 }