From: Heikki Linnakangas <hlinnaka@iki.fi>
To: Michael Paquier <michael.paquier@gmail.com>
Cc: Magnus Hagander <magnus@hagander.net>
Cc: Robert Haas <robertmhaas@gmail.com>
Cc: Andres Freund <andres@anarazel.de>
Cc: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
Cc: David Steele <david@pgmasters.net>
Cc: Tom Lane <tgl@sss.pgh.pa.us>
Cc: David Fetter <david@fetter.org>
Cc: Alvaro Herrera <alvherre@2ndquadrant.com>
Cc: Julian Markwort <julian.markwort@uni-muenster.de>
Cc: Stephen Frost <sfrost@snowman.net>
Cc: PostgreSQL mailing lists <pgsql-hackers@postgresql.org>
Cc: Valery Popov <v.popov@postgrespro.ru>
Subject: Re: pg_authid.rolpassword format (was Re: Password identifiers, protocol aging and SCRAM protocol)
Date: Thu, 15 Dec 2016 14:48:50 +0200
Message-ID: <a150e7c7-7d57-bf1c-7f73-cbbfedf99243@iki.fi> (raw)
In-Reply-To: <CAB7nPqR4RDjr1PYKm4D+aV4gxgQduq8GWb7SmEOnvoctnvVzsw@mail.gmail.com>
References: <947b9aff-8fdb-dbf5-a99c-0ffd4523a73f@2ndquadrant.com>
<CA+TgmoYydfT0ZwPaCMZannstRG207Ymf3s_aWTxkNJx9SBfm4Q@mail.gmail.com>
<CAB7nPqTAKNSBT6zzX8H0R8EwJ24Y8YsPmOqb3j38uzCoU_3+=A@mail.gmail.com>
<CAB7nPqSNMvJO50nJFoprf-wMMa281gJ-G6hfOr+krm8Zh8WLBg@mail.gmail.com>
<CAB7nPqRF=kVxqtC+Ei_t7eTrMti_0P+xUugSOd_7-rcLvREYzw@mail.gmail.com>
<e139fcbc-cfb2-3a64-93ac-afd5ba342960@iki.fi>
<CAB7nPqSqEwRgmVCS_OW3sJn6baymQ6MsO3kKMRvHLC0RZaKaYA@mail.gmail.com>
<21eecdb8-365b-8347-1ae2-5278d1fe798c@iki.fi>
<CAB7nPqT2UGWm0ukdomC+mG-co6nmKer62vuSGRxwtc+jS8uQPA@mail.gmail.com>
<CAB7nPqQtD7jOpYCsB4Ga=Cz+tgh4Ngac6NAV-WLs9zxaw0Uu7w@mail.gmail.com>
<762f4875-a87e-cf0b-c06b-b357294d8be1@iki.fi>
<CAB7nPqTi5jC_AbVMOGidrVy1z6t3v==vWFvdNMxnQEN-WsutKg@mail.gmail.com>
<2d07165c-1793-e243-a2a9-e45b624c7580@iki.fi>
<CABUevEyi7uw41nzyFRh2xUzB6uOh-QmavcwDMhW0jCTTe4KavQ@mail.gmail.com>
<9a7263eb-0980-2072-4424-440bb2513dc7@iki.fi>
<CAB7nPqR4RDjr1PYKm4D+aV4gxgQduq8GWb7SmEOnvoctnvVzsw@mail.gmail.com>
List-Unsubscribe: <mailto:majordomo@postgresql.org?body=unsub%20pgsql-hackers>
On 12/15/2016 03:00 AM, Michael Paquier wrote:
> On Wed, Dec 14, 2016 at 8:33 PM, Heikki Linnakangas <hlinnaka@iki.fi> wrote:
>> But, a password stored in plaintext works with either MD5 or SCRAM, or any
>> future authentication mechanism. So as soon as we have SCRAM authentication,
>> it becomes somewhat useful again.
>>
>> In a nutshell:
>>
>> auth / stored MD5 SCRAM plaintext
>> -----------------------------------------
>> password Y Y Y
>> md5 Y N Y
>> scram N Y Y
>>
>> If a password is stored in plaintext, it can be used with any authentication
>> mechanism. And the plaintext 'password' authentication mechanism works with
>> any kind of a stored password. But an MD5 hash cannot be used with SCRAM
>> authentication, or vice versa.
>
> So.. I have been thinking about this portion of the thread. And what I
> find the most scary is not the fact that we use plain passwords for
> SCRAM authentication, it is the fact that we would need to do a
> catalog lookup earlier in the connection workflow to decide what is
> the connection protocol to use depending on the username provided in
> the startup packet if the pg_hba.conf entry matching the user and
> database names uses "password".
I don't see why we would need to do a catalog lookup any earlier. With
"password" authentication, the server can simply request the client to
send its password. When it receives it, it performs the catalog lookup
to get pg_authid.rolpassword. If it's in plaintext, just compare it, if
it's an MD5 hash, hash the client's password and compare, and if it's a
SCRAM verifier, build a verifier with the same salt and iteration count
and compare.
> And, honestly, why do we actually need to have a support table that
> spread? SCRAM is designed to be secure, so it seems to me that it
> would on the contrary a bad idea to encourage the use of plain
> passwords if we actually think that they should never be used (they
> are actually useful for located, development instances, not production
> ones).
I agree we should not encourage bad password practices. But as long as
we support passwords to be stored in plaintext at all, it makes no sense
to not allow them to be used with SCRAM. The fact that you can use a
password stored in plaintext with both MD5 and SCRAM is literally the
only reason you would store a password in plaintext, so if we don't want
to allow that, we should disallow storing passwords in plaintext altogether.
> So what I would suggest would be to have a support table like
> that:
> auth / stored MD5 SCRAM plaintext
> -----------------------------------------
> password Y Y N
> md5 Y N Y
> scram N N Y
I was using 'Y' to indicate that the combination works, and 'N' to
indicate that it does not. Assuming you're using the same notation, the
above doesn't make any sense.
> So here is an idea for things to do now:
> 1) do not change the format of the existing passwords
> 2) do not change pg_authid
> 3) block access to instances if "password" or "md5" are used in
> pg_hba.conf if the user have a SCRAM verifier.
> 4) block access if "scram" is used and if user has a plain or md5 verifier.
> 5) Allow access if "scram" is used and if user has a SCRAM verifier.
> We had a similar discussion regarding verifier/password formats last
> year but that did not end well. It would be sad to fall back again
> into this discussion and get no result. If somebody wants to support
> access to SCRAM with plain password entries, why not. But that would
> gain a -1 from me regarding the earlier lookup of pg_authid needed to
> do the decision making on the protocol to use. And I think that we
> want SCRAM to be designed to be a maximum stable and secure.
The bottom line is that at the moment, when plaintext passwords are
stored as is, without any indicator that it's a plaintext password, it's
ambiguous whether a password is a SCRAM verifier, or if it's a plaintext
password that just happens to begin with the word "scram:". That is
completely unrelated to which combinations of stored passwords and
authentication mechanisms we actually support or allow to work.
The only way to distinguish, is to know about every verifier kind there
is, and check whether rolpassword looks valid as anything else than a
plaintext password. And we already got tripped by a bug-of-omission on
that once. If we add more verifier formats in the future, it's bound to
happen again. Let's nip that source of bugs in the bud. Attached is a
patch to implement what I have in mind.
Alternatively, you could argue that we should forbid storing passwords
in plaintext altogether. I'm OK with that, too, if that's what people
prefer. Then you cannot have a user that can log in with both MD5 and
SCRAM authentication, but it's certainly more secure, and it's easier to
document.
- Heikki
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackersAttachments:0001-Use-plain-prefix-for-plaintext-passwords-stored-in-p.patch (invalid/octet-stream, 22.5K) download
From 25670580136d6d0cc5f7b519be2176870bca71a1 Mon Sep 17 00:00:00 2001
From: Heikki Linnakangas <heikki.linnakangas@iki.fi>
Date: Thu, 15 Dec 2016 14:25:54 +0200
Subject: [PATCH 1/1] Use "plain:" prefix for plaintext passwords stored in
pg_authid.
So far, the rule has been that if pg_authid.rolpassword begins with "md5"
and has the right length, it's an MD5 hash, otherwise it's a plaintext
password. That kind of pattern matching gets more awkward when we add
new kinds of verifiers, like the upcoming SCRAM verifiers. To be able to
distinguish different kinds of password hashes, use a "plain:" prefix
for plaintext passwords. With that, every password stored in pg_authid
has either "plain:" or "md5" prefix, and future password schemes can
likewise use different prefixes.
Note that this doesn't change the format of MD5 hashed passwords, which
is what almost everyone uses.
This changes check_password_hook in an incompatible way. The
'password_type' argument is removed, and the hook is expected to call
get_password_type() on the password verifier to determine its type.
Moreover, when a plaintext password is passed to the hook, it will have
the "plain:" prefix. That is a more subtle change!
In the passing, remove the bogus notice from the documentation of CREATE
ROLE, claiming that older clients might not work with MD5 hashed passwords.
That was written when we still had "crypt" authentication, and it was
referring to the fact that an older client might support "crypt"
authentication but not "md5". But we haven't supported "crypt" for years.
(As soon as we add a new authentication mechanism that doesn't work with
MD5 hashes, we'll need a similar notice again, though!)
Discussion: https://www.postgresql.org/message-id/2d07165c-1793-e243-a2a9-e45b624c7580@iki.fi
---
contrib/passwordcheck/passwordcheck.c | 133 +++++++++++------------
doc/src/sgml/catalogs.sgml | 4 +-
doc/src/sgml/ref/create_role.sgml | 6 --
src/backend/commands/user.c | 66 ++++++------
src/backend/libpq/crypt.c | 192 +++++++++++++++++++++++++++-------
src/backend/utils/misc/guc.c | 1 +
src/include/commands/user.h | 13 +--
src/include/common/md5.h | 4 -
src/include/libpq/crypt.h | 20 +++-
9 files changed, 273 insertions(+), 166 deletions(-)
diff --git a/contrib/passwordcheck/passwordcheck.c b/contrib/passwordcheck/passwordcheck.cindex a0db89b..3d06964c 100644--- a/contrib/passwordcheck/passwordcheck.c+++ b/contrib/passwordcheck/passwordcheck.c@@ -21,7 +21,7 @@
#endif
#include "commands/user.h"
-#include "common/md5.h"+#include "libpq/crypt.h"
#include "fmgr.h"
PG_MODULE_MAGIC;
@@ -50,87 +50,78 @@ extern void _PG_init(void);
*/
static void
check_password(const char *username,
- const char *password,- int password_type,+ const char *shadow_pass,
Datum validuntil_time,
bool validuntil_null)
{
- int namelen = strlen(username);- int pwdlen = strlen(password);- char encrypted[MD5_PASSWD_LEN + 1];- int i;- bool pwd_has_letter,- pwd_has_nonletter;-- switch (password_type)+ if (get_password_type(shadow_pass) != PASSWORD_TYPE_PLAINTEXT)
{
- case PASSWORD_TYPE_MD5:-- /*- * Unfortunately we cannot perform exhaustive checks on encrypted- * passwords - we are restricted to guessing. (Alternatively, we- * could insist on the password being presented non-encrypted, but- * that has its own security disadvantages.)- *- * We only check for username = password.- */- if (!pg_md5_encrypt(username, username, namelen, encrypted))- elog(ERROR, "password encryption failed");- if (strcmp(password, encrypted) == 0)- ereport(ERROR,- (errcode(ERRCODE_INVALID_PARAMETER_VALUE),- errmsg("password must not contain user name")));- break;-- case PASSWORD_TYPE_PLAINTEXT:-+ /*+ * Unfortunately we cannot perform exhaustive checks on encrypted+ * passwords - we are restricted to guessing. (Alternatively, we+ * could insist on the password being presented non-encrypted, but+ * that has its own security disadvantages.)+ *+ * We only check for username = password.+ */+ char *logdetail;++ if (plain_crypt_verify(username, shadow_pass, username, &logdetail) == STATUS_OK)+ ereport(ERROR,+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),+ errmsg("password must not contain user name")));+ }+ else+ {+ /*+ * For unencrypted passwords we can perform better checks+ */+ char *password = get_plain_password(shadow_pass);+ int pwdlen = strlen(password);+ int i;+ bool pwd_has_letter,+ pwd_has_nonletter;++ /* enforce minimum length */+ if (pwdlen < MIN_PWD_LENGTH)+ ereport(ERROR,+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),+ errmsg("password is too short")));++ /* check if the password contains the username */+ if (strstr(password, username))+ ereport(ERROR,+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),+ errmsg("password must not contain user name")));++ /* check if the password contains both letters and non-letters */+ pwd_has_letter = false;+ pwd_has_nonletter = false;+ for (i = 0; i < pwdlen; i++)+ {
/*
- * For unencrypted passwords we can perform better checks+ * isalpha() does not work for multibyte encodings but let's+ * consider non-ASCII characters non-letters
*/
-- /* enforce minimum length */- if (pwdlen < MIN_PWD_LENGTH)- ereport(ERROR,- (errcode(ERRCODE_INVALID_PARAMETER_VALUE),- errmsg("password is too short")));-- /* check if the password contains the username */- if (strstr(password, username))- ereport(ERROR,- (errcode(ERRCODE_INVALID_PARAMETER_VALUE),- errmsg("password must not contain user name")));-- /* check if the password contains both letters and non-letters */- pwd_has_letter = false;- pwd_has_nonletter = false;- for (i = 0; i < pwdlen; i++)- {- /*- * isalpha() does not work for multibyte encodings but let's- * consider non-ASCII characters non-letters- */- if (isalpha((unsigned char) password[i]))- pwd_has_letter = true;- else- pwd_has_nonletter = true;- }- if (!pwd_has_letter || !pwd_has_nonletter)- ereport(ERROR,- (errcode(ERRCODE_INVALID_PARAMETER_VALUE),+ if (isalpha((unsigned char) password[i]))+ pwd_has_letter = true;+ else+ pwd_has_nonletter = true;+ }+ if (!pwd_has_letter || !pwd_has_nonletter)+ ereport(ERROR,+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("password must contain both letters and nonletters")));
#ifdef USE_CRACKLIB
- /* call cracklib to check password */- if (FascistCheck(password, CRACKLIB_DICTPATH))- ereport(ERROR,- (errcode(ERRCODE_INVALID_PARAMETER_VALUE),- errmsg("password is easily cracked")));+ /* call cracklib to check password */+ if (FascistCheck(password, CRACKLIB_DICTPATH))+ ereport(ERROR,+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),+ errmsg("password is easily cracked")));
#endif
- break;- default:- elog(ERROR, "unrecognized password type: %d", password_type);- break;+ pfree(password);
}
/* all checks passed, password is ok */
diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgmlindex 11c2019..abf98af 100644--- a/doc/src/sgml/catalogs.sgml+++ b/doc/src/sgml/catalogs.sgml@@ -1320,8 +1320,8 @@
will be of the user's password concatenated to their user name.
For example, if user <literal>joe</> has password <literal>xyzzy</>,
<productname>PostgreSQL</> will store the md5 hash of
- <literal>xyzzyjoe</>. A password that does not follow that- format is assumed to be unencrypted.+ <literal>xyzzyjoe</>. If a password is stored unencrypted, the+ column will begin with the string <literal>plain:</literal>.
</entry>
</row>
diff --git a/doc/src/sgml/ref/create_role.sgml b/doc/src/sgml/ref/create_role.sgmlindex 38cd4c8..a3b8ed9 100644--- a/doc/src/sgml/ref/create_role.sgml+++ b/doc/src/sgml/ref/create_role.sgml@@ -235,12 +235,6 @@ CREATE ROLE <replaceable class="PARAMETER">name</replaceable> [ [ WITH ] <replac
password string). This allows reloading of encrypted
passwords during dump/restore.
</para>
-- <para>- Note that older clients might lack support for the MD5- authentication mechanism that is needed to work with passwords- that are stored encrypted.- </para>
</listitem>
</varlistentry>
diff --git a/src/backend/commands/user.c b/src/backend/commands/user.cindex adc6b99..e9061f8 100644--- a/src/backend/commands/user.c+++ b/src/backend/commands/user.c@@ -29,7 +29,7 @@
#include "commands/dbcommands.h"
#include "commands/seclabel.h"
#include "commands/user.h"
-#include "common/md5.h"+#include "libpq/crypt.h"
#include "miscadmin.h"
#include "storage/lmgr.h"
#include "utils/acl.h"
@@ -81,7 +81,6 @@ CreateRole(ParseState *pstate, CreateRoleStmt *stmt)
ListCell *option;
char *password = NULL; /* user password */
int password_type = Password_encryption;
- char encrypted_password[MD5_PASSWD_LEN + 1];
bool issuper = false; /* Make the user a superuser? */
bool inherit = true; /* Auto inherit privileges? */
bool createrole = false; /* Can this user create roles? */
@@ -368,11 +367,23 @@ CreateRole(ParseState *pstate, CreateRoleStmt *stmt)
* Call the password checking hook if there is one defined
*/
if (check_password_hook && password)
+ {+ char *plain_password;++ /*+ * We prefer to pass a plaintext verifier to the password hook, if+ * possible, even if it's eventually stored in a hashed format, so+ * that the hook can perform more extensive tests on it.+ */+ plain_password = encode_password(PASSWORD_TYPE_PLAINTEXT, stmt->role,+ password);+
(*check_password_hook) (stmt->role,
- password,- isMD5(password) ? PASSWORD_TYPE_MD5 : PASSWORD_TYPE_PLAINTEXT,+ plain_password,
validUntil_datum,
validUntil_null);
+ pfree(plain_password);+ }
/*
* Build a tuple to insert
@@ -393,17 +404,10 @@ CreateRole(ParseState *pstate, CreateRoleStmt *stmt)
if (password)
{
- if (password_type == PASSWORD_TYPE_PLAINTEXT || isMD5(password))- new_record[Anum_pg_authid_rolpassword - 1] =- CStringGetTextDatum(password);- else- {- if (!pg_md5_encrypt(password, stmt->role, strlen(stmt->role),- encrypted_password))- elog(ERROR, "password encryption failed");- new_record[Anum_pg_authid_rolpassword - 1] =- CStringGetTextDatum(encrypted_password);- }+ /* Encode the password to the requested format. */+ password = encode_password(password_type, stmt->role, password);+ new_record[Anum_pg_authid_rolpassword - 1] =+ CStringGetTextDatum(password);
}
else
new_record_nulls[Anum_pg_authid_rolpassword - 1] = true;
@@ -506,7 +510,6 @@ AlterRole(AlterRoleStmt *stmt)
char *rolename = NULL;
char *password = NULL; /* user password */
int password_type = Password_encryption;
- char encrypted_password[MD5_PASSWD_LEN + 1];
int issuper = -1; /* Make the user a superuser? */
int inherit = -1; /* Auto inherit privileges? */
int createrole = -1; /* Can this user create roles? */
@@ -743,11 +746,23 @@ AlterRole(AlterRoleStmt *stmt)
* Call the password checking hook if there is one defined
*/
if (check_password_hook && password)
+ {+ char *plain_password;++ /*+ * We prefer to pass a plaintext verifier to the password hook, if+ * possible, even if it's eventually stored in a hashed format, so+ * that the hook can perform more extensive tests on it.+ */+ plain_password = encode_password(PASSWORD_TYPE_PLAINTEXT, rolename,+ password);+
(*check_password_hook) (rolename,
- password,- isMD5(password) ? PASSWORD_TYPE_MD5 : PASSWORD_TYPE_PLAINTEXT,+ plain_password,
validUntil_datum,
validUntil_null);
+ pfree(plain_password);+ }
/*
* Build an updated tuple, perusing the information just obtained
@@ -804,17 +819,8 @@ AlterRole(AlterRoleStmt *stmt)
/* password */
if (password)
{
- if (password_type == PASSWORD_TYPE_PLAINTEXT || isMD5(password))- new_record[Anum_pg_authid_rolpassword - 1] =- CStringGetTextDatum(password);- else- {- if (!pg_md5_encrypt(password, rolename, strlen(rolename),- encrypted_password))- elog(ERROR, "password encryption failed");- new_record[Anum_pg_authid_rolpassword - 1] =- CStringGetTextDatum(encrypted_password);- }+ /* Encode the password to the requested format. */+ password = encode_password(password_type, rolename, password);
new_record_repl[Anum_pg_authid_rolpassword - 1] = true;
}
@@ -1232,7 +1238,7 @@ RenameRole(const char *oldname, const char *newname)
datum = heap_getattr(oldtuple, Anum_pg_authid_rolpassword, dsc, &isnull);
- if (!isnull && isMD5(TextDatumGetCString(datum)))+ if (!isnull && get_password_type(TextDatumGetCString(datum)) == PASSWORD_TYPE_MD5)
{
/* MD5 uses the username as salt, so just clear it on a rename */
repl_repl[Anum_pg_authid_rolpassword - 1] = true;
diff --git a/src/backend/libpq/crypt.c b/src/backend/libpq/crypt.cindex 7e9124f..5df049f 100644--- a/src/backend/libpq/crypt.c+++ b/src/backend/libpq/crypt.c@@ -1,10 +1,8 @@
/*-------------------------------------------------------------------------
*
* crypt.c
- * Look into the password file and check the encrypted password with- * the one passed in from the frontend.- *- * Original coding by Todd A. Brandys+ * Functions for dealing with encrypted passwords stored in+ * pg_authid.rolpassword
*
* Portions Copyright (c) 1996-2016, PostgreSQL Global Development Group
* Portions Copyright (c) 1994, Regents of the University of California
@@ -106,6 +104,98 @@ get_role_password(const char *role, char **shadow_pass, char **logdetail)
}
/*
+ * What kind of a password verifier is 'shadow_pass'?+ */+PasswordType+get_password_type(const char *shadow_pass)+{+ if (strncmp(shadow_pass, "plain:", 6) == 0 && strlen(shadow_pass) >= 6)+ return PASSWORD_TYPE_PLAINTEXT;+ if (strncmp(shadow_pass, "md5", 3) == 0 && strlen(shadow_pass) == MD5_PASSWD_LEN)+ return PASSWORD_TYPE_MD5;+ return PASSWORD_TYPE_UNKNOWN;+}++/*+ * Given a plaintext password verifier, "plain:<password>", return the+ * actual plaintext password without the prefix. The returned password is+ * palloc'd.+ */+char *+get_plain_password(const char *shadow_pass)+{+ if (strlen(shadow_pass) < 6 || memcmp(shadow_pass, "plain:", 6) != 0)+ elog(ERROR, "password is not a plaintext password verifier");++ return pstrdup(&shadow_pass[strlen("plain:")]);+}++/*+ * Given a user-supplied password, convert it into a verifier of+ * 'target_type' kind.+ *+ * If the password looks like a valid MD5 hash, it is stored as it is.+ * We cannot reverse the hash, so even if the caller requested a plaintext+ * plaintext password, the MD5 hash is returned.+ *+ * If the password follows the "plain:<password>" format, it is interpreted+ * as a plaintext password. If an MD5 hash was requested, it is hashed,+ * otherwise it is returned as it is.+ */+char *+encode_password(PasswordType target_type, const char *role,+ const char *password)+{+ PasswordType guessed_type = get_password_type(password);+ const char *plain_password;+ char *encrypted_password;++ switch (target_type)+ {+ case PASSWORD_TYPE_UNKNOWN:+ elog(ERROR, "unknown target password type");++ case PASSWORD_TYPE_PLAINTEXT:+ switch (guessed_type)+ {+ case PASSWORD_TYPE_UNKNOWN:+ return psprintf("plain:%s", password);++ case PASSWORD_TYPE_PLAINTEXT:+ return pstrdup(password);++ case PASSWORD_TYPE_MD5:+ /* We cannot convert an MD5 hash back to plaintext. Store the hash. */+ return pstrdup(password);+ }+ break;++ case PASSWORD_TYPE_MD5:+ switch (guessed_type)+ {+ case PASSWORD_TYPE_UNKNOWN:+ case PASSWORD_TYPE_PLAINTEXT:+ encrypted_password = palloc(MD5_PASSWD_LEN + 1);++ /* strip the possible "plain:" prefix, and hash */+ if (guessed_type == PASSWORD_TYPE_PLAINTEXT)+ plain_password = &password[strlen("plain:")];+ else+ plain_password = password;+ if (!pg_md5_encrypt(plain_password, role, strlen(role),+ encrypted_password))+ elog(ERROR, "password encryption failed");+ return encrypted_password;++ case PASSWORD_TYPE_MD5:+ return pstrdup(password);+ }+ }++ elog(ERROR, "unrecognized password type conversion");+}++/*
* Check MD5 authentication response, and return STATUS_OK or STATUS_ERROR.
*
* 'shadow_pass' is the user's correct password or password hash, as stored
@@ -135,32 +225,40 @@ md5_crypt_verify(const char *role, const char *shadow_pass,
* below: the only possible error is out-of-memory, which is unlikely, and
* if it did happen adding a psprintf call would only make things worse.
*/
- if (isMD5(shadow_pass))- {- /* stored password already encrypted, only do salt */- if (!pg_md5_encrypt(shadow_pass + strlen("md5"),- md5_salt, md5_salt_len,- crypt_pwd))- {- return STATUS_ERROR;- }- }- else+ switch(get_password_type(shadow_pass))
{
- /* stored password is plain, double-encrypt */- if (!pg_md5_encrypt(shadow_pass,- role,- strlen(role),- crypt_pwd2))- {- return STATUS_ERROR;- }- if (!pg_md5_encrypt(crypt_pwd2 + strlen("md5"),- md5_salt, md5_salt_len,- crypt_pwd))- {+ case PASSWORD_TYPE_MD5:+ /* stored password already encrypted, only do salt */+ if (!pg_md5_encrypt(shadow_pass + strlen("md5"),+ md5_salt, md5_salt_len,+ crypt_pwd))+ {+ return STATUS_ERROR;+ }+ break;++ case PASSWORD_TYPE_PLAINTEXT:+ /* stored password is plain, double-encrypt */+ if (!pg_md5_encrypt(&shadow_pass[strlen("plain:")],+ role,+ strlen(role),+ crypt_pwd2))+ {+ return STATUS_ERROR;+ }+ if (!pg_md5_encrypt(crypt_pwd2 + strlen("md5"),+ md5_salt, md5_salt_len,+ crypt_pwd))+ {+ return STATUS_ERROR;+ }+ break;++ default:+ /* unknown password hash format. */+ *logdetail = psprintf(_("User \"%s\" has a password that cannot be used with MD5 authentication."),+ role);
return STATUS_ERROR;
- }
}
if (strcmp(client_pass, crypt_pwd) == 0)
@@ -198,22 +296,36 @@ plain_crypt_verify(const char *role, const char *shadow_pass,
* the password the client sent, and compare the hashes. Otherwise
* compare the plaintext passwords directly.
*/
- if (isMD5(shadow_pass))+ switch (get_password_type(shadow_pass))
{
- if (!pg_md5_encrypt(client_pass,- role,- strlen(role),- crypt_client_pass))- {+ case PASSWORD_TYPE_MD5:+ if (!pg_md5_encrypt(client_pass,+ role,+ strlen(role),+ crypt_client_pass))+ {+ /*+ * We do not bother setting logdetail for pg_md5_encrypt failure:+ * the only possible error is out-of-memory, which is unlikely,+ * and if it did happen adding a psprintf call would only make+ * things worse.+ */+ return STATUS_ERROR;+ }+ client_pass = crypt_client_pass;+ break;+ case PASSWORD_TYPE_PLAINTEXT:+ shadow_pass = &shadow_pass[strlen("plain:")];+ break;++ default:
/*
- * We do not bother setting logdetail for pg_md5_encrypt failure:- * the only possible error is out-of-memory, which is unlikely,- * and if it did happen adding a psprintf call would only make- * things worse.+ * This shouldn't happen. Plain "password" authentication should be+ * possible with any kind of stored password hash.
*/
+ *logdetail = psprintf(_("Password of user \"%s\" is in unrecognized format."),+ role);
return STATUS_ERROR;
- }- client_pass = crypt_client_pass;
}
if (strcmp(client_pass, shadow_pass) == 0)
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.cindex a025117..ab71f47 100644--- a/src/backend/utils/misc/guc.c+++ b/src/backend/utils/misc/guc.c@@ -40,6 +40,7 @@
#include "commands/trigger.h"
#include "funcapi.h"
#include "libpq/auth.h"
+#include "libpq/crypt.h"
#include "libpq/be-fsstubs.h"
#include "libpq/libpq.h"
#include "libpq/pqformat.h"
diff --git a/src/include/commands/user.h b/src/include/commands/user.hindex 102c2a5..9cb3629 100644--- a/src/include/commands/user.h+++ b/src/include/commands/user.h@@ -15,21 +15,10 @@
#include "nodes/parsenodes.h"
#include "parser/parse_node.h"
--/*- * Types of password, for Password_encryption GUC and the password_type- * argument of the check-password hook.- */-typedef enum PasswordType-{- PASSWORD_TYPE_PLAINTEXT = 0,- PASSWORD_TYPE_MD5-} PasswordType;-
extern int Password_encryption; /* GUC */
/* Hook to check passwords in CreateRole() and AlterRole() */
-typedef void (*check_password_hook_type) (const char *username, const char *password, int password_type, Datum validuntil_time, bool validuntil_null);+typedef void (*check_password_hook_type) (const char *username, const char *shadow_pass, Datum validuntil_time, bool validuntil_null);
extern PGDLLIMPORT check_password_hook_type check_password_hook;
diff --git a/src/include/common/md5.h b/src/include/common/md5.hindex 4a04320..a29731b 100644--- a/src/include/common/md5.h+++ b/src/include/common/md5.h@@ -18,10 +18,6 @@
#define MD5_PASSWD_LEN 35
-#define isMD5(passwd) (strncmp(passwd, "md5", 3) == 0 && \- strlen(passwd) == MD5_PASSWD_LEN)--
extern bool pg_md5_hash(const void *buff, size_t len, char *hexsum);
extern bool pg_md5_binary(const void *buff, size_t len, void *outbuf);
extern bool pg_md5_encrypt(const char *passwd, const char *salt,
diff --git a/src/include/libpq/crypt.h b/src/include/libpq/crypt.hindex 229ce76..ed47c05 100644--- a/src/include/libpq/crypt.h+++ b/src/include/libpq/crypt.h@@ -15,7 +15,25 @@
#include "datatype/timestamp.h"
-extern int get_role_password(const char *role, char **shadow_pass, char **logdetail);+/*+ * Types of password.+ *+ * This is also used for the password_encryption GUC.+ */+typedef enum PasswordType+{+ PASSWORD_TYPE_UNKNOWN = -1,+ PASSWORD_TYPE_PLAINTEXT = 0,+ PASSWORD_TYPE_MD5+} PasswordType;++extern PasswordType get_password_type(const char *shadow_pass);+extern char *get_plain_password(const char *shadow_pass);+extern char *encode_password(PasswordType target_type, const char *role,+ const char *password);++extern int get_role_password(const char *role, char **shadow_pass,+ char **logdetail);
extern int md5_crypt_verify(const char *role, const char *shadow_pass,
const char *client_pass, const char *md5_salt,
--
2.10.2
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: hlinnaka@iki.fi, michael.paquier@gmail.com, magnus@hagander.net, robertmhaas@gmail.com, andres@anarazel.de, peter.eisentraut@2ndquadrant.com, david@pgmasters.net, tgl@sss.pgh.pa.us, david@fetter.org, alvherre@2ndquadrant.com, julian.markwort@uni-muenster.de, sfrost@snowman.net, v.popov@postgrespro.ru
Subject: Re: pg_authid.rolpassword format (was Re: Password identifiers, protocol aging and SCRAM protocol)
In-Reply-To: <a150e7c7-7d57-bf1c-7f73-cbbfedf99243@iki.fi>
* 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