extmod/modussl: improve exception error messages #5819
Conversation
|
Hello, here is the issue: Installing to: /lib/ Thanks to help me. |
The good news is that I can repro, the bad news is that I don't like the result :-) (As I mentioned in the forum thread, 0x71 is 113 which is EHOSTUNREACH) |
|
Hello,
So could you help me please.
All test I have done are here with all the issues
https://forum.micropython.org/viewtopic.php?f=18 <https://forum.micropython.org/viewtopic.php?f=18&t=8048> &t=8048
But in fact I think I have to use pycopy and not micropython. But how to use pycopy with my ide ? really don’understand ?
Thanks to help me please.
Regards.
Julien
De : Thorsten von Eicken <notifications@github.com>
Envoyé : samedi 28 mars 2020 16:38
À : micropython/micropython <micropython@noreply.github.com>
Cc : julien launay <julien.launay@laposte.net>; Comment <comment@noreply.github.com>
Objet : Re: [micropython/micropython] extmod/modussl_mbedtls: add message to exception on handshake error (#5819)
Hello,
I have a ESP32 Wroom 32D.
I try to install picoweb.
I use the last firmware
http://micropython.org/resources/firmware/esp32-idf4-20200328-v1.12-310-g9418611c8.bin
here is the issue:
Installing to: /lib/
Warning: micropython.org SSL certificate is not validated
Installing picoweb 1.8.1 from https://files.pythonhosted.org/packages/ba/e4/68983f6150f7fbd98b6b937bf7f76aeb4eb9ba4abb7c93dc05b2ccdbd25c/picoweb-1.8.1.tar.gz
mbedtls_ssl_handshake error: -71
Error installing 'pycopy-uasyncio': [Errno 5] EIO, packages may be partially installed
Traceback (most recent call last):
File "", line 41, in
File "/lib/picoweb/init.py", line 12, in
ImportError: no module named 'pkg_resources'
Thanks to help me.
regards.
JL
The good news is that I can repro, the bad news is that I don't like the result :-)
MicroPython v1.12-317-g995f79a88 on 2020-03-27; ESP32 module with ESP32
Type "help()" for more information.
>> import upip
>> upip.install('picoweb')
Installing to: /lib/
mbedtls_ssl_handshake error: -113/-0x71
Error installing 'picoweb': mbedtls -0x71: UNKNOWN ERROR CODE (0071), packages may be partially installed
>>
(As I mentioned in the forum thread, 0x71 is 113 which is EHOSTUNREACH)
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub <#5819 (comment)> , or unsubscribe <https://github.com/notifications/unsubscribe-auth/AG5TGE6D2JQHC36BDRL25DLRJYKU7ANCNFSM4LVEW2WQ> .
|
|
Hello,
I found the solution, but with an old version V1.11 of the firmware, le last stable one doesn’t works with an error 71 ?
See my solution here..
https://forum.micropython.org/viewtopic.php?f=18 <https://forum.micropython.org/viewtopic.php?f=18&t=8048&start=10> &t=8048&start=10
Thanks to help me.
Regards.
Julien
|
It turns out that error 113 is not EHOSTUNREACH but ECONNABORTED. This is another case of posix errno.h vs newlib errno.h, see #5830 |
|
After several hours of wild goose chase....:
I can reproduce this with >90% probability using |
|
More wild goose chase... it turns out the real issue is: I suspect that what happens is that memory gets very low and then what fails depends on random stuff. If "all goes well" it's mbedtls that reports a memory error, if things go south then some other random part of esp-idf can't allocate some buffer and then the wifi sta interface is being shut down. Sigh. |
|
I updated the PR to resolve the discrepancy between py/mperrno.h and the errno.h used by lwip. The solution turned out to be relatively simple. Lwip's socket implementation really gets lwip-specific error numbers from its netif layer and has a small table translating those to "posix" error numbers using EXXX constants. What I did is remove the lwip err.c file from the build and replace it with a same file that uses MP_EXXX constants instead. This means that if anything else is still using the EXXX constants they may be off, but at least what comes out of lwip is fixed. |
|
Looks like it doesn't like the test I just fixed-up. I can't tell what fails since it prints no info other than "it failed". When I run it, it works fine: |
f871529 to
2340b8d
Compare
ea84352 to
854756b
Compare
|
I rebased on latest master, however, this needs some slight rework after #5850 goes in so it's compatible. |
43260c6 to
fdda2e0
Compare
|
@dpgeorge please review. |
|
Thanks for this. The main discussion point would be whether adding "user friendly" error messages is in the spirit of MicroPython, trying to be minimal. Giving just the error code (and no stripng) is the most minimal thing to do because then at least you can look up this code to see what it corresponds to. But even if you can lookup the error code in the source that's not necessarily easy to do (eg which version of the code? what configuration options were enabled? what if I'm out in the field and get a connection error and don't have access to source code to look it up?). Also, unlike standard errno codes (like EAGAIN), the TLS codes are many and obscure, so you can't really remember what corresponds to what. And experience has shown that users are often confused about the TLS error messages. Since SSL connections are already hard enough and sometimes unreliable, having obscure error codes without error messages just compounds the problem. So IMO this PR is a worthy addition. But maybe it'd be good to make it configurable with a compile-time option, eg |
| // It is set/unset in the MBEDTLS_CONFIG_FILE which is defined in the Makefile. | ||
| // "small" negative integer error codes come from underlying stream/sockets, not mbedtls | ||
| if (err < 0 && err > -256) { | ||
| mp_raise_OSError(-err); |
| @@ -1,5 +1,4 @@ | |||
| ssl_handshake_status: -256 | |||
| wrap_socket: OSError(5,) | |||
| wrap_socket: OSError(-256, 'CONN_LOST') | |||
There was a problem hiding this comment.
This test obviously only works with axtls, but that's ok for now.
There was a problem hiding this comment.
As written the test can only work with axtls, which is a shame. The problem is that using io.Bytes doesn't work with mbedtls.
Yes, after this patch I'd enable
It might be worth doing this, and also shortening the error messages to be like it's done here for axtls, eg for |
|
Yes, the perl script is not great but I kept it as-is for exactly the reasons you mention and I hope that makes it all easier to maintain. It comes from https://github.com/ARMmbed/mbedtls/tree/development/scripts (I used the version in lib/mbedtls), I' can add a note WRT licensing into the README (it's the mbedtls Apache 2.0 license). |
|
Instead of committing the original and modified files for |
I think the new directory |
|
I see there are changes pushed here. Is it ready for re-review? |
|
Thanks for the reminder! I got stuck re-running tests on my pybd before asking for re-review. |
|
I did finally run the tests and things look OK. The latest changes:
@dpgeorge I would appreciate a re-review. |
|
@dpgeorge all comments but one addressed |
|
@tve thanks for updating, it all looks good now. But, it'll need to be squashed/split into a few commits, like:
I'm happy to do this by reworking your existing commits (so your name as will appear as author of them). Just let me know if I should go ahead, or if you want to do it yourself. |
|
Great! I would appreciate if you did the rework 'cause you know what you want so we don't go around the block again... |
|
Ok, will do. |
|
Reworked in #6259 |
Add Piunora and Zero. Also, enable full build
This PR is a proposal for adding human readable error messages when mbedtls or axtls raise an error. Currently often just an EIO error is raised so the user is lost and can't tell whether it's a cert error, buffer overrun, connecting to a non-ssl port, etc.
Mbedtls: fortunately mbedtls provides a strerror function to return proper error messages. Sadly it's pretty huge: about 16KB on the ESP32. We could also just print the numeric error code, but the problem is that often there's no convenient place to look them up due to the way mbedtls combines high-level and low-level error codes.
This PR modifies the mbedtls error raising to be similar to the check_esp_err stuff, i.e., it produces an
OSError(-err_num, "error string")where the error string comes from mbedtls' strerror, and if there is an out of memory condition it just producesOSError(-err_num).Producing the error string is conditioned on them being included in the mbedtls build, i.e., if they're there anyway then the OSErrors will have them, else they won't.
The esp32 build has the mdbedtls error strings, it's in the mbedtls config which is inherited from esp-idf. The pybd doesn't. IMHO by the time one include 8000 s all the mbedtls code adding the 16kB of strings is a drop in the bucket (even though it's a sucky implementation on their end)...
Just to mention: we could replace the insane sequence of if statements in mbedtls' strerror by a table. That seems like a lot of work.
This PR also fixes the axtls lib to behave the same way. For this I created a small error table. I'm not so familiar with axtls but that seemed like the best option. I tested that on unix but can't test the OOM condition there. I'm trying to get a build for the esp8266 going but so far crosstool-ng isn't building for me (I really don't want the docker mess on my machine...).