3174. [bug] Always compute to revoked key tag from scratch.

[RT #24711]

CHANGES

@@ -1,3 +1,6 @@
+3174.	[bug]		Always compute to revoked key tag from scratch.
+			[RT #24711]
+
 3173.	[port]		Correctly validate root DS responses. [RT #25726]
 
 3172.	[port]		darwin 10.* and freebsd [89] are now built threaded by

bin/dnssec/dnssec-keyfromlabel.c

@@ -14,7 +14,7 @@
  * PERFORMANCE OF THIS SOFTWARE.
  */
 
-/* $Id: dnssec-keyfromlabel.c,v 1.36 2011/03/18 02:16:43 marka Exp $ */
+/* $Id: dnssec-keyfromlabel.c,v 1.37 2011/10/20 21:20:01 marka Exp $ */
 
 /*! \file */
 
@@ -527,8 +527,7 @@ main(int argc, char **argv) {
 	 * is a risk of ID collision due to this key or another key
 	 * being revoked.
 	 */
-	if (key_collision(dst_key_id(key), name, directory, alg, mctx, &exact))
+	if (key_collision(key, name, directory, mctx, &exact)) {
-	{
 		isc_buffer_clear(&buf);
 		ret = dst_key_buildfilename(key, 0, directory, &buf);
 		if (ret != ISC_R_SUCCESS)

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.118 2011/03/17 01:40:34 each Exp $ */
+/* $Id: dnssec-keygen.c,v 1.119 2011/10/20 21:20:01 marka Exp $ */
 
 /*! \file */
 
@@ -977,8 +977,7 @@ main(int argc, char **argv) {
 		 * if there is a risk of ID collision due to this key
 		 * or another key being revoked.
 		 */
-		if (key_collision(dst_key_id(key), name, directory,
+		if (key_collision(key, name, directory, mctx, NULL)) {
-				  alg, mctx, NULL)) {
 			conflict = ISC_TRUE;
 			if (null_key) {
 				dst_key_free(&key);

bin/dnssec/dnssec-revoke.c

@@ -14,7 +14,7 @@
  * PERFORMANCE OF THIS SOFTWARE.
  */
 
-/* $Id: dnssec-revoke.c,v 1.22 2010/05/06 23:50:56 tbox Exp $ */
+/* $Id: dnssec-revoke.c,v 1.23 2011/10/20 21:20:01 marka Exp $ */
 
 /*! \file */
 
@@ -92,6 +92,7 @@ main(int argc, char **argv) {
 	isc_buffer_t buf;
 	isc_boolean_t force = ISC_FALSE;
 	isc_boolean_t remove = ISC_FALSE;
+	isc_boolean_t id = ISC_FALSE;
 
 	if (argc == 1)
 		usage();
@@ -104,7 +105,7 @@ main(int argc, char **argv) {
 
 	isc_commandline_errprint = ISC_FALSE;
 
-	while ((ch = isc_commandline_parse(argc, argv, "E:fK:rhv:")) != -1) {
+	while ((ch = isc_commandline_parse(argc, argv, "E:fK:rRhv:")) != -1) {
 		switch (ch) {
 		    case 'E':
 			engine = isc_commandline_argument;
@@ -126,6 +127,9 @@ main(int argc, char **argv) {
 		    case 'r':
 			remove = ISC_TRUE;
 			break;
+		    case 'R':
+			id = ISC_TRUE;
+			break;
 		    case 'v':
 			verbose = strtol(isc_commandline_argument, &endp, 0);
 			if (*endp != '\0')
@@ -186,6 +190,10 @@ main(int argc, char **argv) {
 		fatal("Invalid keyfile name %s: %s",
 		      filename, isc_result_totext(result));
 
+	if (id) {
+		fprintf(stdout, "%u\n", dst_key_rid(key));
+		goto cleanup;
+	}
 	dst_key_format(key, keystr, sizeof(keystr));
 
 	if (verbose > 2)

bin/dnssec/dnssec-revoke.docbook

@@ -17,7 +17,7 @@
  - PERFORMANCE OF THIS SOFTWARE.
 -->
 
-<!-- $Id: dnssec-revoke.docbook,v 1.7 2009/11/03 21:44:46 each Exp $ -->
+<!-- $Id: dnssec-revoke.docbook,v 1.8 2011/10/20 21:20:01 marka Exp $ -->
 <refentry id="man.dnssec-revoke">
   <refentryinfo>
     <date>June 1, 2009</date>
@@ -49,6 +49,7 @@
       <arg><option>-K <replaceable class="parameter">directory</replaceable></option></arg>
       <arg><option>-E <replaceable class="parameter">engine</replaceable></option></arg>
       <arg><option>-f</option></arg>
+      <arg><option>-R</option></arg>
       <arg choice="req">keyfile</arg>
     </cmdsynopsis>
   </refsynopsisdiv>
@@ -123,6 +124,16 @@
           </para>
         </listitem>
       </varlistentry>
+
+      <varlistentry>
+        <term>-R</term>
+        <listitem>
+          <para>
+	    Print the key tag of the key with the REVOKE bit set but do
+	    not revoke the key.
+          </para>
+        </listitem>
+      </varlistentry>
     </variablelist>
   </refsect1>
 

bin/dnssec/dnssectool.c

@@ -15,7 +15,7 @@
  * PERFORMANCE OF THIS SOFTWARE.
  */
 
-/* $Id: dnssectool.c,v 1.60 2010/01/19 23:48:56 tbox Exp $ */
+/* $Id: dnssectool.c,v 1.61 2011/10/20 21:20:01 marka Exp $ */
 
 /*! \file */
 
@@ -406,19 +406,25 @@ set_keyversion(dst_key_t *key) {
 }
 
 isc_boolean_t
-key_collision(isc_uint16_t id, dns_name_t *name, const char *dir,
+key_collision(dst_key_t *dstkey, dns_name_t *name, const char *dir,
-	      dns_secalg_t alg, isc_mem_t *mctx, isc_boolean_t *exact)
+	      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 id, oldid, flags;
-	isc_uint16_t bits = DNS_KEYFLAG_REVOKE;   /* flag bits to look for */
+	isc_uint32_t rid, roldid;
+	dns_secalg_t alg;
 
 	if (exact != NULL)
 		*exact = ISC_FALSE;
  
+	id = dst_key_id(dstkey);
+	rid = dst_key_rid(dstkey);
+	alg = dst_key_alg(dstkey);
+	flags = dst_key_flags(dstkey);
+
 	ISC_LIST_INIT(matchkeys);
 	result = dns_dnssec_findmatchingkeys(name, dir, mctx, &matchkeys);
 	if (result == ISC_R_NOTFOUND)
@@ -430,10 +436,11 @@ key_collision(isc_uint16_t id, dns_name_t *name, const char *dir,
 			goto next;
 
 		oldid = dst_key_id(key->key);
-		diff = (oldid > id) ? (oldid - id) : (id - oldid);
+		roldid = dst_key_rid(key->key);
-		if ((diff & ~bits) == 0) {
+
+		if (oldid == rid || roldid == id || id == oldid) {
 			conflict = ISC_TRUE;
-			if (diff != 0) {
+			if (id != oldid) {
 				if (verbose > 1)
 					fprintf(stderr, "Key ID %d could "
 						"collide with %d\n",
@@ -461,4 +468,3 @@ key_collision(isc_uint16_t id, dns_name_t *name, const char *dir,
 
 	return (conflict);
 }
-

bin/dnssec/dnssectool.h

@@ -15,7 +15,7 @@
  * PERFORMANCE OF THIS SOFTWARE.
  */
 
-/* $Id: dnssectool.h,v 1.31 2010/01/19 23:48:56 tbox Exp $ */
+/* $Id: dnssectool.h,v 1.32 2011/10/20 21:20:01 marka Exp $ */
 
 #ifndef DNSSECTOOL_H
 #define DNSSECTOOL_H 1
@@ -78,6 +78,7 @@ void
 set_keyversion(dst_key_t *key);
 
 isc_boolean_t
-key_collision(isc_uint16_t id, dns_name_t *name, const char *dir,
+key_collision(dst_key_t *key, dns_name_t *name, const char *dir,
-	      dns_secalg_t alg, isc_mem_t *mctx, isc_boolean_t *exact);
+	      isc_mem_t *mctx, isc_boolean_t *exact);
+
 #endif /* DNSSEC_DNSSECTOOL_H */

bin/tests/system/autosign/ns1/keygen.sh

@@ -14,7 +14,7 @@
 # OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 # PERFORMANCE OF THIS SOFTWARE.
 
-# $Id: keygen.sh,v 1.6 2010/01/18 23:48:40 tbox Exp $
+# $Id: keygen.sh,v 1.7 2011/10/20 21:20:01 marka Exp $
 
 SYSTEMTESTTOP=../..
 . $SYSTEMTESTTOP/conf.sh
@@ -72,4 +72,4 @@ echo $zskinact > ../inact.key
 echo $zskunpub > ../unpub.key
 echo $zsknopriv > ../nopriv.key
 echo $zsksby > ../standby.key
-echo $kskrev > ../rev.key
+$REVOKE -R $kskrev > ../rev.key

bin/tests/system/autosign/tests.sh

@@ -14,7 +14,7 @@
 # OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 # PERFORMANCE OF THIS SOFTWARE.
 
-# $Id: tests.sh,v 1.35 2011/10/15 05:00:15 marka Exp $
+# $Id: tests.sh,v 1.36 2011/10/20 21:20:01 marka Exp $
 
 SYSTEMTESTTOP=..
 . $SYSTEMTESTTOP/conf.sh
@@ -699,9 +699,7 @@ status=`expr $status + $ret`
 
 echo "I:checking that revoked key is present ($n)"
 ret=0
-id=`sed 's/^K.+007+0*\([0-9]\)/\1/' < rev.key`
+id=`cat rev.key`
-id=`expr $id + 128`
-[ $id -gt 65535 ] && id=`expr $id % 65536 + 1`
 $DIG $DIGOPTS +multi dnskey . @10.53.0.1 > dig.out.ns1.test$n || ret=1
 grep '; key id = '"$id"'$' dig.out.ns1.test$n > /dev/null || ret=1
 n=`expr $n + 1`
@@ -710,9 +708,7 @@ status=`expr $status + $ret`
 
 echo "I:checking that revoked key self-signs ($n)"
 ret=0
-id=`sed 's/^K.+007+0*\([0-9]\)/\1/' < rev.key`
+id=`cat rev.key`
-id=`expr $id + 128`
-[ $id -gt 65535 ] && id=`expr $id % 65536 + 1`
 $DIG $DIGOPTS dnskey . @10.53.0.1 > dig.out.ns1.test$n || ret=1
 grep 'RRSIG.*'" $id "'\. ' dig.out.ns1.test$n > /dev/null || ret=1
 n=`expr $n + 1`

lib/dns/dst_api.c

@@ -31,7 +31,7 @@
 
 /*
  * Principal Author: Brian Wellington
- * $Id: dst_api.c,v 1.64 2011/09/05 18:00:22 each Exp $
+ * $Id: dst_api.c,v 1.65 2011/10/20 21:20:02 marka Exp $
  */
 
 /*! \file */
@@ -448,7 +448,6 @@ dst_key_fromfile(dns_name_t *name, dns_keytag_t id,
 		dst_key_free(&key);
 		return (DST_R_INVALIDPRIVATEKEY);
 	}
-	key->key_id = id;
 
 	*keyp = key;
 	return (ISC_R_SUCCESS);
@@ -599,7 +598,7 @@ dst_key_fromdns(dns_name_t *name, dns_rdataclass_t rdclass,
 	isc_uint8_t alg, proto;
 	isc_uint32_t flags, extflags;
 	dst_key_t *key = NULL;
-	dns_keytag_t id;
+	dns_keytag_t id, rid;
 	isc_region_t r;
 	isc_result_t result;
 
@@ -614,6 +613,7 @@ dst_key_fromdns(dns_name_t *name, dns_rdataclass_t rdclass,
 	alg = isc_buffer_getuint8(source);
 
 	id = dst_region_computeid(&r, alg);
+	rid = dst_region_computerid(&r, alg);
 
 	if (flags & DNS_KEYFLAG_EXTENDED) {
 		if (isc_buffer_remaininglength(source) < 2)
@@ -627,6 +627,7 @@ dst_key_fromdns(dns_name_t *name, dns_rdataclass_t rdclass,
 	if (result != ISC_R_SUCCESS)
 		return (result);
 	key->key_id = id;
+	key->key_rid = rid;
 
 	*keyp = key;
 	return (ISC_R_SUCCESS);
@@ -928,13 +929,6 @@ comparekeys(const dst_key_t *key1, const dst_key_t *key2,
 	if (key1->key_alg != key2->key_alg)
 		return (ISC_FALSE);
 
-	/*
-	 * For all algorithms except RSAMD5, revoking the key
-	 * changes the key ID, increasing it by 128.  If we want to
-	 * be able to find matching keys even if one of them is the
-	 * revoked version of the other one, then we need to check
-	 * for that possibility.
-	 */
 	if (key1->key_id != key2->key_id) {
 		if (!match_revoked_key)
 			return (ISC_FALSE);
@@ -943,11 +937,8 @@ comparekeys(const dst_key_t *key1, const dst_key_t *key2,
 		if ((key1->key_flags & DNS_KEYFLAG_REVOKE) ==
 		    (key2->key_flags & DNS_KEYFLAG_REVOKE))
 			return (ISC_FALSE);
-		if ((key1->key_flags & DNS_KEYFLAG_REVOKE) != 0 &&
+		if (key1->key_id != key2->key_rid &&
-		    key1->key_id != ((key2->key_id + 128) & 0xffff))
+		    key1->key_rid != key2->key_id)
-			return (ISC_FALSE);
-		if ((key2->key_flags & DNS_KEYFLAG_REVOKE) != 0 &&
-		    key2->key_id != ((key1->key_id + 128) & 0xffff))
 			return (ISC_FALSE);
 	}
 
@@ -1652,6 +1643,7 @@ computeid(dst_key_t *key) {
 
 	isc_buffer_usedregion(&dnsbuf, &r);
 	key->key_id = dst_region_computeid(&r, key->key_alg);
+	key->key_rid = dst_region_computerid(&r, key->key_alg);
 	return (ISC_R_SUCCESS);
 }
 

lib/dns/dst_internal.h

@@ -29,7 +29,7 @@
  * IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
  */
 
-/* $Id: dst_internal.h,v 1.30 2011/03/17 01:40:39 each Exp $ */
+/* $Id: dst_internal.h,v 1.31 2011/10/20 21:20:02 marka Exp $ */
 
 #ifndef DST_DST_INTERNAL_H
 #define DST_DST_INTERNAL_H 1
@@ -94,6 +94,8 @@ struct dst_key {
 	unsigned int	key_alg;	/*%< algorithm of the key */
 	isc_uint32_t	key_flags;	/*%< flags of the public key */
 	isc_uint16_t	key_id;		/*%< identifier of the key */
+	isc_uint16_t	key_rid;	/*%< identifier of the key when
+					     revoked */
 	isc_uint16_t	key_bits;	/*%< hmac digest bits */
 	dns_rdataclass_t key_class;	/*%< class of the key record */
 	dns_ttl_t	key_ttl;	/*%< default/initial dnskey ttl */

lib/dns/include/dst/dst.h

@@ -15,7 +15,7 @@
  * PERFORMANCE OF THIS SOFTWARE.
  */
 
-/* $Id: dst.h,v 1.33 2011/03/21 19:54:03 each Exp $ */
+/* $Id: dst.h,v 1.34 2011/10/20 21:20:02 marka Exp $ */
 
 #ifndef DST_DST_H
 #define DST_DST_H 1
@@ -641,6 +641,9 @@ dst_key_flags(const dst_key_t *key);
 dns_keytag_t
 dst_key_id(const dst_key_t *key);
 
+dns_keytag_t
+dst_key_rid(const dst_key_t *key);
+
 dns_rdataclass_t
 dst_key_class(const dst_key_t *key);
 
@@ -706,9 +709,11 @@ dst_key_secretsize(const dst_key_t *key, unsigned int *n);
 
 isc_uint16_t
 dst_region_computeid(const isc_region_t *source, unsigned int alg);
+isc_uint16_t
+dst_region_computerid(const isc_region_t *source, unsigned int alg);
 /*%<
- * Computes the key id of the key stored in the provided region with the
+ * Computes the (revoked) key id of the key stored in the provided
- * given algorithm.
+ * region with the given algorithm.
  *
  * Requires:
  *\li	"source" contains a valid, non-NULL region.

lib/dns/key.c

@@ -15,7 +15,7 @@
  * PERFORMANCE OF THIS SOFTWARE.
  */
 
-/* $Id: key.c,v 1.10 2011/03/17 23:47:30 tbox Exp $ */
+/* $Id: key.c,v 1.11 2011/10/20 21:20:02 marka Exp $ */
 
 #include <config.h>
 
@@ -56,6 +56,33 @@ dst_region_computeid(const isc_region_t *source, unsigned int alg) {
 	return ((isc_uint16_t)(ac & 0xffff));
 }
 
+isc_uint16_t
+dst_region_computerid(const isc_region_t *source, unsigned int alg) {
+	isc_uint32_t ac;
+	const unsigned char *p;
+	int size;
+
+	REQUIRE(source != NULL);
+	REQUIRE(source->length >= 4);
+
+	p = source->base;
+	size = source->length;
+
+	if (alg == DST_ALG_RSAMD5)
+		return ((p[size - 3] << 8) + p[size - 2]);
+
+	ac = ((*p) << 8) + *(p + 1);
+	ac |= DNS_KEYFLAG_REVOKE;
+	for (size -= 2, p +=2; size > 1; size -= 2, p += 2)
+		ac += ((*p) << 8) + *(p + 1);
+
+	if (size > 0)
+		ac += ((*p) << 8);
+	ac += (ac >> 16) & 0xffff;
+
+	return ((isc_uint16_t)(ac & 0xffff));
+}
+
 dns_name_t *
 dst_key_name(const dst_key_t *key) {
 	REQUIRE(VALID_KEY(key));
@@ -92,6 +119,12 @@ dst_key_id(const dst_key_t *key) {
 	return (key->key_id);
 }
 
+dns_keytag_t
+dst_key_rid(const dst_key_t *key) {
+	REQUIRE(VALID_KEY(key));
+	return (key->key_rid);
+}
+
 dns_rdataclass_t
 dst_key_class(const dst_key_t *key) {
 	REQUIRE(VALID_KEY(key));

lib/dns/win32/libdns.def

@@ -977,6 +977,7 @@ dst_key_paramcompare
 dst_key_proto
 dst_key_pubcompare
 dst_key_restore
+dst_key_rid
 dst_key_secretsize
 dst_key_setbits
 dst_key_setflags
@@ -994,6 +995,7 @@ dst_lib_init
 dst_lib_init2
 dst_lib_initmsgcat
 dst_region_computeid
+dst_region_computerid
 dst_result_register
 dst_result_totext
 

lib/dns/zone.c

@@ -15,7 +15,7 @@
  * PERFORMANCE OF THIS SOFTWARE.
  */
 
-/* $Id: zone.c,v 1.635 2011/10/12 23:46:34 tbox Exp $ */
+/* $Id: zone.c,v 1.636 2011/10/20 21:20:02 marka Exp $ */
 
 /*! \file */
 
@@ -7600,8 +7600,7 @@ revocable(dns_keyfetch_t *kfetch, dns_rdata_keydata_t *keydata) {
 
 		if (dst_key_alg(dstkey) == sig.algorithm &&
 		    (dst_key_id(dstkey) == sig.keyid ||
-		     (sig.algorithm != 1 && sig.keyid ==
+		     dst_key_rid(dstkey) == sig.keyid)) {
-		       ((dst_key_id(dstkey) + 128) & 0xffff)))) {
 			result = dns_dnssec_verify2(keyname,
 					    &kfetch->dnskeyset,
 					    dstkey, ISC_FALSE, mctx, &sigrr,