Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit dc457577 authored by Mike Yu's avatar Mike Yu Committed by Automerger Merge Worker
Browse files

Fix dot_xport_unusable_threshold to work am: dff34e1b 

Original change: https://android-review.googlesource.com/c/platform/packages/modules/DnsResolver/+/2089888



Change-Id: I152403c70f1ea58e25034dff20ed59e7c44ce453
Signed-off-by: default avatarAutomerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
parents eaeeeb86 dff34e1b

DnsTlsDispatcher.cpp

+18 −17
Original line number Diff line number Diff line
@@ -67,8 +67,7 @@ std::list<DnsTlsServer> DnsTlsDispatcher::getOrderedAndUsableServerList( 


        for (const auto& tlsServer : tlsServers) {

            const Key key = std::make_pair(mark, tlsServer);

            if (const Transport* xport = getTransport(key); xport != nullptr) {

                // DoT revalidation specific feature.

            if (Transport* xport = getTransport(key); xport != nullptr) {

                if (!xport->usable()) {

                    // Don't use this xport. It will be removed after timeout

                    // (IDLE_TIMEOUT minutes).
@@ -209,9 +208,14 @@ DnsTlsTransport::Response DnsTlsDispatcher::query(const DnsTlsServer& server, un 
        std::lock_guard guard(sLock);

        --xport->useCount;

        xport->lastUsed = now;

        if (code == DnsTlsTransport::Response::network_error) {

            xport->continuousfailureCount++;

        } else {

            xport->continuousfailureCount = 0;

        }



        // DoT revalidation specific feature.

        if (xport->checkRevalidationNecessary(code)) {

        if (xport->checkRevalidationNecessary()) {

            // Even if the revalidation passes, it doesn't guarantee that DoT queries

            // to the xport can stop failing because revalidation creates a new connection

            // to probe while the xport still uses an existing connection. So far, there isn't
@@ -336,26 +340,23 @@ DnsTlsDispatcher::Transport* DnsTlsDispatcher::getTransport(const Key& key) { 
    return (it == mStore.end() ? nullptr : it->second.get());

}



bool DnsTlsDispatcher::Transport::checkRevalidationNecessary(DnsTlsTransport::Response code) {

bool DnsTlsDispatcher::Transport::checkRevalidationNecessary() {

    if (triggerThreshold <= 0) return false;

    if (continuousfailureCount < triggerThreshold) return false;

    if (isRevalidationThresholdReached) return false;



    if (code == DnsTlsTransport::Response::network_error) {

        continuousfailureCount++;

    } else {

        continuousfailureCount = 0;

    }



    // triggerThreshold must be greater than 0 because the value of revalidationEnabled is true.

    if (usable() && continuousfailureCount == triggerThreshold) {

    isRevalidationThresholdReached = true;

    return true;

}

    return false;

}



bool DnsTlsDispatcher::Transport::usable() const {

bool DnsTlsDispatcher::Transport::usable() {

    if (unusableThreshold <= 0) return true;



    return continuousfailureCount < unusableThreshold;

    if (continuousfailureCount >= unusableThreshold) {

        // Once reach the threshold, mark this Transport as unusable.

        isXportUnusableThresholdReached = true;

    }

    return !isXportUnusableThresholdReached;

}



}  // end of namespace net

DnsTlsDispatcher.h

+10 −4
Original line number Diff line number Diff line
@@ -105,9 +105,12 @@ class DnsTlsDispatcher : public PrivateDnsValidationObserver { 


        // If DoT revalidation is disabled, it returns true; otherwise, it returns

        // whether or not this Transport is usable.

        bool usable() const REQUIRES(sLock);

        bool usable() REQUIRES(sLock);



        bool checkRevalidationNecessary(DnsTlsTransport::Response code) REQUIRES(sLock);

        // Used to track if this Transport is usable.

        int continuousfailureCount GUARDED_BY(sLock) = 0;



        bool checkRevalidationNecessary() REQUIRES(sLock);



        std::chrono::milliseconds timeout() const { return mTimeout; }
@@ -116,8 +119,11 @@ class DnsTlsDispatcher : public PrivateDnsValidationObserver { 
        static constexpr int kDotQueryTimeoutMs = -1;



      private:

        // Used to track if this Transport is usable.

        int continuousfailureCount GUARDED_BY(sLock) = 0;

        // The flag to record whether or not dot_revalidation_threshold is ever reached.

        bool isRevalidationThresholdReached GUARDED_BY(sLock) = false;



        // The flag to record whether or not dot_xport_unusable_threshold is ever reached.

        bool isXportUnusableThresholdReached GUARDED_BY(sLock) = false;



        // If the number of continuous query timeouts reaches the threshold, mark the

        // server as unvalidated and trigger a validation.

tests/resolv_integration_test.cpp

+99 −1
Original line number Diff line number Diff line
@@ -2907,6 +2907,11 @@ TEST_F(ResolverTest, BrokenEdns) { 
        const std::string testHostName = config.asHostName();

        SCOPED_TRACE(testHostName);



        // Don't skip unusable DoT servers and disable revalidation for this test.

        ScopedSystemProperties sp1(kDotXportUnusableThresholdFlag, "-1");

        ScopedSystemProperties sp2(kDotRevalidationThresholdFlag, "-1");

        resetNetwork();



        const char* host_name = testHostName.c_str();

        dns.addMapping(host_name, ns_type::ns_t_a, ADDR4);

        dns.setEdns(config.edns);
@@ -4572,6 +4577,10 @@ TEST_F(ResolverTest, ConnectTlsServerTimeout) { 


        ScopedSystemProperties sp3(kDotAsyncHandshakeFlag, config.asyncHandshake ? "1" : "0");

        ScopedSystemProperties sp4(kDotMaxretriesFlag, std::to_string(config.maxRetries));



        // Don't skip unusable DoT servers and disable revalidation for this test.

        ScopedSystemProperties sp5(kDotXportUnusableThresholdFlag, "-1");

        ScopedSystemProperties sp6(kDotRevalidationThresholdFlag, "-1");

        resetNetwork();



        // Set up resolver to opportunistic mode.
@@ -4662,6 +4671,10 @@ TEST_F(ResolverTest, ConnectTlsServerTimeout_ConcurrentQueries) { 
                                   std::to_string(config.dotConnectTimeoutMs));

        ScopedSystemProperties sp3(kDotAsyncHandshakeFlag, config.asyncHandshake ? "1" : "0");

        ScopedSystemProperties sp4(kDotMaxretriesFlag, std::to_string(config.maxRetries));



        // Don't skip unusable DoT servers and disable revalidation for this test.

        ScopedSystemProperties sp5(kDotXportUnusableThresholdFlag, "-1");

        ScopedSystemProperties sp6(kDotRevalidationThresholdFlag, "-1");

        resetNetwork();



        for (const auto& dnsMode : {"OPPORTUNISTIC", "STRICT"}) {
@@ -4744,6 +4757,10 @@ TEST_F(ResolverTest, QueryTlsServerTimeout) { 
            ASSERT_TRUE(tls.startServer());



            ScopedSystemProperties sp(kDotQueryTimeoutMsFlag, std::to_string(queryTimeoutMs));



            // Don't skip unusable DoT servers and disable revalidation for this test.

            ScopedSystemProperties sp2(kDotXportUnusableThresholdFlag, "-1");

            ScopedSystemProperties sp3(kDotRevalidationThresholdFlag, "-1");

            resetNetwork();



            auto parcel = DnsResponderClient::GetDefaultResolverParamsParcel();
@@ -4792,6 +4809,84 @@ TEST_F(ResolverTest, QueryTlsServerTimeout) { 
    }

}



// Tests that the DnsResolver can skip using unusable DoT servers if dot_xport_unusable_threshold

// flag is set. In this test, we make test DoT servers unresponsive during connection handshake,

// so the DnsResolver will skip using a DoT server if the number of timed out queries reaches

// the threshold.

TEST_F(ResolverTest, SkipUnusableTlsServer) {

    constexpr int DOT_CONNECT_TIMEOUT_MS = 1000;



    static const struct TestConfig {

        int dotXportUnusableThreshold;

        int queries;

        int expectedQueriesSentToDot1;

        int expectedQueriesSentToDot2;

    } testConfigs[] = {

            // clang-format off

            // expectedQueriesSentToDot2 is 0 because dot_quick_fallback flag is set.

            {-1,  3, 3, 0},

            { 1,  3, 1, 1},

            { 3, 10, 3, 3},

            // clang-format on

    };



    for (const auto& config : testConfigs) {

        SCOPED_TRACE(fmt::format("testConfig: [{}, {}, {}, {}]", config.dotXportUnusableThreshold,

                                 config.queries, config.expectedQueriesSentToDot1,

                                 config.expectedQueriesSentToDot2));



        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(config.dotXportUnusableThreshold));

        ScopedSystemProperties sp3(kDotQuickFallbackFlag, "1");

        ScopedSystemProperties sp4(kDotRevalidationThresholdFlag, "-1");

        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 sequential queries

        for (int i = 0; i < config.queries; i++) {

            int fd = resNetworkQuery(TEST_NETID, kHelloExampleCom, ns_c_in, ns_t_aaaa,

                                     ANDROID_RESOLV_NO_CACHE_LOOKUP);

            expectAnswersValid(fd, AF_INET6, kHelloExampleComAddrV6);

        }



        EXPECT_EQ(dot1.acceptConnectionsCount(), config.expectedQueriesSentToDot1);

        EXPECT_EQ(dot2.acceptConnectionsCount(), config.expectedQueriesSentToDot2);

    }

}



// Verifies that the DnsResolver re-validates the DoT server when several DNS queries to

// the server fails in a row.

TEST_F(ResolverTest, TlsServerRevalidation) {
@@ -4814,7 +4909,7 @@ TEST_F(ResolverTest, TlsServerRevalidation) { 
    } testConfigs[] = {

            // clang-format off

            {"OPPORTUNISTIC", -1,  5, false, false},

            {"OPPORTUNISTIC", -1, 10, false, false},

            {"OPPORTUNISTIC", -1, 10, false,  true},

            {"OPPORTUNISTIC",  5,  5,  true, false},

            {"OPPORTUNISTIC",  5, 10,  true,  true},

            {"STRICT",        -1,  5, false, false},
@@ -5061,6 +5156,9 @@ TEST_F(ResolverTest, DotQuickFallback) { 
                                   std::to_string(DOT_CONNECT_TIMEOUT_MS));

        ScopedSystemProperties sp2(kDotQuickFallbackFlag,

                                   std::to_string(config.dotQuickFallbackFlag));



        // Disable revalidation because we are reusing the same IP address of DoT servers.

        ScopedSystemProperties sp3(kDotRevalidationThresholdFlag, "-1");

        resetNetwork();



        auto parcel = DnsResponderClient::GetDefaultResolverParamsParcel();