From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 3D56E60AB1 for ; Wed, 18 Nov 2020 09:48:56 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 271E718DF5 for ; Wed, 18 Nov 2020 09:48:26 +0100 (CET) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [212.186.127.180]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS id 6143518DEB for ; Wed, 18 Nov 2020 09:48:25 +0100 (CET) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 337ED4395B for ; Wed, 18 Nov 2020 09:48:25 +0100 (CET) Date: Wed, 18 Nov 2020 09:48:20 +0100 From: Wolfgang Bumiller To: Fabian =?utf-8?Q?Gr=C3=BCnbichler?= Cc: pbs-devel@lists.proxmox.com Message-ID: <20201118084820.jzsjxappoxs6ditn@wobu-vie.proxmox.com> References: <20201117175725.3634238-1-f.gruenbichler@proxmox.com> <20201117175725.3634238-3-f.gruenbichler@proxmox.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20201117175725.3634238-3-f.gruenbichler@proxmox.com> User-Agent: NeoMutt/20180716 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.009 Adjusted score from AWL reputation of From: address KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [proxmox-backup-client.rs, format.rs, key.rs, benchmark.rs, mount.rs, catalog.rs] Subject: Re: [pbs-devel] [PATCH proxmox-backup 2/7] key: add fingerprint to key config X-BeenThere: pbs-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Backup Server development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 18 Nov 2020 08:48:56 -0000 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 > --- > > 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, > + #[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, path: Option) -> 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 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::>().join(":") > +} > + > +pub mod opt_sha256_as_fingerprint { > + use serde::{self, Serializer, Deserializer}; ^ `self` not needed there > + > + pub fn serialize( > + csum: &Option<[u8; 32]>, > + serializer: S, > + ) -> Result > + 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, 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` here, but unfortunately lose the ability to reuse the existing code from below. match Option::::deserialize(deserializer)? { Some(mut s) => { , } 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(csum: &Option, serializer: S) -> Result 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, D::Error> where D: Deserializer<'de>, T: Default + AsMut<[u8]>, { match Option::::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(csum: &T, serializer: S) -> Result 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 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) } }