Wps pbc

[labno]

Pull request for issue #49

[fhunleth]

Great! I'm really happy to see that you were able to make progress with this feature!

I'm going to have to dig into the spec to understand the WPS response. I would like to do better, but I might change my mind and agree with your decision after I get a chance to look at it.

[fhunleth]

@labno I just merged a CI change into the main branch to work around the Circle CI error on the OTP24 build for now. Could you rebase?

I saw that CI was failing since WPS processing was now globally on for all WiFi configurations. I'm not sure what the implications are if it's on. Unless you know for sure that it's harmless (no security implications or anything like that), I'm thinking that it should be an option that has to be turned on. Then the first thing that the quick_wps function would do is call VintageNet.configure with a configuration that enables wps_cred_processing=1.

[labno]

Not setting wps_cred_processing=1 does not mean incoming WPS_CREDENTIALS are ignored, it means they are used by wpa_supplicant directly to connect to the wifi. That means, if I call VintageNet.ioctl("wlan0", :wps_pbc) without setting wps_cred_processing=1, the received creds are used to connect to the wifi, but without persisting to a wifi config file. This could be confusing.

On the other hand, setting the config in quick_wps would have the benefit of ensuring the system is ready to receive WPS credentials, as it turns out to get a little wonky when it's in AP mode (e.g. because of VintageNetWizard).

So if you're not bothered by the caveat above, I have the commit ready.

[fhunleth]

Thanks. That's an interesting caveat. I like your decision then on always specifying wps_cred_processing=1. Could you update the other tests so that they pass? And add a short one line comment where the wps_cred_processing=1 line is added to say that we always to handle WPS.

I also like forcing the WiFi configuration in quick_wps so that it's in a known configuration. That seems like a good way to avoid some confusing support requests given what you mentioned about AP mode.

[labno]

Ok, so far so good, thank you for your feedback! I had to put in a small pause between forcing the config and sending the wps_pbc ioctl signal, because sometimes the wlan0 interface process doesn't seem to come up again fast enough and I get:

** (exit) exited in: GenServer.call({:via, Registry, {VintageNet.Interface.Registry, {VintageNetWiFi.WPASupplicant, "wlan0"}}}, :wps_pbc, 5000)
    ** (EXIT) no process: the process is not alive or there's no process currently associated with the given name, possibly because its application isn't started
    (elixir 1.11.3) lib/gen_server.ex:1017: GenServer.call/3
    (vintage_net_wifi 0.10.2) lib/vintage_net_wifi.ex:871: VintageNetWiFi.quick_wps/1

Let me know if there's a better way to solve this.

I'll look after the tests in the coming days.

[fhunleth]

Yeah, that pause is fine.

[fhunleth]

I looked at the WPS spec and decoding the credentials doesn't look that bad. However, I tried it on the unit test that you just posted and it didn't work. I'm really, really hoping that you modified the hex string manually.

Here's a start of a decoder: https://github.com/nerves-networking/vintage_net_wifi/compare/wps_data

The WPS data is a tag,length,value string. The first TLV in the string you posted is called "credential" in the spec and the value is WPS data. When I decoded that, I got a SSID and passphrase (called "network key" in the spec) and several other values that I didn't implement. The returned map currently looks like this:

             %{
                credential: %{
                  :network_key => "efg0956957\x10 \0\x06\xB8'",
                  :ssid => "WLAN-AE9456",
                  4099 => <<0, 32>>,
                  4111 => <<0, 8>>,
                  4134 => <<1>>
                }
              }

You can see that the ssid is right and the network_key is really close. I put undecoded tag/value pairs in the map too. I haven't looked yet whether they're interesting.

The spec is at https://www.wi-fi.org/downloads-registered-guest/Wi-Fi_Protected_Setup_Specification_v2.0.8.pdf/35517. See section 12.

I think we only implement what's needed for this use case since there are a whole lot of element types. It's fine with me if we just return numerical keys and raw value binaries for anything that's not properly decoded.

[labno]

Oh, I did alter the key manually, since it's my home wifi. I'm sorry, I hope my paranoia didn't cause too much trouble. Thank you for finding that! Also explains why googling didn't yield much, since the specs are hidden behind a membership form for some reason... I agree, the remaining key value pairs shouldn't be of interest to our use case.

[fhunleth]

I squashed all of the commits together, fixed the minor dialyzer/formatter issues, switched :os.system_time to System.monotonic_time, and merged to main.

Thank you for implementing WPS support! I will plan on making a new vintage_net_wifi release over the next couple days.

See 5fe044166db154b50531c6768f2d435886c4fe32 for squashed/merged commit.

[labno]

Thank you so much for all your help! This has been a pleasure :)