Authoritative hooks incorporating Chris's review comments

offerat_private

Patch is relative to ChangeSetat_private, 2001-10-11 23:28:19-07:00,
chrisat_private

Following on from Chris's review of the authoritative hooks patch, here it
is updated to meet his comments.

What's changed since the orginal proposal ?
===========================================

===== fs/attr.c 1.8 vs edited =====
  moved security_ops->inode_ops->setattr() right after inode_change_ok()
and before the quota is transfered.

===== fs/fcntl.c 1.6 vs edited =====
  use "err" not retval for function returning void.

===== kernel/sys.c 1.18 vs edited =====

sys_setpgid()
    moved hook to before any kernel DAC checks are made (ie. MAC before
DAC). Alternative proposal would be to keep the hook positioned where is is
currently but re-organize the code to not not bypass the hook and add an
errno parameter to the hook (see my message dated Oct 18 03:15:31 PM).
Either would suffice for MAC requirements, moving the hook position is
easier based on the assumption that Wirex doesn't care about this
particular hook, if this is an invalid assumption we'll fix it to fit in
with what they prefer.

sys_setrlimit()
    No change. Chris did make a comment that the code was ugly with all the
!retval's, but that does ensure that we don't follow a code path that we
wouldn't in pre-LSM if we failed a capable check. 

===== kernel/module.c 1.16 vs edited =====

sys_init_module()
    Move the capable check until after we have obtained the module name (in
kernel space). Moved the hook before the module is validated (and right
after the capable check). I raised a question in a message dated Oct 19
08:40:32 AM regarding whether it was enough to have the module name be
valid at the point the hook is called ? No response, so assuming that any
reasonable security policies will not care.

sys_delete_module()
    Eliminate the extra hook call in the automatic reaping case.

===== kernel/ptrace.c 1.6 vs edited =====
    The question about whether rmb() should be called even under error
conditions (failed a capable() test) is unclear. From Greg's much earlier
comment I got the impression that it was important to maintain the same
code path as pre-LSM for race-condition sensitive code such as this, which
is why we only call rmb() if no error exists, left it as originally
submitted.

    Do not allow an already traced process to call the hook.

===== include/linux/security.h 1.92 vs edited =====
    Do not mark "authoritative" clause as being caller dependent.
    Add error code to init_module.

===== security/capability_plug.c 1.68 vs edited =====
    Fixed cap_ptrace() and cap_capset_check() to return kerror if set.

===== security/dummy.c 1.6 vs edited =====
    Fix dummy_ipc_permission() to return kerror.

As I see it, there are two possible outstanding technical issues before
everyone who is going to be happy is happy; 
    init_module(), is not calling the hook before the module is validated
going to bite anyone ? (this behavour is documented) ; 
    ptrace(), we've been over this before and it seems like we have no
definitive answer on the calling of rmb(), it might look ugly, but its
conservative.

Thanks Chris for spending the time to review the earlier work.

Have a good one everyone.

richard 

-- 
-----------------------------------------------------------------------
Richard Offer                     Technical Lead, Trust Technology, SGI
"Specialization is for insects"
___________________________________________On sabatical Nov 8 -> Nov 30