[RFC,0/4] Fix extra settings not being taken post-DPP

Message

James Prestwood Dec. 13, 2023, 5:25 p.m. UTC

The current logic in DPP syncs the settings to disk but this doesn't
actually turn an existing network object into a known network, i.e.
sets network->info, which then allows settings to be loaded from
disk via the open() op. Currently a connection can be made (since the
PSK is set into the network object) but any additional settings like
Hidden/SendHostname are not used.

This set is one way we could solve it, though its a bit intrusive
and requires DPP have deeper knowledge that it really should
regarding known networks. It basically forces the creation of a
known network, and I did have to modify __network_info_init to set
the default ops structure, which was a bit nasty. This is the most
direct route without having to worry about callbacks and/or idle
functions.

An alternative way would be to watch the known networks events from
DPP and connect only after an ADDED event. The reason I didn't go
this route was because I wasn't sure about the event ordering. We
need the watch in network.c to be called first (so the network->info
can be created), then connect from DPP. watchlist_add does append to
the tail of the queue so in theory as the above _should_ work so long
as the network watch is added prior to DPP, which is the case since
network's is added in init versus DPP. But I'd hate to put an implied
dependency on watchlist ordering if this changes later. This could
be coupled with an l_idle to avoid the watchlist ordering, but thats
yet another callback.

The scan cancelation patch was thrown in there as well. Since we route
through network_autoconnect/__station_connect_network after DPP there
is no canceling logic and this leads to scans failing after DPP
connects. This doesn't "break" anything but the scans should be
canceled prior to a connection.

James Prestwood (4):
  knownnetworks: set ops on info in __network_info_init
  station: unify scan cancelation
  dpp: fix non-scan connect path in DPP
  auto-t: add a few more DPP tests for seen/known networks

 autotests/testDPP/pkex_test.py | 40 ++++++++++++++
 src/dpp.c                      | 89 +++++++++++++++++++++----------
 src/knownnetworks.c            | 32 ++++++------
 src/station.c                  | 96 +++++++++++++++-------------------
 4 files changed, 162 insertions(+), 95 deletions(-)

Comments

Denis Kenzior Dec. 13, 2023, 10:59 p.m. UTC | #1

Hi James,

On 12/13/23 11:25, James Prestwood wrote:
> The current logic in DPP syncs the settings to disk but this doesn't
> actually turn an existing network object into a known network, i.e.
> sets network->info, which then allows settings to be loaded from
> disk via the open() op. Currently a connection can be made (since the
> PSK is set into the network object) but any additional settings like
> Hidden/SendHostname are not used.

Okay.  So you can solve this in two ways:

1. Have dpp listen to KNOWN_NETWORKS_EVENT_ADDED and start the connection once 
the new network has been detected.
2. Fix match_known_network to take care of merging the new settings with the 
temporary settings obtained from the agent / set using network_set_*.

Advantage of 2 is that you'd be fixing the current race condition that exists 
today, where if the connection is started and a profile is modified while the 
connection is ongoing, the new settings are not taken into account.

> 
> This set is one way we could solve it, though its a bit intrusive
> and requires DPP have deeper knowledge that it really should
> regarding known networks. It basically forces the creation of a
> known network, and I did have to modify __network_info_init to set
> the default ops structure, which was a bit nasty. This is the most
> direct route without having to worry about callbacks and/or idle
> functions.
> 
> An alternative way would be to watch the known networks events from
> DPP and connect only after an ADDED event. The reason I didn't go
> this route was because I wasn't sure about the event ordering. We
> need the watch in network.c to be called first (so the network->info
> can be created), then connect from DPP. watchlist_add does append to

Just use l_idle

> the tail of the queue so in theory as the above _should_ work so long
> as the network watch is added prior to DPP, which is the case since
> network's is added in init versus DPP. But I'd hate to put an implied
> dependency on watchlist ordering if this changes later. This could
> be coupled with an l_idle to avoid the watchlist ordering, but thats
> yet another callback.
> 

Regards,
-Denis