
> > I am not sure whether the feature does not actually present a security 
> > hole ? Two collaborating users can pass each other their privileges.
> 
> I don't see any (new) security risk here.  Code written by one user can
> be executed with the privileges of another --- so what?  That's the
> situation now, with non-setuid functions.

Hmm? A non-setuid function can execute code written by another user,
but is only allowed to do things the "invoker" has privileges for.
Thus it is a convenience, but does not allow the invoker to do anything
he could not type himself.
Not so with setuid functions, that is exactly why they are handy.
Without making the "definer" need an additional grant for creating such 
a function, it would be like giving him all the privs he has 
"with grant option".

> 
> > And why not use the existing "set session authorization ..." syntax?
> 
> That syntax implies setting authorization permanently (for the rest of
> the session).  If we take over that syntax to mean local privilege
> change inside a function, then it'd be impossible to let a function do a
> global change in the future.  Not sure if we ever want that, but I don't
> think we should foreclose the possibility by using the same syntax to
> mean two different things.

Yes, I was not sure about the intended standards scope of a "set session auth..." 
when called inside a function.

Andreas

Mon, 25 Jun 2001 17:00:37 +0200
Zeugswetter Andreas SB  <ZeugswetterA@wien.spardat.at>
AW: AW: AW: [PATCH] Re: Setuid functions 
Zeugswetter Andreas SB  <ZeugswetterA@wien.spardat.at> writes:
> Without making the "definer" need an additional grant for creating such 
> a function, it would be like giving him all the privs he has 
> "with grant option".

Hmm ... interesting analogy, but does it hold water?  The GRANT OPTION
stuff implies the right to pass on your privileges to someone else
*permanently*.  A setuid function only lets someone else do the same
things you can do at the time it is called.  There's nothing there that
couldn't be done by having the one user ask the other to do something
using an outside-the-database communication channel.  So I really don't
see a security issue.

I also don't see any privilege of this type in SQL92 (which does have
the concept of setuid functions, in the form of modules).

			regards, tom lane

Mon, 25 Jun 2001 11:25:11 -0400
Tom Lane <tgl@sss.pgh.pa.us>
Re: AW: AW: AW: [PATCH] Re: Setuid functions 
Tom Lane writes:

> I also don't see any privilege of this type in SQL92 (which does have
> the concept of setuid functions, in the form of modules).

SQL99 has setuid functions in the form of setuid functions, with a syntax
like CREATE FUNCTION .... SECURITY { INVOKER | DEFINER } (too lazy to look
up the details).  There were some peculiar differences IIRC, such as
trigger functions executing with the permission of the trigger creator
(which is yet different).

Modules are more like "packages", AFAICT.

-- 
Peter Eisentraut   peter_e@gmx.net   http://funkturm.homeip.net/~peter


Mon, 25 Jun 2001 18:14:59 +0200 (CEST)
Peter Eisentraut <peter_e@gmx.net>
Re: AW: AW: AW: [PATCH] Re: Setuid functions 
Zeugswetter Andreas SB writes:

> Hmm? A non-setuid function can execute code written by another user,
> but is only allowed to do things the "invoker" has privileges for.
> Thus it is a convenience, but does not allow the invoker to do anything
> he could not type himself.
> Not so with setuid functions, that is exactly why they are handy.
> Without making the "definer" need an additional grant for creating such
> a function, it would be like giving him all the privs he has
> "with grant option".

SQL99 has an answer for this:

[11.49 GR1]

         1) If R is a schema-level routine, then a privilege descriptor
            is created that defines the EXECUTE privilege on R to the
            <authorization identifier> that owns the schema that includes R.
            The grantor for the privilege descriptor is set to the special
            grantor value "_SYSTEM". This privilege is grantable if and only
            if one of the following is satisfied:

            a) R is an SQL routine and all of the privileges necessary
              for the <authorization identifier> to successfully execute
              the <SQL procedure statement> contained in the <routine
              body> are grantable. The necessary privileges include the
              EXECUTE privilege on every subject routine of every <routine
              invocation> contained in the <SQL procedure statement>.

What this means (to me) is that unless you have grantable privileges for
all the things that your function does, you can't grant the EXECUTE
privilege to anyone.

