From e678d6655f2f991a8d1efc12911b1af0ca315957 Mon Sep 17 00:00:00 2001 From: James Prestwood Date: Fri, 26 Oct 2018 09:44:58 -0700 Subject: [PATCH] netdev: signal handshake complete after setting all keys Currently, netdev triggers the HANDSHAKE_COMPLETE event after completing the SET_STATION (after setting the pairwise key). Depending on the timing this may happen before the GTK/IGTK are set which will result in group traffic not working initially (the GTK/IGTK would still get set, but group traffic would not work immediately after DBus said you were connected, this mainly poses a problem with autotests). In order to fix this, several flags were added in netdev_handshake_state: ptk_installed, gtk_installed, igtk_installed, and completed. Each of these flags are set true when their respective keys are set, and in each key callback we try to trigger the handshake complete event (assuming all the flags are true). Initially the gtk/igtk flags are set to true, for reasons explained below. In the WPA2 case, all the key setter functions are called sequentially from eapol. With this change, the PTK is now set AFTER the gtk/igtk. This is because the gtk/igtk are optional and only set if group traffic is allowed. If the gtk/igtk are not used, we set the PTK and can immediately trigger the handshake complete event (since gtk_installed/igtk_installed are initialized as true). When the gtk/igtk are being set, we immediately set their flags to false and wait for their callbacks in addition to the PTK callback. Doing it this way handles both group traffic and non group traffic paths. WPA1 throws a wrench into this since the group keys are obtained in a separate handshake. For this case a new flag was added to the handshake_state, 'wait_for_gtk'. This allows netdev to set the PTK after the initial 4-way, but still wait for the gtk/igtk setters to get called before triggering the handshake complete event. As a precaution, netdev sets a timeout that will trigger if the gtk/igtk setters are never called. In this case we can still complete the connection, but print a warning that group traffic will not be allowed. --- src/eapol.c | 11 ++++- src/handshake.h | 1 + src/netdev.c | 112 +++++++++++++++++++++++++++++++++++++++++++++--- 3 files changed, 117 insertions(+), 7 deletions(-) diff --git a/src/eapol.c b/src/eapol.c index f98ce70b..7c6cbee3 100644 --- a/src/eapol.c +++ b/src/eapol.c @@ -1462,7 +1462,14 @@ retransmit: if (sm->handshake->ptk_complete) return; - handshake_state_install_ptk(sm->handshake); + /* + * For WPA1 the group handshake should be happening after we set the + * ptk, this flag tells netdev to wait for the gtk/igtk before + * completing the connection. + */ + if (!gtk && sm->handshake->group_cipher != + IE_RSN_CIPHER_SUITE_NO_GROUP_TRAFFIC) + sm->handshake->wait_for_gtk = true; if (gtk) eapol_install_gtk(sm, gtk_key_index, gtk, gtk_len, ek->key_rsc); @@ -1470,6 +1477,8 @@ retransmit: if (igtk) eapol_install_igtk(sm, igtk_key_index, igtk, igtk_len); + handshake_state_install_ptk(sm->handshake); + if (rekey_offload) rekey_offload(sm->handshake->ifindex, ptk->kek, ptk->kck, sm->replay_counter, sm->user_data); diff --git a/src/handshake.h b/src/handshake.h index 37ab511c..92e6b706 100644 --- a/src/handshake.h +++ b/src/handshake.h @@ -104,6 +104,7 @@ struct handshake_state { bool have_anonce : 1; bool have_pmkid : 1; bool authenticator : 1; + bool wait_for_gtk : 1; uint8_t ssid[32]; size_t ssid_len; char *passphrase; diff --git a/src/netdev.c b/src/netdev.c index 991a7988..b1c04f3e 100644 --- a/src/netdev.c +++ b/src/netdev.c @@ -68,6 +68,10 @@ struct netdev_handshake_state { uint32_t group_new_key_cmd_id; uint32_t group_management_new_key_cmd_id; uint32_t set_station_cmd_id; + bool ptk_installed; + bool gtk_installed; + bool igtk_installed; + bool complete; struct netdev *netdev; }; @@ -99,6 +103,7 @@ struct netdev { enum netdev_result result; struct l_timeout *neighbor_report_timeout; struct l_timeout *sa_query_timeout; + struct l_timeout *group_handshake_timeout; uint16_t sa_query_id; uint8_t prev_bssid[ETH_ALEN]; int8_t rssi_levels[16]; @@ -212,6 +217,16 @@ struct handshake_state *netdev_handshake_state_new(struct netdev *netdev) nhs->super.free = netdev_handshake_state_free; nhs->netdev = netdev; + /* + * Since GTK/IGTK are optional (NO_GROUP_TRAFFIC), we set them as + * 'installed' upon initalization. If/When the gtk/igtk callback is + * called they will get set to false until we have received a successful + * callback from nl80211. From these callbacks we can check that all + * the keys have been installed, and only then trigger the handshake + * complete callback. + */ + nhs->gtk_installed = true; + nhs->igtk_installed = true; return &nhs->super; } @@ -546,6 +561,11 @@ static void netdev_connect_free(struct netdev *netdev) netdev->sa_query_timeout = NULL; } + if (netdev->group_handshake_timeout) { + l_timeout_remove(netdev->group_handshake_timeout); + netdev->group_handshake_timeout = NULL; + } + netdev->operational = false; netdev->connected = false; netdev->connect_cb = NULL; @@ -1039,6 +1059,11 @@ static void netdev_setting_keys_failed(struct netdev_handshake_state *nhs, netdev->rekey_offload_cmd_id = 0; } + if (netdev->group_handshake_timeout) { + l_timeout_remove(netdev->group_handshake_timeout); + netdev->group_handshake_timeout = NULL; + } + netdev->result = NETDEV_RESULT_KEY_SETTING_FAILED; handshake_event(&nhs->super, HANDSHAKE_EVENT_SETTING_KEYS_FAILED, NULL); @@ -1058,6 +1083,17 @@ static void netdev_setting_keys_failed(struct netdev_handshake_state *nhs, } } +static void try_handshake_complete(struct netdev_handshake_state *nhs) +{ + if (nhs->ptk_installed && nhs->gtk_installed && nhs->igtk_installed && + !nhs->complete) { + nhs->complete = true; + handshake_event(&nhs->super, HANDSHAKE_EVENT_COMPLETE, NULL); + + netdev_connect_ok(nhs->netdev); + } +} + static void netdev_set_station_cb(struct l_genl_msg *msg, void *user_data) { struct netdev_handshake_state *nhs = user_data; @@ -1065,6 +1101,7 @@ static void netdev_set_station_cb(struct l_genl_msg *msg, void *user_data) int err; nhs->set_station_cmd_id = 0; + nhs->ptk_installed = true; if (netdev->type == NL80211_IFTYPE_STATION && !netdev->connected) return; @@ -1080,10 +1117,8 @@ static void netdev_set_station_cb(struct l_genl_msg *msg, void *user_data) return; } - handshake_event(&nhs->super, HANDSHAKE_EVENT_COMPLETE, NULL); - done: - netdev_connect_ok(netdev); + try_handshake_complete(nhs); } static void netdev_new_group_key_cb(struct l_genl_msg *msg, void *data) @@ -1093,11 +1128,16 @@ static void netdev_new_group_key_cb(struct l_genl_msg *msg, void *data) nhs->group_new_key_cmd_id = 0; - if (l_genl_msg_get_error(msg) >= 0) + if (l_genl_msg_get_error(msg) < 0) { + l_error("New Key for Group Key failed for ifindex: %d", + netdev->index); + netdev_setting_keys_failed(nhs, MMPDU_REASON_CODE_UNSPECIFIED); return; + } - l_error("New Key for Group Key failed for ifindex: %d", netdev->index); - netdev_setting_keys_failed(nhs, MMPDU_REASON_CODE_UNSPECIFIED); + nhs->gtk_installed = true; + + try_handshake_complete(nhs); } static void netdev_new_group_management_key_cb(struct l_genl_msg *msg, @@ -1112,7 +1152,12 @@ static void netdev_new_group_management_key_cb(struct l_genl_msg *msg, l_error("New Key for Group Mgmt failed for ifindex: %d", netdev->index); netdev_setting_keys_failed(nhs, MMPDU_REASON_CODE_UNSPECIFIED); + return; } + + nhs->igtk_installed = true; + + try_handshake_complete(nhs); } static bool netdev_copy_tk(uint8_t *tk_buf, const uint8_t *tk, @@ -1184,6 +1229,8 @@ static void netdev_set_gtk(struct handshake_state *hs, uint8_t key_index, const uint8_t *addr = (netdev->type == NL80211_IFTYPE_ADHOC) ? nhs->super.aa : NULL; + nhs->gtk_installed = false; + l_debug("%d", netdev->index); if (crypto_cipher_key_len(cipher) != gtk_len) { @@ -1199,6 +1246,11 @@ static void netdev_set_gtk(struct handshake_state *hs, uint8_t key_index, return; } + if (hs->wait_for_gtk) { + l_timeout_remove(netdev->group_handshake_timeout); + netdev->group_handshake_timeout = NULL; + } + msg = nl80211_build_new_key_group(netdev->index, cipher, key_index, gtk_buf, gtk_len, rsc, rsc_len, addr); @@ -1224,6 +1276,8 @@ static void netdev_set_igtk(struct handshake_state *hs, uint8_t key_index, struct netdev *netdev = nhs->netdev; struct l_genl_msg *msg; + nhs->igtk_installed = false; + l_debug("%d", netdev->index); if (crypto_cipher_key_len(cipher) != igtk_len) { @@ -1312,6 +1366,30 @@ static struct l_genl_msg *netdev_build_cmd_new_key_pairwise( return msg; } +static void netdev_group_timeout_cb(struct l_timeout *timeout, void *user_data) +{ + struct netdev_handshake_state *nhs = user_data; + + /* + * There was a problem with the ptk, this should have triggered a key + * setting failure event already. + */ + if (!nhs->ptk_installed) + return; + + /* + * If this happens, we never completed the group handshake. We can still + * complete the connection, but we will not have group traffic. + */ + l_warn("completing connection with no group traffic on ifindex %d", + nhs->netdev->index); + + nhs->complete = true; + handshake_event(&nhs->super, HANDSHAKE_EVENT_COMPLETE, NULL); + + netdev_connect_ok(nhs->netdev); +} + static void netdev_set_tk(struct handshake_state *hs, const uint8_t *tk, uint32_t cipher) { @@ -1323,6 +1401,22 @@ static void netdev_set_tk(struct handshake_state *hs, enum mmpdu_reason_code rc; const uint8_t *addr = netdev_choose_key_address(nhs); + /* + * WPA1 does the group handshake after the 4-way finishes so we can't + * rely on the gtk/igtk being set immediately after the ptk. Since + * 'gtk_installed' is initially set to true (to handle NO_GROUP_TRAFFIC) + * we must set it false so we don't notify that the connection was + * successful until we get the gtk/igtk callbacks. Note that we do not + * need to set igtk_installed false because the igtk could not happen at + * all. + */ + if (hs->wait_for_gtk) { + nhs->gtk_installed = false; + + netdev->group_handshake_timeout = l_timeout_create(2, + netdev_group_timeout_cb, nhs, NULL); + } + /* * 802.11 Section 4.10.4.3: * Because in an IBSS there are two 4-way handshakes between @@ -2939,6 +3033,12 @@ int netdev_fast_transition(struct netdev *netdev, struct scan_bss *target_bss, nhs = container_of(netdev->handshake, struct netdev_handshake_state, super); + /* reset key states just as we do in initialization */ + nhs->complete = false; + nhs->ptk_installed = false; + nhs->gtk_installed = true; + nhs->igtk_installed = true; + if (nhs->group_new_key_cmd_id) { l_genl_family_cancel(nl80211, nhs->group_new_key_cmd_id); nhs->group_new_key_cmd_id = 0;