From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Alexander Grund <flamefire89@gmail.com>
Date: Thu, 27 Apr 2023 18:01:32 +0200
Subject: [PATCH] Revert "Revert "[DO NOT MERGE] wifi: remove certificates for
 network factory reset""

Bug: 231985227
This reverts commit f8016d78f53ceedef3eed6a7118a9c8b3ec4bd03.

Reason for revert: re-release security patch

Change-Id: I91ec19ee9f70a676de96b86c212416f171e15943
(cherry picked from commit 13b569eef279e71b90064dc9c3c7a2010fb1cf56)
Merged-In: I91ec19ee9f70a676de96b86c212416f171e15943
---
 .../server/wifi/WifiConfigManager.java        |  2 +-
 .../com/android/server/wifi/WifiInjector.java |  5 ++++
 .../com/android/server/wifi/WifiKeyStore.java |  7 +++--
 .../wifi/WifiNetworkSuggestionsManager.java   |  2 +-
 .../android/server/wifi/WifiServiceImpl.java  |  6 ++++
 .../server/wifi/WifiConfigManagerTest.java    |  6 ++--
 .../android/server/wifi/WifiKeyStoreTest.java | 28 ++++++++++++++++---
 .../WifiNetworkSuggestionsManagerTest.java    |  2 +-
 .../server/wifi/WifiServiceImplTest.java      | 14 ++++++++--
 9 files changed, 56 insertions(+), 16 deletions(-)

diff --git a/service/java/com/android/server/wifi/WifiConfigManager.java b/service/java/com/android/server/wifi/WifiConfigManager.java
index 393a5c395..fce5758e1 100644
--- a/service/java/com/android/server/wifi/WifiConfigManager.java
+++ b/service/java/com/android/server/wifi/WifiConfigManager.java
@@ -1359,7 +1359,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.isPasspoint() && !config.fromWifiNetworkSuggestion && config.isEnterprise()) {
-            mWifiKeyStore.removeKeys(config.enterpriseConfig);
+            mWifiKeyStore.removeKeys(config.enterpriseConfig, false);
         }
 
         removeConnectChoiceFromAllNetworks(config.configKey());
