[PATCH v3 4/6] net: lwip: Enable https:// support for wget

[Simon Glass]

HI Ilias,

On Mon, 11 Nov 2024 at 07:20, Ilias Apalodimas
<ilias.apalodimas at linaro.org> wrote:
>
> Thanks Simon,
>
> On Mon, 11 Nov 2024 at 15:03, Simon Glass <sjg at chromium.org> wrote:
> >
> > On Sun, 10 Nov 2024 at 01:32, Ilias Apalodimas
> > <ilias.apalodimas at linaro.org> wrote:
> > >
> > > With the recent changes of lwip & mbedTLS we can now download from
> > > https:// urls instead of just http://.
> > > Adjust our wget lwip version parsing to support both URLs.
> > > While at it adjust the default TCP window for QEMU since https seems to
> > > require at least 16384
> > >
> > > Signed-off-by: Ilias Apalodimas <ilias.apalodimas at linaro.org>
> > > ---
> > >  cmd/Kconfig      | 19 +++++++++++
> > >  net/lwip/Kconfig |  2 +-
> > >  net/lwip/wget.c  | 86 +++++++++++++++++++++++++++++++++++++++++++-----
> > >  3 files changed, 97 insertions(+), 10 deletions(-)
> >
> > Reviewed-by: Simon Glass <sjg at chromium.org>
> >
> > Some nits / questions
> >
> > I am not sure what mbedtls_hardware_poll() is,
>
> It's an entropy collector used by mbedTLS to ensure the platform has
> enough entropy.
> It's required if your platform doesn't support standards like the
> /dev/urandom or Windows CryptoAPI.
>
> > but if @len is too
> > short, would it be acceptable to return an error? How many bytes is it
> > requesting in the https case?
>
> If you don't return enough entropy https:// will fail and mbedTLS &
> lwIP will print an error. I think we currently use 128 and the default
> for mbedTLS is 32.

OK, then the code is quite strange to me. It seems like it should
check that 'len' is large enough.

But what does 'len' actually mean? Its arguments are not described in
mbedtls. Shouldn't you just pass it on to the dm_rng_read() function?
Why call it with only 8 bytes? It might be more bytes than is
requested (and larger than the buffer), or fewer. It seems that your
function is written with knowledge of the internals of mbedtls.

BTW mbedtls_hardware_poll() is a strange name as it actually reads
random data, so I think it should be renamed.

>
> >
> > uclass_first_device() if you want the first device, not uclass_get_device(...0)
>
> Sure
>

Regards,
SImon