[PATCH] using arc4random for strong randomness matters.

Started by David CARLIERabout 8 years ago12 messages
#1David CARLIER
devnexen@gmail.com
1 attachment(s)

Hello,

This is my first small personal contribution.

Motivation :
- Using fail-safe, file descriptor free solution on *BSD and Darwin system
- Somehow avoiding at the moment FreeBSD as it still uses RC4 (seemingly
updated to Chacha20 for FreeBSD 12.0 and eventually backported later on).
- For implementation based on Chacha* it is known to be enough fast for the
purpose.
- make check passes.

Hope it is good.

Thanks in advance.

Attachments:

0001-Using-arc4random-API-for-strong-randomness-if-availa.patchapplication/octet-stream; name=0001-Using-arc4random-API-for-strong-randomness-if-availa.patchDownload
From 79d0a2c53a87bbf6207fc10c8e2345f27188143d Mon Sep 17 00:00:00 2001
From: David Carlier <devnexen@gmail.com>
Date: Tue, 21 Nov 2017 11:59:16 +0000
Subject: [PATCH] Using arc4random API for strong randomness if available and
 with implementation not using RC4 stream cipher.

---
 configure                   | 12 ++++++++++--
 configure.in                |  9 +++++++--
 src/include/pg_config.h.in  |  3 +++
 src/port/pg_strong_random.c |  9 +++++++++
 4 files changed, 29 insertions(+), 4 deletions(-)

diff --git a/configure b/configure
index b31134832e..dcfe94ec84 100755
--- a/configure
+++ b/configure
@@ -15618,11 +15618,13 @@ fi
 # in the template or configure command line.
 
 # If not selected manually, try to select a source automatically.
-if test "$enable_strong_random" = "yes" && test x"$USE_OPENSSL_RANDOM" = x"" && test x"$USE_WIN32_RANDOM" = x"" && test x"$USE_DEV_URANDOM" = x"" ; then
+if test "$enable_strong_random" = "yes" && test x"$USE_OPENSSL_RANDOM" = x"" && test x"$USE_WIN32_RANDOM" = x"" && test x"$USE_ARC4_RANDOM" = x"" && test x"$USE_DEV_URANDOM" = x"" ; then
   if test x"$with_openssl" = x"yes" ; then
     USE_OPENSSL_RANDOM=1
   elif test "$PORTNAME" = "win32" ; then
     USE_WIN32_RANDOM=1
+  elif test "$PORTNAME" = "freebsd" || test "$PORTNAME" = "openbsd" || test "$PORTNAME" = "netbsd" || test "$PORTNAME" = "darwin" ; then
+    USE_ARC4_RANDOM=1
   else
     { $as_echo "$as_me:${as_lineno-$LINENO}: checking for /dev/urandom" >&5
 $as_echo_n "checking for /dev/urandom... " >&6; }
@@ -15665,6 +15667,12 @@ $as_echo "#define USE_WIN32_RANDOM 1" >>confdefs.h
 
     { $as_echo "$as_me:${as_lineno-$LINENO}: result: Windows native" >&5
 $as_echo "Windows native" >&6; }
+  elif test x"$USE_ARC4_RANDOM" = x"1" ; then
+
+$as_echo "#define USE_ARC4_RANDOM 1" >>confdefs.h
+
+    { $as_echo "$as_me:${as_lineno-$LINENO}: result: arc4random" >&5
+$as_echo "arc4random" >&6; }
   elif test x"$USE_DEV_URANDOM" = x"1" ; then
 
 $as_echo "#define USE_DEV_URANDOM 1" >>confdefs.h
@@ -15674,7 +15682,7 @@ $as_echo "/dev/urandom" >&6; }
   else
     as_fn_error $? "
 no source of strong random numbers was found
-PostgreSQL can use OpenSSL or /dev/urandom as a source of random numbers,
+PostgreSQL can use OpenSSL or arc4random or /dev/urandom as a source of random numbers,
 for authentication protocols. You can use --disable-strong-random to use a
 built-in pseudo random number generator, but that may be insecure." "$LINENO" 5
   fi
diff --git a/configure.in b/configure.in
index 3f26f038d6..bffde1deef 100644
--- a/configure.in
+++ b/configure.in
@@ -1989,11 +1989,13 @@ fi
 # in the template or configure command line.
 
 # If not selected manually, try to select a source automatically.
