diff --git a/CHANGES b/CHANGES index 92ef0bc990..ff22e9efee 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 2101c1ded2..818838b142 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 2ffcfda24e..40310a2d70 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 7f6b79c90d..76e654c64f 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 29c0a88efd..31afb8e1d2 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 daeed6dd05..19ef0cb701 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 fb134742d2..054422de55 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 d222b7faea..71e466df40 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 0000000000..ad3979d7a7 --- /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 0000000000..98204929ad --- /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 d555c4e93e..bb70fe1df3 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 b9806f3431..c5ebc88ac5 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 1ed96d4e0c..0e0864a58b 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 77b3a421f0..4353284f99 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 690b4282d2..12fd87fb9c 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 f6a4b68f4e..3b179425d0 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 6a142fc633..cf285383f5 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 0ecb342083..944b7bf0f4 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 1d8269912e..9a8d9b3819 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 a3dab22baa..baf9ad0a78 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 022ab20458..1421330c70 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 db04dba9fb..23ba292207 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 db9d4baf0d..d0138b800d 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 4966de4cc3..3ca053adbd 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? */