[PATCH 1/1] tools: use cryptographically safe RNG

[PATCH 1/1] tools: use cryptographically safe RNG

[Heinrich Schuchardt]

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(-)

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:
-- 
2.45.2

Re: [PATCH 1/1] tools: use cryptographically safe RNG

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1284 bytes --]

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 1/1] tools: use cryptographically safe RNG

[Heinrich Schuchardt]

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
>

Re: [PATCH 1/1] tools: use cryptographically safe RNG

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1749 bytes --]

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.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 1/1] tools: use cryptographically safe RNG

[Mark Kettenis]

> 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.

Re: [PATCH 1/1] tools: use cryptographically safe RNG

[Rasmus Villemoes]

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

Re: [PATCH 1/1] tools: use cryptographically safe RNG

[Heinrich Schuchardt]

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

Re: [PATCH 1/1] tools: use cryptographically safe RNG

[Rasmus Villemoes]

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

Re: [PATCH 1/1] tools: use cryptographically safe RNG

[Torsten Duwe]

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

Re: [PATCH 1/1] tools: use cryptographically safe RNG

[Mark Kettenis]

> 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.

Re: [PATCH 1/1] tools: use cryptographically safe RNG

[Rasmus Villemoes]

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

Re: [PATCH 1/1] tools: use cryptographically safe RNG

[Mark Kettenis]

> 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.