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

Commit 68951af2 authored by Mike Yu's avatar Mike Yu
Browse files

Send metrics NetworkDnsServerSupportReported to statsd 

The metrics are used to show the validation status of each private
DNS, which can tell us a rough success validation rate and we might
make more improvements based off that. The metrics are per-network
and are filled when destroyNetworkCache() is called.

To avoid adding the dependency stats_proto in resolv_integration_test,
there are also some code changes in resolv_private_dns_test.cpp

Bug: 241788170
Test: atest
Test: m statsd_testdrive; run "statsd_testdrive 504"
Change-Id: I5d79d6280e937ee298485cd85ae71e692fd7a0b8
parent 4eb78901

PrivateDnsConfiguration.cpp

+85 −7
Original line number Diff line number Diff line
@@ -61,18 +61,36 @@ bool ensureNoInvalidIp(const std::vector<std::string>& servers) { 


}  // namespace



PrivateDnsModes convert_enum_type(PrivateDnsMode mode) {

    switch (mode) {

        case PrivateDnsMode::OFF:

            return PrivateDnsModes::PDM_OFF;

        case PrivateDnsMode::OPPORTUNISTIC:

            return PrivateDnsModes::PDM_OPPORTUNISTIC;

        case PrivateDnsMode::STRICT:

            return PrivateDnsModes::PDM_STRICT;

        default:

            return PrivateDnsModes::PDM_UNKNOWN;

    }

}



