password_encryption default

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
36 messages Options
12
Reply | Threaded
Open this post in threaded view
|

password_encryption default

Peter Eisentraut-6
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.

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


Reply | Threaded
Open this post in threaded view
|

Re: password_encryption default

Tom Lane-2
Peter Eisentraut <[hidden email]> 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


Reply | Threaded
Open this post in threaded view
|

Re: password_encryption default

Magnus Hagander-2


On Fri, May 22, 2020 at 4:13 PM Tom Lane <[hidden email]> wrote:
Peter Eisentraut <[hidden email]> 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. 

--
Reply | Threaded
Open this post in threaded view
|

Re: password_encryption default

Stephen Frost
Greetings,

* Magnus Hagander ([hidden email]) wrote:

> On Fri, May 22, 2020 at 4:13 PM Tom Lane <[hidden email]> wrote:
> > Peter Eisentraut <[hidden email]> 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

signature.asc (836 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: password_encryption default

Tom Lane-2
Stephen Frost <[hidden email]> writes:
> * Magnus Hagander ([hidden email]) wrote:
>> On Fri, May 22, 2020 at 4:13 PM Tom Lane <[hidden email]> wrote:
>>> Peter Eisentraut <[hidden email]> 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


Reply | Threaded
Open this post in threaded view
|

Re: password_encryption default

Stephen Frost
Greetings,

* Tom Lane ([hidden email]) wrote:

> Stephen Frost <[hidden email]> writes:
> > * Magnus Hagander ([hidden email]) wrote:
> >> On Fri, May 22, 2020 at 4:13 PM Tom Lane <[hidden email]> wrote:
> >>> Peter Eisentraut <[hidden email]> 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

signature.asc (836 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: password_encryption default

Tom Lane-2
Stephen Frost <[hidden email]> writes:
> * Tom Lane ([hidden email]) 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


Reply | Threaded
Open this post in threaded view
|

Re: password_encryption default

Stephen Frost
Greetings,

* Tom Lane ([hidden email]) wrote:

> Stephen Frost <[hidden email]> writes:
> > * Tom Lane ([hidden email]) 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

signature.asc (836 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: password_encryption default

Tom Lane-2
Stephen Frost <[hidden email]> writes:
> * Tom Lane ([hidden email]) 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


Reply | Threaded
Open this post in threaded view
|

Re: password_encryption default

Stephen Frost
Greetings,

* Tom Lane ([hidden email]) wrote:

> Stephen Frost <[hidden email]> writes:
> > * Tom Lane ([hidden email]) 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

signature.asc (836 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: password_encryption default

Jonathan S. Katz-3
In reply to this post by Tom Lane-2
On 5/22/20 11:34 AM, Tom Lane wrote:

> Stephen Frost <[hidden email]> writes:
>> * Tom Lane ([hidden email]) 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


signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: password_encryption default

Vik Fearing-6
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


Reply | Threaded
Open this post in threaded view
|

Re: password_encryption default

Tom Lane-2
Vik Fearing <[hidden email]> 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


Reply | Threaded
Open this post in threaded view
|

Re: password_encryption default

Jonathan S. Katz-3
On 5/22/20 5:21 PM, Tom Lane wrote:

> Vik Fearing <[hidden email]> 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


signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: password_encryption default

Peter Eisentraut-6
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

0001-Change-default-of-password_encryption-to-scram-sha-2.patch (5K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: password_encryption default

Jonathan S. Katz-3
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


signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: password_encryption default

Peter Eisentraut-6
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

v2-0001-Change-default-of-password_encryption-to-scram-sh.patch (5K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: password_encryption default

Michael Paquier-2
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

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: password_encryption default

Peter Eisentraut-6
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


Reply | Threaded
Open this post in threaded view
|

Re: password_encryption default

Magnus Hagander-2
On Wed, May 27, 2020 at 8:29 AM Peter Eisentraut <[hidden email]> 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? 

--
12