[RFC][PATCH 0/2] (#6 U2) filesystem auditing

[Stephen Smalley]

On Mon, 2005-03-28 at 19:54 -0600, Timothy R. Chavez wrote:
> :: TERMINOLOGY ::
> 
> watch : data that describes a file or directory that should be audited
> watchlist : a linked list of watchlist entries residing on a directory
> watchlist entry (wentry): an entry to a watchlist that contains a watch

Suggestion:  You need to start with a description of your
goal/requirements, and work in your terminology definition as part of it
or immediately after it.

> :: INTRODUCTION ::
> 
> In an effort to make the mainline kernel's audit subsystem Controlled Access 
> Protection Profile (CAPP)/Evaluation Assurance Level (EAL) 4 compliant, this 
> patch adds file system auditing support to the audit subsystem.  Such 
> support is essential in meeting certification requirements because it allows 
> the evaluator to confirm all claims made about the Target of Evaluation (TOE) 
> regarding the behavior of file system objects (which are outlined in the 
> Security Target for the given evaluation) by consulting the audit log.

Suggestion:  Rework this to focus on the specific requirements called
out by CAPP that are not met by the current audit framework and why.
You can't expect all kernel developers to have read CAPP (or to want to
do so) or to even care about it particularly.  Instead, you want to tie
to actual useful functionality that is not met by the current audit
framework that you need for CAPP.

> To achieve such results, it's necessary for the audit subsystem to identify 
> and keep track of such objects.  Due to the abstract nature of "identity" 
> with regards to file system objects and how that "identity" translates 
> between the user's perspective and the kernel's perspective and visa-versa, a 
> fairly strict definition is devised.  This implementation uses a scheme by 
> which parent directories have a "watchlist" that qualifying children may 
> point into at the "watchlist entry" that holds their "watch".  Pointing at a 
> "watchlist entry" translates into "being watched".

I'd expect someone to ask why you can't just register filters on
(device,inode) pairs using the existing audit framework to identify
objects, so you need to explain why that breaks down for preserving
auditing on objects that undergo transactions like /etc/shadow where
they are re-created.

> :: DESCRIPTION ::
> 
> Below is a basic description of this patches capabilities.  These capabilities 
> are enabled by the user space program, auditctl, which is available in the 
> audit package (found at: http://people.redhat.com/sgrubb).  

Possibly "functionality" rather than capabilities, as the latter has
other baggage associated with it (e.g. POSIX.1e capabilities).

> 1.  Insertions
> 
> When the administrator targets a file system object for audit, they do so by 
> <path> name.  This is an absolute target -- meaning, the administrator targets 
> the file system object by name, on a given device, in a given namespace.

I understand what you mean, but others might take you to be saying that
the path is an "absolute path" in some manner.  The point is that the
admin (or auditd) specify objects by names within their own namespaces
(whether absolute or relative paths), but the name (aside from the last
component) becomes irrelevant to the actual management of watches by the
kernel after the initial lookup upon watch insertion.

> Creation:
> We use hooks in d_instantiate() and d_splice_alias() to immediately attach 
> watches, if they exist, to newly created / spliced dentries.

Another possible phrasing:  These hooks cover initial audit setup for
inodes that are newly created and for existing inodes when they are
first looked up, prior to becoming accessible via the dcache.

> Watch/removal:
> We use the __d_lookup() hook for two reasons: to assign a new "watch", if one 
> exists at this location (ie: a hardlink that's just become "unwatched" exists 
> in a location that has a "watch") and to detach unhashed (invalid) watchlist 
> entries (wentries) on inodes.

Another possible phrasing:  This hook covers updating the inode audit
data if necessary upon subsequent lookups when the inode is already
accessible via the dcache, both to reflect removal of watches and to
handle changes in state since the initial setup.

> Deletion:
> The d_delete() hook is used to drain watchlists and detach from a "watch".  
> We've effectively left the "watch".

It occurs to me that this may be the wrong place for this hook.  Note
that if any other thread is still using the dentry, d_delete only
unhashes the dentry and defers actual deletion until it has no users.
So if you always detach at this point, you could lose audit information
for other users.  Looks like you might need to do this in dentry_iput()
instead, where you truly release the dentry's inode.

> Creation:
> For creation, we have hooks in vfs_link()/symlink()/create()/mkdir()/mknod().  
> Once we have the inode (post creation), and we're attached (post 
> audit_attach_watch), we want to generate a record.

You likely need to explain why may_create is not adequate (i.e. no inode
yet).  Although this almost makes me wonder whether i_audit should be
d_audit, i.e. a field of the dentry, as your entire scheme is based on
(parent directory, component name) pairs anyway.  Why MAY_WRITE|MAY_EXEC
here?  It is true that you would have checked search permission to the
parent directory, but that is handled by your permission hook, and this
is for the newly created inode, not the directory, right?  Is there an
issue with regard to the fact that there will be no audit_notify_watch()
call here if the create fails in the fs code? 

> Open:
> I think these hooks can be dropped.  Will do before we send out to 
> linux-fsdevel.

Yes, I don't see what they add.

-- 
Stephen Smalley <sds at tycho.nsa.gov>
National Security Agency