From 336c6ae5c2dd520961b530e97cdb022f2bd62f4d Mon Sep 17 00:00:00 2001 From: Tim Wojtulewicz Date: Fri, 12 Nov 2021 10:02:51 -0700 Subject: [PATCH] Replace nb_dns library with C-Ares --- .gitmodules | 3 + CMakeLists.txt | 1 + auxil/c-ares | 1 + cmake | 2 +- src/DNS_Mgr.cc | 600 ++++++++++++++++++++++------------------------ src/DNS_Mgr.h | 155 ++++++++---- src/zeek-setup.cc | 15 +- src/zeek.bif | 6 +- 8 files changed, 423 insertions(+), 360 deletions(-) create mode 160000 auxil/c-ares diff --git a/.gitmodules b/.gitmodules index 980a23e6eb..0c5cccce1a 100644 --- a/.gitmodules +++ b/.gitmodules @@ -52,3 +52,6 @@ [submodule "auxil/gen-zam"] path = auxil/gen-zam url = https://github.com/zeek/gen-zam +[submodule "auxil/c-ares"] + path = auxil/c-ares + url = https://github.com/c-ares/c-ares diff --git a/CMakeLists.txt b/CMakeLists.txt index 646aeefeb4..402a0311a9 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -480,6 +480,7 @@ include(CheckNameserCompat) include(GetArchitecture) include(RequireCXX17) include(FindKqueue) +include(FindCAres) if ( (OPENSSL_VERSION VERSION_EQUAL "1.1.0") OR (OPENSSL_VERSION VERSION_GREATER "1.1.0") ) set(ZEEK_HAVE_OPENSSL_1_1 true CACHE INTERNAL "" FORCE) diff --git a/auxil/c-ares b/auxil/c-ares new file mode 160000 index 0000000000..2aa086f822 --- /dev/null +++ b/auxil/c-ares @@ -0,0 +1 @@ +Subproject commit 2aa086f822aad5017a6f2061ef656f237a62d0ed diff --git a/cmake b/cmake index 649c319f88..0fa8b3a20c 160000 --- a/cmake +++ b/cmake @@ -1 +1 @@ -Subproject commit 649c319f88e2966931892d55adb2ee50f278662b +Subproject commit 0fa8b3a20ca6d4e32b56cdeee693cb46eec41f5e diff --git a/src/DNS_Mgr.cc b/src/DNS_Mgr.cc index 4b3b4b9201..2e240a3bf2 100644 --- a/src/DNS_Mgr.cc +++ b/src/DNS_Mgr.cc @@ -4,31 +4,29 @@ #include "zeek/zeek-config.h" -#include -#include -#ifdef TIME_WITH_SYS_TIME -#include -#include -#else -#ifdef HAVE_SYS_TIME_H -#include -#else -#include -#endif -#endif - #include #include +#include +#include +#include #include #include #include -#ifdef HAVE_MEMORY_H -#include -#endif -#include #include #include +#ifdef TIME_WITH_SYS_TIME +#include +#include +#elif defined(HAVE_SYS_TIME_H) +#include +#else +#include +#endif + +#include +#include + #include "zeek/3rdparty/doctest.h" #include "zeek/DNS_Mapping.h" #include "zeek/Event.h" @@ -43,65 +41,116 @@ #include "zeek/ZeekString.h" #include "zeek/iosource/Manager.h" -extern "C" - { - extern int select(int, fd_set*, fd_set*, fd_set*, struct timeval*); - -#include - -#include "zeek/3rdparty/nb_dns.h" - } - -using namespace std; +// Number of seconds we'll wait for a reply. +constexpr int DNS_TIMEOUT = 5; namespace zeek::detail { +static void hostbyaddr_callback(void* arg, int status, int timeouts, struct hostent* hostent) + { + printf("host callback\n"); + // TODO: implement this + // TODO: figure out how to get TTL info here + } + +static void addrinfo_callback(void* arg, int status, int timeouts, struct ares_addrinfo* result) + { + printf("addrinfo callback\n"); + + if ( status != ARES_SUCCESS ) + { + // TODO: error or something here, or just give up on it? + ares_freeaddrinfo(result); + return; + } + + // TODO: the existing code doesn't handle hostname aliases at all. Should we? + // TODO: handle IPv6 mode + + std::vector addrs; + for ( ares_addrinfo_node* entry = result->nodes; entry != NULL; entry = entry->ai_next ) + addrs.push_back(&reinterpret_cast(entry->ai_addr)->sin_addr); + + // Push a null on the end so the addr list has a final point during later parsing. + addrs.push_back(NULL); + + struct hostent he; + he.h_name = util::copy_string(result->name); + he.h_aliases = NULL; + he.h_addrtype = AF_INET; + he.h_length = sizeof(in_addr); + he.h_addr_list = reinterpret_cast(addrs.data()); + + auto req = reinterpret_cast(arg); + dns_mgr->AddResult(req, &he, result->nodes[0].ai_ttl); + + delete[] he.h_name; + + ares_freeaddrinfo(result); + } + +static void ares_sock_cb(void* data, int s, int read, int write) + { + printf("Change state fd %d read:%d write:%d\n", s, read, write); + if ( read == 1 ) + iosource_mgr->RegisterFd(s, reinterpret_cast(data)); + else + iosource_mgr->UnregisterFd(s, reinterpret_cast(data)); + } + class DNS_Mgr_Request { public: DNS_Mgr_Request(const char* h, int af, bool is_txt) - : host(util::copy_string(h)), fam(af), qtype(is_txt ? 16 : 0), addr(), request_pending() + : host(util::copy_string(h)), fam(af), qtype(is_txt ? 16 : 0), addr() { } - DNS_Mgr_Request(const IPAddr& a) : host(), fam(), qtype(), addr(a), request_pending() { } + DNS_Mgr_Request(const IPAddr& a) : addr(a) { } ~DNS_Mgr_Request() { delete[] host; } // Returns nil if this was an address request. const char* ReqHost() const { return host; } const IPAddr& ReqAddr() const { return addr; } + int Family() const { return fam; } bool ReqIsTxt() const { return qtype == 16; } - int MakeRequest(nb_dns_info* nb_dns); - int RequestPending() const { return request_pending; } - void RequestDone() { request_pending = 0; } + void MakeRequest(ares_channel channel); + + bool RequestPending() const { return request_pending; } + void RequestDone() { request_pending = false; } protected: - char* host; // if non-nil, this is a host request - int fam; // address family query type for host requests - int qtype; // Query type + char* host = nullptr; // if non-nil, this is a host request + int fam = 0; // address family query type for host requests + int qtype = 0; // Query type IPAddr addr; - int request_pending; + bool request_pending = false; }; -int DNS_Mgr_Request::MakeRequest(nb_dns_info* nb_dns) +void DNS_Mgr_Request::MakeRequest(ares_channel channel) { - if ( ! nb_dns ) - return 0; + request_pending = true; - request_pending = 1; + // TODO: TXT requests? + // TODO: could this use ares_create_query/ares_query instead of the + // ares_get* methods to make it more generic? I think we might need + // to do that for TXT requests. - char err[NB_DNS_ERRSIZE]; if ( host ) - return nb_dns_host_request2(nb_dns, host, fam, qtype, (void*)this, err) >= 0; + { + ares_addrinfo_hints hints = {ARES_AI_CANONNAME, fam, 0, 0}; + ares_getaddrinfo(channel, host, NULL, &hints, addrinfo_callback, this); + } else { const uint32_t* bytes; int len = addr.GetBytes(&bytes); - return nb_dns_addr_request2(nb_dns, (char*)bytes, len == 1 ? AF_INET : AF_INET6, - (void*)this, err) >= 0; + + ares_gethostbyaddr(channel, bytes, len, addr.GetFamily() == IPv4 ? AF_INET : AF_INET6, + hostbyaddr_callback, this); } } @@ -111,22 +160,22 @@ DNS_Mgr::DNS_Mgr(DNS_MgrMode arg_mode) mode = arg_mode; - cache_name = dir = nullptr; - asyncs_pending = 0; num_requests = 0; successful = 0; failed = 0; - nb_dns = nullptr; + ipv6_resolver = false; + + ares_library_init(ARES_LIB_INIT_ALL); } DNS_Mgr::~DNS_Mgr() { - if ( nb_dns ) - nb_dns_finish(nb_dns); + Flush(); - delete[] cache_name; - delete[] dir; + ares_cancel(channel); + ares_destroy(channel); + ares_library_cleanup(); } void DNS_Mgr::InitSource() @@ -134,21 +183,36 @@ void DNS_Mgr::InitSource() if ( did_init ) return; + ares_options options; + int optmask = 0; + + options.flags = ARES_FLAG_STAYOPEN; + optmask |= ARES_OPT_FLAGS; + + options.timeout = DNS_TIMEOUT; + optmask |= ARES_OPT_TIMEOUT; + + options.sock_state_cb = ares_sock_cb; + options.sock_state_cb_data = this; + optmask |= ARES_OPT_SOCK_STATE_CB; + + int status = ares_init_options(&channel, &options, optmask); + if ( status != ARES_SUCCESS ) + reporter->FatalError("Failed to initialize c-ares for DNS resolution: %s", + ares_strerror(status)); + // Note that Init() may be called by way of LookupHost() during the act of // parsing a hostname literal (e.g. google.com), so we can't use a // script-layer option to configure the DNS resolver as it may not be // configured to the user's desired address at the time when we need to to // the lookup. auto dns_resolver = getenv("ZEEK_DNS_RESOLVER"); - auto dns_resolver_addr = dns_resolver ? IPAddr(dns_resolver) : IPAddr(); - char err[NB_DNS_ERRSIZE]; - - if ( dns_resolver_addr == IPAddr() ) - nb_dns = nb_dns_init(err); - else + if ( dns_resolver ) { - // nb_dns expects a sockaddr, so copy the address out of the IPAddr - // object into one so it can be passed. + ares_addr_node servers; + servers.next = nullptr; + + auto dns_resolver_addr = IPAddr(dns_resolver); struct sockaddr_storage ss = {0}; if ( dns_resolver_addr.GetFamily() == IPv4 ) @@ -156,25 +220,21 @@ void DNS_Mgr::InitSource() struct sockaddr_in* sa = (struct sockaddr_in*)&ss; sa->sin_family = AF_INET; dns_resolver_addr.CopyIPv4(&sa->sin_addr); + + servers.family = AF_INET; + memcpy(&(servers.addr.addr4), &sa->sin_addr, sizeof(struct in_addr)); } else { struct sockaddr_in6* sa = (struct sockaddr_in6*)&ss; sa->sin6_family = AF_INET6; dns_resolver_addr.CopyIPv6(&sa->sin6_addr); + + servers.family = AF_INET6; + memcpy(&(servers.addr.addr6), &sa->sin6_addr, sizeof(ares_in6_addr)); } - nb_dns = nb_dns_init2(err, (struct sockaddr*)&ss); - } - - if ( nb_dns ) - { - if ( ! doctest::is_running_in_test && ! iosource_mgr->RegisterFd(nb_dns_fd(nb_dns), this) ) - reporter->FatalError("Failed to register nb_dns file descriptor with iosource_mgr"); - } - else - { - reporter->Warning("problem initializing NB-DNS: %s", err); + ares_set_servers(channel, &servers); } did_init = true; @@ -186,7 +246,7 @@ void DNS_Mgr::InitPostScript() { dm_rec = id::find_type("dns_mapping"); - // Registering will call Init() + // Registering will call InitSource(), which sets up all of the DNS library stuff iosource_mgr->Register(this, true); } else @@ -195,10 +255,10 @@ void DNS_Mgr::InitPostScript() InitSource(); } - const char* cache_dir = dir ? dir : "."; - cache_name = new char[strlen(cache_dir) + 64]; - sprintf(cache_name, "%s/%s", cache_dir, ".zeek-dns-cache"); - LoadCache(fopen(cache_name, "r")); + // Load the DNS cache from disk, if it exists. + std::string cache_dir = dir.empty() ? dir : "."; + cache_name = util::fmt("%s/%s", cache_dir.c_str(), ".zeek-dns-cache"); + LoadCache(cache_name); } static TableValPtr fake_name_lookup_result(const char* name) @@ -229,11 +289,11 @@ TableValPtr DNS_Mgr::LookupHost(const char* name) if ( mode == DNS_FAKE ) return fake_name_lookup_result(name); + // This should have been run already from InitPostScript(), but just run it again just + // in case it hadn't. InitSource(); - if ( ! nb_dns ) - return empty_addr_set(); - + // Check the cache before attempting to look up the name remotely. if ( mode != DNS_PRIME ) { HostMap::iterator it = host_mappings.find(name); @@ -258,23 +318,45 @@ TableValPtr DNS_Mgr::LookupHost(const char* name) } } - // Not found, or priming. + // Not found, or priming. We use ares_getaddrinfo here because we want the TTL value switch ( mode ) { case DNS_PRIME: - requests.push_back(new DNS_Mgr_Request(name, AF_INET, false)); - requests.push_back(new DNS_Mgr_Request(name, AF_INET6, false)); + { + // TODO: not sure we need to do these split like this if we can pass AF_UNSPEC + // in the hints structure. Do we really need the two different request objects? + auto v4 = new DNS_Mgr_Request(name, AF_INET, false); + ares_addrinfo_hints v4_hints = {ARES_AI_CANONNAME, AF_INET, 0, 0}; + ares_getaddrinfo(channel, name, NULL, &v4_hints, addrinfo_callback, v4); + + // TODO: check if ipv6 support is needed if we use AF_UNSPEC above + // auto v6 = new DNS_Mgr_Request(name, AF_INET6, false); + // ares_addrinfo_hints v6_hints = { 0, AF_INET6, 0, 0 }; + // ares_getaddrinfo(channel, name, NULL, &v6_hints, addrinfo_callback, v6); + return empty_addr_set(); + } case DNS_FORCE: reporter->FatalError("can't find DNS entry for %s in cache", name); return nullptr; case DNS_DEFAULT: - requests.push_back(new DNS_Mgr_Request(name, AF_INET, false)); - requests.push_back(new DNS_Mgr_Request(name, AF_INET6, false)); + { + auto v4 = new DNS_Mgr_Request(name, AF_INET, false); + ares_addrinfo_hints v4_hints = {ARES_AI_CANONNAME, AF_INET, 0, 0}; + ares_getaddrinfo(channel, name, NULL, &v4_hints, addrinfo_callback, v4); + + // TODO: check if ipv6 support is needed if we use AF_UNSPEC above + // auto v6 = new DNS_Mgr_Request(name, AF_INET6, false); + // ares_addrinfo_hints v6_hints = { 0, AF_INET6, 0, 0 }; + // ares_getaddrinfo(channel, name, NULL, &v6_hints, addrinfo_callback, v6); + Resolve(); + + // Call LookupHost() a second time to get the newly stored value out of the cache. return LookupHost(name); + } default: reporter->InternalError("bad mode in DNS_Mgr::LookupHost"); @@ -287,11 +369,11 @@ StringValPtr DNS_Mgr::LookupAddr(const IPAddr& addr) if ( mode == DNS_FAKE ) return make_intrusive(fake_addr_lookup_result(addr)); + // This should have been run already from InitPostScript(), but just run it again just + // in case it hadn't. InitSource(); - if ( ! nb_dns ) - return make_intrusive(""); - + // Check the cache before attempting to look up the name remotely. if ( mode != DNS_PRIME ) { AddrMap::iterator it = addr_mappings.find(addr); @@ -303,28 +385,41 @@ StringValPtr DNS_Mgr::LookupAddr(const IPAddr& addr) return d->Host(); else { - string s(addr); + std::string s(addr); reporter->Warning("can't resolve IP address: %s", s.c_str()); return make_intrusive(s.c_str()); } } } + const uint32_t* bytes; + int len = addr.GetBytes(&bytes); + // Not found, or priming. switch ( mode ) { case DNS_PRIME: - requests.push_back(new DNS_Mgr_Request(addr)); + { + auto req = new DNS_Mgr_Request(addr); + ares_gethostbyaddr(channel, bytes, len, addr.GetFamily() == IPv4 ? AF_INET : AF_INET6, + hostbyaddr_callback, req); return make_intrusive(""); + } case DNS_FORCE: reporter->FatalError("can't find DNS entry for %s in cache", addr.AsString().c_str()); return nullptr; case DNS_DEFAULT: - requests.push_back(new DNS_Mgr_Request(addr)); + { + auto req = new DNS_Mgr_Request(addr); + ares_gethostbyaddr(channel, bytes, len, addr.GetFamily() == IPv4 ? AF_INET : AF_INET6, + hostbyaddr_callback, req); Resolve(); + + // Call LookupAddr() a second time to get the newly stored value out of the cache. return LookupAddr(addr); + } default: reporter->InternalError("bad mode in DNS_Mgr::LookupAddr"); @@ -332,109 +427,35 @@ StringValPtr DNS_Mgr::LookupAddr(const IPAddr& addr) } } -void DNS_Mgr::Verify() { } - -#define MAX_PENDING_REQUESTS 20 +constexpr int MAX_PENDING_REQUESTS = 20; void DNS_Mgr::Resolve() { - if ( ! nb_dns ) - return; + int nfds = 0; + struct timeval *tvp, tv; + fd_set read_fds, write_fds; - int i; + tv.tv_sec = DNS_TIMEOUT; + tv.tv_usec = 0; - int first_req = 0; - int num_pending = min(requests.length(), MAX_PENDING_REQUESTS); - int last_req = num_pending - 1; - - // Prime with the initial requests. - for ( i = first_req; i <= last_req; ++i ) - requests[i]->MakeRequest(nb_dns); - - // Start resolving. Each time an answer comes in, we can issue a - // new request, if we have more. - while ( num_pending > 0 ) + for ( int i = 0; i < MAX_PENDING_REQUESTS; i++ ) { - int status = AnswerAvailable(DNS_TIMEOUT); + FD_ZERO(&read_fds); + FD_ZERO(&write_fds); + nfds = ares_fds(channel, &read_fds, &write_fds); + if ( nfds == 0 ) + break; - if ( status <= 0 ) - { - // Error or timeout. Process all pending requests as - // unanswered and reprime. - for ( i = first_req; i <= last_req; ++i ) - { - DNS_Mgr_Request* dr = requests[i]; - if ( dr->RequestPending() ) - { - AddResult(dr, nullptr); - dr->RequestDone(); - } - } - - first_req = last_req + 1; - num_pending = min(requests.length() - first_req, MAX_PENDING_REQUESTS); - last_req = first_req + num_pending - 1; - - for ( i = first_req; i <= last_req; ++i ) - requests[i]->MakeRequest(nb_dns); - - continue; - } - - char err[NB_DNS_ERRSIZE]; - struct nb_dns_result r; - status = nb_dns_activity(nb_dns, &r, err); - if ( status < 0 ) - reporter->Warning("NB-DNS error in DNS_Mgr::WaitForReplies (%s)", err); - else if ( status > 0 ) - { - DNS_Mgr_Request* dr = (DNS_Mgr_Request*)r.cookie; - if ( dr->RequestPending() ) - { - AddResult(dr, &r); - dr->RequestDone(); - } - - // Room for another, if we have it. - if ( last_req < requests.length() - 1 ) - { - ++last_req; - requests[last_req]->MakeRequest(nb_dns); - } - else - --num_pending; - } + tvp = ares_timeout(channel, &tv, &tv); + select(nfds, &read_fds, &write_fds, NULL, tvp); + ares_process(channel, &read_fds, &write_fds); } - - // All done with the list of requests. - for ( i = requests.length() - 1; i >= 0; --i ) - delete requests.remove_nth(i); - } - -bool DNS_Mgr::Save() - { - if ( ! cache_name ) - return false; - - FILE* f = fopen(cache_name, "w"); - - if ( ! f ) - return false; - - Save(f, host_mappings); - Save(f, addr_mappings); - // Save(f, text_mappings); // We don't save the TXT mappings (yet?). - - fclose(f); - - return true; } void DNS_Mgr::Event(EventHandlerPtr e, DNS_Mapping* dm) { if ( ! e ) return; - event_mgr.Enqueue(e, BuildMappingVal(dm)); } @@ -473,14 +494,11 @@ ValPtr DNS_Mgr::BuildMappingVal(DNS_Mapping* dm) return r; } -void DNS_Mgr::AddResult(DNS_Mgr_Request* dr, struct nb_dns_result* r) +void DNS_Mgr::AddResult(DNS_Mgr_Request* dr, struct hostent* h, uint32_t ttl) { - struct hostent* h = (r && r->host_errno == 0) ? r->hostent : nullptr; - u_int32_t ttl = (r && r->host_errno == 0) ? r->ttl : 0; - DNS_Mapping* new_dm; DNS_Mapping* prev_dm; - int keep_prev = 0; + bool keep_prev = false; if ( dr->ReqHost() ) { @@ -502,7 +520,7 @@ void DNS_Mgr::AddResult(DNS_Mgr_Request* dr, struct nb_dns_result* r) if ( new_dm->Failed() && prev_dm && prev_dm->Valid() ) { text_mappings[dr->ReqHost()] = prev_dm; - ++keep_prev; + keep_prev = true; } } else @@ -537,7 +555,7 @@ void DNS_Mgr::AddResult(DNS_Mgr_Request* dr, struct nb_dns_result* r) else host_mappings[dr->ReqHost()].second = prev_dm; - ++keep_prev; + keep_prev = true; } } } @@ -551,7 +569,7 @@ void DNS_Mgr::AddResult(DNS_Mgr_Request* dr, struct nb_dns_result* r) if ( new_dm->Failed() && prev_dm && prev_dm->Valid() ) { addr_mappings[dr->ReqAddr()] = prev_dm; - ++keep_prev; + keep_prev = true; } } @@ -644,11 +662,14 @@ void DNS_Mgr::DumpAddrList(FILE* f, ListVal* al) } } -void DNS_Mgr::LoadCache(FILE* f) +void DNS_Mgr::LoadCache(const std::string& path) { + FILE* f = fopen(path.c_str(), "r"); + if ( ! f ) return; + // Loop until we find a mapping that doesn't initialize correctly. DNS_Mapping* m = new DNS_Mapping(f); for ( ; ! m->NoMapping() && ! m->InitFailed(); m = new DNS_Mapping(f) ) { @@ -677,6 +698,25 @@ void DNS_Mgr::LoadCache(FILE* f) fclose(f); } +bool DNS_Mgr::Save() + { + if ( cache_name.empty() ) + return false; + + FILE* f = fopen(cache_name.c_str(), "w"); + + if ( ! f ) + return false; + + Save(f, host_mappings); + Save(f, addr_mappings); + // Save(f, text_mappings); // We don't save the TXT mappings (yet?). + + fclose(f); + + return true; + } + void DNS_Mgr::Save(FILE* f, const AddrMap& m) { for ( AddrMap::const_iterator it = m.begin(); it != m.end(); ++it ) @@ -688,9 +728,7 @@ void DNS_Mgr::Save(FILE* f, const AddrMap& m) void DNS_Mgr::Save(FILE* f, const HostMap& m) { - HostMap::const_iterator it; - - for ( it = m.begin(); it != m.end(); ++it ) + for ( HostMap::const_iterator it = m.begin(); it != m.end(); ++it ) { if ( it->second.first ) it->second.first->Save(f); @@ -721,7 +759,7 @@ const char* DNS_Mgr::LookupAddrInCache(const IPAddr& addr) return d->names.empty() ? "<\?\?\?>" : d->names[0].c_str(); } -TableValPtr DNS_Mgr::LookupNameInCache(const string& name) +TableValPtr DNS_Mgr::LookupNameInCache(const std::string& name) { HostMap::iterator it = host_mappings.find(name); if ( it == host_mappings.end() ) @@ -750,7 +788,7 @@ TableValPtr DNS_Mgr::LookupNameInCache(const string& name) return tv6; } -const char* DNS_Mgr::LookupTextInCache(const string& name) +const char* DNS_Mgr::LookupTextInCache(const std::string& name) { TextMap::iterator it = text_mappings.find(name); if ( it == text_mappings.end() ) @@ -792,6 +830,8 @@ static void resolve_lookup_cb(DNS_Mgr::LookupCallback* callback, const char* res void DNS_Mgr::AsyncLookupAddr(const IPAddr& host, LookupCallback* callback) { + // This should have been run already from InitPostScript(), but just run it again just + // in case it hadn't. InitSource(); if ( mode == DNS_FAKE ) @@ -828,8 +868,10 @@ void DNS_Mgr::AsyncLookupAddr(const IPAddr& host, LookupCallback* callback) IssueAsyncRequests(); } -void DNS_Mgr::AsyncLookupName(const string& name, LookupCallback* callback) +void DNS_Mgr::AsyncLookupName(const std::string& name, LookupCallback* callback) { + // This should have been run already from InitPostScript(), but just run it again just + // in case it hadn't. InitSource(); if ( mode == DNS_FAKE ) @@ -866,8 +908,10 @@ void DNS_Mgr::AsyncLookupName(const string& name, LookupCallback* callback) IssueAsyncRequests(); } -void DNS_Mgr::AsyncLookupNameText(const string& name, LookupCallback* callback) +void DNS_Mgr::AsyncLookupNameText(const std::string& name, LookupCallback* callback) { + // This should have been run already from InitPostScript(), but just run it again just + // in case it hadn't. InitSource(); if ( mode == DNS_FAKE ) @@ -906,51 +950,35 @@ void DNS_Mgr::AsyncLookupNameText(const string& name, LookupCallback* callback) IssueAsyncRequests(); } -static bool DoRequest(nb_dns_info* nb_dns, DNS_Mgr_Request* dr) - { - if ( dr->MakeRequest(nb_dns) ) - // dr stored in nb_dns cookie and deleted later when results available. - return true; - - reporter->Warning("can't issue DNS request"); - delete dr; - return false; - } - void DNS_Mgr::IssueAsyncRequests() { - while ( asyncs_queued.size() && asyncs_pending < MAX_PENDING_REQUESTS ) + while ( ! asyncs_queued.empty() && asyncs_pending < MAX_PENDING_REQUESTS ) { AsyncRequest* req = asyncs_queued.front(); asyncs_queued.pop_front(); ++num_requests; - - bool success; + req->time = util::current_time(); if ( req->IsAddrReq() ) - success = DoRequest(nb_dns, new DNS_Mgr_Request(req->host)); + { + auto* m_req = new DNS_Mgr_Request(req->host); + m_req->MakeRequest(channel); + } else if ( req->is_txt ) - success = DoRequest(nb_dns, - new DNS_Mgr_Request(req->name.c_str(), AF_INET, req->is_txt)); + { + auto* m_req = new DNS_Mgr_Request(req->name.c_str(), AF_INET, req->is_txt); + m_req->MakeRequest(channel); + } else { // If only one request type succeeds, don't consider it a failure. - success = DoRequest(nb_dns, - new DNS_Mgr_Request(req->name.c_str(), AF_INET, req->is_txt)); - success = DoRequest(nb_dns, - new DNS_Mgr_Request(req->name.c_str(), AF_INET6, req->is_txt)) || - success; + auto* m_req4 = new DNS_Mgr_Request(req->name.c_str(), AF_INET, req->is_txt); + m_req4->MakeRequest(channel); + auto* m_req6 = new DNS_Mgr_Request(req->name.c_str(), AF_INET6, req->is_txt); + m_req6->MakeRequest(channel); } - if ( ! success ) - { - req->Timeout(); - ++failed; - continue; - } - - req->time = util::current_time(); asyncs_timeouts.push(req); ++asyncs_pending; @@ -1088,10 +1116,7 @@ double DNS_Mgr::GetNextTimeout() void DNS_Mgr::Process() { - if ( ! nb_dns ) - return; - - while ( asyncs_timeouts.size() > 0 ) + while ( ! asyncs_timeouts.empty() ) { AsyncRequest* req = asyncs_timeouts.top(); @@ -1112,88 +1137,51 @@ void DNS_Mgr::Process() delete req; } - if ( AnswerAvailable(0) <= 0 ) - return; + Resolve(); + // TODO: what does the rest below do? + /* char err[NB_DNS_ERRSIZE]; struct nb_dns_result r; int status = nb_dns_activity(nb_dns, &r, err); if ( status < 0 ) - reporter->Warning("NB-DNS error in DNS_Mgr::Process (%s)", err); + reporter->Warning("NB-DNS error in DNS_Mgr::Process (%s)", err); else if ( status > 0 ) - { - DNS_Mgr_Request* dr = (DNS_Mgr_Request*)r.cookie; + { + DNS_Mgr_Request* dr = (DNS_Mgr_Request*)r.cookie; - bool do_host_timeout = true; - if ( dr->ReqHost() && host_mappings.find(dr->ReqHost()) == host_mappings.end() ) - // Don't timeout when this is the first result in an expected pair - // (one result each for A and AAAA queries). - do_host_timeout = false; + bool do_host_timeout = true; + if ( dr->ReqHost() && host_mappings.find(dr->ReqHost()) == host_mappings.end() ) + // Don't timeout when this is the first result in an expected pair + // (one result each for A and AAAA queries). + do_host_timeout = false; - if ( dr->RequestPending() ) - { - AddResult(dr, &r); - dr->RequestDone(); - } + if ( dr->RequestPending() ) + { + AddResult(dr, &r); + dr->RequestDone(); + } - if ( ! dr->ReqHost() ) - CheckAsyncAddrRequest(dr->ReqAddr(), true); - else if ( dr->ReqIsTxt() ) - CheckAsyncTextRequest(dr->ReqHost(), do_host_timeout); - else - CheckAsyncHostRequest(dr->ReqHost(), do_host_timeout); + if ( ! dr->ReqHost() ) + CheckAsyncAddrRequest(dr->ReqAddr(), true); + else if ( dr->ReqIsTxt() ) + CheckAsyncTextRequest(dr->ReqHost(), do_host_timeout); + else + CheckAsyncHostRequest(dr->ReqHost(), do_host_timeout); - IssueAsyncRequests(); + IssueAsyncRequests(); - delete dr; - } - } - -int DNS_Mgr::AnswerAvailable(int timeout) - { - if ( ! nb_dns ) - return -1; - - int fd = nb_dns_fd(nb_dns); - if ( fd < 0 ) - { - reporter->Warning("nb_dns_fd() failed in DNS_Mgr::WaitForReplies"); - return -1; - } - - fd_set read_fds; - - FD_ZERO(&read_fds); - FD_SET(fd, &read_fds); - - struct timeval t; - t.tv_sec = timeout; - t.tv_usec = 0; - - int status = select(fd + 1, &read_fds, 0, 0, &t); - - if ( status < 0 ) - { - if ( errno != EINTR ) - reporter->Warning("problem with DNS select"); - - return -1; - } - - if ( status > 1 ) - { - reporter->Warning("strange return from DNS select"); - return -1; - } - - return status; + delete dr; + } + */ } void DNS_Mgr::GetStats(Stats* stats) { + // TODO: can this use the telemetry framework? stats->requests = num_requests; stats->successful = successful; stats->failed = failed; @@ -1203,12 +1191,6 @@ void DNS_Mgr::GetStats(Stats* stats) stats->cached_texts = text_mappings.size(); } -void DNS_Mgr::Terminate() - { - if ( nb_dns ) - iosource_mgr->UnregisterFd(nb_dns_fd(nb_dns), this); - } - void DNS_Mgr::TestProcess() { // Only allow usage of this method when running unit tests. @@ -1218,11 +1200,11 @@ void DNS_Mgr::TestProcess() void DNS_Mgr::AsyncRequest::Resolved(const char* name) { - for ( CallbackList::iterator i = callbacks.begin(); i != callbacks.end(); ++i ) + for ( const auto& cb : callbacks ) { - (*i)->Resolved(name); + cb->Resolved(name); if ( ! doctest::is_running_in_test ) - delete *i; + delete cb; } callbacks.clear(); @@ -1231,11 +1213,11 @@ void DNS_Mgr::AsyncRequest::Resolved(const char* name) void DNS_Mgr::AsyncRequest::Resolved(TableVal* addrs) { - for ( CallbackList::iterator i = callbacks.begin(); i != callbacks.end(); ++i ) + for ( const auto& cb : callbacks ) { - (*i)->Resolved(addrs); + cb->Resolved(addrs); if ( ! doctest::is_running_in_test ) - delete *i; + delete cb; } callbacks.clear(); @@ -1244,11 +1226,11 @@ void DNS_Mgr::AsyncRequest::Resolved(TableVal* addrs) void DNS_Mgr::AsyncRequest::Timeout() { - for ( CallbackList::iterator i = callbacks.begin(); i != callbacks.end(); ++i ) + for ( const auto& cb : callbacks ) { - (*i)->Timeout(); + cb->Timeout(); if ( ! doctest::is_running_in_test ) - delete *i; + delete cb; } callbacks.clear(); @@ -1257,6 +1239,8 @@ void DNS_Mgr::AsyncRequest::Timeout() TableValPtr DNS_Mgr::empty_addr_set() { + // TODO: can this be returned statically as well? Does the result get used in a way + // that would modify the same value being returned repeatedly? auto addr_t = base_type(TYPE_ADDR); auto set_index = make_intrusive(addr_t); set_index->Append(std::move(addr_t)); @@ -1333,7 +1317,6 @@ TEST_CASE("dns_mgr prime,save,load") auto addr_result = mgr.LookupAddr(ones); CHECK(strcmp(addr_result->CheckString(), "") == 0); - mgr.Verify(); mgr.Resolve(); // Save off the resulting values from Resolve() into a file on disk @@ -1373,7 +1356,6 @@ TEST_CASE("dns_mgr alternate server") // DNS_Mgr mgr2(DNS_DEFAULT, true); // mgr2.InitPostScript(); // result = mgr2.LookupAddr("1.1.1.1"); - // mgr2.Verify(); // mgr2.Resolve(); // result = mgr2.LookupAddr("1.1.1.1"); diff --git a/src/DNS_Mgr.h b/src/DNS_Mgr.h index a8c4f6f048..e5bfba9689 100644 --- a/src/DNS_Mgr.h +++ b/src/DNS_Mgr.h @@ -2,6 +2,7 @@ #pragma once +#include #include #include #include @@ -15,9 +16,6 @@ namespace zeek { - -class EventHandler; -class RecordType; class Val; class ListVal; class TableVal; @@ -31,16 +29,9 @@ using StringValPtr = IntrusivePtr; } // namespace zeek -// Defined in nb_dns.h -struct nb_dns_info; -struct nb_dns_result; - namespace zeek::detail { - class DNS_Mgr_Request; -using DNS_mgr_request_list = PList; - class DNS_Mapping; enum DNS_MgrMode @@ -51,49 +42,111 @@ enum DNS_MgrMode DNS_FAKE, // don't look up names, just return dummy results }; -// Number of seconds we'll wait for a reply. -#define DNS_TIMEOUT 5 - class DNS_Mgr final : public iosource::IOSource { public: explicit DNS_Mgr(DNS_MgrMode mode); ~DNS_Mgr() override; + /** + * Finalizes the manager initialization. This should be called only after all + * of the scripts have been parsed at startup. + */ void InitPostScript(); + + /** + * Attempts to process one more round of requests and then flushes the + * mapping caches. + */ void Flush(); - // Looks up the address or addresses of the given host, and returns - // a set of addr. + /** + * Looks up the address(es) of a given host and returns a set of addr. + * This is a synchronous method and will block until results are ready. + * + * @param host The host name to look up an address for. + * @return A set of addresses. + */ TableValPtr LookupHost(const char* host); + /** + * Looks up the hostname of a given address. This is a synchronous method + * and will block until results are ready. + * + * @param host The addr to lookup a hostname for. + * @return The hostname. + */ StringValPtr LookupAddr(const IPAddr& addr); - // Define the directory where to store the data. - void SetDir(const char* arg_dir) { dir = util::copy_string(arg_dir); } + /** + * Sets the directory where to store DNS data when Save() is called. + */ + void SetDir(const char* arg_dir) { dir = arg_dir; } - void Verify(); + /** + * Waits for responses to become available or a timeout to occur, + * and handles any responses. + */ void Resolve(); + + /** + * Saves the current name and address caches to disk. + */ bool Save(); - const char* LookupAddrInCache(const IPAddr& addr); - TableValPtr LookupNameInCache(const std::string& name); - const char* LookupTextInCache(const std::string& name); - - // Support for async lookups. + /** + * Base class for callback handling for asynchronous lookups. + */ class LookupCallback { public: - LookupCallback() { } - virtual ~LookupCallback() { } + virtual ~LookupCallback() = default; + /** + * Called when an address lookup finishes. + * + * @param name The resulting name from the lookup. + */ virtual void Resolved(const char* name){}; + + /** + * Called when a name lookup finishes. + * + * @param addrs A table of the resulting addresses from the lookup. + */ virtual void Resolved(TableVal* addrs){}; + + /** + * Called when a timeout request occurs. + */ virtual void Timeout() = 0; }; + /** + * Schedules an asynchronous request to lookup a hostname for an IP address. + * This is the equivalent of an "A" or "AAAA" request, depending on if the + * address is ipv4 or ipv6. + * + * @param host The address to lookup names for. + * @param callback A callback object to call when the request completes. + */ void AsyncLookupAddr(const IPAddr& host, LookupCallback* callback); + + /** + * Schedules an asynchronous request to lookup an address for a hostname. + * This is the equivalent of a "PTR" request. + * + * @param host The hostname to look up addresses for. + * @param callback A callback object to call when the request completes. + */ void AsyncLookupName(const std::string& name, LookupCallback* callback); + + /** + * Schedules an asynchronous TXT request for a hostname. + * + * @param host The address to lookup names for. + * @param callback A callback object to call when the request completes. + */ void AsyncLookupNameText(const std::string& name, LookupCallback* callback); struct Stats @@ -107,10 +160,27 @@ public: unsigned long cached_texts; }; + /** + * Returns the current statistics for the DNS_Manager. + * + * @param stats A pointer to a stats object to return the data in. + */ void GetStats(Stats* stats); - void Terminate(); + /** + * Adds a result from a request to the caches. This is public so that the + * callback methods can call it from outside of the DNS_Mgr class. + * + * @param dr The request associated with the result. + * @param h A hostent structure containing the actual result data. + * @param ttl A ttl value contained in the response from the server. + */ + void AddResult(DNS_Mgr_Request* dr, struct hostent* h, uint32_t ttl); + /** + * Returns an empty set of addresses, used in various error cases and during + * cache priming. + */ static TableValPtr empty_addr_set(); /** @@ -123,13 +193,16 @@ protected: friend class LookupCallback; friend class DNS_Mgr_Request; + const char* LookupAddrInCache(const IPAddr& addr); + TableValPtr LookupNameInCache(const std::string& name); + const char* LookupTextInCache(const std::string& name); + void Event(EventHandlerPtr e, DNS_Mapping* dm); void Event(EventHandlerPtr e, DNS_Mapping* dm, ListValPtr l1, ListValPtr l2); void Event(EventHandlerPtr e, DNS_Mapping* old_dm, DNS_Mapping* new_dm); ValPtr BuildMappingVal(DNS_Mapping* dm); - void AddResult(DNS_Mgr_Request* dr, struct nb_dns_result* r); void CompareMappings(DNS_Mapping* prev_dm, DNS_Mapping* new_dm); ListValPtr AddrListDelta(ListVal* al1, ListVal* al2); void DumpAddrList(FILE* f, ListVal* al); @@ -137,15 +210,10 @@ protected: using HostMap = std::map>; using AddrMap = std::map; using TextMap = std::map; - void LoadCache(FILE* f); + void LoadCache(const std::string& path); void Save(FILE* f, const AddrMap& m); void Save(FILE* f, const HostMap& m); - // Selects on the fd to see if there is an answer available (timeout - // is secs). Returns 0 on timeout, -1 on EINTR or other error, and 1 - // if answer is ready. - int AnswerAvailable(int timeout); - // Issue as many queued async requests as slots are available. void IssueAsyncRequests(); @@ -167,29 +235,30 @@ protected: AddrMap addr_mappings; TextMap text_mappings; + using DNS_mgr_request_list = PList; DNS_mgr_request_list requests; - nb_dns_info* nb_dns; - char* cache_name; - char* dir; // directory in which cache_name resides + std::string cache_name; + std::string dir; // directory in which cache_name resides - bool did_init; - int asyncs_pending; + bool did_init = false; + int asyncs_pending = 0; RecordTypePtr dm_rec; + ares_channel channel; + bool ipv6_resolver = false; + using CallbackList = std::list; struct AsyncRequest { - double time; + double time = 0.0; IPAddr host; std::string name; CallbackList callbacks; - bool is_txt; - bool processed; - - AsyncRequest() : time(0.0), is_txt(false), processed(false) { } + bool is_txt = false; + bool processed = false; bool IsAddrReq() const { return name.empty(); } diff --git a/src/zeek-setup.cc b/src/zeek-setup.cc index 66a23b5aa7..551dd003fc 100644 --- a/src/zeek-setup.cc +++ b/src/zeek-setup.cc @@ -18,6 +18,7 @@ #include "zeek/3rdparty/sqlite3.h" #define DOCTEST_CONFIG_IMPLEMENT + #include "zeek/3rdparty/doctest.h" #include "zeek/Anon.h" #include "zeek/DFA.h" @@ -330,7 +331,6 @@ static void terminate_zeek() input_mgr->Terminate(); thread_mgr->Terminate(); broker_mgr->Terminate(); - dns_mgr->Terminate(); event_mgr.Drain(); @@ -342,6 +342,7 @@ static void terminate_zeek() delete packet_mgr; delete analyzer_mgr; delete file_mgr; + delete dns_mgr; // broker_mgr, timer_mgr, and supervisor are deleted via iosource_mgr delete iosource_mgr; delete event_registry; @@ -460,6 +461,8 @@ SetupResult setup(int argc, char** argv, Options* zopts) if ( dns_type == DNS_DEFAULT && fake_dns() ) dns_type = DNS_FAKE; + dns_mgr = new DNS_Mgr(dns_type); + RETSIGTYPE (*oldhandler)(int); zeek_script_prefixes = options.script_prefixes; @@ -599,8 +602,6 @@ SetupResult setup(int argc, char** argv, Options* zopts) push_scope(nullptr, nullptr); - dns_mgr = new DNS_Mgr(dns_type); - // It would nice if this were configurable. This is similar to the // chicken and the egg problem. It would be configurable by parsing // policy, but we can't parse policy without DNS resolution. @@ -756,6 +757,9 @@ SetupResult setup(int argc, char** argv, Options* zopts) file_mgr->InitPostScript(); dns_mgr->InitPostScript(); + dns_mgr->LookupHost("www.apple.com"); + // dns_mgr->LookupAddr("17.253.144.10"); + #ifdef USE_PERFTOOLS_DEBUG } #endif @@ -859,9 +863,12 @@ SetupResult setup(int argc, char** argv, Options* zopts) if ( (oldhandler = setsignal(SIGHUP, sig_handler)) != SIG_DFL ) (void)setsignal(SIGHUP, oldhandler); + // If we were priming the DNS cache (i.e. -P was passed as an argument), flush anything + // remaining to be resolved and save the cache to disk. We can just exit now because + // we've done everything we need to do. The run loop isn't started in this case, so + // nothing else should be happening. if ( dns_type == DNS_PRIME ) { - dns_mgr->Verify(); dns_mgr->Resolve(); if ( ! dns_mgr->Save() ) diff --git a/src/zeek.bif b/src/zeek.bif index 911a010761..3fd41e0901 100644 --- a/src/zeek.bif +++ b/src/zeek.bif @@ -3657,7 +3657,7 @@ public: } // Overridden from zeek::detail::DNS_Mgr:Lookup:Callback. - virtual void Resolved(const char* name) + void Resolved(const char* name) override { zeek::Val* result = new zeek::StringVal(name); trigger->Cache(call, result); @@ -3665,14 +3665,14 @@ public: trigger->Release(); } - virtual void Resolved(zeek::TableVal* addrs) + void Resolved(zeek::TableVal* addrs) override { // No Ref() for addrs. trigger->Cache(call, addrs); trigger->Release(); } - virtual void Timeout() + void Timeout() override { if ( lookup_name ) {