Hi Jouni, On Tue, Jan 08, 2019 at 01:04:37PM +0200, Jouni Malinen wrote: > On Mon, Jan 07, 2019 at 03:56:24PM +0100, Arnout Vandecappelle wrote: > > On 20/12/2018 11:19, Jouni Malinen wrote: > > > that it would be better to indicate this as a parameter > > > specific to each instance of WPS provisioning, i.e., as a new optional > > > argument to the WPS_PBC control interface command. > > > > That makes sense. I'm sure you have good reasons for requesting it do be done in that way. It'd be great if you can elaborate on those reasons, see below for the cause of my confusion with that. > > > > > In other words, > > > wpas_wps_start_pbc() should get this from the caller > > > > So an extra argument to wpas_wps_start_pbc, right? > > Yes. I've been going that road for a bit now and more and more come to the conclusion that setting a global variable (just like other WPS- related global configurations) would be much less complex. Also note that a station is most likely really either a regular station or multi-ap backhaul station, also considering the context: multi-ap interface in 4-addr-mode sitting in a bridge while regular station being a whole different use-case. Coming from OpenWrt, whether the interface is a multiap backhaul sta or a regular sta is known at the time of wpa_supplicant.conf being generated. Sure, at this point we could store that somewhere next to wpa_supplicant_$ifname.conf and when pushing the WPS butten checking that file to decide whether multi_ap=1 is being passed along the call of wps_start_pbc or not. A more dynamic decission doesn't make much sense, because higher layers are also already set according to whether the STA interface is part of a bridge or not. > > > I guess then it should be added also to: > > > > * wpa_cli; > > wpa_cli is already accepting arbitrary number of arguments to WPS_PBC. > > > * new D-Bus interface; > > If we expect this functionality to be used over D-Bus, sure. > > > * (for OpenWrt only: ubus interface); > > Sounds reasonable. > > > but not to: > > > > * old D-Bus interfae; > > * p2p enrollment. > > Agreed. > > > And I'm not sure what to do with EVENT_WPS_BUTTON_PUSHED. But the only caller > > of that is apparently some atheros driver. > > I'd ignore that for this, i.e., that wpas_wps_start_pbc() call should > just note that there was no request for Multi-AP. > > > > and > > > wpa_supplicant_wps_cred() should get this from a new field added into > > > struct wps_credential. > > > > Wouldn't struct wps_data be more appropriate? It's not very clear to me where > > that gets initialized though... But actually, the same goes for wps_credential. > > wpa_supplicant_wps_cred() gets called with information parsed from M8, > so this should be filled based on what the Registrar told us. I don't > see why struct wps_data would be involved in that part (i.e., processing > of a received Credential). > > > Is it possible that both of these only get initialized after > > wpas_wps_start_pbc() has already returned? > > Yes and not only possible, but that will happen in practice every time. > > > Maybe the best way to pass it on would be through the phase1 config string? > > Although it feels like a bit of a hack to me, it looks like that is the way to > > pass down context information into EAP, right? > > So this is a bit different direction, i.e., configuration of the local > WPS Enrollee behavior for the request information while the comment > about wpa_supplicant_wps_cred() was on how to process the response > information. Anyway, yes, this is a bit complex due to the way the EAP > peer implementation is kept as a separate module between core > wpa_supplicant and WPS module. I did not consider details for that part, > but yes, a new parameter within the phase1 config string seems to make > most sense for getting information about the new optional WPS_PBC > argument to the actual WPS Enrollee implementation which needs to > generate the WSC messages with some additional information. That kinda adds to it complexity wise... Cheers Daniel > > -- > Jouni Malinen PGP id EFC895FA > > _______________________________________________ > Hostap mailing list > Hostap@xxxxxxxxxxxxxxxxxxx > http://lists.infradead.org/mailman/listinfo/hostap _______________________________________________ Hostap mailing list Hostap@xxxxxxxxxxxxxxxxxxx http://lists.infradead.org/mailman/listinfo/hostap