diff --git a/CHANGES b/CHANGES index 737bff38ed..b9810c96ea 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,8 @@ +4885. [security] update-policy rules that otherwise ignore the name + field now require that it be set to "." to ensure + that any type list present is properly interpreted. + [RT #47126] + 4884. [bug] named could crash on shutdown due to a race between shutdown_server() and ns__client_request(). [RT #47120] diff --git a/bin/named/zoneconf.c b/bin/named/zoneconf.c index 610588ed9e..d132cfb7cc 100644 --- a/bin/named/zoneconf.c +++ b/bin/named/zoneconf.c @@ -233,37 +233,10 @@ configure_zone_ssutable(const cfg_obj_t *zconfig, dns_zone_t *zone, INSIST(0); str = cfg_obj_asstring(matchtype); - if (strcasecmp(str, "name") == 0) - mtype = dns_ssumatchtype_name; - else if (strcasecmp(str, "subdomain") == 0) - mtype = dns_ssumatchtype_subdomain; - else if (strcasecmp(str, "wildcard") == 0) - mtype = dns_ssumatchtype_wildcard; - else if (strcasecmp(str, "self") == 0) - mtype = dns_ssumatchtype_self; - else if (strcasecmp(str, "selfsub") == 0) - mtype = dns_ssumatchtype_selfsub; - else if (strcasecmp(str, "selfwild") == 0) - mtype = dns_ssumatchtype_selfwild; - else if (strcasecmp(str, "ms-self") == 0) - mtype = dns_ssumatchtype_selfms; - else if (strcasecmp(str, "krb5-self") == 0) - mtype = dns_ssumatchtype_selfkrb5; - else if (strcasecmp(str, "ms-subdomain") == 0) - mtype = dns_ssumatchtype_subdomainms; - else if (strcasecmp(str, "krb5-subdomain") == 0) - mtype = dns_ssumatchtype_subdomainkrb5; - else if (strcasecmp(str, "tcp-self") == 0) - mtype = dns_ssumatchtype_tcpself; - else if (strcasecmp(str, "6to4-self") == 0) - mtype = dns_ssumatchtype_6to4self; - else if (strcasecmp(str, "zonesub") == 0) { - mtype = dns_ssumatchtype_subdomain; + CHECK(dns_ssu_mtypefromstring(str, &mtype)); + if (mtype == dns_ssumatchtype_subdomain) { usezone = ISC_TRUE; - } else if (strcasecmp(str, "external") == 0) - mtype = dns_ssumatchtype_external; - else - INSIST(0); + } dns_fixedname_init(&fident); str = cfg_obj_asstring(identity); diff --git a/bin/tests/system/checkconf/bad-update-policy1.conf b/bin/tests/system/checkconf/bad-update-policy1.conf new file mode 100644 index 0000000000..91cc51db1c --- /dev/null +++ b/bin/tests/system/checkconf/bad-update-policy1.conf @@ -0,0 +1,7 @@ +zone "example.com" { + type master; + file "example.com.db"; + update-policy { + grant * self TXT; + }; +}; diff --git a/bin/tests/system/checkconf/bad-update-policy2.conf b/bin/tests/system/checkconf/bad-update-policy2.conf new file mode 100644 index 0000000000..ae393514a9 --- /dev/null +++ b/bin/tests/system/checkconf/bad-update-policy2.conf @@ -0,0 +1,7 @@ +zone "example.com" { + type master; + file "example.com.db"; + update-policy { + grant * selfsub TXT; + }; +}; diff --git a/bin/tests/system/checkconf/bad-update-policy3.conf b/bin/tests/system/checkconf/bad-update-policy3.conf new file mode 100644 index 0000000000..792231d62b --- /dev/null +++ b/bin/tests/system/checkconf/bad-update-policy3.conf @@ -0,0 +1,7 @@ +zone "example.com" { + type master; + file "example.com.db"; + update-policy { + grant * selfwild TXT; + }; +}; diff --git a/bin/tests/system/checkconf/bad-update-policy4.conf b/bin/tests/system/checkconf/bad-update-policy4.conf new file mode 100644 index 0000000000..7175a433a3 --- /dev/null +++ b/bin/tests/system/checkconf/bad-update-policy4.conf @@ -0,0 +1,7 @@ +zone "example.com" { + type master; + file "example.com.db"; + update-policy { + grant * ms-self TXT; + }; +}; diff --git a/bin/tests/system/checkconf/bad-update-policy5.conf b/bin/tests/system/checkconf/bad-update-policy5.conf new file mode 100644 index 0000000000..93bb95cb5c --- /dev/null +++ b/bin/tests/system/checkconf/bad-update-policy5.conf @@ -0,0 +1,7 @@ +zone "example.com" { + type master; + file "example.com.db"; + update-policy { + grant * krb5-self TXT; + }; +}; diff --git a/bin/tests/system/checkconf/bad-update-policy6.conf b/bin/tests/system/checkconf/bad-update-policy6.conf new file mode 100644 index 0000000000..df7c9caee6 --- /dev/null +++ b/bin/tests/system/checkconf/bad-update-policy6.conf @@ -0,0 +1,7 @@ +zone "example.com" { + type master; + file "example.com.db"; + update-policy { + grant * ms-subdomain TXT; + }; +}; diff --git a/bin/tests/system/checkconf/bad-update-policy7.conf b/bin/tests/system/checkconf/bad-update-policy7.conf new file mode 100644 index 0000000000..c11555a80f --- /dev/null +++ b/bin/tests/system/checkconf/bad-update-policy7.conf @@ -0,0 +1,7 @@ +zone "example.com" { + type master; + file "example.com.db"; + update-policy { + grant * krb5-subdomain TXT; + }; +}; diff --git a/bin/tests/system/checkconf/bad-update-policy8.conf b/bin/tests/system/checkconf/bad-update-policy8.conf new file mode 100644 index 0000000000..1b04def92a --- /dev/null +++ b/bin/tests/system/checkconf/bad-update-policy8.conf @@ -0,0 +1,7 @@ +zone "example.com" { + type master; + file "example.com.db"; + update-policy { + grant * tcp-self TXT; + }; +}; diff --git a/bin/tests/system/checkconf/bad-update-policy9.conf b/bin/tests/system/checkconf/bad-update-policy9.conf new file mode 100644 index 0000000000..2286751735 --- /dev/null +++ b/bin/tests/system/checkconf/bad-update-policy9.conf @@ -0,0 +1,7 @@ +zone "example.com" { + type master; + file "example.com.db"; + update-policy { + grant * 6to4-self TXT; + }; +}; diff --git a/bin/tests/system/checkconf/good-update-policy1.conf b/bin/tests/system/checkconf/good-update-policy1.conf new file mode 100644 index 0000000000..7f0b205710 --- /dev/null +++ b/bin/tests/system/checkconf/good-update-policy1.conf @@ -0,0 +1,7 @@ +zone "example.com" { + type master; + file "example.com.db"; + update-policy { + grant * self * TXT; + }; +}; diff --git a/bin/tests/system/checkconf/good-update-policy10.conf b/bin/tests/system/checkconf/good-update-policy10.conf new file mode 100644 index 0000000000..563abdf8bc --- /dev/null +++ b/bin/tests/system/checkconf/good-update-policy10.conf @@ -0,0 +1,7 @@ +zone "example.com" { + type master; + file "example.com.db"; + update-policy { + grant * krb5-subdomain . TXT; + }; +}; diff --git a/bin/tests/system/checkconf/good-update-policy11.conf b/bin/tests/system/checkconf/good-update-policy11.conf new file mode 100644 index 0000000000..a7baf3b8d3 --- /dev/null +++ b/bin/tests/system/checkconf/good-update-policy11.conf @@ -0,0 +1,7 @@ +zone "example.com" { + type master; + file "example.com.db"; + update-policy { + grant * tcp-self . TXT; + }; +}; diff --git a/bin/tests/system/checkconf/good-update-policy12.conf b/bin/tests/system/checkconf/good-update-policy12.conf new file mode 100644 index 0000000000..898c256929 --- /dev/null +++ b/bin/tests/system/checkconf/good-update-policy12.conf @@ -0,0 +1,7 @@ +zone "example.com" { + type master; + file "example.com.db"; + update-policy { + grant * 6to4-self . TXT; + }; +}; diff --git a/bin/tests/system/checkconf/good-update-policy2.conf b/bin/tests/system/checkconf/good-update-policy2.conf new file mode 100644 index 0000000000..81c25bd59b --- /dev/null +++ b/bin/tests/system/checkconf/good-update-policy2.conf @@ -0,0 +1,7 @@ +zone "example.com" { + type master; + file "example.com.db"; + update-policy { + grant * self . TXT; + }; +}; diff --git a/bin/tests/system/checkconf/good-update-policy3.conf b/bin/tests/system/checkconf/good-update-policy3.conf new file mode 100644 index 0000000000..f7fe7f1ca1 --- /dev/null +++ b/bin/tests/system/checkconf/good-update-policy3.conf @@ -0,0 +1,7 @@ +zone "example.com" { + type master; + file "example.com.db"; + update-policy { + grant * selfsub . TXT; + }; +}; diff --git a/bin/tests/system/checkconf/good-update-policy4.conf b/bin/tests/system/checkconf/good-update-policy4.conf new file mode 100644 index 0000000000..c40476109d --- /dev/null +++ b/bin/tests/system/checkconf/good-update-policy4.conf @@ -0,0 +1,7 @@ +zone "example.com" { + type master; + file "example.com.db"; + update-policy { + grant * selfsub * TXT; + }; +}; diff --git a/bin/tests/system/checkconf/good-update-policy5.conf b/bin/tests/system/checkconf/good-update-policy5.conf new file mode 100644 index 0000000000..d1f2f0b6f5 --- /dev/null +++ b/bin/tests/system/checkconf/good-update-policy5.conf @@ -0,0 +1,7 @@ +zone "example.com" { + type master; + file "example.com.db"; + update-policy { + grant * selfwild * TXT; + }; +}; diff --git a/bin/tests/system/checkconf/good-update-policy6.conf b/bin/tests/system/checkconf/good-update-policy6.conf new file mode 100644 index 0000000000..424f51185a --- /dev/null +++ b/bin/tests/system/checkconf/good-update-policy6.conf @@ -0,0 +1,7 @@ +zone "example.com" { + type master; + file "example.com.db"; + update-policy { + grant * selfwild . TXT; + }; +}; diff --git a/bin/tests/system/checkconf/good-update-policy7.conf b/bin/tests/system/checkconf/good-update-policy7.conf new file mode 100644 index 0000000000..e2cf613d0d --- /dev/null +++ b/bin/tests/system/checkconf/good-update-policy7.conf @@ -0,0 +1,7 @@ +zone "example.com" { + type master; + file "example.com.db"; + update-policy { + grant * krb5-self . TXT; + }; +}; diff --git a/bin/tests/system/checkconf/good-update-policy8.conf b/bin/tests/system/checkconf/good-update-policy8.conf new file mode 100644 index 0000000000..ec8610b5f1 --- /dev/null +++ b/bin/tests/system/checkconf/good-update-policy8.conf @@ -0,0 +1,7 @@ +zone "example.com" { + type master; + file "example.com.db"; + update-policy { + grant * ms-self . TXT; + }; +}; diff --git a/bin/tests/system/checkconf/good-update-policy9.conf b/bin/tests/system/checkconf/good-update-policy9.conf new file mode 100644 index 0000000000..9c37dbcd63 --- /dev/null +++ b/bin/tests/system/checkconf/good-update-policy9.conf @@ -0,0 +1,7 @@ +zone "example.com" { + type master; + file "example.com.db"; + update-policy { + grant * ms-subdomain . TXT; + }; +}; diff --git a/bin/tests/system/checkconf/tests.sh b/bin/tests/system/checkconf/tests.sh index b6bb12a9c6..6334c4cdb5 100644 --- a/bin/tests/system/checkconf/tests.sh +++ b/bin/tests/system/checkconf/tests.sh @@ -40,14 +40,24 @@ status=`expr $status + $ret` for bad in bad-*.conf do - n=`expr $n + 1` - echo "I: checking that named-checkconf detects error in $bad ($n)" - ret=0 - $CHECKCONF $bad > checkconf.out 2>&1 - if [ $? != 1 ]; then ret=1; fi - grep "^$bad:[0-9]*: " checkconf.out > /dev/null || ret=1 - if [ $ret != 0 ]; then echo "I:failed"; fi - status=`expr $status + $ret` + n=`expr $n + 1` + echo "I: checking that named-checkconf detects error in $bad ($n)" + ret=0 + $CHECKCONF $bad > checkconf.out 2>&1 + if [ $? != 1 ]; then ret=1; fi + grep "^$bad:[0-9]*: " checkconf.out > /dev/null || ret=1 + case $bad in + bad-update-policy[123].conf) + pat="identity and name fields are not the same" + grep "$pat" checkconf.out > /dev/null || ret=1 + ;; + bad-update-policy*.conf) + pat="name field not set to placeholder value" + grep "$pat" checkconf.out > /dev/null || ret=1 + ;; + esac + if [ $ret != 0 ]; then echo "I:failed"; fi + status=`expr $status + $ret` done for good in good-*.conf diff --git a/doc/arm/Bv9ARM-book.xml b/doc/arm/Bv9ARM-book.xml index 1dbf9bd32f..dde2fb9eb9 100644 --- a/doc/arm/Bv9ARM-book.xml +++ b/doc/arm/Bv9ARM-book.xml @@ -12613,7 +12613,8 @@ example.com. NS ns2.example.net. identity field. The name field is ignored, but should be the same as the - identity field. + identity field or + "." The self nametype is most useful when allowing using one key per name to update, where the key has the same @@ -12662,7 +12663,7 @@ example.com. NS ns2.example.net. and converts it machine.realm allowing the machine to update machine.realm. The REALM to be matched is specified in the identity - field. + field. The name field should be set to "." @@ -12694,7 +12695,7 @@ example.com. NS ns2.example.net. and converts it machine.realm allowing the machine to update machine.realm. The REALM to be matched is specified in the identity - field. + field. The name field should be set to "." @@ -12710,7 +12711,8 @@ example.com. NS ns2.example.net. converts it to machine.realm allowing the machine to update subdomains of machine.realm. The REALM to be matched is specified in the - identity field. + identity field. The + name field should be set to "." @@ -12724,7 +12726,8 @@ example.com. NS ns2.example.net. Allow updates that have been sent via TCP and for which the standard mapping from the initiating IP address into the IN-ADDR.ARPA and IP6.ARPA - namespaces match the name to be updated. + namespaces match the name to be updated. The + name field should be set to "." It is theoretically possible to spoof these TCP diff --git a/lib/bind9/check.c b/lib/bind9/check.c index 4e03bd27fa..5184ca6b06 100644 --- a/lib/bind9/check.c +++ b/lib/bind9/check.c @@ -41,6 +41,7 @@ #include #include #include +#include #include @@ -1723,9 +1724,9 @@ check_update_policy(const cfg_obj_t *policy, isc_log_t *logctx) { isc_result_t tresult; const cfg_listelt_t *element; const cfg_listelt_t *element2; - dns_fixedname_t fixed; + dns_fixedname_t fixed_id, fixed_name; + dns_name_t *id, *name; const char *str; - isc_buffer_t b; /* Check for "update-policy local;" */ if (cfg_obj_isstring(policy) && @@ -1742,27 +1743,36 @@ check_update_policy(const cfg_obj_t *policy, isc_log_t *logctx) { const cfg_obj_t *matchtype = cfg_tuple_get(stmt, "matchtype"); const cfg_obj_t *dname = cfg_tuple_get(stmt, "name"); const cfg_obj_t *typelist = cfg_tuple_get(stmt, "types"); + dns_ssumatchtype_t mtype; + + dns_fixedname_init(&fixed_id); + dns_fixedname_init(&fixed_name); + id = dns_fixedname_name(&fixed_id); + name = dns_fixedname_name(&fixed_name); + + tresult = dns_ssu_mtypefromstring(cfg_obj_asstring(matchtype), + &mtype); + if (tresult != ISC_R_SUCCESS) { + cfg_obj_log(identity, logctx, ISC_LOG_ERROR, + "has a bad match-type"); + } - dns_fixedname_init(&fixed); str = cfg_obj_asstring(identity); - isc_buffer_constinit(&b, str, strlen(str)); - isc_buffer_add(&b, strlen(str)); - tresult = dns_name_fromtext(dns_fixedname_name(&fixed), &b, - dns_rootname, 0, NULL); + tresult = dns_name_fromstring(id, str, 1, NULL); if (tresult != ISC_R_SUCCESS) { cfg_obj_log(identity, logctx, ISC_LOG_ERROR, "'%s' is not a valid name", str); result = tresult; } + /* + * There is no name field for subzone. + */ if (tresult == ISC_R_SUCCESS && - strcasecmp(cfg_obj_asstring(matchtype), "zonesub") != 0) { - dns_fixedname_init(&fixed); + mtype != dns_ssumatchtype_subdomain) + { str = cfg_obj_asstring(dname); - isc_buffer_constinit(&b, str, strlen(str)); - isc_buffer_add(&b, strlen(str)); - tresult = dns_name_fromtext(dns_fixedname_name(&fixed), - &b, dns_rootname, 0, NULL); + tresult = dns_name_fromstring(name, str, 0, NULL); if (tresult != ISC_R_SUCCESS) { cfg_obj_log(dname, logctx, ISC_LOG_ERROR, "'%s' is not a valid name", str); @@ -1771,13 +1781,55 @@ check_update_policy(const cfg_obj_t *policy, isc_log_t *logctx) { } if (tresult == ISC_R_SUCCESS && - strcasecmp(cfg_obj_asstring(matchtype), "wildcard") == 0 && - !dns_name_iswildcard(dns_fixedname_name(&fixed))) { + mtype == dns_ssumatchtype_wildcard && + !dns_name_iswildcard(name)) + { cfg_obj_log(identity, logctx, ISC_LOG_ERROR, "'%s' is not a wildcard", str); result = ISC_R_FAILURE; } + /* + * For some match types, the name should be a placeholder + * value, either "." or the same as identity. + */ + switch (mtype) { + case dns_ssumatchtype_self: + case dns_ssumatchtype_selfsub: + case dns_ssumatchtype_selfwild: + if (tresult == ISC_R_SUCCESS && + (!dns_name_equal(id, name) && + !dns_name_equal(dns_rootname, name))) { + cfg_obj_log(identity, logctx, ISC_LOG_ERROR, + "identity and name fields are not " + "the same"); + result = ISC_R_FAILURE; + } + break; + case dns_ssumatchtype_selfkrb5: + case dns_ssumatchtype_selfms: + case dns_ssumatchtype_subdomainms: + case dns_ssumatchtype_subdomainkrb5: + case dns_ssumatchtype_tcpself: + case dns_ssumatchtype_6to4self: + if (tresult == ISC_R_SUCCESS && + !dns_name_equal(dns_rootname, name)) { + cfg_obj_log(identity, logctx, ISC_LOG_ERROR, + "name field not set to " + "placeholder value '.'"); + result = ISC_R_FAILURE; + } + break; + case dns_ssumatchtype_name: + case dns_ssumatchtype_subdomain: + case dns_ssumatchtype_wildcard: + case dns_ssumatchtype_external: + case dns_ssumatchtype_local: + break; + default: + INSIST(0); + } + for (element2 = cfg_list_first(typelist); element2 != NULL; element2 = cfg_list_next(element2)) diff --git a/lib/dns/include/dns/ssu.h b/lib/dns/include/dns/ssu.h index f22e549433..a8992c13cf 100644 --- a/lib/dns/include/dns/ssu.h +++ b/lib/dns/include/dns/ssu.h @@ -206,15 +206,28 @@ isc_result_t dns_ssutable_nextrule(dns_ssurule_t *rule, *\li #ISC_R_NOMORE */ - -/*%< - * Check a policy rule via an external application - */ isc_boolean_t dns_ssu_external_match(const dns_name_t *identity, const dns_name_t *signer, const dns_name_t *name, const isc_netaddr_t *tcpaddr, dns_rdatatype_t type, const dst_key_t *key, isc_mem_t *mctx); +/*%< + * Check a policy rule via an external application + */ + +isc_result_t +dns_ssu_mtypefromstring(const char *str, dns_ssumatchtype_t *mtype); +/*%< + * Set 'mtype' from 'str' + * + * Requires: + *\li 'str' is not NULL. + *\li 'mtype' is not NULL, + * + * Returns: + *\li #ISC_R_SUCCESS + *\li #ISC_R_NOTFOUND + */ ISC_LANG_ENDDECLS diff --git a/lib/dns/ssu.c b/lib/dns/ssu.c index 0be3ca8d6e..bda986ff44 100644 --- a/lib/dns/ssu.c +++ b/lib/dns/ssu.c @@ -642,3 +642,43 @@ dns_ssutable_createdlz(isc_mem_t *mctx, dns_ssutable_t **tablep, *tablep = table; return (ISC_R_SUCCESS); } + +isc_result_t +dns_ssu_mtypefromstring(const char *str, dns_ssumatchtype_t *mtype) { + + REQUIRE(str != NULL); + REQUIRE(mtype != NULL); + + if (strcasecmp(str, "name") == 0) { + *mtype = dns_ssumatchtype_name; + } else if (strcasecmp(str, "subdomain") == 0) { + *mtype = dns_ssumatchtype_subdomain; + } else if (strcasecmp(str, "wildcard") == 0) { + *mtype = dns_ssumatchtype_wildcard; + } else if (strcasecmp(str, "self") == 0) { + *mtype = dns_ssumatchtype_self; + } else if (strcasecmp(str, "selfsub") == 0) { + *mtype = dns_ssumatchtype_selfsub; + } else if (strcasecmp(str, "selfwild") == 0) { + *mtype = dns_ssumatchtype_selfwild; + } else if (strcasecmp(str, "ms-self") == 0) { + *mtype = dns_ssumatchtype_selfms; + } else if (strcasecmp(str, "krb5-self") == 0) { + *mtype = dns_ssumatchtype_selfkrb5; + } else if (strcasecmp(str, "ms-subdomain") == 0) { + *mtype = dns_ssumatchtype_subdomainms; + } else if (strcasecmp(str, "krb5-subdomain") == 0) { + *mtype = dns_ssumatchtype_subdomainkrb5; + } else if (strcasecmp(str, "tcp-self") == 0) { + *mtype = dns_ssumatchtype_tcpself; + } else if (strcasecmp(str, "6to4-self") == 0) { + *mtype = dns_ssumatchtype_6to4self; + } else if (strcasecmp(str, "zonesub") == 0) { + *mtype = dns_ssumatchtype_subdomain; + } else if (strcasecmp(str, "external") == 0) { + *mtype = dns_ssumatchtype_external; + } else { + return (ISC_R_NOTFOUND); + } + return (ISC_R_SUCCESS); +} diff --git a/lib/dns/win32/libdns.def.in b/lib/dns/win32/libdns.def.in index f25e73c199..cb7fbd40b7 100644 --- a/lib/dns/win32/libdns.def.in +++ b/lib/dns/win32/libdns.def.in @@ -1018,6 +1018,7 @@ dns_soa_setrefresh dns_soa_setretry dns_soa_setserial dns_ssu_external_match +dns_ssu_mtypefromstring dns_ssutable_addrule dns_ssutable_attach dns_ssutable_checkrules