[PATCH 01/10] station: Don't call network_rank_update with NULL network
by Andrew Zaborowski
Move the update of station->networks_sorted order to before we set
station->connected_network NULL to avoid a crash when we attempt to
use the NULL pointer.
---
src/station.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/station.c b/src/station.c
index e44dc145..e53952a7 100644
--- a/src/station.c
+++ b/src/station.c
@@ -1298,15 +1298,15 @@ static void station_reset_connection_state(struct station *station)
station_roam_state_clear(station);
- station->connected_bss = NULL;
- station->connected_network = NULL;
-
/* Refresh the ordered network list */
network_rank_update(station->connected_network, false);
l_queue_remove(station->networks_sorted, station->connected_network);
l_queue_insert(station->networks_sorted, station->connected_network,
network_rank_compare, NULL);
+ station->connected_bss = NULL;
+ station->connected_network = NULL;
+
l_dbus_property_changed(dbus, netdev_get_path(station->netdev),
IWD_STATION_INTERFACE, "ConnectedNetwork");
l_dbus_property_changed(dbus, network_get_path(network),
--
2.25.1
2 years
[PATCH v2 0/2] Fix the documented behaviour of GetOrderedNetworks
by Alvin Šipraga
I noticed that the Station DBus method GetOrderedNetworks() was not
respecting the behaviour documented in doc/station-api.txt, namely that:
1. If the device is currently connected to a network, that network
is always first on the list,
2. followed by any known networks that have been used at least once
before [...]
These two patches should address the two points respectively.
---
v1 -> v2:
- Fix a careless naming mistake which caused the first patch not to
compile.
Alvin Šipraga (2):
station: refresh ordered network list on (dis)connect
treewide: guard compare functions against signed integer overflow
src/network.c | 2 +-
src/scan.c | 2 +-
src/station.c | 16 ++++++++++++++--
tools/hwsim.c | 2 +-
4 files changed, 17 insertions(+), 5 deletions(-)
--
2.28.0
2 years
[PATCH 0/2] Fix the documented behaviour of GetOrderedNetworks
by Alvin Šipraga
I noticed that the Station DBus method GetOrderedNetworks() was not
respecting the behaviour documented in doc/station-api.txt, namely that:
1. If the device is currently connected to a network, that network
is always first on the list,
2. followed by any known networks that have been used at least once
before [...]
These two patches should address the two points respectively.
Alvin Šipraga (2):
station: refresh ordered network list on (dis)connect
treewide: guard compare functions against signed integer overflow
src/network.c | 2 +-
src/scan.c | 2 +-
src/station.c | 16 ++++++++++++++--
tools/hwsim.c | 2 +-
4 files changed, 17 insertions(+), 5 deletions(-)
--
2.28.0
2 years
[PATCH 10/16] eap-wsc: Zero a temporary buffer before freeing
by Andrew Zaborowski
---
src/eap-wsc.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/src/eap-wsc.c b/src/eap-wsc.c
index ccb52979..ca915471 100644
--- a/src/eap-wsc.c
+++ b/src/eap-wsc.c
@@ -1157,11 +1157,13 @@ static bool load_hexencoded(struct l_settings *settings, const char *key,
return false;
if (decoded_len != len) {
+ explicit_bzero(decoded, decoded_len);
l_free(decoded);
return false;
}
memcpy(to, decoded, len);
+ explicit_bzero(decoded, decoded_len);
l_free(decoded);
return true;
--
2.25.1
2 years
[PATCH 01/16] wscutil: Handle a deprecated network key format
by Andrew Zaborowski
Implement a note from the spec saying that implementations should handle
NUL-terminated Network Keys inside credentials structures.
---
src/wscutil.c | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/src/wscutil.c b/src/wscutil.c
index 6b445ce4..a41702e8 100644
--- a/src/wscutil.c
+++ b/src/wscutil.c
@@ -362,13 +362,25 @@ static bool extract_network_key(struct wsc_attr_iter *iter, void *data)
{
struct iovec *network_key = data;
unsigned int len;
+ const uint8_t *key;
len = wsc_attr_iter_get_length(iter);
if (len > 64)
return false;
+ /*
+ * WSC 2.0.5, Section 12, Network Key:
+ * "Some existing implementations based on v1.0h null-terminate the
+ * passphrase value, i.e., add an extra 0x00 octet into the end of
+ * the value. For backwards compatibility, implementations shall be
+ * able to parse such a value"
+ */
+ key = wsc_attr_iter_get_data(iter);
+ if (len && key[len - 1] == 0x00)
+ len--;
+
network_key->iov_len = len;
- network_key->iov_base = (void *) wsc_attr_iter_get_data(iter);
+ network_key->iov_base = (void *) key;
return true;
}
--
2.25.1
2 years
[PATCH 09/16] eap-wsc: Validate enrollee_nonce (N1) in M{2,4,6,8}
by Andrew Zaborowski
---
src/eap-wsc.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/src/eap-wsc.c b/src/eap-wsc.c
index 0fec29c6..ccb52979 100644
--- a/src/eap-wsc.c
+++ b/src/eap-wsc.c
@@ -479,6 +479,10 @@ static void eap_wsc_handle_m8(struct eap_state *eap,
return;
}
+ if (memcmp(m8.enrollee_nonce, wsc->m1->enrollee_nonce,
+ sizeof(m8.enrollee_nonce)))
+ return;
+
if (!authenticator_check(wsc, pdu, len))
return;
@@ -580,6 +584,10 @@ static void eap_wsc_handle_m6(struct eap_state *eap,
if (wsc_parse_m6(pdu, len, &m6, &encrypted) != 0)
goto send_nack;
+ if (memcmp(m6.enrollee_nonce, wsc->m1->enrollee_nonce,
+ sizeof(m6.enrollee_nonce)))
+ return;
+
if (!authenticator_check(wsc, pdu, len))
return;
@@ -677,6 +685,10 @@ static void eap_wsc_handle_m4(struct eap_state *eap,
if (wsc_parse_m4(pdu, len, &m4, &encrypted) != 0)
goto send_nack;
+ if (memcmp(m4.enrollee_nonce, wsc->m1->enrollee_nonce,
+ sizeof(m4.enrollee_nonce)))
+ return;
+
if (!authenticator_check(wsc, pdu, len))
return;
@@ -815,6 +827,10 @@ static void eap_wsc_handle_m2(struct eap_state *eap,
return;
}
+ if (memcmp(wsc->m2->enrollee_nonce, wsc->m1->enrollee_nonce,
+ sizeof(wsc->m2->enrollee_nonce)))
+ return;
+
if (!l_key_validate_dh_payload(wsc->m2->public_key,
sizeof(wsc->m2->public_key),
crypto_dh5_prime,
--
2.25.1
2 years
[PATCH] eapol: prevent key reinstallation on retransmitted Msg4/4
by Mathy Vanhoef
Currently an adversary can retransmit EAPOL Msg4/4 to make the AP
reinstall the PTK. Against older Linux kernels this can subsequently
be used to decrypt, replay, and possibly decrypt frames. See the
KRACK attacks research at krackattacks.com for attack scenarios.
In this case no machine-in-the-middle position is needed to trigger
the key reinstallation.
Fix this by using the ptk_complete boolean to track when the 4-way
handshake has completed (similar to its usage for clients). When
receiving a retransmitted Msg4/4 accept this frame but do not reinstall
the PTK.
Credits to Chris M. Stone, Sam Thomas, and Tom Chothia of Birmingham
University to help discover this issue.
---
src/eapol.c | 15 ++++++++++++---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/src/eapol.c b/src/eapol.c
index b0036c10..e3581cfe 100644
--- a/src/eapol.c
+++ b/src/eapol.c
@@ -1462,7 +1462,6 @@ static void eapol_handle_ptk_2_of_4(struct eapol_sm *sm,
memcpy(sm->handshake->snonce, ek->key_nonce,
sizeof(sm->handshake->snonce));
sm->handshake->have_snonce = true;
- sm->handshake->ptk_complete = true;
sm->frame_retry = 0;
@@ -1782,7 +1781,15 @@ static void eapol_handle_ptk_4_of_4(struct eapol_sm *sm,
l_timeout_remove(sm->timeout);
sm->timeout = NULL;
- handshake_state_install_ptk(sm->handshake);
+ /*
+ * If ptk_complete is set, then we are receiving Message 4 again.
+ * This might be a retransmission, so accept but don't install
+ * the keys again.
+ */
+ if (!sm->handshake->ptk_complete)
+ handshake_state_install_ptk(sm->handshake);
+
+ sm->handshake->ptk_complete = true;
}
static void eapol_handle_gtk_1_of_2(struct eapol_sm *sm,
@@ -2185,6 +2192,7 @@ static void eapol_auth_key_handle(struct eapol_sm *sm,
size_t frame_len = 4 + L_BE16_TO_CPU(frame->header.packet_len);
const struct eapol_key *ek = eapol_key_validate((const void *) frame,
frame_len, sm->mic_len);
+ uint16_t key_data_len;
if (!ek)
return;
@@ -2199,7 +2207,8 @@ static void eapol_auth_key_handle(struct eapol_sm *sm,
if (!sm->handshake->have_anonce)
return; /* Not expecting an EAPoL-Key yet */
- if (!sm->handshake->ptk_complete)
+ key_data_len = EAPOL_KEY_DATA_LEN(ek, sm->mic_len);
+ if (key_data_len != 0)
eapol_handle_ptk_2_of_4(sm, ek);
else
eapol_handle_ptk_4_of_4(sm, ek);
--
2.27.0
2 years
[PATCH 1/6] wfd-source: Fix some races on iwd name owner change
by Andrew Zaborowski
Subscribe to InterfacesAdded/Removed/PropertiesChanged signals before
using GetManagedObjects. For some reason when iwd starts after the
client, we consistently get the managed objects list from before Adapter
interfaces are added but we miss the subsequent InterfacesAdded
signals, probably has to do with the GetManagedObjects and the AddMatch
calls all being synchronous.
Secondly call self.populate_devices() on init as it won't be called if
IWD is not on the bus.
---
test/wfd-source | 24 +++++++++++++-----------
1 file changed, 13 insertions(+), 11 deletions(-)
diff --git a/test/wfd-source b/test/wfd-source
index e74d0adf..a83cd0cb 100755
--- a/test/wfd-source
+++ b/test/wfd-source
@@ -826,6 +826,7 @@ class WFDSource(Gtk.Window):
self.rtsp_port = 7236
self.devices = None
self.objects = {}
+ self.populate_devices()
self.dbus = dbus.SystemBus()
self.dbus.watch_name_owner('net.connman.iwd', self.on_name_owner_change)
self.on_name_owner_change('dummy' if self.dbus.name_has_owner('net.connman.iwd') else '')
@@ -857,17 +858,6 @@ class WFDSource(Gtk.Window):
return True
manager = dbus.Interface(self.dbus.get_object('net.connman.iwd', '/'), 'org.freedesktop.DBus.ObjectManager')
- self.devices = {}
- self.objects = manager.GetManagedObjects()
-
- for path in self.objects:
- if DEVICE_IF in self.objects[path]:
- self.add_dev(path)
- for path in self.objects:
- if PEER_IF in self.objects[path]:
- self.add_peer(path)
-
- self.populate_devices()
self.dbus.add_signal_receiver(self.on_properties_changed,
bus_name="net.connman.iwd",
@@ -883,6 +873,18 @@ class WFDSource(Gtk.Window):
dbus_interface="org.freedesktop.DBus.ObjectManager",
signal_name="InterfacesRemoved")
+ self.objects = manager.GetManagedObjects()
+ self.devices = {}
+
+ for path in self.objects:
+ if DEVICE_IF in self.objects[path]:
+ self.add_dev(path)
+ for path in self.objects:
+ if PEER_IF in self.objects[path]:
+ self.add_peer(path)
+
+ self.populate_devices()
+
svc_mgr = dbus.Interface(self.dbus.get_object('net.connman.iwd', '/net/connman/iwd'), SVC_MGR_IF)
svc_mgr.RegisterDisplayService({
'Source': True,
--
2.25.1
2 years
Re: build: hook up traditional dbus activation
by Marcel Holtmann
Hi Job,
> Hi, I am the original reporter Andreas is talking about. I'm going to admit that I have no idea on how to use dbus activation. I only know how to start a service at boot via shell scripts.
>
> I understand if you won't support non-systemd init systems like sysvinit. I haven't maintained daemons, but from the looks of it, maintaining support for multiple inits is surely hard. However users who don't use systemd as their init, like Devuan users, Gentoo users, and Debian users who replaced systemd with sysvinit/openrc would appreciate it if upstream decided to support multiple inits. Gentoo users have a community which maintains their own iwd init script for openrc. Devuan users however, have to workaround this problem by converting the iwd systemd service file to a sysvinit script, and update-rc.d as root. If I haven't known about this, I would still be using wpa_supplicant by now. iwd actually made my wifi connection more reliable. When I was using wpa_supplicant, I had to reboot my system everytime the connection drops. With iwd, I only have to restart iwd and my connection is back. I figured that I should contribute back to the community by adding sysvinit support.
>
> So whatever decision you guys make, I will respect it. As you said, we can ship a modified version instead for non-systemd users. I am already preparing a fork of the iwd package for Devuan users[1]. The patch is ugly, but it works.
so about 16 something years ago (long before systemd), I tried to provide init scripts for all the distributions. It was a disaster since all of them are a bit different. So I decided to not do that anymore and the distro package maintainers have to do that job as downstream. It makes no sense that upstream carries the work of downstream anyway. It is just the wrong direction.
With systemd unit files there was a unified way across distribution to describe the daemon/service and so I started to include these. So unit files etc. can stay upstream because they are common across all distribution. Any init scripts can’t be accepted.
D-Bus activation for system level daemons outside of systemd is something I found always a bit hackish to begin with and thus it is not something that is worth while supporting. iwd is designed to be start and it can linger around until WiFi hardware is found since it is truly hotplug capable.
In summary, if distributions need a sysvinit or openrc integration, then it is the job of the distro package maintainer to provide that.
Regards
Marcel
2 years
Re: build: hook up traditional dbus activation
by Denis Kenzior
Hi Job,
> I understand if you won't support non-systemd init systems like sysvinit. I haven't maintained daemons, but from the looks of it, maintaining support for multiple inits is surely hard. However users who don't use systemd as their init, like Devuan users, Gentoo users, and Debian users who replaced systemd with sysvinit/openrc would appreciate it if upstream decided to support multiple inits. Gentoo users have a community which maintains their own iwd init script for openrc. Devuan users however, have to workaround this problem by converting the iwd systemd service file to a sysvinit script, and update-rc.d as root. If I haven't known about this, I would still be using wpa_supplicant by now. iwd actually made my wifi connection more reliable. When I was using wpa_supplicant, I had to reboot my system everytime the connection drops. With iwd, I only have to restart iwd and my connection is back. I figured that I should contribute back to the community by adding sysvinit support.
I'm still a bit lost why D-Bus activation is needed? Is sysvinit/openrc somehow
using it to start daemons?
Regards,
-Denis
2 years
