Обсуждение: WIP: SCRAM authentication

There have been numerous threads on replacing our MD5 authentication
method, so I started hacking on that to see what it might look like.
Just to be clear, this is 9.6 material. Attached is a WIP patch series
that adds support for SCRAM. There's no need to look at the details yet,
but it demonstrates what the protocol changes and the code structure
would be like.

I'm not wedded to SCRAM - SRP or JPAKE or something else might be
better. But replacing the algorithm, or adding more of them, should be
straightforward with this.

There is no negotiation of the authentication mechanism. SCRAM is just
added as a new one, alongside all the existing ones. If the server
requests SCRAM authentication, but the client doesn't support it, the
attempt will fail. We might want to do something about that, to make the
transition easier, but it's an orthogonal feature and not absolutely
required.

There are four patches in the series. The first two are just
refactoring: moving the SHA-1 implementation from pgcrypto to
src/common, and some refactoring in src/backend/auth.c that IMHO would
make sense anyway.

Patches three and four are the interesting ones:

3. Allow storing multiple verifiers in pg_authid
------------------------------------------------

Replace the pg_authid.rolpassword text field with an array, and rename
it to 'rolverifiers'. This allows storing multiple password hashes: an
MD5 hash for MD5 authentication, and a SCRAM salt and stored key for
SCRAM authentication, etc. Each element in the array is a string that
begins with the method's name. For example "md5:<MD5 hash>", or
"password:<plaintext>".

For dump/reload, and for clients that wish to create the hashes in the
client-side, there is a new option to CREATE/ALTER USER commands:
PASSWORD VERIFIERS '{ ... }', that allows replacing the array.

The old "ENCRYPTED/UNENCRYPTED PASSWORD 'foo'" options are still
supported for backwards-compatibility, but it's not clear what it should
mean now.

TODO:

* Password-checking hook needs to be redesigned, to allow for more kinds
of hashes.

* With "CREATE USER PASSWORD 'foo'", which hashes/verifiers should be
generated by default? We currently have a boolean password_encryption
setting for that. Needs to be a list.

4. Implement SCRAM
------------------

The protocol and the code is structured so that it would be fairly easy
to add more built-in SASL mechanisms, or to use a SASL library to
provide more. But for now I'm focusing on adding exactly one new
built-in mechanism, to replace MD5 in the long term.

In the protocol, there is a new AuthenticationSASL message, alongside
the existing AuthenticationMD5, AuthenticationSSPI etc. The
AuthenticationSASL message contains the name of the SASL mechanism used
("SCRAM-SHA-1"). Just like in the GSSAPI/SSPI authentication, a number
of PasswordMessage and AuthenticationSASLContinue messages are exchanged
after that, carrying the data specified by the SCRAM spec, until the
authentication succeeds (or not).

TODO:

* Per the SCRAM specification, the client sends the username in the
handshake. But in the FE/BE protocol, we've already sent it in the
startup packet. In the patch, libpq always sends an empty username in
the SCRAM exchange, and the username from the startup packet is what
matters. We could also require it to be the same, but in SCRAM the
username to be UTF-8 encoded, while in PostgreSQL the username can be in
any encoding. That is a source of annoyance in itself, as it's not
well-defined in PostgreSQL which encoding to use when sending a username
to the server. But I don't want to try fixing that in this patch, so it
seems easiest to just require the username to be empty.

* Need a source of randomness in client, to generate random nonces used
in the handshake. The SCRAM specification is not explicit about it, but
I believe it doesn't need to be unpredictable, as long as a different
nonce is used for each authentication.

* The client does not authenticate the server, even though the SCRAM
protocol allows that. The client does verify the proof the server sends,
but nothing stops a malicious server that's impersonating the real
server from not requesting SCRAM authentication in the first place. It
could just send AuthenticationOK without any authentication at all. To
take advantage of the server authentication, we'll need to add something
similar to the "sslmode=verify-ca" option in the client. In that mode,
the client should refuse the connection if the server doesn't request
SCRAM authentication (or some other future authentication mechanism that
authenticates the server to the client).

* Channel binding is not implemented. Not essential, but would be nice
to have. Together with the new client option mentioned in the previous
point, it would allow the client to know that there is no
man-in-the-middle, without having to verify the server's SSL certificate.

- Heikki

* Heikki Linnakangas (hlinnaka@iki.fi) wrote:
> There have been numerous threads on replacing our MD5 authentication
> method, so I started hacking on that to see what it might look like.
> Just to be clear, this is 9.6 material. Attached is a WIP patch
> series that adds support for SCRAM. There's no need to look at the
> details yet, but it demonstrates what the protocol changes and the
> code structure would be like.

Great!  Very glad that you're working on this.

> I'm not wedded to SCRAM - SRP or JPAKE or something else might be
> better. But replacing the algorithm, or adding more of them, should
> be straightforward with this.

Excellent.

> 3. Allow storing multiple verifiers in pg_authid
> ------------------------------------------------
>
> Replace the pg_authid.rolpassword text field with an array, and
> rename it to 'rolverifiers'. This allows storing multiple password
> hashes: an MD5 hash for MD5 authentication, and a SCRAM salt and
> stored key for SCRAM authentication, etc. Each element in the array
> is a string that begins with the method's name. For example
> "md5:<MD5 hash>", or "password:<plaintext>".
>
> For dump/reload, and for clients that wish to create the hashes in
> the client-side, there is a new option to CREATE/ALTER USER
> commands: PASSWORD VERIFIERS '{ ... }', that allows replacing the
> array.
>
> The old "ENCRYPTED/UNENCRYPTED PASSWORD 'foo'" options are still
> supported for backwards-compatibility, but it's not clear what it
> should mean now.
>
> TODO:
>
> * Password-checking hook needs to be redesigned, to allow for more
> kinds of hashes.
>
> * With "CREATE USER PASSWORD 'foo'", which hashes/verifiers should
> be generated by default? We currently have a boolean
> password_encryption setting for that. Needs to be a list.

This generally sounds good to me but we definitely need to have that
list of hashes to be used.  The MIT KDC for Kerberos (and I believe all
the other Kerberos implementations) have a similar setting for what will
be stored and what will be allowed for hashing and encryption options.
It's very important that we allow users to tweak this list, as we will
want to encourage users to migrate off of the existing md5 storage
mechanism and on to the SCRAM based one eventually.

Unfortunately, the first major release with this will certainly need to
default to including md5 as we can't have a password update or change
break clients right off the bat.  What I think would be fantastic would
be a warning, perhaps in the first release or maybe the second, which
deprecates md5 as an auth method and is thrown when a password is set
which includes storing an md5-based password.  I'm sure there will be
plenty of discussion about that in the future.

One additional item is that we need to have a way to prefer SCRAM-based
auth while allowing a fall-back to md5 if the client doesn't support it.
This might have to be driven by the client side explicitly saying "I
support SCRAM" from the start to avoid breaking existing clients.

> 4. Implement SCRAM
> ------------------
>
> The protocol and the code is structured so that it would be fairly
> easy to add more built-in SASL mechanisms, or to use a SASL library
> to provide more. But for now I'm focusing on adding exactly one new
> built-in mechanism, to replace MD5 in the long term.
>
> In the protocol, there is a new AuthenticationSASL message,
> alongside the existing AuthenticationMD5, AuthenticationSSPI etc.
> The AuthenticationSASL message contains the name of the SASL
> mechanism used ("SCRAM-SHA-1"). Just like in the GSSAPI/SSPI
> authentication, a number of PasswordMessage and
> AuthenticationSASLContinue messages are exchanged after that,
> carrying the data specified by the SCRAM spec, until the
> authentication succeeds (or not).
>
> TODO:
>
> * Per the SCRAM specification, the client sends the username in the
> handshake. But in the FE/BE protocol, we've already sent it in the
> startup packet. In the patch, libpq always sends an empty username
> in the SCRAM exchange, and the username from the startup packet is
> what matters. We could also require it to be the same, but in SCRAM
> the username to be UTF-8 encoded, while in PostgreSQL the username
> can be in any encoding. That is a source of annoyance in itself, as
> it's not well-defined in PostgreSQL which encoding to use when
> sending a username to the server. But I don't want to try fixing
> that in this patch, so it seems easiest to just require the username
> to be empty.

I don't like having it be empty..  I'm not looking at the spec right at
the moment, but have you confirmed that the username being empty during
the SCRAM discussion doesn't reduce the effectiveness of the
authentication method overall in some way?  Is it ever used in
generation of the authentication verifier, etc?  One way to address the
risk which you bring up about the different encodings might be to simply
discourage using non-UTF8-compliant encodings by throwing a warning or
refusing to support SCRAM in cases where the role wouldn't be allowed by
SCRAM (eg: in CREATE ROLE or ALTER ROLE when the SCRAM auth verifier
storage is being handled).  Another option might be to define a way to
convert from "whatever" to "UTF8 something" for the purposes of the
SCRAM auth method.

> * Need a source of randomness in client, to generate random nonces
> used in the handshake. The SCRAM specification is not explicit about
> it, but I believe it doesn't need to be unpredictable, as long as a
> different nonce is used for each authentication.

I'd *very* much prefer a well defined and understood way (ideally
implemented in some well known and maintained library) rather than
trying to work out something ourselves.  Further, it'd be good to review
what others have done in this space with SCRAM as there may be lessons
learned or at least well reviewed approaches to consider.

> * The client does not authenticate the server, even though the SCRAM
> protocol allows that. The client does verify the proof the server
> sends, but nothing stops a malicious server that's impersonating the
> real server from not requesting SCRAM authentication in the first
> place. It could just send AuthenticationOK without any
> authentication at all. To take advantage of the server
> authentication, we'll need to add something similar to the
> "sslmode=verify-ca" option in the client. In that mode, the client
> should refuse the connection if the server doesn't request SCRAM
> authentication (or some other future authentication mechanism that
> authenticates the server to the client).

Agreed, we should have a way for the client to require SCRAM.
Presumably we would do this for libpq-based clients and expect other
implementations to look at the options we build into libpq for their own
versions (eg: JDBC).  There's nothing protocol-level to be done here
that I can think of off-hand.

> * Channel binding is not implemented. Not essential, but would be
> nice to have. Together with the new client option mentioned in the
> previous point, it would allow the client to know that there is no
> man-in-the-middle, without having to verify the server's SSL
> certificate.

Agreed, this is definitely one of the good features of SCRAM and should
be included.

Haven't looked at the code at all.
Thanks!
    Stephen

On 03/30/2015 06:46 PM, Stephen Frost wrote:
> * Heikki Linnakangas (hlinnaka@iki.fi) wrote:
>> * With "CREATE USER PASSWORD 'foo'", which hashes/verifiers should
>> be generated by default? We currently have a boolean
>> password_encryption setting for that. Needs to be a list.
>
> This generally sounds good to me but we definitely need to have that
> list of hashes to be used.  The MIT KDC for Kerberos (and I believe all
> the other Kerberos implementations) have a similar setting for what will
> be stored and what will be allowed for hashing and encryption options.
> It's very important that we allow users to tweak this list, as we will
> want to encourage users to migrate off of the existing md5 storage
> mechanism and on to the SCRAM based one eventually.
>
> Unfortunately, the first major release with this will certainly need to
> default to including md5 as we can't have a password update or change
> break clients right off the bat.  What I think would be fantastic would
> be a warning, perhaps in the first release or maybe the second, which
> deprecates md5 as an auth method and is thrown when a password is set
> which includes storing an md5-based password.  I'm sure there will be
> plenty of discussion about that in the future.

Yeah. And even if client are updated, and the server is upgraded, you 
still cannot use SCRAM until all the passwords have been changed and the 
SCRAM verifiers for them generated. Unless we go with the scheme I 
mentioned earlier, and use the MD5 hash of the password as the 
"plaintext" password to SCRAM.

> One additional item is that we need to have a way to prefer SCRAM-based
> auth while allowing a fall-back to md5 if the client doesn't support it.
> This might have to be driven by the client side explicitly saying "I
> support SCRAM" from the start to avoid breaking existing clients.

I'll start a separate thread on this. It's an interesting feature on its 
own. As well as an option in libpq to refuse plaintext authentication 
even if the server asks for it.

>> * Per the SCRAM specification, the client sends the username in the
>> handshake. But in the FE/BE protocol, we've already sent it in the
>> startup packet. In the patch, libpq always sends an empty username
>> in the SCRAM exchange, and the username from the startup packet is
>> what matters. We could also require it to be the same, but in SCRAM
>> the username to be UTF-8 encoded, while in PostgreSQL the username
>> can be in any encoding. That is a source of annoyance in itself, as
>> it's not well-defined in PostgreSQL which encoding to use when
>> sending a username to the server. But I don't want to try fixing
>> that in this patch, so it seems easiest to just require the username
>> to be empty.
>
> I don't like having it be empty..  I'm not looking at the spec right at
> the moment, but have you confirmed that the username being empty during
> the SCRAM discussion doesn't reduce the effectiveness of the
> authentication method overall in some way?

Yes.

> Is it ever used in
> generation of the authentication verifier, etc?  One way to address the
> risk which you bring up about the different encodings might be to simply
> discourage using non-UTF8-compliant encodings by throwing a warning or
> refusing to support SCRAM in cases where the role wouldn't be allowed by
> SCRAM (eg: in CREATE ROLE or ALTER ROLE when the SCRAM auth verifier
> storage is being handled).  Another option might be to define a way to
> convert from "whatever" to "UTF8 something" for the purposes of the
> SCRAM auth method.

Presumably the username used in the SCRAM exchange would have to match 
the username sent in the startup packet. Otherwise things get weird. If 
an empty string is a problem (there actually seems to be some language 
in the spec to forbid or at least discourage using an empty string as 
username), we could also specify some other constant that must be used, 
to mean "same as in startup packet".

- Heikki

Heikki,

* Heikki Linnakangas (hlinnaka@iki.fi) wrote:
> On 03/30/2015 06:46 PM, Stephen Frost wrote:
> >Unfortunately, the first major release with this will certainly need to
> >default to including md5 as we can't have a password update or change
> >break clients right off the bat.  What I think would be fantastic would
> >be a warning, perhaps in the first release or maybe the second, which
> >deprecates md5 as an auth method and is thrown when a password is set
> >which includes storing an md5-based password.  I'm sure there will be
> >plenty of discussion about that in the future.
>
> Yeah. And even if client are updated, and the server is upgraded,
> you still cannot use SCRAM until all the passwords have been changed
> and the SCRAM verifiers for them generated. Unless we go with the
> scheme I mentioned earlier, and use the MD5 hash of the password as
> the "plaintext" password to SCRAM.

I don't like using the MD5 hash of the password for the token or to
generate the verifier.  I'm no cryptographer and can't speak with any
certainty on the topic, but I understand there can be cases where doing
such would actively reduce the complexity required to find a usable
token.  Further, to be frank and slightly paranoid, it's possible that
individuals have built up caches of known-to-be-valid PG-encoded md5
strings from backups or other stolen data and we would not be doing
people who run those systems any favors with that approach.

At the end of the day, I'd much prefer to see a clean break with new
passwords rolled out by administrators working with their users.  That,
of course, would be much easier if we provided the basics that every
other auth system out there does today, which are the capabilities
available through pam_unix, pam_pwquality, pam_cracklib, etc.  In
particular, things like password history, password validity, password
complexity, account inactivity, account login history (successful vs.
failed attempts), are required in many environments, specifically called
out by NIST 800-53 which is required for US Government deployments, and
probably included in other standards also (eg: PCI, HIPPA, etc).

These are all things I was fighting for 10-or-so years ago.  I sincerely
hope that we're ready to consider these capabilities as being desirable.
I'm certainly hoping to work on them for 9.6 and will be happy to
support them going forward.

> >One additional item is that we need to have a way to prefer SCRAM-based
> >auth while allowing a fall-back to md5 if the client doesn't support it.
> >This might have to be driven by the client side explicitly saying "I
> >support SCRAM" from the start to avoid breaking existing clients.
>
> I'll start a separate thread on this. It's an interesting feature on
> its own. As well as an option in libpq to refuse plaintext
> authentication even if the server asks for it.

Agreed on both counts.

> >Is it ever used in
> >generation of the authentication verifier, etc?  One way to address the
> >risk which you bring up about the different encodings might be to simply
> >discourage using non-UTF8-compliant encodings by throwing a warning or
> >refusing to support SCRAM in cases where the role wouldn't be allowed by
> >SCRAM (eg: in CREATE ROLE or ALTER ROLE when the SCRAM auth verifier
> >storage is being handled).  Another option might be to define a way to
> >convert from "whatever" to "UTF8 something" for the purposes of the
> >SCRAM auth method.
>
> Presumably the username used in the SCRAM exchange would have to
> match the username sent in the startup packet. Otherwise things get
> weird. If an empty string is a problem (there actually seems to be
> some language in the spec to forbid or at least discourage using an
> empty string as username), we could also specify some other constant
> that must be used, to mean "same as in startup packet".

Ok..  Having it be a constant which means "same as in startup packet"
may be workable.  My suggestion above was intended to be "let's figure
out a way to encode whatever is in the startup packet to work in UTF8,
and then we will decode it ourselves" kind of idea.  That would make it
byte-wise different (at least in some cases- we might be able to
minimize the number of cases that happens), but semantically identical.
What would be *really* nice would be to say "if your client only speaks
UTF8, then you don't have to worry about this and everything just works
like normal."  That might be too much of a stretch though.
Thanks!
    Stephen

On Tue, Aug 4, 2015 at 4:20 PM, Michael Paquier wrote:
> I have been looking more in depths at this one, which adds essential
> infrastructure to support multiple authentication hashes for more protocols.
> Here are some comments:
> [spec lines]
> I am willing to write a patch for the next CF following more or less those
> lines, depending of course on the outcome of the discussion we can have
> here, so feel free to comment.

OK, attached as 0001 is the patch that respects those lines for the
support of multiple password verifiers in system catalogs. I have
added a new catalog called pg_auth_verifiers that is used at
authentication to fetch a password value depending on the protocol
used. With only this patch attached there are two types of verifiers:
plain and md5. This new catalog is REVOKE'd like pg_authid (pg_authid
could be made readable be this seems sensitive to me so I am not
changing it).

