Skip to content

Commit

Permalink
Validate range of ECDSA signature (r, s) values in Rust.
Browse files Browse the repository at this point in the history
  • Loading branch information
@briansmith
briansmith committed Jun 25, 2016
1 parent 29e8d5c commit 97bb46a
Show file tree
Hide file tree
Showing 6 changed files with 252 additions and 79 deletions.
26 changes: 13 additions & 13 deletions crypto/ecdsa/ecdsa.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,15 +58,15 @@
#include <openssl/bn.h>
#include <openssl/err.h>

#include "../bn/internal.h"
#include "../ec/gfp_internal.h"
#include "../ec/internal.h"


/* Declarations to suppress -Wmissing-prototypes warnings. */
int ECDSA_verify_signed_digest(const EC_GROUP *group, int hash_nid,
const uint8_t *digest, size_t digest_len,
const uint8_t *sig_r, size_t sig_r_len,
const uint8_t *sig_s, size_t sig_s_len,
const GFp_Limb *sig_r, const GFp_Limb *sig_s,
const GFp_Limb *peer_public_key_x,
const GFp_Limb *peer_public_key_y);

Expand All @@ -79,13 +79,12 @@ static int digest_to_bn(BIGNUM *out, const uint8_t *digest, size_t digest_len,
* constitute a valid signature of |digest| for the public key |ec_key| for
* the curve represented by the |EC_GROUP| created by |ec_group_new|.
* |hash_nid| must be the identifier of the digest function used to calculate
* |digest|. The caller must ensure that |sig_r| and |sig_s| are encodings of
* *positive* integers. It returns one on success or zero if the signature is
* invalid or on error. */
* |digest|. The caller must ensure that |sig_r| and |sig_s| are in the range
* [1, n). It returns one on success or zero if the signature is invalid or on
* error. */
int ECDSA_verify_signed_digest(const EC_GROUP *group, int hash_nid,
const uint8_t *digest, size_t digest_len,
const uint8_t *sig_r, size_t sig_r_len,
const uint8_t *sig_s, size_t sig_s_len,
const GFp_Limb *sig_r, const GFp_Limb *sig_s,
const GFp_Limb *peer_public_key_x,
const GFp_Limb *peer_public_key_y) {
(void)hash_nid; /* TODO: Verify |digest_len| is right for |hash_nid|. */
Expand Down Expand Up @@ -121,20 +120,21 @@ int ECDSA_verify_signed_digest(const EC_GROUP *group, int hash_nid,
goto err;
}

if (BN_bin2bn(sig_r, sig_r_len, r) == NULL ||
BN_bin2bn(sig_s, sig_s_len, s) == NULL ||
BN_ucmp(r, &group->order) >= 0 ||
BN_ucmp(s, &group->order) >= 0) {
OPENSSL_PUT_ERROR(ECDSA, ECDSA_R_BAD_SIGNATURE);
ret = 0; /* signature is invalid */
size_t scalar_limbs =
(EC_GROUP_get_degree(group) + (BN_BITS2 - 1)) / BN_BITS2;
if (!bn_set_words(r, sig_r, scalar_limbs) ||
!bn_set_words(s, sig_s, scalar_limbs)) {
OPENSSL_PUT_ERROR(ECDSA, ERR_R_BN_LIB);
goto err;
}

/* These properties are guaranteed by the caller. */
assert(!BN_is_negative(r));
assert(!BN_is_zero(r));
assert(BN_ucmp(r, &group->order) < 0);
assert(!BN_is_negative(s));
assert(!BN_is_zero(s));
assert(BN_ucmp(s, &group->order) < 0);

/* calculate tmp1 = inv(S) mod order */
if (!BN_mod_inverse(u2, s, &group->order, ctx)) {
Expand Down
60 changes: 25 additions & 35 deletions src/ec/suite_b/ecdsa.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,38 +22,40 @@ use super::public_key::*;
use untrusted;

struct ECDSA {
ops: &'static PublicKeyOps,
ops: &'static PublicScalarOps,
digest_alg: &'static digest::Algorithm,
}

#[cfg(feature = "use_heap")]
impl signature_impl::VerificationAlgorithmImpl for ECDSA {
fn verify(&self, public_key: untrusted::Input, msg: untrusted::Input,
signature: untrusted::Input) -> Result<(), ()> {
let digest = digest::digest(self.digest_alg, msg.as_slice_less_safe());

let (r, s) = try!(signature.read_all((), |input| {
der::nested(input, der::Tag::Sequence, (), |input| {
let r = try!(der::positive_integer(input));
let s = try!(der::positive_integer(input));
Ok((r.as_slice_less_safe(), s.as_slice_less_safe()))
let r = try!(self.ops.scalar_parse(input));
let s = try!(self.ops.scalar_parse(input));
Ok((r, s))
})
}));

let (x, y) = try!(parse_uncompressed_point(self.ops, public_key));
let (x, y) =
try!(parse_uncompressed_point(self.ops.public_key_ops, public_key));

bssl::map_result(unsafe {
ECDSA_verify_signed_digest(self.ops.common.ec_group,
ECDSA_verify_signed_digest(self.ops.public_key_ops.common.ec_group,
digest.algorithm().nid,
digest.as_ref().as_ptr(),
digest.as_ref().len(), r.as_ptr(),
r.len(), s.as_ptr(), s.len(),
x.limbs_as_ptr(), y.limbs_as_ptr())
digest.as_ref().len(), r.limbs_as_ptr(),
s.limbs_as_ptr(), x.limbs_as_ptr(),
y.limbs_as_ptr())
})
}
}

macro_rules! ecdsa {
( $VERIFY_ALGORITHM:ident, $curve_name:expr, $public_key_ops:expr,
( $VERIFY_ALGORITHM:ident, $curve_name:expr, $ecdsa_verify_ops:expr,
$digest_alg_name:expr, $digest_alg:expr ) => {
#[doc="Verification of ECDSA signatures using the "]
#[doc=$curve_name]
Expand Down Expand Up @@ -81,45 +83,45 @@ macro_rules! ecdsa {
pub static $VERIFY_ALGORITHM: signature::VerificationAlgorithm =
signature::VerificationAlgorithm {
implementation: &ECDSA {
ops: $public_key_ops,
ops: $ecdsa_verify_ops,
digest_alg: $digest_alg,
}
};
}
}

ecdsa!(ECDSA_P256_SHA1_VERIFY, "P-256 (secp256r1)", &p256::PUBLIC_KEY_OPS,
ecdsa!(ECDSA_P256_SHA1_VERIFY, "P-256 (secp256r1)", &p256::PUBLIC_SCALAR_OPS,
"SHA-1", &digest::SHA1);
ecdsa!(ECDSA_P256_SHA256_VERIFY, "P-256 (secp256r1)", &p256::PUBLIC_KEY_OPS,
ecdsa!(ECDSA_P256_SHA256_VERIFY, "P-256 (secp256r1)", &p256::PUBLIC_SCALAR_OPS,
"SHA-256", &digest::SHA256);
ecdsa!(ECDSA_P256_SHA384_VERIFY, "P-256 (secp256r1)", &p256::PUBLIC_KEY_OPS,
ecdsa!(ECDSA_P256_SHA384_VERIFY, "P-256 (secp256r1)", &p256::PUBLIC_SCALAR_OPS,
"SHA-384", &digest::SHA384);
ecdsa!(ECDSA_P256_SHA512_VERIFY, "P-256 (secp256r1)", &p256::PUBLIC_KEY_OPS,
ecdsa!(ECDSA_P256_SHA512_VERIFY, "P-256 (secp256r1)", &p256::PUBLIC_SCALAR_OPS,
"SHA-512", &digest::SHA512);

ecdsa!(ECDSA_P384_SHA1_VERIFY, "P-384 (secp384r1)", &p384::PUBLIC_KEY_OPS,

ecdsa!(ECDSA_P384_SHA1_VERIFY, "P-384 (secp384r1)", &p384::PUBLIC_SCALAR_OPS,
"SHA-1", &digest::SHA1);
ecdsa!(ECDSA_P384_SHA256_VERIFY, "P-384 (secp384r1)", &p384::PUBLIC_KEY_OPS,
ecdsa!(ECDSA_P384_SHA256_VERIFY, "P-384 (secp384r1)", &p384::PUBLIC_SCALAR_OPS,
"SHA-256", &digest::SHA256);
ecdsa!(ECDSA_P384_SHA384_VERIFY, "P-384 (secp384r1)", &p384::PUBLIC_KEY_OPS,
ecdsa!(ECDSA_P384_SHA384_VERIFY, "P-384 (secp384r1)", &p384::PUBLIC_SCALAR_OPS,
"SHA-384", &digest::SHA384);
ecdsa!(ECDSA_P384_SHA512_VERIFY, "P-384 (secp384r1)", &p384::PUBLIC_KEY_OPS,
ecdsa!(ECDSA_P384_SHA512_VERIFY, "P-384 (secp384r1)", &p384::PUBLIC_SCALAR_OPS,
"SHA-512", &digest::SHA512);


extern {
fn ECDSA_verify_signed_digest(group: &EC_GROUP, hash_nid: c::int,
fn ECDSA_verify_signed_digest(group: *const EC_GROUP, hash_nid: c::int,
digest: *const u8, digest_len: c::size_t,
sig_r: *const u8, sig_r_len: c::size_t,
sig_s: *const u8, sig_s_len: c::size_t,
sig_r: *const Limb, sig_s: *const Limb,
peer_public_key_x: *const Limb,
peer_public_key_y: *const Limb) -> c::int;
}


#[cfg(test)]
mod tests {
use {file_test, der, signature};
use {file_test, signature};
use super::*;
use untrusted;

Expand All @@ -142,18 +144,6 @@ mod tests {
let sig = test_case.consume_bytes("Sig");
let sig = try!(untrusted::Input::new(&sig));

// Sanity check that we correctly DER-encoded the
// originally-provided separate (r, s) components. When we add test
// vectors for improperly-encoded signatures, we'll have to revisit
// this.
try!(sig.read_all((), |input| {
der::nested(input, der::Tag::Sequence, (), |input| {
let _ = try!(der::positive_integer(input));
let _ = try!(der::positive_integer(input));
Ok(())
})
}));

let expected_result = test_case.consume_string("Result");

let actual_result = signature::verify(alg, public_key, msg, sig);
Expand Down
80 changes: 77 additions & 3 deletions src/ec/suite_b/ecdsa_verify_tests.txt
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,15 @@
# Test vectors for Gregory Maxwell's trick.
# Generated Test vectors.
#
# These test vectors were generated by applying the patch in
# The test vectors in this section were generated by applying the patch in
# util/generate-tests.patch to BoringSSL, and then running
# `bssl generate-tests`.
# `bssl generate-tests ecdsa`.
#
# TODO: Test the range of `r` in addition to the range of `s`.
# TODO: Test what happens when the message digests to zero.
# TODO: Additional test coverage. libsecp256k1 is a good example.


# Test vectors for Gregory Maxwell's trick.
#
# In all cases, the `s` component of the signature was selected
# arbitrarily as 4 and then the `r` component was chosen to be the
Expand Down Expand Up @@ -118,6 +125,73 @@ Sig = 301d0218389cb27e0bc8d21fa7e5f24cb74f58851313e696333ad68e020104
Result = F


# Generated Test vectors edge cases of signature (r, s) values.

# s == 0 (out of range)
Curve = P-256
Digest = SHA256
Msg = ""
Q = 0471db746fd153cf5c5a7c7210f9008c0e99c3a936ef0e720b202b304771431a230af53931e70cbe279ca47ce819616ed1db6604490f70abbcef3036732426eb6d
Sig = 3006020106020100
Result = F

# s == 1 (minimum allowed)
Curve = P-256
Digest = SHA256
Msg = ""
Q = 046e3f95fae7606c1cdfab1f1560de160ed806bbc2a85dc5a2d002aa1c0ac3e1fb5bcd5f7a325415824365cc584f08c144118318ce4d0f5df82b7753b291c4fe96
Sig = 3006020106020101
Result = P (0 )

# s == n (out of range
Curve = P-256
Digest = SHA256
Msg = ""
Q = 0471db746fd153cf5c5a7c7210f9008c0e99c3a936ef0e720b202b304771431a230af53931e70cbe279ca47ce819616ed1db6604490f70abbcef3036732426eb6d
Sig = 3026020106022100ffffffff00000000ffffffffffffffffbce6faada7179e84f3b9cac2fc632551
Result = F

# s == n - 1 (maximum allowed)
Curve = P-256
Digest = SHA256
Msg = ""
Q = 04d78f14b53bf825c9f7146193f775458ef5ee46500cd44b18488cb4115c3f00f04b11fc7c6aa1045dc83e4f3e8a14d4a017db8415b5fe3f1a32afba4b8c707ab4
Sig = 3026020106022100ffffffff00000000ffffffffffffffffbce6faada7179e84f3b9cac2fc632550
Result = P (0 )

# s == 0 (out of range)
Curve = P-384
Digest = SHA256
Msg = ""
Q = 0409a0347908afe87a9eeb6e919a2d5e7daee0842da7f6d5493fa5304077c428b0be4f413d54f58eb6f2bee81b233b8b3bd693a99ec039402a88e9b38ffe04ec18b47d0f15f2ce397e4a2fbc472646fc7c00b83ecc268f3fb7393eab47553df327
Sig = 3006020103020100
Result = F

# s == 1 (minimum allowed)
Curve = P-384
Digest = SHA256
Msg = ""
Q = 04724330e317ad06814dec6bd7e31118fe1c3115df7a2507445113f517c17e6b96d1115478802e31049a7b6aebe15f0d976e9428760faf52a1e2042279713fc8d10d168a773a7fc246863a5a2bac619c8c94ed0180cf10c1d9680047bee65f9d0f
Sig = 3006020103020101
Result = P (0 )

# s == n (out of range)
Curve = P-384
Digest = SHA256
Msg = ""
Q = 0409a0347908afe87a9eeb6e919a2d5e7daee0842da7f6d5493fa5304077c428b0be4f413d54f58eb6f2bee81b233b8b3bd693a99ec039402a88e9b38ffe04ec18b47d0f15f2ce397e4a2fbc472646fc7c00b83ecc268f3fb7393eab47553df327
Sig = 3036020103023100ffffffffffffffffffffffffffffffffffffffffffffffffc7634d81f4372ddf581a0db248b0a77aecec196accc52973
Result = F

# s == n - 1 (maximum allowed)
Curve = P-384
Digest = SHA256
Msg = ""
Q = 0484e1c8401dcc79d6071cf7dec48c515d0ff666ba99c401910b64ecf8bedc68bd4e65a673047ba7762deb25c0b966754b86a2268a6d6f3ec5b895354c6e486b6ffdd691aca6b27ab24096a3501205af27a6685a11e49ac4efc28cc52abc7128a9
Sig = 3036020103023100ffffffffffffffffffffffffffffffffffffffffffffffffc7634d81f4372ddf581a0db248b0a77aecec196accc52972
Result = P (0 )


# NIST FIPS 186-4 Test Vectors from the file SigVer.rsp from
# http://csrc.nist.gov/groups/STM/cavp/documents/dss/186-3ecdsatestvectors.zip
# assessible via
Expand Down

0 comments on commit 97bb46a

Please sign in to comment.