From 11ad8ac8e1f6b3c7f50ca45b5de2f40e30f35cfb Mon Sep 17 00:00:00 2001
From: Ken Chen <cken@google.com>
Date: Thu, 30 Jul 2020 13:24:16 +0800
Subject: Fix OOB read in DNS resolver

The remote server specifies resplen, the length of the response it
intends to send. anssiz represents the size of the destination buffer.
If the reported resplen is larger than the anssiz, the code correctly
only reads up to anssiz bytes, but returns resplen. so later functions
will access far out of bounds.

The fix ensures that the length of send_vc return does not exceed the
buffer size.

(Manually backport commit from ag/12280247, since it's different git
project on qt-dev. Use aosp/1302595 as Merged-In tag to avoid conflict)

Bug: 161362564
Test: atest pass
Change-Id: Id4b5df1be4652e4623847b0b0bad0af65b80fdd5
Merged-In: I1ff2dc09f41f76973c5f066b07b15388e722b375
---
 resolv/res_send.cpp    | 5 ++++-
 server/AndroidTest.xml | 1 +
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/resolv/res_send.cpp b/resolv/res_send.cpp
index d89ad7ee..5adf8012 100644
--- a/resolv/res_send.cpp
+++ b/resolv/res_send.cpp
@@ -880,6 +880,9 @@ read_len:
             else
                 break;
         }
+        LOG(WARNING) << __func__ << ": resplen " << resplen << " exceeds buf size " << anssiz;
+        // return size should never exceed container size
+        resplen = anssiz;
     }
     /*
      * If the calling application has bailed out of
@@ -890,7 +893,7 @@ read_len:
      */
     if (hp->id != anhp->id) {
         LOG(DEBUG) << __func__ << ": ld answer (unexpected):";
-        res_pquery(ans, (resplen > anssiz) ? anssiz : resplen);
+        res_pquery(ans, resplen);
         goto read_len;
     }
 
diff --git a/server/AndroidTest.xml b/server/AndroidTest.xml
index 2501d781..fbb0be72 100644
--- a/server/AndroidTest.xml
+++ b/server/AndroidTest.xml
@@ -14,6 +14,7 @@
      limitations under the License.
 -->
 <configuration description="Config for netd_unit_test">
+    <target_preparer class="com.android.tradefed.targetprep.RootTargetPreparer"/>
     <target_preparer class="com.android.tradefed.targetprep.PushFilePreparer">
         <option name="cleanup" value="true" />
         <option name="push" value="netd_unit_test->/data/local/tmp/netd_unit_test" />
-- 
cgit v1.2.3


From 91945c82ec4e8eda4cdd85b77029cceccfa11769 Mon Sep 17 00:00:00 2001
From: Luke Huang <huangluke@google.com>
Date: Tue, 7 Jul 2020 23:11:05 +0800
Subject: Backport: Fix the side channel attack by using async 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
Because DnsResolver had been moved to another git project in R,
use aosp/1302595 as Merged-In tag to avoid conflict.
Merged-In: I1ff2dc09f41f76973c5f066b07b15388e722b375

Change-Id: Ic0ef9b22bb5992b083bebc4f530acd63a02ac31c
---
 resolv/libnetd_resolv_test.cpp | 139 +++++++++++++++++++++++++++++++++++++++++
 resolv/res_cache.cpp           |   9 +--
 2 files changed, 142 insertions(+), 6 deletions(-)

diff --git a/resolv/libnetd_resolv_test.cpp b/resolv/libnetd_resolv_test.cpp
index 7fc5669a..df4521b6 100644
--- a/resolv/libnetd_resolv_test.cpp
+++ b/resolv/libnetd_resolv_test.cpp
@@ -19,17 +19,23 @@
 #include <gtest/gtest.h>
 
 #include <android-base/stringprintf.h>
+#include <android/multinetwork.h>  // ResNsendFlags
 #include <arpa/inet.h>
+#include <arpa/nameser.h>
 #include <netdb.h>
+#include <resolv.h>
 
 #include "dns_responder.h"
 #include "getaddrinfo.h"
 #include "gethnamaddr.h"
+#include "res_send.h"
 #include "resolv_cache.h"
 
 // TODO: make this dynamic and stop depending on implementation details.
 constexpr unsigned int TEST_NETID = 30;
 
+constexpr int MAXPACKET = (8 * 1024);
+
 // Specifying 0 in ai_socktype or ai_protocol of struct addrinfo indicates that any type or
 // protocol can be returned by getaddrinfo().
 constexpr unsigned int ANY = 0;
@@ -459,6 +465,139 @@ TEST_F(GetAddrInfoForNetContextTest, ServerTimeout) {
     if (result) freeaddrinfo(result);
 }
 
