From c235c9fa54e5d8ac9ba71baf03aee073212e9a02 Mon Sep 17 00:00:00 2001 From: Denis Kenzior Date: Fri, 17 Sep 2021 09:09:43 -0500 Subject: [PATCH] handshake: Only bitwise compare when needed handshake_util_ap_ie_matches() is used to make sure that the RSN element received from the Authenticator during handshake / association response is the same as the one advertised in Beacon/Probe Response frames. This utility tries to bitwise compare the element first, and only if that fails, compares RSN members individually. For FT, bitwise comparison will always fail since the PMKID has to be included by the Authenticator in any RSN IEs included in Authenticate & Association Response frames. Perform the bitwise comparison as an optimization only during processing of eapol message 3/4. Also keep the parsed rsn information for future use and to possibly avoid re-parsing it during later checks. --- src/eapol.c | 36 +++++++++++++----------- src/ft.c | 5 ++-- src/handshake.c | 73 ++++++++++++++++++------------------------------- src/handshake.h | 2 +- 4 files changed, 50 insertions(+), 66 deletions(-) diff --git a/src/eapol.c b/src/eapol.c index 5a3ef7a0..fce5ff8e 100644 --- a/src/eapol.c +++ b/src/eapol.c @@ -1623,11 +1623,13 @@ static void eapol_handle_ptk_3_of_4(struct eapol_sm *sm, const uint8_t *igtk = NULL; size_t igtk_len; const uint8_t *rsne; + struct ie_rsn_info rsn_info; const uint8_t *optional_rsne = NULL; const uint8_t *transition_disable; size_t transition_disable_len; uint8_t gtk_key_index; uint16_t igtk_key_index; + int r; l_debug("ifindex=%u", hs->ifindex); @@ -1667,20 +1669,26 @@ static void eapol_handle_ptk_3_of_4(struct eapol_sm *sm, if (!rsne) goto error_ie_different; - if (!handshake_util_ap_ie_matches(rsne, hs->authenticator_ie, - hs->wpa_ie)) + if (!hs->wpa_ie) + r = ie_parse_rsne_from_data(rsne, rsne[1] + 2, &rsn_info); + else + r = ie_parse_wpa_from_data(rsne, rsne[1] + 2, &rsn_info); + + if (r < 0) + goto error_ie_different; + + if ((rsne[1] != hs->authenticator_ie[1] || + memcmp(rsne + 2, hs->authenticator_ie + 2, rsne[1])) && + !handshake_util_ap_ie_matches(&rsn_info, + hs->authenticator_ie, + hs->wpa_ie)) goto error_ie_different; if (hs->akm_suite & (IE_RSN_AKM_SUITE_FT_OVER_8021X | IE_RSN_AKM_SUITE_FT_USING_PSK | IE_RSN_AKM_SUITE_FT_OVER_SAE_SHA256)) { - struct ie_rsn_info ie_info; - - if (ie_parse_rsne_from_data(rsne, rsne[1] + 2, &ie_info) < 0) - goto error_ie_different; - - if (ie_info.num_pmkids != 1 || memcmp(ie_info.pmkids, + if (rsn_info.num_pmkids != 1 || memcmp(rsn_info.pmkids, hs->pmk_r1_name, 16)) goto error_ie_different; @@ -1727,13 +1735,9 @@ static void eapol_handle_ptk_3_of_4(struct eapol_sm *sm, * RSNE or deauthenticates." */ if (optional_rsne) { - struct ie_rsn_info info1; struct ie_rsn_info info2; uint16_t override; - if (ie_parse_rsne_from_data(rsne, rsne[1] + 2, &info1) < 0) - goto error_ie_different; - if (ie_parse_rsne_from_data(optional_rsne, optional_rsne[1] + 2, &info2) < 0) goto error_ie_different; @@ -1757,14 +1761,14 @@ static void eapol_handle_ptk_3_of_4(struct eapol_sm *sm, * and rsne2 * - Check that rsne2 pairwise_ciphers is a subset of rsne */ - if (info1.akm_suites != info2.akm_suites || - info1.group_cipher != info2.group_cipher) + if (rsn_info.akm_suites != info2.akm_suites || + rsn_info.group_cipher != info2.group_cipher) goto error_ie_different; override = info2.pairwise_ciphers; - if (override == info1.pairwise_ciphers || - !(info1.pairwise_ciphers & override) || + if (override == rsn_info.pairwise_ciphers || + !(rsn_info.pairwise_ciphers & override) || __builtin_popcount(override) != 1) { handshake_failed(sm, MMPDU_REASON_CODE_INVALID_PAIRWISE_CIPHER); diff --git a/src/ft.c b/src/ft.c index 13733019..aafb11fc 100644 --- a/src/ft.c +++ b/src/ft.c @@ -323,7 +323,7 @@ static bool ft_verify_rsne(const uint8_t *rsne, const uint8_t *pmk_r0_name, memcmp(msg2_rsne.pmkids, pmk_r0_name, 16)) return false; - if (!handshake_util_ap_ie_matches(rsne, authenticator_ie, false)) + if (!handshake_util_ap_ie_matches(&msg2_rsne, authenticator_ie, false)) return false; return true; @@ -674,7 +674,8 @@ static int ft_rx_associate(struct auth_proto *ap, const uint8_t *frame, memcmp(msg4_rsne.pmkids, hs->pmk_r1_name, 16)) return -EBADMSG; - if (!handshake_util_ap_ie_matches(rsne, hs->authenticator_ie, + if (!handshake_util_ap_ie_matches(&msg4_rsne, + hs->authenticator_ie, false)) return -EBADMSG; } else { diff --git a/src/handshake.c b/src/handshake.c index 88c89b51..59711ca8 100644 --- a/src/handshake.c +++ b/src/handshake.c @@ -759,90 +759,69 @@ void handshake_state_set_gtk(struct handshake_state *s, const uint8_t *key, * results vs the RSN/WPA IE obtained as part of the 4-way handshake. If they * don't match, the EAPoL packet must be silently discarded. */ -bool handshake_util_ap_ie_matches(const uint8_t *msg_ie, +bool handshake_util_ap_ie_matches(const struct ie_rsn_info *msg_info, const uint8_t *scan_ie, bool is_wpa) { - struct ie_rsn_info msg_info; struct ie_rsn_info scan_info; + int r; - /* - * First check that the sizes match, if they do, run a bitwise - * comparison. - */ - if (msg_ie[1] == scan_ie[1] && - !memcmp(msg_ie + 2, scan_ie + 2, msg_ie[1])) - return true; + if (!is_wpa) + r = ie_parse_rsne_from_data(scan_ie, + scan_ie[1] + 2, &scan_info); + else + r = ie_parse_wpa_from_data(scan_ie, scan_ie[1] + 2, &scan_info); - /* - * Otherwise we have to parse the IEs and compare the individual - * fields - */ - if (!is_wpa) { - if (ie_parse_rsne_from_data(msg_ie, msg_ie[1] + 2, - &msg_info) < 0) - return false; - - if (ie_parse_rsne_from_data(scan_ie, scan_ie[1] + 2, - &scan_info) < 0) - return false; - } else { - if (ie_parse_wpa_from_data(msg_ie, msg_ie[1] + 2, - &msg_info) < 0) - return false; - - if (ie_parse_wpa_from_data(scan_ie, scan_ie[1] + 2, - &scan_info) < 0) - return false; - } - - if (msg_info.group_cipher != scan_info.group_cipher) + if (r < 0) return false; - if (msg_info.pairwise_ciphers != scan_info.pairwise_ciphers) + if (msg_info->group_cipher != scan_info.group_cipher) return false; - if (msg_info.akm_suites != scan_info.akm_suites) + if (msg_info->pairwise_ciphers != scan_info.pairwise_ciphers) return false; - if (msg_info.preauthentication != scan_info.preauthentication) + if (msg_info->akm_suites != scan_info.akm_suites) return false; - if (msg_info.no_pairwise != scan_info.no_pairwise) + if (msg_info->preauthentication != scan_info.preauthentication) return false; - if (msg_info.ptksa_replay_counter != scan_info.ptksa_replay_counter) + if (msg_info->no_pairwise != scan_info.no_pairwise) return false; - if (msg_info.gtksa_replay_counter != scan_info.gtksa_replay_counter) + if (msg_info->ptksa_replay_counter != scan_info.ptksa_replay_counter) return false; - if (msg_info.mfpr != scan_info.mfpr) + if (msg_info->gtksa_replay_counter != scan_info.gtksa_replay_counter) return false; - if (msg_info.mfpc != scan_info.mfpc) + if (msg_info->mfpr != scan_info.mfpr) return false; - if (msg_info.peerkey_enabled != scan_info.peerkey_enabled) + if (msg_info->mfpc != scan_info.mfpc) return false; - if (msg_info.spp_a_msdu_capable != scan_info.spp_a_msdu_capable) + if (msg_info->peerkey_enabled != scan_info.peerkey_enabled) return false; - if (msg_info.spp_a_msdu_required != scan_info.spp_a_msdu_required) + if (msg_info->spp_a_msdu_capable != scan_info.spp_a_msdu_capable) return false; - if (msg_info.pbac != scan_info.pbac) + if (msg_info->spp_a_msdu_required != scan_info.spp_a_msdu_required) return false; - if (msg_info.extended_key_id != scan_info.extended_key_id) + if (msg_info->pbac != scan_info.pbac) return false; - if (msg_info.ocvc != scan_info.ocvc) + if (msg_info->extended_key_id != scan_info.extended_key_id) + return false; + + if (msg_info->ocvc != scan_info.ocvc) return false; /* We don't check the PMKIDs since these might actually be different */ - if (msg_info.group_management_cipher != + if (msg_info->group_management_cipher != scan_info.group_management_cipher) return false; diff --git a/src/handshake.h b/src/handshake.h index e41cc744..fa12faa7 100644 --- a/src/handshake.h +++ b/src/handshake.h @@ -242,7 +242,7 @@ bool handshake_decode_fte_key(struct handshake_state *s, const uint8_t *wrapped, void handshake_state_set_gtk(struct handshake_state *s, const uint8_t *key, unsigned int key_index, const uint8_t *rsc); -bool handshake_util_ap_ie_matches(const uint8_t *msg_ie, +bool handshake_util_ap_ie_matches(const struct ie_rsn_info *msg_info, const uint8_t *scan_ie, bool is_wpa); const uint8_t *handshake_util_find_kde(enum handshake_kde selector,