diff options
author | Android Build Coastguard Worker <android-build-coastguard-worker@google.com> | 2023-02-09 00:23:10 +0000 |
---|---|---|
committer | Android Build Coastguard Worker <android-build-coastguard-worker@google.com> | 2023-02-09 00:23:10 +0000 |
commit | d08e38312a32f84a7902cf2770ce71c4f4fdd71f (patch) | |
tree | f566717c8acfc0b7b2ffd98d6f13d228215dcd2d | |
parent | b2df9ddebc73fe40f0e2f1facb3b29f6fd962c0f (diff) | |
parent | 13b569eef279e71b90064dc9c3c7a2010fb1cf56 (diff) | |
download | wifi-d08e38312a32f84a7902cf2770ce71c4f4fdd71f.tar.gz |
Merge cherrypicks of ['googleplex-android-review.googlesource.com/21310542'] into security-aosp-rvc-release.android-security-11.0.0_r68android-security-11.0.0_r67android-security-11.0.0_r66
Change-Id: I0961929c80d9abc988884dc6fc810493b2595c0a
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 c0cb899d2..a8bfc822b 100644 --- a/service/java/com/android/server/wifi/WifiConfigManager.java +++ b/service/java/com/android/server/wifi/WifiConfigManager.java @@ -1388,7 +1388,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 5531c4e98..385b2dc29 100644 --- a/service/java/com/android/server/wifi/WifiInjector.java +++ b/service/java/com/android/server/wifi/WifiInjector.java @@ -847,4 +847,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 70daa0d55..08a101fe3 100644 --- a/service/java/com/android/server/wifi/WifiKeyStore.java +++ b/service/java/com/android/server/wifi/WifiKeyStore.java @@ -200,11 +200,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)) { @@ -218,7 +219,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 f4b5a14e3..bb72876bb 100644 --- a/service/java/com/android/server/wifi/WifiServiceImpl.java +++ b/service/java/com/android/server/wifi/WifiServiceImpl.java @@ -3444,7 +3444,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 9d96116eb..84c9a73ab 100644 --- a/tests/wifitests/src/com/android/server/wifi/WifiConfigManagerTest.java +++ b/tests/wifitests/src/com/android/server/wifi/WifiConfigManagerTest.java @@ -900,7 +900,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)); } /** @@ -1158,7 +1158,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()); @@ -5212,7 +5212,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 8eef7e7d0..c7d0d0caa 100644 --- a/tests/wifitests/src/com/android/server/wifi/WifiKeyStoreTest.java +++ b/tests/wifitests/src/com/android/server/wifi/WifiKeyStoreTest.java @@ -93,7 +93,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); @@ -108,7 +108,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); @@ -123,7 +123,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]); @@ -138,7 +138,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); } @@ -212,8 +229,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 2ae108dc4..cae3c7b2b 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() { @@ -4189,7 +4191,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); @@ -4200,7 +4206,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)); @@ -4212,7 +4218,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(); |