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

Commit 08b13d2b authored by Luke Huang's avatar Luke Huang
Browse files

Fix async DNS flag NO_CACHE_STORE doesn't work as expected 

The stale cache case isn't handled correctly while performing
cahce_lookup with flag NO_CACHE_STORE, which caused this problem.

Fix it and add a test to ensure it won't happen again.

Test: atest
Bug: 148842821
Change-Id: I72a2211a636cadc72009a5542f7c755c30329c43
parent 731dc404

res_cache.cpp

+1 −1
Original line number Original line Diff line number Diff line
@@ -1261,7 +1261,7 @@ ResolvCacheStatus resolv_cache_lookup(unsigned netid, const void* query, int que 
        LOG(INFO) << __func__ << ": NOT IN CACHE (STALE ENTRY " << *lookup << "DISCARDED)";

        LOG(INFO) << __func__ << ": NOT IN CACHE (STALE ENTRY " << *lookup << "DISCARDED)";

        res_pquery(e->query, e->querylen);

        res_pquery(e->query, e->querylen);

        _cache_remove_p(cache, lookup);

        _cache_remove_p(cache, lookup);

        return RESOLV_CACHE_NOTFOUND;

        return (flags & ANDROID_RESOLV_NO_CACHE_STORE) ? RESOLV_CACHE_SKIP : RESOLV_CACHE_NOTFOUND;

    }

    }





    *answerlen = e->answerlen;

    *answerlen = e->answerlen;

tests/dns_responder/dns_responder.cpp

+5 −1
Original line number Original line Diff line number Diff line
@@ -547,6 +547,10 @@ void DNSResponder::setEdns(Edns edns) { 
    edns_ = edns;

    edns_ = edns;

}

}





void DNSResponder::setTtl(unsigned ttl) {

    answer_record_ttl_sec_ = ttl;

}



bool DNSResponder::running() const {

bool DNSResponder::running() const {

    return (udp_socket_.ok()) && (tcp_socket_.ok());

    return (udp_socket_.ok()) && (tcp_socket_.ok());

}

}
@@ -783,7 +787,7 @@ bool DNSResponder::addAnswerRecords(const DNSQuestion& question, 
                    .name = {.name = it->first.name},

                    .name = {.name = it->first.name},

                    .rtype = it->first.type,

                    .rtype = it->first.type,

                    .rclass = ns_class::ns_c_in,

                    .rclass = ns_class::ns_c_in,

                    .ttl = kAnswerRecordTtlSec,  // seconds

                    .ttl = answer_record_ttl_sec_,  // seconds

            };

            };

            if (!fillRdata(it->second, record)) return false;

            if (!fillRdata(it->second, record)) return false;

            answers->push_back(std::move(record));

            answers->push_back(std::move(record));

tests/dns_responder/dns_responder.h

+3 −0
Original line number Original line Diff line number Diff line
@@ -175,6 +175,7 @@ class DNSResponder { 
    void setResponseProbability(double response_probability);

    void setResponseProbability(double response_probability);

    void setResponseProbability(double response_probability, int protocol);

    void setResponseProbability(double response_probability, int protocol);

    void setEdns(Edns edns);

    void setEdns(Edns edns);

    void setTtl(unsigned ttl);

    bool running() const;

    bool running() const;

    bool startServer();

    bool startServer();

    bool stopServer();

    bool stopServer();
@@ -293,6 +294,8 @@ class DNSResponder { 
    // returning error_rcode_ or no response.

    // returning error_rcode_ or no response.

    std::atomic<double> response_probability_udp_ = 1.0;

    std::atomic<double> response_probability_udp_ = 1.0;





    std::atomic<unsigned> answer_record_ttl_sec_ = kAnswerRecordTtlSec;



    // Maximum number of fds for epoll.

    // Maximum number of fds for epoll.

    const int EPOLL_MAX_EVENTS = 2;

    const int EPOLL_MAX_EVENTS = 2;

tests/resolv_integration_test.cpp

+46 −0
Original line number Original line Diff line number Diff line
@@ -2235,6 +2235,52 @@ TEST_F(ResolverTest, Async_CacheFlags) { 
    EXPECT_EQ(4U, GetNumQueries(dns, another_host_name));

    EXPECT_EQ(4U, GetNumQueries(dns, another_host_name));

}

}





TEST_F(ResolverTest, Async_NoCacheStoreFlagDoesNotRefreshStaleCacheEntry) {

    constexpr char listen_addr[] = "127.0.0.4";

    constexpr char host_name[] = "howdy.example.com.";

    const std::vector<DnsRecord> records = {

            {host_name, ns_type::ns_t_a, "1.2.3.4"},

    };



    test::DNSResponder dns(listen_addr);

    StartDns(dns, records);

    std::vector<std::string> servers = {listen_addr};

    ASSERT_TRUE(mDnsClient.SetResolversForNetwork(servers));



    const unsigned SHORT_TTL_SEC = 1;

    dns.setTtl(SHORT_TTL_SEC);



    // Refer to b/148842821 for the purpose of below test steps.

    // Basically, this test is used to ensure stale cache case is handled

    // correctly with ANDROID_RESOLV_NO_CACHE_STORE.

    int fd = resNetworkQuery(TEST_NETID, "howdy.example.com", ns_c_in, ns_t_a, 0);

    EXPECT_TRUE(fd != -1);

    expectAnswersValid(fd, AF_INET, "1.2.3.4");



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

    dns.clearQueries();



    // Wait until cache expired

    sleep(SHORT_TTL_SEC + 0.5);



    // Now request the same hostname again.

    // We should see a new DNS query because the entry in cache has become stale.

    // Due to ANDROID_RESOLV_NO_CACHE_STORE, this query must *not* refresh that stale entry.

    fd = resNetworkQuery(TEST_NETID, "howdy.example.com", ns_c_in, ns_t_a,

                         ANDROID_RESOLV_NO_CACHE_STORE);

    EXPECT_TRUE(fd != -1);

    expectAnswersValid(fd, AF_INET, "1.2.3.4");

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

    dns.clearQueries();



    // If the cache is still stale, we expect to see one more DNS query

    // (this time the cache will be refreshed, but we're not checking for it).

    fd = resNetworkQuery(TEST_NETID, "howdy.example.com", ns_c_in, ns_t_a, 0);

    EXPECT_TRUE(fd != -1);

    expectAnswersValid(fd, AF_INET, "1.2.3.4");

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

}



TEST_F(ResolverTest, Async_NoRetryFlag) {

TEST_F(ResolverTest, Async_NoRetryFlag) {

    constexpr char listen_addr0[] = "127.0.0.4";

    constexpr char listen_addr0[] = "127.0.0.4";

    constexpr char listen_addr1[] = "127.0.0.6";

    constexpr char listen_addr1[] = "127.0.0.6";