From: Wolfgang Bumiller <w.bumiller@proxmox.com>
To: "Fabian Grünbichler" <f.gruenbichler@proxmox.com>
Cc: pbs-devel@lists.proxmox.com
Subject: Re: [pbs-devel] [PATCH proxmox-backup 2/7] key: add fingerprint to key config
Date: Wed, 18 Nov 2020 09:48:20 +0100 [thread overview]
Message-ID: <20201118084820.jzsjxappoxs6ditn@wobu-vie.proxmox.com> (raw)
In-Reply-To: <20201117175725.3634238-3-f.gruenbichler@proxmox.com>
On Tue, Nov 17, 2020 at 06:57:20PM +0100, Fabian Grünbichler wrote:
> and set/generate it on
> - key creation
> - key passphrase change
> - key decryption if not already set (not persisted, but returned)
> - key encryption with master key
>
> Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
> ---
>
> Notes:
> 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 | 8 ++-
> src/backup/key_derivation.rs | 23 +++++++--
> 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 | 17 +++++--
> src/bin/proxmox_backup_client/mount.rs | 2 +-
> src/tools/format.rs | 58 ++++++++++++++++++++++
> 8 files changed, 105 insertions(+), 15 deletions(-)
>
> diff --git a/src/backup/crypt_config.rs b/src/backup/crypt_config.rs
> index 01ab0942..177923b3 100644
> --- a/src/backup/crypt_config.rs
> +++ b/src/backup/crypt_config.rs
> @@ -66,6 +68,10 @@ 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)]
> + #[serde(with = "crate::tools::format::opt_sha256_as_fingerprint")]
> + pub fingerprint: Option<[u8; 32]>,
> }
>
> pub fn store_key_config(
(...)
> diff --git a/src/bin/proxmox_backup_client/key.rs b/src/bin/proxmox_backup_client/key.rs
> index 8b802b3e..915ee970 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;
>
> @@ -121,6 +125,9 @@ fn create(kdf: Option<Kdf>, path: Option<String>) -> Result<(), Error> {
> let kdf = kdf.unwrap_or_default();
>
> let key = proxmox::sys::linux::random_data(32)?;
This allocates, and calls `fill_with_random_data`. You could shorten
this to:
let mut key_array = [0u8; 32];
proxmox::sys::linux::random_data(&mut key_array)?;
> + use std::convert::TryInto;
> + let key_array: [u8; 32] = key.clone()[0..32].try_into().unwrap();
> + let crypt_config = CryptConfig::new(key_array)?;
>
> match kdf {
> Kdf::None => {
> diff --git a/src/tools/format.rs b/src/tools/format.rs
> index 8fe6aa82..c16559a7 100644
> --- a/src/tools/format.rs
> +++ b/src/tools/format.rs
> @@ -102,6 +102,64 @@ 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 opt_sha256_as_fingerprint {
> + use serde::{self, Serializer, Deserializer};
^ `self` not needed there
> +
> + pub fn serialize<S>(
> + csum: &Option<[u8; 32]>,
> + serializer: S,
> + ) -> Result<S::Ok, S::Error>
> + where
> + S: Serializer,
> + {
> + crate::tools::format::sha256_as_fingerprint::serialize(&csum.unwrap(), serializer)
This unwrap makes skipping `None` a hard requirement, so any user of this without
the `#[serde(skip_serializing_if = "Option::is_none")]` attribute will
panic.
Simply translate `None` to `serialize_none()`:
match csum {
Some(csum) => super::sha256_as_fingerprint::serialize(csum, serializer),
None => serializer.serialize_none(),
}
(Also note the possible use of `super::` instead of the full
`crate::tools::format::` prefix.
> + }
> +
> + pub fn deserialize<'de, D>(
> + deserializer: D,
> + ) -> Result<Option<[u8; 32]>, D::Error>
> + where
> + D: Deserializer<'de>,
> + {
> + crate::tools::format::sha256_as_fingerprint::deserialize(deserializer)
> + .map(|digest| Some(digest))
Similarly this fails if there the option type is not skipped and
serialized eg. as json `null`. You can go via `Option<String>` here, but
unfortunately lose the ability to reuse the existing code from below.
match Option::<String>::deserialize(deserializer)? {
Some(mut s) => {
<code from sha256_as_fingerprint::deserialize>,
}
None => Ok(None),
}
I've also played around a bit with making this more general. Not sure we
need/want this, but this did work:
Assuming a `fn hex_to_bin_exact(hex: &str, out: &mut [u8]) -> Result<(), Error>`
being in `proxmox::tools` (same as `hex_to_bin` but writing to `out`
expecting the string to have an exactly matching length):
---8<---
pub mod opt_bytes_as_fingerprint {
use serde::{Deserialize, Deserializer, Serializer};
pub fn serialize<S, T>(csum: &Option<T>, serializer: S) -> Result<S::Ok, S::Error>
where
S: Serializer,
T: AsRef<[u8]>,
{
match csum {
Some(csum) => super::bytes_as_fingerprint::serialize(csum, serializer),
None => serializer.serialize_none(),
}
}
pub fn deserialize<'de, D, T>(deserializer: D) -> Result<Option<T>, D::Error>
where
D: Deserializer<'de>,
T: Default + AsMut<[u8]>,
{
match Option::<String>::deserialize(deserializer)? {
Some(mut s) => {
s.retain(|c| c != ':');
let mut out = T::default();
proxmox::tools::hex_to_bin_exact(&s, out.as_mut())
.map_err(serde::de::Error::custom)?;
Ok(Some(out))
}
None => Ok(None),
}
}
}
pub mod bytes_as_fingerprint {
use serde::{Deserialize, Serializer, Deserializer};
pub fn serialize<S, T>(csum: &T, serializer: S) -> Result<S::Ok, S::Error>
where
S: Serializer,
T: AsRef<[u8]>,
{
let s = super::as_fingerprint(csum.as_ref());
serializer.serialize_str(&s)
}
pub fn deserialize<'de, D, T>(deserializer: D) -> Result<T, D::Error>
where
D: Deserializer<'de>,
T: Default + AsMut<[u8]>,
{
let mut s = String::deserialize(deserializer)?;
s.retain(|c| c != ':');
let mut out = T::default();
proxmox::tools::hex_to_bin_exact(&s, out.as_mut())
.map_err(serde::de::Error::custom)?;
Ok(out)
}
}
next prev parent reply other threads:[~2020-11-18 8:48 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-11-17 17:57 [pbs-devel] [PATCH proxmox-backup 0/7] add, persist and check key fingerprint Fabian Grünbichler
2020-11-17 17:57 ` [pbs-devel] [PATCH proxmox-backup 1/7] crypt config: add fingerprint mechanism Fabian Grünbichler
2020-11-17 17:57 ` [pbs-devel] [PATCH proxmox-backup 2/7] key: add fingerprint to key config Fabian Grünbichler
2020-11-18 8:48 ` Wolfgang Bumiller [this message]
2020-11-17 17:57 ` [pbs-devel] [PATCH proxmox-backup 3/7] client: print key fingerprint and master key Fabian Grünbichler
2020-11-17 18:38 ` Thomas Lamprecht
2020-11-17 17:57 ` [pbs-devel] [PATCH proxmox-backup 4/7] client: add 'key show' command Fabian Grünbichler
2020-11-17 17:57 ` [pbs-devel] [PATCH proxmox-backup 5/7] add key fingerprint to manifest Fabian Grünbichler
2020-11-17 17:57 ` [pbs-devel] [PATCH proxmox-backup 6/7] fix #3139: manifest: check fingerprint when loading with key Fabian Grünbichler
2020-11-17 17:57 ` [pbs-devel] [PATCH proxmox-backup 7/7] client: check fingerprint after downloading manifest Fabian Grünbichler
2020-11-18 5:27 ` [pbs-devel] [PATCH proxmox-backup 0/7] add, persist and check key fingerprint Dietmar Maurer
2020-11-18 5:47 ` Dietmar Maurer
2020-11-18 6:47 ` Thomas Lamprecht
2020-11-18 8:27 ` Fabian Grünbichler
2020-11-18 8:54 ` Dietmar Maurer
2020-11-23 7:55 ` Dietmar Maurer
2020-11-23 8:16 ` Fabian Grünbichler
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20201118084820.jzsjxappoxs6ditn@wobu-vie.proxmox.com \
--to=w.bumiller@proxmox.com \
--cc=f.gruenbichler@proxmox.com \
--cc=pbs-devel@lists.proxmox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox