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

From Mark Volpe
Subject [PATCH] Re: Setuid functions
Date
Msg-id 3B3211BC.18D7517@epa.gov
Whole thread Raw
In response to RE: Setuid functions  ("Christopher Kings-Lynne" <chriskl@familyhealth.com.au>)
Responses Re: [PATCH] Re: Setuid functions  (Bruce Momjian <pgman@candle.pha.pa.us>)
Re: [PATCH] Re: Setuid functions  (Peter Eisentraut <peter_e@gmx.net>)
Re: [PATCH] Re: Setuid functions  (Bruce Momjian <pgman@candle.pha.pa.us>)
List pgsql-hackers
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.ypostgresql-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;        }

pgsql-hackers by date:

Previous
From: Olivier PRENANT
Date:
Subject: openssl+postgresql+unixware
Next
From: darcy@druid.net (D'Arcy J.M. Cain)
Date:
Subject: COPY vs. INSERT