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

Commit 2dac4381 authored by Luke Huang's avatar Luke Huang
Browse files

Use std::vector to store domains of nameservers and minor change 

1.
Drop the old C style used to store domains.
Previously, resolv is limited to use 6 search domains with total 255 length.
(including zero padding)
After this change, the length of each domain could exactly be at most 255. (rfc 1035)
Also, invalid or duplicate domains will be dropped.

2. rename resolv_set_nameservers_for_net to resolv_set_nameservers

Bug: 135506574
Test: cd system/netd && atest
Change-Id: I94129ea521522c817d087332a7b467f616cc4895
parent 93e602d0

ResolverController.cpp

+4 −6
Original line number Diff line number Diff line
@@ -228,9 +228,7 @@ int ResolverController::setResolverConfiguration( 
        server_ptrs.push_back(resolverParams.servers[i].c_str());

    }



    std::string domains_str = android::base::Join(resolverParams.domains, " ");



    // TODO: Change resolv_set_nameservers_for_net() to use ResolverParamsParcel directly.

    // TODO: Change resolv_set_nameservers() to use ResolverParamsParcel directly.

    res_params res_params = {};

    res_params.sample_validity = resolverParams.sampleValiditySeconds;

    res_params.success_threshold = resolverParams.successThreshold;
@@ -240,10 +238,10 @@ int ResolverController::setResolverConfiguration( 
    res_params.retry_count = resolverParams.retryCount;



    LOG(VERBOSE) << "setDnsServers netId = " << resolverParams.netId

                 << ", numservers = " << resolverParams.domains.size();

                 << ", numservers = " << resolverParams.servers.size();



    return -resolv_set_nameservers_for_net(resolverParams.netId, server_ptrs.data(),

                                           server_ptrs.size(), domains_str.c_str(), &res_params);

    return -resolv_set_nameservers(resolverParams.netId, server_ptrs.data(), server_ptrs.size(),

                                   resolverParams.domains, &res_params);

}



