Hi.
I could actually use some comments on the approach. I've attached
a prototype I've been working on (which is a cut down version of
my earlier code; but it's not terribly interesting and you don't
need to read it to comment on my questions below). The attached
patch does the following:
1. Adds a pgaudit.roles = 'role1, role2' GUC setting.
2. Adds a role_is_audited() function that returns true if the given
role OID is mentioned in (or inherits from a role mentioned in)
pgaudit.roles.
3. Adds a call to role_is_audited from log_audit_event with the current
user id (GetSessionUserId in the patch, though it may be better to
use GetUserId; but that's a minor detail).
Earlier, I was using a combination of check and assign hooks to convert
names to OIDs, but (as Andres pointed out) that would have problems with
cache invalidations. I was even playing with caching membership lookups,
but I ripped out all that code.
In the attached patch, role_is_audited does all the hard work to split
up the list of roles, look up the corresponding OIDs, and check if the
user is a member of any of those roles. It works fine, but it doesn't
seem desirable to repeat all that work for every statement.
So does anyone have suggestions about how to make this faster?
-- Abhijit
diff --git a/pgaudit.c b/pgaudit.c
index 30f729a..5ba9886 100644
--- a/pgaudit.c
+++ b/pgaudit.c
@@ -58,6 +58,13 @@ PG_FUNCTION_INFO_V1(pgaudit_func_ddl_command_end);
PG_FUNCTION_INFO_V1(pgaudit_func_sql_drop);
/*
+ * pgaudit_roles_str is the string value of the pgaudit.roles
+ * configuration variable, which is a list of role names.
+ */
+
+char *pgaudit_roles_str = NULL;
+
+/*
* pgaudit_log_str is the string value of the pgaudit.log configuration
* variable, e.g. "read, write, user". Each token corresponds to a flag
* in enum LogClass below. We convert the list of tokens into a bitmap
@@ -117,6 +124,40 @@ typedef struct {
} AuditEvent;
/*
+ * Takes a role OID and returns true or false depending on whether
+ * auditing it enabled for that roles according to the pgaudit.roles
+ * setting.
+ */
+
+static bool
+role_is_audited(Oid roleid)
+{
+ List *roles;
+ ListCell *lt;
+
+ if (!SplitIdentifierString(pgaudit_roles_str, ',', &roles))
+ return false;
+
+ foreach(lt, roles)
+ {
+ char *name = (char *)lfirst(lt);
+ HeapTuple roleTup;
+
+ roleTup = SearchSysCache1(AUTHNAME, PointerGetDatum(name));
+ if (HeapTupleIsValid(roleTup))
+ {
+ Oid parentrole = HeapTupleGetOid(roleTup);
+
+ ReleaseSysCache(roleTup);
+ if (is_member_of_role(roleid, parentrole))
+ return true;
+ }
+ }
+
+ return false;
+}
+
+/*
* Takes an AuditEvent and returns true or false depending on whether
* the event should be logged according to the pgaudit.log setting. If
* it returns true, it also fills in the name of the LogClass which it
@@ -289,18 +330,24 @@ should_be_logged(AuditEvent *e, const char **classname)
static void
log_audit_event(AuditEvent *e)
{
+ Oid userid;
const char *timestamp;
const char *database;
const char *username;
const char *eusername;
const char *classname;
+ userid = GetSessionUserId();
+
+ if (!role_is_audited(userid))
+ return;
+
if (!should_be_logged(e, &classname))
return;
timestamp = timestamptz_to_str(GetCurrentTimestamp());
database = get_database_name(MyDatabaseId);
- username = GetUserNameFromId(GetSessionUserId());
+ username = GetUserNameFromId(userid);
eusername = GetUserNameFromId(GetUserId());
/*
@@ -328,7 +375,7 @@ log_executor_check_perms(List *rangeTabls, bool abort_on_violation)
{
ListCell *lr;
- foreach (lr, rangeTabls)
+ foreach(lr, rangeTabls)
{
Relation rel;
AuditEvent e;
@@ -1127,6 +1174,32 @@ pgaudit_object_access_hook(ObjectAccessType access,
}
/*
+ * Take a pgaudit.roles value such as "role1, role2" and verify that
+ * the string consists of comma-separated tokens.
+ */
+
+static bool
+check_pgaudit_roles(char **newval, void **extra, GucSource source)
+{
+ List *roles;
+ char *rawval;
+
+ /* Make sure we have a comma-separated list of tokens. */
+
+ rawval = pstrdup(*newval);
+ if (!SplitIdentifierString(rawval, ',', &roles))
+ {
+ GUC_check_errdetail("List syntax is invalid");
+ list_free(roles);
+ pfree(rawval);
+ return false;
+ }
+ pfree(rawval);
+
+ return true;
+}
+
+/*
* Take a pgaudit.log value such as "read, write, user", verify that
* each of the comma-separated tokens corresponds to a LogClass value,
* and convert them into a bitmap that log_audit_event can check.
@@ -1232,6 +1305,23 @@ _PG_init(void)
errmsg("pgaudit must be loaded via shared_preload_libraries")));
/*
+ * pgaudit.roles = "role1, role2"
+ *
+ * This variable defines a list of roles for which auditing is
+ * enabled.
+ */
+
+ DefineCustomStringVariable("pgaudit.roles",
+ "Enable auditing for certain roles",
+ NULL,
+ &pgaudit_roles_str,
+ "",
+ PGC_SUSET,
+ GUC_LIST_INPUT | GUC_NOT_IN_SAMPLE,
+ check_pgaudit_roles,
+ NULL, NULL);
+
+ /*
* pgaudit.log = "read, write, user"
*
* This variables controls what classes of commands are logged.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
