Re: Changes to LSM phase 1 for audit.

gregat_private

On Tue, Jul 17, 2001 at 07:09:58AM -0700, richard offer wrote:
> 
> 
> Attached is SGI's second proposal for adding audit support to LSM. We've
> listened to all the comments made after the first patch presented for
> review, even if we haven't actively participated in the discussions.

Ah, but discussions aren't really discussions unless everyone
participates :)

> In particular, the message that the patch was too big and that audit was
> intended for phase 2, so we've redone it. This piece of work only includes
> modifications to the existing hooks, either in terms of their prototypes or
> placement. The intention being to freeze an existing audit friendly API
> before tackling the issues of adding new hooks.

Thanks for splitting this up.  It makes it _much_ more manageable.
However you are really doing 3 different things in this patch.  I'd
recommend submitting 3 different patches next time.

> Prototypes have changed to reflect additional information that audit really
> wants to track, ie fd's. These have been added as additional parameters,
> not replacements (that was my personal mistake last time).

Bleah.  I understand that you would like to track fds purely for an
audit case (from the kernel standpoint, the fd doesn't mean much at all,
the struct file * means everything.)  But why can't you simply get the
fd from the struct file * if you really want it?  That doesn't burden
the hook interface to pass another variable that almost no security
module will need.

And since you don't need to pass the fd, my comment on the
LSM_NOFD_AVAILABLE atrocity is not needed...

> The placement of some hooks has been modified to make it more consistant,
> so for example, we always call the post hook, rather than only if there was
> an error. Some hooks have been moved ahead of DAC checks (or capable()
> calls) in light of Stephen's comments on July 9th.

About moving the hooks before the DAC checks, I don't really mind this,
but I know some people will have a real problem with this.  Anyone else
want to comment on this?  Speak up now, or have to change your security
module greatly later :)

The addition of the error parameter doesn't bother me, looks nice.

> By saying no to our first patch, we were forced to go back to re-think the
> implementation and have come up with something that seems to work much
> better for all concerned. In particular, we think we can get away with not
> needing the return_status() hook. In addition, Stephens idea of using
> systems call interposition was really useful in getting us to re-evaluate
> our design considerations.

Well, since you don't need the fd passed to every function, I'd say no
to this patch again.  Take that out, get buy in from everyone else on
moving the hooks, and then split that into 2 patches (as they do 2
different things).

thanks,

greg k-h

_______________________________________________
linux-security-module mailing list
linux-security-moduleat_private
http://mail.wirex.com/mailman/listinfo/linux-security-module