PURIFY flag in md_rand.c and a strongly worded comment
Nicholas Maniscalco
I ask because I came across this comment, in md_rand.c:
#ifndef PURIFY /* purify complains */
/* DO NOT REMOVE THE FOLLOWING CALL TO MD_Update()! */
if (!MD_Update(&m,buf,j))
goto err;
/* We know that line may cause programs such as
purify and valgrind to complain about use of
uninitialized data. */
#endif
Building with PURIFY essentially removes the call to MD_Update and the
strong wording of the comment makes me think that this call is vital to
correctness. I'm looking for confirmation that using an OpenSSL built with
PURIFY is considered secure. That has been my understanding up until I
saw this comment. Now I'm not so sure.
For reference, the comment was introduced in this change,
http://cvs.openssl.org/chngview?cn=17761
Is this something worth adding to the FAQ?
Thanks.
______________________________________________________________________
OpenSSL Project http://www.openssl.org
Development Mailing List opens...@openssl.org
Automated List Manager majo...@openssl.org
William A. Rowe Jr.
> Is using OpenSSL built with the PURIFY flag considered "secure"?
> I ask because I came across this comment, in md_rand.c:
>
> #ifndef PURIFY /* purify complains */
> /* DO NOT REMOVE THE FOLLOWING CALL TO MD_Update()! */
> if (!MD_Update(&m,buf,j))
> goto err;
> /* We know that line may cause programs such as
> purify and valgrind to complain about use of
> uninitialized data. */
> #endif
The last time someone went by such nonsense[1], they created an entirely
exploitable set of keys on all debian/ubuntu-derived distributions. Good
luck with that, and please let us know what you are maintaining, so that
we might avoid such distributions and products.
[1] http://www.debian.org/security/2008/dsa-1571
Nicholas Maniscalco
> On 6/14/2010 7:59 PM, Nicholas Maniscalco wrote:
>> Is using OpenSSL built with the PURIFY flag considered "secure"?
>> I ask because I came across this comment, in md_rand.c:
>>
>> #ifndef PURIFY /* purify complains */
>> /* DO NOT REMOVE THE FOLLOWING CALL TO MD_Update()! */
>> if (!MD_Update(&m,buf,j))
>> goto err;
>> /* We know that line may cause programs such as
>> purify and valgrind to complain about use of
>> uninitialized data. */
>> #endif
>
> The last time someone went by such nonsense[1], they created an entirely
> exploitable set of keys on all debian/ubuntu-derived distributions. Good
> luck with that, and please let us know what you are maintaining, so that
> we might avoid such distributions and products.
>
> [1] http://www.debian.org/security/2008/dsa-1571
Thanks, William. I am familiar with the Debian issue.
The code I pasted above is from ssleay_rand_bytes. Perhaps you were
thinking I was talking about the call in ssleay_rand_add? I am quite
aware that removing the call from ssleay_rand_add is a very bad idea :)
Are you still of the opinion that an OpenSSL built with PURIFY is
insecure? David Schwartz, indicated otherwise in a similar thread I
started a few weeks back (see his last sentence),
http://www.mail-archive.com/opens...@openssl.org/msg27732.html
I was satisfied with his answer until I saw the comment above, hence the
new thread. Again, I'm just trying to get a definitive answer on
whether the PURIFY flag is considered secure. Thanks.
William A. Rowe Jr.
That is a good question. Sure seems incorrect, if anything.
#else
#warning Invalid randomization in md_rand.c when building with -DPURIFY!
#endif
would help alert builders to this trouble.
Dr. Stephen Henson
Well I can give you an initial provisional opinion... I'm being very guarded
when commenting on the PRNG based on past history ;-)
I think that extra comment within the #ifndef PURIFY was added in error. That
call just uses the (possibly unitialised) buffer passed into
ssleay_rand_bytes() as a very minor source of entropy and is not part of
PURIFY builds with no ill effects other than removing that minor source of
entropy.
Richard, it was your commit. Would you care to comment?
Steve.
--
Dr Stephen N. Henson. OpenSSL project core developer.
Commercial tech support now available see: http://www.openssl.org
David Schwartz
Dr. Stephen Henson wrote:
> On Mon, Jun 14, 2010, Nicholas Maniscalco wrote:
> >>> #ifndef PURIFY /* purify complains */
> >>> /* DO NOT REMOVE THE FOLLOWING CALL TO MD_Update()! */
> >>> if (!MD_Update(&m,buf,j))
> >>> goto err;
> >>> /* We know that line may cause programs such as
> >>> purify and valgrind to complain about use of
> >>> uninitialized data. */
> >>> #endif
> I think that extra comment within the #ifndef PURIFY was added in
> error. That
> call just uses the (possibly unitialised) buffer passed into
> ssleay_rand_bytes() as a very minor source of entropy and is not part
> of
> PURIFY builds with no ill effects other than removing that minor source
> of
> entropy.
I don't think the comment was added in error, just that it's a bit
misleading. The purpose of the comment is to indicate that the decision to
add uninitialized data to the pool was intentional and that this line
reflects an intentional design decision. (And a correct one, IMO.)
If the code is not 100% reliable without this line, then that's a bug in
OpenSSL. However, I am quite convinced that it is, for all practical
purposes, equally secure with or without this line of code.
The code simply makes a different trade-off with PURIFY defined than without
it. Both trade-offs are, IMO, appropriate for their respective conditions.
OpenSSL with PURIFY defined is perfectly suitable for production use.
DS
Richard Levitte
steve> On Mon, Jun 14, 2010, Nicholas Maniscalco wrote:
steve>
steve> > William A. Rowe Jr. wrote:
steve> >> On 6/14/2010 7:59 PM, Nicholas Maniscalco wrote:
steve> >>> Is using OpenSSL built with the PURIFY flag considered "secure"?
steve> >>> I ask because I came across this comment, in md_rand.c:
steve> >>>
steve> >>> #ifndef PURIFY /* purify complains */
steve> >>> /* DO NOT REMOVE THE FOLLOWING CALL TO MD_Update()! */
steve> >>> if (!MD_Update(&m,buf,j))
steve> >>> goto err;
steve> >>> /* We know that line may cause programs such as
steve> >>> purify and valgrind to complain about use of
steve> >>> uninitialized data. */
steve> >>> #endif
steve> >> The last time someone went by such nonsense[1], they created an entirely
steve> >> exploitable set of keys on all debian/ubuntu-derived distributions. Good
steve> >> luck with that, and please let us know what you are maintaining, so that
steve> >> we might avoid such distributions and products.
steve> >> [1] http://www.debian.org/security/2008/dsa-1571
steve> >
steve> > Thanks, William. I am familiar with the Debian issue.
steve> >
steve> > The code I pasted above is from ssleay_rand_bytes. Perhaps you were
steve> > thinking I was talking about the call in ssleay_rand_add? I am quite aware
steve> > that removing the call from ssleay_rand_add is a very bad idea :)
steve> >
steve> > Are you still of the opinion that an OpenSSL built with PURIFY is insecure?
steve> > David Schwartz, indicated otherwise in a similar thread I started a few
steve> > weeks back (see his last sentence),
steve> >
steve> > http://www.mail-archive.com/opens...@openssl.org/msg27732.html
steve> >
steve> > I was satisfied with his answer until I saw the comment above, hence the
steve> > new thread. Again, I'm just trying to get a definitive answer on whether
steve> > the PURIFY flag is considered secure. Thanks.
steve>
steve> Well I can give you an initial provisional opinion... I'm being very guarded
steve> when commenting on the PRNG based on past history ;-)
steve>
steve> I think that extra comment within the #ifndef PURIFY was added in error. That
steve> call just uses the (possibly unitialised) buffer passed into
steve> ssleay_rand_bytes() as a very minor source of entropy and is not part of
steve> PURIFY builds with no ill effects other than removing that minor source of
steve> entropy.
steve>
steve> Richard, it was your commit. Would you care to comment?
Sure.
I added those comments in direct reaction to the Debian issue. At the
time, the removal of both the MD_update() in ssleay_rand_add() and in
ssleay_rand_bytes() were pointed out as problem sources, so I added
that comment on both.
However, the trouble here seems not to be so much about the comments
as they are about the use of PURIFY. For all I care, the condition
can be removed, leaving that piece of code to be compiled with purify
as well as without.
I've honestly not made much analysis about the code that's talked
about here. Randomness on this level is not my strong point, so I'd
rather listen to those for who it is.
If that piece of code is such a small source of entropy, it's quite
possible we can remove it safely. I don't dare, though! ;-)
Cheers,
Richard
--
Richard Levitte ric...@levitte.org
http://richard.levitte.org/
"Life is a tremendous celebration - and I'm invited!"
-- from a friend's blog, translated from Swedish
Dr. Stephen Henson
> In message <20100615164...@openssl.org> on Tue, 15 Jun 2010 18:44:03 +0200, "Dr. Stephen Henson" <st...@openssl.org> said:
>
> If that piece of code is such a small source of entropy, it's quite
> possible we can remove it safely. I don't dare, though! ;-)
>
I don't either ;-) I'll clarify the comment.
Steve.
--
Dr Stephen N. Henson. OpenSSL project core developer.
Commercial tech support now available see: http://www.openssl.org
Nicholas Maniscalco
> On Wed, Jun 16, 2010, Richard Levitte wrote:
>
>
>> In message <20100615164...@openssl.org> on Tue, 15 Jun 2010 18:44:03 +0200, "Dr. Stephen Henson" <st...@openssl.org> said:
>>
>> If that piece of code is such a small source of entropy, it's quite
>> possible we can remove it safely. I don't dare, though! ;-)
>>
>>
>
> I don't either ;-) I'll clarify the comment.
>
Thanks, I appreciate that.
Do you think it's worth updating the existing Valgrind related FAQ
entry to mention that PURIFY builds are intended to be secure for
production use? The one I'm referring to is titled "Why does Valgrind
complain about the use of uninitialized data?"