From 3bc16323fab78911d7ffd983eb5de65eb609838f Mon Sep 17 00:00:00 2001 From: Andrew Zaborowski Date: Tue, 25 May 2021 20:00:41 +0200 Subject: [PATCH 1/2] libnm: Fix error message conditions in verity_ttls In two similar ``if () {} else if () {} else if () {} else {}`` sequences the latter two {} blocks were unreachable. In the identity/anonymous-identity case, anonymous-identity is optional, wpa_supplicant will fall back to identity, so only check that (a likely privacy issue because no NM or wpa_s documentation explains that the "secure" identity is also sent in plaintext if anonymous_identity is missing.) In the phase2_auth/phase2_autheap case change the message to make it clear that exactly one of the properties is expected to be present. Drop the empty string checks because those cases is validated later in verify() anyway. --- src/libnm-core-impl/nm-setting-8021x.c | 84 ++++++-------------------- 1 file changed, 17 insertions(+), 67 deletions(-) diff --git a/src/libnm-core-impl/nm-setting-8021x.c b/src/libnm-core-impl/nm-setting-8021x.c index 72f89f372c..8524a80533 100644 --- a/src/libnm-core-impl/nm-setting-8021x.c +++ b/src/libnm-core-impl/nm-setting-8021x.c @@ -2746,87 +2746,37 @@ verify_ttls(NMSetting8021x *self, gboolean phase2, GError **error) { NMSetting8021xPrivate *priv = NM_SETTING_802_1X_GET_PRIVATE(self); - if ((!priv->identity || !strlen(priv->identity)) - && (!priv->anonymous_identity || !strlen(priv->anonymous_identity))) { + if (!priv->identity || !strlen(priv->identity)) { if (!priv->identity) { g_set_error_literal(error, NM_CONNECTION_ERROR, NM_CONNECTION_ERROR_MISSING_PROPERTY, _("property is missing")); - g_prefix_error(error, - "%s.%s: ", - NM_SETTING_802_1X_SETTING_NAME, - NM_SETTING_802_1X_IDENTITY); - } else if (!strlen(priv->identity)) { - g_set_error_literal(error, - NM_CONNECTION_ERROR, - NM_CONNECTION_ERROR_INVALID_PROPERTY, - _("property is empty")); - g_prefix_error(error, - "%s.%s: ", - NM_SETTING_802_1X_SETTING_NAME, - NM_SETTING_802_1X_IDENTITY); - } else if (!priv->anonymous_identity) { - g_set_error_literal(error, - NM_CONNECTION_ERROR, - NM_CONNECTION_ERROR_MISSING_PROPERTY, - _("property is missing")); - g_prefix_error(error, - "%s.%s: ", - NM_SETTING_802_1X_SETTING_NAME, - NM_SETTING_802_1X_ANONYMOUS_IDENTITY); } else { g_set_error_literal(error, NM_CONNECTION_ERROR, NM_CONNECTION_ERROR_INVALID_PROPERTY, _("property is empty")); - g_prefix_error(error, - "%s.%s: ", - NM_SETTING_802_1X_SETTING_NAME, - NM_SETTING_802_1X_ANONYMOUS_IDENTITY); } + g_prefix_error(error, + "%s.%s: ", + NM_SETTING_802_1X_SETTING_NAME, + NM_SETTING_802_1X_IDENTITY); return FALSE; } - if ((!priv->phase2_auth || !strlen(priv->phase2_auth)) - && (!priv->phase2_autheap || !strlen(priv->phase2_autheap))) { - if (!priv->phase2_auth) { - g_set_error_literal(error, - NM_CONNECTION_ERROR, - NM_CONNECTION_ERROR_MISSING_PROPERTY, - _("property is missing")); - g_prefix_error(error, - "%s.%s: ", - NM_SETTING_802_1X_SETTING_NAME, - NM_SETTING_802_1X_PHASE2_AUTH); - } else if (!strlen(priv->phase2_auth)) { - g_set_error_literal(error, - NM_CONNECTION_ERROR, - NM_CONNECTION_ERROR_INVALID_PROPERTY, - _("property is empty")); - g_prefix_error(error, - "%s.%s: ", - NM_SETTING_802_1X_SETTING_NAME, - NM_SETTING_802_1X_PHASE2_AUTH); - } else if (!priv->phase2_autheap) { - g_set_error_literal(error, - NM_CONNECTION_ERROR, - NM_CONNECTION_ERROR_MISSING_PROPERTY, - _("property is missing")); - g_prefix_error(error, - "%s.%s: ", - NM_SETTING_802_1X_SETTING_NAME, - NM_SETTING_802_1X_PHASE2_AUTHEAP); - } else { - g_set_error_literal(error, - NM_CONNECTION_ERROR, - NM_CONNECTION_ERROR_INVALID_PROPERTY, - _("property is empty")); - g_prefix_error(error, - "%s.%s: ", - NM_SETTING_802_1X_SETTING_NAME, - NM_SETTING_802_1X_PHASE2_AUTHEAP); - } + if ((!priv->phase2_auth && !priv->phase2_autheap) + || (priv->phase2_auth && priv->phase2_autheap)) { + g_set_error_literal(error, + NM_CONNECTION_ERROR, + NM_CONNECTION_ERROR_MISSING_PROPERTY, + _("exactly one property must be set")); + g_prefix_error(error, + "%s.%s, %s.%s: ", + NM_SETTING_802_1X_SETTING_NAME, + NM_SETTING_802_1X_PHASE2_AUTH, + NM_SETTING_802_1X_SETTING_NAME, + NM_SETTING_802_1X_PHASE2_AUTHEAP); return FALSE; } From 6aa8062f333ea01085dacba7f0e27c1434304a02 Mon Sep 17 00:00:00 2001 From: Andrew Zaborowski Date: Wed, 26 May 2021 01:10:11 +0200 Subject: [PATCH 2/2] iwd: If no EAP anonymous identity set fall back to identity Mimic the behaviour of wpa_supplicant where the "secure" identity in TTLS and PEAP (802-1x.identity) is used as a fallback in the anonymous identity (802-1x.anonymous_identity) if that is not provided. This is needed to keep the profiles compatible between the two wifi backends, for users of poorly configured WPA-Enterprise networks that require the user login to be sent in phase 1 or in both phases. The code responsible for this mechanism in wpa_supplicant, at the time of writing, is https://w1.fi/cgit/hostap/tree/src/eap_peer/eap.c?id=c733664be9dd3763c03f2da2cb32a23775dde388#n1688 and offers no comment about the privacy implications. --- src/core/devices/wifi/nm-wifi-utils.c | 29 +++++++++++++++++++++------ 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/src/core/devices/wifi/nm-wifi-utils.c b/src/core/devices/wifi/nm-wifi-utils.c index c486d239a5..15ced990b7 100644 --- a/src/core/devices/wifi/nm-wifi-utils.c +++ b/src/core/devices/wifi/nm-wifi-utils.c @@ -1342,6 +1342,27 @@ eap_optional_password_to_iwd_config(GKeyFile * file, } } +static void +eap_phase1_identity_to_iwd_config(GKeyFile *file, const char *iwd_prefix, NMSetting8021x *s_8021x) +{ + const char *phase1_identity = nm_setting_802_1x_get_anonymous_identity(s_8021x); + + if (!phase1_identity) { + phase1_identity = nm_setting_802_1x_get_identity(s_8021x); + + if (phase1_identity) { + nm_log_info(LOGD_WIFI, + "IWD network config will send the same EAP Identity string in " + "plaintext in phase 1 as in phase 2 (encrypted) to mimic legacy " + "behavior, set [%s].%s=anonymous to prevent exposing the value", + NM_SETTING_802_1X_SETTING_NAME, + NM_SETTING_802_1X_ANONYMOUS_IDENTITY); + } + } + + eap_optional_identity_to_iwd_config(file, iwd_prefix, phase1_identity); +} + static gboolean eap_method_config_to_iwd_config(GKeyFile * file, NMSetting8021x *s_8021x, @@ -1367,9 +1388,7 @@ eap_method_config_to_iwd_config(GKeyFile * file, const char *noneap_method = nm_setting_802_1x_get_phase2_auth(s_8021x); eap_method_name_to_iwd_config(file, iwd_prefix, "TTLS"); - eap_optional_identity_to_iwd_config(file, - iwd_prefix, - nm_setting_802_1x_get_anonymous_identity(s_8021x)); + eap_phase1_identity_to_iwd_config(file, iwd_prefix, s_8021x); if (!eap_certs_to_iwd_config(file, s_8021x, @@ -1423,9 +1442,7 @@ eap_method_config_to_iwd_config(GKeyFile * file, return FALSE; } else if (nm_streq0(method, "peap") && !phase2) { eap_method_name_to_iwd_config(file, iwd_prefix, "PEAP"); - eap_optional_identity_to_iwd_config(file, - iwd_prefix, - nm_setting_802_1x_get_anonymous_identity(s_8021x)); + eap_phase1_identity_to_iwd_config(file, iwd_prefix, s_8021x); if (!eap_certs_to_iwd_config(file, s_8021x,