diff mbox series

[v2,3/6] net: lwip: Add Support Server Name Indication support

Message ID 20241024112449.1362319-4-ilias.apalodimas@linaro.org
State New
Headers show
Series Enable https for wget | expand

Commit Message

Ilias Apalodimas Oct. 24, 2024, 11:24 a.m. UTC
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(-)

Comments

Jerome Forissier Nov. 6, 2024, 1:39 p.m. UTC | #1
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,
Ilias Apalodimas Nov. 8, 2024, 11:18 a.m. UTC | #2
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 mbox series

Patch

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