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

Commit 7d2fbb81 authored by Luke Huang's avatar Luke Huang
Browse files

Fix the unexpected getaddrinfo error code changing due to parallel lookup 

Parallel lookup unexpectedly changed the error code when the queries are
blocked by system.
2 problems here,
    1. the errno from query thread isn't correctly propagated to the main
thread.
    2. The rcode handling is changed.
Fix them in this CL and also add a test for this problem.

Bug: 185097438
Test: atest
Change-Id: I4c8e2895baa9d3d8ece40fe7bb8c3975672997a4
parent 72799f37

getaddrinfo.cpp

+7 −1
Original line number Diff line number Diff line
@@ -1603,6 +1603,7 @@ struct QueryResult { 
    int ancount;

    int rcode;

    int herrno;

    int qerrno;

    NetworkDnsEventReported event;

};
@@ -1635,6 +1636,7 @@ QueryResult doQuery(const char* name, res_target* t, res_state res, 
                .ancount = 0,

                .rcode = -1,

                .herrno = NO_RECOVERY,

                .qerrno = errno,

                .event = event,

        };

    }
@@ -1644,13 +1646,15 @@ QueryResult doQuery(const char* name, res_target* t, res_state res, 
    int rcode = NOERROR;

    n = res_nsend(&res_temp, buf, n, t->answer.data(), anslen, &rcode, 0, sleepTimeMs);

    if (n < 0 || hp->rcode != NOERROR || ntohs(hp->ancount) == 0) {

        // To ensure that the rcode handling is identical to res_queryN().

        if (rcode != RCODE_TIMEOUT) rcode = hp->rcode;

        // if the query choked with EDNS0, retry without EDNS0

        if ((res_temp.netcontext_flags &

             (NET_CONTEXT_FLAG_USE_DNS_OVER_TLS | NET_CONTEXT_FLAG_USE_EDNS)) &&

            (res_temp._flags & RES_F_EDNS0ERR)) {

            LOG(DEBUG) << __func__ << ": retry without EDNS0";

            n = res_nmkquery(QUERY, name, cl, type, /*data=*/nullptr, /*datalen=*/0, buf,

                             sizeof(buf), res->netcontext_flags);

                             sizeof(buf), res_temp.netcontext_flags);

            n = res_nsend(&res_temp, buf, n, t->answer.data(), anslen, &rcode, 0);

        }

    }
@@ -1661,6 +1665,7 @@ QueryResult doQuery(const char* name, res_target* t, res_state res, 
    return {

            .ancount = ntohs(hp->ancount),

            .rcode = rcode,

            .qerrno = errno,

            .event = event,

    };

}
@@ -1695,6 +1700,7 @@ static int res_queryN_parallel(const char* name, res_target* target, res_state r 
        res->event->MergeFrom(r.event);

        ancount += r.ancount;

        rcode = r.rcode;

        errno = r.qerrno;

    }



    if (ancount == 0) {

tests/resolv_integration_test.cpp

+50 −0
Original line number Diff line number Diff line
@@ -4249,6 +4249,56 @@ TEST_F(ResolverTest, BlockDnsQueryWithUidRule) { 
    }

}



TEST_F(ResolverTest, GetAddrinfo_BlockDnsQueryWithUidRule) {

    SKIP_IF_BPF_NOT_SUPPORTED;

    constexpr char listen_addr1[] = "127.0.0.4";

    constexpr char listen_addr2[] = "::1";

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

    const std::vector<DnsRecord> records = {

            {host_name, ns_type::ns_t_a, "1.2.3.4"},

            {host_name, ns_type::ns_t_aaaa, "::1.2.3.4"},

    };

    test::DNSResponder dns1(listen_addr1);

    test::DNSResponder dns2(listen_addr2);

    StartDns(dns1, records);

    StartDns(dns2, records);



    std::vector<std::string> servers = {listen_addr1, listen_addr2};

    ASSERT_TRUE(mDnsClient.SetResolversForNetwork(servers, kDefaultSearchDomains, kDefaultParams));



    const addrinfo hints = {.ai_family = AF_UNSPEC, .ai_socktype = SOCK_DGRAM};



    static struct {

        const char* hname;

        const int expectedErrorCode;

    } kTestData[] = {

            {host_name, EAI_NODATA},

            // To test the query with search domain.

            {"howdy", EAI_AGAIN},

    };



    INetd* netdService = mDnsClient.netdService();

    for (auto& td : kTestData) {

        SCOPED_TRACE(td.hname);

        ScopeBlockedUIDRule scopeBlockUidRule(netdService, TEST_UID);

        // If API level >= 30 (R+), these queries should be blocked.

        if (isAtLeastR) {

            addrinfo* result = nullptr;

            // getaddrinfo() in bionic would convert all errors to EAI_NODATA

            // except EAI_SYSTEM.

            EXPECT_EQ(EAI_NODATA, getaddrinfo(td.hname, nullptr, &hints, &result));

            ExpectDnsEvent(INetdEventListener::EVENT_GETADDRINFO, td.expectedErrorCode, td.hname,

                           {});

        } else {

            ScopedAddrinfo result = safe_getaddrinfo(td.hname, nullptr, &hints);

            EXPECT_NE(nullptr, result);

            EXPECT_THAT(ToStrings(result),

                        testing::UnorderedElementsAreArray({"1.2.3.4", "::1.2.3.4"}));

            // To avoid flaky test, do not evaluate DnsEvent since event order is not guaranteed.

        }

    }

}



TEST_F(ResolverTest, EnforceDnsUid) {

    SKIP_IF_BPF_NOT_SUPPORTED;

    constexpr char listen_addr1[] = "127.0.0.4";