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

@@ -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);

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;

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) {
-		return;
+		l_error("New Key for Group Key failed for ifindex: %d",
-
+				netdev->index);
-	l_error("New Key for Group Key failed for ifindex: %d", netdev->index);
 		netdev_setting_keys_failed(nhs, MMPDU_REASON_CODE_UNSPECIFIED);
+		return;
+	}
+
+	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;