This rule, while logical, isn't exactly pleasant, since we can hardly
evaluate statically what a function will do (shades of the halting
problem).

I think your concern is valid.  Maybe we can do this:

1.  The proposed commands only work in "setuid" functions (like in Unix)

2.  To create a setuid function you need some privilege.

Part 2 will be a problem, but both the implementation process and the
implementation itself might terminate in finite time.  ;-)

-- 
Peter Eisentraut   peter_e@gmx.net   http://funkturm.homeip.net/~peter


Mon, 25 Jun 2001 18:34:03 +0200 (CEST)
Peter Eisentraut <peter_e@gmx.net>
Re: AW: AW: AW: [PATCH] Re: Setuid functions 
I updated the patch to use the SET AUTHORIZATION { INVOKER | DEFINER }
terminology. Also, the function owner is now determined and saved at compile
time (no gotchas here, right?). It is located at

http://volpe.home.mindspring.com/pgsql/set_auth.patch

Mark

Mon, 25 Jun 2001 14:26:17 -0400
Mark Volpe <volpe.mark@epa.gov>
Re: [PATCH] Re: Setuid functions
> I updated the patch to use the SET AUTHORIZATION { INVOKER | DEFINER }
> terminology. Also, the function owner is now determined and saved at compile
> time (no gotchas here, right?). It is located at
> 
> http://volpe.home.mindspring.com/pgsql/set_auth.patch

OK, patch applied.  Can I have some docs with that burger?  :-)

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Index: src/pl/plpgsql/src/gram.y
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/pl/plpgsql/src/gram.y,v
retrieving revision 1.21
diff -c -r1.21 gram.y
*** src/pl/plpgsql/src/gram.y	2001/06/06 18:54:41	1.21
--- src/pl/plpgsql/src/gram.y	2001/07/11 18:37:07
***************
*** 122,132 ****
  %type <stmts>	proc_sect, proc_stmts, stmt_else, loop_body
  %type <stmt>	proc_stmt, pl_block
  %type <stmt>	stmt_assign, stmt_if, stmt_loop, stmt_while, stmt_exit
! %type <stmt>	stmt_return, stmt_raise, stmt_execsql, stmt_fori
  %type <stmt>	stmt_fors, stmt_select, stmt_perform
  %type <stmt>	stmt_dynexecute, stmt_dynfors, stmt_getdiag
  %type <stmt>	stmt_open, stmt_fetch, stmt_close
  
  %type <intlist>	raise_params
  %type <ival>	raise_level, raise_param
  %type <str>		raise_msg
--- 122,134 ----
  %type <stmts>	proc_sect, proc_stmts, stmt_else, loop_body
  %type <stmt>	proc_stmt, pl_block
  %type <stmt>	stmt_assign, stmt_if, stmt_loop, stmt_while, stmt_exit
! %type <stmt>	stmt_return, stmt_raise, stmt_execsql, stmt_fori, stmt_setauth
  %type <stmt>	stmt_fors, stmt_select, stmt_perform
  %type <stmt>	stmt_dynexecute, stmt_dynfors, stmt_getdiag
  %type <stmt>	stmt_open, stmt_fetch, stmt_close
  
+ %type <ival>	auth_level
+ 
  %type <intlist>	raise_params
  %type <ival>	raise_level, raise_param
  %type <str>		raise_msg
***************
*** 172,177 ****
--- 174,183 ----
  %token	K_PERFORM
  %token	K_ROW_COUNT
  %token	K_RAISE
+ %token	K_SET
+ %token	K_AUTHORIZATION
+ %token	K_INVOKER
+ %token	K_DEFINER
  %token	K_RECORD
  %token	K_RENAME
  %token	K_RESULT_OID
***************
*** 726,731 ****
--- 732,739 ----
  						{ $$ = $1; }
  				| stmt_raise
  						{ $$ = $1; }
+ 				| stmt_setauth
+ 						{ $$ = $1; }
  				| stmt_execsql
  						{ $$ = $1; }
  				| stmt_dynexecute
***************
*** 1242,1247 ****
--- 1250,1278 ----
  						$$ = (PLpgSQL_stmt *)new;
  					}
  				;
