[PATCH 1/7] iwmon: Print WSC AuthorizedMACs extended attributes

[PATCH 1/7] iwmon: Print WSC AuthorizedMACs extended attributes

[Andrew Zaborowski]

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

---
 monitor/nlmon.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/monitor/nlmon.c b/monitor/nlmon.c
index 4c0e045f..82acb533 100644
--- a/monitor/nlmon.c
+++ b/monitor/nlmon.c
@@ -2763,9 +2763,25 @@ static void print_wsc_wfa_ext_version2(unsigned int level, const char *label,
 	print_attr(level, "%s: %x.%x", label, bytes[0] >> 4, bytes[0] & 0xf);
 }
 
+static void print_wsc_wfa_ext_authorized_macs(unsigned int level,
+						const char *label,
+						const void *data, uint16_t size)
+{
+	if (size > 30 || size % 6 != 0) {
+		printf("malformed packet\n");
+		return;
+	}
+
+	for (; size; size -= 6, data += 6)
+		print_attr(level, "%s: %s", label, util_address_to_string(data));
+}
+
 static struct attr_entry wsc_wfa_ext_attr_entry[] = {
 	{ WSC_WFA_EXTENSION_VERSION2,			"Version2",
 		ATTR_CUSTOM,	{ .function = print_wsc_wfa_ext_version2 } },
+	{ WSC_WFA_EXTENSION_AUTHORIZED_MACS,		"Authorized MAC",
+		ATTR_CUSTOM,
+		{ .function = print_wsc_wfa_ext_authorized_macs } },
 	{ WSC_WFA_EXTENSION_NETWORK_KEY_SHAREABLE,
 						"Network Key Shareable",
 		ATTR_CUSTOM,	{ .function = print_wsc_bool } },
-- 
2.20.1

[PATCH 2/7] iwmon: Update to use NL80211_ATTR_SOCKET_OWNER

[Andrew Zaborowski]

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

NL80211_ATTR_IFACE_SOCKET_OWNER was renamed to NL80211_ATTR_SOCKET_OWNER.
---
 monitor/nlmon.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/monitor/nlmon.c b/monitor/nlmon.c
index 82acb533..6b0afef8 100644
--- a/monitor/nlmon.c
+++ b/monitor/nlmon.c
@@ -5522,8 +5522,8 @@ static const struct attr_entry attr_table[] = {
 			"Max AP Assoc Station" },
 	{ NL80211_ATTR_TDLS_PEER_CAPABILITY,
 			"TDLS Peer Capability", ATTR_U32 },
-	{ NL80211_ATTR_IFACE_SOCKET_OWNER,
-			"Interface Socket Owner", ATTR_FLAG },
+	{ NL80211_ATTR_SOCKET_OWNER,
+			"Socket Owns Interface/Connection", ATTR_FLAG },
 	{ NL80211_ATTR_CSA_C_OFFSETS_TX,
 			"CSA C Offsets TX" },
 	{ NL80211_ATTR_MAX_CSA_COUNTERS,
-- 
2.20.1

[PATCH 3/7] wsc: Declare the credentials structure in wsc.h

[Andrew Zaborowski]

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

---
 Makefile.am |  2 +-
 src/wsc.c   | 12 ++----------
 src/wsc.h   | 32 ++++++++++++++++++++++++++++++++
 3 files changed, 35 insertions(+), 11 deletions(-)
 create mode 100644 src/wsc.h

diff --git a/Makefile.am b/Makefile.am
index 1d572db5..8c69d9f4 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -202,7 +202,7 @@ src_iwd_SOURCES = src/main.c linux/nl80211.h src/iwd.h src/missing.h \
 					src/agent.h src/agent.c \
 					src/storage.h src/storage.c \
 					src/network.h src/network.c \
-					src/wsc.c \
+					src/wsc.h src/wsc.c \
 					src/backtrace.h src/backtrace.c \
 					src/knownnetworks.h \
 					src/knownnetworks.c \
diff --git a/src/wsc.c b/src/wsc.c
index 4e0e8e19..eb8489f3 100644
--- a/src/wsc.c
+++ b/src/wsc.c
@@ -45,6 +45,7 @@
 #include "src/storage.h"
 #include "src/iwd.h"
 #include "src/network.h"
+#include "src/wsc.h"
 
 #define WALK_TIME 120
 
@@ -61,16 +62,7 @@ struct wsc {
 	uint32_t scan_id;
 	struct scan_bss *target;
 	uint32_t station_state_watch;
-	struct {
-		char ssid[33];
-		enum security security;
-		union {
-			uint8_t psk[32];
-			char passphrase[64];
-		};
-		uint8_t addr[6];
-		bool has_passphrase;
-	} creds[3];
+	struct wsc_credentials_info creds[3];
 	uint32_t n_creds;
 	struct l_settings *eap_settings;
 
diff --git a/src/wsc.h b/src/wsc.h
new file mode 100644
index 00000000..b0e6f24a
--- /dev/null
+++ b/src/wsc.h
@@ -0,0 +1,32 @@
+/*
+ *
+ *  Wireless daemon for Linux
+ *
+ *  Copyright (C) 2019  Intel Corporation. All rights reserved.
+ *
+ *  This library is free software; you can redistribute it and/or
+ *  modify it under the terms of the GNU Lesser General Public
+ *  License as published by the Free Software Foundation; either
+ *  version 2.1 of the License, or (at your option) any later version.
+ *
+ *  This library is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ *  Lesser General Public License for more details.
+ *
+ *  You should have received a copy of the GNU Lesser General Public
+ *  License along with this library; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+
+struct wsc_credentials_info {
+	char ssid[33];
+	enum security security;
+	union {
+		uint8_t psk[32];
+		char passphrase[64];
+	};
+	uint8_t addr[6];
+	bool has_passphrase;
+};
-- 
2.20.1

Re: [PATCH 3/7] wsc: Declare the credentials structure in wsc.h

[Denis Kenzior]

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

Hi Andrew,

On 12/4/19 9:22 PM, Andrew Zaborowski wrote:
> ---
>   Makefile.am |  2 +-
>   src/wsc.c   | 12 ++----------
>   src/wsc.h   | 32 ++++++++++++++++++++++++++++++++
>   3 files changed, 35 insertions(+), 11 deletions(-)
>   create mode 100644 src/wsc.h
> 

Applied, thanks.

Regards,
-Denis

[PATCH 4/7] wsc: Refactor to separate station-specific code

[Andrew Zaborowski]

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

Split the interface-specific logic out of the core WSC logic.  The core
WSC code is the part that we can re-use between P2P and station and
doesn't include the scanning for the target BSS or the attempt to make
a station mode connection.
---
 src/wsc.c | 141 ++++++++++++++++++++++++++++++++++++++----------------
 src/wsc.h |   3 ++
 2 files changed, 103 insertions(+), 41 deletions(-)

diff --git a/src/wsc.c b/src/wsc.c
index eb8489f3..222cc222 100644
--- a/src/wsc.c
+++ b/src/wsc.c
@@ -65,6 +65,11 @@ struct wsc {
 	struct wsc_credentials_info creds[3];
 	uint32_t n_creds;
 	struct l_settings *eap_settings;
+	char *pin;
+
+	wsc_done_cb_t done_cb;
+	void *done_data;
+	uint16_t config_method;
 
 	bool wsc_association : 1;
 };
@@ -153,15 +158,12 @@ static void wsc_try_credentials(struct wsc *wsc)
 		l_dbus_message_unref(wsc->pending);
 		wsc->pending = NULL;
 
-		goto done;
+		return;
 	}
 
 	dbus_pending_reply(&wsc->pending,
 					wsc_error_not_reachable(wsc->pending));
 	station_set_autoconnect(wsc->station, true);
-done:
-	memset(wsc->creds, 0, sizeof(wsc->creds));
-	wsc->n_creds = 0;
 }
 
 static void wsc_store_credentials(struct wsc *wsc)
@@ -217,6 +219,22 @@ static void wsc_disconnect_cb(struct netdev *netdev, bool success,
 	station_set_autoconnect(wsc->station, true);
 }
 
+static void wsc_connect_cleanup(struct wsc *wsc)
+{
+	wsc->wsc_association = false;
+
+	l_settings_free(wsc->eap_settings);
+	wsc->eap_settings = NULL;
+
+	if (wsc->pin) {
+		explicit_bzero(wsc->pin, strlen(wsc->pin));
+		l_free(wsc->pin);
+		wsc->pin = NULL;
+	}
+
+	wsc->target = NULL;
+}
+
 static void wsc_connect_cb(struct netdev *netdev, enum netdev_result result,
 					void *event_data, void *user_data)
 {
@@ -224,33 +242,34 @@ static void wsc_connect_cb(struct netdev *netdev, enum netdev_result result,
 
 	l_debug("%d, result: %d", netdev_get_ifindex(wsc->netdev), result);
 
-	wsc->wsc_association = false;
-
-	l_settings_free(wsc->eap_settings);
-	wsc->eap_settings = NULL;
+	wsc_connect_cleanup(wsc);
 
 	if (result == NETDEV_RESULT_HANDSHAKE_FAILED && wsc->n_creds > 0) {
-		wsc_store_credentials(wsc);
-		wsc_try_credentials(wsc);
+		struct wsc_credentials_info creds[L_ARRAY_SIZE(wsc->creds)];
+		uint32_t n_creds = wsc->n_creds;
+
+		memcpy(creds, wsc->creds, sizeof(creds));
+		explicit_bzero(wsc->creds, sizeof(creds));
+		wsc->n_creds = 0;
+		wsc->done_cb(0, creds, n_creds, wsc->done_data);
+		explicit_bzero(creds, sizeof(creds));
 		return;
 	}
 
 	switch (result) {
 	case NETDEV_RESULT_ABORTED:
-		dbus_pending_reply(&wsc->pending,
-					dbus_error_aborted(wsc->pending));
+		wsc->done_cb(-ECANCELED, NULL, 0, wsc->done_data);
 		return;
 	case NETDEV_RESULT_HANDSHAKE_FAILED:
-		dbus_pending_reply(&wsc->pending,
-					wsc_error_no_credentials(wsc->pending));
+		wsc->done_cb(-ENOKEY, NULL, 0, wsc->done_data);
 		break;
 	default:
-		dbus_pending_reply(&wsc->pending,
-					dbus_error_failed(wsc->pending));
+		wsc->done_cb(-EIO, NULL, 0, wsc->done_data);
 		break;
 	}
 
-	station_set_autoconnect(wsc->station, true);
+	if (wsc->station)
+		station_set_autoconnect(wsc->station, true);
 }
 
 static void wsc_credential_obtained(struct wsc *wsc,
@@ -452,30 +471,24 @@ static void wsc_connect(struct wsc *wsc)
 	l_settings_set_uint(settings, "WSC", "RFBand",
 					freq_to_rf_band(bss->frequency));
 	l_settings_set_uint(settings, "WSC", "ConfigurationMethods",
-				WSC_CONFIGURATION_METHOD_VIRTUAL_DISPLAY_PIN |
-				WSC_CONFIGURATION_METHOD_VIRTUAL_PUSH_BUTTON |
-				WSC_CONFIGURATION_METHOD_KEYPAD);
+					wsc->config_method);
 	l_settings_set_string(settings, "WSC", "PrimaryDeviceType",
 					"0-00000000-0");
 	l_settings_set_string(settings, "WSC", "EnrolleeMAC",
 		util_address_to_string(netdev_get_address(wsc->netdev)));
 
-	if (!strcmp(l_dbus_message_get_member(wsc->pending), "StartPin")) {
-		const char *pin;
-
-		if (l_dbus_message_get_arguments(wsc->pending, "s", &pin)) {
-			enum wsc_device_password_id dpid;
+	if (wsc->config_method == WSC_CONFIGURATION_METHOD_KEYPAD) {
+		enum wsc_device_password_id dpid;
 
-			if (strlen(pin) == 4 || wsc_pin_is_checksum_valid(pin))
-				dpid = WSC_DEVICE_PASSWORD_ID_DEFAULT;
-			else
-				dpid = WSC_DEVICE_PASSWORD_ID_USER_SPECIFIED;
+		if (strlen(wsc->pin) == 4 ||
+				wsc_pin_is_checksum_valid(wsc->pin))
+			dpid = WSC_DEVICE_PASSWORD_ID_DEFAULT;
+		else
+			dpid = WSC_DEVICE_PASSWORD_ID_USER_SPECIFIED;
 
-			l_settings_set_uint(settings, "WSC",
-						"DevicePasswordId", dpid);
-			l_settings_set_string(settings, "WSC",
-						"DevicePassword", pin);
-		}
+		l_settings_set_uint(settings, "WSC", "DevicePasswordId", dpid);
+		l_settings_set_string(settings, "WSC", "DevicePassword",
+					wsc->pin);
 	}
 
 	handshake_state_set_event_func(hs, wsc_handshake_event, wsc);
@@ -511,8 +524,41 @@ static void wsc_connect(struct wsc *wsc)
 	return;
 error:
 	handshake_state_free(hs);
-	dbus_pending_reply(&wsc->pending,
-				dbus_error_failed(wsc->pending));
+	wsc_connect_cleanup(wsc);
+	wsc->done_cb(r, NULL, 0, wsc->done_data);
+}
+
+/* Done callback for when WSC is triggered by DBus methods */
+static void wsc_dbus_done_cb(int err, struct wsc_credentials_info *creds,
+				unsigned int n_creds, void *user_data)
+{
+	struct wsc *wsc = user_data;
+
+	l_debug("err=%i", err);
+
+	switch (err) {
+	case 0:
+		break;
+	case -ECANCELED:
+		dbus_pending_reply(&wsc->pending,
+					dbus_error_aborted(wsc->pending));
+		return;
+	case -ENOKEY:
+		dbus_pending_reply(&wsc->pending,
+					wsc_error_no_credentials(wsc->pending));
+		return;
+	case -EBUSY:
+		dbus_pending_reply(&wsc->pending,
+					dbus_error_busy(wsc->pending));
+		return;
+	default:
+		dbus_pending_reply(&wsc->pending,
+					dbus_error_failed(wsc->pending));
+		return;
+	}
+
+	wsc_store_credentials(wsc);
+	wsc_try_credentials(wsc);
 }
 
 static void station_state_watch(enum station_state state, void *userdata)
@@ -541,6 +587,22 @@ static void wsc_check_can_connect(struct wsc *wsc, struct scan_bss *target)
 	wsc->target = target;
 	station_set_autoconnect(wsc->station, false);
 
+	if (!strcmp(l_dbus_message_get_member(wsc->pending), "StartPin")) {
+		char *pin;
+
+		wsc->config_method = WSC_CONFIGURATION_METHOD_KEYPAD;
+
+		if (!l_dbus_message_get_arguments(wsc->pending, "s", &pin))
+			goto error;
+
+		wsc->pin = l_strdup(pin);
+	} else
+		wsc->config_method =
+			WSC_CONFIGURATION_METHOD_VIRTUAL_PUSH_BUTTON;
+
+	wsc->done_cb = wsc_dbus_done_cb;
+	wsc->done_data = wsc;
+
 	switch (station_get_state(wsc->station)) {
 	case STATION_STATE_DISCONNECTED:
 		wsc_connect(wsc);
@@ -564,7 +626,7 @@ static void wsc_check_can_connect(struct wsc *wsc, struct scan_bss *target)
 		break;
 	}
 error:
-	wsc->target = NULL;
+	wsc_connect_cleanup(wsc);
 	dbus_pending_reply(&wsc->pending, dbus_error_failed(wsc->pending));
 }
 
@@ -1074,12 +1136,12 @@ static void wsc_free(void *userdata)
 	struct wsc *wsc = userdata;
 
 	wsc_cancel_scan(wsc);
+	wsc_connect_cleanup(wsc);
 
 	if (wsc->station_state_watch) {
 		station_remove_state_watch(wsc->station,
 						wsc->station_state_watch);
 		wsc->station_state_watch = 0;
-		wsc->target = NULL;
 	}
 
 	if (wsc->pending)
@@ -1090,9 +1152,6 @@ static void wsc_free(void *userdata)
 		dbus_pending_reply(&wsc->pending_cancel,
 				dbus_error_aborted(wsc->pending_cancel));
 
-	if (wsc->eap_settings)
-		l_settings_free(wsc->eap_settings);
-
 	l_free(wsc);
 }
 
diff --git a/src/wsc.h b/src/wsc.h
index b0e6f24a..ada08c89 100644
--- a/src/wsc.h
+++ b/src/wsc.h
@@ -30,3 +30,6 @@ struct wsc_credentials_info {
 	uint8_t addr[6];
 	bool has_passphrase;
 };
+
+typedef void (*wsc_done_cb_t)(int err, struct wsc_credentials_info *creds,
+				unsigned int n_creds, void *user_data);
-- 
2.20.1

Re: [PATCH 4/7] wsc: Refactor to separate station-specific code

[Denis Kenzior]

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

Hi Andrew,

On 12/4/19 9:22 PM, Andrew Zaborowski wrote:
> Split the interface-specific logic out of the core WSC logic.  The core
> WSC code is the part that we can re-use between P2P and station and
> doesn't include the scanning for the target BSS or the attempt to make
> a station mode connection.
> ---
>   src/wsc.c | 141 ++++++++++++++++++++++++++++++++++++++----------------
>   src/wsc.h |   3 ++
>   2 files changed, 103 insertions(+), 41 deletions(-)
> 

<snip>

> @@ -224,33 +242,34 @@ static void wsc_connect_cb(struct netdev *netdev, enum netdev_result result,
>   
>   	l_debug("%d, result: %d", netdev_get_ifindex(wsc->netdev), result);
>   
> -	wsc->wsc_association = false;
> -
> -	l_settings_free(wsc->eap_settings);
> -	wsc->eap_settings = NULL;
> +	wsc_connect_cleanup(wsc);
>   
>   	if (result == NETDEV_RESULT_HANDSHAKE_FAILED && wsc->n_creds > 0) {
> -		wsc_store_credentials(wsc);
> -		wsc_try_credentials(wsc);
> +		struct wsc_credentials_info creds[L_ARRAY_SIZE(wsc->creds)];
> +		uint32_t n_creds = wsc->n_creds;
> +
> +		memcpy(creds, wsc->creds, sizeof(creds));
> +		explicit_bzero(wsc->creds, sizeof(creds));
> +		wsc->n_creds = 0;
> +		wsc->done_cb(0, creds, n_creds, wsc->done_data);
> +		explicit_bzero(creds, sizeof(creds));

Can you tell me why this temporary creds array is needed?  It would seem 
you can simply call done_cb with wsc->creds directly?

>   		return;
>   	}
>   
>   	switch (result) {
>   	case NETDEV_RESULT_ABORTED:
> -		dbus_pending_reply(&wsc->pending,
> -					dbus_error_aborted(wsc->pending));
> +		wsc->done_cb(-ECANCELED, NULL, 0, wsc->done_data);
>   		return;
>   	case NETDEV_RESULT_HANDSHAKE_FAILED:
> -		dbus_pending_reply(&wsc->pending,
> -					wsc_error_no_credentials(wsc->pending));
> +		wsc->done_cb(-ENOKEY, NULL, 0, wsc->done_data);
>   		break;
>   	default:
> -		dbus_pending_reply(&wsc->pending,
> -					dbus_error_failed(wsc->pending));
> +		wsc->done_cb(-EIO, NULL, 0, wsc->done_data);
>   		break;
>   	}
>   
> -	station_set_autoconnect(wsc->station, true);
> +	if (wsc->station)
> +		station_set_autoconnect(wsc->station, true);
>   }
>   
>   static void wsc_credential_obtained(struct wsc *wsc,
> @@ -452,30 +471,24 @@ static void wsc_connect(struct wsc *wsc)
>   	l_settings_set_uint(settings, "WSC", "RFBand",
>   					freq_to_rf_band(bss->frequency));
>   	l_settings_set_uint(settings, "WSC", "ConfigurationMethods",
> -				WSC_CONFIGURATION_METHOD_VIRTUAL_DISPLAY_PIN |
> -				WSC_CONFIGURATION_METHOD_VIRTUAL_PUSH_BUTTON |
> -				WSC_CONFIGURATION_METHOD_KEYPAD);
> +					wsc->config_method);

This part seems tricky.  You remove DISPLAY_PIN and use either 
PUSH_BUTTON or KEYPAD below.  This won't really work as the spec 
mandates that the DISPLAY bit is used specially and we sort of depend on 
that to be set.  See Section 7.4.3, item 2.  Without Display bit set we 
can't use 4 digit PINs.  Not that this is a good idea, but still...

Also, (a bit fuzzy now, so I might be incorrect here) we might be the 
initiator on a device with no keypad.  So we auto-generate the PIN, 
display it to the user and the user has to enter it into the AP UI.

Generally I find the specs to be rather fuzzy on the use of 
Configuration Methods attribute.  Probe Requests should contain all 
methods we support, and seemingly M1 messages should as well.

I think you probably want to use the DPID as the selector here instead.

>   	l_settings_set_string(settings, "WSC", "PrimaryDeviceType",
>   					"0-00000000-0");
>   	l_settings_set_string(settings, "WSC", "EnrolleeMAC",
>   		util_address_to_string(netdev_get_address(wsc->netdev)));
>   
> -	if (!strcmp(l_dbus_message_get_member(wsc->pending), "StartPin")) {
> -		const char *pin;
> -
> -		if (l_dbus_message_get_arguments(wsc->pending, "s", &pin)) {
> -			enum wsc_device_password_id dpid;
> +	if (wsc->config_method == WSC_CONFIGURATION_METHOD_KEYPAD) {
> +		enum wsc_device_password_id dpid;
>   
> -			if (strlen(pin) == 4 || wsc_pin_is_checksum_valid(pin))
> -				dpid = WSC_DEVICE_PASSWORD_ID_DEFAULT;
> -			else
> -				dpid = WSC_DEVICE_PASSWORD_ID_USER_SPECIFIED;
> +		if (strlen(wsc->pin) == 4 ||
> +				wsc_pin_is_checksum_valid(wsc->pin))
> +			dpid = WSC_DEVICE_PASSWORD_ID_DEFAULT;
> +		else
> +			dpid = WSC_DEVICE_PASSWORD_ID_USER_SPECIFIED;
>   
> -			l_settings_set_uint(settings, "WSC",
> -						"DevicePasswordId", dpid);
> -			l_settings_set_string(settings, "WSC",
> -						"DevicePassword", pin);
> -		}
> +		l_settings_set_uint(settings, "WSC", "DevicePasswordId", dpid);
> +		l_settings_set_string(settings, "WSC", "DevicePassword",
> +					wsc->pin);
>   	}
>   
>   	handshake_state_set_event_func(hs, wsc_handshake_event, wsc);

<snip>

> @@ -541,6 +587,22 @@ static void wsc_check_can_connect(struct wsc *wsc, struct scan_bss *target)
>   	wsc->target = target;
>   	station_set_autoconnect(wsc->station, false);
>   
> +	if (!strcmp(l_dbus_message_get_member(wsc->pending), "StartPin")) {
> +		char *pin;
> +
> +		wsc->config_method = WSC_CONFIGURATION_METHOD_KEYPAD;
> +
> +		if (!l_dbus_message_get_arguments(wsc->pending, "s", &pin))
> +			goto error;
> +
> +		wsc->pin = l_strdup(pin);
> +	} else
> +		wsc->config_method =
> +			WSC_CONFIGURATION_METHOD_VIRTUAL_PUSH_BUTTON;
> +
> +	wsc->done_cb = wsc_dbus_done_cb;
> +	wsc->done_data = wsc;
> +

So this might need to be reworked, see above...

>   	switch (station_get_state(wsc->station)) {
>   	case STATION_STATE_DISCONNECTED:
>   		wsc_connect(wsc);

Regards,
-Denis

Re: [PATCH 4/7] wsc: Refactor to separate station-specific code

[Andrew Zaborowski]

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

Hi Denis,

On Mon, 9 Dec 2019 at 19:02, Denis Kenzior <denkenz@gmail.com> wrote:
> On 12/4/19 9:22 PM, Andrew Zaborowski wrote:
> > Split the interface-specific logic out of the core WSC logic.  The core
> > WSC code is the part that we can re-use between P2P and station and
> > doesn't include the scanning for the target BSS or the attempt to make
> > a station mode connection.
> > ---
> >   src/wsc.c | 141 ++++++++++++++++++++++++++++++++++++++----------------
> >   src/wsc.h |   3 ++
> >   2 files changed, 103 insertions(+), 41 deletions(-)
> >
>
> <snip>
>
> > @@ -224,33 +242,34 @@ static void wsc_connect_cb(struct netdev *netdev, enum netdev_result result,
> >
> >       l_debug("%d, result: %d", netdev_get_ifindex(wsc->netdev), result);
> >
> > -     wsc->wsc_association = false;
> > -
> > -     l_settings_free(wsc->eap_settings);
> > -     wsc->eap_settings = NULL;
> > +     wsc_connect_cleanup(wsc);
> >
> >       if (result == NETDEV_RESULT_HANDSHAKE_FAILED && wsc->n_creds > 0) {
> > -             wsc_store_credentials(wsc);
> > -             wsc_try_credentials(wsc);
> > +             struct wsc_credentials_info creds[L_ARRAY_SIZE(wsc->creds)];
> > +             uint32_t n_creds = wsc->n_creds;
> > +
> > +             memcpy(creds, wsc->creds, sizeof(creds));
> > +             explicit_bzero(wsc->creds, sizeof(creds));
> > +             wsc->n_creds = 0;
> > +             wsc->done_cb(0, creds, n_creds, wsc->done_data);
> > +             explicit_bzero(creds, sizeof(creds));
>
> Can you tell me why this temporary creds array is needed?  It would seem
> you can simply call done_cb with wsc->creds directly?

I wanted to allow done_cb to free the wsc state, meaning that we can't
touch wsc->creds after this call and I couldn't come up with a simpler
solution to this.

>
> >               return;
> >       }
> >
> >       switch (result) {
> >       case NETDEV_RESULT_ABORTED:
> > -             dbus_pending_reply(&wsc->pending,
> > -                                     dbus_error_aborted(wsc->pending));
> > +             wsc->done_cb(-ECANCELED, NULL, 0, wsc->done_data);
> >               return;
> >       case NETDEV_RESULT_HANDSHAKE_FAILED:
> > -             dbus_pending_reply(&wsc->pending,
> > -                                     wsc_error_no_credentials(wsc->pending));
> > +             wsc->done_cb(-ENOKEY, NULL, 0, wsc->done_data);
> >               break;
> >       default:
> > -             dbus_pending_reply(&wsc->pending,
> > -                                     dbus_error_failed(wsc->pending));
> > +             wsc->done_cb(-EIO, NULL, 0, wsc->done_data);
> >               break;
> >       }
> >
> > -     station_set_autoconnect(wsc->station, true);
> > +     if (wsc->station)
> > +             station_set_autoconnect(wsc->station, true);
> >   }
> >
> >   static void wsc_credential_obtained(struct wsc *wsc,
> > @@ -452,30 +471,24 @@ static void wsc_connect(struct wsc *wsc)
> >       l_settings_set_uint(settings, "WSC", "RFBand",
> >                                       freq_to_rf_band(bss->frequency));
> >       l_settings_set_uint(settings, "WSC", "ConfigurationMethods",
> > -                             WSC_CONFIGURATION_METHOD_VIRTUAL_DISPLAY_PIN |
> > -                             WSC_CONFIGURATION_METHOD_VIRTUAL_PUSH_BUTTON |
> > -                             WSC_CONFIGURATION_METHOD_KEYPAD);
> > +                                     wsc->config_method);
>
> This part seems tricky.  You remove DISPLAY_PIN and use either
> PUSH_BUTTON or KEYPAD below.  This won't really work as the spec
> mandates that the DISPLAY bit is used specially and we sort of depend on
> that to be set.  See Section 7.4.3, item 2.  Without Display bit set we
> can't use 4 digit PINs.  Not that this is a good idea, but still...

Ok, good point...

>
> Also, (a bit fuzzy now, so I might be incorrect here) we might be the
> initiator on a device with no keypad.  So we auto-generate the PIN,
> display it to the user and the user has to enter it into the AP UI.
>
> Generally I find the specs to be rather fuzzy on the use of
> Configuration Methods attribute.  Probe Requests should contain all
> methods we support, and seemingly M1 messages should as well.
>
> I think you probably want to use the DPID as the selector here instead.

Or perhaps wsc->pin?  I understand setting config_method to KEYPAD in
wsc_check_can_connect is slightly wrong because we could be using the
DISPLAY_PIN method but so is setting DPID to User-defined since it
implies keypad.

And apparently it's safe to have all three methods set in M1 so
perhaps I should just leave this piece of code as it was.

Best regards

[PATCH 5/7] wsc: Refactor to make the DBus interface reusable

[Andrew Zaborowski]

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

Allow adding the DBus SimpleConfiguration interface on both station mode
netdevs and P2P peers by putting public functions and callbacks between
the DBus method handlers and their actual implementation.  The
implementation provides its own callbacks for each method using the
wsc_ops struct.

For readability I didn't move some functions in wsc.c around to group
them logically but we can do the moving separately.
---
 src/wsc.c | 406 ++++++++++++++++++++++++++++++++----------------------
 src/wsc.h |  21 ++-
 2 files changed, 263 insertions(+), 164 deletions(-)

diff --git a/src/wsc.c b/src/wsc.c
index 222cc222..1901feb6 100644
--- a/src/wsc.c
+++ b/src/wsc.c
@@ -52,6 +52,9 @@
 static uint32_t netdev_watch = 0;
 
 struct wsc {
+	const struct wsc_ops *ops;
+	void *user_data;
+
 	struct netdev *netdev;
 	struct station *station;
 	struct l_dbus_message *pending;
@@ -66,9 +69,6 @@ struct wsc {
 	uint32_t n_creds;
 	struct l_settings *eap_settings;
 	char *pin;
-
-	wsc_done_cb_t done_cb;
-	void *done_data;
 	uint16_t config_method;
 
 	bool wsc_association : 1;
@@ -113,20 +113,20 @@ static struct l_dbus_message *wsc_error_time_expired(struct l_dbus_message *msg)
 					"No APs in PIN mode found in "
 					"the alloted time");
 }
-static void wsc_try_credentials(struct wsc *wsc)
+static void wsc_try_credentials(struct wsc *wsc, struct wsc_credentials_info *creds,
+					unsigned int n_creds)
 {
 	unsigned int i;
 	struct network *network;
 	struct scan_bss *bss;
 
-	for (i = 0; i < wsc->n_creds; i++) {
-		network = station_network_find(wsc->station,
-						wsc->creds[i].ssid,
-						wsc->creds[i].security);
+	for (i = 0; i < n_creds; i++) {
+		network = station_network_find(wsc->station, creds[i].ssid,
+						creds[i].security);
 		if (!network)
 			continue;
 
-		bss = network_bss_find_by_addr(network, wsc->creds[i].addr);
+		bss = network_bss_find_by_addr(network, creds[i].addr);
 
 		if (!bss)
 			bss = network_bss_select(network, true);
@@ -134,7 +134,7 @@ static void wsc_try_credentials(struct wsc *wsc)
 		if (!bss)
 			continue;
 
-		if (wsc->creds[i].security == SECURITY_PSK) {
+		if (creds[i].security == SECURITY_PSK) {
 			bool ret;
 
 			/*
@@ -142,12 +142,11 @@ static void wsc_try_credentials(struct wsc *wsc)
 			 * WPA2 and WPA3 since the PSK can always be generated
 			 * if needed
 			 */
-			if (wsc->creds[i].has_passphrase)
+			if (creds[i].has_passphrase)
 				ret = network_set_passphrase(network,
-						wsc->creds[i].passphrase);
+							creds[i].passphrase);
 			else
-				ret = network_set_psk(network,
-						wsc->creds[i].psk);
+				ret = network_set_psk(network, creds[i].psk);
 
 			if (!ret)
 				continue;
@@ -166,21 +165,22 @@ static void wsc_try_credentials(struct wsc *wsc)
 	station_set_autoconnect(wsc->station, true);
 }
 
-static void wsc_store_credentials(struct wsc *wsc)
+static void wsc_store_credentials(struct wsc_credentials_info *creds,
+					unsigned int n_creds)
 {
 	unsigned int i;
 
-	for (i = 0; i < wsc->n_creds; i++) {
-		enum security security = wsc->creds[i].security;
-		const char *ssid = wsc->creds[i].ssid;
+	for (i = 0; i < n_creds; i++) {
+		enum security security = creds[i].security;
+		const char *ssid = creds[i].ssid;
 		struct l_settings *settings = l_settings_new();
 
 		l_debug("Storing credential for '%s(%s)'", ssid,
 						security_to_str(security));
 
 		if (security == SECURITY_PSK) {
-			char *hex = l_util_hexstring(wsc->creds[i].psk,
-						sizeof(wsc->creds[i].psk));
+			char *hex = l_util_hexstring(creds[i].psk,
+						sizeof(creds[i].psk));
 
 			l_settings_set_value(settings, "Security",
 							"PreSharedKey", hex);
@@ -210,8 +210,6 @@ static void wsc_disconnect_cb(struct netdev *netdev, bool success,
 
 	l_debug("%p, success: %d", wsc, success);
 
-	wsc->wsc_association = false;
-
 	reply = l_dbus_message_new_method_return(wsc->pending_cancel);
 	l_dbus_message_set_arguments(reply, "");
 	dbus_pending_reply(&wsc->pending_cancel, reply);
@@ -251,20 +249,20 @@ static void wsc_connect_cb(struct netdev *netdev, enum netdev_result result,
 		memcpy(creds, wsc->creds, sizeof(creds));
 		explicit_bzero(wsc->creds, sizeof(creds));
 		wsc->n_creds = 0;
-		wsc->done_cb(0, creds, n_creds, wsc->done_data);
+		wsc->ops->enrollee_done_cb(0, creds, n_creds, wsc->user_data);
 		explicit_bzero(creds, sizeof(creds));
 		return;
 	}
 
 	switch (result) {
 	case NETDEV_RESULT_ABORTED:
-		wsc->done_cb(-ECANCELED, NULL, 0, wsc->done_data);
+		wsc->ops->enrollee_done_cb(-ECANCELED, NULL, 0, wsc->user_data);
 		return;
 	case NETDEV_RESULT_HANDSHAKE_FAILED:
-		wsc->done_cb(-ENOKEY, NULL, 0, wsc->done_data);
+		wsc->ops->enrollee_done_cb(-ENOKEY, NULL, 0, wsc->user_data);
 		break;
 	default:
-		wsc->done_cb(-EIO, NULL, 0, wsc->done_data);
+		wsc->ops->enrollee_done_cb(-EIO, NULL, 0, wsc->user_data);
 		break;
 	}
 
@@ -525,40 +523,19 @@ static void wsc_connect(struct wsc *wsc)
 error:
 	handshake_state_free(hs);
 	wsc_connect_cleanup(wsc);
-	wsc->done_cb(r, NULL, 0, wsc->done_data);
+	wsc->ops->enrollee_done_cb(r, NULL, 0, wsc->user_data);
 }
 
-/* Done callback for when WSC is triggered by DBus methods */
-static void wsc_dbus_done_cb(int err, struct wsc_credentials_info *creds,
-				unsigned int n_creds, void *user_data)
+static void wsc_connect_set_method(struct wsc *wsc)
 {
-	struct wsc *wsc = user_data;
-
-	l_debug("err=%i", err);
+	const char *pin;
 
-	switch (err) {
-	case 0:
-		break;
-	case -ECANCELED:
-		dbus_pending_reply(&wsc->pending,
-					dbus_error_aborted(wsc->pending));
-		return;
-	case -ENOKEY:
-		dbus_pending_reply(&wsc->pending,
-					wsc_error_no_credentials(wsc->pending));
-		return;
-	case -EBUSY:
-		dbus_pending_reply(&wsc->pending,
-					dbus_error_busy(wsc->pending));
-		return;
-	default:
-		dbus_pending_reply(&wsc->pending,
-					dbus_error_failed(wsc->pending));
-		return;
-	}
+	if (!strcmp(l_dbus_message_get_member(wsc->pending), "StartPin"))
+		l_dbus_message_get_arguments(wsc->pending, "s", &pin);
+	else
+		pin = NULL;
 
-	wsc_store_credentials(wsc);
-	wsc_try_credentials(wsc);
+	wsc_start_enrollee(wsc, wsc->netdev, wsc->target, pin, NULL, 0);
 }
 
 static void station_state_watch(enum station_state state, void *userdata)
@@ -573,7 +550,7 @@ static void station_state_watch(enum station_state state, void *userdata)
 	station_remove_state_watch(wsc->station, wsc->station_state_watch);
 	wsc->station_state_watch = 0;
 
-	wsc_connect(wsc);
+	wsc_connect_set_method(wsc);
 }
 
 static void wsc_check_can_connect(struct wsc *wsc, struct scan_bss *target)
@@ -587,25 +564,9 @@ static void wsc_check_can_connect(struct wsc *wsc, struct scan_bss *target)
 	wsc->target = target;
 	station_set_autoconnect(wsc->station, false);
 
-	if (!strcmp(l_dbus_message_get_member(wsc->pending), "StartPin")) {
-		char *pin;
-
-		wsc->config_method = WSC_CONFIGURATION_METHOD_KEYPAD;
-
-		if (!l_dbus_message_get_arguments(wsc->pending, "s", &pin))
-			goto error;
-
-		wsc->pin = l_strdup(pin);
-	} else
-		wsc->config_method =
-			WSC_CONFIGURATION_METHOD_VIRTUAL_PUSH_BUTTON;
-
-	wsc->done_cb = wsc_dbus_done_cb;
-	wsc->done_data = wsc;
-
 	switch (station_get_state(wsc->station)) {
 	case STATION_STATE_DISCONNECTED:
-		wsc_connect(wsc);
+		wsc_connect_set_method(wsc);
 		return;
 	case STATION_STATE_CONNECTING:
 	case STATION_STATE_CONNECTED:
@@ -652,20 +613,13 @@ static void walk_timeout(struct l_timeout *timeout, void *user_data)
 
 	wsc_cancel_scan(wsc);
 
-	if (wsc->pending)
-		dbus_pending_reply(&wsc->pending,
-				wsc_error_walk_time_expired(wsc->pending));
-}
+	if (wsc->pending) {
+		struct l_dbus_message *reply = wsc->pin ?
+			wsc_error_time_expired(wsc->pending) :
+			wsc_error_walk_time_expired(wsc->pending);
 
-static void pin_timeout(struct l_timeout *timeout, void *user_data)
-{
-	struct wsc *wsc = user_data;
-
-	wsc_cancel_scan(wsc);
-
-	if (wsc->pending)
-		dbus_pending_reply(&wsc->pending,
-					wsc_error_time_expired(wsc->pending));
+		dbus_pending_reply(&wsc->pending, reply);
+	}
 }
 
 static bool push_button_scan_results(int err, struct l_queue *bss_list,
@@ -1001,20 +955,7 @@ static struct l_dbus_message *wsc_push_button(struct l_dbus *dbus,
 
 	l_debug("");
 
-	if (wsc->pending)
-		return dbus_error_busy(message);
-
-	wsc->station = station_find(netdev_get_ifindex(wsc->netdev));
-	if (!wsc->station)
-		return dbus_error_not_available(message);
-
-	if (!wsc_initiate_scan(wsc, WSC_DEVICE_PASSWORD_ID_PUSH_BUTTON,
-				push_button_scan_results))
-		return dbus_error_failed(message);
-
-	wsc->walk_timer = l_timeout_create(WALK_TIME, walk_timeout, wsc, NULL);
-	wsc->pending = l_dbus_message_ref(message);
-
+	wsc->ops->connect(message, NULL, wsc->user_data);
 	return NULL;
 }
 
@@ -1046,77 +987,29 @@ static struct l_dbus_message *wsc_start_pin(struct l_dbus *dbus,
 {
 	struct wsc *wsc = user_data;
 	const char *pin;
-	enum wsc_device_password_id dpid;
 
 	l_debug("");
 
-	if (wsc->pending)
-		return dbus_error_busy(message);
-
-	wsc->station = station_find(netdev_get_ifindex(wsc->netdev));
-	if (!wsc->station)
-		return dbus_error_not_available(message);
-
 	if (!l_dbus_message_get_arguments(message, "s", &pin))
 		return dbus_error_invalid_args(message);
 
 	if (!wsc_pin_is_valid(pin))
 		return dbus_error_invalid_format(message);
 
-	if (strlen(pin) == 4 || wsc_pin_is_checksum_valid(pin))
-		dpid = WSC_DEVICE_PASSWORD_ID_DEFAULT;
-	else
-		dpid = WSC_DEVICE_PASSWORD_ID_USER_SPECIFIED;
-
-	if (!wsc_initiate_scan(wsc, dpid, pin_scan_results))
-		return dbus_error_failed(message);
-
-	wsc->walk_timer = l_timeout_create(60, pin_timeout, wsc, NULL);
-	wsc->pending = l_dbus_message_ref(message);
-
+	wsc->ops->connect(message, pin, wsc->user_data);
 	return NULL;
 }
 
-static struct l_dbus_message *wsc_cancel(struct l_dbus *dbus,
+static struct l_dbus_message *wsc_dbus_cancel(struct l_dbus *dbus,
 						struct l_dbus_message *message,
 						void *user_data)
 {
 	struct wsc *wsc = user_data;
-	struct l_dbus_message *reply;
 
 	l_debug("");
 
-	if (!wsc->pending)
-		return dbus_error_not_available(message);
-
-	wsc_cancel_scan(wsc);
-
-	if (wsc->station_state_watch) {
-		station_remove_state_watch(wsc->station,
-						wsc->station_state_watch);
-		wsc->station_state_watch = 0;
-		wsc->target = NULL;
-	}
-
-	if (wsc->wsc_association) {
-		int r;
-
-		r = netdev_disconnect(wsc->netdev, wsc_disconnect_cb, wsc);
-		if (r == 0) {
-			wsc->pending_cancel = l_dbus_message_ref(message);
-			return NULL;
-		}
-
-		l_warn("Unable to initiate disconnect: %s", strerror(-r));
-		wsc->wsc_association = false;
-	}
-
-	dbus_pending_reply(&wsc->pending, dbus_error_aborted(wsc->pending));
-
-	reply = l_dbus_message_new_method_return(message);
-	l_dbus_message_set_arguments(reply, "");
-
-	return reply;
+	wsc->ops->cancel(message, wsc->user_data);
+	return NULL;
 }
 
 static void setup_wsc_interface(struct l_dbus_interface *interface)
@@ -1128,7 +1021,7 @@ static void setup_wsc_interface(struct l_dbus_interface *interface)
 	l_dbus_interface_method(interface, "StartPin", 0,
 				wsc_start_pin, "", "s", "pin");
 	l_dbus_interface_method(interface, "Cancel", 0,
-				wsc_cancel, "", "");
+				wsc_dbus_cancel, "", "");
 }
 
 static void wsc_free(void *userdata)
@@ -1155,7 +1048,194 @@ static void wsc_free(void *userdata)
 	l_free(wsc);
 }
 
-static void wsc_add_interface(struct netdev *netdev)
+struct wsc *wsc_new(const struct wsc_ops *ops, void *user_data)
+{
+	struct l_dbus *dbus = dbus_get_bus();
+	struct wsc *wsc;
+	const char *path;
+
+	wsc = l_new(struct wsc, 1);
+	wsc->ops = ops;
+	wsc->user_data = user_data;
+
+	path = wsc->ops->get_path(wsc->user_data);
+	if (!path)
+		return wsc;
+
+	if (l_dbus_object_add_interface(dbus, path, IWD_WSC_INTERFACE, wsc))
+		return wsc;
+
+	wsc_free(wsc);
+	l_info("Unable to register interface %s at %s", IWD_WSC_INTERFACE,
+		path);
+	return NULL;
+}
+
+void wsc_destroy(struct wsc *wsc)
+{
+	l_dbus_object_remove_interface(dbus_get_bus(),
+					wsc->ops->get_path(wsc->user_data),
+					IWD_WSC_INTERFACE);
+}
+
+void wsc_start_enrollee(struct wsc *wsc, struct netdev *netdev,
+			struct scan_bss *target, const char *pin)
+{
+	wsc->netdev = netdev;
+	wsc->target = target;
+	wsc->config_method = pin ? WSC_CONFIGURATION_METHOD_KEYPAD :
+		WSC_CONFIGURATION_METHOD_VIRTUAL_PUSH_BUTTON;
+	wsc->pin = l_strdup(pin);
+
+	wsc_connect(wsc);
+}
+
+void wsc_cancel(struct wsc *wsc, netdev_disconnect_cb_t cb)
+{
+	int r;
+
+	if (!wsc->wsc_association)
+		return;
+
+	r = netdev_disconnect(wsc->netdev, cb, wsc->user_data);
+	if (r == 0)
+		return;
+
+	l_warn("Unable to initiate disconnect: %s", strerror(-r));
+	wsc_connect_cleanup(wsc);
+	wsc->ops->enrollee_done_cb(-ECANCELED, NULL, 0, wsc->user_data);
+
+	if (cb)
+		cb(wsc->netdev, false, wsc->user_data);
+}
+
+static const char *wsc_device_get_path(void *user_data)
+{
+	struct wsc *wsc = user_data;
+
+	if (!wsc || !wsc->netdev)
+		return NULL;
+
+	return netdev_get_path(wsc->netdev);
+}
+
+static void wsc_device_connect(struct l_dbus_message *message, const char *pin,
+				void *user_data)
+{
+	struct wsc *wsc = user_data;
+	struct l_dbus *dbus = dbus_get_bus();
+	unsigned int walk_time;
+	enum wsc_device_password_id dpid;
+	scan_notify_func_t results_cb;
+
+	if (wsc->pending) {
+		l_dbus_send(dbus, dbus_error_busy(message));
+		return;
+	}
+
+	wsc->station = station_find(netdev_get_ifindex(wsc->netdev));
+	if (!wsc->station) {
+		l_dbus_send(dbus, dbus_error_not_available(message));
+		return;
+	}
+
+	if (pin) {
+		if (strlen(pin) == 4 || wsc_pin_is_checksum_valid(pin))
+			dpid = WSC_DEVICE_PASSWORD_ID_DEFAULT;
+		else
+			dpid = WSC_DEVICE_PASSWORD_ID_USER_SPECIFIED;
+
+		walk_time = 60;
+		results_cb = pin_scan_results;
+	} else {
+		dpid = WSC_DEVICE_PASSWORD_ID_PUSH_BUTTON;
+		walk_time = WALK_TIME;
+		results_cb = push_button_scan_results;
+	}
+
+	if (!wsc_initiate_scan(wsc, dpid, results_cb)) {
+		l_dbus_send(dbus, dbus_error_failed(message));
+		return;
+	}
+
+	wsc->walk_timer = l_timeout_create(walk_time, walk_timeout, wsc, NULL);
+	wsc->pending = l_dbus_message_ref(message);
+}
+
+static void wsc_device_cancel(struct l_dbus_message *message, void *user_data)
+{
+	struct wsc *wsc = user_data;
+	struct l_dbus *dbus = dbus_get_bus();
+
+	if (!wsc->pending) {
+		l_dbus_send(dbus, dbus_error_not_available(message));
+		return;
+	}
+
+	if (wsc->pending_cancel) {
+		l_dbus_send(dbus, dbus_error_busy(message));
+		return;
+	}
+
+	wsc_cancel_scan(wsc);
+
+	if (wsc->station_state_watch) {
+		station_remove_state_watch(wsc->station,
+						wsc->station_state_watch);
+		wsc->station_state_watch = 0;
+		wsc->target = NULL;
+	}
+
+	if (wsc->wsc_association) {
+		wsc->pending_cancel = l_dbus_message_ref(message);
+		wsc_cancel(wsc, wsc_disconnect_cb);
+		return;
+	}
+
+	dbus_pending_reply(&wsc->pending, dbus_error_aborted(wsc->pending));
+	l_dbus_send(dbus, l_dbus_message_new_method_return(message));
+}
+
+static void wsc_device_enrollee_done(int err, struct wsc_credentials_info *creds,
+					unsigned int n_creds, void *user_data)
+{
+	struct wsc *wsc = user_data;
+
+	l_debug("err=%i", err);
+
+	switch (err) {
+	case 0:
+		break;
+	case -ECANCELED:
+		dbus_pending_reply(&wsc->pending,
+					dbus_error_aborted(wsc->pending));
+		return;
+	case -ENOKEY:
+		dbus_pending_reply(&wsc->pending,
+					wsc_error_no_credentials(wsc->pending));
+		return;
+	case -EBUSY:
+		dbus_pending_reply(&wsc->pending,
+					dbus_error_busy(wsc->pending));
+		return;
+	default:
+		dbus_pending_reply(&wsc->pending,
+					dbus_error_failed(wsc->pending));
+		return;
+	}
+
+	wsc_store_credentials(creds, n_creds);
+	wsc_try_credentials(wsc, creds, n_creds);
+}
+
+static const struct wsc_ops wsc_device_ops = {
+	.get_path = wsc_device_get_path,
+	.connect = wsc_device_connect,
+	.cancel = wsc_device_cancel,
+	.enrollee_done_cb = wsc_device_enrollee_done,
+};
+
+static void wsc_device_new(struct netdev *netdev)
 {
 	struct l_dbus *dbus = dbus_get_bus();
 	struct wsc *wsc;
@@ -1167,15 +1247,17 @@ static void wsc_add_interface(struct netdev *netdev)
 		return;
 	}
 
-	wsc = l_new(struct wsc, 1);
+	wsc = wsc_new(&wsc_device_ops, NULL);
+	wsc->user_data = wsc;
 	wsc->netdev = netdev;
 
-	if (!l_dbus_object_add_interface(dbus, netdev_get_path(netdev),
-						IWD_WSC_INTERFACE,
-						wsc)) {
-		wsc_free(wsc);
-		l_info("Unable to register %s interface", IWD_WSC_INTERFACE);
-	}
+	if (l_dbus_object_add_interface(dbus, netdev_get_path(netdev),
+					IWD_WSC_INTERFACE, wsc))
+		return;
+
+	wsc_free(wsc);
+	l_info("Unable to register interface %s at %s", IWD_WSC_INTERFACE,
+		netdev_get_path(netdev));
 }
 
 static void wsc_remove_interface(struct netdev *netdev)
@@ -1194,7 +1276,7 @@ static void wsc_netdev_watch(struct netdev *netdev,
 	case NETDEV_WATCH_EVENT_NEW:
 		if (netdev_get_iftype(netdev) == NETDEV_IFTYPE_STATION &&
 				netdev_get_is_up(netdev))
-			wsc_add_interface(netdev);
+			wsc_device_new(netdev);
 		break;
 	case NETDEV_WATCH_EVENT_DOWN:
 	case NETDEV_WATCH_EVENT_DEL:
diff --git a/src/wsc.h b/src/wsc.h
index ada08c89..e8a965f2 100644
--- a/src/wsc.h
+++ b/src/wsc.h
@@ -20,6 +20,11 @@
  *
  */
 
+struct wsc;
+struct netdev;
+struct scan_bss;
+struct iovec;
+
 struct wsc_credentials_info {
 	char ssid[33];
 	enum security security;
@@ -31,5 +36,17 @@ struct wsc_credentials_info {
 	bool has_passphrase;
 };
 
-typedef void (*wsc_done_cb_t)(int err, struct wsc_credentials_info *creds,
-				unsigned int n_creds, void *user_data);
+struct wsc_ops {
+	const char *(*get_path)(void *user_data);
+	void (*connect)(struct l_dbus_message *message, const char *pin,
+			void *user_data);
+	void (*cancel)(struct l_dbus_message *message, void *user_data);
+	void (*enrollee_done_cb)(int err, struct wsc_credentials_info *creds,
+					unsigned int n_creds, void *user_data);
+};
+
+struct wsc *wsc_new(const struct wsc_ops *ops, void *user_data);
+void wsc_start_enrollee(struct wsc *wsc, struct netdev *netdev,
+			struct scan_bss *target, const char *pin);
+void wsc_cancel(struct wsc *wsc, netdev_disconnect_cb_t cb);
+void wsc_destroy(struct wsc *wsc);
-- 
2.20.1

[PATCH 6/7] wsc: Accept extra IEs in wsc_start_enrollee

[Andrew Zaborowski]

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

---
 src/wsc.c | 24 ++++++++++++++----------
 src/wsc.h |  3 ++-
 2 files changed, 16 insertions(+), 11 deletions(-)

diff --git a/src/wsc.c b/src/wsc.c
index 1901feb6..423d2aa8 100644
--- a/src/wsc.c
+++ b/src/wsc.c
@@ -447,7 +447,8 @@ static inline enum wsc_rf_band freq_to_rf_band(uint32_t freq)
 	return WSC_RF_BAND_2_4_GHZ;
 }
 
-static void wsc_connect(struct wsc *wsc)
+static void wsc_connect(struct wsc *wsc, struct iovec *ies,
+			unsigned int ies_num)
 {
 	struct handshake_state *hs;
 	struct l_settings *settings = l_settings_new();
@@ -456,7 +457,7 @@ static void wsc_connect(struct wsc *wsc)
 	struct wsc_association_request request;
 	uint8_t *pdu;
 	size_t pdu_len;
-	struct iovec ie_iov;
+	struct iovec ie_iov[1 + ies_num];
 
 	wsc->target = NULL;
 
@@ -502,18 +503,20 @@ static void wsc_connect(struct wsc *wsc)
 		goto error;
 	}
 
-	ie_iov.iov_base = ie_tlv_encapsulate_wsc_payload(pdu, pdu_len,
-							&ie_iov.iov_len);
+	ie_iov[0].iov_base = ie_tlv_encapsulate_wsc_payload(pdu, pdu_len,
+							&ie_iov[0].iov_len);
 	l_free(pdu);
 
-	if (!ie_iov.iov_base) {
+	if (!ie_iov[0].iov_base) {
 		r = -ENOMEM;
 		goto error;
 	}
 
-	r = netdev_connect(wsc->netdev, bss, hs, &ie_iov, 1, wsc_netdev_event,
-				wsc_connect_cb, wsc);
-	l_free(ie_iov.iov_base);
+	memcpy(ie_iov + 1, ies, sizeof(struct iovec) * ies_num);
+
+	r = netdev_connect(wsc->netdev, bss, hs, ie_iov, 1 + ies_num,
+				wsc_netdev_event, wsc_connect_cb, wsc);
+	l_free(ie_iov[0].iov_base);
 
 	if (r < 0)
 		goto error;
@@ -1079,7 +1082,8 @@ void wsc_destroy(struct wsc *wsc)
 }
 
 void wsc_start_enrollee(struct wsc *wsc, struct netdev *netdev,
-			struct scan_bss *target, const char *pin)
+			struct scan_bss *target, const char *pin,
+			struct iovec *ies, unsigned int ies_num)
 {
 	wsc->netdev = netdev;
 	wsc->target = target;
@@ -1087,7 +1091,7 @@ void wsc_start_enrollee(struct wsc *wsc, struct netdev *netdev,
 		WSC_CONFIGURATION_METHOD_VIRTUAL_PUSH_BUTTON;
 	wsc->pin = l_strdup(pin);
 
-	wsc_connect(wsc);
+	wsc_connect(wsc, ies, ies_num);
 }
 
 void wsc_cancel(struct wsc *wsc, netdev_disconnect_cb_t cb)
diff --git a/src/wsc.h b/src/wsc.h
index e8a965f2..73ffdd33 100644
--- a/src/wsc.h
+++ b/src/wsc.h
@@ -47,6 +47,7 @@ struct wsc_ops {
 
 struct wsc *wsc_new(const struct wsc_ops *ops, void *user_data);
 void wsc_start_enrollee(struct wsc *wsc, struct netdev *netdev,
-			struct scan_bss *target, const char *pin);
+			struct scan_bss *target, const char *pin,
+			struct iovec *ies, unsigned int ies_num);
 void wsc_cancel(struct wsc *wsc, netdev_disconnect_cb_t cb);
 void wsc_destroy(struct wsc *wsc);
-- 
2.20.1

Re: [PATCH 6/7] wsc: Accept extra IEs in wsc_start_enrollee

[Denis Kenzior]

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

On 12/4/19 9:22 PM, Andrew Zaborowski wrote:
> ---
>   src/wsc.c | 24 ++++++++++++++----------
>   src/wsc.h |  3 ++-
>   2 files changed, 16 insertions(+), 11 deletions(-)
> 

This looks fine to me.

Regards,
-Denis

[PATCH 7/7] doc: Generalize wsc-api method descriptions

[Andrew Zaborowski]

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

---
 doc/wsc-api.txt | 42 +++++++++++++++++++++++++-----------------
 1 file changed, 25 insertions(+), 17 deletions(-)

diff --git a/doc/wsc-api.txt b/doc/wsc-api.txt
index cfd34943..561824ce 100644
--- a/doc/wsc-api.txt
+++ b/doc/wsc-api.txt
@@ -4,21 +4,24 @@ SimpleConfiguration hierarchy
 Service		net.connman.iwd
 Interface	net.connman.iwd.SimpleConfiguration
 Object path	/net/connman/iwd/{phy0,phy1,...}/{1,2,...}
+Object path	/net/connman/iwd/{phy0,phy1,...}/p2p_peers/{aa_bb_cc_dd_ee_ff}
 
 Methods		void PushButton()
 
 			Start WSC (formerly known as WPS, Wi-Fi Protected
-			Setup) configuration in PushButton mode.
+			Setup) configuration in PushButton mode or trigger a
+			connection to a specific P2P peer.  The usage will
+			depend on which object this interface is found on.
 
-			Any connected networks on the device will be
-			disconnected and scanning will commence to find the
-			access point in PushButton mode.  If multiple access
-			points are found, then a SessionOverlap error will be
-			returned.
+			In the first use case any connected networks on the
+			device will be disconnected and scanning will commence
+			to find the access point in PushButton mode.  If
+			multiple access points are found, then a
+			SessionOverlap error will be returned.
 
 			This method returns once the configuration has been
-			completed and the network has been successfully
-			connected.
+			completed and the network or the P2P peer has been
+			successfully connected.
 
 			Possible errors:
 			net.connman.iwd.SimpleConfiguration.SessionOverlap
@@ -28,6 +31,8 @@ Methods		void PushButton()
 			net.connman.iwd.Aborted
 			net.connman.iwd.Failed
 			net.connman.iwd.Busy
+			net.connman.iwd.InvalidArguments
+			net.connman.iwd.NotSupported
 
 		string GeneratePin()
 
@@ -35,10 +40,12 @@ Methods		void PushButton()
 			digit suitable for use by most user interfaces.
 
 		void StartPin(string pin)
-			Start WSC in PIN mode.  If iwd's WSC configuration
-			indicates that the device does not support a display,
-			a static PIN from the main.conf configuration file is
-			used.  Contents of pin are ignored in this case.
+
+			Start WSC or connect to a specific P2P peer in PIN
+			mode.  If iwd's WSC configuration indicates that the
+			device does not support a display, a static PIN from
+			the main.conf configuration file is used.  Contents
+			of pin are ignored in this case.
 
 			Otherwise, the pin provided will be utilized.  This
 			can be an automatically generated PIN that contains a
@@ -47,8 +54,8 @@ Methods		void PushButton()
 			8 digit PIN with an included check digit.
 
 			This method returns once the configuration has been
-			completed and the network has been successfully
-			connected.
+			completed and the network or the P2P peer has been
+			successfully connected.
 
 			Possible errors:
 			net.connman.iwd.SimpleConfiguration.NoCredentials
@@ -59,9 +66,10 @@ Methods		void PushButton()
 			net.connman.iwd.Busy
 			net.connman.iwd.InvalidArguments
 			net.connman.iwd.InvalidFormat
+			net.connman.iwd.NotSupported
 
 		void Cancel()
 
-			Aborts any ongoing WSC operations.  If no WSC
-			operation is ongoing, net.connman.iwd.NotAvailable is
-			returned.
+			Aborts any ongoing WSC operations or a P2P connection.
+			If no operation is ongoing, net.connman.iwd.NotAvailable
+			is returned.
-- 
2.20.1

Re: [PATCH 7/7] doc: Generalize wsc-api method descriptions

[Denis Kenzior]

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

On 12/4/19 9:22 PM, Andrew Zaborowski wrote:
> ---
>   doc/wsc-api.txt | 42 +++++++++++++++++++++++++-----------------
>   1 file changed, 25 insertions(+), 17 deletions(-)
> 

<snip>

> @@ -35,10 +40,12 @@ Methods		void PushButton()
>   			digit suitable for use by most user interfaces.
>   
>   		void StartPin(string pin)
> -			Start WSC in PIN mode.  If iwd's WSC configuration
> -			indicates that the device does not support a display,
> -			a static PIN from the main.conf configuration file is
> -			used.  Contents of pin are ignored in this case.
> +
> +			Start WSC or connect to a specific P2P peer in PIN
> +			mode.  If iwd's WSC configuration indicates that the
> +			device does not support a display, a static PIN from
> +			the main.conf configuration file is used.  Contents
> +			of pin are ignored in this case.

Reminds me that I don't think we implement this main.conf part.

>   
>   			Otherwise, the pin provided will be utilized.  This
>   			can be an automatically generated PIN that contains a


Applied, thanks.

Regards,
-Denis

Re: [PATCH 1/7] iwmon: Print WSC AuthorizedMACs extended attributes

[Denis Kenzior]

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

Hi Andrew,

On 12/4/19 9:22 PM, Andrew Zaborowski wrote:
> ---
>   monitor/nlmon.c | 16 ++++++++++++++++
>   1 file changed, 16 insertions(+)
> 

Patch 1 & 2 applied, thanks.

Regards,
-Denis