On Sat, Aug 23, 2014 at 3:44 PM, Amit Kapila <amit.kapil...@gmail.com> wrote:
> On Tue, Aug 5, 2014 at 8:04 PM, Fujii Masao <masao.fu...@gmail.com> wrote:
>>
>> Yep, the attached patch introduces PGC_SU_BACKEND and
>> changes the contexts of log_connections and log_disconnections
>> to PGC_SU_BACKEND. Review?
>>
Thanks for reviewing the patch!
> 1.
> ! else if (context != PGC_POSTMASTER && context != PGC_SU_BACKEND &&
> ! context != PGC_SU_BACKEND && source != PGC_S_CLIENT)
>
> In the above check for PGC_SU_BACKEND is repeated, here
> one of the check should be PGC_SU_BACKEND and other
> should be PGC_BACKEND.
Right. Fixed. Attached is the updated version of the patch.
BTW, I also added the following into the document of log_connections
and log_disconnections.
Only superusers can change this setting at session start.
> 2.
> + case PGC_SU_BACKEND:
> + if (context == PGC_BACKEND)
> + {
> ..
> ..
> + return 0;
> + }
> case PGC_BACKEND:
> if (context == PGC_SIGHUP)
>
> Changing PGC_SU_BACKEND parameter (log_connections) is
> visible even with a non-super user client due to above code.
> Shouldn't it be only visible for super-user logins?
>
> Simple steps to reproduce the problem:
> a. start Server (default configuration)
> b. connect with superuser
> c. change in log_connections to on in postgresql.conf
> d. perform select pg_reload_conf();
> e. connect with non-super-user
> f. show log_connections; --This step shows the value as on,
> --whereas I think it should have been
> off
In this case, log_connections is changed in postgresql.conf and it's
reloaded, so ISTM that it's natural that even non-superuser sees the
changed value. No? Maybe I'm missing something.
Regards,
--
Fujii Masao
*** a/doc/src/sgml/config.sgml
--- b/doc/src/sgml/config.sgml
***************
*** 4231,4236 **** local0.* /var/log/postgresql
--- 4231,4237 ----
<para>
Causes each attempted connection to the server to be logged,
as well as successful completion of client authentication.
+ Only superusers can change this setting at session start.
This parameter cannot be changed after session start.
The default is off.
</para>
***************
*** 4258,4263 **** local0.* /var/log/postgresql
--- 4259,4265 ----
<varname>log_connections</varname> but at session termination,
and includes the duration of the session. This is off by
default.
+ Only superusers can change this setting at session start.
This parameter cannot be changed after session start.
</para>
</listitem>
*** a/src/backend/tcop/postgres.c
--- b/src/backend/tcop/postgres.c
***************
*** 3258,3264 **** get_stats_option_name(const char *arg)
* argv[0] is ignored in either case (it's assumed to be the program name).
*
* ctx is PGC_POSTMASTER for secure options, PGC_BACKEND for insecure options
! * coming from the client, or PGC_SUSET for insecure options coming from
* a superuser client.
*
* If a database name is present in the command line arguments, it's
--- 3258,3264 ----
* argv[0] is ignored in either case (it's assumed to be the program name).
*
* ctx is PGC_POSTMASTER for secure options, PGC_BACKEND for insecure options
! * coming from the client, or PGC_SU_BACKEND for insecure options coming from
* a superuser client.
*
* If a database name is present in the command line arguments, it's
*** a/src/backend/utils/init/postinit.c
--- b/src/backend/utils/init/postinit.c
***************
*** 957,963 **** process_startup_options(Port *port, bool am_superuser)
GucContext gucctx;
ListCell *gucopts;
! gucctx = am_superuser ? PGC_SUSET : PGC_BACKEND;
/*
* First process any command-line switches that were included in the
--- 957,963 ----
GucContext gucctx;
ListCell *gucopts;
! gucctx = am_superuser ? PGC_SU_BACKEND : PGC_BACKEND;
/*
* First process any command-line switches that were included in the
*** a/src/backend/utils/misc/guc.c
--- b/src/backend/utils/misc/guc.c
***************
*** 509,514 **** const char *const GucContext_Names[] =
--- 509,515 ----
/* PGC_INTERNAL */ "internal",
/* PGC_POSTMASTER */ "postmaster",
/* PGC_SIGHUP */ "sighup",
+ /* PGC_SU_BACKEND */ "superuser-backend",
/* PGC_BACKEND */ "backend",
/* PGC_SUSET */ "superuser",
/* PGC_USERSET */ "user"
***************
*** 907,913 **** static struct config_bool ConfigureNamesBool[] =
NULL, NULL, NULL
},
{
! {"log_connections", PGC_BACKEND, LOGGING_WHAT,
gettext_noop("Logs each successful connection."),
NULL
},
--- 908,914 ----
NULL, NULL, NULL
},
{
! {"log_connections", PGC_SU_BACKEND, LOGGING_WHAT,
gettext_noop("Logs each successful connection."),
NULL
},
***************
*** 916,922 **** static struct config_bool ConfigureNamesBool[] =
NULL, NULL, NULL
},
{
! {"log_disconnections", PGC_BACKEND, LOGGING_WHAT,
gettext_noop("Logs end of a session, including duration."),
NULL
},
--- 917,923 ----
NULL, NULL, NULL
},
{
! {"log_disconnections", PGC_SU_BACKEND, LOGGING_WHAT,
gettext_noop("Logs end of a session, including duration."),
NULL
},
***************
*** 5685,5700 **** set_config_option(const char *name, const char *value,
* signals to individual backends only.
*/
break;
case PGC_BACKEND:
if (context == PGC_SIGHUP)
{
/*
! * If a PGC_BACKEND parameter is changed in the config file,
! * we want to accept the new value in the postmaster (whence
! * it will propagate to subsequently-started backends), but
! * ignore it in existing backends. This is a tad klugy, but
! * necessary because we don't re-read the config file during
! * backend start.
*
* In EXEC_BACKEND builds, this works differently: we load all
* nondefault settings from the CONFIG_EXEC_PARAMS file during
--- 5686,5710 ----
* signals to individual backends only.
*/
break;
+ case PGC_SU_BACKEND:
+ if (context == PGC_BACKEND)
+ {
+ ereport(elevel,
+ (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+ errmsg("permission denied to set parameter \"%s\"",
+ name)));
+ return 0;
+ }
case PGC_BACKEND:
if (context == PGC_SIGHUP)
{
/*
! * If a PGC_SU_BACKEND or PGC_BACKEND parameter is changed in
! * the config file, we want to accept the new value in the
! * postmaster (whence it will propagate to subsequently-started
! * backends), but ignore it in existing backends. This is a tad
! * klugy, but necessary because we don't re-read the config file
! * during backend start.
*
* In EXEC_BACKEND builds, this works differently: we load all
* nondefault settings from the CONFIG_EXEC_PARAMS file during
***************
*** 5714,5720 **** set_config_option(const char *name, const char *value,
#endif
}
else if (context != PGC_POSTMASTER && context != PGC_BACKEND &&
! source != PGC_S_CLIENT)
{
ereport(elevel,
(errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM),
--- 5724,5730 ----
#endif
}
else if (context != PGC_POSTMASTER && context != PGC_BACKEND &&
! context != PGC_SU_BACKEND && source != PGC_S_CLIENT)
{
ereport(elevel,
(errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM),
***************
*** 8357,8364 **** read_nondefault_variables(void)
GucContext varscontext;
/*
! * Assert that PGC_BACKEND case in set_config_option() will do the right
! * thing.
*/
Assert(IsInitProcessingMode());
--- 8367,8374 ----
GucContext varscontext;
/*
! * Assert that PGC_SU_BACKEND and PGC_BACKEND case in set_config_option()
! * will do the right thing.
*/
Assert(IsInitProcessingMode());
*** a/src/include/utils/guc.h
--- b/src/include/utils/guc.h
***************
*** 36,47 ****
* certain point in their main loop. It's safer to wait than to read a
* file asynchronously.)
*
! * BACKEND options can only be set at postmaster startup, from the
* configuration file, or by client request in the connection startup
! * packet (e.g., from libpq's PGOPTIONS variable). Furthermore, an
! * already-started backend will ignore changes to such an option in the
! * configuration file. The idea is that these options are fixed for a
! * given backend once it's started, but they can vary across backends.
*
* SUSET options can be set at postmaster startup, with the SIGHUP
* mechanism, or from SQL if you're a superuser.
--- 36,52 ----
* certain point in their main loop. It's safer to wait than to read a
* file asynchronously.)
*
! * SU_BACKEND options can only be set at postmaster startup, from the
* configuration file, or by client request in the connection startup
! * packet (e.g., from libpq's PGOPTIONS variable) if you're a superuser.
! * Furthermore, an already-started backend will ignore changes to
! * such an option in the configuration file. The idea is that these options
! * are fixed for a given backend once it's started, but they can vary
! * across backends.
! *
! * BACKEND options are the same as SU_BACKEND ones, but they can
! * be set by client request in the connection startup packet even when
! * you're not a superuser.
*
* SUSET options can be set at postmaster startup, with the SIGHUP
* mechanism, or from SQL if you're a superuser.
***************
*** 53,58 **** typedef enum
--- 58,64 ----
PGC_INTERNAL,
PGC_POSTMASTER,
PGC_SIGHUP,
+ PGC_SU_BACKEND,
PGC_BACKEND,
PGC_SUSET,
PGC_USERSET
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
