EC keygen updates + changed ecdsa_sign to use BN_secure_new

Reviewed-by: Nicola Tuveri <nic.tuv@gmail.com>
Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/8557)

2 files changed, 47 insertions, 16 deletions

diff --git a/crypto/ec/ec_key.c b/crypto/ec/ec_key.c
index 637ba141ab..416c0e08fc 100644
--- a/crypto/ec/ec_key.c
+++ b/crypto/ec/ec_key.c
@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2018 The OpenSSL Project Authors. All Rights Reserved.
+ * Copyright 2002-2019 The OpenSSL Project Authors. All Rights Reserved.
  * Copyright (c) 2002, Oracle and/or its affiliates. All rights reserved
  *
  * Licensed under the Apache License 2.0 (the "License").  You may not use
@@ -195,59 +195,90 @@ int ossl_ec_key_gen(EC_KEY *eckey)
     return eckey->group->meth->keygen(eckey);
 }
 
+/*
+ * ECC Key generation.
+ * See SP800-56AR3 5.6.1.2.2 "Key Pair Generation by Testing Candidates"
+ *
+ * Params:
+ *     eckey An EC key object that contains domain params. The generated keypair
+ *           is stored in this object.
+ * Returns 1 if the keypair was generated or 0 otherwise.
+ */
 int ec_key_simple_generate_key(EC_KEY *eckey)
 {
     int ok = 0;
-    BN_CTX *ctx = NULL;
     BIGNUM *priv_key = NULL;
     const BIGNUM *order = NULL;
     EC_POINT *pub_key = NULL;
-
-    if ((ctx = BN_CTX_new()) == NULL)
-        goto err;
+    const EC_GROUP *group = eckey->group;
 
     if (eckey->priv_key == NULL) {
-        priv_key = BN_new();
+        priv_key = BN_secure_new();
         if (priv_key == NULL)
             goto err;
     } else
         priv_key = eckey->priv_key;
 
-    order = EC_GROUP_get0_order(eckey->group);
+    /*
+     * Steps (1-2): Check domain parameters and security strength.
+     * These steps must be done by the user. This would need to be
+     * stated in the security policy.
+     */
+
+    order = EC_GROUP_get0_order(group);
     if (order == NULL)
         goto err;
 
+    /*
+     * Steps (3-7): priv_key = DRBG_RAND(order_n_bits) (range [1, n-1]).
+     * Although this is slightly different from the standard, it is effectively
+     * equivalent as it gives an unbiased result ranging from 1..n-1. It is also
+     * faster as the standard needs to retry more often. Also doing
+     * 1 + rand[0..n-2] would effect the way that tests feed dummy entropy into
+     * rand so the simpler backward compatible method has been used here.
+     */
     do
         if (!BN_priv_rand_range(priv_key, order))
             goto err;
     while (BN_is_zero(priv_key)) ;
 
     if (eckey->pub_key == NULL) {
-        pub_key = EC_POINT_new(eckey->group);
+        pub_key = EC_POINT_new(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, ctx))
+    /* Step (8) : pub_key = priv_key * G (where G is a point on the curve) */
+    if (!EC_POINT_mul(group, pub_key, priv_key, NULL, NULL, NULL))
         goto err;
 
     eckey->priv_key = priv_key;
     eckey->pub_key = pub_key;
+    priv_key = NULL;
+    pub_key = NULL;
 
     ok = 1;
 
- err:
-    if (eckey->pub_key == NULL)
-        EC_POINT_free(pub_key);
-    if (eckey->priv_key != priv_key)
-        BN_free(priv_key);
-    BN_CTX_free(ctx);
+err:
+    /* Step (9): If there is an error return an invalid keypair. */
+    if (!ok) {
+        BN_clear(eckey->priv_key);
+        if (eckey->pub_key != NULL)
+            EC_POINT_set_to_infinity(group, eckey->pub_key);
+    }
+
+    EC_POINT_free(pub_key);
+    BN_clear_free(priv_key);
     return ok;
 }
 
 int ec_key_simple_generate_public_key(EC_KEY *eckey)
 {
+    /*
+     * See SP800-56AR3 5.6.1.2.2: Step (8)
+     * pub_key = priv_key * G (where G is a point on the curve)
+     */
     return EC_POINT_mul(eckey->group, eckey->pub_key, eckey->priv_key, NULL,
                         NULL, NULL);
 }
diff --git a/crypto/ec/ecdsa_ossl.c b/crypto/ec/ecdsa_ossl.c
index a790e0adea..a488d5c86d 100644
--- a/crypto/ec/ecdsa_ossl.c
+++ b/crypto/ec/ecdsa_ossl.c
@@ -59,7 +59,7 @@ static int ecdsa_sign_setup(EC_KEY *eckey, BN_CTX *ctx_in,
         }
     }
 
-    k = BN_new();               /* this value is later returned in *kinvp */
+    k = BN_secure_new();        /* this value is later returned in *kinvp */
     r = BN_new();               /* this value is later returned in *rp */
     X = BN_new();
     if (k == NULL || r == NULL || X == NULL) {