password_encryption default

Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:

We didn't get anywhere with making the default authentication method in
a source build anything other than trust. But perhaps we should change
the default for password_encryption to nudge people to adopt SCRAM?
Right now, passwords are still hashed using MD5 by default, unless you
specify scram-sha-256 using initdb -A or similar.

I think what that was waiting on was for client libraries to become
SCRAM-ready. Do we have an idea of the state of play on that side?

regards, tom lane

On Fri, May 22, 2020 at 4:13 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:

Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:

We didn't get anywhere with making the default authentication method in
a source build anything other than trust. But perhaps we should change
the default for password_encryption to nudge people to adopt SCRAM?
Right now, passwords are still hashed using MD5 by default, unless you
specify scram-sha-256 using initdb -A or similar.

I think what that was waiting on was for client libraries to become
SCRAM-ready. Do we have an idea of the state of play on that side?

If the summary table on the wiki at
https://wiki.postgresql.org/wiki/List_of_drivers is to be trusted, every
listed driver except Swift does.

--
Magnus Hagander
Me: https://www.hagander.net/ <http://www.hagander.net/&gt;
Work: https://www.redpill-linpro.com/ <http://www.redpill-linpro.com/&gt;

Greetings,

* Magnus Hagander (magnus@hagander.net) wrote:

On Fri, May 22, 2020 at 4:13 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:

Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:

We didn't get anywhere with making the default authentication method in
a source build anything other than trust. But perhaps we should change
the default for password_encryption to nudge people to adopt SCRAM?
Right now, passwords are still hashed using MD5 by default, unless you
specify scram-sha-256 using initdb -A or similar.

I think what that was waiting on was for client libraries to become
SCRAM-ready. Do we have an idea of the state of play on that side?

If the summary table on the wiki at
https://wiki.postgresql.org/wiki/List_of_drivers is to be trusted, every
listed driver except Swift does.

Yes, Katz actually went through and worked with folks to make that
happen. I'm +1 on moving the default for password_encryption to be
scram. Even better would be changing the pg_hba.conf default, but I
think we still have concerns about that having problems with the
regression tests and the buildfarm.

Thanks,

Stephen

Stephen Frost <sfrost@snowman.net> writes:

* Magnus Hagander (magnus@hagander.net) wrote:

On Fri, May 22, 2020 at 4:13 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:

Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:

We didn't get anywhere with making the default authentication method in
a source build anything other than trust.

I'm +1 on moving the default for password_encryption to be
scram. Even better would be changing the pg_hba.conf default, but I
think we still have concerns about that having problems with the
regression tests and the buildfarm.

As far as that last goes, we *did* get the buildfarm fixed to be all
v11 scripts, so I thought we were ready to move forward on trying
09f08930f again. It's too late to consider that for v13, but
perhaps it'd be reasonable to change the SCRAM default now? Not sure.
Post-beta1 isn't the best time for such things.

regards, tom lane

Greetings,

* Tom Lane (tgl@sss.pgh.pa.us) wrote:

Stephen Frost <sfrost@snowman.net> writes:

* Magnus Hagander (magnus@hagander.net) wrote:

On Fri, May 22, 2020 at 4:13 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:

Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:

We didn't get anywhere with making the default authentication method in
a source build anything other than trust.

I'm +1 on moving the default for password_encryption to be
scram. Even better would be changing the pg_hba.conf default, but I
think we still have concerns about that having problems with the
regression tests and the buildfarm.

As far as that last goes, we *did* get the buildfarm fixed to be all
v11 scripts, so I thought we were ready to move forward on trying
09f08930f again. It's too late to consider that for v13, but
perhaps it'd be reasonable to change the SCRAM default now? Not sure.

I feel like it is. I'm not even sure that I agree that it's really too
late to consider 09f08930f considering that's it's a pretty minor code
change and the up-side to that is having reasonable defaults out of the
box, as it were, something we have *long* been derided for.

Post-beta1 isn't the best time for such things.

It'd be good to be consistent about this between the packagers and the
source builds, imv, and we don't tend to think about that until we have
packages being built and distributed and used and that ends up being
post-beta1. If we want that changed then we should go back to having
alphas..

