Re: [PoC] Let libpq reject unexpected authentication requests

On 10/5/22 06:33, Peter Eisentraut wrote:
> I think it would be good to put some provisions in place here, even if 
> they are elementary.  Otherwise, there will be a significant burden on 
> the person who implements the next SASL method (i.e., you ;-) ) to 
> figure that out then.

Sounds good, I'll work on that. v10 does not yet make changes in this area.

> I think you could just stick a string list of allowed SASL methods into 
> PGconn.
> 
> By the way, I'm not sure all the bit fiddling is really worth it.  An 
> array of integers (or unsigned char or whatever) would work just as 
> well.  Especially if you are going to have a string list for SASL 
> anyway.  You're not really saving any bits or bytes either way in the 
> normal case.

Yeah, with the SASL case added in, the bitmasks might not be long for
this world. It is nice to be able to invert the whole thing, but a
separate boolean saying "invert the list" could accomplish the same goal
and I think we'll need to have that for the SASL mechanism list anyway.

> Minor comments:
> 
> Pasting together error messages like with auth_description() isn't going 
> to work.  You either need to expand the whole message in 
> check_expected_areq(), or perhaps rephrase the message like
> 
> libpq_gettext("auth method \"%s\" required, but server requested \"%s\"\n"),
>                               conn->require_auth,
>                               auth_description(areq)
> 
> and make auth_description() just return a single word not subject to 
> translation.

Right. Michael tried to warn me about that upthread, but I only ended up
fixing one of the two error cases for some reason. I've merged the two
into one code path for v10.

Quick error messaging bikeshed: do you prefer

    auth method "!password,!md5" requirement failed: ...

or

    auth method requirement "!password,!md5" failed: ...

?

> spurious whitespace change in fe-secure-openssl.c

Fixed.

> whitespace error in patch:
> 
> .git/rebase-apply/patch:109: tab in indent.
>                          via TLS, nor GSS authentication via its 
> encrypted transport.)

Fixed.

> In the 0002 patch, the configure test needs to be added to meson.build.

Added.

Thanks,
--Jacob