Browse Source

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)
slontis 1 year ago
parent
commit
5f820bd753
5 changed files with 25 additions and 5 deletions
  1. 2 1
      crypto/ec/ec_err.c
  2. 19 2
      crypto/ec/ecdsa_ossl.c
  3. 1 0
      crypto/err/openssl.txt
  4. 1 1
      include/crypto/ecerr.h
  5. 2 1
      include/openssl/ecerr.h

+ 2 - 1
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"},

+ 19 - 2
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;

+ 1 - 0
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

+ 1 - 1
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

+ 2 - 1
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