From: Evan Hunt Date: Thu, 12 Oct 2017 04:01:13 +0000 (-0700) Subject: [master] tag initializing keys so they can't be used for normal validation X-Git-Tag: v9.12.0b1~17 X-Git-Url: http://git.ipfire.org/gitweb/index.cgi?a=commitdiff_plain;h=560d8b833edceb4b715fe46b45f2009dc09fdb5d;p=thirdparty%2Fbind9.git [master] tag initializing keys so they can't be used for normal validation 4773. [bug] Keys specified in "managed-keys" statements can now only be used when validating key refresh queries during initialization of RFC 5011 key maintenance. If initialization fails, DNSSEC validation of normal queries will also fail. Previously, validation of normal queries could succeed using the initializing key, potentially masking problems with managed-keys. [RT #46077] --- diff --git a/CHANGES b/CHANGES index 92ef0bc9907..ff22e9efeeb 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,12 @@ +4773. [bug] Keys specified in "managed-keys" statements + can now only be used when validating key refresh + queries during initialization of RFC 5011 key + maintenance. If initialization fails, DNSSEC + validation of normal queries will also fail. + Previously, validation of normal queries could + succeed using the initializing key, potentially + masking problems with managed-keys. [RT #46077] + 4772. [test] Expanded unit testing framework for libns, using hooks to interrupt query flow and inspect state at specified locations. [RT #46173] diff --git a/bin/named/server.c b/bin/named/server.c index 2101c1ded26..818838b142a 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -844,7 +844,8 @@ load_view_keys(const cfg_obj_t *keys, const cfg_obj_t *vconfig, continue; } - CHECK(dns_keytable_add(secroots, managed, &dstkey)); + CHECK(dns_keytable_add2(secroots, managed, + managed, &dstkey)); } } @@ -1043,7 +1044,8 @@ configure_view_dnsseckeys(dns_view_t *view, const cfg_obj_t *vconfig, isc_log_write(named_g_lctx, NAMED_LOGCATEGORY_GENERAL, NAMED_LOGMODULE_SERVER, ISC_LOG_ERROR, "managed-keys-directory '%s' " - "is not writable", directory); + "must be writable and accessible", + directory); result = ISC_R_NOPERM; goto cleanup; } @@ -6168,8 +6170,8 @@ directory_callback(const char *clausename, const cfg_obj_t *obj, void *arg) { if (access(directory, DIR_PERM_OK) != 0) { isc_log_write(named_g_lctx, NAMED_LOGCATEGORY_GENERAL, NAMED_LOGMODULE_SERVER, ISC_LOG_ERROR, - "directory '%s' is not writable", - directory); + "working directory '%s' must be " + "writable and accessible", directory); return (ISC_R_NOPERM); } @@ -6434,7 +6436,7 @@ dotat(dns_keytable_t *keytable, dns_keynode_t *keynode, void *arg) { do { dst_key_t *key = dns_keynode_key(keynode); - if (key != NULL) { + if (key != NULL && !dns_keynode_initial(keynode)) { name = dst_key_name(key); if (n < (sizeof(ids)/sizeof(ids[0]))) { ids[n] = dst_key_id(key); @@ -6443,16 +6445,19 @@ dotat(dns_keytable_t *keytable, dns_keynode_t *keynode, void *arg) { } nextnode = NULL; (void)dns_keytable_nextkeynode(keytable, keynode, &nextnode); - if (keynode != firstnode) + if (keynode != firstnode) { dns_keytable_detachkeynode(keytable, &keynode); + } keynode = nextnode; } while (keynode != NULL); - if (n == 0) + if (n == 0) { return; + } - if (n > 1) + if (n > 1) { qsort(ids, n, sizeof(ids[0]), cid); + } /* * Encoded as "_ta-xxxx\(-xxxx\)*" where xxxx is the hex version of @@ -6462,20 +6467,23 @@ dotat(dns_keytable_t *keytable, dns_keynode_t *keynode, void *arg) { r.base = label; r.length = sizeof(label);; m = snprintf(r.base, r.length, "_ta"); - if (m < 0 || (unsigned)m > r.length) + if (m < 0 || (unsigned)m > r.length) { return; + } isc_textregion_consume(&r, m); for (i = 0; i < n; i++) { m = snprintf(r.base, r.length, "-%04x", ids[i]); - if (m < 0 || (unsigned)m > r.length) + if (m < 0 || (unsigned)m > r.length) { return; + } isc_textregion_consume(&r, m); } dns_fixedname_init(&fixed); tatname = dns_fixedname_name(&fixed); result = dns_name_fromstring2(tatname, label, name, 0, NULL); - if (result != ISC_R_SUCCESS) + if (result != ISC_R_SUCCESS) { return; + } dns_name_format(tatname, namebuf, sizeof(namebuf)); isc_log_write(named_g_lctx, NAMED_LOGCATEGORY_GENERAL, @@ -6484,8 +6492,9 @@ dotat(dns_keytable_t *keytable, dns_keynode_t *keynode, void *arg) { view->name, namebuf); tat = isc_mem_get(dotat_arg->view->mctx, sizeof(*tat)); - if (tat == NULL) + if (tat == NULL) { return; + } tat->mctx = NULL; tat->task = NULL; @@ -8490,7 +8499,8 @@ load_configuration(const char *filename, named_server_t *server, if (access(".", DIR_PERM_OK) != 0) { isc_log_write(named_g_lctx, NAMED_LOGCATEGORY_GENERAL, NAMED_LOGMODULE_SERVER, ISC_LOG_ERROR, - "the working directory is not writable"); + "the working directory must be " + "writable and accessible"); result = ISC_R_NOPERM; goto cleanup; } diff --git a/bin/tests/system/mkeys/README b/bin/tests/system/mkeys/README index 2ffcfda24e6..40310a2d70e 100644 --- a/bin/tests/system/mkeys/README +++ b/bin/tests/system/mkeys/README @@ -16,17 +16,8 @@ is used so it will send TAT queries once per second. ns3 is a validator with a broken key in managed-keys. -Tests TODO: - -- initial working KSK - -TODO: test using delv with new trusted key too - -- introduce a REVOKE bit - -- later remove a signature - -- corrupt a signature - -TODO: also same things with dlv auto updates of trust anchor +ns4 is a validator with a deliberately broken managed-keys.bind and +managed-keys.jnl, causing RFC 5011 initialization to fail. +ns5 is a validator which is prevented from getting a response from the +root server, causing key refresh queries to fail. diff --git a/bin/tests/system/mkeys/clean.sh b/bin/tests/system/mkeys/clean.sh index 7f6b79c90d8..76e654c64f6 100644 --- a/bin/tests/system/mkeys/clean.sh +++ b/bin/tests/system/mkeys/clean.sh @@ -15,3 +15,4 @@ rm -f */named.memstats */named.run rm -f dig.out* delv.out* rndc.out* signer.out* rm -f ns1/named.secroots ns1/root.db.signed* ns1/root.db.tmp rm -f ns1/named.conf +rm -rf ns4/nope diff --git a/bin/tests/system/mkeys/ns1/named1.conf b/bin/tests/system/mkeys/ns1/named1.conf index 29c0a88efdb..31afb8e1d2b 100644 --- a/bin/tests/system/mkeys/ns1/named1.conf +++ b/bin/tests/system/mkeys/ns1/named1.conf @@ -10,6 +10,11 @@ controls { /* empty */ }; +acl allowed { + ! 10.53.0.5; + any; +}; + options { query-source address 10.53.0.1; notify-source 10.53.0.1; @@ -22,6 +27,7 @@ options { notify no; dnssec-enable yes; dnssec-validation yes; + allow-query { allowed; }; }; key rndc_key { diff --git a/bin/tests/system/mkeys/ns1/named2.conf b/bin/tests/system/mkeys/ns1/named2.conf index daeed6dd053..19ef0cb7015 100644 --- a/bin/tests/system/mkeys/ns1/named2.conf +++ b/bin/tests/system/mkeys/ns1/named2.conf @@ -10,6 +10,11 @@ controls { /* empty */ }; +acl allowed { + ! 10.53.0.5; + any; +}; + options { query-source address 10.53.0.1; notify-source 10.53.0.1; @@ -22,6 +27,7 @@ options { notify no; dnssec-enable yes; dnssec-validation yes; + allow-query { allowed; }; }; key rndc_key { diff --git a/bin/tests/system/mkeys/ns1/sign.sh b/bin/tests/system/mkeys/ns1/sign.sh index fb134742d26..054422de55e 100644 --- a/bin/tests/system/mkeys/ns1/sign.sh +++ b/bin/tests/system/mkeys/ns1/sign.sh @@ -28,6 +28,8 @@ managed-keys { EOF ' > managed.conf cp managed.conf ../ns2/managed.conf +cp managed.conf ../ns4/managed.conf +cp managed.conf ../ns5/managed.conf # Configure a trusted key statement (used by delve) cat $keyname.key | grep -v '^; ' | $PERL -n -e ' diff --git a/bin/tests/system/mkeys/ns2/named.args b/bin/tests/system/mkeys/ns2/named.args index d222b7faea7..71e466df405 100644 --- a/bin/tests/system/mkeys/ns2/named.args +++ b/bin/tests/system/mkeys/ns2/named.args @@ -1 +1 @@ --m record,size,mctx -T clienttest -c named.conf -d 99 -X named.lock -g -T mkeytimers=2/20/40 +-m record,size,mctx -T clienttest -c named.conf -d 99 -X named.lock -g -T mkeytimers=2/20/40 -T tat=1 diff --git a/bin/tests/system/mkeys/ns4/named.conf b/bin/tests/system/mkeys/ns4/named.conf new file mode 100644 index 00000000000..ad3979d7a72 --- /dev/null +++ b/bin/tests/system/mkeys/ns4/named.conf @@ -0,0 +1,41 @@ +/* + * Copyright (C) 2017 Internet Systems Consortium, Inc. ("ISC") + * + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. + */ + +// NS4 + +controls { /* empty */ }; + +options { + query-source address 10.53.0.4; + notify-source 10.53.0.4; + transfer-source 10.53.0.4; + port 5300; + pid-file "named.pid"; + listen-on { 10.53.0.4; }; + listen-on-v6 { none; }; + recursion yes; + notify no; + dnssec-enable yes; + dnssec-validation auto; + bindkeys-file "managed.conf"; + managed-keys-directory "nope"; +}; + +key rndc_key { + secret "1234abcd8765"; + algorithm hmac-sha256; +}; + +controls { + inet 10.53.0.4 port 9953 allow { any; } keys { rndc_key; }; +}; + +zone "." { + type hint; + file "../../common/root.hint"; +}; diff --git a/bin/tests/system/mkeys/ns5/named.conf b/bin/tests/system/mkeys/ns5/named.conf new file mode 100644 index 00000000000..98204929ad4 --- /dev/null +++ b/bin/tests/system/mkeys/ns5/named.conf @@ -0,0 +1,38 @@ +/* + * Copyright (C) 2017 Internet Systems Consortium, Inc. ("ISC") + * + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. + */ + +// NS5 + +options { + query-source address 10.53.0.5; + notify-source 10.53.0.5; + transfer-source 10.53.0.5; + port 5300; + pid-file "named.pid"; + listen-on { 10.53.0.5; }; + listen-on-v6 { none; }; + recursion yes; + notify no; + dnssec-enable yes; + dnssec-validation auto; + bindkeys-file "managed.conf"; +}; + +key rndc_key { + secret "1234abcd8765"; + algorithm hmac-sha256; +}; + +controls { + inet 10.53.0.5 port 9953 allow { any; } keys { rndc_key; }; +}; + +zone "." { + type hint; + file "../../common/root.hint"; +}; diff --git a/bin/tests/system/mkeys/setup.sh b/bin/tests/system/mkeys/setup.sh index d555c4e93e2..bb70fe1df39 100644 --- a/bin/tests/system/mkeys/setup.sh +++ b/bin/tests/system/mkeys/setup.sh @@ -16,3 +16,9 @@ test -r $RANDFILE || $GENRANDOM 800 $RANDFILE cp ns1/named1.conf ns1/named.conf cd ns1 && $SHELL sign.sh + +cd ../ns4 +mkdir nope +touch nope/managed-keys.bind +touch nope/managed.keys.bind.jnl +chmod 444 nope/* diff --git a/bin/tests/system/mkeys/tests.sh b/bin/tests/system/mkeys/tests.sh index b9806f3431f..c5ebc88ac5a 100644 --- a/bin/tests/system/mkeys/tests.sh +++ b/bin/tests/system/mkeys/tests.sh @@ -28,6 +28,7 @@ status=`expr $status + $ret` n=`expr $n + 1` echo "I: check positive validation with valid trust anchor ($n)" ret=0 +$RNDC -c ../common/rndc.conf -s 10.53.0.2 -p 9953 flush | sed 's/^/I: ns2 /' $DIG $DIGOPTS +noauth example. @10.53.0.2 txt > dig.out.ns2.test$n || ret=1 grep "flags:.*ad.*QUERY" dig.out.ns2.test$n > /dev/null || ret=1 grep "example..*.RRSIG..*TXT" dig.out.ns2.test$n > /dev/null || ret=1 @@ -390,6 +391,7 @@ $PERL $SYSTEMTESTTOP/start.pl --noclean --restart . ns2 n=`expr $n + 1` echo "I: check positive validation ($n)" ret=0 +$RNDC -c ../common/rndc.conf -s 10.53.0.2 -p 9953 flush | sed 's/^/I: ns2 /' $DIG $DIGOPTS +noauth example. @10.53.0.2 txt > dig.out.ns2.test$n || ret=1 grep "flags:.*ad.*QUERY" dig.out.ns2.test$n > /dev/null || ret=1 grep "example..*.RRSIG..*TXT" dig.out.ns2.test$n > /dev/null || ret=1 @@ -446,7 +448,6 @@ rm -f ${revoked}.key ${revoked}.private $SETTIME -D none -R none -K ns1 `cat ns1/managed.key` > /dev/null $SETTIME -D now -K ns1 $standby1 > /dev/null $SETTIME -D now -K ns1 $standby2 > /dev/null -$RNDC -c ../common/rndc.conf -s 10.53.0.2 -p 9953 flush | sed 's/^/I: ns1 /' sleep 1 $SIGNER -Sg -K ns1 -N unixtime -r $RANDFILE -o . ns1/root.db > /dev/null 2>&- $RNDC -c ../common/rndc.conf -s 10.53.0.1 -p 9953 reload . | sed 's/^/I: ns1 /' @@ -454,6 +455,7 @@ sleep 3 $RNDC -c ../common/rndc.conf -s 10.53.0.2 -p 9953 managed-keys refresh | sed 's/^/I: ns2 /' sleep 1 $RNDC -c ../common/rndc.conf -s 10.53.0.2 -p 9953 managed-keys status > rndc.out.$n 2>&1 +$RNDC -c ../common/rndc.conf -s 10.53.0.2 -p 9953 flush | sed 's/^/I: ns1 /' $DIG $DIGOPTS +noauth example. @10.53.0.2 txt > dig.out.ns2.test$n || ret=1 grep "flags:.*ad.*QUERY" dig.out.ns2.test$n > /dev/null || ret=1 grep "example..*.RRSIG..*TXT" dig.out.ns2.test$n > /dev/null || ret=1 @@ -537,7 +539,7 @@ status=`expr $status + $ret` n=`expr $n + 1` echo "I: check that trust-anchor-telemetry queries are logged ($n)" ret=0 -grep "sending trust-anchor-telemetry query '_ta-[0-9a-f]*/NULL" ns3/named.run > /dev/null || ret=1 +grep "sending trust-anchor-telemetry query '_ta-[0-9a-f]*/NULL" ns2/named.run > /dev/null || ret=1 if [ $ret != 0 ]; then echo "I:failed"; fi status=`expr $status + $ret` @@ -562,5 +564,45 @@ grep "name: \." rndc.out.$n > /dev/null || ret=1 if [ $ret != 0 ]; then echo "I:failed"; fi status=`expr $status + $ret` +n=`expr $n + 1` +echo "I: check that trust-anchor-telemetry queries contain the correct key ($n)" +ret=0 +# convert the hexadecimal key from the TAT query into decimal and +# compare against the known key. +tathex=`grep "query '_ta-[0-9a-f]*/NULL/IN' approved" ns1/named.run | awk '{print $6; exit 0}' | sed -e 's/(_ta-\([a-f0-9][a-f0-d]*\)):/\1/'` +tatkey=`$PERL -e 'printf("%d\n", hex(@ARGV[0]));' $tathex` +realkey=`$RNDC -c ../common/rndc.conf -s 10.53.0.2 -p 9953 secroots - | grep '; managed' | sed 's#.*SHA256/\([0-9][0-9]*\) ; managed.*#\1#'` +[ "$tatkey" -eq "$realkey" ] || ret=1 +if [ $ret != 0 ]; then echo "I:failed"; fi +status=`expr $status + $ret` + +n=`expr $n + 1` +echo "I: check initialization fails if managed-keys can't be created ($n)" +ret=0 +$RNDC -c ../common/rndc.conf -s 10.53.0.4 -p 9953 secroots | sed 's/^/I: ns4 /' +grep '; initializing managed' ns4/named.secroots > /dev/null 2>&1 || ret=1 +grep '; managed' ns4/named.secroots > /dev/null 2>&1 && ret=1 +grep '; trusted' ns4/named.secroots > /dev/null 2>&1 && ret=1 +if [ $ret != 0 ]; then echo "I:failed"; fi +status=`expr $status + $ret` + +n=`expr $n + 1` +echo "I: check failure to contact root servers does not prevent key refreshes after restart ($n)" +ret=0 +# By the time we get here, ns5 should have attempted refreshing its managed +# keys. These attempts should fail as ns1 is configured to REFUSE all queries +# from ns5. Note we do not configure ns5 with "-T mkeytimers"; this is to +# ensure key refresh retry will be scheduled one hour in the future instead of +# a few seconds in the future, in order to prevent races when ns5 is restarted. +$PERL $SYSTEMTESTTOP/stop.pl --use-rndc . ns5 +$PERL $SYSTEMTESTTOP/start.pl --noclean --restart . ns5 +sleep 2 +# ns5/named.run will contain logs from both the old instance and the new +# instance. In order for the test to pass, both must attempt a fetch. +count=`grep -c "Creating key fetch" ns5/named.run` +[ $count -lt 2 ] && ret=1 +if [ $ret != 0 ]; then echo "I:failed"; fi +status=`expr $status + $ret` + echo "I:exit status: $status" [ $status -eq 0 ] || exit 1 diff --git a/bin/tests/system/runtime/ns2/named-alt2.conf b/bin/tests/system/runtime/ns2/named-alt2.conf index 1ed96d4e0c0..0e0864a58b4 100644 --- a/bin/tests/system/runtime/ns2/named-alt2.conf +++ b/bin/tests/system/runtime/ns2/named-alt2.conf @@ -13,7 +13,7 @@ controls { /* empty */ }; options { query-source address 10.53.0.2; port 5300; - pid-file "named3.pid"; + pid-file "named2.pid"; listen-on { 10.53.0.2; 10.53.0.3; }; listen-on-v6 { fd92:7065:b8e:ffff::2; }; recursion no; diff --git a/bin/tests/system/runtime/ns2/named-alt3.conf b/bin/tests/system/runtime/ns2/named-alt3.conf index 77b3a421f03..4353284f99f 100644 --- a/bin/tests/system/runtime/ns2/named-alt3.conf +++ b/bin/tests/system/runtime/ns2/named-alt3.conf @@ -13,7 +13,7 @@ controls { /* empty */ }; options { query-source address 10.53.0.2; port 5300; - pid-file "named4.pid"; + pid-file "named2.pid"; lock-file none; listen-on { 10.53.0.2; 10.53.0.3; }; listen-on-v6 { fd92:7065:b8e:ffff::2; }; diff --git a/bin/tests/system/runtime/ns2/named-alt4.conf b/bin/tests/system/runtime/ns2/named-alt4.conf index 690b4282d27..12fd87fb9c7 100644 --- a/bin/tests/system/runtime/ns2/named-alt4.conf +++ b/bin/tests/system/runtime/ns2/named-alt4.conf @@ -7,10 +7,10 @@ */ options { - directory "./nope"; - port 5300; - pid-file "../named4.pid"; - listen-on { 127.0.0.1; }; - listen-on-v6 { none; }; - recursion no; + directory "./nope"; + port 5300; + pid-file "../named.pid"; + listen-on { 127.0.0.1; }; + listen-on-v6 { none; }; + recursion no; }; diff --git a/bin/tests/system/runtime/ns2/named-alt5.conf b/bin/tests/system/runtime/ns2/named-alt5.conf index f6a4b68f4e7..3b179425d0c 100644 --- a/bin/tests/system/runtime/ns2/named-alt5.conf +++ b/bin/tests/system/runtime/ns2/named-alt5.conf @@ -8,9 +8,9 @@ options { managed-keys-directory "./nope"; - port 5300; - pid-file "../named4.pid"; - listen-on { 127.0.0.1; }; - listen-on-v6 { none; }; - recursion no; + port 5300; + pid-file "../named.pid"; + listen-on { 127.0.0.1; }; + listen-on-v6 { none; }; + recursion no; }; diff --git a/bin/tests/system/runtime/tests.sh b/bin/tests/system/runtime/tests.sh index 6a142fc633d..cf285383f51 100644 --- a/bin/tests/system/runtime/tests.sh +++ b/bin/tests/system/runtime/tests.sh @@ -37,7 +37,7 @@ ret=0 (cd ns2; $NAMED -c named-alt2.conf -D ns2-extra-2 -X named.lock -m record,size,mctx -d 99 -g -U 4 >> named3.run 2>&1 & ) sleep 2 grep "another named process" ns2/named3.run > /dev/null || ret=1 -[ -s ns2/named3.pid ] && $KILL -15 `cat ns2/named3.pid` +[ -s ns2/named2.pid ] && $KILL -15 `cat ns2/named2.pid` if [ $ret != 0 ]; then echo "I:failed"; fi status=`expr $status + $ret` @@ -47,55 +47,60 @@ ret=0 (cd ns2; $NAMED -c named-alt3.conf -D ns2-extra-3 -m record,size,mctx -d 99 -g -U 4 >> named4.run 2>&1 & ) sleep 2 grep "another named process" ns2/named4.run > /dev/null && ret=1 -[ -s ns2/named4.pid ] && $KILL -15 `cat ns2/named4.pid` +[ -s ns2/named2.pid ] && $KILL -15 `cat ns2/named2.pid` if [ $ret != 0 ]; then echo "I:failed"; fi status=`expr $status + $ret` -n=`expr $n + 1` -echo "I: checking that named refuses to reconfigure if working directory is not writable ($n)" -ret=0 -cp -f ns2/named-alt4.conf ns2/named.conf -$RNDC -c ../common/rndc.conf -s 10.53.0.2 -p 9953 reconfig > rndc.out.$n 2>&1 -grep "failed: permission denied" rndc.out.$n > /dev/null 2>&1 || ret=1 -sleep 1 -grep "[^-]directory './nope' is not writable" ns2/named.run > /dev/null 2>&1 || ret=1 -if [ $ret != 0 ]; then echo "I:failed"; fi -status=`expr $status + $ret` +if [ ! "$CYGWIN" ]; then + n=`expr $n + 1` + echo "I: checking that named refuses to reconfigure if working directory is not writable ($n)" + ret=0 + cp -f ns2/named-alt4.conf ns2/named.conf + $RNDC -c ../common/rndc.conf -s 10.53.0.2 -p 9953 reconfig > rndc.out.$n 2>&1 + grep "failed: permission denied" rndc.out.$n > /dev/null 2>&1 || ret=1 + sleep 1 + grep "[^-]directory './nope' must be writable" ns2/named.run > /dev/null 2>&1 || ret=1 + if [ $ret != 0 ]; then echo "I:failed"; fi + status=`expr $status + $ret` -n=`expr $n + 1` -echo "I: checking that named refuses to reconfigure if managed-keys-directory is not writable ($n)" -ret=0 -cp -f ns2/named-alt5.conf ns2/named.conf -$RNDC -c ../common/rndc.conf -s 10.53.0.2 -p 9953 reconfig > rndc.out.$n 2>&1 -grep "failed: permission denied" rndc.out.$n > /dev/null 2>&1 || ret=1 -sleep 1 -grep "managed-keys-directory './nope' is not writable" ns2/named.run > /dev/null 2>&1 || ret=1 -if [ $ret != 0 ]; then echo "I:failed"; fi -status=`expr $status + $ret` + n=`expr $n + 1` + echo "I: checking that named refuses to reconfigure if managed-keys-directory is not writable ($n)" + ret=0 + cp -f ns2/named-alt5.conf ns2/named.conf + $RNDC -c ../common/rndc.conf -s 10.53.0.2 -p 9953 reconfig > rndc.out.$n 2>&1 + grep "failed: permission denied" rndc.out.$n > /dev/null 2>&1 || ret=1 + sleep 1 + grep "managed-keys-directory './nope' must be writable" ns2/named.run > /dev/null 2>&1 || ret=1 + if [ $ret != 0 ]; then echo "I:failed"; fi + status=`expr $status + $ret` -n=`expr $n + 1` -echo "I: checking that named refuses to start if working directory is not writable ($n)" -ret=0 -cd ns2 -$NAMED -c named-alt4.conf -d 99 -g > named4.run 2>&1 & -sleep 2 -grep "exiting (due to fatal error)" named4.run > /dev/null || ret=1 -[ -s named4.pid ] && kill -15 `cat named4.pid` > /dev/null 2>&1 -cd .. -if [ $ret != 0 ]; then echo "I:failed"; fi -status=`expr $status + $ret` + echo "I: kill existing named process" + [ -s "ns2/named.pid" ] && kill -15 `cat ns2/named.pid` -n=`expr $n + 1` -echo "I: checking that named refuses to start if managed-keys-directory is not writable ($n)" -ret=0 -cd ns2 -$NAMED -c named-alt5.conf -d 99 -g > named5.run 2>&1 & -sleep 2 -grep "exiting (due to fatal error)" named5.run > /dev/null || ret=1 -[ -s named5.pid ] && kill -15 `cat named5.pid` > /dev/null 2>&1 -cd .. -if [ $ret != 0 ]; then echo "I:failed"; fi -status=`expr $status + $ret` + n=`expr $n + 1` + echo "I: checking that named refuses to start if working directory is not writable ($n)" + ret=0 + cd ns2 + $NAMED -c named-alt4.conf -d 99 -g > named4.run 2>&1 & + sleep 2 + grep "exiting (due to fatal error)" named4.run > /dev/null || ret=1 + cd .. + [ -s named.pid ] && kill -15 `cat named.pid` > /dev/null 2>&1 + if [ $ret != 0 ]; then echo "I:failed"; fi + status=`expr $status + $ret` + + n=`expr $n + 1` + echo "I: checking that named refuses to start if managed-keys-directory is not writable ($n)" + ret=0 + cd ns2 + $NAMED -c named-alt5.conf -d 99 -g > named5.run 2>&1 & + sleep 2 + grep "exiting (due to fatal error)" named5.run > /dev/null || ret=1 + cd .. + [ -s named.pid ] && kill -15 `cat named.pid` > /dev/null 2>&1 + if [ $ret != 0 ]; then echo "I:failed"; fi + status=`expr $status + $ret` +fi echo "I:exit status: $status" [ $status -eq 0 ] || exit 1 diff --git a/doc/arm/notes.xml b/doc/arm/notes.xml index 0ecb3420831..944b7bf0f47 100644 --- a/doc/arm/notes.xml +++ b/doc/arm/notes.xml @@ -492,6 +492,17 @@ are not writable by the effective user ID. [RT #46077] + + + Initializing keys specified in a managed-keys + statement or by dnssec-validation auto; are + no longer treated as valid for any use other than validation of + RFC 5011 initialization queries. The effect of this is that + DNSSEC validation will fail if RFC 5011 key maintenance + cannot be initialized: initialization problems will not be + masked, but will be immediately visible. [RT #46077] + + Previously, update-policy local; accepted diff --git a/lib/dns/client.c b/lib/dns/client.c index 1d8269912e0..9a8d9b3819f 100644 --- a/lib/dns/client.c +++ b/lib/dns/client.c @@ -1546,7 +1546,7 @@ dns_client_addtrustedkey(dns_client_t *client, dns_rdataclass_t rdclass, if (result != ISC_R_SUCCESS) goto cleanup; - result = dns_keytable_add(secroots, ISC_FALSE, &dstkey); + result = dns_keytable_add2(secroots, ISC_FALSE, ISC_FALSE, &dstkey); cleanup: if (dstkey != NULL) diff --git a/lib/dns/include/dns/keytable.h b/lib/dns/include/dns/keytable.h index a3dab22baad..baf9ad0a78e 100644 --- a/lib/dns/include/dns/keytable.h +++ b/lib/dns/include/dns/keytable.h @@ -102,10 +102,19 @@ dns_keytable_detach(dns_keytable_t **keytablep); isc_result_t dns_keytable_add(dns_keytable_t *keytable, isc_boolean_t managed, - dst_key_t **keyp); + dst_key_t **keyp) ISC_DEPRECATED; +isc_result_t +dns_keytable_add2(dns_keytable_t *keytable, isc_boolean_t managed, + isc_boolean_t initial, dst_key_t **keyp); /*%< * Add '*keyp' to 'keytable' (using the name in '*keyp'). - * The value of keynode->managed is set to 'managed' + * The value of keynode->managed is set to 'managed', and the + * value of keynode->initial is set to 'initial'. ('initial' should + * be only used when adding managed-keys from configuration: this + * indicates a secure root which can be *only* used for validating + * RFC 5011 key refresh queries, but not for other DNSSEC validation. + * Once a key refresh query has validated, we update the keynode + * with inital == ISC_FALSE.) * * Notes: * @@ -402,6 +411,19 @@ dns_keynode_managed(dns_keynode_t *keynode); * Is this flagged as a managed key? */ +isc_boolean_t +dns_keynode_initial(dns_keynode_t *keynode); +/*%< + * Is this flagged as an initializing key? + */ + +void +dns_keynode_trust(dns_keynode_t *keynode); +/*%< + * Sets keynode->initial to ISC_FALSE in order mark the key as + * trusted: no longer an initializing key. + */ + isc_result_t dns_keynode_create(isc_mem_t *mctx, dns_keynode_t **target); /*%< diff --git a/lib/dns/keytable.c b/lib/dns/keytable.c index 022ab204587..1421330c701 100644 --- a/lib/dns/keytable.c +++ b/lib/dns/keytable.c @@ -47,6 +47,7 @@ struct dns_keynode { isc_refcount_t refcount; dst_key_t * key; isc_boolean_t managed; + isc_boolean_t initial; struct dns_keynode * next; }; @@ -165,7 +166,7 @@ dns_keytable_detach(dns_keytable_t **keytablep) { } static isc_result_t -insert(dns_keytable_t *keytable, isc_boolean_t managed, +insert(dns_keytable_t *keytable, isc_boolean_t managed, isc_boolean_t initial, const dns_name_t *keyname, dst_key_t **keyp) { isc_result_t result; @@ -180,6 +181,7 @@ insert(dns_keytable_t *keytable, isc_boolean_t managed, return (result); knode->managed = managed; + knode->initial = initial; RWLOCK(&keytable->rwlock, isc_rwlocktype_write); @@ -233,14 +235,21 @@ insert(dns_keytable_t *keytable, isc_boolean_t managed, isc_result_t dns_keytable_add(dns_keytable_t *keytable, isc_boolean_t managed, dst_key_t **keyp) +{ + return (dns_keytable_add2(keytable, managed, ISC_FALSE, keyp)); +} + +isc_result_t +dns_keytable_add2(dns_keytable_t *keytable, isc_boolean_t managed, + isc_boolean_t initial, dst_key_t **keyp) { REQUIRE(keyp != NULL && *keyp != NULL); - return (insert(keytable, managed, dst_key_name(*keyp), keyp)); + return (insert(keytable, managed, initial, dst_key_name(*keyp), keyp)); } isc_result_t dns_keytable_marksecure(dns_keytable_t *keytable, const dns_name_t *name) { - return (insert(keytable, ISC_TRUE, name, NULL)); + return (insert(keytable, ISC_TRUE, ISC_FALSE, name, NULL)); } isc_result_t @@ -644,8 +653,9 @@ dns_keytable_totext(dns_keytable_t *keytable, isc_buffer_t **text) { if (knode->key == NULL) continue; dst_key_format(knode->key, pbuf, sizeof(pbuf)); - snprintf(obuf, sizeof(obuf), "%s ; %s\n", pbuf, - knode->managed ? "managed" : "trusted"); + snprintf(obuf, sizeof(obuf), "%s ; %s%s\n", pbuf, + knode->initial ? "initializing " : "", + knode->managed ? "managed" : "trusted"); result = putstr(text, obuf); if (result != ISC_R_SUCCESS) break; @@ -723,6 +733,26 @@ dns_keynode_managed(dns_keynode_t *keynode) { return (keynode->managed); } +isc_boolean_t +dns_keynode_initial(dns_keynode_t *keynode) { + /* + * Is this an initailizing key? + */ + REQUIRE(VALID_KEYNODE(keynode)); + + return (keynode->initial); +} + +void +dns_keynode_trust(dns_keynode_t *keynode) { + /* + * This is no longer an initializing key. + */ + REQUIRE(VALID_KEYNODE(keynode)); + + keynode->initial = ISC_FALSE; +} + isc_result_t dns_keynode_create(isc_mem_t *mctx, dns_keynode_t **target) { isc_result_t result; @@ -736,6 +766,7 @@ dns_keynode_create(isc_mem_t *mctx, dns_keynode_t **target) { knode->magic = KEYNODE_MAGIC; knode->managed = ISC_FALSE; + knode->initial = ISC_FALSE; knode->key = NULL; knode->next = NULL; diff --git a/lib/dns/tests/keytable_test.c b/lib/dns/tests/keytable_test.c index db04dba9fb6..23ba2922073 100644 --- a/lib/dns/tests/keytable_test.c +++ b/lib/dns/tests/keytable_test.c @@ -126,7 +126,7 @@ create_tables() { /* Add a normal key */ create_key(257, 3, 5, "example.com", keystr1, &key); - ATF_REQUIRE_EQ(dns_keytable_add(keytable, ISC_FALSE, &key), + ATF_REQUIRE_EQ(dns_keytable_add2(keytable, ISC_FALSE, ISC_FALSE, &key), ISC_R_SUCCESS); /* Add a null key */ @@ -185,7 +185,7 @@ ATF_TC_BODY(add, tc) { * nextkeynode() should still return NOTFOUND. */ create_key(257, 3, 5, "example.com", keystr1, &key); - ATF_REQUIRE_EQ(dns_keytable_add(keytable, ISC_FALSE, &key), + ATF_REQUIRE_EQ(dns_keytable_add2(keytable, ISC_FALSE, ISC_FALSE, &key), ISC_R_SUCCESS); ATF_REQUIRE_EQ(dns_keytable_nextkeynode(keytable, keynode, &next_keynode), ISC_R_NOTFOUND); @@ -193,7 +193,7 @@ ATF_TC_BODY(add, tc) { /* Add another key (different keydata) */ dns_keytable_detachkeynode(keytable, &keynode); create_key(257, 3, 5, "example.com", keystr2, &key); - ATF_REQUIRE_EQ(dns_keytable_add(keytable, ISC_FALSE, &key), + ATF_REQUIRE_EQ(dns_keytable_add2(keytable, ISC_FALSE, ISC_FALSE, &key), ISC_R_SUCCESS); ATF_REQUIRE_EQ(dns_keytable_find(keytable, str2name("example.com"), &keynode), ISC_R_SUCCESS); @@ -209,7 +209,7 @@ ATF_TC_BODY(add, tc) { ATF_REQUIRE_EQ(dns_keytable_find(keytable, str2name("null.example"), &null_keynode), ISC_R_SUCCESS); create_key(257, 3, 5, "null.example", keystr2, &key); - ATF_REQUIRE_EQ(dns_keytable_add(keytable, ISC_FALSE, &key), + ATF_REQUIRE_EQ(dns_keytable_add2(keytable, ISC_FALSE, ISC_FALSE, &key), ISC_R_SUCCESS); ATF_REQUIRE_EQ(dns_keytable_find(keytable, str2name("null.example"), &keynode), ISC_R_SUCCESS); @@ -523,7 +523,7 @@ ATF_TC_BODY(nta, tc) { ATF_REQUIRE_EQ(result, ISC_R_SUCCESS); create_key(257, 3, 5, "example", keystr1, &key); - result = dns_keytable_add(keytable, ISC_FALSE, &key); + result = dns_keytable_add2(keytable, ISC_FALSE, ISC_FALSE, &key); ATF_REQUIRE_EQ(result, ISC_R_SUCCESS); isc_stdtime_get(&now); diff --git a/lib/dns/validator.c b/lib/dns/validator.c index db9d4baf0dc..d0138b800d9 100644 --- a/lib/dns/validator.c +++ b/lib/dns/validator.c @@ -1629,6 +1629,7 @@ validate(dns_validator_t *val, isc_boolean_t resume) { if (vresult == ISC_R_SUCCESS) break; if (val->keynode != NULL) { + dst_key_t *key = NULL; dns_keynode_t *nextnode = NULL; result = dns_keytable_findnextkeynode( val->keytable, @@ -1641,9 +1642,13 @@ validate(dns_validator_t *val, isc_boolean_t resume) { val->key = NULL; break; } - val->key = dns_keynode_key(val->keynode); - if (val->key == NULL) + key = dns_keynode_key(val->keynode); + if (key == NULL) break; + if (dns_keynode_initial(val->keynode)) { + continue; + } + val->key = key; } else { if (get_dst_key(val, val->siginfo, val->keyset) != ISC_R_SUCCESS) @@ -1660,10 +1665,10 @@ validate(dns_validator_t *val, isc_boolean_t resume) { val->view->acceptexpired); } - if (val->keynode != NULL) + if (val->keynode != NULL) { dns_keytable_detachkeynode(val->keytable, &val->keynode); - else { + } else { if (val->key != NULL) dst_key_free(&val->key); if (val->keyset != NULL) { @@ -2023,13 +2028,15 @@ validatezonekey(dns_validator_t *val) { &keynode); break; } - result = verify(val, dstkey, &sigrdata, - sig.keyid); - if (result == ISC_R_SUCCESS) { - dns_keytable_detachkeynode( + if (! dns_keynode_initial(keynode)) { + result = verify(val, dstkey, + &sigrdata, sig.keyid); + if (result == ISC_R_SUCCESS) { + dns_keytable_detachkeynode( val->keytable, &keynode); - break; + break; + } } result = dns_keytable_findnextkeynode( val->keytable, diff --git a/lib/dns/zone.c b/lib/dns/zone.c index 4966de4cc35..3ca053adbdb 100644 --- a/lib/dns/zone.c +++ b/lib/dns/zone.c @@ -3934,7 +3934,8 @@ compute_tag(dns_name_t *name, dns_rdata_dnskey_t *dnskey, isc_mem_t *mctx, */ static void trust_key(dns_zone_t *zone, dns_name_t *keyname, - dns_rdata_dnskey_t *dnskey, isc_mem_t *mctx) + dns_rdata_dnskey_t *dnskey, isc_boolean_t initial, + isc_mem_t *mctx) { isc_result_t result; dns_rdata_t rdata = DNS_RDATA_INIT; @@ -3953,7 +3954,7 @@ trust_key(dns_zone_t *zone, dns_name_t *keyname, goto failure; CHECK(dns_dnssec_keyfromrdata(keyname, &rdata, mctx, &dstkey)); - CHECK(dns_keytable_add(sr, ISC_TRUE, &dstkey)); + CHECK(dns_keytable_add2(sr, ISC_TRUE, initial, &dstkey)); dns_keytable_detach(&sr); failure: @@ -4039,7 +4040,8 @@ load_secroots(dns_zone_t *zone, dns_name_t *name, dns_rdataset_t *rdataset) { /* Add to keytables. */ trusted++; - trust_key(zone, name, &dnskey, mctx); + trust_key(zone, name, &dnskey, + ISC_TF(keydata.addhd == 0), mctx); } if (trusted == 0 && pending != 0) { @@ -4774,8 +4776,9 @@ zone_postload(dns_zone_t *zone, dns_db_t *db, isc_time_t loadtime, case dns_zone_key: result = sync_keyzone(zone, db); - if (result != ISC_R_SUCCESS) + if (result != ISC_R_SUCCESS) { goto cleanup; + } break; default: @@ -4925,9 +4928,17 @@ zone_postload(dns_zone_t *zone, dns_db_t *db, isc_time_t loadtime, return (result); cleanup: + if (zone->type == dns_zone_key && result != ISC_R_SUCCESS) { + dns_zone_log(zone, ISC_LOG_ERROR, + "failed to initialize managed-keys (%s): " + "DNSSEC validation WILL FAIL", + isc_result_totext(result)); + } + for (inc = ISC_LIST_HEAD(zone->newincludes); inc != NULL; - inc = ISC_LIST_HEAD(zone->newincludes)) { + inc = ISC_LIST_HEAD(zone->newincludes)) + { ISC_LIST_UNLINK(zone->newincludes, inc, link); isc_mem_free(zone->mctx, inc->name); isc_mem_put(zone->mctx, inc, sizeof(*inc)); @@ -9088,7 +9099,7 @@ keyfetch_done(isc_task_t *task, isc_event_t *event) { dst_key_t *dstkey; isc_stdtime_t now; int pending = 0; - isc_boolean_t secure = ISC_FALSE; + isc_boolean_t secure = ISC_FALSE, initial = ISC_FALSE; isc_boolean_t free_needed; UNUSED(task); @@ -9165,7 +9176,8 @@ keyfetch_done(isc_task_t *task, isc_event_t *event) { */ for (result = dns_rdataset_first(&kfetch->dnskeysigset); result == ISC_R_SUCCESS; - result = dns_rdataset_next(&kfetch->dnskeysigset)) { + result = dns_rdataset_next(&kfetch->dnskeysigset)) + { dns_keynode_t *keynode = NULL; dns_rdata_reset(&sigrr); @@ -9184,7 +9196,8 @@ keyfetch_done(isc_task_t *task, isc_event_t *event) { break; if (dst_key_alg(dstkey) == sig.algorithm && - dst_key_id(dstkey) == sig.keyid) { + dst_key_id(dstkey) == sig.keyid) + { result = dns_dnssec_verify2(keyname, &kfetch->dnskeyset, dstkey, ISC_FALSE, @@ -9202,6 +9215,9 @@ keyfetch_done(isc_task_t *task, isc_event_t *event) { dns_trust_secure; kfetch->dnskeysigset.trust = dns_trust_secure; + secure = ISC_TRUE; + initial = dns_keynode_initial(keynode); + dns_keynode_trust(keynode); break; } } @@ -9212,11 +9228,11 @@ keyfetch_done(isc_task_t *task, isc_event_t *event) { keynode = nextnode; } - if (keynode != NULL) + if (keynode != NULL) { dns_keytable_detachkeynode(secroots, &keynode); + } - if (kfetch->dnskeyset.trust == dns_trust_secure) { - secure = ISC_TRUE; + if (secure) { break; } } @@ -9225,7 +9241,6 @@ keyfetch_done(isc_task_t *task, isc_event_t *event) { * If we were not able to verify the answer using the current * trusted keys then all we can do is look at any revoked keys. */ - if (!secure) { dns_zone_log(zone, ISC_LOG_DEBUG(3), "DNSKEY set for zone '%s' could not be verified " @@ -9465,10 +9480,13 @@ keyfetch_done(isc_task_t *task, isc_event_t *event) { trustkey = ISC_TRUE; dns_zone_log(zone, ISC_LOG_INFO, "Key %d for zone %s " - "acceptance timer " - "complete: " - "key now trusted", - keytag, namebuf); + "%s: key now trusted", + keytag, namebuf, + initial + ? "initializing key " + "verified" + : "acceptance timer " + "complete"); } } else if (keydata.addhd > now) { /* @@ -9567,7 +9585,7 @@ keyfetch_done(isc_task_t *task, isc_event_t *event) { /* Trust this key. */ result = dns_rdata_tostruct(&dnskeyrr, &dnskey, NULL); RUNTIME_CHECK(result == ISC_R_SUCCESS); - trust_key(zone, keyname, &dnskey, mctx); + trust_key(zone, keyname, &dnskey, ISC_FALSE, mctx); } if (secure && !deletekey) { @@ -9589,7 +9607,6 @@ keyfetch_done(isc_task_t *task, isc_event_t *event) { fail_secure(zone, keyname); done: - if (!ISC_LIST_EMPTY(diff.tuples)) { /* Write changes to journal file. */ CHECK(update_soa_serial(kfetch->db, ver, &diff, mctx, @@ -9602,7 +9619,12 @@ keyfetch_done(isc_task_t *task, isc_event_t *event) { } failure: - + if (result != ISC_R_SUCCESS) { + dns_zone_log(zone, ISC_LOG_ERROR, + "error during managed-keys processing (%s): " + "DNSSEC validation may be at risk", + isc_result_totext(result)); + } dns_diff_clear(&diff); if (ver != NULL) dns_db_closeversion(kfetch->db, &ver, commit); @@ -9712,7 +9734,7 @@ zone_refreshkeys(dns_zone_t *zone) { } /* Acceptance timer expired? */ - if (kd.addhd != 0 && kd.addhd < now) + if (kd.addhd < now) timer = kd.addhd; /* Or do we just need to refresh the keyset? */