* [pve-devel] [PATCH proxmox-backup-qemu] api: add master key support
2021-02-08 13:08 [pve-devel] [PATCH-SERIES 0/4] PBS master key integration Fabian Grünbichler
@ 2021-02-08 13:08 ` Fabian Grünbichler
2021-02-12 14:38 ` [pve-devel] applied: " Thomas Lamprecht
2021-02-08 13:08 ` [pve-devel] [PATCH qemu] pbs: " Fabian Grünbichler
` (3 subsequent siblings)
4 siblings, 1 reply; 13+ messages in thread
From: Fabian Grünbichler @ 2021-02-08 13:08 UTC (permalink / raw)
To: pve-devel
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
^ permalink raw reply [flat|nested] 13+ messages in thread* [pve-devel] applied: [PATCH proxmox-backup-qemu] api: add master key support
2021-02-08 13:08 ` [pve-devel] [PATCH proxmox-backup-qemu] api: add master key support Fabian Grünbichler
@ 2021-02-12 14:38 ` Thomas Lamprecht
0 siblings, 0 replies; 13+ messages in thread
From: Thomas Lamprecht @ 2021-02-12 14:38 UTC (permalink / raw)
To: Proxmox VE development discussion, Fabian Grünbichler
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!
^ permalink raw reply [flat|nested] 13+ messages in thread
* [pve-devel] [PATCH qemu] pbs: add master key support
2021-02-08 13:08 [pve-devel] [PATCH-SERIES 0/4] PBS master key integration Fabian Grünbichler
2021-02-08 13:08 ` [pve-devel] [PATCH proxmox-backup-qemu] api: add master key support Fabian Grünbichler
@ 2021-02-08 13:08 ` Fabian Grünbichler
2021-02-10 11:05 ` Stefan Reiter
2021-02-08 13:08 ` [pve-devel] [PATCH v2 storage] pbs: allow setting up a master key Fabian Grünbichler
` (2 subsequent siblings)
4 siblings, 1 reply; 13+ messages in thread
From: Fabian Grünbichler @ 2021-02-08 13:08 UTC (permalink / raw)
To: pve-devel
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
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [pve-devel] [PATCH qemu] pbs: add master key support
2021-02-08 13:08 ` [pve-devel] [PATCH qemu] pbs: " Fabian Grünbichler
@ 2021-02-10 11:05 ` Stefan Reiter
2021-02-10 12:52 ` Fabian Grünbichler
0 siblings, 1 reply; 13+ messages in thread
From: Stefan Reiter @ 2021-02-10 11:05 UTC (permalink / raw)
To: Proxmox VE development discussion, Fabian Grünbichler
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
>
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [pve-devel] [PATCH qemu] pbs: add master key support
2021-02-10 11:05 ` Stefan Reiter
@ 2021-02-10 12:52 ` Fabian Grünbichler
0 siblings, 0 replies; 13+ messages in thread
From: Fabian Grünbichler @ 2021-02-10 12:52 UTC (permalink / raw)
To: Proxmox VE development discussion, Stefan Reiter
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
>>
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [pve-devel] [PATCH v2 storage] pbs: allow setting up a master key
2021-02-08 13:08 [pve-devel] [PATCH-SERIES 0/4] PBS master key integration Fabian Grünbichler
2021-02-08 13:08 ` [pve-devel] [PATCH proxmox-backup-qemu] api: add master key support Fabian Grünbichler
2021-02-08 13:08 ` [pve-devel] [PATCH qemu] pbs: " Fabian Grünbichler
@ 2021-02-08 13:08 ` Fabian Grünbichler
2021-04-22 20:00 ` [pve-devel] applied: " Thomas Lamprecht
2021-02-08 13:08 ` [pve-devel] [PATCH qemu-server] vzdump: add master key support Fabian Grünbichler
2021-05-12 9:54 ` [pve-devel] [PATCH-SERIES 0/4] PBS master key integration Fabian Ebner
4 siblings, 1 reply; 13+ messages in thread
From: Fabian Grünbichler @ 2021-02-08 13:08 UTC (permalink / raw)
To: pve-devel
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
^ permalink raw reply [flat|nested] 13+ messages in thread* [pve-devel] applied: [PATCH v2 storage] pbs: allow setting up a master key
2021-02-08 13:08 ` [pve-devel] [PATCH v2 storage] pbs: allow setting up a master key Fabian Grünbichler
@ 2021-04-22 20:00 ` Thomas Lamprecht
0 siblings, 0 replies; 13+ messages in thread
From: Thomas Lamprecht @ 2021-04-22 20:00 UTC (permalink / raw)
To: Proxmox VE development discussion, Fabian Grünbichler
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.
^ permalink raw reply [flat|nested] 13+ messages in thread
* [pve-devel] [PATCH qemu-server] vzdump: add master key support
2021-02-08 13:08 [pve-devel] [PATCH-SERIES 0/4] PBS master key integration Fabian Grünbichler
` (2 preceding siblings ...)
2021-02-08 13:08 ` [pve-devel] [PATCH v2 storage] pbs: allow setting up a master key Fabian Grünbichler
@ 2021-02-08 13:08 ` Fabian Grünbichler
2021-05-28 11:50 ` Thomas Lamprecht
2021-05-12 9:54 ` [pve-devel] [PATCH-SERIES 0/4] PBS master key integration Fabian Ebner
4 siblings, 1 reply; 13+ messages in thread
From: Fabian Grünbichler @ 2021-02-08 13:08 UTC (permalink / raw)
To: pve-devel
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
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [pve-devel] [PATCH qemu-server] vzdump: add master key support
2021-02-08 13:08 ` [pve-devel] [PATCH qemu-server] vzdump: add master key support Fabian Grünbichler
@ 2021-05-28 11:50 ` Thomas Lamprecht
2021-05-28 12:09 ` [pve-devel] [PATCH REBASE " Fabian Grünbichler
0 siblings, 1 reply; 13+ messages in thread
From: Thomas Lamprecht @ 2021-05-28 11:50 UTC (permalink / raw)
To: Proxmox VE development discussion, Fabian Grünbichler
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;
> }
>
>
^ permalink raw reply [flat|nested] 13+ messages in thread* [pve-devel] [PATCH REBASE qemu-server] vzdump: add master key support
2021-05-28 11:50 ` Thomas Lamprecht
@ 2021-05-28 12:09 ` Fabian Grünbichler
2021-06-02 14:51 ` [pve-devel] applied: " Thomas Lamprecht
0 siblings, 1 reply; 13+ messages in thread
From: Fabian Grünbichler @ 2021-05-28 12:09 UTC (permalink / raw)
To: pve-devel
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
^ permalink raw reply [flat|nested] 13+ messages in thread* [pve-devel] applied: [PATCH REBASE qemu-server] vzdump: add master key support
2021-05-28 12:09 ` [pve-devel] [PATCH REBASE " Fabian Grünbichler
@ 2021-06-02 14:51 ` Thomas Lamprecht
0 siblings, 0 replies; 13+ messages in thread
From: Thomas Lamprecht @ 2021-06-02 14:51 UTC (permalink / raw)
To: Proxmox VE development discussion, Fabian Grünbichler
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.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [pve-devel] [PATCH-SERIES 0/4] PBS master key integration
2021-02-08 13:08 [pve-devel] [PATCH-SERIES 0/4] PBS master key integration Fabian Grünbichler
` (3 preceding siblings ...)
2021-02-08 13:08 ` [pve-devel] [PATCH qemu-server] vzdump: add master key support Fabian Grünbichler
@ 2021-05-12 9:54 ` Fabian Ebner
4 siblings, 0 replies; 13+ messages in thread
From: Fabian Ebner @ 2021-05-12 9:54 UTC (permalink / raw)
To: pve-devel, Fabian Grünbichler
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.
^ permalink raw reply [flat|nested] 13+ messages in thread