I have as well done the following things:
- Added PASSWORD VERIFIER (md5 = 'hoge', plain = 'hoge') which is used
as well by pg_dump all to be able to specify password verifiers one by
one.
- password check hook has been reworked as mentioned to be able to
manage a list of password verifiers instead of a single entry.
contrib/passwordcheck has been updated as well.
- Added regression tests testing UNENCRYPTED/ENCRYPTED, PASSWORD
VERIFIER, PASSWORD, etc.
- The patch does not break backward compatibility regarding CREATE
ROLE and ALTER ROLE.
- password_encryption has been changed to a list with comma-separated
elements, for now the possible elements are 'md5' and 'plain'. This
breaks backward compatibility, so if we care about it we should
consider having a new GUC password_encryption_list or similar. Default
is md5, default that does not change backward compatibility.
- Added documentation.
- pg_shadow has been updated, switching to an array with
method:password as elements.

> I'll have a look more in-depth at the scram patch as well.

The SCRAM patch (0002~0004) has been rebased to use the new facility.
I have as well fixed a syscache leak... But I haven't been able to
enter much in the details yet.

The patch 0001 to add the multiple verifier facility is in a state
good enough to get some review, so I am registering an entry for it in
the next CF. And I'll continue the work on the SCRAM portion next
week, with hopefully a version ready for CF submission.
Regards,
--
Michael

On Fri, Aug 7, 2015 at 3:22 AM, Michael Paquier
<michael.paquier@gmail.com> wrote:
> On Tue, Aug 4, 2015 at 4:20 PM, Michael Paquier wrote:
>> I have been looking more in depths at this one, which adds essential
>> infrastructure to support multiple authentication hashes for more protocols.
>> Here are some comments:
>> [spec lines]
>> I am willing to write a patch for the next CF following more or less those
>> lines, depending of course on the outcome of the discussion we can have
>> here, so feel free to comment.
>
> OK, attached as 0001 is the patch that respects those lines for the
> support of multiple password verifiers in system catalogs. I have
> added a new catalog called pg_auth_verifiers that is used at
> authentication to fetch a password value depending on the protocol
> used. With only this patch attached there are two types of verifiers:
> plain and md5. This new catalog is REVOKE'd like pg_authid (pg_authid
> could be made readable be this seems sensitive to me so I am not
> changing it).
>
> I have as well done the following things:
> - Added PASSWORD VERIFIER (md5 = 'hoge', plain = 'hoge') which is used
> as well by pg_dump all to be able to specify password verifiers one by
> one.

Maybe I'm chiming in too late here but I am sorta unimpressed by this.
If the user's password is stored both MD5-hashed and hashed some other
way in the system catalogs, that's less secure than storing it in the
least secure of those ways.  And I'm afraid that if we introduce this
new mechanism, we won't really gain any security, because everybody
will just pg_dump or pg_upgrade and the old passwords will stick
around in the system forever.  In fact we might lose security if
somebody changes one password verifier but doesn't realize that the
other one is still floating around, memorializing the old password,
and still available to be used for login.

I think we should look for a solution that either (a) allows SCRAM
authentication without requiring any changes to the contents of
pg_authid, like what Heikki proposed before; or (b) forces a hard
break, where at each password change you can decide if you want the
old or new format (probably based on the current value of some
compatibility GUC).

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On 08/07/2015 09:26 PM, Robert Haas wrote:
> Maybe I'm chiming in too late here but I am sorta unimpressed by this.
> If the user's password is stored both MD5-hashed and hashed some other
> way in the system catalogs, that's less secure than storing it in the
> least secure of those ways.  And I'm afraid that if we introduce this
> new mechanism, we won't really gain any security, because everybody
> will just pg_dump or pg_upgrade and the old passwords will stick
> around in the system forever.  In fact we might lose security if
> somebody changes one password verifier but doesn't realize that the
> other one is still floating around, memorializing the old password,
> and still available to be used for login.

Yeah, that's certainly a risk. You wouldn't want to keep around 
verifiers for authentication methods you don't use.

> I think we should look for a solution that either (a) allows SCRAM
> authentication without requiring any changes to the contents of
> pg_authid, like what Heikki proposed before; or (b) forces a hard
> break, where at each password change you can decide if you want the
> old or new format (probably based on the current value of some
> compatibility GUC).

Yeah, something to force a hard break when you want it would be really 
good. Perhaps a command you can run to remove all MD5 hashes, or at 
least find all the roles that have them. And a GUC to disallow creating 
new ones.

- Heikki

On Sat, Aug 8, 2015 at 3:45 AM, Heikki Linnakangas <hlinnaka@iki.fi> wrote:
> On 08/07/2015 09:26 PM, Robert Haas wrote:
>>
>> Maybe I'm chiming in too late here but I am sorta unimpressed by this.
>> If the user's password is stored both MD5-hashed and hashed some other
>> way in the system catalogs, that's less secure than storing it in the
>> least secure of those ways.  And I'm afraid that if we introduce this
>> new mechanism, we won't really gain any security, because everybody
>> will just pg_dump or pg_upgrade and the old passwords will stick
>> around in the system forever.  In fact we might lose security if
>> somebody changes one password verifier but doesn't realize that the
>> other one is still floating around, memorializing the old password,
>> and still available to be used for login.
>
>
> Yeah, that's certainly a risk. You wouldn't want to keep around verifiers
> for authentication methods you don't use.

Yep, I cannot refute that. And there is actually the same problem with
the first version of the patch proposed on this thread if that's what
you are referring at below.

>> I think we should look for a solution that either (a) allows SCRAM
>> authentication without requiring any changes to the contents of
>> pg_authid, like what Heikki proposed before; or (b) forces a hard
>> break, where at each password change you can decide if you want the
>> old or new format (probably based on the current value of some
>> compatibility GUC).

FWIW, the patch resets all the existing entries should any
CREATE/ALTER ROLE involving a password should be run, even if
pg_auth_verifiers has entries for method not specified with PASSWORD
VERIFIERS.

> Yeah, something to force a hard break when you want it would be really good.
> Perhaps a command you can run to remove all MD5 hashes, or at least find all
> the roles that have them. And a GUC to disallow creating new ones.

This filtering machinery definitely looks like a GUC to me, something
like password_forbidden_encryption that PASSWORD VERIFIERS looks at
and discards the methods listed in there. This definitely needs to be
separated from password_encryption.
-- 
Michael

On Fri, Aug 7, 2015 at 6:54 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:
> This filtering machinery definitely looks like a GUC to me, something
> like password_forbidden_encryption that PASSWORD VERIFIERS looks at
> and discards the methods listed in there. This definitely needs to be
> separated from password_encryption.

I don't know what a "password verifier" is and I bet nobody else does
either.  Well, I think I sort of know: I think it's basically an
encrypted password.  Am I right?  Even if I am, I bet the average user
is going to scratch their head and punt.

I don't see that there's any good reason to allow the same password to
be stored in the catalog encrypted more than one way, and I don't
think there's any good reason to introduce the PASSWORD VERIFIER
terminology.  I think we should store (1) your password, either
encrypted or unencrypted; and (2) the method used to encrypt it.  And
that's it.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 08/08/2015 06:27 AM, Robert Haas wrote:
> terminology.  I think we should store (1) your password, either 
> encrypted or unencrypted; and (2) the method used to encrypt it.
> And that's it.

A petty complaint, but it has always bothered me that we say the
password is encrypted when, at least currently, it is a simple hash
(cryptographic hash yes, but not encrypted, and not even an HMAC). I
think we should try to start using accurate terminology.

- -- 
Joe Conway
Crunchy Data
Enterprise PostgreSQL
http://crunchydata.com
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.22 (GNU/Linux)

iQIcBAEBAgAGBQJVxhm7AAoJEDfy90M199hlWRsQAIPliVReOYRRD/BJaZlB9Vjs
4YDolZZD9zR2+VPNxG/VaGHJ68rXlnfU/P0GrIQrS67t1xgwPxbUW6TCBsXJDnIE
wo7i5mJn9yn+AowccFiZTToKK8oNjRd33OJ2q00lAGiuaksnBhcJjMCNUHqf1Oz2
rUA/YiTp7RHXOQfiAxSoMKytK2y+rnQA+rnvPiE7XLKYE9rZ5rLiGhV0MPaNOFms
aHZIcYX5Tl2I3RsCexLMMA1qM001wSTyoti7o9gL71EXLV6ea6xt10a++k6oJ19y
oU7WjwKgV2XOGlQNC3/rUEKvuAtQhTlJpx9Q6xmTYidN0QHkZDdpJUblGZoxR2Vz
lT2zZdcpDhENynFZ1nTsd+CNWsn5T5vTVgnuKpG5qIMgT+kSG2JeiS7h+RY4rRtk
bl08tZmQBUBu/3hrRxQVPrt1NISteKXem2OLGphIKQEOmu/Kf43msYHQ+1qY0FTB
TZ96tVJnYTjQZp2P0IdjMf0qpOzK8qkMx2Tb6WehMd9yD1DtxQyKmxGpvssgEmQ7
1n3L/HCKWXF0MbI8QefIsO70ft4hzib5V+G7YmF00dWQM7NhDZYf6ejn1WmCP26u
w9wOHQcCAAKPI2knh3k2Ngdynl8Gofkxr7Le+NW7TGM+bp2U5EStTEH0r70mzEIg
KvB4dWX+tlZowujUmFhL
=VDCN
-----END PGP SIGNATURE-----

* Robert Haas (robertmhaas@gmail.com) wrote:
> On Fri, Aug 7, 2015 at 6:54 PM, Michael Paquier
> <michael.paquier@gmail.com> wrote:
> > This filtering machinery definitely looks like a GUC to me, something
> > like password_forbidden_encryption that PASSWORD VERIFIERS looks at
> > and discards the methods listed in there. This definitely needs to be
> > separated from password_encryption.
>
> I don't know what a "password verifier" is and I bet nobody else does
> either.  Well, I think I sort of know: I think it's basically an
> encrypted password.  Am I right?  Even if I am, I bet the average user
> is going to scratch their head and punt.

Password verifier is actually a well understood term when it comes to
these protocols and their implementations.  It is not an encrypted
password but rather a value which allows the server to determine if the
client knows the correct password, without having to store the password
directly, or a simple hash of the password, or have the clear password
sent from the client sent to the server.

> I don't see that there's any good reason to allow the same password to
> be stored in the catalog encrypted more than one way, and I don't
> think there's any good reason to introduce the PASSWORD VERIFIER
> terminology.  I think we should store (1) your password, either
> encrypted or unencrypted; and (2) the method used to encrypt it.  And
> that's it.

Perhaps we don't want to expose what a password verifier is to users,
but we shouldn't be missing the distinction between hashed passwords,
encrypted passwords, and password verifiers in the code and in the
implementation of SCRAM.  We really shouldn't use an incorrect term for
what we're storing in pg_authid either though, which is what we do
today.

I can't see us ever storing encrypted passwords as that implies we'd
need a key stored somewhere and further that the server would be able to
get back to the user's original password, neither of which are things we
want to deal with.

You do have a good point that there is some risk associated with having
multiple values in pg_authid related to a user's password and that we
really want to help users move from the old value in pg_authid to the
new one.  I don't believe we should force a hard change as it's going to
cause a lot of trouble for users.  We have to also consider that clients
also have to be changed for this.

As discussed previously, in an ideal world, we would handle the old
values and the new ones while introducing password ageing, client
support for detecting that a password needs to be changed, protocol
support for changing passwords which avoids having them get logged in
cleartext to the server log, password complexity, and perhaps used
password history to boot.  The main issue here is that we really don't
provide any help for large installations to get their userbase moved off
of the old style today.  Password ageing (and good support for it in
clients, etc), would help that greatly.

Unfortunately, it's unlikely that we're going to get all of that done in
one release.  As such, I'd suggest our next release support the existing
values in pg_authid, add the password verifier when the password is
changed, and then add a GUC in the following release which disables the
old pg_authid mechanism, defaulting to true, and the release after that
remove support for the old value and the field for it completely.

We should also provide documentation about how to check if there are any
old style values, for users who want to be proactive about moving off of
the old style.

I'm travelling and so I haven't looked over the patch yet (or even read
the entire thread in depth), so apologies if I've got something confused
about what's being proposed.
Thanks!
    Stephen

On 08/08/2015 04:27 PM, Robert Haas wrote:
> I don't see that there's any good reason to allow the same password to
> be stored in the catalog encrypted more than one way,

Sure there is. If you want to be able to authenticate using different 
mechanism, you need the same password "encrypted" in different ways. 
SCRAM uses verifier that's derived from the password in one way, MD5 
authentication needs an MD5 hash, and yet other protocols have other 
requirements.

> and I don't think there's any good reason to introduce the PASSWORD
> VERIFIER terminology.  I think we should store (1) your password,
> either encrypted or unencrypted; and (2) the method used to encrypt
> it.  And that's it.

Like Joe and Stephen, I actually find it highly confusing that we call 
the MD5 hash an "encrypted password". The term "password verifier" is 
fairly common in the specifications of authentication mechanisms. I 
think we should adopt it.

- Heikki

On Sat, Aug 8, 2015 at 6:23 PM, Heikki Linnakangas <hlinnaka@iki.fi> wrote:
> Like Joe and Stephen, I actually find it highly confusing that we call the
> MD5 hash an "encrypted password". The term "password verifier" is fairly
> common in the specifications of authentication mechanisms. I think we should
> adopt it.

Speaking as someone who hasn't read the specifications I found
"password verifier" surprising. I would have known what "password
hash" was but I misread "verifier" to be something functional like a
PAM plugin. I tend to agree we should just use terminology out of the
specs though even if it's a little opaque, better one opaque piece of
terminology than having to learn and translate between multiple
terminologies.


-- 
greg

On 08/08/2015 10:23 AM, Heikki Linnakangas wrote:
> On 08/08/2015 04:27 PM, Robert Haas wrote:
>> I don't see that there's any good reason to allow the same password to
>> be stored in the catalog encrypted more than one way,
> 
> Sure there is. If you want to be able to authenticate using different
> mechanism, you need the same password "encrypted" in different ways.
> SCRAM uses verifier that's derived from the password in one way, MD5
> authentication needs an MD5 hash, and yet other protocols have other
> requirements.

That's correct.  However, one of the goals of implementing SCRAM
authentication is to allow security-conscious users to get rid of those
reusable md5 hashes, no?

Obviously the backwards-compatibility issues are pretty major ... it'll
be years before all drivers support SCRAM ... but we also want to
provide a path forwards for secure installations in which no md5 hashes
are stored.

This says "backwards-compatible GUC" to me.  Here's one idea on how to
handle this:

1. we drop the parameter password_encryption

2. we add the parameter password_storage, which takes a list:  - plain : plain text  - md5 : current md5 hashes  -
scram: new scram hashed passwords  This defaults to 'md5, scram' if not specified.  This list might be extended in the
future.

3. All password types in the list are generated.  This means having
multiple columns in pg_shadow, or an array.  An array would support the
addition of future password storage methods.

4. CREATE ROLE / ALTER ROLE syntax is changed to accept a parameter to
ENCRYPTED in order to support md5, scram, and future methods.  If no
parameter is supplied, ENCRYPTED will default to 'md5, scram'.

5. we add the superuser-only function pg_apply_password_policy().  This
applies the policy expressed by password_storage, generating or erasing
passwords for each user.

6. We add a new connection error for "authentication __method__ not
supported for user"

7. Two versions from now, we change the defaults.

I thought about the idea of determining password storage based on what's
in pg_hba.conf, but that seems like way too much implied authorization
to me, and liable to be a big foot-gun.

--Josh Berkus

-- 
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com

On Sun, Aug 9, 2015 at 6:51 AM, Josh Berkus <josh@agliodbs.com> wrote:
> Obviously the backwards-compatibility issues are pretty major ... it'll
> be years before all drivers support SCRAM ... but we also want to
> provide a path forwards for secure installations in which no md5 hashes
> are stored.
>
> This says "backwards-compatible GUC" to me.  Here's one idea on how to
> handle this:
>
> 1. we drop the parameter password_encryption
> 2. we add the parameter password_storage, which takes a list:
>    - plain : plain text
>    - md5 : current md5 hashes
>    - scram : new scram hashed passwords
>    This defaults to 'md5, scram' if not specified.
>    This list might be extended in the future.

Perhaps using a different GUC than password_encryption is safer... I
am not that sure. Still that's how I switched password_encryption to
actually handle a list. Default is 'md5' with the first patch, and
'md5,scram' with the scram patch added and it sets the list of
password verifiers created when PASSWORD with ENCRYPTED/UNENCRYPTED is
used.

> 3. All password types in the list are generated.  This means having
> multiple columns in pg_shadow, or an array.  An array would support the
> addition of future password storage methods.

Yeah, the patch switches pg_shadow to an array like that, with as
elements method:value, so you get actually md5:md5blah,scram:stuff in
all the patches applied.

> 4. CREATE ROLE / ALTER ROLE syntax is changed to accept a parameter to
> ENCRYPTED in order to support md5, scram, and future methods.  If no
> parameter is supplied, ENCRYPTED will default to 'md5, scram'.

Like password ENCRYPTED (md5,scram) or similar? If no method is
passed, I think that we should default to password_storage instead.
Also, I still think that something like PASSWORD VERIFIERS is needed,
users may want to set the verifier user for each method after
calculating it on client-side: we authorize that for md5 even now, and
that's not something this spec authorizes.

> 5. we add the superuser-only function pg_apply_password_policy().  This
> applies the policy expressed by password_storage, generating or erasing
> passwords for each user.

pg_upgrade could make use of that to control password aging with an
option to do the cleanup or not. Not sure what the default should be
though. pg_apply_password_policy(roleid) would be useful as well to do
it on a role base.

> 6. We add a new connection error for "authentication __method__ not
> supported for user"

Hm? This would let any user trying to connect with a given method know
that if a method is used or not. What's wrong with failing as we do
now. In case of PASSWORD NULL for example, an attempt of connection
fails all the time with "incorrect password" or similar.

> 7. Two versions from now, we change the defaults.

Or three. We cannot expect users to change immediately, and it is
wiser to let dust set on the new feature in case critical bugs show up
after the first GA.

