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