tape: fix wrongly unloading encryption key

For security, we want to automatically unload the encryption key from
the drive when we're done, so there was a Drop handler for SgTape that
handles that. Sadly, our tool we use to set it in the first place, also
invoked the Drop handler, thus unloading the keys again immediately

To fix that, move the Drop handler one logical level higher to the
LtoTapeHandle, which is not used by the 'sg-tape-cmd'.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>

diff --git a/pbs-tape/src/sg_tape.rs b/pbs-tape/src/sg_tape.rs
index 3a1cd026dfe53c68bcc59cef7e6b279ac444ed1d..bc0c14f0366667c6828dbd56dc69ca25f3515622 100644 (file)
--- a/pbs-tape/src/sg_tape.rs
+++ b/pbs-tape/src/sg_tape.rs
@@ -106,7 +106,6 @@ pub struct SgTape {
     file: File,
     locate_offset: Option<i64>,
     info: InquiryInfo,
-    encryption_key_loaded: bool,
 }
 
 impl SgTape {
@@ -128,7 +127,6 @@ impl SgTape {
         Ok(Self {
             file,
             info,
-            encryption_key_loaded: false,
             locate_offset: None,
         })
     }
@@ -673,7 +671,6 @@ impl SgTape {
         } else {
             None
         };
-        self.encryption_key_loaded = key.is_some();
 
         drive_set_encryption(&mut self.file, key)
     }
@@ -1029,15 +1026,6 @@ impl SgTape {
     }
 }
 
-impl Drop for SgTape {
-    fn drop(&mut self) {
-        // For security reasons, clear the encryption key
-        if self.encryption_key_loaded {
-            let _ = self.set_encryption(None);
-        }
-    }
-}
-
 pub struct SgTapeReader<'a> {
     sg_tape: &'a mut SgTape,
     end_of_file: bool,

diff --git a/src/tape/drive/lto/mod.rs b/src/tape/drive/lto/mod.rs
index ae4bc73428d2fbfedd27e6657c868cf7d75eeb7e..98087b258e78b5a929ad0c27561701b0c2b4b43c 100644 (file)
--- a/src/tape/drive/lto/mod.rs
+++ b/src/tape/drive/lto/mod.rs
@@ -33,16 +33,32 @@ use crate::tape::{
     file_formats::{MediaSetLabel, PROXMOX_BACKUP_MEDIA_SET_LABEL_MAGIC_1_0},
 };
 
+impl Drop for LtoTapeHandle {
+    fn drop(&mut self) {
+        // always unload the encryption key when the handle is dropped for security
+        // but only log an error if we set one in the first place
+        if let Err(err) = self.set_encryption(None) {
+            if self.encryption_key_loaded {
+                log::error!("could not unload encryption key from drive: {err}");
+            }
+        }
+    }
+}
+
 /// Lto Tape device handle
 pub struct LtoTapeHandle {
     sg_tape: SgTape,
+    encryption_key_loaded: bool,
 }
 
 impl LtoTapeHandle {
     /// Creates a new instance
     pub fn new(file: File) -> Result<Self, Error> {
         let sg_tape = SgTape::new(file)?;
-        Ok(Self { sg_tape })
+        Ok(Self {
+            sg_tape,
+            encryption_key_loaded: false,
+        })
     }
 
     /// Open a tape device
@@ -52,7 +68,10 @@ impl LtoTapeHandle {
     pub fn open_lto_drive(config: &LtoTapeDrive) -> Result<Self, Error> {
         let sg_tape = SgTape::open_lto_drive(config)?;
 
-        let handle = Self { sg_tape };
+        let handle = Self {
+            sg_tape,
+            encryption_key_loaded: false,
+        };
 
         Ok(handle)
     }
@@ -278,6 +297,7 @@ impl TapeDriver for LtoTapeHandle {
                 &["--fingerprint", &fingerprint, "--uuid", &uuid.to_string()],
                 self.sg_tape.file_mut().as_raw_fd(),
             )?;
+            self.encryption_key_loaded = true;
             let result: Result<(), String> = serde_json::from_str(&output)?;
             result.map_err(|err| format_err!("{}", err))
         } else {