Modify backend switch parsing to prevent 'insecure' switches

from being accepted when they are passed from client connection request.
Get rid of a couple that no longer do anything (like -P).

src/backend/postmaster/postmaster.c

@@ -10,7 +10,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/postmaster/postmaster.c,v 1.103 1999/02/21 01:41:43 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/postmaster/postmaster.c,v 1.104 1999/05/22 17:47:50 tgl Exp $
  *
  * NOTES
  *
@@ -191,7 +191,7 @@ static int	ServerSock_UNIX = INVALID_SOCK;		/* stream socket server */
 /*
  * Set by the -o option
  */
-static char ExtraOptions[ARGV_SIZE] = "";
+static char ExtraOptions[MAXPATHLEN] = "";
 
 /*
  * These globals control the behavior of the postmaster in case some
@@ -1398,7 +1398,9 @@ BackendStartup(Port *port)
 }
 
 /*
- * split_opts -- destructively load a string into an argv array
+ * split_opts -- split a string of options and append it to an argv array
+ *
+ * NB: the string is destructively modified!
  *
  * Since no current POSTGRES arguments require any quoting characters,
  * we can use the simple-minded tactic of assuming each set of space-
@@ -1416,41 +1418,39 @@ split_opts(char **argv, int *argcp, char *s)
 	{
 		while (isspace(*s))
 			++s;
-		if (*s)
-			argv[i++] = s;
+		if (*s == '\0')
+			break;
+		argv[i++] = s;
 		while (*s && !isspace(*s))
 			++s;
-		if (isspace(*s))
+		if (*s)
 			*s++ = '\0';
 	}
+
 	*argcp = i;
 }
 
 /*
- * DoBackend -- set up the argument list and perform an execv system call
+ * DoBackend -- set up the backend's argument list and invoke backend main().
+ *
+ * This used to perform an execv() but we no longer exec the backend;
+ * it's the same executable as the postmaster.
  *
  * returns:
  *		Shouldn't return at all.
- *		If execv() fails, return status.
+ *		If PostgresMain() fails, return status.
  */
 static int
 DoBackend(Port *port)
 {
+	char	   *av[ARGV_SIZE * 2];
+	int			ac = 0;
 	char		execbuf[MAXPATHLEN];
-	char		portbuf[ARGV_SIZE];
 	char		debugbuf[ARGV_SIZE];
-	char		ttybuf[ARGV_SIZE + 1];
-	char		protobuf[ARGV_SIZE + 1];
-	char		argbuf[(2 * ARGV_SIZE) + 1];
-
-	/*
-	 * each argument takes at least three chars, so we can't have more
-	 * than ARGV_SIZE arguments in (2 * ARGV_SIZE) chars (i.e.,
-	 * port->options plus ExtraOptions)...
-	 */
-	char	   *av[ARGV_SIZE];
-	char		dbbuf[ARGV_SIZE + 1];
-	int			ac = 0;
+	char		protobuf[ARGV_SIZE];
+	char		dbbuf[ARGV_SIZE];
+	char		optbuf[ARGV_SIZE];
+	char		ttybuf[ARGV_SIZE];
 	int			i;
 	struct timeval now;
 	struct timezone tz;
@@ -1491,9 +1491,11 @@ DoBackend(Port *port)
 	StreamClose(ServerSock_UNIX);
 #endif
 
-	/* Save port for ps status */
+	/* Save port etc. for ps status */
 	MyProcPort = port;
 
+	MyProcPid = getpid();
+
 	/*
 	 * Don't want backend to be able to see the postmaster random number
 	 * generator state.  We have to clobber the static random_seed *and*
@@ -1503,11 +1505,16 @@ DoBackend(Port *port)
 	gettimeofday(&now, &tz);
 	srandom(now.tv_usec);
 
-	/* Now, on to standard postgres stuff */
-
-	MyProcPid = getpid();
+	/* ----------------
+	 * Now, build the argv vector that will be given to PostgresMain.
+	 *
+	 * The layout of the command line is
+	 *		postgres [secure switches] -p databasename [insecure switches]
+	 * where the switches after -p come from the client request.
+	 * ----------------
+	 */
 
-	strncpy(execbuf, Execfile, MAXPATHLEN - 1);
+	StrNCpy(execbuf, Execfile, MAXPATHLEN);
 	av[ac++] = execbuf;
 
 	/*
@@ -1528,9 +1535,6 @@ DoBackend(Port *port)
 	real_argv[0] = Execfile;
 #endif
 
-	/* Tell the backend it is being called from the postmaster */
-	av[ac++] = "-p";
-
 	/*
 	 * Pass the requested debugging level along to the backend.  We
 	 * decrement by one; level one debugging in the postmaster traces
@@ -1538,38 +1542,54 @@ DoBackend(Port *port)
 	 * passed along to the backend.  This allows us to watch only the
 	 * postmaster or the postmaster and the backend.
 	 */
-
 	if (DebugLvl > 1)
 	{
 		sprintf(debugbuf, "-d%d", DebugLvl);
 		av[ac++] = debugbuf;
 	}
 
-	/* Pass the requested debugging output file */
-	if (port->tty[0])
-	{
-		strncpy(ttybuf, port->tty, ARGV_SIZE);
-		av[ac++] = "-o";
-		av[ac++] = ttybuf;
-	}
-
-	/* Tell the backend the descriptor of the fe/be socket */
-	sprintf(portbuf, "-P%d", port->sock);
-	av[ac++] = portbuf;
-
-	StrNCpy(argbuf, port->options, ARGV_SIZE);
-	strncat(argbuf, ExtraOptions, ARGV_SIZE);
-	argbuf[(2 * ARGV_SIZE)] = '\0';
-	split_opts(av, &ac, argbuf);
+	/*
+	 * Pass any backend switches specified with -o in the postmaster's
+	 * own command line.  We assume these are secure.
+	 * (It's OK to mangle ExtraOptions since we are now in the child process;
+	 * this won't change the postmaster's copy.)
+	 */
+	split_opts(av, &ac, ExtraOptions);
 
 	/* Tell the backend what protocol the frontend is using. */
-
 	sprintf(protobuf, "-v%u", port->proto);
 	av[ac++] = protobuf;
 
+	/*
+	 * Tell the backend it is being called from the postmaster,
+	 * and which database to use.  -p marks the end of secure switches.
+	 */
+	av[ac++] = "-p";
+
 	StrNCpy(dbbuf, port->database, ARGV_SIZE);
 	av[ac++] = dbbuf;
 
+	/*
+	 * Pass the (insecure) option switches from the connection request.
+	 */
+	StrNCpy(optbuf, port->options, ARGV_SIZE);
+	split_opts(av, &ac, optbuf);
+
+	/*
+	 * Pass the (insecure) debug output file request.
+	 *
+	 * NOTE: currently, this is useless code, since the backend will not
+	 * honor an insecure -o switch.  I left it here since the backend
+	 * could be modified to allow insecure -o, given adequate checking
+	 * that the specified filename is something safe to write on.
+	 */
+	if (port->tty[0])
+	{
+		StrNCpy(ttybuf, port->tty, ARGV_SIZE);
+		av[ac++] = "-o";
+		av[ac++] = ttybuf;
+	}
+
 	av[ac] = (char *) NULL;
 
 	if (DebugLvl > 1)
@@ -1577,7 +1597,7 @@ DoBackend(Port *port)
 		fprintf(stderr, "%s child[%d]: starting with (",
 				progname, MyProcPid);
 		for (i = 0; i < ac; ++i)
-			fprintf(stderr, "%s, ", av[i]);
+			fprintf(stderr, "%s ", av[i]);
 		fprintf(stderr, ")\n");
 	}
 

