Message-ID: <41a690ee7b030e6f41709bd39375641ef934e05f.camel@j-davis.com>
Subject: Re: [PoC/RFC] Multiple passwords, interval expirations
From: Jeff Davis <pgsql@j-davis.com>
To: Gurjeet Singh <gurjeet@singh.im>, Stephen Frost <sfrost@snowman.net>
Cc: "Brindle, Joshua" <joshuqbr@amazon.com>, Jacob Champion
 <jchampion@timescale.com>, PostgreSQL-development
 <pgsql-hackers@postgresql.org>,  Nathan Bossart <nathandbossart@gmail.com>
Date: Tue, 26 Sep 2023 16:36:31 -0700
In-Reply-To: <CABwTF4XZO_429=QzZHt-1Q=KSCR2XjYG+xVy2p-PdPtun9tVdg@mail.gmail.com>
References: 
	<CAGB+Vh5SQQorNDEKP+0G=smxHRhbhs+VkmQWD5Vh98fmn8X4dg@mail.gmail.com>
	 <CAGB+Vh7aLDrZ4RJbfqJ2u0sz-qvV4V5gtyaGr0Bqkp-Q9ojktg@mail.gmail.com>
	 <CAGB+Vh65WkmR1V5qVUerxmsJE9uBFqwd0oWTTNU=dca_ep2N5Q@mail.gmail.com>
	 <CAGB+Vh5Ed65gCmjJxCYguVoBoiDLePYDAeHMkb6-7PR-DFU-yQ@mail.gmail.com>
	 <b951f0c9-b568-4889-8418-b01cf5a3cea2@timescale.com>
	 <CAOuzzgpyFRFbskZW0uYwP4_r5NkFJywTurn0SQ+pkbcuuQchjw@mail.gmail.com>
	 <CABwTF4Uih+xKc8o7W-Qmzo2Oc3BbvnY+JHzcZ-HG2vDzBQpf8w@mail.gmail.com>
	 <20220701002034.GA9030@tamriel.snowman.net>
	 <80684017-18c6-4116-9249-bb049ffe1ecf@amazon.com>
	 <CAOuzzgq2j43X6vwF8LCPcPuKZ2gw0ZbF=CrsB6CmtEt1h2ci0g@mail.gmail.com>
	 <CABwTF4XZO_429=QzZHt-1Q=KSCR2XjYG+xVy2p-PdPtun9tVdg@mail.gmail.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.44.4-0ubuntu2 
MIME-Version: 1.0
Archived-At: <https://www.postgresql.org/message-id/41a690ee7b030e6f41709bd39375641ef934e05f.camel%40j-davis.com>
Precedence: bulk

On Mon, 2023-09-25 at 00:31 -0700, Gurjeet Singh wrote:

> Please see attached v4 of the patch. The patch takes care of rebase
> to
> the master/17-devel branch, and includes some changes, too.

FWIW I got some failures applying. I didn't investigate much, and
instead I looked at your git branch (7a35619e).

> Moreover, before the patch, in case of CheckPasswordAuth(), the error
> (if any) would have been thrown _after_ network communication done by
> sendAuthRequest() call. But with this patch, the error is thrown
> before the network interaction, hence this changes the order of
> network interaction and the error message. This may have security
> implications, too, but I'm unable to articulate one right now.

You mean before v3 or before v4? Is this currently a problem in v4?

> Open question: If a client is capable of providing just md5 passwords
> handshake, and because of pg_hba.conf setting, or because the role
> has
> at least one SCRAM password (essentially the 3rd case you mention
> above: pg_hba md5 + md5 and scram pws -> scram), the server will
> respond with a SASL/SCRAM authentication response, and that would
> break the backwards compatibility and will deny access to the client.
> Does this make it necessary to use a newer libpq/client library?

Perhaps you can try the MD5 passwords first, and only if they fail,
move on to try scram passwords?

> Comments?

IIUC, for the case of multiple scram passwords, we use the salt to
select the right scram password, and then proceed from there?

I'm not very excited about the idea of naming passwords, or having
passwords with default names. I can't think of anything better right
now, so it might be OK.

> - Add tests
> - Add/update documentation

These are needed to provide better review.


Regards,
	Jeff Davis