diff --git a/service/java/com/android/server/wifi/WifiInjector.java b/service/java/com/android/server/wifi/WifiInjector.java
index 17106c187..0bedc9a90 100644
--- a/service/java/com/android/server/wifi/WifiInjector.java
+++ b/service/java/com/android/server/wifi/WifiInjector.java
@@ -793,4 +793,9 @@ public class WifiInjector {
     public String getWifiStackPackageName() {
         return mContext.getPackageName();
     }
+
+    @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 c1706a20d..8e82f4720 100644
--- a/service/java/com/android/server/wifi/WifiKeyStore.java
+++ b/service/java/com/android/server/wifi/WifiKeyStore.java
@@ -221,10 +221,11 @@ 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) {
         // 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 @@ 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();
             // a valid ca certificate is configured
             if (aliases != null) {
diff --git a/service/java/com/android/server/wifi/WifiNetworkSuggestionsManager.java b/service/java/com/android/server/wifi/WifiNetworkSuggestionsManager.java
index 031aec603..ae7892cd2 100644
--- a/service/java/com/android/server/wifi/WifiNetworkSuggestionsManager.java
+++ b/service/java/com/android/server/wifi/WifiNetworkSuggestionsManager.java
@@ -668,7 +668,7 @@ public class WifiNetworkSuggestionsManager {
             if (!config.isEnterprise()) {
                 continue;
             }
-            mWifiKeyStore.removeKeys(config.enterpriseConfig);
+            mWifiKeyStore.removeKeys(config.enterpriseConfig, false);
         }
         // Clear the scan cache.
         removeFromScanResultMatchInfoMap(removingSuggestions);
diff --git a/service/java/com/android/server/wifi/WifiServiceImpl.java b/service/java/com/android/server/wifi/WifiServiceImpl.java
index d375ba4c8..0d6732dd7 100644
--- a/service/java/com/android/server/wifi/WifiServiceImpl.java
+++ b/service/java/com/android/server/wifi/WifiServiceImpl.java
@@ -3002,7 +3002,13 @@ public class WifiServiceImpl extends BaseWifiService {
                 List<WifiConfiguration> networks = mClientModeImpl.syncGetConfiguredNetworks(
                         Binder.getCallingUid(), mClientModeImplChannel, Process.WIFI_UID);
                 if (networks != null) {
+                    EventLog.writeEvent(0x534e4554, "231985227", -1,
+                            "Remove certs for factory reset");
                     for (WifiConfiguration config : networks) {
+                        if (config.isEnterprise()) {
+                            mWifiInjector.getWifiKeyStore().removeKeys(
+                                    config.enterpriseConfig, true);
+                        }
                         removeNetwork(config.networkId, packageName);
                     }
                 }
diff --git a/tests/wifitests/src/com/android/server/wifi/WifiConfigManagerTest.java b/tests/wifitests/src/com/android/server/wifi/WifiConfigManagerTest.java
index f5ad049dd..71d06fb7e 100644
--- a/tests/wifitests/src/com/android/server/wifi/WifiConfigManagerTest.java
+++ b/tests/wifitests/src/com/android/server/wifi/WifiConfigManagerTest.java
@@ -703,7 +703,7 @@ public class WifiConfigManagerTest {
         verify(mWcmListener, never()).onSavedNetworkAdded(suggestionNetwork.networkId);
         assertTrue(mWifiConfigManager
                 .removeNetwork(suggestionNetwork.networkId, TEST_CREATOR_UID));
-        verify(mWifiKeyStore, never()).removeKeys(any());
+        verify(mWifiKeyStore, never()).removeKeys(any(), eq(false));
     }
 
     /**
@@ -947,7 +947,7 @@ public class WifiConfigManagerTest {
         assertTrue(mWifiConfigManager.removeNetwork(passpointNetwork.networkId, Process.WIFI_UID));
 
         // Verify keys are not being removed.
-        verify(mWifiKeyStore, never()).removeKeys(any(WifiEnterpriseConfig.class));
+        verify(mWifiKeyStore, never()).removeKeys(any(WifiEnterpriseConfig.class), eq(false));
         verifyNetworkRemoveBroadcast(passpointNetwork);
         // Ensure that the write was not invoked for Passpoint network remove.
         mContextConfigStoreMockOrder.verify(mWifiConfigStore, never()).write(anyBoolean());
@@ -5169,7 +5169,7 @@ public class WifiConfigManagerTest {
         assertTrue(mWifiConfigManager.removeNetwork(configuration.networkId, TEST_CREATOR_UID));
 
         // Verify keys are not being removed.
-        verify(mWifiKeyStore, never()).removeKeys(any(WifiEnterpriseConfig.class));
+        verify(mWifiKeyStore, never()).removeKeys(any(WifiEnterpriseConfig.class), eq(false));
         verifyNetworkRemoveBroadcast(configuration);
         // 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 7649d1ba4..3f54c3f85 100644
--- a/tests/wifitests/src/com/android/server/wifi/WifiKeyStoreTest.java
+++ b/tests/wifitests/src/com/android/server/wifi/WifiKeyStoreTest.java
@@ -96,7 +96,7 @@ public class WifiKeyStoreTest {
     public void testRemoveKeysForAppInstalledCerts() {
         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).delete(Credentials.USER_PRIVATE_KEY + USER_CERT_ALIAS, Process.WIFI_UID);
@@ -115,7 +115,7 @@ public class WifiKeyStoreTest {
     public void testRemoveKeysForMixedInstalledCerts1() {
         when(mWifiEnterpriseConfig.isAppInstalledDeviceKeyAndCert()).thenReturn(true);
         when(mWifiEnterpriseConfig.isAppInstalledCaCert()).thenReturn(false);
-        mWifiKeyStore.removeKeys(mWifiEnterpriseConfig);
+        mWifiKeyStore.removeKeys(mWifiEnterpriseConfig, false);
 
         // Method calls the KeyStore#delete method 2 times: user key and user cert
         verify(mKeyStore).delete(Credentials.USER_PRIVATE_KEY + USER_CERT_ALIAS, Process.WIFI_UID);
@@ -131,7 +131,7 @@ public class WifiKeyStoreTest {
     public void testRemoveKeysForMixedInstalledCerts2() {
         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).delete(Credentials.CA_CERTIFICATE + USER_CA_CERT_ALIAS[0],
@@ -148,7 +148,27 @@ public class WifiKeyStoreTest {
     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#delete() is called three time for user cert, user key, and 2 CA cert.
+        verify(mKeyStore).delete(Credentials.USER_PRIVATE_KEY + USER_CERT_ALIAS, Process.WIFI_UID);
+        verify(mKeyStore).delete(Credentials.USER_CERTIFICATE + USER_CERT_ALIAS, Process.WIFI_UID);
+        verify(mKeyStore).delete(Credentials.CA_CERTIFICATE + USER_CA_CERT_ALIAS[0],
+                Process.WIFI_UID);
+        verify(mKeyStore).delete(Credentials.CA_CERTIFICATE + USER_CA_CERT_ALIAS[1],
+                Process.WIFI_UID);
         verifyNoMoreInteractions(mKeyStore);
     }
 
diff --git a/tests/wifitests/src/com/android/server/wifi/WifiNetworkSuggestionsManagerTest.java b/tests/wifitests/src/com/android/server/wifi/WifiNetworkSuggestionsManagerTest.java
index ed5bb39ab..60dcf153c 100644
--- a/tests/wifitests/src/com/android/server/wifi/WifiNetworkSuggestionsManagerTest.java
+++ b/tests/wifitests/src/com/android/server/wifi/WifiNetworkSuggestionsManagerTest.java
@@ -344,7 +344,7 @@ public class WifiNetworkSuggestionsManagerTest {
         assertEquals(WifiManager.STATUS_NETWORK_SUGGESTIONS_SUCCESS,
                 mWifiNetworkSuggestionsManager.remove(new ArrayList<>(),
                         TEST_UID_1, TEST_PACKAGE_1));
-        verify(mWifiKeyStore).removeKeys(any());
+        verify(mWifiKeyStore).removeKeys(any(), eq(false));
     }
     /**
      * Verify successful replace (add,remove, add) of network suggestions.
diff --git a/tests/wifitests/src/com/android/server/wifi/WifiServiceImplTest.java b/tests/wifitests/src/com/android/server/wifi/WifiServiceImplTest.java
index b7137634c..53d6d7815 100644
--- a/tests/wifitests/src/com/android/server/wifi/WifiServiceImplTest.java
+++ b/tests/wifitests/src/com/android/server/wifi/WifiServiceImplTest.java
@@ -260,6 +260,7 @@ public class WifiServiceImplTest {
     @Mock WifiScoreCard mWifiScoreCard;
     @Mock PasspointManager mPasspointManager;
     @Mock IDppCallback mDppCallback;
+    @Mock WifiKeyStore mWifiKeyStore;
 
     @Spy FakeWifiLog mLog;
 
@@ -401,6 +402,7 @@ public class WifiServiceImplTest {
         when(mContext.checkPermission(eq(android.Manifest.permission.NETWORK_MANAGED_PROVISIONING),
                 anyInt(), anyInt())).thenReturn(PackageManager.PERMISSION_DENIED);
         when(mScanRequestProxy.startScan(anyInt(), anyString())).thenReturn(true);
+        when(mWifiInjector.getWifiKeyStore()).thenReturn(mWifiKeyStore);
 
         ArgumentCaptor<SoftApCallback> softApCallbackCaptor =
                 ArgumentCaptor.forClass(SoftApCallback.class);
@@ -3641,7 +3643,11 @@ public class WifiServiceImplTest {
                 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);
@@ -3649,7 +3655,7 @@ public class WifiServiceImplTest {
 
         mWifiServiceImpl.mClientModeImplChannel = mAsyncChannel;
         when(mClientModeImpl.syncGetConfiguredNetworks(anyInt(), any(), anyInt()))
-                .thenReturn(Arrays.asList(network));
+                .thenReturn(Arrays.asList(openNetwork, eapNetwork));
         when(mClientModeImpl.syncGetPasspointConfigs(any(), anyBoolean()))
                 .thenReturn(Arrays.asList(config));
 
@@ -3657,7 +3663,9 @@ public class WifiServiceImplTest {
         mLooper.dispatchAll();
 
         verify(mWifiApConfigStore).setApConfiguration(null);
-        verify(mClientModeImpl).syncRemoveNetwork(mAsyncChannel, network.networkId);
+        verify(mClientModeImpl).syncRemoveNetwork(mAsyncChannel, openNetwork.networkId);
+        verify(mClientModeImpl).syncRemoveNetwork(mAsyncChannel, eapNetwork.networkId);
+        verify(mWifiKeyStore).removeKeys(eapNetwork.enterpriseConfig, true);
         verify(mClientModeImpl).syncRemovePasspointConfig(mAsyncChannel, true, fqdn);
         verify(mWifiConfigManager).clearDeletedEphemeralNetworks();
         verify(mClientModeImpl).clearNetworkRequestUserApprovedAccessPoints();
