public inbox for pbs-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [PATCH proxmox-backup] pull: decrypt: set and check change-detection-fingerprint
@ 2026-04-28  9:44 Fabian Grünbichler
  2026-04-28 10:01 ` Dominik Csapak
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Fabian Grünbichler @ 2026-04-28  9:44 UTC (permalink / raw)
  To: pbs-devel

when decrypt-pulling a snapshot that was not created by encrypt-pushing, but by
a regular encrypted backup, resyncing requires matching the local plain text
manifest and the remote encrypted one.

in addition to storing a signature of the plain manifest when encrypt-pushing,
store the already existing signature of the encrypted source manifest when
decrypt-pulling, but only use it for matching if the former does not exist.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
 src/server/pull.rs | 23 +++++++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/src/server/pull.rs b/src/server/pull.rs
index 4a41ea7da..c519ed5d8 100644
--- a/src/server/pull.rs
+++ b/src/server/pull.rs
@@ -21,8 +21,8 @@ use tokio::io::AsyncWriteExt;
 
 use pbs_api_types::{
     print_store_and_ns, ArchiveType, Authid, BackupArchiveName, BackupDir, BackupGroup,
-    BackupNamespace, CryptMode, GroupFilter, Operation, RateLimitConfig, Remote, SnapshotListItem,
-    VerifyState, CLIENT_LOG_BLOB_NAME, MANIFEST_BLOB_NAME, MAX_NAMESPACE_DEPTH,
+    BackupNamespace, CryptMode, Fingerprint, GroupFilter, Operation, RateLimitConfig, Remote,
+    SnapshotListItem, VerifyState, CLIENT_LOG_BLOB_NAME, MANIFEST_BLOB_NAME, MAX_NAMESPACE_DEPTH,
     PRIV_DATASTORE_AUDIT, PRIV_DATASTORE_BACKUP,
 };
 use pbs_client::BackupRepository;
@@ -878,6 +878,11 @@ async fn pull_snapshot<'a>(
             bail!("Encountered unexpected manifest without 'unprotected' section.");
         }
 
+        if let Some(expected) = &manifest.signature {
+            let expected: Fingerprint = expected.parse().with_context(|| prefix.clone())?;
+            new_manifest.set_change_detection_fingerprint(expected.bytes())?;
+        }
+
         let manifest_string = new_manifest.to_string(None)?;
         let manifest_blob = DataBlob::encode(manifest_string.as_bytes(), None, true)?;
         // update contents to be uploaded to backend
@@ -1021,6 +1026,20 @@ async fn optionally_use_decryption_key(
             } else {
                 bail!("Change detection fingerprint mismatch, refuse to continue!");
             }
+        } else if let Some(source_signature) = existing_manifest
+            .get_change_detection_fingerprint()
+            .context("failed to parse change detection fingerprint of existing target manifest")
+            .with_context(|| prefix.clone())?
+        {
+            let Some(expected) = &manifest.signature else {
+                bail!("No signature on source manifest.");
+            };
+            let expected: Fingerprint = expected.parse().with_context(|| prefix.clone())?;
+            if expected == source_signature {
+                skip_resync = true;
+            } else {
+                bail!("Change detection fingerprint mismatch, refuse to continue!");
+            }
         } else {
             bail!("No change detection fingerprint found, refuse to continue!");
         }
-- 
2.47.3





^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH proxmox-backup] pull: decrypt: set and check change-detection-fingerprint
  2026-04-28  9:44 [PATCH proxmox-backup] pull: decrypt: set and check change-detection-fingerprint Fabian Grünbichler
@ 2026-04-28 10:01 ` Dominik Csapak
  2026-04-28 12:56 ` Christian Ebner
  2026-04-28 13:07 ` applied: " Thomas Lamprecht
  2 siblings, 0 replies; 4+ messages in thread
From: Dominik Csapak @ 2026-04-28 10:01 UTC (permalink / raw)
  To: Fabian Grünbichler, pbs-devel

LGTM, not deep enough into the sync code currently to give
a full r-b though..

so:

Tested-by: Dominik Csapak <d.csapak@proxmox.com>

