diff options
-rw-r--r-- | crypto/dsa/dsa_err.c | 3 | ||||
-rw-r--r-- | crypto/dsa/dsa_ossl.c | 28 | ||||
-rw-r--r-- | crypto/err/openssl.txt | 3 | ||||
-rw-r--r-- | include/crypto/dsaerr.h | 2 | ||||
-rw-r--r-- | include/openssl/dsaerr.h | 3 | ||||
-rw-r--r-- | test/dsatest.c | 136 |
6 files changed, 137 insertions, 38 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; diff --git a/crypto/err/openssl.txt b/crypto/err/openssl.txt index 3c44e32f38..83d67ab1ce 100644 --- a/crypto/err/openssl.txt +++ b/crypto/err/openssl.txt @@ -1,4 +1,4 @@ -# Copyright 1999-2022 The OpenSSL Project Authors. All Rights Reserved. +# Copyright 1999-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 @@ -545,6 +545,7 @@ DSA_R_PARAMETER_ENCODING_ERROR:105:parameter encoding error DSA_R_P_NOT_PRIME:115:p not prime DSA_R_Q_NOT_PRIME:113:q not prime DSA_R_SEED_LEN_SMALL:110:seed_len is less than the length of q +DSA_R_TOO_MANY_RETRIES:116:too many retries DSO_R_CTRL_FAILED:100:control command failed DSO_R_DSO_ALREADY_LOADED:110:dso already loaded DSO_R_EMPTY_FILE_STRUCTURE:113:empty file structure diff --git a/include/crypto/dsaerr.h b/include/crypto/dsaerr.h index 9898097d0d..fde8358fc9 100644 --- a/include/crypto/dsaerr.h +++ b/include/crypto/dsaerr.h @@ -1,6 +1,6 @@ /* * Generated by util/mkerr.pl DO NOT EDIT - * Copyright 2020-2021 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/dsaerr.h b/include/openssl/dsaerr.h index 5f0ca8d12a..26ada57d80 100644 --- a/include/openssl/dsaerr.h +++ b/include/openssl/dsaerr.h @@ -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 @@ -38,6 +38,7 @@ # define DSA_R_P_NOT_PRIME 115 # define DSA_R_Q_NOT_PRIME 113 # define DSA_R_SEED_LEN_SMALL 110 +# define DSA_R_TOO_MANY_RETRIES 116 # endif #endif diff --git a/test/dsatest.c b/test/dsatest.c index 2d34ca4261..5c13a0d825 100644 --- a/test/dsatest.c +++ b/test/dsatest.c @@ -32,6 +32,32 @@ #ifndef OPENSSL_NO_DSA static int dsa_cb(int p, int n, BN_GENCB *arg); +static unsigned char out_p[] = { + 0x8d, 0xf2, 0xa4, 0x94, 0x49, 0x22, 0x76, 0xaa, + 0x3d, 0x25, 0x75, 0x9b, 0xb0, 0x68, 0x69, 0xcb, + 0xea, 0xc0, 0xd8, 0x3a, 0xfb, 0x8d, 0x0c, 0xf7, + 0xcb, 0xb8, 0x32, 0x4f, 0x0d, 0x78, 0x82, 0xe5, + 0xd0, 0x76, 0x2f, 0xc5, 0xb7, 0x21, 0x0e, 0xaf, + 0xc2, 0xe9, 0xad, 0xac, 0x32, 0xab, 0x7a, 0xac, + 0x49, 0x69, 0x3d, 0xfb, 0xf8, 0x37, 0x24, 0xc2, + 0xec, 0x07, 0x36, 0xee, 0x31, 0xc8, 0x02, 0x91, +}; +static unsigned char out_q[] = { + 0xc7, 0x73, 0x21, 0x8c, 0x73, 0x7e, 0xc8, 0xee, + 0x99, 0x3b, 0x4f, 0x2d, 0xed, 0x30, 0xf4, 0x8e, + 0xda, 0xce, 0x91, 0x5f, +}; +static unsigned char out_g[] = { + 0x62, 0x6d, 0x02, 0x78, 0x39, 0xea, 0x0a, 0x13, + 0x41, 0x31, 0x63, 0xa5, 0x5b, 0x4c, 0xb5, 0x00, + 0x29, 0x9d, 0x55, 0x22, 0x95, 0x6c, 0xef, 0xcb, + 0x3b, 0xff, 0x10, 0xf3, 0x99, 0xce, 0x2c, 0x2e, + 0x71, 0xcb, 0x9d, 0xe5, 0xfa, 0x24, 0xba, 0xbf, + 0x58, 0xe5, 0xb7, 0x95, 0x21, 0x92, 0x5c, 0x9c, + 0xc4, 0x2e, 0x9f, 0x6f, 0x46, 0x4b, 0x08, 0x8c, + 0xc5, 0x72, 0xaf, 0x53, 0xe6, 0xd7, 0x88, 0x02, +}; + static int dsa_test(void) { BN_GENCB *cb; @@ -51,31 +77,6 @@ static int dsa_test(void) 0xb6, 0x21, 0x1b, 0x40, 0x62, 0xba, 0x32, 0x24, 0xe0, 0x42, 0x7d, 0xd3, }; - static unsigned char out_p[] = { - 0x8d, 0xf2, 0xa4, 0x94, 0x49, 0x22, 0x76, 0xaa, - 0x3d, 0x25, 0x75, 0x9b, 0xb0, 0x68, 0x69, 0xcb, - 0xea, 0xc0, 0xd8, 0x3a, 0xfb, 0x8d, 0x0c, 0xf7, - 0xcb, 0xb8, 0x32, 0x4f, 0x0d, 0x78, 0x82, 0xe5, - 0xd0, 0x76, 0x2f, 0xc5, 0xb7, 0x21, 0x0e, 0xaf, - 0xc2, 0xe9, 0xad, 0xac, 0x32, 0xab, 0x7a, 0xac, - 0x49, 0x69, 0x3d, 0xfb, 0xf8, 0x37, 0x24, 0xc2, - 0xec, 0x07, 0x36, 0xee, 0x31, 0xc8, 0x02, 0x91, - }; - static unsigned char out_q[] = { - 0xc7, 0x73, 0x21, 0x8c, 0x73, 0x7e, 0xc8, 0xee, - 0x99, 0x3b, 0x4f, 0x2d, 0xed, 0x30, 0xf4, 0x8e, - 0xda, 0xce, 0x91, 0x5f, - }; - static unsigned char out_g[] = { - 0x62, 0x6d, 0x02, 0x78, 0x39, 0xea, 0x0a, 0x13, - 0x41, 0x31, 0x63, 0xa5, 0x5b, 0x4c, 0xb5, 0x00, - 0x29, 0x9d, 0x55, 0x22, 0x95, 0x6c, 0xef, 0xcb, - 0x3b, 0xff, 0x10, 0xf3, 0x99, 0xce, 0x2c, 0x2e, - 0x71, 0xcb, 0x9d, 0xe5, 0xfa, 0x24, 0xba, 0xbf, - 0x58, 0xe5, 0xb7, 0x95, 0x21, 0x92, 0x5c, 0x9c, - 0xc4, 0x2e, 0x9f, 0x6f, 0x46, 0x4b, 0x08, 0x8c, - 0xc5, 0x72, 0xaf, 0x53, 0xe6, 0xd7, 0x88, 0x02, - }; static const unsigned char str1[] = "12345678901234567890"; if (!TEST_ptr(cb = BN_GENCB_new())) @@ -114,7 +115,6 @@ static int dsa_test(void) goto end; if (TEST_int_gt(DSA_verify(0, str1, 20, sig, siglen, dsa), 0)) ret = 1; - end: DSA_free(dsa); BN_GENCB_free(cb); @@ -325,6 +325,89 @@ static int test_dsa_default_paramgen_validate(int i) return ret; } +static int test_dsa_sig_infinite_loop(void) +{ + int ret = 0; + DSA *dsa = NULL; + BIGNUM *p = NULL, *q = NULL, *g = NULL, *priv = NULL, *pub = NULL, *priv2 = NULL; + BIGNUM *badq = NULL, *badpriv = NULL; + const unsigned char msg[] = { 0x00 }; + unsigned int signature_len; + unsigned char signature[64]; + + static unsigned char out_priv[] = { + 0x17, 0x00, 0xb2, 0x8d, 0xcb, 0x24, 0xc9, 0x98, + 0xd0, 0x7f, 0x1f, 0x83, 0x1a, 0xa1, 0xc4, 0xa4, + 0xf8, 0x0f, 0x7f, 0x12 + }; + static unsigned char out_pub[] = { + 0x04, 0x72, 0xee, 0x8d, 0xaa, 0x4d, 0x89, 0x60, + 0x0e, 0xb2, 0xd4, 0x38, 0x84, 0xa2, 0x2a, 0x60, + 0x5f, 0x67, 0xd7, 0x9e, 0x24, 0xdd, 0xe8, 0x50, + 0xf2, 0x23, 0x71, 0x55, 0x53, 0x94, 0x0d, 0x6b, + 0x2e, 0xcd, 0x30, 0xda, 0x6f, 0x1e, 0x2c, 0xcf, + 0x59, 0xbe, 0x05, 0x6c, 0x07, 0x0e, 0xc6, 0x38, + 0x05, 0xcb, 0x0c, 0x44, 0x0a, 0x08, 0x13, 0xb6, + 0x0f, 0x14, 0xde, 0x4a, 0xf6, 0xed, 0x4e, 0xc3 + }; + if (!TEST_ptr(p = BN_bin2bn(out_p, sizeof(out_p), NULL)) + || !TEST_ptr(q = BN_bin2bn(out_q, sizeof(out_q), NULL)) + || !TEST_ptr(g = BN_bin2bn(out_g, sizeof(out_g), NULL)) + || !TEST_ptr(pub = BN_bin2bn(out_pub, sizeof(out_pub), NULL)) + || !TEST_ptr(priv = BN_bin2bn(out_priv, sizeof(out_priv), NULL)) + || !TEST_ptr(priv2 = BN_dup(priv)) + || !TEST_ptr(badq = BN_new()) + || !TEST_true(BN_set_word(badq, 1)) + || !TEST_ptr(badpriv = BN_new()) + || !TEST_true(BN_set_word(badpriv, 0)) + || !TEST_ptr(dsa = DSA_new())) + goto err; + + if (!TEST_true(DSA_set0_pqg(dsa, p, q, g))) + goto err; + p = q = g = NULL; + + if (!TEST_true(DSA_set0_key(dsa, pub, priv))) + goto err; + pub = priv = NULL; + + if (!TEST_int_le(DSA_size(dsa), sizeof(signature))) + goto err; + + if (!TEST_true(DSA_sign(0, msg, sizeof(msg), signature, &signature_len, dsa))) + goto err; + + /* Test using a private key of zero fails - this causes an infinite loop without the retry test */ + if (!TEST_true(DSA_set0_key(dsa, NULL, badpriv))) + goto err; + badpriv = NULL; + if (!TEST_false(DSA_sign(0, msg, sizeof(msg), signature, &signature_len, dsa))) + goto err; + + /* Restore private and set a bad q - this caused an infinite loop in the setup */ + if (!TEST_true(DSA_set0_key(dsa, NULL, priv2))) + goto err; + priv2 = NULL; + if (!TEST_true(DSA_set0_pqg(dsa, NULL, badq, NULL))) + goto err; + badq = NULL; + if (!TEST_false(DSA_sign(0, msg, sizeof(msg), signature, &signature_len, dsa))) + goto err; + + ret = 1; +err: + BN_free(badq); + BN_free(badpriv); + BN_free(pub); + BN_free(priv); + BN_free(priv2); + BN_free(g); + BN_free(q); + BN_free(p); + DSA_free(dsa); + return ret; +} + #endif /* OPENSSL_NO_DSA */ int setup_tests(void) @@ -332,6 +415,7 @@ int setup_tests(void) #ifndef OPENSSL_NO_DSA ADD_TEST(dsa_test); ADD_TEST(dsa_keygen_test); + ADD_TEST(test_dsa_sig_infinite_loop); ADD_ALL_TESTS(test_dsa_default_paramgen_validate, 2); #endif return 1; |