Re: [PATCH] seedrng: fix getrandom() detection for non-glibc libc

2023-04-19 Thread Thomas Devoogdt
Hi,

I can't follow everything in your mail,
but about:

>> Having said that, the current implementation as it is here is
>> surely wrong, since the fallback getrandom returns ENOSYS, instead
>> of setting errno to ENOSYS?
>> 200a9669fbf6f06894e42439fc11a1a6073a as of -04-10.

Already fixed here:
https://git.busybox.net/busybox/commit/?id=cb57abb46f06f4ede8d9ccbdaac67377fdf416cf

Kind regards,

Thomas
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox


Re: [PATCH] seedrng: fix getrandom() detection for non-glibc libc

2023-04-19 Thread Steffen Nurpmeso
Thomas Devoogdt wrote in
 :
 |About:
 |>> HAVE_GETRANDOM := $(shell printf '#include \n#include \
 |>> \nint main(void){char buf[256];\ngetrandom(buf,sizeof(buf)\
 |>> ,GRND_NONBLOCK);}' >bb_libtest.c; $(CC) $(CFLAGS) $(CFLAGS_busybox) \
 |>> -D_GNU_SOURCE -o /dev/null
 |
 |Is it a good idea to have GRND_NONBLOCK in that check? What if it's
 |not defined, but getrandom() is, then HAVE_GETRANDOM might incorrectly
 |be false?
 |We do anyway define it in seedrng.c if needed.

Heck, this entire thing is grazy from the start.
The kernel random code mutilates input coming via the /dev/*random
devices, instead of treating that as just the same regular input
as through ioctl or whatever (i am too lazy to look again).
They should all simply reject going that route along the random
maintainer, and insist on using

  # Load random seed
  /bin/cat /var/lib/urandom/seed > /dev/urandom

like my Linux distribution does (even though *i* repeatedly
prodded using my entropy thing, then that Friday the 13th thing
for years, before i had to go, mind you).
Moreover, even GLibC 2.36 / current manual pages still does not
document the other flag that the busybox applet uses.
Even on super-async modern boot, some things have to be
synchronized, why not random.  (If i recall correctly, people
knowing about systemd even said that is done, last time i said
that, hm.)

Just my off-topic one cent.  seedrng.c is an intellectually
penetrated piece of software.

Having said that, the current implementation as it is here is
surely wrong, since the fallback getrandom returns ENOSYS, instead
of setting errno to ENOSYS?
200a9669fbf6f06894e42439fc11a1a6073a as of -04-10.

--steffen
|
|Der Kragenbaer,The moon bear,
|der holt sich munter   he cheerfully and one by one
|einen nach dem anderen runter  wa.ks himself off
|(By Robert Gernhardt)
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox


Re: [PATCH] seedrng: fix getrandom() detection for non-glibc libc

2023-04-19 Thread Raphaël Mélotte
Hello,

On 19 April 2023 22:24:13 CEST, Thomas Devoogdt  wrote:
>Is it a good idea to have GRND_NONBLOCK in that check? What if it's
>not defined, but getrandom() is, then HAVE_GETRANDOM might incorrectly
>be false?
>We do anyway define it in seedrng.c if needed.

Indeed, maybe it should be removed.

Kind regards,

Raphaël
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox


Re: [PATCH] seedrng: fix getrandom() detection for non-glibc libc

2023-04-19 Thread Thomas Devoogdt
Hi,

About:
>> HAVE_GETRANDOM := $(shell printf '#include \n#include 
>> \nint main(void){char 
>> buf[256];\ngetrandom(buf,sizeof(buf),GRND_NONBLOCK);}' >bb_libtest.c; $(CC) 
>> $(CFLAGS) $(CFLAGS_busybox) -D_GNU_SOURCE -o /dev/null

Is it a good idea to have GRND_NONBLOCK in that check? What if it's
not defined, but getrandom() is, then HAVE_GETRANDOM might incorrectly
be false?
We do anyway define it in seedrng.c if needed.

Kind regards,

Thomas
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox


Re: [PATCH] seedrng: fix getrandom() detection for non-glibc libc

2023-04-19 Thread Raphaël Mélotte

On 4/19/23 14:52, Kang-Che Sung wrote:

Just my two cents...

On Wednesday, April 19, 2023, Raphaël Mélotte mailto:raphael.melo...@mind.be>> wrote:
 > +# Not all libc versions have getrandom, so check for it.
 > +HAVE_GETRANDOM := $(shell printf '#include \n#include \nint main(void){char 
buf[256];\ngetrandom(buf,sizeof(buf),GRND_NONBLOCK);}' >bb_libtest.c; $(CC) $(CFLAGS) $(CFLAGS_busybox) -D_GNU_SOURCE -o 
/dev/null bb_libtest.c >/dev/null 2>&1 && echo "y"; rm bb_libtest.c)

How about putting the define line '#define _GNU_SOURCE' into bb_libtest.c ? I 
don't like defining the feature test macros externally in '-D' options.


Sure, it's only added here for the test, either should work.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox


Re: [PATCH] seedrng: fix getrandom() detection for non-glibc libc

2023-04-19 Thread Kang-Che Sung
Just my two cents...

On Wednesday, April 19, 2023, Raphaël Mélotte 
wrote:
> +# Not all libc versions have getrandom, so check for it.
> +HAVE_GETRANDOM := $(shell printf '#include \n#include
\nint main(void){char
buf[256];\ngetrandom(buf,sizeof(buf),GRND_NONBLOCK);}' >bb_libtest.c; $(CC)
$(CFLAGS) $(CFLAGS_busybox) -D_GNU_SOURCE -o /dev/null bb_libtest.c
>/dev/null 2>&1 && echo "y"; rm bb_libtest.c)

How about putting the define line '#define _GNU_SOURCE' into bb_libtest.c ?
I don't like defining the feature test macros externally in '-D' options.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox


[PATCH] seedrng: fix getrandom() detection for non-glibc libc

2023-04-19 Thread Raphaël Mélotte
glibc <= 2.24 does not provide getrandom(). A check for it has been
added in 200a9669fbf6f06894e42439fc11a1a6073a and fixed in
cb57abb46f06f4ede8d9ccbdaac67377fdf416cf.

However, building with a libc other than glibc can lead to the same
problem as not every other libc has getrandom() either:

- uClibc provides it from v1.0.2 onwards, but requires to define
_GNU_SOURCE (all versions - we already define it by default), and
stddef to be included first (when using uClibc < 1.0.35 - we already
include it through libbb.h).

- musl libc has getrandom(), but only from version 1.1.20 onwards. As
musl does not provide __MUSL__ or version information, it's not
possible to check for it like we did for glibc.

All of this makes it difficult (or impossible in case of musl) to
check what we need to do to have getrandom() based on each libc
versions.

On top of that, getrandom() is also not available on older kernels. As
an example, when using a 3.10 kernel with uClibc 1.0.26, getrandom()
is declared so compiling works, but it fails at link time because
getrandom() is not defined.

To make it easier, take a similar approach to what was done for the
crypt library: try to build a sample program to see if we have
getrandom().

Based on the new Makefile variable, we now either use the
libc-provided getrandom() when it's available, or use our own
implementation when it's not (like it was the case already for glibc <
2.25).

This should fix compiling with many libc/kernel combinations.

Signed-off-by: Raphaël Mélotte 
---
Note that I was not able to test every single combination, but I could
confirm it builds successfully for:
uClibc 10.0.24, linux headers 3.10 (libc getrandom NOT used)
uClibc 1.0.36, linux headers 4.9 (libc getrandom used)
musl 1.1.16, linux headers 4.12 (libc getrandom NOT used)
musl 1.2.1, linux headers (libc getrandom used)
glibc 2.25, linux headers 4.10 (libc getrandom used)

 Makefile.flags  | 7 +++
 miscutils/seedrng.c | 8 
 2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/Makefile.flags b/Makefile.flags
index 1cec5ba20..88c11862f 100644
--- a/Makefile.flags
+++ b/Makefile.flags
@@ -161,6 +161,13 @@ ifeq ($(RT_AVAILABLE),y)
 LDLIBS += rt
 endif
 
+# Not all libc versions have getrandom, so check for it.
+HAVE_GETRANDOM := $(shell printf '#include \n#include 
\nint main(void){char 
buf[256];\ngetrandom(buf,sizeof(buf),GRND_NONBLOCK);}' >bb_libtest.c; $(CC) 
$(CFLAGS) $(CFLAGS_busybox) -D_GNU_SOURCE -o /dev/null bb_libtest.c >/dev/null 
2>&1 && echo "y"; rm bb_libtest.c)
+
+ifeq ($(HAVE_GETRANDOM),y)
+CFLAGS += -DHAVE_GETRANDOM
+endif
+
 # libpam may use libpthread, libdl and/or libaudit.
 # On some platforms that requires an explicit -lpthread, -ldl, -laudit.
 # However, on *other platforms* it fails when some of those flags
diff --git a/miscutils/seedrng.c b/miscutils/seedrng.c
index 3bf6e2ea7..2f1e18c32 100644
--- a/miscutils/seedrng.c
+++ b/miscutils/seedrng.c
@@ -44,8 +44,10 @@
 #include 
 #include 
 
-/* Fix up glibc <= 2.24 not having getrandom() */
-#if defined(__GLIBC__) && __GLIBC__ == 2 && __GLIBC_MINOR__ <= 24
+/* Fix up some libc (e.g. glibc <= 2.24) not having getrandom() */
+#if defined HAVE_GETRANDOM
+#include 
+#else /* No getrandom */
 #include 
 static ssize_t getrandom(void *buffer, size_t length, unsigned flags)
 {
@@ -56,8 +58,6 @@ static ssize_t getrandom(void *buffer, size_t length, 
unsigned flags)
return -1;
 # endif
 }
-#else
-#include 
 #endif
 
 /* Apparently some headers don't ship with this yet. */
-- 
2.39.1

___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox