[PROV][KEYMGMT][EC] Import/export of priv_key as padded const time BN
authorNicola Tuveri <nic.tuv@gmail.com>
Tue, 21 Jan 2020 14:48:49 +0000 (16:48 +0200)
committerNicola Tuveri <nic.tuv@gmail.com>
Tue, 18 Feb 2020 17:11:10 +0000 (19:11 +0200)
For EC keys it is particularly important to avoid leaking the bit length
of the secret scalar.

Key import/export should never leak the bit length of the secret
scalar in the key.

For this reason, on export we use padded BIGNUMs with fixed length,
using the new `ossl_param_bld_push_BN_pad()`.

When importing we also should make sure that, even if short lived,
the newly created BIGNUM is marked with the BN_FLG_CONSTTIME flag as
soon as possible, so that any processing of this BIGNUM might opt for
constant time implementations in the backend.

Setting the BN_FLG_CONSTTIME flag alone is never enough, we also have
to preallocate the BIGNUM internal buffer to a fixed size big enough
that operations performed during the processing never trigger a
realloc which would leak the size of the scalar through memory
accesses.

Fixed length
------------

The order of the large prime subgroup of the curve is our choice for
a fixed public size, as that is generally the upper bound for
generating a private key in EC cryptosystems and should fit all valid
secret scalars.

For padding on export we just use the bit length of the order
converted to bytes (rounding up).

For preallocating the BIGNUM storage we look at the number of "words"
required for the internal representation of the order, and we
preallocate 2 extra "words" in case any of the subsequent processing
might temporarily overflow the order length.

Future work
-----------

To ensure the flag and fixed size preallocation persists upon
`EC_KEY_set_private_key()`, we need to further harden
`EC_KEY_set_private_key()` and `BN_copy()`.
This is done in separate commits.

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
Reviewed-by: Shane Lontis <shane.lontis@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/10631)

crypto/ec/ec_ameth.c
providers/implementations/keymgmt/ec_kmgmt.c

