[pbs-devel] [RFC proxmox-backup] add "password hint" to KeyConfig
Dietmar Maurer
dietmar at proxmox.com
Tue Jan 19 13:09:43 CET 2021
---
src/api2/config/tape_encryption_keys.rs | 12 ++--
src/api2/tape/drive.rs | 18 +++--
src/api2/types/mod.rs | 7 ++
src/backup/key_derivation.rs | 17 ++++-
src/bin/proxmox-backup-client.rs | 1 +
src/bin/proxmox_backup_client/key.rs | 88 +++++++++++++++++++------
src/config/tape_encryption_keys.rs | 41 +++---------
src/tape/pool_writer.rs | 5 +-
8 files changed, 122 insertions(+), 67 deletions(-)
diff --git a/src/api2/config/tape_encryption_keys.rs b/src/api2/config/tape_encryption_keys.rs
index 31a4fdfa..c652cbc5 100644
--- a/src/api2/config/tape_encryption_keys.rs
+++ b/src/api2/config/tape_encryption_keys.rs
@@ -26,6 +26,7 @@ use crate::{
api2::types::{
TAPE_ENCRYPTION_KEY_FINGERPRINT_SCHEMA,
PROXMOX_CONFIG_DIGEST_SCHEMA,
+ PASSWORD_HINT_SCHEMA,
TapeKeyMetadata,
},
backup::{
@@ -57,7 +58,7 @@ pub fn list_keys(
for (fingerprint, item) in key_map {
list.push(TapeKeyMetadata {
- hint: item.hint,
+ hint: item.hint.unwrap_or(String::new()),
fingerprint: as_fingerprint(fingerprint.bytes()),
});
}
@@ -75,9 +76,7 @@ pub fn list_keys(
min_length: 5,
},
hint: {
- description: "Password restore hint.",
- min_length: 1,
- max_length: 32,
+ schema: PASSWORD_HINT_SCHEMA,
},
},
},
@@ -92,11 +91,12 @@ pub fn create_key(
_rpcenv: &mut dyn RpcEnvironment
) -> Result<Fingerprint, Error> {
- let (key, key_config) = generate_tape_encryption_key(password.as_bytes())?;
+ let (key, mut key_config) = generate_tape_encryption_key(password.as_bytes())?;
+ key_config.hint = Some(hint);
let fingerprint = key_config.fingerprint.clone().unwrap();
- insert_key(key, key_config, hint)?;
+ insert_key(key, key_config)?;
Ok(fingerprint)
}
diff --git a/src/api2/tape/drive.rs b/src/api2/tape/drive.rs
index 5cf18fe5..2bef55d9 100644
--- a/src/api2/tape/drive.rs
+++ b/src/api2/tape/drive.rs
@@ -484,11 +484,21 @@ pub async fn restore_key(
let (_media_id, key_config) = drive.read_label()?;
if let Some(key_config) = key_config {
- let hint = String::from("fixme: add hint");
- // fixme: howto show restore hint
let password_fn = || { Ok(password.as_bytes().to_vec()) };
- let (key, ..) = decrypt_key_config(&key_config, &password_fn)?;
- config::tape_encryption_keys::insert_key(key, key_config, hint)?;
+ let key = match decrypt_key_config(&key_config, &password_fn) {
+ Ok((key, ..)) => key,
+ Err(_) => {
+ match key_config.hint {
+ Some(hint) => {
+ bail!("decrypt key failed (password hint: {})", hint);
+ }
+ None => {
+ bail!("decrypt key failed (wrong password)");
+ }
+ }
+ }
+ };
+ config::tape_encryption_keys::insert_key(key, key_config)?;
} else {
bail!("media does not contain any encryption key configuration");
}
diff --git a/src/api2/types/mod.rs b/src/api2/types/mod.rs
index 71875324..07289ca1 100644
--- a/src/api2/types/mod.rs
+++ b/src/api2/types/mod.rs
@@ -1249,3 +1249,10 @@ pub const DATASTORE_NOTIFY_STRING_SCHEMA: Schema = StringSchema::new(
"Datastore notification setting")
.format(&ApiStringFormat::PropertyString(&DatastoreNotify::API_SCHEMA))
.schema();
+
+
+pub const PASSWORD_HINT_SCHEMA: Schema = StringSchema::new("Password hint.")
+ .format(&SINGLE_LINE_COMMENT_FORMAT)
+ .min_length(1)
+ .max_length(64)
+ .schema();
diff --git a/src/backup/key_derivation.rs b/src/backup/key_derivation.rs
index b0647618..ec338166 100644
--- a/src/backup/key_derivation.rs
+++ b/src/backup/key_derivation.rs
@@ -94,7 +94,10 @@ pub struct KeyConfig {
#[serde(skip_serializing_if = "Option::is_none")]
#[serde(default)]
pub fingerprint: Option<Fingerprint>,
- }
+ /// Password hint
+ #[serde(skip_serializing_if = "Option::is_none")]
+ pub hint: Option<String>,
+}
pub fn store_key_config(
path: &std::path::Path,
@@ -181,6 +184,7 @@ pub fn encrypt_key_with_passphrase(
modified: created,
data: enc_data,
fingerprint: None,
+ hint: None,
})
}
@@ -192,6 +196,15 @@ pub fn load_and_decrypt_key(
.with_context(|| format!("failed to load decryption key from {:?}", path))
}
+/// Loads a KeyConfig from path
+pub fn load_key_config(
+ path: &std::path::Path,
+) -> Result<KeyConfig, Error> {
+ let keydata = file_get_contents(&path)?;
+ let key_config: KeyConfig = serde_json::from_reader(&keydata[..])?;
+ Ok(key_config)
+}
+
pub fn decrypt_key_config(
key_config: &KeyConfig,
passphrase: &dyn Fn() -> Result<Vec<u8>, Error>,
@@ -243,7 +256,7 @@ pub fn decrypt_key_config(
);
}
}
-
+
Ok((result, key_config.created, fingerprint))
}
diff --git a/src/bin/proxmox-backup-client.rs b/src/bin/proxmox-backup-client.rs
index 2cd00c2e..bb905cc3 100644
--- a/src/bin/proxmox-backup-client.rs
+++ b/src/bin/proxmox-backup-client.rs
@@ -929,6 +929,7 @@ async fn create_backup(
modified: proxmox::tools::time::epoch_i64(),
data: key.to_vec(),
fingerprint: Some(fingerprint),
+ hint: None,
};
let enc_key = rsa_encrypt_key_config(rsa, &key_config)?;
println!("Master key '{:?}'", path);
diff --git a/src/bin/proxmox_backup_client/key.rs b/src/bin/proxmox_backup_client/key.rs
index 109f0384..8618a6ec 100644
--- a/src/bin/proxmox_backup_client/key.rs
+++ b/src/bin/proxmox_backup_client/key.rs
@@ -19,19 +19,24 @@ use proxmox::api::router::ReturnType;
use proxmox::sys::linux::tty;
use proxmox::tools::fs::{file_get_contents, replace_file, CreateOptions};
-use proxmox_backup::backup::{
- encrypt_key_with_passphrase,
- load_and_decrypt_key,
- rsa_decrypt_key_config,
- store_key_config,
- CryptConfig,
- Kdf,
- KeyConfig,
- KeyDerivationConfig,
+use proxmox_backup::{
+ api2::types::{
+ PASSWORD_HINT_SCHEMA,
+ },
+ backup::{
+ encrypt_key_with_passphrase,
+ load_key_config,
+ decrypt_key_config,
+ rsa_decrypt_key_config,
+ store_key_config,
+ CryptConfig,
+ Kdf,
+ KeyConfig,
+ KeyDerivationConfig,
+ },
+ tools,
};
-use proxmox_backup::tools;
-
#[api()]
#[derive(Debug, Serialize, Deserialize)]
#[serde(rename_all = "lowercase")]
@@ -99,12 +104,20 @@ pub fn get_encryption_key_password() -> Result<Vec<u8>, Error> {
description:
"Output file. Without this the key will become the new default encryption key.",
optional: true,
- }
+ },
+ hint: {
+ schema: PASSWORD_HINT_SCHEMA,
+ optional: true,
+ },
},
},
)]
/// Create a new encryption key.
-fn create(kdf: Option<Kdf>, path: 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 => {
@@ -125,6 +138,10 @@ fn create(kdf: Option<Kdf>, path: Option<String>) -> Result<(), Error> {
Kdf::None => {
let created = proxmox::tools::time::epoch_i64();
+ if hint.is_some() {
+ bail!("password hint not allowed for Kdf::None");
+ }
+
store_key_config(
&path,
false,
@@ -134,6 +151,7 @@ fn create(kdf: Option<Kdf>, path: Option<String>) -> Result<(), Error> {
modified: created,
data: key,
fingerprint: Some(crypt_config.fingerprint()),
+ hint: None,
},
)?;
}
@@ -147,6 +165,7 @@ fn create(kdf: Option<Kdf>, path: Option<String>) -> Result<(), Error> {
let mut key_config = encrypt_key_with_passphrase(&key, &password, kdf)?;
key_config.fingerprint = Some(crypt_config.fingerprint());
+ key_config.hint = hint;
store_key_config(&path, false, key_config)?;
}
@@ -172,7 +191,11 @@ fn create(kdf: Option<Kdf>, path: Option<String>) -> Result<(), Error> {
description:
"Output file. Without this the key will become the new default encryption key.",
optional: true,
- }
+ },
+ hint: {
+ schema: PASSWORD_HINT_SCHEMA,
+ optional: true,
+ },
},
},
)]
@@ -182,6 +205,7 @@ async fn import_with_master_key(
encrypted_keyfile: String,
kdf: Option<Kdf>,
path: Option<String>,
+ hint: Option<String>,
) -> Result<(), Error> {
let path = match path {
Some(path) => PathBuf::from(path),
@@ -213,6 +237,10 @@ async fn import_with_master_key(
Kdf::None => {
let modified = proxmox::tools::time::epoch_i64();
+ if hint.is_some() {
+ bail!("password hint not allowed for Kdf::None");
+ }
+
store_key_config(
&path,
true,
@@ -222,6 +250,7 @@ async fn import_with_master_key(
modified,
data: key.to_vec(),
fingerprint: Some(fingerprint),
+ hint: None,
},
)?;
}
@@ -231,6 +260,7 @@ async fn import_with_master_key(
let mut new_key_config = encrypt_key_with_passphrase(&key, &password, kdf)?;
new_key_config.created = created; // keep original value
new_key_config.fingerprint = Some(fingerprint);
+ new_key_config.hint = hint;
store_key_config(&path, true, new_key_config)?;
}
@@ -249,12 +279,20 @@ async fn import_with_master_key(
path: {
description: "Key file. Without this the default key's password will be changed.",
optional: true,
- }
+ },
+ hint: {
+ schema: PASSWORD_HINT_SCHEMA,
+ optional: true,
+ },
},
},
)]
/// Change the encryption key's password.
-fn change_passphrase(kdf: Option<Kdf>, path: Option<String>) -> Result<(), Error> {
+fn change_passphrase(
+ kdf: Option<Kdf>,
+ path: Option<String>,
+ hint: Option<String>,
+) -> Result<(), Error> {
let path = match path {
Some(path) => PathBuf::from(path),
None => {
@@ -273,12 +311,17 @@ fn change_passphrase(kdf: Option<Kdf>, path: Option<String>) -> Result<(), Error
bail!("unable to change passphrase - no tty");
}
- let (key, created, fingerprint) = load_and_decrypt_key(&path, &get_encryption_key_password)?;
+ let key_config = load_key_config(&path)?;
+ let (key, created, fingerprint) = decrypt_key_config(&key_config, &get_encryption_key_password)?;
match kdf {
Kdf::None => {
let modified = proxmox::tools::time::epoch_i64();
+ if hint.is_some() {
+ bail!("password hint not allowed for Kdf::None");
+ }
+
store_key_config(
&path,
true,
@@ -288,6 +331,7 @@ fn change_passphrase(kdf: Option<Kdf>, path: Option<String>) -> Result<(), Error
modified,
data: key.to_vec(),
fingerprint: Some(fingerprint),
+ hint: None,
},
)?;
}
@@ -297,7 +341,7 @@ fn change_passphrase(kdf: Option<Kdf>, path: Option<String>) -> Result<(), Error
let mut new_key_config = encrypt_key_with_passphrase(&key, &password, kdf)?;
new_key_config.created = created; // keep original value
new_key_config.fingerprint = Some(fingerprint);
-
+ new_key_config.hint = hint;
store_key_config(&path, true, new_key_config)?;
}
}
@@ -323,7 +367,11 @@ struct KeyInfo {
/// Key modification time
pub modified: i64,
/// Key fingerprint
+ #[serde(skip_serializing_if="Option::is_none")]
pub fingerprint: Option<String>,
+ /// Password hint
+ #[serde(skip_serializing_if="Option::is_none")]
+ pub hint: Option<String>,
}
#[api(
@@ -374,6 +422,7 @@ fn show_key(
Some(ref fp) => Some(format!("{}", fp)),
None => None,
},
+ hint: config.hint,
};
let options = proxmox::api::cli::default_table_format_options()
@@ -381,7 +430,8 @@ fn show_key(
.column(ColumnConfig::new("kdf"))
.column(ColumnConfig::new("created").renderer(tools::format::render_epoch))
.column(ColumnConfig::new("modified").renderer(tools::format::render_epoch))
- .column(ColumnConfig::new("fingerprint"));
+ .column(ColumnConfig::new("fingerprint"))
+ .column(ColumnConfig::new("hint"));
let return_type = ReturnType::new(false, &KeyInfo::API_SCHEMA);
diff --git a/src/config/tape_encryption_keys.rs b/src/config/tape_encryption_keys.rs
index ff565349..13f6961c 100644
--- a/src/config/tape_encryption_keys.rs
+++ b/src/config/tape_encryption_keys.rs
@@ -1,4 +1,4 @@
-use std::collections::{HashSet, HashMap};
+use std::collections::HashMap;
use anyhow::{bail, Error};
use serde::{Deserialize, Serialize};
@@ -53,13 +53,6 @@ pub struct EncryptionKeyInfo {
pub key: [u8; 32],
}
-/// Store Hardware Encryption keys (public part)
-#[derive(Deserialize, Serialize)]
-pub struct EncryptionKeyConfig {
- pub hint: String,
- pub key_config: KeyConfig,
-}
-
pub fn compute_tape_key_fingerprint(key: &[u8; 32]) -> Result<Fingerprint, Error> {
let crypt_config = CryptConfig::new(key.clone())?;
Ok(crypt_config.fingerprint())
@@ -81,12 +74,6 @@ impl EncryptionKeyInfo {
}
}
-impl EncryptionKeyConfig {
- pub fn new(key_config: KeyConfig, hint: String) -> Self {
- Self { hint, key_config }
- }
-}
-
pub const TAPE_KEYS_FILENAME: &str = "/etc/proxmox-backup/tape-encryption-keys.json";
pub const TAPE_KEY_CONFIG_FILENAME: &str = "/etc/proxmox-backup/tape-encryption-key-config.json";
pub const TAPE_KEYS_LOCKFILE: &str = "/etc/proxmox-backup/.tape-encryption-keys.lck";
@@ -122,25 +109,21 @@ pub fn load_keys() -> Result<(HashMap<Fingerprint, EncryptionKeyInfo>, [u8;32])
}
/// Load tape encryption key configurations (public part)
-pub fn load_key_configs() -> Result<(HashMap<Fingerprint, EncryptionKeyConfig>, [u8;32]), Error> {
+pub fn load_key_configs() -> Result<(HashMap<Fingerprint, KeyConfig>, [u8;32]), Error> {
let content = file_read_optional_string(TAPE_KEY_CONFIG_FILENAME)?;
let content = content.unwrap_or_else(|| String::from("[]"));
let digest = openssl::sha::sha256(content.as_bytes());
- let key_list: Vec<EncryptionKeyConfig> = serde_json::from_str(&content)?;
+ let key_list: Vec<KeyConfig> = serde_json::from_str(&content)?;
let mut map = HashMap::new();
- let mut hint_set = HashSet::new();
- for item in key_list {
- match item.key_config.fingerprint {
+ for key_config in key_list {
+ match key_config.fingerprint {
Some(ref fingerprint) => {
- if !hint_set.insert(item.hint.clone()) {
- bail!("found duplicate password hint '{}'", item.hint);
- }
- if map.insert(fingerprint.clone(), item).is_some() {
+ if map.insert(fingerprint.clone(), key_config).is_some() {
bail!("found duplicate fingerprint");
}
}
@@ -174,16 +157,11 @@ pub fn save_keys(map: HashMap<Fingerprint, EncryptionKeyInfo>) -> Result<(), Err
Ok(())
}
-pub fn save_key_configs(map: HashMap<Fingerprint, EncryptionKeyConfig>) -> Result<(), Error> {
+pub fn save_key_configs(map: HashMap<Fingerprint, KeyConfig>) -> Result<(), Error> {
let mut list = Vec::new();
- let mut hint_set = HashSet::new();
-
for (_fp, item) in map {
- if !hint_set.insert(item.hint.clone()) {
- bail!("found duplicate password hint '{}'", item.hint);
- }
list.push(item);
}
@@ -203,7 +181,7 @@ pub fn save_key_configs(map: HashMap<Fingerprint, EncryptionKeyConfig>) -> Resul
Ok(())
}
-pub fn insert_key(key: [u8;32], key_config: KeyConfig, hint: String) -> Result<(), Error> {
+pub fn insert_key(key: [u8;32], key_config: KeyConfig) -> Result<(), Error> {
let _lock = open_file_locked(
TAPE_KEYS_LOCKFILE,
@@ -227,8 +205,7 @@ pub fn insert_key(key: [u8;32], key_config: KeyConfig, hint: String) -> Result<(
key_map.insert(fingerprint.clone(), item);
save_keys(key_map)?;
- let item = EncryptionKeyConfig::new(key_config, hint);
- config_map.insert(fingerprint.clone(), item);
+ config_map.insert(fingerprint.clone(), key_config);
save_key_configs(config_map)?;
Ok(())
diff --git a/src/tape/pool_writer.rs b/src/tape/pool_writer.rs
index b04bfddb..c085b0b5 100644
--- a/src/tape/pool_writer.rs
+++ b/src/tape/pool_writer.rs
@@ -456,10 +456,7 @@ fn update_media_set_label(
let key_config = if let Some(ref fingerprint) = new_set.encryption_key_fingerprint {
let (config_map, _digest) = load_key_configs()?;
match config_map.get(fingerprint) {
- Some(item) => {
- // fixme: store item.hint??? should be in key-config instead
- Some(item.key_config.clone())
- }
+ Some(key_config) => Some(key_config.clone()),
None => {
bail!("unable to find tape encryption key config '{}'", fingerprint);
}
--
2.20.1
More information about the pbs-devel
mailing list