[pve-devel] [PATCH proxmox-backup-qemu v2 0/3] fix #2866: invalidate bitmap on crypt_mode change

[pve-devel] [PATCH proxmox-backup-qemu v2 0/3] fix #2866: invalidate bitmap on crypt_mode change

[Fabian Grünbichler]

changes since v1:
- refactor code for de-duplication
- drop print to STDERR
- split into three patches

Fabian Grünbichler (3):
  refactor incremental checks
  add archive_name helper
  fix #2866: invalidate bitmap on crypt_mode change

 src/backup.rs   |  8 +++++++-
 src/commands.rs | 44 ++++++++++++++++++++++++++++++++------------
 2 files changed, 39 insertions(+), 13 deletions(-)

-- 
2.20.1

[pve-devel] [PATCH proxmox-backup-qemu v2 1/3] refactor incremental checks

[Fabian Grünbichler]

in preparation for adding more checks to check_incremental

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
 src/backup.rs   |  5 ++++-
 src/commands.rs | 11 +++--------
 2 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/src/backup.rs b/src/backup.rs
index 717e099..dab1da0 100644
--- a/src/backup.rs
+++ b/src/backup.rs
@@ -202,7 +202,10 @@ impl BackupTask {
         device_name: String,
         size: u64,
     ) -> bool {
-        check_last_incremental_csum(self.last_manifest(), device_name, size)
+        match self.last_manifest() {
+            Some(ref manifest) => check_last_incremental_csum(manifest.clone(), &device_name, size),
+            None => false,
+        }
     }
 
     pub async fn register_image(
diff --git a/src/commands.rs b/src/commands.rs
index 6f26324..dd00ca9 100644
--- a/src/commands.rs
+++ b/src/commands.rs
@@ -79,19 +79,14 @@ pub(crate) async fn add_config(
 }
 
 pub(crate) fn check_last_incremental_csum(
-    manifest: Option<Arc<BackupManifest>>,
-    device_name: String,
+    manifest: Arc<BackupManifest>,
+    device_name: &str,
     device_size: u64,
 ) -> bool {
 
-    let manifest = match manifest {
-        Some(ref manifest) => manifest,
-        None => return false,
-    };
-
     let archive_name = format!("{}.img.fidx", device_name);
 
-    match PREVIOUS_CSUMS.lock().unwrap().get(&device_name) {
+    match PREVIOUS_CSUMS.lock().unwrap().get(device_name) {
         Some(csum) => manifest.verify_file(&archive_name, &csum, device_size).is_ok(),
         None => false,
     }
-- 
2.20.1

[pve-devel] [PATCH proxmox-backup-qemu v2 2/3] add archive_name helper

[Fabian Grünbichler]

since we're about to introduce a third duplicate of this line..

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
 src/commands.rs | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/src/commands.rs b/src/commands.rs
index dd00ca9..e2584ab 100644
--- a/src/commands.rs
+++ b/src/commands.rs
@@ -78,16 +78,18 @@ pub(crate) async fn add_config(
     Ok(0)
 }
 
+fn archive_name(device_name: &str) -> String {
+    format!("{}.img.fidx", device_name)
+}
+
 pub(crate) fn check_last_incremental_csum(
     manifest: Arc<BackupManifest>,
     device_name: &str,
     device_size: u64,
 ) -> bool {
 
-    let archive_name = format!("{}.img.fidx", device_name);
-
     match PREVIOUS_CSUMS.lock().unwrap().get(device_name) {
-        Some(csum) => manifest.verify_file(&archive_name, &csum, device_size).is_ok(),
+        Some(csum) => manifest.verify_file(&archive_name(device_name), &csum, device_size).is_ok(),
         None => false,
     }
 }
@@ -105,7 +107,7 @@ pub(crate) async fn register_image(
     incremental: bool,
 ) -> Result<c_int, Error> {
 
-    let archive_name = format!("{}.img.fidx", device_name);
+    let archive_name = archive_name(&device_name);
 
     let index = match manifest {
         Some(manifest) => {
-- 
2.20.1

[pve-devel] [PATCH proxmox-backup-qemu v2 3/3] fix #2866: invalidate bitmap on crypt_mode change

[Fabian Grünbichler]

signed and plain backups share chunks, so bitmap reusal is okay for
those combinations. switching from encrypted to not encrypted or
vice-versa could have pretty fatal consequences - either referencing
plain-text chunks in 'encrypted' backups, or referencing encrypted
chunks in 'unencrypted' backups without still having the corresponding
keys..

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
 src/backup.rs   |  5 ++++-
 src/commands.rs | 23 +++++++++++++++++++++++
 2 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/src/backup.rs b/src/backup.rs
index dab1da0..9e3d66a 100644
--- a/src/backup.rs
+++ b/src/backup.rs
@@ -203,7 +203,10 @@ impl BackupTask {
         size: u64,
     ) -> bool {
         match self.last_manifest() {
-            Some(ref manifest) => check_last_incremental_csum(manifest.clone(), &device_name, size),
+            Some(ref manifest) => {
+                check_last_incremental_csum(manifest.clone(), &device_name, size)
+                    && check_last_encryption_mode(manifest.clone(), &device_name, self.crypt_mode)
+            },
             None => false,
         }
     }
diff --git a/src/commands.rs b/src/commands.rs
index e2584ab..c59ee1b 100644
--- a/src/commands.rs
+++ b/src/commands.rs
@@ -94,6 +94,29 @@ pub(crate) fn check_last_incremental_csum(
     }
 }
 
+pub(crate) fn check_last_encryption_mode(
+    manifest: Arc<BackupManifest>,
+    device_name: &str,
+    crypt_mode: CryptMode,
+) -> bool {
+    match manifest.lookup_file_info(&archive_name(device_name)) {
+        Ok(file) => {
+            match file.crypt_mode {
+                CryptMode::Encrypt => match crypt_mode {
+                    CryptMode::Encrypt => true,
+                    _ => false,
+                },
+                CryptMode::SignOnly | CryptMode::None => match crypt_mode {
+                    CryptMode::Encrypt => false,
+                    _ => true,
+                },
+            }
+        },
+        _ => false,
+    }
+}
+
+
 pub(crate) async fn register_image(
     client: Arc<BackupWriter>,
     crypt_config: Option<Arc<CryptConfig>>,
-- 
2.20.1

[pve-devel] applied: [PATCH proxmox-backup-qemu v2 0/3] fix #2866: invalidate bitmap on crypt_mode change

[Wolfgang Bumiller]

applied series

On Fri, Jul 24, 2020 at 11:53:32AM +0200, Fabian Grünbichler wrote:
> changes since v1:
> - refactor code for de-duplication
> - drop print to STDERR
> - split into three patches
> 
> Fabian Grünbichler (3):
>   refactor incremental checks
>   add archive_name helper
>   fix #2866: invalidate bitmap on crypt_mode change
> 
>  src/backup.rs   |  8 +++++++-
>  src/commands.rs | 44 ++++++++++++++++++++++++++++++++------------
>  2 files changed, 39 insertions(+), 13 deletions(-)
> 
> -- 
> 2.20.1