From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: 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 9E01691EA4 for ; Wed, 31 Jan 2024 14:43:05 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 7C9B83B481 for ; Wed, 31 Jan 2024 14:42:35 +0100 (CET) Received: from bookworm-dev.localdomain (unknown [94.136.29.99]) by firstgate.proxmox.com (Proxmox) with ESMTP for ; Wed, 31 Jan 2024 14:42:34 +0100 (CET) Received: by bookworm-dev.localdomain (Postfix, from userid 1000) id D45AE60E3C; Wed, 31 Jan 2024 14:42:33 +0100 (CET) From: Dominik Csapak To: pbs-devel@lists.proxmox.com Date: Wed, 31 Jan 2024 14:42:33 +0100 Message-Id: <20240131134233.86370-1-d.csapak@proxmox.com> X-Mailer: git-send-email 2.39.2 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL -0.377 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment RDNS_NONE 0.793 Delivered to internal network by a host with no rDNS SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record T_SCC_BODY_TEXT_LINE -0.01 - Subject: [pbs-devel] [PATCH proxmox-backup] tape: fix regression in restoring key from medium X-BeenThere: pbs-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Backup Server development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 31 Jan 2024 13:43:05 -0000 when trying to restore a key from a tape, we automatically try to load the key into the drive after reading the media-set label. Since the key restore is only useful when the key is not already on disk, this fails of course. To fix it but leave the automatism in place, introduce a function 'read_label_without_loading_key' that does the heavy lifting of reading the labels but does not load the encryption key. Then in read_label, call that function and explictely load the key (when one exists). Then, we only have to use this new function in the 'restore-key' api call for the restore to work as intended. Fixes: 1343dcaf ("tape: move 'set_encryption' calls to the TapeDriver (and implementation)") Signed-off-by: Dominik Csapak --- should be cherry-pickable for stable-2 if necessary src/api2/tape/drive.rs | 2 +- src/tape/drive/mod.rs | 42 ++++++++++++++++++++++++++++++------------ 2 files changed, 31 insertions(+), 13 deletions(-) diff --git a/src/api2/tape/drive.rs b/src/api2/tape/drive.rs index 53b14bfd..5a5d39d9 100644 --- a/src/api2/tape/drive.rs +++ b/src/api2/tape/drive.rs @@ -630,7 +630,7 @@ pub async fn restore_key(drive: String, password: String) -> Result<(), Error> { run_drive_blocking_task(drive.clone(), "restore key".to_string(), move |config| { let mut drive = open_drive(&config, &drive)?; - let (_media_id, key_config) = drive.read_label()?; + let (_media_id, key_config) = drive.read_label_without_loading_key()?; if let Some(key_config) = key_config { let password_fn = || Ok(password.as_bytes().to_vec()); diff --git a/src/tape/drive/mod.rs b/src/tape/drive/mod.rs index 73886f14..2fc88ef8 100644 --- a/src/tape/drive/mod.rs +++ b/src/tape/drive/mod.rs @@ -105,11 +105,13 @@ pub trait TapeDriver { key_config: Option<&KeyConfig>, ) -> Result<(), Error>; - /// Read the media label + /// Read the media label without setting the encryption key /// - /// This tries to read both media labels (label and - /// media_set_label). Also returns the optional encryption key configuration. - fn read_label(&mut self) -> Result<(Option, Option), Error> { + /// This is used internally by 'read_label' and when restoring the encryption + /// key from the drive. Should not be used or overwritten otherwise! + fn read_label_without_loading_key( + &mut self, + ) -> Result<(Option, Option), Error> { self.rewind()?; let label = { @@ -182,18 +184,34 @@ pub trait TapeDriver { bail!("got unexpected data after media set label"); } - drop(reader); + media_id.media_set_label = Some(media_set_label); - let encrypt_fingerprint = media_set_label - .encryption_key_fingerprint - .clone() - .map(|fp| (fp, media_set_label.uuid.clone())); + Ok((Some(media_id), key_config)) + } - self.set_encryption(encrypt_fingerprint)?; + /// Read the media label + /// + /// This tries to read both media labels (label and + /// media_set_label). Also returns the optional encryption key configuration. + /// + /// Automatically sets the encryption key on the drive + fn read_label(&mut self) -> Result<(Option, Option), Error> { + let (media_id, key_config) = self.read_label_without_loading_key()?; - media_id.media_set_label = Some(media_set_label); + let encrypt_fingerprint = if let Some(media_set_label) = + media_id.as_ref().and_then(|id| id.media_set_label.clone()) + { + media_set_label + .encryption_key_fingerprint + .clone() + .map(|fp| (fp, media_set_label.uuid.clone())) + } else { + None + }; - Ok((Some(media_id), key_config)) + self.set_encryption(encrypt_fingerprint)?; + + Ok((media_id, key_config)) } /// Eject media -- 2.39.2