Something that sounds more like a detail in this thread by reading
other comments: I think that it is important to store password
verifiers in a different catalog than pg_authid for two reasons:
- that's more user-friendly, a sysadmin could directly join the new
catalog with pg_authid to get all the verifiers for a single user
method
- at password lookup when authorizing connection, there is no need to
fetch all the password verifiers and parse the array with all
verifiers.
- more scalable if we have many verifier methods in the future, though
we are not going to have hundreds of them. Though I am wondering about
per-method validtime and per-method authorization options.
Regards,
-- 
Michael

On 08/08/2015 03:21 PM, Michael Paquier wrote:
> On Sun, Aug 9, 2015 at 6:51 AM, Josh Berkus <josh@agliodbs.com> wrote:
>> 1. we drop the parameter password_encryption
>> 2. we add the parameter password_storage, which takes a list:
>>    - plain : plain text
>>    - md5 : current md5 hashes
>>    - scram : new scram hashed passwords
>>    This defaults to 'md5, scram' if not specified.
>>    This list might be extended in the future.
> 
> Perhaps using a different GUC than password_encryption is safer... I
> am not that sure. Still that's how I switched password_encryption to
> actually handle a list. Default is 'md5' with the first patch, and
> 'md5,scram' with the scram patch added and it sets the list of
> password verifiers created when PASSWORD with ENCRYPTED/UNENCRYPTED is
> used.

Well, generally I feel like if we're going to change the *type* of a GUC
parameter, we ought to change the *name*.  It's far easier for users to
figure out that the contents of a parameter need to change if the name
is also changed.

In other words, I think "invalid parameter 'password_encryption'" is an
easier to understand error message than "invalid password_encryption
type 'on'".  Besides which, password_encryption was always a misnomer.

Unless you're going to still accept "on, off" in some kind of wierd
backwards-compatibitlity mode?  If so, how does that work?

> Like password ENCRYPTED (md5,scram) or similar? If no method is
> passed, I think that we should default to password_storage instead.

Make sense.

> Also, I still think that something like PASSWORD VERIFIERS is needed,
> users may want to set the verifier user for each method after
> calculating it on client-side: we authorize that for md5 even now, and
> that's not something this spec authorizes.

I don't follow this.  Mind you, I'm not sure that I need to.

>> 5. we add the superuser-only function pg_apply_password_policy().  This
>> applies the policy expressed by password_storage, generating or erasing
>> passwords for each user.
> 
> pg_upgrade could make use of that to control password aging with an
> option to do the cleanup or not. Not sure what the default should be
> though. pg_apply_password_policy(roleid) would be useful as well to do
> it on a role base.

No objections to an optional roleid parameter, if you think people will
use it.

>> 6. We add a new connection error for "authentication __method__ not
>> supported for user"
> 
> Hm? This would let any user trying to connect with a given method know
> that if a method is used or not. What's wrong with failing as we do
> now. In case of PASSWORD NULL for example, an attempt of connection
> fails all the time with "incorrect password" or similar.

So, the DBA sets password_storage = 'scram', but doesn't take the md5
lines out of pg_hba.conf.

The app dev tries to connect using a driver which only supports md5.
What error should they get?  A user/DBA who is getting "invalid
password" is going to spend a long time debugging it.  Also, it would be
very useful to have a distinctive error in the log, so that DBAs could
see who is *trying* to connect with the wrong verifier.

-- 
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com

On Sat, Aug 8, 2015 at 1:23 PM, Heikki Linnakangas <hlinnaka@iki.fi> wrote:
> On 08/08/2015 04:27 PM, Robert Haas wrote:
>> I don't see that there's any good reason to allow the same password to
>> be stored in the catalog encrypted more than one way,
>
> Sure there is. If you want to be able to authenticate using different
> mechanism, you need the same password "encrypted" in different ways. SCRAM
> uses verifier that's derived from the password in one way, MD5
> authentication needs an MD5 hash, and yet other protocols have other
> requirements.

Why do we need to be able to authenticate using more than one
mechanism?  If you have some clients that can't support SCRAM yet, you
might as well continue using MD5 across the board until that changes.
You're not going to get much real security out of using MD5 for some
authentication attempts and SCRAM for other ones, and the amount of
infrastructure we're proposing to introduce to support that is pretty
substantial.

>> and I don't think there's any good reason to introduce the PASSWORD
>> VERIFIER terminology.  I think we should store (1) your password,
>> either encrypted or unencrypted; and (2) the method used to encrypt
>> it.  And that's it.
>
>
> Like Joe and Stephen, I actually find it highly confusing that we call the
> MD5 hash an "encrypted password". The term "password verifier" is fairly
> common in the specifications of authentication mechanisms. I think we should
> adopt it.

OK, but it sure looked from Michael's syntax description like you
wrote PASSWORD VERIFIER (md5 'the_actual_password').   Or at least
that was my impression from reading it, maybe I got it wrong.  If you
want to introduce ALTER USER ... PASSWORD VERIFIER as alternative
syntax for what we now call ALTER USER ... ENCRYPTED PASSWORD, that
works for me.  But a plaintext password shouldn't be called a password
verifier under the terminology you are using here, IIUC.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On 08/09/2015 08:09 AM, Robert Haas wrote:
> Why do we need to be able to authenticate using more than one
> mechanism?  If you have some clients that can't support SCRAM yet, you
> might as well continue using MD5 across the board until that changes.
> You're not going to get much real security out of using MD5 for some
> authentication attempts and SCRAM for other ones, 

Speaking as someone who has sheperded several clients through
infrastructure upgrades, I have to disagree with this.

First, people don't upgrade large infrastructures with multiple
applications, ETL processes and APIs which connect with the database all
at once.  They do it one component at a time, verify that component is
working, and then move on to the next one.  Even within a single
application, there could be many servers to upgrade, and you can't do
them all simultaneously.

Now, for shops where they've had the foresight to set up group roles
which own objects so that a new user with SCRAM can be assigned in the
group role, this is no problem.  But for the other 98% of our large-app
users, setting up that kind of infrastructure would itself require a
weekend-long downtime, due to the locking required to reassign object
permissions and all of the app testing required.

Second, you're forgetting hosted PostgreSQL, where there may be only one
user available to each database owner.  So assigning a new login role
for SCRAM isn't even an option.

Plus all of the above requires that some login roles have a SCRAM
verifier, and others have MD5, for some period.  Even if we don't
support multiple verifiers for one login, that still means we need to
deal with "what verifier gets created for a new role" and the required
support functions and GUCs for that.  Switching "across the board" on a
per-installation basis is a complete nonstarter for any running application.

Frankly, switching on a per-postmaster basis isn't even worth discussing
in my book, because some languages/platforms will take years longer than
others to support SCRAM.

Overall, it's to the PostgreSQL project's benefit to have users switch
to SCRAM once we have it available.  For that reason, we should try to
make it easy for them to make the switch.

However ...

> and the amount of
> infrastructure we're proposing to introduce to support that is pretty
> substantial.

... during my exchange with Michael, I was thinking about the bug
potential of taking the password field and multiplexing it in some way,
which is significant.  There is a definite risk of "making this too
complicated" and we'll need to contrast that against ease-of-migration,
because complicated mechanisms tend to be less secure due to user error.

-- 
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com

* Sehrope Sarkuni (sehrope@jackdb.com) wrote:
> It'd be nice if the new auth mechanism supports multiple passwords in the
> same format as well (not just one per format).
>
> That way you could have two different passwords for a user that are active
> at the same time. This would simplify rolling database credentials as it
> wouldn't have to be done all at once. You could add the new credentials,
> update your app servers one by one, then disable the old ones.
>
> A lot of systems that use API keys let you see the last time a particular
> set of keys was used. This helps answer the "Is this going to break
> something if I disable it?" question. Having a last used at timestamp for
> each auth mechanism (per user) would be useful.

Excellent points and +1 to all of these ideas from me.

> I'm not sure how updates should work when connecting to a read-only slave
> though. It would need some way of letting the master know that user X
> connected using credentials Y.

That wouldn't be all that hard to add to the protocol..

What would be nice also would be to include slave connections in
pg_stat_activity, so you could figure out what transaction on what slave
is causing your master to bloat...  And then if we could send signals
from the master to those processes, it'd be even nicer..
Thanks!
    Stephen

On Mon, Aug 10, 2015 at 3:42 AM, Josh Berkus <josh@agliodbs.com> wrote:
> On 08/09/2015 08:09 AM, Robert Haas wrote:
>> Why do we need to be able to authenticate using more than one
>> mechanism?  If you have some clients that can't support SCRAM yet, you
>> might as well continue using MD5 across the board until that changes.
>> You're not going to get much real security out of using MD5 for some
>> authentication attempts and SCRAM for other ones,
>
> Speaking as someone who has sheperded several clients through
> infrastructure upgrades, I have to disagree with this.
>
> [...]

>> and the amount of
>> infrastructure we're proposing to introduce to support that is pretty
>> substantial.

Maybe. But that's worth it IMO. I think that we should keep in mind as
well that we may not stick with SCRAM forever either and that we may
have to do a similar recommended-protocol switch at some point. Or
that we may have to implement additional authorization protocols in
parallel to what we have which would still require manipulation of
multiple verifiers per role.

> ... during my exchange with Michael, I was thinking about the bug
> potential of taking the password field and multiplexing it in some way,
> which is significant.  There is a definite risk of "making this too
> complicated" and we'll need to contrast that against ease-of-migration,
> because complicated mechanisms tend to be less secure due to user error.

Sure. That's why I am all in for adding a compatibility GUC or similar
that enforces the removal of old verifier types after marking those as
deprecated for a couple of years as there's surely a significant risk
to keep old passwords around or bad pg_hba entries. Still we need IMO
a way for a user to save multiple verifiers generated from a client to
manage carefully the password verifier aging, deprecations and support
removal.
-- 
Michael

On Mon, Aug 10, 2015 at 6:05 AM, Stephen Frost <sfrost@snowman.net> wrote:
> * Sehrope Sarkuni (sehrope@jackdb.com) wrote:
>> It'd be nice if the new auth mechanism supports multiple passwords in the
>> same format as well (not just one per format).
>>
>> That way you could have two different passwords for a user that are active
>> at the same time. This would simplify rolling database credentials as it
>> wouldn't have to be done all at once. You could add the new credentials,
>> update your app servers one by one, then disable the old ones.
>>
>> A lot of systems that use API keys let you see the last time a particular
>> set of keys was used. This helps answer the "Is this going to break
>> something if I disable it?" question. Having a last used at timestamp for
>> each auth mechanism (per user) would be useful.
>
> Excellent points and +1 to all of these ideas from me.

Interesting. I haven't thought of that and those are nice suggestions.
I am not convinced that this is something to tackle with a first
version of the patch though, I am sure we'll have enough problems to
deal with to get out a nice base usable for future improvements as
well.
-- 
Michael

On 08/09/2015 07:19 PM, Michael Paquier wrote:
>> ... during my exchange with Michael, I was thinking about the bug
>> > potential of taking the password field and multiplexing it in some way,
>> > which is significant.  There is a definite risk of "making this too
>> > complicated" and we'll need to contrast that against ease-of-migration,
>> > because complicated mechanisms tend to be less secure due to user error.

> Sure. That's why I am all in for adding a compatibility GUC or similar
> that enforces the removal of old verifier types after marking those as
> deprecated for a couple of years as there's surely a significant risk
> to keep old passwords around or bad pg_hba entries. Still we need IMO
> a way for a user to save multiple verifiers generated from a client to
> manage carefully the password verifier aging, deprecations and support
> removal.

That still falls under the heading of "possibly too complicated" though.

As I see it, there's two potential migration paths:

1. Allow multiple verifiers for each login role (Heikki's plan).

2. Set verifier type per login role.

(2) has the advantage of not requiring a bunch of new scaffolding
(although it will require a little) and thus being less likely to
introduce new bugs.  It also doesn't require adding new catalog
structures which are really only needed for the migration period, and
after which will become a wart (i.e. having multiple verifiers per login
role).

In real migration terms, though, (2) has some major drawbacks in terms
of making migration much harder.

a) it won't work for Heroku and other 1-login-per-database hosting.

b) moving to multiple roles from single roles per app is a painful
process currently.

For (a), one could argue that these are good candidates for "all at
once" migrations, and that moving to SCRAM will depend on the host
supporting it.  Someone from Heroku could speak up here.

For (b), there are a lot of things we could do to make migrating to a
multiple-role infra much easier for users, which would continue to be
useful even after the migration to SCRAM is history:

* remove the role requirement for ALTER DEFAULT PRIVILEGES, and ...

* add ALTER ROLE ___ ALTER DEFAULT OWNER, a command which sets the
default owner of newly created objects by that login role to a different
role of which they are a member.  Alternatively, add a way to make a
default SET ROLE whenever a login role logs in.

These two changes, or changes like them that serve the same purpose,
would allow us to prescribe the following migration path for most users:

1. add a new login role which is a member of the old login role and uses
SCRAM;

2. set the defaults for that role so that its objects and permissions
belong to the parent role;

3. move all applications to using SCRAM and the new role;

4. disable logins on the old, parent role.

It's currently (2) which is painfully difficult, and could be made less
so via the two features recommended above.

-- 
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com

* Robert Haas (robertmhaas@gmail.com) wrote:
> On Sat, Aug 8, 2015 at 1:23 PM, Heikki Linnakangas <hlinnaka@iki.fi> wrote:
> > On 08/08/2015 04:27 PM, Robert Haas wrote:
> >> I don't see that there's any good reason to allow the same password to
> >> be stored in the catalog encrypted more than one way,
> >
> > Sure there is. If you want to be able to authenticate using different
> > mechanism, you need the same password "encrypted" in different ways. SCRAM
> > uses verifier that's derived from the password in one way, MD5
> > authentication needs an MD5 hash, and yet other protocols have other
> > requirements.
>
> Why do we need to be able to authenticate using more than one
> mechanism?  If you have some clients that can't support SCRAM yet, you
> might as well continue using MD5 across the board until that changes.
> You're not going to get much real security out of using MD5 for some
> authentication attempts and SCRAM for other ones, and the amount of
> infrastructure we're proposing to introduce to support that is pretty
> substantial.

I agree with Josh that requiring a hard switch simply won't be
acceptable to our user community and if decide "there can only be one
running at a time" then we'll never get people to move off md5 and
therefore we wouldn't ever make real progress here.  Apologies to Josh
if I've misconstrued anything in his excellent response.

> >> and I don't think there's any good reason to introduce the PASSWORD
> >> VERIFIER terminology.  I think we should store (1) your password,
> >> either encrypted or unencrypted; and (2) the method used to encrypt
> >> it.  And that's it.
> >
> >
> > Like Joe and Stephen, I actually find it highly confusing that we call the
> > MD5 hash an "encrypted password". The term "password verifier" is fairly
> > common in the specifications of authentication mechanisms. I think we should
> > adopt it.
>
> OK, but it sure looked from Michael's syntax description like you
> wrote PASSWORD VERIFIER (md5 'the_actual_password').   Or at least
> that was my impression from reading it, maybe I got it wrong.  If you
> want to introduce ALTER USER ... PASSWORD VERIFIER as alternative
> syntax for what we now call ALTER USER ... ENCRYPTED PASSWORD, that
> works for me.  But a plaintext password shouldn't be called a password
> verifier under the terminology you are using here, IIUC.

Apologies for my incomplete up-thread definition of a password verifier,
it was a bit off-the-cuff while I was out of the country and referred to
the specific password verifier in SCRAM, which isn't a cleartext
or encrypted password or a simple hash of it, or a salted hash.

To flip it around as a positive definition instead of the negative "it's
not X, Y or Z" which I provided up-thread, a "password verifier" is a
general term for anything that can be used to verify that the client
knows the right password, so it technically could be the cleartext
version of the password, or a simple hash of the password, or an
encrypted version of the password, or pretty much anything else that
works for the protocol.  The reason it came about was because of this
very issue that there wasn't a general term for the value- it was
referred to as a salted hash, or encrypted, or just hashed, etc, and
those terms don't apply to everything that can be used today as a
password verifier.

As such, it's actually correct usage in this case as it's a general
term, rather than the specific and incorrect term "encrypted" which we
have today.
Thanks!
    Stephen

On Sun, Aug 9, 2015 at 2:42 PM, Josh Berkus <josh@agliodbs.com> wrote:
> On 08/09/2015 08:09 AM, Robert Haas wrote:
>> Why do we need to be able to authenticate using more than one
>> mechanism?  If you have some clients that can't support SCRAM yet, you
>> might as well continue using MD5 across the board until that changes.
>> You're not going to get much real security out of using MD5 for some
>> authentication attempts and SCRAM for other ones,
>
> Speaking as someone who has sheperded several clients through
> infrastructure upgrades, I have to disagree with this.
>
> First, people don't upgrade large infrastructures with multiple
> applications, ETL processes and APIs which connect with the database all
> at once.  They do it one component at a time, verify that component is
> working, and then move on to the next one.  Even within a single
> application, there could be many servers to upgrade, and you can't do
> them all simultaneously.

Right.  So what?  First, you upgrade all of the clients one by one to
a new version of the connector that supports SCRAM.
Second, once all of the clients that access a particular user account
can support SCRAM, you switch that account to use SCRAM.  The problem
you're talking about here would arise if, say, the JDBC maintainers
ripped out MD5 support at the same time they added SCRAM support.  But
that would be dumb.

> Second, you're forgetting hosted PostgreSQL, where there may be only one
> user available to each database owner.  So assigning a new login role
> for SCRAM isn't even an option.

Why do I care about having some of my authentication to a particular
role happen via SCRAM if I can't have it all happen via SCRAM?
Presumably, the benefit of SCRAM is that it's more secure than MD5
authentication.  So, if I can log into role X via MD5 but role Y only
via SCRAM, there might be some security benefit in that: an attacker
who can subvert MD5 but cannot subvert SCRAM can hack into role X but
not into role Y.  Good.  But allowing someone to authenticate to role
X via *either* SCRAM or MD5 doesn't help at all.  In fact it's
strictly worse, from a security perspective, than allowing someone to
authenticate via exactly one of those two methods, because now if the
attacker can subvert *either* of them he can break into that account.

