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)

diff --git a/crypto/ec/ec_err.c b/crypto/ec/ec_err.c
index 4d6f2a76ad2041f86612d819fb3dc565ee9c4f89..480376686b9e70ebb28d9b779c332a18642ba81d 100644 (file)
--- 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 30e130dbba7d6500c1e9e95a46bbd46c42ec6c2b..6ab7be0fe5c7d5067a6bbcab9a13b3c0207e51e8 100644 (file)
--- 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 83d67ab1ce31de5d4da1dd9be22d6dbe23de5f35..e90e5dc033397c5076eda5de149855e1db399db2 100644 (file)
--- 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 4658ae8fb2cdb6faedf34f93b706be9104c952f4..782526bf8565120a3f3c705bd7ea1ac3cfcc7ab7 100644 (file)
--- 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 46405ac62d91b019d85ee17c31dc7d23bf31a2a9..f15f91f6bf05fb08ff82066f32a3bf76ba748b9c 100644 (file)
--- 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