security label support, revised - Mailing list pgsql-hackers

The attached patch is a revised version of security label support.

summary of changes:
* The logic to translate object-name to object-id was rewritten
  with the new get_object_address().

* Right now, it does not support labeling on shared database object
  (ie; pg_database), so wrapper functions to XXXLocalSecLabel() were
  removed.

* The restriction of same security label within whole of inheritance
  tree has gone. And, the 'num_parents' argument was also removed
  from the security hook.

* ExecRelationSecLabel() was also removed, although you suggested
  to rename it, because it translate the supplied relation name
  into relation id and handled child tables, but it get unnecessary.

* The chapter of 'External Security Provider' was added.
  It introduces overview of ESP concept and MAC features.
  Perhaps, other structures of chapters are more preferable,
  but I also think we need a draft at the begining of discussion.

* The '--security-label' option was added to pg_dump/pg_dumpall;
  it allows to include security label of objects in the archives.
  The '--no-security-label' option was also added to pg_restore;
  it allows to skip security labels, even if the archive contains
  security labels.

Thanks,

(2010/08/10 5:02), Robert Haas wrote:
> 2010/7/26 KaiGai Kohei<kaigai@ak.jp.nec.com>:
>> The attached patches are revised ones, as follows.
>
> I think this is pretty good, and I'm generally in favor of committing
> it.  Some concerns:
>
> 1. Since nobody has violently objected to the comment.c refactoring
> patch I recently proposed, I'm hopeful that can go in.  And if that's
> the case, then I'd prefer to see that committed first, and then rework
> this to use that code.  That would eliminate some code here, and it
> would also make it much easier to support labels on other types of
> objects.
>
> 2. Some of this code refers to "local" security labels.  I'm not sure
> what's "local" about them - aren't they just security labels?  On a
> related note, I don't like the trivial wrappers you have here, with
> DeleteSecurityLabel around DeleteLocalSecLabel, SetSecurityLabel
> around SetLocalSecLabel, etc.  Just collapse these into a single set
> of functions.
>
> 3. Is it really appropriate for ExecRelationSecLabel() to have an
> "Exec" prefix?  I don't think so.
>
> 4. Please get rid of the nkeys++ stuff in DeleteLocalSecLabel() and
> just use fixed offsets as we do everywhere else.
>
> 5. Why do we think that the relabel hook needs to be passed the number
> of expected parents?
>
> 6. What are we doing about the assignment of initial security labels?
> I had initially thought that perhaps new objects would just start out
> unlabeled, and the user would be responsible for labeling them as
> needed.  But maybe we can do better.  Perhaps we should extend the
> security provider hook API with a function that gets called when a
> labellable object gets created, and let each loaded security provider
> return any label it would like attached.  Even if we don't do that
> now, esp_relabel_hook_entry needs to be renamed to something more
> generic; we will certainly want to add more fields to that structure
> later.
>
> 7. I think we need to write and include in the fine documentation some
> "big picture" documentation about enhanced security providers.  Of
> course, we have to decide what we want to say.  But the SECURITY LABEL
> documentation is just kind of hanging out there in space right now; it
> needs to connect to a broad introduction to the subject.
>
> 8. Generally, the English in both the comments and documentation needs
> work.  However, we can address that problem when we're closer to
> commit.
>
> I am going to mark this Returned with Feedback because I don't believe
> it's realistic to get the comment code committed in the next week,
> rework this patch, and then get this patch committed also.  However,
> I'm feeling pretty good about this effort and I think we're making
> good progress toward getting this done.
>


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