[RFC 38/40] WPS: add WPS Vendor Extension attribute in Beacon Frame

[Johannes Berg]

On Thu, 2011-03-17 at 14:02 +0200, Jouni Malinen wrote:
> On Wed, Jan 05, 2011 at 08:53:34PM +0100, Johannes Berg wrote:
> > WPS vendor extension can be set in hapd->conf  structure
> > when AP( Group Owner) is configured after p2p groupd has been started.
> > WPS vendor extension can also be updated through the
> > hostapd_update_wps function call in wps_hostapd.c.
> > Number of WPS Vendor Extension is currenlty limited to 1 (defined
> > with WPS_MAX_VENDOR_EXTENSIONS_IN_BEACON macro.
> 
> Why is this added to WSC IEs in Beacon frames, but not in Probe Response
> frames?

I believe that, unfortunately, they're implementing a protocol on top
that has different information in probe responses and beacons. The
commit log here is quite out of date though -- there's no limit of 1 any
more, that was a side effect of a stupid DBus implementation.

> > diff --git a/src/wps/wps_registrar.c b/src/wps/wps_registrar.c
> > @@ -1113,7 +1113,8 @@ static int wps_set_ie(struct wps_registrar *reg)
> >  	    wps_build_device_attrs(&reg->wps->dev, probe) ||
> >  	    wps_build_probe_config_methods(reg, probe) ||
> >  	    wps_build_rf_bands(&reg->wps->dev, probe) ||
> > -	    wps_build_wfa_ext(probe, 0, auth_macs, count)) {
> > +	    wps_build_wfa_ext(probe, 0, auth_macs, count) ||
> > +	    wps_build_vendor_ext(&reg->wps->dev, beacon)) {
> 
> This looks like an odd place for adding something to the beacon
> wpabuf... This is at the end of Probe Request buffer construction!

Hah. Yeah that's confusing. Could be a merge error, or more likely I
wasn't paying attention when I looked at the ordering.

> Anyway, the worse part here is that there does not seem to be any bounds
> checking or code for increasing the size of the beacon wpabuf based on
> the length of the vendor extensions. These extensions could be quite
> long and will result in wpa_supplicant/hostapd terminating if they go
> beyond the 400 octet buffer that is reserved for the WSC IE in this
> function..

Indeed, 400 are reserved a little above this. I suppose we should add
the length of the vendor ext data.

I'll also check this in the other pieces that might touch this.

johannes