[PATCH 1/3] AP: Add support for PASN comeback flow

[Jouni Malinen]

On Wed, Mar 17, 2021 at 06:13:27PM +0200, Ilan Peer wrote:
> diff --git a/src/ap/ieee802_11.c b/src/ap/ieee802_11.c
> +static void handle_auth_pasn_comeback(struct hostapd_data *hapd,
> +				      struct sta_info *sta)

> +	wpa_printf(MSG_DEBUG,
> +		   "PASN: Building comeback frame 2. Comeback after=%u",
> +		   hapd->conf->pasn_comeback_after);

> +	comeback = auth_build_token_req(hapd, sta->pasn->group, sta->addr, 0);

sta->pasn->group seems to be 0 here.. Shouldn't the cookie value use the
group number that the station proposed?

> @@ -3133,6 +3182,27 @@ static void handle_auth_pasn_1(struct hostapd_data *hapd, struct sta_info *sta,
>  		goto send_resp;
> +	if (pasn_params.comeback) {
> +		wpa_printf(MSG_DEBUG, "PASN: Checking peer comeback token");
> +
> +		/* The token includes 2 bytes for the group, so skip them */

Why would it be fine to skip them? Shouldn't we verify that the correct
value is provided? If this is not verified, there is no point in
including the group in the cookie value in the first place.

> +		ret = check_comeback_token(hapd, sta->addr,
> +					   pasn_params.comeback + 2,
> +					   pasn_params.comeback_len - 2);

That is really problematic.. pasn_params.comeback_len can be 1 here and
that would result in (size_t)-1 going to the check_comeback_token()
function and just by chance, this does not result in a security
vulnerability due to the check against SHA256_MAC_LEN.. In other words,
this really should very that pasn_params.comeback_len >= 2 before
decrementing that by 2.

> +	} else if (use_anti_clogging(hapd)) {
> +		wpa_printf(MSG_DEBUG, "PASN: Response with comeback");
> +
> +		handle_auth_pasn_comeback(hapd, sta);
> +		ap_free_sta(hapd, sta);
> +		return;
> +	}
> +
>  	sta->pasn->ecdh = crypto_ecdh_init(pasn_params.group);
...

And this would be followed by setting sta->pasn->group which explains
the 0 value noted above. Maybe it would be better to add
pasn_params.group as a new argument to handle_auth_pasn_comeback() so
that the correct group gets used in the token without changes sta->pasn
in this case?

-- 
Jouni Malinen                                            PGP id EFC895FA