ports/esp32: added WPA-Enterprise (new) #17234
ports/esp32: added WPA-Enterprise (new) #17234h-milz wants to merge 18 commits intomicropython:masterfrom
Conversation
|
Thank you for updating. There are a few minor code formatting complaints. Mostly trailing spaces and wrong indentation. And a wrong formatted commit message. But that is easy to fix. |
No problem, I'll take care of it tomorrow. |
89f82e8 to
7c1623d
Compare
|
OK The checks now finish successfully and all should be clean. If I should add anything to the documentation just drop me a message. |
|
@h-milz
|
|
@Josverl I forgot to mention that since my first tests in December, my uni apparently implemented EAP-TTLS entirely, so the five methods are all tested and working fine. The only test missing is EAP-TLS which is not supported in our network according to our network admins. As far as FAST, Espressif does support it in ESP-IDF as documented in https://docs.espressif.com/projects/esp-idf/en/stable/esp32/api-reference/network/esp_wifi.html but as far as I'm aware, I have no way of testing it, so I omitted it for now. I think this is a call for other developers to jump in and provide the proper interface. But then, the whole WPA3 enterprise stuff is still missing as well. Basically, WPA3 is supported in eduroam but I don't know if my uni implements it. Let me check, and if yes, I may be able to provide some working code as well. |
|
Yes, please add a documentation. |
I understand. Thing is, I intended to be as little intrusive as possible. Integrating it in connect() would probably require adding another switch, or I can just use eap_method and branch accordingly. But we would have to pack the whole arg parsing into connect() ... |
Thanks - Updated the table accordingly
Perhaps that can be a follow-up PR, having this merged in would be a step forward. |
About integrating this, from a Python perspective I think this can be modelled as an overload MicroPython interface `WLAN.connect()`
class WLAN:
# not sure if these should be classvars as that takes some space , or just documented
AUTH_EAP_PWD = 0
AUTH_EAP_PEAP = 1
AUTH_EAP_TTLS = 2
AUTH_EAP_TLS = 3
TTLS_PWD = 0
TTLS_MSCHAPV2= 1
TTLS_MSCHAP = 2
TTLS_PAP = 3
TTLS_CHAP = 4
@overload
def connect(
self,
ssid: str | None = None,
password: str | None = None,
/,
*,
bssid: bytes | None = None,
) -> None:
"""
Connect to the specified wireless network,
...
"""
...
@overload
def connect(
self,
ssid: str | None = None,
/,
password: str | None = None,
*,
bssid: bytes | None = None,
eap_method : int = AUTH_EAP_PWD , # what is a sensible default
username: str | None = None,
identity: str | None = None,
ca_cert: bytes | None = None, # or server_cert ?
ttls_method: int = TTLS_MSCHAPV2,
client_cert: bytes | None = None,
private_key: bytes | None = None,
private_key_passphrase: str | None = None,
disable_time_check: bool = False,
) -> None:
"""
Connect to the specified wireless network using WPA2
"""
...( Actually it could be a Protocol as the same protocols can be used for wired network access as well. but I think that is of less concern now) |
|
So - what should we do? Leave it as-is and leave it to the user to use a wrapper on top, or go the whole nine yards and modify From a timeline perspective, I'd vote for leaving it as it is right now, as an intermediary step so to speak. Let people use the code, find potential issues and pitfalls, identify things to add like FAST and WPA3, and plan for a unified and extended solution for the next release. I must also say I have a pretty busy semester ahead of me, and potentially little spare time until early August. |
|
In the earlier conversation @dpgeorge commented:
And yes, it is port specific and will not help the STM32 or RP2 guys Right, so this is the main thing to discuss and get right first: how this API will generalise to other WLAN drivers. Should it be a new method like eap_connect(), or should the existing connect() method be reused and new security arguments/options added to it? The latter seems the more obvious path to me. |
|
|
Hi guys, I finally managed to find some time to merge wlan_connect() and eap_connect() as discussed, and I also enable WPA3. Sadly, after extending the eap_connect() function, I get an compiler error which leaves me wondering. I'm attaching the local diff against the last successfully compiling version and the new one, as well as the relevant part of the idf.py build log. ATM I'm stuck. Does it have to do with the number of args? I can't see how and where MP_DEFINE_CONST_OBJ_TYPE_NARGS_2 would be used in this case, and why it would complain. network_wlan.c.errs.txt Thanks for any insight. |
G'day! Take a look at this diff when you get a chance. I haven’t tested Wi-Fi. But it builds fine for ESP32-S3, flashed without issues, and all added attributes are in place. |
|
@mrcreatd thank you for the heads up but your diff lacks my latest changes, merging wifi_connect() and eap_connect() as well as a few wpa3 extensions. My earlier version compiles and works fine as well but my question is what in my latest diff causes the problem. Maybe I don't see the forest for the trees. @robert-hh @Josverl sorry to bother you but can you please look at the issue? Thank you ! |
|
I doubt that I can help. I have no access to a WPA-Enterprise net. |
|
OK, the reason was a missing closing brace m( I have everything up and running and thoroughly tested as listed in Jos' list above, and all the new stuff is merged into connect(). I'll do some cleanups by tomorrow EOB, and if there's any documentation to add just ping me. Other than that, I noticed that this extension dropped from the 1.26 merge list because the old PR was closed. Can you add this one then? |
@dpgeorge , @projectgus , can you please re-add this PR to the 1.26 milestone if that is still feasible . |
|
@h-milz I based it on the master branch and just tried to get the PR and your patch compiling. I'm happy to help with testing this PR going forward if needed. |
e79bff1 to
085a8ab
Compare
|
OK, that seems to work now. I had to reword some older commits that still had "ports/esp32" in the commit messages which made the old code pop up again. |
|
@Josverl, @dpgeorge, @projectgus is there anything missing to add this to the 1.26 merge list? |
projectgus
left a comment
There was a problem hiding this comment.
This feature looks potentially pretty useful to me, and I appreciate your tenacity keeping the PR up to date.
I am curious about the binary size impact? In particular, we have a D2WD variant of ESP32_GENERIC that has a number of features disabled to save code size and particularly IRAM, and I wonder whether this feature might need to support being disabled for this use case.
I'm also going to copy-paste @dpgeorge's comment from the old PR, as I'm not sure about at least one of these points (apologies if I missed a reply on the old PR):
Thanks for the contribution, this is a very nice feature to add.
And yes, it is port specific and will not help the STM32 or RP2 guys
Right, so this is the main thing to discuss and get right first: how this API will generalise to other WLAN drivers.
Should it be a new method like eap_connect(), or should the existing connect() method be reused and new security arguments/options added to it? The latter seems the more obvious path to me.
Then, it would be great to try implementing this on at least one other WLAN driver, to see how general it can be.
Regarding PR management, there are commits here that will need to be squashed before merging. We can help with that, however if you're keen then jimmo has a guide here which may be useful. If you find yourself with a fubar PR branch again then please don't make a third PR, instead you can ping me and we'll work out how to restore it.
| ulp | ||
| usb | ||
| vfs | ||
| wpa_supplicant |
| ============= =========== | ||
|
|
||
|
|
||
|
|
docs/library/network.WLAN.rst
Outdated
| in this case). *wpa3* enforces WPA3 authentication and will reject the | ||
| network if WPA3 is not supported. | ||
|
|
||
| WPA Enterprise (ESP32 port only) |
There was a problem hiding this comment.
As this is a large amount of specific documentation I suggest putting it under its own section at the bottom of this doc and linking to that section from here (i.e. "ESP32 port supports additional arguments for WPA Enterprise support, see (link)").
| mp_raise_ValueError(MP_ERROR_TEXT("unknown config value for eap_method")); | ||
| } | ||
|
|
||
| // this is mandatory and should not be None |
There was a problem hiding this comment.
I don't think this is true, at least for non-Enterprise networks you can set bssid instead...?
docs/library/network.WLAN.rst
Outdated
| access-point with that MAC address (the *ssid* must also be specified | ||
| in this case). | ||
| in this case). *wpa3* enforces WPA3 authentication and will reject the | ||
| network if WPA3 is not supported. |
There was a problem hiding this comment.
We might want to make this more general, i.e. "require minimum security level" to future-proof it, as eventually something will replace WPA3 authentication. A feature to require a minimum auth level on the AP is a useful feature by itself (and could be generalised to other ports).
What about having an argument like min_sec=None that can be set to one of the existing SEC_ constants?
|
@projectgus Thank you for your feedback, and I will definitely look into it. Albeit not before the 1.26 merge / release date because I will have to write two uni exams next week and need some prep time. As for some of the comments:
That's what I did, merge my original eap_connect() into connect(). I would have had to add all the new options to connect() anyway in order to then invoke eap_connect(), so this seemed more straightforward.
Not at all, because it is a wrapper around methods in ESP-IDF. Someone else would have to look into this one at a later date, because I have no other Wifi capable / MPY supported MCU at hand. My take is, let's start with this one and other MCUs can be added later if someone volunteers to do so.
No problem, but again, not before release date. |
Ah, in you attempt to pass the file name to ca_cert, not the binary file contents. Try At least, this works for me [TM]. Sorry, my bad, my documentation was wrong. Corrected it for the upcoming commit. |
|
@projectgus one thing about the documentation. Thinking about it, I tend to add a remark and link to library/network.WLAN pointing at esp32/quickref.html#wlan, where I'd add the extensions that I made. Does this make sense? Otherwise a longer section would end up in network.WLAN. |
|
@h-milz This seems to have solved it! Thank you so much for the support and taking on this feature. |
|
NP! |
|
Code size report: |
6632efe to
482297c
Compare
Signed-off-by: Harald Milz <hm@seneca.muc.de>
Signed-off-by: Harald Milz <hm@seneca.muc.de>
Signed-off-by: Harald Milz <hm@seneca.muc.de>
Signed-off-by: Harald Milz <hm@seneca.muc.de>
Signed-off-by: Harald Milz <hm@seneca.muc.de>
Signed-off-by: Harald Milz <hm@seneca.muc.de>
Signed-off-by: Harald Milz <hm@seneca.muc.de>
Signed-off-by: Harald Milz <hm@seneca.muc.de>
Signed-off-by: Harald Milz <hm@seneca.muc.de>
Signed-off-by: Harald Milz <hm@seneca.muc.de>
482297c to
4a23601
Compare
Signed-off-by: Harald Milz <hm@seneca.muc.de>
Signed-off-by: Harald Milz <hm@seneca.muc.de>
Signed-off-by: Harald Milz <hm@seneca.muc.de>
Signed-off-by: Harald Milz <hm@seneca.muc.de>
Signed-off-by: Harald Milz <hm@seneca.muc.de>
Signed-off-by: Harald Milz <hm@seneca.muc.de>
Signed-off-by: Harald Milz <hm@seneca.muc.de>
Signed-off-by: Harald Milz <hm@seneca.muc.de>
4a23601 to
99be7e6
Compare
Summary
This PR supersedes #16463 which is FUBAR.
This PR adds WPA2 Enterprise support to the ESP32 port. In particular, the patch supports EAP-PWD, EAP-PEAP, and EAP-TTLS (all supported ttls phase2 methods). Code for EAP-TLS is also included but UNTESTED and EXPERIMENTAL. The patch is a thin wrapper around the ESP-IDF functions and does not implement any further network or security relevant programming. Consequently, it is specific to the ESP32 port.
Testing
The patch was developed and tested in the Technical University of Munich
eduroamnetwork on an ESP32_GENERIC_S3 board, namely, a CrowPanel 5.0"-HMI ESP32 Display board with a ESP32-S3-WROOM-1-N4R8 module, as well as a generic ESP32-C6 board from DFRobot, on MPY v1.23.0 using ESP-IDF 5.22 and on MPY v1.25.0 using ESP-IDF 5.4.0.Usage example:
Trade-offs and Alternatives
If your board does not have a hardware RTC, odds are that the server certificate validation for EAP-PEAP, -TTLS and potentially -TLS will fail due to the system time being way off. As a workaround, you can set the system time to build time on system start like this:
and from then on, synchronize the internal RTC using NTP in regular intervals.
More documentation is contained in
ports/esp32/README.md.