Revert "Revert "[DO NOT MERGE] wifi: remove certificates for network factory reset"" Bug: 231985227 This reverts commit e7a384b2f89a4502ae288ffa4c0b5642c9acf935. Reason for revert: re-releasing security patch with fix Merged-In: Ifc12240165e7108210b8a5f630eb737a185acad2 Change-Id: Ifc12240165e7108210b8a5f630eb737a185acad2 (cherry picked from commit bca528dcd7634dd13687186a3d6084919efc0e8e) Merged-In: Ifc12240165e7108210b8a5f630eb737a185acad2
diff --git a/service/java/com/android/server/wifi/WifiConfigManager.java b/service/java/com/android/server/wifi/WifiConfigManager.java index 8720b23..d3e20be 100644 --- a/service/java/com/android/server/wifi/WifiConfigManager.java +++ b/service/java/com/android/server/wifi/WifiConfigManager.java
@@ -1738,7 +1738,7 @@ // will remove the enterprise keys when provider is uninstalled. Suggestion enterprise // networks will remove the enterprise keys when suggestion is removed. if (!config.fromWifiNetworkSuggestion && !config.isPasspoint() && config.isEnterprise()) { - mWifiKeyStore.removeKeys(config.enterpriseConfig); + mWifiKeyStore.removeKeys(config.enterpriseConfig, false); } // Do not remove the user choice when passpoint or suggestion networks are removed from
diff --git a/service/java/com/android/server/wifi/WifiInjector.java b/service/java/com/android/server/wifi/WifiInjector.java index 9bcdbf2..d62d890 100644 --- a/service/java/com/android/server/wifi/WifiInjector.java +++ b/service/java/com/android/server/wifi/WifiInjector.java
@@ -1144,4 +1144,9 @@ public BufferedReader createBufferedReader(String filename) throws FileNotFoundException { return new BufferedReader(new FileReader(filename)); } + + @NonNull + public WifiKeyStore getWifiKeyStore() { + return mWifiKeyStore; + } }
diff --git a/service/java/com/android/server/wifi/WifiKeyStore.java b/service/java/com/android/server/wifi/WifiKeyStore.java index 503001e..a696140 100644 --- a/service/java/com/android/server/wifi/WifiKeyStore.java +++ b/service/java/com/android/server/wifi/WifiKeyStore.java
@@ -219,11 +219,12 @@ * Remove enterprise keys from the network config. * * @param config Config corresponding to the network. + * @param forceRemove remove keys regardless of the key installer. */ - public void removeKeys(WifiEnterpriseConfig config) { + public void removeKeys(WifiEnterpriseConfig config, boolean forceRemove) { Preconditions.checkNotNull(mKeyStore); // Do not remove keys that were manually installed by the user - if (config.isAppInstalledDeviceKeyAndCert()) { + if (forceRemove || config.isAppInstalledDeviceKeyAndCert()) { String client = config.getClientCertificateAlias(); // a valid client certificate is configured if (!TextUtils.isEmpty(client)) { @@ -237,7 +238,7 @@ } // Do not remove CA certs that were manually installed by the user - if (config.isAppInstalledCaCert()) { + if (forceRemove || config.isAppInstalledCaCert()) { String[] aliases = config.getCaCertificateAliases(); if (aliases == null || aliases.length == 0) { return;
diff --git a/service/java/com/android/server/wifi/WifiNetworkSuggestionsManager.java b/service/java/com/android/server/wifi/WifiNetworkSuggestionsManager.java index 6227b9d..7a40f68 100644 --- a/service/java/com/android/server/wifi/WifiNetworkSuggestionsManager.java +++ b/service/java/com/android/server/wifi/WifiNetworkSuggestionsManager.java
@@ -1352,7 +1352,7 @@ removeFromPassPointInfoMap(ewns); } else { if (ewns.wns.wifiConfiguration.isEnterprise()) { - mWifiKeyStore.removeKeys(ewns.wns.wifiConfiguration.enterpriseConfig); + mWifiKeyStore.removeKeys(ewns.wns.wifiConfiguration.enterpriseConfig, false); } removeFromScanResultMatchInfoMapAndRemoveRelatedScoreCard(ewns, true); mWifiConfigManager.removeConnectChoiceFromAllNetworks(ewns
diff --git a/service/java/com/android/server/wifi/WifiServiceImpl.java b/service/java/com/android/server/wifi/WifiServiceImpl.java index fa2875c..40acc85 100644 --- a/service/java/com/android/server/wifi/WifiServiceImpl.java +++ b/service/java/com/android/server/wifi/WifiServiceImpl.java
@@ -5005,12 +5005,18 @@ return; } // Delete all Wifi SSIDs - List<WifiConfiguration> networks = mWifiThreadRunner.call( - () -> mWifiConfigManager.getSavedNetworks(Process.WIFI_UID), - Collections.emptyList()); - for (WifiConfiguration network : networks) { - removeNetwork(network.networkId, packageName); - } + mWifiThreadRunner.run(() -> { + List<WifiConfiguration> networks = mWifiConfigManager + .getSavedNetworks(Process.WIFI_UID); + EventLog.writeEvent(0x534e4554, "231985227", -1, + "Remove certs for factory reset"); + for (WifiConfiguration network : networks) { + if (network.isEnterprise()) { + mWifiInjector.getWifiKeyStore().removeKeys(network.enterpriseConfig, true); + } + mWifiConfigManager.removeNetwork(network.networkId, callingUid, packageName); + } + }); // Delete all Passpoint configurations List<PasspointConfiguration> configs = mWifiThreadRunner.call( () -> mPasspointManager.getProviderConfigs(Process.WIFI_UID /* ignored */, true),
diff --git a/service/tests/wifitests/src/com/android/server/wifi/WifiConfigManagerTest.java b/service/tests/wifitests/src/com/android/server/wifi/WifiConfigManagerTest.java index 30ed8cc..efb62b1 100644 --- a/service/tests/wifitests/src/com/android/server/wifi/WifiConfigManagerTest.java +++ b/service/tests/wifitests/src/com/android/server/wifi/WifiConfigManagerTest.java
@@ -1141,7 +1141,7 @@ assertEquals(suggestionNetwork.networkId, wifiConfigCaptor.getValue().networkId); assertTrue(mWifiConfigManager .removeNetwork(suggestionNetwork.networkId, TEST_CREATOR_UID, TEST_CREATOR_NAME)); - verify(mWifiKeyStore, never()).removeKeys(any()); + verify(mWifiKeyStore, never()).removeKeys(any(), eq(false)); } /** @@ -1401,7 +1401,7 @@ passpointNetwork.networkId, Process.WIFI_UID, null)); // Verify keys are not being removed. - verify(mWifiKeyStore, never()).removeKeys(any(WifiEnterpriseConfig.class)); + verify(mWifiKeyStore, never()).removeKeys(any(WifiEnterpriseConfig.class), eq(false)); verifyNetworkRemoveBroadcast(); // Ensure that the write was not invoked for Passpoint network remove. mContextConfigStoreMockOrder.verify(mWifiConfigStore, never()).write(anyBoolean()); @@ -5923,7 +5923,7 @@ configuration.networkId, TEST_CREATOR_UID, TEST_CREATOR_NAME)); // Verify keys are not being removed. - verify(mWifiKeyStore, never()).removeKeys(any(WifiEnterpriseConfig.class)); + verify(mWifiKeyStore, never()).removeKeys(any(WifiEnterpriseConfig.class), eq(false)); verifyNetworkRemoveBroadcast(); // Ensure that the write was not invoked for Passpoint network remove. mContextConfigStoreMockOrder.verify(mWifiConfigStore, never()).write(anyBoolean());
diff --git a/service/tests/wifitests/src/com/android/server/wifi/WifiKeyStoreTest.java b/service/tests/wifitests/src/com/android/server/wifi/WifiKeyStoreTest.java index 75edcaa..9de443d 100644 --- a/service/tests/wifitests/src/com/android/server/wifi/WifiKeyStoreTest.java +++ b/service/tests/wifitests/src/com/android/server/wifi/WifiKeyStoreTest.java
@@ -109,7 +109,7 @@ public void testRemoveKeysForAppInstalledCerts() throws Exception { when(mWifiEnterpriseConfig.isAppInstalledDeviceKeyAndCert()).thenReturn(true); when(mWifiEnterpriseConfig.isAppInstalledCaCert()).thenReturn(true); - mWifiKeyStore.removeKeys(mWifiEnterpriseConfig); + mWifiKeyStore.removeKeys(mWifiEnterpriseConfig, false); // Method calls the KeyStore#delete method 4 times, user key, user cert, and 2 CA cert verify(mKeyStore).deleteEntry(USER_CERT_ALIAS); @@ -124,7 +124,7 @@ public void testRemoveKeysForMixedInstalledCerts1() throws Exception { when(mWifiEnterpriseConfig.isAppInstalledDeviceKeyAndCert()).thenReturn(true); when(mWifiEnterpriseConfig.isAppInstalledCaCert()).thenReturn(false); - mWifiKeyStore.removeKeys(mWifiEnterpriseConfig); + mWifiKeyStore.removeKeys(mWifiEnterpriseConfig, false); // Method calls the KeyStore#deleteEntry method: user key and user cert verify(mKeyStore).deleteEntry(USER_CERT_ALIAS); @@ -139,7 +139,7 @@ public void testRemoveKeysForMixedInstalledCerts2() throws Exception { when(mWifiEnterpriseConfig.isAppInstalledDeviceKeyAndCert()).thenReturn(false); when(mWifiEnterpriseConfig.isAppInstalledCaCert()).thenReturn(true); - mWifiKeyStore.removeKeys(mWifiEnterpriseConfig); + mWifiKeyStore.removeKeys(mWifiEnterpriseConfig, false); // Method calls the KeyStore#delete method 2 times: 2 CA certs verify(mKeyStore).deleteEntry(USER_CA_CERT_ALIASES[0]); @@ -154,7 +154,24 @@ public void testRemoveKeysForUserInstalledCerts() { when(mWifiEnterpriseConfig.isAppInstalledDeviceKeyAndCert()).thenReturn(false); when(mWifiEnterpriseConfig.isAppInstalledCaCert()).thenReturn(false); - mWifiKeyStore.removeKeys(mWifiEnterpriseConfig); + mWifiKeyStore.removeKeys(mWifiEnterpriseConfig, false); + verifyNoMoreInteractions(mKeyStore); + } + + /** + * Verifies that keys and certs are removed when they were not installed by the user + * when forceRemove is true. + */ + @Test + public void testForceRemoveKeysForUserInstalledCerts() throws Exception { + when(mWifiEnterpriseConfig.isAppInstalledDeviceKeyAndCert()).thenReturn(false); + when(mWifiEnterpriseConfig.isAppInstalledCaCert()).thenReturn(false); + mWifiKeyStore.removeKeys(mWifiEnterpriseConfig, true); + + // KeyStore#deleteEntry() is called three time for user cert, and 2 CA cert. + verify(mKeyStore).deleteEntry(USER_CERT_ALIAS); + verify(mKeyStore).deleteEntry(USER_CA_CERT_ALIASES[0]); + verify(mKeyStore).deleteEntry(USER_CA_CERT_ALIASES[1]); verifyNoMoreInteractions(mKeyStore); } @@ -228,8 +245,8 @@ WifiConfiguration suggestionNetwork = new WifiConfiguration(savedNetwork); suggestionNetwork.fromWifiNetworkSuggestion = true; suggestionNetwork.creatorName = TEST_PACKAGE_NAME; - mWifiKeyStore.removeKeys(savedNetwork.enterpriseConfig); - mWifiKeyStore.removeKeys(suggestionNetwork.enterpriseConfig); + mWifiKeyStore.removeKeys(savedNetwork.enterpriseConfig, false); + mWifiKeyStore.removeKeys(suggestionNetwork.enterpriseConfig, false); verify(mKeyStore, never()).deleteEntry(any()); }
diff --git a/service/tests/wifitests/src/com/android/server/wifi/WifiNetworkSuggestionsManagerTest.java b/service/tests/wifitests/src/com/android/server/wifi/WifiNetworkSuggestionsManagerTest.java index 2b1ed72..bfa1725 100644 --- a/service/tests/wifitests/src/com/android/server/wifi/WifiNetworkSuggestionsManagerTest.java +++ b/service/tests/wifitests/src/com/android/server/wifi/WifiNetworkSuggestionsManagerTest.java
@@ -491,7 +491,8 @@ TEST_UID_1, TEST_PACKAGE_1, WifiManager.ACTION_REMOVE_SUGGESTION_DISCONNECT)); // Make sure remove the keyStore with the internal config - verify(mWifiKeyStore).removeKeys(networkSuggestion1.wifiConfiguration.enterpriseConfig); + verify(mWifiKeyStore).removeKeys(eq(networkSuggestion1.wifiConfiguration.enterpriseConfig), + eq(false)); verify(mLruConnectionTracker).removeNetwork(any()); }
diff --git a/service/tests/wifitests/src/com/android/server/wifi/WifiServiceImplTest.java b/service/tests/wifitests/src/com/android/server/wifi/WifiServiceImplTest.java index 0d9063e..503e5f1 100644 --- a/service/tests/wifitests/src/com/android/server/wifi/WifiServiceImplTest.java +++ b/service/tests/wifitests/src/com/android/server/wifi/WifiServiceImplTest.java
@@ -414,6 +414,7 @@ @Mock DevicePolicyManager mDevicePolicyManager; @Mock HalDeviceManager mHalDeviceManager; @Mock WifiDialogManager mWifiDialogManager; + @Mock WifiKeyStore mWifiKeyStore; @Captor ArgumentCaptor<Intent> mIntentCaptor; @@ -565,6 +566,7 @@ when(mContext.getSystemService(DevicePolicyManager.class)).thenReturn(mDevicePolicyManager); when(mWifiInjector.getHalDeviceManager()).thenReturn(mHalDeviceManager); when(mWifiInjector.getWifiDialogManager()).thenReturn(mWifiDialogManager); + when(mWifiInjector.getWifiKeyStore()).thenReturn(mWifiKeyStore); doAnswer(new AnswerWithArguments() { public void answer(Runnable onStoppedListener) throws Throwable { @@ -6127,7 +6129,11 @@ anyInt(), anyInt())).thenReturn(PackageManager.PERMISSION_GRANTED); when(mWifiPermissionsUtil.checkNetworkSettingsPermission(anyInt())).thenReturn(true); final String fqdn = "example.com"; - WifiConfiguration network = WifiConfigurationTestUtil.createOpenNetwork(); + WifiConfiguration openNetwork = WifiConfigurationTestUtil.createOpenNetwork(); + openNetwork.networkId = TEST_NETWORK_ID; + WifiConfiguration eapNetwork = WifiConfigurationTestUtil.createEapNetwork( + WifiEnterpriseConfig.Eap.TLS, WifiEnterpriseConfig.Phase2.NONE); + eapNetwork.networkId = TEST_NETWORK_ID + 1; PasspointConfiguration config = new PasspointConfiguration(); HomeSp homeSp = new HomeSp(); homeSp.setFqdn(fqdn); @@ -6137,7 +6143,7 @@ config.setCredential(credential); when(mWifiConfigManager.getSavedNetworks(anyInt())) - .thenReturn(Arrays.asList(network)); + .thenReturn(Arrays.asList(openNetwork, eapNetwork)); when(mPasspointManager.getProviderConfigs(anyInt(), anyBoolean())) .thenReturn(Arrays.asList(config)); @@ -6150,7 +6156,10 @@ verify(mWifiApConfigStore).setApConfiguration(null); verify(mWifiConfigManager).removeNetwork( - network.networkId, Binder.getCallingUid(), TEST_PACKAGE_NAME); + openNetwork.networkId, Binder.getCallingUid(), TEST_PACKAGE_NAME); + verify(mWifiConfigManager).removeNetwork( + eapNetwork.networkId, Binder.getCallingUid(), TEST_PACKAGE_NAME); + verify(mWifiKeyStore).removeKeys(eapNetwork.enterpriseConfig, true); verify(mPasspointManager).removeProvider(anyInt(), anyBoolean(), eq(config.getUniqueId()), isNull()); verify(mPasspointManager).clearAnqpRequestsAndFlushCache();