FW: [PATCH] P2P: Fix inconsistency in displaying wps pin

[Jouni Malinen]

On Mon, Dec 26, 2011 at 09:23:54PM -0800, Jithu Jance wrote:
> Sounds like for "p2p_connect <devaddr> pin display join" command, suppressing Prov Disc SHOW-PIN would be a better
> approach.

Agreed.

> I would also like to confirm about the handling of wpas_notify_p2p_provision_discovery
> in this particular scenario. Is it okay to invoke the function with generated_pin = 0 value (the below patch uses this approach)
>  or should I suppress the invocation of wpas_notify_p2p_provision_discovery when wpas->p2p_pin is populated??

I think it is better to skip this call completely. Actually, these
provision discovery response notifications should really be skipped
completely in the automatic PD-before-join case since those were not
explicitly requested by any external program and they do not really
provide any additional information.

> diff --git a/wpa_supplicant/p2p_supplicant.c b/wpa_supplicant/p2p_supplicant.c
> @@ -1785,8 +1785,7 @@ void wpas_prov_disc_req(void *ctx, const u8 *peer, u16 config_methods,
> -
> -	if (config_methods & WPS_CONFIG_DISPLAY) {
> +	if ((config_methods & WPS_CONFIG_DISPLAY) && (wpa_s->p2p_pin[0] == '\0')) {
>  		generated_pin = wps_generate_pin();

Why would we need to modify prov_disc_req callback? This is not used on
the P2P device that will be joining a group as a P2P client.

> @@ -1809,7 +1808,7 @@ void wpas_prov_disc_resp(void *ctx, const u8 *peer, u16 config_methods)
>  	if (config_methods & WPS_CONFIG_DISPLAY)
>  		wpas_prov_disc_local_keypad(wpa_s, peer, "");
> -	else if (config_methods & WPS_CONFIG_KEYPAD) {
> +	else if ((config_methods & WPS_CONFIG_KEYPAD) && (wpa_s->p2p_pin[0] == '\0')) {
>  		generated_pin = wps_generate_pin();
>  		wpas_prov_disc_local_display(wpa_s, peer, "", generated_pin);
>  	} else if (config_methods & WPS_CONFIG_PUSHBUTTON)

While this could be fine for most cases, I don't really want to depend
on wpa_s->p2p_pin getting cleared in all cases. I guess this could
potentially skip an event message for Provision Discovery that is done
in other contexts.

I implemented this a bit differently in commit
c19316354ed46e688704c1ebcf1c7efe816ddf31 by skipping the PD Response
notifications in PD-before-join case regardless of what config method
was requested. This looks like the most consistent and cleanest approach
here.

This will also require another commit for some cases where the PD
Response could have been processed after having already started the scan
for the join operation. Commit f63b8542156496ba88ef9f161e5931122d7319b9
makes wpa_supplicant wait for the PD Response prior to continuing the
join with the scan-for-connection step. This is also cleaning up the
sequence of driver operations in the join step and can make it somewhat
easier for some drivers to handle the remain-on-channel/offchannel TX
and scan.
 
-- 
Jouni Malinen                                            PGP id EFC895FA