[pbs-devel] [PATCH proxmox-backup 3/5] chunk readers: ensure chunk/index CryptMode matches
Fabian Grünbichler
f.gruenbichler at proxmox.com
Mon Aug 10 13:25:07 CEST 2020
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 at 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
More information about the pbs-devel
mailing list