* [PATCH v2 2/6] nl80211util: Move nl80211_append_rsn_attributes
2023-11-27 4:38 [PATCH v2 1/6] ie: Move AKM suite converter from netdev Denis Kenzior
@ 2023-11-27 4:38 ` Denis Kenzior
2023-11-27 4:38 ` [PATCH v2 3/6] netdev: Don't duplicate vendor_ies Denis Kenzior
` (4 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Denis Kenzior @ 2023-11-27 4:38 UTC (permalink / raw)
To: iwd; +Cc: Denis Kenzior
---
Makefile.am | 1 +
src/netdev.c | 43 ++-----------------------------------------
src/nl80211util.c | 40 ++++++++++++++++++++++++++++++++++++++++
src/nl80211util.h | 4 ++++
4 files changed, 47 insertions(+), 41 deletions(-)
diff --git a/Makefile.am b/Makefile.am
index d0247faf7ed5..5ed6ab37164b 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -407,6 +407,7 @@ tools_hwsim_SOURCES = tools/hwsim.c src/mpdu.h \
src/storage.h src/storage.c \
src/common.h src/common.c \
src/band.h src/band.c \
+ src/ie.h src/ie.c \
src/crypto.h src/crypto.c
tools_hwsim_LDADD = $(ell_ldadd)
diff --git a/src/netdev.c b/src/netdev.c
index 7b951a6a4797..1c032b32acdc 100644
--- a/src/netdev.c
+++ b/src/netdev.c
@@ -2445,45 +2445,6 @@ static void netdev_driver_connected(struct netdev *netdev)
eapol_register(netdev->sm);
}
-
-static void netdev_append_nl80211_rsn_attributes(struct l_genl_msg *msg,
- struct handshake_state *hs)
-{
- uint32_t nl_cipher;
- uint32_t nl_akm;
- uint32_t wpa_version;
-
- nl_cipher = ie_rsn_cipher_suite_to_cipher(hs->pairwise_cipher);
- L_WARN_ON(!nl_cipher);
- l_genl_msg_append_attr(msg, NL80211_ATTR_CIPHER_SUITES_PAIRWISE,
- 4, &nl_cipher);
-
- nl_cipher = ie_rsn_cipher_suite_to_cipher(hs->group_cipher);
- L_WARN_ON(!nl_cipher);
- l_genl_msg_append_attr(msg, NL80211_ATTR_CIPHER_SUITE_GROUP,
- 4, &nl_cipher);
-
- if (hs->mfp) {
- uint32_t use_mfp = NL80211_MFP_REQUIRED;
-
- l_genl_msg_append_attr(msg, NL80211_ATTR_USE_MFP, 4, &use_mfp);
- }
-
- nl_akm = ie_rsn_akm_suite_to_akm(hs->akm_suite);
- L_WARN_ON(!nl_akm);
- l_genl_msg_append_attr(msg, NL80211_ATTR_AKM_SUITES, 4, &nl_akm);
-
- if (IE_AKM_IS_SAE(hs->akm_suite))
- wpa_version = NL80211_WPA_VERSION_3;
- else if (hs->wpa_ie)
- wpa_version = NL80211_WPA_VERSION_1;
- else
- wpa_version = NL80211_WPA_VERSION_2;
-
- l_genl_msg_append_attr(msg, NL80211_ATTR_WPA_VERSIONS,
- 4, &wpa_version);
-}
-
static struct l_genl_msg *netdev_build_cmd_connect(struct netdev *netdev,
struct handshake_state *hs,
const uint8_t *prev_bssid,
@@ -2540,7 +2501,7 @@ static struct l_genl_msg *netdev_build_cmd_connect(struct netdev *netdev,
l_genl_msg_append_attr(msg, NL80211_ATTR_SOCKET_OWNER, 0, NULL);
if (is_rsn) {
- netdev_append_nl80211_rsn_attributes(msg, hs);
+ nl80211_append_rsn_attributes(msg, hs);
c_iov = iov_ie_append(iov, n_iov, c_iov, hs->supplicant_ie);
}
@@ -2888,7 +2849,7 @@ static struct l_genl_msg *netdev_build_cmd_associate_common(
l_genl_msg_append_attr(msg, NL80211_ATTR_SOCKET_OWNER, 0, NULL);
if (is_rsn)
- netdev_append_nl80211_rsn_attributes(msg, hs);
+ nl80211_append_rsn_attributes(msg, hs);
if (is_rsn || hs->settings_8021x) {
l_genl_msg_append_attr(msg, NL80211_ATTR_CONTROL_PORT,
diff --git a/src/nl80211util.c b/src/nl80211util.c
index ef69cc718e04..0f45c9051330 100644
--- a/src/nl80211util.c
+++ b/src/nl80211util.c
@@ -32,6 +32,8 @@
#include "src/nl80211util.h"
#include "src/band.h"
+#include "src/ie.h"
+#include "src/handshake.h"
#include "src/util.h"
typedef bool (*attr_handler)(const void *data, uint16_t len, void *o);
@@ -687,3 +689,41 @@ int nl80211_parse_supported_frequencies(struct l_genl_attr *band_freqs,
return 0;
}
+
+void nl80211_append_rsn_attributes(struct l_genl_msg *msg,
+ struct handshake_state *hs)
+{
+ uint32_t nl_cipher;
+ uint32_t nl_akm;
+ uint32_t wpa_version;
+
+ nl_cipher = ie_rsn_cipher_suite_to_cipher(hs->pairwise_cipher);
+ L_WARN_ON(!nl_cipher);
+ l_genl_msg_append_attr(msg, NL80211_ATTR_CIPHER_SUITES_PAIRWISE,
+ 4, &nl_cipher);
+
+ nl_cipher = ie_rsn_cipher_suite_to_cipher(hs->group_cipher);
+ L_WARN_ON(!nl_cipher);
+ l_genl_msg_append_attr(msg, NL80211_ATTR_CIPHER_SUITE_GROUP,
+ 4, &nl_cipher);
+
+ if (hs->mfp) {
+ uint32_t use_mfp = NL80211_MFP_REQUIRED;
+
+ l_genl_msg_append_attr(msg, NL80211_ATTR_USE_MFP, 4, &use_mfp);
+ }
+
+ nl_akm = ie_rsn_akm_suite_to_akm(hs->akm_suite);
+ L_WARN_ON(!nl_akm);
+ l_genl_msg_append_attr(msg, NL80211_ATTR_AKM_SUITES, 4, &nl_akm);
+
+ if (IE_AKM_IS_SAE(hs->akm_suite))
+ wpa_version = NL80211_WPA_VERSION_3;
+ else if (hs->wpa_ie)
+ wpa_version = NL80211_WPA_VERSION_1;
+ else
+ wpa_version = NL80211_WPA_VERSION_2;
+
+ l_genl_msg_append_attr(msg, NL80211_ATTR_WPA_VERSIONS,
+ 4, &wpa_version);
+}
diff --git a/src/nl80211util.h b/src/nl80211util.h
index 9f8ae17aeaa4..6f7b9eabbc27 100644
--- a/src/nl80211util.h
+++ b/src/nl80211util.h
@@ -25,6 +25,7 @@
struct band_chandef;
struct scan_freq_set;
struct band_freq_attrs;
+struct handshake_state;
int nl80211_parse_attrs(struct l_genl_msg *msg, int tag, ...);
@@ -90,3 +91,6 @@ int nl80211_parse_supported_frequencies(struct l_genl_attr *band_freqs,
struct scan_freq_set *supported_list,
struct band_freq_attrs *list,
size_t num_channels);
+
+void nl80211_append_rsn_attributes(struct l_genl_msg *msg,
+ struct handshake_state *hs);
--
2.42.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH v2 3/6] netdev: Don't duplicate vendor_ies
2023-11-27 4:38 [PATCH v2 1/6] ie: Move AKM suite converter from netdev Denis Kenzior
2023-11-27 4:38 ` [PATCH v2 2/6] nl80211util: Move nl80211_append_rsn_attributes Denis Kenzior
@ 2023-11-27 4:38 ` Denis Kenzior
2023-11-27 4:38 ` [PATCH v2 4/6] netdev: Do not leak l_genl_msg on error Denis Kenzior
` (3 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Denis Kenzior @ 2023-11-27 4:38 UTC (permalink / raw)
To: iwd; +Cc: Denis Kenzior
vendor_ies stored in handshake_state are already added as part of
netdev_populate_common_ies(), which is already invoked by
netdev_build_cmd_connect().
Normally vendor_ies is NULL for OWE connections, so no IEs are
duplicated as a result.
---
src/netdev.c | 7 +------
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/src/netdev.c b/src/netdev.c
index 1c032b32acdc..e7b502b1c3ba 100644
--- a/src/netdev.c
+++ b/src/netdev.c
@@ -2561,16 +2561,11 @@ static void netdev_cmd_connect_cb(struct l_genl_msg *msg, void *user_data)
static bool netdev_retry_owe(struct netdev *netdev)
{
- struct iovec iov;
-
if (!owe_next_group(netdev->owe_sm))
return false;
- iov.iov_base = netdev->handshake->vendor_ies;
- iov.iov_len = netdev->handshake->vendor_ies_len;
-
netdev->connect_cmd = netdev_build_cmd_connect(netdev,
- netdev->handshake, NULL, &iov, 1);
+ netdev->handshake, NULL, NULL, 0);
netdev->connect_cmd_id = l_genl_family_send(nl80211,
netdev->connect_cmd,
--
2.42.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH v2 4/6] netdev: Do not leak l_genl_msg on error
2023-11-27 4:38 [PATCH v2 1/6] ie: Move AKM suite converter from netdev Denis Kenzior
2023-11-27 4:38 ` [PATCH v2 2/6] nl80211util: Move nl80211_append_rsn_attributes Denis Kenzior
2023-11-27 4:38 ` [PATCH v2 3/6] netdev: Don't duplicate vendor_ies Denis Kenzior
@ 2023-11-27 4:38 ` Denis Kenzior
2023-11-27 4:38 ` [PATCH v2 5/6] netdev: Fix buffer overflow with 32 character ssids Denis Kenzior
` (2 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Denis Kenzior @ 2023-11-27 4:38 UTC (permalink / raw)
To: iwd; +Cc: Denis Kenzior
In netdev_retry_owe, if l_gen_family_send fails, the connect_cmd is
never freed or reset. Fix that.
While here, use a stack variable instead of netdev member, since the use
of such a member is unnecessary and confusing.
---
src/netdev.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/src/netdev.c b/src/netdev.c
index e7b502b1c3ba..901a41900350 100644
--- a/src/netdev.c
+++ b/src/netdev.c
@@ -2561,23 +2561,23 @@ static void netdev_cmd_connect_cb(struct l_genl_msg *msg, void *user_data)
static bool netdev_retry_owe(struct netdev *netdev)
{
+ struct l_genl_msg *connect_cmd;
+
if (!owe_next_group(netdev->owe_sm))
return false;
- netdev->connect_cmd = netdev_build_cmd_connect(netdev,
+ connect_cmd = netdev_build_cmd_connect(netdev,
netdev->handshake, NULL, NULL, 0);
- netdev->connect_cmd_id = l_genl_family_send(nl80211,
- netdev->connect_cmd,
+ netdev->connect_cmd_id = l_genl_family_send(nl80211, connect_cmd,
netdev_cmd_connect_cb, netdev,
NULL);
- if (!netdev->connect_cmd_id)
- return false;
-
- netdev->connect_cmd = NULL;
+ if (netdev->connect_cmd_id > 0)
+ return true;
- return true;
+ l_genl_msg_unref(connect_cmd);
+ return false;
}
static void netdev_connect_event(struct l_genl_msg *msg, struct netdev *netdev)
--
2.42.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH v2 5/6] netdev: Fix buffer overflow with 32 character ssids
2023-11-27 4:38 [PATCH v2 1/6] ie: Move AKM suite converter from netdev Denis Kenzior
` (2 preceding siblings ...)
2023-11-27 4:38 ` [PATCH v2 4/6] netdev: Do not leak l_genl_msg on error Denis Kenzior
@ 2023-11-27 4:38 ` Denis Kenzior
2023-11-27 4:38 ` [PATCH v2 6/6] erp: Fix buffer overflow for 32 byte SSIDs Denis Kenzior
2023-11-27 10:30 ` [PATCH v2 1/6] ie: Move AKM suite converter from netdev Marcel Holtmann
5 siblings, 0 replies; 7+ messages in thread
From: Denis Kenzior @ 2023-11-27 4:38 UTC (permalink / raw)
To: iwd; +Cc: Denis Kenzior
ssid is declared as a 32 byte field in handshake_state, hence using it
as a string which is assumed to be nul-terminated will fail for SSIDs
that are 32 bytes long.
Fixes: 1f1478285725 ("wiphy: add _generate_address_from_ssid")
Fixes: 5a1b1184fca6 ("netdev: support per-network MAC addresses")
---
src/netdev.c | 3 ++-
src/wiphy.c | 5 +++--
src/wiphy.h | 3 ++-
3 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/src/netdev.c b/src/netdev.c
index 901a41900350..208a15b94507 100644
--- a/src/netdev.c
+++ b/src/netdev.c
@@ -3526,7 +3526,8 @@ static int netdev_start_powered_mac_change(struct netdev *netdev)
/* No address set in handshake, use per-network MAC generation */
if (l_memeqzero(netdev->handshake->spa, ETH_ALEN))
wiphy_generate_address_from_ssid(netdev->wiphy,
- (const char *)netdev->handshake->ssid,
+ netdev->handshake->ssid,
+ netdev->handshake->ssid_len,
new_addr);
else
memcpy(new_addr, netdev->handshake->spa, ETH_ALEN);
diff --git a/src/wiphy.c b/src/wiphy.c
index 570f54155717..766df348754f 100644
--- a/src/wiphy.c
+++ b/src/wiphy.c
@@ -796,12 +796,13 @@ void wiphy_generate_random_address(struct wiphy *wiphy, uint8_t addr[static 6])
wiphy_address_constrain(wiphy, addr);
}
-void wiphy_generate_address_from_ssid(struct wiphy *wiphy, const char *ssid,
+void wiphy_generate_address_from_ssid(struct wiphy *wiphy,
+ const uint8_t *ssid, size_t ssid_len,
uint8_t addr[static 6])
{
struct l_checksum *sha = l_checksum_new(L_CHECKSUM_SHA256);
- l_checksum_update(sha, ssid, strlen(ssid));
+ l_checksum_update(sha, ssid, ssid_len);
l_checksum_update(sha, wiphy->permanent_addr,
sizeof(wiphy->permanent_addr));
l_checksum_get_digest(sha, addr, mac_randomize_bytes);
diff --git a/src/wiphy.h b/src/wiphy.h
index 999d0c57a926..bc82a00721e7 100644
--- a/src/wiphy.h
+++ b/src/wiphy.h
@@ -146,7 +146,8 @@ const uint8_t *wiphy_get_ht_capabilities(const struct wiphy *wiphy,
enum band_freq band,
size_t *size);
void wiphy_generate_random_address(struct wiphy *wiphy, uint8_t addr[static 6]);
-void wiphy_generate_address_from_ssid(struct wiphy *wiphy, const char *ssid,
+void wiphy_generate_address_from_ssid(struct wiphy *wiphy,
+ const uint8_t *ssid, size_t ssid_len,
uint8_t addr[static 6]);
int wiphy_estimate_data_rate(struct wiphy *wiphy,
--
2.42.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH v2 6/6] erp: Fix buffer overflow for 32 byte SSIDs
2023-11-27 4:38 [PATCH v2 1/6] ie: Move AKM suite converter from netdev Denis Kenzior
` (3 preceding siblings ...)
2023-11-27 4:38 ` [PATCH v2 5/6] netdev: Fix buffer overflow with 32 character ssids Denis Kenzior
@ 2023-11-27 4:38 ` Denis Kenzior
2023-11-27 10:30 ` [PATCH v2 1/6] ie: Move AKM suite converter from netdev Marcel Holtmann
5 siblings, 0 replies; 7+ messages in thread
From: Denis Kenzior @ 2023-11-27 4:38 UTC (permalink / raw)
To: iwd; +Cc: Denis Kenzior
ssid is declared as a 32 byte field in handshake_state, hence using it
as a string which is assumed to be nul-terminated will fail for SSIDs
that are 32 bytes long.
Fixes: d938d362b212 ("erp: ERP implementation and key cache move")
Fixes: 433373fe28a4 ("eapol: cache ERP keys on EAP success")
---
src/eapol.c | 2 +-
src/erp.c | 10 ++++++++--
src/erp.h | 2 +-
3 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/src/eapol.c b/src/eapol.c
index 6fb2f3068f0b..3d7b3d38fbcd 100644
--- a/src/eapol.c
+++ b/src/eapol.c
@@ -2531,7 +2531,7 @@ static void eapol_eap_results_cb(const uint8_t *msk_data, size_t msk_len,
if (sm->handshake->support_fils && emsk_data && session_id)
erp_cache_add(eap_get_identity(sm->eap), session_id,
session_len, emsk_data, emsk_len,
- (const char *)sm->handshake->ssid);
+ sm->handshake->ssid, sm->handshake->ssid_len);
return;
diff --git a/src/erp.c b/src/erp.c
index 2729cfc874b4..859233465e8b 100644
--- a/src/erp.c
+++ b/src/erp.c
@@ -160,13 +160,19 @@ static void erp_cache_entry_destroy(void *data)
void erp_cache_add(const char *id, const void *session_id,
size_t session_len, const void *emsk, size_t emsk_len,
- const char *ssid)
+ const uint8_t *ssid, size_t ssid_len)
{
struct erp_cache_entry *entry;
if (!unlikely(id || session_id || emsk))
return;
+ if (!util_ssid_is_utf8(ssid_len, ssid))
+ return;
+
+ if (util_ssid_is_hidden(ssid_len, ssid))
+ return;
+
entry = l_new(struct erp_cache_entry, 1);
entry->id = l_strdup(id);
@@ -174,7 +180,7 @@ void erp_cache_add(const char *id, const void *session_id,
entry->emsk_len = emsk_len;
entry->session_id = l_memdup(session_id, session_len);
entry->session_len = session_len;
- entry->ssid = l_strdup(ssid);
+ entry->ssid = l_strndup((char *) ssid, ssid_len);
entry->expire_time = l_time_offset(l_time_now(),
ERP_DEFAULT_KEY_LIFETIME_US);
diff --git a/src/erp.h b/src/erp.h
index d2c9da9624a8..e844aa8ebf90 100644
--- a/src/erp.h
+++ b/src/erp.h
@@ -43,7 +43,7 @@ const void *erp_get_rmsk(struct erp_state *erp, size_t *rmsk_len);
void erp_cache_add(const char *id, const void *session_id, size_t session_len,
const void *emsk, size_t emsk_len,
- const char *ssid);
+ const uint8_t *ssid, size_t ssid_len);
void erp_cache_remove(const char *id);
--
2.42.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH v2 1/6] ie: Move AKM suite converter from netdev
2023-11-27 4:38 [PATCH v2 1/6] ie: Move AKM suite converter from netdev Denis Kenzior
` (4 preceding siblings ...)
2023-11-27 4:38 ` [PATCH v2 6/6] erp: Fix buffer overflow for 32 byte SSIDs Denis Kenzior
@ 2023-11-27 10:30 ` Marcel Holtmann
5 siblings, 0 replies; 7+ messages in thread
From: Marcel Holtmann @ 2023-11-27 10:30 UTC (permalink / raw)
To: Denis Kenzior; +Cc: iwd
Hi Denis,
> It is more logical to host this function inside ie.c than netdev.c.
> Particularly since ie_rsn_cipher_suite_to_cipher is already present in
> ie.c.
> ---
> src/ie.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
> src/ie.h | 2 ++
> src/netdev.c | 47 +----------------------------------------------
> 3 files changed, 49 insertions(+), 46 deletions(-)
all 6 patches haven been applied.
Regards
Marcel
^ permalink raw reply [flat|nested] 7+ messages in thread