On 11/8/19 9:16 AM, Joe Conway wrote:
> On 11/8/19 9:02 AM, Yuli Khodorkovskiy wrote:
>> On Thu, Nov 7, 2019 at 7:46 PM Michael Paquier <mich...@paquier.xyz> wrote:
>>>
>>> On Mon, Sep 30, 2019 at 11:38:05AM -0300, Alvaro Herrera wrote:
>>> > On 2019-Sep-30, Joe Conway wrote:
>>> >
>>> > > I am not sure I will get to this today. I assume it is ok for me to move
>>> > > it forward e.g. next weekend, or is that not in line with commitfest
>>> > > rules?
>>> >
>>> > You can commit whatever patch whenever you feel like it. I will
>>> > probably move this patch to the next commitfest before that, but you can
>>> > mark it committed there as soon as you commit it.
>>>
>>> One month later, nothing has happened here. Joe, are you planning to
>>> look at this patch?
>>>
>>> The last patch I found does not apply properly, so please provide a
>>> rebase. I am switching the patch as waiting on author.
>>
>> Michael,
>>
>> I was able to apply the latest patches in the thread (9/25/19) on top
>> of master. I have attached them for convenience.
>
> Yes, I will look when I am able. Hopefully this weekend, almost
> certainly before the end of this commitfest.
I tested this successfully on Rhinoceros, both with and without
"db_table: { truncate }" loaded in the policy. Updated patches attached
here with some editorialization. If there are no objections I will
commit/push both in about a day or two.
Joe
--
Crunchy Data - http://crunchydata.com
PostgreSQL Support for Secure Enterprises
Consulting, Training, & Open Source Development
Add object TRUNCATE hook
All operations with acl permissions checks should have a corresponding hook
so that, for example, mandatory access control (MAC) may be enforced by an
extension. The command TRUNCATE is missing this hook, so add it. Patch by
Yuli Khodorkovskiy with some editorialization by me. Based on the discussion
not back-patched. A separate patch will exercise the hook in the sepgsql
extension.
Author: Yuli Khodorkovskiy
Reviewed-by: Joe Conway
Discussion: https://postgr.es/m/CAFL5wJcomybj1Xdw7qWmPJRpGuFukKgNrDb6uVBaCMgYS9dkaA%40mail.gmail.com
---
diff --git a/src/backend/catalog/objectaccess.c b/src/backend/catalog/objectaccess.c
index b1619b8..d51d01f 100644
*** a/src/backend/catalog/objectaccess.c
--- b/src/backend/catalog/objectaccess.c
***************
*** 11,16 ****
--- 11,17 ----
#include "postgres.h"
#include "catalog/objectaccess.h"
+ #include "catalog/pg_class.h"
#include "catalog/pg_namespace.h"
#include "catalog/pg_proc.h"
*************** RunObjectDropHook(Oid classId, Oid objec
*** 65,70 ****
--- 66,87 ----
}
/*
+ * RunObjectTruncateHook
+ *
+ * It is the entrypoint of OAT_TRUNCATE event
+ */
+ void
+ RunObjectTruncateHook(Oid objectId)
+ {
+ /* caller should check, but just in case... */
+ Assert(object_access_hook != NULL);
+
+ (*object_access_hook) (OAT_TRUNCATE,
+ RelationRelationId, objectId, 0,
+ NULL);
+ }
+
+ /*
* RunObjectPostAlterHook
*
* It is entrypoint of OAT_POST_ALTER event
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 45aae59..5440eb9 100644
*** a/src/backend/commands/tablecmds.c
--- b/src/backend/commands/tablecmds.c
*************** truncate_check_rel(Oid relid, Form_pg_cl
*** 1937,1942 ****
--- 1937,1944 ----
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
errmsg("permission denied: \"%s\" is a system catalog",
relname)));
+
+ InvokeObjectTruncateHook(relid);
}
/*
diff --git a/src/include/catalog/objectaccess.h b/src/include/catalog/objectaccess.h
index 0170f1f..9824adf 100644
*** a/src/include/catalog/objectaccess.h
--- b/src/include/catalog/objectaccess.h
***************
*** 37,42 ****
--- 37,46 ----
* creation or altering, because OAT_POST_CREATE or OAT_POST_ALTER are
* sufficient for extensions to track these kind of checks.
*
+ * OAT_TRUNCATE should be invoked just before truncation of objects. This
+ * event is equivalent to truncate permission on a relation under the
+ * default access control mechanism.
+ *
* Other types may be added in the future.
*/
typedef enum ObjectAccessType
*************** typedef enum ObjectAccessType
*** 45,51 ****
OAT_DROP,
OAT_POST_ALTER,
OAT_NAMESPACE_SEARCH,
! OAT_FUNCTION_EXECUTE
} ObjectAccessType;
/*
--- 49,56 ----
OAT_DROP,
OAT_POST_ALTER,
OAT_NAMESPACE_SEARCH,
! OAT_FUNCTION_EXECUTE,
! OAT_TRUNCATE
} ObjectAccessType;
/*
*************** extern void RunObjectPostCreateHook(Oid
*** 131,136 ****
--- 136,142 ----
bool is_internal);
extern void RunObjectDropHook(Oid classId, Oid objectId, int subId,
int dropflags);
+ extern void RunObjectTruncateHook(Oid objectId);
extern void RunObjectPostAlterHook(Oid classId, Oid objectId, int subId,
Oid auxiliaryId, bool is_internal);
extern bool RunNamespaceSearchHook(Oid objectId, bool ereport_on_violation);
*************** extern void RunFunctionExecuteHook(Oid o
*** 160,165 ****
--- 166,177 ----
(dropflags)); \
} while(0)
+ #define InvokeObjectTruncateHook(objectId) \
+ do { \
+ if (object_access_hook) \
+ RunObjectTruncateHook(objectId); \
+ } while(0)
+
#define InvokeObjectPostAlterHook(classId,objectId,subId) \
InvokeObjectPostAlterHookArg((classId),(objectId),(subId), \
InvalidOid,false)
Update sepgsql to add mandatory access control for TRUNCATE
Use SELinux "db_table: { truncate }" to check if permission is granted to
TRUNCATE. Update example SELinux policy to grant needed permission for
TRUNCATE. Add new regression test to demonstrate a positive and negative
cases. Test will only be run if the loaded SELinux policy has the
"db_table: { truncate }" permission. Makes use of recent commit which added
object TRUNCATE hook. Patch by Yuli Khodorkovskiy with minor
editorialization by me. Not back-patched because the object TRUNCATE hook
was not.
Author: Yuli Khodorkovskiy
Reviewed-by: Joe Conway
Discussion: https://postgr.es/m/CAFL5wJcomybj1Xdw7qWmPJRpGuFukKgNrDb6uVBaCMgYS9dkaA%40mail.gmail.com
---
diff --git a/contrib/sepgsql/hooks.c b/contrib/sepgsql/hooks.c
index 49f32ac..cdf1452 100644
*** a/contrib/sepgsql/hooks.c
--- b/contrib/sepgsql/hooks.c
*************** sepgsql_object_access(ObjectAccessType a
*** 188,193 ****
--- 188,207 ----
}
break;
+ case OAT_TRUNCATE:
+ {
+ switch (classId)
+ {
+ case RelationRelationId:
+ sepgsql_relation_truncate(objectId);
+ break;
+ default:
+ /* Ignore unsupported object classes */
+ break;
+ }
+ }
+ break;
+
case OAT_POST_ALTER:
{
ObjectAccessPostAlter *pa_arg = arg;
diff --git a/contrib/sepgsql/relation.c b/contrib/sepgsql/relation.c
index 714cffe..fa34221 100644
*** a/contrib/sepgsql/relation.c
--- b/contrib/sepgsql/relation.c
*************** sepgsql_relation_drop(Oid relOid)
*** 517,522 ****
--- 517,562 ----
}
/*
+ * sepgsql_relation_truncate
+ *
+ * Check privileges to TRUNCATE the supplied relation.
+ */
+ void
+ sepgsql_relation_truncate(Oid relOid)
+ {
+ ObjectAddress object;
+ char *audit_name;
+ uint16_t tclass = 0;
+ char relkind = get_rel_relkind(relOid);
+
+ switch (relkind)
+ {
+ case RELKIND_RELATION:
+ case RELKIND_PARTITIONED_TABLE:
+ tclass = SEPG_CLASS_DB_TABLE;
+ break;
+ default:
+ /* ignore other relkinds */
+ return;
+ }
+
+ /*
+ * check db_table:{truncate} permission
+ */
+ object.classId = RelationRelationId;
+ object.objectId = relOid;
+ object.objectSubId = 0;
+ audit_name = getObjectIdentity(&object);
+
+ sepgsql_avc_check_perms(&object,
+ tclass,
+ SEPG_DB_TABLE__TRUNCATE,
+ audit_name,
+ true);
+ pfree(audit_name);
+ }
+
+ /*
* sepgsql_relation_relabel
*
* It checks privileges to relabel the supplied relation by the `seclabel'.
diff --git a/contrib/sepgsql/selinux.c b/contrib/sepgsql/selinux.c
index b7c489c..5e6189a 100644
*** a/contrib/sepgsql/selinux.c
--- b/contrib/sepgsql/selinux.c
*************** static struct
*** 360,365 ****
--- 360,368 ----
"lock", SEPG_DB_TABLE__LOCK
},
{
+ "truncate", SEPG_DB_TABLE__TRUNCATE
+ },
+ {
NULL, 0UL
},
}
diff --git a/contrib/sepgsql/sepgsql-regtest.te b/contrib/sepgsql/sepgsql-regtest.te
index 5d9af1a..569c4da 100644
*** a/contrib/sepgsql/sepgsql-regtest.te
--- b/contrib/sepgsql/sepgsql-regtest.te
*************** allow sepgsql_regtest_var_t sepgsql_regt
*** 152,157 ****
--- 152,165 ----
optional_policy(`
gen_require(`
+ class db_table { truncate };
+ ')
+
+ allow sepgsql_regtest_superuser_t sepgsql_regtest_foo_table_t:db_table { truncate };
+ ')
+
+ optional_policy(`
+ gen_require(`
role unconfined_r;
')
postgresql_role(unconfined_r, sepgsql_regtest_foo_t)
diff --git a/contrib/sepgsql/sepgsql.h b/contrib/sepgsql/sepgsql.h
index 4787934..31828e9 100644
*** a/contrib/sepgsql/sepgsql.h
--- b/contrib/sepgsql/sepgsql.h
***************
*** 145,150 ****
--- 145,151 ----
#define SEPG_DB_TABLE__INSERT (1<<8)
#define SEPG_DB_TABLE__DELETE (1<<9)
#define SEPG_DB_TABLE__LOCK (1<<10)
+ #define SEPG_DB_TABLE__TRUNCATE (1<<11)
#define SEPG_DB_SEQUENCE__CREATE (SEPG_DB_DATABASE__CREATE)
#define SEPG_DB_SEQUENCE__DROP (SEPG_DB_DATABASE__DROP)
*************** extern void sepgsql_attribute_relabel(Oi
*** 312,317 ****
--- 313,319 ----
extern void sepgsql_attribute_setattr(Oid relOid, AttrNumber attnum);
extern void sepgsql_relation_post_create(Oid relOid);
extern void sepgsql_relation_drop(Oid relOid);
+ extern void sepgsql_relation_truncate(Oid relOid);
extern void sepgsql_relation_relabel(Oid relOid, const char *seclabel);
extern void sepgsql_relation_setattr(Oid relOid);
diff --git a/contrib/sepgsql/test_sepgsql b/contrib/sepgsql/test_sepgsql
index 7530363..3a29556 100755
*** a/contrib/sepgsql/test_sepgsql
--- b/contrib/sepgsql/test_sepgsql
*************** echo "found ${NUM}"
*** 287,292 ****
echo
echo "============== running sepgsql regression tests =============="
! make REGRESS="label dml ddl alter misc" REGRESS_OPTS="--launcher ./launcher" installcheck
# exit with the exit code provided by "make"
--- 287,306 ----
echo
echo "============== running sepgsql regression tests =============="
! tests="label dml ddl alter misc"
!
! # Check if the truncate permission exists in the loaded policy, and if so,
! # run the truncate test
! #
! # Testing the TRUNCATE regression test can be done by manually adding
! # the permission with CIL if necessary:
! # sudo semodule -cE base
! # sudo sed -i -E 's/(class db_table.*?) \)/\1 truncate\)/' base.cil
! # sudo semodule -i base.cil
!
! if [ -f /sys/fs/selinux/class/db_table/perms/truncate ]; then
! tests+=" truncate"
! fi
+ make REGRESS="$tests" REGRESS_OPTS="--launcher ./launcher" installcheck
# exit with the exit code provided by "make"
