[PATCH 4/5] net: lwip: add support for built-in root certificates

[Ilias Apalodimas]

Hi Jerome,

On Thu, 27 Feb 2025 at 18:38, Jerome Forissier
<jerome.forissier at linaro.org> wrote:
>
> Sorry for replying to myself, I spotted a small mistake.
>
> On 2/27/25 17:09, Jerome Forissier wrote:
> > Introduce Kconfig symbols WGET_BUILTIN_CACERT and
> > WGET_BUILTIN_CACERT_PATH to provide root certificates at build time. The
> > file may be a DER-encoded (.crt) or PEM-encoded (.pem) X509 collection
> > of one or more certificates. PEM encoding needs MBEDTLS_LIB_X509_PEM.

I understand that for downloaded certificates supporting both is
convenient since DER might not be available. But why don't we limit
our built-in options to DER only, which is smaller anyway?

[...]


> > --- a/cmd/net-lwip.c
> > +++ b/cmd/net-lwip.c
> > @@ -39,6 +39,10 @@ U_BOOT_CMD(wget, 4, 1, do_wget,
> >  #if defined(CONFIG_WGET_CACERT)
> >          "\nwget cacert <address> <length>\n"
> >          "    - provide CA certificates (0 0 to disable verification)"
> > +#if defined(CONFIG_WGET_BUILTIN_CACERT)
> > +        "\nwget cacert builtin\n"
> > +        "    - use the builtin CA certificates"
> > +#endif
> >  #endif
> >  );
> >  #endif
> > diff --git a/net/lwip/Makefile b/net/lwip/Makefile
> > index 79dd6b3fb50..950c5316bb9 100644
> > --- a/net/lwip/Makefile
> > +++ b/net/lwip/Makefile
> > @@ -6,3 +6,9 @@ obj-$(CONFIG_CMD_DNS) += dns.o
> >  obj-$(CONFIG_CMD_PING) += ping.o
> >  obj-$(CONFIG_CMD_TFTPBOOT) += tftp.o
> >  obj-$(CONFIG_WGET) += wget.o
> > +
> > +ifeq (y,$(CONFIG_WGET_BUILTIN_CACERT))
> > +$(obj)/builtin_cacert.c: $(CONFIG_WGET_BUILTIN_CACERT_PATH:"%"=%) FORCE
> > +     $(call if_changed,bin2c,builtin_cacert)
> > +obj-y += builtin_cacert.o
> > +endif
> > diff --git a/net/lwip/wget.c b/net/lwip/wget.c
> > index 14466598d7c..f24aa9c2380 100644
> > --- a/net/lwip/wget.c
> > +++ b/net/lwip/wget.c
> > @@ -288,31 +288,34 @@ static err_t httpc_headers_done_cb(httpc_state_t *connection, void *arg, struct
> >  #if defined CONFIG_WGET_HTTPS
> >  static char *cacert;
> >  size_t cacert_size;
> > +
> > +#if defined CONFIG_WGET_BUILTIN_CACERT
> > +extern char builtin_cacert[];

const as well? IIRC all the bin2 generated variables end up in .rodata

> > +extern const size_t builtin_cacert_size;
> > +static bool cacert_initialized;
> > +#endif
> >  #endif
> >
> > -#if defined CONFIG_WGET_CACERT
> > -static int set_cacert(char * const saddr, char * const ssz)
> > +#if defined CONFIG_WGET_CACERT || defined CONFIG_WGET_BUILTIN_CACERT
> > +static int _set_cacert(void *addr, size_t sz)
> >  {
> >       mbedtls_x509_crt crt;
> > -     ulong addr, sz;
> > +     void *p;
> >       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)
>
> HERE...
>
> > +     p = malloc(sz);
> > +     if (!p)
> >               return CMD_RET_FAILURE;
> > +     cacert = p;
> >       cacert_size = sz;
> >
> >       memcpy(cacert, (void *)addr, sz - 1);
> > @@ -328,10 +331,33 @@ static int set_cacert(char * const saddr, char * const ssz)
> >               return CMD_RET_FAILURE;
> >       }
> >
> > +#if defined CONFIG_WGET_BUILTIN_CACERT
> > +     cacert_initialized = true;
> > +#endif
> >       return CMD_RET_SUCCESS;
> >  }
> > +
> > +#if defined CONFIG_WGET_BUILTIN_CACERT
> > +static int set_cacert_builtin(void)
> > +{
> > +     return _set_cacert(builtin_cacert, builtin_cacert_size);
> > +}
> >  #endif
> >
> > +#if defined CONFIG_WGET_CACERT
> > +static int set_cacert(char * const saddr, char * const ssz)
> > +{
> > +     ulong addr, sz;
> > +
> > +     addr = hextoul(saddr, NULL);
> > +     sz = hextoul(ssz, NULL);
> > +     sz++; /* For the trailing '\0' in case of a text (PEM) file */
>
> This line should have been moved to _set_cacert() at the place I marked
> "HERE" above.
>
> The reason for this hack is, before even attempting to parse a file as
> PEM format (which is text-based), mbedtls_x509_crt_parse() checks that
> buf[buflen - 1] == '\0'. When a text file is obtained via wget there is
> no null terminator. Same when using bin2c. So adding the '\0' is
> necessary.

and if we do DER only for built-in this goes away as well

Thanks
/Ilias