int PrivateDnsConfiguration::set(int32_t netId, uint32_t mark,

                                 const std::vector<std::string>& servers, const std::string& name,

                                 const std::string& caCert) {

                                 const std::vector<std::string>& unencryptedServers,

                                 const std::vector<std::string>& encryptedServers,

                                 const std::string& name, const std::string& caCert) {

    LOG(DEBUG) << "PrivateDnsConfiguration::set(" << netId << ", 0x" << std::hex << mark << std::dec

               << ", " << servers.size() << ", " << name << ")";

               << ", " << encryptedServers.size() << ", " << name << ")";



    if (!ensureNoInvalidIp(servers)) return -EINVAL;

    if (!ensureNoInvalidIp(encryptedServers)) return -EINVAL;



    std::lock_guard guard(mPrivateDnsLock);

    mUnorderedDnsTracker[netId] = unencryptedServers;

    mUnorderedDotTracker[netId] = encryptedServers;

    mUnorderedDohTracker[netId] = encryptedServers;



    if (!name.empty()) {

        mPrivateDnsModes[netId] = PrivateDnsMode::STRICT;

    } else if (!servers.empty()) {

    } else if (!encryptedServers.empty()) {

        mPrivateDnsModes[netId] = PrivateDnsMode::OPPORTUNISTIC;

    } else {

        mPrivateDnsModes[netId] = PrivateDnsMode::OFF;
@@ -82,11 +100,11 @@ int PrivateDnsConfiguration::set(int32_t netId, uint32_t mark, 
        // TODO: signal validation threads to stop.

    }



    if (int n = setDot(netId, mark, servers, name, caCert); n != 0) {

    if (int n = setDot(netId, mark, encryptedServers, name, caCert); n != 0) {

        return n;

    }

    if (isDoHEnabled()) {

        return setDoh(netId, mark, servers, name, caCert);

        return setDoh(netId, mark, encryptedServers, name, caCert);

    }



    return 0;
@@ -170,10 +188,70 @@ PrivateDnsStatus PrivateDnsConfiguration::getStatus(unsigned netId) const { 
    return status;

}



NetworkDnsServerSupportReported PrivateDnsConfiguration::getStatusForMetrics(unsigned netId) const {

    NetworkDnsServerSupportReported event;

    {

        std::lock_guard guard(mPrivateDnsLock);

        if (const auto it = mPrivateDnsModes.find(netId); it != mPrivateDnsModes.end()) {

            event.set_private_dns_modes(convert_enum_type(it->second));

        } else {

            return event;

        }

    }

    event.set_network_type(resolv_get_network_types_for_net(netId));



    const PrivateDnsStatus status = getStatus(netId);

    std::lock_guard guard(mPrivateDnsLock);

    if (const auto it = mUnorderedDnsTracker.find(netId); it != mUnorderedDnsTracker.end()) {

        for (size_t i = 0; i < it->second.size(); i++) {

            Server* server = event.mutable_servers()->add_server();

            server->set_protocol(PROTO_UDP);

            server->set_index(i);

            server->set_validated(false);

        }

    }



    if (const auto it = mUnorderedDotTracker.find(netId); it != mUnorderedDotTracker.end()) {

        int index = 0;

        const std::list<DnsTlsServer> validatedServers = status.validatedServers();

        for (const std::string& s : it->second) {

            const IPSockAddr target = IPSockAddr::toIPSockAddr(s, kDotPort);

            bool validated =

                    std::any_of(validatedServers.begin(), validatedServers.end(),

                                [&target](DnsTlsServer server) { return server.addr() == target; });

            Server* server = event.mutable_servers()->add_server();

            server->set_protocol(PROTO_DOT);

            server->set_index(index++);

            server->set_validated(validated);

        }

    }



    if (const auto it = mUnorderedDohTracker.find(netId); it != mUnorderedDohTracker.end()) {

        int index = 0;

        for (const std::string& s : it->second) {

            const IPSockAddr target = IPSockAddr::toIPSockAddr(s, kDohPort);

            bool validated = std::any_of(status.dohServersMap.begin(), status.dohServersMap.end(),

                                         [&target](const auto& entry) {

                                             return entry.first == target &&

                                                    entry.second == Validation::success;

                                         });

            Server* server = event.mutable_servers()->add_server();

            server->set_protocol(PROTO_DOH);

            server->set_index(index++);

            server->set_validated(validated);

        }

    }



    return event;

}



void PrivateDnsConfiguration::clear(unsigned netId) {

    LOG(DEBUG) << "PrivateDnsConfiguration::clear(" << netId << ")";

    std::lock_guard guard(mPrivateDnsLock);

    mPrivateDnsModes.erase(netId);

    mUnorderedDnsTracker.erase(netId);

    mUnorderedDotTracker.erase(netId);

    mUnorderedDohTracker.erase(netId);

    clearDot(netId);

    clearDoh(netId);

PrivateDnsConfiguration.h

+14 −2
Original line number Diff line number Diff line
@@ -30,6 +30,7 @@ 
#include <netdutils/DumpWriter.h>

#include <netdutils/InternetAddresses.h>

#include <netdutils/Slice.h>

#include <stats.pb.h>



#include "DnsTlsServer.h"

#include "LockedQueue.h"
@@ -39,6 +40,8 @@ 
namespace android {

namespace net {



PrivateDnsModes convert_enum_type(PrivateDnsMode mode);



struct PrivateDnsStatus {

    PrivateDnsMode mode;
@@ -99,12 +102,15 @@ class PrivateDnsConfiguration { 
        return instance;

    }



    int set(int32_t netId, uint32_t mark, const std::vector<std::string>& servers,

            const std::string& name, const std::string& caCert) EXCLUDES(mPrivateDnsLock);

    int set(int32_t netId, uint32_t mark, const std::vector<std::string>& unencryptedServers,

            const std::vector<std::string>& encryptedServers, const std::string& name,

            const std::string& caCert) EXCLUDES(mPrivateDnsLock);



    void initDoh() EXCLUDES(mPrivateDnsLock);



    PrivateDnsStatus getStatus(unsigned netId) const EXCLUDES(mPrivateDnsLock);

    NetworkDnsServerSupportReported getStatusForMetrics(unsigned netId) const

            EXCLUDES(mPrivateDnsLock);



    void clear(unsigned netId) EXCLUDES(mPrivateDnsLock);
@@ -265,6 +271,12 @@ class PrivateDnsConfiguration { 
             false},

    }};



    // For the metrics. Store the current DNS server list in the same order as what is passed

    // in setResolverConfiguration().

    std::map<unsigned, std::vector<std::string>> mUnorderedDnsTracker GUARDED_BY(mPrivateDnsLock);

    std::map<unsigned, std::vector<std::string>> mUnorderedDotTracker GUARDED_BY(mPrivateDnsLock);

    std::map<unsigned, std::vector<std::string>> mUnorderedDohTracker GUARDED_BY(mPrivateDnsLock);



    struct RecordEntry {

        RecordEntry(uint32_t netId, const ServerIdentity& identity, Validation state)

            : netId(netId), serverIdentity(identity), state(state) {}

PrivateDnsConfigurationTest.cpp

+70 −15
Original line number Diff line number Diff line
@@ -17,6 +17,7 @@ 
#include <gmock/gmock.h>

#include <gtest/gtest.h>

#include <netdutils/NetNativeTestBase.h>

#include <resolv_stats_test_utils.h>



#include "PrivateDnsConfiguration.h"

#include "resolv_cache.h"
@@ -150,7 +151,7 @@ TEST_F(PrivateDnsConfigurationTest, ValidationSuccess) { 
    EXPECT_CALL(mObserver, onValidationStateUpdate(kServer1, Validation::in_process, kNetId));

    EXPECT_CALL(mObserver, onValidationStateUpdate(kServer1, Validation::success, kNetId));



    EXPECT_EQ(mPdc.set(kNetId, kMark, {kServer1}, {}, {}), 0);

    EXPECT_EQ(mPdc.set(kNetId, kMark, {}, {kServer1}, {}, {}), 0);

    expectPrivateDnsStatus(PrivateDnsMode::OPPORTUNISTIC);



    ASSERT_TRUE(PollForCondition([&]() { return mObserver.runningThreads == 0; }));
@@ -163,7 +164,7 @@ TEST_F(PrivateDnsConfigurationTest, ValidationFail_Opportunistic) { 
    EXPECT_CALL(mObserver, onValidationStateUpdate(kServer1, Validation::in_process, kNetId));

    EXPECT_CALL(mObserver, onValidationStateUpdate(kServer1, Validation::fail, kNetId));



    EXPECT_EQ(mPdc.set(kNetId, kMark, {kServer1}, {}, {}), 0);

    EXPECT_EQ(mPdc.set(kNetId, kMark, {}, {kServer1}, {}, {}), 0);

    expectPrivateDnsStatus(PrivateDnsMode::OPPORTUNISTIC);



    // Strictly wait for all of the validation finish; otherwise, the test can crash somehow.
@@ -179,7 +180,7 @@ TEST_F(PrivateDnsConfigurationTest, Revalidation_Opportunistic) { 
    EXPECT_CALL(mObserver, onValidationStateUpdate(kServer1, Validation::in_process, kNetId));

    EXPECT_CALL(mObserver, onValidationStateUpdate(kServer1, Validation::success, kNetId));



    EXPECT_EQ(mPdc.set(kNetId, kMark, {kServer1}, {}, {}), 0);

    EXPECT_EQ(mPdc.set(kNetId, kMark, {}, {kServer1}, {}, {}), 0);

    expectPrivateDnsStatus(PrivateDnsMode::OPPORTUNISTIC);

    ASSERT_TRUE(PollForCondition([&]() { return mObserver.runningThreads == 0; }));
@@ -212,25 +213,25 @@ TEST_F(PrivateDnsConfigurationTest, ValidationBlock) { 
    {

        testing::InSequence seq;

        EXPECT_CALL(mObserver, onValidationStateUpdate(kServer1, Validation::in_process, kNetId));

        EXPECT_EQ(mPdc.set(kNetId, kMark, {kServer1}, {}, {}), 0);

        EXPECT_EQ(mPdc.set(kNetId, kMark, {}, {kServer1}, {}, {}), 0);

        ASSERT_TRUE(PollForCondition([&]() { return mObserver.runningThreads == 1; }));

        expectPrivateDnsStatus(PrivateDnsMode::OPPORTUNISTIC);



        EXPECT_CALL(mObserver, onValidationStateUpdate(kServer2, Validation::in_process, kNetId));

        EXPECT_EQ(mPdc.set(kNetId, kMark, {kServer2}, {}, {}), 0);

        EXPECT_EQ(mPdc.set(kNetId, kMark, {}, {kServer2}, {}, {}), 0);

        ASSERT_TRUE(PollForCondition([&]() { return mObserver.runningThreads == 2; }));

        mObserver.removeFromServerStateMap(kServer1);

        expectPrivateDnsStatus(PrivateDnsMode::OPPORTUNISTIC);



        // No duplicate validation as long as not in OFF mode; otherwise, an unexpected

        // onValidationStateUpdate() will be caught.

        EXPECT_EQ(mPdc.set(kNetId, kMark, {kServer1}, {}, {}), 0);

        EXPECT_EQ(mPdc.set(kNetId, kMark, {kServer1, kServer2}, {}, {}), 0);

        EXPECT_EQ(mPdc.set(kNetId, kMark, {kServer2}, {}, {}), 0);

        EXPECT_EQ(mPdc.set(kNetId, kMark, {}, {kServer1}, {}, {}), 0);

        EXPECT_EQ(mPdc.set(kNetId, kMark, {}, {kServer1, kServer2}, {}, {}), 0);

        EXPECT_EQ(mPdc.set(kNetId, kMark, {}, {kServer2}, {}, {}), 0);

        expectPrivateDnsStatus(PrivateDnsMode::OPPORTUNISTIC);



        // The status keeps unchanged if pass invalid arguments.

        EXPECT_EQ(mPdc.set(kNetId, kMark, {"invalid_addr"}, {}, {}), -EINVAL);

        EXPECT_EQ(mPdc.set(kNetId, kMark, {}, {"invalid_addr"}, {}, {}), -EINVAL);

        expectPrivateDnsStatus(PrivateDnsMode::OPPORTUNISTIC);

    }
@@ -256,12 +257,12 @@ TEST_F(PrivateDnsConfigurationTest, Validation_NetworkDestroyedOrOffMode) { 


        testing::InSequence seq;

        EXPECT_CALL(mObserver, onValidationStateUpdate(kServer1, Validation::in_process, kNetId));

        EXPECT_EQ(mPdc.set(kNetId, kMark, {kServer1}, {}, {}), 0);

        EXPECT_EQ(mPdc.set(kNetId, kMark, {}, {kServer1}, {}, {}), 0);

        ASSERT_TRUE(PollForCondition([&]() { return mObserver.runningThreads == 1; }));

        expectPrivateDnsStatus(PrivateDnsMode::OPPORTUNISTIC);



        if (config == "OFF") {

            EXPECT_EQ(mPdc.set(kNetId, kMark, {}, {}, {}), 0);

            EXPECT_EQ(mPdc.set(kNetId, kMark, {}, {}, {}, {}), 0);

        } else if (config == "NETWORK_DESTROYED") {

            mPdc.clear(kNetId);

        }
@@ -285,10 +286,10 @@ TEST_F(PrivateDnsConfigurationTest, NoValidation) { 
        EXPECT_THAT(status.dotServersMap, testing::IsEmpty());

    };



    EXPECT_EQ(mPdc.set(kNetId, kMark, {"invalid_addr"}, {}, {}), -EINVAL);

    EXPECT_EQ(mPdc.set(kNetId, kMark, {}, {"invalid_addr"}, {}, {}), -EINVAL);

    expectStatus();



    EXPECT_EQ(mPdc.set(kNetId, kMark, {}, {}, {}), 0);

    EXPECT_EQ(mPdc.set(kNetId, kMark, {}, {}, {}, {}), 0);

    expectStatus();

}
@@ -332,7 +333,7 @@ TEST_F(PrivateDnsConfigurationTest, RequestValidation) { 
            ASSERT_TRUE(backend.stopServer());

            EXPECT_CALL(mObserver, onValidationStateUpdate(kServer1, Validation::fail, kNetId));

        }

        EXPECT_EQ(mPdc.set(kNetId, kMark, {kServer1}, {}, {}), 0);

        EXPECT_EQ(mPdc.set(kNetId, kMark, {}, {kServer1}, {}, {}), 0);

        expectPrivateDnsStatus(PrivateDnsMode::OPPORTUNISTIC);



        // Wait until the validation state is transitioned.
@@ -378,7 +379,7 @@ TEST_F(PrivateDnsConfigurationTest, GetPrivateDns) { 
    // Suppress the warning.

    EXPECT_CALL(mObserver, onValidationStateUpdate).Times(2);



    EXPECT_EQ(mPdc.set(kNetId, kMark, {kServer1}, {}, {}), 0);

    EXPECT_EQ(mPdc.set(kNetId, kMark, {}, {kServer1}, {}, {}), 0);

    expectPrivateDnsStatus(PrivateDnsMode::OPPORTUNISTIC);



    EXPECT_TRUE(hasPrivateDnsServer(ServerIdentity(server1), kNetId));
@@ -388,6 +389,60 @@ TEST_F(PrivateDnsConfigurationTest, GetPrivateDns) { 
    ASSERT_TRUE(PollForCondition([&]() { return mObserver.runningThreads == 0; }));

}



// Tests that getStatusForMetrics() returns the correct data.

TEST_F(PrivateDnsConfigurationTest, GetStatusForMetrics) {

    tls2.stopServer();

    const DnsTlsServer server1(netdutils::IPSockAddr::toIPSockAddr(kServer1, 853));

    const DnsTlsServer server2(netdutils::IPSockAddr::toIPSockAddr(kServer2, 853));



    // Suppress the warning.

    EXPECT_CALL(mObserver, onValidationStateUpdate).Times(4);



    // Set 1 unencrypted server and 2 encrypted servers (one will pass DoT validation; the other

    // will fail. Both of them don't support DoH).

    EXPECT_EQ(mPdc.set(kNetId, kMark, {kServer2}, {kServer1, kServer2}, {}, {}), 0);

    ASSERT_TRUE(PollForCondition([&]() { return mObserver.runningThreads == 0; }));



    // Get the metric before call clear().

    NetworkDnsServerSupportReported event = mPdc.getStatusForMetrics(kNetId);

    NetworkDnsServerSupportReported expectedEvent;

    // It's NT_UNKNOWN because this test didn't call resolv_set_nameservers() to set

    // the network type.

    expectedEvent.set_network_type(NetworkType::NT_UNKNOWN);

    expectedEvent.set_private_dns_modes(PrivateDnsModes::PDM_OPPORTUNISTIC);

    Server* server = expectedEvent.mutable_servers()->add_server();

    server->set_protocol(PROTO_UDP);  // kServer2

    server->set_index(0);

    server->set_validated(false);

    server = expectedEvent.mutable_servers()->add_server();

    server->set_protocol(PROTO_DOT);  // kServer1

    server->set_index(0);

    server->set_validated(true);

    server = expectedEvent.mutable_servers()->add_server();

    server->set_protocol(PROTO_DOT);  // kServer2

    server->set_index(1);

    server->set_validated(false);

    server = expectedEvent.mutable_servers()->add_server();

    server->set_protocol(PROTO_DOH);  // kServer1

    server->set_index(0);

    server->set_validated(false);

    server = expectedEvent.mutable_servers()->add_server();

    server->set_protocol(PROTO_DOH);  // kServer2

    server->set_index(1);

    server->set_validated(false);

    EXPECT_THAT(event, NetworkDnsServerSupportEq(expectedEvent));



    // Get the metric after call clear().

    mPdc.clear(kNetId);

    event = mPdc.getStatusForMetrics(kNetId);

    expectedEvent.Clear();

    expectedEvent.set_network_type(NetworkType::NT_UNKNOWN);

    expectedEvent.set_private_dns_modes(PrivateDnsModes::PDM_UNKNOWN);

    EXPECT_THAT(event, NetworkDnsServerSupportEq(expectedEvent));



    tls2.startServer();

}



// TODO: add ValidationFail_Strict test.



}  // namespace android::net

ResolverController.cpp

+12 −2
Original line number Diff line number Diff line
@@ -25,6 +25,7 @@ 
#include <aidl/android/net/IDnsResolver.h>

#include <android-base/logging.h>

#include <android-base/strings.h>

#include <statslog_resolv.h>



#include "Dns64Configuration.h"

#include "DnsResolver.h"
@@ -166,9 +167,17 @@ ResolverController::ResolverController() 
void ResolverController::destroyNetworkCache(unsigned netId) {

    LOG(VERBOSE) << __func__ << ": netId = " << netId;



    // Report NetworkDnsServerSupportReported metrics before the cleanup.

    auto& privateDnsConfiguration = PrivateDnsConfiguration::getInstance();

    NetworkDnsServerSupportReported event = privateDnsConfiguration.getStatusForMetrics(netId);

    const std::string str = event.servers().SerializeAsString();

    stats::BytesField bytesField{str.c_str(), str.size()};

    android::net::stats::stats_write(android::net::stats::NETWORK_DNS_SERVER_SUPPORT_REPORTED,

                                     event.network_type(), event.private_dns_modes(), bytesField);



    resolv_delete_cache_for_net(netId);

    mDns64Configuration.stopPrefixDiscovery(netId);

    PrivateDnsConfiguration::getInstance().clear(netId);

    privateDnsConfiguration.clear(netId);



    // Don't get this instance in PrivateDnsConfiguration. It's probe to deadlock.

    DnsTlsDispatcher::getInstance().forceCleanup(netId);
@@ -207,7 +216,8 @@ int ResolverController::setResolverConfiguration(const ResolverParamsParcel& res 
    // applies to UID 0, dns_mark is assigned for default network rathan the VPN. (note that it's

    // possible that a VPN doesn't have any DNS servers but DoT servers in DNS strict mode)

    auto& privateDnsConfiguration = PrivateDnsConfiguration::getInstance();

    int err = privateDnsConfiguration.set(resolverParams.netId, netcontext.app_mark, tlsServers,

    int err = privateDnsConfiguration.set(resolverParams.netId, netcontext.app_mark,

                                          resolverParams.servers, tlsServers,

                                          resolverParams.tlsName, resolverParams.caCertificate);



    if (err != 0) {

tests/resolv_gold_test.cpp

+3 −2
Original line number Diff line number Diff line
@@ -105,8 +105,9 @@ class TestBase : public NetNativeTestBase { 
        fwmark.explicitlySelected = true;

        fwmark.protectedFromVpn = true;

        fwmark.permission = PERMISSION_SYSTEM;

        ASSERT_EQ(privateDnsConfiguration.set(TEST_NETID, fwmark.intValue, tlsServers, tlsHostname,

                                              caCert),

        ASSERT_EQ(privateDnsConfiguration.set(TEST_NETID, fwmark.intValue,

                                              {} /* unencrypted resolvers */, tlsServers,

                                              tlsHostname, caCert),

                  0);

        ASSERT_EQ(resolv_set_nameservers(TEST_NETID, servers, domains, kParams, std::nullopt), 0);

    }