In the hosted PostgreSQL situation you mention, there are really only
two cases.  Either all clients that need to log into that account can
support SCRAM, in which case we can use SCRAM and shut MD5 off.  Or
else they don't, and then we need to leave MD5 enabled.  But if we
have to leave MD5 enabled, then what exactly do we get out of using
SCRAM for some subset of the clients that can support it?

There may be a good answer to this question, but I don't think I've
seen it spelled out clearly.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On 08/11/2015 07:28 AM, Robert Haas wrote:
> There may be a good answer to this question, but I don't think I've
> seen it spelled out clearly.

Please see my follow-up post about making by-login-role migration easier
for users.

-- 
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com

On Tue, Aug 11, 2015 at 12:29 PM, Josh Berkus <josh@agliodbs.com> wrote:
> On 08/11/2015 07:28 AM, Robert Haas wrote:
>> There may be a good answer to this question, but I don't think I've
>> seen it spelled out clearly.
>
> Please see my follow-up post about making by-login-role migration easier
> for users.

I read it, and now I've reread it, but I don't see how it addresses
the points I raised.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On 08/11/2015 09:35 AM, Robert Haas wrote:
> On Tue, Aug 11, 2015 at 12:29 PM, Josh Berkus <josh@agliodbs.com> wrote:
>> On 08/11/2015 07:28 AM, Robert Haas wrote:
>>> There may be a good answer to this question, but I don't think I've
>>> seen it spelled out clearly.
>>
>> Please see my follow-up post about making by-login-role migration easier
>> for users.
> 
> I read it, and now I've reread it, but I don't see how it addresses
> the points I raised.

I'm not disagreeing with your security argument, BTW, which is why I'm
trying to come up with ways that make it easy for users to switch to
SCRAM via gradual rollout.

You're suggesting, then, that the switchover should be relatively easy,
because drivers will support both MD5 and SCRAM, and once all drivers
support both, the DBA can just swap verifiers?

That makes sense if drivers go that way.  I'm concerned that some
drivers will have a different call for a SCRAM connection than for an
MD5 one; we'd want to exert our project influence to prevent that from
happening.

That also makes it a bit harder to test the new auth on a few app
servers before a general rollout, but there's ways around that.

-- 
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com

On Tue, Aug 11, 2015 at 12:49 PM, Josh Berkus <josh@agliodbs.com> wrote:
> You're suggesting, then, that the switchover should be relatively easy,
> because drivers will support both MD5 and SCRAM, and once all drivers
> support both, the DBA can just swap verifiers?

Yes, that's what I was imagining would happen.  I can't imagine driver
authors wanting to remove support from MD5, because even if SCRAM goes
into 9.6, pre-9.6 servers are going to exist for many years to come,
and people are going to want to talk to them.

It seems to me that the protocol flow should be:

(1) Client sends StartupMessage.

(2) Server checks whether this user has an MD5 password verifier or a
SCRAM password verifier.  If the former, it responds with
AuthenticationMD5Password or AuthenticationCleartextPassword just as
it would do today, I guess based on pg_hba.conf.  If the latter, it
responds with a new protocol message AuthenticationScram.

So, if you switch the password verifier, the clients will all
automatically begin using SCRAM, because the server will tell them to.
And if they can't, they'll fail.

> That makes sense if drivers go that way.  I'm concerned that some
> drivers will have a different call for a SCRAM connection than for an
> MD5 one; we'd want to exert our project influence to prevent that from
> happening.

I'm not sure that would be a disaster, but do any existing drivers
have a different call for a cleartext password
(pg_hba.conf='password') than they do for an MD5 password
(pg_hba.conf='md5')?  If not, I'm not sure why they'd add that just
because there is now a third way of doing password-based
authentication.

> That also makes it a bit harder to test the new auth on a few app
> servers before a general rollout, but there's ways around that.

Well, staging servers are a good idea...

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On 08/11/2015 10:06 AM, Robert Haas wrote:
> On Tue, Aug 11, 2015 at 12:49 PM, Josh Berkus <josh@agliodbs.com> wrote:
>> That makes sense if drivers go that way.  I'm concerned that some
>> drivers will have a different call for a SCRAM connection than for an
>> MD5 one; we'd want to exert our project influence to prevent that from
>> happening.
> 
> I'm not sure that would be a disaster, but do any existing drivers
> have a different call for a cleartext password
> (pg_hba.conf='password') than they do for an MD5 password
> (pg_hba.conf='md5')?  If not, I'm not sure why they'd add that just
> because there is now a third way of doing password-based
> authentication.

Well, there is a different send-and-response cycle to the SCRAM
approach, no?  Plus, I've seen driver authors do strange things in the
past, including PHP's various drivers and pypgsql, which IIRC required
you to manually pick a protocol version.  I'm not saying we should plan
for bad design, we should just get the word out to driver authors that
we think it would be a good idea to support both methods transparently.

>> That also makes it a bit harder to test the new auth on a few app
>> servers before a general rollout, but there's ways around that.
> 
> Well, staging servers are a good idea...

Don't get me started. :-b

-- 
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com

On 8/11/15 10:28 AM, Robert Haas wrote:
> Right.  So what?  First, you upgrade all of the clients one by one to
> a new version of the connector that supports SCRAM.

In my experience, upgrading clients is, in many settings, significantly
harder than upgrading servers.  So I think any plan to starts like the
above isn't going to work.

On Tue, Aug 11, 2015 at 2:35 PM, Peter Eisentraut <peter_e@gmx.net> wrote:
> On 8/11/15 10:28 AM, Robert Haas wrote:
>> Right.  So what?  First, you upgrade all of the clients one by one to
>> a new version of the connector that supports SCRAM.
>
> In my experience, upgrading clients is, in many settings, significantly
> harder than upgrading servers.  So I think any plan to starts like the
> above isn't going to work.

Well, the real sequence of steps is actually:

1. Get SCRAM authentication committed to 9.6, and release 9.6.
2. Get driver authors to begin supporting SCRAM in their drivers.
3. Get users to update to those new drivers.
4. Enable SCRAM authentication for any given role once all clients
that need that role are SCRAM-capable.

Since supporting a new authentication method means adding a new
protocol message, there's no feasible method for rolling out SCRAM
without users at some point updating to a newer driver.  So there's
really not much choice but for (1)-(3) to happen.  If you're saying
it's likely to be a really long time before steps (2) and (3) are
completed for substantially all installations, I quite agree.

The thing we're actually debating here is whether enabling SCRAM
authentication for a role should also mean disabling MD5
authentication for that same role, or whether you should be able to
have two password verifiers stored for that role, one for SCRAM and
the other MD5.  If we allowed that, then you could turn SCRAM on for a
role, and only later turn MD5 off.  I think that's a bad plan because,
in most scenarios, allowing two forms of authentication for the same
account is strictly less secure than allowing only one.  And also
because it means adding a bunch of new system catalog machinery and
SQL syntax.  Instead, I think that, for any given role, you should get
to pick the way that password authentication works for that role:
either MD5, or SCRAM, but not "whichever of those two the client
prefers".

We don't actually have any reason to believe that MD5 is insecure in
the way that we're using it.  But if it turns out that somebody finds
an effective preimage attack against MD5, which I think what is what
Heikki said it would take to make our use unsafe, then it's not going
to be good enough to let people use SCRAM.  They're going to need to
enforce the use of SCRAM.  So I really don't understand why we want to
add a lot of complexity to let people mix the two authentication types
on the same user account.  Unless SCRAM is the ONLY way to
authenticate to the server, the fact that some clients are using it
does not help you.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On Fri, Aug 7, 2015 at 4:22 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:
> I have as well done the following things:
> - Added PASSWORD VERIFIER (md5 = 'hoge', plain = 'hoge') which is used
> as well by pg_dump all to be able to specify password verifiers one by
> one.
> - password check hook has been reworked as mentioned to be able to
> manage a list of password verifiers instead of a single entry.
> contrib/passwordcheck has been updated as well.
> - Added regression tests testing UNENCRYPTED/ENCRYPTED, PASSWORD
> VERIFIER, PASSWORD, etc.
> - The patch does not break backward compatibility regarding CREATE
> ROLE and ALTER ROLE.
> - password_encryption has been changed to a list with comma-separated
> elements, for now the possible elements are 'md5' and 'plain'. This
> breaks backward compatibility, so if we care about it we should
> consider having a new GUC password_encryption_list or similar. Default
> is md5, default that does not change backward compatibility.
> - Added documentation.
> - pg_shadow has been updated, switching to an array with
> method:password as elements.
>
>> I'll have a look more in-depth at the scram patch as well.
>
> The SCRAM patch (0002~0004) has been rebased to use the new facility.
> I have as well fixed a syscache leak... But I haven't been able to
> enter much in the details yet.
>
> The patch 0001 to add the multiple verifier facility is in a state
> good enough to get some review, so I am registering an entry for it in
> the next CF. And I'll continue the work on the SCRAM portion next
> week, with hopefully a version ready for CF submission.

