From 0b3eca3c05190e5824638f3da25b8b3167dc9d60 Mon Sep 17 00:00:00 2001 From: xshu Date: Tue, 14 Jan 2020 12:12:22 -0800 Subject: fix soft reboot caused by KeyStore exception The Mac handle obtained from AndroidKeyStore is sometimes invalidated by the AndroidKeyStore based on some LRU technique. This change make sure that we always get a valid handle. And adds exception handling to make sure a crash will not happen for the same reason again. If KeyStore continuously fails to generate MAC address, we will use locally generated MAC as it is the next best option. Bug: 146203882 Test: atest FrameworksWifiTests Merged-In: I8a3b810ba95898a96d81fe57979db4787e1a46c4 Change-Id: I8a3b810ba95898a96d81fe57979db4787e1a46c4 (cherry-picked from e299359a6e6e9e13217862f7f66627eccbff46ce) --- .../com/android/server/wifi/MacAddressUtil.java | 10 +++++++-- .../com/android/server/wifi/WifiConfigManager.java | 22 +++++++++++--------- .../android/server/wifi/MacAddressUtilTest.java | 15 ++++++++++++++ .../android/server/wifi/WifiConfigManagerTest.java | 24 +++++++++++++++++----- 4 files changed, 54 insertions(+), 17 deletions(-) diff --git a/service/java/com/android/server/wifi/MacAddressUtil.java b/service/java/com/android/server/wifi/MacAddressUtil.java index effa931a5..4739b6141 100644 --- a/service/java/com/android/server/wifi/MacAddressUtil.java +++ b/service/java/com/android/server/wifi/MacAddressUtil.java @@ -62,8 +62,14 @@ public class MacAddressUtil { if (config == null || hashFunction == null) { return null; } - byte[] hashedBytes = hashFunction.doFinal( - config.getSsidAndSecurityTypeString().getBytes(StandardCharsets.UTF_8)); + byte[] hashedBytes; + try { + hashedBytes = hashFunction.doFinal(config.getSsidAndSecurityTypeString() + .getBytes(StandardCharsets.UTF_8)); + } catch (ProviderException | IllegalStateException e) { + Log.e(TAG, "Failure in calculatePersistentMac", e); + return null; + } ByteBuffer bf = ByteBuffer.wrap(hashedBytes); long longFromSsid = bf.getLong(); /** diff --git a/service/java/com/android/server/wifi/WifiConfigManager.java b/service/java/com/android/server/wifi/WifiConfigManager.java index 80b583feb..c617b9e1f 100644 --- a/service/java/com/android/server/wifi/WifiConfigManager.java +++ b/service/java/com/android/server/wifi/WifiConfigManager.java @@ -76,8 +76,6 @@ import java.util.List; import java.util.Map; import java.util.Set; -import javax.crypto.Mac; - /** * This class provides the APIs to manage configured Wi-Fi networks. * It deals with the following: @@ -278,7 +276,6 @@ public class WifiConfigManager { private final WifiInjector mWifiInjector; private final MacAddressUtil mMacAddressUtil; private boolean mConnectedMacRandomzationSupported; - private Mac mMac; /** * Local log used for debugging any WifiConfigManager issues. @@ -516,7 +513,18 @@ public class WifiConfigManager { mRandomizedMacAddressMapping.remove(config.getSsidAndSecurityTypeString()); } } - return mMacAddressUtil.calculatePersistentMacForConfiguration(config, mMac); + MacAddress result = mMacAddressUtil.calculatePersistentMacForConfiguration( + config, mMacAddressUtil.obtainMacRandHashFunction(Process.WIFI_UID)); + if (result == null) { + result = mMacAddressUtil.calculatePersistentMacForConfiguration( + config, mMacAddressUtil.obtainMacRandHashFunction(Process.WIFI_UID)); + } + if (result == null) { + Log.wtf(TAG, "Failed to generate MAC address from KeyStore even after retrying. " + + "Using locally generated MAC address instead."); + result = MacAddress.createRandomUnicastAddress(); + } + return result; } /** @@ -3146,12 +3154,6 @@ public class WifiConfigManager { * @return true on success or not needed (fresh install), false otherwise. */ public boolean loadFromStore() { - // Get the hashfunction that is used to generate randomized MACs from the KeyStore - mMac = mMacAddressUtil.obtainMacRandHashFunction(Process.WIFI_UID); - if (mMac == null) { - Log.wtf(TAG, "Failed to obtain secret for MAC randomization." - + " All randomized MAC addresses are lost!"); - } // If the user unlock comes in before we load from store, which means the user store have // not been setup yet for the current user. Setup the user store before the read so that // configurations for the current user will also being loaded. diff --git a/tests/wifitests/src/com/android/server/wifi/MacAddressUtilTest.java b/tests/wifitests/src/com/android/server/wifi/MacAddressUtilTest.java index 253310840..7e598db31 100644 --- a/tests/wifitests/src/com/android/server/wifi/MacAddressUtilTest.java +++ b/tests/wifitests/src/com/android/server/wifi/MacAddressUtilTest.java @@ -29,6 +29,7 @@ import org.junit.Test; import org.mockito.Mock; import org.mockito.MockitoAnnotations; +import java.security.ProviderException; import java.util.Random; import javax.crypto.Mac; @@ -69,4 +70,18 @@ public class MacAddressUtilTest { assertTrue(WifiConfiguration.isValidMacAddressForRandomization(macAddress)); } } + + /** + * Verify the java.security.ProviderException is caught. + */ + @Test + public void testCalculatePersistentMacCatchesException() { + when(mMac.doFinal(any())).thenThrow(new ProviderException("error occurred")); + try { + WifiConfiguration config = WifiConfigurationTestUtil.createOpenNetwork(); + assertNull(mMacAddressUtil.calculatePersistentMacForConfiguration(config, mMac)); + } catch (Exception e) { + fail("Exception not caught."); + } + } } diff --git a/tests/wifitests/src/com/android/server/wifi/WifiConfigManagerTest.java b/tests/wifitests/src/com/android/server/wifi/WifiConfigManagerTest.java index 23eea328f..6fa1868cb 100644 --- a/tests/wifitests/src/com/android/server/wifi/WifiConfigManagerTest.java +++ b/tests/wifitests/src/com/android/server/wifi/WifiConfigManagerTest.java @@ -303,13 +303,27 @@ public class WifiConfigManagerTest { } /** - * Verifies that the Mac randomization secret hashfunction is obtained after |loadFromStore|. + * Verify that a randomized MAC address is generated even if the KeyStore operation fails. */ @Test - public void testMacHashIsObtainedAfterLoadFromStore() { - verify(mMacAddressUtil, never()).obtainMacRandHashFunction(anyInt()); - assertTrue(mWifiConfigManager.loadFromStore()); - verify(mMacAddressUtil).obtainMacRandHashFunction(anyInt()); + public void testRandomizedMacIsGeneratedEvenIfKeyStoreFails() { + when(mMacAddressUtil.calculatePersistentMacForConfiguration(any(), any())).thenReturn(null); + + // Try adding a network. + WifiConfiguration openNetwork = WifiConfigurationTestUtil.createOpenNetwork(); + List networks = new ArrayList<>(); + networks.add(openNetwork); + verifyAddNetworkToWifiConfigManager(openNetwork); + List retrievedNetworks = + mWifiConfigManager.getConfiguredNetworksWithPasswords(); + + // Verify that we have attempted to generate the MAC address twice (1 retry) + verify(mMacAddressUtil, times(2)).calculatePersistentMacForConfiguration(any(), any()); + assertEquals(1, retrievedNetworks.size()); + + // Verify that despite KeyStore returning null, we are still getting a valid MAC address. + assertNotEquals(WifiInfo.DEFAULT_MAC_ADDRESS, + retrievedNetworks.get(0).getRandomizedMacAddress().toString()); } /** -- cgit v1.2.3 From b4486b2a1bce042384b6da156aced9c8ff76b5ec Mon Sep 17 00:00:00 2001 From: xshu Date: Tue, 14 Jan 2020 12:12:22 -0800 Subject: fix soft reboot caused by KeyStore exception The Mac handle obtained from AndroidKeyStore is sometimes invalidated by the AndroidKeyStore based on some LRU technique. This change make sure that we always get a valid handle. And adds exception handling to make sure a crash will not happen for the same reason again. If KeyStore continuously fails to generate MAC address, we will use locally generated MAC as it is the next best option. Bug: 146203882 Bug: 140065828 Test: atest FrameworksWifiTests Merged-In: I8a3b810ba95898a96d81fe57979db4787e1a46c4 Change-Id: I8a3b810ba95898a96d81fe57979db4787e1a46c4 (cherry-picked from 0b3eca3c05190e5824638f3da25b8b3167dc9d60) --- .../com/android/server/wifi/WifiConfigManager.java | 21 ++++++++++++--------- .../android/server/wifi/WifiConfigurationUtil.java | 10 ++++++++-- .../android/server/wifi/WifiConfigManagerTest.java | 22 ++++++++++++++++++++++ .../server/wifi/WifiConfigurationUtilTest.java | 18 ++++++++++++++++++ 4 files changed, 60 insertions(+), 11 deletions(-) diff --git a/service/java/com/android/server/wifi/WifiConfigManager.java b/service/java/com/android/server/wifi/WifiConfigManager.java index 4ac4fb7a6..fb84cf0b1 100644 --- a/service/java/com/android/server/wifi/WifiConfigManager.java +++ b/service/java/com/android/server/wifi/WifiConfigManager.java @@ -76,8 +76,6 @@ import java.util.List; import java.util.Map; import java.util.Set; -import javax.crypto.Mac; - /** * This class provides the APIs to manage configured Wi-Fi networks. * It deals with the following: @@ -268,7 +266,6 @@ public class WifiConfigManager { */ private final Context mContext; private final Clock mClock; - private final Mac mMac; private final UserManager mUserManager; private final BackupManagerProxy mBackupManagerProxy; private final TelephonyManager mTelephonyManager; @@ -449,11 +446,6 @@ public class WifiConfigManager { } catch (PackageManager.NameNotFoundException e) { Log.e(TAG, "Unable to resolve SystemUI's UID."); } - mMac = WifiConfigurationUtil.obtainMacRandHashFunction(Process.WIFI_UID); - if (mMac == null) { - Log.wtf(TAG, "Failed to obtain secret for MAC randomization." - + " All randomized MAC addresses are lost!"); - } } /** @@ -505,7 +497,18 @@ public class WifiConfigManager { mRandomizedMacAddressMapping.remove(config.getSsidAndSecurityTypeString()); } } - return WifiConfigurationUtil.calculatePersistentMacForConfiguration(config, mMac); + MacAddress result = WifiConfigurationUtil.calculatePersistentMacForConfiguration(config, + WifiConfigurationUtil.obtainMacRandHashFunction(Process.WIFI_UID)); + if (result == null) { + result = WifiConfigurationUtil.calculatePersistentMacForConfiguration(config, + WifiConfigurationUtil.obtainMacRandHashFunction(Process.WIFI_UID)); + } + if (result == null) { + Log.wtf(TAG, "Failed to generate MAC address from KeyStore even after retrying. " + + "Using locally generated MAC address instead."); + result = MacAddress.createRandomUnicastAddress(); + } + return result; } /** diff --git a/service/java/com/android/server/wifi/WifiConfigurationUtil.java b/service/java/com/android/server/wifi/WifiConfigurationUtil.java index c59d4faf7..69a655bd4 100644 --- a/service/java/com/android/server/wifi/WifiConfigurationUtil.java +++ b/service/java/com/android/server/wifi/WifiConfigurationUtil.java @@ -259,8 +259,14 @@ public class WifiConfigurationUtil { if (config == null || hashFunction == null) { return null; } - byte[] hashedBytes = hashFunction.doFinal( - config.getSsidAndSecurityTypeString().getBytes(StandardCharsets.UTF_8)); + byte[] hashedBytes; + try { + hashedBytes = hashFunction.doFinal( + config.getSsidAndSecurityTypeString().getBytes(StandardCharsets.UTF_8)); + } catch (ProviderException | IllegalStateException e) { + Log.e(TAG, "Failure in calculatePersistentMac", e); + return null; + } ByteBuffer bf = ByteBuffer.wrap(hashedBytes); long longFromSsid = bf.getLong(); /** diff --git a/tests/wifitests/src/com/android/server/wifi/WifiConfigManagerTest.java b/tests/wifitests/src/com/android/server/wifi/WifiConfigManagerTest.java index dad88f3c0..ce1c6b2ee 100644 --- a/tests/wifitests/src/com/android/server/wifi/WifiConfigManagerTest.java +++ b/tests/wifitests/src/com/android/server/wifi/WifiConfigManagerTest.java @@ -291,6 +291,28 @@ public class WifiConfigManagerTest { mContextConfigStoreMockOrder.verify(mWifiConfigStore).write(anyBoolean()); } + /** + * Verify that a randomized MAC address is generated even if the KeyStore operation fails. + */ + @Test + public void testRandomizedMacIsGeneratedEvenIfKeyStoreFails() { + when(WifiConfigurationUtil.calculatePersistentMacForConfiguration( + any(), any())).thenReturn(null); + + // Try adding a network. + WifiConfiguration openNetwork = WifiConfigurationTestUtil.createOpenNetwork(); + List networks = new ArrayList<>(); + networks.add(openNetwork); + verifyAddNetworkToWifiConfigManager(openNetwork); + List retrievedNetworks = + mWifiConfigManager.getConfiguredNetworksWithPasswords(); + + // Verify that despite KeyStore returning null, we are still getting a valid MAC address. + assertEquals(1, retrievedNetworks.size()); + assertNotEquals(WifiInfo.DEFAULT_MAC_ADDRESS, + retrievedNetworks.get(0).getRandomizedMacAddress().toString()); + } + /** * Verifies the addition of a single network using * {@link WifiConfigManager#addOrUpdateNetwork(WifiConfiguration, int)} diff --git a/tests/wifitests/src/com/android/server/wifi/WifiConfigurationUtilTest.java b/tests/wifitests/src/com/android/server/wifi/WifiConfigurationUtilTest.java index 7173dae5b..6687390c1 100644 --- a/tests/wifitests/src/com/android/server/wifi/WifiConfigurationUtilTest.java +++ b/tests/wifitests/src/com/android/server/wifi/WifiConfigurationUtilTest.java @@ -17,6 +17,7 @@ package com.android.server.wifi; import static org.junit.Assert.*; +import static org.mockito.Mockito.*; import android.content.pm.UserInfo; import android.net.IpConfiguration; @@ -34,6 +35,7 @@ import androidx.test.filters.SmallTest; import org.junit.Test; +import java.security.ProviderException; import java.security.cert.X509Certificate; import java.util.ArrayList; import java.util.Arrays; @@ -991,6 +993,22 @@ public class WifiConfigurationUtilTest { } } + /** + * Verify the java.security.ProviderException is caught. + */ + @Test + public void testCalculatePersistentMacCatchesException() { + Mac hashFunction = mock(Mac.class); + when(hashFunction.doFinal(any())).thenThrow(new ProviderException("error occurred")); + try { + WifiConfiguration config = WifiConfigurationTestUtil.createOpenNetwork(); + assertNull(WifiConfigurationUtil.calculatePersistentMacForConfiguration(config, + hashFunction)); + } catch (Exception e) { + fail("Exception not caught."); + } + } + private static class EnterpriseConfig { public String eap; public String phase2; -- cgit v1.2.3 From d7d14da8eafb570f00287ac715d31ebca6fca164 Mon Sep 17 00:00:00 2001 From: Kai Shi Date: Tue, 25 Feb 2020 17:09:21 -0800 Subject: Add EAP methods in WifiMetrics To help find the root cause of high authentication failure rate in enterprise network, add EAP method and authentication phase2 method in connection event metrics. Bug: 150237135 Test: manual Test: atest com.android.server.wifi Change-Id: I0c4ab436b1e43cc52d4bd4dab6c01db53fec91a6 --- .../java/com/android/server/wifi/WifiMetrics.java | 55 ++++++++++++++++++++++ .../com/android/server/wifi/WifiMetricsTest.java | 22 +++++++++ 2 files changed, 77 insertions(+) diff --git a/service/java/com/android/server/wifi/WifiMetrics.java b/service/java/com/android/server/wifi/WifiMetrics.java index 7578d3723..6db4e9955 100644 --- a/service/java/com/android/server/wifi/WifiMetrics.java +++ b/service/java/com/android/server/wifi/WifiMetrics.java @@ -26,6 +26,7 @@ import android.net.wifi.IOnWifiUsabilityStatsListener; import android.net.wifi.ScanResult; import android.net.wifi.SupplicantState; import android.net.wifi.WifiConfiguration; +import android.net.wifi.WifiEnterpriseConfig; import android.net.wifi.WifiInfo; import android.net.wifi.WifiManager; import android.net.wifi.WifiManager.DeviceMobilityState; @@ -480,6 +481,8 @@ public class WifiMetrics { sb.append(", mHidden=" + mRouterFingerPrintProto.hidden); sb.append(", mRouterTechnology=" + mRouterFingerPrintProto.routerTechnology); sb.append(", mSupportsIpv6=" + mRouterFingerPrintProto.supportsIpv6); + sb.append(", mEapMethod=" + mRouterFingerPrintProto.eapMethod); + sb.append(", mAuthPhase2Method=" + mRouterFingerPrintProto.authPhase2Method); } return sb.toString(); } @@ -516,9 +519,61 @@ public class WifiMetrics { if (candidate != null) { updateMetricsFromScanResult(candidate); } + if (mCurrentConnectionEvent.mRouterFingerPrint.mRouterFingerPrintProto + .authentication == WifiMetricsProto.RouterFingerPrint.AUTH_ENTERPRISE + && config.enterpriseConfig != null) { + int eapMethod = config.enterpriseConfig.getEapMethod(); + mCurrentConnectionEvent.mRouterFingerPrint.mRouterFingerPrintProto + .eapMethod = getEapMethodProto(eapMethod); + int phase2Method = config.enterpriseConfig.getPhase2Method(); + mCurrentConnectionEvent.mRouterFingerPrint.mRouterFingerPrintProto + .authPhase2Method = getAuthPhase2MethodProto(phase2Method); + } } } } + private int getEapMethodProto(int eapMethod) { + switch (eapMethod) { + case WifiEnterpriseConfig.Eap.TLS: + return WifiMetricsProto.RouterFingerPrint.TYPE_EAP_TLS; + case WifiEnterpriseConfig.Eap.UNAUTH_TLS: + return WifiMetricsProto.RouterFingerPrint.TYPE_EAP_UNAUTH_TLS; + case WifiEnterpriseConfig.Eap.PEAP: + return WifiMetricsProto.RouterFingerPrint.TYPE_EAP_PEAP; + case WifiEnterpriseConfig.Eap.PWD: + return WifiMetricsProto.RouterFingerPrint.TYPE_EAP_PWD; + case WifiEnterpriseConfig.Eap.TTLS: + return WifiMetricsProto.RouterFingerPrint.TYPE_EAP_TTLS; + case WifiEnterpriseConfig.Eap.SIM: + return WifiMetricsProto.RouterFingerPrint.TYPE_EAP_SIM; + case WifiEnterpriseConfig.Eap.AKA: + return WifiMetricsProto.RouterFingerPrint.TYPE_EAP_AKA; + case WifiEnterpriseConfig.Eap.AKA_PRIME: + return WifiMetricsProto.RouterFingerPrint.TYPE_EAP_AKA_PRIME; + default: + return WifiMetricsProto.RouterFingerPrint.TYPE_EAP_UNKNOWN; + } + } + private int getAuthPhase2MethodProto(int phase2Method) { + switch (phase2Method) { + case WifiEnterpriseConfig.Phase2.PAP: + return WifiMetricsProto.RouterFingerPrint.TYPE_PHASE2_PAP; + case WifiEnterpriseConfig.Phase2.MSCHAP: + return WifiMetricsProto.RouterFingerPrint.TYPE_PHASE2_MSCHAP; + case WifiEnterpriseConfig.Phase2.MSCHAPV2: + return WifiMetricsProto.RouterFingerPrint.TYPE_PHASE2_MSCHAPV2; + case WifiEnterpriseConfig.Phase2.GTC: + return WifiMetricsProto.RouterFingerPrint.TYPE_PHASE2_GTC; + case WifiEnterpriseConfig.Phase2.SIM: + return WifiMetricsProto.RouterFingerPrint.TYPE_PHASE2_SIM; + case WifiEnterpriseConfig.Phase2.AKA: + return WifiMetricsProto.RouterFingerPrint.TYPE_PHASE2_AKA; + case WifiEnterpriseConfig.Phase2.AKA_PRIME: + return WifiMetricsProto.RouterFingerPrint.TYPE_PHASE2_AKA_PRIME; + default: + return WifiMetricsProto.RouterFingerPrint.TYPE_PHASE2_NONE; + } + } } /** diff --git a/tests/wifitests/src/com/android/server/wifi/WifiMetricsTest.java b/tests/wifitests/src/com/android/server/wifi/WifiMetricsTest.java index 73ac30f41..11769576c 100644 --- a/tests/wifitests/src/com/android/server/wifi/WifiMetricsTest.java +++ b/tests/wifitests/src/com/android/server/wifi/WifiMetricsTest.java @@ -58,6 +58,7 @@ import android.net.wifi.IOnWifiUsabilityStatsListener; import android.net.wifi.ScanResult; import android.net.wifi.SupplicantState; import android.net.wifi.WifiConfiguration; +import android.net.wifi.WifiEnterpriseConfig; import android.net.wifi.WifiInfo; import android.net.wifi.WifiManager; import android.net.wifi.WifiSsid; @@ -1371,10 +1372,17 @@ public class WifiMetricsTest { when(networkDetail.getDtimInterval()).thenReturn(NETWORK_DETAIL_DTIM); ScanResult scanResult = mock(ScanResult.class); scanResult.level = SCAN_RESULT_LEVEL; + scanResult.capabilities = "EAP"; WifiConfiguration config = mock(WifiConfiguration.class); config.SSID = "\"" + SSID + "\""; config.dtimInterval = CONFIG_DTIM; config.macRandomizationSetting = WifiConfiguration.RANDOMIZATION_PERSISTENT; + config.allowedKeyManagement = new BitSet(); + config.allowedKeyManagement.set(WifiConfiguration.KeyMgmt.WPA_EAP); + config.allowedKeyManagement.set(WifiConfiguration.KeyMgmt.IEEE8021X); + config.enterpriseConfig = new WifiEnterpriseConfig(); + config.enterpriseConfig.setEapMethod(WifiEnterpriseConfig.Eap.TTLS); + config.enterpriseConfig.setPhase2Method(WifiEnterpriseConfig.Phase2.MSCHAPV2); WifiConfiguration.NetworkSelectionStatus networkSelectionStat = mock(WifiConfiguration.NetworkSelectionStatus.class); when(networkSelectionStat.getCandidate()).thenReturn(scanResult); @@ -1395,7 +1403,9 @@ public class WifiMetricsTest { WifiMetricsProto.ConnectionEvent.HLF_NONE, WifiMetricsProto.ConnectionEvent.FAILURE_REASON_UNKNOWN); + //Change configuration to open without randomization config.macRandomizationSetting = WifiConfiguration.RANDOMIZATION_NONE; + scanResult.capabilities = ""; //Create a connection event using the config and a scan detail mWifiMetrics.startConnectionEvent(config, "Green", WifiMetricsProto.ConnectionEvent.ROAM_NONE); @@ -1411,8 +1421,20 @@ public class WifiMetricsTest { //Check that the correct values are being flowed through assertEquals(2, mDecodedProto.connectionEvent.length); assertEquals(CONFIG_DTIM, mDecodedProto.connectionEvent[0].routerFingerprint.dtim); + assertEquals(WifiMetricsProto.RouterFingerPrint.AUTH_ENTERPRISE, + mDecodedProto.connectionEvent[0].routerFingerprint.authentication); + assertEquals(WifiMetricsProto.RouterFingerPrint.TYPE_EAP_TTLS, + mDecodedProto.connectionEvent[0].routerFingerprint.eapMethod); + assertEquals(WifiMetricsProto.RouterFingerPrint.TYPE_PHASE2_MSCHAPV2, + mDecodedProto.connectionEvent[0].routerFingerprint.authPhase2Method); assertEquals(SCAN_RESULT_LEVEL, mDecodedProto.connectionEvent[0].signalStrength); assertEquals(NETWORK_DETAIL_DTIM, mDecodedProto.connectionEvent[1].routerFingerprint.dtim); + assertEquals(WifiMetricsProto.RouterFingerPrint.AUTH_OPEN, + mDecodedProto.connectionEvent[1].routerFingerprint.authentication); + assertEquals(WifiMetricsProto.RouterFingerPrint.TYPE_EAP_UNKNOWN, + mDecodedProto.connectionEvent[1].routerFingerprint.eapMethod); + assertEquals(WifiMetricsProto.RouterFingerPrint.TYPE_PHASE2_NONE, + mDecodedProto.connectionEvent[1].routerFingerprint.authPhase2Method); assertEquals(SCAN_RESULT_LEVEL, mDecodedProto.connectionEvent[1].signalStrength); assertEquals(NETWORK_DETAIL_WIFIMODE, mDecodedProto.connectionEvent[1].routerFingerprint.routerTechnology); -- cgit v1.2.3 From e6813a5870911b10561db2c259c072123c1c513a Mon Sep 17 00:00:00 2001 From: "Nate(Qiang) Jiang" Date: Mon, 2 Mar 2020 17:56:31 -0800 Subject: Enterprise suggestion's catificate share same lifecycle as suggestion Enterprise network suggestion's catificate will add to keystore immediately after add the suggestion, and will only remove after suggestion is removed. Bug: 150500247 Test: atest com.android.server.wifi Merged-In: I85fb81a98f16b6a343fb35ce31e1426e333773b0 Change-Id: Icbe49911d7ca93b03dfdf728ad88057c31aa5974 --- .../com/android/server/wifi/WifiConfigManager.java | 18 +++---- .../java/com/android/server/wifi/WifiInjector.java | 4 +- .../server/wifi/WifiNetworkSuggestionsManager.java | 30 +++++++++++- .../android/server/wifi/WifiConfigManagerTest.java | 6 ++- .../com/android/server/wifi/WifiKeyStoreTest.java | 56 ++++++++++++++++++++++ .../wifi/WifiNetworkSuggestionsManagerTest.java | 33 ++++++++++++- 6 files changed, 133 insertions(+), 14 deletions(-) diff --git a/service/java/com/android/server/wifi/WifiConfigManager.java b/service/java/com/android/server/wifi/WifiConfigManager.java index c617b9e1f..a2f10914a 100644 --- a/service/java/com/android/server/wifi/WifiConfigManager.java +++ b/service/java/com/android/server/wifi/WifiConfigManager.java @@ -1230,12 +1230,11 @@ public class WifiConfigManager { return new NetworkUpdateResult(WifiConfiguration.INVALID_NETWORK_ID); } - // Update the keys for non-Passpoint enterprise networks. For Passpoint, the certificates - // and keys are installed at the time the provider is installed. - if (config.enterpriseConfig != null - && config.enterpriseConfig.getEapMethod() != WifiEnterpriseConfig.Eap.NONE - && !config.isPasspoint()) { - if (!(mWifiKeyStore.updateNetworkKeys(newInternalConfig, existingInternalConfig))) { + // Update the keys for saved enterprise networks. For Passpoint, the certificates + // and keys are installed at the time the provider is installed. For suggestion enterprise + // network the certificates and keys are installed at the time the suggestion is added + if (!config.isPasspoint() && !config.fromWifiNetworkSuggestion && config.isEnterprise()) { + if (!mWifiKeyStore.updateNetworkKeys(newInternalConfig, existingInternalConfig)) { return new NetworkUpdateResult(WifiConfiguration.INVALID_NETWORK_ID); } } @@ -1372,9 +1371,10 @@ public class WifiConfigManager { if (mVerboseLoggingEnabled) { Log.v(TAG, "Removing network " + config.getPrintableSsid()); } - // Remove any associated enterprise keys for non-Passpoint networks. - if (!config.isPasspoint() && config.enterpriseConfig != null - && config.enterpriseConfig.getEapMethod() != WifiEnterpriseConfig.Eap.NONE) { + // Remove any associated enterprise keys for saved enterprise networks. Passpoint network + // will remove the enterprise keys when provider is uninstalled. Suggestion enterprise + // networks will remove the enterprise keys when suggestion is removed. + if (!config.isPasspoint() && !config.fromWifiNetworkSuggestion && config.isEnterprise()) { mWifiKeyStore.removeKeys(config.enterpriseConfig); } diff --git a/service/java/com/android/server/wifi/WifiInjector.java b/service/java/com/android/server/wifi/WifiInjector.java index 339e169f6..a234d4d81 100644 --- a/service/java/com/android/server/wifi/WifiInjector.java +++ b/service/java/com/android/server/wifi/WifiInjector.java @@ -277,8 +277,8 @@ public class WifiInjector { mWifiConfigManager, mClock, mConnectivityLocalLog, mWifiConnectivityHelper, subscriptionManager); mWifiNetworkSuggestionsManager = new WifiNetworkSuggestionsManager(mContext, - new Handler(mWifiCoreHandlerThread.getLooper()), this, - mWifiPermissionsUtil, mWifiConfigManager, mWifiConfigStore, mWifiMetrics); + new Handler(mWifiCoreHandlerThread.getLooper()), this, mWifiPermissionsUtil, + mWifiConfigManager, mWifiConfigStore, mWifiMetrics, mWifiKeyStore); mNetworkSuggestionEvaluator = new NetworkSuggestionEvaluator(mWifiNetworkSuggestionsManager, mWifiConfigManager, mConnectivityLocalLog); mScoredNetworkEvaluator = new ScoredNetworkEvaluator(context, clientModeImplLooper, diff --git a/service/java/com/android/server/wifi/WifiNetworkSuggestionsManager.java b/service/java/com/android/server/wifi/WifiNetworkSuggestionsManager.java index 644eb6523..19603d8c6 100644 --- a/service/java/com/android/server/wifi/WifiNetworkSuggestionsManager.java +++ b/service/java/com/android/server/wifi/WifiNetworkSuggestionsManager.java @@ -108,6 +108,7 @@ public class WifiNetworkSuggestionsManager { private final WifiMetrics mWifiMetrics; private final WifiInjector mWifiInjector; private final FrameworkFacade mFrameworkFacade; + private final WifiKeyStore mWifiKeyStore; /** * Per app meta data to store network suggestions, status, etc for each app providing network @@ -166,6 +167,10 @@ public class WifiNetworkSuggestionsManager { @NonNull PerAppInfo perAppInfo) { this.wns = wns; this.perAppInfo = perAppInfo; + this.wns.wifiConfiguration.fromWifiNetworkSuggestion = true; + this.wns.wifiConfiguration.ephemeral = true; + this.wns.wifiConfiguration.creatorName = perAppInfo.packageName; + this.wns.wifiConfiguration.creatorUid = wns.suggestorUid; } @Override @@ -384,7 +389,8 @@ public class WifiNetworkSuggestionsManager { WifiPermissionsUtil wifiPermissionsUtil, WifiConfigManager wifiConfigManager, WifiConfigStore wifiConfigStore, - WifiMetrics wifiMetrics) { + WifiMetrics wifiMetrics, + WifiKeyStore keyStore) { mContext = context; mResources = context.getResources(); mHandler = handler; @@ -397,6 +403,7 @@ public class WifiNetworkSuggestionsManager { mWifiPermissionsUtil = wifiPermissionsUtil; mWifiConfigManager = wifiConfigManager; mWifiMetrics = wifiMetrics; + mWifiKeyStore = keyStore; // register the data store for serializing/deserializing data. wifiConfigStore.registerStoreData( @@ -595,6 +602,19 @@ public class WifiNetworkSuggestionsManager { // Start tracking app-op changes from the app if they have active suggestions. startTrackingAppOpsChange(packageName, uid); } + Iterator iterator = extNetworkSuggestions.iterator(); + // Install enterprise network suggestion catificate. + while (iterator.hasNext()) { + WifiConfiguration config = iterator.next().wns.wifiConfiguration; + if (!config.isEnterprise()) { + continue; + } + if (!mWifiKeyStore.updateNetworkKeys(config, null)) { + Log.e(TAG, "Enterprise network install failure for SSID: " + + config.SSID); + iterator.remove(); + } + } perAppInfo.extNetworkSuggestions.addAll(extNetworkSuggestions); // Update the max size for this app. perAppInfo.maxSize = Math.max(perAppInfo.extNetworkSuggestions.size(), perAppInfo.maxSize); @@ -634,6 +654,14 @@ public class WifiNetworkSuggestionsManager { // Stop tracking app-op changes from the app if they don't have active suggestions. stopTrackingAppOpsChange(packageName); } + // Clean the enterprise certifiacte. + for (ExtendedWifiNetworkSuggestion ewns : extNetworkSuggestions) { + WifiConfiguration config = ewns.wns.wifiConfiguration; + if (!config.isEnterprise()) { + continue; + } + mWifiKeyStore.removeKeys(config.enterpriseConfig); + } // Clear the scan cache. removeFromScanResultMatchInfoMap(extNetworkSuggestions); } diff --git a/tests/wifitests/src/com/android/server/wifi/WifiConfigManagerTest.java b/tests/wifitests/src/com/android/server/wifi/WifiConfigManagerTest.java index 6fa1868cb..6a7785e5e 100644 --- a/tests/wifitests/src/com/android/server/wifi/WifiConfigManagerTest.java +++ b/tests/wifitests/src/com/android/server/wifi/WifiConfigManagerTest.java @@ -683,13 +683,14 @@ public class WifiConfigManagerTest { */ @Test public void testAddSingleSuggestionNetwork() throws Exception { - WifiConfiguration suggestionNetwork = WifiConfigurationTestUtil.createOpenNetwork(); + WifiConfiguration suggestionNetwork = WifiConfigurationTestUtil.createEapNetwork(); suggestionNetwork.ephemeral = true; suggestionNetwork.fromWifiNetworkSuggestion = true; List networks = new ArrayList<>(); networks.add(suggestionNetwork); verifyAddSuggestionOrRequestNetworkToWifiConfigManager(suggestionNetwork); + verify(mWifiKeyStore, never()).updateNetworkKeys(any(), any()); List retrievedNetworks = mWifiConfigManager.getConfiguredNetworksWithPasswords(); @@ -699,6 +700,9 @@ public class WifiConfigManagerTest { // Ensure that this is not returned in the saved network list. assertTrue(mWifiConfigManager.getSavedNetworks(Process.WIFI_UID).isEmpty()); verify(mWcmListener, never()).onSavedNetworkAdded(suggestionNetwork.networkId); + assertTrue(mWifiConfigManager + .removeNetwork(suggestionNetwork.networkId, TEST_CREATOR_UID)); + verify(mWifiKeyStore, never()).removeKeys(any()); } /** diff --git a/tests/wifitests/src/com/android/server/wifi/WifiKeyStoreTest.java b/tests/wifitests/src/com/android/server/wifi/WifiKeyStoreTest.java index 7079a2d53..7649d1ba4 100644 --- a/tests/wifitests/src/com/android/server/wifi/WifiKeyStoreTest.java +++ b/tests/wifitests/src/com/android/server/wifi/WifiKeyStoreTest.java @@ -16,11 +16,19 @@ package com.android.server.wifi; +import static org.junit.Assert.assertNotEquals; +import static org.junit.Assert.assertTrue; +import static org.mockito.AdditionalMatchers.aryEq; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyInt; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.validateMockitoUsage; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.when; +import android.net.wifi.WifiConfiguration; import android.net.wifi.WifiEnterpriseConfig; import android.os.Process; import android.security.Credentials; @@ -34,6 +42,8 @@ import org.junit.Test; import org.mockito.Mock; import org.mockito.MockitoAnnotations; +import java.security.cert.X509Certificate; + /** * Unit tests for {@link com.android.server.wifi.WifiConfigManager}. */ @@ -43,8 +53,10 @@ public class WifiKeyStoreTest { @Mock private KeyStore mKeyStore; private WifiKeyStore mWifiKeyStore; + private static final String TEST_KEY_ID = "blah"; private static final String USER_CERT_ALIAS = "aabbccddee"; private static final String [] USER_CA_CERT_ALIAS = {"aacccddd", "bbbqqqqmmm"}; + private static final String TEST_PACKAGE_NAME = "TestApp"; /** * Setup the mocks and an instance of WifiConfigManager before each test. @@ -57,6 +69,16 @@ public class WifiKeyStoreTest { when(mWifiEnterpriseConfig.getClientCertificateAlias()).thenReturn(USER_CERT_ALIAS); when(mWifiEnterpriseConfig.getCaCertificateAliases()) .thenReturn(USER_CA_CERT_ALIAS); + when(mKeyStore.put(anyString(), any(), anyInt(), anyInt())).thenReturn(true); + when(mKeyStore.importKey(anyString(), any(), anyInt(), anyInt())).thenReturn(true); + when(mWifiEnterpriseConfig.getClientPrivateKey()).thenReturn(FakeKeys.RSA_KEY1); + when(mWifiEnterpriseConfig.getClientCertificate()).thenReturn(FakeKeys.CLIENT_CERT); + when(mWifiEnterpriseConfig.getCaCertificate()).thenReturn(FakeKeys.CA_CERT0); + when(mWifiEnterpriseConfig.getClientCertificateChain()) + .thenReturn(new X509Certificate[] {FakeKeys.CLIENT_CERT}); + when(mWifiEnterpriseConfig.getCaCertificates()) + .thenReturn(new X509Certificate[] {FakeKeys.CA_CERT0}); + when(mWifiEnterpriseConfig.getKeyId(any())).thenReturn(TEST_KEY_ID); } /** @@ -129,4 +151,38 @@ public class WifiKeyStoreTest { mWifiKeyStore.removeKeys(mWifiEnterpriseConfig); verifyNoMoreInteractions(mKeyStore); } + + /** + * Add two same network credential one is from user saved, the other is from suggestion. + * Both oh them should be installed successfully and has different alias, and will not override + * each other. + */ + @Test + public void testAddFromBothSavedAndSuggestionNetwork() throws Exception { + WifiConfiguration savedNetwork = WifiConfigurationTestUtil.createEapNetwork(); + WifiConfiguration suggestionNetwork = new WifiConfiguration(savedNetwork); + savedNetwork.enterpriseConfig = mWifiEnterpriseConfig; + suggestionNetwork.enterpriseConfig = mWifiEnterpriseConfig; + suggestionNetwork.fromWifiNetworkSuggestion = true; + suggestionNetwork.creatorName = TEST_PACKAGE_NAME; + + assertTrue(mWifiKeyStore.updateNetworkKeys(savedNetwork, null)); + assertTrue(mWifiKeyStore.updateNetworkKeys(suggestionNetwork, null)); + + String savedNetworkAlias = savedNetwork.getKeyIdForCredentials(null); + String savedNetworkCaAlias = savedNetworkAlias; + + String suggestionNetworkAlias = suggestionNetwork.getKeyIdForCredentials(null); + String suggestionNetworkCaAlias = suggestionNetworkAlias; + + assertNotEquals(savedNetworkAlias, suggestionNetworkAlias); + + verify(mWifiEnterpriseConfig).setClientCertificateAlias(eq(savedNetworkAlias)); + verify(mWifiEnterpriseConfig).setCaCertificateAliases( + aryEq(new String[] {savedNetworkCaAlias})); + + verify(mWifiEnterpriseConfig).setClientCertificateAlias(eq(suggestionNetworkAlias)); + verify(mWifiEnterpriseConfig).setCaCertificateAliases( + aryEq(new String[] {suggestionNetworkCaAlias})); + } } diff --git a/tests/wifitests/src/com/android/server/wifi/WifiNetworkSuggestionsManagerTest.java b/tests/wifitests/src/com/android/server/wifi/WifiNetworkSuggestionsManagerTest.java index 612fdf575..9e183e729 100644 --- a/tests/wifitests/src/com/android/server/wifi/WifiNetworkSuggestionsManagerTest.java +++ b/tests/wifitests/src/com/android/server/wifi/WifiNetworkSuggestionsManagerTest.java @@ -102,6 +102,7 @@ public class WifiNetworkSuggestionsManagerTest { private @Mock NetworkSuggestionStoreData mNetworkSuggestionStoreData; private @Mock ClientModeImpl mClientModeImpl; private @Mock WifiMetrics mWifiMetrics; + private @Mock WifiKeyStore mWifiKeyStore; private TestLooper mLooper; private ArgumentCaptor mAppOpChangedListenerCaptor = ArgumentCaptor.forClass(AppOpsManager.OnOpChangedListener.class); @@ -161,7 +162,7 @@ public class WifiNetworkSuggestionsManagerTest { mWifiNetworkSuggestionsManager = new WifiNetworkSuggestionsManager(mContext, new Handler(mLooper.getLooper()), mWifiInjector, mWifiPermissionsUtil, mWifiConfigManager, mWifiConfigStore, - mWifiMetrics); + mWifiMetrics, mWifiKeyStore); verify(mContext).getResources(); verify(mContext).getSystemService(Context.APP_OPS_SERVICE); verify(mContext).getSystemService(Context.NOTIFICATION_SERVICE); @@ -308,6 +309,36 @@ public class WifiNetworkSuggestionsManagerTest { assertTrue(mWifiNetworkSuggestionsManager.getAllNetworkSuggestions().isEmpty()); } + @Test + public void testAddRemoveEnterpriseNetworkSuggestion() { + WifiNetworkSuggestion networkSuggestion1 = new WifiNetworkSuggestion( + WifiConfigurationTestUtil.createEapNetwork(), false, false, TEST_UID_1, + TEST_PACKAGE_1); + WifiNetworkSuggestion networkSuggestion2 = new WifiNetworkSuggestion( + WifiConfigurationTestUtil.createEapNetwork(), false, false, TEST_UID_2, + TEST_PACKAGE_2); + + List networkSuggestionList = + new ArrayList() {{ + add(networkSuggestion1); + add(networkSuggestion2); + }}; + when(mWifiKeyStore.updateNetworkKeys(eq(networkSuggestion1.wifiConfiguration), any())) + .thenReturn(true); + when(mWifiKeyStore.updateNetworkKeys(eq(networkSuggestion2.wifiConfiguration), any())) + .thenReturn(false); + assertEquals(WifiManager.STATUS_NETWORK_SUGGESTIONS_SUCCESS, + mWifiNetworkSuggestionsManager.add(networkSuggestionList, TEST_UID_1, + TEST_PACKAGE_1)); + + Set allNetworkSuggestions = + mWifiNetworkSuggestionsManager.getAllNetworkSuggestions(); + assertEquals(1, allNetworkSuggestions.size()); + assertEquals(WifiManager.STATUS_NETWORK_SUGGESTIONS_SUCCESS, + mWifiNetworkSuggestionsManager.remove(new ArrayList<>(), + TEST_UID_1, TEST_PACKAGE_1)); + verify(mWifiKeyStore).removeKeys(any()); + } /** * Verify successful replace (add,remove, add) of network suggestions. */ -- cgit v1.2.3 From 924050178386e73cc459ec19e47afd7c304c4068 Mon Sep 17 00:00:00 2001 From: "Nate(Qiang) Jiang" Date: Fri, 6 Mar 2020 16:05:17 -0800 Subject: Fix removing enterprise suggestion certificate Make when remove a enterprise suggestion, use the internal config object to remove ketStore. Which has certificate alias. Bug: 150500247 Test: atest com.android.server.wifi Merged-In: Id020e16fb6c26f38eb217248f7296b9ae4b0e781 Change-Id: I8015d87599d843e022805ce69fd5a4eae4d7b066 --- .../com/android/server/wifi/WifiNetworkSuggestionsManager.java | 10 +++++++--- .../android/server/wifi/WifiNetworkSuggestionsManagerTest.java | 4 ++++ 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/service/java/com/android/server/wifi/WifiNetworkSuggestionsManager.java b/service/java/com/android/server/wifi/WifiNetworkSuggestionsManager.java index 19603d8c6..426dddb8e 100644 --- a/service/java/com/android/server/wifi/WifiNetworkSuggestionsManager.java +++ b/service/java/com/android/server/wifi/WifiNetworkSuggestionsManager.java @@ -639,11 +639,15 @@ public class WifiNetworkSuggestionsManager { @NonNull Collection extNetworkSuggestions, @NonNull String packageName, @NonNull PerAppInfo perAppInfo) { + // Get internal suggestions + Set removingSuggestions = + new HashSet<>(perAppInfo.extNetworkSuggestions); if (!extNetworkSuggestions.isEmpty()) { + // Keep the internal suggestions need to remove. + removingSuggestions.retainAll(extNetworkSuggestions); perAppInfo.extNetworkSuggestions.removeAll(extNetworkSuggestions); } else { // empty list is used to clear everything for the app. Store a copy for use below. - extNetworkSuggestions = new HashSet<>(perAppInfo.extNetworkSuggestions); perAppInfo.extNetworkSuggestions.clear(); } if (perAppInfo.extNetworkSuggestions.isEmpty()) { @@ -655,7 +659,7 @@ public class WifiNetworkSuggestionsManager { stopTrackingAppOpsChange(packageName); } // Clean the enterprise certifiacte. - for (ExtendedWifiNetworkSuggestion ewns : extNetworkSuggestions) { + for (ExtendedWifiNetworkSuggestion ewns : removingSuggestions) { WifiConfiguration config = ewns.wns.wifiConfiguration; if (!config.isEnterprise()) { continue; @@ -663,7 +667,7 @@ public class WifiNetworkSuggestionsManager { mWifiKeyStore.removeKeys(config.enterpriseConfig); } // Clear the scan cache. - removeFromScanResultMatchInfoMap(extNetworkSuggestions); + removeFromScanResultMatchInfoMap(removingSuggestions); } /** diff --git a/tests/wifitests/src/com/android/server/wifi/WifiNetworkSuggestionsManagerTest.java b/tests/wifitests/src/com/android/server/wifi/WifiNetworkSuggestionsManagerTest.java index 9e183e729..dabdfd569 100644 --- a/tests/wifitests/src/com/android/server/wifi/WifiNetworkSuggestionsManagerTest.java +++ b/tests/wifitests/src/com/android/server/wifi/WifiNetworkSuggestionsManagerTest.java @@ -334,6 +334,10 @@ public class WifiNetworkSuggestionsManagerTest { Set allNetworkSuggestions = mWifiNetworkSuggestionsManager.getAllNetworkSuggestions(); assertEquals(1, allNetworkSuggestions.size()); + WifiNetworkSuggestion removingSuggestion = new WifiNetworkSuggestion( + WifiConfigurationTestUtil.createEapNetwork(), false, false, TEST_UID_1, + TEST_PACKAGE_1); + removingSuggestion.wifiConfiguration.SSID = networkSuggestion1.wifiConfiguration.SSID; assertEquals(WifiManager.STATUS_NETWORK_SUGGESTIONS_SUCCESS, mWifiNetworkSuggestionsManager.remove(new ArrayList<>(), TEST_UID_1, TEST_PACKAGE_1)); -- cgit v1.2.3 From fffc50f24d3a0e7f250a91e40e758a4247560b8c Mon Sep 17 00:00:00 2001 From: Michael Plass Date: Tue, 18 Feb 2020 15:02:20 -0800 Subject: [WifiConfigManager] Log the disable reason when re-enabling. A network may have been disabled a long time ago, and re-enabling it wipes out the reason it was disabled in the first place. This can be an important clue for diagnosing problems from a bug report, so log the previous status and reason before re-enabling a network. Bug: 149238341 Test: atest com.android.server.wifi Test: manual - check that the logging is present dumpsys output: adb shell dumpsys wifi | grep -10 '^WifiConfigManager' Change-Id: Id895918790a58e901b043bf607f83b30df9e8ebc Merged-In: Id895918790a58e901b043bf607f83b30df9e8ebc --- service/java/com/android/server/wifi/WifiConfigManager.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/service/java/com/android/server/wifi/WifiConfigManager.java b/service/java/com/android/server/wifi/WifiConfigManager.java index a2f10914a..9472367fb 100644 --- a/service/java/com/android/server/wifi/WifiConfigManager.java +++ b/service/java/com/android/server/wifi/WifiConfigManager.java @@ -1558,6 +1558,12 @@ public class WifiConfigManager { */ private void setNetworkSelectionEnabled(WifiConfiguration config) { NetworkSelectionStatus status = config.getNetworkSelectionStatus(); + if (status.getNetworkSelectionStatus() + != NetworkSelectionStatus.NETWORK_SELECTION_ENABLED) { + localLog("setNetworkSelectionEnabled: configKey=" + config.configKey() + + " old networkStatus=" + status.getNetworkStatusString() + + " disableReason=" + status.getNetworkDisableReasonString()); + } status.setNetworkSelectionStatus( NetworkSelectionStatus.NETWORK_SELECTION_ENABLED); status.setDisableTime( -- cgit v1.2.3 From 4354285b8de3345d5146f68ce2d17be83b4f34a4 Mon Sep 17 00:00:00 2001 From: Hai Shalom Date: Wed, 11 Mar 2020 13:57:36 -0700 Subject: [Passpoint] Fix Passpoint matching algorithm for Home networks Match Home networks if there is an FQDN match (without realm or method), or if OtherHomePartners element exists and matches the other advertised FQDNs, or if HomeOIList element exists and matches advertised RCOIs. Bug: 151245024 Test: atest PasspointProviderTest ANQPMatcherTest Change-Id: I7bf9c98c853c3973d30ce2651bdea49546dae4b5 Merged-In: I513f83a6d545b9ae7da5577ee163ce7d186f006e --- .../android/server/wifi/hotspot2/ANQPMatcher.java | 63 ++----- .../server/wifi/hotspot2/PasspointManager.java | 6 + .../server/wifi/hotspot2/PasspointProvider.java | 174 +++++++++++++++---- .../server/wifi/hotspot2/ANQPMatcherTest.java | 127 ++++++++------ .../wifi/hotspot2/PasspointProviderTest.java | 189 ++++++++++++++++++++- 5 files changed, 421 insertions(+), 138 deletions(-) diff --git a/service/java/com/android/server/wifi/hotspot2/ANQPMatcher.java b/service/java/com/android/server/wifi/hotspot2/ANQPMatcher.java index d95ab3831..69f98b0ef 100644 --- a/service/java/com/android/server/wifi/hotspot2/ANQPMatcher.java +++ b/service/java/com/android/server/wifi/hotspot2/ANQPMatcher.java @@ -75,10 +75,11 @@ public class ANQPMatcher { * * @param element The Roaming Consortium ANQP element * @param providerOIs The roaming consortium OIs of the provider + * @param matchAll Indicates if a match with all OIs must be done * @return true if a match is found */ public static boolean matchRoamingConsortium(RoamingConsortiumElement element, - long[] providerOIs) { + long[] providerOIs, boolean matchAll) { if (element == null) { return false; } @@ -88,10 +89,14 @@ public class ANQPMatcher { List rcOIs = element.getOIs(); for (long oi : providerOIs) { if (rcOIs.contains(oi)) { - return true; + if (!matchAll) { + return true; + } + } else if (matchAll) { + return false; } } - return false; + return matchAll; } /** @@ -100,27 +105,19 @@ public class ANQPMatcher { * * @param element The NAI Realm ANQP element * @param realm The realm of the provider's credential - * @param eapMethodID The EAP Method ID of the provider's credential - * @param authParam The authentication parameter of the provider's credential * @return an integer indicating the match status */ - public static int matchNAIRealm(NAIRealmElement element, String realm, int eapMethodID, - AuthParam authParam) { + public static boolean matchNAIRealm(NAIRealmElement element, String realm) { if (element == null || element.getRealmDataList().isEmpty()) { - return AuthMatch.INDETERMINATE; + return false; } - int bestMatch = AuthMatch.NONE; for (NAIRealmData realmData : element.getRealmDataList()) { - int match = matchNAIRealmData(realmData, realm, eapMethodID, authParam); - if (match > bestMatch) { - bestMatch = match; - if (bestMatch == AuthMatch.EXACT) { - break; - } + if (matchNAIRealmData(realmData, realm)) { + return true; } } - return bestMatch; + return false; } /** @@ -172,42 +169,16 @@ public class ANQPMatcher { * * @param realmData The NAI Realm data * @param realm The realm of the provider's credential - * @param eapMethodID The EAP Method ID of the provider's credential - * @param authParam The authentication parameter of the provider's credential - * @return an integer indicating the match status + * @return true if a match is found */ - private static int matchNAIRealmData(NAIRealmData realmData, String realm, int eapMethodID, - AuthParam authParam) { + private static boolean matchNAIRealmData(NAIRealmData realmData, String realm) { // Check for realm domain name match. - int realmMatch = AuthMatch.NONE; for (String realmStr : realmData.getRealms()) { if (DomainMatcher.arg2SubdomainOfArg1(realm, realmStr)) { - realmMatch = AuthMatch.REALM; - break; - } - } - - if (realmData.getEAPMethods().isEmpty()) { - return realmMatch; - } - - // Check for EAP method match. - int eapMethodMatch = AuthMatch.NONE; - for (EAPMethod eapMethod : realmData.getEAPMethods()) { - eapMethodMatch = matchEAPMethod(eapMethod, eapMethodID, authParam); - if (eapMethodMatch != AuthMatch.NONE) { - break; + return true; } } - - if (eapMethodMatch == AuthMatch.NONE) { - return AuthMatch.NONE; - } - - if (realmMatch == AuthMatch.NONE) { - return eapMethodMatch; - } - return realmMatch | eapMethodMatch; + return false; } private static int getEapMethodForNAIRealmWithCarrier(String realm, diff --git a/service/java/com/android/server/wifi/hotspot2/PasspointManager.java b/service/java/com/android/server/wifi/hotspot2/PasspointManager.java index c46873761..172d1a13d 100644 --- a/service/java/com/android/server/wifi/hotspot2/PasspointManager.java +++ b/service/java/com/android/server/wifi/hotspot2/PasspointManager.java @@ -217,6 +217,7 @@ public class PasspointManager { public void setProviders(List providers) { mProviders.clear(); for (PasspointProvider provider : providers) { + provider.enableVerboseLogging(mVerboseLoggingEnabled ? 1 : 0); mProviders.put(provider.getConfig().getHomeSp().getFqdn(), provider); if (provider.getPackageName() != null) { startTrackingAppOpsChange(provider.getPackageName(), @@ -377,6 +378,9 @@ public class PasspointManager { public void enableVerboseLogging(int verbose) { mVerboseLoggingEnabled = (verbose > 0) ? true : false; mPasspointProvisioner.enableVerboseLogging(verbose); + for (PasspointProvider provider : mProviders.values()) { + provider.enableVerboseLogging(verbose); + } } /** @@ -433,6 +437,7 @@ public class PasspointManager { mProviders.get(config.getHomeSp().getFqdn()).uninstallCertsAndKeys(); mProviders.remove(config.getHomeSp().getFqdn()); } + newProvider.enableVerboseLogging(mVerboseLoggingEnabled ? 1 : 0); mProviders.put(config.getHomeSp().getFqdn(), newProvider); mWifiConfigManager.saveToStore(true /* forceWrite */); if (newProvider.getPackageName() != null) { @@ -1165,6 +1170,7 @@ public class PasspointManager { Arrays.asList(enterpriseConfig.getCaCertificateAlias()), enterpriseConfig.getClientCertificateAlias(), enterpriseConfig.getClientCertificateAlias(), null, false, false); + provider.enableVerboseLogging(mVerboseLoggingEnabled ? 1 : 0); mProviders.put(passpointConfig.getHomeSp().getFqdn(), provider); return true; } diff --git a/service/java/com/android/server/wifi/hotspot2/PasspointProvider.java b/service/java/com/android/server/wifi/hotspot2/PasspointProvider.java index ca9814aa6..8db71d3e6 100644 --- a/service/java/com/android/server/wifi/hotspot2/PasspointProvider.java +++ b/service/java/com/android/server/wifi/hotspot2/PasspointProvider.java @@ -95,6 +95,7 @@ public class PasspointProvider { private boolean mHasEverConnected; private boolean mIsShared; + private boolean mVerboseLoggingEnabled; /** * This is a flag to indicate if the Provider is created temporarily. @@ -327,39 +328,53 @@ public class PasspointProvider { * Return the matching status with the given AP, based on the ANQP elements from the AP. * * @param anqpElements ANQP elements from the AP - * @param roamingConsortium Roaming Consortium information element from the AP + * @param roamingConsortiumFromAp Roaming Consortium information element from the AP * @return {@link PasspointMatch} */ public PasspointMatch match(Map anqpElements, - RoamingConsortium roamingConsortium) { - PasspointMatch providerMatch = matchProviderExceptFor3GPP(anqpElements, roamingConsortium); + RoamingConsortium roamingConsortiumFromAp) { + // Match FQDN for Home provider or RCOI(s) for Roaming provider + // For SIM credential, the FQDN is in the format of wlan.mnc*.mcc*.3gppnetwork.org + PasspointMatch providerMatch = matchFqdnAndRcoi(anqpElements, roamingConsortiumFromAp); // 3GPP Network matching. if (providerMatch == PasspointMatch.None && ANQPMatcher.matchThreeGPPNetwork( (ThreeGPPNetworkElement) anqpElements.get(ANQPElementType.ANQP3GPPNetwork), mImsiParameter, mMatchingSIMImsiList)) { + if (mVerboseLoggingEnabled) { + Log.d(TAG, "Final RoamingProvider match with " + + anqpElements.get(ANQPElementType.ANQP3GPPNetwork)); + } return PasspointMatch.RoamingProvider; } - // Perform authentication match against the NAI Realm. - int authMatch = ANQPMatcher.matchNAIRealm( + // Perform NAI Realm matching + boolean realmMatch = ANQPMatcher.matchNAIRealm( (NAIRealmElement) anqpElements.get(ANQPElementType.ANQPNAIRealm), - mConfig.getCredential().getRealm(), mEAPMethodID, mAuthParam); - - // In case of Auth mismatch, demote provider match. - if (authMatch == AuthMatch.NONE) { - return PasspointMatch.None; - } + mConfig.getCredential().getRealm()); // In case of no realm match, return provider match as is. - if ((authMatch & AuthMatch.REALM) == 0) { + if (!realmMatch) { + if (mVerboseLoggingEnabled) { + Log.d(TAG, "No NAI realm match, final match: " + providerMatch); + } return providerMatch; } - // Promote the provider match to roaming provider if provider match is not found, but NAI + if (mVerboseLoggingEnabled) { + Log.d(TAG, "NAI realm match with " + mConfig.getCredential().getRealm()); + } + + // Promote the provider match to RoamingProvider if provider match is not found, but NAI // realm is matched. - return providerMatch == PasspointMatch.None ? PasspointMatch.RoamingProvider - : providerMatch; + if (providerMatch == PasspointMatch.None) { + providerMatch = PasspointMatch.RoamingProvider; + } + + if (mVerboseLoggingEnabled) { + Log.d(TAG, "Final match: " + providerMatch); + } + return providerMatch; } /** @@ -569,43 +584,130 @@ public class PasspointProvider { || passpointConfig.getUsageLimitTimeLimitInMinutes() > 0; } + /** + * Match given OIs to the Roaming Consortium OIs + * + * @param providerOis Provider OIs to match against + * @param roamingConsortiumElement RCOIs in the ANQP element + * @param roamingConsortiumFromAp RCOIs in the AP scan results + * @param matchAll Indicates if all providerOis must match the RCOIs elements + * @return {@code true} if there is a match, {@code false} otherwise. + */ + private boolean matchOis(long[] providerOis, + RoamingConsortiumElement roamingConsortiumElement, + RoamingConsortium roamingConsortiumFromAp, + boolean matchAll) { + + + // ANQP Roaming Consortium OI matching. + if (ANQPMatcher.matchRoamingConsortium(roamingConsortiumElement, providerOis, matchAll)) { + if (mVerboseLoggingEnabled) { + Log.e(TAG, "ANQP RCOI match " + roamingConsortiumElement); + } + return true; + } + + // AP Roaming Consortium OI matching. + long[] apRoamingConsortiums = roamingConsortiumFromAp.getRoamingConsortiums(); + if (apRoamingConsortiums == null || providerOis == null) { + return false; + } + // Roaming Consortium OI information element matching. + for (long apOi: apRoamingConsortiums) { + boolean matched = false; + for (long providerOi: providerOis) { + if (apOi == providerOi) { + if (mVerboseLoggingEnabled) { + Log.e(TAG, "AP RCOI match: " + apOi); + } + if (!matchAll) { + return true; + } else { + matched = true; + break; + } + } + } + if (matchAll && !matched) { + return false; + } + } + return matchAll; + } + /** * Perform a provider match based on the given ANQP elements except for matching 3GPP Network. * * @param anqpElements List of ANQP elements - * @param roamingConsortium Roaming Consortium information element from the AP + * @param roamingConsortiumFromAp Roaming Consortium information element from the AP * @return {@link PasspointMatch} */ - private PasspointMatch matchProviderExceptFor3GPP( + private PasspointMatch matchFqdnAndRcoi( Map anqpElements, - RoamingConsortium roamingConsortium) { + RoamingConsortium roamingConsortiumFromAp) { // Domain name matching. if (ANQPMatcher.matchDomainName( (DomainNameElement) anqpElements.get(ANQPElementType.ANQPDomName), mConfig.getHomeSp().getFqdn(), mImsiParameter, mMatchingSIMImsiList)) { + if (mVerboseLoggingEnabled) { + Log.d(TAG, "Domain name " + mConfig.getHomeSp().getFqdn() + + " match: HomeProvider"); + } return PasspointMatch.HomeProvider; } - // ANQP Roaming Consortium OI matching. - long[] providerOIs = mConfig.getHomeSp().getRoamingConsortiumOis(); - if (ANQPMatcher.matchRoamingConsortium( - (RoamingConsortiumElement) anqpElements.get(ANQPElementType.ANQPRoamingConsortium), - providerOIs)) { - return PasspointMatch.RoamingProvider; + // Other Home Partners matching. + if (mConfig.getHomeSp().getOtherHomePartners() != null) { + for (String otherHomePartner : mConfig.getHomeSp().getOtherHomePartners()) { + if (ANQPMatcher.matchDomainName( + (DomainNameElement) anqpElements.get(ANQPElementType.ANQPDomName), + otherHomePartner, null, null)) { + if (mVerboseLoggingEnabled) { + Log.d(TAG, "Other Home Partner " + otherHomePartner + + " match: HomeProvider"); + } + return PasspointMatch.HomeProvider; + } + } } - long[] roamingConsortiums = roamingConsortium.getRoamingConsortiums(); - // Roaming Consortium OI information element matching. - if (roamingConsortiums != null && providerOIs != null) { - for (long sta_oi: roamingConsortiums) { - for (long ap_oi: providerOIs) { - if (sta_oi == ap_oi) { - return PasspointMatch.RoamingProvider; - } + // HomeOI matching + if (mConfig.getHomeSp().getMatchAllOis() != null) { + // Ensure that every HomeOI whose corresponding HomeOIRequired value is true shall match + // an OI in the Roaming Consortium advertised by the hotspot operator. + if (matchOis(mConfig.getHomeSp().getMatchAllOis(), (RoamingConsortiumElement) + anqpElements.get(ANQPElementType.ANQPRoamingConsortium), + roamingConsortiumFromAp, true)) { + if (mVerboseLoggingEnabled) { + Log.e(TAG, "All HomeOI RCOI match: HomeProvider"); + } + return PasspointMatch.HomeProvider; + } + } else if (mConfig.getHomeSp().getMatchAnyOis() != null) { + // Ensure that any HomeOI whose corresponding HomeOIRequired value is false shall match + // an OI in the Roaming Consortium advertised by the hotspot operator. + if (matchOis(mConfig.getHomeSp().getMatchAnyOis(), (RoamingConsortiumElement) + anqpElements.get(ANQPElementType.ANQPRoamingConsortium), + roamingConsortiumFromAp, false)) { + if (mVerboseLoggingEnabled) { + Log.e(TAG, "Any HomeOI RCOI match: HomeProvider"); } + return PasspointMatch.HomeProvider; } } + // Roaming Consortium OI matching. + if (matchOis(mConfig.getHomeSp().getRoamingConsortiumOis(), (RoamingConsortiumElement) + anqpElements.get(ANQPElementType.ANQPRoamingConsortium), + roamingConsortiumFromAp, false)) { + if (mVerboseLoggingEnabled) { + Log.e(TAG, "ANQP RCOI match: RoamingProvider"); + } + return PasspointMatch.RoamingProvider; + } + if (mVerboseLoggingEnabled) { + Log.e(TAG, "No domain name or RCOI match"); + } return PasspointMatch.None; } @@ -768,4 +870,12 @@ public class PasspointProvider { simCredential.setEapType(eapType); return simCredential; } + + /** + * Enable verbose logging + * @param verbose more than 0 enables verbose logging + */ + public void enableVerboseLogging(int verbose) { + mVerboseLoggingEnabled = (verbose > 0) ? true : false; + } } diff --git a/tests/wifitests/src/com/android/server/wifi/hotspot2/ANQPMatcherTest.java b/tests/wifitests/src/com/android/server/wifi/hotspot2/ANQPMatcherTest.java index 4d4ea4487..df9c332a3 100644 --- a/tests/wifitests/src/com/android/server/wifi/hotspot2/ANQPMatcherTest.java +++ b/tests/wifitests/src/com/android/server/wifi/hotspot2/ANQPMatcherTest.java @@ -33,7 +33,6 @@ import com.android.server.wifi.hotspot2.anqp.RoamingConsortiumElement; import com.android.server.wifi.hotspot2.anqp.ThreeGPPNetworkElement; import com.android.server.wifi.hotspot2.anqp.eap.AuthParam; import com.android.server.wifi.hotspot2.anqp.eap.EAPMethod; -import com.android.server.wifi.hotspot2.anqp.eap.InnerAuthEAP; import com.android.server.wifi.hotspot2.anqp.eap.NonEAPInnerAuth; import org.junit.Test; @@ -102,7 +101,7 @@ public class ANQPMatcherTest { */ @Test public void matchRoamingConsortiumWithNullElement() throws Exception { - assertFalse(ANQPMatcher.matchRoamingConsortium(null, new long[0])); + assertFalse(ANQPMatcher.matchRoamingConsortium(null, new long[0], false)); } /** @@ -116,23 +115,22 @@ public class ANQPMatcherTest { long oi = 0x1234L; RoamingConsortiumElement element = new RoamingConsortiumElement(Arrays.asList(new Long[] {oi})); - assertTrue(ANQPMatcher.matchRoamingConsortium(element, new long[] {oi})); + assertTrue(ANQPMatcher.matchRoamingConsortium(element, new long[] {oi}, false)); } /** - * Verify that an indeterminate match will be returned when matching a null NAI Realm + * Verify that no match will be returned when matching a null NAI Realm * ANQP element. * * @throws Exception */ @Test public void matchNAIRealmWithNullElement() throws Exception { - assertEquals(AuthMatch.INDETERMINATE, ANQPMatcher.matchNAIRealm(null, "test.com", - EAPConstants.EAP_TLS, new InnerAuthEAP(EAPConstants.EAP_TTLS))); + assertFalse(ANQPMatcher.matchNAIRealm(null, "test.com")); } /** - * Verify that an indeterminate match will be returned when matching a NAI Realm + * Verify that no match will be returned when matching a NAI Realm * ANQP element contained no NAI realm data. * * @throws Exception @@ -140,8 +138,7 @@ public class ANQPMatcherTest { @Test public void matchNAIRealmWithEmtpyRealmData() throws Exception { NAIRealmElement element = new NAIRealmElement(new ArrayList()); - assertEquals(AuthMatch.INDETERMINATE, ANQPMatcher.matchNAIRealm(element, "test.com", - EAPConstants.EAP_TLS, null)); + assertFalse(ANQPMatcher.matchNAIRealm(element, "test.com")); } /** @@ -157,38 +154,11 @@ public class ANQPMatcherTest { Arrays.asList(new String[] {realm}), new ArrayList()); NAIRealmElement element = new NAIRealmElement( Arrays.asList(new NAIRealmData[] {realmData})); - assertEquals(AuthMatch.REALM, ANQPMatcher.matchNAIRealm(element, realm, - EAPConstants.EAP_TLS, null)); + assertTrue(ANQPMatcher.matchNAIRealm(element, realm)); } /** - * Verify that method match will be returned when the specified EAP - * method only matches a eap method in the NAI Realm ANQP element if the element does not have - * auth params. - * - * @throws Exception - */ - @Test - public void matchNAIRealmWithMethodMatch() throws Exception { - // Test data. - String providerRealm = "test.com"; - String anqpRealm = "test2.com"; - NonEAPInnerAuth authParam = new NonEAPInnerAuth(NonEAPInnerAuth.AUTH_TYPE_MSCHAP); - int eapMethodID = EAPConstants.EAP_TLS; - - // Setup NAI Realm element that has EAP method and no auth params. - EAPMethod method = new EAPMethod(eapMethodID, new HashMap>()); - NAIRealmData realmData = new NAIRealmData( - Arrays.asList(new String[]{anqpRealm}), Arrays.asList(new EAPMethod[]{method})); - NAIRealmElement element = new NAIRealmElement( - Arrays.asList(new NAIRealmData[]{realmData})); - - assertEquals(AuthMatch.METHOD, - ANQPMatcher.matchNAIRealm(element, providerRealm, eapMethodID, authParam)); - } - - /** - * Verify that a realm and method match will be returned when the specified realm and EAP + * Verify that a realm match will be returned when the specified realm and EAP * method matches a realm in the NAI Realm ANQP element. * * @throws Exception @@ -206,12 +176,11 @@ public class ANQPMatcherTest { NAIRealmElement element = new NAIRealmElement( Arrays.asList(new NAIRealmData[] {realmData})); - assertEquals(AuthMatch.REALM | AuthMatch.METHOD, - ANQPMatcher.matchNAIRealm(element, realm, eapMethodID, null)); + assertTrue(ANQPMatcher.matchNAIRealm(element, realm)); } /** - * Verify that an exact match will be returned when the specified realm, EAP + * Verify that a realm match will be returned when the specified realm, EAP * method, and the authentication parameter matches a realm with the associated EAP method and * authentication parameter in the NAI Realm ANQP element. * @@ -235,12 +204,11 @@ public class ANQPMatcherTest { NAIRealmElement element = new NAIRealmElement( Arrays.asList(new NAIRealmData[] {realmData})); - assertEquals(AuthMatch.EXACT, - ANQPMatcher.matchNAIRealm(element, realm, eapMethodID, authParam)); + assertTrue(ANQPMatcher.matchNAIRealm(element, realm)); } /** - * Verify that a mismatch (AuthMatch.NONE) will be returned when the specified EAP method + * Verify that a realm match will be returned when the specified EAP method * doesn't match with the corresponding EAP method in the NAI Realm ANQP element. * * @throws Exception @@ -263,12 +231,11 @@ public class ANQPMatcherTest { NAIRealmElement element = new NAIRealmElement( Arrays.asList(new NAIRealmData[] {realmData})); - assertEquals(AuthMatch.NONE, - ANQPMatcher.matchNAIRealm(element, realm, EAPConstants.EAP_TLS, null)); + assertTrue(ANQPMatcher.matchNAIRealm(element, realm)); } /** - * Verify that a mismatch (AuthMatch.NONE) will be returned when the specified authentication + * Verify that a realm match will be returned when the specified authentication * parameter doesn't match with the corresponding authentication parameter in the NAI Realm * ANQP element. * @@ -292,10 +259,8 @@ public class ANQPMatcherTest { NAIRealmElement element = new NAIRealmElement( Arrays.asList(new NAIRealmData[] {realmData})); - // Mismatch in authentication type. - assertEquals(AuthMatch.NONE, - ANQPMatcher.matchNAIRealm(element, realm, EAPConstants.EAP_TTLS, - new NonEAPInnerAuth(NonEAPInnerAuth.AUTH_TYPE_PAP))); + // Mismatch in authentication type which we ignore. + assertTrue(ANQPMatcher.matchNAIRealm(element, realm)); } /** @@ -458,4 +423,64 @@ public class ANQPMatcherTest { assertEquals(-1, ANQPMatcher.getCarrierEapMethodFromMatchingNAIRealm(TEST_3GPP_FQDN, element)); } + + /** + * Verify that match is found when HomeOI contains some of the RCOIs advertised by an AP marked + * as not required. + * + * @throws Exception + */ + @Test + public void matchAnyHomeOi() throws Exception { + long[] providerOis = new long[] {0x1234L, 0x5678L, 0xabcdL}; + Long[] anqpOis = new Long[] {0x1234L, 0x5678L, 0xdeadL, 0xf0cdL}; + RoamingConsortiumElement element = + new RoamingConsortiumElement(Arrays.asList(anqpOis)); + assertTrue(ANQPMatcher.matchRoamingConsortium(element, providerOis, false)); + } + + /** + * Verify that no match is found when HomeOI does not contain any of the RCOIs advertised by an + * AP marked as not required. + * + * @throws Exception + */ + @Test + public void matchAnyHomeOiNegative() throws Exception { + long[] providerOis = new long[] {0x1234L, 0x5678L, 0xabcdL}; + Long[] anqpOis = new Long[] {0xabc2L, 0x1232L}; + RoamingConsortiumElement element = + new RoamingConsortiumElement(Arrays.asList(anqpOis)); + assertFalse(ANQPMatcher.matchRoamingConsortium(element, providerOis, false)); + } + + /** + * Verify that match is found when HomeOI contains all of the RCOIs advertised by an AP marked + * as required. + * + * @throws Exception + */ + @Test + public void matchAllHomeOi() throws Exception { + long[] providerOis = new long[] {0x1234L, 0x5678L, 0xabcdL}; + Long[] anqpOis = new Long[] {0x1234L, 0x5678L, 0xabcdL, 0xdeadL, 0xf0cdL}; + RoamingConsortiumElement element = + new RoamingConsortiumElement(Arrays.asList(anqpOis)); + assertTrue(ANQPMatcher.matchRoamingConsortium(element, providerOis, true)); + } + + /** + * Verify that match is not found when HomeOI does not contain all of the RCOIs advertised by an + * AP marked as required. + * + * @throws Exception + */ + @Test + public void matchAllHomeOiNegative() throws Exception { + long[] providerOis = new long[] {0x1234L, 0x5678L, 0xabcdL}; + Long[] anqpOis = new Long[] {0x1234L, 0x5678L, 0xdeadL, 0xf0cdL}; + RoamingConsortiumElement element = + new RoamingConsortiumElement(Arrays.asList(anqpOis)); + assertFalse(ANQPMatcher.matchRoamingConsortium(element, providerOis, true)); + } } diff --git a/tests/wifitests/src/com/android/server/wifi/hotspot2/PasspointProviderTest.java b/tests/wifitests/src/com/android/server/wifi/hotspot2/PasspointProviderTest.java index 31229c562..c35d67301 100644 --- a/tests/wifitests/src/com/android/server/wifi/hotspot2/PasspointProviderTest.java +++ b/tests/wifitests/src/com/android/server/wifi/hotspot2/PasspointProviderTest.java @@ -478,9 +478,10 @@ public class PasspointProviderTest { } /** - * Verify that there is no match when the provider's FQDN matches a domain name in the - * Domain Name ANQP element but the provider's credential doesn't match the authentication - * method provided in the NAI realm. + * Verify that Home provider is matched even when the provider's FQDN matches a domain name in + * the Domain Name ANQP element but the provider's credential doesn't match the authentication + * method provided in the NAI realm. This can happen when the infrastructure provider is not + * the identity provider, and authentication method matching is not required in the spec. * * @throws Exception */ @@ -509,7 +510,8 @@ public class PasspointProviderTest { anqpElementMap.put(ANQPElementType.ANQPNAIRealm, createNAIRealmElement(testRealm, EAPConstants.EAP_TLS, null)); - assertEquals(PasspointMatch.None, mProvider.match(anqpElementMap, mRoamingConsortium)); + assertEquals(PasspointMatch.HomeProvider, + mProvider.match(anqpElementMap, mRoamingConsortium)); } /** @@ -657,8 +659,8 @@ public class PasspointProviderTest { } /** - * Verify that there is no match when a roaming consortium OI matches an OI - * in the roaming consortium ANQP element and but NAI realm is not matched. + * Verify that there is Roaming provider match when a roaming consortium OI matches an OI + * in the roaming consortium ANQP element and regardless of NAI realm mismatch. * * @throws Exception */ @@ -689,7 +691,7 @@ public class PasspointProviderTest { anqpElementMap.put(ANQPElementType.ANQPNAIRealm, createNAIRealmElement(testRealm, EAPConstants.EAP_TLS, null)); - assertEquals(PasspointMatch.None, + assertEquals(PasspointMatch.RoamingProvider, mProvider.match(anqpElementMap, mRoamingConsortium)); } @@ -766,8 +768,14 @@ public class PasspointProviderTest { } /** - * Verify that there is no match when a roaming consortium OI matches an OI + * Verify that there is Roaming provider match when a roaming consortium OI matches an OI * in the roaming consortium information element, but NAI realm is not matched. + * This can happen in roaming federation where the infrastructure provider is not the + * identity provider. + * Page 133 in the Hotspot2.0 specification states: + * Per subclause 11.25.8 of [2], if the value of HomeOI matches an OI in the Roaming + * Consortium advertised by a hotspot operator, successful authentication with that hotspot + * is possible. * * @throws Exception */ @@ -799,7 +807,7 @@ public class PasspointProviderTest { anqpElementMap.put(ANQPElementType.ANQPNAIRealm, createNAIRealmElement(testRealm, EAPConstants.EAP_TLS, null)); - assertEquals(PasspointMatch.None, + assertEquals(PasspointMatch.RoamingProvider, mProvider.match(anqpElementMap, mRoamingConsortium)); } @@ -1354,4 +1362,167 @@ public class PasspointProviderTest { mProvider.setHasEverConnected(true); assertTrue(mProvider.getHasEverConnected()); } + + /** + * Verify that an expected WifiConfiguration will be returned for a Passpoint provider + * with a user credential. + * + * @throws Exception + */ + @Test + public void matchOtherPartnersDomainName() throws Exception { + // Setup test provider. + PasspointConfiguration config = new PasspointConfiguration(); + HomeSp homeSp = new HomeSp(); + homeSp.setFqdn("test1.com"); + homeSp.setOtherHomePartners(new String [] {"test3.com"}); + config.setHomeSp(homeSp); + Credential credential = new Credential(); + credential.setUserCredential(new Credential.UserCredential()); + config.setCredential(credential); + mProvider = createProvider(config); + verifyInstalledConfig(config, true); + + // Setup Domain Name ANQP element to test2.com and test3.com + Map anqpElementMap = new HashMap<>(); + anqpElementMap.put(ANQPElementType.ANQPDomName, + createDomainNameElement(new String[] {"test2.com", "test3.com"})); + + assertEquals(PasspointMatch.HomeProvider, + mProvider.match(anqpElementMap, mRoamingConsortium)); + } + + /** + * Verify that matching Any HomeOI results in a Home Provider match + * + * @throws Exception + */ + @Test + public void matchAnyHomeOi() throws Exception { + // Setup test provider. + PasspointConfiguration config = new PasspointConfiguration(); + HomeSp homeSp = new HomeSp(); + homeSp.setFqdn("test1.com"); + homeSp.setMatchAnyOis(new long[] {0x1234L, 0x2345L}); + homeSp.setRoamingConsortiumOis(null); + config.setHomeSp(homeSp); + Credential credential = new Credential(); + credential.setUserCredential(new Credential.UserCredential()); + config.setCredential(credential); + mProvider = createProvider(config); + verifyInstalledConfig(config, true); + Long[] anqpOis = new Long[] {0x1234L, 0xdeadL, 0xf0cdL}; + + // Setup Domain Name ANQP element to test2.com and test3.com + Map anqpElementMap = new HashMap<>(); + anqpElementMap.put(ANQPElementType.ANQPDomName, + createDomainNameElement(new String[] {"test2.com", "test3.com"})); + // Setup RCOIs advertised by the AP + anqpElementMap.put(ANQPElementType.ANQPRoamingConsortium, + createRoamingConsortiumElement(anqpOis)); + + assertEquals(PasspointMatch.HomeProvider, + mProvider.match(anqpElementMap, mRoamingConsortium)); + } + + /** + * Verify that non-matching Any HomeOI results in a None Provider match + * + * @throws Exception + */ + @Test + public void matchAnyHomeOiNegative() throws Exception { + // Setup test provider. + PasspointConfiguration config = new PasspointConfiguration(); + HomeSp homeSp = new HomeSp(); + homeSp.setFqdn("test1.com"); + homeSp.setMatchAnyOis(new long[] {0x1234L, 0x2345L}); + homeSp.setRoamingConsortiumOis(null); + config.setHomeSp(homeSp); + Credential credential = new Credential(); + credential.setUserCredential(new Credential.UserCredential()); + config.setCredential(credential); + mProvider = createProvider(config); + verifyInstalledConfig(config, true); + Long[] anqpOis = new Long[] {0x12a4L, 0xceadL, 0xf0cdL}; + + // Setup Domain Name ANQP element to test2.com and test3.com + Map anqpElementMap = new HashMap<>(); + anqpElementMap.put(ANQPElementType.ANQPDomName, + createDomainNameElement(new String[] {"test2.com", "test3.com"})); + // Setup RCOIs advertised by the AP + anqpElementMap.put(ANQPElementType.ANQPRoamingConsortium, + createRoamingConsortiumElement(anqpOis)); + + assertEquals(PasspointMatch.None, + mProvider.match(anqpElementMap, mRoamingConsortium)); + } + + /** + * Verify that matching All HomeOI results in a Home Provider match + * + * @throws Exception + */ + @Test + public void matchAllHomeOi() throws Exception { + // Setup test provider. + PasspointConfiguration config = new PasspointConfiguration(); + HomeSp homeSp = new HomeSp(); + homeSp.setFqdn("test1.com"); + homeSp.setMatchAllOis(new long[] {0x1234L, 0x2345L}); + homeSp.setRoamingConsortiumOis(null); + config.setHomeSp(homeSp); + Credential credential = new Credential(); + credential.setUserCredential(new Credential.UserCredential()); + config.setCredential(credential); + mProvider = createProvider(config); + verifyInstalledConfig(config, true); + Long[] anqpOis = new Long[] {0x1234L, 0x2345L, 0xabcdL, 0xdeadL, 0xf0cdL}; + + // Setup Domain Name ANQP element to test2.com and test3.com + Map anqpElementMap = new HashMap<>(); + anqpElementMap.put(ANQPElementType.ANQPDomName, + createDomainNameElement(new String[] {"test2.com", "test3.com"})); + // Setup RCOIs advertised by the AP + anqpElementMap.put(ANQPElementType.ANQPRoamingConsortium, + createRoamingConsortiumElement(anqpOis)); + + assertEquals(PasspointMatch.HomeProvider, + mProvider.match(anqpElementMap, mRoamingConsortium)); + } + + /** + * Verify that non-matching All HomeOI results in a None Provider match + * + * @throws Exception + */ + @Test + public void matchAllHomeOiNegative() throws Exception { + // Setup test provider. + PasspointConfiguration config = new PasspointConfiguration(); + HomeSp homeSp = new HomeSp(); + homeSp.setFqdn("test1.com"); + homeSp.setMatchAllOis(new long[] {0x1234L, 0x2345L}); + homeSp.setRoamingConsortiumOis(null); + config.setHomeSp(homeSp); + Credential credential = new Credential(); + credential.setUserCredential(new Credential.UserCredential()); + config.setCredential(credential); + mProvider = createProvider(config); + verifyInstalledConfig(config, true); + + // 0x1234 matches, but 0x2345 does not + Long[] anqpOis = new Long[] {0x1234L, 0x5678L, 0xdeadL, 0xf0cdL}; + + // Setup Domain Name ANQP element to test2.com and test3.com + Map anqpElementMap = new HashMap<>(); + anqpElementMap.put(ANQPElementType.ANQPDomName, + createDomainNameElement(new String[] {"test2.com", "test3.com"})); + // Setup RCOIs advertised by the AP + anqpElementMap.put(ANQPElementType.ANQPRoamingConsortium, + createRoamingConsortiumElement(anqpOis)); + + assertEquals(PasspointMatch.None, + mProvider.match(anqpElementMap, mRoamingConsortium)); + } } -- cgit v1.2.3 From 6853a19f2da130483fbe4082b457f1cf6a21e542 Mon Sep 17 00:00:00 2001 From: "Nate(Qiang) Jiang" Date: Mon, 2 Mar 2020 17:56:31 -0800 Subject: Enterprise suggestion's catificate share same lifecycle as suggestion Enterprise network suggestion's catificate will add to keystore immediately after add the suggestion, and will only remove after suggestion is removed. Bug: 150500247 Test: atest com.android.server.wifi Merged-In: Icbe49911d7ca93b03dfdf728ad88057c31aa5974 Change-Id: Icbe49911d7ca93b03dfdf728ad88057c31aa5974 (cherry picked from commit e6813a5870911b10561db2c259c072123c1c513a) --- .../com/android/server/wifi/WifiConfigManager.java | 18 +++---- .../java/com/android/server/wifi/WifiInjector.java | 4 +- .../server/wifi/WifiNetworkSuggestionsManager.java | 37 +++++++++++++- .../android/server/wifi/WifiConfigManagerTest.java | 6 ++- .../com/android/server/wifi/WifiKeyStoreTest.java | 56 ++++++++++++++++++++++ .../wifi/WifiNetworkSuggestionsManagerTest.java | 38 ++++++++++++++- 6 files changed, 144 insertions(+), 15 deletions(-) diff --git a/service/java/com/android/server/wifi/WifiConfigManager.java b/service/java/com/android/server/wifi/WifiConfigManager.java index fb84cf0b1..59785fd87 100644 --- a/service/java/com/android/server/wifi/WifiConfigManager.java +++ b/service/java/com/android/server/wifi/WifiConfigManager.java @@ -1211,12 +1211,11 @@ public class WifiConfigManager { return new NetworkUpdateResult(WifiConfiguration.INVALID_NETWORK_ID); } - // Update the keys for non-Passpoint enterprise networks. For Passpoint, the certificates - // and keys are installed at the time the provider is installed. - if (config.enterpriseConfig != null - && config.enterpriseConfig.getEapMethod() != WifiEnterpriseConfig.Eap.NONE - && !config.isPasspoint()) { - if (!(mWifiKeyStore.updateNetworkKeys(newInternalConfig, existingInternalConfig))) { + // Update the keys for saved enterprise networks. For Passpoint, the certificates + // and keys are installed at the time the provider is installed. For suggestion enterprise + // network the certificates and keys are installed at the time the suggestion is added + if (!config.isPasspoint() && !config.fromWifiNetworkSuggestion && config.isEnterprise()) { + if (!mWifiKeyStore.updateNetworkKeys(newInternalConfig, existingInternalConfig)) { return new NetworkUpdateResult(WifiConfiguration.INVALID_NETWORK_ID); } } @@ -1353,9 +1352,10 @@ public class WifiConfigManager { if (mVerboseLoggingEnabled) { Log.v(TAG, "Removing network " + config.getPrintableSsid()); } - // Remove any associated enterprise keys for non-Passpoint networks. - if (!config.isPasspoint() && config.enterpriseConfig != null - && config.enterpriseConfig.getEapMethod() != WifiEnterpriseConfig.Eap.NONE) { + // Remove any associated enterprise keys for saved enterprise networks. Passpoint network + // will remove the enterprise keys when provider is uninstalled. Suggestion enterprise + // networks will remove the enterprise keys when suggestion is removed. + if (!config.isPasspoint() && !config.fromWifiNetworkSuggestion && config.isEnterprise()) { mWifiKeyStore.removeKeys(config.enterpriseConfig); } diff --git a/service/java/com/android/server/wifi/WifiInjector.java b/service/java/com/android/server/wifi/WifiInjector.java index 474f27cdd..9ffca1326 100644 --- a/service/java/com/android/server/wifi/WifiInjector.java +++ b/service/java/com/android/server/wifi/WifiInjector.java @@ -269,8 +269,8 @@ public class WifiInjector { mWifiConfigManager, mClock, mConnectivityLocalLog, mWifiConnectivityHelper, subscriptionManager); mWifiNetworkSuggestionsManager = new WifiNetworkSuggestionsManager(mContext, - new Handler(mWifiCoreHandlerThread.getLooper()), this, - mWifiPermissionsUtil, mWifiConfigManager, mWifiConfigStore, mWifiMetrics); + new Handler(mWifiCoreHandlerThread.getLooper()), this, mWifiPermissionsUtil, + mWifiConfigManager, mWifiConfigStore, mWifiMetrics, mWifiKeyStore); mNetworkSuggestionEvaluator = new NetworkSuggestionEvaluator(mWifiNetworkSuggestionsManager, mWifiConfigManager, mConnectivityLocalLog); mScoredNetworkEvaluator = new ScoredNetworkEvaluator(context, clientModeImplLooper, diff --git a/service/java/com/android/server/wifi/WifiNetworkSuggestionsManager.java b/service/java/com/android/server/wifi/WifiNetworkSuggestionsManager.java index 6a5db5d23..8feb3711e 100644 --- a/service/java/com/android/server/wifi/WifiNetworkSuggestionsManager.java +++ b/service/java/com/android/server/wifi/WifiNetworkSuggestionsManager.java @@ -102,6 +102,7 @@ public class WifiNetworkSuggestionsManager { private final WifiMetrics mWifiMetrics; private final WifiInjector mWifiInjector; private final FrameworkFacade mFrameworkFacade; + private final WifiKeyStore mWifiKeyStore; /** * Per app meta data to store network suggestions, status, etc for each app providing network @@ -160,6 +161,10 @@ public class WifiNetworkSuggestionsManager { @NonNull PerAppInfo perAppInfo) { this.wns = wns; this.perAppInfo = perAppInfo; + this.wns.wifiConfiguration.fromWifiNetworkSuggestion = true; + this.wns.wifiConfiguration.ephemeral = true; + this.wns.wifiConfiguration.creatorName = perAppInfo.packageName; + this.wns.wifiConfiguration.creatorUid = wns.suggestorUid; } @Override @@ -378,7 +383,8 @@ public class WifiNetworkSuggestionsManager { WifiPermissionsUtil wifiPermissionsUtil, WifiConfigManager wifiConfigManager, WifiConfigStore wifiConfigStore, - WifiMetrics wifiMetrics) { + WifiMetrics wifiMetrics, + WifiKeyStore keyStore) { mContext = context; mResources = context.getResources(); mHandler = handler; @@ -391,6 +397,7 @@ public class WifiNetworkSuggestionsManager { mWifiPermissionsUtil = wifiPermissionsUtil; mWifiConfigManager = wifiConfigManager; mWifiMetrics = wifiMetrics; + mWifiKeyStore = keyStore; // register the data store for serializing/deserializing data. wifiConfigStore.registerStoreData( @@ -587,6 +594,19 @@ public class WifiNetworkSuggestionsManager { // Start tracking app-op changes from the app if they have active suggestions. startTrackingAppOpsChange(packageName, uid); } + Iterator iterator = extNetworkSuggestions.iterator(); + // Install enterprise network suggestion catificate. + while (iterator.hasNext()) { + WifiConfiguration config = iterator.next().wns.wifiConfiguration; + if (!config.isEnterprise()) { + continue; + } + if (!mWifiKeyStore.updateNetworkKeys(config, null)) { + Log.e(TAG, "Enterprise network install failure for SSID: " + + config.SSID); + iterator.remove(); + } + } perAppInfo.extNetworkSuggestions.addAll(extNetworkSuggestions); // Update the max size for this app. perAppInfo.maxSize = Math.max(perAppInfo.extNetworkSuggestions.size(), perAppInfo.maxSize); @@ -611,7 +631,12 @@ public class WifiNetworkSuggestionsManager { @NonNull Collection extNetworkSuggestions, @NonNull String packageName, @NonNull PerAppInfo perAppInfo) { + // Get internal suggestions + Set removingSuggestions = + new HashSet<>(perAppInfo.extNetworkSuggestions); if (!extNetworkSuggestions.isEmpty()) { + // Keep the internal suggestions need to remove. + removingSuggestions.retainAll(extNetworkSuggestions); perAppInfo.extNetworkSuggestions.removeAll(extNetworkSuggestions); } else { // empty list is used to clear everything for the app. Store a copy for use below. @@ -626,8 +651,16 @@ public class WifiNetworkSuggestionsManager { // Stop tracking app-op changes from the app if they don't have active suggestions. stopTrackingAppOpsChange(packageName); } + // Clean the enterprise certifiacte. + for (ExtendedWifiNetworkSuggestion ewns : removingSuggestions) { + WifiConfiguration config = ewns.wns.wifiConfiguration; + if (!config.isEnterprise()) { + continue; + } + mWifiKeyStore.removeKeys(config.enterpriseConfig); + } // Clear the scan cache. - removeFromScanResultMatchInfoMap(extNetworkSuggestions); + removeFromScanResultMatchInfoMap(removingSuggestions); } /** diff --git a/tests/wifitests/src/com/android/server/wifi/WifiConfigManagerTest.java b/tests/wifitests/src/com/android/server/wifi/WifiConfigManagerTest.java index ce1c6b2ee..fdcd948f9 100644 --- a/tests/wifitests/src/com/android/server/wifi/WifiConfigManagerTest.java +++ b/tests/wifitests/src/com/android/server/wifi/WifiConfigManagerTest.java @@ -670,13 +670,14 @@ public class WifiConfigManagerTest { */ @Test public void testAddSingleSuggestionNetwork() throws Exception { - WifiConfiguration suggestionNetwork = WifiConfigurationTestUtil.createOpenNetwork(); + WifiConfiguration suggestionNetwork = WifiConfigurationTestUtil.createEapNetwork(); suggestionNetwork.ephemeral = true; suggestionNetwork.fromWifiNetworkSuggestion = true; List networks = new ArrayList<>(); networks.add(suggestionNetwork); verifyAddSuggestionOrRequestNetworkToWifiConfigManager(suggestionNetwork); + verify(mWifiKeyStore, never()).updateNetworkKeys(any(), any()); List retrievedNetworks = mWifiConfigManager.getConfiguredNetworksWithPasswords(); @@ -686,6 +687,9 @@ public class WifiConfigManagerTest { // Ensure that this is not returned in the saved network list. assertTrue(mWifiConfigManager.getSavedNetworks(Process.WIFI_UID).isEmpty()); verify(mWcmListener, never()).onSavedNetworkAdded(suggestionNetwork.networkId); + assertTrue(mWifiConfigManager + .removeNetwork(suggestionNetwork.networkId, TEST_CREATOR_UID)); + verify(mWifiKeyStore, never()).removeKeys(any()); } /** diff --git a/tests/wifitests/src/com/android/server/wifi/WifiKeyStoreTest.java b/tests/wifitests/src/com/android/server/wifi/WifiKeyStoreTest.java index 7079a2d53..7649d1ba4 100644 --- a/tests/wifitests/src/com/android/server/wifi/WifiKeyStoreTest.java +++ b/tests/wifitests/src/com/android/server/wifi/WifiKeyStoreTest.java @@ -16,11 +16,19 @@ package com.android.server.wifi; +import static org.junit.Assert.assertNotEquals; +import static org.junit.Assert.assertTrue; +import static org.mockito.AdditionalMatchers.aryEq; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyInt; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.validateMockitoUsage; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.when; +import android.net.wifi.WifiConfiguration; import android.net.wifi.WifiEnterpriseConfig; import android.os.Process; import android.security.Credentials; @@ -34,6 +42,8 @@ import org.junit.Test; import org.mockito.Mock; import org.mockito.MockitoAnnotations; +import java.security.cert.X509Certificate; + /** * Unit tests for {@link com.android.server.wifi.WifiConfigManager}. */ @@ -43,8 +53,10 @@ public class WifiKeyStoreTest { @Mock private KeyStore mKeyStore; private WifiKeyStore mWifiKeyStore; + private static final String TEST_KEY_ID = "blah"; private static final String USER_CERT_ALIAS = "aabbccddee"; private static final String [] USER_CA_CERT_ALIAS = {"aacccddd", "bbbqqqqmmm"}; + private static final String TEST_PACKAGE_NAME = "TestApp"; /** * Setup the mocks and an instance of WifiConfigManager before each test. @@ -57,6 +69,16 @@ public class WifiKeyStoreTest { when(mWifiEnterpriseConfig.getClientCertificateAlias()).thenReturn(USER_CERT_ALIAS); when(mWifiEnterpriseConfig.getCaCertificateAliases()) .thenReturn(USER_CA_CERT_ALIAS); + when(mKeyStore.put(anyString(), any(), anyInt(), anyInt())).thenReturn(true); + when(mKeyStore.importKey(anyString(), any(), anyInt(), anyInt())).thenReturn(true); + when(mWifiEnterpriseConfig.getClientPrivateKey()).thenReturn(FakeKeys.RSA_KEY1); + when(mWifiEnterpriseConfig.getClientCertificate()).thenReturn(FakeKeys.CLIENT_CERT); + when(mWifiEnterpriseConfig.getCaCertificate()).thenReturn(FakeKeys.CA_CERT0); + when(mWifiEnterpriseConfig.getClientCertificateChain()) + .thenReturn(new X509Certificate[] {FakeKeys.CLIENT_CERT}); + when(mWifiEnterpriseConfig.getCaCertificates()) + .thenReturn(new X509Certificate[] {FakeKeys.CA_CERT0}); + when(mWifiEnterpriseConfig.getKeyId(any())).thenReturn(TEST_KEY_ID); } /** @@ -129,4 +151,38 @@ public class WifiKeyStoreTest { mWifiKeyStore.removeKeys(mWifiEnterpriseConfig); verifyNoMoreInteractions(mKeyStore); } + + /** + * Add two same network credential one is from user saved, the other is from suggestion. + * Both oh them should be installed successfully and has different alias, and will not override + * each other. + */ + @Test + public void testAddFromBothSavedAndSuggestionNetwork() throws Exception { + WifiConfiguration savedNetwork = WifiConfigurationTestUtil.createEapNetwork(); + WifiConfiguration suggestionNetwork = new WifiConfiguration(savedNetwork); + savedNetwork.enterpriseConfig = mWifiEnterpriseConfig; + suggestionNetwork.enterpriseConfig = mWifiEnterpriseConfig; + suggestionNetwork.fromWifiNetworkSuggestion = true; + suggestionNetwork.creatorName = TEST_PACKAGE_NAME; + + assertTrue(mWifiKeyStore.updateNetworkKeys(savedNetwork, null)); + assertTrue(mWifiKeyStore.updateNetworkKeys(suggestionNetwork, null)); + + String savedNetworkAlias = savedNetwork.getKeyIdForCredentials(null); + String savedNetworkCaAlias = savedNetworkAlias; + + String suggestionNetworkAlias = suggestionNetwork.getKeyIdForCredentials(null); + String suggestionNetworkCaAlias = suggestionNetworkAlias; + + assertNotEquals(savedNetworkAlias, suggestionNetworkAlias); + + verify(mWifiEnterpriseConfig).setClientCertificateAlias(eq(savedNetworkAlias)); + verify(mWifiEnterpriseConfig).setCaCertificateAliases( + aryEq(new String[] {savedNetworkCaAlias})); + + verify(mWifiEnterpriseConfig).setClientCertificateAlias(eq(suggestionNetworkAlias)); + verify(mWifiEnterpriseConfig).setCaCertificateAliases( + aryEq(new String[] {suggestionNetworkCaAlias})); + } } diff --git a/tests/wifitests/src/com/android/server/wifi/WifiNetworkSuggestionsManagerTest.java b/tests/wifitests/src/com/android/server/wifi/WifiNetworkSuggestionsManagerTest.java index ade54bd14..ed3e6d07f 100644 --- a/tests/wifitests/src/com/android/server/wifi/WifiNetworkSuggestionsManagerTest.java +++ b/tests/wifitests/src/com/android/server/wifi/WifiNetworkSuggestionsManagerTest.java @@ -67,6 +67,7 @@ import org.mockito.Mock; import org.mockito.MockitoAnnotations; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.HashMap; import java.util.HashSet; @@ -101,6 +102,7 @@ public class WifiNetworkSuggestionsManagerTest { private @Mock NetworkSuggestionStoreData mNetworkSuggestionStoreData; private @Mock ClientModeImpl mClientModeImpl; private @Mock WifiMetrics mWifiMetrics; + private @Mock WifiKeyStore mWifiKeyStore; private TestLooper mLooper; private ArgumentCaptor mAppOpChangedListenerCaptor = ArgumentCaptor.forClass(AppOpsManager.OnOpChangedListener.class); @@ -158,7 +160,7 @@ public class WifiNetworkSuggestionsManagerTest { mWifiNetworkSuggestionsManager = new WifiNetworkSuggestionsManager(mContext, new Handler(mLooper.getLooper()), mWifiInjector, mWifiPermissionsUtil, mWifiConfigManager, mWifiConfigStore, - mWifiMetrics); + mWifiMetrics, mWifiKeyStore); verify(mContext).getResources(); verify(mContext).getSystemService(Context.APP_OPS_SERVICE); verify(mContext).getSystemService(Context.NOTIFICATION_SERVICE); @@ -301,6 +303,40 @@ public class WifiNetworkSuggestionsManagerTest { assertTrue(mWifiNetworkSuggestionsManager.getAllNetworkSuggestions().isEmpty()); } + @Test + public void testAddRemoveEnterpriseNetworkSuggestion() { + WifiNetworkSuggestion networkSuggestion1 = new WifiNetworkSuggestion( + WifiConfigurationTestUtil.createEapNetwork(), false, false, TEST_UID_1, + TEST_PACKAGE_1); + WifiNetworkSuggestion networkSuggestion2 = new WifiNetworkSuggestion( + WifiConfigurationTestUtil.createEapNetwork(), false, false, TEST_UID_2, + TEST_PACKAGE_2); + + List networkSuggestionList = + new ArrayList() {{ + add(networkSuggestion1); + add(networkSuggestion2); + }}; + when(mWifiKeyStore.updateNetworkKeys(eq(networkSuggestion1.wifiConfiguration), any())) + .thenReturn(true); + when(mWifiKeyStore.updateNetworkKeys(eq(networkSuggestion2.wifiConfiguration), any())) + .thenReturn(false); + assertEquals(WifiManager.STATUS_NETWORK_SUGGESTIONS_SUCCESS, + mWifiNetworkSuggestionsManager.add(networkSuggestionList, TEST_UID_1, + TEST_PACKAGE_1)); + + Set allNetworkSuggestions = + mWifiNetworkSuggestionsManager.getAllNetworkSuggestions(); + assertEquals(1, allNetworkSuggestions.size()); + WifiNetworkSuggestion removingSuggestion = new WifiNetworkSuggestion( + WifiConfigurationTestUtil.createEapNetwork(), false, false, TEST_UID_1, + TEST_PACKAGE_1); + removingSuggestion.wifiConfiguration.SSID = networkSuggestion1.wifiConfiguration.SSID; + assertEquals(WifiManager.STATUS_NETWORK_SUGGESTIONS_SUCCESS, + mWifiNetworkSuggestionsManager.remove(Arrays.asList(removingSuggestion), + TEST_PACKAGE_1)); + verify(mWifiKeyStore).removeKeys(any()); + } /** * Verify successful replace (add,remove, add) of network suggestions. */ -- cgit v1.2.3