TDLS: Reject TPK-TK reconfiguration

Do not try to reconfigure the same TPK-TK to the driver after it has
been successfully configured. This is an explicit check to avoid issues
related to resetting the TX/RX packet number. There was already a check
for this for TPK M2 (retries of that message are ignored completely), so
that behavior does not get modified.

For TPK M3, the TPK-TK could have been reconfigured, but that was
followed by immediate teardown of the link due to an issue in updating
the STA entry. Furthermore, for TDLS with any real security (i.e.,
ignoring open/WEP), the TPK message exchange is protected on the AP path
and simple replay attacks are not feasible.

As an additional corner case, make sure the local nonce gets updated if
the peer uses a very unlikely "random nonce" of all zeros.

Change-Id: I84131f30c358f27aaf6277e8957d165bca5102aa
Signed-off-by: Jouni Malinen <j@w1.fi>

1 files changed, 36 insertions, 2 deletions

diff --git a/src/rsn_supp/tdls.c b/src/rsn_supp/tdls.c
index 0ac04227..4a323dd0 100644
--- a/src/rsn_supp/tdls.c
+++ b/src/rsn_supp/tdls.c
@@ -111,6 +111,7 @@ struct wpa_tdls_peer {
 		u8 tk[16]; /* TPK-TK; assuming only CCMP will be used */
 	} tpk;
 	int tpk_set;
+	int tk_set; /* TPK-TK configured to the driver */
 	int tpk_success;
 	int tpk_in_progress;
 
@@ -191,6 +192,20 @@ static int wpa_tdls_set_key(struct wpa_sm *sm, struct wpa_tdls_peer *peer)
 	u8 rsc[6];
 	enum wpa_alg alg;
 
+	if (peer->tk_set) {
+		/*
+		 * This same TPK-TK has already been configured to the driver
+		 * and this new configuration attempt (likely due to an
+		 * unexpected retransmitted frame) would result in clearing
+		 * the TX/RX sequence number which can break security, so must
+		 * not allow that to happen.
+		 */
+		wpa_printf(MSG_INFO, "TDLS: TPK-TK for the peer " MACSTR
+			   " has already been configured to the driver - do not reconfigure",
+			   MAC2STR(peer->addr));
+		return -1;
+	}
+
 	os_memset(rsc, 0, 6);
 
 	switch (peer->cipher) {
@@ -208,12 +223,15 @@ static int wpa_tdls_set_key(struct wpa_sm *sm, struct wpa_tdls_peer *peer)
 		return -1;
 	}
 
+	wpa_printf(MSG_DEBUG, "TDLS: Configure pairwise key for peer " MACSTR,
+		   MAC2STR(peer->addr));
 	if (wpa_sm_set_key(sm, alg, peer->addr, -1, 1,
 			   rsc, sizeof(rsc), peer->tpk.tk, key_len) < 0) {
 		wpa_printf(MSG_WARNING, "TDLS: Failed to set TPK to the "
 			   "driver");
 		return -1;
 	}
+	peer->tk_set = 1;
 	return 0;
 }
 
@@ -695,7 +713,7 @@ static void wpa_tdls_peer_clear(struct wpa_sm *sm, struct wpa_tdls_peer *peer)
 	peer->cipher = 0;
 	peer->qos_info = 0;
 	peer->wmm_capable = 0;
-	peer->tpk_set = peer->tpk_success = 0;
+	peer->tk_set = peer->tpk_set = peer->tpk_success = 0;
 	peer->chan_switch_enabled = 0;
 	os_memset(&peer->tpk, 0, sizeof(peer->tpk));
 	os_memset(peer->inonce, 0, WPA_NONCE_LEN);
@@ -1158,6 +1176,7 @@ skip_rsnie:
 		wpa_tdls_peer_free(sm, peer);
 		return -1;
 	}
+	peer->tk_set = 0; /* A new nonce results in a new TK */
 	wpa_hexdump(MSG_DEBUG, "TDLS: Initiator Nonce for TPK handshake",
 		    peer->inonce, WPA_NONCE_LEN);
 	os_memcpy(ftie->Snonce, peer->inonce, WPA_NONCE_LEN);
@@ -1750,6 +1769,19 @@ static int wpa_tdls_addset_peer(struct wpa_sm *sm, struct wpa_tdls_peer *peer,
 }
 
 
+static int tdls_nonce_set(const u8 *nonce)
+{
+	int i;
+
+	for (i = 0; i < WPA_NONCE_LEN; i++) {
+		if (nonce[i])
+			return 1;
+	}
+
+	return 0;
+}
+
+
 static int wpa_tdls_process_tpk_m1(struct wpa_sm *sm, const u8 *src_addr,
 				   const u8 *buf, size_t len)
 {
@@ -2003,7 +2035,8 @@ skip_rsn:
 	peer->rsnie_i_len = kde.rsn_ie_len;
 	peer->cipher = cipher;
 
-	if (os_memcmp(peer->inonce, ftie->Snonce, WPA_NONCE_LEN) != 0) {
+	if (os_memcmp(peer->inonce, ftie->Snonce, WPA_NONCE_LEN) != 0 ||
+	    !tdls_nonce_set(peer->inonce)) {
 		/*
 		 * There is no point in updating the RNonce for every obtained
 		 * TPK M1 frame (e.g., retransmission due to timeout) with the
@@ -2019,6 +2052,7 @@ skip_rsn:
 				"TDLS: Failed to get random data for responder nonce");
 			goto error;
 		}
+		peer->tk_set = 0; /* A new nonce results in a new TK */
 	}
 
 #if 0