In general though, I'm reasonably comfortable with changing of default
values post beta1. I do appreciate that not everyone would agree with
that, but with all the effort that's put into getting everything working
with SCRAM, it'd be a real shame to keep md5 as the default for yet
another year and a half..

Thanks,

Stephen

Stephen Frost <sfrost@snowman.net> writes:

* Tom Lane (tgl@sss.pgh.pa.us) wrote:

As far as that last goes, we *did* get the buildfarm fixed to be all
v11 scripts, so I thought we were ready to move forward on trying
09f08930f again. It's too late to consider that for v13, but
perhaps it'd be reasonable to change the SCRAM default now? Not sure.

I feel like it is. I'm not even sure that I agree that it's really too
late to consider 09f08930f considering that's it's a pretty minor code
change and the up-side to that is having reasonable defaults out of the
box, as it were, something we have *long* been derided for.

Well, the argument against changing right now is that it would invalidate
portability testing done against beta1, which users would be justifiably
upset about.

I'm +1 for changing both of these things as soon as we branch for v14,
but I feel like it's a bit late for v13. If we aren't feature-frozen
now, when will we be?

regards, tom lane

Greetings,

* Tom Lane (tgl@sss.pgh.pa.us) wrote:

Stephen Frost <sfrost@snowman.net> writes:

* Tom Lane (tgl@sss.pgh.pa.us) wrote:

As far as that last goes, we *did* get the buildfarm fixed to be all
v11 scripts, so I thought we were ready to move forward on trying
09f08930f again. It's too late to consider that for v13, but
perhaps it'd be reasonable to change the SCRAM default now? Not sure.

I feel like it is. I'm not even sure that I agree that it's really too
late to consider 09f08930f considering that's it's a pretty minor code
change and the up-side to that is having reasonable defaults out of the
box, as it were, something we have *long* been derided for.

Well, the argument against changing right now is that it would invalidate
portability testing done against beta1, which users would be justifiably
upset about.

I don't think we're in complete agreement about the amount of
portability testing that's done with our beta source builds. To that
point, however, the lack of such testing happening, if there is a lack,
is on us just as much as anyone else- we should be testing, to the
extent possible, as many variations of our configuration options as we
can across as many platforms as we can in the buildfarm. If a
non-default setting doesn't work on one platform or another, that's a
bug to fix regardless and doesn't really impact the question of "what
should be the default".

I'm +1 for changing both of these things as soon as we branch for v14,
but I feel like it's a bit late for v13. If we aren't feature-frozen
now, when will we be?

I really don't consider changing of defaults to be on the same level as
implementation of whole features, even if changing those defaults
requires a few lines of code to go with the change.

Thanks,

Stephen

Stephen Frost <sfrost@snowman.net> writes:

* Tom Lane (tgl@sss.pgh.pa.us) wrote:

I'm +1 for changing both of these things as soon as we branch for v14,
but I feel like it's a bit late for v13. If we aren't feature-frozen
now, when will we be?

I really don't consider changing of defaults to be on the same level as
implementation of whole features, even if changing those defaults
requires a few lines of code to go with the change.

The buildfarm fiasco with 09f08930f should remind us that changing
defaults *does* break things, even if theoretically it shouldn't.
At this phase of the v13 cycle, we should be looking to fix bugs,
not to break more stuff.

regards, tom lane

Greetings,

* Tom Lane (tgl@sss.pgh.pa.us) wrote:

Stephen Frost <sfrost@snowman.net> writes:

* Tom Lane (tgl@sss.pgh.pa.us) wrote:

I'm +1 for changing both of these things as soon as we branch for v14,
but I feel like it's a bit late for v13. If we aren't feature-frozen
now, when will we be?

I really don't consider changing of defaults to be on the same level as
implementation of whole features, even if changing those defaults
requires a few lines of code to go with the change.

The buildfarm fiasco with 09f08930f should remind us that changing
defaults *does* break things, even if theoretically it shouldn't.
At this phase of the v13 cycle, we should be looking to fix bugs,
not to break more stuff.

