Re: Updates of SE-PostgreSQL 8.4devel patches (r1268) - Mailing list pgsql-hackers

Bruce Momjian wrote:
> Bruce Momjian wrote:
>> Second, system catalogs; are you going to have separate columns for
>> SQL-level row security and SE-Linux security?  If so, is the SE-Linux
>> column only going to be created by the --enable-selinux build?  If so,
>> that is going to mean that the /data directory is going to be affected
>> by the --enable-selinux flag, which is not ideal --- it would be good if
>> someone could switch to an SE-Linux binary without to dump/reload.  One
>> nice idea would be to have one "security" column and allow both
>> SQL-level and SE-Linux security values to be stored in the column,
>> perhaps at the same time;  that way there is no new column needed for
>> SE-Linux.
>>
>> I know I suggested the security column act like the system oid column,
>> but now I am thinking I was wrong.  The system oid column stores an
>> auto-generated value so it was necessary to have it be specified at
>> CREATE TABLE time because it is guaranteed to take storage space.  For
>> the security column, in most cases it will be null, meaning it would not
>> take any storage space becuase we use a null bitmap for null column
>> values.  So, perhaps the security column should be in every table and we
>> can just make it default to null;  I think that would give us much more
>> flexibility to add security to existing tables.
> 
> Sorry, I am confusing system columns, like pg_class.relname, with
> user-table system columns, like mytable.xmin.  I don't think we have the
> ability of having user-table system columns nullable, right?

Currently, it assumes system columns should not be NULL.
heap_getsysattr() always set the given 'isnull' 'false'.

> We do have a per-row HEAP_HASOID bit, so I wonder if we can have a
> HEAP_HASSEC bit too.  Right now the HEAP_HASOID is controlled by the
> CREATE/ALTER table;

The current patch add HEAP_HASSECURITY bit to t_infomask. :-)
When it is false, its security field is not available and not allocated.
> I am unclear how hard it would be to allow it to be> controlled via INSERT, meaning it would be present only if the
rowhad a> SEC value supplied.
 

It is impossible, and not suitable for SE-PostgreSQL.
The HeapTuple is allocated prior to fetch INSERT'ed value.
In addition, SE-PostgreSQL assigns its default security context when no
security attribute is given.

> Here is our OID column on some rows but not others:
> 
>     test=> CREATE TABLE test (x INT) WITH oids;
>     CREATE TABLE
>     test=> INSERT INTO test VALUES (1);
>     INSERT 16392 1
>     test=> ALTER TABLE test SET WITHOUT OIDS;
>     ALTER TABLE
>     test=> INSERT INTO test VALUES(2);
>     INSERT 0 1
>     test=> SELECT oid, * FROM test;
>     ERROR:  COLUMN "oid" does not exist
>     LINE 1: SELECT oid, * FROM test;
> 
> but it has per-table granularity;  the oid value for '1' is still
> stored, but is no longer visible;  that would not work for the security
> value.

If we can control the Row-level ACLs via table options, what should be
happen when we turn off the feature?
IMO, it is natural to ignore Row-level ACLs independent from whether
the stored tuple actually has ACLs, or not.

> The complexity is that the column would always exist (unlike OID), but
> would be NULL if not assigned to.  This would allow any table to have
> security by default, and have no overhead if security is not defined for
> the row.  Now, I realize SE-Linux would have security defined for each
> row, but SQL-level row security might not, and this would allow the
> feature to be very useful, perhaps with a GUC that required security for
> SE-Linux, and /data would be the same for SE-Linux and non-SE-Linux.  If
> we could make the row behave like this we might use separate columns for
> SQL-level and SE-Linux security because there would be no storage
> overhead unless security was used.

I basically agree for the idea of separate columns.
My preference is "acl column" as general column (not a system column)
which is defined at limited number of tables with ROW_LEVEL_ACL=ON.

Thanks,
-- 
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai@ak.jp.nec.com>