[PATCH 1/4] ap: Pass vendor IEs between frames and the user

[PATCH 1/4] ap: Pass vendor IEs between frames and the user

[Andrew Zaborowski]

[-- Attachment #1: Type: text/plain, Size: 14922 bytes --]

Add API for the ap.h users to add extra vendor IEs to outgoing
management frames (beacons, etc.).  Pass the string IEs from the
incoming STA association frames to the user in the AP event data.

I drop ap_event_station_added_data.rsn_ie because that probably
wasn't going to ever be useful and the RSN IE is included in the
assoc_ies array in any case.
---
 src/ap.c | 233 ++++++++++++++++++++++++++++++++++++++++---------------
 src/ap.h |  15 +++-
 2 files changed, 186 insertions(+), 62 deletions(-)

diff --git a/src/ap.c b/src/ap.c
index 8ea26a0d..dbc624c2 100644
--- a/src/ap.c
+++ b/src/ap.c
@@ -75,6 +75,8 @@ struct ap_state {
 	uint16_t wsc_dpid;
 	uint8_t wsc_uuid_r[16];
 
+	struct l_queue *vendor_ie_writers;
+
 	uint16_t last_aid;
 	struct l_queue *sta_states;
 
@@ -99,6 +101,8 @@ struct sta_state {
 	struct l_uintset *rates;
 	uint32_t assoc_resp_cmd_id;
 	struct ap_state *ap;
+	uint8_t *assoc_ies;
+	size_t assoc_ies_len;
 	uint8_t *assoc_rsne;
 	struct eapol_sm *sm;
 	struct handshake_state *hs;
@@ -127,6 +131,12 @@ struct ap_ip_pool {
 	struct l_uintset *used;
 };
 
+struct ap_vendor_ie_writer {
+	ap_get_vendor_ie_len get_len_func;
+	ap_write_vendor_ie write_func;
+	void *user_data;
+};
+
 struct ap_ip_pool pool;
 static uint32_t netdev_watch;
 struct l_netlink *rtnl;
@@ -279,7 +289,7 @@ static void ap_sta_free(void *data)
 	struct ap_state *ap = sta->ap;
 
 	l_uintset_free(sta->rates);
-	l_free(sta->assoc_rsne);
+	l_free(sta->assoc_ies);
 
 	if (sta->assoc_resp_cmd_id)
 		l_genl_family_cancel(ap->nl80211, sta->assoc_resp_cmd_id);
@@ -411,7 +421,8 @@ static void ap_new_rsna(struct sta_state *sta)
 	if (ap->ops->handle_event) {
 		struct ap_event_station_added_data event_data = {};
 		event_data.mac = sta->addr;
-		event_data.rsn_ie = sta->assoc_rsne;
+		event_data.assoc_ies = sta->assoc_ies;
+		event_data.assoc_ies_len = sta->assoc_ies_len;
 		ap->ops->handle_event(AP_EVENT_STATION_ADDED, &event_data,
 					ap->user_data);
 	}
@@ -463,11 +474,51 @@ static void ap_set_rsn_info(struct ap_state *ap, struct ie_rsn_info *rsn)
 	rsn->group_cipher = ap->group_cipher;
 }
 
+static size_t ap_get_vendor_ies_len(struct ap_state *ap,
+					enum mpdu_management_subtype type)
+{
+	const struct l_queue_entry *entry;
+	size_t len = 0;
+
+	for (entry = l_queue_get_entries(ap->sta_states); entry;
+			entry = entry->next) {
+		struct ap_vendor_ie_writer *writer = entry->data;
+
+		len += writer->get_len_func(type, writer->user_data);
+	}
+
+	return len;
+}
+
+static size_t ap_write_vendor_ies(struct ap_state *ap,
+					enum mpdu_management_subtype type,
+					uint8_t *out_buf)
+{
+	const struct l_queue_entry *entry;
+	size_t len = 0;
+
+	for (entry = l_queue_get_entries(ap->sta_states); entry;
+			entry = entry->next) {
+		struct ap_vendor_ie_writer *writer = entry->data;
+
+		len += writer->write_func(type, out_buf + len,
+						writer->user_data);
+	}
+
+	return len;
+}
+
 /*
  * Build a Beacon frame or a Probe Response frame's header and body until
  * the TIM IE.  Except for the optional TIM IE which is inserted by the
  * kernel when needed, our contents for both frames are the same.
  * See Beacon format in 8.3.3.2 and Probe Response format in 8.3.3.10.
+ *
+ * 802.11-2016, Section 9.4.2.1:
+ * "The frame body components specified for many management subtypes result
+ * in elements ordered by ascending values of the Element ID field and then
+ * the Element ID Extension field (when present), with the exception of the
+ * MIC Management element (9.4.2.55)."
  */
 static size_t ap_build_beacon_pr_head(struct ap_state *ap,
 					enum mpdu_management_subtype stype,
@@ -533,15 +584,12 @@ static size_t ap_build_beacon_pr_head(struct ap_state *ap,
 }
 
 /* Beacon / Probe Response frame portion after the TIM IE */
-static size_t ap_build_beacon_pr_tail(struct ap_state *ap, bool pr,
+static size_t ap_build_beacon_pr_tail(struct ap_state *ap,
+					enum mpdu_management_subtype stype,
 					uint8_t *out_buf)
 {
 	size_t len;
 	struct ie_rsn_info rsn;
-	uint8_t *wsc_data;
-	size_t wsc_data_size;
-	uint8_t *wsc_ie;
-	size_t wsc_ie_size;
 
 	/* TODO: Country IE between TIM IE and RSNE */
 
@@ -551,8 +599,71 @@ static size_t ap_build_beacon_pr_tail(struct ap_state *ap, bool pr,
 		return 0;
 	len = 2 + out_buf[1];
 
+	len += ap_write_vendor_ies(ap, stype, out_buf + len);
+	return len;
+}
+
+static void ap_set_beacon_cb(struct l_genl_msg *msg, void *user_data)
+{
+	int error = l_genl_msg_get_error(msg);
+
+	if (error < 0)
+		l_error("SET_BEACON failed: %s (%i)", strerror(-error), -error);
+}
+
+static void ap_update_beacon(struct ap_state *ap)
+{
+	struct l_genl_msg *cmd;
+	uint8_t head[256];
+	uint8_t tail[256 + ap_get_vendor_ies_len(ap,
+					MPDU_MANAGEMENT_SUBTYPE_BEACON)];
+	size_t head_len, tail_len;
+	uint64_t wdev_id = netdev_get_wdev_id(ap->netdev);
+	static const uint8_t bcast_addr[6] = {
+		0xff, 0xff, 0xff, 0xff, 0xff, 0xff
+	};
+
+	head_len = ap_build_beacon_pr_head(ap, MPDU_MANAGEMENT_SUBTYPE_BEACON,
+						bcast_addr, head, sizeof(head));
+	tail_len = ap_build_beacon_pr_tail(ap, MPDU_MANAGEMENT_SUBTYPE_BEACON,
+						tail);
+	if (L_WARN_ON(!head_len || !tail_len))
+		return;
+
+	cmd = l_genl_msg_new_sized(NL80211_CMD_SET_BEACON,
+					32 + head_len + tail_len);
+	l_genl_msg_append_attr(cmd, NL80211_ATTR_WDEV, 8, &wdev_id);
+	l_genl_msg_append_attr(cmd, NL80211_ATTR_BEACON_HEAD, head_len, head);
+	l_genl_msg_append_attr(cmd, NL80211_ATTR_BEACON_TAIL, tail_len, tail);
+	l_genl_msg_append_attr(cmd, NL80211_ATTR_IE, 0, "");
+	l_genl_msg_append_attr(cmd, NL80211_ATTR_IE_PROBE_RESP, 0, "");
+	l_genl_msg_append_attr(cmd, NL80211_ATTR_IE_ASSOC_RESP, 0, "");
+
+	if (l_genl_family_send(ap->nl80211, cmd, ap_set_beacon_cb, NULL, NULL))
+		return;
+
+	l_genl_msg_unref(cmd);
+	l_error("Issuing SET_BEACON failed");
+}
+
+static size_t ap_get_wsc_ie_len(enum mpdu_management_subtype type,
+				void *user_data)
+{
+	return 256;
+}
+
+static size_t ap_write_wsc_ie(enum mpdu_management_subtype type,
+				uint8_t *out_buf, void *user_data)
+{
+	struct ap_state *ap = user_data;
+	uint8_t *wsc_data;
+	size_t wsc_data_size;
+	uint8_t *wsc_ie;
+	size_t wsc_ie_size;
+	size_t len = 0;
+
 	/* WSC IE */
-	if (pr) {
+	if (type == MPDU_MANAGEMENT_SUBTYPE_PROBE_RESPONSE) {
 		struct wsc_probe_response wsc_pr = {};
 
 		wsc_pr.version2 = true;
@@ -633,46 +744,6 @@ static size_t ap_build_beacon_pr_tail(struct ap_state *ap, bool pr,
 	return len;
 }
 
-static void ap_set_beacon_cb(struct l_genl_msg *msg, void *user_data)
-{
-	int error = l_genl_msg_get_error(msg);
-
-	if (error < 0)
-		l_error("SET_BEACON failed: %s (%i)", strerror(-error), -error);
-}
-
-static void ap_update_beacon(struct ap_state *ap)
-{
-	struct l_genl_msg *cmd;
-	uint8_t head[256], tail[256];
-	size_t head_len, tail_len;
-	uint64_t wdev_id = netdev_get_wdev_id(ap->netdev);
-	static const uint8_t bcast_addr[6] = {
-		0xff, 0xff, 0xff, 0xff, 0xff, 0xff
-	};
-
-	head_len = ap_build_beacon_pr_head(ap, MPDU_MANAGEMENT_SUBTYPE_BEACON,
-						bcast_addr, head, sizeof(head));
-	tail_len = ap_build_beacon_pr_tail(ap, false, tail);
-	if (L_WARN_ON(!head_len || !tail_len))
-		return;
-
-	cmd = l_genl_msg_new_sized(NL80211_CMD_SET_BEACON,
-					32 + head_len + tail_len);
-	l_genl_msg_append_attr(cmd, NL80211_ATTR_WDEV, 8, &wdev_id);
-	l_genl_msg_append_attr(cmd, NL80211_ATTR_BEACON_HEAD, head_len, head);
-	l_genl_msg_append_attr(cmd, NL80211_ATTR_BEACON_TAIL, tail_len, tail);
-	l_genl_msg_append_attr(cmd, NL80211_ATTR_IE, 0, "");
-	l_genl_msg_append_attr(cmd, NL80211_ATTR_IE_PROBE_RESP, 0, "");
-	l_genl_msg_append_attr(cmd, NL80211_ATTR_IE_ASSOC_RESP, 0, "");
-
-	if (l_genl_family_send(ap->nl80211, cmd, ap_set_beacon_cb, NULL, NULL))
-		return;
-
-	l_genl_msg_unref(cmd);
-	l_error("Issuing SET_BEACON failed");
-}
-
 static void ap_wsc_exit_pbc(struct ap_state *ap)
 {
 	if (!ap->wsc_pbc_timeout)
@@ -1194,7 +1265,10 @@ static uint32_t ap_assoc_resp(struct ap_state *ap, struct sta_state *sta,
 				bool reassoc, frame_xchg_cb_t callback)
 {
 	const uint8_t *addr = netdev_get_address(ap->netdev);
-	uint8_t mpdu_buf[128];
+	enum mpdu_management_subtype stype = reassoc ?
+		MPDU_MANAGEMENT_SUBTYPE_REASSOCIATION_RESPONSE :
+		MPDU_MANAGEMENT_SUBTYPE_ASSOCIATION_RESPONSE;
+	uint8_t mpdu_buf[128 + ap_get_vendor_ies_len(ap, stype)];
 	struct mmpdu_header *mpdu = (void *) mpdu_buf;
 	struct mmpdu_association_response *resp;
 	size_t ies_len = 0;
@@ -1206,9 +1280,7 @@ static uint32_t ap_assoc_resp(struct ap_state *ap, struct sta_state *sta,
 	/* Header */
 	mpdu->fc.protocol_version = 0;
 	mpdu->fc.type = MPDU_TYPE_MANAGEMENT;
-	mpdu->fc.subtype = reassoc ?
-		MPDU_MANAGEMENT_SUBTYPE_REASSOCIATION_RESPONSE :
-		MPDU_MANAGEMENT_SUBTYPE_ASSOCIATION_RESPONSE;
+	mpdu->fc.subtype = stype;
 	memcpy(mpdu->address_1, dest, 6);	/* DA */
 	memcpy(mpdu->address_2, addr, 6);	/* SA */
 	memcpy(mpdu->address_3, addr, 6);	/* BSSID */
@@ -1270,6 +1342,8 @@ static uint32_t ap_assoc_resp(struct ap_state *ap, struct sta_state *sta,
 		l_free(wsc_ie);
 	}
 
+	ies_len += ap_write_vendor_ies(ap, stype, resp->ies + ies_len);
+
 send_frame:
 	return ap_send_mgmt_frame(ap, mpdu, resp->ies + ies_len - mpdu_buf,
 					callback, sta);
@@ -1471,6 +1545,8 @@ static void ap_assoc_reassoc(struct sta_state *sta, bool reassoc,
 		sta->wsc_v2 = wsc_req.version2;
 
 		event_data.mac = sta->addr;
+		event_data.assoc_ies = ies;
+		event_data.assoc_ies_len = ies_len;
 		ap->ops->handle_event(AP_EVENT_REGISTRATION_START, &event_data,
 					ap->user_data);
 
@@ -1521,13 +1597,16 @@ static void ap_assoc_reassoc(struct sta_state *sta, bool reassoc,
 
 	sta->rates = rates;
 
-	if (sta->assoc_rsne)
-		l_free(sta->assoc_rsne);
+	l_free(sta->assoc_ies);
 
-	if (rsn)
-		sta->assoc_rsne = l_memdup(rsn, rsn[1] + 2);
-	else
+	if (rsn) {
+		sta->assoc_ies = l_memdup(ies, ies_len);
+		sta->assoc_ies_len = ies_len;
+		sta->assoc_rsne = sta->assoc_ies + (rsn - ies);
+	} else {
+		sta->assoc_ies = NULL;
 		sta->assoc_rsne = NULL;
+	}
 
 	sta->assoc_resp_cmd_id = ap_assoc_resp(ap, sta, sta->addr, 0, reassoc,
 						ap_success_assoc_resp_cb);
@@ -1774,7 +1853,8 @@ static void ap_probe_req_cb(const struct mmpdu_header *hdr, const void *body,
 	struct ie_tlv_iter iter;
 	const uint8_t *bssid = netdev_get_address(ap->netdev);
 	bool match = false;
-	uint8_t resp[512];
+	uint8_t resp[512 + ap_get_vendor_ies_len(ap,
+				MPDU_MANAGEMENT_SUBTYPE_PROBE_RESPONSE)];
 	uint8_t *wsc_data;
 	ssize_t wsc_data_len;
 
@@ -1864,7 +1944,9 @@ static void ap_probe_req_cb(const struct mmpdu_header *hdr, const void *body,
 	len = ap_build_beacon_pr_head(ap,
 					MPDU_MANAGEMENT_SUBTYPE_PROBE_RESPONSE,
 					hdr->address_2, resp, sizeof(resp));
-	len += ap_build_beacon_pr_tail(ap, true, resp + len);
+	len += ap_build_beacon_pr_tail(ap,
+					MPDU_MANAGEMENT_SUBTYPE_PROBE_RESPONSE,
+					resp + len);
 
 	ap_send_mgmt_frame(ap, (struct mmpdu_header *) resp, len,
 				ap_probe_resp_cb, NULL);
@@ -2102,7 +2184,9 @@ static struct l_genl_msg *ap_build_cmd_start_ap(struct ap_state *ap)
 {
 	struct l_genl_msg *cmd;
 
-	uint8_t head[256], tail[256];
+	uint8_t head[256];
+	uint8_t tail[256 + ap_get_vendor_ies_len(ap,
+					MPDU_MANAGEMENT_SUBTYPE_BEACON)];
 	size_t head_len, tail_len;
 
 	uint32_t dtim_period = 3;
@@ -2132,7 +2216,8 @@ static struct l_genl_msg *ap_build_cmd_start_ap(struct ap_state *ap)
 
 	head_len = ap_build_beacon_pr_head(ap, MPDU_MANAGEMENT_SUBTYPE_BEACON,
 						bcast_addr, head, sizeof(head));
-	tail_len = ap_build_beacon_pr_tail(ap, false, tail);
+	tail_len = ap_build_beacon_pr_tail(ap, MPDU_MANAGEMENT_SUBTYPE_BEACON,
+						tail);
 
 	if (!head_len || !tail_len)
 		return NULL;
@@ -2697,6 +2782,7 @@ struct ap_state *ap_start(struct netdev *netdev, struct ap_config *config,
 	}
 
 	wsc_uuid_from_addr(netdev_get_address(netdev), ap->wsc_uuid_r);
+	ap_add_vendor_ie_writer(ap, ap_get_wsc_ie_len, ap_write_wsc_ie, ap);
 
 	if (config->passphrase[0] &&
 			crypto_psk_from_passphrase(config->passphrase,
@@ -2876,9 +2962,34 @@ void ap_free(struct ap_state *ap)
 	l_genl_family_free(ap->nl80211);
 	if (ap->server)
 		l_dhcp_server_destroy(ap->server);
+
+	l_queue_destroy(ap->vendor_ie_writers, l_free);
 	l_free(ap);
 }
 
+void ap_add_vendor_ie_writer(struct ap_state *ap,
+				ap_get_vendor_ie_len get_len_func,
+				ap_write_vendor_ie write_func,
+				void *user_data)
+{
+	struct ap_vendor_ie_writer *writer =
+		l_new(struct ap_vendor_ie_writer, 1);
+
+	writer->get_len_func = get_len_func;
+	writer->write_func = write_func;
+	writer->user_data = user_data;
+
+	if (!ap->vendor_ie_writers)
+		ap->vendor_ie_writers = l_queue_new();
+
+	/*
+	 * It doesn't look like vendor IEs need to be ordered in a specific
+	 * way so just append at the end, but if needed we can take an OUI
+	 * parameter and switch to inserting at the right place in the queue.
+	 */
+	l_queue_push_tail(ap->vendor_ie_writers, writer);
+}
+
 bool ap_station_disconnect(struct ap_state *ap, const uint8_t *mac,
 				enum mmpdu_reason_code reason)
 {
diff --git a/src/ap.h b/src/ap.h
index dc57a0bb..eb4f96df 100644
--- a/src/ap.h
+++ b/src/ap.h
@@ -22,6 +22,7 @@
 
 struct ap_state;
 struct iovec;
+enum mpdu_management_subtype;
 
 enum ap_event_type {
 	AP_EVENT_START_FAILED,
@@ -36,7 +37,8 @@ enum ap_event_type {
 
 struct ap_event_station_added_data {
 	const uint8_t *mac;
-	const uint8_t *rsn_ie;
+	const uint8_t *assoc_ies;
+	size_t assoc_ies_len;
 };
 
 struct ap_event_station_removed_data {
@@ -46,6 +48,8 @@ struct ap_event_station_removed_data {
 
 struct ap_event_registration_start_data {
 	const uint8_t *mac;
+	const uint8_t *assoc_ies;
+	size_t assoc_ies_len;
 };
 
 struct ap_event_registration_success_data {
@@ -53,6 +57,10 @@ struct ap_event_registration_success_data {
 };
 
 typedef void (*ap_stopped_func_t)(void *user_data);
+typedef size_t (*ap_get_vendor_ie_len)(enum mpdu_management_subtype type,
+					void *user_data);
+typedef size_t (*ap_write_vendor_ie)(enum mpdu_management_subtype type,
+					uint8_t *out_buf, void *user_data);
 
 struct ap_config {
 	char *ssid;
@@ -83,6 +91,11 @@ void ap_shutdown(struct ap_state *ap, ap_stopped_func_t stopped_func,
 			void *user_data);
 void ap_free(struct ap_state *ap);
 
+void ap_add_vendor_ie_writer(struct ap_state *ap,
+				ap_get_vendor_ie_len get_len_func,
+				ap_write_vendor_ie write_func,
+				void *user_data);
+
 bool ap_station_disconnect(struct ap_state *ap, const uint8_t *mac,
 				enum mmpdu_reason_code reason);
 
-- 
2.27.0

[PATCH 2/4] ap: Make ap_update_beacon public

[Andrew Zaborowski]

[-- Attachment #1: Type: text/plain, Size: 1365 bytes --]

Let users call ap_update_beacon when a value has changed which should be
reflected in the beacon IEs.  It will cause the registered vendor IE
writers to be re-run.
---
 src/ap.c | 5 ++++-
 src/ap.h | 1 +
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/src/ap.c b/src/ap.c
index dbc624c2..c9de2651 100644
--- a/src/ap.c
+++ b/src/ap.c
@@ -611,7 +611,7 @@ static void ap_set_beacon_cb(struct l_genl_msg *msg, void *user_data)
 		l_error("SET_BEACON failed: %s (%i)", strerror(-error), -error);
 }
 
-static void ap_update_beacon(struct ap_state *ap)
+void ap_update_beacon(struct ap_state *ap)
 {
 	struct l_genl_msg *cmd;
 	uint8_t head[256];
@@ -623,6 +623,9 @@ static void ap_update_beacon(struct ap_state *ap)
 		0xff, 0xff, 0xff, 0xff, 0xff, 0xff
 	};
 
+	if (L_WARN_ON(!ap->started))
+		return;
+
 	head_len = ap_build_beacon_pr_head(ap, MPDU_MANAGEMENT_SUBTYPE_BEACON,
 						bcast_addr, head, sizeof(head));
 	tail_len = ap_build_beacon_pr_tail(ap, MPDU_MANAGEMENT_SUBTYPE_BEACON,
diff --git a/src/ap.h b/src/ap.h
index eb4f96df..9ee7a0b8 100644
--- a/src/ap.h
+++ b/src/ap.h
@@ -100,3 +100,4 @@ bool ap_station_disconnect(struct ap_state *ap, const uint8_t *mac,
 				enum mmpdu_reason_code reason);
 
 bool ap_push_button(struct ap_state *ap);
+void ap_update_beacon(struct ap_state *ap);
-- 
2.27.0

[PATCH 3/4] p2p: Parse P2P IEs and WFD IEs in Association Requests

[Andrew Zaborowski]

[-- Attachment #1: Type: text/plain, Size: 13454 bytes --]

Roughly validate the IEs and save some information for use in our own
IEs. p2p_extract_wfd_properties and p2p_device_validate_conn_wfd are
being moved unchanged to be usable in p2p_group_event without forward
declarations and to be next to p2p_build_wfd_ie.
---
 src/p2p.c | 385 ++++++++++++++++++++++++++++++++----------------------
 1 file changed, 228 insertions(+), 157 deletions(-)

diff --git a/src/p2p.c b/src/p2p.c
index 5e091f54..e5b8c93d 100644
--- a/src/p2p.c
+++ b/src/p2p.c
@@ -114,6 +114,7 @@ struct p2p_device {
 	uint32_t conn_go_oper_freq;
 	uint8_t conn_peer_interface_addr[6];
 	struct p2p_capability_attr conn_peer_capability;
+	struct p2p_device_info_attr conn_peer_dev_info;
 
 	struct p2p_group_id_attr go_group_id;
 	struct ap_state *group;
@@ -335,6 +336,163 @@ static size_t p2p_build_wfd_ie(const struct p2p_wfd_properties *wfd,
 	return size;
 }
 
+static bool p2p_extract_wfd_properties(const uint8_t *ie, size_t ie_size,
+					struct p2p_wfd_properties *out)
+{
+	struct wfd_subelem_iter iter;
+	const uint8_t *devinfo = NULL;
+	const uint8_t *ext_caps = NULL;
+	const uint8_t *r2 = NULL;
+
+	if (!ie)
+		return false;
+
+	wfd_subelem_iter_init(&iter, ie, ie_size);
+
+	while (wfd_subelem_iter_next(&iter)) {
+		enum wfd_subelem_type type = wfd_subelem_iter_get_type(&iter);
+		size_t len = wfd_subelem_iter_get_length(&iter);
+		const uint8_t *data = wfd_subelem_iter_get_data(&iter);
+
+		switch (type) {
+#define SUBELEM_CHECK(var, expected_len, name)		\
+			if (len != expected_len) {	\
+				l_debug(name " length wrong in WFD IE");\
+				return false;		\
+			}				\
+							\
+			if (var) {			\
+				l_debug("Duplicate" name " in WFD IE");\
+				return false;		\
+			}				\
+							\
+			var = data;
+		case WFD_SUBELEM_WFD_DEVICE_INFORMATION:
+			SUBELEM_CHECK(devinfo, 6, "Device Information");
+			break;
+		case WFD_SUBELEM_EXTENDED_CAPABILITY:
+			SUBELEM_CHECK(ext_caps, 2, "Extended Capability");
+			break;
+		case WFD_SUBELEM_R2_DEVICE_INFORMATION:
+			SUBELEM_CHECK(r2, 2, "R2 Device Information");
+			break;
+#undef SUBELEM_CHECK
+		default:
+			/* Skip unknown IEs */
+			break;
+		}
+	}
+
+	if (devinfo) {
+		uint16_t capability = l_get_be16(devinfo + 0);
+		bool source;
+		bool sink;
+		uint16_t port;
+
+		source = (capability & WFD_DEV_INFO_DEVICE_TYPE) ==
+			WFD_DEV_INFO_TYPE_SOURCE ||
+			(capability & WFD_DEV_INFO_DEVICE_TYPE) ==
+			WFD_DEV_INFO_TYPE_DUAL_ROLE;
+		sink = (capability & WFD_DEV_INFO_DEVICE_TYPE) ==
+			WFD_DEV_INFO_TYPE_PRIMARY_SINK ||
+			(capability & WFD_DEV_INFO_DEVICE_TYPE) ==
+			WFD_DEV_INFO_TYPE_DUAL_ROLE;
+
+		if (!source && !sink)
+			return false;
+
+		port = l_get_be16(devinfo + 2);
+
+		if (source && port == 0) {
+			l_debug("0 port number in WFD IE Device Information");
+			return false;
+		}
+
+		memset(out, 0, sizeof(*out));
+		out->available =
+			(capability & WFD_DEV_INFO_SESSION_AVAILABILITY) ==
+			WFD_DEV_INFO_SESSION_AVAILABLE;
+		out->source = source;
+		out->sink = sink;
+		out->port = port;
+		out->cp = capability & WFD_DEV_INFO_CONTENT_PROTECTION_SUPPORT;
+		out->audio = !sink ||
+			!(capability & WFD_DEV_INFO_NO_AUDIO_AT_PRIMARY_SINK);
+	} else {
+		l_error("Device Information missing in WFD IE");
+		return false;
+	}
+
+	if (ext_caps && (l_get_be16(ext_caps) & 1))
+		out->uibc = 1;
+
+	if (r2) {
+		uint8_t role = l_get_be16(r2) & 3;
+
+		if ((out->source && role != 0 && role != 3) ||
+				(out->sink && role != 1 && role != 3))
+			l_debug("Invalid role in WFD R2 Device Information");
+		else
+			out->r2 = true;
+	}
+
+	return true;
+}
+
+static bool p2p_device_validate_conn_wfd(struct p2p_device *dev,
+						const uint8_t *ie,
+						ssize_t ie_size)
+{
+	struct p2p_wfd_properties wfd;
+
+	if (!dev->conn_own_wfd)
+		return true;
+
+	/*
+	 * WFD IEs are optional in Association Request/Response and P2P Public
+	 * Action frames for R2 devices and required for R1 devices.
+	 * Wi-Fi Display Technical Specification v2.1.0 section 5.2:
+	 * "A WFD R2 Device shall include the WFD IE in Beacon, Probe
+	 * Request/Response, Association Request/Response and P2P Public Action
+	 * frames in order to be interoperable with R1 devices. If a WFD R2
+	 * Device discovers that the peer device is also a WFD R2 Device, then
+	 * it may include the WFD IE in Association Request/Response and P2P
+	 * Public Action frames."
+	 */
+	if (!ie)
+		return dev->conn_own_wfd->r2;
+
+	if (!p2p_extract_wfd_properties(ie, ie_size, &wfd)) {
+		l_error("Could not parse the WFD IE contents");
+		return false;
+	}
+
+	if ((dev->conn_own_wfd->source && !wfd.sink) ||
+			(dev->conn_own_wfd->sink && !wfd.source)) {
+		l_error("Wrong role in peer's WFD IE");
+		return false;
+	}
+
+	if (wfd.r2 != dev->conn_own_wfd->r2) {
+		l_error("Wrong version in peer's WFD IE");
+		return false;
+	}
+
+	/*
+	 * Ignore the session available state because it's not 100% clear
+	 * at what point the peer switches to SESSION_NOT_AVAILABLE in its
+	 * Device Information.
+	 * But we might want to check that other bits have not changed from
+	 * what the peer reported during discovery.
+	 * Wi-Fi Display Technical Specification v2.1.0 section 4.5.2.1:
+	 * "The content of the WFD Device Information subelement should be
+	 * immutable during the period of P2P connection establishment, with
+	 * [...] exceptions [...]"
+	 */
+
+	return true;
+}
+
 /* TODO: convert to iovecs */
 static uint8_t *p2p_build_scan_ies(struct p2p_device *dev, uint8_t *buf,
 					size_t buf_len, size_t *out_len)
@@ -691,9 +849,72 @@ static void p2p_group_event(enum ap_event_type type, const void *event_data,
 		break;
 
 	case AP_EVENT_STATION_ADDED:
+	{
+		const struct ap_event_station_added_data *data = event_data;
+		L_AUTO_FREE_VAR(uint8_t *, p2p_data) = NULL;
+		ssize_t p2p_data_len;
+		L_AUTO_FREE_VAR(uint8_t *, wfd_data) = NULL;
+		ssize_t wfd_data_len;
+		struct p2p_association_req req_info;
+		int r;
+
+		p2p_data = ie_tlv_extract_p2p_payload(data->assoc_ies,
+							data->assoc_ies_len,
+							&p2p_data_len);
+		if (!p2p_data) {
+			l_error("Missing or invalid P2P IEs: %s (%i)",
+				strerror(-p2p_data_len), (int) -p2p_data_len);
+			goto invalid_ie;
+		}
+
+		/*
+		 * We don't need to validate most of the Association Request
+		 * P2P IE contents as we already have all the information there
+		 * may be but we need to save some of the attributes because
+		 * section 3.2.3 requires that our Group Info includes
+		 * specifically the data from the association, not any of the
+		 * earlier exchanges:
+		 * "When a P2P Client associates with a P2P Group Owner, it
+		 * provides [...] the P2P Device Info attribute (see Section
+		 * 4.1.15) and the P2P Capability attribute (see Section 4.1.4)
+		 * in the P2P IE in the Association Request frame.  This
+		 * information shall be used by the P2P Group Owner for Group
+		 * Information Advertisement.
+		 */
+		r = p2p_parse_association_req(p2p_data, p2p_data_len,
+						&req_info);
+		if (r < 0) {
+			l_error("Can't parse P2P Association Request: %s (%i)",
+				strerror(-r), -r);
+			goto invalid_ie;
+		}
+
+		/*
+		 * Most of this duplicates the information we already have in
+		 * dev->conn_peer.
+		 */
+		dev->conn_peer_capability = req_info.capability;
+		dev->conn_peer_dev_info = req_info.device_info;
+		p2p_clear_association_req(&req_info);
+
+		if (dev->conn_own_wfd)
+			wfd_data = ie_tlv_extract_p2p_payload(data->assoc_ies,
+							data->assoc_ies_len,
+							&wfd_data_len);
+
+		if (!p2p_device_validate_conn_wfd(dev, wfd_data,
+							wfd_data_len))
+			goto invalid_ie;
+
+		/* Take the chance to update WFD attributes for Session Info */
+		if (wfd_data)
+			p2p_extract_wfd_properties(wfd_data, wfd_data_len,
+							dev->conn_peer->wfd);
+
 		dev->conn_peer_added = true;
 		p2p_peer_connect_done(dev);
 		break;
+	}
 
 	case AP_EVENT_STATION_REMOVED:
 		dev->conn_peer_added = false;
@@ -709,6 +930,13 @@ static void p2p_group_event(enum ap_event_type type, const void *event_data,
 	case AP_EVENT_PBC_MODE_EXIT:
 		break;
 	};
+
+	return;
+
+invalid_ie:
+	ap_station_disconnect(dev->group, dev->conn_peer_interface_addr,
+				MMPDU_REASON_CODE_INVALID_IE);
+	p2p_connect_failed(dev);
 }
 
 static const struct ap_ops p2p_go_ops = {
@@ -1627,163 +1855,6 @@ static void p2p_device_fill_channel_list(struct p2p_device *dev,
 	l_queue_push_tail(attr->channel_entries, channel_entry);
 }
 
-static bool p2p_extract_wfd_properties(const uint8_t *ie, size_t ie_size,
-					struct p2p_wfd_properties *out)
-{
-	struct wfd_subelem_iter iter;
-	const uint8_t *devinfo = NULL;
-	const uint8_t *ext_caps = NULL;
-	const uint8_t *r2 = NULL;
-
-	if (!ie)
-		return false;
-
-	wfd_subelem_iter_init(&iter, ie, ie_size);
-
-	while (wfd_subelem_iter_next(&iter)) {
-		enum wfd_subelem_type type = wfd_subelem_iter_get_type(&iter);
-		size_t len = wfd_subelem_iter_get_length(&iter);
-		const uint8_t *data = wfd_subelem_iter_get_data(&iter);
-
-		switch (type) {
-#define SUBELEM_CHECK(var, expected_len, name)		\
-			if (len != expected_len) {	\
-				l_debug(name " length wrong in WFD IE");\
-				return false;		\
-			}				\
-							\
-			if (var) {			\
-				l_debug("Duplicate" name " in WFD IE");\
-				return false;		\
-			}				\
-							\
-			var = data;
-		case WFD_SUBELEM_WFD_DEVICE_INFORMATION:
-			SUBELEM_CHECK(devinfo, 6, "Device Information");
-			break;
-		case WFD_SUBELEM_EXTENDED_CAPABILITY:
-			SUBELEM_CHECK(ext_caps, 2, "Extended Capability");
-			break;
-		case WFD_SUBELEM_R2_DEVICE_INFORMATION:
-			SUBELEM_CHECK(r2, 2, "R2 Device Information");
-			break;
-#undef SUBELEM_CHECK
-		default:
-			/* Skip unknown IEs */
-			break;
-		}
-	}
-
-	if (devinfo) {
-		uint16_t capability = l_get_be16(devinfo + 0);
-		bool source;
-		bool sink;
-		uint16_t port;
-
-		source = (capability & WFD_DEV_INFO_DEVICE_TYPE) ==
-			WFD_DEV_INFO_TYPE_SOURCE ||
-			(capability & WFD_DEV_INFO_DEVICE_TYPE) ==
-			WFD_DEV_INFO_TYPE_DUAL_ROLE;
-		sink = (capability & WFD_DEV_INFO_DEVICE_TYPE) ==
-			WFD_DEV_INFO_TYPE_PRIMARY_SINK ||
-			(capability & WFD_DEV_INFO_DEVICE_TYPE) ==
-			WFD_DEV_INFO_TYPE_DUAL_ROLE;
-
-		if (!source && !sink)
-			return false;
-
-		port = l_get_be16(devinfo + 2);
-
-		if (source && port == 0) {
-			l_debug("0 port number in WFD IE Device Information");
-			return false;
-		}
-
-		memset(out, 0, sizeof(*out));
-		out->available =
-			(capability & WFD_DEV_INFO_SESSION_AVAILABILITY) ==
-			WFD_DEV_INFO_SESSION_AVAILABLE;
-		out->source = source;
-		out->sink = sink;
-		out->port = port;
-		out->cp = capability & WFD_DEV_INFO_CONTENT_PROTECTION_SUPPORT;
-		out->audio = !sink ||
-			!(capability & WFD_DEV_INFO_NO_AUDIO_AT_PRIMARY_SINK);
-	} else {
-		l_error("Device Information missing in WFD IE");
-		return false;
-	}
-
-	if (ext_caps && (l_get_be16(ext_caps) & 1))
-		out->uibc = 1;
-
-	if (r2) {
-		uint8_t role = l_get_be16(r2) & 3;
-
-		if ((out->source && role != 0 && role != 3) ||
-				(out->sink && role != 1 && role != 3))
-			l_debug("Invalid role in WFD R2 Device Information");
-		else
-			out->r2 = true;
-	}
-
-	return true;
-}
-
-static bool p2p_device_validate_conn_wfd(struct p2p_device *dev,
-						const uint8_t *ie,
-						ssize_t ie_size)
-{
-	struct p2p_wfd_properties wfd;
-
-	if (!dev->conn_own_wfd)
-		return true;
-
-	/*
-	 * WFD IEs are optional in Association Request/Response and P2P Public
-	 * Action frames for R2 devices and required for R1 devices.
-	 * Wi-Fi Display Technical Specification v2.1.0 section 5.2:
-	 * "A WFD R2 Device shall include the WFD IE in Beacon, Probe
-	 * Request/Response, Association Request/Response and P2P Public Action
-	 * frames in order to be interoperable with R1 devices. If a WFD R2
-	 * Device discovers that the peer device is also a WFD R2 Device, then
-	 * it may include the WFD IE in Association Request/Response and P2P
-	 * Public Action frames."
-	 */
-	if (!ie)
-		return dev->conn_own_wfd->r2;
-
-	if (!p2p_extract_wfd_properties(ie, ie_size, &wfd)) {
-		l_error("Could not parse the WFD IE contents");
-		return false;
-	}
-
-	if ((dev->conn_own_wfd->source && !wfd.sink) ||
-			(dev->conn_own_wfd->sink && !wfd.source)) {
-		l_error("Wrong role in peer's WFD IE");
-		return false;
-	}
-
-	if (wfd.r2 != dev->conn_own_wfd->r2) {
-		l_error("Wrong version in peer's WFD IE");
-		return false;
-	}
-
-	/*
-	 * Ignore the session available state because it's not 100% clear
-	 * at what point the peer switches to SESSION_NOT_AVAILABLE in its
-	 * Device Information.
-	 * But we might want to check that other bits have not changed from
-	 * what the peer reported during discovery.
-	 * Wi-Fi Display Technical Specification v2.1.0 section 4.5.2.1:
-	 * "The content of the WFD Device Information subelement should be
-	 * immutable during the period of P2P connection establishment, with
-	 * [...] exceptions [...]"
-	 */
-
-	return true;
-}
-
 static bool p2p_go_negotiation_confirm_cb(const struct mmpdu_header *mpdu,
 					const void *body, size_t body_len,
 					int rssi, struct p2p_device *dev)
-- 
2.27.0

[PATCH 4/4] p2p: Build P2P and WFD IEs for group's management frames

[Andrew Zaborowski]

[-- Attachment #1: Type: text/plain, Size: 11689 bytes --]

Register P2P group's vendor IE writers using the new API to build and
attach the necessary P2P IE and WFD IEs to the (Re)Association Response,
Probe Response and Beacon frames sent by the GO.
---
 src/p2p.c | 222 +++++++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 211 insertions(+), 11 deletions(-)

diff --git a/src/p2p.c b/src/p2p.c
index e5b8c93d..a7d0fe2a 100644
--- a/src/p2p.c
+++ b/src/p2p.c
@@ -162,6 +162,10 @@ struct p2p_wfd_properties {
 	bool uibc;
 	bool cp;
 	bool r2;
+	uint16_t raw_dev_info;
+	uint8_t associated_bssid[6];
+	uint8_t raw_coupled_sink_status;
+	uint8_t coupled_sink_mac[6];
 };
 
 static struct l_queue *p2p_device_list;
@@ -277,9 +281,9 @@ static void p2p_peer_put(void *user_data)
 static void p2p_device_discovery_start(struct p2p_device *dev);
 static void p2p_device_discovery_stop(struct p2p_device *dev);
 
-/* Callers should reserve 32 bytes */
+/* Callers should reserve 32 bytes, 64 with non-NULL @wfd_clients */
 static size_t p2p_build_wfd_ie(const struct p2p_wfd_properties *wfd,
-				uint8_t *buf)
+				const struct p2p_peer *wfd_client, uint8_t *buf)
 {
 	/*
 	 * Wi-Fi Display Technical Specification v2.1.0
@@ -324,6 +328,30 @@ static size_t p2p_build_wfd_ie(const struct p2p_wfd_properties *wfd,
 		buf[size++] = 0x01;	/* UIBC Support */
 	}
 
+	/*
+	 * Wi-Fi Display Technical Specification v2.1.0 section 5.2.3:
+	 * "If a WFD Capable GO has at least one associated client that is
+	 * WFD capable, the WFD capable GO shall include the WFD Session
+	 * Information subelement in the WFD IE in the Probe Response
+	 * frames it transmits."
+	 */
+	if (wfd_client && !L_WARN_ON(!wfd_client->wfd)) {
+		buf[size++] = WFD_SUBELEM_SESION_INFORMATION;
+		buf[size++] = 0;		/* WFD Subelement length */
+		buf[size++] = 23;
+		memcpy(buf + size, wfd_client->device_addr, 6);
+		size += 6;
+		memcpy(buf + size, wfd_client->wfd->associated_bssid, 6);
+		size += 6;
+		buf[size++] = wfd_client->wfd->raw_dev_info >> 8;
+		buf[size++] = wfd_client->wfd->raw_dev_info & 255;
+		buf[size++] = wfd_client->wfd->throughput >> 8;
+		buf[size++] = wfd_client->wfd->throughput & 255;
+		buf[size++] = wfd_client->wfd->raw_coupled_sink_status;
+		memcpy(buf + size, wfd_client->wfd->coupled_sink_mac, 6);
+		size += 6;
+	}
+
 	if (wfd->r2) {
 		buf[size++] = WFD_SUBELEM_R2_DEVICE_INFORMATION;
 		buf[size++] = 0;	/* WFD Subelement length */
@@ -341,6 +369,8 @@ static bool p2p_extract_wfd_properties(const uint8_t *ie, size_t ie_size,
 {
 	struct wfd_subelem_iter iter;
 	const uint8_t *devinfo = NULL;
+	const uint8_t *associated_bssid = NULL;
+	const uint8_t *coupled_sink_info = NULL;
 	const uint8_t *ext_caps = NULL;
 	const uint8_t *r2 = NULL;
 
@@ -370,6 +400,13 @@ static bool p2p_extract_wfd_properties(const uint8_t *ie, size_t ie_size,
 		case WFD_SUBELEM_WFD_DEVICE_INFORMATION:
 			SUBELEM_CHECK(devinfo, 6, "Device Information");
 			break;
+		case WFD_SUBELEM_ASSOCIATED_BSSID:
+			SUBELEM_CHECK(associated_bssid, 6, "Associated BSSID");
+			break;
+		case WFD_SUBELEM_COUPLED_SINK_INFORMATION:
+			SUBELEM_CHECK(coupled_sink_info, 7,
+					"Coupled Sink Information");
+			break;
 		case WFD_SUBELEM_EXTENDED_CAPABILITY:
 			SUBELEM_CHECK(ext_caps, 2, "Extended Capability");
 			break;
@@ -418,11 +455,20 @@ static bool p2p_extract_wfd_properties(const uint8_t *ie, size_t ie_size,
 		out->cp = capability & WFD_DEV_INFO_CONTENT_PROTECTION_SUPPORT;
 		out->audio = !sink ||
 			!(capability & WFD_DEV_INFO_NO_AUDIO_AT_PRIMARY_SINK);
+		out->raw_dev_info = l_get_be16(devinfo);
 	} else {
 		l_error("Device Information missing in WFD IE");
 		return false;
 	}
 
+	if (associated_bssid)
+		memcpy(out->associated_bssid, associated_bssid, 6);
+
+	if (coupled_sink_info) {
+		out->raw_coupled_sink_status = coupled_sink_info[0];
+		memcpy(out->coupled_sink_mac, coupled_sink_info + 1, 6);
+	}
+
 	if (ext_caps && (l_get_be16(ext_caps) & 1))
 		out->uibc = 1;
 
@@ -559,7 +605,7 @@ static uint8_t *p2p_build_scan_ies(struct p2p_device *dev, uint8_t *buf,
 		return NULL;
 
 	if (p2p_own_wfd)
-		wfd_ie_size = p2p_build_wfd_ie(p2p_own_wfd, wfd_ie);
+		wfd_ie_size = p2p_build_wfd_ie(p2p_own_wfd, NULL, wfd_ie);
 	else
 		wfd_ie_size = 0;
 
@@ -925,7 +971,9 @@ static void p2p_group_event(enum ap_event_type type, const void *event_data,
 		/* Don't validate the P2P IE or WFD IE at this stage */
 		break;
 	case AP_EVENT_REGISTRATION_SUCCESS:
+		/* Update the Group Formation bit in our beacons */
 		dev->capability.group_caps &= ~P2P_GROUP_CAP_GROUP_FORMATION;
+		ap_update_beacon(dev->group);
 		break;
 	case AP_EVENT_PBC_MODE_EXIT:
 		break;
@@ -939,6 +987,150 @@ invalid_ie:
 	p2p_connect_failed(dev);
 }
 
+static size_t p2p_group_get_p2p_ie_len(enum mpdu_management_subtype type,
+					void *user_data)
+{
+	switch (type) {
+	case MPDU_MANAGEMENT_SUBTYPE_ASSOCIATION_RESPONSE:
+	case MPDU_MANAGEMENT_SUBTYPE_REASSOCIATION_RESPONSE:
+	case MPDU_MANAGEMENT_SUBTYPE_PROBE_RESPONSE:
+	case MPDU_MANAGEMENT_SUBTYPE_BEACON:
+		return 256;
+	default:
+		return 0;
+	}
+}
+
+static size_t p2p_group_write_p2p_ie(enum mpdu_management_subtype type,
+					uint8_t *out_buf, void *user_data)
+{
+	struct p2p_device *dev = user_data;
+	L_AUTO_FREE_VAR(uint8_t *, p2p_ie) = NULL;
+	size_t p2p_ie_len;
+
+	switch (type) {
+	case MPDU_MANAGEMENT_SUBTYPE_ASSOCIATION_RESPONSE:
+	case MPDU_MANAGEMENT_SUBTYPE_REASSOCIATION_RESPONSE:
+	{
+		/*
+		 * Wi-Fi P2P Technical Specification v1.7 Section 4.2.5:
+		 * "If neither P2P attribute is required according to the
+		 * conditions in Table 55, then a P2P IE containing no P2P
+		 * attributes is included."
+		 * This is going to be our case.
+		 */
+		struct p2p_association_resp info = {};
+
+		p2p_ie = p2p_build_association_resp(&info, &p2p_ie_len);
+		break;
+	}
+
+	case MPDU_MANAGEMENT_SUBTYPE_PROBE_RESPONSE:
+	{
+		/*
+		 * For simplicity we include the P2P IE and the WFD IE
+		 * (if enabled) in our Probe Responses whether we were asked
+		 * for them in the Probe Request or not.
+		 */
+		struct p2p_probe_resp info = {};
+
+		info.capability = dev->capability;
+		info.device_info = dev->device_info;
+
+		if (dev->conn_peer_added) {
+			struct p2p_client_info_descriptor client = {};
+
+			memcpy(client.device_addr,
+				dev->conn_peer_dev_info.device_addr, 6);
+			memcpy(client.interface_addr,
+				dev->conn_peer_interface_addr, 6);
+			client.device_caps =
+				dev->conn_peer_capability.device_caps;
+			client.wsc_config_methods =
+				dev->conn_peer_dev_info.wsc_config_methods;
+			client.primary_device_type =
+				dev->conn_peer_dev_info.primary_device_type;
+			l_strlcpy(client.device_name,
+					dev->conn_peer_dev_info.device_name,
+					sizeof(client.device_name));
+
+			info.group_clients = l_queue_new();
+			l_queue_push_tail(info.group_clients,
+					l_memdup(&client, sizeof(client)));
+		}
+
+		p2p_ie = p2p_build_probe_resp(&info, &p2p_ie_len);
+		p2p_clear_probe_resp(&info);
+		break;
+	}
+
+	case MPDU_MANAGEMENT_SUBTYPE_BEACON:
+	{
+		struct p2p_beacon info = {};
+
+		info.capability = dev->capability;
+		memcpy(info.device_addr, dev->addr, 6);
+		p2p_ie = p2p_build_beacon(&info, &p2p_ie_len);
+		break;
+	}
+
+	default:
+		return 0;
+	}
+
+	memcpy(out_buf, p2p_ie, p2p_ie_len);
+	return p2p_ie_len;
+}
+
+static size_t p2p_group_get_wfd_ie_len(enum mpdu_management_subtype type,
+					void *user_data)
+{
+	struct p2p_device *dev = user_data;
+
+	switch (type) {
+	case MPDU_MANAGEMENT_SUBTYPE_ASSOCIATION_RESPONSE:
+	case MPDU_MANAGEMENT_SUBTYPE_REASSOCIATION_RESPONSE:
+		return (dev->conn_own_wfd && !dev->conn_own_wfd->r2) ? 32 : 0;
+	case MPDU_MANAGEMENT_SUBTYPE_PROBE_RESPONSE:
+	case MPDU_MANAGEMENT_SUBTYPE_BEACON:
+		return p2p_own_wfd ? 64 : 0;
+	default:
+		return 0;
+	}
+}
+
+static size_t p2p_group_write_wfd_ie(enum mpdu_management_subtype type,
+					uint8_t *out_buf, void *user_data)
+{
+	struct p2p_device *dev = user_data;
+
+	switch (type) {
+	case MPDU_MANAGEMENT_SUBTYPE_ASSOCIATION_RESPONSE:
+	case MPDU_MANAGEMENT_SUBTYPE_REASSOCIATION_RESPONSE:
+		if (dev->conn_own_wfd && !dev->conn_own_wfd->r2)
+			return p2p_build_wfd_ie(dev->conn_own_wfd, NULL,
+						out_buf);
+
+		break;
+	case MPDU_MANAGEMENT_SUBTYPE_PROBE_RESPONSE:
+		if (p2p_own_wfd)
+			return p2p_build_wfd_ie(p2p_own_wfd,
+						dev->conn_own_wfd ?
+						dev->conn_peer : NULL, out_buf);
+
+		break;
+	case MPDU_MANAGEMENT_SUBTYPE_BEACON:
+		if (p2p_own_wfd)
+			return p2p_build_wfd_ie(p2p_own_wfd, NULL, out_buf);
+
+		break;
+	default:
+		break;
+	}
+
+	return 0;
+}
+
 static const struct ap_ops p2p_go_ops = {
 	.handle_event = p2p_group_event,
 };
@@ -988,6 +1180,11 @@ static void p2p_group_start(struct p2p_device *dev)
 		p2p_connect_failed(dev);
 		return;
 	}
+
+	ap_add_vendor_ie_writer(dev->group, p2p_group_get_p2p_ie_len,
+				p2p_group_write_p2p_ie, dev);
+	ap_add_vendor_ie_writer(dev->group, p2p_group_get_wfd_ie_len,
+				p2p_group_write_wfd_ie, dev);
 }
 
 static void p2p_netconfig_event_handler(enum netconfig_event event,
@@ -1227,7 +1424,7 @@ static void p2p_try_connect_group(struct p2p_device *dev)
 	if (dev->conn_own_wfd) {
 		ie_iov[ie_num].iov_base = wfd_ie;
 		ie_iov[ie_num].iov_len = p2p_build_wfd_ie(dev->conn_own_wfd,
-								wfd_ie);
+								NULL, wfd_ie);
 		ie_num++;
 	}
 
@@ -1376,7 +1573,7 @@ static void p2p_provision_connect(struct p2p_device *dev)
 	if (dev->conn_own_wfd) {
 		iov[iov_num].iov_base = wfd_ie;
 		iov[iov_num].iov_len = p2p_build_wfd_ie(dev->conn_own_wfd,
-							wfd_ie);
+							NULL, wfd_ie);
 		iov_num++;
 	}
 
@@ -2211,7 +2408,7 @@ respond:
 	if (dev->conn_own_wfd) {
 		resp_info.wfd = wfd_ie;
 		resp_info.wfd_size = p2p_build_wfd_ie(dev->conn_own_wfd,
-							wfd_ie);
+							NULL, wfd_ie);
 	}
 
 	resp_body = p2p_build_go_negotiation_resp(&resp_info, &resp_len);
@@ -2488,7 +2685,7 @@ static bool p2p_go_negotiation_resp_cb(const struct mmpdu_header *mpdu,
 	if (dev->conn_own_wfd) {
 		confirm_info.wfd = wfd_ie;
 		confirm_info.wfd_size = p2p_build_wfd_ie(dev->conn_own_wfd,
-								wfd_ie);
+								NULL, wfd_ie);
 	}
 
 	confirm_body = p2p_build_go_negotiation_confirmation(&confirm_info,
@@ -2576,7 +2773,8 @@ static void p2p_start_go_negotiation(struct p2p_device *dev)
 
 	if (dev->conn_own_wfd) {
 		info.wfd = wfd_ie;
-		info.wfd_size = p2p_build_wfd_ie(dev->conn_own_wfd, wfd_ie);
+		info.wfd_size = p2p_build_wfd_ie(dev->conn_own_wfd,
+							NULL, wfd_ie);
 	}
 
 	req_body = p2p_build_go_negotiation_req(&info, &req_len);
@@ -2714,7 +2912,8 @@ static void p2p_start_provision_discovery(struct p2p_device *dev)
 
 	if (dev->conn_own_wfd) {
 		info.wfd = wfd_ie;
-		info.wfd_size = p2p_build_wfd_ie(dev->conn_own_wfd, wfd_ie);
+		info.wfd_size = p2p_build_wfd_ie(dev->conn_own_wfd,
+							NULL, wfd_ie);
 	}
 
 	req_body = p2p_build_provision_disc_req(&info, &req_len);
@@ -3696,11 +3895,12 @@ static void p2p_device_send_probe_resp(struct p2p_device *dev,
 	if (to_conn_peer && dev->conn_own_wfd) {
 		iov[iov_len].iov_base = wfd_ie;
 		iov[iov_len].iov_len = p2p_build_wfd_ie(dev->conn_own_wfd,
-							wfd_ie);
+							NULL, wfd_ie);
 		iov_len++;
 	} else if (p2p_own_wfd) {
 		iov[iov_len].iov_base = wfd_ie;
-		iov[iov_len].iov_len = p2p_build_wfd_ie(p2p_own_wfd, wfd_ie);
+		iov[iov_len].iov_len = p2p_build_wfd_ie(p2p_own_wfd,
+							NULL, wfd_ie);
 		iov_len++;
 	}
 
-- 
2.27.0

Re: [PATCH 1/4] ap: Pass vendor IEs between frames and the user

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 4284 bytes --]

Hi Andrew,

On 2/5/21 5:46 PM, Andrew Zaborowski wrote:
> Add API for the ap.h users to add extra vendor IEs to outgoing
> management frames (beacons, etc.).  Pass the string IEs from the
> incoming STA association frames to the user in the AP event data.

This patch seems to be doing 2 separate things, no?  If so, it should really be 
two commits.  Especially since patch 3 only cares about the latter part.

> 
> I drop ap_event_station_added_data.rsn_ie because that probably
> wasn't going to ever be useful and the RSN IE is included in the
> assoc_ies array in any case.
> ---
>   src/ap.c | 233 ++++++++++++++++++++++++++++++++++++++++---------------
>   src/ap.h |  15 +++-
>   2 files changed, 186 insertions(+), 62 deletions(-)
> 

<snip>

> @@ -463,11 +474,51 @@ static void ap_set_rsn_info(struct ap_state *ap, struct ie_rsn_info *rsn)
>   	rsn->group_cipher = ap->group_cipher;
>   }
>   
> +static size_t ap_get_vendor_ies_len(struct ap_state *ap,
> +					enum mpdu_management_subtype type)
> +{
> +	const struct l_queue_entry *entry;
> +	size_t len = 0;
> +
> +	for (entry = l_queue_get_entries(ap->sta_states); entry;
> +			entry = entry->next) {
> +		struct ap_vendor_ie_writer *writer = entry->data;
> +
> +		len += writer->get_len_func(type, writer->user_data);
> +	}
> +
> +	return len;
> +}
> +
> +static size_t ap_write_vendor_ies(struct ap_state *ap,
> +					enum mpdu_management_subtype type,
> +					uint8_t *out_buf)
> +{
> +	const struct l_queue_entry *entry;
> +	size_t len = 0;
> +
> +	for (entry = l_queue_get_entries(ap->sta_states); entry;
> +			entry = entry->next) {
> +		struct ap_vendor_ie_writer *writer = entry->data;
> +
> +		len += writer->write_func(type, out_buf + len,
> +						writer->user_data);
> +	}
> +
> +	return len;
> +}
> +

I sort of wonder why you go through all these lengths when p2p is the only 
external user of this and registers the same user data object.

<snip>

> @@ -53,6 +57,10 @@ struct ap_event_registration_success_data {
>   };
>   
>   typedef void (*ap_stopped_func_t)(void *user_data);
> +typedef size_t (*ap_get_vendor_ie_len)(enum mpdu_management_subtype type,
> +					void *user_data);
> +typedef size_t (*ap_write_vendor_ie)(enum mpdu_management_subtype type,
> +					uint8_t *out_buf, void *user_data);
>   
>   struct ap_config {
>   	char *ssid;
> @@ -83,6 +91,11 @@ void ap_shutdown(struct ap_state *ap, ap_stopped_func_t stopped_func,
>   			void *user_data);
>   void ap_free(struct ap_state *ap);
>   
> +void ap_add_vendor_ie_writer(struct ap_state *ap,
> +				ap_get_vendor_ie_len get_len_func,
> +				ap_write_vendor_ie write_func,
> +				void *user_data);
> +

This part would likely look better as an .ops structure...

Taking a step back here though, it feels like this API is really incomplete:

- There's no way for an external observer to grab an ap_state object in order 
for them to add a vendor IE (for example, some vendor plugin might want to 
advertise extra IEs or something).

- There's no way for an observer to react to any events since they're handled by 
the singleton 'handle_event' operation passed in to ap_start.  This part is 
particularly asymmetric with how ap_add_vendor_ie_writer() is meant to be used.

So what is the end goal for 'struct ap_state' APIs?  Should they be externally 
observable like station?  In that case you may want to add watchlists and such 
for this.

If they're meant only to be used by the P2P subclass, then you might as well 
move all these details into 'ap_ops' instead.

Also, you may consider flipping things around and instead of using a 'pull' 
mechanism, using a 'push' mechanism instead.  Add a couple of new events to 
signal that vendor IEs need to be updated (beaconing, association response, etc) 
and have the clients invoke a function to write the new IEs.  This may be more 
flexible and would allow you hide some implementation details (i.e. add a 
growable buffer instead of a variable-length stack buffer, something which we 
probably should not use in this instance)

>   bool ap_station_disconnect(struct ap_state *ap, const uint8_t *mac,
>   				enum mmpdu_reason_code reason);
>   
> 

Regards,
-Denis

Re: [PATCH 1/4] ap: Pass vendor IEs between frames and the user

[Andrew Zaborowski]

[-- Attachment #1: Type: text/plain, Size: 6404 bytes --]

Hi Denis,

On Thu, 18 Feb 2021 at 18:17, Denis Kenzior <denkenz@gmail.com> wrote:
> Hi Andrew,
>
> On 2/5/21 5:46 PM, Andrew Zaborowski wrote:
> > Add API for the ap.h users to add extra vendor IEs to outgoing
> > management frames (beacons, etc.).  Pass the string IEs from the
> > incoming STA association frames to the user in the AP event data.
>
> This patch seems to be doing 2 separate things, no?  If so, it should really be
> two commits.  Especially since patch 3 only cares about the latter part.

They seemed related enough but yes.

>
> >
> > I drop ap_event_station_added_data.rsn_ie because that probably
> > wasn't going to ever be useful and the RSN IE is included in the
> > assoc_ies array in any case.
> > ---
> >   src/ap.c | 233 ++++++++++++++++++++++++++++++++++++++++---------------
> >   src/ap.h |  15 +++-
> >   2 files changed, 186 insertions(+), 62 deletions(-)
> >
>
> <snip>
>
> > @@ -463,11 +474,51 @@ static void ap_set_rsn_info(struct ap_state *ap, struct ie_rsn_info *rsn)
> >       rsn->group_cipher = ap->group_cipher;
> >   }
> >
> > +static size_t ap_get_vendor_ies_len(struct ap_state *ap,
> > +                                     enum mpdu_management_subtype type)
> > +{
> > +     const struct l_queue_entry *entry;
> > +     size_t len = 0;
> > +
> > +     for (entry = l_queue_get_entries(ap->sta_states); entry;
> > +                     entry = entry->next) {
> > +             struct ap_vendor_ie_writer *writer = entry->data;
> > +
> > +             len += writer->get_len_func(type, writer->user_data);
> > +     }
> > +
> > +     return len;
> > +}
> > +
> > +static size_t ap_write_vendor_ies(struct ap_state *ap,
> > +                                     enum mpdu_management_subtype type,
> > +                                     uint8_t *out_buf)
> > +{
> > +     const struct l_queue_entry *entry;
> > +     size_t len = 0;
> > +
> > +     for (entry = l_queue_get_entries(ap->sta_states); entry;
> > +                     entry = entry->next) {
> > +             struct ap_vendor_ie_writer *writer = entry->data;
> > +
> > +             len += writer->write_func(type, out_buf + len,
> > +                                             writer->user_data);
> > +     }
> > +
> > +     return len;
> > +}
> > +
>
> I sort of wonder why you go through all these lengths when p2p is the only
> external user of this and registers the same user data object.
>
> <snip>
>
> > @@ -53,6 +57,10 @@ struct ap_event_registration_success_data {
> >   };
> >
> >   typedef void (*ap_stopped_func_t)(void *user_data);
> > +typedef size_t (*ap_get_vendor_ie_len)(enum mpdu_management_subtype type,
> > +                                     void *user_data);
> > +typedef size_t (*ap_write_vendor_ie)(enum mpdu_management_subtype type,
> > +                                     uint8_t *out_buf, void *user_data);
> >
> >   struct ap_config {
> >       char *ssid;
> > @@ -83,6 +91,11 @@ void ap_shutdown(struct ap_state *ap, ap_stopped_func_t stopped_func,
> >                       void *user_data);
> >   void ap_free(struct ap_state *ap);
> >
> > +void ap_add_vendor_ie_writer(struct ap_state *ap,
> > +                             ap_get_vendor_ie_len get_len_func,
> > +                             ap_write_vendor_ie write_func,
> > +                             void *user_data);
> > +
>
> This part would likely look better as an .ops structure...

Ok, I can do it that way, in fact the previous version I sent did exactly that.

>
> Taking a step back here though, it feels like this API is really incomplete:
>
> - There's no way for an external observer to grab an ap_state object in order
> for them to add a vendor IE (for example, some vendor plugin might want to
> advertise extra IEs or something).
>
> - There's no way for an observer to react to any events since they're handled by
> the singleton 'handle_event' operation passed in to ap_start.  This part is
> particularly asymmetric with how ap_add_vendor_ie_writer() is meant to be used.

True, those two approaches can be seen as conflicting so we probably
should go one way or the other, i.e.

 * have a single ops callback that writes all the extra IEs as one, or

 * allow registering multiple IE writers but also add ways for ap.h
users (other than the caller of ap_start()) to iterate over ap_states,
or perhaps add their IEs globally without needing an ap_state.

In any case I don't see any need for the latter option right now.  You
also seem to prefer the simpler approach too.  The reason I went for
ap_add_vendor_ie_writer() is that it can be extended in the future if
we needed the IE order-by-tag logic mentioned in 802.11-2016 9.4.2.1
and would only require one-line changes in the callers.

>
> So what is the end goal for 'struct ap_state' APIs?  Should they be externally
> observable like station?  In that case you may want to add watchlists and such
> for this.

I don't see much use for that.

>
> If they're meant only to be used by the P2P subclass, then you might as well
> move all these details into 'ap_ops' instead.
>
> Also, you may consider flipping things around and instead of using a 'pull'
> mechanism, using a 'push' mechanism instead.  Add a couple of new events to
> signal that vendor IEs need to be updated (beaconing, association response, etc)
> and have the clients invoke a function to write the new IEs.  This may be more
> flexible and would allow you hide some implementation details (i.e. add a
> growable buffer instead of a variable-length stack buffer, something which we
> probably should not use in this instance)

That will force us to memcpy those buffers when sending one of those
frames, and we would probably need reference counting too.  We may
also end up generating the IE contents even if the related frame never
gets sent, so we may be doing extra work.

Also we may need to build the probe response IEs based specifically on
the contents of the probe request (that happens to not be a problem
with Association Req / Association Response in P2P right now) but my
current patch doesn't handle that either, I think I'll add a separate,
but similar, ops-> callback for writing the probe response IEs which
takes the probe request IEs string as an extra parameter.

Best regards

Re: [PATCH 1/4] ap: Pass vendor IEs between frames and the user

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 2507 bytes --]

Hi Andrew,


> In any case I don't see any need for the latter option right now.  You
> also seem to prefer the simpler approach too.  The reason I went for
> ap_add_vendor_ie_writer() is that it can be extended in the future if
> we needed the IE order-by-tag logic mentioned in 802.11-2016 9.4.2.1
> and would only require one-line changes in the callers.

If this API is only used for Vendor IEs, then I don't believe there are any 
ordering requirements..

>> Also, you may consider flipping things around and instead of using a 'pull'
>> mechanism, using a 'push' mechanism instead.  Add a couple of new events to
>> signal that vendor IEs need to be updated (beaconing, association response, etc)
>> and have the clients invoke a function to write the new IEs.  This may be more
>> flexible and would allow you hide some implementation details (i.e. add a
>> growable buffer instead of a variable-length stack buffer, something which we
>> probably should not use in this instance)
> 
> That will force us to memcpy those buffers when sending one of those
> frames, and we would probably need reference counting too.  We may
> also end up generating the IE contents even if the related frame never
> gets sent, so we may be doing extra work.

I don't really see how your last point can happen, but putting that aside...  It 
is possible that you'd incur a few extra memcpys.  It depends on how you set 
things up.  If you make the callers pass in a refence-countable buffer for 
example, then memcpy could be kept to a minimum.

I'm somewhat against using variable length stack buffers for the underlying 
implementation.  It may be too easy to screw up and overflow since the logic is 
in two places.

> 
> Also we may need to build the probe response IEs based specifically on
> the contents of the probe request (that happens to not be a problem
> with Association Req / Association Response in P2P right now) but my
> current patch doesn't handle that either, I think I'll add a separate,
> but similar, ops-> callback for writing the probe response IEs which
> takes the probe request IEs string as an extra parameter.

Yes, this is another good alternative.  In theory, the setup you describe is 
somewhat similar to how we handled 'auth_proto' inside netdev.c.  So I can 
definitely see how you may want to filter authenticate / associate requests 
through the P2P subclass before moving on to generating the auth/assoc response.

Regards,
-Denis