Re: [COMMITTERS] pgsql: Replace PostmasterRandom() with a stronger way of generating ran

From: Heikki Linnakangas <hlinnaka(at)iki(dot)fi>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: [COMMITTERS] pgsql: Replace PostmasterRandom() with a stronger way of generating ran
Date: 2016-10-17 17:48:58
Message-ID: f89dc573-902c-3f6a-380c-a8a3f5ec59ad@iki.fi
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers pgsql-hackers

On 10/17/2016 06:21 PM, Tom Lane wrote:
> Heikki Linnakangas <hlinnaka(at)iki(dot)fi> writes:
>> On 10/17/2016 05:50 PM, Tom Lane wrote:
>>> The real issue here is whether we are willing to say that
>>> Postgres simply does not work anymore on machines without standard entropy
>>> sources. Doesn't matter whether the user cares about the strength of
>>> cancel keys, we're just blowing them off. That seems a bit extreme
>>> from here. I think we should be willing to fall back to the old code
>>> if we can't find a real entropy source.
>
>> I'm scared of having pg_strong_random() that is willing to fall back to
>> not-so-strong values. We can rename it, of course, but it seems
>> dangerous to use a weak random-number generator for authentication
>> purposes (query cancel, MD5 salts, SCRAM nonces).
>
> I think that it's probably moot on all modern platforms, and even on
> platforms as old as pademelon, the answer for people who care about
> strong security is "--with-openssl". What I'm on about here is whether
> we should make people who don't care about that jump through hoops.
> Not caring is a perfectly reasonable stance for non-exposed postmasters;
> otherwise we wouldn't have the "trust" auth method.
>
> I would be satisfied with making it a non-default build option, eg
> add this to pg_strong_random:
>
> if (random_from_file("/dev/random", buf, len))
> return true;
>
> +#ifdef ALLOW_WEAK_SECURITY
> + ... old PostmasterRandom method here ...
> +#endif
> +
> /* None of the sources were available. */
> return false;

I also changed pgcrypto to use pg_strong_random(). Using a strong random
number generator is even more important for pgcrypto, since it might be
used for generating encryption keys. Would we allow a weak generator for
pgcrypto too, with ALLOW_WEAK_SECURITY? If we don't, then pgcrypto test
cases will still fail on pademelon. If we consider the option to be just
for testing, never for production use, then we could allow it, but it
feels a bit dangerous, and I'm not sure what's the point of testing a
configuration that you should never use in production.

I'm going to try implementing prngd support. It seems easy enough, and
prngd can be run on modern systems too, which is important for testing it.

In addition to that, I'm going to see if we can make postmaster to work
sensibly without query cancel keys, if pg_strong_random() fails. That
way, you could still run on platforms that don't have /dev/[u]random or
prngd, just without support for query cancels, MD5 authentication, and
pgcrypto key generation functions. I'd consider that similar to
--disable-spinlocks; it would be a helpful step when porting to a new
platform, but you wouldn't want to use it in production.

One more thing: in hindsight I think would be better to not fall back to
/dev/ random, if compiled with OpenSSL support. It really shouldn't
fail, and if it does, it seems better to complain loudly.

- Heikki

In response to

Responses

Browse pgsql-committers by date

  From Date Subject
Next Message Tom Lane 2016-10-17 18:14:23 Re: Re: [COMMITTERS] pgsql: Replace PostmasterRandom() with a stronger way of generating ran
Previous Message Magnus Hagander 2016-10-17 15:33:50 Re: [COMMITTERS] pgsql: Replace PostmasterRandom() with a stronger way of generating ran

Browse pgsql-hackers by date

  From Date Subject
Next Message Kenaniah Cerny 2016-10-17 18:05:49 Idempotency for all DDL statements
Previous Message Corey Huinker 2016-10-17 17:48:47 Re: COPY as a set returning function