-
-
Notifications
You must be signed in to change notification settings - Fork 8.2k
extmod/modussl: improve exception error messages #5819
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
Closed
Closed
Changes from all commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
7d374b2
extmod/modussl: improve exception error messages
tve e6ad6c4
codeformat
tve 2e94be9
fix ussl_basic test
tve cb6fe57
remove unrelated change to err_to_errno
tve 1ead21e
mbedtls: produce shortened error strings
tve a2c669b
address review comments
tve e5f0fea
rework mbedtls error message generation using a diff
8000
tve 90c7b21
remove esp32_mbedtls_errors.c
tve 3a85292
codeformat on tester.c
tve b56866a
remove MICROPY_SSL_MBEDTLS_ERRSTR
tve File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -34,6 +34,7 @@ | |
|
||
#include "py/runtime.h" | ||
#include "py/stream.h" | ||
#include "py/objstr.h" | ||
|
||
// mbedtls_time_t | ||
#include "mbedtls/platform.h" | ||
|
@@ -43,6 +44,7 @@ | |
#include "mbedtls/entropy.h" | ||
#include "mbedtls/ctr_drbg.h" | ||
#include "mbedtls/debug.h" | ||
#include "mbedtls/error.h" | ||
|
||
typedef struct _mp_obj_ssl_socket_t { | ||
mp_obj_base_t base; | ||
|
@@ -74,6 +76,46 @@ STATIC void mbedtls_debug(void *ctx, int level, const char *file, int line, cons | |
} | ||
#endif | ||
|
||
STATIC NORETURN void mbedtls_raise_error(int err) { | ||
// _mbedtls_ssl_send and _mbedtls_ssl_recv (below) turn positive error codes from the | ||
// underlying socket into negative codes to pass them through mbedtls. Here we turn them | ||
// positive again so they get interpreted as the OSError they really are. The | ||
// cut-off of -256 is a bit hacky, sigh. | ||
if (err < 0 && err > -256) { | ||
mp_raise_OSError(-err); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this code path tested? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
} | ||
|
||
#if defined(MBEDTLS_ERROR_C) | ||
// Including mbedtls_strerror takes about 1.5KB due to the error strings. | ||
// MBEDTLS_ERROR_C is the define used by mbedtls to conditionally include mbedtls_strerror. | ||
// It is set/unset in the MBEDTLS_CONFIG_FILE which is defined in the Makefile. | ||
|
||
// Try to allocate memory for the message | ||
#define ERR_STR_MAX 80 // mbedtls_strerror truncates if it doesn't fit | ||
mp_obj_str_t *o_str = m_new_obj_maybe(mp_obj_str_t); | ||
byte *o_str_buf = m_new_maybe(byte, ERR_STR_MAX); | ||
if (o_str == NULL || o_str_buf == NULL) { | ||
mp_raise_OSError(err); | ||
} | ||
|
||
// print the error message into the allocated buffer | ||
mbedtls_strerror(err, (char *)o_str_buf, ERR_STR_MAX); | ||
size_t len = strlen((char *)o_str_buf); | ||
|
||
// Put the exception object together | ||
o_str->base.type = &mp_type_str; | ||
o_str->data = o_str_buf; | ||
o_str->len = len; | ||
o_str->hash = qstr_compute_hash(o_str->data, o_str->len); | ||
// raise | ||
mp_obj_t args[2] = { MP_OBJ_NEW_SMALL_INT(err), MP_OBJ_FROM_PTR(o_str)}; | ||
nlr_raise(mp_obj_exception_make_new(&mp_type_OSError, 2, 0, args)); | ||
#else | ||
// mbedtls is compiled without error strings so we simply return the err number | ||
mp_raise_OSError(err); // err is typically a large negative number | ||
#endif | ||
} | ||
|
||
STATIC int _mbedtls_ssl_send(void *ctx, const byte *buf, size_t len) { | ||
mp_obj_t sock = *(mp_obj_t *)ctx; | ||
|
||
|
@@ -85,7 +127,7 @@ STATIC int _mbedtls_ssl_send(void *ctx, const byte *buf, size_t len) { | |
if (mp_is_nonblocking_error(err)) { | ||
return MBEDTLS_ERR_SSL_WANT_WRITE; | ||
} | ||
return -err; | ||
return -err; // convert an MP_ERRNO to something mbedtls passes through as error | ||
} else { | ||
return out_sz; | ||
} | ||
|
@@ -197,7 +239,6 @@ STATIC mp_obj_ssl_socket_t *socket_new(mp_obj_t sock, struct ssl_args *args) { | |
if (args->do_handshake.u_bool) { | ||
while ((ret = mbedtls_ssl_handshake(&o->ssl)) != 0) { | ||
if (ret != MBEDTLS_ERR_SSL_WANT_READ && ret != MBEDTLS_ERR_SSL_WANT_WRITE) { | ||
printf("mbedtls_ssl_handshake error: -%x\n", -ret); | ||
goto cleanup; | ||
} | ||
} | ||
|
@@ -221,7 +262,7 @@ STATIC mp_obj_ssl_socket_t *socket_new(mp_obj_t sock, struct ssl_args *args) { | |
} else if (ret == MBEDTLS_ERR_X509_BAD_INPUT_DATA) { | ||
mp_raise_ValueError(MP_ERROR_TEXT("invalid cert")); | ||
} else { | ||
mp_raise_OSError(MP_EIO); | ||
mbedtls_raise_error(ret); | ||
} | ||
} | ||
|
||
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,42 @@ | ||
MBEDTLS Error Strings for MicroPython | ||
===================================== | ||
|
||
This directory contains source code and tools to rework the Mbedtls error strings for | ||
micropython to use less space. In short, instead of storing and printing something like | ||
"SSL - Our own certificate(s) is/are too large to send in an SSL message" it prints | ||
the name of the error #define, which would be "MBEDTLS_ERR_SSL_CERTIFICATE_TOO_LARGE" in | ||
this case, and only stores `SSL_CERTIFICATE_TOO_LARGE` in flash. The exact Mbedtls error | ||
defines are used because they're easy to search for to find more detailed information. | ||
|
||
Mbedtls defines a specific format for error value #defines and | ||
includes a Perl script to gather all `MBEDTLS_ERR` defines from includes files together with | ||
english error text. From that the Perl script generates `mbedtls_strerror()`. The files in this | ||
directory modify this process to produce a more space efficient error lookup table with | ||
shorter error strings. | ||
|
||
The files are as follows: | ||
- `generate_errors.diff` - diff for original mbedtls perl script | ||
- `error.fmt` - modified code template for MicroPython | ||
- `mp_mbedtls_errors.c` - source file with `mbedtls_strerror` this is built using the include | ||
files in `../mbedtls` | ||
- `do-mp.sh` - shell script to produce `mp_mbedtls_errors.c` | ||
- `tester.c` - simple C main to test `mp_mbedtls_errors.c` locally on a dev box | ||
- `do-test.sh` - shell script to produce `mp_mbedtls_errors.c` and compile the `tester` app | ||
- `do-esp32.sh` - shell script to produce `esp32_mbedtls_errors.c` -- see below | ||
|
||
In order not to store multiple copies of `mbedtls_errors.c` | ||
([https://github.com/micropython/micropython/pull/5819#discussion_r445528006](see)) | ||
it is assumed that all ports use the same version of mbedtls with the same error #defines. | ||
This is true as of MP v1.13, and ESP-IDF versions 3.3.2 and 4.0.1. If anything changes in the | ||
future the `do-esp32.sh` script can be used to generate an esp32-specific version. | ||
|
||
### How-to | ||
|
||
- To build MicroPython all that is needed is to include the `mp_mbedtls_errors.c` into the build | ||
(the Makefiles do this automatically). Note that Perl is not needed for routine MicroPython | ||
builds. | ||
- When a new version of Mbedtls is pulled-in the `do-mp.sh` script should be run to | ||
re-generate `mp_mbedtls_errors.c`. | ||
- The `tester` app should be run if changes to the string handling in `error.fmt` are made: | ||
it tests that there is not an off-by-one error in the string copying/appending, etc. | ||
- To include `mbedtls_strerror` error strings define `MBEDTLS_ERROR_C` in the build. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
#! /bin/bash -e | ||
# Generate esp32_mbedtls_errors.c for use in the Esp32 port, with the ESP-IDF version of mbedtls | ||
# The IDF_PATH env var must be set to the top-level dir of ESPIDF | ||
echo "IDF_PATH=$IDF_PATH" | ||
MBEDTLS=$IDF_PATH/components/mbedtls/mbedtls | ||
patch -o esp32_generate_errors.pl $MBEDTLS/scripts/generate_errors.pl <generate_errors.diff | ||
perl ./esp32_generate_errors.pl $MBEDTLS/include/mbedtls . esp32_mbedtls_errors.c |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
#! /bin/bash -e | ||
# Generate mp_mbedtls_errors.c for inclusion in ports that use $MPY/lib/mbedtls | ||
patch -o mp_generate_errors.pl ../mbedtls/scripts/generate_errors.pl <generate_errors.diff | ||
perl ./mp_generate_errors.pl ../mbedtls/include/mbedtls . mp_mbedtls_errors.c |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
#! /bin/bash -e | ||
# Generate mp_mbedtls_errors.c and build the tester app | ||
./do-mp.sh | ||
cc -o tester -I../mbedtls/include/ mp_mbedtls_errors.c tester.c |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.