Sure it does- for the special case of the buildfarm, and that takes
buildfarm code to fix. Having users make changes to whatever scripts
they're using with PG between major versions is certainly not
unreasonable, or even between beta and final. These things are not set
in stone at this point, they're the defaults, and it's beta time now,
not post release or RC.

If it breaks for regular users who are using the system properly then we
want to know about that and we'd ideally like to get that fixed before
the release.

Thanks,

Stephen

On 5/22/20 11:34 AM, Tom Lane wrote:

Stephen Frost <sfrost@snowman.net> writes:

* Tom Lane (tgl@sss.pgh.pa.us) wrote:

As far as that last goes, we *did* get the buildfarm fixed to be all
v11 scripts, so I thought we were ready to move forward on trying
09f08930f again. It's too late to consider that for v13, but
perhaps it'd be reasonable to change the SCRAM default now? Not sure.

I feel like it is. I'm not even sure that I agree that it's really too
late to consider 09f08930f considering that's it's a pretty minor code
change and the up-side to that is having reasonable defaults out of the
box, as it were, something we have *long* been derided for.

Well, the argument against changing right now is that it would invalidate
portability testing done against beta1, which users would be justifiably
upset about.

I'm +1 for changing both of these things as soon as we branch for v14,
but I feel like it's a bit late for v13. If we aren't feature-frozen
now, when will we be?

As someone who is an unabashed SCRAM fan and was hoping the default
would be up'd for v13, I would actually +1 making it the default in v14,
i.e. because 9.5 will be EOL at that point, and as such we both have
every* driver supporting SCRAM AND every version of PostgreSQL
supporting SCRAM.

(Would I personally love to do it sooner? Yes...but I think the stars
align for doing it in v14).

Jonathan

On 5/22/20 9:09 PM, Jonathan S. Katz wrote:

As someone who is an unabashed SCRAM fan and was hoping the default
would be up'd for v13, I would actually +1 making it the default in v14,
i.e. because 9.5 will be EOL at that point, and as such we both have
every* driver supporting SCRAM AND every version of PostgreSQL
supporting SCRAM.

Wasn't SCRAM introduced in 10?
--
Vik Fearing

Vik Fearing <vik@postgresfriends.org> writes:

On 5/22/20 9:09 PM, Jonathan S. Katz wrote:

As someone who is an unabashed SCRAM fan and was hoping the default
would be up'd for v13, I would actually +1 making it the default in v14,
i.e. because 9.5 will be EOL at that point, and as such we both have
every* driver supporting SCRAM AND every version of PostgreSQL
supporting SCRAM.

Wasn't SCRAM introduced in 10?

Yeah. But there's still something to Jonathan's argument, because 9.6
will go EOL in November 2021, which is pretty close to when v14 will
reach public release (assuming we can hold to the typical schedule).
If we do it in v13, there'll be a full year where still-supported
versions of PG can't do SCRAM, implying that clients would likely
fail to connect to an up-to-date server.

regards, tom lane

On 5/22/20 5:21 PM, Tom Lane wrote:

Vik Fearing <vik@postgresfriends.org> writes:

On 5/22/20 9:09 PM, Jonathan S. Katz wrote:

As someone who is an unabashed SCRAM fan and was hoping the default
would be up'd for v13, I would actually +1 making it the default in v14,
i.e. because 9.5 will be EOL at that point, and as such we both have
every* driver supporting SCRAM AND every version of PostgreSQL
supporting SCRAM.

Wasn't SCRAM introduced in 10?

Yeah. But there's still something to Jonathan's argument, because 9.6
will go EOL in November 2021, which is pretty close to when v14 will
reach public release (assuming we can hold to the typical schedule).
If we do it in v13, there'll be a full year where still-supported
versions of PG can't do SCRAM, implying that clients would likely
fail to connect to an up-to-date server.

^ that's what I meant.

Jonathan

On 2020-05-22 23:23, Jonathan S. Katz wrote:

Yeah. But there's still something to Jonathan's argument, because 9.6
will go EOL in November 2021, which is pretty close to when v14 will
reach public release (assuming we can hold to the typical schedule).
If we do it in v13, there'll be a full year where still-supported
versions of PG can't do SCRAM, implying that clients would likely
fail to connect to an up-to-date server.

