Run private DNS validation asynchronously To allow private DNS validation to be restarted while the probes are in-flight, do not block the handler thread, but instead run it on background threads. Test: atest Bug: 253698734 Change-Id: I462f891ba18ce09e9d71fba9c62cbba53dd83cd9
diff --git a/src/com/android/networkstack/util/NetworkStackUtils.java b/src/com/android/networkstack/util/NetworkStackUtils.java index 85da400..874b1bd 100755 --- a/src/com/android/networkstack/util/NetworkStackUtils.java +++ b/src/com/android/networkstack/util/NetworkStackUtils.java
@@ -256,6 +256,12 @@ public static final String IPCLIENT_IGNORE_LOW_RA_LIFETIME_VERSION = "ipclient_ignore_low_ra_lifetime_version"; + /** + * Feature flag to send private DNS resolution queries and probes on a background thread. + */ + public static final String NETWORKMONITOR_ASYNC_PRIVDNS_RESOLUTION = + "networkmonitor_async_privdns_resolution"; + /**** BEGIN Feature Kill Switch Flags ****/ /**
diff --git a/src/com/android/server/connectivity/NetworkMonitor.java b/src/com/android/server/connectivity/NetworkMonitor.java index 9303f95..78a47ea 100755 --- a/src/com/android/server/connectivity/NetworkMonitor.java +++ b/src/com/android/server/connectivity/NetworkMonitor.java
@@ -23,6 +23,9 @@ import static android.net.ConnectivityManager.EXTRA_CAPTIVE_PORTAL_PROBE_SPEC; import static android.net.ConnectivityManager.EXTRA_CAPTIVE_PORTAL_URL; import static android.net.DnsResolver.FLAG_EMPTY; +import static android.net.DnsResolver.FLAG_NO_CACHE_LOOKUP; +import static android.net.DnsResolver.TYPE_A; +import static android.net.DnsResolver.TYPE_AAAA; import static android.net.INetworkMonitor.NETWORK_TEST_RESULT_INVALID; import static android.net.INetworkMonitor.NETWORK_TEST_RESULT_PARTIAL_CONNECTIVITY; import static android.net.INetworkMonitor.NETWORK_TEST_RESULT_VALID; @@ -122,6 +125,7 @@ import android.net.wifi.WifiManager; import android.os.Build; import android.os.Bundle; +import android.os.CancellationSignal; import android.os.Message; import android.os.Process; import android.os.RemoteException; @@ -140,7 +144,9 @@ import android.telephony.SignalStrength; import android.telephony.TelephonyManager; import android.text.TextUtils; +import android.util.ArraySet; import android.util.Log; +import android.util.Pair; import android.util.SparseArray; import androidx.annotation.ArrayRes; @@ -234,6 +240,10 @@ @VisibleForTesting static final String CONFIG_CAPTIVE_PORTAL_DNS_PROBE_TIMEOUT = "captive_portal_dns_probe_timeout"; + @VisibleForTesting + static final String CONFIG_ASYNC_PRIVDNS_PROBE_TIMEOUT_MS = + "async_privdns_probe_timeout_ms"; + private static final int DEFAULT_PRIVDNS_PROBE_TIMEOUT_MS = 10_000; private static final int SOCKET_TIMEOUT_MS = 10000; private static final int PROBE_TIMEOUT_MS = 3000; @@ -404,6 +414,32 @@ */ private static final int EVENT_RESOURCE_CONFIG_CHANGED = 25; + /** + * Message to self to notify that private DNS strict mode hostname resolution has finished. + * + * <p>arg2 = Last DNS rcode. + * obj = Pair<List<InetAddress>, DnsCallback>: query results and DnsCallback used. + */ + private static final int CMD_STRICT_MODE_RESOLUTION_COMPLETED = 26; + + /** + * Message to self to notify that the private DNS probe has finished. + * + * <p>arg2 = Last DNS rcode. + * obj = Pair<List<InetAddress>, DnsCallback>: query results and DnsCallback used. + */ + private static final int CMD_PRIVATE_DNS_PROBE_COMPLETED = 27; + + /** + * Message to self to notify that private DNS hostname resolution or probing has failed. + */ + private static final int CMD_PRIVATE_DNS_EVALUATION_FAILED = 28; + + /** + * Message to self to notify that a DNS query has timed out. + */ + private static final int CMD_DNS_TIMEOUT = 29; + // Start mReevaluateDelayMs at this value and double. @VisibleForTesting static final int INITIAL_REEVALUATE_DELAY_MS = 1000; @@ -504,6 +540,10 @@ private final State mEvaluatingState = new EvaluatingState(); private final State mCaptivePortalState = new CaptivePortalState(); private final State mEvaluatingPrivateDnsState = new EvaluatingPrivateDnsState(); + private final State mStartingPrivateDnsEvaluation = new StartingPrivateDnsEvaluation(); + private final State mResolvingPrivateDnsState = new ResolvingPrivateDnsState(); + private final State mProbingForPrivateDnsState = new ProbingForPrivateDnsState(); + private final State mProbingState = new ProbingState(); private final State mWaitingForNextProbeState = new WaitingForNextProbeState(); private final State mEvaluatingBandwidthState = new EvaluatingBandwidthState(); @@ -544,6 +584,8 @@ private final boolean mMetricsEnabled; private final boolean mReevaluateWhenResumeEnabled; + private final boolean mAsyncPrivdnsResolutionEnabled; + @NonNull private final NetworkInformationShim mInfoShim = NetworkInformationShimImpl.newInstance(); @@ -614,6 +656,9 @@ addState(mWaitingForNextProbeState, mEvaluatingState); addState(mCaptivePortalState, mMaybeNotifyState); addState(mEvaluatingPrivateDnsState, mDefaultState); + addState(mStartingPrivateDnsEvaluation, mEvaluatingPrivateDnsState); + addState(mResolvingPrivateDnsState, mEvaluatingPrivateDnsState); + addState(mProbingForPrivateDnsState, mEvaluatingPrivateDnsState); addState(mEvaluatingBandwidthState, mDefaultState); addState(mValidatedState, mDefaultState); setInitialState(mDefaultState); @@ -632,6 +677,8 @@ NetworkStackUtils.VALIDATION_METRICS_VERSION); mReevaluateWhenResumeEnabled = deps.isFeatureEnabled( context, NetworkStackUtils.REEVALUATE_WHEN_RESUME); + mAsyncPrivdnsResolutionEnabled = deps.isFeatureEnabled(context, + NetworkStackUtils.NETWORKMONITOR_ASYNC_PRIVDNS_RESOLUTION); mUseHttps = getUseHttpsValidation(); mCaptivePortalUserAgent = getCaptivePortalUserAgent(); mCaptivePortalFallbackSpecs = @@ -1049,6 +1096,13 @@ if (tst != null) { tst.setOpportunisticMode(cfg.inOpportunisticMode()); } + if (mAsyncPrivdnsResolutionEnabled) { + // When using async privdns validation, reevaluate on any change of + // configuration (even if turning it off), as this will handle + // cancelling current attempts and transitioning to validated state. + removeMessages(CMD_EVALUATE_PRIVATE_DNS); + sendMessage(CMD_EVALUATE_PRIVATE_DNS); + } break; } @@ -1599,13 +1653,28 @@ @Override public boolean processMessage(Message msg) { switch (msg.what) { - case CMD_EVALUATE_PRIVATE_DNS: + case CMD_EVALUATE_PRIVATE_DNS: { + if (mAsyncPrivdnsResolutionEnabled) { + // Cancel any previously scheduled retry attempt + removeMessages(CMD_EVALUATE_PRIVATE_DNS); + + if (inStrictMode()) { + // Note this may happen even in the case where the current state is + // resolve or probe: private DNS evaluation would then restart. + transitionTo(mStartingPrivateDnsEvaluation); + } else { + mEvaluationState.removeProbeResult(NETWORK_VALIDATION_PROBE_PRIVDNS); + transitionToPrivateDnsEvaluationSuccessState(); + } + break; + } + if (inStrictMode()) { - if (!isStrictModeHostnameResolved()) { + if (!isStrictModeHostnameResolved(mPrivateDnsConfig)) { resolveStrictModeHostname(); - if (isStrictModeHostnameResolved()) { - notifyPrivateDnsConfigResolved(); + if (isStrictModeHostnameResolved(mPrivateDnsConfig)) { + notifyPrivateDnsConfigResolved(mPrivateDnsConfig); } else { handlePrivateDnsEvaluationFailure(); // The private DNS probe fails-fast if the server hostname cannot @@ -1630,23 +1699,24 @@ handlePrivateDnsEvaluationFailure(); break; } - handlePrivateDnsEvaluationSuccess(); + mEvaluationState.noteProbeResult(NETWORK_VALIDATION_PROBE_PRIVDNS, + true /* succeeded */); } else { mEvaluationState.removeProbeResult(NETWORK_VALIDATION_PROBE_PRIVDNS); } - - if (needEvaluatingBandwidth()) { - transitionTo(mEvaluatingBandwidthState); - } else { - // All good! - transitionTo(mValidatedState); - } + transitionToPrivateDnsEvaluationSuccessState(); break; - case CMD_PRIVATE_DNS_SETTINGS_CHANGED: + } + case CMD_PRIVATE_DNS_SETTINGS_CHANGED: { // When settings change the reevaluation timer must be reset. mPrivateDnsReevalDelayMs = INITIAL_REEVALUATE_DELAY_MS; // Let the message bubble up and be handled by parent states as usual. return NOT_HANDLED; + } + // Only used with mAsyncPrivdnsResolutionEnabled + case CMD_PRIVATE_DNS_EVALUATION_FAILED: { + reschedulePrivateDnsEvaluation(); + } default: return NOT_HANDLED; } @@ -1657,12 +1727,6 @@ return !TextUtils.isEmpty(mPrivateDnsProviderHostname); } - private boolean isStrictModeHostnameResolved() { - return (mPrivateDnsConfig != null) - && mPrivateDnsConfig.hostname.equals(mPrivateDnsProviderHostname) - && (mPrivateDnsConfig.ips.length > 0); - } - private void resolveStrictModeHostname() { try { // Do a blocking DNS resolution using the network-assigned nameservers. @@ -1675,24 +1739,15 @@ } } - private void notifyPrivateDnsConfigResolved() { - try { - mCallback.notifyPrivateDnsConfigResolved(mPrivateDnsConfig.toParcel()); - } catch (RemoteException e) { - Log.e(TAG, "Error sending private DNS config resolved notification", e); - } - } - - private void handlePrivateDnsEvaluationSuccess() { - mEvaluationState.noteProbeResult(NETWORK_VALIDATION_PROBE_PRIVDNS, - true /* succeeded */); - } - private void handlePrivateDnsEvaluationFailure() { mEvaluationState.noteProbeResult(NETWORK_VALIDATION_PROBE_PRIVDNS, false /* succeeded */); mEvaluationState.reportEvaluationResult(NETWORK_VALIDATION_RESULT_INVALID, null /* redirectUrl */); + reschedulePrivateDnsEvaluation(); + } + + private void reschedulePrivateDnsEvaluation() { // Queue up a re-evaluation with backoff. // // TODO: Consider abandoning this state after a few attempts and @@ -1730,6 +1785,285 @@ } } + private void transitionToPrivateDnsEvaluationSuccessState() { + if (needEvaluatingBandwidth()) { + transitionTo(mEvaluatingBandwidthState); + } else { + // All good! + transitionTo(mValidatedState); + } + } + + private class StartingPrivateDnsEvaluation extends State { + @Override + public void enter() { + transitionTo(mResolvingPrivateDnsState); + } + } + + private class DnsCallback implements DnsResolver.Callback<List<InetAddress>> { + private final int mReplyMessage; + final CancellationSignal mCancellationSignal; + final boolean mHighPriorityResults; + + DnsCallback(int replyMessage, boolean highPriorityResults) { + mReplyMessage = replyMessage; + mCancellationSignal = new CancellationSignal(); + mHighPriorityResults = highPriorityResults; + } + + @Override + public void onAnswer(List<InetAddress> answer, int rcode) { + sendMessage(mReplyMessage, 0, rcode, new Pair<>(answer, this)); + } + + @Override + public void onError(DnsResolver.DnsException error) { + sendMessage(mReplyMessage, 0, error.code, new Pair<>(null, this)); + } + } + + /** + * Base class for a state that is sending a DNS query, cancelled if the state is exited. + */ + private abstract class DnsQueryState extends State { + private static final int ERROR_TIMEOUT = -1; + private final int mCompletedCommand; + private final ArraySet<DnsCallback> mPendingQueries = new ArraySet<>(2); + private final List<InetAddress> mResults = new ArrayList<>(); + private String mQueryName; + private long mStartTime; + + private DnsQueryState(int completedCommand) { + mCompletedCommand = completedCommand; + } + + @Override + public void enter() { + mPendingQueries.clear(); + mResults.clear(); + mStartTime = SystemClock.elapsedRealtimeNanos(); + + mQueryName = getQueryName(); + if (TextUtils.isEmpty(mQueryName)) { + // No query necessary (in particular not in strict mode): skip DNS query states + mEvaluationState.removeProbeResult(NETWORK_VALIDATION_PROBE_PRIVDNS); + transitionToPrivateDnsEvaluationSuccessState(); + return; + } + + final DnsResolver resolver = mDependencies.getDnsResolver(); + mPendingQueries.addAll(sendQueries(mQueryName, resolver)); + sendMessageDelayed(CMD_DNS_TIMEOUT, getTimeoutMs()); + } + + @Override + public void exit() { + removeMessages(CMD_DNS_TIMEOUT); + cancelAllQueries(); + } + + @Override + public boolean processMessage(Message msg) { + if (msg.what == mCompletedCommand) { + final Pair<List<InetAddress>, DnsCallback> result = + (Pair<List<InetAddress>, DnsCallback>) msg.obj; + if (!mPendingQueries.remove(result.second)) { + // Ignore previous queries if the state was exited and re-entered. This state + // calls cancelAllQueries on exit, but this may still happen if results were + // already posted when the querier processed the cancel request. + return HANDLED; + } + + if (result.first != null) { + if (result.second.mHighPriorityResults) { + mResults.addAll(0, result.first); + } else { + mResults.addAll(result.first); + } + } + + if (mPendingQueries.isEmpty()) { + removeMessages(CMD_DNS_TIMEOUT); + final long time = SystemClock.elapsedRealtimeNanos() - mStartTime; + onQueryDone(mQueryName, mResults, msg.arg2 /* lastRCode */, time); + } + return HANDLED; + } else if (msg.what == CMD_DNS_TIMEOUT) { + cancelAllQueries(); + // If some queries were successful, onQueryDone will still proceed, even if + // lastRCode is not a success code. + onQueryDone(mQueryName, mResults, ERROR_TIMEOUT /* lastRCode */, + SystemClock.elapsedRealtimeNanos() - mStartTime); + return HANDLED; + } + return NOT_HANDLED; + } + + private void cancelAllQueries() { + for (int i = 0; i < mPendingQueries.size(); i++) { + mPendingQueries.valueAt(i).mCancellationSignal.cancel(); + } + mPendingQueries.clear(); + } + + abstract void onQueryDone(@NonNull String queryName, @NonNull List<InetAddress> answer, + int lastRCode, long elapsedNanos); + + @NonNull + abstract String getQueryName(); + + abstract List<DnsCallback> sendQueries(@NonNull String queryName, + @NonNull DnsResolver resolver); + + abstract long getTimeoutMs(); + } + + private class ResolvingPrivateDnsState extends DnsQueryState { + private ResolvingPrivateDnsState() { + super(CMD_STRICT_MODE_RESOLUTION_COMPLETED); + } + + @Override + List<DnsCallback> sendQueries(@NonNull String queryName, @NonNull DnsResolver resolver) { + // Follow legacy behavior that sent AAAA and A queries synchronously in sequence: AAAA + // is marked as highPriorityResults, so they are placed first in the resulting list. + final DnsCallback v6Cb = new DnsCallback(CMD_STRICT_MODE_RESOLUTION_COMPLETED, + true /* highPriorityResults */); + final DnsCallback v4Cb = new DnsCallback(CMD_STRICT_MODE_RESOLUTION_COMPLETED, + false /* highPriorityResults */); + + resolver.query(mCleartextDnsNetwork, queryName, TYPE_AAAA, FLAG_NO_CACHE_LOOKUP, + Runnable::run, v6Cb.mCancellationSignal, v6Cb); + resolver.query(mCleartextDnsNetwork, queryName, TYPE_A, FLAG_NO_CACHE_LOOKUP, + Runnable::run, v4Cb.mCancellationSignal, v4Cb); + + return List.of(v6Cb, v4Cb); + } + + @Override + void onQueryDone(@NonNull String queryName, @NonNull List<InetAddress> answer, + int lastRCode, long elapsedNanos) { + if (!Objects.equals(queryName, mPrivateDnsProviderHostname)) { + validationLog("Ignoring stale private DNS resolve answers for " + queryName + + " (now \"" + mPrivateDnsProviderHostname + "\"): " + answer); + // This may happen if mPrivateDnsProviderHostname was changed, in which case + // reevaluation must have been queued (CMD_EVALUATE_PRIVATE_DNS), but results for + // the first evaluation are received before the reevaluation command gets processed. + // Just ignore the results and wait for reevaluation to be processed. + // More generally, reevaluation is scheduled every time the hostname changes, so + // IP addresses matching the hostname are eventually received, but intermediate + // results should be ignored to avoid reporting a PrivateDnsConfig with IP addresses + // that don't match mPrivateDnsProviderHostname. + return; + } + + if (!answer.isEmpty()) { + final InetAddress[] ips = answer.toArray(new InetAddress[0]); + final PrivateDnsConfig config = + new PrivateDnsConfig(mPrivateDnsProviderHostname, ips); + notifyPrivateDnsConfigResolved(config); + + validationLog("Strict mode hostname resolution " + elapsedNanos + "ns OK " + + answer + " for " + mPrivateDnsProviderHostname); + transitionTo(mProbingForPrivateDnsState); + } else { + mEvaluationState.noteProbeResult(NETWORK_VALIDATION_PROBE_PRIVDNS, + false /* succeeded */); + mEvaluationState.reportEvaluationResult(NETWORK_VALIDATION_RESULT_INVALID, + null /* redirectUrl */); + + validationLog("Strict mode hostname resolution " + elapsedNanos + "ns FAIL " + + "lastRCode " + lastRCode + " for " + mPrivateDnsProviderHostname); + sendMessage(CMD_PRIVATE_DNS_EVALUATION_FAILED); + + // The private DNS probe fails-fast if the server hostname cannot + // be resolved. Record it as a failure with zero latency. + recordProbeEventMetrics(ProbeType.PT_PRIVDNS, 0 /* latency */, + ProbeResult.PR_FAILURE, null /* capportData */); + } + } + + @NonNull + @Override + String getQueryName() { + return mPrivateDnsProviderHostname; + } + + @Override + long getTimeoutMs() { + return getDnsProbeTimeout(); + } + } + + private class ProbingForPrivateDnsState extends DnsQueryState { + private ProbingForPrivateDnsState() { + super(CMD_PRIVATE_DNS_PROBE_COMPLETED); + } + + @Override + public void enter() { + super.enter(); + } + + @Override + List<DnsCallback> sendQueries(@NonNull String queryName, @NonNull DnsResolver resolver) { + final DnsCallback cb = new DnsCallback(CMD_PRIVATE_DNS_PROBE_COMPLETED, + false /* highPriorityResults */); + resolver.query(mNetwork, queryName, FLAG_EMPTY, Runnable::run, cb.mCancellationSignal, + cb); + return Collections.singletonList(cb); + } + + @Override + void onQueryDone(@NonNull String queryName, @NonNull List<InetAddress> answer, + int lastRCode, long elapsedNanos) { + final boolean success = !answer.isEmpty(); + recordProbeEventMetrics(ProbeType.PT_PRIVDNS, elapsedNanos, + success ? ProbeResult.PR_SUCCESS : + ProbeResult.PR_FAILURE, null /* capportData */); + logValidationProbe(elapsedNanos, PROBE_PRIVDNS, success ? DNS_SUCCESS : DNS_FAILURE); + + final String strIps = Objects.toString(answer); + validationLog(PROBE_PRIVDNS, queryName, + String.format("%dus: %s", elapsedNanos / 1000, strIps)); + + mEvaluationState.noteProbeResult(NETWORK_VALIDATION_PROBE_PRIVDNS, success); + if (success) { + transitionToPrivateDnsEvaluationSuccessState(); + } else { + mEvaluationState.reportEvaluationResult(NETWORK_VALIDATION_RESULT_INVALID, + null /* redirectUrl */); + sendMessage(CMD_PRIVATE_DNS_EVALUATION_FAILED); + } + } + + @Override + long getTimeoutMs() { + return getAsyncPrivateDnsProbeTimeout(); + } + + @NonNull + @Override + String getQueryName() { + return UUID.randomUUID().toString().substring(0, 8) + PRIVATE_DNS_PROBE_HOST_SUFFIX; + } + } + + private boolean isStrictModeHostnameResolved(PrivateDnsConfig config) { + return (config != null) + && config.hostname.equals(mPrivateDnsProviderHostname) + && (config.ips.length > 0); + } + + private void notifyPrivateDnsConfigResolved(@NonNull PrivateDnsConfig config) { + try { + mCallback.notifyPrivateDnsConfigResolved(config.toParcel()); + } catch (RemoteException e) { + Log.e(TAG, "Error sending private DNS config resolved notification", e); + } + } + private class ProbingState extends State { private Thread mThread; @@ -2186,6 +2520,11 @@ CONFIG_CAPTIVE_PORTAL_DNS_PROBE_TIMEOUT, DEFAULT_CAPTIVE_PORTAL_DNS_PROBE_TIMEOUT); } + private int getAsyncPrivateDnsProbeTimeout() { + return mDependencies.getDeviceConfigPropertyInt(NAMESPACE_CONNECTIVITY, + CONFIG_ASYNC_PRIVDNS_PROBE_TIMEOUT_MS, DEFAULT_PRIVDNS_PROBE_TIMEOUT_MS); + } + /** * Gets an integer setting from resources or device config *
diff --git a/tests/unit/src/com/android/server/connectivity/NetworkMonitorTest.java b/tests/unit/src/com/android/server/connectivity/NetworkMonitorTest.java index 77e3a12..43bee55 100644 --- a/tests/unit/src/com/android/server/connectivity/NetworkMonitorTest.java +++ b/tests/unit/src/com/android/server/connectivity/NetworkMonitorTest.java
@@ -19,6 +19,7 @@ import static android.content.Intent.ACTION_CONFIGURATION_CHANGED; import static android.net.CaptivePortal.APP_RETURN_DISMISSED; import static android.net.CaptivePortal.APP_RETURN_WANTED_AS_IS; +import static android.net.ConnectivitySettingsManager.PRIVATE_DNS_MODE_PROVIDER_HOSTNAME; import static android.net.DnsResolver.TYPE_A; import static android.net.DnsResolver.TYPE_AAAA; import static android.net.INetworkMonitor.NETWORK_VALIDATION_PROBE_DNS; @@ -29,6 +30,7 @@ import static android.net.INetworkMonitor.NETWORK_VALIDATION_RESULT_PARTIAL; import static android.net.INetworkMonitor.NETWORK_VALIDATION_RESULT_SKIPPED; import static android.net.INetworkMonitor.NETWORK_VALIDATION_RESULT_VALID; +import static android.net.InetAddresses.parseNumericAddress; import static android.net.NetworkCapabilities.NET_CAPABILITY_INTERNET; import static android.net.NetworkCapabilities.NET_CAPABILITY_NOT_METERED; import static android.net.NetworkCapabilities.NET_CAPABILITY_NOT_RESTRICTED; @@ -64,6 +66,7 @@ import static com.android.networkstack.util.NetworkStackUtils.DEFAULT_CAPTIVE_PORTAL_DNS_PROBE_TIMEOUT; import static com.android.networkstack.util.NetworkStackUtils.DNS_PROBE_PRIVATE_IP_NO_INTERNET_VERSION; import static com.android.networkstack.util.NetworkStackUtils.REEVALUATE_WHEN_RESUME; +import static com.android.server.connectivity.NetworkMonitor.CONFIG_ASYNC_PRIVDNS_PROBE_TIMEOUT_MS; import static com.android.server.connectivity.NetworkMonitor.INITIAL_REEVALUATE_DELAY_MS; import static com.android.server.connectivity.NetworkMonitor.extractCharset; @@ -123,6 +126,7 @@ import android.net.NetworkAgentConfig; import android.net.NetworkCapabilities; import android.net.NetworkTestResultParcelable; +import android.net.PrivateDnsConfigParcel; import android.net.Uri; import android.net.captiveportal.CaptivePortalProbeResult; import android.net.metrics.IpConnectivityLog; @@ -168,6 +172,7 @@ import com.android.networkstack.metrics.DataStallDetectionStats; import com.android.networkstack.metrics.DataStallStatsUtils; import com.android.networkstack.netlink.TcpSocketTracker; +import com.android.networkstack.util.NetworkStackUtils; import com.android.server.NetworkStackService.NetworkStackServiceManager; import com.android.server.connectivity.nano.CellularData; import com.android.server.connectivity.nano.DnsEvent; @@ -214,8 +219,12 @@ import java.util.Map; import java.util.Objects; import java.util.Random; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutionException; import java.util.concurrent.Executor; import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; import javax.net.ssl.SSLHandshakeException; @@ -366,11 +375,11 @@ class DnsEntry { final String mHostname; final int mType; - final List<InetAddress> mAddresses; - DnsEntry(String host, int type, List<InetAddress> addr) { + final AddressSupplier mAddressesSupplier; + DnsEntry(String host, int type, AddressSupplier addr) { mHostname = host; mType = type; - mAddresses = addr; + mAddressesSupplier = addr; } // Full match or partial match that target host contains the entry hostname to support // random private dns probe hostname. @@ -378,6 +387,21 @@ return hostname.endsWith(mHostname) && type == mType; } } + interface AddressSupplier { + List<InetAddress> get() throws DnsResolver.DnsException; + } + + class InstantAddressSupplier implements AddressSupplier { + private final List<InetAddress> mAddresses; + InstantAddressSupplier(List<InetAddress> addresses) { + mAddresses = addresses; + } + @Override + public List<InetAddress> get() { + return mAddresses; + } + } + private final ArrayList<DnsEntry> mAnswers = new ArrayList<DnsEntry>(); private boolean mNonBypassPrivateDnsWorking = true; @@ -387,40 +411,76 @@ } /** Clears all DNS entries. */ - private synchronized void clearAll() { - mAnswers.clear(); + private void clearAll() { + synchronized (mAnswers) { + mAnswers.clear(); + } } /** Returns the answer for a given name and type on the given mock network. */ - private synchronized List<InetAddress> getAnswer(Object mock, String hostname, int type) { - if (mock == mNetwork && !mNonBypassPrivateDnsWorking) { - return null; + private CompletableFuture<List<InetAddress>> getAnswer(Network mockNetwork, String hostname, + int type) { + if (mockNetwork == mNetwork && !mNonBypassPrivateDnsWorking) { + return CompletableFuture.completedFuture(null); } - return mAnswers.stream().filter(e -> e.matches(hostname, type)) - .map(answer -> answer.mAddresses).findFirst().orElse(null); + final AddressSupplier answerSupplier; + + synchronized (mAnswers) { + answerSupplier = mAnswers.stream() + .filter(e -> e.matches(hostname, type)) + .map(answer -> answer.mAddressesSupplier).findFirst().orElse(null); + } + if (answerSupplier == null) { + return CompletableFuture.completedFuture(null); + } + + if (answerSupplier instanceof InstantAddressSupplier) { + // Save latency waiting for a query thread if the answer is hardcoded. + return CompletableFuture.completedFuture( + ((InstantAddressSupplier) answerSupplier).get()); + } + final CompletableFuture<List<InetAddress>> answerFuture = new CompletableFuture<>(); + new Thread(() -> { + try { + answerFuture.complete(answerSupplier.get()); + } catch (DnsResolver.DnsException e) { + answerFuture.completeExceptionally(e); + } + }).start(); + return answerFuture; } /** Sets the answer for a given name and type. */ - private synchronized void setAnswer(String hostname, String[] answer, int type) - throws UnknownHostException { - DnsEntry record = new DnsEntry(hostname, type, generateAnswer(answer)); - // Remove the existing one. - mAnswers.removeIf(entry -> entry.matches(hostname, type)); - // Add or replace a new record. - mAnswers.add(record); + private void setAnswer(String hostname, String[] answer, int type) { + setAnswer(hostname, new InstantAddressSupplier(generateAnswer(answer)), type); + } + + private void setAnswer(String hostname, AddressSupplier answerSupplier, int type) { + DnsEntry record = new DnsEntry(hostname, type, answerSupplier); + synchronized (mAnswers) { + // Remove the existing one. + mAnswers.removeIf(entry -> entry.matches(hostname, type)); + // Add or replace a new record. + mAnswers.add(record); + } } private List<InetAddress> generateAnswer(String[] answer) { if (answer == null) return new ArrayList<>(); - return Arrays.stream(answer).map(addr -> InetAddress.parseNumericAddress(addr)) - .collect(toList()); + return Arrays.stream(answer).map(InetAddresses::parseNumericAddress).collect(toList()); } /** Simulates a getAllByName call for the specified name on the specified mock network. */ - private InetAddress[] getAllByName(Object mock, String hostname) + private InetAddress[] getAllByName(Network mockNetwork, String hostname) throws UnknownHostException { - List<InetAddress> answer = queryAllTypes(mock, hostname); + final List<InetAddress> answer; + try { + answer = queryAllTypes(mockNetwork, hostname).get( + HANDLER_TIMEOUT_MS, TimeUnit.MILLISECONDS); + } catch (ExecutionException | InterruptedException | TimeoutException e) { + throw new AssertionError("No mock DNS reply within timeout", e); + } if (answer == null || answer.size() == 0) { throw new UnknownHostException(hostname); } @@ -428,57 +488,76 @@ } // Regardless of the type, depends on what the responses contained in the network. - private List<InetAddress> queryAllTypes(Object mock, String hostname) { - List<InetAddress> answer = new ArrayList<>(); - addAllIfNotNull(answer, getAnswer(mock, hostname, TYPE_A)); - addAllIfNotNull(answer, getAnswer(mock, hostname, TYPE_AAAA)); - return answer; - } - - private void addAllIfNotNull(List<InetAddress> list, List<InetAddress> c) { - if (c != null) { - list.addAll(c); + private CompletableFuture<List<InetAddress>> queryAllTypes( + Network mockNetwork, String hostname) { + if (mockNetwork == mNetwork && !mNonBypassPrivateDnsWorking) { + return CompletableFuture.completedFuture(null); } + + final CompletableFuture<List<InetAddress>> aFuture = + getAnswer(mockNetwork, hostname, TYPE_A) + .exceptionally(e -> Collections.emptyList()); + final CompletableFuture<List<InetAddress>> aaaaFuture = + getAnswer(mockNetwork, hostname, TYPE_AAAA) + .exceptionally(e -> Collections.emptyList()); + + final CompletableFuture<List<InetAddress>> combinedFuture = new CompletableFuture<>(); + aFuture.thenAcceptBoth(aaaaFuture, (res1, res2) -> { + final List<InetAddress> answer = new ArrayList<>(); + if (res1 != null) answer.addAll(res1); + if (res2 != null) answer.addAll(res2); + combinedFuture.complete(answer); + }); + return combinedFuture; } /** Starts mocking DNS queries. */ private void startMocking() throws UnknownHostException { // Queries on mNetwork using getAllByName. doAnswer(invocation -> { - return getAllByName(invocation.getMock(), invocation.getArgument(0)); + return getAllByName((Network) invocation.getMock(), invocation.getArgument(0)); }).when(mNetwork).getAllByName(any()); // Queries on mCleartextDnsNetwork using DnsResolver#query. doAnswer(invocation -> { - return mockQuery(invocation, 1 /* posHostname */, 3 /* posExecutor */, - 5 /* posCallback */, -1 /* posType */); + return mockQuery(invocation, 0 /* posNetwork */, 1 /* posHostname */, + 3 /* posExecutor */, 5 /* posCallback */, -1 /* posType */); }).when(mDnsResolver).query(any(), any(), anyInt(), any(), any(), any()); // Queries on mCleartextDnsNetwork using DnsResolver#query with QueryType. doAnswer(invocation -> { - return mockQuery(invocation, 1 /* posHostname */, 4 /* posExecutor */, - 6 /* posCallback */, 2 /* posType */); + return mockQuery(invocation, 0 /* posNetwork */, 1 /* posHostname */, + 4 /* posExecutor */, 6 /* posCallback */, 2 /* posType */); }).when(mDnsResolver).query(any(), any(), anyInt(), anyInt(), any(), any(), any()); } // Mocking queries on DnsResolver#query. - private Answer mockQuery(InvocationOnMock invocation, int posHostname, int posExecutor, - int posCallback, int posType) { + private Answer mockQuery(InvocationOnMock invocation, int posNetwork, int posHostname, + int posExecutor, int posCallback, int posType) { String hostname = (String) invocation.getArgument(posHostname); Executor executor = (Executor) invocation.getArgument(posExecutor); DnsResolver.Callback<List<InetAddress>> callback = invocation.getArgument(posCallback); - List<InetAddress> answer; + Network network = invocation.getArgument(posNetwork); - answer = posType != -1 - ? getAnswer(invocation.getMock(), hostname, invocation.getArgument(posType)) : - queryAllTypes(invocation.getMock(), hostname); + final CompletableFuture<List<InetAddress>> answerFuture = posType != -1 + ? getAnswer(network, hostname, invocation.getArgument(posType)) + : queryAllTypes(network, hostname); - if (answer != null && answer.size() > 0) { - new Handler(Looper.getMainLooper()).post(() -> { - executor.execute(() -> callback.onAnswer(answer, 0)); - }); - } - // If no answers, do nothing. sendDnsProbeWithTimeout will time out and throw UHE. + answerFuture.whenComplete((answer, exception) -> { + new Handler(Looper.getMainLooper()).post(() -> executor.execute(() -> { + if (exception != null) { + if (!(exception instanceof DnsResolver.DnsException)) { + throw new AssertionError("Test error building DNS response", exception); + } + callback.onError((DnsResolver.DnsException) exception); + return; + } + if (answer != null && answer.size() > 0) { + callback.onAnswer(answer, 0); + } + })); + }); + // If the future does not complete or has no answer do nothing. The timeout should fire. return null; } } @@ -528,6 +607,8 @@ // it will fail the test because of timeout expired for querying AAAA and A sequentially. doReturn(200).when(mResources) .getInteger(eq(R.integer.config_captive_portal_dns_probe_timeout)); + doReturn(200).when(mDependencies).getDeviceConfigPropertyInt( + eq(NAMESPACE_CONNECTIVITY), eq(CONFIG_ASYNC_PRIVDNS_PROBE_TIMEOUT_MS), anyInt()); doAnswer((invocation) -> { URL url = invocation.getArgument(0); @@ -1239,7 +1320,7 @@ setPortal302(mHttpConnection); final String httpHost = new URL(TEST_HTTP_URL).getHost(); mFakeDns.setAnswer(httpHost, new String[] { "2001:db8::123" }, TYPE_AAAA); - final InetAddress parsedPrivateAddr = InetAddresses.parseNumericAddress(privateAddr); + final InetAddress parsedPrivateAddr = parseNumericAddress(privateAddr); mFakeDns.setAnswer(httpHost, new String[] { privateAddr }, (parsedPrivateAddr instanceof Inet6Address) ? TYPE_AAAA : TYPE_A); } @@ -2199,8 +2280,7 @@ NETWORK_VALIDATION_RESULT_VALID, 0 /* probesSucceeded */)); } - @Test - public void testPrivateDnsSuccess() throws Exception { + private void runPrivateDnsSuccessTest() throws Exception { setStatus(mHttpsConnection, 204); setStatus(mHttpConnection, 204); @@ -2232,7 +2312,20 @@ } @Test - public void testProbeStatusChanged() throws Exception { + public void testPrivateDnsSuccess_SyncDns() throws Exception { + doReturn(false).when(mDependencies).isFeatureEnabled( + any(), eq(NetworkStackUtils.NETWORKMONITOR_ASYNC_PRIVDNS_RESOLUTION)); + runPrivateDnsSuccessTest(); + } + + @Test + public void testPrivateDnsSuccess_AsyncDns() throws Exception { + doReturn(true).when(mDependencies).isFeatureEnabled( + any(), eq(NetworkStackUtils.NETWORKMONITOR_ASYNC_PRIVDNS_RESOLUTION)); + runPrivateDnsSuccessTest(); + } + + private void runProbeStatusChangedTest() throws Exception { // Set no record in FakeDns and expect validation to fail. setStatus(mHttpsConnection, 204); setStatus(mHttpConnection, 204); @@ -2255,7 +2348,20 @@ } @Test - public void testPrivateDnsResolutionRetryUpdate() throws Exception { + public void testProbeStatusChanged_SyncDns() throws Exception { + doReturn(false).when(mDependencies).isFeatureEnabled( + any(), eq(NetworkStackUtils.NETWORKMONITOR_ASYNC_PRIVDNS_RESOLUTION)); + runProbeStatusChangedTest(); + } + + @Test + public void testProbeStatusChanged_AsyncDns() throws Exception { + doReturn(true).when(mDependencies).isFeatureEnabled( + any(), eq(NetworkStackUtils.NETWORKMONITOR_ASYNC_PRIVDNS_RESOLUTION)); + runProbeStatusChangedTest(); + } + + private void runPrivateDnsResolutionRetryUpdateTest() throws Exception { // Set no record in FakeDns and expect validation to fail. setStatus(mHttpsConnection, 204); setStatus(mHttpConnection, 204); @@ -2300,6 +2406,197 @@ } @Test + public void testPrivateDnsResolutionRetryUpdate_SyncDns() throws Exception { + doReturn(false).when(mDependencies).isFeatureEnabled( + any(), eq(NetworkStackUtils.NETWORKMONITOR_ASYNC_PRIVDNS_RESOLUTION)); + runPrivateDnsResolutionRetryUpdateTest(); + } + + @Test + public void testPrivateDnsResolutionRetryUpdate_AsyncDns() throws Exception { + doReturn(true).when(mDependencies).isFeatureEnabled( + any(), eq(NetworkStackUtils.NETWORKMONITOR_ASYNC_PRIVDNS_RESOLUTION)); + runPrivateDnsResolutionRetryUpdateTest(); + } + + @Test + public void testAsyncPrivateDnsResolution_PartialTimeout() throws Exception { + doReturn(true).when(mDependencies).isFeatureEnabled( + any(), eq(NetworkStackUtils.NETWORKMONITOR_ASYNC_PRIVDNS_RESOLUTION)); + setStatus(mHttpsConnection, 204); + setStatus(mHttpConnection, 204); + + WrappedNetworkMonitor wnm = makeCellNotMeteredNetworkMonitor(); + wnm.notifyPrivateDnsSettingsChanged(new PrivateDnsConfig("dns.google", new InetAddress[0])); + + // Only provide AAAA answer + mFakeDns.setAnswer("dns.google", new String[]{"2001:db8::1"}, TYPE_AAAA); + + notifyNetworkConnected(wnm, CELL_NOT_METERED_CAPABILITIES); + verifyNetworkTestedValidFromPrivateDns(1 /* interactions */); + + final PrivateDnsConfigParcel expectedConfig = new PrivateDnsConfigParcel(); + expectedConfig.hostname = "dns.google"; + expectedConfig.ips = new String[] {"2001:db8::1"}; + expectedConfig.privateDnsMode = PRIVATE_DNS_MODE_PROVIDER_HOSTNAME; + + verify(mCallbacks).notifyPrivateDnsConfigResolved(expectedConfig); + } + + @Test + public void testAsyncPrivateDnsResolution_PartialFailure() throws Exception { + doReturn(true).when(mDependencies).isFeatureEnabled( + any(), eq(NetworkStackUtils.NETWORKMONITOR_ASYNC_PRIVDNS_RESOLUTION)); + setStatus(mHttpsConnection, 204); + setStatus(mHttpConnection, 204); + + WrappedNetworkMonitor wnm = makeCellNotMeteredNetworkMonitor(); + wnm.notifyPrivateDnsSettingsChanged(new PrivateDnsConfig("dns.google", new InetAddress[0])); + + // A succeeds, AAAA fails + mFakeDns.setAnswer("dns.google", new String[]{"192.0.2.123"}, TYPE_A); + mFakeDns.setAnswer("dns.google", () -> { + // DnsResolver.DnsException constructor is T+, so use a mock instead + throw mock(DnsResolver.DnsException.class); + }, TYPE_AAAA); + + notifyNetworkConnected(wnm, CELL_NOT_METERED_CAPABILITIES); + verifyNetworkTestedValidFromPrivateDns(1 /* interactions */); + + final PrivateDnsConfigParcel expectedConfig = new PrivateDnsConfigParcel(); + expectedConfig.hostname = "dns.google"; + expectedConfig.ips = new String[] {"192.0.2.123"}; + expectedConfig.privateDnsMode = PRIVATE_DNS_MODE_PROVIDER_HOSTNAME; + + verify(mCallbacks).notifyPrivateDnsConfigResolved(expectedConfig); + } + + @Test + public void testAsyncPrivateDnsResolution_AQuerySucceedsFirst_PrioritizeAAAA() + throws Exception { + doReturn(true).when(mDependencies).isFeatureEnabled( + any(), eq(NetworkStackUtils.NETWORKMONITOR_ASYNC_PRIVDNS_RESOLUTION)); + setStatus(mHttpsConnection, 204); + setStatus(mHttpConnection, 204); + + WrappedNetworkMonitor wnm = makeCellNotMeteredNetworkMonitor(); + wnm.notifyPrivateDnsSettingsChanged(new PrivateDnsConfig("dns.google", new InetAddress[0])); + + final ConditionVariable v4Queried = new ConditionVariable(); + mFakeDns.setAnswer("dns.google", () -> { + v4Queried.open(); + return List.of(parseNumericAddress("192.0.2.123")); + }, TYPE_A); + mFakeDns.setAnswer("dns.google", () -> { + // Make sure the v6 query processing is a bit slower than the v6 one. The small delay + // below still does not guarantee that the v4 query will complete first, but it should + // the large majority of the time, which should be enough to test it. Even if it does + // not, the test should pass. + v4Queried.block(HANDLER_TIMEOUT_MS); + SystemClock.sleep(10L); + return List.of(parseNumericAddress("2001:db8::1"), parseNumericAddress("2001:db8::2")); + }, TYPE_AAAA); + + notifyNetworkConnected(wnm, CELL_NOT_METERED_CAPABILITIES); + verifyNetworkTestedValidFromPrivateDns(1 /* interactions */); + + final PrivateDnsConfigParcel expectedConfig = new PrivateDnsConfigParcel(); + expectedConfig.hostname = "dns.google"; + // The IPv6 addresses are still first + expectedConfig.ips = new String[] {"2001:db8::1", "2001:db8::2", "192.0.2.123"}; + expectedConfig.privateDnsMode = PRIVATE_DNS_MODE_PROVIDER_HOSTNAME; + + verify(mCallbacks).notifyPrivateDnsConfigResolved(expectedConfig); + } + + @Test + public void testAsyncPrivateDnsResolution_ConfigChange_RestartsWithNewConfig() + throws Exception { + doReturn(true).when(mDependencies).isFeatureEnabled( + any(), eq(NetworkStackUtils.NETWORKMONITOR_ASYNC_PRIVDNS_RESOLUTION)); + setStatus(mHttpsConnection, 204); + setStatus(mHttpConnection, 204); + + WrappedNetworkMonitor wnm = makeCellNotMeteredNetworkMonitor(); + wnm.notifyPrivateDnsSettingsChanged(new PrivateDnsConfig("v1.google", new InetAddress[0])); + + final ConditionVariable blockReplies = new ConditionVariable(); + final CountDownLatch queriedLatch = new CountDownLatch(2); + mFakeDns.setAnswer("v1.google", () -> { + queriedLatch.countDown(); + blockReplies.block(HANDLER_TIMEOUT_MS); + return List.of(parseNumericAddress("192.0.2.123")); + }, TYPE_A); + mFakeDns.setAnswer("v1.google", () -> { + queriedLatch.countDown(); + blockReplies.block(HANDLER_TIMEOUT_MS); + return List.of(parseNumericAddress("2001:db8::1")); + }, TYPE_AAAA); + + notifyNetworkConnected(wnm, CELL_NOT_METERED_CAPABILITIES); + + queriedLatch.await(HANDLER_TIMEOUT_MS, TimeUnit.MILLISECONDS); + + // Send config update while DNS queries are in flight + mFakeDns.setAnswer("v2.google", new String[] { "192.0.2.124" }, TYPE_A); + mFakeDns.setAnswer("v2.google", new String[] { "2001:db8::2" }, TYPE_AAAA); + wnm.notifyPrivateDnsSettingsChanged(new PrivateDnsConfig("v2.google", new InetAddress[0])); + + // Let the original queries finish. Once DNS queries finish results are posted to the + // handler, so they will be processed on the handler after the DNS settings change. + blockReplies.open(); + + // Expect only callbacks for the 2nd configuration + verifyNetworkTestedValidFromPrivateDns(1 /* interactions */); + + final PrivateDnsConfigParcel expectedConfig = new PrivateDnsConfigParcel(); + expectedConfig.hostname = "v2.google"; + expectedConfig.ips = new String[] {"2001:db8::2", "192.0.2.124"}; + expectedConfig.privateDnsMode = PRIVATE_DNS_MODE_PROVIDER_HOSTNAME; + + verify(mCallbacks).notifyPrivateDnsConfigResolved(expectedConfig); + } + + @Test + public void testAsyncPrivateDnsResolution_TurnOffStrictMode_SkipsDnsValidation() + throws Exception { + doReturn(true).when(mDependencies).isFeatureEnabled( + any(), eq(NetworkStackUtils.NETWORKMONITOR_ASYNC_PRIVDNS_RESOLUTION)); + setStatus(mHttpsConnection, 204); + setStatus(mHttpConnection, 204); + + WrappedNetworkMonitor wnm = makeCellNotMeteredNetworkMonitor(); + wnm.notifyPrivateDnsSettingsChanged(new PrivateDnsConfig("v1.google", new InetAddress[0])); + + final ConditionVariable blockReplies = new ConditionVariable(); + final CountDownLatch queriedLatch = new CountDownLatch(2); + mFakeDns.setAnswer("v1.google", () -> { + queriedLatch.countDown(); + blockReplies.block(HANDLER_TIMEOUT_MS); + return List.of(parseNumericAddress("192.0.2.123")); + }, TYPE_A); + mFakeDns.setAnswer("v1.google", () -> { + queriedLatch.countDown(); + blockReplies.block(HANDLER_TIMEOUT_MS); + return List.of(parseNumericAddress("2001:db8::1")); + }, TYPE_AAAA); + + notifyNetworkConnected(wnm, CELL_NOT_METERED_CAPABILITIES); + + queriedLatch.await(HANDLER_TIMEOUT_MS, TimeUnit.MILLISECONDS); + + // Send config update while DNS queries are in flight + wnm.notifyPrivateDnsSettingsChanged(new PrivateDnsConfig(true /* useTls */)); + + // Let the original queries finish. Once DNS queries finish results are posted to the + // handler, so they will be processed on the handler after the DNS settings change. + blockReplies.open(); + + verifyNetworkTestedValidFromHttps(1 /* interactions */); + verify(mCallbacks, never()).notifyPrivateDnsConfigResolved(any()); + } + + @Test public void testReevaluationInterval_networkResume() throws Exception { // Setup nothing and expect validation to fail. doReturn(true).when(mDependencies).isFeatureEnabled(any(), eq(REEVALUATE_WHEN_RESUME)); @@ -2725,8 +3022,8 @@ } catch (UnknownHostException e) { } - mFakeDns.setAnswer("www.android.com", null, TYPE_A); - mFakeDns.setAnswer("www.android.com", null, TYPE_AAAA); + mFakeDns.setAnswer("www.android.com", (String[]) null, TYPE_A); + mFakeDns.setAnswer("www.android.com", (String[]) null, TYPE_AAAA); try { wnm.sendDnsProbeWithTimeout("www.android.com", shortTimeoutMs); fail("DNS query timed out, expected UnknownHostException");