libnm: don't check for valid passwords in NMSetting:verify()
authorThomas Haller <thaller@redhat.com>
Mon, 9 Mar 2015 11:52:28 +0000 (12:52 +0100)
committerThomas Haller <thaller@redhat.com>
Fri, 20 Mar 2015 12:01:04 +0000 (13:01 +0100)
We must never fail verification of a connection based on a password
because the password is re-requested during activation.

Otherwise, if the user enters an invalid password for a (previously)
valid connection, the connection becomes invalid. NetworkManager does
not expect or handle that requesting password can make a connection
invalid.
Invalid passwords should be treated as wrong passwords. Only a UI
(such as nm-connection-editor or nmcli) should validate passwords
against a certain scheme.

Note that there is need_secrets() which on the contrary must check for
valid passwords.

Error scenario:

  Connect to a WEP Wi-Fi, via `nmcli device wifi connect SSID`. The
  generated connection has wep-key-type=0 (UNKNOWN) and wep-key-flags=0.
  When trying to connect, NM will ask for secrets and set the wep-key0
  field. After that, verification can fail (e.g. if the password is longer
  then 64 chars).

libnm-core/nm-setting-adsl.c
libnm-core/nm-setting-cdma.c
libnm-core/nm-setting-gsm.c
libnm-core/nm-setting-wireless-security.c
libnm-util/nm-setting-adsl.c
libnm-util/nm-setting-cdma.c
libnm-util/nm-setting-gsm.c
libnm-util/nm-setting-wireless-security.c

index 00bcef5..2e71f8e 100644 (file)
@@ -199,15 +199,6 @@ verify (NMSetting *setting, NMConnection *connection, GError **error)
                return FALSE;
        }
 
