diff options
author | Roshan Pius <rpius@google.com> | 2020-05-11 15:07:05 -0700 |
---|---|---|
committer | Roshan Pius <rpius@google.com> | 2020-05-13 09:51:05 -0700 |
commit | fdec8fe5462205c8ecfd55c015d0f0773d2ae15a (patch) | |
tree | f3fc9aaf61a88d0bebfa3145a70e0d8790efe66d | |
parent | 61c22b90db0d4be53a23ca85938ea014e388bb2d (diff) |
WifiNetworkFactory: Trigger connection immediately if pre-approved
If a network is pre-approved for an app (either by the user or via
CompanionDeviceManager), we currently issue a scan to ensure that the
network is around if there is no fresh cached results. This causes us
to bring up the UI (since we don't want to scan on behalf of the apps
without UI attribution). Instead, if the network requested was already
pre-approved, directly trigger a connection. This will ensure that the
platform does not need to trigger a scan for finding the requested
network. So, there is no need to bring up the UI for apps triggering
connection to networks which have been pre-approved
Bug: 155218555
Test: atest com.android.server.wifi
Test: act.py -c wifi_manager_cross.config -tb dut-name -tc
WifiNetworkRequestTest
Change-Id: If1baf014bf8b5e8cbfa326660b9a401486d7bab6
-rw-r--r-- | service/java/com/android/server/wifi/WifiNetworkFactory.java | 141 | ||||
-rw-r--r-- | tests/wifitests/src/com/android/server/wifi/WifiNetworkFactoryTest.java | 139 |
2 files changed, 78 insertions, 202 deletions
diff --git a/service/java/com/android/server/wifi/WifiNetworkFactory.java b/service/java/com/android/server/wifi/WifiNetworkFactory.java index b47afc851..98ba5bc94 100644 --- a/service/java/com/android/server/wifi/WifiNetworkFactory.java +++ b/service/java/com/android/server/wifi/WifiNetworkFactory.java @@ -58,7 +58,6 @@ import android.util.Pair; import com.android.internal.annotations.VisibleForTesting; import com.android.server.wifi.proto.nano.WifiMetricsProto; -import com.android.server.wifi.util.ArrayUtils; import com.android.server.wifi.util.ExternalCallbackTracker; import com.android.server.wifi.util.ScanResultUtil; import com.android.server.wifi.util.WifiPermissionsUtil; @@ -236,12 +235,9 @@ public class WifiNetworkFactory extends NetworkFactory { if (mVerboseLoggingEnabled) { Log.v(TAG, "Received " + scanResults.length + " scan results"); } - if (!handleScanResultsAndTriggerConnectIfUserApprovedMatchFound(scanResults)) { - // Didn't find an approved match, send the matching results to UI and schedule the - // next scan. - sendNetworkRequestMatchCallbacksForActiveRequest(mActiveMatchedScanResults); - scheduleNextPeriodicScan(); - } + handleScanResults(scanResults); + sendNetworkRequestMatchCallbacksForActiveRequest(mActiveMatchedScanResults); + scheduleNextPeriodicScan(); } @Override @@ -601,16 +597,17 @@ public class WifiNetworkFactory extends NetworkFactory { wns.ssidPatternMatcher, wns.bssidPatternMatcher, wns.wifiConfiguration); mWifiMetrics.incrementNetworkRequestApiNumRequest(); - // Fetch the latest cached scan results to speed up network matching. - ScanResult[] cachedScanResults = getFilteredCachedScanResults(); - if (mVerboseLoggingEnabled) { - Log.v(TAG, "Using cached " + cachedScanResults.length + " scan results"); - } - if (!handleScanResultsAndTriggerConnectIfUserApprovedMatchFound(cachedScanResults)) { + if (!triggerConnectIfUserApprovedMatchFound()) { // Start UI to let the user grant/disallow this request from the app. startUi(); // Didn't find an approved match, send the matching results to UI and trigger // periodic scans for finding a network in the request. + // Fetch the latest cached scan results to speed up network matching. + ScanResult[] cachedScanResults = getFilteredCachedScanResults(); + if (mVerboseLoggingEnabled) { + Log.v(TAG, "Using cached " + cachedScanResults.length + " scan results"); + } + handleScanResults(cachedScanResults); sendNetworkRequestMatchCallbacksForActiveRequest(mActiveMatchedScanResults); startPeriodicScans(); } @@ -790,9 +787,15 @@ public class WifiNetworkFactory extends NetworkFactory { new WifiConfiguration(mActiveSpecificNetworkRequestSpecifier.wifiConfiguration); networkToConnect.SSID = network.SSID; // Set the WifiConfiguration.BSSID field to prevent roaming. - networkToConnect.BSSID = - findBestBssidFromActiveMatchedScanResultsForNetwork( - ScanResultMatchInfo.fromWifiConfiguration(networkToConnect)); + if (network.BSSID != null) { + // If pre-approved, use the bssid from the request. + networkToConnect.BSSID = network.BSSID; + } else { + // If not pre-approved, find the best bssid matching the request. + networkToConnect.BSSID = + findBestBssidFromActiveMatchedScanResultsForNetwork( + ScanResultMatchInfo.fromWifiConfiguration(networkToConnect)); + } networkToConnect.ephemeral = true; // Mark it user private to avoid conflicting with any saved networks the user might have. // TODO (b/142035508): Use a more generic mechanism to fix this. @@ -1269,68 +1272,63 @@ public class WifiNetworkFactory extends NetworkFactory { } private boolean isAccessPointApprovedInInternalApprovalList( - @NonNull ScanResult scanResult, @NonNull String requestorPackageName) { + @NonNull String ssid, @NonNull MacAddress bssid, @SecurityType int networkType, + @NonNull String requestorPackageName) { Set<AccessPoint> approvedAccessPoints = mUserApprovedAccessPointMap.get(requestorPackageName); if (approvedAccessPoints == null) return false; - ScanResultMatchInfo fromScanResult = ScanResultMatchInfo.fromScanResult(scanResult); AccessPoint accessPoint = - new AccessPoint(scanResult.SSID, - MacAddress.fromString(scanResult.BSSID), fromScanResult.networkType); + new AccessPoint(ssid, bssid, networkType); if (!approvedAccessPoints.contains(accessPoint)) return false; // keep the most recently used AP in the end approvedAccessPoints.remove(accessPoint); approvedAccessPoints.add(accessPoint); if (mVerboseLoggingEnabled) { - Log.v(TAG, "Found " + scanResult + Log.v(TAG, "Found " + bssid + " in internal user approved access point for " + requestorPackageName); } return true; } private boolean isAccessPointApprovedInCompanionDeviceManager( - @NonNull ScanResult scanResult, @NonNull UserHandle requestorUserHandle, + @NonNull MacAddress bssid, + @NonNull UserHandle requestorUserHandle, @NonNull String requestorPackageName) { if (mCompanionDeviceManager == null) { mCompanionDeviceManager = mContext.getSystemService(CompanionDeviceManager.class); } boolean approved = mCompanionDeviceManager.isDeviceAssociatedForWifiConnection( - requestorPackageName, MacAddress.fromString(scanResult.BSSID), requestorUserHandle); + requestorPackageName, bssid, requestorUserHandle); if (!approved) return false; if (mVerboseLoggingEnabled) { - Log.v(TAG, "Found " + scanResult - + " in CDM user approved access point for " + requestorPackageName); + Log.v(TAG, "Found " + bssid + + " in CompanionDeviceManager approved access point for " + + requestorPackageName); } return true; } - private @Nullable ScanResult - findUserApprovedAccessPointForActiveRequestFromActiveMatchedScanResults() { - if (mActiveSpecificNetworkRequestSpecifier == null - || ArrayUtils.size(mActiveMatchedScanResults) != 1) { - return null; - } - // There should only 1 matched scan result since the request contains a specific - // SSID + BSSID mentioned. - ScanResult scanResult = mActiveMatchedScanResults.get(0); + private boolean isAccessPointApprovedForActiveRequest(@NonNull String ssid, + @NonNull MacAddress bssid, @SecurityType int networkType) { String requestorPackageName = mActiveSpecificNetworkRequest.getRequestorPackageName(); UserHandle requestorUserHandle = UserHandle.getUserHandleForUid(mActiveSpecificNetworkRequest.getRequestorUid()); - // Check if access point is approved via CDM first. + // Check if access point is approved via CompanionDeviceManager first. if (isAccessPointApprovedInCompanionDeviceManager( - scanResult, requestorUserHandle, requestorPackageName)) { - return scanResult; + bssid, requestorUserHandle, requestorPackageName)) { + return true; } // Check if access point is approved in internal approval list next. - if (isAccessPointApprovedInInternalApprovalList(scanResult, requestorPackageName)) { - return scanResult; + if (isAccessPointApprovedInInternalApprovalList( + ssid, bssid, networkType, requestorPackageName)) { + return true; } // Shell approved app if (TextUtils.equals(mApprovedApp, requestorPackageName)) { - return scanResult; + return true; } // no bypass approvals, show UI. - return null; + return false; } @@ -1378,17 +1376,44 @@ public class WifiNetworkFactory extends NetworkFactory { } /** - * Handle scan results - * a) Find all scan results matching the active network request. - * b) If the request is for a single bssid, check if the matching ScanResult was pre-approved + * 1) If the request is for a single bssid, check if the matching ScanResult was pre-approved * by the user. - * c) If yes to (b), trigger a connect immediately and returns true. Else, returns false. + * 2) If yes to (b), trigger a connect immediately and returns true. Else, returns false. * - * @param scanResults Array of {@link ScanResult} to be processed. * @return true if a pre-approved network was found for connection, false otherwise. */ - private boolean handleScanResultsAndTriggerConnectIfUserApprovedMatchFound( - ScanResult[] scanResults) { + private boolean triggerConnectIfUserApprovedMatchFound() { + if (mActiveSpecificNetworkRequestSpecifier == null) return false; + if (!isActiveRequestForSingleAccessPoint()) return false; + String ssid = mActiveSpecificNetworkRequestSpecifier.ssidPatternMatcher.getPath(); + MacAddress bssid = mActiveSpecificNetworkRequestSpecifier.bssidPatternMatcher.first; + int networkType = + ScanResultMatchInfo.fromWifiConfiguration( + mActiveSpecificNetworkRequestSpecifier.wifiConfiguration).networkType; + if (!isAccessPointApprovedForActiveRequest(ssid, bssid, networkType) + || mWifiConfigManager.isNetworkTemporarilyDisabledByUser( + ScanResultUtil.createQuotedSSID(ssid))) { + if (mVerboseLoggingEnabled) { + Log.v(TAG, "No approved access point found"); + } + return false; + } + Log.v(TAG, "Approved access point found in matching scan results. " + + "Triggering connect " + ssid + "/" + bssid); + WifiConfiguration config = mActiveSpecificNetworkRequestSpecifier.wifiConfiguration; + config.SSID = "\"" + ssid + "\""; + config.BSSID = bssid.toString(); + handleConnectToNetworkUserSelectionInternal(config); + mWifiMetrics.incrementNetworkRequestApiNumUserApprovalBypass(); + return true; + } + + /** + * Handle scan results + * + * @param scanResults Array of {@link ScanResult} to be processed. + */ + private void handleScanResults(ScanResult[] scanResults) { List<ScanResult> matchedScanResults = getNetworksMatchingActiveNetworkRequest(scanResults); if ((mActiveMatchedScanResults == null || mActiveMatchedScanResults.isEmpty()) @@ -1399,26 +1424,6 @@ public class WifiNetworkFactory extends NetworkFactory { matchedScanResults.size()); } mActiveMatchedScanResults = matchedScanResults; - - ScanResult approvedScanResult = null; - if (isActiveRequestForSingleAccessPoint()) { - approvedScanResult = - findUserApprovedAccessPointForActiveRequestFromActiveMatchedScanResults(); - } - if (approvedScanResult != null - && !mWifiConfigManager.isNetworkTemporarilyDisabledByUser( - ScanResultUtil.createQuotedSSID(approvedScanResult.SSID))) { - Log.v(TAG, "Approved access point found in matching scan results. " - + "Triggering connect " + approvedScanResult); - handleConnectToNetworkUserSelectionInternal( - ScanResultUtil.createNetworkFromScanResult(approvedScanResult)); - mWifiMetrics.incrementNetworkRequestApiNumUserApprovalBypass(); - return true; - } - if (mVerboseLoggingEnabled) { - Log.v(TAG, "No approved access points found in matching scan results"); - } - return false; } /** diff --git a/tests/wifitests/src/com/android/server/wifi/WifiNetworkFactoryTest.java b/tests/wifitests/src/com/android/server/wifi/WifiNetworkFactoryTest.java index 17159b5e7..25aaeffd6 100644 --- a/tests/wifitests/src/com/android/server/wifi/WifiNetworkFactoryTest.java +++ b/tests/wifitests/src/com/android/server/wifi/WifiNetworkFactoryTest.java @@ -86,7 +86,6 @@ import org.xmlpull.v1.XmlSerializer; import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; import java.nio.charset.StandardCharsets; -import java.util.Arrays; import java.util.Collections; import java.util.HashMap; import java.util.HashSet; @@ -2095,50 +2094,6 @@ public class WifiNetworkFactoryTest extends WifiBaseTest { } /** - * Verify the user approval bypass for a specific request for an access point that was already - * approved previously with no cached scan results matching. - */ - @Test - public void testNetworkSpecifierMatchSuccessUsingLiteralSsidAndBssidMatchApprovedWithNoCache() - throws Exception { - // 1. First request (no user approval bypass) - sendNetworkRequestAndSetupForConnectionStatus(); - - mWifiNetworkFactory.removeCallback(TEST_CALLBACK_IDENTIFIER); - reset(mNetworkRequestMatchCallback, mWifiScanner, mAlarmManager, mClientModeImpl); - - // 2. Second request for the same access point (user approval bypass). - ScanResult matchingScanResult = mTestScanDatas[0].getResults()[0]; - PatternMatcher ssidPatternMatch = - new PatternMatcher(TEST_SSID_1, PatternMatcher.PATTERN_LITERAL); - Pair<MacAddress, MacAddress> bssidPatternMatch = - Pair.create(MacAddress.fromString(matchingScanResult.BSSID), - MacAddress.BROADCAST_ADDRESS); - attachWifiNetworkSpecifierAndAppInfo( - ssidPatternMatch, bssidPatternMatch, - WifiConfigurationTestUtil.createPskNetwork(), TEST_UID_1, TEST_PACKAGE_NAME_1); - mWifiNetworkFactory.needNetworkFor(mNetworkRequest, 0); - - validateUiStartParams(true); - - mWifiNetworkFactory.addCallback(mAppBinder, mNetworkRequestMatchCallback, - TEST_CALLBACK_IDENTIFIER); - // Trigger scan results & ensure we triggered a connect. - verify(mWifiScanner).startScan(any(), any(), mScanListenerArgumentCaptor.capture(), any()); - ScanListener scanListener = mScanListenerArgumentCaptor.getValue(); - assertNotNull(scanListener); - scanListener.onResults(mTestScanDatas); - - // Verify we did not trigger the match callback. - verify(mNetworkRequestMatchCallback, never()).onMatch(anyList()); - // Verify that we sent a connection attempt to ClientModeImpl - verify(mClientModeImpl).connect(eq(null), anyInt(), - any(Binder.class), mConnectListenerArgumentCaptor.capture(), anyInt(), anyInt()); - - verify(mWifiMetrics).incrementNetworkRequestApiNumUserApprovalBypass(); - } - - /** * Verify that we don't bypass user approval for a specific request for an access point that was * approved previously, but then the user forgot it sometime after. */ @@ -2401,13 +2356,8 @@ public class WifiNetworkFactoryTest extends WifiBaseTest { mWifiNetworkFactory.needNetworkFor(mNetworkRequest, 0); mWifiNetworkFactory.addCallback(mAppBinder, mNetworkRequestMatchCallback, TEST_CALLBACK_IDENTIFIER); - // Trigger scan results & ensure we triggered a connect. - setupScanData(SCAN_RESULT_TYPE_WPA_PSK, - TEST_SSID_1, TEST_SSID_2, TEST_SSID_3, TEST_SSID_4); - verify(mWifiScanner).startScan(any(), any(), mScanListenerArgumentCaptor.capture(), any()); - ScanListener scanListener = mScanListenerArgumentCaptor.getValue(); - assertNotNull(scanListener); - scanListener.onResults(mTestScanDatas); + // Ensure we triggered a connect without issuing any scans. + verify(mWifiScanner, never()).startScan(any(), any(), any(), any()); // Verify we did not trigger the match callback. verify(mNetworkRequestMatchCallback, never()).onMatch(anyList()); @@ -2450,7 +2400,7 @@ public class WifiNetworkFactoryTest extends WifiBaseTest { * approved previously and the scan result is present in the cached scan results. */ @Test - public void testNetworkSpecifierMatchSuccessUsingLiteralSsidAndBssidMatchApprovedWithCache() + public void testNetworkSpecifierMatchSuccessUsingLiteralSsidAndBssidMatchApproved() throws Exception { // 1. First request (no user approval bypass) sendNetworkRequestAndSetupForConnectionStatus(); @@ -2460,11 +2410,6 @@ public class WifiNetworkFactoryTest extends WifiBaseTest { // 2. Second request for the same access point (user approval bypass). ScanResult matchingScanResult = mTestScanDatas[0].getResults()[0]; - // simulate no cache expiry - when(mClock.getElapsedSinceBootMillis()).thenReturn(0L); - // Simulate the cached results matching. - when(mWifiScanner.getSingleScanResults()) - .thenReturn(Arrays.asList(mTestScanDatas[0].getResults())); PatternMatcher ssidPatternMatch = new PatternMatcher(TEST_SSID_1, PatternMatcher.PATTERN_LITERAL); @@ -2495,7 +2440,7 @@ public class WifiNetworkFactoryTest extends WifiBaseTest { */ @Test public void - testNetworkSpecifierMatchSuccessUsingLiteralSsidAndBssidMatchApprovedViaCDMWithCache() + testNetworkSpecifierMatchSuccessUsingLiteralSsidAndBssidMatchApprovedViaCDM() throws Exception { // Setup scan data for WPA-PSK networks. setupScanData(SCAN_RESULT_TYPE_WPA_PSK, @@ -2510,12 +2455,6 @@ public class WifiNetworkFactoryTest extends WifiBaseTest { MacAddress.fromString(matchingScanResult.BSSID), UserHandle.getUserHandleForUid(TEST_UID_1))).thenReturn(true); - // simulate no cache expiry - when(mClock.getElapsedSinceBootMillis()).thenReturn(0L); - // Simulate the cached results matching. - when(mWifiScanner.getSingleScanResults()) - .thenReturn(Arrays.asList(mTestScanDatas[0].getResults())); - PatternMatcher ssidPatternMatch = new PatternMatcher(TEST_SSID_1, PatternMatcher.PATTERN_LITERAL); Pair<MacAddress, MacAddress> bssidPatternMatch = @@ -2546,7 +2485,7 @@ public class WifiNetworkFactoryTest extends WifiBaseTest { */ @Test public void - testNetworkSpecifierMatchSuccessUsingLiteralSsidAndBssidMatchApprovedViaShellWithCache() + testNetworkSpecifierMatchSuccessUsingLiteralSsidAndBssidMatchApprovedViaShell() throws Exception { // Setup scan data for WPA-PSK networks. setupScanData(SCAN_RESULT_TYPE_WPA_PSK, @@ -2558,12 +2497,6 @@ public class WifiNetworkFactoryTest extends WifiBaseTest { // Setup shell approval for the scan result. mWifiNetworkFactory.setUserApprovedApp(TEST_PACKAGE_NAME_1, true); - // simulate no cache expiry - when(mClock.getElapsedSinceBootMillis()).thenReturn(0L); - // Simulate the cached results matching. - when(mWifiScanner.getSingleScanResults()) - .thenReturn(Arrays.asList(mTestScanDatas[0].getResults())); - PatternMatcher ssidPatternMatch = new PatternMatcher(TEST_SSID_1, PatternMatcher.PATTERN_LITERAL); Pair<MacAddress, MacAddress> bssidPatternMatch = @@ -2588,68 +2521,6 @@ public class WifiNetworkFactoryTest extends WifiBaseTest { } /** - * Verify the user approval bypass for a specific request for an access point that was already - * approved previously and the scan result is present in the cached scan results, but the - * results are stale. - */ - @Test - public void - testNetworkSpecifierMatchSuccessUsingLiteralSsidAndBssidMatchApprovedWithStaleCache() - throws Exception { - // 1. First request (no user approval bypass) - sendNetworkRequestAndSetupForConnectionStatus(); - - mWifiNetworkFactory.removeCallback(TEST_CALLBACK_IDENTIFIER); - reset(mNetworkRequestMatchCallback, mWifiScanner, mAlarmManager, mClientModeImpl); - - long scanResultsTimestampInUs = 39484839202L; - mTestScanDatas[0].getResults()[0].timestamp = scanResultsTimestampInUs; - mTestScanDatas[0].getResults()[1].timestamp = scanResultsTimestampInUs; - mTestScanDatas[0].getResults()[2].timestamp = scanResultsTimestampInUs; - mTestScanDatas[0].getResults()[3].timestamp = scanResultsTimestampInUs; - - // 2. Second request for the same access point (user approval bypass). - ScanResult matchingScanResult = mTestScanDatas[0].getResults()[0]; - // simulate cache expiry - when(mClock.getElapsedSinceBootMillis()) - .thenReturn(Long.valueOf( - scanResultsTimestampInUs / 1000 - + WifiNetworkFactory.CACHED_SCAN_RESULTS_MAX_AGE_IN_MILLIS + 1)); - // Simulate the cached results matching. - when(mWifiScanner.getSingleScanResults()) - .thenReturn(Arrays.asList(mTestScanDatas[0].getResults())); - - PatternMatcher ssidPatternMatch = - new PatternMatcher(TEST_SSID_1, PatternMatcher.PATTERN_LITERAL); - Pair<MacAddress, MacAddress> bssidPatternMatch = - Pair.create(MacAddress.fromString(matchingScanResult.BSSID), - MacAddress.BROADCAST_ADDRESS); - attachWifiNetworkSpecifierAndAppInfo( - ssidPatternMatch, bssidPatternMatch, - WifiConfigurationTestUtil.createPskNetwork(), TEST_UID_1, TEST_PACKAGE_NAME_1); - mWifiNetworkFactory.needNetworkFor(mNetworkRequest, 0); - - // Ensure we brought up the UI while the scan is ongoing. - validateUiStartParams(true); - - mWifiNetworkFactory.addCallback(mAppBinder, mNetworkRequestMatchCallback, - TEST_CALLBACK_IDENTIFIER); - // Trigger scan results & ensure we triggered a connect. - verify(mWifiScanner).startScan(any(), any(), mScanListenerArgumentCaptor.capture(), any()); - ScanListener scanListener = mScanListenerArgumentCaptor.getValue(); - assertNotNull(scanListener); - scanListener.onResults(mTestScanDatas); - - // Verify we did not trigger the match callback. - verify(mNetworkRequestMatchCallback, never()).onMatch(anyList()); - // Verify that we sent a connection attempt to ClientModeImpl - verify(mClientModeImpl).connect(eq(null), anyInt(), - any(Binder.class), mConnectListenerArgumentCaptor.capture(), anyInt(), anyInt()); - - verify(mWifiMetrics).incrementNetworkRequestApiNumUserApprovalBypass(); - } - - /** * Verify network specifier matching for a specifier containing a specific SSID match using * 4 WPA_PSK scan results, each with unique SSID when the UI callback registration is delayed. */ |