8000 extmod/modussl: improve exception error messages by tve · Pull Request #5819 · micropython/micropython · GitHub
[go: up one dir, main page]

Skip to content

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
wants to merge 10 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 52 additions & 3 deletions extmod/modussl_axtls.c
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@

#include "py/runtime.h"
#include "py/stream.h"
#include "py/objstr.h"

#if MICROPY_PY_USSL && MICROPY_SSL_AXTLS

Expand All @@ -54,6 +55,56 @@ struct ssl_args {

STATIC const mp_obj_type_t ussl_socket_type;

// Table of errors
struct ssl_errs {
int16_t errnum;
const char *errstr;
};
STATIC const struct ssl_errs ssl_error_tab[] = {
{ SSL_NOT_OK, "NOT_OK" },
{ SSL_ERROR_DEAD, "DEAD" },
{ SSL_CLOSE_NOTIFY, "CLOSE_NOTIFY" },
{ SSL_EAGAIN, "EAGAIN" },
{ SSL_ERROR_CONN_LOST, "CONN_LOST" },
{ SSL_ERROR_RECORD_OVERFLOW, "RECORD_OVERFLOW" },
{ SSL_ERROR_SOCK_SETUP_FAILURE, "SOCK_SETUP_FAILURE" },
{ SSL_ERROR_INVALID_HANDSHAKE, "INVALID_HANDSHAKE" },
{ SSL_ERROR_INVALID_PROT_MSG, "INVALID_PROT_MSG" },
{ SSL_ERROR_INVALID_HMAC, "INVALID_HMAC" },
{ SSL_ERROR_INVALID_VERSION, "INVALID_VERSION" },
{ SSL_ERROR_UNSUPPORTED_EXTENSION, "UNSUPPORTED_EXTENSION" },
{ SSL_ERROR_INVALID_SESSION, "INVALID_SESSION" },
{ SSL_ERROR_NO_CIPHER, "NO_CIPHER" },
{ SSL_ERROR_INVALID_CERT_HASH_ALG, "INVALID_CERT_HASH_ALG" },
{ SSL_ERROR_BAD_CERTIFICATE, "BAD_CERTIFICATE" },
{ SSL_ERROR_INVALID_KEY, "INVALID_KEY" },
{ SSL_ERROR_FINISHED_INVALID, "FINISHED_INVALID" },
{ SSL_ERROR_NO_CERT_DEFINED, "NO_CERT_DEFINED" },
{ SSL_ERROR_NO_CLIENT_RENOG, "NO_CLIENT_RENOG" },
{ SSL_ERROR_NOT_SUPPORTED, "NOT_SUPPORTED" },
};

STATIC NORETURN void ussl_raise_error(int err) {
for (size_t i = 0; i < MP_ARRAY_SIZE(ssl_error_tab); i++) {
if (ssl_error_tab[i].errnum == err) {
// construct string object
mp_obj_str_t *o_str = m_new_obj_maybe(mp_obj_str_t);
if (o_str == NULL) {
break;
}
o_str->base.type = &mp_type_str;
o_str->data = (const byte *)ssl_error_tab[i].errstr;
o_str->len = strlen((char *)o_str->data);
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));
}
}
mp_raise_OSError(err);
}


STATIC mp_obj_ssl_socket_t *ussl_socket_new(mp_obj_t sock, struct ssl_args *args) {
#if MICROPY_PY_USSL_FINALISER
mp_obj_ssl_socket_t *o = m_new_obj_with_finaliser(mp_obj_ssl_socket_t);
Expand Down Expand Up @@ -107,9 +158,7 @@ STATIC mp_obj_ssl_socket_t *ussl_socket_new(mp_obj_t sock, struct ssl_args *args
int res = ssl_handshake_status(o->ssl_sock);

if (res != SSL_OK) {
printf("ssl_handshake_status: %d\n", res);
ssl_display_error(res);
mp_raise_OSError(MP_EIO);
ussl_raise_error(res);
}
}

Expand Down
47 changes: 44 additions & 3 deletions extmod/modussl_mbedtls.c
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@

#include "py/runtime.h"
#include "py/stream.h"
#include "py/objstr.h"

// mbedtls_time_t
#include "mbedtls/platform.h"
Expand All @@ -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;
Expand Down Expand Up @@ -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);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this code path tested?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No. I just tried to write a test and failed to come up with something. It will be tested as part of #5825, however. What's missing is that at the current point read/write don't go through the SSL error string stuff. #5825 fixes that and 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;

Expand All @@ -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;
}
Expand Down Expand Up @@ -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;
}
}
Expand All @@ -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);
}
}

Expand Down
17 changes: 17 additions & 0 deletions lib/libc/string0.c
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,23 @@ char *strcpy(char *dest, const char *src) {
return dest;
}

// Public Domain implementation of strncpy from:
// http://en.wikibooks.org/wiki/C_Programming/Strings#The_strncpy_function
char *strncpy(char *s1, const char *s2, size_t n) {
char *dst = s1;
const char *src = s2;
/* Copy bytes, one at a time. */
while (n > 0) {
n--;
if ((*dst++ = *src++) == '\0') {
/* If we get here, we found a null character at the end of s2 */
*dst = '\0';
break;
}
}
return s1;
}

// needed because gcc optimises strcpy + strcat to this
char *stpcpy(char *dest, const char *src) {
while (*src) {
Expand Down
42 changes: 42 additions & 0 deletions lib/mbedtls_errors/README.md
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.
7 changes: 7 additions & 0 deletions lib/mbedtls_errors/do-esp32.sh
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
4 changes: 4 additions & 0 deletions lib/mbedtls_errors/do-mp.sh
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
4 changes: 4 additions & 0 deletions lib/mbedtls_errors/do-test.sh
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
Loading
0