diff --git a/Android.bp b/Android.bp index 4ba757a7ad6b2..9df0372b91530 100644 --- a/Android.bp +++ b/Android.bp @@ -186,6 +186,7 @@ java_defaults { "core/java/android/hardware/radio/ITunerCallback.aidl", "core/java/android/hardware/soundtrigger/IRecognitionStatusCallback.aidl", "core/java/android/hardware/usb/IUsbManager.aidl", + "core/java/android/net/ICaptivePortal.aidl", "core/java/android/net/IConnectivityManager.aidl", "core/java/android/net/IIpConnectivityMetrics.aidl", "core/java/android/net/IEthernetManager.aidl", @@ -815,7 +816,6 @@ aidl_interface { srcs: [ "core/java/android/net/ApfCapabilitiesParcelable.aidl", "core/java/android/net/DhcpResultsParcelable.aidl", - "core/java/android/net/ICaptivePortal.aidl", "core/java/android/net/INetworkMonitor.aidl", "core/java/android/net/INetworkMonitorCallbacks.aidl", "core/java/android/net/IIpMemoryStore.aidl", diff --git a/api/system-current.txt b/api/system-current.txt index 33cc4162ca549..8efc3de8b4b9e 100644 --- a/api/system-current.txt +++ b/api/system-current.txt @@ -3101,7 +3101,7 @@ package android.net { method @RequiresPermission(android.Manifest.permission.TETHER_PRIVILEGED) public void getLatestTetheringEntitlementValue(int, boolean, @NonNull android.net.ConnectivityManager.TetheringEntitlementValueListener, @Nullable android.os.Handler); method @RequiresPermission(anyOf={android.Manifest.permission.TETHER_PRIVILEGED, android.Manifest.permission.WRITE_SETTINGS}) public boolean isTetheringSupported(); method @RequiresPermission(anyOf={"android.permission.NETWORK_SETTINGS", android.Manifest.permission.NETWORK_SETUP_WIZARD, "android.permission.NETWORK_STACK"}) public void setAirplaneMode(boolean); - method @RequiresPermission(android.net.NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK) public void startCaptivePortalApp(android.os.Bundle); + method @RequiresPermission(android.net.NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK) public void startCaptivePortalApp(android.net.Network, android.os.Bundle); method @RequiresPermission(android.Manifest.permission.TETHER_PRIVILEGED) public void startTethering(int, boolean, android.net.ConnectivityManager.OnStartTetheringCallback); method @RequiresPermission(android.Manifest.permission.TETHER_PRIVILEGED) public void startTethering(int, boolean, android.net.ConnectivityManager.OnStartTetheringCallback, android.os.Handler); method @RequiresPermission(android.Manifest.permission.TETHER_PRIVILEGED) public void stopTethering(int); diff --git a/api/test-current.txt b/api/test-current.txt index 01127249c1fc5..aceddea67f67d 100644 --- a/api/test-current.txt +++ b/api/test-current.txt @@ -608,7 +608,7 @@ package android.net { } public class ConnectivityManager { - method @RequiresPermission(android.net.NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK) public void startCaptivePortalApp(android.os.Bundle); + method @RequiresPermission(android.net.NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK) public void startCaptivePortalApp(android.net.Network, android.os.Bundle); field public static final String EXTRA_CAPTIVE_PORTAL_PROBE_SPEC = "android.net.extra.CAPTIVE_PORTAL_PROBE_SPEC"; field public static final String EXTRA_CAPTIVE_PORTAL_USER_AGENT = "android.net.extra.CAPTIVE_PORTAL_USER_AGENT"; } diff --git a/core/java/android/net/ConnectivityManager.java b/core/java/android/net/ConnectivityManager.java index 68ac46c874ff8..99db040bbec67 100644 --- a/core/java/android/net/ConnectivityManager.java +++ b/core/java/android/net/ConnectivityManager.java @@ -3911,15 +3911,16 @@ public class ConnectivityManager { * *

This endpoint is exclusively for use by the NetworkStack and is protected by the * corresponding permission. + * @param network Network on which the captive portal was detected. * @param appExtras Extras to include in the app start intent. * @hide */ @SystemApi @TestApi @RequiresPermission(NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK) - public void startCaptivePortalApp(Bundle appExtras) { + public void startCaptivePortalApp(Network network, Bundle appExtras) { try { - mService.startCaptivePortalAppInternal(appExtras); + mService.startCaptivePortalAppInternal(network, appExtras); } catch (RemoteException e) { throw e.rethrowFromSystemServer(); } diff --git a/core/java/android/net/IConnectivityManager.aidl b/core/java/android/net/IConnectivityManager.aidl index 92a5839ac2af9..83bb3a0c8c154 100644 --- a/core/java/android/net/IConnectivityManager.aidl +++ b/core/java/android/net/IConnectivityManager.aidl @@ -168,7 +168,7 @@ interface IConnectivityManager void setAcceptUnvalidated(in Network network, boolean accept, boolean always); void setAvoidUnvalidated(in Network network); void startCaptivePortalApp(in Network network); - void startCaptivePortalAppInternal(in Bundle appExtras); + void startCaptivePortalAppInternal(in Network network, in Bundle appExtras); boolean getAvoidBadWifi(); int getMultipathPreference(in Network Network); diff --git a/core/java/android/net/INetworkMonitor.aidl b/core/java/android/net/INetworkMonitor.aidl index 41f969acad6fd..c94cdde15aa8c 100644 --- a/core/java/android/net/INetworkMonitor.aidl +++ b/core/java/android/net/INetworkMonitor.aidl @@ -34,6 +34,7 @@ oneway interface INetworkMonitor { void start(); void launchCaptivePortalApp(); + void notifyCaptivePortalAppFinished(int response); void forceReevaluation(int uid); void notifyPrivateDnsChanged(in PrivateDnsConfigParcel config); void notifyDnsResponse(int returnCode); diff --git a/core/java/android/net/INetworkMonitorCallbacks.aidl b/core/java/android/net/INetworkMonitorCallbacks.aidl index 5146585496516..2c61511feb725 100644 --- a/core/java/android/net/INetworkMonitorCallbacks.aidl +++ b/core/java/android/net/INetworkMonitorCallbacks.aidl @@ -26,5 +26,4 @@ oneway interface INetworkMonitorCallbacks { void notifyPrivateDnsConfigResolved(in PrivateDnsConfigParcel config); void showProvisioningNotification(String action, String packageName); void hideProvisioningNotification(); - void logCaptivePortalLoginEvent(int eventId, String packageName); } \ No newline at end of file diff --git a/packages/NetworkStack/src/com/android/server/NetworkStackService.java b/packages/NetworkStack/src/com/android/server/NetworkStackService.java index c6a207f265772..90db207c9902a 100644 --- a/packages/NetworkStack/src/com/android/server/NetworkStackService.java +++ b/packages/NetworkStack/src/com/android/server/NetworkStackService.java @@ -246,6 +246,12 @@ public class NetworkStackService extends Service { mNm.launchCaptivePortalApp(); } + @Override + public void notifyCaptivePortalAppFinished(int response) { + checkNetworkStackCallingPermission(); + mNm.notifyCaptivePortalAppFinished(response); + } + @Override public void forceReevaluation(int uid) { checkNetworkStackCallingPermission(); diff --git a/packages/NetworkStack/src/com/android/server/connectivity/NetworkMonitor.java b/packages/NetworkStack/src/com/android/server/connectivity/NetworkMonitor.java index 2e72d8296a373..0c159eafc319a 100644 --- a/packages/NetworkStack/src/com/android/server/connectivity/NetworkMonitor.java +++ b/packages/NetworkStack/src/com/android/server/connectivity/NetworkMonitor.java @@ -39,9 +39,7 @@ import android.content.BroadcastReceiver; import android.content.Context; import android.content.Intent; import android.content.IntentFilter; -import android.net.CaptivePortal; import android.net.ConnectivityManager; -import android.net.ICaptivePortal; import android.net.INetworkMonitor; import android.net.INetworkMonitorCallbacks; import android.net.LinkProperties; @@ -466,6 +464,13 @@ public class NetworkMonitor extends StateMachine { sendMessage(CMD_LAUNCH_CAPTIVE_PORTAL_APP); } + /** + * Notify that the captive portal app was closed with the provided response code. + */ + public void notifyCaptivePortalAppFinished(int response) { + sendMessage(CMD_CAPTIVE_PORTAL_APP_FINISHED, response); + } + @Override protected void log(String s) { if (DBG) Log.d(TAG + "/" + mNetwork.toString(), s); @@ -677,29 +682,8 @@ public class NetworkMonitor extends StateMachine { case CMD_LAUNCH_CAPTIVE_PORTAL_APP: final Bundle appExtras = new Bundle(); // OneAddressPerFamilyNetwork is not parcelable across processes. - appExtras.putParcelable( - ConnectivityManager.EXTRA_NETWORK, new Network(mNetwork)); - appExtras.putParcelable(ConnectivityManager.EXTRA_CAPTIVE_PORTAL, - new CaptivePortal(new ICaptivePortal.Stub() { - @Override - public void appResponse(int response) { - if (response == APP_RETURN_WANTED_AS_IS) { - mContext.enforceCallingPermission( - PERMISSION_NETWORK_SETTINGS, - "CaptivePortal"); - } - sendMessage(CMD_CAPTIVE_PORTAL_APP_FINISHED, response); - } - - @Override - public void logEvent(int eventId, String packageName) - throws RemoteException { - mContext.enforceCallingPermission( - PERMISSION_NETWORK_SETTINGS, - "CaptivePortal"); - mCallback.logCaptivePortalLoginEvent(eventId, packageName); - } - })); + final Network network = new Network(mNetwork); + appExtras.putParcelable(ConnectivityManager.EXTRA_NETWORK, network); final CaptivePortalProbeResult probeRes = mLastPortalProbeResult; appExtras.putString(EXTRA_CAPTIVE_PORTAL_URL, probeRes.detectUrl); if (probeRes.probeSpec != null) { @@ -708,7 +692,7 @@ public class NetworkMonitor extends StateMachine { } appExtras.putString(ConnectivityManager.EXTRA_CAPTIVE_PORTAL_USER_AGENT, mCaptivePortalUserAgent); - mCm.startCaptivePortalApp(appExtras); + mCm.startCaptivePortalApp(network, appExtras); return HANDLED; default: return NOT_HANDLED; diff --git a/packages/NetworkStack/tests/src/com/android/server/connectivity/NetworkMonitorTest.java b/packages/NetworkStack/tests/src/com/android/server/connectivity/NetworkMonitorTest.java index b98b0f798fe3d..9a16bb77182ec 100644 --- a/packages/NetworkStack/tests/src/com/android/server/connectivity/NetworkMonitorTest.java +++ b/packages/NetworkStack/tests/src/com/android/server/connectivity/NetworkMonitorTest.java @@ -16,8 +16,7 @@ package com.android.server.connectivity; -import static android.net.ConnectivityManager.ACTION_CAPTIVE_PORTAL_SIGN_IN; -import static android.net.ConnectivityManager.EXTRA_CAPTIVE_PORTAL; +import static android.net.CaptivePortal.APP_RETURN_DISMISSED; import static android.net.INetworkMonitor.NETWORK_TEST_RESULT_INVALID; import static android.net.INetworkMonitor.NETWORK_TEST_RESULT_VALID; import static android.net.NetworkCapabilities.NET_CAPABILITY_INTERNET; @@ -41,8 +40,6 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import android.content.Context; -import android.content.Intent; -import android.net.CaptivePortal; import android.net.ConnectivityManager; import android.net.INetworkMonitorCallbacks; import android.net.InetAddresses; @@ -54,10 +51,10 @@ import android.net.captiveportal.CaptivePortalProbeResult; import android.net.metrics.IpConnectivityLog; import android.net.util.SharedLog; import android.net.wifi.WifiManager; +import android.os.Bundle; import android.os.ConditionVariable; import android.os.Handler; import android.os.SystemClock; -import android.os.UserHandle; import android.provider.Settings; import android.support.test.filters.SmallTest; import android.support.test.runner.AndroidJUnit4; @@ -487,19 +484,23 @@ public class NetworkMonitorTest { // Check that startCaptivePortalApp sends the expected intent. nm.launchCaptivePortalApp(); - final ArgumentCaptor intentCaptor = ArgumentCaptor.forClass(Intent.class); - verify(mContext, timeout(HANDLER_TIMEOUT_MS).times(1)) - .startActivityAsUser(intentCaptor.capture(), eq(UserHandle.CURRENT)); - final Intent intent = intentCaptor.getValue(); - assertEquals(ACTION_CAPTIVE_PORTAL_SIGN_IN, intent.getAction()); - final Network network = intent.getParcelableExtra(ConnectivityManager.EXTRA_NETWORK); - assertEquals(TEST_NETID, network.netId); + final ArgumentCaptor bundleCaptor = ArgumentCaptor.forClass(Bundle.class); + final ArgumentCaptor networkCaptor = ArgumentCaptor.forClass(Network.class); + verify(mCm, timeout(HANDLER_TIMEOUT_MS).times(1)) + .startCaptivePortalApp(networkCaptor.capture(), bundleCaptor.capture()); + final Bundle bundle = bundleCaptor.getValue(); + final Network bundleNetwork = bundle.getParcelable(ConnectivityManager.EXTRA_NETWORK); + assertEquals(TEST_NETID, bundleNetwork.netId); + // network is passed both in bundle and as parameter, as the bundle is opaque to the + // framework and only intended for the captive portal app, but the framework needs + // the network to identify the right NetworkMonitor. + assertEquals(TEST_NETID, networkCaptor.getValue().netId); // Have the app report that the captive portal is dismissed, and check that we revalidate. setStatus(mHttpsConnection, 204); setStatus(mHttpConnection, 204); - final CaptivePortal captivePortal = intent.getParcelableExtra(EXTRA_CAPTIVE_PORTAL); - captivePortal.reportCaptivePortalDismissed(); + + nm.notifyCaptivePortalAppFinished(APP_RETURN_DISMISSED); verify(mCallbacks, timeout(HANDLER_TIMEOUT_MS).times(1)) .notifyNetworkTested(NETWORK_TEST_RESULT_VALID, null); } diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index af2bbfbb1dc3b..2cd25e443a20e 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -57,8 +57,10 @@ import android.content.Intent; import android.content.IntentFilter; import android.content.res.Configuration; import android.database.ContentObserver; +import android.net.CaptivePortal; import android.net.ConnectionInfo; import android.net.ConnectivityManager; +import android.net.ICaptivePortal; import android.net.IConnectivityManager; import android.net.IIpConnectivityMetrics; import android.net.INetd; @@ -2689,11 +2691,6 @@ public class ConnectivityService extends IConnectivityManager.Stub EVENT_PROVISIONING_NOTIFICATION, PROVISIONING_NOTIFICATION_HIDE, mNai.network.netId)); } - - @Override - public void logCaptivePortalLoginEvent(int eventId, String packageName) { - new MetricsLogger().action(eventId, packageName); - } } private boolean networkRequiresValidation(NetworkAgentInfo nai) { @@ -3237,22 +3234,63 @@ public class ConnectivityService extends IConnectivityManager.Stub /** * NetworkStack endpoint to start the captive portal app. The NetworkStack needs to use this * endpoint as it does not have INTERACT_ACROSS_USERS_FULL itself. + * @param network Network on which the captive portal was detected. * @param appExtras Bundle to use as intent extras for the captive portal application. * Must be treated as opaque to avoid preventing the captive portal app to * update its arguments. */ @Override - public void startCaptivePortalAppInternal(Bundle appExtras) { + public void startCaptivePortalAppInternal(Network network, Bundle appExtras) { mContext.checkCallingOrSelfPermission(NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK); final Intent appIntent = new Intent(ConnectivityManager.ACTION_CAPTIVE_PORTAL_SIGN_IN); appIntent.putExtras(appExtras); + appIntent.putExtra(ConnectivityManager.EXTRA_CAPTIVE_PORTAL, + new CaptivePortal(new CaptivePortalImpl(network).asBinder())); appIntent.setFlags(Intent.FLAG_ACTIVITY_BROUGHT_TO_FRONT | Intent.FLAG_ACTIVITY_NEW_TASK); Binder.withCleanCallingIdentity(() -> mContext.startActivityAsUser(appIntent, UserHandle.CURRENT)); } + private class CaptivePortalImpl extends ICaptivePortal.Stub { + private final Network mNetwork; + + private CaptivePortalImpl(Network network) { + mNetwork = network; + } + + @Override + public void appResponse(final int response) throws RemoteException { + if (response == CaptivePortal.APP_RETURN_WANTED_AS_IS) { + enforceSettingsPermission(); + } + + // getNetworkAgentInfoForNetwork is thread-safe + final NetworkAgentInfo nai = getNetworkAgentInfoForNetwork(mNetwork); + if (nai == null) return; + + // nai.networkMonitor() is thread-safe + final INetworkMonitor nm = nai.networkMonitor(); + if (nm == null) return; + + final long token = Binder.clearCallingIdentity(); + try { + nm.notifyCaptivePortalAppFinished(response); + } finally { + // Not using Binder.withCleanCallingIdentity() to keep the checked RemoteException + Binder.restoreCallingIdentity(token); + } + } + + @Override + public void logEvent(int eventId, String packageName) { + enforceSettingsPermission(); + + new MetricsLogger().action(eventId, packageName); + } + } + public boolean avoidBadWifi() { return mMultinetworkPolicyTracker.getAvoidBadWifi(); }