Re: [PATCH] Re: Setuid functions - Mailing list pgsql-hackers

Removed from queue pending review.

> 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@postgresql.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@postgresql.org so that your
> message can get through to the mailing list cleanly

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