+ 
+ stmt_setauth		: K_SET K_AUTHORIZATION auth_level lno ';'
+ 				{
+ 					PLpgSQL_stmt_setauth *new;
+ 
+ 					new=malloc(sizeof(PLpgSQL_stmt_setauth));
+ 
+ 					new->cmd_type = PLPGSQL_STMT_SETAUTH;
+ 					new->auth_level = $3;
+                                         new->lineno = $4;
+                                         
+ 					$$ = (PLpgSQL_stmt *)new;
+ 				}
+ 
+ auth_level : K_DEFINER
+ 		{
+ 			$$=PLPGSQL_AUTH_DEFINER;
+                 }
+ 	   | K_INVOKER
+            	{
+                 	$$=PLPGSQL_AUTH_INVOKER;
+                 }
+ ;
  
  stmt_raise		: K_RAISE lno raise_level raise_msg raise_params ';'
  					{
Index: src/pl/plpgsql/src/pl_comp.c
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/pl/plpgsql/src/pl_comp.c,v
retrieving revision 1.31
diff -c -r1.31 pl_comp.c
*** src/pl/plpgsql/src/pl_comp.c	2001/05/21 14:22:18	1.31
--- src/pl/plpgsql/src/pl_comp.c	2001/07/11 18:37:07
***************
*** 169,174 ****
--- 169,175 ----
  
  	function->fn_functype = functype;
  	function->fn_oid = fn_oid;
+         function->definer_uid = procStruct->proowner;
  	function->fn_name = strdup(DatumGetCString(DirectFunctionCall1(nameout,
  								 NameGetDatum(&(procStruct->proname)))));
  
Index: src/pl/plpgsql/src/pl_exec.c
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/pl/plpgsql/src/pl_exec.c,v
retrieving revision 1.44
diff -c -r1.44 pl_exec.c
*** src/pl/plpgsql/src/pl_exec.c	2001/05/28 19:33:24	1.44
--- src/pl/plpgsql/src/pl_exec.c	2001/07/11 18:37:07
***************
*** 47,52 ****
--- 47,53 ----
  #include "plpgsql.h"
  #include "pl.tab.h"
  
+ #include "miscadmin.h"
  #include "access/heapam.h"
  #include "catalog/pg_proc.h"
  #include "catalog/pg_type.h"
***************
*** 105,110 ****
--- 106,113 ----
  			   PLpgSQL_stmt_exit * stmt);
  static int exec_stmt_return(PLpgSQL_execstate * estate,
  				 PLpgSQL_stmt_return * stmt);
+ static int exec_stmt_setauth(PLpgSQL_execstate * estate,
+ 				PLpgSQL_stmt_setauth * stmt);
  static int exec_stmt_raise(PLpgSQL_execstate * estate,
  				PLpgSQL_stmt_raise * stmt);
  static int exec_stmt_execsql(PLpgSQL_execstate * estate,
***************
*** 226,231 ****
--- 229,237 ----
  					case PLPGSQL_STMT_RETURN:
  						stmttype = "return";
  						break;
+ 					case PLPGSQL_STMT_SETAUTH:
+ 						stmttype = "setauth";
+ 						break;
  					case PLPGSQL_STMT_RAISE:
  						stmttype = "raise";
  						break;
***************
*** 277,283 ****
  	estate.retistuple = func->fn_retistuple;
  	estate.retisset = func->fn_retset;
  	estate.exitlabel = NULL;
! 
  	estate.found_varno = func->found_varno;
  	estate.ndatums = func->ndatums;
  	estate.datums = palloc(sizeof(PLpgSQL_datum *) * estate.ndatums);
--- 283,292 ----
  	estate.retistuple = func->fn_retistuple;
  	estate.retisset = func->fn_retset;
  	estate.exitlabel = NULL;
! 	estate.invoker_uid = GetUserId();
! 	estate.definer_uid = func->definer_uid;
! 	estate.auth_level = PLPGSQL_AUTH_INVOKER;
!         
  	estate.found_varno = func->found_varno;
  	estate.ndatums = func->ndatums;
  	estate.datums = palloc(sizeof(PLpgSQL_datum *) * estate.ndatums);
***************
*** 397,402 ****
--- 406,414 ----
  		elog(ERROR, "control reaches end of function without RETURN");
  	}
  
