From 2c311d61eaf331818e601f97485f88c4cf26384d Mon Sep 17 00:00:00 2001 From: Paul Jensen Date: Mon, 17 Nov 2014 12:34:51 -0500 Subject: [PATCH] Don't bother validating networks that don't satisfy the default request. The only immediate change in behavior is not validating untrusted networks. bug:18299572 bug:18394654 Change-Id: I8d626baf37db0bd0f55ddf3af8a0abf094a12369 --- .../android/server/ConnectivityService.java | 5 ++- .../server/connectivity/NetworkAgentInfo.java | 4 +-- .../server/connectivity/NetworkMonitor.java | 33 ++++++++++++++----- 3 files changed, 30 insertions(+), 12 deletions(-) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 17889eae772a2..6d161a236befd 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -3615,6 +3615,7 @@ public class ConnectivityService extends IConnectivityManager.Stub private final HashMap mNetworkAgentInfos = new HashMap(); + // Note: if mDefaultRequest is changed, NetworkMonitor needs to be updated. private final NetworkRequest mDefaultRequest; private boolean isDefaultNetwork(NetworkAgentInfo nai) { @@ -3626,10 +3627,12 @@ public class ConnectivityService extends IConnectivityManager.Stub int currentScore, NetworkMisc networkMisc) { enforceConnectivityInternalPermission(); + // TODO: Instead of passing mDefaultRequest, provide an API to determine whether a Network + // satisfies mDefaultRequest. NetworkAgentInfo nai = new NetworkAgentInfo(messenger, new AsyncChannel(), new NetworkInfo(networkInfo), new LinkProperties(linkProperties), new NetworkCapabilities(networkCapabilities), currentScore, mContext, mTrackerHandler, - new NetworkMisc(networkMisc)); + new NetworkMisc(networkMisc), mDefaultRequest); synchronized (this) { nai.networkMonitor.systemReady = mSystemReady; } diff --git a/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java b/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java index 4cf2a4acb86ec..12da5c3c57e47 100644 --- a/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java +++ b/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java @@ -68,7 +68,7 @@ public class NetworkAgentInfo { public NetworkAgentInfo(Messenger messenger, AsyncChannel ac, NetworkInfo info, LinkProperties lp, NetworkCapabilities nc, int score, Context context, Handler handler, - NetworkMisc misc) { + NetworkMisc misc, NetworkRequest defaultRequest) { this.messenger = messenger; asyncChannel = ac; network = null; @@ -76,7 +76,7 @@ public class NetworkAgentInfo { linkProperties = lp; networkCapabilities = nc; currentScore = score; - networkMonitor = new NetworkMonitor(context, handler, this); + networkMonitor = new NetworkMonitor(context, handler, this, defaultRequest); networkMisc = misc; created = false; validated = false; diff --git a/services/core/java/com/android/server/connectivity/NetworkMonitor.java b/services/core/java/com/android/server/connectivity/NetworkMonitor.java index 593a28a02cc47..78f37055d460a 100644 --- a/services/core/java/com/android/server/connectivity/NetworkMonitor.java +++ b/services/core/java/com/android/server/connectivity/NetworkMonitor.java @@ -27,6 +27,7 @@ import android.net.ConnectivityManager; import android.net.Network; import android.net.NetworkCapabilities; import android.net.NetworkInfo; +import android.net.NetworkRequest; import android.net.TrafficStats; import android.net.wifi.WifiInfo; import android.net.wifi.WifiManager; @@ -222,6 +223,7 @@ public class NetworkMonitor extends StateMachine { private final TelephonyManager mTelephonyManager; private final WifiManager mWifiManager; private final AlarmManager mAlarmManager; + private final NetworkRequest mDefaultRequest; private String mServer; private boolean mIsCaptivePortalCheckEnabled = false; @@ -239,7 +241,8 @@ public class NetworkMonitor extends StateMachine { private State mCaptivePortalState = new CaptivePortalState(); private State mLingeringState = new LingeringState(); - public NetworkMonitor(Context context, Handler handler, NetworkAgentInfo networkAgentInfo) { + public NetworkMonitor(Context context, Handler handler, NetworkAgentInfo networkAgentInfo, + NetworkRequest defaultRequest) { // Add suffix indicating which NetworkMonitor we're talking about. super(TAG + networkAgentInfo.name()); @@ -249,6 +252,7 @@ public class NetworkMonitor extends StateMachine { mTelephonyManager = (TelephonyManager) context.getSystemService(Context.TELEPHONY_SERVICE); mWifiManager = (WifiManager) context.getSystemService(Context.WIFI_SERVICE); mAlarmManager = (AlarmManager) context.getSystemService(Context.ALARM_SERVICE); + mDefaultRequest = defaultRequest; addState(mDefaultState); addState(mOfflineState, mDefaultState); @@ -369,14 +373,25 @@ public class NetworkMonitor extends StateMachine { case CMD_REEVALUATE: if (message.arg1 != mReevaluateToken) return HANDLED; - if (mNetworkAgentInfo.isVPN()) { - transitionTo(mValidatedState); - return HANDLED; - } - // If network provides no internet connectivity adjust evaluation. - if (!mNetworkAgentInfo.networkCapabilities.hasCapability( - NetworkCapabilities.NET_CAPABILITY_INTERNET)) { - // TODO: Try to verify something works. Do all gateways respond to pings? + // Don't bother validating networks that don't satisify 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 + // 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 + // such networks (an app did) so the user may be unhappily surprised when + // asked to sign-in to a network they didn't want to connect to in the + // first place. Validation could be done to adjust the network scores + // however these networks are app-requested and may not be intended for + // general usage, in which case general validation may not be an accurate + // measure of the network's quality. Only the app knows how to evaluate + // the network so don't bother validating here. Furthermore sending HTTP + // packets over the network may be undesirable, for example an extremely + // expensive metered network, or unwanted leaking of the User Agent string. + if (!mDefaultRequest.networkCapabilities.satisfiedByNetworkCapabilities( + mNetworkAgentInfo.networkCapabilities)) { transitionTo(mValidatedState); return HANDLED; }