From cffe3a3ef63086e5cd78675777d679126da3a8e6 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 2 Jan 2020 14:42:28 +0100 Subject: [PATCH 1/7] libnm: return early from nm_utils_security_valid() Once we know the outcome of the check, just return it instead of falling though to return a variable "good" which was initialized two pages earlier. Also, avoid the "default" switch case. This way, we get a compiler warning about missing enum values. --- libnm-core/nm-utils.c | 35 ++++++++++++++++++----------------- 1 file changed, 18 insertions(+), 17 deletions(-) diff --git a/libnm-core/nm-utils.c b/libnm-core/nm-utils.c index bb79476fa..dc938d353 100644 --- a/libnm-core/nm-utils.c +++ b/libnm-core/nm-utils.c @@ -1122,7 +1122,12 @@ nm_utils_ap_mode_security_valid (NMUtilsSecurityType type, case NMU_SEC_SAE: case NMU_SEC_OWE: return TRUE; - default: + case NMU_SEC_LEAP: + case NMU_SEC_DYNAMIC_WEP: + case NMU_SEC_WPA_ENTERPRISE: + case NMU_SEC_WPA2_ENTERPRISE: + return FALSE; + case NMU_SEC_INVALID: break; } return FALSE; @@ -1161,8 +1166,6 @@ nm_utils_security_valid (NMUtilsSecurityType type, NM80211ApSecurityFlags ap_wpa, NM80211ApSecurityFlags ap_rsn) { - gboolean good = TRUE; - if (!have_ap) { if (type == NMU_SEC_NONE) return TRUE; @@ -1171,8 +1174,7 @@ nm_utils_security_valid (NMUtilsSecurityType type, || ((type == NMU_SEC_LEAP) && !adhoc)) { if (wifi_caps & (NM_WIFI_DEVICE_CAP_CIPHER_WEP40 | NM_WIFI_DEVICE_CAP_CIPHER_WEP104)) return TRUE; - else - return FALSE; + return FALSE; } } @@ -1183,7 +1185,7 @@ nm_utils_security_valid (NMUtilsSecurityType type, return FALSE; if (ap_wpa || ap_rsn) return FALSE; - break; + return TRUE; case NMU_SEC_LEAP: /* require PRIVACY bit for LEAP? */ if (adhoc) return FALSE; @@ -1197,7 +1199,7 @@ nm_utils_security_valid (NMUtilsSecurityType type, if (!device_supports_ap_ciphers (wifi_caps, ap_rsn, TRUE)) return FALSE; } - break; + return TRUE; case NMU_SEC_DYNAMIC_WEP: if (adhoc) return FALSE; @@ -1211,7 +1213,7 @@ nm_utils_security_valid (NMUtilsSecurityType type, if (!device_supports_ap_ciphers (wifi_caps, ap_wpa, FALSE)) return FALSE; } - break; + return TRUE; case NMU_SEC_WPA_PSK: if (adhoc) return FALSE; @@ -1228,7 +1230,7 @@ nm_utils_security_valid (NMUtilsSecurityType type, } return FALSE; } - break; + return TRUE; case NMU_SEC_WPA2_PSK: if (!(wifi_caps & NM_WIFI_DEVICE_CAP_RSN)) return FALSE; @@ -1251,7 +1253,7 @@ nm_utils_security_valid (NMUtilsSecurityType type, } return FALSE; } - break; + return TRUE; case NMU_SEC_WPA_ENTERPRISE: if (adhoc) return FALSE; @@ -1264,7 +1266,7 @@ nm_utils_security_valid (NMUtilsSecurityType type, if (!device_supports_ap_ciphers (wifi_caps, ap_wpa, FALSE)) return FALSE; } - break; + return TRUE; case NMU_SEC_WPA2_ENTERPRISE: if (adhoc) return FALSE; @@ -1277,7 +1279,7 @@ nm_utils_security_valid (NMUtilsSecurityType type, if (!device_supports_ap_ciphers (wifi_caps, ap_rsn, FALSE)) return FALSE; } - break; + return TRUE; case NMU_SEC_SAE: if (!(wifi_caps & NM_WIFI_DEVICE_CAP_RSN)) return FALSE; @@ -1300,7 +1302,7 @@ nm_utils_security_valid (NMUtilsSecurityType type, } return FALSE; } - break; + return TRUE; case NMU_SEC_OWE: if (adhoc) return FALSE; @@ -1310,13 +1312,12 @@ nm_utils_security_valid (NMUtilsSecurityType type, if (!(ap_rsn & NM_802_11_AP_SEC_KEY_MGMT_OWE)) return FALSE; } - break; - default: - good = FALSE; + return TRUE; + case NMU_SEC_INVALID: break; } - return good; + return FALSE; } /** From 2e72403cb75883a277628d8e87252cc2be0f8285 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 2 Jan 2020 15:03:39 +0100 Subject: [PATCH 2/7] libnm: add missing braces to multi-line condition in nm_utils_security_valid() --- libnm-core/nm-utils.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/libnm-core/nm-utils.c b/libnm-core/nm-utils.c index dc938d353..1f30fd159 100644 --- a/libnm-core/nm-utils.c +++ b/libnm-core/nm-utils.c @@ -1195,9 +1195,10 @@ nm_utils_security_valid (NMUtilsSecurityType type, if (!(ap_flags & NM_802_11_AP_FLAGS_PRIVACY)) return FALSE; if (ap_wpa || ap_rsn) { - if (!device_supports_ap_ciphers (wifi_caps, ap_wpa, TRUE)) + if (!device_supports_ap_ciphers (wifi_caps, ap_wpa, TRUE)) { if (!device_supports_ap_ciphers (wifi_caps, ap_rsn, TRUE)) return FALSE; + } } return TRUE; case NMU_SEC_DYNAMIC_WEP: From 936bb8716b6b9c925cf40145a1565f5522dc696c Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 2 Jan 2020 15:09:34 +0100 Subject: [PATCH 3/7] libnm: break lines in conditions of nm_utils_security_valid() --- libnm-core/nm-utils.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/libnm-core/nm-utils.c b/libnm-core/nm-utils.c index 1f30fd159..7d6e159b8 100644 --- a/libnm-core/nm-utils.c +++ b/libnm-core/nm-utils.c @@ -1183,7 +1183,8 @@ nm_utils_security_valid (NMUtilsSecurityType type, g_assert (have_ap); if (ap_flags & NM_802_11_AP_FLAGS_PRIVACY) return FALSE; - if (ap_wpa || ap_rsn) + if ( ap_wpa + || ap_rsn) return FALSE; return TRUE; case NMU_SEC_LEAP: /* require PRIVACY bit for LEAP? */ @@ -1194,7 +1195,8 @@ nm_utils_security_valid (NMUtilsSecurityType type, g_assert (have_ap); if (!(ap_flags & NM_802_11_AP_FLAGS_PRIVACY)) return FALSE; - if (ap_wpa || ap_rsn) { + if ( ap_wpa + || ap_rsn) { if (!device_supports_ap_ciphers (wifi_caps, ap_wpa, TRUE)) { if (!device_supports_ap_ciphers (wifi_caps, ap_rsn, TRUE)) return FALSE; @@ -1205,7 +1207,8 @@ nm_utils_security_valid (NMUtilsSecurityType type, if (adhoc) return FALSE; g_assert (have_ap); - if (ap_rsn || !(ap_flags & NM_802_11_AP_FLAGS_PRIVACY)) + if ( ap_rsn + || !(ap_flags & NM_802_11_AP_FLAGS_PRIVACY)) return FALSE; /* Some APs broadcast minimal WPA-enabled beacons that must be handled */ if (ap_wpa) { From e9d4980d6b22b90ef1017b9c94bfc6ce2b6ceafe Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 2 Jan 2020 15:03:39 +0100 Subject: [PATCH 4/7] libnm: cleanup conditions by moving pre-check in nm_utils_security_valid() Do the switch based on the type on the top level, don't split the conditions to first handle some cases, and some later. --- libnm-core/nm-utils.c | 27 ++++++++++++--------------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/libnm-core/nm-utils.c b/libnm-core/nm-utils.c index 7d6e159b8..f5da87b7c 100644 --- a/libnm-core/nm-utils.c +++ b/libnm-core/nm-utils.c @@ -1166,21 +1166,10 @@ nm_utils_security_valid (NMUtilsSecurityType type, NM80211ApSecurityFlags ap_wpa, NM80211ApSecurityFlags ap_rsn) { - if (!have_ap) { - if (type == NMU_SEC_NONE) - return TRUE; - if ( (type == NMU_SEC_STATIC_WEP) - || ((type == NMU_SEC_DYNAMIC_WEP) && !adhoc) - || ((type == NMU_SEC_LEAP) && !adhoc)) { - if (wifi_caps & (NM_WIFI_DEVICE_CAP_CIPHER_WEP40 | NM_WIFI_DEVICE_CAP_CIPHER_WEP104)) - return TRUE; - return FALSE; - } - } - switch (type) { case NMU_SEC_NONE: - g_assert (have_ap); + if (!have_ap) + return TRUE; if (ap_flags & NM_802_11_AP_FLAGS_PRIVACY) return FALSE; if ( ap_wpa @@ -1192,7 +1181,11 @@ nm_utils_security_valid (NMUtilsSecurityType type, return FALSE; /* fall through */ case NMU_SEC_STATIC_WEP: - g_assert (have_ap); + if (!have_ap) { + if (wifi_caps & (NM_WIFI_DEVICE_CAP_CIPHER_WEP40 | NM_WIFI_DEVICE_CAP_CIPHER_WEP104)) + return TRUE; + return FALSE; + } if (!(ap_flags & NM_802_11_AP_FLAGS_PRIVACY)) return FALSE; if ( ap_wpa @@ -1206,7 +1199,11 @@ nm_utils_security_valid (NMUtilsSecurityType type, case NMU_SEC_DYNAMIC_WEP: if (adhoc) return FALSE; - g_assert (have_ap); + if (!have_ap) { + if (wifi_caps & (NM_WIFI_DEVICE_CAP_CIPHER_WEP40 | NM_WIFI_DEVICE_CAP_CIPHER_WEP104)) + return TRUE; + return FALSE; + } if ( ap_rsn || !(ap_flags & NM_802_11_AP_FLAGS_PRIVACY)) return FALSE; From 3d20c9985d78a316441fcbdf2069c66f30d91f2e Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 2 Jan 2020 15:12:15 +0100 Subject: [PATCH 5/7] libnm: avoid deep nesting in checks of nm_utils_security_valid() --- libnm-core/nm-utils.c | 127 +++++++++++++++++++++--------------------- 1 file changed, 62 insertions(+), 65 deletions(-) diff --git a/libnm-core/nm-utils.c b/libnm-core/nm-utils.c index f5da87b7c..5e1f70e41 100644 --- a/libnm-core/nm-utils.c +++ b/libnm-core/nm-utils.c @@ -1220,99 +1220,96 @@ nm_utils_security_valid (NMUtilsSecurityType type, return FALSE; if (!(wifi_caps & NM_WIFI_DEVICE_CAP_WPA)) return FALSE; - if (have_ap) { - if (ap_wpa & NM_802_11_AP_SEC_KEY_MGMT_PSK) { - if ( (ap_wpa & NM_802_11_AP_SEC_PAIR_TKIP) - && (wifi_caps & NM_WIFI_DEVICE_CAP_CIPHER_TKIP)) - return TRUE; - if ( (ap_wpa & NM_802_11_AP_SEC_PAIR_CCMP) - && (wifi_caps & NM_WIFI_DEVICE_CAP_CIPHER_CCMP)) - return TRUE; - } - return FALSE; + if (!have_ap) + return TRUE; + if (ap_wpa & NM_802_11_AP_SEC_KEY_MGMT_PSK) { + if ( (ap_wpa & NM_802_11_AP_SEC_PAIR_TKIP) + && (wifi_caps & NM_WIFI_DEVICE_CAP_CIPHER_TKIP)) + return TRUE; + if ( (ap_wpa & NM_802_11_AP_SEC_PAIR_CCMP) + && (wifi_caps & NM_WIFI_DEVICE_CAP_CIPHER_CCMP)) + return TRUE; } - return TRUE; + return FALSE; case NMU_SEC_WPA2_PSK: if (!(wifi_caps & NM_WIFI_DEVICE_CAP_RSN)) return FALSE; - if (have_ap) { - if (adhoc) { - if (!(wifi_caps & NM_WIFI_DEVICE_CAP_IBSS_RSN)) - return FALSE; - if ( (ap_rsn & NM_802_11_AP_SEC_PAIR_CCMP) - && (wifi_caps & NM_WIFI_DEVICE_CAP_CIPHER_CCMP)) - return TRUE; - } else { - if (ap_rsn & NM_802_11_AP_SEC_KEY_MGMT_PSK) { - if ( (ap_rsn & NM_802_11_AP_SEC_PAIR_TKIP) - && (wifi_caps & NM_WIFI_DEVICE_CAP_CIPHER_TKIP)) - return TRUE; - if ( (ap_rsn & NM_802_11_AP_SEC_PAIR_CCMP) - && (wifi_caps & NM_WIFI_DEVICE_CAP_CIPHER_CCMP)) - return TRUE; - } - } + if (!have_ap) + return TRUE; + if (adhoc) { + if (!(wifi_caps & NM_WIFI_DEVICE_CAP_IBSS_RSN)) + return FALSE; + if ( (ap_rsn & NM_802_11_AP_SEC_PAIR_CCMP) + && (wifi_caps & NM_WIFI_DEVICE_CAP_CIPHER_CCMP)) + return TRUE; return FALSE; } - return TRUE; + if (ap_rsn & NM_802_11_AP_SEC_KEY_MGMT_PSK) { + if ( (ap_rsn & NM_802_11_AP_SEC_PAIR_TKIP) + && (wifi_caps & NM_WIFI_DEVICE_CAP_CIPHER_TKIP)) + return TRUE; + if ( (ap_rsn & NM_802_11_AP_SEC_PAIR_CCMP) + && (wifi_caps & NM_WIFI_DEVICE_CAP_CIPHER_CCMP)) + return TRUE; + } + return FALSE; case NMU_SEC_WPA_ENTERPRISE: if (adhoc) return FALSE; if (!(wifi_caps & NM_WIFI_DEVICE_CAP_WPA)) return FALSE; - if (have_ap) { - if (!(ap_wpa & NM_802_11_AP_SEC_KEY_MGMT_802_1X)) - return FALSE; - /* Ensure at least one WPA cipher is supported */ - if (!device_supports_ap_ciphers (wifi_caps, ap_wpa, FALSE)) - return FALSE; - } + if (!have_ap) + return TRUE; + if (!(ap_wpa & NM_802_11_AP_SEC_KEY_MGMT_802_1X)) + return FALSE; + /* Ensure at least one WPA cipher is supported */ + if (!device_supports_ap_ciphers (wifi_caps, ap_wpa, FALSE)) + return FALSE; return TRUE; case NMU_SEC_WPA2_ENTERPRISE: if (adhoc) return FALSE; if (!(wifi_caps & NM_WIFI_DEVICE_CAP_RSN)) return FALSE; - if (have_ap) { - if (!(ap_rsn & NM_802_11_AP_SEC_KEY_MGMT_802_1X)) - return FALSE; - /* Ensure at least one WPA cipher is supported */ - if (!device_supports_ap_ciphers (wifi_caps, ap_rsn, FALSE)) - return FALSE; - } + if (!have_ap) + return TRUE; + if (!(ap_rsn & NM_802_11_AP_SEC_KEY_MGMT_802_1X)) + return FALSE; + /* Ensure at least one WPA cipher is supported */ + if (!device_supports_ap_ciphers (wifi_caps, ap_rsn, FALSE)) + return FALSE; return TRUE; case NMU_SEC_SAE: if (!(wifi_caps & NM_WIFI_DEVICE_CAP_RSN)) return FALSE; - if (have_ap) { - if (adhoc) { - if (!(wifi_caps & NM_WIFI_DEVICE_CAP_IBSS_RSN)) - return FALSE; - if ( (ap_rsn & NM_802_11_AP_SEC_PAIR_CCMP) - && (wifi_caps & NM_WIFI_DEVICE_CAP_CIPHER_CCMP)) - return TRUE; - } else { - if (ap_rsn & NM_802_11_AP_SEC_KEY_MGMT_SAE) { - if ( (ap_rsn & NM_802_11_AP_SEC_PAIR_TKIP) - && (wifi_caps & NM_WIFI_DEVICE_CAP_CIPHER_TKIP)) - return TRUE; - if ( (ap_rsn & NM_802_11_AP_SEC_PAIR_CCMP) - && (wifi_caps & NM_WIFI_DEVICE_CAP_CIPHER_CCMP)) - return TRUE; - } - } + if (!have_ap) + return TRUE; + if (adhoc) { + if (!(wifi_caps & NM_WIFI_DEVICE_CAP_IBSS_RSN)) + return FALSE; + if ( (ap_rsn & NM_802_11_AP_SEC_PAIR_CCMP) + && (wifi_caps & NM_WIFI_DEVICE_CAP_CIPHER_CCMP)) + return TRUE; return FALSE; } - return TRUE; + if (ap_rsn & NM_802_11_AP_SEC_KEY_MGMT_SAE) { + if ( (ap_rsn & NM_802_11_AP_SEC_PAIR_TKIP) + && (wifi_caps & NM_WIFI_DEVICE_CAP_CIPHER_TKIP)) + return TRUE; + if ( (ap_rsn & NM_802_11_AP_SEC_PAIR_CCMP) + && (wifi_caps & NM_WIFI_DEVICE_CAP_CIPHER_CCMP)) + return TRUE; + } + return FALSE; case NMU_SEC_OWE: if (adhoc) return FALSE; if (!(wifi_caps & NM_WIFI_DEVICE_CAP_RSN)) return FALSE; - if (have_ap) { - if (!(ap_rsn & NM_802_11_AP_SEC_KEY_MGMT_OWE)) - return FALSE; - } + if (!have_ap) + return TRUE; + if (!(ap_rsn & NM_802_11_AP_SEC_KEY_MGMT_OWE)) + return FALSE; return TRUE; case NMU_SEC_INVALID: break; From 31aac7a9d8f817708fa8b6ef2fb4f1a45e1affe1 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 7 Jan 2020 11:28:25 +0100 Subject: [PATCH 6/7] libnm: let nm_utils_security_valid() reject adhoc mode with SAE --- libnm-core/nm-utils.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/libnm-core/nm-utils.c b/libnm-core/nm-utils.c index 5e1f70e41..cf5c1de28 100644 --- a/libnm-core/nm-utils.c +++ b/libnm-core/nm-utils.c @@ -1282,16 +1282,10 @@ nm_utils_security_valid (NMUtilsSecurityType type, case NMU_SEC_SAE: if (!(wifi_caps & NM_WIFI_DEVICE_CAP_RSN)) return FALSE; + if (adhoc) + return FALSE; if (!have_ap) return TRUE; - if (adhoc) { - if (!(wifi_caps & NM_WIFI_DEVICE_CAP_IBSS_RSN)) - return FALSE; - if ( (ap_rsn & NM_802_11_AP_SEC_PAIR_CCMP) - && (wifi_caps & NM_WIFI_DEVICE_CAP_CIPHER_CCMP)) - return TRUE; - return FALSE; - } if (ap_rsn & NM_802_11_AP_SEC_KEY_MGMT_SAE) { if ( (ap_rsn & NM_802_11_AP_SEC_PAIR_TKIP) && (wifi_caps & NM_WIFI_DEVICE_CAP_CIPHER_TKIP)) From 4e9119c52efd4800b51101fb1a378e626280d0e8 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 7 Jan 2020 11:29:09 +0100 Subject: [PATCH 7/7] libnm: let nm_utils_security_valid() reject TKIP with SAE (WPA3) SAE should always use CCMP. --- libnm-core/nm-utils.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/libnm-core/nm-utils.c b/libnm-core/nm-utils.c index cf5c1de28..b6d085d96 100644 --- a/libnm-core/nm-utils.c +++ b/libnm-core/nm-utils.c @@ -1287,9 +1287,6 @@ nm_utils_security_valid (NMUtilsSecurityType type, if (!have_ap) return TRUE; if (ap_rsn & NM_802_11_AP_SEC_KEY_MGMT_SAE) { - if ( (ap_rsn & NM_802_11_AP_SEC_PAIR_TKIP) - && (wifi_caps & NM_WIFI_DEVICE_CAP_CIPHER_TKIP)) - return TRUE; if ( (ap_rsn & NM_802_11_AP_SEC_PAIR_CCMP) && (wifi_caps & NM_WIFI_DEVICE_CAP_CIPHER_CCMP)) return TRUE;