diff options
author | slontis <shane.lontis@oracle.com> | 2023-02-27 13:53:25 +1000 |
---|---|---|
committer | Pauli <pauli@openssl.org> | 2023-03-01 09:20:49 +1100 |
commit | 5f820bd7535b871fdfdc0303c3af23ba4be901f0 (patch) | |
tree | 8731a54f88181ea254b78b1a25016f8fd241bd19 /crypto | |
parent | 3a4e09ab42654b3d223f0f8dd1a9c58b2902ddcc (diff) |
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 <matt@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/20384)
Diffstat (limited to 'crypto')
-rw-r--r-- | crypto/ec/ec_err.c | 3 | ||||
-rw-r--r-- | crypto/ec/ecdsa_ossl.c | 21 | ||||
-rw-r--r-- | crypto/err/openssl.txt | 1 |
3 files changed, 22 insertions, 3 deletions
diff --git a/crypto/ec/ec_err.c b/crypto/ec/ec_err.c index 4d6f2a76ad..480376686b 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 30e130dbba..6ab7be0fe5 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 83d67ab1ce..e90e5dc033 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 |