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: 'openssl-dev@openssl.org' 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);
