[PATCH 1/8] ap: Fix NULL ap->rares

[PATCH 1/8] ap: Fix NULL ap->rares

[Andrew Zaborowski]

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

Make sure ap->rates is non-NULL both with and without no_cck_rates.
---
 src/ap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/ap.c b/src/ap.c
index c972cfcb..ae33b461 100644
--- a/src/ap.c
+++ b/src/ap.c
@@ -2074,6 +2074,7 @@ struct ap_state *ap_start(struct netdev *netdev, struct ap_config *config,
 	ap->ciphers = wiphy_select_cipher(wiphy, 0xffff);
 	ap->group_cipher = wiphy_select_cipher(wiphy, 0xffff);
 	ap->beacon_interval = 100;
+	ap->rates = l_uintset_new(200);
 
 	/* TODO: Pick from actual supported rates */
 	if (config->no_cck_rates) {
@@ -2086,7 +2087,6 @@ struct ap_state *ap_start(struct netdev *netdev, struct ap_config *config,
 		l_uintset_put(ap->rates, 96); /* 48 Mbps*/
 		l_uintset_put(ap->rates, 108); /* 54 Mbps*/
 	} else {
-		ap->rates = l_uintset_new(200);
 		l_uintset_put(ap->rates, 2); /* 1 Mbps*/
 		l_uintset_put(ap->rates, 11); /* 5.5 Mbps*/
 		l_uintset_put(ap->rates, 22); /* 11 Mbps*/
-- 
2.25.1

[PATCH 2/8] ap: Fix setting the basic rate in Supported Rates IE

[Andrew Zaborowski]

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

---
 src/ap.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/ap.c b/src/ap.c
index ae33b461..2a979661 100644
--- a/src/ap.c
+++ b/src/ap.c
@@ -380,10 +380,10 @@ static size_t ap_build_beacon_pr_head(struct ap_state *ap,
 				flag = 0x80;
 
 			*rates++ = r | flag;
+			count++;
 		}
 
-	ie_tlv_builder_set_length(&builder, rates -
-					ie_tlv_builder_get_data(&builder));
+	ie_tlv_builder_set_length(&builder, count);
 
 	/* DSSS Parameter Set IE for DSSS, HR, ERP and HT PHY rates */
 	ie_tlv_builder_next(&builder, IE_TYPE_DSSS_PARAMETER_SET);
-- 
2.25.1

[PATCH 3/8] p2putil: Add p2p_get_random_string

[Andrew Zaborowski]

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

Add a utility to select random characters from the set defined in P2P
v1.7 Section 3.2.1.  In this version the assumption is that we're only
actually using this for the two SSID characters.
---
 src/p2putil.c | 33 +++++++++++++++++++++++++++++++++
 src/p2putil.h |  2 ++
 2 files changed, 35 insertions(+)

diff --git a/src/p2putil.c b/src/p2putil.c
index d0f3a444..6ea18b74 100644
--- a/src/p2putil.c
+++ b/src/p2putil.c
@@ -2611,3 +2611,36 @@ uint8_t *p2p_build_go_disc_req(size_t *out_len)
 	return p2p_build_action_frame(false, P2P_ACTION_GO_DISCOVERABILITY_REQ,
 					0, NULL, NULL, NULL, 0, out_len);
 }
+
+/*
+ * Select a string of random characters from the set defined in section
+ * 3.2.1:
+ *
+ * "Following "DIRECT-" the SSID shall contain two ASCII characters "xy",
+ * randomly selected with a uniform distribution from the following
+ * character set: upper case letters, lower case letters and numbers."
+ *
+ * "The WPA2-Personal pass-phrase shall contain at least eight ASCII
+ * characters randomly selected with a uniform distribution from the
+ * following character set: upper case letters, lower case letters and
+ * numbers."
+ *
+ * Our random distribution is not uniform but close enough for use in
+ * the SSID.
+ */
+void p2p_get_random_string(char *buf, size_t len)
+{
+#define CHARSET "ABCDEFGHIJKLMNOPQRSTUVWXYZ" "abcdefghijklmnopqrstuvwxyz" \
+		"0123456789"
+	static const int set_size = strlen(CHARSET);
+
+	l_getrandom(buf, len);
+
+	while (len) {
+		int index = *buf % set_size;
+
+		*buf++ = CHARSET[index];
+		len--;
+	}
+#undef CHARSET
+}
diff --git a/src/p2putil.h b/src/p2putil.h
index 6aad5574..23bf1dae 100644
--- a/src/p2putil.h
+++ b/src/p2putil.h
@@ -603,3 +603,5 @@ uint8_t *p2p_build_presence_req(const struct p2p_presence_req *data,
 uint8_t *p2p_build_presence_resp(const struct p2p_presence_resp *data,
 					size_t *out_len);
 uint8_t *p2p_build_go_disc_req(size_t *out_len);
+
+void p2p_get_random_string(char *buf, size_t len);
-- 
2.25.1

[PATCH 4/8] p2p: Add GO-side of GO Negotiation (initiator)

[Andrew Zaborowski]

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

Allow the possibility of becoming the Group-owner when we build the GO
Negotiation Request, parse GO Negotiation Response and build the GO
Negotiation Confirmation, i.e. if we're the initiator of the
negotiation.

Until now the code assumed we can't become the GO or we'd report error.
---
 src/p2p.c | 181 +++++++++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 144 insertions(+), 37 deletions(-)

diff --git a/src/p2p.c b/src/p2p.c
index d2ae169d..832e1a20 100644
--- a/src/p2p.c
+++ b/src/p2p.c
@@ -82,6 +82,7 @@ struct p2p_device {
 	unsigned int listen_duration;
 	struct l_queue *discovery_users;
 	struct l_queue *peer_list;
+	unsigned int next_tie_breaker;
 
 	struct p2p_peer *conn_peer;
 	uint16_t conn_config_method;
@@ -106,9 +107,10 @@ struct p2p_device {
 	uint8_t conn_go_dialog_token;
 	unsigned int conn_go_scan_retry;
 	uint32_t conn_go_oper_freq;
-	struct p2p_group_id_attr go_group_id;
 	uint8_t conn_peer_interface_addr[6];
 
+	struct p2p_group_id_attr go_group_id;
+
 	bool enabled : 1;
 	bool have_roc_cookie : 1;
 	/*
@@ -119,6 +121,8 @@ struct p2p_device {
 	 * netdev_disconnect, or similar, finishes.
 	 */
 	bool disconnecting : 1;
+	bool is_go : 1;
+	bool conn_go_tie_breaker : 1;
 };
 
 struct p2p_discovery_user {
@@ -163,6 +167,12 @@ static unsigned int p2p_wfd_disconnect_watch;
 static const int channels_social[] = { 1, 6, 11 };
 static const int channels_scan_2_4_other[] = { 2, 3, 4, 5, 7, 8, 9, 10 };
 
+/*
+ * The client side generally receives more testing and we know of fewer
+ * problematic drivers so set a low default Group Owner intent value.
+ */
+#define P2P_GO_INTENT 2
+
 enum {
 	FRAME_GROUP_DEFAULT = 0,
 	FRAME_GROUP_LISTEN,
@@ -501,6 +511,7 @@ static void p2p_connection_reset(struct p2p_device *dev)
 
 	explicit_bzero(dev->conn_psk, 32);
 	dev->conn_retry_count = 0;
+	dev->is_go = false;
 
 	if (dev->enabled && !dev->start_stop_cmd_id &&
 			!l_queue_isempty(dev->discovery_users))
@@ -1694,6 +1705,18 @@ static bool p2p_go_negotiation_confirm_cb(const struct mmpdu_header *mpdu,
 	return true;
 }
 
+static void p2p_set_group_id(struct p2p_device *dev)
+{
+	const char *name = dev->device_info.device_name;
+	char buf[2];
+
+	/* SSID format following section 3.2.1 */
+	p2p_get_random_string(buf, 2);
+	snprintf(dev->go_group_id.ssid, sizeof(dev->go_group_id.ssid),
+			"DIRECT-%c%c-%.22s", buf[0], buf[1], name);
+	memcpy(dev->go_group_id.device_addr, dev->addr, 6);
+}
+
 static void p2p_device_go_negotiation_req_cb(const struct mmpdu_header *mpdu,
 						const void *body,
 						size_t body_len, int rssi,
@@ -1885,10 +1908,18 @@ static void p2p_go_negotiation_confirm_done(int error, void *user_data)
 	}
 
 	/*
-	 * Frame was ACKed.  For simplicity wait idly the maximum amount of
-	 * time indicated by the peer in the GO Negotiation Response's
-	 * Configuration Timeout attribute and start the provisioning phase.
+	 * Frame was ACKed.  On the GO start setting the group up right
+	 * away and we'll add the client's Configuation Timeout to the
+	 * WSC start timeout's value.  On the client wait idly the
+	 * maximum amount of time indicated by the peer in the GO
+	 * Negotiation Response's Configuration Timeout attribute and
+	 * start the provisioning phase.
 	 */
+	if (dev->is_go) {
+		p2p_device_interface_create(dev);
+		return;
+	}
+
 	dev->conn_peer_config_timeout = l_timeout_create_ms(
 						dev->conn_config_delay,
 						p2p_config_timeout, dev,
@@ -1974,17 +2005,17 @@ static bool p2p_go_negotiation_resp_cb(const struct mmpdu_header *mpdu,
 	 * shall be toggled from the corresponding GO Negotiation Request
 	 * frame."
 	 */
-	if (!resp_info.go_tie_breaker) {
+	if (resp_info.go_tie_breaker == dev->conn_go_tie_breaker) {
 		l_error("GO Negotiation Response tie breaker value wrong");
-
-		if (resp_info.go_intent == 0) {
-			/* Can't continue */
-			p2p_connect_failed(dev);
-			goto p2p_free;
-		}
+		/* Proceed anyway */
+		dev->conn_go_tie_breaker = !resp_info.go_tie_breaker;
 	}
 
-	if (resp_info.capability.group_caps & P2P_GROUP_CAP_PERSISTENT_GROUP) {
+	dev->is_go = P2P_GO_INTENT * 2 + dev->conn_go_tie_breaker >
+		resp_info.go_intent * 2;
+
+	if ((resp_info.capability.group_caps & P2P_GROUP_CAP_PERSISTENT_GROUP)
+			&& !dev->is_go) {
 		l_error("Persistent groups not supported");
 		p2p_connect_failed(dev);
 		goto p2p_free;
@@ -2002,6 +2033,44 @@ static bool p2p_go_negotiation_resp_cb(const struct mmpdu_header *mpdu,
 		goto p2p_free;
 	}
 
+	if (dev->is_go) {
+		const struct l_queue_entry *entry;
+		const struct p2p_channel_entries *entries;
+		int i;
+
+		/*
+		 * Check that our listen channel is in the set supported by the
+		 * Client.
+		 * TODO: if it's not, look for a different channel.
+		 */
+		for (entry = l_queue_get_entries(
+					resp_info.channel_list.channel_entries);
+				entry; entry = entry->next) {
+			entries = entry->data;
+
+			if (entries->oper_class == dev->listen_oper_class)
+				break;
+		}
+
+		if (!entry) {
+			l_error("Our Operating Class not listed in "
+				"the GO Negotiation Response");
+			p2p_connect_failed(dev);
+			goto p2p_free;
+		}
+
+		for (i = 0; i < entries->n_channels; i++)
+			if (entries->channels[i] == dev->listen_channel)
+				break;
+
+		if (i == entries->n_channels) {
+			l_error("Our listen channel not listed in "
+				"the GO Negotiation Response");
+			p2p_connect_failed(dev);
+			goto p2p_free;
+		}
+	}
+
 	/* Check whether WFD IE is required, validate it if present */
 	if (!p2p_device_validate_conn_wfd(dev, resp_info.wfd,
 						resp_info.wfd_size)) {
@@ -2009,33 +2078,63 @@ static bool p2p_go_negotiation_resp_cb(const struct mmpdu_header *mpdu,
 		goto p2p_free;
 	}
 
-	band = scan_oper_class_to_band(
+	memcpy(dev->conn_peer_interface_addr, resp_info.intended_interface_addr,
+		6);
+
+	if (dev->is_go) {
+		struct l_queue *channel_list = l_queue_new();
+		struct p2p_channel_entries *channel_entries =
+			l_malloc(sizeof(struct p2p_channel_entries) + 1);
+
+		p2p_set_group_id(dev);
+
+		dev->conn_config_delay =
+			resp_info.config_timeout.client_config_timeout * 10;
+
+		channel_entries->oper_class = dev->listen_oper_class;
+		channel_entries->n_channels = 1;
+		channel_entries->channels[0] = dev->listen_channel;
+		l_queue_push_tail(channel_list, channel_entries);
+
+		/* Build and send the GO Negotiation Confirmation */
+		confirm_info.capability = dev->capability;
+		memcpy(confirm_info.operating_channel.country,
+			dev->listen_country, 3);
+		confirm_info.operating_channel.oper_class = dev->listen_oper_class;
+		confirm_info.operating_channel.channel_num = dev->listen_channel;
+		memcpy(confirm_info.channel_list.country, dev->listen_country, 3);
+		confirm_info.channel_list.channel_entries = channel_list;
+		memcpy(&confirm_info.group_id, &dev->go_group_id,
+			sizeof(struct p2p_group_id_attr));
+	} else {
+		band = scan_oper_class_to_band(
 			(const uint8_t *) resp_info.operating_channel.country,
 			resp_info.operating_channel.oper_class);
-	frequency = scan_channel_to_freq(
+		frequency = scan_channel_to_freq(
 					resp_info.operating_channel.channel_num,
 					band);
-	if (!frequency) {
-		l_error("Bad operating channel in GO Negotiation Response");
-		p2p_connect_failed(dev);
-		return true;
-	}
+		if (!frequency) {
+			l_error("Bad operating channel in GO Negotiation Response");
+			p2p_connect_failed(dev);
+			goto p2p_free;
+		}
 
-	dev->conn_config_delay =
+		dev->conn_config_delay =
 			resp_info.config_timeout.go_config_timeout * 10;
-	dev->conn_go_oper_freq = frequency;
-	memcpy(&dev->go_group_id, &resp_info.group_id,
-		sizeof(struct p2p_group_id_attr));
-	memcpy(dev->conn_peer_interface_addr,
-		resp_info.intended_interface_addr, 6);
+		dev->conn_go_oper_freq = frequency;
+		memcpy(&dev->go_group_id, &resp_info.group_id,
+			sizeof(struct p2p_group_id_attr));
+
+		/* Build and send the GO Negotiation Confirmation */
+		confirm_info.capability.device_caps = 0;	/* Reserved */
+		confirm_info.capability.group_caps = 0;		/* Reserved */
+		confirm_info.operating_channel = resp_info.operating_channel;
+		confirm_info.channel_list = resp_info.channel_list;
+		resp_info.channel_list.channel_entries = NULL;
+	}
 
-	/* Build and send the GO Negotiation Confirmation */
 	confirm_info.dialog_token = resp_info.dialog_token;
 	confirm_info.status = P2P_STATUS_SUCCESS;
-	confirm_info.capability.device_caps = 0;	/* Reserved */
-	confirm_info.capability.group_caps = 0;		/* Reserved */
-	confirm_info.channel_list = resp_info.channel_list;
-	confirm_info.operating_channel = resp_info.operating_channel;
 
 	if (dev->conn_own_wfd) {
 		confirm_info.wfd = wfd_ie;
@@ -2046,6 +2145,7 @@ static bool p2p_go_negotiation_resp_cb(const struct mmpdu_header *mpdu,
 	confirm_body = p2p_build_go_negotiation_confirmation(&confirm_info,
 								&confirm_len);
 	confirm_info.wfd = NULL;
+	p2p_clear_go_negotiation_confirmation(&confirm_info);
 
 	if (!confirm_body) {
 		p2p_connect_failed(dev);
@@ -2099,10 +2199,12 @@ static void p2p_start_go_negotiation(struct p2p_device *dev)
 	 */
 	unsigned int resp_timeout = 600;
 
+	dev->conn_go_tie_breaker = dev->next_tie_breaker++ & 1;
+
 	info.dialog_token = 1;
 	info.capability = dev->capability;
-	info.go_intent = 0;	/* Don't want to be the GO */
-	info.go_tie_breaker = 0;
+	info.go_intent = P2P_GO_INTENT;
+	info.go_tie_breaker = dev->conn_go_tie_breaker;
 	info.config_timeout.go_config_timeout = 50;	/* 500ms */
 	info.config_timeout.client_config_timeout = 50;	/* 500ms */
 	memcpy(info.listen_channel.country, dev->listen_country, 3);
@@ -2111,11 +2213,9 @@ static void p2p_start_go_negotiation(struct p2p_device *dev)
 	memcpy(info.intended_interface_addr, dev->conn_addr, 6);
 
 	/*
-	 * In theory we support an empty set of operating channels for
-	 * our potential group as a GO but we have to include our
-	 * supported channels because the peer can only choose their
-	 * own channels from our list.  Use the listen channel as the
-	 * preferred operating channel because we have no preference.
+	 * Even if P2P_GO_INTENT is 0, we have to include our supported
+	 * channels because the peer can only choose their own channels
+	 * from our list.
 	 */
 	p2p_device_fill_channel_list(dev, &info.channel_list);
 	memcpy(info.operating_channel.country, dev->listen_country, 3);
@@ -3615,6 +3715,13 @@ struct p2p_device *p2p_device_update_from_genl(struct l_genl_msg *msg,
 	gethostname(hostname, sizeof(hostname));
 	dev->connections_left = 1;
 
+	/*
+	 * Section 3.1.4.2: "The Tie breaker bit in a first GO Negotiation
+	 * Request frame (for instance after power up) shall be set to 0 or 1
+	 * randomly, such that both values occur equally on average."
+	 */
+	dev->next_tie_breaker = l_getrandom_uint32();
+
 	/* TODO: allow masking capability bits through a setting? */
 	dev->capability.device_caps = P2P_DEVICE_CAP_CONCURRENT_OP;
 	dev->capability.group_caps = 0;
-- 
2.25.1

[PATCH 5/8] p2p: Add GO-side of GO Negotiation (responder)

[Andrew Zaborowski]

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

Allow the possibility of becoming the Group-owner when we parse the GO
Negotiation Request, build GO Negotiation Response and parse the GO
Negotiation Confirmation, i.e. if we're responding to a negotiation
initiated by the peer after it needed to request user action.

Until now the code assumed we can't become the GO or we'd report error.
---
 src/p2p.c | 217 ++++++++++++++++++++++++++++++++++++++++++------------
 1 file changed, 169 insertions(+), 48 deletions(-)

diff --git a/src/p2p.c b/src/p2p.c
index 832e1a20..ac9a8f97 100644
--- a/src/p2p.c
+++ b/src/p2p.c
@@ -1636,8 +1636,6 @@ static bool p2p_go_negotiation_confirm_cb(const struct mmpdu_header *mpdu,
 {
 	struct p2p_go_negotiation_confirmation info;
 	int r;
-	enum scan_band band;
-	uint32_t frequency;
 
 	l_debug("");
 
@@ -1668,40 +1666,79 @@ static bool p2p_go_negotiation_confirm_cb(const struct mmpdu_header *mpdu,
 		return true;
 	}
 
-	if (!p2p_device_validate_channel_list(dev, &info.channel_list,
-						&info.operating_channel))
-		return true;
-
-	band = scan_oper_class_to_band(
-			(const uint8_t *) info.operating_channel.country,
-			info.operating_channel.oper_class);
-	frequency = scan_channel_to_freq(info.operating_channel.channel_num,
-						band);
-	if (!frequency) {
-		l_error("Bad operating channel in GO Negotiation Confirmation");
+	/* Check whether WFD IE is required, validate it if present */
+	if (!p2p_device_validate_conn_wfd(dev, info.wfd, info.wfd_size)) {
 		p2p_connect_failed(dev);
 		return true;
 	}
 
-	/* Check whether WFD IE is required, validate it if present */
-	if (!p2p_device_validate_conn_wfd(dev, info.wfd, info.wfd_size)) {
+	/*
+	 * In both scenarios the Channel List is a subset of what we previously
+	 * sent in the GO Negotiation Response and must contain the Operating
+	 * Channel.
+	 */
+	if (!p2p_device_validate_channel_list(dev, &info.channel_list,
+						&info.operating_channel)) {
 		p2p_connect_failed(dev);
 		return true;
 	}
 
-	dev->conn_go_oper_freq = frequency;
-	memcpy(&dev->go_group_id, &info.group_id,
-		sizeof(struct p2p_group_id_attr));
-
 	/*
-	 * Confirmation received.  For simplicity wait idly the maximum amount
-	 * of time indicated by the peer in the GO Negotiation Response's
-	 * Configuration Timeout attribute and start the provisioning phase.
+	 * Not validating .capability.group_caps, it's either reserved
+	 * (dev->is_go) or has to be identical to that in the GO Negotiation
+	 * Request (!dev->is_go).
 	 */
-	dev->conn_peer_config_timeout = l_timeout_create_ms(
+
+	if (dev->is_go) {
+		if (memcmp(info.operating_channel.country,
+				dev->listen_country, 3) ||
+				info.operating_channel.oper_class !=
+				dev->listen_oper_class ||
+				info.operating_channel.channel_num !=
+				dev->listen_channel) {
+			l_error("Bad operating channel in GO Negotiation "
+				"Confirmation");
+			p2p_connect_failed(dev);
+			return true;
+		}
+
+		/*
+		 * Start setting the group up right away and we'll add the
+		 * client's Configuation Timeout to the WSC start timeout's
+		 * value.
+		 */
+		p2p_device_interface_create(dev);
+	} else {
+		enum scan_band band = scan_oper_class_to_band(
+			(const uint8_t *) info.operating_channel.country,
+			info.operating_channel.oper_class);
+		uint32_t frequency = scan_channel_to_freq(
+					info.operating_channel.channel_num,
+					band);
+
+		if (!frequency) {
+			l_error("Bad operating channel in GO Negotiation "
+				"Confirmation");
+			p2p_connect_failed(dev);
+			return true;
+		}
+
+		dev->conn_go_oper_freq = frequency;
+		memcpy(&dev->go_group_id, &info.group_id,
+			sizeof(struct p2p_group_id_attr));
+
+		/*
+		 * Confirmation received.  For simplicity wait idly the maximum
+		 * amount of time indicated by the peer in the GO Negotiation
+		 * Request's Configuration Timeout attribute and start the
+		 * provisioning phase.
+		 */
+		dev->conn_peer_config_timeout = l_timeout_create_ms(
 						dev->conn_config_delay,
 						p2p_config_timeout, dev,
 						p2p_config_timeout_destroy);
+	}
+
 	return true;
 }
 
@@ -1733,7 +1770,6 @@ static void p2p_device_go_negotiation_req_cb(const struct mmpdu_header *mpdu,
 	int iov_len = 0;
 	struct p2p_peer *peer;
 	enum p2p_attr_status_code status = P2P_STATUS_SUCCESS;
-	bool tie_breaker = false;
 
 	l_debug("");
 
@@ -1782,24 +1818,12 @@ static void p2p_device_go_negotiation_req_cb(const struct mmpdu_header *mpdu,
 		goto respond;
 	}
 
-	if (req_info.go_intent == 0 && !req_info.go_tie_breaker) {
-		l_error("Can't negotiate client role and GO operation not "
-			"supported");
-
-		if (peer->wsc.pending_connect) {
-			struct l_dbus_message *reply =
-				dbus_error_not_supported(
-						peer->wsc.pending_connect);
-
-			dbus_pending_reply(&peer->wsc.pending_connect, reply);
-		}
-
-		p2p_connect_failed(dev);
-		status = P2P_STATUS_FAIL_INCOMPATIBLE_PARAMS;
-		goto p2p_free;
-	}
+	dev->conn_go_tie_breaker = !req_info.go_tie_breaker;
+	dev->is_go = P2P_GO_INTENT * 2 + dev->conn_go_tie_breaker >
+		req_info.go_intent * 2;
 
-	if (req_info.capability.group_caps & P2P_GROUP_CAP_PERSISTENT_GROUP) {
+	if ((req_info.capability.group_caps & P2P_GROUP_CAP_PERSISTENT_GROUP) &&
+			!dev->is_go) {
 		if (peer->wsc.pending_connect) {
 			struct l_dbus_message *reply =
 				dbus_error_not_supported(
@@ -1821,6 +1845,70 @@ static void p2p_device_go_negotiation_req_cb(const struct mmpdu_header *mpdu,
 		goto p2p_free;
 	}
 
+	if (!p2p_device_validate_channel_list(dev, &req_info.channel_list,
+						&req_info.operating_channel)) {
+		p2p_connect_failed(dev);
+		status = P2P_STATUS_FAIL_INCOMPATIBLE_PARAMS;
+		goto p2p_free;
+	}
+
+	if (dev->is_go) {
+		const struct l_queue_entry *entry;
+		const struct p2p_channel_entries *entries;
+		int i;
+
+		/*
+		 * Section 3.1.4.2.1: "The Channel List attribute shall
+		 * indicate the channels that the P2P Device can support as
+		 * Operating Channel of the P2P Group if it becomes P2P Group
+		 * Owner."
+		 * Section 3.1.4.2.2: "The channels indicated in the Channel
+		 * List shall only include channels from the Channel List
+		 * attribute in the GO Negotiation Request frame. [...]
+		 * The channel indicated in the Operating Channel attribute
+		 * shall be one of the channels in the Channel List attribute
+		 * in the GO Negotiation Response frame."
+		 *
+		 * Since the sender is not becoming the GO this shouldn't
+		 * affect us but following 3.1.4.2.2 our operating channel
+		 * should be one of those listed in the GO Negotiation
+		 * Response which in turn are the subset of those in the
+		 * Request.  So effectively the list in the Request limits
+		 * the peer's set of supported operating channels both as the
+		 * GO and the Client.  Check that our listen channel is in
+		 * that set.
+		 * TODO: if it's not, look for a different channel.
+		 */
+		for (entry = l_queue_get_entries(
+					req_info.channel_list.channel_entries);
+				entry; entry = entry->next) {
+			entries = entry->data;
+
+			if (entries->oper_class == dev->listen_oper_class)
+				break;
+		}
+
+		if (!entry) {
+			l_error("Our Operating Class not listed in "
+				"the GO Negotiation Request");
+			p2p_connect_failed(dev);
+			status = P2P_STATUS_FAIL_NO_COMMON_CHANNELS;
+			goto p2p_free;
+		}
+
+		for (i = 0; i < entries->n_channels; i++)
+			if (entries->channels[i] == dev->listen_channel)
+				break;
+
+		if (i == entries->n_channels) {
+			l_error("Our listen channel not listed in "
+				"the GO Negotiation Request");
+			p2p_connect_failed(dev);
+			status = P2P_STATUS_FAIL_NO_COMMON_CHANNELS;
+			goto p2p_free;
+		}
+	}
+
 	/* Check whether WFD IE is required, validate it if present */
 	if (!p2p_device_validate_conn_wfd(dev, req_info.wfd,
 						req_info.wfd_size)) {
@@ -1833,29 +1921,62 @@ static void p2p_device_go_negotiation_req_cb(const struct mmpdu_header *mpdu,
 	p2p_device_discovery_stop(dev);
 
 	dev->conn_go_dialog_token = req_info.dialog_token;
-	dev->conn_config_delay = req_info.config_timeout.go_config_timeout * 10;
 	memcpy(dev->conn_peer_interface_addr, req_info.intended_interface_addr,
 		6);
 
+	if (dev->is_go && dev->conn_peer) {
+		p2p_set_group_id(dev);
+
+		dev->conn_config_delay =
+			req_info.config_timeout.client_config_timeout * 10;
+	} else {
+		dev->conn_config_delay =
+			req_info.config_timeout.go_config_timeout * 10;
+	}
+
 p2p_free:
-	tie_breaker = !req_info.go_tie_breaker;
+	dev->conn_go_tie_breaker = !req_info.go_tie_breaker;
 	p2p_clear_go_negotiation_req(&req_info);
 
 respond:
 	/* Build and send the GO Negotiation Response */
 	resp_info.dialog_token = dev->conn_go_dialog_token;
 	resp_info.status = status;
-	resp_info.capability.device_caps = dev->capability.device_caps;
-	resp_info.capability.group_caps = 0;	/* Reserved */
-	resp_info.go_intent = 0;		/* Don't want to be the GO */
-	resp_info.go_tie_breaker = tie_breaker;
+
+	if (dev->is_go && dev->conn_peer) {
+		struct l_queue *channel_list = l_queue_new();
+		struct p2p_channel_entries *channel_entries =
+			l_malloc(sizeof(struct p2p_channel_entries) + 1);
+
+		resp_info.capability = dev->capability;
+		memcpy(resp_info.operating_channel.country,
+			dev->listen_country, 3);
+		resp_info.operating_channel.oper_class = dev->listen_oper_class;
+		resp_info.operating_channel.channel_num = dev->listen_channel;
+		memcpy(&resp_info.group_id, &dev->go_group_id,
+			sizeof(struct p2p_group_id_attr));
+
+		channel_entries->oper_class = dev->listen_oper_class;
+		channel_entries->n_channels = 1;
+		channel_entries->channels[0] = dev->listen_channel;
+		l_queue_push_tail(channel_list, channel_entries);
+
+		memcpy(resp_info.channel_list.country, dev->listen_country, 3);
+		resp_info.channel_list.channel_entries = channel_list;
+	} else {
+		resp_info.capability.device_caps = dev->capability.device_caps;
+		resp_info.capability.group_caps = 0;	/* Reserved */
+		p2p_device_fill_channel_list(dev, &resp_info.channel_list);
+	}
+
+	resp_info.go_intent = P2P_GO_INTENT;
+	resp_info.go_tie_breaker = dev->conn_go_tie_breaker;
 	resp_info.config_timeout.go_config_timeout = 50;	/* 500ms */
 	resp_info.config_timeout.client_config_timeout = 50;	/* 500ms */
 
 	if (dev->conn_peer)
 		memcpy(resp_info.intended_interface_addr, dev->conn_addr, 6);
 
-	p2p_device_fill_channel_list(dev, &resp_info.channel_list);
 	resp_info.device_info = dev->device_info;
 	resp_info.device_info.wsc_config_methods = dev->conn_config_method;
 	resp_info.device_password_id = dev->conn_password_id;
-- 
2.25.1

[PATCH 6/8] ap: Pass "ops" struct to ap_start()

[Andrew Zaborowski]

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

Pass the event callback function pointer in a "struct ap_ops" instead of
as individual ap_start() argument to make adding new callbacks easier.
---
 src/ap.c | 65 +++++++++++++++++++++++++++++++-------------------------
 src/ap.h |  9 +++++---
 2 files changed, 42 insertions(+), 32 deletions(-)

diff --git a/src/ap.c b/src/ap.c
index 2a979661..19c443c3 100644
--- a/src/ap.c
+++ b/src/ap.c
@@ -52,7 +52,7 @@
 struct ap_state {
 	struct netdev *netdev;
 	struct l_genl_family *nl80211;
-	ap_event_func_t event_func;
+	const struct ap_ops *ops;
 	ap_stopped_func_t stopped_func;
 	void *user_data;
 	struct ap_config *config;
@@ -210,7 +210,7 @@ static void ap_del_station(struct sta_state *sta, uint16_t reason,
 	sta->associated = false;
 
 	if (sta->rsna) {
-		if (ap->event_func) {
+		if (ap->ops->handle_event) {
 			memset(&event_data, 0, sizeof(event_data));
 			event_data.mac = sta->addr;
 			event_data.reason = reason;
@@ -228,8 +228,8 @@ static void ap_del_station(struct sta_state *sta, uint16_t reason,
 	ap_stop_handshake(sta);
 
 	if (send_event)
-		ap->event_func(AP_EVENT_STATION_REMOVED, &event_data,
-				ap->user_data);
+		ap->ops->handle_event(AP_EVENT_STATION_REMOVED, &event_data,
+					ap->user_data);
 }
 
 static bool ap_sta_match_addr(const void *a, const void *b)
@@ -269,12 +269,12 @@ static void ap_new_rsna(struct sta_state *sta)
 
 	sta->rsna = true;
 
-	if (ap->event_func) {
+	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;
-		ap->event_func(AP_EVENT_STATION_ADDED, &event_data,
-				ap->user_data);
+		ap->ops->handle_event(AP_EVENT_STATION_ADDED, &event_data,
+					ap->user_data);
 	}
 }
 
@@ -308,11 +308,11 @@ static void ap_drop_rsna(struct sta_state *sta)
 
 	ap_stop_handshake(sta);
 
-	if (ap->event_func) {
+	if (ap->ops->handle_event) {
 		struct ap_event_station_removed_data event_data = {};
 		event_data.mac = sta->addr;
-		ap->event_func(AP_EVENT_STATION_REMOVED, &event_data,
-				ap->user_data);
+		ap->ops->handle_event(AP_EVENT_STATION_REMOVED, &event_data,
+					ap->user_data);
 	}
 }
 
@@ -543,7 +543,7 @@ static void ap_wsc_exit_pbc(struct ap_state *ap)
 	ap->wsc_dpid = 0;
 	ap_update_beacon(ap);
 
-	ap->event_func(AP_EVENT_PBC_MODE_EXIT, NULL, ap->user_data);
+	ap->ops->handle_event(AP_EVENT_PBC_MODE_EXIT, NULL, ap->user_data);
 }
 
 static uint32_t ap_send_mgmt_frame(struct ap_state *ap,
@@ -750,8 +750,9 @@ static void ap_wsc_handshake_event(struct handshake_state *hs,
 					&expiry_data);
 
 		event_data.mac = sta->addr;
-		sta->ap->event_func(AP_EVENT_REGISTRATION_SUCCESS, &event_data,
-					sta->ap->user_data);
+		sta->ap->ops->handle_event(AP_EVENT_REGISTRATION_SUCCESS,
+						&event_data,
+						sta->ap->user_data);
 		break;
 	default:
 		break;
@@ -1335,8 +1336,8 @@ static void ap_assoc_reassoc(struct sta_state *sta, bool reassoc,
 		sta->wsc_v2 = wsc_req.version2;
 
 		event_data.mac = sta->addr;
-		ap->event_func(AP_EVENT_REGISTRATION_START, &event_data,
-				ap->user_data);
+		ap->ops->handle_event(AP_EVENT_REGISTRATION_START, &event_data,
+					ap->user_data);
 
 		/*
 		 * Since we're starting the PBC Registration Protocol
@@ -1919,7 +1920,8 @@ static void ap_start_cb(struct l_genl_msg *msg, void *user_data)
 	if (l_genl_msg_get_error(msg) < 0) {
 		l_error("START_AP failed: %i", l_genl_msg_get_error(msg));
 
-		ap->event_func(AP_EVENT_START_FAILED, NULL, ap->user_data);
+		ap->ops->handle_event(AP_EVENT_START_FAILED, NULL,
+					ap->user_data);
 		ap_reset(ap);
 		l_genl_family_free(ap->nl80211);
 		l_free(ap);
@@ -1927,7 +1929,7 @@ static void ap_start_cb(struct l_genl_msg *msg, void *user_data)
 	}
 
 	ap->started = true;
-	ap->event_func(AP_EVENT_STARTED, NULL, ap->user_data);
+	ap->ops->handle_event(AP_EVENT_STARTED, NULL, ap->user_data);
 }
 
 static struct l_genl_msg *ap_build_cmd_start_ap(struct ap_state *ap)
@@ -2014,11 +2016,12 @@ static void ap_mlme_notify(struct l_genl_msg *msg, void *user_data)
 			l_genl_family_cancel(ap->nl80211,
 						ap->start_stop_cmd_id);
 			ap->start_stop_cmd_id = 0;
-			ap->event_func(AP_EVENT_START_FAILED, NULL,
-					ap->user_data);
+			ap->ops->handle_event(AP_EVENT_START_FAILED, NULL,
+						ap->user_data);
 		} else if (ap->started) {
 			ap->started = false;
-			ap->event_func(AP_EVENT_STOPPING, NULL, ap->user_data);
+			ap->ops->handle_event(AP_EVENT_STOPPING, NULL,
+						ap->user_data);
 		}
 
 		ap_reset(ap);
@@ -2031,17 +2034,17 @@ static void ap_mlme_notify(struct l_genl_msg *msg, void *user_data)
 /*
  * Start a simple independent WPA2 AP on given netdev.
  *
- * @event_func is required and must react to AP_EVENT_START_FAILED
- * and AP_EVENT_STOPPING by forgetting the ap_state struct, which
- * is going to be freed automatically.
- * In the @config struct only .ssid and .psk need to be non-NUL,
+ * @ops.handle_event is required and must react to AP_EVENT_START_FAILED
+ * and AP_EVENT_STOPPING by forgetting the ap_state struct, which is
+ * going to be freed automatically.
+ * In the @config struct only .ssid and .psk need to be non-NULL,
  * other fields are optional.  If @ap_start succeeds, the returned
  * ap_state takes ownership of @config and the caller shouldn't
  * free it or any of the memory pointed to by its members (they
  * also can't be static).
  */
 struct ap_state *ap_start(struct netdev *netdev, struct ap_config *config,
-				ap_event_func_t event_func, void *user_data)
+				const struct ap_ops *ops, void *user_data)
 {
 	struct ap_state *ap;
 	struct wiphy *wiphy = netdev_get_wiphy(netdev);
@@ -2052,7 +2055,7 @@ struct ap_state *ap_start(struct netdev *netdev, struct ap_config *config,
 	ap->nl80211 = l_genl_family_new(iwd_get_genl(), NL80211_GENL_NAME);
 	ap->config = config;
 	ap->netdev = netdev;
-	ap->event_func = event_func;
+	ap->ops = ops;
 	ap->user_data = user_data;
 
 	if (!config->channel)
@@ -2183,7 +2186,7 @@ static struct l_genl_msg *ap_build_cmd_stop_ap(struct ap_state *ap)
 
 /*
  * Schedule the running @ap to be stopped and freed.  The original
- * event_func and user_data are forgotten and a new callback can be
+ * ops and user_data are forgotten and a new callback can be
  * provided if the caller needs to know when the interface becomes
  * free, for example for a new ap_start call.
  *
@@ -2200,7 +2203,7 @@ void ap_shutdown(struct ap_state *ap, ap_stopped_func_t stopped_func,
 
 	if (ap->started) {
 		ap->started = false;
-		ap->event_func(AP_EVENT_STOPPING, NULL, ap->user_data);
+		ap->ops->handle_event(AP_EVENT_STOPPING, NULL, ap->user_data);
 	}
 
 	ap_reset(ap);
@@ -2370,6 +2373,10 @@ static void ap_if_event_func(enum ap_event_type type, const void *event_data,
 	}
 }
 
+static const struct ap_ops ap_dbus_ops = {
+	.handle_event = ap_if_event_func,
+};
+
 static struct l_dbus_message *ap_dbus_start(struct l_dbus *dbus,
 		struct l_dbus_message *message, void *user_data)
 {
@@ -2390,7 +2397,7 @@ static struct l_dbus_message *ap_dbus_start(struct l_dbus *dbus,
 	config->ssid = l_strdup(ssid);
 	config->psk = l_strdup(wpa2_psk);
 
-	ap_if->ap = ap_start(ap_if->netdev, config, ap_if_event_func, ap_if);
+	ap_if->ap = ap_start(ap_if->netdev, config, &ap_dbus_ops, ap_if);
 	if (!ap_if->ap) {
 		ap_config_free(config);
 		return dbus_error_invalid_args(message);
diff --git a/src/ap.h b/src/ap.h
index cb5238d1..bbfecd6d 100644
--- a/src/ap.h
+++ b/src/ap.h
@@ -52,8 +52,6 @@ struct ap_event_registration_success_data {
 	const uint8_t *mac;
 };
 
-typedef void (*ap_event_func_t)(enum ap_event_type type, const void *event_data,
-				void *user_data);
 typedef void (*ap_stopped_func_t)(void *user_data);
 
 struct ap_config {
@@ -67,10 +65,15 @@ struct ap_config {
 	bool no_cck_rates : 1;
 };
 
+struct ap_ops {
+	void (*handle_event)(enum ap_event_type type, const void *event_data,
+				void *user_data);
+};
+
 void ap_config_free(struct ap_config *config);
 
 struct ap_state *ap_start(struct netdev *netdev, struct ap_config *config,
-				ap_event_func_t event_func, void *user_data);
+				const struct ap_ops *ops, void *user_data);
 void ap_shutdown(struct ap_state *ap, ap_stopped_func_t stopped_func,
 			void *user_data);
 void ap_free(struct ap_state *ap);
-- 
2.25.1

[PATCH 7/8] ap: Support working without passphrase

[Andrew Zaborowski]

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

Add a "psk" setting to allow the user to pass the binary PSK directly
instead of generating it from the passphrase and the SSID.  In that case
we'll only send the PSK to WSC enrollees.
---
 src/ap.c | 40 ++++++++++++++++++++++++++--------------
 src/ap.h |  3 ++-
 2 files changed, 28 insertions(+), 15 deletions(-)

diff --git a/src/ap.c b/src/ap.c
index 19c443c3..57b35b74 100644
--- a/src/ap.c
+++ b/src/ap.c
@@ -61,7 +61,6 @@ struct ap_state {
 	enum ie_rsn_cipher_suite group_cipher;
 	uint32_t beacon_interval;
 	struct l_uintset *rates;
-	uint8_t pmk[32];
 	uint32_t start_stop_cmd_id;
 	uint32_t mlme_watch;
 	uint8_t gtk[CRYPTO_MAX_GTK_LEN];
@@ -113,11 +112,12 @@ void ap_config_free(struct ap_config *config)
 
 	l_free(config->ssid);
 
-	if (config->psk) {
-		explicit_bzero(config->psk, strlen(config->psk));
-		l_free(config->psk);
+	if (config->passphrase) {
+		explicit_bzero(config->passphrase, strlen(config->passphrase));
+		l_free(config->passphrase);
 	}
 
+	explicit_bzero(config->psk, 32);
 	l_free(config->authorized_macs);
 	l_free(config->wsc_name);
 	l_free(config);
@@ -176,8 +176,6 @@ static void ap_reset(struct ap_state *ap)
 {
 	struct netdev *netdev = ap->netdev;
 
-	explicit_bzero(ap->pmk, sizeof(ap->pmk));
-
 	if (ap->mlme_watch)
 		l_genl_family_unregister(ap->nl80211, ap->mlme_watch);
 
@@ -651,7 +649,7 @@ static void ap_start_rsna(struct sta_state *sta, const uint8_t *gtk_rsc)
 	handshake_state_set_authenticator(sta->hs, true);
 	handshake_state_set_event_func(sta->hs, ap_handshake_event, sta);
 	handshake_state_set_supplicant_ie(sta->hs, sta->assoc_rsne);
-	handshake_state_set_pmk(sta->hs, sta->ap->pmk, 32);
+	handshake_state_set_pmk(sta->hs, sta->ap->config->psk, 32);
 
 	if (gtk_rsc)
 		handshake_state_set_gtk(sta->hs, sta->ap->gtk,
@@ -794,8 +792,18 @@ static void ap_start_eap_wsc(struct sta_state *sta)
 				WSC_CONFIGURATION_METHOD_PUSH_BUTTON);
 	l_settings_set_string(sta->wsc_settings, "WSC", "WPA2-SSID",
 				ap->config->ssid);
-	l_settings_set_string(sta->wsc_settings, "WSC", "WPA2-Passphrase",
-				ap->config->psk);
+
+	if (ap->config->passphrase)
+		l_settings_set_string(sta->wsc_settings,
+					"WSC", "WPA2-Passphrase",
+					ap->config->passphrase);
+	else {
+		L_AUTO_FREE_VAR(char *, psk_str) =
+			l_util_hexstring(ap->config->psk, 32);
+
+		l_settings_set_string(sta->wsc_settings,
+					"WSC", "WPA2-PSK", psk_str);
+	}
 
 	sta->hs = netdev_handshake_state_new(ap->netdev);
 	handshake_state_set_authenticator(sta->hs, true);
@@ -2037,8 +2045,9 @@ static void ap_mlme_notify(struct l_genl_msg *msg, void *user_data)
  * @ops.handle_event is required and must react to AP_EVENT_START_FAILED
  * and AP_EVENT_STOPPING by forgetting the ap_state struct, which is
  * going to be freed automatically.
- * In the @config struct only .ssid and .psk need to be non-NULL,
- * other fields are optional.  If @ap_start succeeds, the returned
+ * In the @config struct only .ssid and either of .passprhase and .psk
+ * need to be present, other fields are optional.  If .psk is used
+ * whenever .passphrase is NULL.  If @ap_start succeeds, the returned
  * ap_state takes ownership of @config and the caller shouldn't
  * free it or any of the memory pointed to by its members (they
  * also can't be static).
@@ -2097,8 +2106,11 @@ struct ap_state *ap_start(struct netdev *netdev, struct ap_config *config,
 
 	wsc_uuid_from_addr(netdev_get_address(netdev), ap->wsc_uuid_r);
 
-	if (crypto_psk_from_passphrase(config->psk, (uint8_t *) config->ssid,
-					strlen(config->ssid), ap->pmk) < 0)
+	if (!config->passphrase &&
+			crypto_psk_from_passphrase(config->passphrase,
+						(uint8_t *) config->ssid,
+						strlen(config->ssid),
+						config->psk) < 0)
 		goto error;
 
 	if (!frame_watch_add(wdev_id, 0, 0x0000 |
@@ -2395,7 +2407,7 @@ static struct l_dbus_message *ap_dbus_start(struct l_dbus *dbus,
 
 	config = l_new(struct ap_config, 1);
 	config->ssid = l_strdup(ssid);
-	config->psk = l_strdup(wpa2_psk);
+	config->passphrase = l_strdup(wpa2_psk);
 
 	ap_if->ap = ap_start(ap_if->netdev, config, &ap_dbus_ops, ap_if);
 	if (!ap_if->ap) {
diff --git a/src/ap.h b/src/ap.h
index bbfecd6d..4232a735 100644
--- a/src/ap.h
+++ b/src/ap.h
@@ -56,7 +56,8 @@ typedef void (*ap_stopped_func_t)(void *user_data);
 
 struct ap_config {
 	char *ssid;
-	char *psk;
+	char *passphrase;
+	uint8_t psk[32];
 	uint8_t channel;
 	uint8_t *authorized_macs;
 	unsigned int authorized_macs_num;
-- 
2.25.1

Re: [PATCH 7/8] ap: Support working without passphrase

[Denis Kenzior]

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

Hi Andrew,

On 9/11/20 2:51 PM, Andrew Zaborowski wrote:
> Add a "psk" setting to allow the user to pass the binary PSK directly
> instead of generating it from the passphrase and the SSID.  In that case
> we'll only send the PSK to WSC enrollees.
> ---
>   src/ap.c | 40 ++++++++++++++++++++++++++--------------
>   src/ap.h |  3 ++-
>   2 files changed, 28 insertions(+), 15 deletions(-)
> 

<snip>

> @@ -794,8 +792,18 @@ static void ap_start_eap_wsc(struct sta_state *sta)
>   				WSC_CONFIGURATION_METHOD_PUSH_BUTTON);
>   	l_settings_set_string(sta->wsc_settings, "WSC", "WPA2-SSID",
>   				ap->config->ssid);
> -	l_settings_set_string(sta->wsc_settings, "WSC", "WPA2-Passphrase",
> -				ap->config->psk);
> +
> +	if (ap->config->passphrase)
> +		l_settings_set_string(sta->wsc_settings,
> +					"WSC", "WPA2-Passphrase",
> +					ap->config->passphrase);
> +	else {
> +		L_AUTO_FREE_VAR(char *, psk_str) =
> +			l_util_hexstring(ap->config->psk, 32);
> +
> +		l_settings_set_string(sta->wsc_settings,
> +					"WSC", "WPA2-PSK", psk_str);

Just nitpicking, but, maybe we should consider adding l_settings_set_hexstring();

> +	}
>   
>   	sta->hs = netdev_handshake_state_new(ap->netdev);
>   	handshake_state_set_authenticator(sta->hs, true);
> @@ -2037,8 +2045,9 @@ static void ap_mlme_notify(struct l_genl_msg *msg, void *user_data)
>    * @ops.handle_event is required and must react to AP_EVENT_START_FAILED
>    * and AP_EVENT_STOPPING by forgetting the ap_state struct, which is
>    * going to be freed automatically.
> - * In the @config struct only .ssid and .psk need to be non-NULL,
> - * other fields are optional.  If @ap_start succeeds, the returned
> + * In the @config struct only .ssid and either of .passprhase and .psk
> + * need to be present, other fields are optional.  If .psk is used

Not sure I can parse the last sentence here..?

> + * whenever .passphrase is NULL.  If @ap_start succeeds, the returned
>    * ap_state takes ownership of @config and the caller shouldn't
>    * free it or any of the memory pointed to by its members (they
>    * also can't be static).
> @@ -2097,8 +2106,11 @@ struct ap_state *ap_start(struct netdev *netdev, struct ap_config *config,
>   
>   	wsc_uuid_from_addr(netdev_get_address(netdev), ap->wsc_uuid_r);
>   
> -	if (crypto_psk_from_passphrase(config->psk, (uint8_t *) config->ssid,
> -					strlen(config->ssid), ap->pmk) < 0)
> +	if (!config->passphrase &&

This part seems like it should be config->passphrase?

> +			crypto_psk_from_passphrase(config->passphrase,
> +						(uint8_t *) config->ssid,
> +						strlen(config->ssid),
> +						config->psk) < 0)

In other words, if passphrase && passphrase derivation fails, then error.

>   		goto error;

Also, should there be a sanity check that psk was provided somehow?

>   
>   	if (!frame_watch_add(wdev_id, 0, 0x0000 |
> @@ -2395,7 +2407,7 @@ static struct l_dbus_message *ap_dbus_start(struct l_dbus *dbus,
>   
>   	config = l_new(struct ap_config, 1);
>   	config->ssid = l_strdup(ssid);
> -	config->psk = l_strdup(wpa2_psk);
> +	config->passphrase = l_strdup(wpa2_psk);

This argument should probably be renamed.

>   
>   	ap_if->ap = ap_start(ap_if->netdev, config, &ap_dbus_ops, ap_if);
>   	if (!ap_if->ap) {
> diff --git a/src/ap.h b/src/ap.h
> index bbfecd6d..4232a735 100644
> --- a/src/ap.h
> +++ b/src/ap.h
> @@ -56,7 +56,8 @@ typedef void (*ap_stopped_func_t)(void *user_data);
>   
>   struct ap_config {
>   	char *ssid;
> -	char *psk;
> +	char *passphrase;

In theory passphrase is 63 bytes, so no real need to malloc...

> +	uint8_t psk[32];
>   	uint8_t channel;
>   	uint8_t *authorized_macs;
>   	unsigned int authorized_macs_num;
> 

Regards,
-Denis

[PATCH 8/8] p2p: Start a basic P2P Group after GO Negotiation

[Andrew Zaborowski]

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

Use the ap.c API to start an AP on a P2P_GO interface after we've been
selected as the GO in the GO Negotiation.
---
 src/p2p.c | 135 ++++++++++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 122 insertions(+), 13 deletions(-)

diff --git a/src/p2p.c b/src/p2p.c
index ac9a8f97..cf4d4bf3 100644
--- a/src/p2p.c
+++ b/src/p2p.c
@@ -55,6 +55,7 @@
 #include "src/frame-xchg.h"
 #include "src/nl80211util.h"
 #include "src/netconfig.h"
+#include "src/ap.h"
 #include "src/p2p.h"
 
 struct p2p_device {
@@ -110,6 +111,7 @@ struct p2p_device {
 	uint8_t conn_peer_interface_addr[6];
 
 	struct p2p_group_id_attr go_group_id;
+	struct ap_state *group;
 
 	bool enabled : 1;
 	bool have_roc_cookie : 1;
@@ -123,6 +125,7 @@ struct p2p_device {
 	bool disconnecting : 1;
 	bool is_go : 1;
 	bool conn_go_tie_breaker : 1;
+	bool conn_peer_added : 1;
 };
 
 struct p2p_discovery_user {
@@ -217,8 +220,9 @@ static void p2p_discovery_user_free(void *data)
 
 static inline bool p2p_peer_operational(struct p2p_peer *peer)
 {
-	return peer && peer->dev->conn_netdev && !peer->dev->conn_wsc_bss &&
-		!peer->wsc.pending_connect && !peer->dev->disconnecting;
+	return peer && peer->dev->conn_netdev && !peer->dev->disconnecting &&
+		((!peer->dev->is_go && !peer->dev->conn_wsc_bss) ||
+		 (peer->dev->is_go && peer->dev->conn_peer_added));
 }
 
 static bool p2p_peer_match(const void *a, const void *b)
@@ -457,6 +461,14 @@ static void p2p_connection_reset(struct p2p_device *dev)
 	if (dev->conn_enrollee)
 		wsc_enrollee_cancel(dev->conn_enrollee, false);
 
+	if (dev->group) {
+		ap_free(dev->group);
+		dev->group = NULL;
+		dev->conn_peer_added = false;
+	}
+
+	dev->capability.group_caps = 0;
+
 	if (dev->conn_netdev) {
 		struct l_genl_msg *msg;
 		uint64_t wdev_id = netdev_get_wdev_id(dev->conn_netdev);
@@ -628,10 +640,12 @@ static void p2p_peer_connect_done(struct p2p_device *dev)
 {
 	struct p2p_peer *peer = dev->conn_peer;
 
-	/* We can free anything potentially needed for a retry */
-	scan_bss_free(dev->conn_wsc_bss);
-	dev->conn_wsc_bss = NULL;
-	explicit_bzero(dev->conn_psk, 32);
+	if (!dev->is_go) {
+		/* We can free anything potentially needed for a retry */
+		scan_bss_free(dev->conn_wsc_bss);
+		dev->conn_wsc_bss = NULL;
+		explicit_bzero(dev->conn_psk, 32);
+	}
 
 	dbus_pending_reply(&peer->wsc.pending_connect,
 				l_dbus_message_new_method_return(
@@ -649,6 +663,96 @@ static void p2p_peer_connect_done(struct p2p_device *dev)
 				"ConnectedIP");
 }
 
+static void p2p_group_event(enum ap_event_type type, const void *event_data,
+				void *user_data)
+{
+	struct p2p_device *dev = user_data;
+
+	l_debug("type=%i", type);
+
+	switch (type) {
+	case AP_EVENT_START_FAILED:
+	case AP_EVENT_STOPPING:
+		dev->group = NULL;
+		p2p_connect_failed(dev);
+		break;
+
+	case AP_EVENT_STARTED:
+		ap_push_button(dev->group);
+		break;
+
+	case AP_EVENT_STATION_ADDED:
+		dev->conn_peer_added = true;
+		p2p_peer_connect_done(dev);
+		break;
+
+	case AP_EVENT_STATION_REMOVED:
+		dev->conn_peer_added = false;
+		p2p_connect_failed(dev);
+		break;
+
+	case AP_EVENT_REGISTRATION_START:
+		/* Don't validate the P2P IE or WFD IE at this stage */
+		break;
+	case AP_EVENT_REGISTRATION_SUCCESS:
+		dev->capability.group_caps &= ~P2P_GROUP_CAP_GROUP_FORMATION;
+		break;
+	case AP_EVENT_PBC_MODE_EXIT:
+		break;
+	};
+}
+
+static const struct ap_ops p2p_go_ops = {
+	.handle_event = p2p_group_event,
+};
+
+static void p2p_group_start(struct p2p_device *dev)
+{
+	struct ap_config *config = l_new(struct ap_config, 1);
+
+	config->ssid = l_strdup(dev->go_group_id.ssid);
+	config->channel = dev->listen_channel;
+	config->wsc_name = l_strdup(dev->device_info.device_name);
+	config->wsc_primary_device_type = dev->device_info.primary_device_type;
+	config->no_cck_rates = true;
+
+	/*
+	 * Section 3.2.1: "The Credentials for a P2P Group issued to a
+	 * P2P Device shall: [...]
+	 *  - Use a Network Key Type of 64 Hex characters."
+	 *
+	 * This implies we have to send the PSK and not the passphrase to
+	 * the WSC clients.  For simplicity we directly generate random
+	 * PSKs and don't currently respect the requirement to maintain
+	 * a passphrase.  We have no practical use for the passphrase and
+	 * it's a little costlier to generate for the same cryptographic
+	 * strength as the PSK.
+	 */
+	if (!l_getrandom(config->psk, 32)) {
+		l_error("l_getrandom() failed");
+		ap_config_free(config);
+		p2p_connect_failed(dev);
+		return;
+	}
+
+	/*
+	 * Section 3.1.4.4: "It shall only allow association by the
+	 * P2P Device that it is currently in Group Formation with."
+	 */
+	config->authorized_macs = l_memdup(dev->conn_peer_interface_addr, 6);
+	config->authorized_macs_num = 1;
+
+	dev->capability.group_caps |= P2P_GROUP_CAP_GO;
+	dev->capability.group_caps |= P2P_GROUP_CAP_GROUP_FORMATION;
+
+	dev->group = ap_start(dev->conn_netdev, config, &p2p_go_ops, dev);
+	if (!dev->group) {
+		ap_config_free(config);
+		p2p_connect_failed(dev);
+		return;
+	}
+}
+
 static void p2p_netconfig_event_handler(enum netconfig_event event,
 					void *user_data)
 {
@@ -682,7 +786,7 @@ static void p2p_dhcp_timeout_destroy(void *user_data)
 	dev->conn_dhcp_timeout = NULL;
 }
 
-static void p2p_start_dhcp(struct p2p_device *dev)
+static void p2p_start_dhcp_client(struct p2p_device *dev)
 {
 	uint32_t ifindex = netdev_get_ifindex(dev->conn_netdev);
 	unsigned int dhcp_timeout_val;
@@ -723,7 +827,7 @@ static void p2p_netdev_connect_cb(struct netdev *netdev,
 
 	switch (result) {
 	case NETDEV_RESULT_OK:
-		p2p_start_dhcp(dev);
+		p2p_start_dhcp_client(dev);
 		break;
 	case NETDEV_RESULT_AUTHENTICATION_FAILED:
 	case NETDEV_RESULT_ASSOCIATION_FAILED:
@@ -1034,10 +1138,14 @@ static void p2p_device_netdev_notify(struct netdev *netdev,
 	case NETDEV_WATCH_EVENT_NEW:
 		/* Ignore the event if we're already connecting/connected */
 		if (dev->conn_enrollee || dev->conn_retry_count ||
-				!netdev_get_is_up(netdev))
+				dev->group || !netdev_get_is_up(netdev))
 			break;
 
-		p2p_provision_connect(dev);
+		if (dev->is_go)
+			p2p_group_start(dev);
+		else
+			p2p_provision_connect(dev);
+
 		break;
 	case NETDEV_WATCH_EVENT_DEL:
 		dev->conn_netdev = NULL;
@@ -1094,13 +1202,14 @@ static void p2p_device_new_interface_destroy(void *user_data)
 
 static void p2p_device_interface_create(struct p2p_device *dev)
 {
-	uint32_t iftype = NL80211_IFTYPE_P2P_CLIENT;
+	uint32_t iftype = dev->is_go ? NL80211_IFTYPE_P2P_GO :
+		NL80211_IFTYPE_P2P_CLIENT;
 	char ifname[32];
 	uint32_t wiphy_id = dev->wdev_id >> 32;
 	struct l_genl_msg *msg;
 
-	snprintf(ifname, sizeof(ifname), "wlan%i-p2p-cl%i",
-			wiphy_id, dev->conn_num++);
+	snprintf(ifname, sizeof(ifname), "wlan%i-p2p-%s%i",
+			wiphy_id, dev->is_go ? "go" : "cl", dev->conn_num++);
 	l_debug("creating %s", ifname);
 
 	msg = l_genl_msg_new(NL80211_CMD_NEW_INTERFACE);
-- 
2.25.1

Re: [PATCH 1/8] ap: Fix NULL ap->rares

[Denis Kenzior]

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

Hi Andrew,

On 9/11/20 2:50 PM, Andrew Zaborowski wrote:
> Make sure ap->rates is non-NULL both with and without no_cck_rates.
> ---
>   src/ap.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 

Patches 1-5 applied, thanks.

Regards,
-Denis