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>
diff --git a/crypto/ec_extra/ec_asn1.c b/crypto/ec_extra/ec_asn1.c
index c125af2..bde6d0b 100644
--- a/crypto/ec_extra/ec_asn1.c
+++ b/crypto/ec_extra/ec_asn1.c
@@ -86,6 +86,7 @@
// 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 @@
// 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 @@
(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 @@
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 @@
!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;
}
diff --git a/crypto/ecdh/ecdh.c b/crypto/ecdh/ecdh.c
index f38de2f..7634ba5 100644
--- a/crypto/ecdh/ecdh.c
+++ b/crypto/ecdh/ecdh.c
@@ -74,6 +74,7 @@
#include <openssl/err.h>
#include <openssl/mem.h>
+#include "../fipsmodule/ec/internal.h"
#include "../internal.h"
@@ -81,11 +82,11 @@
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 @@
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;
}
diff --git a/crypto/fipsmodule/bn/random.c b/crypto/fipsmodule/bn/random.c
index 733520d..c6f9f08 100644
--- a/crypto/fipsmodule/bn/random.c
+++ b/crypto/fipsmodule/bn/random.c
@@ -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_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)) {
diff --git a/crypto/fipsmodule/ec/ec_key.c b/crypto/fipsmodule/ec/ec_key.c
index b56139d..a6d4697 100644
--- a/crypto/fipsmodule/ec/ec_key.c
+++ b/crypto/fipsmodule/ec/ec_key.c
@@ -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 @@
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 @@
(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 @@
}
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 @@
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 @@
}
// 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 @@
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) {
diff --git a/crypto/fipsmodule/ec/ec_test.cc b/crypto/fipsmodule/ec/ec_test.cc
index 923ce1e..5cce361 100644
--- a/crypto/fipsmodule/ec/ec_test.cc
+++ b/crypto/fipsmodule/ec/ec_test.cc
@@ -359,6 +359,14 @@
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) {
diff --git a/crypto/fipsmodule/ec/internal.h b/crypto/fipsmodule/ec/internal.h
index e1a3e71..75fe81c 100644
--- a/crypto/fipsmodule/ec/internal.h
+++ b/crypto/fipsmodule/ec/internal.h
@@ -277,11 +277,18 @@
// 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.
diff --git a/crypto/fipsmodule/ecdsa/ecdsa.c b/crypto/fipsmodule/ecdsa/ecdsa.c
index e010dbd..1d08123 100644
--- a/crypto/fipsmodule/ecdsa/ecdsa.c
+++ b/crypto/fipsmodule/ecdsa/ecdsa.c
@@ -392,17 +392,17 @@
}
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 @@
}
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 @@
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 @@
}
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));
