GUC --- prevent non-super user changes - Mailing list pgsql-hackers

Tom Lane wrote:
> The recent change to make log_min_messages SUSET provokes the following
> behavior:
>
> $ export PGOPTIONS="-d 5"
> $ psql
> psql: FATAL:  'log_min_messages': permission denied
> $
>
> Considering that I *am* superuser, this is quite unacceptable.
> If you don't want to revert the change, propose another solution.

Here is a proposed fix for the new SUSET of various variables.  The
solution is to create a new GUC context called PGC_USERLIMIT, which
limits changes by non-super users.  For example, non-super users can
turn on logging, but can't turn it off, and log_min_* logging can have
added output, but not less output.

The first part of the patch prevent client PGOPTIONS from lowering the
debug level.  The second part adds this new GUC context, then allows it
to be set properly.  The tests are in two parts --- the first prevents
non-super users from changing the value inappropriately, and the second
allows postgresql.conf changes to apply to existing backends, i.e.  if
postgresql.conf turns logging off via SET, turning it on via
postgresql.conf should propogate to the client, because the client can't
turn something off that the admin wants turned on --- that is the tricky
part that we have to be able to handle the settings in any order.

Peter, how does this look?  Is reset_val the proper value to test?


--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073
Index: src/backend/tcop/postgres.c
===================================================================
RCS file: /cvsroot/pgsql-server/src/backend/tcop/postgres.c,v
retrieving revision 1.346
diff -c -c -r1.346 postgres.c
*** src/backend/tcop/postgres.c    27 May 2003 17:49:46 -0000    1.346
--- src/backend/tcop/postgres.c    11 Jun 2003 04:29:55 -0000
***************
*** 1921,1927 ****
      bool        secure;
      int            errs = 0;
      int            debug_flag = 0;
!     GucContext    ctx;
      GucSource    gucsource;
      char       *tmp;
      int            firstchar;
--- 1921,1927 ----
      bool        secure;
      int            errs = 0;
      int            debug_flag = 0;
!     GucContext    ctx, debug_context;
      GucSource    gucsource;
      char       *tmp;
      int            firstchar;
***************
*** 1996,2002 ****

      /* all options are allowed until '-p' */
      secure = true;
!     ctx = PGC_POSTMASTER;
      gucsource = PGC_S_ARGV;        /* initial switches came from command line */

      while ((flag = getopt(argc, argv, "A:B:c:CD:d:Eef:FiNOPo:p:S:st:v:W:x:-:")) != -1)
--- 1996,2002 ----

      /* all options are allowed until '-p' */
      secure = true;
!     ctx = debug_context = PGC_POSTMASTER;
      gucsource = PGC_S_ARGV;        /* initial switches came from command line */

      while ((flag = getopt(argc, argv, "A:B:c:CD:d:Eef:FiNOPo:p:S:st:v:W:x:-:")) != -1)
***************
*** 2033,2057 ****

              case 'd':            /* debug level */
                  {
!                     debug_flag = atoi(optarg);
!                     /* Set server debugging level. */
!                     if (atoi(optarg) != 0)
                      {
!                         char       *debugstr = palloc(strlen("debug") + strlen(optarg) + 1);
!
!                         sprintf(debugstr, "debug%s", optarg);
!                         SetConfigOption("log_min_messages", debugstr, ctx, gucsource);
!                         pfree(debugstr);
!
                      }
-                     else
-                         /*
-                          * -d0 allows user to prevent postmaster debug
-                          * from propagating to backend.  It would be nice
-                          * to set it to the postgresql.conf value here.
-                          */
-                         SetConfigOption("log_min_messages", "notice",
-                                         ctx, gucsource);
                  }
                  break;

