Some cleaup in PrivateDnsConfiguration::startValidation
Also rename maybeNotifyObserver to notifyValidationStateUpdate.
No functionality changes.
Bug: 79727473
Test: cd packages/modules/DnsResolver && atest
Change-Id: Icaf31d6d2d425d263e5b8e5289df6bb7f3e863e8
diff --git a/DnsTlsTransport.cpp b/DnsTlsTransport.cpp
index bcbb0a1..a2964cc 100644
--- a/DnsTlsTransport.cpp
+++ b/DnsTlsTransport.cpp
@@ -158,8 +158,8 @@
// static
// TODO: Use this function to preheat the session cache.
// That may require moving it to DnsTlsDispatcher.
-bool DnsTlsTransport::validate(const DnsTlsServer& server, unsigned netid, uint32_t mark) {
- LOG(DEBUG) << "Beginning validation on " << netid;
+bool DnsTlsTransport::validate(const DnsTlsServer& server, uint32_t mark) {
+ LOG(DEBUG) << "Beginning validation with mark " << std::hex << mark;
// Generate "<random>-dnsotls-ds.metric.gstatic.com", which we will lookup through |ss| in
// order to prove that it is actually a working DNS over TLS server.
static const char kDnsSafeChars[] =
@@ -195,7 +195,7 @@
DnsTlsTransport transport(server, mark, &factory);
auto r = transport.query(netdutils::Slice(query, qlen)).get();
if (r.code != Response::success) {
- LOG(DEBUG) << "query failed";
+ LOG(WARNING) << "query failed";
return false;
}
@@ -212,7 +212,7 @@
}
const int ancount = (recvbuf[6] << 8) | recvbuf[7];
- LOG(DEBUG) << netid << " answer count: " << ancount;
+ LOG(DEBUG) << "answer count: " << ancount;
// TODO: Further validate the response contents (check for valid AAAA record, ...).
// Note that currently, integration tests rely on this function accepting a
diff --git a/DnsTlsTransport.h b/DnsTlsTransport.h
index c0fbaef..7b7f81f 100644
--- a/DnsTlsTransport.h
+++ b/DnsTlsTransport.h
@@ -52,10 +52,10 @@
// Given a |query|, this method sends it to the server and returns the result asynchronously.
std::future<Result> query(const netdutils::Slice query) EXCLUDES(mLock);
- // Check that a given TLS server is fully working on the specified netid.
+ // Check that a given TLS server is fully working with a specified mark.
// This function is used in ResolverController to ensure that we don't enable DNS over TLS
// on networks where it doesn't actually work.
- static bool validate(const DnsTlsServer& server, unsigned netid, uint32_t mark);
+ static bool validate(const DnsTlsServer& server, uint32_t mark);
int getConnectCounter() const EXCLUDES(mLock);
diff --git a/PrivateDnsConfiguration.cpp b/PrivateDnsConfiguration.cpp
index 8fbd573..472f1cd 100644
--- a/PrivateDnsConfiguration.cpp
+++ b/PrivateDnsConfiguration.cpp
@@ -107,7 +107,7 @@
if (needsValidation(server)) {
updateServerState(identity, Validation::in_process, netId);
- startValidation(server, netId, mark);
+ startValidation(server, netId);
}
}
@@ -168,14 +168,14 @@
if (target.mark != mark) return false;
updateServerState(identity, Validation::in_process, netId);
- startValidation(target, netId, mark);
+ startValidation(target, netId);
return true;
}
-void PrivateDnsConfiguration::startValidation(const DnsTlsServer& server, unsigned netId,
- uint32_t mark) REQUIRES(mPrivateDnsLock) {
+void PrivateDnsConfiguration::startValidation(const DnsTlsServer& server, unsigned netId)
+ REQUIRES(mPrivateDnsLock) {
// Note that capturing |server| and |netId| in this lambda create copies.
- std::thread validate_thread([this, server, netId, mark] {
+ std::thread validate_thread([this, server, netId] {
setThreadName(StringPrintf("TlsVerify_%u", netId).c_str());
// cat /proc/sys/net/ipv4/tcp_syn_retries yields "6".
@@ -199,10 +199,11 @@
while (true) {
// ::validate() is a blocking call that performs network operations.
// It can take milliseconds to minutes, up to the SYN retry limit.
- LOG(WARNING) << "Validating DnsTlsServer on netId " << netId;
- const bool success = DnsTlsTransport::validate(server, netId, mark);
- LOG(DEBUG) << "validateDnsTlsServer returned " << success << " for "
- << server.toIpString();
+ LOG(WARNING) << "Validating DnsTlsServer " << server.toIpString() << " with mark 0x"
+ << std::hex << server.mark;
+ const bool success = DnsTlsTransport::validate(server, server.mark);
+ LOG(WARNING) << "validateDnsTlsServer returned " << success << " for "
+ << server.toIpString();
const bool needs_reeval = this->recordPrivateDnsValidation(server, netId, success);
if (!needs_reeval) {
@@ -231,14 +232,14 @@
auto netPair = mPrivateDnsTransports.find(netId);
if (netPair == mPrivateDnsTransports.end()) {
LOG(WARNING) << "netId " << netId << " was erased during private DNS validation";
- maybeNotifyObserver(identity.ip.toString(), Validation::fail, netId);
+ notifyValidationStateUpdate(identity.ip.toString(), Validation::fail, netId);
return DONT_REEVALUATE;
}
const auto mode = mPrivateDnsModes.find(netId);
if (mode == mPrivateDnsModes.end()) {
LOG(WARNING) << "netId " << netId << " has no private DNS validation mode";
- maybeNotifyObserver(identity.ip.toString(), Validation::fail, netId);
+ notifyValidationStateUpdate(identity.ip.toString(), Validation::fail, netId);
return DONT_REEVALUATE;
}
const bool modeDoesReevaluation = (mode->second == PrivateDnsMode::STRICT);
@@ -299,18 +300,18 @@
uint32_t netId) {
auto netPair = mPrivateDnsTransports.find(netId);
if (netPair == mPrivateDnsTransports.end()) {
- maybeNotifyObserver(identity.ip.toString(), Validation::fail, netId);
+ notifyValidationStateUpdate(identity.ip.toString(), Validation::fail, netId);
return;
}
auto& tracker = netPair->second;
if (tracker.find(identity) == tracker.end()) {
- maybeNotifyObserver(identity.ip.toString(), Validation::fail, netId);
+ notifyValidationStateUpdate(identity.ip.toString(), Validation::fail, netId);
return;
}
tracker[identity].setValidationState(state);
- maybeNotifyObserver(identity.ip.toString(), state, netId);
+ notifyValidationStateUpdate(identity.ip.toString(), state, netId);
}
bool PrivateDnsConfiguration::needsValidation(const DnsTlsServer& server) {
@@ -334,8 +335,9 @@
mObserver = observer;
}
-void PrivateDnsConfiguration::maybeNotifyObserver(const std::string& serverIp,
- Validation validation, uint32_t netId) const {
+void PrivateDnsConfiguration::notifyValidationStateUpdate(const std::string& serverIp,
+ Validation validation,
+ uint32_t netId) const {
if (mObserver) {
mObserver->onValidationStateUpdate(serverIp, validation, netId);
}
diff --git a/PrivateDnsConfiguration.h b/PrivateDnsConfiguration.h
index 20b4fa2..26169c2 100644
--- a/PrivateDnsConfiguration.h
+++ b/PrivateDnsConfiguration.h
@@ -94,8 +94,7 @@
PrivateDnsConfiguration() = default;
- void startValidation(const DnsTlsServer& server, unsigned netId, uint32_t mark)
- REQUIRES(mPrivateDnsLock);
+ void startValidation(const DnsTlsServer& server, unsigned netId) REQUIRES(mPrivateDnsLock);
bool recordPrivateDnsValidation(const DnsTlsServer& server, unsigned netId, bool success)
EXCLUDES(mPrivateDnsLock);
@@ -131,8 +130,8 @@
};
void setObserver(Observer* observer);
- void maybeNotifyObserver(const std::string& serverIp, Validation validation,
- uint32_t netId) const REQUIRES(mPrivateDnsLock);
+ void notifyValidationStateUpdate(const std::string& serverIp, Validation validation,
+ uint32_t netId) const REQUIRES(mPrivateDnsLock);
Observer* mObserver GUARDED_BY(mPrivateDnsLock);