From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 3E0AC6983C for ; Tue, 19 Jan 2021 13:10:24 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 2EA5A1DB09 for ; Tue, 19 Jan 2021 13:09:54 +0100 (CET) Received: from elsa.proxmox.com (212-186-127-178.static.upcbusiness.at [212.186.127.178]) by firstgate.proxmox.com (Proxmox) with ESMTP id 3851E1DAFC for ; Tue, 19 Jan 2021 13:09:52 +0100 (CET) Received: by elsa.proxmox.com (Postfix, from userid 0) id 11C9FAEA3B3; Tue, 19 Jan 2021 13:09:52 +0100 (CET) From: Dietmar Maurer To: pbs-devel@lists.proxmox.com Date: Tue, 19 Jan 2021 13:09:43 +0100 Message-Id: <20210119120943.19363-1-dietmar@proxmox.com> X-Mailer: git-send-email 2.20.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL -1.042 Adjusted score from AWL reputation of From: address KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [proxmox-backup-client.rs, key.rs, drive.rs, mod.rs] Subject: [pbs-devel] [RFC proxmox-backup] add "password hint" to KeyConfig X-BeenThere: pbs-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Backup Server development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 19 Jan 2021 12:10:24 -0000 --- 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 { - 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, - } + /// Password hint + #[serde(skip_serializing_if = "Option::is_none")] + pub hint: Option, +} 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 { + 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, 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, 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, path: Option) -> Result<(), Error> { +fn create( + kdf: Option, + path: Option, + hint: Option +) -> Result<(), Error> { let path = match path { Some(path) => PathBuf::from(path), None => { @@ -125,6 +138,10 @@ fn create(kdf: Option, path: Option) -> 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, path: Option) -> Result<(), Error> { modified: created, data: key, fingerprint: Some(crypt_config.fingerprint()), + hint: None, }, )?; } @@ -147,6 +165,7 @@ fn create(kdf: Option, path: Option) -> 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, path: Option) -> 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, path: Option, + hint: Option, ) -> 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, path: Option) -> Result<(), Error> { +fn change_passphrase( + kdf: Option, + path: Option, + hint: Option, +) -> Result<(), Error> { let path = match path { Some(path) => PathBuf::from(path), None => { @@ -273,12 +311,17 @@ fn change_passphrase(kdf: Option, path: Option) -> 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, path: Option) -> Result<(), Error modified, data: key.to_vec(), fingerprint: Some(fingerprint), + hint: None, }, )?; } @@ -297,7 +341,7 @@ fn change_passphrase(kdf: Option, path: Option) -> 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, + /// Password hint + #[serde(skip_serializing_if="Option::is_none")] + pub hint: Option, } #[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 { 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, [u8;32]) } /// Load tape encryption key configurations (public part) -pub fn load_key_configs() -> Result<(HashMap, [u8;32]), Error> { +pub fn load_key_configs() -> Result<(HashMap, [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 = serde_json::from_str(&content)?; + let key_list: Vec = 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) -> Result<(), Err Ok(()) } -pub fn save_key_configs(map: HashMap) -> Result<(), Error> { +pub fn save_key_configs(map: HashMap) -> 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) -> 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