--- 2033,2066 ----

              case 'd':            /* debug level */
                  {
!                     /*
!                      *    Client option can't decrease debug level.
!                      *    We have to do the test here because we group priv and client
!                      *    set GUC calls below, after we know the final debug value.
!                      */
!                     if (ctx != PGC_BACKEND || atoi(optarg) > debug_flag)
                      {
!                         debug_flag = atoi(optarg);
!                         debug_context = ctx;    /* save context for use below */
!                         /* Set server debugging level. */
!                         if (debug_flag != 0)
!                         {
!                             char       *debugstr = palloc(strlen("debug") + strlen(optarg) + 1);
!
!                             sprintf(debugstr, "debug%s", optarg);
!                             SetConfigOption("log_min_messages", debugstr, ctx, gucsource);
!                             pfree(debugstr);
!
!                         }
!                         else
!                             /*
!                              * -d0 allows user to prevent postmaster debug
!                              * from propagating to backend.  It would be nice
!                              * to set it to the postgresql.conf value here.
!                              */
!                             SetConfigOption("log_min_messages", "notice",
!                                             ctx, gucsource);
                      }
                  }
                  break;

***************
*** 2301,2320 ****


      /*
!      * -d is not the same as setting
!      * log_min_messages because it enables other
!      * output options.
       */
      if (debug_flag >= 1)
!         SetConfigOption("log_connections", "true", ctx, gucsource);
      if (debug_flag >= 2)
!         SetConfigOption("log_statement", "true", ctx, gucsource);
      if (debug_flag >= 3)
!         SetConfigOption("debug_print_parse", "true", ctx, gucsource);
      if (debug_flag >= 4)
!         SetConfigOption("debug_print_plan", "true", ctx, gucsource);
      if (debug_flag >= 5)
!         SetConfigOption("debug_print_rewritten", "true", ctx, gucsource);

      /*
       * Process any additional GUC variable settings passed in startup packet.
--- 2310,2328 ----


      /*
!      * -d is not the same as setting log_min_messages because it enables
!      * other output options.
       */
      if (debug_flag >= 1)
!         SetConfigOption("log_connections", "true", debug_context, gucsource);
      if (debug_flag >= 2)
!         SetConfigOption("log_statement", "true", debug_context, gucsource);
      if (debug_flag >= 3)
!         SetConfigOption("debug_print_parse", "true", debug_context, gucsource);
      if (debug_flag >= 4)
!         SetConfigOption("debug_print_plan", "true", debug_context, gucsource);
      if (debug_flag >= 5)
!         SetConfigOption("debug_print_rewritten", "true", debug_context, gucsource);

      /*
       * Process any additional GUC variable settings passed in startup packet.
Index: src/backend/utils/misc/guc.c
===================================================================
RCS file: /cvsroot/pgsql-server/src/backend/utils/misc/guc.c,v
retrieving revision 1.127
diff -c -c -r1.127 guc.c
*** src/backend/utils/misc/guc.c    28 May 2003 18:19:09 -0000    1.127
--- src/backend/utils/misc/guc.c    11 Jun 2003 04:30:01 -0000
***************
*** 406,416 ****
      },

      {
!         {"log_statement", PGC_SUSET}, &log_statement,
          false, NULL, NULL
      },
      {
!         {"log_duration", PGC_SUSET}, &log_duration,
          false, NULL, NULL
      },
      {
--- 406,416 ----
      },

      {
!         {"log_statement", PGC_USERLIMIT}, &log_statement,
          false, NULL, NULL
      },
      {
!         {"log_duration", PGC_USERLIMIT}, &log_duration,
          false, NULL, NULL
      },
      {
***************
*** 431,449 ****
      },

      {
!         {"log_parser_stats", PGC_SUSET}, &log_parser_stats,
          false, NULL, NULL
      },
      {
!         {"log_planner_stats", PGC_SUSET}, &log_planner_stats,
          false, NULL, NULL
      },
      {
!         {"log_executor_stats", PGC_SUSET}, &log_executor_stats,
          false, NULL, NULL
      },
      {
!         {"log_statement_stats", PGC_SUSET}, &log_statement_stats,
          false, NULL, NULL
      },
  #ifdef BTREE_BUILD_STATS
--- 431,449 ----
      },

      {
!         {"log_parser_stats", PGC_USERLIMIT}, &log_parser_stats,
          false, NULL, NULL
      },
      {
!         {"log_planner_stats", PGC_USERLIMIT}, &log_planner_stats,
          false, NULL, NULL
      },
      {
!         {"log_executor_stats", PGC_USERLIMIT}, &log_executor_stats,
          false, NULL, NULL
      },
      {
!         {"log_statement_stats", PGC_USERLIMIT}, &log_statement_stats,
          false, NULL, NULL
      },
  #ifdef BTREE_BUILD_STATS
***************
*** 797,803 ****
      },

      {
!         {"log_min_error_statement", PGC_SUSET}, &log_min_error_statement_str,
          "panic", assign_min_error_statement, NULL
      },

--- 797,803 ----
      },

      {
!         {"log_min_error_statement", PGC_USERLIMIT}, &log_min_error_statement_str,
          "panic", assign_min_error_statement, NULL
      },

***************
*** 884,890 ****
      },

      {
!         {"log_min_messages", PGC_SUSET}, &log_min_messages_str,
          "notice", assign_log_min_messages, NULL
      },

--- 884,890 ----
      },

      {
!         {"log_min_messages", PGC_USERLIMIT}, &log_min_messages_str,
          "notice", assign_log_min_messages, NULL
      },

***************
*** 1180,1185 ****
--- 1180,1189 ----
                      struct config_string *conf = (struct config_string *) gconf;
                      char       *str;

+                     Assert(conf->gen.context != PGC_USERLIMIT ||
+                            conf->assign_hook == assign_log_min_messages ||
+                            conf->assign_hook == assign_client_min_messages ||
+                            conf->assign_hook == assign_min_error_statement);
                      *conf->variable = NULL;
                      conf->reset_val = NULL;
                      conf->session_val = NULL;
***************
*** 1276,1282 ****
          struct config_generic *gconf = guc_variables[i];

          /* Don't reset non-SET-able values */
