Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1hIg0e-00044t-DA for pgsql-bugs@arkaria.postgresql.org; Mon, 22 Apr 2019 20:57:52 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.89) (envelope-from ) id 1hIg0d-0000JO-6a for pgsql-bugs@arkaria.postgresql.org; Mon, 22 Apr 2019 20:57:51 +0000 Received: from makus.postgresql.org ([2001:4800:3e1:1::229]) by malur.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1hIg0c-0000JF-Ds for pgsql-bugs@lists.postgresql.org; Mon, 22 Apr 2019 20:57:51 +0000 Received: from meldrar.postgresql.org ([2a02:c0:301:0:ffff::31]) by makus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1hIg0Z-0000v1-7s for pgsql-bugs@lists.postgresql.org; Mon, 22 Apr 2019 20:57:49 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=postgresql.org; s=20171124; h=Content-Type:In-Reply-To:MIME-Version:Date: Message-ID:References:Cc:To:From:Subject:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description; bh=uWd+8ubrkOzAPFodclrqPMKf+9xOzXzaOo546VdWv6Y=; b=jEhiq1WYhWNquH1aY1KJNyJUcg 5pAFZ9SFE6hh1R2Ma5N7PoEJ+0dV5l2wTm/NgCrv4tP0piFdZpVZP/CGnqyUH4HN/LrZD92RSasDH Xhsi4Kmaf3s2ul6PR33Zt7r1KE9WtNs533l0gn7x/qh/Wh/rsBzGfnCSACgGMIVdewRJQOJ1sO11u vgSR3lpCePPdfoplHMgzZU3lVPlizYrxk6fr6vW8yiGUFxmeJc97JON/en51u05Xx0wI1HADKrHfa wpI6QPR8tEQEfKbWHPAJz0yvlUjUsVXkIyw74C0Sl0CGxV+M9es+wlMoFUe8ygyiqId0ggjdQFotZ hMZys7bA==; Received: from [63.118.15.50] (helo=Ph33r-Retina.local) by meldrar.postgresql.org with esmtpsa (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1hIg0V-00022k-BB; Mon, 22 Apr 2019 20:57:45 +0000 Subject: Re: Possible to store invalid SCRAM-SHA-256 Passwords From: "Jonathan S. Katz" To: Michael Paquier Cc: pgsql-bugs@lists.postgresql.org, Stephen Frost References: <016deb6b-1f0a-8e9f-1833-a8675b170aa9@postgresql.org> <4240a41f-a957-fd7d-7c24-31683f259b9d@postgresql.org> <20190422015051.GA3433@paquier.xyz> <17ed1a12-14df-a501-8b58-f4d1eba49e3f@postgresql.org> Openpgp: preference=signencrypt Autocrypt: addr=jkatz@postgresql.org; keydata= mQINBFtShwABEAC7PNHDUOTYuifpUCk23KqfxdQQkn4nkoxOXRK0+rAj36FiwqGB4TJFuOVZ sDFAEiSlC8Jt4y5Cs7B5tetT8JNd2cs6zp/udMJJDz9d65O9PDpdlMgAmIiTzpLlSdx8FG56 DTksaDv1d8j3cTJPSE4/fWSxqzA7o3Y9UuL7atZPrzfImgpRKs0of7elIHwOa8GucjyhYqcR h60wFBJc2KXqQdDYRTZy43DSnY/0VNc0omiH355fustvpm+m5HjD3w7qZyfN3fpKJpnX1LCF f3MnPHaDGITIYGRCBXvf0UqUtD6OEVWPv2C2gyqWMIpWmZTOgDufltKyIByKBoS9x0PlFkij 04X3KODCngt+N8Ssc9OICc6QSxhjoP48PYPdmiTmkrGuf0LX084wj1xeo1NX7XxZK39F6dTJ DhsIiW0sNS0xMxQHLHG9VLbPjx3SANQBh6BuryPz5ZupW9/TIDmkvprtU/oXfKgtfYm3fxmk EctxbWrEPsFTFPyuMqQu6l+xyQv0s1VLZfjNWaua6H1/gGoIt6kRnn5qMXDVVpijuWkHbv7G ngaQMd258UrrOEHnnjzhQ7jxMWV9D+emxbAtlIxnYvCWlV4IwAQhEHfvudqYaIY3hNWrvQ6H GB2KXoTZYN9g5djm14/5nj1IU5zOcovkjJnKhoo9iStnpFF2cwARAQABtCdKb25hdGhhbiBT LiBLYXR6IDxqa2F0ekBwb3N0Z3Jlc3FsLm9yZz6JAlQEEwEIAD4CGwMFCQeGH4AFCwkIBwIG FQoJCAsCBBYCAwECHgECF4AWIQT6hLaVryv3miBkP/HxBJxynxxlJwUCW1KJJQAKCRDxBJxy nxxlJwjrD/kBgqsW4QpNpTFw7ifRokZV08CCX4huPBJQ91rrv+UEWlEcotFBHVkYyHnpzARl tcZxhJ9CbFxjniH9cOTty5T/O1yolbOHtZSW8Z8aWV6BVEbjMb+BFxSSLm7RnvJdzQbGCZq2 ZZvfVpB6z3EHYph4KDdVKvMFjoLskxmdS1DE0tE3zTxvoQsi24Q+HOS07kUjs6fsu/WICMfz mgO++AWG9Y0CvN0mm4TkujESzyKM9E5irD+leEMIcddl51Aa2c/VMfBXQbRmpHIgUFTmuHQD CnQih+9i3OJAksDg66SP8a7yiXv5mwvyDi1EfTGVKYR2j+pwyjwnC3oIbvDMmB3uTn2JIjnT iZKPVtAcAylXjubFltihQgNyuShdP4W+kBwZizhUFqUVL8Anx+KoytYmJPfMRFLGuK4obXKq a2ZS3k9KB+H+isOx2nFJOsc7V360Zp1DVaNmuiK10TT6QndShSPaqkJqFtCb6r92rZ9sZM/L 3vtCI4Rrl3Pt1MgtENXupS8gZpJnAYS0j5A1PAZ09r6ANoaeMHspF+5J5fOHeEvqphXr36mm a83Vl1t4orPb0+QmmijmlpseDU63M88Aw5p3c4qj7t8Qr2EZ5zrn7/sFn5wOfbs8Nymxafif QCnlV2vg9p0m7vSk/yLJ4PFZvs52FgqAGRCdRn0s2EC99bkCDQRbUocAARAAv8ho/toQ9DG3 j4f9h9n1aRHr2FlviN2Utpy6L8+dfDggO0geilmkGQOolZ2E60gGfye/kUtF9W3NByO4hxDR 9u6qbOXcdqnuA+cc68EfqlWFJrVtYFxt0h4ElWYOYnIezKthriWch/FY70FGrxs3z8UHOHq5 0wBW433eTvZm90WixBiXEt2v1DgW4Vr3ymfO7Aap/IYyPuE4JzgudAuAl0HKPyNEHWHG1dAb jX1RiCw9gknIDWQOF0B4UAaJctWGVcnZ3A2ULwNGMa1P9ZJlBWf1vcj01aiHMU0yQ7JjJiSp vfm9eM0uSLwRdDrJjyl5ZZqVumjdv2SMNQ8GvYRbEMys3GGDSt9zXgfCSUnPnJfYxjzBHRI6 x44Wfsx8S6hWxepOogCJJ/g67Bk9mY8YV4klWIXDJVOL5jnBC09DbsZG81JaE2QxB8Y7W36Z Mroi9XMxg3s805hQAQUvdG/poU8hN8BWdrnTm/+4eQQp7gDY1ePDmGM6bJC+OHOSnFtR/f+7 0zpKJ10cc7cBygGnl1yR3KjhFyAWUFvP4ZGziKCcpMwXZfe9PGuyA/YOubMphxIn3YsK2wrd faKZYX2GMZCZhMMcvx9IpQrxIJgU+VlwXu/O+Lk10VIPcxPJJwmpdI6HzcS8ZgG6IMcC444X XTuLaP8j2mgcMvYak3ScCykAEQEAAYkCPAQYAQgAJhYhBPqEtpWvK/eaIGQ/8fEEnHKfHGUn BQJbUocAAhsMBQkHhh+AAAoJEPEEnHKfHGUnReMP/RA2UhGQj+G4uBshkRLjpRysabdPqgQB dEBk6wYbio88Wg/2/hgY7UzmDDEwX3sZfQDcrI6+vIobI8uqstZID+WgAAa1JLfChMyVQnSy 0zfWMOABXscc5tGuvFRZvJklTissMFjXUwaREEKp4ZikTvJ/62MCjSdtrUhnPLvoTsHTKRKD ichE+b5A54alwsubTijw12O6N22r5IjZiiSZV0u7dsShyKw+7wCSax9fuBoE43NMYf+dnjMK nerAQYUcZWYMnk+EC8RaqYAxv6XZ2tKx1AkGGktwkQIBwrz9IlDSvJ3LWJ2UIIuLRTdngNgg GIL4zzuUa2F56FqskQIuYMaNETk6LYfalBDQ6TVLAcgCPQxp4k4i/PRsZ3lZ2ZhRHRYciOvm kp+I6EfHwllQpYrWs1thluBGqlJSVJgKl0IOFvKLsQ6KKfqzAwh6FxrO5qajp4viNIgtWoFw O5Bp0jgFTbH2OrMWIRfUdUCH1Djbuo65svhNj7FNsQVYzHDI+Nd1I/LOBoPc3UorRMF2M4JV kUR7skOHWUbPTSNUr7qc14NSMY3PKjGeVGAsBVHBPvmRx/Ss2tW/TpJWpxg4pmquFQSXuaYh Yf4FN8Sxy320pcr+FqN8AhZkYJcNY82OTtR5VEKRC/mYcyq61qXIKqngydn79bJjGxHipxyF FmEC Message-ID: <32661faa-0254-d37e-7930-00d823f38f9f@postgresql.org> Date: Mon, 22 Apr 2019 16:57:40 -0400 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.12; rv:60.0) Gecko/20100101 Thunderbird/60.6.1 MIME-Version: 1.0 In-Reply-To: <17ed1a12-14df-a501-8b58-f4d1eba49e3f@postgresql.org> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="CzbNLiJ5z6HD5y51KnFOd3b9WqrSyENNv" X-Host-Lookup-Failed: Reverse DNS lookup failed for 63.118.15.50 (failed) List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Precedence: bulk This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --CzbNLiJ5z6HD5y51KnFOd3b9WqrSyENNv Content-Type: multipart/mixed; boundary="iNDBLQfOHylN6cFzfWCKl4bOMYI37WthL"; protected-headers="v1" From: "Jonathan S. Katz" To: Michael Paquier Cc: pgsql-bugs@lists.postgresql.org, Stephen Frost Message-ID: <32661faa-0254-d37e-7930-00d823f38f9f@postgresql.org> Subject: Re: Possible to store invalid SCRAM-SHA-256 Passwords References: <016deb6b-1f0a-8e9f-1833-a8675b170aa9@postgresql.org> <4240a41f-a957-fd7d-7c24-31683f259b9d@postgresql.org> <20190422015051.GA3433@paquier.xyz> <17ed1a12-14df-a501-8b58-f4d1eba49e3f@postgresql.org> In-Reply-To: <17ed1a12-14df-a501-8b58-f4d1eba49e3f@postgresql.org> --iNDBLQfOHylN6cFzfWCKl4bOMYI37WthL Content-Type: multipart/mixed; boundary="------------45D6F424C51D6B46FA9A1BB9" Content-Language: en-US This is a multi-part message in MIME format. --------------45D6F424C51D6B46FA9A1BB9 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: quoted-printable On 4/22/19 9:04 AM, Jonathan S. Katz wrote: > On 4/21/19 9:50 PM, Michael Paquier wrote: >> On Sat, Apr 20, 2019 at 04:12:56PM -0400, Jonathan S. Katz wrote: >>> 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. >> >> Since v10, we don't allow the storage of plain verifiers so if a >> string does not match what we think is a correct SCRAM or MD5 >> verifier, then it should be processed according to >> password_encryption when storing the verifier or processed according >> to the auth protocol with the HBA entry matching. Your patch looks >> fine to me, I would have just added a test case in password.sql (no=20 >> need to send a new patch I can take care of it). >=20 > Thanks for verifying. I'm happy to add the test case -- I first wanted > to ensure I was on the right track. Attached is modified patch with tests included. For the patch against HEAD, I only test to demonstrate that storing an invalid SCRAM-SHA-256 lookalike hash is stored as an actual SCRAM-SHA-256 password, given we do not allow storing passwords UNENCRYPTED anymore (yay!). I followed the testing format that was used in the other cases in the file. What I would love to add as a test would be a function that checks whether or not a user/password combo of something like "jkatz/SCRAM-SHA-256$1234" where the password is stored as plaintext would validate, but I don't believe we expose anything that does that in the SQL API. So I feel this is just a light sanity check. Thanks, Jonathan --------------45D6F424C51D6B46FA9A1BB9 Content-Type: text/plain; charset=UTF-8; x-mac-type="0"; x-mac-creator="0"; name="0001-Modified-SCRAM-SHA-256-validation-check-for-get_pass_v2.patch" Content-Transfer-Encoding: quoted-printable Content-Disposition: attachment; filename*0="0001-Modified-SCRAM-SHA-256-validation-check-for-get_pass_v2"; filename*1=".patch" =46rom 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 passwo= rd started with "SCRAM-SHA-256$" to determine whether or not it was a SCRAM-SHA-256 hashed password. However, this would cause passwords create= d 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 wil= l 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-scra= m.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, con= st char *password, * * Returns true if the SCRAM verifier has been parsed, and false otherwi= se. */ -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 @@ =20 #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) =3D=3D 0 && strlen(shadow_pass) =3D=3D= MD5_PASSWD_LEN) return PASSWORD_TYPE_MD5; - if (strncmp(shadow_pass, "SCRAM-SHA-256$", strlen("SCRAM-SHA-256$")) =3D= =3D 0) + if (strncmp(shadow_pass, "SCRAM-SHA-256$", strlen("SCRAM-SHA-256$")) =3D= =3D 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, =20 /* 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); =20 diff --git a/src/test/regress/expected/password.out b/src/test/regress/ex= pected/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-SH= A-256)\$(\d+):([a-zA-Z0-9+ regress_passwd5 | md5e73a4b11df52a6068f8b39f90be36023 (5 rows) =20 +-- 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+/=3D]+)\$([a-zA-Z0-9+=3D/]+):([a-zA-Z0-9+/=3D]+)', '\1$\2:$= :') as rolpassword_masked + FROM pg_authid + WHERE rolname=3D'regress_passwd6'; + rolname | rolpassword_masked =20 +-----------------+--------------------------------------------------- + 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/pas= sword.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-SH= A-256)\$(\d+):([a-zA-Z0-9+ WHERE rolname LIKE 'regress_passwd%' ORDER BY rolname, rolpassword; =20 +-- 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+/=3D]+)\$([a-zA-Z0-9+=3D/]+):([a-zA-Z0-9+/=3D]+)', '\1$\2:$= :') as rolpassword_masked + FROM pg_authid + WHERE rolname=3D'regress_passwd6'; + -- An empty password is not allowed, in any form CREATE ROLE regress_passwd_empty PASSWORD ''; ALTER ROLE regress_passwd_empty PASSWORD 'md585939a5ce845f1a1b620742e3c6= 59e0a'; @@ -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; =20 -- all entries should have been removed --=20 2.14.3 (Apple Git-98) --------------45D6F424C51D6B46FA9A1BB9-- --iNDBLQfOHylN6cFzfWCKl4bOMYI37WthL-- --CzbNLiJ5z6HD5y51KnFOd3b9WqrSyENNv Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEE+oS2la8r95ogZD/x8QSccp8cZScFAly+KsQACgkQ8QSccp8c ZScoKw//WjYFtEyuwXUgq8fTQ/IOd5GkkqNUDyKEf2XTNVPUEUZXeN4OJPaJgqGI +cNHnLXIJOhPxcLH8LrzJeH2ECbpKA2/vhB3c/ZL0muOAi2geIhARJxIN3CZV9mF yuHhsmR+Xv2+57WuFD58PlSooCqNRl78+cCx1RFTeEszDBR79+r0DNi3qInUBOj4 RF+IJyWtV0WD02KmNh3uCOwQlDTaLf5TL9cRX7O1b/CKY5LH9ycmYYoTrxdUut8E BjB0DWo3PpkKgphwR2oXS1fkYdsNA/z0J57GBSnfO8Uka/C1SyKSbX0FXjMtI+ec sthIFr51I9I3WqLDrOfccPdICUm7MN63tE4nwr/9XClrrGSBWpuMzru0YVNKe50/ RDSns7W3n228VTprXfzM+YrF341DNvPsORTuSa2IW823troIypaZeDMZv2hXpgnL k3fFjcPY8+WmDkBgmuRFk+ugtcNiCkDb3V2S5TRZrunH/dwAUAWlO1OYN42YhpmM t1ovjeSICLr2ogrr5IjRibJdQ/RAXojhzZAg6QfOiUY908v9Jcvaix/XulD51bIv 9bq8lfA/uxeL1uRww72EBrC3/BX3yBIW04xb6YeRNs9rNvbL30TuDJzOCtvB4xCV 0TpSKnwQD3AIMxIRoz2QWZbYfO/P27zIZTCtCm3qYyizn70zhWk= =H8WP -----END PGP SIGNATURE----- --CzbNLiJ5z6HD5y51KnFOd3b9WqrSyENNv--