From 9dd9f9ff06c15b06b4b35959196610d5cb952cf0 Mon Sep 17 00:00:00 2001 From: Damien George Date: Thu, 31 Oct 2019 16:22:42 +1100 Subject: [PATCH] extmod/modussl_mbedtls: Check for invalid key/cert data. --- extmod/modussl_mbedtls.c | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/extmod/modussl_mbedtls.c b/extmod/modussl_mbedtls.c index 4105d3fa0..a71adc5b3 100644 --- a/extmod/modussl_mbedtls.c +++ b/extmod/modussl_mbedtls.c @@ -169,21 +169,29 @@ STATIC mp_obj_ssl_socket_t *socket_new(mp_obj_t sock, struct ssl_args *args) { mbedtls_ssl_set_bio(&o->ssl, &o->sock, _mbedtls_ssl_send, _mbedtls_ssl_recv, NULL); - if (args->key.u_obj != MP_OBJ_NULL) { + if (args->key.u_obj != mp_const_none) { size_t key_len; const byte *key = (const byte*)mp_obj_str_get_data(args->key.u_obj, &key_len); // len should include terminating null ret = mbedtls_pk_parse_key(&o->pkey, key, key_len + 1, NULL, 0); - assert(ret == 0); + if (ret != 0) { + ret = MBEDTLS_ERR_PK_BAD_INPUT_DATA; // use general error for all key errors + goto cleanup; + } size_t cert_len; const byte *cert = (const byte*)mp_obj_str_get_data(args->cert.u_obj, &cert_len); // len should include terminating null ret = mbedtls_x509_crt_parse(&o->cert, cert, cert_len + 1); - assert(ret == 0); + if (ret != 0) { + ret = MBEDTLS_ERR_X509_BAD_INPUT_DATA; // use general error for all cert errors + goto cleanup; + } ret = mbedtls_ssl_conf_own_cert(&o->conf, &o->cert, &o->pkey); - assert(ret == 0); + if (ret != 0) { + goto cleanup; + } } if (args->do_handshake.u_bool) { @@ -208,6 +216,10 @@ cleanup: if (ret == MBEDTLS_ERR_SSL_ALLOC_FAILED) { mp_raise_OSError(MP_ENOMEM); + } else if (ret == MBEDTLS_ERR_PK_BAD_INPUT_DATA) { + mp_raise_ValueError("invalid key"); + } else if (ret == MBEDTLS_ERR_X509_BAD_INPUT_DATA) { + mp_raise_ValueError("invalid cert"); } else { mp_raise_OSError(MP_EIO); } @@ -334,8 +346,8 @@ STATIC const mp_obj_type_t ussl_socket_type = { STATIC mp_obj_t mod_ssl_wrap_socket(size_t n_args, const mp_obj_t *pos_args, mp_map_t *kw_args) { // TODO: Implement more args static const mp_arg_t allowed_args[] = { - { MP_QSTR_key, MP_ARG_KW_ONLY | MP_ARG_OBJ, {.u_obj = MP_OBJ_NULL} }, - { MP_QSTR_cert, MP_ARG_KW_ONLY | MP_ARG_OBJ, {.u_obj = MP_OBJ_NULL} }, + { MP_QSTR_key, MP_ARG_KW_ONLY | MP_ARG_OBJ, {.u_rom_obj = MP_ROM_PTR(&mp_const_none_obj)} }, + { MP_QSTR_cert, MP_ARG_KW_ONLY | MP_ARG_OBJ, {.u_rom_obj = MP_ROM_PTR(&mp_const_none_obj)} }, { MP_QSTR_server_side, MP_ARG_KW_ONLY | MP_ARG_BOOL, {.u_bool = false} }, { MP_QSTR_server_hostname, MP_ARG_KW_ONLY | MP_ARG_OBJ, {.u_rom_obj = MP_ROM_PTR(&mp_const_none_obj)} }, { MP_QSTR_do_handshake, MP_ARG_KW_ONLY | MP_ARG_BOOL, {.u_bool = true} },