| Message ID | 20241024112449.1362319-4-ilias.apalodimas@linaro.org |
|---|---|
| State | Changes Requested |
| Delegated to: | Ilias Apalodimas |
| Headers | show |
| Series | Enable https for wget | expand |
On 10/24/24 12:24, Ilias Apalodimas wrote: > From: Javier Tia <javier.tia@linaro.org> > > SNI, or Server Name Indication, is an addition to the TLS encryption > protocol that enables a client device to specify the domain name it is > trying to reach in the first step of the TLS handshake, preventing > common name mismatch errors and not reaching to HTTPS server that > enforce this condition. Since most of the websites require it nowadays > add support for it. > > It's worth noting that this is already sent to lwIP [0] > > [0] https://github.com/lwip-tcpip/lwip/pull/47 > > Signed-off-by: Javier Tia <javier.tia@linaro.org> > Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> > --- > lib/lwip/lwip/src/apps/altcp_tls/altcp_tls_mbedtls.c | 11 +++++++---- > lib/lwip/lwip/src/include/lwip/altcp_tls.h | 2 +- > 2 files changed, 8 insertions(+), 5 deletions(-) > > diff --git a/lib/lwip/lwip/src/apps/altcp_tls/altcp_tls_mbedtls.c b/lib/lwip/lwip/src/apps/altcp_tls/altcp_tls_mbedtls.c > index ef19821b89e0..24b432966312 100644 > --- a/lib/lwip/lwip/src/apps/altcp_tls/altcp_tls_mbedtls.c > +++ b/lib/lwip/lwip/src/apps/altcp_tls/altcp_tls_mbedtls.c > @@ -106,6 +106,7 @@ struct altcp_tls_config { > u8_t pkey_count; > u8_t pkey_max; > mbedtls_x509_crt *ca; > + char host[256]; > #if defined(MBEDTLS_SSL_CACHE_C) && ALTCP_MBEDTLS_USE_SESSION_CACHE > /** Inter-connection cache for fast connection startup */ > struct mbedtls_ssl_cache_context cache; > @@ -642,6 +643,7 @@ altcp_mbedtls_setup(void *conf, struct altcp_pcb *conn, struct altcp_pcb *inner_ > /* tell mbedtls about our I/O functions */ > mbedtls_ssl_set_bio(&state->ssl_context, conn, altcp_mbedtls_bio_send, altcp_mbedtls_bio_recv, NULL); > > + mbedtls_ssl_set_hostname(&state->ssl_context, config->host); > altcp_mbedtls_setup_callbacks(conn, inner_conn); > conn->inner_conn = inner_conn; > conn->fns = &altcp_mbedtls_functions; > @@ -951,7 +953,7 @@ altcp_tls_create_config_server_privkey_cert(const u8_t *privkey, size_t privkey_ > } > > static struct altcp_tls_config * > -altcp_tls_create_config_client_common(const u8_t *ca, size_t ca_len, int is_2wayauth) > +altcp_tls_create_config_client_common(const u8_t *ca, size_t ca_len, int is_2wayauth, char *host) > { > int ret; > struct altcp_tls_config *conf = altcp_tls_create_config(0, (is_2wayauth) ? 1 : 0, (is_2wayauth) ? 1 : 0, ca != NULL); > @@ -973,13 +975,14 @@ altcp_tls_create_config_client_common(const u8_t *ca, size_t ca_len, int is_2way > > mbedtls_ssl_conf_ca_chain(&conf->conf, conf->ca, NULL); > } > + memcpy(conf->host, host, sizeof(conf->host)); What if host is smaller than sizeof(conf->host)? Should be strncpy() IMHO. > return conf; > } > > struct altcp_tls_config * > -altcp_tls_create_config_client(const u8_t *ca, size_t ca_len) > +altcp_tls_create_config_client(const u8_t *ca, size_t ca_len, char *host) > { > - return altcp_tls_create_config_client_common(ca, ca_len, 0); > + return altcp_tls_create_config_client_common(ca, ca_len, 0, host); > } > > struct altcp_tls_config * > @@ -995,7 +998,7 @@ altcp_tls_create_config_client_2wayauth(const u8_t *ca, size_t ca_len, const u8_ > return NULL; > } > > - conf = altcp_tls_create_config_client_common(ca, ca_len, 1); > + conf = altcp_tls_create_config_client_common(ca, ca_len, 1, NULL); > if (conf == NULL) { > return NULL; > } > diff --git a/lib/lwip/lwip/src/include/lwip/altcp_tls.h b/lib/lwip/lwip/src/include/lwip/altcp_tls.h > index fcb784d89d70..fb0618234481 100644 > --- a/lib/lwip/lwip/src/include/lwip/altcp_tls.h > +++ b/lib/lwip/lwip/src/include/lwip/altcp_tls.h > @@ -92,7 +92,7 @@ struct altcp_tls_config *altcp_tls_create_config_server_privkey_cert(const u8_t > /** @ingroup altcp_tls > * Create an ALTCP_TLS client configuration handle > */ > -struct altcp_tls_config *altcp_tls_create_config_client(const u8_t *cert, size_t cert_len); > +struct altcp_tls_config *altcp_tls_create_config_client(const u8_t *cert, size_t cert_len, char *host); > > /** @ingroup altcp_tls > * Create an ALTCP_TLS client configuration handle with two-way server/client authentication With the above comment addressed: Reviewed-by: Jerome Forissier <jerome.forissier@linaro.org> Thanks,
Hi Jerome, On Wed, 6 Nov 2024 at 13:39, Jerome Forissier <jerome.forissier@linaro.org> wrote: > > > > On 10/24/24 12:24, Ilias Apalodimas wrote: > > From: Javier Tia <javier.tia@linaro.org> > > > > SNI, or Server Name Indication, is an addition to the TLS encryption > > protocol that enables a client device to specify the domain name it is > > trying to reach in the first step of the TLS handshake, preventing > > common name mismatch errors and not reaching to HTTPS server that > > enforce this condition. Since most of the websites require it nowadays > > add support for it. > > > > It's worth noting that this is already sent to lwIP [0] > > > > [0] https://github.com/lwip-tcpip/lwip/pull/47 > > > > Signed-off-by: Javier Tia <javier.tia@linaro.org> > > Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> > > --- > > lib/lwip/lwip/src/apps/altcp_tls/altcp_tls_mbedtls.c | 11 +++++++---- > > lib/lwip/lwip/src/include/lwip/altcp_tls.h | 2 +- > > 2 files changed, 8 insertions(+), 5 deletions(-) > > > > diff --git a/lib/lwip/lwip/src/apps/altcp_tls/altcp_tls_mbedtls.c b/lib/lwip/lwip/src/apps/altcp_tls/altcp_tls_mbedtls.c > > index ef19821b89e0..24b432966312 100644 > > --- a/lib/lwip/lwip/src/apps/altcp_tls/altcp_tls_mbedtls.c > > +++ b/lib/lwip/lwip/src/apps/altcp_tls/altcp_tls_mbedtls.c > > @@ -106,6 +106,7 @@ struct altcp_tls_config { > > u8_t pkey_count; > > u8_t pkey_max; > > mbedtls_x509_crt *ca; > > + char host[256]; > > #if defined(MBEDTLS_SSL_CACHE_C) && ALTCP_MBEDTLS_USE_SESSION_CACHE > > /** Inter-connection cache for fast connection startup */ > > struct mbedtls_ssl_cache_context cache; > > @@ -642,6 +643,7 @@ altcp_mbedtls_setup(void *conf, struct altcp_pcb *conn, struct altcp_pcb *inner_ > > /* tell mbedtls about our I/O functions */ > > mbedtls_ssl_set_bio(&state->ssl_context, conn, altcp_mbedtls_bio_send, altcp_mbedtls_bio_recv, NULL); > > > > + mbedtls_ssl_set_hostname(&state->ssl_context, config->host); > > altcp_mbedtls_setup_callbacks(conn, inner_conn); > > conn->inner_conn = inner_conn; > > conn->fns = &altcp_mbedtls_functions; > > @@ -951,7 +953,7 @@ altcp_tls_create_config_server_privkey_cert(const u8_t *privkey, size_t privkey_ > > } > > > > static struct altcp_tls_config * > > -altcp_tls_create_config_client_common(const u8_t *ca, size_t ca_len, int is_2wayauth) > > +altcp_tls_create_config_client_common(const u8_t *ca, size_t ca_len, int is_2wayauth, char *host) > > { > > int ret; > > struct altcp_tls_config *conf = altcp_tls_create_config(0, (is_2wayauth) ? 1 : 0, (is_2wayauth) ? 1 : 0, ca != NULL); > > @@ -973,13 +975,14 @@ altcp_tls_create_config_client_common(const u8_t *ca, size_t ca_len, int is_2way > > > > mbedtls_ssl_conf_ca_chain(&conf->conf, conf->ca, NULL); > > } > > + memcpy(conf->host, host, sizeof(conf->host)); > > What if host is smaller than sizeof(conf->host)? Should be strncpy() IMHO. Looking at it, if host is smaller than sizeof(conf->host) we will copy random bytes from that and we dont know what... So I think it's better to memcpy the min value of strlen(host) and sizeof(conf->host)? Thanks /Ilias > > > return conf; > > } > > > > struct altcp_tls_config * > > -altcp_tls_create_config_client(const u8_t *ca, size_t ca_len) > > +altcp_tls_create_config_client(const u8_t *ca, size_t ca_len, char *host) > > { > > - return altcp_tls_create_config_client_common(ca, ca_len, 0); > > + return altcp_tls_create_config_client_common(ca, ca_len, 0, host); > > } > > > > struct altcp_tls_config * > > @@ -995,7 +998,7 @@ altcp_tls_create_config_client_2wayauth(const u8_t *ca, size_t ca_len, const u8_ > > return NULL; > > } > > > > - conf = altcp_tls_create_config_client_common(ca, ca_len, 1); > > + conf = altcp_tls_create_config_client_common(ca, ca_len, 1, NULL); > > if (conf == NULL) { > > return NULL; > > } > > diff --git a/lib/lwip/lwip/src/include/lwip/altcp_tls.h b/lib/lwip/lwip/src/include/lwip/altcp_tls.h > > index fcb784d89d70..fb0618234481 100644 > > --- a/lib/lwip/lwip/src/include/lwip/altcp_tls.h > > +++ b/lib/lwip/lwip/src/include/lwip/altcp_tls.h > > @@ -92,7 +92,7 @@ struct altcp_tls_config *altcp_tls_create_config_server_privkey_cert(const u8_t > > /** @ingroup altcp_tls > > * Create an ALTCP_TLS client configuration handle > > */ > > -struct altcp_tls_config *altcp_tls_create_config_client(const u8_t *cert, size_t cert_len); > > +struct altcp_tls_config *altcp_tls_create_config_client(const u8_t *cert, size_t cert_len, char *host); > > > > /** @ingroup altcp_tls > > * Create an ALTCP_TLS client configuration handle with two-way server/client authentication > > With the above comment addressed: > > Reviewed-by: Jerome Forissier <jerome.forissier@linaro.org> > > Thanks, > -- > Jerome
diff --git a/lib/lwip/lwip/src/apps/altcp_tls/altcp_tls_mbedtls.c b/lib/lwip/lwip/src/apps/altcp_tls/altcp_tls_mbedtls.c index ef19821b89e0..24b432966312 100644 --- a/lib/lwip/lwip/src/apps/altcp_tls/altcp_tls_mbedtls.c +++ b/lib/lwip/lwip/src/apps/altcp_tls/altcp_tls_mbedtls.c @@ -106,6 +106,7 @@ struct altcp_tls_config { u8_t pkey_count; u8_t pkey_max; mbedtls_x509_crt *ca; + char host[256]; #if defined(MBEDTLS_SSL_CACHE_C) && ALTCP_MBEDTLS_USE_SESSION_CACHE /** Inter-connection cache for fast connection startup */ struct mbedtls_ssl_cache_context cache; @@ -642,6 +643,7 @@ altcp_mbedtls_setup(void *conf, struct altcp_pcb *conn, struct altcp_pcb *inner_ /* tell mbedtls about our I/O functions */ mbedtls_ssl_set_bio(&state->ssl_context, conn, altcp_mbedtls_bio_send, altcp_mbedtls_bio_recv, NULL); + mbedtls_ssl_set_hostname(&state->ssl_context, config->host); altcp_mbedtls_setup_callbacks(conn, inner_conn); conn->inner_conn = inner_conn; conn->fns = &altcp_mbedtls_functions; @@ -951,7 +953,7 @@ altcp_tls_create_config_server_privkey_cert(const u8_t *privkey, size_t privkey_ } static struct altcp_tls_config * -altcp_tls_create_config_client_common(const u8_t *ca, size_t ca_len, int is_2wayauth) +altcp_tls_create_config_client_common(const u8_t *ca, size_t ca_len, int is_2wayauth, char *host) { int ret; struct altcp_tls_config *conf = altcp_tls_create_config(0, (is_2wayauth) ? 1 : 0, (is_2wayauth) ? 1 : 0, ca != NULL); @@ -973,13 +975,14 @@ altcp_tls_create_config_client_common(const u8_t *ca, size_t ca_len, int is_2way mbedtls_ssl_conf_ca_chain(&conf->conf, conf->ca, NULL); } + memcpy(conf->host, host, sizeof(conf->host)); return conf; } struct altcp_tls_config * -altcp_tls_create_config_client(const u8_t *ca, size_t ca_len) +altcp_tls_create_config_client(const u8_t *ca, size_t ca_len, char *host) { - return altcp_tls_create_config_client_common(ca, ca_len, 0); + return altcp_tls_create_config_client_common(ca, ca_len, 0, host); } struct altcp_tls_config * @@ -995,7 +998,7 @@ altcp_tls_create_config_client_2wayauth(const u8_t *ca, size_t ca_len, const u8_ return NULL; } - conf = altcp_tls_create_config_client_common(ca, ca_len, 1); + conf = altcp_tls_create_config_client_common(ca, ca_len, 1, NULL); if (conf == NULL) { return NULL; } diff --git a/lib/lwip/lwip/src/include/lwip/altcp_tls.h b/lib/lwip/lwip/src/include/lwip/altcp_tls.h index fcb784d89d70..fb0618234481 100644 --- a/lib/lwip/lwip/src/include/lwip/altcp_tls.h +++ b/lib/lwip/lwip/src/include/lwip/altcp_tls.h @@ -92,7 +92,7 @@ struct altcp_tls_config *altcp_tls_create_config_server_privkey_cert(const u8_t /** @ingroup altcp_tls * Create an ALTCP_TLS client configuration handle */ -struct altcp_tls_config *altcp_tls_create_config_client(const u8_t *cert, size_t cert_len); +struct altcp_tls_config *altcp_tls_create_config_client(const u8_t *cert, size_t cert_len, char *host); /** @ingroup altcp_tls * Create an ALTCP_TLS client configuration handle with two-way server/client authentication