2 files changed, 50 insertions, 27 deletions

diff --git a/ChangeLog b/ChangeLog
index 4b8a82591..10de1077e 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -20,6 +20,9 @@
      [ssh_config.5]
      explicitly mention that IdentitiesOnly can be used with IdentityFile
      to control which keys are offered from an agent.
+   - djm@cvs.openbsd.org 2013/06/21 05:42:32
+     [dh.c]
+     sprinkle in some error() to explain moduli(5) parse failures
 
 20130702
  - (dtucker) [contrib/cygwin/README contrib/cygwin/ssh-host-config
diff --git a/dh.c b/dh.c
index d943ca1e1..a7d0e3ab4 100644
--- a/dh.c
+++ b/dh.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: dh.c,v 1.49 2011/12/07 05:44:38 djm Exp $ */
+/* $OpenBSD: dh.c,v 1.50 2013/06/21 05:42:32 djm Exp $ */
 /*
  * Copyright (c) 2000 Niels Provos.  All rights reserved.
  *
@@ -48,6 +48,7 @@ parse_prime(int linenum, char *line, struct dhgroup *dhg)
 	const char *errstr = NULL;
 	long long n;
 
+	dhg->p = dhg-> g = NULL;
 	cp = line;
 	if ((arg = strdelim(&cp)) == NULL)
 		return 0;
@@ -59,66 +60,85 @@ parse_prime(int linenum, char *line, struct dhgroup *dhg)
 
 	/* time */
 	if (cp == NULL || *arg == '\0')
-		goto fail;
+		goto truncated;
 	arg = strsep(&cp, " "); /* type */
 	if (cp == NULL || *arg == '\0')
-		goto fail;
+		goto truncated;
 	/* Ensure this is a safe prime */
 	n = strtonum(arg, 0, 5, &errstr);
-	if (errstr != NULL || n != MODULI_TYPE_SAFE)
+	if (errstr != NULL || n != MODULI_TYPE_SAFE) {
+		error("moduli:%d: type is not %d", linenum, MODULI_TYPE_SAFE);
 		goto fail;
+	}
 	arg = strsep(&cp, " "); /* tests */
 	if (cp == NULL || *arg == '\0')
-		goto fail;
+		goto truncated;
 	/* Ensure prime has been tested and is not composite */
 	n = strtonum(arg, 0, 0x1f, &errstr);
 	if (errstr != NULL ||
-	    (n & MODULI_TESTS_COMPOSITE) || !(n & ~MODULI_TESTS_COMPOSITE))
+	    (n & MODULI_TESTS_COMPOSITE) || !(n & ~MODULI_TESTS_COMPOSITE)) {
+		error("moduli:%d: invalid moduli tests flag", linenum);
 		goto fail;
+	}
 	arg = strsep(&cp, " "); /* tries */
 	if (cp == NULL || *arg == '\0')
-		goto fail;
+		goto truncated;
 	n = strtonum(arg, 0, 1<<30, &errstr);
-	if (errstr != NULL || n == 0)
+	if (errstr != NULL || n == 0) {
+		error("moduli:%d: invalid primality trial count", linenum);
 		goto fail;
+	}
 	strsize = strsep(&cp, " "); /* size */
 	if (cp == NULL || *strsize == '\0' ||
 	    (dhg->size = (int)strtonum(strsize, 0, 64*1024, &errstr)) == 0 ||
-	    errstr)
+	    errstr) {
+		error("moduli:%d: invalid prime length", linenum);
 		goto fail;
+	}
 	/* The whole group is one bit larger */
 	dhg->size++;
 	gen = strsep(&cp, " "); /* gen */
 	if (cp == NULL || *gen == '\0')
-		goto fail;
+		goto truncated;
 	prime = strsep(&cp, " "); /* prime */
-	if (cp != NULL || *prime == '\0')
+	if (cp != NULL || *prime == '\0') {
+ truncated:
+		error("moduli:%d: truncated", linenum);
 		goto fail;
+	}
 
 	if ((dhg->g = BN_new()) == NULL)
 		fatal("parse_prime: BN_new failed");
 	if ((dhg->p = BN_new()) == NULL)
 		fatal("parse_prime: BN_new failed");
-	if (BN_hex2bn(&dhg->g, gen) == 0)
-		goto failclean;
-
-	if (BN_hex2bn(&dhg->p, prime) == 0)
-		goto failclean;
-
-	if (BN_num_bits(dhg->p) != dhg->size)
-		goto failclean;
-
-	if (BN_is_zero(dhg->g) || BN_is_one(dhg->g))
-		goto failclean;
+	if (BN_hex2bn(&dhg->g, gen) == 0) {
+		error("moduli:%d: could not parse generator value", linenum);
+		goto fail;
+	}
+	if (BN_hex2bn(&dhg->p, prime) == 0) {
+		error("moduli:%d: could not parse prime value", linenum);
+		goto fail;
+	}
+	if (BN_num_bits(dhg->p) != dhg->size) {
+		error("moduli:%d: prime has wrong size: actual %d listed %d",
+		    linenum, BN_num_bits(dhg->p), dhg->size - 1);
+		goto fail;
+	}
+	if (BN_cmp(dhg->g, BN_value_one()) <= 0) {
+		error("moduli:%d: generator is invalid", linenum);
+		goto fail;
+	}
 
-	return (1);
+	return 1;
 
- failclean:
-	BN_clear_free(dhg->g);
-	BN_clear_free(dhg->p);
  fail:
+	if (dhg->g != NULL)
+		BN_clear_free(dhg->g);
+	if (dhg->p != NULL)
+		BN_clear_free(dhg->p);
+	dhg->g = dhg->p = NULL;
 	error("Bad prime description in line %d", linenum);
-	return (0);
+	return 0;
 }
 
 DH *