From 8ef24afe7a2274c14b6fa9032af52033ea5d478a Mon Sep 17 00:00:00 2001 From: Tim Hawes <me@timhawes.com> Date: Sat, 27 Nov 2021 12:43:29 +0000 Subject: [PATCH 1/5] Documentation fix for SSLContext.wrap_socket --- shared-bindings/ssl/SSLContext.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/shared-bindings/ssl/SSLContext.c b/shared-bindings/ssl/SSLContext.c index 885a156eddc6..c41dfa00195d 100644 --- a/shared-bindings/ssl/SSLContext.c +++ b/shared-bindings/ssl/SSLContext.c @@ -51,10 +51,9 @@ STATIC mp_obj_t ssl_sslcontext_make_new(const mp_obj_type_t *type, size_t n_args return MP_OBJ_FROM_PTR(s); } -//| def wrap_socket(sock: socketpool.Socket, *, server_side: bool = False, server_hostname: Optional[str] = None) -> ssl.SSLSocket: -//| """Wraps the socket into a socket-compatible class that handles SSL negotiation. -//| The socket must be of type SOCK_STREAM.""" -//| ... +//| def wrap_socket(self, sock: socketpool.Socket, *, server_side: bool = False, server_hostname: Optional[str] = None) -> ssl.SSLSocket: +//| """Wraps the socket into a socket-compatible class that handles SSL negotiation. +//| The socket must be of type SOCK_STREAM.""" //| STATIC mp_obj_t ssl_sslcontext_wrap_socket(size_t n_args, const mp_obj_t *pos_args, mp_map_t *kw_args) { From bcb516c49626ead2fd20a4df5465701f5a490d36 Mon Sep 17 00:00:00 2001 From: Tim Hawes <me@timhawes.com> Date: Sat, 27 Nov 2021 12:45:25 +0000 Subject: [PATCH 2/5] Handle server_hostname=None in SSLContext.wrap_socket --- shared-bindings/ssl/SSLContext.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/shared-bindings/ssl/SSLContext.c b/shared-bindings/ssl/SSLContext.c index c41dfa00195d..839715f4eccf 100644 --- a/shared-bindings/ssl/SSLContext.c +++ b/shared-bindings/ssl/SSLContext.c @@ -68,7 +68,10 @@ STATIC mp_obj_t ssl_sslcontext_wrap_socket(size_t n_args, const mp_obj_t *pos_ar mp_arg_val_t args[MP_ARRAY_SIZE(allowed_args)]; mp_arg_parse_all(n_args - 1, pos_args + 1, kw_args, MP_ARRAY_SIZE(allowed_args), allowed_args, args); - const char *server_hostname = mp_obj_str_get_str(args[ARG_server_hostname].u_obj); + const char *server_hostname = NULL; + if (args[ARG_server_hostname].u_obj != mp_const_none) { + server_hostname = mp_obj_str_get_str(args[ARG_server_hostname].u_obj); + } bool server_side = args[ARG_server_side].u_bool; if (server_side && server_hostname != NULL) { mp_raise_ValueError(translate("Server side context cannot have hostname")); From ef414bf1bd112f4f062cad914199826a67b1e0b5 Mon Sep 17 00:00:00 2001 From: Tim Hawes <me@timhawes.com> Date: Mon, 13 Dec 2021 17:11:31 +0000 Subject: [PATCH 3/5] Handle server_hostname argument in espressif SSLContext.wrap_socket --- ports/espressif/common-hal/ssl/SSLContext.c | 6 +++++- ports/espressif/common-hal/ssl/SSLSocket.c | 4 +--- ports/espressif/common-hal/ssl/SSLSocket.h | 1 + 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/ports/espressif/common-hal/ssl/SSLContext.c b/ports/espressif/common-hal/ssl/SSLContext.c index a94c1df1eb24..e4767716a26d 100644 --- a/ports/espressif/common-hal/ssl/SSLContext.c +++ b/ports/espressif/common-hal/ssl/SSLContext.c @@ -47,6 +47,11 @@ ssl_sslsocket_obj_t *common_hal_ssl_sslcontext_wrap_socket(ssl_sslcontext_obj_t sock->ssl_context = self; sock->sock = socket; + // Create a copy of the ESP-TLS config object and store the server hostname + // Note that ESP-TLS will use common_name for both SNI and verification + memcpy(&sock->ssl_config, &self->ssl_config, sizeof(self->ssl_config)); + sock->ssl_config.common_name = server_hostname; + esp_tls_t *tls_handle = esp_tls_init(); if (tls_handle == NULL) { mp_raise_espidf_MemoryError(); @@ -55,6 +60,5 @@ ssl_sslsocket_obj_t *common_hal_ssl_sslcontext_wrap_socket(ssl_sslcontext_obj_t // TODO: do something with the original socket? Don't call a close on the internal LWIP. - // Should we store server hostname on the socket in case connect is called with an ip? return sock; } diff --git a/ports/espressif/common-hal/ssl/SSLSocket.c b/ports/espressif/common-hal/ssl/SSLSocket.c index b1d0720be1e1..281e356d77f3 100644 --- a/ports/espressif/common-hal/ssl/SSLSocket.c +++ b/ports/espressif/common-hal/ssl/SSLSocket.c @@ -55,9 +55,7 @@ void common_hal_ssl_sslsocket_close(ssl_sslsocket_obj_t *self) { void common_hal_ssl_sslsocket_connect(ssl_sslsocket_obj_t *self, const char *host, size_t hostlen, uint32_t port) { - esp_tls_cfg_t *tls_config = NULL; - tls_config = &self->ssl_context->ssl_config; - int result = esp_tls_conn_new_sync(host, hostlen, port, tls_config, self->tls); + int result = esp_tls_conn_new_sync(host, hostlen, port, &self->ssl_config, self->tls); self->sock->connected = result >= 0; if (result < 0) { int esp_tls_code; diff --git a/ports/espressif/common-hal/ssl/SSLSocket.h b/ports/espressif/common-hal/ssl/SSLSocket.h index dd1dcda4acb1..097f19857bb0 100644 --- a/ports/espressif/common-hal/ssl/SSLSocket.h +++ b/ports/espressif/common-hal/ssl/SSLSocket.h @@ -39,6 +39,7 @@ typedef struct { socketpool_socket_obj_t *sock; esp_tls_t *tls; ssl_sslcontext_obj_t *ssl_context; + esp_tls_cfg_t ssl_config; } ssl_sslsocket_obj_t; #endif // MICROPY_INCLUDED_ESPRESSIF_COMMON_HAL_SSL_SSLSOCKET_H From c325633f8e4530c03cd7b5486dfd0c207a7bcb93 Mon Sep 17 00:00:00 2001 From: Tim Hawes <me@timhawes.com> Date: Sat, 27 Nov 2021 13:42:11 +0000 Subject: [PATCH 4/5] Add methods to ssl.SSLContext for handling self-signed certs --- ports/espressif/common-hal/ssl/SSLContext.c | 33 +++++++++++ shared-bindings/ssl/SSLContext.c | 64 +++++++++++++++++++++ shared-bindings/ssl/SSLContext.h | 8 +++ 3 files changed, 105 insertions(+) diff --git a/ports/espressif/common-hal/ssl/SSLContext.c b/ports/espressif/common-hal/ssl/SSLContext.c index e4767716a26d..1924e6df1c6f 100644 --- a/ports/espressif/common-hal/ssl/SSLContext.c +++ b/ports/espressif/common-hal/ssl/SSLContext.c @@ -29,6 +29,8 @@ #include "bindings/espidf/__init__.h" +#include "components/mbedtls/esp_crt_bundle/include/esp_crt_bundle.h" + #include "py/runtime.h" void common_hal_ssl_sslcontext_construct(ssl_sslcontext_obj_t *self) { @@ -62,3 +64,34 @@ ssl_sslsocket_obj_t *common_hal_ssl_sslcontext_wrap_socket(ssl_sslcontext_obj_t return sock; } + +void common_hal_ssl_sslcontext_load_verify_locations(ssl_sslcontext_obj_t *self, + const char *cadata) { + self->ssl_config.crt_bundle_attach = NULL; + self->ssl_config.use_global_ca_store = false; + self->ssl_config.cacert_buf = (const unsigned char *)cadata; + self->ssl_config.cacert_bytes = strlen(cadata) + 1; +} + +void common_hal_ssl_sslcontext_set_default_verify_paths(ssl_sslcontext_obj_t *self) { + self->ssl_config.crt_bundle_attach = esp_crt_bundle_attach; + self->ssl_config.use_global_ca_store = true; + self->ssl_config.cacert_buf = NULL; + self->ssl_config.cacert_bytes = 0; +} + +bool common_hal_ssl_sslcontext_get_check_hostname(ssl_sslcontext_obj_t *self) { + if (self->ssl_config.skip_common_name) { + return 0; + } else { + return 1; + } +} + +void common_hal_ssl_sslcontext_set_check_hostname(ssl_sslcontext_obj_t *self, bool value) { + if (value) { + self->ssl_config.skip_common_name = 0; + } else { + self->ssl_config.skip_common_name = 1; + } +} diff --git a/shared-bindings/ssl/SSLContext.c b/shared-bindings/ssl/SSLContext.c index 839715f4eccf..2b38768f6a37 100644 --- a/shared-bindings/ssl/SSLContext.c +++ b/shared-bindings/ssl/SSLContext.c @@ -29,6 +29,7 @@ #include "py/objtuple.h" #include "py/objlist.h" +#include "py/objproperty.h" #include "py/runtime.h" #include "py/mperrno.h" @@ -51,6 +52,66 @@ STATIC mp_obj_t ssl_sslcontext_make_new(const mp_obj_type_t *type, size_t n_args return MP_OBJ_FROM_PTR(s); } +//| def load_verify_locations(self, cadata: Optional[str] = None) -> None: +//| """Load a set of certification authority (CA) certificates used to validate +//| other peers' certificates.""" +//| + +STATIC mp_obj_t ssl_sslcontext_load_verify_locations(size_t n_args, const mp_obj_t *pos_args, mp_map_t *kw_args) { + enum { ARG_cadata }; + static const mp_arg_t allowed_args[] = { + { MP_QSTR_cadata, MP_ARG_KW_ONLY | MP_ARG_OBJ, {.u_obj = mp_const_none} }, + }; + ssl_sslcontext_obj_t *self = MP_OBJ_TO_PTR(pos_args[0]); + + mp_arg_val_t args[MP_ARRAY_SIZE(allowed_args)]; + mp_arg_parse_all(n_args - 1, pos_args + 1, kw_args, MP_ARRAY_SIZE(allowed_args), allowed_args, args); + + const char *cadata = mp_obj_str_get_str(args[ARG_cadata].u_obj); + + common_hal_ssl_sslcontext_load_verify_locations(self, cadata); + return mp_const_none; +} +STATIC MP_DEFINE_CONST_FUN_OBJ_KW(ssl_sslcontext_load_verify_locations_obj, 1, ssl_sslcontext_load_verify_locations); + +//| def set_default_verify_paths(self) -> None: +//| """Load a set of default certification authority (CA) certificates.""" +//| + +STATIC mp_obj_t ssl_sslcontext_set_default_verify_paths(size_t n_args, const mp_obj_t *pos_args, mp_map_t *kw_args) { + ssl_sslcontext_obj_t *self = MP_OBJ_TO_PTR(pos_args[0]); + + common_hal_ssl_sslcontext_set_default_verify_paths(self); + return mp_const_none; +} +STATIC MP_DEFINE_CONST_FUN_OBJ_KW(ssl_sslcontext_set_default_verify_paths_obj, 1, ssl_sslcontext_set_default_verify_paths); + +//| check_hostname: bool +//| """Whether to match the peer certificate's hostname.""" +//| + +STATIC mp_obj_t ssl_sslcontext_get_check_hostname(mp_obj_t self_in) { + ssl_sslcontext_obj_t *self = MP_OBJ_TO_PTR(self_in); + + return mp_obj_new_bool(common_hal_ssl_sslcontext_get_check_hostname(self)); +} +STATIC MP_DEFINE_CONST_FUN_OBJ_1(ssl_sslcontext_get_check_hostname_obj, ssl_sslcontext_get_check_hostname); + +STATIC mp_obj_t ssl_sslcontext_set_check_hostname(mp_obj_t self_in, mp_obj_t value) { + ssl_sslcontext_obj_t *self = MP_OBJ_TO_PTR(self_in); + + common_hal_ssl_sslcontext_set_check_hostname(self, mp_obj_is_true(value)); + return mp_const_none; +} +STATIC MP_DEFINE_CONST_FUN_OBJ_2(ssl_sslcontext_set_check_hostname_obj, ssl_sslcontext_set_check_hostname); + +const mp_obj_property_t ssl_sslcontext_check_hostname_obj = { + .base.type = &mp_type_property, + .proxy = {(mp_obj_t)&ssl_sslcontext_get_check_hostname_obj, + (mp_obj_t)&ssl_sslcontext_set_check_hostname_obj, + MP_ROM_NONE}, +}; + //| def wrap_socket(self, sock: socketpool.Socket, *, server_side: bool = False, server_hostname: Optional[str] = None) -> ssl.SSLSocket: //| """Wraps the socket into a socket-compatible class that handles SSL negotiation. //| The socket must be of type SOCK_STREAM.""" @@ -85,6 +146,9 @@ STATIC MP_DEFINE_CONST_FUN_OBJ_KW(ssl_sslcontext_wrap_socket_obj, 1, ssl_sslcont STATIC const mp_rom_map_elem_t ssl_sslcontext_locals_dict_table[] = { { MP_ROM_QSTR(MP_QSTR_wrap_socket), MP_ROM_PTR(&ssl_sslcontext_wrap_socket_obj) }, + { MP_ROM_QSTR(MP_QSTR_load_verify_locations), MP_ROM_PTR(&ssl_sslcontext_load_verify_locations_obj) }, + { MP_ROM_QSTR(MP_QSTR_set_default_verify_paths), MP_ROM_PTR(&ssl_sslcontext_set_default_verify_paths_obj) }, + { MP_ROM_QSTR(MP_QSTR_check_hostname), MP_ROM_PTR(&ssl_sslcontext_check_hostname_obj) }, }; STATIC MP_DEFINE_CONST_DICT(ssl_sslcontext_locals_dict, ssl_sslcontext_locals_dict_table); diff --git a/shared-bindings/ssl/SSLContext.h b/shared-bindings/ssl/SSLContext.h index 1e1986a48db9..ef04f25d435e 100644 --- a/shared-bindings/ssl/SSLContext.h +++ b/shared-bindings/ssl/SSLContext.h @@ -39,4 +39,12 @@ void common_hal_ssl_sslcontext_construct(ssl_sslcontext_obj_t *self); ssl_sslsocket_obj_t *common_hal_ssl_sslcontext_wrap_socket(ssl_sslcontext_obj_t *self, socketpool_socket_obj_t *sock, bool server_side, const char *server_hostname); +void common_hal_ssl_sslcontext_load_verify_locations(ssl_sslcontext_obj_t *self, + const char *cadata); + +void common_hal_ssl_sslcontext_set_default_verify_paths(ssl_sslcontext_obj_t *self); + +bool common_hal_ssl_sslcontext_get_check_hostname(ssl_sslcontext_obj_t *self); +void common_hal_ssl_sslcontext_set_check_hostname(ssl_sslcontext_obj_t *self, bool value); + #endif // MICROPY_INCLUDED_SHARED_BINDINGS_SSL_SSLCONTEXT_H From 54e87d366096a9ccc137e6824ea61e9479b238db Mon Sep 17 00:00:00 2001 From: Tim Hawes <me@timhawes.com> Date: Tue, 14 Dec 2021 20:03:44 +0000 Subject: [PATCH 5/5] Apply suggestions from code review Co-authored-by: Scott Shawcroft <scott@tannewt.org> --- ports/espressif/common-hal/ssl/SSLContext.c | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/ports/espressif/common-hal/ssl/SSLContext.c b/ports/espressif/common-hal/ssl/SSLContext.c index 1924e6df1c6f..866024bf0001 100644 --- a/ports/espressif/common-hal/ssl/SSLContext.c +++ b/ports/espressif/common-hal/ssl/SSLContext.c @@ -81,17 +81,9 @@ void common_hal_ssl_sslcontext_set_default_verify_paths(ssl_sslcontext_obj_t *se } bool common_hal_ssl_sslcontext_get_check_hostname(ssl_sslcontext_obj_t *self) { - if (self->ssl_config.skip_common_name) { - return 0; - } else { - return 1; - } + return !self->ssl_config.skip_common_name; } void common_hal_ssl_sslcontext_set_check_hostname(ssl_sslcontext_obj_t *self, bool value) { - if (value) { - self->ssl_config.skip_common_name = 0; - } else { - self->ssl_config.skip_common_name = 1; - } + self->ssl_config.skip_common_name = !value; }