Monday, December 24, 2018

Re: security/wpa_supplicant: Reassoc on NWID change

I've started using this at 35C3, and it mostly works for me.

The one thing it doesn't handle for me is when I change the lladdr, but
I have some ideas for it.

I fixed the whitespace, and changed the main bit to use a switch instead
of an if elseif dance.

This is what I am running right now, what do you think?



Index: Makefile
===================================================================
RCS file: /cvs/openbsd/ports/security/wpa_supplicant/Makefile,v
retrieving revision 1.39
diff -u -p -u -p -r1.39 Makefile
--- Makefile 24 Oct 2018 17:16:19 -0000 1.39
+++ Makefile 20 Dec 2018 19:37:31 -0000
@@ -3,7 +3,7 @@
COMMENT= IEEE 802.1X supplicant

DISTNAME= wpa_supplicant-2.6
-REVISION= 4
+REVISION= 5
CATEGORIES= security net

HOMEPAGE= http://w1.fi/wpa_supplicant/
Index: patches/patch-src_drivers_driver_openbsd_c
===================================================================
RCS file: /cvs/openbsd/ports/security/wpa_supplicant/patches/patch-src_drivers_driver_openbsd_c,v
retrieving revision 1.5
diff -u -p -u -p -r1.5 patch-src_drivers_driver_openbsd_c
--- patches/patch-src_drivers_driver_openbsd_c 17 May 2016 08:29:27 -0000 1.5
+++ patches/patch-src_drivers_driver_openbsd_c 24 Dec 2018 22:46:25 -0000
@@ -1,24 +1,159 @@
$OpenBSD: patch-src_drivers_driver_openbsd_c,v 1.5 2016/05/17 08:29:27 dcoppa Exp $

-Fix includes
+Fix includes and react to NWID changes and suspend/resume.

---- src/drivers/driver_openbsd.c.orig Sun Sep 27 21:02:05 2015
-+++ src/drivers/driver_openbsd.c Mon Sep 28 09:51:53 2015
-@@ -9,13 +9,14 @@
+Index: src/drivers/driver_openbsd.c
+--- src/drivers/driver_openbsd.c.orig
++++ src/drivers/driver_openbsd.c
+@@ -9,19 +9,34 @@
#include "includes.h"
#include <sys/ioctl.h>

+#include "common.h"
+#include "driver.h"
++#include "eloop.h"
+
++#include <sys/socket.h>
#include <net/if.h>
+#include <net/if_var.h>
++#include <net/route.h>
#include <net80211/ieee80211.h>
#include <net80211/ieee80211_crypto.h>
#include <net80211/ieee80211_ioctl.h>
--
+
-#include "common.h"
-#include "driver.h"
++#define RTM_READSZ 2048

