From f210755c2ac34dc466bcab188b9716b2482bbfe1 Mon Sep 17 00:00:00 2001 From: Roshan Pius Date: Tue, 3 Apr 2018 11:01:29 -0700 Subject: WifiConfigStorLegacy: Add null check during restore One of our OEM's is running into a crash in the config store migration logic added in O. See the below bug for more details. This issue is not seen on Pixel devices, because the |configKey| can never be null there. Also, added a wrapper for IpConfigStore to help in mocking static method in IpConfigstore. Note: This code is pretty much unused on Pixel devices & other OEM's who have upgraded from N to O. Only will help OEM's who upgrade from N to P directly. Bug: 73877225 Test: Unit tests Change-Id: I1b24c05bbb2bc488d77f3f93ec550517c1d1e5c3 --- .../android/server/wifi/WifiConfigStoreLegacy.java | 23 +++++++++--- .../java/com/android/server/wifi/WifiInjector.java | 2 +- .../server/wifi/WifiConfigStoreLegacyTest.java | 42 ++++++++++++++++++++-- 3 files changed, 58 insertions(+), 9 deletions(-) diff --git a/service/java/com/android/server/wifi/WifiConfigStoreLegacy.java b/service/java/com/android/server/wifi/WifiConfigStoreLegacy.java index 184ee2f67..ef6d82f31 100644 --- a/service/java/com/android/server/wifi/WifiConfigStoreLegacy.java +++ b/service/java/com/android/server/wifi/WifiConfigStoreLegacy.java @@ -81,16 +81,28 @@ public class WifiConfigStoreLegacy { */ private final WifiNetworkHistory mWifiNetworkHistory; private final WifiNative mWifiNative; - private final IpConfigStore mIpconfigStore; + private final IpConfigStoreWrapper mIpconfigStoreWrapper; private final LegacyPasspointConfigParser mPasspointConfigParser; + /** + * Used to help mocking the static methods of IpconfigStore. + */ + public static class IpConfigStoreWrapper { + /** + * Read IP configurations from Ip config store. + */ + public SparseArray readIpAndProxyConfigurations(String filePath) { + return IpConfigStore.readIpAndProxyConfigurations(filePath); + } + } + WifiConfigStoreLegacy(WifiNetworkHistory wifiNetworkHistory, - WifiNative wifiNative, IpConfigStore ipConfigStore, + WifiNative wifiNative, IpConfigStoreWrapper ipConfigStore, LegacyPasspointConfigParser passpointConfigParser) { mWifiNetworkHistory = wifiNetworkHistory; mWifiNative = wifiNative; - mIpconfigStore = ipConfigStore; + mIpconfigStoreWrapper = ipConfigStore; mPasspointConfigParser = passpointConfigParser; } @@ -105,7 +117,7 @@ public class WifiConfigStoreLegacy { private static WifiConfiguration lookupWifiConfigurationUsingConfigKeyHash( Map configurationMap, int hashCode) { for (Map.Entry entry : configurationMap.entrySet()) { - if (entry.getKey().hashCode() == hashCode) { + if (entry.getKey() != null && entry.getKey().hashCode() == hashCode) { return entry.getValue(); } } @@ -122,7 +134,8 @@ public class WifiConfigStoreLegacy { // This is a map of the hash code of the network's configKey to the corresponding // IpConfiguration. SparseArray ipConfigurations = - mIpconfigStore.readIpAndProxyConfigurations(IP_CONFIG_FILE.getAbsolutePath()); + mIpconfigStoreWrapper.readIpAndProxyConfigurations( + IP_CONFIG_FILE.getAbsolutePath()); if (ipConfigurations == null || ipConfigurations.size() == 0) { Log.w(TAG, "No ip configurations found in ipconfig store"); return; diff --git a/service/java/com/android/server/wifi/WifiInjector.java b/service/java/com/android/server/wifi/WifiInjector.java index 32bf1aed3..a1ecca2c7 100644 --- a/service/java/com/android/server/wifi/WifiInjector.java +++ b/service/java/com/android/server/wifi/WifiInjector.java @@ -214,7 +214,7 @@ public class WifiInjector { mWifiNetworkHistory = new WifiNetworkHistory(mContext, writer); mIpConfigStore = new IpConfigStore(writer); mWifiConfigStoreLegacy = new WifiConfigStoreLegacy( - mWifiNetworkHistory, mWifiNative, mIpConfigStore, + mWifiNetworkHistory, mWifiNative, new WifiConfigStoreLegacy.IpConfigStoreWrapper(), new LegacyPasspointConfigParser()); // Config Manager mWifiConfigManager = new WifiConfigManager(mContext, mClock, diff --git a/tests/wifitests/src/com/android/server/wifi/WifiConfigStoreLegacyTest.java b/tests/wifitests/src/com/android/server/wifi/WifiConfigStoreLegacyTest.java index 0404a244b..ae4a396f6 100644 --- a/tests/wifitests/src/com/android/server/wifi/WifiConfigStoreLegacyTest.java +++ b/tests/wifitests/src/com/android/server/wifi/WifiConfigStoreLegacyTest.java @@ -26,7 +26,7 @@ import android.support.test.filters.SmallTest; import android.text.TextUtils; import android.util.SparseArray; -import com.android.server.net.IpConfigStore; +import com.android.server.wifi.WifiConfigStoreLegacy.IpConfigStoreWrapper; import com.android.server.wifi.hotspot2.LegacyPasspointConfig; import com.android.server.wifi.hotspot2.LegacyPasspointConfigParser; @@ -51,7 +51,7 @@ public class WifiConfigStoreLegacyTest { // Test mocks @Mock private WifiNative mWifiNative; @Mock private WifiNetworkHistory mWifiNetworkHistory; - @Mock private IpConfigStore mIpconfigStore; + @Mock private IpConfigStoreWrapper mIpconfigStoreWrapper; @Mock private LegacyPasspointConfigParser mPasspointConfigParser; /** @@ -68,7 +68,7 @@ public class WifiConfigStoreLegacyTest { MockitoAnnotations.initMocks(this); mWifiConfigStore = new WifiConfigStoreLegacy(mWifiNetworkHistory, mWifiNative, - mIpconfigStore, mPasspointConfigParser); + mIpconfigStoreWrapper, mPasspointConfigParser); } /** @@ -140,6 +140,8 @@ public class WifiConfigStoreLegacyTest { any(), any(Map.class), any(SparseArray.class)); when(mPasspointConfigParser.parseConfig(anyString())).thenReturn(passpointConfigs); + when(mIpconfigStoreWrapper.readIpAndProxyConfigurations(anyString())) + .thenReturn(createIpConfigStoreLoadData(networks)); WifiConfigStoreLegacy.WifiConfigStoreDataLegacy storeData = mWifiConfigStore.read(); // Update the expected configuration for Passpoint network. @@ -154,6 +156,40 @@ public class WifiConfigStoreLegacyTest { networks, storeData.getConfigurations()); } + /** + * Verify loading of network configurations from legacy stores with a null configKey. + * This happens if the 'ID_STR' field in wpa_supplicant.conf is not populated for some reason. + */ + @Test + public void testLoadFromStoresWithNullConfigKey() throws Exception { + WifiConfiguration pskNetwork = WifiConfigurationTestUtil.createPskNetwork(); + WifiConfiguration wepNetwork = WifiConfigurationTestUtil.createWepNetwork(); + WifiConfiguration eapNetwork = WifiConfigurationTestUtil.createEapNetwork(); + eapNetwork.enterpriseConfig.setPassword("EapPassword"); + + final List networks = new ArrayList<>(); + networks.add(pskNetwork); + networks.add(wepNetwork); + networks.add(eapNetwork); + + // Return the config data with null configKey. + doAnswer(new AnswerWithArguments() { + public boolean answer(String ifaceName, Map configs, + SparseArray> networkExtras) { + for (Map.Entry entry: + createWpaSupplicantLoadData(networks).entrySet()) { + configs.put(null, entry.getValue()); + } + return true; + } + }).when(mWifiNative).migrateNetworksFromSupplicant( + any(), any(Map.class), any(SparseArray.class)); + + when(mIpconfigStoreWrapper.readIpAndProxyConfigurations(anyString())) + .thenReturn(createIpConfigStoreLoadData(networks)); + assertNotNull(mWifiConfigStore.read()); + } + private SparseArray createIpConfigStoreLoadData( List configurations) { SparseArray newIpConfigurations = new SparseArray<>(); -- cgit v1.2.3