| Message ID | 20241018142235.715571-5-ilias.apalodimas@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Enable https for wget | expand |
Hi Ilias, On Fri, 18 Oct 2024 at 08:23, Ilias Apalodimas <ilias.apalodimas@linaro.org> 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 > requite 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 8c677b1e4864..e58566a9ba34 100644 > --- a/cmd/Kconfig > +++ b/cmd/Kconfig > @@ -2118,6 +2118,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 Is it possible to do wget programmatically, i.e. without CMDLINE? > + 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. > + > 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..b4f039d38962 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 err; ret > + > + *olen = 0; > + > + err = uclass_get_device(UCLASS_RNG, 0, &dev); Using uclass_get_device_by_seq() would allow aliases to select which device is used. > + if (err) > + return err; > + err = dm_rng_read(dev, &rng, sizeof(rng)); > + if (err) { > + log_err("Failed to get an rng: %d\n", err); If you are showing an error, I think it is more likely to happen when trying to find the device, so perhaps do this on the uclass_get_device() call? > + return err; > + } > + > + memcpy(output, &rng, len); > + *olen = sizeof(rng); But then why not dm_rng_read() into output and avoid the u64? Actually, dm_rng_ops-read() should return the number of bytes, perhaps? > + > + 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 (IS_ENABLED(CONFIG_WGET_HTTPS)) > + if (port == HTTPS_PORT_DEFAULT) { > + 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; > -- > 2.45.2 > Regards, Simon
Hi Simon, On Sat, 19 Oct 2024 at 14:50, Simon Glass <sjg@chromium.org> wrote: > > Hi Ilias, > > On Fri, 18 Oct 2024 at 08:23, Ilias Apalodimas > <ilias.apalodimas@linaro.org> 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 > > requite 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 8c677b1e4864..e58566a9ba34 100644 > > --- a/cmd/Kconfig > > +++ b/cmd/Kconfig > > @@ -2118,6 +2118,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 > > Is it possible to do wget programmatically, i.e. without CMDLINE? Yes. But that would require more untangling of wget. I'll look into that once we are done with features. > > > + 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. > > + > > 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..b4f039d38962 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 err; > > ret > > > + > > + *olen = 0; > > + > > + err = uclass_get_device(UCLASS_RNG, 0, &dev); > > Using uclass_get_device_by_seq() would allow aliases to select which > device is used. Ok, but I don't think we need it here. We just need a random number from any device that can send us one no? > > > + if (err) > > + return err; > > + err = dm_rng_read(dev, &rng, sizeof(rng)); > > + if (err) { > > + log_err("Failed to get an rng: %d\n", err); > > If you are showing an error, I think it is more likely to happen when > trying to find the device, so perhaps do this on the > uclass_get_device() call? Sure > > > + return err; > > + } > > + > > + memcpy(output, &rng, len); > > + *olen = sizeof(rng); > > But then why not dm_rng_read() into output and avoid the u64? > Actually, dm_rng_ops-read() should return the number of bytes, > perhaps? The caller defines the length. Copying blindly a u64 might overflow. But the current code should be *olen = len > > > + > > + 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 (IS_ENABLED(CONFIG_WGET_HTTPS)) > Unfortunately, I don't think we can use that here. If CONFIG_WGET_HTTPS is not enabled LWIP_ALTCP will not be defined, and the altcp_allocator_t field will be missing from the struct leading to a compilation error Thanks /Ilias > > + if (port == HTTPS_PORT_DEFAULT) { > > + 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; > > -- > > 2.45.2 > > > > Regards, > Simon
Hi Simon, [...] > > > memset(&conn, 0, sizeof(conn)); > > > +#if defined CONFIG_WGET_HTTPS > > > > if (IS_ENABLED(CONFIG_WGET_HTTPS)) > > > > Unfortunately, I don't think we can use that here. If > CONFIG_WGET_HTTPS is not enabled LWIP_ALTCP will not be defined, and > the altcp_allocator_t field will be missing from the struct leading to > a compilation error I forgot to update this in v2 [0]. I'll send a v3 regardless and will change this to #if IS_ENABLED(CONFIG_MBEDTLS_LIB_TLS) which is a better fit anyway [...] Cheers /Ilias
diff --git a/cmd/Kconfig b/cmd/Kconfig index 8c677b1e4864..e58566a9ba34 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -2118,6 +2118,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. + 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..b4f039d38962 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 err; + + *olen = 0; + + err = uclass_get_device(UCLASS_RNG, 0, &dev); + if (err) + return err; + err = dm_rng_read(dev, &rng, sizeof(rng)); + if (err) { + log_err("Failed to get an rng: %d\n", err); + return err; + } + + 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) { + 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;
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 requite 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(-)