Horton

Thread

Commits

  1. Fix the new SASLprep tests to work with non-UTF-8 locales.

  2. Use SASLprep to normalize passwords for SCRAM authentication.

  1. pgsql: Use SASLprep to normalize passwords for SCRAM authentication.

    Heikki Linnakangas <heikki.linnakangas@iki.fi> — 2017-04-07T11:58:10Z

    Use SASLprep to normalize passwords for SCRAM authentication.

An important step of SASLprep normalization, is to convert the string to
Unicode normalization form NFKC. Unicode normalization requires a fairly
large table of character decompositions, which is generated from data
published by the Unicode consortium. The script to generate the table is
put in src/common/unicode, as well test code for the normalization.
A pre-generated version of the tables is included in src/include/common,
so you don't need the code in src/common/unicode to build PostgreSQL, only
if you wish to modify the normalization tables.

The SASLprep implementation depends on the UTF-8 functions from
src/backend/utils/mb/wchar.c. So to use it, you must also compile and link
that. That doesn't change anything for the current users of these
functions, the backend and libpq, as they both already link with wchar.o.
It would be good to move those functions into a separate file in
src/commmon, but I'll leave that for another day.

No documentation changes included, because there is no details on the
SCRAM mechanism in the docs anyway. An overview on that in the protocol
specification would probably be good, even though SCRAM is documented in
detail in RFC5802. I'll write that as a separate patch. An important thing
to mention there is that we apply SASLprep even on invalid UTF-8 strings,
to support other encodings.

Patch by Michael Paquier and me.

Discussion: https://www.postgresql.org/message-id/CAB7nPqSByyEmAVLtEf1KxTRh=PWNKiWKEKQR=e1yGehz=wbymQ@mail.gmail.com

Branch
------
master

Details
-------
http://git.postgresql.org/pg/commitdiff/60f11b87a2349985230c08616fa8a34ffde934c8

Modified Files
--------------
src/backend/libpq/auth-scram.c                    |   63 +-
src/common/Makefile                               |    3 +-
src/common/saslprep.c                             | 1279 +++
src/common/scram-common.c                         |   25 +-
src/common/unicode/.gitignore                     |    7 +
src/common/unicode/Makefile                       |   53 +
src/common/unicode/README                         |   35 +
src/common/unicode/generate-norm_test_table.pl    |  102 +
src/common/unicode/generate-unicode_norm_table.pl |  226 +
src/common/unicode/norm_test.c                    |   80 +
src/common/unicode_norm.c                         |  437 +
src/include/common/saslprep.h                     |   30 +
src/include/common/unicode_norm.h                 |   21 +
src/include/common/unicode_norm_table.h           | 8859 +++++++++++++++++++++
src/interfaces/libpq/.gitignore                   |    2 +
src/interfaces/libpq/Makefile                     |    4 +-
src/interfaces/libpq/fe-auth-scram.c              |   27 +-
src/test/authentication/t/002_saslprep.pl         |   98 +
src/tools/msvc/Mkvcbuild.pm                       |    3 +-
19 files changed, 11322 insertions(+), 32 deletions(-)

  2. Re: pgsql: Use SASLprep to normalize passwords for SCRAM authentication.

    Tom Lane <tgl@sss.pgh.pa.us> — 2017-04-08T01:23:51Z

    Heikki Linnakangas <heikki.linnakangas@iki.fi> writes:
> Use SASLprep to normalize passwords for SCRAM authentication.

The test script that this adds appears to fail unless the environment
selects a UTF8-based locale.  On my RHEL6 machine, I see:

LANG=C make check			fail
LANG=en_US.iso88591 make check		fail
LANG=en_US.utf8 make check		ok

I'm surprised that more of the buildfarm hasn't fallen over.
Please do something about that.

			regards, tom lane

  3. Running make check-world in buildfarm (was Re: [COMMITTERS] pgsql: Use SASLprep to normalize passwords for SCRAM authentication.)

    Heikki Linnakangas <hlinnaka@iki.fi> — 2017-04-08T10:33:41Z

    On 04/08/2017 04:23 AM, Tom Lane wrote:
