From 33cde5dbeee934269f16d72e26e651d56a13733e Mon Sep 17 00:00:00 2001
From: Arc Wang <arcwang@google.com>
Date: Fri, 31 Jul 2020 10:28:04 +0800
Subject: [PATCH] [Wi-Fi] Remove 'Do not validate' option in CA certificate
 spinner

"Do not validate" is not secure and must not be used in any production
network. R Suggestions API already does not allow connections with
no Root CA certificate.

Bug: 162466484
Test: make RunSettingsRoboTests ROBOTEST_FILTER=WifiConfigControllerTest
      make RunSettingsRoboTests ROBOTEST_FILTER=WifiConfigController2Test
      Manually add or edit  an EAP method Wi-Fi network and observe CA
      certificate spinner options.
Change-Id: Ica5ecf8805c51d7cdce7b16e02e0dd79d5f64754
Merged-In: Icf97199967faa2e359b0120a8c989bd1df2c6cea
---
 res/layout/wifi_dialog.xml                    | 12 -----
 res/values/strings.xml                        |  2 -
 .../settings/wifi/WifiConfigController.java   | 45 +++++++------------
 .../settings/wifi/WifiConfigController2.java  | 45 +++++++------------
 4 files changed, 32 insertions(+), 72 deletions(-)

diff --git a/res/layout/wifi_dialog.xml b/res/layout/wifi_dialog.xml
index 45ecd610e7e..a0f49e4b35a 100644
--- a/res/layout/wifi_dialog.xml
+++ b/res/layout/wifi_dialog.xml
@@ -196,18 +196,6 @@
                             android:entries="@array/eap_ocsp_type" />
                 </LinearLayout>
 
-                <LinearLayout android:id="@+id/no_ca_cert_warning"
-                        android:layout_width="match_parent"
-                        android:layout_height="wrap_content"
-                        android:visibility="gone"
-                        style="@style/wifi_item" >
-                    <TextView
-                            android:layout_width="wrap_content"
-                            android:layout_height="wrap_content"
-                            style="@style/wifi_item_warning"
-                            android:text="@string/wifi_do_not_validate_eap_server_warning" />
-                </LinearLayout>
-
                 <LinearLayout android:id="@+id/l_domain"
                     android:layout_width="match_parent"
                     android:layout_height="wrap_content"
diff --git a/res/values/strings.xml b/res/values/strings.xml
index 1a8eae2540a..56fd5c0db52 100644
--- a/res/values/strings.xml
+++ b/res/values/strings.xml
@@ -2141,8 +2141,6 @@
     <string name="wifi_do_not_provide_eap_user_cert">Do not provide</string>
     <!-- Menu option for not validating the EAP server -->
     <string name="wifi_do_not_validate_eap_server">Do not validate</string>
-    <!-- Warning message displayed if user choses not to validate the EAP server -->
-    <string name="wifi_do_not_validate_eap_server_warning">No certificate specified. Your connection will not be private.</string>
     <!-- Warning message displayed if network name (ssid) is too long -->
     <string name="wifi_ssid_too_long">Network name is too long.</string>
     <!-- Warning message displayed if user does not specify a domain for the CA certificate.
diff --git a/src/com/android/settings/wifi/WifiConfigController.java b/src/com/android/settings/wifi/WifiConfigController.java
index 45cc8e510ea..9b4ef6072cc 100644
--- a/src/com/android/settings/wifi/WifiConfigController.java
+++ b/src/com/android/settings/wifi/WifiConfigController.java
@@ -159,7 +159,6 @@ public class WifiConfigController implements TextWatcher,
     private String mMultipleCertSetString;
     private String mUseSystemCertsString;
     private String mDoNotProvideEapUserCertString;
-    private String mDoNotValidateEapServerString;
 
     private ScrollView mDialogContainer;
     private Spinner mSecuritySpinner;
@@ -264,8 +263,6 @@ public class WifiConfigController implements TextWatcher,
         mUseSystemCertsString = mContext.getString(R.string.wifi_use_system_certs);
         mDoNotProvideEapUserCertString =
             mContext.getString(R.string.wifi_do_not_provide_eap_user_cert);
-        mDoNotValidateEapServerString =
-            mContext.getString(R.string.wifi_do_not_validate_eap_server);
 
         mSsidScanButton = (ImageButton) mView.findViewById(R.id.ssid_scanner_button);
         mDialogContainer = mView.findViewById(R.id.dialog_scrollview);
@@ -554,8 +551,7 @@ public class WifiConfigController implements TextWatcher,
                 // Disallow submit if the user has not selected a CA certificate for an EAP network
                 // configuration.
                 enabled = false;
