Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date: Mon, 17 Jun 2019 12:49:42 +0200
From: Raphael Geissert <geissert@...ian.org>
To: Open Source Security <oss-security@...ts.openwall.com>
Cc: security@...tpractical.com
Subject: Re: Apache::Session's use of md5 and more

Hi,

On Sat, 15 Jun 2019 at 19:42, Solar Designer <solar@...nwall.com> wrote:
> On Sat, Jun 15, 2019 at 05:09:53PM +0200, Raphael Geissert wrote:
> > Not only does it use MD5,
>
> Which is perfectly fine for this use case, except that it distracts
> attention from real issues, so might need to be "fixed" to be e.g.
> SHA-256 for that reason.
>
> Let's not confuse technical and psychological aspects.
[...]
> > and does two rounds of hashing.
>
> This is fine, but can be optimized out along with the move to SHA-256.

Right, though I must argue that they are indicators, smells. Also, I
hope that by getting rid of those people won't copy that code
elsewhere.

> I didn't review Perl's rand(), but apparently Nuel thought the
> initialization from /dev/urandom on newer Perl somehow made rand() safe
> from having its seed inferred?  I doubt this is the case, as I expect
> the seed and/or the internal state is tiny either way.  And I doubt it
> takes as many as "30 values of rand() to determine the srand (the
> seed)."  I'd expect 1 to be enough.  But we need to review the code
> before making any claims.
>
> ...OK, I just took a look.  Perl's util.c: Perl_seed() reads just 32
> bits from /dev/urandom, with compile-time and runtime fallbacks to
> gettimeofday() and getpid() and some more ASLR leaks.  (Fun fact: the
> fallbacks will also occur when the 32-bit value read from /dev/urandom
> just happens to be 0.  As a result, the seed is almost never a 0.)

Which is more worrisome in the specific case of lemonldap-ng given the
use of rand for quite many things. Not sure how RT is affected in that
regard. From issue 1633 [2]:

> From a quick survey through the code, I found that Perl's rand is used
>
> For password reset (::Portal::Lib::SMTP) through String::Random
> For OpenID registration (::Portal::Issues::OpenIDConnect) through String::Random
> For CSRF and OTP login token generation (::Portal::Lib::OneTimeToken)
> For Session ID generation (::Common::Apache::Session::Generate::SHA256)
> For password hashing in databases (::Portal::Lib::DBI)
> For TOTP registration (::Common::TOTP)

So far the use of rand in the session id generation code has been
replaced by data from urandom - but they left time, pid, and {}.

FWIW I had opened issue 1803 [3] for the uses of String::Random, but
it looks like it is best to just reopen 1633 - or whatever is
necessary so that the remaining uses of rand are fixed.

Oh and it appears that Apache::SessionX is a fork of Apache::Session,
with the same session id generation function.

[2] https://gitlab.ow2.org/lemonldap-ng/lemonldap-ng/issues/1633
[3] https://gitlab.ow2.org/lemonldap-ng/lemonldap-ng/issues/1803

Cheers,
--
Raphael Geissert - Debian Developer
www.debian.org

Powered by blists - more mailing lists

Please check out the Open Source Software Security Wiki, which is counterpart to this mailing list.

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.