[pbs-devel] [PATCH proxmox-backup 3/5] chunk readers: ensure chunk/index CryptMode matches

["Fabian Grünbichler" <f.gruenbichler@proxmox.com>]

an encrypted Index should never reference a plain-text chunk, and an
unencrypted Index should never reference an encrypted chunk.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
 src/api2/admin/datastore.rs              |  8 +++---
 src/backup/manifest.rs                   | 14 +++++++++++
 src/backup/read_chunk.rs                 | 32 ++++++++++++++++++++----
 src/bin/proxmox-backup-client.rs         | 14 ++++++++---
 src/bin/proxmox_backup_client/catalog.rs | 12 ++++++---
 src/bin/proxmox_backup_client/mount.rs   |  4 ++-
 src/client/pull.rs                       |  4 +--
 src/client/remote_chunk_reader.rs        | 22 +++++++++++++---
 8 files changed, 88 insertions(+), 22 deletions(-)

diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
index 462b8d9c..81ca02eb 100644
--- a/src/api2/admin/datastore.rs
+++ b/src/api2/admin/datastore.rs
@@ -973,7 +973,7 @@ fn download_file_decoded(
                 let (csum, size) = index.compute_csum();
                 manifest.verify_file(&file_name, &csum, size)?;
 
-                let chunk_reader = LocalChunkReader::new(datastore, None);
+                let chunk_reader = LocalChunkReader::new(datastore, None, CryptMode::None);
                 let reader = AsyncIndexReader::new(index, chunk_reader);
                 Body::wrap_stream(AsyncReaderStream::new(reader)
                     .map_err(move |err| {
@@ -988,7 +988,7 @@ fn download_file_decoded(
                 let (csum, size) = index.compute_csum();
                 manifest.verify_file(&file_name, &csum, size)?;
 
-                let chunk_reader = LocalChunkReader::new(datastore, None);
+                let chunk_reader = LocalChunkReader::new(datastore, None, CryptMode::None);
                 let reader = AsyncIndexReader::new(index, chunk_reader);
                 Body::wrap_stream(AsyncReaderStream::with_buffer_size(reader, 4*1024*1024)
                     .map_err(move |err| {
@@ -1159,7 +1159,7 @@ fn catalog(
     let (csum, size) = index.compute_csum();
     manifest.verify_file(&file_name, &csum, size)?;
 
-    let chunk_reader = LocalChunkReader::new(datastore, None);
+    let chunk_reader = LocalChunkReader::new(datastore, None, CryptMode::None);
     let reader = BufferedDynamicReader::new(index, chunk_reader);
 
     let mut catalog_reader = CatalogReader::new(reader);
@@ -1282,7 +1282,7 @@ fn pxar_file_download(
         let (csum, size) = index.compute_csum();
         manifest.verify_file(&pxar_name, &csum, size)?;
 
-        let chunk_reader = LocalChunkReader::new(datastore, None);
+        let chunk_reader = LocalChunkReader::new(datastore, None, CryptMode::None);
         let reader = BufferedDynamicReader::new(index, chunk_reader);
         let archive_size = reader.archive_size();
         let reader = LocalDynamicReadAt::new(reader);
diff --git a/src/backup/manifest.rs b/src/backup/manifest.rs
index a42cdeb7..6af110d1 100644
--- a/src/backup/manifest.rs
+++ b/src/backup/manifest.rs
@@ -49,6 +49,20 @@ pub struct FileInfo {
     pub csum: [u8; 32],
 }
 
+impl FileInfo {
+
+    /// Return expected CryptMode of referenced chunks
+    ///
+    /// Encrypted Indices should only reference encrypted chunks, while signed or plain indices
+    /// should only reference plain chunks.
+    pub fn chunk_crypt_mode (&self) -> CryptMode {
+        match self.crypt_mode {
+            CryptMode::Encrypt => CryptMode::Encrypt,
+            CryptMode::SignOnly | CryptMode::None => CryptMode::None,
+        }
+    }
+}
+
 #[derive(Serialize, Deserialize)]
 #[serde(rename_all="kebab-case")]
 pub struct BackupManifest {
diff --git a/src/backup/read_chunk.rs b/src/backup/read_chunk.rs
index 200f53ea..078779de 100644
--- a/src/backup/read_chunk.rs
+++ b/src/backup/read_chunk.rs
@@ -2,9 +2,9 @@ use std::future::Future;
 use std::pin::Pin;
 use std::sync::Arc;
 
-use anyhow::Error;
+use anyhow::{bail, Error};
 
-use super::crypt_config::CryptConfig;
+use super::crypt_config::{CryptConfig, CryptMode};
 use super::data_blob::DataBlob;
 use super::datastore::DataStore;
 
@@ -21,20 +21,41 @@ pub trait ReadChunk {
 pub struct LocalChunkReader {
     store: Arc<DataStore>,
     crypt_config: Option<Arc<CryptConfig>>,
+    crypt_mode: CryptMode,
 }
 
 impl LocalChunkReader {
-    pub fn new(store: Arc<DataStore>, crypt_config: Option<Arc<CryptConfig>>) -> Self {
+    pub fn new(store: Arc<DataStore>, crypt_config: Option<Arc<CryptConfig>>, crypt_mode: CryptMode) -> Self {
         Self {
             store,
             crypt_config,
+            crypt_mode,
+        }
+    }
+
+    fn ensure_crypt_mode(&self, chunk_mode: CryptMode) -> Result<(), Error> {
+        match self.crypt_mode {
+            CryptMode::Encrypt => {
+                match chunk_mode {
+                    CryptMode::Encrypt => Ok(()),
+                    CryptMode::SignOnly | CryptMode::None => bail!("Index and chunk CryptMode don't match."),
+                }
+            },
+            CryptMode::SignOnly | CryptMode::None => {
+                match chunk_mode {
+                    CryptMode::Encrypt => bail!("Index and chunk CryptMode don't match."),
+                    CryptMode::SignOnly | CryptMode::None => Ok(()),
+                }
+            },
         }
     }
 }
 
 impl ReadChunk for LocalChunkReader {
     fn read_raw_chunk(&self, digest: &[u8; 32]) -> Result<DataBlob, Error> {
-        self.store.load_chunk(digest)
+        let chunk = self.store.load_chunk(digest)?;
+        self.ensure_crypt_mode(chunk.crypt_mode()?)?;
+        Ok(chunk)
     }
 
     fn read_chunk(&self, digest: &[u8; 32]) -> Result<Vec<u8>, Error> {
@@ -71,7 +92,8 @@ impl AsyncReadChunk for LocalChunkReader {
             let raw_data = tokio::fs::read(&path).await?;
 
             let chunk = DataBlob::load_from_reader(&mut &raw_data[..])?;
-           
+            self.ensure_crypt_mode(chunk.crypt_mode()?)?;
+
             Ok(chunk)
         })
     }
diff --git a/src/bin/proxmox-backup-client.rs b/src/bin/proxmox-backup-client.rs
index c6c11c75..9a6f309d 100644
--- a/src/bin/proxmox-backup-client.rs
+++ b/src/bin/proxmox-backup-client.rs
@@ -1182,6 +1182,7 @@ fn complete_backup_source(arg: &str, param: &HashMap<String, String>) -> Vec<Str
 async fn dump_image<W: Write>(
     client: Arc<BackupReader>,
     crypt_config: Option<Arc<CryptConfig>>,
+    crypt_mode: CryptMode,
     index: FixedIndexReader,
     mut writer: W,
     verbose: bool,
@@ -1189,7 +1190,7 @@ async fn dump_image<W: Write>(
 
     let most_used = index.find_most_used_chunks(8);
 
-    let chunk_reader = RemoteChunkReader::new(client.clone(), crypt_config, most_used);
+    let chunk_reader = RemoteChunkReader::new(client.clone(), crypt_config, crypt_mode, most_used);
 
     // Note: we avoid using BufferedFixedReader, because that add an additional buffer/copy
     // and thus slows down reading. Instead, directly use RemoteChunkReader
@@ -1340,7 +1341,12 @@ async fn restore(param: Value) -> Result<Value, Error> {
                 .map_err(|err| format_err!("unable to pipe data - {}", err))?;
         }
 
-    } else if archive_type == ArchiveType::Blob {
+        return Ok(Value::Null);
+    }
+
+    let file_info = manifest.lookup_file_info(&archive_name)?;
+
+    if archive_type == ArchiveType::Blob {
 
         let mut reader = client.download_blob(&manifest, &archive_name).await?;
 
@@ -1365,7 +1371,7 @@ async fn restore(param: Value) -> Result<Value, Error> {
 
         let most_used = index.find_most_used_chunks(8);
 
-        let chunk_reader = RemoteChunkReader::new(client.clone(), crypt_config, most_used);
+        let chunk_reader = RemoteChunkReader::new(client.clone(), crypt_config, file_info.chunk_crypt_mode(), most_used);
 
         let mut reader = BufferedDynamicReader::new(index, chunk_reader);
 
@@ -1412,7 +1418,7 @@ async fn restore(param: Value) -> Result<Value, Error> {
                 .map_err(|err| format_err!("unable to open /dev/stdout - {}", err))?
         };
 
-        dump_image(client.clone(), crypt_config.clone(), index, &mut writer, verbose).await?;
+        dump_image(client.clone(), crypt_config.clone(), file_info.chunk_crypt_mode(), index, &mut writer, verbose).await?;
     }
 
     Ok(Value::Null)
diff --git a/src/bin/proxmox_backup_client/catalog.rs b/src/bin/proxmox_backup_client/catalog.rs
index 1c0865e6..b419728e 100644
--- a/src/bin/proxmox_backup_client/catalog.rs
+++ b/src/bin/proxmox_backup_client/catalog.rs
@@ -97,7 +97,9 @@ async fn dump_catalog(param: Value) -> Result<Value, Error> {
 
     let most_used = index.find_most_used_chunks(8);
 
-    let chunk_reader = RemoteChunkReader::new(client.clone(), crypt_config, most_used);
+    let file_info = manifest.lookup_file_info(&CATALOG_NAME)?;
+
+    let chunk_reader = RemoteChunkReader::new(client.clone(), crypt_config, file_info.chunk_crypt_mode(), most_used);
 
     let mut reader = BufferedDynamicReader::new(index, chunk_reader);
 
@@ -200,7 +202,9 @@ async fn catalog_shell(param: Value) -> Result<(), Error> {
 
     let index = client.download_dynamic_index(&manifest, &server_archive_name).await?;
     let most_used = index.find_most_used_chunks(8);
-    let chunk_reader = RemoteChunkReader::new(client.clone(), crypt_config.clone(), most_used);
+
+    let file_info = manifest.lookup_file_info(&server_archive_name)?;
+    let chunk_reader = RemoteChunkReader::new(client.clone(), crypt_config.clone(), file_info.chunk_crypt_mode(), most_used);
     let reader = BufferedDynamicReader::new(index, chunk_reader);
     let archive_size = reader.archive_size();
     let reader: proxmox_backup::pxar::fuse::Reader =
@@ -216,7 +220,9 @@ async fn catalog_shell(param: Value) -> Result<(), Error> {
     manifest.verify_file(CATALOG_NAME, &csum, size)?;
 
     let most_used = index.find_most_used_chunks(8);
-    let chunk_reader = RemoteChunkReader::new(client.clone(), crypt_config, most_used);
+
+    let file_info = manifest.lookup_file_info(&CATALOG_NAME)?;
+    let chunk_reader = RemoteChunkReader::new(client.clone(), crypt_config, file_info.chunk_crypt_mode(), most_used);
     let mut reader = BufferedDynamicReader::new(index, chunk_reader);
     let mut catalogfile = std::fs::OpenOptions::new()
         .write(true)
diff --git a/src/bin/proxmox_backup_client/mount.rs b/src/bin/proxmox_backup_client/mount.rs
index 73bb8d4c..7646e98c 100644
--- a/src/bin/proxmox_backup_client/mount.rs
+++ b/src/bin/proxmox_backup_client/mount.rs
@@ -141,10 +141,12 @@ async fn mount_do(param: Value, pipe: Option<RawFd>) -> Result<Value, Error> {
 
     let (manifest, _) = client.download_manifest().await?;
 
+    let file_info = manifest.lookup_file_info(&archive_name)?;
+
     if server_archive_name.ends_with(".didx") {
         let index = client.download_dynamic_index(&manifest, &server_archive_name).await?;
         let most_used = index.find_most_used_chunks(8);
-        let chunk_reader = RemoteChunkReader::new(client.clone(), crypt_config, most_used);
+        let chunk_reader = RemoteChunkReader::new(client.clone(), crypt_config, file_info.chunk_crypt_mode(), most_used);
         let reader = BufferedDynamicReader::new(index, chunk_reader);
         let archive_size = reader.archive_size();
         let reader: proxmox_backup::pxar::fuse::Reader =
diff --git a/src/client/pull.rs b/src/client/pull.rs
index f1dde9b4..05b7c66c 100644
--- a/src/client/pull.rs
+++ b/src/client/pull.rs
@@ -224,8 +224,6 @@ async fn pull_snapshot(
 
     let manifest = BackupManifest::try_from(tmp_manifest_blob)?;
 
-    let mut chunk_reader = RemoteChunkReader::new(reader.clone(), None, HashMap::new());
-
     for item in manifest.files() {
         let mut path = tgt_store.base_path();
         path.push(snapshot.relative_path());
@@ -266,6 +264,8 @@ async fn pull_snapshot(
             }
         }
 
+        let mut chunk_reader = RemoteChunkReader::new(reader.clone(), None, item.chunk_crypt_mode(), HashMap::new());
+
         pull_single_archive(
             worker,
             &reader,
diff --git a/src/client/remote_chunk_reader.rs b/src/client/remote_chunk_reader.rs
index b30d0567..2670bf22 100644
--- a/src/client/remote_chunk_reader.rs
+++ b/src/client/remote_chunk_reader.rs
@@ -3,10 +3,10 @@ use std::collections::HashMap;
 use std::pin::Pin;
 use std::sync::{Arc, Mutex};
 
-use anyhow::Error;
+use anyhow::{bail, Error};
 
 use super::BackupReader;
-use crate::backup::{AsyncReadChunk, CryptConfig, DataBlob, ReadChunk};
+use crate::backup::{AsyncReadChunk, CryptConfig, CryptMode, DataBlob, ReadChunk};
 use crate::tools::runtime::block_on;
 
 /// Read chunks from remote host using ``BackupReader``
@@ -14,6 +14,7 @@ use crate::tools::runtime::block_on;
 pub struct RemoteChunkReader {
     client: Arc<BackupReader>,
     crypt_config: Option<Arc<CryptConfig>>,
+    crypt_mode: CryptMode,
     cache_hint: HashMap<[u8; 32], usize>,
     cache: Arc<Mutex<HashMap<[u8; 32], Vec<u8>>>>,
 }
@@ -25,11 +26,13 @@ impl RemoteChunkReader {
     pub fn new(
         client: Arc<BackupReader>,
         crypt_config: Option<Arc<CryptConfig>>,
+        crypt_mode: CryptMode,
         cache_hint: HashMap<[u8; 32], usize>,
     ) -> Self {
         Self {
             client,
             crypt_config,
+            crypt_mode,
             cache_hint,
             cache: Arc::new(Mutex::new(HashMap::new())),
         }
@@ -46,7 +49,20 @@ impl RemoteChunkReader {
 
         let chunk = DataBlob::load_from_reader(&mut &chunk_data[..])?;
 
-        Ok(chunk)
+        match self.crypt_mode {
+            CryptMode::Encrypt => {
+                match chunk.crypt_mode()? {
+                    CryptMode::Encrypt => Ok(chunk),
+                    CryptMode::SignOnly | CryptMode::None => bail!("Index and chunk CryptMode don't match."),
+                }
+            },
+            CryptMode::SignOnly | CryptMode::None => {
+                match chunk.crypt_mode()? {
+                    CryptMode::Encrypt => bail!("Index and chunk CryptMode don't match."),
+                    CryptMode::SignOnly | CryptMode::None => Ok(chunk),
+                }
+            },
+        }
     }
 }
 
-- 
2.20.1