| Message ID | 542ACC15.2080202@aminocom.com |
|---|---|
| State | Changes Requested |
| Headers | show |
On Tue, Sep 30, 2014 at 04:28:21PM +0100, Michalis Pappas wrote: > During WPS registrar authentication, wpa_supplicant won't complete the > negotiation if the newly authorized registrar does not wish to push an updated > configuration to the AP. Instead, upon reception of M7, wpa_supplicant will > terminate the negotiation with a WPN_NACK and issue a WPS-FAIL event. Nevertheless, > later it will connect to the AP using the received credentials. This is done by design to avoid reconfiguring the AP which can result in the AP rebooting or at least disconnecting all stations when WPS is used just to learn the current settings. > This behaviour seems to be broken both semantically and from the standard's point of > view. WPS-FAIL will confuse any applications interfacing with wpa_supplicant to think > that the negotiation failed. Moreover, according to the standard, M8 is not > optional, so WPS_NACK is expected to be sent only when failing to authenticating > or processing a message. WSC_NACK can be sent as a response to any M2..M8 message. > This patch gets wpa_supplicant complete the WPS negotiation and issue a WPS-SUCCESS. This would break the intended behavior. I guess I could consider changing the WPS-FAIL event to WPS-SUCCESS in this case where the AP settings have been learned and the requested operation was only for learning the current settings. However, that would be a change in a behavior that has remained this way for five years, so I'm not sure this would really be justifiable since there may be existing implementations that expect to see WPS-CRED-RECEIVED followed by WPS-FAIL in this specific case. The WPS protocol itself does indeed fail here, but that is on purpose for the learn-current-settings case.
diff --git a/src/wps/wps_registrar.c b/src/wps/wps_registrar.c index b90cc25..d28b153 100644 --- a/src/wps/wps_registrar.c +++ b/src/wps/wps_registrar.c @@ -2814,19 +2814,11 @@ static int wps_process_ap_settings_r(struct wps_data *wps, wpa_printf(MSG_INFO, "WPS: Update AP configuration based on " "new settings"); wps_cred_update(&wps->cred, wps->new_ap_settings); - return 0; } else { /* * Use the AP PIN only to receive the current AP settings, not * to reconfigure the AP. */ - - /* - * Clear selected registrar here since we do not get to - * WSC_Done in this protocol run. - */ - wps_registrar_pin_completed(wps->wps->registrar); - msg = wps_build_ap_cred(wps); if (msg == NULL) return -1; @@ -2844,9 +2836,8 @@ static int wps_process_ap_settings_r(struct wps_data *wps, wps->cred.cred_attr = NULL; wps->cred.cred_attr_len = 0; wpabuf_free(msg); - - return 1; } + return 0; }
During WPS registrar authentication, wpa_supplicant won't complete the negotiation if the newly authorized registrar does not wish to push an updated configuration to the AP. Instead, upon reception of M7, wpa_supplicant will terminate the negotiation with a WPN_NACK and issue a WPS-FAIL event. Nevertheless, later it will connect to the AP using the received credentials. This behaviour seems to be broken both semantically and from the standard's point of view. WPS-FAIL will confuse any applications interfacing with wpa_supplicant to think that the negotiation failed. Moreover, according to the standard, M8 is not optional, so WPS_NACK is expected to be sent only when failing to authenticating or processing a message. This patch gets wpa_supplicant complete the WPS negotiation and issue a WPS-SUCCESS. Signed-off-by: Michalis Pappas <mpappas@aminocom.com> --- src/wps/wps_registrar.c | 11 +---------- 1 files changed, 1 insertions(+), 10 deletions(-) -- 1.7.2.5.amino.1