security label support, revised

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(at)ak(dot)jp(dot)nec(dot)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(at)ak(dot)jp(dot)nec(dot)com>