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

[Ilias Apalodimas]

Hi Jerome

On Wed, 5 Mar 2025 at 16:27, Jerome Forissier
<jerome.forissier at linaro.org> wrote:
>

[...]

> @@ -304,28 +304,34 @@ static int set_auth(enum auth_mode auth)
>
>         return CMD_RET_SUCCESS;
>  }
> +#endif
>
> -static int set_cacert(char * const saddr, char * const ssz)
> +#if CONFIG_IS_ENABLED(WGET_BUILTIN_CACERT)
> +extern const char builtin_cacert[];
> +extern const size_t builtin_cacert_size;
> +static bool cacert_initialized;
> +#endif

These are better off under WGET_CACERT || WGET_BUILTIN_CACERT ?

> +
> +#if CONFIG_IS_ENABLED(WGET_CACERT) || CONFIG_IS_ENABLED(WGET_BUILTIN_CACERT)
> +static int _set_cacert(const 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);
> -
>         if (!addr) {
>                 cacert = NULL;
>                 cacert_size = 0;
>                 return CMD_RET_SUCCESS;
>         }
>
> -       cacert = malloc(sz);
> -       if (!cacert)
> +       p = malloc(sz);
> +       if (!p)
>                 return CMD_RET_FAILURE;
> +       cacert = p;
>         cacert_size = sz;
>
>         memcpy(cacert, (void *)addr, sz);
> @@ -340,10 +346,32 @@ static int set_cacert(char * const saddr, char * const ssz)
>                 return CMD_RET_FAILURE;
>         }
>
> +#if CONFIG_IS_ENABLED(WGET_BUILTIN_CACERT)
> +       cacert_initialized = true;
> +#endif
>         return CMD_RET_SUCCESS;
>  }
> +
> +#if CONFIG_IS_ENABLED(WGET_BUILTIN_CACERT)
> +static int set_cacert_builtin(void)
> +{
> +       return _set_cacert(builtin_cacert, builtin_cacert_size);
> +}
>  #endif
>
> +#if CONFIG_IS_ENABLED(WGET_CACERT)
> +static int set_cacert(char * const saddr, char * const ssz)
> +{
> +       ulong addr, sz;
> +
> +       addr = hextoul(saddr, NULL);
> +       sz = hextoul(ssz, NULL);
> +
> +       return _set_cacert((void *)addr, sz);
> +}
> +#endif
> +#endif  /* CONFIG_WGET_CACERT || CONFIG_WGET_BUILTIN_CACERT */
> +
>  static int wget_loop(struct udevice *udev, ulong dst_addr, char *uri)
>  {
>  #if CONFIG_IS_ENABLED(WGET_HTTPS)
> @@ -373,8 +401,15 @@ static int wget_loop(struct udevice *udev, ulong dst_addr, char *uri)
>         memset(&conn, 0, sizeof(conn));
>  #if CONFIG_IS_ENABLED(WGET_HTTPS)
>         if (is_https) {
> -               char *ca = cacert;
> -               size_t ca_sz = cacert_size;
> +               char *ca;
> +               size_t ca_sz;
> +
> +#if CONFIG_IS_ENABLED(WGET_BUILTIN_CACERT)
> +               if (!cacert_initialized)
> +                       set_cacert_builtin();
> +#endif

The code and the rest of the patch seems fine, but the builtin vs
downloaded cert is a bit confusing here.
Since the built-in cert always gets initialized in the wget loop it
would overwrite any certificates that are downloaded in memory?

[...]

Cheers
/Ilias