Merge "[WifiNetworkFactory] Trim user approval list per App" into qt-qpr1-dev

3 files changed, 200 insertions, 18 deletions

diff --git a/service/java/com/android/server/wifi/NetworkRequestStoreData.java b/service/java/com/android/server/wifi/NetworkRequestStoreData.java
index b74e64423..8d1244f05 100644
--- a/service/java/com/android/server/wifi/NetworkRequestStoreData.java
+++ b/service/java/com/android/server/wifi/NetworkRequestStoreData.java
@@ -29,7 +29,7 @@ import org.xmlpull.v1.XmlSerializer;
 
 import java.io.IOException;
 import java.util.HashMap;
-import java.util.HashSet;
+import java.util.LinkedHashSet;
 import java.util.Map;
 import java.util.Map.Entry;
 import java.util.Set;
@@ -213,7 +213,7 @@ public class NetworkRequestStoreData implements WifiConfigStore.StoreData {
      */
     private Set<AccessPoint> parseApprovedAccessPoints(XmlPullParser in, int outerTagDepth)
             throws XmlPullParserException, IOException {
-        Set<AccessPoint> approvedAccessPoints = new HashSet<>();
+        Set<AccessPoint> approvedAccessPoints = new LinkedHashSet<>();
         while (XmlUtil.gotoNextSectionWithNameOrEnd(
                 in, XML_TAG_SECTION_HEADER_ACCESS_POINT, outerTagDepth)) {
             // Try/catch only runtime exceptions (like illegal args), any XML/IO exceptions are
diff --git a/service/java/com/android/server/wifi/WifiNetworkFactory.java b/service/java/com/android/server/wifi/WifiNetworkFactory.java
index ecc1a9f18..1f060cf73 100644
--- a/service/java/com/android/server/wifi/WifiNetworkFactory.java
+++ b/service/java/com/android/server/wifi/WifiNetworkFactory.java
@@ -68,6 +68,7 @@ import java.util.Comparator;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Iterator;
+import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
@@ -97,6 +98,9 @@ public class WifiNetworkFactory extends NetworkFactory {
     @VisibleForTesting
     public static final String UI_START_INTENT_EXTRA_REQUEST_IS_FOR_SINGLE_NETWORK =
             "com.android.settings.wifi.extra.REQUEST_IS_FOR_SINGLE_NETWORK";
+    // Capacity limit of approved Access Point per App
+    @VisibleForTesting
+    public static final int NUM_OF_ACCESS_POINT_LIMIT_PER_APP = 50;
 
     private final Context mContext;
     private final ActivityManager mActivityManager;
@@ -117,8 +121,8 @@ public class WifiNetworkFactory extends NetworkFactory {
     private final ExternalCallbackTracker<INetworkRequestMatchCallback> mRegisteredCallbacks;
     private final Messenger mSrcMessenger;
     // Store all user approved access points for apps.
-    // TODO(b/122658039): Persist this.
-    private final Map<String, Set<AccessPoint>> mUserApprovedAccessPointMap = new HashMap<>();
+    @VisibleForTesting
+    public final Map<String, LinkedHashSet<AccessPoint>> mUserApprovedAccessPointMap;
     private WifiScanner mWifiScanner;
 
     private int mGenericConnectionReqCount = 0;
@@ -346,12 +350,13 @@ public class WifiNetworkFactory extends NetworkFactory {
         public Map<String, Set<AccessPoint>> toSerialize() {
             // Clear the flag after writing to disk.
             mHasNewDataToSerialize = false;
-            return mUserApprovedAccessPointMap;
+            return new HashMap<>(mUserApprovedAccessPointMap);
         }
 
         @Override
         public void fromDeserialized(Map<String, Set<AccessPoint>> approvedAccessPointMap) {
-            mUserApprovedAccessPointMap.putAll(approvedAccessPointMap);
+            approvedAccessPointMap.forEach((key, value) ->
+                    mUserApprovedAccessPointMap.put(key, new LinkedHashSet<>(value)));
         }
 
         @Override
@@ -397,6 +402,7 @@ public class WifiNetworkFactory extends NetworkFactory {
         mConnectionTimeoutAlarmListener = new ConnectionTimeoutAlarmListener();
         mRegisteredCallbacks = new ExternalCallbackTracker<INetworkRequestMatchCallback>(mHandler);
         mSrcMessenger = new Messenger(new Handler(looper, mNetworkConnectionTriggerCallback));
+        mUserApprovedAccessPointMap = new HashMap<>();
 
         // register the data store for serializing/deserializing data.
         configStore.registerStoreData(
@@ -1235,6 +1241,9 @@ public class WifiNetworkFactory extends NetworkFactory {
                     new AccessPoint(scanResult.SSID,
                             MacAddress.fromString(scanResult.BSSID), fromScanResult.networkType);
             if (approvedAccessPoints.contains(accessPoint)) {
+                // keep the most recently used AP in the end
+                approvedAccessPoints.remove(accessPoint);
+                approvedAccessPoints.add(accessPoint);
                 if (mVerboseLoggingEnabled) {
                     Log.v(TAG, "Found " + accessPoint
                             + " in user approved access point for " + requestorPackageName);
@@ -1254,10 +1263,11 @@ public class WifiNetworkFactory extends NetworkFactory {
         // object representing an entire network from UI, we need to ensure that all the visible
         // BSSIDs matching the original request and the selected network are stored.
         Set<AccessPoint> newUserApprovedAccessPoints = new HashSet<>();
+
+        ScanResultMatchInfo fromWifiConfiguration =
+                ScanResultMatchInfo.fromWifiConfiguration(network);
         for (ScanResult scanResult : mActiveMatchedScanResults) {
             ScanResultMatchInfo fromScanResult = ScanResultMatchInfo.fromScanResult(scanResult);
-            ScanResultMatchInfo fromWifiConfiguration =
-                    ScanResultMatchInfo.fromWifiConfiguration(network);
             if (fromScanResult.equals(fromWifiConfiguration)) {
                 AccessPoint approvedAccessPoint =
                         new AccessPoint(scanResult.SSID, MacAddress.fromString(scanResult.BSSID),
@@ -1268,10 +1278,10 @@ public class WifiNetworkFactory extends NetworkFactory {
         if (newUserApprovedAccessPoints.isEmpty()) return;
 
         String requestorPackageName = mActiveSpecificNetworkRequestSpecifier.requestorPackageName;
-        Set<AccessPoint> approvedAccessPoints =
+        LinkedHashSet<AccessPoint> approvedAccessPoints =
                 mUserApprovedAccessPointMap.get(requestorPackageName);
         if (approvedAccessPoints == null) {
-            approvedAccessPoints = new HashSet<>();
+            approvedAccessPoints = new LinkedHashSet<>();
             mUserApprovedAccessPointMap.put(requestorPackageName, approvedAccessPoints);
             // Note the new app in metrics.
             mWifiMetrics.incrementNetworkRequestApiNumApps();
@@ -1280,22 +1290,33 @@ public class WifiNetworkFactory extends NetworkFactory {
             Log.v(TAG, "Adding " + newUserApprovedAccessPoints
                     + " to user approved access point for " + requestorPackageName);
         }
+        // keep the most recently added APs in the end
+        approvedAccessPoints.removeAll(newUserApprovedAccessPoints);
         approvedAccessPoints.addAll(newUserApprovedAccessPoints);
+        cleanUpLRUAccessPoints(approvedAccessPoints);
         saveToStore();
     }
 
     /**
+     * Clean up least recently used Access Points if specified app reach the limit.
+     */
+    private static void cleanUpLRUAccessPoints(Set<AccessPoint> approvedAccessPoints) {
+        if (approvedAccessPoints.size() <= NUM_OF_ACCESS_POINT_LIMIT_PER_APP) {
+            return;
+        }
+        Iterator iter = approvedAccessPoints.iterator();
+        while (iter.hasNext() && approvedAccessPoints.size() > NUM_OF_ACCESS_POINT_LIMIT_PER_APP) {
+            iter.next();
+            iter.remove();
+        }
+    }
+
+    /**
      * Remove all user approved access points for the specified app.
      */
     public void removeUserApprovedAccessPointsForApp(@NonNull String packageName) {
-        Iterator<Map.Entry<String, Set<AccessPoint>>> iter =
-                mUserApprovedAccessPointMap.entrySet().iterator();
-        while (iter.hasNext()) {
-            Map.Entry<String, Set<AccessPoint>> entry = iter.next();
-            if (packageName.equals(entry.getKey())) {
-                Log.i(TAG, "Removing all approved access points for " + packageName);
-                iter.remove();
-            }
+        if (mUserApprovedAccessPointMap.remove(packageName) != null) {
+            Log.i(TAG, "Removing all approved access points for " + packageName);
         }
         saveToStore();
     }
diff --git a/tests/wifitests/src/com/android/server/wifi/WifiNetworkFactoryTest.java b/tests/wifitests/src/com/android/server/wifi/WifiNetworkFactoryTest.java
index d4e6594c8..968527a8d 100644
--- a/tests/wifitests/src/com/android/server/wifi/WifiNetworkFactoryTest.java
+++ b/tests/wifitests/src/com/android/server/wifi/WifiNetworkFactoryTest.java
@@ -61,8 +61,10 @@ import android.os.WorkSource;
 import android.os.test.TestLooper;
 import android.test.suitebuilder.annotation.SmallTest;
 import android.util.Pair;
+import android.util.Xml;
 
 import com.android.internal.util.AsyncChannel;
+import com.android.internal.util.FastXmlSerializer;
 import com.android.server.wifi.WifiNetworkFactory.AccessPoint;
 import com.android.server.wifi.nano.WifiMetricsProto;
 import com.android.server.wifi.util.ScanResultUtil;
@@ -76,9 +78,15 @@ import org.mockito.ArgumentMatcher;
 import org.mockito.InOrder;
 import org.mockito.Mock;
 import org.mockito.MockitoAnnotations;
+import org.xmlpull.v1.XmlPullParser;
+import org.xmlpull.v1.XmlSerializer;
 
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import java.nio.charset.StandardCharsets;
 import java.util.HashMap;
 import java.util.HashSet;
+import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -146,6 +154,7 @@ public class WifiNetworkFactoryTest {
 
     private WifiNetworkFactory mWifiNetworkFactory;
     private NetworkRequestStoreData.DataSource mDataSource;
+    private NetworkRequestStoreData mNetworkRequestStoreData;
 
     /**
      * Setup the mocks.
@@ -185,6 +194,7 @@ public class WifiNetworkFactoryTest {
         verify(mWifiInjector).makeNetworkRequestStoreData(dataSourceArgumentCaptor.capture());
         mDataSource = dataSourceArgumentCaptor.getValue();
         assertNotNull(mDataSource);
+        mNetworkRequestStoreData = new NetworkRequestStoreData(mDataSource);
 
         // Register and establish full connection to connectivity manager.
         mWifiNetworkFactory.register();
@@ -1083,6 +1093,73 @@ public class WifiNetworkFactoryTest {
     }
 
     /**
+     * Verify when number of user approved access points exceed the capacity, framework should trim
+     * the Set by removing the least recently used elements.
+     */
+    @Test
+    public void testNetworkSpecifierHandleUserSelectionConnectToNetworkExceedApprovedListCapacity()
+            throws Exception {
+        int userApproveAccessPointCapacity = mWifiNetworkFactory.NUM_OF_ACCESS_POINT_LIMIT_PER_APP;
+        int numOfApPerSsid = userApproveAccessPointCapacity / 2 + 1;
+        String[] testIds = new String[]{TEST_SSID_1, TEST_SSID_2};
+
+        // Setup up scan data
+        setupScanDataSameSsidWithDiffBssid(SCAN_RESULT_TYPE_WPA_PSK, numOfApPerSsid, testIds);
+
+        // Setup network specifier for WPA-PSK networks.
+        PatternMatcher ssidPatternMatch =
+                new PatternMatcher(TEST_SSID_1, PatternMatcher.PATTERN_PREFIX);
+        Pair<MacAddress, MacAddress> bssidPatternMatch =
+                Pair.create(MacAddress.ALL_ZEROS_ADDRESS, MacAddress.ALL_ZEROS_ADDRESS);
+        WifiConfiguration wifiConfiguration = WifiConfigurationTestUtil.createPskNetwork();
+        wifiConfiguration.preSharedKey = TEST_WPA_PRESHARED_KEY;
+        WifiNetworkSpecifier specifier = new WifiNetworkSpecifier(
+                ssidPatternMatch, bssidPatternMatch, wifiConfiguration, TEST_UID_1,
+                TEST_PACKAGE_NAME_1);
+
+        // request network, trigger scan and get matched set.
+        mNetworkRequest.networkCapabilities.setNetworkSpecifier(specifier);
+        mWifiNetworkFactory.needNetworkFor(mNetworkRequest, 0);
+
+        mWifiNetworkFactory.addCallback(mAppBinder, mNetworkRequestMatchCallback,
+                TEST_CALLBACK_IDENTIFIER);
+        verify(mNetworkRequestMatchCallback).onUserSelectionCallbackRegistration(
+                mNetworkRequestUserSelectionCallback.capture());
+
+        verifyPeriodicScans(0, PERIODIC_SCAN_INTERVAL_MS);
+
+        INetworkRequestUserSelectionCallback networkRequestUserSelectionCallback =
+                mNetworkRequestUserSelectionCallback.getValue();
+        assertNotNull(networkRequestUserSelectionCallback);
+
+        // Now trigger user selection to one of the network.
+        mSelectedNetwork = WifiConfigurationTestUtil.createPskNetwork();
+        mSelectedNetwork.SSID = "\"" + mTestScanDatas[0].getResults()[0].SSID + "\"";
+        networkRequestUserSelectionCallback.select(mSelectedNetwork);
+        mLooper.dispatchAll();
+
+        // Verifier num of Approved access points.
+        assertEquals(mWifiNetworkFactory.mUserApprovedAccessPointMap
+                .get(TEST_PACKAGE_NAME_1).size(), numOfApPerSsid);
+
+        // Now trigger user selection to another network with different SSID.
+        mSelectedNetwork = WifiConfigurationTestUtil.createPskNetwork();
+        mSelectedNetwork.SSID = "\"" + mTestScanDatas[0].getResults()[numOfApPerSsid].SSID + "\"";
+        networkRequestUserSelectionCallback.select(mSelectedNetwork);
+        mLooper.dispatchAll();
+
+        // Verify triggered trim when user Approved Access Points exceed capacity.
+        Set<AccessPoint> userApprovedAccessPoints = mWifiNetworkFactory.mUserApprovedAccessPointMap
+                .get(TEST_PACKAGE_NAME_1);
+        assertEquals(userApprovedAccessPoints.size(), userApproveAccessPointCapacity);
+        long numOfSsid1Aps = userApprovedAccessPoints
+                .stream()
+                .filter(a->a.ssid.equals(TEST_SSID_1))
+                .count();
+        assertEquals(numOfSsid1Aps, userApproveAccessPointCapacity - numOfApPerSsid);
+    }
+
+    /**
      * Verify handling of user selection to trigger connection to an existing saved network.
      */
     @Test
@@ -2322,6 +2399,35 @@ public class WifiNetworkFactoryTest {
         verify(mClientModeImpl).sendMessage(any());
     }
 
+    /**
+     * Verify the config store save and load could preserve the elements order.
+     */
+    @Test
+    public void testStoteConfigSaveAndLoadPreserveOrder() throws Exception {
+        LinkedHashSet<AccessPoint> approvedApSet = new LinkedHashSet<>();
+        approvedApSet.add(new AccessPoint(TEST_SSID_1,
+                MacAddress.fromString(TEST_BSSID_1), WifiConfiguration.SECURITY_TYPE_PSK));
+        approvedApSet.add(new AccessPoint(TEST_SSID_2,
+                MacAddress.fromString(TEST_BSSID_2), WifiConfiguration.SECURITY_TYPE_PSK));
+        approvedApSet.add(new AccessPoint(TEST_SSID_3,
+                MacAddress.fromString(TEST_BSSID_3), WifiConfiguration.SECURITY_TYPE_PSK));
+        approvedApSet.add(new AccessPoint(TEST_SSID_4,
+                MacAddress.fromString(TEST_BSSID_4), WifiConfiguration.SECURITY_TYPE_PSK));
+        mWifiNetworkFactory.mUserApprovedAccessPointMap.put(TEST_PACKAGE_NAME_1,
+                new LinkedHashSet<>(approvedApSet));
+        // Save config.
+        byte[] xmlData = serializeData();
+        mWifiNetworkFactory.mUserApprovedAccessPointMap.clear();
+        // Load config.
+        deserializeData(xmlData);
+
+        LinkedHashSet<AccessPoint> storedApSet = mWifiNetworkFactory
+                .mUserApprovedAccessPointMap.get(TEST_PACKAGE_NAME_1);
+        // Check load config success and order preserved.
+        assertNotNull(storedApSet);
+        assertArrayEquals(approvedApSet.toArray(), storedApSet.toArray());
+    }
+
     private Messenger sendNetworkRequestAndSetupForConnectionStatus() throws RemoteException {
         return sendNetworkRequestAndSetupForConnectionStatus(TEST_SSID_1);
     }
@@ -2621,4 +2727,59 @@ public class WifiNetworkFactoryTest {
         expectedWifiConfiguration.fromWifiNetworkSpecifier = true;
         WifiConfigurationTestUtil.assertConfigurationEqual(expectedWifiConfiguration, network);
     }
+
+    /**
+     * Create a test scan data for target SSID list with specified number and encryption type
+     * @param scanResultType   network encryption type
+     * @param nums             Number of results with different BSSIDs for one SSID
+     * @param ssids            target SSID list
+     */
+    private void setupScanDataSameSsidWithDiffBssid(int scanResultType, int nums, String[] ssids) {
+        String baseBssid = "11:34:56:78:90:";
+        int[] freq = new int[nums * ssids.length];
+        for (int i = 0; i < nums; i++) {
+            freq[i] = 2417 + i;
+        }
+        mTestScanDatas = ScanTestUtil.createScanDatas(new int[][]{ freq });
+        assertEquals(1, mTestScanDatas.length);
+        ScanResult[] scanResults = mTestScanDatas[0].getResults();
+        assertEquals(nums * ssids.length, scanResults.length);
+        String caps = getScanResultCapsForType(scanResultType);
+        for (int i = 0; i < ssids.length; i++) {
+            for (int j = i * nums; j < (i + 1) * nums; j++) {
+                scanResults[j].SSID = ssids[i];
+                scanResults[j].BSSID = baseBssid + Integer.toHexString(16 + j);
+                scanResults[j].capabilities = caps;
+                scanResults[j].level = -45;
+            }
+        }
+    }
+
+    /**
+     * Helper function for serializing configuration data to a XML block.
+     *
+     * @return byte[] of the XML data
+     * @throws Exception
+     */
+    private byte[] serializeData() throws Exception {
+        final XmlSerializer out = new FastXmlSerializer();
+        final ByteArrayOutputStream outputStream = new ByteArrayOutputStream();
+        out.setOutput(outputStream, StandardCharsets.UTF_8.name());
+        mNetworkRequestStoreData.serializeData(out);
+        out.flush();
+        return outputStream.toByteArray();
+    }
+
+    /**
+     * Helper function for parsing configuration data from a XML block.
+     *
+     * @param data XML data to parse from
+     * @throws Exception
+     */
+    private void deserializeData(byte[] data) throws Exception {
+        final XmlPullParser in = Xml.newPullParser();
+        final ByteArrayInputStream inputStream = new ByteArrayInputStream(data);
+        in.setInput(inputStream, StandardCharsets.UTF_8.name());
+        mNetworkRequestStoreData.deserializeData(in, in.getDepth());
+    }
 }