diff --git a/service/src/com/android/server/ConnectivityService.java b/service/src/com/android/server/ConnectivityService.java index 04ed3c94d7..1a9ff25f33 100644 --- a/service/src/com/android/server/ConnectivityService.java +++ b/service/src/com/android/server/ConnectivityService.java @@ -55,6 +55,7 @@ import static android.net.ConnectivityManager.isNetworkTypeValid; import static android.net.ConnectivitySettingsManager.PRIVATE_DNS_MODE_OPPORTUNISTIC; import static android.net.INetworkMonitor.NETWORK_VALIDATION_PROBE_PRIVDNS; 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.NetworkCapabilities.NET_CAPABILITY_CAPTIVE_PORTAL; import static android.net.NetworkCapabilities.NET_CAPABILITY_ENTERPRISE; @@ -541,9 +542,9 @@ public class ConnectivityService extends IConnectivityManager.Stub private static final int EVENT_SET_AVOID_UNVALIDATED = 35; /** - * used to trigger revalidation of a network. + * used to handle reported network connectivity. May trigger revalidation of a network. */ - private static final int EVENT_REVALIDATE_NETWORK = 36; + private static final int EVENT_REPORT_NETWORK_CONNECTIVITY = 36; // Handle changes in Private DNS settings. private static final int EVENT_PRIVATE_DNS_SETTINGS_CHANGED = 37; @@ -3541,8 +3542,14 @@ public class ConnectivityService extends IConnectivityManager.Stub final NetworkAgentInfo nai = getNetworkAgentInfoForNetId(mNetId); if (nai == null) return; + // NetworkMonitor reports the network validation result as a bitmask while + // ConnectivityDiagnostics treats this value as an int. Convert the result to a single + // logical value for ConnectivityDiagnostics. + final int validationResult = networkMonitorValidationResultToConnDiagsValidationResult( + p.result); + final PersistableBundle extras = new PersistableBundle(); - extras.putInt(KEY_NETWORK_VALIDATION_RESULT, p.result); + extras.putInt(KEY_NETWORK_VALIDATION_RESULT, validationResult); extras.putInt(KEY_NETWORK_PROBES_SUCCEEDED_BITMASK, p.probesSucceeded); extras.putInt(KEY_NETWORK_PROBES_ATTEMPTED_BITMASK, p.probesAttempted); @@ -3618,6 +3625,22 @@ public class ConnectivityService extends IConnectivityManager.Stub } } + /** + * Converts the given NetworkMonitor-specific validation result bitmask to a + * ConnectivityDiagnostics-specific validation result int. + */ + private int networkMonitorValidationResultToConnDiagsValidationResult(int validationResult) { + if ((validationResult & NETWORK_VALIDATION_RESULT_SKIPPED) != 0) { + return ConnectivityReport.NETWORK_VALIDATION_RESULT_SKIPPED; + } + if ((validationResult & NETWORK_VALIDATION_RESULT_VALID) == 0) { + return ConnectivityReport.NETWORK_VALIDATION_RESULT_INVALID; + } + return (validationResult & NETWORK_VALIDATION_RESULT_PARTIAL) != 0 + ? ConnectivityReport.NETWORK_VALIDATION_RESULT_PARTIALLY_VALID + : ConnectivityReport.NETWORK_VALIDATION_RESULT_VALID; + } + private void notifyDataStallSuspected(DataStallReportParcelable p, int netId) { log("Data stall detected with methods: " + p.detectionMethod); @@ -4867,8 +4890,9 @@ public class ConnectivityService extends IConnectivityManager.Stub mKeepaliveTracker.handleStopKeepalive(nai, slot, reason); break; } - case EVENT_REVALIDATE_NETWORK: { - handleReportNetworkConnectivity((Network) msg.obj, msg.arg1, toBool(msg.arg2)); + case EVENT_REPORT_NETWORK_CONNECTIVITY: { + handleReportNetworkConnectivity((NetworkAgentInfo) msg.obj, msg.arg1, + toBool(msg.arg2)); break; } case EVENT_PRIVATE_DNS_SETTINGS_CHANGED: @@ -5031,41 +5055,34 @@ public class ConnectivityService extends IConnectivityManager.Stub final int uid = mDeps.getCallingUid(); final int connectivityInfo = encodeBool(hasConnectivity); - // Handle ConnectivityDiagnostics event before attempting to revalidate the network. This - // forces an ordering of ConnectivityDiagnostics events in the case where hasConnectivity - // does not match the known connectivity of the network - this causes NetworkMonitor to - // revalidate the network and generate a ConnectivityDiagnostics ConnectivityReport event. final NetworkAgentInfo nai; if (network == null) { nai = getDefaultNetwork(); } else { nai = getNetworkAgentInfoForNetwork(network); } - if (nai != null) { - mConnectivityDiagnosticsHandler.sendMessage( - mConnectivityDiagnosticsHandler.obtainMessage( - ConnectivityDiagnosticsHandler.EVENT_NETWORK_CONNECTIVITY_REPORTED, - connectivityInfo, 0, nai)); - } mHandler.sendMessage( - mHandler.obtainMessage(EVENT_REVALIDATE_NETWORK, uid, connectivityInfo, network)); + mHandler.obtainMessage( + EVENT_REPORT_NETWORK_CONNECTIVITY, uid, connectivityInfo, nai)); } private void handleReportNetworkConnectivity( - Network network, int uid, boolean hasConnectivity) { - final NetworkAgentInfo nai; - if (network == null) { - nai = getDefaultNetwork(); - } else { - nai = getNetworkAgentInfoForNetwork(network); - } - if (nai == null || nai.networkInfo.getState() == NetworkInfo.State.DISCONNECTING || - nai.networkInfo.getState() == NetworkInfo.State.DISCONNECTED) { + @Nullable NetworkAgentInfo nai, int uid, boolean hasConnectivity) { + // TODO(b/192611346): remove NetworkInfo.State.DISCONNECTING as it's not used + if (nai == null + || nai != getNetworkAgentInfoForNetwork(nai.network) + || nai.networkInfo.getState() == NetworkInfo.State.DISCONNECTING + || nai.networkInfo.getState() == NetworkInfo.State.DISCONNECTED) { return; } // Revalidate if the app report does not match our current validated state. if (hasConnectivity == nai.lastValidated) { + mConnectivityDiagnosticsHandler.sendMessage( + mConnectivityDiagnosticsHandler.obtainMessage( + ConnectivityDiagnosticsHandler.EVENT_NETWORK_CONNECTIVITY_REPORTED, + new ReportedNetworkConnectivityInfo( + hasConnectivity, false /* isNetworkRevalidating */, uid, nai))); return; } if (DBG) { @@ -5081,6 +5098,16 @@ public class ConnectivityService extends IConnectivityManager.Stub if (isNetworkWithCapabilitiesBlocked(nc, uid, false)) { return; } + + // Send CONNECTIVITY_REPORTED event before re-validating the Network to force an ordering of + // ConnDiags events. This ensures that #onNetworkConnectivityReported() will be called + // before #onConnectivityReportAvailable(), which is called once Network evaluation is + // completed. + mConnectivityDiagnosticsHandler.sendMessage( + mConnectivityDiagnosticsHandler.obtainMessage( + ConnectivityDiagnosticsHandler.EVENT_NETWORK_CONNECTIVITY_REPORTED, + new ReportedNetworkConnectivityInfo( + hasConnectivity, true /* isNetworkRevalidating */, uid, nai))); nai.networkMonitor().forceReevaluation(uid); } @@ -9040,8 +9067,7 @@ public class ConnectivityService extends IConnectivityManager.Stub * the platform. This event will invoke {@link * IConnectivityDiagnosticsCallback#onNetworkConnectivityReported} for permissioned * callbacks. - * obj = Network that was reported on - * arg1 = boolint for the quality reported + * obj = ReportedNetworkConnectivityInfo with info on reported Network connectivity. */ private static final int EVENT_NETWORK_CONNECTIVITY_REPORTED = 5; @@ -9079,7 +9105,7 @@ public class ConnectivityService extends IConnectivityManager.Stub break; } case EVENT_NETWORK_CONNECTIVITY_REPORTED: { - handleNetworkConnectivityReported((NetworkAgentInfo) msg.obj, toBool(msg.arg1)); + handleNetworkConnectivityReported((ReportedNetworkConnectivityInfo) msg.obj); break; } default: { @@ -9149,6 +9175,28 @@ public class ConnectivityService extends IConnectivityManager.Stub } } + /** + * Class used for sending info for a call to {@link #reportNetworkConnectivity()} to {@link + * ConnectivityDiagnosticsHandler}. + */ + private static class ReportedNetworkConnectivityInfo { + public final boolean hasConnectivity; + public final boolean isNetworkRevalidating; + public final int reporterUid; + @NonNull public final NetworkAgentInfo nai; + + private ReportedNetworkConnectivityInfo( + boolean hasConnectivity, + boolean isNetworkRevalidating, + int reporterUid, + @NonNull NetworkAgentInfo nai) { + this.hasConnectivity = hasConnectivity; + this.isNetworkRevalidating = isNetworkRevalidating; + this.reporterUid = reporterUid; + this.nai = nai; + } + } + private void handleRegisterConnectivityDiagnosticsCallback( @NonNull ConnectivityDiagnosticsCallbackInfo cbInfo) { ensureRunningOnConnectivityServiceThread(); @@ -9256,13 +9304,14 @@ public class ConnectivityService extends IConnectivityManager.Stub networkCapabilities, extras); nai.setConnectivityReport(report); + final List results = - getMatchingPermissionedCallbacks(nai); + getMatchingPermissionedCallbacks(nai, Process.INVALID_UID); for (final IConnectivityDiagnosticsCallback cb : results) { try { cb.onConnectivityReportAvailable(report); } catch (RemoteException ex) { - loge("Error invoking onConnectivityReport", ex); + loge("Error invoking onConnectivityReportAvailable", ex); } } } @@ -9281,7 +9330,7 @@ public class ConnectivityService extends IConnectivityManager.Stub networkCapabilities, extras); final List results = - getMatchingPermissionedCallbacks(nai); + getMatchingPermissionedCallbacks(nai, Process.INVALID_UID); for (final IConnectivityDiagnosticsCallback cb : results) { try { cb.onDataStallSuspected(report); @@ -9292,15 +9341,39 @@ public class ConnectivityService extends IConnectivityManager.Stub } private void handleNetworkConnectivityReported( - @NonNull NetworkAgentInfo nai, boolean connectivity) { + @NonNull ReportedNetworkConnectivityInfo reportedNetworkConnectivityInfo) { + final NetworkAgentInfo nai = reportedNetworkConnectivityInfo.nai; + final ConnectivityReport cachedReport = nai.getConnectivityReport(); + + // If the Network is being re-validated as a result of this call to + // reportNetworkConnectivity(), notify all permissioned callbacks. Otherwise, only notify + // permissioned callbacks registered by the reporter. final List results = - getMatchingPermissionedCallbacks(nai); + getMatchingPermissionedCallbacks( + nai, + reportedNetworkConnectivityInfo.isNetworkRevalidating + ? Process.INVALID_UID + : reportedNetworkConnectivityInfo.reporterUid); + for (final IConnectivityDiagnosticsCallback cb : results) { try { - cb.onNetworkConnectivityReported(nai.network, connectivity); + cb.onNetworkConnectivityReported( + nai.network, reportedNetworkConnectivityInfo.hasConnectivity); } catch (RemoteException ex) { loge("Error invoking onNetworkConnectivityReported", ex); } + + // If the Network isn't re-validating, also provide the cached report. If there is no + // cached report, the Network is still being validated and a report will be sent once + // validation is complete. Note that networks which never undergo validation will still + // have a cached ConnectivityReport with RESULT_SKIPPED. + if (!reportedNetworkConnectivityInfo.isNetworkRevalidating && cachedReport != null) { + try { + cb.onConnectivityReportAvailable(cachedReport); + } catch (RemoteException ex) { + loge("Error invoking onConnectivityReportAvailable", ex); + } + } } } @@ -9313,20 +9386,38 @@ public class ConnectivityService extends IConnectivityManager.Stub return sanitized; } + /** + * Gets a list of ConnectivityDiagnostics callbacks that match the specified Network and uid. + * + *

