From 169803f490992a0c462803ff904fd9d457d8b4ac Mon Sep 17 00:00:00 2001 From: Brian Smith Date: Wed, 10 Apr 2019 10:47:00 -1000 Subject: [PATCH] Use distinct types for PBKDF2 and HMAC algorithms. --- src/digest.rs | 13 ------- src/hkdf.rs | 20 +++++++---- src/hmac.rs | 78 +++++++++++++++++++++++++---------------- src/pbkdf2.rs | 33 ++++++++++++++---- tests/hmac_tests.rs | 81 ++++++++++++++++++++++++------------------- tests/pbkdf2_tests.rs | 21 ++++++++--- 6 files changed, 150 insertions(+), 96 deletions(-) diff --git a/src/digest.rs b/src/digest.rs index 93f7b2ab5..29b7ec4cf 100644 --- a/src/digest.rs +++ b/src/digest.rs @@ -483,19 +483,6 @@ const SHA512_BLOCK_LEN: usize = 1024 / 8; /// The length of the length field for SHA-512-based algorithms, in bytes. const SHA512_LEN_LEN: usize = 128 / 8; -#[cfg(test)] -pub mod test_util { - use super::super::digest; - - pub static ALL_ALGORITHMS: [&digest::Algorithm; 5] = [ - &digest::SHA1, - &digest::SHA256, - &digest::SHA384, - &digest::SHA512, - &digest::SHA512_256, - ]; -} - #[cfg(test)] mod tests { diff --git a/src/hkdf.rs b/src/hkdf.rs index accb50410..35dc7ad49 100644 --- a/src/hkdf.rs +++ b/src/hkdf.rs @@ -31,20 +31,20 @@ //! //! [RFC 5869]: https://tools.ietf.org/html/rfc5869 -use crate::{digest, error, hmac}; +use crate::{error, hmac}; /// An HKDF algorithm. #[derive(Clone, Copy, Debug, Eq, PartialEq)] -pub struct Algorithm(&'static digest::Algorithm); +pub struct Algorithm(hmac::Algorithm); /// HKDF using HMAC-SHA-256. -pub static HKDF_SHA256: Algorithm = Algorithm(&digest::SHA256); +pub static HKDF_SHA256: Algorithm = Algorithm(hmac::HMAC_SHA256); /// HKDF using HMAC-SHA-384. -pub static HKDF_SHA384: Algorithm = Algorithm(&digest::SHA384); +pub static HKDF_SHA384: Algorithm = Algorithm(hmac::HMAC_SHA384); /// HKDF using HMAC-SHA-512. -pub static HKDF_SHA512: Algorithm = Algorithm(&digest::SHA512); +pub static HKDF_SHA512: Algorithm = Algorithm(hmac::HMAC_SHA512); /// A salt for HKDF operations. #[derive(Debug)] @@ -72,7 +72,13 @@ impl Salt { // zero-length string. let salt = &self.0; let prk = hmac::sign(salt, secret); - Prk(hmac::Key::new(salt.digest_algorithm(), prk.as_ref())) + Prk(hmac::Key::new(salt.algorithm(), prk.as_ref())) + } + + /// The algorithm used to derive this salt. + #[inline] + pub fn algorithm(&self) -> Algorithm { + Algorithm(self.0.algorithm()) } } @@ -109,7 +115,7 @@ impl Okm<'_> { /// imposed by the HKDF specification due to the way HKDF's counter is /// constructed.) pub fn fill(self, out: &mut [u8]) -> Result<(), error::Unspecified> { - let digest_alg = self.prk.0.digest_algorithm(); + let digest_alg = self.prk.0.algorithm().digest_algorithm(); assert!(digest_alg.block_len >= digest_alg.output_len); let mut ctx = hmac::Context::with_key(&self.prk.0); diff --git a/src/hmac.rs b/src/hmac.rs index ef71a11b8..5ead0189d 100644 --- a/src/hmac.rs +++ b/src/hmac.rs @@ -29,10 +29,10 @@ //! ## Signing a value and verifying it wasn't tampered with //! //! ``` -//! use ring::{digest, hmac, rand}; +//! use ring::{hmac, rand}; //! //! let rng = rand::SystemRandom::new(); -//! let key = hmac::Key::generate(&digest::SHA256, &rng)?; +//! let key = hmac::Key::generate(hmac::HMAC_SHA256, &rng)?; //! //! let msg = "hello, world"; //! @@ -60,12 +60,12 @@ //! let rng = rand::SystemRandom::new(); //! let key_value: [u8; digest::SHA256_OUTPUT_LEN] = rand::generate(&rng)?.expose(); //! -//! let s_key = hmac::Key::new(&digest::SHA256, &key_value); +//! let s_key = hmac::Key::new(hmac::HMAC_SHA256, key_value.as_ref()); //! let tag = hmac::sign(&s_key, msg.as_bytes()); //! //! // The receiver (somehow!) knows the key value, and uses it to verify the //! // integrity of the message. -//! let v_key = hmac::Key::new(&digest::SHA256, key_value.as_ref()); +//! let v_key = hmac::Key::new(hmac::HMAC_SHA256, key_value.as_ref()); //! hmac::verify(&v_key, msg.as_bytes(), tag.as_ref())?; //! //! # Ok::<(), ring::error::Unspecified>(()) @@ -84,7 +84,7 @@ //! let rng = rand::SystemRandom::new(); //! let mut key_value: [u8; digest::SHA384_OUTPUT_LEN] = rand::generate(&rng)?.expose(); //! -//! let s_key = hmac::Key::new(&digest::SHA384, &key_value); +//! let s_key = hmac::Key::new(hmac::HMAC_SHA384, key_value.as_ref()); //! let mut s_ctx = hmac::Context::with_key(&s_key); //! for part in &parts { //! s_ctx.update(part.as_bytes()); @@ -93,7 +93,7 @@ //! //! // The receiver (somehow!) knows the key value, and uses it to verify the //! // integrity of the message. -//! let v_key = hmac::Key::new(&digest::SHA384, key_value.as_ref()); +//! let v_key = hmac::Key::new(hmac::HMAC_SHA384, key_value.as_ref()); //! let mut msg = Vec::::new(); //! for part in &parts { //! msg.extend(part.as_bytes()); @@ -109,7 +109,31 @@ //! [code for `ring::hkdf`]: //! https://github.com/briansmith/ring/blob/master/src/hkdf.rs -use crate::{constant_time, digest, error, hkdf, rand}; +use crate::{constant_time, digest, error, rand}; + +/// An HMAC algorithm. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub struct Algorithm(&'static digest::Algorithm); + +impl Algorithm { + /// The digest algorithm this HMAC algorithm is based on. + #[inline] + pub fn digest_algorithm(&self) -> &'static digest::Algorithm { + self.0 + } +} + +/// HMAC using SHA-1. Obsolete. +pub static HMAC_SHA1_FOR_LEGACY_USE_ONLY: Algorithm = Algorithm(&digest::SHA1); + +/// HMAC using SHA-256. +pub static HMAC_SHA256: Algorithm = Algorithm(&digest::SHA256); + +/// HMAC using SHA-384. +pub static HMAC_SHA384: Algorithm = Algorithm(&digest::SHA384); + +/// HMAC using SHA-512. +pub static HMAC_SHA512: Algorithm = Algorithm(&digest::SHA512); /// A deprecated alias for `Tag`. #[deprecated(note = "`Signature` was renamed to `Tag`. This alias will be removed soon.")] @@ -147,36 +171,25 @@ pub type VerificationKey = Key; impl core::fmt::Debug for Key { fn fmt(&self, f: &mut core::fmt::Formatter) -> Result<(), core::fmt::Error> { f.debug_struct("Key") - .field("algorithm", self.digest_algorithm()) + .field("algorithm", self.algorithm().digest_algorithm()) .finish() } } impl Key { - /// Derive an HMAC key from the output of an `HKDF-Expand` operation. - /// - /// The key will be `digest_alg.output_len` bytes long, based on the - /// recommendation in https://tools.ietf.org/html/rfc2104#section-3. - pub fn derive(digest_alg: &'static digest::Algorithm, okm: hkdf::Okm) -> Self { - let mut key_bytes = [0; digest::MAX_OUTPUT_LEN]; - let key_bytes = &mut key_bytes[..digest_alg.output_len]; - okm.fill(key_bytes).unwrap(); - Self::new(digest_alg, key_bytes) - } - /// Generate an HMAC signing key using the given digest algorithm with a /// random value generated from `rng`. /// /// The key will be `digest_alg.output_len` bytes long, based on the /// recommendation in https://tools.ietf.org/html/rfc2104#section-3. pub fn generate( - digest_alg: &'static digest::Algorithm, - rng: &dyn rand::SecureRandom, + algorithm: Algorithm, + rng: &rand::SecureRandom, ) -> Result { let mut key_bytes = [0; digest::MAX_OUTPUT_LEN]; - let key_bytes = &mut key_bytes[..digest_alg.output_len]; + let key_bytes = &mut key_bytes[..algorithm.0.output_len]; rng.fill(key_bytes)?; - Ok(Self::new(digest_alg, key_bytes)) + Ok(Self::new(algorithm, key_bytes)) } /// Construct an HMAC signing key using the given digest algorithm and key @@ -198,7 +211,8 @@ impl Key { /// the truncation described above reduces their strength to only /// `digest_alg.output_len * 8` bits. Support for such keys is likely to be /// removed in a future version of *ring*. - pub fn new(digest_alg: &'static digest::Algorithm, key_value: &[u8]) -> Self { + pub fn new(algorithm: Algorithm, key_value: &[u8]) -> Self { + let digest_alg = algorithm.0; let mut key = Self { ctx_prototype: Context { inner: digest::Context::new(digest_alg), @@ -240,8 +254,9 @@ impl Key { } /// The digest algorithm for the key. - pub fn digest_algorithm(&self) -> &'static digest::Algorithm { - self.ctx_prototype.inner.algorithm() + #[inline] + pub fn algorithm(&self) -> Algorithm { + Algorithm(self.ctx_prototype.inner.algorithm()) } } @@ -317,7 +332,7 @@ pub fn verify(key: &Key, data: &[u8], tag: &[u8]) -> Result<(), error::Unspecifi #[cfg(test)] mod tests { - use crate::{digest, hmac, rand}; + use crate::{hmac, rand}; // Make sure that `Key::generate` and `verify_with_own_key` aren't // completely wacky. @@ -328,8 +343,13 @@ mod tests { const HELLO_WORLD_GOOD: &[u8] = b"hello, world"; const HELLO_WORLD_BAD: &[u8] = b"hello, worle"; - for d in &digest::test_util::ALL_ALGORITHMS { - let key = hmac::Key::generate(d, &mut rng).unwrap(); + for algorithm in &[ + hmac::HMAC_SHA1_FOR_LEGACY_USE_ONLY, + hmac::HMAC_SHA256, + hmac::HMAC_SHA384, + hmac::HMAC_SHA512, + ] { + let key = hmac::Key::generate(*algorithm, &mut rng).unwrap(); let tag = hmac::sign(&key, HELLO_WORLD_GOOD); assert!(hmac::verify(&key, HELLO_WORLD_GOOD, tag.as_ref()).is_ok()); assert!(hmac::verify(&key, HELLO_WORLD_BAD, tag.as_ref()).is_err()) diff --git a/src/pbkdf2.rs b/src/pbkdf2.rs index 078059f58..4f957f453 100644 --- a/src/pbkdf2.rs +++ b/src/pbkdf2.rs @@ -33,7 +33,7 @@ //! use ring::{digest, pbkdf2}; //! use std::{collections::HashMap, num::NonZeroU32}; //! -//! static DIGEST_ALG: &digest::Algorithm = &digest::SHA256; +//! static PBKDF2_ALG: pbkdf2::Algorithm = pbkdf2::PBKDF2_HMAC_SHA256; //! const CREDENTIAL_LEN: usize = digest::SHA256_OUTPUT_LEN; //! pub type Credential = [u8; CREDENTIAL_LEN]; //! @@ -53,7 +53,7 @@ //! pub fn store_password(&mut self, username: &str, password: &str) { //! let salt = self.salt(username); //! let mut to_store: Credential = [0u8; CREDENTIAL_LEN]; -//! pbkdf2::derive(DIGEST_ALG, self.pbkdf2_iterations, &salt, +//! pbkdf2::derive(PBKDF2_ALG, self.pbkdf2_iterations, &salt, //! password.as_bytes(), &mut to_store); //! self.storage.insert(String::from(username), to_store); //! } @@ -63,7 +63,7 @@ //! match self.storage.get(username) { //! Some(actual_password) => { //! let salt = self.salt(username); -//! pbkdf2::verify(DIGEST_ALG, self.pbkdf2_iterations, &salt, +//! pbkdf2::verify(PBKDF2_ALG, self.pbkdf2_iterations, &salt, //! attempted_password.as_bytes(), //! actual_password) //! .map_err(|_| Error::WrongUsernameOrPassword) @@ -115,6 +115,22 @@ use crate::{constant_time, digest, error, hmac, polyfill}; use core::num::NonZeroU32; +/// A PBKDF2 algorithm. +#[derive(Clone, Copy, PartialEq, Eq)] +pub struct Algorithm(hmac::Algorithm); + +/// PBKDF2 using HMAC-SHA1. +pub static PBKDF2_HMAC_SHA1: Algorithm = Algorithm(hmac::HMAC_SHA1_FOR_LEGACY_USE_ONLY); + +/// PBKDF2 using HMAC-h. +pub static PBKDF2_HMAC_SHA256: Algorithm = Algorithm(hmac::HMAC_SHA256); + +/// PBKDF2 using HMAC-SHA384. +pub static PBKDF2_HMAC_SHA384: Algorithm = Algorithm(hmac::HMAC_SHA384); + +/// PBKDF2 using HMAC-SHA512. +pub static PBKDF2_HMAC_SHA512: Algorithm = Algorithm(hmac::HMAC_SHA512); + /// Fills `out` with the key derived using PBKDF2 with the given inputs. /// /// Do not use `derive` as part of verifying a secret; use `verify` instead, to @@ -137,12 +153,13 @@ use core::num::NonZeroU32; /// `derive` panics if `out.len()` is larger than (2**32 - 1) * the digest /// algorithm's output length, per the PBKDF2 specification. pub fn derive( - digest_alg: &'static digest::Algorithm, + algorithm: Algorithm, iterations: NonZeroU32, salt: &[u8], secret: &[u8], out: &mut [u8], ) { + let digest_alg = algorithm.0.digest_algorithm(); let output_len = digest_alg.output_len; // This implementation's performance is asymptotically optimal as described @@ -150,7 +167,7 @@ pub fn derive( // hasn't been optimized to the same extent as fastpbkdf2. In particular, // this implementation is probably doing a lot of unnecessary copying. - let secret = hmac::Key::new(digest_alg, secret); + let secret = hmac::Key::new(algorithm.0, secret); // Clear |out|. polyfill::slice::fill(out, 0); @@ -206,12 +223,14 @@ fn derive_block(secret: &hmac::Key, iterations: NonZeroU32, salt: &[u8], idx: u3 /// `verify` panics if `out.len()` is larger than (2**32 - 1) * the digest /// algorithm's output length, per the PBKDF2 specification. pub fn verify( - digest_alg: &'static digest::Algorithm, + algorithm: Algorithm, iterations: NonZeroU32, salt: &[u8], secret: &[u8], previously_derived: &[u8], ) -> Result<(), error::Unspecified> { + let digest_alg = algorithm.0.digest_algorithm(); + if previously_derived.is_empty() { return Err(error::Unspecified); } @@ -219,7 +238,7 @@ pub fn verify( let mut derived_buf = [0u8; digest::MAX_OUTPUT_LEN]; let output_len = digest_alg.output_len; - let secret = hmac::Key::new(digest_alg, secret); + let secret = hmac::Key::new(algorithm.0, secret); let mut idx: u32 = 0; let mut matches = 1; diff --git a/tests/hmac_tests.rs b/tests/hmac_tests.rs index d3655c9e2..ed8d6f194 100644 --- a/tests/hmac_tests.rs +++ b/tests/hmac_tests.rs @@ -42,14 +42,27 @@ fn hmac_tests() { let mut input = test_case.consume_bytes("Input"); let output = test_case.consume_bytes("Output"); - let digest_alg = match digest_alg { - Some(digest_alg) => digest_alg, - None => { - return Ok(()); - } // Unsupported digest algorithm + let algorithm = { + let digest_alg = match digest_alg { + Some(digest_alg) => digest_alg, + None => { + return Ok(()); + } // Unsupported digest algorithm + }; + if digest_alg == &digest::SHA1 { + hmac::HMAC_SHA1_FOR_LEGACY_USE_ONLY + } else if digest_alg == &digest::SHA256 { + hmac::HMAC_SHA256 + } else if digest_alg == &digest::SHA384 { + hmac::HMAC_SHA384 + } else if digest_alg == &digest::SHA512 { + hmac::HMAC_SHA512 + } else { + unreachable!() + } }; - hmac_test_case_inner(digest_alg, &key_value[..], &input[..], &output[..], true)?; + hmac_test_case_inner(algorithm, &key_value[..], &input[..], &output[..], true)?; // Tamper with the input and check that verification fails. if input.is_empty() { @@ -58,54 +71,50 @@ fn hmac_tests() { input[0] ^= 1; } - hmac_test_case_inner(digest_alg, &key_value[..], &input[..], &output[..], false) + hmac_test_case_inner(algorithm, &key_value[..], &input[..], &output[..], false) }); } fn hmac_test_case_inner( - digest_alg: &'static digest::Algorithm, + algorithm: hmac::Algorithm, key_value: &[u8], input: &[u8], output: &[u8], is_ok: bool, ) -> Result<(), error::Unspecified> { - let do_test = |key| { - // One-shot API. - { - let signature = hmac::sign(&key, input); - assert_eq!(is_ok, signature.as_ref() == output); - assert_eq!(is_ok, hmac::verify(&key, input, output).is_ok()); - } + let key = hmac::Key::new(algorithm, key_value); - // Multi-part API, one single part. - { - let mut s_ctx = hmac::Context::with_key(&key); - s_ctx.update(input); - let signature = s_ctx.sign(); - assert_eq!(is_ok, signature.as_ref() == output); - } + // One-shot API. + { + let signature = hmac::sign(&key, input); + assert_eq!(is_ok, signature.as_ref() == output); + assert_eq!(is_ok, hmac::verify(&key, input, output).is_ok()); + } - // Multi-part API, byte by byte. - { - let mut ctx = hmac::Context::with_key(&key); - for b in input { - ctx.update(&[*b]); - } - let signature = ctx.sign(); - assert_eq!(is_ok, signature.as_ref() == output); - } - }; + // Multi-part API, one single part. + { + let mut s_ctx = hmac::Context::with_key(&key); + s_ctx.update(input); + let signature = s_ctx.sign(); + assert_eq!(is_ok, signature.as_ref() == output); + } - let key = hmac::Key::new(digest_alg, key_value); - do_test(key.clone()); - do_test(key); + // Multi-part API, byte by byte. + { + let mut ctx = hmac::Context::with_key(&key); + for b in input { + ctx.update(&[*b]); + } + let signature = ctx.sign(); + assert_eq!(is_ok, signature.as_ref() == output); + } Ok(()) } #[test] fn hmac_debug() { - let key = hmac::Key::new(&digest::SHA256, &[0; 32]); + let key = hmac::Key::new(hmac::HMAC_SHA256, &[0; 32]); assert_eq!("Key { algorithm: SHA256 }", format!("{:?}", &key)); let ctx = hmac::Context::with_key(&key); diff --git a/tests/pbkdf2_tests.rs b/tests/pbkdf2_tests.rs index e875e86d8..db127bfb9 100644 --- a/tests/pbkdf2_tests.rs +++ b/tests/pbkdf2_tests.rs @@ -31,14 +31,27 @@ warnings )] -use ring::{error, pbkdf2, test, test_file}; +use ring::{digest, error, pbkdf2, test, test_file}; use std::num::NonZeroU32; #[test] pub fn pbkdf2_tests() { test::run(test_file!("pbkdf2_tests.txt"), |section, test_case| { assert_eq!(section, ""); - let digest_alg = &test_case.consume_digest_alg("Hash").unwrap(); + let algorithm = { + let digest_alg = test_case.consume_digest_alg("Hash").unwrap(); + if digest_alg == &digest::SHA1 { + pbkdf2::PBKDF2_HMAC_SHA1 + } else if digest_alg == &digest::SHA256 { + pbkdf2::PBKDF2_HMAC_SHA256 + } else if digest_alg == &digest::SHA384 { + pbkdf2::PBKDF2_HMAC_SHA384 + } else if digest_alg == &digest::SHA512 { + pbkdf2::PBKDF2_HMAC_SHA512 + } else { + unreachable!() + } + }; let iterations = test_case.consume_usize("c"); let iterations = NonZeroU32::new(iterations as u32).unwrap(); let secret = test_case.consume_bytes("P"); @@ -53,12 +66,12 @@ pub fn pbkdf2_tests() { { let mut out = vec![0u8; dk.len()]; - pbkdf2::derive(digest_alg, iterations, &salt, &secret, &mut out); + pbkdf2::derive(algorithm, iterations, &salt, &secret, &mut out); assert_eq!(dk == out, verify_expected_result.is_ok() || dk.is_empty()); } assert_eq!( - pbkdf2::verify(digest_alg, iterations, &salt, &secret, &dk), + pbkdf2::verify(algorithm, iterations, &salt, &secret, &dk), verify_expected_result );