Printing backtrace of postgres processes

>
> On Mon, Feb 1, 2021 at 6:14 AM Bharath Rupireddy
> <[hidden email]> wrote:
> > On Fri, Jan 29, 2021 at 7:10 PM vignesh C <[hidden email]> wrote:
> > > > 4) How about following
> > > > +                     errmsg("must be a superuser to print backtrace
> > > > of backend process")));
> > > > instead of
> > > > +                     errmsg("must be a superuser to print backtrace
> > > > of superuser query process")));
> > > >
> > >
> > > Here the message should include superuser, we cannot remove it. Non
> > > super user can log non super user provided if user has permissions for
> > > it.
> > >
> > > > 5) How about following
> > > >                      errmsg("must be a member of the role whose backed
> > > > process's backtrace is being printed or member of
> > > > pg_signal_backend")));
> > > > instead of
> > > > +                     errmsg("must be a member of the role whose
> > > > backtrace is being logged or member of pg_signal_backend")));
> > > >
> > >
> > > Modified it.
> >
> > Maybe I'm confused here to understand the difference between
> > SIGNAL_BACKEND_NOSUPERUSER and SIGNAL_BACKEND_NOPERMISSION macros and
> > corresponding error messages. Some clarification/use case to know in
> > which scenarios we hit those error messages might help me. Did we try
> > to add test cases that show up these error messages for
> > pg_print_backtrace? If not, can we add?
>
> Are these superuser and permission checks enough from a security
> standpoint that we don't expose some sensitive information to the
> user? Although I'm not sure, say from the backtrace printed and
> attached to GDB, can users see the passwords or other sensitive
> information from the system that they aren't supposed to see?
>
> I'm sure this point would have been discussed upthread.

>
> vignesh C <[hidden email]> writes:
> > On Mon, Feb 1, 2021 at 11:04 AM Bharath Rupireddy
> > <[hidden email]> wrote:
> >> Are these superuser and permission checks enough from a security
> >> standpoint that we don't expose some sensitive information to the
> >> user?
>
> > This will just print the backtrace of the current backend. Users
> > cannot get password information from this.
>
> Really?
>
> A backtrace normally exposes the text of the current query, for
> instance, which could contain very sensitive data (passwords in ALTER
> USER, customer credit card numbers in ordinary data, etc etc).  We
> don't allow the postmaster log to be seen by any but very privileged
> users; it's not sane to think that this data is any less
> security-critical than the postmaster log.
>
> This point is entirely separate from the question of whether
> triggering stack traces at inopportune moments could cause system
> malfunctions, but that question is also not to be ignored.
>
> TBH, I'm leaning to the position that this should be superuser
> only.  I do NOT agree with the idea that ordinary users should
> be able to trigger it, even against backends theoretically
> belonging to their own userid.  (Do I need to point out that
> some levels of the call stack might be from security-definer
> functions with more privilege than the session's nominal user?)
>

>
> On Wed, Feb 3, 2021 at 1:00 PM Tom Lane <[hidden email]> wrote:
> >
> > vignesh C <[hidden email]> writes:
> > > On Mon, Feb 1, 2021 at 11:04 AM Bharath Rupireddy
> > > <[hidden email]> wrote:
> > >> Are these superuser and permission checks enough from a security
> > >> standpoint that we don't expose some sensitive information to the
> > >> user?
> >
> > > This will just print the backtrace of the current backend. Users
> > > cannot get password information from this.
> >
> > Really?
> >
> > A backtrace normally exposes the text of the current query, for
> > instance, which could contain very sensitive data (passwords in ALTER
> > USER, customer credit card numbers in ordinary data, etc etc).  We
> > don't allow the postmaster log to be seen by any but very privileged
> > users; it's not sane to think that this data is any less
> > security-critical than the postmaster log.
> >
> > This point is entirely separate from the question of whether
> > triggering stack traces at inopportune moments could cause system
> > malfunctions, but that question is also not to be ignored.
> >
> > TBH, I'm leaning to the position that this should be superuser
> > only.  I do NOT agree with the idea that ordinary users should
> > be able to trigger it, even against backends theoretically
> > belonging to their own userid.  (Do I need to point out that
> > some levels of the call stack might be from security-definer
> > functions with more privilege than the session's nominal user?)
> >
>
> I had seen that the log that will be logged will be something like:
>         postgres: test postgres [local]
> idle(ProcessClientReadInterrupt+0x3a) [0x9500ec]
>         postgres: test postgres [local] idle(secure_read+0x183) [0x787f43]
>         postgres: test postgres [local] idle() [0x7919de]
>         postgres: test postgres [local] idle(pq_getbyte+0x32) [0x791a8e]
>         postgres: test postgres [local] idle() [0x94fc16]
>         postgres: test postgres [local] idle() [0x950099]
>         postgres: test postgres [local] idle(PostgresMain+0x6d5) [0x954bd5]
>         postgres: test postgres [local] idle() [0x898a09]
>         postgres: test postgres [local] idle() [0x89838f]
>         postgres: test postgres [local] idle() [0x894953]
>         postgres: test postgres [local] idle(PostmasterMain+0x116b) [0x89422a]
>         postgres: test postgres [local] idle() [0x79725b]
>         /lib64/libc.so.6(__libc_start_main+0xf5) [0x7f6e68d75555]
>         postgres: test postgres [local] idle() [0x484249]
> I was not sure if we would be able to get any secure information from
> this. I did not notice the function arguments being printed. I felt
> the function name, offset  and the return address will be logged. I
> might be missing something here.
> Thoughts?

> Since the current patch use BackendPidGetProc(), it does not
> support this feature not only postmaster, logging, and
> statistics but also checkpointer, background writer, and
> walwriter.
>
> And when I specify pid of these PostgreSQL processes, it
> says "PID xxxx is not a PostgreSQL server process".
>
> I think it may confuse users, so it might be worth
> changing messages for those PostgreSQL processes.
> AuxiliaryPidGetProc() may help to do it.

> On Mon, Mar 1, 2021 at 10:43 AM torikoshia <[hidden email]>
> wrote:
>> Since the current patch use BackendPidGetProc(), it does not
>> support this feature not only postmaster, logging, and
>> statistics but also checkpointer, background writer, and
>> walwriter.
>>
>> And when I specify pid of these PostgreSQL processes, it
>> says "PID xxxx is not a PostgreSQL server process".
>>
>> I think it may confuse users, so it might be worth
>> changing messages for those PostgreSQL processes.
>> AuxiliaryPidGetProc() may help to do it.
>
> Exactly this was the doubt I got when I initially reviewed this patch.
> And I felt it should be discussed in a separate thread, you may want
> to update your thoughts there [1].
>
> [1] -
> https://www.postgresql.org/message-id/CALj2ACW7Rr-R7mBcBQiXWPp%3DJV5chajjTdudLiF5YcpW-BmHhg%40mail.gmail.com