Re: [HACKERS] [PATCH] Re: Setuid functions - Mailing list pgsql-patches

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 <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@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