add "password hint" to KeyConfig

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.",
+                schema: PASSWORD_HINT_SCHEMA,
-                min_length: 1,
-                max_length: 32,
             },
         },
     },
@@ -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)
 }

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)?;
+            let key = match decrypt_key_config(&key_config, &password_fn) {
-            config::tape_encryption_keys::insert_key(key, key_config, hint)?;
+                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");
         }

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();

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))
 }
 
@@ -311,6 +324,7 @@ fn encrypt_decrypt_test() -> Result<(), Error> {
             14, 171, 212, 70, 11, 110, 185, 202, 52, 80, 35, 222, 226, 183, 120, 199, 144, 229, 74,
             22, 131, 185, 101, 156, 10, 87, 174, 25, 144, 144, 21, 155,
         ])),
+        hint: None,
     };
 
     let encrypted = rsa_encrypt_key_config(public.clone(), &key).expect("encryption failed");
@@ -333,6 +347,7 @@ fn fingerprint_checks() -> Result<(), Error> {
         modified: proxmox::tools::time::epoch_i64(),
         data: (0u8..32u8).collect(),
         fingerprint: Some(Fingerprint::new([0u8; 32])), // wrong FP
+        hint: None,
     };
 
     let expected_fingerprint = Fingerprint::new([
@@ -349,6 +364,7 @@ fn fingerprint_checks() -> Result<(), Error> {
         modified: proxmox::tools::time::epoch_i64(),
         data: (0u8..32u8).collect(),
         fingerprint: None,
+        hint: None,
     };
 
 

src/bin/proxmox-backup-client.rs

@@ -923,13 +923,11 @@ async fn create_backup(
                 Some(ref path) if path.exists() => {
                     let pem_data = file_get_contents(path)?;
                     let rsa = openssl::rsa::Rsa::public_key_from_pem(&pem_data)?;
-                    let key_config = KeyConfig {
+
-                        kdf: None,
+                    let mut key_config = KeyConfig::without_password(key);
-                        created,
+                    key_config.created = created; // keep original value
-                        modified: proxmox::tools::time::epoch_i64(),
+                    key_config.fingerprint = Some(fingerprint);
-                        data: key.to_vec(),
+
-                        fingerprint: Some(fingerprint),
-                    };
                     let enc_key = rsa_encrypt_key_config(rsa, &key_config)?;
                     println!("Master key '{:?}'", path);
 

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::{
+use proxmox_backup::{
-    encrypt_key_with_passphrase,
+    api2::types::{
-    load_and_decrypt_key,
+        PASSWORD_HINT_SCHEMA,
-    rsa_decrypt_key_config,
+    },
-    store_key_config,
+    backup::{
-    CryptConfig,
+        encrypt_key_with_passphrase,
-    Kdf,
+        load_key_config,
-    KeyConfig,
+        decrypt_key_config,
-    KeyDerivationConfig,
+        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);
 

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 {
+    for key_config in key_list {
-        match item.key_config.fingerprint {
+        match key_config.fingerprint {
             Some(ref fingerprint) => {
-                if !hint_set.insert(item.hint.clone()) {
+                if map.insert(fingerprint.clone(), key_config).is_some() {
-                    bail!("found duplicate password hint '{}'", item.hint);
-                }
-                if map.insert(fingerprint.clone(), item).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(), key_config);
-    config_map.insert(fingerprint.clone(), item);
     save_key_configs(config_map)?;
 
     Ok(())

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) => {
+            Some(key_config) => Some(key_config.clone()),
-                // fixme: store item.hint??? should be in key-config instead
-                Some(item.key_config.clone())
-            }
             None => {
                 bail!("unable to find tape encryption key config '{}'", fingerprint);
             }