Loading PrivateDnsConfiguration.cpp +38 −26 Original line number Diff line number Diff line Loading @@ -122,7 +122,27 @@ int PrivateDnsConfiguration::set(int32_t netId, uint32_t mark, // Add any new or changed servers to the tracker, and initiate async checks for them. for (const auto& server : tlsServers) { if (needsValidation(tracker, server)) { validatePrivateDnsProvider(server, tracker, netId, mark); // This is temporarily required. Consider the following scenario, for example, // Step 1) A DoTServer (s1) is set for the network. A validation (v1) for s1 starts. // tracker has s1 alone. // Step 2) The configuration changes. DotServer2 (s2) is set for the network. A // validation (v2) for s2 starts. tracker has s2 alone. // Step 3) Assume v1 and v2 somehow block. Now, if the configuration changes back to // set s1, there won't be a v1' starts because needValidateThread() will // return false. // // If we didn't add servers to tracker before needValidateThread(), tracker would // become empty. We would report s1 validation failed. tracker[server] = Validation::in_process; LOG(DEBUG) << "Server " << addrToString(&server.ss) << " marked as in_process on netId " << netId << ". Tracker now has size " << tracker.size(); // This judge must be after "tracker[server] = Validation::in_process;" if (!needValidateThread(server, netId)) { continue; } updateServerState(server, Validation::in_process, netId); startValidation(server, netId, mark); } } Loading Loading @@ -155,19 +175,8 @@ void PrivateDnsConfiguration::clear(unsigned netId) { mPrivateDnsValidateThreads.erase(netId); } void PrivateDnsConfiguration::validatePrivateDnsProvider(const DnsTlsServer& server, PrivateDnsTracker& tracker, unsigned netId, void PrivateDnsConfiguration::startValidation(const DnsTlsServer& server, unsigned netId, uint32_t mark) REQUIRES(mPrivateDnsLock) { tracker[server] = Validation::in_process; LOG(DEBUG) << "Server " << addrToString(&server.ss) << " marked as in_process on netId " << netId << ". Tracker now has size " << tracker.size(); // This judge must be after "tracker[server] = Validation::in_process;" if (!needValidateThread(server, netId)) { return; } maybeNotifyObserver(server, Validation::in_process, netId); // Note that capturing |server| and |netId| in this lambda create copies. std::thread validate_thread([this, server, netId, mark] { setThreadName(StringPrintf("TlsVerify_%u", netId).c_str()); Loading Loading @@ -273,18 +282,14 @@ bool PrivateDnsConfiguration::recordPrivateDnsValidation(const DnsTlsServer& ser } if (success) { tracker[server] = Validation::success; maybeNotifyObserver(server, Validation::success, netId); updateServerState(server, Validation::success, netId); } else { // Validation failure is expected if a user is on a captive portal. // TODO: Trigger a second validation attempt after captive portal login // succeeds. tracker[server] = (reevaluationStatus == NEEDS_REEVALUATION) ? Validation::in_process const auto result = (reevaluationStatus == NEEDS_REEVALUATION) ? Validation::in_process : Validation::fail; maybeNotifyObserver(server, (reevaluationStatus == NEEDS_REEVALUATION) ? Validation::in_process : Validation::fail, netId); updateServerState(server, result, netId); } LOG(WARNING) << "Validation " << (success ? "success" : "failed"); Loading Loading @@ -319,6 +324,17 @@ bool PrivateDnsConfiguration::needValidateThread(const DnsTlsServer& server, uns } } void PrivateDnsConfiguration::updateServerState(const DnsTlsServer& server, Validation state, uint32_t netId) { auto netPair = mPrivateDnsTransports.find(netId); if (netPair != mPrivateDnsTransports.end()) { auto& tracker = netPair->second; tracker[server] = state; } maybeNotifyObserver(server, state, netId); } void PrivateDnsConfiguration::cleanValidateThreadTracker(const DnsTlsServer& server, unsigned netId) { std::lock_guard<std::mutex> guard(mPrivateDnsLock); Loading @@ -335,10 +351,6 @@ void PrivateDnsConfiguration::cleanValidateThreadTracker(const DnsTlsServer& ser } } // Start validation for newly added servers as well as any servers that have // landed in Validation::fail state. Note that servers that have failed // multiple validation attempts but for which there is still a validating // thread running are marked as being Validation::in_process. bool PrivateDnsConfiguration::needsValidation(const PrivateDnsTracker& tracker, const DnsTlsServer& server) { const auto& iter = tracker.find(server); Loading PrivateDnsConfiguration.h +14 −5 Original line number Diff line number Diff line Loading @@ -71,19 +71,25 @@ class PrivateDnsConfiguration { PrivateDnsConfiguration() = default; void validatePrivateDnsProvider(const DnsTlsServer& server, PrivateDnsTracker& tracker, unsigned netId, uint32_t mark) REQUIRES(mPrivateDnsLock); void startValidation(const DnsTlsServer& server, unsigned netId, uint32_t mark) REQUIRES(mPrivateDnsLock); bool recordPrivateDnsValidation(const DnsTlsServer& server, unsigned netId, bool success); bool recordPrivateDnsValidation(const DnsTlsServer& server, unsigned netId, bool success) EXCLUDES(mPrivateDnsLock); bool needValidateThread(const DnsTlsServer& server, unsigned netId) REQUIRES(mPrivateDnsLock); void cleanValidateThreadTracker(const DnsTlsServer& server, unsigned netId); void cleanValidateThreadTracker(const DnsTlsServer& server, unsigned netId) EXCLUDES(mPrivateDnsLock); // Start validation for newly added servers as well as any servers that have // landed in Validation::fail state. Note that servers that have failed // multiple validation attempts but for which there is still a validating // thread running are marked as being Validation::in_process. bool needsValidation(const PrivateDnsTracker& tracker, const DnsTlsServer& server); bool needsValidation(const PrivateDnsTracker& tracker, const DnsTlsServer& server) REQUIRES(mPrivateDnsLock); void updateServerState(const DnsTlsServer& server, Validation state, uint32_t netId) REQUIRES(mPrivateDnsLock); std::mutex mPrivateDnsLock; std::map<unsigned, PrivateDnsMode> mPrivateDnsModes GUARDED_BY(mPrivateDnsLock); Loading @@ -94,6 +100,9 @@ class PrivateDnsConfiguration { // For testing. The observer is notified of onValidationStateUpdate 1) when a validation is // about to begin or 2) when a validation finishes. // WARNING: The Observer is notified while the lock is being held. Be careful not to call any // method of PrivateDnsConfiguration from the observer. // TODO: fix the reentrancy problem. class Observer { public: virtual ~Observer(){}; Loading Loading
PrivateDnsConfiguration.cpp +38 −26 Original line number Diff line number Diff line Loading @@ -122,7 +122,27 @@ int PrivateDnsConfiguration::set(int32_t netId, uint32_t mark, // Add any new or changed servers to the tracker, and initiate async checks for them. for (const auto& server : tlsServers) { if (needsValidation(tracker, server)) { validatePrivateDnsProvider(server, tracker, netId, mark); // This is temporarily required. Consider the following scenario, for example, // Step 1) A DoTServer (s1) is set for the network. A validation (v1) for s1 starts. // tracker has s1 alone. // Step 2) The configuration changes. DotServer2 (s2) is set for the network. A // validation (v2) for s2 starts. tracker has s2 alone. // Step 3) Assume v1 and v2 somehow block. Now, if the configuration changes back to // set s1, there won't be a v1' starts because needValidateThread() will // return false. // // If we didn't add servers to tracker before needValidateThread(), tracker would // become empty. We would report s1 validation failed. tracker[server] = Validation::in_process; LOG(DEBUG) << "Server " << addrToString(&server.ss) << " marked as in_process on netId " << netId << ". Tracker now has size " << tracker.size(); // This judge must be after "tracker[server] = Validation::in_process;" if (!needValidateThread(server, netId)) { continue; } updateServerState(server, Validation::in_process, netId); startValidation(server, netId, mark); } } Loading Loading @@ -155,19 +175,8 @@ void PrivateDnsConfiguration::clear(unsigned netId) { mPrivateDnsValidateThreads.erase(netId); } void PrivateDnsConfiguration::validatePrivateDnsProvider(const DnsTlsServer& server, PrivateDnsTracker& tracker, unsigned netId, void PrivateDnsConfiguration::startValidation(const DnsTlsServer& server, unsigned netId, uint32_t mark) REQUIRES(mPrivateDnsLock) { tracker[server] = Validation::in_process; LOG(DEBUG) << "Server " << addrToString(&server.ss) << " marked as in_process on netId " << netId << ". Tracker now has size " << tracker.size(); // This judge must be after "tracker[server] = Validation::in_process;" if (!needValidateThread(server, netId)) { return; } maybeNotifyObserver(server, Validation::in_process, netId); // Note that capturing |server| and |netId| in this lambda create copies. std::thread validate_thread([this, server, netId, mark] { setThreadName(StringPrintf("TlsVerify_%u", netId).c_str()); Loading Loading @@ -273,18 +282,14 @@ bool PrivateDnsConfiguration::recordPrivateDnsValidation(const DnsTlsServer& ser } if (success) { tracker[server] = Validation::success; maybeNotifyObserver(server, Validation::success, netId); updateServerState(server, Validation::success, netId); } else { // Validation failure is expected if a user is on a captive portal. // TODO: Trigger a second validation attempt after captive portal login // succeeds. tracker[server] = (reevaluationStatus == NEEDS_REEVALUATION) ? Validation::in_process const auto result = (reevaluationStatus == NEEDS_REEVALUATION) ? Validation::in_process : Validation::fail; maybeNotifyObserver(server, (reevaluationStatus == NEEDS_REEVALUATION) ? Validation::in_process : Validation::fail, netId); updateServerState(server, result, netId); } LOG(WARNING) << "Validation " << (success ? "success" : "failed"); Loading Loading @@ -319,6 +324,17 @@ bool PrivateDnsConfiguration::needValidateThread(const DnsTlsServer& server, uns } } void PrivateDnsConfiguration::updateServerState(const DnsTlsServer& server, Validation state, uint32_t netId) { auto netPair = mPrivateDnsTransports.find(netId); if (netPair != mPrivateDnsTransports.end()) { auto& tracker = netPair->second; tracker[server] = state; } maybeNotifyObserver(server, state, netId); } void PrivateDnsConfiguration::cleanValidateThreadTracker(const DnsTlsServer& server, unsigned netId) { std::lock_guard<std::mutex> guard(mPrivateDnsLock); Loading @@ -335,10 +351,6 @@ void PrivateDnsConfiguration::cleanValidateThreadTracker(const DnsTlsServer& ser } } // Start validation for newly added servers as well as any servers that have // landed in Validation::fail state. Note that servers that have failed // multiple validation attempts but for which there is still a validating // thread running are marked as being Validation::in_process. bool PrivateDnsConfiguration::needsValidation(const PrivateDnsTracker& tracker, const DnsTlsServer& server) { const auto& iter = tracker.find(server); Loading
PrivateDnsConfiguration.h +14 −5 Original line number Diff line number Diff line Loading @@ -71,19 +71,25 @@ class PrivateDnsConfiguration { PrivateDnsConfiguration() = default; void validatePrivateDnsProvider(const DnsTlsServer& server, PrivateDnsTracker& tracker, unsigned netId, uint32_t mark) REQUIRES(mPrivateDnsLock); void startValidation(const DnsTlsServer& server, unsigned netId, uint32_t mark) REQUIRES(mPrivateDnsLock); bool recordPrivateDnsValidation(const DnsTlsServer& server, unsigned netId, bool success); bool recordPrivateDnsValidation(const DnsTlsServer& server, unsigned netId, bool success) EXCLUDES(mPrivateDnsLock); bool needValidateThread(const DnsTlsServer& server, unsigned netId) REQUIRES(mPrivateDnsLock); void cleanValidateThreadTracker(const DnsTlsServer& server, unsigned netId); void cleanValidateThreadTracker(const DnsTlsServer& server, unsigned netId) EXCLUDES(mPrivateDnsLock); // Start validation for newly added servers as well as any servers that have // landed in Validation::fail state. Note that servers that have failed // multiple validation attempts but for which there is still a validating // thread running are marked as being Validation::in_process. bool needsValidation(const PrivateDnsTracker& tracker, const DnsTlsServer& server); bool needsValidation(const PrivateDnsTracker& tracker, const DnsTlsServer& server) REQUIRES(mPrivateDnsLock); void updateServerState(const DnsTlsServer& server, Validation state, uint32_t netId) REQUIRES(mPrivateDnsLock); std::mutex mPrivateDnsLock; std::map<unsigned, PrivateDnsMode> mPrivateDnsModes GUARDED_BY(mPrivateDnsLock); Loading @@ -94,6 +100,9 @@ class PrivateDnsConfiguration { // For testing. The observer is notified of onValidationStateUpdate 1) when a validation is // about to begin or 2) when a validation finishes. // WARNING: The Observer is notified while the lock is being held. Be careful not to call any // method of PrivateDnsConfiguration from the observer. // TODO: fix the reentrancy problem. class Observer { public: virtual ~Observer(){}; Loading