mac80211 drivers seem to send the disconnect event which is triggered by
CMD_DISCONNECT prior to the CMD_DISCONNECT response. However, some
drivers, namely brcmfmac, send the response first and then send the
disconnect event. This confused iwd when a connection was immediately
triggered after a disconnection (network switch operation).
Fix this by making sure that connected variable isn't set until the
connect event is actually processed, and ignore disconnect events which
come after CMD_DISCONNECT has alredy succeeded.
Do agent registration as part of agent manager proxy creation.
This ensures that the registration call is made only after the agent
manager’s interface becomes available on the bus.
Add the newly created proxy objects into the queue before the
interface specific initialization logic takes place. This way the new
proxy objects can be used within the initialization procedures.
For nl80211 sockets other than our main l_genl object use socket io
directly, to avoid creating many instances of l_genl. The only reason
we use multiple sockets is to work around an nl80211 design quirk that
requires closing the socket to unregister management frame watches.
Normally there should not be a need to create multiple sockets in a
program.
Add a little state machine and a related API, to simplify sending out a
frame, receiving the Ack / No-ack status and (if acked) waiting for a
response frame from the target device, one of a list of possible
frame prefixes. The nl80211 API for this makes it complicated
enough that this new API seems to be justified, on top of that there's a
quirk when using the brcmfmac driver where the nl80211 response
(containing the operation's cookie), the Tx Status event and the response
Frame event are received from nl80211 in reverse order (not seen with
other drivers so far), further complicating what should be a pretty
simple task.
Try to better deduplicate the frame watches. Until now we'd check if
we'd already registered a given frame body prefix with the kernel, or a
matching more general prefix (shorter). Now also try to check if we
have already have a watch with the same callback pointer and user_data
value, and:
* an identical or shorter (more general) prefix, in that case ignore
the new watch completely.
* a longer (more specific) prefix, in that case forget the existing
watch.
The use case for this is when we have a single callback for multiple
watches and multiple frame types, and inside that callback we're looking
at the frame body again and matching it to frame types. In that case
we don't want that function to be called multiple times for one frame
event.
In frame_watch_group_remove I forgot to actually match the group to be
removed by both wdev_id and group_id. group_ids are unique only in the
scope of one wdev.
I forgot to actually add new groups being created in
frame_watch_group_get to the watch_groups queue, meaning that we'd
re-create the group every time a new watch was added to the group.
Previously, the parsing of the OMs objects has been done in one pass,
therefore, the proxy object's dependencies may not have been parsed at the
time when they were looked up for the dependency assignments. Now, the
parsing of the OM objects is done in two passes: 1) Create proxy objects -
one per interface and path, 2) Populate the proxy objects with properties
and assign dependencies. Therefore, we are guaranteed to have the proxy
objects created by the time they are looked up for the dependency
assignments.
Processing the duplicated TLVs while connecting to a malicious AP may lead
to overflow of the response buffer. This patch ensures that the
duplicated TLVs are not parsed.
The pending wiphy state 'use_default' variable was not set early enough
in some circumstances resulting in weird behavior for blacklisted
drivers. Fix this by adding a manager_wiphy_dump_done callback which
will properly initialize the use_default value.
Fixes: c4b2f10483 ("manager: Handle missing NEW_WIPHY events")
brcmfmac does not allow the removal of the default / primary interface.
So there isn't much point in having iwd attempt this.
Another issue is that brcmfmac _does_ allow the deletion of non-default
interfaces. So starting iwd on a system with a station & ap interface
active can result in iwd attempting to delete all the interfaces. Given
the above, it succeeds in deleting the ap interface but not the station
one. In strange circumstances it might end up thinking that the ap
interface is the 'default' and trying to use it, whereas it was just
successfully removed.
==192== Conditional jump or move depends on uninitialised value(s)
==192== at 0x4531D3: l_queue_find (queue.c:346)
==192== by 0x42F1F8: manager_config_notify (manager.c:667)
==192== by 0x45A895: process_multicast (genl.c:970)
==192== by 0x45A895: received_data (genl.c:1037)
==192== by 0x4577B2: io_callback (io.c:126)
==192== by 0x456B0D: l_main_iterate (main.c:473)
==192== by 0x456BCB: l_main_run (main.c:520)
==192== by 0x456DDA: l_main_run_with_signal (main.c:642)
==192== by 0x4034B0: main (main.c:497)
The kernel emits NEW_WIPHY events whenever a new wiphy is registered.
Unfortunately these events are emitted under the 'legacy' semantics and
have a hard size limit of 4096 bytes. Unfortunately, it is possible for
a NEW_WIPHY message to exceed this limit (ath10k cards seem to be
affected in particular), which results in the kernel never sending these
messages out. This can lead to NEW_INTERFACE events being emitted with
a wiphy_id that had no corresponding NEW_WIPHY event emitted. Such a
sequence can confuse iwd's hardware detection logic, particularly during
hot-plug or system boot.
Fix this by re-dumping the wiphy if such a condition is detected. This
has some interaction with blacklisted wiphys, so the wiphy objects are
now always tracked and marked as blacklisted. Before, the blacklisted
wiphys were simply not added to the iwd list of tracked wiphys.
For the inner EAP methods that support generation of the key material
include it into imck generation. This allows to cryptographically
bind the inner method with the tunnel.
Windows Server 2008 - Network Policy Server (NPS) generates an invalid
Compound MAC for Cryptobinding TLV when is used within PEAPv0 due to
incorrect parsing of the message containing TLS Client Hello.
Setting L bit and including TLS Message Length field, even for the
packets that do not require fragmentation, corrects the issue. The
redundant TLS Message Length field in unfragmented packets doesn't
seem to affect the other server implementations.
Sometimes, at least with brcmfmac, the default interface apparently
takes a moment to get created after the NEW_WIPHY event. We didn't
really consider this case in the NEW_WIPHY handler and we've got a race
condition. It fixes the following bug for me:
https://bugs.archlinux.org/task/63912 -- tested by removing and
re-modprobing the brcmfmac module rather than rebooting.
To work around this wait for the NEW_INTERFACE event and then retry the
setup. We still do the initial attempt directly after NEW_WIPHY to
handle cases like wiphys with no default interfaces and pre-existing
wiphys.
We track mtime as the 'LastConnectedTime' of the network, and also sort
the known network list according to the last connected time.
Unfortunately we were never reacting to ATTRIB changes, and so were
never updating the network_info->connected_time whenever a network was
connected to.
Rework the logic to address this. This also fixes a small bug where the
connected_time was not set properly prior to removal / re-insertion of
the network_info.
These arrays should have been declared extern in the first place.
Newer versions of gcc now complain about this:
/usr/lib/gcc/x86_64-pc-linux-gnu/9.2.0/../../../../x86_64-pc-linux-gnu/bin/ld: client/dbus-proxy.o:(.bss+0x0): multiple definition of `properties_yes_no_opts'; client/adapter.o:(.bss+0x0): first defined here
/usr/lib/gcc/x86_64-pc-linux-gnu/9.2.0/../../../../x86_64-pc-linux-gnu/bin/ld: client/dbus-proxy.o:(.bss+0x20): multiple definition of `properties_on_off_opts'; client/adapter.o:(.bss+0x20): first defined here
/usr/lib/gcc/x86_64-pc-linux-gnu/9.2.0/../../../../x86_64-pc-linux-gnu/bin/ld: client/device.o:(.bss+0x20): multiple definition of `properties_on_off_opts'; client/adapter.o:(.bss+0x20): first defined here
/usr/lib/gcc/x86_64-pc-linux-gnu/9.2.0/../../../../x86_64-pc-linux-gnu/bin/ld: client/device.o:(.bss+0x0): multiple definition of `properties_yes_no_opts'; client/adapter.o:(.bss+0x0): first defined here
/usr/lib/gcc/x86_64-pc-linux-gnu/9.2.0/../../../../x86_64-pc-linux-gnu/bin/ld: client/known-networks.o:(.bss+0x0): multiple definition of `properties_yes_no_opts'; client/adapter.o:(.bss+0x0): first defined here
/usr/lib/gcc/x86_64-pc-linux-gnu/9.2.0/../../../../x86_64-pc-linux-gnu/bin/ld: client/known-networks.o:(.bss+0x20): multiple definition of `properties_on_off_opts'; client/adapter.o:(.bss+0x20): first defined here
/usr/lib/gcc/x86_64-pc-linux-gnu/9.2.0/../../../../x86_64-pc-linux-gnu/bin/ld: client/properties.o:(.data.rel.local+0x0): multiple definition of `properties_yes_no_opts'; client/adapter.o:(.bss+0x0): first defined here
/usr/lib/gcc/x86_64-pc-linux-gnu/9.2.0/../../../../x86_64-pc-linux-gnu/bin/ld: client/properties.o:(.data.rel.local+0x20): multiple definition of `properties_on_off_opts'; client/adapter.o:
NLMSG_OK and NLMSG_NEXT expect to operate on nlmsg_len which is an int
(signed type). The current code uses an unsigned type which means that
it cannot detect underflows. Such underflows can happen when NLMSG_NEXT
tries to advance nlmsg_len by a number of bytes (due to alignment) which
are greater than the current nlmsg_len itself. This causes iwmon to
crash on certain messages.
Reported-By: Daniel Wagner <wagi@monom.org>
We use the mtime on the network profile as the 'Last Connected Time'.
When we update any property and sync the file to disk, the mtime was not
preserved (since we were creating a new temporary file instead of
modifying the old one). This led to LastConnectedTime property change
being emitted / updated incorrectly when a writable property on the
KnownNetwork interface was updated.
Our design preference is to not call any callbacks in the _free/_destroy
method of a class (with the exception of explicit destroy callbacks
provided, if any).
Invoking the callback in this case was unnecessary: wsc_dbus_free was
already replying to pending connect / cancel messages. The only other
thing the callback would attempt to do is to set station back into
autoconnect mode. This was unnecessary as well since the netdev is
already down.
This change removes the callback invocation. Since wsc_enrollee_destroy
is now just calling wsc_enrollee_free, remove this from the API and
expose wsc_enrollee_free instead.
Split the WSC D-Bus interface class (struct wsc) into a base class
common to station mode and P2P mode (struct wsc_dbus) and station-
specific logic like scanning, saving the credentials as a known network
and triggering the station-mode connection (struct wsc_station_dbus).
Make the base class and its utilities public in wsc.h for P2P use.
Create struct wsc_enrollee which is allocated with wsc_enrollee_new,
taking a done callback as a parameter. The callback is always
called so there's no need for a separate destroy callback. The object
only lives until the done callback happens so wsc_enrollee_cancel/destroy
can only be used before this.
Looks like the rest of the file is simplified thanks to this.