From 0580d9cd8c08472c29876daa368c770beaaad87a Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Sun, 15 Dec 2019 16:45:17 -0800 Subject: [PATCH 1/8] style cleanup clean up style in rndc and the control channel in preparation for changing them to use the new network manager. --- bin/named/controlconf.c | 104 ++++++++++++++++++---------------------- bin/rndc/rndc.c | 25 +++++----- lib/isccc/ccmsg.c | 45 ++++++----------- 3 files changed, 74 insertions(+), 100 deletions(-) diff --git a/bin/named/controlconf.c b/bin/named/controlconf.c index 6d6e4078d7..5e3fe4f36e 100644 --- a/bin/named/controlconf.c +++ b/bin/named/controlconf.c @@ -198,8 +198,8 @@ maybe_free_connection(controlconnection_t *conn) { static void shutdown_listener(controllistener_t *listener) { - controlconnection_t *conn; - controlconnection_t *next; + controlconnection_t *conn = NULL; + controlconnection_t *next = NULL; if (!listener->exiting) { char socktext[ISC_SOCKADDR_FORMATSIZE]; @@ -341,7 +341,7 @@ control_recvmessage(isc_task_t *task, isc_event_t *event) { isc_stdtime_t now; isc_buffer_t b; isc_region_t r; - isc_buffer_t *text; + isc_buffer_t *text = NULL; isc_result_t result; isc_result_t eresult; isccc_sexpr_t *_ctrl = NULL; @@ -573,7 +573,7 @@ control_timeout(isc_task_t *task, isc_event_t *event) { static isc_result_t newconnection(controllistener_t *listener, isc_socket_t *sock) { - controlconnection_t *conn; + controlconnection_t *conn = NULL; isc_interval_t interval; isc_result_t result; @@ -631,7 +631,7 @@ static void control_newconn(isc_task_t *task, isc_event_t *event) { isc_socket_newconnev_t *nevent = (isc_socket_newconnev_t *)event; controllistener_t *listener = event->ev_arg; - isc_socket_t *sock; + isc_socket_t *sock = NULL; isc_sockaddr_t peeraddr; isc_result_t result; @@ -693,8 +693,8 @@ cleanup: static void controls_shutdown(named_controls_t *controls) { - controllistener_t *listener; - controllistener_t *next; + controllistener_t *listener = NULL; + controllistener_t *next = NULL; for (listener = ISC_LIST_HEAD(controls->listeners); listener != NULL; listener = next) @@ -717,9 +717,9 @@ named_controls_shutdown(named_controls_t *controls) { static isc_result_t cfgkeylist_find(const cfg_obj_t *keylist, const char *keyname, const cfg_obj_t **objp) { - const cfg_listelt_t *element; - const char *str; - const cfg_obj_t *obj; + const cfg_listelt_t *element = NULL; + const char *str = NULL; + const cfg_obj_t *obj = NULL; for (element = cfg_list_first(keylist); element != NULL; element = cfg_list_next(element)) @@ -741,11 +741,11 @@ cfgkeylist_find(const cfg_obj_t *keylist, const char *keyname, static void controlkeylist_fromcfg(const cfg_obj_t *keylist, isc_mem_t *mctx, controlkeylist_t *keyids) { - const cfg_listelt_t *element; + const cfg_listelt_t *element = NULL; char *newstr = NULL; - const char *str; - const cfg_obj_t *obj; - controlkey_t *key; + const char *str = NULL; + const cfg_obj_t *obj = NULL; + controlkey_t *key = NULL; for (element = cfg_list_first(keylist); element != NULL; element = cfg_list_next(element)) @@ -767,8 +767,8 @@ controlkeylist_fromcfg(const cfg_obj_t *keylist, isc_mem_t *mctx, static void register_keys(const cfg_obj_t *control, const cfg_obj_t *keylist, controlkeylist_t *keyids, isc_mem_t *mctx, const char *socktext) { - controlkey_t *keyid, *next; - const cfg_obj_t *keydef; + controlkey_t *keyid = NULL, *next = NULL; + const cfg_obj_t *keydef = NULL; char secret[1024]; isc_buffer_t b; isc_result_t result; @@ -801,10 +801,9 @@ register_keys(const cfg_obj_t *control, const cfg_obj_t *keylist, algstr = cfg_obj_asstring(algobj); secretstr = cfg_obj_asstring(secretobj); - if (named_config_getkeyalgorithm2(algstr, NULL, - &algtype, NULL) != - ISC_R_SUCCESS) - { + result = named_config_getkeyalgorithm2(algstr, NULL, + &algtype, NULL); + if (result != ISC_R_SUCCESS) { cfg_obj_log(control, named_g_lctx, ISC_LOG_WARNING, "unsupported algorithm '%s' in " @@ -841,11 +840,12 @@ register_keys(const cfg_obj_t *control, const cfg_obj_t *keylist, } } -#define CHECK(x) \ - do { \ - result = (x); \ - if (result != ISC_R_SUCCESS) \ - goto cleanup; \ +#define CHECK(x) \ + do { \ + result = (x); \ + if (result != ISC_R_SUCCESS) { \ + goto cleanup; \ + } \ } while (0) static isc_result_t @@ -895,9 +895,8 @@ get_rndckey(isc_mem_t *mctx, controlkeylist_t *keyids) { algstr = cfg_obj_asstring(algobj); secretstr = cfg_obj_asstring(secretobj); - if (named_config_getkeyalgorithm2(algstr, NULL, &algtype, NULL) != - ISC_R_SUCCESS) - { + result = named_config_getkeyalgorithm2(algstr, NULL, &algtype, NULL); + if (result != ISC_R_SUCCESS) { cfg_obj_log(key, named_g_lctx, ISC_LOG_WARNING, "unsupported algorithm '%s' in " "key '%s' for use with command " @@ -970,8 +969,8 @@ update_listener(named_controls_t *cp, controllistener_t **listenerp, const cfg_obj_t *control, const cfg_obj_t *config, isc_sockaddr_t *addr, cfg_aclconfctx_t *aclconfctx, const char *socktext, isc_sockettype_t type) { - controllistener_t *listener; - const cfg_obj_t *allow; + controllistener_t *listener = NULL; + const cfg_obj_t *allow = NULL; const cfg_obj_t *global_keylist = NULL; const cfg_obj_t *control_keylist = NULL; dns_acl_t *new_acl = NULL; @@ -1064,7 +1063,7 @@ update_listener(named_controls_t *cp, controllistener_t **listenerp, } if (control != NULL) { - const cfg_obj_t *readonly; + const cfg_obj_t *readonly = NULL; readonly = cfg_tuple_get(control, "read-only"); if (!cfg_obj_isvoid(readonly)) { @@ -1123,35 +1122,25 @@ add_listener(named_controls_t *cp, controllistener_t **listenerp, isc_sockaddr_t *addr, cfg_aclconfctx_t *aclconfctx, const char *socktext, isc_sockettype_t type) { isc_mem_t *mctx = cp->server->mctx; - controllistener_t *listener; - const cfg_obj_t *allow; + controllistener_t *listener = NULL; + const cfg_obj_t *allow = NULL; const cfg_obj_t *global_keylist = NULL; const cfg_obj_t *control_keylist = NULL; dns_acl_t *new_acl = NULL; isc_result_t result = ISC_R_SUCCESS; listener = isc_mem_get(mctx, sizeof(*listener)); - - listener->mctx = NULL; + *listener = (controllistener_t){ .controls = cp, + .task = cp->server->task, + .address = *addr, + .type = type }; isc_mem_attach(mctx, &listener->mctx); - listener->controls = cp; - listener->task = cp->server->task; - listener->address = *addr; - listener->sock = NULL; - listener->listening = false; - listener->exiting = false; - listener->acl = NULL; - listener->type = type; - listener->perm = 0; - listener->owner = 0; - listener->group = 0; - listener->readonly = false; ISC_LINK_INIT(listener, link); ISC_LIST_INIT(listener->keys); ISC_LIST_INIT(listener->connections); /* - * Make the acl. + * Make the ACL. */ if (control != NULL && type == isc_sockettype_tcp) { allow = cfg_tuple_get(control, "allow"); @@ -1161,8 +1150,8 @@ add_listener(named_controls_t *cp, controllistener_t **listenerp, result = dns_acl_any(mctx, &new_acl); } - if ((result == ISC_R_SUCCESS) && (control != NULL)) { - const cfg_obj_t *readonly; + if (result == ISC_R_SUCCESS && control != NULL) { + const cfg_obj_t *readonly = NULL; readonly = cfg_tuple_get(control, "read-only"); if (!cfg_obj_isvoid(readonly)) { @@ -1242,6 +1231,7 @@ add_listener(named_controls_t *cp, controllistener_t **listenerp, result = isc_socket_permunix(&listener->address, listener->perm, listener->owner, listener->group); } + if (result == ISC_R_SUCCESS) { result = control_listen(listener); } @@ -1279,7 +1269,7 @@ add_listener(named_controls_t *cp, controllistener_t **listenerp, isc_result_t named_controls_configure(named_controls_t *cp, const cfg_obj_t *config, cfg_aclconfctx_t *aclconfctx) { - controllistener_t *listener; + controllistener_t *listener = NULL; controllistenerlist_t new_listeners; const cfg_obj_t *controlslist = NULL; const cfg_listelt_t *element, *element2; @@ -1304,7 +1294,7 @@ named_controls_configure(named_controls_t *cp, const cfg_obj_t *config, for (element = cfg_list_first(controlslist); element != NULL; element = cfg_list_next(element)) { - const cfg_obj_t *controls; + const cfg_obj_t *controls = NULL; const cfg_obj_t *inetcontrols = NULL; controls = cfg_listelt_value(element); @@ -1317,8 +1307,8 @@ named_controls_configure(named_controls_t *cp, const cfg_obj_t *config, element2 != NULL; element2 = cfg_list_next(element2)) { - const cfg_obj_t *control; - const cfg_obj_t *obj; + const cfg_obj_t *control = NULL; + const cfg_obj_t *obj = NULL; isc_sockaddr_t addr; /* @@ -1375,7 +1365,7 @@ named_controls_configure(named_controls_t *cp, const cfg_obj_t *config, for (element = cfg_list_first(controlslist); element != NULL; element = cfg_list_next(element)) { - const cfg_obj_t *controls; + const cfg_obj_t *controls = NULL; const cfg_obj_t *unixcontrols = NULL; controls = cfg_listelt_value(element); @@ -1388,8 +1378,8 @@ named_controls_configure(named_controls_t *cp, const cfg_obj_t *config, element2 != NULL; element2 = cfg_list_next(element2)) { - const cfg_obj_t *control; - const cfg_obj_t *path; + const cfg_obj_t *control = NULL; + const cfg_obj_t *path = NULL; isc_sockaddr_t addr; isc_result_t result; diff --git a/bin/rndc/rndc.c b/bin/rndc/rndc.c index 7a3123e19a..58e9a43d8e 100644 --- a/bin/rndc/rndc.c +++ b/bin/rndc/rndc.c @@ -55,11 +55,11 @@ #define SERVERADDRS 10 -const char *progname; +const char *progname = NULL; bool verbose; -static const char *admin_conffile; -static const char *admin_keyfile; +static const char *admin_conffile = NULL; +static const char *admin_keyfile = NULL; static const char *version = PACKAGE_VERSION; static const char *servername = NULL; static isc_sockaddr_t serveraddrs[SERVERADDRS]; @@ -69,18 +69,18 @@ static int nserveraddrs; static int currentaddr = 0; static unsigned int remoteport = 0; static isc_socketmgr_t *socketmgr = NULL; -static isc_buffer_t *databuf; +static isc_buffer_t *databuf = NULL; static isccc_ccmsg_t ccmsg; static uint32_t algorithm; static isccc_region_t secret; static bool failed = false; static bool c_flag = false; -static isc_mem_t *rndc_mctx; +static isc_mem_t *rndc_mctx = NULL; static atomic_uint_fast32_t sends = ATOMIC_VAR_INIT(0); static atomic_uint_fast32_t recvs = ATOMIC_VAR_INIT(0); static atomic_uint_fast32_t connects = ATOMIC_VAR_INIT(0); -static char *command; -static char *args; +static char *command = NULL; +static char *args = NULL; static char program[256]; static isc_socket_t *sock = NULL; static uint32_t serial; @@ -114,7 +114,7 @@ command is one of the following:\n\ Close, rename and re-open the DNSTAP output file(s).\n\ dumpdb [-all|-cache|-zones|-adb|-bad|-fail] [view ...]\n\ Dump cache(s) to the dump file (named_dump.db).\n\ - flush Flushes all of the server's caches.\n\ + flush Flushes all of the server's caches.\n\ flush [view] Flushes the server's cache for a view.\n\ flushname name [view]\n\ Flush the given name from the server's cache(s)\n\ @@ -652,7 +652,7 @@ parse_config(isc_mem_t *mctx, isc_log_t *log, const char *keyname, if (servers != NULL) { for (elt = cfg_list_first(servers); elt != NULL; elt = cfg_list_next(elt)) { - const char *name; + const char *name = NULL; server = cfg_listelt_value(elt); name = cfg_obj_asstring( cfg_map_getname(server)); @@ -689,9 +689,11 @@ parse_config(isc_mem_t *mctx, isc_log_t *log, const char *keyname, DO("get config key list", cfg_map_get(config, "key", &keys)); for (elt = cfg_list_first(keys); elt != NULL; elt = cfg_list_next(elt)) { + const char *name = NULL; + key = cfg_listelt_value(elt); - if (strcasecmp(cfg_obj_asstring(cfg_map_getname(key)), - keyname) == 0) { + name = cfg_obj_asstring(cfg_map_getname(key)); + if (strcasecmp(name, keyname) == 0) { break; } } @@ -990,7 +992,6 @@ main(int argc, char **argv) { DO("create task manager", isc_taskmgr_create(rndc_mctx, 1, 0, NULL, &taskmgr)); DO("create task", isc_task_create(taskmgr, 0, &task)); - isc_log_create(rndc_mctx, &log, &logconfig); isc_log_setcontext(log); isc_log_settag(logconfig, progname); diff --git a/lib/isccc/ccmsg.c b/lib/isccc/ccmsg.c index 07a2d403ff..b3ce556efb 100644 --- a/lib/isccc/ccmsg.c +++ b/lib/isccc/ccmsg.c @@ -46,7 +46,7 @@ recv_message(isc_task_t *, isc_event_t *); static void recv_length(isc_task_t *task, isc_event_t *ev_in) { isc_socketevent_t *ev = (isc_socketevent_t *)ev_in; - isc_event_t *dev; + isc_event_t *dev = NULL; isccc_ccmsg_t *ccmsg = ev_in->ev_arg; isc_region_t region; isc_result_t result; @@ -101,10 +101,10 @@ send_and_free: static void recv_message(isc_task_t *task, isc_event_t *ev_in) { isc_socketevent_t *ev = (isc_socketevent_t *)ev_in; - isc_event_t *dev; + isc_event_t *dev = NULL; isccc_ccmsg_t *ccmsg = ev_in->ev_arg; - (void)task; + UNUSED(task); INSIST(VALID_CCMSG(ccmsg)); @@ -131,19 +131,17 @@ isccc_ccmsg_init(isc_mem_t *mctx, isc_socket_t *sock, isccc_ccmsg_t *ccmsg) { REQUIRE(sock != NULL); REQUIRE(ccmsg != NULL); - ccmsg->magic = CCMSG_MAGIC; - ccmsg->size = 0; - ccmsg->buffer.base = NULL; - ccmsg->buffer.length = 0; - ccmsg->maxsize = 4294967295U; /* Largest message possible. */ - ccmsg->mctx = mctx; - ccmsg->sock = sock; - ccmsg->task = NULL; /* None yet. */ - ccmsg->result = ISC_R_UNEXPECTED; /* None yet. */ - /* - * Should probably initialize the - *event here, but it can wait. - */ + *ccmsg = (isccc_ccmsg_t){ + .magic = CCMSG_MAGIC, + .maxsize = 0xffffffffU, /* Largest message possible. */ + .mctx = mctx, + .sock = sock, + .result = ISC_R_UNEXPECTED /* None yet. */ + }; + + /* + * Should probably initialize the event here, but it can wait. + */ } void @@ -197,21 +195,6 @@ isccc_ccmsg_cancelread(isccc_ccmsg_t *ccmsg) { isc_socket_cancel(ccmsg->sock, NULL, ISC_SOCKCANCEL_RECV); } -#if 0 -void -isccc_ccmsg_freebuffer(isccc_ccmsg_t*ccmsg) { - REQUIRE(VALID_CCMSG(ccmsg)); - - if (ccmsg->buffer.base == NULL) { - return; - } - - isc_mem_put(ccmsg->mctx,ccmsg->buffer.base,ccmsg->buffer.length); - ccmsg->buffer.base = NULL; - ccmsg->buffer.length = 0; -} -#endif /* if 0 */ - void isccc_ccmsg_invalidate(isccc_ccmsg_t *ccmsg) { REQUIRE(VALID_CCMSG(ccmsg)); From 002c328437e7dbc59bbbc23d5bfea5bd6150bdc9 Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Wed, 15 Apr 2020 14:37:47 -0700 Subject: [PATCH 2/8] don't use exclusive mode for rndc commands that don't need it "showzone" and "tsig-list" both used exclusive mode unnecessarily; changing this will simplify future refactoring a bit. --- bin/named/server.c | 15 +-------------- 1 file changed, 1 insertion(+), 14 deletions(-) diff --git a/bin/named/server.c b/bin/named/server.c index 5df22c8476..0d3111c024 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -12086,12 +12086,10 @@ cleanup: isc_result_t named_server_tsiglist(named_server_t *server, isc_buffer_t **text) { - isc_result_t result; + isc_result_t result = ISC_R_SUCCESS; dns_view_t *view; unsigned int foundkeys = 0; - result = isc_task_beginexclusive(server->task); - RUNTIME_CHECK(result == ISC_R_SUCCESS); for (view = ISC_LIST_HEAD(server->viewlist); view != NULL; view = ISC_LIST_NEXT(view, link)) { @@ -12100,7 +12098,6 @@ named_server_tsiglist(named_server_t *server, isc_buffer_t **text) { &foundkeys); RWUNLOCK(&view->statickeys->lock, isc_rwlocktype_read); if (result != ISC_R_SUCCESS) { - isc_task_endexclusive(server->task); return (result); } RWLOCK(&view->dynamickeys->lock, isc_rwlocktype_read); @@ -12108,11 +12105,9 @@ named_server_tsiglist(named_server_t *server, isc_buffer_t **text) { &foundkeys); RWUNLOCK(&view->dynamickeys->lock, isc_rwlocktype_read); if (result != ISC_R_SUCCESS) { - isc_task_endexclusive(server->task); return (result); } } - isc_task_endexclusive(server->task); if (foundkeys == 0) { CHECK(putstr(text, "no tsig keys found.")); @@ -14161,7 +14156,6 @@ named_server_showzone(named_server_t *server, isc_lex_t *lex, dns_view_t *view = NULL; dns_zone_t *zone = NULL; ns_cfgctx_t *cfg = NULL; - bool exclusive = false; #ifdef HAVE_LMDB cfg_obj_t *nzconfig = NULL; #endif /* HAVE_LMDB */ @@ -14186,10 +14180,6 @@ named_server_showzone(named_server_t *server, isc_lex_t *lex, goto cleanup; } - result = isc_task_beginexclusive(server->task); - RUNTIME_CHECK(result == ISC_R_SUCCESS); - exclusive = true; - if (!added) { /* Find the view statement */ vconfig = find_name_in_list_from_map(cfg->config, "view", @@ -14248,9 +14238,6 @@ cleanup: if (isc_buffer_usedlength(*text) > 0) { (void)putnull(text); } - if (exclusive) { - isc_task_endexclusive(server->task); - } return (result); } From 3551d3ffd2afe6542e79a5571b326bd77fdd265e Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Thu, 16 Apr 2020 13:06:42 -0700 Subject: [PATCH 3/8] convert rndc and control channel to use netmgr - updated libisccc to use netmgr events - updated rndc to use isc_nm_tcpconnect() to establish connections - updated control channel to use isc_nm_listentcp() open issues: - the control channel timeout was previously 60 seconds, but it is now overridden by the TCP idle timeout setting, which defaults to 30 seconds. we should add a function that sets the timeout value for a specific listener socket, instead of always using the global value set in the netmgr. (for the moment, since 30 seconds is a reasonable timeout for the control channel, I'm not prioritizing this.) - the netmgr currently has no support for UNIX-domain sockets; until this is addressed, it will not be possible to configure rndc to use them. we will need to either fix this or document the change in behavior. --- bin/named/controlconf.c | 486 ++++++++++++-------------------- bin/rndc/rndc.c | 230 +++++++-------- doc/man/dnssec-importkey.1in | 2 +- lib/isccc/ccmsg.c | 195 +++++++------ lib/isccc/include/isccc/ccmsg.h | 39 ++- 5 files changed, 430 insertions(+), 522 deletions(-) diff --git a/bin/named/controlconf.c b/bin/named/controlconf.c index 5e3fe4f36e..37a19b57b6 100644 --- a/bin/named/controlconf.c +++ b/bin/named/controlconf.c @@ -21,12 +21,12 @@ #include #include #include +#include #include #include #include #include #include -#include #include #include @@ -49,12 +49,6 @@ #include #include -/* - * Note: Listeners and connections are not locked. All event handlers are - * executed by the server task, and all callers of exported routines must - * be running under the server task. - */ - typedef struct controlkey controlkey_t; typedef ISC_LIST(controlkey_t) controlkeylist_t; @@ -72,11 +66,10 @@ struct controlkey { }; struct controlconnection { - isc_socket_t *sock; + isc_nmhandle_t *handle; isccc_ccmsg_t ccmsg; bool ccmsg_valid; bool sending; - isc_timer_t *timer; isc_buffer_t *buffer; controllistener_t *listener; uint32_t nonce; @@ -86,15 +79,14 @@ struct controlconnection { struct controllistener { named_controls_t *controls; isc_mem_t *mctx; - isc_task_t *task; isc_sockaddr_t address; - isc_socket_t *sock; + isc_nmsocket_t *sock; dns_acl_t *acl; bool listening; bool exiting; controlkeylist_t keys; controlconnectionlist_t connections; - isc_sockettype_t type; + isc_socktype_t type; uint32_t perm; uint32_t owner; uint32_t group; @@ -109,10 +101,10 @@ struct named_controls { isccc_symtab_t *symtab; }; +static isc_result_t +control_newconn(isc_nmhandle_t *handle, isc_result_t result, void *arg); static void -control_newconn(isc_task_t *task, isc_event_t *event); -static void -control_recvmessage(isc_task_t *task, isc_event_t *event); +control_recvmessage(isc_nmhandle_t *handle, isc_result_t result, void *arg); #define CLOCKSKEW 300 @@ -143,7 +135,7 @@ free_listener(controllistener_t *listener) { INSIST(ISC_LIST_EMPTY(listener->connections)); if (listener->sock != NULL) { - isc_socket_detach(&listener->sock); + isc_nmsocket_close(&listener->sock); } free_controlkeylist(&listener->keys, listener->mctx); @@ -164,43 +156,8 @@ maybe_free_listener(controllistener_t *listener) { } } -static void -maybe_free_connection(controlconnection_t *conn) { - controllistener_t *listener = conn->listener; - - if (conn->buffer != NULL) { - isc_buffer_free(&conn->buffer); - } - - if (conn->timer != NULL) { - isc_timer_detach(&conn->timer); - } - - if (conn->ccmsg_valid) { - isccc_ccmsg_cancelread(&conn->ccmsg); - return; - } - - if (conn->sending) { - isc_socket_cancel(conn->sock, listener->task, - ISC_SOCKCANCEL_SEND); - return; - } - - ISC_LIST_UNLINK(listener->connections, conn, link); -#ifdef ENABLE_AFL - if (named_g_fuzz_type == isc_fuzz_rndc) { - named_fuzz_notify(); - } -#endif /* ifdef ENABLE_AFL */ - isc_mem_put(listener->mctx, conn, sizeof(*conn)); -} - static void shutdown_listener(controllistener_t *listener) { - controlconnection_t *conn = NULL; - controlconnection_t *next = NULL; - if (!listener->exiting) { char socktext[ISC_SOCKADDR_FORMATSIZE]; @@ -211,117 +168,77 @@ shutdown_listener(controllistener_t *listener) { isc_log_write(named_g_lctx, NAMED_LOGCATEGORY_GENERAL, NAMED_LOGMODULE_CONTROL, ISC_LOG_NOTICE, "stopping command channel on %s", socktext); - if (listener->type == isc_sockettype_unix) { +#if 0 + /* XXX: no unix domain socket support */ + if (listener->type == isc_socktype_unix) { isc_socket_cleanunix(&listener->address, true); } +#endif listener->exiting = true; + listener->listening = false; } - for (conn = ISC_LIST_HEAD(listener->connections); conn != NULL; - conn = next) { - next = ISC_LIST_NEXT(conn, link); - maybe_free_connection(conn); - } - - if (listener->listening) { - isc_socket_cancel(listener->sock, listener->task, - ISC_SOCKCANCEL_ACCEPT); - return; - } - + isc_nm_stoplistening(listener->sock); maybe_free_listener(listener); } static bool -address_ok(isc_sockaddr_t *sockaddr, dns_acl_t *acl) { +address_ok(isc_sockaddr_t *sockaddr, controllistener_t *listener) { dns_aclenv_t *env = ns_interfacemgr_getaclenv(named_g_server->interfacemgr); isc_netaddr_t netaddr; isc_result_t result; int match; + /* ACL doesn't apply to unix domain sockets */ + if (listener->type != isc_socktype_tcp) { + return (true); + } + isc_netaddr_fromsockaddr(&netaddr, sockaddr); - result = dns_acl_match(&netaddr, NULL, acl, env, &match, NULL); + result = dns_acl_match(&netaddr, NULL, listener->acl, env, &match, + NULL); return (result == ISC_R_SUCCESS && match > 0); } -static isc_result_t -control_accept(controllistener_t *listener) { - isc_result_t result; - result = isc_socket_accept(listener->sock, listener->task, - control_newconn, listener); - if (result != ISC_R_SUCCESS) { - UNEXPECTED_ERROR(__FILE__, __LINE__, - "isc_socket_accept() failed: %s", - isc_result_totext(result)); - } else { - listener->listening = true; - } - return (result); -} - -static isc_result_t -control_listen(controllistener_t *listener) { - isc_result_t result; - - result = isc_socket_listen(listener->sock, 0); - if (result != ISC_R_SUCCESS) { - UNEXPECTED_ERROR(__FILE__, __LINE__, - "isc_socket_listen() failed: %s", - isc_result_totext(result)); - } - return (result); -} - static void -control_next(controllistener_t *listener) { - (void)control_accept(listener); -} - -static void -control_senddone(isc_task_t *task, isc_event_t *event) { - isc_socketevent_t *sevent = (isc_socketevent_t *)event; - controlconnection_t *conn = event->ev_arg; +control_senddone(isc_nmhandle_t *handle, isc_result_t result, void *arg) { + controlconnection_t *conn = (controlconnection_t *)arg; controllistener_t *listener = conn->listener; - isc_socket_t *sock = (isc_socket_t *)sevent->ev_sender; - isc_result_t result; REQUIRE(conn->sending); - UNUSED(task); - conn->sending = false; - if (sevent->result != ISC_R_SUCCESS && sevent->result != ISC_R_CANCELED) - { + if (result == ISC_R_CANCELED) { + return; + } else if (result != ISC_R_SUCCESS) { char socktext[ISC_SOCKADDR_FORMATSIZE]; - isc_sockaddr_t peeraddr; + isc_sockaddr_t peeraddr = isc_nmhandle_peeraddr(handle); - (void)isc_socket_getpeername(sock, &peeraddr); isc_sockaddr_format(&peeraddr, socktext, sizeof(socktext)); isc_log_write(named_g_lctx, NAMED_LOGCATEGORY_GENERAL, NAMED_LOGMODULE_CONTROL, ISC_LOG_WARNING, "error sending command response to %s: %s", - socktext, isc_result_totext(sevent->result)); + socktext, isc_result_totext(result)); + return; } - isc_event_free(&event); - result = isccc_ccmsg_readmessage(&conn->ccmsg, listener->task, - control_recvmessage, conn); + result = isccc_ccmsg_readmessage(&conn->ccmsg, control_recvmessage, + conn); if (result != ISC_R_SUCCESS) { - isc_socket_detach(&conn->sock); - maybe_free_connection(conn); maybe_free_listener(listener); } + + listener->listening = true; } static inline void log_invalid(isccc_ccmsg_t *ccmsg, isc_result_t result) { char socktext[ISC_SOCKADDR_FORMATSIZE]; - isc_sockaddr_t peeraddr; + isc_sockaddr_t peeraddr = isc_nmhandle_peeraddr(ccmsg->handle); - (void)isc_socket_getpeername(ccmsg->sock, &peeraddr); isc_sockaddr_format(&peeraddr, socktext, sizeof(socktext)); isc_log_write(named_g_lctx, NAMED_LOGCATEGORY_GENERAL, NAMED_LOGMODULE_CONTROL, ISC_LOG_ERROR, @@ -330,8 +247,8 @@ log_invalid(isccc_ccmsg_t *ccmsg, isc_result_t result) { } static void -control_recvmessage(isc_task_t *task, isc_event_t *event) { - controlconnection_t *conn = NULL; +control_recvmessage(isc_nmhandle_t *handle, isc_result_t result, void *arg) { + controlconnection_t *conn = (controlconnection_t *)arg; controllistener_t *listener = NULL; controlkey_t *key = NULL; isccc_sexpr_t *request = NULL; @@ -342,7 +259,6 @@ control_recvmessage(isc_task_t *task, isc_event_t *event) { isc_buffer_t b; isc_region_t r; isc_buffer_t *text = NULL; - isc_result_t result; isc_result_t eresult; isccc_sexpr_t *_ctrl = NULL; isccc_time_t sent; @@ -350,9 +266,6 @@ control_recvmessage(isc_task_t *task, isc_event_t *event) { uint32_t nonce; isccc_sexpr_t *data = NULL; - REQUIRE(event->ev_type == ISCCC_EVENT_CCMSG); - - conn = event->ev_arg; listener = conn->listener; algorithm = DST_ALG_UNKNOWN; secret.rstart = NULL; @@ -363,23 +276,21 @@ control_recvmessage(isc_task_t *task, isc_event_t *event) { goto cleanup; } - if (conn->ccmsg.result != ISC_R_SUCCESS) { - if (conn->ccmsg.result != ISC_R_CANCELED && - conn->ccmsg.result != ISC_R_EOF) { - log_invalid(&conn->ccmsg, conn->ccmsg.result); + if (result != ISC_R_SUCCESS) { + if (result != ISC_R_CANCELED && result != ISC_R_EOF) { + log_invalid(&conn->ccmsg, result); } + goto cleanup; } - request = NULL; - for (key = ISC_LIST_HEAD(listener->keys); key != NULL; key = ISC_LIST_NEXT(key, link)) { isccc_region_t ccregion; - ccregion.rstart = isc_buffer_base(&conn->ccmsg.buffer); - ccregion.rend = isc_buffer_used(&conn->ccmsg.buffer); + ccregion.rstart = isc_buffer_base(conn->ccmsg.buffer); + ccregion.rend = isc_buffer_used(conn->ccmsg.buffer); secret.rstart = isc_mem_get(listener->mctx, key->secret.length); memmove(secret.rstart, key->secret.base, key->secret.length); secret.rend = secret.rstart + key->secret.length; @@ -529,7 +440,7 @@ control_recvmessage(isc_task_t *task, isc_event_t *event) { r.base = conn->buffer->base; r.length = conn->buffer->used; - result = isc_socket_send(conn->sock, &r, task, control_senddone, conn); + result = isc_nm_send(handle, &r, control_senddone, conn); if (result != ISC_R_SUCCESS) { goto cleanup_response; } @@ -552,57 +463,78 @@ cleanup_request: } cleanup: - isc_socket_detach(&conn->sock); - isccc_ccmsg_invalidate(&conn->ccmsg); conn->ccmsg_valid = false; - maybe_free_connection(conn); - maybe_free_listener(listener); } static void -control_timeout(isc_task_t *task, isc_event_t *event) { - controlconnection_t *conn = event->ev_arg; +conn_reset(void *arg) { + controlconnection_t *conn = (controlconnection_t *)arg; + controllistener_t *listener = conn->listener; - UNUSED(task); + if (conn->buffer != NULL) { + isc_buffer_free(&conn->buffer); + } - isc_timer_detach(&conn->timer); - maybe_free_connection(conn); + if (conn->ccmsg_valid) { + isccc_ccmsg_cancelread(&conn->ccmsg); + return; + } - isc_event_free(&event); + ISC_LIST_UNLINK(listener->connections, conn, link); +#ifdef ENABLE_AFL + if (named_g_fuzz_type == isc_fuzz_rndc) { + named_fuzz_notify(); + } +#endif /* ifdef ENABLE_AFL */ + + isccc_ccmsg_invalidate(&conn->ccmsg); +} + +static void +conn_put(void *arg) { + controlconnection_t *conn = (controlconnection_t *)arg; + controllistener_t *listener = conn->listener; + + isc_log_write(named_g_lctx, NAMED_LOGCATEGORY_GENERAL, + NAMED_LOGMODULE_CONTROL, ISC_LOG_DEBUG(3), + "freeing control connection"); + + maybe_free_listener(listener); } static isc_result_t -newconnection(controllistener_t *listener, isc_socket_t *sock) { +newconnection(controllistener_t *listener, isc_nmhandle_t *handle) { controlconnection_t *conn = NULL; - isc_interval_t interval; isc_result_t result; - conn = isc_mem_get(listener->mctx, sizeof(*conn)); + conn = isc_nmhandle_getdata(handle); + if (conn == NULL) { + conn = isc_nmhandle_getextra(handle); + isc_log_write(named_g_lctx, NAMED_LOGCATEGORY_GENERAL, + NAMED_LOGMODULE_CONTROL, ISC_LOG_DEBUG(3), + "allocate new control connection"); + *conn = (controlconnection_t){ .handle = NULL }; + } - conn->sock = sock; - isccc_ccmsg_init(listener->mctx, sock, &conn->ccmsg); + if (conn->handle == NULL) { + isc_nmhandle_setdata(handle, conn, conn_reset, conn_put); + } else { + INSIST(conn->handle == handle); + } + + *conn = (controlconnection_t){ .handle = handle, + .listener = listener, + .ccmsg_valid = true }; + + isccc_ccmsg_init(listener->mctx, handle, &conn->ccmsg); /* Set a 32 KiB upper limit on incoming message. */ isccc_ccmsg_setmaxsize(&conn->ccmsg, 32768); - conn->ccmsg_valid = true; - conn->sending = false; - conn->buffer = NULL; - conn->timer = NULL; - isc_interval_set(&interval, 60, 0); - result = isc_timer_create(named_g_timermgr, isc_timertype_once, NULL, - &interval, listener->task, control_timeout, - conn, &conn->timer); - if (result != ISC_R_SUCCESS) { - goto cleanup; - } - - conn->listener = listener; - conn->nonce = 0; ISC_LINK_INIT(conn, link); - result = isccc_ccmsg_readmessage(&conn->ccmsg, listener->task, - control_recvmessage, conn); + result = isccc_ccmsg_readmessage(&conn->ccmsg, control_recvmessage, + conn); if (result != ISC_R_SUCCESS) { goto cleanup; } @@ -614,10 +546,9 @@ cleanup: if (conn->buffer != NULL) { isc_buffer_free(&conn->buffer); } + isccc_ccmsg_invalidate(&conn->ccmsg); - if (conn->timer != NULL) { - isc_timer_detach(&conn->timer); - } + isc_mem_put(listener->mctx, conn, sizeof(*conn)); #ifdef ENABLE_AFL if (named_g_fuzz_type == isc_fuzz_rndc) { @@ -627,53 +558,32 @@ cleanup: return (result); } -static void -control_newconn(isc_task_t *task, isc_event_t *event) { - isc_socket_newconnev_t *nevent = (isc_socket_newconnev_t *)event; - controllistener_t *listener = event->ev_arg; - isc_socket_t *sock = NULL; +static isc_result_t +control_newconn(isc_nmhandle_t *handle, isc_result_t result, void *arg) { + controllistener_t *listener = arg; isc_sockaddr_t peeraddr; - isc_result_t result; - - UNUSED(task); - - REQUIRE(listener->listening); listener->listening = false; - if (nevent->result != ISC_R_SUCCESS) { - if (nevent->result == ISC_R_CANCELED) { + if (result != ISC_R_SUCCESS) { + if (result == ISC_R_CANCELED) { shutdown_listener(listener); - goto cleanup; } - goto restart; + return (result); } - sock = nevent->newsocket; - - /* Is the server shutting down? */ - if (listener->controls->shuttingdown) { - isc_socket_detach(&sock); - shutdown_listener(listener); - goto cleanup; - } - - isc_socket_setname(sock, "control", NULL); - (void)isc_socket_getpeername(sock, &peeraddr); - if (listener->type == isc_sockettype_tcp && - !address_ok(&peeraddr, listener->acl)) - { + peeraddr = isc_nmhandle_peeraddr(handle); + if (!address_ok(&peeraddr, listener)) { char socktext[ISC_SOCKADDR_FORMATSIZE]; isc_sockaddr_format(&peeraddr, socktext, sizeof(socktext)); isc_log_write(named_g_lctx, NAMED_LOGCATEGORY_GENERAL, NAMED_LOGMODULE_CONTROL, ISC_LOG_WARNING, "rejected command channel message from %s", socktext); - isc_socket_detach(&sock); - goto restart; + return (ISC_R_FAILURE); } - result = newconnection(listener, sock); + result = newconnection(listener, handle); if (result != ISC_R_SUCCESS) { char socktext[ISC_SOCKADDR_FORMATSIZE]; isc_sockaddr_format(&peeraddr, socktext, sizeof(socktext)); @@ -681,14 +591,10 @@ control_newconn(isc_task_t *task, isc_event_t *event) { NAMED_LOGMODULE_CONTROL, ISC_LOG_WARNING, "dropped command channel from %s: %s", socktext, isc_result_totext(result)); - isc_socket_detach(&sock); - goto restart; + return (result); } -restart: - control_next(listener); -cleanup: - isc_event_free(&event); + return (ISC_R_SUCCESS); } static void @@ -968,7 +874,7 @@ static void update_listener(named_controls_t *cp, controllistener_t **listenerp, const cfg_obj_t *control, const cfg_obj_t *config, isc_sockaddr_t *addr, cfg_aclconfctx_t *aclconfctx, - const char *socktext, isc_sockettype_t type) { + const char *socktext, isc_socktype_t type) { controllistener_t *listener = NULL; const cfg_obj_t *allow = NULL; const cfg_obj_t *global_keylist = NULL; @@ -1053,7 +959,7 @@ update_listener(named_controls_t *cp, controllistener_t **listenerp, /* * Now, keep the old access list unless a new one can be made. */ - if (control != NULL && type == isc_sockettype_tcp) { + if (control != NULL && type == isc_socktype_tcp) { allow = cfg_tuple_get(control, "allow"); result = cfg_acl_fromconfig(allow, config, named_g_lctx, aclconfctx, listener->mctx, 0, @@ -1089,7 +995,7 @@ update_listener(named_controls_t *cp, controllistener_t **listenerp, socktext, isc_result_totext(result)); } - if (result == ISC_R_SUCCESS && type == isc_sockettype_unix) { + if (result == ISC_R_SUCCESS && type == isc_socktype_unix) { uint32_t perm, owner, group; perm = cfg_obj_asuint32(cfg_tuple_get(control, "perm")); owner = cfg_obj_asuint32(cfg_tuple_get(control, "owner")); @@ -1120,7 +1026,7 @@ static void add_listener(named_controls_t *cp, controllistener_t **listenerp, const cfg_obj_t *control, const cfg_obj_t *config, isc_sockaddr_t *addr, cfg_aclconfctx_t *aclconfctx, - const char *socktext, isc_sockettype_t type) { + const char *socktext, isc_socktype_t type) { isc_mem_t *mctx = cp->server->mctx; controllistener_t *listener = NULL; const cfg_obj_t *allow = NULL; @@ -1128,10 +1034,10 @@ add_listener(named_controls_t *cp, controllistener_t **listenerp, const cfg_obj_t *control_keylist = NULL; dns_acl_t *new_acl = NULL; isc_result_t result = ISC_R_SUCCESS; + int pf; listener = isc_mem_get(mctx, sizeof(*listener)); *listener = (controllistener_t){ .controls = cp, - .task = cp->server->task, .address = *addr, .type = type }; isc_mem_attach(mctx, &listener->mctx); @@ -1142,41 +1048,36 @@ add_listener(named_controls_t *cp, controllistener_t **listenerp, /* * Make the ACL. */ - if (control != NULL && type == isc_sockettype_tcp) { - allow = cfg_tuple_get(control, "allow"); - result = cfg_acl_fromconfig(allow, config, named_g_lctx, - aclconfctx, mctx, 0, &new_acl); - } else { - result = dns_acl_any(mctx, &new_acl); - } - - if (result == ISC_R_SUCCESS && control != NULL) { + if (control != NULL && type == isc_socktype_tcp) { const cfg_obj_t *readonly = NULL; + allow = cfg_tuple_get(control, "allow"); + CHECK(cfg_acl_fromconfig(allow, config, named_g_lctx, + aclconfctx, mctx, 0, &new_acl)); + readonly = cfg_tuple_get(control, "read-only"); if (!cfg_obj_isvoid(readonly)) { listener->readonly = cfg_obj_asboolean(readonly); } + } else { + CHECK(dns_acl_any(mctx, &new_acl)); } - if (result == ISC_R_SUCCESS) { - dns_acl_attach(new_acl, &listener->acl); - dns_acl_detach(&new_acl); + dns_acl_attach(new_acl, &listener->acl); + dns_acl_detach(&new_acl); - if (config != NULL) { - get_key_info(config, control, &global_keylist, - &control_keylist); - } - - if (control_keylist != NULL) { - controlkeylist_fromcfg(control_keylist, listener->mctx, - &listener->keys); - register_keys(control, global_keylist, &listener->keys, - listener->mctx, socktext); - } else { - result = get_rndckey(mctx, &listener->keys); - } + if (config != NULL) { + get_key_info(config, control, &global_keylist, + &control_keylist); + } + if (control_keylist != NULL) { + controlkeylist_fromcfg(control_keylist, listener->mctx, + &listener->keys); + register_keys(control, global_keylist, &listener->keys, + listener->mctx, socktext); + } else { + result = get_rndckey(mctx, &listener->keys); if (result != ISC_R_SUCCESS && control != NULL) { cfg_obj_log(control, named_g_lctx, ISC_LOG_WARNING, "couldn't install keys for " @@ -1185,43 +1086,31 @@ add_listener(named_controls_t *cp, controllistener_t **listenerp, } } - if (result == ISC_R_SUCCESS) { - int pf = isc_sockaddr_pf(&listener->address); - if ((pf == AF_INET && isc_net_probeipv4() != ISC_R_SUCCESS) || + pf = isc_sockaddr_pf(&listener->address); + if ((pf == AF_INET && isc_net_probeipv4() != ISC_R_SUCCESS) || #ifndef _WIN32 - (pf == AF_UNIX && isc_net_probeunix() != ISC_R_SUCCESS) || -#endif - (pf == AF_INET6 && isc_net_probeipv6() != ISC_R_SUCCESS)) - { - result = ISC_R_FAMILYNOSUPPORT; - } + (pf == AF_UNIX && isc_net_probeunix() != ISC_R_SUCCESS) || +#endif /* ifdef _WIN32 */ + (pf == AF_INET6 && isc_net_probeipv6() != ISC_R_SUCCESS)) + { + CHECK(ISC_R_FAMILYNOSUPPORT); } - if (result == ISC_R_SUCCESS && type == isc_sockettype_unix) { +#if 0 + /* XXX: no unix socket support yet */ + if (type == isc_socktype_unix) { isc_socket_cleanunix(&listener->address, false); } +#endif - if (result == ISC_R_SUCCESS) { - result = isc_socket_create(named_g_socketmgr, - isc_sockaddr_pf(&listener->address), - type, &listener->sock); - } - if (result == ISC_R_SUCCESS) { - isc_socket_setname(listener->sock, "control", NULL); - } - -#ifndef ISC_ALLOW_MAPPED - if (result == ISC_R_SUCCESS) { - isc_socket_ipv6only(listener->sock, true); - } -#endif /* ifndef ISC_ALLOW_MAPPED */ - - if (result == ISC_R_SUCCESS) { - result = isc_socket_bind(listener->sock, &listener->address, - ISC_SOCKET_REUSEADDRESS); - } - - if (result == ISC_R_SUCCESS && type == isc_sockettype_unix) { + CHECK(isc_nm_listentcp(named_g_nm, (isc_nmiface_t *)&listener->address, + control_newconn, listener, + sizeof(controlconnection_t), 5, NULL, + &listener->sock)); + listener->listening = true; +#if 0 + /* XXX: no unix socket support yet */ + if (type == isc_socktype_unix) { listener->perm = cfg_obj_asuint32(cfg_tuple_get(control, "perm")); listener->owner = @@ -1231,38 +1120,33 @@ add_listener(named_controls_t *cp, controllistener_t **listenerp, result = isc_socket_permunix(&listener->address, listener->perm, listener->owner, listener->group); } +#endif - if (result == ISC_R_SUCCESS) { - result = control_listen(listener); - } + isc_log_write(named_g_lctx, NAMED_LOGCATEGORY_GENERAL, + NAMED_LOGMODULE_CONTROL, ISC_LOG_NOTICE, + "command channel listening on %s", socktext); + *listenerp = listener; + return; - if (result == ISC_R_SUCCESS) { - result = control_accept(listener); - } - - if (result == ISC_R_SUCCESS) { - isc_log_write(named_g_lctx, NAMED_LOGCATEGORY_GENERAL, - NAMED_LOGMODULE_CONTROL, ISC_LOG_NOTICE, - "command channel listening on %s", socktext); - *listenerp = listener; - } else { +cleanup: + if (listener != NULL) { listener->exiting = true; free_listener(listener); - - if (control != NULL) { - cfg_obj_log(control, named_g_lctx, ISC_LOG_WARNING, - "couldn't add command channel %s: %s", - socktext, isc_result_totext(result)); - } else { - isc_log_write(named_g_lctx, NAMED_LOGCATEGORY_GENERAL, - NAMED_LOGMODULE_CONTROL, ISC_LOG_NOTICE, - "couldn't add command channel %s: %s", - socktext, isc_result_totext(result)); - } - - *listenerp = NULL; } + if (control != NULL) { + cfg_obj_log(control, named_g_lctx, ISC_LOG_WARNING, + "couldn't add command channel %s: %s", socktext, + isc_result_totext(result)); + } else { + isc_log_write(named_g_lctx, NAMED_LOGCATEGORY_GENERAL, + NAMED_LOGMODULE_CONTROL, ISC_LOG_NOTICE, + "couldn't add command channel %s: %s", socktext, + isc_result_totext(result)); + } + + *listenerp = NULL; + /* XXXDCL return error results? fail hard? */ } @@ -1337,7 +1221,7 @@ named_controls_configure(named_controls_t *cp, const cfg_obj_t *config, update_listener(cp, &listener, control, config, &addr, aclconfctx, socktext, - isc_sockettype_tcp); + isc_socktype_tcp); if (listener != NULL) { /* @@ -1353,7 +1237,7 @@ named_controls_configure(named_controls_t *cp, const cfg_obj_t *config, add_listener(cp, &listener, control, config, &addr, aclconfctx, socktext, - isc_sockettype_tcp); + isc_socktype_tcp); } if (listener != NULL) { @@ -1374,6 +1258,12 @@ named_controls_configure(named_controls_t *cp, const cfg_obj_t *config, continue; } + cfg_obj_log(controls, named_g_lctx, ISC_LOG_ERROR, + "UNIX domain sockets not yet supported"); + return (ISC_R_FAILURE); + +#if 0 + /* XXX: no unix domain socket support in netmgr */ for (element2 = cfg_list_first(unixcontrols); element2 != NULL; element2 = cfg_list_next(element2)) @@ -1415,7 +1305,7 @@ named_controls_configure(named_controls_t *cp, const cfg_obj_t *config, update_listener(cp, &listener, control, config, &addr, aclconfctx, cfg_obj_asstring(path), - isc_sockettype_unix); + isc_socktype_unix); if (listener != NULL) { /* @@ -1431,7 +1321,7 @@ named_controls_configure(named_controls_t *cp, const cfg_obj_t *config, add_listener(cp, &listener, control, config, &addr, aclconfctx, cfg_obj_asstring(path), - isc_sockettype_unix); + isc_socktype_unix); } if (listener != NULL) { @@ -1439,6 +1329,7 @@ named_controls_configure(named_controls_t *cp, const cfg_obj_t *config, link); } } +#endif } } else { int i; @@ -1466,7 +1357,7 @@ named_controls_configure(named_controls_t *cp, const cfg_obj_t *config, isc_sockaddr_format(&addr, socktext, sizeof(socktext)); update_listener(cp, &listener, NULL, NULL, &addr, NULL, - socktext, isc_sockettype_tcp); + socktext, isc_socktype_tcp); if (listener != NULL) { /* @@ -1479,8 +1370,7 @@ named_controls_configure(named_controls_t *cp, const cfg_obj_t *config, * This is a new listener. */ add_listener(cp, &listener, NULL, NULL, &addr, - NULL, socktext, - isc_sockettype_tcp); + NULL, socktext, isc_socktype_tcp); } if (listener != NULL) { diff --git a/bin/rndc/rndc.c b/bin/rndc/rndc.c index 58e9a43d8e..7753f7f463 100644 --- a/bin/rndc/rndc.c +++ b/bin/rndc/rndc.c @@ -58,6 +58,9 @@ const char *progname = NULL; bool verbose; +static isc_taskmgr_t *taskmgr = NULL; +static isc_task_t *rndc_task = NULL; + static const char *admin_conffile = NULL; static const char *admin_keyfile = NULL; static const char *version = PACKAGE_VERSION; @@ -68,9 +71,9 @@ static bool local4set = false, local6set = false; static int nserveraddrs; static int currentaddr = 0; static unsigned int remoteport = 0; -static isc_socketmgr_t *socketmgr = NULL; +static isc_nm_t *netmgr = NULL; static isc_buffer_t *databuf = NULL; -static isccc_ccmsg_t ccmsg; +static isccc_ccmsg_t rndc_ccmsg; static uint32_t algorithm; static isccc_region_t secret; static bool failed = false; @@ -82,13 +85,13 @@ static atomic_uint_fast32_t connects = ATOMIC_VAR_INIT(0); static char *command = NULL; static char *args = NULL; static char program[256]; -static isc_socket_t *sock = NULL; static uint32_t serial; static bool quiet = false; static bool showresult = false; +static bool shuttingdown = false; static void -rndc_startconnect(isc_sockaddr_t *addr, isc_task_t *task); +rndc_startconnect(isc_sockaddr_t *addr); ISC_NORETURN static void usage(int status); @@ -278,51 +281,54 @@ get_addresses(const char *host, in_port_t port) { } static void -rndc_senddone(isc_task_t *task, isc_event_t *event) { - isc_socketevent_t *sevent = (isc_socketevent_t *)event; +rndc_senddone(isc_nmhandle_t *handle, isc_result_t result, void *arg) { + UNUSED(arg); - UNUSED(task); - - if (sevent->result != ISC_R_SUCCESS) { - fatal("send failed: %s", isc_result_totext(sevent->result)); + if (result != ISC_R_SUCCESS) { + fatal("send failed: %s", isc_result_totext(result)); } - isc_event_free(&event); + if (atomic_fetch_sub_release(&sends, 1) == 1 && atomic_load_acquire(&recvs) == 0) { - isc_socket_detach(&sock); - isc_task_shutdown(task); + shuttingdown = true; + isc_task_shutdown(rndc_task); isc_app_shutdown(); + isc_nmhandle_unref(handle); } } static void -rndc_recvdone(isc_task_t *task, isc_event_t *event) { +rndc_recvdone(isc_nmhandle_t *handle, isc_result_t result, void *arg) { + isccc_ccmsg_t *ccmsg = (isccc_ccmsg_t *)arg; isccc_sexpr_t *response = NULL; isccc_sexpr_t *data; isccc_region_t source; char *errormsg = NULL; char *textmsg = NULL; - isc_result_t result; + + REQUIRE(ccmsg != NULL); atomic_fetch_sub_release(&recvs, 1); - if (ccmsg.result == ISC_R_EOF) { - fatal("connection to remote host closed\n" - "This may indicate that\n" - "* the remote server is using an older version of" - " the command protocol,\n" + if (shuttingdown && (result == ISC_R_EOF || result == ISC_R_CANCELED)) { + isc_nmhandle_unref(handle); + return; + } else if (result == ISC_R_EOF) { + fatal("connection to remote host closed.\n" + "* This may indicate that the\n" + "* remote server is using an older\n" + "* version of the command protocol,\n" "* this host is not authorized to connect,\n" - "* the clocks are not synchronized, or\n" - "* the key is invalid."); + "* the clocks are not synchronized,\n" + "* the key signing algorithm is incorrect,\n" + "* or the key is invalid."); + } else if (result != ISC_R_SUCCESS && result != ISC_R_CANCELED) { + fatal("recv failed: %s", isc_result_totext(result)); } - if (ccmsg.result != ISC_R_SUCCESS) { - fatal("recv failed: %s", isc_result_totext(ccmsg.result)); - } - - source.rstart = isc_buffer_base(&ccmsg.buffer); - source.rend = isc_buffer_used(&ccmsg.buffer); + source.rstart = isc_buffer_base(ccmsg->buffer); + source.rend = isc_buffer_used(ccmsg->buffer); DO("parse message", isccc_cc_fromwire(&source, &response, algorithm, &secret)); @@ -362,22 +368,23 @@ rndc_recvdone(isc_task_t *task, isc_event_t *event) { } } - isc_event_free(&event); isccc_sexpr_free(&response); + if (atomic_load_acquire(&sends) == 0 && atomic_load_acquire(&recvs) == 0) { - isc_socket_detach(&sock); - isc_task_shutdown(task); + shuttingdown = true; + isc_task_shutdown(rndc_task); isc_app_shutdown(); + isc_nmhandle_unref(handle); } } static void -rndc_recvnonce(isc_task_t *task, isc_event_t *event) { +rndc_recvnonce(isc_nmhandle_t *handle, isc_result_t result, void *arg) { + isccc_ccmsg_t *ccmsg = (isccc_ccmsg_t *)arg; isccc_sexpr_t *response = NULL; isccc_sexpr_t *_ctrl; isccc_region_t source; - isc_result_t result; uint32_t nonce; isccc_sexpr_t *request = NULL; isccc_time_t now; @@ -385,25 +392,28 @@ rndc_recvnonce(isc_task_t *task, isc_event_t *event) { isccc_sexpr_t *data; isc_buffer_t b; + REQUIRE(ccmsg != NULL); + atomic_fetch_sub_release(&recvs, 1); - if (ccmsg.result == ISC_R_EOF) { - fatal("connection to remote host closed\n" - "This may indicate that\n" - "* the remote server is using an older version of" - " the command protocol,\n" + if (shuttingdown && result == ISC_R_EOF) { + isc_nmhandle_unref(handle); + return; + } else if (result == ISC_R_EOF) { + fatal("connection to remote host closed.\n" + "* This may indicate that the\n" + "* remote server is using an older\n" + "* version of the command protocol,\n" "* this host is not authorized to connect,\n" "* the clocks are not synchronized,\n" - "* the key signing algorithm is incorrect, or\n" - "* the key is invalid."); + "* the key signing algorithm is incorrect\n" + "* or the key is invalid."); + } else if (result != ISC_R_SUCCESS) { + fatal("recv failed: %s", isc_result_totext(result)); } - if (ccmsg.result != ISC_R_SUCCESS) { - fatal("recv failed: %s", isc_result_totext(ccmsg.result)); - } - - source.rstart = isc_buffer_base(&ccmsg.buffer); - source.rend = isc_buffer_used(&ccmsg.buffer); + source.rstart = isc_buffer_base(ccmsg->buffer); + source.rend = isc_buffer_used(ccmsg->buffer); DO("parse message", isccc_cc_fromwire(&source, &response, algorithm, &secret)); @@ -451,48 +461,44 @@ rndc_recvnonce(isc_task_t *task, isc_event_t *event) { r.base = databuf->base; r.length = databuf->used; - isccc_ccmsg_cancelread(&ccmsg); DO("schedule recv", - isccc_ccmsg_readmessage(&ccmsg, task, rndc_recvdone, NULL)); + isccc_ccmsg_readmessage(ccmsg, rndc_recvdone, ccmsg)); atomic_fetch_add_relaxed(&recvs, 1); - DO("send message", - isc_socket_send(sock, &r, task, rndc_senddone, NULL)); + + DO("send message", isc_nm_send(handle, &r, rndc_senddone, NULL)); atomic_fetch_add_relaxed(&sends, 1); - isc_event_free(&event); isccc_sexpr_free(&response); isccc_sexpr_free(&request); return; } static void -rndc_connected(isc_task_t *task, isc_event_t *event) { +rndc_connected(isc_nmhandle_t *handle, isc_result_t result, void *arg) { + isccc_ccmsg_t *ccmsg = (isccc_ccmsg_t *)arg; char socktext[ISC_SOCKADDR_FORMATSIZE]; - isc_socketevent_t *sevent = (isc_socketevent_t *)event; isccc_sexpr_t *request = NULL; isccc_sexpr_t *data; isccc_time_t now; isc_region_t r; isc_buffer_t b; - isc_result_t result; + + REQUIRE(ccmsg != NULL); atomic_fetch_sub_release(&connects, 1); - if (sevent->result != ISC_R_SUCCESS) { + if (result != ISC_R_SUCCESS) { isc_sockaddr_format(&serveraddrs[currentaddr], socktext, sizeof(socktext)); - if (sevent->result != ISC_R_CANCELED && - ++currentaddr < nserveraddrs) { + if (++currentaddr < nserveraddrs) { notify("connection failed: %s: %s", socktext, - isc_result_totext(sevent->result)); - isc_socket_detach(&sock); - isc_event_free(&event); - rndc_startconnect(&serveraddrs[currentaddr], task); + isc_result_totext(result)); + rndc_startconnect(&serveraddrs[currentaddr]); return; - } else { - fatal("connect failed: %s: %s", socktext, - isc_result_totext(sevent->result)); } + + fatal("connect failed: %s: %s", socktext, + isc_result_totext(result)); } isc_stdtime_get(&now); @@ -519,50 +525,52 @@ rndc_connected(isc_task_t *task, isc_event_t *event) { r.base = databuf->base; r.length = databuf->used; - isccc_ccmsg_init(rndc_mctx, sock, &ccmsg); - isccc_ccmsg_setmaxsize(&ccmsg, 1024 * 1024); + isccc_ccmsg_init(rndc_mctx, handle, ccmsg); + isccc_ccmsg_setmaxsize(ccmsg, 1024 * 1024); + + isc_nmhandle_ref(handle); DO("schedule recv", - isccc_ccmsg_readmessage(&ccmsg, task, rndc_recvnonce, NULL)); + isccc_ccmsg_readmessage(ccmsg, rndc_recvnonce, ccmsg)); atomic_fetch_add_relaxed(&recvs, 1); - DO("send message", - isc_socket_send(sock, &r, task, rndc_senddone, NULL)); + + DO("send message", isc_nm_send(handle, &r, rndc_senddone, NULL)); atomic_fetch_add_relaxed(&sends, 1); - isc_event_free(&event); + isccc_sexpr_free(&request); } static void -rndc_startconnect(isc_sockaddr_t *addr, isc_task_t *task) { +rndc_startconnect(isc_sockaddr_t *addr) { isc_result_t result; - int pf; - isc_sockettype_t type; - char socktext[ISC_SOCKADDR_FORMATSIZE]; + isc_sockaddr_t *local = NULL; isc_sockaddr_format(addr, socktext, sizeof(socktext)); notify("using server %s (%s)", servername, socktext); - pf = isc_sockaddr_pf(addr); - if (pf == AF_INET || pf == AF_INET6) { - type = isc_sockettype_tcp; - } else { - type = isc_sockettype_unix; - } - DO("create socket", isc_socket_create(socketmgr, pf, type, &sock)); switch (isc_sockaddr_pf(addr)) { case AF_INET: - DO("bind socket", isc_socket_bind(sock, &local4, 0)); + local = &local4; break; case AF_INET6: - DO("bind socket", isc_socket_bind(sock, &local6, 0)); + local = &local6; break; + case AF_UNIX: + /* + * TODO: support UNIX domain sockets in netgmr. + */ + fatal("UNIX domain sockets not currently supported"); default: - break; + INSIST(0); + ISC_UNREACHABLE(); } - DO("connect", - isc_socket_connect(sock, addr, task, rndc_connected, NULL)); + + DO("create connection", + isc_nm_tcpconnect(netmgr, (isc_nmiface_t *)local, + (isc_nmiface_t *)addr, rndc_connected, &rndc_ccmsg, + 0)); atomic_fetch_add_relaxed(&connects, 1); } @@ -570,8 +578,10 @@ static void rndc_start(isc_task_t *task, isc_event_t *event) { isc_event_free(&event); + UNUSED(task); + currentaddr = 0; - rndc_startconnect(&serveraddrs[currentaddr], task); + rndc_startconnect(&serveraddrs[currentaddr]); } static void @@ -853,8 +863,6 @@ int main(int argc, char **argv) { isc_result_t result = ISC_R_SUCCESS; bool show_final_mem = false; - isc_taskmgr_t *taskmgr = NULL; - isc_task_t *task = NULL; isc_log_t *log = NULL; isc_logconfig_t *logconfig = NULL; isc_logdestination_t logdest; @@ -980,18 +988,23 @@ main(int argc, char **argv) { argc -= isc_commandline_index; argv += isc_commandline_index; - if (argc < 1) { + if (argv[0] == NULL) { usage(1); + } else { + command = argv[0]; + if (strcmp(command, "restart") == 0) { + fatal("'%s' is not implemented", command); + } + notify("%s", command); } serial = isc_random32(); isc_mem_create(&rndc_mctx); - DO("create socket manager", - isc_socketmgr_create(rndc_mctx, &socketmgr)); + netmgr = isc_nm_start(rndc_mctx, 1); DO("create task manager", isc_taskmgr_create(rndc_mctx, 1, 0, NULL, &taskmgr)); - DO("create task", isc_task_create(taskmgr, 0, &task)); + DO("create task", isc_task_create(taskmgr, 0, &rndc_task)); isc_log_create(rndc_mctx, &log, &logconfig); isc_log_setcontext(log); isc_log_settag(logconfig, progname); @@ -1009,8 +1022,6 @@ main(int argc, char **argv) { isccc_result_register(); - command = *argv; - isc_buffer_allocate(rndc_mctx, &databuf, 2048); /* @@ -1037,32 +1048,30 @@ main(int argc, char **argv) { *p++ = '\0'; INSIST(p == args + argslen); - notify("%s", command); - - if (strcmp(command, "restart") == 0) { - fatal("'%s' is not implemented", command); - } - if (nserveraddrs == 0 && servername != NULL) { get_addresses(servername, (in_port_t)remoteport); } - DO("post event", isc_app_onrun(rndc_mctx, task, rndc_start, NULL)); + DO("post event", isc_app_onrun(rndc_mctx, rndc_task, rndc_start, NULL)); result = isc_app_run(); if (result != ISC_R_SUCCESS) { fatal("isc_app_run() failed: %s", isc_result_totext(result)); } - if (atomic_load_acquire(&connects) > 0 || - atomic_load_acquire(&sends) > 0 || atomic_load_acquire(&recvs) > 0) - { - isc_socket_cancel(sock, task, ISC_SOCKCANCEL_ALL); - } - - isc_task_detach(&task); + isc_task_detach(&rndc_task); isc_taskmgr_destroy(&taskmgr); - isc_socketmgr_destroy(&socketmgr); + + isc_nm_destroy(&netmgr); + + /* + * Note: when TCP connections are shut down, there will be a final + * call to the isccc callback routine with &rndc_ccmsg as its + * argument. We therefore need to delay invalidating it until + * after the netmgr is destroyed. + */ + isccc_ccmsg_invalidate(&rndc_ccmsg); + isc_log_destroy(&log); isc_log_setcontext(NULL); @@ -1070,7 +1079,6 @@ main(int argc, char **argv) { cfg_parser_destroy(&pctx); isc_mem_put(rndc_mctx, args, argslen); - isccc_ccmsg_invalidate(&ccmsg); isc_buffer_free(&databuf); diff --git a/doc/man/dnssec-importkey.1in b/doc/man/dnssec-importkey.1in index 619e5edade..174c25c2a3 100644 --- a/doc/man/dnssec-importkey.1in +++ b/doc/man/dnssec-importkey.1in @@ -39,7 +39,7 @@ level margin: \\n[rst2man-indent\\n[rst2man-indent-level]] .sp \fBdnssec\-importkey\fP reads a public DNSKEY record and generates a pair of .key/.private files. The DNSKEY record may be read from an existing -.key file, in which case a corresponding .private file is +\&.key file, in which case a corresponding .private file is generated, or it may be read from any other file or from the standard input, in which case both .key and .private files are generated. .sp diff --git a/lib/isccc/ccmsg.c b/lib/isccc/ccmsg.c index b3ce556efb..5b79e6148d 100644 --- a/lib/isccc/ccmsg.c +++ b/lib/isccc/ccmsg.c @@ -28,8 +28,9 @@ #include #include +#include #include -#include +#include #include #include @@ -39,109 +40,140 @@ #define VALID_CCMSG(foo) ISC_MAGIC_VALID(foo, CCMSG_MAGIC) static void -recv_length(isc_task_t *, isc_event_t *); -static void -recv_message(isc_task_t *, isc_event_t *); +recv_message(isc_nmhandle_t *handle, isc_result_t eresult, isc_region_t *region, + void *arg); static void -recv_length(isc_task_t *task, isc_event_t *ev_in) { - isc_socketevent_t *ev = (isc_socketevent_t *)ev_in; - isc_event_t *dev = NULL; - isccc_ccmsg_t *ccmsg = ev_in->ev_arg; - isc_region_t region; +recv_nonce(isc_nmhandle_t *handle, isc_result_t eresult, isc_region_t *region, + void *arg) { + isccc_ccmsg_t *ccmsg = arg; isc_result_t result; + if (eresult == ISC_R_CANCELED || eresult == ISC_R_EOF) { + ccmsg->result = eresult; + goto done; + } + INSIST(VALID_CCMSG(ccmsg)); - dev = &ccmsg->event; - - if (ev->result != ISC_R_SUCCESS) { - ccmsg->result = ev->result; - goto send_and_free; + if (region == NULL && eresult == ISC_R_SUCCESS) { + ccmsg->result = ISC_R_EOF; + goto done; + } else if (eresult != ISC_R_SUCCESS) { + ccmsg->result = eresult; + goto done; + } else { + ccmsg->result = eresult; } - /* - * Success. - */ - ccmsg->size = ntohl(ccmsg->size); + if (region->length < sizeof(uint32_t)) { + ccmsg->result = ISC_R_UNEXPECTEDEND; + goto done; + } + + ccmsg->size = ntohl(*(uint32_t *)region->base); if (ccmsg->size == 0) { ccmsg->result = ISC_R_UNEXPECTEDEND; - goto send_and_free; + goto done; } if (ccmsg->size > ccmsg->maxsize) { ccmsg->result = ISC_R_RANGE; - goto send_and_free; + goto done; } - region.base = isc_mem_get(ccmsg->mctx, ccmsg->size); - region.length = ccmsg->size; - if (region.base == NULL) { - ccmsg->result = ISC_R_NOMEMORY; - goto send_and_free; + isc_region_consume(region, sizeof(uint32_t)); + isc_buffer_allocate(ccmsg->mctx, &ccmsg->buffer, ccmsg->size); + + /* + * If there's more of the message waiting, pass it to + * recv_message() directly. + */ + if (region->length != 0) { + recv_message(handle, ISC_R_SUCCESS, region, ccmsg); + return; } - isc_buffer_init(&ccmsg->buffer, region.base, region.length); - result = isc_socket_recv(ccmsg->sock, ®ion, 0, task, recv_message, - ccmsg); - if (result != ISC_R_SUCCESS) { - ccmsg->result = result; - goto send_and_free; + /* + * Otherwise, continue reading and handle it in + * recv_message(). + */ + result = isc_nm_read(handle, recv_message, ccmsg); + if (result == ISC_R_SUCCESS) { + return; } - isc_event_free(&ev_in); - return; + ccmsg->result = result; -send_and_free: - isc_task_send(ccmsg->task, &dev); - ccmsg->task = NULL; - isc_event_free(&ev_in); - return; +done: + ccmsg->cb(handle, ccmsg->result, ccmsg->cbarg); + isc_nmhandle_unref(handle); } static void -recv_message(isc_task_t *task, isc_event_t *ev_in) { - isc_socketevent_t *ev = (isc_socketevent_t *)ev_in; - isc_event_t *dev = NULL; - isccc_ccmsg_t *ccmsg = ev_in->ev_arg; +recv_message(isc_nmhandle_t *handle, isc_result_t eresult, isc_region_t *region, + void *arg) { + isc_result_t result; + isccc_ccmsg_t *ccmsg = arg; + size_t size; - UNUSED(task); + if (eresult == ISC_R_CANCELED || eresult == ISC_R_EOF) { + ccmsg->result = eresult; + goto done; + } INSIST(VALID_CCMSG(ccmsg)); - dev = &ccmsg->event; - - if (ev->result != ISC_R_SUCCESS) { - ccmsg->result = ev->result; - goto send_and_free; + if (region == NULL && eresult == ISC_R_SUCCESS) { + ccmsg->result = ISC_R_EOF; + goto done; + } else if (eresult != ISC_R_SUCCESS) { + ccmsg->result = eresult; + goto done; + } else { + ccmsg->result = eresult; } - ccmsg->result = ISC_R_SUCCESS; - isc_buffer_add(&ccmsg->buffer, ev->n); - ccmsg->address = ev->address; + if (region->length == 0) { + ccmsg->result = ISC_R_UNEXPECTEDEND; + goto done; + } -send_and_free: - isc_task_send(ccmsg->task, &dev); - ccmsg->task = NULL; - isc_event_free(&ev_in); + size = ISC_MIN(isc_buffer_availablelength(ccmsg->buffer), + region->length); + isc_buffer_putmem(ccmsg->buffer, region->base, size); + isc_region_consume(region, size); + + if (isc_buffer_usedlength(ccmsg->buffer) == ccmsg->size) { + ccmsg->result = ISC_R_SUCCESS; + goto done; + } + + result = isc_nm_read(handle, recv_message, ccmsg); + if (result == ISC_R_SUCCESS) { + return; + } + + ccmsg->result = result; + +done: + ccmsg->cb(handle, ccmsg->result, ccmsg->cbarg); + isc_nmhandle_unref(handle); } void -isccc_ccmsg_init(isc_mem_t *mctx, isc_socket_t *sock, isccc_ccmsg_t *ccmsg) { +isccc_ccmsg_init(isc_mem_t *mctx, isc_nmhandle_t *handle, + isccc_ccmsg_t *ccmsg) { REQUIRE(mctx != NULL); - REQUIRE(sock != NULL); + REQUIRE(handle != NULL); REQUIRE(ccmsg != NULL); *ccmsg = (isccc_ccmsg_t){ .magic = CCMSG_MAGIC, .maxsize = 0xffffffffU, /* Largest message possible. */ .mctx = mctx, - .sock = sock, + .handle = handle, .result = ISC_R_UNEXPECTED /* None yet. */ }; - - /* - * Should probably initialize the event here, but it can wait. - */ } void @@ -152,37 +184,23 @@ isccc_ccmsg_setmaxsize(isccc_ccmsg_t *ccmsg, unsigned int maxsize) { } isc_result_t -isccc_ccmsg_readmessage(isccc_ccmsg_t *ccmsg, isc_task_t *task, - isc_taskaction_t action, void *arg) { +isccc_ccmsg_readmessage(isccc_ccmsg_t *ccmsg, isc_nm_cb_t cb, void *cbarg) { isc_result_t result; - isc_region_t region; REQUIRE(VALID_CCMSG(ccmsg)); - REQUIRE(task != NULL); - REQUIRE(ccmsg->task == NULL); /* not currently in use */ - if (ccmsg->buffer.base != NULL) { - isc_mem_put(ccmsg->mctx, ccmsg->buffer.base, - ccmsg->buffer.length); - ccmsg->buffer.base = NULL; - ccmsg->buffer.length = 0; + if (ccmsg->buffer != NULL) { + isc_buffer_free(&ccmsg->buffer); } - ccmsg->task = task; - ccmsg->action = action; - ccmsg->arg = arg; + ccmsg->cb = cb; + ccmsg->cbarg = cbarg; ccmsg->result = ISC_R_UNEXPECTED; /* unknown right now */ - ISC_EVENT_INIT(&ccmsg->event, sizeof(isc_event_t), 0, 0, - ISCCC_EVENT_CCMSG, action, arg, ccmsg, NULL, NULL); - - region.base = (unsigned char *)&ccmsg->size; - region.length = 4; /* uint32_t */ - result = isc_socket_recv(ccmsg->sock, ®ion, 0, ccmsg->task, - recv_length, ccmsg); - + isc_nmhandle_ref(ccmsg->handle); + result = isc_nm_read(ccmsg->handle, recv_nonce, ccmsg); if (result != ISC_R_SUCCESS) { - ccmsg->task = NULL; + isc_nmhandle_unref(ccmsg->handle); } return (result); @@ -192,7 +210,7 @@ void isccc_ccmsg_cancelread(isccc_ccmsg_t *ccmsg) { REQUIRE(VALID_CCMSG(ccmsg)); - isc_socket_cancel(ccmsg->sock, NULL, ISC_SOCKCANCEL_RECV); + isc_nm_cancelread(ccmsg->handle); } void @@ -201,10 +219,7 @@ isccc_ccmsg_invalidate(isccc_ccmsg_t *ccmsg) { ccmsg->magic = 0; - if (ccmsg->buffer.base != NULL) { - isc_mem_put(ccmsg->mctx, ccmsg->buffer.base, - ccmsg->buffer.length); - ccmsg->buffer.base = NULL; - ccmsg->buffer.length = 0; + if (ccmsg->buffer != NULL) { + isc_buffer_free(&ccmsg->buffer); } } diff --git a/lib/isccc/include/isccc/ccmsg.h b/lib/isccc/include/isccc/ccmsg.h index 082abb2eef..3451007bf9 100644 --- a/lib/isccc/include/isccc/ccmsg.h +++ b/lib/isccc/include/isccc/ccmsg.h @@ -32,39 +32,37 @@ #include #include -#include +#include +#include /*% ISCCC Message Structure */ typedef struct isccc_ccmsg { /* private (don't touch!) */ - unsigned int magic; - uint32_t size; - isc_buffer_t buffer; - unsigned int maxsize; - isc_mem_t * mctx; - isc_socket_t * sock; - isc_task_t * task; - isc_taskaction_t action; - void * arg; - isc_event_t event; + unsigned int magic; + uint32_t size; + isc_buffer_t * buffer; + unsigned int maxsize; + isc_mem_t * mctx; + isc_nmhandle_t *handle; + isc_nm_cb_t cb; + void * cbarg; /* public (read-only) */ - isc_result_t result; - isc_sockaddr_t address; + isc_result_t result; } isccc_ccmsg_t; ISC_LANG_BEGINDECLS void -isccc_ccmsg_init(isc_mem_t *mctx, isc_socket_t *sock, isccc_ccmsg_t *ccmsg); +isccc_ccmsg_init(isc_mem_t *mctx, isc_nmhandle_t *handle, isccc_ccmsg_t *ccmsg); /*% * Associate a cc message state with a given memory context and - * TCP socket. + * netmgr handle. * * Requires: * - *\li "mctx" and "sock" be non-NULL and valid types. + *\li "mctx" be a valid memory context. * - *\li "sock" be a read/write TCP socket. + *\li "handle" be a netmgr handle for a stream socket. * *\li "ccmsg" be non-NULL and an uninitialized or invalidated structure. * @@ -86,8 +84,7 @@ isccc_ccmsg_setmaxsize(isccc_ccmsg_t *ccmsg, unsigned int maxsize); */ isc_result_t -isccc_ccmsg_readmessage(isccc_ccmsg_t *ccmsg, isc_task_t *task, - isc_taskaction_t action, void *arg); +isccc_ccmsg_readmessage(isccc_ccmsg_t *ccmsg, isc_nm_cb_t cb, void *cbarg); /*% * Schedule an event to be delivered when a command channel message is * readable, or when an error occurs on the socket. @@ -96,12 +93,10 @@ isccc_ccmsg_readmessage(isccc_ccmsg_t *ccmsg, isc_task_t *task, * *\li "ccmsg" be valid. * - *\li "task", "taskaction", and "arg" be valid. - * * Returns: * *\li #ISC_R_SUCCESS -- no error - *\li Anything that the isc_socket_recv() call can return. XXXMLG + *\li Anything that the isc_nm_read() call can return. * * Notes: * From 45ab0603eb9e76f78d173b687fdf3cb8ab598913 Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Thu, 14 May 2020 14:03:37 -0700 Subject: [PATCH 4/8] use an isc_task to execute rndc commands - using an isc_task to execute all rndc functions makes it relatively simple for them to acquire task exclusive mode when needed - control_recvmessage() has been separated into two functions, control_recvmessage() and control_respond(). the respond function can be called immediately from control_recvmessage() when processing a nonce, or it can be called after returning from the task event that ran the rndc command function. --- bin/named/controlconf.c | 398 ++++++++++++++++++------------- bin/named/include/named/server.h | 3 +- bin/named/server.c | 27 +-- bin/rndc/rndc.c | 2 +- lib/isccc/ccmsg.c | 8 +- 5 files changed, 240 insertions(+), 198 deletions(-) diff --git a/bin/named/controlconf.c b/bin/named/controlconf.c index 37a19b57b6..924b8a74f6 100644 --- a/bin/named/controlconf.c +++ b/bin/named/controlconf.c @@ -27,6 +27,7 @@ #include #include #include +#include #include #include @@ -70,9 +71,17 @@ struct controlconnection { isccc_ccmsg_t ccmsg; bool ccmsg_valid; bool sending; - isc_buffer_t *buffer; controllistener_t *listener; + isccc_sexpr_t *ctrl; + isc_buffer_t *buffer; + isc_buffer_t *text; + isccc_sexpr_t *request; + isccc_sexpr_t *response; + uint32_t alg; + isccc_region_t secret; uint32_t nonce; + isc_stdtime_t now; + isc_result_t result; ISC_LINK(controlconnection_t) link; }; @@ -206,16 +215,18 @@ static void control_senddone(isc_nmhandle_t *handle, isc_result_t result, void *arg) { controlconnection_t *conn = (controlconnection_t *)arg; controllistener_t *listener = conn->listener; + isc_sockaddr_t peeraddr = isc_nmhandle_peeraddr(handle); REQUIRE(conn->sending); conn->sending = false; + isc_nmhandle_unref(handle); + if (result == ISC_R_CANCELED) { return; } else if (result != ISC_R_SUCCESS) { char socktext[ISC_SOCKADDR_FORMATSIZE]; - isc_sockaddr_t peeraddr = isc_nmhandle_peeraddr(handle); isc_sockaddr_format(&peeraddr, socktext, sizeof(socktext)); isc_log_write(named_g_lctx, NAMED_LOGCATEGORY_GENERAL, @@ -247,178 +258,50 @@ log_invalid(isccc_ccmsg_t *ccmsg, isc_result_t result) { } static void -control_recvmessage(isc_nmhandle_t *handle, isc_result_t result, void *arg) { +control_respond(isc_nmhandle_t *handle, isc_result_t result, void *arg) { controlconnection_t *conn = (controlconnection_t *)arg; - controllistener_t *listener = NULL; - controlkey_t *key = NULL; - isccc_sexpr_t *request = NULL; - isccc_sexpr_t *response = NULL; - uint32_t algorithm; - isccc_region_t secret; - isc_stdtime_t now; + controllistener_t *listener = conn->listener; + isccc_sexpr_t *data = NULL; isc_buffer_t b; isc_region_t r; - isc_buffer_t *text = NULL; - isc_result_t eresult; - isccc_sexpr_t *_ctrl = NULL; - isccc_time_t sent; - isccc_time_t exp; - uint32_t nonce; - isccc_sexpr_t *data = NULL; - - listener = conn->listener; - algorithm = DST_ALG_UNKNOWN; - secret.rstart = NULL; - text = NULL; - - /* Is the server shutting down? */ - if (listener->controls->shuttingdown) { - goto cleanup; - } + result = isccc_cc_createresponse(conn->request, conn->now, + conn->now + 60, &conn->response); if (result != ISC_R_SUCCESS) { - if (result != ISC_R_CANCELED && result != ISC_R_EOF) { - log_invalid(&conn->ccmsg, result); - } - goto cleanup; } - for (key = ISC_LIST_HEAD(listener->keys); key != NULL; - key = ISC_LIST_NEXT(key, link)) - { - isccc_region_t ccregion; - - ccregion.rstart = isc_buffer_base(conn->ccmsg.buffer); - ccregion.rend = isc_buffer_used(conn->ccmsg.buffer); - secret.rstart = isc_mem_get(listener->mctx, key->secret.length); - memmove(secret.rstart, key->secret.base, key->secret.length); - secret.rend = secret.rstart + key->secret.length; - algorithm = key->algorithm; - result = isccc_cc_fromwire(&ccregion, &request, algorithm, - &secret); - if (result == ISC_R_SUCCESS) { - break; - } - isc_mem_put(listener->mctx, secret.rstart, REGION_SIZE(secret)); - if (result != ISCCC_R_BADAUTH) { - log_invalid(&conn->ccmsg, result); + data = isccc_alist_lookup(conn->response, "_data"); + if (data != NULL) { + if (isccc_cc_defineuint32(data, "result", conn->result) == NULL) + { goto cleanup; } } - if (key == NULL) { - log_invalid(&conn->ccmsg, ISCCC_R_BADAUTH); - goto cleanup; - } - - /* We shouldn't be getting a reply. */ - if (isccc_cc_isreply(request)) { - log_invalid(&conn->ccmsg, ISC_R_FAILURE); - goto cleanup_request; - } - - isc_stdtime_get(&now); - - /* - * Limit exposure to replay attacks. - */ - _ctrl = isccc_alist_lookup(request, "_ctrl"); - if (!isccc_alist_alistp(_ctrl)) { - log_invalid(&conn->ccmsg, ISC_R_FAILURE); - goto cleanup_request; - } - - if (isccc_cc_lookupuint32(_ctrl, "_tim", &sent) == ISC_R_SUCCESS) { - if ((sent + CLOCKSKEW) < now || (sent - CLOCKSKEW) > now) { - log_invalid(&conn->ccmsg, ISCCC_R_CLOCKSKEW); - goto cleanup_request; - } - } else { - log_invalid(&conn->ccmsg, ISC_R_FAILURE); - goto cleanup_request; - } - - /* - * Expire messages that are too old. - */ - if (isccc_cc_lookupuint32(_ctrl, "_exp", &exp) == ISC_R_SUCCESS && - now > exp) { - log_invalid(&conn->ccmsg, ISCCC_R_EXPIRED); - goto cleanup_request; - } - - /* - * Duplicate suppression (required for UDP). - */ - isccc_cc_cleansymtab(listener->controls->symtab, now); - result = isccc_cc_checkdup(listener->controls->symtab, request, now); - if (result != ISC_R_SUCCESS) { - if (result == ISC_R_EXISTS) { - result = ISCCC_R_DUPLICATE; - } - log_invalid(&conn->ccmsg, result); - goto cleanup_request; - } - - if (conn->nonce != 0 && - (isccc_cc_lookupuint32(_ctrl, "_nonce", &nonce) != ISC_R_SUCCESS || - conn->nonce != nonce)) - { - log_invalid(&conn->ccmsg, ISCCC_R_BADAUTH); - goto cleanup_request; - } - - isc_buffer_allocate(listener->mctx, &text, 2 * 2048); - - /* - * Establish nonce. - */ - if (conn->nonce == 0) { - while (conn->nonce == 0) { - isc_nonce_buf(&conn->nonce, sizeof(conn->nonce)); - } - eresult = ISC_R_SUCCESS; - } else { - eresult = named_control_docommand(request, listener->readonly, - &text); - } - - result = isccc_cc_createresponse(request, now, now + 60, &response); - if (result != ISC_R_SUCCESS) { - goto cleanup_request; - } - - data = isccc_alist_lookup(response, "_data"); - if (data != NULL) { - if (isccc_cc_defineuint32(data, "result", eresult) == NULL) { - goto cleanup_response; - } - } - - if (eresult != ISC_R_SUCCESS) { + if (conn->result != ISC_R_SUCCESS) { if (data != NULL) { - const char *estr = isc_result_totext(eresult); + const char *estr = isc_result_totext(conn->result); if (isccc_cc_definestring(data, "err", estr) == NULL) { - goto cleanup_response; + goto cleanup; } } } - if (isc_buffer_usedlength(text) > 0) { + if (isc_buffer_usedlength(conn->text) > 0) { if (data != NULL) { - char *str = (char *)isc_buffer_base(text); + char *str = (char *)isc_buffer_base(conn->text); if (isccc_cc_definestring(data, "text", str) == NULL) { - goto cleanup_response; + goto cleanup; } } } - _ctrl = isccc_alist_lookup(response, "_ctrl"); - if (_ctrl == NULL || - isccc_cc_defineuint32(_ctrl, "_nonce", conn->nonce) == NULL) + conn->ctrl = isccc_alist_lookup(conn->response, "_ctrl"); + if (conn->ctrl == NULL || + isccc_cc_defineuint32(conn->ctrl, "_nonce", conn->nonce) == NULL) { - goto cleanup_response; + goto cleanup; } if (conn->buffer == NULL) { @@ -429,9 +312,10 @@ control_recvmessage(isc_nmhandle_t *handle, isc_result_t result, void *arg) { /* Skip the length field (4 bytes) */ isc_buffer_add(conn->buffer, 4); - result = isccc_cc_towire(response, &conn->buffer, algorithm, &secret); + result = isccc_cc_towire(conn->response, &conn->buffer, conn->alg, + &conn->secret); if (result != ISC_R_SUCCESS) { - goto cleanup_response; + goto cleanup; } isc_buffer_init(&b, conn->buffer->base, 4); @@ -440,30 +324,203 @@ control_recvmessage(isc_nmhandle_t *handle, isc_result_t result, void *arg) { r.base = conn->buffer->base; r.length = conn->buffer->used; + isc_nmhandle_ref(handle); + conn->sending = true; result = isc_nm_send(handle, &r, control_senddone, conn); if (result != ISC_R_SUCCESS) { - goto cleanup_response; - } - conn->sending = true; - - isc_mem_put(listener->mctx, secret.rstart, REGION_SIZE(secret)); - isccc_sexpr_free(&request); - isccc_sexpr_free(&response); - isc_buffer_free(&text); - return; - -cleanup_response: - isccc_sexpr_free(&response); - -cleanup_request: - isccc_sexpr_free(&request); - isc_mem_put(listener->mctx, secret.rstart, REGION_SIZE(secret)); - if (text != NULL) { - isc_buffer_free(&text); + isc_nmhandle_unref(handle); + conn->sending = false; + goto cleanup; } cleanup: + if (conn->response != NULL) { + isccc_sexpr_free(&conn->response); + } + if (conn->request != NULL) { + isccc_sexpr_free(&conn->request); + } + + if (conn->secret.rstart != NULL) { + isc_mem_put(listener->mctx, conn->secret.rstart, + REGION_SIZE(conn->secret)); + } + if (conn->text != NULL) { + isc_buffer_free(&conn->text); + } +} + +static void +control_command(isc_task_t *task, isc_event_t *event) { + controlconnection_t *conn = event->ev_arg; + controllistener_t *listener = conn->listener; + + UNUSED(task); + + conn->result = named_control_docommand(conn->request, + listener->readonly, &conn->text); + control_respond(conn->handle, conn->result, conn); + isc_nmhandle_unref(conn->handle); + isc_event_free(&event); +} + +static void +control_recvmessage(isc_nmhandle_t *handle, isc_result_t result, void *arg) { + controlconnection_t *conn = (controlconnection_t *)arg; + controllistener_t *listener = conn->listener; + controlkey_t *key = NULL; + isc_event_t *event = NULL; + isccc_time_t sent; + isccc_time_t exp; + uint32_t nonce; + conn->ccmsg_valid = false; + + /* Is the server shutting down? */ + if (listener->controls->shuttingdown) { + return; + } + + if (result != ISC_R_SUCCESS) { + if (result != ISC_R_CANCELED && result != ISC_R_EOF) { + log_invalid(&conn->ccmsg, result); + } + + return; + } + + for (key = ISC_LIST_HEAD(listener->keys); key != NULL; + key = ISC_LIST_NEXT(key, link)) + { + isccc_region_t ccregion; + + ccregion.rstart = isc_buffer_base(conn->ccmsg.buffer); + ccregion.rend = isc_buffer_used(conn->ccmsg.buffer); + conn->secret.rstart = isc_mem_get(listener->mctx, + key->secret.length); + memmove(conn->secret.rstart, key->secret.base, + key->secret.length); + conn->secret.rend = conn->secret.rstart + key->secret.length; + conn->alg = key->algorithm; + result = isccc_cc_fromwire(&ccregion, &conn->request, conn->alg, + &conn->secret); + if (result == ISC_R_SUCCESS) { + break; + } + isc_mem_put(listener->mctx, conn->secret.rstart, + REGION_SIZE(conn->secret)); + if (result != ISCCC_R_BADAUTH) { + log_invalid(&conn->ccmsg, result); + return; + } + } + + if (key == NULL) { + log_invalid(&conn->ccmsg, ISCCC_R_BADAUTH); + goto cleanup; + } + + /* We shouldn't be getting a reply. */ + if (isccc_cc_isreply(conn->request)) { + log_invalid(&conn->ccmsg, ISC_R_FAILURE); + goto cleanup; + } + + isc_stdtime_get(&conn->now); + + /* + * Limit exposure to replay attacks. + */ + conn->ctrl = isccc_alist_lookup(conn->request, "_ctrl"); + if (!isccc_alist_alistp(conn->ctrl)) { + log_invalid(&conn->ccmsg, ISC_R_FAILURE); + goto cleanup; + } + + if (isccc_cc_lookupuint32(conn->ctrl, "_tim", &sent) == ISC_R_SUCCESS) { + if ((sent + CLOCKSKEW) < conn->now || + (sent - CLOCKSKEW) > conn->now) { + log_invalid(&conn->ccmsg, ISCCC_R_CLOCKSKEW); + goto cleanup; + } + } else { + log_invalid(&conn->ccmsg, ISC_R_FAILURE); + goto cleanup; + } + + /* + * Expire messages that are too old. + */ + if (isccc_cc_lookupuint32(conn->ctrl, "_exp", &exp) == ISC_R_SUCCESS && + conn->now > exp) + { + log_invalid(&conn->ccmsg, ISCCC_R_EXPIRED); + goto cleanup; + } + + /* + * Duplicate suppression (required for UDP). + */ + isccc_cc_cleansymtab(listener->controls->symtab, conn->now); + result = isccc_cc_checkdup(listener->controls->symtab, conn->request, + conn->now); + if (result != ISC_R_SUCCESS) { + if (result == ISC_R_EXISTS) { + result = ISCCC_R_DUPLICATE; + } + log_invalid(&conn->ccmsg, result); + goto cleanup; + } + + if (conn->nonce != 0 && + (isccc_cc_lookupuint32(conn->ctrl, "_nonce", &nonce) != + ISC_R_SUCCESS || + conn->nonce != nonce)) + { + log_invalid(&conn->ccmsg, ISCCC_R_BADAUTH); + goto cleanup; + } + + isc_buffer_allocate(listener->mctx, &conn->text, 2 * 2048); + + conn->ccmsg_valid = true; + + if (conn->nonce == 0) { + /* + * Establish nonce. + */ + while (conn->nonce == 0) { + isc_nonce_buf(&conn->nonce, sizeof(conn->nonce)); + } + conn->result = ISC_R_SUCCESS; + control_respond(handle, result, conn); + return; + } + + /* + * Trigger the command. + */ + isc_nmhandle_ref(handle); + event = isc_event_allocate(listener->mctx, conn, NAMED_EVENT_COMMAND, + control_command, conn, sizeof(isc_event_t)); + isc_task_send(named_g_server->task, &event); + return; + +cleanup: + if (conn->response != NULL) { + isccc_sexpr_free(&conn->response); + } + if (conn->request != NULL) { + isccc_sexpr_free(&conn->request); + } + + if (conn->secret.rstart != NULL) { + isc_mem_put(listener->mctx, conn->secret.rstart, + REGION_SIZE(conn->secret)); + } + if (conn->text != NULL) { + isc_buffer_free(&conn->text); + } } static void @@ -524,7 +581,8 @@ newconnection(controllistener_t *listener, isc_nmhandle_t *handle) { *conn = (controlconnection_t){ .handle = handle, .listener = listener, - .ccmsg_valid = true }; + .ccmsg_valid = true, + .alg = DST_ALG_UNKNOWN }; isccc_ccmsg_init(listener->mctx, handle, &conn->ccmsg); diff --git a/bin/named/include/named/server.h b/bin/named/include/named/server.h index fdc89958a3..90c9980e17 100644 --- a/bin/named/include/named/server.h +++ b/bin/named/include/named/server.h @@ -38,6 +38,7 @@ #define NAMED_EVENTCLASS ISC_EVENTCLASS(0x4E43) #define NAMED_EVENT_RELOAD (NAMED_EVENTCLASS + 0) #define NAMED_EVENT_DELZONE (NAMED_EVENTCLASS + 1) +#define NAMED_EVENT_COMMAND (NAMED_EVENTCLASS + 2) /*% * Name server state. Better here than in lots of separate global variables. @@ -80,8 +81,6 @@ struct named_server { uint32_t interface_interval; uint32_t heartbeat_interval; - isc_mutex_t reload_event_lock; - isc_event_t * reload_event; named_reload_t reload_status; bool flushonshutdown; diff --git a/bin/named/server.c b/bin/named/server.c index 0d3111c024..da1566ebb8 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -9936,14 +9936,6 @@ named_server_create(isc_mem_t *mctx, named_server_t **serverp) { &server->in_roothints), "setting up root hints"); - isc_mutex_init(&server->reload_event_lock); - - server->reload_event = isc_event_allocate( - named_g_mctx, server, NAMED_EVENT_RELOAD, named_server_reload, - server, sizeof(isc_event_t)); - CHECKFATAL(server->reload_event == NULL ? ISC_R_NOMEMORY - : ISC_R_SUCCESS, - "allocating reload event"); server->reload_status = NAMED_RELOAD_IN_PROGRESS; /* @@ -10113,9 +10105,6 @@ named_server_destroy(named_server_t **serverp) { dst_lib_destroy(); - isc_event_free(&server->reload_event); - isc_mutex_destroy(&server->reload_event_lock); - INSIST(ISC_LIST_EMPTY(server->kasplist)); INSIST(ISC_LIST_EMPTY(server->viewlist)); INSIST(ISC_LIST_EMPTY(server->cachelist)); @@ -10293,7 +10282,7 @@ cleanup: */ static void named_server_reload(isc_task_t *task, isc_event_t *event) { - named_server_t *server = (named_server_t *)event->ev_arg; + named_server_t *server = (named_server_t *)event->ev_sender; INSIST(task == server->task); UNUSED(task); @@ -10303,19 +10292,15 @@ named_server_reload(isc_task_t *task, isc_event_t *event) { "received SIGHUP signal to reload zones"); (void)reload(server); - LOCK(&server->reload_event_lock); - INSIST(server->reload_event == NULL); - server->reload_event = event; - UNLOCK(&server->reload_event_lock); + isc_event_free(&event); } void named_server_reloadwanted(named_server_t *server) { - LOCK(&server->reload_event_lock); - if (server->reload_event != NULL) { - isc_task_send(server->task, &server->reload_event); - } - UNLOCK(&server->reload_event_lock); + isc_event_t *event = isc_event_allocate( + named_g_mctx, server, NAMED_EVENT_RELOAD, named_server_reload, + NULL, sizeof(isc_event_t)); + isc_task_send(server->task, &event); } void diff --git a/bin/rndc/rndc.c b/bin/rndc/rndc.c index 7753f7f463..ef6b31c531 100644 --- a/bin/rndc/rndc.c +++ b/bin/rndc/rndc.c @@ -373,9 +373,9 @@ rndc_recvdone(isc_nmhandle_t *handle, isc_result_t result, void *arg) { if (atomic_load_acquire(&sends) == 0 && atomic_load_acquire(&recvs) == 0) { shuttingdown = true; + isc_nmhandle_unref(handle); isc_task_shutdown(rndc_task); isc_app_shutdown(); - isc_nmhandle_unref(handle); } } diff --git a/lib/isccc/ccmsg.c b/lib/isccc/ccmsg.c index 5b79e6148d..72e79cd9ca 100644 --- a/lib/isccc/ccmsg.c +++ b/lib/isccc/ccmsg.c @@ -49,13 +49,13 @@ recv_nonce(isc_nmhandle_t *handle, isc_result_t eresult, isc_region_t *region, isccc_ccmsg_t *ccmsg = arg; isc_result_t result; + INSIST(VALID_CCMSG(ccmsg)); + if (eresult == ISC_R_CANCELED || eresult == ISC_R_EOF) { ccmsg->result = eresult; goto done; } - INSIST(VALID_CCMSG(ccmsg)); - if (region == NULL && eresult == ISC_R_SUCCESS) { ccmsg->result = ISC_R_EOF; goto done; @@ -116,13 +116,13 @@ recv_message(isc_nmhandle_t *handle, isc_result_t eresult, isc_region_t *region, isccc_ccmsg_t *ccmsg = arg; size_t size; + INSIST(VALID_CCMSG(ccmsg)); + if (eresult == ISC_R_CANCELED || eresult == ISC_R_EOF) { ccmsg->result = eresult; goto done; } - INSIST(VALID_CCMSG(ccmsg)); - if (region == NULL && eresult == ISC_R_SUCCESS) { ccmsg->result = ISC_R_EOF; goto done; From 29dcdeba1b99236253298808fa513f114b8162bc Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Fri, 3 Jul 2020 15:34:51 -0700 Subject: [PATCH 5/8] purge pending command events when shutting down When we're shutting the system down via "rndc stop" or "rndc halt", or reconfiguring the control channel, there are potential shutdown races between the server task and network manager. These are adressed by: - purging any pending command tasks when shutting down the control channel - adding an extra handle reference before the command handler to ensure the handle can't be deleted out from under us before calling command_respond() --- bin/named/controlconf.c | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/bin/named/controlconf.c b/bin/named/controlconf.c index 924b8a74f6..c69650ace5 100644 --- a/bin/named/controlconf.c +++ b/bin/named/controlconf.c @@ -330,7 +330,6 @@ control_respond(isc_nmhandle_t *handle, isc_result_t result, void *arg) { if (result != ISC_R_SUCCESS) { isc_nmhandle_unref(handle); conn->sending = false; - goto cleanup; } cleanup: @@ -357,10 +356,17 @@ control_command(isc_task_t *task, isc_event_t *event) { UNUSED(task); + /* + * An extra ref and two unrefs are needed here to + * ensure the handle isn't cleaned up if we're running + * an "rndc stop" command. + */ + isc_nmhandle_ref(conn->handle); conn->result = named_control_docommand(conn->request, listener->readonly, &conn->text); control_respond(conn->handle, conn->result, conn); isc_nmhandle_unref(conn->handle); + isc_nmhandle_unref(conn->handle); isc_event_free(&event); } @@ -382,7 +388,14 @@ control_recvmessage(isc_nmhandle_t *handle, isc_result_t result, void *arg) { } if (result != ISC_R_SUCCESS) { - if (result != ISC_R_CANCELED && result != ISC_R_EOF) { + if (result == ISC_R_CANCELED) { + /* + * Don't bother with any more scheduled command events. + */ + listener->controls->shuttingdown = true; + isc_task_purge(named_g_server->task, NULL, + NAMED_EVENT_COMMAND, NULL); + } else if (result != ISC_R_EOF) { log_invalid(&conn->ccmsg, result); } From 55896df79d9f537577834375cb21676e8176b5ed Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Wed, 1 Jul 2020 16:17:09 -0700 Subject: [PATCH 6/8] use handles for isc_nm_pauseread() and isc_nm_resumeread() by having these functions act on netmgr handles instead of socket objects, they can be used in callback functions outside the netgmr. --- lib/isc/include/isc/netmgr.h | 15 +++++++++------ lib/isc/netmgr/netmgr.c | 12 ++++++++---- lib/isc/netmgr/tcpdns.c | 10 +++++----- lib/isc/win32/libisc.def.in | 2 ++ 4 files changed, 24 insertions(+), 15 deletions(-) diff --git a/lib/isc/include/isc/netmgr.h b/lib/isc/include/isc/netmgr.h index 99d41f01f4..3f6d8f6f05 100644 --- a/lib/isc/include/isc/netmgr.h +++ b/lib/isc/include/isc/netmgr.h @@ -204,9 +204,12 @@ isc_result_t isc_nm_read(isc_nmhandle_t *handle, isc_nm_recv_cb_t cb, void *cbarg); isc_result_t -isc_nm_pauseread(isc_nmsocket_t *sock); +isc_nm_pauseread(isc_nmhandle_t *handle); /*%< - * Pause reading on this socket, while still remembering the callback. + * Pause reading on this handle's socket, but remember the callback. + * + * Requires: + * \li 'handle' is a valid netmgr handle. */ void @@ -221,13 +224,13 @@ isc_nm_cancelread(isc_nmhandle_t *handle); */ isc_result_t -isc_nm_resumeread(isc_nmsocket_t *sock); +isc_nm_resumeread(isc_nmhandle_t *handle); /*%< - * Resume reading from socket. + * Resume reading on the handle's socket. * * Requires: - * \li 'sock' is a valid netmgr socket - * \li ...for which a read/recv callback has been defined. + * \li 'handle' is a valid netmgr handle. + * \li ...for a socket with a defined read/recv callback. */ isc_result_t diff --git a/lib/isc/netmgr/netmgr.c b/lib/isc/netmgr/netmgr.c index 432d612dbe..0cdf5b3e9c 100644 --- a/lib/isc/netmgr/netmgr.c +++ b/lib/isc/netmgr/netmgr.c @@ -1379,8 +1379,10 @@ isc_nm_cancelread(isc_nmhandle_t *handle) { } isc_result_t -isc_nm_pauseread(isc_nmsocket_t *sock) { - REQUIRE(VALID_NMSOCK(sock)); +isc_nm_pauseread(isc_nmhandle_t *handle) { + REQUIRE(VALID_NMHANDLE(handle)); + + isc_nmsocket_t *sock = handle->sock; switch (sock->type) { case isc_nm_tcpsocket: @@ -1392,8 +1394,10 @@ isc_nm_pauseread(isc_nmsocket_t *sock) { } isc_result_t -isc_nm_resumeread(isc_nmsocket_t *sock) { - REQUIRE(VALID_NMSOCK(sock)); +isc_nm_resumeread(isc_nmhandle_t *handle) { + REQUIRE(VALID_NMHANDLE(handle)); + + isc_nmsocket_t *sock = handle->sock; switch (sock->type) { case isc_nm_tcpsocket: diff --git a/lib/isc/netmgr/tcpdns.c b/lib/isc/netmgr/tcpdns.c index 05cc410d68..512f88be03 100644 --- a/lib/isc/netmgr/tcpdns.c +++ b/lib/isc/netmgr/tcpdns.c @@ -283,7 +283,7 @@ dnslisten_readcb(isc_nmhandle_t *handle, isc_result_t eresult, * one packet, so we're done until the next read * completes. */ - isc_nm_pauseread(dnssock->outerhandle->sock); + isc_nm_pauseread(dnssock->outerhandle); done = true; } else { /* @@ -295,7 +295,7 @@ dnslisten_readcb(isc_nmhandle_t *handle, isc_result_t eresult, */ if (atomic_load(&dnssock->ah) >= TCPDNS_CLIENTS_PER_CONN) { - isc_nm_pauseread(dnssock->outerhandle->sock); + isc_nm_pauseread(dnssock->outerhandle); done = true; } } @@ -376,7 +376,7 @@ isc_nm_tcpdns_sequential(isc_nmhandle_t *handle) { * closehandle_cb callback, called whenever a handle * is released. */ - isc_nm_pauseread(handle->sock->outerhandle->sock); + isc_nm_pauseread(handle->sock->outerhandle); atomic_store(&handle->sock->sequential, true); } @@ -431,7 +431,7 @@ resume_processing(void *arg) { } isc_nmhandle_unref(handle); } else if (sock->outerhandle != NULL) { - result = isc_nm_resumeread(sock->outerhandle->sock); + result = isc_nm_resumeread(sock->outerhandle); if (result != ISC_R_SUCCESS) { isc_nmhandle_unref(sock->outerhandle); sock->outerhandle = NULL; @@ -454,7 +454,7 @@ resume_processing(void *arg) { * Nothing in the buffer; resume reading. */ if (sock->outerhandle != NULL) { - isc_nm_resumeread(sock->outerhandle->sock); + isc_nm_resumeread(sock->outerhandle); } break; diff --git a/lib/isc/win32/libisc.def.in b/lib/isc/win32/libisc.def.in index 7313a73acb..102a2263ed 100644 --- a/lib/isc/win32/libisc.def.in +++ b/lib/isc/win32/libisc.def.in @@ -457,7 +457,9 @@ isc_nm_listentcpdns isc_nm_listentcp isc_nm_listenudp isc_nm_maxudp +isc_nm_pauseread isc_nm_read +isc_nm_resumeread isc_nm_send isc_nm_setstats isc_nm_start From ae5d316f649cf2db6d133f9e036f3528ab934aeb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Witold=20Kr=C4=99cicki?= Date: Wed, 1 Jul 2020 19:07:04 +0200 Subject: [PATCH 7/8] isccc: merge recv_message and recv_nonce into one function - make isccc message receiving code clearer by merging recv_nonce and recv_message into a single recv_data function and adding a boolean state field. --- lib/isccc/ccmsg.c | 138 ++++++++++++-------------------- lib/isccc/include/isccc/ccmsg.h | 2 + 2 files changed, 52 insertions(+), 88 deletions(-) diff --git a/lib/isccc/ccmsg.c b/lib/isccc/ccmsg.c index 72e79cd9ca..5e80cf82da 100644 --- a/lib/isccc/ccmsg.c +++ b/lib/isccc/ccmsg.c @@ -40,79 +40,8 @@ #define VALID_CCMSG(foo) ISC_MAGIC_VALID(foo, CCMSG_MAGIC) static void -recv_message(isc_nmhandle_t *handle, isc_result_t eresult, isc_region_t *region, - void *arg); - -static void -recv_nonce(isc_nmhandle_t *handle, isc_result_t eresult, isc_region_t *region, - void *arg) { - isccc_ccmsg_t *ccmsg = arg; - isc_result_t result; - - INSIST(VALID_CCMSG(ccmsg)); - - if (eresult == ISC_R_CANCELED || eresult == ISC_R_EOF) { - ccmsg->result = eresult; - goto done; - } - - if (region == NULL && eresult == ISC_R_SUCCESS) { - ccmsg->result = ISC_R_EOF; - goto done; - } else if (eresult != ISC_R_SUCCESS) { - ccmsg->result = eresult; - goto done; - } else { - ccmsg->result = eresult; - } - - if (region->length < sizeof(uint32_t)) { - ccmsg->result = ISC_R_UNEXPECTEDEND; - goto done; - } - - ccmsg->size = ntohl(*(uint32_t *)region->base); - if (ccmsg->size == 0) { - ccmsg->result = ISC_R_UNEXPECTEDEND; - goto done; - } - if (ccmsg->size > ccmsg->maxsize) { - ccmsg->result = ISC_R_RANGE; - goto done; - } - - isc_region_consume(region, sizeof(uint32_t)); - isc_buffer_allocate(ccmsg->mctx, &ccmsg->buffer, ccmsg->size); - - /* - * If there's more of the message waiting, pass it to - * recv_message() directly. - */ - if (region->length != 0) { - recv_message(handle, ISC_R_SUCCESS, region, ccmsg); - return; - } - - /* - * Otherwise, continue reading and handle it in - * recv_message(). - */ - result = isc_nm_read(handle, recv_message, ccmsg); - if (result == ISC_R_SUCCESS) { - return; - } - - ccmsg->result = result; - -done: - ccmsg->cb(handle, ccmsg->result, ccmsg->cbarg); - isc_nmhandle_unref(handle); -} - -static void -recv_message(isc_nmhandle_t *handle, isc_result_t eresult, isc_region_t *region, - void *arg) { - isc_result_t result; +recv_data(isc_nmhandle_t *handle, isc_result_t eresult, isc_region_t *region, + void *arg) { isccc_ccmsg_t *ccmsg = arg; size_t size; @@ -121,9 +50,7 @@ recv_message(isc_nmhandle_t *handle, isc_result_t eresult, isc_region_t *region, if (eresult == ISC_R_CANCELED || eresult == ISC_R_EOF) { ccmsg->result = eresult; goto done; - } - - if (region == NULL && eresult == ISC_R_SUCCESS) { + } else if (region == NULL && eresult == ISC_R_SUCCESS) { ccmsg->result = ISC_R_EOF; goto done; } else if (eresult != ISC_R_SUCCESS) { @@ -133,11 +60,38 @@ recv_message(isc_nmhandle_t *handle, isc_result_t eresult, isc_region_t *region, ccmsg->result = eresult; } - if (region->length == 0) { - ccmsg->result = ISC_R_UNEXPECTEDEND; - goto done; + if (!ccmsg->length_received) { + if (region->length < sizeof(uint32_t)) { + ccmsg->result = ISC_R_UNEXPECTEDEND; + goto done; + } + + ccmsg->size = ntohl(*(uint32_t *)region->base); + + if (ccmsg->size == 0) { + ccmsg->result = ISC_R_UNEXPECTEDEND; + goto done; + } + if (ccmsg->size > ccmsg->maxsize) { + ccmsg->result = ISC_R_RANGE; + goto done; + } + + isc_region_consume(region, sizeof(uint32_t)); + isc_buffer_allocate(ccmsg->mctx, &ccmsg->buffer, ccmsg->size); + + ccmsg->length_received = true; } + /* + * If there's no more data, wait for more + */ + if (region->length == 0) { + return; + } + + /* We have some data in the buffer, read it */ + size = ISC_MIN(isc_buffer_availablelength(ccmsg->buffer), region->length); isc_buffer_putmem(ccmsg->buffer, region->base, size); @@ -148,14 +102,11 @@ recv_message(isc_nmhandle_t *handle, isc_result_t eresult, isc_region_t *region, goto done; } - result = isc_nm_read(handle, recv_message, ccmsg); - if (result == ISC_R_SUCCESS) { - return; - } - - ccmsg->result = result; + /* Wait for more data to come */ + return; done: + isc_nm_pauseread(handle); ccmsg->cb(handle, ccmsg->result, ccmsg->cbarg); isc_nmhandle_unref(handle); } @@ -196,10 +147,19 @@ isccc_ccmsg_readmessage(isccc_ccmsg_t *ccmsg, isc_nm_cb_t cb, void *cbarg) { ccmsg->cb = cb; ccmsg->cbarg = cbarg; ccmsg->result = ISC_R_UNEXPECTED; /* unknown right now */ + ccmsg->length_received = false; isc_nmhandle_ref(ccmsg->handle); - result = isc_nm_read(ccmsg->handle, recv_nonce, ccmsg); - if (result != ISC_R_SUCCESS) { + if (ccmsg->reading) { + result = isc_nm_resumeread(ccmsg->handle); + } else { + result = isc_nm_read(ccmsg->handle, recv_data, ccmsg); + ccmsg->reading = true; + } + if (result == ISC_R_CANCELED) { + ccmsg->reading = false; + } else if (result != ISC_R_SUCCESS) { + ccmsg->reading = false; isc_nmhandle_unref(ccmsg->handle); } @@ -210,7 +170,9 @@ void isccc_ccmsg_cancelread(isccc_ccmsg_t *ccmsg) { REQUIRE(VALID_CCMSG(ccmsg)); - isc_nm_cancelread(ccmsg->handle); + if (ccmsg->reading) { + isc_nm_cancelread(ccmsg->handle); + } } void diff --git a/lib/isccc/include/isccc/ccmsg.h b/lib/isccc/include/isccc/ccmsg.h index 3451007bf9..baa2c19bb3 100644 --- a/lib/isccc/include/isccc/ccmsg.h +++ b/lib/isccc/include/isccc/ccmsg.h @@ -40,12 +40,14 @@ typedef struct isccc_ccmsg { /* private (don't touch!) */ unsigned int magic; uint32_t size; + bool length_received; isc_buffer_t * buffer; unsigned int maxsize; isc_mem_t * mctx; isc_nmhandle_t *handle; isc_nm_cb_t cb; void * cbarg; + bool reading; /* public (read-only) */ isc_result_t result; } isccc_ccmsg_t; From 7c703c851f3efc16827a0f96a71766bc46118b8d Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Wed, 1 Jul 2020 10:59:33 -0700 Subject: [PATCH 8/8] CHANGES, release note --- CHANGES | 12 ++++++++++++ doc/notes/notes-current.rst | 9 ++++++++- 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/CHANGES b/CHANGES index f8df942d7d..4ba2e31864 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,15 @@ +5467. [func] The control channel and the rndc utility have been + updated to use the new network manager. To support + this, the network manager was updated to enable + wthe initiation of client TCP connections. Its + internal reference counting has been refactored. + + Note: As side effects of this change, rndc cannot + currently be used with UNIX-domain sockets, and its + default timeout has changed from 60 seconds to 30. + These will be addressed in a future release. + [GL #1759] + 5466. [bug] Addressed an error in recursive clients stats reporting. [GL #1719] diff --git a/doc/notes/notes-current.rst b/doc/notes/notes-current.rst index 8aab454039..1ce674d1e5 100644 --- a/doc/notes/notes-current.rst +++ b/doc/notes/notes-current.rst @@ -24,7 +24,14 @@ Known Issues New Features ~~~~~~~~~~~~ -- None. +- ``rndc`` has been updated to use the new BIND network manager API. + This change had the side effect of altering the TCP timeout for RNDC + connections from 60 seconds to the ``tcp-idle-timeout`` value, which + defaults to 30 seconds. Also, because the network manager currently + has no support for UNIX-domain sockets, those cannot now be used + with ``rndc``. This will be addressed in a future release, either by + restoring UNIX-domain socket support or by formally declaring them + to be obsolete in the control channel. [GL #1759] Feature Changes ~~~~~~~~~~~~~~~