Pick a q size consistent with the digest for DSA param generation

There are two undocumented DSA parameter generation options available in
the genpkey command line app:
dsa_paramgen_md and dsa_paramgen_q_bits.

These can also be accessed via the EVP API but only by using
EVP_PKEY_CTX_ctrl() or EVP_PKEY_CTX_ctrl_str() directly. There are no
helper macros for these options.

dsa_paramgen_q_bits sets the length of q in bits (default 160 bits).
dsa_paramgen_md sets the digest that is used during the parameter
generation (default SHA1). In particular the output length of the digest
used must be equal to or greater than the number of bits in q because of
this code:

            if (!EVP_Digest(seed, qsize, md, NULL, evpmd, NULL))
                goto err;
            if (!EVP_Digest(buf, qsize, buf2, NULL, evpmd, NULL))
                goto err;
            for (i = 0; i < qsize; i++)
                md[i] ^= buf2[i];

            /* step 3 */
            md[0] |= 0x80;
            md[qsize - 1] |= 0x01;
            if (!BN_bin2bn(md, qsize, q))
                goto err;

qsize here is the number of bits in q and evpmd is the digest set via
dsa_paramgen_md. md and buf2 are buffers of length SHA256_DIGEST_LENGTH.
buf2 has been filled with qsize bits of random seed data, and md is
uninitialised.

If the output size of evpmd is less than qsize then the line "md[i] ^=
buf2[i]" will be xoring an uninitialised value and the random seed data
together to form the least significant bits of q (and not using the
output of the digest at all for those bits) - which is probably not what
was intended. The same seed is then used as an input to generating p. If
the uninitialised data is actually all zeros (as seems quite likely)
then the least significant bits of q will exactly match the least
significant bits of the seed.

This problem only occurs if you use these undocumented and difficult to
find options and you set the size of q to be greater than the message
digest output size. This is for parameter generation only not key
generation. This scenario is considered highly unlikely and
therefore the security risk of this is considered negligible.

Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/5800)

diff --git a/crypto/dsa/dsa_err.c b/crypto/dsa/dsa_err.c
index 168dadf05944f240a22110ce3701b3ad7c84cee9..8f97f6f3f9eed6ddc84798fd42d3eb07fb8bc872 100644 (file)
--- 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-2017 The OpenSSL Project Authors. All Rights Reserved.
+ * Copyright 1995-2018 The OpenSSL Project Authors. All Rights Reserved.
  *
  * Licensed under the OpenSSL license (the "License").  You may not use
  * this file except in compliance with the License.  You can obtain a copy
@@ -38,6 +38,7 @@ static const ERR_STRING_DATA DSA_str_functs[] = {
     {ERR_PACK(ERR_LIB_DSA, DSA_F_OLD_DSA_PRIV_DECODE, 0),
      "old_dsa_priv_decode"},
     {ERR_PACK(ERR_LIB_DSA, DSA_F_PKEY_DSA_CTRL, 0), "pkey_dsa_ctrl"},
+    {ERR_PACK(ERR_LIB_DSA, DSA_F_PKEY_DSA_CTRL_STR, 0), "pkey_dsa_ctrl_str"},
     {ERR_PACK(ERR_LIB_DSA, DSA_F_PKEY_DSA_KEYGEN, 0), "pkey_dsa_keygen"},
     {0, NULL}
 };

diff --git a/crypto/dsa/dsa_gen.c b/crypto/dsa/dsa_gen.c
index e58ad8d70d315eb771b1940514b8073887cf554b..dc61660cc52afb9d722cb95e673deb42644a1433 100644 (file)
--- a/crypto/dsa/dsa_gen.c
+++ b/crypto/dsa/dsa_gen.c
@@ -64,9 +64,16 @@ int dsa_builtin_paramgen(DSA *ret, size_t bits, size_t qbits,
         /* invalid q size */
         return 0;
 
-    if (evpmd == NULL)
-        /* use SHA1 as default */
-        evpmd = EVP_sha1();
+    if (evpmd == NULL) {
+        if (qsize == SHA_DIGEST_LENGTH)
+            evpmd = EVP_sha1();
+        else if (qsize == SHA224_DIGEST_LENGTH)
+            evpmd = EVP_sha224();
+        else
+            evpmd = EVP_sha256();
+    } else {
+        qsize = EVP_MD_size(evpmd);
+    }
 
     if (bits < 512)
         bits = 512;

diff --git a/crypto/err/openssl.txt b/crypto/err/openssl.txt
index 51bd461316309ff5915fe6e29422dc5b4173f1c9..d1cc039b83cfef7ae9d033cf70497ae9b8445217 100644 (file)
--- a/crypto/err/openssl.txt
+++ b/crypto/err/openssl.txt
@@ -424,6 +424,7 @@ DSA_F_DSA_SIGN_SETUP:107:DSA_sign_setup
 DSA_F_DSA_SIG_NEW:102:DSA_SIG_new
 DSA_F_OLD_DSA_PRIV_DECODE:122:old_dsa_priv_decode
 DSA_F_PKEY_DSA_CTRL:120:pkey_dsa_ctrl
+DSA_F_PKEY_DSA_CTRL_STR:104:pkey_dsa_ctrl_str
 DSA_F_PKEY_DSA_KEYGEN:121:pkey_dsa_keygen
 DSO_F_DLFCN_BIND_FUNC:100:dlfcn_bind_func
 DSO_F_DLFCN_LOAD:102:dlfcn_load

diff --git a/include/openssl/dsaerr.h b/include/openssl/dsaerr.h
index a5248ab700fbb2330c30666e9ace00888e0d11cf..d94f97bba8f213c089cdced5e86533db4e3fc654 100644 (file)
--- a/include/openssl/dsaerr.h
+++ b/include/openssl/dsaerr.h
@@ -44,6 +44,7 @@ int ERR_load_DSA_strings(void);
 #  define DSA_F_DSA_SIG_NEW                                102
 #  define DSA_F_OLD_DSA_PRIV_DECODE                        122
 #  define DSA_F_PKEY_DSA_CTRL                              120
+#  define DSA_F_PKEY_DSA_CTRL_STR                          104
 #  define DSA_F_PKEY_DSA_KEYGEN                            121
 
 /*