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

Commit d1b3b803 authored by Mike Yu's avatar Mike Yu Committed by Gerrit Code Review
Browse files

Merge "Revert "Support evaluating private DNS by latency""

parents 9d08a4f7 5daa40db

DnsTlsDispatcher.cpp

+2 −19
Original line number Diff line number Diff line
@@ -182,18 +182,9 @@ DnsTlsTransport::Response DnsTlsDispatcher::query(const DnsTlsServer& server, un 
    // stuck, this function also gets blocked.

    const int connectCounter = xport->transport.getConnectCounter();



    Stopwatch stopwatch;

    const auto& result = queryInternal(*xport, query);

    const int64_t timeTaken = saturate_cast<int64_t>(stopwatch.timeTakenUs() / 1000);

    *connectTriggered = (xport->transport.getConnectCounter() > connectCounter);



    const int64_t targetTime = server.latencyThreshold().value_or(INT64_MAX);

    const bool latencyTooHigh = timeTaken > targetTime;

    if (latencyTooHigh) {

        LOG(WARNING) << "DoT query took too long: " << timeTaken << " ms (threshold: " << targetTime

                     << "ms)";

    }



    DnsTlsTransport::Response code = result.code;

    if (code == DnsTlsTransport::Response::success) {

        if (result.response.size() > ans.size()) {
@@ -215,7 +206,7 @@ DnsTlsTransport::Response DnsTlsDispatcher::query(const DnsTlsServer& server, un 
        xport->lastUsed = now;



        // DoT revalidation specific feature.

        if (xport->checkRevalidationNecessary(code, latencyTooHigh)) {

        if (xport->checkRevalidationNecessary(code)) {

            // Even if the revalidation passes, it doesn't guarantee that DoT queries

            // to the xport can stop failing because revalidation creates a new connection

            // to probe while the xport still uses an existing connection. So far, there isn't
@@ -342,21 +333,13 @@ DnsTlsDispatcher::Transport* DnsTlsDispatcher::getTransport(const Key& key) { 
    return (it == mStore.end() ? nullptr : it->second.get());

}



bool DnsTlsDispatcher::Transport::checkRevalidationNecessary(DnsTlsTransport::Response code,

                                                             bool latencyTooHigh) {

bool DnsTlsDispatcher::Transport::checkRevalidationNecessary(DnsTlsTransport::Response code) {

    if (!revalidationEnabled) return false;



    if (code == DnsTlsTransport::Response::network_error) {

        continuousfailureCount++;

        LOG(WARNING) << "continuousfailureCount incremented: network_error, count = "

                     << continuousfailureCount;

    } else if (latencyTooHigh) {

        continuousfailureCount++;

        LOG(WARNING) << "continuousfailureCount incremented: latency too High, count = "

                     << continuousfailureCount;

    } else {

        continuousfailureCount = 0;

        LOG(WARNING) << "continuousfailureCount reset";

    }



    // triggerThreshold must be greater than 0 because the value of revalidationEnabled is true.

DnsTlsDispatcher.h

+1 −2
Original line number Diff line number Diff line
@@ -107,8 +107,7 @@ class DnsTlsDispatcher : public PrivateDnsValidationObserver { 
        // whether or not this Transport is usable.

        bool usable() const REQUIRES(sLock);



        bool checkRevalidationNecessary(DnsTlsTransport::Response code, bool latencyTooHigh)

                REQUIRES(sLock);

        bool checkRevalidationNecessary(DnsTlsTransport::Response code) REQUIRES(sLock);



        std::chrono::milliseconds timeout() const { return mTimeout; }

DnsTlsServer.h

+0 −9
Original line number Diff line number Diff line
@@ -86,19 +86,10 @@ struct DnsTlsServer : public IPrivateDnsServer { 
    bool active() const override { return mActive; }

    void setActive(bool val) override { mActive = val; }



    // Setter and getter for the latency constraint.

    void setLatencyThreshold(std::optional<int64_t> val) { mLatencyThresholdMs = val; }

    std::optional<int64_t> latencyThreshold() const { return mLatencyThresholdMs; }



  private:

    // State, unrelated to the comparison of DnsTlsServer objects.

    Validation mValidation = Validation::unknown_server;

    bool mActive = false;



    // The DNS response time threshold. If it is set, the latency of this server will be

    // considered when evaluating the server. It is used for the DoT engine to evaluate whether

    // this server, compared to cleartext DNS servers, has relatively high latency or not.

    std::optional<int64_t> mLatencyThresholdMs = std::nullopt;

};



// This comparison only checks the IP address.  It ignores ports, names, and fingerprints.

Experiments.h

+4 −12
Original line number Diff line number Diff line
@@ -49,19 +49,11 @@ class Experiments { 
    // TODO: Migrate other experiment flags to here.

    // (retry_count, retransmission_time_interval)

    static constexpr const char* const kExperimentFlagKeyList[] = {

            "keep_listening_udp",

            "parallel_lookup_release",

            "parallel_lookup_sleep_time",

            "sort_nameservers",

            "dot_async_handshake",

            "dot_connect_timeout_ms",

            "dot_maxtries",

            "dot_revalidation_threshold",

            "dot_xport_unusable_threshold",

            "keep_listening_udp",   "parallel_lookup_release",    "parallel_lookup_sleep_time",

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

            "dot_maxtries",         "dot_revalidation_threshold", "dot_xport_unusable_threshold",

            "dot_query_timeout_ms",

            "avoid_bad_private_dns",

            "min_private_dns_latency_threshold_ms",

            "max_private_dns_latency_threshold_ms"};

    };

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

    static constexpr int kFlagIntDefault = INT_MIN;

    // For testing.

PrivateDnsConfiguration.cpp

+12 −71
Original line number Diff line number Diff line
@@ -21,23 +21,18 @@ 
#include <android-base/format.h>

#include <android-base/logging.h>

#include <android-base/stringprintf.h>

#include <netdutils/Stopwatch.h>

#include <netdutils/ThreadUtil.h>

#include <sys/socket.h>



#include "DnsTlsTransport.h"

#include "Experiments.h"

#include "ResolverEventReporter.h"

#include "netd_resolv/resolv.h"

#include "resolv_cache.h"

#include "resolv_private.h"

#include "util.h"



using aidl::android::net::resolv::aidl::IDnsResolverUnsolicitedEventListener;

using aidl::android::net::resolv::aidl::PrivateDnsValidationEventParcel;

using android::base::StringPrintf;

using android::netdutils::setThreadName;

using android::netdutils::Stopwatch;

using std::chrono::milliseconds;



namespace android {
@@ -200,23 +195,6 @@ void PrivateDnsConfiguration::startValidation(const ServerIdentity& identity, un 
    std::thread validate_thread([this, identity, server, netId, isRevalidation] {

        setThreadName(StringPrintf("TlsVerify_%u", netId).c_str());



        const bool avoidBadPrivateDns =

                Experiments::getInstance()->getFlag("avoid_bad_private_dns", 0);

        std::optional<int64_t> latencyThreshold;

        if (avoidBadPrivateDns) {

            const int maxLatency = Experiments::getInstance()->getFlag(

                    "max_private_dns_latency_threshold_ms", kMaxPrivateDnsLatencyThresholdMs);

            const int minLatency = Experiments::getInstance()->getFlag(

                    "min_private_dns_latency_threshold_ms", kMinPrivateDnsLatencyThresholdMs);

            const auto do53Latency = resolv_stats_get_average_response_time(netId, PROTO_UDP);

            const int target =

                    do53Latency.has_value() ? (3 * do53Latency.value().count() / 1000) : 0;



            // The time is limited to the range [minLatency, maxLatency].

            latencyThreshold = std::clamp(target, minLatency, maxLatency);

        }

        const bool isOpportunisticMode = server.name.empty();



        // cat /proc/sys/net/ipv4/tcp_syn_retries yields "6".

        //

        // Start with a 1 minute delay and backoff to once per hour.
@@ -237,24 +215,12 @@ void PrivateDnsConfiguration::startValidation(const ServerIdentity& identity, un 
            // It can take milliseconds to minutes, up to the SYN retry limit.

            LOG(WARNING) << "Validating DnsTlsServer " << server.toIpString() << " with mark 0x"

                         << std::hex << server.validationMark();

            const bool success = DnsTlsTransport::validate(server, server.validationMark());

            LOG(WARNING) << "validateDnsTlsServer returned " << success << " for "

                         << server.toIpString();



            Stopwatch stopwatch;

            const bool gotAnswer = DnsTlsTransport::validate(server, server.validationMark());

            const int32_t timeTaken = saturate_cast<int32_t>(stopwatch.timeTakenUs() / 1000);

            LOG(WARNING) << fmt::format("validateDnsTlsServer returned {} for {}, took {}ms",

                                        gotAnswer, server.toIpString(), timeTaken);



            const int64_t targetTime = latencyThreshold.value_or(INT64_MAX);

            bool latencyTooHigh = false;

            if (isOpportunisticMode && timeTaken > targetTime) {

                latencyTooHigh = true;

                LOG(WARNING) << "validateDnsTlsServer took too long: threshold is " << targetTime

                             << "ms";

            }



            // TODO: combine these boolean variables into a bitwise variable.

            const bool needs_reeval = this->recordPrivateDnsValidation(

                    identity, netId, gotAnswer, isRevalidation, latencyTooHigh);

            const bool needs_reeval =

                    this->recordPrivateDnsValidation(identity, netId, success, isRevalidation);



            if (!needs_reeval) {

                break;
@@ -267,8 +233,6 @@ void PrivateDnsConfiguration::startValidation(const ServerIdentity& identity, un 
                break;

            }

        }



        this->updateServerLatencyThreshold(identity, latencyThreshold, netId);

    });

    validate_thread.detach();

}
@@ -304,8 +268,8 @@ void PrivateDnsConfiguration::sendPrivateDnsValidationEvent(const ServerIdentity 
}



bool PrivateDnsConfiguration::recordPrivateDnsValidation(const ServerIdentity& identity,

                                                         unsigned netId, bool gotAnswer,

                                                         bool isRevalidation, bool latencyTooHigh) {

                                                         unsigned netId, bool success,

                                                         bool isRevalidation) {

    constexpr bool NEEDS_REEVALUATION = true;

    constexpr bool DONT_REEVALUATE = false;
@@ -326,17 +290,14 @@ bool PrivateDnsConfiguration::recordPrivateDnsValidation(const ServerIdentity& i 
    }



    bool reevaluationStatus = NEEDS_REEVALUATION;

    if (gotAnswer) {

        if (!latencyTooHigh) {

    if (success) {

        reevaluationStatus = DONT_REEVALUATE;

        }

    } else if (mode->second == PrivateDnsMode::OFF) {

        reevaluationStatus = DONT_REEVALUATE;

    } else if (mode->second == PrivateDnsMode::OPPORTUNISTIC && !isRevalidation) {

        reevaluationStatus = DONT_REEVALUATE;

    }



    bool success = gotAnswer;

    auto& tracker = netPair->second;

    auto serverPair = tracker.find(identity);

    if (serverPair == tracker.end()) {
@@ -351,12 +312,10 @@ bool PrivateDnsConfiguration::recordPrivateDnsValidation(const ServerIdentity& i 
        reevaluationStatus = DONT_REEVALUATE;

    }



    const bool succeededQuickly = success && !latencyTooHigh;



    // Send private dns validation result to listeners.

    sendPrivateDnsValidationEvent(identity, netId, succeededQuickly);

    sendPrivateDnsValidationEvent(identity, netId, success);



    if (succeededQuickly) {

    if (success) {

        updateServerState(identity, Validation::success, netId);

    } else {

        // Validation failure is expected if a user is on a captive portal.
@@ -366,7 +325,7 @@ bool PrivateDnsConfiguration::recordPrivateDnsValidation(const ServerIdentity& i 
                                                                       : Validation::fail;

        updateServerState(identity, result, netId);

    }

    LOG(WARNING) << "Validation " << (succeededQuickly ? "success" : "failed");

    LOG(WARNING) << "Validation " << (success ? "success" : "failed");



    return reevaluationStatus;

}
@@ -426,24 +385,6 @@ base::Result<IPrivateDnsServer*> PrivateDnsConfiguration::getPrivateDnsLocked( 
    return iter->second.get();

}



void PrivateDnsConfiguration::updateServerLatencyThreshold(const ServerIdentity& identity,

                                                           std::optional<int64_t> latencyThreshold,

                                                           uint32_t netId) {

    std::lock_guard guard(mPrivateDnsLock);



    const auto result = getPrivateDnsLocked(identity, netId);

    if (!result.ok()) return;



    if (result.value()->isDot()) {

        DnsTlsServer& server = *static_cast<DnsTlsServer*>(result.value());

        server.setLatencyThreshold(latencyThreshold);

        LOG(INFO) << "Set latencyThreshold "

                  << (latencyThreshold ? std::to_string(latencyThreshold.value()) + "ms"

                                       : "nullopt")

                  << " to " << server.toIpString();

    }

}



void PrivateDnsConfiguration::setObserver(PrivateDnsValidationObserver* observer) {

    std::lock_guard guard(mPrivateDnsLock);

    mObserver = observer;