diff mbox series

[v2,1/2] wpa_supplicant: Send EAPoL-Key frames over NL80211 where available

Message ID 20200103151742.43551-2-markus.theil@tu-ilmenau.de
State Accepted
Headers show
Series Fixes and Rx path for Brendan's EAPoL over NL80211 | expand

Commit Message

Markus Theil Jan. 3, 2020, 3:17 p.m. UTC
From: Brendan Jackman <brendan.jackman@bluwireless.co.uk>

Linux kernel v4.17 added the ability to request sending control port
frames via NL80211 instead of a normal network socket. Doing this
provides the device driver with ordering information between the
control port frames and the installation of keys. This empowers it to
avoid race conditions between, for example, PTK replacement and the
sending of frame 4 of the 4-way rekeying handshake in an RSNA. The
key difference between a TX_CONTROL_PORT and normal socket send is
that the device driver will certainly get any EAPoL frames comprising
a 4-way handshake before it gets the key installation call
for the derived key. By flushing its TX buffers it can then ensure
that no pending EAPoL frames are inadvertently encrypted with a key
that the peer will not yet have installed.

This patch adds a TX_CONTROL_PORT flag to the hostap driver API to report
that it supports, for a given device, a new operation called
tx_control_port. This operation is exactly like an ethernet send except
for the extra ordering information it provides for device drivers. The
nl80211 driver is updated to support this operation when the device
reports the CONTROL_PORT_OVER_NL80211 extended feature. Finally the RSN
supplicant system is updated to use this new operation for sending
EAPoL-Key frames when the driver reports that it is available; otherwise
falling back to a normal ethernet TX.

There may be other cases than these EAPoL-Key frames that would benefit
from using the new operation but I do not know of them.

I have tested this on DMG (11ad/ay) devices with an out-of-tree Linux
driver that does not use mac80211. Without this patch I consistently see
PTK rekeying fail if message 4/4 shares a stream with other in-flight
traffic. With this patch, and the driver updated to flush the relevant TX
queue before overwriting a PTK (knowing, now, that if there was a message
4/4 related to the key installation, it has already entered the driver
queue), rekeying is reliable.

There is still data loss surrounding key installation - this problem is
alluded to in 802.11-2016 12.6.21, where extended Key ID support is
described as the eventual solution. This patch aims to at least prevent
rekeying from totally breaking the association, in a way that works on
kernels as far back as 4.17 (as per Alexander Wetzel extended Key ID
support should be possible on 5.2).

See http://lists.infradead.org/pipermail/hostap/2019-May/040089.html for a
little more context.

Cc: Chaitanya Tata <chaitanya.tata@bluwireless.co.uk>
Cc: Antony King <antony.king@bluwireless.co.uk>
Cc: Johannes Berg <johannes@sipsolutions.net>
Cc: Alexander Wetzel <alexander@wetzel-home.de>
Cc: Krishna Chaitanya <chaitanya.mgit@gmail.com>
Signed-off-by: Brendan Jackman <brendan.jackman@bluwireless.co.uk>
---
 src/drivers/driver.h              | 26 +++++++++++++
 src/drivers/driver_nl80211.c      | 39 ++++++++++++++++++++
 src/drivers/driver_nl80211_capa.c |  4 ++
 src/rsn_supp/wpa.c                |  2 +-
 src/rsn_supp/wpa.h                |  2 +
 src/rsn_supp/wpa_i.h              |  7 ++++
 wpa_supplicant/driver_i.h         |  8 ++++
 wpa_supplicant/ibss_rsn.c         | 18 +++++++++
 wpa_supplicant/wpas_glue.c        | 61 ++++++++++++++++++++++++-------
 9 files changed, 153 insertions(+), 14 deletions(-)

Comments

