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

[Jerome Forissier]

On 3/10/25 12:52, Ilias Apalodimas wrote:
> Hi Jerome,
> 
> [...]
> 
> 
>>>>
>>>> +#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?
>>
>> The built-in certs are enabled only when cacert_initialized is false.
>> set_cacert_builtin() will set it to true (via _set_cacert()), so it will
>> happen only once. Note also that any successful "wget cacert" command
>> will also do the same. So effectively these two lines enable the
>> built-in certificates by default, that's all they do.
> 
> Ok, so if you download a cert in memory and have u-boot with a builtin
> certificate, then the memory one will be overwritten in the first run.

No, because the downloaded cert must have be made active via "wget cacert
<addr> <size>", which will set cacert_initialized to true, and thus the
built-in certs won't overwrite them. Or did I miss something?

Cheers,
-- 
Jerome

> This is not easy to solve, I was trying to think of ways to make the
> functionality clearer to users.
> 
> Cheers
> /Ilias
>>
>> Cheers,
>> --
>> Jerome
>>
>>>
>>> [...]
>>>
>>> Cheers
>>> /Ilias