Thread: Possible to store invalid SCRAM-SHA-256 Passwords
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.
Jonathan
Attachment
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
Attachment
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 need to send a new patch I can take care of it). Any objections to back-patch that stuff to v10? -- Michael
Attachment
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 > need to send a new patch I can take care of it). Thanks for verifying. I'm happy to add the test case -- I first wanted to ensure I was on the right track. > Any objections to back-patch that stuff to v10? +1; I did not try it out, but am very confident that scenario #2 would demonstrate the bug exists in 10 as well. Thanks, Jonathan
Attachment
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.
> Any objections to back-patch that stuff to v10?
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.
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 ...
regards, tom lane
Greetings, * Tom Lane (tgl@sss.pgh.pa.us) 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. > > > Any objections to back-patch that stuff to v10? > > 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. > > 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 ... I agree we should also handle md5 better. I realize this needs to be back-patched and so we have to deal with the existing catalog structure, but this really screams out, in my mind anyway, that we shouldn't have ever tried to just stash the password-encoding-type into the password field and that we should have pulled it out into its own column, so that we aren't having to guess about things as important as a password. I recall having exactly that debate when SCRAM was being worked on and the push-back basically being that it was more work and we'd have to have additional syntax for ALTER USER, et al. I wish I had had more time to spend on that discussion. Water under the bridge now, but hopefully we learn from this and maybe someone refactors how this works sometime soon (or, at least, whenever we add the next password encoding). Thanks! Stephen
Attachment
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. > >> Any objections to back-patch that stuff to v10? > > 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. > > 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 ... Yeah, but at least if people look back at it as a reference for how to properly(?) validate various password hashes it would look better. If it was any higher effort, I'd say "no" given the status of md5, but this is fairly innocuous. I'm happy to put something together in a separate patch. I can take a look later today. Jonathan
Attachment
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 >> need to send a new patch I can take care of it). > > 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
Attachment
Stephen Frost wrote: > I agree we should also handle md5 better. I realize this needs to be > back-patched and so we have to deal with the existing catalog structure, > but this really screams out, in my mind anyway, that we shouldn't have > ever tried to just stash the password-encoding-type into the password > field and that we should have pulled it out into its own column, so that > we aren't having to guess about things as important as a password. > > Thanks! > > Stephen I don't think there's anything wrong with prefixing a password hash with an identifier for the password hashing scheme (and any parameters for that scheme). This is done all the time in many systems. It just has to be unambiguoous.
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.
>
>> Any objections to back-patch that stuff to v10?
>
> 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.
>
> 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) == 0 &&
strlen(shadow_pass) == MD5_PASSWD_LEN &&
strspn(shadow_pass, MD5_PASSWD_CHARSET) == MD5_PASSWD_LEN
)
where MD5_PASSWD_CHARSET = "mabcdef0123456789"
...but you may notice something: the CHARSET contains an "m" as we store
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
Attachment
"Jonathan S. Katz" <jkatz@postgresql.org> writes:
> OK, so I have something that sort of works, i.e:
> if (strncmp(shadow_pass, "md5", 3) == 0 &&
> strlen(shadow_pass) == MD5_PASSWD_LEN &&
> strspn(shadow_pass, MD5_PASSWD_CHARSET) == MD5_PASSWD_LEN
> )
> where MD5_PASSWD_CHARSET = "mabcdef0123456789"
> ...but you may notice something: the CHARSET contains an "m" as we store
> that "md5" prefix on the md5 hashed passwords.
Yeah, that's silly; why not
strspn(shadow_pass + 3, MD5_PASSWD_CHARSET) == MD5_PASSWD_LEN - 3
It's not like this code isn't very well aware of the first 3 characters
being not like the others.
regards, tom lane
On 4/22/19 6:42 PM, Tom Lane wrote: > "Jonathan S. Katz" <jkatz@postgresql.org> writes: >> OK, so I have something that sort of works, i.e: > >> if (strncmp(shadow_pass, "md5", 3) == 0 && >> strlen(shadow_pass) == MD5_PASSWD_LEN && >> strspn(shadow_pass, MD5_PASSWD_CHARSET) == MD5_PASSWD_LEN >> ) > >> where MD5_PASSWD_CHARSET = "mabcdef0123456789" > >> ...but you may notice something: the CHARSET contains an "m" as we store >> that "md5" prefix on the md5 hashed passwords. > > Yeah, that's silly; why not > > strspn(shadow_pass + 3, MD5_PASSWD_CHARSET) == MD5_PASSWD_LEN - 3 > > It's not like this code isn't very well aware of the first 3 characters > being not like the others. I like that :) Please see attached patch, which is diff'd from the one upthread. I tested using the following: /* Log in with "abc" */ CREATE ROLE test1 PASSWORD 'md5cdde562ece166a02f5392b656dcf2502' LOGIN; /* Logs in with "md5cdde562ece166a02f5392b656dcf250g" */ CREATE ROLE test2 PASSWORD 'md5cdde562ece166a02f5392b656dcf250g' LOGIN; /* Logs in with "md5cdde562ece166a02f5392b656dcf250m" */ CREATE ROLE test3 PASSWORD 'md5cdde562ece166a02f5392b656dcf250m' LOGIN; I debated adding a test...without being able to simulate a log in, I don't know if it tests much other than "yes, you can store an invalid md5 hash and it treats it as plaintext." Thanks, Jonathan
Attachment
"Jonathan S. Katz" <jkatz@postgresql.org> writes:
> I debated adding a test...without being able to simulate a log in, I
> don't know if it tests much other than "yes, you can store an invalid
> md5 hash and it treats it as plaintext."
You could probably build something using TAP testing, cf
src/test/authentication/t/001_password.pl
But I really doubt it's worth the trouble ...
regards, tom lane
On Mon, Apr 22, 2019 at 09:52:15AM -0400, Stephen Frost wrote: > I recall having exactly that debate when SCRAM was being worked on and > the push-back basically being that it was more work and we'd have to > have additional syntax for ALTER USER, et al. I wish I had had more > time to spend on that discussion. Water under the bridge now, but > hopefully we learn from this and maybe someone refactors how this works > sometime soon (or, at least, whenever we add the next password > encoding). I am not sure that this would have been more work for ALTER TABLE as we could have relied on just password_encryption to do the work as we do now. The reluctance was to have more additional columns in pg_authid as far as I recall, and I sided with having a separate catalog, and more independent verifier type checks in the catalogs, as you may recall, which would have also eased password rollups for a given role. -- Michael
Attachment
On Mon, Apr 22, 2019 at 07:36:45PM -0400, Jonathan S. Katz wrote:
> On 4/22/19 6:42 PM, Tom Lane wrote:
>> Yeah, that's silly; why not
>>
>> strspn(shadow_pass + 3, MD5_PASSWD_CHARSET) == MD5_PASSWD_LEN - 3
>>
>> It's not like this code isn't very well aware of the first 3 characters
>> being not like the others.
>
> I like that :) Please see attached patch, which is diff'd from the one
> upthread.
That's exactly what I would have done for that. However...
+ if (strncmp(shadow_pass, "md5", 3) == 0 && strlen(shadow_pass) == MD5_PASSWD_LEN &&
+ strspn(shadow_pass + 3, MD5_PASSWD_CHARSET) == MD5_PASSWD_LEN - 3)
return PASSWORD_TYPE_MD5;
There is no point for the second strlen() check, as strspn does the
same work.
Also, the extra SELECT query with regexp_replace() is a bit overkill
for the purpose, and copying again a copy of the regexp around is no
fun.
In short, I would simplify things as the attached. What do you think?
--
Michael
Attachment
Michael Paquier <michael@paquier.xyz> writes:
> There is no point for the second strlen() check, as strspn does the
> same work.
Um, no --- the strspn call will count the number of bytes of hex
data, but without also checking strlen, you don't know that there's
not non-hex trailing junk.
regards, tom lane
On 4/22/19 9:10 PM, Tom Lane wrote: > Michael Paquier <michael@paquier.xyz> writes: >> There is no point for the second strlen() check, as strspn does the >> same work. > > Um, no --- the strspn call will count the number of bytes of hex > data, but without also checking strlen, you don't know that there's > not non-hex trailing junk. +1; that's why I left the comparison in. (e.g. "md512345678901234567890123456789012zzz" would pass without strlen). Jonathan
Attachment
On 4/22/19 9:01 PM, Michael Paquier wrote: > > Also, the extra SELECT query with regexp_replace() is a bit overkill > for the purpose, and copying again a copy of the regexp around is no > fun. > > In short, I would simplify things as the attached. What do you think? Outside of the strlen comments, I do like the changes to the tests, +1 I did purposely keep the SCRAM + MD5 changes as separate patches as I felt they were both atomic, but I have no strong opinions on that. Thanks, Jonathan
Attachment
"Jonathan S. Katz" <jkatz@postgresql.org> writes:
> I did purposely keep the SCRAM + MD5 changes as separate patches as I
> felt they were both atomic, but I have no strong opinions on that.
FWIW, it looks like one patch to me: tighten our checks on whether
supposedly-encoded passwords fit the encoding rules.
regards, tom lane
On Mon, Apr 22, 2019 at 09:16:49PM -0400, Jonathan S. Katz wrote: > +1; that's why I left the comparison in. > > (e.g. "md512345678901234567890123456789012zzz" would pass without strlen). That's a hard morning... Yes you are right and I can see the failure. By the way, grouping everything in one patch looks more adapted to me as this tightens all the checks for the different verifier types. -- Michael
Attachment
On Tue, Apr 23, 2019 at 11:10:18AM +0900, Michael Paquier wrote: > That's a hard morning... Yes you are right and I can see the failure. > By the way, grouping everything in one patch looks more adapted to me > as this tightens all the checks for the different verifier types. The afternoon has been better. I have double-checked your patch and committed it down to v10. Now, there are two things which may need extra handling: - Do we add a note in the release notes about that with a SQL query checking the state of pg_authid? - In ~9.6 we include in md5.h a macro which does not care about hex characters in the MD5 hash. I think that we should fix that as well, or perhaps that's not worth caring per the lack of complaints? Attached is what would be needed. -- Michael
Attachment
On 4/23/19 2:57 AM, Michael Paquier wrote: > On Tue, Apr 23, 2019 at 11:10:18AM +0900, Michael Paquier wrote: >> That's a hard morning... Yes you are right and I can see the failure. >> By the way, grouping everything in one patch looks more adapted to me >> as this tightens all the checks for the different verifier types. > > The afternoon has been better. I have double-checked your patch and > committed it down to v10. Thanks! > Now, there are two things which may need > extra handling: > - Do we add a note in the release notes about that with a SQL query > checking the state of pg_authid? Hmm...well, let's sound it out: - If you have an invalid SCRAM-SHA-256 password (e.g. SCRAM-SHA-256$1234), you would have been unable to log in anyway, so in all likelihood you would either have had an admin reset your password, or you gave up. - If you had a md5 hash with bogus characters in it, it'd be the above as well So likely it's been resolved in some way: the user has been issued a new password or has given up on PostgreSQL With that said, we could do something like: "To determine if this release affects an of your users ability to log in using either the SCRAM-SHA-256 on MD5 password based methods, you can run the following query: <query that finds the now invalid hashes> We advise that you reset the passwords for these users. " > - In ~9.6 we include in md5.h a macro which does not care about hex > characters in the MD5 hash. I think that we should fix that as well, > or perhaps that's not worth caring per the lack of complaints? > Attached is what would be needed. +1 for fixing so its consistent (at least from a behavior standpoint). I confirmed that it's in 9.5 & 9.4 as well. Thanks, Jonathan
Attachment
Greetings, * raf@raf.org (raf@raf.org) wrote: > Stephen Frost wrote: > > I agree we should also handle md5 better. I realize this needs to be > > back-patched and so we have to deal with the existing catalog structure, > > but this really screams out, in my mind anyway, that we shouldn't have > > ever tried to just stash the password-encoding-type into the password > > field and that we should have pulled it out into its own column, so that > > we aren't having to guess about things as important as a password. > > I don't think there's anything wrong with prefixing a > password hash with an identifier for the password > hashing scheme (and any parameters for that scheme). > This is done all the time in many systems. It just has > to be unambiguoous. There isn't a way to make it unambiguous given that we accept more-or-less anything as a plaintext password though, that would be the issue here.. Thanks! Stephen
Attachment
Greetings, * Michael Paquier (michael@paquier.xyz) wrote: > On Mon, Apr 22, 2019 at 09:52:15AM -0400, Stephen Frost wrote: > > I recall having exactly that debate when SCRAM was being worked on and > > the push-back basically being that it was more work and we'd have to > > have additional syntax for ALTER USER, et al. I wish I had had more > > time to spend on that discussion. Water under the bridge now, but > > hopefully we learn from this and maybe someone refactors how this works > > sometime soon (or, at least, whenever we add the next password > > encoding). > > I am not sure that this would have been more work for ALTER TABLE as > we could have relied on just password_encryption to do the work as we > do now. The reluctance was to have more additional columns in > pg_authid as far as I recall, and I sided with having a separate > catalog, and more independent verifier type checks in the catalogs, as > you may recall, which would have also eased password rollups for a > given role. Yes, having an indepedent catalog table would have been a good approach too, much better than where we're at now. I hope someone has time to work on that for a future version. Thanks! Stephen
Attachment
Stephen Frost <sfrost@snowman.net> writes:
> * raf@raf.org (raf@raf.org) wrote:
>> I don't think there's anything wrong with prefixing a
>> password hash with an identifier for the password
>> hashing scheme (and any parameters for that scheme).
>> This is done all the time in many systems. It just has
>> to be unambiguoous.
> There isn't a way to make it unambiguous given that we accept
> more-or-less anything as a plaintext password though, that would be the
> issue here..
In practice, particularly with the extra validation we just added,
it seems vanishingly unlikely that anyone would choose a password
that just happened to look like one of the hashed formats.
If somebody intentionally chooses such a password, well, it's on
their heads whether the outcome is what they want.
regards, tom lane
On Tue, Apr 23, 2019 at 10:19:30AM -0400, Jonathan S. Katz wrote: > - If you have an invalid SCRAM-SHA-256 password (e.g. > SCRAM-SHA-256$1234), you would have been unable to log in anyway, so in > all likelihood you would either have had an admin reset your password, > or you gave up. > - If you had a md5 hash with bogus characters in it, it'd be the above > as well > > So likely it's been resolved in some way: the user has been issued a new > password or has given up on PostgreSQL > > With that said, we could do something like: > > "To determine if this release affects an of your users ability to log in > using either the SCRAM-SHA-256 on MD5 password based methods, you can > run the following query: s/an of/any of/. > We advise that you reset the passwords for these users. Sounds fine to me, thanks. I am not sure if we would want to have something in the release notes, on a wiki page with the release notes including a link to it, or just no direct mention in the release notes. In the past, say for the issue with the incorrect VM page references, we have been a wiki page with queries and such for diagnostics. > +1 for fixing so its consistent (at least from a behavior standpoint). > > I confirmed that it's in 9.5 & 9.4 as well. Thanks for confirming, I am going to patch 9.4~9.6 with that. -- Michael
Attachment
On Wed, Apr 24, 2019 at 08:12:16AM +0900, Michael Paquier wrote: > Thanks for confirming, I am going to patch 9.4~9.6 with that. And done. -- Michael