valgrind and openssl

[John Parker-5]

In the wake of the issues with Debian, is it possible to modify the
source so that it is possible to use valgrind with openssl without
reducing the key space?

Are we really relying on uninitialized memory for randomness?

-JP
______________________________________________________________________
OpenSSL Project                                 http://www.openssl.org
Development Mailing List                       openssl-dev@...
Automated List Manager                           majordomo@...

[Patrick Patterson-3]

On May 15, 2008 10:58:07 am John Parker wrote:
> In the wake of the issues with Debian, is it possible to modify the
> source so that it is possible to use valgrind with openssl without
> reducing the key space?
>
It is already possible to use openssl and valgrind - just build OpenSSL
with -DPURIFY, and it is quite clean.

(we do it all the time here with WvStreams and Pathfinder, and it works like a
charm).

Have fun.

--
Patrick Patterson
President and Chief PKI Architect,
Carillon Information Security Inc.
http://www.carillon.ca
______________________________________________________________________
OpenSSL Project                                 http://www.openssl.org
Development Mailing List                       openssl-dev@...
Automated List Manager                           majordomo@...

[Bodo Moeller]

On Thu, May 15, 2008 at 4:58 PM, John Parker <namejohnparker@...> wrote:

> In the wake of the issues with Debian, is it possible to modify the
> source so that it is possible to use valgrind with openssl without
> reducing the key space?

Sure.  This might happen with the next release.

> Are we really relying on uninitialized memory for randomness?

Not at all.  It's just that OpenSSL in some situations tries to feed
possibly uninitialized memory into the random number generator anyway,
essentially just for fun and because their *might* be some actual
randomness there from whatever happened earlier in the same process.

The Debian-internal patch was blatantly overbroad in disabling the
essential functionality of the RAND_add() function rather than just
avoiding the one case where this function might have been called with
uninitialized memory.  (That one case is in RAND_load_file(), which
would intentionally feed a complete 1024-byte buffer to RAND_add()
even if fewer than 1024 bytes had been put into the buffer by
fread().)

Bodo
______________________________________________________________________
OpenSSL Project                                 http://www.openssl.org
Development Mailing List                       openssl-dev@...
Automated List Manager                           majordomo@...

[Geoff Thorpe]

Just to follow up on Bodo's comment here;

On Thursday 15 May 2008 11:11:50 Bodo Moeller wrote:
> > Are we really relying on uninitialized memory for randomness?
>
> Not at all.  It's just that OpenSSL in some situations tries to feed
> possibly uninitialized memory into the random number generator anyway,
> essentially just for fun and because their *might* be some actual
> randomness there from whatever happened earlier in the same process.

As Bodo says, the worst-case is that this doesn't help but doesn't harm
either. In the best case, buffers provided by the caller may contain some
data we can mix into the PRNG. However, *if* (and I stress the conditional
here) the caller buffers *do* happen to contain any entropy, that provides
two potential benefits to openssl and the caller application;
(1) callers are not just draining the PRNG one-directionally but are also
helping to provide some entropy as well (NB, we don't score this entropy
because it may be low-quality or even useless),
(2) if the buffers passed from different points in the caller code vary at all
(even if the entropy within each such buffer is low-quality or even 100%
predictable) then the PRNG state becomes caller-pattern-dependent. Ie. if the
pattern of calls to openssl's PRNG varies between runs, then the PRNG state
is fundamentally altered between runs too. Each such buffer could always
contain the same data on entry, but the *order* in which those buffers hits
the PRNG could provide some entropy if, for example, threading is
unpredictable. Ie. there is potentially entropy in the order of the calls,
rather than in the buffers being passed in those calls.

All of this is independent of proper entropy seeding to the PRNG, which is
what the debian patch crushed and which in turn led to the high seismic
reading in the blogosphere. But it may help explain why I do *not* want us to
unilaterally remove the use of uninitialised data in the PRNG. That seems to
be motivated by a capitulation to the weight of users (or packagers) who
don't know how to read the FAQ. Perhaps what we should do instead is
change -DPURIFY to -DNO_UNINIT_DATA or something else which has a clearer
intention, so that debug packages (or even base packages that want to be
valgrind-friendly) have a straightforward mechanism to apply. Well, a
straightforward mechanism that doesn't kill the PRNG outright, I mean
(otherwise there is already a highly-publicised patch we could apply...)

Cheers,
Geoff

______________________________________________________________________
OpenSSL Project                                 http://www.openssl.org
Development Mailing List                       openssl-dev@...
Automated List Manager                           majordomo@...

