Fix the side channel attack by using aysnc DnsResolver API with FLAG_NO_CACHE_STORE

Before this CL, FLAG_NO_CACHE_STORE could be used to perform side
channel attack. Because this flag ensures the result is never
added to the cache, but will return a cached response if one exists.
So make FLAG_NO_CACHE_STORE imply FLAG_NO_CACHE_LOOKUP to block the
possibility of side channel attacking.

Bug: 150371903
Test: atest
Merged-In: I37391ffe315b90c0cdfd86888c6bf68b2b89f601

Change-Id: I94d8544d85c615ee985d70e9aeb8f368f100cf9b
(cherry picked from commit 9931b28c98218dac4fa50288b4deb10da85073f1)

3 files changed, 54 insertions, 46 deletions

diff --git a/res_cache.cpp b/res_cache.cpp
index 0cd1f39d..07985d17 100644
--- a/res_cache.cpp
+++ b/res_cache.cpp
@@ -1190,7 +1190,9 @@ ResolvCacheStatus resolv_cache_lookup(unsigned netid, const void* query, int que
     // possible to cache the answer of this query.
     // If ANDROID_RESOLV_NO_CACHE_STORE is set, return RESOLV_CACHE_SKIP to skip possible cache
     // storing.
-    if (flags & ANDROID_RESOLV_NO_CACHE_LOOKUP) {
+    // (b/150371903): ANDROID_RESOLV_NO_CACHE_STORE should imply ANDROID_RESOLV_NO_CACHE_LOOKUP
+    // to avoid side channel attack.
+    if (flags & (ANDROID_RESOLV_NO_CACHE_LOOKUP | ANDROID_RESOLV_NO_CACHE_STORE)) {
         return flags & ANDROID_RESOLV_NO_CACHE_STORE ? RESOLV_CACHE_SKIP : RESOLV_CACHE_NOTFOUND;
     }
     Entry key;
@@ -1221,10 +1223,6 @@ ResolvCacheStatus resolv_cache_lookup(unsigned netid, const void* query, int que
 
     if (e == NULL) {
         LOG(INFO) << __func__ << ": NOT IN CACHE";
-        // If it is no-cache-store mode, we won't wait for possible query.
-        if (flags & ANDROID_RESOLV_NO_CACHE_STORE) {
-            return RESOLV_CACHE_SKIP;
-        }
 
         if (!cache_has_pending_request_locked(cache, &key, true)) {
             return RESOLV_CACHE_NOTFOUND;
@@ -1264,7 +1262,7 @@ ResolvCacheStatus resolv_cache_lookup(unsigned netid, const void* query, int que
         LOG(INFO) << __func__ << ": NOT IN CACHE (STALE ENTRY " << *lookup << "DISCARDED)";
         res_pquery(e->query, e->querylen);
         _cache_remove_p(cache, lookup);
-        return (flags & ANDROID_RESOLV_NO_CACHE_STORE) ? RESOLV_CACHE_SKIP : RESOLV_CACHE_NOTFOUND;
+        return RESOLV_CACHE_NOTFOUND;
     }
 
     *answerlen = e->answerlen;
diff --git a/resolv_cache_unit_test.cpp b/resolv_cache_unit_test.cpp
index 40f1ec6c..7b022bbe 100644
--- a/resolv_cache_unit_test.cpp
+++ b/resolv_cache_unit_test.cpp
@@ -353,7 +353,7 @@ TEST_F(ResolvCacheTest, CacheLookup_CacheFlags) {
     EXPECT_TRUE(cacheLookup(RESOLV_CACHE_NOTFOUND, TEST_NETID, ce, ANDROID_RESOLV_NO_CACHE_LOOKUP));
 
     // Now no-cache-store has no effect if a same entry is existent in the cache.
-    EXPECT_TRUE(cacheLookup(RESOLV_CACHE_FOUND, TEST_NETID, ce, ANDROID_RESOLV_NO_CACHE_STORE));
+    EXPECT_TRUE(cacheLookup(RESOLV_CACHE_SKIP, TEST_NETID, ce, ANDROID_RESOLV_NO_CACHE_STORE));
 
     // Skip the cache lookup again regardless of a same entry being already in the cache.
     EXPECT_TRUE(cacheLookup(RESOLV_CACHE_SKIP, TEST_NETID, ce,
diff --git a/tests/resolv_integration_test.cpp b/tests/resolv_integration_test.cpp
index 2e19bf82..a2a694e1 100644
--- a/tests/resolv_integration_test.cpp
+++ b/tests/resolv_integration_test.cpp
@@ -2239,13 +2239,13 @@ TEST_F(ResolverTest, Async_MalformedQuery) {
 
 TEST_F(ResolverTest, Async_CacheFlags) {
     constexpr char listen_addr[] = "127.0.0.4";
-    constexpr char host_name[] = "howdy.example.com.";
-    constexpr char another_host_name[] = "howdy.example2.com.";
+    constexpr char host_name1[] = "howdy.example.com.";
+    constexpr char host_name2[] = "howdy.example2.com.";
+    constexpr char host_name3[] = "howdy.example3.com.";
     const std::vector<DnsRecord> records = {
-            {host_name, ns_type::ns_t_a, "1.2.3.4"},
-            {host_name, ns_type::ns_t_aaaa, "::1.2.3.4"},
-            {another_host_name, ns_type::ns_t_a, "1.2.3.5"},
-            {another_host_name, ns_type::ns_t_aaaa, "::1.2.3.5"},
+            {host_name1, ns_type::ns_t_a, "1.2.3.4"}, {host_name1, ns_type::ns_t_aaaa, "::1.2.3.4"},
+            {host_name2, ns_type::ns_t_a, "1.2.3.5"}, {host_name2, ns_type::ns_t_aaaa, "::1.2.3.5"},
+            {host_name3, ns_type::ns_t_a, "1.2.3.6"}, {host_name3, ns_type::ns_t_aaaa, "::1.2.3.6"},
     };
 
     test::DNSResponder dns(listen_addr);
@@ -2269,17 +2269,28 @@ TEST_F(ResolverTest, Async_CacheFlags) {
     expectAnswersValid(fd1, AF_INET, "1.2.3.4");
 
     // No cache exists, expect 3 queries
-    EXPECT_EQ(3U, GetNumQueries(dns, host_name));
+    EXPECT_EQ(3U, GetNumQueries(dns, host_name1));
 
-    // Re-query and cache
+    // Raise a query with no flags to ensure no cache exists. Also make an cache entry for the
+    // query.
     fd1 = resNetworkQuery(TEST_NETID, "howdy.example.com", ns_c_in, ns_t_a, 0);
 
     EXPECT_TRUE(fd1 != -1);
 
     expectAnswersValid(fd1, AF_INET, "1.2.3.4");
 
-    // Now we have cache, expect 4 queries
-    EXPECT_EQ(4U, GetNumQueries(dns, host_name));
+    // Expect 4 queries because there should be no cache before this query.
+    EXPECT_EQ(4U, GetNumQueries(dns, host_name1));
+
+    // Now we have the cache entry, re-query with ANDROID_RESOLV_NO_CACHE_STORE to ensure
+    // that ANDROID_RESOLV_NO_CACHE_STORE implied ANDROID_RESOLV_NO_CACHE_LOOKUP.
+    fd1 = resNetworkQuery(TEST_NETID, "howdy.example.com", ns_c_in, ns_t_a,
+                          ANDROID_RESOLV_NO_CACHE_STORE);
+    EXPECT_TRUE(fd1 != -1);
+    expectAnswersValid(fd1, AF_INET, "1.2.3.4");
+    // Expect 5 queries because we shouldn't do cache lookup for the query which has
+    // ANDROID_RESOLV_NO_CACHE_STORE.
+    EXPECT_EQ(5U, GetNumQueries(dns, host_name1));
 
     // ANDROID_RESOLV_NO_CACHE_LOOKUP
     fd1 = resNetworkQuery(TEST_NETID, "howdy.example.com", ns_c_in, ns_t_a,
@@ -2293,78 +2304,77 @@ TEST_F(ResolverTest, Async_CacheFlags) {
     expectAnswersValid(fd2, AF_INET, "1.2.3.4");
     expectAnswersValid(fd1, AF_INET, "1.2.3.4");
 
-    // Skip cache, expect 6 queries
-    EXPECT_EQ(6U, GetNumQueries(dns, host_name));
+    // Cache was skipped, expect 2 more queries.
+    EXPECT_EQ(7U, GetNumQueries(dns, host_name1));
 
     // Re-query verify cache works
-    fd1 = resNetworkQuery(TEST_NETID, "howdy.example.com", ns_c_in, ns_t_a,
-                          ANDROID_RESOLV_NO_CACHE_STORE);
+    fd1 = resNetworkQuery(TEST_NETID, "howdy.example.com", ns_c_in, ns_t_a, 0);
     EXPECT_TRUE(fd1 != -1);
     expectAnswersValid(fd1, AF_INET, "1.2.3.4");
 
-    // Cache hits,  expect still 6 queries
-    EXPECT_EQ(6U, GetNumQueries(dns, host_name));
+    // Cache hits,  expect still 7 queries
+    EXPECT_EQ(7U, GetNumQueries(dns, host_name1));
 
     // Start to verify if ANDROID_RESOLV_NO_CACHE_LOOKUP does write response into cache
     dns.clearQueries();
 
-    fd1 = resNetworkQuery(TEST_NETID, "howdy.example.com", ns_c_in, ns_t_aaaa,
+    fd1 = resNetworkQuery(TEST_NETID, "howdy.example2.com", ns_c_in, ns_t_aaaa,
                           ANDROID_RESOLV_NO_CACHE_LOOKUP);
-    fd2 = resNetworkQuery(TEST_NETID, "howdy.example.com", ns_c_in, ns_t_aaaa,
+    fd2 = resNetworkQuery(TEST_NETID, "howdy.example2.com", ns_c_in, ns_t_aaaa,
                           ANDROID_RESOLV_NO_CACHE_LOOKUP);
 
     EXPECT_TRUE(fd1 != -1);
     EXPECT_TRUE(fd2 != -1);
 
-    expectAnswersValid(fd2, AF_INET6, "::1.2.3.4");
-    expectAnswersValid(fd1, AF_INET6, "::1.2.3.4");
+    expectAnswersValid(fd2, AF_INET6, "::1.2.3.5");
+    expectAnswersValid(fd1, AF_INET6, "::1.2.3.5");
 
     // Skip cache, expect 2 queries
-    EXPECT_EQ(2U, GetNumQueries(dns, host_name));
+    EXPECT_EQ(2U, GetNumQueries(dns, host_name2));
 
     // Re-query without flags
-    fd1 = resNetworkQuery(TEST_NETID, "howdy.example.com", ns_c_in, ns_t_aaaa, 0);
-    fd2 = resNetworkQuery(TEST_NETID, "howdy.example.com", ns_c_in, ns_t_aaaa, 0);
+    fd1 = resNetworkQuery(TEST_NETID, "howdy.example2.com", ns_c_in, ns_t_aaaa, 0);
+    fd2 = resNetworkQuery(TEST_NETID, "howdy.example2.com", ns_c_in, ns_t_aaaa, 0);
 
     EXPECT_TRUE(fd1 != -1);
     EXPECT_TRUE(fd2 != -1);
 
-    expectAnswersValid(fd2, AF_INET6, "::1.2.3.4");
-    expectAnswersValid(fd1, AF_INET6, "::1.2.3.4");
+    expectAnswersValid(fd2, AF_INET6, "::1.2.3.5");
+    expectAnswersValid(fd1, AF_INET6, "::1.2.3.5");
 
     // Cache hits, expect still 2 queries
-    EXPECT_EQ(2U, GetNumQueries(dns, host_name));
+    EXPECT_EQ(2U, GetNumQueries(dns, host_name2));
 
     // Test both ANDROID_RESOLV_NO_CACHE_STORE and ANDROID_RESOLV_NO_CACHE_LOOKUP are set
     dns.clearQueries();
 
-    // Make sure that the cache of "howdy.example2.com" exists.
-    fd1 = resNetworkQuery(TEST_NETID, "howdy.example2.com", ns_c_in, ns_t_aaaa, 0);
+    // Make sure that the cache of "howdy.example3.com" exists.
+    fd1 = resNetworkQuery(TEST_NETID, "howdy.example3.com", ns_c_in, ns_t_aaaa, 0);
     EXPECT_TRUE(fd1 != -1);
-    expectAnswersValid(fd1, AF_INET6, "::1.2.3.5");
-    EXPECT_EQ(1U, GetNumQueries(dns, another_host_name));
+    expectAnswersValid(fd1, AF_INET6, "::1.2.3.6");
+    EXPECT_EQ(1U, GetNumQueries(dns, host_name3));
 
     // Re-query with testFlags
     const int testFlag = ANDROID_RESOLV_NO_CACHE_STORE | ANDROID_RESOLV_NO_CACHE_LOOKUP;
-    fd1 = resNetworkQuery(TEST_NETID, "howdy.example2.com", ns_c_in, ns_t_aaaa, testFlag);
+    fd1 = resNetworkQuery(TEST_NETID, "howdy.example3.com", ns_c_in, ns_t_aaaa, testFlag);
     EXPECT_TRUE(fd1 != -1);
-    expectAnswersValid(fd1, AF_INET6, "::1.2.3.5");
+    expectAnswersValid(fd1, AF_INET6, "::1.2.3.6");
     // Expect cache lookup is skipped.
-    EXPECT_EQ(2U, GetNumQueries(dns, another_host_name));
+    EXPECT_EQ(2U, GetNumQueries(dns, host_name3));
 
     // Do another query with testFlags
-    fd1 = resNetworkQuery(TEST_NETID, "howdy.example2.com", ns_c_in, ns_t_a, testFlag);
+    fd1 = resNetworkQuery(TEST_NETID, "howdy.example3.com", ns_c_in, ns_t_a, testFlag);
     EXPECT_TRUE(fd1 != -1);
-    expectAnswersValid(fd1, AF_INET, "1.2.3.5");
+    expectAnswersValid(fd1, AF_INET, "1.2.3.6");
     // Expect cache lookup is skipped.
-    EXPECT_EQ(3U, GetNumQueries(dns, another_host_name));
+    EXPECT_EQ(3U, GetNumQueries(dns, host_name3));
 
     // Re-query with no flags
-    fd1 = resNetworkQuery(TEST_NETID, "howdy.example2.com", ns_c_in, ns_t_a, 0);
+    fd1 = resNetworkQuery(TEST_NETID, "howdy.example3.com", ns_c_in, ns_t_a, 0);
     EXPECT_TRUE(fd1 != -1);
-    expectAnswersValid(fd1, AF_INET, "1.2.3.5");
+    expectAnswersValid(fd1, AF_INET, "1.2.3.6");
     // Expect no cache hit because cache storing is also skipped in previous query.
-    EXPECT_EQ(4U, GetNumQueries(dns, another_host_name));
+    EXPECT_EQ(4U, GetNumQueries(dns, host_name3));
 }
 
 TEST_F(ResolverTest, Async_NoCacheStoreFlagDoesNotRefreshStaleCacheEntry) {