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

@@ -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);
+    }
+}

rfc6979/src/ct_cmp.rs

@@ -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);
-    }
-}

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<D, N>(
     x: &Array<u8, N>,
-    n: &Array<u8, N>,
+    q: &Array<u8, N>,
     h: &Array<u8, N>,
     data: &[u8],
 ) -> Array<u8, N>
@@ -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::<D>(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<D>(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::<D>::new(x, h, data);
 
     loop {
-        hmac_drbg.fill_bytes(k);
+        hmac_drbg.fill_bytes(&mut k);
 
-        let k_is_zero = ct_cmp::ct_is_zero(k);
-        if (!k_is_zero & ct_cmp::ct_lt(k, n)).into() {
-            return;
+        if shift != 0 {
+            ct::rshift(&mut k, shift);
+        }
+
+        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::<Sha256, U21>(&x.into(), &q.into(), &h2.into(), aad);
+        assert_eq!(k, hex!("023AF4074C90A02B3FE61D286D5C87F425E6BDD81B"));
+    }
+}