* [pbs-devel] [PATCH proxmox-backup] src/backup/data_blob.rs: new load_from_reader(), which verifies the CRC
@ 2020-07-28 8:41 Dietmar Maurer
0 siblings, 0 replies; only message in thread
From: Dietmar Maurer @ 2020-07-28 8:41 UTC (permalink / raw)
To: pbs-devel
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
^ permalink raw reply [flat|nested] only message in thread
only message in thread, other threads:[~2020-07-28 8:41 UTC | newest]
Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-28 8:41 [pbs-devel] [PATCH proxmox-backup] src/backup/data_blob.rs: new load_from_reader(), which verifies the CRC Dietmar Maurer
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.