The attached patch is a revised one.
It provides two hooks; the one informs core PG whether the supplied
function needs to be hooked, or not. the other is an actual hook on
prepare, start, end and abort of function invocations.
typedef bool (*needs_function_call_type)(Oid fn_oid);
typedef void (*function_call_type)(FunctionCallEventType event,
FmgrInfo *flinfo, Datum *private);
The hook prototype was a bit modified since the suggestion from
Robert. Because FmgrInfo structure contain OID of the function,
it might be redundant to deliver OID of the function individually.
Rest of parts are revised according to the comment.
I also fixed up source code comments which might become incorrect.
Thanks,
(2010/11/18 11:30), Robert Haas wrote:
> 2010/11/17 KaiGai Kohei<kai...@ak.jp.nec.com>:
>> I revised my patch as I attached.
>>
>> The hook function is modified and consolidated as follows:
>>
>> typedef enum FunctionCallEventType
>> {
>> FCET_BE_HOOKED,
>> FCET_PREPARE,
>> FCET_START,
>> FCET_END,
>> FCET_ABORT,
>> } FunctionCallEventType;
>>
>> typedef Datum (*function_call_event_type)(Oid functionId,
>> FunctionCallEventType event,
>> Datum event_arg);
>> extern PGDLLIMPORT function_call_event_type function_call_event_hook;
>>
>> Unlike the subject of this e-mail, now it does not focus on only switching
>> security labels during execution of a certain functions.
>> For example, we may use this hook to track certain functions for security
>> auditing, performance tuning, and others.
>>
>> In the case of SE-PgSQL, it shall return BoolGetDatum(true), if the target
>> function is configured as a trusted procedure, then, this invocation will
>> be hooked by fmgr_security_definer. In the first call, it shall compute
>> the security context to be assigned during execution on FCET_PREPARE event.
>> Then, it switches to the computed label on the FCET_START event, and
>> restore it on the FCET_END or ECET_ABORT event.
>
> This seems like it's a lot simpler than before, which is good. It
> looks to me as though there should really be two separate hooks,
> though, one for what is now FCET_BE_HOOKED and one for everything
> else. For FCET_BE_HOOKED, you want a function that takes an Oid and
> returns a bool. For the other event types, the functionId and event
> arguments are OK, but I think you should forget about the save_datum
> stuff and just always pass fcache->flinfo and&fcache->private. The
> plugin can get the effect of save_datum by passing around whatever
> state it needs to hold on to using fcache->private. So:
>
> bool (*needs_function_call_hook)(Oid fn_oid);
> void (*function_call_hook)(Oid fn_oid, FunctionCallEventType event,
> FmgrInfo flinfo, Datum *private);
>
> Another general comment is that you've not done a very complete job
> updating the comments; there are several of them in fmgr.c that are no
> longer accurate. Also, please zap the unnecessary whitespace changes.
>
> Thanks,
>
--
KaiGai Kohei <kai...@ak.jp.nec.com>
contrib/dummy_seclabel/dummy_seclabel.c | 102 ++++++++++++++++++++++++-
src/backend/optimizer/util/clauses.c | 8 ++
src/backend/utils/fmgr/fmgr.c | 44 ++++++++---
src/include/fmgr.h | 55 +++++++++++++
src/test/regress/input/security_label.source | 28 +++++++
src/test/regress/output/security_label.source | 43 ++++++++++-
6 files changed, 267 insertions(+), 13 deletions(-)
diff --git a/contrib/dummy_seclabel/dummy_seclabel.c b/contrib/dummy_seclabel/dummy_seclabel.c
index 8bd50a3..7acb512 100644
--- a/contrib/dummy_seclabel/dummy_seclabel.c
+++ b/contrib/dummy_seclabel/dummy_seclabel.c
@@ -12,14 +12,105 @@
*/
#include "postgres.h"
+#include "catalog/pg_proc.h"
#include "commands/seclabel.h"
#include "miscadmin.h"
PG_MODULE_MAGIC;
+PG_FUNCTION_INFO_V1(dummy_client_label);
+
/* Entrypoint of the module */
void _PG_init(void);
+/* SQL functions */
+Datum dummay_client_label(PG_FUNCTION_ARGS);
+
+static const char *client_label = "unclassified";
+
+typedef struct {
+ const char *old_label;
+ const char *new_label;
+ Datum next_private;
+} dummy_stack;
+
+static needs_function_call_type needs_function_call_next = NULL;
+static function_call_type function_call_next = NULL;
+
+static bool
+dummy_needs_function_call(Oid fn_oid)
+{
+ char *label;
+ bool result = false;
+ ObjectAddress object = { .classId = ProcedureRelationId,
+ .objectId = fn_oid,
+ .objectSubId = 0 };
+
+ if (needs_function_call_next &&
+ (*needs_function_call_next)(fn_oid))
+ return true;
+
+ label = GetSecurityLabel(&object, "dummy");
+ if (label && strcmp(label, "trusted") == 0)
+ result = true;
+
+ if (label)
+ pfree(label);
+
+ return result;
+}
+
+static void
+dummy_function_call(FunctionCallEventType event,
+ FmgrInfo *flinfo, Datum *private)
+{
+ dummy_stack *stack;
+
+ switch (event)
+ {
+ case FCET_PREPARE:
+ stack = MemoryContextAlloc(flinfo->fn_mcxt,
+ sizeof(dummy_stack));
+ stack->old_label = NULL;
+ stack->new_label = (!superuser() ? "secret" : "top secret");
+ stack->next_private = 0;
+
+ if (function_call_next)
+ (*function_call_next)(event, flinfo, &stack->next_private);
+
+ *private = PointerGetDatum(stack);
+ break;
+
+ case FCET_START:
+ stack = (dummy_stack *)DatumGetPointer(*private);
+
+ stack->old_label = client_label;
+ client_label = stack->new_label;
+ break;
+
+ case FCET_END:
+ case FCET_ABORT:
+ stack = (dummy_stack *)DatumGetPointer(*private);
+
+ client_label = stack->old_label;
+ stack->old_label = NULL;
+ break;
+
+ default:
+ elog(ERROR, "unexpected event type: %d", (int)event);
+ break;
+ }
+}
+
+Datum
+dummy_client_label(PG_FUNCTION_ARGS)
+{
+ if (!client_label)
+ PG_RETURN_NULL();
+
+ PG_RETURN_TEXT_P(cstring_to_text(client_label));
+}
+
static void
dummy_object_relabel(const ObjectAddress *object, const char *seclabel)
{
@@ -28,7 +119,8 @@ dummy_object_relabel(const ObjectAddress *object, const char *seclabel)
strcmp(seclabel, "classified") == 0)
return;
- if (strcmp(seclabel, "secret") == 0 ||
+ if (strcmp(seclabel, "trusted") == 0 ||
+ strcmp(seclabel, "secret") == 0 ||
strcmp(seclabel, "top secret") == 0)
{
if (!superuser())
@@ -46,4 +138,12 @@ void
_PG_init(void)
{
register_label_provider("dummy", dummy_object_relabel);
+
+ /* needs_function_call_hook */
+ needs_function_call_next = needs_function_call_hook;
+ needs_function_call_hook = dummy_needs_function_call;
+
+ /* function_call_hook */
+ function_call_next = function_call_hook;
+ function_call_hook = dummy_function_call;
}
diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c
index de2e66b..c31c36d 100644
--- a/src/backend/optimizer/util/clauses.c
+++ b/src/backend/optimizer/util/clauses.c
@@ -3721,6 +3721,10 @@ inline_function(Oid funcid, Oid result_type, List *args,
if (pg_proc_aclcheck(funcid, GetUserId(), ACL_EXECUTE) != ACLCHECK_OK)
return NULL;
+ /* Check whether the function shall be hooked by plugins */
+ if (FunctionCallIsHooked(funcid))
+ return NULL;
+
/*
* Make a temporary memory context, so that we don't leak all the stuff
* that parsing might create.
@@ -4153,6 +4157,10 @@ inline_set_returning_function(PlannerInfo *root, RangeTblEntry *rte)
if (pg_proc_aclcheck(func_oid, GetUserId(), ACL_EXECUTE) != ACLCHECK_OK)
return NULL;
+ /* Check whether the function shall be hooked by plugins */
+ if (FunctionCallIsHooked(func_oid))
+ return NULL;
+
/*
* OK, let's take a look at the function's pg_proc entry.
*/
diff --git a/src/backend/utils/fmgr/fmgr.c b/src/backend/utils/fmgr/fmgr.c
index 1c9d2c2..c11e075 100644
--- a/src/backend/utils/fmgr/fmgr.c
+++ b/src/backend/utils/fmgr/fmgr.c
@@ -30,6 +30,11 @@
#include "utils/lsyscache.h"
#include "utils/syscache.h"
+/*
+ * Hooks for function calls
+ */
+PGDLLIMPORT needs_function_call_type needs_function_call_hook = NULL;
+PGDLLIMPORT function_call_type function_call_hook = NULL;
/*
* Declaration for old-style function pointer type. This is now used only
@@ -216,7 +221,7 @@ fmgr_info_cxt_security(Oid functionId, FmgrInfo *finfo, MemoryContext mcxt,
finfo->fn_retset = procedureStruct->proretset;
/*
- * If it has prosecdef set, or non-null proconfig, use
+ * If it has prosecdef set, non-null proconfig, hook by plugins use
* fmgr_security_definer call handler --- unless we are being called again
* by fmgr_security_definer.
*
@@ -230,7 +235,8 @@ fmgr_info_cxt_security(Oid functionId, FmgrInfo *finfo, MemoryContext mcxt,
*/
if (!ignore_security &&
(procedureStruct->prosecdef ||
- !heap_attisnull(procedureTuple, Anum_pg_proc_proconfig)))
+ !heap_attisnull(procedureTuple, Anum_pg_proc_proconfig) ||
+ FunctionCallIsHooked(functionId)))
{
finfo->fn_addr = fmgr_security_definer;
finfo->fn_stats = TRACK_FUNC_ALL; /* ie, never track */
@@ -857,17 +863,18 @@ struct fmgr_security_definer_cache
FmgrInfo flinfo; /* lookup info for target function */
Oid userid; /* userid to set, or InvalidOid */
ArrayType *proconfig; /* GUC values to set, or NULL */
+ Datum private; /* private usage for plugin modules */
};
/*
- * Function handler for security-definer/proconfig functions. We extract the
- * OID of the actual function and do a fmgr lookup again. Then we fetch the
- * pg_proc row and copy the owner ID and proconfig fields. (All this info
- * is cached for the duration of the current query.) To execute a call,
- * we temporarily replace the flinfo with the cached/looked-up one, while
- * keeping the outer fcinfo (which contains all the actual arguments, etc.)
- * intact. This is not re-entrant, but then the fcinfo itself can't be used
- * re-entrantly anyway.
+ * Function handler for security-definer/proconfig/plugin-hooked functions.
+ * We extract the OID of the actual function and do a fmgr lookup again.
+ * Then we fetch the pg_proc row and copy the owner ID and proconfig fields.
+ * (All this info is cached for the duration of the current query.)
+ * To execute a call, we temporarily replace the flinfo with the cached
+ * and looked-up one, while keeping the outer fcinfo (which contains all
+ * the actual arguments, etc.) intact. This is not re-entrant, but then
+ * the fcinfo itself can't be used re-entrantly anyway.
*/
static Datum
fmgr_security_definer(PG_FUNCTION_ARGS)
@@ -916,6 +923,11 @@ fmgr_security_definer(PG_FUNCTION_ARGS)
ReleaseSysCache(tuple);
+ /* function call event hook */
+ if (function_call_hook)
+ (*function_call_hook)(FCET_PREPARE,
+ &fcache->flinfo, &fcache->private);
+
fcinfo->flinfo->fn_extra = fcache;
}
else
@@ -940,6 +952,11 @@ fmgr_security_definer(PG_FUNCTION_ARGS)
GUC_ACTION_SAVE);
}
+ /* function call event hook */
+ if (function_call_hook)
+ (*function_call_hook)(FCET_START,
+ &fcache->flinfo, &fcache->private);
+
/*
* We don't need to restore GUC or userid settings on error, because the
* ensuing xact or subxact abort will do that. The PG_TRY block is only
@@ -968,6 +985,9 @@ fmgr_security_definer(PG_FUNCTION_ARGS)
PG_CATCH();
{
fcinfo->flinfo = save_flinfo;
+ if (function_call_hook)
+ (*function_call_hook)(FCET_ABORT,
+ &fcache->flinfo, &fcache->private);
PG_RE_THROW();
}
PG_END_TRY();
@@ -978,7 +998,9 @@ fmgr_security_definer(PG_FUNCTION_ARGS)
AtEOXact_GUC(true, save_nestlevel);
if (OidIsValid(fcache->userid))
SetUserIdAndSecContext(save_userid, save_sec_context);
-
+ if (function_call_hook)
+ (*function_call_hook)(FCET_END,
+ &fcache->flinfo, &fcache->private);
return result;
}
diff --git a/src/include/fmgr.h b/src/include/fmgr.h
index ca5a5ea..9cbaa80 100644
--- a/src/include/fmgr.h
+++ b/src/include/fmgr.h
@@ -544,6 +544,61 @@ extern void **find_rendezvous_variable(const char *varName);
extern int AggCheckCallContext(FunctionCallInfo fcinfo,
MemoryContext *aggcontext);
+/*
+ * function_call_hook
+ * ------------------
+ * This hook allows plugin modules to hook events of function calls.
+ * It enables to switch some of its internal state (such as privilege
+ * of the client) during execution of the hooked function.
+ *
+ * The plugin module has to set up two hooks for this feature at least.
+ * The one is 'needs_function_call_hook' which enables to inform whether
+ * the plugin module wants to hook the supplied function, or not.
+ * It returns 'true', if the supplied function shall be hooked. Elsewhere,
+ * 'false' shall be returned.
+ *
+ * The other hook is 'function_call_hook' that enables plugin modules
+ * to acquire control when the supplied function is prepared, started,
+ * ended and aborted.
+ * It takes four arguments: OID of the function, event type (defined
+ * as FunctionCallEventType), FmgrInfo and pointer to the private
+ * data field.
+ *
+ * FCET_PREPARE event type allows plugins to acquire control on the
+ * first invocation time of the function only once. We intend to use
+ * this callback to set up private data structure for later uses.
+ * If you want to call palloc(), note that CurrentMemoryContext is not
+ * available during whole of execution, so use FmgrInfo->mcxt instead.
+ *
+ * FCET_START event type allows plugins to acquire control just before
+ * invocation of the hooked function for each time. If necessary, the
+ * plugin module can switch its internal state.
+ *
+ * FCET_END and FCET_ABORT allow plugins to acquire control just after
+ * invocation of the hooked function for each time. If necessary, the
+ * plugin module can restore its internal state.
+ */
+typedef enum FunctionCallEventType
+{
+ FCET_PREPARE,
+ FCET_START,
+ FCET_END,
+ FCET_ABORT
+} FunctionCallEventType;
+
+typedef bool (*needs_function_call_type)(Oid fn_oid);
+
+typedef void (*function_call_type)(FunctionCallEventType event,
+ FmgrInfo *flinfo, Datum *private);
+
+extern PGDLLIMPORT needs_function_call_type needs_function_call_hook;
+extern PGDLLIMPORT function_call_type function_call_hook;
+
+#define FunctionCallIsHooked(fn_oid) \
+ (!needs_function_call_hook \
+ ? false \
+ : (*needs_function_call_hook)(fn_oid) \
+ )
/*
* !!! OLD INTERFACE !!!
diff --git a/src/test/regress/input/security_label.source b/src/test/regress/input/security_label.source
index 810a721..9fb2a2d 100644
--- a/src/test/regress/input/security_label.source
+++ b/src/test/regress/input/security_label.source
@@ -37,6 +37,15 @@ SECURITY LABEL ON TABLE seclabel_tbl3 IS 'unclassified'; -- fail
-- Load dummy external security provider
LOAD '@libdir@/dummy_secla...@dlsuffix@';
+CREATE FUNCTION dummy_client_label() RETURNS text LANGUAGE 'c'
+ AS '@libdir@/dummy_secla...@dlsuffix@', 'dummy_client_label';
+
+CREATE FUNCTION seclabel_regular() RETURNS text LANGUAGE 'sql'
+ AS 'SELECT dummy_client_label()';
+
+CREATE FUNCTION seclabel_trusted() RETURNS text LANGUAGE 'sql'
+ AS 'SELECT dummy_client_label()';
+
--
-- Test of SECURITY LABEL statement with a plugin
--
@@ -70,7 +79,26 @@ SELECT objtype, objname, provider, label FROM pg_seclabels
SECURITY LABEL ON LANGUAGE plpgsql IS NULL; -- OK
SECURITY LABEL ON SCHEMA public IS NULL; -- OK
+-- test for trusted procedures
+SECURITY LABEL ON FUNCTION seclabel_regular() IS 'unclassified'; -- OK
+SECURITY LABEL ON FUNCTION seclabel_trusted() IS 'trusted'; -- OK
+
+-- should be 'unclassified' and 'top secret'
+SELECT seclabel_regular(), seclabel_trusted();
+
+-- should be 'unclassified' and 'secret'
+SET SESSION AUTHORIZATION seclabel_user1;
+SELECT seclabel_regular(), seclabel_trusted();
+RESET SESSION AUTHORIZATION;
+
+-- hooked functions shall not be inlined
+EXPLAIN SELECT * FROM seclabel_regular(); -- shall be inlined
+EXPLAIN SELECT * FROM seclabel_trusted(); -- shall not be inlined
+
-- clean up objects
+RESET SESSION AUTHORIZATION;
+DROP FUNCTION seclabel_regular();
+DROP FUNCTION seclabel_trusted();
DROP FUNCTION seclabel_four();
DROP DOMAIN seclabel_domain;
DROP VIEW seclabel_view1;
diff --git a/src/test/regress/output/security_label.source b/src/test/regress/output/security_label.source
index 4bc803d..fb3809e 100644
--- a/src/test/regress/output/security_label.source
+++ b/src/test/regress/output/security_label.source
@@ -30,7 +30,13 @@ ERROR: no security label providers have been loaded
SECURITY LABEL ON TABLE seclabel_tbl3 IS 'unclassified'; -- fail
ERROR: no security label providers have been loaded
-- Load dummy external security provider
-LOAD '@abs_builddir@/dummy_secla...@dlsuffix@';
+LOAD '@libdir@/dummy_secla...@dlsuffix@';
+CREATE FUNCTION dummy_client_label() RETURNS text LANGUAGE 'c'
+ AS '@libdir@/dummy_secla...@dlsuffix@', 'dummy_client_label';
+CREATE FUNCTION seclabel_regular() RETURNS text LANGUAGE 'sql'
+ AS 'SELECT dummy_client_label()';
+CREATE FUNCTION seclabel_trusted() RETURNS text LANGUAGE 'sql'
+ AS 'SELECT dummy_client_label()';
--
-- Test of SECURITY LABEL statement with a plugin
--
@@ -75,7 +81,42 @@ SELECT objtype, objname, provider, label FROM pg_seclabels
SECURITY LABEL ON LANGUAGE plpgsql IS NULL; -- OK
SECURITY LABEL ON SCHEMA public IS NULL; -- OK
+-- test for trusted procedures
+SECURITY LABEL ON FUNCTION seclabel_regular() IS 'unclassified'; -- OK
+SECURITY LABEL ON FUNCTION seclabel_trusted() IS 'trusted'; -- OK
+-- should be 'unclassified' and 'top secret'
+SELECT seclabel_regular(), seclabel_trusted();
+ seclabel_regular | seclabel_trusted
+------------------+------------------
+ unclassified | top secret
+(1 row)
+
+-- should be 'unclassified' and 'secret'
+SET SESSION AUTHORIZATION seclabel_user1;
+SELECT seclabel_regular(), seclabel_trusted();
+ seclabel_regular | seclabel_trusted
+------------------+------------------
+ unclassified | secret
+(1 row)
+
+RESET SESSION AUTHORIZATION;
+-- hooked functions shall not be inlined
+EXPLAIN SELECT * FROM seclabel_regular(); -- shall be inlined
+ QUERY PLAN
+-----------------------------------------------------------------------------------------
+ Function Scan on dummy_client_label seclabel_regular (cost=0.00..0.01 rows=1 width=32)
+(1 row)
+
+EXPLAIN SELECT * FROM seclabel_trusted(); -- shall not be inlined
+ QUERY PLAN
+----------------------------------------------------------------------
+ Function Scan on seclabel_trusted (cost=0.25..0.26 rows=1 width=32)
+(1 row)
+
-- clean up objects
+RESET SESSION AUTHORIZATION;
+DROP FUNCTION seclabel_regular();
+DROP FUNCTION seclabel_trusted();
DROP FUNCTION seclabel_four();
DROP DOMAIN seclabel_domain;
DROP VIEW seclabel_view1;
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
