Re: [HACKERS] [PATCH] Re: Setuid functions

I am backing out this SET AUTHORIZATION patch until we can resolve the
security issues. It will remain in the patch queue at:

http://candle.pha.pa.us/cgi-bin/pgpatches

> > 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(at)candle(dot)pha(dot)pa(dot)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; }

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