hotspot2: Passpoint configuration APIs update

- renamed addPasspointConfiguration to addOrUpdatePasspointConfiguration
  to match the implementation of the API
- updated PasspointManager to update passpoint provider using exact FQDN
  string matching instead of base domain matching, to avoid unnecessary
  complexities.
- updated getPasspointConfigurations to return an empty list instead of
  a null when no configuration is installled
- updated PasspointManager to use a 64-bit counter for assigning provider
  IDs instead of using the wall clock to avoid possible ID conflicts.  The
  use of an unique provider ID is necessary to avoid leaving keys and
  certificates in a bad state when failing to install keys and certificates
  on the update operation.

Bug: 33587910
Test: frameworks/opt/net/wifi/tests/wifitests/runtests.sh

Change-Id: Iee304fe742085ec91d8d448e0939201d293ab7e5

3 files changed, 56 insertions, 94 deletions

diff --git a/service/java/com/android/server/wifi/WifiServiceImpl.java b/service/java/com/android/server/wifi/WifiServiceImpl.java
index 27605e9bb..ff8c0b99a 100644
--- a/service/java/com/android/server/wifi/WifiServiceImpl.java
+++ b/service/java/com/android/server/wifi/WifiServiceImpl.java
@@ -1020,15 +1020,15 @@ public class WifiServiceImpl extends IWifiManager.Stub {
     }
 
     /**
-     * Add a Passpoint configuration.
+     * Add or update a Passpoint configuration.
      *
      * @param config The Passpoint configuration to be added
      * @return true on success or false on failure
      */
     @Override
