[pve-devel] [PATCH-SERIES 0/4] PBS master key integration

[pve-devel] [PATCH-SERIES 0/4] PBS master key integration

[Fabian Grünbichler]

this series enables master key integration for PVE->PBS, by allowing the
configuration of a per-storage master key which is used for (encrypted)
VM and CT backups.

while the diffs are small, the following bumps/dependencies/.. are required:

proxmox-backup needs a bump (commits from other series)
pve-storage needs a bump + a versioned-dep on proxmox-backup-client (new
CLI parameters)
qemu-server needs a versioned-dependency on bumped pve-storage (new
storage plugin methods)

qemu needs a bump + a versioned-dependency (build + RT) on
libproxmox-backup-qemu (API change)
libproxmox-backup-qemu needs a bump + breaks on pre-bump qemu (API
change)

it might make sense to queue the libproxmox-backup-qemu and
proxmox-backup bumps together (the former directly references git of the
latter at the moment, instead of a tag).

[pve-devel] [PATCH proxmox-backup-qemu] api: add master key support

[Fabian Grünbichler]

this is a breaking change/API extension.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---

Notes:
    requires appropriate Breaks on old pve-qemu-kvm, and versioned build and
    runtime dep from pve-qemu-kvm on bumped libproxmox-backup-qemu.
    
    backwards compat with outdated QEMU + lib versions is handled in qemu-server

 current-api.h   |  1 +
 src/backup.rs   | 45 ++++++++++++++++++++++++++++++++++++---------
 src/commands.rs | 10 ++++++++++
 src/lib.rs      |  9 +++++++++
 4 files changed, 56 insertions(+), 9 deletions(-)

