[PATCH] wpa_supplicant: Use nl80211_send_eapol_data for station

[Peer, Ilan]

HI,

Couple of comments below (in addition to changing this to use the nl80211 transport as discussed).

> -----Original Message-----
> From: Hostap [mailto:hostap-bounces at lists.infradead.org] On Behalf Of
> Wojciech Dubowik
> Sent: Friday, June 09, 2017 12:22
> To: hostap at lists.infradead.org
> Cc: Wojciech Dubowik <Wojciech.Dubowik at neratec.com>
> Subject: [PATCH] wpa_supplicant: Use nl80211_send_eapol_data for station
> 
> Supplicant is using generic L2 send function for EAPOL messages which
> doesn't give back status whether frame has been acked or not. It can lead to
> wrong wpa states when EAPOL 4/4 is lost i.e. client is in connected state but
> keys aren't established on AP side.
> Fix that by using nl80211_send_eapol_data as for AP side and check in
> conneced state that 4/4 EAPOL has been acked.
> 
> Signed-off-by: Wojciech Dubowik <Wojciech.Dubowik at neratec.com>
> ---
>  src/drivers/driver.h         | 12 ++++++++++++
>  src/drivers/driver_nl80211.c | 11 +++++++++++
>  wpa_supplicant/driver_i.h    | 10 ++++++++++
>  wpa_supplicant/events.c      | 17 ++++++++++++++++-
>  wpa_supplicant/wpas_glue.c   |  6 ++++++
>  5 files changed, 55 insertions(+), 1 deletion(-)

...

> 
> diff --git a/wpa_supplicant/events.c b/wpa_supplicant/events.c index
> 3a2ec64..af9bf2a 100644
> --- a/wpa_supplicant/events.c
> +++ b/wpa_supplicant/events.c
> @@ -3966,13 +3966,28 @@ void wpa_supplicant_event(void *ctx, enum
> wpa_event_type event,
>  		}
>  #endif /* CONFIG_AP */
>  		break;
> -#ifdef CONFIG_AP
>  	case EVENT_EAPOL_TX_STATUS:
> +#ifdef CONFIG_AP
>  		ap_eapol_tx_status(wpa_s, data->eapol_tx_status.dst,
>  				   data->eapol_tx_status.data,
>  				   data->eapol_tx_status.data_len,
>  				   data->eapol_tx_status.ack);
> +#else

This would not work in case that CONFIG_AP is set (wpa_supplicant support for AP/P2P GO etc. interfaces). Instead check wpa_s->ap_iface to see if this is an AP interface or not.

> +		wpa_dbg(wpa_s, MSG_DEBUG,
> +				"EAPOL_TX_STATUS: ACK(%d)",
> +				   data->eapol_tx_status.ack);
> +		if (!data->eapol_tx_status.ack &&
> +		   wpa_s->wpa_state == WPA_COMPLETED) {
> +			wpa_dbg(wpa_s, MSG_DEBUG,
> +				"EAPOL 4/4 Not acked, disconnecting");
> +			wpa_s->own_disconnect_req = 1;
> +			wpa_supplicant_deauthenticate(
> +				wpa_s,
> WLAN_REASON_4WAY_HANDSHAKE_TIMEOUT);
> +

It is possible that the Authenticator can recover from loss of message 4 by retrying to send message 3 again (as done in hostap), so it might be better to have the wpa PAE state machine handle such a case more gracefully.

> +		}
> +#endif
>  		break;
> +#ifdef CONFIG_AP
>  	case EVENT_DRIVER_CLIENT_POLL_OK:
>  		ap_client_poll_ok(wpa_s, data->client_poll.addr);
>  		break;
> diff --git a/wpa_supplicant/wpas_glue.c b/wpa_supplicant/wpas_glue.c
> index ae246f9..da81ac0 100644
> --- a/wpa_supplicant/wpas_glue.c
> +++ b/wpa_supplicant/wpas_glue.c
> @@ -97,6 +97,7 @@ static u8 * wpa_alloc_eapol(const struct wpa_supplicant
> *wpa_s, u8 type,  static int wpa_ether_send(struct wpa_supplicant *wpa_s,
> const u8 *dest,
>  			  u16 proto, const u8 *buf, size_t len)  {
> +	int ret;
>  #ifdef CONFIG_TESTING_OPTIONS
>  	if (wpa_s->ext_eapol_frame_io && proto == ETH_P_EAPOL) {
>  		size_t hex_len = 2 * len + 1;
> @@ -111,6 +112,11 @@ static int wpa_ether_send(struct wpa_supplicant
> *wpa_s, const u8 *dest,
>  		return 0;
>  	}
>  #endif /* CONFIG_TESTING_OPTIONS */
> +	ret = wpa_drv_send_eapol(wpa_s, dest, buf, len);

FWIW, I do not think that drv_send_eapol and l2 should be mixed. I think that it would be better to decide which API to use during init.

Regards,

Ilan.