Re: Vnode scope implementation

[Elad Efrat <elad%NetBSD.org@localhost>]

On Mon, Jul 6, 2009 at 1:53 AM, David 
Holland<dholland-tech%netbsd.org@localhost> wrote:

>>> Either way, I'd like to see the complete set of actions you propose so
>>> it can be checked for completeness and orthogonality.
>>
>> Unfortunately that will not happen.
>
> It should. The set of possible distinct actions is an important part
> of the interface, and (particularly for security!) it should be
> designed rather than allowed to agglomerate.
>
> 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. ;) 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!

>> 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. Some developers that I bugged constantly
off-list can tell you that I examined each piece of code I've replaced
a KAUTH_GENERIC_ISSUSER in rather than blindly sticking arbitrary
requests. We've modified names, implementations, and what not -- but
that's natural evolution of code, so I would not expect things to be
different in this case.

>> David, I've got a feeling that you're very emotional about the vnode
>> scope [...]
>
> I'm emotional because I disagree with you? Right, that's the ticket.

Okay, I take that back. :)

>  > +                    /* 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).

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.

One way to address it, which I thought of originally, was to simply
issue a VOP_GETATTR() in the listeners, and check for the relevant
modes/flags (I've also thought about "caching" the vattr of a vnode in
it). That, however, will require code duplication. What I thought we
could do instead was accommodate for this, let's call it
"short-sighted design", by saying "okay, these are here for long
enough, let's give them special flags, and force some new convention
on newly introduced code" -- which I didn't invent myself, as Apple
does something similar, only that they refer to the immutable flag
explicitly, and I decided to have a general "system flags", and add an
indication for "sticky bit".

FWIW, we don't *have* to have this flag: we can just as easily say,
"this mode bit has been here long enough, let's request file-systems
to export it if they have it through getattr", and secmodels that want
to take action on it have to "query" for it explicitly. Maybe there's
another way I didn't come with. Personally? I don't think it matters
that much. :)

Anyway, what I suggest is that we first agree to check in the code
implementing just the scope and the authorization back-end, so any
-current user can potentially check diffs for the file-systems
themselves.

Thanks,

-e.