On Thu, Sep 22, 2022 at 4:52 AM Peter Eisentraut
<peter.eisentraut@enterprisedb.com> wrote:
> On 22.09.22 01:37, Jacob Champion wrote:
> > 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.
Sounds good. v9 removes the TODO and adds a better explanation.
> > If you're okay with those [GSS] 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.
I've added a comment there explaining the gssencmode interaction. That
leaves no TODOs inside the code itself.
I removed the commit message note about not being able to prevent
unexpected client cert requests or GSS encryption, since we've decided
to handle those cases outside of require_auth.
I'm not able to test SSPI easily at the moment; if anyone is able to
try it out, that'd be really helpful. There's also the question of
SASL forwards compatibility -- if someone adds a new SASL mechanism,
the code will treat it like scram-sha-256 until it's changed, and
there will be no test to catch it. Should we leave that to the future
mechanism implementer to fix, or add a mechanism check now so the
client is safe even if they forget?
Thanks!
--Jacob