int ResolverController::getResolverInfo(int32_t netId, std::vector<std::string>* servers,

getaddrinfo.cpp

+3 −3
Original line number Diff line number Diff line
@@ -1679,7 +1679,7 @@ static int res_queryN(const char* name, res_target* target, res_state res, int* 
 * is detected.  Error code, if any, is left in *herrno.

 */

static int res_searchN(const char* name, res_target* target, res_state res, int* herrno) {

    const char *cp, *const *domain;

    const char* cp;

    HEADER* hp;

    u_int dots;

    int ret, saved_herrno;
@@ -1722,8 +1722,8 @@ static int res_searchN(const char* name, res_target* target, res_state res, int* 
         */

        _resolv_populate_res_for_net(res);



        for (domain = (const char* const*) res->dnsrch; *domain && !done; domain++) {

            ret = res_querydomainN(name, *domain, target, res, herrno);

        for (const auto& domain : res->search_domains) {

            ret = res_querydomainN(name, domain.c_str(), target, res, herrno);

            if (ret > 0) return ret;



            /*

libnetd_resolv_test.cpp

+33 −33
Original line number Diff line number Diff line
@@ -368,7 +368,7 @@ TEST_F(ResolvGetAddrInfoTest, AlphabeticalHostname_NoData) { 
    dns.addMapping(v4_host_name, ns_type::ns_t_a, "1.2.3.3");

    ASSERT_TRUE(dns.startServer());

    const char* servers[] = {listen_addr};

    ASSERT_EQ(0, resolv_set_nameservers_for_net(TEST_NETID, servers, std::size(servers),

    ASSERT_EQ(0, resolv_set_nameservers(TEST_NETID, servers, std::size(servers),

                                        mDefaultSearchDomains, &mDefaultParams_Binder));

    dns.clearQueries();
@@ -395,7 +395,7 @@ TEST_F(ResolvGetAddrInfoTest, AlphabeticalHostname) { 
    dns.addMapping(host_name, ns_type::ns_t_aaaa, v6addr);

    ASSERT_TRUE(dns.startServer());

    const char* servers[] = {listen_addr};

    ASSERT_EQ(0, resolv_set_nameservers_for_net(TEST_NETID, servers, std::size(servers),

    ASSERT_EQ(0, resolv_set_nameservers(TEST_NETID, servers, std::size(servers),

                                        mDefaultSearchDomains, &mDefaultParams_Binder));



    static const struct TestConfig {
@@ -430,7 +430,7 @@ TEST_F(ResolvGetAddrInfoTest, IllegalHostname) { 
    ASSERT_TRUE(dns.startServer());



    const char* servers[] = {listen_addr};

    ASSERT_EQ(0, resolv_set_nameservers_for_net(TEST_NETID, servers, std::size(servers),

    ASSERT_EQ(0, resolv_set_nameservers(TEST_NETID, servers, std::size(servers),

                                        mDefaultSearchDomains, &mDefaultParams_Binder));



    // Illegal hostname is verified by res_hnok() in system/netd/resolv/res_comp.cpp.
@@ -497,7 +497,7 @@ TEST_F(ResolvGetAddrInfoTest, ServerResponseError) { 
        dns.setResponseProbability(0.0);  // always ignore requests and response preset rcode

        ASSERT_TRUE(dns.startServer());

        const char* servers[] = {listen_addr};

        ASSERT_EQ(0, resolv_set_nameservers_for_net(TEST_NETID, servers, std::size(servers),

        ASSERT_EQ(0, resolv_set_nameservers(TEST_NETID, servers, std::size(servers),

                                            mDefaultSearchDomains, &mDefaultParams_Binder));



        addrinfo* result = nullptr;
@@ -519,7 +519,7 @@ TEST_F(ResolvGetAddrInfoTest, ServerTimeout) { 
    dns.setResponseProbability(0.0);  // always ignore requests and don't response

    ASSERT_TRUE(dns.startServer());

    const char* servers[] = {listen_addr};

    ASSERT_EQ(0, resolv_set_nameservers_for_net(TEST_NETID, servers, std::size(servers),

    ASSERT_EQ(0, resolv_set_nameservers(TEST_NETID, servers, std::size(servers),

                                        mDefaultSearchDomains, &mDefaultParams_Binder));



    addrinfo* result = nullptr;
@@ -543,7 +543,7 @@ TEST_F(ResolvGetAddrInfoTest, CnamesNoIpAddress) { 
    ASSERT_TRUE(dns.startServer());



    const char* servers[] = {listen_addr};

    ASSERT_EQ(0, resolv_set_nameservers_for_net(TEST_NETID, servers, std::size(servers),

    ASSERT_EQ(0, resolv_set_nameservers(TEST_NETID, servers, std::size(servers),

                                        mDefaultSearchDomains, &mDefaultParams_Binder));



    static const struct TestConfig {
@@ -582,7 +582,7 @@ TEST_F(ResolvGetAddrInfoTest, CnamesBrokenChainByIllegalCname) { 
    ASSERT_TRUE(dns.startServer());



    const char* servers[] = {listen_addr};

    ASSERT_EQ(0, resolv_set_nameservers_for_net(TEST_NETID, servers, std::size(servers),

    ASSERT_EQ(0, resolv_set_nameservers(TEST_NETID, servers, std::size(servers),

                                        mDefaultSearchDomains, &mDefaultParams_Binder));



    static const struct TestConfig {
@@ -642,7 +642,7 @@ TEST_F(ResolvGetAddrInfoTest, CnamesInfiniteLoop) { 
    ASSERT_TRUE(dns.startServer());



    const char* servers[] = {listen_addr};

    ASSERT_EQ(0, resolv_set_nameservers_for_net(TEST_NETID, servers, std::size(servers),

    ASSERT_EQ(0, resolv_set_nameservers(TEST_NETID, servers, std::size(servers),

                                        mDefaultSearchDomains, &mDefaultParams_Binder));



    for (const auto& family : {AF_INET, AF_INET6, AF_UNSPEC}) {
@@ -670,7 +670,7 @@ TEST_F(GetHostByNameForNetContextTest, AlphabeticalHostname) { 
    dns.addMapping(host_name, ns_type::ns_t_aaaa, v6addr);

    ASSERT_TRUE(dns.startServer());

    const char* servers[] = {listen_addr};

    ASSERT_EQ(0, resolv_set_nameservers_for_net(TEST_NETID, servers, std::size(servers),

    ASSERT_EQ(0, resolv_set_nameservers(TEST_NETID, servers, std::size(servers),

                                        mDefaultSearchDomains, &mDefaultParams_Binder));



    static const struct TestConfig {
@@ -704,7 +704,7 @@ TEST_F(GetHostByNameForNetContextTest, IllegalHostname) { 
    ASSERT_TRUE(dns.startServer());



    const char* servers[] = {listen_addr};

    ASSERT_EQ(0, resolv_set_nameservers_for_net(TEST_NETID, servers, std::size(servers),

    ASSERT_EQ(0, resolv_set_nameservers(TEST_NETID, servers, std::size(servers),

                                        mDefaultSearchDomains, &mDefaultParams_Binder));



    // Illegal hostname is verified by res_hnok() in system/netd/resolv/res_comp.cpp.
@@ -749,7 +749,7 @@ TEST_F(GetHostByNameForNetContextTest, NoData) { 
    dns.addMapping(v4_host_name, ns_type::ns_t_a, "1.2.3.3");

    ASSERT_TRUE(dns.startServer());

    const char* servers[] = {listen_addr};

    ASSERT_EQ(0, resolv_set_nameservers_for_net(TEST_NETID, servers, std::size(servers),

    ASSERT_EQ(0, resolv_set_nameservers(TEST_NETID, servers, std::size(servers),

                                        mDefaultSearchDomains, &mDefaultParams_Binder));

    dns.clearQueries();
@@ -793,7 +793,7 @@ TEST_F(GetHostByNameForNetContextTest, ServerResponseError) { 
        dns.setResponseProbability(0.0);  // always ignore requests and response preset rcode

        ASSERT_TRUE(dns.startServer());

        const char* servers[] = {listen_addr};

        ASSERT_EQ(0, resolv_set_nameservers_for_net(TEST_NETID, servers, std::size(servers),

        ASSERT_EQ(0, resolv_set_nameservers(TEST_NETID, servers, std::size(servers),

                                            mDefaultSearchDomains, &mDefaultParams_Binder));



        hostent* hp = nullptr;
@@ -814,7 +814,7 @@ TEST_F(GetHostByNameForNetContextTest, ServerTimeout) { 
    dns.setResponseProbability(0.0);  // always ignore requests and don't response

    ASSERT_TRUE(dns.startServer());

    const char* servers[] = {listen_addr};

    ASSERT_EQ(0, resolv_set_nameservers_for_net(TEST_NETID, servers, std::size(servers),

    ASSERT_EQ(0, resolv_set_nameservers(TEST_NETID, servers, std::size(servers),

                                        mDefaultSearchDomains, &mDefaultParams_Binder));



    hostent* hp = nullptr;
@@ -836,7 +836,7 @@ TEST_F(GetHostByNameForNetContextTest, CnamesNoIpAddress) { 
    ASSERT_TRUE(dns.startServer());



    const char* servers[] = {listen_addr};

    ASSERT_EQ(0, resolv_set_nameservers_for_net(TEST_NETID, servers, std::size(servers),

    ASSERT_EQ(0, resolv_set_nameservers(TEST_NETID, servers, std::size(servers),

                                        mDefaultSearchDomains, &mDefaultParams_Binder));



    static const struct TestConfig {
@@ -870,7 +870,7 @@ TEST_F(GetHostByNameForNetContextTest, CnamesBrokenChainByIllegalCname) { 
    ASSERT_TRUE(dns.startServer());



    const char* servers[] = {listen_addr};

    ASSERT_EQ(0, resolv_set_nameservers_for_net(TEST_NETID, servers, std::size(servers),

    ASSERT_EQ(0, resolv_set_nameservers(TEST_NETID, servers, std::size(servers),

                                        mDefaultSearchDomains, &mDefaultParams_Binder));



    static const struct TestConfig {
@@ -929,7 +929,7 @@ TEST_F(GetHostByNameForNetContextTest, CnamesInfiniteLoop) { 
    ASSERT_TRUE(dns.startServer());



    const char* servers[] = {listen_addr};

    ASSERT_EQ(0, resolv_set_nameservers_for_net(TEST_NETID, servers, std::size(servers),

    ASSERT_EQ(0, resolv_set_nameservers(TEST_NETID, servers, std::size(servers),

                                        mDefaultSearchDomains, &mDefaultParams_Binder));



    for (const auto& family : {AF_INET, AF_INET6}) {
@@ -946,7 +946,7 @@ TEST_F(GetHostByNameForNetContextTest, CnamesInfiniteLoop) { 
// Note that local host file function, files_getaddrinfo(), of resolv_getaddrinfo()

// is not tested because it only returns a boolean (success or failure) without any error number.



// TODO: Simplify the DNS server configuration, DNSResponder and resolv_set_nameservers_for_net, as

// TODO: Simplify the DNS server configuration, DNSResponder and resolv_set_nameservers, as

//       ResolverTest does.

// TODO: Add test for resolv_getaddrinfo().

//       - DNS response message parsing.

res_cache.cpp

+42 −52
Original line number Diff line number Diff line
@@ -35,7 +35,10 @@ 
#include <stdlib.h>

#include <string.h>

#include <time.h>

#include <algorithm>

#include <mutex>

#include <set>

#include <vector>



#include <arpa/inet.h>

#include <arpa/nameser.h>
@@ -46,6 +49,7 @@ 


#include <android-base/logging.h>

#include <android-base/parseint.h>

#include <android-base/strings.h>

#include <android-base/thread_annotations.h>

#include <android/multinetwork.h>  // ResNsendFlags
@@ -1142,8 +1146,7 @@ struct resolv_cache_info { 
    int revision_id;  // # times the nameservers have been replaced

    res_params params;

    struct res_stats nsstats[MAXNS];

    char defdname[MAXDNSRCHPATH];

    int dnsrch_offset[MAXDNSRCH + 1];  // offsets into defdname

    std::vector<std::string> search_domains;

    int wait_for_pending_req_timeout_count;

};
@@ -1719,12 +1722,35 @@ static void resolv_set_experiment_params(res_params* params) { 
    }

}



int resolv_set_nameservers_for_net(unsigned netid, const char** servers, const int numservers,

int resolv_set_nameservers(unsigned netid, const char** servers, int numservers,

                           const char* domains, const res_params* params) {

    char* cp;

    int* offset;

    struct addrinfo* nsaddrinfo[MAXNS];

    return resolv_set_nameservers(netid, servers, numservers, android::base::Split(domains, " "),

                                  params);

}



namespace {



// Returns valid domains without duplicates which are limited to max size |MAXDNSRCH|.

std::vector<std::string> filter_domains(const std::vector<std::string>& domains) {

    std::set<std::string> tmp_set;

    std::vector<std::string> res;



    std::copy_if(domains.begin(), domains.end(), std::back_inserter(res),

                 [&tmp_set](const std::string& str) {

                     return !(str.size() > MAXDNSRCHPATH - 1) && (tmp_set.insert(str).second);

                 });

    if (res.size() > MAXDNSRCH) {

        LOG(WARNING) << __func__ << ": valid domains=" << res.size()

                     << ", but MAXDNSRCH=" << MAXDNSRCH;

        res.resize(MAXDNSRCH);

    }

    return res;

}



}  // namespace



int resolv_set_nameservers(unsigned netid, const char** servers, int numservers,

                           const std::vector<std::string>& domains, const res_params* params) {

    if (numservers > MAXNS) {

        LOG(ERROR) << __func__ << ": numservers=" << numservers << ", MAXNS=" << MAXNS;

        return E2BIG;
@@ -1732,6 +1758,8 @@ int resolv_set_nameservers_for_net(unsigned netid, const char** servers, const i 


    // Parse the addresses before actually locking or changing any state, in case there is an error.

    // As a side effect this also reduces the time the lock is kept.

    // TODO: find a better way to replace addrinfo*, something like std::vector<SafeAddrinfo>

    addrinfo* nsaddrinfo[MAXNS];

    char sbuf[NI_MAXSERV];

    snprintf(sbuf, sizeof(sbuf), "%u", NAMESERVER_PORT);

    for (int i = 0; i < numservers; i++) {
@@ -1795,31 +1823,9 @@ int resolv_set_nameservers_for_net(unsigned netid, const char** servers, const i 
        }

    }



    // Always update the search paths, since determining whether they actually changed is

    // complex due to the zero-padding, and probably not worth the effort. Cache-flushing

    // however is not necessary, since the stored cache entries do contain the domain, not

    // just the host name.

    strlcpy(cache_info->defdname, domains, sizeof(cache_info->defdname));

    if ((cp = strchr(cache_info->defdname, '\n')) != NULL) *cp = '\0';

    LOG(INFO) << __func__ << ": domains=\"" << cache_info->defdname << "\"";



    cp = cache_info->defdname;

    offset = cache_info->dnsrch_offset;

    while (offset < cache_info->dnsrch_offset + MAXDNSRCH) {

        while (*cp == ' ' || *cp == '\t') /* skip leading white space */

            cp++;

        if (*cp == '\0') /* stop if nothing more to do */

            break;

        *offset++ = cp - cache_info->defdname; /* record this search domain */

        while (*cp) {                          /* zero-terminate it */

            if (*cp == ' ' || *cp == '\t') {

                *cp++ = '\0';

                break;

            }

            cp++;

        }

    }

    *offset = -1; /* cache_info->dnsrch_offset has MAXDNSRCH+1 items */

    // Always update the search paths. Cache-flushing however is not necessary,

    // since the stored cache entries do contain the domain, not just the host name.

    cache_info->search_domains = filter_domains(domains);



    return 0;

}
@@ -1899,15 +1905,7 @@ void _resolv_populate_res_for_net(res_state statp) { 
            }

        }

        statp->nscount = nserv;

        // now do search domains.  Note that we cache the offsets as this code runs alot

        // but the setting/offset-computer only runs when set/changed

        // WARNING: Don't use str*cpy() here, this string contains zeroes.

        memcpy(statp->defdname, info->defdname, sizeof(statp->defdname));

        char** pp = statp->dnsrch;

        int* p = info->dnsrch_offset;

        while (pp < statp->dnsrch + MAXDNSRCH && *p != -1) {

            *pp++ = &statp->defdname[0] + *p++;

        }

        statp->search_domains = info->search_domains;

    }

}
@@ -1979,17 +1977,9 @@ int android_net_res_stats_get_info_for_net(unsigned netid, int* nscount, 
            memcpy(&servers[i], info->nsaddrinfo[i]->ai_addr, info->nsaddrinfo[i]->ai_addrlen);

            stats[i] = info->nsstats[i];

        }

        for (i = 0; i < MAXDNSRCH; i++) {

            const char* cur_domain = info->defdname + info->dnsrch_offset[i];

            // dnsrch_offset[i] can either be -1 or point to an empty string to indicate the end

            // of the search offsets. Checking for < 0 is not strictly necessary, but safer.

            // TODO: Pass in a search domain array instead of a string to

            // resolv_set_nameservers_for_net() and make this double check unnecessary.

            if (info->dnsrch_offset[i] < 0 ||

                ((size_t) info->dnsrch_offset[i]) >= sizeof(info->defdname) || !cur_domain[0]) {

                break;

            }

            strlcpy(domains[i], cur_domain, MAXDNSRCHPATH);



        for (i = 0; i < static_cast<int>(info->search_domains.size()); i++) {

            strlcpy(domains[i], info->search_domains[i].c_str(), MAXDNSRCHPATH);

        }

        *dcount = i;

        *params = info->params;

res_init.cpp

+0 −29
Original line number Diff line number Diff line
@@ -125,11 +125,7 @@ int res_ninit(res_state statp) { 


/* This function has to be reachable by res_data.c but not publicly. */

int res_vinit(res_state statp, int preinit) {

    char *cp, **pp;

    char buf[BUFSIZ];

    int nserv = 0; /* number of nameserver records read from file */

    int havesearch = 0;

    int dots;

    sockaddr_union u[2];



    if ((statp->options & RES_INIT) != 0U) res_ndestroy(statp);
@@ -162,31 +158,6 @@ int res_vinit(res_state statp, int preinit) { 
    statp->nsort = 0;

    res_setservers(statp, u, nserv);



    if (statp->defdname[0] == 0 && gethostname(buf, sizeof(statp->defdname) - 1) == 0 &&

        (cp = strchr(buf, '.')) != NULL)

        strcpy(statp->defdname, cp + 1);



    /* find components of local domain that might be searched */

    if (havesearch == 0) {

        pp = statp->dnsrch;

        *pp++ = statp->defdname;

        *pp = NULL;



        dots = 0;

        for (cp = statp->defdname; *cp; cp++) dots += (*cp == '.');



        cp = statp->defdname;

        while (pp < statp->dnsrch + MAXDFLSRCH) {

            if (dots < LOCALDOMAINPARTS) break;

            cp = strchr(cp, '.') + 1; /* we know there is one */

            *pp++ = cp;

            dots--;

        }

        *pp = NULL;

        LOG(DEBUG) << __func__ << ": dnsrch list:";

        for (pp = statp->dnsrch; *pp; pp++) LOG(DEBUG) << "\t" << *pp;

    }



    if (nserv > 0) {

        statp->nscount = nserv;

        statp->options |= RES_INIT;