* [pbs-devel] [PATCH proxmox-backup 01/13] crypt config: add fingerprint mechanism
2020-11-20 16:38 [pbs-devel] [PATCH v2 proxmox-backup(-qemu) 00/15] add, persist and check fingerprint Fabian Grünbichler
@ 2020-11-20 16:38 ` Fabian Grünbichler
2020-11-20 16:38 ` [pbs-devel] [PATCH proxmox-backup 02/13] key: add fingerprint to key config Fabian Grünbichler
` (14 subsequent siblings)
15 siblings, 0 replies; 26+ messages in thread
From: Fabian Grünbichler @ 2020-11-20 16:38 UTC (permalink / raw)
To: pbs-devel
by computing the ID digest of a hash of a static string.
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
Notes:
v2: wrap fingerprint in Struct
src/backup/crypt_config.rs | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/src/backup/crypt_config.rs b/src/backup/crypt_config.rs
index 4be728d9..8a4fe0e3 100644
--- a/src/backup/crypt_config.rs
+++ b/src/backup/crypt_config.rs
@@ -7,6 +7,8 @@
//! encryption](https://en.wikipedia.org/wiki/Authenticated_encryption)
//! for a short introduction.
+use std::fmt;
+use std::fmt::Display;
use std::io::Write;
use anyhow::{bail, Error};
@@ -17,6 +19,11 @@ use serde::{Deserialize, Serialize};
use proxmox::api::api;
+// openssl::sha::sha256(b"Proxmox Backup Encryption Key Fingerprint")
+const FINGERPRINT_INPUT: [u8; 32] = [ 110, 208, 239, 119, 71, 31, 255, 77,
+ 85, 199, 168, 254, 74, 157, 182, 33,
+ 97, 64, 127, 19, 76, 114, 93, 223,
+ 48, 153, 45, 37, 236, 69, 237, 38, ];
#[api(default: "encrypt")]
#[derive(Copy, Clone, Debug, Eq, PartialEq, Deserialize, Serialize)]
#[serde(rename_all = "kebab-case")]
@@ -30,6 +37,17 @@ pub enum CryptMode {
SignOnly,
}
+/// 32-byte fingerprint, usually calculated with SHA256.
+pub struct Fingerprint {
+ bytes: [u8; 32],
+}
+
+impl Display for Fingerprint {
+ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+ write!(f, "{:?}", self.bytes)
+ }
+}
+
/// Encryption Configuration with secret key
///
/// This structure stores the secret key and provides helpers for
@@ -101,6 +119,12 @@ impl CryptConfig {
tag
}
+ pub fn fingerprint(&self) -> Fingerprint {
+ Fingerprint {
+ bytes: self.compute_digest(&FINGERPRINT_INPUT)
+ }
+ }
+
pub fn data_crypter(&self, iv: &[u8; 16], mode: Mode) -> Result<Crypter, Error> {
let mut crypter = openssl::symm::Crypter::new(self.cipher, mode, &self.enc_key, Some(iv))?;
crypter.aad_update(b"")?; //??
--
2.20.1
^ permalink raw reply [flat|nested] 26+ messages in thread* [pbs-devel] [PATCH proxmox-backup 02/13] key: add fingerprint to key config
2020-11-20 16:38 [pbs-devel] [PATCH v2 proxmox-backup(-qemu) 00/15] add, persist and check fingerprint Fabian Grünbichler
2020-11-20 16:38 ` [pbs-devel] [PATCH proxmox-backup 01/13] crypt config: add fingerprint mechanism Fabian Grünbichler
@ 2020-11-20 16:38 ` Fabian Grünbichler
2020-11-23 8:07 ` Wolfgang Bumiller
2020-11-20 16:38 ` [pbs-devel] [PATCH proxmox-backup 03/13] client: print key fingerprint and master key Fabian Grünbichler
` (13 subsequent siblings)
15 siblings, 1 reply; 26+ messages in thread
From: Fabian Grünbichler @ 2020-11-20 16:38 UTC (permalink / raw)
To: pbs-devel
and set/generate it on
- key creation
- key passphrase change
- key decryption if not already set
- key encryption with master key
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
Notes:
v2: simplify after switchover to Fingerprint struct, incorporate
feedback
an existing passwordless key can be 'updated' non-interactively with
proxmox-backup-client key change-passphrase $path --kdf none
something like this could go into the/some postinst to retro-actively give all
the PVE generated storage keys a fingerprint field. this only affects the 'key
show' subcommand (introduced later in this series) or manual checks of the key
file, any operations actually using the key have access to its CryptConfig and
thus the fingerprint() function anyway..
an existing passwordless key can be 'updated' non-interactively with
proxmox-backup-client key change-passphrase $path --kdf none
something like this could go into the/some postinst to retro-actively give all
the PVE generated storage keys a fingerprint field. this only affects the 'key
show' subcommand (introduced later in this series) or manual checks of the key
file, any operations actually using the key have access to its CryptConfig and
thus the fingerprint() function anyway..
src/backup/crypt_config.rs | 16 ++++++++--
src/backup/key_derivation.rs | 22 +++++++++++---
src/bin/proxmox-backup-client.rs | 6 ++--
src/bin/proxmox_backup_client/benchmark.rs | 2 +-
src/bin/proxmox_backup_client/catalog.rs | 4 +--
src/bin/proxmox_backup_client/key.rs | 19 +++++++++---
src/bin/proxmox_backup_client/mount.rs | 2 +-
src/tools/format.rs | 34 ++++++++++++++++++++++
8 files changed, 88 insertions(+), 17 deletions(-)
diff --git a/src/backup/crypt_config.rs b/src/backup/crypt_config.rs
index 8a4fe0e3..7d27706a 100644
--- a/src/backup/crypt_config.rs
+++ b/src/backup/crypt_config.rs
@@ -17,6 +17,8 @@ use openssl::pkcs5::pbkdf2_hmac;
use openssl::symm::{decrypt_aead, Cipher, Crypter, Mode};
use serde::{Deserialize, Serialize};
+use crate::tools::format::{as_fingerprint, bytes_as_fingerprint};
+
use proxmox::api::api;
// openssl::sha::sha256(b"Proxmox Backup Encryption Key Fingerprint")
@@ -37,14 +39,18 @@ pub enum CryptMode {
SignOnly,
}
+#[derive(Debug, Eq, PartialEq, Deserialize, Serialize)]
+#[serde(transparent)]
/// 32-byte fingerprint, usually calculated with SHA256.
pub struct Fingerprint {
+ #[serde(with = "bytes_as_fingerprint")]
bytes: [u8; 32],
}
+/// Display as short key ID
impl Display for Fingerprint {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
- write!(f, "{:?}", self.bytes)
+ write!(f, "{}", as_fingerprint(&self.bytes[0..8]))
}
}
@@ -243,7 +249,13 @@ impl CryptConfig {
) -> Result<Vec<u8>, Error> {
let modified = proxmox::tools::time::epoch_i64();
- let key_config = super::KeyConfig { kdf: None, created, modified, data: self.enc_key.to_vec() };
+ let key_config = super::KeyConfig {
+ kdf: None,
+ created,
+ modified,
+ data: self.enc_key.to_vec(),
+ fingerprint: Some(self.fingerprint()),
+ };
let data = serde_json::to_string(&key_config)?.as_bytes().to_vec();
let mut buffer = vec![0u8; rsa.size() as usize];
diff --git a/src/backup/key_derivation.rs b/src/backup/key_derivation.rs
index 2c807723..e7b266ab 100644
--- a/src/backup/key_derivation.rs
+++ b/src/backup/key_derivation.rs
@@ -2,6 +2,8 @@ use anyhow::{bail, format_err, Context, Error};
use serde::{Deserialize, Serialize};
+use crate::backup::{CryptConfig, Fingerprint};
+
use proxmox::tools::fs::{file_get_contents, replace_file, CreateOptions};
use proxmox::try_block;
@@ -66,6 +68,9 @@ pub struct KeyConfig {
pub modified: i64,
#[serde(with = "proxmox::tools::serde::bytes_as_base64")]
pub data: Vec<u8>,
+ #[serde(skip_serializing_if = "Option::is_none")]
+ #[serde(default)]
+ pub fingerprint: Option<Fingerprint>,
}
pub fn store_key_config(
@@ -142,13 +147,14 @@ pub fn encrypt_key_with_passphrase(
created,
modified: created,
data: enc_data,
+ fingerprint: None,
})
}
pub fn load_and_decrypt_key(
path: &std::path::Path,
passphrase: &dyn Fn() -> Result<Vec<u8>, Error>,
-) -> Result<([u8;32], i64), Error> {
+) -> Result<([u8;32], i64, Fingerprint), Error> {
do_load_and_decrypt_key(path, passphrase)
.with_context(|| format!("failed to load decryption key from {:?}", path))
}
@@ -156,14 +162,14 @@ pub fn load_and_decrypt_key(
fn do_load_and_decrypt_key(
path: &std::path::Path,
passphrase: &dyn Fn() -> Result<Vec<u8>, Error>,
-) -> Result<([u8;32], i64), Error> {
+) -> Result<([u8;32], i64, Fingerprint), Error> {
decrypt_key(&file_get_contents(&path)?, passphrase)
}
pub fn decrypt_key(
mut keydata: &[u8],
passphrase: &dyn Fn() -> Result<Vec<u8>, Error>,
-) -> Result<([u8;32], i64), Error> {
+) -> Result<([u8;32], i64, Fingerprint), Error> {
let key_config: KeyConfig = serde_json::from_reader(&mut keydata)?;
let raw_data = key_config.data;
@@ -203,5 +209,13 @@ pub fn decrypt_key(
let mut result = [0u8; 32];
result.copy_from_slice(&key);
- Ok((result, created))
+ let fingerprint = match key_config.fingerprint {
+ Some(fingerprint) => fingerprint,
+ None => {
+ let crypt_config = CryptConfig::new(result.clone())?;
+ crypt_config.fingerprint()
+ },
+ };
+
+ Ok((result, created, fingerprint))
}
diff --git a/src/bin/proxmox-backup-client.rs b/src/bin/proxmox-backup-client.rs
index 70aba77f..c63c7eb6 100644
--- a/src/bin/proxmox-backup-client.rs
+++ b/src/bin/proxmox-backup-client.rs
@@ -1069,7 +1069,7 @@ async fn create_backup(
let (crypt_config, rsa_encrypted_key) = match keydata {
None => (None, None),
Some(key) => {
- let (key, created) = decrypt_key(&key, &key::get_encryption_key_password)?;
+ let (key, created, _fingerprint) = decrypt_key(&key, &key::get_encryption_key_password)?;
let crypt_config = CryptConfig::new(key)?;
@@ -1380,7 +1380,7 @@ async fn restore(param: Value) -> Result<Value, Error> {
let crypt_config = match keydata {
None => None,
Some(key) => {
- let (key, _) = decrypt_key(&key, &key::get_encryption_key_password)?;
+ let (key, _, _) = decrypt_key(&key, &key::get_encryption_key_password)?;
Some(Arc::new(CryptConfig::new(key)?))
}
};
@@ -1538,7 +1538,7 @@ async fn upload_log(param: Value) -> Result<Value, Error> {
let crypt_config = match keydata {
None => None,
Some(key) => {
- let (key, _created) = decrypt_key(&key, &key::get_encryption_key_password)?;
+ let (key, _created, _) = decrypt_key(&key, &key::get_encryption_key_password)?;
let crypt_config = CryptConfig::new(key)?;
Some(Arc::new(crypt_config))
}
diff --git a/src/bin/proxmox_backup_client/benchmark.rs b/src/bin/proxmox_backup_client/benchmark.rs
index b0d769e8..e53e43ce 100644
--- a/src/bin/proxmox_backup_client/benchmark.rs
+++ b/src/bin/proxmox_backup_client/benchmark.rs
@@ -151,7 +151,7 @@ pub async fn benchmark(
let crypt_config = match keyfile {
None => None,
Some(path) => {
- let (key, _) = load_and_decrypt_key(&path, &crate::key::get_encryption_key_password)?;
+ let (key, _, _) = load_and_decrypt_key(&path, &crate::key::get_encryption_key_password)?;
let crypt_config = CryptConfig::new(key)?;
Some(Arc::new(crypt_config))
}
diff --git a/src/bin/proxmox_backup_client/catalog.rs b/src/bin/proxmox_backup_client/catalog.rs
index e4931e10..37ad842f 100644
--- a/src/bin/proxmox_backup_client/catalog.rs
+++ b/src/bin/proxmox_backup_client/catalog.rs
@@ -73,7 +73,7 @@ async fn dump_catalog(param: Value) -> Result<Value, Error> {
let crypt_config = match keydata {
None => None,
Some(key) => {
- let (key, _created) = decrypt_key(&key, &get_encryption_key_password)?;
+ let (key, _created, _fingerprint) = decrypt_key(&key, &get_encryption_key_password)?;
let crypt_config = CryptConfig::new(key)?;
Some(Arc::new(crypt_config))
}
@@ -170,7 +170,7 @@ async fn catalog_shell(param: Value) -> Result<(), Error> {
let crypt_config = match keydata {
None => None,
Some(key) => {
- let (key, _created) = decrypt_key(&key, &get_encryption_key_password)?;
+ let (key, _created, _fingerprint) = decrypt_key(&key, &get_encryption_key_password)?;
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 8b802b3e..193367e3 100644
--- a/src/bin/proxmox_backup_client/key.rs
+++ b/src/bin/proxmox_backup_client/key.rs
@@ -11,7 +11,11 @@ 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, store_key_config, KeyConfig,
+ encrypt_key_with_passphrase,
+ load_and_decrypt_key,
+ store_key_config,
+ CryptConfig,
+ KeyConfig,
};
use proxmox_backup::tools;
@@ -120,7 +124,10 @@ fn create(kdf: Option<Kdf>, path: Option<String>) -> Result<(), Error> {
let kdf = kdf.unwrap_or_default();
- let key = proxmox::sys::linux::random_data(32)?;
+ let mut key_array = [0u8; 32];
+ proxmox::sys::linux::fill_with_random_data(&mut key_array)?;
+ let crypt_config = CryptConfig::new(key_array.clone())?;
+ let key = key_array.to_vec();
match kdf {
Kdf::None => {
@@ -134,6 +141,7 @@ fn create(kdf: Option<Kdf>, path: Option<String>) -> Result<(), Error> {
created,
modified: created,
data: key,
+ fingerprint: Some(crypt_config.fingerprint()),
},
)?;
}
@@ -145,7 +153,8 @@ fn create(kdf: Option<Kdf>, path: Option<String>) -> Result<(), Error> {
let password = tty::read_and_verify_password("Encryption Key Password: ")?;
- let key_config = encrypt_key_with_passphrase(&key, &password)?;
+ let mut key_config = encrypt_key_with_passphrase(&key, &password)?;
+ key_config.fingerprint = Some(crypt_config.fingerprint());
store_key_config(&path, false, key_config)?;
}
@@ -188,7 +197,7 @@ fn change_passphrase(kdf: Option<Kdf>, path: Option<String>) -> Result<(), Error
bail!("unable to change passphrase - no tty");
}
- let (key, created) = load_and_decrypt_key(&path, &get_encryption_key_password)?;
+ let (key, created, fingerprint) = load_and_decrypt_key(&path, &get_encryption_key_password)?;
match kdf {
Kdf::None => {
@@ -202,6 +211,7 @@ fn change_passphrase(kdf: Option<Kdf>, path: Option<String>) -> Result<(), Error
created, // keep original value
modified,
data: key.to_vec(),
+ fingerprint: Some(fingerprint),
},
)?;
}
@@ -210,6 +220,7 @@ fn change_passphrase(kdf: Option<Kdf>, path: Option<String>) -> Result<(), Error
let mut new_key_config = encrypt_key_with_passphrase(&key, &password)?;
new_key_config.created = created; // keep original value
+ new_key_config.fingerprint = Some(fingerprint);
store_key_config(&path, true, new_key_config)?;
}
diff --git a/src/bin/proxmox_backup_client/mount.rs b/src/bin/proxmox_backup_client/mount.rs
index 415fbf9d..6283961e 100644
--- a/src/bin/proxmox_backup_client/mount.rs
+++ b/src/bin/proxmox_backup_client/mount.rs
@@ -182,7 +182,7 @@ async fn mount_do(param: Value, pipe: Option<RawFd>) -> Result<Value, Error> {
let crypt_config = match keyfile {
None => None,
Some(path) => {
- let (key, _) = load_and_decrypt_key(&path, &crate::key::get_encryption_key_password)?;
+ let (key, _, _) = load_and_decrypt_key(&path, &crate::key::get_encryption_key_password)?;
Some(Arc::new(CryptConfig::new(key)?))
}
};
diff --git a/src/tools/format.rs b/src/tools/format.rs
index 8fe6aa82..c9f50053 100644
--- a/src/tools/format.rs
+++ b/src/tools/format.rs
@@ -102,6 +102,40 @@ impl From<u64> for HumanByte {
}
}
+pub fn as_fingerprint(bytes: &[u8]) -> String {
+ proxmox::tools::digest_to_hex(bytes)
+ .as_bytes()
+ .chunks(2)
+ .map(|v| std::str::from_utf8(v).unwrap())
+ .collect::<Vec<&str>>().join(":")
+}
+
+pub mod bytes_as_fingerprint {
+ use serde::{Deserialize, Serializer, Deserializer};
+
+ pub fn serialize<S>(
+ bytes: &[u8; 32],
+ serializer: S,
+ ) -> Result<S::Ok, S::Error>
+ where
+ S: Serializer,
+ {
+ let s = crate::tools::format::as_fingerprint(bytes);
+ serializer.serialize_str(&s)
+ }
+
+ pub fn deserialize<'de, D>(
+ deserializer: D,
+ ) -> Result<[u8; 32], D::Error>
+ where
+ D: Deserializer<'de>,
+ {
+ let mut s = String::deserialize(deserializer)?;
+ s.retain(|c| c != ':');
+ proxmox::tools::hex_to_digest(&s).map_err(serde::de::Error::custom)
+ }
+}
+
#[test]
fn correct_byte_convert() {
fn convert(b: usize) -> String {
--
2.20.1
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [pbs-devel] [PATCH proxmox-backup 02/13] key: add fingerprint to key config
2020-11-20 16:38 ` [pbs-devel] [PATCH proxmox-backup 02/13] key: add fingerprint to key config Fabian Grünbichler
@ 2020-11-23 8:07 ` Wolfgang Bumiller
0 siblings, 0 replies; 26+ messages in thread
From: Wolfgang Bumiller @ 2020-11-23 8:07 UTC (permalink / raw)
To: Fabian Grünbichler; +Cc: pbs-devel
On Fri, Nov 20, 2020 at 05:38:32PM +0100, Fabian Grünbichler wrote:
> diff --git a/src/tools/format.rs b/src/tools/format.rs
> index 8fe6aa82..c9f50053 100644
> --- a/src/tools/format.rs
> +++ b/src/tools/format.rs
> @@ -102,6 +102,40 @@ impl From<u64> for HumanByte {
> }
> }
>
Minor nit (maybe for a follow-up cleanup depending on how the rest of
the series goes):
> +pub fn as_fingerprint(bytes: &[u8]) -> String {
> + proxmox::tools::digest_to_hex(bytes)
> + .as_bytes()
> + .chunks(2)
> + .map(|v| std::str::from_utf8(v).unwrap())
^ this performs an unnecessary check, even if you just unwrap it, use
unsafe { std::str::from_utf8_unchecked(v) }
> + .collect::<Vec<&str>>().join(":")
^ this allocates multiple tiny vectors.
Since you already know even the length of the resulting string it's
nicer to just allocate it before hand and fill it in a loop.
lex hex = proxmox::tools::digest_to_hex(bytes);
// avoid underflow:
if hex.is_empty() {
return String::new();
}
let mut out = String::with_capacity((hex.len() / 2) - 1);
for pair in hex.as_bytes().chunks(2) {
if !out.is_empty() {
out.push(':');
}
out.push_str(unsafe { std::str::from_utf8_unchecked(pair) });
}
out
^ permalink raw reply [flat|nested] 26+ messages in thread
* [pbs-devel] [PATCH proxmox-backup 03/13] client: print key fingerprint and master key
2020-11-20 16:38 [pbs-devel] [PATCH v2 proxmox-backup(-qemu) 00/15] add, persist and check fingerprint Fabian Grünbichler
2020-11-20 16:38 ` [pbs-devel] [PATCH proxmox-backup 01/13] crypt config: add fingerprint mechanism Fabian Grünbichler
2020-11-20 16:38 ` [pbs-devel] [PATCH proxmox-backup 02/13] key: add fingerprint to key config Fabian Grünbichler
@ 2020-11-20 16:38 ` Fabian Grünbichler
2020-11-20 16:38 ` [pbs-devel] [PATCH proxmox-backup 04/13] client: add 'key show' command Fabian Grünbichler
` (12 subsequent siblings)
15 siblings, 0 replies; 26+ messages in thread
From: Fabian Grünbichler @ 2020-11-20 16:38 UTC (permalink / raw)
To: pbs-devel
for operations where it makes sense.
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
Notes:
v2: print FP directly (short key ID via Display)
src/bin/proxmox-backup-client.rs | 9 +++++++--
src/bin/proxmox_backup_client/mount.rs | 4 +++-
2 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/src/bin/proxmox-backup-client.rs b/src/bin/proxmox-backup-client.rs
index c63c7eb6..ee2623f0 100644
--- a/src/bin/proxmox-backup-client.rs
+++ b/src/bin/proxmox-backup-client.rs
@@ -1069,7 +1069,8 @@ async fn create_backup(
let (crypt_config, rsa_encrypted_key) = match keydata {
None => (None, None),
Some(key) => {
- let (key, created, _fingerprint) = decrypt_key(&key, &key::get_encryption_key_password)?;
+ let (key, created, fingerprint) = decrypt_key(&key, &key::get_encryption_key_password)?;
+ println!("Encryption key fingerprint: {}", fingerprint);
let crypt_config = CryptConfig::new(key)?;
@@ -1078,6 +1079,8 @@ async fn create_backup(
let pem_data = file_get_contents(path)?;
let rsa = openssl::rsa::Rsa::public_key_from_pem(&pem_data)?;
let enc_key = crypt_config.generate_rsa_encoded_key(rsa, created)?;
+ println!("Master key '{:?}'", path);
+
(Some(Arc::new(crypt_config)), Some(enc_key))
}
_ => (Some(Arc::new(crypt_config)), None),
@@ -1380,7 +1384,8 @@ async fn restore(param: Value) -> Result<Value, Error> {
let crypt_config = match keydata {
None => None,
Some(key) => {
- let (key, _, _) = decrypt_key(&key, &key::get_encryption_key_password)?;
+ let (key, _, fingerprint) = decrypt_key(&key, &key::get_encryption_key_password)?;
+ println!("Encryption key fingerprint: '{}'", fingerprint);
Some(Arc::new(CryptConfig::new(key)?))
}
};
diff --git a/src/bin/proxmox_backup_client/mount.rs b/src/bin/proxmox_backup_client/mount.rs
index 6283961e..187deab5 100644
--- a/src/bin/proxmox_backup_client/mount.rs
+++ b/src/bin/proxmox_backup_client/mount.rs
@@ -182,7 +182,9 @@ async fn mount_do(param: Value, pipe: Option<RawFd>) -> Result<Value, Error> {
let crypt_config = match keyfile {
None => None,
Some(path) => {
- let (key, _, _) = load_and_decrypt_key(&path, &crate::key::get_encryption_key_password)?;
+ println!("Encryption key file: '{:?}'", path);
+ let (key, _, fingerprint) = load_and_decrypt_key(&path, &crate::key::get_encryption_key_password)?;
+ println!("Encryption key fingerprint: '{}'", fingerprint);
Some(Arc::new(CryptConfig::new(key)?))
}
};
--
2.20.1
^ permalink raw reply [flat|nested] 26+ messages in thread* [pbs-devel] [PATCH proxmox-backup 04/13] client: add 'key show' command
2020-11-20 16:38 [pbs-devel] [PATCH v2 proxmox-backup(-qemu) 00/15] add, persist and check fingerprint Fabian Grünbichler
` (2 preceding siblings ...)
2020-11-20 16:38 ` [pbs-devel] [PATCH proxmox-backup 03/13] client: print key fingerprint and master key Fabian Grünbichler
@ 2020-11-20 16:38 ` Fabian Grünbichler
2020-11-20 16:38 ` [pbs-devel] [PATCH proxmox-backup 05/13] fix #3139: add key fingerprint to manifest Fabian Grünbichler
` (11 subsequent siblings)
15 siblings, 0 replies; 26+ messages in thread
From: Fabian Grünbichler @ 2020-11-20 16:38 UTC (permalink / raw)
To: pbs-devel
for (pretty-)printing a keyfile.
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
Notes:
v2: display fp directly as well, add output-format support
src/bin/proxmox_backup_client/key.rs | 68 +++++++++++++++++++++++++++-
1 file changed, 67 insertions(+), 1 deletion(-)
diff --git a/src/bin/proxmox_backup_client/key.rs b/src/bin/proxmox_backup_client/key.rs
index 915ee970..ea7e8c82 100644
--- a/src/bin/proxmox_backup_client/key.rs
+++ b/src/bin/proxmox_backup_client/key.rs
@@ -4,9 +4,16 @@ use std::process::{Stdio, Command};
use anyhow::{bail, format_err, Error};
use serde::{Deserialize, Serialize};
+use serde_json::Value;
use proxmox::api::api;
-use proxmox::api::cli::{CliCommand, CliCommandMap};
+use proxmox::api::cli::{
+ CliCommand,
+ CliCommandMap,
+ format_and_print_result,
+ get_output_format,
+ OUTPUT_FORMAT,
+};
use proxmox::sys::linux::tty;
use proxmox::tools::fs::{file_get_contents, replace_file, CreateOptions};
@@ -16,6 +23,7 @@ use proxmox_backup::backup::{
store_key_config,
CryptConfig,
KeyConfig,
+ KeyDerivationConfig,
};
use proxmox_backup::tools;
@@ -229,6 +237,59 @@ fn change_passphrase(kdf: Option<Kdf>, path: Option<String>) -> Result<(), Error
Ok(())
}
+#[api(
+ input: {
+ properties: {
+ path: {
+ description: "Key file. Without this the default key's metadata will be shown.",
+ optional: true,
+ },
+ "output-format": {
+ schema: OUTPUT_FORMAT,
+ optional: true,
+ },
+ },
+ },
+)]
+/// Print the encryption key's metadata.
+fn show_key(
+ path: Option<String>,
+ param: Value,
+) -> Result<(), Error> {
+ let path = match path {
+ Some(path) => PathBuf::from(path),
+ None => {
+ let path = find_default_encryption_key()?
+ .ok_or_else(|| {
+ format_err!("no encryption file provided and no default file found")
+ })?;
+ path
+ }
+ };
+
+ let output_format = get_output_format(¶m);
+ let config: KeyConfig = serde_json::from_slice(&file_get_contents(path.clone())?)?;
+
+ if output_format == "text" {
+ println!("Path: {:?}", path);
+ match config.kdf {
+ Some(KeyDerivationConfig::PBKDF2 { .. }) => println!("KDF: pbkdf2"),
+ Some(KeyDerivationConfig::Scrypt { .. }) => println!("KDF: scrypt"),
+ None => println!("KDF: none (plaintext key)"),
+ };
+ println!("Created: {}", proxmox::tools::time::epoch_to_rfc3339_utc(config.created)?);
+ println!("Modified: {}", proxmox::tools::time::epoch_to_rfc3339_utc(config.modified)?);
+ match config.fingerprint {
+ Some(fp) => println!("Fingerprint: {}", fp),
+ None => println!("Fingerprint: none (legacy key)"),
+ };
+ } else {
+ format_and_print_result(&serde_json::to_value(config)?, &output_format);
+ }
+
+ Ok(())
+}
+
#[api(
input: {
properties: {
@@ -348,6 +409,10 @@ pub fn cli() -> CliCommandMap {
.arg_param(&["path"])
.completion_cb("path", tools::complete_file_name);
+ let key_show_cmd_def = CliCommand::new(&API_METHOD_SHOW_KEY)
+ .arg_param(&["path"])
+ .completion_cb("path", tools::complete_file_name);
+
let paper_key_cmd_def = CliCommand::new(&API_METHOD_PAPER_KEY)
.arg_param(&["path"])
.completion_cb("path", tools::complete_file_name);
@@ -357,6 +422,7 @@ pub fn cli() -> CliCommandMap {
.insert("create-master-key", key_create_master_key_cmd_def)
.insert("import-master-pubkey", key_import_master_pubkey_cmd_def)
.insert("change-passphrase", key_change_passphrase_cmd_def)
+ .insert("show", key_show_cmd_def)
.insert("paperkey", paper_key_cmd_def)
}
--
2.20.1
^ permalink raw reply [flat|nested] 26+ messages in thread* [pbs-devel] [PATCH proxmox-backup 05/13] fix #3139: add key fingerprint to manifest
2020-11-20 16:38 [pbs-devel] [PATCH v2 proxmox-backup(-qemu) 00/15] add, persist and check fingerprint Fabian Grünbichler
` (3 preceding siblings ...)
2020-11-20 16:38 ` [pbs-devel] [PATCH proxmox-backup 04/13] client: add 'key show' command Fabian Grünbichler
@ 2020-11-20 16:38 ` Fabian Grünbichler
2020-11-20 16:38 ` [pbs-devel] [PATCH proxmox-backup 06/13] manifest: check fingerprint when loading with key Fabian Grünbichler
` (10 subsequent siblings)
15 siblings, 0 replies; 26+ messages in thread
From: Fabian Grünbichler @ 2020-11-20 16:38 UTC (permalink / raw)
To: pbs-devel
if the manifest is signed/the contained archives/blobs are encrypted.
stored in 'unprotected' area, since there is already a strong binding
between key and manifest via the signature, and this avoids breaking
backwards compatibility for a simple usability improvement.
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
Notes:
v2: use Fingerprint struct here as well
src/backup/manifest.rs | 38 +++++++++++++++++++++++++++++++++++++-
1 file changed, 37 insertions(+), 1 deletion(-)
diff --git a/src/backup/manifest.rs b/src/backup/manifest.rs
index 51980a07..00dafbd6 100644
--- a/src/backup/manifest.rs
+++ b/src/backup/manifest.rs
@@ -5,7 +5,7 @@ use std::path::Path;
use serde_json::{json, Value};
use ::serde::{Deserialize, Serialize};
-use crate::backup::{BackupDir, CryptMode, CryptConfig};
+use crate::backup::{BackupDir, CryptMode, CryptConfig, Fingerprint};
pub const MANIFEST_BLOB_NAME: &str = "index.json.blob";
pub const MANIFEST_LOCK_NAME: &str = ".index.json.lck";
@@ -223,12 +223,48 @@ impl BackupManifest {
if let Some(crypt_config) = crypt_config {
let sig = self.signature(crypt_config)?;
manifest["signature"] = proxmox::tools::digest_to_hex(&sig).into();
+ let fingerprint = &crypt_config.fingerprint();
+ manifest["unprotected"]["key-fingerprint"] = serde_json::to_value(fingerprint)?;
}
let manifest = serde_json::to_string_pretty(&manifest).unwrap().into();
Ok(manifest)
}
+ pub fn fingerprint(&self) -> Result<Option<Fingerprint>, Error> {
+ match &self.unprotected["key-fingerprint"] {
+ Value::Null => Ok(None),
+ value => Ok(Some(serde_json::from_value(value.clone())?))
+ }
+ }
+
+ /// Checks if a BackupManifest and a CryptConfig share a valid fingerprint combination.
+ ///
+ /// An unsigned manifest is valid with any or no CryptConfig.
+ /// A signed manifest is only valid with a matching CryptConfig.
+ pub fn check_fingerprint(&self, crypt_config: Option<&CryptConfig>) -> Result<(), Error> {
+ if let Some(fingerprint) = self.fingerprint()? {
+ match crypt_config {
+ None => bail!(
+ "missing key - manifest was created with key {}",
+ fingerprint,
+ ),
+ Some(crypt_config) => {
+ let config_fp = crypt_config.fingerprint();
+ if config_fp != fingerprint {
+ bail!(
+ "wrong key - manifest's key {} does not match provided key {}",
+ fingerprint,
+ config_fp
+ );
+ }
+ }
+ }
+ };
+
+ Ok(())
+ }
+
/// Try to read the manifest. This verifies the signature if there is a crypt_config.
pub fn from_data(data: &[u8], crypt_config: Option<&CryptConfig>) -> Result<BackupManifest, Error> {
let json: Value = serde_json::from_slice(data)?;
--
2.20.1
^ permalink raw reply [flat|nested] 26+ messages in thread* [pbs-devel] [PATCH proxmox-backup 06/13] manifest: check fingerprint when loading with key
2020-11-20 16:38 [pbs-devel] [PATCH v2 proxmox-backup(-qemu) 00/15] add, persist and check fingerprint Fabian Grünbichler
` (4 preceding siblings ...)
2020-11-20 16:38 ` [pbs-devel] [PATCH proxmox-backup 05/13] fix #3139: add key fingerprint to manifest Fabian Grünbichler
@ 2020-11-20 16:38 ` Fabian Grünbichler
2020-11-20 16:38 ` [pbs-devel] [PATCH proxmox-backup 07/13] client: check fingerprint after downloading manifest Fabian Grünbichler
` (9 subsequent siblings)
15 siblings, 0 replies; 26+ messages in thread
From: Fabian Grünbichler @ 2020-11-20 16:38 UTC (permalink / raw)
To: pbs-devel
otherwise loading will run into the signature mismatch which is
technically true, but not the complete picture in this case.
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
src/backup/manifest.rs | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/src/backup/manifest.rs b/src/backup/manifest.rs
index 00dafbd6..a64cbe15 100644
--- a/src/backup/manifest.rs
+++ b/src/backup/manifest.rs
@@ -273,6 +273,19 @@ impl BackupManifest {
if let Some(ref crypt_config) = crypt_config {
if let Some(signature) = signature {
let expected_signature = proxmox::tools::digest_to_hex(&Self::json_signature(&json, crypt_config)?);
+
+ let fingerprint = &json["unprotected"]["key-fingerprint"];
+ if fingerprint != &Value::Null {
+ let fingerprint = serde_json::from_value(fingerprint.clone())?;
+ let config_fp = crypt_config.fingerprint();
+ if config_fp != fingerprint {
+ bail!(
+ "wrong key - unable to verify signature since manifest's key {} does not match provided key {}",
+ fingerprint,
+ config_fp
+ );
+ }
+ }
if signature != expected_signature {
bail!("wrong signature in manifest");
}
--
2.20.1
^ permalink raw reply [flat|nested] 26+ messages in thread* [pbs-devel] [PATCH proxmox-backup 07/13] client: check fingerprint after downloading manifest
2020-11-20 16:38 [pbs-devel] [PATCH v2 proxmox-backup(-qemu) 00/15] add, persist and check fingerprint Fabian Grünbichler
` (5 preceding siblings ...)
2020-11-20 16:38 ` [pbs-devel] [PATCH proxmox-backup 06/13] manifest: check fingerprint when loading with key Fabian Grünbichler
@ 2020-11-20 16:38 ` Fabian Grünbichler
2020-11-20 16:38 ` [pbs-devel] [PATCH proxmox-backup 08/13] paperkey: refactor common code Fabian Grünbichler
` (8 subsequent siblings)
15 siblings, 0 replies; 26+ messages in thread
From: Fabian Grünbichler @ 2020-11-20 16:38 UTC (permalink / raw)
To: pbs-devel
this is stricter than the check that happened on manifest load, as it
also fails if the manifest is signed but we don't have a key available.
add some additional output at the start of a backup to indicate whether
a previous manifest is available to base the backup on.
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
src/bin/proxmox-backup-client.rs | 22 ++++++++++++++++++----
src/bin/proxmox_backup_client/catalog.rs | 2 ++
src/bin/proxmox_backup_client/mount.rs | 1 +
3 files changed, 21 insertions(+), 4 deletions(-)
diff --git a/src/bin/proxmox-backup-client.rs b/src/bin/proxmox-backup-client.rs
index ee2623f0..b4f87071 100644
--- a/src/bin/proxmox-backup-client.rs
+++ b/src/bin/proxmox-backup-client.rs
@@ -1100,10 +1100,23 @@ async fn create_backup(
false
).await?;
- let previous_manifest = if let Ok(previous_manifest) = client.download_previous_manifest().await {
- Some(Arc::new(previous_manifest))
- } else {
- None
+ let previous_manifest = match client.download_previous_manifest().await {
+ Ok(previous_manifest) => {
+ match previous_manifest.check_fingerprint(crypt_config.as_ref().map(Arc::as_ref)) {
+ Ok(()) => {
+ println!("Successfully downloaded previous manifest.");
+ Some(Arc::new(previous_manifest))
+ },
+ Err(err) => {
+ println!("Couldn't re-use pevious manifest - {}", err);
+ None
+ },
+ }
+ },
+ Err(err) => {
+ println!("Couldn't download pevious manifest - {}", err);
+ None
+ },
};
let snapshot = BackupDir::new(backup_type, backup_id, backup_time)?;
@@ -1401,6 +1414,7 @@ async fn restore(param: Value) -> Result<Value, Error> {
).await?;
let (manifest, backup_index_data) = client.download_manifest().await?;
+ manifest.check_fingerprint(crypt_config.as_ref().map(Arc::as_ref))?;
let (archive_name, archive_type) = parse_archive_type(archive_name);
diff --git a/src/bin/proxmox_backup_client/catalog.rs b/src/bin/proxmox_backup_client/catalog.rs
index 37ad842f..61bcc57f 100644
--- a/src/bin/proxmox_backup_client/catalog.rs
+++ b/src/bin/proxmox_backup_client/catalog.rs
@@ -92,6 +92,7 @@ async fn dump_catalog(param: Value) -> Result<Value, Error> {
).await?;
let (manifest, _) = client.download_manifest().await?;
+ manifest.check_fingerprint(crypt_config.as_ref().map(Arc::as_ref))?;
let index = client.download_dynamic_index(&manifest, CATALOG_NAME).await?;
@@ -199,6 +200,7 @@ async fn catalog_shell(param: Value) -> Result<(), Error> {
.open("/tmp")?;
let (manifest, _) = client.download_manifest().await?;
+ manifest.check_fingerprint(crypt_config.as_ref().map(Arc::as_ref))?;
let index = client.download_dynamic_index(&manifest, &server_archive_name).await?;
let most_used = index.find_most_used_chunks(8);
diff --git a/src/bin/proxmox_backup_client/mount.rs b/src/bin/proxmox_backup_client/mount.rs
index 187deab5..c586b764 100644
--- a/src/bin/proxmox_backup_client/mount.rs
+++ b/src/bin/proxmox_backup_client/mount.rs
@@ -214,6 +214,7 @@ async fn mount_do(param: Value, pipe: Option<RawFd>) -> Result<Value, Error> {
).await?;
let (manifest, _) = client.download_manifest().await?;
+ manifest.check_fingerprint(crypt_config.as_ref().map(Arc::as_ref))?;
let file_info = manifest.lookup_file_info(&server_archive_name)?;
--
2.20.1
^ permalink raw reply [flat|nested] 26+ messages in thread* [pbs-devel] [PATCH proxmox-backup 08/13] paperkey: refactor common code
2020-11-20 16:38 [pbs-devel] [PATCH v2 proxmox-backup(-qemu) 00/15] add, persist and check fingerprint Fabian Grünbichler
` (6 preceding siblings ...)
2020-11-20 16:38 ` [pbs-devel] [PATCH proxmox-backup 07/13] client: check fingerprint after downloading manifest Fabian Grünbichler
@ 2020-11-20 16:38 ` Fabian Grünbichler
2020-11-20 16:38 ` [pbs-devel] [PATCH proxmox-backup 09/13] paperkey: add short key ID to subject Fabian Grünbichler
` (7 subsequent siblings)
15 siblings, 0 replies; 26+ messages in thread
From: Fabian Grünbichler @ 2020-11-20 16:38 UTC (permalink / raw)
To: pbs-devel
from formatting functions to main function, and pass along the key data
lines instead of the full string.
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
src/bin/proxmox_backup_client/key.rs | 104 +++++++++++++--------------
1 file changed, 52 insertions(+), 52 deletions(-)
diff --git a/src/bin/proxmox_backup_client/key.rs b/src/bin/proxmox_backup_client/key.rs
index ea7e8c82..e56308b7 100644
--- a/src/bin/proxmox_backup_client/key.rs
+++ b/src/bin/proxmox_backup_client/key.rs
@@ -385,13 +385,47 @@ fn paper_key(
};
let data = file_get_contents(&path)?;
- let data = std::str::from_utf8(&data)?;
+ let data = String::from_utf8(data)?;
+
+ let (data, is_private_key) = if data.starts_with("-----BEGIN ENCRYPTED PRIVATE KEY-----\n") {
+ let lines: Vec<String> = data
+ .lines()
+ .map(|s| s.trim_end())
+ .filter(|s| !s.is_empty())
+ .map(String::from)
+ .collect();
+
+ if !lines[lines.len()-1].starts_with("-----END ENCRYPTED PRIVATE KEY-----") {
+ bail!("unexpected key format");
+ }
+
+ if lines.len() < 20 {
+ bail!("unexpected key format");
+ }
+
+ (lines, true)
+ } else {
+ match serde_json::from_str::<KeyConfig>(&data) {
+ Ok(key_config) => {
+ let lines = serde_json::to_string_pretty(&key_config)?
+ .lines()
+ .map(String::from)
+ .collect();
+
+ (lines, false)
+ },
+ Err(err) => {
+ eprintln!("Couldn't parse '{:?}' as KeyConfig - {}", path, err);
+ bail!("Neither a PEM-formatted private key, nor a PBS key file.");
+ },
+ }
+ };
let format = output_format.unwrap_or(PaperkeyFormat::Html);
match format {
- PaperkeyFormat::Html => paperkey_html(data, subject),
- PaperkeyFormat::Text => paperkey_text(data, subject),
+ PaperkeyFormat::Html => paperkey_html(&data, subject, is_private_key),
+ PaperkeyFormat::Text => paperkey_text(&data, subject, is_private_key),
}
}
@@ -426,7 +460,7 @@ pub fn cli() -> CliCommandMap {
.insert("paperkey", paper_key_cmd_def)
}
-fn paperkey_html(data: &str, subject: Option<String>) -> Result<(), Error> {
+fn paperkey_html(lines: &[String], subject: Option<String>, is_private: bool) -> Result<(), Error> {
let img_size_pt = 500;
@@ -455,21 +489,7 @@ fn paperkey_html(data: &str, subject: Option<String>) -> Result<(), Error> {
println!("<p>Subject: {}</p>", subject);
}
- if data.starts_with("-----BEGIN ENCRYPTED PRIVATE KEY-----\n") {
- let lines: Vec<String> = data.lines()
- .map(|s| s.trim_end())
- .filter(|s| !s.is_empty())
- .map(String::from)
- .collect();
-
- if !lines[lines.len()-1].starts_with("-----END ENCRYPTED PRIVATE KEY-----") {
- bail!("unexpected key format");
- }
-
- if lines.len() < 20 {
- bail!("unexpected key format");
- }
-
+ if is_private {
const BLOCK_SIZE: usize = 20;
let blocks = (lines.len() + BLOCK_SIZE -1)/BLOCK_SIZE;
@@ -490,8 +510,7 @@ fn paperkey_html(data: &str, subject: Option<String>) -> Result<(), Error> {
println!("</p>");
- let data = data.join("\n");
- let qr_code = generate_qr_code("svg", data.as_bytes())?;
+ let qr_code = generate_qr_code("svg", data)?;
let qr_code = base64::encode_config(&qr_code, base64::STANDARD_NO_PAD);
println!("<center>");
@@ -507,16 +526,13 @@ fn paperkey_html(data: &str, subject: Option<String>) -> Result<(), Error> {
return Ok(());
}
- let key_config: KeyConfig = serde_json::from_str(&data)?;
- let key_text = serde_json::to_string_pretty(&key_config)?;
-
println!("<div style=\"page-break-inside: avoid\">");
println!("<p>");
println!("-----BEGIN PROXMOX BACKUP KEY-----");
- for line in key_text.lines() {
+ for line in lines {
println!("{}", line);
}
@@ -524,7 +540,7 @@ fn paperkey_html(data: &str, subject: Option<String>) -> Result<(), Error> {
println!("</p>");
- let qr_code = generate_qr_code("svg", key_text.as_bytes())?;
+ let qr_code = generate_qr_code("svg", lines)?;
let qr_code = base64::encode_config(&qr_code, base64::STANDARD_NO_PAD);
println!("<center>");
@@ -541,27 +557,13 @@ fn paperkey_html(data: &str, subject: Option<String>) -> Result<(), Error> {
Ok(())
}
-fn paperkey_text(data: &str, subject: Option<String>) -> Result<(), Error> {
+fn paperkey_text(lines: &[String], subject: Option<String>, is_private: bool) -> Result<(), Error> {
if let Some(subject) = subject {
println!("Subject: {}\n", subject);
}
- if data.starts_with("-----BEGIN ENCRYPTED PRIVATE KEY-----\n") {
- let lines: Vec<String> = data.lines()
- .map(|s| s.trim_end())
- .filter(|s| !s.is_empty())
- .map(String::from)
- .collect();
-
- if !lines[lines.len()-1].starts_with("-----END ENCRYPTED PRIVATE KEY-----") {
- bail!("unexpected key format");
- }
-
- if lines.len() < 20 {
- bail!("unexpected key format");
- }
-
+ if is_private {
const BLOCK_SIZE: usize = 5;
let blocks = (lines.len() + BLOCK_SIZE -1)/BLOCK_SIZE;
@@ -576,8 +578,7 @@ fn paperkey_text(data: &str, subject: Option<String>) -> Result<(), Error> {
for l in start..end {
println!("{:-2}: {}", l, lines[l]);
}
- let data = data.join("\n");
- let qr_code = generate_qr_code("utf8i", data.as_bytes())?;
+ let qr_code = generate_qr_code("utf8i", data)?;
let qr_code = String::from_utf8(qr_code)
.map_err(|_| format_err!("Failed to read qr code (got non-utf8 data)"))?;
println!("{}", qr_code);
@@ -587,14 +588,13 @@ fn paperkey_text(data: &str, subject: Option<String>) -> Result<(), Error> {
return Ok(());
}
- let key_config: KeyConfig = serde_json::from_str(&data)?;
- let key_text = serde_json::to_string_pretty(&key_config)?;
-
println!("-----BEGIN PROXMOX BACKUP KEY-----");
- println!("{}", key_text);
+ for line in lines {
+ println!("{}", line);
+ }
println!("-----END PROXMOX BACKUP KEY-----");
- let qr_code = generate_qr_code("utf8i", key_text.as_bytes())?;
+ let qr_code = generate_qr_code("utf8i", &lines)?;
let qr_code = String::from_utf8(qr_code)
.map_err(|_| format_err!("Failed to read qr code (got non-utf8 data)"))?;
@@ -603,8 +603,7 @@ fn paperkey_text(data: &str, subject: Option<String>) -> Result<(), Error> {
Ok(())
}
-fn generate_qr_code(output_type: &str, data: &[u8]) -> Result<Vec<u8>, Error> {
-
+fn generate_qr_code(output_type: &str, lines: &[String]) -> Result<Vec<u8>, Error> {
let mut child = Command::new("qrencode")
.args(&["-t", output_type, "-m0", "-s1", "-lm", "--output", "-"])
.stdin(Stdio::piped())
@@ -614,7 +613,8 @@ fn generate_qr_code(output_type: &str, data: &[u8]) -> Result<Vec<u8>, Error> {
{
let stdin = child.stdin.as_mut()
.ok_or_else(|| format_err!("Failed to open stdin"))?;
- stdin.write_all(data)
+ let data = lines.join("\n");
+ stdin.write_all(data.as_bytes())
.map_err(|_| format_err!("Failed to write to stdin"))?;
}
--
2.20.1
^ permalink raw reply [flat|nested] 26+ messages in thread* [pbs-devel] [PATCH proxmox-backup 09/13] paperkey: add short key ID to subject
2020-11-20 16:38 [pbs-devel] [PATCH v2 proxmox-backup(-qemu) 00/15] add, persist and check fingerprint Fabian Grünbichler
` (7 preceding siblings ...)
2020-11-20 16:38 ` [pbs-devel] [PATCH proxmox-backup 08/13] paperkey: refactor common code Fabian Grünbichler
@ 2020-11-20 16:38 ` Fabian Grünbichler
2020-11-23 7:07 ` Dietmar Maurer
2020-11-20 16:38 ` [pbs-devel] [RFC proxmox-backup 10/13] expose previous backup time in backup env Fabian Grünbichler
` (6 subsequent siblings)
15 siblings, 1 reply; 26+ messages in thread
From: Fabian Grünbichler @ 2020-11-20 16:38 UTC (permalink / raw)
To: pbs-devel
and strip fingerprint from keyfile data, since it's rather long and only
informational anyway.
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
src/bin/proxmox_backup_client/key.rs | 18 ++++++++++++++----
1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/src/bin/proxmox_backup_client/key.rs b/src/bin/proxmox_backup_client/key.rs
index e56308b7..baae8726 100644
--- a/src/bin/proxmox_backup_client/key.rs
+++ b/src/bin/proxmox_backup_client/key.rs
@@ -387,7 +387,7 @@ fn paper_key(
let data = file_get_contents(&path)?;
let data = String::from_utf8(data)?;
- let (data, is_private_key) = if data.starts_with("-----BEGIN ENCRYPTED PRIVATE KEY-----\n") {
+ let (data, subject, is_private_key) = if data.starts_with("-----BEGIN ENCRYPTED PRIVATE KEY-----\n") {
let lines: Vec<String> = data
.lines()
.map(|s| s.trim_end())
@@ -403,16 +403,26 @@ fn paper_key(
bail!("unexpected key format");
}
- (lines, true)
+ (lines, subject, true)
} else {
match serde_json::from_str::<KeyConfig>(&data) {
- Ok(key_config) => {
+ Ok(mut key_config) => {
+ // add display version of fingerprint to subject and strip from key data
+ let subject = match (subject, key_config.fingerprint.take()) {
+ (Some(mut subject), Some(fingerprint)) => {
+ subject.push_str(&format!(" ({})", fingerprint));
+ Some(subject)
+ },
+ (None, Some(fingerprint)) => Some(format!("Fingerprint: {}", fingerprint)),
+ (subject, _) => subject,
+ };
+
let lines = serde_json::to_string_pretty(&key_config)?
.lines()
.map(String::from)
.collect();
- (lines, false)
+ (lines, subject, false)
},
Err(err) => {
eprintln!("Couldn't parse '{:?}' as KeyConfig - {}", path, err);
--
2.20.1
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [pbs-devel] [PATCH proxmox-backup 09/13] paperkey: add short key ID to subject
2020-11-20 16:38 ` [pbs-devel] [PATCH proxmox-backup 09/13] paperkey: add short key ID to subject Fabian Grünbichler
@ 2020-11-23 7:07 ` Dietmar Maurer
2020-11-23 8:16 ` Fabian Grünbichler
0 siblings, 1 reply; 26+ messages in thread
From: Dietmar Maurer @ 2020-11-23 7:07 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Fabian Grünbichler
> + Ok(mut key_config) => {
> + // add display version of fingerprint to subject and strip from key data
> + let subject = match (subject, key_config.fingerprint.take()) {
> + (Some(mut subject), Some(fingerprint)) => {
> + subject.push_str(&format!(" ({})", fingerprint));
> + Some(subject)
> + },
> + (None, Some(fingerprint)) => Some(format!("Fingerprint: {}", fingerprint)),
> + (subject, _) => subject,
> + };
> +
I still don't get why we need a 32byte fingerprint - this is the same length as the key itself!
I want to avoid having keys with different content (strip fingerprint), because this only
confuse people.
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [pbs-devel] [PATCH proxmox-backup 09/13] paperkey: add short key ID to subject
2020-11-23 7:07 ` Dietmar Maurer
@ 2020-11-23 8:16 ` Fabian Grünbichler
2020-11-23 8:30 ` Dietmar Maurer
` (2 more replies)
0 siblings, 3 replies; 26+ messages in thread
From: Fabian Grünbichler @ 2020-11-23 8:16 UTC (permalink / raw)
To: Dietmar Maurer, Proxmox Backup Server development discussion
On November 23, 2020 8:07 am, Dietmar Maurer wrote:
>
>> + Ok(mut key_config) => {
>> + // add display version of fingerprint to subject and strip from key data
>> + let subject = match (subject, key_config.fingerprint.take()) {
>> + (Some(mut subject), Some(fingerprint)) => {
>> + subject.push_str(&format!(" ({})", fingerprint));
>> + Some(subject)
>> + },
>> + (None, Some(fingerprint)) => Some(format!("Fingerprint: {}", fingerprint)),
>> + (subject, _) => subject,
>> + };
>> +
>
> I still don't get why we need a 32byte fingerprint - this is the same length as the key itself!
the key (on disk) is 8 + 8 + 8 + 32 (key derivation) + 64 (encrypted key
data) + 8 + 8 (timestamps), totalling 136 bytes. serialized it's a bit
more, although there the fingerprint skews the numbers more heavily
(because I opted for a readable serialization, not one optimized for
size). even in-memory, the key is not 32-byte long, but 32+32+however
long the PKey struct from openssl is.
I want to have the "full" fingerprint because we persist this in places
where we can't (easily) update it, so it's more future-proof to keep the
full value there. it also makes it possible to use the full value for
the actual comparison done on manifest load/check (where we not only
have to think about collisions for a single user, but potentially
hundreds/thousands if they share a datastore!).
> I want to avoid having keys with different content (strip fingerprint), because this only
> confuse people.
I originally wanted to keep the full fingerprint in, but conceded to
your space arguments here and only included the short version.. I also
don't really buy the confusion, since unless the user has manually
looked inside the key file they don't even know that or how the
fingerprint is stored there - in the user-facing parts we only ever show
the short key ID. furthermore we already paperkey the pretty-printed
version so if the user restores that the checksum of the keyfile is
different anyhow.
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [pbs-devel] [PATCH proxmox-backup 09/13] paperkey: add short key ID to subject
2020-11-23 8:16 ` Fabian Grünbichler
@ 2020-11-23 8:30 ` Dietmar Maurer
2020-11-23 8:47 ` Fabian Grünbichler
2020-11-23 8:41 ` Dietmar Maurer
2020-11-23 8:55 ` Dietmar Maurer
2 siblings, 1 reply; 26+ messages in thread
From: Dietmar Maurer @ 2020-11-23 8:30 UTC (permalink / raw)
To: Fabian Grünbichler, Proxmox Backup Server development discussion
> I originally wanted to keep the full fingerprint in, but conceded to
> your space arguments here and only included the short version.. I also
> don't really buy the confusion, since unless the user has manually
> looked inside the key file they don't even know that or how the
> fingerprint is stored there - in the user-facing parts we only ever show
> the short key ID. furthermore we already paperkey the pretty-printed
> version so if the user restores that the checksum of the keyfile is
> different anyhow.
Don't get that. You delete the fingerprint, so if a user restores that key
he don't have a fingerprint to compare?
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [pbs-devel] [PATCH proxmox-backup 09/13] paperkey: add short key ID to subject
2020-11-23 8:30 ` Dietmar Maurer
@ 2020-11-23 8:47 ` Fabian Grünbichler
0 siblings, 0 replies; 26+ messages in thread
From: Fabian Grünbichler @ 2020-11-23 8:47 UTC (permalink / raw)
To: Dietmar Maurer, Proxmox Backup Server development discussion
On November 23, 2020 9:30 am, Dietmar Maurer wrote:
>> I originally wanted to keep the full fingerprint in, but conceded to
>> your space arguments here and only included the short version.. I also
>> don't really buy the confusion, since unless the user has manually
>> looked inside the key file they don't even know that or how the
>> fingerprint is stored there - in the user-facing parts we only ever show
>> the short key ID. furthermore we already paperkey the pretty-printed
>> version so if the user restores that the checksum of the keyfile is
>> different anyhow.
>
> Don't get that. You delete the fingerprint, so if a user restores that key
> he don't have a fingerprint to compare?
the short key ID is put into the subject now to make human matching of
paperkey printouts and manifest/snapshot lists easier.
if the user restores the paperkey into a keyfile, anytime that keyfile
is used it will re-generate the fingerprint on the fly. if the keyfile
is modified (e.g. on passphrase/KDF change), the generated fingerprint
will also be persisted again. we don't have a 'restore paperkey' command
(yet), but if we ever do, that could of course also regenerate the full
fingerprint and persist it on writing out the keyfile.
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [pbs-devel] [PATCH proxmox-backup 09/13] paperkey: add short key ID to subject
2020-11-23 8:16 ` Fabian Grünbichler
2020-11-23 8:30 ` Dietmar Maurer
@ 2020-11-23 8:41 ` Dietmar Maurer
2020-11-23 8:55 ` Dietmar Maurer
2 siblings, 0 replies; 26+ messages in thread
From: Dietmar Maurer @ 2020-11-23 8:41 UTC (permalink / raw)
To: Fabian Grünbichler, Proxmox Backup Server development discussion
> I want to have the "full" fingerprint because we persist this in places
> where we can't (easily) update it, so it's more future-proof to keep the
> full value there. it also makes it possible to use the full value for
> the actual comparison done on manifest load/check (where we not only
> have to think about collisions for a single user, but potentially
> hundreds/thousands if they share a datastore!).
We have backup "owners", so users can only access backups they own.
But even with thousands of keys, collisions are very unlikely...
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [pbs-devel] [PATCH proxmox-backup 09/13] paperkey: add short key ID to subject
2020-11-23 8:16 ` Fabian Grünbichler
2020-11-23 8:30 ` Dietmar Maurer
2020-11-23 8:41 ` Dietmar Maurer
@ 2020-11-23 8:55 ` Dietmar Maurer
2020-11-23 9:44 ` Fabian Grünbichler
2 siblings, 1 reply; 26+ messages in thread
From: Dietmar Maurer @ 2020-11-23 8:55 UTC (permalink / raw)
To: Fabian Grünbichler, Proxmox Backup Server development discussion
> > I still don't get why we need a 32byte fingerprint - this is the same length as the key itself!
>
> the key (on disk) is 8 + 8 + 8 + 32 (key derivation) + 64 (encrypted key
> data) + 8 + 8 (timestamps), totalling 136 bytes. serialized it's a bit
> more, although there the fingerprint skews the numbers more heavily
> (because I opted for a readable serialization, not one optimized for
> size). even in-memory, the key is not 32-byte long, but 32+32+however
> long the PKey struct from openssl is.
For the record, the fingerprint is computed using 32 secret bytes (not more).
Note: FINGERPRINT_INPUT is const
+ pub fn fingerprint(&self) -> Fingerprint {
+ Fingerprint {
+ bytes: self.compute_digest(&FINGERPRINT_INPUT)
+ }
+ }
+
pub fn compute_digest(&self, data: &[u8]) -> [u8; 32] {
let mut hasher = openssl::sha::Sha256::new();
hasher.update(data); // this is const
hasher.update(&self.id_key); // this is 32 bytes
hasher.finish()
}
So the fingerprint has exactly the same size as the secret key.
I really do not understand your arguments...
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [pbs-devel] [PATCH proxmox-backup 09/13] paperkey: add short key ID to subject
2020-11-23 8:55 ` Dietmar Maurer
@ 2020-11-23 9:44 ` Fabian Grünbichler
0 siblings, 0 replies; 26+ messages in thread
From: Fabian Grünbichler @ 2020-11-23 9:44 UTC (permalink / raw)
To: Dietmar Maurer, Proxmox Backup Server development discussion
On November 23, 2020 9:55 am, Dietmar Maurer wrote:
>> > I still don't get why we need a 32byte fingerprint - this is the same length as the key itself!
>>
>> the key (on disk) is 8 + 8 + 8 + 32 (key derivation) + 64 (encrypted key
>> data) + 8 + 8 (timestamps), totalling 136 bytes. serialized it's a bit
>> more, although there the fingerprint skews the numbers more heavily
>> (because I opted for a readable serialization, not one optimized for
>> size). even in-memory, the key is not 32-byte long, but 32+32+however
>> long the PKey struct from openssl is.
>
> For the record, the fingerprint is computed using 32 secret bytes (not more).
>
> Note: FINGERPRINT_INPUT is const
>
> + pub fn fingerprint(&self) -> Fingerprint {
> + Fingerprint {
> + bytes: self.compute_digest(&FINGERPRINT_INPUT)
> + }
> + }
> +
>
> pub fn compute_digest(&self, data: &[u8]) -> [u8; 32] {
> let mut hasher = openssl::sha::Sha256::new();
> hasher.update(data); // this is const
> hasher.update(&self.id_key); // this is 32 bytes
> hasher.finish()
> }
>
> So the fingerprint has exactly the same size as the secret key.
> I really do not understand your arguments...
I specifically wrote what my numbers refer to (structures kept on-disk
and in-memory, both for the FULL key). yes, the PART of the key that is
used to calculated digests (and the fingerprint) is only 32 bytes. my
argument is that for both on-disk and in-memory, adding a full 32 bytes
fingerprint does not 'double' the size, as you imply, since the
fingerprint is for the full key and not just the derived ID key part.
if everybody besides me thinks that storing the truncated 8 byte
"fingerprint" is a good idea, then I can send a v3 that does that.
IMHO we don't have much to gain by storing it truncated:
- key files are a bit smaller (they are client-side only anyway)
- manifests are a bit smaller (they are in a different order of
magnitude anyway)
while storing the full one has advantages:
- fingerprint has same level of security as chunk digest (if that breaks
everything is broken anyway!), allowing us to base non-informational
features on it as well (such as Qemu key tracking for bitmap
invalidation)
- we can change how to display it in the future and still have the full
value to derive the display value from
^ permalink raw reply [flat|nested] 26+ messages in thread
* [pbs-devel] [RFC proxmox-backup 10/13] expose previous backup time in backup env
2020-11-20 16:38 [pbs-devel] [PATCH v2 proxmox-backup(-qemu) 00/15] add, persist and check fingerprint Fabian Grünbichler
` (8 preceding siblings ...)
2020-11-20 16:38 ` [pbs-devel] [PATCH proxmox-backup 09/13] paperkey: add short key ID to subject Fabian Grünbichler
@ 2020-11-20 16:38 ` Fabian Grünbichler
2020-11-20 16:38 ` [pbs-devel] [PATCH proxmox-backup 11/13] refactor BackupInfo -> SnapshotListItem helper Fabian Grünbichler
` (5 subsequent siblings)
15 siblings, 0 replies; 26+ messages in thread
From: Fabian Grünbichler @ 2020-11-20 16:38 UTC (permalink / raw)
To: pbs-devel
and use this information to add more information to client backup log
and guide the download manifest decision.
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
Notes:
new in v2
backwards-compatible API change!
src/api2/backup.rs | 26 ++++++++++++++++
src/bin/proxmox-backup-client.rs | 51 ++++++++++++++++++++++----------
src/client/backup_writer.rs | 7 +++++
3 files changed, 68 insertions(+), 16 deletions(-)
diff --git a/src/api2/backup.rs b/src/api2/backup.rs
index ce9a34ae..f4eed074 100644
--- a/src/api2/backup.rs
+++ b/src/api2/backup.rs
@@ -311,6 +311,10 @@ pub const BACKUP_API_SUBDIRS: SubdirMap = &[
"previous", &Router::new()
.download(&API_METHOD_DOWNLOAD_PREVIOUS)
),
+ (
+ "previous_backup_time", &Router::new()
+ .get(&API_METHOD_GET_PREVIOUS_BACKUP_TIME)
+ ),
(
"speedtest", &Router::new()
.upload(&API_METHOD_UPLOAD_SPEEDTEST)
@@ -694,6 +698,28 @@ fn finish_backup (
Ok(Value::Null)
}
+#[sortable]
+pub const API_METHOD_GET_PREVIOUS_BACKUP_TIME: ApiMethod = ApiMethod::new(
+ &ApiHandler::Sync(&get_previous_backup_time),
+ &ObjectSchema::new(
+ "Get previous backup time.",
+ &[],
+ )
+);
+
+fn get_previous_backup_time(
+ _param: Value,
+ _info: &ApiMethod,
+ rpcenv: &mut dyn RpcEnvironment,
+) -> Result<Value, Error> {
+
+ let env: &BackupEnvironment = rpcenv.as_ref();
+
+ let backup_time = env.last_backup.as_ref().map(|info| info.backup_dir.backup_time());
+
+ Ok(json!(backup_time))
+}
+
#[sortable]
pub const API_METHOD_DOWNLOAD_PREVIOUS: ApiMethod = ApiMethod::new(
&ApiHandler::AsyncHttp(&download_previous),
diff --git a/src/bin/proxmox-backup-client.rs b/src/bin/proxmox-backup-client.rs
index b4f87071..3e9931b8 100644
--- a/src/bin/proxmox-backup-client.rs
+++ b/src/bin/proxmox-backup-client.rs
@@ -1100,23 +1100,42 @@ async fn create_backup(
false
).await?;
- let previous_manifest = match client.download_previous_manifest().await {
- Ok(previous_manifest) => {
- match previous_manifest.check_fingerprint(crypt_config.as_ref().map(Arc::as_ref)) {
- Ok(()) => {
- println!("Successfully downloaded previous manifest.");
- Some(Arc::new(previous_manifest))
- },
- Err(err) => {
- println!("Couldn't re-use pevious manifest - {}", err);
- None
- },
+ let download_previous_manifest = match client.previous_backup_time().await {
+ Ok(Some(backup_time)) => {
+ println!(
+ "Downloading previous manifest ({})",
+ strftime_local("%c", backup_time)?
+ );
+ true
+ }
+ Ok(None) => {
+ println!("No previous manifest available.");
+ false
+ }
+ Err(_) => {
+ // Fallback for outdated server, TODO remove/bubble up with 2.0
+ true
+ }
+ };
+
+ let previous_manifest = if download_previous_manifest {
+ match client.download_previous_manifest().await {
+ Ok(previous_manifest) => {
+ match previous_manifest.check_fingerprint(crypt_config.as_ref().map(Arc::as_ref)) {
+ Ok(()) => Some(Arc::new(previous_manifest)),
+ Err(err) => {
+ println!("Couldn't re-use previous manifest - {}", err);
+ None
+ }
+ }
}
- },
- Err(err) => {
- println!("Couldn't download pevious manifest - {}", err);
- None
- },
+ Err(err) => {
+ println!("Couldn't download previous manifest - {}", err);
+ None
+ }
+ }
+ } else {
+ None
};
let snapshot = BackupDir::new(backup_type, backup_id, backup_time)?;
diff --git a/src/client/backup_writer.rs b/src/client/backup_writer.rs
index c5ce5b42..39cd574d 100644
--- a/src/client/backup_writer.rs
+++ b/src/client/backup_writer.rs
@@ -475,6 +475,13 @@ impl BackupWriter {
Ok(index)
}
+ /// Retrieve backup time of last backup
+ pub async fn previous_backup_time(&self) -> Result<Option<i64>, Error> {
+ let data = self.h2.get("previous_backup_time", None).await?;
+ serde_json::from_value(data)
+ .map_err(|err| format_err!("Failed to parse backup time value returned by server - {}", err))
+ }
+
/// Download backup manifest (index.json) of last backup
pub async fn download_previous_manifest(&self) -> Result<BackupManifest, Error> {
--
2.20.1
^ permalink raw reply [flat|nested] 26+ messages in thread* [pbs-devel] [PATCH proxmox-backup 11/13] refactor BackupInfo -> SnapshotListItem helper
2020-11-20 16:38 [pbs-devel] [PATCH v2 proxmox-backup(-qemu) 00/15] add, persist and check fingerprint Fabian Grünbichler
` (9 preceding siblings ...)
2020-11-20 16:38 ` [pbs-devel] [RFC proxmox-backup 10/13] expose previous backup time in backup env Fabian Grünbichler
@ 2020-11-20 16:38 ` Fabian Grünbichler
2020-11-20 16:38 ` [pbs-devel] [PATCH proxmox-backup 12/13] list_snapshots: return manifest fingerprint Fabian Grünbichler
` (4 subsequent siblings)
15 siblings, 0 replies; 26+ messages in thread
From: Fabian Grünbichler @ 2020-11-20 16:38 UTC (permalink / raw)
To: pbs-devel
before adding more fields to the tuple, let's just create the struct
inside the match arms to improve readability.
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
Notes:
new in v2
src/api2/admin/datastore.rs | 50 +++++++++++++++++++++----------------
1 file changed, 28 insertions(+), 22 deletions(-)
diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
index 872d081e..de0c8de3 100644
--- a/src/api2/admin/datastore.rs
+++ b/src/api2/admin/datastore.rs
@@ -391,9 +391,11 @@ pub fn list_snapshots (
};
let info_to_snapshot_list_item = |group: &BackupGroup, owner, info: BackupInfo| {
+ let backup_type = group.backup_type().to_string();
+ let backup_id = group.backup_id().to_string();
let backup_time = info.backup_dir.backup_time();
- let (comment, verification, files, size) = match get_all_snapshot_files(&datastore, &info) {
+ match get_all_snapshot_files(&datastore, &info) {
Ok((manifest, files)) => {
// extract the first line from notes
let comment: Option<String> = manifest.unprotected["notes"]
@@ -401,8 +403,8 @@ pub fn list_snapshots (
.and_then(|notes| notes.lines().next())
.map(String::from);
- let verify = manifest.unprotected["verify_state"].clone();
- let verify: Option<SnapshotVerifyState> = match serde_json::from_value(verify) {
+ let verification = manifest.unprotected["verify_state"].clone();
+ let verification: Option<SnapshotVerifyState> = match serde_json::from_value(verification) {
Ok(verify) => verify,
Err(err) => {
eprintln!("error parsing verification state : '{}'", err);
@@ -412,14 +414,20 @@ pub fn list_snapshots (
let size = Some(files.iter().map(|x| x.size.unwrap_or(0)).sum());
- (comment, verify, files, size)
+ SnapshotListItem {
+ backup_type,
+ backup_id,
+ backup_time,
+ comment,
+ verification,
+ files,
+ size,
+ owner,
+ }
},
Err(err) => {
eprintln!("error during snapshot file listing: '{}'", err);
- (
- None,
- None,
- info
+ let files = info
.files
.into_iter()
.map(|x| BackupContent {
@@ -427,21 +435,19 @@ pub fn list_snapshots (
size: None,
crypt_mode: None,
})
- .collect(),
- None,
- )
+ .collect();
+
+ SnapshotListItem {
+ backup_type,
+ backup_id,
+ backup_time,
+ comment: None,
+ verification: None,
+ files,
+ size: None,
+ owner,
+ }
},
- };
-
- SnapshotListItem {
- backup_type: group.backup_type().to_string(),
- backup_id: group.backup_id().to_string(),
- backup_time,
- comment,
- verification,
- files,
- size,
- owner,
}
};
--
2.20.1
^ permalink raw reply [flat|nested] 26+ messages in thread* [pbs-devel] [PATCH proxmox-backup 12/13] list_snapshots: return manifest fingerprint
2020-11-20 16:38 [pbs-devel] [PATCH v2 proxmox-backup(-qemu) 00/15] add, persist and check fingerprint Fabian Grünbichler
` (10 preceding siblings ...)
2020-11-20 16:38 ` [pbs-devel] [PATCH proxmox-backup 11/13] refactor BackupInfo -> SnapshotListItem helper Fabian Grünbichler
@ 2020-11-20 16:38 ` Fabian Grünbichler
2020-11-20 16:38 ` [pbs-devel] [PATCH proxmox-backup 13/13] gui: add snapshot/file fingerprint tooltip Fabian Grünbichler
` (3 subsequent siblings)
15 siblings, 0 replies; 26+ messages in thread
From: Fabian Grünbichler @ 2020-11-20 16:38 UTC (permalink / raw)
To: pbs-devel
for display in clients.
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
Notes:
new in v2
backwards-compatible API change (adds new field)
src/api2/admin/datastore.rs | 10 ++++++++++
src/api2/types/mod.rs | 9 ++++++++-
2 files changed, 18 insertions(+), 1 deletion(-)
diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
index de0c8de3..00f98c66 100644
--- a/src/api2/admin/datastore.rs
+++ b/src/api2/admin/datastore.rs
@@ -403,6 +403,14 @@ pub fn list_snapshots (
.and_then(|notes| notes.lines().next())
.map(String::from);
+ let fingerprint = match manifest.fingerprint() {
+ Ok(fp) => fp,
+ Err(err) => {
+ eprintln!("error parsing fingerprint: '{}'", err);
+ None
+ },
+ };
+
let verification = manifest.unprotected["verify_state"].clone();
let verification: Option<SnapshotVerifyState> = match serde_json::from_value(verification) {
Ok(verify) => verify,
@@ -420,6 +428,7 @@ pub fn list_snapshots (
backup_time,
comment,
verification,
+ fingerprint,
files,
size,
owner,
@@ -443,6 +452,7 @@ pub fn list_snapshots (
backup_time,
comment: None,
verification: None,
+ fingerprint: None,
files,
size: None,
owner,
diff --git a/src/api2/types/mod.rs b/src/api2/types/mod.rs
index d7d5a8af..b347fc29 100644
--- a/src/api2/types/mod.rs
+++ b/src/api2/types/mod.rs
@@ -5,7 +5,7 @@ use proxmox::api::{api, schema::*};
use proxmox::const_regex;
use proxmox::{IPRE, IPRE_BRACKET, IPV4RE, IPV6RE, IPV4OCTET, IPV6H16, IPV6LS32};
-use crate::backup::{CryptMode, BACKUP_ID_REGEX};
+use crate::backup::{CryptMode, Fingerprint, BACKUP_ID_REGEX};
use crate::server::UPID;
#[macro_use]
@@ -484,6 +484,10 @@ pub struct SnapshotVerifyState {
type: SnapshotVerifyState,
optional: true,
},
+ fingerprint: {
+ type: String,
+ optional: true,
+ },
files: {
items: {
schema: BACKUP_ARCHIVE_NAME_SCHEMA
@@ -508,6 +512,9 @@ pub struct SnapshotListItem {
/// The result of the last run verify task
#[serde(skip_serializing_if="Option::is_none")]
pub verification: Option<SnapshotVerifyState>,
+ /// Fingerprint of encryption key
+ #[serde(skip_serializing_if="Option::is_none")]
+ pub fingerprint: Option<Fingerprint>,
/// List of contained archive files.
pub files: Vec<BackupContent>,
/// Overall snapshot size (sum of all archive sizes).
--
2.20.1
^ permalink raw reply [flat|nested] 26+ messages in thread* [pbs-devel] [PATCH proxmox-backup 13/13] gui: add snapshot/file fingerprint tooltip
2020-11-20 16:38 [pbs-devel] [PATCH v2 proxmox-backup(-qemu) 00/15] add, persist and check fingerprint Fabian Grünbichler
` (11 preceding siblings ...)
2020-11-20 16:38 ` [pbs-devel] [PATCH proxmox-backup 12/13] list_snapshots: return manifest fingerprint Fabian Grünbichler
@ 2020-11-20 16:38 ` Fabian Grünbichler
2020-11-20 16:38 ` [pbs-devel] [PATCH proxmox-backup-qemu 1/2] adapt to proxmox-backup fingerprint changes Fabian Grünbichler
` (2 subsequent siblings)
15 siblings, 0 replies; 26+ messages in thread
From: Fabian Grünbichler @ 2020-11-20 16:38 UTC (permalink / raw)
To: pbs-devel
display short key ID, like backend's Display trait.
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
Notes:
new in v2
www/Utils.js | 5 +++++
www/datastore/Content.js | 13 ++++++++++++-
2 files changed, 17 insertions(+), 1 deletion(-)
diff --git a/www/Utils.js b/www/Utils.js
index ab48bdcf..775c03d7 100644
--- a/www/Utils.js
+++ b/www/Utils.js
@@ -161,6 +161,11 @@ Ext.define('PBS.Utils', {
return `Datastore ${what} ${id}`;
},
+ // mimics Display trait in backend
+ renderKeyID: function(fingerprint) {
+ return fingerprint.substring(0, 23);
+ },
+
parse_datastore_worker_id: function(type, id) {
let result;
let res;
diff --git a/www/datastore/Content.js b/www/datastore/Content.js
index acd4184b..12dacb89 100644
--- a/www/datastore/Content.js
+++ b/www/datastore/Content.js
@@ -12,6 +12,7 @@ Ext.define('pbs-data-store-snapshots', {
'files',
'owner',
'verification',
+ 'fingerprint',
{ name: 'size', type: 'int', allowNull: true },
{
name: 'crypt-mode',
@@ -182,6 +183,7 @@ Ext.define('PBS.DataStoreContent', {
for (const file of data.files) {
file.text = file.filename;
file['crypt-mode'] = PBS.Utils.cryptmap.indexOf(file['crypt-mode']);
+ file.fingerprint = data.fingerprint;
file.leaf = true;
file.matchesFilter = true;
@@ -699,7 +701,16 @@ Ext.define('PBS.DataStoreContent', {
if (iconCls) {
iconTxt = `<i class="fa fa-fw fa-${iconCls}"></i> `;
}
- return (iconTxt + PBS.Utils.cryptText[v]) || Proxmox.Utils.unknownText;
+ let tip;
+ if (v !== PBS.Utils.cryptmap.indexOf('none') && record.data.fingerprint !== undefined) {
+ tip = "Key: " + PBS.Utils.renderKeyID(record.data.fingerprint);
+ }
+ let txt = (iconTxt + PBS.Utils.cryptText[v]) || Proxmox.Utils.unknownText;
+ if (record.parentNode.id === 'root' || tip === undefined) {
+ return txt;
+ } else {
+ return `<span data-qtip="${tip}">${txt}</span>`;
+ }
},
},
{
--
2.20.1
^ permalink raw reply [flat|nested] 26+ messages in thread* [pbs-devel] [PATCH proxmox-backup-qemu 1/2] adapt to proxmox-backup fingerprint changes
2020-11-20 16:38 [pbs-devel] [PATCH v2 proxmox-backup(-qemu) 00/15] add, persist and check fingerprint Fabian Grünbichler
` (12 preceding siblings ...)
2020-11-20 16:38 ` [pbs-devel] [PATCH proxmox-backup 13/13] gui: add snapshot/file fingerprint tooltip Fabian Grünbichler
@ 2020-11-20 16:38 ` Fabian Grünbichler
2020-11-24 8:07 ` [pbs-devel] applied: " Dietmar Maurer
2020-11-20 16:38 ` [pbs-devel] [PATCH proxmox-backup-qemu 2/2] restore: improve error if key is missing Fabian Grünbichler
2020-11-24 7:47 ` [pbs-devel] [PATCH v2 proxmox-backup(-qemu) 00/15] add, persist and check fingerprint Dietmar Maurer
15 siblings, 1 reply; 26+ messages in thread
From: Fabian Grünbichler @ 2020-11-20 16:38 UTC (permalink / raw)
To: pbs-devel
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
requires proxmox-backup with other patches from this series applied
src/backup.rs | 2 +-
src/restore.rs | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/backup.rs b/src/backup.rs
index 184ac64..c3a7c11 100644
--- a/src/backup.rs
+++ b/src/backup.rs
@@ -47,7 +47,7 @@ impl BackupTask {
let crypt_config = match setup.keyfile {
None => None,
Some(ref path) => {
- let (key, _) = load_and_decrypt_key(path, & || {
+ let (key, _, _) = load_and_decrypt_key(path, & || {
match setup.key_password {
Some(ref key_password) => Ok(key_password.as_bytes().to_vec()),
None => bail!("no key_password specified"),
diff --git a/src/restore.rs b/src/restore.rs
index ad825bd..6cba71f 100644
--- a/src/restore.rs
+++ b/src/restore.rs
@@ -41,7 +41,7 @@ impl RestoreTask {
let crypt_config = match setup.keyfile {
None => None,
Some(ref path) => {
- let (key, _) = load_and_decrypt_key(path, & || {
+ let (key, _, _) = load_and_decrypt_key(path, & || {
match setup.key_password {
Some(ref key_password) => Ok(key_password.as_bytes().to_vec()),
None => bail!("no key_password specified"),
--
2.20.1
^ permalink raw reply [flat|nested] 26+ messages in thread* [pbs-devel] [PATCH proxmox-backup-qemu 2/2] restore: improve error if key is missing
2020-11-20 16:38 [pbs-devel] [PATCH v2 proxmox-backup(-qemu) 00/15] add, persist and check fingerprint Fabian Grünbichler
` (13 preceding siblings ...)
2020-11-20 16:38 ` [pbs-devel] [PATCH proxmox-backup-qemu 1/2] adapt to proxmox-backup fingerprint changes Fabian Grünbichler
@ 2020-11-20 16:38 ` Fabian Grünbichler
2020-11-24 7:47 ` [pbs-devel] [PATCH v2 proxmox-backup(-qemu) 00/15] add, persist and check fingerprint Dietmar Maurer
15 siblings, 0 replies; 26+ messages in thread
From: Fabian Grünbichler @ 2020-11-20 16:38 UTC (permalink / raw)
To: pbs-devel
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
Notes: regular users should never run into this check, since we first
restore the config file via proxmox-backup-client, which already checks
this and fails.
requires proxmox-backup patches from this series
src/restore.rs | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/restore.rs b/src/restore.rs
index 6cba71f..24983dd 100644
--- a/src/restore.rs
+++ b/src/restore.rs
@@ -92,6 +92,7 @@ impl RestoreTask {
).await?;
let (manifest, _) = client.download_manifest().await?;
+ manifest.check_fingerprint(self.crypt_config.as_ref().map(Arc::as_ref))?;
self.manifest.set(Arc::new(manifest))
.map_err(|_| format_err!("already connected!"))?;
--
2.20.1
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [pbs-devel] [PATCH v2 proxmox-backup(-qemu) 00/15] add, persist and check fingerprint
2020-11-20 16:38 [pbs-devel] [PATCH v2 proxmox-backup(-qemu) 00/15] add, persist and check fingerprint Fabian Grünbichler
` (14 preceding siblings ...)
2020-11-20 16:38 ` [pbs-devel] [PATCH proxmox-backup-qemu 2/2] restore: improve error if key is missing Fabian Grünbichler
@ 2020-11-24 7:47 ` Dietmar Maurer
15 siblings, 0 replies; 26+ messages in thread
From: Dietmar Maurer @ 2020-11-24 7:47 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Fabian Grünbichler
finally applied, without patch 9/13
also changed the "key show" command to use format_and_print_result_full()
> On 11/20/2020 5:38 PM Fabian Grünbichler <f.gruenbichler@proxmox.com> wrote:
>
>
> changes since v1:
> - switch to wrapping fingerprint in (transparent) Fingerprint struct
> - display as 'short key ID' of 8 bytes (easily swappable to other
> format if desired)
> - return in list_snapshots, display in GUI
> - improve log messages for (non-)incremental backups
> - handle fingerprint in paperkey
> - adapt qemu library
>
^ permalink raw reply [flat|nested] 26+ messages in thread