Re: Apache::Session's use of md5 and more

[Raphael Geissert <geissert () debian org>]

Hi,

On Sat, 15 Jun 2019 at 19:42, Solar Designer <solar () openwall 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