warmcat / lws-esp32-test-server-demos

Libwebsockets test demos running on ESP32 OTA APP
Other
33 stars 10 forks source link

nvs basic auth accepts any user and pwd #11

Closed FredrikFornstad closed 6 years ago

FredrikFornstad commented 6 years ago

Something is missing in the new nvs basic auth implementation. I do not have my ordinary Environment up at the moment so I have difficulties to trouble shoot.

It seems that the input-checking against the stored values does not work. I can log in giving any username and/or password...

Potentially I guess I could be the one to blame even though I do not think so: I noticed a weakness in the init of the lwsdemoba in main.c program. I modified the code to what I belive is a better and more robust implementation ("commit" was missing and the Close statement was only executed the first time the code ran). With the code below those have been corrected I hope:

if (nvs_open("lwsdemoba", NVS_READWRITE, &nvh) != ESP_OK) { nvs_set_str(nvh, "Username", "Password"); nvs_commit(nvh); } nvs_close(nvh);

lws-team commented 6 years ago

Thanks... I fixed that and otther lws breakage.

FredrikFornstad commented 6 years ago

Tested the basic auth fix and it seems to work ok. Thanks!

Comment on "my code suggestion" on the nvs_open statement (for any potential reader who read this thread): I had the wrong assumption that nvs_open would not return an ESP_OK if the lwsdemoba never had been written before. This is not the case, so do not copy my code above.

If one would like to reduce flash write wear, I guess (me guessing again...) one could encapsulate the nvs_set and nvs_commit message with an additional "if(nvs_get_str(...) == ESP_ERR_NVS_NOT_FOUND) as the current progam will write to flash at every boot, even if it is not needed. I guess this can be interesting for applications which are expected to reboot the esp32 frequently.

lws-team commented 6 years ago

Glad it's working.

Normally you'd only write the credential when you changed it, not every boot; the idea is normally the enduser selected the username / pw and you can't learn the credential by dumping the firmware. Because it's in nvs, it's also fine if the code setting the credential is in -factory or whatever.

Since the test demo is just a demo it just wants to show how to set the credential and how to make mounts require it in a standalone way.