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

[Ilias Apalodimas]

On Mon, 10 Mar 2025 at 14:48, Jerome Forissier
<jerome.forissier at linaro.org> wrote:
>
>
>
> On 3/10/25 13:38, Ilias Apalodimas wrote:
> > On Mon, 10 Mar 2025 at 14:13, Jerome Forissier
> > <jerome.forissier at linaro.org> wrote:
> >>
> >>
> >>
> >> 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?
> >
> > Nop I did, when reading the patch. But should the command that clears
> > the downloaded cert set cacert_initialized; to false now?
>
> It's probably easier if it does not, so that "wget cacert 0 0" really clears
> the certs. We have a command to restore the built-in ones ("wget cacert
> builtin").

So right now if  a user defines a builtin cert it will be used on the
first download. If after that he defines a memory one, that will be
used on the next download, but if he unloads it shouldn't the built in
be restired automatically?

Thanks
/Ilias
>
> Thanks,
> --
> Jerome
>
> >
> > Thanks
> > /Ilias
> >>
> >> 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