Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix WPS regression and cred conversion #379

Merged
merged 6 commits into from
May 16, 2024
Merged

Conversation

torkleyy
Copy link
Contributor

@torkleyy torkleyy commented Mar 5, 2024

Fixes #337
Also fixes the conversion of ssid/password in the config to no longer assume 0 termination - this lead to problems when routers send a 64 byte access token to us instead of a password

@ivmarkov
Copy link
Collaborator

ivmarkov commented Mar 6, 2024

Fixes #337 Also fixes the conversion of ssid/password in the config to no longer assume 0 termination - this lead to problems when routers send a 64 byte access token to us instead of a password

@torkleyy
For my understanding could you elaborate on the access token case you've mentioned?

@torkleyy
Copy link
Contributor Author

torkleyy commented Mar 6, 2024

I'm not quite sure how it works, but some routers will send 64-byte access tokens instead of the passphrase. Until now, the code assumed that ssid / password are always 0-terminated, but it's actually possible for them to use the maximum length of 32/64.

I had no success actually connecting to the Wifi using that token on the esp (it does work on other devices). I ended up setting CONFIG_ESP_WIFI_WPS_PASSPHRASE to receive the passphrase instead.

@ivmarkov
Copy link
Collaborator

ivmarkov commented May 9, 2024

I've delayed merging this for way too long, and now it has a conflict. :(
@torkleyy if you don't have time or will to resolve the conflicts in this PR, I'll understand and will do over the weekend.

(BTW: I've finally checked the (under-documented) ESP IDF Wifi API, and I confirm you are likely correct in the weird behavior w.r.t. SSID (and likely password) in that is is a null-terminated string except when it is full length).

@ivmarkov
Copy link
Collaborator

ivmarkov commented May 9, 2024

BTW, I'm worried that we might have other places to fix: specifically, the reverse conversion of native SSID into heapless::String SSID. But these we can of course fix later.

torkleyy added 6 commits May 16, 2024 14:16
Fixes esp-rs#337
Also fixes the conversion of ssid/password in the config
to no longer assume 0 termination - this lead to problems
when routers send a 64 byte access token to us instead of a password
@torkleyy
Copy link
Contributor Author

@ivmarkov rebased.

@ivmarkov
Copy link
Collaborator

Thank you!

@ivmarkov ivmarkov merged commit 053fb48 into esp-rs:master May 16, 2024
12 checks passed
@torkleyy torkleyy deleted the wps branch May 16, 2024 12:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enabling esp_idf_svc::ws::client, documentation needed
2 participants