Replace Argon2's pepper with a random root predecessor nonce

Cargo.toml

@@ -36,6 +36,7 @@ itertools = "0.13.0"
 proptest = "1.5.0"
 proptest-arbitrary-interop = "0.1.0"
 proptest-derive = "0.5.0"
+rand = "0.8.5"
 serde = { version = "1.0.203", features = ["derive"] }
 serde_with = "3.8.1"
 sha2 = "0.10.8"

README.md

@@ -27,7 +27,6 @@ Launch it by running the following:
     ```
 
 ## TODO
-* [ ] Replace Argon2's pepper with configured initial succession nonce, do not use pepper by default.
 * [ ] Splitting records into multiple files.
     * [x] Basic library implementation
     * [ ] Pad contents to a registry-wide constant length.

benches/benches.rs

@@ -24,21 +24,20 @@ fn with_samples(samples: usize) -> Criterion {
 #[criterion(with_samples(10))]
 fn bench_hash_params(c: &mut Criterion) {
     let hash = RegistryConfigHash {
-        algorithm: PasswordHashAlgorithm::Argon2(
-            Argon2Params::default_with_random_pepper_of_recommended_length(OsRng),
-        ),
+        algorithm: PasswordHashAlgorithm::Argon2(Argon2Params::default()),
         output_length_in_bytes: ConfigParam::try_from(32).unwrap(),
     };
-    let kdf = RegistryConfigKdf {
-        algorithm: KdfAlgorithm::Hkdf(HkdfParams {
+    let kdf = RegistryConfigKdf::builder()
+        .with_algorithm(KdfAlgorithm::Hkdf(HkdfParams {
             prf: HkdfPrf::Sha256,
-        }),
-        succession_nonce_length_in_bytes: ConfigParam::try_from(32).unwrap(),
-        file_name_length_in_bytes: ConfigParam::try_from(8).unwrap(),
-    };
+        }))
+        .with_file_name_length_in_bytes(8)
+        .with_succession_nonce_length_in_bytes(32)
+        .build_with_random_root_predecessor_nonce(OsRng)
+        .unwrap();
     let key = RecordKey {
         record_name: RecordName::from(b"hello".to_vec()),
-        predecessor_nonce: kdf.get_root_record_predecessor_nonce(),
+        predecessor_nonce: kdf.get_root_record_predecessor_nonce().clone(),
     };
 
     c.bench_function("hash_params", |b| {

cddl/registry.cddl

@@ -11,8 +11,9 @@ RRR_Registry = {
     },
     kdf: {
         algorithm: RRR_KdfAlgorithm,
-        succession_nonce_length_in_bytes: uint .ge 16,
         file_name_length_in_bytes: uint .ge 1,
+        succession_nonce_length_in_bytes: uint .ge 16,
+        root_predecessor_nonce: bstr, ; .size (16..1000), ; The upper bound is arbitrary TODO: not supported by cddl-rs
     },
     ? verifying_keys: [ * RRR_VerifyingKey ],
 }

src/cmd/mod.rs

@@ -147,7 +147,11 @@ async fn resolve_path<L>(
             .next()
             .expect("record path should never be empty")
             .clone(),
-        predecessor_nonce: registry.config.kdf.get_root_record_predecessor_nonce(),
+        predecessor_nonce: registry
+            .config
+            .kdf
+            .get_root_record_predecessor_nonce()
+            .clone(),
     }
     .hash(&registry.config.hash)
     .await?;

src/crypto/password_hash/argon2.rs

@@ -8,7 +8,6 @@ use proptest_derive::Arbitrary;
 use std::fmt::Debug;
 
 use crate::error::Result;
-use aes_gcm::aead::rand_core::{CryptoRng, RngCore};
 use argon2::Argon2;
 use serde::{Deserialize, Serialize};
 use zeroize::{Zeroize, ZeroizeOnDrop};
@@ -40,31 +39,19 @@ pub struct Argon2Params {
     /// Degree of parallelism. Between 1 and (2^24)-1.
     #[zeroize(skip)]
     pub p_cost: u32,
-    pub pepper: Secret<BytesOrHexString<Vec<u8>>>,
+    pub pepper: Option<Secret<BytesOrHexString<Vec<u8>>>>,
 }
 
-impl Argon2Params {
-    pub fn default_with_pepper(pepper: impl Into<Vec<u8>>) -> Self {
+impl Default for Argon2Params {
+    fn default() -> Self {
         Self {
             variant: Variant::Argon2id,
             m_cost: 1 << 10, // 1 MiB
             t_cost: 1 << 10,
             p_cost: 1,
-            pepper: Secret(BytesOrHexString(pepper.into())),
+            pepper: None,
         }
     }
-
-    pub fn default_with_random_pepper<const PEPPER_BYTES: usize>(
-        mut rng: impl CryptoRng + RngCore,
-    ) -> Self {
-        let mut pepper = vec![0; PEPPER_BYTES];
-        rng.fill_bytes(&mut pepper);
-        Self::default_with_pepper(pepper)
-    }
-
-    pub fn default_with_random_pepper_of_recommended_length(rng: impl CryptoRng + RngCore) -> Self {
-        Self::default_with_random_pepper::<{ super::BYTES_HASH_PEPPER_RECOMMENDED }>(rng)
-    }
 }
 
 impl PasswordHash for Argon2Params {
@@ -74,12 +61,20 @@ impl PasswordHash for Argon2Params {
             Variant::Argon2i => argon2::Algorithm::Argon2i,
             Variant::Argon2id => argon2::Algorithm::Argon2id,
         };
-        let argon2 = Argon2::new_with_secret(
-            &self.pepper,
-            variant,
-            argon2::Version::V0x13,
-            argon2::Params::new(self.m_cost, self.t_cost, self.p_cost, Some(output.len()))?,
-        )?;
+        let argon2 = if let Some(pepper) = self.pepper.as_ref() {
+            Argon2::new_with_secret(
+                pepper,
+                variant,
+                argon2::Version::V0x13,
+                argon2::Params::new(self.m_cost, self.t_cost, self.p_cost, Some(output.len()))?,
+            )?
+        } else {
+            Argon2::new(
+                variant,
+                argon2::Version::V0x13,
+                argon2::Params::new(self.m_cost, self.t_cost, self.p_cost, Some(output.len()))?,
+            )
+        };
         argon2.hash_password_into(password, salt, output)?;
 
         Ok(())
@@ -95,9 +90,15 @@ prop_compose! {
         m_cost in (8 * p_cost)..(std::cmp::max(8 * p_cost + 1, 1 << log_max_cost)),
         t_cost in Just(t_cost),
         p_cost in Just(p_cost),
-        pepper in proptest::collection::vec(any::<u8>(), 0..(1 << 10)),
+        pepper in proptest::option::of(proptest::collection::vec(any::<u8>(), 0..(1 << 10))),
     ) -> Argon2Params {
-        Argon2Params { variant, m_cost, t_cost, p_cost, pepper: Secret(BytesOrHexString(pepper)) }
+        Argon2Params {
+            variant,
+            m_cost,
+            t_cost,
+            p_cost,
+            pepper: pepper.map(|pepper| Secret(BytesOrHexString(pepper))),
+        }
     }
 }
 

src/crypto/password_hash/mod.rs

@@ -11,8 +11,6 @@ use crate::error::Result;
 
 pub mod argon2;
 
-pub const BYTES_HASH_PEPPER_RECOMMENDED: usize = 32;
-
 pub trait PasswordHash {
     fn hash_password(&self, password: &[u8], salt: &[u8], output: &mut [u8]) -> Result<()>;
 }

src/error.rs

@@ -2,8 +2,11 @@ use std::borrow::Cow;
 
 use async_fd_lock::LockError;
 use coset::RegisteredLabel;
+use derive_more::From;
 use thiserror::Error;
 
+use crate::registry::{ConfigParamTooLowError, ConfigParamTrait};
+
 pub type Result<T> = std::result::Result<T, Error>;
 
 #[derive(Error, Debug)]
@@ -44,6 +47,8 @@ pub enum Error {
     UnrecognizedEncryptionAlgorithm { alg: Option<coset::Algorithm> },
     #[error("Failed to resolve collisions")]
     CollisionResolutionFailed,
+    #[error(transparent)]
+    InvalidParameter(#[from] InvalidParameterError),
 }
 
 impl<T> From<LockError<T>> for Error {
@@ -53,3 +58,60 @@ impl<T> From<LockError<T>> for Error {
 }
 
 pub struct SignatureMismatch;
+
+#[derive(Error, Debug)]
+#[error("Invalid parameter `{label}`: {source}")]
+pub struct InvalidParameterError {
+    pub label: &'static str,
+    #[source]
+    pub source: Box<dyn std::error::Error + Send + Sync + 'static>,
+}
+
+impl<P> From<ConfigParamTooLowError<P>> for InvalidParameterError
+where
+    P: ConfigParamTrait + 'static,
+{
+    fn from(value: ConfigParamTooLowError<P>) -> Self {
+        InvalidParameterError {
+            label: P::LABEL,
+            source: Box::new(value),
+        }
+    }
+}
+
+#[derive(Error, Debug, From)]
+#[error("{0}")]
+pub struct GenericError(pub Cow<'static, str>);
+
+impl From<&'static str> for GenericError {
+    fn from(value: &'static str) -> Self {
+        Self(Cow::Borrowed(value))
+    }
+}
+
+impl From<String> for GenericError {
+    fn from(value: String) -> Self {
+        Self(Cow::Owned(value))
+    }
+}
+
+pub trait OptionExt<T> {
+    fn unwrap_builder_parameter(
+        &self,
+        label: &'static str,
+    ) -> std::result::Result<T, InvalidParameterError>;
+}
+
+impl<T: Clone> OptionExt<T> for Option<T> {
+    fn unwrap_builder_parameter(
+        &self,
+        label: &'static str,
+    ) -> std::result::Result<T, InvalidParameterError> {
+        self.as_ref().cloned().ok_or_else(|| InvalidParameterError {
+            label,
+            source: Box::new(GenericError::from(
+                "value is required but was not specified",
+            )),
+        })
+    }
+}

src/record/key.rs

@@ -3,14 +3,14 @@ use crate::crypto::kdf::KdfExt;
 use crate::crypto::password_hash::PasswordHash;
 use crate::error::Result;
 use crate::record::segment::{
-    FragmentEncryptionKeyBytes, FragmentFileNameBytes, KdfUsage,
-    KdfUsageFragmentParameters, KdfUsageFragmentUsage,
+    FragmentEncryptionKeyBytes, FragmentFileNameBytes, KdfUsage, KdfUsageFragmentParameters,
+    KdfUsageFragmentUsage,
 };
 use crate::registry::{Registry, RegistryConfigHash, RegistryConfigKdf};
 use crate::utils::serde::{BytesOrHexString, Secret};
 use async_trait::async_trait;
-use std::ops::{Deref, DerefMut};
 use std::fmt::Debug;
+use std::ops::{Deref, DerefMut};
 use zeroize::{Zeroize, ZeroizeOnDrop};
 
 use super::{RecordName, SuccessionNonce};
@@ -70,7 +70,7 @@ impl HashedRecordKey {
         okm: &mut [u8],
     ) -> Result<()> {
         kdf_params
-            .algorithm
+            .get_algorithm()
             .derive_key_from_canonicalized_cbor(self, usage, okm)?;
         Ok(())
     }
@@ -101,21 +101,22 @@ impl HashedRecordKey {
         &self,
         kdf_params: &RegistryConfigKdf,
     ) -> Result<SuccessionNonce> {
-        let mut okm =
-            vec![0_u8; *kdf_params.succession_nonce_length_in_bytes as usize].into_boxed_slice();
+        let mut okm = vec![0_u8; kdf_params.get_succession_nonce_length_in_bytes() as usize]
+            .into_boxed_slice();
 
         self.derive_key(&KdfUsage::SuccessionNonce {}, kdf_params, &mut okm)
             .await?;
 
-        Ok(SuccessionNonce(Secret(okm)))
+        Ok(SuccessionNonce(Secret(BytesOrHexString(okm))))
     }
 
     pub async fn derive_fragment_file_name(
         &self,
         kdf_params: &RegistryConfigKdf,
         fragment_parameters: &KdfUsageFragmentParameters,
     ) -> Result<FragmentFileNameBytes> {
-        let mut okm = vec![0_u8; *kdf_params.file_name_length_in_bytes as usize].into_boxed_slice();
+        let mut okm =
+            vec![0_u8; kdf_params.get_file_name_length_in_bytes() as usize].into_boxed_slice();
         let usage = KdfUsage::Fragment {
             usage: KdfUsageFragmentUsage::FileName,
             parameters: fragment_parameters.clone(),

src/record/mod.rs

@@ -3,7 +3,7 @@ use crate::crypto::encryption::EncryptionAlgorithm;
 use crate::crypto::signature::SigningKey;
 use crate::error::{Error, Result};
 use crate::registry::{Registry, WriteLock};
-use crate::utils::serde::{BytesOrAscii, Secret};
+use crate::utils::serde::{BytesOrAscii, BytesOrHexString, Secret};
 use async_fd_lock::{LockRead, LockWrite};
 use chrono::{DateTime, FixedOffset, TimeZone};
 use coset::cbor::tag;
@@ -15,9 +15,8 @@ use proptest::strategy::{BoxedStrategy, Strategy};
 use proptest_arbitrary_interop::arb;
 use proptest_derive::Arbitrary;
 use segment::{
-    FragmentFileNameBytes, FragmentKey, FragmentReadSuccess,
-    KdfUsageFragmentParameters, RecordNonce, RecordParameters,
-    RecordVersion, Segment, SegmentMetadata,
+    FragmentFileNameBytes, FragmentKey, FragmentReadSuccess, KdfUsageFragmentParameters,
+    RecordNonce, RecordParameters, RecordVersion, Segment, SegmentMetadata,
 };
 use serde::{Deserialize, Serialize};
 use std::ops::{Deref, DerefMut};
@@ -36,8 +35,8 @@ mod path;
 pub use key::*;
 pub use path::*;
 
-#[derive(Clone, PartialEq, Eq, Debug, Zeroize, ZeroizeOnDrop)]
-pub struct SuccessionNonce(pub(crate) Secret<Box<[u8]>>);
+#[derive(Clone, PartialEq, Eq, Debug, Zeroize, ZeroizeOnDrop, Serialize, Deserialize)]
+pub struct SuccessionNonce(pub(crate) Secret<BytesOrHexString<Box<[u8]>>>);
 
 impl Deref for SuccessionNonce {
     type Target = [u8];

src/record/path.rs

@@ -81,7 +81,11 @@ impl HashRecordPath for RecordPath {
                 .next()
                 .expect("record path should never be empty")
                 .clone(),
-            predecessor_nonce: registry.config.kdf.get_root_record_predecessor_nonce(),
+            predecessor_nonce: registry
+                .config
+                .kdf
+                .get_root_record_predecessor_nonce()
+                .clone(),
         }
         .hash(&registry.config.hash)
         .await?;