On Thu, Jun 7, 2018 at 6:37 PM, Eric Covener wrote:
> On Thu, Jun 7, 2018 at 11:59 AM, Yann Ylavic wrote:
>> I'd like to propose a new RNG for APR, based on a design from D.J.
>> Bernstein ([1]).
>>
>> Called "Fast-key-erasure random-number generators" by the author, it
>> requires 256bits (32 bytes) of initial entropy only, is fast, and
>> ensures Forward Secrecy.
>>
>> The current RNGs available in APR are:
>> 1/ apr_generate_random_bytes(); the wrapper around the system call,
>> 2/ apr_random_(in)secure_bytes(); the ones from "random/unix/apr_random.c".
>>
>> Both are obviously considered secure, but 1/ is usually not very fast
>> (and could block on some systems), and 2/ requires kilos of initial
>> entropy (taken from 1/ thus possibly blocking too).
>> Also the system RNG wrapped by 1/ is unlikely to return a great number
>> of bytes at once (256 max on some systems like BSDs), and better suits
>> as a source of entropy (like in 2/).
>> Neither ensures forward secrecy (AFAICT).
>>
>
> I think the guts of 2) really needs to go, it is totally orphaned.
That was the plan, with a substitute (if it makes sense).
So we could:
a/ Replace 2/ with this new RNG,
b/ Axe 2/ from APR-2 and add the new RNG in apr_crypto,
c/ Axe 2/ from APR-2 and leave 1/ as the only RNG.
While c/ is straightforward, it leaves users with 1/ only which is not
ideal on some systems, so it's not what I'd propose :)
I'm fine with a/ or b/ and actually started to implement b/ (see
attached) until I realized that the crypto libraries needed to be
chosen by the user at runtime and loaded explicitly/dynamically with a
call to apr_crypto_get_driver().
I don't find this very convenient for a RNG (where the user needs not
have the choice on the underlying crypto), so I turned --with-openssl
(only for now) to link APR-2 with libcrypto at build time (like
--with-xml2 does for libxml2).
So at this point I'm not very far from a/ while I first thought it
would have required to implement a stream cipher in APR-2 (e.g.
CHACHA20 which is quite simple code, like it's currently done for
SHA256 for apr_random) to avoid any crypto lib dependency.
All this to say... :p
First WDYT of this new RNG (and attached first implementation, missing
some tests still)?
If it sounds good, how about we link the configured --with-whatever
crypto library directly with APR-2 without requiring runtime loading?
And if it's still fine, what about a/ vs b/ (where a/ wouldn't need
custom crypto in APR)?
Thanks for reading so far, if anyone :)
Regards,
Yann.
Index: CMakeLists.txt
===
--- CMakeLists.txt (revision 1833215)
+++ CMakeLists.txt (working copy)
@@ -239,6 +239,7 @@ SET(APR_SOURCES
buckets/apr_buckets_simple.c
buckets/apr_buckets_socket.c
crypto/apr_crypto.c
+ crypto/apr_crypto_prng.c
crypto/apr_md4.c
crypto/apr_md5.c
crypto/apr_passwd.c
Index: build/crypto.m4
===
--- build/crypto.m4 (revision 1833215)
+++ build/crypto.m4 (working copy)
@@ -23,6 +23,7 @@ dnl APU_CHECK_CRYPTO: look for crypto libraries an
dnl
AC_DEFUN([APU_CHECK_CRYPTO], [
apu_have_crypto=0
+ apu_have_crypto_prng=0
apu_have_openssl=0
apu_have_nss=0
apu_have_commoncrypto=0
@@ -66,13 +67,18 @@ AC_DEFUN([APU_CHECK_CRYPTO], [
dnl add checks for other varieties of ssl here
if test "$apu_have_crypto" = "0"; then
AC_ERROR([Crypto was requested but no crypto library could be enabled; specify the location of a crypto library using --with-openssl, --with-nss, and/or --with-commoncrypto.])
+ elif test "$apu_have_openssl" = "1"; then
+dnl PRNG only implemented with openssl for now
+apu_have_crypto_prng=1
fi
fi
], [
apu_have_crypto=0
+ apu_have_crypto_prng=0
])
AC_SUBST(apu_have_crypto)
+ AC_SUBST(apu_have_crypto_prng)
])
dnl
@@ -150,6 +156,9 @@ AC_DEFUN([APU_CHECK_CRYPTO_OPENSSL], [
AC_SUBST(LDADD_crypto_openssl)
AC_SUBST(apu_have_crypto)
+ APR_ADDTO(APRUTIL_EXPORT_LIBS, [-lcrypto])
+ APR_ADDTO(LIBS, [-lcrypto])
+
LIBS="$old_libs"
CPPFLAGS="$old_cppflags"
LDFLAGS="$old_ldflags"
Index: build.conf
===
--- build.conf (revision 1833215)
+++ build.conf (working copy)
@@ -11,6 +11,7 @@ paths =
tables/*.c
buckets/*.c
crypto/apr_crypto.c
+ crypto/apr_crypto_prng.c
crypto/apr_md4.c
crypto/apr_md5.c
crypto/apr_passwd.c
Index: crypto/apr_crypto.c
===
--- crypto/apr_crypto.c (revision 1833215)
+++ crypto/apr_crypto.c (working copy)
@@ -86,7 +86,7 @@ static apr_status_t apr_crypto_term(void *ptr)
APR_DECLARE(apr_status_t) apr_crypto_init(apr_pool_t *pool)
{
-apr_status_t ret = APR_SUCCESS;
+apr_status_t rv;
apr_pool_t *parent;
if (drivers != NULL) {
@@ -107,7 +107,15 @@ APR_DECLARE(apr_status_t
