Re: [pbs-devel] [PATCH proxmox-backup v2] tape: skip setting encryption if we can't and don't want to

[Dominik Csapak <d.csapak@proxmox.com>]

ping, should still apply as is, but we got an additional user
running into this problem:

https://forum.proxmox.com/threads/fips-mode-on-tape-library-pbs-errors-out-on-labeling.164941/#post-782914

On 4/16/25 09:07, Dominik Csapak wrote:
> 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})");



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