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

[Jithu Jance]

>>> In this particular sequence, I don't really see much point in even
>>> displaying the P2P-PROV-DISC-SHOW-PIN event. In fact, it does not
>>> necessarily even get shown if the Provision Discovery Response is not
>>> received

> but the UI program that issued the
> p2p_connect command in the first place better know which end is going to
> be displaying the PIN and it should be able to figure out easily that
> the value returned by p2p_connect is this.

> there is no
> guarantee on this event being shown in this join-a-running-group case.. 

Sounds like for "p2p_connect <devaddr> pin display join" command, suppressing Prov Disc SHOW-PIN would be a better
approach. 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??


Signed-hostap: Jithu Jance <jithu at broadcom.com>

---
 wpa_supplicant/p2p_supplicant.c |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/wpa_supplicant/p2p_supplicant.c b/wpa_supplicant/p2p_supplicant.c
index 5ff94ef..85be08a 100644
--- 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,
 		    group ? " group=" : "",
 		    group ? group->ifname : "");
 	params[sizeof(params) - 1] = '\0';
-
-	if (config_methods & WPS_CONFIG_DISPLAY) {
+	if ((config_methods & WPS_CONFIG_DISPLAY) && (wpa_s->p2p_pin[0] == '\0')) {
 		generated_pin = wps_generate_pin();
 		wpas_prov_disc_local_display(wpa_s, peer, params,
 					     generated_pin);
@@ -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)
-- 
1.7.4.1




- Jithu Jance


-----Original Message-----
From: hostap-bounces at lists.shmoo.com [mailto:hostap-bounces at lists.shmoo.com] On Behalf Of Jouni Malinen
Sent: Sunday, December 18, 2011 11:49 PM
To: hostap at lists.shmoo.com
Subject: Re: [PATCH] P2P: Fix inconsistency in displaying wps pin

On Sun, Dec 11, 2011 at 09:35:58PM -0800, Jithu Jance wrote:
> Sorry that I didn't understand the above context properly. UI Application listens?
> on P2P-PROV-DISC-SHOW-PIN ctrl event to?display the pin to user and the peer
> uses this pin to activate wps pin session on his device. So without SHOW-PIN ctrl event, 
> is there a?way to let the application know the about wps_pin to be displayed?
> 
> Do you mean that the pin returned by "p2p_connect pin display join" command should be used 
> instead of waiting for SHOW-PIN event?? 

Yes.

> But this approach is inconsistent with the explicit provision discovery req - resp sequence used when the peer
> is using keypad. In this case local device should display the pin and this display is done on SHOW-PIN event.

Sure, this is different sequence, but the UI program that issued the
p2p_connect command in the first place better know which end is going to
be displaying the PIN and it should be able to figure out easily that
the value returned by p2p_connect is this.

> I understand that strict waiting for provision disc resp is not done during "p2p_connect join". This waiting is not necessary
> when local device uses keypad/pbc option. But consider a scenario where local device chooses pin display 
> method where the peer is expected to enter the PIN which local device displays., Here local device requires some event to
> let the application know about the PIN it uses. Does it make sense to mandate pd response if the local device uses display
> method??

Waiting for the Provision Discovery Response frame is not really
required for any case - displaying the correct PIN is. The correct PIN
is the one that was used (or returned) with p2p_connect.

> > This is a bit problematic since wpa_s->p2p_pin does not get cleared and
> > the same PIN would be used in number of operations where it was not
> > supposed to be used.
> ?
> Would the patch make sense, if the p2p_pin is cleared properly after?
> wpas_start_wps_enrollee.???

It could still be problematic in some sequences. I don't have anything
against making the P2P-PROV-DISC-SHOW-PIN value match with the
p2p_connect return value, but it needs to be understood that there is no
guarantee on this event being shown in this join-a-running-group case.
Since the real PIN that gets used during the provisioning step is the
one used with p2p_connect, any change to the provision discovery events
needs to be done with the main use case with p2p_connect in mind. It is
much more important for that to work than any partial improvement for
the provision discovery events.

-- 
Jouni Malinen                                            PGP id EFC895FA
_______________________________________________
HostAP mailing list
HostAP at lists.shmoo.com
http://lists.shmoo.com/mailman/listinfo/hostap