[RFC] hostapd: MFP: Handle auth request from an associated station

From: Beni Lev <beni.lev@intel.com>

One of the purposes of the MFP mechanism is to protect from an attacker
to cause a disconnection of a STA connected to an AP.
Such an attack can be done by injecting an auth request on behalf of
the connected STA.

In the current implementation, when an auth request is received
from an associated station, the station might be removed and re-added,
keys are freed and more changes to the station are made.
In order to protect such a station that uses MFP from being kicked out
by an auth request injection, just reply to the auth request without changing
the sta's state.

In the case that the STA wants to reassociate, the STA will proceed to association.
In this case, the AP will send an assoc response with code 30(rejected temporarily),
an initiate an SA query. Since the sta will fail this, as it is not
associated anymore, the AP will deauth the STA, and the STA will go over the whole
process again, but this time not as an associated STA.

In case of an injection attack, the attacker may proceed with association request
injection, in this case, the SA query will succeed, all this without having any
change to the STA's state.

Signed-off-by: Beni Lev <beni.lev@intel.com>
---
 src/ap/ieee802_11.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

On Sat, Jun 18, 2016 at 09:24:46PM +0300, Ilan Peer wrote:
> One of the purposes of the MFP mechanism is to protect from an attacker
> to cause a disconnection of a STA connected to an AP.
> Such an attack can be done by injecting an auth request on behalf of
> the connected STA.
> 
> In the current implementation, when an auth request is received
> from an associated station, the station might be removed and re-added,
> keys are freed and more changes to the station are made.

I didn't figure out initially what this was about since I did originally
implement protection for this exact case and have tested it to work
fine.. Looking at the pending items in my to-do list, I figure out what
has happened here. This STA entry removal and re-addition is a new
change from this year (commit bb598c3bdd0616f0c15e1a42e99591d8f3ff3323)
when the support for "full AP client state" was added. The PMF
protection mechanism is still working fine with drivers that don't use
this new capability..

> In order to protect such a station that uses MFP from being kicked out
> by an auth request injection, just reply to the auth request without changing
> the sta's state.

That's what was happening before that full AP client state addition and
I agree that this should be the behavior with full AP client state as
well. However, this:

> diff --git a/src/ap/ieee802_11.c b/src/ap/ieee802_11.c
> @@ -1181,6 +1181,20 @@ static void handle_auth(struct hostapd_data *hapd,
> +#ifdef CONFIG_IEEE80211W
> +		/* TODO: handle other authentication algorithms */
> +		if (sta->flags & WLAN_STA_MFP && ap_sta_is_authorized(sta) &&
> +		    auth_alg == WLAN_AUTH_OPEN) {
> +			wpa_printf(MSG_WARNING, "STA " MACSTR
> +				   " got authentication frame while already authorized and uses MFP - reply without changing STA's state",
> +				   MAC2STR(mgmt->sa));
> +
> +			send_auth_reply(hapd, mgmt->sa, mgmt->bssid, auth_alg,
> +					auth_transaction + 1, resp, resp_ies,
> +					resp_ies_len);
> +			return;

is not really doing that nicely. This is adding yet another copy of
authentication frame processing and would indeed need a lot more
duplicated code to handle FT, SAE, and FILS authentication algorithms.

It looks much simpler to fix the regression in the full state state
commit mentioned above with the following change to leave a single
authentication frame processing implementation in use for both cases:

diff --git a/src/ap/ieee802_11.c b/src/ap/ieee802_11.c
@@ -1580,8 +1580,15 @@ static void handle_auth(struct hostapd_data *hapd,
 	 *
 	 * In mesh mode, the station was already added to the driver when the
 	 * NEW_PEER_CANDIDATE event is received.
+	 *
+	 * If PMF was negotiated for the existing association, skip this to
+	 * avoid dropping the STA entry and the associated keys. This is needed
+	 * to allow the original connection work until the attempt can complete
+	 * (re)association, so that unprotected Authentication frame cannot be
+	 * used to bypass PMF protection.
 	 */
 	if (FULL_AP_CLIENT_STATE_SUPP(hapd->iface->drv_flags) &&
+	    (!(sta->flags & WLAN_STA_MFP) || !ap_sta_is_authorized(sta)) &&
 	    !(hapd->conf->mesh & MESH_ENABLED) &&
 	    !(sta->added_unassoc)) {
 		/*

> is not really doing that nicely. This is adding yet another copy of

> authentication frame processing and would indeed need a lot more

> duplicated code to handle FT, SAE, and FILS authentication algorithms.

> 

> It looks much simpler to fix the regression in the full state state

> commit mentioned above with the following change to leave a single

> authentication frame processing implementation in use for both cases:

> 

> diff --git a/src/ap/ieee802_11.c b/src/ap/ieee802_11.c

> @@ -1580,8 +1580,15 @@ static void handle_auth(struct hostapd_data *hapd,

>  	 *

>  	 * In mesh mode, the station was already added to the driver when the

>  	 * NEW_PEER_CANDIDATE event is received.

> +	 *

> +	 * If PMF was negotiated for the existing association, skip this to

> +	 * avoid dropping the STA entry and the associated keys. This is needed

> +	 * to allow the original connection work until the attempt can complete

> +	 * (re)association, so that unprotected Authentication frame cannot be

> +	 * used to bypass PMF protection.

>  	 */

>  	if (FULL_AP_CLIENT_STATE_SUPP(hapd->iface->drv_flags) &&

> +	    (!(sta->flags & WLAN_STA_MFP) || !ap_sta_is_authorized(sta)) &&

>  	    !(hapd->conf->mesh & MESH_ENABLED) &&

>  	    !(sta->added_unassoc)) {

>  		/*

>  


Simpler and nicer. 

Thanks for handling this,

Ilan.