Printing backtrace of postgres processes

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
47 messages Options
123
Reply | Threaded
Open this post in threaded view
|

Re: Printing backtrace of postgres processes

vignesh C
On Mon, Feb 1, 2021 at 11:04 AM Bharath Rupireddy
<[hidden email]> wrote:

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

This will just print the backtrace of the current backend. Users
cannot get password information from this. This backtrace will be sent
from customer side to the customer support. Developers will use gdb to
check the file and line number using the addresses. We are suggesting
to use gdb to get the file and line number from the address.  Users
can attach gdb to the process even now without this feature, I think
that behavior will be the same as is.  That will not be impacted
because of this feature. It was discussed to keep the checks similar
to pg_terminate_backend as discussed in [1].
[1] https://www.postgresql.org/message-id/CA%2BTgmoZ8XeQVCCqfq826kAr804a1ZnYy46FnQB9r2n_iOofh8Q%40mail.gmail.com

Regards,
Vignesh


Reply | Threaded
Open this post in threaded view
|

Re: Printing backtrace of postgres processes

Tom Lane-2
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?)

                        regards, tom lane


Reply | Threaded
Open this post in threaded view
|

Re: Printing backtrace of postgres processes

vignesh C
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?

Regards,
Vignesh


Reply | Threaded
Open this post in threaded view
|

Re: Printing backtrace of postgres processes

Bharath Rupireddy
On Wed, Feb 3, 2021 at 1:49 PM vignesh C <[hidden email]> wrote:

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

First of all, we need to see if the output of pg_print_backtrace shows
up function parameter addresses or only function start addresses along
with line and file information when attached to gdb. In either case,
IMO, it will be easy for experienced hackers(I'm not one though) to
calculate and fetch the query string or other function parameters or
the variables inside the functions from the stack by looking at the
code (which is available openly, of course).

Say, if a backend is in a long running scan or insert operation, then
pg_print_backtrace is issued from another session, the
exec_simple_query function shows up query_string. Below is captured
from attached gdb though, I'm not sure whether the logged backtrace
will have function address or the function parameters addresses, I
think we can check that by having a long running query which
frequently checks interrupts and issue pg_print_backtrace from another
session to that backend. Now, attach gdb to the backend in which the
query is running, then take bt, see if the logged backtrace and the
gdb bt have the same or closer addresses.

#13 0x00005644f4320729 in exec_simple_query (
    query_string=0x5644f6771bf0 "select pg_backend_pid();") at postgres.c:1240
#14 0x00005644f4324ff4 in PostgresMain (argc=1, argv=0x7ffd819bd5e0,
    dbname=0x5644f679d2b8 "postgres", username=0x5644f679d298 "bharath")
    at postgres.c:4394
#15 0x00005644f4256f9d in BackendRun (port=0x5644f67935c0) at postmaster.c:4484
#16 0x00005644f4256856 in BackendStartup (port=0x5644f67935c0) at
postmaster.c:4206
#17 0x00005644f4252a11 in ServerLoop () at postmaster.c:1730
#18 0x00005644f42521aa in PostmasterMain (argc=3, argv=0x5644f676b1f0)
    at postmaster.c:1402
#19 0x00005644f4148789 in main (argc=3, argv=0x5644f676b1f0) at main.c:209

As suggested by Tom, I'm okay if this function is callable only by the
superusers. In that case, the superusers can fetch the backtrace and
send it for further analysis in case of any hangs or issues.

Others may have better thoughts.

With Regards,
Bharath Rupireddy.
EnterpriseDB: http://www.enterprisedb.com


Reply | Threaded
Open this post in threaded view
|

Re: Printing backtrace of postgres processes

torikoshia
In reply to this post by vignesh C
Hi,

I also think this feature would be useful when supporting
environments that lack debugger or debug symbols.
I think such environments are not rare.


+        <xref linkend="runtime-config-logging"/> for more information.
This
+        will help in identifying where exactly the backend process is
currently
+        executing.

When I read this, I expected a backtrace would be generated at
the moment when it receives the signal, but actually it just
sets a flag that causes the next CHECK_FOR_INTERRUPTS to print
a backtrace.

How about explaining the timing of the backtrace generation?


+        print backtrace of superuser backends. This feature is not
supported
+        for postmaster, logging and statistics process.

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.


diff --git a/src/backend/postmaster/checkpointer.c
b/src/backend/postmaster/checkpointer.c
index 54a818b..5fae328 100644
--- a/src/backend/postmaster/checkpointer.c
+++ b/src/backend/postmaster/checkpointer.c
@@ -57,6 +57,7 @@
  #include "storage/shmem.h"
  #include "storage/smgr.h"
  #include "storage/spin.h"
+#include "tcop/tcopprot.h"
  #include "utils/guc.h"
  #include "utils/memutils.h"
  #include "utils/resowner.h"
@@ -547,6 +548,13 @@ HandleCheckpointerInterrupts(void)
         if (ProcSignalBarrierPending)
                 ProcessProcSignalBarrier();

+       /* Process printing backtrace */
+       if (PrintBacktracePending)
+       {
+               PrintBacktracePending = false;
+               set_backtrace(NULL, 0);
+       }
+

Although it implements backtrace for checkpointer, when
I specified pid of checkpointer it was refused from
BackendPidGetProc().


Regards,

--
Atsushi Torikoshi
NTT DATA CORPORATION


Reply | Threaded
Open this post in threaded view
|

Re: Printing backtrace of postgres processes

Bharath Rupireddy
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

With Regards,
Bharath Rupireddy.
EnterpriseDB: http://www.enterprisedb.com


Reply | Threaded
Open this post in threaded view
|

Re: Printing backtrace of postgres processes

torikoshia
On 2021-03-04 21:55, Bharath Rupireddy wrote:

> 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

Thanks!
I'm going to join the discussion there.


Regards,

--
Atsushi Torikoshi
NTT DATA CORPORATION


123