| Message ID | 20240410233116.57402-1-heinrich.schuchardt@canonical.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | [1/1] tools: use adequate entropy source for initialization vector | expand |
> From: Heinrich Schuchardt <heinrich.schuchardt@canonical.com> > Date: Thu, 11 Apr 2024 01:31:16 +0200 > > The random() function is unsafe to initialize cryptographic data. > Use getrandom() which reads from /dev/urandom instead. > > getrandom() is available on Linux sine release 3.17 and on BSD. NACK getrandom() isn't evailable on OpenBSD. Either use getentropy() or arc4random_buf() instead. > Addresses-Coverity-ID: 312953 Calling risky function > Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com> > --- > tools/image-host.c | 36 +++--------------------------------- > 1 file changed, 3 insertions(+), 33 deletions(-) > > diff --git a/tools/image-host.c b/tools/image-host.c > index b2a0f2e6d16..d30a235baf6 100644 > --- a/tools/image-host.c > +++ b/tools/image-host.c > @@ -13,11 +13,11 @@ > #include <fdt_region.h> > #include <image.h> > #include <version.h> > - > #if CONFIG_IS_ENABLED(FIT_SIGNATURE) > #include <openssl/pem.h> > #include <openssl/evp.h> > #endif > +#include <sys/random.h> > > /** > * fit_set_hash_value - set hash value in requested has node > @@ -364,36 +364,6 @@ static int fit_image_read_key_iv_data(const char *keydir, const char *key_iv_nam > return ret; > } > > -static int get_random_data(void *data, int size) > -{ > - unsigned char *tmp = data; > - struct timespec date; > - int i, ret; > - > - if (!tmp) { > - fprintf(stderr, "%s: pointer data is NULL\n", __func__); > - ret = -1; > - goto out; > - } > - > - ret = clock_gettime(CLOCK_MONOTONIC, &date); > - if (ret) { > - fprintf(stderr, "%s: clock_gettime has failed (%s)\n", __func__, > - strerror(errno)); > - goto out; > - } > - > - srandom(date.tv_nsec); > - > - for (i = 0; i < size; i++) { > - *tmp = random() & 0xff; > - tmp++; > - } > - > - out: > - return ret; > -} > - > static int fit_image_setup_cipher(struct image_cipher_info *info, > const char *keydir, void *fit, > const char *image_name, int image_noffset, > @@ -465,8 +435,8 @@ static int fit_image_setup_cipher(struct image_cipher_info *info, > if (ret < 0) > goto out; > } else { > - /* Generate an ramdom IV */ > - ret = get_random_data((void *)info->iv, info->cipher->iv_len); > + /* Generate a ramdom initialization vector */ > + ret = getrandom((void *)info->iv, info->cipher->iv_len, 0); > } > > out: > -- > 2.43.0 > >
On 4/11/24 01:46, Mark Kettenis wrote: >> From: Heinrich Schuchardt <heinrich.schuchardt@canonical.com> >> Date: Thu, 11 Apr 2024 01:31:16 +0200 >> >> The random() function is unsafe to initialize cryptographic data. >> Use getrandom() which reads from /dev/urandom instead. >> >> getrandom() is available on Linux sine release 3.17 and on BSD. > > NACK > > getrandom() isn't evailable on OpenBSD. Either use getentropy() or > arc4random_buf() instead. Thanks for reviewing. arc4random_buf() was added to glibc only in 2022. So there might be too many users not using a recent enough Linux distro. So I guess getentropy() is a better choice. Best regards Heinrich > >> Addresses-Coverity-ID: 312953 Calling risky function >> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com> >> --- >> tools/image-host.c | 36 +++--------------------------------- >> 1 file changed, 3 insertions(+), 33 deletions(-) >> >> diff --git a/tools/image-host.c b/tools/image-host.c >> index b2a0f2e6d16..d30a235baf6 100644 >> --- a/tools/image-host.c >> +++ b/tools/image-host.c >> @@ -13,11 +13,11 @@ >> #include <fdt_region.h> >> #include <image.h> >> #include <version.h> >> - >> #if CONFIG_IS_ENABLED(FIT_SIGNATURE) >> #include <openssl/pem.h> >> #include <openssl/evp.h> >> #endif >> +#include <sys/random.h> >> >> /** >> * fit_set_hash_value - set hash value in requested has node >> @@ -364,36 +364,6 @@ static int fit_image_read_key_iv_data(const char *keydir, const char *key_iv_nam >> return ret; >> } >> >> -static int get_random_data(void *data, int size) >> -{ >> - unsigned char *tmp = data; >> - struct timespec date; >> - int i, ret; >> - >> - if (!tmp) { >> - fprintf(stderr, "%s: pointer data is NULL\n", __func__); >> - ret = -1; >> - goto out; >> - } >> - >> - ret = clock_gettime(CLOCK_MONOTONIC, &date); >> - if (ret) { >> - fprintf(stderr, "%s: clock_gettime has failed (%s)\n", __func__, >> - strerror(errno)); >> - goto out; >> - } >> - >> - srandom(date.tv_nsec); >> - >> - for (i = 0; i < size; i++) { >> - *tmp = random() & 0xff; >> - tmp++; >> - } >> - >> - out: >> - return ret; >> -} >> - >> static int fit_image_setup_cipher(struct image_cipher_info *info, >> const char *keydir, void *fit, >> const char *image_name, int image_noffset, >> @@ -465,8 +435,8 @@ static int fit_image_setup_cipher(struct image_cipher_info *info, >> if (ret < 0) >> goto out; >> } else { >> - /* Generate an ramdom IV */ >> - ret = get_random_data((void *)info->iv, info->cipher->iv_len); >> + /* Generate a ramdom initialization vector */ >> + ret = getrandom((void *)info->iv, info->cipher->iv_len, 0); >> } >> >> out: >> -- >> 2.43.0 >> >>
diff --git a/tools/image-host.c b/tools/image-host.c index b2a0f2e6d16..d30a235baf6 100644 --- a/tools/image-host.c +++ b/tools/image-host.c @@ -13,11 +13,11 @@ #include <fdt_region.h> #include <image.h> #include <version.h> - #if CONFIG_IS_ENABLED(FIT_SIGNATURE) #include <openssl/pem.h> #include <openssl/evp.h> #endif +#include <sys/random.h> /** * fit_set_hash_value - set hash value in requested has node @@ -364,36 +364,6 @@ static int fit_image_read_key_iv_data(const char *keydir, const char *key_iv_nam return ret; } -static int get_random_data(void *data, int size) -{ - unsigned char *tmp = data; - struct timespec date; - int i, ret; - - if (!tmp) { - fprintf(stderr, "%s: pointer data is NULL\n", __func__); - ret = -1; - goto out; - } - - ret = clock_gettime(CLOCK_MONOTONIC, &date); - if (ret) { - fprintf(stderr, "%s: clock_gettime has failed (%s)\n", __func__, - strerror(errno)); - goto out; - } - - srandom(date.tv_nsec); - - for (i = 0; i < size; i++) { - *tmp = random() & 0xff; - tmp++; - } - - out: - return ret; -} - static int fit_image_setup_cipher(struct image_cipher_info *info, const char *keydir, void *fit, const char *image_name, int image_noffset, @@ -465,8 +435,8 @@ static int fit_image_setup_cipher(struct image_cipher_info *info, if (ret < 0) goto out; } else { - /* Generate an ramdom IV */ - ret = get_random_data((void *)info->iv, info->cipher->iv_len); + /* Generate a ramdom initialization vector */ + ret = getrandom((void *)info->iv, info->cipher->iv_len, 0); } out:
The random() function is unsafe to initialize cryptographic data. Use getrandom() which reads from /dev/urandom instead. getrandom() is available on Linux sine release 3.17 and on BSD. Addresses-Coverity-ID: 312953 Calling risky function Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com> --- tools/image-host.c | 36 +++--------------------------------- 1 file changed, 3 insertions(+), 33 deletions(-)