Re: [PATCH v2 4/6] net: lwip: Enable https:// support for wget

Hi Jerome

On Wed, 6 Nov 2024 at 13:50, Jerome Forissier jerome.forissier@linaro.org wrote:

Hi Ilias,

On 10/24/24 12:24, Ilias Apalodimas wrote:

...

With the recent changes of lwip & mbedTLS we can now download from https:// urls instead of just http://. Adjust our wget lwip version parsing to support both URLs. While at it adjust the default TCP window for QEMU since https seems to require at least 16384

Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org

cmd/Kconfig | 19 ++++++++++++ net/lwip/Kconfig | 2 +- net/lwip/wget.c | 78 +++++++++++++++++++++++++++++++++++++++++++----- 3 files changed, 91 insertions(+), 8 deletions(-)

diff --git a/cmd/Kconfig b/cmd/Kconfig index 3ee70f31b142..1d90a884e2c1 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -2126,6 +2126,25 @@ config CMD_WGET wget is a simple command to download kernel, or other files, from a http server over TCP.

+config WGET_HTTPS

• bool "wget https"
  

• depends on CMD_WGET
  

• depends on PROT_TCP_LWIP
  

• depends on MBEDTLS_LIB
  

•    select SHA256
  

•    select RSA
  

•    select ASYMMETRIC_KEY_TYPE
  

•    select ASYMMETRIC_PUBLIC_KEY_SUBTYPE
  

•    select X509_CERTIFICATE_PARSER
  

•    select PKCS7_MESSAGE_PARSER
  

• select MBEDTLS_LIB_CRYPTO
  

• select MBEDTLS_LIB_TLS
  

• select RSA_VERIFY_WITH_PKEY
  

• select X509_CERTIFICATE_PARSER
  

• select PKCS7_MESSAGE_PARSER
  

• help
  

•   Enable TLS over http for wget.
  

s/http/HTTP/ Maybe add "This adds support for https:// URLs"?

Sure

...

endif # if CMD_NET

config CMD_PXE diff --git a/net/lwip/Kconfig b/net/lwip/Kconfig index 8a67de4cf335..a9ae9bf7fa2a 100644 --- a/net/lwip/Kconfig +++ b/net/lwip/Kconfig @@ -37,7 +37,7 @@ config PROT_UDP_LWIP

config LWIP_TCP_WND int "Value of TCP_WND"

• default 8000 if ARCH_QEMU
  

• default 32768 if ARCH_QEMU
  default 3000000
  help
    Default value for TCP_WND in the lwIP configuration
  

diff --git a/net/lwip/wget.c b/net/lwip/wget.c index b495ebd1aa96..1a2ecdcddf34 100644 --- a/net/lwip/wget.c +++ b/net/lwip/wget.c @@ -7,13 +7,17 @@ #include <efi_loader.h> #include <image.h> #include <lwip/apps/http_client.h> +#include "lwip/altcp_tls.h" #include <lwip/timeouts.h> +#include <rng.h> #include <mapmem.h> #include <net.h> #include <time.h> +#include <dm/uclass.h>

#define SERVER_NAME_SIZE 200 #define HTTP_PORT_DEFAULT 80 +#define HTTPS_PORT_DEFAULT 443 #define PROGRESS_PRINT_STEP_BYTES (100 * 1024)

