[PATCH] AP: Drop retransmitted auth/assoc frames

Peer, Ilan ilan.peer at intel.com
Wed Nov 19 06:16:35 EST 2014


Hi Jouni,

Minor comment below. Did not get a chance to test it myself.

Thanks,

Ilan.

> -----Original Message-----
> From: hostap-bounces at lists.shmoo.com [mailto:hostap-
> bounces at lists.shmoo.com] On Behalf Of Jouni Malinen
> Sent: Tuesday, November 18, 2014 20:55
> To: hostap at lists.shmoo.com
> Subject: Re: [PATCH] AP: Drop retransmitted auth/assoc frames
> 
> On Wed, Nov 05, 2014 at 03:50:34AM -0500, Ilan Peer wrote:
> > It is possible that a station device might miss an ACK for an
> > authentication or association frame, and thus retransmit the same
> > frame although the frame is already being processed in the stack.
> >
> > In such a case, the local AP will process the retransmitted frame
> > although it has already handled the request, which might cause the
> > station to get confused and as a result disconnect from the AP, blacklist it
> etc.
> >
> > To avoid such a case, save the sequence control of the last processed
> > management frame and in case of retransmissions drop them.
> 
> Thanks. This should really be the responsibility of the driver or well, kernel
> code (mac80211) that takes care of duplicate detection in general. Anyway,
> I'm thinking of adding this into hostapd taken into account mac80211 does
> not handle the pre-association cases today and it is unclear whether that gets
> added there in the short term.
> 

Agree .. Johannes wants this as well .. just never get the time to do it. Regardless, once we change the flow to add a station before pre-association, this might be required for older kernels that will have the support for adding station pre-association.

