[RESEND] crypto: add option to use getrandom()

Message ID 20171016073307.11924-1-lkundrak@v3.sk
State Changes Requested
Headers show
Series
  • [RESEND] crypto: add option to use getrandom()
Related show

Commit Message

Lubomir Rintel Oct. 16, 2017, 7:33 a.m.
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(-)

Comments

Jouni Malinen Nov. 26, 2017, 11:49 a.m. | #1
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");

??
Lubomir Rintel Nov. 27, 2017, 1:36 p.m. | #2
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

Patch

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