-if test "$enable_strong_random" = "yes" && test x"$USE_OPENSSL_RANDOM" = x"" && test x"$USE_WIN32_RANDOM" = x"" && test x"$USE_DEV_URANDOM" = x"" ; then
+if test "$enable_strong_random" = "yes" && test x"$USE_OPENSSL_RANDOM" = x"" && test x"$USE_WIN32_RANDOM" = x"" && test x"$USE_ARC4_RANDOM" = x"" && test x"$USE_DEV_URANDOM" = x"" ; then
   if test x"$with_openssl" = x"yes" ; then
     USE_OPENSSL_RANDOM=1
   elif test "$PORTNAME" = "win32" ; then
     USE_WIN32_RANDOM=1
+  elif test "$PORTNAME" = "freebsd" || test "$PORTNAME" = "openbsd" || test "$PORTNAME" = "netbsd" || test "$PORTNAME" = "darwin" ; then
+    USE_ARC4_RANDOM=1
   else
     AC_CHECK_FILE([/dev/urandom], [], [])
 
@@ -2011,13 +2013,16 @@ if test "$enable_strong_random" = yes ; then
   elif test x"$USE_WIN32_RANDOM" = x"1" ; then
     AC_DEFINE(USE_WIN32_RANDOM, 1, [Define to use native Windows API for random number generation])
     AC_MSG_RESULT([Windows native])
+  elif test x"$USE_ARC4_RANDOM" = x"1" ; then
+    AC_DEFINE(USE_ARC4_RANDOM, 1, [Define to use arc4random API for random number generation])
+    AC_MSG_RESULT([arc4random])
   elif test x"$USE_DEV_URANDOM" = x"1" ; then
     AC_DEFINE(USE_DEV_URANDOM, 1, [Define to use /dev/urandom for random number generation])
     AC_MSG_RESULT([/dev/urandom])
   else
     AC_MSG_ERROR([
 no source of strong random numbers was found
-PostgreSQL can use OpenSSL or /dev/urandom as a source of random numbers,
+PostgreSQL can use OpenSSL or arc4random or /dev/urandom as a source of random numbers,
 for authentication protocols. You can use --disable-strong-random to use a
 built-in pseudo random number generator, but that may be insecure.])
   fi
diff --git a/src/include/pg_config.h.in b/src/include/pg_config.h.in
index 84d59f12b2..20373bf587 100644
--- a/src/include/pg_config.h.in
+++ b/src/include/pg_config.h.in
@@ -805,6 +805,9 @@
 /* Define to 1 if your <sys/time.h> declares `struct tm'. */
 #undef TM_IN_SYS_TIME
 
+/* Define to use arc4random API for random number generation */
+#undef USE_ARC4_RANDOM
+
 /* Define to 1 to build with assertion checks. (--enable-cassert) */
 #undef USE_ASSERT_CHECKING
 
diff --git a/src/port/pg_strong_random.c b/src/port/pg_strong_random.c
index c6ee5ea1d4..e3e6804ee2 100644
--- a/src/port/pg_strong_random.c
+++ b/src/port/pg_strong_random.c
@@ -134,6 +134,15 @@ pg_strong_random(void *buf, size_t len)
 	}
 	return false;
 
+	/*
+	 * Using arc4random call to fill the buffer
+	 * with strong randomness which never fails
+	 * not needs a file descriptor
+	 */
+#elif defined(USE_ARC4_RANDOM)
+	arc4random_buf(buf, len);
+	return true;
+
 	/*
 	 * Read /dev/urandom ourselves.
 	 */
-- 
2.15.0

#2Michael Paquier
michael.paquier@gmail.com
In reply to: David CARLIER (#1)
Re: [PATCH] using arc4random for strong randomness matters.

On Tue, Nov 21, 2017 at 9:08 PM, David CARLIER <devnexen@gmail.com> wrote:

Motivation :
- Using fail-safe, file descriptor free solution on *BSD and Darwin system
- Somehow avoiding at the moment FreeBSD as it still uses RC4 (seemingly
updated to Chacha20 for FreeBSD 12.0 and eventually backported later on).
- For implementation based on Chacha* it is known to be enough fast for the
purpose.
- make check passes.

This looks like a good idea to me at quick glance. Please make sure to
register it in the next commit fest if yo uwould like to get feedback
about this feature:
https://commitfest.postgresql.org/16/
--
Michael

#3David Fetter
david@fetter.org
In reply to: David CARLIER (#1)
Re: [PATCH] using arc4random for strong randomness matters.

On Tue, Nov 21, 2017 at 12:08:46PM +0000, David CARLIER wrote:

Hello,

This is my first small personal contribution.

Motivation :
- Using fail-safe, file descriptor free solution on *BSD and Darwin system
- Somehow avoiding at the moment FreeBSD as it still uses RC4 (seemingly
updated to Chacha20 for FreeBSD 12.0 and eventually backported later on).
- For implementation based on Chacha* it is known to be enough fast for the
purpose.
- make check passes.

Hope it is good.

Thanks in advance.

This is neat. Apparently, it's useable on Linux with a gizmo called
libbsd. Would it be worth it to test for that library on that
platform?

Best,
David.
--
David Fetter <david(at)fetter(dot)org> http://fetter.org/
Phone: +1 415 235 3778

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

#4David CARLIER
devnexen@gmail.com
In reply to: David Fetter (#3)
Re: [PATCH] using arc4random for strong randomness matters.

I m not against as such that depends of the implementation but I ve seen in
quick glance it s RC4 ?

Regards.

On 22 November 2017 at 15:37, David Fetter <david@fetter.org> wrote:

Show quoted text

On Tue, Nov 21, 2017 at 12:08:46PM +0000, David CARLIER wrote:

Hello,

This is my first small personal contribution.

Motivation :
- Using fail-safe, file descriptor free solution on *BSD and Darwin

system

- Somehow avoiding at the moment FreeBSD as it still uses RC4 (seemingly
updated to Chacha20 for FreeBSD 12.0 and eventually backported later on).
- For implementation based on Chacha* it is known to be enough fast for

the

purpose.
- make check passes.

Hope it is good.

Thanks in advance.

This is neat. Apparently, it's useable on Linux with a gizmo called
libbsd. Would it be worth it to test for that library on that
platform?

Best,
David.
--
David Fetter <david(at)fetter(dot)org> http://fetter.org/
Phone: +1 415 235 3778

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

#5Tom Lane
tgl@sss.pgh.pa.us
In reply to: David CARLIER (#4)
Re: [PATCH] using arc4random for strong randomness matters.

David CARLIER <devnexen@gmail.com> writes:

I m not against as such that depends of the implementation but I ve seen in
quick glance it s RC4 ?

More generally, why should we bother with an additional implementation?
Is this better than /dev/urandom, and if so why?

regards, tom lane

#6David CARLIER
devnexen@gmail.com
In reply to: Tom Lane (#5)
Re: [PATCH] using arc4random for strong randomness matters.

Basically the call never fails, always generating high quality random data
(especially the implementations based on Chacha* family, RC4 has
predictability issues), there is no need of a file descriptor.

On 22 November 2017 at 16:06, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Show quoted text

David CARLIER <devnexen@gmail.com> writes:

I m not against as such that depends of the implementation but I ve seen

in

quick glance it s RC4 ?

More generally, why should we bother with an additional implementation?
Is this better than /dev/urandom, and if so why?

regards, tom lane

#7Andres Freund
andres@anarazel.de
In reply to: David CARLIER (#6)
Re: [PATCH] using arc4random for strong randomness matters.

Hi,

Please don't top-quote on postgres mailing lists.

On 2017-11-22 16:16:35 +0000, David CARLIER wrote:

David CARLIER <devnexen@gmail.com> writes:

I m not against as such that depends of the implementation but I ve seen

in

quick glance it s RC4 ?

More generally, why should we bother with an additional implementation?
Is this better than /dev/urandom, and if so why?

Basically the call never fails, always generating high quality random data
(especially the implementations based on Chacha* family, RC4 has
predictability issues), there is no need of a file descriptor.

I don't really see much benefit in those properties for postgres
specifically. Not needing an fd is nice for cases where you're not
guaranteed to have access to a filesystem, but postgres isn't going to
work in those cases anyway.

Greetings,

Andres Freund

#8Noname
ilmari@ilmari.org
In reply to: Tom Lane (#5)
Re: [PATCH] using arc4random for strong randomness matters.

Tom Lane <tgl@sss.pgh.pa.us> writes:

David CARLIER <devnexen@gmail.com> writes:

I m not against as such that depends of the implementation but I ve seen in
quick glance it s RC4 ?

arc4random uses ChaCha20 since OpenBSD 5.5 (and libbsd 0.8.0 on Linux).
It uses getentropy(2) to seed itself at regular intervals and at fork().

http://man.openbsd.org/arc4random.3

More generally, why should we bother with an additional implementation?
Is this better than /dev/urandom, and if so why?

If what is wanted is something more like /dev/urandom, one can call
getentropy(2) (or on Linux, getrandom(2)) directly, which avoids having
to open the device file each time.

http://man.openbsd.org/getentropy.2
https://manpages.debian.org/stretch/manpages-dev/getrandom.2.en.html

- ilmari
--
"The surreality of the universe tends towards a maximum" -- Skud's Law
"Never formulate a law or axiom that you're not prepared to live with
the consequences of." -- Skud's Meta-Law

#9Andres Freund
andres@anarazel.de
In reply to: Noname (#8)
Re: [PATCH] using arc4random for strong randomness matters.

On November 22, 2017 8:51:07 AM PST, ilmari@ilmari.org wrote:

If what is wanted is something more like /dev/urandom, one can call
getentropy(2) (or on Linux, getrandom(2)) directly, which avoids having
to open the device file each time.

What does that buy us for our usages?

Andres
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.

#10Tom Lane
tgl@sss.pgh.pa.us
In reply to: Noname (#8)
Re: [PATCH] using arc4random for strong randomness matters.

ilmari@ilmari.org (Dagfinn Ilmari =?utf-8?Q?Manns=C3=A5ker?=) writes:

Tom Lane <tgl@sss.pgh.pa.us> writes:

More generally, why should we bother with an additional implementation?
Is this better than /dev/urandom, and if so why?

If what is wanted is something more like /dev/urandom, one can call
getentropy(2) (or on Linux, getrandom(2)) directly, which avoids having
to open the device file each time.

I dunno, it seems like this is opening us to a new set of portability
hazards (ie, sub-par implementations of arc4random) with not much gain to
show for it.

IIUC, what this code actually does is reseed itself from /dev/urandom
every so often and work from a PRNG in between. That's not a layer that
we need, because the code on top is already designed to cope with the
foibles of /dev/urandom --- or, to the extent it isn't, that's something
we have to fix anyway. So it seems like having this optionally in place
just reduces what we can assume about the randomness properties of
pg_strong_random output, which doesn't seem like a good idea.

regards, tom lane

#11David CARLIER
devnexen@gmail.com
In reply to: Tom Lane (#10)
Re: [PATCH] using arc4random for strong randomness matters.

I dunno, it seems like this is opening us to a new set of portability
hazards (ie, sub-par implementations of arc4random) with not much gain to
show for it.

Hence I reduced to three platforms only.

IIUC, what this code actually does is reseed itself from /dev/urandom
every so often and work from a PRNG in between. That's not a layer that
we need, because the code on top is already designed to cope with the
foibles of /dev/urandom --- or, to the extent it isn't, that's something
we have to fix anyway. So it seems like having this optionally in place
just reduces what we can assume about the randomness properties of
pg_strong_random output, which doesn't seem like a good idea.

That I admit these are valid points.

Cheers.

Show quoted text

regards, tom lane

#12Stephen Frost
sfrost@snowman.net
In reply to: David CARLIER (#11)
Re: [PATCH] using arc4random for strong randomness matters.

David, all,

* David CARLIER (devnexen@gmail.com) wrote:

IIUC, what this code actually does is reseed itself from /dev/urandom
every so often and work from a PRNG in between. That's not a layer that
we need, because the code on top is already designed to cope with the
foibles of /dev/urandom --- or, to the extent it isn't, that's something
we have to fix anyway. So it seems like having this optionally in place
just reduces what we can assume about the randomness properties of
pg_strong_random output, which doesn't seem like a good idea.

That I admit these are valid points.

Based on the discussion, it looks like this patch should be marked as
'Rejected', so I've gone ahead and done that.

We can reopen it if that's incorrect for some reason.

Thanks!

Stephen