On 12/09/2016 01:10 PM, Michael Paquier wrote:
On Fri, Dec 09, 2016 at 11:51:45AM +0200, Heikki Linnakangas wrote:
On 12/09/2016 05:58 AM, Michael Paquier wrote:

One thing is: when do we look up at pg_authid? After receiving the
first message from client or before beginning the exchange? As the
first message from client has the user name, it would make sense to do
the lookup after receiving it, but from PG prospective it would just
make sense to use the data already present in the startup packet. The
current patch does the latter. What do you think?

While hacking on this, I came up with the attached refactoring, against
current master. I think it makes the current code more readable, anyway, and
it provides a get_role_password() function that SCRAM can use, to look up
the stored password. (This is essentially the same refactoring that was
included in the SCRAM patch set, that introduced the get_role_details()

Barring objections, I'll go ahead and commit this first.

Ok, committed.

-       shadow_pass = TextDatumGetCString(datum);
+       *shadow_pass = TextDatumGetCString(datum);

        datum = SysCacheGetAttr(AUTHNAME, roleTup,
Anum_pg_authid_rolvaliduntil, &isnull);
@@ -83,100 +83,146 @@ md5_crypt_verify(const char *role, char *client_pass,
                *logdetail = psprintf(_("User \"%s\" has an empty password."),
+               *shadow_pass = NULL;
                return STATUS_ERROR;    /* empty password */

Here the password is allocated by text_to_cstring(), that's only 1 byte
but it should be free()'d.

Fixed. Thanks, good catch! It doesn't matter in practice as we'll disconnect shortly afterwards anyway, but given that the callers pfree() other things on error, let's be tidy.

- Heikki

Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:

Reply via email to