| Message ID | 20171016073307.11924-1-lkundrak@v3.sk |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [RESEND] crypto: add option to use getrandom() | expand |
On Mon, Oct 16, 2017 at 09:33:07AM +0200, Lubomir Rintel wrote: > According to random(4) manual, /dev/random is essentially deprecated on Linux > for quite some time: > > The /dev/random interface is considered a legacy interface, and > /dev/urandom is preferred and sufficient in all use cases, with the > exception of applications which require randomness during early boot time; > for these applications, getrandom(2) must be used instead, because it will > block until the entropy pool is initialized. 'man 4 random' on Ubuntu 16.04 does not have such language, so I'm not sure I'd agree about the "quite some time" part.. > An attempt to use it would cause unnecessary blocking on machines > without a good hwrng even when it shouldn't be needed. Since Linux 3.17, > a getrandom(2) call is available that will block only until the > randomness pool has been seeded. What unnecessary blocking are you referring to here? /dev/random is opened in non-blocking mode and more data is fetched from it once it becomes available. How would getrandom(2) help here? Please also note that I won't be accepting changes that would practically move from /dev/random to /dev/urandom pool without very thorough explanation on how that would not have a negative impact on security especially as far as key derivation is concerned (this data is used to generate encryption keys). There is something odd here: > diff --git a/hostapd/Makefile b/hostapd/Makefile > ifdef CONFIG_NO_RANDOM_POOL > +ifdef CONFIG_GETRANDOM > +CFLAGS += -DCONFIG_GETRANDOM > +endif > CFLAGS += -DCONFIG_NO_RANDOM_POOL > else > OBJS += ../src/crypto/random.o > diff --git a/wpa_supplicant/Makefile b/wpa_supplicant/Makefile > @@ -1476,6 +1476,9 @@ endif > ifdef CONFIG_NO_RANDOM_POOL > CFLAGS += -DCONFIG_NO_RANDOM_POOL > else > +ifdef CONFIG_GETRANDOM > +CFLAGS += -DCONFIG_GETRANDOM > +endif > OBJS += ../src/crypto/random.o > endif Why would this be different (related to CONFIG_NO_RANDOM_POOL) between wpa_supplicant and hostapd? Furthermore, Android.mk should likely have similar changes for both. > diff --git a/src/crypto/random.c b/src/crypto/random.c > @@ -229,30 +232,49 @@ int random_pool_ready(void) > return 1; /* Already initialized - good to continue */ > > /* > - * Try to fetch some more data from the kernel high quality > - * /dev/random. There may not be enough data available at this point, > + * Try to fetch some more data from the kernel high quality RNG > + * There may not be enough data available at this point, > * so use non-blocking read to avoid blocking the application > * completely. > */ > - fd = open("/dev/random", O_RDONLY | O_NONBLOCK); Please note that this is using /dev/random on purpose (i.e., not /dev/urandom) since the material is used for key derivation.. > +#ifdef CONFIG_GETRANDOM > + res = getrandom(dummy_key + dummy_key_avail, > + sizeof(dummy_key) - dummy_key_avail, GRND_NONBLOCK); While this would default to use the /dev/urandom pool since GRND_RANDOM is not specified. I don't think this is a good change taken into account the need for secure enough randomness for key derivation. > @@ -272,8 +294,8 @@ int random_pool_ready(void) > return 1; > } > > - wpa_printf(MSG_INFO, "random: Not enough entropy pool available for " > - "secure operations"); > + wpa_printf(MSG_INFO, "random: Not enough entropy pool available " > + "from for secure operations"); ??
On Sun, 2017-11-26 at 13:49 +0200, Jouni Malinen wrote: > On Mon, Oct 16, 2017 at 09:33:07AM +0200, Lubomir Rintel wrote: > > According to random(4) manual, /dev/random is essentially deprecated on Linux > > for quite some time: > > > > The /dev/random interface is considered a legacy interface, and > > /dev/urandom is preferred and sufficient in all use cases, with the > > exception of applications which require randomness during early boot time; > > for these applications, getrandom(2) must be used instead, because it will > > block until the entropy pool is initialized. > > 'man 4 random' on Ubuntu 16.04 does not have such language, so I'm not > sure I'd agree about the "quite some time" part.. > > > An attempt to use it would cause unnecessary blocking on machines > > without a good hwrng even when it shouldn't be needed. Since Linux 3.17, > > a getrandom(2) call is available that will block only until the > > randomness pool has been seeded. > > What unnecessary blocking are you referring to here? /dev/random is > opened in non-blocking mode and more data is fetched from it once it > becomes available. How would getrandom(2) help here? Poor wording on my side. What I meant here is that the actual service provided by hostapd is not operational, not that the daemon is blocked on a system call. The issue I've encountered was that hostapd would stop working after a few restarts on a virtual machine (used for regression testing). My understanding was hardware randomness pool is a rather scarce resource in some setups and should be used with caution. (The actual fix was, of course, exposing a hwrng via virtio channel from the hypervisor.) > Please also note that I won't be accepting changes that would > practically move from /dev/random to /dev/urandom pool without very > thorough explanation on how that would not have a negative impact on > security especially as far as key derivation is concerned (this data is > used to generate encryption keys). Fair enough. I'm not really qualified to provide an explanation more technical than pointing to the random(4) manual and thus will not try to get this merged anymore. Thanks, Lubo
diff --git a/hostapd/Makefile b/hostapd/Makefile index c54de3917..842fe97b5 100644 --- a/hostapd/Makefile +++ b/hostapd/Makefile @@ -1026,6 +1026,9 @@ CFLAGS += -DCONFIG_ECC endif ifdef CONFIG_NO_RANDOM_POOL +ifdef CONFIG_GETRANDOM +CFLAGS += -DCONFIG_GETRANDOM +endif CFLAGS += -DCONFIG_NO_RANDOM_POOL else OBJS += ../src/crypto/random.o diff --git a/hostapd/defconfig b/hostapd/defconfig index 26be9f8d4..a2e2b998b 100644 --- a/hostapd/defconfig +++ b/hostapd/defconfig @@ -252,6 +252,11 @@ CONFIG_IPV6=y # requirements described above. #CONFIG_NO_RANDOM_POOL=y +# Should we attempt to use the getrandom(2) call that provides more reliable +# yet secure randomness source than /dev/random on Linux 3.17 and newer. +# Requires glibc 2.25 to build, falls back to /dev/random if unavailable. +#CONFIG_GETRANDOM=y + # Should we use poll instead of select? Select is used by default. #CONFIG_ELOOP_POLL=y diff --git a/src/crypto/random.c b/src/crypto/random.c index fb9241762..5cb852853 100644 --- a/src/crypto/random.c +++ b/src/crypto/random.c @@ -25,6 +25,9 @@ #include "utils/includes.h" #ifdef __linux__ #include <fcntl.h> +#ifdef CONFIG_GETRANDOM +#include <sys/random.h> +#endif /* CONFIG_GETRANDOM */ #endif /* __linux__ */ #include "utils/common.h" @@ -229,30 +232,49 @@ int random_pool_ready(void) return 1; /* Already initialized - good to continue */ /* - * Try to fetch some more data from the kernel high quality - * /dev/random. There may not be enough data available at this point, + * Try to fetch some more data from the kernel high quality RNG + * There may not be enough data available at this point, * so use non-blocking read to avoid blocking the application * completely. */ - fd = open("/dev/random", O_RDONLY | O_NONBLOCK); - if (fd < 0) { - wpa_printf(MSG_ERROR, "random: Cannot open /dev/random: %s", - strerror(errno)); - return -1; - } - res = read(fd, dummy_key + dummy_key_avail, - sizeof(dummy_key) - dummy_key_avail); +#ifdef CONFIG_GETRANDOM + res = getrandom(dummy_key + dummy_key_avail, + sizeof(dummy_key) - dummy_key_avail, GRND_NONBLOCK); if (res < 0) { - wpa_printf(MSG_ERROR, "random: Cannot read from /dev/random: " - "%s", strerror(errno)); - res = 0; + if (errno == ENOSYS) { + wpa_printf(MSG_DEBUG, "random: getrandom() not supported, falling " + "back to /dev/random"); + } else { + wpa_printf(MSG_ERROR, "random: no data from getrandom(): " + "%s", strerror(errno)); + res = 0; + } } - wpa_printf(MSG_DEBUG, "random: Got %u/%u bytes from " - "/dev/random", (unsigned) res, +#else + res = -1; +#endif /* CONFIG_GETRANDOM */ + if (res < 0) { + fd = open("/dev/random", O_RDONLY | O_NONBLOCK); + if (fd < 0) { + wpa_printf(MSG_ERROR, "random: Cannot open /dev/random: %s", + strerror(errno)); + return -1; + } + + res = read(fd, dummy_key + dummy_key_avail, + sizeof(dummy_key) - dummy_key_avail); + if (res < 0) { + wpa_printf(MSG_ERROR, "random: Cannot read from /dev/random: " + "%s", strerror(errno)); + res = 0; + } + close(fd); + } + + wpa_printf(MSG_DEBUG, "random: Got %u/%u random bytes", (unsigned) res, (unsigned) (sizeof(dummy_key) - dummy_key_avail)); dummy_key_avail += res; - close(fd); if (dummy_key_avail == sizeof(dummy_key)) { if (own_pool_ready < MIN_READY_MARK) @@ -262,7 +284,7 @@ int random_pool_ready(void) } wpa_printf(MSG_INFO, "random: Only %u/%u bytes of strong " - "random data available from /dev/random", + "random data available", (unsigned) dummy_key_avail, (unsigned) sizeof(dummy_key)); if (own_pool_ready >= MIN_READY_MARK || @@ -272,8 +294,8 @@ int random_pool_ready(void) return 1; } - wpa_printf(MSG_INFO, "random: Not enough entropy pool available for " - "secure operations"); + wpa_printf(MSG_INFO, "random: Not enough entropy pool available " + "from for secure operations"); return 0; #else /* __linux__ */ /* TODO: could do similar checks on non-Linux platforms */ @@ -415,6 +437,13 @@ void random_init(const char *entropy_file) if (random_fd >= 0) return; +#ifdef CONFIG_GETRANDOM + if (getrandom(NULL, 0, GRND_NONBLOCK) == 0 || errno != ENOSYS) { + wpa_printf(MSG_INFO, "random: getrandom() support available"); + return; + } +#endif /* CONFIG_GETRANDOM */ + random_fd = open("/dev/random", O_RDONLY | O_NONBLOCK); if (random_fd < 0) { wpa_printf(MSG_ERROR, "random: Cannot open /dev/random: %s", diff --git a/wpa_supplicant/Makefile b/wpa_supplicant/Makefile index b62b898d6..f629fc200 100644 --- a/wpa_supplicant/Makefile +++ b/wpa_supplicant/Makefile @@ -1476,6 +1476,9 @@ endif ifdef CONFIG_NO_RANDOM_POOL CFLAGS += -DCONFIG_NO_RANDOM_POOL else +ifdef CONFIG_GETRANDOM +CFLAGS += -DCONFIG_GETRANDOM +endif OBJS += ../src/crypto/random.o endif diff --git a/wpa_supplicant/defconfig b/wpa_supplicant/defconfig index 1797ad359..c35272eb2 100644 --- a/wpa_supplicant/defconfig +++ b/wpa_supplicant/defconfig @@ -456,6 +456,11 @@ CONFIG_PEERKEY=y # that meet the requirements described above. #CONFIG_NO_RANDOM_POOL=y +# Should we attempt to use the getrandom(2) call that provides more reliable +# yet secure randomness source than /dev/random on Linux 3.17 and newer. +# Requires glibc 2.25 to build, falls back to /dev/random if unavailable. +#CONFIG_GETRANDOM=y + # IEEE 802.11n (High Throughput) support (mainly for AP mode) #CONFIG_IEEE80211N=y
According to random(4) manual, /dev/random is essentially deprecated on Linux for quite some time: The /dev/random interface is considered a legacy interface, and /dev/urandom is preferred and sufficient in all use cases, with the exception of applications which require randomness during early boot time; for these applications, getrandom(2) must be used instead, because it will block until the entropy pool is initialized. An attempt to use it would cause unnecessary blocking on machines without a good hwrng even when it shouldn't be needed. Since Linux 3.17, a getrandom(2) call is available that will block only until the randomness pool has been seeded. It is probably not a good default yet as it requires a fairly recent kernel and glibc (3.17 and 2.25 respectively). Signed-off-by: Lubomir Rintel <lkundrak@v3.sk> --- hostapd/Makefile | 3 +++ hostapd/defconfig | 5 ++++ src/crypto/random.c | 67 ++++++++++++++++++++++++++++++++++-------------- wpa_supplicant/Makefile | 3 +++ wpa_supplicant/defconfig | 5 ++++ 5 files changed, 64 insertions(+), 19 deletions(-)