From 1946c596b47b0495ce745fe2fff7da799919b0d2 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Thu, 20 Oct 2011 21:20:02 +0000 Subject: [PATCH] 3174. [bug] Always compute to revoked key tag from scratch. [RT #24711] --- CHANGES | 3 +++ bin/dnssec/dnssec-keyfromlabel.c | 5 ++-- bin/dnssec/dnssec-keygen.c | 5 ++-- bin/dnssec/dnssec-revoke.c | 12 +++++++-- bin/dnssec/dnssec-revoke.docbook | 13 ++++++++- bin/dnssec/dnssectool.c | 24 ++++++++++------- bin/dnssec/dnssectool.h | 7 ++--- bin/tests/system/autosign/ns1/keygen.sh | 4 +-- bin/tests/system/autosign/tests.sh | 10 +++---- lib/dns/dst_api.c | 22 +++++----------- lib/dns/dst_internal.h | 4 ++- lib/dns/include/dst/dst.h | 11 +++++--- lib/dns/key.c | 35 ++++++++++++++++++++++++- lib/dns/win32/libdns.def | 2 ++ lib/dns/zone.c | 5 ++-- 15 files changed, 109 insertions(+), 53 deletions(-) diff --git a/CHANGES b/CHANGES index fe77ebd85c..f05395176c 100644 --- a/CHANGES +++ b/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 diff --git a/bin/dnssec/dnssec-keyfromlabel.c b/bin/dnssec/dnssec-keyfromlabel.c index 0c7b4fde0c..51eac501aa 100644 --- 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.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) diff --git a/bin/dnssec/dnssec-keygen.c b/bin/dnssec/dnssec-keygen.c index 633610a104..1f39d2181b 100644 --- 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.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, - alg, mctx, NULL)) { + if (key_collision(key, name, directory, mctx, NULL)) { conflict = ISC_TRUE; if (null_key) { dst_key_free(&key); diff --git a/bin/dnssec/dnssec-revoke.c b/bin/dnssec/dnssec-revoke.c index e0eabc6aff..107e02dccb 100644 --- a/bin/dnssec/dnssec-revoke.c +++ b/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) diff --git a/bin/dnssec/dnssec-revoke.docbook b/bin/dnssec/dnssec-revoke.docbook index 0c74968694..933cd491d9 100644 --- a/bin/dnssec/dnssec-revoke.docbook +++ b/bin/dnssec/dnssec-revoke.docbook @@ -17,7 +17,7 @@ - PERFORMANCE OF THIS SOFTWARE. --> - + June 1, 2009 @@ -49,6 +49,7 @@ + keyfile @@ -123,6 +124,16 @@ + + + -R + + + Print the key tag of the key with the REVOKE bit set but do + not revoke the key. + + + diff --git a/bin/dnssec/dnssectool.c b/bin/dnssec/dnssectool.c index c2195822fe..31d44ce648 100644 --- a/bin/dnssec/dnssectool.c +++ b/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,18 +406,24 @@ 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) +key_collision(dst_key_t *dstkey, dns_name_t *name, const char *dir, + 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 */ + isc_uint16_t id, oldid, flags; + 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); @@ -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); - if ((diff & ~bits) == 0) { + roldid = dst_key_rid(key->key); + + 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); } - diff --git a/bin/dnssec/dnssectool.h b/bin/dnssec/dnssectool.h index 2215e7213c..54913d9a5a 100644 --- a/bin/dnssec/dnssectool.h +++ b/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, - dns_secalg_t alg, isc_mem_t *mctx, isc_boolean_t *exact); +key_collision(dst_key_t *key, dns_name_t *name, const char *dir, + isc_mem_t *mctx, isc_boolean_t *exact); + #endif /* DNSSEC_DNSSECTOOL_H */ diff --git a/bin/tests/system/autosign/ns1/keygen.sh b/bin/tests/system/autosign/ns1/keygen.sh index fcd8177bf7..26f598210d 100644 --- a/bin/tests/system/autosign/ns1/keygen.sh +++ b/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 diff --git a/bin/tests/system/autosign/tests.sh b/bin/tests/system/autosign/tests.sh index 605cd54842..e16bcc7b9a 100644 --- a/bin/tests/system/autosign/tests.sh +++ b/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=`expr $id + 128` -[ $id -gt 65535 ] && id=`expr $id % 65536 + 1` +id=`cat rev.key` $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=`expr $id + 128` -[ $id -gt 65535 ] && id=`expr $id % 65536 + 1` +id=`cat rev.key` $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` diff --git a/lib/dns/dst_api.c b/lib/dns/dst_api.c index 47d18a181f..a94abff0ee 100644 --- a/lib/dns/dst_api.c +++ b/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 && - key1->key_id != ((key2->key_id + 128) & 0xffff)) - return (ISC_FALSE); - if ((key2->key_flags & DNS_KEYFLAG_REVOKE) != 0 && - key2->key_id != ((key1->key_id + 128) & 0xffff)) + if (key1->key_id != key2->key_rid && + key1->key_rid != key2->key_id) 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); } diff --git a/lib/dns/dst_internal.h b/lib/dns/dst_internal.h index c6bae505eb..f2906d7530 100644 --- a/lib/dns/dst_internal.h +++ b/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 */ diff --git a/lib/dns/include/dst/dst.h b/lib/dns/include/dst/dst.h index d71ab1af3b..7a1d938d94 100644 --- a/lib/dns/include/dst/dst.h +++ b/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 - * given algorithm. + * Computes the (revoked) key id of the key stored in the provided + * region with the given algorithm. * * Requires: *\li "source" contains a valid, non-NULL region. diff --git a/lib/dns/key.c b/lib/dns/key.c index 24035d3934..ccac157c13 100644 --- a/lib/dns/key.c +++ b/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 @@ -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)); diff --git a/lib/dns/win32/libdns.def b/lib/dns/win32/libdns.def index 11efee00ac..a443cfa75a 100644 --- a/lib/dns/win32/libdns.def +++ b/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 diff --git a/lib/dns/zone.c b/lib/dns/zone.c index 2e41d2772f..c53bb6d65d 100644 --- a/lib/dns/zone.c +++ b/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_id(dstkey) + 128) & 0xffff)))) { + dst_key_rid(dstkey) == sig.keyid)) { result = dns_dnssec_verify2(keyname, &kfetch->dnskeyset, dstkey, ISC_FALSE, mctx, &sigrr,