From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <d.csapak@proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68])
 (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
 key-exchange X25519 server-signature RSA-PSS (2048 bits))
 (No client certificate requested)
 by lists.proxmox.com (Postfix) with ESMTPS id 9A6F8602F3
 for <pbs-devel@lists.proxmox.com>; Tue,  1 Sep 2020 15:47:37 +0200 (CEST)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
 by firstgate.proxmox.com (Proxmox) with ESMTP id 8771F9D11
 for <pbs-devel@lists.proxmox.com>; Tue,  1 Sep 2020 15:47:07 +0200 (CEST)
Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com
 [212.186.127.180])
 (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
 key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256)
 (No client certificate requested)
 by firstgate.proxmox.com (Proxmox) with ESMTPS id E74CA9D04
 for <pbs-devel@lists.proxmox.com>; Tue,  1 Sep 2020 15:47:06 +0200 (CEST)
Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1])
 by proxmox-new.maurer-it.com (Proxmox) with ESMTP id B0BEE445AD
 for <pbs-devel@lists.proxmox.com>; Tue,  1 Sep 2020 15:47:06 +0200 (CEST)
To: pbs-devel@lists.proxmox.com
References: <20200901103341.10075-1-s.reiter@proxmox.com>
From: Dominik Csapak <d.csapak@proxmox.com>
Message-ID: <0f626acf-ea04-748f-d76e-67ee46425147@proxmox.com>
Date: Tue, 1 Sep 2020 15:47:05 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:80.0) Gecko/20100101
 Thunderbird/80.0
MIME-Version: 1.0
In-Reply-To: <20200901103341.10075-1-s.reiter@proxmox.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
X-SPAM-LEVEL: Spam detection results:  0
 AWL 1.696 Adjusted score from AWL reputation of From: address
 KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment
 NICE_REPLY_A            -2.13 Looks like a legit reply (A)
 RCVD_IN_DNSWL_MED        -2.3 Sender listed at https://www.dnswl.org/,
 medium trust
 SPF_HELO_NONE           0.001 SPF: HELO does not publish an SPF Record
 SPF_PASS               -0.001 SPF: sender matches SPF record
 URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See
 http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more
 information. [backup.rs]
Subject: Re: [pbs-devel] [PATCH proxmox-backup] backup: check verify state
 of previous backup before allowing reuse
X-BeenThere: pbs-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox Backup Server development discussion
 <pbs-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pbs-devel>, 
 <mailto:pbs-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pbs-devel/>
List-Post: <mailto:pbs-devel@lists.proxmox.com>
List-Help: <mailto:pbs-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel>, 
 <mailto:pbs-devel-request@lists.proxmox.com?subject=subscribe>
X-List-Received-Date: Tue, 01 Sep 2020 13:47:37 -0000

sadly this is not enough to handle this in a good way

short excerpt from src/backup/chunk_store.rs
fn insert_chunk ...
--->8---
if let Ok(metadata) = std::fs::metadata(&chunk_path) {
     if metadata.is_file() {
         return Ok((true, metadata.len()));
     } else {
         bail!("Got unexpected file type on store '{}' for chunk {}", 
self.name, digest_str);
     }
}
---8<---

so if the chunk already exists, the server does not write it again
we need here some 'force' parameter that overwrites the corrupt chunks

otherwise the client sends all chunks, but the server never writes them

On 9/1/20 12:33 PM, Stefan Reiter wrote:
> Do not allow clients to reuse chunks from the previous backup if it has
> a failed validation result. This would result in a new "successful"
> backup that potentially references broken chunks.
> 
> If the previous backup has not been verified, assume it is fine and
> continue on.
> 
> Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
> ---
>   src/api2/backup.rs | 13 +++++++++++++
>   1 file changed, 13 insertions(+)
> 
> diff --git a/src/api2/backup.rs b/src/api2/backup.rs
> index ad608d85..c0b1d985 100644
> --- a/src/api2/backup.rs
> +++ b/src/api2/backup.rs
> @@ -652,6 +652,19 @@ fn download_previous(
>               None => bail!("no previous backup"),
>           };
>   
> +        let (manifest, _) = env.datastore.load_manifest(&last_backup.backup_dir)?;
> +        let verify = manifest.unprotected["verify_state"].clone();
> +        match serde_json::from_value::<SnapshotVerifyState>(verify) {
> +            Ok(verify) => {
> +                if verify.state != "ok" {
> +                    bail!("previous backup has failed verification");
> +                }
> +            },
> +            Err(_) => {
> +                // no verify state found, ignore and treat as valid
> +            }
> +        };
> +
>           let mut path = env.datastore.snapshot_path(&last_backup.backup_dir);
>           path.push(&archive_name);
>   
>