From 035c4c3776ee0d04a9db97213891367b7998de55 Mon Sep 17 00:00:00 2001 From: Oliver Welsh <160491261+oliverwelsh@users.noreply.github.com> Date: Thu, 22 Feb 2024 18:34:06 +0000 Subject: [PATCH] Add flag to not use a default local named server on channel initialization (#713) Hello, I work on an application for Microsoft which uses c-ares to perform DNS lookups. We have made some minor changes to the library over time, and would like to contribute these back to the project in case they are useful more widely. This PR adds a new channel init flag, described below. Please let me know if I can include any more information to make this PR better/easier for you to review. Thanks! **Summary** When initializing a channel with `ares_init_options()`, if there are no nameservers available (because `ARES_OPT_SERVERS` is not used and `/etc/resolv.conf` is either empty or not available) then a default local named server will be added to the channel. However in some applications a local named server will never be available. In this case, all subsequent queries on the channel will fail. If we know this ahead of time, then it may be preferred to fail channel initialization directly rather than wait for the queries to fail. This gives better visibility, since we know that the failure is due to missing servers rather than something going wrong with the queries. This PR adds a new flag `ARES_FLAG_NO_DFLT_SVR`, to indicate that a default local named server should not be added to a channel in this scenario. Instead, a new error `ARES_EINITNOSERVER` is returned and initialization fails. **Testing** I have added 2 new FV tests: - `ContainerNoDfltSvrEmptyInit` to test that initialization fails when no nameservers are available and the flag is set. - `ContainerNoDfltSvrFullInit` to test that initialization still succeeds when the flag is set but other nameservers are available. Existing FVs are all passing. **Documentation** I have had a go at manually updating the docs to describe the new flag/error, but couldn't see any contributing guidance about testing this. Please let me know if you'd like anything more here. --------- Fix By: Oliver Welsh (@oliverwelsh) --- docs/ares_init_options.3 | 7 +++++++ docs/ares_query.3 | 4 ++++ docs/ares_search.3 | 3 +++ docs/ares_send.3 | 4 ++++ include/ares.h | 7 +++++-- src/lib/ares_init.c | 16 ++++++++++++---- src/lib/ares_process.c | 4 ++-- src/lib/ares_send.c | 4 ++-- src/lib/ares_strerror.c | 2 ++ test/ares-test-init.cc | 31 +++++++++++++++++++++++++++++++ test/ares-test-misc.cc | 2 +- 11 files changed, 73 insertions(+), 11 deletions(-) diff --git a/docs/ares_init_options.3 b/docs/ares_init_options.3 index a2b602a5..000cc1d9 100644 --- a/docs/ares_init_options.3 +++ b/docs/ares_init_options.3 @@ -120,6 +120,10 @@ used to test or debug name servers. .B ARES_FLAG_EDNS Include an EDNS pseudo-resource record (RFC 2671) in generated requests. As of v1.22, this is on by default if flags are otherwise not set. +.TP 23 +.B ARES_FLAG_NO_DFLT_SVR +Do not attempt to add a default local named server if there are no other +servers available. Instead, fail initialization with \fIARES_ENOSERVER\fP. .RE .TP 18 .B ARES_OPT_TIMEOUT @@ -342,6 +346,9 @@ The process's available memory was exhausted. .TP 14 .B ARES_ENOTINITIALIZED c-ares library initialization not yet performed. +.TP 14 +.B ARES_ENOSERVER +No DNS servers were available to use. .SH NOTES When initializing from .B /etc/resolv.conf, diff --git a/docs/ares_query.3 b/docs/ares_query.3 index e86bdebb..00e44f52 100644 --- a/docs/ares_query.3 +++ b/docs/ares_query.3 @@ -121,6 +121,10 @@ The query was cancelled. The name service channel .I channel is being destroyed; the query will not be completed. +.TP 19 +.B ARES_ENOSERVER +The query will not be completed because no DNS servers were configured on the +channel. .PP The callback argument .I timeouts diff --git a/docs/ares_search.3 b/docs/ares_search.3 index 570e2e23..08246d34 100644 --- a/docs/ares_search.3 +++ b/docs/ares_search.3 @@ -122,6 +122,9 @@ The query was cancelled. The name service channel .I channel is being destroyed; the query will not be completed. +.TP 19 +.B ARES_ENOSERVER +No query completed successfully; no DNS servers were configured on the channel. .PP The callback argument .I timeouts diff --git a/docs/ares_send.3 b/docs/ares_send.3 index 8126647f..1fe1c027 100644 --- a/docs/ares_send.3 +++ b/docs/ares_send.3 @@ -77,6 +77,10 @@ The query was cancelled. The name service channel .I channel is being destroyed; the query will not be completed. +.TP 19 +.B ARES_ENOSERVER +The query will not be completed because no DNS servers were configured on the +channel. .PP The callback argument .I timeouts diff --git a/include/ares.h b/include/ares.h index 45517838..acbd6583 100644 --- a/include/ares.h +++ b/include/ares.h @@ -161,8 +161,10 @@ typedef enum { ARES_ECANCELLED = 24, /* introduced in 1.7.0 */ /* More ares_getaddrinfo error codes */ - ARES_ESERVICE = 25 /* ares_getaddrinfo() was passed a text service name that - * is not recognized. introduced in 1.16.0 */ + ARES_ESERVICE = 25, /* ares_getaddrinfo() was passed a text service name that + * is not recognized. introduced in 1.16.0 */ + + ARES_ENOSERVER = 26 /* No DNS servers were configured */ } ares_status_t; typedef enum { @@ -196,6 +198,7 @@ typedef enum { #define ARES_FLAG_NOALIASES (1 << 6) #define ARES_FLAG_NOCHECKRESP (1 << 7) #define ARES_FLAG_EDNS (1 << 8) +#define ARES_FLAG_NO_DFLT_SVR (1 << 9) /* Option mask values */ #define ARES_OPT_FLAGS (1 << 0) diff --git a/src/lib/ares_init.c b/src/lib/ares_init.c index 014226f3..bae7c72f 100644 --- a/src/lib/ares_init.c +++ b/src/lib/ares_init.c @@ -133,6 +133,8 @@ static ares_status_t init_by_defaults(ares_channel_t *channel) #ifdef HAVE_GETHOSTNAME const char *dot; #endif + struct ares_addr addr; + ares__llist_t *sconfig = NULL; /* Enable EDNS by default */ if (!(channel->optmask & ARES_OPT_FLAGS)) { @@ -155,22 +157,27 @@ static ares_status_t init_by_defaults(ares_channel_t *channel) } if (ares__slist_len(channel->servers) == 0) { - struct ares_addr addr; - ares__llist_t *sconfig = NULL; + /* Add a default local named server to the channel unless configured not + * to (in which case return an error). + */ + if (channel->flags & ARES_FLAG_NO_DFLT_SVR) { + rc = ARES_ENOSERVER; + goto error; + } addr.family = AF_INET; addr.addr.addr4.s_addr = htonl(INADDR_LOOPBACK); rc = ares__sconfig_append(&sconfig, &addr, 0, 0, NULL); if (rc != ARES_SUCCESS) { - return rc; + goto error; } rc = ares__servers_update(channel, sconfig, ARES_FALSE); ares__llist_destroy(sconfig); if (rc != ARES_SUCCESS) { - return rc; + goto error; } } @@ -387,6 +394,7 @@ int ares_init_options(ares_channel_t **channelptr, if (status != ARES_SUCCESS) { DEBUGF(fprintf(stderr, "Error: init_by_defaults failed: %s\n", ares_strerror(status))); + goto done; } /* Initialize the event thread */ diff --git a/src/lib/ares_process.c b/src/lib/ares_process.c index 6361e00d..bd84d09e 100644 --- a/src/lib/ares_process.c +++ b/src/lib/ares_process.c @@ -893,8 +893,8 @@ ares_status_t ares__send_query(struct query *query, struct timeval *now) } if (server == NULL) { - end_query(channel, query, ARES_ESERVFAIL /* ? */, NULL, 0); - return ARES_ECONNREFUSED; + end_query(channel, query, ARES_ENOSERVER /* ? */, NULL, 0); + return ARES_ENOSERVER; } if (query->using_tcp) { diff --git a/src/lib/ares_send.c b/src/lib/ares_send.c index 642c1507..6cefdb6a 100644 --- a/src/lib/ares_send.c +++ b/src/lib/ares_send.c @@ -66,8 +66,8 @@ ares_status_t ares_send_ex(ares_channel_t *channel, const unsigned char *qbuf, return ARES_EBADQUERY; } if (ares__slist_len(channel->servers) == 0) { - callback(arg, ARES_ESERVFAIL, 0, NULL, 0); - return ARES_ESERVFAIL; + callback(arg, ARES_ENOSERVER, 0, NULL, 0); + return ARES_ENOSERVER; } /* Check query cache */ diff --git a/src/lib/ares_strerror.c b/src/lib/ares_strerror.c index fd583c5c..ae94f961 100644 --- a/src/lib/ares_strerror.c +++ b/src/lib/ares_strerror.c @@ -85,6 +85,8 @@ const char *ares_strerror(int code) return "DNS query cancelled"; case ARES_ESERVICE: return "Invalid service name or number"; + case ARES_ENOSERVER: + return "No DNS servers were configured"; } return "unknown"; diff --git a/test/ares-test-init.cc b/test/ares-test-init.cc index 19c99f40..8d87d7d5 100644 --- a/test/ares-test-init.cc +++ b/test/ares-test-init.cc @@ -713,6 +713,37 @@ CONTAINED_TEST_F(LibraryTest, ContainerEmptyInit, return HasFailure(); } +// Test that init fails if the flag to not use a default local named server is +// enabled and no other nameservers are available. +CONTAINED_TEST_F(LibraryTest, ContainerNoDfltSvrEmptyInit, + "myhostname", "mydomainname.org", empty) { + ares_channel_t *channel = nullptr; + struct ares_options opts = {0}; + int optmask = ARES_OPT_FLAGS; + opts.flags = ARES_FLAG_NO_DFLT_SVR; + EXPECT_EQ(ARES_ENOSERVER, ares_init_options(&channel, &opts, optmask)); + + EXPECT_EQ(nullptr, channel); + return HasFailure(); +} +// Test that init succeeds if the flag to not use a default local named server +// is enabled but other nameservers are available. +CONTAINED_TEST_F(LibraryTest, ContainerNoDfltSvrFullInit, + "myhostname", "mydomainname.org", filelist) { + ares_channel_t *channel = nullptr; + struct ares_options opts = {0}; + int optmask = ARES_OPT_FLAGS; + opts.flags = ARES_FLAG_NO_DFLT_SVR; + EXPECT_EQ(ARES_SUCCESS, ares_init_options(&channel, &opts, optmask)); + + std::string actual = GetNameServers(channel); + std::string expected = "1.2.3.4:53"; + EXPECT_EQ(expected, actual); + + ares_destroy(channel); + return HasFailure(); +} + #endif } // namespace test diff --git a/test/ares-test-misc.cc b/test/ares-test-misc.cc index a9bf4088..e680481a 100644 --- a/test/ares-test-misc.cc +++ b/test/ares-test-misc.cc @@ -64,7 +64,7 @@ TEST_F(DefaultChannelTest, SetServers) { ares_gethostbyname(channel_, "www.google.com.", AF_INET, HostCallback, &result); Process(); EXPECT_TRUE(result.done_); - EXPECT_EQ(ARES_ESERVFAIL, result.status_); + EXPECT_EQ(ARES_ENOSERVER, result.status_); struct ares_addr_node server1;