Re: Updates of SE-PostgreSQL 8.4devel patches (r1268)

Bruce Momjian wrote:
> KaiGai Kohei wrote:
>>> But your larger point is that SQL-row-level security should always be
>>> available, which I just posted about.
>> If so, it should be hardcoded on somewhere, no need to be implemented
>> as a guest of PGACE security framework. Its purpose is to implement
>> enhanced security mechanisms with minimum impact to core facilities.
>>
>> If you intend to implement is as a hardcoded feature, I can agree.
>> Please wait for a few days, I'll try to implement it.
>> So, ignore the 6th patch during the days and make progress to review
>> the rest of patches.
> 
> Hold, please don't code yet.  You might have already read our
> developer's FAQ:
> 
>     http://wiki.postgresql.org/wiki/Developer_FAQ#What_do_I_do_after_choosing_an_item_to_work_on.3F
> 
> but I feel we need to follow part of it to prevent you from repeatedly
> having to adjust your patch, which I think has happened in the past.
> 
> Let's decide exactly what configure options, GUC options, system catalog
> changes, and user-visible SQL command syntax we are going to use for the
> patch before you recode anything.

The guest of PGACE security framework should be chosen by configure option.
It also means any other facilities except for the guest of PGACE can be
enabled/disabled by other kind of options.

If the Row-level ACLs should be always enabled (independent from SE-PostgreSQL),
I think it should be hardcoded outside of PGACE, and enabled/disabled by
any other way. Table option is a candidate, like:
  CREATE TABLE t (      a   int,      b   text  ) WITH (ROW_LEVEL_ACL=ON);

> For example, what configure flags are we going to have?  I assume just
> '--enable-selinux', right?

Yes, currently.

> Second, system catalogs; are you going to have separate columns for
> SQL-level row security and SE-Linux security?

Yes, I think these different properties are stored within separate
columns, as if a file has both of permission masks (-rwxr-x---) and
security context of SELinux.

> 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.

As you noticed, a "conditional system column" can be open to discuss.
If someone switch his binary to SE- version, the tables already defined
in $PGDATA don't have "security_context" system column. SE-PostgreSQL
handles tuples stored within the table as "unlabeled" one, but it does
not allow users to access the attribute due to lack of proper system
column.

In addition, we may have to refer security context of tuples via
"tuple_acl" system column, when someone switch his binary from
the Row-level ACL to SE-PostgreSQL. :)

The naming is a difficult matter. It seems to me "security" is too
general term. How do you think "security_label" or "security_attribute"?

> 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.

Here is a differences in frequency of valid value in security column
between SE-PostgreSQL and Row-level ACLs.
SE-PostgreSQL requires all the tuple should be labeled properly.
(And, I guess any other upcoming feature to support another secure OS
has similar requirement.)
But we assume the Row-level ACLs is enabled on the limited number of
tables, so it need to specify an option to activate on CREATE TABLE.

Please note that I distinguish between "security column" and "acl column"
here. In my opinion, the security column should be a system column, but
the acl column can be implemented as a regular column that is appended
depending on user's needs.

When we represent a NULL of "security column", it requires HEAP_HASSECURITY
to be set on t_infomask. Here is no additional storage consumption.
If it is represented as NULL bitmaps, it requires a tuple has its null bitmaps
which need 4-bytes at least, so it can make additional storage consumption.
Thus, NULL-bitmap approach is not suitable to represent "security column"
because it has to be defined for any tables. But, it can be suitable for
"acl column".

The following proposal is my idea:

- The Row-level ACLs is implemented as a hardcoded feature, not a guest of  PGACE security framework.
- It can be enabled/disabled via table options as:      CREATE TABLE t (          a   int,          b   text      )
WITH(ROW_LEVEL_ACL=ON);
 
- When the Row-level ACLs is enabled on CREATE TABLE, it implicitly add  a column to represent Row-level ACLs. This
columnis defined as aclitem[].
 
- The "acl column" is not expanded via "SELECT * FROM ...", but we can specify  it explicitly like "SELECT tuple_acl, *
FROM..." as system column doing.
 
- The "acl column" is allowed to update by the table owner or superuser.
- If a table has Row-level ACLs enabled, ExecScan() checks visibility of  fetched tuples, and ignore violated tuples.

> Finally, I am wondering what other things should be adjusted that I have
> not thought of yet.  I would like to make KaiGai's job as easy as
> possible.

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