search

reedtaylor / plucky

ESP32 Decent Espresso bridge
10 stars 2 forks source link

maybe migrate to AutoConnect instead of IotWebConf #16

Open reedtaylor opened 4 years ago

reedtaylor commented 4 years ago

https://hieromon.github.io/AutoConnect/

though also also should look into the asyncwebserver equivalent

lddubeau commented 4 years ago

IotWebConf definitely needs to go.

Issues I immediately ran into:

  1. The WiFi password is limited to 33 characters. The standard supports up to 63 characters for a password. I use 63 characters on my network, therefore I cannot use Plucky. The issue has been reported but there's been no action. (If PSK are also meant to be supported then 64 characters are needed. As far as the standard is concerned, 64 characters means you have a PSK, 63 or less is a password that is converted to PSK.)

  2. The AP password is also limited to 33 characters. But the GUI does not mention this. Users should not have to read the source code to discover this information. Not only should the GUI make this crystal clear but it should also be designed to try to detect if a user pastes a password that is too large into the field. (Definitely doable because I've seen it on some web registration forms, but I've not investigated how it is done, specifically.)

  3. The documentation for IotWebConf states:

NOTE: When connecting through a WiFi router (WiFi mode), your communication is not hidden from devices connecting to the same network. So either: Do not allow ambiguous devices connecting to your WiFi router, or configure your Thing only in AP mode!

This is a doozie. People using Plucky need to know this because that's a security hole. Or, better, the library needs to be changed to one that does not compromise security.

I cannot recommend a library, and I have no opinion regarding AutoConnect. Back when I was working on my esp32 project, I pretty much developed custom logic for doing what IotWebConf does. (Unfortunately, it is not in a useful shareable state. And because that project was not meant as a general solution, I may have cut corners that should not be cut in a project meant for a more general audience. Though I definitely supported 63 characters for WiFi passwords. I remember looking it up to make sure.)

reedtaylor commented 4 years ago

Thanks for highlighting these issues and the ramifications. This helps with prioritization.

Short-term, I could easily add commands accessible via USB that would allow users to poke in values for config parameters. This would (a) require physical access, (b) would sidestep the config portal string-length limitations for SSID and wpa2, and (c) could obviate the need for webconfig in general. At least for those who disprefer it.

In the current state, doing the above would enable users to more securely establish a network-attached DE1... But then that thing would have exposed, unsecured serial interface ports on TCP 9090. That wouldn't bother me personally, but I can see how others might balk at "a more secure way to get into a readily compromised endstate"....

Thinking about that, I was already planning to enable websockets as an alternative or possible replacement to TCP ( #10 ). I haven't looked closely at this element of asyncwebserver but I'm hoping that it opens the door to TLS as a primary way to manage security across the board.