Date: Sat, 17 Oct 2020 12:59:26 -0300
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
To: Julien Rouhaud <rjuju123@gmail.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, Bruce Momjian <bruce@momjian.us>,
	Robert Haas <robertmhaas@gmail.com>,
	Michael Paquier <michael@paquier.xyz>,
	torikoshia <torikoshia@oss.nttdata.com>,
	Atsushi Torikoshi <atorik@gmail.com>,
	Tatsuro Yamada <tatsuro.yamada.tf@nttcom.co.jp>,
	Tomas Vondra <tomas.vondra@2ndquadrant.com>,
	Evgeny Efimkin <efimkin@yandex-team.ru>,
	PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: Feature improvement: can we add queryId for
 pg_catalog.pg_stat_activity view?
Message-ID: <20201017155926.GA11995@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: 
 <CAOBaU_ZxQXd6DuvvhesCBcFHsGnRUX0b-SmR+mTFAEE0a41AVg@mail.gmail.com>
User-Agent: Mutt/1.10.1 (2018-07-13)
Precedence: bulk

On 2020-Oct-17, Julien Rouhaud wrote:

> On Sat, Oct 17, 2020 at 12:23 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:

> > then there's a potential security issue if the GUC is USERSET level:
> > a user could hide her queries from pg_stat_statement by turning the
> > GUC off.  So this line of thought suggests the GUC needs to be at
> > least SUSET, and maybe higher ... doesn't pg_stat_statement need it
> > to have the same value cluster-wide?
> 
> Well, I don't think that there's any guarantee that pg_stat_statemens
> will display all activity that has been run, since there's a limited
> amount of (userid, dbid, queryid) that can be stored, but I agree that
> allowing random user to hide their activity isn't nice.  Note that I
> defined the GUC as SUSET, but maybe it should be SIGHUP?

I don't think we should consider pg_stat_statement a bulletproof defense
for security problems.  It is already lossy by design.

I do think it'd be preferrable if we allowed it to be disabled at the
config file level only, not with SET (prevent users from hiding stuff);
but I think it is useful to allow users to enable it for specific
queries or for specific sessions only, while globally disabled.  This
might mean we need to mark it PGC_SIGHUP and then have the check hook
disallow it from being changed under such-and-such conditions.