^ that's what I meant.

Here is a proposed patch for PG14 then.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On 5/25/20 5:45 AM, Peter Eisentraut wrote:

On 2020-05-22 23:23, Jonathan S. Katz wrote:

Yeah.  But there's still something to Jonathan's argument, because 9.6
will go EOL in November 2021, which is pretty close to when v14 will
reach public release (assuming we can hold to the typical schedule).
If we do it in v13, there'll be a full year where still-supported
versions of PG can't do SCRAM, implying that clients would likely
fail to connect to an up-to-date server.

^ that's what I meant.

Here is a proposed patch for PG14 then.

This makes me happy :D

I took a look over, it looks good. One question on the initdb.c diff:

- if (strcmp(authmethodlocal, "scram-sha-256") == 0 ||
- strcmp(authmethodhost, "scram-sha-256") == 0)
- {
- conflines = replace_token(conflines,
- "#password_encryption = md5",
- "password_encryption = scram-sha-256");
- }
-

Would we reverse this, i.e. if someone chooses authmethodlocal to be
"md5", we would then set "password_encryption = md5"?

Thanks,

Jonathan

On 2020-05-25 17:57, Jonathan S. Katz wrote:

I took a look over, it looks good. One question on the initdb.c diff:

- if (strcmp(authmethodlocal, "scram-sha-256") == 0 ||
- strcmp(authmethodhost, "scram-sha-256") == 0)
- {
- conflines = replace_token(conflines,
- "#password_encryption = md5",
- "password_encryption = scram-sha-256");
- }
-

Would we reverse this, i.e. if someone chooses authmethodlocal to be
"md5", we would then set "password_encryption = md5"?

Yeah, I was too enthusiastic about removing that. Here is a better patch.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On Tue, May 26, 2020 at 10:25:25AM +0200, Peter Eisentraut wrote:

Yeah, I was too enthusiastic about removing that. Here is a better patch.

+        as an MD5 hash.  (<literal>on</literal> is also accepted, as an alias
+        for <literal>md5</literal>.)  The default is
+        <literal>scram-sha-256</literal>.
Shouldn't password_encryption = on/true/1/yes be an equivalent of
scram-sha-256 as the default gets changed?
--
Michael

On 2020-05-27 08:00, Michael Paquier wrote:

On Tue, May 26, 2020 at 10:25:25AM +0200, Peter Eisentraut wrote:

Yeah, I was too enthusiastic about removing that. Here is a better patch.

+        as an MD5 hash.  (<literal>on</literal> is also accepted, as an alias
+        for <literal>md5</literal>.)  The default is
+        <literal>scram-sha-256</literal>.
Shouldn't password_encryption = on/true/1/yes be an equivalent of
scram-sha-256 as the default gets changed?

I think these are mostly legacy options anyway, so if we wanted to make
a change, we should remove them.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On Wed, May 27, 2020 at 8:29 AM Peter Eisentraut <
peter.eisentraut@2ndquadrant.com> wrote:

On 2020-05-27 08:00, Michael Paquier wrote:

On Tue, May 26, 2020 at 10:25:25AM +0200, Peter Eisentraut wrote:

Yeah, I was too enthusiastic about removing that. Here is a better

patch.

+ as an MD5 hash. (<literal>on</literal> is also accepted, as an

alias

+        for <literal>md5</literal>.)  The default is
+        <literal>scram-sha-256</literal>.
Shouldn't password_encryption = on/true/1/yes be an equivalent of
scram-sha-256 as the default gets changed?

I think these are mostly legacy options anyway, so if we wanted to make
a change, we should remove them.

Seems like the better choice yeah. Since we're changing the default anyway,
maybe now is the time to do that? Or if not, maybe have it log an explicit
deprecation warning when it loads a config with it?

--
Magnus Hagander
Me: https://www.hagander.net/ <http://www.hagander.net/&gt;
Work: https://www.redpill-linpro.com/ <http://www.redpill-linpro.com/&gt;