public inbox for pbs-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pbs-devel] [RFC proxmox-backup] api: datastore: determine size and blob type if not in manifest
@ 2020-10-14 15:04 Stefan Reiter
  2020-10-14 15:30 ` Thomas Lamprecht
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Stefan Reiter @ 2020-10-14 15:04 UTC (permalink / raw)
  To: pbs-devel

Try to read the first 8 byte (DataBlobHeader magic) from any .blob file
not mentioned in the manifest. This allows classifying the
client.log.blob file as encrypted if it is, and showing the correct
symbol in the GUI for it.

While already open, also determine the file size by seeking to End(0).

Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
---

I noticed that the GUI would show the "download" icon enabled for
'client.log.blob' files, even if they were encrypted. The button wouldn't work
of course, since the server can't decode encrypted blobs, so nothing would
happen except an error logged to the console.

The problem is that currently we don't know if a blob file not listed in the
manifest is encrypted - but we can find out, since it's encoded into the header.
Not sure if this is worth the open() and read() for every unknown blob file, but
it would solve the problem...


 src/api2/admin/datastore.rs | 25 +++++++++++++++++++++++--
 src/backup/data_blob.rs     | 12 ++++++++++++
 2 files changed, 35 insertions(+), 2 deletions(-)

diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
index 11223e6a..2a9ac469 100644
--- a/src/api2/admin/datastore.rs
+++ b/src/api2/admin/datastore.rs
@@ -2,6 +2,7 @@ use std::collections::{HashSet, HashMap};
 use std::ffi::OsStr;
 use std::os::unix::ffi::OsStrExt;
 use std::sync::{Arc, Mutex};
+use std::io::{Read, Seek, SeekFrom};
 
 use anyhow::{bail, format_err, Error};
 use futures::*;
@@ -91,10 +92,30 @@ fn get_all_snapshot_files(
 
     for file in &info.files {
         if file_set.contains(file) { continue; }
+
+        // Try to determine size and crypt mode for unknown blobs
+        let mut crypt_mode = None;
+        let mut size = None;
+        if file.ends_with(".blob") {
+            let mut path = store.snapshot_path(&info.backup_dir);
+            path.push(file);
+            if let Ok(mut file) = std::fs::File::open(path) {
+                let mut buffer = [0u8; 8];
+                if let Ok(n) = file.read(&mut buffer[..]) {
+                    if n == buffer.len() {
+                        crypt_mode = DataBlob::is_blob_magic(&buffer);
+                    }
+                }
+                if let Ok(pos) = file.seek(SeekFrom::End(0)) {
+                    size = Some(pos);
+                }
+            }
+        }
+
         files.push(BackupContent {
             filename: file.to_string(),
-            size: None,
-            crypt_mode: None,
+            size,
+            crypt_mode,
         });
     }
 
diff --git a/src/backup/data_blob.rs b/src/backup/data_blob.rs
index 284dc243..626a5fe4 100644
--- a/src/backup/data_blob.rs
+++ b/src/backup/data_blob.rs
@@ -321,6 +321,18 @@ impl DataBlob {
 
         Ok(())
     }
+
+    /// Determine if the given value is a valid blob magic number.
+    /// Returns CryptMode::Encrypt or CryptMode::None depending on type.
+    pub fn is_blob_magic(magic: &[u8; 8]) -> Option<CryptMode> {
+        if magic == &UNCOMPRESSED_BLOB_MAGIC_1_0 || magic == &COMPRESSED_BLOB_MAGIC_1_0 {
+            Some(CryptMode::None)
+        } else if magic == &ENCRYPTED_BLOB_MAGIC_1_0 || magic == &ENCR_COMPR_BLOB_MAGIC_1_0 {
+            Some(CryptMode::Encrypt)
+        } else {
+            None
+        }
+    }
 }
 
 /// Builder for chunk DataBlobs
-- 
2.20.1





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

* Re: [pbs-devel] [RFC proxmox-backup] api: datastore: determine size and blob type if not in manifest
  2020-10-14 15:04 [pbs-devel] [RFC proxmox-backup] api: datastore: determine size and blob type if not in manifest Stefan Reiter
@ 2020-10-14 15:30 ` Thomas Lamprecht
  2020-10-15  4:25 ` Dietmar Maurer
  2020-10-15  7:07 ` Fabian Grünbichler
  2 siblings, 0 replies; 4+ messages in thread
From: Thomas Lamprecht @ 2020-10-14 15:30 UTC (permalink / raw)
  To: Proxmox Backup Server development discussion, Stefan Reiter

On 14.10.20 17:04, Stefan Reiter wrote:
> I noticed that the GUI would show the "download" icon enabled for
> 'client.log.blob' files, even if they were encrypted. The button wouldn't work
> of course, since the server can't decode encrypted blobs, so nothing would
> happen except an error logged to the console.

On another node, why not allow downloading an encrypted archive, the server view
may be easier to search for a backup, as all is available there. Then we could
provide a client command (or separate tool) to decrypt that archive, wherever
the user downloaded it.

