[pbs-devel] [PATCH v2 proxmox-backup 09/31] api: backup: add no-timestamp-check flag to backup endpoint

[Fabian Grünbichler]

Quoting Christian Ebner (2024-08-01 09:43:41)
> By setting the optional `no-timestamp-check` flag when calling the
> `backup` endpoint, the check for a strictly monotonically increase
> of the snapshots backup timestamp will be skipped.
> This has to be used when pushing backups to a target, where monotonic
> increase of the snapshot timestamp with respect to the previous
> snapshot present in the group cannot be guaranteed.

why is this the case for push based syncing, but not pull based syncing? both
could have the same semantics, and use "last snapshot on sync target" as
starting point for syncing..

I am not sure this needs to be part of a minimal implementation of push based
syncing, we should probably rather tackle this for both kinds of syncs as a
follow-up (e.g., the "re-sync corrupt and/or missing snapshots" feature).

it's also a bit dangerous, since it allows "stuffing" a backup group
retro-actively with fake backups, which in turn allows an attacker to abuse
automatic pruning to prune all real backup snapshots..

> Signed-off-by: Christian Ebner <c.ebner at proxmox.com>
> ---
> changes since version 1:
> - rename flag from `ignore-previous` to the now more fitting
>   `no-timestamp-check`
> - Only ignore the timestamp check if the flag is set, but nevertheless
>   get the previous snapshots manifest if present to allow the upload of
>   reused chunks indexed in the previous snapshot.
> 
>  examples/upload-speed.rs               | 1 +
>  pbs-client/src/backup_writer.rs        | 4 +++-
>  proxmox-backup-client/src/benchmark.rs | 1 +
>  proxmox-backup-client/src/main.rs      | 1 +
>  src/api2/backup/mod.rs                 | 4 +++-
>  5 files changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/examples/upload-speed.rs b/examples/upload-speed.rs
> index e4b570ec5..8a6594a47 100644
> --- a/examples/upload-speed.rs
> +++ b/examples/upload-speed.rs
> @@ -25,6 +25,7 @@ async fn upload_speed() -> Result<f64, Error> {
>          &(BackupType::Host, "speedtest".to_string(), backup_time).into(),
>          false,
>          true,
> +        false,
>      )
>      .await?;
>  
> diff --git a/pbs-client/src/backup_writer.rs b/pbs-client/src/backup_writer.rs
> index dbb7db0e8..9e59bbee8 100644
> --- a/pbs-client/src/backup_writer.rs
> +++ b/pbs-client/src/backup_writer.rs
> @@ -101,6 +101,7 @@ impl BackupWriter {
>          backup: &BackupDir,
>          debug: bool,
>          benchmark: bool,
> +        no_timestamp_check: bool,
>      ) -> Result<Arc<BackupWriter>, Error> {
>          let mut param = json!({
>              "backup-type": backup.ty(),
> @@ -108,7 +109,8 @@ impl BackupWriter {
>              "backup-time": backup.time,
>              "store": datastore,
>              "debug": debug,
> -            "benchmark": benchmark
> +            "benchmark": benchmark,
> +            "no-timestamp-check": no_timestamp_check,
>          });
>  
>          if !ns.is_root() {
> diff --git a/proxmox-backup-client/src/benchmark.rs b/proxmox-backup-client/src/benchmark.rs
> index 2145d45e2..3fe45f5fe 100644
> --- a/proxmox-backup-client/src/benchmark.rs
> +++ b/proxmox-backup-client/src/benchmark.rs
> @@ -236,6 +236,7 @@ async fn test_upload_speed(
>          &(BackupType::Host, "benchmark".to_string(), backup_time).into(),
>          false,
>          true,
> +        false,
>      )
>      .await?;
>  
> diff --git a/proxmox-backup-client/src/main.rs b/proxmox-backup-client/src/main.rs
> index 5edb2a824..c5d0da0aa 100644
> --- a/proxmox-backup-client/src/main.rs
> +++ b/proxmox-backup-client/src/main.rs
> @@ -967,6 +967,7 @@ async fn create_backup(
>          &snapshot,
>          true,
>          false,
> +        false,
>      )
>      .await?;
>  
> diff --git a/src/api2/backup/mod.rs b/src/api2/backup/mod.rs
> index ea0d0292e..59a302f53 100644
> --- a/src/api2/backup/mod.rs
> +++ b/src/api2/backup/mod.rs
> @@ -52,6 +52,7 @@ pub const API_METHOD_UPGRADE_BACKUP: ApiMethod = ApiMethod::new(
>              ("backup-time", false, &BACKUP_TIME_SCHEMA),
>              ("debug", true, &BooleanSchema::new("Enable verbose debug logging.").schema()),
>              ("benchmark", true, &BooleanSchema::new("Job is a benchmark (do not keep data).").schema()),
> +            ("no-timestamp-check", true, &BooleanSchema::new("Ignore timestamp check relative to previous snapshot.").schema()),
>          ]),
>      )
>  ).access(
> @@ -78,6 +79,7 @@ fn upgrade_to_backup_protocol(
>      async move {
>          let debug = param["debug"].as_bool().unwrap_or(false);
>          let benchmark = param["benchmark"].as_bool().unwrap_or(false);
> +        let no_timestamp_check = param["no-timestamp-check"].as_bool().unwrap_or(false);
>  
>          let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
>  
> @@ -178,7 +180,7 @@ fn upgrade_to_backup_protocol(
>          let backup_dir = backup_group.backup_dir(backup_dir_arg.time)?;
>  
>          let _last_guard = if let Some(last) = &last_backup {
> -            if backup_dir.backup_time() <= last.backup_dir.backup_time() {
> +            if !no_timestamp_check && backup_dir.backup_time() <= last.backup_dir.backup_time() {
>                  bail!("backup timestamp is older than last backup.");
>              }
>  
> -- 
> 2.39.2
> 
> 
> 
> _______________________________________________
> pbs-devel mailing list
> pbs-devel at lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
> 
>