Any further thoughts on this patch? I'd love to get these -DPURIFY builds
working properly.
--Kevin
crypto/rand/randfile.c :
@@ -102,6 +102,14 @@
if (file == NULL) return(0);
+#ifdef PURIFY
+ /* struct stat has padding and unused fields that may not be
+ * initialized in the call to stat(). We need to clear the entire
+ * structure before calling RAND_add() to avoid complaints from
+ * applications such as Valgrind.
+ */
+ memset(&sb, 0, sizeof(sb));
+#endif
if (stat(file,&sb) < 0) return(0);
RAND_add(&sb,sizeof(sb),0.0);
if (bytes == 0) return(ret);
From: Kevin Regan
Sent: Friday, January 08, 2010 11:01 AM
To: Kevin Regan; '[email protected]'
Subject: RE: RE: Change needed for "-DPURIFY" builds.
I also wanted to mention that the -DPURIFY flag should not weaken the PRNG (you
should be able to run a production system with -DPURIFY). It should only be
used to remove uninitialized data that may (or may not) add some entropy to the
PRNG. I believe that my patch handles this correctly.
--Kevin
From: Kevin Regan
Sent: Friday, January 08, 2010 10:53 AM
To: '[email protected]'
Subject: RE: Change needed for "-DPURIFY" builds.
>> You're missing the point -- your comment is the height of irony, in a
>> way.
>>
>> Use a suppression to make Valgrind shut up.
>>
>> /r$
>
> I think you misunderstand his issue. His issue is not "valgrind reports a
> spurious error/warning". His issue is "-DPURIFY does not do what I think
> it's supposed to do". Perhaps my recollection is incorrect, but my
> recollection was that "-DPURIFY" was supposed to make OpenSSL not process
> uninitialized data with code precisely like that suggested here, accepting
> that there might be some reduction in the amount of entropy that OpenSSL
> gets in exchange for having only one switch to flip.
>
> The "-DPURIFY" flag is used precisely this way in other projects, shutting
> off all manner of 'optimizations' that interfere with tools like Purify,
> Valgrind, and others.
>
> DS
This is correct. Here is the relevant entry in the OpenSSL FAQ (#14):
http://www.openssl.org/support/faq.html#PROG14
The -PURIFY flag is meant to remove additions to the PRNG that would cause
trouble for applications such as Purify and Valgrind. The issue that I pointed
out is a location where the -DPURIFY flag was missed. It shouldn't be
controversial. This is exactly what the flag is meant for. My change simply
make the FAQ statement correct by fixing a bug.
As for suppressions, it isn't possible in this case. The unintialized data
taints all users of the openssl random number generator (directly or
indirectly). This causes problems throughout the code. You would need dozens
or hundreds of suppressions, none of which actually mention the RAND_add
function (so, I guess it is "possible", but prohibitively expensive).
As a reference, here are the other two PURIFY entries in the code:
crypto/rand/randfile.c:
#ifdef PURIFY
RAND_add(buf,i,(double)i);
#else
/* even if n != i, use the full array */
RAND_add(buf,n,(double)i);
#endif
crypto/rand/md_rand.c:
MD_Update(&m,local_md,MD_DIGEST_LENGTH);
MD_Update(&m,(unsigned char
*)&(md_c[0]),sizeof(md_c));
#ifndef PURIFY
MD_Update(&m,buf,j); /* purify complains */
#endif
--Kevin
p.s. Here, again, is the patch I am suggestion:
crypto/rand/randfile.c :
@@ -102,6 +102,14 @@
if (file == NULL) return(0);
+#ifdef PURIFY
+ /* struct stat has padding and unused fields that may not be
+ * initialized in the call to stat(). We need to clear the entire
+ * structure before calling RAND_add() to avoid complaints from
+ * applications such as Valgrind.
+ */
+ memset(&sb, 0, sizeof(sb));
+#endif
if (stat(file,&sb) < 0) return(0);
RAND_add(&sb,sizeof(sb),0.0);
if (bytes == 0) return(ret);