struct openbsd_driver_data {
- char ifname[IFNAMSIZ + 1];
+- char ifname[IFNAMSIZ + 1];
+ void *ctx;
+
+- int sock; /* open socket for 802.11 ioctls */
++ char ifname[IFNAMSIZ + 1];
++ int ifindex; /* Ifindex of the configured interface */
++
++ int sock; /* open socket for 802.11 ioctls */
++ int rtsock; /* routing socket for interface state messages */
++
++ /* These fields are used to track the last seen (and associated) access point
++ to determine whether we should kick off an association event */
++ int nwid_len; /* Length of last seen SSID (as per routing message) */
++ char nwid[IEEE80211_NWID_LEN]; /* Last seen SSID (as per routing message) */
++ char addr[IEEE80211_ADDR_LEN]; /* Last seen BSSID (as per routing message) */
+ };
+
+
+@@ -90,6 +105,78 @@ wpa_driver_openbsd_set_key(const char *ifname, void *p
+ return 0;
+ }
+
++static void
++wpa_driver_openbsd_event_receive(int sock, void *global, void *sock_ctx)
++{
++ struct openbsd_driver_data *drv = sock_ctx;
++ struct rt_msghdr *rtm;
++ struct if_ieee80211_data *ifie;
++ char *rtmmsg;
++ ssize_t n;
++
++ rtmmsg = os_zalloc(RTM_READSZ);
++ if (rtmmsg == NULL) {
++ wpa_printf(MSG_ERROR, "Can't allocate space for routing message");
++ return;
++ }
++
++ do {
++ n = read(sock, rtmmsg, RTM_READSZ);
++ } while (n == -1 && errno == EINTR);
++
++ if (n == -1)
++ goto done;
++
++ rtm = (struct rt_msghdr *)rtmmsg;
++
++ if ((size_t)n < sizeof(rtm->rtm_msglen) ||
++ n < rtm->rtm_msglen ||
++ rtm->rtm_version != RTM_VERSION)
++ goto done;
++
++ if (rtm->rtm_index != drv->ifindex)
++ goto done;
++
++ switch (rtm->rtm_type) {
++ case RTM_80211INFO:
++ ifie = &((struct if_ieee80211_msghdr *)rtm)->ifim_ifie;
++
++ if ((ifie->ifie_nwid_len != drv->nwid_len) ||
++ (os_memcmp(drv->nwid, ifie->ifie_nwid,
++ ifie->ifie_nwid_len) != 0) ||
++ (os_memcmp(drv->addr, ifie->ifie_addr,
++ IEEE80211_ADDR_LEN) != 0)) {
++ os_memcpy(drv->addr, ifie->ifie_addr,
++ IEEE80211_ADDR_LEN);
++
++ os_memcpy(drv->nwid, ifie->ifie_nwid,
++ ifie->ifie_nwid_len);
++ drv->nwid_len = ifie->ifie_nwid_len;
++
++ /* Emit ASSOC event */
++ wpa_supplicant_event(drv->ctx, EVENT_ASSOC, NULL);
++ }
++ break;
++ case RTM_IFINFO:
++ /* This is here so we can react to suspend/resume.
++ *
++ * This is a bit rough, sometimes there are two or more
++ * IFINFOs notifying us that the device just got "up"
++ * again. It doesn't seem to hurt to issue multiple
++ * EVENT_ASSOC in those cases though.
++ */
++ if (rtm->rtm_flags & RTF_UP)
++ wpa_supplicant_event(drv->ctx, EVENT_ASSOC, NULL);
++ break;
++ default:
++ ;;
++ }
++
++
++done:
++ os_free(rtmmsg);
++}
++
+ static void *
+ wpa_driver_openbsd_init(void *ctx, const char *ifname)
+ {
+@@ -103,9 +190,21 @@ wpa_driver_openbsd_init(void *ctx, const char *ifname)
+ if (drv->sock < 0)
+ goto fail;
+
++ drv->rtsock = socket(PF_ROUTE, SOCK_RAW, AF_UNSPEC);
++ if (drv->rtsock < 0)
++ goto fail;
++
+ drv->ctx = ctx;
+ os_strlcpy(drv->ifname, ifname, sizeof(drv->ifname));
+
++ drv->ifindex = if_nametoindex(drv->ifname);
++ if (drv->ifindex == 0) /* No interface with that name */
++ goto fail;
++
++ drv->nwid_len = wpa_driver_openbsd_get_ssid(drv, drv->nwid);
++ wpa_driver_openbsd_get_bssid(drv, drv->addr);
++
++ eloop_register_read_sock(drv->rtsock, wpa_driver_openbsd_event_receive, NULL, drv);
+ return drv;
+
+ fail:
+@@ -119,7 +218,11 @@ wpa_driver_openbsd_deinit(void *priv)
+ {
+ struct openbsd_driver_data *drv = priv;
+
++ eloop_unregister_read_sock(drv->rtsock);
++
+ close(drv->sock);
++ close(drv->rtsock);
++
+ os_free(drv);
+ }
+



On 2018 Dec 13 (Thu) at 19:30:11 +0100 (+0100), Gregor Best wrote:
:
:Hi Mikolaj et al,
:
:Mikolaj Kucharski <mikolaj@kucharski.name> writes:
:
:>
:> Here is debug output of one day session, with suspend in the middle
:> when going home, back to work and home again.
:> [...]
:> Tomorrow I may give you example of above log when walking around the
:> office, to compare events while switching APs.
:> [...]
:
:That'd be good, thank you.
:
:> The other day I did testing by walking around and it works most of the
:> time, however there is a race somewhere, as while I was walking around
:> I sometimes ended up without connection and ifconfig(8) at the same
:> time was showing:
:>
:> status: active
:> ieee80211: join lighthouse chan 100 bssid <random-ap-close> <level>dBm wpaprotos wpa2 wpaakms 802.1x wpaciphers ccmp wpagroupcipher ccmp
:>
:> like network should work, but it didn't and connection was totally
:> offline.
:
:> This is very empirical, but it looked to me, when I walked
:> slower to give wpa_supplicant time to reassoc with each access point on
:> the way it all worked. When I got bored and walked faster, passing by
:> multiple APs on the way and with reassocs happening one after another,
:> before previous assoc was able to finish and me finally stopping, I
:> ended up with iwn0 card with status active, but no connectivity.
:> Restarting slaacd(8) or dhclient(8) was not helping as problem was layer
:> below them.
:> [...]
:
:Yeah, I've got the feeling that wpa_supplicant really doesn't like
:getting poked while it's doing its thing. Maybe I've got to work on the
:timing here a bit or sort of deduplicate ASSOC events after multiple
:IFINFO/802INFO are received.
:
:> [...]
:> I think your diff makes sense and with handling of RTM_IFINFO it's good
:> progress in correct direction. I even took your diff and tested with
:> latest wpa_supplicant 2.7 and it works there too.
:> [...]
:
:That is good to hear. I'm planning on sending this upstream as soon as
:we've agreed it's more or less working.
:
:> [...]
:> If you would like to see any specific info tomorrow, please let me know
:> I will try to arrange something. After that I'll go offline for family
:> time.
:> [...]
:
:I'm afraid I don't have anything more specific I could ask you to test.
:Thanks a bunch for your help this far. I'll just have to walk around my
:university some time next week, I'm not at work then anyway :)
:
:> [...]
:>> ++ if (rtm->rtm_type == RTM_80211INFO) {
:>> ++ printf("RTM_80211INFO received\n");
:>
:> This printf(3) should be removed or changed into wpa_printf(). For my tests
:> I've used wpa_printf(MSG_DEBUG, ...), which I think is good enough balance
:> of being not noisy in logs by default, but still providing info when there
:> is a need via debug flag to the cli. However this is your call and giving
:> the current sparsity of debug log lines in this c-file, I would just remove
:> it.
:> [...]
:
:Absolutely. This was an oversight that I wanted to remove before sending
:out the patch. Same for the weird indentation :)
:
:--
: Gregor



--
If someone had told me I would be Pope one day, I would have studied
harder.
-- Pope John Paul I

No comments:

Post a Comment