[pbs-devel] [PATCH proxmox-backup] src/backup/data_blob.rs: new load_from_reader(), which verifies the CRC
Dietmar Maurer
dietmar at proxmox.com
Tue Jul 28 10:41:14 CEST 2020
And make verify_crc private for now. We always call load_from_reader() to
verify the CRC.
Also add load_chunk() to datastore.rs (from chunk_store::read_chunk())
---
src/api2/admin/datastore.rs | 9 ++++----
src/api2/backup/environment.rs | 5 ++---
src/backup/chunk_store.rs | 16 --------------
src/backup/data_blob.rs | 21 +++++++++++++------
src/backup/datastore.rs | 35 ++++++++++++++++++++++---------
src/backup/read_chunk.rs | 11 +++-------
src/backup/verify.rs | 7 +++----
src/client/backup_reader.rs | 3 +--
src/client/backup_writer.rs | 3 +--
src/client/pull.rs | 6 ++----
src/client/remote_chunk_reader.rs | 5 +++--
tests/blob_writer.rs | 3 +--
12 files changed, 60 insertions(+), 64 deletions(-)
diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
index 0a93b47..4fcff86 100644
--- a/src/api2/admin/datastore.rs
+++ b/src/api2/admin/datastore.rs
@@ -1037,11 +1037,10 @@ fn upload_backup_log(
})
.await?;
- let blob = DataBlob::from_raw(data)?;
- // always verify CRC at server side
- blob.verify_crc()?;
- let raw_data = blob.raw_data();
- replace_file(&path, raw_data, CreateOptions::new())?;
+ // always verify blob/CRC at server side
+ let blob = DataBlob::load_from_reader(&mut &data[..])?;
+
+ replace_file(&path, blob.raw_data(), CreateOptions::new())?;
// fixme: use correct formatter
Ok(crate::server::formatter::json_response(Ok(Value::Null)))
diff --git a/src/api2/backup/environment.rs b/src/api2/backup/environment.rs
index 8d3d427..2a3632a 100644
--- a/src/api2/backup/environment.rs
+++ b/src/api2/backup/environment.rs
@@ -416,9 +416,8 @@ impl BackupEnvironment {
let blob_len = data.len();
let orig_len = data.len(); // fixme:
- let blob = DataBlob::from_raw(data)?;
- // always verify CRC at server side
- blob.verify_crc()?;
+ // always verify blob/CRC at server side
+ let blob = DataBlob::load_from_reader(&mut &data[..])?;
let raw_data = blob.raw_data();
replace_file(&path, raw_data, CreateOptions::new())?;
diff --git a/src/backup/chunk_store.rs b/src/backup/chunk_store.rs
index d57befd..5cc944c 100644
--- a/src/backup/chunk_store.rs
+++ b/src/backup/chunk_store.rs
@@ -184,22 +184,6 @@ impl ChunkStore {
Ok(true)
}
- pub fn read_chunk(&self, digest: &[u8; 32]) -> Result<DataBlob, Error> {
-
- let (chunk_path, digest_str) = self.chunk_path(digest);
- let mut file = std::fs::File::open(&chunk_path)
- .map_err(|err| {
- format_err!(
- "store '{}', unable to read chunk '{}' - {}",
- self.name,
- digest_str,
- err,
- )
- })?;
-
- DataBlob::load(&mut file)
- }
-
pub fn get_chunk_iterator(
&self,
) -> Result<
diff --git a/src/backup/data_blob.rs b/src/backup/data_blob.rs
index 07e7ca6..af9ebf8 100644
--- a/src/backup/data_blob.rs
+++ b/src/backup/data_blob.rs
@@ -36,6 +36,11 @@ impl DataBlob {
&self.raw_data
}
+ /// Returns raw_data size
+ pub fn raw_size(&self) -> u64 {
+ self.raw_data.len() as u64
+ }
+
/// Consume self and returns raw_data
pub fn into_inner(self) -> Vec<u8> {
self.raw_data
@@ -66,8 +71,8 @@ impl DataBlob {
hasher.finalize()
}
- /// verify the CRC32 checksum
- pub fn verify_crc(&self) -> Result<(), Error> {
+ // verify the CRC32 checksum
+ fn verify_crc(&self) -> Result<(), Error> {
let expected_crc = self.compute_crc();
if expected_crc != self.crc() {
bail!("Data blob has wrong CRC checksum.");
@@ -212,13 +217,17 @@ impl DataBlob {
}
}
- /// Load blob from ``reader``
- pub fn load(reader: &mut dyn std::io::Read) -> Result<Self, Error> {
+ /// Load blob from ``reader``, verify CRC
+ pub fn load_from_reader(reader: &mut dyn std::io::Read) -> Result<Self, Error> {
let mut data = Vec::with_capacity(1024*1024);
reader.read_to_end(&mut data)?;
- Self::from_raw(data)
+ let blob = Self::from_raw(data)?;
+
+ blob.verify_crc()?;
+
+ Ok(blob)
}
/// Create Instance from raw data
@@ -254,7 +263,7 @@ impl DataBlob {
/// To do that, we need to decompress data first. Please note that
/// this is not possible for encrypted chunks. This function simply return Ok
/// for encrypted chunks.
- /// Note: This does not call verify_crc
+ /// Note: This does not call verify_crc, because this is usually done in load
pub fn verify_unencrypted(
&self,
expected_chunk_size: usize,
diff --git a/src/backup/datastore.rs b/src/backup/datastore.rs
index e66ae84..92f7b06 100644
--- a/src/backup/datastore.rs
+++ b/src/backup/datastore.rs
@@ -499,28 +499,43 @@ impl DataStore {
}
pub fn verify_stored_chunk(&self, digest: &[u8; 32], expected_chunk_size: u64) -> Result<(), Error> {
- let blob = self.chunk_store.read_chunk(digest)?;
- blob.verify_crc()?;
+ let blob = self.load_chunk(digest)?;
blob.verify_unencrypted(expected_chunk_size as usize, digest)?;
Ok(())
}
- pub fn load_blob(&self, backup_dir: &BackupDir, filename: &str) -> Result<(DataBlob, u64), Error> {
+ pub fn load_blob(&self, backup_dir: &BackupDir, filename: &str) -> Result<DataBlob, Error> {
let mut path = self.base_path();
path.push(backup_dir.relative_path());
path.push(filename);
- let raw_data = proxmox::tools::fs::file_get_contents(&path)?;
- let raw_size = raw_data.len() as u64;
- let blob = DataBlob::from_raw(raw_data)?;
- Ok((blob, raw_size))
- }
-
+ proxmox::try_block!({
+ let mut file = std::fs::File::open(&path)?;
+ DataBlob::load_from_reader(&mut file)
+ }).map_err(|err| format_err!("unable to load blob '{:?}' - {}", path, err))
+ }
+
+ pub fn load_chunk(&self, digest: &[u8; 32]) -> Result<DataBlob, Error> {
+
+ let (chunk_path, digest_str) = self.chunk_store.chunk_path(digest);
+
+ proxmox::try_block!({
+ let mut file = std::fs::File::open(&chunk_path)?;
+ DataBlob::load_from_reader(&mut file)
+ }).map_err(|err| format_err!(
+ "store '{}', unable to load chunk '{}' - {}",
+ self.name(),
+ digest_str,
+ err,
+ ))
+ }
+
pub fn load_manifest(
&self,
backup_dir: &BackupDir,
) -> Result<(BackupManifest, CryptMode, u64), Error> {
- let (blob, raw_size) = self.load_blob(backup_dir, MANIFEST_BLOB_NAME)?;
+ let blob = self.load_blob(backup_dir, MANIFEST_BLOB_NAME)?;
+ let raw_size = blob.raw_size();
let crypt_mode = blob.crypt_mode()?;
let manifest = BackupManifest::try_from(blob)?;
Ok((manifest, crypt_mode, raw_size))
diff --git a/src/backup/read_chunk.rs b/src/backup/read_chunk.rs
index b489067..fb8296f 100644
--- a/src/backup/read_chunk.rs
+++ b/src/backup/read_chunk.rs
@@ -34,12 +34,7 @@ impl LocalChunkReader {
impl ReadChunk for LocalChunkReader {
fn read_raw_chunk(&self, digest: &[u8; 32]) -> Result<DataBlob, Error> {
- let (path, _) = self.store.chunk_path(digest);
- let raw_data = proxmox::tools::fs::file_get_contents(&path)?;
- let chunk = DataBlob::from_raw(raw_data)?;
- chunk.verify_crc()?;
-
- Ok(chunk)
+ self.store.load_chunk(digest)
}
fn read_chunk(&self, digest: &[u8; 32]) -> Result<Vec<u8>, Error> {
@@ -76,9 +71,9 @@ impl AsyncReadChunk for LocalChunkReader {
let (path, _) = self.store.chunk_path(digest);
let raw_data = tokio::fs::read(&path).await?;
- let chunk = DataBlob::from_raw(raw_data)?;
- chunk.verify_crc()?;
+ let chunk = DataBlob::load_from_reader(&mut &raw_data[..])?;
+
Ok(chunk)
})
}
diff --git a/src/backup/verify.rs b/src/backup/verify.rs
index 3d9ff7b..c968a49 100644
--- a/src/backup/verify.rs
+++ b/src/backup/verify.rs
@@ -10,19 +10,18 @@ use super::{
fn verify_blob(datastore: &DataStore, backup_dir: &BackupDir, info: &FileInfo) -> Result<(), Error> {
- let (blob, raw_size) = datastore.load_blob(backup_dir, &info.filename)?;
+ let blob = datastore.load_blob(backup_dir, &info.filename)?;
- let csum = openssl::sha::sha256(blob.raw_data());
+ let raw_size = blob.raw_size();
if raw_size != info.size {
bail!("wrong size ({} != {})", info.size, raw_size);
}
+ let csum = openssl::sha::sha256(blob.raw_data());
if csum != info.csum {
bail!("wrong index checksum");
}
- blob.verify_crc()?;
-
let magic = blob.magic();
if magic == &ENCR_COMPR_BLOB_MAGIC_1_0 || magic == &ENCRYPTED_BLOB_MAGIC_1_0 {
diff --git a/src/client/backup_reader.rs b/src/client/backup_reader.rs
index b0b43c3..c60b652 100644
--- a/src/client/backup_reader.rs
+++ b/src/client/backup_reader.rs
@@ -129,8 +129,7 @@ impl BackupReader {
let mut raw_data = Vec::with_capacity(64 * 1024);
self.download(MANIFEST_BLOB_NAME, &mut raw_data).await?;
- let blob = DataBlob::from_raw(raw_data)?;
- blob.verify_crc()?;
+ let blob = DataBlob::load_from_reader(&mut &raw_data[..])?;
let data = blob.decode(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 e10ff45..b201ef2 100644
--- a/src/client/backup_writer.rs
+++ b/src/client/backup_writer.rs
@@ -479,8 +479,7 @@ impl BackupWriter {
let param = json!({ "archive-name": MANIFEST_BLOB_NAME });
self.h2.download("previous", Some(param), &mut raw_data).await?;
- let blob = DataBlob::from_raw(raw_data)?;
- blob.verify_crc()?;
+ let blob = DataBlob::load_from_reader(&mut &raw_data[..])?;
let data = blob.decode(self.crypt_config.as_ref().map(Arc::as_ref))?;
let manifest = BackupManifest::from_data(&data[..], self.crypt_config.as_ref().map(Arc::as_ref))?;
diff --git a/src/client/pull.rs b/src/client/pull.rs
index 421260a..758cb57 100644
--- a/src/client/pull.rs
+++ b/src/client/pull.rs
@@ -174,16 +174,14 @@ async fn pull_snapshot(
};
},
};
- let tmp_manifest_blob = DataBlob::load(&mut tmp_manifest_file)?;
- tmp_manifest_blob.verify_crc()?;
+ let tmp_manifest_blob = DataBlob::load_from_reader(&mut tmp_manifest_file)?;
if manifest_name.exists() {
let manifest_blob = proxmox::try_block!({
let mut manifest_file = std::fs::File::open(&manifest_name)
.map_err(|err| format_err!("unable to open local manifest {:?} - {}", manifest_name, err))?;
- let manifest_blob = DataBlob::load(&mut manifest_file)?;
- manifest_blob.verify_crc()?;
+ let manifest_blob = DataBlob::load_from_reader(&mut manifest_file)?;
Ok(manifest_blob)
}).map_err(|err: Error| {
format_err!("unable to read local manifest {:?} - {}", manifest_name, err)
diff --git a/src/client/remote_chunk_reader.rs b/src/client/remote_chunk_reader.rs
index eeb4851..bf195d6 100644
--- a/src/client/remote_chunk_reader.rs
+++ b/src/client/remote_chunk_reader.rs
@@ -42,8 +42,9 @@ impl RemoteChunkReader {
.download_chunk(&digest, &mut chunk_data)
.await?;
- let chunk = DataBlob::from_raw(chunk_data)?;
- chunk.verify_crc()?;
+ let chunk = DataBlob::load_from_reader(&mut &chunk_data[..])?;
+
+ // fixme: verify digest?
Ok(chunk)
}
diff --git a/tests/blob_writer.rs b/tests/blob_writer.rs
index dcd306a..3d17ebd 100644
--- a/tests/blob_writer.rs
+++ b/tests/blob_writer.rs
@@ -50,8 +50,7 @@ fn verify_test_blob(mut cursor: Cursor<Vec<u8>>) -> Result<(), Error> {
let raw_data = cursor.into_inner();
- let blob = DataBlob::from_raw(raw_data)?;
- blob.verify_crc()?;
+ let blob = DataBlob::load_from_reader(&mut &raw_data[..])?;
let data = blob.decode(Some(&CRYPT_CONFIG))?;
if data != *TEST_DATA {
--
2.20.1
More information about the pbs-devel
mailing list