[Bruce Stephens-4]

Patrick Patterson <ppatterson@...> writes:

> On May 15, 2008 10:58:07 am John Parker wrote:
>> In the wake of the issues with Debian, is it possible to modify the
>> source so that it is possible to use valgrind with openssl without
>> reducing the key space?
>>
> It is already possible to use openssl and valgrind - just build OpenSSL
> with -DPURIFY, and it is quite clean.

Even with -DPURIFY there was (prior to 0.9.8f) some uninitialised
memory got used if you didn't have a .rnd file (the stat() in
crypto/rand/randfile.c).  (I'm not sure whether that change ever got
on to 0.9.7.  Hmm, I think it didn't.  Likely not critical, I guess,
though Debian/stable is on 0.9.7.  Backporting that (trivial) change
seems like something a distribution could uncontroversially do.)
______________________________________________________________________
OpenSSL Project                                 http://www.openssl.org
Development Mailing List                       openssl-dev@...
Automated List Manager                           majordomo@...

[John Parker-5]

> It is already possible to use openssl and valgrind - just build OpenSSL
> with -DPURIFY, and it is quite clean.
>
> (we do it all the time here with WvStreams and Pathfinder, and it works like a
> charm).

The problem is that this may reduce the keyspace so that keys are guessable.

http://blog.isotoma.com/2008/05/debians_openssl_disaster.html

-JP
______________________________________________________________________
OpenSSL Project                                 http://www.openssl.org
Development Mailing List                       openssl-dev@...
Automated List Manager                           majordomo@...

[Geoff Thorpe]

On Thursday 15 May 2008 11:52:08 John Parker wrote:
> > It is already possible to use openssl and valgrind - just build OpenSSL
> > with -DPURIFY, and it is quite clean.
> >
> > (we do it all the time here with WvStreams and Pathfinder, and it works
> > like a charm).
>
> The problem is that this may reduce the keyspace so that keys are
> guessable.

No it won't, it removes an "entropy source" whose quality is known to be
unknown, ie. it may add nothing useful, it gets used "just in case". Removing
it does not "reduce the keypsace" at all. All you can say is that leaving it
there *may* improve the PRNG depending on the user, the environment, the
application, and quite probably, the alignment of the planets...

The debian patch went further than -DPURIFY, as it removed more than just
this "unreliable" source, it removed all use of reliable sources as well.

> http://blog.isotoma.com/2008/05/debians_openssl_disaster.html

This blog does not suggest that building with -DPURIFY would a problem and nor
should it. I think you may have misunderstood the details of this issue.

Cheers,
Geoff

______________________________________________________________________
OpenSSL Project                                 http://www.openssl.org
Development Mailing List                       openssl-dev@...
Automated List Manager                           majordomo@...

[John Parker-5]

> All of this is independent of proper entropy seeding to the PRNG, which is
> what the debian patch crushed and which in turn led to the high seismic
> reading in the blogosphere. But it may help explain why I do *not* want us to
> unilaterally remove the use of uninitialised data in the PRNG. That seems to
> be motivated by a capitulation to the weight of users (or packagers) who
> don't know how to read the FAQ. Perhaps what we should do instead is

I think we should be less worried how things "seem" and more worried
about the practical consequences.

> change -DPURIFY to -DNO_UNINIT_DATA or something else which has a clearer
> intention, so that debug packages (or even base packages that want to be
> valgrind-friendly) have a straightforward mechanism to apply. Well, a
> straightforward mechanism that doesn't kill the PRNG outright, I mean
> (otherwise there is already a highly-publicised patch we could apply...)

What I was hoping for was a -DNO_UNINIT_DATA that wouldn't be the
default, but wouldn't reduce the keyspace either.

Can someone provide a pointer to this highly-publicized patch?  I'm
afraid I'm dreadfully ignorant of the blogosphere.

-JP
______________________________________________________________________
OpenSSL Project                                 http://www.openssl.org
Development Mailing List                       openssl-dev@...
Automated List Manager                           majordomo@...

[John Parker-5]

>> > It is already possible to use openssl and valgrind - just build OpenSSL
>> > with -DPURIFY, and it is quite clean.

Actually on my system, just -DPURIFY doesn't satisfy valgrind.  What
I'm asking for is something that both satisfies valgrind and doesn't
reduce the keyspace.

