From 91aa5bc4630c02500698697d7350eafcc0bfedeb Mon Sep 17 00:00:00 2001 From: Remi NGUYEN VAN Date: Thu, 12 Dec 2019 12:57:11 +0900 Subject: [PATCH 1/2] Send LinkProperties update on new capport data When new CaptivePortalData is received from NetworkMonitor, send a LinkProperties updated callback. The updated LinkProperties only contain CaptivePortalData if the receiver has NETWORK_SETTINGS or MAINLINE_NETWORK_STACK permissions, as defined in the current callback code. Test: atest FrameworksNetTests Bug: 139269711 Change-Id: I68595a519171b31792259849efff5f58c43cacd4 --- .../android/server/ConnectivityService.java | 33 ++++++++++ .../server/connectivity/NetworkAgentInfo.java | 5 ++ .../server/ConnectivityServiceTest.java | 61 ++++++++++++++++--- 3 files changed, 91 insertions(+), 8 deletions(-) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 1b43fc0fa10d2..1f027d62c16d8 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -66,6 +66,7 @@ import android.content.IntentFilter; import android.content.res.Configuration; import android.database.ContentObserver; import android.net.CaptivePortal; +import android.net.CaptivePortalData; import android.net.ConnectionInfo; import android.net.ConnectivityDiagnosticsManager.ConnectivityReport; import android.net.ConnectivityDiagnosticsManager.DataStallReport; @@ -547,6 +548,14 @@ public class ConnectivityService extends IConnectivityManager.Stub */ public static final int EVENT_PROBE_STATUS_CHANGED = 46; + /** + * Event for NetworkMonitor to inform ConnectivityService that captive portal data has changed. + * arg1 = unused + * arg2 = netId + * obj = captive portal data + */ + private static final int EVENT_CAPPORT_DATA_CHANGED = 47; + /** * Argument for {@link #EVENT_PROVISIONING_NOTIFICATION} to indicate that the notification * should be shown. @@ -2817,6 +2826,12 @@ public class ConnectivityService extends IConnectivityManager.Stub updatePrivateDns(nai, (PrivateDnsConfig) msg.obj); break; } + case EVENT_CAPPORT_DATA_CHANGED: { + final NetworkAgentInfo nai = getNetworkAgentInfoForNetId(msg.arg2); + if (nai == null) break; + handleCaptivePortalDataUpdate(nai, (CaptivePortalData) msg.obj); + break; + } } return true; } @@ -2983,6 +2998,13 @@ public class ConnectivityService extends IConnectivityManager.Stub probesCompleted, probesSucceeded, new Integer(mNetId))); } + @Override + public void notifyCaptivePortalDataChanged(CaptivePortalData data) { + mTrackerHandler.sendMessage(mTrackerHandler.obtainMessage( + EVENT_CAPPORT_DATA_CHANGED, + 0, mNetId, data)); + } + @Override public void showProvisioningNotification(String action, String packageName) { final Intent intent = new Intent(action); @@ -3111,6 +3133,13 @@ public class ConnectivityService extends IConnectivityManager.Stub handleUpdateLinkProperties(nai, new LinkProperties(nai.linkProperties)); } + private void handleCaptivePortalDataUpdate(@NonNull final NetworkAgentInfo nai, + @Nullable final CaptivePortalData data) { + nai.captivePortalData = data; + // CaptivePortalData will be merged into LinkProperties from NetworkAgentInfo + handleUpdateLinkProperties(nai, new LinkProperties(nai.linkProperties)); + } + /** * Updates the linger state from the network requests inside the NAI. * @param nai the agent info to update @@ -5847,6 +5876,10 @@ public class ConnectivityService extends IConnectivityManager.Stub updateWakeOnLan(newLp); + // Captive portal data is obtained from NetworkMonitor and stored in NetworkAgentInfo, + // it is not contained in LinkProperties sent from NetworkAgents so needs to be merged here. + newLp.setCaptivePortalData(networkAgent.captivePortalData); + // TODO - move this check to cover the whole function if (!Objects.equals(newLp, oldLp)) { synchronized (networkAgent) { diff --git a/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java b/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java index d30a32041c628..58b5cba477dad 100644 --- a/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java +++ b/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java @@ -21,6 +21,7 @@ import static android.net.NetworkCapabilities.transportNamesOf; import android.annotation.NonNull; import android.annotation.Nullable; import android.content.Context; +import android.net.CaptivePortalData; import android.net.IDnsResolver; import android.net.INetd; import android.net.INetworkMonitor; @@ -167,6 +168,10 @@ public class NetworkAgentInfo implements Comparable { // Set to true when partial connectivity was detected. public boolean partialConnectivity; + // Captive portal info of the network, if any. + // Obtained by ConnectivityService and merged into NetworkAgent-provided information. + public CaptivePortalData captivePortalData; + // Networks are lingered when they become unneeded as a result of their NetworkRequests being // satisfied by a higher-scoring network. so as to allow communication to wrap up before the // network is taken down. This usually only happens to the default network. Lingering ends with diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java index f40e57fe46aa8..64591a3a87c1b 100644 --- a/tests/net/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java @@ -799,6 +799,14 @@ public class ConnectivityServiceTest { mProbesSucceeded = probesSucceeded; } + void notifyCaptivePortalDataChanged(CaptivePortalData data) { + try { + mNmCallbacks.notifyCaptivePortalDataChanged(data); + } catch (RemoteException e) { + throw new AssertionError("This cannot happen", e); + } + } + public String waitForRedirectUrl() { assertTrue(mNetworkStatusReceived.block(TIMEOUT_MS)); return mRedirectUrl; @@ -1845,18 +1853,21 @@ public class ConnectivityServiceTest { final Uri capportUrl = Uri.parse("https://capport.example.com/api"); final CaptivePortalData capportData = new CaptivePortalData.Builder() .setCaptive(true).build(); - newLp.setCaptivePortalApiUrl(capportUrl); - newLp.setCaptivePortalData(capportData); - mWiFiNetworkAgent.sendLinkProperties(newLp); final Uri expectedCapportUrl = sanitized ? null : capportUrl; - final CaptivePortalData expectedCapportData = sanitized ? null : capportData; + newLp.setCaptivePortalApiUrl(capportUrl); + mWiFiNetworkAgent.sendLinkProperties(newLp); callback.expectLinkPropertiesThat(mWiFiNetworkAgent, lp -> - Objects.equals(expectedCapportUrl, lp.getCaptivePortalApiUrl()) - && Objects.equals(expectedCapportData, lp.getCaptivePortalData())); + Objects.equals(expectedCapportUrl, lp.getCaptivePortalApiUrl())); defaultCallback.expectLinkPropertiesThat(mWiFiNetworkAgent, lp -> - Objects.equals(expectedCapportUrl, lp.getCaptivePortalApiUrl()) - && Objects.equals(expectedCapportData, lp.getCaptivePortalData())); + Objects.equals(expectedCapportUrl, lp.getCaptivePortalApiUrl())); + + final CaptivePortalData expectedCapportData = sanitized ? null : capportData; + mWiFiNetworkAgent.notifyCaptivePortalDataChanged(capportData); + callback.expectLinkPropertiesThat(mWiFiNetworkAgent, lp -> + Objects.equals(expectedCapportData, lp.getCaptivePortalData())); + defaultCallback.expectLinkPropertiesThat(mWiFiNetworkAgent, lp -> + Objects.equals(expectedCapportData, lp.getCaptivePortalData())); final LinkProperties lp = mCm.getLinkProperties(mWiFiNetworkAgent.getNetwork()); assertEquals(expectedCapportUrl, lp.getCaptivePortalApiUrl()); @@ -2810,6 +2821,40 @@ public class ConnectivityServiceTest { assertNoCallbacks(captivePortalCallback, validatedCallback); } + @Test + public void testCaptivePortalApi() throws Exception { + mServiceContext.setPermission( + android.Manifest.permission.NETWORK_SETTINGS, PERMISSION_GRANTED); + + final TestNetworkCallback captivePortalCallback = new TestNetworkCallback(); + final NetworkRequest captivePortalRequest = new NetworkRequest.Builder() + .addCapability(NET_CAPABILITY_CAPTIVE_PORTAL).build(); + mCm.registerNetworkCallback(captivePortalRequest, captivePortalCallback); + + mWiFiNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_WIFI); + final String redirectUrl = "http://example.com/firstPath"; + + mWiFiNetworkAgent.connectWithCaptivePortal(redirectUrl, false /* isStrictMode */); + captivePortalCallback.expectAvailableCallbacksUnvalidated(mWiFiNetworkAgent); + + final CaptivePortalData testData = new CaptivePortalData.Builder() + .setUserPortalUrl(Uri.parse(redirectUrl)) + .setBytesRemaining(12345L) + .build(); + + mWiFiNetworkAgent.notifyCaptivePortalDataChanged(testData); + + captivePortalCallback.expectLinkPropertiesThat(mWiFiNetworkAgent, + lp -> testData.equals(lp.getCaptivePortalData())); + + final LinkProperties newLps = new LinkProperties(); + newLps.setMtu(1234); + mWiFiNetworkAgent.sendLinkProperties(newLps); + // CaptivePortalData is not lost and unchanged when LPs are received from the NetworkAgent + captivePortalCallback.expectLinkPropertiesThat(mWiFiNetworkAgent, + lp -> testData.equals(lp.getCaptivePortalData()) && lp.getMtu() == 1234); + } + private NetworkRequest.Builder newWifiRequestBuilder() { return new NetworkRequest.Builder().addTransportType(TRANSPORT_WIFI); } From 763587e2f1ff86cb0f5bb3d4bad54cf20a614be3 Mon Sep 17 00:00:00 2001 From: Remi NGUYEN VAN Date: Thu, 24 Oct 2019 17:54:07 +0900 Subject: [PATCH 2/2] Add CaptivePortalApiUrl to DhcpResults The URL will be used by DhcpClient to return it in its results. It will not be parceled in DhcpResultsParcelable, but instead sent through LinkProperties to network agents. Bug: 139269711 Test: atest NetworkStackTests with associated NetworkStack change Change-Id: I4ec9e7f5efece3ede9b0da5eb1b75d8d43b94ba9 --- core/java/android/net/DhcpResults.java | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/core/java/android/net/DhcpResults.java b/core/java/android/net/DhcpResults.java index 059cd94accfeb..5ab035496a43c 100644 --- a/core/java/android/net/DhcpResults.java +++ b/core/java/android/net/DhcpResults.java @@ -16,6 +16,7 @@ package android.net; +import android.annotation.Nullable; import android.compat.annotation.UnsupportedAppUsage; import android.net.shared.InetAddressUtils; import android.os.Parcel; @@ -66,6 +67,9 @@ public final class DhcpResults implements Parcelable { public String serverHostName; + @Nullable + public String captivePortalApiUrl; + public DhcpResults() { super(); } @@ -100,6 +104,7 @@ public final class DhcpResults implements Parcelable { leaseDuration = source.leaseDuration; mtu = source.mtu; serverHostName = source.serverHostName; + captivePortalApiUrl = source.captivePortalApiUrl; } } @@ -133,6 +138,7 @@ public final class DhcpResults implements Parcelable { leaseDuration = 0; mtu = 0; serverHostName = null; + captivePortalApiUrl = null; } @Override @@ -144,6 +150,9 @@ public final class DhcpResults implements Parcelable { str.append(" lease ").append(leaseDuration).append(" seconds"); if (mtu != 0) str.append(" MTU ").append(mtu); str.append(" Servername ").append(serverHostName); + if (captivePortalApiUrl != null) { + str.append(" CaptivePortalApiUrl ").append(captivePortalApiUrl); + } return str.toString(); } @@ -161,7 +170,8 @@ public final class DhcpResults implements Parcelable { && Objects.equals(vendorInfo, target.vendorInfo) && Objects.equals(serverHostName, target.serverHostName) && leaseDuration == target.leaseDuration - && mtu == target.mtu; + && mtu == target.mtu + && Objects.equals(captivePortalApiUrl, target.captivePortalApiUrl); } /** @@ -186,6 +196,7 @@ public final class DhcpResults implements Parcelable { InetAddressUtils.parcelInetAddress(dest, serverAddress, flags); dest.writeString(vendorInfo); dest.writeString(serverHostName); + dest.writeString(captivePortalApiUrl); } @Override @@ -201,6 +212,7 @@ public final class DhcpResults implements Parcelable { dhcpResults.serverAddress = (Inet4Address) InetAddressUtils.unparcelInetAddress(in); dhcpResults.vendorInfo = in.readString(); dhcpResults.serverHostName = in.readString(); + dhcpResults.captivePortalApiUrl = in.readString(); return dhcpResults; } @@ -305,4 +317,12 @@ public final class DhcpResults implements Parcelable { public void setMtu(int mtu) { this.mtu = mtu; } + + public String getCaptivePortalApiUrl() { + return captivePortalApiUrl; + } + + public void setCaptivePortalApiUrl(String url) { + captivePortalApiUrl = url; + } }