Re: Vnode scope implementation

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

On Sat, Jul 04, 2009 at 08:14:19PM +0300, Elad Efrat wrote:
> Once the back-end is in place, I'll follow-up with some more diffs
> transitioning various file-systems to use kauth(9) in different places
> -- access, chflags, chmod, etc.

I'd like to see some examples of what this is expected to look like.

 >  /*
 > + * Vnode scope - actions.
 > + */
 > +#define     KAUTH_VNODE_READ_DATA           (1 << 0)
 > +#define     KAUTH_VNODE_LIST_DIRECTORY      KAUTH_VNODE_READ_DATA
 > +#define     KAUTH_VNODE_WRITE_DATA          (1 << 1)
 > +#define     KAUTH_VNODE_ADD_FILE            KAUTH_VNODE_WRITE_DATA
 > +#define     KAUTH_VNODE_EXECUTE             (1 << 2)
 > +#define     KAUTH_VNODE_SEARCH              KAUTH_VNODE_EXECUTE
 > +
 > +#define     KAUTH_VNODE_ACCESS              (1 << 31)

I assume there's some reason these need to be bits rather than an
enumeration; however, when you go to implement you'll find you've left
a few things off here.

These are the vnode actions VINO's security system defined. I believe
that they're sufficient, and that at least most of them are necessary.
I don't claim that there's anything particularly good about this way
of structuring things; on the other hand, it *was* implemented and
*did* work.

/* These are what you can do to a file. */
enum file_may_t {
        MAY_READ,               // Read from a file (or device or...)
        MAY_APPEND,             // Write at end of a file
        MAY_OVERWRITE,          // Write anywhere in a file
        MAY_LINK,               // Link (or unlink) a file
        MAY_REMSTICKY,          // Delete a file from a public area
        MAY_EXECUTE,            // Execute contents of a file
        MAY_LOCK,               // Lock this file
        MAY_CHMOD,              // Chmod this file
};

/* These are what you can do to a directory. */
enum dir_may_t {
        MAY_READDIR,            // list a directory
        MAY_SEARCH,             // Name/open a subfile
        MAY_MOUNT,              // use as a mount point
        MAY_CREATE,             // add a new file
};

/* These are what you can do to a directory and a file inside it. */
enum dir_contents_may_t {
        MAY_ADDLINK,            // add new link to an existing file
        MAY_UNLINK,             // remove a file or directory
};
 

 > +int
 > +secmodel_bsd44_suser_vnode_cb(kauth_cred_t cred, kauth_action_t action,
 > +    void *cookie, void *arg0, void *arg1, void *arg2,
 > +    void *arg3)
 > +{
 >  [...]
 > +    int fs_decision;
 >  [...]
 > +    fs_decision = (int)(unsigned long)arg2;

Can't you figure out some way to arrange this that doesn't require
casting integers to pointers and back?

(And if you really really must do that, use {u,}intptr_t, not unsigned
long.)

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