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

Commit deecd67e authored by Bernie Innocenti's avatar Bernie Innocenti Committed by Gerrit Code Review
Browse files

Merge changes I923d28d5,Ib801a550 

* changes:
  Fix netd crash on gethostbyname(NULL) and add a regression test
  Replace hand-allocated buffers with std::string in DnsProxyListener
parents e250e133 c0d8ada5

DnsProxyListener.cpp

+68 −101
Original line number Diff line number Diff line
@@ -557,21 +557,18 @@ void DnsProxyListener::Handler::spawn() { 
    delete this;

}



DnsProxyListener::GetAddrInfoHandler::GetAddrInfoHandler(SocketClient* c, char* host, char* service,

                                                         addrinfo* hints,

DnsProxyListener::GetAddrInfoHandler::GetAddrInfoHandler(SocketClient* c, std::string host,

                                                         std::string service,

                                                         std::unique_ptr<addrinfo> hints,

                                                         const android_net_context& netcontext)

    : Handler(c), mHost(host), mService(service), mHints(hints), mNetContext(netcontext) {}



DnsProxyListener::GetAddrInfoHandler::~GetAddrInfoHandler() {

    free(mHost);

    free(mService);

    free(mHints);

}

    : Handler(c),

      mHost(move(host)),

      mService(move(service)),

      mHints(move(hints)),

      mNetContext(netcontext) {}



static bool evaluate_domain_name(const android_net_context &netcontext,

                                 const char *host) {

    if (!gResNetdCallbacks.evaluate_domain_name)

        return true;

static bool evaluate_domain_name(const android_net_context& netcontext, const char* host) {

    if (!gResNetdCallbacks.evaluate_domain_name) return true;

    return gResNetdCallbacks.evaluate_domain_name(netcontext, host);

}
@@ -628,11 +625,8 @@ static bool sendaddrinfo(SocketClient* c, addrinfo* ai) { 


    // Write the struct piece by piece because we might be a 64-bit netd

    // talking to a 32-bit process.

    bool success =

            sendBE32(c, ai->ai_flags) &&

            sendBE32(c, ai->ai_family) &&

            sendBE32(c, ai->ai_socktype) &&

            sendBE32(c, ai->ai_protocol);

    bool success = sendBE32(c, ai->ai_flags) && sendBE32(c, ai->ai_family) &&

                   sendBE32(c, ai->ai_socktype) && sendBE32(c, ai->ai_protocol);

    if (!success) {

        return false;

    }
@@ -652,8 +646,6 @@ static bool sendaddrinfo(SocketClient* c, addrinfo* ai) { 


void DnsProxyListener::GetAddrInfoHandler::doDns64Synthesis(int32_t* rv, addrinfo** res,

                                                            NetworkDnsEventReported* event) {

    if (mHost == nullptr) return;



    const bool ipv6WantedButNoData = (mHints && mHints->ai_family == AF_INET6 && *rv == EAI_NODATA);

    const bool unspecWantedButNoIPv6 =

            ((!mHints || mHints->ai_family == AF_UNSPEC) && *rv == 0 && onlyIPv4Answers(*res));
@@ -671,10 +663,12 @@ void DnsProxyListener::GetAddrInfoHandler::doDns64Synthesis(int32_t* rv, addrinf 
        // If caller wants IPv6 answers but no data, try to query IPv4 answers for synthesis

        const uid_t uid = mClient->getUid();

        if (queryLimiter.start(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(mHost, mService, mHints, &mNetContext, res, event);

            *rv = resolv_getaddrinfo(host, service, mHints.get(), &mNetContext, res, event);

            queryLimiter.finish(uid);

            if (*rv) {

                *rv = EAI_NODATA;  // return original error code
@@ -712,9 +706,10 @@ void DnsProxyListener::GetAddrInfoHandler::run() { 
    NetworkDnsEventReported event;

    initDnsEvent(&event, mNetContext);

    if (queryLimiter.start(uid)) {

        if (evaluate_domain_name(mNetContext, mHost)) {

            rv = resolv_getaddrinfo(mHost, mService, mHints, &mNetContext, &result,

                                    &event);

        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)) {

            rv = resolv_getaddrinfo(host, service, mHints.get(), &mNetContext, &result, &event);

        } else {

            rv = EAI_SYSTEM;

        }
@@ -782,8 +777,7 @@ void addIpAddrWithinLimit(std::vector<std::string>* ip_addrs, const sockaddr* ad 


DnsProxyListener::GetAddrInfoCmd::GetAddrInfoCmd() : FrameworkCommand("getaddrinfo") {}



int DnsProxyListener::GetAddrInfoCmd::runCommand(SocketClient *cli,

                                            int argc, char **argv) {

int DnsProxyListener::GetAddrInfoCmd::runCommand(SocketClient* cli, int argc, char** argv) {

    logArguments(argc, argv);



    if (argc != 8) {
@@ -795,21 +789,8 @@ int DnsProxyListener::GetAddrInfoCmd::runCommand(SocketClient *cli, 
        return -1;

    }



    char* name = argv[1];

    if (strcmp("^", name) == 0) {

        name = nullptr;

    } else {

        name = strdup(name);

    }



    char* service = argv[2];

    if (strcmp("^", service) == 0) {

        service = nullptr;

    } else {

        service = strdup(service);

    }



    addrinfo* hints = nullptr;

    const std::string name = argv[1];

    const std::string service = argv[2];

    int ai_flags = strtol(argv[3], nullptr, 10);

    int ai_family = strtol(argv[4], nullptr, 10);

    int ai_socktype = strtol(argv[5], nullptr, 10);
@@ -825,16 +806,16 @@ int DnsProxyListener::GetAddrInfoCmd::runCommand(SocketClient *cli, 
        netcontext.flags |= NET_CONTEXT_FLAG_USE_LOCAL_NAMESERVERS;

    }



    if (ai_flags != -1 || ai_family != -1 ||

        ai_socktype != -1 || ai_protocol != -1) {

        hints = (addrinfo*) calloc(1, sizeof(addrinfo));

    std::unique_ptr<addrinfo> hints;

    if (ai_flags != -1 || ai_family != -1 || ai_socktype != -1 || ai_protocol != -1) {

        hints.reset((addrinfo*)calloc(1, sizeof(addrinfo)));

        hints->ai_flags = ai_flags;

        hints->ai_family = ai_family;

        hints->ai_socktype = ai_socktype;

        hints->ai_protocol = ai_protocol;

    }



    (new GetAddrInfoHandler(cli, name, service, hints, netcontext))->spawn();

    (new GetAddrInfoHandler(cli, name, service, move(hints), netcontext))->spawn();

    return 0;

}
@@ -1043,8 +1024,7 @@ int DnsProxyListener::GetDnsNetIdCommand::runCommand(SocketClient* cli, int argc 
 *******************************************************/

DnsProxyListener::GetHostByNameCmd::GetHostByNameCmd() : FrameworkCommand("gethostbyname") {}



int DnsProxyListener::GetHostByNameCmd::runCommand(SocketClient *cli,

                                            int argc, char **argv) {

int DnsProxyListener::GetHostByNameCmd::runCommand(SocketClient* cli, int argc, char** argv) {

    logArguments(argc, argv);



    if (argc != 4) {
@@ -1059,15 +1039,9 @@ int DnsProxyListener::GetHostByNameCmd::runCommand(SocketClient *cli, 
    uid_t uid = cli->getUid();

    unsigned netId = strtoul(argv[1], nullptr, 10);

    const bool useLocalNameservers = checkAndClearUseLocalNameserversFlag(&netId);

    char* name = argv[2];

    std::string name = argv[2];

    int af = strtol(argv[3], nullptr, 10);



    if (strcmp(name, "^") == 0) {

        name = nullptr;

    } else {

        name = strdup(name);

    }



    android_net_context netcontext;

    gResNetdCallbacks.get_network_context(netId, uid, &netcontext);
@@ -1079,13 +1053,10 @@ int DnsProxyListener::GetHostByNameCmd::runCommand(SocketClient *cli, 
    return 0;

}



DnsProxyListener::GetHostByNameHandler::GetHostByNameHandler(SocketClient* c, char* name, int af,

DnsProxyListener::GetHostByNameHandler::GetHostByNameHandler(SocketClient* c, std::string name,

                                                             int af,

                                                             const android_net_context& netcontext)

    : Handler(c), mName(name), mAf(af), mNetContext(netcontext) {}



DnsProxyListener::GetHostByNameHandler::~GetHostByNameHandler() {

    free(mName);

}

    : Handler(c), mName(move(name)), mAf(af), mNetContext(netcontext) {}



void DnsProxyListener::GetHostByNameHandler::doDns64Synthesis(int32_t* rv, hostent* hbuf, char* buf,

                                                              size_t buflen, struct hostent** hpp,
@@ -1106,7 +1077,8 @@ 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 (queryLimiter.start(uid)) {

        *rv = resolv_gethostbyname(mName, AF_INET, hbuf, buf, buflen, &mNetContext, hpp, event);

        const char* name = mName.starts_with('^') ? nullptr : mName.c_str();

        *rv = resolv_gethostbyname(name, AF_INET, hbuf, buf, buflen, &mNetContext, hpp, event);

        queryLimiter.finish(uid);

        if (*rv) {

            *rv = EAI_NODATA;  // return original error code
@@ -1135,8 +1107,9 @@ void DnsProxyListener::GetHostByNameHandler::run() { 
    NetworkDnsEventReported event;

    initDnsEvent(&event, mNetContext);

    if (queryLimiter.start(uid)) {

        if (evaluate_domain_name(mNetContext, mName)) {

            rv = resolv_gethostbyname(mName, mAf, &hbuf, tmpbuf, sizeof tmpbuf, &mNetContext, &hp,

        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,

                                      &event);

        } else {

            rv = EAI_SYSTEM;
@@ -1184,8 +1157,7 @@ std::string DnsProxyListener::GetHostByNameHandler::threadName() { 
 *******************************************************/

DnsProxyListener::GetHostByAddrCmd::GetHostByAddrCmd() : FrameworkCommand("gethostbyaddr") {}



int DnsProxyListener::GetHostByAddrCmd::runCommand(SocketClient *cli,

                                            int argc, char **argv) {

int DnsProxyListener::GetHostByAddrCmd::runCommand(SocketClient* cli, int argc, char** argv) {

    logArguments(argc, argv);



    if (argc != 5) {
@@ -1204,15 +1176,14 @@ int DnsProxyListener::GetHostByAddrCmd::runCommand(SocketClient *cli, 
    unsigned netId = strtoul(argv[4], nullptr, 10);

    const bool useLocalNameservers = checkAndClearUseLocalNameserversFlag(&netId);



    void* addr = malloc(sizeof(in6_addr));

    in6_addr addr;

    errno = 0;

    int result = inet_pton(addrFamily, addrStr, addr);

    int result = inet_pton(addrFamily, addrStr, &addr);

    if (result <= 0) {

        char* msg = nullptr;

        asprintf(&msg, "inet_pton(\"%s\") failed %s", addrStr, strerror(errno));

        LOG(WARNING) << "GetHostByAddrCmd::runCommand: " << (msg ? msg : "null");

        cli->sendMsg(ResponseCode::OperationFailed, msg, false);

        free(addr);

        free(msg);

        return -1;

    }
@@ -1228,7 +1199,7 @@ int DnsProxyListener::GetHostByAddrCmd::runCommand(SocketClient *cli, 
    return 0;

}



DnsProxyListener::GetHostByAddrHandler::GetHostByAddrHandler(SocketClient* c, void* address,

DnsProxyListener::GetHostByAddrHandler::GetHostByAddrHandler(SocketClient* c, in6_addr address,

                                                             int addressLen, int addressFamily,

                                                             const android_net_context& netcontext)

    : Handler(c),
@@ -1237,15 +1208,11 @@ DnsProxyListener::GetHostByAddrHandler::GetHostByAddrHandler(SocketClient* c, vo 
      mAddressFamily(addressFamily),

      mNetContext(netcontext) {}



DnsProxyListener::GetHostByAddrHandler::~GetHostByAddrHandler() {

    free(mAddress);

}



void DnsProxyListener::GetHostByAddrHandler::doDns64ReverseLookup(hostent* hbuf, char* buf,

                                                                  size_t buflen,

                                                                  struct hostent** hpp,

                                                                  NetworkDnsEventReported* event) {

    if (*hpp != nullptr || mAddressFamily != AF_INET6 || !mAddress) {

    if (*hpp != nullptr || mAddressFamily != AF_INET6) {

        return;

    }
@@ -1260,7 +1227,7 @@ void DnsProxyListener::GetHostByAddrHandler::doDns64ReverseLookup(hostent* hbuf, 


    struct sockaddr_storage ss = netdutils::IPSockAddr(prefix.ip());

    struct sockaddr_in6* v6prefix = (struct sockaddr_in6*)&ss;

    struct in6_addr v6addr = *(in6_addr*) mAddress;

    struct in6_addr v6addr = mAddress;

    // Check if address has NAT64 prefix. Only /96 IPv6 NAT64 prefixes are supported

    if ((v6addr.s6_addr32[0] != v6prefix->sin6_addr.s6_addr32[0]) ||

        (v6addr.s6_addr32[1] != v6prefix->sin6_addr.s6_addr32[1]) ||
@@ -1300,7 +1267,7 @@ void DnsProxyListener::GetHostByAddrHandler::run() { 
    NetworkDnsEventReported event;

    initDnsEvent(&event, mNetContext);

    if (queryLimiter.start(uid)) {

        rv = resolv_gethostbyaddr(mAddress, mAddressLen, mAddressFamily, &hbuf, tmpbuf,

        rv = resolv_gethostbyaddr(&mAddress, mAddressLen, mAddressFamily, &hbuf, tmpbuf,

                                  sizeof tmpbuf, &mNetContext, &hp, &event);

        queryLimiter.finish(uid);

    } else {

DnsProxyListener.h

+12 −12
Original line number Diff line number Diff line
@@ -65,9 +65,9 @@ class DnsProxyListener : public FrameworkListener { 
    class GetAddrInfoHandler : public Handler {

      public:

        // Note: All of host, service, and hints may be NULL

        GetAddrInfoHandler(SocketClient* c, char* host, char* service, addrinfo* hints,

                           const android_net_context& netcontext);

        ~GetAddrInfoHandler() override;

        GetAddrInfoHandler(SocketClient* c, std::string host, std::string service,

                           std::unique_ptr<addrinfo> hints, const android_net_context& netcontext);

        ~GetAddrInfoHandler() override = default;



        void run() override;

        std::string threadName() override;
@@ -75,9 +75,9 @@ class DnsProxyListener : public FrameworkListener { 
      private:

        void doDns64Synthesis(int32_t* rv, addrinfo** res, NetworkDnsEventReported* event);



        char* mHost;            // owned. TODO: convert to std::string.

        char* mService;         // owned. TODO: convert to std::string.

        addrinfo* mHints;       // owned

        std::string mHost;

        std::string mService;

        std::unique_ptr<addrinfo> mHints;

        android_net_context mNetContext;

    };
@@ -91,9 +91,9 @@ class DnsProxyListener : public FrameworkListener { 


    class GetHostByNameHandler : public Handler {

      public:

        GetHostByNameHandler(SocketClient* c, char* name, int af,

        GetHostByNameHandler(SocketClient* c, std::string name, int af,

                             const android_net_context& netcontext);

        ~GetHostByNameHandler() override;

        ~GetHostByNameHandler() override = default;



        void run() override;

        std::string threadName() override;
@@ -102,7 +102,7 @@ class DnsProxyListener : public FrameworkListener { 
        void doDns64Synthesis(int32_t* rv, hostent* hbuf, char* buf, size_t buflen, hostent** hpp,

                              NetworkDnsEventReported* event);



        char* mName;            // owned. TODO: convert to std::string.

        std::string mName;

        int mAf;

        android_net_context mNetContext;

    };
@@ -117,9 +117,9 @@ class DnsProxyListener : public FrameworkListener { 


    class GetHostByAddrHandler : public Handler {

      public:

        GetHostByAddrHandler(SocketClient* c, void* address, int addressLen, int addressFamily,

        GetHostByAddrHandler(SocketClient* c, in6_addr address, int addressLen, int addressFamily,

                             const android_net_context& netcontext);

        ~GetHostByAddrHandler() override;

        ~GetHostByAddrHandler() override = default;



        void run() override;

        std::string threadName() override;
@@ -128,7 +128,7 @@ class DnsProxyListener : public FrameworkListener { 
        void doDns64ReverseLookup(hostent* hbuf, char* buf, size_t buflen, hostent** hpp,

                                  NetworkDnsEventReported* event);



        void* mAddress;         // address to lookup; owned

        in6_addr mAddress;

        int mAddressLen;        // length of address to look up

        int mAddressFamily;     // address family

        android_net_context mNetContext;

gethnamaddr.cpp

+4 −1
Original line number Diff line number Diff line
@@ -388,8 +388,11 @@ nospc: 
int resolv_gethostbyname(const char* name, int af, hostent* hp, char* buf, size_t buflen,

                         const android_net_context* netcontext, hostent** result,

                         NetworkDnsEventReported* event) {

    getnamaddr info;

    if (name == nullptr || hp == nullptr) {

        return EAI_SYSTEM;

    }



    getnamaddr info;

    ResState res;

    res_init(&res, netcontext, event);

tests/resolv_integration_test.cpp

+14 −1
Original line number Diff line number Diff line
@@ -393,7 +393,7 @@ TEST_F(ResolverTest, GetHostByName) { 
    result = gethostbyname("nonexistent");

    EXPECT_EQ(1U, GetNumQueriesForType(dns, ns_type::ns_t_a, nonexistent_host_name));

    ASSERT_TRUE(result == nullptr);

    ASSERT_EQ(HOST_NOT_FOUND, h_errno);

    EXPECT_EQ(HOST_NOT_FOUND, h_errno);



    dns.clearQueries();

    result = gethostbyname("hello");
@@ -405,6 +405,19 @@ TEST_F(ResolverTest, GetHostByName) { 
    EXPECT_TRUE(result->h_addr_list[1] == nullptr);

}



TEST_F(ResolverTest, GetHostByName_NULL) {

    // Most libc implementations would just crash on gethostbyname(NULL). Instead, Bionic

    // serializes the null argument over dnsproxyd, causing the server-side to crash!

    // This is a regression test.

    const char* const testcases[] = {nullptr, "", "^"};

    for (const char* name : testcases) {

        SCOPED_TRACE(fmt::format("gethostbyname({})", name ? name : "NULL"));

        const hostent* result = gethostbyname(name);

        EXPECT_TRUE(result == nullptr);

        EXPECT_EQ(HOST_NOT_FOUND, h_errno);

    }

}



TEST_F(ResolverTest, GetHostByName_cnames) {

    constexpr char host_name[] = "host.example.com.";

    size_t cnamecount = 0;