Re: [PATCH] Include application_name in "connection authorized" log message

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Stephen Frost <sfrost(at)snowman(dot)net>
Cc: Don Seiler <don(at)seiler(dot)us>, Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: [PATCH] Include application_name in "connection authorized" log message
Date: 2018-08-07 14:42:51
Message-ID: 5987.1533652971@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Stephen Frost <sfrost(at)snowman(dot)net> writes:
> * Tom Lane (tgl(at)sss(dot)pgh(dot)pa(dot)us) wrote:
>> Moreover, if you don't check it then the appname recorded
>> by log_connections would not match appearances for the same session
>> later in the log, which puts the entire use-case for this patch into
>> question. So no, this concern must not be dismissed.

> If the call to check_application_name() fails then I had been expecting
> the connection to fail. If we continue to let the connection go on then
> we've already got an issue as someone might pass in an application name
> that isn't being set to the GUC and isn't being therefore used in the
> existing log_line_prefix where it should be.

No, check_application_name doesn't reject funny names, it just silently
modifies them in-place.

>> However ... I've not looked at the patch, but I thought the idea was to
>> allow assignment of that GUC to occur before the log_connections log entry
>> is emitted, so that it'd show up in the entry's log_line_prefix. Wouldn't
>> check_application_name happen automatically at that point?

> We log that message quite early and it certainly didn't look trivial to
> set up the GUC to be already in place at that point, so the plan was to
> simply spit out what gets passed in (as we were doing for "user", if I'm
> remembering that code correctly...).

Hm. Well, the code isn't exactly complicated, you could duplicate it.
Or maybe better refactor to allow it to be called from $wherever.
Looks like check_cluster_name, for one, could also share use of
an ascii-lobotomizing subroutine.

But having said that, I don't exactly see why you couldn't force it
with an ultimately-redundant SetConfigOption call to put the value
in place before the ereport happens. The GUC machinery is surely
functional before we do authorization.

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Stephen Frost 2018-08-07 14:57:59 Re: [PATCH] Include application_name in "connection authorized" log message
Previous Message Stephen Frost 2018-08-07 14:34:31 Re: Negotiating the SCRAM channel binding type