| Message ID | 20241102163259.305802-1-heinrich.schuchardt@canonical.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | [1/1] tools: use cryptographically safe RNG | expand |
On Sat, Nov 02, 2024 at 05:32:59PM +0100, Heinrich Schuchardt wrote: > The PRNG implementing the random() function only has 2^31 states and > therefore is unsafe to use for cryptography. Use arc4random() instead. > > Fixes: cc34f04efd63 ("tools: image-host.c: use random instead of rand") > Addresses-Coverity-ID: 312953 Calling risky function > Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com> > --- > tools/image-host.c | 35 +++-------------------------------- > 1 file changed, 3 insertions(+), 32 deletions(-) Now I get: /home/uboot/u-boot/u-boot/tools/image-host.c: In function 'fit_image_setup_cipher': /home/uboot/u-boot/u-boot/tools/image-host.c:439:17: warning: implicit declaration of function 'arc4random_buf' [-Wimplicit-function-declaration] 439 | arc4random_buf((void *)info->iv, info->cipher->iv_len); | ^~~~~~~~~~~~~~ /usr/bin/ld: tools/image-host.o: in function `fit_image_cipher_data': image-host.c:(.text+0xb41): undefined reference to `arc4random_buf' collect2: error: ld returned 1 exit status make[3]: *** [scripts/Makefile.host:104: tools/dumpimage] Error 1 in the docker container. I gather this means arc4random_buf is not as widely available as assumed.
Tom Rini <trini@konsulko.com> schrieb am Do., 14. Nov. 2024, 18:27: > On Sat, Nov 02, 2024 at 05:32:59PM +0100, Heinrich Schuchardt wrote: > > > The PRNG implementing the random() function only has 2^31 states and > > therefore is unsafe to use for cryptography. Use arc4random() instead. > > > > Fixes: cc34f04efd63 ("tools: image-host.c: use random instead of rand") > > Addresses-Coverity-ID: 312953 Calling risky function > > Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com> > > --- > > tools/image-host.c | 35 +++-------------------------------- > > 1 file changed, 3 insertions(+), 32 deletions(-) > > Now I get: > /home/uboot/u-boot/u-boot/tools/image-host.c: In function > 'fit_image_setup_cipher': > /home/uboot/u-boot/u-boot/tools/image-host.c:439:17: warning: implicit > declaration of function 'arc4random_buf' [-Wimplicit-function-declaration] > 439 | arc4random_buf((void *)info->iv, > info->cipher->iv_len); > | ^~~~~~~~~~~~~~ > /usr/bin/ld: tools/image-host.o: in function `fit_image_cipher_data': > image-host.c:(.text+0xb41): undefined reference to `arc4random_buf' > collect2: error: ld returned 1 exit status > make[3]: *** [scripts/Makefile.host:104: tools/dumpimage] Error 1 > > in the docker container. I gather this means arc4random_buf is not as > widely available as assumed. > glibc 2.36 is required published 2022-08. Ubuntu Jammy is 22.04. Besr regards Heinrich > -- > Tom >
On Thu, Nov 14, 2024 at 06:35:44PM +0100, Heinrich Schuchardt wrote: > Tom Rini <trini@konsulko.com> schrieb am Do., 14. Nov. 2024, 18:27: > > > On Sat, Nov 02, 2024 at 05:32:59PM +0100, Heinrich Schuchardt wrote: > > > > > The PRNG implementing the random() function only has 2^31 states and > > > therefore is unsafe to use for cryptography. Use arc4random() instead. > > > > > > Fixes: cc34f04efd63 ("tools: image-host.c: use random instead of rand") > > > Addresses-Coverity-ID: 312953 Calling risky function > > > Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com> > > > --- > > > tools/image-host.c | 35 +++-------------------------------- > > > 1 file changed, 3 insertions(+), 32 deletions(-) > > > > Now I get: > > /home/uboot/u-boot/u-boot/tools/image-host.c: In function > > 'fit_image_setup_cipher': > > /home/uboot/u-boot/u-boot/tools/image-host.c:439:17: warning: implicit > > declaration of function 'arc4random_buf' [-Wimplicit-function-declaration] > > 439 | arc4random_buf((void *)info->iv, > > info->cipher->iv_len); > > | ^~~~~~~~~~~~~~ > > /usr/bin/ld: tools/image-host.o: in function `fit_image_cipher_data': > > image-host.c:(.text+0xb41): undefined reference to `arc4random_buf' > > collect2: error: ld returned 1 exit status > > make[3]: *** [scripts/Makefile.host:104: tools/dumpimage] Error 1 > > > > in the docker container. I gather this means arc4random_buf is not as > > widely available as assumed. > > > > glibc 2.36 is required published 2022-08. Ubuntu Jammy is 22.04. Yeah, that's likely (a) too new and (b) strange because: https://source.denx.de/u-boot/u-boot/-/jobs/945810 and so is jammy 22.04.
> Date: Thu, 14 Nov 2024 11:39:27 -0600 > From: Tom Rini <trini@konsulko.com> > > On Thu, Nov 14, 2024 at 06:35:44PM +0100, Heinrich Schuchardt wrote: > > Tom Rini <trini@konsulko.com> schrieb am Do., 14. Nov. 2024, 18:27: > > > > > On Sat, Nov 02, 2024 at 05:32:59PM +0100, Heinrich Schuchardt wrote: > > > > > > > The PRNG implementing the random() function only has 2^31 states and > > > > therefore is unsafe to use for cryptography. Use arc4random() instead. > > > > > > > > Fixes: cc34f04efd63 ("tools: image-host.c: use random instead of rand") > > > > Addresses-Coverity-ID: 312953 Calling risky function > > > > Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com> > > > > --- > > > > tools/image-host.c | 35 +++-------------------------------- > > > > 1 file changed, 3 insertions(+), 32 deletions(-) > > > > > > Now I get: > > > /home/uboot/u-boot/u-boot/tools/image-host.c: In function > > > 'fit_image_setup_cipher': > > > /home/uboot/u-boot/u-boot/tools/image-host.c:439:17: warning: implicit > > > declaration of function 'arc4random_buf' [-Wimplicit-function-declaration] > > > 439 | arc4random_buf((void *)info->iv, > > > info->cipher->iv_len); > > > | ^~~~~~~~~~~~~~ > > > /usr/bin/ld: tools/image-host.o: in function `fit_image_cipher_data': > > > image-host.c:(.text+0xb41): undefined reference to `arc4random_buf' > > > collect2: error: ld returned 1 exit status > > > make[3]: *** [scripts/Makefile.host:104: tools/dumpimage] Error 1 > > > > > > in the docker container. I gather this means arc4random_buf is not as > > > widely available as assumed. > > > > > > > glibc 2.36 is required published 2022-08. Ubuntu Jammy is 22.04. > > Yeah, that's likely (a) too new and (b) strange because: > https://source.denx.de/u-boot/u-boot/-/jobs/945810 and so is jammy > 22.04. Linking against libbsd might be an alternative on older systems.
On Fri, Nov 15 2024, Mark Kettenis <mark.kettenis@xs4all.nl> wrote: >> Date: Thu, 14 Nov 2024 11:39:27 -0600 >> From: Tom Rini <trini@konsulko.com> >> >> On Thu, Nov 14, 2024 at 06:35:44PM +0100, Heinrich Schuchardt wrote: >> > Tom Rini <trini@konsulko.com> schrieb am Do., 14. Nov. 2024, 18:27: >> > >> > > On Sat, Nov 02, 2024 at 05:32:59PM +0100, Heinrich Schuchardt wrote: >> > > >> > > > The PRNG implementing the random() function only has 2^31 states and >> > > > therefore is unsafe to use for cryptography. Use arc4random() instead. >> > > > >> > > > Fixes: cc34f04efd63 ("tools: image-host.c: use random instead of rand") >> > > > Addresses-Coverity-ID: 312953 Calling risky function >> > > > Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com> >> > > > --- >> > > > tools/image-host.c | 35 +++-------------------------------- >> > > > 1 file changed, 3 insertions(+), 32 deletions(-) >> > > >> > > Now I get: >> > > /home/uboot/u-boot/u-boot/tools/image-host.c: In function >> > > 'fit_image_setup_cipher': >> > > /home/uboot/u-boot/u-boot/tools/image-host.c:439:17: warning: implicit >> > > declaration of function 'arc4random_buf' [-Wimplicit-function-declaration] >> > > 439 | arc4random_buf((void *)info->iv, >> > > info->cipher->iv_len); >> > > | ^~~~~~~~~~~~~~ >> > > /usr/bin/ld: tools/image-host.o: in function `fit_image_cipher_data': >> > > image-host.c:(.text+0xb41): undefined reference to `arc4random_buf' >> > > collect2: error: ld returned 1 exit status >> > > make[3]: *** [scripts/Makefile.host:104: tools/dumpimage] Error 1 >> > > >> > > in the docker container. I gather this means arc4random_buf is not as >> > > widely available as assumed. >> > > >> > >> > glibc 2.36 is required published 2022-08. Ubuntu Jammy is 22.04. >> >> Yeah, that's likely (a) too new and (b) strange because: >> https://source.denx.de/u-boot/u-boot/-/jobs/945810 and so is jammy >> 22.04. > > Linking against libbsd might be an alternative on older systems. Or use getrandom(), which according to the man page has been exposed via glibc since glibc 2.25. Or just read from /dev/urandom which should work everywhere. Rasmus
Rasmus Villemoes <ravi@prevas.dk> schrieb am Fr., 15. Nov. 2024, 08:18: > On Fri, Nov 15 2024, Mark Kettenis <mark.kettenis@xs4all.nl> wrote: > > >> Date: Thu, 14 Nov 2024 11:39:27 -0600 > >> From: Tom Rini <trini@konsulko.com> > >> > >> On Thu, Nov 14, 2024 at 06:35:44PM +0100, Heinrich Schuchardt wrote: > >> > Tom Rini <trini@konsulko.com> schrieb am Do., 14. Nov. 2024, 18:27: > >> > > >> > > On Sat, Nov 02, 2024 at 05:32:59PM +0100, Heinrich Schuchardt wrote: > >> > > > >> > > > The PRNG implementing the random() function only has 2^31 states > and > >> > > > therefore is unsafe to use for cryptography. Use arc4random() > instead. > >> > > > > >> > > > Fixes: cc34f04efd63 ("tools: image-host.c: use random instead of > rand") > >> > > > Addresses-Coverity-ID: 312953 Calling risky function > >> > > > Signed-off-by: Heinrich Schuchardt < > heinrich.schuchardt@canonical.com> > >> > > > --- > >> > > > tools/image-host.c | 35 +++-------------------------------- > >> > > > 1 file changed, 3 insertions(+), 32 deletions(-) > >> > > > >> > > Now I get: > >> > > /home/uboot/u-boot/u-boot/tools/image-host.c: In function > >> > > 'fit_image_setup_cipher': > >> > > /home/uboot/u-boot/u-boot/tools/image-host.c:439:17: warning: > implicit > >> > > declaration of function 'arc4random_buf' > [-Wimplicit-function-declaration] > >> > > 439 | arc4random_buf((void *)info->iv, > >> > > info->cipher->iv_len); > >> > > | ^~~~~~~~~~~~~~ > >> > > /usr/bin/ld: tools/image-host.o: in function > `fit_image_cipher_data': > >> > > image-host.c:(.text+0xb41): undefined reference to `arc4random_buf' > >> > > collect2: error: ld returned 1 exit status > >> > > make[3]: *** [scripts/Makefile.host:104: tools/dumpimage] Error 1 > >> > > > >> > > in the docker container. I gather this means arc4random_buf is not > as > >> > > widely available as assumed. > >> > > > >> > > >> > glibc 2.36 is required published 2022-08. Ubuntu Jammy is 22.04. > >> > >> Yeah, that's likely (a) too new and (b) strange because: > >> https://source.denx.de/u-boot/u-boot/-/jobs/945810 and so is jammy > >> 22.04. > > > > Linking against libbsd might be an alternative on older systems. > > Or use getrandom(), which according to the man page has been exposed via > glibc since glibc 2.25. Or just read from /dev/urandom which should work > everywhere. > > Rasmus > /dev/urandom is not available in containers. getrandom is not available in OpenBSD. Best regards
On Fri, Nov 15 2024, Heinrich Schuchardt <heinrich.schuchardt@canonical.com> wrote: >> > >> > Linking against libbsd might be an alternative on older systems. >> >> Or use getrandom(), which according to the man page has been exposed via >> glibc since glibc 2.25. Or just read from /dev/urandom which should work >> everywhere. >> >> Rasmus >> > > > /dev/urandom is not available in containers. What container runtime doesn't provide such basic nodes to containers? Is /dev/null also not available in those containers? Strange. And how, in that case, would the C library (or libbsd, or whatever implements arc4random) then actually obtain random bytes to hand out or seed its internal state? Using arc4random() or rand48() or xkcd221() doesn't fix lack of access to proper random numbers, it may just hide the problem and silence some static checker that knows "random() is bad!", but doesn't know that arc4random() might be just as bad if used in a crippled environment. Rasmus
On Fri, 15 Nov 2024 09:34:54 +0100 Heinrich Schuchardt <heinrich.schuchardt@canonical.com> wrote: > Rasmus Villemoes <ravi@prevas.dk> schrieb am Fr., 15. Nov. 2024, > 08:18: [...] > > Or use getrandom(), which according to the man page has been +1 > > exposed via glibc since glibc 2.25. Or just read from /dev/urandom > > which should work everywhere. > > /dev/urandom is not available in containers. > getrandom is not available in OpenBSD. https://man.openbsd.org/OpenBSD-6.0/getentropy Yes, "getentropy() is not intended for regular code; [...]", but when in doubt and there is no one size that fits all, there's nothing wrong with a good old #ifdef or a compat wrapper IMHO. Torsten
> From: Rasmus Villemoes <ravi@prevas.dk> > Date: Fri, 15 Nov 2024 08:18:17 +0100 > > On Fri, Nov 15 2024, Mark Kettenis <mark.kettenis@xs4all.nl> wrote: > > >> Date: Thu, 14 Nov 2024 11:39:27 -0600 > >> From: Tom Rini <trini@konsulko.com> > >> > >> On Thu, Nov 14, 2024 at 06:35:44PM +0100, Heinrich Schuchardt wrote: > >> > Tom Rini <trini@konsulko.com> schrieb am Do., 14. Nov. 2024, 18:27: > >> > > >> > > On Sat, Nov 02, 2024 at 05:32:59PM +0100, Heinrich Schuchardt wrote: > >> > > > >> > > > The PRNG implementing the random() function only has 2^31 states and > >> > > > therefore is unsafe to use for cryptography. Use arc4random() instead. > >> > > > > >> > > > Fixes: cc34f04efd63 ("tools: image-host.c: use random instead of rand") > >> > > > Addresses-Coverity-ID: 312953 Calling risky function > >> > > > Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com> > >> > > > --- > >> > > > tools/image-host.c | 35 +++-------------------------------- > >> > > > 1 file changed, 3 insertions(+), 32 deletions(-) > >> > > > >> > > Now I get: > >> > > /home/uboot/u-boot/u-boot/tools/image-host.c: In function > >> > > 'fit_image_setup_cipher': > >> > > /home/uboot/u-boot/u-boot/tools/image-host.c:439:17: warning: implicit > >> > > declaration of function 'arc4random_buf' [-Wimplicit-function-declaration] > >> > > 439 | arc4random_buf((void *)info->iv, > >> > > info->cipher->iv_len); > >> > > | ^~~~~~~~~~~~~~ > >> > > /usr/bin/ld: tools/image-host.o: in function `fit_image_cipher_data': > >> > > image-host.c:(.text+0xb41): undefined reference to `arc4random_buf' > >> > > collect2: error: ld returned 1 exit status > >> > > make[3]: *** [scripts/Makefile.host:104: tools/dumpimage] Error 1 > >> > > > >> > > in the docker container. I gather this means arc4random_buf is not as > >> > > widely available as assumed. > >> > > > >> > > >> > glibc 2.36 is required published 2022-08. Ubuntu Jammy is 22.04. > >> > >> Yeah, that's likely (a) too new and (b) strange because: > >> https://source.denx.de/u-boot/u-boot/-/jobs/945810 and so is jammy > >> 22.04. > > > > Linking against libbsd might be an alternative on older systems. > > Or use getrandom(), which according to the man page has been exposed via > glibc since glibc 2.25. Or just read from /dev/urandom which should work > everywhere. $ man getrandom man: No entry for getrandom in the manual.
On Fri, Nov 15 2024, Mark Kettenis <mark.kettenis@xs4all.nl> wrote: >> >> Or use getrandom(), which according to the man page has been exposed via >> glibc since glibc 2.25. Or just read from /dev/urandom which should work >> everywhere. > > $ man getrandom > man: No entry for getrandom in the manual. I assume this is intended to inform me that getrandom() doesn't exist on *BSD? As I said, reading from /dev/urandom is probably better as that also works on BSDs automatically. If somebody tries to do crypto stuff in an environment where they've removed such a basic device node, they get to keep both pieces (i.e. the code should just fail) Rasmus
> From: Rasmus Villemoes <ravi@prevas.dk> > Date: Fri, 15 Nov 2024 20:21:41 +0100 > > On Fri, Nov 15 2024, Mark Kettenis <mark.kettenis@xs4all.nl> wrote: > > >> > >> Or use getrandom(), which according to the man page has been exposed via > >> glibc since glibc 2.25. Or just read from /dev/urandom which should work > >> everywhere. > > > > $ man getrandom > > man: No entry for getrandom in the manual. > > I assume this is intended to inform me that getrandom() doesn't exist on > *BSD? Right. We have getentropy(3) though and that made it into the recent POSIX update. > As I said, reading from /dev/urandom is probably better as that also > works on BSDs automatically. If somebody tries to do crypto stuff in an > environment where they've removed such a basic device node, they get to > keep both pieces (i.e. the code should just fail) Should work on OpenBSD, but it is still unportable.
diff --git a/tools/image-host.c b/tools/image-host.c index 5e01b853c50..e24e053825b 100644 --- a/tools/image-host.c +++ b/tools/image-host.c @@ -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,9 @@ 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 random IV */ + arc4random_buf((void *)info->iv, info->cipher->iv_len); + ret = 0; } out:
The PRNG implementing the random() function only has 2^31 states and therefore is unsafe to use for cryptography. Use arc4random() instead. Fixes: cc34f04efd63 ("tools: image-host.c: use random instead of rand") Addresses-Coverity-ID: 312953 Calling risky function Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com> --- tools/image-host.c | 35 +++-------------------------------- 1 file changed, 3 insertions(+), 32 deletions(-)