-            } else if (!caCertSelection.equals(mDoNotValidateEapServerString)
-                    && mEapDomainView != null
+            } else if (mEapDomainView != null
                     && mView.findViewById(R.id.l_domain).getVisibility() != View.GONE
                     && TextUtils.isEmpty(mEapDomainView.getText().toString())) {
                 // Disallow submit if the user chooses to use a certificate for EAP server
@@ -576,7 +572,6 @@ public class WifiConfigController implements TextWatcher,
     }
 
     void showWarningMessagesIfAppropriate() {
-        mView.findViewById(R.id.no_ca_cert_warning).setVisibility(View.GONE);
         mView.findViewById(R.id.no_user_cert_warning).setVisibility(View.GONE);
         mView.findViewById(R.id.no_domain_warning).setVisibility(View.GONE);
         mView.findViewById(R.id.ssid_too_long_warning).setVisibility(View.GONE);
@@ -589,13 +584,7 @@ public class WifiConfigController implements TextWatcher,
         }
         if (mEapCaCertSpinner != null
                 && mView.findViewById(R.id.l_ca_cert).getVisibility() != View.GONE) {
-            String caCertSelection = (String) mEapCaCertSpinner.getSelectedItem();
-            if (caCertSelection.equals(mDoNotValidateEapServerString)) {
-                // Display warning if user chooses not to validate the EAP server with a
-                // user-supplied CA certificate in an EAP network configuration.
-                mView.findViewById(R.id.no_ca_cert_warning).setVisibility(View.VISIBLE);
-            } else if (!caCertSelection.equals(mUnspecifiedCertString)
-                    && mEapDomainView != null
+            if (mEapDomainView != null
                     && mView.findViewById(R.id.l_domain).getVisibility() != View.GONE
                     && TextUtils.isEmpty(mEapDomainView.getText().toString())) {
                 // Display warning if user chooses to use a certificate without restricting the
@@ -733,8 +722,7 @@ public class WifiConfigController implements TextWatcher,
                 config.enterpriseConfig.setCaCertificateAliases(null);
                 config.enterpriseConfig.setCaPath(null);
                 config.enterpriseConfig.setDomainSuffixMatch(mEapDomainView.getText().toString());
-                if (caCert.equals(mUnspecifiedCertString)
-                        || caCert.equals(mDoNotValidateEapServerString)) {
+                if (caCert.equals(mUnspecifiedCertString)) {
                     // ca_cert already set to null, so do nothing.
                 } else if (caCert.equals(mUseSystemCertsString)) {
                     config.enterpriseConfig.setCaPath(SYSTEM_CA_STORE_PATH);
@@ -768,8 +756,7 @@ public class WifiConfigController implements TextWatcher,
                 }
 
                 // Only set OCSP option if there is a valid CA certificate.
-                if (caCert.equals(mUnspecifiedCertString)
-                        || caCert.equals(mDoNotValidateEapServerString)) {
+                if (caCert.equals(mUnspecifiedCertString)) {
                     config.enterpriseConfig.setOcsp(WifiEnterpriseConfig.OCSP_NONE);
                 } else {
                     config.enterpriseConfig.setOcsp(mEapOcspSpinner.getSelectedItemPosition());
@@ -1049,15 +1036,15 @@ public class WifiConfigController implements TextWatcher,
             loadCertificates(
                     mEapCaCertSpinner,
                     Credentials.CA_CERTIFICATE,
-                    mDoNotValidateEapServerString,
-                    false,
-                    true);
+                    null /* noCertificateString */,
+                    false /* showMultipleCerts */,
+                    true /* showUsePreinstalledCertOption */);
             loadCertificates(
                     mEapUserCertSpinner,
                     Credentials.USER_PRIVATE_KEY,
                     mDoNotProvideEapUserCertString,
-                    false,
-                    false);
+                    false /* showMultipleCerts */,
+                    false /* showUsePreinstalledCertOption */);
             // To avoid the user connects to a non-secure network unexpectedly,
             // request using system trusted certificates by default
             // unless the user explicitly chooses "Do not validate" or other
@@ -1122,7 +1109,7 @@ public class WifiConfigController implements TextWatcher,
             } else {
                 String[] caCerts = enterpriseConfig.getCaCertificateAliases();
                 if (caCerts == null) {
-                    setSelection(mEapCaCertSpinner, mDoNotValidateEapServerString);
+                    setSelection(mEapCaCertSpinner, mUnspecifiedCertString);
                 } else if (caCerts.length == 1) {
                     setSelection(mEapCaCertSpinner, caCerts[0]);
                 } else {
@@ -1130,9 +1117,9 @@ public class WifiConfigController implements TextWatcher,
                     loadCertificates(
                             mEapCaCertSpinner,
                             Credentials.CA_CERTIFICATE,
-                            mDoNotValidateEapServerString,
-                            true,
-                            true);
+                            null /* noCertificateString */,
+                            true /* showMultipleCerts */,
+                            true /* showUsePreinstalledCertOption */);
                     setSelection(mEapCaCertSpinner, mMultipleCertSetString);
                 }
             }
@@ -1259,8 +1246,7 @@ public class WifiConfigController implements TextWatcher,
 
         if (mView.findViewById(R.id.l_ca_cert).getVisibility() != View.GONE) {
             String eapCertSelection = (String) mEapCaCertSpinner.getSelectedItem();
-            if (eapCertSelection.equals(mDoNotValidateEapServerString)
-                    || eapCertSelection.equals(mUnspecifiedCertString)) {
+            if (eapCertSelection.equals(mUnspecifiedCertString)) {
                 // Domain suffix matching is not relevant if the user hasn't chosen a CA
                 // certificate yet, or chooses not to validate the EAP server.
                 setDomainInvisible();
@@ -1482,7 +1468,8 @@ public class WifiConfigController implements TextWatcher,
                     }).collect(Collectors.toList()));
         }
 
-        if (mAccessPointSecurity != AccessPoint.SECURITY_EAP_SUITE_B) {
+        if (!TextUtils.isEmpty(noCertificateString)
+                && mAccessPointSecurity != AccessPoint.SECURITY_EAP_SUITE_B) {
             certs.add(noCertificateString);
         }
 
diff --git a/src/com/android/settings/wifi/WifiConfigController2.java b/src/com/android/settings/wifi/WifiConfigController2.java
index d80873fc40d..576ffa2f3d2 100644
--- a/src/com/android/settings/wifi/WifiConfigController2.java
+++ b/src/com/android/settings/wifi/WifiConfigController2.java
@@ -156,7 +156,6 @@ public class WifiConfigController2 implements TextWatcher,
     private String mMultipleCertSetString;
     private String mUseSystemCertsString;
     private String mDoNotProvideEapUserCertString;
-    private String mDoNotValidateEapServerString;
 
     private ScrollView mDialogContainer;
     private Spinner mSecuritySpinner;
@@ -253,8 +252,6 @@ public class WifiConfigController2 implements TextWatcher,
         mUseSystemCertsString = mContext.getString(R.string.wifi_use_system_certs);
         mDoNotProvideEapUserCertString =
             mContext.getString(R.string.wifi_do_not_provide_eap_user_cert);
-        mDoNotValidateEapServerString =
-            mContext.getString(R.string.wifi_do_not_validate_eap_server);
 
         mSsidScanButton = (ImageButton) mView.findViewById(R.id.ssid_scanner_button);
         mDialogContainer = mView.findViewById(R.id.dialog_scrollview);
@@ -533,8 +530,7 @@ public class WifiConfigController2 implements TextWatcher,
                 // Disallow submit if the user has not selected a CA certificate for an EAP network
                 // configuration.
                 enabled = false;
-            } else if (!caCertSelection.equals(mDoNotValidateEapServerString)
-                    && mEapDomainView != null
+            } else if (mEapDomainView != null
                     && mView.findViewById(R.id.l_domain).getVisibility() != View.GONE
                     && TextUtils.isEmpty(mEapDomainView.getText().toString())) {
                 // Disallow submit if the user chooses to use a certificate for EAP server
@@ -555,7 +551,6 @@ public class WifiConfigController2 implements TextWatcher,
     }
 
     void showWarningMessagesIfAppropriate() {
-        mView.findViewById(R.id.no_ca_cert_warning).setVisibility(View.GONE);
         mView.findViewById(R.id.no_user_cert_warning).setVisibility(View.GONE);
         mView.findViewById(R.id.no_domain_warning).setVisibility(View.GONE);
         mView.findViewById(R.id.ssid_too_long_warning).setVisibility(View.GONE);
@@ -568,13 +563,7 @@ public class WifiConfigController2 implements TextWatcher,
         }
         if (mEapCaCertSpinner != null
                 && mView.findViewById(R.id.l_ca_cert).getVisibility() != View.GONE) {
-            String caCertSelection = (String) mEapCaCertSpinner.getSelectedItem();
-            if (caCertSelection.equals(mDoNotValidateEapServerString)) {
-                // Display warning if user chooses not to validate the EAP server with a
-                // user-supplied CA certificate in an EAP network configuration.
-                mView.findViewById(R.id.no_ca_cert_warning).setVisibility(View.VISIBLE);
-            } else if (!caCertSelection.equals(mUnspecifiedCertString)
-                    && mEapDomainView != null
+            if (mEapDomainView != null
                     && mView.findViewById(R.id.l_domain).getVisibility() != View.GONE
                     && TextUtils.isEmpty(mEapDomainView.getText().toString())) {
                 // Display warning if user chooses to use a certificate without restricting the
@@ -715,8 +704,7 @@ public class WifiConfigController2 implements TextWatcher,
                 config.enterpriseConfig.setCaCertificateAliases(null);
                 config.enterpriseConfig.setCaPath(null);
                 config.enterpriseConfig.setDomainSuffixMatch(mEapDomainView.getText().toString());
-                if (caCert.equals(mUnspecifiedCertString)
-                        || caCert.equals(mDoNotValidateEapServerString)) {
+                if (caCert.equals(mUnspecifiedCertString)) {
                     // ca_cert already set to null, so do nothing.
                 } else if (caCert.equals(mUseSystemCertsString)) {
                     config.enterpriseConfig.setCaPath(SYSTEM_CA_STORE_PATH);
@@ -749,8 +737,7 @@ public class WifiConfigController2 implements TextWatcher,
                 }
 
                 // Only set OCSP option if there is a valid CA certificate.
-                if (caCert.equals(mUnspecifiedCertString)
-                        || caCert.equals(mDoNotValidateEapServerString)) {
+                if (caCert.equals(mUnspecifiedCertString)) {
                     config.enterpriseConfig.setOcsp(WifiEnterpriseConfig.OCSP_NONE);
                 } else {
                     config.enterpriseConfig.setOcsp(mEapOcspSpinner.getSelectedItemPosition());
@@ -1031,15 +1018,15 @@ public class WifiConfigController2 implements TextWatcher,
             loadCertificates(
                     mEapCaCertSpinner,
                     Credentials.CA_CERTIFICATE,
-                    mDoNotValidateEapServerString,
-                    false,
-                    true);
+                    null /* noCertificateString */,
+                    false /* showMultipleCerts */,
+                    true /* showUsePreinstalledCertOption */);
             loadCertificates(
                     mEapUserCertSpinner,
                     Credentials.USER_PRIVATE_KEY,
                     mDoNotProvideEapUserCertString,
-                    false,
-                    false);
+                    false /* showMultipleCerts */,
+                    false /* showUsePreinstalledCertOption */);
             // To avoid the user connects to a non-secure network unexpectedly,
             // request using system trusted certificates by default
             // unless the user explicitly chooses "Do not validate" or other
@@ -1105,7 +1092,7 @@ public class WifiConfigController2 implements TextWatcher,
             } else {
                 String[] caCerts = enterpriseConfig.getCaCertificateAliases();
                 if (caCerts == null) {
-                    setSelection(mEapCaCertSpinner, mDoNotValidateEapServerString);
+                    setSelection(mEapCaCertSpinner, mUnspecifiedCertString);
                 } else if (caCerts.length == 1) {
                     setSelection(mEapCaCertSpinner, caCerts[0]);
                 } else {
@@ -1113,9 +1100,9 @@ public class WifiConfigController2 implements TextWatcher,
                     loadCertificates(
                             mEapCaCertSpinner,
                             Credentials.CA_CERTIFICATE,
-                            mDoNotValidateEapServerString,
-                            true,
-                            true);
+                            null /* noCertificateString */,
+                            true /* showMultipleCerts */,
+                            true /* showUsePreinstalledCertOption */);
                     setSelection(mEapCaCertSpinner, mMultipleCertSetString);
                 }
             }
@@ -1242,8 +1229,7 @@ public class WifiConfigController2 implements TextWatcher,
 
         if (mView.findViewById(R.id.l_ca_cert).getVisibility() != View.GONE) {
             String eapCertSelection = (String) mEapCaCertSpinner.getSelectedItem();
-            if (eapCertSelection.equals(mDoNotValidateEapServerString)
-                    || eapCertSelection.equals(mUnspecifiedCertString)) {
+            if (eapCertSelection.equals(mUnspecifiedCertString)) {
                 // Domain suffix matching is not relevant if the user hasn't chosen a CA
                 // certificate yet, or chooses not to validate the EAP server.
                 setDomainInvisible();
@@ -1465,7 +1451,8 @@ public class WifiConfigController2 implements TextWatcher,
                     }).collect(Collectors.toList()));
         }
 
-        if (mWifiEntrySecurity != WifiEntry.SECURITY_EAP_SUITE_B) {
+        if (!TextUtils.isEmpty(noCertificateString)
+                && mWifiEntrySecurity != WifiEntry.SECURITY_EAP_SUITE_B) {
             certs.add(noCertificateString);
         }