From a520662ed4b1a89f5fb8d4206ec6bb75743cbc46 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Wed, 2 Jan 2019 17:29:59 +1100 Subject: [PATCH 1/3] allow dlz to signal that the view's transfer acl should be used --- lib/dns/dlz.c | 12 +++++++++--- lib/dns/include/dns/dlz.h | 9 +++++---- lib/dns/include/dns/view.h | 22 +++++++++++----------- lib/dns/sdlz.c | 16 +++++++++++----- lib/isc/include/isc/result.h | 3 ++- lib/isc/result.c | 2 ++ lib/ns/xfrout.c | 29 +++++++++++++++++++++-------- 7 files changed, 61 insertions(+), 32 deletions(-) diff --git a/lib/dns/dlz.c b/lib/dns/dlz.c index 03d699c15e..b475e36dfe 100644 --- a/lib/dns/dlz.c +++ b/lib/dns/dlz.c @@ -133,11 +133,17 @@ dns_dlzallowzonexfr(dns_view_t *view, const dns_name_t *name, view->rdclass, name, clientaddr, dbp); /* - * if ISC_R_NOPERM, we found the right database but - * the zone may not transfer. + * In these cases, we found the right database. Non-success + * result codes indicate the zone might not transfer. */ - if (result == ISC_R_SUCCESS || result == ISC_R_NOPERM) + switch (result) { + case ISC_R_SUCCESS: + case ISC_R_NOPERM: + case ISC_R_DEFAULT: return (result); + default: + break; + } } if (result == ISC_R_NOTIMPLEMENTED) diff --git a/lib/dns/include/dns/dlz.h b/lib/dns/include/dns/dlz.h index 0f8cd3eb30..e769a97dc6 100644 --- a/lib/dns/include/dns/dlz.h +++ b/lib/dns/include/dns/dlz.h @@ -107,10 +107,11 @@ typedef isc_result_t * the DNS server is performing a zone transfer query. The driver's * method should return ISC_R_SUCCESS and a database pointer to the * name server if the zone is supported by the database, and zone - * transfer is allowed. Otherwise it will return ISC_R_NOTFOUND if - * the zone is not supported by the database, or ISC_R_NOPERM if zone - * transfers are not allowed. If an error occurs it should return a - * result code indicating the type of error. + * transfer is allowed. If the view's transfer acl should be used, + * then the driver's method should return ISC_R_DEFAULT. Otherwise, + * it should return ISC_R_NOTFOUND if the zone is not supported by + * the database, or ISC_R_NOPERM if zone transfers are not allowed. + * If an error occurs, the result code should indicate the type of error. */ typedef isc_result_t diff --git a/lib/dns/include/dns/view.h b/lib/dns/include/dns/view.h index 43a3e9e016..f17e9733d0 100644 --- a/lib/dns/include/dns/view.h +++ b/lib/dns/include/dns/view.h @@ -100,7 +100,7 @@ struct dns_view { dns_ntatable_t * ntatable_priv; isc_mutex_t lock; - bool frozen; + bool frozen; isc_task_t * task; isc_event_t resevent; isc_event_t adbevent; @@ -108,7 +108,7 @@ struct dns_view { isc_stats_t * adbstats; isc_stats_t * resstats; dns_stats_t * resquerystats; - bool cacheshared; + bool cacheshared; /* Configurable data. */ dns_tsig_keyring_t * statickeys; @@ -144,14 +144,14 @@ struct dns_view { dns_acl_t * upfwdacl; dns_acl_t * denyansweracl; dns_acl_t * nocasecompress; - bool msgcompression; + bool msgcompression; dns_rbt_t * answeracl_exclude; dns_rbt_t * denyanswernames; dns_rbt_t * answernames_exclude; dns_rrl_t * rrl; - bool provideixfr; - bool requestnsid; - bool sendcookie; + bool provideixfr; + bool requestnsid; + bool sendcookie; dns_ttl_t maxcachettl; dns_ttl_t maxncachettl; dns_ttl_t mincachettl; @@ -164,17 +164,17 @@ struct dns_view { in_port_t dstport; dns_aclenv_t aclenv; dns_rdatatype_t preferred_glue; - bool flush; + bool flush; dns_namelist_t * delonly; - bool rootdelonly; + bool rootdelonly; dns_namelist_t * rootexclude; - bool checknames; + bool checknames; dns_name_t * dlv; dns_fixedname_t dlv_fixed; uint16_t maxudp; dns_ttl_t staleanswerttl; dns_stale_answer_t staleanswersok; /* rndc setting */ - bool staleanswersenable; /* named.conf setting */ + bool staleanswersenable; /* named.conf setting */ uint16_t nocookieudp; uint16_t padding; dns_acl_t * pad_acl; @@ -194,7 +194,7 @@ struct dns_view { */ dns_acl_t * matchclients; dns_acl_t * matchdestinations; - bool matchrecursiveonly; + bool matchrecursiveonly; /* Locked by themselves. */ isc_refcount_t references; diff --git a/lib/dns/sdlz.c b/lib/dns/sdlz.c index 22c231de17..2fd1f900a8 100644 --- a/lib/dns/sdlz.c +++ b/lib/dns/sdlz.c @@ -1610,17 +1610,23 @@ dns_sdlzallowzonexfr(void *driverarg, void *dbdata, isc_mem_t *mctx, /* Call SDLZ driver's find zone method */ if (imp->methods->allowzonexfr != NULL) { + isc_result_t rresult = ISC_R_SUCCESS; + MAYBE_LOCK(imp); result = imp->methods->allowzonexfr(imp->driverarg, dbdata, namestr, clientstr); MAYBE_UNLOCK(imp); /* - * if zone is supported and transfers allowed build a 'bind' - * database driver + * if zone is supported and transfers are (or might be) + * allowed, build a 'bind' database driver */ - if (result == ISC_R_SUCCESS) - result = dns_sdlzcreateDBP(mctx, driverarg, dbdata, - name, rdclass, dbp); + if (result == ISC_R_SUCCESS || result == ISC_R_DEFAULT) { + rresult = dns_sdlzcreateDBP(mctx, driverarg, dbdata, + name, rdclass, dbp); + } + if (rresult != ISC_R_SUCCESS) { + result = rresult; + } return (result); } diff --git a/lib/isc/include/isc/result.h b/lib/isc/include/isc/result.h index 36792fa0dc..634a343b5d 100644 --- a/lib/isc/include/isc/result.h +++ b/lib/isc/include/isc/result.h @@ -87,9 +87,10 @@ #define ISC_R_CRYPTOFAILURE 65 /*%< cryptography library failure */ #define ISC_R_DISCQUOTA 66 /*%< disc quota */ #define ISC_R_DISCFULL 67 /*%< disc full */ +#define ISC_R_DEFAULT 68 /*%< default */ /*% Not a result code: the number of results. */ -#define ISC_R_NRESULTS 68 +#define ISC_R_NRESULTS 69 ISC_LANG_BEGINDECLS diff --git a/lib/isc/result.c b/lib/isc/result.c index efec952484..3cfaa86c72 100644 --- a/lib/isc/result.c +++ b/lib/isc/result.c @@ -99,6 +99,7 @@ static const char *description[ISC_R_NRESULTS] = { "crypto failure", /*%< 65 */ "disc quota", /*%< 66 */ "disc full", /*%< 67 */ + "default", /*%< 68 */ }; static const char *identifier[ISC_R_NRESULTS] = { @@ -170,6 +171,7 @@ static const char *identifier[ISC_R_NRESULTS] = { "ISC_R_CRYPTOFAILURE", "ISC_R_DISCQUOTA", "ISC_R_DISCFULL", + "ISC_R_DEFAULT", }; #define ISC_RESULT_RESULTSET 2 diff --git a/lib/ns/xfrout.c b/lib/ns/xfrout.c index e46eaf41e3..ff383f68fe 100644 --- a/lib/ns/xfrout.c +++ b/lib/ns/xfrout.c @@ -763,6 +763,7 @@ ns_xfr_start(ns_client_t *client, dns_rdatatype_t reqtype) { bool is_poll = false; bool is_dlz = false; bool is_ixfr = false; + bool useviewacl = false; uint32_t begin_serial = 0, current_serial; switch (reqtype) { @@ -826,7 +827,10 @@ ns_xfr_start(ns_client_t *client, dns_rdatatype_t reqtype) { question_name, &client->peeraddr, &db); - + if (result == ISC_R_DEFAULT) { + useviewacl = true; + result = ISC_R_SUCCESS; + } if (result == ISC_R_NOPERM) { char _buf1[DNS_NAME_FORMATSIZE]; char _buf2[DNS_RDATACLASS_FORMATSIZE]; @@ -843,9 +847,10 @@ ns_xfr_start(ns_client_t *client, dns_rdatatype_t reqtype) { _buf1, _buf2); goto failure; } - if (result != ISC_R_SUCCESS) + if (result != ISC_R_SUCCESS) { FAILQ(DNS_R_NOTAUTH, "non-authoritative zone", question_name, question_class); + } is_dlz = true; } else { /* @@ -926,14 +931,21 @@ ns_xfr_start(ns_client_t *client, dns_rdatatype_t reqtype) { "%s authority section OK", mnemonic); /* - * If not a DLZ zone, decide whether to allow this transfer. + * If not a DLZ zone or we are falling back to the view's transfer + * ACL, decide whether to allow this transfer. */ - if (!is_dlz) { + if (!is_dlz || useviewacl) { + dns_acl_t *acl; + ns_client_aclmsg("zone transfer", question_name, reqtype, client->view->rdclass, msg, sizeof(msg)); - CHECK(ns_client_checkacl(client, NULL, msg, - dns_zone_getxfracl(zone), - true, ISC_LOG_ERROR)); + if (useviewacl) { + acl = client->view->transferacl; + } else { + acl = dns_zone_getxfracl(zone); + } + CHECK(ns_client_checkacl(client, NULL, msg, acl, true, + ISC_LOG_ERROR)); } /* @@ -958,8 +970,9 @@ ns_xfr_start(ns_client_t *client, dns_rdatatype_t reqtype) { /* * Get a dynamically allocated copy of the current SOA. */ - if (is_dlz) + if (is_dlz) { dns_db_currentversion(db, &ver); + } CHECK(dns_db_createsoatuple(db, ver, mctx, DNS_DIFFOP_EXISTS, ¤t_soa_tuple)); From e2062879c18be5a8dc0750c83d5d5f20345d9a2f Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Thu, 7 Mar 2019 22:55:16 -0800 Subject: [PATCH 2/3] test the use of the view ACL in DLZ --- bin/tests/system/dlzexternal/driver.c | 35 +++++++++++++++++++ bin/tests/system/dlzexternal/ns1/dlzs.conf.in | 4 +++ .../system/dlzexternal/ns1/named.conf.in | 1 + bin/tests/system/dlzexternal/tests.sh | 10 ++++++ 4 files changed, 50 insertions(+) diff --git a/bin/tests/system/dlzexternal/driver.c b/bin/tests/system/dlzexternal/driver.c index 35151441d4..6767bfc64a 100644 --- a/bin/tests/system/dlzexternal/driver.c +++ b/bin/tests/system/dlzexternal/driver.c @@ -534,21 +534,56 @@ dlz_lookup(const char *zone, const char *name, void *dbdata, */ isc_result_t dlz_allowzonexfr(void *dbdata, const char *name, const char *client) { + struct dlz_example_data *state = (struct dlz_example_data *)dbdata; isc_result_t result; + if (state->log != NULL) { + state->log(ISC_LOG_INFO, + "dlz_example: dlz_allowzonexfr called for %s", + name); + } + result = dlz_findzonedb(dbdata, name, NULL, NULL); if (result != ISC_R_SUCCESS) { + if (state->log != NULL) { + state->log(ISC_LOG_INFO, + "dlz_example: findzonedb returned %s", + isc_result_totext(result)); + } return (result); } + /* + * Exception for "example.org" so we can test the use of + * the view ACL. + */ + if (strcmp(name, "example.org") == 0) { + if (state->log != NULL) { + state->log(ISC_LOG_INFO, + "dlz_example: use view ACL " + "for example.org"); + } + return (ISC_R_DEFAULT); + } + /* * Exception for 10.53.0.5 so we can test that allow-transfer * is effective. */ if (strcmp(client, "10.53.0.5") == 0) { + if (state->log != NULL) { + state->log(ISC_LOG_INFO, + "dlz_example: disallow transfer " + "to 10.53.0.5"); + } return (ISC_R_NOPERM); } + if (state->log != NULL) { + state->log(ISC_LOG_INFO, + "dlz_example: transfer allowed for %s", name); + } + return (ISC_R_SUCCESS); } diff --git a/bin/tests/system/dlzexternal/ns1/dlzs.conf.in b/bin/tests/system/dlzexternal/ns1/dlzs.conf.in index d583cb4e1a..07bf329b50 100644 --- a/bin/tests/system/dlzexternal/ns1/dlzs.conf.in +++ b/bin/tests/system/dlzexternal/ns1/dlzs.conf.in @@ -17,6 +17,10 @@ dlz "example two" { database "dlopen ../driver.@SO@ alternate.nil"; }; +dlz "example three" { + database "dlopen ../driver.@SO@ example.org"; +}; + dlz "unsearched1" { database "dlopen ../driver.@SO@ other.nil"; search no; diff --git a/bin/tests/system/dlzexternal/ns1/named.conf.in b/bin/tests/system/dlzexternal/ns1/named.conf.in index d35061a8cb..591061cb85 100644 --- a/bin/tests/system/dlzexternal/ns1/named.conf.in +++ b/bin/tests/system/dlzexternal/ns1/named.conf.in @@ -18,6 +18,7 @@ options { session-keyfile "session.key"; listen-on { 10.53.0.1; 127.0.0.1; }; listen-on-v6 { none; }; + allow-transfer { !10.53.0.1; any; }; recursion no; notify yes; }; diff --git a/bin/tests/system/dlzexternal/tests.sh b/bin/tests/system/dlzexternal/tests.sh index 6c84ad10eb..b9d44057be 100644 --- a/bin/tests/system/dlzexternal/tests.sh +++ b/bin/tests/system/dlzexternal/tests.sh @@ -125,6 +125,16 @@ grep "; Transfer failed" dig.out.alternate.ns1.test$n > /dev/null || ret=1 [ "$ret" -eq 0 ] || echo_i "failed" status=`expr $status + $ret` +newtest "testing AXFR denied based on view ACL" +# 10.53.0.1 should be disallowed +$DIG $DIGOPTS -b 10.53.0.1 +noall +answer axfr example.org > dig.out.example.ns1.test$n.1 +grep "; Transfer failed" dig.out.example.ns1.test$n.1 > /dev/null || ret=1 +# 10.53.0.2 should be allowed +$DIG $DIGOPTS -b 10.53.0.2 +noall +answer axfr example.org > dig.out.example.ns1.test$n.2 +grep "; Transfer failed" dig.out.example.ns1.test$n.2 > /dev/null && ret=1 +[ "$ret" -eq 0 ] || echo_i "failed" +status=`expr $status + $ret` + newtest "testing unsearched/unregistered DLZ zone is not found" $DIG $DIGOPTS +noall +answer ns other.nil > dig.out.ns1.test$n grep "3600.IN.NS.other.nil." dig.out.ns1.test$n > /dev/null && ret=1 From 7cc241ca394191d785cdfc004cf82eb7dcae1e4e Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Thu, 7 Mar 2019 23:30:30 -0800 Subject: [PATCH 3/3] CHANGES --- CHANGES | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGES b/CHANGES index 8ce64fbb24..eeb7b203e3 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,7 @@ +5181. [func] Add a mechanism for a DLZ module to signal that + the view's allow-transfer ACL should be used to + determine whether transfers are allowed. [GL #803] + 5180. [bug] delv now honors the operating system's preferred ephemeral port range. [GL #925]