>> > (we do it all the time here with WvStreams and Pathfinder, and it works
>> > like a charm).
>>
>> The problem is that this may reduce the keyspace so that keys are
>> guessable.
>
> No it won't, it removes an "entropy source" whose quality is known to be
> unknown, ie. it may add nothing useful, it gets used "just in case". Removing
> it does not "reduce the keypsace" at all. All you can say is that leaving it
> there *may* improve the PRNG depending on the user, the environment, the
> application, and quite probably, the alignment of the planets...
>
> The debian patch went further than -DPURIFY, as it removed more than just
> this "unreliable" source, it removed all use of reliable sources as well.
>
>> http://blog.isotoma.com/2008/05/debians_openssl_disaster.html
>
> This blog does not suggest that building with -DPURIFY would a problem and nor
> should it. I think you may have misunderstood the details of this issue.

I am clearly misunderstanding something.  You seem to be saying that
-DPURIFY satisfies valgrind but doesn't reduce the keyspace.  I'm
prepared to take it on faith that -DPURIFY doesn't reduce the
keyspace.

-JP
______________________________________________________________________
OpenSSL Project                                 http://www.openssl.org
Development Mailing List                       openssl-dev@...
Automated List Manager                           majordomo@...

[Leandro Santi]

John Parker, 2008-05-15:
> >> > It is already possible to use openssl and valgrind - just build OpenSSL
> >> > with -DPURIFY, and it is quite clean.
>
> Actually on my system, just -DPURIFY doesn't satisfy valgrind.  What
> I'm asking for is something that both satisfies valgrind and doesn't
> reduce the keyspace.

Valgrind can be told to ignore specific errors, using a
suppressions file. Never used this with OpenSSL, though.

http://valgrind.org/docs/manual/manual-core.html#manual-core.suppress

Leandro
______________________________________________________________________
OpenSSL Project                                 http://www.openssl.org
Development Mailing List                       openssl-dev@...
Automated List Manager                           majordomo@...

[Patrick Patterson-3]

On May 15, 2008 12:38:24 pm John Parker wrote:
> >> > It is already possible to use openssl and valgrind - just build
> >> > OpenSSL with -DPURIFY, and it is quite clean.
>
> Actually on my system, just -DPURIFY doesn't satisfy valgrind.  What
> I'm asking for is something that both satisfies valgrind and doesn't
> reduce the keyspace.
>

What you simply need to do is make a supplemental "ignore known issues" file
for valgrind - for WvStreams, we use one that has the following in it:

{
   more_fun_libcrypto_junk
   Memcheck:Value4
   fun:BF_encrypt
}
{
   more_fun_libcrypto_junk_2
   Memcheck:Value4
   fun:AES_encrypt
}
{
   more_fun_libcrypto_junk_3
   Memcheck:Addr4
   fun:AES_cbc_encrypt
}
{
   more_fun_libcrypto_junk_4
   Memcheck:Value4
   fun:_x86_AES_encrypt
}

And then we run all of the unit tests through valgrind with the options:

valgrind --tool=memcheck --leak-check=yes --num-callers=10 --suppressions=$(WVSTREAMS_SRC)/wvstreams.supp

Between -DPURIFY and the suppressions, the unit tests come back clean (when we
haven't made any silly mistakes in our own code, of course :)

BTW: I'm not claiming that the above list of suppressions will work 100% for
you - the suppressions above are for things that our code tickles - you may
want to add more of them for those specific areas that your code touches that
ours does not.

Have fun.

--
Patrick Patterson
President and Chief PKI Architect,
Carillon Information Security Inc.
http://www.carillon.ca
______________________________________________________________________
OpenSSL Project                                 http://www.openssl.org
Development Mailing List                       openssl-dev@...
Automated List Manager                           majordomo@...

[Geoff Thorpe]

On Thursday 15 May 2008 12:38:24 John Parker wrote:
> >> > It is already possible to use openssl and valgrind - just build
> >> > OpenSSL with -DPURIFY, and it is quite clean.
>
> Actually on my system, just -DPURIFY doesn't satisfy valgrind.  What
> I'm asking for is something that both satisfies valgrind and doesn't
> reduce the keyspace.

If you're using an up-to-date version of openssl when you see this (ie. a
recent CVS snapshot from our website, even if it's from a stable branch for
compatibility reasons), then please post details. -DPURIFY exists to
facilitate debuggers that don't like reading uninitialised data, so if that's
not the case then please provide details. Note however that there are a
variety of gotchas that allow you to create little leaks if you're not
careful, and valgrind could well be complaining about those instead.

> > This blog does not suggest that building with -DPURIFY would a problem
> > and nor should it. I think you may have misunderstood the details of this
> > issue.
>
> I am clearly misunderstanding something.  You seem to be saying that
> -DPURIFY satisfies valgrind but doesn't reduce the keyspace.  I'm
> prepared to take it on faith that -DPURIFY doesn't reduce the
> keyspace.