> Heikki Linnakangas <heikki.linnakangas@iki.fi> writes:
>> Use SASLprep to normalize passwords for SCRAM authentication.
>
> The test script that this adds appears to fail unless the environment
> selects a UTF8-based locale.  On my RHEL6 machine, I see:
>
> LANG=C make check			fail
> LANG=en_US.iso88591 make check		fail
> LANG=en_US.utf8 make check		ok

Fixed, thanks!

> I'm surprised that more of the buildfarm hasn't fallen over.

Hmm. It looks like none of the buildfarm members are running the 
authentication tests. Nor recovery tests, nor subscription tests. We're 
missing a trick here, at least some of the buildfarm members really 
ought to run "make check-world", we're missing a lot of coverage otherwise.

- Heikki

  4. Re: Running make check-world in buildfarm (was Re: [COMMITTERS] pgsql: Use SASLprep to normalize passwords for SCRAM authentication.)

    Michael Paquier <michael.paquier@gmail.com> — 2017-04-08T14:01:06Z

    On Sat, Apr 8, 2017 at 7:33 PM, Heikki Linnakangas <hlinnaka@iki.fi> wrote:
> Hmm. It looks like none of the buildfarm members are running the
> authentication tests. Nor recovery tests, nor subscription tests. We're
> missing a trick here, at least some of the buildfarm members really ought to
> run "make check-world", we're missing a lot of coverage otherwise.

I recall that Andrew has been favoring as much as possible one folder
path per test series in the buildfarm client (perhaps to keep the
tests separated and have the logs easier to analyze?). I would not
mind much if this is replaced by a pure make check-world, which is
what most of the serious hackers do, or at least a make check running
from src/test to save us a lot of maintenance pain.
-- 
Michael

  5. Re: Running make check-world in buildfarm (was Re: [COMMITTERS] pgsql: Use SASLprep to normalize passwords for SCRAM authentication.)

    Andres Freund <andres@anarazel.de> — 2017-04-08T14:05:54Z

    On 2017-04-08 23:01:06 +0900, Michael Paquier wrote:
> On Sat, Apr 8, 2017 at 7:33 PM, Heikki Linnakangas <hlinnaka@iki.fi> wrote:
> > Hmm. It looks like none of the buildfarm members are running the
> > authentication tests. Nor recovery tests, nor subscription tests. We're
> > missing a trick here, at least some of the buildfarm members really ought to
> > run "make check-world", we're missing a lot of coverage otherwise.
> 
> I recall that Andrew has been favoring as much as possible one folder
> path per test series in the buildfarm client (perhaps to keep the
> tests separated and have the logs easier to analyze?). I would not
> mind much if this is replaced by a pure make check-world, which is
> what most of the serious hackers do, or at least a make check running
> from src/test to save us a lot of maintenance pain.

I think it's partially knowing which target failed, and which
regression.diffs to display.  If we were able to revamp check-world so
it outputs a list of targets the regression machinery were able to run
individually, it'd probably help?

- Andres

  6. Re: Fwd: Re: Running make check-world in buildfarm (was Re: [COMMITTERS] pgsql: Use SASLprep to normalize passwords for SCRAM authentication.)

    Andrew Dunstan <andrew.dunstan@2ndquadrant.com> — 2017-04-08T14:26:00Z

    
On 04/08/2017 10:11 AM, Andrew Dunstan wrote:
>
>
> -------- Forwarded Message --------
> Subject: 	Re: [HACKERS] Running make check-world in buildfarm (was Re:
> [COMMITTERS] pgsql: Use SASLprep to normalize passwords for SCRAM
> authentication.)
> Date: 	Sat, 8 Apr 2017 07:05:54 -0700
> From: 	Andres Freund <andres@anarazel.de>
> To: 	Michael Paquier <michael.paquier@gmail.com>, Andrew Dunstan
> <andrew@dunslane.net>
> CC: 	Heikki Linnakangas <hlinnaka@iki.fi>, Tom Lane
> <tgl@sss.pgh.pa.us>, pgsql-hackers <pgsql-hackers@postgresql.org>
>
>
>
> On 2017-04-08 23:01:06 +0900, Michael Paquier wrote:
> > On Sat, Apr 8, 2017 at 7:33 PM, Heikki Linnakangas <hlinnaka@iki.fi> wrote:
> > > Hmm. It looks like none of the buildfarm members are running the
> > > authentication tests. Nor recovery tests, nor subscription tests. We're
> > > missing a trick here, at least some of the buildfarm members really ought to
> > > run "make check-world", we're missing a lot of coverage otherwise.
> > 
> > I recall that Andrew has been favoring as much as possible one folder
> > path per test series in the buildfarm client (perhaps to keep the
> > tests separated and have the logs easier to analyze?). I would not
> > mind much if this is replaced by a pure make check-world, which is
> > what most of the serious hackers do, or at least a make check running
> > from src/test to save us a lot of maintenance pain.
>
> I think it's partially knowing which target failed, and which
> regression.diffs to display.  If we were able to revamp check-world so
> it outputs a list of targets the regression machinery were able to run
> individually, it'd probably help?
>



Yes, I don't want just to run check-world.

I am aware of a few test sets that need to be added, and I'm planning on
doing that this weekend, in fact. Specifically: recovery, subscription,
authentication and SSL. Peter Eisentraut raised this with me about a
week ago.

Instead of just adding targets to check-world, perhaps we need to look
at what we can do so that the buildfarm client can discover what checks
it might run and run them, just as we specify test schedules for pg_regress.

cheers

andrew

-- 

Andrew Dunstan                https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

  7. Re: Fwd: Re: Running make check-world in buildfarm (was Re: [COMMITTERS] pgsql: Use SASLprep to normalize passwords for SCRAM authentication.)

    Tom Lane <tgl@sss.pgh.pa.us> — 2017-04-08T16:11:00Z

    Andrew Dunstan <andrew.dunstan@2ndquadrant.com> writes:
>> I think it's partially knowing which target failed, and which
>> regression.diffs to display.  If we were able to revamp check-world so
>> it outputs a list of targets the regression machinery were able to run
>> individually, it'd probably help?

> Yes, I don't want just to run check-world.

Yup.  The situation with the TAP tests (bin-check step) is already a
usability fail: when there's a failure, your first problem is to root
through megabytes of poorly-differentiated logs just to figure out
what actually failed.  Treating all of check-world as a single buildfarm
step would be a disaster.

> Instead of just adding targets to check-world, perhaps we need to look
> at what we can do so that the buildfarm client can discover what checks
> it might run and run them, just as we specify test schedules for pg_regress.

+1.  In the meantime, is there any chance of breaking down bin-check into
a separate step per src/bin/ subdirectory?

			regards, tom lane

  8. Re: Fwd: Re: Running make check-world in buildfarm (was Re: [COMMITTERS] pgsql: Use SASLprep to normalize passwords for SCRAM authentication.)

    Heikki Linnakangas <hlinnaka@iki.fi> — 2017-04-08T16:12:44Z

    On 04/08/2017 05:26 PM, Andrew Dunstan wrote:
> Yes, I don't want just to run check-world.
>
> I am aware of a few test sets that need to be added, and I'm planning on
> doing that this weekend, in fact. Specifically: recovery, subscription,
> authentication and SSL. Peter Eisentraut raised this with me about a
> week ago.

Thanks, that'd be great!

Beware that src/test/ssl is not safe to run on a multi-user system, 
because it allows connections from localhost with well-known user 
certificates. I've been running it on chipmunk for a while, with the 
attached module script.

- Heikki

  9. Re: Fwd: Re: Running make check-world in buildfarm (was Re: [COMMITTERS] pgsql: Use SASLprep to normalize passwords for SCRAM authentication.)

    Andrew Dunstan <andrew.dunstan@2ndquadrant.com> — 2017-04-08T18:49:10Z

    
On 04/08/2017 12:11 PM, Tom Lane wrote:
> Andrew Dunstan <andrew.dunstan@2ndquadrant.com> writes:
>>> I think it's partially knowing which target failed, and which
>>> regression.diffs to display.  If we were able to revamp check-world so
>>> it outputs a list of targets the regression machinery were able to run
>>> individually, it'd probably help?
>> Yes, I don't want just to run check-world.
> Yup.  The situation with the TAP tests (bin-check step) is already a
> usability fail: when there's a failure, your first problem is to root
> through megabytes of poorly-differentiated logs just to figure out
> what actually failed.  Treating all of check-world as a single buildfarm
> step would be a disaster.
>
>> Instead of just adding targets to check-world, perhaps we need to look
>> at what we can do so that the buildfarm client can discover what checks
>> it might run and run them, just as we specify test schedules for pg_regress.
> +1.  In the meantime, is there any chance of breaking down bin-check into
> a separate step per src/bin/ subdirectory?
>
> 			

Possibly. I will look when I go to do the missing checks, later today or
tomorrow.

cheers

andrew




-- 
Andrew Dunstan                https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

  10. Re: Fwd: Re: Running make check-world in buildfarm (was Re: [COMMITTERS] pgsql: Use SASLprep to normalize passwords for SCRAM authentication.)

    Andrew Dunstan <andrew.dunstan@2ndquadrant.com> — 2017-04-09T02:37:56Z

    
On 04/08/2017 02:49 PM, Andrew Dunstan wrote:
>
> On 04/08/2017 12:11 PM, Tom Lane wrote:
>> Andrew Dunstan <andrew.dunstan@2ndquadrant.com> writes:
>>>> I think it's partially knowing which target failed, and which
>>>> regression.diffs to display.  If we were able to revamp check-world so
>>>> it outputs a list of targets the regression machinery were able to run
>>>> individually, it'd probably help?
>>> Yes, I don't want just to run check-world.
>> Yup.  The situation with the TAP tests (bin-check step) is already a
>> usability fail: when there's a failure, your first problem is to root
>> through megabytes of poorly-differentiated logs just to figure out
>> what actually failed.  Treating all of check-world as a single buildfarm
>> step would be a disaster.
>>
>>> Instead of just adding targets to check-world, perhaps we need to look
>>> at what we can do so that the buildfarm client can discover what checks
>>> it might run and run them, just as we specify test schedules for pg_regress.
>> +1.  In the meantime, is there any chance of breaking down bin-check into
>> a separate step per src/bin/ subdirectory?
>>
>> 			
> Possibly. I will look when I go to do the missing checks, later today or
> tomorrow.
>


OK, crake is running this code now.  See
<https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=crake&dt=2017-04-09%2001%3A58%3A15?

I've left off the SSL tests for now. We should look into how we can do
that more safely. Meanwhile Heikki is running the tests.

Note that some of these tests are quite expensive in terms of time,
particularly recover, subscription and pg_rewind.

This who want to play along can get the bleeding edge code from git,
either by cloning or grabbing a zip of the latest code. I have one or
two things I want to do before wrapping up another client release.

cheers

andrew

-- 
Andrew Dunstan                https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

  11. Re: pgsql: Use SASLprep to normalize passwords for SCRAM authentication.

    Noah Misch <noah@leadboat.com> — 2017-04-11T04:22:18Z

    On Fri, Apr 07, 2017 at 11:58:10AM +0000, Heikki Linnakangas wrote:
> No documentation changes included, because there is no details on the
> SCRAM mechanism in the docs anyway. An overview on that in the protocol
> specification would probably be good, even though SCRAM is documented in
> detail in RFC5802. I'll write that as a separate patch. An important thing
> to mention there is that we apply SASLprep even on invalid UTF-8 strings,
> to support other encodings.

[Action required within three days.  This is a generic notification.]

The above-described topic is currently a PostgreSQL 10 open item.  Heikki,
since you committed the patch believed to have created it, you own this open
item.  If some other commit is more relevant or if this does not belong as a
v10 open item, please let us know.  Otherwise, please observe the policy on
open item ownership[1] and send a status update within three calendar days of
this message.  Include a date for your subsequent status update.  Testers may
discover new open items at any time, and I want to plan to get them all fixed
well in advance of shipping v10.  Consequently, I will appreciate your efforts
toward speedy resolution.  Thanks.

[1] https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.com