On 04/25/2017 06:26 PM, Heikki Linnakangas wrote:
Thoughts? Unless someone has better ideas or objections, I'll go implement that.
This is what I came up with in the end. Some highlights and differences vs the plan I posted earlier:
* If algorithm is not given explicitly, PQencryptPasswordConn() queries "SHOW password_encryption", and uses that. That is documented, and it is also documented that it will *not* issue queries, and hence will not block, if the algorithm is given explicitly. That's an important property for some applications. If you want the default behavior without blocking, query "SHOW password_encryption" yourself, and pass the result as the 'algorithm'.
* In the previous draft, I envisioned an algorithm-specific 'options' argument. I left it out, on second thoughts (more on that below).
* The old PQencryptPassword() function is unchanged, and always uses md5. Per public opinion.
We talked about the alternative where PQencryptPasswordConn() would not look at password_encryption, but would always use the strongest possible algorithm supported by the server. That would avoid querying the server. But then I started thinking how this would work, if we make the number of iterations in the SCRAM verifier configurable in the future. The client would not know the desired number of iterations based only on the server version, it would need to query the server, and we would be back to square one. We could add an "options" argument to PQencryptPasswordConn() that the application could use to pass that information, but documenting how to fetch that information, if you don't want PQencryptPasswordConn() to block, gets tedious, and puts a lot of burden to applications. That is why I left out the "options" argument, after all.
I'm now thinking that if we add password hashing options like the iteration count in the future, they will be tacked on to password_encryption. For example, "password_encryption='scram-sha-256, iterations=10000". That way, "password_encryption" will always contain enough information to construct password verifiers.
What will happen to existing applications using PQencryptPasswordConn() if a new password_encryption algorithm (or options) is added in the future? With this scheme, the application doesn't need to know what algorithms exist. An application can pass algorithm=NULL, to use the server default, or do "show password_encryption" and pass that, for the non-blocking behavior. If you use an older libpq against a new server, so that libpq doesn't know about the algorithm used by the server, you get an error.
For reviewer convenience, I put up the patched docs at http://hlinnaka.iki.fi/temp/scram-wip-docs/libpq-misc.html#libpq-pqencryptpasswordconn.
Thoughts? Am I missing anything?As an alternative, I considered making password_encryption GUC_REPORT, so that libpq would always know it without having to issue a query. But it feels wrong to send that option to the client on every connection, when it's only needed in the rare case that you use PQencryptPasswordConn(). And if we added new settings like the iteration count in the future, those would need to be GUC_REPORT too.
- Heikki
0001-Move-scram_build_verifier-to-src-common.patch
Description: application/download
0002-Add-PQencryptPasswordConn-function-to-libpq.patch
Description: application/download
0003-Use-new-PQencryptPasswordConn-function-in-psql-and-c.patch
Description: application/download
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
