diff --git a/CHANGES b/CHANGES index 19b891cad0..ec4a709c58 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,6 @@ +5452. [bug] The "blackhole" ACL was accidentally disabled with + respect to client queries. [GL #1936] + 5451. [func] Add 'rndc dnssec -status' command. [GL #1612] 5450. [placeholder] diff --git a/bin/tests/system/acl/ns2/named5.conf.in b/bin/tests/system/acl/ns2/named5.conf.in index 4b4e05027a..7e20bac49d 100644 --- a/bin/tests/system/acl/ns2/named5.conf.in +++ b/bin/tests/system/acl/ns2/named5.conf.in @@ -31,6 +31,7 @@ options { ixfr-from-differences yes; check-integrity no; allow-query-on { 10.53.0.2; }; + blackhole { 10.53.0.8; }; }; key one { diff --git a/bin/tests/system/acl/tests.sh b/bin/tests/system/acl/tests.sh index 6f291bfc61..4d915244ba 100644 --- a/bin/tests/system/acl/tests.sh +++ b/bin/tests/system/acl/tests.sh @@ -143,6 +143,26 @@ $DIG -p ${PORT} +tcp soa example. \ @10.53.0.2 -b 10.53.0.3 > dig.out.${t} grep "status: NOERROR" dig.out.${t} > /dev/null 2>&1 || { echo_i "test $t failed" ; status=1; } +echo_i "testing blackhole ACL processing" +t=`expr $t + 1` +ret=0 +$DIG -p ${PORT} +tcp soa example. \ + @10.53.0.2 -b 10.53.0.3 > dig.out.1.${t} +grep "status: NOERROR" dig.out.1.${t} > /dev/null 2>&1 || ret=1 +$DIG -p ${PORT} +tcp soa example. \ + @10.53.0.2 -b 10.53.0.8 > dig.out.2.${t} +grep "status: NOERROR" dig.out.2.${t} > /dev/null 2>&1 && ret=1 +grep "communications error" dig.out.2.${t} > /dev/null 2>&1 || ret=1 +$DIG -p ${PORT} soa example. \ + @10.53.0.2 -b 10.53.0.3 > dig.out.3.${t} +grep "status: NOERROR" dig.out.3.${t} > /dev/null 2>&1 || ret=1 +$DIG -p ${PORT} soa example. \ + @10.53.0.2 -b 10.53.0.8 > dig.out.4.${t} +grep "status: NOERROR" dig.out.4.${t} > /dev/null 2>&1 && ret=1 +grep "connection timed out" dig.out.4.${t} > /dev/null 2>&1 || ret=1 +[ $ret -eq 0 ] || echo_i "failed" +status=`expr $status + $ret` + # AXFR tests against ns3 echo_i "testing allow-transfer ACLs against ns3 (no existing zones)" diff --git a/doc/notes/notes-current.rst b/doc/notes/notes-current.rst index 7d161d4709..f5c274b16f 100644 --- a/doc/notes/notes-current.rst +++ b/doc/notes/notes-current.rst @@ -57,3 +57,8 @@ Bug Fixes unsupported algorithm appeared earlier in the DNSKEY RRset than a supported algorithm. It could also stop if it detected a malformed public key. [GL #1689] + +- The ``blackhole`` ACL was inadvertently disabled with respect to + client queries. Blocked IP addresses were not used for upstream + queries but queries from those addresses could still be answered. + [GL #1936] diff --git a/lib/isc/include/isc/netmgr.h b/lib/isc/include/isc/netmgr.h index 4cb5b12289..67d95f3876 100644 --- a/lib/isc/include/isc/netmgr.h +++ b/lib/isc/include/isc/netmgr.h @@ -24,6 +24,47 @@ typedef enum { NMEV_SHUTDOWN } isc_nm_eventtype; +typedef void (*isc_nm_recv_cb_t)(isc_nmhandle_t *handle, isc_result_t eresult, + isc_region_t *region, void *cbarg); +/*%< + * Callback function to be used when receiving a packet. + * + * 'handle' the handle that can be used to send back the answer. + * 'eresult' the result of the event. + * 'region' contains the received data, if any. It will be freed + * after return by caller. + * 'cbarg' the callback argument passed to isc_nm_listenudp(), + * isc_nm_listentcpdns(), or isc_nm_read(). + */ +typedef isc_result_t (*isc_nm_accept_cb_t)(isc_nmhandle_t *handle, + isc_result_t result, void *cbarg); +/*%< + * Callback function to be used when accepting a connection. (This differs + * from isc_nm_cb_t below in that it returns a result code.) + * + * 'handle' the handle that can be used to send back the answer. + * 'eresult' the result of the event. + * 'cbarg' the callback argument passed to isc_nm_listentcp() or + * isc_nm_listentcpdns(). + */ + +typedef void (*isc_nm_cb_t)(isc_nmhandle_t *handle, isc_result_t result, + void *cbarg); +/*%< + * Callback function for other network completion events (send, connect). + * + * 'handle' the handle on which the event took place. + * 'eresult' the result of the event. + * 'cbarg' the callback argument passed to isc_nm_send(), + * isc_nm_tcp_connect(), or isc_nm_listentcp() + */ + +typedef void (*isc_nm_opaquecb_t)(void *arg); +/*%< + * Opaque callback function, used for isc_nmhandle 'reset' and 'free' + * callbacks. + */ + isc_nm_t * isc_nm_start(isc_mem_t *mctx, uint32_t workers); /*%< @@ -89,8 +130,6 @@ isc_nmhandle_getdata(isc_nmhandle_t *handle); void * isc_nmhandle_getextra(isc_nmhandle_t *handle); -typedef void (*isc_nm_opaquecb_t)(void *arg); - bool isc_nmhandle_is_stream(isc_nmhandle_t *handle); @@ -122,31 +161,6 @@ isc_nmhandle_netmgr(isc_nmhandle_t *handle); * Return a pointer to the netmgr object for the given handle. */ -typedef void (*isc_nm_recv_cb_t)(isc_nmhandle_t *handle, isc_result_t eresult, - isc_region_t *region, void *cbarg); -/*%< - * Callback function to be used when receiving a packet. - * - * 'handle' the handle that can be used to send back the answer. - * 'eresult' the result of the event. - * 'region' contains the received data, if any. It will be freed - * after return by caller. - * 'cbarg' the callback argument passed to isc_nm_listenudp(), - * isc_nm_listentcpdns(), or isc_nm_read(). - */ - -typedef void (*isc_nm_cb_t)(isc_nmhandle_t *handle, isc_result_t eresult, - void *cbarg); -/*%< - * Callback function for other network completion events (send, connect, - * accept). - * - * 'handle' the handle on which the event took place. - * 'eresult' the result of the event. - * 'cbarg' the callback argument passed to isc_nm_send(), - * isc_nm_tcp_connect(), or isc_nm_listentcp() - */ - isc_result_t isc_nm_listenudp(isc_nm_t *mgr, isc_nmiface_t *iface, isc_nm_recv_cb_t cb, void *cbarg, size_t extrasize, isc_nmsocket_t **sockp); @@ -226,9 +240,10 @@ isc_nm_send(isc_nmhandle_t *handle, isc_region_t *region, isc_nm_cb_t cb, */ isc_result_t -isc_nm_listentcp(isc_nm_t *mgr, isc_nmiface_t *iface, isc_nm_cb_t cb, - void *cbarg, size_t extrahandlesize, int backlog, - isc_quota_t *quota, isc_nmsocket_t **sockp); +isc_nm_listentcp(isc_nm_t *mgr, isc_nmiface_t *iface, + isc_nm_accept_cb_t accept_cb, void *accept_cbarg, + size_t extrahandlesize, int backlog, isc_quota_t *quota, + isc_nmsocket_t **sockp); /*%< * Start listening for raw messages over the TCP interface 'iface', using * net manager 'mgr'. @@ -236,8 +251,8 @@ isc_nm_listentcp(isc_nm_t *mgr, isc_nmiface_t *iface, isc_nm_cb_t cb, * On success, 'sockp' will be updated to contain a new listening TCP * socket. * - * When a message is received on the socket, 'cb' will be called with 'cbarg' - * as its argument. + * When connection is accepted on the socket, 'accept_cb' will be called with + * 'accept_cbarg' as its argument. The callback is expected to start a read. * * When handles are allocated for the socket, 'extrasize' additional bytes * will be allocated along with the handle for an associated object. @@ -267,9 +282,9 @@ isc_nm_tcpconnect(isc_nm_t *mgr, isc_nmiface_t *local, isc_nmiface_t *peer, isc_result_t isc_nm_listentcpdns(isc_nm_t *mgr, isc_nmiface_t *iface, isc_nm_recv_cb_t cb, - void *cbarg, isc_nm_cb_t accept_cb, void *accept_cbarg, - size_t extrahandlesize, int backlog, isc_quota_t *quota, - isc_nmsocket_t **sockp); + void *cbarg, isc_nm_accept_cb_t accept_cb, + void *accept_cbarg, size_t extrahandlesize, int backlog, + isc_quota_t *quota, isc_nmsocket_t **sockp); /*%< * Start listening for DNS messages over the TCP interface 'iface', using * net manager 'mgr'. @@ -283,7 +298,7 @@ isc_nm_listentcpdns(isc_nm_t *mgr, isc_nmiface_t *iface, isc_nm_recv_cb_t cb, * When a complete DNS message is received on the socket, 'cb' will be * called with 'cbarg' as its argument. * - * When a new TCP connection is accepted, 'accept_cb' will be called + * When a new TCPDNS connection is accepted, 'accept_cb' will be called * with 'accept_cbarg' as its argument. * * When handles are allocated for the socket, 'extrasize' additional bytes diff --git a/lib/isc/netmgr/netmgr-int.h b/lib/isc/netmgr/netmgr-int.h index 44938e7c56..1637741f51 100644 --- a/lib/isc/netmgr/netmgr-int.h +++ b/lib/isc/netmgr/netmgr-int.h @@ -150,7 +150,7 @@ typedef enum isc__netievent_type { */ typedef union { isc_nm_recv_cb_t recv; - isc_nm_cb_t accept; + isc_nm_accept_cb_t accept; } isc__nm_readcb_t; typedef union { @@ -160,7 +160,7 @@ typedef union { typedef union { isc_nm_recv_cb_t recv; - isc_nm_cb_t accept; + isc_nm_accept_cb_t accept; isc_nm_cb_t send; isc_nm_cb_t connect; } isc__nm_cb_t; diff --git a/lib/isc/netmgr/tcp.c b/lib/isc/netmgr/tcp.c index e3e222dead..5710e38bae 100644 --- a/lib/isc/netmgr/tcp.c +++ b/lib/isc/netmgr/tcp.c @@ -243,9 +243,10 @@ isc_nm_tcpconnect(isc_nm_t *mgr, isc_nmiface_t *local, isc_nmiface_t *peer, } isc_result_t -isc_nm_listentcp(isc_nm_t *mgr, isc_nmiface_t *iface, isc_nm_cb_t cb, - void *cbarg, size_t extrahandlesize, int backlog, - isc_quota_t *quota, isc_nmsocket_t **sockp) { +isc_nm_listentcp(isc_nm_t *mgr, isc_nmiface_t *iface, + isc_nm_accept_cb_t accept_cb, void *accept_cbarg, + size_t extrahandlesize, int backlog, isc_quota_t *quota, + isc_nmsocket_t **sockp) { isc_nmsocket_t *nsock = NULL; isc__netievent_tcplisten_t *ievent = NULL; @@ -253,8 +254,8 @@ isc_nm_listentcp(isc_nm_t *mgr, isc_nmiface_t *iface, isc_nm_cb_t cb, nsock = isc_mem_get(mgr->mctx, sizeof(*nsock)); isc__nmsocket_init(nsock, mgr, isc_nm_tcplistener, iface); - nsock->accept_cb.accept = cb; - nsock->accept_cbarg = cbarg; + nsock->accept_cb.accept = accept_cb; + nsock->accept_cbarg = accept_cbarg; nsock->extrahandlesize = extrahandlesize; nsock->backlog = backlog; nsock->result = ISC_R_SUCCESS; diff --git a/lib/isc/netmgr/tcpdns.c b/lib/isc/netmgr/tcpdns.c index 4e49e47c34..05cc410d68 100644 --- a/lib/isc/netmgr/tcpdns.c +++ b/lib/isc/netmgr/tcpdns.c @@ -102,7 +102,7 @@ dnstcp_readtimeout(uv_timer_t *timer) { /* * Accept callback for TCP-DNS connection. */ -static void +static isc_result_t dnslisten_acceptcb(isc_nmhandle_t *handle, isc_result_t result, void *cbarg) { isc_nmsocket_t *dnslistensock = (isc_nmsocket_t *)cbarg; isc_nmsocket_t *dnssock = NULL; @@ -110,14 +110,16 @@ dnslisten_acceptcb(isc_nmhandle_t *handle, isc_result_t result, void *cbarg) { REQUIRE(VALID_NMSOCK(dnslistensock)); REQUIRE(dnslistensock->type == isc_nm_tcpdnslistener); - /* If accept() was unnsuccessful we can't do anything */ if (result != ISC_R_SUCCESS) { - return; + return (result); } if (dnslistensock->accept_cb.accept != NULL) { - dnslistensock->accept_cb.accept(handle, ISC_R_SUCCESS, - dnslistensock->accept_cbarg); + result = dnslistensock->accept_cb.accept( + handle, ISC_R_SUCCESS, dnslistensock->accept_cbarg); + if (result != ISC_R_SUCCESS) { + return (result); + } } /* We need to create a 'wrapper' dnssocket for this connection */ @@ -144,12 +146,15 @@ dnslisten_acceptcb(isc_nmhandle_t *handle, isc_result_t result, void *cbarg) { dnssock->timer_initialized = true; uv_timer_start(&dnssock->timer, dnstcp_readtimeout, dnssock->read_timeout, 0); + isc_nmhandle_ref(handle); result = isc_nm_read(handle, dnslisten_readcb, dnssock); if (result != ISC_R_SUCCESS) { isc_nmhandle_unref(handle); } isc__nmsocket_detach(&dnssock); + + return (ISC_R_SUCCESS); } /* @@ -306,9 +311,9 @@ dnslisten_readcb(isc_nmhandle_t *handle, isc_result_t eresult, */ isc_result_t isc_nm_listentcpdns(isc_nm_t *mgr, isc_nmiface_t *iface, isc_nm_recv_cb_t cb, - void *cbarg, isc_nm_cb_t accept_cb, void *accept_cbarg, - size_t extrahandlesize, int backlog, isc_quota_t *quota, - isc_nmsocket_t **sockp) { + void *cbarg, isc_nm_accept_cb_t accept_cb, + void *accept_cbarg, size_t extrahandlesize, int backlog, + isc_quota_t *quota, isc_nmsocket_t **sockp) { /* A 'wrapper' socket object with outer set to true TCP socket */ isc_nmsocket_t *dnslistensock = isc_mem_get(mgr->mctx, sizeof(*dnslistensock)); diff --git a/lib/ns/client.c b/lib/ns/client.c index d91380aed0..6b00dae889 100644 --- a/lib/ns/client.c +++ b/lib/ns/client.c @@ -1625,7 +1625,6 @@ void ns__client_request(isc_nmhandle_t *handle, isc_result_t eresult, isc_region_t *region, void *arg) { ns_client_t *client; - bool newclient = false; ns_clientmgr_t *mgr; ns_interface_t *ifp; isc_result_t result; @@ -1724,29 +1723,23 @@ ns__client_request(isc_nmhandle_t *handle, isc_result_t eresult, } #endif /* if NS_CLIENT_DROPPORT */ + env = ns_interfacemgr_getaclenv(client->manager->interface->mgr); + if (client->sctx->blackholeacl != NULL && + (dns_acl_match(&netaddr, NULL, client->sctx->blackholeacl, env, + &match, NULL) == ISC_R_SUCCESS) && + match > 0) + { + ns_client_log(client, DNS_LOGCATEGORY_SECURITY, + NS_LOGMODULE_CLIENT, ISC_LOG_DEBUG(10), + "dropped request: blackholed peer"); + isc_task_unpause(client->task); + return; + } + ns_client_log(client, NS_LOGCATEGORY_CLIENT, NS_LOGMODULE_CLIENT, ISC_LOG_DEBUG(3), "%s request", TCP_CLIENT(client) ? "TCP" : "UDP"); - /* - * Check the blackhole ACL for UDP only, since TCP is done in - * client_newconn. - */ - env = ns_interfacemgr_getaclenv(client->manager->interface->mgr); - if (newclient) { - if (client->sctx->blackholeacl != NULL && - (dns_acl_match(&netaddr, NULL, client->sctx->blackholeacl, - env, &match, NULL) == ISC_R_SUCCESS) && - match > 0) - { - ns_client_log(client, DNS_LOGCATEGORY_SECURITY, - NS_LOGMODULE_CLIENT, ISC_LOG_DEBUG(10), - "blackholed UDP datagram"); - isc_task_unpause(client->task); - return; - } - } - result = dns_message_peekheader(buffer, &id, &flags); if (result != ISC_R_SUCCESS) { /* @@ -2213,17 +2206,36 @@ ns__client_request(isc_nmhandle_t *handle, isc_result_t eresult, isc_task_unpause(client->task); } -void +isc_result_t ns__client_tcpconn(isc_nmhandle_t *handle, isc_result_t result, void *arg) { - ns_server_t *sctx = (ns_server_t *)arg; + ns_interface_t *ifp = (ns_interface_t *)arg; + dns_aclenv_t *env = ns_interfacemgr_getaclenv(ifp->mgr); + ns_server_t *sctx = ns_interfacemgr_getserver(ifp->mgr); unsigned int tcpquota; + isc_sockaddr_t peeraddr; + isc_netaddr_t netaddr; + int match; - UNUSED(handle); UNUSED(result); + if (handle != NULL) { + peeraddr = isc_nmhandle_peeraddr(handle); + isc_netaddr_fromsockaddr(&netaddr, &peeraddr); + + if (sctx->blackholeacl != NULL && + (dns_acl_match(&netaddr, NULL, sctx->blackholeacl, env, + &match, NULL) == ISC_R_SUCCESS) && + match > 0) + { + return (ISC_R_CONNREFUSED); + } + } + tcpquota = isc_quota_getused(&sctx->tcpquota); ns_stats_update_if_greater(sctx->nsstats, ns_statscounter_tcphighwater, tcpquota); + + return (ISC_R_SUCCESS); } static void diff --git a/lib/ns/include/ns/client.h b/lib/ns/include/ns/client.h index bc6ec751e9..ee5dcef8bc 100644 --- a/lib/ns/include/ns/client.h +++ b/lib/ns/include/ns/client.h @@ -476,7 +476,7 @@ ns__client_request(isc_nmhandle_t *handle, isc_result_t eresult, * (Not intended for use outside this module and associated tests.) */ -void +isc_result_t ns__client_tcpconn(isc_nmhandle_t *handle, isc_result_t result, void *arg); /*%< diff --git a/lib/ns/include/ns/interfacemgr.h b/lib/ns/include/ns/interfacemgr.h index e60dc9d412..d55bd1b82f 100644 --- a/lib/ns/include/ns/interfacemgr.h +++ b/lib/ns/include/ns/interfacemgr.h @@ -197,6 +197,12 @@ ns_interfacemgr_dumprecursing(FILE *f, ns_interfacemgr_t *mgr); bool ns_interfacemgr_listeningon(ns_interfacemgr_t *mgr, const isc_sockaddr_t *addr); +ns_server_t * +ns_interfacemgr_getserver(ns_interfacemgr_t *mgr); +/*%< + * Returns the ns_server object associated with the interface manager. + */ + ns_interface_t * ns__interfacemgr_getif(ns_interfacemgr_t *mgr); ns_interface_t * diff --git a/lib/ns/interfacemgr.c b/lib/ns/interfacemgr.c index 30ecbcda29..b343970e3d 100644 --- a/lib/ns/interfacemgr.c +++ b/lib/ns/interfacemgr.c @@ -467,7 +467,7 @@ ns_interface_listentcp(ns_interface_t *ifp) { result = isc_nm_listentcpdns( ifp->mgr->nm, (isc_nmiface_t *)&ifp->addr, ns__client_request, - ifp, ns__client_tcpconn, ifp->mgr->sctx, sizeof(ns_client_t), + ifp, ns__client_tcpconn, ifp, sizeof(ns_client_t), ifp->mgr->backlog, &ifp->mgr->sctx->tcpquota, &ifp->tcplistensocket); if (result != ISC_R_SUCCESS) { @@ -481,7 +481,12 @@ ns_interface_listentcp(ns_interface_t *ifp) { * this is necessary because we are adding to the TCP quota just * by listening. */ - ns__client_tcpconn(NULL, ISC_R_SUCCESS, ifp->mgr->sctx); + result = ns__client_tcpconn(NULL, ISC_R_SUCCESS, ifp); + if (result != ISC_R_SUCCESS) { + isc_log_write(IFMGR_COMMON_LOGARGS, ISC_LOG_ERROR, + "connecting TCP socket: %s", + isc_result_totext(result)); + } #if 0 #ifndef ISC_ALLOW_MAPPED @@ -1267,6 +1272,13 @@ ns_interfacemgr_listeningon(ns_interfacemgr_t *mgr, return (result); } +ns_server_t * +ns_interfacemgr_getserver(ns_interfacemgr_t *mgr) { + REQUIRE(NS_INTERFACEMGR_VALID(mgr)); + + return (mgr->sctx); +} + ns_interface_t * ns__interfacemgr_getif(ns_interfacemgr_t *mgr) { ns_interface_t *head; diff --git a/lib/ns/win32/libns.def b/lib/ns/win32/libns.def index d2b9ad652f..1680e4982d 100644 --- a/lib/ns/win32/libns.def +++ b/lib/ns/win32/libns.def @@ -55,6 +55,7 @@ ns_interfacemgr_create ns_interfacemgr_detach ns_interfacemgr_dumprecursing ns_interfacemgr_getaclenv +ns_interfacemgr_getserver ns_interfacemgr_islistening ns_interfacemgr_listeningon ns_interfacemgr_scan