[PATCH] wpa_supplicant: Resolve secondary VAP association issue

[Jouni Malinen]

On Sun, Jul 03, 2022 at 05:37:54PM -0700, Cy Schubert wrote:
> Association will fail on a secondary open unprotected VAP when the
> primary VAP is configured for WPA. Examples of secondary VAPs are,
> hotels, universities, and commodity routers' guest networks.
> 
> A broadly similar bug was discussed on Red Hat's bugzilla affecting
> association to a D-Link DIR-842.
> 
> This suggests that as IEs were added to the 802.11 protocol the old code
> was increasingly inadaquate to handle the additional IEs, not only a
> secondary VAP.
> 
> FreeBSD PR:		264238

> diff --git a/src/drivers/driver_bsd.c b/src/drivers/driver_bsd.c

> +static int
> +wpa_driver_bsd_set_rsn_wpa_ie(struct bsd_driver_data * drv,
> +    struct wpa_driver_associate_params *params, const u8 *ie)
> +{
> +	int privacy;
> +	size_t ie_len = ie[1] ? ie[1] + 2 : 0;
> +
> +	/* XXX error handling is wrong but unclear what to do... */
> +	if (wpa_driver_bsd_set_wpa_ie(drv, ie, ie_len) < 0)
> +		return -1;

So this part was just moving the call to a separate function..

> +	privacy = !(params->pairwise_suite == WPA_CIPHER_NONE &&
> +	    params->group_suite == WPA_CIPHER_NONE &&
> +	    params->key_mgmt_suite == WPA_KEY_MGMT_NONE);

While there is a change here: condition of params->wpa_ie_len == 0 was
removed. And well, this function would not actually be called without an
IE, so that would have been just forcing privacy = 1.

> +	wpa_printf(MSG_DEBUG, "%s: set PRIVACY %u", __func__,
> +	    privacy);
> +
> +	if (set80211param(drv, IEEE80211_IOC_PRIVACY, privacy) < 0)
> +		return -1;

I'm not completely sure how exactly the IEEE80211_IOC_PRIVACY parameter
is supposed to behave, but it would feel very strange to me not to
expect there to be privacy protection with WPA IE/RSNE included. That
said, it looks like privacy would really be hardcoded to 1 here anyway
for all the cases that end up calling this function, so I cannot really
follow what this is trying to do.

> +	if (ie_len &&
> +	    set80211param(drv, IEEE80211_IOC_WPA,
> +	    ie[0] == WLAN_EID_RSN ? 2 : 1) < 0)
> +		return -1;

This is just from moving to a separate function.

>  static int
>  wpa_driver_bsd_associate(void *priv, struct wpa_driver_associate_params 
> *params)

>  	if (wpa_driver_bsd_set_auth_alg(drv, params->auth_alg) < 0)
>  		ret = -1;
> -	/* XXX error handling is wrong but unclear what to do... */
> -	if (wpa_driver_bsd_set_wpa_ie(drv, params->wpa_ie, params->wpa_ie_len) < 
> 0)
> -		return -1;

This call was practically made condition on there being either a WPA IE
or an RSNE in the IE buffer.

> -	privacy = !(params->pairwise_suite == WPA_CIPHER_NONE &&
> -	    params->group_suite == WPA_CIPHER_NONE &&
> -	    params->key_mgmt_suite == WPA_KEY_MGMT_NONE &&
> -	    params->wpa_ie_len == 0);
> -	wpa_printf(MSG_DEBUG, "%s: set PRIVACY %u", __func__, privacy);
> -
> -	if (set80211param(drv, IEEE80211_IOC_PRIVACY, privacy) < 0)
> -		return -1;

This is the call that would not be issued anymore unless either a WPA IE
or an RSNE is included. Is that on purpose? What about WEP?

> +	if (params->wpa_ie_len) {
> +		rsn_ie = get_ie(params->wpa_ie, params->wpa_ie_len,
> +		    WLAN_EID_RSN);
> +		if (rsn_ie) {
> +			if (wpa_driver_bsd_set_rsn_wpa_ie(drv, params,
> +			    rsn_ie) < 0)
> +				return -1;
> +		}

Is this filtering out any other potential information element from the
buffer other than RSNE? If so, why?

> +			wpa_ie = get_vendor_ie(params->wpa_ie,
> +			    params->wpa_ie_len, WPA_IE_VENDOR_TYPE);
> +			if (wpa_ie) {
> +				if (wpa_driver_bsd_set_rsn_wpa_ie(drv, params,
> +				    wpa_ie) < 0)
> +					return -1;
> +			}

And same here for filtering anything other than WPA IE (not that I would
expect this part to make much of a difference)?

-- 
Jouni Malinen                                            PGP id EFC895FA