Skip to contentSkip to conversation

(#3) file system auditing

RFC PATCH In review Timothy R. Chavez 4 participants 5 messages Apr 26, 2005 – Apr 28, 2005

Subsystems (3)
VFS SELinux security module Audit
lore
Share & tools
lore

Timothy R. Chavez

author
#0 lore raw

RFC PATCH 1/1 (#3) file system auditing

Hello,

The audit subsystem is currently incapable of auditing a file system object based on its location and name. This is critical for auditing well-defined and security-relevant locations such as /etc/shadow, where the file is re-created on each transaction, and cannot rely on the (device, inode)-based filters to ensure persistence of auditing across transactions. This patch adds the necessary functionality to the audit subsystem and VFS to support file system auditing in which an object is audited based on its location and name. This work is being done to make the audit subsystem compliant with Common Criteria's Controlled Access Protection Profile (CAPP) specification.

This is the third (#3) RFC on linux-fsdevel.

I've omitted the basic design overview and hook descriptions that were featured in submission #1:

Part 1/2:
http://marc.theaimsgroup.com/?l=linux-fsdevel&m=111273925602430&w=2
Part 2/2:
http://marc.theaimsgroup.com/?l=linux-fsdevel&m=111274108905604&w=2

CHANGELOG (from submission #2 to #3)

- Removed audit.h include from fs.h and did a forward declaration of audit_data
- Fixed audit_master_watchlist locking
- Removed audit_master_* helper functions
- Added dput needed for d_lookup in audit_watch_insert
- Added audit_filesystem_init() to Redhat/audit.c
- Fixed watch listing in Redhat/audit.c
- A audit_wentry reference leak was fixed in audit_send_watch()
- Changed order of deletion in audit_destroy_entry to preserve correct watch
- Initialized spinlock *cough*
- Removed LOOKUP_FOLLOWs coupled with LOOKUP_PARENT

-tim

fs/dcache.c +9 -0
diff -Nurp linux-2.6.12-rc2-mm1~orig/fs/dcache.c linux-2.6.12-rc2-mm1~audit/fs/dcache.c
@@ -32,6 +32,7 @@
 #include <linux/seqlock.h>
 #include <linux/swap.h>
 #include <linux/bootmem.h>
+#include <linux/audit.h>
 
 /* #define DCACHE_DEBUG 1 */
 

@@ -97,6 +98,7 @@ static inline void dentry_iput(struct de
 {
 	struct inode *inode = dentry->d_inode;
 	if (inode) {
+		audit_attach_watch(dentry, 1);
 		dentry->d_inode = NULL;
 		list_del_init(&dentry->d_alias);
 		spin_unlock(&dentry->d_lock);

@@ -802,6 +804,7 @@ void d_instantiate(struct dentry *entry,
 	if (inode)
 		list_add(&entry->d_alias, &inode->i_dentry);
 	entry->d_inode = inode;
+	audit_attach_watch(entry, 0);
 	spin_unlock(&dcache_lock);
 	security_d_instantiate(entry, inode);
 }

@@ -978,6 +981,7 @@ struct dentry *d_splice_alias(struct ino
 		new = __d_find_alias(inode, 1);
 		if (new) {
 			BUG_ON(!(new->d_flags & DCACHE_DISCONNECTED));
+			audit_attach_watch(new, 0);
 			spin_unlock(&dcache_lock);
 			security_d_instantiate(new, inode);
 			d_rehash(dentry);

@@ -987,6 +991,7 @@ struct dentry *d_splice_alias(struct ino
 			/* d_instantiate takes dcache_lock, so we do it by hand */
 			list_add(&dentry->d_alias, &inode->i_dentry);
 			dentry->d_inode = inode;
+			audit_attach_watch(dentry, 0);
 			spin_unlock(&dcache_lock);
 			security_d_instantiate(dentry, inode);
 			d_rehash(dentry);

@@ -1090,6 +1095,7 @@ struct dentry * __d_lookup(struct dentry
 		if (!d_unhashed(dentry)) {
 			atomic_inc(&dentry->d_count);
 			found = dentry;
+			audit_attach_watch(found, 0);
 		}
 		spin_unlock(&dentry->d_lock);
 		break;

@@ -1299,6 +1305,8 @@ void d_move(struct dentry * dentry, stru
 		spin_lock(&target->d_lock);
 	}
 
+	audit_attach_watch(dentry, 1);
+
 	/* Move the dentry to the target hash queue, if on different bucket */
 	if (dentry->d_flags & DCACHE_UNHASHED)
 		goto already_unhashed;

@@ -1332,6 +1340,7 @@ already_unhashed:
 		list_add(&target->d_child, &target->d_parent->d_subdirs);
 	}
 
+	audit_attach_watch(dentry, 0);
 	list_add(&dentry->d_child, &dentry->d_parent->d_subdirs);
 	spin_unlock(&target->d_lock);
 	spin_unlock(&dentry->d_lock);
fs/inode.c +5 -1
diff -Nurp linux-2.6.12-rc2-mm1~orig/fs/inode.c linux-2.6.12-rc2-mm1~audit/fs/inode.c
@@ -22,6 +22,7 @@
 #include <linux/cdev.h>
 #include <linux/bootmem.h>
 #include <linux/inotify.h>
+#include <linux/audit.h>
 
 /*
  * This is needed for the following functions:

@@ -140,9 +141,11 @@ static struct inode *alloc_inode(struct 
 		inode->i_bdev = NULL;
 		inode->i_cdev = NULL;
 		inode->i_rdev = 0;
+		inode->i_audit = NULL;
 		inode->i_security = NULL;
 		inode->dirtied_when = 0;
-		if (security_inode_alloc(inode)) {
+		if (audit_inode_alloc(inode) || security_inode_alloc(inode)) {
+			audit_inode_free(inode);
 			if (inode->i_sb->s_op->destroy_inode)
 				inode->i_sb->s_op->destroy_inode(inode);
 			else

@@ -180,6 +183,7 @@ void destroy_inode(struct inode *inode) 
 {
 	if (inode_has_buffers(inode))
 		BUG();
+	audit_inode_free(inode);
 	security_inode_free(inode);
 	if (inode->i_sb->s_op->destroy_inode)
 		inode->i_sb->s_op->destroy_inode(inode);
fs/namei.c +13 -0
diff -Nurp linux-2.6.12-rc2-mm1~orig/fs/namei.c linux-2.6.12-rc2-mm1~audit/fs/namei.c
@@ -225,6 +225,8 @@ int permission(struct inode *inode, int 
 {
 	int retval, submask;
 
+	audit_notify_watch(inode, mask);
+
 	if (mask & MAY_WRITE) {
 		umode_t mode = inode->i_mode;
 

@@ -358,6 +360,8 @@ static inline int exec_permission_lite(s
 	if (inode->i_op && inode->i_op->permission)
 		return -EAGAIN;
 
+	audit_notify_watch(inode, MAY_EXEC);
+
 	if (current->fsuid == inode->i_uid)
 		mode >>= 6;
 	else if (in_group_p(inode->i_gid))

@@ -1172,6 +1176,8 @@ static inline int may_delete(struct inod
 
 	BUG_ON(victim->d_parent->d_inode != dir);
 
+	audit_notify_watch(victim->d_inode, MAY_WRITE);
+
 	error = permission(dir,MAY_WRITE | MAY_EXEC, NULL);
 	if (error)
 		return error;

@@ -1296,6 +1302,7 @@ int vfs_create(struct inode *dir, struct
 	DQUOT_INIT(dir);
 	error = dir->i_op->create(dir, dentry, mode, nd);
 	if (!error) {
+		audit_notify_watch(dentry->d_inode, MAY_WRITE);
 		fsnotify_create(dir, dentry->d_name.name);
 		security_inode_post_create(dir, dentry, mode);
 	}

@@ -1601,6 +1608,7 @@ int vfs_mknod(struct inode *dir, struct 
 	DQUOT_INIT(dir);
 	error = dir->i_op->mknod(dir, dentry, mode, dev);
 	if (!error) {
+		audit_notify_watch(dentry->d_inode, MAY_WRITE);
 		fsnotify_create(dir, dentry->d_name.name);
 		security_inode_post_mknod(dir, dentry, mode, dev);
 	}

@@ -1674,6 +1682,7 @@ int vfs_mkdir(struct inode *dir, struct 
 	DQUOT_INIT(dir);
 	error = dir->i_op->mkdir(dir, dentry, mode);
 	if (!error) {
+		audit_notify_watch(dentry->d_inode, MAY_WRITE);
 		fsnotify_mkdir(dir, dentry->d_name.name);
 		security_inode_post_mkdir(dir,dentry, mode);
 	}

@@ -1915,6 +1924,7 @@ int vfs_symlink(struct inode *dir, struc
 	DQUOT_INIT(dir);
 	error = dir->i_op->symlink(dir, dentry, oldname);
 	if (!error) {
+		audit_notify_watch(dentry->d_inode, MAY_WRITE);
 		fsnotify_create(dir, dentry->d_name.name);
 		security_inode_post_symlink(dir, dentry, oldname);
 	}

@@ -1988,6 +1998,7 @@ int vfs_link(struct dentry *old_dentry, 
 	error = dir->i_op->link(old_dentry, dir, new_dentry);
 	up(&old_dentry->d_inode->i_sem);
 	if (!error) {
+		audit_notify_watch(new_dentry->d_inode, MAY_WRITE);
 		fsnotify_create(dir, new_dentry->d_name.name);
 		security_inode_post_link(old_dentry, dir, new_dentry);
 	}

@@ -2111,6 +2122,7 @@ int vfs_rename_dir(struct inode *old_dir
 	}
 	if (!error) {
 		d_move(old_dentry,new_dentry);
+		audit_notify_watch(old_dentry->d_inode, MAY_WRITE);
 		security_inode_post_rename(old_dir, old_dentry,
 					   new_dir, new_dentry);
 	}

@@ -2139,6 +2151,7 @@ int vfs_rename_other(struct inode *old_d
 		/* The following d_move() should become unconditional */
 		if (!(old_dir->i_sb->s_type->fs_flags & FS_ODD_RENAME))
 			d_move(old_dentry, new_dentry);
+		audit_notify_watch(old_dentry->d_inode, MAY_WRITE);
 		security_inode_post_rename(old_dir, old_dentry, new_dir, new_dentry);
 	}
 	if (target)
include/linux/audit.h +77 -8

121 lines not on this page · raw message

include/linux/fs.h +2 -0
diff -Nurp linux-2.6.12-rc2-mm1~orig/include/linux/fs.h linux-2.6.12-rc2-mm1~audit/include/linux/fs.h
@@ -226,6 +226,7 @@ struct poll_table_struct;
 struct kstatfs;
 struct vm_area_struct;
 struct vfsmount;
+struct audit_data;
 
 /* Used to be a macro which just called the function, now just a function */
 extern void update_atime (struct inode *);

@@ -477,6 +478,7 @@ struct inode {
 	struct semaphore	inotify_sem;	/* protects the watches list */
 #endif
 
+	struct audit_data	*i_audit;
 	unsigned long		i_state;
 	unsigned long		dirtied_when;	/* jiffies of first dirtying */
init/Kconfig +10 -0
diff -Nurp linux-2.6.12-rc2-mm1~orig/init/Kconfig linux-2.6.12-rc2-mm1~audit/init/Kconfig
@@ -198,6 +198,16 @@ config AUDITSYSCALL
 	  can be used independently or with another kernel subsystem,
 	  such as SELinux.
 
+config AUDITFILESYSTEM
+	bool "Enable file system auditing support"
+	depends on AUDITSYSCALL
+	default n
+	help
+	  Enable file system auditing for regular files and directories.
+	  When a targeted file or directory is accessed, an audit record
+	  is generated describing the inode accessed, how it was accessed,
+	  and by whom (ie: pid and system call).
+
 config HOTPLUG
 	bool "Support for hot-pluggable devices" if !ARCH_S390
 	default ARCH_S390
kernel/Makefile +1 -0
diff -Nurp linux-2.6.12-rc2-mm1~orig/kernel/Makefile linux-2.6.12-rc2-mm1~audit/kernel/Makefile
@@ -25,6 +25,7 @@ obj-$(CONFIG_IKCONFIG_PROC) += configs.o
 obj-$(CONFIG_STOP_MACHINE) += stop_machine.o
 obj-$(CONFIG_AUDIT) += audit.o
 obj-$(CONFIG_AUDITSYSCALL) += auditsc.o
+obj-$(CONFIG_AUDITFILESYSTEM) += auditfs.o
 obj-$(CONFIG_KPROBES) += kprobes.o
 obj-$(CONFIG_SYSFS) += ksysfs.o
 obj-$(CONFIG_DETECT_SOFTLOCKUP) += softlockup.o
kernel/audit.c +14 -0
diff -Nurp linux-2.6.12-rc2-mm1~orig/kernel/audit.c linux-2.6.12-rc2-mm1~audit/kernel/audit.c
@@ -322,6 +322,8 @@ static int audit_netlink_ok(kernel_cap_t
 	case AUDIT_SET:
 	case AUDIT_ADD:
 	case AUDIT_DEL:
+	case AUDIT_WATCH_INS:
+	case AUDIT_WATCH_REM:
 		if (!cap_raised(eff_cap, CAP_AUDIT_CONTROL))
 			err = -EPERM;
 		break;

@@ -410,12 +412,23 @@ static int audit_receive_msg(struct sk_b
 		/* fallthrough */
 	case AUDIT_LIST:
 #ifdef CONFIG_AUDITSYSCALL
+		err = audit_list_watches(NETLINK_CB(skb).pid, seq);
+		if (err < 0)
+			break;
 		err = audit_receive_filter(nlh->nlmsg_type, NETLINK_CB(skb).pid,
 					   uid, seq, data);
 #else
 		err = -EOPNOTSUPP;
 #endif
 		break;
+	case AUDIT_WATCH_INS:
+	case AUDIT_WATCH_REM:
+		if (nlh->nlmsg_len < sizeof(struct audit_transport))
+			return -EINVAL;
+		err = audit_receive_watch(nlh->nlmsg_type,
+					  NETLINK_CB(skb).pid,
+					  uid, seq, data);
+		break;
 	default:
 		err = -EINVAL;
 		break;

@@ -560,6 +573,7 @@ static int __init audit_init(void)
 
 	audit_initialized = 1;
 	audit_enabled = audit_default;
+	audit_filesystem_init();
 	audit_log(NULL, "initialized");
 	return 0;
 }
kernel/auditfs.c +542 -0

542 lines not on this page · raw message

kernel/auditsc.c +78 -0

99 lines not on this page · raw message

security/selinux/nlmsgtab.c +3 -0
diff -Nurp linux-2.6.12-rc2-mm1~orig/security/selinux/nlmsgtab.c linux-2.6.12-rc2-mm1~audit/security/selinux/nlmsgtab.c
@@ -98,6 +98,9 @@ static struct nlmsg_perm nlmsg_audit_per
 	{ AUDIT_DEL,		NETLINK_AUDIT_SOCKET__NLMSG_WRITE },
 	{ AUDIT_USER,		NETLINK_AUDIT_SOCKET__NLMSG_WRITE },
 	{ AUDIT_LOGIN,		NETLINK_AUDIT_SOCKET__NLMSG_WRITE },
+	{ AUDIT_WATCH_INS,	NETLINK_AUDIT_SOCKET__NLMSG_WRITE },
+	{ AUDIT_WATCH_REM,	NETLINK_AUDIT_SOCKET__NLMSG_WRITE },
+	{ AUDIT_WATCH_LIST,	NETLINK_AUDIT_SOCKET__NLMSG_WRITE },
 };
#1 lore raw
Quoted 4 lines · Timothy R. Chavez

Hello,

The audit subsystem is currently incapable of auditing a file system object based on its location and name.

Which doesn't make sense in our world of per-process namespaces. Unless this is not just a silly checkbox item for government certification please add it only to your favourite vendor tree.

#2 lore raw
Quoted 9 lines · Christoph Hellwig
Quoted 4 lines · Timothy R. Chavez

Hello,

The audit subsystem is currently incapable of auditing a file system object based on its location and name.

Which doesn't make sense in our world of per-process namespaces. Unless this is not just a silly checkbox item for government certification please add it only to your favourite vendor tree.

Hi Christoph,

How would you suggest meeting the higher level requirement of ensuring that auditing of accesses to a file is preserved across transactions that involve re-creating the file on each transaction, as with /etc/shadow transactions? The existing (device,inode)-based syscall audit filters aren't sufficient for that purpose, as the inode changes on each transaction.

For the similar requirement that we preserve the security label on /etc/shadow for SELinux across transactions, we use access control to ensure that only authorized programs can modify /etc/shadow, and extend those programs to preserve the security attributes from the existing /etc/shadow inode when creating the new file for the transaction. But if people want to be able to use the audit support apart from mandatory access controls / SELinux, then they can't enforce such restrictions, so they need a different mechanism for preservation of auditing.

Signature · 2 lines
Stephen Smalley <[email protected]>
National Security Agency
author
#3 lore raw
In reply to Christoph Hellwig
Quoted 6 lines · Christoph Hellwig
Quoted 4 lines · Timothy R. Chavez

Hello,

The audit subsystem is currently incapable of auditing a file system object based on its location and name.

Hello Christoph,

I apologize for the delay in my response. Thank you for your response. I'll try to be succinct.

Quoted 1 line

Which doesn't make sense in our world of per-process namespaces.

The audit subsystem is responsible for generating accounts of activity in the kernel for the purposes of examination and verification. Thus, in terms of audit, how we capture becomes less important than what we capture, as long as it remains consistent, right? Given A, B follows.

The syscall filtering on (inode,device)-pairs is not enough to construct complete accounts of activity across transactions in which an underlying inode is subject to change. Once that inode changes, we've lost audit. So, we've introduced an abstract methodology by which the administrator is able to "watch" by location and name. This allows us to ultimately examine and verify actions taken against, say, "/etc/shadow", and not simply the inode currently associated with it.

Quoted 2 lines

Unless this is not just a silly checkbox item for government certification please add it only to your favourite vendor tree.

I think that certs are a perfectly legitimate use-case for a kernel's _audit_ subsystem.

I'd like to maintain a relative focus here on linux-fsdevel. When I RFC on LKML, we can argue the virtues of audit and certifications there.

-tim

#4 lore raw
Quoted 4 lines · Timothy R. Chavez
Quoted 3 lines · Christoph Hellwig
Quoted 2 lines · Timothy R. Chavez

Hello,

Quoted 10 lines
Quoted 3 lines
Quoted 2 lines

The audit subsystem is currently incapable of auditing a file system object based on its location and name.

Hello Christoph,

I apologize for the delay in my response. Thank you for your response. I'll try to be succinct.

Quoted 1 line

Which doesn't make sense in our world of per-process namespaces.

AFAICT, it does so long as we are trying to audit non-admin users. I may be in a different namespace from the admin, but so long only the admin can mount over /etc, the auditing should still happen. An admin can mess us up by mounting /root/dummy/etc on top of /etc, but we expect the admin to know what he is doing.

Note that this patch is not auditing based on full pathname. It audits based on the parent directory and filename. So even if /etc is bind mounted onto /mnt/root/etc, accessing /mnt/root/etc/passwd will still trigger an audit entry.

Am I missing something? How do namespaces mess this up?

thanks,
-serge

Signature · 1 line
Serge Hallyn <[email protected]>