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

Commit 153b5b85 authored by Mike Yu's avatar Mike Yu
Browse files

Add tests to cover repeated setResolverConfiguration 

Two tests are added to protect the resolver when it continually
receives the same setup requests.

When the resolver receives repeated and same setup:
[1] Do not clear the cache.
[2] Do not clear the stats (the stats for cleartext DNS servers
    got from GetResolverInfo()).
[3] Do not start a new re-evaluation process for the private DNS
    servers if they have been marked as in_progress.
[4] Need not to re-validate the private DNS servers if they have
    been validated.

Another test is added to protect the implementation of aosp/1108695.

Fix: 150678049
Test: resolv_integration_test passed
Change-Id: I7a866e7e305c0fb703ccb9546d1c70ce77e2d3c7
parent e2162e54

PrivateDnsConfiguration.cpp

+4 −0
Original line number Diff line number Diff line
@@ -102,6 +102,10 @@ int PrivateDnsConfiguration::set(int32_t netId, uint32_t mark, 
        mPrivateDnsTransports.erase(netId);

        resolv_stats_set_servers_for_dot(netId, {});

        mPrivateDnsValidateThreads.erase(netId);

        // TODO: As mPrivateDnsValidateThreads is reset, validation threads which haven't yet

        // finished are considered outdated. Consider signaling the outdated validation threads to

        // stop them from updating the state of PrivateDnsConfiguration (possibly disallow them to

        // report onPrivateDnsValidationEvent).

        return 0;

    }

tests/dns_metrics_listener/dns_metrics_listener.h