!         if (gconf->context != PGC_SUSET && gconf->context != PGC_USERSET)
              continue;
          /* Don't reset if special exclusion from RESET ALL */
          if (gconf->flags & GUC_NO_RESET_ALL)
--- 1280,1288 ----
          struct config_generic *gconf = guc_variables[i];

          /* Don't reset non-SET-able values */
!         if (gconf->context != PGC_SUSET &&
!             gconf->context != PGC_USERLIMIT &&
!             gconf->context != PGC_USERSET)
              continue;
          /* Don't reset if special exclusion from RESET ALL */
          if (gconf->flags & GUC_NO_RESET_ALL)
***************
*** 1810,1815 ****
--- 1816,1822 ----
                  return false;
              }
              break;
+         case PGC_USERLIMIT:    /* USERLIMIT permissions checked below */
          case PGC_USERSET:
              /* always okay */
              break;
***************
*** 1861,1866 ****
--- 1868,1889 ----
                               name);
                          return false;
                      }
+                     /* Limit non-super user changes */
+                     if (record->context == PGC_USERLIMIT &&
+                         source > PGC_S_USERSTART &&
+                         newval < conf->reset_val && !superuser())
+                     {
+                         elog(elevel, "'%s': permission denied\n"
+                                 "Only super-users can set this value to false.",
+                                 name);
+                         return false;
+                     }
+                     /* Allow admin change to override user change */
+                     if (!DoIt && record->context == PGC_USERLIMIT &&
+                         source < PGC_S_USERSTART &&
+                         record->reset_source > PGC_S_USERSTART &&
+                         newval > conf->reset_val && !superuser())
+                             DoIt = true;
                  }
                  else
                  {
***************
*** 1932,1937 ****
--- 1955,1976 ----
                               name, newval, conf->min, conf->max);
                          return false;
                      }
+                     /* Limit non-super user changes */
+                     if (record->context == PGC_USERLIMIT &&
+                         source > PGC_S_USERSTART &&
+                         newval < conf->reset_val && !superuser())
+                     {
+                         elog(elevel, "'%s': permission denied\n"
+                                 "Only super-users can increase this value.",
+                                 name);
+                         return false;
+                     }
+                     /* Allow admin change to override user change */
+                     if (!DoIt && record->context == PGC_USERLIMIT &&
+                         source < PGC_S_USERSTART &&
+                         record->reset_source > PGC_S_USERSTART &&
+                         newval > conf->reset_val && !superuser())
+                             DoIt = true;
                  }
                  else
                  {
***************
*** 2003,2008 ****
--- 2042,2063 ----
                               name, newval, conf->min, conf->max);
                          return false;
                      }