> However, there are couple of things in this specific version that do not look
> desirable:
> 
> > @@ -1945,6 +1979,8 @@ static void handle_assoc_cb(struct hostapd_data
> > *hapd,
> > +	sta->last_seq_ctrl = WLAN_INVALID_MGMT_SEQ;
> 
> This would prevent duplicate detection for a case where the duplicated
> (re)association request frame gets delivered to hostapd after (re)association
> response frame TX status is processed. Apparently that can happen in some
> cases. Same for authentication and well, there is is actually even more
> complex with different number of roundtrips getting used.
> 

Ack. I missed such cases in my original patch.

> Resetting last_seq_ctrl to WLAN_INVALID_MGMT_SEQ could potentially be
> done "some time" after completion of association processing, but I'm not
> sure whether it is really worth the effort to reduce the already pretty
> minimal likelihood of hitting false duplicates with the addition I describe
> below.
> 

I was concerned about case where the station for some reason does not complete the association flow and decides to authenticate again, but this is probably a rare corner case.

> > diff --git a/src/ap/sta_info.h b/src/ap/sta_info.h
> > +	/* last auth/assoc seq. control */
> > +	u16 last_seq_ctrl;
> 
> Since this is not perfect in the sense of not seeing all management frames
> from the STA, it would likely be useful to track the last management frame
> subtype as well to reduce likelihood of dropping non-duplicates in some
> unexpected cases.
> 
> I would also like to get Action frames covered (well for parts where there is
> an actual STA entry present or added like GAS).
> 
> I've been testing with the following version and it seems to be working
> pretty well, so I'm planning on committing it shortly unless someone comes
> up with concerns.
> 
> [PATCH] AP: Drop retransmitted auth/assoc/action frames
> 
> It is possible that a station device might miss an ACK for an authentication,
> association, or action frame, and thus retransmit the same frame although
> the frame is already being processed in the stack.
> While the duplicated frame should really be dropped in the kernel or
> firmware code where duplicate detection is implemented for data frames, it
> is possible that pre-association cases are not fully addressed (which is the
> case at least with mac80211 today) and the frame may be delivered to upper
> layer stack.
> 
> In such a case, the local AP will process the retransmitted frame although it
> has already handled the request, which might cause the station to get
> confused and as a result disconnect from the AP, blacklist it, etc.
> 
> To avoid such a case, save the sequence control of the last processed
> management frame and in case of retransmissions drop them.
> 
> Signed-off-by: Ilan Peer <ilan.peer at intel.com>
> ---
>  src/ap/ieee802_11.c          | 88
> +++++++++++++++++++++++++++++++++++++-------
>  src/ap/sta_info.c            |  3 ++
>  src/ap/sta_info.h            |  5 +++
>  src/common/ieee802_11_defs.h |  2 +
>  4 files changed, 85 insertions(+), 13 deletions(-)
> 
> diff --git a/src/ap/ieee802_11.c b/src/ap/ieee802_11.c index
> 4e389d0..8b903b3 100644
> --- a/src/ap/ieee802_11.c
> +++ b/src/ap/ieee802_11.c
> @@ -709,6 +709,7 @@ static void handle_auth(struct hostapd_data *hapd,
>  	size_t resp_ies_len = 0;
>  	char *identity = NULL;
>  	char *radius_cui = NULL;
> +	u16 seq_ctrl;
> 
>  	if (len < IEEE80211_HDRLEN + sizeof(mgmt->u.auth)) {
>  		wpa_printf(MSG_INFO, "handle_auth - too short payload
> (len=%lu)", @@ -730,6 +731,7 @@ static void handle_auth(struct
> hostapd_data *hapd,
>  	auth_transaction = le_to_host16(mgmt->u.auth.auth_transaction);
>  	status_code = le_to_host16(mgmt->u.auth.status_code);
>  	fc = le_to_host16(mgmt->frame_control);
> +	seq_ctrl = le_to_host16(mgmt->seq_ctrl);
> 
>  	if (len >= IEEE80211_HDRLEN + sizeof(mgmt->u.auth) +
>  	    2 + WLAN_AUTH_CHALLENGE_LEN &&
> @@ -738,10 +740,12 @@ static void handle_auth(struct hostapd_data
> *hapd,
>  		challenge = &mgmt->u.auth.variable[2];
> 
>  	wpa_printf(MSG_DEBUG, "authentication: STA=" MACSTR "
> auth_alg=%d "
> -		   "auth_transaction=%d status_code=%d wep=%d%s",
> +		   "auth_transaction=%d status_code=%d wep=%d%s "
> +		   "seq_ctrl=0x%x%s",
>  		   MAC2STR(mgmt->sa), auth_alg, auth_transaction,
>  		   status_code, !!(fc & WLAN_FC_ISWEP),
> -		   challenge ? " challenge" : "");
> +		   challenge ? " challenge" : "",
> +		   seq_ctrl, (fc & WLAN_FC_RETRY) ? " retry" : "");
> 
>  	if (hapd->tkip_countermeasures) {
>  		resp = WLAN_REASON_MICHAEL_MIC_FAILURE; @@ -802,21
> +806,36 @@ static void handle_auth(struct hostapd_data *hapd,
>  		return;
>  	}
> 
> +	sta = ap_get_sta(hapd, mgmt->sa);
> +	if (sta) {
> +		if ((fc & WLAN_FC_RETRY) &&
> +		    sta->last_seq_ctrl != WLAN_INVALID_MGMT_SEQ &&
> +		    sta->last_seq_ctrl == seq_ctrl &&
> +		    sta->last_subtype == WLAN_FC_STYPE_AUTH) {
> +			hostapd_logger(hapd, sta->addr,
> +				       HOSTAPD_MODULE_IEEE80211,
> +				       HOSTAPD_LEVEL_DEBUG,
> +				       "Drop repeated authentication frame
> seq_ctrl=0x%x",
> +				       seq_ctrl);
> +			return;
> +		}
> +	} else {
>  #ifdef CONFIG_MESH
> -	if (hapd->conf->mesh & MESH_ENABLED) {
> -		/* if the mesh peer is not available, we don't do auth. */
> -		sta = ap_get_sta(hapd, mgmt->sa);
> -		if (!sta)
> +		if (hapd->conf->mesh & MESH_ENABLED) {
> +			/* if the mesh peer is not available, we don't do auth.
> +			 */
>  			return;
> -	} else
> +		}
>  #endif /* CONFIG_MESH */
> -	{
> +
>  		sta = ap_sta_add(hapd, mgmt->sa);
>  		if (!sta) {
>  			resp =
> WLAN_STATUS_AP_UNABLE_TO_HANDLE_NEW_STA;
>  			goto fail;
>  		}
>  	}
> +	sta->last_seq_ctrl = seq_ctrl;
> +	sta->last_subtype = WLAN_FC_STYPE_AUTH;
> 
>  	if (vlan_id > 0) {
>  		if (!hostapd_vlan_id_valid(hapd->conf->vlan, vlan_id)) { @@
> -1464,7 +1483,7 @@ static void handle_assoc(struct hostapd_data *hapd,
>  			 const struct ieee80211_mgmt *mgmt, size_t len,
>  			 int reassoc)
>  {
> -	u16 capab_info, listen_interval;
> +	u16 capab_info, listen_interval, seq_ctrl, fc;
>  	u16 resp = WLAN_STATUS_SUCCESS;
>  	const u8 *pos;
>  	int left, i;
> @@ -1497,15 +1516,19 @@ static void handle_assoc(struct hostapd_data
> *hapd,
>  	}
>  #endif /* CONFIG_TESTING_OPTIONS */
> 
> +	fc = le_to_host16(mgmt->frame_control);
> +	seq_ctrl = le_to_host16(mgmt->seq_ctrl);
> +
>  	if (reassoc) {
>  		capab_info = le_to_host16(mgmt-
> >u.reassoc_req.capab_info);
>  		listen_interval = le_to_host16(
>  			mgmt->u.reassoc_req.listen_interval);
>  		wpa_printf(MSG_DEBUG, "reassociation request: STA="
> MACSTR
>  			   " capab_info=0x%02x listen_interval=%d
> current_ap="
> -			   MACSTR,
> +			   MACSTR " seq_ctrl=0x%x%s",
>  			   MAC2STR(mgmt->sa), capab_info, listen_interval,
> -			   MAC2STR(mgmt->u.reassoc_req.current_ap));
> +			   MAC2STR(mgmt->u.reassoc_req.current_ap),
> +			   seq_ctrl, (fc & WLAN_FC_RETRY) ? " retry" : "");
>  		left = len - (IEEE80211_HDRLEN + sizeof(mgmt-
> >u.reassoc_req));
>  		pos = mgmt->u.reassoc_req.variable;
>  	} else {
> @@ -1513,8 +1536,10 @@ static void handle_assoc(struct hostapd_data
> *hapd,
>  		listen_interval = le_to_host16(
>  			mgmt->u.assoc_req.listen_interval);
>  		wpa_printf(MSG_DEBUG, "association request: STA="
> MACSTR
> -			   " capab_info=0x%02x listen_interval=%d",
> -			   MAC2STR(mgmt->sa), capab_info, listen_interval);
> +			   " capab_info=0x%02x listen_interval=%d "
> +			   "seq_ctrl=0x%x%s",
> +			   MAC2STR(mgmt->sa), capab_info, listen_interval,
> +			   seq_ctrl, (fc & WLAN_FC_RETRY) ? " retry" : "");
>  		left = len - (IEEE80211_HDRLEN + sizeof(mgmt-
> >u.assoc_req));
>  		pos = mgmt->u.assoc_req.variable;
>  	}
> @@ -1540,6 +1565,21 @@ static void handle_assoc(struct hostapd_data
> *hapd,
>  		return;
>  	}
> 
> +	if ((fc & WLAN_FC_RETRY) &&
> +	    sta->last_seq_ctrl != WLAN_INVALID_MGMT_SEQ &&
> +	    sta->last_seq_ctrl == seq_ctrl &&
> +	    sta->last_subtype == reassoc ? WLAN_FC_STYPE_REASSOC_REQ :
> +	    WLAN_FC_STYPE_ASSOC_REQ) {
> +		hostapd_logger(hapd, sta->addr,
> HOSTAPD_MODULE_IEEE80211,
> +			       HOSTAPD_LEVEL_DEBUG,
> +			       "Drop repeated association frame
> seq_ctrl=0x%x",
> +			       seq_ctrl);
> +		return;
> +	}
> +	sta->last_seq_ctrl = seq_ctrl;
> +	sta->last_subtype = reassoc ? WLAN_FC_STYPE_REASSOC_REQ :
> +		WLAN_FC_STYPE_ASSOC_REQ;
> +
>  	if (hapd->tkip_countermeasures) {
>  		resp = WLAN_REASON_MICHAEL_MIC_FAILURE;
>  		goto fail;
> @@ -1665,6 +1705,7 @@ static void handle_disassoc(struct hostapd_data
> *hapd,
>  	}
> 
>  	ap_sta_set_authorized(hapd, sta, 0);
> +	sta->last_seq_ctrl = WLAN_INVALID_MGMT_SEQ;
>  	sta->flags &= ~(WLAN_STA_ASSOC | WLAN_STA_ASSOC_REQ_OK);
>  	wpa_auth_sm_event(sta->wpa_sm, WPA_DISASSOC);
>  	hostapd_logger(hapd, sta->addr, HOSTAPD_MODULE_IEEE80211,
> @@ -1716,6 +1757,7 @@ static void handle_deauth(struct hostapd_data
> *hapd,
>  	}
> 
>  	ap_sta_set_authorized(hapd, sta, 0);
> +	sta->last_seq_ctrl = WLAN_INVALID_MGMT_SEQ;
>  	sta->flags &= ~(WLAN_STA_AUTH | WLAN_STA_ASSOC |
>  			WLAN_STA_ASSOC_REQ_OK);
>  	wpa_auth_sm_event(sta->wpa_sm, WPA_DEAUTH); @@ -1815,6
> +1857,26 @@ static int handle_action(struct hostapd_data *hapd,
>  	}
>  #endif /* CONFIG_IEEE80211W */
> 
> +	if (sta) {
> +		u16 fc = le_to_host16(mgmt->frame_control);
> +		u16 seq_ctrl = le_to_host16(mgmt->seq_ctrl);
> +
> +		if ((fc & WLAN_FC_RETRY) &&
> +		    sta->last_seq_ctrl != WLAN_INVALID_MGMT_SEQ &&
> +		    sta->last_seq_ctrl == seq_ctrl &&
> +		    sta->last_subtype == WLAN_FC_STYPE_ACTION) {
> +			hostapd_logger(hapd, sta->addr,
> +				       HOSTAPD_MODULE_IEEE80211,
> +				       HOSTAPD_LEVEL_DEBUG,
> +				       "Drop repeated action frame
> seq_ctrl=0x%x",
> +				       seq_ctrl);
> +			return 1;
> +		}
> +
> +		sta->last_seq_ctrl = seq_ctrl;
> +		sta->last_subtype = WLAN_FC_STYPE_ACTION;
> +	}
> +
>  	switch (mgmt->u.action.category) {
>  #ifdef CONFIG_IEEE80211R
>  	case WLAN_ACTION_FT:
> diff --git a/src/ap/sta_info.c b/src/ap/sta_info.c index c48222e..19292a4
> 100644
> --- a/src/ap/sta_info.c
> +++ b/src/ap/sta_info.c
> @@ -604,6 +604,7 @@ struct sta_info * ap_sta_add(struct hostapd_data
> *hapd, const u8 *addr)
>  	ap_sta_hash_add(hapd, sta);
>  	sta->ssid = &hapd->conf->ssid;
>  	ap_sta_remove_in_other_bss(hapd, sta);
> +	sta->last_seq_ctrl = WLAN_INVALID_MGMT_SEQ;
> 
>  	return sta;
>  }
> @@ -668,6 +669,7 @@ void ap_sta_disassociate(struct hostapd_data *hapd,
> struct sta_info *sta,  {
>  	wpa_printf(MSG_DEBUG, "%s: disassociate STA " MACSTR,
>  		   hapd->conf->iface, MAC2STR(sta->addr));
> +	sta->last_seq_ctrl = WLAN_INVALID_MGMT_SEQ;
>  	sta->flags &= ~(WLAN_STA_ASSOC | WLAN_STA_ASSOC_REQ_OK);
>  	ap_sta_set_authorized(hapd, sta, 0);
>  	sta->timeout_next = STA_DEAUTH;
> @@ -706,6 +708,7 @@ void ap_sta_deauthenticate(struct hostapd_data
> *hapd, struct sta_info *sta,  {
>  	wpa_printf(MSG_DEBUG, "%s: deauthenticate STA " MACSTR,
>  		   hapd->conf->iface, MAC2STR(sta->addr));
> +	sta->last_seq_ctrl = WLAN_INVALID_MGMT_SEQ;
>  	sta->flags &= ~(WLAN_STA_AUTH | WLAN_STA_ASSOC);
>  	ap_sta_set_authorized(hapd, sta, 0);
>  	sta->timeout_next = STA_REMOVE;
> diff --git a/src/ap/sta_info.h b/src/ap/sta_info.h index 76e34e4..89f0ef8
> 100644
> --- a/src/ap/sta_info.h
> +++ b/src/ap/sta_info.h
> @@ -158,6 +158,11 @@ struct sta_info {
>  #endif /* CONFIG_SAE */
> 
>  	u32 session_timeout; /* valid only if session_timeout_set == 1 */
> +
> +	/* Last Authentication/(Re)Association Request frame seuence
> control */
> +	u16 last_seq_ctrl;
> +	/* Last Authentication/(Re)Association Request frame subtype */

Add to the documentation public action frames.

> +	u8 last_subtype;
>  };
> 
> 
> diff --git a/src/common/ieee802_11_defs.h
> b/src/common/ieee802_11_defs.h index 14a0ddc..10ad2ee 100644
> --- a/src/common/ieee802_11_defs.h
> +++ b/src/common/ieee802_11_defs.h
> @@ -25,6 +25,8 @@
>  #define WLAN_FC_GET_TYPE(fc)	(((fc) & 0x000c) >> 2)
>  #define WLAN_FC_GET_STYPE(fc)	(((fc) & 0x00f0) >> 4)
> 
> +#define WLAN_INVALID_MGMT_SEQ   0xFFFF
> +
>  #define WLAN_GET_SEQ_FRAG(seq) ((seq) & (BIT(3) | BIT(2) | BIT(1) |
> BIT(0)))  #define WLAN_GET_SEQ_SEQ(seq) \
>  	(((seq) & (~(BIT(3) | BIT(2) | BIT(1) | BIT(0)))) >> 4)
> --
> 1.9.1
> 
> 
> --
> Jouni Malinen                                            PGP id EFC895FA
> _______________________________________________
> HostAP mailing list
> HostAP at lists.shmoo.com
> http://lists.shmoo.com/mailman/listinfo/hostap


More information about the HostAP mailing list