public inbox for pbs-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pbs-devel] [PATCH proxmox-backup] tape: fix regression in restoring key from medium
@ 2024-01-31 13:42 Dominik Csapak
  2024-02-02 16:07 ` [pbs-devel] applied: " Thomas Lamprecht
  0 siblings, 1 reply; 3+ messages in thread
From: Dominik Csapak @ 2024-01-31 13:42 UTC (permalink / raw)
  To: pbs-devel

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




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

end of thread, other threads:[~2024-02-06  8:07 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-01-31 13:42 [pbs-devel] [PATCH proxmox-backup] tape: fix regression in restoring key from medium Dominik Csapak
2024-02-02 16:07 ` [pbs-devel] applied: " Thomas Lamprecht
2024-02-06  8:07   ` Dominik Csapak

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