Re: security label support, part.2

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.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company