+                     /* Limit non-super user changes */
+                     if (record->context == PGC_USERLIMIT &&
+                         source > PGC_S_USERSTART &&
+                         newval < conf->reset_val && !superuser())
+                     {
+                         elog(elevel, "'%s': permission denied\n"
+                                 "Only super-users can increase this value.",
+                                 name);
+                         return false;
+                     }
+                     /* Allow admin change to override user change */
+                     if (!DoIt && record->context == PGC_USERLIMIT &&
+                         source < PGC_S_USERSTART &&
+                         record->reset_source > PGC_S_USERSTART &&
+                         newval > conf->reset_val && !superuser())
+                             DoIt = true;
                  }
                  else
                  {
***************
*** 2067,2072 ****
--- 2122,2151 ----
                          elog(elevel, "out of memory");
                          return false;
                      }
+
+                     if (*conf->variable)
+                     {
+                         int old_int_value, new_int_value;
+
+                         /* Limit non-super user changes */
+                         assign_msglvl(&old_int_value, conf->reset_val, true, interactive);
+                         assign_msglvl(&new_int_value, newval, true, interactive);
+                         if (record->context == PGC_USERLIMIT &&
+                             source > PGC_S_USERSTART &&
+                             new_int_value > old_int_value && !superuser())
+                         {
+                             elog(elevel, "'%s': permission denied\n"
+                                         "Only super-users can increase this value.",
+                                         name);
+                             return false;
+                         }
+                         /* Allow admin change to override user change */
+                         if (!DoIt && record->context == PGC_USERLIMIT &&
+                             source < PGC_S_USERSTART &&
+                             record->reset_source > PGC_S_USERSTART &&
+                             newval > conf->reset_val && !superuser())
+                                 DoIt = true;
+                         }
                  }
                  else if (conf->reset_val)
                  {
***************
*** 3454,3457 ****


  #include "guc-file.c"
-
--- 3533,3535 ----
Index: src/include/utils/guc.h
===================================================================
RCS file: /cvsroot/pgsql-server/src/include/utils/guc.h,v
retrieving revision 1.31
diff -c -c -r1.31 guc.h
*** src/include/utils/guc.h    6 May 2003 20:26:28 -0000    1.31
--- src/include/utils/guc.h    11 Jun 2003 04:30:02 -0000
***************
*** 48,53 ****
--- 48,56 ----
   * be set in the connection startup packet, because when it is processed
   * we don't yet know if the user is a superuser.
   *
+  * USERLIMIT options can only be manipulated in certain ways by
+  * non-super users.
+  *
   * USERSET options can be set by anyone any time.
   */
  typedef enum
***************
*** 57,62 ****
--- 60,66 ----
      PGC_SIGHUP,
      PGC_BACKEND,
      PGC_SUSET,
+     PGC_USERLIMIT,
      PGC_USERSET
  } GucContext;

***************
*** 76,86 ****
      PGC_S_ENV_VAR = 1,            /* postmaster environment variable */
      PGC_S_FILE = 2,                /* postgresql.conf */
      PGC_S_ARGV = 3,                /* postmaster command line */
!     PGC_S_DATABASE = 4,            /* per-database setting */
!     PGC_S_USER = 5,                /* per-user setting */
!     PGC_S_CLIENT = 6,            /* from client connection request */
!     PGC_S_OVERRIDE = 7,            /* special case to forcibly set default */
!     PGC_S_SESSION = 8            /* SET command */
  } GucSource;


--- 80,95 ----
      PGC_S_ENV_VAR = 1,            /* postmaster environment variable */
      PGC_S_FILE = 2,                /* postgresql.conf */
      PGC_S_ARGV = 3,                /* postmaster command line */
!     PGC_S_USERSTART=4,            /*
!                                  *    Settings below are controlled by users.
!                                  *    This is used by PGC_USERLIMT to prevent
!                                  *    non-super users from changing certain settings.
!                                  */
!     PGC_S_DATABASE = 5,            /* per-database setting */
!     PGC_S_USER = 6,                /* per-user setting */
!     PGC_S_CLIENT = 7,            /* from client connection request */
!     PGC_S_OVERRIDE = 8,            /* special case to forcibly set default */
!     PGC_S_SESSION = 9            /* SET command */
  } GucSource;