public inbox for pbs-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Dominik Csapak <d.csapak@proxmox.com>
To: pbs-devel@lists.proxmox.com
Subject: [pbs-devel] [PATCH proxmox-backup] tape: fix regression in restoring key from medium
Date: Wed, 31 Jan 2024 14:42:33 +0100	[thread overview]
Message-ID: <20240131134233.86370-1-d.csapak@proxmox.com> (raw)

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 <d.csapak@proxmox.com>
---
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<MediaId>, Option<KeyConfig>), 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<MediaId>, Option<KeyConfig>), 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<MediaId>, Option<KeyConfig>), 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




             reply	other threads:[~2024-01-31 13:43 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-01-31 13:42 Dominik Csapak [this message]
2024-02-02 16:07 ` [pbs-devel] applied: " Thomas Lamprecht
2024-02-06  8:07   ` Dominik Csapak

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240131134233.86370-1-d.csapak@proxmox.com \
    --to=d.csapak@proxmox.com \
    --cc=pbs-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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