-       if (priv->password && !strlen (priv->password)) {
-               g_set_error_literal (error,
-                                    NM_CONNECTION_ERROR,
-                                    NM_CONNECTION_ERROR_INVALID_PROPERTY,
-                                    _("property is empty"));
-               g_prefix_error (error, "%s.%s: ", NM_SETTING_ADSL_SETTING_NAME, NM_SETTING_ADSL_PASSWORD);
-               return FALSE;
-       }
-
        if (   !priv->protocol
            || (   strcmp (priv->protocol, NM_SETTING_ADSL_PROTOCOL_PPPOA)
                && strcmp (priv->protocol, NM_SETTING_ADSL_PROTOCOL_PPPOE)
@@ -242,7 +233,7 @@ need_secrets (NMSetting *setting)
        NMSettingAdslPrivate *priv = NM_SETTING_ADSL_GET_PRIVATE (setting);
        GPtrArray *secrets = NULL;
 
-       if (priv->password)
+       if (priv->password && *priv->password)
                return NULL;
 
        if (!(priv->password_flags & NM_SETTING_SECRET_FLAG_NOT_REQUIRED)) {
index ebbbda7..ecc387c 100644 (file)
@@ -160,15 +160,6 @@ verify (NMSetting *setting, NMConnection *connection, GError **error)
                return FALSE;
        }
 
-       if (priv->password && !strlen (priv->password)) {
-               g_set_error_literal (error,
-                                    NM_CONNECTION_ERROR,
-                                    NM_CONNECTION_ERROR_INVALID_PROPERTY,
-                                    _("property is empty"));
-               g_prefix_error (error, "%s.%s: ", NM_SETTING_CDMA_SETTING_NAME, NM_SETTING_CDMA_PASSWORD);
-               return FALSE;
-       }
-
        return TRUE;
 }
 
@@ -178,7 +169,7 @@ need_secrets (NMSetting *setting)
        NMSettingCdmaPrivate *priv = NM_SETTING_CDMA_GET_PRIVATE (setting);
        GPtrArray *secrets = NULL;
 
-       if (priv->password)
+       if (priv->password && *priv->password)
                return NULL;
 
        if (priv->username) {
index fb76d18..36557a2 100644 (file)
@@ -285,15 +285,6 @@ verify (NMSetting *setting, NMConnection *connection, GError **error)
                return FALSE;
        }
 
-       if (priv->password && !strlen (priv->password)) {
-               g_set_error_literal (error,
-                                    NM_CONNECTION_ERROR,
-                                    NM_CONNECTION_ERROR_INVALID_PROPERTY,
-                                    _("property is empty"));
-               g_prefix_error (error, "%s.%s: ", NM_SETTING_GSM_SETTING_NAME, NM_SETTING_GSM_PASSWORD);
-               return FALSE;
-       }
-
        if (priv->network_id) {
                guint32 nid_len = strlen (priv->network_id);
                guint32 i;
@@ -331,7 +322,7 @@ need_secrets (NMSetting *setting)
        NMSettingGsmPrivate *priv = NM_SETTING_GSM_GET_PRIVATE (setting);
        GPtrArray *secrets = NULL;
 
-       if (priv->password)
+       if (priv->password && *priv->password)
                return NULL;
 
        if (priv->username) {
index c95f924..a56a8e9 100644 (file)
@@ -822,7 +822,7 @@ need_secrets (NMSetting *setting)
        if (   priv->auth_alg
            && !strcmp (priv->auth_alg, "leap")
            && !strcmp (priv->key_mgmt, "ieee8021x")) {
-               if (!priv->leap_password || !strlen (priv->leap_password)) {
+               if (!priv->leap_password || !*priv->leap_password) {
                        g_ptr_array_add (secrets, NM_SETTING_WIRELESS_SECURITY_LEAP_PASSWORD);
                        return secrets;
                }
@@ -893,14 +893,6 @@ verify (NMSetting *setting, NMConnection *connection, GError **error)
                        g_prefix_error (error, "%s.%s: ", NM_SETTING_WIRELESS_SECURITY_SETTING_NAME, NM_SETTING_WIRELESS_SECURITY_LEAP_USERNAME);
                        return FALSE;
                }
-               if (priv->leap_password && !strlen (priv->leap_password)) {
-                       g_set_error_literal (error,
-                                            NM_CONNECTION_ERROR,
-                                            NM_CONNECTION_ERROR_MISSING_PROPERTY,
-                                            _("property is empty"));
-                       g_prefix_error (error, "%s.%s: ", NM_SETTING_WIRELESS_SECURITY_SETTING_NAME, NM_SETTING_WIRELESS_SECURITY_LEAP_PASSWORD);
-                       return FALSE;
-               }
        } else {
                if (   (strcmp (priv->key_mgmt, "ieee8021x") == 0)
                    || (strcmp (priv->key_mgmt, "wpa-eap") == 0)) {
@@ -945,39 +937,6 @@ verify (NMSetting *setting, NMConnection *connection, GError **error)
                return FALSE;
        }
 
-       if (priv->wep_key0 && !nm_utils_wep_key_valid (priv->wep_key0, priv->wep_key_type)) {
-               g_set_error_literal (error,
-                                    NM_CONNECTION_ERROR,
-                                    NM_CONNECTION_ERROR_INVALID_PROPERTY,
-                                    _("property is invalid"));
-               g_prefix_error (error, "%s.%s: ", NM_SETTING_WIRELESS_SECURITY_SETTING_NAME, NM_SETTING_WIRELESS_SECURITY_WEP_KEY0);
-               return FALSE;
-       }
-       if (priv->wep_key1 && !nm_utils_wep_key_valid (priv->wep_key1, priv->wep_key_type)) {
-               g_set_error_literal (error,
-                                    NM_CONNECTION_ERROR,
-                                    NM_CONNECTION_ERROR_INVALID_PROPERTY,
-                                    _("property is invalid"));
-               g_prefix_error (error, "%s.%s: ", NM_SETTING_WIRELESS_SECURITY_SETTING_NAME, NM_SETTING_WIRELESS_SECURITY_WEP_KEY1);
-               return FALSE;
-       }
-       if (priv->wep_key2 && !nm_utils_wep_key_valid (priv->wep_key2, priv->wep_key_type)) {
-               g_set_error_literal (error,
-                                    NM_CONNECTION_ERROR,
-                                    NM_CONNECTION_ERROR_INVALID_PROPERTY,
-                                    _("property is invalid"));
-               g_prefix_error (error, "%s.%s: ", NM_SETTING_WIRELESS_SECURITY_SETTING_NAME, NM_SETTING_WIRELESS_SECURITY_WEP_KEY2);
-               return FALSE;
-       }
-       if (priv->wep_key3 && !nm_utils_wep_key_valid (priv->wep_key3, priv->wep_key_type)) {
-               g_set_error_literal (error,
-                                    NM_CONNECTION_ERROR,
-                                    NM_CONNECTION_ERROR_INVALID_PROPERTY,
-                                    _("property is invalid"));
-               g_prefix_error (error, "%s.%s: ", NM_SETTING_WIRELESS_SECURITY_SETTING_NAME, NM_SETTING_WIRELESS_SECURITY_WEP_KEY3);
-               return FALSE;
-       }
-
        if (priv->auth_alg && !_nm_utils_string_in_list (priv->auth_alg, valid_auth_algs)) {
                g_set_error_literal (error,
                                     NM_CONNECTION_ERROR,
@@ -987,15 +946,6 @@ verify (NMSetting *setting, NMConnection *connection, GError **error)
                return FALSE;
        }
 
-       if (priv->psk && !nm_utils_wpa_psk_valid (priv->psk)) {
-               g_set_error_literal (error,
-                                    NM_CONNECTION_ERROR,
-                                    NM_CONNECTION_ERROR_INVALID_PROPERTY,
-                                    _("property is invalid"));
-               g_prefix_error (error, "%s.%s: ", NM_SETTING_WIRELESS_SECURITY_SETTING_NAME, NM_SETTING_WIRELESS_SECURITY_PSK);
-               return FALSE;
-       }
-
        if (priv->proto && !_nm_utils_string_slist_validate (priv->proto, valid_protos)) {
                g_set_error_literal (error,
                                     NM_CONNECTION_ERROR,
index 601ebc2..5355011 100644 (file)
@@ -219,15 +219,6 @@ verify (NMSetting *setting, GSList *all_settings, GError **error)
                return FALSE;
        }
 
-       if (priv->password && !strlen (priv->password)) {
-               g_set_error_literal (error,
-                                    NM_SETTING_ADSL_ERROR,
-                                    NM_SETTING_ADSL_ERROR_INVALID_PROPERTY,
-                                    _("property is empty"));
-               g_prefix_error (error, "%s.%s: ", NM_SETTING_ADSL_SETTING_NAME, NM_SETTING_ADSL_PASSWORD);
-               return FALSE;
-       }
-
        if (   !priv->protocol
            || (   strcmp (priv->protocol, NM_SETTING_ADSL_PROTOCOL_PPPOA)
                && strcmp (priv->protocol, NM_SETTING_ADSL_PROTOCOL_PPPOE)
@@ -262,7 +253,7 @@ need_secrets (NMSetting *setting)
        NMSettingAdslPrivate *priv = NM_SETTING_ADSL_GET_PRIVATE (setting);
        GPtrArray *secrets = NULL;
 
-       if (priv->password)
+       if (priv->password && *priv->password)
                return NULL;
 
        if (!(priv->password_flags & NM_SETTING_SECRET_FLAG_NOT_REQUIRED)) {
index 44a893c..53f886e 100644 (file)
@@ -181,15 +181,6 @@ verify (NMSetting *setting, GSList *all_settings, GError **error)
                return FALSE;
        }
 
-       if (priv->password && !strlen (priv->password)) {
-               g_set_error_literal (error,
-                                    NM_SETTING_CDMA_ERROR,
-                                    NM_SETTING_CDMA_ERROR_INVALID_PROPERTY,
-                                    _("property is empty"));
-               g_prefix_error (error, "%s.%s: ", NM_SETTING_CDMA_SETTING_NAME, NM_SETTING_CDMA_PASSWORD);
-               return FALSE;
-       }
-
        return TRUE;
 }
 
@@ -199,7 +190,7 @@ need_secrets (NMSetting *setting)
        NMSettingCdmaPrivate *priv = NM_SETTING_CDMA_GET_PRIVATE (setting);
        GPtrArray *secrets = NULL;
 
-       if (priv->password)
+       if (priv->password && *priv->password)
                return NULL;
 
        if (priv->username) {
index 95c7a10..e422c92 100644 (file)
@@ -342,15 +342,6 @@ verify (NMSetting *setting, GSList *all_settings, GError **error)
                return FALSE;
        }
 
-       if (priv->password && !strlen (priv->password)) {
-               g_set_error_literal (error,
-                                    NM_SETTING_GSM_ERROR,
-                                    NM_SETTING_GSM_ERROR_INVALID_PROPERTY,
-                                    _("property is empty"));
-               g_prefix_error (error, "%s.%s: ", NM_SETTING_GSM_SETTING_NAME, NM_SETTING_GSM_PASSWORD);
-               return FALSE;
-       }
-
        if (priv->network_id) {
                guint32 nid_len = strlen (priv->network_id);
                guint32 i;
@@ -388,7 +379,7 @@ need_secrets (NMSetting *setting)
        NMSettingGsmPrivate *priv = NM_SETTING_GSM_GET_PRIVATE (setting);
        GPtrArray *secrets = NULL;
 
-       if (priv->password)
+       if (priv->password && *priv->password)
                return NULL;
 
        if (priv->username) {
index 5e3456c..574ff3b 100644 (file)
@@ -852,7 +852,7 @@ need_secrets (NMSetting *setting)
        if (   priv->auth_alg
            && !strcmp (priv->auth_alg, "leap")
            && !strcmp (priv->key_mgmt, "ieee8021x")) {
-               if (!priv->leap_password || !strlen (priv->leap_password)) {
+               if (!priv->leap_password || !*priv->leap_password) {
                        g_ptr_array_add (secrets, NM_SETTING_WIRELESS_SECURITY_LEAP_PASSWORD);
                        return secrets;
                }
@@ -923,14 +923,6 @@ verify (NMSetting *setting, GSList *all_settings, GError **error)
                        g_prefix_error (error, "%s.%s: ", NM_SETTING_WIRELESS_SECURITY_SETTING_NAME, NM_SETTING_WIRELESS_SECURITY_LEAP_USERNAME);
                        return FALSE;
                }
-               if (priv->leap_password && !strlen (priv->leap_password)) {
-                       g_set_error_literal (error,
-                                            NM_SETTING_WIRELESS_SECURITY_ERROR,
-                                            NM_SETTING_WIRELESS_SECURITY_ERROR_INVALID_PROPERTY,
-                                            _("property is empty"));
-                       g_prefix_error (error, "%s.%s: ", NM_SETTING_WIRELESS_SECURITY_SETTING_NAME, NM_SETTING_WIRELESS_SECURITY_LEAP_PASSWORD);
-                       return FALSE;
-               }
        } else {
                if (   (strcmp (priv->key_mgmt, "ieee8021x") == 0)
                    || (strcmp (priv->key_mgmt, "wpa-eap") == 0)) {
@@ -975,39 +967,6 @@ verify (NMSetting *setting, GSList *all_settings, GError **error)
                return FALSE;
        }
 
-       if (priv->wep_key0 && !nm_utils_wep_key_valid (priv->wep_key0, priv->wep_key_type)) {
-               g_set_error_literal (error,
-                                    NM_SETTING_WIRELESS_SECURITY_ERROR,
-                                    NM_SETTING_WIRELESS_SECURITY_ERROR_INVALID_PROPERTY,
-                                    _("property is invalid"));
-               g_prefix_error (error, "%s.%s: ", NM_SETTING_WIRELESS_SECURITY_SETTING_NAME, NM_SETTING_WIRELESS_SECURITY_WEP_KEY0);
-               return FALSE;
-       }
-       if (priv->wep_key1 && !nm_utils_wep_key_valid (priv->wep_key1, priv->wep_key_type)) {
-               g_set_error_literal (error,
-                                    NM_SETTING_WIRELESS_SECURITY_ERROR,
-                                    NM_SETTING_WIRELESS_SECURITY_ERROR_INVALID_PROPERTY,
-                                    _("property is invalid"));
-               g_prefix_error (error, "%s.%s: ", NM_SETTING_WIRELESS_SECURITY_SETTING_NAME, NM_SETTING_WIRELESS_SECURITY_WEP_KEY1);
-               return FALSE;
-       }
-       if (priv->wep_key2 && !nm_utils_wep_key_valid (priv->wep_key2, priv->wep_key_type)) {
-               g_set_error_literal (error,
-                                    NM_SETTING_WIRELESS_SECURITY_ERROR,
-                                    NM_SETTING_WIRELESS_SECURITY_ERROR_INVALID_PROPERTY,
-                                    _("property is invalid"));
-               g_prefix_error (error, "%s.%s: ", NM_SETTING_WIRELESS_SECURITY_SETTING_NAME, NM_SETTING_WIRELESS_SECURITY_WEP_KEY2);
-               return FALSE;
-       }
-       if (priv->wep_key3 && !nm_utils_wep_key_valid (priv->wep_key3, priv->wep_key_type)) {
-               g_set_error_literal (error,
-                                    NM_SETTING_WIRELESS_SECURITY_ERROR,
-                                    NM_SETTING_WIRELESS_SECURITY_ERROR_INVALID_PROPERTY,
-                                    _("property is invalid"));
-               g_prefix_error (error, "%s.%s: ", NM_SETTING_WIRELESS_SECURITY_SETTING_NAME, NM_SETTING_WIRELESS_SECURITY_WEP_KEY3);
-               return FALSE;
-       }
-
        if (priv->auth_alg && !_nm_utils_string_in_list (priv->auth_alg, valid_auth_algs)) {
                g_set_error_literal (error,
                                     NM_SETTING_WIRELESS_SECURITY_ERROR,
@@ -1017,15 +976,6 @@ verify (NMSetting *setting, GSList *all_settings, GError **error)
                return FALSE;
        }
 
-       if (priv->psk && !nm_utils_wpa_psk_valid (priv->psk)) {
-               g_set_error_literal (error,
-                                    NM_SETTING_WIRELESS_SECURITY_ERROR,
-                                    NM_SETTING_WIRELESS_SECURITY_ERROR_INVALID_PROPERTY,
-                                    _("property is invalid"));
-               g_prefix_error (error, "%s.%s: ", NM_SETTING_WIRELESS_SECURITY_SETTING_NAME, NM_SETTING_WIRELESS_SECURITY_PSK);
-               return FALSE;
-       }
-
        if (priv->proto && !_nm_utils_string_slist_validate (priv->proto, valid_protos)) {
                g_set_error_literal (error,
                                     NM_SETTING_WIRELESS_SECURITY_ERROR,