From: slontis Date: Mon, 27 Feb 2023 03:53:25 +0000 (+1000) Subject: Fix potential infinite loops in ECDSA signing. X-Git-Tag: openssl-3.2.0-alpha1~1218 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=5f820bd7535b871fdfdc0303c3af23ba4be901f0;p=thirdparty%2Fopenssl.git Fix potential infinite loops in ECDSA signing. Similiar checks to the DSA code have been added for ECDSA also. This should not be a problem when using named groups. Reviewed-by: Matt Caswell Reviewed-by: Paul Dale (Merged from https://github.com/openssl/openssl/pull/20384) --- diff --git a/crypto/ec/ec_err.c b/crypto/ec/ec_err.c index 4d6f2a76ad2..480376686b9 100644 --- a/crypto/ec/ec_err.c +++ b/crypto/ec/ec_err.c @@ -1,6 +1,6 @@ /* * Generated by util/mkerr.pl DO NOT EDIT - * Copyright 1995-2022 The OpenSSL Project Authors. All Rights Reserved. + * Copyright 1995-2023 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 @@ -108,6 +108,7 @@ static const ERR_STRING_DATA EC_str_reasons[] = { "random number generation failed"}, {ERR_PACK(ERR_LIB_EC, 0, EC_R_SHARED_INFO_ERROR), "shared info error"}, {ERR_PACK(ERR_LIB_EC, 0, EC_R_SLOT_FULL), "slot full"}, + {ERR_PACK(ERR_LIB_EC, 0, EC_R_TOO_MANY_RETRIES), "too many retries"}, {ERR_PACK(ERR_LIB_EC, 0, EC_R_UNDEFINED_GENERATOR), "undefined generator"}, {ERR_PACK(ERR_LIB_EC, 0, EC_R_UNDEFINED_ORDER), "undefined order"}, {ERR_PACK(ERR_LIB_EC, 0, EC_R_UNKNOWN_COFACTOR), "unknown cofactor"}, diff --git a/crypto/ec/ecdsa_ossl.c b/crypto/ec/ecdsa_ossl.c index 30e130dbba7..6ab7be0fe5c 100644 --- a/crypto/ec/ecdsa_ossl.c +++ b/crypto/ec/ecdsa_ossl.c @@ -21,6 +21,15 @@ #include "ec_local.h" #include "internal/deterministic_nonce.h" +#define MIN_ECDSA_SIGN_ORDERBITS 64 +/* + * It is highly unlikely that a retry will happen, + * Multiple retries would indicate that something is wrong + * with the group parameters (which would normally only happen + * with a bad custom group). + */ +#define MAX_ECDSA_SIGN_RETRIES 8 + static int ecdsa_sign_setup(EC_KEY *eckey, BN_CTX *ctx_in, BIGNUM **kinvp, BIGNUM **rp, const unsigned char *dgst, int dlen, @@ -157,13 +166,15 @@ static int ecdsa_sign_setup(EC_KEY *eckey, BN_CTX *ctx_in, /* Preallocate space */ order_bits = BN_num_bits(order); - if (!BN_set_bit(k, order_bits) + /* Check the number of bits here so that an infinite loop is not possible */ + if (order_bits < MIN_ECDSA_SIGN_ORDERBITS + || !BN_set_bit(k, order_bits) || !BN_set_bit(r, order_bits) || !BN_set_bit(X, order_bits)) goto err; do { - /* get random or determinstic value of k */ + /* get random or deterministic value of k */ do { int res = 0; @@ -243,6 +254,7 @@ ECDSA_SIG *ossl_ecdsa_simple_sign_sig(const unsigned char *dgst, int dgst_len, EC_KEY *eckey) { int ok = 0, i; + int retries = 0; BIGNUM *kinv = NULL, *s, *m = NULL; const BIGNUM *order, *ckinv; BN_CTX *ctx = NULL; @@ -353,6 +365,11 @@ ECDSA_SIG *ossl_ecdsa_simple_sign_sig(const unsigned char *dgst, int dgst_len, ERR_raise(ERR_LIB_EC, EC_R_NEED_NEW_SETUP_VALUES); goto err; } + /* Avoid infinite loops cause by invalid group parameters */ + if (retries++ > MAX_ECDSA_SIGN_RETRIES) { + ERR_raise(ERR_LIB_EC, EC_R_TOO_MANY_RETRIES); + goto err; + } } else { /* s != 0 => we have a valid signature */ break; diff --git a/crypto/err/openssl.txt b/crypto/err/openssl.txt index 83d67ab1ce3..e90e5dc0333 100644 --- a/crypto/err/openssl.txt +++ b/crypto/err/openssl.txt @@ -629,6 +629,7 @@ EC_R_POINT_IS_NOT_ON_CURVE:107:point is not on curve EC_R_RANDOM_NUMBER_GENERATION_FAILED:158:random number generation failed EC_R_SHARED_INFO_ERROR:150:shared info error EC_R_SLOT_FULL:108:slot full +EC_R_TOO_MANY_RETRIES:176:too many retries EC_R_UNDEFINED_GENERATOR:113:undefined generator EC_R_UNDEFINED_ORDER:128:undefined order EC_R_UNKNOWN_COFACTOR:164:unknown cofactor diff --git a/include/crypto/ecerr.h b/include/crypto/ecerr.h index 4658ae8fb2c..782526bf856 100644 --- a/include/crypto/ecerr.h +++ b/include/crypto/ecerr.h @@ -1,6 +1,6 @@ /* * Generated by util/mkerr.pl DO NOT EDIT - * Copyright 2020-2022 The OpenSSL Project Authors. All Rights Reserved. + * Copyright 2020-2023 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 diff --git a/include/openssl/ecerr.h b/include/openssl/ecerr.h index 46405ac62d9..f15f91f6bf0 100644 --- a/include/openssl/ecerr.h +++ b/include/openssl/ecerr.h @@ -1,6 +1,6 @@ /* * Generated by util/mkerr.pl DO NOT EDIT - * Copyright 1995-2022 The OpenSSL Project Authors. All Rights Reserved. + * Copyright 1995-2023 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 @@ -90,6 +90,7 @@ # define EC_R_RANDOM_NUMBER_GENERATION_FAILED 158 # define EC_R_SHARED_INFO_ERROR 150 # define EC_R_SLOT_FULL 108 +# define EC_R_TOO_MANY_RETRIES 176 # define EC_R_UNDEFINED_GENERATOR 113 # define EC_R_UNDEFINED_ORDER 128 # define EC_R_UNKNOWN_COFACTOR 164