From: Ashesh Vashi <ashesh.vashi@enterprisedb.com>
To: Heikki Linnakangas <hlinnaka@iki.fi>
Cc: Michael Paquier <michael.paquier@gmail.com>
Cc: Noah Misch <noah@leadboat.com>
Cc: Robert Haas <robertmhaas@gmail.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Jeff Janes <jeff.janes@gmail.com>
Cc: Joe Conway <mail@joeconway.com>
Cc: pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: Re: scram and \password
Date: Thu, 27 Apr 2017 09:55:05 +0530
Message-ID: <CAG7mmowsAmmSDpaxspUuuAC83v_ZjoA-RV_5YLAa79hpDLA9og@mail.gmail.com> (raw)
In-Reply-To: <c32a2be9-f25d-30e8-6d48-64e1801a910f@iki.fi>
References: <a8b0d5cb-35d4-d686-3fae-f743659ae346@iki.fi>
<CAB7nPqSjSmYK7H3S8A98giR6Jb=jKKTOMMruK1Kxw5Lfxc+Baw@mail.gmail.com>
<fe2c096e-7b06-3e8e-0804-109def6b5d98@iki.fi>
<CAB7nPqQ3QiPMgGv5OCz1wsocx7PHdZg5ursQOpyOb96AZuvmGA@mail.gmail.com>
<5478ab75-404f-9490-9355-dc1365ce3d7f@iki.fi>
<CAB7nPqTLzbHp5XmREoqiL31uF8VRz4jM1VhDRMZLOJuVg8gZoQ@mail.gmail.com>
<dead641a-1fc8-0fd7-89d6-b818ac61b31a@iki.fi>
<CAB7nPqQmJ5t=rd67paE7DWxuSi0C4X=5_pnzZchH7qFkBHy==A@mail.gmail.com>
<CA+TgmoYk+xfnvYC4xtQD4qFdu7xKn6bRLC-baH5MDdtyCSxg6w@mail.gmail.com>
<f56a2116-a466-ad6c-03d5-413294bfca9f@iki.fi>
<20170410035323.GA2846583@tornado.leadboat.com>
<CAB7nPqSbsCBCxy8-DtwzRxYgTnbGUtY4uFEkLQhG=R=uo=g8Fw@mail.gmail.com>
<66e776c9-e02c-3683-28df-339f7f64ea66@iki.fi>
<CAB7nPqSWP9J9PzeC7=-WjuuaL57_d+EawmzJNVsM+m7Y4gRRBA@mail.gmail.com>
<c32a2be9-f25d-30e8-6d48-64e1801a910f@iki.fi>
List-Unsubscribe: <mailto:majordomo@postgresql.org?body=unsub%20pgsql-hackers>
On Tue, Apr 25, 2017 at 8:56 PM, Heikki Linnakangas <hlinnaka@iki.fi> wrote:
> On 04/22/2017 01:20 AM, Michael Paquier wrote:
>
>> On Sat, Apr 22, 2017 at 5:04 AM, Heikki Linnakangas <hlinnaka@iki.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@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: ashesh.vashi@enterprisedb.com, hlinnaka@iki.fi, michael.paquier@gmail.com, noah@leadboat.com, robertmhaas@gmail.com, tgl@sss.pgh.pa.us, jeff.janes@gmail.com, mail@joeconway.com
Subject: Re: scram and \password
In-Reply-To: <CAG7mmowsAmmSDpaxspUuuAC83v_ZjoA-RV_5YLAa79hpDLA9og@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox