[PATCH] wpa_supplicant: Resolve secondary VAP association issue

[Cy Schubert]

Sorry for the late reply. This slipped off my radar, by other competing 
FreeBSD and non-FreeBSD work.

In message <20221127174159.GB1443811 at w1.fi>, Jouni Malinen writes:
> 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..

Yes, easier to compare.

>
> > +	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.

The problem solved is the secondary open AP, with a primary WPA AP, will 
also have a privacy IE. Even though it is open.

>
> > +	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.

To me at the time it appeared as if the privacy parameter was leaking from 
the private primary AP to the open secondary AP. It's not quite valid when 
the secondary AP is open and its privacy is set.

>
> > +	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.

No. The secondary open AP will send a privacy IE even though it is open. We 
fix it the same way Red Hat did.

>
> >  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.

The problem was that the open secondary AP (because of its WPA primary AP) 
sends a WPA IE regardless.

>
> > -	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?

Again the purpose of this is not to assume privacy is set just because 
we've received an erroneous IE.

>
> > +	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?

Again, the erroneous IE sent by the secondary open AP.

>
> > +			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

This is similar to a bug discussed on Red Hat's bugzilla with association 
to DIR-8482. The original problem was WRT association to university and 
hotel APs -- which I assume were configured with WPA primary AP for staff 
and open secondary AP for students or guests. I was able to reproduce this 
each with two WNDR3700v3's with the primary AP configured for WPA and the 
secondary AP configured open.

The patch has been in FreeBSD since.

Again, sorry for the late reply. I've been pulled away from this 
problem/project to another, to another... My apologies for letting this 
drop.


-- 
Cheers,
Cy Schubert <Cy.Schubert at cschubert.com>
FreeBSD UNIX:  <cy at FreeBSD.org>   Web:  https://FreeBSD.org
NTP:           <cy at nwtime.org>    Web:  https://nwtime.org

			e^(i*pi)+1=0