Skip the legacy stats recording if DNS requests are denied by network policy
In order to prevent the false INTERNAL_ERROR event which is caused by
network policy fill up the legacy DNS stats, skip adding new legacy DNS
stats when we got errno == EPERM.
Bug: 151166599
Test: atest
Merged-In: I4c1d2a5c9f43650a5346a5ee0bb88eb36f850096
Change-Id: I5486638556c0a8fdfabbc37a1b29ed2b42f9b399
(cherry picked from commit 3991974b8cf5671b49f39531a6deb9f6fe81ebf0)
diff --git a/tests/Android.bp b/tests/Android.bp
index efdcc6f..4a258cb 100644
--- a/tests/Android.bp
+++ b/tests/Android.bp
@@ -11,8 +11,10 @@
"libutils",
],
static_libs: [
+ "netd_aidl_interface-ndk_platform",
"libnetd_test_dnsresponder_ndk",
"libnetdutils",
+ "libgmock",
],
}
diff --git a/tests/resolv_integration_test.cpp b/tests/resolv_integration_test.cpp
index a2a694e..bf26174 100644
--- a/tests/resolv_integration_test.cpp
+++ b/tests/resolv_integration_test.cpp
@@ -73,9 +73,6 @@
constexpr int TEST_VPN_NETID = 65502;
constexpr int MAXPACKET = (8 * 1024);
-// Use maximum reserved appId for applications to avoid conflict with existing uids.
-static const int TEST_UID = 99999;
-
// Semi-public Bionic hook used by the NDK (frameworks/base/native/android/net.c)
// Tested here for convenience.
extern "C" int android_getaddrinfofornet(const char* hostname, const char* servname,
@@ -4004,24 +4001,7 @@
dns1.clearQueries();
dns2.clearQueries();
- // Add drop rule for TEST_UID. Also enable the standby chain because it might not be enabled.
- // Unfortunately we cannot use FIREWALL_CHAIN_NONE, or custom iptables rules, for this purpose
- // because netd calls fchown() on the DNS query sockets, and "iptables -m owner" matches the
- // UID of the socket creator, not the UID set by fchown().
- //
- // TODO: migrate FIREWALL_CHAIN_NONE to eBPF as well.
- EXPECT_TRUE(netdService->firewallEnableChildChain(INetd::FIREWALL_CHAIN_STANDBY, true).isOk());
- EXPECT_TRUE(netdService
- ->firewallSetUidRule(INetd::FIREWALL_CHAIN_STANDBY, TEST_UID,
- INetd::FIREWALL_RULE_DENY)
- .isOk());
-
- // Save uid
- int suid = getuid();
-
- // Switch to TEST_UID
- EXPECT_TRUE(seteuid(TEST_UID) == 0);
-
+ ScopeBlockedUIDRule scopeBlockUidRule(netdService, TEST_UID);
// Dns Query
int fd1 = resNetworkQuery(TEST_NETID, host_name, ns_c_in, ns_t_a, 0);
int fd2 = resNetworkQuery(TEST_NETID, host_name, ns_c_in, ns_t_aaaa, 0);
@@ -4036,16 +4016,6 @@
memset(buf, 0, MAXPACKET);
res = getAsyncResponse(fd1, &rcode, buf, MAXPACKET);
EXPECT_EQ(-ECONNREFUSED, res);
-
- // Restore uid
- EXPECT_TRUE(seteuid(suid) == 0);
-
- // Remove drop rule for TEST_UID, and disable the standby chain.
- EXPECT_TRUE(netdService
- ->firewallSetUidRule(INetd::FIREWALL_CHAIN_STANDBY, TEST_UID,
- INetd::FIREWALL_RULE_ALLOW)
- .isOk());
- EXPECT_TRUE(netdService->firewallEnableChildChain(INetd::FIREWALL_CHAIN_STANDBY, false).isOk());
}
namespace {
@@ -4996,3 +4966,38 @@
EXPECT_GT(PARALLEL_LOOKUP_SLEEP_TIME_MS, timeTakenMs);
EXPECT_EQ(0U, GetNumQueries(dns, kHelloExampleCom));
}
+
+TEST_F(ResolverTest, BlockDnsQueryUidDoesNotLeadToBadServer) {
+ // This test relies on blocking traffic on loopback, which xt_qtaguid does not do.
+ // See aosp/358413 and b/34444781 for why.
+ SKIP_IF_BPF_NOT_SUPPORTED;
+
+ constexpr char listen_addr1[] = "127.0.0.4";
+ constexpr char listen_addr2[] = "::1";
+ test::DNSResponder dns1(listen_addr1);
+ test::DNSResponder dns2(listen_addr2);
+ StartDns(dns1, {});
+ StartDns(dns2, {});
+
+ std::vector<std::string> servers = {listen_addr1, listen_addr2};
+ ASSERT_TRUE(mDnsClient.SetResolversForNetwork(servers));
+ dns1.clearQueries();
+ dns2.clearQueries();
+ {
+ ScopeBlockedUIDRule scopeBlockUidRule(mDnsClient.netdService(), TEST_UID);
+ // Start querying ten times.
+ for (int i = 0; i < 10; i++) {
+ std::string hostName = fmt::format("blocked{}.com", i);
+ const addrinfo hints = {.ai_family = AF_INET, .ai_socktype = SOCK_DGRAM};
+ EXPECT_EQ(safe_getaddrinfo(hostName.c_str(), nullptr, &hints), nullptr);
+ }
+ }
+ // Since all query packets are blocked, we should not see any stats of them.
+ const std::vector<NameserverStats> expectedEmptyDnsStats = {
+ NameserverStats(listen_addr1),
+ NameserverStats(listen_addr2),
+ };
+ expectStatsFromGetResolverInfo(expectedEmptyDnsStats);
+ EXPECT_EQ(dns1.queries().size(), 0U);
+ EXPECT_EQ(dns2.queries().size(), 0U);
+}
diff --git a/tests/resolv_test_utils.h b/tests/resolv_test_utils.h
index 2177510..21d2936 100644
--- a/tests/resolv_test_utils.h
+++ b/tests/resolv_test_utils.h
@@ -23,10 +23,45 @@
#include <string>
#include <vector>
+#include <aidl/android/net/INetd.h>
+#include <gtest/gtest.h>
#include <netdutils/InternetAddresses.h>
#include "dns_responder/dns_responder.h"
+class ScopeBlockedUIDRule {
+ using INetd = aidl::android::net::INetd;
+
+ public:
+ ScopeBlockedUIDRule(INetd* netSrv, uid_t testUid)
+ : mNetSrv(netSrv), mTestUid(testUid), mSavedUid(getuid()) {
+ // Add drop rule for testUid. Also enable the standby chain because it might not be
+ // enabled. Unfortunately we cannot use FIREWALL_CHAIN_NONE, or custom iptables rules, for
+ // this purpose because netd calls fchown() on the DNS query sockets, and "iptables -m
+ // owner" matches the UID of the socket creator, not the UID set by fchown().
+ // TODO: migrate FIREWALL_CHAIN_NONE to eBPF as well.
+ EXPECT_TRUE(mNetSrv->firewallEnableChildChain(INetd::FIREWALL_CHAIN_STANDBY, true).isOk());
+ EXPECT_TRUE(mNetSrv->firewallSetUidRule(INetd::FIREWALL_CHAIN_STANDBY, mTestUid,
+ INetd::FIREWALL_RULE_DENY)
+ .isOk());
+ EXPECT_TRUE(seteuid(mTestUid) == 0);
+ };
+ ~ScopeBlockedUIDRule() {
+ // Restore uid
+ EXPECT_TRUE(seteuid(mSavedUid) == 0);
+ // Remove drop rule for testUid, and disable the standby chain.
+ EXPECT_TRUE(mNetSrv->firewallSetUidRule(INetd::FIREWALL_CHAIN_STANDBY, mTestUid,
+ INetd::FIREWALL_RULE_ALLOW)
+ .isOk());
+ EXPECT_TRUE(mNetSrv->firewallEnableChildChain(INetd::FIREWALL_CHAIN_STANDBY, false).isOk());
+ }
+
+ private:
+ INetd* mNetSrv;
+ const uid_t mTestUid;
+ const uid_t mSavedUid;
+};
+
struct DnsRecord {
std::string host_name; // host name
ns_type type; // record type
@@ -35,6 +70,8 @@
// TODO: make this dynamic and stop depending on implementation details.
constexpr int TEST_NETID = 30;
+// Use maximum reserved appId for applications to avoid conflict with existing uids.
+constexpr int TEST_UID = 99999;
static constexpr char kLocalHost[] = "localhost";
static constexpr char kLocalHostAddr[] = "127.0.0.1";