+namespace {
+
+std::string toString(uint8_t* buf, int bufLen, int ipType) {
+    ns_msg handle;
+    int ancount, n = 0;
+    ns_rr rr;
+
+    if (ns_initparse((const uint8_t*)buf, bufLen, &handle) >= 0) {
+        ancount = ns_msg_count(handle, ns_s_an);
+        if (ns_parserr(&handle, ns_s_an, n, &rr) == 0) {
+            const uint8_t* rdata = ns_rr_rdata(rr);
+            char buffer[INET6_ADDRSTRLEN];
+            if (inet_ntop(ipType, (const char*)rdata, buffer, sizeof(buffer))) {
+                return buffer;
+            }
+        }
+    }
+    return "";
+}
+
+std::string resolvResNsendHelper(const android_net_context* _Nonnull netContext, const char* host,
+                                 int nsType, int flag) {
+    std::vector<uint8_t> ansBuf(MAXPACKET, 0);
+    int rcode = ns_r_noerror;
+    std::vector<uint8_t> msg(MAXPACKET, 0);
+    const int msgLen = res_mkquery(ns_o_query, host, ns_c_in, nsType, nullptr, 0, nullptr,
+                                   msg.data(), MAXPACKET);
+    const int nsendAns = resolv_res_nsend(netContext, msg.data(), msgLen, ansBuf.data(), MAXPACKET,
+                                          &rcode, static_cast<ResNsendFlags>(flag));
+    return toString(ansBuf.data(), nsendAns, (nsType == ns_t_a) ? AF_INET : AF_INET6);
+}
+
+}  // namespace
+
+TEST_F(GetAddrInfoForNetContextTest, Async_CacheFlags) {
+    constexpr char listen_addr[] = "127.0.0.3";
+    constexpr char listen_srv[] = "53";
+    constexpr char host1[] = "howdy.example.com.";
+    constexpr char host2[] = "howdy.example2.com.";
+    constexpr char host3[] = "howdy.example3.com.";
+    test::DNSResponder dns(listen_addr, listen_srv, 250, ns_rcode::ns_r_servfail);
+    dns.addMapping(host1, ns_type::ns_t_a, "1.2.3.4");
+    dns.addMapping(host3, ns_type::ns_t_a, "1.2.3.6");
+    dns.addMapping(host2, ns_type::ns_t_aaaa, "::1.2.3.5");
+    dns.addMapping(host3, ns_type::ns_t_aaaa, "::1.2.3.6");
+    ASSERT_TRUE(dns.startServer());
+    const char* servers[] = {listen_addr};
+    ASSERT_EQ(0, resolv_set_nameservers_for_net(TEST_NETID, servers,
+                                                sizeof(servers) / sizeof(servers[0]),
+                                                mDefaultSearchDomains, &mDefaultParams_Binder));
+
+    EXPECT_EQ(resolvResNsendHelper(&mNetcontext, host1, ns_t_a, ANDROID_RESOLV_NO_CACHE_STORE),
+              "1.2.3.4");
+    EXPECT_EQ(resolvResNsendHelper(&mNetcontext, host1, ns_t_a, ANDROID_RESOLV_NO_CACHE_STORE),
+              "1.2.3.4");
+    EXPECT_EQ(resolvResNsendHelper(&mNetcontext, host1, ns_t_a, ANDROID_RESOLV_NO_CACHE_STORE),
+              "1.2.3.4");
+
+    // No cache exists, expect 3 queries
+    EXPECT_EQ(3U, GetNumQueries(dns, host1));  // no query because of the cache
+
+    // Raise a query with no flags to ensure no cache exists. Also make an cache entry for the
+    // query.
+    EXPECT_EQ(resolvResNsendHelper(&mNetcontext, host1, ns_t_a, 0), "1.2.3.4");
+
+    // Expect 4 queries because there should be no cache before this query.
+    EXPECT_EQ(4U, GetNumQueries(dns, host1));
+
+    // 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.
+    EXPECT_EQ(resolvResNsendHelper(&mNetcontext, host1, ns_t_a, ANDROID_RESOLV_NO_CACHE_STORE),
+              "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, host1));
+
+    // ANDROID_RESOLV_NO_CACHE_LOOKUP
+    EXPECT_EQ(resolvResNsendHelper(&mNetcontext, host1, ns_t_a, ANDROID_RESOLV_NO_CACHE_LOOKUP),
+              "1.2.3.4");
+    EXPECT_EQ(resolvResNsendHelper(&mNetcontext, host1, ns_t_a, ANDROID_RESOLV_NO_CACHE_LOOKUP),
+              "1.2.3.4");
+
+    // Cache was skipped, expect 2 more queries.
+    EXPECT_EQ(7U, GetNumQueries(dns, host1));
+
+    // Re-query verify cache works
+    EXPECT_EQ(resolvResNsendHelper(&mNetcontext, host1, ns_t_a, 0), "1.2.3.4");
+
+    // Cache hits,  expect still 7 queries
+    EXPECT_EQ(7U, GetNumQueries(dns, host1));
+
+    // Start to verify if ANDROID_RESOLV_NO_CACHE_LOOKUP does write response into cache
+    dns.clearQueries();
+    EXPECT_EQ(resolvResNsendHelper(&mNetcontext, host2, ns_t_aaaa, ANDROID_RESOLV_NO_CACHE_LOOKUP),
+              "::1.2.3.5");
+    EXPECT_EQ(resolvResNsendHelper(&mNetcontext, host2, ns_t_aaaa, ANDROID_RESOLV_NO_CACHE_LOOKUP),
+              "::1.2.3.5");
+
+    // Skip cache, expect 2 queries
+    EXPECT_EQ(2U, GetNumQueries(dns, host2));
+
+    // Re-query without flags
+    EXPECT_EQ(resolvResNsendHelper(&mNetcontext, host2, ns_t_aaaa, 0), "::1.2.3.5");
+    EXPECT_EQ(resolvResNsendHelper(&mNetcontext, host2, ns_t_aaaa, 0), "::1.2.3.5");
+
+    // Cache hits, expect still 2 queries
+    EXPECT_EQ(2U, GetNumQueries(dns, host2));
+
+    // Test both ANDROID_RESOLV_NO_CACHE_STORE and ANDROID_RESOLV_NO_CACHE_LOOKUP are set
+    dns.clearQueries();
+
+    // Make sure that the cache of "howdy.example3.com" exists.
+    EXPECT_EQ(resolvResNsendHelper(&mNetcontext, host3, ns_t_aaaa, 0), "::1.2.3.6");
+    EXPECT_EQ(1U, GetNumQueries(dns, host3));
+
+    // Re-query with testFlags
+    const int testFlag = ANDROID_RESOLV_NO_CACHE_STORE | ANDROID_RESOLV_NO_CACHE_LOOKUP;
+    EXPECT_EQ(resolvResNsendHelper(&mNetcontext, host3, ns_t_aaaa, testFlag), "::1.2.3.6");
+    // Expect cache lookup is skipped.
+    EXPECT_EQ(2U, GetNumQueries(dns, host3));
+
+    // Do another query with testFlags
+    EXPECT_EQ(resolvResNsendHelper(&mNetcontext, host3, ns_t_a, testFlag), "1.2.3.6");
+    // Expect cache lookup is skipped.
+    EXPECT_EQ(3U, GetNumQueries(dns, host3));
+
+    // Re-query with no flags
+    EXPECT_EQ(resolvResNsendHelper(&mNetcontext, host3, ns_t_a, testFlag), "1.2.3.6");
+    // Expect no cache hit because cache storing is also skipped in previous query.
+    EXPECT_EQ(4U, GetNumQueries(dns, host3));
+}
+
 TEST_F(GetHostByNameForNetContextTest, AlphabeticalHostname) {
     constexpr char listen_addr[] = "127.0.0.3";
     constexpr char listen_srv[] = "53";
diff --git a/resolv/res_cache.cpp b/resolv/res_cache.cpp
index f0ff5641..b4f0340d 100644
--- a/resolv/res_cache.cpp
+++ b/resolv/res_cache.cpp
@@ -1415,7 +1415,9 @@ ResolvCacheStatus _resolv_cache_lookup(unsigned netid, const void* query, int qu
     // 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;
@@ -1448,11 +1450,6 @@ ResolvCacheStatus _resolv_cache_lookup(unsigned netid, const void* query, int qu
 
     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;
 
-- 
cgit v1.2.3


From ac72fc56126dbc1d65592325ef21080bdf487524 Mon Sep 17 00:00:00 2001
From: Luke Huang <huangluke@google.com>
Date: Tue, 6 Oct 2020 14:09:29 +0800
Subject: Fix build break in libnetd_resolv_test

Bug: 12764641
Test: atest
Merged-In: I1ff2dc09f41f76973c5f066b07b15388e722b375
Change-Id: Iddcbc600b7c73d7910e937cb63d5adf20bbc54ac
---
 resolv/libnetd_resolv_test.cpp | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/resolv/libnetd_resolv_test.cpp b/resolv/libnetd_resolv_test.cpp
index ddbd126a..f961e790 100644
--- a/resolv/libnetd_resolv_test.cpp
+++ b/resolv/libnetd_resolv_test.cpp
@@ -506,8 +506,9 @@ std::string resolvResNsendHelper(const android_net_context* _Nonnull netContext,
     std::vector<uint8_t> msg(MAXPACKET, 0);
     const int msgLen = res_mkquery(ns_o_query, host, ns_c_in, nsType, nullptr, 0, nullptr,
                                    msg.data(), MAXPACKET);
+    NetworkDnsEventReported event;
     const int nsendAns = resolv_res_nsend(netContext, msg.data(), msgLen, ansBuf.data(), MAXPACKET,
-                                          &rcode, static_cast<ResNsendFlags>(flag));
+                                          &rcode, static_cast<ResNsendFlags>(flag), &event);
     return toString(ansBuf.data(), nsendAns, (nsType == ns_t_a) ? AF_INET : AF_INET6);
 }
 
-- 
cgit v1.2.3