Hi Rich, Ismo, I'm surprised this doesn't cause the compiler to warn/error out (inline)
On 8/27/15, 21:57, "Rich Salz" <rs...@openssl.org> wrote: >The branch master has been updated > via f00a10b89734e84fe80f98ad9e2e77b557c701ae (commit) > from 3c65047d30dacca345d30269b95af4a5c413e8d1 (commit) > > >- Log ----------------------------------------------------------------- >commit f00a10b89734e84fe80f98ad9e2e77b557c701ae >Author: Ismo Puustinen <ismo.puusti...@intel.com> >Date: Fri Aug 7 22:14:47 2015 -0400 > > GH367: Fix dsa keygen for too-short seed > > If the seed value for dsa key generation is too short (< qsize), > return an error. Also update the documentation. > > Signed-off-by: Rich Salz <rs...@akamai.com> > Reviewed-by: Emilia Käsper <emi...@openssl.org> > >----------------------------------------------------------------------- > >diff --git a/crypto/dsa/dsa_gen.c b/crypto/dsa/dsa_gen.c >index e030cfa..a4fae17 100644 >--- a/crypto/dsa/dsa_gen.c >+++ b/crypto/dsa/dsa_gen.c >@@ -132,18 +132,15 @@ int dsa_builtin_paramgen(DSA *ret, size_t bits, >size_t qbits, > > bits = (bits + 63) / 64 * 64; > >- /* >- * NB: seed_len == 0 is special case: copy generated seed to seed_in >if >- * it is not NULL. >- */ >- if (seed_len && (seed_len < (size_t)qsize)) >- seed_in = NULL; /* seed buffer too small -- ignore */ >- if (seed_len > (size_t)qsize) >- seed_len = qsize; /* App. 2.2 of FIPS PUB 186 allows larger >- * SEED, but our internal buffers are >- * restricted to 160 bits */ >- if (seed_in != NULL) >+ if (seed_in != NULL) { >+ if (seed_len < (size_t)qsize) >+ return 0; >+ if (seed_len > (size_t)qsize) { >+ /* Don't overflow seed local variable. */ The comment and code here are a slight mismatch, since qsize is dynamically computed (but limited to three values, the largest of which is used to size the local variable). It's not clear that using SHA256_DIGEST_LENGTH for the check would actually be better, though. >+ seed_len = qsize; >+ } > memcpy(seed, seed_in, seed_len); >+ } > > if ((ctx = BN_CTX_new()) == NULL) > goto err; >@@ -166,20 +163,18 @@ int dsa_builtin_paramgen(DSA *ret, size_t bits, >size_t qbits, > > for (;;) { > for (;;) { /* find q */ >- int seed_is_random; >+ int seed_is_random = seed_in == NULL; This part seems really bogus; seed_is_random is an int, but seed_in is const unsigned char *; the assignment makes no sense. > > /* step 1 */ > if (!BN_GENCB_call(cb, 0, m++)) > goto err; > >- if (!seed_len) { >+ if (seed_is_random) { and this chunk can never execute since seed_is_random was just set to 0 (er, NULL). I guess the intent is to declare the variable in the outer loop? > if (RAND_bytes(seed, qsize) <= 0) > goto err; >- seed_is_random = 1; > } else { >- seed_is_random = 0; >- seed_len = 0; /* use random seed if 'seed_in' turns >out to >- * be bad */ >+ /* If we come back through, use random seed next time. */ >+ seed_in = NULL; and seed_in is never read after this point. > } > memcpy(buf, seed, qsize); > memcpy(buf2, seed, qsize); >diff --git a/doc/crypto/DSA_generate_parameters.pod >b/doc/crypto/DSA_generate_parameters.pod >index d2a0418..92c89a0 100644 >--- a/doc/crypto/DSA_generate_parameters.pod >+++ b/doc/crypto/DSA_generate_parameters.pod >@@ -23,13 +23,12 @@ Deprecated: > DSA_generate_parameters_ex() generates primes p and q and a generator g > for use in the DSA and stores the result in B<dsa>. > >-B<bits> is the length of the prime to be generated; the DSS allows a >-maximum of 1024 bits. >+B<bits> is the length of the prime p to be generated. >+For lengths under 2048 bits, the length of q is 160 bits; for lengths >+at least 2048, it is set to 256 bits. The grammar here is slightly unusual; "for lengths of at least 2048 bits" or "for lengths 2048 bits and larger" would feel more natural to me. -Ben > >-If B<seed> is B<NULL> or B<seed_len> E<lt> 20, the primes will be >-generated at random. Otherwise, the seed is used to generate >-them. If the given seed does not yield a prime q, a new random >-seed is chosen and placed at B<seed>. >+If B<seed> is NULL, the primes will be generated at random. >+If B<seed_len> is less than the length of q, an error is returned. > > DSA_generate_parameters_ex() places the iteration count in > *B<counter_ret> and a counter used for finding a generator in >_____ >openssl-commits mailing list >To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-commits > _______________________________________________ openssl-dev mailing list To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev