From 93af3137fca25cbfe907f28ecd7eb303b4f85cba Mon Sep 17 00:00:00 2001 From: Janis Danisevskis Date: Mon, 25 Jan 2021 14:56:47 -0800 Subject: [PATCH] Keystore 2.0: Update Wifi settings to use mostly public keystore API. Test: N/A Bug: 171305607 Bug: 171305388 Merged-In: Ib794c5f2d904c2b187d7d5fd00b81afc852d0052 Change-Id: Ib794c5f2d904c2b187d7d5fd00b81afc852d0052 --- .../utils/AndroidKeystoreAliasLoader.java | 124 ++++++++++++++++++ .../settings/wifi/WifiConfigController.java | 33 +++-- .../settings/wifi/WifiConfigController2.java | 32 +++-- .../wifi/WifiConfigController2Test.java | 42 ++---- .../wifi/WifiConfigControllerTest.java | 25 +--- 5 files changed, 168 insertions(+), 88 deletions(-) create mode 100644 src/com/android/settings/utils/AndroidKeystoreAliasLoader.java diff --git a/src/com/android/settings/utils/AndroidKeystoreAliasLoader.java b/src/com/android/settings/utils/AndroidKeystoreAliasLoader.java new file mode 100644 index 00000000000..a4af7aa29eb --- /dev/null +++ b/src/com/android/settings/utils/AndroidKeystoreAliasLoader.java @@ -0,0 +1,124 @@ +/* + * Copyright (C) 2021 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.settings.utils; + +import android.os.Process; +import android.security.keystore.AndroidKeyStoreProvider; +import android.security.keystore.KeyProperties; +import android.security.keystore2.AndroidKeyStoreLoadStoreParameter; +import android.util.Log; + +import java.security.Key; +import java.security.KeyStore; +import java.security.KeyStoreException; +import java.security.NoSuchAlgorithmException; +import java.security.PrivateKey; +import java.security.UnrecoverableKeyException; +import java.util.ArrayList; +import java.util.Collection; +import java.util.Enumeration; + +/** + * This class provides a portable and unified way to load the content of AndroidKeyStore through + * public API. + * @hide + */ +public class AndroidKeystoreAliasLoader { + private static final String TAG = "SettingsKeystoreUtils"; + + private final Collection mKeyCertAliases; + private final Collection mCaCertAliases; + /** + * This Constructor loads all aliases of asymmetric keys pairs and certificates in the + * AndroidKeyStore within the given namespace. + * Viable namespaces are {@link KeyProperties#NAMESPACE_WIFI}, + * {@link KeyProperties#NAMESPACE_APPLICATION}, or null. The latter two are equivalent in + * that they will load the keystore content of the app's own namespace. In case of settings, + * this is the namespace of the AID_SYSTEM. + * + * @param namespace {@link KeyProperties#NAMESPACE_WIFI}, + * {@link KeyProperties#NAMESPACE_APPLICATION}, or null + * @hide + */ + public AndroidKeystoreAliasLoader(Integer namespace) { + mKeyCertAliases = new ArrayList<>(); + mCaCertAliases = new ArrayList<>(); + KeyStore keyStore = null; + final Enumeration aliases; + try { + if (namespace != null && namespace != KeyProperties.NAMESPACE_APPLICATION) { + if (AndroidKeyStoreProvider.isKeystore2Enabled()) { + keyStore = KeyStore.getInstance("AndroidKeyStore"); + keyStore.load(new AndroidKeyStoreLoadStoreParameter(namespace)); + } else { + // In the legacy case we pass in the WIFI UID because that is the only + // possible special namespace that existed as of this writing, + // and new namespaces must only be added using the new mechanism. + keyStore = AndroidKeyStoreProvider.getKeyStoreForUid(Process.WIFI_UID); + } + } else { + keyStore = KeyStore.getInstance("AndroidKeyStore"); + keyStore.load(null); + } + aliases = keyStore.aliases(); + } catch (Exception e) { + Log.e(TAG, "Failed to open Android Keystore.", e); + // Will return empty lists. + return; + } + + + while (aliases.hasMoreElements()) { + String alias = aliases.nextElement(); + try { + Key key = keyStore.getKey(alias, null); + if (key != null) { + if (key instanceof PrivateKey) { + mKeyCertAliases.add(alias); + } + } else { + if (keyStore.getCertificate(alias) != null) { + mCaCertAliases.add(alias); + } + } + } catch (KeyStoreException | NoSuchAlgorithmException | UnrecoverableKeyException e) { + Log.e(TAG, "Failed to load alias: " + + alias + " from Android Keystore. Ignoring.", e); + } + } + } + + /** + * Returns the aliases of the key pairs and certificates stored in the Android KeyStore at the + * time the constructor was called. + * @return Collection of keystore aliases. + * @hide + */ + public Collection getKeyCertAliases() { + return mKeyCertAliases; + } + + /** + * Returns the aliases of the trusted certificates stored in the Android KeyStore at the + * time the constructor was called. + * @return Collection of keystore aliases. + * @hide + */ + public Collection getCaCertAliases() { + return mCaCertAliases; + } +} diff --git a/src/com/android/settings/wifi/WifiConfigController.java b/src/com/android/settings/wifi/WifiConfigController.java index 3c95e910196..b2eca781a8a 100644 --- a/src/com/android/settings/wifi/WifiConfigController.java +++ b/src/com/android/settings/wifi/WifiConfigController.java @@ -34,8 +34,7 @@ import android.net.wifi.WifiEnterpriseConfig.Phase2; import android.net.wifi.WifiInfo; import android.net.wifi.WifiManager; import android.os.IBinder; -import android.security.Credentials; -import android.security.KeyStore; +import android.security.keystore.KeyProperties; import android.telephony.SubscriptionInfo; import android.telephony.SubscriptionManager; import android.text.Editable; @@ -72,6 +71,7 @@ import com.android.net.module.util.ProxyUtils; import com.android.settings.ProxySelector; import com.android.settings.R; import com.android.settings.network.SubscriptionUtil; +import com.android.settings.utils.AndroidKeystoreAliasLoader; import com.android.settings.wifi.dpp.WifiDppUtils; import com.android.settingslib.Utils; import com.android.settingslib.utils.ThreadUtils; @@ -80,7 +80,7 @@ import com.android.settingslib.wifi.AccessPoint; import java.net.Inet4Address; import java.net.InetAddress; import java.util.ArrayList; -import java.util.Arrays; +import java.util.Collection; import java.util.Collections; import java.util.Iterator; import java.util.List; @@ -1031,15 +1031,17 @@ public class WifiConfigController implements TextWatcher, if (refreshCertificates) { loadSims(); + final AndroidKeystoreAliasLoader androidKeystoreAliasLoader = + getAndroidKeystoreAliasLoader(); loadCertificates( mEapCaCertSpinner, - Credentials.CA_CERTIFICATE, + androidKeystoreAliasLoader.getCaCertAliases(), null /* noCertificateString */, false /* showMultipleCerts */, true /* showUsePreinstalledCertOption */); loadCertificates( mEapUserCertSpinner, - Credentials.USER_PRIVATE_KEY, + androidKeystoreAliasLoader.getKeyCertAliases(), mDoNotProvideEapUserCertString, false /* showMultipleCerts */, false /* showUsePreinstalledCertOption */); @@ -1122,10 +1124,13 @@ public class WifiConfigController implements TextWatcher, } else if (caCerts.length == 1) { setSelection(mEapCaCertSpinner, caCerts[0]); } else { + final AndroidKeystoreAliasLoader androidKeystoreAliasLoader = + getAndroidKeystoreAliasLoader(); + // Reload the cert spinner with an extra "multiple certificates added" item. loadCertificates( mEapCaCertSpinner, - Credentials.CA_CERTIFICATE, + androidKeystoreAliasLoader.getCaCertAliases(), null /* noCertificateString */, true /* showMultipleCerts */, true /* showUsePreinstalledCertOption */); @@ -1444,8 +1449,8 @@ public class WifiConfigController implements TextWatcher, } @VisibleForTesting - KeyStore getKeyStore() { - return KeyStore.getInstance(); + AndroidKeystoreAliasLoader getAndroidKeystoreAliasLoader() { + return new AndroidKeystoreAliasLoader(KeyProperties.NAMESPACE_WIFI); } @VisibleForTesting @@ -1489,7 +1494,7 @@ public class WifiConfigController implements TextWatcher, @VisibleForTesting void loadCertificates( Spinner spinner, - String prefix, + Collection choices, String noCertificateString, boolean showMultipleCerts, boolean showUsePreinstalledCertOption) { @@ -1504,14 +1509,8 @@ public class WifiConfigController implements TextWatcher, certs.add(mUseSystemCertsString); } - String[] certificateNames = null; - try { - certificateNames = getKeyStore().list(prefix, android.os.Process.WIFI_UID); - } catch (Exception e) { - Log.e(TAG, "can't get the certificate list from KeyStore"); - } - if (certificateNames != null && certificateNames.length != 0) { - certs.addAll(Arrays.stream(certificateNames) + if (choices != null && choices.size() != 0) { + certs.addAll(choices.stream() .filter(certificateName -> { for (String undesired : UNDESIRED_CERTIFICATES) { if (certificateName.startsWith(undesired)) { diff --git a/src/com/android/settings/wifi/WifiConfigController2.java b/src/com/android/settings/wifi/WifiConfigController2.java index 5933d6f84fb..b4e26a6ab94 100644 --- a/src/com/android/settings/wifi/WifiConfigController2.java +++ b/src/com/android/settings/wifi/WifiConfigController2.java @@ -32,8 +32,7 @@ import android.net.wifi.WifiEnterpriseConfig.Eap; import android.net.wifi.WifiEnterpriseConfig.Phase2; import android.net.wifi.WifiManager; import android.os.IBinder; -import android.security.Credentials; -import android.security.KeyStore; +import android.security.keystore.KeyProperties; import android.telephony.SubscriptionInfo; import android.telephony.SubscriptionManager; import android.text.Editable; @@ -70,6 +69,7 @@ import com.android.net.module.util.ProxyUtils; import com.android.settings.ProxySelector; import com.android.settings.R; import com.android.settings.network.SubscriptionUtil; +import com.android.settings.utils.AndroidKeystoreAliasLoader; import com.android.settings.wifi.details2.WifiPrivacyPreferenceController2; import com.android.settings.wifi.dpp.WifiDppUtils; import com.android.settingslib.Utils; @@ -80,7 +80,7 @@ import com.android.wifitrackerlib.WifiEntry.ConnectedInfo; import java.net.Inet4Address; import java.net.InetAddress; import java.util.ArrayList; -import java.util.Arrays; +import java.util.Collection; import java.util.Collections; import java.util.Iterator; import java.util.List; @@ -994,15 +994,17 @@ public class WifiConfigController2 implements TextWatcher, if (refreshCertificates) { loadSims(); + final AndroidKeystoreAliasLoader androidKeystoreAliasLoader = + getAndroidKeystoreAliasLoader(); loadCertificates( mEapCaCertSpinner, - Credentials.CA_CERTIFICATE, + androidKeystoreAliasLoader.getCaCertAliases(), null /* noCertificateString */, false /* showMultipleCerts */, true /* showUsePreinstalledCertOption */); loadCertificates( mEapUserCertSpinner, - Credentials.USER_PRIVATE_KEY, + androidKeystoreAliasLoader.getKeyCertAliases(), mDoNotProvideEapUserCertString, false /* showMultipleCerts */, false /* showUsePreinstalledCertOption */); @@ -1087,9 +1089,11 @@ public class WifiConfigController2 implements TextWatcher, setSelection(mEapCaCertSpinner, caCerts[0]); } else { // Reload the cert spinner with an extra "multiple certificates added" item. + final AndroidKeystoreAliasLoader androidKeystoreAliasLoader = + getAndroidKeystoreAliasLoader(); loadCertificates( mEapCaCertSpinner, - Credentials.CA_CERTIFICATE, + androidKeystoreAliasLoader.getCaCertAliases(), null /* noCertificateString */, true /* showMultipleCerts */, true /* showUsePreinstalledCertOption */); @@ -1408,8 +1412,8 @@ public class WifiConfigController2 implements TextWatcher, } @VisibleForTesting - KeyStore getKeyStore() { - return KeyStore.getInstance(); + AndroidKeystoreAliasLoader getAndroidKeystoreAliasLoader() { + return new AndroidKeystoreAliasLoader(KeyProperties.NAMESPACE_WIFI); } @VisibleForTesting @@ -1453,7 +1457,7 @@ public class WifiConfigController2 implements TextWatcher, @VisibleForTesting void loadCertificates( Spinner spinner, - String prefix, + Collection choices, String noCertificateString, boolean showMultipleCerts, boolean showUsePreinstalledCertOption) { @@ -1468,14 +1472,8 @@ public class WifiConfigController2 implements TextWatcher, certs.add(mUseSystemCertsString); } - String[] certificateNames = null; - try { - certificateNames = getKeyStore().list(prefix, android.os.Process.WIFI_UID); - } catch (Exception e) { - Log.e(TAG, "can't get the certificate list from KeyStore"); - } - if (certificateNames != null && certificateNames.length != 0) { - certs.addAll(Arrays.stream(certificateNames) + if (choices != null && choices.size() != 0) { + certs.addAll(choices.stream() .filter(certificateName -> { for (String undesired : UNDESIRED_CERTIFICATES) { if (certificateName.startsWith(undesired)) { diff --git a/tests/robotests/src/com/android/settings/wifi/WifiConfigController2Test.java b/tests/robotests/src/com/android/settings/wifi/WifiConfigController2Test.java index 22e18bbe8ae..d227ac1bb1d 100644 --- a/tests/robotests/src/com/android/settings/wifi/WifiConfigController2Test.java +++ b/tests/robotests/src/com/android/settings/wifi/WifiConfigController2Test.java @@ -18,9 +18,6 @@ package com.android.settings.wifi; import static com.google.common.truth.Truth.assertThat; -import static org.mockito.Mockito.anyInt; -import static org.mockito.Mockito.anyString; -import static org.mockito.Mockito.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; import static org.robolectric.Shadows.shadowOf; @@ -33,9 +30,6 @@ import android.net.wifi.WifiEnterpriseConfig; import android.net.wifi.WifiEnterpriseConfig.Eap; import android.net.wifi.WifiEnterpriseConfig.Phase2; import android.net.wifi.WifiManager; -import android.os.ServiceSpecificException; -import android.security.Credentials; -import android.security.KeyStore; import android.telephony.SubscriptionInfo; import android.telephony.SubscriptionManager; import android.telephony.TelephonyManager; @@ -51,9 +45,12 @@ import android.widget.TextView; import com.android.settings.R; import com.android.settings.network.SubscriptionUtil; import com.android.settings.testutils.shadow.ShadowConnectivityManager; +import com.android.settings.utils.AndroidKeystoreAliasLoader; import com.android.settings.wifi.details2.WifiPrivacyPreferenceController2; import com.android.wifitrackerlib.WifiEntry; +import com.google.common.collect.ImmutableList; + import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -79,7 +76,7 @@ public class WifiConfigController2Test { @Mock private WifiEntry mWifiEntry; @Mock - private KeyStore mKeyStore; + private AndroidKeystoreAliasLoader mAndroidKeystoreAliasLoader; private View mView; private Spinner mHiddenSettingsSpinner; private Spinner mEapCaCertSpinner; @@ -285,28 +282,12 @@ public class WifiConfigController2Test { assertThat(mController.getSignalString()).isNull(); } - @Test - public void loadCertificates_keyStoreListFail_shouldNotCrash() { - // Set up - when(mWifiEntry.getSecurity()).thenReturn(WifiEntry.SECURITY_EAP); - when(mKeyStore.list(anyString())) - .thenThrow(new ServiceSpecificException(-1, "permission error")); - - mController = new TestWifiConfigController2(mConfigUiBase, mView, mWifiEntry, - WifiConfigUiBase2.MODE_CONNECT); - - // Verify that the EAP method menu is visible. - assertThat(mView.findViewById(R.id.eap).getVisibility()).isEqualTo(View.VISIBLE); - // No Crash - } - @Test public void loadCertificates_undesiredCertificates_shouldNotLoadUndesiredCertificates() { final Spinner spinner = new Spinner(mContext); - when(mKeyStore.list(anyString())).thenReturn(WifiConfigController2.UNDESIRED_CERTIFICATES); mController.loadCertificates(spinner, - "prefix", + Arrays.asList(WifiConfigController.UNDESIRED_CERTIFICATES), "doNotProvideEapUserCertString", false /* showMultipleCerts */, false /* showUsePreinstalledCertOption */); @@ -432,8 +413,8 @@ public class WifiConfigController2Test { } @Override - KeyStore getKeyStore() { - return mKeyStore; + AndroidKeystoreAliasLoader getAndroidKeystoreAliasLoader() { + return mAndroidKeystoreAliasLoader; } } @@ -882,6 +863,7 @@ public class WifiConfigController2Test { String savedUserCertificate) { final WifiConfiguration mockWifiConfig = mock(WifiConfiguration.class); final WifiEnterpriseConfig mockWifiEnterpriseConfig = mock(WifiEnterpriseConfig.class); + mockWifiConfig.enterpriseConfig = mockWifiEnterpriseConfig; when(mWifiEntry.isSaved()).thenReturn(true); when(mWifiEntry.getSecurity()).thenReturn(WifiEntry.SECURITY_EAP); @@ -892,15 +874,15 @@ public class WifiConfigController2Test { String[] savedCaCertificates = new String[]{savedCaCertificate}; when(mockWifiEnterpriseConfig.getCaCertificateAliases()) .thenReturn(savedCaCertificates); - when(mKeyStore.list(eq(Credentials.CA_CERTIFICATE), anyInt())) - .thenReturn(savedCaCertificates); + when(mAndroidKeystoreAliasLoader.getCaCertAliases()) + .thenReturn(ImmutableList.of(savedCaCertificate)); } if (savedUserCertificate != null) { String[] savedUserCertificates = new String[]{savedUserCertificate}; when(mockWifiEnterpriseConfig.getClientCertificateAlias()) .thenReturn(savedUserCertificate); - when(mKeyStore.list(eq(Credentials.USER_PRIVATE_KEY), anyInt())) - .thenReturn(savedUserCertificates); + when(mAndroidKeystoreAliasLoader.getKeyCertAliases()) + .thenReturn(ImmutableList.of(savedUserCertificate)); } mController = new TestWifiConfigController2(mConfigUiBase, mView, mWifiEntry, diff --git a/tests/robotests/src/com/android/settings/wifi/WifiConfigControllerTest.java b/tests/robotests/src/com/android/settings/wifi/WifiConfigControllerTest.java index 455e9f186b1..80543ec9427 100644 --- a/tests/robotests/src/com/android/settings/wifi/WifiConfigControllerTest.java +++ b/tests/robotests/src/com/android/settings/wifi/WifiConfigControllerTest.java @@ -21,7 +21,6 @@ import static com.android.settings.wifi.WifiConfigController.PRIVACY_SPINNER_IND import static com.google.common.truth.Truth.assertThat; -import static org.mockito.Mockito.anyString; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; import static org.robolectric.Shadows.shadowOf; @@ -34,8 +33,6 @@ import android.net.wifi.WifiEnterpriseConfig; import android.net.wifi.WifiEnterpriseConfig.Eap; import android.net.wifi.WifiEnterpriseConfig.Phase2; import android.net.wifi.WifiManager; -import android.os.ServiceSpecificException; -import android.security.KeyStore; import android.telephony.SubscriptionInfo; import android.telephony.SubscriptionManager; import android.telephony.TelephonyManager; @@ -77,8 +74,6 @@ public class WifiConfigControllerTest { private Context mContext; @Mock private AccessPoint mAccessPoint; - @Mock - private KeyStore mKeyStore; private View mView; private Spinner mHiddenSettingsSpinner; private ShadowSubscriptionManager mShadowSubscriptionManager; @@ -266,28 +261,12 @@ public class WifiConfigControllerTest { assertThat(mController.getSignalString()).isNull(); } - @Test - public void loadCertificates_keyStoreListFail_shouldNotCrash() { - // Set up - when(mAccessPoint.getSecurity()).thenReturn(AccessPoint.SECURITY_EAP); - when(mKeyStore.list(anyString())) - .thenThrow(new ServiceSpecificException(-1, "permission error")); - - mController = new TestWifiConfigController(mConfigUiBase, mView, mAccessPoint, - WifiConfigUiBase.MODE_CONNECT); - - // Verify that the EAP method menu is visible. - assertThat(mView.findViewById(R.id.eap).getVisibility()).isEqualTo(View.VISIBLE); - // No Crash - } - @Test public void loadCertificates_undesiredCertificates_shouldNotLoadUndesiredCertificates() { final Spinner spinner = new Spinner(mContext); - when(mKeyStore.list(anyString())).thenReturn(WifiConfigController.UNDESIRED_CERTIFICATES); mController.loadCertificates(spinner, - "prefix", + Arrays.asList(WifiConfigController.UNDESIRED_CERTIFICATES), "doNotProvideEapUserCertString", false /* showMultipleCerts */, false /* showUsePreinstalledCertOption */); @@ -412,8 +391,6 @@ public class WifiConfigControllerTest { super(parent, view, accessPoint, mode, wifiManager); } - @Override - KeyStore getKeyStore() { return mKeyStore; } } @Test