Well, more generally than some "keyspace" is the randomness of the PRNG
itself. (Your keys are only random if the PRNG's output is random.) But yes,
I'm saying that -DPURIFY does not diminish the quality of the PRNG, except
*possibly* by some unquantifiable amount that you couldn't safely depend on
anyway.

As for your other mail;

On Thursday 15 May 2008 12:09:46 John Parker wrote:

> > All of this is independent of proper entropy seeding to the PRNG, which
> > is what the debian patch crushed and which in turn led to the high
> > seismic reading in the blogosphere. But it may help explain why I do
> > *not* want us to unilaterally remove the use of uninitialised data in the
> > PRNG. That seems to be motivated by a capitulation to the weight of users
> > (or packagers) who don't know how to read the FAQ. Perhaps what we should
> > do instead is
>
> I think we should be less worried how things "seem" and more worried
> about the practical consequences.

That is more or less what I was doing. I hope that was clear.

> > change -DPURIFY to -DNO_UNINIT_DATA or something else which has a clearer
> > intention, so that debug packages (or even base packages that want to be
> > valgrind-friendly) have a straightforward mechanism to apply. Well, a
> > straightforward mechanism that doesn't kill the PRNG outright, I mean
> > (otherwise there is already a highly-publicised patch we could apply...)
>
> What I was hoping for was a -DNO_UNINIT_DATA that wouldn't be the
> default, but wouldn't reduce the keyspace either.

I believe this has been answered. For now, it's called -DPURIFY.

> Can someone provide a pointer to this highly-publicized patch?  I'm
> afraid I'm dreadfully ignorant of the blogosphere.

You started this mail thread, so you go and find it! :-) The patch I was
referring to, tongue-in-cheek, is the debian patch that crippled the PRNG. As
for the blogosphere, you aren't missing much, I'd recommend that
continued "ignorance" would be far from dreadful - in fact I intend to join
you in that respect, once this was-it-debian's-fault-or-openssl's-fault
nonsense has died down a bit.

Cheers,
Geoff

______________________________________________________________________
OpenSSL Project                                 http://www.openssl.org
Development Mailing List                       openssl-dev@...
Automated List Manager                           majordomo@...

[Geoff Thorpe]

I forgot to mention something;

> On Thursday 15 May 2008 12:38:24 John Parker wrote:
> > >> > It is already possible to use openssl and valgrind - just build
> > >> > OpenSSL with -DPURIFY, and it is quite clean.
> >
> > Actually on my system, just -DPURIFY doesn't satisfy valgrind.  What
> > I'm asking for is something that both satisfies valgrind and doesn't
> > reduce the keyspace.
>
> If you're using an up-to-date version of openssl when you see this (ie. a
> recent CVS snapshot from our website, even if it's from a stable branch for
> compatibility reasons), then please post details. -DPURIFY exists to
> facilitate debuggers that don't like reading uninitialised data, so if
> that's not the case then please provide details. Note however that there
> are a variety of gotchas that allow you to create little leaks if you're
> not careful, and valgrind could well be complaining about those instead.

Note that you should always build with "no-asm" if you're doing this kind of
debug analysis. The assembly optimisations are likely to operate at
granularities and in ways that valgrind could easily complain about. I don't
know that this is the case, but it would certainly make sense to compare
before posting a bug report.

Cheers,
Geoff

______________________________________________________________________
OpenSSL Project                                 http://www.openssl.org
Development Mailing List                       openssl-dev@...
Automated List Manager                           majordomo@...

[Theodore Tso]

On Thu, May 15, 2008 at 11:09:46AM -0500, John Parker wrote:
> > change -DPURIFY to -DNO_UNINIT_DATA or something else which has a clearer
> > intention, so that debug packages (or even base packages that want to be
> > valgrind-friendly) have a straightforward mechanism to apply. Well, a
> > straightforward mechanism that doesn't kill the PRNG outright, I mean
> > (otherwise there is already a highly-publicised patch we could apply...)
>
> What I was hoping for was a -DNO_UNINIT_DATA that wouldn't be the
> default, but wouldn't reduce the keyspace either.

-DPURIFY *does* do what you want.  It doesn't reduce the keyspace.

The problem was that what Debian did went far beyond -DPURIFY.  The
Debian developer in question disabled one call that used uninitialized
memory, but then later on, removed another similar call that looked
the same, but in fact *was* using initialized data --- said
initialized data being real randomness critically necessary for
security.

                                                        - Ted
