Thread: [HACKERS] password_encryption, default and 'plain' support
Hi, In various threads on SCRAM, we've skirted around the question of whether we should still allow storing passwords in plaintext. I've avoided discussing that in those other threads, because it's been an orthogonal question, but it's a good question and we should discuss it. So, I propose that we remove support for password_encryption='plain' in PostgreSQL 10. If you try to do that, you'll get an error. Another question that's been touched upon but not explicitly discussed, is whether we should change the default to "scram-sha-256". I propose that we do that as well. If you need to stick to md5, e.g. because you use drivers that don't support SCRAM yet, you can change it in postgresql.conf, but the majority of installations that use modern clients will be more secure by default. - Heikki
On Wed, May 3, 2017 at 1:31 PM, Heikki Linnakangas <hlinnaka@iki.fi> wrote:
Hi,
In various threads on SCRAM, we've skirted around the question of whether we should still allow storing passwords in plaintext. I've avoided discussing that in those other threads, because it's been an orthogonal question, but it's a good question and we should discuss it.
So, I propose that we remove support for password_encryption='plain' in PostgreSQL 10. If you try to do that, you'll get an error.
Is there any usecase at all for it today?
+1 for getting rid of it :)
Another question that's been touched upon but not explicitly discussed, is whether we should change the default to "scram-sha-256". I propose that we do that as well. If you need to stick to md5, e.g. because you use drivers that don't support SCRAM yet, you can change it in postgresql.conf, but the majority of installations that use modern clients will be more secure by default.
Much as that's going to cause issues for some people, I think it's worth doing. We should probably put something specific in the release notes mentioning the error message you get in libpq, and possibly some of the other most common drivers.
On Wed, May 3, 2017 at 8:38 PM, Magnus Hagander <magnus@hagander.net> wrote: > On Wed, May 3, 2017 at 1:31 PM, Heikki Linnakangas <hlinnaka@iki.fi> wrote: >> In various threads on SCRAM, we've skirted around the question of whether >> we should still allow storing passwords in plaintext. I've avoided >> discussing that in those other threads, because it's been an orthogonal >> question, but it's a good question and we should discuss it. >> >> So, I propose that we remove support for password_encryption='plain' in >> PostgreSQL 10. If you try to do that, you'll get an error. > > Is there any usecase at all for it today? For developers running applications on top of Postgres? >> Another question that's been touched upon but not explicitly discussed, is >> whether we should change the default to "scram-sha-256". I propose that we >> do that as well. If you need to stick to md5, e.g. because you use drivers >> that don't support SCRAM yet, you can change it in postgresql.conf, but the >> majority of installations that use modern clients will be more secure by >> default. > > Much as that's going to cause issues for some people, I think it's worth > doing. We should probably put something specific in the release notes > mentioning the error message you get in libpq, and possibly some of the > other most common drivers. My original view on the matter was, and is still, to wait for one or two releases before switching the default to scram. -- Michael
On Wed, May 3, 2017 at 2:25 PM, Michael Paquier <michael.paquier@gmail.com> wrote:
On Wed, May 3, 2017 at 8:38 PM, Magnus Hagander <magnus@hagander.net> wrote:
> On Wed, May 3, 2017 at 1:31 PM, Heikki Linnakangas <hlinnaka@iki.fi> wrote:
>> In various threads on SCRAM, we've skirted around the question of whether
>> we should still allow storing passwords in plaintext. I've avoided
>> discussing that in those other threads, because it's been an orthogonal
>> question, but it's a good question and we should discuss it.
>>
>> So, I propose that we remove support for password_encryption='plain' in
>> PostgreSQL 10. If you try to do that, you'll get an error.
>
> Is there any usecase at all for it today?
For developers running applications on top of Postgres?
I don't get it. How does password_encryption=plain help them?
On Wed, May 3, 2017 at 7:31 AM, Heikki Linnakangas <hlinnaka@iki.fi> wrote: > In various threads on SCRAM, we've skirted around the question of whether we > should still allow storing passwords in plaintext. I've avoided discussing > that in those other threads, because it's been an orthogonal question, but > it's a good question and we should discuss it. > > So, I propose that we remove support for password_encryption='plain' in > PostgreSQL 10. If you try to do that, you'll get an error. I have no idea how widely used that option is. > Another question that's been touched upon but not explicitly discussed, is > whether we should change the default to "scram-sha-256". I propose that we > do that as well. If you need to stick to md5, e.g. because you use drivers > that don't support SCRAM yet, you can change it in postgresql.conf, but the > majority of installations that use modern clients will be more secure by > default. I think that we should investigate how many connectors have support for SCRAM or are likely to do so by the time v10 is released. A *lot* of people are using connectors that are not based on libpq, especially JDBC but I think many of the others as well. If most of those are going to support SCRAM by the time v10 comes out, cool, but if not, maybe it's wise to hold off for a release before flipping the default. Not sure. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Robert Haas <robertmhaas@gmail.com> writes:
> On Wed, May 3, 2017 at 7:31 AM, Heikki Linnakangas <hlinnaka@iki.fi> wrote:
>> So, I propose that we remove support for password_encryption='plain' in
>> PostgreSQL 10. If you try to do that, you'll get an error.
> I have no idea how widely used that option is.
Is it possible that there are still client libraries that don't support
password encryption at all? If so, are we willing to break them?
I'd say "yes" but it's worth thinking about.
>> Another question that's been touched upon but not explicitly discussed, is
>> whether we should change the default to "scram-sha-256". I propose that we
>> do that as well. If you need to stick to md5, e.g. because you use drivers
>> that don't support SCRAM yet, you can change it in postgresql.conf, but the
>> majority of installations that use modern clients will be more secure by
>> default.
> I think that we should investigate how many connectors have support
> for SCRAM or are likely to do so by the time v10 is released. A *lot*
> of people are using connectors that are not based on libpq, especially
> JDBC but I think many of the others as well.
Yes, there's an awful lot out there besides libpq. I do not think it is
reasonable at all to change this default in v10. Maybe v11, depending on
how fast JDBC et al move. If we try to force it in v10, we are going to
get a heck of a lot of complaints about "I changed my password and now
I can't get in at all using <x>", where <x> is also going to include
back-rev psql. (And I'm not even considering the possibility of nasty
bugs in the SCRAM code.) Making SCRAM the default in the first version
where it's even available is moving way too fast IMO.
regards, tom lane
On 05/03/2017 07:14 PM, Tom Lane wrote: > Robert Haas <robertmhaas@gmail.com> writes: >> On Wed, May 3, 2017 at 7:31 AM, Heikki Linnakangas <hlinnaka@iki.fi> wrote: >>> So, I propose that we remove support for password_encryption='plain' in >>> PostgreSQL 10. If you try to do that, you'll get an error. > >> I have no idea how widely used that option is. > > Is it possible that there are still client libraries that don't support > password encryption at all? If so, are we willing to break them? > I'd say "yes" but it's worth thinking about. That doesn't make sense. The client doesn't even know what password_encryption is set to. I think you're confusing password_encryption='plain' with the plaintext "password" authentication method. If the server has an MD5 hash stored in pg_authid, the server will ask the client to do MD5 authentication. If the server has a SCRAM verifier in pg_authid, it will ask the client to do SCRAM authentication. If the server has a plaintext password in pg_authid, it will also ask the client to do SCRAM authentication (it could ask for MD5 authentication, but as the code stands, it will ask for SCRAM). The server will only ask the client to do plaintext password authentication, if you put "password" as the authentication method in pg_hba.conf. But that works regardless of what password_encryption is set to. No, I don't think there's any valid reason to store passwords in plaintext anymore. In theory, you could use either MD5 or SCRAM authentication with a plaintext password, which would be an advantage, but we don't provide an option for that. - Heikki
Heikki Linnakangas <hlinnaka@iki.fi> writes:
> On 05/03/2017 07:14 PM, Tom Lane wrote:
>> Is it possible that there are still client libraries that don't support
>> password encryption at all? If so, are we willing to break them?
>> I'd say "yes" but it's worth thinking about.
> That doesn't make sense. The client doesn't even know what
> password_encryption is set to. I think you're confusing
> password_encryption='plain' with the plaintext "password" authentication
> method.
Ah, you're right.
> If the server has an MD5 hash stored in pg_authid, the server will ask
> the client to do MD5 authentication. If the server has a SCRAM verifier
> in pg_authid, it will ask the client to do SCRAM authentication. If the
> server has a plaintext password in pg_authid, it will also ask the
> client to do SCRAM authentication (it could ask for MD5 authentication,
> but as the code stands, it will ask for SCRAM).
Um. That would be a backwards compatibility break ... but it doesn't
matter if we get rid of the option to store in plaintext.
The other question I can think to ask is what will happen during
pg_upgrade, given an existing installation with one or more passwords
stored plain. If the answer is "silently convert to MD5", I'd be
good with that.
regards, tom lane
On Wed, May 3, 2017 at 5:52 PM, Robert Haas <robertmhaas@gmail.com> wrote:
-- On Wed, May 3, 2017 at 7:31 AM, Heikki Linnakangas <hlinnaka@iki.fi> wrote:
> In various threads on SCRAM, we've skirted around the question of whether we
> should still allow storing passwords in plaintext. I've avoided discussing
> that in those other threads, because it's been an orthogonal question, but
> it's a good question and we should discuss it.
>
> So, I propose that we remove support for password_encryption='plain' in
> PostgreSQL 10. If you try to do that, you'll get an error.
I have no idea how widely used that option is.
> Another question that's been touched upon but not explicitly discussed, is
> whether we should change the default to "scram-sha-256". I propose that we
> do that as well. If you need to stick to md5, e.g. because you use drivers
> that don't support SCRAM yet, you can change it in postgresql.conf, but the
> majority of installations that use modern clients will be more secure by
> default.
I think that we should investigate how many connectors have support
for SCRAM or are likely to do so by the time v10 is released. A *lot*
of people are using connectors that are not based on libpq, especially
JDBC but I think many of the others as well. If most of those are
going to support SCRAM by the time v10 comes out, cool, but if not,
maybe it's wise to hold off for a release before flipping the default.
Not sure.
From the traffic on the list it sounds like the JDBC people are working on it already -- hopefully they will have something in time.
It might make sense to ping other "major drivers" people as well -- such as maybe npgsql. What else?
A good approach might be to change the default now, before beta. Then if drivers don't change, or if we get a lot of pushback from beta testers, we change it back before release. But if we don't change it, we will not know how big the impact would be...
On Wed, May 3, 2017 at 9:57 PM, Magnus Hagander <magnus@hagander.net> wrote: > > > On Wed, May 3, 2017 at 2:25 PM, Michael Paquier <michael.paquier@gmail.com> > wrote: >> >> On Wed, May 3, 2017 at 8:38 PM, Magnus Hagander <magnus@hagander.net> >> wrote: >> > On Wed, May 3, 2017 at 1:31 PM, Heikki Linnakangas <hlinnaka@iki.fi> >> > wrote: >> >> In various threads on SCRAM, we've skirted around the question of >> >> whether >> >> we should still allow storing passwords in plaintext. I've avoided >> >> discussing that in those other threads, because it's been an orthogonal >> >> question, but it's a good question and we should discuss it. >> >> >> >> So, I propose that we remove support for password_encryption='plain' in >> >> PostgreSQL 10. If you try to do that, you'll get an error. >> > >> > Is there any usecase at all for it today? >> >> For developers running applications on top of Postgres? > > > I don't get it. How does password_encryption=plain help them? Sanity checks at development stage of web applications to make sure that the password strength automatically generated by the application at first login is strong enough. I personally found that helpful for this purpose. -- Michael
On 05/03/2017 08:40 PM, Tom Lane wrote: > The other question I can think to ask is what will happen during > pg_upgrade, given an existing installation with one or more passwords > stored plain. If the answer is "silently convert to MD5", I'd be > good with that. Yes, it will silently convert to MD5. That happened even on earlier versions, if you had password_encryption=on in the new cluster (which was the default). I'm planning to go ahead with the attached patch for this (removing password_encryption='plain' support, but keeping the default as 'md5'). - Heikki -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Attachment
Tom Lane wrote: > Robert Haas <robertmhaas@gmail.com> writes: >> On Wed, May 3, 2017 at 7:31 AM, Heikki Linnakangas <hlinnaka@iki.fi> wrote: >>> So, I propose that we remove support for password_encryption='plain' in >>> PostgreSQL 10. If you try to do that, you'll get an error. >> I have no idea how widely used that option is. > Is it possible that there are still client libraries that don't support > password encryption at all? If so, are we willing to break them? > I'd say "yes" but it's worth thinking about. We have one application that has been reduced to "password" authentication ever since "crypt" authentication was removed, because they implemented the line protocol rather than using libpq and never bothered to move to "md5". But then, it might be a good idea to break this application, because that would force the vendor to implement something that is not a blatant security problem. Yours, Laurenz Albe
On Fri, May 5, 2017 at 9:38 AM, Albe Laurenz <laurenz.albe@wien.gv.at> wrote:
Tom Lane wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
>> On Wed, May 3, 2017 at 7:31 AM, Heikki Linnakangas <hlinnaka@iki.fi> wrote:
>>> So, I propose that we remove support for password_encryption='plain' in
>>> PostgreSQL 10. If you try to do that, you'll get an error.
>> I have no idea how widely used that option is.
> Is it possible that there are still client libraries that don't support
> password encryption at all? If so, are we willing to break them?
> I'd say "yes" but it's worth thinking about.
We have one application that has been reduced to "password" authentication
ever since "crypt" authentication was removed, because they implemented the
line protocol rather than using libpq and never bothered to move to "md5".
But then, it might be a good idea to break this application, because that
would force the vendor to implement something that is not a
blatant security problem.
It might. But I'm pretty sure the suggestion does not include removing the "password" authentication type, that one will still exist. This is just about password *storage*.
On Thu, May 4, 2017 at 8:37 PM, Heikki Linnakangas <hlinnaka@iki.fi> wrote: > On 05/03/2017 08:40 PM, Tom Lane wrote: >> >> The other question I can think to ask is what will happen during >> pg_upgrade, given an existing installation with one or more passwords >> stored plain. If the answer is "silently convert to MD5", I'd be >> good with that. > > > Yes, it will silently convert to MD5. That happened even on earlier > versions, if you had password_encryption=on in the new cluster (which was > the default). > > I'm planning to go ahead with the attached patch for this (removing > password_encryption='plain' support, but keeping the default as 'md5'). The patch attached does not apply on HEAD at 499ae5f, regression tests are conflicting. + This option is obsolete but still accepted for backwards + compatibility. Isn't that incorrect English? It seems to me that this be non-plural, as "for backward compatibility". The comment at the top of check_password() in passwordcheck.c does not mention scram, you may want to update that. + /* + * We never store passwords in plaintext, so this shouldn't + * happen. + */ break; An error here is overthinking? -- consistency of password entries -SET password_encryption = 'plain'; -CREATE ROLE regress_passwd1 PASSWORD 'role_pwd1';SET password_encryption = 'md5'; Nit: this is skipping directly to role number 2. -- Michael
On 05/05/2017 02:42 PM, Michael Paquier wrote: > + This option is obsolete but still accepted for backwards > + compatibility. > Isn't that incorrect English? No. > It seems to me that this be non-plural, > as "for backward compatibility". "Backwards" is not plural, it's a regional variation of "backward" (or vice versa depending on which region you come from). Both are correct. -- Vik Fearing +33 6 46 75 15 36 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
On 06/05/17 22:44, Vik Fearing wrote: > On 05/05/2017 02:42 PM, Michael Paquier wrote: >> + This option is obsolete but still accepted for backwards >> + compatibility. >> Isn't that incorrect English? > No. > >> It seems to me that this be non-plural, >> as "for backward compatibility". > "Backwards" is not plural, it's a regional variation of "backward" (or > vice versa depending on which region you come from). Both are correct. > I am English, born & bred, and 'Backwards' feels a lot more natural to me. Cheers, Gavin
On 05/06/2017 01:56 PM, Gavin Flower wrote: > On 06/05/17 22:44, Vik Fearing wrote: >> On 05/05/2017 02:42 PM, Michael Paquier wrote: >>> + This option is obsolete but still accepted for backwards >>> + compatibility. >>> Isn't that incorrect English? >> No. >> >>> It seems to me that this be non-plural, >>> as "for backward compatibility". >> "Backwards" is not plural, it's a regional variation of "backward" (or >> vice versa depending on which region you come from). Both are correct. > > I am English, born & bred, and 'Backwards' feels a lot more natural to me. Another data point: $ grep "backwards-comp" doc/src/sgml/ref/*.sgml doc/src/sgml/*.sgml | wc -l 7 $ grep "backward-comp" doc/src/sgml/ref/*.sgml doc/src/sgml/*.sgml | wc -l 3 Another important question is whether there should be a hyphen there or not? $ grep "backward comp" doc/src/sgml/ref/*.sgml doc/src/sgml/*.sgml | wc -l 21 ~/git-sandbox-pgsql/master (master)$ grep "backwards comp" doc/src/sgml/ref/*.sgml doc/src/sgml/*.sgml | wc -l 11 It looks like the most popular spelling in our docs is "backward compatibility". I'll go with that. - Heikki
On 05/05/2017 03:42 PM, Michael Paquier wrote: > + This option is obsolete but still accepted for backwards > + compatibility. > Isn't that incorrect English? It seems to me that this be non-plural, > as "for backward compatibility". I changed most cases to "backward compatibility", except the one in create_role.sgml, because there were other instances of "backwards compatibility" on that page, and I didn't want this to stick out. > The comment at the top of check_password() in passwordcheck.c does not > mention scram, you may want to update that. Reworded the comment, to not list all the possible values. > + /* > + * We never store passwords in plaintext, so > this shouldn't > + * happen. > + */ > break; > An error here is overthinking? It's not shown in the diff's context, but an error is returned just after the switch statement. I considered leaving out the "case PASSWORD_TYPE_PLAINTEXT" altogether, but then you might get compiler warnings complaining that that enum value is not handled. I also considered putting a an explicit "default:" there, which returns an error, but then you'd again miss out on compiler warnings, if you add a new password hash type and forget to add a case here to handle it. > -- consistency of password entries > -SET password_encryption = 'plain'; > -CREATE ROLE regress_passwd1 PASSWORD 'role_pwd1'; > SET password_encryption = 'md5'; > Nit: this is skipping directly to role number 2. Renumbered the test roles. Committed with those little cleanups. Thanks for the review! - Heikki
Re: [HACKERS] password_encryption, default and 'plain' support
From
ilmari@ilmari.org (Dagfinn Ilmari Mannsåker)
Date:
Heikki Linnakangas <hlinnaka@iki.fi> writes: > On 05/06/2017 01:56 PM, Gavin Flower wrote: >> On 06/05/17 22:44, Vik Fearing wrote: >>> On 05/05/2017 02:42 PM, Michael Paquier wrote: >>>> + This option is obsolete but still accepted for backwards >>>> + compatibility. >>>> Isn't that incorrect English? >>> No. >>> >>>> It seems to me that this be non-plural, >>>> as "for backward compatibility". >>> "Backwards" is not plural, it's a regional variation of "backward" (or >>> vice versa depending on which region you come from). Both are correct. >> >> I am English, born & bred, and 'Backwards' feels a lot more natural to me. > > Another data point: > > $ grep "backwards-comp" doc/src/sgml/ref/*.sgml doc/src/sgml/*.sgml | wc -l > 7 > $ grep "backward-comp" doc/src/sgml/ref/*.sgml doc/src/sgml/*.sgml | wc -l > 3 > > Another important question is whether there should be a hyphen there or not? > > $ grep "backward comp" doc/src/sgml/ref/*.sgml doc/src/sgml/*.sgml | > wc -l > 21 > ~/git-sandbox-pgsql/master (master)$ grep "backwards comp" > doc/src/sgml/ref/*.sgml doc/src/sgml/*.sgml | wc -l > 11 This misses out instances where the phrase is broken across lines. Using a less line-orented tool, ag (aka. The Silver Searcher), gives more hits for the non-hyphenated case, but doesn't significantly change the s-to-non-s ratio. $ ag 'backwards-\s*comp' doc/**/*.sgml|grep -c backward 7 $ ag 'backward-\s*comp' doc/**/*.sgml|grep -c backward 3 $ ag 'backward\s+comp' doc/**/*.sgml | grep -c backward 29 * ag 'backwards\s+comp' doc/**/*.sgml | grep -c backward 20 -- "I use RMS as a guide in the same way that a boat captain would usea lighthouse. It's good to know where it is, but yougenerallydon't want to find yourself in the same spot." - Tollef Fog Heen