From 452f032db3d9def1ff496ee51f0dc09aa1b1b3ac Mon Sep 17 00:00:00 2001 From: Ken Chen Date: Thu, 15 Jun 2023 17:46:16 +0800 Subject: Fix use-after-free in DNS64 discovery thread DNS64 discovery thread is detached from binder requesting thread. But the discovery thread references resources not belongs to itself, which can be destroyed in dnsresolver destruction. Holds a strong pointer of Dns64Configuration in DNS64 discovery thread so that the instance of Dns64Configuration will keep until the DNS64 thread is force terminated. Ignore-AOSP-First: Fix security vulnerability Bug: 278303745 Test: atest Merged-In: Id74ea4e6f54a00805d3cc8a9d7e15e58a473b7d3 Change-Id: Id74ea4e6f54a00805d3cc8a9d7e15e58a473b7d3 (cherry picked from commit 254115584ff558fb87ee6ec5f5bb043f76219910) --- Android.bp | 1 + Dns64Configuration.cpp | 17 ++++++++++------- Dns64Configuration.h | 3 ++- ResolverController.cpp | 14 +++++++------- ResolverController.h | 6 +++--- tests/Android.bp | 15 +++++++++++++-- 6 files changed, 36 insertions(+), 20 deletions(-) diff --git a/Android.bp b/Android.bp index beead98b..5b3833c2 100644 --- a/Android.bp +++ b/Android.bp @@ -231,6 +231,7 @@ cc_library { "libstatslog_resolv", "libstatspush_compat", "libsysutils", + "libutils", "netd_event_listener_interface-lateststable-ndk", "server_configurable_flags", "stats_proto", diff --git a/Dns64Configuration.cpp b/Dns64Configuration.cpp index a1fe8717..fc1428db 100644 --- a/Dns64Configuration.cpp +++ b/Dns64Configuration.cpp @@ -24,6 +24,7 @@ #include #include #include +#include #include #include @@ -36,6 +37,7 @@ namespace android { +using android::sp; using netdutils::DumpWriter; using netdutils::IPAddress; using netdutils::IPPrefix; @@ -61,8 +63,9 @@ void Dns64Configuration::startPrefixDiscovery(unsigned netId) { // Emplace a copy of |cfg| in the map. mDns64Configs.emplace(std::make_pair(netId, cfg)); + const sp thiz = sp::fromExisting(this); // Note that capturing |cfg| in this lambda creates a copy. - std::thread discovery_thread([this, cfg, netId] { + std::thread discovery_thread([thiz, cfg, netId] { setThreadName(fmt::format("Nat64Pfx_{}", netId)); // Make a mutable copy rather than mark the whole lambda mutable. @@ -75,28 +78,28 @@ void Dns64Configuration::startPrefixDiscovery(unsigned netId) { .build(); while (true) { - if (!this->shouldContinueDiscovery(evalCfg)) break; + if (!thiz->shouldContinueDiscovery(evalCfg)) break; android_net_context netcontext{}; - mGetNetworkContextCallback(evalCfg.netId, 0, &netcontext); + thiz->mGetNetworkContextCallback(evalCfg.netId, 0, &netcontext); // Prefix discovery must bypass private DNS because in strict mode // the server generally won't know the NAT64 prefix. netcontext.flags |= NET_CONTEXT_FLAG_USE_LOCAL_NAMESERVERS; if (doRfc7050PrefixDiscovery(netcontext, &evalCfg)) { - this->recordDns64Config(evalCfg); + thiz->recordDns64Config(evalCfg); break; } - if (!this->shouldContinueDiscovery(evalCfg)) break; + if (!thiz->shouldContinueDiscovery(evalCfg)) break; if (!backoff.hasNextTimeout()) break; { - std::unique_lock cvGuard(mMutex); + std::unique_lock cvGuard(thiz->mMutex); // TODO: Consider some chrono math, combined with wait_until() // perhaps, to prevent early re-resolves from the removal of // other netids with IPv6-only nameservers. - mCv.wait_for(cvGuard, backoff.getNextTimeout()); + thiz->mCv.wait_for(cvGuard, backoff.getNextTimeout()); } } }); diff --git a/Dns64Configuration.h b/Dns64Configuration.h index 387de785..ebc63dbe 100644 --- a/Dns64Configuration.h +++ b/Dns64Configuration.h @@ -26,6 +26,7 @@ #include #include #include +#include struct android_net_context; @@ -47,7 +48,7 @@ namespace net { * Thread-safety: All public methods in this class MUST be thread-safe. * (In other words: this class handles all its locking privately.) */ -class Dns64Configuration { +class Dns64Configuration : virtual public RefBase { public: // Simple data struct for passing back packet NAT64 prefix event information to the // Dns64PrefixCallback callback. diff --git a/ResolverController.cpp b/ResolverController.cpp index 7fe01d48..66361143 100644 --- a/ResolverController.cpp +++ b/ResolverController.cpp @@ -155,11 +155,11 @@ int getDnsInfo(unsigned netId, std::vector* servers, std::vector::make( [](uint32_t netId, uint32_t uid, android_net_context* netcontext) { gResNetdCallbacks.get_network_context(netId, uid, netcontext); }, - std::bind(sendNat64PrefixEvent, std::placeholders::_1)) {} + std::bind(sendNat64PrefixEvent, std::placeholders::_1))) {} void ResolverController::destroyNetworkCache(unsigned netId) { LOG(VERBOSE) << __func__ << ": netId = " << netId; @@ -173,7 +173,7 @@ void ResolverController::destroyNetworkCache(unsigned netId) { event.network_type(), event.private_dns_modes(), bytesField); resolv_delete_cache_for_net(netId); - mDns64Configuration.stopPrefixDiscovery(netId); + mDns64Configuration->stopPrefixDiscovery(netId); privateDnsConfiguration.clear(netId); // Don't get this instance in PrivateDnsConfiguration. It's probe to deadlock. @@ -276,16 +276,16 @@ int ResolverController::getResolverInfo(int32_t netId, std::vector* } void ResolverController::startPrefix64Discovery(int32_t netId) { - mDns64Configuration.startPrefixDiscovery(netId); + mDns64Configuration->startPrefixDiscovery(netId); } void ResolverController::stopPrefix64Discovery(int32_t netId) { - return mDns64Configuration.stopPrefixDiscovery(netId); + return mDns64Configuration->stopPrefixDiscovery(netId); } // TODO: use StatusOr to wrap the result. int ResolverController::getPrefix64(unsigned netId, netdutils::IPPrefix* prefix) { - netdutils::IPPrefix p = mDns64Configuration.getPrefix64(netId); + netdutils::IPPrefix p = mDns64Configuration->getPrefix64(netId); if (p.family() != AF_INET6 || p.length() == 0) { return -ENOENT; } @@ -352,7 +352,7 @@ void ResolverController::dump(DumpWriter& dw, unsigned netId) { params.max_samples, params.base_timeout_msec, params.retry_count); } - mDns64Configuration.dump(dw, netId); + mDns64Configuration->dump(dw, netId); const auto privateDnsStatus = PrivateDnsConfiguration::getInstance().getStatus(netId); dw.println("Private DNS mode: %s", getPrivateDnsModeString(privateDnsStatus.mode)); if (privateDnsStatus.dotServersMap.size() == 0) { diff --git a/ResolverController.h b/ResolverController.h index 0af830ee..b74cff92 100644 --- a/ResolverController.h +++ b/ResolverController.h @@ -55,10 +55,10 @@ class ResolverController { // Set or clear a NAT64 prefix discovered by other sources (e.g., RA). int setPrefix64(unsigned netId, const netdutils::IPPrefix& prefix) { - return mDns64Configuration.setPrefix64(netId, prefix); + return mDns64Configuration->setPrefix64(netId, prefix); } - int clearPrefix64(unsigned netId) { return mDns64Configuration.clearPrefix64(netId); } + int clearPrefix64(unsigned netId) { return mDns64Configuration->clearPrefix64(netId); } // Return the current NAT64 prefix network, regardless of how it was discovered. int getPrefix64(unsigned netId, netdutils::IPPrefix* prefix); @@ -66,7 +66,7 @@ class ResolverController { void dump(netdutils::DumpWriter& dw, unsigned netId); private: - Dns64Configuration mDns64Configuration; + android::sp mDns64Configuration; }; } // namespace net } // namespace android diff --git a/tests/Android.bp b/tests/Android.bp index 40b909f8..1923e5ad 100644 --- a/tests/Android.bp +++ b/tests/Android.bp @@ -269,6 +269,7 @@ cc_test { "libstatslog_resolv", "libstatspush_compat", "libsysutils", + "libutils", "resolv_stats_test_utils", "server_configurable_flags", "stats_proto", @@ -372,6 +373,18 @@ cc_defaults { "server_configurable_flags", "stats_proto", ], + target: { + android: { + shared_libs: [ + "libutils", + ], + }, + host: { + static_libs: [ + "libutils", + ], + }, + }, fuzz_config: { cc: [ "cken@google.com", @@ -416,14 +429,12 @@ cc_fuzz { shared_libs: [ "libbinder_ndk", "libbinder", - "libutils", ], }, host: { static_libs: [ "libbinder_ndk", "libbinder", - "libutils", ], }, darwin: { -- cgit v1.2.3 From cad5a8c884345689f5f2a1b6679c89524b066d15 Mon Sep 17 00:00:00 2001 From: Ken Chen Date: Thu, 15 Jun 2023 17:46:16 +0800 Subject: Fix use-after-free in DNS64 discovery thread DNS64 discovery thread is detached from binder requesting thread. But the discovery thread references resources not belongs to itself, which can be destroyed in dnsresolver destruction. Holds a strong pointer of Dns64Configuration in DNS64 discovery thread so that the instance of Dns64Configuration will keep until the DNS64 thread is force terminated. Ignore-AOSP-First: Fix security vulnerability Bug: 278303745 Test: m, fuzzing Fuzzing: mma resolv_service_fuzzer && adb sync data && adb shell /data/fuzz/arm64/resolv_service_fuzzer/resolv_service_fuzzer Change-Id: Id74ea4e6f54a00805d3cc8a9d7e15e58a473b7d3 Merged-In: Id74ea4e6f54a00805d3cc8a9d7e15e58a473b7d3 (cherry picked from commit 254115584ff558fb87ee6ec5f5bb043f76219910) --- Android.bp | 1 + Dns64Configuration.cpp | 17 ++++++++++------- Dns64Configuration.h | 3 ++- ResolverController.cpp | 15 +++++++-------- ResolverController.h | 6 +++--- 5 files changed, 23 insertions(+), 19 deletions(-) diff --git a/Android.bp b/Android.bp index db610a63..86af506a 100644 --- a/Android.bp +++ b/Android.bp @@ -136,6 +136,7 @@ cc_library { "libstatspush_compat", "libsysutils", "netd_event_listener_interface-ndk_platform", + "libutils", "server_configurable_flags", "stats_proto", ], diff --git a/Dns64Configuration.cpp b/Dns64Configuration.cpp index 77a27e79..94abedae 100644 --- a/Dns64Configuration.cpp +++ b/Dns64Configuration.cpp @@ -24,6 +24,7 @@ #include #include #include +#include #include #include @@ -38,6 +39,7 @@ namespace android { using android::base::StringPrintf; using android::net::NetworkDnsEventReported; +using android::sp; using netdutils::DumpWriter; using netdutils::IPAddress; using netdutils::IPPrefix; @@ -63,8 +65,9 @@ void Dns64Configuration::startPrefixDiscovery(unsigned netId) { // Emplace a copy of |cfg| in the map. mDns64Configs.emplace(std::make_pair(netId, cfg)); + const sp thiz = this; // Note that capturing |cfg| in this lambda creates a copy. - std::thread discovery_thread([this, cfg, netId] { + std::thread discovery_thread([thiz, cfg, netId] { setThreadName(StringPrintf("Nat64Pfx_%u", netId).c_str()); // Make a mutable copy rather than mark the whole lambda mutable. @@ -77,28 +80,28 @@ void Dns64Configuration::startPrefixDiscovery(unsigned netId) { .build(); while (true) { - if (!this->shouldContinueDiscovery(evalCfg)) break; + if (!thiz->shouldContinueDiscovery(evalCfg)) break; android_net_context netcontext{}; - mGetNetworkContextCallback(evalCfg.netId, 0, &netcontext); + thiz->mGetNetworkContextCallback(evalCfg.netId, 0, &netcontext); // Prefix discovery must bypass private DNS because in strict mode // the server generally won't know the NAT64 prefix. netcontext.flags |= NET_CONTEXT_FLAG_USE_LOCAL_NAMESERVERS; if (doRfc7050PrefixDiscovery(netcontext, &evalCfg)) { - this->recordDns64Config(evalCfg); + thiz->recordDns64Config(evalCfg); break; } - if (!this->shouldContinueDiscovery(evalCfg)) break; + if (!thiz->shouldContinueDiscovery(evalCfg)) break; if (!backoff.hasNextTimeout()) break; { - std::unique_lock cvGuard(mMutex); + std::unique_lock cvGuard(thiz->mMutex); // TODO: Consider some chrono math, combined with wait_until() // perhaps, to prevent early re-resolves from the removal of // other netids with IPv6-only nameservers. - mCv.wait_for(cvGuard, backoff.getNextTimeout()); + thiz->mCv.wait_for(cvGuard, backoff.getNextTimeout()); } } }); diff --git a/Dns64Configuration.h b/Dns64Configuration.h index 387de785..ebc63dbe 100644 --- a/Dns64Configuration.h +++ b/Dns64Configuration.h @@ -26,6 +26,7 @@ #include #include #include +#include struct android_net_context; @@ -47,7 +48,7 @@ namespace net { * Thread-safety: All public methods in this class MUST be thread-safe. * (In other words: this class handles all its locking privately.) */ -class Dns64Configuration { +class Dns64Configuration : virtual public RefBase { public: // Simple data struct for passing back packet NAT64 prefix event information to the // Dns64PrefixCallback callback. diff --git a/ResolverController.cpp b/ResolverController.cpp index be0b989f..f4790837 100644 --- a/ResolverController.cpp +++ b/ResolverController.cpp @@ -173,17 +173,17 @@ int getDnsInfo(unsigned netId, std::vector* servers, std::vectordump(dw, netId); const auto privateDnsStatus = gPrivateDnsConfiguration.getStatus(netId); dw.println("Private DNS mode: %s", getPrivateDnsModeString(privateDnsStatus.mode)); if (privateDnsStatus.serversMap.size() == 0) { diff --git a/ResolverController.h b/ResolverController.h index e81e1edb..3802e36c 100644 --- a/ResolverController.h +++ b/ResolverController.h @@ -55,10 +55,10 @@ class ResolverController { // Set or clear a NAT64 prefix discovered by other sources (e.g., RA). int setPrefix64(unsigned netId, const netdutils::IPPrefix& prefix) { - return mDns64Configuration.setPrefix64(netId, prefix); + return mDns64Configuration->setPrefix64(netId, prefix); } - int clearPrefix64(unsigned netId) { return mDns64Configuration.clearPrefix64(netId); } + int clearPrefix64(unsigned netId) { return mDns64Configuration->clearPrefix64(netId); } // Return the current NAT64 prefix network, regardless of how it was discovered. int getPrefix64(unsigned netId, netdutils::IPPrefix* prefix); @@ -66,7 +66,7 @@ class ResolverController { void dump(netdutils::DumpWriter& dw, unsigned netId); private: - Dns64Configuration mDns64Configuration; + android::sp mDns64Configuration; }; } // namespace net } // namespace android -- cgit v1.2.3 From 87676fe7104d91428f126ba13ddf640b96c736ca Mon Sep 17 00:00:00 2001 From: Ken Chen Date: Thu, 15 Jun 2023 17:46:16 +0800 Subject: Fix use-after-free in DNS64 discovery thread DNS64 discovery thread is detached from binder requesting thread. But the discovery thread references resources not belongs to itself, which can be destroyed in dnsresolver destruction. Holds a strong pointer of Dns64Configuration in DNS64 discovery thread so that the instance of Dns64Configuration will keep until the DNS64 thread is force terminated. Ignore-AOSP-First: Fix security vulnerability Bug: 278303745 Test: m, fuzzing Fuzzing: mma resolv_service_fuzzer && adb sync data && adb shell /data/fuzz/arm64/resolv_service_fuzzer/resolv_service_fuzzer Change-Id: Id74ea4e6f54a00805d3cc8a9d7e15e58a473b7d3 Merged-In: Id74ea4e6f54a00805d3cc8a9d7e15e58a473b7d3 (cherry picked from commit 254115584ff558fb87ee6ec5f5bb043f76219910) --- Android.bp | 1 + Dns64Configuration.cpp | 17 ++++++++++------- Dns64Configuration.h | 3 ++- ResolverController.cpp | 15 +++++++-------- ResolverController.h | 6 +++--- 5 files changed, 23 insertions(+), 19 deletions(-) diff --git a/Android.bp b/Android.bp index 45425aa0..b0cfee0c 100644 --- a/Android.bp +++ b/Android.bp @@ -203,6 +203,7 @@ cc_library { "libstatspush_compat", "libsysutils", "netd_event_listener_interface-lateststable-ndk", + "libutils", "server_configurable_flags", "stats_proto", ], diff --git a/Dns64Configuration.cpp b/Dns64Configuration.cpp index a1fe8717..6e5ca5bd 100644 --- a/Dns64Configuration.cpp +++ b/Dns64Configuration.cpp @@ -24,6 +24,7 @@ #include #include #include +#include #include #include @@ -36,6 +37,7 @@ namespace android { +using android::sp; using netdutils::DumpWriter; using netdutils::IPAddress; using netdutils::IPPrefix; @@ -61,8 +63,9 @@ void Dns64Configuration::startPrefixDiscovery(unsigned netId) { // Emplace a copy of |cfg| in the map. mDns64Configs.emplace(std::make_pair(netId, cfg)); + const sp thiz = this; // Note that capturing |cfg| in this lambda creates a copy. - std::thread discovery_thread([this, cfg, netId] { + std::thread discovery_thread([thiz, cfg, netId] { setThreadName(fmt::format("Nat64Pfx_{}", netId)); // Make a mutable copy rather than mark the whole lambda mutable. @@ -75,28 +78,28 @@ void Dns64Configuration::startPrefixDiscovery(unsigned netId) { .build(); while (true) { - if (!this->shouldContinueDiscovery(evalCfg)) break; + if (!thiz->shouldContinueDiscovery(evalCfg)) break; android_net_context netcontext{}; - mGetNetworkContextCallback(evalCfg.netId, 0, &netcontext); + thiz->mGetNetworkContextCallback(evalCfg.netId, 0, &netcontext); // Prefix discovery must bypass private DNS because in strict mode // the server generally won't know the NAT64 prefix. netcontext.flags |= NET_CONTEXT_FLAG_USE_LOCAL_NAMESERVERS; if (doRfc7050PrefixDiscovery(netcontext, &evalCfg)) { - this->recordDns64Config(evalCfg); + thiz->recordDns64Config(evalCfg); break; } - if (!this->shouldContinueDiscovery(evalCfg)) break; + if (!thiz->shouldContinueDiscovery(evalCfg)) break; if (!backoff.hasNextTimeout()) break; { - std::unique_lock cvGuard(mMutex); + std::unique_lock cvGuard(thiz->mMutex); // TODO: Consider some chrono math, combined with wait_until() // perhaps, to prevent early re-resolves from the removal of // other netids with IPv6-only nameservers. - mCv.wait_for(cvGuard, backoff.getNextTimeout()); + thiz->mCv.wait_for(cvGuard, backoff.getNextTimeout()); } } }); diff --git a/Dns64Configuration.h b/Dns64Configuration.h index 387de785..ebc63dbe 100644 --- a/Dns64Configuration.h +++ b/Dns64Configuration.h @@ -26,6 +26,7 @@ #include #include #include +#include struct android_net_context; @@ -47,7 +48,7 @@ namespace net { * Thread-safety: All public methods in this class MUST be thread-safe. * (In other words: this class handles all its locking privately.) */ -class Dns64Configuration { +class Dns64Configuration : virtual public RefBase { public: // Simple data struct for passing back packet NAT64 prefix event information to the // Dns64PrefixCallback callback. diff --git a/ResolverController.cpp b/ResolverController.cpp index a430a08c..12bf7f92 100644 --- a/ResolverController.cpp +++ b/ResolverController.cpp @@ -156,17 +156,17 @@ int getDnsInfo(unsigned netId, std::vector* servers, std::vectorsetPrefix64(netId, prefix); } - int clearPrefix64(unsigned netId) { return mDns64Configuration.clearPrefix64(netId); } + int clearPrefix64(unsigned netId) { return mDns64Configuration->clearPrefix64(netId); } // Return the current NAT64 prefix network, regardless of how it was discovered. int getPrefix64(unsigned netId, netdutils::IPPrefix* prefix); @@ -66,7 +66,7 @@ class ResolverController { void dump(netdutils::DumpWriter& dw, unsigned netId); private: - Dns64Configuration mDns64Configuration; + android::sp mDns64Configuration; }; } // namespace net } // namespace android -- cgit v1.2.3 From 292b59e1b0ff91cd4b1f1835de7bfb7fb744c9c0 Mon Sep 17 00:00:00 2001 From: Ken Chen Date: Thu, 15 Jun 2023 17:46:16 +0800 Subject: Fix use-after-free in DNS64 discovery thread DNS64 discovery thread is detached from binder requesting thread. But the discovery thread references resources not belongs to itself, which can be destroyed in dnsresolver destruction. Holds a strong pointer of Dns64Configuration in DNS64 discovery thread so that the instance of Dns64Configuration will keep until the DNS64 thread is force terminated. Ignore-AOSP-First: Fix security vulnerability Bug: 278303745 Test: m, fuzzing Fuzzing: mma resolv_service_fuzzer && adb sync data && adb shell /data/fuzz/arm64/resolv_service_fuzzer/resolv_service_fuzzer Change-Id: Id74ea4e6f54a00805d3cc8a9d7e15e58a473b7d3 Merged-In: Id74ea4e6f54a00805d3cc8a9d7e15e58a473b7d3 (cherry picked from commit 254115584ff558fb87ee6ec5f5bb043f76219910) --- Android.bp | 1 + Dns64Configuration.cpp | 17 ++++++++++------- Dns64Configuration.h | 3 ++- ResolverController.cpp | 15 +++++++-------- ResolverController.h | 6 +++--- 5 files changed, 23 insertions(+), 19 deletions(-) diff --git a/Android.bp b/Android.bp index 696d7215..b46aeff6 100644 --- a/Android.bp +++ b/Android.bp @@ -201,6 +201,7 @@ cc_library { "libstatspush_compat", "libsysutils", "netd_event_listener_interface-lateststable-ndk_platform", + "libutils", "server_configurable_flags", "stats_proto", ], diff --git a/Dns64Configuration.cpp b/Dns64Configuration.cpp index 77a27e79..94abedae 100644 --- a/Dns64Configuration.cpp +++ b/Dns64Configuration.cpp @@ -24,6 +24,7 @@ #include #include #include +#include #include #include @@ -38,6 +39,7 @@ namespace android { using android::base::StringPrintf; using android::net::NetworkDnsEventReported; +using android::sp; using netdutils::DumpWriter; using netdutils::IPAddress; using netdutils::IPPrefix; @@ -63,8 +65,9 @@ void Dns64Configuration::startPrefixDiscovery(unsigned netId) { // Emplace a copy of |cfg| in the map. mDns64Configs.emplace(std::make_pair(netId, cfg)); + const sp thiz = this; // Note that capturing |cfg| in this lambda creates a copy. - std::thread discovery_thread([this, cfg, netId] { + std::thread discovery_thread([thiz, cfg, netId] { setThreadName(StringPrintf("Nat64Pfx_%u", netId).c_str()); // Make a mutable copy rather than mark the whole lambda mutable. @@ -77,28 +80,28 @@ void Dns64Configuration::startPrefixDiscovery(unsigned netId) { .build(); while (true) { - if (!this->shouldContinueDiscovery(evalCfg)) break; + if (!thiz->shouldContinueDiscovery(evalCfg)) break; android_net_context netcontext{}; - mGetNetworkContextCallback(evalCfg.netId, 0, &netcontext); + thiz->mGetNetworkContextCallback(evalCfg.netId, 0, &netcontext); // Prefix discovery must bypass private DNS because in strict mode // the server generally won't know the NAT64 prefix. netcontext.flags |= NET_CONTEXT_FLAG_USE_LOCAL_NAMESERVERS; if (doRfc7050PrefixDiscovery(netcontext, &evalCfg)) { - this->recordDns64Config(evalCfg); + thiz->recordDns64Config(evalCfg); break; } - if (!this->shouldContinueDiscovery(evalCfg)) break; + if (!thiz->shouldContinueDiscovery(evalCfg)) break; if (!backoff.hasNextTimeout()) break; { - std::unique_lock cvGuard(mMutex); + std::unique_lock cvGuard(thiz->mMutex); // TODO: Consider some chrono math, combined with wait_until() // perhaps, to prevent early re-resolves from the removal of // other netids with IPv6-only nameservers. - mCv.wait_for(cvGuard, backoff.getNextTimeout()); + thiz->mCv.wait_for(cvGuard, backoff.getNextTimeout()); } } }); diff --git a/Dns64Configuration.h b/Dns64Configuration.h index 387de785..ebc63dbe 100644 --- a/Dns64Configuration.h +++ b/Dns64Configuration.h @@ -26,6 +26,7 @@ #include #include #include +#include struct android_net_context; @@ -47,7 +48,7 @@ namespace net { * Thread-safety: All public methods in this class MUST be thread-safe. * (In other words: this class handles all its locking privately.) */ -class Dns64Configuration { +class Dns64Configuration : virtual public RefBase { public: // Simple data struct for passing back packet NAT64 prefix event information to the // Dns64PrefixCallback callback. diff --git a/ResolverController.cpp b/ResolverController.cpp index 6c7bed49..ae89bcb3 100644 --- a/ResolverController.cpp +++ b/ResolverController.cpp @@ -155,17 +155,17 @@ int getDnsInfo(unsigned netId, std::vector* servers, std::vectordump(dw, netId); const auto privateDnsStatus = PrivateDnsConfiguration::getInstance().getStatus(netId); dw.println("Private DNS mode: %s", getPrivateDnsModeString(privateDnsStatus.mode)); if (privateDnsStatus.serversMap.size() == 0) { diff --git a/ResolverController.h b/ResolverController.h index e81e1edb..3802e36c 100644 --- a/ResolverController.h +++ b/ResolverController.h @@ -55,10 +55,10 @@ class ResolverController { // Set or clear a NAT64 prefix discovered by other sources (e.g., RA). int setPrefix64(unsigned netId, const netdutils::IPPrefix& prefix) { - return mDns64Configuration.setPrefix64(netId, prefix); + return mDns64Configuration->setPrefix64(netId, prefix); } - int clearPrefix64(unsigned netId) { return mDns64Configuration.clearPrefix64(netId); } + int clearPrefix64(unsigned netId) { return mDns64Configuration->clearPrefix64(netId); } // Return the current NAT64 prefix network, regardless of how it was discovered. int getPrefix64(unsigned netId, netdutils::IPPrefix* prefix); @@ -66,7 +66,7 @@ class ResolverController { void dump(netdutils::DumpWriter& dw, unsigned netId); private: - Dns64Configuration mDns64Configuration; + android::sp mDns64Configuration; }; } // namespace net } // namespace android -- cgit v1.2.3 From aa510cee9a3bc043dbd6c1ae1f32c9b6cabe7489 Mon Sep 17 00:00:00 2001 From: Ken Chen Date: Fri, 8 Sep 2023 14:36:27 +0000 Subject: [Code Health] Delete unused code. Change-Id: Ia8db6f9c186f72b47e661d2dda779143e4e67cfc --- ResolverController.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/ResolverController.cpp b/ResolverController.cpp index 7fe01d48..4295569d 100644 --- a/ResolverController.cpp +++ b/ResolverController.cpp @@ -192,8 +192,6 @@ int ResolverController::flushNetworkCache(unsigned netId) { } int ResolverController::setResolverConfiguration(const ResolverParamsParcel& resolverParams) { - using aidl::android::net::IDnsResolver; - if (!has_named_cache(resolverParams.netId)) { return -ENOENT; } -- cgit v1.2.3 From 592e8227b2ac8eda3685682fd4ed50d8ccf10c13 Mon Sep 17 00:00:00 2001 From: Ken Chen Date: Mon, 11 Sep 2023 04:16:37 +0000 Subject: [Code Health] Move 'using' out of functions Refactor only. No functionality change. Change-Id: I242af40d421b7b8da89253d6781d55b3f4784d85 --- ResolverController.cpp | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/ResolverController.cpp b/ResolverController.cpp index 4295569d..53eb3dc9 100644 --- a/ResolverController.cpp +++ b/ResolverController.cpp @@ -37,9 +37,11 @@ #include "stats.h" #include "util.h" +using aidl::android::net::IDnsResolver; using aidl::android::net::ResolverParamsParcel; using aidl::android::net::resolv::aidl::IDnsResolverUnsolicitedEventListener; using aidl::android::net::resolv::aidl::Nat64PrefixEventParcel; +using android::net::ResolverStats; namespace android { @@ -79,8 +81,6 @@ void sendNat64PrefixEvent(const Dns64Configuration::Nat64PrefixInfo& args) { int getDnsInfo(unsigned netId, std::vector* servers, std::vector* domains, res_params* params, std::vector* stats, int* wait_for_pending_req_timeout_count) { - using aidl::android::net::IDnsResolver; - using android::net::ResolverStats; static_assert(ResolverStats::STATS_SUCCESSES == IDnsResolver::RESOLVER_STATS_SUCCESSES && ResolverStats::STATS_ERRORS == IDnsResolver::RESOLVER_STATS_ERRORS && ResolverStats::STATS_TIMEOUTS == IDnsResolver::RESOLVER_STATS_TIMEOUTS && @@ -245,8 +245,6 @@ int ResolverController::getResolverInfo(int32_t netId, std::vector* std::vector* tlsServers, std::vector* params, std::vector* stats, int* wait_for_pending_req_timeout_count) { - using aidl::android::net::IDnsResolver; - using android::net::ResolverStats; res_params res_params; std::vector res_stats; int ret = getDnsInfo(netId, servers, domains, &res_params, &res_stats, @@ -293,7 +291,6 @@ int ResolverController::getPrefix64(unsigned netId, netdutils::IPPrefix* prefix) void ResolverController::dump(DumpWriter& dw, unsigned netId) { // No lock needed since Bionic's resolver locks all accessed data structures internally. - using android::net::ResolverStats; std::vector servers; std::vector domains; res_params params = {}; -- cgit v1.2.3 From 79f571069c4db536e7c1bfecbb50c926f0ef0548 Mon Sep 17 00:00:00 2001 From: Ken Chen Date: Thu, 15 Jun 2023 17:46:16 +0800 Subject: Fix use-after-free in DNS64 discovery thread DNS64 discovery thread is detached from binder requesting thread. But the discovery thread references resources not belongs to itself, which can be destroyed in dnsresolver destruction. Holds a strong pointer of Dns64Configuration in DNS64 discovery thread so that the instance of Dns64Configuration will keep until the DNS64 thread is force terminated. Ignore-AOSP-First: Fix security vulnerability Bug: 278303745 Test: m, fuzzing Fuzzing: mma resolv_service_fuzzer && adb sync data && adb shell /data/fuzz/arm64/resolv_service_fuzzer/resolv_service_fuzzer Change-Id: Id74ea4e6f54a00805d3cc8a9d7e15e58a473b7d3 Merged-In: Id74ea4e6f54a00805d3cc8a9d7e15e58a473b7d3 Merged-In: I3445f9ccdbb9746c0f78ad8b50e3193c6a1868a3 (cherry picked from commit 254115584ff558fb87ee6ec5f5bb043f76219910) --- Android.bp | 1 + Dns64Configuration.cpp | 17 ++++++++++------- Dns64Configuration.h | 3 ++- ResolverController.cpp | 15 +++++++-------- ResolverController.h | 6 +++--- 5 files changed, 23 insertions(+), 19 deletions(-) diff --git a/Android.bp b/Android.bp index 45425aa0..b0cfee0c 100644 --- a/Android.bp +++ b/Android.bp @@ -203,6 +203,7 @@ cc_library { "libstatspush_compat", "libsysutils", "netd_event_listener_interface-lateststable-ndk", + "libutils", "server_configurable_flags", "stats_proto", ], diff --git a/Dns64Configuration.cpp b/Dns64Configuration.cpp index a1fe8717..6e5ca5bd 100644 --- a/Dns64Configuration.cpp +++ b/Dns64Configuration.cpp @@ -24,6 +24,7 @@ #include #include #include +#include #include #include @@ -36,6 +37,7 @@ namespace android { +using android::sp; using netdutils::DumpWriter; using netdutils::IPAddress; using netdutils::IPPrefix; @@ -61,8 +63,9 @@ void Dns64Configuration::startPrefixDiscovery(unsigned netId) { // Emplace a copy of |cfg| in the map. mDns64Configs.emplace(std::make_pair(netId, cfg)); + const sp thiz = this; // Note that capturing |cfg| in this lambda creates a copy. - std::thread discovery_thread([this, cfg, netId] { + std::thread discovery_thread([thiz, cfg, netId] { setThreadName(fmt::format("Nat64Pfx_{}", netId)); // Make a mutable copy rather than mark the whole lambda mutable. @@ -75,28 +78,28 @@ void Dns64Configuration::startPrefixDiscovery(unsigned netId) { .build(); while (true) { - if (!this->shouldContinueDiscovery(evalCfg)) break; + if (!thiz->shouldContinueDiscovery(evalCfg)) break; android_net_context netcontext{}; - mGetNetworkContextCallback(evalCfg.netId, 0, &netcontext); + thiz->mGetNetworkContextCallback(evalCfg.netId, 0, &netcontext); // Prefix discovery must bypass private DNS because in strict mode // the server generally won't know the NAT64 prefix. netcontext.flags |= NET_CONTEXT_FLAG_USE_LOCAL_NAMESERVERS; if (doRfc7050PrefixDiscovery(netcontext, &evalCfg)) { - this->recordDns64Config(evalCfg); + thiz->recordDns64Config(evalCfg); break; } - if (!this->shouldContinueDiscovery(evalCfg)) break; + if (!thiz->shouldContinueDiscovery(evalCfg)) break; if (!backoff.hasNextTimeout()) break; { - std::unique_lock cvGuard(mMutex); + std::unique_lock cvGuard(thiz->mMutex); // TODO: Consider some chrono math, combined with wait_until() // perhaps, to prevent early re-resolves from the removal of // other netids with IPv6-only nameservers. - mCv.wait_for(cvGuard, backoff.getNextTimeout()); + thiz->mCv.wait_for(cvGuard, backoff.getNextTimeout()); } } }); diff --git a/Dns64Configuration.h b/Dns64Configuration.h index 387de785..ebc63dbe 100644 --- a/Dns64Configuration.h +++ b/Dns64Configuration.h @@ -26,6 +26,7 @@ #include #include #include +#include struct android_net_context; @@ -47,7 +48,7 @@ namespace net { * Thread-safety: All public methods in this class MUST be thread-safe. * (In other words: this class handles all its locking privately.) */ -class Dns64Configuration { +class Dns64Configuration : virtual public RefBase { public: // Simple data struct for passing back packet NAT64 prefix event information to the // Dns64PrefixCallback callback. diff --git a/ResolverController.cpp b/ResolverController.cpp index a430a08c..12bf7f92 100644 --- a/ResolverController.cpp +++ b/ResolverController.cpp @@ -156,17 +156,17 @@ int getDnsInfo(unsigned netId, std::vector* servers, std::vectorsetPrefix64(netId, prefix); } - int clearPrefix64(unsigned netId) { return mDns64Configuration.clearPrefix64(netId); } + int clearPrefix64(unsigned netId) { return mDns64Configuration->clearPrefix64(netId); } // Return the current NAT64 prefix network, regardless of how it was discovered. int getPrefix64(unsigned netId, netdutils::IPPrefix* prefix); @@ -66,7 +66,7 @@ class ResolverController { void dump(netdutils::DumpWriter& dw, unsigned netId); private: - Dns64Configuration mDns64Configuration; + android::sp mDns64Configuration; }; } // namespace net } // namespace android -- cgit v1.2.3 From 37ade6cca6e0cdb554e7c409ab9210c6a32ba84d Mon Sep 17 00:00:00 2001 From: Ken Chen Date: Fri, 15 Sep 2023 13:05:42 +0800 Subject: Add a function to query enforceDnsUid setting The "DNS query fail-fast when network access is restricted" feature needs to know whether the enforceDnsUid is set or not in DnsProxyListener. Bug: 288340533 Test: atest resolv_unit_test:ResolvCacheTest#IsEnforceDnsUidEnabled Change-Id: I8e7a5d5d030602eced05c6f7f3809a57bfabebc3 --- res_cache.cpp | 10 +++++++++- resolv_cache.h | 3 +++ tests/resolv_cache_unit_test.cpp | 32 ++++++++++++++++++++++++++++++++ 3 files changed, 44 insertions(+), 1 deletion(-) diff --git a/res_cache.cpp b/res_cache.cpp index 86def626..90bf5261 100644 --- a/res_cache.cpp +++ b/res_cache.cpp @@ -2101,4 +2101,12 @@ int resolv_get_max_cache_entries(unsigned netid) { return -1; } return info->cache->get_max_cache_entries(); -} \ No newline at end of file +} + +bool resolv_is_enforceDnsUid_enabled_network(unsigned netid) { + std::lock_guard guard(cache_mutex); + if (const auto info = find_netconfig_locked(netid); info != nullptr) { + return info->enforceDnsUid; + } + return false; +} diff --git a/resolv_cache.h b/resolv_cache.h index 07697666..f8437dee 100644 --- a/resolv_cache.h +++ b/resolv_cache.h @@ -143,3 +143,6 @@ void resolv_netconfig_dump(android::netdutils::DumpWriter& dw, unsigned netid); // Get the maximum cache size of a network. // Return positive value on success, -1 on failure. int resolv_get_max_cache_entries(unsigned netid); + +// Return true if the enforceDnsUid is enabled on the network. +bool resolv_is_enforceDnsUid_enabled_network(unsigned netid); diff --git a/tests/resolv_cache_unit_test.cpp b/tests/resolv_cache_unit_test.cpp index defd2da6..2306438e 100644 --- a/tests/resolv_cache_unit_test.cpp +++ b/tests/resolv_cache_unit_test.cpp @@ -928,6 +928,38 @@ TEST_F(ResolvCacheTest, GetResolverStats) { } } +TEST_F(ResolvCacheTest, IsEnforceDnsUidEnabled) { + const SetupParams unenforcedDnsUidCfg = { + .servers = {"127.0.0.1", "::127.0.0.2", "fe80::3"}, + .domains = {"domain1.com", "domain2.com"}, + .params = kParams, + }; + // Network #1 + EXPECT_EQ(0, cacheCreate(TEST_NETID)); + EXPECT_EQ(0, cacheSetupResolver(TEST_NETID, unenforcedDnsUidCfg)); + EXPECT_FALSE(resolv_is_enforceDnsUid_enabled_network(TEST_NETID)); + + // Network #2 + EXPECT_EQ(0, cacheCreate(TEST_NETID + 1)); + EXPECT_EQ(0, cacheSetupResolver(TEST_NETID + 1, unenforcedDnsUidCfg)); + EXPECT_FALSE(resolv_is_enforceDnsUid_enabled_network(TEST_NETID + 1)); + + // Change the enforceDnsUid setting on network #1 + const SetupParams enforcedDnsUidCfg = { + .servers = {"127.0.0.1", "::127.0.0.2", "fe80::3"}, + .domains = {"domain1.com", "domain2.com"}, + .params = kParams, + .resolverOptions = {.enforceDnsUid = true}, + }; + EXPECT_EQ(0, cacheSetupResolver(TEST_NETID, enforcedDnsUidCfg)); + EXPECT_TRUE(resolv_is_enforceDnsUid_enabled_network(TEST_NETID)); + + // Network #2 is unaffected + EXPECT_FALSE(resolv_is_enforceDnsUid_enabled_network(TEST_NETID + 1)); + + // Returns false on non-existent network + EXPECT_FALSE(resolv_is_enforceDnsUid_enabled_network(TEST_NETID + 2)); +} namespace { constexpr int EAI_OK = 0; -- cgit v1.2.3 From c3f77721fc51d70ddc65efbdd8c4923019762361 Mon Sep 17 00:00:00 2001 From: Ken Chen Date: Sat, 16 Sep 2023 02:05:06 +0800 Subject: Move doDns64Synthesis and doDns64ReverseLookup around It needs to do a DNS64 synthesis or DNS64 reverse lookup only if the requester can do a DNS query or a reversed lookup. Move DNS64 handlings right after DNS (and reversed) lookups. Bug: 288340533 Test: atest Change-Id: I488bfb5e6dabe254769a1c199459c5960653038f --- DnsProxyListener.cpp | 78 +++++++++++++++++++--------------------------------- 1 file changed, 29 insertions(+), 49 deletions(-) diff --git a/DnsProxyListener.cpp b/DnsProxyListener.cpp index 8f49eef6..f282a188 100644 --- a/DnsProxyListener.cpp +++ b/DnsProxyListener.cpp @@ -818,21 +818,14 @@ void DnsProxyListener::GetAddrInfoHandler::doDns64Synthesis(int32_t* rv, addrinf if (ipv6WantedButNoData) { // If caller wants IPv6 answers but no data, try to query IPv4 answers for synthesis - const uid_t uid = mClient->getUid(); - if (startQueryLimiter(uid)) { - const char* host = mHost.starts_with('^') ? nullptr : mHost.c_str(); - const char* service = mService.starts_with('^') ? nullptr : mService.c_str(); - mHints->ai_family = AF_INET; - // Don't need to do freeaddrinfo(res) before starting new DNS lookup because previous - // DNS lookup is failed with error EAI_NODATA. - *rv = resolv_getaddrinfo(host, service, mHints.get(), &mNetContext, res, event); - endQueryLimiter(uid); - if (*rv) { - *rv = EAI_NODATA; // return original error code - return; - } - } else { - LOG(ERROR) << __func__ << ": from UID " << uid << ", max concurrent queries reached"; + const char* host = mHost.starts_with('^') ? nullptr : mHost.c_str(); + const char* service = mService.starts_with('^') ? nullptr : mService.c_str(); + mHints->ai_family = AF_INET; + // Don't need to do freeaddrinfo(res) before starting new DNS lookup because previous + // DNS lookup is failed with error EAI_NODATA. + *rv = resolv_getaddrinfo(host, service, mHints.get(), &mNetContext, res, event); + if (*rv) { + *rv = EAI_NODATA; // return original error code return; } } @@ -865,6 +858,7 @@ void DnsProxyListener::GetAddrInfoHandler::run() { const char* service = mService.starts_with('^') ? nullptr : mService.c_str(); if (evaluate_domain_name(mNetContext, host)) { rv = resolv_getaddrinfo(host, service, mHints.get(), &mNetContext, &result, &event); + doDns64Synthesis(&rv, &result, &event); } else { rv = EAI_SYSTEM; } @@ -877,7 +871,6 @@ void DnsProxyListener::GetAddrInfoHandler::run() { << ", max concurrent queries reached"; } - doDns64Synthesis(&rv, &result, &event); const int32_t latencyUs = saturate_cast(s.timeTakenUs()); event.set_latency_micros(latencyUs); event.set_event_type(EVENT_GETADDRINFO); @@ -1244,17 +1237,10 @@ void DnsProxyListener::GetHostByNameHandler::doDns64Synthesis(int32_t* rv, hoste } // If caller wants IPv6 answers but no data, try to query IPv4 answers for synthesis - const uid_t uid = mClient->getUid(); - if (startQueryLimiter(uid)) { - const char* name = mName.starts_with('^') ? nullptr : mName.c_str(); - *rv = resolv_gethostbyname(name, AF_INET, hbuf, buf, buflen, &mNetContext, hpp, event); - endQueryLimiter(uid); - if (*rv) { - *rv = EAI_NODATA; // return original error code - return; - } - } else { - LOG(ERROR) << __func__ << ": from UID " << uid << ", max concurrent queries reached"; + const char* name = mName.starts_with('^') ? nullptr : mName.c_str(); + *rv = resolv_gethostbyname(name, AF_INET, hbuf, buf, buflen, &mNetContext, hpp, event); + if (*rv) { + *rv = EAI_NODATA; // return original error code return; } @@ -1281,6 +1267,7 @@ void DnsProxyListener::GetHostByNameHandler::run() { if (evaluate_domain_name(mNetContext, name)) { rv = resolv_gethostbyname(name, mAf, &hbuf, tmpbuf, sizeof tmpbuf, &mNetContext, &hp, &event); + doDns64Synthesis(&rv, &hbuf, tmpbuf, sizeof tmpbuf, &hp, &event); } else { rv = EAI_SYSTEM; } @@ -1291,7 +1278,6 @@ void DnsProxyListener::GetHostByNameHandler::run() { << ", max concurrent queries reached"; } - doDns64Synthesis(&rv, &hbuf, tmpbuf, sizeof tmpbuf, &hp, &event); const int32_t latencyUs = saturate_cast(s.timeTakenUs()); event.set_latency_micros(latencyUs); event.set_event_type(EVENT_GETHOSTBYNAME); @@ -1406,27 +1392,21 @@ void DnsProxyListener::GetHostByAddrHandler::doDns64ReverseLookup(hostent* hbuf, return; } - const uid_t uid = mClient->getUid(); - if (startQueryLimiter(uid)) { - // Remove NAT64 prefix and do reverse DNS query - struct in_addr v4addr = {.s_addr = v6addr.s6_addr32[3]}; - resolv_gethostbyaddr(&v4addr, sizeof(v4addr), AF_INET, hbuf, buf, buflen, &mNetContext, hpp, - event); - endQueryLimiter(uid); - if (*hpp && (*hpp)->h_addr_list[0]) { - // Replace IPv4 address with original queried IPv6 address in place. The space has - // reserved by dns_gethtbyaddr() and netbsd_gethostent_r() in - // system/netd/resolv/gethnamaddr.cpp. - // Note that resolv_gethostbyaddr() returns only one entry in result. - char* addr = (*hpp)->h_addr_list[0]; - memcpy(addr, &v6addr, sizeof(v6addr)); - (*hpp)->h_addrtype = AF_INET6; - (*hpp)->h_length = sizeof(struct in6_addr); - } else { - LOG(ERROR) << __func__ << ": hpp or (*hpp)->h_addr_list[0] is null"; - } + // Remove NAT64 prefix and do reverse DNS query + struct in_addr v4addr = {.s_addr = v6addr.s6_addr32[3]}; + resolv_gethostbyaddr(&v4addr, sizeof(v4addr), AF_INET, hbuf, buf, buflen, &mNetContext, hpp, + event); + if (*hpp && (*hpp)->h_addr_list[0]) { + // Replace IPv4 address with original queried IPv6 address in place. The space has + // reserved by dns_gethtbyaddr() and netbsd_gethostent_r() in + // system/netd/resolv/gethnamaddr.cpp. + // Note that resolv_gethostbyaddr() returns only one entry in result. + char* addr = (*hpp)->h_addr_list[0]; + memcpy(addr, &v6addr, sizeof(v6addr)); + (*hpp)->h_addrtype = AF_INET6; + (*hpp)->h_length = sizeof(struct in6_addr); } else { - LOG(ERROR) << __func__ << ": from UID " << uid << ", max concurrent queries reached"; + LOG(ERROR) << __func__ << ": hpp or (*hpp)->h_addr_list[0] is null"; } } @@ -1453,6 +1433,7 @@ void DnsProxyListener::GetHostByAddrHandler::run() { } else { rv = resolv_gethostbyaddr(&mAddress, mAddressLen, mAddressFamily, &hbuf, tmpbuf, sizeof tmpbuf, &mNetContext, &hp, &event); + doDns64ReverseLookup(&hbuf, tmpbuf, sizeof tmpbuf, &hp, &event); } endQueryLimiter(uid); } else { @@ -1461,7 +1442,6 @@ void DnsProxyListener::GetHostByAddrHandler::run() { << ", max concurrent queries reached"; } - doDns64ReverseLookup(&hbuf, tmpbuf, sizeof tmpbuf, &hp, &event); const int32_t latencyUs = saturate_cast(s.timeTakenUs()); event.set_latency_micros(latencyUs); event.set_event_type(EVENT_GETHOSTBYADDR); -- cgit v1.2.3 From 78c999954995fc56fe8ab7963811fbd10660035e Mon Sep 17 00:00:00 2001 From: Ryan Prichard Date: Fri, 21 Jul 2023 00:59:25 -0700 Subject: Accommodate a change in the type of std::span's size The WIP version of std::span in external/libcxx uses a ptrdiff_t size, but the final standardized version of std::span uses size_t instead. Use std::span() constructor calls rather than {}-syntax, which will convert the signed length to unsigned and works with either the old or the new libc++. Test: treehugger Change-Id: I5b5a16d0949e77a74269b9f6cf24382dd69a5973 --- DnsProxyListener.cpp | 10 +++++----- DnsTlsTransport.cpp | 2 +- getaddrinfo.cpp | 6 ++++-- res_cache.cpp | 6 +++--- res_mkquery.cpp | 2 +- res_query.cpp | 2 +- res_send.cpp | 21 +++++++++++++-------- tests/dns_responder/dns_responder.cpp | 3 ++- 8 files changed, 30 insertions(+), 22 deletions(-) diff --git a/DnsProxyListener.cpp b/DnsProxyListener.cpp index 8f49eef6..c0d766ca 100644 --- a/DnsProxyListener.cpp +++ b/DnsProxyListener.cpp @@ -1055,8 +1055,8 @@ void DnsProxyListener::ResNSendHandler::run() { uint16_t original_query_id = 0; // TODO: Handle the case which is msg contains more than one query - if (!parseQuery({msg.data(), msgLen}, &original_query_id, &rr_type, &rr_name) || - !setQueryId({msg.data(), msgLen}, arc4random_uniform(65536))) { + if (!parseQuery(std::span(msg.data(), msgLen), &original_query_id, &rr_type, &rr_name) || + !setQueryId(std::span(msg.data(), msgLen), arc4random_uniform(65536))) { // If the query couldn't be parsed, block the request. LOG(WARNING) << "ResNSendHandler::run: resnsend: from UID " << uid << ", invalid query"; sendBE32(mClient, -EINVAL); @@ -1071,7 +1071,7 @@ void DnsProxyListener::ResNSendHandler::run() { initDnsEvent(&event, mNetContext); if (startQueryLimiter(uid)) { if (evaluate_domain_name(mNetContext, rr_name.c_str())) { - ansLen = resolv_res_nsend(&mNetContext, {msg.data(), msgLen}, ansBuf, &rcode, + ansLen = resolv_res_nsend(&mNetContext, std::span(msg.data(), msgLen), ansBuf, &rcode, static_cast(mFlags), &event); } else { ansLen = -EAI_SYSTEM; @@ -1109,7 +1109,7 @@ void DnsProxyListener::ResNSendHandler::run() { } // Restore query id - if (!setQueryId({ansBuf.data(), ansLen}, original_query_id)) { + if (!setQueryId(std::span(ansBuf.data(), ansLen), original_query_id)) { LOG(WARNING) << "ResNSendHandler::run: resnsend: failed to restore query id"; return; } @@ -1124,7 +1124,7 @@ void DnsProxyListener::ResNSendHandler::run() { if (rr_type == ns_t_a || rr_type == ns_t_aaaa) { std::vector ip_addrs; const int total_ip_addr_count = - extractResNsendAnswers({ansBuf.data(), ansLen}, rr_type, &ip_addrs); + extractResNsendAnswers(std::span(ansBuf.data(), ansLen), rr_type, &ip_addrs); reportDnsEvent(INetdEventListener::EVENT_RES_NSEND, mNetContext, latencyUs, resNSendToAiError(ansLen, rcode), event, rr_name, ip_addrs, total_ip_addr_count); diff --git a/DnsTlsTransport.cpp b/DnsTlsTransport.cpp index 172a1e36..de40aac1 100644 --- a/DnsTlsTransport.cpp +++ b/DnsTlsTransport.cpp @@ -113,7 +113,7 @@ base::Result sendUdpQuery(netdutils::IPAddress ip, uint32_t mark, return ErrnoErrorf("connect failed"); } - if (send(fd, query.data(), query.size(), 0) != query.size()) { + if (send(fd, query.data(), query.size(), 0) != static_cast(query.size())) { return ErrnoErrorf("send failed"); } diff --git a/getaddrinfo.cpp b/getaddrinfo.cpp index 6bae41d7..4ad24248 100644 --- a/getaddrinfo.cpp +++ b/getaddrinfo.cpp @@ -1637,7 +1637,8 @@ QueryResult doQuery(const char* name, res_target* t, ResState* res, ResState res_temp = res->clone(&event); int rcode = NOERROR; - n = res_nsend(&res_temp, {buf, n}, {t->answer.data(), anslen}, &rcode, 0, sleepTimeMs); + n = res_nsend(&res_temp, std::span(buf, n), std::span(t->answer.data(), anslen), &rcode, 0, + sleepTimeMs); if (n < 0 || hp->rcode != NOERROR || ntohs(hp->ancount) == 0) { if (rcode != RCODE_TIMEOUT) rcode = hp->rcode; // if the query choked with EDNS0, retry without EDNS0 @@ -1646,7 +1647,8 @@ QueryResult doQuery(const char* name, res_target* t, ResState* res, (res_temp.flags & RES_F_EDNS0ERR)) { LOG(INFO) << __func__ << ": retry without EDNS0"; n = res_nmkquery(QUERY, name, cl, type, {}, buf, res_temp.netcontext_flags); - n = res_nsend(&res_temp, {buf, n}, {t->answer.data(), anslen}, &rcode, 0); + n = res_nsend(&res_temp, std::span(buf, n), std::span(t->answer.data(), anslen), &rcode, + 0); } } diff --git a/res_cache.cpp b/res_cache.cpp index 86def626..97da40d2 100644 --- a/res_cache.cpp +++ b/res_cache.cpp @@ -1214,7 +1214,7 @@ static void _cache_remove_oldest(Cache* cache) { return; } LOG(DEBUG) << __func__ << ": Cache full - removing oldest"; - res_pquery({oldest->query, oldest->querylen}); + res_pquery(std::span(oldest->query, oldest->querylen)); _cache_remove_p(cache, lookup); } @@ -1318,13 +1318,13 @@ ResolvCacheStatus resolv_cache_lookup(unsigned netid, span query, /* remove stale entries here */ if (now >= e->expires) { LOG(DEBUG) << __func__ << ": NOT IN CACHE (STALE ENTRY " << *lookup << "DISCARDED)"; - res_pquery({e->query, e->querylen}); + res_pquery(std::span(e->query, e->querylen)); _cache_remove_p(cache, lookup); return RESOLV_CACHE_NOTFOUND; } *answerlen = e->answerlen; - if (e->answerlen > answer.size()) { + if (e->answerlen > static_cast(answer.size())) { /* NOTE: we return UNSUPPORTED if the answer buffer is too short */ LOG(INFO) << __func__ << ": ANSWER TOO LONG"; return RESOLV_CACHE_UNSUPPORTED; diff --git a/res_mkquery.cpp b/res_mkquery.cpp index 11fbfdb7..24cee30e 100644 --- a/res_mkquery.cpp +++ b/res_mkquery.cpp @@ -166,7 +166,7 @@ int res_nmkquery(int op, // opcode of query /* * Initialize answer section */ - if (ep - cp < 1 + RRFIXEDSZ + data.size()) return (-1); + if (ep - cp < static_cast(1 + RRFIXEDSZ + data.size())) return (-1); *cp++ = '\0'; /* no domain name */ *reinterpret_cast(cp) = htons(type); cp += INT16SZ; diff --git a/res_query.cpp b/res_query.cpp index 036a6e31..be176b6c 100644 --- a/res_query.cpp +++ b/res_query.cpp @@ -126,7 +126,7 @@ again: *herrno = NO_RECOVERY; return n; } - n = res_nsend(statp, {buf, n}, answer, &rcode, 0); + n = res_nsend(statp, std::span(buf, n), answer, &rcode, 0); if (n < 0) { // If the query choked with EDNS0, retry without EDNS0 that when the server // has no response, resovler won't retry and do nothing. Even fallback to UDP, diff --git a/res_send.cpp b/res_send.cpp index 38b9c6ac..554ace6f 100644 --- a/res_send.cpp +++ b/res_send.cpp @@ -491,7 +491,7 @@ int res_nsend(ResState* statp, span msg, span ans, int* res_pquery(ans.first(resplen)); if (cache_status == RESOLV_CACHE_NOTFOUND) { - resolv_cache_add(statp->netid, msg, {ans.data(), resplen}); + resolv_cache_add(statp->netid, msg, std::span(ans.data(), resplen)); } return resplen; } @@ -667,7 +667,7 @@ int res_nsend(ResState* statp, span msg, span ans, int* res_pquery(ans.first(resplen)); if (cache_status == RESOLV_CACHE_NOTFOUND) { - resolv_cache_add(statp->netid, msg, {ans.data(), resplen}); + resolv_cache_add(statp->netid, msg, std::span(ans.data(), resplen)); } statp->closeSockets(); return (resplen); @@ -807,7 +807,7 @@ same_ns: {.iov_base = const_cast(msg.data()), .iov_len = static_cast(msg.size())}, }; - if (writev(statp->tcp_nssock, iov, 2) != (INT16SZ + msg.size())) { + if (writev(statp->tcp_nssock, iov, 2) != static_cast(INT16SZ + msg.size())) { *terrno = errno; PLOG(DEBUG) << __func__ << ": write failed: "; statp->closeSockets(); @@ -1111,7 +1111,8 @@ static int send_dg(ResState* statp, res_params* params, span msg, } LOG(DEBUG) << __func__ << ": new DG socket"; } - if (send(statp->udpsocks[*ns], msg.data(), msg.size(), 0) != msg.size()) { + if (send(statp->udpsocks[*ns], msg.data(), msg.size(), 0) != + static_cast(msg.size())) { *terrno = errno; PLOG(DEBUG) << __func__ << ": send: "; statp->closeSockets(); @@ -1155,11 +1156,14 @@ static int send_dg(ResState* statp, res_params* params, span msg, *terrno = EMSGSIZE; continue; } + if (resplen > static_cast(ans.size())) { + LOG(FATAL) << __func__ << ": invalid resplen (too large): " << resplen; + } int receivedFromNs = *ns; if (needRetry = ignoreInvalidAnswer(statp, from, msg, ans, &receivedFromNs); needRetry) { - res_pquery({ans.data(), (resplen > ans.size()) ? ans.size() : resplen}); + res_pquery(ans.first(resplen)); continue; } @@ -1169,7 +1173,7 @@ static int send_dg(ResState* statp, res_params* params, span msg, // The case has to be captured here, as FORMERR packet do not // carry query section, hence res_queriesmatch() returns 0. LOG(DEBUG) << __func__ << ": server rejected query with EDNS0:"; - res_pquery({ans.data(), (resplen > ans.size()) ? ans.size() : resplen}); + res_pquery(ans.first(resplen)); // record the error statp->flags |= RES_F_EDNS0ERR; *terrno = EREMOTEIO; @@ -1178,7 +1182,7 @@ static int send_dg(ResState* statp, res_params* params, span msg, if (anhp->rcode == SERVFAIL || anhp->rcode == NOTIMP || anhp->rcode == REFUSED) { LOG(DEBUG) << __func__ << ": server rejected query:"; - res_pquery({ans.data(), (resplen > ans.size()) ? ans.size() : resplen}); + res_pquery(ans.first(resplen)); *rcode = anhp->rcode; continue; } @@ -1212,7 +1216,8 @@ static int send_mdns(ResState* statp, span msg, span ans if (setupUdpSocket(statp, mdnsap, &fd, terrno) <= 0) return 0; - if (sendto(fd, msg.data(), msg.size(), 0, mdnsap, sockaddrSize(mdnsap)) != msg.size()) { + if (sendto(fd, msg.data(), msg.size(), 0, mdnsap, sockaddrSize(mdnsap)) != + static_cast(msg.size())) { *terrno = errno; return 0; } diff --git a/tests/dns_responder/dns_responder.cpp b/tests/dns_responder/dns_responder.cpp index c4551ebf..75a2c599 100644 --- a/tests/dns_responder/dns_responder.cpp +++ b/tests/dns_responder/dns_responder.cpp @@ -730,7 +730,8 @@ void DNSResponder::requestHandler() { bool DNSResponder::handleDNSRequest(const char* buffer, ssize_t len, int protocol, char* response, size_t* response_len) const { - LOG(DEBUG) << "request: '" << bytesToHexStr({reinterpret_cast(buffer), len}) + LOG(DEBUG) << "request: '" + << bytesToHexStr(std::span(reinterpret_cast(buffer), len)) << "', on " << dnsproto2str(protocol); const char* buffer_end = buffer + len; DNSHeader header; -- cgit v1.2.3 From 9621e75349ec809e876d299455ef9c9e60dea19d Mon Sep 17 00:00:00 2001 From: Mike Yu Date: Wed, 27 Sep 2023 09:25:28 +0000 Subject: Test: Enable verbose logging in doh_ffi_test Enable the verbose logging to debug doh_ffi_test. This change will increase ~250 lines of log when running doh_ffi_test. Bug: 300679076 Test: atest doh_ffi_test Change-Id: Icc4e264af9587ce1e8fa9eba13c13a3f69e80bff --- tests/doh_ffi_test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/doh_ffi_test.cpp b/tests/doh_ffi_test.cpp index 0e51402a..6d5b0492 100644 --- a/tests/doh_ffi_test.cpp +++ b/tests/doh_ffi_test.cpp @@ -73,7 +73,7 @@ bool haveIpv6() { class DoHFFITest : public NetNativeTestBase { public: - static void SetUpTestSuite() { doh_init_logger(DOH_LOG_LEVEL_DEBUG); } + static void SetUpTestSuite() { doh_init_logger(DOH_LOG_LEVEL_TRACE); } }; TEST_F(DoHFFITest, SmokeTest) { -- cgit v1.2.3 From 70a5dab7281089eb9ac750d38d11ca60ddad5020 Mon Sep 17 00:00:00 2001 From: Ken Chen Date: Thu, 12 Oct 2023 12:54:53 +0800 Subject: [Test] Deflake ResolverTest#GetAddrInfoV4_deferred_resp There are two threads in the test that will set different DNS servers and send queries to the DNS resolver. It relies on a 100ms sleep between the two threads. When the problem occurs, the work of the first thread is somehow delayed, causing the second thread's DNS query to be sent to the DNS server configured by the first thread. Expected behavior: 1. Thread 1 create. 2. Thread 1 set DNS server to 127.0.0.10. 3. Thread 1 send DNS query to 127.0.0.10. 4. Thread 2 create. 5. Thread 2 set DNS server to 127.0.0.11. 6. Thread 2 send DNS query to 127.0.0.11. Failed case: 1. Thread 1 create (and then being delayed for over than 100ms) 2. Thread 2 create. 3. Thread 2 set DNS server to 127.0.0.11. 4. Thread 1 set DNS server to 127.0.0.10. 5. Thread 2 send DNS query to 127.0.0.10. <-- NG 6. Thread 1 send DNS query to 127.0.0.10. Replacing unreliable sleep by a boolean flag. Bug: 303371206 Bug: 302623174 Test: atest resolv_integration_test Change-Id: I56222894a8600c7cb4c1911dd453756d6069bd02 --- tests/resolv_integration_test.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/resolv_integration_test.cpp b/tests/resolv_integration_test.cpp index ff62da45..68d5d5fd 100644 --- a/tests/resolv_integration_test.cpp +++ b/tests/resolv_integration_test.cpp @@ -860,6 +860,7 @@ TEST_F(ResolverTest, GetAddrInfoV4_deferred_resp) { addrinfo hints = {.ai_family = AF_INET}; const std::array params = {300, 25, 8, 8, 5000, 0}; bool t3_task_done = false; + bool t2_sv_setup_done = false; dns1.setDeferredResp(true); std::thread t1([&, this]() { @@ -884,6 +885,7 @@ TEST_F(ResolverTest, GetAddrInfoV4_deferred_resp) { .setDotServers({}) .setParams(params) .build())); + t2_sv_setup_done = true; ScopedAddrinfo result = safe_getaddrinfo(host_name_deferred, nullptr, &hints); EXPECT_TRUE(t3_task_done); EXPECT_EQ(0U, GetNumQueries(dns2, host_name_deferred)); @@ -895,7 +897,7 @@ TEST_F(ResolverTest, GetAddrInfoV4_deferred_resp) { }); // ensuring t2 and t3 handler functions are processed in order - usleep(100 * 1000); + EXPECT_TRUE(PollForCondition([&]() { return t2_sv_setup_done; })); std::thread t3([&, this]() { ASSERT_TRUE(mDnsClient.SetResolversFromParcel(ResolverParams::Builder() .setDnsServers(servers_for_t3) -- cgit v1.2.3 From d9abf8372ae9232475f848e3068389678daeb00e Mon Sep 17 00:00:00 2001 From: Mike Yu Date: Thu, 12 Oct 2023 09:52:57 +0000 Subject: Test: Deflake PrivateDnsDohTest.ExcessDnsRequests The test is flaky because `timeout_queries` is not always (MAX_BUFFERED_COMMANDS + 2). It can be (MAX_BUFFERED_COMMANDS + 1). Bug: 301701119 Test: Ran PrivateDnsDohTest.ExcessDnsRequests 200 times Change-Id: Ia3aabcd3664a2f8a65ae520146375d52c678dcf3 --- tests/resolv_private_dns_test.cpp | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/tests/resolv_private_dns_test.cpp b/tests/resolv_private_dns_test.cpp index 59a70ffa..10c6b9cb 100644 --- a/tests/resolv_private_dns_test.cpp +++ b/tests/resolv_private_dns_test.cpp @@ -52,6 +52,7 @@ using android::netdutils::ScopedAddrinfo; using android::netdutils::Stopwatch; using std::chrono::milliseconds; using std::this_thread::sleep_for; +using ::testing::AnyOf; constexpr int MAXPACKET = (8 * 1024); @@ -804,9 +805,19 @@ TEST_F(PrivateDnsDohTest, TemporaryConnectionStalled) { TEST_F(PrivateDnsDohTest, ExcessDnsRequests) { const int total_queries = 70; - // The number is from MAX_BUFFERED_COMMANDS + 2 (one that will be queued in - // connection mpsc channel; the other one that will get blocked at dispatcher sending channel). - const int timeout_queries = 52; + // In most cases, the number of timed-out DoH queries is MAX_BUFFERED_COMMANDS + 2 (one that + // will be queued in connection's mpsc::channel; the other one that will get blocked at + // dispatcher's mpsc::channel), as shown below: + // + // dispatcher's mpsc::channel -----> network's mpsc:channel -----> connection's mpsc::channel + // (expect 1 query queued here) (size: MAX_BUFFERED_COMMANDS) (expect 1 query queued here) + // + // However, it's still possible that the (MAX_BUFFERED_COMMANDS + 2)th query is sent to the DoH + // engine before the DoH engine moves a query to connection's mpsc::channel. In that case, + // the (MAX_BUFFERED_COMMANDS + 2)th query will be fallback'ed to DoT immediately rather than + // be waiting until DoH timeout, which result in only (MAX_BUFFERED_COMMANDS + 1) timed-out + // DoH queries. + const int doh_timeout_queries = 52; // If early data flag is enabled, DnsResolver doesn't wait for the connection established. // It will send DNS queries along with 0-RTT rather than queue them in connection mpsc channel. @@ -829,7 +840,7 @@ TEST_F(PrivateDnsDohTest, ExcessDnsRequests) { dns.clearQueries(); // Set the DoT server not to close the connection until it receives enough queries or timeout. - dot.setDelayQueries(total_queries - timeout_queries); + dot.setDelayQueries(total_queries - doh_timeout_queries); dot.setDelayQueriesTimeout(200); // Set the server blocking, wait for the connection closed, and send some DNS requests. @@ -847,8 +858,10 @@ TEST_F(PrivateDnsDohTest, ExcessDnsRequests) { // There are some queries that fall back to DoT rather than UDP since the DoH client rejects // any new DNS requests when the capacity is full. - EXPECT_NO_FAILURE(expectQueries(timeout_queries /* dns */, - total_queries - timeout_queries /* dot */, 0 /* doh */)); + EXPECT_THAT(dns.queries().size(), AnyOf(doh_timeout_queries, doh_timeout_queries - 1)); + EXPECT_THAT(dot.queries(), AnyOf(total_queries - doh_timeout_queries, + total_queries - doh_timeout_queries + 1)); + EXPECT_EQ(doh.queries(), 0); // Set up another network and send a DNS query. Expect that this network is unaffected. constexpr int TEST_NETID_2 = 31; -- cgit v1.2.3 From 098116b9bbb1b567957ed92e218769d3591f6f79 Mon Sep 17 00:00:00 2001 From: Ken Chen Date: Mon, 24 Jul 2023 14:07:25 +0800 Subject: Make DNS query fail-fast when network access is restricted There are many functions that limit the app's network access, such as doze mode, battery saver, etc. Before this commit, the DNS resolver had no relevant information, it would send data and rely on the BPF program in Kernel to block the sending. This has two problems: (1) Waste of CPU resources. (2) Private DNS should be but is not restricted by those features. In this commit, the DNS resolver calls a function in a new added library to know whether apps are blocked by network restriction rules. If so, it returns failures early for both plaintext and encrypted queries. Bug: 288340533 Test: Auto test TBD Test: Manually test 1. Install a test app that keeps sending DNS in background. 2. Force enable doze mode by adb commands. 3. Check that DNS query results of the test app is failed. 4. Add the test app into white list. 5. Check that DNS query results of the test app is successful. Change-Id: I58b0f5e5ff0494f8d190ef601b984a96b2673911 --- Android.bp | 3 ++ DnsProxyListener.cpp | 73 ++++++++++++++++++++++++++++++++++++--- tests/resolv_integration_test.cpp | 4 +-- util.h | 5 +++ 4 files changed, 79 insertions(+), 6 deletions(-) diff --git a/Android.bp b/Android.bp index f2b5b4eb..d483d3de 100644 --- a/Android.bp +++ b/Android.bp @@ -250,6 +250,9 @@ cc_library { "libssl", "libstatssocket", ], + runtime_libs: [ + "libcom.android.tethering.dns_helper", + ], header_libs: [ "libnetdbinder_utils_headers", ], diff --git a/DnsProxyListener.cpp b/DnsProxyListener.cpp index 4b84fd2b..327b9079 100644 --- a/DnsProxyListener.cpp +++ b/DnsProxyListener.cpp @@ -18,6 +18,7 @@ #include #include +#include #include #include #include @@ -661,6 +662,54 @@ std::string makeThreadName(unsigned netId, uint32_t uid) { return fmt::format("Dns_{}_{}", netId, multiuser_get_app_id(uid)); } +typedef int (*InitFn)(); +typedef int (*IsUidBlockedFn)(uid_t, bool); + +IsUidBlockedFn ADnsHelper_isUidNetworkingBlocked; + +IsUidBlockedFn resolveIsUidNetworkingBlockedFn() { + // Related BPF maps were mainlined from T. + if (!isAtLeastT()) return nullptr; + + // TODO: Check whether it is safe to shared link the .so without using dlopen when the carrier + // APEX module (tethering) is fully released. + void* handle = dlopen("libcom.android.tethering.dns_helper.so", RTLD_NOW | RTLD_LOCAL); + if (!handle) { + LOG(WARNING) << __func__ << ": " << dlerror(); + return nullptr; + } + + InitFn ADnsHelper_init = reinterpret_cast(dlsym(handle, "ADnsHelper_init")); + if (!ADnsHelper_init) { + LOG(ERROR) << __func__ << ": " << dlerror(); + abort(); + } + const int ret = (*ADnsHelper_init)(); + if (ret) { + LOG(ERROR) << __func__ << ": ADnsHelper_init failed " << strerror(-ret); + abort(); + } + + IsUidBlockedFn f = + reinterpret_cast(dlsym(handle, "ADnsHelper_isUidNetworkingBlocked")); + if (!f) { + LOG(ERROR) << __func__ << ": " << dlerror(); + abort(); + } + return f; +} + +bool isUidNetworkingBlocked(uid_t uid, unsigned netId) { + if (!ADnsHelper_isUidNetworkingBlocked) return false; + + // The enforceDnsUid is an OEM feature that sets DNS packet with AID_DNS instead of the + // application's UID. Its DNS packets are not subject to certain network restriction features. + if (resolv_is_enforceDnsUid_enabled_network(netId)) return false; + + // TODO: Pass metered information from CS to DNS resolver and replace the hardcode value. + return (*ADnsHelper_isUidNetworkingBlocked)(uid, /*metered=*/false) == 1; +} + } // namespace DnsProxyListener::DnsProxyListener() : FrameworkListener(SOCKET_NAME) { @@ -678,6 +727,8 @@ DnsProxyListener::DnsProxyListener() : FrameworkListener(SOCKET_NAME) { mGetDnsNetIdCommand = std::make_unique(); registerCmd(mGetDnsNetIdCommand.get()); + + ADnsHelper_isUidNetworkingBlocked = resolveIsUidNetworkingBlockedFn(); } void DnsProxyListener::Handler::spawn() { @@ -853,7 +904,10 @@ void DnsProxyListener::GetAddrInfoHandler::run() { int32_t rv = 0; NetworkDnsEventReported event; initDnsEvent(&event, mNetContext); - if (startQueryLimiter(uid)) { + if (isUidNetworkingBlocked(mNetContext.uid, mNetContext.dns_netid)) { + LOG(INFO) << "GetAddrInfoHandler::run: network access blocked"; + rv = EAI_FAIL; + } else if (startQueryLimiter(uid)) { const char* host = mHost.starts_with('^') ? nullptr : mHost.c_str(); const char* service = mService.starts_with('^') ? nullptr : mService.c_str(); if (evaluate_domain_name(mNetContext, host)) { @@ -1062,11 +1116,15 @@ void DnsProxyListener::ResNSendHandler::run() { int ansLen = -1; NetworkDnsEventReported event; initDnsEvent(&event, mNetContext); - if (startQueryLimiter(uid)) { + if (isUidNetworkingBlocked(mNetContext.uid, mNetContext.dns_netid)) { + LOG(INFO) << "ResNSendHandler::run: network access blocked"; + ansLen = -ECONNREFUSED; + } else if (startQueryLimiter(uid)) { if (evaluate_domain_name(mNetContext, rr_name.c_str())) { ansLen = resolv_res_nsend(&mNetContext, std::span(msg.data(), msgLen), ansBuf, &rcode, static_cast(mFlags), &event); } else { + // TODO(b/307048182): It should return -errno. ansLen = -EAI_SYSTEM; } endQueryLimiter(uid); @@ -1262,7 +1320,10 @@ void DnsProxyListener::GetHostByNameHandler::run() { int32_t rv = 0; NetworkDnsEventReported event; initDnsEvent(&event, mNetContext); - if (startQueryLimiter(uid)) { + if (isUidNetworkingBlocked(mNetContext.uid, mNetContext.dns_netid)) { + LOG(INFO) << "GetHostByNameHandler::run: network access blocked"; + rv = EAI_FAIL; + } else if (startQueryLimiter(uid)) { const char* name = mName.starts_with('^') ? nullptr : mName.c_str(); if (evaluate_domain_name(mNetContext, name)) { rv = resolv_gethostbyname(name, mAf, &hbuf, tmpbuf, sizeof tmpbuf, &mNetContext, &hp, @@ -1421,7 +1482,11 @@ void DnsProxyListener::GetHostByAddrHandler::run() { int32_t rv = 0; NetworkDnsEventReported event; initDnsEvent(&event, mNetContext); - if (startQueryLimiter(uid)) { + + if (isUidNetworkingBlocked(mNetContext.uid, mNetContext.dns_netid)) { + LOG(INFO) << "GetHostByAddrHandler::run: network access blocked"; + rv = EAI_FAIL; + } else if (startQueryLimiter(uid)) { // From Android U, evaluate_domain_name() is not only for OEM customization, but also tells // DNS resolver whether the UID can send DNS on the specified network. The function needs // to be called even when there is no domain name to evaluate (GetHostByAddr). This is diff --git a/tests/resolv_integration_test.cpp b/tests/resolv_integration_test.cpp index 68d5d5fd..6706e7c5 100644 --- a/tests/resolv_integration_test.cpp +++ b/tests/resolv_integration_test.cpp @@ -4505,9 +4505,9 @@ TEST_F(ResolverTest, GetAddrinfo_BlockDnsQueryWithUidRule) { const char* hname; const int expectedErrorCode; } kTestData[] = { - {host_name, EAI_NODATA}, + {host_name, isAtLeastT() ? EAI_FAIL : EAI_NODATA}, // To test the query with search domain. - {"howdy", EAI_AGAIN}, + {"howdy", isAtLeastT() ? EAI_FAIL : EAI_AGAIN}, }; INetd* netdService = mDnsClient.netdService(); diff --git a/util.h b/util.h index fd9e2d40..8c03b470 100644 --- a/util.h +++ b/util.h @@ -62,6 +62,11 @@ inline bool isDebuggable() { return android::base::GetBoolProperty("ro.debuggable", false); } +inline bool isAtLeastT() { + const static bool isAtLeastT = android::modules::sdklevel::IsAtLeastT(); + return isAtLeastT; +} + inline bool isAtLeastU() { const static bool isAtLeastU = android::modules::sdklevel::IsAtLeastU(); return isAtLeastU; -- cgit v1.2.3 From c36de978f40200847c9d175ef455cd6c7f64fb93 Mon Sep 17 00:00:00 2001 From: Ken Chen Date: Sun, 17 Sep 2023 20:18:42 +0800 Subject: [Test] DNS query blocked by UID network restrictions Test is conducted on 4 DNS entrances over private DNS only. There is no effective way to know whether a DNS query is rejected by the DNS resolver or by the BPF program in kernel. However, private DNS queries will not be rejected by BPF programs before the "DNS query fail-fast" feature, and will be rejected after the implementation. So we use private DNS to verify the functionality. Bug: 288340533 Test: atest resolv_integration_test Change-Id: Ie4a18995826a4bd7705d71b12a2af91f895ba683 --- tests/resolv_private_dns_test.cpp | 57 +++++++++++++++++++++++++++++++++++++++ tests/resolv_test_utils.h | 8 ++++++ 2 files changed, 65 insertions(+) diff --git a/tests/resolv_private_dns_test.cpp b/tests/resolv_private_dns_test.cpp index 10c6b9cb..e9d3f8e5 100644 --- a/tests/resolv_private_dns_test.cpp +++ b/tests/resolv_private_dns_test.cpp @@ -528,6 +528,63 @@ TEST_P(TransportParameterizedTest, MdnsGetAddrInfo_fallback) { } } +TEST_P(TransportParameterizedTest, BlockDnsQueryWithUidRule) { + SKIP_IF_BEFORE_T; + constexpr char ptr_name[] = "v4v6.example.com."; + // PTR record for IPv6 address 2001:db8::102:304 + constexpr char ptr_addr_v6[] = + "4.0.3.0.2.0.1.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.8.b.d.0.1.0.0.2.ip6.arpa."; + const DnsRecord r = {ptr_addr_v6, ns_type::ns_t_ptr, ptr_name}; + dns.addMapping(r.host_name, r.type, r.addr); + dot_backend.addMapping(r.host_name, r.type, r.addr); + doh_backend.addMapping(r.host_name, r.type, r.addr); + + const auto parcel = DnsResponderClient::GetDefaultResolverParamsParcel(); + ASSERT_TRUE(mDnsClient.SetResolversFromParcel(parcel)); + + if (testParamHasDoh()) EXPECT_TRUE(WaitForDohValidationSuccess(test::kDefaultListenAddr)); + if (testParamHasDot()) EXPECT_TRUE(WaitForDotValidationSuccess(test::kDefaultListenAddr)); + + // This waiting time is expected to avoid that the DoH validation event interferes other tests. + if (!testParamHasDoh()) waitForDohValidationFailed(); + + // Have the test independent of the number of sent queries in private DNS validation, because + // the DnsResolver can send either 1 or 2 queries in DoT validation. + if (testParamHasDoh()) { + doh.clearQueries(); + } + if (testParamHasDot()) { + EXPECT_TRUE(dot.waitForQueries(1)); + dot.clearQueries(); + } + dns.clearQueries(); + + // Block TEST_UID's network access + ScopeBlockedUIDRule scopeBlockUidRule(mDnsClient.netdService(), TEST_UID); + + // getaddrinfo should fail + const addrinfo hints = {.ai_socktype = SOCK_DGRAM}; + EXPECT_FALSE(safe_getaddrinfo(kQueryHostname, nullptr, &hints)); + + // gethostbyname should fail + EXPECT_FALSE(gethostbyname(kQueryHostname)); + + // gethostbyaddr should fail + in6_addr v6addr; + inet_pton(AF_INET6, "2001:db8::102:304", &v6addr); + EXPECT_FALSE(gethostbyaddr(&v6addr, sizeof(v6addr), AF_INET6)); + + // resNetworkQuery should fail + int fd = resNetworkQuery(TEST_NETID, kQueryHostname, ns_c_in, ns_t_aaaa, 0); + EXPECT_TRUE(fd != -1); + + uint8_t buf[MAXPACKET] = {}; + int rcode; + EXPECT_EQ(-ECONNREFUSED, getAsyncResponse(fd, &rcode, buf, MAXPACKET)); + + expectQueries(0 /* dns */, 0 /* dot */, 0 /* doh */); +} + class PrivateDnsDohTest : public BasePrivateDnsTest { protected: void SetUp() override { diff --git a/tests/resolv_test_utils.h b/tests/resolv_test_utils.h index a94c6234..c5c84bc0 100644 --- a/tests/resolv_test_utils.h +++ b/tests/resolv_test_utils.h @@ -32,6 +32,7 @@ #include #include "dns_responder/dns_responder.h" +#include "util.h" class ScopeBlockedUIDRule { using INetd = aidl::android::net::INetd; @@ -402,3 +403,10 @@ android::netdutils::ScopedAddrinfo safe_getaddrinfo(const char* node, const char void SetMdnsRoute(); void RemoveMdnsRoute(); + +#define SKIP_IF_BEFORE_T \ + do { \ + if (!isAtLeastT()) { \ + GTEST_SKIP() << "Skipping test because SDK version is less than T."; \ + } \ + } while (0) -- cgit v1.2.3 From a3abee95841e68d0c66d7908cb8f1bcdb01d4319 Mon Sep 17 00:00:00 2001 From: Ken Chen Date: Mon, 30 Oct 2023 15:57:16 +0800 Subject: Add 'metered' in ResolverParamsParcel 1. Adds 'metered' in ResolverParamsParcel. 2. m dnsresolver_aidl_interface-update-api. Bug: 288340533 Test: will run test with the commit that uses the new field Change-Id: I4c857d18c7773c82cb3f10fce468bf6d383523b2 --- Android.bp | 67 +++++++++++++++++----- .../current/android/net/IDnsResolver.aidl | 2 +- .../current/android/net/ResolverParamsParcel.aidl | 1 + binder/android/net/ResolverParamsParcel.aidl | 5 ++ 4 files changed, 59 insertions(+), 16 deletions(-) diff --git a/Android.bp b/Android.bp index f2b5b4eb..a7613d2c 100644 --- a/Android.bp +++ b/Android.bp @@ -96,22 +96,59 @@ aidl_interface { min_sdk_version: "30", }, }, - versions: [ - "1", - "2", - "3", - "4", - "5", - "6", - "7", - "8", - "9", - "10", - "11", - ], + dumpapi: { no_license: true, }, + versions_with_info: [ + { + version: "1", + imports: ["netd_event_listener_interface-V1"], + }, + { + version: "2", + imports: ["netd_event_listener_interface-V1"], + }, + { + version: "3", + imports: ["netd_event_listener_interface-V1"], + }, + { + version: "4", + imports: ["netd_event_listener_interface-V1"], + }, + { + version: "5", + imports: ["netd_event_listener_interface-V1"], + }, + { + version: "6", + imports: ["netd_event_listener_interface-V1"], + }, + { + version: "7", + imports: ["netd_event_listener_interface-V1"], + }, + { + version: "8", + imports: ["netd_event_listener_interface-V1"], + }, + { + version: "9", + imports: ["netd_event_listener_interface-V1"], + }, + { + version: "10", + imports: ["netd_event_listener_interface-V1"], + }, + { + version: "11", + imports: ["netd_event_listener_interface-V1"], + }, + + ], + frozen: false, + } cc_defaults { @@ -154,7 +191,7 @@ cc_defaults { // after the build process. host_required: [ "net-tests-utils-host-common", - ] + ], } cc_defaults { @@ -176,7 +213,7 @@ cc_defaults { // after the build process. host_required: [ "net-tests-utils-host-common", - ] + ], } cc_library { diff --git a/aidl_api/dnsresolver_aidl_interface/current/android/net/IDnsResolver.aidl b/aidl_api/dnsresolver_aidl_interface/current/android/net/IDnsResolver.aidl index 19313e40..6b539c47 100644 --- a/aidl_api/dnsresolver_aidl_interface/current/android/net/IDnsResolver.aidl +++ b/aidl_api/dnsresolver_aidl_interface/current/android/net/IDnsResolver.aidl @@ -55,7 +55,7 @@ interface IDnsResolver { const int DNS_RESOLVER_LOG_ERROR = 4; const int TC_MODE_DEFAULT = 0; const int TC_MODE_UDP_TCP = 1; - const int TRANSPORT_UNKNOWN = -1; + const int TRANSPORT_UNKNOWN = (-1) /* -1 */; const int TRANSPORT_CELLULAR = 0; const int TRANSPORT_WIFI = 1; const int TRANSPORT_BLUETOOTH = 2; diff --git a/aidl_api/dnsresolver_aidl_interface/current/android/net/ResolverParamsParcel.aidl b/aidl_api/dnsresolver_aidl_interface/current/android/net/ResolverParamsParcel.aidl index 8d0bf75e..8a7bf997 100644 --- a/aidl_api/dnsresolver_aidl_interface/current/android/net/ResolverParamsParcel.aidl +++ b/aidl_api/dnsresolver_aidl_interface/current/android/net/ResolverParamsParcel.aidl @@ -35,4 +35,5 @@ parcelable ResolverParamsParcel { int tlsConnectTimeoutMs = 0; @nullable android.net.ResolverOptionsParcel resolverOptions; int[] transportTypes = {}; + boolean meteredNetwork = false; } diff --git a/binder/android/net/ResolverParamsParcel.aidl b/binder/android/net/ResolverParamsParcel.aidl index 5511f281..8128fb7d 100644 --- a/binder/android/net/ResolverParamsParcel.aidl +++ b/binder/android/net/ResolverParamsParcel.aidl @@ -113,4 +113,9 @@ parcelable ResolverParamsParcel { * reasonable network type by DnsResolver, it would be considered as unknown. */ int[] transportTypes = {}; + + /** + * Whether the network is metered or not. + */ + boolean meteredNetwork = false; } -- cgit v1.2.3 From 06e5f421d38a4bb72bb1efd2e9f232a5797166ef Mon Sep 17 00:00:00 2001 From: Ken Chen Date: Mon, 30 Oct 2023 16:10:25 +0800 Subject: Freeze DnsResolver AIDL interfaces to v12 m dnsresolver_aidl_interface-freeze-api Bug: 288340533 Test: presubmit Change-Id: Ib9762e9166578f13c78be06b831c2fdc7086daf9 --- Android.bp | 6 +- aidl_api/dnsresolver_aidl_interface/12/.hash | 1 + .../12/android/net/IDnsResolver.aidl | 69 ++++++++++++++++++++++ .../12/android/net/ResolverHostsParcel.aidl | 24 ++++++++ .../12/android/net/ResolverOptionsParcel.aidl | 25 ++++++++ .../12/android/net/ResolverParamsParcel.aidl | 39 ++++++++++++ .../net/resolv/aidl/DnsHealthEventParcel.aidl | 26 ++++++++ .../aidl/IDnsResolverUnsolicitedEventListener.aidl | 33 +++++++++++ .../net/resolv/aidl/Nat64PrefixEventParcel.aidl | 27 +++++++++ .../aidl/PrivateDnsValidationEventParcel.aidl | 28 +++++++++ 10 files changed, 277 insertions(+), 1 deletion(-) create mode 100644 aidl_api/dnsresolver_aidl_interface/12/.hash create mode 100644 aidl_api/dnsresolver_aidl_interface/12/android/net/IDnsResolver.aidl create mode 100644 aidl_api/dnsresolver_aidl_interface/12/android/net/ResolverHostsParcel.aidl create mode 100644 aidl_api/dnsresolver_aidl_interface/12/android/net/ResolverOptionsParcel.aidl create mode 100644 aidl_api/dnsresolver_aidl_interface/12/android/net/ResolverParamsParcel.aidl create mode 100644 aidl_api/dnsresolver_aidl_interface/12/android/net/resolv/aidl/DnsHealthEventParcel.aidl create mode 100644 aidl_api/dnsresolver_aidl_interface/12/android/net/resolv/aidl/IDnsResolverUnsolicitedEventListener.aidl create mode 100644 aidl_api/dnsresolver_aidl_interface/12/android/net/resolv/aidl/Nat64PrefixEventParcel.aidl create mode 100644 aidl_api/dnsresolver_aidl_interface/12/android/net/resolv/aidl/PrivateDnsValidationEventParcel.aidl diff --git a/Android.bp b/Android.bp index a7613d2c..69aed259 100644 --- a/Android.bp +++ b/Android.bp @@ -145,9 +145,13 @@ aidl_interface { version: "11", imports: ["netd_event_listener_interface-V1"], }, + { + version: "12", + imports: ["netd_event_listener_interface-V1"], + }, ], - frozen: false, + frozen: true, } diff --git a/aidl_api/dnsresolver_aidl_interface/12/.hash b/aidl_api/dnsresolver_aidl_interface/12/.hash new file mode 100644 index 00000000..788aef87 --- /dev/null +++ b/aidl_api/dnsresolver_aidl_interface/12/.hash @@ -0,0 +1 @@ +a65a6755e2e5f5c160e7be2be814019e2d5491b1 diff --git a/aidl_api/dnsresolver_aidl_interface/12/android/net/IDnsResolver.aidl b/aidl_api/dnsresolver_aidl_interface/12/android/net/IDnsResolver.aidl new file mode 100644 index 00000000..6b539c47 --- /dev/null +++ b/aidl_api/dnsresolver_aidl_interface/12/android/net/IDnsResolver.aidl @@ -0,0 +1,69 @@ +/////////////////////////////////////////////////////////////////////////////// +// THIS FILE IS IMMUTABLE. DO NOT EDIT IN ANY CASE. // +/////////////////////////////////////////////////////////////////////////////// + +// This file is a snapshot of an AIDL file. Do not edit it manually. There are +// two cases: +// 1). this is a frozen version file - do not edit this in any case. +// 2). this is a 'current' file. If you make a backwards compatible change to +// the interface (from the latest frozen version), the build system will +// prompt you to update this file with `m -update-api`. +// +// You must not make a backward incompatible change to any AIDL file built +// with the aidl_interface module type with versions property set. The module +// type is used to build AIDL files in a way that they can be used across +// independently updatable components of the system. If a device is shipped +// with such a backward incompatible change, it has a high risk of breaking +// later when a module using the interface is updated, e.g., Mainline modules. + +package android.net; +/* @hide */ +interface IDnsResolver { + boolean isAlive(); + void registerEventListener(android.net.metrics.INetdEventListener listener); + void setResolverConfiguration(in android.net.ResolverParamsParcel resolverParams); + void getResolverInfo(int netId, out @utf8InCpp String[] servers, out @utf8InCpp String[] domains, out @utf8InCpp String[] tlsServers, out int[] params, out int[] stats, out int[] wait_for_pending_req_timeout_count); + void startPrefix64Discovery(int netId); + void stopPrefix64Discovery(int netId); + @utf8InCpp String getPrefix64(int netId); + void createNetworkCache(int netId); + void destroyNetworkCache(int netId); + void setLogSeverity(int logSeverity); + void flushNetworkCache(int netId); + void setPrefix64(int netId, @utf8InCpp String prefix); + void registerUnsolicitedEventListener(android.net.resolv.aidl.IDnsResolverUnsolicitedEventListener listener); + void setResolverOptions(int netId, in android.net.ResolverOptionsParcel optionParams); + const int RESOLVER_PARAMS_SAMPLE_VALIDITY = 0; + const int RESOLVER_PARAMS_SUCCESS_THRESHOLD = 1; + const int RESOLVER_PARAMS_MIN_SAMPLES = 2; + const int RESOLVER_PARAMS_MAX_SAMPLES = 3; + const int RESOLVER_PARAMS_BASE_TIMEOUT_MSEC = 4; + const int RESOLVER_PARAMS_RETRY_COUNT = 5; + const int RESOLVER_PARAMS_COUNT = 6; + const int RESOLVER_STATS_SUCCESSES = 0; + const int RESOLVER_STATS_ERRORS = 1; + const int RESOLVER_STATS_TIMEOUTS = 2; + const int RESOLVER_STATS_INTERNAL_ERRORS = 3; + const int RESOLVER_STATS_RTT_AVG = 4; + const int RESOLVER_STATS_LAST_SAMPLE_TIME = 5; + const int RESOLVER_STATS_USABLE = 6; + const int RESOLVER_STATS_COUNT = 7; + const int DNS_RESOLVER_LOG_VERBOSE = 0; + const int DNS_RESOLVER_LOG_DEBUG = 1; + const int DNS_RESOLVER_LOG_INFO = 2; + const int DNS_RESOLVER_LOG_WARNING = 3; + const int DNS_RESOLVER_LOG_ERROR = 4; + const int TC_MODE_DEFAULT = 0; + const int TC_MODE_UDP_TCP = 1; + const int TRANSPORT_UNKNOWN = (-1) /* -1 */; + const int TRANSPORT_CELLULAR = 0; + const int TRANSPORT_WIFI = 1; + const int TRANSPORT_BLUETOOTH = 2; + const int TRANSPORT_ETHERNET = 3; + const int TRANSPORT_VPN = 4; + const int TRANSPORT_WIFI_AWARE = 5; + const int TRANSPORT_LOWPAN = 6; + const int TRANSPORT_TEST = 7; + const int TRANSPORT_USB = 8; + const int TRANSPORT_THREAD = 9; +} diff --git a/aidl_api/dnsresolver_aidl_interface/12/android/net/ResolverHostsParcel.aidl b/aidl_api/dnsresolver_aidl_interface/12/android/net/ResolverHostsParcel.aidl new file mode 100644 index 00000000..c24eb619 --- /dev/null +++ b/aidl_api/dnsresolver_aidl_interface/12/android/net/ResolverHostsParcel.aidl @@ -0,0 +1,24 @@ +/////////////////////////////////////////////////////////////////////////////// +// THIS FILE IS IMMUTABLE. DO NOT EDIT IN ANY CASE. // +/////////////////////////////////////////////////////////////////////////////// + +// This file is a snapshot of an AIDL file. Do not edit it manually. There are +// two cases: +// 1). this is a frozen version file - do not edit this in any case. +// 2). this is a 'current' file. If you make a backwards compatible change to +// the interface (from the latest frozen version), the build system will +// prompt you to update this file with `m -update-api`. +// +// You must not make a backward incompatible change to any AIDL file built +// with the aidl_interface module type with versions property set. The module +// type is used to build AIDL files in a way that they can be used across +// independently updatable components of the system. If a device is shipped +// with such a backward incompatible change, it has a high risk of breaking +// later when a module using the interface is updated, e.g., Mainline modules. + +package android.net; +/* @hide */ +parcelable ResolverHostsParcel { + @utf8InCpp String ipAddr; + @utf8InCpp String hostName = ""; +} diff --git a/aidl_api/dnsresolver_aidl_interface/12/android/net/ResolverOptionsParcel.aidl b/aidl_api/dnsresolver_aidl_interface/12/android/net/ResolverOptionsParcel.aidl new file mode 100644 index 00000000..e806d040 --- /dev/null +++ b/aidl_api/dnsresolver_aidl_interface/12/android/net/ResolverOptionsParcel.aidl @@ -0,0 +1,25 @@ +/////////////////////////////////////////////////////////////////////////////// +// THIS FILE IS IMMUTABLE. DO NOT EDIT IN ANY CASE. // +/////////////////////////////////////////////////////////////////////////////// + +// This file is a snapshot of an AIDL file. Do not edit it manually. There are +// two cases: +// 1). this is a frozen version file - do not edit this in any case. +// 2). this is a 'current' file. If you make a backwards compatible change to +// the interface (from the latest frozen version), the build system will +// prompt you to update this file with `m -update-api`. +// +// You must not make a backward incompatible change to any AIDL file built +// with the aidl_interface module type with versions property set. The module +// type is used to build AIDL files in a way that they can be used across +// independently updatable components of the system. If a device is shipped +// with such a backward incompatible change, it has a high risk of breaking +// later when a module using the interface is updated, e.g., Mainline modules. + +package android.net; +/* @hide */ +parcelable ResolverOptionsParcel { + android.net.ResolverHostsParcel[] hosts = {}; + int tcMode = 0; + boolean enforceDnsUid = false; +} diff --git a/aidl_api/dnsresolver_aidl_interface/12/android/net/ResolverParamsParcel.aidl b/aidl_api/dnsresolver_aidl_interface/12/android/net/ResolverParamsParcel.aidl new file mode 100644 index 00000000..8a7bf997 --- /dev/null +++ b/aidl_api/dnsresolver_aidl_interface/12/android/net/ResolverParamsParcel.aidl @@ -0,0 +1,39 @@ +/////////////////////////////////////////////////////////////////////////////// +// THIS FILE IS IMMUTABLE. DO NOT EDIT IN ANY CASE. // +/////////////////////////////////////////////////////////////////////////////// + +// This file is a snapshot of an AIDL file. Do not edit it manually. There are +// two cases: +// 1). this is a frozen version file - do not edit this in any case. +// 2). this is a 'current' file. If you make a backwards compatible change to +// the interface (from the latest frozen version), the build system will +// prompt you to update this file with `m -update-api`. +// +// You must not make a backward incompatible change to any AIDL file built +// with the aidl_interface module type with versions property set. The module +// type is used to build AIDL files in a way that they can be used across +// independently updatable components of the system. If a device is shipped +// with such a backward incompatible change, it has a high risk of breaking +// later when a module using the interface is updated, e.g., Mainline modules. + +package android.net; +/* @hide */ +parcelable ResolverParamsParcel { + int netId; + int sampleValiditySeconds; + int successThreshold; + int minSamples; + int maxSamples; + int baseTimeoutMsec; + int retryCount; + @utf8InCpp String[] servers; + @utf8InCpp String[] domains; + @utf8InCpp String tlsName; + @utf8InCpp String[] tlsServers; + @utf8InCpp String[] tlsFingerprints = {}; + @utf8InCpp String caCertificate = ""; + int tlsConnectTimeoutMs = 0; + @nullable android.net.ResolverOptionsParcel resolverOptions; + int[] transportTypes = {}; + boolean meteredNetwork = false; +} diff --git a/aidl_api/dnsresolver_aidl_interface/12/android/net/resolv/aidl/DnsHealthEventParcel.aidl b/aidl_api/dnsresolver_aidl_interface/12/android/net/resolv/aidl/DnsHealthEventParcel.aidl new file mode 100644 index 00000000..d32be919 --- /dev/null +++ b/aidl_api/dnsresolver_aidl_interface/12/android/net/resolv/aidl/DnsHealthEventParcel.aidl @@ -0,0 +1,26 @@ +/////////////////////////////////////////////////////////////////////////////// +// THIS FILE IS IMMUTABLE. DO NOT EDIT IN ANY CASE. // +/////////////////////////////////////////////////////////////////////////////// + +// This file is a snapshot of an AIDL file. Do not edit it manually. There are +// two cases: +// 1). this is a frozen version file - do not edit this in any case. +// 2). this is a 'current' file. If you make a backwards compatible change to +// the interface (from the latest frozen version), the build system will +// prompt you to update this file with `m -update-api`. +// +// You must not make a backward incompatible change to any AIDL file built +// with the aidl_interface module type with versions property set. The module +// type is used to build AIDL files in a way that they can be used across +// independently updatable components of the system. If a device is shipped +// with such a backward incompatible change, it has a high risk of breaking +// later when a module using the interface is updated, e.g., Mainline modules. + +package android.net.resolv.aidl; +/* @hide */ +@JavaDerive(toString=true) +parcelable DnsHealthEventParcel { + int netId; + int healthResult; + int[] successRttMicros; +} diff --git a/aidl_api/dnsresolver_aidl_interface/12/android/net/resolv/aidl/IDnsResolverUnsolicitedEventListener.aidl b/aidl_api/dnsresolver_aidl_interface/12/android/net/resolv/aidl/IDnsResolverUnsolicitedEventListener.aidl new file mode 100644 index 00000000..32963dfd --- /dev/null +++ b/aidl_api/dnsresolver_aidl_interface/12/android/net/resolv/aidl/IDnsResolverUnsolicitedEventListener.aidl @@ -0,0 +1,33 @@ +/////////////////////////////////////////////////////////////////////////////// +// THIS FILE IS IMMUTABLE. DO NOT EDIT IN ANY CASE. // +/////////////////////////////////////////////////////////////////////////////// + +// This file is a snapshot of an AIDL file. Do not edit it manually. There are +// two cases: +// 1). this is a frozen version file - do not edit this in any case. +// 2). this is a 'current' file. If you make a backwards compatible change to +// the interface (from the latest frozen version), the build system will +// prompt you to update this file with `m -update-api`. +// +// You must not make a backward incompatible change to any AIDL file built +// with the aidl_interface module type with versions property set. The module +// type is used to build AIDL files in a way that they can be used across +// independently updatable components of the system. If a device is shipped +// with such a backward incompatible change, it has a high risk of breaking +// later when a module using the interface is updated, e.g., Mainline modules. + +package android.net.resolv.aidl; +/* @hide */ +interface IDnsResolverUnsolicitedEventListener { + oneway void onDnsHealthEvent(in android.net.resolv.aidl.DnsHealthEventParcel dnsHealthEvent); + oneway void onNat64PrefixEvent(in android.net.resolv.aidl.Nat64PrefixEventParcel nat64PrefixEvent); + oneway void onPrivateDnsValidationEvent(in android.net.resolv.aidl.PrivateDnsValidationEventParcel privateDnsValidationEvent); + const int DNS_HEALTH_RESULT_OK = 0; + const int DNS_HEALTH_RESULT_TIMEOUT = 255; + const int PREFIX_OPERATION_ADDED = 1; + const int PREFIX_OPERATION_REMOVED = 2; + const int VALIDATION_RESULT_SUCCESS = 1; + const int VALIDATION_RESULT_FAILURE = 2; + const int PROTOCOL_DOT = 1; + const int PROTOCOL_DOH = 2; +} diff --git a/aidl_api/dnsresolver_aidl_interface/12/android/net/resolv/aidl/Nat64PrefixEventParcel.aidl b/aidl_api/dnsresolver_aidl_interface/12/android/net/resolv/aidl/Nat64PrefixEventParcel.aidl new file mode 100644 index 00000000..2daccb0e --- /dev/null +++ b/aidl_api/dnsresolver_aidl_interface/12/android/net/resolv/aidl/Nat64PrefixEventParcel.aidl @@ -0,0 +1,27 @@ +/////////////////////////////////////////////////////////////////////////////// +// THIS FILE IS IMMUTABLE. DO NOT EDIT IN ANY CASE. // +/////////////////////////////////////////////////////////////////////////////// + +// This file is a snapshot of an AIDL file. Do not edit it manually. There are +// two cases: +// 1). this is a frozen version file - do not edit this in any case. +// 2). this is a 'current' file. If you make a backwards compatible change to +// the interface (from the latest frozen version), the build system will +// prompt you to update this file with `m -update-api`. +// +// You must not make a backward incompatible change to any AIDL file built +// with the aidl_interface module type with versions property set. The module +// type is used to build AIDL files in a way that they can be used across +// independently updatable components of the system. If a device is shipped +// with such a backward incompatible change, it has a high risk of breaking +// later when a module using the interface is updated, e.g., Mainline modules. + +package android.net.resolv.aidl; +/* @hide */ +@JavaDerive(toString=true) +parcelable Nat64PrefixEventParcel { + int netId; + int prefixOperation; + @utf8InCpp String prefixAddress; + int prefixLength; +} diff --git a/aidl_api/dnsresolver_aidl_interface/12/android/net/resolv/aidl/PrivateDnsValidationEventParcel.aidl b/aidl_api/dnsresolver_aidl_interface/12/android/net/resolv/aidl/PrivateDnsValidationEventParcel.aidl new file mode 100644 index 00000000..f3bfbc76 --- /dev/null +++ b/aidl_api/dnsresolver_aidl_interface/12/android/net/resolv/aidl/PrivateDnsValidationEventParcel.aidl @@ -0,0 +1,28 @@ +/////////////////////////////////////////////////////////////////////////////// +// THIS FILE IS IMMUTABLE. DO NOT EDIT IN ANY CASE. // +/////////////////////////////////////////////////////////////////////////////// + +// This file is a snapshot of an AIDL file. Do not edit it manually. There are +// two cases: +// 1). this is a frozen version file - do not edit this in any case. +// 2). this is a 'current' file. If you make a backwards compatible change to +// the interface (from the latest frozen version), the build system will +// prompt you to update this file with `m -update-api`. +// +// You must not make a backward incompatible change to any AIDL file built +// with the aidl_interface module type with versions property set. The module +// type is used to build AIDL files in a way that they can be used across +// independently updatable components of the system. If a device is shipped +// with such a backward incompatible change, it has a high risk of breaking +// later when a module using the interface is updated, e.g., Mainline modules. + +package android.net.resolv.aidl; +/* @hide */ +@JavaDerive(toString=true) +parcelable PrivateDnsValidationEventParcel { + int netId; + @utf8InCpp String ipAddress; + @utf8InCpp String hostname; + int validation; + int protocol; +} -- cgit v1.2.3 From 69390aa5cf446fc6a3465cf1c2bd00e79973623c Mon Sep 17 00:00:00 2001 From: Ken Chen Date: Tue, 24 Oct 2023 14:49:16 +0800 Subject: Take metered information from setResolverConfiguration 1. Uses dnsresolver_aidl_interface_lateststable_version 12. 2. Get the 'metered' parameter from setResolverConfiguration and keep it in NetConfig of each network. 3. Add resolv_is_metered_network() for DnsProxyListener. Bug: 288340533 Test: atest resolv_integration_test resolv_unit_test Change-Id: I390199b93a9f5b3c0abc8f072d91153ef9fac32e --- Android.bp | 2 +- ResolverController.cpp | 3 ++- res_cache.cpp | 13 ++++++++- resolv_cache.h | 5 +++- tests/dns_responder/dns_responder_client_ndk.h | 4 +++ tests/dnsresolver_binder_test.cpp | 20 ++++++++++++-- tests/resolv_cache_unit_test.cpp | 37 +++++++++++++++++++++++++- 7 files changed, 77 insertions(+), 7 deletions(-) diff --git a/Android.bp b/Android.bp index 12e11623..f23d6062 100644 --- a/Android.bp +++ b/Android.bp @@ -53,7 +53,7 @@ cc_library_headers { ], } -dnsresolver_aidl_interface_lateststable_version = "V11" +dnsresolver_aidl_interface_lateststable_version = "V12" cc_library_static { name: "dnsresolver_aidl_interface-lateststable-ndk", diff --git a/ResolverController.cpp b/ResolverController.cpp index 53eb3dc9..9151c548 100644 --- a/ResolverController.cpp +++ b/ResolverController.cpp @@ -237,7 +237,8 @@ int ResolverController::setResolverConfiguration(const ResolverParamsParcel& res return resolv_set_nameservers(resolverParams.netId, resolverParams.servers, resolverParams.domains, res_params, - resolverParams.resolverOptions, resolverParams.transportTypes); + resolverParams.resolverOptions, resolverParams.transportTypes, + resolverParams.meteredNetwork); } int ResolverController::getResolverInfo(int32_t netId, std::vector* servers, diff --git a/res_cache.cpp b/res_cache.cpp index 68cc1e41..fbe426c8 100644 --- a/res_cache.cpp +++ b/res_cache.cpp @@ -1065,6 +1065,7 @@ struct NetConfig { int tc_mode = aidl::android::net::IDnsResolver::TC_MODE_DEFAULT; bool enforceDnsUid = false; std::vector transportTypes; + bool metered = false; }; /* gets cache associated with a network, or NULL if none exists */ @@ -1642,7 +1643,7 @@ std::vector getCustomizedTableByName(const size_t netid, const char int resolv_set_nameservers(unsigned netid, const std::vector& servers, const std::vector& domains, const res_params& params, const std::optional optionalResolverOptions, - const std::vector& transportTypes) { + const std::vector& transportTypes, bool metered) { std::vector nameservers = filter_nameservers(servers); const int numservers = static_cast(nameservers.size()); @@ -1709,6 +1710,7 @@ int resolv_set_nameservers(unsigned netid, const std::vector& serve return -EINVAL; } netconfig->transportTypes = transportTypes; + netconfig->metered = metered; if (optionalResolverOptions.has_value()) { const ResolverOptionsParcel& resolverOptions = optionalResolverOptions.value(); return netconfig->setOptions(resolverOptions); @@ -2090,6 +2092,7 @@ void resolv_netconfig_dump(DumpWriter& dw, unsigned netid) { // TODO: dump info->hosts dw.println("TC mode: %s", tc_mode_to_str(info->tc_mode)); dw.println("TransportType: %s", transport_type_to_str(info->transportTypes)); + dw.println("Metered: %s", info->metered ? "true" : "false"); } } @@ -2110,3 +2113,11 @@ bool resolv_is_enforceDnsUid_enabled_network(unsigned netid) { } return false; } + +bool resolv_is_metered_network(unsigned netid) { + std::lock_guard guard(cache_mutex); + if (const auto info = find_netconfig_locked(netid); info != nullptr) { + return info->metered; + } + return false; +} diff --git a/resolv_cache.h b/resolv_cache.h index f8437dee..18f5b117 100644 --- a/resolv_cache.h +++ b/resolv_cache.h @@ -81,7 +81,7 @@ std::vector getCustomizedTableByName(const size_t netid, const char int resolv_set_nameservers(unsigned netid, const std::vector& servers, const std::vector& domains, const res_params& params, std::optional resolverOptions, - const std::vector& transportTypes = {}); + const std::vector& transportTypes = {}, bool metered = false); // Sets options for a given network. int resolv_set_options(unsigned netid, const aidl::android::net::ResolverOptionsParcel& options); @@ -146,3 +146,6 @@ int resolv_get_max_cache_entries(unsigned netid); // Return true if the enforceDnsUid is enabled on the network. bool resolv_is_enforceDnsUid_enabled_network(unsigned netid); + +// Return true if the network is metered. +bool resolv_is_metered_network(unsigned netid); diff --git a/tests/dns_responder/dns_responder_client_ndk.h b/tests/dns_responder/dns_responder_client_ndk.h index 1b4ba35d..da064ab2 100644 --- a/tests/dns_responder/dns_responder_client_ndk.h +++ b/tests/dns_responder/dns_responder_client_ndk.h @@ -91,6 +91,10 @@ class ResolverParams { mParcel.retryCount = params[IDnsResolver::RESOLVER_PARAMS_RETRY_COUNT]; return *this; } + constexpr Builder& setMetered(const bool metered) { + mParcel.meteredNetwork = metered; + return *this; + } aidl::android::net::ResolverParamsParcel build() { return mParcel; } private: diff --git a/tests/dnsresolver_binder_test.cpp b/tests/dnsresolver_binder_test.cpp index dc30f71f..b06ce5d8 100644 --- a/tests/dnsresolver_binder_test.cpp +++ b/tests/dnsresolver_binder_test.cpp @@ -224,14 +224,14 @@ class DnsResolverBinderTest : public NetNativeTestBase { "tlsName: {}, tlsServers: [{}], " "tlsFingerprints: [{}], " "caCertificate: {}, tlsConnectTimeoutMs: {}, " - "resolverOptions: {}, transportTypes: [{}]}}", + "resolverOptions: {}, transportTypes: [{}], meteredNetwork: {}}}", parms.netId, parms.sampleValiditySeconds, parms.successThreshold, parms.minSamples, parms.maxSamples, parms.baseTimeoutMsec, parms.retryCount, fmt::join(parms.servers, ", "), fmt::join(parms.domains, ", "), parms.tlsName, fmt::join(parms.tlsServers, ", "), fmt::join(parms.tlsFingerprints, ", "), android::base::StringReplace(parms.caCertificate, "\n", "\\n", true), parms.tlsConnectTimeoutMs, toString(parms.resolverOptions), - fmt::join(parms.transportTypes, ", ")); + fmt::join(parms.transportTypes, ", "), parms.meteredNetwork); } PossibleLogData toSetResolverConfigurationLogData(const ResolverParamsParcel& parms, @@ -498,6 +498,22 @@ TEST_F(DnsResolverBinderTest, SetResolverConfiguration_TransportTypes_Default) { EXPECT_THAT(str, HasSubstr("UNKNOWN")); } +class MeteredNetworkParameterizedTest : public DnsResolverBinderTest, + public testing::WithParamInterface {}; + +INSTANTIATE_TEST_SUITE_P(SetResolverConfigurationTest, MeteredNetworkParameterizedTest, + testing::Bool(), [](const testing::TestParamInfo& info) { + return info.param ? "Metered" : "NotMetered"; + }); + +TEST_P(MeteredNetworkParameterizedTest, MeteredTest) { + const auto resolverParams = ResolverParams::Builder().setMetered(GetParam()).build(); + ::ndk::ScopedAStatus status = mDnsResolver->setResolverConfiguration(resolverParams); + EXPECT_TRUE(status.isOk()) << status.getMessage(); + + mExpectedLogDataWithPacel.push_back(toSetResolverConfigurationLogData(resolverParams)); +} + TEST_F(DnsResolverBinderTest, GetResolverInfo) { std::vector servers = {"127.0.0.1", "127.0.0.2"}; std::vector domains = {"example.com"}; diff --git a/tests/resolv_cache_unit_test.cpp b/tests/resolv_cache_unit_test.cpp index 2306438e..ef9ecb30 100644 --- a/tests/resolv_cache_unit_test.cpp +++ b/tests/resolv_cache_unit_test.cpp @@ -66,6 +66,7 @@ struct SetupParams { res_params params; aidl::android::net::ResolverOptionsParcel resolverOptions; std::vector transportTypes; + bool metered; }; struct CacheStats { @@ -206,7 +207,7 @@ class ResolvCacheTest : public NetNativeTestBase { int cacheSetupResolver(uint32_t netId, const SetupParams& setup) { return resolv_set_nameservers(netId, setup.servers, setup.domains, setup.params, - setup.resolverOptions, setup.transportTypes); + setup.resolverOptions, setup.transportTypes, setup.metered); } void cacheAddStats(uint32_t netId, int revision_id, const IPSockAddr& ipsa, @@ -960,6 +961,40 @@ TEST_F(ResolvCacheTest, IsEnforceDnsUidEnabled) { // Returns false on non-existent network EXPECT_FALSE(resolv_is_enforceDnsUid_enabled_network(TEST_NETID + 2)); } + +TEST_F(ResolvCacheTest, IsNetworkMetered) { + const SetupParams defaultCfg = { + .servers = {"127.0.0.1"}, + .domains = {"domain1.com"}, + .params = kParams, + }; + // Network #1 + EXPECT_EQ(0, cacheCreate(TEST_NETID)); + EXPECT_EQ(0, cacheSetupResolver(TEST_NETID, defaultCfg)); + EXPECT_FALSE(resolv_is_metered_network(TEST_NETID)); + + // Network #2 + EXPECT_EQ(0, cacheCreate(TEST_NETID + 1)); + EXPECT_EQ(0, cacheSetupResolver(TEST_NETID + 1, defaultCfg)); + EXPECT_FALSE(resolv_is_metered_network(TEST_NETID + 1)); + + // Change the metered setting on network #1 + const SetupParams meteredCfg = { + .servers = {"127.0.0.1"}, + .domains = {"domain1.com"}, + .params = kParams, + .metered = true, + }; + EXPECT_EQ(0, cacheSetupResolver(TEST_NETID, meteredCfg)); + EXPECT_TRUE(resolv_is_metered_network(TEST_NETID)); + + // Network #2 is unaffected + EXPECT_FALSE(resolv_is_metered_network(TEST_NETID + 1)); + + // Returns false on non-existent network + EXPECT_FALSE(resolv_is_metered_network(TEST_NETID + 2)); +} + namespace { constexpr int EAI_OK = 0; -- cgit v1.2.3 From 55525d1c6c398b3f9a821975cdfd9391e641ee0b Mon Sep 17 00:00:00 2001 From: Ken Chen Date: Tue, 24 Oct 2023 15:11:49 +0800 Subject: Replace hardcoded metered info by the real setting Pass the actual metered information into ADnsHelper_isUidNetworkingBlocked(). Extend an existing test so that it can verify the data saver feature on a metered network. Bug: 288340533 Test: atest resolv_integration_test Change-Id: I5e9ac44d0ab78f4ec48f320fbf23393e63e3898b --- DnsProxyListener.cpp | 3 +- tests/resolv_private_dns_test.cpp | 68 ++++++++++++++++++++++++--------------- tests/resolv_test_utils.h | 27 ++++++++++++++++ 3 files changed, 70 insertions(+), 28 deletions(-) diff --git a/DnsProxyListener.cpp b/DnsProxyListener.cpp index 327b9079..64b785e3 100644 --- a/DnsProxyListener.cpp +++ b/DnsProxyListener.cpp @@ -706,8 +706,7 @@ bool isUidNetworkingBlocked(uid_t uid, unsigned netId) { // application's UID. Its DNS packets are not subject to certain network restriction features. if (resolv_is_enforceDnsUid_enabled_network(netId)) return false; - // TODO: Pass metered information from CS to DNS resolver and replace the hardcode value. - return (*ADnsHelper_isUidNetworkingBlocked)(uid, /*metered=*/false) == 1; + return (*ADnsHelper_isUidNetworkingBlocked)(uid, resolv_is_metered_network(netId)) == 1; } } // namespace diff --git a/tests/resolv_private_dns_test.cpp b/tests/resolv_private_dns_test.cpp index e9d3f8e5..3e270c9c 100644 --- a/tests/resolv_private_dns_test.cpp +++ b/tests/resolv_private_dns_test.cpp @@ -345,6 +345,28 @@ class BasePrivateDnsTest : public BaseTest { EXPECT_EQ(mDnsClient.resolvService()->dump(fd, querylogCmd, std::size(querylogCmd)), 0); } + void expectQueriesAreBlocked() { + // getaddrinfo should fail + const addrinfo hints = {.ai_socktype = SOCK_DGRAM}; + EXPECT_FALSE(safe_getaddrinfo(kQueryHostname, nullptr, &hints)); + + // gethostbyname should fail + EXPECT_FALSE(gethostbyname(kQueryHostname)); + + // gethostbyaddr should fail + in6_addr v6addr; + inet_pton(AF_INET6, "2001:db8::102:304", &v6addr); + EXPECT_FALSE(gethostbyaddr(&v6addr, sizeof(v6addr), AF_INET6)); + + // resNetworkQuery should fail + int fd = resNetworkQuery(TEST_NETID, kQueryHostname, ns_c_in, ns_t_aaaa, 0); + EXPECT_TRUE(fd != -1); + + uint8_t buf[MAXPACKET] = {}; + int rcode; + EXPECT_EQ(-ECONNREFUSED, getAsyncResponse(fd, &rcode, buf, MAXPACKET)); + } + static constexpr milliseconds kExpectedDohValidationTimeWhenTimeout{1000}; static constexpr milliseconds kExpectedDohValidationTimeWhenServerUnreachable{1000}; static constexpr char kQueryHostname[] = "TransportParameterizedTest.example.com."; @@ -528,7 +550,7 @@ TEST_P(TransportParameterizedTest, MdnsGetAddrInfo_fallback) { } } -TEST_P(TransportParameterizedTest, BlockDnsQueryWithUidRule) { +TEST_P(TransportParameterizedTest, BlockDnsQuery) { SKIP_IF_BEFORE_T; constexpr char ptr_name[] = "v4v6.example.com."; // PTR record for IPv6 address 2001:db8::102:304 @@ -539,7 +561,7 @@ TEST_P(TransportParameterizedTest, BlockDnsQueryWithUidRule) { dot_backend.addMapping(r.host_name, r.type, r.addr); doh_backend.addMapping(r.host_name, r.type, r.addr); - const auto parcel = DnsResponderClient::GetDefaultResolverParamsParcel(); + auto parcel = DnsResponderClient::GetDefaultResolverParamsParcel(); ASSERT_TRUE(mDnsClient.SetResolversFromParcel(parcel)); if (testParamHasDoh()) EXPECT_TRUE(WaitForDohValidationSuccess(test::kDefaultListenAddr)); @@ -559,30 +581,24 @@ TEST_P(TransportParameterizedTest, BlockDnsQueryWithUidRule) { } dns.clearQueries(); - // Block TEST_UID's network access - ScopeBlockedUIDRule scopeBlockUidRule(mDnsClient.netdService(), TEST_UID); - - // getaddrinfo should fail - const addrinfo hints = {.ai_socktype = SOCK_DGRAM}; - EXPECT_FALSE(safe_getaddrinfo(kQueryHostname, nullptr, &hints)); - - // gethostbyname should fail - EXPECT_FALSE(gethostbyname(kQueryHostname)); - - // gethostbyaddr should fail - in6_addr v6addr; - inet_pton(AF_INET6, "2001:db8::102:304", &v6addr); - EXPECT_FALSE(gethostbyaddr(&v6addr, sizeof(v6addr), AF_INET6)); - - // resNetworkQuery should fail - int fd = resNetworkQuery(TEST_NETID, kQueryHostname, ns_c_in, ns_t_aaaa, 0); - EXPECT_TRUE(fd != -1); - - uint8_t buf[MAXPACKET] = {}; - int rcode; - EXPECT_EQ(-ECONNREFUSED, getAsyncResponse(fd, &rcode, buf, MAXPACKET)); - - expectQueries(0 /* dns */, 0 /* dot */, 0 /* doh */); + for (const bool testDataSaver : {false, true}) { + SCOPED_TRACE(fmt::format("test {}", testDataSaver ? "data saver" : "UID firewall rules")); + if (testDataSaver) { + // Data Saver applies on metered networks only. + parcel.meteredNetwork = true; + ASSERT_TRUE(mDnsClient.SetResolversFromParcel(parcel)); + + // Block network access by enabling data saver. + ScopedSetDataSaverByBPF scopedSetDataSaverByBPF(true); + ScopedChangeUID scopedChangeUID(TEST_UID); + expectQueriesAreBlocked(); + } else { + // Block network access by setting UID firewall rules. + ScopeBlockedUIDRule scopeBlockUidRule(mDnsClient.netdService(), TEST_UID); + expectQueriesAreBlocked(); + } + expectQueries(0 /* dns */, 0 /* dot */, 0 /* doh */); + } } class PrivateDnsDohTest : public BasePrivateDnsTest { diff --git a/tests/resolv_test_utils.h b/tests/resolv_test_utils.h index c5c84bc0..1406d494 100644 --- a/tests/resolv_test_utils.h +++ b/tests/resolv_test_utils.h @@ -81,6 +81,33 @@ class ScopeBlockedUIDRule { const uid_t mSavedUid; }; +// Supported from T+ only. +class ScopedSetDataSaverByBPF { + public: + ScopedSetDataSaverByBPF(bool wanted) { + if (android::modules::sdklevel::IsAtLeastT()) { + mFw = Firewall::getInstance(); + // Backup current setting. + const Result current = mFw->getDataSaverSetting(); + EXPECT_RESULT_OK(current); + if (wanted != current.value()) { + mSavedDataSaverSetting = current; + EXPECT_RESULT_OK(mFw->setDataSaver(wanted)); + } + } + }; + ~ScopedSetDataSaverByBPF() { + // Restore the setting. + if (mSavedDataSaverSetting.has_value()) { + EXPECT_RESULT_OK(mFw->setDataSaver(mSavedDataSaverSetting.value())); + } + } + + private: + Firewall* mFw; + Result mSavedDataSaverSetting; +}; + class ScopedChangeUID { public: ScopedChangeUID(uid_t testUid) : mTestUid(testUid), mSavedUid(getuid()) { -- cgit v1.2.3 From b8d156e62fa7419971609194c01d4f18c8ef0314 Mon Sep 17 00:00:00 2001 From: Ken Chen Date: Sat, 16 Sep 2023 03:33:40 +0800 Subject: Not report DNS failures caused by network restriction errors Failures caused by network access restrictions account for a large portion of metrics traffic (~27%). The change stops reporting this type of errors to cloud. Bug: 288340533 Test: Manually test 1. Install a test app that keeps sending DNS in background. 2. Force enable doze mode by adb commands. 3. Check that DNS query results of the test app is failed. 4. Use statsd_testdrive to confirm that query failures are not reported to statsd when network for the app is blocked. Change-Id: I00d9d633e8cff2a75e9d2b6c55562a7cba5f39ad --- DnsProxyListener.cpp | 34 ++++++++++++++++++++-------------- 1 file changed, 20 insertions(+), 14 deletions(-) diff --git a/DnsProxyListener.cpp b/DnsProxyListener.cpp index 327b9079..c5566f5f 100644 --- a/DnsProxyListener.cpp +++ b/DnsProxyListener.cpp @@ -322,10 +322,12 @@ void maybeLogQuery(int eventType, const android_net_context& netContext, void reportDnsEvent(int eventType, const android_net_context& netContext, int latencyUs, int returnCode, NetworkDnsEventReported& event, const std::string& query_name, - const std::vector& ip_addrs = {}, int total_ip_addr_count = 0) { - uint32_t rate = - (query_name.ends_with(".local") && is_mdns_supported_network(netContext.dns_netid) && - android::net::Experiments::getInstance()->getFlag("mdns_resolution", 1)) + bool skipStats, const std::vector& ip_addrs = {}, + int total_ip_addr_count = 0) { + int32_t rate = + skipStats ? 0 + : (query_name.ends_with(".local") && is_mdns_supported_network(netContext.dns_netid) && + android::net::Experiments::getInstance()->getFlag("mdns_resolution", 1)) ? getDnsEventSubsamplingRate(netContext.dns_netid, returnCode, true) : getDnsEventSubsamplingRate(netContext.dns_netid, returnCode, false); @@ -904,7 +906,8 @@ void DnsProxyListener::GetAddrInfoHandler::run() { int32_t rv = 0; NetworkDnsEventReported event; initDnsEvent(&event, mNetContext); - if (isUidNetworkingBlocked(mNetContext.uid, mNetContext.dns_netid)) { + const bool isUidBlocked = isUidNetworkingBlocked(mNetContext.uid, mNetContext.dns_netid); + if (isUidBlocked) { LOG(INFO) << "GetAddrInfoHandler::run: network access blocked"; rv = EAI_FAIL; } else if (startQueryLimiter(uid)) { @@ -952,7 +955,7 @@ void DnsProxyListener::GetAddrInfoHandler::run() { std::vector ip_addrs; const int total_ip_addr_count = extractGetAddrInfoAnswers(result, &ip_addrs); reportDnsEvent(INetdEventListener::EVENT_GETADDRINFO, mNetContext, latencyUs, rv, event, mHost, - ip_addrs, total_ip_addr_count); + isUidBlocked, ip_addrs, total_ip_addr_count); freeaddrinfo(result); } @@ -1116,7 +1119,8 @@ void DnsProxyListener::ResNSendHandler::run() { int ansLen = -1; NetworkDnsEventReported event; initDnsEvent(&event, mNetContext); - if (isUidNetworkingBlocked(mNetContext.uid, mNetContext.dns_netid)) { + const bool isUidBlocked = isUidNetworkingBlocked(mNetContext.uid, mNetContext.dns_netid); + if (isUidBlocked) { LOG(INFO) << "ResNSendHandler::run: network access blocked"; ansLen = -ECONNREFUSED; } else if (startQueryLimiter(uid)) { @@ -1147,7 +1151,7 @@ void DnsProxyListener::ResNSendHandler::run() { } if (rr_type == ns_t_a || rr_type == ns_t_aaaa) { reportDnsEvent(INetdEventListener::EVENT_RES_NSEND, mNetContext, latencyUs, - resNSendToAiError(ansLen, rcode), event, rr_name); + resNSendToAiError(ansLen, rcode), event, rr_name, isUidBlocked); } return; } @@ -1177,8 +1181,8 @@ void DnsProxyListener::ResNSendHandler::run() { const int total_ip_addr_count = extractResNsendAnswers(std::span(ansBuf.data(), ansLen), rr_type, &ip_addrs); reportDnsEvent(INetdEventListener::EVENT_RES_NSEND, mNetContext, latencyUs, - resNSendToAiError(ansLen, rcode), event, rr_name, ip_addrs, - total_ip_addr_count); + resNSendToAiError(ansLen, rcode), event, rr_name, /*skipStats=*/false, + ip_addrs, total_ip_addr_count); } } @@ -1320,7 +1324,8 @@ void DnsProxyListener::GetHostByNameHandler::run() { int32_t rv = 0; NetworkDnsEventReported event; initDnsEvent(&event, mNetContext); - if (isUidNetworkingBlocked(mNetContext.uid, mNetContext.dns_netid)) { + const bool isUidBlocked = isUidNetworkingBlocked(mNetContext.uid, mNetContext.dns_netid); + if (isUidBlocked) { LOG(INFO) << "GetHostByNameHandler::run: network access blocked"; rv = EAI_FAIL; } else if (startQueryLimiter(uid)) { @@ -1364,7 +1369,7 @@ void DnsProxyListener::GetHostByNameHandler::run() { std::vector ip_addrs; const int total_ip_addr_count = extractGetHostByNameAnswers(hp, &ip_addrs); reportDnsEvent(INetdEventListener::EVENT_GETHOSTBYNAME, mNetContext, latencyUs, rv, event, - mName, ip_addrs, total_ip_addr_count); + mName, isUidBlocked, ip_addrs, total_ip_addr_count); } std::string DnsProxyListener::GetHostByNameHandler::threadName() { @@ -1483,7 +1488,8 @@ void DnsProxyListener::GetHostByAddrHandler::run() { NetworkDnsEventReported event; initDnsEvent(&event, mNetContext); - if (isUidNetworkingBlocked(mNetContext.uid, mNetContext.dns_netid)) { + const bool isUidBlocked = isUidNetworkingBlocked(mNetContext.uid, mNetContext.dns_netid); + if (isUidBlocked) { LOG(INFO) << "GetHostByAddrHandler::run: network access blocked"; rv = EAI_FAIL; } else if (startQueryLimiter(uid)) { @@ -1529,7 +1535,7 @@ void DnsProxyListener::GetHostByAddrHandler::run() { } reportDnsEvent(INetdEventListener::EVENT_GETHOSTBYADDR, mNetContext, latencyUs, rv, event, - (hp && hp->h_name) ? hp->h_name : "null", {}, 0); + (hp && hp->h_name) ? hp->h_name : "null", isUidBlocked, {}, 0); } std::string DnsProxyListener::GetHostByAddrHandler::threadName() { -- cgit v1.2.3 From 1479a87c22e7aefbda8f9aa8097ed740aa047b7a Mon Sep 17 00:00:00 2001 From: Dario Freni Date: Fri, 3 Nov 2023 13:30:49 +0000 Subject: Assign default bug component to targets in this directory. This CL is being created to improve the test attribution in android. It has been found that the tests owned by this OWNERS are not not being attributed to a buganiser component. This is part of a bigger effort to attribute all of the aosp code. If you think that the buganiser component is not correct please update this CL with correct component and drop a +2. We will use the updated component and merge the CL Bug: 309090038 Test: N/A Change-Id: If4e6828b4f3567972758a5793ea29beff1b7c77a --- OWNERS | 1 + 1 file changed, 1 insertion(+) diff --git a/OWNERS b/OWNERS index c24680e9..b0e134e2 100644 --- a/OWNERS +++ b/OWNERS @@ -1,2 +1,3 @@ +# Bug component: 31808 set noparent file:platform/packages/modules/Connectivity:main:/OWNERS_core_networking -- cgit v1.2.3 From 9fc009ea6943588008cb659b85c4a3e169cb0f78 Mon Sep 17 00:00:00 2001 From: Henri Chataing Date: Fri, 3 Nov 2023 23:05:34 +0000 Subject: Update fmtlib to 10.1.1 Test: m Change-Id: I3e6d11c49244ebe9a3ecf41e0e5ac8bb94a5f8d0 --- PrivateDnsConfiguration.cpp | 4 ++-- res_debug.cpp | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/PrivateDnsConfiguration.cpp b/PrivateDnsConfiguration.cpp index d32d1007..cb1a8987 100644 --- a/PrivateDnsConfiguration.cpp +++ b/PrivateDnsConfiguration.cpp @@ -534,8 +534,8 @@ base::Result PrivateDnsConfiguration::getDotServerLocked( auto iter = netPair->second.find(identity); if (iter == netPair->second.end()) { - return Errorf("Failed to get private DNS: server {{{}/{}}} not found", identity.sockaddr, - identity.provider); + return Errorf("Failed to get private DNS: server {{{}/{}}} not found", + identity.sockaddr.toString(), identity.provider); } return &iter->second; diff --git a/res_debug.cpp b/res_debug.cpp index 848fed59..55356fea 100644 --- a/res_debug.cpp +++ b/res_debug.cpp @@ -166,7 +166,7 @@ static void do_section(ns_msg* handle, ns_sect section) { rdatalen = ns_rr_rdlen(rr); format_to(out, "; EDNS: version: {}, udp={}, flags={}\n", (rr.ttl >> 16) & 0xff, - ns_rr_class(rr), rr.ttl & 0xffff); + static_cast(ns_rr_class(rr)), rr.ttl & 0xffff); const uint8_t* cp = ns_rr_rdata(rr); while (rdatalen <= ns_rr_rdlen(rr) && rdatalen >= 4) { int i; -- cgit v1.2.3 From 57997e80cdd1a9cf6153aa1d7453a98665d74924 Mon Sep 17 00:00:00 2001 From: Ken Chen Date: Sat, 4 Nov 2023 18:02:02 +0800 Subject: [Test] Fix MTS failure on GetAddrinfo_BlockDnsQueryWithUidRule The test has different expected behavior when the device under test has the libcom.android.tethering.dns_helper and does not have the library. The test needs to check whether the libcom.android.tethering.dns_helper is installed or not. Bug: 309164580 Test: atest resolv_integration_test:ResolverTest#GetAddrinfo_BlockDnsQue ryWithUidRule Change-Id: I0b54224634e873374566fa5bf83c9d0af352e84d --- tests/Android.bp | 1 + tests/resolv_integration_test.cpp | 7 +++++-- tests/resolv_test_utils.h | 5 +++++ 3 files changed, 11 insertions(+), 2 deletions(-) diff --git a/tests/Android.bp b/tests/Android.bp index 72fb008b..aa60b1b0 100644 --- a/tests/Android.bp +++ b/tests/Android.bp @@ -201,6 +201,7 @@ cc_test { "libnetd_test_resolv_utils", "libnetdutils", "libssl", + "libc++fs", "libcutils", "netd_aidl_interface-lateststable-ndk", "netd_event_listener_interface-lateststable-ndk", diff --git a/tests/resolv_integration_test.cpp b/tests/resolv_integration_test.cpp index 6706e7c5..cae91f8a 100644 --- a/tests/resolv_integration_test.cpp +++ b/tests/resolv_integration_test.cpp @@ -48,6 +48,7 @@ #include #include +#include #include #include #include @@ -127,6 +128,8 @@ using android::netdutils::ScopedAddrinfo; using android::netdutils::Stopwatch; using android::netdutils::toHex; +namespace fs = std::filesystem; + namespace { std::pair safe_getaddrinfo_time_taken(const char* node, const char* service, @@ -4505,9 +4508,9 @@ TEST_F(ResolverTest, GetAddrinfo_BlockDnsQueryWithUidRule) { const char* hname; const int expectedErrorCode; } kTestData[] = { - {host_name, isAtLeastT() ? EAI_FAIL : EAI_NODATA}, + {host_name, (isAtLeastT() && fs::exists(DNS_HELPER)) ? EAI_FAIL : EAI_NODATA}, // To test the query with search domain. - {"howdy", isAtLeastT() ? EAI_FAIL : EAI_AGAIN}, + {"howdy", (isAtLeastT() && fs::exists(DNS_HELPER)) ? EAI_FAIL : EAI_AGAIN}, }; INetd* netdService = mDnsClient.netdService(); diff --git a/tests/resolv_test_utils.h b/tests/resolv_test_utils.h index 1406d494..03926aa8 100644 --- a/tests/resolv_test_utils.h +++ b/tests/resolv_test_utils.h @@ -437,3 +437,8 @@ void RemoveMdnsRoute(); GTEST_SKIP() << "Skipping test because SDK version is less than T."; \ } \ } while (0) + +static const std::string DNS_HELPER = + android::bpf::isUserspace64bit() + ? "/apex/com.android.tethering/lib64/libcom.android.tethering.dns_helper.so" + : "/apex/com.android.tethering/lib/libcom.android.tethering.dns_helper.so"; -- cgit v1.2.3 From ed9a0b7e1afca7eaa248fab361714a352d2d32e8 Mon Sep 17 00:00:00 2001 From: Ken Chen Date: Sat, 4 Nov 2023 18:23:59 +0800 Subject: [Test] Run BlockDnsQuery only when dependency is fulfilled Bug: 309164580 Test: atest PrivateDns/TransportParameterizedTest#BlockDnsQuery/DoH Change-Id: I80a6e4499e71d901148cc35c0af3bf588756c4ef --- tests/resolv_integration_test.cpp | 1 - tests/resolv_private_dns_test.cpp | 2 ++ tests/resolv_test_utils.h | 7 +++++++ 3 files changed, 9 insertions(+), 1 deletion(-) diff --git a/tests/resolv_integration_test.cpp b/tests/resolv_integration_test.cpp index cae91f8a..38d5567e 100644 --- a/tests/resolv_integration_test.cpp +++ b/tests/resolv_integration_test.cpp @@ -48,7 +48,6 @@ #include #include -#include #include #include #include diff --git a/tests/resolv_private_dns_test.cpp b/tests/resolv_private_dns_test.cpp index 3e270c9c..f62495ec 100644 --- a/tests/resolv_private_dns_test.cpp +++ b/tests/resolv_private_dns_test.cpp @@ -552,6 +552,8 @@ TEST_P(TransportParameterizedTest, MdnsGetAddrInfo_fallback) { TEST_P(TransportParameterizedTest, BlockDnsQuery) { SKIP_IF_BEFORE_T; + SKIP_IF_DEPENDENT_LIB_DOES_NOT_EXIST(DNS_HELPER); + constexpr char ptr_name[] = "v4v6.example.com."; // PTR record for IPv6 address 2001:db8::102:304 constexpr char ptr_addr_v6[] = diff --git a/tests/resolv_test_utils.h b/tests/resolv_test_utils.h index 03926aa8..540dd2b6 100644 --- a/tests/resolv_test_utils.h +++ b/tests/resolv_test_utils.h @@ -20,6 +20,7 @@ #include #include +#include #include #include #include @@ -442,3 +443,9 @@ static const std::string DNS_HELPER = android::bpf::isUserspace64bit() ? "/apex/com.android.tethering/lib64/libcom.android.tethering.dns_helper.so" : "/apex/com.android.tethering/lib/libcom.android.tethering.dns_helper.so"; + +#define SKIP_IF_DEPENDENT_LIB_DOES_NOT_EXIST(libPath) \ + do { \ + if (!std::filesystem::exists(libPath)) \ + GTEST_SKIP() << "Required " << (libPath) << " not found."; \ + } while (0) -- cgit v1.2.3 From b06d45edddcf58fac60275c1b4d2c972fa7a8767 Mon Sep 17 00:00:00 2001 From: Ken Chen Date: Fri, 3 Nov 2023 08:33:21 +0800 Subject: Remove "Add skip_4a_query_on_v6_linklocal_addr flag" The feature has been merged over than 1 year. It's stable and no any regression is reported. Bug: 256507503 Test: presubmit Change-Id: I3b1a66c0ae1287d7b7ea0dcd48a385415619c610 --- Experiments.h | 1 - getaddrinfo.cpp | 3 +-- tests/resolv_integration_test.cpp | 49 --------------------------------------- tests/resolv_test_utils.h | 2 -- 4 files changed, 1 insertion(+), 54 deletions(-) diff --git a/Experiments.h b/Experiments.h index e087a90a..106705b5 100644 --- a/Experiments.h +++ b/Experiments.h @@ -69,7 +69,6 @@ class Experiments { "parallel_lookup_sleep_time", "retransmission_time_interval", "retry_count", - "skip_4a_query_on_v6_linklocal_addr", "sort_nameservers", }; // This value is used in updateInternal as the default value if any flags can't be found. diff --git a/getaddrinfo.cpp b/getaddrinfo.cpp index 4ad24248..8d9ed6d4 100644 --- a/getaddrinfo.cpp +++ b/getaddrinfo.cpp @@ -1310,8 +1310,7 @@ static int _find_src_addr(const struct sockaddr* addr, struct sockaddr* src_addr return -1; } - if (Experiments::getInstance()->getFlag("skip_4a_query_on_v6_linklocal_addr", 1) && - src_addr->sa_family == AF_INET6) { + if (src_addr->sa_family == AF_INET6) { sockaddr_in6* sin6 = reinterpret_cast(src_addr); if (!allow_v6_linklocal && IN6_IS_ADDR_LINKLOCAL(&sin6->sin6_addr)) { // There is no point in sending an AAAA query because the device does not have a global diff --git a/tests/resolv_integration_test.cpp b/tests/resolv_integration_test.cpp index 6706e7c5..2bba30f6 100644 --- a/tests/resolv_integration_test.cpp +++ b/tests/resolv_integration_test.cpp @@ -7811,55 +7811,6 @@ TEST_F(ResolverMultinetworkTest, IPv6LinkLocalWithDefaultRoute) { EXPECT_EQ(GetNumQueriesForType(*dnsPair->dnsServer, ns_type::ns_t_aaaa, host_name), 1U); } -// Test if the "do not send AAAA query when IPv6 address is link-local with a default route" feature -// can be toggled by flag. -TEST_F(ResolverMultinetworkTest, IPv6LinkLocalWithDefaultRouteFlag) { - // Kernel 4.4 does not provide an IPv6 link-local address when an interface is added to a - // network. Skip it because v6 link-local address is a prerequisite for this test. - SKIP_IF_KERNEL_VERSION_LOWER_THAN(4, 9, 0); - - constexpr char host_name[] = "ohayou.example.com."; - const struct TestConfig { - std::string flagValue; - std::vector ips; - unsigned numOfQuadAQuery; - } TestConfigs[]{{"0", {"192.0.2.0", "2001:db8:cafe:d00d::31"}, 1U}, {"1", {"192.0.2.0"}, 0U}}; - - for (const auto& config : TestConfigs) { - SCOPED_TRACE(fmt::format("flagValue = {}, numOfQuadAQuery = {}", config.flagValue, - config.numOfQuadAQuery)); - - ScopedSystemProperties sp1(kSkip4aQueryOnV6LinklocalAddrFlag, config.flagValue); - ScopedPhysicalNetwork network = CreateScopedPhysicalNetwork(ConnectivityType::V4); - ASSERT_RESULT_OK(network.init()); - - // Add IPv6 default route - ASSERT_TRUE(mDnsClient.netdService() - ->networkAddRoute(network.netId(), network.ifname(), "::/0", "") - .isOk()); - - // Ensuring that routing is applied. This is required for mainline test (b/257404586). - usleep(1000 * 1000); - - const Result dnsPair = network.addIpv4Dns(); - ASSERT_RESULT_OK(dnsPair); - StartDns(*dnsPair->dnsServer, {{host_name, ns_type::ns_t_a, "192.0.2.0"}, - {host_name, ns_type::ns_t_aaaa, "2001:db8:cafe:d00d::31"}}); - - ASSERT_TRUE(network.setDnsConfiguration()); - ASSERT_TRUE(network.startTunForwarder()); - - auto result = android_getaddrinfofornet_wrapper(host_name, network.netId()); - ASSERT_RESULT_OK(result); - ScopedAddrinfo ai_results(std::move(result.value())); - std::vector result_strs = ToStrings(ai_results); - EXPECT_THAT(result_strs, testing::UnorderedElementsAreArray(config.ips)); - EXPECT_EQ(GetNumQueriesForType(*dnsPair->dnsServer, ns_type::ns_t_a, host_name), 1U); - EXPECT_EQ(GetNumQueriesForType(*dnsPair->dnsServer, ns_type::ns_t_aaaa, host_name), - config.numOfQuadAQuery); - } -} - // v6 mdns is expected to be sent when the IPv6 address is a link-local with a default route. TEST_F(ResolverMultinetworkTest, MdnsIPv6LinkLocalWithDefaultRoute) { // Kernel 4.4 does not provide an IPv6 link-local address when an interface is added to a diff --git a/tests/resolv_test_utils.h b/tests/resolv_test_utils.h index c5c84bc0..39f469c8 100644 --- a/tests/resolv_test_utils.h +++ b/tests/resolv_test_utils.h @@ -160,8 +160,6 @@ const std::string kKeepListeningUdpFlag(kFlagPrefix + "keep_listening_udp"); const std::string kParallelLookupSleepTimeFlag(kFlagPrefix + "parallel_lookup_sleep_time"); const std::string kRetransIntervalFlag(kFlagPrefix + "retransmission_time_interval"); const std::string kRetryCountFlag(kFlagPrefix + "retry_count"); -const std::string kSkip4aQueryOnV6LinklocalAddrFlag(kFlagPrefix + - "skip_4a_query_on_v6_linklocal_addr"); const std::string kSortNameserversFlag(kFlagPrefix + "sort_nameservers"); const std::string kPersistNetPrefix("persist.net."); -- cgit v1.2.3 From aa97b738af4c375e12aea1db0a17fdb6cf9081f2 Mon Sep 17 00:00:00 2001 From: Mike Yu Date: Tue, 21 Mar 2023 06:31:26 +0000 Subject: Extend ResolverParamsParcel for DoH information 1. Add DohParamsParcel.aidl used for DoH configuration parameters. 2. Extend ResolverParamsParcel to carry a nullable DohParamsParcel. 3. Run `m dnsresolver_aidl_interface-update-api` to update the AIDL interface. Bug: 240259333 Test: mm Change-Id: Icd382e66b5df45537f3d8dfd91180908516a49fb --- Android.bp | 2 +- .../current/android/net/ResolverParamsParcel.aidl | 1 + .../android/net/resolv/aidl/DohParamsParcel.aidl | 27 ++++++++++++ binder/android/net/ResolverParamsParcel.aidl | 7 +++ .../android/net/resolv/aidl/DohParamsParcel.aidl | 50 ++++++++++++++++++++++ 5 files changed, 86 insertions(+), 1 deletion(-) create mode 100644 aidl_api/dnsresolver_aidl_interface/current/android/net/resolv/aidl/DohParamsParcel.aidl create mode 100644 binder/android/net/resolv/aidl/DohParamsParcel.aidl diff --git a/Android.bp b/Android.bp index f23d6062..88dca6bf 100644 --- a/Android.bp +++ b/Android.bp @@ -151,7 +151,7 @@ aidl_interface { }, ], - frozen: true, + frozen: false, } diff --git a/aidl_api/dnsresolver_aidl_interface/current/android/net/ResolverParamsParcel.aidl b/aidl_api/dnsresolver_aidl_interface/current/android/net/ResolverParamsParcel.aidl index 8a7bf997..b6d31953 100644 --- a/aidl_api/dnsresolver_aidl_interface/current/android/net/ResolverParamsParcel.aidl +++ b/aidl_api/dnsresolver_aidl_interface/current/android/net/ResolverParamsParcel.aidl @@ -36,4 +36,5 @@ parcelable ResolverParamsParcel { @nullable android.net.ResolverOptionsParcel resolverOptions; int[] transportTypes = {}; boolean meteredNetwork = false; + @nullable android.net.resolv.aidl.DohParamsParcel dohParams; } diff --git a/aidl_api/dnsresolver_aidl_interface/current/android/net/resolv/aidl/DohParamsParcel.aidl b/aidl_api/dnsresolver_aidl_interface/current/android/net/resolv/aidl/DohParamsParcel.aidl new file mode 100644 index 00000000..ba1ea747 --- /dev/null +++ b/aidl_api/dnsresolver_aidl_interface/current/android/net/resolv/aidl/DohParamsParcel.aidl @@ -0,0 +1,27 @@ +/////////////////////////////////////////////////////////////////////////////// +// THIS FILE IS IMMUTABLE. DO NOT EDIT IN ANY CASE. // +/////////////////////////////////////////////////////////////////////////////// + +// This file is a snapshot of an AIDL file. Do not edit it manually. There are +// two cases: +// 1). this is a frozen version file - do not edit this in any case. +// 2). this is a 'current' file. If you make a backwards compatible change to +// the interface (from the latest frozen version), the build system will +// prompt you to update this file with `m -update-api`. +// +// You must not make a backward incompatible change to any AIDL file built +// with the aidl_interface module type with versions property set. The module +// type is used to build AIDL files in a way that they can be used across +// independently updatable components of the system. If a device is shipped +// with such a backward incompatible change, it has a high risk of breaking +// later when a module using the interface is updated, e.g., Mainline modules. + +package android.net.resolv.aidl; +/* @hide */ +@JavaDerive(equals=true, toString=true) @JavaOnlyImmutable +parcelable DohParamsParcel { + String name = ""; + String[] ips = {}; + String dohpath = ""; + int port = (-1) /* -1 */; +} diff --git a/binder/android/net/ResolverParamsParcel.aidl b/binder/android/net/ResolverParamsParcel.aidl index 8128fb7d..8ceb9372 100644 --- a/binder/android/net/ResolverParamsParcel.aidl +++ b/binder/android/net/ResolverParamsParcel.aidl @@ -17,6 +17,7 @@ package android.net; import android.net.ResolverOptionsParcel; +import android.net.resolv.aidl.DohParamsParcel; /** * Configuration for a resolver parameters. @@ -118,4 +119,10 @@ parcelable ResolverParamsParcel { * Whether the network is metered or not. */ boolean meteredNetwork = false; + + /** + * Information about DNS-over-HTTPS servers to use + */ + @nullable + DohParamsParcel dohParams; } diff --git a/binder/android/net/resolv/aidl/DohParamsParcel.aidl b/binder/android/net/resolv/aidl/DohParamsParcel.aidl new file mode 100644 index 00000000..d9c05db1 --- /dev/null +++ b/binder/android/net/resolv/aidl/DohParamsParcel.aidl @@ -0,0 +1,50 @@ +/* + * Copyright (C) 2023 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package android.net.resolv.aidl; + +/** + * DNS-over-HTTPS configuration parameters. Represents a single DoH server. + * + * Note that although this parcelable is for DNS-over-HTTPS configuration parameters, there is + * no field in this parcelable to specify an exact HTTPS protocol (h2 or h3) because DnsResolver + * only supports DNS-over-HTTPS/3. The configuration parameters are for h3. + * + * {@hide} + */ +@JavaDerive(equals=true, toString=true) @JavaOnlyImmutable +parcelable DohParamsParcel { + /** + * The server hostname. + */ + String name = ""; + + /** + * The server IP addresses. They are not sorted. + */ + String[] ips = {}; + + /** + * A part of the URI template used to construct the URL for DNS resolution. + * It's derived only from DNS SVCB SvcParamKey "dohpath". + */ + String dohpath = ""; + + /** + * The port used to reach the servers. + */ + int port = -1; +} -- cgit v1.2.3 From d73fa0201ffcdf7c07e9087b6947144f9de541d0 Mon Sep 17 00:00:00 2001 From: Mike Yu Date: Wed, 4 Oct 2023 09:10:06 +0000 Subject: Add JavaDerive annotation to ResolverParamsParcel - Adds equals() for ResolverParamsParcel, ResolverOptionsParcel, and ResolverHostsParcel. - Adds toString() for ResolverParamsParcel and ResolverOptionsParcel only. ResolverHostsParcel is excluded at the moment because the array of ResolverHostsParcel elements (ResolverOptionsParcel.hosts) could potentially be very large. This CL would simplify the code for logging as well as these parcelables' unit tests. Bug: 240259333 Test: mm Change-Id: Ifcaae33928aac3d2b6a155b0ca642913e6ab2a65 --- .../current/android/net/ResolverHostsParcel.aidl | 1 + .../current/android/net/ResolverOptionsParcel.aidl | 1 + .../current/android/net/ResolverParamsParcel.aidl | 1 + binder/android/net/ResolverHostsParcel.aidl | 1 + binder/android/net/ResolverOptionsParcel.aidl | 1 + binder/android/net/ResolverParamsParcel.aidl | 1 + 6 files changed, 6 insertions(+) diff --git a/aidl_api/dnsresolver_aidl_interface/current/android/net/ResolverHostsParcel.aidl b/aidl_api/dnsresolver_aidl_interface/current/android/net/ResolverHostsParcel.aidl index c24eb619..2a1c748f 100644 --- a/aidl_api/dnsresolver_aidl_interface/current/android/net/ResolverHostsParcel.aidl +++ b/aidl_api/dnsresolver_aidl_interface/current/android/net/ResolverHostsParcel.aidl @@ -18,6 +18,7 @@ package android.net; /* @hide */ +@JavaDerive(equals=true) parcelable ResolverHostsParcel { @utf8InCpp String ipAddr; @utf8InCpp String hostName = ""; diff --git a/aidl_api/dnsresolver_aidl_interface/current/android/net/ResolverOptionsParcel.aidl b/aidl_api/dnsresolver_aidl_interface/current/android/net/ResolverOptionsParcel.aidl index e806d040..b07263f8 100644 --- a/aidl_api/dnsresolver_aidl_interface/current/android/net/ResolverOptionsParcel.aidl +++ b/aidl_api/dnsresolver_aidl_interface/current/android/net/ResolverOptionsParcel.aidl @@ -18,6 +18,7 @@ package android.net; /* @hide */ +@JavaDerive(equals=true, toString=true) parcelable ResolverOptionsParcel { android.net.ResolverHostsParcel[] hosts = {}; int tcMode = 0; diff --git a/aidl_api/dnsresolver_aidl_interface/current/android/net/ResolverParamsParcel.aidl b/aidl_api/dnsresolver_aidl_interface/current/android/net/ResolverParamsParcel.aidl index b6d31953..2dd93dd5 100644 --- a/aidl_api/dnsresolver_aidl_interface/current/android/net/ResolverParamsParcel.aidl +++ b/aidl_api/dnsresolver_aidl_interface/current/android/net/ResolverParamsParcel.aidl @@ -18,6 +18,7 @@ package android.net; /* @hide */ +@JavaDerive(equals=true, toString=true) parcelable ResolverParamsParcel { int netId; int sampleValiditySeconds; diff --git a/binder/android/net/ResolverHostsParcel.aidl b/binder/android/net/ResolverHostsParcel.aidl index 6b372b18..054fca79 100644 --- a/binder/android/net/ResolverHostsParcel.aidl +++ b/binder/android/net/ResolverHostsParcel.aidl @@ -23,6 +23,7 @@ package android.net; * * {@hide} */ +@JavaDerive(equals=true) parcelable ResolverHostsParcel { /** * The IPv4 or IPv6 address corresponding to |hostName| field. diff --git a/binder/android/net/ResolverOptionsParcel.aidl b/binder/android/net/ResolverOptionsParcel.aidl index bca9fb65..dd64cf8c 100644 --- a/binder/android/net/ResolverOptionsParcel.aidl +++ b/binder/android/net/ResolverOptionsParcel.aidl @@ -23,6 +23,7 @@ import android.net.ResolverHostsParcel; * * {@hide} */ +@JavaDerive(equals=true, toString=true) parcelable ResolverOptionsParcel { /** * An IP/hostname mapping table for DNS local lookup customization. diff --git a/binder/android/net/ResolverParamsParcel.aidl b/binder/android/net/ResolverParamsParcel.aidl index 8ceb9372..1aee26bd 100644 --- a/binder/android/net/ResolverParamsParcel.aidl +++ b/binder/android/net/ResolverParamsParcel.aidl @@ -24,6 +24,7 @@ import android.net.resolv.aidl.DohParamsParcel; * * {@hide} */ +@JavaDerive(equals=true, toString=true) parcelable ResolverParamsParcel { /** * The network ID of the network for which information should be configured. -- cgit v1.2.3 From fea7ecb3a5e7adde0d51231b7f797944a4282390 Mon Sep 17 00:00:00 2001 From: Mike Yu Date: Thu, 2 Nov 2023 06:12:16 +0000 Subject: Freeze DnsResolver AIDL interfaces to v13 This CL is generated by `m dnsresolver_aidl_interface-freeze-api`. Bug: 240259333 Test: will run atest after Andorid.bp is updated to use DnsResolver v13 Change-Id: I21a3a8470472a211f72f09b0a4d0104df6d911eb --- Android.bp | 6 +- aidl_api/dnsresolver_aidl_interface/13/.hash | 1 + .../13/android/net/IDnsResolver.aidl | 69 ++++++++++++++++++++++ .../13/android/net/ResolverHostsParcel.aidl | 25 ++++++++ .../13/android/net/ResolverOptionsParcel.aidl | 26 ++++++++ .../13/android/net/ResolverParamsParcel.aidl | 41 +++++++++++++ .../net/resolv/aidl/DnsHealthEventParcel.aidl | 26 ++++++++ .../android/net/resolv/aidl/DohParamsParcel.aidl | 27 +++++++++ .../aidl/IDnsResolverUnsolicitedEventListener.aidl | 33 +++++++++++ .../net/resolv/aidl/Nat64PrefixEventParcel.aidl | 27 +++++++++ .../aidl/PrivateDnsValidationEventParcel.aidl | 28 +++++++++ 11 files changed, 308 insertions(+), 1 deletion(-) create mode 100644 aidl_api/dnsresolver_aidl_interface/13/.hash create mode 100644 aidl_api/dnsresolver_aidl_interface/13/android/net/IDnsResolver.aidl create mode 100644 aidl_api/dnsresolver_aidl_interface/13/android/net/ResolverHostsParcel.aidl create mode 100644 aidl_api/dnsresolver_aidl_interface/13/android/net/ResolverOptionsParcel.aidl create mode 100644 aidl_api/dnsresolver_aidl_interface/13/android/net/ResolverParamsParcel.aidl create mode 100644 aidl_api/dnsresolver_aidl_interface/13/android/net/resolv/aidl/DnsHealthEventParcel.aidl create mode 100644 aidl_api/dnsresolver_aidl_interface/13/android/net/resolv/aidl/DohParamsParcel.aidl create mode 100644 aidl_api/dnsresolver_aidl_interface/13/android/net/resolv/aidl/IDnsResolverUnsolicitedEventListener.aidl create mode 100644 aidl_api/dnsresolver_aidl_interface/13/android/net/resolv/aidl/Nat64PrefixEventParcel.aidl create mode 100644 aidl_api/dnsresolver_aidl_interface/13/android/net/resolv/aidl/PrivateDnsValidationEventParcel.aidl diff --git a/Android.bp b/Android.bp index 88dca6bf..9aa7f554 100644 --- a/Android.bp +++ b/Android.bp @@ -149,9 +149,13 @@ aidl_interface { version: "12", imports: ["netd_event_listener_interface-V1"], }, + { + version: "13", + imports: ["netd_event_listener_interface-V1"], + }, ], - frozen: false, + frozen: true, } diff --git a/aidl_api/dnsresolver_aidl_interface/13/.hash b/aidl_api/dnsresolver_aidl_interface/13/.hash new file mode 100644 index 00000000..6b78f7b1 --- /dev/null +++ b/aidl_api/dnsresolver_aidl_interface/13/.hash @@ -0,0 +1 @@ +a1e64ca6e3e9aafead71a3415d939207725d39d5 diff --git a/aidl_api/dnsresolver_aidl_interface/13/android/net/IDnsResolver.aidl b/aidl_api/dnsresolver_aidl_interface/13/android/net/IDnsResolver.aidl new file mode 100644 index 00000000..6b539c47 --- /dev/null +++ b/aidl_api/dnsresolver_aidl_interface/13/android/net/IDnsResolver.aidl @@ -0,0 +1,69 @@ +/////////////////////////////////////////////////////////////////////////////// +// THIS FILE IS IMMUTABLE. DO NOT EDIT IN ANY CASE. // +/////////////////////////////////////////////////////////////////////////////// + +// This file is a snapshot of an AIDL file. Do not edit it manually. There are +// two cases: +// 1). this is a frozen version file - do not edit this in any case. +// 2). this is a 'current' file. If you make a backwards compatible change to +// the interface (from the latest frozen version), the build system will +// prompt you to update this file with `m -update-api`. +// +// You must not make a backward incompatible change to any AIDL file built +// with the aidl_interface module type with versions property set. The module +// type is used to build AIDL files in a way that they can be used across +// independently updatable components of the system. If a device is shipped +// with such a backward incompatible change, it has a high risk of breaking +// later when a module using the interface is updated, e.g., Mainline modules. + +package android.net; +/* @hide */ +interface IDnsResolver { + boolean isAlive(); + void registerEventListener(android.net.metrics.INetdEventListener listener); + void setResolverConfiguration(in android.net.ResolverParamsParcel resolverParams); + void getResolverInfo(int netId, out @utf8InCpp String[] servers, out @utf8InCpp String[] domains, out @utf8InCpp String[] tlsServers, out int[] params, out int[] stats, out int[] wait_for_pending_req_timeout_count); + void startPrefix64Discovery(int netId); + void stopPrefix64Discovery(int netId); + @utf8InCpp String getPrefix64(int netId); + void createNetworkCache(int netId); + void destroyNetworkCache(int netId); + void setLogSeverity(int logSeverity); + void flushNetworkCache(int netId); + void setPrefix64(int netId, @utf8InCpp String prefix); + void registerUnsolicitedEventListener(android.net.resolv.aidl.IDnsResolverUnsolicitedEventListener listener); + void setResolverOptions(int netId, in android.net.ResolverOptionsParcel optionParams); + const int RESOLVER_PARAMS_SAMPLE_VALIDITY = 0; + const int RESOLVER_PARAMS_SUCCESS_THRESHOLD = 1; + const int RESOLVER_PARAMS_MIN_SAMPLES = 2; + const int RESOLVER_PARAMS_MAX_SAMPLES = 3; + const int RESOLVER_PARAMS_BASE_TIMEOUT_MSEC = 4; + const int RESOLVER_PARAMS_RETRY_COUNT = 5; + const int RESOLVER_PARAMS_COUNT = 6; + const int RESOLVER_STATS_SUCCESSES = 0; + const int RESOLVER_STATS_ERRORS = 1; + const int RESOLVER_STATS_TIMEOUTS = 2; + const int RESOLVER_STATS_INTERNAL_ERRORS = 3; + const int RESOLVER_STATS_RTT_AVG = 4; + const int RESOLVER_STATS_LAST_SAMPLE_TIME = 5; + const int RESOLVER_STATS_USABLE = 6; + const int RESOLVER_STATS_COUNT = 7; + const int DNS_RESOLVER_LOG_VERBOSE = 0; + const int DNS_RESOLVER_LOG_DEBUG = 1; + const int DNS_RESOLVER_LOG_INFO = 2; + const int DNS_RESOLVER_LOG_WARNING = 3; + const int DNS_RESOLVER_LOG_ERROR = 4; + const int TC_MODE_DEFAULT = 0; + const int TC_MODE_UDP_TCP = 1; + const int TRANSPORT_UNKNOWN = (-1) /* -1 */; + const int TRANSPORT_CELLULAR = 0; + const int TRANSPORT_WIFI = 1; + const int TRANSPORT_BLUETOOTH = 2; + const int TRANSPORT_ETHERNET = 3; + const int TRANSPORT_VPN = 4; + const int TRANSPORT_WIFI_AWARE = 5; + const int TRANSPORT_LOWPAN = 6; + const int TRANSPORT_TEST = 7; + const int TRANSPORT_USB = 8; + const int TRANSPORT_THREAD = 9; +} diff --git a/aidl_api/dnsresolver_aidl_interface/13/android/net/ResolverHostsParcel.aidl b/aidl_api/dnsresolver_aidl_interface/13/android/net/ResolverHostsParcel.aidl new file mode 100644 index 00000000..2a1c748f --- /dev/null +++ b/aidl_api/dnsresolver_aidl_interface/13/android/net/ResolverHostsParcel.aidl @@ -0,0 +1,25 @@ +/////////////////////////////////////////////////////////////////////////////// +// THIS FILE IS IMMUTABLE. DO NOT EDIT IN ANY CASE. // +/////////////////////////////////////////////////////////////////////////////// + +// This file is a snapshot of an AIDL file. Do not edit it manually. There are +// two cases: +// 1). this is a frozen version file - do not edit this in any case. +// 2). this is a 'current' file. If you make a backwards compatible change to +// the interface (from the latest frozen version), the build system will +// prompt you to update this file with `m -update-api`. +// +// You must not make a backward incompatible change to any AIDL file built +// with the aidl_interface module type with versions property set. The module +// type is used to build AIDL files in a way that they can be used across +// independently updatable components of the system. If a device is shipped +// with such a backward incompatible change, it has a high risk of breaking +// later when a module using the interface is updated, e.g., Mainline modules. + +package android.net; +/* @hide */ +@JavaDerive(equals=true) +parcelable ResolverHostsParcel { + @utf8InCpp String ipAddr; + @utf8InCpp String hostName = ""; +} diff --git a/aidl_api/dnsresolver_aidl_interface/13/android/net/ResolverOptionsParcel.aidl b/aidl_api/dnsresolver_aidl_interface/13/android/net/ResolverOptionsParcel.aidl new file mode 100644 index 00000000..b07263f8 --- /dev/null +++ b/aidl_api/dnsresolver_aidl_interface/13/android/net/ResolverOptionsParcel.aidl @@ -0,0 +1,26 @@ +/////////////////////////////////////////////////////////////////////////////// +// THIS FILE IS IMMUTABLE. DO NOT EDIT IN ANY CASE. // +/////////////////////////////////////////////////////////////////////////////// + +// This file is a snapshot of an AIDL file. Do not edit it manually. There are +// two cases: +// 1). this is a frozen version file - do not edit this in any case. +// 2). this is a 'current' file. If you make a backwards compatible change to +// the interface (from the latest frozen version), the build system will +// prompt you to update this file with `m -update-api`. +// +// You must not make a backward incompatible change to any AIDL file built +// with the aidl_interface module type with versions property set. The module +// type is used to build AIDL files in a way that they can be used across +// independently updatable components of the system. If a device is shipped +// with such a backward incompatible change, it has a high risk of breaking +// later when a module using the interface is updated, e.g., Mainline modules. + +package android.net; +/* @hide */ +@JavaDerive(equals=true, toString=true) +parcelable ResolverOptionsParcel { + android.net.ResolverHostsParcel[] hosts = {}; + int tcMode = 0; + boolean enforceDnsUid = false; +} diff --git a/aidl_api/dnsresolver_aidl_interface/13/android/net/ResolverParamsParcel.aidl b/aidl_api/dnsresolver_aidl_interface/13/android/net/ResolverParamsParcel.aidl new file mode 100644 index 00000000..2dd93dd5 --- /dev/null +++ b/aidl_api/dnsresolver_aidl_interface/13/android/net/ResolverParamsParcel.aidl @@ -0,0 +1,41 @@ +/////////////////////////////////////////////////////////////////////////////// +// THIS FILE IS IMMUTABLE. DO NOT EDIT IN ANY CASE. // +/////////////////////////////////////////////////////////////////////////////// + +// This file is a snapshot of an AIDL file. Do not edit it manually. There are +// two cases: +// 1). this is a frozen version file - do not edit this in any case. +// 2). this is a 'current' file. If you make a backwards compatible change to +// the interface (from the latest frozen version), the build system will +// prompt you to update this file with `m -update-api`. +// +// You must not make a backward incompatible change to any AIDL file built +// with the aidl_interface module type with versions property set. The module +// type is used to build AIDL files in a way that they can be used across +// independently updatable components of the system. If a device is shipped +// with such a backward incompatible change, it has a high risk of breaking +// later when a module using the interface is updated, e.g., Mainline modules. + +package android.net; +/* @hide */ +@JavaDerive(equals=true, toString=true) +parcelable ResolverParamsParcel { + int netId; + int sampleValiditySeconds; + int successThreshold; + int minSamples; + int maxSamples; + int baseTimeoutMsec; + int retryCount; + @utf8InCpp String[] servers; + @utf8InCpp String[] domains; + @utf8InCpp String tlsName; + @utf8InCpp String[] tlsServers; + @utf8InCpp String[] tlsFingerprints = {}; + @utf8InCpp String caCertificate = ""; + int tlsConnectTimeoutMs = 0; + @nullable android.net.ResolverOptionsParcel resolverOptions; + int[] transportTypes = {}; + boolean meteredNetwork = false; + @nullable android.net.resolv.aidl.DohParamsParcel dohParams; +} diff --git a/aidl_api/dnsresolver_aidl_interface/13/android/net/resolv/aidl/DnsHealthEventParcel.aidl b/aidl_api/dnsresolver_aidl_interface/13/android/net/resolv/aidl/DnsHealthEventParcel.aidl new file mode 100644 index 00000000..d32be919 --- /dev/null +++ b/aidl_api/dnsresolver_aidl_interface/13/android/net/resolv/aidl/DnsHealthEventParcel.aidl @@ -0,0 +1,26 @@ +/////////////////////////////////////////////////////////////////////////////// +// THIS FILE IS IMMUTABLE. DO NOT EDIT IN ANY CASE. // +/////////////////////////////////////////////////////////////////////////////// + +// This file is a snapshot of an AIDL file. Do not edit it manually. There are +// two cases: +// 1). this is a frozen version file - do not edit this in any case. +// 2). this is a 'current' file. If you make a backwards compatible change to +// the interface (from the latest frozen version), the build system will +// prompt you to update this file with `m -update-api`. +// +// You must not make a backward incompatible change to any AIDL file built +// with the aidl_interface module type with versions property set. The module +// type is used to build AIDL files in a way that they can be used across +// independently updatable components of the system. If a device is shipped +// with such a backward incompatible change, it has a high risk of breaking +// later when a module using the interface is updated, e.g., Mainline modules. + +package android.net.resolv.aidl; +/* @hide */ +@JavaDerive(toString=true) +parcelable DnsHealthEventParcel { + int netId; + int healthResult; + int[] successRttMicros; +} diff --git a/aidl_api/dnsresolver_aidl_interface/13/android/net/resolv/aidl/DohParamsParcel.aidl b/aidl_api/dnsresolver_aidl_interface/13/android/net/resolv/aidl/DohParamsParcel.aidl new file mode 100644 index 00000000..ba1ea747 --- /dev/null +++ b/aidl_api/dnsresolver_aidl_interface/13/android/net/resolv/aidl/DohParamsParcel.aidl @@ -0,0 +1,27 @@ +/////////////////////////////////////////////////////////////////////////////// +// THIS FILE IS IMMUTABLE. DO NOT EDIT IN ANY CASE. // +/////////////////////////////////////////////////////////////////////////////// + +// This file is a snapshot of an AIDL file. Do not edit it manually. There are +// two cases: +// 1). this is a frozen version file - do not edit this in any case. +// 2). this is a 'current' file. If you make a backwards compatible change to +// the interface (from the latest frozen version), the build system will +// prompt you to update this file with `m -update-api`. +// +// You must not make a backward incompatible change to any AIDL file built +// with the aidl_interface module type with versions property set. The module +// type is used to build AIDL files in a way that they can be used across +// independently updatable components of the system. If a device is shipped +// with such a backward incompatible change, it has a high risk of breaking +// later when a module using the interface is updated, e.g., Mainline modules. + +package android.net.resolv.aidl; +/* @hide */ +@JavaDerive(equals=true, toString=true) @JavaOnlyImmutable +parcelable DohParamsParcel { + String name = ""; + String[] ips = {}; + String dohpath = ""; + int port = (-1) /* -1 */; +} diff --git a/aidl_api/dnsresolver_aidl_interface/13/android/net/resolv/aidl/IDnsResolverUnsolicitedEventListener.aidl b/aidl_api/dnsresolver_aidl_interface/13/android/net/resolv/aidl/IDnsResolverUnsolicitedEventListener.aidl new file mode 100644 index 00000000..32963dfd --- /dev/null +++ b/aidl_api/dnsresolver_aidl_interface/13/android/net/resolv/aidl/IDnsResolverUnsolicitedEventListener.aidl @@ -0,0 +1,33 @@ +/////////////////////////////////////////////////////////////////////////////// +// THIS FILE IS IMMUTABLE. DO NOT EDIT IN ANY CASE. // +/////////////////////////////////////////////////////////////////////////////// + +// This file is a snapshot of an AIDL file. Do not edit it manually. There are +// two cases: +// 1). this is a frozen version file - do not edit this in any case. +// 2). this is a 'current' file. If you make a backwards compatible change to +// the interface (from the latest frozen version), the build system will +// prompt you to update this file with `m -update-api`. +// +// You must not make a backward incompatible change to any AIDL file built +// with the aidl_interface module type with versions property set. The module +// type is used to build AIDL files in a way that they can be used across +// independently updatable components of the system. If a device is shipped +// with such a backward incompatible change, it has a high risk of breaking +// later when a module using the interface is updated, e.g., Mainline modules. + +package android.net.resolv.aidl; +/* @hide */ +interface IDnsResolverUnsolicitedEventListener { + oneway void onDnsHealthEvent(in android.net.resolv.aidl.DnsHealthEventParcel dnsHealthEvent); + oneway void onNat64PrefixEvent(in android.net.resolv.aidl.Nat64PrefixEventParcel nat64PrefixEvent); + oneway void onPrivateDnsValidationEvent(in android.net.resolv.aidl.PrivateDnsValidationEventParcel privateDnsValidationEvent); + const int DNS_HEALTH_RESULT_OK = 0; + const int DNS_HEALTH_RESULT_TIMEOUT = 255; + const int PREFIX_OPERATION_ADDED = 1; + const int PREFIX_OPERATION_REMOVED = 2; + const int VALIDATION_RESULT_SUCCESS = 1; + const int VALIDATION_RESULT_FAILURE = 2; + const int PROTOCOL_DOT = 1; + const int PROTOCOL_DOH = 2; +} diff --git a/aidl_api/dnsresolver_aidl_interface/13/android/net/resolv/aidl/Nat64PrefixEventParcel.aidl b/aidl_api/dnsresolver_aidl_interface/13/android/net/resolv/aidl/Nat64PrefixEventParcel.aidl new file mode 100644 index 00000000..2daccb0e --- /dev/null +++ b/aidl_api/dnsresolver_aidl_interface/13/android/net/resolv/aidl/Nat64PrefixEventParcel.aidl @@ -0,0 +1,27 @@ +/////////////////////////////////////////////////////////////////////////////// +// THIS FILE IS IMMUTABLE. DO NOT EDIT IN ANY CASE. // +/////////////////////////////////////////////////////////////////////////////// + +// This file is a snapshot of an AIDL file. Do not edit it manually. There are +// two cases: +// 1). this is a frozen version file - do not edit this in any case. +// 2). this is a 'current' file. If you make a backwards compatible change to +// the interface (from the latest frozen version), the build system will +// prompt you to update this file with `m -update-api`. +// +// You must not make a backward incompatible change to any AIDL file built +// with the aidl_interface module type with versions property set. The module +// type is used to build AIDL files in a way that they can be used across +// independently updatable components of the system. If a device is shipped +// with such a backward incompatible change, it has a high risk of breaking +// later when a module using the interface is updated, e.g., Mainline modules. + +package android.net.resolv.aidl; +/* @hide */ +@JavaDerive(toString=true) +parcelable Nat64PrefixEventParcel { + int netId; + int prefixOperation; + @utf8InCpp String prefixAddress; + int prefixLength; +} diff --git a/aidl_api/dnsresolver_aidl_interface/13/android/net/resolv/aidl/PrivateDnsValidationEventParcel.aidl b/aidl_api/dnsresolver_aidl_interface/13/android/net/resolv/aidl/PrivateDnsValidationEventParcel.aidl new file mode 100644 index 00000000..f3bfbc76 --- /dev/null +++ b/aidl_api/dnsresolver_aidl_interface/13/android/net/resolv/aidl/PrivateDnsValidationEventParcel.aidl @@ -0,0 +1,28 @@ +/////////////////////////////////////////////////////////////////////////////// +// THIS FILE IS IMMUTABLE. DO NOT EDIT IN ANY CASE. // +/////////////////////////////////////////////////////////////////////////////// + +// This file is a snapshot of an AIDL file. Do not edit it manually. There are +// two cases: +// 1). this is a frozen version file - do not edit this in any case. +// 2). this is a 'current' file. If you make a backwards compatible change to +// the interface (from the latest frozen version), the build system will +// prompt you to update this file with `m -update-api`. +// +// You must not make a backward incompatible change to any AIDL file built +// with the aidl_interface module type with versions property set. The module +// type is used to build AIDL files in a way that they can be used across +// independently updatable components of the system. If a device is shipped +// with such a backward incompatible change, it has a high risk of breaking +// later when a module using the interface is updated, e.g., Mainline modules. + +package android.net.resolv.aidl; +/* @hide */ +@JavaDerive(toString=true) +parcelable PrivateDnsValidationEventParcel { + int netId; + @utf8InCpp String ipAddress; + @utf8InCpp String hostname; + int validation; + int protocol; +} -- cgit v1.2.3 From 921db2ae9c5f5483aa0a79e6687f2667588ca619 Mon Sep 17 00:00:00 2001 From: Mike Yu Date: Thu, 12 Jan 2023 12:28:06 +0000 Subject: Update Andorid.bp to use DnsResolver v13 Bug: 240259333 Test: atest Change-Id: I09a7d3059462cf8355f43a1d26723f9e5a9138f1 --- Android.bp | 2 +- tests/dns_responder/dns_responder_client_ndk.cpp | 2 ++ tests/dns_responder/dns_responder_client_ndk.h | 5 +++++ tests/dnsresolver_binder_test.cpp | 28 ++++++++++++++++++++++-- 4 files changed, 34 insertions(+), 3 deletions(-) diff --git a/Android.bp b/Android.bp index 9aa7f554..f0fd5c63 100644 --- a/Android.bp +++ b/Android.bp @@ -53,7 +53,7 @@ cc_library_headers { ], } -dnsresolver_aidl_interface_lateststable_version = "V12" +dnsresolver_aidl_interface_lateststable_version = "V13" cc_library_static { name: "dnsresolver_aidl_interface-lateststable-ndk", diff --git a/tests/dns_responder/dns_responder_client_ndk.cpp b/tests/dns_responder/dns_responder_client_ndk.cpp index ee360dc2..52a170a7 100644 --- a/tests/dns_responder/dns_responder_client_ndk.cpp +++ b/tests/dns_responder/dns_responder_client_ndk.cpp @@ -30,6 +30,7 @@ using aidl::android::net::IDnsResolver; using aidl::android::net::INetd; using aidl::android::net::ResolverOptionsParcel; using aidl::android::net::ResolverParamsParcel; +using aidl::android::net::resolv::aidl::DohParamsParcel; using android::base::Error; using android::base::Result; using android::net::ResolverStats; @@ -51,6 +52,7 @@ ResolverParams::Builder::Builder() { mParcel.tlsServers = {kDefaultServer}; mParcel.caCertificate = kCaCert; mParcel.resolverOptions = ResolverOptionsParcel{}; // optional, must be explicitly set. + mParcel.dohParams = std::nullopt; } void DnsResponderClient::SetupMappings(unsigned numHosts, const std::vector& domains, diff --git a/tests/dns_responder/dns_responder_client_ndk.h b/tests/dns_responder/dns_responder_client_ndk.h index da064ab2..2adc3c89 100644 --- a/tests/dns_responder/dns_responder_client_ndk.h +++ b/tests/dns_responder/dns_responder_client_ndk.h @@ -95,6 +95,11 @@ class ResolverParams { mParcel.meteredNetwork = metered; return *this; } + constexpr Builder& setDohParams( + const aidl::android::net::resolv::aidl::DohParamsParcel& dohParams) { + mParcel.dohParams = dohParams; + return *this; + } aidl::android::net::ResolverParamsParcel build() { return mParcel; } private: diff --git a/tests/dnsresolver_binder_test.cpp b/tests/dnsresolver_binder_test.cpp index b06ce5d8..2d9801e0 100644 --- a/tests/dnsresolver_binder_test.cpp +++ b/tests/dnsresolver_binder_test.cpp @@ -52,6 +52,7 @@ using aidl::android::net::ResolverHostsParcel; using aidl::android::net::ResolverOptionsParcel; using aidl::android::net::ResolverParamsParcel; using aidl::android::net::metrics::INetdEventListener; +using aidl::android::net::resolv::aidl::DohParamsParcel; using android::base::ReadFdToString; using android::base::unique_fd; using android::net::ResolverStats; @@ -215,6 +216,10 @@ class DnsResolverBinderTest : public NetNativeTestBase { toString(parms->hosts), parms->tcMode, parms->enforceDnsUid); } + std::string toString(const std::optional& params) { + return params.has_value() ? params.value().toString() : "(null)"; + } + std::string toString(const ResolverParamsParcel& parms) { return fmt::format( "ResolverParamsParcel{{netId: {}, sampleValiditySeconds: {}, successThreshold: {}, " @@ -224,14 +229,15 @@ class DnsResolverBinderTest : public NetNativeTestBase { "tlsName: {}, tlsServers: [{}], " "tlsFingerprints: [{}], " "caCertificate: {}, tlsConnectTimeoutMs: {}, " - "resolverOptions: {}, transportTypes: [{}], meteredNetwork: {}}}", + "resolverOptions: {}, transportTypes: [{}], meteredNetwork: {}, dohParams: {}}}", parms.netId, parms.sampleValiditySeconds, parms.successThreshold, parms.minSamples, parms.maxSamples, parms.baseTimeoutMsec, parms.retryCount, fmt::join(parms.servers, ", "), fmt::join(parms.domains, ", "), parms.tlsName, fmt::join(parms.tlsServers, ", "), fmt::join(parms.tlsFingerprints, ", "), android::base::StringReplace(parms.caCertificate, "\n", "\\n", true), parms.tlsConnectTimeoutMs, toString(parms.resolverOptions), - fmt::join(parms.transportTypes, ", "), parms.meteredNetwork); + fmt::join(parms.transportTypes, ", "), parms.meteredNetwork, + toString(parms.dohParams)); } PossibleLogData toSetResolverConfigurationLogData(const ResolverParamsParcel& parms, @@ -498,6 +504,24 @@ TEST_F(DnsResolverBinderTest, SetResolverConfiguration_TransportTypes_Default) { EXPECT_THAT(str, HasSubstr("UNKNOWN")); } +TEST_F(DnsResolverBinderTest, SetResolverConfiguration_DohParams) { + const auto paramsWithoutDohParams = ResolverParams::Builder().build(); + ::ndk::ScopedAStatus status = mDnsResolver->setResolverConfiguration(paramsWithoutDohParams); + EXPECT_TRUE(status.isOk()) << status.getMessage(); + mExpectedLogDataWithPacel.push_back(toSetResolverConfigurationLogData(paramsWithoutDohParams)); + + const DohParamsParcel dohParams = { + .name = "doh.google", + .ips = {"1.2.3.4", "2001:db8::2"}, + .dohpath = "/dns-query{?dns}", + .port = 443, + }; + const auto paramsWithDohParams = ResolverParams::Builder().setDohParams(dohParams).build(); + status = mDnsResolver->setResolverConfiguration(paramsWithDohParams); + EXPECT_TRUE(status.isOk()) << status.getMessage(); + mExpectedLogDataWithPacel.push_back(toSetResolverConfigurationLogData(paramsWithDohParams)); +} + class MeteredNetworkParameterizedTest : public DnsResolverBinderTest, public testing::WithParamInterface {}; -- cgit v1.2.3 From 08422a0dcb5e93da7f7be65e5e5237c64b1000f7 Mon Sep 17 00:00:00 2001 From: Ken Chen Date: Wed, 8 Nov 2023 16:46:26 +0800 Subject: Add a default ON flag for DNS fail-fast feature This is an emergency stop button if any unexpected regression is reported. Bug: 309739930 Test: atest resolv_integration_test Change-Id: Ib6aa5f619e34e3da33b82586d3e78b4fed5a666b --- DnsProxyListener.cpp | 6 ++++ Experiments.h | 1 + tests/resolv_private_dns_test.cpp | 64 +++++++++++++++++++++++++++++++++++++++ tests/resolv_test_utils.h | 2 ++ 4 files changed, 73 insertions(+) diff --git a/DnsProxyListener.cpp b/DnsProxyListener.cpp index 10468dcc..7534ed34 100644 --- a/DnsProxyListener.cpp +++ b/DnsProxyListener.cpp @@ -708,6 +708,12 @@ bool isUidNetworkingBlocked(uid_t uid, unsigned netId) { // application's UID. Its DNS packets are not subject to certain network restriction features. if (resolv_is_enforceDnsUid_enabled_network(netId)) return false; + // Feature flag that can disable the feature. + if (!android::net::Experiments::getInstance()->getFlag("fail_fast_on_uid_network_blocking", + 1)) { + return false; + } + return (*ADnsHelper_isUidNetworkingBlocked)(uid, resolv_is_metered_network(netId)) == 1; } diff --git a/Experiments.h b/Experiments.h index 106705b5..db58630c 100644 --- a/Experiments.h +++ b/Experiments.h @@ -62,6 +62,7 @@ class Experiments { "dot_validation_latency_factor", "dot_validation_latency_offset_ms", "dot_xport_unusable_threshold", + "fail_fast_on_uid_network_blocking", "keep_listening_udp", "max_cache_entries", "max_queries_global", diff --git a/tests/resolv_private_dns_test.cpp b/tests/resolv_private_dns_test.cpp index f62495ec..1af5570b 100644 --- a/tests/resolv_private_dns_test.cpp +++ b/tests/resolv_private_dns_test.cpp @@ -603,6 +603,70 @@ TEST_P(TransportParameterizedTest, BlockDnsQuery) { } } +// Verify whether the DNS fail-fast feature can be turned off by flag. +TEST_P(TransportParameterizedTest, BlockDnsQuery_FlaggedOff) { + SKIP_IF_BEFORE_T; + SKIP_IF_DEPENDENT_LIB_DOES_NOT_EXIST(DNS_HELPER); + + constexpr char ptr_name[] = "v4v6.example.com."; + // PTR record for IPv6 address 2001:db8::102:304 + constexpr char ptr_addr_v6[] = + "4.0.3.0.2.0.1.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.8.b.d.0.1.0.0.2.ip6.arpa."; + const DnsRecord r = {ptr_addr_v6, ns_type::ns_t_ptr, ptr_name}; + dns.addMapping(r.host_name, r.type, r.addr); + dot_backend.addMapping(r.host_name, r.type, r.addr); + doh_backend.addMapping(r.host_name, r.type, r.addr); + + ScopedSystemProperties sp1(kFailFastOnUidNetworkBlockingFlag, "0"); + resetNetwork(); + + auto parcel = DnsResponderClient::GetDefaultResolverParamsParcel(); + ASSERT_TRUE(mDnsClient.SetResolversFromParcel(parcel)); + + if (testParamHasDoh()) EXPECT_TRUE(WaitForDohValidationSuccess(test::kDefaultListenAddr)); + if (testParamHasDot()) EXPECT_TRUE(WaitForDotValidationSuccess(test::kDefaultListenAddr)); + + // This waiting time is expected to avoid that the DoH validation event interferes other tests. + if (!testParamHasDoh()) waitForDohValidationFailed(); + + // Have the test independent of the number of sent queries in private DNS validation, because + // the DnsResolver can send either 1 or 2 queries in DoT validation. + if (testParamHasDoh()) { + doh.clearQueries(); + } + if (testParamHasDot()) { + EXPECT_TRUE(dot.waitForQueries(1)); + dot.clearQueries(); + } + dns.clearQueries(); + + for (const bool testDataSaver : {false, true}) { + SCOPED_TRACE(fmt::format("test {}", testDataSaver ? "data saver" : "UID firewall rules")); + if (testDataSaver) { + // Data Saver applies on metered networks only. + parcel.meteredNetwork = true; + ASSERT_TRUE(mDnsClient.SetResolversFromParcel(parcel)); + + // Block network access by enabling data saver. + ScopedSetDataSaverByBPF scopedSetDataSaverByBPF(true); + ScopedChangeUID scopedChangeUID(TEST_UID); + EXPECT_NO_FAILURE(sendQueryAndCheckResult()); + } else { + // Block network access by setting UID firewall rules. + ScopeBlockedUIDRule scopeBlockUidRule(mDnsClient.netdService(), TEST_UID); + EXPECT_NO_FAILURE(sendQueryAndCheckResult()); + } + + if (testParamHasDoh()) { + EXPECT_NO_FAILURE(expectQueries(0 /* dns */, 0 /* dot */, 2 /* doh */)); + dot.clearQueries(); + } else { + EXPECT_NO_FAILURE(expectQueries(0 /* dns */, 2 /* dot */, 0 /* doh */)); + doh.clearQueries(); + } + } +} + class PrivateDnsDohTest : public BasePrivateDnsTest { protected: void SetUp() override { diff --git a/tests/resolv_test_utils.h b/tests/resolv_test_utils.h index 4d6e8640..e7f3a026 100644 --- a/tests/resolv_test_utils.h +++ b/tests/resolv_test_utils.h @@ -184,6 +184,8 @@ const std::string kDotXportUnusableThresholdFlag(kFlagPrefix + "dot_xport_unusab const std::string kDotValidationLatencyFactorFlag(kFlagPrefix + "dot_validation_latency_factor"); const std::string kDotValidationLatencyOffsetMsFlag(kFlagPrefix + "dot_validation_latency_offset_ms"); +const std::string kFailFastOnUidNetworkBlockingFlag(kFlagPrefix + + "fail_fast_on_uid_network_blocking"); const std::string kKeepListeningUdpFlag(kFlagPrefix + "keep_listening_udp"); const std::string kParallelLookupSleepTimeFlag(kFlagPrefix + "parallel_lookup_sleep_time"); const std::string kRetransIntervalFlag(kFlagPrefix + "retransmission_time_interval"); -- cgit v1.2.3 From 51ded2d233527ee100fb6e004a0ea52ede0647a3 Mon Sep 17 00:00:00 2001 From: Henri Chataing Date: Tue, 7 Nov 2023 23:31:15 +0000 Subject: Update fmtlib to 10.1.1 Update test sources Bug: 310046696 Test: mmm packages/modules/DnsResolver Change-Id: Ic60c62d5283791f7c98e3d4aaa3776b9afc550dd --- tests/resolv_integration_test.cpp | 13 +++++++------ tests/resolv_unit_test.cpp | 4 ++-- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/tests/resolv_integration_test.cpp b/tests/resolv_integration_test.cpp index cb04cf1b..b28ccfaf 100644 --- a/tests/resolv_integration_test.cpp +++ b/tests/resolv_integration_test.cpp @@ -5782,8 +5782,9 @@ TEST_F(ResolverTest, RepeatedSetup_KeepChangingPrivateDnsServers) { 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())); + SCOPED_TRACE(fmt::format("serverState:{} testIndex:{} testConfig:[{}]", + static_cast(serverState), testIndex++, + config.asTestName())); auto& tls = (config.tlsServer == addr1) ? tls1 : tls2; if (serverState == UNSUPPORTED && tls.running()) ASSERT_TRUE(tls.stopServer()); @@ -7431,7 +7432,7 @@ TEST_F(ResolverMultinetworkTest, GetAddrInfo_AI_ADDRCONFIG) { ConnectivityType::V4V6, }; for (const auto& type : allTypes) { - SCOPED_TRACE(fmt::format("ConnectivityType: {}", type)); + SCOPED_TRACE(fmt::format("ConnectivityType: {}", static_cast(type))); // Create a network. ScopedPhysicalNetwork network = CreateScopedPhysicalNetwork(type); @@ -7563,7 +7564,7 @@ TEST_F(ResolverMultinetworkTest, DnsWithVpn) { {ConnectivityType::V4V6, {ipv6_addr, ipv4_addr}}, }; for (const auto& [type, result] : testPairs) { - SCOPED_TRACE(fmt::format("ConnectivityType: {}", type)); + SCOPED_TRACE(fmt::format("ConnectivityType: {}", static_cast(type))); // Create a network. ScopedPhysicalNetwork underlyingNetwork = CreateScopedPhysicalNetwork(type, "Underlying"); @@ -7683,7 +7684,7 @@ TEST_F(ResolverMultinetworkTest, PerAppDefaultNetwork) { {ConnectivityType::V4V6, {ipv6_addr, ipv4_addr}}, }; for (const auto& [ipVersion, expectedDnsReply] : testPairs) { - SCOPED_TRACE(fmt::format("ConnectivityType: {}", ipVersion)); + SCOPED_TRACE(fmt::format("ConnectivityType: {}", static_cast(ipVersion))); // Create networks. ScopedPhysicalNetwork sysDefaultNetwork = @@ -7928,7 +7929,7 @@ TEST_F(ResolverMultinetworkTest, UidAllowedNetworks) { {ConnectivityType::V4V6, {ipv6_addr, ipv4_addr}}, }; for (const auto& [ipVersion, expectedDnsReply] : testPairs) { - SCOPED_TRACE(fmt::format("ConnectivityType: {}", ipVersion)); + SCOPED_TRACE(fmt::format("ConnectivityType: {}", static_cast(ipVersion))); // Create networks. ScopedPhysicalNetwork sysDefaultNetwork = diff --git a/tests/resolv_unit_test.cpp b/tests/resolv_unit_test.cpp index d7471b0b..af4cbf4e 100644 --- a/tests/resolv_unit_test.cpp +++ b/tests/resolv_unit_test.cpp @@ -638,7 +638,7 @@ TEST_F(ResolvGetAddrInfoTest, ServerResponseError) { }; for (const auto& config : testConfigs) { - SCOPED_TRACE(fmt::format("rcode: {}", config.rcode)); + SCOPED_TRACE(fmt::format("rcode: {}", static_cast(config.rcode))); test::DNSResponder dns(config.rcode); dns.addMapping(host_name, ns_type::ns_t_a, "1.2.3.4"); @@ -1538,7 +1538,7 @@ TEST_F(GetHostByNameForNetContextTest, ServerResponseError) { }; for (const auto& config : testConfigs) { - SCOPED_TRACE(fmt::format("rcode: {}", config.rcode)); + SCOPED_TRACE(fmt::format("rcode: {}", static_cast(config.rcode))); test::DNSResponder dns(config.rcode); dns.addMapping(host_name, ns_type::ns_t_a, "1.2.3.4"); -- cgit v1.2.3 From 6e2472374c1b6f38da25a72137ab00740e9af295 Mon Sep 17 00:00:00 2001 From: Mike Yu Date: Fri, 10 Nov 2023 02:07:08 +0000 Subject: Test: Delete unused SetResolversForNetwork() Bug: 310112409 Test: mm Change-Id: Ie22f88a3650cecc7518e0fb65354a76b7313c417 --- tests/dns_responder/dns_responder_client_ndk.cpp | 16 ---------------- tests/dns_responder/dns_responder_client_ndk.h | 5 ----- 2 files changed, 21 deletions(-) diff --git a/tests/dns_responder/dns_responder_client_ndk.cpp b/tests/dns_responder/dns_responder_client_ndk.cpp index 52a170a7..b69ce183 100644 --- a/tests/dns_responder/dns_responder_client_ndk.cpp +++ b/tests/dns_responder/dns_responder_client_ndk.cpp @@ -117,22 +117,6 @@ Result DnsResponderClient::getResolverInfo() { return std::move(out); } -bool DnsResponderClient::SetResolversForNetwork(const std::vector& servers, - const std::vector& domains, - std::vector params) { - params.resize(IDnsResolver::RESOLVER_PARAMS_COUNT); - std::array arr; - std::copy_n(params.begin(), arr.size(), arr.begin()); - const auto resolverParams = ResolverParams::Builder() - .setDomains(domains) - .setDnsServers(servers) - .setDotServers({}) - .setParams(arr) - .build(); - const auto rv = mDnsResolvSrv->setResolverConfiguration(resolverParams); - return rv.isOk(); -} - bool DnsResponderClient::SetResolversForNetwork(const std::vector& servers, const std::vector& domains) { const auto resolverParams = ResolverParams::Builder() diff --git a/tests/dns_responder/dns_responder_client_ndk.h b/tests/dns_responder/dns_responder_client_ndk.h index 2adc3c89..0713a7ca 100644 --- a/tests/dns_responder/dns_responder_client_ndk.h +++ b/tests/dns_responder/dns_responder_client_ndk.h @@ -123,11 +123,6 @@ class DnsResponderClient { static void SetupMappings(unsigned num_hosts, const std::vector& domains, std::vector* mappings); - // For dns_benchmark built from tm-mainline-prod. - // TODO: Remove it when possible. - bool SetResolversForNetwork(const std::vector& servers, - const std::vector& domains, std::vector params); - // Sets up DnsResolver with given DNS servers. This is used to set up for private DNS off mode. bool SetResolversForNetwork(const std::vector& servers = {kDefaultServer}, const std::vector& domains = {kDefaultSearchDomain}); -- cgit v1.2.3 From 29dc0977fcae02403724c924fc3347afca7737f9 Mon Sep 17 00:00:00 2001 From: Mike Yu Date: Thu, 9 Nov 2023 08:00:55 +0000 Subject: Test: Use nettestutils to dump binder service The library nettestutils provides dumpService methods to dump binder services. Use them in DnsResolver tests. Bug: 310112409 Test: resolv_integration_test passed Change-Id: Ic9421614a383f7970f0e95145849054a6b27b7e0 --- tests/Android.bp | 10 ++++++---- tests/dnsresolver_binder_test.cpp | 37 +++++-------------------------------- tests/resolv_private_dns_test.cpp | 38 ++++++++------------------------------ 3 files changed, 19 insertions(+), 66 deletions(-) diff --git a/tests/Android.bp b/tests/Android.bp index aa60b1b0..908cc09b 100644 --- a/tests/Android.bp +++ b/tests/Android.bp @@ -192,22 +192,24 @@ cc_test { ], static_libs: [ "dnsresolver_aidl_interface-lateststable-ndk", + "libc++fs", "libconnectivity_native_test_utils", "libcrypto_static", + "libcutils", + "libdoh_frontend_ffi", "libgmock", + "libip_checksum", "libmodules-utils-build", "libnetd_test_dnsresponder_ndk", "libnetd_test_metrics_listener", "libnetd_test_resolv_utils", "libnetdutils", + "libnettestutils", "libssl", - "libc++fs", - "libcutils", + "libutils", "netd_aidl_interface-lateststable-ndk", "netd_event_listener_interface-lateststable-ndk", - "libip_checksum", "resolv_unsolicited_listener", - "libdoh_frontend_ffi", ], // This test talks to the DnsResolver module over a binary protocol on a socket, so keep it as // multilib setting is worth because we might be able to get some coverage for the case where diff --git a/tests/dnsresolver_binder_test.cpp b/tests/dnsresolver_binder_test.cpp index 2d9801e0..85c10720 100644 --- a/tests/dnsresolver_binder_test.cpp +++ b/tests/dnsresolver_binder_test.cpp @@ -37,6 +37,7 @@ #include #include #include +#include #include #include "dns_metrics_listener/base_metrics_listener.h" @@ -64,37 +65,6 @@ using android::netdutils::Stopwatch; // Sync from TEST_NETID in dns_responder_client.cpp as resolv_integration_test.cpp does. constexpr int TEST_NETID = 30; -namespace { - -std::vector dumpService(ndk::SpAIBinder binder) { - unique_fd localFd, remoteFd; - bool success = Pipe(&localFd, &remoteFd); - EXPECT_TRUE(success) << "Failed to open pipe for dumping: " << strerror(errno); - if (!success) return {}; - - // dump() blocks until another thread has consumed all its output. - std::thread dumpThread = std::thread([binder, remoteFd{std::move(remoteFd)}]() { - EXPECT_EQ(STATUS_OK, AIBinder_dump(binder.get(), remoteFd, nullptr, 0)); - }); - - std::string dumpContent; - - EXPECT_TRUE(ReadFdToString(localFd.get(), &dumpContent)) - << "Error during dump: " << strerror(errno); - dumpThread.join(); - - std::stringstream dumpStream(std::move(dumpContent)); - std::vector lines; - std::string line; - while (std::getline(dumpStream, line)) { - lines.push_back(std::move(line)); - } - - return lines; -} - -} // namespace - class DnsResolverBinderTest : public NetNativeTestBase { public: DnsResolverBinderTest() { @@ -119,7 +89,10 @@ class DnsResolverBinderTest : public NetNativeTestBase { // This could happen when the test isn't running as root, or if netd isn't running. assert(nullptr != netdBinder.get()); // Send the service dump request to netd. - std::vector lines = dumpService(netdBinder); + std::vector lines; + const android::status_t ret = + dumpService(netdBinder, /*args=*/nullptr, /*num_args=*/0, lines); + ASSERT_EQ(android::OK, ret) << "Error dumping service: " << android::statusToString(ret); // Basic regexp to match dump output lines. Matches the beginning and end of the line, and // puts the output of the command itself into the first match group. diff --git a/tests/resolv_private_dns_test.cpp b/tests/resolv_private_dns_test.cpp index f62495ec..fb0ddec5 100644 --- a/tests/resolv_private_dns_test.cpp +++ b/tests/resolv_private_dns_test.cpp @@ -29,6 +29,7 @@ #include #include #include +#include #include "doh_frontend.h" #include "tests/dns_responder/dns_responder.h" @@ -61,33 +62,6 @@ constexpr int kDohIdleDefaultTimeoutMs = 55000; namespace { -std::vector dumpService(ndk::SpAIBinder binder) { - unique_fd localFd, remoteFd; - bool success = Pipe(&localFd, &remoteFd); - EXPECT_TRUE(success) << "Failed to open pipe for dumping: " << strerror(errno); - if (!success) return {}; - - // dump() blocks until another thread has consumed all its output. - std::thread dumpThread = std::thread([binder, remoteFd{std::move(remoteFd)}]() { - EXPECT_EQ(STATUS_OK, AIBinder_dump(binder.get(), remoteFd, nullptr, 0)); - }); - - std::string dumpContent; - - EXPECT_TRUE(ReadFdToString(localFd.get(), &dumpContent)) - << "Error during dump: " << strerror(errno); - dumpThread.join(); - - std::stringstream dumpStream(std::move(dumpContent)); - std::vector lines; - std::string line; - while (std::getline(dumpStream, line)) { - lines.push_back(std::move(line)); - } - - return lines; -} - int getAsyncResponse(int fd, int* rcode, uint8_t* buf, int bufLen) { struct pollfd wait_fd[1]; wait_fd[0].fd = fd; @@ -241,9 +215,13 @@ class BaseTest : public NetNativeTestBase { } bool expectLog(const std::string& ipAddrOrNoData, const std::string& port) { - ndk::SpAIBinder resolvBinder = ndk::SpAIBinder(AServiceManager_getService("dnsresolver")); - assert(nullptr != resolvBinder.get()); - std::vector lines = dumpService(resolvBinder); + std::vector lines; + const android::status_t ret = + dumpService(sResolvBinder, /*args=*/nullptr, /*num_args=*/0, lines); + if (ret != android::OK) { + ADD_FAILURE() << "Error dumping service: " << android::statusToString(ret); + return false; + } const std::string expectedLog = port.empty() ? ipAddrOrNoData -- cgit v1.2.3 From c090fb1ed52928187aee99ddc2f7c8159fed3715 Mon Sep 17 00:00:00 2001 From: Mike Yu Date: Wed, 8 Nov 2023 09:55:18 +0000 Subject: Test: Replace manual code with generated toString method Binder already automatically implements toString() for parcelables. The generated toString() returns the same string as what the below hand-made methods return, except that "\n" is not replaced with "\\n". std::string toString(const std::vector& parms) std::string toString(const std::optional& parms) std::string toString(const std::optional& params) std::string toString(const ResolverParamsParcel& parms) This CL replace these hand-made toString() with binder-generated toString(). Bug: 310112409 Test: resolv_integration_test passed Change-Id: I47a95b50346021a998f755fdbc3fbe4b4b2b6116 --- tests/dnsresolver_binder_test.cpp | 52 ++++++--------------------------------- 1 file changed, 7 insertions(+), 45 deletions(-) diff --git a/tests/dnsresolver_binder_test.cpp b/tests/dnsresolver_binder_test.cpp index 85c10720..799e16a6 100644 --- a/tests/dnsresolver_binder_test.cpp +++ b/tests/dnsresolver_binder_test.cpp @@ -55,6 +55,7 @@ using aidl::android::net::ResolverParamsParcel; using aidl::android::net::metrics::INetdEventListener; using aidl::android::net::resolv::aidl::DohParamsParcel; using android::base::ReadFdToString; +using android::base::StringReplace; using android::base::unique_fd; using android::net::ResolverStats; using android::net::metrics::TestOnDnsEvent; @@ -116,7 +117,6 @@ class DnsResolverBinderTest : public NetNativeTestBase { // information. To keep it working on Q/R/..., remove what has been // added for now. TODO(b/266248339) std::string output = match[1].str(); - using android::base::StringReplace; output = StringReplace(output, "(null)", "", /*all=*/true); output = StringReplace(output, "", "", /*all=*/true); output = StringReplace(output, "", "", /*all=*/true); @@ -172,50 +172,12 @@ class DnsResolverBinderTest : public NetNativeTestBase { LogData withoutPacel; }; - std::string toString(const std::vector& parms) { - std::string o; - const size_t size = parms.size(); - for (size_t i = 0; i < size; ++i) { - o.append(fmt::format("ResolverHostsParcel{{ipAddr: {}, hostName: {}}}", parms[i].ipAddr, - parms[i].hostName)); - if (i + 1 < size) o.append(", "); - } - return o; - } - - std::string toString(const std::optional& parms) { - if (!parms.has_value()) return "(null)"; - return fmt::format("ResolverOptionsParcel{{hosts: [{}], tcMode: {}, enforceDnsUid: {}}}", - toString(parms->hosts), parms->tcMode, parms->enforceDnsUid); - } - - std::string toString(const std::optional& params) { - return params.has_value() ? params.value().toString() : "(null)"; - } - - std::string toString(const ResolverParamsParcel& parms) { - return fmt::format( - "ResolverParamsParcel{{netId: {}, sampleValiditySeconds: {}, successThreshold: {}, " - "minSamples: {}, " - "maxSamples: {}, baseTimeoutMsec: {}, retryCount: {}, " - "servers: [{}], domains: [{}], " - "tlsName: {}, tlsServers: [{}], " - "tlsFingerprints: [{}], " - "caCertificate: {}, tlsConnectTimeoutMs: {}, " - "resolverOptions: {}, transportTypes: [{}], meteredNetwork: {}, dohParams: {}}}", - parms.netId, parms.sampleValiditySeconds, parms.successThreshold, parms.minSamples, - parms.maxSamples, parms.baseTimeoutMsec, parms.retryCount, - fmt::join(parms.servers, ", "), fmt::join(parms.domains, ", "), parms.tlsName, - fmt::join(parms.tlsServers, ", "), fmt::join(parms.tlsFingerprints, ", "), - android::base::StringReplace(parms.caCertificate, "\n", "\\n", true), - parms.tlsConnectTimeoutMs, toString(parms.resolverOptions), - fmt::join(parms.transportTypes, ", "), parms.meteredNetwork, - toString(parms.dohParams)); - } - PossibleLogData toSetResolverConfigurationLogData(const ResolverParamsParcel& parms, int returnCode = 0) { - std::string outputWithParcel = "setResolverConfiguration(" + toString(parms) + ")"; + // Replace "\n" with "\\n" in parms.caCertificate. + std::string outputWithParcel = + fmt::format("setResolverConfiguration({})", + StringReplace(parms.toString(), "\n", "\\n", /*all=*/true)); std::string hintRegexWithParcel = fmt::format("setResolverConfiguration.*{}", parms.netId); std::string outputWithoutParcel = "setResolverConfiguration()"; @@ -653,9 +615,9 @@ TEST_F(DnsResolverBinderTest, SetResolverOptions) { options.enforceDnsUid = true; EXPECT_TRUE(mDnsResolver->setResolverOptions(TEST_NETID, options).isOk()); mExpectedLogData.push_back( - {"setResolverOptions(30, " + toString(options) + ")", "setResolverOptions.*30"}); + {"setResolverOptions(30, " + options.toString() + ")", "setResolverOptions.*30"}); EXPECT_EQ(ENONET, mDnsResolver->setResolverOptions(-1, options).getServiceSpecificError()); - mExpectedLogData.push_back({"setResolverOptions(-1, " + toString(options) + + mExpectedLogData.push_back({"setResolverOptions(-1, " + options.toString() + ") -> ServiceSpecificException(64, \"Machine is not on the " "network\")", "setResolverOptions.*-1.*64"}); -- cgit v1.2.3 From eb327b91d310671608c536a12d4d1ede7ffd6f67 Mon Sep 17 00:00:00 2001 From: Ken Chen Date: Thu, 16 Nov 2023 03:21:56 +0800 Subject: Remove redundant log in setResolverConfiguration The log of setResolverConfiguration binder interface is printed twice in both "adb logcat" and "dumpsys netd". Now that all parameters can be printed by gen_log (b/129732660), the legacy logging for setResolverConfiguration is no longer needed. [Without this change] 11-16 02:52:57.877 864 864 I netd : DnsResolverService::setResolverConfiguration(101, [192.168.1.1], [], 1800, 25, 8, 64, 0, 0, "", [192.168.1.1]) -> (0) (0.3ms) 11-16 02:52:57.877 864 864 I netd : setResolverConfiguration(ResolverParamsParcel{netId: 101, sampleValiditySeconds: 1800, successThreshold: 25, minSamples: 8, maxSamples: 64, baseTimeoutMsec: 0, retryCount: 0, servers: [192.168.1.1], domains: [], tlsName: , tlsServers: [192.168.1.1], tlsFingerprints: [], caCertificate: , tlsConnectTimeoutMs: 0, resolverOptions: (null), transportTypes: [1], meteredNetwork: false, dohParams: (null)}) <0.39ms> [With this change] 11-16 03:23:53.098 1011 1149 I netd : setResolverConfiguration(ResolverParamsParcel{netId: 100, sampleValiditySeconds: 1800, successThreshold: 25, minSamples: 8, maxSamples: 64, baseTimeoutMsec: 0, retryCount: 0, servers: [192.168.1.1], domains: [], tlsName: , tlsServers: [192.168.1.1], tlsFingerprints: [], caCertificate: , tlsConnectTimeoutMs: 0, resolverOptions: (null), transportTypes: [1], meteredNetwork: false, dohParams: (null)}) <0.40ms> Test: atest Test: adb logcat; adb dumpsys netd Change-Id: Ia21b999e37d13e86176e153d9cad5ac6f3256135 --- DnsResolver.cpp | 1 - DnsResolver.h | 2 -- DnsResolverService.cpp | 12 ------------ 3 files changed, 15 deletions(-) diff --git a/DnsResolver.cpp b/DnsResolver.cpp index 5abfaea6..372252d6 100644 --- a/DnsResolver.cpp +++ b/DnsResolver.cpp @@ -68,7 +68,6 @@ bool verifyCallbacks() { DnsResolver* gDnsResolv = nullptr; ResolverNetdCallbacks gResNetdCallbacks; -netdutils::Log gDnsResolverLog("dnsResolver"); uint64_t gApiLevel = 0; DnsResolver* DnsResolver::getInstance() { diff --git a/DnsResolver.h b/DnsResolver.h index 9c2f3d8c..a3819f15 100644 --- a/DnsResolver.h +++ b/DnsResolver.h @@ -21,7 +21,6 @@ #include "DnsQueryLog.h" #include "ResolverController.h" #include "netd_resolv/resolv.h" -#include "netdutils/Log.h" namespace android { namespace net { @@ -48,7 +47,6 @@ class DnsResolver { extern DnsResolver* gDnsResolv; extern ResolverNetdCallbacks gResNetdCallbacks; -extern netdutils::Log gDnsResolverLog; extern uint64_t gApiLevel; } // namespace net diff --git a/DnsResolverService.cpp b/DnsResolverService.cpp index ee129514..9b399b97 100644 --- a/DnsResolverService.cpp +++ b/DnsResolverService.cpp @@ -199,19 +199,7 @@ binder_status_t DnsResolverService::dump(int fd, const char** args, uint32_t num return ::ndk::ScopedAStatus(AStatus_fromExceptionCodeWithMessage(EX_SECURITY, err.c_str())); } - // TODO: Remove this log after AIDL gen_log supporting more types, b/129732660 - auto entry = - gDnsResolverLog.newEntry() - .prettyFunction(__PRETTY_FUNCTION__) - .args(resolverParams.netId, resolverParams.servers, resolverParams.domains, - resolverParams.sampleValiditySeconds, resolverParams.successThreshold, - resolverParams.minSamples, resolverParams.maxSamples, - resolverParams.baseTimeoutMsec, resolverParams.retryCount, - resolverParams.tlsName, resolverParams.tlsServers); - int res = gDnsResolv->resolverCtrl.setResolverConfiguration(resolverParams); - gResNetdCallbacks.log(entry.returns(res).withAutomaticDuration().toString().c_str()); - return statusFromErrcode(res); } -- cgit v1.2.3 From 602b5933b2b519c9c8ecee81e47960af5e1c0859 Mon Sep 17 00:00:00 2001 From: Ken Chen Date: Thu, 16 Nov 2023 04:45:05 +0800 Subject: Delete obsolete comment The comment is obsolete. Test: m Change-Id: I09268490cd802891296204ea86a48b02f5d2052b --- DnsResolverService.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/DnsResolverService.cpp b/DnsResolverService.cpp index ee129514..a28daa16 100644 --- a/DnsResolverService.cpp +++ b/DnsResolverService.cpp @@ -90,8 +90,6 @@ binder_status_t DnsResolverService::start() { ABinderProcess_startThreadPool(); - // TODO: register log callback if binder NDK backend support it. b/126501406 - return STATUS_OK; } -- cgit v1.2.3 From 0b2bd5927b983ae21cb68e8f7ececb06e0bb7543 Mon Sep 17 00:00:00 2001 From: Frank Li Date: Thu, 16 Nov 2023 11:24:32 +0800 Subject: Adding UID to NetwrokDnsEventReported atom Ignore-AOSP-First: Change in topic has merge conflicts, will cherry-pick immediately afterwards Bug: 309575211 Test: atest resolv_unit_test Change-Id: Ied14657053cbef280311badd23cecb3951ce6835 --- DnsProxyListener.cpp | 11 ++++++----- stats.proto | 3 +++ 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/DnsProxyListener.cpp b/DnsProxyListener.cpp index 7534ed34..e70ddb40 100644 --- a/DnsProxyListener.cpp +++ b/DnsProxyListener.cpp @@ -336,11 +336,12 @@ void reportDnsEvent(int eventType, const android_net_context& netContext, int la stats::BytesField dnsQueryBytesField{dnsQueryStats.c_str(), dnsQueryStats.size()}; event.set_return_code(static_cast(returnCode)); event.set_network_type(resolv_get_network_types_for_net(netContext.dns_netid)); - android::net::stats::stats_write(android::net::stats::NETWORK_DNS_EVENT_REPORTED, - event.event_type(), event.return_code(), - event.latency_micros(), event.hints_ai_flags(), - event.res_nsend_flags(), event.network_type(), - event.private_dns_modes(), dnsQueryBytesField, rate); + event.set_uid(netContext.uid); + android::net::stats::stats_write( + android::net::stats::NETWORK_DNS_EVENT_REPORTED, event.event_type(), + event.return_code(), event.latency_micros(), event.hints_ai_flags(), + event.res_nsend_flags(), event.network_type(), event.private_dns_modes(), + dnsQueryBytesField, rate, event.uid()); } maybeLogQuery(eventType, netContext, event, query_name, ip_addrs); diff --git a/stats.proto b/stats.proto index 4052ec7d..1356b7fc 100644 --- a/stats.proto +++ b/stats.proto @@ -412,6 +412,9 @@ message NetworkDnsEventReported { // The sample rate of DNS stats (to statsd) is 1/sampling_rate_denom. optional int32 sampling_rate_denom = 9; + + // UID sends the DNS query. + optional int32 uid = 10; } enum HandshakeResult { -- cgit v1.2.3 From 18f4d9eb091b6d7632ec3f58e40cb7e95fe6ba79 Mon Sep 17 00:00:00 2001 From: Mike Yu Date: Mon, 20 Nov 2023 06:04:31 +0000 Subject: Test: Replace wait_until() with wait_for() Refactor waitForPrivateDnsValidation() to use wait_for(). The predicate is used to check whether the waiting should be continued or not. Since waitForNat64Prefix() and popDnsEvent() will be changed to use condition variable, writing this way would make it easier to change them and avoid unnecessary awakenings. Bug: 310108475 Test: resolv_integration_test passed Change-Id: I2282450a6f3626cbb749030154b581554df7b174 --- tests/dns_metrics_listener/dns_metrics_listener.cpp | 14 +++----------- tests/unsolicited_listener/unsolicited_event_listener.cpp | 14 +++----------- 2 files changed, 6 insertions(+), 22 deletions(-) diff --git a/tests/dns_metrics_listener/dns_metrics_listener.cpp b/tests/dns_metrics_listener/dns_metrics_listener.cpp index 1715118d..e6cdef88 100644 --- a/tests/dns_metrics_listener/dns_metrics_listener.cpp +++ b/tests/dns_metrics_listener/dns_metrics_listener.cpp @@ -95,18 +95,10 @@ bool DnsMetricsListener::waitForNat64Prefix(ExpectNat64PrefixStatus status, mill bool DnsMetricsListener::waitForPrivateDnsValidation(const std::string& serverAddr, const bool validated) { - const auto now = std::chrono::steady_clock::now(); - std::unique_lock lock(mMutex); - ScopedLockAssertion assume_lock(mMutex); - - // onPrivateDnsValidationEvent() might already be invoked. Search for the record first. - do { - if (findAndRemoveValidationRecord({mNetId, serverAddr}, validated)) return true; - } while (mCv.wait_until(lock, now + kEventTimeoutMs) != std::cv_status::timeout); - - // Timeout. - return false; + return mCv.wait_for(lock, kEventTimeoutMs, [&]() REQUIRES(mMutex) { + return findAndRemoveValidationRecord({mNetId, serverAddr}, validated); + }); } bool DnsMetricsListener::findAndRemoveValidationRecord(const ServerKey& key, const bool value) { diff --git a/tests/unsolicited_listener/unsolicited_event_listener.cpp b/tests/unsolicited_listener/unsolicited_event_listener.cpp index 20edae13..367df972 100644 --- a/tests/unsolicited_listener/unsolicited_event_listener.cpp +++ b/tests/unsolicited_listener/unsolicited_event_listener.cpp @@ -68,18 +68,10 @@ constexpr milliseconds kRetryIntervalMs{20}; bool UnsolicitedEventListener::waitForPrivateDnsValidation(const std::string& serverAddr, int validation, int protocol) { - const auto now = std::chrono::steady_clock::now(); - std::unique_lock lock(mMutex); - ScopedLockAssertion assume_lock(mMutex); - - // onPrivateDnsValidationEvent() might already be invoked. Search for the record first. - do { - if (findAndRemoveValidationRecord({mNetId, serverAddr, protocol}, validation)) return true; - } while (mCv.wait_until(lock, now + kEventTimeoutMs) != std::cv_status::timeout); - - // Timeout. - return false; + return mCv.wait_for(lock, kEventTimeoutMs, [&]() REQUIRES(mMutex) { + return findAndRemoveValidationRecord({mNetId, serverAddr, protocol}, validation); + }); } bool UnsolicitedEventListener::findAndRemoveValidationRecord(const ServerKey& key, int value) { -- cgit v1.2.3 From 4f4f57a842138b76534743789b7914303f0d63b9 Mon Sep 17 00:00:00 2001 From: Mike Yu Date: Fri, 10 Nov 2023 03:08:14 +0000 Subject: Test: Use condition_variable to wait for onNat64PrefixEvent This CL improves the test running time. PrefixDiscoveryBypassTls: 360ms -> 307ms SetAndClearNat64Prefix: 232ms -> 200ms Bug: 310108475 Test: resolv_integration_test passed Change-Id: Ia427e45d4045ad2ce17ed52dd31250adc5ba6ce5 --- .../dns_metrics_listener/dns_metrics_listener.cpp | 33 ++++++++++------------ tests/dns_metrics_listener/dns_metrics_listener.h | 9 ++---- .../unsolicited_event_listener.cpp | 31 +++++++++++--------- 3 files changed, 34 insertions(+), 39 deletions(-) diff --git a/tests/dns_metrics_listener/dns_metrics_listener.cpp b/tests/dns_metrics_listener/dns_metrics_listener.cpp index e6cdef88..2ce99966 100644 --- a/tests/dns_metrics_listener/dns_metrics_listener.cpp +++ b/tests/dns_metrics_listener/dns_metrics_listener.cpp @@ -21,9 +21,7 @@ #include #include -namespace android { -namespace net { -namespace metrics { +namespace android::net::metrics { using android::base::ScopedLockAssertion; using std::chrono::milliseconds; @@ -49,6 +47,7 @@ std::ostream& operator<<(std::ostream& os, const DnsMetricsListener::DnsEvent& d std::lock_guard lock(mMutex); mUnexpectedNat64PrefixUpdates++; if (netId == mNetId) mNat64Prefix = added ? prefixString : ""; + mCv.notify_all(); return ::ndk::ScopedAStatus::ok(); } @@ -60,7 +59,7 @@ std::ostream& operator<<(std::ostream& os, const DnsMetricsListener::DnsEvent& d // keep updating the server to have latest validation status. mValidationRecords.insert_or_assign({netId, ipAddress}, validated); } - mCv.notify_one(); + mCv.notify_all(); return ::ndk::ScopedAStatus::ok(); } @@ -78,18 +77,18 @@ std::ostream& operator<<(std::ostream& os, const DnsMetricsListener::DnsEvent& d } bool DnsMetricsListener::waitForNat64Prefix(ExpectNat64PrefixStatus status, milliseconds timeout) { - android::base::Timer t; - while (t.duration() < timeout) { - { - std::lock_guard lock(mMutex); - if ((status == EXPECT_FOUND && !mNat64Prefix.empty()) || - (status == EXPECT_NOT_FOUND && mNat64Prefix.empty())) { - mUnexpectedNat64PrefixUpdates--; - return true; - } - } - std::this_thread::sleep_for(kRetryIntervalMs); + std::unique_lock lock(mMutex); + ScopedLockAssertion assume_lock(mMutex); + + if (mCv.wait_for(lock, timeout, [&]() REQUIRES(mMutex) { + return (status == EXPECT_FOUND && !mNat64Prefix.empty()) || + (status == EXPECT_NOT_FOUND && mNat64Prefix.empty()); + })) { + mUnexpectedNat64PrefixUpdates--; + return true; } + + // Timeout. return false; } @@ -129,6 +128,4 @@ std::optional DnsMetricsListener::popDnsEvent() { return ret; } -} // namespace metrics -} // namespace net -} // namespace android +} // namespace android::net::metrics diff --git a/tests/dns_metrics_listener/dns_metrics_listener.h b/tests/dns_metrics_listener/dns_metrics_listener.h index ff714664..e34662b2 100644 --- a/tests/dns_metrics_listener/dns_metrics_listener.h +++ b/tests/dns_metrics_listener/dns_metrics_listener.h @@ -30,11 +30,8 @@ enum ExpectNat64PrefixStatus : bool { EXPECT_NOT_FOUND, }; -namespace android { -namespace net { -namespace metrics { +namespace android::net::metrics { -// TODO: Perhaps use condition variable but continually polling. // TODO: Perhaps create a queue to monitor the event changes. That improves the unit test which can // verify the event count, the event change order, and so on. class DnsMetricsListener : public BaseMetricsListener { @@ -131,6 +128,4 @@ class DnsMetricsListener : public BaseMetricsListener { std::condition_variable mCv; }; -} // namespace metrics -} // namespace net -} // namespace android +} // namespace android::net::metrics diff --git a/tests/unsolicited_listener/unsolicited_event_listener.cpp b/tests/unsolicited_listener/unsolicited_event_listener.cpp index 367df972..bae42754 100644 --- a/tests/unsolicited_listener/unsolicited_event_listener.cpp +++ b/tests/unsolicited_listener/unsolicited_event_listener.cpp @@ -51,6 +51,7 @@ constexpr milliseconds kRetryIntervalMs{20}; ? event.prefixAddress : ""; } + mCv.notify_all(); return ::ndk::ScopedAStatus::ok(); } @@ -62,7 +63,7 @@ constexpr milliseconds kRetryIntervalMs{20}; mValidationRecords.insert_or_assign({event.netId, event.ipAddress, event.protocol}, event.validation); } - mCv.notify_one(); + mCv.notify_all(); return ::ndk::ScopedAStatus::ok(); } @@ -84,20 +85,22 @@ bool UnsolicitedEventListener::findAndRemoveValidationRecord(const ServerKey& ke } bool UnsolicitedEventListener::waitForNat64Prefix(int operation, const milliseconds& timeout) { - android::base::Timer t; - while (t.duration() < timeout) { - { - std::lock_guard lock(mMutex); - if ((operation == IDnsResolverUnsolicitedEventListener::PREFIX_OPERATION_ADDED && - !mNat64PrefixAddress.empty()) || - (operation == IDnsResolverUnsolicitedEventListener::PREFIX_OPERATION_REMOVED && - mNat64PrefixAddress.empty())) { - mUnexpectedNat64PrefixUpdates--; - return true; - } - } - std::this_thread::sleep_for(kRetryIntervalMs); + const auto now = std::chrono::steady_clock::now(); + + std::unique_lock lock(mMutex); + ScopedLockAssertion assume_lock(mMutex); + + if (mCv.wait_for(lock, timeout, [&]() REQUIRES(mMutex) { + return (operation == IDnsResolverUnsolicitedEventListener::PREFIX_OPERATION_ADDED && + !mNat64PrefixAddress.empty()) || + (operation == IDnsResolverUnsolicitedEventListener::PREFIX_OPERATION_REMOVED && + mNat64PrefixAddress.empty()); + })) { + mUnexpectedNat64PrefixUpdates--; + return true; } + + // Timeout. return false; } -- cgit v1.2.3 From f47c007446cbb459bdab1e6f1cb778d2a41b0dca Mon Sep 17 00:00:00 2001 From: Mike Yu Date: Fri, 10 Nov 2023 03:36:48 +0000 Subject: Test: Use condition_variable to wait for onDnsEvent This CL improves the test running time. ResolverTest.BrokenEdns: 8197ms -> 7667ms The timeout for onDnsEvent is changed from 1s to 5s. Bug: 310108475 Test: resolv_integration_test passed Change-Id: Ic881dcd58dcfb6b4fd66cbb91db10c6660325808 --- tests/dns_metrics_listener/dns_metrics_listener.cpp | 20 +++++++------------- .../unsolicited_event_listener.cpp | 19 +++++++------------ 2 files changed, 14 insertions(+), 25 deletions(-) diff --git a/tests/dns_metrics_listener/dns_metrics_listener.cpp b/tests/dns_metrics_listener/dns_metrics_listener.cpp index 2ce99966..c1d4883d 100644 --- a/tests/dns_metrics_listener/dns_metrics_listener.cpp +++ b/tests/dns_metrics_listener/dns_metrics_listener.cpp @@ -18,7 +18,6 @@ #include -#include #include namespace android::net::metrics { @@ -26,7 +25,6 @@ namespace android::net::metrics { using android::base::ScopedLockAssertion; using std::chrono::milliseconds; -constexpr milliseconds kRetryIntervalMs{20}; constexpr milliseconds kEventTimeoutMs{5000}; bool DnsMetricsListener::DnsEvent::operator==(const DnsMetricsListener::DnsEvent& o) const { @@ -73,6 +71,7 @@ std::ostream& operator<<(std::ostream& os, const DnsMetricsListener::DnsEvent& d mDnsEventRecords.push( {netId, eventType, returnCode, hostname, ipAddresses, ipAddressesCount}); } + mCv.notify_all(); return ::ndk::ScopedAStatus::ok(); } @@ -110,18 +109,13 @@ bool DnsMetricsListener::findAndRemoveValidationRecord(const ServerKey& key, con } std::optional DnsMetricsListener::popDnsEvent() { - // Wait until the queue is not empty or timeout. - android::base::Timer t; - while (t.duration() < milliseconds{1000}) { - { - std::lock_guard lock(mMutex); - if (!mDnsEventRecords.empty()) break; - } - std::this_thread::sleep_for(kRetryIntervalMs); - } + std::unique_lock lock(mMutex); + ScopedLockAssertion assume_lock(mMutex); - std::lock_guard lock(mMutex); - if (mDnsEventRecords.empty()) return std::nullopt; + if (!mCv.wait_for(lock, kEventTimeoutMs, + [&]() REQUIRES(mMutex) { return !mDnsEventRecords.empty(); })) { + return std::nullopt; + } auto ret = mDnsEventRecords.front(); mDnsEventRecords.pop(); diff --git a/tests/unsolicited_listener/unsolicited_event_listener.cpp b/tests/unsolicited_listener/unsolicited_event_listener.cpp index bae42754..b337496f 100644 --- a/tests/unsolicited_listener/unsolicited_event_listener.cpp +++ b/tests/unsolicited_listener/unsolicited_event_listener.cpp @@ -33,11 +33,11 @@ using android::base::ScopedLockAssertion; using std::chrono::milliseconds; constexpr milliseconds kEventTimeoutMs{5000}; -constexpr milliseconds kRetryIntervalMs{20}; ::ndk::ScopedAStatus UnsolicitedEventListener::onDnsHealthEvent(const DnsHealthEventParcel& event) { std::lock_guard lock(mMutex); if (event.netId == mNetId) mDnsHealthResultRecords.push(event.healthResult); + mCv.notify_all(); return ::ndk::ScopedAStatus::ok(); } @@ -105,18 +105,13 @@ bool UnsolicitedEventListener::waitForNat64Prefix(int operation, const milliseco } Result UnsolicitedEventListener::popDnsHealthResult() { - // Wait until the queue is not empty or timeout. - android::base::Timer t; - while (t.duration() < milliseconds{1000}) { - { - std::lock_guard lock(mMutex); - if (!mDnsHealthResultRecords.empty()) break; - } - std::this_thread::sleep_for(kRetryIntervalMs); - } + std::unique_lock lock(mMutex); + ScopedLockAssertion assume_lock(mMutex); - std::lock_guard lock(mMutex); - if (mDnsHealthResultRecords.empty()) return Error() << "Dns health result record is empty"; + if (!mCv.wait_for(lock, kEventTimeoutMs, + [&]() REQUIRES(mMutex) { return !mDnsHealthResultRecords.empty(); })) { + return Error() << "Dns health result record is empty"; + } auto ret = mDnsHealthResultRecords.front(); mDnsHealthResultRecords.pop(); -- cgit v1.2.3 From b9058840665c6d525ce4c04652d7c27f1126561b Mon Sep 17 00:00:00 2001 From: Frank Li Date: Thu, 16 Nov 2023 11:24:32 +0800 Subject: Adding UID to NetwrokDnsEventReported atom (Cherry-pick from ag/25395794) Bug: 309575211 Test: atest resolv_unit_test Change-Id: Ied14657053cbef280311badd23cecb3951ce6835 Merged-In: Ied14657053cbef280311badd23cecb3951ce6835 --- DnsProxyListener.cpp | 11 ++++++----- stats.proto | 3 +++ 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/DnsProxyListener.cpp b/DnsProxyListener.cpp index 7534ed34..e70ddb40 100644 --- a/DnsProxyListener.cpp +++ b/DnsProxyListener.cpp @@ -336,11 +336,12 @@ void reportDnsEvent(int eventType, const android_net_context& netContext, int la stats::BytesField dnsQueryBytesField{dnsQueryStats.c_str(), dnsQueryStats.size()}; event.set_return_code(static_cast(returnCode)); event.set_network_type(resolv_get_network_types_for_net(netContext.dns_netid)); - android::net::stats::stats_write(android::net::stats::NETWORK_DNS_EVENT_REPORTED, - event.event_type(), event.return_code(), - event.latency_micros(), event.hints_ai_flags(), - event.res_nsend_flags(), event.network_type(), - event.private_dns_modes(), dnsQueryBytesField, rate); + event.set_uid(netContext.uid); + android::net::stats::stats_write( + android::net::stats::NETWORK_DNS_EVENT_REPORTED, event.event_type(), + event.return_code(), event.latency_micros(), event.hints_ai_flags(), + event.res_nsend_flags(), event.network_type(), event.private_dns_modes(), + dnsQueryBytesField, rate, event.uid()); } maybeLogQuery(eventType, netContext, event, query_name, ip_addrs); diff --git a/stats.proto b/stats.proto index 4052ec7d..1356b7fc 100644 --- a/stats.proto +++ b/stats.proto @@ -412,6 +412,9 @@ message NetworkDnsEventReported { // The sample rate of DNS stats (to statsd) is 1/sampling_rate_denom. optional int32 sampling_rate_denom = 9; + + // UID sends the DNS query. + optional int32 uid = 10; } enum HandshakeResult { -- cgit v1.2.3 From a064114d5df052f04b5c00b8cfad23d0984382bf Mon Sep 17 00:00:00 2001 From: Mike Yu Date: Mon, 20 Nov 2023 11:13:26 +0000 Subject: Test: Deflake ResolverTest#QueryTlsServerTimeout Occasionally, the test starts the second DNS query while DnsResolver is still cleaning up the DoT socket created for the first DNS query. In that case, the second DNS query immediately gets an error from DoT engine, and then the test fails. Actually, the second DNS query can be removed from the test, because the value of dot_query_timeout_ms is verified by the first DNS query. Therefore, this CL removes the second DNS query. In addition, to speed up the test, DoT server response time is reduced from 5s to 2s. Bug: 310556479 Test: run QueryTlsServerTimeout 100 times Change-Id: I0cea60667f802861bf31fd4ecb0832b278930a93 --- tests/resolv_integration_test.cpp | 113 +++++++++++++++++++------------------- 1 file changed, 56 insertions(+), 57 deletions(-) diff --git a/tests/resolv_integration_test.cpp b/tests/resolv_integration_test.cpp index b28ccfaf..ef2bf1e5 100644 --- a/tests/resolv_integration_test.cpp +++ b/tests/resolv_integration_test.cpp @@ -4827,77 +4827,76 @@ TEST_F(ResolverTest, ConnectTlsServerTimeout_ConcurrentQueries) { } } +// Tests that the DoT query timeout is configurable via the feature flag "dot_query_timeout_ms". +// The test DoT server is configured to postpone DNS queries for DOT_SERVER_UNRESPONSIVE_TIME_MS +// (2s). If the feature flag is set to a positive value smaller than +// DOT_SERVER_UNRESPONSIVE_TIME_MS, DoT queries should timeout. TEST_F(ResolverTest, QueryTlsServerTimeout) { - constexpr uint32_t cacheFlag = ANDROID_RESOLV_NO_CACHE_LOOKUP; - constexpr int INFINITE_QUERY_TIMEOUT = -1; - constexpr int DOT_SERVER_UNRESPONSIVE_TIME_MS = 5000; + constexpr int DOT_SERVER_UNRESPONSIVE_TIME_MS = 2000; + constexpr int TIMING_TOLERANCE_MS = 200; constexpr char hostname1[] = "query1.example.com."; - constexpr char hostname2[] = "query2.example.com."; const std::vector records = { {hostname1, ns_type::ns_t_a, "1.2.3.4"}, - {hostname2, ns_type::ns_t_a, "1.2.3.5"}, }; - for (const int queryTimeoutMs : {INFINITE_QUERY_TIMEOUT, 1000}) { - for (const std::string_view dnsMode : {"OPPORTUNISTIC", "STRICT"}) { - SCOPED_TRACE(fmt::format("testConfig: [{}] [{}]", dnsMode, queryTimeoutMs)); - - const std::string addr = getUniqueIPv4Address(); - test::DNSResponder dns(addr); - StartDns(dns, records); - test::DnsTlsFrontend tls(addr, "853", addr, "53"); - ASSERT_TRUE(tls.startServer()); + static const struct TestConfig { + std::string dnsMode; + int queryTimeoutMs; + int expectResultTimedOut; + int expectedTimeTakenMs; + } testConfigs[] = { + // clang-format off + {"OPPORTUNISTIC", -1, false, DOT_SERVER_UNRESPONSIVE_TIME_MS}, + {"OPPORTUNISTIC", 1000, false, 1000}, + {"STRICT", -1, false, DOT_SERVER_UNRESPONSIVE_TIME_MS}, + // `expectResultTimedOut` is true in the following testcase because in strict mode + // DnsResolver doesn't try Do53 servers after the DoT query is timed out. + {"STRICT", 1000, true, 1000}, + // clang-format on + }; + for (const auto& config : testConfigs) { + SCOPED_TRACE(fmt::format("testConfig: [{}] [{}]", config.dnsMode, config.queryTimeoutMs)); - ScopedSystemProperties sp(kDotQueryTimeoutMsFlag, std::to_string(queryTimeoutMs)); + const std::string addr = getUniqueIPv4Address(); + test::DNSResponder dns(addr); + StartDns(dns, records); + test::DnsTlsFrontend tls(addr, "853", addr, "53"); + ASSERT_TRUE(tls.startServer()); - // Don't skip unusable DoT servers and disable revalidation for this test. - ScopedSystemProperties sp2(kDotXportUnusableThresholdFlag, "-1"); - ScopedSystemProperties sp3(kDotRevalidationThresholdFlag, "-1"); - resetNetwork(); + ScopedSystemProperties sp(kDotQueryTimeoutMsFlag, std::to_string(config.queryTimeoutMs)); - auto parcel = DnsResponderClient::GetDefaultResolverParamsParcel(); - parcel.servers = {addr}; - parcel.tlsServers = {addr}; - if (dnsMode == "STRICT") parcel.tlsName = kDefaultPrivateDnsHostName; + // Don't skip unusable DoT servers and disable revalidation for this test. + ScopedSystemProperties sp2(kDotXportUnusableThresholdFlag, "-1"); + ScopedSystemProperties sp3(kDotRevalidationThresholdFlag, "-1"); + resetNetwork(); - ASSERT_TRUE(mDnsClient.SetResolversFromParcel(parcel)); - EXPECT_TRUE(WaitForPrivateDnsValidation(tls.listen_address(), true)); - EXPECT_TRUE(tls.waitForQueries(1)); - tls.clearQueries(); + auto parcel = DnsResponderClient::GetDefaultResolverParamsParcel(); + parcel.servers = {addr}; + parcel.tlsServers = {addr}; + if (config.dnsMode == "STRICT") parcel.tlsName = kDefaultPrivateDnsHostName; - // Set the DoT server to be unresponsive to DNS queries until either it receives - // 2 queries or 5s later. - tls.setDelayQueries(2); - tls.setDelayQueriesTimeout(DOT_SERVER_UNRESPONSIVE_TIME_MS); + ASSERT_TRUE(mDnsClient.SetResolversFromParcel(parcel)); + EXPECT_TRUE(WaitForPrivateDnsValidation(tls.listen_address(), true)); + EXPECT_TRUE(tls.waitForQueries(1)); + tls.clearQueries(); - // First query. - Stopwatch s; - int fd = resNetworkQuery(TEST_NETID, hostname1, ns_c_in, ns_t_a, cacheFlag); - if (dnsMode == "STRICT" && queryTimeoutMs != INFINITE_QUERY_TIMEOUT) { - expectAnswersNotValid(fd, -ETIMEDOUT); - } else { - expectAnswersValid(fd, AF_INET, "1.2.3.4"); - } + // Set the DoT server to be unresponsive to DNS queries for + // `DOT_SERVER_UNRESPONSIVE_TIME_MS` ms. + tls.setDelayQueries(999); + tls.setDelayQueriesTimeout(DOT_SERVER_UNRESPONSIVE_TIME_MS); - // Besides checking the result of the query, check how much time the - // resolver processed the query. - int timeTakenMs = s.getTimeAndResetUs() / 1000; - const int expectedTimeTakenMs = (queryTimeoutMs == INFINITE_QUERY_TIMEOUT) - ? DOT_SERVER_UNRESPONSIVE_TIME_MS - : queryTimeoutMs; - EXPECT_GE(timeTakenMs, expectedTimeTakenMs); - EXPECT_LE(timeTakenMs, expectedTimeTakenMs + 1000); - - // Second query. - tls.setDelayQueries(1); - fd = resNetworkQuery(TEST_NETID, hostname2, ns_c_in, ns_t_a, cacheFlag); - expectAnswersValid(fd, AF_INET, "1.2.3.5"); - - // Also check how much time the resolver processed the query. - timeTakenMs = s.timeTakenUs() / 1000; - EXPECT_LE(timeTakenMs, 500); - EXPECT_TRUE(tls.waitForQueries(2)); + // Send a DNS query, and then check the result and the response time. + Stopwatch s; + int fd = resNetworkQuery(TEST_NETID, hostname1, ns_c_in, ns_t_a, + ANDROID_RESOLV_NO_CACHE_LOOKUP); + if (config.expectResultTimedOut) { + expectAnswersNotValid(fd, -ETIMEDOUT); + } else { + expectAnswersValid(fd, AF_INET, "1.2.3.4"); } + const int timeTakenMs = s.getTimeAndResetUs() / 1000; + EXPECT_NEAR(config.expectedTimeTakenMs, timeTakenMs, TIMING_TOLERANCE_MS); + EXPECT_TRUE(tls.waitForQueries(1)); } } -- cgit v1.2.3 From 06c961955b4eb0ae56a2a1959477998f7bde668a Mon Sep 17 00:00:00 2001 From: Ken Chen Date: Fri, 24 Nov 2023 00:30:52 +0800 Subject: Find out whether the system is 32-bit or 64-bit by reading "ro.product.cpu.abi". The original android::bpf::isUserspace64bit() cannot be used in test because MTS will run 32-bit tests on 64-bit platforms. The android::bpf::isUserspace64bit() checks whether the caller (test) itself is compiled to 32 or 64 bit, not the platform. It end up with that the 32 bit test checking the wrong path of libcom.android.tethering.dns_helper.so on a 64 bit platform. Bug: 310105002 Bug: 309369011 Bug: 309739930 Test: atest Test: m mts && mts-tradefed run mts -m resolv_integration_test Change-Id: I4b243fbb8b197abe44a4890252ed8f9adfcc5afb --- tests/resolv_test_utils.cpp | 4 ++++ tests/resolv_test_utils.h | 7 ++++--- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/tests/resolv_test_utils.cpp b/tests/resolv_test_utils.cpp index 17c6c1db..4b09b213 100644 --- a/tests/resolv_test_utils.cpp +++ b/tests/resolv_test_utils.cpp @@ -229,3 +229,7 @@ void RemoveMdnsRoute() { }; EXPECT_EQ(0, ForkAndRun(args_v6)); } + +bool is64bitAbi() { + return android::base::GetProperty("ro.product.cpu.abi", "").find("64") != std::string::npos; +} diff --git a/tests/resolv_test_utils.h b/tests/resolv_test_utils.h index e7f3a026..e3f744ce 100644 --- a/tests/resolv_test_utils.h +++ b/tests/resolv_test_utils.h @@ -439,10 +439,11 @@ void RemoveMdnsRoute(); } \ } while (0) +bool is64bitAbi(); + static const std::string DNS_HELPER = - android::bpf::isUserspace64bit() - ? "/apex/com.android.tethering/lib64/libcom.android.tethering.dns_helper.so" - : "/apex/com.android.tethering/lib/libcom.android.tethering.dns_helper.so"; + is64bitAbi() ? "/apex/com.android.tethering/lib64/libcom.android.tethering.dns_helper.so" + : "/apex/com.android.tethering/lib/libcom.android.tethering.dns_helper.so"; #define SKIP_IF_DEPENDENT_LIB_DOES_NOT_EXIST(libPath) \ do { \ -- cgit v1.2.3 From 21072606e4dd76e04a56b59770c0deb21bbde03c Mon Sep 17 00:00:00 2001 From: Frank Li Date: Tue, 28 Nov 2023 23:30:24 +0800 Subject: Added the test for UID field of NetwrokDnsEventReported atom Bug: 309575211 Test: atest resolv_unit_test, resolv_stats_test_utils_test statsd_testdrive Metrics dump: event_type: EVENT_GETADDRINFO, return_code: RC_EAI_NO_ERROR, latency_micros: 105375, hints_ai_flags: 1024, res_nsend_flags: -1, network_type: NT_CELLULAR, private_dns_modes: PDM_OPPORTUNISTIC, dns_query_events: { dns_query_event: [{ rcode: NS_R_NO_ERROR, type: NS_T_AAAA, cache_hit: CS_NOTFOUND, ip_version: IV_IPV6, protocol: PROTO_UDP, retry_times: 0, dns_server_index: 0, latency_micros: 98227, linux_errno: SYS_NO_ERROR },{ rcode: NS_R_NO_ERROR, type: NS_T_A, cache_hit: CS_NOTFOUND, ip_version: IV_IPV6, protocol: PROTO_UDP, retry_times: 0, dns_server_index: 0, latency_micros: 101843, linux_errno: SYS_NO_ERROR }] }, sampling_rate_denom: 400, uid: 10230 Change-Id: I113e1671ae28feed8751afd720743bc0f6745e3b --- tests/resolv_stats_test_utils.cpp | 3 +++ tests/resolv_stats_test_utils.h | 4 +++- tests/resolv_stats_test_utils_test.cpp | 4 +++- 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/tests/resolv_stats_test_utils.cpp b/tests/resolv_stats_test_utils.cpp index 492080b8..1704922c 100644 --- a/tests/resolv_stats_test_utils.cpp +++ b/tests/resolv_stats_test_utils.cpp @@ -107,6 +107,8 @@ NetworkDnsEventReported fromNetworkDnsEventReportedStr(const std::string& str) { event.set_private_dns_modes(static_cast(value)); } else if (protoField[1] == "sampling_rate_denom" && ParseInt(protoField[2], &value)) { event.set_sampling_rate_denom(value); + } else if (protoField[1] == "uid" && ParseInt(protoField[2], &value)) { + event.set_uid(value); } } // Parsing each field of the proto DnsQueryEvent @@ -169,6 +171,7 @@ void PrintTo(const NetworkDnsEventReported& event, std::ostream* os) { *os << " network_type: " << event.network_type() << "\n"; *os << " private_dns_modes: " << event.private_dns_modes() << "\n"; *os << " dns_query_event_size: " << event.dns_query_events().dns_query_event_size() << "\n"; + *os << " uid: " << event.uid() << "\n"; *os << "}"; } diff --git a/tests/resolv_stats_test_utils.h b/tests/resolv_stats_test_utils.h index 90fe511a..24e46858 100644 --- a/tests/resolv_stats_test_utils.h +++ b/tests/resolv_stats_test_utils.h @@ -120,7 +120,9 @@ MATCHER_P(NetworkDnsEventEq, other, "") { */ ::testing::Property("dns_query_events", &android::net::NetworkDnsEventReported::dns_query_events, - DnsQueryEventsEq(other.dns_query_events()))), + DnsQueryEventsEq(other.dns_query_events())), + ::testing::Property("uid", &android::net::NetworkDnsEventReported::uid, + ::testing::Eq(other.uid()))), arg, result_listener); } diff --git a/tests/resolv_stats_test_utils_test.cpp b/tests/resolv_stats_test_utils_test.cpp index 3b30e086..af67796c 100644 --- a/tests/resolv_stats_test_utils_test.cpp +++ b/tests/resolv_stats_test_utils_test.cpp @@ -58,7 +58,8 @@ TEST_F(ResolvStatsUtilsTest, NetworkDnsEventEq) { latency_micros: 0, } ] - } + }, + uid: 1000, })Event"; // TODO: Add integration test to verify Level 1 fields of NetworkDnsEventReported. @@ -83,6 +84,7 @@ TEST_F(ResolvStatsUtilsTest, NetworkDnsEventEq) { dnsQueryEvent2->set_dns_server_index(1); dnsQueryEvent2->set_connected(0); dnsQueryEvent2->set_latency_micros(5); + event1.set_uid(1000); EXPECT_THAT(event1, NetworkDnsEventEq(fromNetworkDnsEventReportedStr(event2))); } -- cgit v1.2.3