| Message ID | 20241114073514.418723-1-ilias.apalodimas@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | net: lwip: Change https entropy requests | expand |
On 11/14/24 08:35, Ilias Apalodimas wrote: > mbedTLS requires some randomess in order to setup a TLS conection. MBed TLS, randomness, connection > Since we don't have known APIs -- e.g /dev/urandom, we must define > our own function which mbedTLS uses. > The crypto library will call that function recursively until it gets all s/recursively/repeatedly/ > the randomness it needs. Instead of doing it in 8b chunks fill in whatever > mbedTLS asks for in one call. > > It's worth noting that 'len' in this function is controlled by mbedTLS > at build-time options and currently defaults to 128b. The description does not reflect well what the patch does and why IMO. The function already exists, there is no need to explain that. How about: === net: lwip: provide entropy to MBed TLS in one go Take into account the 'len' parameter passed by MBed TLS to the entropy gathering function instead of doing 8-byte chunks. Note that the current code works because len is always 16 (defined at compile time), therefore mbedtls_hardware_poll() is called twice and the buffer is filled exactly. But passing 'len' to dm_rng_read() is both more correct and simpler. === > > Suggested-by: Simon Glass <sjg@chromium.org> > Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> > --- > net/lwip/wget.c | 6 ++---- > 1 file changed, 2 insertions(+), 4 deletions(-) With or without an re-worded description: Reviewed-by: Jerome Forissier <jerome.forissier@linaro.org> Thanks,
Hi Jerome, On Thu, 14 Nov 2024 at 10:48, Jerome Forissier <jerome.forissier@linaro.org> wrote: > > > > On 11/14/24 08:35, Ilias Apalodimas wrote: > > mbedTLS requires some randomess in order to setup a TLS conection. > > MBed TLS, randomness, connection > > > Since we don't have known APIs -- e.g /dev/urandom, we must define > > our own function which mbedTLS uses. > > The crypto library will call that function recursively until it gets all > > s/recursively/repeatedly/ > > > the randomness it needs. Instead of doing it in 8b chunks fill in whatever > > mbedTLS asks for in one call. > > > > It's worth noting that 'len' in this function is controlled by mbedTLS > > at build-time options and currently defaults to 128b. > > The description does not reflect well what the patch does and why IMO. > The function already exists, there is no need to explain that. How about: > > === > net: lwip: provide entropy to MBed TLS in one go > > Take into account the 'len' parameter passed by MBed TLS to the entropy > gathering function instead of doing 8-byte chunks. Note that the current > code works because len is always 16 (defined at compile time), therefore > mbedtls_hardware_poll() is called twice and the buffer is filled > exactly. But passing 'len' to dm_rng_read() is both more correct and > simpler. Sure, I can send a v2, with a minor change, len is 128 currently not 16 Thanks /Ilias > === > > > > > Suggested-by: Simon Glass <sjg@chromium.org> > > Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> > > --- > > net/lwip/wget.c | 6 ++---- > > 1 file changed, 2 insertions(+), 4 deletions(-) > > With or without an re-worded description: > > Reviewed-by: Jerome Forissier <jerome.forissier@linaro.org> > > Thanks, > -- > Jerome > > > > > diff --git a/net/lwip/wget.c b/net/lwip/wget.c > > index ba8579899002..4fd552fd306e 100644 > > --- a/net/lwip/wget.c > > +++ b/net/lwip/wget.c > > @@ -42,7 +42,6 @@ int mbedtls_hardware_poll(void *data, unsigned char *output, size_t len, > > size_t *olen) > > { > > struct udevice *dev; > > - u64 rng = 0; > > int ret; > > > > *olen = 0; > > @@ -52,12 +51,11 @@ int mbedtls_hardware_poll(void *data, unsigned char *output, size_t len, > > log_err("Failed to get an rng: %d\n", ret); > > return ret; > > } > > - ret = dm_rng_read(dev, &rng, sizeof(rng)); > > + ret = dm_rng_read(dev, output, len); > > if (ret) > > return ret; > > > > - memcpy(output, &rng, len); > > - *olen = sizeof(rng); > > + *olen = len; > > > > return 0; > > }
diff --git a/net/lwip/wget.c b/net/lwip/wget.c index ba8579899002..4fd552fd306e 100644 --- a/net/lwip/wget.c +++ b/net/lwip/wget.c @@ -42,7 +42,6 @@ int mbedtls_hardware_poll(void *data, unsigned char *output, size_t len, size_t *olen) { struct udevice *dev; - u64 rng = 0; int ret; *olen = 0; @@ -52,12 +51,11 @@ int mbedtls_hardware_poll(void *data, unsigned char *output, size_t len, log_err("Failed to get an rng: %d\n", ret); return ret; } - ret = dm_rng_read(dev, &rng, sizeof(rng)); + ret = dm_rng_read(dev, output, len); if (ret) return ret; - memcpy(output, &rng, len); - *olen = sizeof(rng); + *olen = len; return 0; }
mbedTLS requires some randomess in order to setup a TLS conection. Since we don't have known APIs -- e.g /dev/urandom, we must define our own function which mbedTLS uses. The crypto library will call that function recursively until it gets all the randomness it needs. Instead of doing it in 8b chunks fill in whatever mbedTLS asks for in one call. It's worth noting that 'len' in this function is controlled by mbedTLS at build-time options and currently defaults to 128b. Suggested-by: Simon Glass <sjg@chromium.org> Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> --- net/lwip/wget.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)