On 4/28/26 11:43 AM, Fabian Grünbichler wrote:
> when decrypt-pulling a snapshot that was not created by encrypt-pushing, but by
> a regular encrypted backup, resyncing requires matching the local plain text
> manifest and the remote encrypted one.
> 
> in addition to storing a signature of the plain manifest when encrypt-pushing,
> store the already existing signature of the encrypted source manifest when
> decrypt-pulling, but only use it for matching if the former does not exist.
> 
> Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
> ---
>   src/server/pull.rs | 23 +++++++++++++++++++++--
>   1 file changed, 21 insertions(+), 2 deletions(-)
> 
> diff --git a/src/server/pull.rs b/src/server/pull.rs
> index 4a41ea7da..c519ed5d8 100644
> --- a/src/server/pull.rs
> +++ b/src/server/pull.rs
> @@ -21,8 +21,8 @@ use tokio::io::AsyncWriteExt;
>   
>   use pbs_api_types::{
>       print_store_and_ns, ArchiveType, Authid, BackupArchiveName, BackupDir, BackupGroup,
> -    BackupNamespace, CryptMode, GroupFilter, Operation, RateLimitConfig, Remote, SnapshotListItem,
> -    VerifyState, CLIENT_LOG_BLOB_NAME, MANIFEST_BLOB_NAME, MAX_NAMESPACE_DEPTH,
> +    BackupNamespace, CryptMode, Fingerprint, GroupFilter, Operation, RateLimitConfig, Remote,
> +    SnapshotListItem, VerifyState, CLIENT_LOG_BLOB_NAME, MANIFEST_BLOB_NAME, MAX_NAMESPACE_DEPTH,
>       PRIV_DATASTORE_AUDIT, PRIV_DATASTORE_BACKUP,
>   };
>   use pbs_client::BackupRepository;
> @@ -878,6 +878,11 @@ async fn pull_snapshot<'a>(
>               bail!("Encountered unexpected manifest without 'unprotected' section.");
>           }
>   
> +        if let Some(expected) = &manifest.signature {
> +            let expected: Fingerprint = expected.parse().with_context(|| prefix.clone())?;
> +            new_manifest.set_change_detection_fingerprint(expected.bytes())?;
> +        }
> +
>           let manifest_string = new_manifest.to_string(None)?;
>           let manifest_blob = DataBlob::encode(manifest_string.as_bytes(), None, true)?;
>           // update contents to be uploaded to backend
> @@ -1021,6 +1026,20 @@ async fn optionally_use_decryption_key(
>               } else {
>                   bail!("Change detection fingerprint mismatch, refuse to continue!");
>               }
> +        } else if let Some(source_signature) = existing_manifest
> +            .get_change_detection_fingerprint()
> +            .context("failed to parse change detection fingerprint of existing target manifest")
> +            .with_context(|| prefix.clone())?
> +        {
> +            let Some(expected) = &manifest.signature else {
> +                bail!("No signature on source manifest.");
> +            };
> +            let expected: Fingerprint = expected.parse().with_context(|| prefix.clone())?;
> +            if expected == source_signature {
> +                skip_resync = true;
> +            } else {
> +                bail!("Change detection fingerprint mismatch, refuse to continue!");
> +            }
>           } else {
>               bail!("No change detection fingerprint found, refuse to continue!");
>           }





^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH proxmox-backup] pull: decrypt: set and check change-detection-fingerprint
  2026-04-28  9:44 [PATCH proxmox-backup] pull: decrypt: set and check change-detection-fingerprint Fabian Grünbichler
  2026-04-28 10:01 ` Dominik Csapak
@ 2026-04-28 12:56 ` Christian Ebner
  2026-04-28 13:07 ` applied: " Thomas Lamprecht
  2 siblings, 0 replies; 4+ messages in thread
From: Christian Ebner @ 2026-04-28 12:56 UTC (permalink / raw)
  To: Fabian Grünbichler, pbs-devel

On 4/28/26 11:43 AM, Fabian Grünbichler wrote:
> when decrypt-pulling a snapshot that was not created by encrypt-pushing, but by
> a regular encrypted backup, resyncing requires matching the local plain text
> manifest and the remote encrypted one.
> 
> in addition to storing a signature of the plain manifest when encrypt-pushing,
> store the already existing signature of the encrypted source manifest when
> decrypt-pulling, but only use it for matching if the former does not exist.
> 
> Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
> ---

Can also confirm that this fixes the issue and looks good to me!

Reviewed-by: Christian Ebner <c.ebner@proxmox.com>
Signed-off-by: Christian Ebner <c.ebner@proxmox.com>




^ permalink raw reply	[flat|nested] 4+ messages in thread

* applied: [PATCH proxmox-backup] pull: decrypt: set and check change-detection-fingerprint
  2026-04-28  9:44 [PATCH proxmox-backup] pull: decrypt: set and check change-detection-fingerprint Fabian Grünbichler
  2026-04-28 10:01 ` Dominik Csapak
  2026-04-28 12:56 ` Christian Ebner
@ 2026-04-28 13:07 ` Thomas Lamprecht
  2 siblings, 0 replies; 4+ messages in thread
From: Thomas Lamprecht @ 2026-04-28 13:07 UTC (permalink / raw)
  To: pbs-devel, Fabian Grünbichler

On Tue, 28 Apr 2026 11:44:17 +0200, Fabian Grünbichler wrote:
> when decrypt-pulling a snapshot that was not created by encrypt-pushing, but by
> a regular encrypted backup, resyncing requires matching the local plain text
> manifest and the remote encrypted one.
> 
> in addition to storing a signature of the plain manifest when encrypt-pushing,
> store the already existing signature of the encrypted source manifest when
> decrypt-pulling, but only use it for matching if the former does not exist.
> 
> [...]

Applied, thanks!

[1/1] pull: decrypt: set and check change-detection-fingerprint
      commit: 40186dc465011c0645260115258015d010f54c7d




^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-04-28 13:08 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-28  9:44 [PATCH proxmox-backup] pull: decrypt: set and check change-detection-fingerprint Fabian Grünbichler
2026-04-28 10:01 ` Dominik Csapak
2026-04-28 12:56 ` Christian Ebner
2026-04-28 13:07 ` applied: " Thomas Lamprecht

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal