Re: Password identifiers, protocol aging and SCRAM protocol

From: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Stephen Frost <sfrost(at)snowman(dot)net>, David Steele <david(at)pgmasters(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-03-19 12:30:11
Message-ID: CAB7nPqRChAg7fUMZJ3FJpVjzaq9yYDPzxtBLOHVMnwjDAgyx=w@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Sat, Mar 19, 2016 at 3:52 AM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> On Fri, Mar 18, 2016 at 2:12 PM, Stephen Frost <sfrost(at)snowman(dot)net> wrote:
>> I'm not sure that I agree with the above. This patch has been through
>> the ringer multiple times regarding the user-facing bits and, by and
>> large, the results appear reasonable. Further, getting a better auth
>> method into PG is something which I do view as a priority considering
>> the concerns and complaints that have been, justifiably, raised against
>> our current password-based authentication support.
>>
>> This isn't a new patch set either, it was submitted initially over the
>> summer after it was pointed out, over a year ago, that people actually
>> do care about the problems with our current implementation (amusingly, I
>> recall having pointed out the same 5+ years ago, but only did so to this
>> list).
>
> I am not disputing the importance of the topic, and I do realize that
> the patch has been around in some form since March. However, I don't
> think there's been a whole heck of a lot in terms of detailed
> code-level review, and I think that's pretty important for something
> that necessarily involves wire protocol changes.

Yep, that's the desert here, though there are surely a lot of people
would like a way to get out of md5 and get into something more modern
(see STIG), and many companies want to get something, my company
included, though this is really a complicated task, and there are few
people who could really help out here I guess.

> Doing that with the
> level of detail and care that it seems to me to require seems like an
> almost-impossible task. Most of the major features I've committed
> this CommitFest are patches where I've personally done multiple rounds
> of review on over the last several months, and in many cases, other
> people have been doing code reviews for months before that. I'm not
> denying that this patch has prompted a good deal of discussion and
> what I would call design review, but detailed code review? I just
> haven't seen much of that.

There has been none, as well as no real discussion regarding what we
want to do. The current result, particularly for the management of
protocol aging, is based on things I wrote by myself which negate the
many negative opinions received up to now for the past patches (mainly
the feedback was "I don't like that", without real output or fresh
ideas during discussion to explain why that's the case).

>>> And I'd rather see all of the changes in one release than split them
>>> across two releases.
>>
>> I agree with this. If we aren't going to get SCRAM into 9.6 then the
>> rest is just breaking things with little benefit. I'm optimistic that
>> we will be able to include SCRAM support in 9.6, but if that ends up not
>> being feasible then we need to put all of the changes to the next
>> release.
>
> OK, glad we agree on that.

Speaking as a co-author of the stuff of this thread, the two main
patches are 0001, introducing pg_auth_verifiers and 0009, adding
SCRAM-SHA1. The rest is just refactoring and addition of a couple of
utilities to manage the protocol aging, which are really
straight-forward, and all the user-visible changes are introduced by
0001. While I really like the shape of 0001, 0009 is not there yet,
and really requires more time than 3 weeks, that's more than what I
can do by feature freeze of 9.6. So if the conclusion is if there is
no SCRAM, all the other changes don't make much sense, let's bump it
to 9.7. There is honestly still interest from here, and I would guess
that the only thing I could do on top of having patches for the first
CF of 9.7 is discussing the topic at the dev unconference of PGCon.

>> I do think that if we push this off to 9.7 then we're going to have
>> SCRAM *plus* a bunch of other changes around password policies in that
>> release, and it'd be better to introduce SCRAM independently of the
>> other changes.
>
> Well, for my part, I'd be happy enough to do all of that in a release
> cycle - maybe SCRAM at the beginning and those other changes a little
> later on. I don't see that as a real conflict, and in fact, sometimes
> when you do several things like that in a single cycle, people start
> to see whatever the common theme is - security, say - as part of the
> message of that release a little more than they would if a feature
> lands here and another there. That's not all a bad thing.

Having a centralized theme for a given release cycle is not a bad
thing, I agree. And I'd like to think that the same discussion is not
going to happen again in one year...
--
Michael

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2016-03-19 12:43:25 Re: GinPageIs* don't actually return a boolean
Previous Message Dilip Kumar 2016-03-19 12:22:27 Re: Move PinBuffer and UnpinBuffer to atomics