From f6426cfb136b20d8f35d14781f68cb66bf4335ca Mon Sep 17 00:00:00 2001 From: Tony Arcieri Date: Wed, 17 Jan 2024 04:25:52 +0000 Subject: [PATCH] rfc6979: add K-163 test vector; fix nonaligned use (#781) RFC6979 Appendix A.1. provides a "Detailed Example" which exercises several edge cases in the protocol: - `bits2int` for an input which is not byte-aligned - Rejecting inputs which exceed the modulus This commit adds what was missing from the previous implementation which assumed inputs were always aligned to the size of the digest output: a constant-time right shift by the number of bits by which the modulus is smaller than a byte-aligned value. --- rfc6979/src/ct.rs | 102 ++++++++++++++++++++++++++++++++++++++++++ rfc6979/src/ct_cmp.rs | 81 --------------------------------- rfc6979/src/lib.rs | 78 +++++++++++++++++++------------- 3 files changed, 148 insertions(+), 113 deletions(-) create mode 100644 rfc6979/src/ct.rs delete mode 100644 rfc6979/src/ct_cmp.rs diff --git a/rfc6979/src/ct.rs b/rfc6979/src/ct.rs new file mode 100644 index 00000000..e7c3d398 --- /dev/null +++ b/rfc6979/src/ct.rs @@ -0,0 +1,102 @@ +//! Constant-time helpers. +// TODO(tarcieri): replace this with `crypto-bigint`? + +use subtle::{Choice, ConditionallySelectable, ConstantTimeEq}; + +/// Count the number of leading zeros in constant-time. +#[inline] +pub(crate) fn leading_zeros(n: &[u8]) -> u32 { + n[0].leading_zeros() +} + +/// Constant-time bitwise right shift. +#[inline] +pub(crate) fn rshift(n: &mut [u8], shift: u32) { + debug_assert!(shift < 8); + let mask = (1 << shift) - 1; + let mut carry = 0; + + for byte in n.iter_mut() { + let new_carry = (*byte & mask) << (8 - shift); + *byte = (*byte >> shift) | carry; + carry = new_carry; + } +} + +/// Constant-time test that a given byte slice contains only zeroes. +#[inline] +pub(crate) fn is_zero(n: &[u8]) -> Choice { + let mut ret = Choice::from(1); + + for byte in n { + ret.conditional_assign(&Choice::from(0), byte.ct_ne(&0)); + } + + ret +} + +/// Constant-time less than. +/// +/// Inputs are interpreted as big endian integers. +#[inline] +pub(crate) fn lt(a: &[u8], b: &[u8]) -> Choice { + debug_assert_eq!(a.len(), b.len()); + + let mut borrow = 0; + + // Perform subtraction with borrow a byte-at-a-time, interpreting a + // no-borrow condition as the less-than case + for (&a, &b) in a.iter().zip(b.iter()).rev() { + let c = (b as u16).wrapping_add(borrow >> (u8::BITS - 1)); + borrow = (a as u16).wrapping_sub(c) >> u8::BITS as u8; + } + + !borrow.ct_eq(&0) +} + +#[cfg(test)] +mod tests { + const A: [u8; 4] = [0, 0, 0, 0]; + const B: [u8; 4] = [0, 0, 0, 1]; + const C: [u8; 4] = [0xFF, 0, 0, 0]; + const D: [u8; 4] = [0xFF, 0, 0, 1]; + const E: [u8; 4] = [0xFF, 0xFF, 0xFF, 0xFE]; + const F: [u8; 4] = [0xFF, 0xFF, 0xFF, 0xFF]; + + #[test] + fn ct_is_zero() { + use super::is_zero; + assert_eq!(is_zero(&A).unwrap_u8(), 1); + assert_eq!(is_zero(&B).unwrap_u8(), 0); + } + + #[test] + fn ct_lt() { + use super::lt; + + assert_eq!(lt(&A, &A).unwrap_u8(), 0); + assert_eq!(lt(&B, &B).unwrap_u8(), 0); + assert_eq!(lt(&C, &C).unwrap_u8(), 0); + assert_eq!(lt(&D, &D).unwrap_u8(), 0); + assert_eq!(lt(&E, &E).unwrap_u8(), 0); + assert_eq!(lt(&F, &F).unwrap_u8(), 0); + + assert_eq!(lt(&A, &B).unwrap_u8(), 1); + assert_eq!(lt(&A, &C).unwrap_u8(), 1); + assert_eq!(lt(&B, &A).unwrap_u8(), 0); + assert_eq!(lt(&C, &A).unwrap_u8(), 0); + + assert_eq!(lt(&B, &C).unwrap_u8(), 1); + assert_eq!(lt(&B, &D).unwrap_u8(), 1); + assert_eq!(lt(&C, &B).unwrap_u8(), 0); + assert_eq!(lt(&D, &B).unwrap_u8(), 0); + + assert_eq!(lt(&C, &D).unwrap_u8(), 1); + assert_eq!(lt(&C, &E).unwrap_u8(), 1); + assert_eq!(lt(&D, &C).unwrap_u8(), 0); + assert_eq!(lt(&E, &C).unwrap_u8(), 0); + + assert_eq!(lt(&E, &F).unwrap_u8(), 1); + assert_eq!(lt(&F, &E).unwrap_u8(), 0); + } +} diff --git a/rfc6979/src/ct_cmp.rs b/rfc6979/src/ct_cmp.rs deleted file mode 100644 index 4cca7adc..00000000 --- a/rfc6979/src/ct_cmp.rs +++ /dev/null @@ -1,81 +0,0 @@ -//! Constant-time comparison helpers for [`ByteArray`]. - -use subtle::{Choice, ConditionallySelectable, ConstantTimeEq}; - -/// Constant-time test that a given byte slice contains only zeroes. -#[inline] -pub(crate) fn ct_is_zero(n: &[u8]) -> Choice { - let mut ret = Choice::from(1); - - for byte in n { - ret.conditional_assign(&Choice::from(0), byte.ct_ne(&0)); - } - - ret -} - -/// Constant-time less than. -/// -/// Inputs are interpreted as big endian integers. -#[inline] -pub(crate) fn ct_lt(a: &[u8], b: &[u8]) -> Choice { - debug_assert_eq!(a.len(), b.len()); - - let mut borrow = 0; - - // Perform subtraction with borrow a byte-at-a-time, interpreting a - // no-borrow condition as the less-than case - for (&a, &b) in a.iter().zip(b.iter()).rev() { - let c = (b as u16).wrapping_add(borrow >> (u8::BITS - 1)); - borrow = (a as u16).wrapping_sub(c) >> u8::BITS as u8; - } - - !borrow.ct_eq(&0) -} - -#[cfg(test)] -mod tests { - const A: [u8; 4] = [0, 0, 0, 0]; - const B: [u8; 4] = [0, 0, 0, 1]; - const C: [u8; 4] = [0xFF, 0, 0, 0]; - const D: [u8; 4] = [0xFF, 0, 0, 1]; - const E: [u8; 4] = [0xFF, 0xFF, 0xFF, 0xFE]; - const F: [u8; 4] = [0xFF, 0xFF, 0xFF, 0xFF]; - - #[test] - fn ct_is_zero() { - use super::ct_is_zero; - assert_eq!(ct_is_zero(&A).unwrap_u8(), 1); - assert_eq!(ct_is_zero(&B).unwrap_u8(), 0); - } - - #[test] - fn ct_lt() { - use super::ct_lt; - - assert_eq!(ct_lt(&A, &A).unwrap_u8(), 0); - assert_eq!(ct_lt(&B, &B).unwrap_u8(), 0); - assert_eq!(ct_lt(&C, &C).unwrap_u8(), 0); - assert_eq!(ct_lt(&D, &D).unwrap_u8(), 0); - assert_eq!(ct_lt(&E, &E).unwrap_u8(), 0); - assert_eq!(ct_lt(&F, &F).unwrap_u8(), 0); - - assert_eq!(ct_lt(&A, &B).unwrap_u8(), 1); - assert_eq!(ct_lt(&A, &C).unwrap_u8(), 1); - assert_eq!(ct_lt(&B, &A).unwrap_u8(), 0); - assert_eq!(ct_lt(&C, &A).unwrap_u8(), 0); - - assert_eq!(ct_lt(&B, &C).unwrap_u8(), 1); - assert_eq!(ct_lt(&B, &D).unwrap_u8(), 1); - assert_eq!(ct_lt(&C, &B).unwrap_u8(), 0); - assert_eq!(ct_lt(&D, &B).unwrap_u8(), 0); - - assert_eq!(ct_lt(&C, &D).unwrap_u8(), 1); - assert_eq!(ct_lt(&C, &E).unwrap_u8(), 1); - assert_eq!(ct_lt(&D, &C).unwrap_u8(), 0); - assert_eq!(ct_lt(&E, &C).unwrap_u8(), 0); - - assert_eq!(ct_lt(&E, &F).unwrap_u8(), 1); - assert_eq!(ct_lt(&F, &E).unwrap_u8(), 0); - } -} diff --git a/rfc6979/src/lib.rs b/rfc6979/src/lib.rs index 220c5631..4e718128 100644 --- a/rfc6979/src/lib.rs +++ b/rfc6979/src/lib.rs @@ -37,7 +37,7 @@ //! assert_eq!(k.as_slice(), &RFC6979_EXPECTED_K); //! ``` -mod ct_cmp; +mod ct; pub use hmac::digest::array::typenum::consts; @@ -55,13 +55,13 @@ use hmac::{ /// Accepts the following parameters and inputs: /// /// - `x`: secret key -/// - `n`: field modulus +/// - `q`: field modulus /// - `h`: hash/digest of input message: must be reduced modulo `n` in advance /// - `data`: additional associated data, e.g. CSRNG output used as added entropy #[inline] pub fn generate_k( x: &Array, - n: &Array, + q: &Array, h: &Array, data: &[u8], ) -> Array @@ -69,39 +69,20 @@ where D: Digest + BlockSizeUser + FixedOutput + FixedOutputReset, N: ArraySize, { + let shift = ct::leading_zeros(q); let mut k = Array::default(); - generate_k_mut::(x, n, h, data, &mut k); - k -} - -/// Deterministically generate ephemeral scalar `k` by writing it into the provided output buffer. -/// -/// This is an API which accepts dynamically sized inputs intended for use cases where the sizes -/// are determined at runtime, such as the legacy Digital Signature Algorithm (DSA). -/// -/// Accepts the following parameters and inputs: -/// -/// - `x`: secret key -/// - `n`: field modulus -/// - `h`: hash/digest of input message: must be reduced modulo `n` in advance -/// - `data`: additional associated data, e.g. CSRNG output used as added entropy -#[inline] -pub fn generate_k_mut(x: &[u8], n: &[u8], h: &[u8], data: &[u8], k: &mut [u8]) -where - D: Digest + BlockSizeUser + FixedOutput + FixedOutputReset, -{ - assert_eq!(k.len(), x.len()); - assert_eq!(k.len(), n.len()); - assert_eq!(k.len(), h.len()); - let mut hmac_drbg = HmacDrbg::::new(x, h, data); loop { - hmac_drbg.fill_bytes(k); + hmac_drbg.fill_bytes(&mut k); + + if shift != 0 { + ct::rshift(&mut k, shift); + } - let k_is_zero = ct_cmp::ct_is_zero(k); - if (!k_is_zero & ct_cmp::ct_lt(k, n)).into() { - return; + let k_is_zero = ct::is_zero(&k); + if (!k_is_zero & ct::lt(&k, q)).into() { + return k; } } } @@ -154,12 +135,21 @@ where /// Write the next `HMAC_DRBG` output to the given byte slice. pub fn fill_bytes(&mut self, out: &mut [u8]) { - for out_chunk in out.chunks_mut(self.v.len()) { + let mut out_chunks = out.chunks_exact_mut(self.v.len()); + + for out_chunk in &mut out_chunks { self.k.update(&self.v); self.v = self.k.finalize_reset().into_bytes(); out_chunk.copy_from_slice(&self.v[..out_chunk.len()]); } + let out_remainder = out_chunks.into_remainder(); + if !out_remainder.is_empty() { + self.k.update(&self.v); + self.v = self.k.finalize_reset().into_bytes(); + out_remainder.copy_from_slice(&self.v[..out_remainder.len()]); + } + self.k.update(&self.v); self.k.update(&[0x00]); self.k = @@ -168,3 +158,27 @@ where self.v = self.k.finalize_reset().into_bytes(); } } + +#[cfg(test)] +mod tests { + use crate::{consts::U21, generate_k}; + use hex_literal::hex; + use sha2::Sha256; + + /// "Detailed Example" from RFC6979 Appendix A.1. + /// + /// Example for ECDSA on the curve K-163 described in FIPS 186-4 (also known as + /// "ansix9t163k1" in X9.62), defined over a field GF(2^163) + #[test] + fn k163_sha256() { + let q = hex!("04000000000000000000020108A2E0CC0D99F8A5EF"); + let x = hex!("009A4D6792295A7F730FC3F2B49CBC0F62E862272F"); + + // Note: SHA-256 digest of "sample" with the output run through `bits2octets` transform + let h2 = hex!("01795EDF0D54DB760F156D0DAC04C0322B3A204224"); + + let aad = b""; + let k = generate_k::(&x.into(), &q.into(), &h2.into(), aad); + assert_eq!(k, hex!("023AF4074C90A02B3FE61D286D5C87F425E6BDD81B")); + } +}