IMO, it does not make sense to limit the user artificially here, it just should be
made clear that the downloaded archive is/will be still encrypted, e.g., by adding
a ".encrypted" file ending, or some visual GUI hint.





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

* Re: [pbs-devel] [RFC proxmox-backup] api: datastore: determine size and blob type if not in manifest
  2020-10-14 15:04 [pbs-devel] [RFC proxmox-backup] api: datastore: determine size and blob type if not in manifest Stefan Reiter
  2020-10-14 15:30 ` Thomas Lamprecht
@ 2020-10-15  4:25 ` Dietmar Maurer
  2020-10-15  7:07 ` Fabian Grünbichler
  2 siblings, 0 replies; 4+ messages in thread
From: Dietmar Maurer @ 2020-10-15  4:25 UTC (permalink / raw)
  To: Proxmox Backup Server development discussion, Stefan Reiter

This is a critical path, and I do not really want an additional delay here.
We need a better solution for this.


> On 10/14/2020 5:04 PM Stefan Reiter <s.reiter@proxmox.com> wrote:
> 
>  
> Try to read the first 8 byte (DataBlobHeader magic) from any .blob file
> not mentioned in the manifest. This allows classifying the
> client.log.blob file as encrypted if it is, and showing the correct
> symbol in the GUI for it.
> 
> While already open, also determine the file size by seeking to End(0).
> 
> Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
> ---
> 
> I noticed that the GUI would show the "download" icon enabled for
> 'client.log.blob' files, even if they were encrypted. The button wouldn't work
> of course, since the server can't decode encrypted blobs, so nothing would
> happen except an error logged to the console.
> 
> The problem is that currently we don't know if a blob file not listed in the
> manifest is encrypted - but we can find out, since it's encoded into the header.
> Not sure if this is worth the open() and read() for every unknown blob file, but
> it would solve the problem...
> 
> 
>  src/api2/admin/datastore.rs | 25 +++++++++++++++++++++++--
>  src/backup/data_blob.rs     | 12 ++++++++++++
>  2 files changed, 35 insertions(+), 2 deletions(-)
> 
> diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
> index 11223e6a..2a9ac469 100644
> --- a/src/api2/admin/datastore.rs
> +++ b/src/api2/admin/datastore.rs
> @@ -2,6 +2,7 @@ use std::collections::{HashSet, HashMap};
>  use std::ffi::OsStr;
>  use std::os::unix::ffi::OsStrExt;
>  use std::sync::{Arc, Mutex};
> +use std::io::{Read, Seek, SeekFrom};
>  
>  use anyhow::{bail, format_err, Error};
>  use futures::*;
> @@ -91,10 +92,30 @@ fn get_all_snapshot_files(
>  
>      for file in &info.files {
>          if file_set.contains(file) { continue; }
> +
> +        // Try to determine size and crypt mode for unknown blobs
> +        let mut crypt_mode = None;
> +        let mut size = None;
> +        if file.ends_with(".blob") {
> +            let mut path = store.snapshot_path(&info.backup_dir);
> +            path.push(file);
> +            if let Ok(mut file) = std::fs::File::open(path) {
> +                let mut buffer = [0u8; 8];
> +                if let Ok(n) = file.read(&mut buffer[..]) {
> +                    if n == buffer.len() {
> +                        crypt_mode = DataBlob::is_blob_magic(&buffer);
> +                    }
> +                }
> +                if let Ok(pos) = file.seek(SeekFrom::End(0)) {
> +                    size = Some(pos);
> +                }
> +            }
> +        }
> +
>          files.push(BackupContent {
>              filename: file.to_string(),
> -            size: None,
> -            crypt_mode: None,
> +            size,
> +            crypt_mode,
>          });
>      }
>  
> diff --git a/src/backup/data_blob.rs b/src/backup/data_blob.rs
> index 284dc243..626a5fe4 100644
> --- a/src/backup/data_blob.rs
> +++ b/src/backup/data_blob.rs
> @@ -321,6 +321,18 @@ impl DataBlob {
>  
>          Ok(())
>      }
> +
> +    /// Determine if the given value is a valid blob magic number.
> +    /// Returns CryptMode::Encrypt or CryptMode::None depending on type.
> +    pub fn is_blob_magic(magic: &[u8; 8]) -> Option<CryptMode> {
> +        if magic == &UNCOMPRESSED_BLOB_MAGIC_1_0 || magic == &COMPRESSED_BLOB_MAGIC_1_0 {
> +            Some(CryptMode::None)
> +        } else if magic == &ENCRYPTED_BLOB_MAGIC_1_0 || magic == &ENCR_COMPR_BLOB_MAGIC_1_0 {
> +            Some(CryptMode::Encrypt)
> +        } else {
> +            None
> +        }
> +    }
>  }
>  
>  /// Builder for chunk DataBlobs
> -- 
> 2.20.1
> 
> 
> 
> _______________________________________________
> pbs-devel mailing list
> pbs-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel




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

* Re: [pbs-devel] [RFC proxmox-backup] api: datastore: determine size and blob type if not in manifest
  2020-10-14 15:04 [pbs-devel] [RFC proxmox-backup] api: datastore: determine size and blob type if not in manifest Stefan Reiter
  2020-10-14 15:30 ` Thomas Lamprecht
  2020-10-15  4:25 ` Dietmar Maurer
@ 2020-10-15  7:07 ` Fabian Grünbichler
  2 siblings, 0 replies; 4+ messages in thread