src/backend/utils/init/globals.c

@@ -7,7 +7,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/utils/init/globals.c,v 1.28 1999/03/17 22:53:19 momjian Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/utils/init/globals.c,v 1.29 1999/05/22 17:47:46 tgl Exp $
  *
  * NOTES
  *	  Globals used all over the place should be declared here and not
@@ -36,7 +36,6 @@
 #include "catalog/catname.h"
 
 ProtocolVersion FrontendProtocol = PG_PROTOCOL_LATEST;
-int			Portfd = -1;
 
 bool		Noversion = false;
 bool		Quiet = false;
@@ -47,11 +46,11 @@ struct Port *MyProcPort;
 long		MyCancelKey;
 
 char	   *DataDir = NULL;
-
  /*
   * The PGDATA directory user says to use, or defaults to via environment
   * variable.  NULL if no option given and no environment variable set
   */
+
 Relation	reldesc;			/* current relation descriptor */
 
 char		OutputFileName[MAXPGPATH] = "";

src/backend/utils/init/miscinit.c

@@ -7,7 +7,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/utils/init/miscinit.c,v 1.27 1999/05/09 00:54:30 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/utils/init/miscinit.c,v 1.28 1999/05/22 17:47:46 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -37,6 +37,7 @@
 #include "utils/syscache.h"
 
 #include "storage/fd.h"			/* for O_ */
+#include "storage/ipc.h"		/* for proc_exit */
 
 /*
  * EnableAbortEnvVarName 

src/backend/utils/misc/trace.c

@@ -257,9 +257,13 @@ set_option_flag(int flag, int value)
 /*
  * Parse an option string like "name,name+,name-,name=value".
  * Single options are delimited by ',',space,tab,newline or cr.
+ *
+ * If 'secure' is false, the option string came from a remote client via
+ * connection "debug options" field --- do not obey any requests that
+ * might potentially be security loopholes.
  */
 void
