From d01ba607a6aca0076ceace1d943a1ef0d2641f44 Mon Sep 17 00:00:00 2001 From: Andrew Zaborowski Date: Fri, 15 Jun 2018 06:24:02 +0200 Subject: [PATCH 01/11] iwd: handle empty wireless mode as Infrastructure Blank mode property in the wireless settings is documented in libnm-core/nm-setting-wireless.c to mean infrastructure mode. --- src/devices/wifi/nm-device-iwd.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/devices/wifi/nm-device-iwd.c b/src/devices/wifi/nm-device-iwd.c index 579851851e..33ee955453 100644 --- a/src/devices/wifi/nm-device-iwd.c +++ b/src/devices/wifi/nm-device-iwd.c @@ -545,7 +545,7 @@ check_connection_compatible (NMDevice *device, NMConnection *connection) return FALSE; mode = nm_setting_wireless_get_mode (s_wireless); - if (g_strcmp0 (mode, NM_SETTING_WIRELESS_MODE_INFRA) != 0) + if (mode && g_strcmp0 (mode, NM_SETTING_WIRELESS_MODE_INFRA) != 0) return FALSE; /* 8021x networks can only be used if they've been provisioned on the IWD side and @@ -575,7 +575,7 @@ check_connection_available (NMDevice *device, /* Only Infrastrusture mode at this time */ mode = nm_setting_wireless_get_mode (s_wifi); - if (g_strcmp0 (mode, NM_SETTING_WIRELESS_MODE_INFRA) != 0) + if (mode && g_strcmp0 (mode, NM_SETTING_WIRELESS_MODE_INFRA) != 0) return FALSE; /* Hidden SSIDs not supported yet */ @@ -630,7 +630,7 @@ complete_connection (NMDevice *device, mode = s_wifi ? nm_setting_wireless_get_mode (s_wifi) : NULL; - if (s_wifi && !nm_streq0 (mode, NM_SETTING_WIRELESS_MODE_INFRA)) { + if (mode && !nm_streq0 (mode, NM_SETTING_WIRELESS_MODE_INFRA)) { g_set_error_literal (error, NM_DEVICE_ERROR, NM_DEVICE_ERROR_INVALID_CONNECTION, @@ -806,7 +806,7 @@ can_auto_connect (NMDevice *device, /* Only Infrastrusture mode */ mode = nm_setting_wireless_get_mode (s_wifi); - if (g_strcmp0 (mode, NM_SETTING_WIRELESS_MODE_INFRA) != 0) + if (mode && g_strcmp0 (mode, NM_SETTING_WIRELESS_MODE_INFRA) != 0) return FALSE; /* Don't autoconnect to networks that have been tried at least once From 0876332bae2afefa4d0fdfac014d7ab6e1d357a0 Mon Sep 17 00:00:00 2001 From: Andrew Zaborowski Date: Fri, 15 Jun 2018 06:30:35 +0200 Subject: [PATCH 02/11] iwd: in prepare stage check that matching AP exists Check the return value of nm_wifi_aps_find_first_compatible in act_stage1_prepare. Also a small formatting fix. --- src/devices/wifi/nm-device-iwd.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/devices/wifi/nm-device-iwd.c b/src/devices/wifi/nm-device-iwd.c index 33ee955453..2b04a9e5a0 100644 --- a/src/devices/wifi/nm-device-iwd.c +++ b/src/devices/wifi/nm-device-iwd.c @@ -1219,15 +1219,13 @@ act_stage1_prepare (NMDevice *device, NMDeviceStateReason *out_failure_reason) ap = ap_path ? nm_wifi_ap_lookup_for_device (NM_DEVICE (self), ap_path) : NULL; if (!ap) { ap = nm_wifi_aps_find_first_compatible (&priv->aps_lst_head, connection); - - /* TODO: assuming hidden networks aren't supported do we need - * to consider the case of APs that are not in the scan list - * yet, for which nm-device-wifi.c creates the temporary fake - * AP object? - */ + if (!ap) { + NM_SET_OUT (out_failure_reason, NM_DEVICE_STATE_REASON_CONFIG_FAILED); + return NM_ACT_STAGE_RETURN_FAILURE; + } nm_active_connection_set_specific_object (NM_ACTIVE_CONNECTION (req), - nm_dbus_object_get_path (NM_DBUS_OBJECT (ap))); + nm_dbus_object_get_path (NM_DBUS_OBJECT (ap))); } set_current_ap (self, ap, FALSE); From ffd96edf765f0d1ef8a9d7cc3ffdc407bea3cb65 Mon Sep 17 00:00:00 2001 From: Andrew Zaborowski Date: Thu, 21 Jun 2018 01:30:04 +0200 Subject: [PATCH 03/11] iwd: save secrets request invocation in request user_data To improve the code logic and reduce space for bugs, don't save the dbus invocation object as priv->secrets_request, instead move it to the nm_act_request_get_secrets()'s user_data as we only need the invocation object for exactly the life time of the request. See https://github.com/NetworkManager/NetworkManager/pull/139 for discussion. --- src/devices/wifi/nm-device-iwd.c | 47 ++++++++++++++------------------ 1 file changed, 20 insertions(+), 27 deletions(-) diff --git a/src/devices/wifi/nm-device-iwd.c b/src/devices/wifi/nm-device-iwd.c index 2b04a9e5a0..b447da4ccb 100644 --- a/src/devices/wifi/nm-device-iwd.c +++ b/src/devices/wifi/nm-device-iwd.c @@ -73,7 +73,6 @@ typedef struct { GCancellable * cancellable; NMDeviceWifiCapabilities capabilities; NMActRequestGetSecretsCallId *wifi_secrets_id; - GDBusMethodInvocation *secrets_request; guint periodic_scan_id; bool enabled:1; bool can_scan:1; @@ -379,14 +378,6 @@ wifi_secrets_cancel (NMDeviceIwd *self) if (priv->wifi_secrets_id) nm_act_request_cancel_secrets (NULL, priv->wifi_secrets_id); nm_assert (!priv->wifi_secrets_id); - - if (priv->secrets_request) { - g_dbus_method_invocation_return_error_literal (priv->secrets_request, NM_DEVICE_ERROR, - NM_DEVICE_ERROR_INVALID_CONNECTION, - "NM secrets request cancelled"); - priv->secrets_request = NULL; - } - } static void @@ -1008,25 +999,31 @@ wifi_secrets_cb (NMActRequest *req, GError *error, gpointer user_data) { - NMDevice *device = user_data; - NMDeviceIwd *self = user_data; + NMDeviceIwd *self; NMDeviceIwdPrivate *priv; + NMDevice *device; NMSettingWirelessSecurity *s_wireless_sec; const gchar *psk; + GDBusMethodInvocation *invocation; + + nm_utils_user_data_unpack (user_data, &self, &invocation); g_return_if_fail (NM_IS_DEVICE_IWD (self)); - g_return_if_fail (NM_IS_ACT_REQUEST (req)); priv = NM_DEVICE_IWD_GET_PRIVATE (self); + device = NM_DEVICE (self); g_return_if_fail (priv->wifi_secrets_id == call_id); priv->wifi_secrets_id = NULL; - if (g_error_matches (error, G_IO_ERROR, G_IO_ERROR_CANCELLED)) + if (g_error_matches (error, G_IO_ERROR, G_IO_ERROR_CANCELLED)) { + g_dbus_method_invocation_return_error_literal (invocation, NM_DEVICE_ERROR, + NM_DEVICE_ERROR_INVALID_CONNECTION, + "NM secrets request cancelled"); return; + } - g_return_if_fail (priv->secrets_request); g_return_if_fail (req == nm_device_get_act_request (device)); g_return_if_fail (nm_act_request_get_settings_connection (req) == s_connection); @@ -1049,21 +1046,17 @@ wifi_secrets_cb (NMActRequest *req, _LOGD (LOGD_DEVICE | LOGD_WIFI, "Returning a new PSK to the IWD Agent"); - g_dbus_method_invocation_return_value (priv->secrets_request, + g_dbus_method_invocation_return_value (invocation, g_variant_new ("(s)", psk)); - priv->secrets_request = NULL; /* Change state back to what it was before NEED_AUTH */ nm_device_state_changed (device, NM_DEVICE_STATE_CONFIG, NM_DEVICE_STATE_REASON_NONE); return; secrets_error: - if (priv->secrets_request) { - g_dbus_method_invocation_return_error_literal (priv->secrets_request, NM_DEVICE_ERROR, - NM_DEVICE_ERROR_INVALID_CONNECTION, - "NM secrets request failed"); - priv->secrets_request = NULL; - } + g_dbus_method_invocation_return_error_literal (invocation, NM_DEVICE_ERROR, + NM_DEVICE_ERROR_INVALID_CONNECTION, + "NM secrets request failed"); nm_device_state_changed (device, NM_DEVICE_STATE_FAILED, @@ -1075,7 +1068,8 @@ secrets_error: static void wifi_secrets_get_secrets (NMDeviceIwd *self, const char *setting_name, - NMSecretAgentGetSecretsFlags flags) + NMSecretAgentGetSecretsFlags flags, + GDBusMethodInvocation *invocation) { NMDeviceIwdPrivate *priv = NM_DEVICE_IWD_GET_PRIVATE (self); NMActRequest *req; @@ -1091,7 +1085,7 @@ wifi_secrets_get_secrets (NMDeviceIwd *self, flags, NULL, wifi_secrets_cb, - self); + nm_utils_user_data_pack (self, invocation)); } static void @@ -1731,7 +1725,6 @@ gboolean nm_device_iwd_agent_psk_query (NMDeviceIwd *self, GDBusMethodInvocation *invocation) { - NMDeviceIwdPrivate *priv = NM_DEVICE_IWD_GET_PRIVATE (self); NMActRequest *req; NMSettingWirelessSecurity *s_wireless_sec; const gchar *psk; @@ -1759,9 +1752,9 @@ nm_device_iwd_agent_psk_query (NMDeviceIwd *self, wifi_secrets_get_secrets (self, NM_SETTING_WIRELESS_SECURITY_SETTING_NAME, NM_SECRET_AGENT_GET_SECRETS_FLAG_ALLOW_INTERACTION - | NM_SECRET_AGENT_GET_SECRETS_FLAG_REQUEST_NEW); + | NM_SECRET_AGENT_GET_SECRETS_FLAG_REQUEST_NEW, + invocation); - priv->secrets_request = invocation; return TRUE; } From 74d9e04a66dce87cddee6476cdbd244e83ae6379 Mon Sep 17 00:00:00 2001 From: Andrew Zaborowski Date: Fri, 15 Jun 2018 06:36:35 +0200 Subject: [PATCH 04/11] iwd: handle new secret request types from IWD agent The IWD DBus interface currently (https://git.kernel.org/pub/scm/network/wireless/iwd.git/tree/doc/agent-api.txt?id=38952813dddd776f66d2ed5e88eca9a892964c06) knows about 3 secret types related to 802.1x authentication in addition to the PSK secret request. Add support for the new methods and the new secret types in NM's implementation of the IWD secret agent. Note that the secret types are mapped to NMSetting8021x property keys and they are then sent to the NM Secret Agent in the hints parameter to GetSecrets, this will need support in the NM clients as the exact usage of the hints parameter is specified a little ambiguously, but this seems to be one of the permitted usages. Rework the IWD agent interface info initialization to use NM convenience macros. --- src/devices/wifi/nm-device-iwd.c | 199 ++++++++++++++++++++++++------ src/devices/wifi/nm-device-iwd.h | 4 +- src/devices/wifi/nm-iwd-manager.c | 118 +++++++++--------- 3 files changed, 224 insertions(+), 97 deletions(-) diff --git a/src/devices/wifi/nm-device-iwd.c b/src/devices/wifi/nm-device-iwd.c index b447da4ccb..efbc33fbc8 100644 --- a/src/devices/wifi/nm-device-iwd.c +++ b/src/devices/wifi/nm-device-iwd.c @@ -992,6 +992,127 @@ scanning_prohibited (NMDeviceIwd *self, gboolean periodic) return !priv->can_scan; } +/* + * try_reply_agent_request + * + * Check if the connection settings already have the secrets corresponding + * to the IWD agent method that was invoked. If they do, send the method reply + * with the appropriate secrets. Otherwise return the missing secret's setting + * name and key so the caller can send a NM secrets request with this data. + * Return TRUE in either case, return FALSE if an error is detected. + */ +static gboolean +try_reply_agent_request (NMDeviceIwd *self, + NMConnection *connection, + GDBusMethodInvocation *invocation, + const gchar **setting_name, + const gchar **setting_key, + gboolean *replied) +{ + const gchar *method_name = g_dbus_method_invocation_get_method_name (invocation); + NMSettingWirelessSecurity *s_wireless_sec; + NMSetting8021x *s_8021x; + + s_wireless_sec = nm_connection_get_setting_wireless_security (connection); + s_8021x = nm_connection_get_setting_802_1x (connection); + + *replied = FALSE; + + if (!strcmp (method_name, "RequestPassphrase")) { + const gchar *psk; + + if (!s_wireless_sec) + return FALSE; + + psk = nm_setting_wireless_security_get_psk (s_wireless_sec); + if (psk) { + _LOGD (LOGD_DEVICE | LOGD_WIFI, + "Returning the PSK to the IWD Agent"); + + g_dbus_method_invocation_return_value (invocation, + g_variant_new ("(s)", psk)); + *replied = TRUE; + return TRUE; + } + + *setting_name = NM_SETTING_WIRELESS_SECURITY_SETTING_NAME; + *setting_key = NM_SETTING_WIRELESS_SECURITY_PSK; + return TRUE; + } else if (!strcmp (method_name, "RequestPrivateKeyPassphrase")) { + const gchar *password; + + if (!s_8021x) + return FALSE; + + password = nm_setting_802_1x_get_private_key_password (s_8021x); + if (password) { + _LOGD (LOGD_DEVICE | LOGD_WIFI, + "Returning the private key password to the IWD Agent"); + + g_dbus_method_invocation_return_value (invocation, + g_variant_new ("(s)", password)); + *replied = TRUE; + return TRUE; + } + + *setting_name = NM_SETTING_802_1X_SETTING_NAME; + *setting_key = NM_SETTING_802_1X_PRIVATE_KEY_PASSWORD; + return TRUE; + } else if (!strcmp (method_name, "RequestUserNameAndPassword")) { + const gchar *identity, *password; + + if (!s_8021x) + return FALSE; + + identity = nm_setting_802_1x_get_identity (s_8021x); + password = nm_setting_802_1x_get_password (s_8021x); + if (identity && password) { + _LOGD (LOGD_DEVICE | LOGD_WIFI, + "Returning the username and password to the IWD Agent"); + + g_dbus_method_invocation_return_value (invocation, + g_variant_new ("(ss)", identity, password)); + *replied = TRUE; + return TRUE; + } + + *setting_name = NM_SETTING_802_1X_SETTING_NAME; + if (!identity) + *setting_key = NM_SETTING_802_1X_IDENTITY; + else + *setting_key = NM_SETTING_802_1X_PASSWORD; + return TRUE; + } else if (!strcmp (method_name, "RequestUserPassword")) { + const gchar *password; + + if (!s_8021x) + return FALSE; + + password = nm_setting_802_1x_get_password (s_8021x); + if (password) { + _LOGD (LOGD_DEVICE | LOGD_WIFI, + "Returning the user password to the IWD Agent"); + + g_dbus_method_invocation_return_value (invocation, + g_variant_new ("(s)", password)); + *replied = TRUE; + return TRUE; + } + + *setting_name = NM_SETTING_802_1X_SETTING_NAME; + *setting_key = NM_SETTING_802_1X_PASSWORD; + return TRUE; + } else + return FALSE; +} + +static void +wifi_secrets_get_one (NMDeviceIwd *self, + const char *setting_name, + NMSecretAgentGetSecretsFlags flags, + const char *setting_key, + GDBusMethodInvocation *invocation); + static void wifi_secrets_cb (NMActRequest *req, NMActRequestGetSecretsCallId *call_id, @@ -1002,9 +1123,10 @@ wifi_secrets_cb (NMActRequest *req, NMDeviceIwd *self; NMDeviceIwdPrivate *priv; NMDevice *device; - NMSettingWirelessSecurity *s_wireless_sec; - const gchar *psk; GDBusMethodInvocation *invocation; + const gchar *setting_name; + const gchar *setting_key; + gboolean replied; nm_utils_user_data_unpack (user_data, &self, &invocation); @@ -1035,22 +1157,24 @@ wifi_secrets_cb (NMActRequest *req, goto secrets_error; } - s_wireless_sec = nm_connection_get_setting_wireless_security (nm_act_request_get_applied_connection (req)); - if (!s_wireless_sec) + if (!try_reply_agent_request (self, nm_act_request_get_applied_connection (req), + invocation, &setting_name, &setting_key, + &replied)) goto secrets_error; - psk = nm_setting_wireless_security_get_psk (s_wireless_sec); - if (!psk) - goto secrets_error; + if (replied) { + /* Change state back to what it was before NEED_AUTH */ + nm_device_state_changed (device, NM_DEVICE_STATE_CONFIG, NM_DEVICE_STATE_REASON_NONE); + return; + } - _LOGD (LOGD_DEVICE | LOGD_WIFI, - "Returning a new PSK to the IWD Agent"); - - g_dbus_method_invocation_return_value (invocation, - g_variant_new ("(s)", psk)); - - /* Change state back to what it was before NEED_AUTH */ - nm_device_state_changed (device, NM_DEVICE_STATE_CONFIG, NM_DEVICE_STATE_REASON_NONE); + /* Request further secrets if we still need something */ + wifi_secrets_get_one (self, + setting_name, + NM_SECRET_AGENT_GET_SECRETS_FLAG_ALLOW_INTERACTION + | NM_SECRET_AGENT_GET_SECRETS_FLAG_REQUEST_NEW, + setting_key, + invocation); return; secrets_error: @@ -1066,10 +1190,11 @@ secrets_error: } static void -wifi_secrets_get_secrets (NMDeviceIwd *self, - const char *setting_name, - NMSecretAgentGetSecretsFlags flags, - GDBusMethodInvocation *invocation) +wifi_secrets_get_one (NMDeviceIwd *self, + const char *setting_name, + NMSecretAgentGetSecretsFlags flags, + const char *setting_key, + GDBusMethodInvocation *invocation) { NMDeviceIwdPrivate *priv = NM_DEVICE_IWD_GET_PRIVATE (self); NMActRequest *req; @@ -1083,7 +1208,7 @@ wifi_secrets_get_secrets (NMDeviceIwd *self, TRUE, setting_name, flags, - NULL, + setting_key, wifi_secrets_cb, nm_utils_user_data_pack (self, invocation)); } @@ -1722,38 +1847,34 @@ nm_device_iwd_set_dbus_object (NMDeviceIwd *self, GDBusObject *object) } gboolean -nm_device_iwd_agent_psk_query (NMDeviceIwd *self, - GDBusMethodInvocation *invocation) +nm_device_iwd_agent_query (NMDeviceIwd *self, + GDBusMethodInvocation *invocation) { NMActRequest *req; - NMSettingWirelessSecurity *s_wireless_sec; - const gchar *psk; + const gchar *setting_name; + const gchar *setting_key; + gboolean replied; req = nm_device_get_act_request (NM_DEVICE (self)); if (!req) return FALSE; - s_wireless_sec = nm_connection_get_setting_wireless_security (nm_act_request_get_applied_connection (req)); - if (!s_wireless_sec) + if (!try_reply_agent_request (self, nm_act_request_get_applied_connection (req), + invocation, &setting_name, &setting_key, + &replied)) return FALSE; - psk = nm_setting_wireless_security_get_psk (s_wireless_sec); - if (psk) { - _LOGD (LOGD_DEVICE | LOGD_WIFI, - "Returning the PSK to the IWD Agent"); - - g_dbus_method_invocation_return_value (invocation, - g_variant_new ("(s)", psk)); + if (replied) return TRUE; - } nm_device_state_changed (NM_DEVICE (self), NM_DEVICE_STATE_NEED_AUTH, NM_DEVICE_STATE_REASON_NO_SECRETS); - wifi_secrets_get_secrets (self, - NM_SETTING_WIRELESS_SECURITY_SETTING_NAME, - NM_SECRET_AGENT_GET_SECRETS_FLAG_ALLOW_INTERACTION - | NM_SECRET_AGENT_GET_SECRETS_FLAG_REQUEST_NEW, - invocation); + wifi_secrets_get_one (self, + setting_name, + NM_SECRET_AGENT_GET_SECRETS_FLAG_ALLOW_INTERACTION + | NM_SECRET_AGENT_GET_SECRETS_FLAG_REQUEST_NEW, + setting_key, + invocation); return TRUE; } diff --git a/src/devices/wifi/nm-device-iwd.h b/src/devices/wifi/nm-device-iwd.h index a4253f5217..825123b1d8 100644 --- a/src/devices/wifi/nm-device-iwd.h +++ b/src/devices/wifi/nm-device-iwd.h @@ -51,8 +51,8 @@ NMDevice *nm_device_iwd_new (const char *iface, NMDeviceWifiCapabilities capabil void nm_device_iwd_set_dbus_object (NMDeviceIwd *device, GDBusObject *object); -gboolean nm_device_iwd_agent_psk_query (NMDeviceIwd *device, - GDBusMethodInvocation *invocation); +gboolean nm_device_iwd_agent_query (NMDeviceIwd *device, + GDBusMethodInvocation *invocation); const CList *_nm_device_iwd_get_aps (NMDeviceIwd *self); diff --git a/src/devices/wifi/nm-iwd-manager.c b/src/devices/wifi/nm-iwd-manager.c index 39db3a04cd..daa796cbb9 100644 --- a/src/devices/wifi/nm-iwd-manager.c +++ b/src/devices/wifi/nm-iwd-manager.c @@ -84,12 +84,12 @@ G_DEFINE_TYPE (NMIwdManager, nm_iwd_manager, G_TYPE_OBJECT) /*****************************************************************************/ static void -psk_agent_dbus_method_cb (GDBusConnection *connection, - const gchar *sender, const gchar *object_path, - const gchar *interface_name, const gchar *method_name, - GVariant *parameters, - GDBusMethodInvocation *invocation, - gpointer user_data) +agent_dbus_method_cb (GDBusConnection *connection, + const gchar *sender, const gchar *object_path, + const gchar *interface_name, const gchar *method_name, + GVariant *parameters, + GDBusMethodInvocation *invocation, + gpointer user_data) { NMIwdManager *self = user_data; NMIwdManagerPrivate *priv = NM_IWD_MANAGER_GET_PRIVATE (self); @@ -104,7 +104,10 @@ psk_agent_dbus_method_cb (GDBusConnection *connection, if (!nm_streq0 (g_dbus_object_manager_client_get_name_owner (omc), sender)) goto return_error; - g_variant_get (parameters, "(&o)", &network_path); + if (!strcmp (method_name, "RequestUserPassword")) + g_variant_get (parameters, "(&os)", &network_path, NULL); + else + g_variant_get (parameters, "(&o)", &network_path); network = g_dbus_object_manager_get_interface (priv->object_manager, network_path, @@ -145,7 +148,7 @@ psk_agent_dbus_method_cb (GDBusConnection *connection, goto return_error; } - if (nm_device_iwd_agent_psk_query (NM_DEVICE_IWD (device), invocation)) + if (nm_device_iwd_agent_query (NM_DEVICE_IWD (device), invocation)) return; _LOGE ("Device %s did not handle the IWD Agent request", ifname); @@ -154,56 +157,59 @@ return_error: /* IWD doesn't look at the specific error */ g_dbus_method_invocation_return_error_literal (invocation, NM_DEVICE_ERROR, NM_DEVICE_ERROR_INVALID_CONNECTION, - "No PSK available for this connection"); + "Secrets not available for this connection"); } +static const GDBusInterfaceInfo iwd_agent_iface_info = NM_DEFINE_GDBUS_INTERFACE_INFO_INIT ( + "net.connman.iwd.Agent", + .methods = NM_DEFINE_GDBUS_METHOD_INFOS ( + NM_DEFINE_GDBUS_METHOD_INFO ( + "RequestPassphrase", + .in_args = NM_DEFINE_GDBUS_ARG_INFOS ( + NM_DEFINE_GDBUS_ARG_INFO ("network", "o"), + ), + .out_args = NM_DEFINE_GDBUS_ARG_INFOS ( + NM_DEFINE_GDBUS_ARG_INFO ("passphrase", "s"), + ), + ), + NM_DEFINE_GDBUS_METHOD_INFO ( + "RequestPrivateKeyPassphrase", + .in_args = NM_DEFINE_GDBUS_ARG_INFOS ( + NM_DEFINE_GDBUS_ARG_INFO ("network", "o"), + ), + .out_args = NM_DEFINE_GDBUS_ARG_INFOS ( + NM_DEFINE_GDBUS_ARG_INFO ("passphrase", "s"), + ), + ), + NM_DEFINE_GDBUS_METHOD_INFO ( + "RequestUserNameAndPassword", + .in_args = NM_DEFINE_GDBUS_ARG_INFOS ( + NM_DEFINE_GDBUS_ARG_INFO ("network", "o"), + ), + .out_args = NM_DEFINE_GDBUS_ARG_INFOS ( + NM_DEFINE_GDBUS_ARG_INFO ("user", "s"), + NM_DEFINE_GDBUS_ARG_INFO ("password", "s"), + ), + ), + NM_DEFINE_GDBUS_METHOD_INFO ( + "RequestUserPassword", + .in_args = NM_DEFINE_GDBUS_ARG_INFOS ( + NM_DEFINE_GDBUS_ARG_INFO ("network", "o"), + NM_DEFINE_GDBUS_ARG_INFO ("user", "s"), + ), + .out_args = NM_DEFINE_GDBUS_ARG_INFOS ( + NM_DEFINE_GDBUS_ARG_INFO ("password", "s"), + ), + ), + ), +); + static guint -psk_agent_export (GDBusConnection *connection, gpointer user_data, +iwd_agent_export (GDBusConnection *connection, gpointer user_data, gchar **agent_path, GError **error) { - static const GDBusArgInfo request_passphrase_arg_network = { - -1, - (gchar *) "network", - (gchar *) "o", - NULL, - }; - static const GDBusArgInfo *const request_passphrase_in_args[] = { - &request_passphrase_arg_network, - NULL, - }; - static const GDBusArgInfo request_passphrase_arg_passphrase = { - -1, - (gchar *) "passphrase", - (gchar *) "s", - NULL, - }; - static const GDBusArgInfo *const request_passphrase_out_args[] = { - &request_passphrase_arg_passphrase, - NULL, - }; - static const GDBusMethodInfo request_passphrase_info = { - -1, - (gchar *) "RequestPassphrase", - (GDBusArgInfo **) &request_passphrase_in_args, - (GDBusArgInfo **) &request_passphrase_out_args, - NULL, - }; - static const GDBusMethodInfo *const method_info[] = { - &request_passphrase_info, - NULL, - }; - static GDBusInterfaceInfo interface_info = { - -1, - (gchar *) "net.connman.iwd.Agent", - (GDBusMethodInfo **) &method_info, - NULL, - NULL, - NULL, - }; - static GDBusInterfaceVTable vtable = { - psk_agent_dbus_method_cb, - NULL, - NULL, + static const GDBusInterfaceVTable vtable = { + .method_call = agent_dbus_method_cb, }; gchar path[50]; @@ -221,8 +227,8 @@ psk_agent_export (GDBusConnection *connection, gpointer user_data, nm_sprintf_buf (path, "/agent/%u", rnd); id = g_dbus_connection_register_object (connection, path, - &interface_info, &vtable, - user_data, NULL, error); + NM_UNCONST_PTR (GDBusInterfaceInfo, &iwd_agent_iface_info), + &vtable, user_data, NULL, error); if (id) *agent_path = g_strdup (path); @@ -549,7 +555,7 @@ got_object_manager (GObject *object, GAsyncResult *result, gpointer user_data) connection = g_dbus_object_manager_client_get_connection (G_DBUS_OBJECT_MANAGER_CLIENT (object_manager)); - priv->agent_id = psk_agent_export (connection, self, + priv->agent_id = iwd_agent_export (connection, self, &priv->agent_path, &error); if (!priv->agent_id) { _LOGE ("failed to export the IWD Agent: PSK/8021x WiFi networks will not work: %s", From 24f5cf23e5c29045ccd2431df76b52e4574b5696 Mon Sep 17 00:00:00 2001 From: Andrew Zaborowski Date: Thu, 21 Jun 2018 09:41:25 +0200 Subject: [PATCH 05/11] iwd: don't set REQUEST_NEW secret request flag on first connection Allow the IWD backend to use secrets provided in the connection settings on initial connection attempt, only require new secrets on subsequent connections when IWD asks for them -- it only asks if fresh secrets are required. --- src/devices/wifi/nm-device-iwd.c | 34 +++++++++++++++++++++----------- 1 file changed, 22 insertions(+), 12 deletions(-) diff --git a/src/devices/wifi/nm-device-iwd.c b/src/devices/wifi/nm-device-iwd.c index efbc33fbc8..7667816a70 100644 --- a/src/devices/wifi/nm-device-iwd.c +++ b/src/devices/wifi/nm-device-iwd.c @@ -1127,6 +1127,7 @@ wifi_secrets_cb (NMActRequest *req, const gchar *setting_name; const gchar *setting_key; gboolean replied; + NMSecretAgentGetSecretsFlags get_secret_flags = NM_SECRET_AGENT_GET_SECRETS_FLAG_ALLOW_INTERACTION; nm_utils_user_data_unpack (user_data, &self, &invocation); @@ -1168,13 +1169,13 @@ wifi_secrets_cb (NMActRequest *req, return; } + if (nm_settings_connection_get_timestamp (nm_act_request_get_settings_connection (req), + NULL)) + get_secret_flags |= NM_SECRET_AGENT_GET_SECRETS_FLAG_REQUEST_NEW; + /* Request further secrets if we still need something */ - wifi_secrets_get_one (self, - setting_name, - NM_SECRET_AGENT_GET_SECRETS_FLAG_ALLOW_INTERACTION - | NM_SECRET_AGENT_GET_SECRETS_FLAG_REQUEST_NEW, - setting_key, - invocation); + wifi_secrets_get_one (self, setting_name, get_secret_flags, + setting_key, invocation); return; secrets_error: @@ -1854,6 +1855,7 @@ nm_device_iwd_agent_query (NMDeviceIwd *self, const gchar *setting_name; const gchar *setting_key; gboolean replied; + NMSecretAgentGetSecretsFlags get_secret_flags = NM_SECRET_AGENT_GET_SECRETS_FLAG_ALLOW_INTERACTION; req = nm_device_get_act_request (NM_DEVICE (self)); if (!req) @@ -1867,14 +1869,22 @@ nm_device_iwd_agent_query (NMDeviceIwd *self, if (replied) return TRUE; + /* Normally require new secrets every time IWD asks for them. + * IWD only queries us if it has not saved the secrets (e.g. by policy) + * or a previous attempt has failed with current secrets so it wants + * a fresh set. However if this is a new connection it may include + * all of the needed settings already so allow using these, too. + * Connection timestamp is set after activation or after first + * activation failure (to 0). + */ + if (nm_settings_connection_get_timestamp (nm_act_request_get_settings_connection (req), + NULL)) + get_secret_flags |= NM_SECRET_AGENT_GET_SECRETS_FLAG_REQUEST_NEW; + nm_device_state_changed (NM_DEVICE (self), NM_DEVICE_STATE_NEED_AUTH, NM_DEVICE_STATE_REASON_NO_SECRETS); - wifi_secrets_get_one (self, - setting_name, - NM_SECRET_AGENT_GET_SECRETS_FLAG_ALLOW_INTERACTION - | NM_SECRET_AGENT_GET_SECRETS_FLAG_REQUEST_NEW, - setting_key, - invocation); + wifi_secrets_get_one (self, setting_name, get_secret_flags, + setting_key, invocation); return TRUE; } From 1a6e53808db8bb8e75317fd5feacd74a9a98860f Mon Sep 17 00:00:00 2001 From: Andrew Zaborowski Date: Fri, 15 Jun 2018 05:59:41 +0200 Subject: [PATCH 06/11] cli: use the hints from 802.1x secrets requests if given If the hints parameter to the agent request wasn't empty, ask specifically for the 802-1x keys listed in the hints and skip the guessing. I didn't add human readable names for all of the 802-1x settings, it could be useful to do for at least the three 802-1x properties that add_8021x_secrets already knows about because those may have translations. --- clients/common/nm-secret-agent-simple.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/clients/common/nm-secret-agent-simple.c b/clients/common/nm-secret-agent-simple.c index 4db57ca54b..1468f0f335 100644 --- a/clients/common/nm-secret-agent-simple.c +++ b/clients/common/nm-secret-agent-simple.c @@ -217,6 +217,21 @@ add_8021x_secrets (NMSecretAgentSimpleRequest *request, const char *eap_method; NMSecretAgentSimpleSecret *secret; + /* If hints are given, then always ask for what the hints require */ + if (request->hints) { + char **iter; + for (iter = request->hints; *iter; iter++) { + secret = nm_secret_agent_simple_secret_new (NM_SECRET_AGENT_SECRET_TYPE_SECRET, + _(*iter), + NM_SETTING (s_8021x), + *iter, + NULL); + g_ptr_array_add (secrets, secret); + } + + return TRUE; + } + eap_method = nm_setting_802_1x_get_eap_method (s_8021x, 0); if (!eap_method) return FALSE; From f11246154eafe82b4e8acd52aa1c8ce21f0f5500 Mon Sep 17 00:00:00 2001 From: Andrew Zaborowski Date: Fri, 15 Jun 2018 04:46:36 +0200 Subject: [PATCH 07/11] settings-connection: don't expect system_secrets always present priv->system_secrets may be updated by e.g. nm_settings_connection_new_secrets and nm_settings_connection_update, but if the plugin creates the object with g_object_new, then adds some settings but never adds any secrets there's no reason to call either of those two methods. A call to nm_settings_connection_get_secrets should still be able to request new secrets (and may then update priv->system_secrets as a result). --- src/settings/nm-settings-connection.c | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/src/settings/nm-settings-connection.c b/src/settings/nm-settings-connection.c index 50cc14274d..c09f680400 100644 --- a/src/settings/nm-settings-connection.c +++ b/src/settings/nm-settings-connection.c @@ -1033,7 +1033,7 @@ get_secrets_done_cb (NMAgentManager *manager, NMSettingsConnectionPrivate *priv; NMConnection *applied_connection; gs_free_error GError *local = NULL; - GVariant *dict; + GVariant *dict = NULL; gboolean agent_had_system = FALSE; ForEachSecretFlags cmp_flags = { NM_SETTING_SECRET_FLAG_NONE, NM_SETTING_SECRET_FLAG_NONE }; @@ -1096,7 +1096,8 @@ get_secrets_done_cb (NMAgentManager *manager, setting_name, call_id); - dict = nm_connection_to_dbus (priv->system_secrets, NM_CONNECTION_SERIALIZE_ONLY_SECRETS); + if (priv->system_secrets) + dict = nm_connection_to_dbus (priv->system_secrets, NM_CONNECTION_SERIALIZE_ONLY_SECRETS); /* Update the connection with our existing secrets from backing storage */ nm_connection_clear_secrets (NM_CONNECTION (self)); @@ -1240,7 +1241,7 @@ nm_settings_connection_get_secrets (NMSettingsConnection *self, gpointer callback_data) { NMSettingsConnectionPrivate *priv = NM_SETTINGS_CONNECTION_GET_PRIVATE (self); - GVariant *existing_secrets; + GVariant *existing_secrets = NULL; NMAgentManagerCallId call_id_a; gs_free char *joined_hints = NULL; NMSettingsConnectionCallId *call_id; @@ -1262,15 +1263,6 @@ nm_settings_connection_get_secrets (NMSettingsConnection *self, call_id->callback_data = callback_data; c_list_link_tail (&priv->call_ids_lst_head, &call_id->call_ids_lst); - /* Use priv->secrets to work around the fact that nm_connection_clear_secrets() - * will clear secrets on this object's settings. - */ - if (!priv->system_secrets) { - g_set_error_literal (&local, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_FAILED, - "secrets cache invalid"); - goto schedule_dummy; - } - /* Make sure the request actually requests something we can return */ if (!nm_connection_get_setting_by_name (NM_CONNECTION (self), setting_name)) { g_set_error (&local, NM_CONNECTION_ERROR, NM_CONNECTION_ERROR_SETTING_NOT_FOUND, @@ -1286,7 +1278,11 @@ nm_settings_connection_get_secrets (NMSettingsConnection *self, goto schedule_dummy; } - existing_secrets = nm_connection_to_dbus (priv->system_secrets, NM_CONNECTION_SERIALIZE_ONLY_SECRETS); + /* Use priv->system_secrets to work around the fact that nm_connection_clear_secrets() + * will clear secrets on this object's settings. + */ + if (priv->system_secrets) + existing_secrets = nm_connection_to_dbus (priv->system_secrets, NM_CONNECTION_SERIALIZE_ONLY_SECRETS); if (existing_secrets) g_variant_ref_sink (existing_secrets); From 44cd60e82077a8f90511b8534ee12cf3ff182ff9 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 22 Jun 2018 15:16:59 +0200 Subject: [PATCH 08/11] wifi/iwd: don't check return value for nm_utils_random_bytes() nm_utils_random_bytes() will always try its best to give some random numbers. A failure only means, that the kernel interfaces get_random() or /dev/urandom failed to provide good randomness. We don't really need good random numbers here, so no need to handle a failure. --- src/devices/wifi/nm-iwd-manager.c | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/src/devices/wifi/nm-iwd-manager.c b/src/devices/wifi/nm-iwd-manager.c index daa796cbb9..185384dc07 100644 --- a/src/devices/wifi/nm-iwd-manager.c +++ b/src/devices/wifi/nm-iwd-manager.c @@ -211,24 +211,17 @@ iwd_agent_export (GDBusConnection *connection, gpointer user_data, static const GDBusInterfaceVTable vtable = { .method_call = agent_dbus_method_cb, }; - gchar path[50]; unsigned int rnd; guint id; - if (!nm_utils_random_bytes (&rnd, sizeof (rnd))) { - g_set_error_literal (error, - NM_DEVICE_ERROR, - NM_DEVICE_ERROR_FAILED, - "Can't read urandom."); - return 0; - } + nm_utils_random_bytes (&rnd, sizeof (rnd)); nm_sprintf_buf (path, "/agent/%u", rnd); id = g_dbus_connection_register_object (connection, path, NM_UNCONST_PTR (GDBusInterfaceInfo, &iwd_agent_iface_info), - &vtable, user_data, NULL, error); + &vtable, user_data, NULL, error); if (id) *agent_path = g_strdup (path); From 412a1fb46d4257f49b07b6aed262904506777d1e Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 22 Jun 2018 15:36:46 +0200 Subject: [PATCH 09/11] wifi/iwd: fix leaking name-owner in agent_dbus_method_cb() --- src/devices/wifi/nm-iwd-manager.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/devices/wifi/nm-iwd-manager.c b/src/devices/wifi/nm-iwd-manager.c index 185384dc07..b4047e5b03 100644 --- a/src/devices/wifi/nm-iwd-manager.c +++ b/src/devices/wifi/nm-iwd-manager.c @@ -93,15 +93,16 @@ agent_dbus_method_cb (GDBusConnection *connection, { NMIwdManager *self = user_data; NMIwdManagerPrivate *priv = NM_IWD_MANAGER_GET_PRIVATE (self); - GDBusObjectManagerClient *omc = G_DBUS_OBJECT_MANAGER_CLIENT (priv->object_manager); const gchar *network_path, *device_path, *ifname; gs_unref_object GDBusInterface *network = NULL, *device_obj = NULL; gs_unref_variant GVariant *value = NULL; gint ifindex; NMDevice *device; + gs_free char *name_owner = NULL; /* Be paranoid and check the sender address */ - if (!nm_streq0 (g_dbus_object_manager_client_get_name_owner (omc), sender)) + name_owner = g_dbus_object_manager_client_get_name_owner (G_DBUS_OBJECT_MANAGER_CLIENT (priv->object_manager)); + if (!nm_streq0 (name_owner, sender)) goto return_error; if (!strcmp (method_name, "RequestUserPassword")) From aef5110fa60ab4c6ceb14b91a2e5ca04f529219f Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 22 Jun 2018 15:43:22 +0200 Subject: [PATCH 10/11] wifi/iwd: downgrade error levels for agent-request failures level is for something really bad happening. When another party (iwd in this case) sends a D-Bus request that we cannot meaningfully handle, that is hardly reason to warn about. level is enough in this case. Also, give all messages a common prefix "agent-request" so that we have something to grep for. --- src/devices/wifi/nm-iwd-manager.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/devices/wifi/nm-iwd-manager.c b/src/devices/wifi/nm-iwd-manager.c index b4047e5b03..0f95fa0801 100644 --- a/src/devices/wifi/nm-iwd-manager.c +++ b/src/devices/wifi/nm-iwd-manager.c @@ -117,7 +117,7 @@ agent_dbus_method_cb (GDBusConnection *connection, device_path = g_variant_get_string (value, NULL); if (!device_path) { - _LOGE ("Device not cached for network %s in IWD Agent request", + _LOGD ("agent-request: device not cached for network %s in IWD Agent request", network_path); goto return_error; } @@ -130,29 +130,29 @@ agent_dbus_method_cb (GDBusConnection *connection, ifname = g_variant_get_string (value, NULL); if (!ifname) { - _LOGE ("Name not cached for device %s in IWD Agent request", + _LOGD ("agent-request: name not cached for device %s in IWD Agent request", device_path); goto return_error; } ifindex = if_nametoindex (ifname); if (!ifindex) { - _LOGE ("if_nametoindex failed for Name %s for Device at %s: %i", + _LOGD ("agent-request: if_nametoindex failed for Name %s for Device at %s: %i", ifname, device_path, errno); goto return_error; } device = nm_manager_get_device_by_ifindex (priv->manager, ifindex); if (!NM_IS_DEVICE_IWD (device)) { - _LOGE ("IWD device named %s is not a Wifi device in IWD Agent request", - ifname); + _LOGD ("agent-request: IWD device named %s is not a Wifi device in IWD Agent request", + ifname); goto return_error; } if (nm_device_iwd_agent_query (NM_DEVICE_IWD (device), invocation)) return; - _LOGE ("Device %s did not handle the IWD Agent request", ifname); + _LOGD ("agent-request: device %s did not handle the IWD Agent request", ifname); return_error: /* IWD doesn't look at the specific error */ From 31245cdd625966191731fcd795df87f0a10d613f Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 22 Jun 2018 15:47:41 +0200 Subject: [PATCH 11/11] manager: return NULL for invalid ifindex in nm_manager_get_device_by_ifindex() Internally, the device migth have negative or zero ifindex. When calling nm_manager_get_device_by_ifindex(), the caller wants to find a device with a valid ifindex, hence filter out non-positive values. --- src/nm-manager.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/nm-manager.c b/src/nm-manager.c index 13903d9b64..0fea13de69 100644 --- a/src/nm-manager.c +++ b/src/nm-manager.c @@ -1210,9 +1210,11 @@ nm_manager_get_device_by_ifindex (NMManager *self, int ifindex) NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (self); NMDevice *device; - c_list_for_each_entry (device, &priv->devices_lst_head, devices_lst) { - if (nm_device_get_ifindex (device) == ifindex) - return device; + if (ifindex > 0) { + c_list_for_each_entry (device, &priv->devices_lst_head, devices_lst) { + if (nm_device_get_ifindex (device) == ifindex) + return device; + } } return NULL;