| Message ID | 20231206150708.2080336-5-prestwoj@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Reassoc/FT roaming unification | expand |
| Context | Check | Description |
|---|---|---|
| tedd_an/pre-ci_am | success | Success |
| prestwoj/iwd-ci-gitlint | success | GitLint |
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
Hi Denis, On 12/6/23 08:36, Denis Kenzior wrote: > 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> >> + 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. Good question. Rekeys do appear to work as-is but you are right, FILS/FT uses set_fte() for the authenticators element, but eapol seems to use hs->fte for building message 2/4, as well as checks that the handshakes FTE matches what the authenticator sends in 3/4. maybe this is actually a bug in eapol? I think the reason everything "works" is because the FTE should be the same between both peers. We may want to refactor and do: handshake_state_set_authenticator_fte() handshake_state_set_supplicant_fte() Thanks, James > >> + >> + return true; >> } >> static bool ft_send_action(struct wiphy_radio_work_item *work) > > Regards, > -Denis
Hi James, >>> + 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. > > Good question. Rekeys do appear to work as-is but you are right, FILS/FT uses > set_fte() for the authenticators element, but eapol seems to use hs->fte for I'm pretty sure the intent was for the FTE element to be from the authenticator. > building message 2/4, as well as checks that the handshakes FTE matches what the I'll have to look at how ptk_2_of_4 uses it. Memory is fuzzy now. Need to open the spec. > authenticator sends in 3/4. maybe this is actually a bug in eapol? I think the > reason everything "works" is because the FTE should be the same between both peers. Yes, but also we have logic in: netdev_connect_event() that sets the FTE from the response IEs. So that's probably how things end up working in the end. > > We may want to refactor and do: > > handshake_state_set_authenticator_fte() > > handshake_state_set_supplicant_fte() Yeah, that seems reasonable. Regards, -Denis
On 12/6/23 09:14, Denis Kenzior wrote: > Hi James, > >>>> + 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. >> >> Good question. Rekeys do appear to work as-is but you are right, >> FILS/FT uses set_fte() for the authenticators element, but eapol >> seems to use hs->fte for > > I'm pretty sure the intent was for the FTE element to be from the > authenticator. > >> building message 2/4, as well as checks that the handshakes FTE >> matches what the > > I'll have to look at how ptk_2_of_4 uses it. Memory is fuzzy now. > Need to open the spec. All I could find quickly was: 12.7.6.3 4-way handshake message 2 "and that the FTE and MDE are the same as those provided in the AP’s (Re)Association Response frame" So it does appear for EAPoL its the authenticators FTE. > >> authenticator sends in 3/4. maybe this is actually a bug in eapol? I >> think the reason everything "works" is because the FTE should be the >> same between both peers. > > Yes, but also we have logic in: netdev_connect_event() that sets the > FTE from the response IEs. So that's probably how things end up > working in the end. > >> >> We may want to refactor and do: >> >> handshake_state_set_authenticator_fte() >> >> handshake_state_set_supplicant_fte() > > Yeah, that seems reasonable. > > Regards, > -Denis
diff --git a/src/ft.c b/src/ft.c index 2cc611b8..358a4594 100644 --- a/src/ft.c +++ b/src/ft.c @@ -904,9 +904,15 @@ static void ft_info_destroy(void *data) l_free(info); } -static void ft_prepare_handshake(struct ft_info *info, +static bool ft_prepare_handshake(struct ft_info *info, struct handshake_state *hs) { + uint32_t kck_len = handshake_state_get_kck_len(hs); + struct ie_rsn_info rsn_info; + struct ie_ft_info ft_info; + uint8_t *fte; + uint8_t *rsne; + handshake_state_set_authenticator_address(hs, info->aa); memcpy(hs->mde + 2, info->mde, 3); @@ -914,7 +920,7 @@ static void ft_prepare_handshake(struct ft_info *info, handshake_state_set_chandef(hs, NULL); if (!hs->supplicant_ie) - return; + return true; if (info->authenticator_ie) handshake_state_set_authenticator_ie(hs, @@ -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); + + /* + * 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); + + return true; } static bool ft_send_action(struct wiphy_radio_work_item *work)