Re: scram and \password

On Tue, Apr 25, 2017 at 8:56 PM, Heikki Linnakangas <hlinnaka(at)iki(dot)fi> wrote:

> On 04/22/2017 01:20 AM, Michael Paquier wrote:
>
>> On Sat, Apr 22, 2017 at 5:04 AM, Heikki Linnakangas <hlinnaka(at)iki(dot)fi>
>> wrote:
>>
>>> I'll continue reviewing the rest of the patch on Monday, but one glaring
>>> issue is that we cannot add an argument to the existing libpq
>>> PQencryptPassword() function, because that's part of the public API. It
>>> would break all applications that use PQencryptPassword().
>>>
>>> What we need to do is to add a new function. What should we call that? We
>>> don't have a tradition of using "Ex" or such suffix to mark extended
>>> versions of existing functions. PQencryptPasswordWithMethod(user, pass,
>>> method) ?
>>>
>>
>> Do we really want to add a new function or have a hard failure? Any
>> application calling PQencryptPassword may trap itself silently if the
>> server uses scram as hba key or if the default is switched to that,
>> from this point of view extending the function makes sense as well.
>>
>
> Yeah, there is that. But we simply cannot change the signature of an
> existing function. It would not only produce compile-time errors when
> building old applications, which would arguably be a good thing, but it
> would also cause old binaries to segfault when dynamically linked with the
> new libpq.
>
> I think it's clear that we should have a new function that takes the
> algorithm as argument. But there are open decisions on what the old
> PQencryptPassword() function should do, and also what the new function
> should do by default, if you don't specify an algorithm:
>
> A) Have PQencryptPassword() return an md5 hash.
>
> B) Have PQencryptPassword() return a SCRAM verifier
>
> C) Have PQencryptPassword() return a SCRAM verifier if connected to a v10
> server, and an md5 hash otherwise. This is tricky, because
> PQencryptPassword doesn't take a PGconn argument. It could behave like
> PQescapeString() does, and choose md5/scram depending on the server version
> of the last connection that was established.
>
> For the new function, it's probably best to pass a PGconn argument. That
> way we can use the connection to determine the default, and it seems to be
> a good idea for future-proofing too. And an extra "options" argument might
> be good, while we're at it, to e.g. specify the number of iterations for
> SCRAM. So all in all, I propose the documentation for these functions to be
> (I chose option C from above for this):
>
> ----
> char *
> PQencryptPasswordConn(PGconn *conn,
> const char *passwd,
> const char *user,
> const char *method,
> const char *options)
>
I was just thinking:
- Do we need to provide the method here?
We have connection object itself, it can decide from the type of
connection, which method to be used.

-- Thanks, Ashesh

>
> [this paragraph is the same as current PQencryptPassword()]
> This function is intended to be used by client applications that wish to
> send commands like ALTER ROLE joe PASSWORD 'pwd'. It is good practice to
> not send the original cleartext password in such a command, because it
> might be exposed in command logs, activity displays and so on. Instead, use
> this function to convert the password to encrypted form before it is sent.
> [end of unchanged part]
>
> This function may execute internal queries to the server to determine
> appropriate defaults, using the given connection object. The call can
> therefore fail if the connection is busy executing another query, or the
> current transaction is aborted.
>
> The return value is a string allocated by malloc, or NULL if out of memory
> or other error. On error, a suitable message is stored in the 'conn'
> object. The caller can assume the string doesn't contain any special
> characters that would require escaping. Use PQfreemem to free the result
> when done with it.
>
> The function arguments are:
>
> conn
> Connection object for the database where the password is to be changed.
>
> passwd
> The new password
>
> user
> Name of the role whose password is to be changed
>
> method
> Name of the password encryption method to use. Currently supported
> methods are "md5" or "scram-sha-256". If method is NULL, the default for
> the current database is used. [i.e. this looks at password_encryption]
>
> options
> Options specific to the encryption method, or NULL to use the defaults.
> (This argument is for future expansion, there are currently no options, and
> you should always pass NULL.)
>
>
> char *
> PQencryptPassword(const char *passwd, const char *user)
>
> PQencryptPassword is an older, deprecated version of PQencryptPasswodConn.
> The difference is that PQencryptPassword does not require a connection
> object. The encryption method will be chosen depending on the server
> version of the last established connection, and built-in default options.
>
> ----
>
> Thoughts? Unless someone has better ideas or objections, I'll go implement
> that.
>
> - Heikki
>
>
>
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers(at)postgresql(dot)org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>