Re: [HACKERS] [PATCH] Re: Setuid functions
Re: [HACKERS] [PATCH] Re: Setuid functions
От:
Bruce Momjian <pgman@candle.pha.pa.us>
Дата:
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@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 proc_sect, proc_stmts, stmt_else, loop_body
> %type proc_stmt, pl_block
> %type stmt_assign, stmt_if, stmt_loop, stmt_while, stmt_exit
> ! %type stmt_return, stmt_raise, stmt_execsql, stmt_fori
> %type stmt_fors, stmt_select, stmt_perform
> %type stmt_dynexecute, stmt_dynfors, stmt_getdiag
> %type stmt_open, stmt_fetch, stmt_close
>
> %type raise_params
> %type raise_level, raise_param
> %type raise_msg
> --- 122,134 ----
> %type proc_sect, proc_stmts, stmt_else, loop_body
> %type proc_stmt, pl_block
> %type stmt_assign, stmt_if, stmt_loop, stmt_while, stmt_exit
> ! %type stmt_return, stmt_raise, stmt_execsql, stmt_fori, stmt_setauth
> %type stmt_fors, stmt_select, stmt_perform
> %type stmt_dynexecute, stmt_dynfors, stmt_getdiag
> %type stmt_open, stmt_fetch, stmt_close
>
> + %type auth_level
> +
> %type raise_params
> %type raise_level, raise_param
> %type 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@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
Re: [HACKERS] [PATCH] Re: Setuid functions
От:
Mark Volpe <volpe.mark@epa.gov>
Дата:
Might as well just get rid of that one; Peter's right about the security hole. The simplest fix I see is to allow SET AUTHORIZATION only in superuser-owned functions. It would still be potentially useful that way. Doing this the "right" way (with users needing regrantable privileges, etc.) would involve too much effort for too little reward, IMHO. Mark Bruce Momjian wrote: > > 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 >
Re: [HACKERS] [PATCH] Re: Setuid functions
От:
Mark Volpe <volpe.mark@epa.gov>
Дата:
Might as well just get rid of that one; Peter's right about the security hole. The simplest fix I see is to allow SET AUTHORIZATION only in superuser-owned functions. It would still be potentially useful that way. Doing this the "right" way (with users needing regrantable privileges, etc.) would involve too much effort for too little reward, IMHO. (Repost - I don't think this made it to the list) Mark Bruce Momjian wrote: > > 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 >
