Re: Password identifiers, protocol aging and SCRAM protocol

From: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
To: Heikki Linnakangas <hlinnaka(at)iki(dot)fi>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, David Steele <david(at)pgmasters(dot)net>, Robert Haas <robertmhaas(at)gmail(dot)com>, David Fetter <david(at)fetter(dot)org>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Magnus Hagander <magnus(at)hagander(dot)net>, Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>, Julian Markwort <julian(dot)markwort(at)uni-muenster(dot)de>, Stephen Frost <sfrost(at)snowman(dot)net>, PostgreSQL mailing lists <pgsql-hackers(at)postgresql(dot)org>, Valery Popov <v(dot)popov(at)postgrespro(dot)ru>
Subject: Re: Password identifiers, protocol aging and SCRAM protocol
Date: 2016-09-03 12:36:59
Message-ID: CAB7nPqRevzWkivfXZxpsBeKXhyUgHR+41mWKs78Fu=7srYFg4w@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, Sep 2, 2016 at 10:23 PM, Michael Paquier
<michael(dot)paquier(at)gmail(dot)com> wrote:
> On Fri, Sep 2, 2016 at 7:57 PM, Heikki Linnakangas <hlinnaka(at)iki(dot)fi> wrote:
>> I decided to split ip.c anyway. I'd like to keep the files in
>> src/common/ip.c as small as possible, so I think it makes sense to be quite
>> surgical when moving things there. I kept the pg_foreach_ifaddr() function
>> in src/backend/libpq/ifaddr.c (I renamed the file to avoid confusion with
>> the ip.c that got moved), even though it means that test_ifaddr will have to
>> continue to copy the file directly from src/backend/libpq. I'm OK with that,
>> because test_ifaddrs is just a little test program that mimics the backend's
>> behaviour of enumerating interfaces. I don't consider it to be a "real"
>> frontend application.
>>
>> Pushed, after splitting. Thanks! Now let's move on to the more substantial
>> patches.

Thanks for the push.

> Before I send a new series of patches... There is one thing that I am
> still troubled with: the compilation of pgcrypto. First from
> contrib/pgcrypto/Makefile I am noticing the following issue with this
> block:
> CF_SRCS = $(if $(subst no,,$(with_openssl)), $(OSSL_SRCS), $(INT_SRCS))
> CF_TESTS = $(if $(subst no,,$(with_openssl)), $(OSSL_TESTS), $(INT_TESTS))
> CF_PGP_TESTS = $(if $(subst no,,$(with_zlib)), $(ZLIB_TST), $(ZLIB_OFF_TST))
> How is that correct if src/Makefile.global is not loaded first?
> Variables like with_openssl are still not loaded at that point.
>
> Then, as per patch 0001 there are two files holding the SHA routines:
> sha.c with the interface taken from OpenBSD, and sha_openssl.c that
> uses the interface of OpenSSL. And when compiling pgcrypto, the choice
> of file is made depending on the value of $(with_openssl).

So I have solved my identity crisis here by just using INT_SRCS and
OSSL_SRCS to list the correct files holding the SHA files. Thanks Tom
for the hint. I need to study more my Makefile-fu.

Attached is a new series:
- 0001, refactoring of SHA functions into src/common.
- 0002, move encoding routines to src/common/
- 0003, make password_encryption an enum
- 0004, refactor some code in CREATE/ALTER role code paths related the
use of password_encryption
- 0005, refactor some code to have a single routine to fetch password
and valid_until from pg_authid
- 0006, The core implementation of SCRAM-SHA-256, with the SASL
communication protocol. if you want to use SCRAM with that, things go
with password_encryption = 'scram'. I have spotted here a bug with the
MSVC build on the way.
- 0007, addition of PASSWORD val USING protocol
- 0008. regression tests for passwords. Those do not trigger the
internal sha routines, which lead to inconsistent results.
--
Michael

Attachment Content-Type Size
0001-Refactor-SHA-functions-and-move-them-to-src-common.patch application/x-patch 73.1 KB
0002-Move-encoding-routines-to-src-common.patch application/x-patch 23.8 KB
0003-Switch-password_encryption-to-a-enum.patch application/x-patch 9.1 KB
0004-Refactor-decision-making-of-password-encryption-into.patch application/x-patch 4.5 KB
0005-Create-generic-routine-to-fetch-password-and-valid-u.patch application/x-patch 4.4 KB
0006-Support-for-SCRAM-SHA-256-authentication-RFC-5802-an.patch application/x-patch 75.0 KB
0007-Add-clause-PASSWORD-val-USING-protocol-to-CREATE-ALT.patch application/x-patch 8.7 KB
0008-Add-regression-tests-for-passwords.patch application/x-patch 9.1 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andreas Seltenreich 2016-09-03 13:19:49 [sqlsmith] Failed assertion in numeric aggregate
Previous Message Greg Stark 2016-09-03 12:25:32 Re: autonomous transactions