Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit ccae190

Browse files
committed
Fix detection of passwords hashed with MD5 or SCRAM-SHA-256
This commit fixes a couple of issues related to the way password verifiers hashed with MD5 or SCRAM-SHA-256 are detected, leading to being able to store in catalogs passwords which do not follow the supported hash formats: - A MD5-hashed entry was checked based on if its header uses "md5" and if the string length matches what is expected. Unfortunately the code never checked if the hash only used hexadecimal characters, as reported by Tom Lane. - A SCRAM-hashed entry was checked based on only its header, which should be "SCRAM-SHA-256$", but it never checked for any fields afterwards, as reported by Jonathan Katz. Backpatch down to v10, which is where SCRAM has been introduced, and where password verifiers in plain format have been removed. Author: Jonathan Katz Reviewed-by: Tom Lane, Michael Paquier Discussion: https://postgr.es/m/016deb6b-1f0a-8e9f-1833-a8675b170aa9@postgresql.org Backpatch-through: 10
1 parent b5f58cf commit ccae190

File tree

6 files changed

+44
-6
lines changed

6 files changed

+44
-6
lines changed

src/backend/libpq/auth-scram.c

+1-3
Original file line numberDiff line numberDiff line change
@@ -161,8 +161,6 @@ static char *build_server_first_message(scram_state *state);
161161
static char *build_server_final_message(scram_state *state);
162162
static bool verify_client_proof(scram_state *state);
163163
static bool verify_final_nonce(scram_state *state);
164-
static bool parse_scram_verifier(const char *verifier, int *iterations,
165-
char **salt, uint8 *stored_key, uint8 *server_key);
166164
static void mock_scram_verifier(const char *username, int *iterations,
167165
char **salt, uint8 *stored_key, uint8 *server_key);
168166
static bool is_scram_printable(char *p);
@@ -546,7 +544,7 @@ scram_verify_plain_password(const char *username, const char *password,
546544
*
547545
* Returns true if the SCRAM verifier has been parsed, and false otherwise.
548546
*/
549-
static bool
547+
bool
550548
parse_scram_verifier(const char *verifier, int *iterations, char **salt,
551549
uint8 *stored_key, uint8 *server_key)
552550
{

src/backend/libpq/crypt.c

+11-2
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020

2121
#include "catalog/pg_authid.h"
2222
#include "common/md5.h"
23+
#include "common/scram-common.h"
2324
#include "libpq/crypt.h"
2425
#include "libpq/scram.h"
2526
#include "miscadmin.h"
@@ -90,9 +91,17 @@ get_role_password(const char *role, char **logdetail)
9091
PasswordType
9192
get_password_type(const char *shadow_pass)
9293
{
93-
if (strncmp(shadow_pass, "md5", 3) == 0 && strlen(shadow_pass) == MD5_PASSWD_LEN)
94+
char *encoded_salt;
95+
int iterations;
96+
uint8 stored_key[SCRAM_KEY_LEN];
97+
uint8 server_key[SCRAM_KEY_LEN];
98+
99+
if (strncmp(shadow_pass, "md5", 3) == 0 &&
100+
strlen(shadow_pass) == MD5_PASSWD_LEN &&
101+
strspn(shadow_pass + 3, MD5_PASSWD_CHARSET) == MD5_PASSWD_LEN - 3)
94102
return PASSWORD_TYPE_MD5;
95-
if (strncmp(shadow_pass, "SCRAM-SHA-256$", strlen("SCRAM-SHA-256$")) == 0)
103+
if (parse_scram_verifier(shadow_pass, &iterations, &encoded_salt,
104+
stored_key, server_key))
96105
return PASSWORD_TYPE_SCRAM_SHA_256;
97106
return PASSWORD_TYPE_PLAINTEXT;
98107
}

src/include/common/md5.h

+1
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
#ifndef PG_MD5_H
1717
#define PG_MD5_H
1818

19+
#define MD5_PASSWD_CHARSET "0123456789abcdef"
1920
#define MD5_PASSWD_LEN 35
2021

2122
extern bool pg_md5_hash(const void *buff, size_t len, char *hexsum);

src/include/libpq/scram.h

+2
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@ extern int pg_be_scram_exchange(void *opaq, const char *input, int inputlen,
2929

3030
/* Routines to handle and check SCRAM-SHA-256 verifier */
3131
extern char *pg_be_scram_build_verifier(const char *password);
32+
extern bool parse_scram_verifier(const char *verifier, int *iterations, char **salt,
33+
uint8 *stored_key, uint8 *server_key);
3234
extern bool scram_verify_plain_password(const char *username,
3335
const char *password, const char *verifier);
3436

src/test/regress/expected/password.out

+16-1
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,15 @@ SET password_encryption = 'scram-sha-256';
6262
ALTER ROLE regress_passwd4 PASSWORD 'foo';
6363
-- already encrypted with MD5, use as it is
6464
CREATE ROLE regress_passwd5 PASSWORD 'md5e73a4b11df52a6068f8b39f90be36023';
65+
-- This looks like a valid SCRAM-SHA-256 verifier, but it is not
66+
-- so it should be hashed with SCRAM-SHA-256.
67+
CREATE ROLE regress_passwd6 PASSWORD 'SCRAM-SHA-256$1234';
68+
-- These may look like valid MD5 verifiers, but they are not, so they
69+
-- should be hashed with SCRAM-SHA-256.
70+
-- trailing garbage at the end
71+
CREATE ROLE regress_passwd7 PASSWORD 'md5012345678901234567890123456789zz';
72+
-- invalid length
73+
CREATE ROLE regress_passwd8 PASSWORD 'md501234567890123456789012345678901zz';
6574
SELECT rolname, regexp_replace(rolpassword, '(SCRAM-SHA-256)\$(\d+):([a-zA-Z0-9+/=]+)\$([a-zA-Z0-9+=/]+):([a-zA-Z0-9+/=]+)', '\1$\2:<salt>$<storedkey>:<serverkey>') as rolpassword_masked
6675
FROM pg_authid
6776
WHERE rolname LIKE 'regress_passwd%'
@@ -73,7 +82,10 @@ SELECT rolname, regexp_replace(rolpassword, '(SCRAM-SHA-256)\$(\d+):([a-zA-Z0-9+
7382
regress_passwd3 | SCRAM-SHA-256$4096:<salt>$<storedkey>:<serverkey>
7483
regress_passwd4 | SCRAM-SHA-256$4096:<salt>$<storedkey>:<serverkey>
7584
regress_passwd5 | md5e73a4b11df52a6068f8b39f90be36023
76-
(5 rows)
85+
regress_passwd6 | SCRAM-SHA-256$4096:<salt>$<storedkey>:<serverkey>
86+
regress_passwd7 | SCRAM-SHA-256$4096:<salt>$<storedkey>:<serverkey>
87+
regress_passwd8 | SCRAM-SHA-256$4096:<salt>$<storedkey>:<serverkey>
88+
(8 rows)
7789

7890
-- An empty password is not allowed, in any form
7991
CREATE ROLE regress_passwd_empty PASSWORD '';
@@ -93,6 +105,9 @@ DROP ROLE regress_passwd2;
93105
DROP ROLE regress_passwd3;
94106
DROP ROLE regress_passwd4;
95107
DROP ROLE regress_passwd5;
108+
DROP ROLE regress_passwd6;
109+
DROP ROLE regress_passwd7;
110+
DROP ROLE regress_passwd8;
96111
DROP ROLE regress_passwd_empty;
97112
-- all entries should have been removed
98113
SELECT rolname, rolpassword

src/test/regress/sql/password.sql

+13
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,16 @@ ALTER ROLE regress_passwd4 PASSWORD 'foo';
5454
-- already encrypted with MD5, use as it is
5555
CREATE ROLE regress_passwd5 PASSWORD 'md5e73a4b11df52a6068f8b39f90be36023';
5656

57+
-- This looks like a valid SCRAM-SHA-256 verifier, but it is not
58+
-- so it should be hashed with SCRAM-SHA-256.
59+
CREATE ROLE regress_passwd6 PASSWORD 'SCRAM-SHA-256$1234';
60+
-- These may look like valid MD5 verifiers, but they are not, so they
61+
-- should be hashed with SCRAM-SHA-256.
62+
-- trailing garbage at the end
63+
CREATE ROLE regress_passwd7 PASSWORD 'md5012345678901234567890123456789zz';
64+
-- invalid length
65+
CREATE ROLE regress_passwd8 PASSWORD 'md501234567890123456789012345678901zz';
66+
5767
SELECT rolname, regexp_replace(rolpassword, '(SCRAM-SHA-256)\$(\d+):([a-zA-Z0-9+/=]+)\$([a-zA-Z0-9+=/]+):([a-zA-Z0-9+/=]+)', '\1$\2:<salt>$<storedkey>:<serverkey>') as rolpassword_masked
5868
FROM pg_authid
5969
WHERE rolname LIKE 'regress_passwd%'
@@ -70,6 +80,9 @@ DROP ROLE regress_passwd2;
7080
DROP ROLE regress_passwd3;
7181
DROP ROLE regress_passwd4;
7282
DROP ROLE regress_passwd5;
83+
DROP ROLE regress_passwd6;
84+
DROP ROLE regress_passwd7;
85+
DROP ROLE regress_passwd8;
7386
DROP ROLE regress_passwd_empty;
7487

7588
-- all entries should have been removed

0 commit comments

Comments
 (0)