2842. [func] Prevent dnssec-keygen and dnssec-keyfromlabel from

creating key files if there is a chance that the new key ID will collide with an existing one after either of the keys has been revoked. (To override this in the case of dnssec-keyfromlabel, use the -y option. dnssec-keygen will simply create a different, noncolliding key, so an override is not necessary.) [RT #20838]
2010-01-19 20:26:07 +00:00
parent 1aa8830b74
commit 8a198fa776
6 changed files with 141 additions and 39 deletions
--- a/9
+++ b/9
@@ -1,3 +1,12 @@
+2842.	[func]		Prevent dnssec-keygen and dnssec-keyfromlabel from
+			creating key files if there is a chance that the new
+			key ID will collide with an existing one after
+			either of the keys has been revoked.  (To override
+			this in the case of dnssec-keyfromlabel, use the -y
+			option.  dnssec-keygen will simply create a
+			different, noncolliding key, so an override is
+			not necessary.) [RT #20838]
+
 2841.	[func]		Added "smartsign" and improved "autosign" and
 			"dnssec" regression tests. [RT #20865]

--- a/bin/dnssec/dnssec-keyfromlabel.c
+++ b/bin/dnssec/dnssec-keyfromlabel.c
@@ -14,7 +14,7 @@
 * PERFORMANCE OF THIS SOFTWARE.
 */

-/* $Id: dnssec-keyfromlabel.c,v 1.29 2009/11/25 23:00:32 marka Exp $ */
+/* $Id: dnssec-keyfromlabel.c,v 1.30 2010/01/19 20:26:07 each Exp $ */

 /*! \file */

@@ -32,6 +32,7 @@
 #include <isc/string.h>
 #include <isc/util.h>

+#include <dns/dnssec.h>
 #include <dns/fixedname.h>
 #include <dns/keyvalues.h>
 #include <dns/log.h>
@@ -82,13 +83,14 @@ usage(void) {
 	fprintf(stderr, "    -f keyflag: KSK | REVOKE\n");
 	fprintf(stderr, "    -K directory: directory in which to place "
 			"key files\n");
-	fprintf(stderr, "    -k : generate a TYPE=KEY key\n");
+	fprintf(stderr, "    -k: generate a TYPE=KEY key\n");
 	fprintf(stderr, "    -n nametype: ZONE | HOST | ENTITY | USER | OTHER\n");
 	fprintf(stderr, "        (DNSKEY generation defaults to ZONE\n");
 	fprintf(stderr, "    -p protocol: default: 3 [dnssec]\n");
 	fprintf(stderr, "    -t type: "
 		"AUTHCONF | NOAUTHCONF | NOAUTH | NOCONF "
 		"(default: AUTHCONF)\n");
+	fprintf(stderr, "    -y: permit keys that might collide\n");
 	fprintf(stderr, "    -v verbose level\n");
 	fprintf(stderr, "Date options:\n");
 	fprintf(stderr, "    -P date/[+-]offset: set key publication date\n");
@@ -117,7 +119,7 @@ main(int argc, char **argv) {
 #endif
 	char		*classname = NULL;
 	char		*endp;
-	dst_key_t	*key = NULL, *oldkey = NULL;
+	dst_key_t	*key = NULL;
 	dns_fixedname_t	fname;
 	dns_name_t	*name;
 	isc_uint16_t	flags = 0, kskflag = 0, revflag = 0;
@@ -146,6 +148,8 @@ main(int argc, char **argv) {
 	isc_boolean_t	unsetdel = ISC_FALSE;
 	isc_boolean_t	genonly = ISC_FALSE;
 	isc_boolean_t	use_nsec3 = ISC_FALSE;
+	isc_boolean_t   avoid_collisions = ISC_TRUE;
+	isc_boolean_t	exact;
 	unsigned char	c;

 	if (argc == 1)
@@ -160,7 +164,7 @@ main(int argc, char **argv) {
 	isc_stdtime_get(&now);

 	while ((ch = isc_commandline_parse(argc, argv,
-				"3a:Cc:E:f:K:kl:n:p:t:v:FhGP:A:R:I:D:")) != -1)
+				"3a:Cc:E:f:K:kl:n:p:t:v:yFhGP:A:R:I:D:")) != -1)
 	{
 	    switch (ch) {
 		case '3':
@@ -218,6 +222,9 @@ main(int argc, char **argv) {
 			if (*endp != '\0')
 				fatal("-v must be followed by a number");
 			break;
+		case 'y':
+			avoid_collisions = ISC_FALSE;
+			break;
 		case 'G':
 			genonly = ISC_TRUE;
 			break;
@@ -502,16 +509,26 @@ main(int argc, char **argv) {
 	}

 	/*
-	 * Try to read a key with the same name, alg and id from disk.
-	 * If there is one we must return failure.
+	 * Do not overwrite an existing key.  Warn LOUDLY if there
+	 * is a risk of ID collision due to this key or another key
+	 * being revoked.
 	 */
-	ret = dst_key_fromfile(name, dst_key_id(key), alg,
-			       DST_TYPE_PRIVATE, directory, mctx, &oldkey);
-	/* do not overwrite an existing key  */
-	if (ret == ISC_R_SUCCESS) {
+	if (key_collision(dst_key_id(key), name, directory, alg, mctx, &exact))
+	{
 		isc_buffer_clear(&buf);
 		ret = dst_key_buildfilename(key, 0, directory, &buf);
-		fatal("%s: %s already exists\n", program, filename);
+		if (exact)
+			fatal("%s: %s already exists\n", program, filename);
+
+		if (avoid_collisions)
+			fatal("%s: %s could collide with another key upon "
+			      "revokation\n", program, filename);
+
+		fprintf(stderr, "%s: WARNING: Key %s could collide with "
+				"another key upon revokation.  If you plan "
+				"to revoke keys, destroy this key and "
+				"generate a different one.\n",
+				program, filename);
 	}

 	ret = dst_key_tofile(key, options, directory);
--- a/bin/dnssec/dnssec-keyfromlabel.docbook
+++ b/bin/dnssec/dnssec-keyfromlabel.docbook
@@ -17,7 +17,7 @@
 - PERFORMANCE OF THIS SOFTWARE.
 -->

-<!-- $Id: dnssec-keyfromlabel.docbook,v 1.15 2009/11/03 21:44:46 each Exp $ -->
+<!-- $Id: dnssec-keyfromlabel.docbook,v 1.16 2010/01/19 20:26:07 each Exp $ -->
 <refentry id="man.dnssec-keyfromlabel">
  <refentryinfo>
    <date>February 8, 2008</date>
@@ -63,6 +63,7 @@
      <arg><option>-R <replaceable class="parameter">date/offset</replaceable></option></arg>
      <arg><option>-t <replaceable class="parameter">type</replaceable></option></arg>
      <arg><option>-v <replaceable class="parameter">level</replaceable></option></arg>
+      <arg><option>-y</option></arg>
      <arg choice="req">name</arg>
    </cmdsynopsis>
  </refsynopsisdiv>
@@ -264,6 +265,19 @@
        </listitem>
      </varlistentry>

+      <varlistentry>
+        <term>-y</term>
+        <listitem>
+          <para>
+            Allows DNSSEC key files to be generated even if the key ID
+	    would collide with that of an existing key, in the event of
+	    either key being revoked.  (This is only safe to use if you
+            are sure you won't be using RFC 5011 trust anchor maintenance
+            with either of the keys involved.)
+          </para>
+        </listitem>
+      </varlistentry>
+
    </variablelist>
  </refsect1>

--- a/bin/dnssec/dnssec-keygen.c
+++ b/bin/dnssec/dnssec-keygen.c
@@ -29,7 +29,7 @@
 * IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
 */

-/* $Id: dnssec-keygen.c,v 1.110 2010/01/07 23:48:53 tbox Exp $ */
+/* $Id: dnssec-keygen.c,v 1.111 2010/01/19 20:26:07 each Exp $ */

 /*! \file */

@@ -47,6 +47,7 @@
 #include <isc/string.h>
 #include <isc/util.h>

+#include <dns/dnssec.h>
 #include <dns/fixedname.h>
 #include <dns/keyvalues.h>
 #include <dns/log.h>
@@ -67,11 +68,6 @@ int verbose;
 #define DEFAULT_ALGORITHM "RSASHA1"
 #define DEFAULT_NSEC3_ALGORITHM "NSEC3RSASHA1"

-static isc_boolean_t
-dsa_size_ok(int size) {
-	return (ISC_TF(size >= 512 && size <= 1024 && size % 64 == 0));
-}
-
 ISC_PLATFORM_NORETURN_PRE static void
 usage(void) ISC_PLATFORM_NORETURN_POST;

@@ -162,6 +158,11 @@ usage(void) {
 	exit (-1);
 }

+static isc_boolean_t
+dsa_size_ok(int size) {
+	return (ISC_TF(size >= 512 && size <= 1024 && size % 64 == 0));
+}
+
 static void
 progress(int p)
 {
@@ -192,7 +193,7 @@ main(int argc, char **argv) {
 	char	        *algname = NULL, *nametype = NULL, *type = NULL;
 	char		*classname = NULL;
 	char		*endp;
-	dst_key_t	*key = NULL, *oldkey;
+	dst_key_t	*key = NULL;
 	dns_fixedname_t	fname;
 	dns_name_t	*name;
 	isc_uint16_t	flags = 0, kskflag = 0, revflag = 0;
@@ -730,7 +731,6 @@ main(int argc, char **argv) {

 	do {
 		conflict = ISC_FALSE;
-		oldkey = NULL;

 		if (!quiet && show_progress) {
 			fprintf(stderr, "Generating key pair.");
@@ -818,37 +818,35 @@ main(int argc, char **argv) {
 		}

 		/*
-		 * Try to read a key with the same name, alg and id from disk.
-		 * If there is one we must continue generating a different
-		 * key unless we were asked to generate a null key, in which
-		 * case we return failure.
+		 * Do not overwrite an existing key, or create a key
+		 * if there is a risk of ID collision due to this key
+		 * or another key being revoked.
 		 */
-		ret = dst_key_fromfile(name, dst_key_id(key), alg,
-				       DST_TYPE_PRIVATE, directory,
-				       mctx, &oldkey);
-		/* do not overwrite an existing key  */
-		if (ret == ISC_R_SUCCESS) {
-			dst_key_free(&oldkey);
+		if (key_collision(dst_key_id(key), name, directory,
+				  alg, mctx, NULL)) {
 			conflict = ISC_TRUE;
-			if (null_key)
+			if (null_key) {
+                                dst_key_free(&key);
 				break;
-		}
-		if (conflict == ISC_TRUE) {
+                        }
+
 			if (verbose > 0) {
 				isc_buffer_clear(&buf);
 				dst_key_buildfilename(key, 0, directory, &buf);
 				fprintf(stderr,
-					"%s: %s already exists, "
-					"generating a new key\n",
+					"%s: %s already exists, or might "
+					"collide with another key upon "
+					"revokation.  Generating a new key\n",
 					program, filename);
 			}
+
 			dst_key_free(&key);
 		}
 	} while (conflict == ISC_TRUE);

 	if (conflict)
-		fatal("cannot generate a null key when a key with id 0 "
-		      "already exists");
+		fatal("cannot generate a null key due to possible key ID "
+		      "collision");

 	ret = dst_key_tofile(key, options, directory);
 	if (ret != ISC_R_SUCCESS) {
--- a/bin/dnssec/dnssectool.c
+++ b/bin/dnssec/dnssectool.c
@@ -15,7 +15,7 @@
 * PERFORMANCE OF THIS SOFTWARE.
 */

-/* $Id: dnssectool.c,v 1.58 2009/10/26 23:47:35 tbox Exp $ */
+/* $Id: dnssectool.c,v 1.59 2010/01/19 20:26:07 each Exp $ */

 /*! \file */

@@ -37,6 +37,8 @@
 #include <isc/util.h>
 #include <isc/print.h>

+#include <dns/dnssec.h>
+#include <dns/keyvalues.h>
 #include <dns/log.h>
 #include <dns/name.h>
 #include <dns/rdatastruct.h>
@@ -402,3 +404,61 @@ set_keyversion(dst_key_t *key) {
 		dst_key_settime(key, DST_TIME_CREATED, now);
 	}
 }
+
+isc_boolean_t
+key_collision(isc_uint16_t id, dns_name_t *name, const char *dir,
+	      dns_secalg_t alg, isc_mem_t *mctx, isc_boolean_t *exact)
+{
+	isc_result_t result;
+	isc_boolean_t conflict = ISC_FALSE;
+	dns_dnsseckeylist_t matchkeys;
+	dns_dnsseckey_t *key = NULL;
+	isc_uint16_t oldid, diff;
+        isc_uint16_t bits = DNS_KEYFLAG_REVOKE;   /* flag bits to look for */
+
+	if (exact != NULL)
+		*exact = ISC_FALSE;
+
+	ISC_LIST_INIT(matchkeys);
+	result = dns_dnssec_findmatchingkeys(name, dir, mctx, &matchkeys);
+	if (result == ISC_R_NOTFOUND)
+		return (ISC_FALSE);
+
+	while (!ISC_LIST_EMPTY(matchkeys) && !conflict) {
+		key = ISC_LIST_HEAD(matchkeys);
+		if (dst_key_alg(key->key) != alg)
+			goto next;
+
+		oldid = dst_key_id(key->key);
+		diff = (oldid > id) ? (oldid - id) : (id - oldid);
+		if ((diff & ~bits) == 0) {
+			conflict = ISC_TRUE;
+			if (diff != 0) {
+				if (verbose > 1)
+                                        fprintf(stderr, "Key ID %d could "
+						"collide with %d\n",
+						id, oldid);
+			} else {
+				if (exact != NULL)
+					*exact = ISC_TRUE;
+				if (verbose > 1)
+                                        fprintf(stderr, "Key ID %d exists\n",
+						id);
+                        }
+		}
+
+ next:
+		ISC_LIST_UNLINK(matchkeys, key, link);
+		dns_dnsseckey_destroy(mctx, &key);
+	}
+
+        /* Finish freeing the list */
+	while (!ISC_LIST_EMPTY(matchkeys)) {
+		key = ISC_LIST_HEAD(matchkeys);
+		ISC_LIST_UNLINK(matchkeys, key, link);
+		dns_dnsseckey_destroy(mctx, &key);
+	}
+
+	return (conflict);
+}
+
--- a/bin/dnssec/dnssectool.h
+++ b/bin/dnssec/dnssectool.h
@@ -15,7 +15,7 @@
 * PERFORMANCE OF THIS SOFTWARE.
 */

-/* $Id: dnssectool.h,v 1.29 2009/10/26 21:18:24 each Exp $ */
+/* $Id: dnssectool.h,v 1.30 2010/01/19 20:26:07 each Exp $ */

 #ifndef DNSSECTOOL_H
 #define DNSSECTOOL_H 1
@@ -76,4 +76,8 @@ check_keyversion(dst_key_t *key, char *keystr);

 void
 set_keyversion(dst_key_t *key);
+
+isc_boolean_t
+key_collision(isc_uint16_t id, dns_name_t *name, const char *dir,
+	      dns_secalg_t alg, isc_mem_t *mctx, isc_boolean_t *exact);
 #endif /* DNSSEC_DNSSECTOOL_H */