If Process.INVALID_UID is specified, all matching callbacks will be returned. + */ private List getMatchingPermissionedCallbacks( - @NonNull NetworkAgentInfo nai) { + @NonNull NetworkAgentInfo nai, int uid) { final List results = new ArrayList<>(); for (Entry entry : mConnectivityDiagnosticsCallbacks.entrySet()) { final ConnectivityDiagnosticsCallbackInfo cbInfo = entry.getValue(); final NetworkRequestInfo nri = cbInfo.mRequestInfo; + // Connectivity Diagnostics rejects multilayer requests at registration hence get(0). - if (nai.satisfies(nri.mRequests.get(0))) { - if (checkConnectivityDiagnosticsPermissions( - nri.mPid, nri.mUid, nai, cbInfo.mCallingPackageName)) { - results.add(entry.getValue().mCb); - } + if (!nai.satisfies(nri.mRequests.get(0))) { + continue; } + + // UID for this callback must either be: + // - INVALID_UID (which sends callbacks to all UIDs), or + // - The callback's owner (the owner called reportNetworkConnectivity() and is being + // notified as a result) + if (uid != Process.INVALID_UID && uid != nri.mUid) { + continue; + } + + if (!checkConnectivityDiagnosticsPermissions( + nri.mPid, nri.mUid, nai, cbInfo.mCallingPackageName)) { + continue; + } + + results.add(entry.getValue().mCb); } return results; } diff --git a/tests/unit/java/com/android/server/ConnectivityServiceTest.java b/tests/unit/java/com/android/server/ConnectivityServiceTest.java index e494a039d5..6d7d3d2abe 100644 --- a/tests/unit/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/unit/java/com/android/server/ConnectivityServiceTest.java @@ -10221,6 +10221,9 @@ public class ConnectivityServiceTest { public void testConnectivityDiagnosticsCallbackOnConnectivityReported() throws Exception { setUpConnectivityDiagnosticsCallback(); + // reset to ignore callbacks from setup + reset(mConnectivityDiagnosticsCallback); + final Network n = mCellNetworkAgent.getNetwork(); final boolean hasConnectivity = true; mService.reportNetworkConnectivity(n, hasConnectivity); @@ -10231,6 +10234,8 @@ public class ConnectivityServiceTest { // Verify onNetworkConnectivityReported fired verify(mConnectivityDiagnosticsCallback) .onNetworkConnectivityReported(eq(n), eq(hasConnectivity)); + verify(mConnectivityDiagnosticsCallback).onConnectivityReportAvailable( + argThat(report -> areConnDiagCapsRedacted(report.getNetworkCapabilities()))); final boolean noConnectivity = false; mService.reportNetworkConnectivity(n, noConnectivity); @@ -10241,6 +10246,54 @@ public class ConnectivityServiceTest { // Wait for onNetworkConnectivityReported to fire verify(mConnectivityDiagnosticsCallback) .onNetworkConnectivityReported(eq(n), eq(noConnectivity)); + + // Also expect a ConnectivityReport after NetworkMonitor asynchronously re-validates + verify(mConnectivityDiagnosticsCallback, timeout(TIMEOUT_MS).times(2)) + .onConnectivityReportAvailable( + argThat(report -> + areConnDiagCapsRedacted(report.getNetworkCapabilities()))); + } + + @Test + public void testConnectivityDiagnosticsCallbackOnConnectivityReportedSeparateUid() + throws Exception { + setUpConnectivityDiagnosticsCallback(); + + // reset to ignore callbacks from setup + reset(mConnectivityDiagnosticsCallback); + + // report known Connectivity from a different uid. Verify that network is not re-validated + // and this callback is not notified. + final Network n = mCellNetworkAgent.getNetwork(); + final boolean hasConnectivity = true; + doAsUid(Process.myUid() + 1, () -> mService.reportNetworkConnectivity(n, hasConnectivity)); + + // Block until all other events are done processing. + HandlerUtils.waitForIdle(mCsHandlerThread, TIMEOUT_MS); + + // Verify onNetworkConnectivityReported did not fire + verify(mConnectivityDiagnosticsCallback, never()) + .onNetworkConnectivityReported(any(), anyBoolean()); + verify(mConnectivityDiagnosticsCallback, never()) + .onConnectivityReportAvailable(any()); + + // report different Connectivity from a different uid. Verify that network is re-validated + // and that this callback is notified. + final boolean noConnectivity = false; + doAsUid(Process.myUid() + 1, () -> mService.reportNetworkConnectivity(n, noConnectivity)); + + // Block until all other events are done processing. + HandlerUtils.waitForIdle(mCsHandlerThread, TIMEOUT_MS); + + // Wait for onNetworkConnectivityReported to fire + verify(mConnectivityDiagnosticsCallback, timeout(TIMEOUT_MS)) + .onNetworkConnectivityReported(eq(n), eq(noConnectivity)); + + // Also expect a ConnectivityReport after NetworkMonitor asynchronously re-validates + verify(mConnectivityDiagnosticsCallback, timeout(TIMEOUT_MS)) + .onConnectivityReportAvailable( + argThat(report -> + areConnDiagCapsRedacted(report.getNetworkCapabilities()))); } @Test