From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <pbs-devel-bounces@lists.proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68])
	by lore.proxmox.com (Postfix) with ESMTPS id D88C51FF17C
	for <inbox@lore.proxmox.com>; Wed, 16 Apr 2025 09:07:39 +0200 (CEST)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
	by firstgate.proxmox.com (Proxmox) with ESMTP id A97CF1E487;
	Wed, 16 Apr 2025 09:07:37 +0200 (CEST)
From: Dominik Csapak <d.csapak@proxmox.com>
To: pbs-devel@lists.proxmox.com
Date: Wed, 16 Apr 2025 09:07:03 +0200
Message-Id: <20250416070703.493585-1-d.csapak@proxmox.com>
X-Mailer: git-send-email 2.39.5
MIME-Version: 1.0
X-SPAM-LEVEL: Spam detection results:  0
 AWL 0.021 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
 RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to
 Validity was blocked. See
 https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more
 information.
 RCVD_IN_VALIDITY_RPBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to
 Validity was blocked. See
 https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more
 information.
 RCVD_IN_VALIDITY_SAFE_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to
 Validity was blocked. See
 https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more
 information.
 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. [proxmox.com, mod.rs, encryption.rs]
Subject: [pbs-devel] [PATCH proxmox-backup v2] tape: skip setting encryption
 if we can't and don't want to
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>
Reply-To: Proxmox Backup Server development discussion
 <pbs-devel@lists.proxmox.com>
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Errors-To: pbs-devel-bounces@lists.proxmox.com
Sender: "pbs-devel" <pbs-devel-bounces@lists.proxmox.com>

Some settings on changers prevents changing the encryption parameters
via the application, e.g. some libraries have a 'encryption disabled' or
'encryption is library managed' option. While the former situation can
be fixed by setting the library to 'application managed', the latter is
sometimes necessary for FIPS compliance (to ensure the tape data is
encrypted).

When libraries are configured this way, the code currently fails with
'drive does not support AES-GCM encryption'. Instead of failing, check
on first call to set_encryption if we could set it, and save that
result.

Only fail when encryption is to be enabled but it is not allowed, but
ignore the error when the backup should be done unencrypted.

`assert_encryption_mode` must also check if it's possible, and skip any
error if it's not possible and we wanted no encryption.

With these changes, it should be possible to use such configured libraries
when there is no encryption configured on the PBS side. (We currently
don't have a library with such capabilities to test.)

Note that in contrast to normal operation, the tape label will also be
encrypted then and will not be readable in case the encryption key is
lost or changed.

Additionally, return an error for 'drive_set_encryption' in case the
drive reports that it does not support hardware encryption, because this
is now already caught one level above in 'set_encryption'.

Also, slightly change the error message to make it clear that the drive
does not support *setting* encryption, not that it does not support
it at all.

This was reported in the community forum:

https://forum.proxmox.com/threads/107383/
https://forum.proxmox.com/threads/164941/

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
changes from v1:
* adapted error message to make it clear what is not supported
* also adapt `assert_encryption_mode`, otherwise a backup is not possible

 pbs-tape/src/sg_tape.rs            | 30 +++++++++++++++++++++++++++++-
 pbs-tape/src/sg_tape/encryption.rs | 12 ++----------
 src/tape/drive/lto/mod.rs          |  4 ++++
 3 files changed, 35 insertions(+), 11 deletions(-)

diff --git a/pbs-tape/src/sg_tape.rs b/pbs-tape/src/sg_tape.rs
index 39e6a2f94..9c7a254d5 100644
--- a/pbs-tape/src/sg_tape.rs
+++ b/pbs-tape/src/sg_tape.rs
@@ -136,6 +136,8 @@ pub struct SgTape {
     file: File,
     locate_offset: Option<i64>,
     info: InquiryInfo,
+    // auto-detect if we can set the encryption mode
+    encryption_possible: Option<bool>,
 }
 
 impl SgTape {
@@ -158,6 +160,7 @@ impl SgTape {
             file,
             info,
             locate_offset: None,
+            encryption_possible: None,
         })
     }
 