Using rfc5802 as reference (http://tools.ietf.org/html/rfc5802), we
use "scram" as a name for the authentication method, like in
pg_hba.conf, but the correct name would be "scram-sha-1". Does it
really matter? We may want to implement other scram protocols using
different hash function than sha1, no? (in case channel binding is
supported, -plus can be appended).

There are still a list of TODO items in the patch that need to be
addressed and I have spotted a couple of bugs:
- the salt used when building the SCRAM verifier is for now the MD5
salt. We definitely need to use a longer one than MD5. but how much?
For now I have bumped it to 10.
- Regarding that (got a lesson from Tom a couple of months ago):
+# FIXME: any reason not to link with libpgcommon?
+OBJS += scram-common.o sha1.o
libpq cannot be linked with libpgcommon, because libpgcommon may not
be compiled to be position-independent code (depends on platform).
- When client receives the last message from server, it did not fetch
the server proof and did not check for it. I fixed that by
implementing what was needed.
- No randomness was used when generating the frontend nonce. I
switched to a call for random() without setting up a seed, which I
think could be improved with for example something based on
gettimeofday(). Thoughts about that are welcome. I think this could be
a separate patch as well.
- fe-connect.c is broken in PQconnectPoll because of this code block
like MD5 authentication:
+                               /* Get additional payload, if any */
+                               if (msgLength > 4)
+                               {
+                                       int                     llen =
msgLength - 4;
+ [...]
A correct fix seems to me to use this code path only for AUTH_REQ_SASL
and AUTH_REQ_SASL_CONT.
- The definition of nonce and salt length used by both the frontend
and backend looks better placed in common/scram-common.h.
- For now the password is not normalized with SASLprep. Do you think
that's necessary as a first shot in the authorization protocol?

Other things:
- SASL is a superset of protocols and SCRAM-SHA1 is one of them. We
may want to split the basic stuff of SASL and SCRAM-SHA1 into two
different patches, though having SASL alone does not make much sense
as without any protocols implemented we would just have some dead code
around until a protocol is implemented. We also refer to scram in
pg_hba.conf, but that's not completely correct to me, we may want to
use scram_sha1 actually, particularly if we implement new SASL methods
based on SCRAM, like for example SCRAM-SHA256.
- RandomSalt() in postmaster.c could be refactored such as it gets the
salt length as well in input. I implemented that as a separate patch
attached.
- SCRAM authentication uses the routines for encoding/decoding in
base64 in both frontend and backend, which are located in encode.c as
static routines on HEAD. It looks to me that it is a bad idea to
duplicate it in both frontend and backend as the scram patch does now.
It also does not seem to be a problem to move them in src/common/,
though the error handling needs to be reworked as those routines use
some elog calls which need to be removed. And actually, it seems to me
that it would be better to make a larger move and move all the
encoding/decoding routines of encode.c to libpgcommon (I mean hex,
escape *and* base64). I have done nothing about that yet, and thoughts
are welcome. This should be a separate patch as well that SCRAM relies
on.

For now, I am attaching a new series of patches, and the SCRAM
authentication is still using the new catalog pg_auth_verifiers.
Switching to a one-verifier-per-role approach or similar does not seem
to be a huge task to me.

Regards,
--
Michael

On 8/11/15 5:18 PM, Robert Haas wrote:
> The thing we're actually debating here is whether enabling SCRAM
> authentication for a role should also mean disabling MD5
> authentication for that same role, or whether you should be able to
> have two password verifiers stored for that role, one for SCRAM and
> the other MD5.  If we allowed that, then you could turn SCRAM on for a
> role, and only later turn MD5 off.  I think that's a bad plan because,
> in most scenarios, allowing two forms of authentication for the same
> account is strictly less secure than allowing only one.  And also
> because it means adding a bunch of new system catalog machinery and
> SQL syntax.  Instead, I think that, for any given role, you should get
> to pick the way that password authentication works for that role:
> either MD5, or SCRAM, but not "whichever of those two the client
> prefers".

I understand this idea, but I think it's not practical for many uses.
There is no way to find out, on the server, whether all current clients
would support a switch to SCRAM.  Let alone all not-current clients.
The only way to do such a switch would be to do the switch and then
check for connection failures in the log, which is not good.

It would be better if we allowed both methods side by side.  Then an
administrator can check in the logs which clients are using an old
method and track those down without interrupting production.

(Now that I think about this, to counter my point, this is very similar
to the switch from crypt to md5.  You couldn't turn that on until you
were sure that all clients would support it.  I don't remember the
experience from back then, though.)

Also, correct me if I'm wrong, but I believe using SCRAM would also make
it harder to use the password exchange sniffed from the wire.  So there
might be a benefit to using SCRAM even if you have to keep old and
supposedly insecure md5 hashes around for a while.

On Wed, Aug 12, 2015 at 10:50 AM, Peter Eisentraut <peter_e@gmx.net> wrote:
> I understand this idea, but I think it's not practical for many uses.
> There is no way to find out, on the server, whether all current clients
> would support a switch to SCRAM.  Let alone all not-current clients.
> The only way to do such a switch would be to do the switch and then
> check for connection failures in the log, which is not good.

Well, number one, I don't think we log anything currently that would
let you figure that out; and number two, I'm not sure I believe that's
the only way to make sure you've updated your clients.

> It would be better if we allowed both methods side by side.  Then an
> administrator can check in the logs which clients are using an old
> method and track those down without interrupting production.
>
> (Now that I think about this, to counter my point, this is very similar
> to the switch from crypt to md5.  You couldn't turn that on until you
> were sure that all clients would support it.  I don't remember the
> experience from back then, though.)

Maybe we should try to find out how that played out.  It could inform
the current discussion.

> Also, correct me if I'm wrong, but I believe using SCRAM would also make
> it harder to use the password exchange sniffed from the wire.  So there
> might be a benefit to using SCRAM even if you have to keep old and
> supposedly insecure md5 hashes around for a while.

Yeah.  I guess there's the scenario where you use SCRAM with clients
outside the firewall and MD5 with clients inside the firewall.  But,
meh.  For every person who benefits from the ability to configure
things that way, there will be 3 or 4 or 10 who enable SCRAM and never
get rid of their old password verifiers.  That will open up a
vulnerability for people to attack the old verifiers, or perhaps allow
some kind of attack where they can triangulate based on knowing that
the MD5 verifiers and the SCRAM verifier are based on the same
underlying password.

Another thing we might want to try to find out is: if we add SCRAM
authentication to 9.6, how committed are drivers authors to adding
that support to their drivers?  If we poll the maintainers of the
drivers for Perl, Python, Ruby, Node.JS, Java, ODBC, etc. and involve
them in this conversation, we might learn useful things.  This is a
big change we're talking about, and it's only to work (regardless of
the details) if the driver authors are on board.  We haven't, AFAIK,
talked to them about this at all.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On 8/12/15 12:19 PM, Robert Haas wrote:
> On Wed, Aug 12, 2015 at 10:50 AM, Peter Eisentraut <peter_e@gmx.net> wrote:
>> I understand this idea, but I think it's not practical for many uses.
>> There is no way to find out, on the server, whether all current clients
>> would support a switch to SCRAM.  Let alone all not-current clients.
>> The only way to do such a switch would be to do the switch and then
>> check for connection failures in the log, which is not good.
> 
> Well, number one, I don't think we log anything currently that would
> let you figure that out;

We have options to log details about authenticated connections.  This
kind of thing could easily be added there.

> and number two, I'm not sure I believe that's
> the only way to make sure you've updated your clients.

Well, if there is another one, I haven't heard about it so far.

>> (Now that I think about this, to counter my point, this is very similar
>> to the switch from crypt to md5.  You couldn't turn that on until you
>> were sure that all clients would support it.  I don't remember the
>> experience from back then, though.)
> 
> Maybe we should try to find out how that played out.  It could inform
> the current discussion.

I think in those days, installations weren't very long-lived.  md5 was
introduced in version 7.2, which is also the first version with lazy vacuum.

> Another thing we might want to try to find out is: if we add SCRAM
> authentication to 9.6, how committed are drivers authors to adding
> that support to their drivers?  If we poll the maintainers of the
> drivers for Perl, Python, Ruby, Node.JS, Java, ODBC, etc. and involve
> them in this conversation, we might learn useful things.  This is a
> big change we're talking about, and it's only to work (regardless of
> the details) if the driver authors are on board.  We haven't, AFAIK,
> talked to them about this at all.

I'm not concerned about whether drivers are willing to support this.
These sort of things have usually worked out in the past.  But just
because a driver supports a new authentication method, that doesn't mean
everyone can or will upgrade to it right away.

All,

* Peter Eisentraut (peter_e@gmx.net) wrote:
> On 8/12/15 12:19 PM, Robert Haas wrote:
> > On Wed, Aug 12, 2015 at 10:50 AM, Peter Eisentraut <peter_e@gmx.net> wrote:
> >> I understand this idea, but I think it's not practical for many uses.
> >> There is no way to find out, on the server, whether all current clients
> >> would support a switch to SCRAM.  Let alone all not-current clients.
> >> The only way to do such a switch would be to do the switch and then
> >> check for connection failures in the log, which is not good.
> >
> > Well, number one, I don't think we log anything currently that would
> > let you figure that out;
>
> We have options to log details about authenticated connections.  This
> kind of thing could easily be added there.
>
> > and number two, I'm not sure I believe that's
> > the only way to make sure you've updated your clients.
>
> Well, if there is another one, I haven't heard about it so far.

Agreed.

> >> (Now that I think about this, to counter my point, this is very similar
> >> to the switch from crypt to md5.  You couldn't turn that on until you
> >> were sure that all clients would support it.  I don't remember the
> >> experience from back then, though.)
> >
> > Maybe we should try to find out how that played out.  It could inform
> > the current discussion.
>
> I think in those days, installations weren't very long-lived.  md5 was
> introduced in version 7.2, which is also the first version with lazy vacuum.

I don't believe that's any basis for comparison given that it was well
over 10 years ago that we released 7.2.

> > Another thing we might want to try to find out is: if we add SCRAM
> > authentication to 9.6, how committed are drivers authors to adding
> > that support to their drivers?  If we poll the maintainers of the
> > drivers for Perl, Python, Ruby, Node.JS, Java, ODBC, etc. and involve
> > them in this conversation, we might learn useful things.  This is a
> > big change we're talking about, and it's only to work (regardless of
> > the details) if the driver authors are on board.  We haven't, AFAIK,
> > talked to them about this at all.
>
> I'm not concerned about whether drivers are willing to support this.
> These sort of things have usually worked out in the past.  But just
> because a driver supports a new authentication method, that doesn't mean
> everyone can or will upgrade to it right away.

I certainly want driver authors to support this and would be willing to
work with them to make it happen.  Discussing the change with them
certainly makes sense, in general.

As for the notion of dropping md5 from 9.6 or even forcing it to be
one-or-the-other on a per-role basis, I'm surprised we're even
discussing it.  We've already got people who aren't upgrading to newer,
supported, versions of PG because the application they have won't work
for some reason, this is creating a much harder upgrade path for those
users, and I don't believe that the security concern holds any water
compared to that issue.

For starters, if the attacker has access to the md5 hash in the DB, the
game is over already, full stop, when it comes to accessing the database
as that user.  We won't fix that until everyone's on SCRAM, so we should
be making it as easy as possible to get there.  I don't believe having
the SCRAM password verifier in the DB will seriously improve an
attacker's chances of getting the plaintext password simply because our
md5 "salt" completely sucks (no one serious about security would use it)
and md5 simply doesn't stand up to attacks very well these days.

I'm all for having an eventual flag day where we ship a release that
doesn't support md5 on the server side as an authentication mechanism
and pg_upgrade has a check which prevents upgrades if there are any md5
entries left, but that's at least two and probably three releases out.
What we need to be doing now is getting that ball rolling or we're never
going to actually get there.
Thanks!
    Stephen

On Wed, Aug 12, 2015 at 4:09 PM, Stephen Frost <sfrost@snowman.net> wrote:
> As for the notion of dropping md5 from 9.6 or even forcing it to be
> one-or-the-other on a per-role basis, ...

Please don't conflate those two things.  They are radically different
in terms of the amount of upgrade pain that they cause.  The first one
would be completely insane.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Robert,

* Robert Haas (robertmhaas@gmail.com) wrote:
> On Wed, Aug 12, 2015 at 4:09 PM, Stephen Frost <sfrost@snowman.net> wrote:
> > As for the notion of dropping md5 from 9.6 or even forcing it to be
> > one-or-the-other on a per-role basis, ...
>
> Please don't conflate those two things.  They are radically different
> in terms of the amount of upgrade pain that they cause.  The first one
> would be completely insane.

Thanks for the clarification.  I had gotten the (apparently mistaken)
impression[1] that there was some consideration for a hard break from
one release to the next to move from md5 to SCRAM.

Would be great to get comments on the other comments, specifically that
adding SCRAM's password verifier won't seriously change the security of
a user's account or password based on an attack vector where the
contents of pg_authid is compromised.  I do agree with the general
concern that the additional complexity involved in supporting multiple
password verifiers may result in bugs, and likely security ones, but I
really expect the larger risk to be from the SCRAM implementation itself
than how we get data into and back out of our own catalogs.

Thanks!
Stephen

[1]: CA+TgmoYQ=8BR87xgGkEWs8HJSe3KQh5v4fq+bZ2SAZhAnH3MtQ@mail.gmail.com

On 08/12/2015 01:37 PM, Stephen Frost wrote:
> Would be great to get comments on the other comments, specifically that
> adding SCRAM's password verifier won't seriously change the security of
> a user's account or password based on an attack vector where the
> contents of pg_authid is compromised.  I do agree with the general
> concern that the additional complexity involved in supporting multiple
> password verifiers may result in bugs, and likely security ones, but I
> really expect the larger risk to be from the SCRAM implementation itself
> than how we get data into and back out of our own catalogs.

There's also the concern that the additional complexity will cause
*users* to make security-compromising mistakes, which I think is the
greater risk.  Robert has mostly won me over to his point of view on this.

The only case where I can see multiple verifiers per role making a real
difference in migrations is for PGAAS hosting.  But the folks from
Heroku and AWS have been notably silent on this; lemme ping them.

-- 
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com

On Wed, Aug 12, 2015 at 4:37 PM, Stephen Frost <sfrost@snowman.net> wrote:
>> Please don't conflate those two things.  They are radically different
>> in terms of the amount of upgrade pain that they cause.  The first one
>> would be completely insane.
>
> Thanks for the clarification.  I had gotten the (apparently mistaken)
> impression[1] that there was some consideration for a hard break from
> one release to the next to move from md5 to SCRAM.

Gosh, no way.  Sorry if I gave the contrary impression; that was
certainly not my intention.  As I said before, I have no reason to
believe that MD5 is insecure in the way we are using it.  Unless much
better attacks are published, I don't really care if people are still
using it 5 years from now, or even 10.  I just want to give people an
*option* to move away from it, because:

(1) It's possible that better attacks may be published, and
(2) Even if they aren't, some people don't want to use anything that
says "MD5" on the tin.

> Would be great to get comments on the other comments, specifically that
> adding SCRAM's password verifier won't seriously change the security of
> a user's account or password based on an attack vector where the
> contents of pg_authid is compromised.  I do agree with the general
> concern that the additional complexity involved in supporting multiple
> password verifiers may result in bugs, and likely security ones, but I
> really expect the larger risk to be from the SCRAM implementation itself
> than how we get data into and back out of our own catalogs.

Yes, the SCRAM implementation could be buggy.  But also, OpenSSL has
repeatedly had security bugs that were due to forcing servers to
downgrade to older protocols.  I wouldn't like us to start growing
similar vulnerabilities, where SCRAM would have been just fine but an
attack that involves forcing a downgrade to MD5 is possible.

I don't think you are quite correct about the scenario where pg_authid
is compromised.  Even if the hash stored there is functionally
equivalent to the password itself as far as this instance of
PostgreSQL is concerned, the same password may have been used for
other services, so cracking it has a purpose.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On Thu, Aug 13, 2015 at 6:21 AM, Josh Berkus wrote:
> The only case where I can see multiple verifiers per role making a real
> difference in migrations is for PGAAS hosting.  But the folks from
> Heroku and AWS have been notably silent on this; lemme ping them.

Yes, I would be curious to hear from their requirements in this area.
I think it is actually a big deal for those folks to be able to get
multiple verifiers living in parallel for one role.
-- 
Michael

* Josh Berkus (josh@agliodbs.com) wrote:
> On 08/12/2015 01:37 PM, Stephen Frost wrote:
> > Would be great to get comments on the other comments, specifically that
> > adding SCRAM's password verifier won't seriously change the security of
> > a user's account or password based on an attack vector where the
> > contents of pg_authid is compromised.  I do agree with the general
> > concern that the additional complexity involved in supporting multiple
> > password verifiers may result in bugs, and likely security ones, but I
> > really expect the larger risk to be from the SCRAM implementation itself
> > than how we get data into and back out of our own catalogs.
>
> There's also the concern that the additional complexity will cause
> *users* to make security-compromising mistakes, which I think is the
> greater risk.  Robert has mostly won me over to his point of view on this.

That is certainly an issue to address- but that's one which I believe we
can address a great deal better than what we're doing with the currently
proposed patch.  I do feel we need to provide flexibility and options,
but we also need to consider the simple case and make sure that it
remains simple.

> The only case where I can see multiple verifiers per role making a real
> difference in migrations is for PGAAS hosting.  But the folks from
> Heroku and AWS have been notably silent on this; lemme ping them.

While their insight is certainly valuable, they are certainly not the
only cases of one-user-to-rule-them-all environments.  Further, there's
going to be cases where multiple applications from different languages
are accessing the database through the same account because there's only
one account.

I'd rather not punt on those cases and, further, assume that we'll
always be able to keep it to only one password verifier per account.  As
I tried to outline up-thread, there are a set of features which would be
very nice for us to have which require further information to be saved
beyond even these different password verifiers for each.

As mentioned elsewhere, even SCRAM is possible of having multiple
password verifiers based on the various algorithms used.  In other
words, I doubt the 'only one password verifier per role' approach is
going to work out for us long term in any case.
Thanks!    Stephen

On Thu, Aug 13, 2015 at 10:22 AM, Stephen Frost wrote:
>> The only case where I can see multiple verifiers per role making a real
>> difference in migrations is for PGAAS hosting.  But the folks from
>> Heroku and AWS have been notably silent on this; lemme ping them.
>
> While their insight is certainly valuable, they are certainly not the
> only cases of one-user-to-rule-them-all environments.  Further, there's
> going to be cases where multiple applications from different languages
> are accessing the database through the same account because there's only
> one account.
>
> I'd rather not punt on those cases and, further, assume that we'll
> always be able to keep it to only one password verifier per account.  As
> I tried to outline up-thread, there are a set of features which would be
> very nice for us to have which require further information to be saved
> beyond even these different password verifiers for each.

While looking at this stuff, I have been wondering as well about
moving the validutil field into the verifier catalog as well for
example. That's one.

> As mentioned elsewhere, even SCRAM is possible of having multiple
> password verifiers based on the various algorithms used.  In other
> words, I doubt the 'only one password verifier per role' approach is
> going to work out for us long term in any case.

SCRAM is a definition for an authorization protocol which includes
many verifiers, and the minimal requirement to consider that SCRAM is
implemented in a system is to have SCRAM-SHA1, per here:
http://tools.ietf.org/html/rfc5802
For example we may want to have in parallel one verifier for
SCRAM-SHA1 and one for SCRAM-SHA256 for the same user, and I think
that we cannot close the door either to other SASL protocols, which is
why it may make sense to split the SCRAM patch into two with the basic
message protocol infrastructure in place.
-- 
Michael

Robert,

* Robert Haas (robertmhaas@gmail.com) wrote:
> On Wed, Aug 12, 2015 at 4:37 PM, Stephen Frost <sfrost@snowman.net> wrote:
> >> Please don't conflate those two things.  They are radically different
> >> in terms of the amount of upgrade pain that they cause.  The first one
> >> would be completely insane.
> >
> > Thanks for the clarification.  I had gotten the (apparently mistaken)
> > impression[1] that there was some consideration for a hard break from
> > one release to the next to move from md5 to SCRAM.
>
> Gosh, no way.  Sorry if I gave the contrary impression; that was
> certainly not my intention.  As I said before, I have no reason to
> believe that MD5 is insecure in the way we are using it.  Unless much
> better attacks are published, I don't really care if people are still
> using it 5 years from now, or even 10.  I just want to give people an
> *option* to move away from it, because:

No worries, thanks again for the clarification and my apologies for
misunderstanding and jumping to an incorrect assumption.

As for the question about MD5, I do feel that we're using it in about
the best way possible, but I don't feel it's a terribly secure algorithm
today.  I'd really like to see people moving away from it sooner rather
than later and that our defaults move to much more secure solutions.

> (1) It's possible that better attacks may be published, and
> (2) Even if they aren't, some people don't want to use anything that
> says "MD5" on the tin.
>
> > Would be great to get comments on the other comments, specifically that
> > adding SCRAM's password verifier won't seriously change the security of
> > a user's account or password based on an attack vector where the
> > contents of pg_authid is compromised.  I do agree with the general
> > concern that the additional complexity involved in supporting multiple
> > password verifiers may result in bugs, and likely security ones, but I
> > really expect the larger risk to be from the SCRAM implementation itself
> > than how we get data into and back out of our own catalogs.
>
> Yes, the SCRAM implementation could be buggy.  But also, OpenSSL has
> repeatedly had security bugs that were due to forcing servers to
> downgrade to older protocols.  I wouldn't like us to start growing
> similar vulnerabilities, where SCRAM would have been just fine but an
> attack that involves forcing a downgrade to MD5 is possible.

I agree that such similar vulnerabilities would be unfortunate, but
the way to avoid that is to not implement the actual hashing or
encryption algorithms ourselves and to stick to the protocol as defined
in the specification.

> I don't think you are quite correct about the scenario where pg_authid
> is compromised.  Even if the hash stored there is functionally
> equivalent to the password itself as far as this instance of
> PostgreSQL is concerned, the same password may have been used for
> other services, so cracking it has a purpose.

I attempted to address that also by stating that, should an attacker
compromise a system with the goal of gaining the cleartext password,
they would attempt the following, in order:

1) attempt to compromise a superuser account, if not already done, and
then modify the system to get the 'password' auth mechanism to be used
whereby the password is sent in the clear

2) change the existing password, or encourge the user to do so and
somehow capture that activity

3) social engineering attacks

4) attempt to crack the md5 hash

5) attempt to crack the SCRAM password verifier

6) try to work out a way to use both the md5 hash and the SCRAM password
verifier to figure out the password

In terms of difficulty, while I believe the above is the right overall
ordering, the level of difficulty goes up seriously between 4 and 5,
with 6 requiring a level of expertise in hash algorithms and SCRAM that
I have a very hard time seeing anyone bothering to go there, rather than
just throw more CPUs at the md5 hash.
Thanks!
    Stephen

* Michael Paquier (michael.paquier@gmail.com) wrote:
> On Thu, Aug 13, 2015 at 10:22 AM, Stephen Frost wrote:
> >> The only case where I can see multiple verifiers per role making a real
> >> difference in migrations is for PGAAS hosting.  But the folks from
> >> Heroku and AWS have been notably silent on this; lemme ping them.
> >
> > While their insight is certainly valuable, they are certainly not the
> > only cases of one-user-to-rule-them-all environments.  Further, there's
> > going to be cases where multiple applications from different languages
> > are accessing the database through the same account because there's only
> > one account.
> >
> > I'd rather not punt on those cases and, further, assume that we'll
> > always be able to keep it to only one password verifier per account.  As
> > I tried to outline up-thread, there are a set of features which would be
> > very nice for us to have which require further information to be saved
> > beyond even these different password verifiers for each.
>
> While looking at this stuff, I have been wondering as well about
> moving the validutil field into the verifier catalog as well for
> example. That's one.

Agreed.

> > As mentioned elsewhere, even SCRAM is possible of having multiple
> > password verifiers based on the various algorithms used.  In other
> > words, I doubt the 'only one password verifier per role' approach is
> > going to work out for us long term in any case.
>
> SCRAM is a definition for an authorization protocol which includes
> many verifiers, and the minimal requirement to consider that SCRAM is
> implemented in a system is to have SCRAM-SHA1, per here:
> http://tools.ietf.org/html/rfc5802
> For example we may want to have in parallel one verifier for
> SCRAM-SHA1 and one for SCRAM-SHA256 for the same user, and I think
> that we cannot close the door either to other SASL protocols, which is
> why it may make sense to split the SCRAM patch into two with the basic
> message protocol infrastructure in place.

and further agreed here.  In addition, I'd point out that this is not a
novel area in secure applications- Kerberos/GSSAPI have long been doing
this, with both MIT Kerberos and Heimdal providing multiple algorithms
and supporting multiple password verifiers per user.  Active Directory
also, actually.  When it comes to enterprise authentication systems and
administrators who work with them, this is not overly complex nor
suprising.
Thanks!
    Stephen

On Wed, Aug 12, 2015 at 9:36 PM, Stephen Frost <sfrost@snowman.net> wrote:
>> Yes, the SCRAM implementation could be buggy.  But also, OpenSSL has
>> repeatedly had security bugs that were due to forcing servers to
>> downgrade to older protocols.  I wouldn't like us to start growing
>> similar vulnerabilities, where SCRAM would have been just fine but an
>> attack that involves forcing a downgrade to MD5 is possible.
>
> I agree that such similar vulnerabilities would be unfortunate, but
> the way to avoid that is to not implement the actual hashing or
> encryption algorithms ourselves and to stick to the protocol as defined
> in the specification.

Nothing in that will protect us if the client can request a non-SCRAM
form of authentication.

>> I don't think you are quite correct about the scenario where pg_authid
>> is compromised.  Even if the hash stored there is functionally
>> equivalent to the password itself as far as this instance of
>> PostgreSQL is concerned, the same password may have been used for
>> other services, so cracking it has a purpose.
>
> I attempted to address that also by stating that, should an attacker
> compromise a system with the goal of gaining the cleartext password,
> they would attempt the following, in order:

What if they steal a pg_dump?  All of the password verifiers are
there, but the live system is not.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

* Robert Haas (robertmhaas@gmail.com) wrote:
> On Wed, Aug 12, 2015 at 9:36 PM, Stephen Frost <sfrost@snowman.net> wrote:
> >> Yes, the SCRAM implementation could be buggy.  But also, OpenSSL has
> >> repeatedly had security bugs that were due to forcing servers to
> >> downgrade to older protocols.  I wouldn't like us to start growing
> >> similar vulnerabilities, where SCRAM would have been just fine but an
> >> attack that involves forcing a downgrade to MD5 is possible.
> >
> > I agree that such similar vulnerabilities would be unfortunate, but
> > the way to avoid that is to not implement the actual hashing or
> > encryption algorithms ourselves and to stick to the protocol as defined
> > in the specification.
>
> Nothing in that will protect us if the client can request a non-SCRAM
> form of authentication.

The exact same risk exists for OpenSSL, as you mention, but also for
Kerberos-based authentication systems as well.  The way to address those
risks is to not have an md5-based password verifier for the role.  If
only one password verifier exists per role then it makes moving off of
md5 more difficult.  I am not proposing a solution where both verifiers
exist forever and I specifically proposed that we even remove all MD5
based verifiers in the future, similar to how older encryption
algorithms are no longer supported by modern releases of MIT Kerberos.

