>From 5bbda2ed5c1e8fc06c2a6761e4924224ecb34adf Mon Sep 17 00:00:00 2001 From: Heikki Linnakangas Date: Tue, 3 Jan 2017 13:37:49 +0200 Subject: [PATCH 2/2] 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! Discussion: https://www.postgresql.org/message-id/2d07165c-1793-e243-a2a9-e45b624c7580@iki.fi --- contrib/passwordcheck/passwordcheck.c | 7 +++-- doc/src/sgml/catalogs.sgml | 4 +-- src/backend/commands/user.c | 32 ++++++++++++++++++--- src/backend/libpq/crypt.c | 53 +++++++++++++++++++++++++++++------ src/include/commands/user.h | 2 +- src/include/libpq/crypt.h | 1 + 6 files changed, 81 insertions(+), 18 deletions(-) diff --git a/contrib/passwordcheck/passwordcheck.c b/contrib/passwordcheck/passwordcheck.c index 03dfdfd0e1..3d06964cab 100644 --- a/contrib/passwordcheck/passwordcheck.c +++ b/contrib/passwordcheck/passwordcheck.c @@ -51,11 +51,10 @@ extern void _PG_init(void); static void check_password(const char *username, const char *shadow_pass, - PasswordType password_type, Datum validuntil_time, bool validuntil_null) { - if (password_type != PASSWORD_TYPE_PLAINTEXT) + if (get_password_type(shadow_pass) != PASSWORD_TYPE_PLAINTEXT) { /* * Unfortunately we cannot perform exhaustive checks on encrypted @@ -77,7 +76,7 @@ check_password(const char *username, /* * For unencrypted passwords we can perform better checks */ - const char *password = shadow_pass; + char *password = get_plain_password(shadow_pass); int pwdlen = strlen(password); int i; bool pwd_has_letter, @@ -121,6 +120,8 @@ check_password(const char *username, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), errmsg("password is easily cracked"))); #endif + + pfree(password); } /* all checks passed, password is ok */ diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml index 493050618d..5df31797e6 100644 --- a/doc/src/sgml/catalogs.sgml +++ b/doc/src/sgml/catalogs.sgml @@ -1325,8 +1325,8 @@ will be of the user's password concatenated to their user name. For example, if user joe has password xyzzy, PostgreSQL will store the md5 hash of - xyzzyjoe. A password that does not follow that - format is assumed to be unencrypted. + xyzzyjoe. If a password is stored unencrypted, the + column will begin with the string plain:. diff --git a/src/backend/commands/user.c b/src/backend/commands/user.c index 48969d3a47..6811f0fa3d 100644 --- a/src/backend/commands/user.c +++ b/src/backend/commands/user.c @@ -367,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 = encrypt_password(PASSWORD_TYPE_PLAINTEXT, stmt->role, + password); + (*check_password_hook) (stmt->role, - password, - get_password_type(password), + plain_password, validUntil_datum, validUntil_null); + pfree(plain_password); + } /* * Build a tuple to insert @@ -734,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 = encrypt_password(PASSWORD_TYPE_PLAINTEXT, rolename, + password); + (*check_password_hook) (rolename, - password, - get_password_type(password), + plain_password, validUntil_datum, validUntil_null); + pfree(plain_password); + } /* * Build an updated tuple, perusing the information just obtained diff --git a/src/backend/libpq/crypt.c b/src/backend/libpq/crypt.c index 5ec5951ece..57806a38e8 100644 --- a/src/backend/libpq/crypt.c +++ b/src/backend/libpq/crypt.c @@ -109,9 +109,25 @@ get_role_password(const char *role, char **shadow_pass, char **logdetail) 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_PLAINTEXT; + return PASSWORD_TYPE_UNKNOWN; +} + +/* + * Given a plaintext password verifier, "plain:", 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:")]); } /* @@ -121,31 +137,52 @@ get_password_type(const char *shadow_pass) * 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:" 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 * encrypt_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: - /* - * We cannot convert an MD5 hash back to plaintext, - * so no need to what what the input was. Just store - * it as it is. - */ - return pstrdup(password); + 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); - if (!pg_md5_encrypt(password, role, strlen(role), + /* 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; diff --git a/src/include/commands/user.h b/src/include/commands/user.h index 08037e0f81..c1c4d431d4 100644 --- a/src/include/commands/user.h +++ b/src/include/commands/user.h @@ -20,7 +20,7 @@ extern int Password_encryption; /* Hook to check passwords in CreateRole() and AlterRole() */ -typedef void (*check_password_hook_type) (const char *username, const char *shadow_pass, PasswordType 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/libpq/crypt.h b/src/include/libpq/crypt.h index 338be28b9d..4c94054718 100644 --- a/src/include/libpq/crypt.h +++ b/src/include/libpq/crypt.h @@ -22,6 +22,7 @@ */ typedef enum PasswordType { + PASSWORD_TYPE_UNKNOWN = -1, PASSWORD_TYPE_PLAINTEXT = 0, PASSWORD_TYPE_MD5 } PasswordType; -- 2.11.0