Jouni Malinen Jan. 5, 2020, 7:57 p.m. UTC | #1
On Fri, Jan 03, 2020 at 04:17:41PM +0100, Markus Theil wrote:
> From: Brendan Jackman <brendan.jackman@bluwireless.co.uk>
> 
> Linux kernel v4.17 added the ability to request sending control port
> frames via NL80211 instead of a normal network socket. Doing this
> provides the device driver with ordering information between the
> control port frames and the installation of keys. This empowers it to
> avoid race conditions between, for example, PTK replacement and the
> sending of frame 4 of the 4-way rekeying handshake in an RSNA. The
> key difference between a TX_CONTROL_PORT and normal socket send is
> that the device driver will certainly get any EAPoL frames comprising
> a 4-way handshake before it gets the key installation call
> for the derived key. By flushing its TX buffers it can then ensure
> that no pending EAPoL frames are inadvertently encrypted with a key
> that the peer will not yet have installed.
> 
> This patch adds a TX_CONTROL_PORT flag to the hostap driver API to report
> that it supports, for a given device, a new operation called
> tx_control_port. This operation is exactly like an ethernet send except
> for the extra ordering information it provides for device drivers. The
> nl80211 driver is updated to support this operation when the device
> reports the CONTROL_PORT_OVER_NL80211 extended feature. Finally the RSN
> supplicant system is updated to use this new operation for sending
> EAPoL-Key frames when the driver reports that it is available; otherwise
> falling back to a normal ethernet TX.
> 
> There may be other cases than these EAPoL-Key frames that would benefit
> from using the new operation but I do not know of them.
..

Thanks, applied with cleanup and changes to make this apply to all EAPOL
frames. There is no benefit from trying to limit this to only EAPOL-Key
frames; that makes it just more complex to understand what is happening.
In fact, that actually simplifies the implementation significantly since
it eliminates need for the separate new tx_control_port() handler in the
EAPOL state machines.
diff mbox series

Patch

diff --git a/src/drivers/driver.h b/src/drivers/driver.h
index f1027c08e..ce7d62abf 100644
--- a/src/drivers/driver.h
+++ b/src/drivers/driver.h
@@ -1704,6 +1704,8 @@  struct wpa_driver_capa {
 #define WPA_DRIVER_FLAGS_FTM_RESPONDER		0x0100000000000000ULL
 /** Driver support 4-way handshake offload for WPA-Personal */
 #define WPA_DRIVER_FLAGS_4WAY_HANDSHAKE_PSK	0x0200000000000000ULL
+/** Driver has working tx_control_port */
+#define WPA_DRIVER_FLAGS_TX_CONTROL_PORT	0x0400000000000000ULL
 	u64 flags;
 
 #define FULL_AP_CLIENT_STATE_SUPP(drv_flags) \
@@ -2359,6 +2361,30 @@  struct wpa_driver_ops {
 		       const u8 *seq, size_t seq_len,
 		       const u8 *key, size_t key_len);
 
+	/**
+	 * tx_control_port - Send a frame over the 802.1X controlled port
+	 * @priv: private driver interface data
+	 * @dest: Destination MAC address
+	 * @proto: Ethertype in host byte order
+	 * @buf: Frame payload starting from IEEE 802.1X header
+	 * @len: Frame payload length
+	 *
+	 * Returns 0 on success, else an error
+	 *
+	 * This is like a normal ethernet send except that the OS device driver
+	 * is aware (by other means than the ethertype) that this frame is
+	 * ~special~, and more importantly it gains an ordering between the
+	 * transmission of the frame and other driver operations such as key
+	 * installations. This can be used to work around known limitations in
+	 * 802.11 protocols such as race conditions between 802.1X rekeying
+	 * handshake message 4/4 and a PTK being overwritten.
+	 *
+	 * This function is only implemented for a given interface if the driver
+	 * instance reports WPA_DRIVER_FLAGS_TX_CONTROL_PORT. Otherwise API
+	 * users should fall back to sending the frame via a normal socket.
+	 */
+	int (*tx_control_port)(void *priv, const u8 *dest,
+			       u16 proto, const u8 *buf, size_t len);
 	/**
 	 * init - Initialize driver interface
 	 * @ctx: context to be used when calling wpa_supplicant functions,
diff --git a/src/drivers/driver_nl80211.c b/src/drivers/driver_nl80211.c
index 967a24225..d20abce87 100644
--- a/src/drivers/driver_nl80211.c
+++ b/src/drivers/driver_nl80211.c
@@ -3011,6 +3011,44 @@  static int nl80211_set_pmk(struct wpa_driver_nl80211_data *drv,
 }
 
 
+static int driver_nl80211_tx_control_port(void *priv, const u8 *dest,
+					  u16 proto, const u8 *buf, size_t len)
+{
+	struct i802_bss *bss = priv;
+	struct nl_msg *msg = NULL;
+	int ifindex = if_nametoindex(bss->ifname);
+	int ret = 0;
+
+	wpa_printf(MSG_DEBUG,
+		   "nl80211: tx_control_port "MACSTR" proto=0x%04x len=%zu",
+		   MAC2STR(dest), proto, len);
+
+	msg = nl80211_ifindex_msg(bss->drv, ifindex, 0,
+				  NL80211_CMD_CONTROL_PORT_FRAME);
+	if (!msg)
+		return -ENOBUFS;
+
+	if (nla_put_u16(msg, NL80211_ATTR_CONTROL_PORT_ETHERTYPE, proto))
+		goto fail;
+	if (nla_put(msg, NL80211_ATTR_MAC, ETH_ALEN, dest))
+		goto fail;
+	if (nla_put(msg, NL80211_ATTR_FRAME, len, buf))
+		goto fail;
+
+	ret = send_and_recv_msgs(bss->drv, msg, NULL, NULL);
+	if (ret)
+		wpa_printf(MSG_DEBUG,
+			   "nl80211: tx_control_port failed: ret=%d (%s)",
+			   ret, strerror(ret));
+
+	return ret;
+
+fail:
+	nl80211_nlmsg_clear(msg);
+	nlmsg_free(msg);
+	return -ENOBUFS;
+}
+
 static int wpa_driver_nl80211_set_key(const char *ifname, struct i802_bss *bss,
 				      enum wpa_alg alg, const u8 *addr,
 				      int key_idx, int set_tx,
@@ -11194,6 +11232,7 @@  const struct wpa_driver_ops wpa_driver_nl80211_ops = {
 	.get_bssid = wpa_driver_nl80211_get_bssid,
 	.get_ssid = wpa_driver_nl80211_get_ssid,
 	.set_key = driver_nl80211_set_key,
+	.tx_control_port = driver_nl80211_tx_control_port,
 	.scan2 = driver_nl80211_scan2,
 	.sched_scan = wpa_driver_nl80211_sched_scan,
 	.stop_sched_scan = wpa_driver_nl80211_stop_sched_scan,
diff --git a/src/drivers/driver_nl80211_capa.c b/src/drivers/driver_nl80211_capa.c
index 9a82cd1e5..8305b05bb 100644
--- a/src/drivers/driver_nl80211_capa.c
+++ b/src/drivers/driver_nl80211_capa.c
@@ -433,6 +433,10 @@  static void wiphy_info_ext_feature_flags(struct wiphy_info_data *info,
 	if (ext_feature_isset(ext_features, len,
 			      NL80211_EXT_FEATURE_ENABLE_FTM_RESPONDER))
 		capa->flags |= WPA_DRIVER_FLAGS_FTM_RESPONDER;
+
+	if (ext_feature_isset(ext_features, len,
+			      NL80211_EXT_FEATURE_CONTROL_PORT_OVER_NL80211))
+		capa->flags |= WPA_DRIVER_FLAGS_TX_CONTROL_PORT;
 }
 
 
diff --git a/src/rsn_supp/wpa.c b/src/rsn_supp/wpa.c
index e9df04e11..e55351352 100644
--- a/src/rsn_supp/wpa.c
+++ b/src/rsn_supp/wpa.c
@@ -158,7 +158,7 @@  int wpa_eapol_key_send(struct wpa_sm *sm, struct wpa_ptk *ptk,
 	}
 
 	wpa_hexdump(MSG_MSGDUMP, "WPA: TX EAPOL-Key", msg, msg_len);
-	ret = wpa_sm_ether_send(sm, dest, proto, msg, msg_len);
+	ret = wpa_sm_tx_control_port(sm, dest, proto, msg, msg_len);
 	eapol_sm_notify_tx_eapol_key(sm->eapol);
 out:
 	os_free(msg);
diff --git a/src/rsn_supp/wpa.h b/src/rsn_supp/wpa.h
index f1fbb1bb5..1ef87dbb7 100644
--- a/src/rsn_supp/wpa.h
+++ b/src/rsn_supp/wpa.h
@@ -33,6 +33,8 @@  struct wpa_sm_ctx {
 		       const u8 *key, size_t key_len);
 	void * (*get_network_ctx)(void *ctx);
 	int (*get_bssid)(void *ctx, u8 *bssid);
+	int (*tx_control_port)(void *ctx, const u8 *dest, u16 proto,
+			       const u8 *buf, size_t len);
 	int (*ether_send)(void *ctx, const u8 *dest, u16 proto, const u8 *buf,
 			  size_t len);
 	int (*get_beacon_ie)(void *ctx);
diff --git a/src/rsn_supp/wpa_i.h b/src/rsn_supp/wpa_i.h
index 2a433425c..ec2e8e83d 100644
--- a/src/rsn_supp/wpa_i.h
+++ b/src/rsn_supp/wpa_i.h
@@ -217,6 +217,13 @@  static inline int wpa_sm_get_bssid(struct wpa_sm *sm, u8 *bssid)
 	return sm->ctx->get_bssid(sm->ctx->ctx, bssid);
 }
 
+static inline int wpa_sm_tx_control_port(struct wpa_sm *sm, const u8 *dest,
+					 u16 proto, const u8 *buf, size_t len)
+{
+	WPA_ASSERT(sm->ctx->tx_control_port);
+	return sm->ctx->tx_control_port(sm->ctx->ctx, dest, proto, buf, len);
+}
+
 static inline int wpa_sm_ether_send(struct wpa_sm *sm, const u8 *dest,
 				    u16 proto, const u8 *buf, size_t len)
 {
diff --git a/wpa_supplicant/driver_i.h b/wpa_supplicant/driver_i.h
index f8d63a07b..738666e40 100644
--- a/wpa_supplicant/driver_i.h
+++ b/wpa_supplicant/driver_i.h
@@ -143,6 +143,14 @@  static inline int wpa_drv_get_ssid(struct wpa_supplicant *wpa_s, u8 *ssid)
 	return -1;
 }
 
+static inline int wpa_drv_tx_control_port(struct wpa_supplicant *wpa_s,
+                                          const u8 *dest,
+                                          u16 proto, const u8 *buf, size_t len)
+{
+	return wpa_s->driver->tx_control_port(wpa_s->drv_priv,
+					      dest, proto, buf, len);
+}
+
 static inline int wpa_drv_set_key(struct wpa_supplicant *wpa_s,
 				  enum wpa_alg alg, const u8 *addr,
 				  int key_idx, int set_tx,
diff --git a/wpa_supplicant/ibss_rsn.c b/wpa_supplicant/ibss_rsn.c
index 48dd66b12..cd940e40c 100644
--- a/wpa_supplicant/ibss_rsn.c
+++ b/wpa_supplicant/ibss_rsn.c
@@ -76,6 +76,23 @@  static int supp_ether_send(void *ctx, const u8 *dest, u16 proto, const u8 *buf,
 }
 
 
+static int supp_tx_control_port(void *ctx, const u8 *dest, u16 proto,
+				const u8 *buf, size_t len)
+{
+	struct ibss_rsn_peer *peer = ctx;
+	struct wpa_supplicant *wpa_s = peer->ibss_rsn->wpa_s;
+
+	wpa_printf(MSG_DEBUG, "SUPP: %s(dest=" MACSTR " proto=0x%04x "
+		   "len=%lu)",
+		   __func__, MAC2STR(dest), proto, (unsigned long) len);
+
+	if (wpa_s->drv_flags & WPA_DRIVER_FLAGS_TX_CONTROL_PORT)
+		return wpa_drv_tx_control_port(wpa_s, dest, proto, buf, len);
+	else
+		return supp_ether_send(wpa_s, dest, proto, buf, len);
+}
+
+
 static u8 * supp_alloc_eapol(void *ctx, u8 type, const void *data,
 			     u16 data_len, size_t *msg_len, void **data_pos)
 {
@@ -212,6 +229,7 @@  static int ibss_rsn_supp_init(struct ibss_rsn_peer *peer, const u8 *own_addr,
 	ctx->set_state = supp_set_state;
 	ctx->get_state = supp_get_state;
 	ctx->ether_send = supp_ether_send;
+	ctx->tx_control_port = supp_tx_control_port;
 	ctx->get_beacon_ie = supp_get_beacon_ie;
 	ctx->alloc_eapol = supp_alloc_eapol;
 	ctx->set_key = supp_set_key;
diff --git a/wpa_supplicant/wpas_glue.c b/wpa_supplicant/wpas_glue.c
index d80b8f28d..eb56c8b5b 100644
--- a/wpa_supplicant/wpas_glue.c
+++ b/wpa_supplicant/wpas_glue.c
@@ -85,17 +85,9 @@  static u8 * wpa_alloc_eapol(const struct wpa_supplicant *wpa_s, u8 type,
 }
 
 
-/**
- * wpa_ether_send - Send Ethernet frame
- * @wpa_s: Pointer to wpa_supplicant data
- * @dest: Destination MAC address
- * @proto: Ethertype in host byte order
- * @buf: Frame payload starting from IEEE 802.1X header
- * @len: Frame payload length
- * Returns: >=0 on success, <0 on failure
- */
-static int wpa_ether_send(struct wpa_supplicant *wpa_s, const u8 *dest,
-			  u16 proto, const u8 *buf, size_t len)
+static inline int ext_eapol_frame_io_notify_tx(struct wpa_supplicant *wpa_s,
+					       const u8 *dest, u16 proto,
+					       const u8 *buf, size_t len)
 {
 #ifdef CONFIG_TESTING_OPTIONS
 	if (wpa_s->ext_eapol_frame_io && proto == ETH_P_EAPOL) {
@@ -108,18 +100,60 @@  static int wpa_ether_send(struct wpa_supplicant *wpa_s, const u8 *dest,
 		wpa_msg(wpa_s, MSG_INFO, "EAPOL-TX " MACSTR " %s",
 			MAC2STR(dest), hex);
 		os_free(hex);
-		return 0;
+		return -1;
 	}
 #endif /* CONFIG_TESTING_OPTIONS */
 
+	return 0;
+}
+
+/**
+ * wpa_ether_send - Send Ethernet frame
+ * @wpa_s: Pointer to wpa_supplicant data
+ * @dest: Destination MAC address
+ * @proto: Ethertype in host byte order
+ * @buf: Frame payload starting from IEEE 802.1X header
+ * @len: Frame payload length
+ * Returns: >=0 on success, <0 on failure
+ */
+static int wpa_ether_send(struct wpa_supplicant *wpa_s, const u8 *dest,
+			  u16 proto, const u8 *buf, size_t len)
+{
+	if (ext_eapol_frame_io_notify_tx(wpa_s, dest, proto, buf, len))
+		return 0;
+
 	if (wpa_s->l2) {
 		return l2_packet_send(wpa_s->l2, dest, proto, buf, len);
 	}
 
 	return -1;
 }
-#endif /* IEEE8021X_EAPOL || !CONFIG_NO_WPA */
 
+/**
+ * wpa_supplicant_tx_control_port - Send Ethernet frame over 802.1X control port
+ * @wpa_s: Pointer to wpa_supplicant data
+ * @dest: Destination MAC address
+ * @proto: Ethertype in host byte order
+ * @buf: Frame payload starting from IEEE 802.1X header
+ * @len: Frame payload length
+ * Just like wpa_ether_send, but when this function is used the driver may be
+ * able to handle control port frames specially.
+ * Returns: >=0 on success, <0 on failure
+ */
+static int wpa_supplicant_tx_control_port(void *ctx, const u8 *dest,
+					  u16 proto, const u8 *buf, size_t len)
+{
+	struct wpa_supplicant *wpa_s = ctx;
+
+	if (wpa_s->drv_flags & WPA_DRIVER_FLAGS_TX_CONTROL_PORT) {
+		if (ext_eapol_frame_io_notify_tx(wpa_s, dest, proto, buf, len))
+			return 0;
+		return wpa_drv_tx_control_port(wpa_s, dest, proto, buf, len);
+	} else {
+		return wpa_ether_send(wpa_s, dest, proto, buf, len);
+	}
+}
+#endif /* IEEE8021X_EAPOL || !CONFIG_NO_WPA */
 
 #ifdef IEEE8021X_EAPOL
 
@@ -1215,6 +1249,7 @@  int wpa_supplicant_init_wpa(struct wpa_supplicant *wpa_s)
 	ctx->get_network_ctx = wpa_supplicant_get_network_ctx;
 	ctx->get_bssid = wpa_supplicant_get_bssid;
 	ctx->ether_send = _wpa_ether_send;
+	ctx->tx_control_port = wpa_supplicant_tx_control_port;
 	ctx->get_beacon_ie = wpa_supplicant_get_beacon_ie;
 	ctx->alloc_eapol = _wpa_alloc_eapol;
 	ctx->cancel_auth_timeout = _wpa_supplicant_cancel_auth_timeout;