Re: [PATCH] Re: Setuid functions

Your patch has been added to the PostgreSQL unapplied patches list at:

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

I will try to apply it within the next 48 hours.

> Sorry, I have decided not to follow the SQL standard ;-) PRIVILEGE is spelled
> correctly in my patch.
>
> This patch will implement the "ENABLE PRIVILEGE" and "DISABLE PRIVILEGE"
> commands in PL/pgSQL, which, respectively, change the effective uid to that
> of the function owner and back. It doesn't break security (I hope). The
> commands can be abbreviated as "ENABLE" and "DISABLE" for the poor saps that
> have trouble with "PRIVILEGE" :) Easier than adding a setuid bit to the
> catalog, no?
>
> Apologies if the patch is not in the correct format. Apply with
>
> patch -p1 < enable_disable.patch
>
> in the tippety-top of the 7.1.2 tree.
>
> Regression example:
>
> CREATE USER sample_user;
> CREATE TABLE test_log(stamp datetime);
> GRANT SELECT ON test_log TO PUBLIC;
>
> DROP FUNCTION test_enable();
> CREATE FUNCTION test_enable() RETURNS boolean AS
> '
> DECLARE
> user name;
> BEGIN
> user:=current_user;
> RAISE NOTICE ''Username: %'', user;
> ENABLE PRIVILEGE;
> user:=current_user;
> RAISE NOTICE ''Username: %'', user;
> INSERT INTO test_log VALUES(''now''::text);
> DISABLE PRIVILEGE; -- Actually unnecessary at the end of the function
> RETURN TRUE;
> END;
> ' LANGUAGE 'plpgsql';
>
> \c - sample_user
> SELECT test_enable();
> SELECT * FROM test_log;
>
> stamp
> ------------------------
> 2001-06-21 11:17:29-04
>
> (Note current time logged into a table where sample_user could not normally
> write)
>
> Hope you will find this useful
> - Mark
>
> "Ross J. Reedstrom" wrote:
> >
> > Come on, Chris, you've never heard about SQL standard LEDGE? That's
> > the nomenclature they chose to describe a collection of permissions:
> > a SHELF or LEDGE. PUBLEDGE, USERLEDGE, PRIVLEDGE. So, the last is the
> > PRIVATE LEDGE, reserved for the owner of the object whose access is
> > being determined (or was that PRIVITHEDGE? now I'm confused)
> >
> > ... or something. ;-) Actually, not too far from how some of the SQL92
> > standards docs actually seem to read, especially after falling asleep
> > face down on the keyboard will trying to understand them, and having
> > vivid dreams.
> >
> > Ross (who's in the office much too late, working on budget justifications
> > for grants that are due tomorrow!)
> >
> > ---------------------------(end of broadcast)---------------------------
> > TIP 2: you can get off all lists at once with the unregister command
> > (send "unregister YourEmailAddressHere" to majordomo(at)postgresql(dot)org)

> diff -ur postgresql-7.1.2/src/pl/plpgsql/src/gram.y postgresql-7.1.2-patch/src/pl/plpgsql/src/gram.y
> --- postgresql-7.1.2/src/pl/plpgsql/src/gram.y Wed Jun 20 20:07:45 2001
> +++ postgresql-7.1.2-patch/src/pl/plpgsql/src/gram.y Wed Jun 20 19:48:18 2001
> @@ -121,7 +121,7 @@
> %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_return, stmt_raise, stmt_execsql, stmt_fori, stmt_enable, stmt_disable
> %type <stmt> stmt_fors, stmt_select, stmt_perform
> %type <stmt> stmt_dynexecute, stmt_dynfors, stmt_getdiag
>
> @@ -164,6 +164,9 @@
> %token K_PERFORM
> %token K_ROW_COUNT
> %token K_RAISE
> +%token K_ENABLE
> +%token K_DISABLE
> +%token K_PRIVILEGE
> %token K_RECORD
> %token K_RENAME
> %token K_RESULT_OID
> @@ -569,6 +572,10 @@
> { $$ = $1; }
> | stmt_raise
> { $$ = $1; }
> + | stmt_enable
> + { $$ = $1; }
> + | stmt_disable
> + { $$ = $1; }
> | stmt_execsql
> { $$ = $1; }
> | stmt_dynexecute
> @@ -1033,6 +1040,34 @@
> $$ = (PLpgSQL_stmt *)new;
> }
> ;
> +
> +stmt_enable : K_ENABLE opt_privilege lno ';'
> + {
> + PLpgSQL_stmt_privilege *new;
> +
> + new=malloc(sizeof(PLpgSQL_stmt_privilege));
> +
> + new->cmd_type = PLPGSQL_STMT_ENABLE;
> + new->lineno = $3;
> +
> + $$ = (PLpgSQL_stmt *)new;
> + }
> +
> +stmt_disable : K_DISABLE opt_privilege lno ';'
> + {
> + PLpgSQL_stmt_privilege *new;
> +
> + new=malloc(sizeof(PLpgSQL_stmt_privilege));
> +
> + new->cmd_type = PLPGSQL_STMT_DISABLE;
> + new->lineno = $3;
> +
> + $$ = (PLpgSQL_stmt *)new;
> + }
> +
> +opt_privilege :
> + | K_PRIVILEGE
> +;
>
> stmt_raise : K_RAISE lno raise_level raise_msg raise_params ';'
> {
> diff -ur postgresql-7.1.2/src/pl/plpgsql/src/pl_exec.c postgresql-7.1.2-patch/src/pl/plpgsql/src/pl_exec.c
> --- postgresql-7.1.2/src/pl/plpgsql/src/pl_exec.c Wed Jun 20 20:07:45 2001
> +++ postgresql-7.1.2-patch/src/pl/plpgsql/src/pl_exec.c Wed Jun 20 19:48:18 2001
> @@ -99,6 +99,8 @@
> PLpgSQL_stmt_exit * stmt);
> static int exec_stmt_return(PLpgSQL_execstate * estate,
> PLpgSQL_stmt_return * stmt);
> +static int exec_stmt_privilege(PLpgSQL_execstate * estate,
> + PLpgSQL_stmt_privilege * stmt);
> static int exec_stmt_raise(PLpgSQL_execstate * estate,
> PLpgSQL_stmt_raise * stmt);
> static int exec_stmt_execsql(PLpgSQL_execstate * estate,
> @@ -220,6 +222,12 @@
> case PLPGSQL_STMT_RETURN:
> stmttype = "return";
> break;
> + case PLPGSQL_STMT_ENABLE:
> + stmttype = "enable";
> + break;
> + case PLPGSQL_STMT_DISABLE:
> + stmttype = "disable";
> + break;
> case PLPGSQL_STMT_RAISE:
> stmttype = "raise";
> break;
> @@ -265,6 +273,8 @@
> estate.retistuple = func->fn_retistuple;
> estate.retisset = func->fn_retset;
> estate.exitlabel = NULL;
> + estate.save_uid = InvalidOid;
> + estate.fn_oid = func->fn_oid;
>
> estate.found_varno = func->found_varno;
> estate.ndatums = func->ndatums;
> @@ -385,6 +395,9 @@
> elog(ERROR, "control reaches end of function without RETURN");
> }
>
> + if (estate.save_uid!=InvalidOid)
> + SetUserId(estate.save_uid);
> +
> /*
> * We got a return value - process it
> */
> @@ -565,6 +578,8 @@
> estate.retistuple = func->fn_retistuple;
> estate.retisset = func->fn_retset;
> estate.exitlabel = NULL;
> + estate.save_uid = InvalidOid;
> + estate.fn_oid = func->fn_oid;
>
> estate.found_varno = func->found_varno;
> estate.ndatums = func->ndatums;
> @@ -742,6 +757,9 @@
> elog(ERROR, "control reaches end of trigger procedure without RETURN");
> }
>
> + if (estate.save_uid!=InvalidOid)
> + SetUserId(estate.save_uid);
> +
> /*
> * Check that the returned tuple structure has the same attributes,
> * the relation that fired the trigger has.
> @@ -986,6 +1004,11 @@
> rc = exec_stmt_return(estate, (PLpgSQL_stmt_return *) stmt);
> break;
>
> + case PLPGSQL_STMT_ENABLE:
> + case PLPGSQL_STMT_DISABLE:
> + rc = exec_stmt_privilege(estate, (PLpgSQL_stmt_privilege *) stmt);
> + break;
> +
> case PLPGSQL_STMT_RAISE:
> rc = exec_stmt_raise(estate, (PLpgSQL_stmt_raise *) stmt);
> break;
> @@ -1536,6 +1559,41 @@
> &(estate->rettype));
>
> return PLPGSQL_RC_RETURN;
> +}
> +
> +/* ----------
> + * exec_stmt_privilege Changes user ID to/from
> + * that of the function owner's
> + * ----------
> + */
> +
> +static int
> +exec_stmt_privilege(PLpgSQL_execstate * estate, PLpgSQL_stmt_privilege * stmt)
> +{
> + HeapTuple procedureTuple;
> + Form_pg_proc procedureStruct;
> +
> + if (stmt->cmd_type==PLPGSQL_STMT_ENABLE)
> + {
> + if (estate->save_uid!=InvalidOid) return PLPGSQL_RC_OK; /* Already enabled */
> + procedureTuple = SearchSysCache(PROCOID, ObjectIdGetDatum(estate->fn_oid), 0, 0, 0);
> +
> + if (!HeapTupleIsValid(procedureTuple))
> + elog(ERROR, "exec_stmt_privilege: cache lookup failed");
> +
> + procedureStruct = (Form_pg_proc) GETSTRUCT(procedureTuple);
> +
> + estate->save_uid = GetUserId();
> + SetUserId(procedureStruct->proowner);
> + ReleaseSysCache(procedureTuple);
> + } else
> + {
> + if (estate->save_uid==InvalidOid) return PLPGSQL_RC_OK; /* Already disabled */
> + SetUserId(estate->save_uid);
> + estate->save_uid = InvalidOid;
> + }
> +
> + return PLPGSQL_RC_OK;
> }
>
>
> diff -ur postgresql-7.1.2/src/pl/plpgsql/src/pl_funcs.c postgresql-7.1.2-patch/src/pl/plpgsql/src/pl_funcs.c
> --- postgresql-7.1.2/src/pl/plpgsql/src/pl_funcs.c Wed Jun 20 20:07:45 2001
> +++ postgresql-7.1.2-patch/src/pl/plpgsql/src/pl_funcs.c Wed Jun 20 20:05:14 2001
> @@ -382,6 +382,7 @@
> 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_privilege(PLpgSQL_stmt_privilege * 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);
> @@ -435,6 +436,10 @@
> case PLPGSQL_STMT_RETURN:
> dump_return((PLpgSQL_stmt_return *) stmt);
> break;
> + case PLPGSQL_STMT_ENABLE:
> + case PLPGSQL_STMT_DISABLE:
> + dump_privilege((PLpgSQL_stmt_privilege *) stmt);
> + break;
> case PLPGSQL_STMT_RAISE:
> dump_raise((PLpgSQL_stmt_raise *) stmt);
> break;
> @@ -647,6 +652,16 @@
> dump_expr(stmt->expr);
> }
> printf("\n");
> +}
> +
> +static void
> +dump_privilege(PLpgSQL_stmt_privilege * stmt)
> +{
> + dump_ind();
> + if (stmt->cmd_type==PLPGSQL_STMT_ENABLE)
> + printf("ENABLE PRIVILEGE\n");
> + else
> + printf("DISABLE PRIVILEGE\n");
> }
>
> static void
> diff -ur postgresql-7.1.2/src/pl/plpgsql/src/plpgsql.h postgresql-7.1.2-patch/src/pl/plpgsql/src/plpgsql.h
> --- postgresql-7.1.2/src/pl/plpgsql/src/plpgsql.h Wed Jun 20 20:07:45 2001
> +++ postgresql-7.1.2-patch/src/pl/plpgsql/src/plpgsql.h Wed Jun 20 20:08:57 2001
> @@ -94,7 +94,9 @@
> PLPGSQL_STMT_EXECSQL,
> PLPGSQL_STMT_DYNEXECUTE,
> PLPGSQL_STMT_DYNFORS,
> - PLPGSQL_STMT_GETDIAG
> + PLPGSQL_STMT_GETDIAG,
> + PLPGSQL_STMT_ENABLE,
> + PLPGSQL_STMT_DISABLE
> };
>
>
> @@ -387,6 +389,11 @@
> int retrecno;
> } PLpgSQL_stmt_return;
>
> +typedef struct
> +{ /* ENABLE/DISABLE statement */
> + int cmd_type;
> + int lineno;
> +} PLpgSQL_stmt_privilege;
>
> typedef struct
> { /* RAISE statement */
> @@ -464,6 +471,8 @@
> int found_varno;
> int ndatums;
> PLpgSQL_datum **datums;
> + Oid save_uid;
> + Oid fn_oid;
> } PLpgSQL_execstate;
>
>
> Only in postgresql-7.1.2/src/pl/plpgsql/src: plpgsql.h.orig
> Only in postgresql-7.1.2/src/pl/plpgsql/src: plpgsql.h.rej
> diff -ur postgresql-7.1.2/src/pl/plpgsql/src/scan.l postgresql-7.1.2-patch/src/pl/plpgsql/src/scan.l
> --- postgresql-7.1.2/src/pl/plpgsql/src/scan.l Wed Jun 20 20:07:45 2001
> +++ postgresql-7.1.2-patch/src/pl/plpgsql/src/scan.l Wed Jun 20 19:48:18 2001
> @@ -115,6 +115,9 @@
> null { return K_NULL; }
> perform { return K_PERFORM; }
> raise { return K_RAISE; }
> +enable { return K_ENABLE; }
> +disable { return K_DISABLE; }
> +privilege { return K_PRIVILEGE; }
> record { return K_RECORD; }
> rename { return K_RENAME; }
> result_oid { return K_RESULT_OID; }

>
> ---------------------------(end of broadcast)---------------------------
> TIP 3: if posting/reading through Usenet, please send an appropriate
> subscribe-nomail command to majordomo(at)postgresql(dot)org so that your
> message can get through to the mailing list cleanly

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