diff mbox series

[1/5] net: lwip: extend wget to support CA (root) certificates

Message ID 3a93751157801fe709d995eae1883f9e3219733c.1740672437.git.jerome.forissier@linaro.org
State New
Headers show
Series net: lwip: root certificates | expand

Commit Message

Jerome Forissier Feb. 27, 2025, 4:09 p.m. UTC 
Add the "cacert" (Certification Authority certificates) subcommand to
wget to pass root certificates to the code handling the HTTPS protocol.
The subcommand is enabled by the WGET_CACERT Kconfig symbol.

Usage example:

 => dhcp
 # Download some root certificates (note: not authenticated!)
 => wget https://curl.se/ca/cacert.pem
 # Enable certificate verification
 => wget cacert $loadaddr $filesize
 # Disable certificate verification
 => wget cacert 0 0

Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
---
 cmd/Kconfig                      | 15 +++++++++
 cmd/net-lwip.c                   | 15 +++++++--
 lib/mbedtls/Makefile             |  3 ++
 lib/mbedtls/mbedtls_def_config.h |  5 +++
 net/lwip/wget.c                  | 55 +++++++++++++++++++++++++++++++-
 5 files changed, 89 insertions(+), 4 deletions(-)

Comments

Ilias Apalodimas Feb. 28, 2025, 9:24 p.m. UTC | #1 
Hi Jerome

>
> +config WGET_CACERT
> +       bool "wget cacert"
> +       depends on CMD_WGET
> +       depends on WGET_HTTPS
> +       help
> +         Adds the "cacert" sub-command to wget to provide root certificates
> +         to the HTTPS engine.
> +
> +config MBEDTLS_LIB_X509_PEM
> +       depends on WGET_CACERT
> +       bool "Support for PEM-encoded X509 certificates"
> +       help
> +         This option enables MbedTLS to parse PEM-encoded X509 certificates.
> +         When disabled, only DER format is accepted.
> +
>  endif  # if CMD_NET

I guess that's needed because most of the RootCAs you can download are in PEM?

[...]

>  }
>
> +#if defined CONFIG_WGET_HTTPS

you can do #if IS_ENABLED() here

> +static char *cacert;
> +size_t cacert_size;
> +#endif
> +
> +#if defined CONFIG_WGET_CACERT
> +static int set_cacert(char * const saddr, char * const ssz)
> +{
> +       mbedtls_x509_crt crt;
> +       ulong addr, sz;
> +       int ret;
> +
> +       if (cacert)
> +               free(cacert);
> +
> +       addr = hextoul(saddr, NULL);
> +       sz = hextoul(ssz, NULL);
> +       sz++; /* For the trailing '\0' in case of a text (PEM) file */
> +
> +       if (!addr) {
> +               cacert = NULL;

cacert is already allocated. Can't we just free it here if it's
supposed to be removed and reuse the memory otherwise, instead of
doing free/alloc on every command?

> +               cacert_size = 0;
> +               return CMD_RET_SUCCESS;
> +       }
> +
> +       cacert = malloc(sz);
> +       if (!cacert)
> +               return CMD_RET_FAILURE;
> +       cacert_size = sz;
> +
> +       memcpy(cacert, (void *)addr, sz - 1);
> +       cacert[sz] = '\0';
> +
> +       mbedtls_x509_crt_init(&crt);
> +       ret = mbedtls_x509_crt_parse(&crt, cacert, cacert_size);
> +       if (ret) {
> +               printf("Could not parse certificates (%d)\n", ret);
> +               free(cacert);
> +               cacert = NULL;
> +               cacert_size = 0;
> +               return CMD_RET_FAILURE;
> +       }
> +
> +       return CMD_RET_SUCCESS;

[...]

Thanks
/Ilias
Jerome Forissier March 5, 2025, 12:09 p.m. UTC | #2 
Hi Ilias,

On 2/28/25 22:24, Ilias Apalodimas wrote:
> Hi Jerome
> 
>>
>> +config WGET_CACERT
>> +       bool "wget cacert"
>> +       depends on CMD_WGET
>> +       depends on WGET_HTTPS
>> +       help
>> +         Adds the "cacert" sub-command to wget to provide root certificates
>> +         to the HTTPS engine.
>> +
>> +config MBEDTLS_LIB_X509_PEM
>> +       depends on WGET_CACERT
>> +       bool "Support for PEM-encoded X509 certificates"
>> +       help
>> +         This option enables MbedTLS to parse PEM-encoded X509 certificates.
>> +         When disabled, only DER format is accepted.
>> +
>>  endif  # if CMD_NET
> 
> I guess that's needed because most of the RootCAs you can download are in PEM?

Yes, but thinking about it I'll just drop the PEM support for now as it makes
things a bit more complex (the `\0` issue) for no good reason.


> 
> [...]
> 
>>  }
>>
>> +#if defined CONFIG_WGET_HTTPS
> 
> you can do #if IS_ENABLED() here

Better yet: #if CONFIG_IS_ENABLED(WGET_HTTPS) I suppose.
 
>> +static char *cacert;
>> +size_t cacert_size;
>> +#endif
>> +
>> +#if defined CONFIG_WGET_CACERT
>> +static int set_cacert(char * const saddr, char * const ssz)
>> +{
>> +       mbedtls_x509_crt crt;
>> +       ulong addr, sz;
>> +       int ret;
>> +
>> +       if (cacert)
>> +               free(cacert);
>> +
>> +       addr = hextoul(saddr, NULL);
>> +       sz = hextoul(ssz, NULL);
>> +       sz++; /* For the trailing '\0' in case of a text (PEM) file */
>> +
>> +       if (!addr) {
>> +               cacert = NULL;
> 
> cacert is already allocated. Can't we just free it here if it's
> supposed to be removed and reuse the memory otherwise, instead of
> doing free/alloc on every command?

The size of the certificates may change so it's easier to free/malloc
every time.


Thanks,
diff mbox series

Patch

diff --git a/cmd/Kconfig b/cmd/Kconfig
index 8dd42571abc..a188a2ef24b 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -2177,6 +2177,21 @@  config WGET_HTTPS
 	help
 	  Enable TLS over http for wget.
 
+config WGET_CACERT
+	bool "wget cacert"
+	depends on CMD_WGET
+	depends on WGET_HTTPS
+	help
+	  Adds the "cacert" sub-command to wget to provide root certificates
+	  to the HTTPS engine.
+
+config MBEDTLS_LIB_X509_PEM
+	depends on WGET_CACERT
+	bool "Support for PEM-encoded X509 certificates"
+	help
+	  This option enables MbedTLS to parse PEM-encoded X509 certificates.
+	  When disabled, only DER format is accepted.
+
 endif  # if CMD_NET
 
 config CMD_PXE
diff --git a/cmd/net-lwip.c b/cmd/net-lwip.c
index 0fd446ecb20..0672f48a7a8 100644
--- a/cmd/net-lwip.c
+++ b/cmd/net-lwip.c
@@ -27,9 +27,18 @@  U_BOOT_CMD(dns, 3, 1, do_dns, "lookup the IP of a hostname",
 #endif
 
 #if defined(CONFIG_CMD_WGET)
-U_BOOT_CMD(wget, 3, 1, do_wget,
-	   "boot image via network using HTTP/HTTPS protocol",
+U_BOOT_CMD(wget, 4, 1, do_wget,
+	   "boot image via network using HTTP/HTTPS protocol"
+#if defined(CONFIG_WGET_CACERT)
+	   "\nwget cacert - configure wget root certificates"
+#endif
+	   ,
 	   "[loadAddress] url\n"
-	   "wget [loadAddress] [host:]path"
+	   "wget [loadAddress] [host:]path\n"
+	   "    - load file"
+#if defined(CONFIG_WGET_CACERT)
+	   "\nwget cacert <address> <length>\n"
+	   "    - provide CA certificates (0 0 to disable verification)"
+#endif
 );
 #endif
diff --git a/lib/mbedtls/Makefile b/lib/mbedtls/Makefile
index e66c2018d97..8a0a984e149 100644
--- a/lib/mbedtls/Makefile
+++ b/lib/mbedtls/Makefile
@@ -57,6 +57,9 @@  mbedtls_lib_x509-$(CONFIG_$(SPL_)X509_CERTIFICATE_PARSER_MBEDTLS) += \
 	$(MBEDTLS_LIB_DIR)/x509_crt.o
 mbedtls_lib_x509-$(CONFIG_$(SPL_)PKCS7_MESSAGE_PARSER_MBEDTLS) += \
 	$(MBEDTLS_LIB_DIR)/pkcs7.o
+mbedtls_lib_x509-$(CONFIG_MBEDTLS_LIB_X509_PEM) += \
+	$(MBEDTLS_LIB_DIR)/base64.o \
+	$(MBEDTLS_LIB_DIR)/pem.o
 
 #mbedTLS TLS support
 obj-$(CONFIG_MBEDTLS_LIB_TLS) += mbedtls_lib_tls.o
diff --git a/lib/mbedtls/mbedtls_def_config.h b/lib/mbedtls/mbedtls_def_config.h
index fd440c392f9..7b6a7f482f0 100644
--- a/lib/mbedtls/mbedtls_def_config.h
+++ b/lib/mbedtls/mbedtls_def_config.h
@@ -138,6 +138,11 @@ 
 #define MBEDTLS_ECP_DP_BP384R1_ENABLED
 #define MBEDTLS_ECP_DP_BP512R1_ENABLED
 
+/* CA certificates parsing */
+#if CONFIG_IS_ENABLED(MBEDTLS_LIB_X509_PEM)
+#define MBEDTLS_PEM_PARSE_C
+#define MBEDTLS_BASE64_C
+#endif
 #endif /* #if defined CONFIG_MBEDTLS_LIB_TLS */
 
 #endif /* #if defined CONFIG_MBEDTLS_LIB */
diff --git a/net/lwip/wget.c b/net/lwip/wget.c
index 14f27d42998..14466598d7c 100644
--- a/net/lwip/wget.c
+++ b/net/lwip/wget.c
@@ -285,6 +285,53 @@  static err_t httpc_headers_done_cb(httpc_state_t *connection, void *arg, struct
 	return ERR_OK;
 }
 
+#if defined CONFIG_WGET_HTTPS
+static char *cacert;
+size_t cacert_size;
+#endif
+
+#if defined CONFIG_WGET_CACERT
+static int set_cacert(char * const saddr, char * const ssz)
+{
+	mbedtls_x509_crt crt;
+	ulong addr, sz;
+	int ret;
+
+	if (cacert)
+		free(cacert);
+
+	addr = hextoul(saddr, NULL);
+	sz = hextoul(ssz, NULL);
+	sz++; /* For the trailing '\0' in case of a text (PEM) file */
+
+	if (!addr) {
+		cacert = NULL;
+		cacert_size = 0;
+		return CMD_RET_SUCCESS;
+	}
+
+	cacert = malloc(sz);
+	if (!cacert)
+		return CMD_RET_FAILURE;
+	cacert_size = sz;
+
+	memcpy(cacert, (void *)addr, sz - 1);
+	cacert[sz] = '\0';
+
+	mbedtls_x509_crt_init(&crt);
+	ret = mbedtls_x509_crt_parse(&crt, cacert, cacert_size);
+	if (ret) {
+		printf("Could not parse certificates (%d)\n", ret);
+		free(cacert);
+		cacert = NULL;
+		cacert_size = 0;
+		return CMD_RET_FAILURE;
+	}
+
+	return CMD_RET_SUCCESS;
+}
+#endif
+
 static int wget_loop(struct udevice *udev, ulong dst_addr, char *uri)
 {
 #if defined CONFIG_WGET_HTTPS
@@ -316,7 +363,8 @@  static int wget_loop(struct udevice *udev, ulong dst_addr, char *uri)
 	if (is_https) {
 		tls_allocator.alloc = &altcp_tls_alloc;
 		tls_allocator.arg =
-			altcp_tls_create_config_client(NULL, 0, ctx.server_name);
+			altcp_tls_create_config_client(cacert, cacert_size,
+						       ctx.server_name);
 
 		if (!tls_allocator.arg) {
 			log_err("error: Cannot create a TLS connection\n");
@@ -369,6 +417,11 @@  int do_wget(struct cmd_tbl *cmdtp, int flag, int argc, char * const argv[])
 	ulong dst_addr;
 	char nurl[1024];
 
+#if defined CONFIG_WGET_CACERT
+	if (argc == 4 && !strncmp(argv[1], "cacert", strlen("cacert")))
+		return set_cacert(argv[2], argv[3]);
+#endif
+
 	if (argc < 2 || argc > 3)
 		return CMD_RET_USAGE;