On Tue, Dec 20, 2016 at 9:23 PM, Heikki Linnakangas <hlinn...@iki.fi> wrote: > On 12/16/2016 03:31 AM, Michael Paquier wrote: > Actually, it does still perform that check. There's a new function, > plain_crypt_verify, that passwordcheck uses now. plain_crypt_verify() is > intended to work with any future hash formats we might introduce in the > future (including SCRAM), so that passwordcheck doesn't need to know about > all the hash formats.
Bah. I have misread the first version of the patch, and it is indeed keeping the username checks. Now that things don't crash that behaves as expected: =# load 'passwordcheck'; LOAD =# alter role mpaquier password 'mpaquier'; ERROR: 22023: password must not contain user name LOCATION: check_password, passwordcheck.c:101 =# alter role mpaquier password 'md58349d3a1bc8f4f7399b1ff9dea493b15'; ERROR: 22023: password must not contain user name LOCATION: check_password, passwordcheck.c:82 With the patch: >> + case PASSWORD_TYPE_PLAINTEXT: >> + shadow_pass = &shadow_pass[strlen("plain:")]; >> + break; >> It would be a good idea to have a generic routine able to get the plain >> password value. In short I think that we should reduce the amount of >> locations where "plain:" prefix is hardcoded. > > There is such a function included in the patch, get_plain_password(char > *shadow_pass), actually. Contrib/passwordcheck uses it. I figured that in > crypt.c itself, it's OK to do the above directly, but get_plain_password() > is intended to be used elsewhere. The idea would be to have the function not return an allocated string, just a position to it. That would be useful in plain_crypt_verify() for example, for a total of 4 places, including get_plain_password() where the new string allocation is done. Well, it's not like this prefix "plain:" would change anyway in the future nor that it is going to spread much. > Thanks for having a look! Attached is a new version, with that bug fixed. I have been able more advanced testing without the crash and things seem to work properly. The attached set of tests is also able to pass for all the combinations of hba configurations and password formats. And looking at the code I don't have more comments. -- Michael
Description: Perl program
-- Sent via pgsql-hackers mailing list (email@example.com) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers