From e62e52dfe11c2ae3d5074f92d83c3daa8f6aa114 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 24 Oct 2017 11:11:18 +0200 Subject: [PATCH] device: handle authentication retries using 802-1x.auth-retries setting Since commit 4a6fd0e83ec0d83547b1f3a1a916f85e9f450d8c (device: honor the connection.autoconnect-retries for 802.1X) and the related bug bgo#723084, we reuse the autoconnect-retries setting to control the retry count for requesting passwords. I think that is wrong. These are two different settings, we should not reuse the autoconnect retry counter while the device is still active. For example, the user might wish to set autoconnect-retries to infinity (zero). In that case, we would retry indefinitly to request a password. That could be problematic, if there is a different issue with the connection, that makes it appear tha the password is wrong. A full re-activation might succeed, but we would never stop retrying to authenticate. Instead, we should have two different settings for retrying to authenticate and to autoconnect. This is a change in behavior compared to 1.8. --- src/devices/nm-device-ethernet.c | 22 ++++++--- src/devices/nm-device-macsec.c | 27 +++++++---- src/devices/nm-device.c | 69 ++++++++++++++++++--------- src/devices/nm-device.h | 8 +++- src/settings/nm-settings-connection.c | 13 ----- src/settings/nm-settings-connection.h | 1 - 6 files changed, 86 insertions(+), 54 deletions(-) diff --git a/src/devices/nm-device-ethernet.c b/src/devices/nm-device-ethernet.c index bb71cf6222..672ae3cd52 100644 --- a/src/devices/nm-device-ethernet.c +++ b/src/devices/nm-device-ethernet.c @@ -115,6 +115,8 @@ typedef struct _NMDeviceEthernetPrivate { DcbWait dcb_wait; guint dcb_timeout_id; + int auth_retries; + bool dcb_handle_carrier_changes:1; } NMDeviceEthernetPrivate; @@ -260,14 +262,18 @@ device_state_changed (NMDevice *device, NMDeviceState old_state, NMDeviceStateReason reason) { + NMDeviceEthernetPrivate *priv; + if (new_state > NM_DEVICE_STATE_ACTIVATED) wired_secrets_cancel (NM_DEVICE_ETHERNET (device)); if (NM_IN_SET (new_state, NM_DEVICE_STATE_ACTIVATED, NM_DEVICE_STATE_FAILED, - NM_DEVICE_STATE_DISCONNECTED)) - nm_device_autoconnect_retries_reset (device, NM_TYPE_SETTING_802_1X); + NM_DEVICE_STATE_DISCONNECTED)) { + priv = NM_DEVICE_ETHERNET_GET_PRIVATE (NM_DEVICE_ETHERNET (device)); + priv->auth_retries = NM_DEVICE_802_1X_AUTH_RETRIES_UNSET; + } } static void @@ -278,6 +284,7 @@ nm_device_ethernet_init (NMDeviceEthernet *self) priv = G_TYPE_INSTANCE_GET_PRIVATE (self, NM_TYPE_DEVICE_ETHERNET, NMDeviceEthernetPrivate); self->_priv = priv; + priv->auth_retries = NM_DEVICE_802_1X_AUTH_RETRIES_UNSET; priv->s390_options = g_hash_table_new_full (nm_str_hash, g_str_equal, g_free, g_free); } @@ -664,20 +671,21 @@ handle_auth_or_fail (NMDeviceEthernet *self, NMActRequest *req, gboolean new_secrets) { + NMDeviceEthernetPrivate *priv; const char *setting_name; NMConnection *applied_connection; - NMSettingsConnection *settings_connection; - applied_connection = nm_act_request_get_applied_connection (req); - settings_connection = nm_act_request_get_settings_connection (req); + priv = NM_DEVICE_ETHERNET_GET_PRIVATE (self); - if (!nm_settings_connection_autoconnect_retries_try_next (settings_connection)) + if (!nm_device_802_1x_auth_retries_try_next (NM_DEVICE (self), + &priv->auth_retries)) return NM_ACT_STAGE_RETURN_FAILURE; nm_device_state_changed (NM_DEVICE (self), NM_DEVICE_STATE_NEED_AUTH, NM_DEVICE_STATE_REASON_NONE); nm_active_connection_clear_secrets (NM_ACTIVE_CONNECTION (req)); + applied_connection = nm_act_request_get_applied_connection (req); setting_name = nm_connection_need_secrets (applied_connection, NULL); if (setting_name) { wired_secrets_get_secrets (self, setting_name, @@ -1336,7 +1344,7 @@ deactivate (NMDevice *device) GError *error = NULL; /* Clear wired secrets tries when deactivating */ - nm_device_autoconnect_retries_reset (device, NM_TYPE_SETTING_802_1X); + priv->auth_retries = NM_DEVICE_802_1X_AUTH_RETRIES_UNSET; nm_clear_g_source (&priv->pppoe_wait_id); diff --git a/src/devices/nm-device-macsec.c b/src/devices/nm-device-macsec.c index bd081a9108..d8e2cc9dbb 100644 --- a/src/devices/nm-device-macsec.c +++ b/src/devices/nm-device-macsec.c @@ -72,6 +72,7 @@ typedef struct { Supplicant supplicant; guint supplicant_timeout_id; NMActRequestGetSecretsCallId macsec_secrets_id; + int auth_retries; } NMDeviceMacsecPrivate; struct _NMDeviceMacsec { @@ -477,20 +478,21 @@ handle_auth_or_fail (NMDeviceMacsec *self, NMActRequest *req, gboolean new_secrets) { + NMDeviceMacsecPrivate *priv; const char *setting_name; NMConnection *applied_connection; - NMSettingsConnection *settings_connection; - applied_connection = nm_act_request_get_applied_connection (req); - settings_connection = nm_act_request_get_settings_connection (req); + priv = NM_DEVICE_MACSEC_GET_PRIVATE (self); - if (!nm_settings_connection_autoconnect_retries_try_next (settings_connection)) + if (!nm_device_802_1x_auth_retries_try_next (NM_DEVICE (self), + &priv->auth_retries)) return NM_ACT_STAGE_RETURN_FAILURE; nm_device_state_changed (NM_DEVICE (self), NM_DEVICE_STATE_NEED_AUTH, NM_DEVICE_STATE_REASON_NONE); nm_active_connection_clear_secrets (NM_ACTIVE_CONNECTION (req)); + applied_connection = nm_act_request_get_applied_connection (req); setting_name = nm_connection_need_secrets (applied_connection, NULL); if (setting_name) { macsec_secrets_get_secrets (self, setting_name, @@ -739,13 +741,17 @@ device_state_changed (NMDevice *device, NMDeviceState old_state, NMDeviceStateReason reason) { + NMDeviceMacsecPrivate *priv; + if (new_state > NM_DEVICE_STATE_ACTIVATED) macsec_secrets_cancel (NM_DEVICE_MACSEC (device)); - if ( new_state == NM_DEVICE_STATE_ACTIVATED - || new_state == NM_DEVICE_STATE_FAILED - || new_state == NM_DEVICE_STATE_DISCONNECTED) - nm_device_autoconnect_retries_reset (device, G_TYPE_NONE); + if (NM_IN_SET (new_state, NM_DEVICE_STATE_ACTIVATED, + NM_DEVICE_STATE_FAILED, + NM_DEVICE_STATE_DISCONNECTED)) { + priv = NM_DEVICE_MACSEC_GET_PRIVATE (NM_DEVICE_MACSEC (device)); + priv->auth_retries = NM_DEVICE_802_1X_AUTH_RETRIES_UNSET; + } } /******************************************************************/ @@ -802,8 +808,11 @@ get_property (GObject *object, guint prop_id, } static void -nm_device_macsec_init (NMDeviceMacsec * self) +nm_device_macsec_init (NMDeviceMacsec *self) { + NMDeviceMacsecPrivate *priv = NM_DEVICE_MACSEC_GET_PRIVATE (self); + + priv->auth_retries = NM_DEVICE_802_1X_AUTH_RETRIES_UNSET; } static void diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index a6ad278e6d..36cfc9c9af 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -4213,29 +4213,6 @@ nm_device_set_enabled (NMDevice *self, gboolean enabled) NM_DEVICE_GET_CLASS (self)->set_enabled (self, enabled); } -void -nm_device_autoconnect_retries_reset (NMDevice *device, GType required_applied_setting) -{ - NMActRequest *req; - NMSettingsConnection *connection; - - req = nm_device_get_act_request (device); - if (!req) - return; - - if ( !NM_IN_SET (required_applied_setting, G_TYPE_INVALID, G_TYPE_NONE) - && !nm_device_get_applied_setting (device, required_applied_setting)) { - /* if the setting doesn't have the required setting in the applied - * connection, we do nothing. */ - return; - } - - connection = nm_act_request_get_settings_connection (req); - - /* Reset autoconnect retries on success, failure, or when deactivating */ - nm_settings_connection_autoconnect_retries_reset (connection); -} - /** * nm_device_get_autoconnect: * @self: the #NMDevice @@ -14046,6 +14023,52 @@ nm_device_get_supplicant_timeout (NMDevice *self) SUPPLICANT_DEFAULT_TIMEOUT); } +gboolean +nm_device_802_1x_auth_retries_try_next (NMDevice *self, int *p_auth_retries) +{ + NMConnection *applied_connection; + NMSetting8021x *security; + int auth_retries = *p_auth_retries; + + if (G_UNLIKELY (auth_retries == NM_DEVICE_802_1X_AUTH_RETRIES_UNSET)) { + auth_retries = -1; + + applied_connection = nm_device_get_applied_connection (NM_DEVICE (self)); + if (applied_connection) { + security = nm_connection_get_setting_802_1x (applied_connection); + if (security) + auth_retries = nm_setting_802_1x_get_auth_retries (security); + } + + if (auth_retries == -1) { + gs_free char *value = NULL; + + value = nm_config_data_get_connection_default (NM_CONFIG_GET_DATA, + "802-1x.auth-retries", + self); + auth_retries = _nm_utils_ascii_str_to_int64 (value, 10, -1, G_MAXINT32, -1); + } + + if (auth_retries == 0) + auth_retries = NM_DEVICE_802_1X_AUTH_RETRIES_INFINITY; + else if (auth_retries == -1) + auth_retries = NM_DEVICE_802_1X_AUTH_RETRIES_DEFAULT; + else + nm_assert (auth_retries > 0); + + *p_auth_retries = auth_retries; + } + + if (auth_retries == NM_DEVICE_802_1X_AUTH_RETRIES_INFINITY) + return TRUE; + if (auth_retries <= 0) { + nm_assert (auth_retries == 0); + return FALSE; + } + (*p_auth_retries)--; + return TRUE; +} + /*****************************************************************************/ static const char * diff --git a/src/devices/nm-device.h b/src/devices/nm-device.h index aafa31d3af..6e4aa0d9e2 100644 --- a/src/devices/nm-device.h +++ b/src/devices/nm-device.h @@ -663,7 +663,6 @@ gboolean nm_device_unrealize (NMDevice *device, void nm_device_update_from_platform_link (NMDevice *self, const NMPlatformLink *plink); -void nm_device_autoconnect_retries_reset (NMDevice *device, GType required_applied_setting); gboolean nm_device_get_autoconnect (NMDevice *device); void nm_device_set_autoconnect_intern (NMDevice *device, gboolean autoconnect); void nm_device_emit_recheck_auto_activate (NMDevice *device); @@ -736,6 +735,13 @@ void nm_device_update_initial_hw_address (NMDevice *self); void nm_device_update_permanent_hw_address (NMDevice *self, gboolean force_freeze); void nm_device_update_dynamic_ip_setup (NMDevice *self); guint nm_device_get_supplicant_timeout (NMDevice *self); + +#define NM_DEVICE_802_1X_AUTH_RETRIES_UNSET -1 +#define NM_DEVICE_802_1X_AUTH_RETRIES_INFINITY -2 +#define NM_DEVICE_802_1X_AUTH_RETRIES_DEFAULT 3 + +gboolean nm_device_802_1x_auth_retries_try_next (NMDevice *self, int *p_auth_retry); + gboolean nm_device_hw_addr_get_cloned (NMDevice *self, NMConnection *connection, gboolean is_wifi, diff --git a/src/settings/nm-settings-connection.c b/src/settings/nm-settings-connection.c index 4ed69e16a2..a7b28792b2 100644 --- a/src/settings/nm-settings-connection.c +++ b/src/settings/nm-settings-connection.c @@ -2599,19 +2599,6 @@ nm_settings_connection_autoconnect_retries_reset (NMSettingsConnection *self) nm_settings_connection_autoconnect_retries_set (self, AUTOCONNECT_RETRIES_UNSET); } -gboolean -nm_settings_connection_autoconnect_retries_try_next (NMSettingsConnection *self) -{ - int tries; - - tries = nm_settings_connection_autoconnect_retries_get (self); - if (tries == 0) - return FALSE; - if (tries > 0) - nm_settings_connection_autoconnect_retries_set (self, tries - 1); - return TRUE; -} - gint32 nm_settings_connection_autoconnect_blocked_until_get (NMSettingsConnection *self) { diff --git a/src/settings/nm-settings-connection.h b/src/settings/nm-settings-connection.h index 69aa792305..a983bddd3d 100644 --- a/src/settings/nm-settings-connection.h +++ b/src/settings/nm-settings-connection.h @@ -217,7 +217,6 @@ int nm_settings_connection_autoconnect_retries_get (NMSettingsConnection *self); void nm_settings_connection_autoconnect_retries_set (NMSettingsConnection *self, int retries); void nm_settings_connection_autoconnect_retries_reset (NMSettingsConnection *self); -gboolean nm_settings_connection_autoconnect_retries_try_next (NMSettingsConnection *self); gint32 nm_settings_connection_autoconnect_blocked_until_get (NMSettingsConnection *self);