Commit 53c22b8e authored by Evan Hunt's avatar Evan Hunt
Browse files

2685. [bug] Fixed dnssec-signzone -S handling of revoked keys.

			Also, added warnings when revoking a ZSK, as this is
			not defined by protocol (but is legal).  [RT #19943]
parent 4d0e2cf9
2685. [bug] Fixed dnssec-signzone -S handling of revoked keys.
Also, added warnings when revoking a ZSK, as this is
not defined by protocol (but is legal). [RT #19943]
2684. [bug] dnssec-signzone should clean the old NSEC chain when
signing with NSEC3 and vica versa. [RT #20301]
......
......@@ -14,7 +14,7 @@
* PERFORMANCE OF THIS SOFTWARE.
*/
/* $Id: dnssec-keyfromlabel.c,v 1.14 2009/09/14 18:45:45 each Exp $ */
/* $Id: dnssec-keyfromlabel.c,v 1.15 2009/09/23 16:01:56 each Exp $ */
/*! \file */
......@@ -406,8 +406,15 @@ main(int argc, char **argv) {
else if (!genonly)
dst_key_settime(key, DST_TIME_ACTIVATE, now);
if (setrev)
if (setrev) {
if (kskflag == 0)
fprintf(stderr, "%s: warning: Key is "
"not flagged as a KSK, but -R "
"was used. Revoking a ZSK is "
"legal, but undefined.\n",
program);
dst_key_settime(key, DST_TIME_REVOKE, revoke);
}
if (setinact)
dst_key_settime(key, DST_TIME_INACTIVE, inactive);
......
......@@ -29,7 +29,7 @@
* IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
*/
/* $Id: dnssec-keygen.c,v 1.95 2009/09/14 18:45:45 each Exp $ */
/* $Id: dnssec-keygen.c,v 1.96 2009/09/23 16:01:56 each Exp $ */
/*! \file */
......@@ -686,17 +686,24 @@ main(int argc, char **argv) {
if (setpub)
dst_key_settime(key, DST_TIME_PUBLISH, publish);
else if (!genonly)
else if (!genonly && !setact)
dst_key_settime(key, DST_TIME_PUBLISH, now);
if (setact)
dst_key_settime(key, DST_TIME_ACTIVATE,
activate);
else if (!genonly)
else if (!genonly && !setpub)
dst_key_settime(key, DST_TIME_ACTIVATE, now);
if (setrev)
if (setrev) {
if (kskflag == 0)
fprintf(stderr, "%s: warning: Key is "
"not flagged as a KSK, but -R "
"was used. Revoking a ZSK is "
"legal, but undefined.\n",
program);
dst_key_settime(key, DST_TIME_REVOKE, revoke);
}
if (setinact)
dst_key_settime(key, DST_TIME_INACTIVE,
......
......@@ -14,7 +14,7 @@
* PERFORMANCE OF THIS SOFTWARE.
*/
/* $Id: dnssec-revoke.c,v 1.11 2009/09/04 16:57:22 each Exp $ */
/* $Id: dnssec-revoke.c,v 1.12 2009/09/23 16:01:56 each Exp $ */
/*! \file */
......@@ -171,6 +171,13 @@ main(int argc, char **argv) {
if ((flags & DNS_KEYFLAG_REVOKE) == 0) {
isc_stdtime_t now;
if ((flags & DNS_KEYFLAG_KSK) == 0)
fprintf(stderr, "%s: warning: Key is not flagged "
"as a KSK. Revoking a ZSK is "
"legal, but undefined.\n",
program);
isc_stdtime_get(&now);
dst_key_settime(key, DST_TIME_REVOKE, now);
......
......@@ -14,7 +14,7 @@
* PERFORMANCE OF THIS SOFTWARE.
*/
/* $Id: dnssec-settime.c,v 1.12 2009/09/14 18:45:45 each Exp $ */
/* $Id: dnssec-settime.c,v 1.13 2009/09/23 16:01:56 each Exp $ */
/*! \file */
......@@ -365,6 +365,11 @@ main(int argc, char **argv) {
"revoked; changing the revocation date "
"will not affect this.\n",
program, keystr);
if ((dst_key_flags(key) & DNS_KEYFLAG_KSK) == 0)
fprintf(stderr, "%s: warning: Key %s is not flagged as "
"a KSK, but -R was used. Revoking a "
"ZSK is legal, but undefined.\n",
program, keystr);
dst_key_settime(key, DST_TIME_REVOKE, rev);
} else if (unsetrev) {
if ((dst_key_flags(key) & DNS_KEYFLAG_REVOKE) != 0)
......
......@@ -29,7 +29,7 @@
* IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
*/
/* $Id: dnssec-signzone.c,v 1.231 2009/09/23 14:05:11 marka Exp $ */
/* $Id: dnssec-signzone.c,v 1.232 2009/09/23 16:01:56 each Exp $ */
/*! \file */
......@@ -271,13 +271,13 @@ static dns_dnsseckey_t *
keythatsigned_unlocked(dns_rdata_rrsig_t *rrsig) {
dns_dnsseckey_t *key;
key = ISC_LIST_HEAD(keylist);
while (key != NULL) {
for (key = ISC_LIST_HEAD(keylist);
key != NULL;
key = ISC_LIST_NEXT(key, link)) {
if (rrsig->keyid == dst_key_id(key->key) &&
rrsig->algorithm == dst_key_alg(key->key) &&
dns_name_equal(&rrsig->signer, dst_key_name(key->key)))
return (key);
key = ISC_LIST_NEXT(key, link);
}
return (NULL);
}
......@@ -327,13 +327,11 @@ keythatsigned(dns_rdata_rrsig_t *rrsig) {
if (result == ISC_R_SUCCESS) {
dst_key_free(&pubkey);
dns_dnsseckey_create(mctx, &privkey, &key);
key->force_publish = ISC_TRUE;
key->force_sign = ISC_FALSE;
} else {
dns_dnsseckey_create(mctx, &pubkey, &key);
key->force_publish = ISC_TRUE;
key->force_sign = ISC_FALSE;
}
key->force_publish = ISC_TRUE;
key->force_sign = ISC_FALSE;
ISC_LIST_APPEND(keylist, key, link);
isc_rwlock_unlock(&keylist_lock, isc_rwlocktype_write);
......@@ -372,11 +370,11 @@ expecttofindkey(dns_name_t *name) {
}
static inline isc_boolean_t
setverifies(dns_name_t *name, dns_rdataset_t *set, dns_dnsseckey_t *key,
setverifies(dns_name_t *name, dns_rdataset_t *set, dst_key_t *key,
dns_rdata_t *rrsig)
{
isc_result_t result;
result = dns_dnssec_verify(name, set, key->key, ISC_FALSE, mctx, rrsig);
result = dns_dnssec_verify(name, set, key, ISC_FALSE, mctx, rrsig);
if (result == ISC_R_SUCCESS) {
INCSTAT(nverified);
return (ISC_TRUE);
......@@ -479,8 +477,8 @@ signset(dns_diff_t *del, dns_diff_t *add, dns_dbnode_t *node, dns_name_t *name,
if (!expired)
keep = ISC_TRUE;
} else if (issigningkey(key)) {
if (!expired && setverifies(name, set, key, &sigrdata))
{
if (!expired && setverifies(name, set, key->key,
&sigrdata)) {
vbprintf(2, "\trrsig by %s retained\n", sigstr);
keep = ISC_TRUE;
wassignedby[key->index] = ISC_TRUE;
......@@ -494,8 +492,8 @@ signset(dns_diff_t *del, dns_diff_t *add, dns_dbnode_t *node, dns_name_t *name,
resign = ISC_TRUE;
}
} else if (iszonekey(key)) {
if (!expired && setverifies(name, set, key, &sigrdata))
{
if (!expired && setverifies(name, set, key->key,
&sigrdata)) {
vbprintf(2, "\trrsig by %s retained\n", sigstr);
keep = ISC_TRUE;
wassignedby[key->index] = ISC_TRUE;
......@@ -1443,8 +1441,10 @@ verifyzone(void) {
isc_boolean_t goodksk = ISC_FALSE;
isc_boolean_t goodzsk = ISC_FALSE;
isc_result_t result;
unsigned char revoked[256];
unsigned char standby[256];
unsigned char revoked_ksk[256];
unsigned char revoked_zsk[256];
unsigned char standby_ksk[256];
unsigned char standby_zsk[256];
unsigned char ksk_algorithms[256];
unsigned char zsk_algorithms[256];
unsigned char bad_algorithms[256];
......@@ -1473,8 +1473,10 @@ verifyzone(void) {
if (!dns_rdataset_isassociated(&sigrdataset))
fatal("cannot find DNSKEY RRSIGs\n");
memset(revoked, 0, sizeof(revoked));
memset(standby, 0, sizeof(revoked));
memset(revoked_ksk, 0, sizeof(revoked_ksk));
memset(revoked_zsk, 0, sizeof(revoked_zsk));
memset(standby_ksk, 0, sizeof(standby_ksk));
memset(standby_zsk, 0, sizeof(standby_zsk));
memset(ksk_algorithms, 0, sizeof(ksk_algorithms));
memset(zsk_algorithms, 0, sizeof(zsk_algorithms));
memset(bad_algorithms, 0, sizeof(bad_algorithms));
......@@ -1514,8 +1516,11 @@ verifyzone(void) {
(int)isc_buffer_usedlength(&buf), buffer);
}
if ((dnskey.flags & DNS_KEYFLAG_KSK) != 0 &&
revoked[dnskey.algorithm] != 255)
revoked[dnskey.algorithm]++;
revoked_ksk[dnskey.algorithm] != 255)
revoked_ksk[dnskey.algorithm]++;
else if ((dnskey.flags & DNS_KEYFLAG_KSK) == 0 &&
revoked_zsk[dnskey.algorithm] != 255)
revoked_zsk[dnskey.algorithm]++;
} else if ((dnskey.flags & DNS_KEYFLAG_KSK) != 0) {
if (dns_dnssec_selfsigns(&rdata, gorigin, &rdataset,
&sigrdataset, ISC_FALSE, mctx)) {
......@@ -1523,8 +1528,8 @@ verifyzone(void) {
ksk_algorithms[dnskey.algorithm]++;
goodksk = ISC_TRUE;
} else {
if (standby[dnskey.algorithm] != 255)
standby[dnskey.algorithm]++;
if (standby_ksk[dnskey.algorithm] != 255)
standby_ksk[dnskey.algorithm]++;
}
} else if (dns_dnssec_selfsigns(&rdata, gorigin, &rdataset,
&sigrdataset, ISC_FALSE,
......@@ -1537,8 +1542,8 @@ verifyzone(void) {
zsk_algorithms[dnskey.algorithm]++;
goodzsk = ISC_TRUE;
} else {
if (zsk_algorithms[dnskey.algorithm] != 255)
zsk_algorithms[dnskey.algorithm]++;
if (standby_zsk[dnskey.algorithm] != 255)
standby_zsk[dnskey.algorithm]++;
#ifdef ALLOW_KSKLESS_ZONES
allzsksigned = ISC_FALSE;
#endif
......@@ -1686,13 +1691,18 @@ verifyzone(void) {
for (i = 0; i < 256; i++) {
if ((zsk_algorithms[i] != 0) ||
(ksk_algorithms[i] != 0) ||
(revoked[i] != 0) || (standby[i] != 0)) {
(standby_zsk[i] != 0) || (standby_ksk[i] != 0) ||
(revoked_ksk[i] != 0) || (revoked_zsk[i] != 0)) {
alg_format(i, algbuf, sizeof(algbuf));
fprintf(stderr, "Algorithm: %s: ZSKs: %u, "
"KSKs: %u active, %u revoked, %u "
"stand-by\n", algbuf,
zsk_algorithms[i], ksk_algorithms[i],
revoked[i], standby[i]);
fprintf(stderr, "Algorithm: %s: KSKs: "
"%u active, %u stand-by, %u revoked\n",
algbuf, ksk_algorithms[i],
standby_ksk[i], revoked_ksk[i]);
fprintf(stderr, "%*sZSKs: "
"%u active, %u stand-by, %u revoked\n",
(int) strlen(algbuf) + 13, "",
zsk_algorithms[i],
standby_zsk[i], revoked_zsk[i]);
}
}
}
......@@ -2623,8 +2633,10 @@ loadzonekeys(dns_db_t *db) {
dns_dnsseckey_t *key = NULL;
dns_dnsseckey_create(mctx, &keys[i], &key);
key->force_publish = ISC_TRUE;
key->force_sign = dst_key_isprivate(key->key);
if (key->legacy) {
key->force_publish = ISC_TRUE;
key->force_sign = dst_key_isprivate(key->key);
}
key->source = dns_keysource_zoneapex;
ISC_LIST_APPEND(keylist, key, link);
}
......@@ -2680,8 +2692,8 @@ loadzonepubkeys(dns_db_t *db) {
}
dns_dnsseckey_create(mctx, &pubkey, &key);
key->force_publish = ISC_TRUE;
key->force_sign = ISC_FALSE;
if (key->legacy)
key->force_publish = ISC_TRUE;
ISC_LIST_APPEND(keylist, key, link);
next:
result = dns_rdataset_next(&rdataset);
......@@ -2748,18 +2760,20 @@ build_final_keylist(dns_db_t *db, const char *directory, isc_mem_t *mctx) {
* - If so, and if the metadata says it should be removed:
* remove it from keylist and from the DNSKEY set
* - Otherwise, make sure keylist has up-to-date metadata
*
* (XXXEACH: logic is needed to make sure revoked keys
* can be matched correctly with nonrevoked)
*/
key1 = ISC_LIST_HEAD(matchkeys);
while (key1 != NULL) {
isc_boolean_t key_revoked = ISC_FALSE;
for (key2 = ISC_LIST_HEAD(keylist);
key2 != NULL;
key2 = ISC_LIST_NEXT(key2, link)) {
if (dst_key_compare(key1->key, key2->key))
if (dst_key_pubcompare(key1->key, key2->key,
ISC_TRUE)) {
key_revoked = ISC_TF(dst_key_flags(key1->key) !=
dst_key_flags(key2->key));
break;
}
}
/*
......@@ -2794,6 +2808,47 @@ build_final_keylist(dns_db_t *db, const char *directory, isc_mem_t *mctx) {
&dnskey, &tuple);
check_result(result, "dns_difftuple_create");
dns_diff_append(&del, &tuple);
} else if (key_revoked &&
(dst_key_flags(key1->key) & DNS_KEYFLAG_REVOKE) != 0) {
dns_dnsseckey_t *next;
/*
* A key in the DNSKEY set has been revoked in the
* key repository. We need to remove the old
* version and pull in the new one.
*/
make_dnskey(key2->key, &dnskey);
alg_format(dst_key_alg(key2->key), alg, sizeof(alg));
fprintf(stderr, "Replacing revoked key %d/%s in "
"DNSKEY RRset.\n",
dst_key_id(key2->key), alg);
result = dns_difftuple_create(mctx, DNS_DIFFOP_DEL,
gorigin, keyttl,
&dnskey, &tuple);
check_result(result, "dns_difftuple_create");
dns_diff_append(&del, &tuple);
ISC_LIST_UNLINK(keylist, key2, link);
dns_dnsseckey_destroy(mctx, &key2);
next = ISC_LIST_NEXT(key1, link);
ISC_LIST_UNLINK(matchkeys, key1, link);
ISC_LIST_APPEND(keylist, key1, link);
/*
* XXX: The revoke flag is only defined for trust
* anchors. Setting the flag on a non-KSK is legal,
* but not defined in any RFC. It seems reasonable
* to treat it the same as a KSK: keep it in the
* zone and sign the DNSKEY set with it, but not
* sign other records with it.
*/
if (iszsk(key1))
key1->ksk = ISC_TRUE;
key1 = next;
continue;
} else {
key2->hint_publish = key1->hint_publish;
key2->hint_sign = key1->hint_sign;
......@@ -3575,51 +3630,51 @@ main(int argc, char *argv[]) {
ISC_LIST_INIT(keylist);
isc_rwlock_init(&keylist_lock, 0, 0);
if (argc == 0) {
if (argc == 0)
loadzonekeys(gdb);
} else {
for (i = 0; i < argc; i++) {
dst_key_t *newkey = NULL;
result = dst_key_fromnamedfile(argv[i], directory,
DST_TYPE_PUBLIC |
DST_TYPE_PRIVATE,
mctx, &newkey);
if (result != ISC_R_SUCCESS)
fatal("cannot load dnskey %s: %s", argv[i],
isc_result_totext(result));
for (i = 0; i < argc; i++) {
dst_key_t *newkey = NULL;
if (!dns_name_equal(gorigin, dst_key_name(newkey)))
fatal("key %s not at origin\n", argv[i]);
key = ISC_LIST_HEAD(keylist);
while (key != NULL) {
dst_key_t *dkey = key->key;
if (dst_key_id(dkey) == dst_key_id(newkey) &&
dst_key_alg(dkey) == dst_key_alg(newkey) &&
dns_name_equal(dst_key_name(dkey),
dst_key_name(newkey)))
{
if (!dst_key_isprivate(dkey))
fatal("cannot sign zone with "
"non-private dnskey %s",
argv[i]);
break;
}
key = ISC_LIST_NEXT(key, link);
result = dst_key_fromnamedfile(argv[i], directory,
DST_TYPE_PUBLIC |
DST_TYPE_PRIVATE,
mctx, &newkey);
if (result != ISC_R_SUCCESS)
fatal("cannot load dnskey %s: %s", argv[i],
isc_result_totext(result));
if (!dns_name_equal(gorigin, dst_key_name(newkey)))
fatal("key %s not at origin\n", argv[i]);
/* Skip any duplicates */
for (key = ISC_LIST_HEAD(keylist);
key != NULL;
key = ISC_LIST_NEXT(key, link)) {
dst_key_t *dkey = key->key;
if (dst_key_id(dkey) == dst_key_id(newkey) &&
dst_key_alg(dkey) == dst_key_alg(newkey) &&
dns_name_equal(dst_key_name(dkey), gorigin)) {
if (!dst_key_isprivate(dkey))
fatal("cannot sign zone with "
"non-private dnskey %s",
argv[i]);
break;
}
if (key == NULL) {
dns_dnsseckey_create(mctx, &newkey, &key);
key->force_publish = ISC_TRUE;
key->force_sign = ISC_TRUE;
key->source = dns_keysource_user;
ISC_LIST_APPEND(keylist, key, link);
} else
dst_key_free(&newkey);
}
if (key == NULL) {
/* We haven't seen this key before */
dns_dnsseckey_create(mctx, &newkey, &key);
key->force_publish = ISC_TRUE;
key->force_sign = ISC_TRUE;
key->source = dns_keysource_user;
ISC_LIST_APPEND(keylist, key, link);
} else
dst_key_free(&newkey);
}
if (argc != 0)
loadzonepubkeys(gdb);
}
for (i = 0; i < ndskeys; i++) {
dst_key_t *newkey = NULL;
......@@ -3635,32 +3690,34 @@ main(int argc, char *argv[]) {
if (!dns_name_equal(gorigin, dst_key_name(newkey)))
fatal("key %s not at origin\n", dskeyfile[i]);
key = ISC_LIST_HEAD(keylist);
while (key != NULL) {
/* Skip any duplicates */
for (key = ISC_LIST_HEAD(keylist);
key != NULL;
key = ISC_LIST_NEXT(key, link)) {
dst_key_t *dkey = key->key;
if (dst_key_id(dkey) == dst_key_id(newkey) &&
dst_key_alg(dkey) == dst_key_alg(newkey) &&
dns_name_equal(dst_key_name(dkey),
dst_key_name(newkey)))
{
/* Override key flags. */
dns_name_equal(dst_key_name(dkey), gorigin)) {
/*
* Key was already in keylist, but we
* must make sure it has the right
* dnsseckey flags.
*/
key->ksk = ISC_TRUE;
key->force_publish = ISC_TRUE;
key->force_sign = ISC_TRUE;
key->source = dns_keysource_user;
key->ksk = ISC_TRUE;
dst_key_free(&dkey);
key->key = newkey;
dst_key_free(&newkey);
break;
}
key = ISC_LIST_NEXT(key, link);
}
if (key == NULL) {
/* Override dnskey flags. */
/* We haven't seen this key before */
dns_dnsseckey_create(mctx, &newkey, &key);
key->ksk = ISC_TRUE;
key->force_publish = ISC_TRUE;
key->force_sign = ISC_TRUE;
key->source = dns_keysource_user;
key->ksk = ISC_TRUE;
ISC_LIST_APPEND(keylist, key, link);
}
}
......
......@@ -31,7 +31,7 @@
/*
* Principal Author: Brian Wellington
* $Id: dst_api.c,v 1.30 2009/09/14 18:45:45 each Exp $
* $Id: dst_api.c,v 1.31 2009/09/23 16:01:57 each Exp $
*/
/*! \file */
......@@ -825,25 +825,100 @@ dst_key_setprivateformat(dst_key_t *key, int major, int minor) {
key->fmt_minor = minor;
}
isc_boolean_t
dst_key_compare(const dst_key_t *key1, const dst_key_t *key2) {
static isc_boolean_t
comparekeys(const dst_key_t *key1, const dst_key_t *key2,
isc_boolean_t match_revoked_key,
isc_boolean_t (*compare)())
{
REQUIRE(dst_initialized == ISC_TRUE);
REQUIRE(VALID_KEY(key1));
REQUIRE(VALID_KEY(key2));
if (key1 == key2)
return (ISC_TRUE);
if (key1 == NULL || key2 == NULL)
return (ISC_FALSE);
if (key1->key_alg == key2->key_alg &&
key1->key_id == key2->key_id &&
key1->func->compare != NULL &&
key1->func->compare(key1, key2) == ISC_TRUE)
return (ISC_TRUE);
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);
if (key1->key_alg == DST_ALG_RSAMD5)
return (ISC_FALSE);
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))
return (ISC_FALSE);
}
if (compare != NULL)
return (compare(key1, key2));
else
return (ISC_FALSE);
}
/*
* Compares only the public portion of two keys, by converting them
* both to wire format and comparing the results.
*/
static isc_boolean_t
pub_compare(dst_key_t *key1, dst_key_t *key2) {
isc_result_t result;
unsigned char txt1[DST_KEY_MAXSIZE], txt2[DST_KEY_MAXSIZE];
isc_buffer_t b1, b2;
isc_region_t r1, r2;
isc_uint16_t flags;
flags = key1->key_flags;
key1->key_flags = 0;
isc_buffer_init(&b1, txt1, sizeof(txt1));
result = dst_key_todns(key1, &b1);
key1->key_flags = flags;
if (result != ISC_R_SUCCESS)
return (ISC_FALSE);
flags = key2->key_flags;
key2->key_flags = 0;
isc_buffer_init(&b2, txt2, sizeof(txt2));
result = dst_key_todns(key2, &b2);
key2->key_flags = flags;
if (result != ISC_R_SUCCESS)
return (ISC_FALSE);
isc_buffer_usedregion(&b1, &r1);
isc_buffer_usedregion(&b2, &r2);
return (ISC_TF(isc_region_compare(&r1, &r2) == 0));
}
isc_boolean_t
dst_key_compare(const dst_key_t *key1, const dst_key_t *key2) {