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

Greetings,

* Tom Lane (tgl(at)sss(dot)pgh(dot)pa(dot)us) wrote:
> Stephen Frost <sfrost(at)snowman(dot)net> writes:
> > * Don Seiler (don(at)seiler(dot)us) wrote:
> >> Is the concern that any user can set their client's application name value
> >> to any string they want? Is there a reason we can't call
> >> check_application_name() before setting it in the Port struct in
> >> postmaster.c?
>
> > I've not looked very closely, but I don't think it's necessairly a big
> > deal to print out the application name provided by the client system
> > into the log before we run check_application_name(), as long as there
> > isn't any risk that printing it out or passing it around without
> > performing that check will cause incorrect operation or such.
>
> I think the issue is exactly that putting a malformed appname into the
> postmaster log could confuse log-reading apps (eg by causing encoding
> problems).

Evidently, I need to go look at this, since it seems like this would be
the exact same risk with "user", which is part of why I wasn't terribly
concerned about it here. Considering the concern raised here, if we're
serious about that issue, then I wonder if we have an existing issue.

> 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.

> 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...).

Thanks!

Stephen