-parse_options(char *str)
+parse_options(char *str, bool secure)
 {
 	char	   *s,
 			   *name;
@@ -384,7 +388,7 @@ read_pg_options(SIGNAL_ARGS)
 			p--;
 		*p = '\0';
 		verbose = pg_options[TRACE_VERBOSE];
-		parse_options(buffer);
+		parse_options(buffer, true);
 		verbose |= pg_options[TRACE_VERBOSE];
 		if (verbose || postgres_signal_arg == SIGHUP)
 			tprintf(TRACE_ALL, "read_pg_options: %s", buffer);

src/include/miscadmin.h

@@ -11,7 +11,7 @@
  *
  * Copyright (c) 1994, Regents of the University of California
  *
- * $Id: miscadmin.h,v 1.37 1999/05/03 19:10:09 momjian Exp $
+ * $Id: miscadmin.h,v 1.38 1999/05/22 17:47:47 tgl Exp $
  *
  * NOTES
  *	  some of the information in this file will be moved to
@@ -34,7 +34,6 @@ extern int	PostmasterMain(int argc, char *argv[]);
 /*
  * from utils/init/globals.c
  */
-extern int	Portfd;
 extern bool Noversion;
 extern bool Quiet;
 extern bool QueryCancel;

src/include/utils/trace.h

@@ -33,7 +33,7 @@ extern int	eprintf(const char *fmt,...);
 extern int	option_flag(int flag);
 extern int	set_option_flag(int flag, int value);
 extern void write_syslog(int level, char *line);
-extern void parse_options(char *str);
+extern void parse_options(char *str, bool secure);
 extern void read_pg_options(SIGNAL_ARGS);
 
 /*

src/man/postgres.1

 .\" This is -*-nroff-*-
 .\" XXX standard disclaimer belongs here....
-.\" $Header: /cvsroot/pgsql/src/man/Attic/postgres.1,v 1.15 1999/05/19 23:30:43 tgl Exp $
+.\" $Header: /cvsroot/pgsql/src/man/Attic/postgres.1,v 1.16 1999/05/22 17:47:47 tgl Exp $
 .TH POSTGRESQL UNIX 05/19/99 PostgreSQL PostgreSQL
 .SH NAME
 postgres - the Postgres backend server
@@ -25,13 +25,10 @@ data_directory]
 .BR "-O"
 ]
 [\c
-.BR "-P"
-filedes]
-[\c
 .BR "-Q"
 ]
 [\c
-.BR "-S n_buffers"
+.BR "-S kbytes"
 ]
 [\c
 .BR "-d"
@@ -111,17 +108,10 @@ while a transaction is in progress will probably cause data loss.
 .BR "-O"
 Override restrictions, so system table structures can be modified(pg_*).
 .TP
-.BR "-P" " filedes"
-.IR "filedes"
-specifies the file descriptor that corresponds to the socket (port) on
-which to communicate to the frontend process.  This option is
-.BR not
-useful for interactive use.
-.TP
 .BR "-Q"
 Specifies \*(lqquiet\*(rq mode.
 .TP
-.BR "-S"
+.BR "-S" " kbytes"
 Specifies the amount of memory to be used by internal sorts and hashes
 before resorting to temporary disk files.  The value is specified in
 kilobytes, and defaults to 512 kilobytes.  Note that for a complex query,
@@ -201,11 +191,12 @@ plan types if it has any other alternative.)
 .BR "-i"
 Prevents query execution, but shows the plan tree.
 .TP
-.BR "-p"
+.BR "-p" " databasename"
 Indicates to the backend server that it has been started by a 
 .IR postmaster
 and make different assumptions about buffer pool management, file
-descriptors, etc.
+descriptors, etc.  Switches following -p are restricted to those
+considered "secure".
 .TP
 .BR "-t" "pa[rser]|pl[anner]|e[xecutor]"
 Print timing statistics for each query relating to each of the major

src/man/postmaster.1

 .\" This is -*-nroff-*-
 .\" XXX standard disclaimer belongs here....
-.\" $Header: /cvsroot/pgsql/src/man/Attic/postmaster.1,v 1.16 1999/05/19 23:30:43 tgl Exp $
+.\" $Header: /cvsroot/pgsql/src/man/Attic/postmaster.1,v 1.17 1999/05/22 17:47:48 tgl Exp $
 .TH POSTMASTER UNIX 05/19/99 PostgreSQL PostgreSQL
 .SH "NAME"
 postmaster - run the Postgres postmaster
@@ -90,7 +90,7 @@ set at compile-time is used.
 .IR "n_backends"
 is the maximum number of backend server processes that this postmaster
 is allowed to start.  In the stock configuration, this value defaults
-to 64, and can be set as high as 1024 if your system will support that
+to 32, and can be set as high as 1024 if your system will support that
 many processes. Both the default and upper limit values can be altered
 when building Postgres (see src/include/config.h).
 .TP