From 9025312aa6d451cab12b44a5bc65248615e93e66 Mon Sep 17 00:00:00 2001 From: Wolfgang Bumiller Date: Fri, 16 Aug 2019 09:19:01 +0200 Subject: [PATCH] avoid lifetimes in blob reader/writer --- src/backup/checksum_reader.rs | 24 ++++++++++++++++----- src/backup/checksum_writer.rs | 26 ++++++++++++++++------ src/backup/crypt_reader.rs | 3 ++- src/backup/crypt_writer.rs | 3 ++- src/backup/data_blob.rs | 7 +++--- src/backup/data_blob_reader.rs | 31 +++++++++++++------------- src/backup/data_blob_writer.rs | 37 ++++++++++++++++---------------- src/bin/proxmox-backup-client.rs | 17 +++++---------- src/client/http_client.rs | 10 +++------ tests/blob_writer.rs | 19 ++++++++-------- 10 files changed, 98 insertions(+), 79 deletions(-) diff --git a/src/backup/checksum_reader.rs b/src/backup/checksum_reader.rs index da8cf702..4b90349d 100644 --- a/src/backup/checksum_reader.rs +++ b/src/backup/checksum_reader.rs @@ -1,16 +1,30 @@ use failure::*; +use std::sync::Arc; use std::io::Read; -pub struct ChecksumReader<'a, R> { +use super::CryptConfig; +use crate::tools::borrow::Tied; + +pub struct ChecksumReader { reader: R, hasher: crc32fast::Hasher, - signer: Option>, + signer: Option, openssl::sign::Signer<'static>>>, } -impl <'a, R: Read> ChecksumReader<'a, R> { +impl ChecksumReader { - pub fn new(reader: R, signer: Option>) -> Self { + pub fn new(reader: R, config: Option>) -> Self { let hasher = crc32fast::Hasher::new(); + let signer = match config { + Some(config) => { + let tied_signer = Tied::new(config, |config| { + Box::new(unsafe { (*config).data_signer() }) + }); + Some(tied_signer) + } + None => None, + }; + Self { reader, hasher, signer } } @@ -27,7 +41,7 @@ impl <'a, R: Read> ChecksumReader<'a, R> { } } -impl <'a, R: Read> Read for ChecksumReader<'a, R> { +impl Read for ChecksumReader { fn read(&mut self, buf: &mut [u8]) -> Result { let count = self.reader.read(buf)?; diff --git a/src/backup/checksum_writer.rs b/src/backup/checksum_writer.rs index f3bbe30c..c9c3fabc 100644 --- a/src/backup/checksum_writer.rs +++ b/src/backup/checksum_writer.rs @@ -1,16 +1,30 @@ -use failure::*; +use std::sync::Arc; use std::io::Write; -pub struct ChecksumWriter<'a, W> { +use failure::*; + +use super::CryptConfig; +use crate::tools::borrow::Tied; + +pub struct ChecksumWriter { writer: W, hasher: crc32fast::Hasher, - signer: Option>, + signer: Option, openssl::sign::Signer<'static>>>, } -impl <'a, W: Write> ChecksumWriter<'a, W> { +impl ChecksumWriter { - pub fn new(writer: W, signer: Option>) -> Self { + pub fn new(writer: W, config: Option>) -> Self { let hasher = crc32fast::Hasher::new(); + let signer = match config { + Some(config) => { + let tied_signer = Tied::new(config.clone(), |config| { + Box::new(unsafe { (*config).data_signer() }) + }); + Some(tied_signer) + } + None => None, + }; Self { writer, hasher, signer } } @@ -27,7 +41,7 @@ impl <'a, W: Write> ChecksumWriter<'a, W> { } } -impl <'a, W: Write> Write for ChecksumWriter<'a, W> { +impl Write for ChecksumWriter { fn write(&mut self, buf: &[u8]) -> Result { self.hasher.update(buf); diff --git a/src/backup/crypt_reader.rs b/src/backup/crypt_reader.rs index c36c8b66..5b77ec22 100644 --- a/src/backup/crypt_reader.rs +++ b/src/backup/crypt_reader.rs @@ -1,4 +1,5 @@ use failure::*; +use std::sync::Arc; use std::io::{Read, BufRead}; use super::CryptConfig; @@ -13,7 +14,7 @@ pub struct CryptReader { impl CryptReader { - pub fn new(reader: R, iv: [u8; 16], tag: [u8; 16], config: &CryptConfig) -> Result { + pub fn new(reader: R, iv: [u8; 16], tag: [u8; 16], config: Arc) -> Result { let block_size = config.cipher().block_size(); // Note: block size is normally 1 byte for stream ciphers if block_size.count_ones() != 1 || block_size > 512 { bail!("unexpected Cipher block size {}", block_size); diff --git a/src/backup/crypt_writer.rs b/src/backup/crypt_writer.rs index 06a6aed9..bb3294f1 100644 --- a/src/backup/crypt_writer.rs +++ b/src/backup/crypt_writer.rs @@ -1,4 +1,5 @@ use failure::*; +use std::sync::Arc; use std::io::Write; use super::CryptConfig; @@ -13,7 +14,7 @@ pub struct CryptWriter { impl CryptWriter { - pub fn new(writer: W, config: &CryptConfig) -> Result { + pub fn new(writer: W, config: Arc) -> Result { let mut iv = [0u8; 16]; proxmox::sys::linux::fill_with_random_data(&mut iv)?; let block_size = config.cipher().block_size(); diff --git a/src/backup/data_blob.rs b/src/backup/data_blob.rs index 1f9e3642..148de248 100644 --- a/src/backup/data_blob.rs +++ b/src/backup/data_blob.rs @@ -1,5 +1,6 @@ use failure::*; use std::convert::TryInto; +use std::sync::Arc; use proxmox::tools::io::{ReadExt, WriteExt}; @@ -69,7 +70,7 @@ impl DataBlob { /// Create a DataBlob, optionally compressed and/or encrypted pub fn encode( data: &[u8], - config: Option<&CryptConfig>, + config: Option>, compress: bool, ) -> Result { @@ -158,7 +159,7 @@ impl DataBlob { } /// Decode blob data - pub fn decode(self, config: Option<&CryptConfig>) -> Result, Error> { + pub fn decode(self, config: Option>) -> Result, Error> { let magic = self.magic(); @@ -215,7 +216,7 @@ impl DataBlob { /// Create a signed DataBlob, optionally compressed pub fn create_signed( data: &[u8], - config: &CryptConfig, + config: Arc, compress: bool, ) -> Result { diff --git a/src/backup/data_blob_reader.rs b/src/backup/data_blob_reader.rs index 99aabd78..9ae112ad 100644 --- a/src/backup/data_blob_reader.rs +++ b/src/backup/data_blob_reader.rs @@ -1,26 +1,27 @@ use failure::*; +use std::sync::Arc; use std::io::{Read, BufReader}; use proxmox::tools::io::ReadExt; use super::*; -enum BlobReaderState<'a, R: Read> { - Uncompressed { expected_crc: u32, csum_reader: ChecksumReader<'a, R> }, - Compressed { expected_crc: u32, decompr: zstd::stream::read::Decoder>> }, - Signed { expected_crc: u32, expected_hmac: [u8; 32], csum_reader: ChecksumReader<'a, R> }, - SignedCompressed { expected_crc: u32, expected_hmac: [u8; 32], decompr: zstd::stream::read::Decoder>> }, - Encrypted { expected_crc: u32, decrypt_reader: CryptReader>> }, - EncryptedCompressed { expected_crc: u32, decompr: zstd::stream::read::Decoder>>>> }, +enum BlobReaderState { + Uncompressed { expected_crc: u32, csum_reader: ChecksumReader }, + Compressed { expected_crc: u32, decompr: zstd::stream::read::Decoder>> }, + Signed { expected_crc: u32, expected_hmac: [u8; 32], csum_reader: ChecksumReader }, + SignedCompressed { expected_crc: u32, expected_hmac: [u8; 32], decompr: zstd::stream::read::Decoder>> }, + Encrypted { expected_crc: u32, decrypt_reader: CryptReader>> }, + EncryptedCompressed { expected_crc: u32, decompr: zstd::stream::read::Decoder>>>> }, } /// Read data blobs -pub struct DataBlobReader<'a, R: Read> { - state: BlobReaderState<'a, R>, +pub struct DataBlobReader { + state: BlobReaderState, } -impl <'a, R: Read> DataBlobReader<'a, R> { +impl DataBlobReader { - pub fn new(mut reader: R, config: Option<&'a CryptConfig>) -> Result { + pub fn new(mut reader: R, config: Option>) -> Result { let head: DataBlobHeader = unsafe { reader.read_le_value()? }; match head.magic { @@ -40,16 +41,14 @@ impl <'a, R: Read> DataBlobReader<'a, R> { let expected_crc = u32::from_le_bytes(head.crc); let mut expected_hmac = [0u8; 32]; reader.read_exact(&mut expected_hmac)?; - let signer = config.map(|c| c.data_signer()); - let csum_reader = ChecksumReader::new(reader, signer); + let csum_reader = ChecksumReader::new(reader, config); Ok(Self { state: BlobReaderState::Signed { expected_crc, expected_hmac, csum_reader }}) } AUTH_COMPR_BLOB_MAGIC_1_0 => { let expected_crc = u32::from_le_bytes(head.crc); let mut expected_hmac = [0u8; 32]; reader.read_exact(&mut expected_hmac)?; - let signer = config.map(|c| c.data_signer()); - let csum_reader = ChecksumReader::new(reader, signer); + let csum_reader = ChecksumReader::new(reader, config); let decompr = zstd::stream::read::Decoder::new(csum_reader)?; Ok(Self { state: BlobReaderState::SignedCompressed { expected_crc, expected_hmac, decompr }}) @@ -142,7 +141,7 @@ impl <'a, R: Read> DataBlobReader<'a, R> { } } -impl <'a, R: Read> Read for DataBlobReader<'a, R> { +impl Read for DataBlobReader { fn read(&mut self, buf: &mut [u8]) -> Result { match &mut self.state { diff --git a/src/backup/data_blob_writer.rs b/src/backup/data_blob_writer.rs index cf024dc5..86aa9d65 100644 --- a/src/backup/data_blob_writer.rs +++ b/src/backup/data_blob_writer.rs @@ -1,24 +1,25 @@ use failure::*; +use std::sync::Arc; use std::io::{Write, Seek, SeekFrom}; use proxmox::tools::io::WriteExt; use super::*; -enum BlobWriterState<'a, W: Write> { - Uncompressed { csum_writer: ChecksumWriter<'a, W> }, - Compressed { compr: zstd::stream::write::Encoder> }, - Signed { csum_writer: ChecksumWriter<'a, W> }, - SignedCompressed { compr: zstd::stream::write::Encoder> }, - Encrypted { crypt_writer: CryptWriter> }, - EncryptedCompressed { compr: zstd::stream::write::Encoder>> }, +enum BlobWriterState { + Uncompressed { csum_writer: ChecksumWriter }, + Compressed { compr: zstd::stream::write::Encoder> }, + Signed { csum_writer: ChecksumWriter }, + SignedCompressed { compr: zstd::stream::write::Encoder> }, + Encrypted { crypt_writer: CryptWriter> }, + EncryptedCompressed { compr: zstd::stream::write::Encoder>> }, } /// Data blob writer -pub struct DataBlobWriter<'a, W: Write> { - state: BlobWriterState<'a, W>, +pub struct DataBlobWriter { + state: BlobWriterState, } -impl <'a, W: Write + Seek> DataBlobWriter<'a, W> { +impl DataBlobWriter { pub fn new_uncompressed(mut writer: W) -> Result { writer.seek(SeekFrom::Start(0))?; @@ -41,7 +42,7 @@ impl <'a, W: Write + Seek> DataBlobWriter<'a, W> { Ok(Self { state: BlobWriterState::Compressed { compr }}) } - pub fn new_signed(mut writer: W, config: &'a CryptConfig) -> Result { + pub fn new_signed(mut writer: W, config: Arc) -> Result { writer.seek(SeekFrom::Start(0))?; let head = AuthenticatedDataBlobHeader { head: DataBlobHeader { magic: AUTHENTICATED_BLOB_MAGIC_1_0, crc: [0; 4] }, @@ -50,12 +51,11 @@ impl <'a, W: Write + Seek> DataBlobWriter<'a, W> { unsafe { writer.write_le_value(head)?; } - let signer = config.data_signer(); - let csum_writer = ChecksumWriter::new(writer, Some(signer)); + let csum_writer = ChecksumWriter::new(writer, Some(config)); Ok(Self { state: BlobWriterState::Signed { csum_writer }}) } - pub fn new_signed_compressed(mut writer: W, config: &'a CryptConfig) -> Result { + pub fn new_signed_compressed(mut writer: W, config: Arc) -> Result { writer.seek(SeekFrom::Start(0))?; let head = AuthenticatedDataBlobHeader { head: DataBlobHeader { magic: AUTH_COMPR_BLOB_MAGIC_1_0, crc: [0; 4] }, @@ -64,13 +64,12 @@ impl <'a, W: Write + Seek> DataBlobWriter<'a, W> { unsafe { writer.write_le_value(head)?; } - let signer = config.data_signer(); - let csum_writer = ChecksumWriter::new(writer, Some(signer)); + let csum_writer = ChecksumWriter::new(writer, Some(config)); let compr = zstd::stream::write::Encoder::new(csum_writer, 1)?; Ok(Self { state: BlobWriterState::SignedCompressed { compr }}) } - pub fn new_encrypted(mut writer: W, config: &'a CryptConfig) -> Result { + pub fn new_encrypted(mut writer: W, config: Arc) -> Result { writer.seek(SeekFrom::Start(0))?; let head = EncryptedDataBlobHeader { head: DataBlobHeader { magic: ENCRYPTED_BLOB_MAGIC_1_0, crc: [0; 4] }, @@ -86,7 +85,7 @@ impl <'a, W: Write + Seek> DataBlobWriter<'a, W> { Ok(Self { state: BlobWriterState::Encrypted { crypt_writer }}) } - pub fn new_encrypted_compressed(mut writer: W, config: &'a CryptConfig) -> Result { + pub fn new_encrypted_compressed(mut writer: W, config: Arc) -> Result { writer.seek(SeekFrom::Start(0))?; let head = EncryptedDataBlobHeader { head: DataBlobHeader { magic: ENCR_COMPR_BLOB_MAGIC_1_0, crc: [0; 4] }, @@ -194,7 +193,7 @@ impl <'a, W: Write + Seek> DataBlobWriter<'a, W> { } } -impl <'a, W: Write + Seek> Write for DataBlobWriter<'a, W> { +impl Write for DataBlobWriter { fn write(&mut self, buf: &[u8]) -> Result { match self.state { diff --git a/src/bin/proxmox-backup-client.rs b/src/bin/proxmox-backup-client.rs index 47c1daa0..9b5082d1 100644 --- a/src/bin/proxmox-backup-client.rs +++ b/src/bin/proxmox-backup-client.rs @@ -435,7 +435,7 @@ fn dump_catalog( None => None, Some(path) => { let (key, _) = load_and_decrtypt_key(&path, get_encryption_key_password)?; - Some(CryptConfig::new(key)?) + Some(Arc::new(CryptConfig::new(key)?)) } }; @@ -457,7 +457,7 @@ fn dump_catalog( blob_file.seek(SeekFrom::Start(0))?; - let reader = BufReader::new(DataBlobReader::new(blob_file, crypt_config.as_ref())?); + let reader = BufReader::new(DataBlobReader::new(blob_file, crypt_config)?); let mut catalog_reader = pxar::catalog::SimpleCatalogReader::new(reader); @@ -888,10 +888,7 @@ fn restore( let blob = DataBlob::from_raw(blob_data)?; blob.verify_crc()?; - let raw_data = match crypt_config { - Some(ref crypt_config) => blob.decode(Some(crypt_config))?, - None => blob.decode(None)?, - }; + let raw_data = blob.decode(crypt_config)?; if let Some(target) = target { file_set_contents(target, &raw_data, None)?; @@ -991,17 +988,13 @@ fn upload_log( Some(path) => { let (key, _created) = load_and_decrtypt_key(&path, get_encryption_key_password)?; let crypt_config = CryptConfig::new(key)?; - Some(crypt_config) + Some(Arc::new(crypt_config)) } }; let data = file_get_contents(logfile)?; - let blob = if let Some(ref crypt_config) = crypt_config { - DataBlob::encode(&data, Some(crypt_config), true)? - } else { - DataBlob::encode(&data, None, true)? - }; + let blob = DataBlob::encode(&data, crypt_config, true)?; let raw_data = blob.into_inner(); diff --git a/src/client/http_client.rs b/src/client/http_client.rs index 6e6322ff..ab68582a 100644 --- a/src/client/http_client.rs +++ b/src/client/http_client.rs @@ -641,11 +641,11 @@ impl BackupClient { futures::future::ok(()) .and_then(move |_| { - let blob = if let Some(ref crypt_config) = crypt_config { + let blob = if let Some(crypt_config) = crypt_config { if sign_only { DataBlob::create_signed(&data, crypt_config, compress)? } else { - DataBlob::encode(&data, Some(crypt_config), compress)? + DataBlob::encode(&data, Some(crypt_config.clone()), compress)? } } else { DataBlob::encode(&data, None, compress)? @@ -683,11 +683,7 @@ impl BackupClient { tokio::io::read_to_end(file, contents) .map_err(Error::from) .and_then(move |(_, contents)| { - let blob = if let Some(ref crypt_config) = crypt_config { - DataBlob::encode(&contents, Some(crypt_config), compress)? - } else { - DataBlob::encode(&contents, None, compress)? - }; + let blob = DataBlob::encode(&contents, crypt_config, compress)?; let raw_data = blob.into_inner(); Ok((raw_data, contents.len() as u64)) }) diff --git a/tests/blob_writer.rs b/tests/blob_writer.rs index 651009a9..7579c348 100644 --- a/tests/blob_writer.rs +++ b/tests/blob_writer.rs @@ -1,6 +1,7 @@ use failure::*; +use std::sync::Arc; use std::io::Cursor; -use std::io::{ Read, Write, Seek, SeekFrom }; +use std::io::{Read, Write, Seek, SeekFrom }; use lazy_static::lazy_static; use proxmox_backup::backup::*; @@ -16,9 +17,9 @@ lazy_static! { data }; - static ref CRYPT_CONFIG: CryptConfig = { + static ref CRYPT_CONFIG: Arc = { let key = [1u8; 32]; - CryptConfig::new(key).unwrap() + Arc::new(CryptConfig::new(key).unwrap()) }; } @@ -30,7 +31,7 @@ fn verify_test_blob(mut cursor: Cursor>) -> Result<(), Error> { println!("Starting DataBlobReader test (size = {})", size); cursor.seek(SeekFrom::Start(0))?; - let mut reader = DataBlobReader::new(&mut cursor, Some(&CRYPT_CONFIG))?; + let mut reader = DataBlobReader::new(&mut cursor, Some(CRYPT_CONFIG.clone()))?; let mut buffer = Vec::::new(); // read the whole file //reader.read_to_end(&mut buffer)?; @@ -52,7 +53,7 @@ fn verify_test_blob(mut cursor: Cursor>) -> Result<(), Error> { let blob = DataBlob::from_raw(raw_data)?; blob.verify_crc()?; - let data = blob.decode(Some(&CRYPT_CONFIG))?; + let data = blob.decode(Some(CRYPT_CONFIG.clone()))?; if data != *TEST_DATA { bail!("blob data is wrong (decode)"); } @@ -80,7 +81,7 @@ fn test_compressed_blob_writer() -> Result<(), Error> { #[test] fn test_signed_blob_writer() -> Result<(), Error> { let tmp = Cursor::new(Vec::::new()); - let mut blob_writer = DataBlobWriter::new_signed(tmp, &CRYPT_CONFIG)?; + let mut blob_writer = DataBlobWriter::new_signed(tmp, CRYPT_CONFIG.clone())?; blob_writer.write_all(&TEST_DATA)?; verify_test_blob(blob_writer.finish()?) @@ -89,7 +90,7 @@ fn test_signed_blob_writer() -> Result<(), Error> { #[test] fn test_signed_compressed_blob_writer() -> Result<(), Error> { let tmp = Cursor::new(Vec::::new()); - let mut blob_writer = DataBlobWriter::new_signed_compressed(tmp, &CRYPT_CONFIG)?; + let mut blob_writer = DataBlobWriter::new_signed_compressed(tmp, CRYPT_CONFIG.clone())?; blob_writer.write_all(&TEST_DATA)?; verify_test_blob(blob_writer.finish()?) @@ -98,7 +99,7 @@ fn test_signed_compressed_blob_writer() -> Result<(), Error> { #[test] fn test_encrypted_blob_writer() -> Result<(), Error> { let tmp = Cursor::new(Vec::::new()); - let mut blob_writer = DataBlobWriter::new_encrypted(tmp, &CRYPT_CONFIG)?; + let mut blob_writer = DataBlobWriter::new_encrypted(tmp, CRYPT_CONFIG.clone())?; blob_writer.write_all(&TEST_DATA)?; verify_test_blob(blob_writer.finish()?) @@ -107,7 +108,7 @@ fn test_encrypted_blob_writer() -> Result<(), Error> { #[test] fn test_encrypted_compressed_blob_writer() -> Result<(), Error> { let tmp = Cursor::new(Vec::::new()); - let mut blob_writer = DataBlobWriter::new_encrypted_compressed(tmp, &CRYPT_CONFIG)?; + let mut blob_writer = DataBlobWriter::new_encrypted_compressed(tmp, CRYPT_CONFIG.clone())?; blob_writer.write_all(&TEST_DATA)?; verify_test_blob(blob_writer.finish()?)