-    public boolean addPasspointConfiguration(PasspointConfiguration config) {
+    public boolean addOrUpdatePasspointConfiguration(PasspointConfiguration config) {
         enforceChangePermission();
-        return mPasspointManager.addProvider(config);
+        return mPasspointManager.addOrUpdateProvider(config);
     }
 
     /**
@@ -1046,7 +1046,9 @@ public class WifiServiceImpl extends IWifiManager.Stub {
     /**
      * Return the list of the installed Passpoint configurations.
      *
-     * @return A list of {@link PasspointConfiguration} or null
+     * An empty list will be returned when no configuration is installed.
+     *
+     * @return A list of {@link PasspointConfiguration}
      */
     @Override
     public List<PasspointConfiguration> getPasspointConfigurations() {
diff --git a/service/java/com/android/server/wifi/hotspot2/PasspointManager.java b/service/java/com/android/server/wifi/hotspot2/PasspointManager.java
index d0ced7b00..31a0b5ef6 100644
--- a/service/java/com/android/server/wifi/hotspot2/PasspointManager.java
+++ b/service/java/com/android/server/wifi/hotspot2/PasspointManager.java
@@ -31,7 +31,6 @@ import android.content.Context;
 import android.content.Intent;
 import android.net.wifi.hotspot2.PasspointConfiguration;
 import android.os.UserHandle;
-import android.text.TextUtils;
 import android.util.Log;
 import android.util.Pair;
 
@@ -47,12 +46,24 @@ import com.android.server.wifi.hotspot2.anqp.Constants;
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.HashMap;
-import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 
 /**
- * Responsible for managing passpoint networks.
+ * This class provides the APIs to manage Passpoint provider configurations.
+ * It deals with the following:
+ * - Maintaining a list of configured Passpoint providers for provider matching.
+ * - Persisting the providers configurations to store when required.
+ * - matching Passpoint providers based on the scan results
+ * - Supporting WifiManager Public API calls:
+ *   > addOrUpdatePasspointConfiguration()
+ *   > removePasspointConfiguration()
+ *   > getPasspointConfigurations()
+ *
+ * The provider matching requires obtaining additional information from the AP (ANQP elements).
+ * The ANQP elements will be cached using {@link AnqpCache} to avoid unnecessary requests.
+ *
+ * NOTE: These API's are not thread safe and should only be used from WifiStateMachine thread.
  */
 public class PasspointManager {
     private static final String TAG = "PasspointManager";
@@ -60,12 +71,14 @@ public class PasspointManager {
     private final PasspointEventHandler mHandler;
     private final SIMAccessor mSimAccessor;
     private final WifiKeyStore mKeyStore;
-    private final Clock mClock;
     private final PasspointObjectFactory mObjectFactory;
     private final Map<String, PasspointProvider> mProviders;
     private final AnqpCache mAnqpCache;
     private final Map<Long, ANQPNetworkKey> mPendingAnqpQueries;
 
+    // Counter used for assigning unique identifier to a provider.
+    private long mProviderID;
+
     private class CallbackHandler implements PasspointEventHandler.Callbacks {
         private final Context mContext;
         CallbackHandler(Context context) {
@@ -133,26 +146,26 @@ public class PasspointManager {
         mHandler = objectFactory.makePasspointEventHandler(wifiNative,
                 new CallbackHandler(context));
         mKeyStore = keyStore;
-        mClock = clock;
         mSimAccessor = simAccessor;
         mObjectFactory = objectFactory;
         mProviders = new HashMap<>();
         mAnqpCache = objectFactory.makeAnqpCache(clock);
         mPendingAnqpQueries = new HashMap<>();
+        mProviderID = 0;
         // TODO(zqiu): load providers from the persistent storage.
     }
 
     /**
-     * Add or install a Passpoint provider with the given configuration.
+     * Add or update a Passpoint provider with the given configuration.
      *
      * Each provider is uniquely identified by its FQDN (Fully Qualified Domain Name).
-     * In the case when there is an existing configuration with the same base
-     * domain, a provider with the new configuration will replace the existing provider.
+     * In the case when there is an existing configuration with the same FQDN
+     * a provider with the new configuration will replace the existing provider.
      *
      * @param config Configuration of the Passpoint provider to be added
      * @return true if provider is added, false otherwise
      */
-    public boolean addProvider(PasspointConfiguration config) {
+    public boolean addOrUpdateProvider(PasspointConfiguration config) {
         if (config == null) {
             Log.e(TAG, "Configuration not provided");
             return false;
@@ -177,20 +190,17 @@ public class PasspointManager {
 
         // Create a provider and install the necessary certificates and keys.
         PasspointProvider newProvider = mObjectFactory.makePasspointProvider(
-                config, mKeyStore, mClock.getWallClockMillis());
+                config, mKeyStore, mProviderID++);
 
         if (!newProvider.installCertsAndKeys()) {
             Log.e(TAG, "Failed to install certificates and keys to keystore");
             return false;
         }
 
-        // Detect existing configuration in the same base domain.
-        PasspointProvider existingProvider = findProviderInSameBaseDomain(config.homeSp.fqdn);
-        if (existingProvider != null) {
-            Log.d(TAG, "Replacing configuration for " + existingProvider.getConfig().homeSp.fqdn
-                    + " with " + config.homeSp.fqdn);
-            existingProvider.uninstallCertsAndKeys();
-            mProviders.remove(existingProvider.getConfig().homeSp.fqdn);
+        // Remove existing provider with the same FQDN.
+        if (mProviders.containsKey(config.homeSp.fqdn)) {
+            Log.d(TAG, "Replacing configuration for " + config.homeSp.fqdn);
+            removeProvider(config.homeSp.fqdn);
         }
 
         mProviders.put(config.homeSp.fqdn, newProvider);
@@ -220,13 +230,11 @@ public class PasspointManager {
     /**
      * Return the installed Passpoint provider configurations.
      *
-     * @return A list of {@link PasspointConfiguration} or null if none is installed
+     * An empty list will be returned when no provider is installed.
+     *
+     * @return A list of {@link PasspointConfiguration}
      */
     public List<PasspointConfiguration> getProviderConfigs() {
-        if (mProviders.size() == 0) {
-            return null;
-        }
-
         List<PasspointConfiguration> configs = new ArrayList<>();
         for (Map.Entry<String, PasspointProvider> entry : mProviders.entrySet()) {
             configs.add(entry.getValue().getConfig());
@@ -317,45 +325,4 @@ public class PasspointManager {
     public boolean queryPasspointIcon(long bssid, String fileName) {
         return mHandler.requestIcon(bssid, fileName);
     }
-
-    /**
-     * Find a provider that have FQDN in the same base domain as the given domain.
-     *
-     * @param domain The domain to be compared
-     * @return {@link PasspointProvider} if a match is found, null otherwise
-     */
-    private PasspointProvider findProviderInSameBaseDomain(String domain) {
-        for (Map.Entry<String, PasspointProvider> entry : mProviders.entrySet()) {
-            if (isSameBaseDomain(entry.getKey(), domain)) {
-                return entry.getValue();
-            }
-        }
-        return null;
-    }
-
-    /**
-     * Check if one domain is the base domain for the other.  For example, "test1.test.com"
-     * and "test.com" should return true.
-     *
-     * @param domain1 First domain to be compared
-     * @param domain2 Second domain to be compared
-     * @return true if one domain is a base domain for the other, false otherwise.
-     */
-    private static boolean isSameBaseDomain(String domain1, String domain2) {
-        if (domain1 == null || domain2 == null) {
-            return false;
-        }
-
-        List<String> labelList1 = Utils.splitDomain(domain1);
-        List<String> labelList2 = Utils.splitDomain(domain2);
-        Iterator<String> l1 = labelList1.iterator();
-        Iterator<String> l2 = labelList2.iterator();
-
-        while (l1.hasNext() && l2.hasNext()) {
-            if (!TextUtils.equals(l1.next(), l2.next())) {
-                return false;
-            }
-        }
-        return true;
-    }
 }
diff --git a/tests/wifitests/src/com/android/server/wifi/hotspot2/PasspointManagerTest.java b/tests/wifitests/src/com/android/server/wifi/hotspot2/PasspointManagerTest.java
index d776b3195..af66dc0b7 100644
--- a/tests/wifitests/src/com/android/server/wifi/hotspot2/PasspointManagerTest.java
+++ b/tests/wifitests/src/com/android/server/wifi/hotspot2/PasspointManagerTest.java
@@ -25,6 +25,7 @@ import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 import static org.mockito.Mockito.any;
+import static org.mockito.Mockito.anyLong;
 import static org.mockito.Mockito.anyMap;
 import static org.mockito.Mockito.eq;
 import static org.mockito.Mockito.mock;
@@ -66,11 +67,9 @@ public class PasspointManagerTest {
     private static final long BSSID = 0x112233445566L;
     private static final String ICON_FILENAME = "test";
     private static final String TEST_FQDN = "test1.test.com";
-    private static final String TEST_FQDN1 = "test.com";
     private static final String TEST_FRIENDLY_NAME = "friendly name";
     private static final String TEST_REALM = "realm.test.com";
     private static final String TEST_IMSI = "1234*";
-    private static final long PROVIDER_ID = 1L;
 
     private static final String TEST_SSID = "TestSSID";
     private static final long TEST_BSSID = 0x1234L;
@@ -167,10 +166,9 @@ public class PasspointManagerTest {
         config.credential.userCredential.eapType = EAPConstants.EAP_TTLS;
         config.credential.userCredential.nonEapInnerMethod = "MS-CHAP";
         PasspointProvider provider = createMockProvider(config);
-        when(mClock.getWallClockMillis()).thenReturn(PROVIDER_ID);
-        when(mObjectFactory.makePasspointProvider(config, mWifiKeyStore, PROVIDER_ID))
+        when(mObjectFactory.makePasspointProvider(eq(config), eq(mWifiKeyStore), anyLong()))
                 .thenReturn(provider);
-        assertTrue(mManager.addProvider(config));
+        assertTrue(mManager.addOrUpdateProvider(config));
 
         return provider;
     }
@@ -222,7 +220,7 @@ public class PasspointManagerTest {
      */
     @Test
     public void addProviderWithNullConfig() throws Exception {
-        assertFalse(mManager.addProvider(null));
+        assertFalse(mManager.addOrUpdateProvider(null));
     }
 
     /**
@@ -232,7 +230,7 @@ public class PasspointManagerTest {
      */
     @Test
     public void addProviderWithEmptyConfig() throws Exception {
-        assertFalse(mManager.addProvider(new PasspointConfiguration()));
+        assertFalse(mManager.addOrUpdateProvider(new PasspointConfiguration()));
     }
 
     /**
@@ -256,7 +254,7 @@ public class PasspointManagerTest {
         // EAP-TLS not allowed for user credential.
         config.credential.userCredential.eapType = EAPConstants.EAP_TLS;
         config.credential.userCredential.nonEapInnerMethod = "MS-CHAP";
-        assertFalse(mManager.addProvider(config));
+        assertFalse(mManager.addOrUpdateProvider(config));
     }
 
     /**
@@ -279,16 +277,15 @@ public class PasspointManagerTest {
         config.credential.userCredential.eapType = EAPConstants.EAP_TTLS;
         config.credential.userCredential.nonEapInnerMethod = "MS-CHAP";
         PasspointProvider provider = createMockProvider(config);
-        when(mClock.getWallClockMillis()).thenReturn(PROVIDER_ID);
-        when(mObjectFactory.makePasspointProvider(config, mWifiKeyStore, PROVIDER_ID))
+        when(mObjectFactory.makePasspointProvider(eq(config), eq(mWifiKeyStore), anyLong()))
                 .thenReturn(provider);
-        assertTrue(mManager.addProvider(config));
+        assertTrue(mManager.addOrUpdateProvider(config));
         verifyInstalledConfig(config);
 
         // Remove the provider.
         assertTrue(mManager.removeProvider(TEST_FQDN));
         verify(provider).uninstallCertsAndKeys();
-        assertEquals(null, mManager.getProviderConfigs());
+        assertTrue(mManager.getProviderConfigs().isEmpty());
     }
 
     /**
@@ -310,16 +307,15 @@ public class PasspointManagerTest {
         when(mSimAccessor.getMatchingImsis(new IMSIParameter(TEST_IMSI)))
                 .thenReturn(new ArrayList<String>());
         PasspointProvider provider = createMockProvider(config);
-        when(mClock.getWallClockMillis()).thenReturn(PROVIDER_ID);
-        when(mObjectFactory.makePasspointProvider(config, mWifiKeyStore, PROVIDER_ID))
+        when(mObjectFactory.makePasspointProvider(eq(config), eq(mWifiKeyStore), anyLong()))
                 .thenReturn(provider);
-        assertTrue(mManager.addProvider(config));
+        assertTrue(mManager.addOrUpdateProvider(config));
         verifyInstalledConfig(config);
 
         // Remove the provider.
         assertTrue(mManager.removeProvider(TEST_FQDN));
         verify(provider).uninstallCertsAndKeys();
-        assertEquals(null, mManager.getProviderConfigs());
+        assertTrue(mManager.getProviderConfigs().isEmpty());
     }
 
     /**
@@ -340,7 +336,7 @@ public class PasspointManagerTest {
         config.credential.simCredential.imsi = TEST_IMSI;
         config.credential.simCredential.eapType = EAPConstants.EAP_SIM;
         when(mSimAccessor.getMatchingImsis(new IMSIParameter(TEST_IMSI))).thenReturn(null);
-        assertFalse(mManager.addProvider(config));
+        assertFalse(mManager.addOrUpdateProvider(config));
     }
 
     /**
@@ -365,17 +361,16 @@ public class PasspointManagerTest {
         when(mSimAccessor.getMatchingImsis(new IMSIParameter(TEST_IMSI)))
                 .thenReturn(new ArrayList<String>());
         PasspointProvider origProvider = createMockProvider(origConfig);
-        when(mClock.getWallClockMillis()).thenReturn(PROVIDER_ID);
-        when(mObjectFactory.makePasspointProvider(origConfig, mWifiKeyStore, PROVIDER_ID))
+        when(mObjectFactory.makePasspointProvider(eq(origConfig), eq(mWifiKeyStore), anyLong()))
                 .thenReturn(origProvider);
-        assertTrue(mManager.addProvider(origConfig));
+        assertTrue(mManager.addOrUpdateProvider(origConfig));
         verifyInstalledConfig(origConfig);
 
         // Add another provider with the same base domain as the existing provider.
         // This should replace the existing provider with the new configuration.
         PasspointConfiguration newConfig = new PasspointConfiguration();
         newConfig.homeSp = new HomeSP();
-        newConfig.homeSp.fqdn = TEST_FQDN1;
+        newConfig.homeSp.fqdn = TEST_FQDN;
         newConfig.homeSp.friendlyName = TEST_FRIENDLY_NAME;
         newConfig.credential = new Credential();
         newConfig.credential.realm = TEST_REALM;
@@ -386,10 +381,9 @@ public class PasspointManagerTest {
         newConfig.credential.userCredential.eapType = EAPConstants.EAP_TTLS;
         newConfig.credential.userCredential.nonEapInnerMethod = "MS-CHAP";
         PasspointProvider newProvider = createMockProvider(newConfig);
-        when(mClock.getWallClockMillis()).thenReturn(PROVIDER_ID);
-        when(mObjectFactory.makePasspointProvider(newConfig, mWifiKeyStore, PROVIDER_ID))
+        when(mObjectFactory.makePasspointProvider(eq(newConfig), eq(mWifiKeyStore), anyLong()))
                 .thenReturn(newProvider);
-        assertTrue(mManager.addProvider(newConfig));
+        assertTrue(mManager.addOrUpdateProvider(newConfig));
         verifyInstalledConfig(newConfig);
     }
 
@@ -415,10 +409,9 @@ public class PasspointManagerTest {
         config.credential.userCredential.nonEapInnerMethod = "MS-CHAP";
         PasspointProvider provider = mock(PasspointProvider.class);
         when(provider.installCertsAndKeys()).thenReturn(false);
-        when(mClock.getWallClockMillis()).thenReturn(PROVIDER_ID);
-        when(mObjectFactory.makePasspointProvider(config, mWifiKeyStore, PROVIDER_ID))
+        when(mObjectFactory.makePasspointProvider(eq(config), eq(mWifiKeyStore), anyLong()))
                 .thenReturn(provider);
-        assertFalse(mManager.addProvider(config));
+        assertFalse(mManager.addOrUpdateProvider(config));
     }
 
     /**