* [pbs-devel] [PATCH proxmox-backup 00/11] extend master key feature
@ 2021-02-05 15:35 Fabian Grünbichler
2021-02-05 15:35 ` [pbs-devel] [PATCH proxmox-backup 01/10] key: make 'default' master key explicit Fabian Grünbichler
` (10 more replies)
0 siblings, 11 replies; 14+ messages in thread
From: Fabian Grünbichler @ 2021-02-05 15:35 UTC (permalink / raw)
To: pbs-devel
this series extends the existing master key feature:
- add additional 'show' command and print more info
- allow specifying master key file or FD on client command line
- setup PBS in PVE to use per-storage master key (LXC only as PoC)
and adds tests + cleanups to the crypto parameter handling.
Qemu will require some additional changes and a versioned break/depends
between the lib and qemu packages, as it does not use the client for the
actual backup part.. I'll write and send those next week unless this
series gets rejected altogether ;)
^ permalink raw reply [flat|nested] 14+ messages in thread
* [pbs-devel] [PATCH proxmox-backup 01/10] key: make 'default' master key explicit
2021-02-05 15:35 [pbs-devel] [PATCH proxmox-backup 00/11] extend master key feature Fabian Grünbichler
@ 2021-02-05 15:35 ` Fabian Grünbichler
2021-02-05 15:35 ` [pbs-devel] [PATCH storage] pbs: allow setting up a master key Fabian Grünbichler
` (9 subsequent siblings)
10 siblings, 0 replies; 14+ messages in thread
From: Fabian Grünbichler @ 2021-02-05 15:35 UTC (permalink / raw)
To: pbs-devel
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
src/bin/proxmox-backup-client.rs | 2 +-
src/bin/proxmox_backup_client/key.rs | 15 +++++++++------
2 files changed, 10 insertions(+), 7 deletions(-)
diff --git a/src/bin/proxmox-backup-client.rs b/src/bin/proxmox-backup-client.rs
index dfb944f0..58f8740d 100644
--- a/src/bin/proxmox-backup-client.rs
+++ b/src/bin/proxmox-backup-client.rs
@@ -896,7 +896,7 @@ async fn create_backup(
let crypt_config = CryptConfig::new(key)?;
- match key::find_master_pubkey()? {
+ match key::find_default_master_pubkey()? {
Some(ref path) if path.exists() => {
let pem_data = file_get_contents(path)?;
let rsa = openssl::rsa::Rsa::public_key_from_pem(&pem_data)?;
diff --git a/src/bin/proxmox_backup_client/key.rs b/src/bin/proxmox_backup_client/key.rs
index 405cb818..037ee0eb 100644
--- a/src/bin/proxmox_backup_client/key.rs
+++ b/src/bin/proxmox_backup_client/key.rs
@@ -34,14 +34,14 @@ use proxmox_backup::{
};
pub const DEFAULT_ENCRYPTION_KEY_FILE_NAME: &str = "encryption-key.json";
-pub const MASTER_PUBKEY_FILE_NAME: &str = "master-public.pem";
+pub const DEFAULT_MASTER_PUBKEY_FILE_NAME: &str = "master-public.pem";
-pub fn find_master_pubkey() -> Result<Option<PathBuf>, Error> {
- super::find_xdg_file(MASTER_PUBKEY_FILE_NAME, "main public key file")
+pub fn find_default_master_pubkey() -> Result<Option<PathBuf>, Error> {
+ super::find_xdg_file(DEFAULT_MASTER_PUBKEY_FILE_NAME, "default master public key file")
}
-pub fn place_master_pubkey() -> Result<PathBuf, Error> {
- super::place_xdg_file(MASTER_PUBKEY_FILE_NAME, "main public key file")
+pub fn place_default_master_pubkey() -> Result<PathBuf, Error> {
+ super::place_xdg_file(DEFAULT_MASTER_PUBKEY_FILE_NAME, "default master public key file")
}
pub fn find_default_encryption_key() -> Result<Option<PathBuf>, Error> {
@@ -360,6 +360,9 @@ fn show_key(path: Option<String>, param: Value) -> Result<(), Error> {
)]
/// Import an RSA public key used to put an encrypted version of the symmetric backup encryption
/// key onto the backup server along with each backup.
+///
+/// The imported key will be used as default master key for future invocations by the same local
+/// user.
fn import_master_pubkey(path: String) -> Result<(), Error> {
let pem_data = file_get_contents(&path)?;
@@ -367,7 +370,7 @@ fn import_master_pubkey(path: String) -> Result<(), Error> {
bail!("Unable to decode PEM data - {}", err);
}
- let target_path = place_master_pubkey()?;
+ let target_path = place_default_master_pubkey()?;
replace_file(&target_path, &pem_data, CreateOptions::new())?;
--
2.20.1
^ permalink raw reply [flat|nested] 14+ messages in thread
* [pbs-devel] [PATCH storage] pbs: allow setting up a master key
2021-02-05 15:35 [pbs-devel] [PATCH proxmox-backup 00/11] extend master key feature Fabian Grünbichler
2021-02-05 15:35 ` [pbs-devel] [PATCH proxmox-backup 01/10] key: make 'default' master key explicit Fabian Grünbichler
@ 2021-02-05 15:35 ` Fabian Grünbichler
2021-02-05 15:35 ` [pbs-devel] [PATCH proxmox-backup 02/10] key: add show-master-pubkey command Fabian Grünbichler
` (8 subsequent siblings)
10 siblings, 0 replies; 14+ messages in thread
From: Fabian Grünbichler @ 2021-02-05 15:35 UTC (permalink / raw)
To: pbs-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>
---
requires a versioned dep on PBS client supporting these features.
PVE/API2/Storage/Config.pm | 2 +-
PVE/CLI/pvesm.pm | 14 +++++-
PVE/Storage/PBSPlugin.pm | 89 +++++++++++++++++++++++++++++++++++++-
3 files changed, 101 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..ba6e197 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 => "Public part of a master key pair. An encrypted copy of the encryption-key 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) = @_;
@@ -188,7 +244,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 +252,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 (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 +457,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 +501,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 +524,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] 14+ messages in thread
* [pbs-devel] [PATCH proxmox-backup 02/10] key: add show-master-pubkey command
2021-02-05 15:35 [pbs-devel] [PATCH proxmox-backup 00/11] extend master key feature Fabian Grünbichler
2021-02-05 15:35 ` [pbs-devel] [PATCH proxmox-backup 01/10] key: make 'default' master key explicit Fabian Grünbichler
2021-02-05 15:35 ` [pbs-devel] [PATCH storage] pbs: allow setting up a master key Fabian Grünbichler
@ 2021-02-05 15:35 ` Fabian Grünbichler
2021-02-05 15:35 ` [pbs-devel] [PATCH proxmox-backup 03/10] key: rustfmt module Fabian Grünbichler
` (7 subsequent siblings)
10 siblings, 0 replies; 14+ messages in thread
From: Fabian Grünbichler @ 2021-02-05 15:35 UTC (permalink / raw)
To: pbs-devel
and print public key when generating/importing..
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
src/api2/types/mod.rs | 32 +++++++++++
src/bin/proxmox_backup_client/key.rs | 82 ++++++++++++++++++++++++++--
2 files changed, 110 insertions(+), 4 deletions(-)
diff --git a/src/api2/types/mod.rs b/src/api2/types/mod.rs
index 1e239d27..5611c54c 100644
--- a/src/api2/types/mod.rs
+++ b/src/api2/types/mod.rs
@@ -1360,3 +1360,35 @@ pub struct KeyInfo {
#[serde(skip_serializing_if="Option::is_none")]
pub hint: Option<String>,
}
+
+#[api]
+#[derive(Deserialize, Serialize)]
+/// RSA public key information
+pub struct RsaPubKeyInfo {
+ /// Path to key (if stored in a file)
+ #[serde(skip_serializing_if="Option::is_none")]
+ pub path: Option<String>,
+ /// RSA exponent
+ pub exponent: String,
+ /// Hex-encoded RSA modulus
+ pub modulus: String,
+ /// Key (modulus) length in bits
+ pub length: usize,
+}
+
+impl std::convert::TryFrom<openssl::rsa::Rsa<openssl::pkey::Public>> for RsaPubKeyInfo {
+ type Error = anyhow::Error;
+
+ fn try_from(value: openssl::rsa::Rsa<openssl::pkey::Public>) -> Result<Self, Self::Error> {
+ let modulus = value.n().to_hex_str()?.to_string();
+ let exponent = value.e().to_dec_str()?.to_string();
+ let length = value.size() as usize * 8;
+
+ Ok(Self {
+ path: None,
+ exponent,
+ modulus,
+ length,
+ })
+ }
+}
diff --git a/src/bin/proxmox_backup_client/key.rs b/src/bin/proxmox_backup_client/key.rs
index 037ee0eb..43eaab5c 100644
--- a/src/bin/proxmox_backup_client/key.rs
+++ b/src/bin/proxmox_backup_client/key.rs
@@ -1,4 +1,5 @@
use std::path::PathBuf;
+use std::convert::TryFrom;
use anyhow::{bail, format_err, Error};
use serde_json::Value;
@@ -25,6 +26,7 @@ use proxmox_backup::{
PASSWORD_HINT_SCHEMA,
KeyInfo,
Kdf,
+ RsaPubKeyInfo,
},
backup::{
rsa_decrypt_key_config,
@@ -366,9 +368,16 @@ fn show_key(path: Option<String>, param: Value) -> Result<(), Error> {
fn import_master_pubkey(path: String) -> Result<(), Error> {
let pem_data = file_get_contents(&path)?;
- if let Err(err) = openssl::pkey::PKey::public_key_from_pem(&pem_data) {
- bail!("Unable to decode PEM data - {}", err);
- }
+ match openssl::pkey::PKey::public_key_from_pem(&pem_data) {
+ Ok(key) => {
+ let info = RsaPubKeyInfo::try_from(key.rsa()?)?;
+ println!("Found following key at {:?}", path);
+ println!("Modulus: {}", info.modulus);
+ println!("Exponent: {}", info.exponent);
+ println!("Length: {}", info.length);
+ },
+ Err(err) => bail!("Unable to decode PEM data - {}", err),
+ };
let target_path = place_default_master_pubkey()?;
@@ -388,7 +397,18 @@ fn create_master_key() -> Result<(), Error> {
bail!("unable to create master key - no tty");
}
- let rsa = openssl::rsa::Rsa::generate(4096)?;
+ let bits = 4096;
+ println!("Generating {}-bit RSA key..", bits);
+ let rsa = openssl::rsa::Rsa::generate(bits)?;
+ let public = openssl::rsa::Rsa::from_public_components(
+ rsa.n().to_owned()?,
+ rsa.e().to_owned()?,
+ )?;
+ let info = RsaPubKeyInfo::try_from(public)?;
+ println!("Modulus: {}", info.modulus);
+ println!("Exponent: {}", info.exponent);
+ println!();
+
let pkey = openssl::pkey::PKey::from_rsa(rsa)?;
let password = String::from_utf8(tty::read_and_verify_password("Master Key Password: ")?)?;
@@ -408,6 +428,56 @@ fn create_master_key() -> Result<(), Error> {
Ok(())
}
+#[api(
+ input: {
+ properties: {
+ path: {
+ description: "Path to the PEM formatted RSA public key. Default location will be used if not specified.",
+ optional: true,
+ },
+ "output-format": {
+ schema: OUTPUT_FORMAT,
+ optional: true,
+ },
+ },
+ },
+)]
+/// List information about master key
+fn show_master_pubkey(path: Option<String>, param: Value) -> Result<(), Error> {
+ let path = match path {
+ Some(path) => PathBuf::from(path),
+ None => find_default_master_pubkey()?
+ .ok_or_else(|| format_err!("No path specified and no default master key available."))?,
+ };
+
+ let path = path.canonicalize()?;
+
+ let output_format = get_output_format(¶m);
+
+ let pem_data = file_get_contents(path.clone())?;
+ let rsa = openssl::rsa::Rsa::public_key_from_pem(&pem_data)?;
+
+ let mut info = RsaPubKeyInfo::try_from(rsa)?;
+ info.path = Some(path.display().to_string());
+
+ let options = proxmox::api::cli::default_table_format_options()
+ .column(ColumnConfig::new("path"))
+ .column(ColumnConfig::new("modulus"))
+ .column(ColumnConfig::new("exponent"))
+ .column(ColumnConfig::new("length"));
+
+ let return_type = ReturnType::new(false, &RsaPubKeyInfo::API_SCHEMA);
+
+ format_and_print_result_full(
+ &mut serde_json::to_value(info)?,
+ &return_type,
+ &output_format,
+ &options,
+ );
+
+ Ok(())
+}
+
#[api(
input: {
properties: {
@@ -467,6 +537,9 @@ pub fn cli() -> CliCommandMap {
let key_import_master_pubkey_cmd_def = CliCommand::new(&API_METHOD_IMPORT_MASTER_PUBKEY)
.arg_param(&["path"])
.completion_cb("path", tools::complete_file_name);
+ let key_show_master_pubkey_cmd_def = CliCommand::new(&API_METHOD_SHOW_MASTER_PUBKEY)
+ .arg_param(&["path"])
+ .completion_cb("path", tools::complete_file_name);
let key_show_cmd_def = CliCommand::new(&API_METHOD_SHOW_KEY)
.arg_param(&["path"])
@@ -483,5 +556,6 @@ pub fn cli() -> CliCommandMap {
.insert("import-master-pubkey", key_import_master_pubkey_cmd_def)
.insert("change-passphrase", key_change_passphrase_cmd_def)
.insert("show", key_show_cmd_def)
+ .insert("show-master-pubkey", key_show_master_pubkey_cmd_def)
.insert("paperkey", paper_key_cmd_def)
}
--
2.20.1
^ permalink raw reply [flat|nested] 14+ messages in thread
* [pbs-devel] [PATCH proxmox-backup 03/10] key: rustfmt module
2021-02-05 15:35 [pbs-devel] [PATCH proxmox-backup 00/11] extend master key feature Fabian Grünbichler
` (2 preceding siblings ...)
2021-02-05 15:35 ` [pbs-devel] [PATCH proxmox-backup 02/10] key: add show-master-pubkey command Fabian Grünbichler
@ 2021-02-05 15:35 ` Fabian Grünbichler
2021-02-05 15:35 ` [pbs-devel] [PATCH proxmox-backup 04/10] client: add test for keyfile_parameters Fabian Grünbichler
` (6 subsequent siblings)
10 siblings, 0 replies; 14+ messages in thread
From: Fabian Grünbichler @ 2021-02-05 15:35 UTC (permalink / raw)
To: pbs-devel
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
src/bin/proxmox_backup_client/key.rs | 75 ++++++++++++----------------
1 file changed, 32 insertions(+), 43 deletions(-)
diff --git a/src/bin/proxmox_backup_client/key.rs b/src/bin/proxmox_backup_client/key.rs
index 43eaab5c..8a55e1ab 100644
--- a/src/bin/proxmox_backup_client/key.rs
+++ b/src/bin/proxmox_backup_client/key.rs
@@ -1,16 +1,12 @@
-use std::path::PathBuf;
use std::convert::TryFrom;
+use std::path::PathBuf;
use anyhow::{bail, format_err, Error};
use serde_json::Value;
use proxmox::api::api;
use proxmox::api::cli::{
- ColumnConfig,
- CliCommand,
- CliCommandMap,
- format_and_print_result_full,
- get_output_format,
+ format_and_print_result_full, get_output_format, CliCommand, CliCommandMap, ColumnConfig,
OUTPUT_FORMAT,
};
use proxmox::api::router::ReturnType;
@@ -18,40 +14,41 @@ use proxmox::sys::linux::tty;
use proxmox::tools::fs::{file_get_contents, replace_file, CreateOptions};
use proxmox_backup::{
- tools::paperkey::{
- PaperkeyFormat,
- generate_paper_key,
- },
- api2::types::{
- PASSWORD_HINT_SCHEMA,
- KeyInfo,
- Kdf,
- RsaPubKeyInfo,
- },
- backup::{
- rsa_decrypt_key_config,
- KeyConfig,
- },
+ api2::types::{Kdf, KeyInfo, RsaPubKeyInfo, PASSWORD_HINT_SCHEMA},
+ backup::{rsa_decrypt_key_config, KeyConfig},
tools,
+ tools::paperkey::{generate_paper_key, PaperkeyFormat},
};
pub const DEFAULT_ENCRYPTION_KEY_FILE_NAME: &str = "encryption-key.json";
pub const DEFAULT_MASTER_PUBKEY_FILE_NAME: &str = "master-public.pem";
pub fn find_default_master_pubkey() -> Result<Option<PathBuf>, Error> {
- super::find_xdg_file(DEFAULT_MASTER_PUBKEY_FILE_NAME, "default master public key file")
+ super::find_xdg_file(
+ DEFAULT_MASTER_PUBKEY_FILE_NAME,
+ "default master public key file",
+ )
}
pub fn place_default_master_pubkey() -> Result<PathBuf, Error> {
- super::place_xdg_file(DEFAULT_MASTER_PUBKEY_FILE_NAME, "default master public key file")
+ super::place_xdg_file(
+ DEFAULT_MASTER_PUBKEY_FILE_NAME,
+ "default master public key file",
+ )
}
pub fn find_default_encryption_key() -> Result<Option<PathBuf>, Error> {
- super::find_xdg_file(DEFAULT_ENCRYPTION_KEY_FILE_NAME, "default encryption key file")
+ super::find_xdg_file(
+ DEFAULT_ENCRYPTION_KEY_FILE_NAME,
+ "default encryption key file",
+ )
}
pub fn place_default_encryption_key() -> Result<PathBuf, Error> {
- super::place_xdg_file(DEFAULT_ENCRYPTION_KEY_FILE_NAME, "default encryption key file")
+ super::place_xdg_file(
+ DEFAULT_ENCRYPTION_KEY_FILE_NAME,
+ "default encryption key file",
+ )
}
pub fn read_optional_default_encryption_key() -> Result<Option<Vec<u8>>, Error> {
@@ -100,11 +97,7 @@ pub fn get_encryption_key_password() -> Result<Vec<u8>, Error> {
},
)]
/// Create a new encryption key.
-fn create(
- kdf: Option<Kdf>,
- path: Option<String>,
- hint: Option<String>
-) -> Result<(), Error> {
+fn create(kdf: Option<Kdf>, path: Option<String>, hint: Option<String>) -> Result<(), Error> {
let path = match path {
Some(path) => PathBuf::from(path),
None => {
@@ -196,8 +189,7 @@ async fn import_with_master_key(
let master_key = file_get_contents(&master_keyfile)?;
let password = tty::read_password("Master Key Password: ")?;
- let master_key =
- openssl::pkey::PKey::private_key_from_pem_passphrase(&master_key, &password)
+ let master_key = openssl::pkey::PKey::private_key_from_pem_passphrase(&master_key, &password)
.map_err(|err| format_err!("failed to read PEM-formatted private key - {}", err))?
.rsa()
.map_err(|err| format_err!("not a valid private RSA key - {}", err))?;
@@ -216,7 +208,6 @@ async fn import_with_master_key(
key_config.created = created; // keep original value
key_config.store(path, true)?;
-
}
Kdf::Scrypt | Kdf::PBKDF2 => {
let password = tty::read_and_verify_password("New Password: ")?;
@@ -259,10 +250,9 @@ fn change_passphrase(
let path = match path {
Some(path) => PathBuf::from(path),
None => {
- let path = find_default_encryption_key()?
- .ok_or_else(|| {
- format_err!("no encryption file provided and no default file found")
- })?;
+ let path = find_default_encryption_key()?.ok_or_else(|| {
+ format_err!("no encryption file provided and no default file found")
+ })?;
println!("updating default key at: {:?}", path);
path
}
@@ -284,7 +274,7 @@ fn change_passphrase(
}
let mut key_config = KeyConfig::without_password(key)?;
- key_config.created = created; // keep original value
+ key_config.created = created; // keep original value
key_config.store(&path, true)?;
}
@@ -375,7 +365,7 @@ fn import_master_pubkey(path: String) -> Result<(), Error> {
println!("Modulus: {}", info.modulus);
println!("Exponent: {}", info.exponent);
println!("Length: {}", info.length);
- },
+ }
Err(err) => bail!("Unable to decode PEM data - {}", err),
};
@@ -400,10 +390,8 @@ fn create_master_key() -> Result<(), Error> {
let bits = 4096;
println!("Generating {}-bit RSA key..", bits);
let rsa = openssl::rsa::Rsa::generate(bits)?;
- let public = openssl::rsa::Rsa::from_public_components(
- rsa.n().to_owned()?,
- rsa.e().to_owned()?,
- )?;
+ let public =
+ openssl::rsa::Rsa::from_public_components(rsa.n().to_owned()?, rsa.e().to_owned()?)?;
let info = RsaPubKeyInfo::try_from(public)?;
println!("Modulus: {}", info.modulus);
println!("Exponent: {}", info.exponent);
@@ -419,7 +407,8 @@ fn create_master_key() -> Result<(), Error> {
replace_file(filename_pub, pub_key.as_slice(), CreateOptions::new())?;
let cipher = openssl::symm::Cipher::aes_256_cbc();
- let priv_key: Vec<u8> = pkey.private_key_to_pem_pkcs8_passphrase(cipher, password.as_bytes())?;
+ let priv_key: Vec<u8> =
+ pkey.private_key_to_pem_pkcs8_passphrase(cipher, password.as_bytes())?;
let filename_priv = "master-private.pem";
println!("Writing private master key to {}", filename_priv);
--
2.20.1
^ permalink raw reply [flat|nested] 14+ messages in thread
* [pbs-devel] [PATCH proxmox-backup 04/10] client: add test for keyfile_parameters
2021-02-05 15:35 [pbs-devel] [PATCH proxmox-backup 00/11] extend master key feature Fabian Grünbichler
` (3 preceding siblings ...)
2021-02-05 15:35 ` [pbs-devel] [PATCH proxmox-backup 03/10] key: rustfmt module Fabian Grünbichler
@ 2021-02-05 15:35 ` Fabian Grünbichler
2021-02-06 8:00 ` Dietmar Maurer
2021-02-05 15:35 ` [pbs-devel] [PATCH proxmox-backup 05/10] client: refactor keyfile_parameters Fabian Grünbichler
` (5 subsequent siblings)
10 siblings, 1 reply; 14+ messages in thread
From: Fabian Grünbichler @ 2021-02-05 15:35 UTC (permalink / raw)
To: pbs-devel
this will get more complex soon, so add test to document current
behaviour.
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
src/bin/proxmox-backup-client.rs | 123 +++++++++++++++++++++++++++
src/bin/proxmox_backup_client/key.rs | 21 +++++
2 files changed, 144 insertions(+)
diff --git a/src/bin/proxmox-backup-client.rs b/src/bin/proxmox-backup-client.rs
index 58f8740d..4a783abe 100644
--- a/src/bin/proxmox-backup-client.rs
+++ b/src/bin/proxmox-backup-client.rs
@@ -675,6 +675,129 @@ fn keyfile_parameters(param: &Value) -> Result<(Option<Vec<u8>>, CryptMode), Err
})
}
+#[test]
+// WARNING: there must only be one test for keyfile_parameters as the default key handling is not
+// safe w.r.t. concurrency
+fn test_keyfile_parameters_handling() -> Result<(), Error> {
+ let some_key = Some(vec![1;1]);
+ let default_key = Some(vec![2;1]);
+
+ let no_key_res: (Option<Vec<u8>>, CryptMode) = (None, CryptMode::None);
+ let some_key_res = (some_key.clone(), CryptMode::Encrypt);
+ let some_key_sign_res = (some_key.clone(), CryptMode::SignOnly);
+ let default_key_res = (default_key.clone(), CryptMode::Encrypt);
+ let default_key_sign_res = (default_key.clone(), CryptMode::SignOnly);
+
+ let keypath = "./tests/keyfile.test";
+ replace_file(&keypath, some_key.as_ref().unwrap(), CreateOptions::default())?;
+ let invalid_keypath = "./tests/invalid_keyfile.test";
+
+ // no params, no default key == no key
+ let res = keyfile_parameters(&json!({}));
+ assert_eq!(res.unwrap(), no_key_res);
+
+ // keyfile param == key from keyfile
+ let res = keyfile_parameters(&json!({"keyfile": keypath}));
+ assert_eq!(res.unwrap(), some_key_res);
+
+ // crypt mode none == no key
+ let res = keyfile_parameters(&json!({"crypt-mode": "none"}));
+ assert_eq!(res.unwrap(), no_key_res);
+
+ // crypt mode encrypt/sign-only, no keyfile, no default key == Error
+ assert!(keyfile_parameters(&json!({"crypt-mode": "sign-only"})).is_err());
+ assert!(keyfile_parameters(&json!({"crypt-mode": "encrypt"})).is_err());
+
+ // crypt mode none with explicit key == Error
+ assert!(keyfile_parameters(&json!({"crypt-mode": "none", "keyfile": keypath})).is_err());
+
+ // crypt mode sign-only/encrypt with keyfile == key from keyfile with correct mode
+ let res = keyfile_parameters(&json!({"crypt-mode": "sign-only", "keyfile": keypath}));
+ assert_eq!(res.unwrap(), some_key_sign_res);
+ let res = keyfile_parameters(&json!({"crypt-mode": "encrypt", "keyfile": keypath}));
+ assert_eq!(res.unwrap(), some_key_res);
+
+ // invalid keyfile parameter always errors
+ assert!(keyfile_parameters(&json!({"keyfile": invalid_keypath})).is_err());
+ assert!(keyfile_parameters(&json!({"keyfile": invalid_keypath, "crypt-mode": "none"})).is_err());
+ assert!(keyfile_parameters(&json!({"keyfile": invalid_keypath, "crypt-mode": "sign-only"})).is_err());
+ assert!(keyfile_parameters(&json!({"keyfile": invalid_keypath, "crypt-mode": "encrypt"})).is_err());
+
+ // now set a default key
+ unsafe { key::set_test_encryption_key(Ok(default_key.clone())); }
+
+ // and repeat
+
+ // no params but default key == default key
+ let res = keyfile_parameters(&json!({}));
+ assert_eq!(res.unwrap(), default_key_res);
+
+ // keyfile param == key from keyfile
+ let res = keyfile_parameters(&json!({"keyfile": keypath}));
+ assert_eq!(res.unwrap(), some_key_res);
+
+ // crypt mode none == no key
+ let res = keyfile_parameters(&json!({"crypt-mode": "none"}));
+ assert_eq!(res.unwrap(), no_key_res);
+
+ // crypt mode encrypt/sign-only, no keyfile, default key == default key with correct mode
+ let res = keyfile_parameters(&json!({"crypt-mode": "sign-only"}));
+ assert_eq!(res.unwrap(), default_key_sign_res);
+ let res = keyfile_parameters(&json!({"crypt-mode": "encrypt"}));
+ assert_eq!(res.unwrap(), default_key_res);
+
+ // crypt mode none with explicit key == Error
+ assert!(keyfile_parameters(&json!({"crypt-mode": "none", "keyfile": keypath})).is_err());
+
+ // crypt mode sign-only/encrypt with keyfile == key from keyfile with correct mode
+ let res = keyfile_parameters(&json!({"crypt-mode": "sign-only", "keyfile": keypath}));
+ assert_eq!(res.unwrap(), some_key_sign_res);
+ let res = keyfile_parameters(&json!({"crypt-mode": "encrypt", "keyfile": keypath}));
+ assert_eq!(res.unwrap(), some_key_res);
+
+ // invalid keyfile parameter always errors
+ assert!(keyfile_parameters(&json!({"keyfile": invalid_keypath})).is_err());
+ assert!(keyfile_parameters(&json!({"keyfile": invalid_keypath, "crypt-mode": "none"})).is_err());
+ assert!(keyfile_parameters(&json!({"keyfile": invalid_keypath, "crypt-mode": "sign-only"})).is_err());
+ assert!(keyfile_parameters(&json!({"keyfile": invalid_keypath, "crypt-mode": "encrypt"})).is_err());
+
+ // now make default key retrieval error
+ unsafe { key::set_test_encryption_key(Err(format_err!("test error"))); }
+
+ // and repeat
+
+ // no params, default key retrieval errors == Error
+ assert!(keyfile_parameters(&json!({})).is_err());
+
+ // keyfile param == key from keyfile
+ let res = keyfile_parameters(&json!({"keyfile": keypath}));
+ assert_eq!(res.unwrap(), some_key_res);
+
+ // crypt mode none == no key
+ let res = keyfile_parameters(&json!({"crypt-mode": "none"}));
+ assert_eq!(res.unwrap(), no_key_res);
+
+ // crypt mode encrypt/sign-only, no keyfile, default key error == Error
+ assert!(keyfile_parameters(&json!({"crypt-mode": "sign-only"})).is_err());
+ assert!(keyfile_parameters(&json!({"crypt-mode": "encrypt"})).is_err());
+
+ // crypt mode none with explicit key == Error
+ assert!(keyfile_parameters(&json!({"crypt-mode": "none", "keyfile": keypath})).is_err());
+
+ // crypt mode sign-only/encrypt with keyfile == key from keyfile with correct mode
+ let res = keyfile_parameters(&json!({"crypt-mode": "sign-only", "keyfile": keypath}));
+ assert_eq!(res.unwrap(), some_key_sign_res);
+ let res = keyfile_parameters(&json!({"crypt-mode": "encrypt", "keyfile": keypath}));
+ assert_eq!(res.unwrap(), some_key_res);
+
+ // invalid keyfile parameter always errors
+ assert!(keyfile_parameters(&json!({"keyfile": invalid_keypath})).is_err());
+ assert!(keyfile_parameters(&json!({"keyfile": invalid_keypath, "crypt-mode": "none"})).is_err());
+ assert!(keyfile_parameters(&json!({"keyfile": invalid_keypath, "crypt-mode": "sign-only"})).is_err());
+ assert!(keyfile_parameters(&json!({"keyfile": invalid_keypath, "crypt-mode": "encrypt"})).is_err());
+ Ok(())
+}
+
#[api(
input: {
properties: {
diff --git a/src/bin/proxmox_backup_client/key.rs b/src/bin/proxmox_backup_client/key.rs
index 8a55e1ab..070f2a0b 100644
--- a/src/bin/proxmox_backup_client/key.rs
+++ b/src/bin/proxmox_backup_client/key.rs
@@ -51,12 +51,33 @@ pub fn place_default_encryption_key() -> Result<PathBuf, Error> {
)
}
+#[cfg(not(test))]
pub fn read_optional_default_encryption_key() -> Result<Option<Vec<u8>>, Error> {
find_default_encryption_key()?
.map(file_get_contents)
.transpose()
}
+#[cfg(test)]
+static mut TEST_DEFAULT_ENCRYPTION_KEY: Result<Option<Vec<u8>>, Error> = Ok(None);
+
+#[cfg(test)]
+pub fn read_optional_default_encryption_key() -> Result<Option<Vec<u8>>, Error> {
+ // not safe when multiple concurrent test cases end up here!
+ unsafe {
+ match &TEST_DEFAULT_ENCRYPTION_KEY {
+ Ok(key) => Ok(key.clone()),
+ Err(_) => bail!("test error"),
+ }
+ }
+}
+
+#[cfg(test)]
+// not safe when multiple concurrent test cases end up here!
+pub unsafe fn set_test_encryption_key(value: Result<Option<Vec<u8>>, Error>) {
+ TEST_DEFAULT_ENCRYPTION_KEY = value;
+}
+
pub fn get_encryption_key_password() -> Result<Vec<u8>, Error> {
// fixme: implement other input methods
--
2.20.1
^ permalink raw reply [flat|nested] 14+ messages in thread
* [pbs-devel] [PATCH proxmox-backup 05/10] client: refactor keyfile_parameters
2021-02-05 15:35 [pbs-devel] [PATCH proxmox-backup 00/11] extend master key feature Fabian Grünbichler
` (4 preceding siblings ...)
2021-02-05 15:35 ` [pbs-devel] [PATCH proxmox-backup 04/10] client: add test for keyfile_parameters Fabian Grünbichler
@ 2021-02-05 15:35 ` Fabian Grünbichler
2021-02-05 15:35 ` [pbs-devel] [PATCH proxmox-backup 06/10] client: allow passing specific master key Fabian Grünbichler
` (4 subsequent siblings)
10 siblings, 0 replies; 14+ messages in thread
From: Fabian Grünbichler @ 2021-02-05 15:35 UTC (permalink / raw)
To: pbs-devel
no semantic changes intended
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
src/bin/proxmox-backup-client.rs | 147 +++++++++++-----------
src/bin/proxmox_backup_client/catalog.rs | 10 +-
src/bin/proxmox_backup_client/snapshot.rs | 8 +-
3 files changed, 86 insertions(+), 79 deletions(-)
diff --git a/src/bin/proxmox-backup-client.rs b/src/bin/proxmox-backup-client.rs
index 4a783abe..8268098e 100644
--- a/src/bin/proxmox-backup-client.rs
+++ b/src/bin/proxmox-backup-client.rs
@@ -597,7 +597,13 @@ fn spawn_catalog_upload(
Ok(CatalogUploadResult { catalog_writer, result: catalog_result_rx })
}
-fn keyfile_parameters(param: &Value) -> Result<(Option<Vec<u8>>, CryptMode), Error> {
+#[derive(Debug, Eq, PartialEq)]
+struct CryptoParams {
+ mode: CryptMode,
+ enc_key: Option<Vec<u8>>,
+}
+
+fn crypto_parameters(param: &Value) -> Result<CryptoParams, Error> {
let keyfile = match param.get("keyfile") {
Some(Value::String(keyfile)) => Some(keyfile),
Some(_) => bail!("bad --keyfile parameter type"),
@@ -616,7 +622,7 @@ fn keyfile_parameters(param: &Value) -> Result<(Option<Vec<u8>>, CryptMode), Err
None => None,
};
- let crypt_mode: Option<CryptMode> = match param.get("crypt-mode") {
+ let mode: Option<CryptMode> = match param.get("crypt-mode") {
Some(mode) => Some(serde_json::from_value(mode.clone())?),
None => None,
};
@@ -640,30 +646,31 @@ fn keyfile_parameters(param: &Value) -> Result<(Option<Vec<u8>>, CryptMode), Err
}
};
- Ok(match (keydata, crypt_mode) {
+ Ok(match (keydata, mode) {
// no parameters:
(None, None) => match key::read_optional_default_encryption_key()? {
- Some(key) => {
+ None => CryptoParams { enc_key: None, mode: CryptMode::None },
+ enc_key => {
eprintln!("Encrypting with default encryption key!");
- (Some(key), CryptMode::Encrypt)
+ CryptoParams { enc_key, mode: CryptMode::Encrypt }
},
- None => (None, CryptMode::None),
},
// just --crypt-mode=none
- (None, Some(CryptMode::None)) => (None, CryptMode::None),
+ (None, Some(CryptMode::None)) => CryptoParams { enc_key: None, mode: CryptMode::None },
// just --crypt-mode other than none
- (None, Some(crypt_mode)) => match key::read_optional_default_encryption_key()? {
+ (None, Some(mode)) => match key::read_optional_default_encryption_key()? {
None => bail!("--crypt-mode without --keyfile and no default key file available"),
- Some(key) => {
+ enc_key => {
eprintln!("Encrypting with default encryption key!");
- (Some(key), crypt_mode)
+
+ CryptoParams { enc_key, mode }
},
}
// just --keyfile
- (Some(key), None) => (Some(key), CryptMode::Encrypt),
+ (enc_key, None) => CryptoParams { enc_key, mode: CryptMode::Encrypt },
// --keyfile and --crypt-mode=none
(Some(_), Some(CryptMode::None)) => {
@@ -671,57 +678,57 @@ fn keyfile_parameters(param: &Value) -> Result<(Option<Vec<u8>>, CryptMode), Err
}
// --keyfile and --crypt-mode other than none
- (Some(key), Some(crypt_mode)) => (Some(key), crypt_mode),
+ (enc_key, Some(mode)) => CryptoParams { enc_key, mode },
})
}
#[test]
-// WARNING: there must only be one test for keyfile_parameters as the default key handling is not
+// WARNING: there must only be one test for crypto_parameters as the default key handling is not
// safe w.r.t. concurrency
-fn test_keyfile_parameters_handling() -> Result<(), Error> {
+fn test_crypto_parameters_handling() -> Result<(), Error> {
let some_key = Some(vec![1;1]);
let default_key = Some(vec![2;1]);
- let no_key_res: (Option<Vec<u8>>, CryptMode) = (None, CryptMode::None);
- let some_key_res = (some_key.clone(), CryptMode::Encrypt);
- let some_key_sign_res = (some_key.clone(), CryptMode::SignOnly);
- let default_key_res = (default_key.clone(), CryptMode::Encrypt);
- let default_key_sign_res = (default_key.clone(), CryptMode::SignOnly);
+ let no_key_res = CryptoParams { enc_key: None, mode: CryptMode::None };
+ let some_key_res = CryptoParams { enc_key: some_key.clone(), mode: CryptMode::Encrypt };
+ let some_key_sign_res = CryptoParams { enc_key: some_key.clone(), mode: CryptMode::SignOnly };
+ let default_key_res = CryptoParams { enc_key: default_key.clone(), mode: CryptMode::Encrypt };
+ let default_key_sign_res = CryptoParams { enc_key: default_key.clone(), mode: CryptMode::SignOnly };
let keypath = "./tests/keyfile.test";
replace_file(&keypath, some_key.as_ref().unwrap(), CreateOptions::default())?;
let invalid_keypath = "./tests/invalid_keyfile.test";
// no params, no default key == no key
- let res = keyfile_parameters(&json!({}));
+ let res = crypto_parameters(&json!({}));
assert_eq!(res.unwrap(), no_key_res);
// keyfile param == key from keyfile
- let res = keyfile_parameters(&json!({"keyfile": keypath}));
+ let res = crypto_parameters(&json!({"keyfile": keypath}));
assert_eq!(res.unwrap(), some_key_res);
// crypt mode none == no key
- let res = keyfile_parameters(&json!({"crypt-mode": "none"}));
+ let res = crypto_parameters(&json!({"crypt-mode": "none"}));
assert_eq!(res.unwrap(), no_key_res);
// crypt mode encrypt/sign-only, no keyfile, no default key == Error
- assert!(keyfile_parameters(&json!({"crypt-mode": "sign-only"})).is_err());
- assert!(keyfile_parameters(&json!({"crypt-mode": "encrypt"})).is_err());
+ assert!(crypto_parameters(&json!({"crypt-mode": "sign-only"})).is_err());
+ assert!(crypto_parameters(&json!({"crypt-mode": "encrypt"})).is_err());
// crypt mode none with explicit key == Error
- assert!(keyfile_parameters(&json!({"crypt-mode": "none", "keyfile": keypath})).is_err());
+ assert!(crypto_parameters(&json!({"crypt-mode": "none", "keyfile": keypath})).is_err());
// crypt mode sign-only/encrypt with keyfile == key from keyfile with correct mode
- let res = keyfile_parameters(&json!({"crypt-mode": "sign-only", "keyfile": keypath}));
+ let res = crypto_parameters(&json!({"crypt-mode": "sign-only", "keyfile": keypath}));
assert_eq!(res.unwrap(), some_key_sign_res);
- let res = keyfile_parameters(&json!({"crypt-mode": "encrypt", "keyfile": keypath}));
+ let res = crypto_parameters(&json!({"crypt-mode": "encrypt", "keyfile": keypath}));
assert_eq!(res.unwrap(), some_key_res);
// invalid keyfile parameter always errors
- assert!(keyfile_parameters(&json!({"keyfile": invalid_keypath})).is_err());
- assert!(keyfile_parameters(&json!({"keyfile": invalid_keypath, "crypt-mode": "none"})).is_err());
- assert!(keyfile_parameters(&json!({"keyfile": invalid_keypath, "crypt-mode": "sign-only"})).is_err());
- assert!(keyfile_parameters(&json!({"keyfile": invalid_keypath, "crypt-mode": "encrypt"})).is_err());
+ assert!(crypto_parameters(&json!({"keyfile": invalid_keypath})).is_err());
+ assert!(crypto_parameters(&json!({"keyfile": invalid_keypath, "crypt-mode": "none"})).is_err());
+ assert!(crypto_parameters(&json!({"keyfile": invalid_keypath, "crypt-mode": "sign-only"})).is_err());
+ assert!(crypto_parameters(&json!({"keyfile": invalid_keypath, "crypt-mode": "encrypt"})).is_err());
// now set a default key
unsafe { key::set_test_encryption_key(Ok(default_key.clone())); }
@@ -729,37 +736,37 @@ fn test_keyfile_parameters_handling() -> Result<(), Error> {
// and repeat
// no params but default key == default key
- let res = keyfile_parameters(&json!({}));
+ let res = crypto_parameters(&json!({}));
assert_eq!(res.unwrap(), default_key_res);
// keyfile param == key from keyfile
- let res = keyfile_parameters(&json!({"keyfile": keypath}));
+ let res = crypto_parameters(&json!({"keyfile": keypath}));
assert_eq!(res.unwrap(), some_key_res);
// crypt mode none == no key
- let res = keyfile_parameters(&json!({"crypt-mode": "none"}));
+ let res = crypto_parameters(&json!({"crypt-mode": "none"}));
assert_eq!(res.unwrap(), no_key_res);
// crypt mode encrypt/sign-only, no keyfile, default key == default key with correct mode
- let res = keyfile_parameters(&json!({"crypt-mode": "sign-only"}));
+ let res = crypto_parameters(&json!({"crypt-mode": "sign-only"}));
assert_eq!(res.unwrap(), default_key_sign_res);
- let res = keyfile_parameters(&json!({"crypt-mode": "encrypt"}));
+ let res = crypto_parameters(&json!({"crypt-mode": "encrypt"}));
assert_eq!(res.unwrap(), default_key_res);
// crypt mode none with explicit key == Error
- assert!(keyfile_parameters(&json!({"crypt-mode": "none", "keyfile": keypath})).is_err());
+ assert!(crypto_parameters(&json!({"crypt-mode": "none", "keyfile": keypath})).is_err());
// crypt mode sign-only/encrypt with keyfile == key from keyfile with correct mode
- let res = keyfile_parameters(&json!({"crypt-mode": "sign-only", "keyfile": keypath}));
+ let res = crypto_parameters(&json!({"crypt-mode": "sign-only", "keyfile": keypath}));
assert_eq!(res.unwrap(), some_key_sign_res);
- let res = keyfile_parameters(&json!({"crypt-mode": "encrypt", "keyfile": keypath}));
+ let res = crypto_parameters(&json!({"crypt-mode": "encrypt", "keyfile": keypath}));
assert_eq!(res.unwrap(), some_key_res);
// invalid keyfile parameter always errors
- assert!(keyfile_parameters(&json!({"keyfile": invalid_keypath})).is_err());
- assert!(keyfile_parameters(&json!({"keyfile": invalid_keypath, "crypt-mode": "none"})).is_err());
- assert!(keyfile_parameters(&json!({"keyfile": invalid_keypath, "crypt-mode": "sign-only"})).is_err());
- assert!(keyfile_parameters(&json!({"keyfile": invalid_keypath, "crypt-mode": "encrypt"})).is_err());
+ assert!(crypto_parameters(&json!({"keyfile": invalid_keypath})).is_err());
+ assert!(crypto_parameters(&json!({"keyfile": invalid_keypath, "crypt-mode": "none"})).is_err());
+ assert!(crypto_parameters(&json!({"keyfile": invalid_keypath, "crypt-mode": "sign-only"})).is_err());
+ assert!(crypto_parameters(&json!({"keyfile": invalid_keypath, "crypt-mode": "encrypt"})).is_err());
// now make default key retrieval error
unsafe { key::set_test_encryption_key(Err(format_err!("test error"))); }
@@ -767,34 +774,34 @@ fn test_keyfile_parameters_handling() -> Result<(), Error> {
// and repeat
// no params, default key retrieval errors == Error
- assert!(keyfile_parameters(&json!({})).is_err());
+ assert!(crypto_parameters(&json!({})).is_err());
// keyfile param == key from keyfile
- let res = keyfile_parameters(&json!({"keyfile": keypath}));
+ let res = crypto_parameters(&json!({"keyfile": keypath}));
assert_eq!(res.unwrap(), some_key_res);
// crypt mode none == no key
- let res = keyfile_parameters(&json!({"crypt-mode": "none"}));
+ let res = crypto_parameters(&json!({"crypt-mode": "none"}));
assert_eq!(res.unwrap(), no_key_res);
// crypt mode encrypt/sign-only, no keyfile, default key error == Error
- assert!(keyfile_parameters(&json!({"crypt-mode": "sign-only"})).is_err());
- assert!(keyfile_parameters(&json!({"crypt-mode": "encrypt"})).is_err());
+ assert!(crypto_parameters(&json!({"crypt-mode": "sign-only"})).is_err());
+ assert!(crypto_parameters(&json!({"crypt-mode": "encrypt"})).is_err());
// crypt mode none with explicit key == Error
- assert!(keyfile_parameters(&json!({"crypt-mode": "none", "keyfile": keypath})).is_err());
+ assert!(crypto_parameters(&json!({"crypt-mode": "none", "keyfile": keypath})).is_err());
// crypt mode sign-only/encrypt with keyfile == key from keyfile with correct mode
- let res = keyfile_parameters(&json!({"crypt-mode": "sign-only", "keyfile": keypath}));
+ let res = crypto_parameters(&json!({"crypt-mode": "sign-only", "keyfile": keypath}));
assert_eq!(res.unwrap(), some_key_sign_res);
- let res = keyfile_parameters(&json!({"crypt-mode": "encrypt", "keyfile": keypath}));
+ let res = crypto_parameters(&json!({"crypt-mode": "encrypt", "keyfile": keypath}));
assert_eq!(res.unwrap(), some_key_res);
// invalid keyfile parameter always errors
- assert!(keyfile_parameters(&json!({"keyfile": invalid_keypath})).is_err());
- assert!(keyfile_parameters(&json!({"keyfile": invalid_keypath, "crypt-mode": "none"})).is_err());
- assert!(keyfile_parameters(&json!({"keyfile": invalid_keypath, "crypt-mode": "sign-only"})).is_err());
- assert!(keyfile_parameters(&json!({"keyfile": invalid_keypath, "crypt-mode": "encrypt"})).is_err());
+ assert!(crypto_parameters(&json!({"keyfile": invalid_keypath})).is_err());
+ assert!(crypto_parameters(&json!({"keyfile": invalid_keypath, "crypt-mode": "none"})).is_err());
+ assert!(crypto_parameters(&json!({"keyfile": invalid_keypath, "crypt-mode": "sign-only"})).is_err());
+ assert!(crypto_parameters(&json!({"keyfile": invalid_keypath, "crypt-mode": "encrypt"})).is_err());
Ok(())
}
@@ -906,7 +913,7 @@ async fn create_backup(
verify_chunk_size(size)?;
}
- let (keydata, crypt_mode) = keyfile_parameters(¶m)?;
+ let crypto = crypto_parameters(¶m)?;
let backup_id = param["backup-id"].as_str().unwrap_or(&proxmox::tools::nodename());
@@ -1011,7 +1018,7 @@ async fn create_backup(
println!("Starting backup protocol: {}", strftime_local("%c", epoch_i64())?);
- let (crypt_config, rsa_encrypted_key) = match keydata {
+ let (crypt_config, rsa_encrypted_key) = match crypto.enc_key {
None => (None, None),
Some(key) => {
let (key, created, fingerprint) = decrypt_key(&key, &key::get_encryption_key_password)?;
@@ -1097,7 +1104,7 @@ async fn create_backup(
BackupSpecificationType::CONFIG => {
let upload_options = UploadOptions {
compress: true,
- encrypt: crypt_mode == CryptMode::Encrypt,
+ encrypt: crypto.mode == CryptMode::Encrypt,
..UploadOptions::default()
};
@@ -1105,12 +1112,12 @@ async fn create_backup(
let stats = client
.upload_blob_from_file(&filename, &target, upload_options)
.await?;
- manifest.add_file(target, stats.size, stats.csum, crypt_mode)?;
+ manifest.add_file(target, stats.size, stats.csum, crypto.mode)?;
}
BackupSpecificationType::LOGFILE => { // fixme: remove - not needed anymore ?
let upload_options = UploadOptions {
compress: true,
- encrypt: crypt_mode == CryptMode::Encrypt,
+ encrypt: crypto.mode == CryptMode::Encrypt,
..UploadOptions::default()
};
@@ -1118,12 +1125,12 @@ async fn create_backup(
let stats = client
.upload_blob_from_file(&filename, &target, upload_options)
.await?;
- manifest.add_file(target, stats.size, stats.csum, crypt_mode)?;
+ manifest.add_file(target, stats.size, stats.csum, crypto.mode)?;
}
BackupSpecificationType::PXAR => {
// start catalog upload on first use
if catalog.is_none() {
- let catalog_upload_res = spawn_catalog_upload(client.clone(), crypt_mode == CryptMode::Encrypt)?;
+ let catalog_upload_res = spawn_catalog_upload(client.clone(), crypto.mode == CryptMode::Encrypt)?;
catalog = Some(catalog_upload_res.catalog_writer);
catalog_result_rx = Some(catalog_upload_res.result);
}
@@ -1143,7 +1150,7 @@ async fn create_backup(
let upload_options = UploadOptions {
previous_manifest: previous_manifest.clone(),
compress: true,
- encrypt: crypt_mode == CryptMode::Encrypt,
+ encrypt: crypto.mode == CryptMode::Encrypt,
..UploadOptions::default()
};
@@ -1156,7 +1163,7 @@ async fn create_backup(
pxar_options,
upload_options,
).await?;
- manifest.add_file(target, stats.size, stats.csum, crypt_mode)?;
+ manifest.add_file(target, stats.size, stats.csum, crypto.mode)?;
catalog.lock().unwrap().end_directory()?;
}
BackupSpecificationType::IMAGE => {
@@ -1166,7 +1173,7 @@ async fn create_backup(
previous_manifest: previous_manifest.clone(),
fixed_size: Some(size),
compress: true,
- encrypt: crypt_mode == CryptMode::Encrypt,
+ encrypt: crypto.mode == CryptMode::Encrypt,
};
let stats = backup_image(
@@ -1176,7 +1183,7 @@ async fn create_backup(
chunk_size_opt,
upload_options,
).await?;
- manifest.add_file(target, stats.size, stats.csum, crypt_mode)?;
+ manifest.add_file(target, stats.size, stats.csum, crypto.mode)?;
}
}
}
@@ -1193,7 +1200,7 @@ async fn create_backup(
if let Some(catalog_result_rx) = catalog_result_rx {
let stats = catalog_result_rx.await??;
- manifest.add_file(CATALOG_NAME.to_owned(), stats.size, stats.csum, crypt_mode)?;
+ manifest.add_file(CATALOG_NAME.to_owned(), stats.size, stats.csum, crypto.mode)?;
}
}
@@ -1204,7 +1211,7 @@ async fn create_backup(
let stats = client
.upload_blob_from_data(rsa_encrypted_key, target, options)
.await?;
- manifest.add_file(target.to_string(), stats.size, stats.csum, crypt_mode)?;
+ manifest.add_file(target.to_string(), stats.size, stats.csum, crypto.mode)?;
}
// create manifest (index.json)
@@ -1379,9 +1386,9 @@ async fn restore(param: Value) -> Result<Value, Error> {
let target = tools::required_string_param(¶m, "target")?;
let target = if target == "-" { None } else { Some(target) };
- let (keydata, _crypt_mode) = keyfile_parameters(¶m)?;
+ let crypto = crypto_parameters(¶m)?;
- let crypt_config = match keydata {
+ let crypt_config = match crypto.enc_key {
None => None,
Some(key) => {
let (key, _, fingerprint) = decrypt_key(&key, &key::get_encryption_key_password)?;
diff --git a/src/bin/proxmox_backup_client/catalog.rs b/src/bin/proxmox_backup_client/catalog.rs
index 61bcc57f..b69d9c37 100644
--- a/src/bin/proxmox_backup_client/catalog.rs
+++ b/src/bin/proxmox_backup_client/catalog.rs
@@ -16,7 +16,6 @@ use crate::{
KEYFD_SCHEMA,
extract_repository_from_value,
record_repository,
- keyfile_parameters,
key::get_encryption_key_password,
decrypt_key,
api_datastore_latest_snapshot,
@@ -25,6 +24,7 @@ use crate::{
complete_group_or_snapshot,
complete_pxar_archive_name,
connect,
+ crypto_parameters,
BackupDir,
BackupGroup,
BufferedDynamicReader,
@@ -68,9 +68,9 @@ async fn dump_catalog(param: Value) -> Result<Value, Error> {
let path = tools::required_string_param(¶m, "snapshot")?;
let snapshot: BackupDir = path.parse()?;
- let (keydata, _) = keyfile_parameters(¶m)?;
+ let crypto = crypto_parameters(¶m)?;
- let crypt_config = match keydata {
+ let crypt_config = match crypto.enc_key {
None => None,
Some(key) => {
let (key, _created, _fingerprint) = decrypt_key(&key, &get_encryption_key_password)?;
@@ -166,9 +166,9 @@ async fn catalog_shell(param: Value) -> Result<(), Error> {
(snapshot.group().backup_type().to_owned(), snapshot.group().backup_id().to_owned(), snapshot.backup_time())
};
- let (keydata, _) = keyfile_parameters(¶m)?;
+ let crypto = crypto_parameters(¶m)?;
- let crypt_config = match keydata {
+ let crypt_config = match crypto.enc_key {
None => None,
Some(key) => {
let (key, _created, _fingerprint) = decrypt_key(&key, &get_encryption_key_password)?;
diff --git a/src/bin/proxmox_backup_client/snapshot.rs b/src/bin/proxmox_backup_client/snapshot.rs
index 3bdc5f33..0c694598 100644
--- a/src/bin/proxmox_backup_client/snapshot.rs
+++ b/src/bin/proxmox_backup_client/snapshot.rs
@@ -30,9 +30,9 @@ use crate::{
complete_backup_group,
complete_repository,
connect,
+ crypto_parameters,
extract_repository_from_value,
record_repository,
- keyfile_parameters,
};
#[api(
@@ -234,9 +234,9 @@ async fn upload_log(param: Value) -> Result<Value, Error> {
let mut client = connect(&repo)?;
- let (keydata, crypt_mode) = keyfile_parameters(¶m)?;
+ let crypto = crypto_parameters(¶m)?;
- let crypt_config = match keydata {
+ let crypt_config = match crypto.enc_key {
None => None,
Some(key) => {
let (key, _created, _) = decrypt_key(&key, &crate::key::get_encryption_key_password)?;
@@ -248,7 +248,7 @@ async fn upload_log(param: Value) -> Result<Value, Error> {
let data = file_get_contents(logfile)?;
// fixme: howto sign log?
- let blob = match crypt_mode {
+ let blob = match crypto.mode {
CryptMode::None | CryptMode::SignOnly => DataBlob::encode(&data, None, true)?,
CryptMode::Encrypt => DataBlob::encode(&data, crypt_config.as_ref().map(Arc::as_ref), true)?,
};
--
2.20.1
^ permalink raw reply [flat|nested] 14+ messages in thread
* [pbs-devel] [PATCH proxmox-backup 06/10] client: allow passing specific master key
2021-02-05 15:35 [pbs-devel] [PATCH proxmox-backup 00/11] extend master key feature Fabian Grünbichler
` (5 preceding siblings ...)
2021-02-05 15:35 ` [pbs-devel] [PATCH proxmox-backup 05/10] client: refactor keyfile_parameters Fabian Grünbichler
@ 2021-02-05 15:35 ` Fabian Grünbichler
2021-02-05 15:35 ` [pbs-devel] [PATCH proxmox-backup 07/10] client: extend tests for master key handling Fabian Grünbichler
` (3 subsequent siblings)
10 siblings, 0 replies; 14+ messages in thread
From: Fabian Grünbichler @ 2021-02-05 15:35 UTC (permalink / raw)
To: pbs-devel
it's needed for PVE's LXC integration, and might be interesting for
other more special usage scenarios as well.
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
src/bin/proxmox-backup-client.rs | 180 ++++++++++++++++++++++-----
src/bin/proxmox_backup_client/key.rs | 27 ++++
2 files changed, 178 insertions(+), 29 deletions(-)
diff --git a/src/bin/proxmox-backup-client.rs b/src/bin/proxmox-backup-client.rs
index 8268098e..b737d9f0 100644
--- a/src/bin/proxmox-backup-client.rs
+++ b/src/bin/proxmox-backup-client.rs
@@ -77,15 +77,24 @@ pub const REPO_URL_SCHEMA: Schema = StringSchema::new("Repository URL.")
.max_length(256)
.schema();
-pub const KEYFILE_SCHEMA: Schema = StringSchema::new(
- "Path to encryption key. All data will be encrypted using this key.")
- .schema();
+pub const KEYFILE_SCHEMA: Schema =
+ StringSchema::new("Path to encryption key. All data will be encrypted using this key.")
+ .schema();
+
+pub const KEYFD_SCHEMA: Schema =
+ IntegerSchema::new("Pass an encryption key via an already opened file descriptor.")
+ .minimum(0)
+ .schema();
-pub const KEYFD_SCHEMA: Schema = IntegerSchema::new(
- "Pass an encryption key via an already opened file descriptor.")
- .minimum(0)
+pub const MASTER_PUBKEY_FILE_SCHEMA: Schema = StringSchema::new(
+ "Path to master public key. The encryption key used for a backup will be encrypted using this key and appended to the backup.")
.schema();
+pub const MASTER_PUBKEY_FD_SCHEMA: Schema =
+ IntegerSchema::new("Pass a master public key via an already opened file descriptor.")
+ .minimum(0)
+ .schema();
+
const CHUNK_SIZE_SCHEMA: Schema = IntegerSchema::new(
"Chunk size in KB. Must be a power of 2.")
.minimum(64)
@@ -601,6 +610,8 @@ fn spawn_catalog_upload(
struct CryptoParams {
mode: CryptMode,
enc_key: Option<Vec<u8>>,
+ // FIXME switch to openssl::rsa::rsa<openssl::pkey::Public> once that is Eq?
+ master_pubkey: Option<Vec<u8>>,
}
fn crypto_parameters(param: &Value) -> Result<CryptoParams, Error> {
@@ -622,6 +633,24 @@ fn crypto_parameters(param: &Value) -> Result<CryptoParams, Error> {
None => None,
};
+ let master_pubkey_file = match param.get("master-pubkey-file") {
+ Some(Value::String(keyfile)) => Some(keyfile),
+ Some(_) => bail!("bad --master-pubkey-file parameter type"),
+ None => None,
+ };
+
+ let master_pubkey_fd = match param.get("master-pubkey-fd") {
+ Some(Value::Number(key_fd)) => Some(
+ RawFd::try_from(key_fd
+ .as_i64()
+ .ok_or_else(|| format_err!("bad master public key fd: {:?}", key_fd))?
+ )
+ .map_err(|err| format_err!("bad public master key fd: {:?}: {}", key_fd, err))?
+ ),
+ Some(_) => bail!("bad --master-pubkey-fd parameter type"),
+ None => None,
+ };
+
let mode: Option<CryptMode> = match param.get("crypt-mode") {
Some(mode) => Some(serde_json::from_value(mode.clone())?),
None => None,
@@ -646,39 +675,105 @@ fn crypto_parameters(param: &Value) -> Result<CryptoParams, Error> {
}
};
- Ok(match (keydata, mode) {
+ let master_pubkey_data = match (master_pubkey_file, master_pubkey_fd) {
+ (None, None) => None,
+ (Some(_), Some(_)) => bail!("--keyfile and --keyfd are mutually exclusive"),
+ (Some(keyfile), None) => {
+ eprintln!("Using master key from file: {}", keyfile);
+ Some(file_get_contents(keyfile)?)
+ },
+ (None, Some(fd)) => {
+ let input = unsafe { std::fs::File::from_raw_fd(fd) };
+ let mut data = Vec::new();
+ let _len: usize = { input }.read_to_end(&mut data)
+ .map_err(|err| {
+ format_err!("error reading master key from fd {}: {}", fd, err)
+ })?;
+ eprintln!("Using master key from file descriptor");
+ Some(data)
+ }
+ };
+
+ Ok(match (keydata, master_pubkey_data, mode) {
// no parameters:
- (None, None) => match key::read_optional_default_encryption_key()? {
- None => CryptoParams { enc_key: None, mode: CryptMode::None },
+ (None, None, None) => match key::read_optional_default_encryption_key()? {
+ None => CryptoParams { mode: CryptMode::None, enc_key: None, master_pubkey: None },
enc_key => {
eprintln!("Encrypting with default encryption key!");
- CryptoParams { enc_key, mode: CryptMode::Encrypt }
+ let master_pubkey = key::read_optional_default_master_pubkey()?;
+ CryptoParams {
+ mode: CryptMode::Encrypt,
+ enc_key,
+ master_pubkey,
+ }
},
},
// just --crypt-mode=none
- (None, Some(CryptMode::None)) => CryptoParams { enc_key: None, mode: CryptMode::None },
+ (None, None, Some(CryptMode::None)) => CryptoParams { mode: CryptMode::None, enc_key: None, master_pubkey: None },
+
+ // --keyfile and --crypt-mode=none
+ (Some(_), _, Some(CryptMode::None)) => {
+ bail!("--keyfile/--keyfd and --crypt-mode=none are mutually exclusive");
+ },
+
+ // --master-pubkey-file and --crypt-mode=none
+ (_, Some(_), Some(CryptMode::None)) => {
+ bail!("--master-pubkey-file/--master-pubkey-fd and --crypt-mode=none are mutually exclusive");
+ },
- // just --crypt-mode other than none
- (None, Some(mode)) => match key::read_optional_default_encryption_key()? {
+ // --master-pubkey-file and nothing else
+ (None, master_pubkey, None) => {
+ match key::read_optional_default_encryption_key()? {
+ None => bail!("--master-pubkey-file/--master-pubkey-fd specified, but no key available"),
+ enc_key => {
+ eprintln!("Encrypting with default encryption key!");
+ CryptoParams {
+ mode: CryptMode::Encrypt,
+ enc_key,
+ master_pubkey,
+ }
+ },
+ }
+ },
+
+ // --crypt-mode other than none, without keyfile, with or without master key
+ (None, master_pubkey, Some(mode)) => match key::read_optional_default_encryption_key()? {
None => bail!("--crypt-mode without --keyfile and no default key file available"),
enc_key => {
eprintln!("Encrypting with default encryption key!");
+ let master_pubkey = match master_pubkey {
+ None => key::read_optional_default_master_pubkey()?,
+ master_pubkey => master_pubkey,
+ };
- CryptoParams { enc_key, mode }
+ CryptoParams {
+ mode,
+ enc_key,
+ master_pubkey,
+ }
},
}
// just --keyfile
- (enc_key, None) => CryptoParams { enc_key, mode: CryptMode::Encrypt },
+ (enc_key, master_pubkey, None) => {
+ let master_pubkey = match master_pubkey {
+ None => key::read_optional_default_master_pubkey()?,
+ master_pubkey => master_pubkey,
+ };
- // --keyfile and --crypt-mode=none
- (Some(_), Some(CryptMode::None)) => {
- bail!("--keyfile/--keyfd and --crypt-mode=none are mutually exclusive");
- }
+ CryptoParams { mode: CryptMode::Encrypt, enc_key, master_pubkey }
+ },
// --keyfile and --crypt-mode other than none
- (enc_key, Some(mode)) => CryptoParams { enc_key, mode },
+ (enc_key, master_pubkey, Some(mode)) => {
+ let master_pubkey = match master_pubkey {
+ None => key::read_optional_default_master_pubkey()?,
+ master_pubkey => master_pubkey,
+ };
+
+ CryptoParams { mode, enc_key, master_pubkey }
+ },
})
}
@@ -689,11 +784,31 @@ fn test_crypto_parameters_handling() -> Result<(), Error> {
let some_key = Some(vec![1;1]);
let default_key = Some(vec![2;1]);
- let no_key_res = CryptoParams { enc_key: None, mode: CryptMode::None };
- let some_key_res = CryptoParams { enc_key: some_key.clone(), mode: CryptMode::Encrypt };
- let some_key_sign_res = CryptoParams { enc_key: some_key.clone(), mode: CryptMode::SignOnly };
- let default_key_res = CryptoParams { enc_key: default_key.clone(), mode: CryptMode::Encrypt };
- let default_key_sign_res = CryptoParams { enc_key: default_key.clone(), mode: CryptMode::SignOnly };
+ let no_key_res = CryptoParams {
+ enc_key: None,
+ master_pubkey: None,
+ mode: CryptMode::None,
+ };
+ let some_key_res = CryptoParams {
+ enc_key: some_key.clone(),
+ master_pubkey: None,
+ mode: CryptMode::Encrypt,
+ };
+ let some_key_sign_res = CryptoParams {
+ enc_key: some_key.clone(),
+ master_pubkey: None,
+ mode: CryptMode::SignOnly,
+ };
+ let default_key_res = CryptoParams {
+ enc_key: default_key.clone(),
+ master_pubkey: None,
+ mode: CryptMode::Encrypt,
+ };
+ let default_key_sign_res = CryptoParams {
+ enc_key: default_key.clone(),
+ master_pubkey: None,
+ mode: CryptMode::SignOnly,
+ };
let keypath = "./tests/keyfile.test";
replace_file(&keypath, some_key.as_ref().unwrap(), CreateOptions::default())?;
@@ -840,6 +955,14 @@ fn test_crypto_parameters_handling() -> Result<(), Error> {
schema: KEYFD_SCHEMA,
optional: true,
},
+ "master-pubkey-file": {
+ schema: MASTER_PUBKEY_FILE_SCHEMA,
+ optional: true,
+ },
+ "master-pubkey-fd": {
+ schema: MASTER_PUBKEY_FD_SCHEMA,
+ optional: true,
+ },
"crypt-mode": {
type: CryptMode,
optional: true,
@@ -1026,16 +1149,14 @@ async fn create_backup(
let crypt_config = CryptConfig::new(key)?;
- match key::find_default_master_pubkey()? {
- Some(ref path) if path.exists() => {
- let pem_data = file_get_contents(path)?;
+ match crypto.master_pubkey {
+ Some(pem_data) => {
let rsa = openssl::rsa::Rsa::public_key_from_pem(&pem_data)?;
let mut key_config = KeyConfig::without_password(key)?;
key_config.created = created; // keep original value
let enc_key = rsa_encrypt_key_config(rsa, &key_config)?;
- println!("Master key '{:?}'", path);
(Some(Arc::new(crypt_config)), Some(enc_key))
}
@@ -1941,6 +2062,7 @@ fn main() {
.completion_cb("repository", complete_repository)
.completion_cb("backupspec", complete_backup_source)
.completion_cb("keyfile", tools::complete_file_name)
+ .completion_cb("master-pubkey-file", tools::complete_file_name)
.completion_cb("chunk-size", complete_chunk_size);
let benchmark_cmd_def = CliCommand::new(&API_METHOD_BENCHMARK)
diff --git a/src/bin/proxmox_backup_client/key.rs b/src/bin/proxmox_backup_client/key.rs
index 070f2a0b..7dd28473 100644
--- a/src/bin/proxmox_backup_client/key.rs
+++ b/src/bin/proxmox_backup_client/key.rs
@@ -58,6 +58,13 @@ pub fn read_optional_default_encryption_key() -> Result<Option<Vec<u8>>, Error>
.transpose()
}
+#[cfg(not(test))]
+pub fn read_optional_default_master_pubkey() -> Result<Option<Vec<u8>>, Error> {
+ find_default_master_pubkey()?
+ .map(file_get_contents)
+ .transpose()
+}
+
#[cfg(test)]
static mut TEST_DEFAULT_ENCRYPTION_KEY: Result<Option<Vec<u8>>, Error> = Ok(None);
@@ -78,6 +85,26 @@ pub unsafe fn set_test_encryption_key(value: Result<Option<Vec<u8>>, Error>) {
TEST_DEFAULT_ENCRYPTION_KEY = value;
}
+#[cfg(test)]
+static mut TEST_DEFAULT_MASTER_PUBKEY: Result<Option<Vec<u8>>, Error> = Ok(None);
+
+#[cfg(test)]
+pub fn read_optional_default_master_pubkey() -> Result<Option<Vec<u8>>, Error> {
+ // not safe when multiple concurrent test cases end up here!
+ unsafe {
+ match &TEST_DEFAULT_MASTER_PUBKEY {
+ Ok(key) => Ok(key.clone()),
+ Err(_) => bail!("test error"),
+ }
+ }
+}
+
+#[cfg(test)]
+// not safe when multiple concurrent test cases end up here!
+pub unsafe fn set_test_default_master_pubkey(value: Result<Option<Vec<u8>>, Error>) {
+ TEST_DEFAULT_MASTER_PUBKEY = value;
+}
+
pub fn get_encryption_key_password() -> Result<Vec<u8>, Error> {
// fixme: implement other input methods
--
2.20.1
^ permalink raw reply [flat|nested] 14+ messages in thread
* [pbs-devel] [PATCH proxmox-backup 07/10] client: extend tests for master key handling
2021-02-05 15:35 [pbs-devel] [PATCH proxmox-backup 00/11] extend master key feature Fabian Grünbichler
` (6 preceding siblings ...)
2021-02-05 15:35 ` [pbs-devel] [PATCH proxmox-backup 06/10] client: allow passing specific master key Fabian Grünbichler
@ 2021-02-05 15:35 ` Fabian Grünbichler
2021-02-05 15:35 ` [pbs-devel] [PATCH proxmox-backup 08/10] client: refactor crypto_parameter handling Fabian Grünbichler
` (2 subsequent siblings)
10 siblings, 0 replies; 14+ messages in thread
From: Fabian Grünbichler @ 2021-02-05 15:35 UTC (permalink / raw)
To: pbs-devel
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
src/bin/proxmox-backup-client.rs | 62 ++++++++++++++++++++++++++++++++
1 file changed, 62 insertions(+)
diff --git a/src/bin/proxmox-backup-client.rs b/src/bin/proxmox-backup-client.rs
index b737d9f0..76e82184 100644
--- a/src/bin/proxmox-backup-client.rs
+++ b/src/bin/proxmox-backup-client.rs
@@ -784,6 +784,9 @@ fn test_crypto_parameters_handling() -> Result<(), Error> {
let some_key = Some(vec![1;1]);
let default_key = Some(vec![2;1]);
+ let some_master_key = Some(vec![3;1]);
+ let default_master_key = Some(vec![4;1]);
+
let no_key_res = CryptoParams {
enc_key: None,
master_pubkey: None,
@@ -794,6 +797,17 @@ fn test_crypto_parameters_handling() -> Result<(), Error> {
master_pubkey: None,
mode: CryptMode::Encrypt,
};
+ let some_key_some_master_res = CryptoParams {
+ enc_key: some_key.clone(),
+ master_pubkey: some_master_key.clone(),
+ mode: CryptMode::Encrypt,
+ };
+ let some_key_default_master_res = CryptoParams {
+ enc_key: some_key.clone(),
+ master_pubkey: default_master_key.clone(),
+ mode: CryptMode::Encrypt,
+ };
+
let some_key_sign_res = CryptoParams {
enc_key: some_key.clone(),
master_pubkey: None,
@@ -812,6 +826,8 @@ fn test_crypto_parameters_handling() -> Result<(), Error> {
let keypath = "./tests/keyfile.test";
replace_file(&keypath, some_key.as_ref().unwrap(), CreateOptions::default())?;
+ let master_keypath = "./tests/masterkeyfile.test";
+ replace_file(&master_keypath, some_master_key.as_ref().unwrap(), CreateOptions::default())?;
let invalid_keypath = "./tests/invalid_keyfile.test";
// no params, no default key == no key
@@ -917,6 +933,52 @@ fn test_crypto_parameters_handling() -> Result<(), Error> {
assert!(crypto_parameters(&json!({"keyfile": invalid_keypath, "crypt-mode": "none"})).is_err());
assert!(crypto_parameters(&json!({"keyfile": invalid_keypath, "crypt-mode": "sign-only"})).is_err());
assert!(crypto_parameters(&json!({"keyfile": invalid_keypath, "crypt-mode": "encrypt"})).is_err());
+
+ // now remove default key again
+ unsafe { key::set_test_encryption_key(Ok(None)); }
+ // set a default master key
+ unsafe { key::set_test_default_master_pubkey(Ok(default_master_key.clone())); }
+
+ // and use an explicit master key
+ assert!(crypto_parameters(&json!({"master-pubkey-file": master_keypath})).is_err());
+ // just a default == no key
+ let res = crypto_parameters(&json!({}));
+ assert_eq!(res.unwrap(), no_key_res);
+
+ // keyfile param == key from keyfile
+ let res = crypto_parameters(&json!({"keyfile": keypath, "master-pubkey-file": master_keypath}));
+ assert_eq!(res.unwrap(), some_key_some_master_res);
+ // same with fallback to default master key
+ let res = crypto_parameters(&json!({"keyfile": keypath}));
+ assert_eq!(res.unwrap(), some_key_default_master_res);
+
+ // crypt mode none == error
+ assert!(crypto_parameters(&json!({"crypt-mode": "none", "master-pubkey-file": master_keypath})).is_err());
+ // with just default master key == no key
+ let res = crypto_parameters(&json!({"crypt-mode": "none"}));
+ assert_eq!(res.unwrap(), no_key_res);
+
+ // crypt mode encrypt without enc key == error
+ assert!(crypto_parameters(&json!({"crypt-mode": "encrypt", "master-pubkey-file": master_keypath})).is_err());
+ assert!(crypto_parameters(&json!({"crypt-mode": "encrypt"})).is_err());
+
+ // crypt mode none with explicit key == Error
+ assert!(crypto_parameters(&json!({"crypt-mode": "none", "keyfile": keypath, "master-pubkey-file": master_keypath})).is_err());
+ assert!(crypto_parameters(&json!({"crypt-mode": "none", "keyfile": keypath})).is_err());
+
+ // crypt mode encrypt with keyfile == key from keyfile with correct mode
+ let res = crypto_parameters(&json!({"crypt-mode": "encrypt", "keyfile": keypath, "master-pubkey-file": master_keypath}));
+ assert_eq!(res.unwrap(), some_key_some_master_res);
+ let res = crypto_parameters(&json!({"crypt-mode": "encrypt", "keyfile": keypath}));
+ assert_eq!(res.unwrap(), some_key_default_master_res);
+
+ // invalid master keyfile parameter always errors when a key is passed, even with a valid
+ // default master key
+ assert!(crypto_parameters(&json!({"keyfile": keypath, "master-pubkey-file": invalid_keypath})).is_err());
+ assert!(crypto_parameters(&json!({"keyfile": keypath, "master-pubkey-file": invalid_keypath,"crypt-mode": "none"})).is_err());
+ assert!(crypto_parameters(&json!({"keyfile": keypath, "master-pubkey-file": invalid_keypath,"crypt-mode": "sign-only"})).is_err());
+ assert!(crypto_parameters(&json!({"keyfile": keypath, "master-pubkey-file": invalid_keypath,"crypt-mode": "encrypt"})).is_err());
+
Ok(())
}
--
2.20.1
^ permalink raw reply [flat|nested] 14+ messages in thread
* [pbs-devel] [PATCH proxmox-backup 08/10] client: refactor crypto_parameter handling
2021-02-05 15:35 [pbs-devel] [PATCH proxmox-backup 00/11] extend master key feature Fabian Grünbichler
` (7 preceding siblings ...)
2021-02-05 15:35 ` [pbs-devel] [PATCH proxmox-backup 07/10] client: extend tests for master key handling Fabian Grünbichler
@ 2021-02-05 15:35 ` Fabian Grünbichler
2021-02-05 15:35 ` [pbs-devel] [PATCH proxmox-backup 09/10] client: track key source, print when used Fabian Grünbichler
2021-02-06 8:13 ` [pbs-devel] applied: [PATCH proxmox-backup 00/11] extend master key feature Dietmar Maurer
10 siblings, 0 replies; 14+ messages in thread
From: Fabian Grünbichler @ 2021-02-05 15:35 UTC (permalink / raw)
To: pbs-devel
pull out the crypt-mode to logically group arms and make the whole mess
a bit more "human-parsable".
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
IMHO this makes more sense this way, otherwise we have too many
combinations that we have to keep in mind in a single match..
src/bin/proxmox-backup-client.rs | 117 ++++++++++++++++---------------
1 file changed, 59 insertions(+), 58 deletions(-)
diff --git a/src/bin/proxmox-backup-client.rs b/src/bin/proxmox-backup-client.rs
index 76e82184..89d77d04 100644
--- a/src/bin/proxmox-backup-client.rs
+++ b/src/bin/proxmox-backup-client.rs
@@ -694,87 +694,88 @@ fn crypto_parameters(param: &Value) -> Result<CryptoParams, Error> {
}
};
- Ok(match (keydata, master_pubkey_data, mode) {
- // no parameters:
- (None, None, None) => match key::read_optional_default_encryption_key()? {
- None => CryptoParams { mode: CryptMode::None, enc_key: None, master_pubkey: None },
- enc_key => {
- eprintln!("Encrypting with default encryption key!");
- let master_pubkey = key::read_optional_default_master_pubkey()?;
- CryptoParams {
- mode: CryptMode::Encrypt,
- enc_key,
- master_pubkey,
- }
+ let res = match mode {
+ // no crypt mode, enable encryption if keys are available
+ None => match (keydata, master_pubkey_data) {
+ // only default keys if available
+ (None, None) => match key::read_optional_default_encryption_key()? {
+ None => CryptoParams { mode: CryptMode::None, enc_key: None, master_pubkey: None },
+ enc_key => {
+ eprintln!("Encrypting with default encryption key!");
+ let master_pubkey = key::read_optional_default_master_pubkey()?;
+ CryptoParams {
+ mode: CryptMode::Encrypt,
+ enc_key,
+ master_pubkey,
+ }
+ },
+ },
+
+ // explicit master key, default enc key needed
+ (None, master_pubkey) => match key::read_optional_default_encryption_key()? {
+ None => bail!("--master-pubkey-file/--master-pubkey-fd specified, but no key available"),
+ enc_key => {
+ eprintln!("Encrypting with default encryption key!");
+ CryptoParams {
+ mode: CryptMode::Encrypt,
+ enc_key,
+ master_pubkey,
+ }
+ },
},
- },
- // just --crypt-mode=none
- (None, None, Some(CryptMode::None)) => CryptoParams { mode: CryptMode::None, enc_key: None, master_pubkey: None },
+ // explicit keyfile, maybe default master key
+ (enc_key, None) => CryptoParams { mode: CryptMode::Encrypt, enc_key, master_pubkey: key::read_optional_default_master_pubkey()? },
- // --keyfile and --crypt-mode=none
- (Some(_), _, Some(CryptMode::None)) => {
- bail!("--keyfile/--keyfd and --crypt-mode=none are mutually exclusive");
+ // explicit keyfile and master key
+ (enc_key, master_pubkey) => CryptoParams { mode: CryptMode::Encrypt, enc_key, master_pubkey },
},
- // --master-pubkey-file and --crypt-mode=none
- (_, Some(_), Some(CryptMode::None)) => {
- bail!("--master-pubkey-file/--master-pubkey-fd and --crypt-mode=none are mutually exclusive");
+ // explicitly disabled encryption
+ Some(CryptMode::None) => match (keydata, master_pubkey_data) {
+ // no keys => OK, no encryption
+ (None, None) => CryptoParams { mode: CryptMode::None, enc_key: None, master_pubkey: None },
+
+ // --keyfile and --crypt-mode=none
+ (Some(_), _) => bail!("--keyfile/--keyfd and --crypt-mode=none are mutually exclusive"),
+
+ // --master-pubkey-file and --crypt-mode=none
+ (_, Some(_)) => bail!("--master-pubkey-file/--master-pubkey-fd and --crypt-mode=none are mutually exclusive"),
},
- // --master-pubkey-file and nothing else
- (None, master_pubkey, None) => {
- match key::read_optional_default_encryption_key()? {
- None => bail!("--master-pubkey-file/--master-pubkey-fd specified, but no key available"),
+ // explicitly enabled encryption
+ Some(mode) => match (keydata, master_pubkey_data) {
+ // no key, maybe master key
+ (None, master_pubkey) => match key::read_optional_default_encryption_key()? {
+ None => bail!("--crypt-mode without --keyfile and no default key file available"),
enc_key => {
eprintln!("Encrypting with default encryption key!");
+ let master_pubkey = match master_pubkey {
+ None => key::read_optional_default_master_pubkey()?,
+ master_pubkey => master_pubkey,
+ };
+
CryptoParams {
- mode: CryptMode::Encrypt,
+ mode,
enc_key,
master_pubkey,
}
},
- }
- },
+ },
- // --crypt-mode other than none, without keyfile, with or without master key
- (None, master_pubkey, Some(mode)) => match key::read_optional_default_encryption_key()? {
- None => bail!("--crypt-mode without --keyfile and no default key file available"),
- enc_key => {
- eprintln!("Encrypting with default encryption key!");
+ // --keyfile and --crypt-mode other than none
+ (enc_key, master_pubkey) => {
let master_pubkey = match master_pubkey {
None => key::read_optional_default_master_pubkey()?,
master_pubkey => master_pubkey,
};
- CryptoParams {
- mode,
- enc_key,
- master_pubkey,
- }
+ CryptoParams { mode, enc_key, master_pubkey }
},
- }
-
- // just --keyfile
- (enc_key, master_pubkey, None) => {
- let master_pubkey = match master_pubkey {
- None => key::read_optional_default_master_pubkey()?,
- master_pubkey => master_pubkey,
- };
-
- CryptoParams { mode: CryptMode::Encrypt, enc_key, master_pubkey }
},
+ };
- // --keyfile and --crypt-mode other than none
- (enc_key, master_pubkey, Some(mode)) => {
- let master_pubkey = match master_pubkey {
- None => key::read_optional_default_master_pubkey()?,
- master_pubkey => master_pubkey,
- };
-
- CryptoParams { mode, enc_key, master_pubkey }
- },
- })
+ Ok(res)
}
#[test]
--
2.20.1
^ permalink raw reply [flat|nested] 14+ messages in thread
* [pbs-devel] [PATCH proxmox-backup 09/10] client: track key source, print when used
2021-02-05 15:35 [pbs-devel] [PATCH proxmox-backup 00/11] extend master key feature Fabian Grünbichler
` (8 preceding siblings ...)
2021-02-05 15:35 ` [pbs-devel] [PATCH proxmox-backup 08/10] client: refactor crypto_parameter handling Fabian Grünbichler
@ 2021-02-05 15:35 ` Fabian Grünbichler
2021-02-06 8:13 ` [pbs-devel] applied: [PATCH proxmox-backup 00/11] extend master key feature Dietmar Maurer
10 siblings, 0 replies; 14+ messages in thread
From: Fabian Grünbichler @ 2021-02-05 15:35 UTC (permalink / raw)
To: pbs-devel
to avoid confusing messages about using encryption keys when restoring
plaintext backups, or about loading master keys when they are not
actually used for the current operation.
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
src/bin/proxmox-backup-client.rs | 185 +++++++++++++++-------
src/bin/proxmox_backup_client/catalog.rs | 13 +-
src/bin/proxmox_backup_client/key.rs | 24 +--
src/bin/proxmox_backup_client/snapshot.rs | 2 +-
4 files changed, 155 insertions(+), 69 deletions(-)
diff --git a/src/bin/proxmox-backup-client.rs b/src/bin/proxmox-backup-client.rs
index 89d77d04..2db5cf7d 100644
--- a/src/bin/proxmox-backup-client.rs
+++ b/src/bin/proxmox-backup-client.rs
@@ -606,12 +606,56 @@ fn spawn_catalog_upload(
Ok(CatalogUploadResult { catalog_writer, result: catalog_result_rx })
}
+#[derive(Clone, Debug, Eq, PartialEq)]
+enum KeySource {
+ DefaultKey,
+ Fd,
+ Path(String),
+}
+
+fn format_key_source(source: &KeySource, key_type: &str) -> String {
+ match source {
+ KeySource::DefaultKey => format!("Using default {} key..", key_type),
+ KeySource::Fd => format!("Using {} key from file descriptor..", key_type),
+ KeySource::Path(path) => format!("Using {} key from '{}'..", key_type, path),
+ }
+}
+
+#[derive(Clone, Debug, Eq, PartialEq)]
+struct KeyWithSource {
+ pub source: KeySource,
+ pub key: Vec<u8>,
+}
+
+impl KeyWithSource {
+ pub fn from_fd(key: Vec<u8>) -> Self {
+ Self {
+ source: KeySource::Fd,
+ key,
+ }
+ }
+
+ pub fn from_default(key: Vec<u8>) -> Self {
+ Self {
+ source: KeySource::DefaultKey,
+ key,
+ }
+ }
+
+ pub fn from_path(path: String, key: Vec<u8>) -> Self {
+ Self {
+ source: KeySource::Path(path),
+ key,
+ }
+ }
+}
+
#[derive(Debug, Eq, PartialEq)]
struct CryptoParams {
mode: CryptMode,
- enc_key: Option<Vec<u8>>,
+ enc_key: Option<KeyWithSource>,
// FIXME switch to openssl::rsa::rsa<openssl::pkey::Public> once that is Eq?
- master_pubkey: Option<Vec<u8>>,
+ master_pubkey: Option<KeyWithSource>,
}
fn crypto_parameters(param: &Value) -> Result<CryptoParams, Error> {
@@ -656,52 +700,47 @@ fn crypto_parameters(param: &Value) -> Result<CryptoParams, Error> {
None => None,
};
- let keydata = match (keyfile, key_fd) {
+ let key = match (keyfile, key_fd) {
(None, None) => None,
(Some(_), Some(_)) => bail!("--keyfile and --keyfd are mutually exclusive"),
- (Some(keyfile), None) => {
- eprintln!("Using encryption key file: {}", keyfile);
- Some(file_get_contents(keyfile)?)
- },
+ (Some(keyfile), None) => Some(KeyWithSource::from_path(
+ keyfile.clone(),
+ file_get_contents(keyfile)?,
+ )),
(None, Some(fd)) => {
let input = unsafe { std::fs::File::from_raw_fd(fd) };
let mut data = Vec::new();
- let _len: usize = { input }.read_to_end(&mut data)
- .map_err(|err| {
- format_err!("error reading encryption key from fd {}: {}", fd, err)
- })?;
- eprintln!("Using encryption key from file descriptor");
- Some(data)
+ let _len: usize = { input }.read_to_end(&mut data).map_err(|err| {
+ format_err!("error reading encryption key from fd {}: {}", fd, err)
+ })?;
+ Some(KeyWithSource::from_fd(data))
}
};
- let master_pubkey_data = match (master_pubkey_file, master_pubkey_fd) {
+ let master_pubkey = match (master_pubkey_file, master_pubkey_fd) {
(None, None) => None,
(Some(_), Some(_)) => bail!("--keyfile and --keyfd are mutually exclusive"),
- (Some(keyfile), None) => {
- eprintln!("Using master key from file: {}", keyfile);
- Some(file_get_contents(keyfile)?)
- },
+ (Some(keyfile), None) => Some(KeyWithSource::from_path(
+ keyfile.clone(),
+ file_get_contents(keyfile)?,
+ )),
(None, Some(fd)) => {
let input = unsafe { std::fs::File::from_raw_fd(fd) };
let mut data = Vec::new();
- let _len: usize = { input }.read_to_end(&mut data)
- .map_err(|err| {
- format_err!("error reading master key from fd {}: {}", fd, err)
- })?;
- eprintln!("Using master key from file descriptor");
- Some(data)
+ let _len: usize = { input }
+ .read_to_end(&mut data)
+ .map_err(|err| format_err!("error reading master key from fd {}: {}", fd, err))?;
+ Some(KeyWithSource::from_fd(data))
}
};
let res = match mode {
// no crypt mode, enable encryption if keys are available
- None => match (keydata, master_pubkey_data) {
+ None => match (key, master_pubkey) {
// only default keys if available
(None, None) => match key::read_optional_default_encryption_key()? {
None => CryptoParams { mode: CryptMode::None, enc_key: None, master_pubkey: None },
enc_key => {
- eprintln!("Encrypting with default encryption key!");
let master_pubkey = key::read_optional_default_master_pubkey()?;
CryptoParams {
mode: CryptMode::Encrypt,
@@ -715,7 +754,6 @@ fn crypto_parameters(param: &Value) -> Result<CryptoParams, Error> {
(None, master_pubkey) => match key::read_optional_default_encryption_key()? {
None => bail!("--master-pubkey-file/--master-pubkey-fd specified, but no key available"),
enc_key => {
- eprintln!("Encrypting with default encryption key!");
CryptoParams {
mode: CryptMode::Encrypt,
enc_key,
@@ -732,7 +770,7 @@ fn crypto_parameters(param: &Value) -> Result<CryptoParams, Error> {
},
// explicitly disabled encryption
- Some(CryptMode::None) => match (keydata, master_pubkey_data) {
+ Some(CryptMode::None) => match (key, master_pubkey) {
// no keys => OK, no encryption
(None, None) => CryptoParams { mode: CryptMode::None, enc_key: None, master_pubkey: None },
@@ -744,7 +782,7 @@ fn crypto_parameters(param: &Value) -> Result<CryptoParams, Error> {
},
// explicitly enabled encryption
- Some(mode) => match (keydata, master_pubkey_data) {
+ Some(mode) => match (key, master_pubkey) {
// no key, maybe master key
(None, master_pubkey) => match key::read_optional_default_encryption_key()? {
None => bail!("--crypt-mode without --keyfile and no default key file available"),
@@ -782,11 +820,15 @@ fn crypto_parameters(param: &Value) -> Result<CryptoParams, Error> {
// WARNING: there must only be one test for crypto_parameters as the default key handling is not
// safe w.r.t. concurrency
fn test_crypto_parameters_handling() -> Result<(), Error> {
- let some_key = Some(vec![1;1]);
- let default_key = Some(vec![2;1]);
+ let some_key = vec![1;1];
+ let default_key = vec![2;1];
- let some_master_key = Some(vec![3;1]);
- let default_master_key = Some(vec![4;1]);
+ let some_master_key = vec![3;1];
+ let default_master_key = vec![4;1];
+
+ let keypath = "./tests/keyfile.test";
+ let master_keypath = "./tests/masterkeyfile.test";
+ let invalid_keypath = "./tests/invalid_keyfile.test";
let no_key_res = CryptoParams {
enc_key: None,
@@ -794,42 +836,54 @@ fn test_crypto_parameters_handling() -> Result<(), Error> {
mode: CryptMode::None,
};
let some_key_res = CryptoParams {
- enc_key: some_key.clone(),
+ enc_key: Some(KeyWithSource::from_path(
+ keypath.to_string(),
+ some_key.clone(),
+ )),
master_pubkey: None,
mode: CryptMode::Encrypt,
};
let some_key_some_master_res = CryptoParams {
- enc_key: some_key.clone(),
- master_pubkey: some_master_key.clone(),
+ enc_key: Some(KeyWithSource::from_path(
+ keypath.to_string(),
+ some_key.clone(),
+ )),
+ master_pubkey: Some(KeyWithSource::from_path(
+ master_keypath.to_string(),
+ some_master_key.clone(),
+ )),
mode: CryptMode::Encrypt,
};
let some_key_default_master_res = CryptoParams {
- enc_key: some_key.clone(),
- master_pubkey: default_master_key.clone(),
+ enc_key: Some(KeyWithSource::from_path(
+ keypath.to_string(),
+ some_key.clone(),
+ )),
+ master_pubkey: Some(KeyWithSource::from_default(default_master_key.clone())),
mode: CryptMode::Encrypt,
};
let some_key_sign_res = CryptoParams {
- enc_key: some_key.clone(),
+ enc_key: Some(KeyWithSource::from_path(
+ keypath.to_string(),
+ some_key.clone(),
+ )),
master_pubkey: None,
mode: CryptMode::SignOnly,
};
let default_key_res = CryptoParams {
- enc_key: default_key.clone(),
+ enc_key: Some(KeyWithSource::from_default(default_key.clone())),
master_pubkey: None,
mode: CryptMode::Encrypt,
};
let default_key_sign_res = CryptoParams {
- enc_key: default_key.clone(),
+ enc_key: Some(KeyWithSource::from_default(default_key.clone())),
master_pubkey: None,
mode: CryptMode::SignOnly,
};
- let keypath = "./tests/keyfile.test";
- replace_file(&keypath, some_key.as_ref().unwrap(), CreateOptions::default())?;
- let master_keypath = "./tests/masterkeyfile.test";
- replace_file(&master_keypath, some_master_key.as_ref().unwrap(), CreateOptions::default())?;
- let invalid_keypath = "./tests/invalid_keyfile.test";
+ replace_file(&keypath, &some_key, CreateOptions::default())?;
+ replace_file(&master_keypath, &some_master_key, CreateOptions::default())?;
// no params, no default key == no key
let res = crypto_parameters(&json!({}));
@@ -863,7 +917,7 @@ fn test_crypto_parameters_handling() -> Result<(), Error> {
assert!(crypto_parameters(&json!({"keyfile": invalid_keypath, "crypt-mode": "encrypt"})).is_err());
// now set a default key
- unsafe { key::set_test_encryption_key(Ok(default_key.clone())); }
+ unsafe { key::set_test_encryption_key(Ok(Some(default_key.clone()))); }
// and repeat
@@ -938,7 +992,7 @@ fn test_crypto_parameters_handling() -> Result<(), Error> {
// now remove default key again
unsafe { key::set_test_encryption_key(Ok(None)); }
// set a default master key
- unsafe { key::set_test_default_master_pubkey(Ok(default_master_key.clone())); }
+ unsafe { key::set_test_default_master_pubkey(Ok(Some(default_master_key.clone()))); }
// and use an explicit master key
assert!(crypto_parameters(&json!({"master-pubkey-file": master_keypath})).is_err());
@@ -1206,15 +1260,23 @@ async fn create_backup(
let (crypt_config, rsa_encrypted_key) = match crypto.enc_key {
None => (None, None),
- Some(key) => {
- let (key, created, fingerprint) = decrypt_key(&key, &key::get_encryption_key_password)?;
+ Some(key_with_source) => {
+ println!(
+ "{}",
+ format_key_source(&key_with_source.source, "encryption")
+ );
+
+ let (key, created, fingerprint) =
+ decrypt_key(&key_with_source.key, &key::get_encryption_key_password)?;
println!("Encryption key fingerprint: {}", fingerprint);
let crypt_config = CryptConfig::new(key)?;
match crypto.master_pubkey {
- Some(pem_data) => {
- let rsa = openssl::rsa::Rsa::public_key_from_pem(&pem_data)?;
+ Some(pem_with_source) => {
+ println!("{}", format_key_source(&pem_with_source.source, "master"));
+
+ let rsa = openssl::rsa::Rsa::public_key_from_pem(&pem_with_source.key)?;
let mut key_config = KeyConfig::without_password(key)?;
key_config.created = created; // keep original value
@@ -1222,7 +1284,7 @@ async fn create_backup(
let enc_key = rsa_encrypt_key_config(rsa, &key_config)?;
(Some(Arc::new(crypt_config)), Some(enc_key))
- }
+ },
_ => (Some(Arc::new(crypt_config)), None),
}
}
@@ -1574,9 +1636,12 @@ async fn restore(param: Value) -> Result<Value, Error> {
let crypt_config = match crypto.enc_key {
None => None,
- Some(key) => {
- let (key, _, fingerprint) = decrypt_key(&key, &key::get_encryption_key_password)?;
- eprintln!("Encryption key fingerprint: '{}'", fingerprint);
+ Some(ref key) => {
+ let (key, _, _) =
+ decrypt_key(&key.key, &key::get_encryption_key_password).map_err(|err| {
+ eprintln!("{}", format_key_source(&key.source, "encryption"));
+ err
+ })?;
Some(Arc::new(CryptConfig::new(key)?))
}
};
@@ -1598,6 +1663,14 @@ async fn restore(param: Value) -> Result<Value, Error> {
if archive_name == ENCRYPTED_KEY_BLOB_NAME && crypt_config.is_none() {
eprintln!("Restoring encrypted key blob without original key - skipping manifest fingerprint check!")
} else {
+ if manifest.signature.is_some() {
+ if let Some(key) = &crypto.enc_key {
+ eprintln!("{}", format_key_source(&key.source, "encryption"));
+ }
+ if let Some(config) = &crypt_config {
+ eprintln!("Fingerprint: {}", config.fingerprint());
+ }
+ }
manifest.check_fingerprint(crypt_config.as_ref().map(Arc::as_ref))?;
}
diff --git a/src/bin/proxmox_backup_client/catalog.rs b/src/bin/proxmox_backup_client/catalog.rs
index b69d9c37..659200ff 100644
--- a/src/bin/proxmox_backup_client/catalog.rs
+++ b/src/bin/proxmox_backup_client/catalog.rs
@@ -15,6 +15,7 @@ use crate::{
REPO_URL_SCHEMA,
KEYFD_SCHEMA,
extract_repository_from_value,
+ format_key_source,
record_repository,
key::get_encryption_key_password,
decrypt_key,
@@ -73,7 +74,11 @@ async fn dump_catalog(param: Value) -> Result<Value, Error> {
let crypt_config = match crypto.enc_key {
None => None,
Some(key) => {
- let (key, _created, _fingerprint) = decrypt_key(&key, &get_encryption_key_password)?;
+ let (key, _created, _fingerprint) = decrypt_key(&key.key, &get_encryption_key_password)
+ .map_err(|err| {
+ eprintln!("{}", format_key_source(&key.source, "encryption"));
+ err
+ })?;
let crypt_config = CryptConfig::new(key)?;
Some(Arc::new(crypt_config))
}
@@ -171,7 +176,11 @@ async fn catalog_shell(param: Value) -> Result<(), Error> {
let crypt_config = match crypto.enc_key {
None => None,
Some(key) => {
- let (key, _created, _fingerprint) = decrypt_key(&key, &get_encryption_key_password)?;
+ let (key, _created, _fingerprint) = decrypt_key(&key.key, &get_encryption_key_password)
+ .map_err(|err| {
+ eprintln!("{}", format_key_source(&key.source, "encryption"));
+ err
+ })?;
let crypt_config = CryptConfig::new(key)?;
Some(Arc::new(crypt_config))
}
diff --git a/src/bin/proxmox_backup_client/key.rs b/src/bin/proxmox_backup_client/key.rs
index 7dd28473..6e18a026 100644
--- a/src/bin/proxmox_backup_client/key.rs
+++ b/src/bin/proxmox_backup_client/key.rs
@@ -20,6 +20,8 @@ use proxmox_backup::{
tools::paperkey::{generate_paper_key, PaperkeyFormat},
};
+use crate::KeyWithSource;
+
pub const DEFAULT_ENCRYPTION_KEY_FILE_NAME: &str = "encryption-key.json";
pub const DEFAULT_MASTER_PUBKEY_FILE_NAME: &str = "master-public.pem";
@@ -52,16 +54,16 @@ pub fn place_default_encryption_key() -> Result<PathBuf, Error> {
}
#[cfg(not(test))]
-pub fn read_optional_default_encryption_key() -> Result<Option<Vec<u8>>, Error> {
+pub(crate) fn read_optional_default_encryption_key() -> Result<Option<KeyWithSource>, Error> {
find_default_encryption_key()?
- .map(file_get_contents)
+ .map(|path| file_get_contents(path).map(KeyWithSource::from_default))
.transpose()
}
#[cfg(not(test))]
-pub fn read_optional_default_master_pubkey() -> Result<Option<Vec<u8>>, Error> {
+pub(crate) fn read_optional_default_master_pubkey() -> Result<Option<KeyWithSource>, Error> {
find_default_master_pubkey()?
- .map(file_get_contents)
+ .map(|path| file_get_contents(path).map(KeyWithSource::from_default))
.transpose()
}
@@ -69,11 +71,12 @@ pub fn read_optional_default_master_pubkey() -> Result<Option<Vec<u8>>, Error> {
static mut TEST_DEFAULT_ENCRYPTION_KEY: Result<Option<Vec<u8>>, Error> = Ok(None);
#[cfg(test)]
-pub fn read_optional_default_encryption_key() -> Result<Option<Vec<u8>>, Error> {
+pub(crate) fn read_optional_default_encryption_key() -> Result<Option<KeyWithSource>, Error> {
// not safe when multiple concurrent test cases end up here!
unsafe {
match &TEST_DEFAULT_ENCRYPTION_KEY {
- Ok(key) => Ok(key.clone()),
+ Ok(Some(key)) => Ok(Some(KeyWithSource::from_default(key.clone()))),
+ Ok(None) => Ok(None),
Err(_) => bail!("test error"),
}
}
@@ -81,7 +84,7 @@ pub fn read_optional_default_encryption_key() -> Result<Option<Vec<u8>>, Error>
#[cfg(test)]
// not safe when multiple concurrent test cases end up here!
-pub unsafe fn set_test_encryption_key(value: Result<Option<Vec<u8>>, Error>) {
+pub(crate) unsafe fn set_test_encryption_key(value: Result<Option<Vec<u8>>, Error>) {
TEST_DEFAULT_ENCRYPTION_KEY = value;
}
@@ -89,11 +92,12 @@ pub unsafe fn set_test_encryption_key(value: Result<Option<Vec<u8>>, Error>) {
static mut TEST_DEFAULT_MASTER_PUBKEY: Result<Option<Vec<u8>>, Error> = Ok(None);
#[cfg(test)]
-pub fn read_optional_default_master_pubkey() -> Result<Option<Vec<u8>>, Error> {
+pub(crate) fn read_optional_default_master_pubkey() -> Result<Option<KeyWithSource>, Error> {
// not safe when multiple concurrent test cases end up here!
unsafe {
match &TEST_DEFAULT_MASTER_PUBKEY {
- Ok(key) => Ok(key.clone()),
+ Ok(Some(key)) => Ok(Some(KeyWithSource::from_default(key.clone()))),
+ Ok(None) => Ok(None),
Err(_) => bail!("test error"),
}
}
@@ -101,7 +105,7 @@ pub fn read_optional_default_master_pubkey() -> Result<Option<Vec<u8>>, Error> {
#[cfg(test)]
// not safe when multiple concurrent test cases end up here!
-pub unsafe fn set_test_default_master_pubkey(value: Result<Option<Vec<u8>>, Error>) {
+pub(crate) unsafe fn set_test_default_master_pubkey(value: Result<Option<Vec<u8>>, Error>) {
TEST_DEFAULT_MASTER_PUBKEY = value;
}
diff --git a/src/bin/proxmox_backup_client/snapshot.rs b/src/bin/proxmox_backup_client/snapshot.rs
index 0c694598..5988ebf6 100644
--- a/src/bin/proxmox_backup_client/snapshot.rs
+++ b/src/bin/proxmox_backup_client/snapshot.rs
@@ -239,7 +239,7 @@ async fn upload_log(param: Value) -> Result<Value, Error> {
let crypt_config = match crypto.enc_key {
None => None,
Some(key) => {
- let (key, _created, _) = decrypt_key(&key, &crate::key::get_encryption_key_password)?;
+ let (key, _created, _) = decrypt_key(&key.key, &crate::key::get_encryption_key_password)?;
let crypt_config = CryptConfig::new(key)?;
Some(Arc::new(crypt_config))
}
--
2.20.1
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [pbs-devel] [PATCH proxmox-backup 04/10] client: add test for keyfile_parameters
2021-02-05 15:35 ` [pbs-devel] [PATCH proxmox-backup 04/10] client: add test for keyfile_parameters Fabian Grünbichler
@ 2021-02-06 8:00 ` Dietmar Maurer
0 siblings, 0 replies; 14+ messages in thread
From: Dietmar Maurer @ 2021-02-06 8:00 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Fabian Grünbichler
I would prefer another location for generated test files.
For tape backup tests, I now use:
./target/testout/<test_fn_name>
> + let keypath = "./tests/keyfile.test";
> + replace_file(&keypath, some_key.as_ref().unwrap(), CreateOptions::default())?;
> + let invalid_keypath = "./tests/invalid_keyfile.test";
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [pbs-devel] applied: [PATCH proxmox-backup 00/11] extend master key feature
2021-02-05 15:35 [pbs-devel] [PATCH proxmox-backup 00/11] extend master key feature Fabian Grünbichler
` (9 preceding siblings ...)
2021-02-05 15:35 ` [pbs-devel] [PATCH proxmox-backup 09/10] client: track key source, print when used Fabian Grünbichler
@ 2021-02-06 8:13 ` Dietmar Maurer
2021-02-08 11:02 ` Fabian Grünbichler
10 siblings, 1 reply; 14+ messages in thread
From: Dietmar Maurer @ 2021-02-06 8:13 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Fabian Grünbichler
applied patches 1..10
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [pbs-devel] applied: [PATCH proxmox-backup 00/11] extend master key feature
2021-02-06 8:13 ` [pbs-devel] applied: [PATCH proxmox-backup 00/11] extend master key feature Dietmar Maurer
@ 2021-02-08 11:02 ` Fabian Grünbichler
0 siblings, 0 replies; 14+ messages in thread
From: Fabian Grünbichler @ 2021-02-08 11:02 UTC (permalink / raw)
To: Dietmar Maurer, Proxmox Backup Server development discussion
On February 6, 2021 9:13 am, Dietmar Maurer wrote:
> applied patches 1..10
I'll send a v2 for pve-storage together with the Qemu changes!
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2021-02-08 11:03 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-05 15:35 [pbs-devel] [PATCH proxmox-backup 00/11] extend master key feature Fabian Grünbichler
2021-02-05 15:35 ` [pbs-devel] [PATCH proxmox-backup 01/10] key: make 'default' master key explicit Fabian Grünbichler
2021-02-05 15:35 ` [pbs-devel] [PATCH storage] pbs: allow setting up a master key Fabian Grünbichler
2021-02-05 15:35 ` [pbs-devel] [PATCH proxmox-backup 02/10] key: add show-master-pubkey command Fabian Grünbichler
2021-02-05 15:35 ` [pbs-devel] [PATCH proxmox-backup 03/10] key: rustfmt module Fabian Grünbichler
2021-02-05 15:35 ` [pbs-devel] [PATCH proxmox-backup 04/10] client: add test for keyfile_parameters Fabian Grünbichler
2021-02-06 8:00 ` Dietmar Maurer
2021-02-05 15:35 ` [pbs-devel] [PATCH proxmox-backup 05/10] client: refactor keyfile_parameters Fabian Grünbichler
2021-02-05 15:35 ` [pbs-devel] [PATCH proxmox-backup 06/10] client: allow passing specific master key Fabian Grünbichler
2021-02-05 15:35 ` [pbs-devel] [PATCH proxmox-backup 07/10] client: extend tests for master key handling Fabian Grünbichler
2021-02-05 15:35 ` [pbs-devel] [PATCH proxmox-backup 08/10] client: refactor crypto_parameter handling Fabian Grünbichler
2021-02-05 15:35 ` [pbs-devel] [PATCH proxmox-backup 09/10] client: track key source, print when used Fabian Grünbichler
2021-02-06 8:13 ` [pbs-devel] applied: [PATCH proxmox-backup 00/11] extend master key feature Dietmar Maurer
2021-02-08 11:02 ` Fabian Grünbichler
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox