diff --git a/CHANGELOG.md b/CHANGELOG.md index 445fbe2..aa08a03 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,11 +8,63 @@ and this library adheres to Rust's notion of ## [Unreleased] ### Added - `fpe::ff1::{InvalidRadix, NumeralStringError}` +- `fpe::ff1::FF1NewError`: new error type returned by `FF1fr::new`, with + variants `InvalidRadix` and `InvalidKeyLength`. +- `fpe::ff1::NumeralStringError::TweakTooLong`: returned by `encrypt`/`decrypt` + when the tweak length exceeds `u32::MAX` bytes (NIST SP 800-38G §5.1). +- `fpe::ff1::NumeralStringError::NotByteAligned`: returned by + `BinaryNumeralString::to_bytes_le` when the length is not a multiple of 8. + +### Security +- `fpe::ff1`: `Prf` now implements `Drop` and zeroes its CBC output block + buffer on drop, removing key-derived bytes from memory. +- `fpe::ff1::FF1fr`: the expanded cipher key schedule is zeroed on drop for + any `CIPH` that implements `ZeroizeOnDrop` (all `aes 0.8` types do so by + default). +- `fpe::ff1::FF1fr::new`: construction now panics if `FEISTEL_ROUNDS < 8`, + preventing the creation of instances that would perform identity encryption. +- `fpe::ff1::FF1fr::new`: construction now panics if the cipher's block size + is not 128 bits, enforcing the requirement of NIST SP 800-38G §4.3. + +### Fixed +- `fpe::ff1::FF1fr::new`: now uses `KeyInit::new_from_slice` and returns + `FF1NewError::InvalidKeyLength` instead of panicking on wrong-length keys. +- `fpe::ff1::alloc::BinaryNumeralString::to_bytes_le`: no longer panics on + non-byte-aligned input; returns `Err(NumeralStringError::NotByteAligned)`. +- `fpe::ff1::alloc`: replaced O(n) manual exponentiation loop in `pow` with + `num_traits::pow::pow`, which uses O(log n) binary exponentiation, closing a + DoS vector for large numeral strings. +- `fpe::ff1::alloc`: `assert_eq!(radix, 2)` in `BinaryNumeralString::num_radix` + and `str_radix` replaced with `debug_assert_eq!`; these are internal + invariants always satisfied by the validated call path. +- `fpe::ff1::alloc`: removed redundant parentheses in `is_valid` closures for + both `FlexibleNumeralString` and `BinaryNumeralString` (Clippy lint + `clippy::unused_parens`). +- `fpe::ff1::test_vectors`: replaced deprecated `array::IntoIter::new([...])` + with `IntoIterator::into_iter([...])` and removed the associated + `#[allow(deprecated)]` attribute and unused `use core::array` import. ### Changed -- MSRV is now 1.56.0. -- Bumped dependencies to `cipher 0.4`, `cbc 0.1`. +- MSRV is now 1.70.0. +- Bumped dependencies to `cipher 0.4`, `cbc 0.1`; added `zeroize = "1"`. - `aes 0.8` is now the minimum compatible crate version. +- `fpe::ff1::FF1fr::new` now returns `Result` instead of + `Result`; `FF1NewError` also covers invalid key length. +- `fpe::ff1::encrypt` and `decrypt` now return + `Err(NumeralStringError::TweakTooLong)` when the tweak exceeds `u32::MAX` + bytes (previously the tweak length would silently truncate in the P-block). +- `fpe::ff1`: the 16-byte P-block construction is annotated with inline + comments mapping each field to NIST SP 800-38G §6.2 Step 5. The `u` field + now uses the explicit cast `(u % 256) as u8` instead of relying on implicit + truncation. +- `fpe::ff1::Radix::to_u32`: duplicate match arms consolidated using an OR + pattern (`Radix::Any { .. } | Radix::PowerTwo { .. } => radix`). +- `fpe::ff1`: documentation updated to reference the final NIST SP 800-38G + (removed the draft Revision 1 URL). +- `fpe::ff1::FF1h`: documented as a non-standard extension that breaks CAVP + compliance and interoperability with conforming FF1 implementations. +- `fpe::ff1`: module documentation notes that only FF1 is implemented; FF3 + (also defined in NIST SP 800-38G) is not provided. - `fpe::ff1`: - `FF1::new` now returns `Result<_, InvalidRadix>`. - `FF1::{encrypt, decrypt}` now return `Result<_, NumeralStringError>`. diff --git a/Cargo.toml b/Cargo.toml index aa0bd3d..25e5529 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,10 +1,10 @@ [package] name = "fpe" version = "0.5.1" -authors = ["Jack Grigg "] +authors = ["Jack Grigg ", "Bruno Grieder ", "Hatem Mnaouer "] license = "MIT/Apache-2.0" edition = "2018" -rust-version = "1.56" +rust-version = "1.70" description = "Format-preserving encryption" documentation = "https://docs.rs/fpe/" homepage = "https://github.com/str4d/fpe" @@ -17,17 +17,13 @@ cbc = { version = "0.1", default-features = false } cipher = "0.4" libm = "0.2" static_assertions = "1.1" - +zeroize = { version = "1.8", default-features = false } num-bigint = { version = "0.4", optional = true, default-features = false } num-integer = { version = "0.1", optional = true, default-features = false } num-traits = { version = "0.2", optional = true, default-features = false } [dev-dependencies] aes = "0.8" - -# Benchmarks -#aes-old = { package = "aes", version = "0.3" } -#binary-ff1 = "0.1" criterion = "0.4" [target.'cfg(any(target_arch = "x86", target_arch = "x86_64"))'.dev-dependencies] diff --git a/src/ff1.rs b/src/ff1.rs index 59f8f4c..c22c158 100644 --- a/src/ff1.rs +++ b/src/ff1.rs @@ -1,18 +1,22 @@ //! A Rust implementation of the FF1 algorithm, specified in -//! [NIST Special Publication 800-38G](http://dx.doi.org/10.6028/NIST.SP.800-38G). +//! [NIST Special Publication 800-38G](https://nvlpubs.nist.gov/nistpubs/SpecialPublications/NIST.SP.800-38G.pdf). +//! +//! This crate implements **FF1 only**. FF3 (also defined in NIST SP 800-38G) is +//! not provided. use core::cmp; use cipher::{ generic_array::GenericArray, Block, BlockCipher, BlockEncrypt, BlockEncryptMut, InnerIvInit, - KeyInit, + KeyInit, Unsigned, }; +use zeroize::Zeroize; #[cfg(test)] use static_assertions::const_assert; mod error; -pub use error::{InvalidRadix, NumeralStringError}; +pub use error::{FF1NewError, InvalidRadix, NumeralStringError}; #[cfg(feature = "alloc")] mod alloc; @@ -22,6 +26,9 @@ pub use self::alloc::{BinaryNumeralString, FlexibleNumeralString}; #[cfg(test)] mod test_vectors; +#[cfg(test)] +mod ff1_18; + /// The minimum allowed numeral string length for any radix. const MIN_NS_LEN: u32 = 2; /// The maximum allowed numeral string length for any radix. @@ -29,7 +36,7 @@ const MAX_NS_LEN: usize = u32::MAX as usize; /// The minimum allowed value of radix^minlen. /// -/// Defined in [NIST SP 800-38G Revision 1](https://nvlpubs.nist.gov/nistpubs/SpecialPublications/NIST.SP.800-38Gr1-draft.pdf). +/// Defined in [NIST SP 800-38G](https://nvlpubs.nist.gov/nistpubs/SpecialPublications/NIST.SP.800-38G.pdf). #[cfg(test)] const MIN_NS_DOMAIN_SIZE: u32 = 1_000_000; @@ -121,8 +128,7 @@ impl Radix { fn to_u32(&self) -> u32 { match *self { - Radix::Any { radix, .. } => radix, - Radix::PowerTwo { radix, .. } => radix, + Radix::Any { radix, .. } | Radix::PowerTwo { radix, .. } => radix, } } } @@ -181,6 +187,14 @@ struct Prf { offset: usize, } +impl Drop for Prf { + fn drop(&mut self) { + // Zero the CBC output block to remove any key-derived bytes. + // Note: `Block = GenericArray` implements Zeroize because u8: Zeroize. + self.buf[0].zeroize(); + } +} + impl Prf { fn new(ciph: &CIPH) -> Self { let ciph = ciph.clone(); @@ -232,24 +246,65 @@ fn generate_s<'a, CIPH: BlockEncrypt>( .take(d) } -/// A struct for performing FF1 encryption and decryption operations. -pub struct FF1 { +/// A struct for performing FF1 encryption and decryption operations +/// using the default 10 Feistel rounds +pub type FF1 = FF1fr<10, CIPH>; + +/// A struct for performing hardened FF1 encryption and decryption operations +/// using 18 Feistel rounds. +/// +/// # ⚠ Non-standard +/// NIST SP 800-38G standardises exactly **10 rounds** for FF1. This 18-round +/// variant provides a higher security margin but is **not NIST-compliant** and +/// will fail CAVP certification. It is **not interoperable** with conforming +/// implementations. +pub type FF1h = FF1fr<18, CIPH>; + +/// A struct for performing FF1 encryption and decryption operations +/// with an adjustable number of Feistel rounds. +/// +/// # Key material +/// This struct holds the expanded cipher key schedule. When `FF1fr` is dropped +/// the key schedule is zeroed if `CIPH` implements [`ZeroizeOnDrop`] — all +/// `aes 0.8` types (`Aes128`, `Aes192`, `Aes256`) do so by default. +/// The internal CBC output buffer is always zeroed on drop. +/// +/// [`ZeroizeOnDrop`]: zeroize::ZeroizeOnDrop +pub struct FF1fr { ciph: CIPH, radix: Radix, } -impl FF1 { +impl FF1fr { /// Creates a new FF1 object for the given key and radix. /// - /// Returns an error if the given radix is not in [2..2^16]. - pub fn new(key: &[u8], radix: u32) -> Result { - let ciph = CIPH::new(GenericArray::from_slice(key)); + /// Returns an error if: + /// - the radix is not in `[2..2^16]`, or + /// - the key length does not match the cipher's requirement. + /// + /// # Panics + /// Panics at construction time if `FEISTEL_ROUNDS < 8` or if the cipher's + /// block size is not 128 bits (16 bytes), as required by NIST SP 800-38G §4.3. + pub fn new(key: &[u8], radix: u32) -> Result { + assert!( + FEISTEL_ROUNDS >= 8, + "FF1fr requires at least 8 Feistel rounds; got FEISTEL_ROUNDS = {}", + FEISTEL_ROUNDS + ); + assert_eq!( + CIPH::BlockSize::USIZE, + 16, + "FF1 requires a 128-bit (16-byte) block cipher (NIST SP 800-38G §4.3)" + ); + let ciph = CIPH::new_from_slice(key).map_err(|_| FF1NewError::InvalidKeyLength)?; let radix = Radix::from_u32(radix)?; - Ok(FF1 { ciph, radix }) + Ok(FF1fr { ciph, radix }) } } -impl FF1 { +impl + FF1fr +{ /// Encrypts the given numeral string. /// /// Returns an error if the numeral string is not in the required radix. @@ -267,6 +322,11 @@ impl FF1 { let n = x.numeral_count(); let t = tweak.len(); + // Enforce NIST SP 800-38G §5.1: t must fit in 4 bytes in the P-block. + if t > u32::MAX as usize { + return Err(NumeralStringError::TweakTooLong); + } + // 1. Let u = floor(n / 2); v = n - u let u = n / 2; let v = n - u; @@ -280,8 +340,16 @@ impl FF1 { // 4. Let d = 4 * ceil(b / 4) + 4. let d = 4 * ((b + 3) / 4) + 4; - // 5. Let P = [1, 2, 1] || [radix] || [10] || [u mod 256] || [n] || [t]. - let mut p = [1, 2, 1, 0, 0, 0, 10, u as u8, 0, 0, 0, 0, 0, 0, 0, 0]; + // 5. Build the 16-byte P-block (NIST SP 800-38G §6.2, Step 5): + // P = [1]₁ VERS: 1 (FF1) + // || [2]₁ ALGO: 2 (AES-CBCMAC) + // || [1]₁ constant + // || [radix]₃ radix in 3 big-endian bytes + // || [10]₁ constant + // || [u mod 256]₁ + // || [n]₄ numeral string length + // || [t]₄ tweak length + let mut p = [1, 2, 1, 0, 0, 0, 10, (u % 256) as u8, 0, 0, 0, 0, 0, 0, 0, 0]; p[3..6].copy_from_slice(&self.radix.to_u32().to_be_bytes()[1..]); p[8..12].copy_from_slice(&(n as u32).to_be_bytes()); p[12..16].copy_from_slice(&(t as u32).to_be_bytes()); @@ -294,7 +362,7 @@ impl FF1 { for _ in 0..((((-(t as i32) - (b as i32) - 1) % 16) + 16) % 16) { prf.update(&[0]); } - for i in 0..10 { + for i in 0..FEISTEL_ROUNDS { let mut prf = prf.clone(); prf.update(&[i]); prf.update(x_b.num_radix(self.radix.to_u32()).to_bytes(b).as_ref()); @@ -345,6 +413,11 @@ impl FF1 { let n = x.numeral_count(); let t = tweak.len(); + // Enforce NIST SP 800-38G §5.1: t must fit in 4 bytes in the P-block. + if t > u32::MAX as usize { + return Err(NumeralStringError::TweakTooLong); + } + // 1. Let u = floor(n / 2); v = n - u let u = n / 2; let v = n - u; @@ -358,8 +431,16 @@ impl FF1 { // 4. Let d = 4 * ceil(b / 4) + 4. let d = 4 * ((b + 3) / 4) + 4; - // 5. Let P = [1, 2, 1] || [radix] || [10] || [u mod 256] || [n] || [t]. - let mut p = [1, 2, 1, 0, 0, 0, 10, u as u8, 0, 0, 0, 0, 0, 0, 0, 0]; + // 5. Build the 16-byte P-block (NIST SP 800-38G §6.2, Step 5): + // P = [1]₁ VERS: 1 (FF1) + // || [2]₁ ALGO: 2 (AES-CBCMAC) + // || [1]₁ constant + // || [radix]₃ radix in 3 big-endian bytes + // || [10]₁ constant + // || [u mod 256]₁ + // || [n]₄ numeral string length + // || [t]₄ tweak length + let mut p = [1, 2, 1, 0, 0, 0, 10, (u % 256) as u8, 0, 0, 0, 0, 0, 0, 0, 0]; p[3..6].copy_from_slice(&self.radix.to_u32().to_be_bytes()[1..]); p[8..12].copy_from_slice(&(n as u32).to_be_bytes()); p[12..16].copy_from_slice(&(t as u32).to_be_bytes()); @@ -372,8 +453,8 @@ impl FF1 { for _ in 0..((((-(t as i32) - (b as i32) - 1) % 16) + 16) % 16) { prf.update(&[0]); } - for i in 0..10 { - let i = 9 - i; + for i in 0..FEISTEL_ROUNDS { + let i = FEISTEL_ROUNDS - 1 - i; let mut prf = prf.clone(); prf.update(&[i]); prf.update(x_a.num_radix(self.radix.to_u32()).to_bytes(b).as_ref()); diff --git a/src/ff1/alloc.rs b/src/ff1/alloc.rs index f5c1569..f90a37a 100644 --- a/src/ff1/alloc.rs +++ b/src/ff1/alloc.rs @@ -14,11 +14,7 @@ use num_traits::{ use super::{Numeral, NumeralString}; fn pow(x: u32, e: usize) -> BigUint { - let mut res = BigUint::one(); - for _ in 0..e { - res *= x; - } - res + num_traits::pow::pow(BigUint::from(x), e) } impl Numeral for BigUint { @@ -76,7 +72,7 @@ impl NumeralString for FlexibleNumeralString { type Num = BigUint; fn is_valid(&self, radix: u32) -> bool { - self.0.iter().all(|n| (u32::from(*n) < radix)) + self.0.iter().all(|n| u32::from(*n) < radix) } fn numeral_count(&self) -> usize { @@ -132,11 +128,15 @@ impl BinaryNumeralString { BinaryNumeralString(data) } - /// Returns a Vec, with each byte written from the BinaryNumeralString + /// Returns a `Vec`, with each byte written from the BinaryNumeralString /// in little-endian bit order. - pub fn to_bytes_le(&self) -> Vec { - // We should always have a multiple of eight bits - assert_eq!((self.0.len() + 7) / 8, self.0.len() / 8); + /// + /// Returns [`NumeralStringError::NotByteAligned`] if the length is not a + /// multiple of 8. + pub fn to_bytes_le(&self) -> Result, super::NumeralStringError> { + if self.0.len() % 8 != 0 { + return Err(super::NumeralStringError::NotByteAligned); + } let mut data = Vec::with_capacity(self.0.len() / 8); let mut acc = 0; let mut shift = 0; @@ -149,7 +149,7 @@ impl BinaryNumeralString { shift = 0; } } - data + Ok(data) } } @@ -157,7 +157,7 @@ impl NumeralString for BinaryNumeralString { type Num = BigUint; fn is_valid(&self, radix: u32) -> bool { - self.0.iter().all(|n| (u32::from(*n) < radix)) + self.0.iter().all(|n| u32::from(*n) < radix) } fn numeral_count(&self) -> usize { @@ -178,8 +178,8 @@ impl NumeralString for BinaryNumeralString { fn num_radix(&self, radix: u32) -> BigUint { let zero = BigUint::zero(); let one = BigUint::one(); - // Check that radix == 2 - assert_eq!(radix, 2); + // BinaryNumeralString is only valid for radix 2. + debug_assert_eq!(radix, 2); let mut res = zero; for i in &self.0 { res <<= 1; @@ -191,8 +191,8 @@ impl NumeralString for BinaryNumeralString { } fn str_radix(mut x: BigUint, radix: u32, m: usize) -> Self { - // Check that radix == 2 - assert_eq!(radix, 2); + // BinaryNumeralString is only valid for radix 2. + debug_assert_eq!(radix, 2); let mut res = vec![0; m]; for i in 0..m { if x.is_odd() { @@ -354,13 +354,13 @@ mod tests { { let pt = BinaryNumeralString::from_bytes_le(&tvb.pt); let (a, b) = pt.split(pt.numeral_count() / 2); - assert_eq!(BinaryNumeralString::concat(a, b).to_bytes_le(), tvb.pt); + assert_eq!(BinaryNumeralString::concat(a, b).to_bytes_le().unwrap(), tvb.pt); } { let ct = BinaryNumeralString::from_bytes_le(&tvb.ct); let (a, b) = ct.split(ct.numeral_count() / 2); - assert_eq!(BinaryNumeralString::concat(a, b).to_bytes_le(), tvb.ct); + assert_eq!(BinaryNumeralString::concat(a, b).to_bytes_le().unwrap(), tvb.ct); } } } @@ -387,10 +387,10 @@ mod tests { .unwrap(), ) }; - assert_eq!(pt.to_bytes_le(), tvb.pt); - assert_eq!(ct.to_bytes_le(), tvb.ct); - assert_eq!(bpt.to_bytes_le(), tvb.pt); - assert_eq!(bct.to_bytes_le(), tvb.ct); + assert_eq!(pt.to_bytes_le().unwrap(), tvb.pt); + assert_eq!(ct.to_bytes_le().unwrap(), tvb.ct); + assert_eq!(bpt.to_bytes_le().unwrap(), tvb.pt); + assert_eq!(bct.to_bytes_le().unwrap(), tvb.ct); assert_eq!(pt.0, tvpt); assert_eq!(ct.0, tvct); assert_eq!(bpt.0, tvpt); diff --git a/src/ff1/error.rs b/src/ff1/error.rs index 5f38fef..d09d3a0 100644 --- a/src/ff1/error.rs +++ b/src/ff1/error.rs @@ -13,6 +13,35 @@ impl fmt::Display for InvalidRadix { #[cfg(feature = "std")] impl std::error::Error for InvalidRadix {} +/// Error returned by [`FF1fr::new`](crate::ff1::FF1fr). +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum FF1NewError { + /// The provided radix is not in the supported range. + InvalidRadix(InvalidRadix), + /// The key length does not match the chosen cipher's requirement. + InvalidKeyLength, +} + +impl From for FF1NewError { + fn from(e: InvalidRadix) -> Self { + FF1NewError::InvalidRadix(e) + } +} + +impl fmt::Display for FF1NewError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + FF1NewError::InvalidRadix(e) => e.fmt(f), + FF1NewError::InvalidKeyLength => { + write!(f, "Invalid key length for the chosen cipher") + } + } + } +} + +#[cfg(feature = "std")] +impl std::error::Error for FF1NewError {} + /// Errors that can occur while using FF1 for encryption or decryption. #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub enum NumeralStringError { @@ -32,6 +61,11 @@ pub enum NumeralStringError { /// The minimum length allowed (in numerals) for a numeral string of its radix. min_len: usize, }, + /// The tweak was longer than `u32::MAX` bytes. + TweakTooLong, + /// A [`BinaryNumeralString`](crate::ff1::BinaryNumeralString) length is not a + /// multiple of 8 and cannot be converted to bytes. + NotByteAligned, } impl fmt::Display for NumeralStringError { @@ -50,6 +84,12 @@ impl fmt::Display for NumeralStringError { "The given numeral string is too short for FF1 ({} < {})", ns_len, min_len, ), + NumeralStringError::TweakTooLong => { + write!(f, "The tweak is longer than u32::MAX bytes") + } + NumeralStringError::NotByteAligned => { + write!(f, "BinaryNumeralString length is not a multiple of 8") + } } } } diff --git a/src/ff1/ff1_18.rs b/src/ff1/ff1_18.rs new file mode 100644 index 0000000..3d49844 --- /dev/null +++ b/src/ff1/ff1_18.rs @@ -0,0 +1,19 @@ +use aes::Aes256; + +use crate::ff1::{BinaryNumeralString, FF1h}; + +#[test] +fn test_doc_example_18_rounds() { + let key = [0; 32]; + let radix = 2; + let pt = [0xab, 0xcd, 0xef]; + + let ff = FF1h::::new(&key, radix).unwrap(); + let ct = ff + .encrypt(&[], &BinaryNumeralString::from_bytes_le(&pt)) + .unwrap(); + assert_eq!(ct.to_bytes_le().unwrap(), [0x5a, 0x6c, 0x20]); + + let p2 = ff.decrypt(&[], &ct).unwrap(); + assert_eq!(p2.to_bytes_le().unwrap(), pt); +} diff --git a/src/ff1/test_vectors.rs b/src/ff1/test_vectors.rs index 5ea3c86..d0f4336 100644 --- a/src/ff1/test_vectors.rs +++ b/src/ff1/test_vectors.rs @@ -1,4 +1,3 @@ -use core::array; #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub(crate) enum AesType { @@ -23,8 +22,7 @@ pub(crate) struct BinaryTestVector { } pub(crate) fn get() -> impl Iterator { - #[allow(deprecated)] - array::IntoIter::new([ + IntoIterator::into_iter([ // From https://csrc.nist.gov/CSRC/media/Projects/Cryptographic-Standards-and-Guidelines/documents/examples/FF1samples.pdf TestVector { // Sample #1 diff --git a/src/lib.rs b/src/lib.rs index 9bf7d3b..50e184e 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -15,10 +15,10 @@ //! //! let ff = FF1::::new(&key, radix).unwrap(); //! let ct = ff.encrypt(&[], &BinaryNumeralString::from_bytes_le(&pt)).unwrap(); -//! assert_eq!(ct.to_bytes_le(), [0x75, 0xfb, 0x62]); +//! assert_eq!(ct.to_bytes_le().unwrap(), [0x75, 0xfb, 0x62]); //! //! let p2 = ff.decrypt(&[], &ct).unwrap(); -//! assert_eq!(p2.to_bytes_le(), pt); +//! assert_eq!(p2.to_bytes_le().unwrap(), pt); //! ``` #![cfg_attr(not(any(feature = "std", test)), no_std)]