* [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* [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