[pbs-devel] [PATCH proxmox-backup 05/10] client: refactor keyfile_parameters
Fabian Grünbichler
f.gruenbichler at proxmox.com
Fri Feb 5 16:35:31 CET 2021
no semantic changes intended
Signed-off-by: Fabian Grünbichler <f.gruenbichler at 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
More information about the pbs-devel
mailing list