Bruce Momjian wrote:
KaiGai Kohei wrote:
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);

As I mentioned below, this might not be necessary, meaning that row
security is enabled for rows where you set the row security value and
does not need to be turned on at create table time.

The reason why I proposed the ROW_LEVEL_ACL option is to reduce storage
consumption for NULL bitmap. For example, the following INSERT originally
didn't need NULL bitmap, but the "acl column" is implicitly NULL, so the
HeapTuple has HEAP_HASNULL flag and NULL bitmap is allocated.

  INSERT INTO t VALUES (1, 'aaa');

If it is acceptable, I don't think the option is necessary.

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.

Right, if the "security_context" always exists, but is null, moving to
SE-Linux would allow them to add security without dump/reload.

Yes, but it is not recommendable in generally. For meaningful access
controls, "unlabeled" objects should be labeled properly. :)

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"?

So "security_context" has to be the PGACE column name.  I would say
security_acl or just secacl because I think it is going to match our
existing ACL system pretty closely.

Agreed,

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

If we can make the column not take any space if it is null then we can
have both columns always present and that way /data is the same for
SE-Linux and non-SE-Linux.  We do have columns like xmin which don't
show in SELECT * and I don't see a problem adding two more.

I imagine we would need to modify COPY and pg_dump to specify if we are
to dump/load secacl; I see you already did "security_context".

Yes, it has to be also done.

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

Ah, that is a good point, that if we have "security column" which is
usually null then we are requiring the NULL bitmask.

I was thinking of having them be system columns and therefore only the
bitmask would specify if the value exists or not.  I am again thinking
of having both columns always exist, making your code clearer and more
portable for people going to and leaving SE-Postgres.

The HEAP_HASSECURITY flag looks like a specific purpose NULL-bitmap
which only shows whether the tuple has security value, or not.

The following proposal is my idea:

- The Row-level ACLs is implemented as a hardcoded feature, not a guest of
   PGACE security framework.

Yep, good.

- It can be enabled/disabled via table options as:
       CREATE TABLE t (
           a   int,
           b   text
       ) WITH (ROW_LEVEL_ACL=ON);

Can we make it just always on?  See above.

If we assume users set up Row-level ACLs for specific tables, per-table
option is meaningful for reduction of NULL-bitmap space in the tuple
without any NULL-values on general columns.

- When the Row-level ACLs is enabled on CREATE TABLE, it implicitly add
   a column to represent Row-level ACLs. This column is 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.

Right, just like other system columns, e.g. xmin.

- The "acl column" is allowed to update by the table owner or superuser.

Yep.

- If a table has Row-level ACLs enabled, ExecScan() checks visibility of
   fetched tuples, and ignore violated tuples.

Yep.

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.

I feel there must still be open issues that I hope we can address before
you need to do more recoding of the patch.  And hopefully other
community members will be involved as well.


Thanks,
--
OSS Platform Development Division, NEC
KaiGai Kohei <[EMAIL PROTECTED]>

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to