@@ -690,6 +693,14 @@ impl SgTape {
     }
 
     pub fn set_encryption(&mut self, key_data: Option<([u8; 32], Uuid)>) -> Result<(), Error> {
+        if self.encryption_possible == Some(false) {
+            if key_data.is_some() {
+                bail!("Drive does not support setting encryption mode");
+            } else {
+                // skip trying to set encryption if not supported and don't wanted
+                return Ok(());
+            }
+        }
         let key = if let Some((ref key, ref uuid)) = key_data {
             // derive specialized key for each media-set
 
@@ -710,7 +721,24 @@ impl SgTape {
             None
         };
 
-        drive_set_encryption(&mut self.file, key)
+        match drive_set_encryption(&mut self.file, key) {
+            Ok(()) => self.encryption_possible = Some(true),
+            Err(err) => {
+                self.encryption_possible = Some(false);
+                if key.is_some() {
+                    bail!("could not set encryption mode on drive: {err}");
+                } else {
+                    log::info!("could not set encryption mode on drive: {err}, ignoring.");
+                }
+            }
+        }
+        Ok(())
+    }
+
+    /// Returns if encryption is possible. Returns [`None`] if it's unknown, because
+    /// no attempt was made to set the mode yet.
+    pub fn encryption_possible(&self) -> Option<bool> {
+        self.encryption_possible
     }
 
     // Note: use alloc_page_aligned_buffer to alloc data transfer buffer
diff --git a/pbs-tape/src/sg_tape/encryption.rs b/pbs-tape/src/sg_tape/encryption.rs
index 7247d257f..c24a6c658 100644
--- a/pbs-tape/src/sg_tape/encryption.rs
+++ b/pbs-tape/src/sg_tape/encryption.rs
@@ -12,15 +12,7 @@ use crate::sgutils2::{alloc_page_aligned_buffer, SgRaw};
 ///
 /// We always use mixed mode,
 pub fn drive_set_encryption<F: AsRawFd>(file: &mut F, key: Option<[u8; 32]>) -> Result<(), Error> {
-    let data = match sg_spin_data_encryption_caps(file) {
-        Ok(data) => data,
-        Err(_) if key.is_none() => {
-            // Assume device does not support HW encryption
-            // We can simply ignore the clear key request
-            return Ok(());
-        }
-        Err(err) => return Err(err),
-    };
+    let data = sg_spin_data_encryption_caps(file)?;
 
     let algorithm_index = decode_spin_data_encryption_caps(&data)?;
 
@@ -266,7 +258,7 @@ fn decode_spin_data_encryption_caps(data: &[u8]) -> Result<u8, Error> {
 
         match aes_gcm_index {
             Some(index) => Ok(index),
-            None => bail!("drive does not support AES-GCM encryption"),
+            None => bail!("drive does not support setting AES-GCM encryption"),
         }
     })
     .map_err(|err: Error| format_err!("decode data encryption caps page failed - {}", err))
diff --git a/src/tape/drive/lto/mod.rs b/src/tape/drive/lto/mod.rs
index 23e043ce6..bd5ec8ae6 100644
--- a/src/tape/drive/lto/mod.rs
+++ b/src/tape/drive/lto/mod.rs
@@ -264,6 +264,10 @@ impl TapeDriver for LtoTapeHandle {
     }
 
     fn assert_encryption_mode(&mut self, encryption_wanted: bool) -> Result<(), Error> {
+        if Some(false) == self.sg_tape.encryption_possible() && !encryption_wanted {
+            // ignore assertion for unencrypted mode, because we can't set it anyway
+            return Ok(());
+        }
         let encryption_set = drive_get_encryption(self.sg_tape.file_mut())?;
         if encryption_wanted != encryption_set {
             bail!("Set encryption mode not what was desired (set: {encryption_set}, wanted: {encryption_wanted})");
-- 
2.39.5



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel