From: Denis Kenzior <denkenz@gmail.com>
To: iwd@lists.01.org
Subject: Re: [PATCH 08/11] wsc: Add wsc_new_p2p_enrollee, refactor
Date: Tue, 22 Oct 2019 09:47:28 -0500 [thread overview]
Message-ID: <c02be0e1-950b-decb-d1bc-2d208e20c42b@gmail.com> (raw)
In-Reply-To: <20191021135510.12657-8-balrogg@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 10912 bytes --]
Hi Andrew,
On 10/21/19 8:55 AM, Andrew Zaborowski wrote:
> From: Andrew Zaborowski <andrew.zaborowski@intel.com>
>
> Until now WSC could only be triggered by calling a DBus method and
> success and error paths would end with a reply to the DBus method.
> Instead use a wsc done callback pointer so that the WSC connection
> logic can be triggered through DBus or a C API. The DBus method reply
> is sent from inside the callback when WSC is triggered through DBus.
>
> Export a function that adds a new entry point to start the WSC
> connection for the P2P provisioning phase. The function doesn't have
> anything specific to P2P at this point.
> ---
> Makefile.am | 2 +-
> src/wsc.c | 181 +++++++++++++++++++++++++++++++++++++---------------
> 2 files changed, 131 insertions(+), 52 deletions(-)
>
> diff --git a/Makefile.am b/Makefile.am
> index ecd1c228..03be9577 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -199,7 +199,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 \
Looks like you forgot to add wsc.h itself?
> src/backtrace.h src/backtrace.c \
> src/knownnetworks.h \
> src/knownnetworks.c \
> diff --git a/src/wsc.c b/src/wsc.c
> index d971797a..61683e7b 100644
> --- a/src/wsc.c
> +++ b/src/wsc.c
> @@ -44,6 +44,7 @@
> #include "src/storage.h"
> #include "src/iwd.h"
> #include "src/network.h"
> +#include "src/wsc.h"
>
> #define WALK_TIME 120
>
> @@ -60,18 +61,14 @@ 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 creds[3];
This seems like it belongs in a separate patch
> 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;
> };
> @@ -160,15 +157,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;
This again looks like a separate issue and deserves a separate commit
> }
>
> static void wsc_store_credentials(struct wsc *wsc)
> @@ -224,6 +218,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)
> {
> @@ -231,33 +241,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 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));
So we don't try credentials now?
> 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,
> @@ -451,30 +462,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 +516,38 @@ 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 *creds,
> + unsigned int n_creds, void *user_data)
> +{
> + struct wsc *wsc = user_data;
> +
> + l_debug("err=%i", err);
> +
> + if (err == -ECANCELED) {
> + dbus_pending_reply(&wsc->pending,
> + dbus_error_aborted(wsc->pending));
> + return;
> + } else if (err == -ENOKEY) {
> + dbus_pending_reply(&wsc->pending,
> + wsc_error_no_credentials(wsc->pending));
> + return;
> + } else if (err == -EBUSY) {
> + dbus_pending_reply(&wsc->pending,
> + dbus_error_busy(wsc->pending));
> + return;
> + } else if (err) {
> + dbus_pending_reply(&wsc->pending,
> + dbus_error_failed(wsc->pending));
> + return;
> + }
switch/case might look cleaner
> +
> + wsc_store_credentials(wsc);
> + wsc_try_credentials(wsc);
> }
>
> static void station_state_watch(enum station_state state, void *userdata)
> @@ -541,6 +576,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 +615,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 +1125,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 +1141,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);
> }
>
> @@ -1139,6 +1187,37 @@ static void wsc_netdev_watch(struct netdev *netdev,
> }
> }
>
> +struct wsc *wsc_new_p2p_enrollee(struct netdev *netdev, struct scan_bss *target,
> + char *pin, wsc_done_cb_t done_cb,
> + void *user_data)
> +{
> + struct wsc *wsc;
> +
> + wsc = l_new(struct wsc, 1);
> + wsc->netdev = netdev;
> + wsc->target = target;
> + wsc->config_method = pin ? WSC_CONFIGURATION_METHOD_KEYPAD :
> + WSC_CONFIGURATION_METHOD_PUSH_BUTTON;
> + wsc->done_cb = done_cb;
> + wsc->done_data = user_data;
> + wsc->pin = l_strdup(pin);
> +
> + wsc_connect(wsc);
> +
> + /*
> + * Wsc objects created this way are not handled in wsc_remove_interface,
> + * the caller is expected to watch interfaces going away and there's no
> + * need to handle the event in both places.
> + */
> +
> + return wsc;
> +}
This again looks like it should be a separate commit
> +
> +void wsc_destroy(struct wsc *wsc)
> +{
> + wsc_free(wsc);
> +}
> +
> static int wsc_init(void)
> {
> l_debug("");
>
Regards,
-Denis
next prev parent reply other threads:[~2019-10-22 14:47 UTC|newest]
Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-10-21 13:55 [PATCH 01/11] netdev: Add a wdev_id based frame watch API Andrew Zaborowski
2019-10-21 13:55 ` [PATCH 02/11] netdev: Report RSSI to frame watch callbacks Andrew Zaborowski
2019-10-22 3:34 ` Denis Kenzior
2019-10-22 13:46 ` Andrew Zaborowski
2019-10-21 13:55 ` [PATCH 03/11] netdev: Extend checks for P2P scenarios Andrew Zaborowski
2019-10-22 3:36 ` Denis Kenzior
2019-10-21 13:55 ` [PATCH 04/11] eapol: Move the EAP event handler to handshake state Andrew Zaborowski
2019-10-22 4:11 ` Denis Kenzior
2019-10-22 14:00 ` Andrew Zaborowski
2019-10-22 14:34 ` Denis Kenzior
2019-10-21 13:55 ` [PATCH 05/11] unit: Update test-wsc to use handshake_state_set_eap_event_func Andrew Zaborowski
2019-10-21 13:55 ` [PATCH 06/11] wsc: Replace netdev_connect_wsc with netdev_connect usage Andrew Zaborowski
2019-10-21 13:55 ` [PATCH 07/11] netdev: Drop unused netdev_connect_wsc Andrew Zaborowski
2019-10-21 13:55 ` [PATCH 08/11] wsc: Add wsc_new_p2p_enrollee, refactor Andrew Zaborowski
2019-10-22 14:47 ` Denis Kenzior [this message]
2019-10-22 23:46 ` Andrew Zaborowski
2019-10-21 13:55 ` [PATCH 09/11] wsc: Accept extra IEs in wsc_new_p2p_enrollee Andrew Zaborowski
2019-10-21 13:55 ` [PATCH 10/11] wiphy: Add wiphy_get_max_roc_duration Andrew Zaborowski
2019-10-22 3:26 ` Denis Kenzior
2019-10-21 13:55 ` [PATCH 11/11] wiphy: Add wiphy_get_supported_rates Andrew Zaborowski
2019-10-22 14:53 ` [PATCH 01/11] netdev: Add a wdev_id based frame watch API Denis Kenzior
2019-10-22 23:56 ` Andrew Zaborowski
2019-10-23 0:23 ` Denis Kenzior
2019-10-23 1:04 ` Andrew Zaborowski
2019-10-23 1:32 ` Denis Kenzior
2019-10-24 0:59 ` Andrew Zaborowski
2019-10-24 2:53 ` Denis Kenzior
2019-10-24 3:22 ` Andrew Zaborowski
2019-10-24 15:29 ` Denis Kenzior
2019-10-24 21:47 ` Andrew Zaborowski
2019-10-24 22:16 ` Denis Kenzior
2019-10-24 22:45 ` Andrew Zaborowski
2019-10-25 1:27 ` Denis Kenzior
2019-10-25 2:59 ` Andrew Zaborowski
2019-10-25 3:56 ` Denis Kenzior
2019-10-25 4:42 ` Andrew Zaborowski
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=c02be0e1-950b-decb-d1bc-2d208e20c42b@gmail.com \
--to=denkenz@gmail.com \
--cc=iwd@lists.01.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.