From 26e52e72010a4c25a69ff32f3a960298b3a99250 Mon Sep 17 00:00:00 2001 From: "Jonathan S. Katz" Date: Sat, 20 Apr 2019 16:02:32 -0400 Subject: [PATCH] Modified SCRAM-SHA-256 validation check for get_password_type The get_password_type function previously just checked to see if a password started with "SCRAM-SHA-256$" to determine whether or not it was a SCRAM-SHA-256 hashed password. However, this would cause passwords created in this was to be unusuable. This changes get_password_type methodology to actually perform SCRAM-SHA-256 verification on the password to see if it is a valid hash. If not, it will proceed to the next password type check. --- src/backend/libpq/auth-scram.c | 4 +--- src/backend/libpq/crypt.c | 9 ++++++++- src/include/libpq/scram.h | 2 ++ src/test/regress/expected/password.out | 11 +++++++++++ src/test/regress/sql/password.sql | 7 +++++++ 5 files changed, 29 insertions(+), 4 deletions(-) diff --git a/src/backend/libpq/auth-scram.c b/src/backend/libpq/auth-scram.c index 3cbe902ace..4c7b149570 100644 --- a/src/backend/libpq/auth-scram.c +++ b/src/backend/libpq/auth-scram.c @@ -161,8 +161,6 @@ static char *build_server_first_message(scram_state *state); static char *build_server_final_message(scram_state *state); static bool verify_client_proof(scram_state *state); static bool verify_final_nonce(scram_state *state); -static bool parse_scram_verifier(const char *verifier, int *iterations, - char **salt, uint8 *stored_key, uint8 *server_key); static void mock_scram_verifier(const char *username, int *iterations, char **salt, uint8 *stored_key, uint8 *server_key); static bool is_scram_printable(char *p); @@ -546,7 +544,7 @@ scram_verify_plain_password(const char *username, const char *password, * * Returns true if the SCRAM verifier has been parsed, and false otherwise. */ -static bool +bool parse_scram_verifier(const char *verifier, int *iterations, char **salt, uint8 *stored_key, uint8 *server_key) { diff --git a/src/backend/libpq/crypt.c b/src/backend/libpq/crypt.c index c34e4a3d85..db19f51c80 100644 --- a/src/backend/libpq/crypt.c +++ b/src/backend/libpq/crypt.c @@ -20,6 +20,7 @@ #include "catalog/pg_authid.h" #include "common/md5.h" +#include "common/scram-common.h" #include "libpq/crypt.h" #include "libpq/scram.h" #include "miscadmin.h" @@ -90,9 +91,15 @@ get_role_password(const char *role, char **logdetail) PasswordType get_password_type(const char *shadow_pass) { + char *encoded_salt; + int iterations; + uint8 stored_key[SCRAM_KEY_LEN]; + uint8 server_key[SCRAM_KEY_LEN]; + if (strncmp(shadow_pass, "md5", 3) == 0 && strlen(shadow_pass) == MD5_PASSWD_LEN) return PASSWORD_TYPE_MD5; - if (strncmp(shadow_pass, "SCRAM-SHA-256$", strlen("SCRAM-SHA-256$")) == 0) + if (strncmp(shadow_pass, "SCRAM-SHA-256$", strlen("SCRAM-SHA-256$")) == 0 && + parse_scram_verifier(shadow_pass, &iterations, &encoded_salt, stored_key, server_key)) return PASSWORD_TYPE_SCRAM_SHA_256; return PASSWORD_TYPE_PLAINTEXT; } diff --git a/src/include/libpq/scram.h b/src/include/libpq/scram.h index d7f4c094c9..bc297e2a33 100644 --- a/src/include/libpq/scram.h +++ b/src/include/libpq/scram.h @@ -29,6 +29,8 @@ extern int pg_be_scram_exchange(void *opaq, const char *input, int inputlen, /* Routines to handle and check SCRAM-SHA-256 verifier */ extern char *pg_be_scram_build_verifier(const char *password); +extern bool parse_scram_verifier(const char *verifier, int *iterations, char **salt, + uint8 *stored_key, uint8 *server_key); extern bool scram_verify_plain_password(const char *username, const char *password, const char *verifier); diff --git a/src/test/regress/expected/password.out b/src/test/regress/expected/password.out index 393d836ead..d40dcf9a43 100644 --- a/src/test/regress/expected/password.out +++ b/src/test/regress/expected/password.out @@ -75,6 +75,16 @@ SELECT rolname, regexp_replace(rolpassword, '(SCRAM-SHA-256)\$(\d+):([a-zA-Z0-9+ regress_passwd5 | md5e73a4b11df52a6068f8b39f90be36023 (5 rows) +-- An invalid SCRAM-SHA-256 that is encrypted is streated as a SCRAM-SHA-256 password +CREATE ROLE regress_passwd6 PASSWORD 'SCRAM-SHA-256$1234'; +SELECT rolname, regexp_replace(rolpassword, '(SCRAM-SHA-256)\$(\d+):([a-zA-Z0-9+/=]+)\$([a-zA-Z0-9+=/]+):([a-zA-Z0-9+/=]+)', '\1$\2:$:') as rolpassword_masked + FROM pg_authid + WHERE rolname='regress_passwd6'; + rolname | rolpassword_masked +-----------------+--------------------------------------------------- + regress_passwd6 | SCRAM-SHA-256$4096:$: +(1 row) + -- An empty password is not allowed, in any form CREATE ROLE regress_passwd_empty PASSWORD ''; NOTICE: empty string is not a valid password, clearing password @@ -93,6 +103,7 @@ DROP ROLE regress_passwd2; DROP ROLE regress_passwd3; DROP ROLE regress_passwd4; DROP ROLE regress_passwd5; +DROP ROLE regress_passwd6; DROP ROLE regress_passwd_empty; -- all entries should have been removed SELECT rolname, rolpassword diff --git a/src/test/regress/sql/password.sql b/src/test/regress/sql/password.sql index 8f8252d127..b5a0a89182 100644 --- a/src/test/regress/sql/password.sql +++ b/src/test/regress/sql/password.sql @@ -59,6 +59,12 @@ SELECT rolname, regexp_replace(rolpassword, '(SCRAM-SHA-256)\$(\d+):([a-zA-Z0-9+ WHERE rolname LIKE 'regress_passwd%' ORDER BY rolname, rolpassword; +-- An invalid SCRAM-SHA-256 that is encrypted is streated as a SCRAM-SHA-256 password +CREATE ROLE regress_passwd6 PASSWORD 'SCRAM-SHA-256$1234'; +SELECT rolname, regexp_replace(rolpassword, '(SCRAM-SHA-256)\$(\d+):([a-zA-Z0-9+/=]+)\$([a-zA-Z0-9+=/]+):([a-zA-Z0-9+/=]+)', '\1$\2:$:') as rolpassword_masked + FROM pg_authid + WHERE rolname='regress_passwd6'; + -- An empty password is not allowed, in any form CREATE ROLE regress_passwd_empty PASSWORD ''; ALTER ROLE regress_passwd_empty PASSWORD 'md585939a5ce845f1a1b620742e3c659e0a'; @@ -70,6 +76,7 @@ DROP ROLE regress_passwd2; DROP ROLE regress_passwd3; DROP ROLE regress_passwd4; DROP ROLE regress_passwd5; +DROP ROLE regress_passwd6; DROP ROLE regress_passwd_empty; -- all entries should have been removed -- 2.14.3 (Apple Git-98)