From: | Michael Paquier <michael(at)paquier(dot)xyz> |
---|---|
To: | Stephen Frost <sfrost(at)snowman(dot)net> |
Cc: | Jacob Champion <jchampion(at)timescale(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, "Jonathan S(dot) Katz" <jkatz(at)postgresql(dot)org> |
Subject: | Re: Docs: Encourage strong server verification with SCRAM |
Date: | 2023-05-24 01:37:35 |
Message-ID: | ZG1qX8gX9kebf3zC@paquier.xyz |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Tue, May 23, 2023 at 05:02:50PM -0400, Stephen Frost wrote:
> * Jacob Champion (jchampion(at)timescale(dot)com) wrote:
>> As touched on in past threads, our SCRAM implementation is slightly
>> nonstandard and doesn't always protect the entirety of the
>> authentication handshake:
>>
>> - the username in the startup packet is not covered by the SCRAM
>> crypto and can be tampered with if channel binding is not in effect,
>> or by an attacker holding only the server's key
>
> Encouraging channel binding when using TLS certainly makes a lot of
> sense, particularly in environments where the trust anchors are not
> strongly managed (that is- if you trust the huge number of root
> certificates that a system may have installed). Perhaps explicitly
> encouraging users to *not* trust every root server installed on their
> client for their PG connections would also be a good idea. We should
> probably add language to that effect to this section.
Currently the user name is not treated by the backend, like that in
auth-scram.c:
/*
* Read username. Note: this is ignored. We use the username from the
* startup message instead, still it is kept around if provided as it
* proves to be useful for debugging purposes.
*/
state->client_username = read_attr_value(&p, 'n');
Could it make sense to cross-check that with the contents of the
startup package instead, with or without channel binding?
>> - low iteration counts accepted by the client make it easier than it
>> probably should be for a MITM to brute-force passwords (note that
>> PG16's scram_iterations GUC, being server-side, does not mitigate
>> this)
>
> This would be good to improve on.
Hmm. Interesting.
> > - by default, a SCRAM exchange can be exited by the server prior to
> > sending its verifier, skipping the client's server authentication step
> > (this is mitigated by requiring channel binding, and PG16 adds
> > require_auth=scram-sha-256 to help as well)
>
> Yeah, encouraging this would also be good and should be mentioned as
> something to do when one is using SCRAM. Clearly that would go into a
> PG16 version of this.
Agreed to improve the docs about ways to mitigate any risks.
--
Michael
From | Date | Subject | |
---|---|---|---|
Next Message | Andres Freund | 2023-05-24 01:37:59 | Re: unnecessary #include "pg_getopt.h"? |
Previous Message | Michael Paquier | 2023-05-24 01:26:03 | Re: Allow pg_archivecleanup to remove backup history files |