From: Fabian Grünbichler @ 2020-10-15  7:07 UTC (permalink / raw)
  To: Proxmox Backup Server development discussion

On October 14, 2020 5:04 pm, Stefan Reiter wrote:
> Try to read the first 8 byte (DataBlobHeader magic) from any .blob file
> not mentioned in the manifest. This allows classifying the
> client.log.blob file as encrypted if it is, and showing the correct
> symbol in the GUI for it.
> 
> While already open, also determine the file size by seeking to End(0).
> 
> Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
> ---
> 
> I noticed that the GUI would show the "download" icon enabled for
> 'client.log.blob' files, even if they were encrypted. The button wouldn't work
> of course, since the server can't decode encrypted blobs, so nothing would
> happen except an error logged to the console.
> 
> The problem is that currently we don't know if a blob file not listed in the
> manifest is encrypted - but we can find out, since it's encoded into the header.
> Not sure if this is worth the open() and read() for every unknown blob file, but
> it would solve the problem...

couldn't we just store that information (once) when uploading the client log?

> 
> 
>  src/api2/admin/datastore.rs | 25 +++++++++++++++++++++++--
>  src/backup/data_blob.rs     | 12 ++++++++++++
>  2 files changed, 35 insertions(+), 2 deletions(-)
> 
> diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
> index 11223e6a..2a9ac469 100644
> --- a/src/api2/admin/datastore.rs
> +++ b/src/api2/admin/datastore.rs
> @@ -2,6 +2,7 @@ use std::collections::{HashSet, HashMap};
>  use std::ffi::OsStr;
>  use std::os::unix::ffi::OsStrExt;
>  use std::sync::{Arc, Mutex};
> +use std::io::{Read, Seek, SeekFrom};
>  
>  use anyhow::{bail, format_err, Error};
>  use futures::*;
> @@ -91,10 +92,30 @@ fn get_all_snapshot_files(
>  
>      for file in &info.files {
>          if file_set.contains(file) { continue; }
> +
> +        // Try to determine size and crypt mode for unknown blobs
> +        let mut crypt_mode = None;
> +        let mut size = None;
> +        if file.ends_with(".blob") {
> +            let mut path = store.snapshot_path(&info.backup_dir);
> +            path.push(file);
> +            if let Ok(mut file) = std::fs::File::open(path) {
> +                let mut buffer = [0u8; 8];
> +                if let Ok(n) = file.read(&mut buffer[..]) {
> +                    if n == buffer.len() {
> +                        crypt_mode = DataBlob::is_blob_magic(&buffer);
> +                    }
> +                }
> +                if let Ok(pos) = file.seek(SeekFrom::End(0)) {
> +                    size = Some(pos);
> +                }
> +            }
> +        }
> +
>          files.push(BackupContent {
>              filename: file.to_string(),
> -            size: None,
> -            crypt_mode: None,
> +            size,
> +            crypt_mode,
>          });
>      }
>  
> diff --git a/src/backup/data_blob.rs b/src/backup/data_blob.rs
> index 284dc243..626a5fe4 100644
> --- a/src/backup/data_blob.rs
> +++ b/src/backup/data_blob.rs
> @@ -321,6 +321,18 @@ impl DataBlob {
>  
>          Ok(())
>      }
> +
> +    /// Determine if the given value is a valid blob magic number.
> +    /// Returns CryptMode::Encrypt or CryptMode::None depending on type.
> +    pub fn is_blob_magic(magic: &[u8; 8]) -> Option<CryptMode> {
> +        if magic == &UNCOMPRESSED_BLOB_MAGIC_1_0 || magic == &COMPRESSED_BLOB_MAGIC_1_0 {
> +            Some(CryptMode::None)
> +        } else if magic == &ENCRYPTED_BLOB_MAGIC_1_0 || magic == &ENCR_COMPR_BLOB_MAGIC_1_0 {
> +            Some(CryptMode::Encrypt)
> +        } else {
> +            None
> +        }
> +    }
>  }
>  
>  /// Builder for chunk DataBlobs
> -- 
> 2.20.1
> 
> 
> 
> _______________________________________________
> pbs-devel mailing list
> pbs-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
> 
> 
> 




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

end of thread, other threads:[~2020-10-15  7:08 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-14 15:04 [pbs-devel] [RFC proxmox-backup] api: datastore: determine size and blob type if not in manifest Stefan Reiter
2020-10-14 15:30 ` Thomas Lamprecht
2020-10-15  4:25 ` Dietmar Maurer
2020-10-15  7:07 ` Fabian Grünbichler

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