diff --git a/current-api.h b/current-api.h
index abe7e89..ddf65d5 100644
--- a/current-api.h
+++ b/current-api.h
@@ -176,6 +176,7 @@ ProxmoxBackupHandle *proxmox_backup_new(const char *repo,
                                         const char *password,
                                         const char *keyfile,
                                         const char *key_password,
+                                        const char *master_keyfile,
                                         bool compress,
                                         bool encrypt,
                                         const char *fingerprint,
diff --git a/src/backup.rs b/src/backup.rs
index e2062e7..8f64979 100644
--- a/src/backup.rs
+++ b/src/backup.rs
@@ -8,9 +8,11 @@ use futures::future::{Future, Either, FutureExt};
 use tokio::runtime::Runtime;
 
 use proxmox_backup::tools::runtime::get_runtime_with_builder;
-use proxmox_backup::backup::{CryptConfig, CryptMode, BackupDir, BackupManifest, load_and_decrypt_key};
+use proxmox_backup::backup::{CryptConfig, CryptMode, BackupDir, BackupManifest, KeyConfig, load_and_decrypt_key, rsa_encrypt_key_config};
 use proxmox_backup::client::{HttpClient, HttpClientOptions, BackupWriter};
 
+use proxmox::tools::fs::file_get_contents;
+
 use super::BackupSetup;
 use crate::capi_types::*;
 use crate::registry::Registry;
@@ -22,6 +24,7 @@ pub(crate) struct BackupTask {
     compress: bool,
     crypt_mode: CryptMode,
     crypt_config: Option<Arc<CryptConfig>>,
+    rsa_encrypted_key: Option<Vec<u8>>,
     writer: OnceCell<Arc<BackupWriter>>,
     last_manifest: OnceCell<Arc<BackupManifest>>,
     manifest: Arc<Mutex<BackupManifest>>,
@@ -44,16 +47,28 @@ impl BackupTask {
         runtime: Arc<Runtime>
     ) -> Result<Self, Error> {
 
-        let crypt_config = match setup.keyfile {
-            None => None,
+        let (crypt_config, rsa_encrypted_key) = match setup.keyfile {
+            None => (None, None),
             Some(ref path) => {
-                let (key, _, _) = load_and_decrypt_key(path, & || {
+                let (key, created, _) = load_and_decrypt_key(path, & || {
                     match setup.key_password {
                         Some(ref key_password) => Ok(key_password.as_bytes().to_vec()),
                         None => bail!("no key_password specified"),
                     }
                 })?;
-                Some(Arc::new(CryptConfig::new(key)?))
+                let rsa_encrypted_key = match setup.master_keyfile {
+                    Some(ref master_keyfile) => {
+                        let pem = file_get_contents(master_keyfile)?;
+                        let rsa = openssl::rsa::Rsa::public_key_from_pem(&pem)?;
+
+                        let mut key_config = KeyConfig::without_password(key)?;
+                        key_config.created = created; // keep original value
+
+                        Some(rsa_encrypt_key_config(rsa, &key_config)?)
+                    },
+                    None => None,
+                };
+                (Some(Arc::new(CryptConfig::new(key)?)), rsa_encrypted_key)
             }
         };
 
@@ -65,10 +80,21 @@ impl BackupTask {
         let registry = Arc::new(Mutex::new(Registry::<ImageUploadInfo>::new()));
         let known_chunks = Arc::new(Mutex::new(HashSet::new()));
 
-        Ok(Self { runtime, setup, compress, crypt_mode, crypt_config, abort,
-                  registry, manifest, known_chunks,
-                  writer: OnceCell::new(), last_manifest: OnceCell::new(),
-                  aborted: OnceCell::new() })
+        Ok(Self {
+            runtime,
+            setup,
+            compress,
+            crypt_mode,
+            crypt_config,
+            rsa_encrypted_key,
+            abort,
+            registry,
+            manifest,
+            known_chunks,
+            writer: OnceCell::new(),
+            last_manifest: OnceCell::new(),
+            aborted: OnceCell::new()
+        })
     }
 
     pub fn new(setup: BackupSetup, compress: bool, crypt_mode: CryptMode) -> Result<Self, Error> {
@@ -258,6 +284,7 @@ impl BackupTask {
         let command_future = finish_backup(
             self.need_writer()?,
             self.crypt_config.clone(),
+            self.rsa_encrypted_key.clone(),
             Arc::clone(&self.manifest),
         );
 
diff --git a/src/commands.rs b/src/commands.rs
index c63c4f7..5fdf318 100644
--- a/src/commands.rs
+++ b/src/commands.rs
@@ -438,8 +438,18 @@ pub(crate) async fn write_data(
 pub(crate) async fn finish_backup(
     client: Arc<BackupWriter>,
     crypt_config: Option<Arc<CryptConfig>>,
+    rsa_encrypted_key: Option<Vec<u8>>,
     manifest: Arc<Mutex<BackupManifest>>,
 ) -> Result<c_int, Error> {
+    if let Some(rsa_encrypted_key) = rsa_encrypted_key {
+        let target = ENCRYPTED_KEY_BLOB_NAME;
+        let options = UploadOptions { compress: false, encrypt: false, ..UploadOptions::default() };
+        let stats = client
+            .upload_blob_from_data(rsa_encrypted_key, target, options)
+            .await?;
+        manifest.lock().unwrap().add_file(target.to_string(), stats.size, stats.csum, CryptMode::Encrypt)?;
+    };
+
 
     let manifest = {
         let guard = manifest.lock().unwrap();
diff --git a/src/lib.rs b/src/lib.rs
index fd7c064..05d7b58 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -137,6 +137,7 @@ pub(crate) struct BackupSetup {
     pub password: String,
     pub keyfile: Option<std::path::PathBuf>,
     pub key_password: Option<String>,
+    pub master_keyfile: Option<std::path::PathBuf>,
     pub fingerprint: Option<String>,
 }
 
@@ -208,6 +209,7 @@ pub extern "C" fn proxmox_backup_new(
     password: *const c_char,
     keyfile: *const c_char,
     key_password: *const c_char,
+    master_keyfile: *const c_char,
     compress: bool,
     encrypt: bool,
     fingerprint: *const c_char,
@@ -227,6 +229,11 @@ pub extern "C" fn proxmox_backup_new(
         let keyfile = tools::utf8_c_string(keyfile)?.map(std::path::PathBuf::from);
         let key_password = tools::utf8_c_string(key_password)?;
         let fingerprint = tools::utf8_c_string(fingerprint)?;
+        let master_keyfile = tools::utf8_c_string(master_keyfile)?.map(std::path::PathBuf::from);
+
+        if master_keyfile.is_some() && keyfile.is_none() {
+            return Err(format_err!("can't use master keyfile without keyfile"));
+        }
 
         let crypt_mode = if keyfile.is_some() {
             if encrypt { CryptMode::Encrypt } else {  CryptMode::SignOnly }
@@ -246,6 +253,7 @@ pub extern "C" fn proxmox_backup_new(
             backup_time: backup_time as i64,
             keyfile,
             key_password,
+            master_keyfile,
             fingerprint,
         };
 
@@ -720,6 +728,7 @@ pub extern "C" fn proxmox_restore_new(
             backup_time,
             keyfile,
             key_password,
+            master_keyfile: None,
             fingerprint,
         };
 
-- 
2.20.1

[pve-devel] [PATCH qemu] pbs: add master key support

[Fabian Grünbichler]

this requires a new enough libproxmox-backup-qemu0, and allows querying
from the PVE side to avoid QMP calls with unsupported parameters.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---

Notes:
    requires versioned build and runtime dep on libproxmox-backup-qemu with changed API for masterkey support

 .../pve/0059-pbs-backup-add-masterkey.patch   | 96 +++++++++++++++++++
 debian/patches/series                         |  1 +
 2 files changed, 97 insertions(+)
 create mode 100644 debian/patches/pve/0059-pbs-backup-add-masterkey.patch

diff --git a/debian/patches/pve/0059-pbs-backup-add-masterkey.patch b/debian/patches/pve/0059-pbs-backup-add-masterkey.patch
new file mode 100644
index 0000000..12fce05
--- /dev/null
+++ b/debian/patches/pve/0059-pbs-backup-add-masterkey.patch
@@ -0,0 +1,96 @@
+Index: qemu/pve-backup.c
+===================================================================
+--- qemu.orig/pve-backup.c
++++ qemu/pve-backup.c
+@@ -539,6 +539,8 @@ typedef struct QmpBackupTask {
+     const char *keyfile;
+     bool has_key_password;
+     const char *key_password;
++    bool has_master_keyfile;
++    const char *master_keyfile;
+     bool has_backup_id;
+     const char *backup_id;
+     bool has_backup_time;
+@@ -710,6 +712,7 @@ static void coroutine_fn pvebackup_co_pr
+             task->has_password ? task->password : NULL,
+             task->has_keyfile ? task->keyfile : NULL,
+             task->has_key_password ? task->key_password : NULL,
++            task->has_master_keyfile ? task->master_keyfile : NULL,
+             task->has_compress ? task->compress : true,
+             task->has_encrypt ? task->encrypt : task->has_keyfile,
+             task->has_fingerprint ? task->fingerprint : NULL,
+@@ -989,6 +992,7 @@ UuidInfo *qmp_backup(
+     bool has_password, const char *password,
+     bool has_keyfile, const char *keyfile,
+     bool has_key_password, const char *key_password,
++    bool has_master_keyfile, const char *master_keyfile,
+     bool has_fingerprint, const char *fingerprint,
+     bool has_backup_id, const char *backup_id,
+     bool has_backup_time, int64_t backup_time,
+@@ -1009,6 +1013,8 @@ UuidInfo *qmp_backup(
+         .keyfile = keyfile,
+         .has_key_password = has_key_password,
+         .key_password = key_password,
++        .has_master_keyfile = has_master_keyfile,
++        .master_keyfile = master_keyfile,
+         .has_fingerprint = has_fingerprint,
+         .fingerprint = fingerprint,
+         .has_backup_id = has_backup_id,
+@@ -1131,5 +1137,6 @@ ProxmoxSupportStatus *qmp_query_proxmox_
+     ret->pbs_dirty_bitmap = true;
+     ret->query_bitmap_info = true;
+     ret->pbs_dirty_bitmap_migration = true;
++    ret->pbs_masterkey = true;
+     return ret;
+ }
+Index: qemu/block/monitor/block-hmp-cmds.c
+===================================================================
+--- qemu.orig/block/monitor/block-hmp-cmds.c
++++ qemu/block/monitor/block-hmp-cmds.c
+@@ -1035,6 +1035,7 @@ void hmp_backup(Monitor *mon, const QDic
+         false, NULL, // PBS password
+         false, NULL, // PBS keyfile
+         false, NULL, // PBS key_password
++        false, NULL, // PBS master_keyfile
+         false, NULL, // PBS fingerprint
+         false, NULL, // PBS backup-id
+         false, 0, // PBS backup-time
+Index: qemu/qapi/block-core.json
+===================================================================
+--- qemu.orig/qapi/block-core.json
++++ qemu/qapi/block-core.json
+@@ -827,6 +827,8 @@
+ #
+ # @key-password: password for keyfile (optional for format 'pbs')
+ #
++# @master_keyfile: PEM-formatted master public keyfile (optional for format 'pbs')
++#
+ # @fingerprint: server cert fingerprint (optional for format 'pbs')
+ #
+ # @backup-id: backup ID (required for format 'pbs')
+@@ -846,6 +848,7 @@
+                                     '*password': 'str',
+                                     '*keyfile': 'str',
+                                     '*key-password': 'str',
++                                    '*master_keyfile': 'str',
+                                     '*fingerprint': 'str',
+                                     '*backup-id': 'str',
+                                     '*backup-time': 'int',
+@@ -895,6 +898,9 @@
+ #                              migration cap if this is false/unset may lead
+ #                              to crashes on migration!
+ #
++# @pbs-masterkey: True if the QMP backup call supports the 'master_keyfile'
++#                 parameter.
++#
+ # @pbs-library-version: Running version of libproxmox-backup-qemu0 library.
+ #
+ ##
+@@ -902,6 +908,7 @@
+   'data': { 'pbs-dirty-bitmap': 'bool',
+             'query-bitmap-info': 'bool',
+             'pbs-dirty-bitmap-migration': 'bool',
++            'pbs-masterkey': 'bool',
+             'pbs-library-version': 'str' } }
+ 
+ ##
diff --git a/debian/patches/series b/debian/patches/series
index 1ef7185..433efda 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -59,3 +59,4 @@ pve/0055-PVE-Migrate-dirty-bitmap-state-via-savevm.patch
 pve/0056-migration-block-dirty-bitmap-migrate-other-bitmaps-e.patch
 pve/0057-PVE-fix-aborting-multiple-CREATED-jobs-in-sequential.patch
 pve/0058-PVE-fall-back-to-open-iscsi-initiatorname.patch
+pve/0059-pbs-backup-add-masterkey.patch
-- 
2.20.1

[pve-devel] [PATCH v2 storage] pbs: allow setting up a master key

[Fabian Grünbichler]

similar to the existing encryption key handling, but without
auto-generation since we only have the public part here.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---

Notes:
    v2: only use master key for backup command

 PVE/API2/Storage/Config.pm |  2 +-
 PVE/CLI/pvesm.pm           | 14 +++++-
 PVE/Storage/PBSPlugin.pm   | 94 +++++++++++++++++++++++++++++++++++++-
 3 files changed, 106 insertions(+), 4 deletions(-)

diff --git a/PVE/API2/Storage/Config.pm b/PVE/API2/Storage/Config.pm
index 00abd13..ea655c5 100755
--- a/PVE/API2/Storage/Config.pm
+++ b/PVE/API2/Storage/Config.pm
@@ -112,7 +112,7 @@ __PACKAGE__->register_method ({
 	return &$api_storage_config($cfg, $param->{storage});
     }});
 
-my $sensitive_params = [qw(password encryption-key)];
+my $sensitive_params = [qw(password encryption-key master-pubkey)];
 
 __PACKAGE__->register_method ({
     name => 'create',
diff --git a/PVE/CLI/pvesm.pm b/PVE/CLI/pvesm.pm
index 3594774..fe147d9 100755
--- a/PVE/CLI/pvesm.pm
+++ b/PVE/CLI/pvesm.pm
@@ -6,6 +6,7 @@ use warnings;
 use POSIX qw(O_RDONLY O_WRONLY O_CREAT O_TRUNC);
 use Fcntl ':flock';
 use File::Path;
+use MIME::Base64 qw(encode_base64);
 
 use PVE::SafeSyslog;
 use PVE::Cluster;
@@ -50,13 +51,22 @@ sub param_mapping {
 	}
     };
 
+    my $master_key_map = {
+	name => 'master-pubkey',
+	desc => 'a file containing a PEM-formatted master public key',
+	func => sub {
+	    my ($value) = @_;
+	    return encode_base64(PVE::Tools::file_get_contents($value), '');
+	}
+    };
+
 
     my $mapping = {
 	'cifsscan' => [ $password_map ],
 	'cifs' => [ $password_map ],
 	'pbs' => [ $password_map ],
-	'create' => [ $password_map, $enc_key_map ],
-	'update' => [ $password_map, $enc_key_map ],
+	'create' => [ $password_map, $enc_key_map, $master_key_map ],
+	'update' => [ $password_map, $enc_key_map, $master_key_map ],
     };
     return $mapping->{$name};
 }
diff --git a/PVE/Storage/PBSPlugin.pm b/PVE/Storage/PBSPlugin.pm
index 6c6816e..e78c631 100644
--- a/PVE/Storage/PBSPlugin.pm
+++ b/PVE/Storage/PBSPlugin.pm
@@ -8,6 +8,7 @@ use warnings;
 use Fcntl qw(F_GETFD F_SETFD FD_CLOEXEC);
 use IO::File;
 use JSON;
+use MIME::Base64 qw(decode_base64);
 use POSIX qw(strftime ENOENT);
 
 use PVE::APIClient::LWP;
@@ -43,6 +44,10 @@ sub properties {
 	    description => "Encryption key. Use 'autogen' to generate one automatically without passphrase.",
 	    type => 'string',
 	},
+	'master-pubkey' => {
+	    description => "Base64-encoded, PEM-formatted public RSA key. Used tp encrypt a copy of the encryption-key which will be added to each encrypted backup.",
+	    type => 'string',
+	},
 	port => {
 	    description => "For non default port.",
 	    type => 'integer',
@@ -64,6 +69,7 @@ sub options {
 	username => { optional => 1 },
 	password => { optional => 1 },
 	'encryption-key' => { optional => 1 },
+	'master-pubkey' => { optional => 1 },
 	maxfiles => { optional => 1 },
 	'prune-backups' => { optional => 1 },
 	fingerprint => { optional => 1 },
@@ -153,6 +159,56 @@ sub pbs_open_encryption_key {
     return $keyfd;
 }
 
+sub pbs_master_pubkey_file_name {
+    my ($scfg, $storeid) = @_;
+
+    return "/etc/pve/priv/storage/${storeid}.master.pem";
+}
+
+sub pbs_set_master_pubkey {
+    my ($scfg, $storeid, $key) = @_;
+
+    my $pwfile = pbs_master_pubkey_file_name($scfg, $storeid);
+    mkdir "/etc/pve/priv/storage";
+
+    PVE::Tools::file_set_contents($pwfile, "$key\n");
+}
+
+sub pbs_delete_master_pubkey {
+    my ($scfg, $storeid) = @_;
+
+    my $pwfile = pbs_master_pubkey_file_name($scfg, $storeid);
+
+    if (!unlink $pwfile) {
+	return if $! == ENOENT;
+	die "failed to delete master public key! $!\n";
+    }
+    delete $scfg->{'master-pubkey'};
+}
+
+sub pbs_get_master_pubkey {
+    my ($scfg, $storeid) = @_;
+
+    my $pwfile = pbs_master_pubkey_file_name($scfg, $storeid);
+
+    return PVE::Tools::file_get_contents($pwfile);
+}
+
+# Returns a file handle if there is a master key, or `undef` if there is not. Dies on error.
+sub pbs_open_master_pubkey {
+    my ($scfg, $storeid) = @_;
+
+    my $master_pubkey_file = pbs_master_pubkey_file_name($scfg, $storeid);
+
+    my $keyfd;
+    if (!open($keyfd, '<', $master_pubkey_file)) {
+	return undef if $! == ENOENT;
+	die "failed to open master public key: $master_pubkey_file: $!\n";
+    }
+
+    return $keyfd;
+}
+
 sub print_volid {
     my ($storeid, $btype, $bid, $btime) = @_;
 
@@ -168,10 +224,15 @@ my $USE_CRYPT_PARAMS = {
     'upload-log' => 1,
 };
 
+my $USE_MASTER_KEY = {
+    backup => 1,
+};
+
 my sub do_raw_client_cmd {
     my ($scfg, $storeid, $client_cmd, $param, %opts) = @_;
 
     my $use_crypto = $USE_CRYPT_PARAMS->{$client_cmd};
+    my $use_master = $USE_MASTER_KEY->{$client_cmd};
 
     my $client_exe = '/usr/bin/proxmox-backup-client';
     die "executable not found '$client_exe'! Proxmox backup client not installed?\n"
@@ -188,7 +249,7 @@ my sub do_raw_client_cmd {
     push @$cmd, $client_exe, $client_cmd;
 
     # This must live in the top scope to not get closed before the `run_command`
-    my $keyfd;
+    my ($keyfd, $master_fd);
     if ($use_crypto) {
 	if (defined($keyfd = pbs_open_encryption_key($scfg, $storeid))) {
 	    my $flags = fcntl($keyfd, F_GETFD, 0)
@@ -196,6 +257,13 @@ my sub do_raw_client_cmd {
 	    fcntl($keyfd, F_SETFD, $flags & ~FD_CLOEXEC)
 		or die "failed to remove FD_CLOEXEC from encryption key file descriptor\n";
 	    push @$cmd, '--crypt-mode=encrypt', '--keyfd='.fileno($keyfd);
+	    if ($use_master && defined($master_fd = pbs_open_master_pubkey($scfg, $storeid))) {
+		my $flags = fcntl($master_fd, F_GETFD, 0)
+		    // die "failed to get file descriptor flags: $!\n";
+		fcntl($master_fd, F_SETFD, $flags & ~FD_CLOEXEC)
+		    or die "failed to remove FD_CLOEXEC from master public key file descriptor\n";
+		push @$cmd, '--master-pubkey-fd='.fileno($master_fd);
+	    }
 	} else {
 	    push @$cmd, '--crypt-mode=none';
 	}
@@ -394,6 +462,17 @@ sub on_add_hook {
 	pbs_delete_encryption_key($scfg, $storeid);
     }
 
+    if (defined(my $master_key = delete $param{'master-pubkey'})) {
+	die "'master-pubkey' can only be used together with 'encryption-key'\n"
+	    if !defined($scfg->{'encryption-key'});
+
+	my $decoded = decode_base64($master_key);
+	pbs_set_master_pubkey($scfg, $storeid, $decoded);
+	$scfg->{'master-pubkey'} = 1;
+    } else {
+	pbs_delete_master_pubkey($scfg, $storeid);
+    }
+
     return $res;
 }
 
@@ -427,6 +506,18 @@ sub on_update_hook {
 	    $scfg->{'encryption-key'} = $decoded_key->{fingerprint} || 1;
 	} else {
 	    pbs_delete_encryption_key($scfg, $storeid);
+	    delete $scfg->{'encryption-key'};
+	}
+    }
+
+    if (exists($param{'master-pubkey'})) {
+	if (defined(my $master_key = delete($param{'master-pubkey'}))) {
+	    my $decoded = decode_base64($master_key);
+
+	    pbs_set_master_pubkey($scfg, $storeid, $decoded);
+	    $scfg->{'master-pubkey'} = 1;
+	} else {
+	    pbs_delete_master_pubkey($scfg, $storeid);
 	}
     }
 
@@ -438,6 +529,7 @@ sub on_delete_hook {
 
     pbs_delete_password($scfg, $storeid);
     pbs_delete_encryption_key($scfg, $storeid);
+    pbs_delete_master_pubkey($scfg, $storeid);
 
     return;
 }
-- 
2.20.1

[pve-devel] [PATCH qemu-server] vzdump: add master key support

[Fabian Grünbichler]

running outdated VMs without master key support will generate a warning
but proceed with a backup without encrypted key upload.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---

Notes:
    requires libpve-storage-perl with master key support.

 PVE/VZDump/QemuServer.pm | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/PVE/VZDump/QemuServer.pm b/PVE/VZDump/QemuServer.pm
index b5e74d3..e3f785a 100644
--- a/PVE/VZDump/QemuServer.pm
+++ b/PVE/VZDump/QemuServer.pm
@@ -485,6 +485,7 @@ sub archive_pbs {
     my $repo = PVE::PBSClient::get_repository($scfg);
     my $password = PVE::Storage::PBSPlugin::pbs_get_password($scfg, $opts->{storage});
     my $keyfile = PVE::Storage::PBSPlugin::pbs_encryption_key_file_name($scfg, $opts->{storage});
+    my $master_keyfile = PVE::Storage::PBSPlugin::pbs_master_pubkey_file_name($scfg, $opts->{storage});
 
     my $diskcount = scalar(@{$task->{disks}});
     # proxmox-backup-client can only handle raw files and block devs
@@ -533,6 +534,12 @@ sub archive_pbs {
 	      . "sure you've installed the latest version and the VM has been restarted.\n";
 	}
 
+	if (!defined($qemu_support->{"pbs-masterkey"}) && -e $master_keyfile) {
+	    $self->loginfo("WARNING: backup target is configured with master key, but running QEMU version does not support master keys.");
+	    $self->loginfo("Please make sure you've installed the latest version and the VM has been restarted to use master key feature.");
+	    $master_keyfile = undef; # skip rest of master key handling below
+	}
+
 	my $fs_frozen = $self->qga_fs_freeze($task, $vmid);
 
 	my $params = {
@@ -551,7 +558,13 @@ sub archive_pbs {
 	    $self->loginfo("enabling encryption");
 	    $params->{keyfile} = $keyfile;
 	    $params->{encrypt} = JSON::true;
+	    if (defined($master_keyfile) && -e $master_keyfile) {
+		$self->loginfo("enabling master key feature");
+		$params->{"master-keyfile"} = $master_keyfile;
+	    }
 	} else {
+	    $self->loginfo("WARNING: backup target is configured with master key, but this backup is not encrypted - master key settings will be ignored!")
+		if defined($master_keyfile) && -e $master_keyfile;
 	    $params->{encrypt} = JSON::false;
 	}
 
-- 
2.20.1

Re: [pve-devel] [PATCH qemu] pbs: add master key support

[Stefan Reiter]

Patch looks good in general, but the added file does not follow our 
formatting for the other patches. I'd prefer to keep them the same, or 
at least applicable with 'git am'.

Since one of us is going to have to rebase anyway, I can also send along 
a fixed up version with v2 of my 5.2 series if you want.

On 08/02/2021 14:08, Fabian Grünbichler wrote:
> this requires a new enough libproxmox-backup-qemu0, and allows querying
> from the PVE side to avoid QMP calls with unsupported parameters.
> 
> Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
> ---
> 
> Notes:
>      requires versioned build and runtime dep on libproxmox-backup-qemu with changed API for masterkey support
> 
>   .../pve/0059-pbs-backup-add-masterkey.patch   | 96 +++++++++++++++++++
>   debian/patches/series                         |  1 +
>   2 files changed, 97 insertions(+)
>   create mode 100644 debian/patches/pve/0059-pbs-backup-add-masterkey.patch
> 
> diff --git a/debian/patches/pve/0059-pbs-backup-add-masterkey.patch b/debian/patches/pve/0059-pbs-backup-add-masterkey.patch
> new file mode 100644
> index 0000000..12fce05
> --- /dev/null
> +++ b/debian/patches/pve/0059-pbs-backup-add-masterkey.patch
> @@ -0,0 +1,96 @@
> +Index: qemu/pve-backup.c
> +===================================================================
> +--- qemu.orig/pve-backup.c
> ++++ qemu/pve-backup.c
> +@@ -539,6 +539,8 @@ typedef struct QmpBackupTask {
> +     const char *keyfile;
> +     bool has_key_password;
> +     const char *key_password;
> ++    bool has_master_keyfile;
> ++    const char *master_keyfile;
> +     bool has_backup_id;
> +     const char *backup_id;
> +     bool has_backup_time;
> +@@ -710,6 +712,7 @@ static void coroutine_fn pvebackup_co_pr
> +             task->has_password ? task->password : NULL,
> +             task->has_keyfile ? task->keyfile : NULL,
> +             task->has_key_password ? task->key_password : NULL,
> ++            task->has_master_keyfile ? task->master_keyfile : NULL,
> +             task->has_compress ? task->compress : true,
> +             task->has_encrypt ? task->encrypt : task->has_keyfile,
> +             task->has_fingerprint ? task->fingerprint : NULL,
> +@@ -989,6 +992,7 @@ UuidInfo *qmp_backup(
> +     bool has_password, const char *password,
> +     bool has_keyfile, const char *keyfile,
> +     bool has_key_password, const char *key_password,
> ++    bool has_master_keyfile, const char *master_keyfile,
> +     bool has_fingerprint, const char *fingerprint,
> +     bool has_backup_id, const char *backup_id,
> +     bool has_backup_time, int64_t backup_time,
> +@@ -1009,6 +1013,8 @@ UuidInfo *qmp_backup(
> +         .keyfile = keyfile,
> +         .has_key_password = has_key_password,
> +         .key_password = key_password,
> ++        .has_master_keyfile = has_master_keyfile,
> ++        .master_keyfile = master_keyfile,
> +         .has_fingerprint = has_fingerprint,
> +         .fingerprint = fingerprint,
> +         .has_backup_id = has_backup_id,
> +@@ -1131,5 +1137,6 @@ ProxmoxSupportStatus *qmp_query_proxmox_
> +     ret->pbs_dirty_bitmap = true;
> +     ret->query_bitmap_info = true;
> +     ret->pbs_dirty_bitmap_migration = true;
> ++    ret->pbs_masterkey = true;
> +     return ret;
> + }
> +Index: qemu/block/monitor/block-hmp-cmds.c
> +===================================================================
> +--- qemu.orig/block/monitor/block-hmp-cmds.c
> ++++ qemu/block/monitor/block-hmp-cmds.c
> +@@ -1035,6 +1035,7 @@ void hmp_backup(Monitor *mon, const QDic
> +         false, NULL, // PBS password
> +         false, NULL, // PBS keyfile
> +         false, NULL, // PBS key_password
> ++        false, NULL, // PBS master_keyfile
> +         false, NULL, // PBS fingerprint
> +         false, NULL, // PBS backup-id
> +         false, 0, // PBS backup-time
> +Index: qemu/qapi/block-core.json
> +===================================================================
> +--- qemu.orig/qapi/block-core.json
> ++++ qemu/qapi/block-core.json
> +@@ -827,6 +827,8 @@
> + #
> + # @key-password: password for keyfile (optional for format 'pbs')
> + #
> ++# @master_keyfile: PEM-formatted master public keyfile (optional for format 'pbs')
> ++#

please use master-keyfile with a dash

> + # @fingerprint: server cert fingerprint (optional for format 'pbs')
> + #
> + # @backup-id: backup ID (required for format 'pbs')
> +@@ -846,6 +848,7 @@
> +                                     '*password': 'str',
> +                                     '*keyfile': 'str',
> +                                     '*key-password': 'str',
> ++                                    '*master_keyfile': 'str',

here too

Upstream seems to use _ in some places, but at least keep it consistent 
in our code ;)

> +                                     '*fingerprint': 'str',
> +                                     '*backup-id': 'str',
> +                                     '*backup-time': 'int',
> +@@ -895,6 +898,9 @@
> + #                              migration cap if this is false/unset may lead
> + #                              to crashes on migration!
> + #
> ++# @pbs-masterkey: True if the QMP backup call supports the 'master_keyfile'
> ++#                 parameter.
> ++#
> + # @pbs-library-version: Running version of libproxmox-backup-qemu0 library.
> + #
> + ##
> +@@ -902,6 +908,7 @@
> +   'data': { 'pbs-dirty-bitmap': 'bool',
> +             'query-bitmap-info': 'bool',
> +             'pbs-dirty-bitmap-migration': 'bool',
> ++            'pbs-masterkey': 'bool',
> +             'pbs-library-version': 'str' } }
> +
> + ##
> diff --git a/debian/patches/series b/debian/patches/series
> index 1ef7185..433efda 100644
> --- a/debian/patches/series
> +++ b/debian/patches/series
> @@ -59,3 +59,4 @@ pve/0055-PVE-Migrate-dirty-bitmap-state-via-savevm.patch
>   pve/0056-migration-block-dirty-bitmap-migrate-other-bitmaps-e.patch
>   pve/0057-PVE-fix-aborting-multiple-CREATED-jobs-in-sequential.patch
>   pve/0058-PVE-fall-back-to-open-iscsi-initiatorname.patch
> +pve/0059-pbs-backup-add-masterkey.patch
> 

Re: [pve-devel] [PATCH qemu] pbs: add master key support

[Fabian Grünbichler]

On February 10, 2021 12:05 pm, Stefan Reiter wrote:
> Patch looks good in general, but the added file does not follow our 
> formatting for the other patches. I'd prefer to keep them the same, or 
> at least applicable with 'git am'.
> 
> Since one of us is going to have to rebase anyway, I can also send along 
> a fixed up version with v2 of my 5.2 series if you want.

no objections from my side. sorry for not converting before sending (I 
used quilt to apply all patches)

> 
> On 08/02/2021 14:08, Fabian Grünbichler wrote:
>> this requires a new enough libproxmox-backup-qemu0, and allows querying
>> from the PVE side to avoid QMP calls with unsupported parameters.
>> 
>> Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
>> ---
>> 
>> Notes:
>>      requires versioned build and runtime dep on libproxmox-backup-qemu with changed API for masterkey support
>> 
>>   .../pve/0059-pbs-backup-add-masterkey.patch   | 96 +++++++++++++++++++
>>   debian/patches/series                         |  1 +
>>   2 files changed, 97 insertions(+)
>>   create mode 100644 debian/patches/pve/0059-pbs-backup-add-masterkey.patch
>> 
>> diff --git a/debian/patches/pve/0059-pbs-backup-add-masterkey.patch b/debian/patches/pve/0059-pbs-backup-add-masterkey.patch
>> new file mode 100644
>> index 0000000..12fce05
>> --- /dev/null
>> +++ b/debian/patches/pve/0059-pbs-backup-add-masterkey.patch
>> @@ -0,0 +1,96 @@
>> +Index: qemu/pve-backup.c
>> +===================================================================
>> +--- qemu.orig/pve-backup.c
>> ++++ qemu/pve-backup.c
>> +@@ -539,6 +539,8 @@ typedef struct QmpBackupTask {
>> +     const char *keyfile;
>> +     bool has_key_password;
>> +     const char *key_password;
>> ++    bool has_master_keyfile;
>> ++    const char *master_keyfile;
>> +     bool has_backup_id;
>> +     const char *backup_id;
>> +     bool has_backup_time;
>> +@@ -710,6 +712,7 @@ static void coroutine_fn pvebackup_co_pr
>> +             task->has_password ? task->password : NULL,
>> +             task->has_keyfile ? task->keyfile : NULL,
>> +             task->has_key_password ? task->key_password : NULL,
>> ++            task->has_master_keyfile ? task->master_keyfile : NULL,
>> +             task->has_compress ? task->compress : true,
>> +             task->has_encrypt ? task->encrypt : task->has_keyfile,
>> +             task->has_fingerprint ? task->fingerprint : NULL,
>> +@@ -989,6 +992,7 @@ UuidInfo *qmp_backup(
>> +     bool has_password, const char *password,
>> +     bool has_keyfile, const char *keyfile,
>> +     bool has_key_password, const char *key_password,
>> ++    bool has_master_keyfile, const char *master_keyfile,
>> +     bool has_fingerprint, const char *fingerprint,
>> +     bool has_backup_id, const char *backup_id,
>> +     bool has_backup_time, int64_t backup_time,
>> +@@ -1009,6 +1013,8 @@ UuidInfo *qmp_backup(
>> +         .keyfile = keyfile,
>> +         .has_key_password = has_key_password,
>> +         .key_password = key_password,
>> ++        .has_master_keyfile = has_master_keyfile,
>> ++        .master_keyfile = master_keyfile,
>> +         .has_fingerprint = has_fingerprint,
>> +         .fingerprint = fingerprint,
>> +         .has_backup_id = has_backup_id,
>> +@@ -1131,5 +1137,6 @@ ProxmoxSupportStatus *qmp_query_proxmox_
>> +     ret->pbs_dirty_bitmap = true;
>> +     ret->query_bitmap_info = true;
>> +     ret->pbs_dirty_bitmap_migration = true;
>> ++    ret->pbs_masterkey = true;
>> +     return ret;
>> + }
>> +Index: qemu/block/monitor/block-hmp-cmds.c
>> +===================================================================
>> +--- qemu.orig/block/monitor/block-hmp-cmds.c
>> ++++ qemu/block/monitor/block-hmp-cmds.c
>> +@@ -1035,6 +1035,7 @@ void hmp_backup(Monitor *mon, const QDic
>> +         false, NULL, // PBS password
>> +         false, NULL, // PBS keyfile
>> +         false, NULL, // PBS key_password
>> ++        false, NULL, // PBS master_keyfile
>> +         false, NULL, // PBS fingerprint
>> +         false, NULL, // PBS backup-id
>> +         false, 0, // PBS backup-time
>> +Index: qemu/qapi/block-core.json
>> +===================================================================
>> +--- qemu.orig/qapi/block-core.json
>> ++++ qemu/qapi/block-core.json
>> +@@ -827,6 +827,8 @@
>> + #
>> + # @key-password: password for keyfile (optional for format 'pbs')
>> + #
>> ++# @master_keyfile: PEM-formatted master public keyfile (optional for format 'pbs')
>> ++#
> 
> please use master-keyfile with a dash
> 
>> + # @fingerprint: server cert fingerprint (optional for format 'pbs')
>> + #
>> + # @backup-id: backup ID (required for format 'pbs')
>> +@@ -846,6 +848,7 @@
>> +                                     '*password': 'str',
>> +                                     '*keyfile': 'str',
>> +                                     '*key-password': 'str',
>> ++                                    '*master_keyfile': 'str',
> 
> here too
> 
> Upstream seems to use _ in some places, but at least keep it consistent 
> in our code ;)
> 
>> +                                     '*fingerprint': 'str',
>> +                                     '*backup-id': 'str',
>> +                                     '*backup-time': 'int',
>> +@@ -895,6 +898,9 @@
>> + #                              migration cap if this is false/unset may lead
>> + #                              to crashes on migration!
>> + #
>> ++# @pbs-masterkey: True if the QMP backup call supports the 'master_keyfile'
>> ++#                 parameter.
>> ++#
>> + # @pbs-library-version: Running version of libproxmox-backup-qemu0 library.
>> + #
>> + ##
>> +@@ -902,6 +908,7 @@
>> +   'data': { 'pbs-dirty-bitmap': 'bool',
>> +             'query-bitmap-info': 'bool',
>> +             'pbs-dirty-bitmap-migration': 'bool',
>> ++            'pbs-masterkey': 'bool',
>> +             'pbs-library-version': 'str' } }
>> +
>> + ##
>> diff --git a/debian/patches/series b/debian/patches/series
>> index 1ef7185..433efda 100644
>> --- a/debian/patches/series
>> +++ b/debian/patches/series
>> @@ -59,3 +59,4 @@ pve/0055-PVE-Migrate-dirty-bitmap-state-via-savevm.patch
>>   pve/0056-migration-block-dirty-bitmap-migrate-other-bitmaps-e.patch
>>   pve/0057-PVE-fix-aborting-multiple-CREATED-jobs-in-sequential.patch
>>   pve/0058-PVE-fall-back-to-open-iscsi-initiatorname.patch
>> +pve/0059-pbs-backup-add-masterkey.patch
>> 
> 

[pve-devel] applied: [PATCH proxmox-backup-qemu] api: add master key support

[Thomas Lamprecht]

On 08.02.21 14:08, Fabian Grünbichler wrote:
> this is a breaking change/API extension.
> 
> Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
> ---
> 
> Notes:
>     requires appropriate Breaks on old pve-qemu-kvm, and versioned build and
>     runtime dep from pve-qemu-kvm on bumped libproxmox-backup-qemu.
>     

I recorded the break, the dep was solved by the symbol patches.

>     backwards compat with outdated QEMU + lib versions is handled in qemu-server
> 
>  current-api.h   |  1 +
>  src/backup.rs   | 45 ++++++++++++++++++++++++++++++++++++---------
>  src/commands.rs | 10 ++++++++++
>  src/lib.rs      |  9 +++++++++
>  4 files changed, 56 insertions(+), 9 deletions(-)
> 
>

applied, thanks!

[pve-devel] applied: [PATCH v2 storage] pbs: allow setting up a master key

[Thomas Lamprecht]

On 08.02.21 14:08, Fabian Grünbichler wrote:
> similar to the existing encryption key handling, but without
> auto-generation since we only have the public part here.
> 
> Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
> ---
> 
> Notes:
>     v2: only use master key for backup command
> 
>  PVE/API2/Storage/Config.pm |  2 +-
>  PVE/CLI/pvesm.pm           | 14 +++++-
>  PVE/Storage/PBSPlugin.pm   | 94 +++++++++++++++++++++++++++++++++++++-
>  3 files changed, 106 insertions(+), 4 deletions(-)
> 
>

applied, thanks! The web-interface would still need support for that, we could reuse
the existing fields and detect if it's a master key when a user uploads one there
(we would have the full value available), and change the param name for submission.

Re: [pve-devel] [PATCH-SERIES 0/4] PBS master key integration

[Fabian Ebner]

Am 08.02.21 um 14:08 schrieb Fabian Grünbichler:
> this series enables master key integration for PVE->PBS, by allowing the
> configuration of a per-storage master key which is used for (encrypted)
> VM and CT backups.
> 
> while the diffs are small, the following bumps/dependencies/.. are required:
> 
> proxmox-backup needs a bump (commits from other series)
> pve-storage needs a bump + a versioned-dep on proxmox-backup-client (new
> CLI parameters)
> qemu-server needs a versioned-dependency on bumped pve-storage (new
> storage plugin methods)
> 
> qemu needs a bump + a versioned-dependency (build + RT) on
> libproxmox-backup-qemu (API change)
> libproxmox-backup-qemu needs a bump + breaks on pre-bump qemu (API
> change)
> 
> it might make sense to queue the libproxmox-backup-qemu and
> proxmox-backup bumps together (the former directly references git of the
> latter at the moment, instead of a tag).
> 
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 

Tried out the series and works as advertised. Test log with a few nits 
(one of them not even about this series, but the PBS docs):


For the storage parameter in the man page:

--master-pubkey a file containing a PEM-formatted master public key 
     Base64-encoded, PEM-formatted public RSA key. Used tp encrypt a 
copy of the encryption-key which will be added to each encrypted backup.

The description might be confusing, as one might wrongly think the file 
has to be base64 encoded. Also, there's a typo: "Used tp encrypt".


Tested storage adding/update/removal with master-pubkey. The update hook 
does not check if there's an encryption key, but the add hook does. 
Maybe we can also check that the file's contents are actually a PEM 
public key?


Tested backup+restore of a VM, with
1) downgraded pve-qemu-kvm=5.1.0-8 libproxmox-backup-qemu0=1.0.2-1
2) storage with master-pubkey but no encryption key
Warnings show up in the log and master key is not used as expected.


Tested backup+restore of a VM and a CT with correctly configured 
storage, also worked as expected.


Removed the encryption key on the storage, restore fails.

Restored the key from the backup with a master key as described in the 
PBS docs. Well, except for using '--kdf none' in the command below, so I 
could actually upload the keyfile to the storage again without running 
into an "Error: no password input mechanism available". From the PBS docs:

6. Then, use the previously generated master key to decrypt the file:

# proxmox-backup-client key import-with-master-key /path/to/target 
--master-keyfile /path/to/master-private.pem --encrypted-keyfile 
/path/to/rsa-encrypted.key

7. The target file will now contain the encryption key information in 
plain text. The success of this can be confirmed by passing the 
resulting json file, with the --keyfile parameter, when decrypting files 
from the backup.

Maybe we should mention something about the kdf, as the "file will now 
contain the encryption key information in plain text" is a bit 
misleading. Technically true, the information about the key is in plain 
text, but not the key itself ;)

Now, restoring worked again. Also recovered the key from the CT backup 
and checked that it matched.

Re: [pve-devel] [PATCH qemu-server] vzdump: add master key support

[Thomas Lamprecht]

On 08.02.21 14:08, Fabian Grünbichler wrote:
> running outdated VMs without master key support will generate a warning
> but proceed with a backup without encrypted key upload.
> 
> Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
> ---
> 
> Notes:
>     requires libpve-storage-perl with master key support.
> 

needs a rebase

>  PVE/VZDump/QemuServer.pm | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/PVE/VZDump/QemuServer.pm b/PVE/VZDump/QemuServer.pm
> index b5e74d3..e3f785a 100644
> --- a/PVE/VZDump/QemuServer.pm
> +++ b/PVE/VZDump/QemuServer.pm
> @@ -485,6 +485,7 @@ sub archive_pbs {
>      my $repo = PVE::PBSClient::get_repository($scfg);
>      my $password = PVE::Storage::PBSPlugin::pbs_get_password($scfg, $opts->{storage});
>      my $keyfile = PVE::Storage::PBSPlugin::pbs_encryption_key_file_name($scfg, $opts->{storage});
> +    my $master_keyfile = PVE::Storage::PBSPlugin::pbs_master_pubkey_file_name($scfg, $opts->{storage});
>  
>      my $diskcount = scalar(@{$task->{disks}});
>      # proxmox-backup-client can only handle raw files and block devs
> @@ -533,6 +534,12 @@ sub archive_pbs {
>  	      . "sure you've installed the latest version and the VM has been restarted.\n";
>  	}
>  
> +	if (!defined($qemu_support->{"pbs-masterkey"}) && -e $master_keyfile) {
> +	    $self->loginfo("WARNING: backup target is configured with master key, but running QEMU version does not support master keys.");
> +	    $self->loginfo("Please make sure you've installed the latest version and the VM has been restarted to use master key feature.");
> +	    $master_keyfile = undef; # skip rest of master key handling below
> +	}
> +
>  	my $fs_frozen = $self->qga_fs_freeze($task, $vmid);
>  
>  	my $params = {
> @@ -551,7 +558,13 @@ sub archive_pbs {
>  	    $self->loginfo("enabling encryption");
>  	    $params->{keyfile} = $keyfile;
>  	    $params->{encrypt} = JSON::true;
> +	    if (defined($master_keyfile) && -e $master_keyfile) {
> +		$self->loginfo("enabling master key feature");
> +		$params->{"master-keyfile"} = $master_keyfile;
> +	    }
>  	} else {
> +	    $self->loginfo("WARNING: backup target is configured with master key, but this backup is not encrypted - master key settings will be ignored!")
> +		if defined($master_keyfile) && -e $master_keyfile;
>  	    $params->{encrypt} = JSON::false;
>  	}
>  
> 

[pve-devel] [PATCH REBASE qemu-server] vzdump: add master key support

[Fabian Grünbichler]

running outdated VMs without master key support will generate a warning
but proceed with a backup without encrypted key upload.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
context in second hunk changed..

 PVE/VZDump/QemuServer.pm | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/PVE/VZDump/QemuServer.pm b/PVE/VZDump/QemuServer.pm
index 42a60fc..34ca624 100644
--- a/PVE/VZDump/QemuServer.pm
+++ b/PVE/VZDump/QemuServer.pm
@@ -447,6 +447,7 @@ sub archive_pbs {
     my $repo = PVE::PBSClient::get_repository($scfg);
     my $password = PVE::Storage::PBSPlugin::pbs_get_password($scfg, $opts->{storage});
     my $keyfile = PVE::Storage::PBSPlugin::pbs_encryption_key_file_name($scfg, $opts->{storage});
+    my $master_keyfile = PVE::Storage::PBSPlugin::pbs_master_pubkey_file_name($scfg, $opts->{storage});
 
     my $diskcount = scalar(@{$task->{disks}});
     # proxmox-backup-client can only handle raw files and block devs
@@ -501,6 +502,12 @@ sub archive_pbs {
 	    }
 	}
 
+	if (!defined($qemu_support->{"pbs-masterkey"}) && -e $master_keyfile) {
+	    $self->loginfo("WARNING: backup target is configured with master key, but running QEMU version does not support master keys.");
+	    $self->loginfo("Please make sure you've installed the latest version and the VM has been restarted to use master key feature.");
+	    $master_keyfile = undef; # skip rest of master key handling below
+	}
+
 	my $fs_frozen = $self->qga_fs_freeze($task, $vmid);
 
 	my $params = {
@@ -519,7 +526,13 @@ sub archive_pbs {
 	    $self->loginfo("enabling encryption");
 	    $params->{keyfile} = $keyfile;
 	    $params->{encrypt} = JSON::true;
+	    if (defined($master_keyfile) && -e $master_keyfile) {
+		$self->loginfo("enabling master key feature");
+		$params->{"master-keyfile"} = $master_keyfile;
+	    }
 	} else {
+	    $self->loginfo("WARNING: backup target is configured with master key, but this backup is not encrypted - master key settings will be ignored!")
+		if defined($master_keyfile) && -e $master_keyfile;
 	    $params->{encrypt} = JSON::false;
 	}
 
-- 
2.20.1

[pve-devel] applied: [PATCH REBASE qemu-server] vzdump: add master key support

[Thomas Lamprecht]

On 28.05.21 14:09, Fabian Grünbichler wrote:
> running outdated VMs without master key support will generate a warning
> but proceed with a backup without encrypted key upload.
> 
> Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
> ---
> context in second hunk changed..
> 
>  PVE/VZDump/QemuServer.pm | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
>

applied, thanks!

But please avoid replying patches to an existing thread, I almost overlooked this one.
Also, a rebase is just a vX+1, no need for REBASE/RESEND or other new inventions,
versions are way better for tracking and one can just note that it's only rebased
in the diff-stat area as always.