diff options
4 files changed, 408 insertions, 183 deletions
diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 995f92647982..7a4ac9b10ff3 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -902,7 +902,7 @@ public class ConnectivityService extends IConnectivityManager.Stub // Used only for testing. // TODO: Delete this and either: - // 1. Give Fake SettingsProvider the ability to send settings change notifications (requires + // 1. Give FakeSettingsProvider the ability to send settings change notifications (requires // changing ContentResolver to make registerContentObserver non-final). // 2. Give FakeSettingsProvider an alternative notification mechanism and have the test use it // by subclassing SettingsObserver. @@ -911,6 +911,12 @@ public class ConnectivityService extends IConnectivityManager.Stub mHandler.sendEmptyMessage(EVENT_CONFIGURE_MOBILE_DATA_ALWAYS_ON); } + // See FakeSettingsProvider comment above. + @VisibleForTesting + void updatePrivateDnsSettings() { + mHandler.sendEmptyMessage(EVENT_PRIVATE_DNS_SETTINGS_CHANGED); + } + private void handleMobileDataAlwaysOn() { final boolean enable = toBool(Settings.Global.getInt( mContext.getContentResolver(), Settings.Global.MOBILE_DATA_ALWAYS_ON, 1)); @@ -940,8 +946,8 @@ public class ConnectivityService extends IConnectivityManager.Stub } private void registerPrivateDnsSettingsCallbacks() { - for (Uri u : DnsManager.getPrivateDnsSettingsUris()) { - mSettingsObserver.observe(u, EVENT_PRIVATE_DNS_SETTINGS_CHANGED); + for (Uri uri : DnsManager.getPrivateDnsSettingsUris()) { + mSettingsObserver.observe(uri, EVENT_PRIVATE_DNS_SETTINGS_CHANGED); } } @@ -994,8 +1000,12 @@ public class ConnectivityService extends IConnectivityManager.Stub if (network == null) { return null; } + return getNetworkAgentInfoForNetId(network.netId); + } + + private NetworkAgentInfo getNetworkAgentInfoForNetId(int netId) { synchronized (mNetworkForNetId) { - return mNetworkForNetId.get(network.netId); + return mNetworkForNetId.get(netId); } } @@ -1135,9 +1145,7 @@ public class ConnectivityService extends IConnectivityManager.Stub } NetworkAgentInfo nai; if (vpnNetId != NETID_UNSET) { - synchronized (mNetworkForNetId) { - nai = mNetworkForNetId.get(vpnNetId); - } + nai = getNetworkAgentInfoForNetId(vpnNetId); if (nai != null) return nai.network; } nai = getDefaultNetwork(); @@ -2113,41 +2121,21 @@ public class ConnectivityService extends IConnectivityManager.Stub default: return false; case NetworkMonitor.EVENT_NETWORK_TESTED: { - final NetworkAgentInfo nai; - synchronized (mNetworkForNetId) { - nai = mNetworkForNetId.get(msg.arg2); - } + final NetworkAgentInfo nai = getNetworkAgentInfoForNetId(msg.arg2); if (nai == null) break; final boolean valid = (msg.arg1 == NetworkMonitor.NETWORK_TEST_RESULT_VALID); final boolean wasValidated = nai.lastValidated; final boolean wasDefault = isDefaultNetwork(nai); - final PrivateDnsConfig privateDnsCfg = (msg.obj instanceof PrivateDnsConfig) - ? (PrivateDnsConfig) msg.obj : null; final String redirectUrl = (msg.obj instanceof String) ? (String) msg.obj : ""; - final boolean reevaluationRequired; - final String logMsg; - if (valid) { - reevaluationRequired = updatePrivateDns(nai, privateDnsCfg); - logMsg = (DBG && (privateDnsCfg != null)) - ? " with " + privateDnsCfg.toString() : ""; - } else { - reevaluationRequired = false; - logMsg = (DBG && !TextUtils.isEmpty(redirectUrl)) - ? " with redirect to " + redirectUrl : ""; - } if (DBG) { + final String logMsg = !TextUtils.isEmpty(redirectUrl) + ? " with redirect to " + redirectUrl + : ""; log(nai.name() + " validation " + (valid ? "passed" : "failed") + logMsg); } - // If there is a change in Private DNS configuration, - // trigger reevaluation of the network to test it. - if (reevaluationRequired) { - nai.networkMonitor.sendMessage( - NetworkMonitor.CMD_FORCE_REEVALUATION, Process.SYSTEM_UID); - break; - } if (valid != nai.lastValidated) { if (wasDefault) { metricsLogger().defaultNetworkMetrics().logDefaultNetworkValidity( @@ -2176,10 +2164,7 @@ public class ConnectivityService extends IConnectivityManager.Stub case NetworkMonitor.EVENT_PROVISIONING_NOTIFICATION: { final int netId = msg.arg2; final boolean visible = toBool(msg.arg1); - final NetworkAgentInfo nai; - synchronized (mNetworkForNetId) { - nai = mNetworkForNetId.get(netId); - } + final NetworkAgentInfo nai = getNetworkAgentInfoForNetId(netId); // If captive portal status has changed, update capabilities or disconnect. if (nai != null && (visible != nai.lastCaptivePortalDetected)) { final int oldScore = nai.getCurrentScore(); @@ -2210,18 +2195,10 @@ public class ConnectivityService extends IConnectivityManager.Stub break; } case NetworkMonitor.EVENT_PRIVATE_DNS_CONFIG_RESOLVED: { - final NetworkAgentInfo nai; - synchronized (mNetworkForNetId) { - nai = mNetworkForNetId.get(msg.arg2); - } + final NetworkAgentInfo nai = getNetworkAgentInfoForNetId(msg.arg2); if (nai == null) break; - final PrivateDnsConfig cfg = (PrivateDnsConfig) msg.obj; - final boolean reevaluationRequired = updatePrivateDns(nai, cfg); - if (nai.lastValidated && reevaluationRequired) { - nai.networkMonitor.sendMessage( - NetworkMonitor.CMD_FORCE_REEVALUATION, Process.SYSTEM_UID); - } + updatePrivateDns(nai, (PrivateDnsConfig) msg.obj); break; } } @@ -2259,61 +2236,38 @@ public class ConnectivityService extends IConnectivityManager.Stub } } + private boolean networkRequiresValidation(NetworkAgentInfo nai) { + return NetworkMonitor.isValidationRequired( + mDefaultRequest.networkCapabilities, nai.networkCapabilities); + } + private void handlePrivateDnsSettingsChanged() { final PrivateDnsConfig cfg = mDnsManager.getPrivateDnsConfig(); for (NetworkAgentInfo nai : mNetworkAgentInfos.values()) { - // Private DNS only ever applies to networks that might provide - // Internet access and therefore also require validation. - if (!NetworkMonitor.isValidationRequired( - mDefaultRequest.networkCapabilities, nai.networkCapabilities)) { - continue; - } - - // Notify the NetworkMonitor thread in case it needs to cancel or - // schedule DNS resolutions. If a DNS resolution is required the - // result will be sent back to us. - nai.networkMonitor.notifyPrivateDnsSettingsChanged(cfg); - - if (!cfg.inStrictMode()) { - // No strict mode hostname DNS resolution needed, so just update - // DNS settings directly. In opportunistic and "off" modes this - // just reprograms netd with the network-supplied DNS servers - // (and of course the boolean of whether or not to attempt TLS). - // - // TODO: Consider code flow parity with strict mode, i.e. having - // NetworkMonitor relay the PrivateDnsConfig back to us and then - // performing this call at that time. - updatePrivateDns(nai, cfg); - } + handlePerNetworkPrivateDnsConfig(nai, cfg); } } - private boolean updatePrivateDns(NetworkAgentInfo nai, PrivateDnsConfig newCfg) { - final boolean reevaluationRequired = true; - final boolean dontReevaluate = false; + private void handlePerNetworkPrivateDnsConfig(NetworkAgentInfo nai, PrivateDnsConfig cfg) { + // Private DNS only ever applies to networks that might provide + // Internet access and therefore also require validation. + if (!networkRequiresValidation(nai)) return; - final PrivateDnsConfig oldCfg = mDnsManager.updatePrivateDns(nai.network, newCfg); - updateDnses(nai.linkProperties, null, nai.network.netId); + // Notify the NetworkMonitor thread in case it needs to cancel or + // schedule DNS resolutions. If a DNS resolution is required the + // result will be sent back to us. + nai.networkMonitor.notifyPrivateDnsSettingsChanged(cfg); - if (newCfg == null) { - if (oldCfg == null) return dontReevaluate; - return oldCfg.useTls ? reevaluationRequired : dontReevaluate; - } - - if (oldCfg == null) { - return newCfg.useTls ? reevaluationRequired : dontReevaluate; - } - - if (oldCfg.useTls != newCfg.useTls) { - return reevaluationRequired; - } - - if (newCfg.inStrictMode() && !Objects.equals(oldCfg.hostname, newCfg.hostname)) { - return reevaluationRequired; - } + // With Private DNS bypass support, we can proceed to update the + // Private DNS config immediately, even if we're in strict mode + // and have not yet resolved the provider name into a set of IPs. + updatePrivateDns(nai, cfg); + } - return dontReevaluate; + private void updatePrivateDns(NetworkAgentInfo nai, PrivateDnsConfig newCfg) { + mDnsManager.updatePrivateDns(nai.network, newCfg); + updateDnses(nai.linkProperties, null, nai.network.netId); } private void updateLingerState(NetworkAgentInfo nai, long now) { @@ -3252,7 +3206,7 @@ public class ConnectivityService extends IConnectivityManager.Stub if (isNetworkWithLinkPropertiesBlocked(lp, uid, false)) { return; } - nai.networkMonitor.sendMessage(NetworkMonitor.CMD_FORCE_REEVALUATION, uid); + nai.networkMonitor.forceReevaluation(uid); } private ProxyInfo getDefaultProxy() { @@ -4871,7 +4825,7 @@ public class ConnectivityService extends IConnectivityManager.Stub } public void handleUpdateLinkProperties(NetworkAgentInfo nai, LinkProperties newLp) { - if (mNetworkForNetId.get(nai.network.netId) != nai) { + if (getNetworkAgentInfoForNetId(nai.network.netId) != nai) { // Ignore updates for disconnected networks return; } @@ -5451,6 +5405,7 @@ public class ConnectivityService extends IConnectivityManager.Stub Slog.wtf(TAG, networkAgent.name() + " connected with null LinkProperties"); } + handlePerNetworkPrivateDnsConfig(networkAgent, mDnsManager.getPrivateDnsConfig()); updateLinkProperties(networkAgent, null); networkAgent.networkMonitor.sendMessage(NetworkMonitor.CMD_NETWORK_CONNECTED); diff --git a/services/core/java/com/android/server/connectivity/DnsManager.java b/services/core/java/com/android/server/connectivity/DnsManager.java index 55798491cdef..2a361a02d7d2 100644 --- a/services/core/java/com/android/server/connectivity/DnsManager.java +++ b/services/core/java/com/android/server/connectivity/DnsManager.java @@ -61,6 +61,51 @@ import java.util.StringJoiner; * This class it NOT designed for concurrent access. Furthermore, all non-static * methods MUST be called from ConnectivityService's thread. * + * [ Private DNS ] + * The code handling Private DNS is spread across several components, but this + * seems like the least bad place to collect all the observations. + * + * Private DNS handling and updating occurs in response to several different + * events. Each is described here with its corresponding intended handling. + * + * [A] Event: A new network comes up. + * Mechanics: + * [1] ConnectivityService gets notifications from NetworkAgents. + * [2] in updateNetworkInfo(), the first time the NetworkAgent goes into + * into CONNECTED state, the Private DNS configuration is retrieved, + * programmed, and strict mode hostname resolution (if applicable) is + * enqueued in NetworkAgent's NetworkMonitor, via a call to + * handlePerNetworkPrivateDnsConfig(). + * [3] Re-resolution of strict mode hostnames that fail to return any + * IP addresses happens inside NetworkMonitor; it sends itself a + * delayed CMD_EVALUATE_PRIVATE_DNS message in a simple backoff + * schedule. + * [4] Successfully resolved hostnames are sent to ConnectivityService + * inside an EVENT_PRIVATE_DNS_CONFIG_RESOLVED message. The resolved + * IP addresses are programmed into netd via: + * + * updatePrivateDns() -> updateDnses() + * + * both of which make calls into DnsManager. + * [5] Upon a successful hostname resolution NetworkMonitor initiates a + * validation attempt in the form of a lookup for a one-time hostname + * that uses Private DNS. + * + * [B] Event: Private DNS settings are changed. + * Mechanics: + * [1] ConnectivityService gets notifications from its SettingsObserver. + * [2] handlePrivateDnsSettingsChanged() is called, which calls + * handlePerNetworkPrivateDnsConfig() and the process proceeds + * as if from A.3 above. + * + * [C] Event: An application calls ConnectivityManager#reportBadNetwork(). + * Mechanics: + * [1] NetworkMonitor is notified and initiates a reevaluation, which + * always bypasses Private DNS. + * [2] Once completed, NetworkMonitor checks if strict mode is in operation + * and if so enqueues another evaluation of Private DNS, as if from + * step A.5 above. + * * @hide */ public class DnsManager { diff --git a/services/core/java/com/android/server/connectivity/NetworkMonitor.java b/services/core/java/com/android/server/connectivity/NetworkMonitor.java index 8a2e71c1fba8..284538342a72 100644 --- a/services/core/java/com/android/server/connectivity/NetworkMonitor.java +++ b/services/core/java/com/android/server/connectivity/NetworkMonitor.java @@ -34,6 +34,7 @@ import android.net.NetworkRequest; import android.net.ProxyInfo; import android.net.TrafficStats; import android.net.Uri; +import android.net.dns.ResolvUtil; import android.net.metrics.IpConnectivityLog; import android.net.metrics.NetworkEvent; import android.net.metrics.ValidationProbeEvent; @@ -64,6 +65,7 @@ import com.android.internal.annotations.VisibleForTesting; import com.android.internal.util.Protocol; import com.android.internal.util.State; import com.android.internal.util.StateMachine; +import com.android.server.connectivity.DnsManager.PrivateDnsConfig; import java.io.IOException; import java.net.HttpURLConnection; @@ -77,6 +79,7 @@ import java.util.Collections; import java.util.LinkedHashMap; import java.util.List; import java.util.Random; +import java.util.UUID; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; @@ -165,7 +168,7 @@ public class NetworkMonitor extends StateMachine { * Force evaluation even if it has succeeded in the past. * arg1 = UID responsible for requesting this reeval. Will be billed for data. */ - public static final int CMD_FORCE_REEVALUATION = BASE + 8; + private static final int CMD_FORCE_REEVALUATION = BASE + 8; /** * Message to self indicating captive portal app finished. @@ -205,9 +208,15 @@ public class NetworkMonitor extends StateMachine { * Private DNS. If a DNS resolution is required, e.g. for DNS-over-TLS in * strict mode, then an event is sent back to ConnectivityService with the * result of the resolution attempt. + * + * A separate message is used to trigger (re)evaluation of the Private DNS + * configuration, so that the message can be handled as needed in different + * states, including being ignored until after an ongoing captive portal + * validation phase is completed. */ private static final int CMD_PRIVATE_DNS_SETTINGS_CHANGED = BASE + 13; public static final int EVENT_PRIVATE_DNS_CONFIG_RESOLVED = BASE + 14; + private static final int CMD_EVALUATE_PRIVATE_DNS = BASE + 15; // Start mReevaluateDelayMs at this value and double. private static final int INITIAL_REEVALUATE_DELAY_MS = 1000; @@ -215,6 +224,7 @@ public class NetworkMonitor extends StateMachine { // Before network has been evaluated this many times, ignore repeated reevaluate requests. private static final int IGNORE_REEVALUATE_ATTEMPTS = 5; private int mReevaluateToken = 0; + private static final int NO_UID = 0; private static final int INVALID_UID = -1; private int mUidResponsibleForReeval = INVALID_UID; // Stop blaming UID that requested re-evaluation after this many attempts. @@ -224,6 +234,8 @@ public class NetworkMonitor extends StateMachine { private static final int NUM_VALIDATION_LOG_LINES = 20; + private String mPrivateDnsProviderHostname = ""; + public static boolean isValidationRequired( NetworkCapabilities dfltNetCap, NetworkCapabilities nc) { // TODO: Consider requiring validation for DUN networks. @@ -261,13 +273,12 @@ public class NetworkMonitor extends StateMachine { public boolean systemReady = false; - private DnsManager.PrivateDnsConfig mPrivateDnsCfg = null; - private final State mDefaultState = new DefaultState(); private final State mValidatedState = new ValidatedState(); private final State mMaybeNotifyState = new MaybeNotifyState(); private final State mEvaluatingState = new EvaluatingState(); private final State mCaptivePortalState = new CaptivePortalState(); + private final State mEvaluatingPrivateDnsState = new EvaluatingPrivateDnsState(); private CustomIntentReceiver mLaunchCaptivePortalAppBroadcastReceiver = null; @@ -293,6 +304,10 @@ public class NetworkMonitor extends StateMachine { // Add suffix indicating which NetworkMonitor we're talking about. super(TAG + networkAgentInfo.name()); + // Logs with a tag of the form given just above, e.g. + // <timestamp> 862 2402 D NetworkMonitor/NetworkAgentInfo [WIFI () - 100]: ... + setDbg(VDBG); + mContext = context; mMetricsLog = logger; mConnectivityServiceHandler = handler; @@ -305,10 +320,11 @@ public class NetworkMonitor extends StateMachine { mDefaultRequest = defaultRequest; addState(mDefaultState); - addState(mValidatedState, mDefaultState); addState(mMaybeNotifyState, mDefaultState); addState(mEvaluatingState, mMaybeNotifyState); addState(mCaptivePortalState, mMaybeNotifyState); + addState(mEvaluatingPrivateDnsState, mDefaultState); + addState(mValidatedState, mDefaultState); setInitialState(mDefaultState); mIsCaptivePortalCheckEnabled = getIsCaptivePortalCheckEnabled(); @@ -321,6 +337,17 @@ public class NetworkMonitor extends StateMachine { start(); } + public void forceReevaluation(int responsibleUid) { + sendMessage(CMD_FORCE_REEVALUATION, responsibleUid, 0); + } + + public void notifyPrivateDnsSettingsChanged(PrivateDnsConfig newCfg) { + // Cancel any outstanding resolutions. + removeMessages(CMD_PRIVATE_DNS_SETTINGS_CHANGED); + // Send the update to the proper thread. + sendMessage(CMD_PRIVATE_DNS_SETTINGS_CHANGED, newCfg); + } + @Override protected void log(String s) { if (DBG) Log.d(TAG + "/" + mNetworkAgentInfo.name(), s); @@ -349,6 +376,12 @@ public class NetworkMonitor extends StateMachine { mDefaultRequest.networkCapabilities, mNetworkAgentInfo.networkCapabilities); } + + private void notifyNetworkTestResultInvalid(Object obj) { + mConnectivityServiceHandler.sendMessage(obtainMessage( + EVENT_NETWORK_TESTED, NETWORK_TEST_RESULT_INVALID, mNetId, obj)); + } + // DefaultState is the parent of all States. It exists only to handle CMD_* messages but // does not entail any real state (hence no enter() or exit() routines). private class DefaultState extends State { @@ -392,41 +425,66 @@ public class NetworkMonitor extends StateMachine { switch (message.arg1) { case APP_RETURN_DISMISSED: - sendMessage(CMD_FORCE_REEVALUATION, 0 /* no UID */, 0); + sendMessage(CMD_FORCE_REEVALUATION, NO_UID, 0); break; case APP_RETURN_WANTED_AS_IS: mDontDisplaySigninNotification = true; // TODO: Distinguish this from a network that actually validates. - // Displaying the "!" on the system UI icon may still be a good idea. - transitionTo(mValidatedState); + // Displaying the "x" on the system UI icon may still be a good idea. + transitionTo(mEvaluatingPrivateDnsState); break; case APP_RETURN_UNWANTED: mDontDisplaySigninNotification = true; mUserDoesNotWant = true; - mConnectivityServiceHandler.sendMessage(obtainMessage( - EVENT_NETWORK_TESTED, NETWORK_TEST_RESULT_INVALID, - mNetId, null)); + notifyNetworkTestResultInvalid(null); // TODO: Should teardown network. mUidResponsibleForReeval = 0; transitionTo(mEvaluatingState); break; } return HANDLED; - case CMD_PRIVATE_DNS_SETTINGS_CHANGED: - if (isValidationRequired()) { - // This performs a blocking DNS resolution of the - // strict mode hostname, if required. - resolvePrivateDnsConfig((DnsManager.PrivateDnsConfig) message.obj); - if ((mPrivateDnsCfg != null) && mPrivateDnsCfg.inStrictMode()) { - mConnectivityServiceHandler.sendMessage(obtainMessage( - EVENT_PRIVATE_DNS_CONFIG_RESOLVED, 0, mNetId, - new DnsManager.PrivateDnsConfig(mPrivateDnsCfg))); - } + case CMD_PRIVATE_DNS_SETTINGS_CHANGED: { + final PrivateDnsConfig cfg = (PrivateDnsConfig) message.obj; + if (!isValidationRequired() || cfg == null || !cfg.inStrictMode()) { + // No DNS resolution required. + // + // We don't force any validation in opportunistic mode + // here. Opportunistic mode nameservers are validated + // separately within netd. + // + // Reset Private DNS settings state. + mPrivateDnsProviderHostname = ""; + break; } - return HANDLED; + + mPrivateDnsProviderHostname = cfg.hostname; + + // DNS resolutions via Private DNS strict mode block for a + // few seconds (~4.2) checking for any IP addresses to + // arrive and validate. Initiating a (re)evaluation now + // should not significantly alter the validation outcome. + // + // No matter what: enqueue a validation request; one of + // three things can happen with this request: + // [1] ignored (EvaluatingState or CaptivePortalState) + // [2] transition to EvaluatingPrivateDnsState + // (DefaultState and ValidatedState) + // [3] handled (EvaluatingPrivateDnsState) + // + // The Private DNS configuration to be evaluated will: + // [1] be skipped (not in strict mode), or + // [2] validate (huzzah), or + // [3] encounter some problem (invalid hostname, + // no resolved IP addresses, IPs unreachable, + // port 853 unreachable, port 853 is not running a + // DNS-over-TLS server, et cetera). + sendMessage(CMD_EVALUATE_PRIVATE_DNS); + break; + } default: - return HANDLED; + break; } + return HANDLED; } } @@ -440,7 +498,7 @@ public class NetworkMonitor extends StateMachine { maybeLogEvaluationResult( networkEventType(validationStage(), EvaluationResult.VALIDATED)); mConnectivityServiceHandler.sendMessage(obtainMessage(EVENT_NETWORK_TESTED, - NETWORK_TEST_RESULT_VALID, mNetId, mPrivateDnsCfg)); + NETWORK_TEST_RESULT_VALID, mNetId, null)); mValidations++; } @@ -449,10 +507,14 @@ public class NetworkMonitor extends StateMachine { switch (message.what) { case CMD_NETWORK_CONNECTED: transitionTo(mValidatedState); - return HANDLED; + break; + case CMD_EVALUATE_PRIVATE_DNS: + transitionTo(mEvaluatingPrivateDnsState); + break; default: return NOT_HANDLED; } + return HANDLED; } } @@ -569,11 +631,11 @@ public class NetworkMonitor extends StateMachine { case CMD_REEVALUATE: if (message.arg1 != mReevaluateToken || mUserDoesNotWant) return HANDLED; - // Don't bother validating networks that don't satisify the default request. + // Don't bother validating networks that don't satisfy the default request. // This includes: // - VPNs which can be considered explicitly desired by the user and the // user's desire trumps whether the network validates. - // - Networks that don't provide internet access. It's unclear how to + // - Networks that don't provide Internet access. It's unclear how to // validate such networks. // - Untrusted networks. It's unsafe to prompt the user to sign-in to // such networks and the user didn't express interest in connecting to @@ -588,7 +650,6 @@ public class NetworkMonitor extends StateMachine { // expensive metered network, or unwanted leaking of the User Agent string. if (!isValidationRequired()) { validationLog("Network would not satisfy default request, not validating"); - mPrivateDnsCfg = null; transitionTo(mValidatedState); return HANDLED; } @@ -601,20 +662,18 @@ public class NetworkMonitor extends StateMachine { // if this is found to cause problems. CaptivePortalProbeResult probeResult = isCaptivePortal(); if (probeResult.isSuccessful()) { - resolvePrivateDnsConfig(); - transitionTo(mValidatedState); + // Transit EvaluatingPrivateDnsState to get to Validated + // state (even if no Private DNS validation required). + transitionTo(mEvaluatingPrivateDnsState); } else if (probeResult.isPortal()) { - mConnectivityServiceHandler.sendMessage(obtainMessage(EVENT_NETWORK_TESTED, - NETWORK_TEST_RESULT_INVALID, mNetId, probeResult.redirectUrl)); + notifyNetworkTestResultInvalid(probeResult.redirectUrl); mLastPortalProbeResult = probeResult; transitionTo(mCaptivePortalState); } else { final Message msg = obtainMessage(CMD_REEVALUATE, ++mReevaluateToken, 0); sendMessageDelayed(msg, mReevaluateDelayMs); logNetworkEvent(NetworkEvent.NETWORK_VALIDATION_FAILED); - mConnectivityServiceHandler.sendMessage(obtainMessage( - EVENT_NETWORK_TESTED, NETWORK_TEST_RESULT_INVALID, mNetId, - probeResult.redirectUrl)); + notifyNetworkTestResultInvalid(probeResult.redirectUrl); if (mAttempts >= BLAME_FOR_EVALUATION_ATTEMPTS) { // Don't continue to blame UID forever. TrafficStats.clearThreadStatsUid(); @@ -700,6 +759,110 @@ public class NetworkMonitor extends StateMachine { } } + private class EvaluatingPrivateDnsState extends State { + private int mPrivateDnsReevalDelayMs; + private PrivateDnsConfig mPrivateDnsConfig; + + @Override + public void enter() { + mPrivateDnsReevalDelayMs = INITIAL_REEVALUATE_DELAY_MS; + mPrivateDnsConfig = null; + sendMessage(CMD_EVALUATE_PRIVATE_DNS); + } + + @Override + public boolean processMessage(Message msg) { + switch (msg.what) { + case CMD_EVALUATE_PRIVATE_DNS: + if (inStrictMode()) { + if (!isStrictModeHostnameResolved()) { + resolveStrictModeHostname(); + + if (isStrictModeHostnameResolved()) { + notifyPrivateDnsConfigResolved(); + } else { + handlePrivateDnsEvaluationFailure(); + break; + } + } + + // Look up a one-time hostname, to bypass caching. + // + // Note that this will race with ConnectivityService + // code programming the DNS-over-TLS server IP addresses + // into netd (if invoked, above). If netd doesn't know + // the IP addresses yet, or if the connections to the IP + // addresses haven't yet been validated, netd will block + // for up to a few seconds before failing the lookup. + if (!sendPrivateDnsProbe()) { + handlePrivateDnsEvaluationFailure(); + break; + } + } + + // All good! + transitionTo(mValidatedState); + break; + default: + return NOT_HANDLED; + } + return HANDLED; + } + + private boolean inStrictMode() { + 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. + mPrivateDnsConfig = new PrivateDnsConfig( + mPrivateDnsProviderHostname, + mNetwork.getAllByName(mPrivateDnsProviderHostname)); + } catch (UnknownHostException uhe) { + mPrivateDnsConfig = null; + } + } + + private void notifyPrivateDnsConfigResolved() { + mConnectivityServiceHandler.sendMessage(obtainMessage( + EVENT_PRIVATE_DNS_CONFIG_RESOLVED, 0, mNetId, mPrivateDnsConfig)); + } + + private void handlePrivateDnsEvaluationFailure() { + notifyNetworkTestResultInvalid(null); + + // Queue up a re-evaluation with backoff. + // + // TODO: Consider abandoning this state after a few attempts and + // transitioning back to EvaluatingState, to perhaps give ourselves + // the opportunity to (re)detect a captive portal or something. + sendMessageDelayed(CMD_EVALUATE_PRIVATE_DNS, mPrivateDnsReevalDelayMs); + mPrivateDnsReevalDelayMs *= 2; + if (mPrivateDnsReevalDelayMs > MAX_REEVALUATE_DELAY_MS) { + mPrivateDnsReevalDelayMs = MAX_REEVALUATE_DELAY_MS; + } + } + + private boolean sendPrivateDnsProbe() { + // q.v. system/netd/server/dns/DnsTlsTransport.cpp + final String ONE_TIME_HOSTNAME_SUFFIX = "-dnsotls-ds.metric.gstatic.com"; + final String host = UUID.randomUUID().toString().substring(0, 8) + + ONE_TIME_HOSTNAME_SUFFIX; + try { + final InetAddress[] ips = mNetworkAgentInfo.network().getAllByName(host); + return (ips != null && ips.length > 0); + } catch (UnknownHostException uhe) {} + return false; + } + } + // Limits the list of IP addresses returned by getAllByName or tried by openConnection to at // most one per address family. This ensures we only wait up to 20 seconds for TCP connections // to complete, regardless of how many IP addresses a host has. @@ -710,7 +873,9 @@ public class NetworkMonitor extends StateMachine { @Override public InetAddress[] getAllByName(String host) throws UnknownHostException { - List<InetAddress> addrs = Arrays.asList(super.getAllByName(host)); + // Always bypass Private DNS. + final List<InetAddress> addrs = Arrays.asList( + ResolvUtil.blockingResolveAllLocally(this, host)); // Ensure the address family of the first address is tried first. LinkedHashMap<Class, InetAddress> addressByFamily = new LinkedHashMap<>(); @@ -1065,44 +1230,6 @@ public class NetworkMonitor extends StateMachine { return null; } - public void notifyPrivateDnsSettingsChanged(DnsManager.PrivateDnsConfig newCfg) { - // Cancel any outstanding resolutions. - removeMessages(CMD_PRIVATE_DNS_SETTINGS_CHANGED); - // Send the update to the proper thread. - sendMessage(CMD_PRIVATE_DNS_SETTINGS_CHANGED, newCfg); - } - - private void resolvePrivateDnsConfig() { - resolvePrivateDnsConfig(DnsManager.getPrivateDnsConfig(mContext.getContentResolver())); - } - - private void resolvePrivateDnsConfig(DnsManager.PrivateDnsConfig cfg) { - // Nothing to do. - if (cfg == null) { - mPrivateDnsCfg = null; - return; - } - - // No DNS resolution required. - if (!cfg.inStrictMode()) { - mPrivateDnsCfg = cfg; - return; - } - - if ((mPrivateDnsCfg != null) && mPrivateDnsCfg.inStrictMode() && - (mPrivateDnsCfg.ips.length > 0) && mPrivateDnsCfg.hostname.equals(cfg.hostname)) { - // We have already resolved this strict mode hostname. Assume that - // Private DNS services won't be changing serving IP addresses very - // frequently and save ourselves one re-resolve. - return; - } - - mPrivateDnsCfg = cfg; - final DnsManager.PrivateDnsConfig resolvedCfg = DnsManager.tryBlockingResolveOf( - mNetwork, mPrivateDnsCfg.hostname); - if (resolvedCfg != null) mPrivateDnsCfg = resolvedCfg; - } - /** * @param responseReceived - whether or not we received a valid HTTP response to our request. * If false, isCaptivePortal and responseTimestampMs are ignored diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java index 82b7bec6e929..d04160eae9e4 100644 --- a/tests/net/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java @@ -17,6 +17,9 @@ package com.android.server; import static android.net.ConnectivityManager.CONNECTIVITY_ACTION; +import static android.net.ConnectivityManager.PRIVATE_DNS_MODE_OFF; +import static android.net.ConnectivityManager.PRIVATE_DNS_MODE_OPPORTUNISTIC; +import static android.net.ConnectivityManager.PRIVATE_DNS_MODE_PROVIDER_HOSTNAME; import static android.net.ConnectivityManager.TYPE_ETHERNET; import static android.net.ConnectivityManager.TYPE_MOBILE; import static android.net.ConnectivityManager.TYPE_MOBILE_FOTA; @@ -70,6 +73,7 @@ import static org.mockito.Mockito.reset; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.when; @@ -181,6 +185,9 @@ public class ConnectivityServiceTest { private static final int TIMEOUT_MS = 500; private static final int TEST_LINGER_DELAY_MS = 120; + private static final String MOBILE_IFNAME = "test_rmnet_data0"; + private static final String WIFI_IFNAME = "test_wlan0"; + private MockContext mServiceContext; private WrappedConnectivityService mService; private WrappedConnectivityManager mCm; @@ -751,7 +758,7 @@ public class ConnectivityServiceTest { // NetworkMonitor implementation allowing overriding of Internet connectivity probe result. private class WrappedNetworkMonitor extends NetworkMonitor { - public Handler connectivityHandler; + public final Handler connectivityHandler; // HTTP response code fed back to NetworkMonitor for Internet connectivity probe. public int gen204ProbeResult = 500; public String gen204ProbeRedirectUrl = null; @@ -926,6 +933,7 @@ public class ConnectivityServiceTest { // Ensure that the default setting for Captive Portals is used for most tests setCaptivePortalMode(Settings.Global.CAPTIVE_PORTAL_MODE_PROMPT); setMobileDataAlwaysOn(false); + setPrivateDnsSettings(PRIVATE_DNS_MODE_OFF, "ignored.example.com"); } @After @@ -2580,6 +2588,14 @@ public class ConnectivityServiceTest { waitForIdle(); } + private void setPrivateDnsSettings(String mode, String specifier) { + final ContentResolver cr = mServiceContext.getContentResolver(); + Settings.Global.putString(cr, Settings.Global.PRIVATE_DNS_MODE, mode); + Settings.Global.putString(cr, Settings.Global.PRIVATE_DNS_SPECIFIER, specifier); + mService.updatePrivateDnsSettings(); + waitForIdle(); + } + private boolean isForegroundNetwork(MockNetworkAgent network) { NetworkCapabilities nc = mCm.getNetworkCapabilities(network.getNetwork()); assertNotNull(nc); @@ -3579,7 +3595,7 @@ public class ConnectivityServiceTest { mCm.registerNetworkCallback(networkRequest, networkCallback); LinkProperties lp = new LinkProperties(); - lp.setInterfaceName("wlan0"); + lp.setInterfaceName(WIFI_IFNAME); LinkAddress myIpv4Address = new LinkAddress("192.168.12.3/24"); RouteInfo myIpv4DefaultRoute = new RouteInfo((IpPrefix) null, NetworkUtils.numericToInetAddress("192.168.12.1"), lp.getInterfaceName()); @@ -3668,52 +3684,63 @@ public class ConnectivityServiceTest { @Test public void testBasicDnsConfigurationPushed() throws Exception { - final String IFNAME = "test_rmnet_data0"; - final String[] EMPTY_TLS_SERVERS = new String[0]; + setPrivateDnsSettings(PRIVATE_DNS_MODE_OPPORTUNISTIC, "ignored.example.com"); + ArgumentCaptor<String[]> tlsServers = ArgumentCaptor.forClass(String[].class); + + // Clear any interactions that occur as a result of CS starting up. + reset(mNetworkManagementService); + + final String[] EMPTY_STRING_ARRAY = new String[0]; mCellNetworkAgent = new MockNetworkAgent(TRANSPORT_CELLULAR); waitForIdle(); verify(mNetworkManagementService, never()).setDnsConfigurationForNetwork( - anyInt(), any(), any(), any(), anyString(), eq(EMPTY_TLS_SERVERS)); + anyInt(), eq(EMPTY_STRING_ARRAY), any(), any(), eq(""), eq(EMPTY_STRING_ARRAY)); + verifyNoMoreInteractions(mNetworkManagementService); final LinkProperties cellLp = new LinkProperties(); - cellLp.setInterfaceName(IFNAME); + cellLp.setInterfaceName(MOBILE_IFNAME); // Add IPv4 and IPv6 default routes, because DNS-over-TLS code does // "is-reachable" testing in order to not program netd with unreachable // nameservers that it might try repeated to validate. cellLp.addLinkAddress(new LinkAddress("192.0.2.4/24")); - cellLp.addRoute(new RouteInfo((IpPrefix) null, InetAddress.getByName("192.0.2.4"), IFNAME)); + cellLp.addRoute(new RouteInfo((IpPrefix) null, InetAddress.getByName("192.0.2.4"), + MOBILE_IFNAME)); cellLp.addLinkAddress(new LinkAddress("2001:db8:1::1/64")); - cellLp.addRoute( - new RouteInfo((IpPrefix) null, InetAddress.getByName("2001:db8:1::1"), IFNAME)); + cellLp.addRoute(new RouteInfo((IpPrefix) null, InetAddress.getByName("2001:db8:1::1"), + MOBILE_IFNAME)); mCellNetworkAgent.sendLinkProperties(cellLp); mCellNetworkAgent.connect(false); waitForIdle(); - verify(mNetworkManagementService, times(1)).setDnsConfigurationForNetwork( - anyInt(), mStringArrayCaptor.capture(), any(), any(), - anyString(), eq(EMPTY_TLS_SERVERS)); // CS tells netd about the empty DNS config for this network. - assertEmpty(mStringArrayCaptor.getValue()); + verify(mNetworkManagementService, atLeastOnce()).setDnsConfigurationForNetwork( + anyInt(), eq(EMPTY_STRING_ARRAY), any(), any(), eq(""), eq(EMPTY_STRING_ARRAY)); reset(mNetworkManagementService); cellLp.addDnsServer(InetAddress.getByName("2001:db8::1")); mCellNetworkAgent.sendLinkProperties(cellLp); waitForIdle(); - verify(mNetworkManagementService, times(1)).setDnsConfigurationForNetwork( + verify(mNetworkManagementService, atLeastOnce()).setDnsConfigurationForNetwork( anyInt(), mStringArrayCaptor.capture(), any(), any(), - anyString(), eq(EMPTY_TLS_SERVERS)); + eq(""), tlsServers.capture()); assertEquals(1, mStringArrayCaptor.getValue().length); assertTrue(ArrayUtils.contains(mStringArrayCaptor.getValue(), "2001:db8::1")); + // Opportunistic mode. + assertTrue(ArrayUtils.contains(tlsServers.getValue(), "2001:db8::1")); reset(mNetworkManagementService); cellLp.addDnsServer(InetAddress.getByName("192.0.2.1")); mCellNetworkAgent.sendLinkProperties(cellLp); waitForIdle(); - verify(mNetworkManagementService, times(1)).setDnsConfigurationForNetwork( + verify(mNetworkManagementService, atLeastOnce()).setDnsConfigurationForNetwork( anyInt(), mStringArrayCaptor.capture(), any(), any(), - anyString(), eq(EMPTY_TLS_SERVERS)); + eq(""), tlsServers.capture()); assertEquals(2, mStringArrayCaptor.getValue().length); assertTrue(ArrayUtils.containsAll(mStringArrayCaptor.getValue(), new String[]{"2001:db8::1", "192.0.2.1"})); + // Opportunistic mode. + assertEquals(2, tlsServers.getValue().length); + assertTrue(ArrayUtils.containsAll(tlsServers.getValue(), + new String[]{"2001:db8::1", "192.0.2.1"})); reset(mNetworkManagementService); final String TLS_SPECIFIER = "tls.example.com"; @@ -3726,7 +3753,7 @@ public class ConnectivityServiceTest { mCellNetworkAgent.getNetwork().netId, new DnsManager.PrivateDnsConfig(TLS_SPECIFIER, TLS_IPS))); waitForIdle(); - verify(mNetworkManagementService, times(1)).setDnsConfigurationForNetwork( + verify(mNetworkManagementService, atLeastOnce()).setDnsConfigurationForNetwork( anyInt(), mStringArrayCaptor.capture(), any(), any(), eq(TLS_SPECIFIER), eq(TLS_SERVERS)); assertEquals(2, mStringArrayCaptor.getValue().length); @@ -3735,6 +3762,77 @@ public class ConnectivityServiceTest { reset(mNetworkManagementService); } + @Test + public void testPrivateDnsSettingsChange() throws Exception { + final String[] EMPTY_STRING_ARRAY = new String[0]; + ArgumentCaptor<String[]> tlsServers = ArgumentCaptor.forClass(String[].class); + + // Clear any interactions that occur as a result of CS starting up. + reset(mNetworkManagementService); + + // The default on Android is opportunistic mode ("Automatic"). + setPrivateDnsSettings(PRIVATE_DNS_MODE_OPPORTUNISTIC, "ignored.example.com"); + + mCellNetworkAgent = new MockNetworkAgent(TRANSPORT_CELLULAR); + waitForIdle(); + // CS tells netd about the empty DNS config for this network. + verify(mNetworkManagementService, never()).setDnsConfigurationForNetwork( + anyInt(), eq(EMPTY_STRING_ARRAY), any(), any(), eq(""), eq(EMPTY_STRING_ARRAY)); + verifyNoMoreInteractions(mNetworkManagementService); + + final LinkProperties cellLp = new LinkProperties(); + cellLp.setInterfaceName(MOBILE_IFNAME); + // Add IPv4 and IPv6 default routes, because DNS-over-TLS code does + // "is-reachable" testing in order to not program netd with unreachable + // nameservers that it might try repeated to validate. + cellLp.addLinkAddress(new LinkAddress("192.0.2.4/24")); + cellLp.addRoute(new RouteInfo((IpPrefix) null, InetAddress.getByName("192.0.2.4"), + MOBILE_IFNAME)); + cellLp.addLinkAddress(new LinkAddress("2001:db8:1::1/64")); + cellLp.addRoute(new RouteInfo((IpPrefix) null, InetAddress.getByName("2001:db8:1::1"), + MOBILE_IFNAME)); + cellLp.addDnsServer(InetAddress.getByName("2001:db8::1")); + cellLp.addDnsServer(InetAddress.getByName("192.0.2.1")); + + mCellNetworkAgent.sendLinkProperties(cellLp); + mCellNetworkAgent.connect(false); + waitForIdle(); + verify(mNetworkManagementService, atLeastOnce()).setDnsConfigurationForNetwork( + anyInt(), mStringArrayCaptor.capture(), any(), any(), + eq(""), tlsServers.capture()); + assertEquals(2, mStringArrayCaptor.getValue().length); + assertTrue(ArrayUtils.containsAll(mStringArrayCaptor.getValue(), + new String[]{"2001:db8::1", "192.0.2.1"})); + // Opportunistic mode. + assertEquals(2, tlsServers.getValue().length); + assertTrue(ArrayUtils.containsAll(tlsServers.getValue(), + new String[]{"2001:db8::1", "192.0.2.1"})); + reset(mNetworkManagementService); + + setPrivateDnsSettings(PRIVATE_DNS_MODE_OFF, "ignored.example.com"); + verify(mNetworkManagementService, times(1)).setDnsConfigurationForNetwork( + anyInt(), mStringArrayCaptor.capture(), any(), any(), + eq(""), eq(EMPTY_STRING_ARRAY)); + assertEquals(2, mStringArrayCaptor.getValue().length); + assertTrue(ArrayUtils.containsAll(mStringArrayCaptor.getValue(), + new String[]{"2001:db8::1", "192.0.2.1"})); + reset(mNetworkManagementService); + + setPrivateDnsSettings(PRIVATE_DNS_MODE_OPPORTUNISTIC, "ignored.example.com"); + verify(mNetworkManagementService, atLeastOnce()).setDnsConfigurationForNetwork( + anyInt(), mStringArrayCaptor.capture(), any(), any(), + eq(""), tlsServers.capture()); + assertEquals(2, mStringArrayCaptor.getValue().length); + assertTrue(ArrayUtils.containsAll(mStringArrayCaptor.getValue(), + new String[]{"2001:db8::1", "192.0.2.1"})); + assertEquals(2, tlsServers.getValue().length); + assertTrue(ArrayUtils.containsAll(tlsServers.getValue(), + new String[]{"2001:db8::1", "192.0.2.1"})); + reset(mNetworkManagementService); + + // Can't test strict mode without properly mocking out the DNS lookups. + } + private void checkDirectlyConnectedRoutes(Object callbackObj, Collection<LinkAddress> linkAddresses, Collection<RouteInfo> otherRoutes) { assertTrue(callbackObj instanceof LinkProperties); |