fix DH_generate_parameters for general 'generator'

5 files changed, 58 insertions, 9 deletions

diff --git a/CHANGES b/CHANGES
index 722c4d8854..67aa57833b 100644
--- a/CHANGES
+++ b/CHANGES
@@ -4,6 +4,14 @@
 
  Changes between 0.9.7 and 0.9.8  [xx XXX 2002]
 
+  *) Add a function EC_GROUP_check_discriminant() (defined via
+     EC_METHOD) that verifies that the curve discriminant is non-zero.
+
+     Add a function EC_GROUP_check() that makes some sanity tests
+     on a EC_GROUP, its generator and order.  This includes
+     EC_GROUP_check_discriminant().
+     [Nils Larsch <nla@trustcenter.de>]
+
   *) Add ECDSA in new directory crypto/ecdsa/.
 
      Add applications 'openssl ecdsaparam' and 'openssl ecdsa'
@@ -41,6 +49,17 @@
          *) applies to 0.9.6a ... 0.9.6d and 0.9.7
          +) applies to 0.9.7 only
 
+  *) Fix DH_generate_parameters() so that it works for 'non-standard'
+     generators, i.e. generators other than 2 and 5.  (Previously, the
+     code did not properly initialise the 'add' and 'rem' values to
+     BN_generate_prime().)
+
+     In the new general case, we do not insist that 'generator' is
+     actually a primitive root: This requirement is rather pointless;
+     a generator of the order-q subgroup is just as good, if not
+     better.
+     [Bodo Moeller]
+
   *) Map new X509 verification errors to alerts. Discovered and submitted by
      Tom Wu <tom@arcot.com>.
      [Lutz Jaenicke]
diff --git a/crypto/dh/dh.h b/crypto/dh/dh.h
index 15cf70de0b..05851f8429 100644
--- a/crypto/dh/dh.h
+++ b/crypto/dh/dh.h
@@ -198,6 +198,7 @@ void ERR_load_DH_strings(void);
 #define DH_F_DH_NEW_METHOD				 105
 
 /* Reason codes. */
+#define DH_R_BAD_GENERATOR				 101
 #define DH_R_NO_PRIVATE_VALUE				 100
 
 #ifdef  __cplusplus
diff --git a/crypto/dh/dh_err.c b/crypto/dh/dh_err.c
index 225779336c..d837950aec 100644
--- a/crypto/dh/dh_err.c
+++ b/crypto/dh/dh_err.c
@@ -1,6 +1,6 @@
 /* crypto/dh/dh_err.c */
 /* ====================================================================
- * Copyright (c) 1999 The OpenSSL Project.  All rights reserved.
+ * Copyright (c) 1999-2002 The OpenSSL Project.  All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -77,6 +77,7 @@ static ERR_STRING_DATA DH_str_functs[]=
 
 static ERR_STRING_DATA DH_str_reasons[]=
 	{
+{DH_R_BAD_GENERATOR                      ,"bad generator"},
 {DH_R_NO_PRIVATE_VALUE                   ,"no private value"},
 {0,NULL}
 	};
diff --git a/crypto/dh/dh_gen.c b/crypto/dh/dh_gen.c
index 7a6a38fbb4..06f78b35ab 100644
--- a/crypto/dh/dh_gen.c
+++ b/crypto/dh/dh_gen.c
@@ -82,7 +82,10 @@
  * Since DH should be using a safe prime (both p and q are prime),
  * this generator function can take a very very long time to run.
  */
-
+/* Actually there is no reason to insist that 'generator' be a generator.
+ * It's just as OK (and in some sense better) to use a generator of the
+ * order-q subgroup.
+ */
 DH *DH_generate_parameters(int prime_len, int generator,
 	     void (*callback)(int,int,void *), void *cb_arg)
 	{
@@ -100,30 +103,43 @@ DH *DH_generate_parameters(int prime_len, int generator,
 	t2 = BN_CTX_get(ctx);
 	if (t1 == NULL || t2 == NULL) goto err;
 	
+	if (generator <= 1)
+		{
+		DHerr(DH_F_DH_GENERATE_PARAMETERS, DH_R_BAD_GENERATOR);
+		goto err;
+		}
 	if (generator == DH_GENERATOR_2)
 		{
-		BN_set_word(t1,24);
-		BN_set_word(t2,11);
+		if (!BN_set_word(t1,24)) goto err;
+		if (!BN_set_word(t2,11)) goto err;
 		g=2;
 		}
-#ifdef undef  /* does not work for safe primes */
+#if 0 /* does not work for safe primes */
 	else if (generator == DH_GENERATOR_3)
 		{
-		BN_set_word(t1,12);
-		BN_set_word(t2,5);
+		if (!BN_set_word(t1,12)) goto err;
+		if (!BN_set_word(t2,5)) goto err;
 		g=3;
 		}
 #endif
 	else if (generator == DH_GENERATOR_5)
 		{
-		BN_set_word(t1,10);
-		BN_set_word(t2,3);
+		if (!BN_set_word(t1,10)) goto err;
+		if (!BN_set_word(t2,3)) goto err;
 		/* BN_set_word(t3,7); just have to miss
 		 * out on these ones :-( */
 		g=5;
 		}
 	else
+		{
+		/* in the general case, don't worry if 'generator' is a
+		 * generator or not: since we are using safe primes,
+		 * it will generate either an order-q or an order-2q group,
+		 * which both is OK */
+		if (!BN_set_word(t1,2)) goto err;
+		if (!BN_set_word(t2,1)) goto err;
 		g=generator;
+		}
 	
 	p=BN_generate_prime(NULL,prime_len,1,t1,t2,callback,cb_arg);
 	if (p == NULL) goto err;
diff --git a/crypto/dh/dhtest.c b/crypto/dh/dhtest.c
index 0176436a55..34894ced73 100644
--- a/crypto/dh/dhtest.c
+++ b/crypto/dh/dhtest.c
@@ -117,6 +117,16 @@ int main(int argc, char *argv[])
 	a=DH_generate_parameters(64,DH_GENERATOR_5,cb,out);
 	if (a == NULL) goto err;
 
+	if (!DH_check(a, &i)) goto err;
+	if (i & DH_CHECK_P_NOT_PRIME)
+		BIO_puts(out, "p value is not prime\n");
+	if (i & DH_CHECK_P_NOT_SAFE_PRIME)
+		BIO_puts(out, "p value is not a safe prime\n");
+	if (i & DH_UNABLE_TO_CHECK_GENERATOR)
+		BIO_puts(out, "unable to check the generator value\n");
+	if (i & DH_NOT_SUITABLE_GENERATOR)
+		BIO_puts(out, "the g value is not a generator\n");
+
 	BIO_puts(out,"\np    =");
 	BN_print(out,a->p);
 	BIO_puts(out,"\ng    =");
@@ -175,6 +185,8 @@ int main(int argc, char *argv[])
 	else
 		ret=0;
 err:
+	ERR_print_errors_fp(stderr);
+
 	if (abuf != NULL) OPENSSL_free(abuf);
 	if (bbuf != NULL) OPENSSL_free(bbuf);
 	if(b != NULL) DH_free(b);