diff options
author | slontis <shane.lontis@oracle.com> | 2023-02-27 13:48:24 +1000 |
---|---|---|
committer | Pauli <pauli@openssl.org> | 2023-03-01 09:20:49 +1100 |
commit | 3a4e09ab42654b3d223f0f8dd1a9c58b2902ddcc (patch) | |
tree | ef08d17eec3bef6ddbf411f5876c9e9d1e8ddcab /crypto/dsa | |
parent | a6d7093a1dc890493d3577c738e729c0265d7b40 (diff) |
Fix infinite loops in DSA sign code.
Fixes #20268
Values such as q=1 or priv=0 caused infinite loops when calling
DSA_sign() without these changes.
There are other cases where bad domain parameters may have caused
infinite loops where the retry counter has been added. The simpler case
of priv=0 also hits this case. q=1 caused an infinite loop in the setup.
The max retry value has been set to an arbitrary value of 8 (it is
unlikely to ever do a single retry for valid values).
The minimum q bits was set to an arbitrary value of 128 (160 is still
used for legacy reasons when using 512 bit keys).
Thanks @guidovranken for detecting this, and @davidben for his
insightful analysis.
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/dsa')
-rw-r--r-- | crypto/dsa/dsa_err.c | 3 | ||||
-rw-r--r-- | crypto/dsa/dsa_ossl.c | 28 |
2 files changed, 22 insertions, 9 deletions
diff --git a/crypto/dsa/dsa_err.c b/crypto/dsa/dsa_err.c index 5685d5e83e..a92ca61664 100644 --- a/crypto/dsa/dsa_err.c +++ b/crypto/dsa/dsa_err.c @@ -1,6 +1,6 @@ /* * Generated by util/mkerr.pl DO NOT EDIT - * Copyright 1995-2021 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 @@ -36,6 +36,7 @@ static const ERR_STRING_DATA DSA_str_reasons[] = { {ERR_PACK(ERR_LIB_DSA, 0, DSA_R_Q_NOT_PRIME), "q not prime"}, {ERR_PACK(ERR_LIB_DSA, 0, DSA_R_SEED_LEN_SMALL), "seed_len is less than the length of q"}, + {ERR_PACK(ERR_LIB_DSA, 0, DSA_R_TOO_MANY_RETRIES), "too many retries"}, {0, NULL} }; diff --git a/crypto/dsa/dsa_ossl.c b/crypto/dsa/dsa_ossl.c index 888e415724..822e51786a 100644 --- a/crypto/dsa/dsa_ossl.c +++ b/crypto/dsa/dsa_ossl.c @@ -22,6 +22,9 @@ #include <openssl/asn1.h> #include "internal/deterministic_nonce.h" +#define MIN_DSA_SIGN_QBITS 128 +#define MAX_DSA_SIGN_RETRIES 8 + static DSA_SIG *dsa_do_sign(const unsigned char *dgst, int dlen, DSA *dsa); static int dsa_sign_setup_no_digest(DSA *dsa, BN_CTX *ctx_in, BIGNUM **kinvp, BIGNUM **rp); @@ -80,6 +83,7 @@ DSA_SIG *ossl_dsa_do_sign_int(const unsigned char *dgst, int dlen, DSA *dsa, int reason = ERR_R_BN_LIB; DSA_SIG *ret = NULL; int rv = 0; + int retries = 0; if (dsa->params.p == NULL || dsa->params.q == NULL @@ -135,7 +139,10 @@ DSA_SIG *ossl_dsa_do_sign_int(const unsigned char *dgst, int dlen, DSA *dsa, * s := blind^-1 * k^-1 * (blind * m + blind * r * priv_key) mod q */ - /* Generate a blinding value */ + /* + * Generate a blinding value + * The size of q is tested in dsa_sign_setup() so there should not be an infinite loop here. + */ do { if (!BN_priv_rand_ex(blind, BN_num_bits(dsa->params.q) - 1, BN_RAND_TOP_ANY, BN_RAND_BOTTOM_ANY, 0, ctx)) @@ -170,14 +177,19 @@ DSA_SIG *ossl_dsa_do_sign_int(const unsigned char *dgst, int dlen, DSA *dsa, goto err; /* - * Redo if r or s is zero as required by FIPS 186-3: this is very - * unlikely. + * Redo if r or s is zero as required by FIPS 186-4: Section 4.6 + * This is very unlikely. + * Limit the retries so there is no possibility of an infinite + * loop for bad domain parameter values. */ - if (BN_is_zero(ret->r) || BN_is_zero(ret->s)) + if (BN_is_zero(ret->r) || BN_is_zero(ret->s)) { + if (retries++ > MAX_DSA_SIGN_RETRIES) { + reason = DSA_R_TOO_MANY_RETRIES; + goto err; + } goto redo; - + } rv = 1; - err: if (rv == 0) { ERR_raise(ERR_LIB_DSA, reason); @@ -230,7 +242,6 @@ static int dsa_sign_setup(DSA *dsa, BN_CTX *ctx_in, ERR_raise(ERR_LIB_DSA, DSA_R_MISSING_PRIVATE_KEY); return 0; } - k = BN_new(); l = BN_new(); if (k == NULL || l == NULL) @@ -246,7 +257,8 @@ static int dsa_sign_setup(DSA *dsa, BN_CTX *ctx_in, /* Preallocate space */ q_bits = BN_num_bits(dsa->params.q); q_words = bn_get_top(dsa->params.q); - if (!bn_wexpand(k, q_words + 2) + if (q_bits < MIN_DSA_SIGN_QBITS + || !bn_wexpand(k, q_words + 2) || !bn_wexpand(l, q_words + 2)) goto err; |