On 22.09.22 01:37, Jacob Champion wrote:
On Wed, Sep 21, 2022 at 3:36 PM Peter Eisentraut
<peter.eisentr...@enterprisedb.com> wrote:
So let's look at the two TODO comments you have:
* TODO: how should !auth_required interact with an incomplete
* SCRAM exchange?
What specific combination of events are you thinking of here?
Let's say the client is using `require_auth=!password`. If the server
starts a SCRAM exchange. but doesn't finish it, the connection will
still succeed with the current implementation (because it satisfies
the "none" case). This is also true for a client setting of
`require_auth=scram-sha-256,none`. I think this is potentially
dangerous, but it mirrors the current behavior of libpq and I'm not
sure that we should change it as part of this patch.
It might be worth reviewing that behavior for other reasons, but I think
semantics of your patch are correct.
In any case, it seems to me that it would be safer to *not*
make this assumption at first and then have someone more knowledgeable
make the argument that it would be safe.
I think I'm okay with that, regardless. Here's one of the wrinkles:
right now, both of the following connstrings work:
require_auth=gss gssencmode=require
require_auth=gss gssencmode=prefer
If we don't treat gssencmode as providing GSS auth, then the first
case will always fail, because there will be no GSS authentication
packet over an encrypted connection. Likewise, the second case will
almost always fail, unless the server doesn't support gssencmode at
all (so why are you using prefer?).
If you're okay with those limitations, I will rip out the code. The
reason I'm not too worried about it is, I don't think it makes much
sense to be strict about your authentication requirements while at the
same time leaving the choice of transport encryption up to the server.
The way I understand what you explained here is that it would be more
sensible to leave that code in. I would be okay with that.
