Re: [RFC] Security label support - Mailing list pgsql-hackers

(2010/05/28 4:12), Stephen Frost wrote:
> KaiGai,
> 
> * KaiGai Kohei (kaigai@ak.jp.nec.com) wrote:
>> As we talked at the developer meeting on Ottawa, it needs to provide
>> a capability to assign a short text identifier on database objects
>> to support label based ESP (such as SELinux).
>> So, I'd like to propose a few approaches to support security label
>> as a draft of discussion.
> [...]
>> [2] Using OID as a key of text representation in separated catalog
>> ------------------------------------------------------------------
>>
>> This idea is similar to pg_description/pg_shdescription.
>> A new system catalog pg_seclabel and pg_shseclabel stores text form
>> of labels for pair of the relation-Id, object-Oid and object-Subid.
>> It does not damage to the schema of existing system catalog,
> 
> Right, this is the approach that was agreed to during various
> discussions and is, I believe, what Robert is currently working on.
> 
Good.

I've been allowed to work it with high priority.
If your hands are not free, should I work on it?

>> It adds two new system catalogs; pg_seclabel (local) and pg_shseclabel (shared).
> 
> Do we really need a shared catalog initially?  Right now, we're just
> talking about labels for tables and columns.  Let's not overcomplicate
> this.  We can always add it later.
> 
Perhaps, I can agree to add it later. I believe the pg_seclabel and pg_shseclabel
shall have identical schema, so we can easily add it.

>> We also add a dependency between the labeled object and the security
>> label itself. It also enables to clean up orphan labels automatically,
>> without any new invention.
> 
> I agree that we need to address this.  I am kind of curious how this is
> handled for comments?  It appears to be, but I don't see an entry in
> pg_depend when a comment is added to an object, yet the entry in
> pg_description disappears when a table is dropped.<Shrug>
> 

Sorry, I missed the special handling of pg_description.
However, what we should do here is same as pg_description without
any new invention.

>> However, it also has a limitation from the viewpoint of long-term.
>> > From the definition, OID of database objects are unique. So, we cannot
>> share text form of labels even if massive number of database objects
>> have an identical security label; it can lead waste of storage consumption
>> because of the duplicated security labels. So, this idea shall be switched
>> to the [3] when we support row-level security with ESP.
>> But I think the idea [2] is reasonable in short-term development.
> 
> Row level security might not even use this catalog directly but perhaps
> another one.  That's a discussion for a much later time though.
> 

Yep, we can design it later.

>> * SQL Statement
>> ---------------
>>
>> It also need to provide SQL statement to manage security label of the database
>> object. I plan the following statement to change the security label.
>>
>>    ALTER xxx<name>  SECURITY LABEL TO 'label';
> 
> Rather than adding more cruft around ALTER, I believe the plan is to add
> a new top-level command (eg: 'SECURITY LABEL ON'), just like the COMMENT
> ON syntax.
> 

It seems to me the syntax like COMMENT ON gives us an incorrect impression.
The role of ALTER is (basically) to alter an existing property of the object,
such as owner, name, schema and so on. Meanwhile, COMMENT ON is used for
both assignment of new description and update of existing description.

The security label is a part of the properties of object to be assigned on
the creation time, such as owner-id. (Even if ESP module is not loaded on
the creation time, it assumes someone is labeled on the unlabeled object.)

The ALTER SECURITY LABEL TO shall be implemented as an individual code path,
like ALTER xxx RENAME TO or ALTER xxx SCHEMA TO, so the patch shall not be
invasive to existing ALTER implementation.

I don't think a new top level 'SECURITY LABEL ON' is not a good naming,
although its internal catalog layout is similar to pg_description.

>> When the ALTER command is executed, ESP module validate the given label,
>> in addition to permission checks to relabel it.
> 
>> If no ESP module is available, the ALTER always raises a feature-not-supported
>> error.
> 
> Right.  We do need to identify what the hook needs to look like.  I
> would think just passing a pg_seclabel (or whatever) structure which
> would represent the new row in the table (possibly replacing an existing
> row, if one exists, but the hook can figure that out).  The hook can
> then figure out the user and any other information it needs to know
> based on that and either allow or not allow the change.
> 

I think ESP should not need to handle whether the target object is already
labeled, or not. It can be push down into a common function.
For example, we can easily provide seclabel_insert_or_update() that allows
to insert a new label into pg_seclabel (if not exist) or update an old label
of pg_seclabel (if exist).

I plan the following design typically. In this case, the ESP hook validates
the given new label and check its permission to relabel it.
 void AlterRelationSecLabel(RangeVar *relation, const char *new_label) {     relOid = RangeVarGetRelid(relation,
false);
     /* Default PG Permission check */     if (!pg_class_ownercheck(relOid, GetUserId()))
aclcheck_error(....);
     /* ESP Permission check */     if (!check_relation_relabel_hook)         ereport(ERROR, ..."feature not
supported"...);    else         (*check_relation_relabel_hook)(relOid, new_label);
 
     /* call a function to insert/update a entry to pg_seclabel */     seclabel_update_or_insert(RelationRelationId,
relOid,0, new_label); }
 

>> In my original SE-PostgreSQL design, it provided an option to specify
>> an explicit security label in CREATE xxx statement, but I discarded
>> the idea, because the implementation of CREATE statement has much
>> variations for each object class (so the patch will be invasive),
>> and it is a fungible functionality using ALTER.
> 
> As with other things, this could be done in a transaction rather than
> cluttering up CREATE, etc, statements.  Supporting a default label for
> objects might be something which could be added later.
> 

Yes, like an ownership of object, something default (that will be given
by ESP) shall be assigned on the creation time, then we will be able to
alter the default one to another one using individual statement.
I agree that we can implement the default label support later.

During we don't have default label support, SELinux will presume unlabeled
objects have something alternative label. But no matter in this stage.

Anyway, what I want to say was an option to provide an explicit label
other than the default in CREATE statement may be unnecessary, because
the patch will become invasive. It should be computed internally.
One other reason is we also don't have an option to give an explicit owner-Id
other than the default.

Thanks,
-- 
KaiGai Kohei <kaigai@ak.jp.nec.com>