______________________________________________________________________
OpenSSL Project                                 http://www.openssl.org
Development Mailing List                       openssl-dev@...
Automated List Manager                           majordomo@...

[Jeffrey Altman-2]

John Parker wrote:
>> change -DPURIFY to -DNO_UNINIT_DATA or something else which has a clearer
>> intention, so that debug packages (or even base packages that want to be
>> valgrind-friendly) have a straightforward mechanism to apply. Well, a
>> straightforward mechanism that doesn't kill the PRNG outright, I mean
>> (otherwise there is already a highly-publicised patch we could apply...)
>
> What I was hoping for was a -DNO_UNINIT_DATA that wouldn't be the
> default, but wouldn't reduce the keyspace either.
That is -DPURIFY.
>
> Can someone provide a pointer to this highly-publicized patch?  I'm
> afraid I'm dreadfully ignorant of the blogosphere.
The Debian patch is the highly publicized patch that kills the PRNG
outright.

Jeffrey Altman

[Bodo Moeller]

On Thu, May 15, 2008 at 7:53 PM, Theodore Tso <tytso@...> wrote:
> On Thu, May 15, 2008 at 11:09:46AM -0500, John Parker wrote:

>> What I was hoping for was a -DNO_UNINIT_DATA that wouldn't be the
>> default, but wouldn't reduce the keyspace either.
>
> -DPURIFY *does* do what you want.  It doesn't reduce the keyspace.
>
> The problem was that what Debian did went far beyond -DPURIFY.  The
> Debian developer in question disabled one call that used uninitialized
> memory, but then later on, removed another similar call that looked
> the same, but in fact *was* using initialized data --- said
> initialized data being real randomness critically necessary for
> security.

This similar call would, under certain conditions, use uninitialized
data too.  I guess Valgrind is more thorough than Purify, because it
seems that those using Purify were not shown this as suspicious, and
thus -DPURIFY didn't cover this particular case.  Of course, totally
disabling the offending call in md_rand.c as was done in Debian was
blatantly wrong.  The correct way would have been to change
RAND_load_file() in randfile.c; that's the function thatt might
sometimes pass uninitialized data to RAND_add() (intentionally, but
without relying on this uninitialized data as a source of randomness).

One of the offending RAND_add() calls has already been taken care of
about a year ago:

    http://cvs.openssl.org/filediff?f=openssl/crypto/rand/randfile.c&v1=1.47.2.1&v2=1.47.2.2

However, another intentional use of potentially unitialized data is
still left as of
http://cvs.openssl.org/getfile/openssl/crypto/rand/randfile.c?v=1.47.2.2
:

                i=fread(buf,1,n,in);
                if (i <= 0) break;
                /* even if n != i, use the full array */
                RAND_add(buf,n,(double)i);

Changing this into RAND_add(buf,i,(double)i) should make verification
tools happier.  Or it could be

#ifdef PURIFY
                RAND_add(buf,i,(double)i);
#else
                RAND_add(buf,n,(double)i);
#endif

(abusing the "PURIFY" macro with a more general meaning).

Bodo
______________________________________________________________________
OpenSSL Project                                 http://www.openssl.org
Development Mailing List                       openssl-dev@...
Automated List Manager                           majordomo@...

[John Parker-5]

On Thu, May 15, 2008 at 12:29 PM, Geoff Thorpe <geoff@...> wrote:

> I forgot to mention something;
>
>> On Thursday 15 May 2008 12:38:24 John Parker wrote:
>> > >> > It is already possible to use openssl and valgrind - just build
>> > >> > OpenSSL with -DPURIFY, and it is quite clean.
>> >
>> > Actually on my system, just -DPURIFY doesn't satisfy valgrind.  What
>> > I'm asking for is something that both satisfies valgrind and doesn't
>> > reduce the keyspace.
>>
>> If you're using an up-to-date version of openssl when you see this (ie. a
>> recent CVS snapshot from our website, even if it's from a stable branch for
>> compatibility reasons), then please post details. -DPURIFY exists to
>> facilitate debuggers that don't like reading uninitialised data, so if
>> that's not the case then please provide details. Note however that there
>> are a variety of gotchas that allow you to create little leaks if you're
>> not careful, and valgrind could well be complaining about those instead.
>
> Note that you should always build with "no-asm" if you're doing this kind of
> debug analysis. The assembly optimisations are likely to operate at
> granularities and in ways that valgrind could easily complain about. I don't
> know that this is the case, but it would certainly ma