+ 	if (estate.auth_level!=PLPGSQL_AUTH_INVOKER)
+ 		SetUserId(estate.invoker_uid);
+ 
  	/*
  	 * We got a return value - process it
  	 */
***************
*** 577,582 ****
--- 589,597 ----
  	estate.retistuple = func->fn_retistuple;
  	estate.retisset = func->fn_retset;
  	estate.exitlabel = NULL;
+ 	estate.invoker_uid = GetUserId();
+ 	estate.definer_uid = func->definer_uid;
+ 	estate.auth_level = PLPGSQL_AUTH_INVOKER;
  
  	estate.found_varno = func->found_varno;
  	estate.ndatums = func->ndatums;
***************
*** 760,765 ****
--- 775,783 ----
  		elog(ERROR, "control reaches end of trigger procedure without RETURN");
  	}
  
+ 	if (estate.auth_level!=PLPGSQL_AUTH_INVOKER)
+ 		SetUserId(estate.invoker_uid);
+ 
  	/*
  	 * Check that the returned tuple structure has the same attributes,
  	 * the relation that fired the trigger has.
***************
*** 1022,1027 ****
--- 1040,1049 ----
  			rc = exec_stmt_return(estate, (PLpgSQL_stmt_return *) stmt);
  			break;
  
+ 		case PLPGSQL_STMT_SETAUTH:
+ 			rc = exec_stmt_setauth(estate, (PLpgSQL_stmt_setauth *) stmt);
+ 			break;
+ 
  		case PLPGSQL_STMT_RAISE:
  			rc = exec_stmt_raise(estate, (PLpgSQL_stmt_raise *) stmt);
  			break;
***************
*** 1643,1648 ****
--- 1665,1693 ----
  									&(estate->rettype));
  
  	return PLPGSQL_RC_RETURN;
+ }
+ 
+ /* ----------
+  * exec_stmt_setauth            Changes user ID to/from
+  *                              that of the function owner's
+  * ----------
+  */
+ 
+ static int
+ exec_stmt_setauth(PLpgSQL_execstate * estate, PLpgSQL_stmt_setauth * stmt)
+ {
+ 	switch(stmt->auth_level)
+         {
+         	case PLPGSQL_AUTH_DEFINER:
+                 	SetUserId(estate->definer_uid);
+                         break;
+                 case PLPGSQL_AUTH_INVOKER:
+                 	SetUserId(estate->invoker_uid);
+                         break;
+ 	}
+ 
+ 	estate->auth_level=stmt->auth_level;
+ 	return PLPGSQL_RC_OK;
  }
  
  
Index: src/pl/plpgsql/src/pl_funcs.c
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/pl/plpgsql/src/pl_funcs.c,v
retrieving revision 1.13
diff -c -r1.13 pl_funcs.c
*** src/pl/plpgsql/src/pl_funcs.c	2001/05/21 14:22:19	1.13
--- src/pl/plpgsql/src/pl_funcs.c	2001/07/11 18:37:08
***************
*** 382,387 ****
--- 382,388 ----
  static void dump_select(PLpgSQL_stmt_select * stmt);
  static void dump_exit(PLpgSQL_stmt_exit * stmt);
  static void dump_return(PLpgSQL_stmt_return * stmt);
+ static void dump_setauth(PLpgSQL_stmt_setauth * stmt);
  static void dump_raise(PLpgSQL_stmt_raise * stmt);
  static void dump_execsql(PLpgSQL_stmt_execsql * stmt);
  static void dump_dynexecute(PLpgSQL_stmt_dynexecute * stmt);
***************
*** 438,443 ****
--- 439,447 ----
  		case PLPGSQL_STMT_RETURN:
  			dump_return((PLpgSQL_stmt_return *) stmt);
  			break;
+ 		case PLPGSQL_STMT_SETAUTH:
+ 			dump_setauth((PLpgSQL_stmt_setauth *) stmt);
+ 			break;
  		case PLPGSQL_STMT_RAISE:
  			dump_raise((PLpgSQL_stmt_raise *) stmt);
  			break;
***************
*** 719,724 ****
--- 723,743 ----
  			dump_expr(stmt->expr);
  	}
  	printf("\n");
+ }
+ 
+ static void
+ dump_setauth(PLpgSQL_stmt_setauth * stmt)
+ {
+ 	dump_ind();
+         switch (stmt->auth_level)
+         {
+         	case PLPGSQL_AUTH_DEFINER:
+                 	printf("SET AUTHORIZATION DEFINER\n");
+                         break;
+                 case PLPGSQL_AUTH_INVOKER:
+                 	printf("SET AUTHORIZATION INVOKER\n");
+                         break;
+         }
  }
  
  static void
Index: src/pl/plpgsql/src/plpgsql.h
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/pl/plpgsql/src/plpgsql.h,v
retrieving revision 1.14
diff -c -r1.14 plpgsql.h
*** src/pl/plpgsql/src/plpgsql.h	2001/05/21 14:22:19	1.14
--- src/pl/plpgsql/src/plpgsql.h	2001/07/11 18:37:08
***************
*** 95,100 ****
--- 95,101 ----
  	PLPGSQL_STMT_DYNEXECUTE,
  	PLPGSQL_STMT_DYNFORS,
  	PLPGSQL_STMT_GETDIAG,
+ 	PLPGSQL_STMT_SETAUTH,
  	PLPGSQL_STMT_OPEN,
  	PLPGSQL_STMT_FETCH,
  	PLPGSQL_STMT_CLOSE
***************
*** 112,117 ****
--- 113,128 ----
  	PLPGSQL_RC_RETURN
  };
  
+ /* ---------
+  * Authorization levels
+  * ---------
+  */
+ enum
+ {
+ 	PLPGSQL_AUTH_INVOKER,
+         PLPGSQL_AUTH_DEFINER,
+ };
+ 
  /* ----------
   * GET DIAGNOSTICS system attrs
   * ----------
***************
*** 425,430 ****
--- 436,447 ----
  	int			retrecno;
  }			PLpgSQL_stmt_return;
  
+ typedef struct
+ {                               /* SET AUTHORIZATION statement */
+     int         cmd_type;
+     int         lineno;
+     int		auth_level;
+ }           PLpgSQL_stmt_setauth;
  
  typedef struct
  {								/* RAISE statement			*/
***************
*** 480,485 ****
--- 497,503 ----
  	int			tg_nargs_varno;
  
  	int			ndatums;
+         Oid			definer_uid;
  	PLpgSQL_datum **datums;
  	PLpgSQL_stmt_block *action;
  	struct PLpgSQL_function *next;
***************
*** 502,507 ****
--- 520,528 ----
  	int			found_varno;
  	int			ndatums;
  	PLpgSQL_datum **datums;
+ 	Oid		invoker_uid;
+ 	Oid		definer_uid;
+         int		auth_level;
  }			PLpgSQL_execstate;
  
  
Index: src/pl/plpgsql/src/scan.l
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/pl/plpgsql/src/scan.l,v
retrieving revision 1.12
diff -c -r1.12 scan.l
*** src/pl/plpgsql/src/scan.l	2001/05/21 14:22:19	1.12
--- src/pl/plpgsql/src/scan.l	2001/07/11 18:37:08
***************
*** 121,126 ****
--- 121,130 ----
  open			{ return K_OPEN;			}
  perform			{ return K_PERFORM;			}
  raise			{ return K_RAISE;			}
+ set			{ return K_SET;				}
+ authorization		{ return K_AUTHORIZATION;		}
+ invoker			{ return K_INVOKER;			}
+ definer			{ return K_DEFINER;			}
  record			{ return K_RECORD;			}
  rename			{ return K_RENAME;			}
  result_oid		{ return K_RESULT_OID;		}
Wed, 11 Jul 2001 14:53:28 -0400 (EDT)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: [HACKERS] [PATCH] Re: Setuid functions
Bruce Momjian writes:

> > I updated the patch to use the SET AUTHORIZATION { INVOKER | DEFINER }
> > terminology. Also, the function owner is now determined and saved at compile
> > time (no gotchas here, right?). It is located at
> >
> > http://volpe.home.mindspring.com/pgsql/set_auth.patch
>
> OK, patch applied.  Can I have some docs with that burger?  :-)

I think we concluded that this feature introduced a security hole.

-- 
Peter Eisentraut   peter_e@gmx.net   http://funkturm.homeip.net/~peter


Wed, 11 Jul 2001 21:54:02 +0200 (CEST)
Peter Eisentraut <peter_e@gmx.net>
Re: Re: [HACKERS] [PATCH] Re: Setuid functions
> Bruce Momjian writes:
> 
> > > I updated the patch to use the SET AUTHORIZATION { INVOKER | DEFINER }
> > > terminology. Also, the function owner is now determined and saved at compile
> > > time (no gotchas here, right?). It is located at
> > >
> > > http://volpe.home.mindspring.com/pgsql/set_auth.patch
> >
> > OK, patch applied.  Can I have some docs with that burger?  :-)
> 
> I think we concluded that this feature introduced a security hole.

I thought that was addressed in the patch with the mention of:

> > > Also, the function owner is now determined and saved at compile
> > > time (no gotchas here, right?).

Does anyone remember?

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Wed, 11 Jul 2001 15:58:55 -0400 (EDT)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: Re: [HACKERS] [PATCH] Re: Setuid functions
> Bruce Momjian writes:
> 
> > > I updated the patch to use the SET AUTHORIZATION { INVOKER | DEFINER }
> > > terminology. Also, the function owner is now determined and saved at compile
> > > time (no gotchas here, right?). It is located at
> > >
> > > http://volpe.home.mindspring.com/pgsql/set_auth.patch
> >
> > OK, patch applied.  Can I have some docs with that burger?  :-)
> 
> I think we concluded that this feature introduced a security hole.

Peter, I see this in the archives.  Is this it?

	http://fts.postgresql.org/db/mw/msg.html?mid=1022748

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Wed, 11 Jul 2001 16:10:09 -0400 (EDT)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: Re: [HACKERS] [PATCH] Re: Setuid functions
Peter might be referring to this:

http://fts.postgresql.org/db/mw/msg.html?mid=1022775

There was some discussion afterward, but I don't think a definite conclusion
was reached.

Mark

> Peter, I see this in the archives.  Is this it?
> 
>         http://fts.postgresql.org/db/mw/msg.html?mid=1022748
> 
> --
>   Bruce Momjian                        |  http://candle.pha.pa.us
>   pgman@candle.pha.pa.us               |  (610) 853-3000
>   +  If your life is a hard drive,     |  830 Blythe Avenue
>   +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Wed, 11 Jul 2001 16:23:17 -0400
Mark Volpe <volpe.mark@epa.gov>
Re: Re: [HACKERS] [PATCH] Re: Setuid functions
> Peter might be referring to this:
> 
> http://fts.postgresql.org/db/mw/msg.html?mid=1022775
> 
> There was some discussion afterward, but I don't think a definite conclusion
> was reached.

But I see Tom Lane saying he doesn't see a security issue:

	http://fts.postgresql.org/db/mw/msg.html?mid=1022758

I don't pretend to understand it.  Just tell me what to do with the
patch.  :-)  

Maybe back it out until we discuss it more.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Wed, 11 Jul 2001 16:26:20 -0400 (EDT)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: Re: [HACKERS] [PATCH] Re: Setuid functions
I'm going to say apply it, of course! But then, I'd just like to tell the boss
that my app is running on an unmodified PostgreSQL :-)

Mark

Bruce Momjian wrote:
> 
> I don't pretend to understand it.  Just tell me what to do with the
> patch.  :-)
>

Wed, 11 Jul 2001 16:46:32 -0400
Mark Volpe <volpe.mark@epa.gov>
Re: Re: [HACKERS] [PATCH] Re: Setuid functions
Bruce Momjian writes:

> > Peter might be referring to this:
> >
> > http://fts.postgresql.org/db/mw/msg.html?mid=1022775
> >
> > There was some discussion afterward, but I don't think a definite conclusion
> > was reached.
>
> But I see Tom Lane saying he doesn't see a security issue:
>
> 	http://fts.postgresql.org/db/mw/msg.html?mid=1022758
>
> I don't pretend to understand it.  Just tell me what to do with the
> patch.  :-)

The problem with setuid functions in general is that a database user can
effectively re-grant privileges to which he has no grant privileges.
E.g.,

user1=> create table table1 (id int, secret_content text);
user1=> grant select on test to user2;

/* made up the syntax */
user2=> create function testfunc (int) returns text as '
user2'>   begin
user2'>     set authorization definer;
user2'>     return select secret_content from table1 where id = $1;
user2'>   end;' as 'plpgsql';

user3=> select * from table1 where id = 5;
(fails)
user3=> select testfunc(5);
(succeeds)

Tom has a point that as soon as user2 has the select privilege, he can
make a private copy of table1 and send it to user3.

But if you take this attitude you might as well get rid of the
fine-grained privilege system, you'd just need 'select to public'.  Also,
there may be other security or at least auditing mechanisms to supervise
the communication between user2 and user3.  Or maybe user2 and user3 are
just pseudo-users implementing some sort of "least privilege" paranoid
design.

At least we should discuss whether we'd eventually like to have grantable
privileges, and if so, how this would fit in.

-- 
Peter Eisentraut   peter_e@gmx.net   http://funkturm.homeip.net/~peter


Wed, 11 Jul 2001 23:53:28 +0200 (CEST)
Peter Eisentraut <peter_e@gmx.net>
Re: [PATCHES] Re: [PATCH] Re: Setuid functions
Good point. Would the issue be resolved by either:

- Only allowing the database superuser to use this mechanism?
- Allowing it only in trigger functions? (That way a user has to actually own
one of the tables)

Mark

Peter Eisentraut wrote:
> 
> Bruce Momjian writes:
> 
> > > Peter might be referring to this:
> > >
> > > http://fts.postgresql.org/db/mw/msg.html?mid=1022775
> > >
> > > There was some discussion afterward, but I don't think a definite conclusion
> > > was reached.
> >
> > But I see Tom Lane saying he doesn't see a security issue:
> >
> >       http://fts.postgresql.org/db/mw/msg.html?mid=1022758
> >
> > I don't pretend to understand it.  Just tell me what to do with the
> > patch.  :-)
> 
> The problem with setuid functions in general is that a database user can
> effectively re-grant privileges to which he has no grant privileges.
> E.g.,
> 
> user1=> create table table1 (id int, secret_content text);
> user1=> grant select on test to user2;
> 
> /* made up the syntax */
> user2=> create function testfunc (int) returns text as '
> user2'>   begin
> user2'>     set authorization definer;
> user2'>     return select secret_content from table1 where id = $1;
> user2'>   end;' as 'plpgsql';
> 
> user3=> select * from table1 where id = 5;
> (fails)
> user3=> select testfunc(5);
> (succeeds)
> 
> Tom has a point that as soon as user2 has the select privilege, he can
> make a private copy of table1 and send it to user3.
> 
> But if you take this attitude you might as well get rid of the
> fine-grained privilege system, you'd just need 'select to public'.  Also,
> there may be other security or at least auditing mechanisms to supervise
> the communication between user2 and user3.  Or maybe user2 and user3 are
> just pseudo-users implementing some sort of "least privilege" paranoid
> design.
> 
> At least we should discuss whether we'd eventually like to have grantable
> privileges, and if so, how this would fit in.
> 
> --
> Peter Eisentraut   peter_e@gmx.net   http://funkturm.homeip.net/~peter

Wed, 11 Jul 2001 18:06:56 -0400
Mark Volpe <volpe.mark@epa.gov>
Re: [PATCHES] Re: [PATCH] Re: Setuid functions
Mark Volpe writes:

> Good point. Would the issue be resolved by either:
>
> - Only allowing the database superuser to use this mechanism?

If you mean "only allow a superuser do define functions using this
mechanism", that could work.  But it would probably make this feature a
lot less attractive, because any setuid function would have to run with
super powers.

> - Allowing it only in trigger functions? (That way a user has to actually own
> one of the tables)

Your premise is no longer correct in 7.2devel.

>
> Mark
>
> Peter Eisentraut wrote:
> >
> > Bruce Momjian writes:
> >
> > > > Peter might be referring to this:
> > > >
> > > > http://fts.postgresql.org/db/mw/msg.html?mid=1022775
> > > >
> > > > There was some discussion afterward, but I don't think a definite conclusion
> > > > was reached.
> > >
> > > But I see Tom Lane saying he doesn't see a security issue:
> > >
> > >       http://fts.postgresql.org/db/mw/msg.html?mid=1022758
> > >
> > > I don't pretend to understand it.  Just tell me what to do with the
> > > patch.  :-)
> >
> > The problem with setuid functions in general is that a database user can
> > effectively re-grant privileges to which he has no grant privileges.
> > E.g.,
> >
> > user1=> create table table1 (id int, secret_content text);
> > user1=> grant select on test to user2;
> >
> > /* made up the syntax */
> > user2=> create function testfunc (int) returns text as '
> > user2'>   begin
> > user2'>     set authorization definer;
> > user2'>     return select secret_content from table1 where id = $1;
> > user2'>   end;' as 'plpgsql';
> >
> > user3=> select * from table1 where id = 5;
> > (fails)
> > user3=> select testfunc(5);
> > (succeeds)
> >
> > Tom has a point that as soon as user2 has the select privilege, he can
> > make a private copy of table1 and send it to user3.
> >
> > But if you take this attitude you might as well get rid of the
> > fine-grained privilege system, you'd just need 'select to public'.  Also,
> > there may be other security or at least auditing mechanisms to supervise
> > the communication between user2 and user3.  Or maybe user2 and user3 are
> > just pseudo-users implementing some sort of "least privilege" paranoid
> > design.
> >
> > At least we should discuss whether we'd eventually like to have grantable
> > privileges, and if so, how this would fit in.
> >
> > --
> > Peter Eisentraut   peter_e@gmx.net   http://funkturm.homeip.net/~peter
>
>

-- 
Peter Eisentraut   peter_e@gmx.net   http://funkturm.homeip.net/~peter


Thu, 12 Jul 2001 16:51:39 +0200 (CEST)
Peter Eisentraut <peter_e@gmx.net>
Re: [PATCHES] Re: [PATCH] Re: Setuid functions
> Mark Volpe writes:
> 
> > Good point. Would the issue be resolved by either:
> >
> > - Only allowing the database superuser to use this mechanism?
> 
> If you mean "only allow a superuser do define functions using this
> mechanism", that could work.  But it would probably make this feature a
> lot less attractive, because any setuid function would have to run with
> super powers.

Patch backed out until we resolve the security issues.  It remains in
the patch queue.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Thu, 12 Jul 2001 13:44:49 -0400 (EDT)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: [PATCHES] Re: [PATCH] Re: Setuid functions
OK, I finally got around to adding the superuser check to my patch. So I try
to test it and...

ERROR:  Only users with Postgres superuser privilege are permitted to create a
function in the 'plpgsql' language.

D'oh! So, if this is the case, then the last patch should be fine after all.

Mark

Peter Eisentraut wrote:
> 
> If you mean "only allow a superuser do define functions using this
> mechanism", that could work.  But it would probably make this feature a
> lot less attractive, because any setuid function would have to run with
> super powers.
>

Thu, 19 Jul 2001 12:40:13 -0400
Mark Volpe <volpe.mark@epa.gov>
Re: [PATCHES] Re: [PATCH] Re: Setuid functions
Mark Volpe <volpe.mark@epa.gov> writes:
> ERROR:  Only users with Postgres superuser privilege are permitted to create a
> function in the 'plpgsql' language.

> D'oh! So, if this is the case, then the last patch should be fine after all.

No, evidently you broke something, or your plpgsql is installed wrong.

			regards, tom lane

Thu, 19 Jul 2001 13:08:52 -0400
Tom Lane <tgl@sss.pgh.pa.us>
Re: [PATCHES] Re: [PATCH] Re: Setuid functions 
You are right; I have forgotten to create a "trusted" language.

Mark

Tom Lane wrote:
> 
> Mark Volpe <volpe.mark@epa.gov> writes:
> > ERROR:  Only users with Postgres superuser privilege are permitted to create a
> > function in the 'plpgsql' language.
> 
> > D'oh! So, if this is the case, then the last patch should be fine after all.
> 
> No, evidently you broke something, or your plpgsql is installed wrong.
> 
>                         regards, tom lane

Thu, 19 Jul 2001 13:13:44 -0400
Mark Volpe <volpe.mark@epa.gov>
Re: [PATCHES] Re: [PATCH] Re: Setuid functions