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

Commit 425d16ee authored by Mike Yu's avatar Mike Yu
Browse files

Remove parseServer 

IPAddress and IPSockAddr are well-tested utilities. Use them in
DnsTlsServer so that many code related to sockaddr_storage can
be removed.

Bug: 186177613
Test: cd packages/modules/DnsResolver && atest
Change-Id: I59f59f6f50bd7b376fa0f8775c8999eee7cf9004
parent 88d5702d

DnsTlsServer.h

+3 −2
Original line number Diff line number Diff line
@@ -35,8 +35,9 @@ struct DnsTlsServer : public IPrivateDnsServer { 
    // Default constructor.

    DnsTlsServer() {}



    // Allow sockaddr_storage to be promoted to DnsTlsServer automatically.

    DnsTlsServer(const sockaddr_storage& ss) : ss(ss) {}

    explicit DnsTlsServer(const netdutils::IPAddress& ip)

        : DnsTlsServer(netdutils::IPSockAddr(ip, 853)) {}

    explicit DnsTlsServer(const netdutils::IPSockAddr& addr) : ss(addr) {}



    // The server location, including IP and port.

    // TODO: make it const.

PrivateDnsConfiguration.cpp

+6 −21
Original line number Diff line number Diff line
@@ -38,6 +38,7 @@ using aidl::android::net::resolv::aidl::IDnsResolverUnsolicitedEventListener; 
using aidl::android::net::resolv::aidl::PrivateDnsValidationEventParcel;

using android::base::StringPrintf;

using android::netdutils::IPAddress;

using android::netdutils::IPSockAddr;

using android::netdutils::setThreadName;

using android::netdutils::Slice;

using std::chrono::milliseconds;
@@ -45,24 +46,6 @@ using std::chrono::milliseconds; 
namespace android {

namespace net {



bool parseServer(const char* server, sockaddr_storage* parsed) {

    addrinfo hints = {

            .ai_flags = AI_NUMERICHOST | AI_NUMERICSERV,

            .ai_family = AF_UNSPEC,

    };

    addrinfo* res;



    int err = getaddrinfo(server, "853", &hints, &res);

    if (err != 0) {

        LOG(WARNING) << "Failed to parse server address (" << server << "): " << gai_strerror(err);

        return false;

    }



    memcpy(parsed, res->ai_addr, res->ai_addrlen);

    freeaddrinfo(res);

    return true;

}



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

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

                                 const std::string& caCert) {
@@ -72,11 +55,13 @@ int PrivateDnsConfiguration::set(int32_t netId, uint32_t mark, 
    // Parse the list of servers that has been passed in

    PrivateDnsTracker tmp;

    for (const auto& s : servers) {

        sockaddr_storage parsed;

        if (!parseServer(s.c_str(), &parsed)) {

        IPAddress ip;

        if (!IPAddress::forString(s, &ip)) {

            LOG(WARNING) << "Failed to parse server address (" << s << ")";

            return -EINVAL;

        }

        auto server = std::make_unique<DnsTlsServer>(parsed);



        auto server = std::make_unique<DnsTlsServer>(ip);

        server->name = name;

        server->certificate = caCert;

        server->mark = mark;

tests/resolv_tls_unit_test.cpp

+23 −68
Original line number Diff line number Diff line
@@ -41,53 +41,29 @@ 
namespace android {

namespace net {



using netdutils::IPAddress;

using netdutils::IPSockAddr;

using netdutils::makeSlice;

using netdutils::Slice;



static const std::string DOT_MAXTRIES_FLAG = "dot_maxtries";



typedef std::vector<uint8_t> bytevec;



static void parseServer(const char* server, in_port_t port, sockaddr_storage* parsed) {

    sockaddr_in* sin = reinterpret_cast<sockaddr_in*>(parsed);

    if (inet_pton(AF_INET, server, &(sin->sin_addr)) == 1) {

        // IPv4 parse succeeded, so it's IPv4

        sin->sin_family = AF_INET;

        sin->sin_port = htons(port);

        return;

    }

    sockaddr_in6* sin6 = reinterpret_cast<sockaddr_in6*>(parsed);

    if (inet_pton(AF_INET6, server, &(sin6->sin6_addr)) == 1){

        // IPv6 parse succeeded, so it's IPv6.

        sin6->sin6_family = AF_INET6;

        sin6->sin6_port = htons(port);

        return;

    }

    LOG(ERROR) << "Failed to parse server address: " << server;

}



std::string SERVERNAME1 = "dns.example.com";

std::string SERVERNAME2 = "dns.example.org";

static const std::string DOT_MAXTRIES_FLAG = "dot_maxtries";

static const std::string SERVERNAME1 = "dns.example.com";

static const std::string SERVERNAME2 = "dns.example.org";

static const IPAddress V4ADDR1 = IPAddress::forString("192.0.2.1");

static const IPAddress V4ADDR2 = IPAddress::forString("192.0.2.2");

static const IPAddress V6ADDR1 = IPAddress::forString("2001:db8::1");

static const IPAddress V6ADDR2 = IPAddress::forString("2001:db8::2");



// BaseTest just provides constants that are useful for the tests.

class BaseTest : public ::testing::Test {

  protected:

    BaseTest() {

        parseServer("192.0.2.1", 853, &V4ADDR1);

        parseServer("192.0.2.2", 853, &V4ADDR2);

        parseServer("2001:db8::1", 853, &V6ADDR1);

        parseServer("2001:db8::2", 853, &V6ADDR2);



        SERVER1 = DnsTlsServer(V4ADDR1);

        SERVER1.name = SERVERNAME1;

    }



    sockaddr_storage V4ADDR1;

    sockaddr_storage V4ADDR2;

    sockaddr_storage V6ADDR1;

    sockaddr_storage V6ADDR2;



    DnsTlsServer SERVER1;

    DnsTlsServer SERVER1{V4ADDR1};

};



bytevec make_query(uint16_t id, size_t size) {
@@ -817,26 +793,23 @@ void checkEqual(const DnsTlsServer& s1, const DnsTlsServer& s2) { 
class ServerTest : public BaseTest {};



TEST_F(ServerTest, IPv4) {

    checkUnequal(V4ADDR1, V4ADDR2);

    EXPECT_FALSE(isAddressEqual(V4ADDR1, V4ADDR2));

    checkUnequal(DnsTlsServer(V4ADDR1), DnsTlsServer(V4ADDR2));

    EXPECT_FALSE(isAddressEqual(DnsTlsServer(V4ADDR1), DnsTlsServer(V4ADDR2)));

}



TEST_F(ServerTest, IPv6) {

    checkUnequal(V6ADDR1, V6ADDR2);

    EXPECT_FALSE(isAddressEqual(V6ADDR1, V6ADDR2));

    checkUnequal(DnsTlsServer(V6ADDR1), DnsTlsServer(V6ADDR2));

    EXPECT_FALSE(isAddressEqual(DnsTlsServer(V6ADDR1), DnsTlsServer(V6ADDR2)));

}



TEST_F(ServerTest, MixedAddressFamily) {

    checkUnequal(V6ADDR1, V4ADDR1);

    EXPECT_FALSE(isAddressEqual(V6ADDR1, V4ADDR1));

    checkUnequal(DnsTlsServer(V6ADDR1), DnsTlsServer(V4ADDR1));

    EXPECT_FALSE(isAddressEqual(DnsTlsServer(V6ADDR1), DnsTlsServer(V4ADDR1)));

}



TEST_F(ServerTest, IPv6ScopeId) {

    DnsTlsServer s1(V6ADDR1), s2(V6ADDR1);

    sockaddr_in6* addr1 = reinterpret_cast<sockaddr_in6*>(&s1.ss);

    addr1->sin6_scope_id = 1;

    sockaddr_in6* addr2 = reinterpret_cast<sockaddr_in6*>(&s2.ss);

    addr2->sin6_scope_id = 2;

    DnsTlsServer s1(IPAddress::forString("fe80::1%1"));

    DnsTlsServer s2(IPAddress::forString("fe80::1%2"));

    checkUnequal(s1, s2);

    EXPECT_FALSE(isAddressEqual(s1, s2));
@@ -844,32 +817,16 @@ TEST_F(ServerTest, IPv6ScopeId) { 
    EXPECT_FALSE(s2.wasExplicitlyConfigured());

}



TEST_F(ServerTest, IPv6FlowInfo) {

    DnsTlsServer s1(V6ADDR1), s2(V6ADDR1);

    sockaddr_in6* addr1 = reinterpret_cast<sockaddr_in6*>(&s1.ss);

    addr1->sin6_flowinfo = 1;

    sockaddr_in6* addr2 = reinterpret_cast<sockaddr_in6*>(&s2.ss);

    addr2->sin6_flowinfo = 2;

    // All comparisons ignore flowinfo.

    EXPECT_EQ(s1, s2);

    EXPECT_TRUE(isAddressEqual(s1, s2));



    EXPECT_FALSE(s1.wasExplicitlyConfigured());

    EXPECT_FALSE(s2.wasExplicitlyConfigured());

}



TEST_F(ServerTest, Port) {

    DnsTlsServer s1, s2;

    parseServer("192.0.2.1", 853, &s1.ss);

    parseServer("192.0.2.1", 854, &s2.ss);

    DnsTlsServer s1(IPSockAddr::toIPSockAddr("192.0.2.1", 853));

    DnsTlsServer s2(IPSockAddr::toIPSockAddr("192.0.2.1", 854));

    checkUnequal(s1, s2);

    EXPECT_TRUE(isAddressEqual(s1, s2));

    EXPECT_EQ(s1.toIpString(), "192.0.2.1");

    EXPECT_EQ(s2.toIpString(), "192.0.2.1");



    DnsTlsServer s3, s4;

    parseServer("2001:db8::1", 853, &s3.ss);

    parseServer("2001:db8::1", 852, &s4.ss);

    DnsTlsServer s3(IPSockAddr::toIPSockAddr("2001:db8::1", 853));

    DnsTlsServer s4(IPSockAddr::toIPSockAddr("2001:db8::1", 854));

    checkUnequal(s3, s4);

    EXPECT_TRUE(isAddressEqual(s3, s4));

    EXPECT_EQ(s3.toIpString(), "2001:db8::1");
@@ -1017,8 +974,6 @@ class DnsTlsSocketTest : public ::testing::Test { 
        MOCK_METHOD(void, onResponse, (std::vector<uint8_t>), (override));

    };



    DnsTlsSocketTest() { parseServer(kTlsAddr, std::stoi(kTlsPort), &server.ss); }



    std::unique_ptr<DnsTlsSocket> makeDnsTlsSocket(IDnsTlsSocketObserver* observer) {

        return std::make_unique<DnsTlsSocket>(this->server, MARK, observer, &this->cache);

    }
@@ -1037,7 +992,7 @@ class DnsTlsSocketTest : public ::testing::Test { 


    test::DnsTlsFrontend tls{kTlsAddr, kTlsPort, kBackendAddr, kBackendPort};



    DnsTlsServer server;

    const DnsTlsServer server{IPSockAddr::toIPSockAddr(kTlsAddr, std::stoi(kTlsPort))};

    DnsTlsSessionCache cache;

};