Store EC_KEY's private key as an EC_SCALAR.

This isn't strictly necessary now that BIGNUMs are safe, but we get to
rely on type-system annotations from EC_SCALAR. Additionally,
EC_POINT_mul depends on BN_div, while the EC_SCALAR version does not.

Change-Id: I75e6967f3d35aef17278b94862f4e506baff5c23
Reviewed-on: https://boringssl-review.googlesource.com/26424
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
This commit is contained in:
David Benjamin 2018-03-06 20:21:28 -05:00 committed by CQ bot account: commit-bot@chromium.org
parent d62fe6f3e8
commit a6bfc45b62
7 changed files with 92 additions and 93 deletions

View File

@ -86,6 +86,7 @@ EC_KEY *EC_KEY_parse_private_key(CBS *cbs, const EC_GROUP *group) {
// Parse the optional parameters field.
EC_GROUP *inner_group = NULL;
EC_KEY *ret = NULL;
BIGNUM *priv_key = NULL;
if (CBS_peek_asn1_tag(&ec_private_key, kParametersTag)) {
// Per SEC 1, as an alternative to omitting it, one is allowed to specify
// this field and put in a NULL to mean inheriting this value. This was
@ -126,15 +127,10 @@ EC_KEY *EC_KEY_parse_private_key(CBS *cbs, const EC_GROUP *group) {
// Although RFC 5915 specifies the length of the key, OpenSSL historically
// got this wrong, so accept any length. See upstream's
// 30cd4ff294252c4b6a4b69cbef6a5b4117705d22.
ret->priv_key =
BN_bin2bn(CBS_data(&private_key), CBS_len(&private_key), NULL);
priv_key = BN_bin2bn(CBS_data(&private_key), CBS_len(&private_key), NULL);
ret->pub_key = EC_POINT_new(group);
if (ret->priv_key == NULL || ret->pub_key == NULL) {
goto err;
}
if (BN_cmp(ret->priv_key, EC_GROUP_get0_order(group)) >= 0) {
OPENSSL_PUT_ERROR(EC, EC_R_WRONG_ORDER);
if (priv_key == NULL || ret->pub_key == NULL ||
!EC_KEY_set_private_key(ret, priv_key)) {
goto err;
}
@ -163,7 +159,8 @@ EC_KEY *EC_KEY_parse_private_key(CBS *cbs, const EC_GROUP *group) {
(point_conversion_form_t)(CBS_data(&public_key)[0] & ~0x01);
} else {
// Compute the public key instead.
if (!EC_POINT_mul(group, ret->pub_key, ret->priv_key, NULL, NULL, NULL)) {
if (!ec_point_mul_scalar(group, ret->pub_key, &ret->priv_key->scalar, NULL,
NULL, NULL)) {
goto err;
}
// Remember the original private-key-only encoding.
@ -181,11 +178,13 @@ EC_KEY *EC_KEY_parse_private_key(CBS *cbs, const EC_GROUP *group) {
goto err;
}
BN_free(priv_key);
EC_GROUP_free(inner_group);
return ret;
err:
EC_KEY_free(ret);
BN_free(priv_key);
EC_GROUP_free(inner_group);
return NULL;
}
@ -203,7 +202,7 @@ int EC_KEY_marshal_private_key(CBB *cbb, const EC_KEY *key,
!CBB_add_asn1(&ec_private_key, &private_key, CBS_ASN1_OCTETSTRING) ||
!BN_bn2cbb_padded(&private_key,
BN_num_bytes(EC_GROUP_get0_order(key->group)),
key->priv_key)) {
EC_KEY_get0_private_key(key))) {
OPENSSL_PUT_ERROR(EC, EC_R_ENCODE_ERROR);
return 0;
}

View File

@ -74,6 +74,7 @@
#include <openssl/err.h>
#include <openssl/mem.h>
#include "../fipsmodule/ec/internal.h"
#include "../internal.h"
@ -81,11 +82,11 @@ int ECDH_compute_key(void *out, size_t outlen, const EC_POINT *pub_key,
const EC_KEY *priv_key,
void *(*kdf)(const void *in, size_t inlen, void *out,
size_t *outlen)) {
const BIGNUM *const priv = EC_KEY_get0_private_key(priv_key);
if (priv == NULL) {
if (priv_key->priv_key == NULL) {
OPENSSL_PUT_ERROR(ECDH, ECDH_R_NO_PRIVATE_VALUE);
return -1;
}
const EC_SCALAR *const priv = &priv_key->priv_key->scalar;
BN_CTX *ctx = BN_CTX_new();
if (ctx == NULL) {
@ -104,7 +105,7 @@ int ECDH_compute_key(void *out, size_t outlen, const EC_POINT *pub_key,
goto err;
}
if (!EC_POINT_mul(group, tmp, NULL, pub_key, priv, ctx)) {
if (!ec_point_mul_scalar(group, tmp, NULL, pub_key, priv, ctx)) {
OPENSSL_PUT_ERROR(ECDH, ECDH_R_POINT_ARITHMETIC_FAILURE);
goto err;
}

View File

@ -120,8 +120,6 @@
#include "../rand/internal.h"
static const uint8_t kDefaultAdditionalData[32] = {0};
int BN_rand(BIGNUM *rnd, int bits, int top, int bottom) {
uint8_t *buf = NULL;
int ret = 0, bit, bytes, mask;
@ -278,6 +276,7 @@ int bn_rand_range_words(BN_ULONG *out, BN_ULONG min_inclusive,
int BN_rand_range_ex(BIGNUM *r, BN_ULONG min_inclusive,
const BIGNUM *max_exclusive) {
static const uint8_t kDefaultAdditionalData[32] = {0};
if (!bn_wexpand(r, max_exclusive->width) ||
!bn_rand_range_words(r->d, min_inclusive, max_exclusive->d,
max_exclusive->width, kDefaultAdditionalData)) {

View File

@ -84,6 +84,25 @@
DEFINE_STATIC_EX_DATA_CLASS(g_ec_ex_data_class);
static EC_WRAPPED_SCALAR *ec_wrapped_scalar_new(const EC_GROUP *group) {
EC_WRAPPED_SCALAR *wrapped = OPENSSL_malloc(sizeof(EC_WRAPPED_SCALAR));
if (wrapped == NULL) {
OPENSSL_PUT_ERROR(EC, ERR_R_MALLOC_FAILURE);
return NULL;
}
OPENSSL_memset(wrapped, 0, sizeof(EC_WRAPPED_SCALAR));
wrapped->bignum.d = wrapped->scalar.words;
wrapped->bignum.width = group->order.width;
wrapped->bignum.dmax = group->order.width;
wrapped->bignum.flags = BN_FLG_STATIC_DATA;
return wrapped;
}
static void ec_wrapped_scalar_free(EC_WRAPPED_SCALAR *scalar) {
OPENSSL_free(scalar);
}
EC_KEY *EC_KEY_new(void) { return EC_KEY_new_method(NULL); }
EC_KEY *EC_KEY_new_method(const ENGINE *engine) {
@ -151,7 +170,7 @@ void EC_KEY_free(EC_KEY *r) {
EC_GROUP_free(r->group);
EC_POINT_free(r->pub_key);
BN_clear_free(r->priv_key);
ec_wrapped_scalar_free(r->priv_key);
BN_free(r->fixed_k);
CRYPTO_free_ex_data(g_ec_ex_data_class_bss_get(), r, &r->ex_data);
@ -175,7 +194,7 @@ EC_KEY *EC_KEY_dup(const EC_KEY *src) {
(src->pub_key != NULL &&
!EC_KEY_set_public_key(ret, src->pub_key)) ||
(src->priv_key != NULL &&
!EC_KEY_set_private_key(ret, src->priv_key))) {
!EC_KEY_set_private_key(ret, EC_KEY_get0_private_key(src)))) {
EC_KEY_free(ret);
return NULL;
}
@ -215,7 +234,7 @@ int EC_KEY_set_group(EC_KEY *key, const EC_GROUP *group) {
}
const BIGNUM *EC_KEY_get0_private_key(const EC_KEY *key) {
return key->priv_key;
return key->priv_key != NULL ? &key->priv_key->bignum : NULL;
}
int EC_KEY_set_private_key(EC_KEY *key, const BIGNUM *priv_key) {
@ -224,14 +243,18 @@ int EC_KEY_set_private_key(EC_KEY *key, const BIGNUM *priv_key) {
return 0;
}
if (BN_is_negative(priv_key) ||
BN_cmp(priv_key, EC_GROUP_get0_order(key->group)) >= 0) {
OPENSSL_PUT_ERROR(EC, EC_R_WRONG_ORDER);
EC_WRAPPED_SCALAR *scalar = ec_wrapped_scalar_new(key->group);
if (scalar == NULL) {
return 0;
}
BN_clear_free(key->priv_key);
key->priv_key = BN_dup(priv_key);
return (key->priv_key == NULL) ? 0 : 1;
if (!ec_bignum_to_scalar(key->group, &scalar->scalar, priv_key)) {
OPENSSL_PUT_ERROR(EC, EC_R_WRONG_ORDER);
ec_wrapped_scalar_free(scalar);
return 0;
}
ec_wrapped_scalar_free(key->priv_key);
key->priv_key = scalar;
return 1;
}
const EC_POINT *EC_KEY_get0_public_key(const EC_KEY *key) {
@ -296,15 +319,11 @@ int EC_KEY_check_key(const EC_KEY *eckey) {
}
// in case the priv_key is present :
// check if generator * priv_key == pub_key
if (eckey->priv_key) {
if (BN_is_negative(eckey->priv_key) ||
BN_cmp(eckey->priv_key, EC_GROUP_get0_order(eckey->group)) >= 0) {
OPENSSL_PUT_ERROR(EC, EC_R_WRONG_ORDER);
goto err;
}
if (eckey->priv_key != NULL) {
point = EC_POINT_new(eckey->group);
if (point == NULL ||
!EC_POINT_mul(eckey->group, point, eckey->priv_key, NULL, NULL, ctx)) {
!ec_point_mul_scalar(eckey->group, point, &eckey->priv_key->scalar,
NULL, NULL, ctx)) {
OPENSSL_PUT_ERROR(EC, ERR_R_EC_LIB);
goto err;
}
@ -375,65 +394,37 @@ err:
return ok;
}
int EC_KEY_generate_key(EC_KEY *eckey) {
int ok = 0;
BIGNUM *priv_key = NULL;
EC_POINT *pub_key = NULL;
if (!eckey || !eckey->group) {
int EC_KEY_generate_key(EC_KEY *key) {
if (key == NULL || key->group == NULL) {
OPENSSL_PUT_ERROR(EC, ERR_R_PASSED_NULL_PARAMETER);
return 0;
}
if (eckey->priv_key == NULL) {
priv_key = BN_new();
if (priv_key == NULL) {
goto err;
}
} else {
priv_key = eckey->priv_key;
}
const BIGNUM *order = EC_GROUP_get0_order(eckey->group);
// Check that the size of the group order is FIPS compliant (FIPS 186-4
// B.4.2).
if (BN_num_bits(order) < 160) {
// Check that the group order is FIPS compliant (FIPS 186-4 B.4.2).
if (BN_num_bits(EC_GROUP_get0_order(key->group)) < 160) {
OPENSSL_PUT_ERROR(EC, EC_R_INVALID_GROUP_ORDER);
goto err;
return 0;
}
// Generate the private key by testing candidates (FIPS 186-4 B.4.2).
if (!BN_rand_range_ex(priv_key, 1, order)) {
goto err;
}
if (eckey->pub_key == NULL) {
pub_key = EC_POINT_new(eckey->group);
if (pub_key == NULL) {
goto err;
}
} else {
pub_key = eckey->pub_key;
}
if (!EC_POINT_mul(eckey->group, pub_key, priv_key, NULL, NULL, NULL)) {
goto err;
}
eckey->priv_key = priv_key;
eckey->pub_key = pub_key;
ok = 1;
err:
if (eckey->pub_key == NULL) {
static const uint8_t kDefaultAdditionalData[32] = {0};
EC_WRAPPED_SCALAR *priv_key = ec_wrapped_scalar_new(key->group);
EC_POINT *pub_key = EC_POINT_new(key->group);
if (priv_key == NULL || pub_key == NULL ||
// Generate the private key by testing candidates (FIPS 186-4 B.4.2).
!ec_random_nonzero_scalar(key->group, &priv_key->scalar,
kDefaultAdditionalData) ||
!ec_point_mul_scalar(key->group, pub_key, &priv_key->scalar, NULL, NULL,
NULL)) {
EC_POINT_free(pub_key);
ec_wrapped_scalar_free(priv_key);
return 0;
}
if (eckey->priv_key == NULL) {
BN_free(priv_key);
}
return ok;
ec_wrapped_scalar_free(key->priv_key);
key->priv_key = priv_key;
EC_POINT_free(key->pub_key);
key->pub_key = pub_key;
return 1;
}
int EC_KEY_generate_key_fips(EC_KEY *eckey) {

View File

@ -359,6 +359,14 @@ TEST(ECTest, GroupMismatch) {
EC_KEY_set_public_key(key.get(), EC_GROUP_get0_generator(p256.get())));
}
TEST(ECTest, EmptyKey) {
bssl::UniquePtr<EC_KEY> key(EC_KEY_new());
ASSERT_TRUE(key);
EXPECT_FALSE(EC_KEY_get0_group(key.get()));
EXPECT_FALSE(EC_KEY_get0_public_key(key.get()));
EXPECT_FALSE(EC_KEY_get0_private_key(key.get()));
}
class ECCurveTest : public testing::TestWithParam<EC_builtin_curve> {};
TEST_P(ECCurveTest, SetAffine) {

View File

@ -277,11 +277,18 @@ const EC_METHOD *EC_GFp_nistp256_method(void);
// x86-64 optimized P256. See http://eprint.iacr.org/2013/816.
const EC_METHOD *EC_GFp_nistz256_method(void);
// An EC_WRAPPED_SCALAR is an |EC_SCALAR| with a parallel |BIGNUM|
// representation. It exists to support the |EC_KEY_get0_private_key| API.
typedef struct {
BIGNUM bignum;
EC_SCALAR scalar;
} EC_WRAPPED_SCALAR;
struct ec_key_st {
EC_GROUP *group;
EC_POINT *pub_key;
BIGNUM *priv_key;
EC_WRAPPED_SCALAR *priv_key;
// fixed_k may contain a specific value of 'k', to be used in ECDSA signing.
// This is only for the FIPS power-on tests.

View File

@ -392,17 +392,17 @@ ECDSA_SIG *ECDSA_do_sign(const uint8_t *digest, size_t digest_len,
}
const EC_GROUP *group = EC_KEY_get0_group(eckey);
const BIGNUM *priv_key_bn = EC_KEY_get0_private_key(eckey);
if (group == NULL || priv_key_bn == NULL) {
if (group == NULL || eckey->priv_key == NULL) {
OPENSSL_PUT_ERROR(ECDSA, ERR_R_PASSED_NULL_PARAMETER);
return NULL;
}
const BIGNUM *order = EC_GROUP_get0_order(group);
const EC_SCALAR *priv_key = &eckey->priv_key->scalar;
int ok = 0;
ECDSA_SIG *ret = ECDSA_SIG_new();
BN_CTX *ctx = BN_CTX_new();
EC_SCALAR kinv_mont, priv_key, r_mont, s;
EC_SCALAR kinv_mont, r_mont, s;
EC_LOOSE_SCALAR m, tmp;
if (ret == NULL || ctx == NULL) {
OPENSSL_PUT_ERROR(ECDSA, ERR_R_MALLOC_FAILURE);
@ -410,14 +410,9 @@ ECDSA_SIG *ECDSA_do_sign(const uint8_t *digest, size_t digest_len,
}
digest_to_scalar(group, &m, digest, digest_len);
// TODO(davidben): Store the private key as an |EC_SCALAR| so this is obvious
// via the type system.
if (!ec_bignum_to_scalar_unchecked(group, &priv_key, priv_key_bn)) {
goto err;
}
for (;;) {
if (!ecdsa_sign_setup(eckey, ctx, &kinv_mont, &ret->r, digest, digest_len,
&priv_key)) {
priv_key)) {
goto err;
}
@ -427,7 +422,7 @@ ECDSA_SIG *ECDSA_do_sign(const uint8_t *digest, size_t digest_len,
if (!ec_bignum_to_scalar(group, &r_mont, ret->r) ||
!bn_to_montgomery_small(r_mont.words, order->width, r_mont.words,
order->width, group->order_mont) ||
!scalar_mod_mul_montgomery(group, &s, &priv_key, &r_mont)) {
!scalar_mod_mul_montgomery(group, &s, priv_key, &r_mont)) {
goto err;
}
@ -455,7 +450,6 @@ err:
}
BN_CTX_free(ctx);
OPENSSL_cleanse(&kinv_mont, sizeof(kinv_mont));
OPENSSL_cleanse(&priv_key, sizeof(priv_key));
OPENSSL_cleanse(&r_mont, sizeof(r_mont));
OPENSSL_cleanse(&s, sizeof(s));
OPENSSL_cleanse(&tmp, sizeof(tmp));