Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

k256: fix edge case in Scalar::is_high #385

Merged
merged 1 commit into from
Jul 19, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 9 additions & 5 deletions k256/src/arithmetic/scalar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -607,16 +607,20 @@ mod tests {
let high: bool = Scalar::zero().is_high().into();
assert!(!high);

// 1 is not high
let one = 1.to_biguint().unwrap();
let high: bool = Scalar::from(&one).is_high().into();
assert!(!high);

let m = Scalar::modulus_as_biguint();
let m_by_2 = &m >> 1;
let one = 1.to_biguint().unwrap();

// M / 2 - 1 is not high
let high: bool = Scalar::from(&m_by_2 - &one).is_high().into();
// M / 2 is not high
let high: bool = Scalar::from(&m_by_2).is_high().into();
assert!(!high);

// M / 2 is high
let high: bool = Scalar::from(&m_by_2).is_high().into();
// M / 2 + 1 is high
let high: bool = Scalar::from(&m_by_2 + &one).is_high().into();
assert!(high);

// MODULUS - 1 is high
Expand Down
4 changes: 2 additions & 2 deletions k256/src/arithmetic/scalar/scalar_4x64.rs
Original file line number Diff line number Diff line change
Expand Up @@ -240,8 +240,8 @@ impl Scalar4x64 {

/// Is this scalar greater than or equal to n / 2?
pub fn is_high(&self) -> Choice {
let (_, underflow) = sbb_array_with_underflow(&(self.0), &FRAC_MODULUS_2);
!underflow
let (_, underflow) = sbb_array_with_underflow(&FRAC_MODULUS_2, &self.0);
underflow
}

/// Is this scalar equal to 0?
Expand Down
4 changes: 2 additions & 2 deletions k256/src/arithmetic/scalar/scalar_8x32.rs
Original file line number Diff line number Diff line change
Expand Up @@ -282,8 +282,8 @@ impl Scalar8x32 {

/// Is this scalar greater than or equal to n / 2?
pub fn is_high(&self) -> Choice {
let (_, underflow) = sbb_array_with_underflow(&(self.0), &FRAC_MODULUS_2);
!underflow
let (_, underflow) = sbb_array_with_underflow(&FRAC_MODULUS_2, &self.0);
underflow
}

/// Is this scalar equal to 0?
Expand Down
6 changes: 2 additions & 4 deletions k256/src/ecdsa/normalize.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,7 @@ mod tests {
]).unwrap();

let mut sig_normalized = sig_hi;
sig_normalized.normalize_s().unwrap();

assert!(sig_normalized.normalize_s().unwrap());
assert_eq!(sig_lo, sig_normalized);
}

Expand All @@ -63,8 +62,7 @@ mod tests {
]).unwrap();

let mut sig_normalized = sig;
sig_normalized.normalize_s().unwrap();

assert!(!sig_normalized.normalize_s().unwrap());
assert_eq!(sig, sig_normalized);
}
}
16 changes: 16 additions & 0 deletions k256/src/ecdsa/verify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,22 @@ impl FromStr for VerifyingKey {

#[cfg(test)]
mod tests {
use super::VerifyingKey;
use crate::{test_vectors::ecdsa::ECDSA_TEST_VECTORS, Secp256k1};
use ecdsa_core::signature::Verifier;
use hex_literal::hex;

ecdsa_core::new_verification_test!(Secp256k1, ECDSA_TEST_VECTORS);

/// Wycheproof tcId: 304
#[test]
fn malleability_edge_case_valid() {
let verifying_key_bytes = hex!("043a3150798c8af69d1e6e981f3a45402ba1d732f4be8330c5164f49e10ec555b4221bd842bc5e4d97eff37165f60e3998a424d72a450cf95ea477c78287d0343a");
let verifying_key = VerifyingKey::from_sec1_bytes(&verifying_key_bytes).unwrap();

let msg = hex!("313233343030");
let mut sig = Signature::from_der(&hex!("304402207fffffffffffffffffffffffffffffff5d576e7357a4501ddfe92f46681b20a002207fffffffffffffffffffffffffffffff5d576e7357a4501ddfe92f46681b20a0")).unwrap();
assert!(!sig.normalize_s().unwrap());
assert!(verifying_key.verify(&msg, &sig).is_ok());
}
}