Re: Password identifiers, protocol aging and SCRAM protocol

On Tue, Oct 18, 2016 at 4:35 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:
> On Mon, Oct 17, 2016 at 6:18 PM, Michael Paquier
> <michael.paquier@gmail.com> wrote:
>> On Mon, Oct 17, 2016 at 5:55 PM, Heikki Linnakangas <hlinnaka@iki.fi> wrote:
>>> Ok, committed to HEAD.
>
> Attached is a rebased patch set for SCRAM, with the following things:
> [...]

And as the PostmasterRandom() patch has been reverted, here is once
again a new set:
- 0001, moving all the SHA2 functions to src/common/ and introducing a
PG-like interface. No actual changes here.
- 0002, replacing PostmasterRandom by pg_strong_random(), with a fix
for the cancel key problem.
- 0003, adding for pg_strong_random() a fallback for any nix platform
not having /dev/random. This should be grouped with 0002, but I split
it for clarity.
- 0004, Add encoding routines for base64 without whitespace in
src/common/. I improved the error handling here by making them return
-1 in case of error and let the caller handle the error.
- 0005, Refactor decision-making of password encryption into a single routine.
- 0006, Add clause PASSWORD val USING protocol to CREATE/ALTER ROLE.
- 0007, the SCRAM implementation. I have reworked the error handling
on both the frontend and the backend. In the frontend, there were many
code paths that did not bother much about many sanity checks like
OOMs, so I addressed that as a whole thing. For the backend, in the
event of an error, the backend sends back to the client a e= message
with an error string corresponding to what happened per RFC5802.
Sanity checks of the user data on the server (get the SCRAM verifier,
its validuntil, empty password and the user name itself), are made
part of the message exchange as in case of errors we need to return
errors like e=unknown-user, e=other-errors and stuff similar to that.
This makes the code in auth.c slightly cleaner btw.
- 0008 is a set of regression tests.

The PostmasterRandom() patch sent in this set contains the fix for
cancel keys that were previously broken. I have also implemented a
fallback method in 0003 inspired by pgcrypto's try_unix_std. It simply
uses gettimeofday() (should be put in the upper loop actually now that
I think about it!), getpid() and random() to generate some randomness,
and then processes the whole through a SHA-256 hash, generating chunks
of random data worth of SHA256_DIGEST_LENGTH bytes. I have not added a
./configure switch for it, but there were voices in favor of that. And
this is not available on Windows (no need to care anyway as there are
crypto APIs). A requirement of this patch is to have the SHA-256
routines in src/common/ first, and this will allow any platform
without /dev/random to generate random numbers like pademelon.

The fallback method for the pg_strong_random() is clearly not ready
for commit, one reason is that libpgport should stand at a level lower
than libpgcommon as far as I understand. But this patch makes
pg_strong_random() in src/port depend on the SHA2 routines in
src/common so it would make more sense if pg_strong_random() is moved
as well to src/common instead of src/port. Honestly I think that we'd
get away better with something like that than trying for example to
reimplement a dependency with PRNG knowing that OpenSSL does it
already, and perhaps better than we could do it.

Thoughts welcome. A lot of bits are independent of that part in the
patch set anyway.
-- 
Michael