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

Commit bb499099 authored by Mike Yu's avatar Mike Yu
Browse files

Make DoT retries configurable 

DnsTlsTransport re-issues pending queries when onClosed is called.
The call to onClosed is triggered when 1) asynchronous handshake
fails and 2) SSL socket idles for 20 seconds. In either case, retry
on the same DoT server is not always a good solution. Instead, there
are some considerable options, like trying next DoT server, fallbacking
to Do53, or simply returning query failure.

Tuning DoT retries is especially significant to asynchronous
handshake feature because the timeout of the feature is calculated as:

  timeout = dot_connect_timeout_ms * dot_maxtries

Bug: 149445907
Test: cd packages/modules/DnsResolver
      atest with combination of (dot_async_handshake, dot_maxtries)
      which are (0, 3), (0, 1), (1, 3), and (1, 1)
Change-Id: Iceb7bc7f0f6736d900384d1a11eea470761ee32c
parent 19192712

DnsTlsQueryMap.cpp

+8 −1
Original line number Diff line number Diff line
@@ -20,9 +20,16 @@ 


#include <android-base/logging.h>



#include "Experiments.h"



namespace android {

namespace net {



DnsTlsQueryMap::DnsTlsQueryMap() {

    mMaxTries = Experiments::getInstance()->getFlag("dot_maxtries", kMaxTries);

    if (mMaxTries < 1) mMaxTries = 1;

}



std::unique_ptr<DnsTlsQueryMap::QueryFuture> DnsTlsQueryMap::recordQuery(

        const netdutils::Slice query) {

    std::lock_guard guard(mLock);
@@ -67,7 +74,7 @@ void DnsTlsQueryMap::cleanup() { 
    std::lock_guard guard(mLock);

    for (auto it = mQueries.begin(); it != mQueries.end();) {

        auto& p = it->second;

        if (p.tries >= kMaxTries) {

        if (p.tries >= mMaxTries) {

            expire(&p);

            it = mQueries.erase(it);

        } else {

DnsTlsQueryMap.h

+3 −0
Original line number Diff line number Diff line
@@ -36,6 +36,8 @@ class DnsTlsQueryMap { 
  public:

    enum class Response : uint8_t { success, network_error, limit_error, internal_error };



    DnsTlsQueryMap();



    struct Query {

        // The new ID number assigned to this query.

        uint16_t newId;
@@ -80,6 +82,7 @@ class DnsTlsQueryMap { 


    // The maximum number of times we will send a query before abandoning it.

    static constexpr int kMaxTries = 3;

    int mMaxTries;



  private:

    std::mutex mLock;

Experiments.h

+1 −0
Original line number Diff line number Diff line
@@ -51,6 +51,7 @@ class Experiments { 
    static constexpr const char* const kExperimentFlagKeyList[] = {

            "keep_listening_udp", "parallel_lookup",     "parallel_lookup_sleep_time",

            "sort_nameservers",   "dot_async_handshake", "dot_connect_timeout_ms",

            "dot_maxtries",

    };

    // This value is used in updateInternal as the default value if any flags can't be found.

    static constexpr int kFlagIntDefault = INT_MIN;

resolv_tls_unit_test.cpp

+9 −4
Original line number Diff line number Diff line
@@ -32,6 +32,7 @@ 
#include "DnsTlsSessionCache.h"

#include "DnsTlsSocket.h"

#include "DnsTlsTransport.h"

#include "Experiments.h"

#include "IDnsTlsSocket.h"

#include "IDnsTlsSocketFactory.h"

#include "IDnsTlsSocketObserver.h"
@@ -43,6 +44,8 @@ namespace net { 
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) {
@@ -476,8 +479,9 @@ TEST_F(TransportTest, CloseRetryFail) { 
    EXPECT_EQ(DnsTlsTransport::Response::network_error, r.code);

    EXPECT_TRUE(r.response.empty());



    // Reconnections are triggered since DnsTlsQueryMap is not empty.

    EXPECT_EQ(transport.getConnectCounter(), DnsTlsQueryMap::kMaxTries);

    // Reconnections might be triggered depending on the flag.

    EXPECT_EQ(transport.getConnectCounter(),

              Experiments::getInstance()->getFlag(DOT_MAXTRIES_FLAG, DnsTlsQueryMap::kMaxTries));

}



// Simulate a server that occasionally closes the connection and silently
@@ -572,8 +576,9 @@ TEST_F(TransportTest, SilentDrop) { 
        EXPECT_TRUE(r.response.empty());

    }



    // Reconnections are triggered since DnsTlsQueryMap is not empty.

    EXPECT_EQ(transport.getConnectCounter(), DnsTlsQueryMap::kMaxTries);

    // Reconnections might be triggered depending on the flag.

    EXPECT_EQ(transport.getConnectCounter(),

              Experiments::getInstance()->getFlag(DOT_MAXTRIES_FLAG, DnsTlsQueryMap::kMaxTries));

}



TEST_F(TransportTest, PartialDrop) {

tests/resolv_integration_test.cpp

+30 −12
Original line number Diff line number Diff line
@@ -81,6 +81,7 @@ const std::string kSortNameserversFlag("persist.device_config.netd_native.sort_n 
const std::string kDotConnectTimeoutMsFlag(

        "persist.device_config.netd_native.dot_connect_timeout_ms");

const std::string kDotAsyncHandshakeFlag("persist.device_config.netd_native.dot_async_handshake");

const std::string kDotMaxretriesFlag("persist.device_config.netd_native.dot_maxtries");



// Semi-public Bionic hook used by the NDK (frameworks/base/native/android/net.c)

// Tested here for convenience.
@@ -4283,17 +4284,22 @@ TEST_F(ResolverTest, ConnectTlsServerTimeout) { 


    static const struct TestConfig {

        bool asyncHandshake;

        int maxRetries;



        // if asyncHandshake:

        //   expectedTimeout = dotConnectTimeoutMs * maxRetries

        // otherwise:

        //   expectedTimeout = dotConnectTimeoutMs

        int expectedTimeout;

    } testConfigs[] = {

            // expectedTimeout = dotConnectTimeoutMs

            {false, 1000},

            // Test mis-configured dot_maxtries flag.

            {false, 0, 1000}, {true, 0, 1000},



            // expectedTimeout = dotConnectTimeoutMs * DnsTlsQueryMap::kMaxTries

            {true, 3000},

            {false, 1, 1000}, {false, 3, 1000}, {true, 1, 1000}, {true, 3, 3000},

    };



    for (const auto& config : testConfigs) {

        SCOPED_TRACE(fmt::format("testConfig: [{}]", config.asyncHandshake));

        SCOPED_TRACE(fmt::format("testConfig: [{}, {}]", config.asyncHandshake, config.maxRetries));



        // Because a DnsTlsTransport lasts at least 5 minutes in spite of network

        // destroyed, let the resolver creates an unique DnsTlsTransport every time
@@ -4304,8 +4310,10 @@ TEST_F(ResolverTest, ConnectTlsServerTimeout) { 
        test::DnsTlsFrontend tls(addr, "853", addr, "53");

        ASSERT_TRUE(tls.startServer());



        ScopedSystemProperties scopedSystemProperties(kDotAsyncHandshakeFlag,

        ScopedSystemProperties scopedSystemProperties1(kDotAsyncHandshakeFlag,

                                                       config.asyncHandshake ? "1" : "0");

        ScopedSystemProperties scopedSystemProperties2(kDotMaxretriesFlag,

                                                       std::to_string(config.maxRetries));

        resetNetwork();



        // Set up resolver to opportunistic mode.
@@ -4362,14 +4370,22 @@ TEST_F(ResolverTest, ConnectTlsServerTimeout_ConcurrentQueries) { 
    static const struct TestConfig {

        bool asyncHandshake;

        int dotConnectTimeoutMs;

        int maxRetries;

        int concurrency;



        // if asyncHandshake:

        //   expectedTimeout = dotConnectTimeoutMs * maxRetries

        // otherwise:

        //   expectedTimeout = dotConnectTimeoutMs * concurrency

        int expectedTimeout;

    } testConfigs[] = {

            // expectedTimeout = dotConnectTimeoutMs * concurrency

            {false, 1000, 5, 5000},



            // expectedTimeout = dotConnectTimeoutMs * DnsTlsQueryMap::kMaxTries

            {true, 1000, 5, 3000},

            // clang-format off

            {false, 1000, 1, 5, 5000},

            {false, 1000, 3, 5, 5000},

            {true, 1000, 1, 5, 1000},

            {true, 2500, 1, 10, 2500},

            {true, 1000, 3, 5, 3000},

            // clang-format on

    };



    // Launch query threads. Expected behaviors are:
@@ -4382,6 +4398,8 @@ TEST_F(ResolverTest, ConnectTlsServerTimeout_ConcurrentQueries) { 
                                                       std::to_string(config.dotConnectTimeoutMs));

        ScopedSystemProperties scopedSystemProperties2(kDotAsyncHandshakeFlag,

                                                       config.asyncHandshake ? "1" : "0");

        ScopedSystemProperties scopedSystemProperties3(kDotMaxretriesFlag,

                                                       std::to_string(config.maxRetries));

        resetNetwork();



        for (const auto& dnsMode : {"OPPORTUNISTIC", "STRICT"}) {