+7 −0
Original line number Diff line number Diff line
@@ -57,6 +57,13 @@ class DnsMetricsListener : public BaseMetricsListener { 
    // Wait for the expected private DNS validation result until timeout.

    bool waitForPrivateDnsValidation(const std::string& serverAddr, const bool validated);



    // Return true if a validation result for |serverAddr| is found; otherwise, return false.

    // Only exists for testing.

    bool findValidationRecord(const std::string& serverAddr) const EXCLUDES(mMutex) {

        std::lock_guard lock(mMutex);

        return mValidationRecords.find({mNetId, serverAddr}) != mValidationRecords.end();

    }



  private:

    typedef std::pair<int32_t, std::string> ServerKey;

tests/dns_responder/dns_tls_frontend.cpp

+1 −0
Original line number Diff line number Diff line
@@ -200,6 +200,7 @@ void DnsTlsFrontend::requestHandler() { 
                break;

            }



            accept_connection_count_++;

            if (hangOnHandshake_) {

                LOG(DEBUG) << "TEST ONLY: unresponsive to SSL handshake";

tests/dns_responder/dns_tls_frontend.h

+2 −0
Original line number Diff line number Diff line
@@ -57,6 +57,7 @@ class DnsTlsFrontend { 
    int queries() const { return queries_; }

    void clearQueries() { queries_ = 0; }

    bool waitForQueries(int expected_count) const;

    int acceptConnectionsCount() const { return accept_connection_count_; }



    void set_chain_length(int length) { chain_length_ = length; }

    void setHangOnHandshakeForTesting(bool hangOnHandshake) { hangOnHandshake_ = hangOnHandshake; }
@@ -88,6 +89,7 @@ class DnsTlsFrontend { 
    // Eventfd used to signal for the handler thread termination.

    android::base::unique_fd event_fd_;

    std::atomic<int> queries_ = 0;

    std::atomic<int> accept_connection_count_ = 0;

    std::thread handler_thread_ GUARDED_BY(update_mutex_);

    std::mutex update_mutex_;

    int chain_length_ = 1;

tests/resolv_integration_test.cpp

+301 −0
Original line number Diff line number Diff line
@@ -81,6 +81,8 @@ extern "C" int android_getaddrinfofornet(const char* hostname, const char* servn 
                                         const addrinfo* hints, unsigned netid, unsigned mark,

                                         struct addrinfo** result);



using namespace std::chrono_literals;



using aidl::android::net::IDnsResolver;

using aidl::android::net::INetd;

using aidl::android::net::ResolverParamsParcel;
@@ -217,6 +219,10 @@ class ResolverTest : public ::testing::Test { 
        return sDnsMetricsListener->waitForPrivateDnsValidation(serverAddr, validated);

    }



    bool hasUncaughtPrivateDnsValidation(const std::string& serverAddr) {

        return sDnsMetricsListener->findValidationRecord(serverAddr);

    }



    bool expectStatsFromGetResolverInfo(const std::vector<NameserverStats>& nameserversStats) {

        std::vector<std::string> res_servers;

        std::vector<std::string> res_domains;
@@ -265,6 +271,17 @@ class ResolverTest : public ::testing::Test { 
        return true;

    }



    // Since there's no way to terminate private DNS validation threads at any time. Tests that

    // focus on the results of private DNS validation can interfere with each other if they use the

    // same IP address for test servers. getUniqueIPv4Address() is a workaround to reduce the

    // possibility of tests being flaky. A feasible solution is to forbid the validation threads,

    // which are considered as outdated (e.g. switch the resolver to private DNS OFF mode), updating

    // the result to the PrivateDnsConfiguration instance.

    static std::string getUniqueIPv4Address() {

        static int counter = 0;

        return fmt::format("127.0.100.{}", (++counter & 0xff));

    }



    DnsResponderClient mDnsClient;



    // Use a shared static DNS listener for all tests to avoid registering lots of listeners
@@ -4143,6 +4160,290 @@ TEST_F(ResolverTest, TruncatedRspMode) { 
    }

}



TEST_F(ResolverTest, RepeatedSetup_ResolverStatusRemains) {

    constexpr char unusable_listen_addr[] = "127.0.0.3";

    constexpr char listen_addr[] = "127.0.0.4";

    constexpr char hostname[] = "a.hello.query.";

    const auto repeatedSetResolversFromParcel = [&](const ResolverParamsParcel& parcel) {

        ASSERT_TRUE(mDnsClient.SetResolversFromParcel(parcel));

        ASSERT_TRUE(mDnsClient.SetResolversFromParcel(parcel));

        ASSERT_TRUE(mDnsClient.SetResolversFromParcel(parcel));

    };



    test::DNSResponder dns(listen_addr);

    StartDns(dns, {{hostname, ns_type::ns_t_a, "1.2.3.3"}});

    test::DnsTlsFrontend tls1(listen_addr, "853", listen_addr, "53");

    ASSERT_TRUE(tls1.startServer());



    // Private DNS off mode.

    ResolverParamsParcel parcel = DnsResponderClient::GetDefaultResolverParamsParcel();

    parcel.servers = {unusable_listen_addr, listen_addr};

    parcel.tlsServers.clear();

    ASSERT_TRUE(mDnsClient.SetResolversFromParcel(parcel));



    // Send a query.

    const addrinfo hints = {.ai_family = AF_INET, .ai_socktype = SOCK_DGRAM};

    EXPECT_NE(safe_getaddrinfo(hostname, nullptr, &hints), nullptr);



    // Check the stats as expected.

    const std::vector<NameserverStats> expectedCleartextDnsStats = {

            NameserverStats(unusable_listen_addr).setInternalErrors(1),

            NameserverStats(listen_addr).setSuccesses(1),

    };

    EXPECT_TRUE(expectStatsFromGetResolverInfo(expectedCleartextDnsStats));

    EXPECT_EQ(GetNumQueries(dns, hostname), 1U);



    // The stats is supposed to remain as long as the list of cleartext DNS servers is unchanged.

    static const struct TestConfig {

        std::vector<std::string> servers;

        std::vector<std::string> tlsServers;

        std::string tlsName;

    } testConfigs[] = {

            // Private DNS opportunistic mode.

            {{listen_addr, unusable_listen_addr}, {listen_addr, unusable_listen_addr}, ""},

            {{unusable_listen_addr, listen_addr}, {unusable_listen_addr, listen_addr}, ""},



            // Private DNS strict mode.

            {{listen_addr, unusable_listen_addr}, {"127.0.0.100"}, kDefaultPrivateDnsHostName},

            {{unusable_listen_addr, listen_addr}, {"127.0.0.100"}, kDefaultPrivateDnsHostName},



            // Private DNS off mode.

            {{unusable_listen_addr, listen_addr}, {}, ""},

            {{listen_addr, unusable_listen_addr}, {}, ""},

    };



    for (const auto& config : testConfigs) {

        SCOPED_TRACE(fmt::format("testConfig: [{}] [{}] [{}]", fmt::join(config.servers, ","),

                                 fmt::join(config.tlsServers, ","), config.tlsName));

        parcel = DnsResponderClient::GetDefaultResolverParamsParcel();

        parcel.servers = config.servers;

        parcel.tlsServers = config.tlsServers;

        parcel.tlsName = config.tlsName;

        repeatedSetResolversFromParcel(parcel);

        EXPECT_TRUE(expectStatsFromGetResolverInfo(expectedCleartextDnsStats));



        // The stats remains when the list of search domains changes.

        parcel.domains.push_back("tmp.domains");

        repeatedSetResolversFromParcel(parcel);

        EXPECT_TRUE(expectStatsFromGetResolverInfo(expectedCleartextDnsStats));



        // The stats remains when the parameters change (except maxSamples).

        parcel.sampleValiditySeconds++;

        parcel.successThreshold++;

        parcel.minSamples++;

        parcel.baseTimeoutMsec++;

        parcel.retryCount++;

        repeatedSetResolversFromParcel(parcel);

        EXPECT_TRUE(expectStatsFromGetResolverInfo(expectedCleartextDnsStats));

    }



    // The cache remains.

    EXPECT_NE(safe_getaddrinfo(hostname, nullptr, &hints), nullptr);

    EXPECT_EQ(GetNumQueries(dns, hostname), 1U);

}



TEST_F(ResolverTest, RepeatedSetup_NoRedundantPrivateDnsValidation) {

    const std::string addr1 = getUniqueIPv4Address();  // For a workable DNS server.

    const std::string addr2 = getUniqueIPv4Address();  // For an unresponsive DNS server.

    const std::string unusable_addr = getUniqueIPv4Address();



    test::DNSResponder dns1(addr1);

    test::DNSResponder dns2(addr2);

    StartDns(dns1, {});

    StartDns(dns2, {});

    test::DnsTlsFrontend workableTls(addr1, "853", addr1, "53");

    test::DnsTlsFrontend unresponsiveTls(addr2, "853", addr2, "53");

    unresponsiveTls.setHangOnHandshakeForTesting(true);

    ASSERT_TRUE(workableTls.startServer());

    ASSERT_TRUE(unresponsiveTls.startServer());



    // First setup.

    ResolverParamsParcel parcel = DnsResponderClient::GetDefaultResolverParamsParcel();

    parcel.servers = {addr1, addr2, unusable_addr};

    parcel.tlsServers = {addr1, addr2, unusable_addr};

    ASSERT_TRUE(mDnsClient.SetResolversFromParcel(parcel));



    // Check the validation results.

    EXPECT_TRUE(WaitForPrivateDnsValidation(workableTls.listen_address(), true));

    EXPECT_TRUE(WaitForPrivateDnsValidation(unusable_addr, false));

    EXPECT_EQ(unresponsiveTls.acceptConnectionsCount(), 1);  // The validation is still in progress.



    static const struct TestConfig {

        std::vector<std::string> tlsServers;

        std::string tlsName;

    } testConfigs[] = {

            {{addr1, addr2, unusable_addr}, ""},

            {{unusable_addr, addr1, addr2}, ""},

            {{unusable_addr, addr1, addr2}, kDefaultPrivateDnsHostName},

            {{addr1, addr2, unusable_addr}, kDefaultPrivateDnsHostName},

    };



    std::string TlsNameLastTime;

    for (const auto& config : testConfigs) {

        SCOPED_TRACE(fmt::format("testConfig: [{}] [{}]", fmt::join(config.tlsServers, ","),

                                 config.tlsName));

        parcel.servers = config.tlsServers;

        parcel.tlsServers = config.tlsServers;

        parcel.tlsName = config.tlsName;

        parcel.caCertificate = config.tlsName.empty() ? "" : kCaCert;



        const bool dnsModeChanged = (TlsNameLastTime != config.tlsName);

        ASSERT_TRUE(mDnsClient.SetResolversFromParcel(parcel));



        for (const auto& serverAddr : parcel.tlsServers) {

            SCOPED_TRACE(serverAddr);

            if (serverAddr == workableTls.listen_address()) {

                if (dnsModeChanged) {

                    // In despite of the identical IP address, the server is regarded as a different

                    // server when DnsTlsServer.name is different. The resolver treats it as a

                    // different object and begins the validation process.

                    EXPECT_TRUE(WaitForPrivateDnsValidation(serverAddr, true));

                }

            } else if (serverAddr == unresponsiveTls.listen_address()) {

                // No revalidation needed for the server which have been marked as in_progesss.

            } else {

                // Must be unusable_addr.

                // In opportunistic mode, when a validation for a private DNS server fails, the

                // resolver just marks the server as failed and doesn't re-evaluate it, but the

                // server can be re-evaluated when setResolverConfiguration() is called.

                // However, in strict mode, the resolver automatically re-evaluates the server and

                // marks the server as in_progress until the validation succeeds, so repeated setup

                // makes no effect.

                if (dnsModeChanged || config.tlsName.empty() /* not in strict mode */) {

                    EXPECT_TRUE(WaitForPrivateDnsValidation(serverAddr, false));

                }

            }

        }



        // Repeated setups make no effect in strict mode.

        ASSERT_TRUE(mDnsClient.SetResolversFromParcel(parcel));

        if (config.tlsName.empty()) {

            EXPECT_TRUE(WaitForPrivateDnsValidation(unusable_addr, false));

        }

        ASSERT_TRUE(mDnsClient.SetResolversFromParcel(parcel));

        if (config.tlsName.empty()) {

            EXPECT_TRUE(WaitForPrivateDnsValidation(unusable_addr, false));

        }



        EXPECT_EQ(unresponsiveTls.acceptConnectionsCount(), 1);



        TlsNameLastTime = config.tlsName;

    }



    // Check that all the validation results are caught.

    // Note: it doesn't mean no validation being in progress.

    EXPECT_FALSE(hasUncaughtPrivateDnsValidation(addr1));

    EXPECT_FALSE(hasUncaughtPrivateDnsValidation(addr2));

    EXPECT_FALSE(hasUncaughtPrivateDnsValidation(unusable_addr));

}



TEST_F(ResolverTest, RepeatedSetup_KeepChangingPrivateDnsServers) {

    enum TlsServerState { WORKING, UNSUPPORTED, UNRESPONSIVE };

    const std::string addr1 = getUniqueIPv4Address();

    const std::string addr2 = getUniqueIPv4Address();



    test::DNSResponder dns1(addr1);

    test::DNSResponder dns2(addr2);

    StartDns(dns1, {});

    StartDns(dns2, {});

    test::DnsTlsFrontend tls1(addr1, "853", addr1, "53");

    test::DnsTlsFrontend tls2(addr2, "853", addr2, "53");

    ASSERT_TRUE(tls1.startServer());

    ASSERT_TRUE(tls2.startServer());



    static const struct TestConfig {

        std::string tlsServer;

        std::string tlsName;

        bool expectNothingHappenWhenServerUnsupported;

        bool expectNothingHappenWhenServerUnresponsive;

        std::string asTestName() const {

            return fmt::format("{}, {}, {}, {}", tlsServer, tlsName,

                               expectNothingHappenWhenServerUnsupported,

                               expectNothingHappenWhenServerUnresponsive);

        }

    } testConfigs[] = {

            {{addr1}, "", false, false},

            {{addr2}, "", false, false},

            {{addr1}, "", false, true},

            {{addr2}, "", false, true},

            {{addr1}, kDefaultPrivateDnsHostName, false, true},

            {{addr2}, kDefaultPrivateDnsHostName, false, true},

            {{addr1}, kDefaultPrivateDnsHostName, true, true},

            {{addr2}, kDefaultPrivateDnsHostName, true, true},



            // There's no new validation to start because there are already two validation threads

            // running (one is for addr1, the other is for addr2). This is because the comparator

            // doesn't compare DnsTlsServer.name. Keep the design as-is until it's known to be

            // harmful.

            {{addr1}, "", true, true},

            {{addr2}, "", true, true},

            {{addr1}, "", true, true},

            {{addr2}, "", true, true},

    };



    for (const auto& serverState : {WORKING, UNSUPPORTED, UNRESPONSIVE}) {

        int testIndex = 0;

        for (const auto& config : testConfigs) {

            SCOPED_TRACE(fmt::format("serverState:{} testIndex:{} testConfig:[{}]", serverState,

                                     testIndex++, config.asTestName()));

            auto& tls = (config.tlsServer == addr1) ? tls1 : tls2;



            if (serverState == UNSUPPORTED && tls.running()) ASSERT_TRUE(tls.stopServer());

            if (serverState != UNSUPPORTED && !tls.running()) ASSERT_TRUE(tls.startServer());



            tls.setHangOnHandshakeForTesting(serverState == UNRESPONSIVE);

            const int connectCountsBefore = tls.acceptConnectionsCount();



            ResolverParamsParcel parcel = DnsResponderClient::GetDefaultResolverParamsParcel();

            parcel.servers = {config.tlsServer};

            parcel.tlsServers = {config.tlsServer};

            parcel.tlsName = config.tlsName;

            parcel.caCertificate = config.tlsName.empty() ? "" : kCaCert;

            ASSERT_TRUE(mDnsClient.SetResolversFromParcel(parcel));



            if (serverState == WORKING) {

                EXPECT_TRUE(WaitForPrivateDnsValidation(config.tlsServer, true));

            } else if (serverState == UNSUPPORTED) {

                if (config.expectNothingHappenWhenServerUnsupported) {

                    // It's possible that the resolver hasn't yet started to

                    // connect. Wait a while.

                    // TODO: See if we can get rid of the hard waiting time, such as comparing

                    // the CountDiff across two tests.

                    std::this_thread::sleep_for(100ms);

                    EXPECT_EQ(tls.acceptConnectionsCount(), connectCountsBefore);

                } else {

                    EXPECT_TRUE(WaitForPrivateDnsValidation(config.tlsServer, false));

                }

            } else {

                // Must be UNRESPONSIVE.

                // DnsTlsFrontend is the only signal for checking whether or not the resolver starts

                // another validation when the server is unresponsive.

                const int expectCountDiff =

                        config.expectNothingHappenWhenServerUnresponsive ? 0 : 1;

                if (expectCountDiff == 0) {

                    // It's possible that the resolver hasn't yet started to

                    // connect. Wait a while.

                    std::this_thread::sleep_for(100ms);

                }

                const auto condition = [&]() {

                    return tls.acceptConnectionsCount() == connectCountsBefore + expectCountDiff;

                };

                EXPECT_TRUE(PollForCondition(condition));

            }

        }



        // Set to off mode to reset the PrivateDnsConfiguration state.

        ResolverParamsParcel setupOffmode = DnsResponderClient::GetDefaultResolverParamsParcel();

        setupOffmode.tlsServers.clear();

        ASSERT_TRUE(mDnsClient.SetResolversFromParcel(setupOffmode));

    }



    // Check that all the validation results are caught.

    // Note: it doesn't mean no validation being in progress.

    EXPECT_FALSE(hasUncaughtPrivateDnsValidation(addr1));

    EXPECT_FALSE(hasUncaughtPrivateDnsValidation(addr2));

}



// Parameterized tests.

// TODO: Merge the existing tests as parameterized test if possible.

// TODO: Perhaps move parameterized tests to an independent file.