Re: [PATCH] Reseed PRNG on PID change
On 01/16/2014 05:03 PM, David Jacobson wrote: If you want to make sure they diverge, and make sure that multiple forks diverge differently, you should push in the process ID. Pushing in time helps with (but does not perfectly cure) the virtual machine copying problem. So I suggest pushing in the PID concatenated with some sort of time. The PID is already mixed in. It turned out this was not sufficient because PIDs are reused (see e.g. CVE-2013-1900). My initial idea was to reseed the pool if a PID change was detected, but reseeding with a high-resolution timer appears preferable. It also sidesteps the LinuxThreads issue. -- Florian Weimer / Red Hat Product Security Team __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager majord...@openssl.org
Re: [PATCH] Reseed PRNG on PID change
On 1/16/14 4:57 AM, Dr. Stephen Henson wrote: On Thu, Jan 16, 2014, Florian Weimer wrote: The additional resolution of a tick counter might make reseeding after fork unnecessary, but it's difficult to be sure. Something not based on timing information looks desirable to me. I should point out that the aim of the current code is not to completely reseed after fork() but to make the PRNG state diverge so that the two processes do not share the same internal PRNG state. Steve. -- Dr Stephen N. Henson. OpenSSL project core developer. Commercial tech support now available see: http://www.openssl.org __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager majord...@openssl.org If you want to make sure they diverge, and make sure that multiple forks diverge differently, you should push in the process ID. Pushing in time helps with (but does not perfectly cure) the virtual machine copying problem. So I suggest pushing in the PID concatenated with some sort of time. The reason I say that it is not perfect is that that might be some installation that has taken a snapshot of a VM. Every time they need a new VM, they just start up a new one from that snapshot. If time is only to a second, it is quite possible that 2 VMs start up the same second. Higher resolution time would be better. Of course, modern Intel chips have a really good RNG. Use that if you can. --David Jacobson __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager majord...@openssl.org
Re: [PATCH] Reseed PRNG on PID change
Am Donnerstag, 16. Januar 2014, 15:55:36 schrieb Florian Weimer: Hi Florian, >On 01/16/2014 03:42 PM, Florian Weimer wrote: >> On 01/16/2014 03:39 PM, Florian Weimer wrote: >>> I still propose to get rid of the time() call (md_rand-time.patch, >>> AFAICT num == 0 at this point, so I pulled the initialization out of >>> the loop). >> >> Disregard the patches, this doesn't work. > >These patches should be better. Overall, things are still the same: >the first patch removes the call to time(), and the second patch uses >OPENSSL_rdtsc() as a fallback if RDRAND is not available. > >I don't know what kinds of changes are acceptable to the FIPS PRNG. >From a FIPS 140-2 regulations perspective, there are no special considerations needed for this scenario. Thus, mixing the time stamp into those DRNGs is no problem either. Ciao Stephan __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager majord...@openssl.org
Re: [PATCH] Reseed PRNG on PID change
On 01/16/2014 03:42 PM, Florian Weimer wrote:
On 01/16/2014 03:39 PM, Florian Weimer wrote:
I still propose to get rid of the time() call (md_rand-time.patch,
AFAICT num == 0 at this point, so I pulled the initialization out of the
loop).
Disregard the patches, this doesn't work.
These patches should be better. Overall, things are still the same: the
first patch removes the call to time(), and the second patch uses
OPENSSL_rdtsc() as a fallback if RDRAND is not available.
I don't know what kinds of changes are acceptable to the FIPS PRNG.
--
Florian Weimer / Red Hat Product Security Team
>From 562e65769d3a0a228addf0c3ceb11322671fe719 Mon Sep 17 00:00:00 2001
From: Florian Weimer
Date: Thu, 16 Jan 2014 15:50:12 +0100
Subject: [PATCH 1/2] ssleay_rand_seed: optimize fork protection: do not call
time()
---
crypto/rand/md_rand.c | 19 ---
1 file changed, 8 insertions(+), 11 deletions(-)
diff --git a/crypto/rand/md_rand.c b/crypto/rand/md_rand.c
index 6cab308..c7c0f2c 100644
--- a/crypto/rand/md_rand.c
+++ b/crypto/rand/md_rand.c
@@ -368,7 +368,7 @@ static int ssleay_rand_bytes(unsigned char *buf, int num, int pseudo)
#ifndef GETPID_IS_MEANINGLESS
pid_t curr_pid = getpid();
#endif
- time_t curr_time = time(NULL);
+ int do_diverge = 1;
int do_stir_pool = 0;
/* time value for various platforms */
#ifdef OPENSSL_SYS_WIN32
@@ -515,26 +515,23 @@ static int ssleay_rand_bytes(unsigned char *buf, int num, int pseudo)
num-=j;
if (!MD_Init(&m))
goto err;
-#ifndef GETPID_IS_MEANINGLESS
- if (curr_pid) /* just in the first iteration to save time */
+
+ if (do_diverge) /* just in the first iteration to save time */
{
+ /* try to diverge the random sequence after
+ fork and VM resumption */
+#ifndef GETPID_IS_MEANINGLESS
if (!MD_Update(&m,(unsigned char*)&curr_pid,
sizeof curr_pid))
goto err;
- curr_pid = 0;
- }
#endif
- if (curr_time) /* just in the first iteration to save time */
- {
- if (!MD_Update(&m,(unsigned char*)&curr_time,
- sizeof curr_time))
-goto err;
if (!MD_Update(&m,(unsigned char*)&tv,
sizeof tv))
goto err;
- curr_time = 0;
rand_hw_seed(&m);
+ do_diverge = 0;
}
+
if (!MD_Update(&m,local_md,MD_DIGEST_LENGTH))
goto err;
if (!MD_Update(&m,(unsigned char *)&(md_c[0]),sizeof(md_c)))
--
1.8.3.1
>From bd31a066c63d3347e65626f9922fb7ba3aef50f7 Mon Sep 17 00:00:00 2001
From: Florian Weimer
Date: Thu, 16 Jan 2014 15:51:04 +0100
Subject: [PATCH 2/2] ssleay_rand_bytes: use cycle counter if RDRAND is not
available
---
crypto/rand/md_rand.c | 14 ++
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/crypto/rand/md_rand.c b/crypto/rand/md_rand.c
index c7c0f2c..4457cbf 100644
--- a/crypto/rand/md_rand.c
+++ b/crypto/rand/md_rand.c
@@ -383,9 +383,6 @@ static int ssleay_rand_bytes(unsigned char *buf, int num, int pseudo)
#elif defined(OPENSSL_SYS_VXWORKS)
struct timespec tv;
clock_gettime(CLOCK_REALTIME, &ts);
-#elif defined(OPENSSL_SYSNAME_DSPBIOS)
- unsigned long long tv, OPENSSL_rdtsc();
- tv = OPENSSL_rdtsc();
#else
struct timeval tv;
gettimeofday(&tv, NULL);
@@ -683,7 +680,13 @@ static void rand_hw_seed(EVP_MD_CTX *ctx)
{
int i;
if (!(OPENSSL_ia32cap_P[1] & (1<<(62-32
+ {
+ /* using the cycle counter is better than nothing at all */
+ unsigned long long tv, OPENSSL_rdtsc();
+ tv = OPENSSL_rdtsc();
+ MD_Update(ctx, (unsigned char *)&tv, sizeof(tv));
return;
+ }
for (i = 0; i < RDRAND_CALLS; i++)
{
size_t rnd;
@@ -730,7 +733,10 @@ void rand_hw_xor(unsigned char *buf, size_t num)
static void rand_hw_seed(EVP_MD_CTX *ctx)
{
- return;
+ /* using the cycle counter is better than nothing at all */
+ unsigned long long tv, OPENSSL_rdtsc();
+ tv = OPENSSL_rdtsc();
+ MD_Update(ctx, (unsigned char *)&tv, sizeof(tv));
}
void rand_hw_xor(unsigned char *buf, size_t num)
--
1.8.3.1
Re: [PATCH] Reseed PRNG on PID change
Am Donnerstag, 16. Januar 2014, 15:39:04 schrieb Florian Weimer: Hi Florian, >On 01/16/2014 01:57 PM, Dr. Stephen Henson wrote: >> On Thu, Jan 16, 2014, Florian Weimer wrote: >>> The additional resolution of a tick counter might make reseeding >>> after fork unnecessary, but it's difficult to be sure. Something >>> not based on timing information looks desirable to me. >> >> I should point out that the aim of the current code is not to >> completely reseed after fork() but to make the PRNG state diverge so >> that the two processes do not share the same internal PRNG state. > >I get that. I've reconsidered things. Time seems the only thing that >deals with the related VM snapshot issue. > >I still propose to get rid of the time() call (md_rand-time.patch, >AFAICT num == 0 at this point, so I pulled the initialization out of >the loop). OPENSSL_rdtsc() appears to return 0 if the cycle counter >is not available, so md_rand-rtdsc.patch calls it unconditionally if >RDRAND is not available. Isn't the same issue applicable to the FIPS DRNGs? Especially considering that RHEL/Fedora ships these and have them enabled with a kernel command line flag, I am wondering about those. Ciao Stephan __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager majord...@openssl.org
Re: [PATCH] Reseed PRNG on PID change
On 01/16/2014 03:39 PM, Florian Weimer wrote: I still propose to get rid of the time() call (md_rand-time.patch, AFAICT num == 0 at this point, so I pulled the initialization out of the loop). Disregard the patches, this doesn't work. -- Florian Weimer / Red Hat Product Security Team __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager majord...@openssl.org
Re: [PATCH] Reseed PRNG on PID change
On 01/16/2014 01:57 PM, Dr. Stephen Henson wrote:
On Thu, Jan 16, 2014, Florian Weimer wrote:
The additional resolution of a tick counter might make reseeding
after fork unnecessary, but it's difficult to be sure. Something
not based on timing information looks desirable to me.
I should point out that the aim of the current code is not to completely
reseed after fork() but to make the PRNG state diverge so that the two
processes do not share the same internal PRNG state.
I get that. I've reconsidered things. Time seems the only thing that
deals with the related VM snapshot issue.
I still propose to get rid of the time() call (md_rand-time.patch,
AFAICT num == 0 at this point, so I pulled the initialization out of the
loop). OPENSSL_rdtsc() appears to return 0 if the cycle counter is not
available, so md_rand-rtdsc.patch calls it unconditionally if RDRAND is
not available.
--
Florian Weimer / Red Hat Product Security Team
commit 594dcc12d4ede5cd6f4e11cdc23fff3eea20cdcf
Author: Florian Weimer
Date: Thu Jan 16 15:16:38 2014 +0100
ssleay_rand_seed: optimize fork protection: do not call time()
diff --git a/crypto/rand/md_rand.c b/crypto/rand/md_rand.c
index 6cab308..f2165cd 100644
--- a/crypto/rand/md_rand.c
+++ b/crypto/rand/md_rand.c
@@ -368,7 +368,6 @@ static int ssleay_rand_bytes(unsigned char *buf, int num, int pseudo)
#ifndef GETPID_IS_MEANINGLESS
pid_t curr_pid = getpid();
#endif
- time_t curr_time = time(NULL);
int do_stir_pool = 0;
/* time value for various platforms */
#ifdef OPENSSL_SYS_WIN32
@@ -508,6 +507,15 @@ static int ssleay_rand_bytes(unsigned char *buf, int num, int pseudo)
crypto_lock_rand = 0;
CRYPTO_w_unlock(CRYPTO_LOCK_RAND);
+ /* try to diverge the random sequence after fork and VM resumption */
+#ifndef GETPID_IS_MEANINGLESS
+ if (!MD_Update(&m,(unsigned char*)&curr_pid, sizeof curr_pid))
+ goto err;
+#endif
+ if (!MD_Update(&m,(unsigned char*)&tv, sizeof tv))
+ goto err;
+ rand_hw_seed(&m);
+
while (num > 0)
{
/* num_ceil -= MD_DIGEST_LENGTH/2 */
@@ -515,26 +523,6 @@ static int ssleay_rand_bytes(unsigned char *buf, int num, int pseudo)
num-=j;
if (!MD_Init(&m))
goto err;
-#ifndef GETPID_IS_MEANINGLESS
- if (curr_pid) /* just in the first iteration to save time */
- {
- if (!MD_Update(&m,(unsigned char*)&curr_pid,
- sizeof curr_pid))
-goto err;
- curr_pid = 0;
- }
-#endif
- if (curr_time) /* just in the first iteration to save time */
- {
- if (!MD_Update(&m,(unsigned char*)&curr_time,
- sizeof curr_time))
-goto err;
- if (!MD_Update(&m,(unsigned char*)&tv,
- sizeof tv))
-goto err;
- curr_time = 0;
- rand_hw_seed(&m);
- }
if (!MD_Update(&m,local_md,MD_DIGEST_LENGTH))
goto err;
if (!MD_Update(&m,(unsigned char *)&(md_c[0]),sizeof(md_c)))
commit 8cfc51d808a3d3b3118f3d3168fcba6c0ce79b9c
Author: Florian Weimer
Date: Thu Jan 16 15:35:40 2014 +0100
ssleay_rand_bytes: use cycle counter if RDRAND is not available
diff --git a/crypto/rand/md_rand.c b/crypto/rand/md_rand.c
index f2165cd..51acf44 100644
--- a/crypto/rand/md_rand.c
+++ b/crypto/rand/md_rand.c
@@ -382,9 +382,6 @@ static int ssleay_rand_bytes(unsigned char *buf, int num, int pseudo)
#elif defined(OPENSSL_SYS_VXWORKS)
struct timespec tv;
clock_gettime(CLOCK_REALTIME, &ts);
-#elif defined(OPENSSL_SYSNAME_DSPBIOS)
- unsigned long long tv, OPENSSL_rdtsc();
- tv = OPENSSL_rdtsc();
#else
struct timeval tv;
gettimeofday(&tv, NULL);
@@ -674,7 +671,13 @@ static void rand_hw_seed(EVP_MD_CTX *ctx)
{
int i;
if (!(OPENSSL_ia32cap_P[1] & (1<<(62-32
+ {
+ /* using the cycle counter is better than nothing at all */
+ unsigned long long tv, OPENSSL_rdtsc();
+ tv = OPENSSL_rdtsc();
+ MD_Update(ctx, (unsigned char *)&tv, sizeof(tv));
return;
+ }
for (i = 0; i < RDRAND_CALLS; i++)
{
size_t rnd;
@@ -721,7 +724,10 @@ void rand_hw_xor(unsigned char *buf, size_t num)
static void rand_hw_seed(EVP_MD_CTX *ctx)
{
- return;
+ /* using the cycle counter is better than nothing at all */
+ unsigned long long tv, OPENSSL_rdtsc();
+ tv = OPENSSL_rdtsc();
+ MD_Update(ctx, (unsigned char *)&tv, sizeof(tv));
}
void rand_hw_xor(unsigned char *buf, size_t num)
Re: [PATCH] Reseed PRNG on PID change
On Thu, Jan 16, 2014, Florian Weimer wrote: > > The additional resolution of a tick counter might make reseeding > after fork unnecessary, but it's difficult to be sure. Something > not based on timing information looks desirable to me. > I should point out that the aim of the current code is not to completely reseed after fork() but to make the PRNG state diverge so that the two processes do not share the same internal PRNG state. Steve. -- Dr Stephen N. Henson. OpenSSL project core developer. Commercial tech support now available see: http://www.openssl.org __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager majord...@openssl.org
Re: [PATCH] Reseed PRNG on PID change
On 01/15/2014 05:44 PM, Dr. Stephen Henson wrote: On Wed, Jan 15, 2014, Florian Weimer wrote: Commit 3cd8547a2018ada88a4303067a2aa15eadc17f39 mixed the current time into the randomness pool each time RAND_bytes is called. As the resolution of gettimeofday() is limited, I propose to reseed the PRNG each time a PID change is detected. I know historically some platforms have different PIDs for different threads. That would cause problems with this patch. Ah, good old LinuxThreads. I admit it's not totally dead yet. I can add a Linux- and glibc-specific detection function which disables the reseeding if LinuxThreads is detected. (Other systems had strange threading libraries as well, but those generally shared PIDs.) Regarding replacing gettimeofday() with a CPU tick counter, I expect that many systems have a very fast gettimeofday() implementation these days (based in part on a CPU tick counter). However, time() has no such acceleration on the machines I've tested, so the current code shows a significant performance hit compared to 3cd8547a2^. The additional resolution of a tick counter might make reseeding after fork unnecessary, but it's difficult to be sure. Something not based on timing information looks desirable to me. -- Florian Weimer / Red Hat Product Security Team __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager majord...@openssl.org
Re: [PATCH] Reseed PRNG on PID change
Am Donnerstag, 16. Januar 2014, 08:11:32 schrieb Peter Waltenberg: Hi Peter, >The necessary code for MOST platforms is already in OpenSSL > >Look for OPENSSL_rdtsc in the crypto. directory. >O.K. Not all platforms have this, but Sparc, x86/x86_64, s390, ppc, >Itanium, apha,PA-RISC , ARM have some variant - admitted, quite a few >hardware variants don't have a TSC, but that's determinable at runtime >and you could drop back to gettimeofday() to cover those. Great, then using these high-res timers should be definitely considered for the platforms where available (i.e. most common platforms). For any other platform, the gettimeofday can be called. > >It's at least "better" than gettimeofday() for this purpose - i.e. most >of the bits can't be determined from outside the box, it moves faster, >and it's cheaper to read. Agreed. Ciao Stephan __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager majord...@openssl.org
Re: [PATCH] Reseed PRNG on PID change
The necessary code for MOST platforms is already in OpenSSL Look for OPENSSL_rdtsc in the crypto. directory. O.K. Not all platforms have this, but Sparc, x86/x86_64, s390, ppc, Itanium, apha,PA-RISC , ARM have some variant - admitted, quite a few hardware variants don't have a TSC, but that's determinable at runtime and you could drop back to gettimeofday() to cover those. It's at least "better" than gettimeofday() for this purpose - i.e. most of the bits can't be determined from outside the box, it moves faster, and it's cheaper to read. Peter From: Stephan Mueller To: openssl-dev@openssl.org, Date: 16/01/2014 07:45 Subject: Re: [PATCH] Reseed PRNG on PID change Sent by:owner-openssl-...@openssl.org Am Donnerstag, 16. Januar 2014, 07:41:21 schrieb Peter Waltenberg: Hi Peter, >You have access to high speed event counters on most platforms now. >Where those are available, use them for reseed data instead of >gettimeofday(). Far higher resolution, far less performance impact. That implies, however, either hardware-specific code (i.e. special CPU instruction) or per operating system specific code (special system call, library call). Ciao Stephan __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager majord...@openssl.org __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager majord...@openssl.org
Re: [PATCH] Reseed PRNG on PID change
Am Donnerstag, 16. Januar 2014, 07:41:21 schrieb Peter Waltenberg: Hi Peter, >You have access to high speed event counters on most platforms now. >Where those are available, use them for reseed data instead of >gettimeofday(). Far higher resolution, far less performance impact. That implies, however, either hardware-specific code (i.e. special CPU instruction) or per operating system specific code (special system call, library call). Ciao Stephan __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager majord...@openssl.org
Re: [PATCH] Reseed PRNG on PID change
You have access to high speed event counters on most platforms now. Where those are available, use them for reseed data instead of gettimeofday(). Far higher resolution, far less performance impact. Peter From: "Dr. Stephen Henson" To: openssl-dev@openssl.org, Date: 16/01/2014 02:50 Subject: Re: [PATCH] Reseed PRNG on PID change Sent by:owner-openssl-...@openssl.org On Wed, Jan 15, 2014, Florian Weimer wrote: > Commit 3cd8547a2018ada88a4303067a2aa15eadc17f39 mixed the current > time into the randomness pool each time RAND_bytes is called. As > the resolution of gettimeofday() is limited, I propose to reseed the > PRNG each time a PID change is detected. > I know historically some platforms have different PIDs for different threads. That would cause problems with this patch. Steve. -- Dr Stephen N. Henson. OpenSSL project core developer. Commercial tech support now available see: http://www.openssl.org __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager majord...@openssl.org __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager majord...@openssl.org
Re: [PATCH] Reseed PRNG on PID change
On Wed, Jan 15, 2014, Florian Weimer wrote: > Commit 3cd8547a2018ada88a4303067a2aa15eadc17f39 mixed the current > time into the randomness pool each time RAND_bytes is called. As > the resolution of gettimeofday() is limited, I propose to reseed the > PRNG each time a PID change is detected. > I know historically some platforms have different PIDs for different threads. That would cause problems with this patch. Steve. -- Dr Stephen N. Henson. OpenSSL project core developer. Commercial tech support now available see: http://www.openssl.org __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager majord...@openssl.org
[PATCH] Reseed PRNG on PID change
Commit 3cd8547a2018ada88a4303067a2aa15eadc17f39 mixed the current time
into the randomness pool each time RAND_bytes is called. As the
resolution of gettimeofday() is limited, I propose to reseed the PRNG
each time a PID change is detected.
This change might also be an alternative for platforms where the
gettimeofday() call is prohibitively slow.
--
Florian Weimer / Red Hat Product Security Team
commit 136a8da88ff52ad32f894fb0ecbcab5f4205ca49
Author: Florian Weimer
Date: Wed Jan 15 16:46:17 2014 +0100
ssleay_rand_bytes: Re-seed on PID change
diff --git a/crypto/rand/md_rand.c b/crypto/rand/md_rand.c
index 6cab308..75382b5 100644
--- a/crypto/rand/md_rand.c
+++ b/crypto/rand/md_rand.c
@@ -153,6 +153,9 @@ static unsigned char md[MD_DIGEST_LENGTH];
static long md_count[2]={0,0};
static double entropy=0;
static int initialized=0;
+#ifndef GETPID_IS_MEANINGLESS
+static pid_t prev_pid=0;
+#endif
static unsigned int crypto_lock_rand = 0; /* may be set only when a thread
* holds CRYPTO_LOCK_RAND
@@ -201,6 +204,7 @@ static void ssleay_rand_cleanup(void)
md_count[1]=0;
entropy=0;
initialized=0;
+ prev_pid=0;
}
static int ssleay_rand_add(const void *buf, int num, double add)
@@ -439,7 +443,16 @@ static int ssleay_rand_bytes(unsigned char *buf, int num, int pseudo)
{
RAND_poll();
initialized = 1;
+ prev_pid = curr_pid;
+ }
+#ifndef GETPID_IS_MEANINGLESS
+ if (curr_pid != prev_pid)
+ {
+ RAND_poll();
+ prev_pid = curr_pid;
}
+#endif
+
if (!stirred_pool)
do_stir_pool = 1;