enum done_state { @@ -32,18 +36,53 @@ struct wget_ctx { enum done_state done; };

+bool wget_validate_uri(char *uri);

+int mbedtls_hardware_poll(void *data, unsigned char *output, size_t len,

•                   size_t *olen)
  

+{

• struct udevice *dev;
  

• u64 rng = 0;
  

• int ret;
  

• *olen = 0;
  

• ret = uclass_get_device(UCLASS_RNG, 0, &dev);
  

• if (ret) {
  

•         log_err("Failed to get an rng: %d\n", ret);
  

•         return ret;
  

• }
  

• ret = dm_rng_read(dev, &rng, sizeof(rng));
  

• if (ret)
  

•         return ret;
  

• memcpy(output, &rng, len);
  

• *olen = sizeof(rng);
  

• return 0;
  

+}

static int parse_url(char *url, char *host, u16 *port, char **path) { char *p, *pp; long lport;

• size_t prefix_len = 0;
  

• if (!wget_validate_uri(url)) {
  

•         log_err("Invalid URL. Use http(s)://\n");
  

•         return -EINVAL;
  

• }
  
  

• *port = HTTP_PORT_DEFAULT;
  

• prefix_len = strlen("http://");
  p = strstr(url, "http://");
  if (!p) {
  

•         log_err("only http:// is supported\n");
  

•         return -EINVAL;
  

•         p = strstr(url, "https://");
  

•         prefix_len = strlen("https://");
  

•         *port = HTTPS_PORT_DEFAULT;
  }
  
  

• p += strlen("http://");
  

• p += prefix_len;
  
  /* Parse hostname */
  pp = strchr(p, ':');
  

@@ -67,9 +106,8 @@ static int parse_url(char *url, char *host, u16 *port, char **path) if (lport > 65535) return -EINVAL; *port = (u16)lport;

• } else {
  

•         *port = HTTP_PORT_DEFAULT;
  }
  

• if (*pp != '/')
          return -EINVAL;
  *path = pp;
  

@@ -210,6 +248,9 @@ static void httpc_result_cb(void *arg, httpc_result_t httpc_result, static int wget_loop(struct udevice *udev, ulong dst_addr, char *uri) { char server_name[SERVER_NAME_SIZE]; +#if defined CONFIG_WGET_HTTPS

• altcp_allocator_t tls_allocator;
  

+#endif httpc_connection_t conn; httpc_state_t *state; struct netif *netif; @@ -232,6 +273,22 @@ static int wget_loop(struct udevice *udev, ulong dst_addr, char *uri) return -1;

  memset(&conn, 0, sizeof(conn));

+#if defined CONFIG_WGET_HTTPS

• if (port == HTTPS_PORT_DEFAULT) {
  

Does this mean the port has to be 443 for HTTPS to be used? Or that plain HTTP is not possible over port 443? That would be overly restrictive IMO.

We set that port in parse_url(). So if http:// is parsed we will, by default, set it to 80 and if https:// is found we set it to 443. But I think you are right, we already support configurable port numbers which I missed, so I'll just pass a bool to parse_url and only enable this for https:// regardless of the port

Cheers /Ilias

...

•         tls_allocator.alloc = &altcp_tls_alloc;
  

•         tls_allocator.arg =
  

•                 altcp_tls_create_config_client(NULL, 0, server_name);
  

•         if (!tls_allocator.arg) {
  

•                 log_err("error: Cannot create a TLS connection\n");
  

•                 net_lwip_remove_netif(netif);
  

•                 return -1;
  

•         }
  

•         conn.altcp_allocator = &tls_allocator;
  

• }
  

+#endif

• conn.result_fn = httpc_result_cb;
  ctx.path = path;
  if (httpc_get_file_dns(server_name, port, path, &conn, httpc_recv_cb,
  

@@ -316,6 +373,7 @@ bool wget_validate_uri(char *uri) char c; bool ret = true; char *str_copy, *s, *authority;

• size_t prefix_len = 0;
  
  for (c = 0x1; c < 0x21; c++) {
          if (strchr(uri, c)) {
  

@@ -323,15 +381,21 @@ bool wget_validate_uri(char *uri) return false; } }

• if (strchr(uri, 0x7f)) {
          log_err("invalid character is used\n");
          return false;
  }
  
  

• if (strncmp(uri, "http://", 7)) {
  

•         log_err("only http:// is supported\n");
  

• if (!strncmp(uri, "http://", strlen("http://"))) {
  

•         prefix_len = strlen("http://");
  

• } else if (!strncmp(uri, "https://", strlen("https://"))) {
  

•         prefix_len = strlen("https://");
  

• } else {
  

•         log_err("only http(s):// is supported\n");
          return false;
  }
  

• str_copy = strdup(uri);
  if (!str_copy)
          return false;
  

Thanks,

Jerome