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_WGETIs 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_LWIPdepends on MBEDTLS_LIBselect SHA256select RSAselect ASYMMETRIC_KEY_TYPEselect ASYMMETRIC_PUBLIC_KEY_SUBTYPEselect X509_CERTIFICATE_PARSERselect PKCS7_MESSAGE_PARSERselect MBEDTLS_LIB_CRYPTOselect MBEDTLS_LIB_TLSselect RSA_VERIFY_WITH_PKEYselect X509_CERTIFICATE_PARSERselect PKCS7_MESSAGE_PARSERhelpEnable 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 configurationdiff --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