On Thu, Oct 12, 2023 at 02:59:29PM +0530, Vinayak Yadawad wrote: > In the current change we enable support for SAE offload by > 1. Adding support of 4-way HS offload for AP interface > 2. Adding support for SAE authentication offload > The changes basically involve handling of necessary parameter > plumbing to the driver in the connect path for STA interface. > Also the changes involve AP parameter plumbing in AP bringup > path and Port authorized event handling. SAE offload for STA mode was added in an earlier patch: https://w1.fi/cgit/hostap/commit/?id=c3b8452e0e04038349fbfe757639ff3b377e98da https://w1.fi/cgit/hostap/commit/?id=236c0cfbcdf7459be1727afb7eb540c4ee081cbc So that part would need some small changes here. As far as the AP mode case is concerned, does this work only if both SAE and the following 4-way handshake are offloaded? Or would there be possibility for offload SAE and performing 4-way handshake from hostapd (with the PMK from the driver provided to user space)? It would be good for the commit message to be clear on this detail. > diff --git a/src/ap/beacon.c b/src/ap/beacon.c > @@ -2011,6 +2012,36 @@ int ieee802_11_build_ap_params(struct hostapd_data *hapd, > + /* If SAE offload is enabled, provide passphrase to lower layer for > + * PMK generation > + */ > + if ((wpa_key_mgmt_sae(hapd->conf->wpa_key_mgmt)) && > + (hapd->iface->drv_flags2 & WPA_DRIVER_FLAGS2_SAE_OFFLOAD_AP)) { > + if (hapd->conf->ssid.wpa_passphrase) { > + params->sae_passphrase_len = > + os_strlen(hapd->conf->ssid.wpa_passphrase); > + if (params->sae_passphrase_len) { > + os_memcpy(params->sae_passphrase, > + hapd->conf->ssid.wpa_passphrase, > + params->sae_passphrase_len); > + } > + } > + if (hapd->conf->sae_pwe) { > + params->sae_pwe = hapd->conf->sae_pwe; > + } > + } There is no "SAE passphrase". It is "SAE password". Please note that hostapd supports multiple SAE passwords and selection of which one to use based on either STA MAC address or SAE password identifier. Even if the SAE-in-driver case would not support those options, the implementation hostapd would need to check for these being enabled in the configuration and if so, refuse to try to configure SAE offload that won't work in a way that was configured. By default, this should use hapd->conf->sae_passwords and fall back to hapd->conf->ssid.wpa_passphrase only if no SAE-specific password is configured. > + /* If key mgmt offload is enabled, configure PSK */ > + if ((hapd->conf->wpa_key_mgmt & WPA_KEY_MGMT_PSK) && > + (hapd->iface->drv_flags2 & WPA_DRIVER_FLAGS2_4WAY_HANDSHAKE_AP_PSK)) { > + if (hapd->conf->ssid.wpa_psk && hapd->conf->ssid.wpa_psk_set) { > + os_memcpy(params->psk, hapd->conf->ssid.wpa_psk->psk, PMK_LEN); > + } else if (hapd->conf->ssid.wpa_passphrase) { > + pbkdf2_sha1(hapd->conf->ssid.wpa_passphrase, hapd->conf->ssid.ssid, > + hapd->conf->ssid.ssid_len, 4096, params->psk, PMK_LEN); > + } > + } The commit title identified this as adding SAE offload, but this seems to be adding WPA2-Personal (PSK) offload.. It would be better to split these into two separate patches to make the exact applicable changes easier to understand. > diff --git a/src/ap/wpa_auth_ie.c b/src/ap/wpa_auth_ie.c > +#include "wpa_auth_glue.h" That is not supposed to done in wpa_auth_ie.c, i.e., the WPA authenticator functionality is supposed to be generic that does not depend on hostapd-specific implementation. > #ifdef CONFIG_SAE > - if (sm->wpa_key_mgmt == WPA_KEY_MGMT_SAE && data.num_pmkid && > - !sm->pmksa) { > + if (sm->wpa_key_mgmt == WPA_KEY_MGMT_SAE && > + !(hostapd_wpa_auth_is_sae_offload_enabled(wpa_auth->cb_ctx)) && > + data.num_pmkid && !sm->pmksa) { In other words, this direct hostapd_wpa_auth_is_sae_offload_enabled() call is expected to be done using struct wpa_auth_callbacks instead to avoid direct dependency on hostapd-specific wpa_auth_glue.c. > diff --git a/src/drivers/driver.h b/src/drivers/driver.h > @@ -69,6 +69,8 @@ enum hostapd_chan_width_attr { > +#define MAX_PASSPHRASE_LEN 63 > @@ -1785,6 +1787,21 @@ struct wpa_driver_ap_params { > + /** > + * sae_passphrase - sae passphrase for SAE offload > + */ > + u8 sae_passphrase[MAX_PASSPHRASE_LEN]; That should be called sae_password. 63 is not the correct length limit for this. Why would this even need to be a fixed length array instead of being const pointer to the password in some upper layer struct owned by the caller? > @@ -2253,6 +2270,12 @@ struct wpa_driver_capa { > +/** Driver support AP_PSK authentication offload */ > +#define WPA_DRIVER_FLAGS2_4WAY_HANDSHAKE_AP_PSK 0x0000000000008000ULL This should be in a separate patch. > +/** Driver support SAE STA authentication offload */ > +#define WPA_DRIVER_FLAGS2_SAE_OFFLOAD 0x0000000000010000ULL This is already included in hostap.git. > +/** Driver support SAE AP authentication offload */ > +#define WPA_DRIVER_FLAGS2_SAE_OFFLOAD_AP 0x0000000000020000ULL > u64 flags2; And this would be in the AP mode SAE offload patch. > @@ -6645,6 +6668,7 @@ union wpa_event_data { > struct port_authorized { > const u8 *td_bitmap; > size_t td_bitmap_len; > + u8 sta_addr[ETH_ALEN]; > } port_authorized; This should now really be documented since the previous uses of struct port_authorized were for STA mode while this last one is for AP mode and such a mix of parameters is quite confusing without clear documentation. > diff --git a/src/drivers/driver_nl80211.c b/src/drivers/driver_nl80211.c > @@ -6926,6 +6937,17 @@ static int nl80211_connect_common(struct wpa_driver_nl80211_data *drv, > + /* Add SAE passphrase in case of SAE offload */ > + if (wpa_key_mgmt_sae(params->key_mgmt_suite) && > + (drv->capa.flags2 & WPA_DRIVER_FLAGS2_SAE_OFFLOAD) && > + params->passphrase) { > + wpa_hexdump_key(MSG_DEBUG, " * SAE passphrase", > + params->passphrase, os_strlen(params->passphrase)); > + if (nla_put(msg, NL80211_ATTR_SAE_PASSWORD, > + os_strlen(params->passphrase), params->passphrase)) > + return -1; > + } This is in incorrect function (i.e., this should be only for CONNECT, not for ASSOCIATE). Anyway, the STA case is already covered, so no need for this anymore. > diff --git a/src/drivers/driver_nl80211_event.c b/src/drivers/driver_nl80211_event.c > @@ -3505,7 +3505,14 @@ static void nl80211_port_authorized(struct wpa_driver_nl80211_data *drv, > } > > addr = nla_data(tb[NL80211_ATTR_MAC]); > - if (os_memcmp(addr, drv->bssid, ETH_ALEN) != 0) { > + if (is_ap_interface(drv->nlmode) && drv->device_ap_sme) { > + /* Update STA assoc address */ > + os_memcpy(event.port_authorized.sta_addr, addr, ETH_ALEN); > + wpa_printf(MSG_DEBUG, > + "nl80211: Port authorized for STA BSSID " MACSTR, > + MAC2STR(addr)); > + } else if (is_sta_interface(drv->nlmode) && > + os_memcmp(addr, drv->bssid, ETH_ALEN) != 0) { > wpa_printf(MSG_DEBUG, > "nl80211: Ignore port authorized event for " MACSTR > " (not the currently connected BSSID " MACSTR ")", Is this use for NL80211_CMD_PORT_AUTHORIZED for AP mode documented somewhere? nl80211.h does not seem to describe such use case at all. The documentation implies this even to be for STA mode only even if interpreted more widely to cover AP mode, it would be only for 802.1X FT. > diff --git a/wpa_supplicant/wpa_supplicant.c b/wpa_supplicant/wpa_supplicant.c > @@ -4269,6 +4269,21 @@ static void wpas_start_assoc_cb(struct wpa_radio_work *work, int deinit) > params.key_mgmt_suite == WPA_KEY_MGMT_IEEE8021X_SUITE_B_192)) > params.req_handshake_offload = 1; > > + params.sae_pwe = wpa_s->conf->sae_pwe; > + if ((wpa_s->drv_flags & WPA_DRIVER_FLAGS_SAE) && > + (wpa_key_mgmt_sae(params.key_mgmt_suite))) { > + if (ssid->sae_password) { > + wpa_printf(MSG_DEBUG, "sae enabled join..Using sae_password"); > + params.passphrase = ssid->sae_password; > + } else if (ssid->passphrase) { > + wpa_printf(MSG_DEBUG, "sae enabled join..Using passphrase"); > + params.passphrase = ssid->passphrase; > + } else { > + wpa_printf(MSG_ERROR, "sae enabled join.." > + "But neither sae_password nor passphrase set"); > + } > + } No need for this change anymore. -- Jouni Malinen PGP id EFC895FA _______________________________________________ Hostap mailing list Hostap@xxxxxxxxxxxxxxxxxxx http://lists.infradead.org/mailman/listinfo/hostap
