Re: Vnode scope implementation

[David Holland <dholland-tech%netbsd.org@localhost>]

On Mon, Jul 06, 2009 at 05:02:39AM +0300, Elad Efrat wrote:
 > > I realize that it's a considerable amount of work up front to wade
 > > through all the places that will need to be adjusted to come up with a
 > > complete list of all actions that will be required.
 > >
 > > However, I think it's an important exercise at this stage, just as
 > > taking a survey of all the users of namei was an important
 > > prerequisite for the namei changes I've been planning.
 > 
 > I've already waded through a lot of code, file-systems and otherwise,
 > so I'm not worried about that. ;)

Sure, so then you should already have the list of actions you need,
right? :-)

 > All I'm saying is that the list of
 > available actions is not directly related to the scope back-end
 > itself, which (at least as far as bsd44/suser goes) will not even
 > examine the actions -- in other words, I'd like to post diffs like the
 > one I've attached to my email so people can test rather than also
 > attach a vnode scope implementation each time!

That's perfectly true, but what you're proposing to commit include
actions, and I don't think it's a good idea to commit a partial set of
actions in the expectation of sorting it out later. This is partly
because it's a fairly delicate design issue and partly from routine
maintenance/entropy concerns.

For current purposes it would be enough to have one action "ANY",
right? Maybe going with that as an intermediate step would be a good
compromise.

 > >> If you are suggesting that I will be checking in kauth(9) functionality
 > >> without peer review or approval, I suggest you go through some of the
 > >> relevant commit logs.
 > >
 > > Yes, in fact, that history is what concerns me.
 > 
 > I have to disagree here -- since the initial commit regarding the
 > secmodel abstraction, I've always discussed added
 > scopes/actions/requests.

Well, discussion is supposed to be a joint activity. What tends to
happen is more along the lines of proclamation; yes, you do change
things when asked but you tend to commit before the exercise is
complete. It has caused grumbling. It also causes me concern when it
involves API design issues, which are painful to fix up later, and
particularly security, which merits careful consideration if anything
does.

I know I have had trouble keeping up with the rate you've been
proposing and committing things; there are still unanswered mails from
several previous rounds sitting in my tech-kern mailbox. This is
partly because I tend to put off writing long or possibly-contentious
mail, and because I don't generally have time to keep up except in
bursts. But it's also because you've been moving too fast.

(And I realize it's annoying to have to wait. But if you pipeline your
changesets it doesn't have to hold you up much. For some time I've
been using Mercurial to create and maintain local branches; it works
pretty well.)

 > > > +   /* Indicate if the parent directory is sticky. */
 > > > +   if ((dnode->tn_mode & S_ISTXT) != 0) {
 > > > +           action |= KAUTH_VNODE_PARENT_STICKY;
 > > > +   }
 > >
 > > ...and this is the kind of thing that concerns me. The parent
 > > directory being sticky is not an action. The parent directory being
 > > sticky is part of the context in which an action is taken.
 > 
 > Like I said, do not take it as a proposed implementation, it's just
 > something I'm playing with to (a) have a sample file-system working
 > entirely on kauth(9) and (b) see what actions we need to implement to
 > cover all functionality (as you suggest above).

Sure. Except that this is an issue in the interface design.

If you look closely at what I posted upthread you'll note that when I
did this for VINO (abstracting out permission checks) I set up three
different scopes, or at least I think they're analogous to your
scopes, one for directories, one for files, and one for a pair
consisting of a file and its containing directory. To check for unlink
permission you use the pair context, and any consideration of
stickiness can be handled abstractly. (From what I can tell, and from
comments in the source, the MAY_REMSTICKY action that's in the
file-only context is supposed to be internal use only, and ideally
wouldn't have been exposed outside the security code.)

I don't claim that this design of mine, which is from ~1997 and was
meant for a somewhat different purpose, is particularly good, but I do
think this issue needs to be handled more robustly than slipping a
magic flag in, and I do think it some solution ought to be worked out
up front.

 > But, since you did bring it up, I will explain why I did that.
 > 
 > There are two issues with the traditional implementation, or better
 > said, one issue that manifests twice: internals of two different
 > subsystems are taken for granted. Think about the sticky bit
 > directories and root, or the immutable flag and securelevel: a mode
 > bit and a flag -- file-system internals -- are examined with regards
 > to the "admin" user (uid 0) and the securelevel.

Right. I seem to recall saying something at some past point about
needing to abstract either credentials or permissions but not both.
That is, you can either pass a specific credential (euid 0,
securelevel) to something abstracted that knows how to apply it to
fs-specific permissions, or you can give a specific permission
(sticky, immutable) to something abstract that knows how to apply
model-specific credentials to it. Doing neither gives you next to no
flexibility and doing both is messy and difficult.

From what I've seen so far (and from rewriting this section of this
mail repeatedly trying to clarify it) my understanding is that kauth
is not supposed to abstract either credentials or permissions;
instead, it allows patching the decision that arises from applying a
specific representation of credentials to a specific representation of
permissions.

This alone provides next to no flexibility; flexibility is added by
allowing listeners to maintain additional information to use as the
basis of decisions. This additional information might (AIUI) function
as either credentials or permissions, or both.

This is in turn problematic as soon as there are fs-specific
permissions, because applying arbitrary additional credential
information to existing permissions requires that the permissions be
specific and not abstracted. Therefore, every fs-specific permission
that might appear has to be explicitly built into and visible in kauth
so an arbitrary listener can interact with it correctly.

I don't really like that model, although I'm not sure what else to
suggest at the moment.

I've been thinking in terms of abstract representations of credentials
and permissions (because encapsulating each is the best way to go
about centralizing security decisions) and I think this is one of the
reasons we have been talking past each other.

Anyway, if you're going to be potentially working in arbitrary
additional credentials *and* permissions you definitely need to handle
stickiness by providing both the containing directory and the
contained file and letting the listener check for the sticky bit.
Otherwise you can't write a listener that e.g. denies non-exclusive
create in a world-writeable directory, which would clearly be a useful
thing to be able to do.

-- 
David A. Holland
dholland%netbsd.org@localhost