Merge "Revert "Revert "[DO NOT MERGE] wifi: remove certificates for network factory reset""" into rvc-qpr-dev

9 files changed, 58 insertions, 18 deletions

diff --git a/service/java/com/android/server/wifi/WifiConfigManager.java b/service/java/com/android/server/wifi/WifiConfigManager.java
index 8fc91cb09..f40d65624 100644
--- a/service/java/com/android/server/wifi/WifiConfigManager.java
+++ b/service/java/com/android/server/wifi/WifiConfigManager.java
@@ -1387,7 +1387,7 @@ public class WifiConfigManager {
         // 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);
         }
 
         removeConnectChoiceFromAllNetworks(config.getKey());
diff --git a/service/java/com/android/server/wifi/WifiInjector.java b/service/java/com/android/server/wifi/WifiInjector.java
index 48f1b3f2e..a90577e41 100644
--- a/service/java/com/android/server/wifi/WifiInjector.java
+++ b/service/java/com/android/server/wifi/WifiInjector.java
@@ -848,4 +848,9 @@ public class WifiInjector {
     public DeviceConfigFacade getDeviceConfigFacade() {
         return mDeviceConfigFacade;
     }
+
+    @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 c248d2269..a91446242 100644
--- a/service/java/com/android/server/wifi/WifiKeyStore.java
+++ b/service/java/com/android/server/wifi/WifiKeyStore.java
@@ -201,11 +201,12 @@ public class WifiKeyStore {
      * 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)) {
@@ -219,7 +220,7 @@ public class WifiKeyStore {
         }
 
         // 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 5483e39cc..9d3a75cf5 100644
--- a/service/java/com/android/server/wifi/WifiNetworkSuggestionsManager.java
+++ b/service/java/com/android/server/wifi/WifiNetworkSuggestionsManager.java
@@ -1117,7 +1117,7 @@ public class WifiNetworkSuggestionsManager {
                 removeFromPassPointInfoMap(ewns);
             } else {
                 if (ewns.wns.wifiConfiguration.isEnterprise()) {
-                    mWifiKeyStore.removeKeys(ewns.wns.wifiConfiguration.enterpriseConfig);
+                    mWifiKeyStore.removeKeys(ewns.wns.wifiConfiguration.enterpriseConfig, false);
                 }
                 removeFromScanResultMatchInfoMapAndRemoveRelatedScoreCard(ewns);
             }
diff --git a/service/java/com/android/server/wifi/WifiServiceImpl.java b/service/java/com/android/server/wifi/WifiServiceImpl.java
index 93029bc53..d29c88510 100644
--- a/service/java/com/android/server/wifi/WifiServiceImpl.java
+++ b/service/java/com/android/server/wifi/WifiServiceImpl.java
@@ -3449,7 +3449,14 @@ public class WifiServiceImpl extends BaseWifiService {
         List<WifiConfiguration> networks = mWifiThreadRunner.call(
                 () -> mWifiConfigManager.getSavedNetworks(Process.WIFI_UID),
                 Collections.emptyList());
+        EventLog.writeEvent(0x534e4554, "231985227", -1,
+                "Remove certs for factory reset");
         for (WifiConfiguration network : networks) {
+            if (network.isEnterprise()) {
+                mWifiThreadRunner.run(() ->
+                        mWifiInjector.getWifiKeyStore()
+                                .removeKeys(network.enterpriseConfig, true));
+            }
             removeNetwork(network.networkId, packageName);
         }
         // Delete all Passpoint configurations
diff --git a/tests/wifitests/src/com/android/server/wifi/WifiConfigManagerTest.java b/tests/wifitests/src/com/android/server/wifi/WifiConfigManagerTest.java
index 34567e969..8f822383f 100644
--- a/tests/wifitests/src/com/android/server/wifi/WifiConfigManagerTest.java
+++ b/tests/wifitests/src/com/android/server/wifi/WifiConfigManagerTest.java
@@ -901,7 +901,7 @@ public class WifiConfigManagerTest extends WifiBaseTest {
         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));
     }
 
     /**
@@ -1159,7 +1159,7 @@ public class WifiConfigManagerTest extends WifiBaseTest {
                 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());
@@ -5274,7 +5274,7 @@ public class WifiConfigManagerTest extends WifiBaseTest {
                 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/tests/wifitests/src/com/android/server/wifi/WifiKeyStoreTest.java b/tests/wifitests/src/com/android/server/wifi/WifiKeyStoreTest.java
index f6cae66db..164c0f998 100644
--- a/tests/wifitests/src/com/android/server/wifi/WifiKeyStoreTest.java
+++ b/tests/wifitests/src/com/android/server/wifi/WifiKeyStoreTest.java
@@ -94,7 +94,7 @@ public class WifiKeyStoreTest extends WifiBaseTest {
     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);
@@ -109,7 +109,7 @@ public class WifiKeyStoreTest extends WifiBaseTest {
     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);
@@ -124,7 +124,7 @@ public class WifiKeyStoreTest extends WifiBaseTest {
     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]);
@@ -139,7 +139,24 @@ public class WifiKeyStoreTest extends WifiBaseTest {
     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);
     }
 
@@ -213,8 +230,8 @@ public class WifiKeyStoreTest extends WifiBaseTest {
         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/tests/wifitests/src/com/android/server/wifi/WifiNetworkSuggestionsManagerTest.java b/tests/wifitests/src/com/android/server/wifi/WifiNetworkSuggestionsManagerTest.java
index 580640158..c8236a865 100644
--- a/tests/wifitests/src/com/android/server/wifi/WifiNetworkSuggestionsManagerTest.java
+++ b/tests/wifitests/src/com/android/server/wifi/WifiNetworkSuggestionsManagerTest.java
@@ -428,7 +428,8 @@ public class WifiNetworkSuggestionsManagerTest extends WifiBaseTest {
                         new ArrayList<WifiNetworkSuggestion>() {{ add(removingSuggestion); }},
                         TEST_UID_1, TEST_PACKAGE_1));
         // Make sure remove the keyStore with the internal config
-        verify(mWifiKeyStore).removeKeys(networkSuggestion1.wifiConfiguration.enterpriseConfig);
+        verify(mWifiKeyStore).removeKeys(networkSuggestion1.wifiConfiguration.enterpriseConfig,
+                false);
         verify(mLruConnectionTracker).removeNetwork(any());
     }
 
diff --git a/tests/wifitests/src/com/android/server/wifi/WifiServiceImplTest.java b/tests/wifitests/src/com/android/server/wifi/WifiServiceImplTest.java
index 9306e0833..2fd5c877e 100644
--- a/tests/wifitests/src/com/android/server/wifi/WifiServiceImplTest.java
+++ b/tests/wifitests/src/com/android/server/wifi/WifiServiceImplTest.java
@@ -316,6 +316,7 @@ public class WifiServiceImplTest extends WifiBaseTest {
     @Mock WifiSettingsConfigStore mWifiSettingsConfigStore;
     @Mock WifiScanAlwaysAvailableSettingsCompatibility mScanAlwaysAvailableSettingsCompatibility;
     @Mock PackageInfo mPackageInfo;
+    @Mock WifiKeyStore mWifiKeyStore;
 
     WifiLog mLog = new LogcatLog(TAG);
 
@@ -409,6 +410,7 @@ public class WifiServiceImplTest extends WifiBaseTest {
         when(mScanRequestProxy.startScan(anyInt(), anyString())).thenReturn(true);
         when(mLohsCallback.asBinder()).thenReturn(mock(IBinder.class));
         when(mWifiSettingsConfigStore.get(eq(WIFI_VERBOSE_LOGGING_ENABLED))).thenReturn(true);
+        when(mWifiInjector.getWifiKeyStore()).thenReturn(mWifiKeyStore);
 
         mWifiServiceImpl = makeWifiServiceImpl();
         mDppCallback = new IDppCallback() {
@@ -4194,7 +4196,11 @@ public class WifiServiceImplTest extends WifiBaseTest {
                 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);
@@ -4205,7 +4211,7 @@ public class WifiServiceImplTest extends WifiBaseTest {
 
         mWifiServiceImpl.mClientModeImplChannel = mAsyncChannel;
         when(mWifiConfigManager.getSavedNetworks(anyInt()))
-                .thenReturn(Arrays.asList(network));
+                .thenReturn(Arrays.asList(openNetwork, eapNetwork));
         when(mPasspointManager.getProviderConfigs(anyInt(), anyBoolean()))
                 .thenReturn(Arrays.asList(config));
 
@@ -4217,7 +4223,10 @@ public class WifiServiceImplTest extends WifiBaseTest {
         mLooper.dispatchAll();
         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();