Enforce that |EC_KEY| private key is in [0, group->order). Change-Id: I16abea5769737c7edd1be717f9a4f38678af43ce Reviewed-on: https://boringssl-review.googlesource.com/6564 Reviewed-by: Adam Langley <agl@google.com>
diff --git a/crypto/ec/ec_asn1.c b/crypto/ec/ec_asn1.c index 7c4be07..a085be5 100644 --- a/crypto/ec/ec_asn1.c +++ b/crypto/ec/ec_asn1.c
@@ -329,6 +329,11 @@ goto err; } + if (BN_cmp(ret->priv_key, EC_GROUP_get0_order(ret->group)) >= 0) { + OPENSSL_PUT_ERROR(EC, EC_R_WRONG_ORDER); + goto err; + } + EC_POINT_free(ret->pub_key); ret->pub_key = EC_POINT_new(ret->group); if (ret->pub_key == NULL) {
diff --git a/crypto/ec/ec_key.c b/crypto/ec/ec_key.c index 344ebb2..0a80366 100644 --- a/crypto/ec/ec_key.c +++ b/crypto/ec/ec_key.c
@@ -249,7 +249,15 @@ /* TODO(fork): duplicating the group seems wasteful but see * |EC_KEY_set_conv_form|. */ key->group = EC_GROUP_dup(group); - return (key->group == NULL) ? 0 : 1; + if (key->group == NULL) { + return 0; + } + /* XXX: |BN_cmp| is not constant time. */ + if (key->priv_key != NULL && + BN_cmp(key->priv_key, EC_GROUP_get0_order(group)) >= 0) { + return 0; + } + return 1; } const BIGNUM *EC_KEY_get0_private_key(const EC_KEY *key) { @@ -257,6 +265,12 @@ } int EC_KEY_set_private_key(EC_KEY *key, const BIGNUM *priv_key) { + /* XXX: |BN_cmp| is not constant time. */ + if (key->group != NULL && + BN_cmp(priv_key, EC_GROUP_get0_order(key->group)) >= 0) { + OPENSSL_PUT_ERROR(EC, EC_R_WRONG_ORDER); + return 0; + } BN_clear_free(key->priv_key); key->priv_key = BN_dup(priv_key); return (key->priv_key == NULL) ? 0 : 1; @@ -324,6 +338,7 @@ * check if generator * priv_key == pub_key */ if (eckey->priv_key) { + /* XXX: |BN_cmp| is not constant time. */ if (BN_cmp(eckey->priv_key, EC_GROUP_get0_order(eckey->group)) >= 0) { OPENSSL_PUT_ERROR(EC, EC_R_WRONG_ORDER); goto err;