On Wed, Dec 14, 2016 at 9:51 AM, Heikki Linnakangas <hlinn...@iki.fi> wrote:
> On 12/09/2016 10:19 AM, Michael Paquier wrote: > >> On Fri, Dec 9, 2016 at 5:11 PM, Heikki Linnakangas <hlinn...@iki.fi> >> wrote: >> >>> Couple of things I should write down before I forget: >>> >>> 1. It's a bit cumbersome that the scram verifiers stored in >>> pg_authid.rolpassword don't have any clear indication that they're scram >>> verifiers. MD5 hashes are readily identifiable by the "md5" prefix. I >>> think >>> we should use a "scram-sha-256:" for scram verifiers. >>> >> >> scram-sha-256 would make the most sense to me. >> >> Actually, I think it'd be awfully nice to also prefix plaintext passwords >>> with "plain:", but I'm not sure it's worth breaking the compatibility, if >>> there are tools out there that peek into rolpassword. Thoughts? >>> >> >> pgbouncer is the only thing coming up in mind. It looks at pg_shadow >> for password values. pg_dump'ing data from pre-10 instances will also >> need to adapt. I see tricky the compatibility with the exiting CREATE >> USER PASSWORD command though, so I am wondering if that's worth the >> complication. >> >> 2. It's currently not possible to use the plaintext "password" >>> authentication method, for a user that has a SCRAM verifier in >>> rolpassword. >>> That seems like an oversight. We can't do MD5 authentication with a SCRAM >>> verifier, but "password" we could. >>> >> >> Yeah, that should be possible... >> > > The tip of the work branch can now do SCRAM authentication, when a user > has a plaintext password in pg_authid.rolpassword. The reverse doesn't > work, however: you cannot do plain "password" authentication, when the user > has a SCRAM verifier in pg_authid.rolpassword. It gets worse: plain > "password" authentication doesn't check if the string stored in > pg_authid.rolpassword is a SCRAM authenticator, and treats it as a > plaintext password, so you can do this: > > PGPASSWORD="scram-sha-256:mDBuqO1mEekieg==:4096:17dc259499c1 > a184c26ee5b19715173d9354195f510b4d3af8be585acb39ae33:d3d7131 > 49c6becbbe56bae259aafe4e95b79ab7e3b50f2fbd850ea7d7b7c114f" psql > postgres -h localhost -U scram_user > > I think we're going to have a more bugs like this, if we don't start to > explicitly label plaintext passwords as such. > > So, let's add "plain:" prefix to plaintext passwords, in > pg_authid.rolpassword. With that, these would be valid values in > pg_authid.rolpassword: > > plain:foo > md55a962ce7a24371a10e85627a484cac28 > scram-sha-256:mDBuqO1mEekieg==:4096:17dc259499c1a184c26ee5b1 > 9715173d9354195f510b4d3af8be585acb39ae33:d3d713149c6becbbe56 > bae259aafe4e95b79ab7e3b50f2fbd850ea7d7b7c114f > I would so like to just drop support for plain passwords completely :) But there's a backwards compatibility issue to think about of course. But -- is there any actual usecase for them anymore? If not, another option could be to just specifically check that it's *not* "md5<something>" or "scram-<something>:<something>". That would invalidate plaintext passwords that have those texts in them of course, but what's the likelyhood of that in reality? Though I guess that might at least in theory be more bug-prone, so going with a "plain:" prefix seems like a good idea as well. > But anything that doesn't begin with "plain:", "md5", or "scram-sha-256:" > would be invalid. You shouldn't have invalid values in the column, but if > you do, all the authentication mechanisms would reject it. > > It would be nice to also change the format of MD5 passwords to have a > colon, as in "md5:<hash>", but that's probably not worth breaking > compatibility for. Almost no-one stores passwords in plaintext, so changing > the format of that wouldn't affect many people, but there might well be > tools out there that peek into MD5 hashes. There are definitely tools that do that, so +1 on leaving that alone. -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/