From 2f26b8668ac97aff9176528293dff52a3eb70275 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Fabian=20Gr=C3=BCnbichler?= Date: Fri, 5 Feb 2021 16:35:35 +0100 Subject: [PATCH] client: track key source, print when used MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit 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 --- 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, +} + +impl KeyWithSource { + pub fn from_fd(key: Vec) -> Self { + Self { + source: KeySource::Fd, + key, + } + } + + pub fn from_default(key: Vec) -> Self { + Self { + source: KeySource::DefaultKey, + key, + } + } + + pub fn from_path(path: String, key: Vec) -> Self { + Self { + source: KeySource::Path(path), + key, + } + } +} + #[derive(Debug, Eq, PartialEq)] struct CryptoParams { mode: CryptMode, - enc_key: Option>, + enc_key: Option, // FIXME switch to openssl::rsa::rsa once that is Eq? - master_pubkey: Option>, + master_pubkey: Option, } fn crypto_parameters(param: &Value) -> Result { @@ -656,52 +700,47 @@ fn crypto_parameters(param: &Value) -> Result { 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 { (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 { }, // 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 { }, // 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 { // 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 { 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 { 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 { 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 { } #[cfg(not(test))] -pub fn read_optional_default_encryption_key() -> Result>, Error> { +pub(crate) fn read_optional_default_encryption_key() -> Result, 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>, Error> { +pub(crate) fn read_optional_default_master_pubkey() -> Result, 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>, Error> { static mut TEST_DEFAULT_ENCRYPTION_KEY: Result>, Error> = Ok(None); #[cfg(test)] -pub fn read_optional_default_encryption_key() -> Result>, Error> { +pub(crate) fn read_optional_default_encryption_key() -> Result, 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>, Error> #[cfg(test)] // not safe when multiple concurrent test cases end up here! -pub unsafe fn set_test_encryption_key(value: Result>, Error>) { +pub(crate) unsafe fn set_test_encryption_key(value: Result>, Error>) { TEST_DEFAULT_ENCRYPTION_KEY = value; } @@ -89,11 +92,12 @@ pub unsafe fn set_test_encryption_key(value: Result>, Error>) { static mut TEST_DEFAULT_MASTER_PUBKEY: Result>, Error> = Ok(None); #[cfg(test)] -pub fn read_optional_default_master_pubkey() -> Result>, Error> { +pub(crate) fn read_optional_default_master_pubkey() -> Result, 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>, Error> { #[cfg(test)] // not safe when multiple concurrent test cases end up here! -pub unsafe fn set_test_default_master_pubkey(value: Result>, Error>) { +pub(crate) unsafe fn set_test_default_master_pubkey(value: Result>, 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 { 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.39.2