Fix using EDNS0 when no private DNS validated
Do not add NET_CONTEXT_FLAG_USE_EDNS to android_net_context flag
if there is no private DNS server validated.
A server not supporting EDNS might respond with FORMERR or ignore
the request. In former case, we will remove OPT RR and retry again.
In later case, we will not retry, so the DNS query might timeout.
Also fix the bug which dns_responder responds to the query even
though it's been set unresponsive.
Bug: 120257033
Test: system/netd/tests/runtests.sh passed
Checked packets not containging OPT RR when using cleartext DNS
Change-Id: I8250a800ddade0ff810445bc912ea5799b99ec8c
diff --git a/resolv/dns_responder/dns_responder.cpp b/resolv/dns_responder/dns_responder.cpp
index 4df737f..db91875 100644
--- a/resolv/dns_responder/dns_responder.cpp
+++ b/resolv/dns_responder/dns_responder.cpp
@@ -563,6 +563,10 @@
response_probability_ = response_probability;
}
+void DNSResponder::setEdns(Edns edns) {
+ edns_ = edns;
+}
+
bool DNSResponder::running() const {
return socket_ != -1;
}
@@ -763,11 +767,15 @@
return makeErrorResponse(&header, ns_rcode::ns_r_formerr, response,
response_len);
}
- if (!header.additionals.empty() && fail_on_edns_) {
+ if (!header.additionals.empty() && edns_ != Edns::ON) {
ALOGI("DNS request has an additional section (assumed EDNS). "
- "Simulating an ancient (pre-EDNS) server.");
- return makeErrorResponse(&header, ns_rcode::ns_r_formerr, response,
- response_len);
+ "Simulating an ancient (pre-EDNS) server, and returning %s",
+ edns_ == Edns::FORMERR ? "RCODE FORMERR." : "no response.");
+ if (edns_ == Edns::FORMERR) {
+ return makeErrorResponse(&header, ns_rcode::ns_r_formerr, response, response_len);
+ }
+ // No response.
+ return false;
}
{
std::lock_guard lock(queries_mutex_);
@@ -782,7 +790,7 @@
if (arc4random_uniform(bound) > bound * response_probability_) {
if (error_rcode_ < 0) {
ALOGI("Returning no response");
- return true;
+ return false;
} else {
ALOGI("returning RCODE %d in accordance with probability distribution",
static_cast<int>(error_rcode_));
diff --git a/resolv/dns_responder/dns_responder.h b/resolv/dns_responder/dns_responder.h
index e3da72d..2b6b835 100644
--- a/resolv/dns_responder/dns_responder.h
+++ b/resolv/dns_responder/dns_responder.h
@@ -45,10 +45,17 @@
DNSResponder(std::string listen_address, std::string listen_service, int poll_timeout_ms,
ns_rcode error_rcode);
~DNSResponder();
+
+ enum class Edns : uint8_t {
+ ON,
+ FORMERR, // DNS server not supporting EDNS will reply FORMERR.
+ DROP // DNS server not supporting EDNS will not do any response.
+ };
+
void addMapping(const std::string& name, ns_type type, const std::string& addr);
void removeMapping(const std::string& name, ns_type type);
void setResponseProbability(double response_probability);
- void setFailOnEdns(bool fail) { fail_on_edns_ = fail; }
+ void setEdns(Edns edns);
bool running() const;
bool startServer();
bool stopServer();
@@ -86,11 +93,11 @@
}
};
- // DNS request handler.
void requestHandler();
// Parses and generates a response message for incoming DNS requests.
- // Returns false on parsing errors.
+ // Returns false to ignore the request, which might be due to either parsing error
+ // or unresponsiveness.
bool handleDNSRequest(const char* buffer, ssize_t buffer_len,
char* response, size_t* response_len) const;
@@ -114,9 +121,12 @@
// instead of returning error_rcode_.
std::atomic<double> response_probability_ = 1.0;
- // If true, behave like an old DNS server that doesn't support EDNS.
- // Default false.
- std::atomic<bool> fail_on_edns_ = false;
+ // Control how the DNS server behaves when it receives the requests containing OPT RR.
+ // If it's set Edns::ON, the server can recognize and reply the response; if it's set
+ // Edns::FORMERR, the server behaves like an old DNS server that doesn't support EDNS0, and
+ // replying FORMERR; if it's Edns::DROP, the server doesn't support EDNS0 either, and ignoring
+ // the requests.
+ std::atomic<Edns> edns_ = Edns::ON;
// Mappings from (name, type) to registered response and the
// mutex protecting them.
diff --git a/resolv/res_send.cpp b/resolv/res_send.cpp
index 5d165f2..e3e62e9 100644
--- a/resolv/res_send.cpp
+++ b/resolv/res_send.cpp
@@ -1251,6 +1251,15 @@
// Sleep and iterate some small number of times checking for the
// arrival of resolved and validated server IP addresses, instead
// of returning an immediate error.
+ // This is needed because as soon as a network becomes the default network, apps will
+ // send DNS queries on that network. If no servers have yet validated, and we do not
+ // block those queries, they would immediately fail, causing application-visible errors.
+ // Note that this can happen even before the network validates, since an unvalidated
+ // network can become the default network if no validated networks are available.
+ //
+ // TODO: see if there is a better way to address this problem, such as buffering the
+ // queries in a queue or only blocking queries for the first few seconds after a default
+ // network change.
for (int i = 0; i < 42; i++) {
std::this_thread::sleep_for(std::chrono::milliseconds(100));
if (!gPrivateDnsConfiguration.getStatus(netId).validatedServers.empty()) {
diff --git a/resolv/resolver_test.cpp b/resolv/resolver_test.cpp
index bab7e22..2c2771e 100644
--- a/resolv/resolver_test.cpp
+++ b/resolv/resolver_test.cpp
@@ -584,47 +584,6 @@
EXPECT_EQ(addr_ip6, ToString(result));
}
-TEST_F(ResolverTest, GetHostByNameBrokenEdns) {
- const char* listen_addr = "127.0.0.3";
- const char* listen_srv = "53";
- const char* host_name = "edns.example.com.";
- test::DNSResponder dns(listen_addr, listen_srv, 250, ns_rcode::ns_r_servfail);
- dns.addMapping(host_name, ns_type::ns_t_a, "1.2.3.3");
- dns.setFailOnEdns(true); // This is the only change from the basic test.
- ASSERT_TRUE(dns.startServer());
- std::vector<std::string> servers = { listen_addr };
- ASSERT_TRUE(SetResolversForNetwork(servers, mDefaultSearchDomains, mDefaultParams_Binder));
-
- const hostent* result;
-
- dns.clearQueries();
- result = gethostbyname("edns");
- EXPECT_EQ(1U, GetNumQueriesForType(dns, ns_type::ns_t_a, host_name));
- ASSERT_FALSE(result == nullptr);
- ASSERT_EQ(4, result->h_length);
- ASSERT_FALSE(result->h_addr_list[0] == nullptr);
- EXPECT_EQ("1.2.3.3", ToString(result));
- EXPECT_TRUE(result->h_addr_list[1] == nullptr);
-}
-
-TEST_F(ResolverTest, GetAddrInfoBrokenEdns) {
- const char* listen_addr = "127.0.0.5";
- const char* listen_srv = "53";
- const char* host_name = "edns2.example.com.";
- test::DNSResponder dns(listen_addr, listen_srv, 250, ns_rcode::ns_r_servfail);
- dns.addMapping(host_name, ns_type::ns_t_a, "1.2.3.5");
- dns.setFailOnEdns(true); // This is the only change from the basic test.
- ASSERT_TRUE(dns.startServer());
- std::vector<std::string> servers = { listen_addr };
- ASSERT_TRUE(SetResolversForNetwork(servers, mDefaultSearchDomains, mDefaultParams_Binder));
-
- addrinfo hints = {.ai_family = AF_INET};
- ScopedAddrinfo result = safe_getaddrinfo("edns2", nullptr, &hints);
- EXPECT_TRUE(result != nullptr);
- EXPECT_EQ(1U, GetNumQueries(dns, host_name));
- EXPECT_EQ("1.2.3.5", ToString(result));
-}
-
TEST_F(ResolverTest, MultidomainResolution) {
std::vector<std::string> searchDomains = { "example1.com", "example2.com", "example3.com" };
const char* listen_addr = "127.0.0.6";
@@ -1831,4 +1790,152 @@
u_char buf[MAXPACKET] = {};
rc = getAsyncResponse(fd, &rcode, buf, MAXPACKET);
EXPECT_EQ("1.2.3.4", toString(buf, rc, AF_INET));
-}
\ No newline at end of file
+}
+
+// This test checks that the resolver should not generate the request containing OPT RR when using
+// cleartext DNS. If we query the DNS server not supporting EDNS0 and it reponds with FORMERR, we
+// will fallback to no EDNS0 and try again. If the server does no response, we won't retry so that
+// we get no answer.
+TEST_F(ResolverTest, BrokenEdns) {
+ typedef test::DNSResponder::Edns Edns;
+ enum ExpectResult { EXPECT_FAILURE, EXPECT_SUCCESS };
+
+ const char OFF[] = "off";
+ const char OPPORTUNISTIC_UDP[] = "opportunistic_udp";
+ const char OPPORTUNISTIC_TLS[] = "opportunistic_tls";
+ const char STRICT[] = "strict";
+ const char GETHOSTBYNAME[] = "gethostbyname";
+ const char GETADDRINFO[] = "getaddrinfo";
+ const std::vector<uint8_t> NOOP_FINGERPRINT(SHA256_SIZE, 0U);
+ const char ADDR4[] = "192.0.2.1";
+ const char CLEARTEXT_ADDR[] = "127.0.0.53";
+ const char CLEARTEXT_PORT[] = "53";
+ const char TLS_PORT[] = "853";
+ const std::vector<std::string> servers = { CLEARTEXT_ADDR };
+
+ test::DNSResponder dns(CLEARTEXT_ADDR, CLEARTEXT_PORT, 250, ns_rcode::ns_r_servfail);
+ ASSERT_TRUE(dns.startServer());
+
+ test::DnsTlsFrontend tls(CLEARTEXT_ADDR, TLS_PORT, CLEARTEXT_ADDR, CLEARTEXT_PORT);
+
+ static const struct TestConfig {
+ std::string mode;
+ std::string method;
+ Edns edns;
+ ExpectResult expectResult;
+
+ std::string asHostName() const {
+ const char* ednsString;
+ switch (edns) {
+ case Edns::ON:
+ ednsString = "ednsOn";
+ break;
+ case Edns::FORMERR:
+ ednsString = "ednsFormerr";
+ break;
+ case Edns::DROP:
+ ednsString = "ednsDrop";
+ break;
+ default:
+ ednsString = "";
+ break;
+ }
+ return StringPrintf("%s.%s.%s.", mode.c_str(), method.c_str(), ednsString);
+ }
+ } testConfigs[] = {
+ // In OPPORTUNISTIC_TLS, we get no answer if the DNS server supports TLS but not EDNS0.
+ // Could such server exist? if so, we might need to fallback to query cleartext DNS.
+ // Another thing is that {OPPORTUNISTIC_TLS, Edns::DROP} and {STRICT, Edns::DROP} are
+ // commented out since TLS timeout is not configurable.
+ // TODO: Uncomment them after TLS timeout is configurable.
+ {OFF, GETHOSTBYNAME, Edns::ON, EXPECT_SUCCESS},
+ {OPPORTUNISTIC_UDP, GETHOSTBYNAME, Edns::ON, EXPECT_SUCCESS},
+ {OPPORTUNISTIC_TLS, GETHOSTBYNAME, Edns::ON, EXPECT_SUCCESS},
+ {STRICT, GETHOSTBYNAME, Edns::ON, EXPECT_SUCCESS},
+ {OFF, GETHOSTBYNAME, Edns::FORMERR, EXPECT_SUCCESS},
+ {OPPORTUNISTIC_UDP, GETHOSTBYNAME, Edns::FORMERR, EXPECT_SUCCESS},
+ {OPPORTUNISTIC_TLS, GETHOSTBYNAME, Edns::FORMERR, EXPECT_FAILURE},
+ {STRICT, GETHOSTBYNAME, Edns::FORMERR, EXPECT_FAILURE},
+ {OFF, GETHOSTBYNAME, Edns::DROP, EXPECT_SUCCESS},
+ {OPPORTUNISTIC_UDP, GETHOSTBYNAME, Edns::DROP, EXPECT_SUCCESS},
+ //{OPPORTUNISTIC_TLS, GETHOSTBYNAME, Edns::DROP, EXPECT_FAILURE},
+ //{STRICT, GETHOSTBYNAME, Edns::DROP, EXPECT_FAILURE},
+ {OFF, GETADDRINFO, Edns::ON, EXPECT_SUCCESS},
+ {OPPORTUNISTIC_UDP, GETADDRINFO, Edns::ON, EXPECT_SUCCESS},
+ {OPPORTUNISTIC_TLS, GETADDRINFO, Edns::ON, EXPECT_SUCCESS},
+ {STRICT, GETADDRINFO, Edns::ON, EXPECT_SUCCESS},
+ {OFF, GETADDRINFO, Edns::FORMERR, EXPECT_SUCCESS},
+ {OPPORTUNISTIC_UDP, GETADDRINFO, Edns::FORMERR, EXPECT_SUCCESS},
+ {OPPORTUNISTIC_TLS, GETADDRINFO, Edns::FORMERR, EXPECT_FAILURE},
+ {STRICT, GETADDRINFO, Edns::FORMERR, EXPECT_FAILURE},
+ {OFF, GETADDRINFO, Edns::DROP, EXPECT_SUCCESS},
+ {OPPORTUNISTIC_UDP, GETADDRINFO, Edns::DROP, EXPECT_SUCCESS},
+ //{OPPORTUNISTIC_TLS, GETADDRINFO, Edns::DROP, EXPECT_FAILURE},
+ //{STRICT, GETADDRINFO, Edns::DROP, EXPECT_FAILURE},
+ };
+
+ for (const auto& config : testConfigs) {
+ const std::string testHostName = config.asHostName();
+ SCOPED_TRACE(testHostName);
+
+ const char* host_name = testHostName.c_str();
+ dns.addMapping(host_name, ns_type::ns_t_a, ADDR4);
+ dns.setEdns(config.edns);
+
+ if (config.mode == OFF) {
+ ASSERT_TRUE(
+ SetResolversForNetwork(servers, mDefaultSearchDomains, mDefaultParams_Binder));
+ } else if (config.mode == OPPORTUNISTIC_UDP) {
+ ASSERT_TRUE(SetResolversWithTls(servers, mDefaultSearchDomains, mDefaultParams_Binder,
+ "", {}));
+ } else if (config.mode == OPPORTUNISTIC_TLS) {
+ ASSERT_TRUE(tls.startServer());
+ ASSERT_TRUE(SetResolversWithTls(servers, mDefaultSearchDomains, mDefaultParams_Binder,
+ "", {}));
+ // Wait for validation to complete.
+ EXPECT_TRUE(tls.waitForQueries(1, 5000));
+ } else if (config.mode == STRICT) {
+ ASSERT_TRUE(tls.startServer());
+ ASSERT_TRUE(SetResolversWithTls(servers, mDefaultSearchDomains, mDefaultParams_Binder,
+ "", {base64Encode(tls.fingerprint())}));
+ // Wait for validation to complete.
+ EXPECT_TRUE(tls.waitForQueries(1, 5000));
+ }
+
+ if (config.method == GETHOSTBYNAME) {
+ const hostent* h_result = gethostbyname(host_name);
+ if (config.expectResult == EXPECT_SUCCESS) {
+ EXPECT_LE(1U, GetNumQueries(dns, host_name));
+ ASSERT_TRUE(h_result != nullptr);
+ ASSERT_EQ(4, h_result->h_length);
+ ASSERT_FALSE(h_result->h_addr_list[0] == nullptr);
+ EXPECT_EQ(ADDR4, ToString(h_result));
+ EXPECT_TRUE(h_result->h_addr_list[1] == nullptr);
+ } else {
+ EXPECT_EQ(0U, GetNumQueriesForType(dns, ns_type::ns_t_a, host_name));
+ ASSERT_TRUE(h_result == nullptr);
+ ASSERT_EQ(HOST_NOT_FOUND, h_errno);
+ }
+ } else if (config.method == GETADDRINFO) {
+ ScopedAddrinfo ai_result;
+ addrinfo hints = {.ai_family = AF_INET, .ai_socktype = SOCK_DGRAM};
+ ai_result = safe_getaddrinfo(host_name, nullptr, &hints);
+ if (config.expectResult == EXPECT_SUCCESS) {
+ EXPECT_TRUE(ai_result != nullptr);
+ EXPECT_EQ(1U, GetNumQueries(dns, host_name));
+ const std::string result_str = ToString(ai_result);
+ EXPECT_EQ(ADDR4, result_str);
+ } else {
+ EXPECT_TRUE(ai_result == nullptr);
+ EXPECT_EQ(0U, GetNumQueries(dns, host_name));
+ }
+ } else {
+ FAIL() << "Unsupported query method: " << config.method;
+ }
+
+ tls.stopServer();
+ dns.clearQueries();
+ }
+
+ dns.stopServer();
+}