> >> I don't think you are quite correct about the scenario where pg_authid
> >> is compromised.  Even if the hash stored there is functionally
> >> equivalent to the password itself as far as this instance of
> >> PostgreSQL is concerned, the same password may have been used for
> >> other services, so cracking it has a purpose.
> >
> > I attempted to address that also by stating that, should an attacker
> > compromise a system with the goal of gaining the cleartext password,
> > they would attempt the following, in order:
>
> What if they steal a pg_dump?  All of the password verifiers are
> there, but the live system is not.

From my previous list, 4, 5, and 6 are all data-at-rest attacks.  My
comments below the list also indicated my feeling about the data-at-rest
attacks, but to clarify, I strongly feel that an attacker would go after
the md5 hash far before even considering attacking either the SCRAM
password verifier or trying to use the combination of the two in a novel
way.  That isn't to say that there's no way the combination couldn't be
combined or that having both doesn't increase the risk- it certainly
does, but that exposure risk is really that we would then have two
algorithms on which we depend on to not be broken, for the period of
time that both are current.

More generally, however, the way to address these kinds of data-at-rest
attacks and loss of backup data is to have a password aging system where
the passwords are changed on a regular basis.  I certainly feel that we
should be looking to add that functionality and that we need to step up
and seriously move forward on implementing these capabilities that other
systems have had for decades or more.  We have excellent examples of
what enterprise authentication systems do and the kinds of capabilities
which they provide, both directly from examples such as Active Directory
and PAM, but also from requirements definitions used around the world
(eg: PCI compliance, NIST standards, and similar standards used by other
governments).
Thanks,
    Stephen

On 08/12/2015 06:36 PM, Stephen Frost wrote:
> I attempted to address that also by stating that, should an attacker
> compromise a system with the goal of gaining the cleartext password,
> they would attempt the following, in order:
> 
> 1) attempt to compromise a superuser account, if not already done, and
> then modify the system to get the 'password' auth mechanism to be used
> whereby the password is sent in the clear
> 
> 2) change the existing password, or encourge the user to do so and
> somehow capture that activity
> 
> 3) social engineering attacks
> 
> 4) attempt to crack the md5 hash
> 
> 5) attempt to crack the SCRAM password verifier
> 
> 6) try to work out a way to use both the md5 hash and the SCRAM password
> verifier to figure out the password
> 

I don't feel like you've correctly assessed the risk inherent in the
md5 auth method, which is that, having captured an md5auth string by
whatever means, and attacker can reuse that md5 string on other
databases in the network *without* cracking it.  That's the biggest risk
as long as md5 is present.

Aside from code complexity, the user security concern with a "multiple
verifier per role" approach is that the DBAs would never remember to
completely disable md5auth and would capture md5 hashes either in flight
or from backups.  This approach can be used to capture an md5hash from a
non-critical database which is poorly secured, and then re-use it
against an important database.

Now, the counter-argument to this is that a DBA is just as likely to
rememeber to remove md5 verifiers as she is to remember to remove roles
with md5auth.

Regardless of the approach we take, encouraging users to migrate is
going to be more of a matter of documentation, publicity, and
administrative tools than one of multiple verifiers vs. multiple roles.That is, giving DBAs the ability to see and log
who'susing what kind
 
of verifier, and what account has what verifier(s) available, will make
more of a difference.

-- 
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com

On Wed, Aug 12, 2015 at 1:37 PM, Michael Paquier wrote:
> For now, I am attaching a new series of patches, and the SCRAM
> authentication is still using the new catalog pg_auth_verifiers.
> Switching to a one-verifier-per-role approach or similar does not seem
> to be a huge task to me.

Attached is a new series of patches. Compared to the last version
submitted, encoding routines for base64, escape and hex have been
moved to src/common to avoid duplication between frontend and backend.
I have as well fix a couple of issues in the SCRAM implementation and
made some cosmetic changes to make the code easier to read and more
understandable.
Regards,
--
Michael

Josh,

* Josh Berkus (josh@agliodbs.com) wrote:
> I don't feel like you've correctly assessed the risk inherent in the
> md5 auth method, which is that, having captured an md5auth string by
> whatever means, and attacker can reuse that md5 string on other
> databases in the network *without* cracking it.  That's the biggest risk
> as long as md5 is present.

Robert and I, I believe, are both aware of that risk and had moved on to
discussing the risk to the cleartext password by keeping an md5 present,
even if it wasn't able to be used for login.  We will want to make it
simple for admins to enable/disable md5 (eg: pg_hba), but we also have
to consider how to make it simple for admins to remove the old md5
password verifier or there will be risk of the cleartext password being
compromised from backups, etc.

I fully agree that it's a serious risk that, today, what's stored in the
database is directly usable for logging into the running system (or any
system where the user has the same username and password).

> Aside from code complexity, the user security concern with a "multiple
> verifier per role" approach is that the DBAs would never remember to
> completely disable md5auth and would capture md5 hashes either in flight
> or from backups.  This approach can be used to capture an md5hash from a
> non-critical database which is poorly secured, and then re-use it
> against an important database.

The 'in flight' case is at least a bit less of an issue, as we don't
ship the password verifier directly over the network.  The backup
concern is certainly corret though.  I fully agree that we need a way to
make sure users don't end up having the old password verifiers around
longer than necessary.

That was the imputus for my earlier suggestion that in a release or two,
we actively make pg_upgrade error (or perhaps warn first, then error,
across two releases) if any of the old verifiers exist.

> Now, the counter-argument to this is that a DBA is just as likely to
> rememeber to remove md5 verifiers as she is to remember to remove roles
> with md5auth.

Indeed, that's a serious concern also.

The other concern with a single password verifier is that we're locking
ourselves into a one-verifier-per-role solution when most of the serious
solutions in use today (Active Directory, Kerberos, certificate based
systems) allow for more than one.

> Regardless of the approach we take, encouraging users to migrate is
> going to be more of a matter of documentation, publicity, and
> administrative tools than one of multiple verifiers vs. multiple roles.
>  That is, giving DBAs the ability to see and log who's using what kind
> of verifier, and what account has what verifier(s) available, will make
> more of a difference.

Fully agreed.
Thanks!
    Stephen

On Tue, Aug 18, 2015 at 10:06 AM, Stephen Frost <sfrost@snowman.net> wrote:
> That was the imputus for my earlier suggestion that in a release or two,
> we actively make pg_upgrade error (or perhaps warn first, then error,
> across two releases) if any of the old verifiers exist.

I think there's basically no chance of that being acceptable.  The
time at which it's possible to get rid of MD5 is going to vary widely
between installations.  People who are using only libpq or
libpq-derived connectors will be able to get rid of it almost
immediately, if they want, though some won't.  People who are using
obscure connectors that are poorly maintained may not even have a
version that supports SCRAM for 5 years.  Think about how long it took
us to roll out the standard_conforming_strings changes, and there were
still people who got bitten.

> The other concern with a single password verifier is that we're locking
> ourselves into a one-verifier-per-role solution when most of the serious
> solutions in use today (Active Directory, Kerberos, certificate based
> systems) allow for more than one.

So what?  If you want to delegate authentication to AD or Kerberos, we
already support that.  That's not a reason to invent the same
functionality inside the server.  If you've got a tangible plan, other
than SCRAM, that would require us to support multiple verifiers, then
please say what it is.  If not, the mere fact that some other people
support it is not a reason why we should.  In fact, we generally have
taken the approach that needs which are already handled adequately by
other tools to do not need to also be handled inside the database.
That's not a perfect approach and we always argue about it around the
edges, but generally, I think it's served us pretty well.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Robert,

* Robert Haas (robertmhaas@gmail.com) wrote:
> On Tue, Aug 18, 2015 at 10:06 AM, Stephen Frost <sfrost@snowman.net> wrote:
> > That was the imputus for my earlier suggestion that in a release or two,
> > we actively make pg_upgrade error (or perhaps warn first, then error,
> > across two releases) if any of the old verifiers exist.
>
> I think there's basically no chance of that being acceptable.  The
> time at which it's possible to get rid of MD5 is going to vary widely
> between installations.  People who are using only libpq or
> libpq-derived connectors will be able to get rid of it almost
> immediately, if they want, though some won't.  People who are using
> obscure connectors that are poorly maintained may not even have a
> version that supports SCRAM for 5 years.  Think about how long it took
> us to roll out the standard_conforming_strings changes, and there were
> still people who got bitten.

I would expect there to be people who would run into pg_upgrade
complaining, that's why there would be the check.  That's actually a
much better situation than what happened around
standard_conforming_strings.  Further, users would be able to continue
with their existing version until they're ready to move or it goes out
of support, by which time, if their connector isn't updated, they should
be moving off of it also.  We hear about people running 8.4 and older
because of some application which was never maintained or updated, and
that sucks, but it doesn't prevent us from making the changes we need to
make to move the project forward for the users who properly manage their
systems and use supported connectors.

> > The other concern with a single password verifier is that we're locking
> > ourselves into a one-verifier-per-role solution when most of the serious
> > solutions in use today (Active Directory, Kerberos, certificate based
> > systems) allow for more than one.
>
> So what?  If you want to delegate authentication to AD or Kerberos, we
> already support that.  That's not a reason to invent the same
> functionality inside the server.  If you've got a tangible plan, other
> than SCRAM, that would require us to support multiple verifiers, then
> please say what it is.  If not, the mere fact that some other people
> support it is not a reason why we should.

SCRAM itself, as has been discussed, supports multiple password
verifiers- that's a specific case all by itself, and it's done
specifically to address the issue that one or another of the algorithms
used is compromised, or that a new algorithm becomes available which is
better.  AD and Kerberos support multiple password verifiers because of
this and that it allows you to migrate from one to the next without
having to do wholesale replacment across all systems involved.  I bring
them up as examples of the advances in password-based authentication
which we've missed and because they are what users expect from current
password-based authentication systems, not because we support them and
therefore should just push everyone to them.

That said, if we aren't going to move password-based authentication in
PG to beyond the stone age it's currently in, then perhaps we should
just drop it completely and force users to pick a solution which is well
written, uses standard protocols, supports multiple ciphers and hashes
(in case one or more become a problem) and is updated to keep up with
the field.  Obviously, that's a strawman and I appreciate that you are
not arguing to keep the status-quo, but I'm trying to make a point that
multiple password verifiers is part of modern authentication technology,
used by multiple standard and broadly installed solutions.

> In fact, we generally have
> taken the approach that needs which are already handled adequately by
> other tools to do not need to also be handled inside the database.
> That's not a perfect approach and we always argue about it around the
> edges, but generally, I think it's served us pretty well.

Unless we take my strawman literally, which I don't think we can, we
can't simply punt on pushing all password-based authentication out of
the server.  If we agree that we need to support password-based
authentication then we should be working to maintain it as well as we
do the WAL and other subsystems.

I fully agree that we should leverage existing systems, technologies,
and protocols where possible, to avoid having to add a lot of extra
code which we have to maintain.  I believe that this proposal already
meets that goal and we should move forward with full support for SCRAM.
That, in my view at least, means that we also need to provide for
multiple password verifiers.
Thanks!
    Stephen

On Tue, Aug 18, 2015 at 2:07 PM, Stephen Frost <sfrost@snowman.net> wrote:
> I would expect there to be people who would run into pg_upgrade
> complaining, that's why there would be the check.  That's actually a
> much better situation than what happened around
> standard_conforming_strings.  Further, users would be able to continue
> with their existing version until they're ready to move or it goes out
> of support, by which time, if their connector isn't updated, they should
> be moving off of it also.  We hear about people running 8.4 and older
> because of some application which was never maintained or updated, and
> that sucks, but it doesn't prevent us from making the changes we need to
> make to move the project forward for the users who properly manage their
> systems and use supported connectors.

Sorry, that's a completely bogus argument.  We do not "need" to
prevent people from upgrading if they haven't moved off of MD5
authentication; that's just an arbitrary - and IMHO extremely
user-hostile - policy decision.  We have a legitimate need, to move
the project forward, to introduce a better system for password
authentication.  Ripping out the old one is not a real need.  I'm sure
at some point it will seem like antiquated cruft that nobody uses any
more, but that will not be in "a year or two" or anything close to
that.

> SCRAM itself, as has been discussed, supports multiple password
> verifiers- that's a specific case all by itself, and it's done
> specifically to address the issue that one or another of the algorithms
> used is compromised, or that a new algorithm becomes available which is
> better.  AD and Kerberos support multiple password verifiers because of
> this and that it allows you to migrate from one to the next without
> having to do wholesale replacment across all systems involved.  I bring
> them up as examples of the advances in password-based authentication
> which we've missed and because they are what users expect from current
> password-based authentication systems, not because we support them and
> therefore should just push everyone to them.

OK, that's an interesting argument.  If SCRAM supports multiple
password verifiers, and we support SCRAM, then I guess we should
probably do that, too.  I still don't like it all that much, though.
I think it's absolutely inevitable that people are going to end up
with an account with 3 or more different passwords that can all be
used to log into it, and that won't be good.  How do other systems
avoid this pitfall?

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

* Robert Haas (robertmhaas@gmail.com) wrote:
> On Tue, Aug 18, 2015 at 2:07 PM, Stephen Frost <sfrost@snowman.net> wrote:
> > SCRAM itself, as has been discussed, supports multiple password
> > verifiers- that's a specific case all by itself, and it's done
> > specifically to address the issue that one or another of the algorithms
> > used is compromised, or that a new algorithm becomes available which is
> > better.  AD and Kerberos support multiple password verifiers because of
> > this and that it allows you to migrate from one to the next without
> > having to do wholesale replacment across all systems involved.  I bring
> > them up as examples of the advances in password-based authentication
> > which we've missed and because they are what users expect from current
> > password-based authentication systems, not because we support them and
> > therefore should just push everyone to them.
>
> OK, that's an interesting argument.  If SCRAM supports multiple
> password verifiers, and we support SCRAM, then I guess we should
> probably do that, too.  I still don't like it all that much, though.
> I think it's absolutely inevitable that people are going to end up
> with an account with 3 or more different passwords that can all be
> used to log into it, and that won't be good.  How do other systems
> avoid this pitfall?

They provide:

a) ability to configure which algorithms are used at change-password
 MIT Kerberos, kdc.conf: supported_enctypes and                 kadmin: enctype-salttype

b) ability to configure which algorithms are allowed to be used at all
 MIT Kerberos, libdefaults: default_tgs_enctypes, default_tkt_enctypes, and permitted_enctypes, along with
allow_weak_cryptowhich removes all those considered 'weak' from the other sets if set to false (the default). 

c) password aging (to get users to change their password regularly, so  you know after, say, 90 days of the change to
theset configured in  'a' that all users will have either expired passwords or have been  updated to the latest set). 

Note that we don't need quite so many options, these were done over time
for Kerberos and also address the different types of tickets involved in
that system.

Today, they discourage explicitly setting default_tgs_enctypes and
default_tkt_enctypes.  We really just need to allow configuration of
which should be stored at change-password time and the set of allowed
types.  I wouldn't want those to be the same because you'd want to
remove a compromised type as soon as possible from the "change-password"
set while still allowing those users to connect, to get them to change
their password using self-service (doing it all through the
adminstrators of the system would be very painful in large
organizations).

We also don't strictly need password aging, but it certainly helps.  We
definitely do need to provide administrators with easy interfaces
through functions and tools to manage the password verifiers used.
Thanks!
    Stephen

On Tue, Aug 18, 2015 at 7:19 PM, Robert Haas <robertmhaas@gmail.com> wrote:

> Sorry, that's a completely bogus argument.  We do not "need" to
> prevent people from upgrading if they haven't moved off of MD5
> authentication; that's just an arbitrary - and IMHO extremely
> user-hostile - policy decision.  We have a legitimate need, to move
> the project forward, to introduce a better system for password
> authentication.  Ripping out the old one is not a real need.  I'm sure
> at some point it will seem like antiquated cruft that nobody uses any
> more, but that will not be in "a year or two" or anything close to
> that.

I would imagine a GUC like enable_legacy_authentication=true which we
would just start defaulting to false after a few years and perhaps
consider removing five years after that. pg_upgrade could check if
there are any users which require it to be set to true and warn users
that they must enable it but should check the documentation so they
understand the impact.

> OK, that's an interesting argument.  If SCRAM supports multiple
> password verifiers, and we support SCRAM, then I guess we should
> probably do that, too.  I still don't like it all that much, though.
> I think it's absolutely inevitable that people are going to end up
> with an account with 3 or more different passwords that can all be
> used to log into it, and that won't be good.  How do other systems
> avoid this pitfall?

Fwiw having multiple passwords would make automated credential
rotations *so* much easier. Heroku has a really baroque solution to
this problem in Postgres involving creating new child roles and
swapping them around. My team in Google wasted many man hours dealing
with fallout from the quarterly password rotations.

(I wish we could just drop the account management and authentication
system entirely and dump the whole work on a system designed for this
particular problem. It's a hard problem that's far outside our core
features and Postgres is never going to be a good system for anyone
let alone everyone when there are many different types of users.)

-- 
greg

On Wed, Aug 19, 2015 at 5:30 AM, Greg Stark <stark@mit.edu> wrote:
> On Tue, Aug 18, 2015 at 7:19 PM, Robert Haas <robertmhaas@gmail.com> wrote:
>
>> Sorry, that's a completely bogus argument.  We do not "need" to
>> prevent people from upgrading if they haven't moved off of MD5
>> authentication; that's just an arbitrary - and IMHO extremely
>> user-hostile - policy decision.  We have a legitimate need, to move
>> the project forward, to introduce a better system for password
>> authentication.  Ripping out the old one is not a real need.  I'm sure
>> at some point it will seem like antiquated cruft that nobody uses any
>> more, but that will not be in "a year or two" or anything close to
>> that.
>
> I would imagine a GUC like enable_legacy_authentication=true which we
> would just start defaulting to false after a few years and perhaps
> consider removing five years after that. pg_upgrade could check if
> there are any users which require it to be set to true and warn users
> that they must enable it but should check the documentation so they
> understand the impact.

Yep, that's one of the ideas mentioned upstread. This parameter should
actually be filled with a list of verifiers that we consider
out-of-support, deprecated, well things that users should be warned
about. One solution may be to log in warnings when parsing pg_hba.conf
should a deprecated method be used.

