[PATCH] AP: Drop retransmitted auth/assoc frames
Peer, Ilan
ilan.peer
Wed Nov 19 03:16:35 PST 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