index c4e8177..d680766 100644 (file)
@@ -663,20 +663,61 @@ int ec_pkey_export_to(const EVP_PKEY *from, void *to_keydata,
         goto err;
 
     if (priv_key != NULL) {
         goto err;
 
     if (priv_key != NULL) {
+        size_t sz;
+        int ecbits;
+        int ecdh_cofactor_mode;
+
+        /*
+         * Key import/export should never leak the bit length of the secret
+         * scalar in the key.
+         *
+         * For this reason, on export we use padded BIGNUMs with fixed length.
+         *
+         * When importing we also should make sure that, even if short lived,
+         * the newly created BIGNUM is marked with the BN_FLG_CONSTTIME flag as
+         * soon as possible, so that any processing of this BIGNUM might opt for
+         * constant time implementations in the backend.
+         *
+         * Setting the BN_FLG_CONSTTIME flag alone is never enough, we also have
+         * to preallocate the BIGNUM internal buffer to a fixed public size big
+         * enough that operations performed during the processing never trigger
+         * a realloc which would leak the size of the scalar through memory
+         * accesses.
+         *
+         * Fixed Length
+         * ------------
+         *
+         * The order of the large prime subgroup of the curve is our choice for
+         * a fixed public size, as that is generally the upper bound for
+         * generating a private key in EC cryptosystems and should fit all valid
+         * secret scalars.
+         *
+         * For padding on export we just use the bit length of the order
+         * converted to bytes (rounding up).
+         *
+         * For preallocating the BIGNUM storage we look at the number of "words"
+         * required for the internal representation of the order, and we
+         * preallocate 2 extra "words" in case any of the subsequent processing
+         * might temporarily overflow the order length.
+         */
+        ecbits = EC_GROUP_order_bits(ecg);
+        if (ecbits <= 0)
+            goto err;
+
+        sz = (ecbits + 7 ) / 8;
+        if (!ossl_param_bld_push_BN_pad(&tmpl,
+                                        OSSL_PKEY_PARAM_PRIV_KEY,
+                                        priv_key, sz))
+            goto err;
+
         /*
          * The ECDH Cofactor Mode is defined only if the EC_KEY actually
          * contains a private key, so we check for the flag and export it only
          * in this case.
          */
         /*
          * The ECDH Cofactor Mode is defined only if the EC_KEY actually
          * contains a private key, so we check for the flag and export it only
          * in this case.
          */
-        int ecdh_cofactor_mode =
+        ecdh_cofactor_mode =
             (EC_KEY_get_flags(eckey) & EC_FLAG_COFACTOR_ECDH) ? 1 : 0;
 
             (EC_KEY_get_flags(eckey) & EC_FLAG_COFACTOR_ECDH) ? 1 : 0;
 
-        /* Export the actual private key */
-        if (!ossl_param_bld_push_BN(&tmpl,
-                                    OSSL_PKEY_PARAM_PRIV_KEY,
-                                    priv_key))
-            goto err;
-
         /* Export the ECDH_COFACTOR_MODE parameter */
         if (!ossl_param_bld_push_int(&tmpl,
                                      OSSL_PKEY_PARAM_USE_COFACTOR_ECDH,
         /* Export the ECDH_COFACTOR_MODE parameter */
         if (!ossl_param_bld_push_int(&tmpl,
                                      OSSL_PKEY_PARAM_USE_COFACTOR_ECDH,
index 8190747..794dd92 100644 (file)
@@ -1,5 +1,5 @@
 /*
 /*
- * Copyright 2019 The OpenSSL Project Authors. All Rights Reserved.
+ * Copyright 2020 The OpenSSL Project Authors. All Rights Reserved.
  *
  * Licensed under the Apache License 2.0 (the "License").  You may not use
  * this file except in compliance with the License.  You can obtain a copy
  *
  * Licensed under the Apache License 2.0 (the "License").  You may not use
  * this file except in compliance with the License.  You can obtain a copy
@@ -19,6 +19,7 @@
 #include <openssl/ec.h>
 #include <openssl/objects.h>
 #include <openssl/params.h>
 #include <openssl/ec.h>
 #include <openssl/objects.h>
 #include <openssl/params.h>
+#include "crypto/bn.h"
 #include "internal/param_build.h"
 #include "prov/implementations.h"
 #include "prov/providercommon.h"
 #include "internal/param_build.h"
 #include "prov/implementations.h"
 #include "prov/providercommon.h"
@@ -177,9 +178,58 @@ int params_to_key(EC_KEY *ec, const OSSL_PARAM params[], int include_private)
                                    pub_key, pub_key_len, NULL))
         goto err;
 
                                    pub_key, pub_key_len, NULL))
         goto err;
 
-    if (param_priv_key != NULL && include_private
-            && !OSSL_PARAM_get_BN(param_priv_key, &priv_key))
-        goto err;
+    if (param_priv_key != NULL && include_private) {
+        int fixed_top;
+        const BIGNUM *order;
+
+        /*
+         * Key import/export should never leak the bit length of the secret
+         * scalar in the key.
+         *
+         * For this reason, on export we use padded BIGNUMs with fixed length.
+         *
+         * When importing we also should make sure that, even if short lived,
+         * the newly created BIGNUM is marked with the BN_FLG_CONSTTIME flag as
+         * soon as possible, so that any processing of this BIGNUM might opt for
+         * constant time implementations in the backend.
+         *
+         * Setting the BN_FLG_CONSTTIME flag alone is never enough, we also have
+         * to preallocate the BIGNUM internal buffer to a fixed public size big
+         * enough that operations performed during the processing never trigger
+         * a realloc which would leak the size of the scalar through memory
+         * accesses.
+         *
+         * Fixed Length
+         * ------------
+         *
+         * The order of the large prime subgroup of the curve is our choice for
+         * a fixed public size, as that is generally the upper bound for
+         * generating a private key in EC cryptosystems and should fit all valid
+         * secret scalars.
+         *
+         * For padding on export we just use the bit length of the order
+         * converted to bytes (rounding up).
+         *
+         * For preallocating the BIGNUM storage we look at the number of "words"
+         * required for the internal representation of the order, and we
+         * preallocate 2 extra "words" in case any of the subsequent processing
+         * might temporarily overflow the order length.
+         */
+        order = EC_GROUP_get0_order(ecg);
+        if (order == NULL || BN_is_zero(order))
+            goto err;
+
+        fixed_top = bn_get_top(order) + 2;
+
+        if ((priv_key = BN_new()) == NULL)
+            goto err;
+        if (bn_wexpand(priv_key, fixed_top) == NULL)
+            goto err;
+        BN_set_flags(priv_key, BN_FLG_CONSTTIME);
+
+        if (!OSSL_PARAM_get_BN(param_priv_key, &priv_key))
+            goto err;
+    }
 
     if (priv_key != NULL
             && !EC_KEY_set_private_key(ec, priv_key))
 
     if (priv_key != NULL
             && !EC_KEY_set_private_key(ec, priv_key))
@@ -234,11 +284,52 @@ int key_to_params(const EC_KEY *eckey, OSSL_PARAM_BLD *tmpl, int include_private
                                           pub_key, pub_key_len))
         goto err;
 
                                           pub_key, pub_key_len))
         goto err;
 
-    if (priv_key != NULL && include_private
-            && !ossl_param_bld_push_BN(tmpl,
-                                       OSSL_PKEY_PARAM_PRIV_KEY,
-                                       priv_key))
-        goto err;
+    if (priv_key != NULL && include_private) {
+        size_t sz;
+        int ecbits;
+
+        /*
+         * Key import/export should never leak the bit length of the secret
+         * scalar in the key.
+         *
+         * For this reason, on export we use padded BIGNUMs with fixed length.
+         *
+         * When importing we also should make sure that, even if short lived,
+         * the newly created BIGNUM is marked with the BN_FLG_CONSTTIME flag as
+         * soon as possible, so that any processing of this BIGNUM might opt for
+         * constant time implementations in the backend.
+         *
+         * Setting the BN_FLG_CONSTTIME flag alone is never enough, we also have
+         * to preallocate the BIGNUM internal buffer to a fixed public size big
+         * enough that operations performed during the processing never trigger
+         * a realloc which would leak the size of the scalar through memory
+         * accesses.
+         *
+         * Fixed Length
+         * ------------
+         *
+         * The order of the large prime subgroup of the curve is our choice for
+         * a fixed public size, as that is generally the upper bound for
+         * generating a private key in EC cryptosystems and should fit all valid
+         * secret scalars.
+         *
+         * For padding on export we just use the bit length of the order
+         * converted to bytes (rounding up).
+         *
+         * For preallocating the BIGNUM storage we look at the number of "words"
+         * required for the internal representation of the order, and we
+         * preallocate 2 extra "words" in case any of the subsequent processing
+         * might temporarily overflow the order length.
+         */
+        ecbits = EC_GROUP_order_bits(ecg);
+        if (ecbits <= 0)
+            goto err;
+        sz = (ecbits + 7 ) / 8;
+        if (!ossl_param_bld_push_BN_pad(tmpl,
+                                        OSSL_PKEY_PARAM_PRIV_KEY,
+                                        priv_key, sz))
+            goto err;
+    }
 
     ret = 1;
 
 
     ret = 1;