From: Jonathan S. Katz <jkatz@postgresql.org>
To: pgsql-bugs@lists.postgresql.org
Cc: Stephen Frost <sfrost@snowman.net>
Subject: Re: Possible to store invalid SCRAM-SHA-256 Passwords
Date: Sat, 20 Apr 2019 16:12:56 -0400
Message-ID: <4240a41f-a957-fd7d-7c24-31683f259b9d@postgresql.org> (raw)
In-Reply-To: <016deb6b-1f0a-8e9f-1833-a8675b170aa9@postgresql.org>
References: <016deb6b-1f0a-8e9f-1833-a8675b170aa9@postgresql.org>
On 4/20/19 3:26 PM, Jonathan S. Katz wrote:
> Hi,
>
> With some guidance from Stephen, I've discovered some scenarios where
> one can store invalid SCRAM-SHA-256 passwords.
>
> Scenario #1: Directly from CREATE/ALTER ROLE
>
> for example on PostgreSQL 11:
>
> CREATE ROLE test1 PASSWORD 'SCRAM-SHA-256$1234' LOGIN;
>
> In the logs, one sees:
>
> 2019-04-20 18:36:07.883 UTC [22251] postgres@postgres LOG: invalid
> SCRAM verifier for user "test1"
> 2019-04-20 18:36:07.883 UTC [22251] postgres@postgres STATEMENT:
> CREATE USER test1 PASSWORD 'SCRAM-SHA-256$1234' LOGIN;
>
> pg_authid contains:
>
> -[ RECORD 1 ]--+-------------------
> rolname | test1
> rolcanlogin | t
> rolpassword | SCRAM-SHA-256$1234
>
> and when I try to login with the password "SCRAM-SHA-256$1234" e.g.
>
> psql -U test1 postgres
>
> psql: FATAL: password authentication failed for user "test1"
> FATAL: password authentication failed for user "test1"
>
> Scenario #2: On an upgrade from PG < 10 => PG >= 10
>
> On a PostgreSQL 9.6.12, I created a user as the following:
>
> CREATE ROLE test2 WITH UNENCRYPTED PASSWORD 'SCRAM-SHA-256$1234' LOGIN;
>
> with pg_authid contents:
>
> -[ RECORD 1 ]--+-------------------
> rolname | test2
> rolcanlogin | t
> rolpassword | SCRAM-SHA-256$1234
>
> And was able to **successfully login.**
>
> I installed PostgreSQL 11 and upgrading from 9.6.12 => 11.2
>
> When I attempt to login as test2, I get the following error:
>
> psql: FATAL: password authentication failed for user "tester"
> FATAL: password authentication failed for user "tester"
>
> While my hunch is that Scenario #2 is less likely to happen in the wild,
> Scenario #1 is a real possibility. Examples, a non-libpq passed driver
> wants to send a hashed password directly to a server and has a mistake
> in the algorithm, or a program calls "ALTER ROLE" and modifies a
> password with an invalid SCRAM-SHA-256 hash in it, etc.
Attached is a patch that I believe resolves this.
I modified the "get_password_type" function to perform a SCRAM
verification to see if it is a properly hashed SCRAM password. If it is,
we treat the password as a SCRAM hashed one. Otherwise, we proceed to
the next step, which is to treat it as a plainly stored one.
Thoughts?
Jonathan
From a8b999ed44fd636fce05bee8fcaffedb9b0fb9a7 Mon Sep 17 00:00:00 2001
From: "Jonathan S. Katz" <jonathan.katz@excoventures.com>
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 ++
3 files changed, 11 insertions(+), 4 deletions(-)
diff --git a/src/backend/libpq/auth-scram.c b/src/backend/libpq/auth-scram.cindex 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.cindex 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.hindex 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);
--
2.14.3 (Apple Git-98)
Attachments:
[text/plain] 0001-Modified-SCRAM-SHA-256-validation-check-for-get_pass.patch (3.6K, ../4240a41f-a957-fd7d-7c24-31683f259b9d@postgresql.org/2-0001-Modified-SCRAM-SHA-256-validation-check-for-get_pass.patch)
download | inline diff:
From a8b999ed44fd636fce05bee8fcaffedb9b0fb9a7 Mon Sep 17 00:00:00 2001
From: "Jonathan S. Katz" <jonathan.katz@excoventures.com>
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 ++
3 files changed, 11 insertions(+), 4 deletions(-)
diff --git a/src/backend/libpq/auth-scram.c b/src/backend/libpq/auth-scram.cindex 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.cindex 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.hindex 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);
--
2.14.3 (Apple Git-98)
[application/pgp-signature] signature.asc (832B, ../4240a41f-a957-fd7d-7c24-31683f259b9d@postgresql.org/3-signature.asc)
download
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-bugs@postgresql.org
Cc: jkatz@postgresql.org, pgsql-bugs@lists.postgresql.org, sfrost@snowman.net
Subject: Re: Possible to store invalid SCRAM-SHA-256 Passwords
In-Reply-To: <4240a41f-a957-fd7d-7c24-31683f259b9d@postgresql.org>
* 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