Re: [PoC] Let libpq reject unexpected authentication requests

On Thu, Sep 8, 2022 at 6:25 AM Peter Eisentraut
<peter(dot)eisentraut(at)enterprisedb(dot)com> wrote:
> For example, before long someone is going to try putting "ldap" into
> require_auth. The fact that the methods in pg_hba.conf are not what
> libpq sees is not something that was really exposed to users until now.
> "none" vs. "trust" takes advantage of that. But then I think we could
> also make "password" clearer, which surely sounds like any kind of
> password, encrypted or not, and that's also how pg_hba.conf behaves.
> The protocol specification calls that "AuthenticationCleartextPassword";
> maybe we could pick a name based on that.

Sounds fair. "cleartext"? "plaintext"? "plain" (like SASL's PLAIN)?

> And then, what if we add a new method in the future, and someone puts
> that into their connection string. Old clients will just refuse to
> parse that. Ok, that effectively gives you the same behavior as
> rejecting the server's authentication offer. But what about the negated
> version?

I assume the alternative behavior you're thinking of is to ignore
negated "future methods"? I think the danger with that (for a feature
that's supposed to be locking communication down) is that it's not
possible to differentiate between a maybe-future method and a typo. If
I want "!password" because my intention is to disallow a plaintext
exchange, I really don't want "!pasword" to silently allow anything.

> Also, what if we add new SASL methods. How would we modify
> this code to be able to pick and choose and also have backward and
> forward compatible behavior?

On the SASL front: In the back of my head I'd been considering adding
a "sasl:" prefix to "scram-sha-256", so that we have a namespace for
new SASL methods. That would also give us a jumping-off point in the
future if we decide to add SASL method negotiation to the protocol.
What do you think about that?

Backwards compatibility will, I think, be handled trivially by a newer
client. The only way to break backwards compatibility would be to
remove support for a method, which I assume would be independent of
this feature.

Forwards compatibility doesn't seem like something this feature can
add by itself (old clients can't speak new methods). Though we could
backport new method names to allow them to be used in negations, if
maintaining that aspect of compatibility is worth the effort.

> In general, I like this. We just need to think about the above things a
> bit more.

Thanks!

--Jacob