>> OK, that's an interesting argument.  If SCRAM supports multiple
>> password verifiers, and we support SCRAM, then I guess we should
>> probably do that, too.  I still don't like it all that much, though.
>> I think it's absolutely inevitable that people are going to end up
>> with an account with 3 or more different passwords that can all be
>> used to log into it, and that won't be good.  How do other systems
>> avoid this pitfall?
>
> Fwiw having multiple passwords would make automated credential
> rotations *so* much easier. Heroku has a really baroque solution to
> this problem in Postgres involving creating new child roles and
> swapping them around. My team in Google wasted many man hours dealing
> with fallout from the quarterly password rotations.
>
> (I wish we could just drop the account management and authentication
> system entirely and dump the whole work on a system designed for this
> particular problem. It's a hard problem that's far outside our core
> features and Postgres is never going to be a good system for anyone
> let alone everyone when there are many different types of users.)

This makes me think that we may need actually a finer grammar than
what the patch is proposing:
ADD PASSWORD VERIFIERS (method = 'value' [, ...] ) [ OPTIONS
(validUntil = 'timestamp') ]
DROP PASSWORD VERIFIERS (method [, ...])
PASSWORD VERIFIERS (method = 'value' [, ...]) [OPTIONS validUntil =
'timestamp'] -- replace all the existing ones

Reaching to the following catalog for pg_auth_verifiers:
Oid roleoid;
char method;
text value;
timestamp valueUntil;

I am not sure if we would want to be able to have multiple verifier
values for the same method, but feel free to comment.
Regards,
-- 
Michael

On Tue, Aug 18, 2015 at 09:30:39PM +0100, Greg Stark wrote:
> > OK, that's an interesting argument.  If SCRAM supports multiple
> > password verifiers, and we support SCRAM, then I guess we should
> > probably do that, too.  I still don't like it all that much, though.
> > I think it's absolutely inevitable that people are going to end up
> > with an account with 3 or more different passwords that can all be
> > used to log into it, and that won't be good.  How do other systems
> > avoid this pitfall?
> 
> Fwiw having multiple passwords would make automated credential
> rotations *so* much easier. Heroku has a really baroque solution to
> this problem in Postgres involving creating new child roles and
> swapping them around. My team in Google wasted many man hours dealing
> with fallout from the quarterly password rotations.

Coming in late, but can you explain how multiple passwords allow for
easier automated credential rotation?  If you have five applications
with stored passwords, I imagine you can't change them all at once, so
with multiples you could change it on one, then go to the others and
change it there, and finally, remove the old password.  Is that the
process?  I am not realizing that without multiple plasswords, this is a
hard problem.

--  Bruce Momjian  <bruce@momjian.us>        http://momjian.us EnterpriseDB
http://enterprisedb.com
 + Everyone has their own god. +

Bruce,

* Bruce Momjian (bruce@momjian.us) wrote:
> On Tue, Aug 18, 2015 at 09:30:39PM +0100, Greg Stark wrote:
> > > OK, that's an interesting argument.  If SCRAM supports multiple
> > > password verifiers, and we support SCRAM, then I guess we should
> > > probably do that, too.  I still don't like it all that much, though.
> > > I think it's absolutely inevitable that people are going to end up
> > > with an account with 3 or more different passwords that can all be
> > > used to log into it, and that won't be good.  How do other systems
> > > avoid this pitfall?
> >
> > Fwiw having multiple passwords would make automated credential
> > rotations *so* much easier. Heroku has a really baroque solution to
> > this problem in Postgres involving creating new child roles and
> > swapping them around. My team in Google wasted many man hours dealing
> > with fallout from the quarterly password rotations.
>
> Coming in late, but can you explain how multiple passwords allow for
> easier automated credential rotation?  If you have five applications
> with stored passwords, I imagine you can't change them all at once, so
> with multiples you could change it on one, then go to the others and
> change it there, and finally, remove the old password.  Is that the
> process?  I am not realizing that without multiple plasswords, this is a
> hard problem.

That's exactly the process if multiple passwords can be used.  If
there's only one account and one password supported then you have to
change all the systems all at once and that certainly can be a hard
problem.

One way to deal with this is to have a bunch of different accounts, but
that's certainly not simple either and can get quite painful.

Thanks!

Stephen

On Fri, Sep  4, 2015 at 04:51:33PM -0400, Stephen Frost wrote:
> > Coming in late, but can you explain how multiple passwords allow for
> > easier automated credential rotation?  If you have five applications
> > with stored passwords, I imagine you can't change them all at once, so
> > with multiples you could change it on one, then go to the others and
> > change it there, and finally, remove the old password.  Is that the
> > process?  I am not realizing that without multiple plasswords, this is a
> > hard problem.
> 
> That's exactly the process if multiple passwords can be used.  If
> there's only one account and one password supported then you have to
> change all the systems all at once and that certainly can be a hard
> problem.
> 
> One way to deal with this is to have a bunch of different accounts, but
> that's certainly not simple either and can get quite painful.

OK, for me, if we can explain the benefit for users, it seems worth
doing just to allow that.

--  Bruce Momjian  <bruce@momjian.us>        http://momjian.us EnterpriseDB
http://enterprisedb.com
 + Everyone has their own god. +

On Sat, Sep 5, 2015 at 9:31 AM, Bruce Momjian wrote:
> On Fri, Sep  4, 2015 at 04:51:33PM -0400, Stephen Frost wrote:
>> > Coming in late, but can you explain how multiple passwords allow for
>> > easier automated credential rotation?  If you have five applications
>> > with stored passwords, I imagine you can't change them all at once, so
>> > with multiples you could change it on one, then go to the others and
>> > change it there, and finally, remove the old password.  Is that the
>> > process?  I am not realizing that without multiple plasswords, this is a
>> > hard problem.
>>
>> That's exactly the process if multiple passwords can be used.  If
>> there's only one account and one password supported then you have to
>> change all the systems all at once and that certainly can be a hard
>> problem.
>>
>> One way to deal with this is to have a bunch of different accounts, but
>> that's certainly not simple either and can get quite painful.
>
> OK, for me, if we can explain the benefit for users, it seems worth
> doing just to allow that.

Reviving an old thread for a patch still registered in this commit
fest to make the arguing move on.

Supporting multiple verifiers for a single role has IMO clear advantages:
- help transition to new protocols and decommission of old protocols
without the need to create alternative roles in the backend when
switching from one or the other.
- move on to a more complex password aging system. The patch currently
submitted allows only one verifier per type and per role so this is
not a complete system. Still, the new catalog table pg_auth_verifiers
could be later easily extended based on other aging properties that we
consider adapted to reach this goal.

There are clear concerns about protocol aging and how to facilitate
the life of users to switch to a new system. Hence, I think that the
patch could include the following:
- A compatibility GUC allowed_password_verifiers defaulting to a list
of verifier protocols that we think are safe. This would be added in
the patch adding infrastructure for multiple verifiers, with default
to md5. In the patch adding SCRAM, the value of this parameter is
changed to md5,scram. If a user create a password verifier with a
protocol not listed in this parameter we return to him a WARNING.
ERROR may be too much but opinions are welcome.
- A superuser cleanup function for pg_auth_verifiers aimed at being
used by pg_upgrade to do needed cleanup of this catalog based on the
previous GUC to remove outdated verifiers. Optionally, we could have
an argument for a list of protocols to clean up.
Using both things we could leverage the upgrade experience and
transition between systems. Say even if at some point we decide to
decommission SCRAM we could reuse the same infrastructure to move on
to a new major version.

Thoughts?
-- 
Michael

On Mon, Nov 16, 2015 at 10:53 PM, Michael Paquier wrote:
> Reviving an old thread for a patch still registered in this commit
> fest to make the arguing move on.
>
> Supporting multiple verifiers for a single role has IMO clear advantages:
> - help transition to new protocols and decommission of old protocols
> without the need to create alternative roles in the backend when
> switching from one or the other.
> - move on to a more complex password aging system. The patch currently
> submitted allows only one verifier per type and per role so this is
> not a complete system. Still, the new catalog table pg_auth_verifiers
> could be later easily extended based on other aging properties that we
> consider adapted to reach this goal.
>
> There are clear concerns about protocol aging and how to facilitate
> the life of users to switch to a new system. Hence, I think that the
> patch could include the following:
> - A compatibility GUC allowed_password_verifiers defaulting to a list
> of verifier protocols that we think are safe. This would be added in
> the patch adding infrastructure for multiple verifiers, with default
> to md5. In the patch adding SCRAM, the value of this parameter is
> changed to md5,scram. If a user create a password verifier with a
> protocol not listed in this parameter we return to him a WARNING.
> ERROR may be too much but opinions are welcome.
> - A superuser cleanup function for pg_auth_verifiers aimed at being
> used by pg_upgrade to do needed cleanup of this catalog based on the
> previous GUC to remove outdated verifiers. Optionally, we could have
> an argument for a list of protocols to clean up.
> Using both things we could leverage the upgrade experience and
> transition between systems. Say even if at some point we decide to
> decommission SCRAM we could reuse the same infrastructure to move on
> to a new major version.
>
> Thoughts?

I am going to mark this patch as returned with feedback because of a
visible lack of interest. It would be nice if I could get some
feedback about the suggestions above to help move on for (why not) a
patch aiming for January's CF.
-- 
Michael

Hi Michael and Heikki,

On 11/16/15 8:53 AM, Michael Paquier wrote:
> On Sat, Sep 5, 2015 at 9:31 AM, Bruce Momjian wrote:
>> On Fri, Sep  4, 2015 at 04:51:33PM -0400, Stephen Frost wrote:
>>>> Coming in late, but can you explain how multiple passwords allow for
>>>> easier automated credential rotation?  If you have five applications
>>>> with stored passwords, I imagine you can't change them all at once, so
>>>> with multiples you could change it on one, then go to the others and
>>>> change it there, and finally, remove the old password.  Is that the
>>>> process?  I am not realizing that without multiple plasswords, this is a
>>>> hard problem.
>>> That's exactly the process if multiple passwords can be used.  If
>>> there's only one account and one password supported then you have to
>>> change all the systems all at once and that certainly can be a hard
>>> problem.
>>>
>>> One way to deal with this is to have a bunch of different accounts, but
>>> that's certainly not simple either and can get quite painful.
>> OK, for me, if we can explain the benefit for users, it seems worth
>> doing just to allow that.
> Reviving an old thread for a patch still registered in this commit
> fest to make the arguing move on.

I was wondering if this patch was going to be submitted for the 2016-03 CF?

If so I am interesting in testing/reviewing or doing any other work that
would be helpful.

> Supporting multiple verifiers for a single role has IMO clear advantages:
> - help transition to new protocols and decommission of old protocols
> without the need to create alternative roles in the backend when
> switching from one or the other.

Agreed.

> - move on to a more complex password aging system. The patch currently
> submitted allows only one verifier per type and per role so this is
> not a complete system. Still, the new catalog table pg_auth_verifiers
> could be later easily extended based on other aging properties that we
> consider adapted to reach this goal.

Yes, with some careful design the pg_auth_verifiers table could support
multiple passwords using a single verifier in the future.  I think the
major changes would be to ALTER ROLE WITH PASSWORD so coming up with an
extensible syntax is important.

In addition, I would prefer to maintain the current structure of the
pg_authid table and use the rolpassword and rolvaliduntil columns to
store only the md5 verifier which will also be stored in
pg_auth_verifiers.  This would provide a smoother migration path with
the idea that rolpassword and rolvaliduntil will be removed from
pg_authid in a future version of Postgres.

> There are clear concerns about protocol aging and how to facilitate
> the life of users to switch to a new system. Hence, I think that the
> patch could include the following:
> - A compatibility GUC allowed_password_verifiers defaulting to a list
> of verifier protocols that we think are safe. This would be added in
> the patch adding infrastructure for multiple verifiers, with default
> to md5. In the patch adding SCRAM, the value of this parameter is
> changed to md5,scram. If a user create a password verifier with a
> protocol not listed in this parameter we return to him a WARNING.
> ERROR may be too much but opinions are welcome.

It seems like an error would be better here.

> - A superuser cleanup function for pg_auth_verifiers aimed at being
> used by pg_upgrade to do needed cleanup of this catalog based on the
> previous GUC to remove outdated verifiers. Optionally, we could have
> an argument for a list of protocols to clean up.
> Using both things we could leverage the upgrade experience and
> transition between systems. Say even if at some point we decide to
> decommission SCRAM we could reuse the same infrastructure to move on
> to a new major version.

Yes - and although the eventual migration process may not need to be
worked out it its entirety we should have a very good idea of what it's
going to look like as that will inform some of the decisions that need
to be made now.

Please let me know if there's anything I can do to expedite this patch.

--
-David
david@pgmasters.net

On Sat, Feb 13, 2016 at 3:05 AM, David Steele <david@pgmasters.net> wrote:
> On 11/16/15 8:53 AM, Michael Paquier wrote:
>> On Sat, Sep 5, 2015 at 9:31 AM, Bruce Momjian wrote:
>>> On Fri, Sep  4, 2015 at 04:51:33PM -0400, Stephen Frost wrote:
>>>>> Coming in late, but can you explain how multiple passwords allow for
>>>>> easier automated credential rotation?  If you have five applications
>>>>> with stored passwords, I imagine you can't change them all at once, so
>>>>> with multiples you could change it on one, then go to the others and
>>>>> change it there, and finally, remove the old password.  Is that the
>>>>> process?  I am not realizing that without multiple plasswords, this is a
>>>>> hard problem.
>>>> That's exactly the process if multiple passwords can be used.  If
>>>> there's only one account and one password supported then you have to
>>>> change all the systems all at once and that certainly can be a hard
>>>> problem.
>>>>
>>>> One way to deal with this is to have a bunch of different accounts, but
>>>> that's certainly not simple either and can get quite painful.
>>> OK, for me, if we can explain the benefit for users, it seems worth
>>> doing just to allow that.
>> Reviving an old thread for a patch still registered in this commit
>> fest to make the arguing move on.
>
> I was wondering if this patch was going to be submitted for the 2016-03 CF?

For 9.6 I am afraid this is too late, per the rule that there cannot
be huge patches for the last CF of a development cycle. But I have
plans for this set of features afterwards with the first CF of 9.7 and
I was planning to talk about it at PgCon's unconference if I can get
there to gather some feedback. There is still cruel need for it on my
side..

> If so I am interesting in testing/reviewing or doing any other work that
> would be helpful.

Thanks.

> In addition, I would prefer to maintain the current structure of the
> pg_authid table and use the rolpassword and rolvaliduntil columns to
> store only the md5 verifier which will also be stored in
> pg_auth_verifiers.  This would provide a smoother migration path with
> the idea that rolpassword and rolvaliduntil will be removed from
> pg_authid in a future version of Postgres.

Actually, I am of the opinion that both rolpassword and rolvaliduntil
should be directly part of pg_auth_verifiers. Being able to handle
multiple verifiers for the same protocol is a feature that is being
asked for with a given password handling policy (was pinged again
about that in Moscow last week). Rolling in new verifiers needs now
extra roles to be created.

>> There are clear concerns about protocol aging and how to facilitate
>> the life of users to switch to a new system. Hence, I think that the
>> patch could include the following:
>> - A compatibility GUC allowed_password_verifiers defaulting to a list
>> of verifier protocols that we think are safe. This would be added in
>> the patch adding infrastructure for multiple verifiers, with default
>> to md5. In the patch adding SCRAM, the value of this parameter is
>> changed to md5,scram. If a user create a password verifier with a
>> protocol not listed in this parameter we return to him a WARNING.
>> ERROR may be too much but opinions are welcome.
>
> It seems like an error would be better here.

Noted.

>> - A superuser cleanup function for pg_auth_verifiers aimed at being
>> used by pg_upgrade to do needed cleanup of this catalog based on the
>> previous GUC to remove outdated verifiers. Optionally, we could have
>> an argument for a list of protocols to clean up.
>> Using both things we could leverage the upgrade experience and
>> transition between systems. Say even if at some point we decide to
>> decommission SCRAM we could reuse the same infrastructure to move on
>> to a new major version.
>
> Yes - and although the eventual migration process may not need to be
> worked out it its entirety we should have a very good idea of what it's
> going to look like as that will inform some of the decisions that need
> to be made now.

Thinking about that again a combination of a GUC and an interface
dedicated to pg_upgrade sounds the saner way of going here.

> Please let me know if there's anything I can do to expedite this patch.

I am planning to work on a new patch following the ideas I have sent
upthread, after the last CF of 9.6 is wrapped up. This thread is high
on my priority list.
-- 
Michael

Michael,

* Michael Paquier (michael.paquier@gmail.com) wrote:
> On Sat, Feb 13, 2016 at 3:05 AM, David Steele <david@pgmasters.net> wrote:
> > On 11/16/15 8:53 AM, Michael Paquier wrote:
> >> On Sat, Sep 5, 2015 at 9:31 AM, Bruce Momjian wrote:
> >>> On Fri, Sep  4, 2015 at 04:51:33PM -0400, Stephen Frost wrote:
> >>>>> Coming in late, but can you explain how multiple passwords allow for
> >>>>> easier automated credential rotation?  If you have five applications
> >>>>> with stored passwords, I imagine you can't change them all at once, so
> >>>>> with multiples you could change it on one, then go to the others and
> >>>>> change it there, and finally, remove the old password.  Is that the
> >>>>> process?  I am not realizing that without multiple plasswords, this is a
> >>>>> hard problem.
> >>>> That's exactly the process if multiple passwords can be used.  If
> >>>> there's only one account and one password supported then you have to
> >>>> change all the systems all at once and that certainly can be a hard
> >>>> problem.
> >>>>
> >>>> One way to deal with this is to have a bunch of different accounts, but
> >>>> that's certainly not simple either and can get quite painful.
> >>> OK, for me, if we can explain the benefit for users, it seems worth
> >>> doing just to allow that.
> >> Reviving an old thread for a patch still registered in this commit
> >> fest to make the arguing move on.
> >
> > I was wondering if this patch was going to be submitted for the 2016-03 CF?
>
> For 9.6 I am afraid this is too late, per the rule that there cannot
> be huge patches for the last CF of a development cycle. But I have
> plans for this set of features afterwards with the first CF of 9.7 and
> I was planning to talk about it at PgCon's unconference if I can get
> there to gather some feedback. There is still cruel need for it on my
> side..

