Re: [PATCH v2 4/9] ft: add FTE/RSNE building to ft_prepare_handshake

[Denis Kenzior <denkenz@gmail.com>]

Hi James,

On 12/6/23 09:07, James Prestwood wrote:
> In preparation to remove ft_associate build the FTE/RSNE in
> ft_prepare_handshake and set into the handshake object directly.
> ---
>   src/ft.c | 84 ++++++++++++++++++++++++++++++++++++++++++++++++++++++--
>   1 file changed, 82 insertions(+), 2 deletions(-)
> 

<snip>

> @@ -931,6 +937,80 @@ static void ft_prepare_handshake(struct ft_info *info,
>   						info->ft_info.r1khid);
>   
>   	handshake_state_derive_ptk(hs);
> +
> +	/*
> +	 * Rebuild the RSNE to include the PMKR1Name and append
> +	 * MDE + FTE.
> +	 *
> +	 * 12.8.4: "If present, the RSNE shall be set as follows:
> +	 * - Version field shall be set to 1.
> +	 * - PMKID Count field shall be set to 1.
> +	 * - PMKID field shall contain the PMKR1Name.
> +	 * - All other fields shall be as specified in 8.4.2.27
> +	 *   and 11.5.3."
> +	 */
> +	if (ie_parse_rsne_from_data(hs->supplicant_ie,
> +					hs->supplicant_ie[1] + 2,
> +					&rsn_info) < 0)
> +		return false;
> +
> +	rsn_info.num_pmkids = 1;
> +	rsn_info.pmkids = hs->pmk_r1_name;
> +	/* Always set OCVC false for FT for now */
> +	rsn_info.ocvc = false;
> +	rsne = alloca(256);
> +
> +	ie_build_rsne(&rsn_info, rsne);
> +	handshake_state_set_supplicant_ie(hs, rsne);

This is probably safe since we over-write the supplicant ie in 
netdev_connect_event() -> parse_request_ies()

> +
> +	/*
> +	 * 12.8.4: "If present, the FTE shall be set as follows:
> +	 * - ANonce, SNonce, R0KH-ID, and R1KH-ID shall be set to
> +	 *   the values contained in the second message of this
> +	 *   sequence.
> +	 * - The Element Count field of the MIC Control field shall
> +	 *   be set to the number of elements protected in this
> +	 *   frame (variable).
> +	 * [...]
> +	 * - All other fields shall be set to 0."
> +	 */
> +	memset(&ft_info, 0, sizeof(ft_info));
> +	ft_info.mic_element_count = 3;
> +	memcpy(ft_info.r0khid, hs->r0khid, hs->r0khid_len);
> +	ft_info.r0khid_len = hs->r0khid_len;
> +	memcpy(ft_info.r1khid, hs->r1khid, 6);
> +	ft_info.r1khid_present = true;
> +	memcpy(ft_info.anonce, hs->anonce, 32);
> +	memcpy(ft_info.snonce, hs->snonce, 32);
> +
> +	/*
> +	 * IEEE 802.11-2020 Section 13.7.1 FT reassociation in an RSN
> +	 *
> +	 * "If dot11RSNAOperatingChannelValidationActivated is true and
> +	 *  the FTO indicates OCVC capability, the target AP shall
> +	 *  ensure that OCI subelement of the FTE matches by ensuring
> +	 *  that all of the following are true:
> +	 *      - OCI subelement is present
> +	 *      - Channel information in the OCI matches current
> +	 *        operating channel parameters (see 12.2.9)"
> +	 */
> +	if (hs->supplicant_ocvc && hs->chandef) {
> +		oci_from_chandef(hs->chandef, ft_info.oci);
> +		ft_info.oci_present = true;
> +	}
> +
> +	fte = alloca(256);
> +	ie_build_fast_bss_transition(&ft_info, kck_len, fte);
> +
> +	if (!ft_calculate_fte_mic(hs, 5, rsne, fte, NULL, ft_info.mic))
> +		return false;
> +
> +	/* Rebuild the FT IE now with the MIC included */
> +	ie_build_fast_bss_transition(&ft_info, kck_len, fte);
> +
> +	handshake_state_set_fte(hs, fte);

However, this is less clear to me.  Looking at how FILS and FT uses this API, it 
seems that set_fte is meant for the authenticator FTE element?  So I think 
rekeying after FT would be broken by this change.

> +
> +	return true;
>   }
>   
>   static bool ft_send_action(struct wiphy_radio_work_item *work)

Regards,
-Denis