On Mon, 2021-04-05 at 14:47 +0900, Michael Paquier wrote:
> On Sat, Apr 03, 2021 at 09:30:25PM +0900, Michael Paquier wrote:
> > Slight rebase for this one to take care of the updates with the SSL
> > error messages.
>
> I have been looking again at that and applied it as c50624cd after
> some slight modifications.
This loses the test fixes I made in my v19 [1]; some of the tests on
HEAD aren't testing anything anymore. I've put those fixups into 0001,
attached.
> Attached is the main, refactored, patch
> that plugs on top of the existing infrastructure. connect_ok() and
> connect_fails() gain two parameters each to match or to not match the
> logs of the backend, with a truncation of the logs done before any
> connection attempt.
It looks like this is a reimplementation of v19, but it loses the
additional tests I wrote? Not sure. Maybe my v19 was sent to spam?
In any case I have attached my Friday patch as 0002.
> I have spent more time reviewing the backend code while on it and
> there was one thing that stood out:
> + ereport(FATAL,
> + (errmsg("connection was re-authenticated"),
> + errdetail_log("previous ID: \"%s\"; new ID: \"%s\"",
> + port->authn_id, id)));
> This message would not actually trigger because auth_failed() is the
> code path in charge of showing an error here
It triggers just fine for me (you can duplicate one of the
set_authn_id() calls to see):
FATAL: connection was re-authenticated
DETAIL: previous ID: "uid=test2,dc=example,dc=net"; new ID: "uid=test2,dc=example,dc=net"
> so this could just be
> replaced by an assertion on authn_id being NULL?
An assertion seems like the wrong way to go; in the event that a future
code path accidentally performs a duplicated authentication, the FATAL
will just kill off an attacker's connection, while an assertion will
DoS the server.
> The contents of this
> log were a bit in contradiction with the comments a couple of lines
> above anyway.
What do you mean by this? I took another look at the comment and it
seems to match the implementation.
v21 attached, which is just a rebase of my original v19.
--Jacob
[1] https://www.postgresql.org/message-id/8c08c6402051b5348d599c0e07bbd83f8614fa16.camel%40vmware.com
