[PATCH 1/1] hostapd: Add support for SAE offload for STA/AP interface

[Jouni Malinen]

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