Merge "Revert "Add a flag dot_keep_unusable_xport_sec"" (b9868dfb) · Commits · e / os / android_packages_modules_DnsResolver

DnsTlsDispatcher.cpp

+3 −23

Original line number	Diff line number	Diff line
		@@ -26,7 +26,6 @@
		#include "resolv_cache.h"
		#include "resolv_private.h"
		#include "stats.pb.h"
		#include "util.h"

		#include <android-base/logging.h>

		@@ -244,7 +243,7 @@ DnsTlsTransport::Response DnsTlsDispatcher::query(const DnsTlsServer& server, un

		void DnsTlsDispatcher::forceCleanup(unsigned netId) {
		std::lock_guard guard(sLock);
		cleanup(std::chrono::steady_clock::now(), std::chrono::seconds(-1), netId);
		cleanup(std::chrono::steady_clock::now(), netId);
		}

		DnsTlsTransport::Result DnsTlsDispatcher::queryInternal(Transport& xport,
		@@ -277,41 +276,22 @@ DnsTlsTransport::Result DnsTlsDispatcher::queryInternal(Transport& xport,
		// This timeout effectively controls how long to keep SSL session tickets.
		static constexpr std::chrono::minutes IDLE_TIMEOUT(5);
		void DnsTlsDispatcher::maybeCleanup(std::chrono::time_point<std::chrono::steady_clock> now) {
		// Make the timeout tunable via experiment flag for testing.
		std::chrono::seconds unusable_xport_idle_timeout{-1};
		const int value = Experiments::getInstance()->getFlag("dot_keep_unusable_xport_sec", -1);
		if (value > -1 && isUserDebugBuild() && std::chrono::seconds(value) < IDLE_TIMEOUT) {
		unusable_xport_idle_timeout = std::chrono::seconds(value);
		}

		// To avoid scanning mStore after every query, return early if a cleanup has been
		// performed recently.
		const std::chrono::seconds timeout = (unusable_xport_idle_timeout < IDLE_TIMEOUT)
		? unusable_xport_idle_timeout
		: IDLE_TIMEOUT;
		if (now - mLastCleanup < timeout) {
		if (now - mLastCleanup < IDLE_TIMEOUT) {
		return;
		}
		cleanup(now, unusable_xport_idle_timeout, std::nullopt);
		cleanup(now, std::nullopt);
		mLastCleanup = now;
		}

		void DnsTlsDispatcher::cleanup(std::chrono::time_point<std::chrono::steady_clock> now,
		std::chrono::seconds unusable_xport_idle_timeout,
		std::optional<unsigned> netId) {
		std::erase_if(mStore, [&](const auto& item) REQUIRES(sLock) {
		auto const& [_, xport] = item;
		if (xport->useCount == 0) {
		// Remove the Transports of the associated network.
		if (netId.has_value() && xport->mNetId == netId.value()) return true;

		// Remove all expired Transports.
		if (now - xport->lastUsed > IDLE_TIMEOUT) return true;

		// Unusable Transports should be removed earlier.
		if (!xport->usable() && unusable_xport_idle_timeout.count() >= 0 &&
		now - xport->lastUsed > unusable_xport_idle_timeout)
		return true;
		}
		return false;
		});

DnsTlsDispatcher.h

+1 −2

Original line number	Diff line number	Diff line
		@@ -165,8 +165,7 @@ class DnsTlsDispatcher : public PrivateDnsValidationObserver {
		// Drop any cache entries whose useCount is zero and which have not been used recently.
		// This function performs a linear scan of mStore.
		void cleanup(std::chrono::time_point<std::chrono::steady_clock> now,
		std::chrono::seconds unusable_xport_idle_timeout, std::optional<unsigned> netId)
		REQUIRES(sLock);
		std::optional<unsigned> netId) REQUIRES(sLock);

		// Return a sorted list of usable DnsTlsServers in preference order.
		std::list<DnsTlsServer> getOrderedAndUsableServerList(const std::list<DnsTlsServer>& tlsServers,

Experiments.h

+0 −1

Original line number	Diff line number	Diff line
		@@ -55,7 +55,6 @@ class Experiments {
		"sort_nameservers",
		"dot_async_handshake",
		"dot_connect_timeout_ms",
		"dot_keep_unusable_xport_sec",
		"dot_maxtries",
		"dot_revalidation_threshold",
		"dot_xport_unusable_threshold",

tests/resolv_integration_test.cpp

+0 −84

Original line number	Diff line number	Diff line
		@@ -94,9 +94,6 @@ const std::string kDotValidationLatencyFactorFlag(
		const std::string kDotValidationLatencyOffsetMsFlag(
		"persist.device_config.netd_native.dot_validation_latency_offset_ms");
		const std::string kDotQuickFallbackFlag("persist.device_config.netd_native.dot_quick_fallback");
		const std::string kDotKeepUnusableXportSecFlag(
		"persist.device_config.netd_native.dot_keep_unusable_xport_sec");

		// Semi-public Bionic hook used by the NDK (frameworks/base/native/android/net.c)
		// Tested here for convenience.
		extern "C" int android_getaddrinfofornet(const char* hostname, const char* servname,
		@@ -4912,87 +4909,6 @@ TEST_F(ResolverTest, SkipUnusableTlsServer) {
		}
		}

		// Tests that, even if all DoT servers are mark as unusable, DnsResolver will try to send DNS
		// queries to DoT servers after their expiration.
		TEST_F(ResolverTest, UnusableTlsServersWorkAgain) {
		const auto sendQueryAndCheckResult = [&]() {
		int fd = resNetworkQuery(TEST_NETID, kHelloExampleCom, ns_c_in, ns_t_aaaa,
		ANDROID_RESOLV_NO_CACHE_LOOKUP);
		expectAnswersValid(fd, AF_INET6, kHelloExampleComAddrV6);
		};

		constexpr int DOT_CONNECT_TIMEOUT_MS = 1000;
		constexpr int DOT_XPORT_UNUSABLE_THRESHOLD = 1;
		constexpr int DOT_KEEP_UNUSABLE_XPORT_SEC = 2;

		const std::string addr1 = getUniqueIPv4Address();
		const std::string addr2 = getUniqueIPv4Address();
		test::DNSResponder dns1(addr1);
		test::DNSResponder dns2(addr2);
		test::DnsTlsFrontend dot1(addr1, "853", addr1, "53");
		test::DnsTlsFrontend dot2(addr2, "853", addr2, "53");
		dns1.addMapping(kHelloExampleCom, ns_type::ns_t_aaaa, kHelloExampleComAddrV6);
		dns2.addMapping(kHelloExampleCom, ns_type::ns_t_aaaa, kHelloExampleComAddrV6);
		ASSERT_TRUE(dns1.startServer());
		ASSERT_TRUE(dns2.startServer());
		ASSERT_TRUE(dot1.startServer());
		ASSERT_TRUE(dot2.startServer());

		ScopedSystemProperties sp1(kDotConnectTimeoutMsFlag, std::to_string(DOT_CONNECT_TIMEOUT_MS));
		ScopedSystemProperties sp2(kDotXportUnusableThresholdFlag,
		std::to_string(DOT_XPORT_UNUSABLE_THRESHOLD));
		ScopedSystemProperties sp3(kDotQuickFallbackFlag, "1");
		ScopedSystemProperties sp4(kDotRevalidationThresholdFlag, "-1");
		ScopedSystemProperties sp5(kDotKeepUnusableXportSecFlag,
		std::to_string(DOT_KEEP_UNUSABLE_XPORT_SEC));
		resetNetwork();

		// Private DNS opportunistic mode.
		auto parcel = DnsResponderClient::GetDefaultResolverParamsParcel();
		parcel.servers = {addr1, addr2};
		parcel.tlsServers = {addr1, addr2};
		ASSERT_TRUE(mDnsClient.SetResolversFromParcel(parcel));

		EXPECT_TRUE(WaitForPrivateDnsValidation(dot1.listen_address(), true));
		EXPECT_TRUE(WaitForPrivateDnsValidation(dot2.listen_address(), true));
		EXPECT_TRUE(dot1.waitForQueries(1));
		EXPECT_TRUE(dot2.waitForQueries(1));
		dot1.clearQueries();
		dot2.clearQueries();
		dot1.clearConnectionsCount();
		dot2.clearConnectionsCount();

		// Set the DoT servers as unresponsive to connection handshake.
		dot1.setHangOnHandshakeForTesting(true);
		dot2.setHangOnHandshakeForTesting(true);

		// Send two queries in series. The two DoT servers will be marked as unusable.
		sendQueryAndCheckResult();
		sendQueryAndCheckResult();
		EXPECT_EQ(dot1.acceptConnectionsCount(), 1);
		EXPECT_EQ(dot2.acceptConnectionsCount(), 1);
		EXPECT_EQ(dot1.queries(), 0);
		EXPECT_EQ(dot2.queries(), 0);

		// Wait for a bit more than `dot_keep_unusable_xport_sec` seconds, and then send one
		// DNS query. DnsResolver should remove the unusable mark from the two DoT servers.
		std::this_thread::sleep_for(std::chrono::milliseconds(2500));
		sendQueryAndCheckResult();
		EXPECT_EQ(dot1.acceptConnectionsCount(), 1);
		EXPECT_EQ(dot2.acceptConnectionsCount(), 1);
		EXPECT_EQ(dot1.queries(), 0);
		EXPECT_EQ(dot2.queries(), 0);

		// Recover the DoT servers, and then send one DNS query. Expect that DnsResolver uses DoT again.
		dot1.setHangOnHandshakeForTesting(false);
		dot2.setHangOnHandshakeForTesting(false);
		sendQueryAndCheckResult();
		EXPECT_EQ(dot1.acceptConnectionsCount(), 2);
		EXPECT_EQ(dot2.acceptConnectionsCount(), 1);
		EXPECT_EQ(dot1.queries(), 1);
		EXPECT_EQ(dot2.queries(), 0);
		}

		// Verifies that the DnsResolver re-validates the DoT server when several DNS queries to
		// the server fails in a row.
		TEST_F(ResolverTest, TlsServerRevalidation) {