Subject: Re: Possible to store invalid SCRAM-SHA-256 Passwords
To: Tom Lane <tgl@sss.pgh.pa.us>, Michael Paquier <michael@paquier.xyz>
Cc: pgsql-bugs@lists.postgresql.org, Stephen Frost <sfrost@snowman.net>
References: <016deb6b-1f0a-8e9f-1833-a8675b170aa9@postgresql.org>
 <4240a41f-a957-fd7d-7c24-31683f259b9d@postgresql.org>
 <20190422015051.GA3433@paquier.xyz> <3033.1555940554@sss.pgh.pa.us>
From: "Jonathan S. Katz" <jkatz@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: <5a93333c-dde6-ee52-f870-4dccfc50eb6f@postgresql.org>
Date: Mon, 22 Apr 2019 18:32:45 -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: <3033.1555940554@sss.pgh.pa.us>
Content-Type: multipart/signed; micalg=pgp-sha256;
 protocol="application/pgp-signature";
 boundary="JzSc1rUru0nhF5CnX64WbqCBHmm8ffFHY"
Precedence: bulk

This is an OpenPGP/MIME signed message (RFC 4880 and 3156)
--JzSc1rUru0nhF5CnX64WbqCBHmm8ffFHY
Content-Type: multipart/mixed; boundary="9F2ponAO2n1Q5hYjHHNRJy3zIwoBHla4M";
 protected-headers="v1"
From: "Jonathan S. Katz" <jkatz@postgresql.org>
To: Tom Lane <tgl@sss.pgh.pa.us>, Michael Paquier <michael@paquier.xyz>
Cc: pgsql-bugs@lists.postgresql.org, Stephen Frost <sfrost@snowman.net>
Message-ID: <5a93333c-dde6-ee52-f870-4dccfc50eb6f@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> <3033.1555940554@sss.pgh.pa.us>
In-Reply-To: <3033.1555940554@sss.pgh.pa.us>

--9F2ponAO2n1Q5hYjHHNRJy3zIwoBHla4M
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: quoted-printable

On 4/22/19 9:42 AM, Tom Lane wrote:
> Michael Paquier <michael@paquier.xyz> writes:
>> 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.
>=20
>> Any objections to back-patch that stuff to v10?
>=20
> Patch looks OK as far as it goes, but ISTM it raises an obvious
> question: shouldn't the immediately-preceding test to see if a
> password is MD5 also be trying harder?  Currently it only checks
> the length, but verifying that the rest is valid hex data would
> go far towards preventing the same set of problems for MD5.
>=20
> You might argue that MD5 is deprecated and we shouldn't expend
> any effort on it, but a simple strspn check would only require
> about one more line ...

OK, so I have something that sort of works, i.e:

if (strncmp(shadow_pass, "md5", 3) =3D=3D 0 &&
    strlen(shadow_pass) =3D=3D MD5_PASSWD_LEN &&
    strspn(shadow_pass, MD5_PASSWD_CHARSET) =3D=3D MD5_PASSWD_LEN
)

where MD5_PASSWD_CHARSET =3D "mabcdef0123456789"

=2E..but you may notice something: the CHARSET contains an "m" as we stor=
e
that "md5" prefix on the md5 hashed passwords.

So this leads to a few options:

1. Make a copy of the shadow password w/o the "md5" prefixed to it then
then perform the strspn sans "m" in the char set.

2. Count how many times "m" appears in the shadow password. If count >
1, then it's clearly an invalid md5 hash.

3. Leave the proposed check as is, knowing that someone could have a
hash like "md51234567890123456789012345678901m" that is borked.

4. Do nothing.

If there is a concise way to do #2 I like that approach; I'm not sure if
we have any helpers to perform counts like that or if I have to write my
own.

My preference is to go down path #2, otherwise I'd vote for #4 given the
(hopefully) planned deprecation.

Thanks,

Jonathan


--9F2ponAO2n1Q5hYjHHNRJy3zIwoBHla4M--

--JzSc1rUru0nhF5CnX64WbqCBHmm8ffFHY
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+QQ0ACgkQ8QSccp8c
ZSeFrRAAuJXGZm5Naf8zGTZ57Xhcxb8uOTVUKdzNXAttsEDmp/YjbyXvel2JkqA8
ASWzhn7zBHjTuEu+OOE4LsabTll6vxsuWayBg3sMcY5wWd92LyIui2prPjdnVuFm
/5ho6kmvC0b2pkUDzqlpNmsVbKHtQTOSL2sdO8xbpXXRqDW/ku6FxG7MLk1qUcV3
F9Fzc2Atii9JdH6PyyiWttGaEUkjDxZzjkSKQHws5PtFjvGmsy2eaChrvQXjr2QM
4bEh7Is4ZMN0RSDC4y9NMApG8HqcPVyY9mMm68DgAUAxRrdyZd0TMbROgvQOMrhd
O13pGoIaP/kErHykR4Dq2ycOsnXGTz/7yTOAqCQeqgFCg+NfBLTtJ8HgrtmBC1j1
5U5nQyWqdcvhV07g4NVcG83KxDr3bWv73J+Pvp82Ukh2WWhwY0KoURMtiGr+FmMu
Rha0JYI6EnyRiT0N2gQcVaydeNNtD4LWccUBnldKW4LkahINVzASK+NOPcBd3sPs
aJWyeY+6bZxTplA7lfquXep516OH8aq4O1RBLvcWq5/dEr6jK0wihMKWv/EGmqX2
4xmr0xFVoww43pR6G53WsH0TcYoqA/LlJ+5p1Vj4xseTNR95tYLe0R9Uvrzumj8F
Do7idPC8syWIrwkaIDY5Ucv4tXGRjyxWNRmLEnWQ+Anjij1nd64=
=DEMg
-----END PGP SIGNATURE-----

--JzSc1rUru0nhF5CnX64WbqCBHmm8ffFHY--