There's a lot of good reason to get SCRAM added as a protocol,
considering our current password-based implementation is rather..
lacking.  Regarding the specific comment about the timing, that rule is
specifically to prevent large patches from landing in the last CF
without any prior discussion or review, as I recall, so I'm not sure it
really applies here as there's been quite a bit of discussion and review
already.

That said, per various discussions, we'd really want a more-or-less
fully formed patch to land prior to the last CF, for this to have any
chance.  Perhaps that means it's not going to happen, which would be
unfortunate, but it's not beyond the possible, in my view, at least.

> > In addition, I would prefer to maintain the current structure of the
> > pg_authid table and use the rolpassword and rolvaliduntil columns to
> > store only the md5 verifier which will also be stored in
> > pg_auth_verifiers.  This would provide a smoother migration path with
> > the idea that rolpassword and rolvaliduntil will be removed from
> > pg_authid in a future version of Postgres.
>
> Actually, I am of the opinion that both rolpassword and rolvaliduntil
> should be directly part of pg_auth_verifiers. Being able to handle
> multiple verifiers for the same protocol is a feature that is being
> asked for with a given password handling policy (was pinged again
> about that in Moscow last week). Rolling in new verifiers needs now
> extra roles to be created.

I'm on Michael's side here.  I don't believe it makes sense to try and
maintain the exact structure of pg_authid.  We are certainly happy to
whack around the other catalogs, and I'm unimpressed with my prior
efforts to provide backwards-compatible catalog (see pg_user, et al) for
just a few releases- we seem unable to get rid of them now, even though
we should have years ago, really.  Indeed, I'd be just as happy if we
got rid of them during this work..

Thanks!

Stephen

On Mon, Feb 15, 2016 at 9:17 AM, Stephen Frost <sfrost@snowman.net> wrote:
> Michael,
>
> * Michael Paquier (michael.paquier@gmail.com) wrote:
>> On Sat, Feb 13, 2016 at 3:05 AM, David Steele <david@pgmasters.net> wrote:
>> > On 11/16/15 8:53 AM, Michael Paquier wrote:
>> >> On Sat, Sep 5, 2015 at 9:31 AM, Bruce Momjian wrote:
>> >>> On Fri, Sep  4, 2015 at 04:51:33PM -0400, Stephen Frost wrote:
>> >>>>> Coming in late, but can you explain how multiple passwords allow for
>> >>>>> easier automated credential rotation?  If you have five applications
>> >>>>> with stored passwords, I imagine you can't change them all at once, so
>> >>>>> with multiples you could change it on one, then go to the others and
>> >>>>> change it there, and finally, remove the old password.  Is that the
>> >>>>> process?  I am not realizing that without multiple plasswords, this is a
>> >>>>> hard problem.
>> >>>> That's exactly the process if multiple passwords can be used.  If
>> >>>> there's only one account and one password supported then you have to
>> >>>> change all the systems all at once and that certainly can be a hard
>> >>>> problem.
>> >>>>
>> >>>> One way to deal with this is to have a bunch of different accounts, but
>> >>>> that's certainly not simple either and can get quite painful.
>> >>> OK, for me, if we can explain the benefit for users, it seems worth
>> >>> doing just to allow that.
>> >> Reviving an old thread for a patch still registered in this commit
>> >> fest to make the arguing move on.
>> >
>> > I was wondering if this patch was going to be submitted for the 2016-03 CF?
>>
>> For 9.6 I am afraid this is too late, per the rule that there cannot
>> be huge patches for the last CF of a development cycle. But I have
>> plans for this set of features afterwards with the first CF of 9.7 and
>> I was planning to talk about it at PgCon's unconference if I can get
>> there to gather some feedback. There is still cruel need for it on my
>> side..
>
> There's a lot of good reason to get SCRAM added as a protocol,
> considering our current password-based implementation is rather..
> lacking.  Regarding the specific comment about the timing, that rule is
> specifically to prevent large patches from landing in the last CF
> without any prior discussion or review, as I recall, so I'm not sure it
> really applies here as there's been quite a bit of discussion and review
> already.

Honestly I don't know what to answer to that.

> That said, per various discussions, we'd really want a more-or-less
> fully formed patch to land prior to the last CF, for this to have any
> chance.  Perhaps that means it's not going to happen, which would be
> unfortunate, but it's not beyond the possible, in my view, at least.

Well, I could send a rebased patch with the new things proposed
upthread, and with things split in as many patches as I can get out,
roughly:
1) One patch for the catalog split
2) One for the GUC param controlling recommended protocols
3) One for the pg_upgrade function cleaning up automatically the
entries of unsupported protocols
4) SCRAM on top of the rest, which is at more or less 75% something
that Heikki produced.

>> > In addition, I would prefer to maintain the current structure of the
>> > pg_authid table and use the rolpassword and rolvaliduntil columns to
>> > store only the md5 verifier which will also be stored in
>> > pg_auth_verifiers.  This would provide a smoother migration path with
>> > the idea that rolpassword and rolvaliduntil will be removed from
>> > pg_authid in a future version of Postgres.
>>
>> Actually, I am of the opinion that both rolpassword and rolvaliduntil
>> should be directly part of pg_auth_verifiers. Being able to handle
>> multiple verifiers for the same protocol is a feature that is being
>> asked for with a given password handling policy (was pinged again
>> about that in Moscow last week). Rolling in new verifiers needs now
>> extra roles to be created.
>
> I'm on Michael's side here.  I don't believe it makes sense to try and
> maintain the exact structure of pg_authid.  We are certainly happy to
> whack around the other catalogs, and I'm unimpressed with my prior
> efforts to provide backwards-compatible catalog (see pg_user, et al) for
> just a few releases- we seem unable to get rid of them now, even though
> we should have years ago, really.  Indeed, I'd be just as happy if we
> got rid of them during this work..

We'd need as well to switch pg_shadow to have an array of elements
made of protocol:identifier instead of a single password field. There
can be only one valid identifier per protocol, user and valid_until
for a single point in time, and I can't believe that we should
recommend only one authentication protocol per single major version of
Postgres.
-- 
Michael

Michael,

* Michael Paquier (michael.paquier@gmail.com) wrote:
> On Mon, Feb 15, 2016 at 9:17 AM, Stephen Frost <sfrost@snowman.net> wrote:
> > That said, per various discussions, we'd really want a more-or-less
> > fully formed patch to land prior to the last CF, for this to have any
> > chance.  Perhaps that means it's not going to happen, which would be
> > unfortunate, but it's not beyond the possible, in my view, at least.
>
> Well, I could send a rebased patch with the new things proposed
> upthread, and with things split in as many patches as I can get out,
> roughly:
> 1) One patch for the catalog split
> 2) One for the GUC param controlling recommended protocols
> 3) One for the pg_upgrade function cleaning up automatically the
> entries of unsupported protocols
> 4) SCRAM on top of the rest, which is at more or less 75% something
> that Heikki produced.

That sounds like a pretty reasonable split, to me at least.

> >> > In addition, I would prefer to maintain the current structure of the
> >> > pg_authid table and use the rolpassword and rolvaliduntil columns to
> >> > store only the md5 verifier which will also be stored in
> >> > pg_auth_verifiers.  This would provide a smoother migration path with
> >> > the idea that rolpassword and rolvaliduntil will be removed from
> >> > pg_authid in a future version of Postgres.
> >>
> >> Actually, I am of the opinion that both rolpassword and rolvaliduntil
> >> should be directly part of pg_auth_verifiers. Being able to handle
> >> multiple verifiers for the same protocol is a feature that is being
> >> asked for with a given password handling policy (was pinged again
> >> about that in Moscow last week). Rolling in new verifiers needs now
> >> extra roles to be created.
> >
> > I'm on Michael's side here.  I don't believe it makes sense to try and
> > maintain the exact structure of pg_authid.  We are certainly happy to
> > whack around the other catalogs, and I'm unimpressed with my prior
> > efforts to provide backwards-compatible catalog (see pg_user, et al) for
> > just a few releases- we seem unable to get rid of them now, even though
> > we should have years ago, really.  Indeed, I'd be just as happy if we
> > got rid of them during this work..
>
> We'd need as well to switch pg_shadow to have an array of elements
> made of protocol:identifier instead of a single password field. There
> can be only one valid identifier per protocol, user and valid_until
> for a single point in time, and I can't believe that we should
> recommend only one authentication protocol per single major version of
> Postgres.

Ugh, that sounds pretty grotty to me.

Applications which consider these fields will need to be updated, one
way or the other, and I'd much rather they be updated to work with
reasonable structures rather than something we've hacked together in
some faint hope that it'd be useful.  An array in pg_shadow for a field
which used to be a text field does *not* sound like a simpler solution
to me, and I'd rather simply do away with those views entirely, or at
least nuke the fields which are at issue, than try to come up with
something between wholesale change and no change that ends up being
worse than both.

Thanks!

Stephen

On Mon, Feb 15, 2016 at 9:56 AM, Stephen Frost <sfrost@snowman.net> wrote:
> * Michael Paquier (michael.paquier@gmail.com) wrote:
>> We'd need as well to switch pg_shadow to have an array of elements
>> made of protocol:identifier instead of a single password field. There
>> can be only one valid identifier per protocol, user and valid_until
>> for a single point in time, and I can't believe that we should
>> recommend only one authentication protocol per single major version of
>> Postgres.
>
> Ugh, that sounds pretty grotty to me.
>
> Applications which consider these fields will need to be updated, one
> way or the other, and I'd much rather they be updated to work with
> reasonable structures rather than something we've hacked together in
> some faint hope that it'd be useful.  An array in pg_shadow for a field
> which used to be a text field does *not* sound like a simpler solution
> to me, and I'd rather simply do away with those views entirely, or at
> least nuke the fields which are at issue, than try to come up with
> something between wholesale change and no change that ends up being
> worse than both.

It seems to me that applications are going to need a refresh anyway...
Among the other possibilities I can foresee:
- Add a column "protocol" to pg_shadow and produce one row per
protocol, so one user will be listed for all the protocol it has. Any
application could then filter out things with an additional WHERE
clause.
- Nuke passwd from pg_shadow and have a new view pg_shadow_verifiers
made of the user OID, the protocol and the verifier. This maps quite
well with pg_auth_verifiers.
- Give up and nuke pg_shadow, which is here for compatibility down to
8.1, and add a protocol column to pg_user, or even better create a new
view pg_user_verifiers that has all the data of all the protocols. If
we care a lot about backward-compatibility, pg_user could as well map
with pg_auth_verifiers with the md5 protocol.
I would go with the last one.
-- 
Michael

Michael,

* Michael Paquier (michael.paquier@gmail.com) wrote:
> It seems to me that applications are going to need a refresh anyway...

Indeed.

> Among the other possibilities I can foresee:
> - Add a column "protocol" to pg_shadow and produce one row per
> protocol, so one user will be listed for all the protocol it has. Any
> application could then filter out things with an additional WHERE
> clause.
> - Nuke passwd from pg_shadow and have a new view pg_shadow_verifiers
> made of the user OID, the protocol and the verifier. This maps quite
> well with pg_auth_verifiers.
> - Give up and nuke pg_shadow, which is here for compatibility down to
> 8.1, and add a protocol column to pg_user, or even better create a new
> view pg_user_verifiers that has all the data of all the protocols. If
> we care a lot about backward-compatibility, pg_user could as well map
> with pg_auth_verifiers with the md5 protocol.
> I would go with the last one.

I would start by pointing out that pg_user currently uses pg_shadow..

Why do we need pg_shadow or pg_user or related views at all..?
Applications will need to be updated, we might as well simply nuke them
and expect applications to use the new catalogs.  Perhaps there is a
useful view or two which we can provide over the new catalogs, but I'd
rather consider how to create brand new, useful, views over the new
catalogs than consider any kind of way to provides backwards
compatible-ish views.

Thanks!

Stephen

Stephen Frost <sfrost@snowman.net> writes:
> Why do we need pg_shadow or pg_user or related views at all..?

A lot of code looks at those just to get usernames.  I am not in favor of
breaking such stuff without need.

How about we just say that the password in these old views always reads
out as '********' even when there is a password, and we invent new views
that carry real auth information?  (Hopefully in an extensible way.)
        regards, tom lane

On Mon, Feb 15, 2016 at 10:23 AM, Stephen Frost <sfrost@snowman.net> wrote:
> I would start by pointing out that pg_user currently uses pg_shadow..
> Why do we need pg_shadow or pg_user or related views at all..?

pg_user/pg_shadow have the advantage to be world-readable and mask
password values.
-- 
Michael

* Tom Lane (tgl@sss.pgh.pa.us) wrote:
> Stephen Frost <sfrost@snowman.net> writes:
> > Why do we need pg_shadow or pg_user or related views at all..?
>
> A lot of code looks at those just to get usernames.  I am not in favor of
> breaking such stuff without need.

Alright.

> How about we just say that the password in these old views always reads
> out as '********' even when there is a password, and we invent new views
> that carry real auth information?  (Hopefully in an extensible way.)

I'd be alright with that approach, I'd just rather that any clients
which actually want to read the password field be updated to look at the
extensible and sensible base catalogs, and not some hacked up array that
we shoved into that field.

Thanks!

Stephen

* Michael Paquier (michael.paquier@gmail.com) wrote:
> On Mon, Feb 15, 2016 at 10:23 AM, Stephen Frost <sfrost@snowman.net> wrote:
> > I would start by pointing out that pg_user currently uses pg_shadow..
> > Why do we need pg_shadow or pg_user or related views at all..?
>
> pg_user/pg_shadow have the advantage to be world-readable and mask
> password values.

New views would have that same advantage, should we implement them that
way.  Tom's approach is also workable though, where we make the existing
views have a reducaed charter, which is mainly around providing user
lists and simply not include any info about password verifiers or the
like.

Thanks!

Stephen

Stephen Frost <sfrost@snowman.net> writes:
> * Tom Lane (tgl@sss.pgh.pa.us) wrote:
>> How about we just say that the password in these old views always reads
>> out as '********' even when there is a password, and we invent new views
>> that carry real auth information?  (Hopefully in an extensible way.)

> I'd be alright with that approach, I'd just rather that any clients
> which actually want to read the password field be updated to look at the
> extensible and sensible base catalogs, and not some hacked up array that
> we shoved into that field.

Yeah, I'm good with that.  I just don't think the breakage needs to extend
to clients that aren't trying to read auth-related information.

BTW, if we haven't learned this lesson by now: I'm pretty sure that every
single one of these views is an attempt to emulate what *used* to be the
real base catalog, in some previous release.  Maybe we should stop
expecting clients to read the real catalog, ever, in favor of a sanitized
view?  Although I don't know exactly what that would lead to in terms of
what we'd expose that's different from what the base catalog is.  But it's
worth thinking about whether there is a way to avoid having this same
discussion again in five or ten years.
        regards, tom lane

On Mon, Feb 15, 2016 at 10:51 AM, Stephen Frost <sfrost@snowman.net> wrote:
> * Tom Lane (tgl@sss.pgh.pa.us) wrote:
>> Stephen Frost <sfrost@snowman.net> writes:
>> > Why do we need pg_shadow or pg_user or related views at all..?
>>
>> A lot of code looks at those just to get usernames.  I am not in favor of
>> breaking such stuff without need.
>
> Alright.
>
>> How about we just say that the password in these old views always reads
>> out as '********' even when there is a password, and we invent new views
>> that carry real auth information?  (Hopefully in an extensible way.)
>
> I'd be alright with that approach, I'd just rather that any clients
> which actually want to read the password field be updated to look at the
> extensible and sensible base catalogs, and not some hacked up array that
> we shoved into that field.

Well, then let's mask it, and just have pg_auth_verifiers. Another
possible problem that I can see with this patch is what do we do with
valid_until? The last set of patches sent did not switch this field to
be per-verifier settable. I would consider a saner approach to keep
things simple and still do that. Allowing multiple verifiers per
protocol is a problem, and having a solution for it would be nice.
Should this be prioritized before having more protocols like SCRAM?

FWIW, browsing through pgbouncer, it has a look at pg_shadow for
user's password to build a basic configuration file.

(My mistake, while pg_user is world-readable, that's not the case of pg_shadow).
-- 
Michael

On Mon, Feb 15, 2016 at 11:05 AM, Michael Paquier
<michael.paquier@gmail.com> wrote:
> On Mon, Feb 15, 2016 at 10:51 AM, Stephen Frost <sfrost@snowman.net> wrote:
>> * Tom Lane (tgl@sss.pgh.pa.us) wrote:
>>> Stephen Frost <sfrost@snowman.net> writes:
>>> > Why do we need pg_shadow or pg_user or related views at all..?
>>>
>>> A lot of code looks at those just to get usernames.  I am not in favor of
>>> breaking such stuff without need.
>>
>> Alright.
>>
>>> How about we just say that the password in these old views always reads
>>> out as '********' even when there is a password, and we invent new views
>>> that carry real auth information?  (Hopefully in an extensible way.)
>>
>> I'd be alright with that approach, I'd just rather that any clients
>> which actually want to read the password field be updated to look at the
>> extensible and sensible base catalogs, and not some hacked up array that
>> we shoved into that field.
>
> Well, then let's mask it, and just have pg_auth_verifiers. Another
> possible problem that I can see with this patch is what do we do with
> valid_until? The last set of patches sent did not switch this field to
> be per-verifier settable. I would consider a saner approach to keep
> things simple and still do that. Allowing multiple verifiers per
> protocol is a problem, and having a solution for it would be nice.
> Should this be prioritized before having more protocols like SCRAM?
>
> FWIW, browsing through pgbouncer, it has a look at pg_shadow for
> user's password to build a basic configuration file.
>
> (My mistake, while pg_user is world-readable, that's not the case of pg_shadow).

FWIW, I am going to create a new thread once I am done with the set of
patches I have in mind for the upcoming CF (yes there will be
refreshed patches), because this thread has moved on a bit larger
discussion than SCRAM itself, summarizing what is more or less the
conclusion of this thread, explaining what the patches are doing, what
they are not doing, what could be done afterwards, etc, etc. I'll keep
a clear scope regarding what I am aiming at.
-- 
Michael