From a377871db10afcdfb080c79f3245baf441fe07fc Mon Sep 17 00:00:00 2001 From: Nicola Tuveri Date: Tue, 21 Jan 2020 16:48:49 +0200 Subject: [PATCH] [PROV][KEYMGMT][EC] Import/export of priv_key as padded const time BN 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 Reviewed-by: Richard Levitte Reviewed-by: Shane Lontis (Merged from https://github.com/openssl/openssl/pull/10631) --- crypto/ec/ec_ameth.c | 55 ++++++++-- providers/implementations/keymgmt/ec_kmgmt.c | 109 +++++++++++++++++-- 2 files changed, 148 insertions(+), 16 deletions(-) diff --git a/crypto/ec/ec_ameth.c b/crypto/ec/ec_ameth.c index c4e8177c28..d6807661ff 100644 --- a/crypto/ec/ec_ameth.c +++ b/crypto/ec/ec_ameth.c @@ -663,20 +663,61 @@ int ec_pkey_export_to(const EVP_PKEY *from, void *to_keydata, 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. */ - int ecdh_cofactor_mode = + ecdh_cofactor_mode = (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, diff --git a/providers/implementations/keymgmt/ec_kmgmt.c b/providers/implementations/keymgmt/ec_kmgmt.c index 81907476d9..794dd92499 100644 --- a/providers/implementations/keymgmt/ec_kmgmt.c +++ b/providers/implementations/keymgmt/ec_kmgmt.c @@ -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 @@ -19,6 +19,7 @@ #include #include #include +#include "crypto/bn.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; - 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)) @@ -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; - 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; -- 2.25.1