(#2) 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 second (#2) RFC on linux-fsdevel.

fs/dcache.c +1 -0

@@ -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);

kernel/auditfs.c +8 -0

+static inline int audit_insert_watch(char *path, char *filterkey, __u32 perms)
+{
+	int ret;
+	struct nameidata nd;
+
+	ret = path_lookup(path, LOOKUP_PARENT|LOOKUP_FOLLOW, &nd);
+	if (ret < 0)
+		goto audit_insert_watch_exit;

kernel/auditfs.c +20 -0

+void audit_attach_watch(struct dentry *dentry, int remove)
+{
+	struct audit_wentry *wentry;
+	struct audit_data *data, *parent;
+
+	if (!dentry || !dentry->d_inode)
+		return;
+
+	if (!dentry->d_parent || !dentry->d_parent->d_inode)
+		return;
+
+	data = dentry->d_inode->i_audit;
+	parent = dentry->d_parent->d_inode->i_audit;
+
+	wentry = audit_wentry_fetch_lock(dentry->d_name.name, parent);
+
+	write_lock(&data->lock);
+	/* FIXME: long watchlist == too much spinning? */
+	if (remove) {
+		audit_drain_watchlist(data);

kernel/auditfs.c +9 -0

+int audit_list_watches(int pid, int seq)
+{
+	int ret = 0;
+	struct audit_wentry *wentry;
+	struct hlist_node *pos;
+
+	spin_lock(&audit_master_watchlist_lock);
+	hlist_for_each_entry(wentry, pos, &audit_master_watchlist, w_master) {
+		ret = audit_send_watch(pid, seq, wentry->w_watch);

kernel/auditfs.c +91 -0

+int audit_receive_watch(int type, int pid, int uid, int seq,
+			struct audit_transport *req)
+{
+	int ret = 0;
+	char *path = NULL;
+	char *filterkey = NULL;
+
+	ret = -ENOMEM;
+	path = kmalloc(req->pathlen, GFP_KERNEL);
+	if (!path)
+		goto audit_receive_watch_exit;
+	strncpy(path, req->buf, req->pathlen);
+
+	if (req->fklen) {
+		filterkey = kmalloc(req->fklen, GFP_KERNEL);
+		if (!filterkey)
+			goto audit_receive_watch_exit;
+	}
+	strncpy(filterkey, req->buf+req->pathlen, req->fklen);
+
+	ret = -EINVAL;
+	if (!path || strlen(path) + 1 > PATH_MAX)
+		goto audit_receive_watch_exit;
+
+	/* Includes terminating '\0' */
+	if (req->fklen > AUDIT_FILTERKEY_MAX)
+		goto audit_receive_watch_exit;
+
+	if (req->perms > (MAY_READ|MAY_WRITE|MAY_EXEC|MAY_APPEND))
+		goto audit_receive_watch_exit;
+
+	switch (type) {
+	case AUDIT_WATCH_INS:
+		ret = audit_insert_watch(path, filterkey, req->perms);
+		break;
+	case AUDIT_WATCH_REM:
+		ret = audit_remove_watch(path);
+		break;
+	default:
+		ret = -EINVAL;
+	}
+
+audit_receive_watch_exit:
+	kfree(path);
+	kfree(filterkey);
+	return ret;
+}
+
+int audit_inode_alloc(struct inode *inode)
+{
+	if (inode) {
+		inode->i_audit = audit_data_alloc();
+		if (!inode->i_audit)
+			return ENOMEM;
+	}
+
+	return 0;
+}
+
+void audit_inode_free(struct inode *inode)
+{
+	if (inode)
+		audit_data_free(inode->i_audit);
+}
+
+int audit_filesystem_init()
+{
+	int ret = 0;
+
+	audit_watch_cache =
+	    kmem_cache_create("audit_watch_cache",
+			      sizeof(struct audit_watch), 0, 0, NULL, NULL);
+	if (!audit_watch_cache)
+		goto audit_filesystem_init_fail;
+
+	audit_wentry_cache =
+	    kmem_cache_create("audit_wentry_cache",
+			      sizeof(struct audit_wentry), 0, 0, NULL, NULL);
+	if (!audit_wentry_cache)
+		goto audit_filesystem_init_fail;
+
+	goto audit_filesystem_init_exit;
+
+audit_filesystem_init_fail:
+	ret = -ENOMEM;
+	kmem_cache_destroy(audit_watch_cache);
+	kmem_cache_destroy(audit_wentry_cache);
+audit_filesystem_init_exit:
+	return ret;
+
+}

diff -Nurp linux-2.6.12-rc2-mm1~orig/kernel/auditsc.c linux-2.6.12-rc2-mm1~audit/kernel/auditsc.c

kernel/auditsc.c +78 -0

@@ -102,6 +102,7 @@ struct audit_aux_data {
 };
 
 #define AUDIT_AUX_IPCPERM	0
+#define AUDIT_AUX_WATCH		1
 
 struct audit_aux_data_ipcctl {
 	struct audit_aux_data	d;

@@ -112,6 +113,16 @@ struct audit_aux_data_ipcctl {
 	mode_t			mode;
 };
 
+struct audit_aux_data_watched {
+	struct audit_aux_data	link;
+	struct audit_wentry 	*wentry;
+	unsigned long		ino;
+	int			mask;
+	uid_t			uid;
+	gid_t			gid;
+	dev_t			dev;
+	dev_t			rdev;
+};
 
 /* The per-task audit context. */
 struct audit_context {

@@ -665,6 +676,24 @@ static void audit_log_exit(struct audit_
 			audit_log_format(ab, 
 					 " qbytes=%lx uid=%d gid=%d mode=%x",
 					 axi->qbytes, axi->uid, axi->gid, axi->mode);
+			break;
+			}
+
+		case AUDIT_AUX_WATCH: {
+			struct audit_aux_data_watched *axi = (void *)aux;
+			audit_log_format(ab, " name=");
+			audit_log_untrustedstring(ab, axi->wentry->w_watch->name);
+			audit_log_format(ab,
+				" filterkey=%s perm=%u perm_mask=%d"
+				" inode=%lu inode_uid=%d inode_gid=%d"
+				" inode_dev=%02x:%02x inode_rdev=%02x:%02x",
+				axi->wentry->w_watch->filterkey,
+				axi->wentry->w_watch->perms,
+				axi->mask, axi->ino, axi->uid, axi->gid,
+				MAJOR(axi->dev), MINOR(axi->dev),
+				MAJOR(axi->rdev), MINOR(axi->rdev));
+				audit_wentry_put(axi->wentry);
+			break;
 			}
 		}
 		audit_log_end(ab);

@@ -1024,3 +1053,52 @@ int audit_ipc_perms(unsigned long qbytes
 	context->aux = (void *)ax;
 	return 0;
 }
+
+int audit_notify_watch(struct inode *inode, int mask)
+{
+	int ret = 0;
+	struct audit_context *context = current->audit_context;
+	struct audit_aux_data_watched *ax;
+	struct audit_wentry *wentry = NULL;
+
+	if (likely(!context))
+		goto audit_notify_watch_fail;
+
+	if (!inode)
+		goto audit_notify_watch_fail;
+
+	wentry = audit_wentry_get(inode->i_audit->wentry);
+	if (!wentry)
+		goto audit_notify_watch_fail;
+
+	if (mask && (wentry->w_watch->perms && !(wentry->w_watch->perms&mask)))
+		goto audit_notify_watch_fail;
+
+	ret = -ENOMEM;
+	ax = kmalloc(sizeof(*ax), GFP_KERNEL);
+	if (!ax)
+		goto audit_notify_watch_fail;
+
+	ret = 0;
+	if (context->in_syscall && !context->auditable)
+		context->auditable = 1;
+
+	ax->wentry = wentry;
+	ax->mask = mask;
+	ax->ino = inode->i_ino;
+	ax->uid = inode->i_uid;
+	ax->gid = inode->i_gid;
+	ax->dev = inode->i_sb->s_dev;
+	ax->rdev = inode->i_rdev;
+
+	ax->link.type = AUDIT_AUX_WATCH;
+	ax->link.next = context->aux;
+	context->aux = (void *)ax;
+
+	goto audit_notify_watch_exit;
+
+audit_notify_watch_fail:
+	audit_wentry_put(wentry);
+audit_notify_watch_exit:
+	return ret;
+}

diff -Nurp linux-2.6.12-rc2-mm1~orig/security/selinux/nlmsgtab.c linux-2.6.12-rc2-mm1~audit/security/selinux/nlmsgtab.c

security/selinux/nlmsgtab.c +3 -0

@@ -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 },
 };
 

Quoted 12 lines · Timothy R. Chavez

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.

Quoted 2 lines

This is the second (#2) RFC on linux-fsdevel.

Please see the comments interspread below.

Thanks
Maneesh

PS: Sorry to miss earlier versions.

[..]

Quoted 13 lines

fs/dcache.c +2 -0

@@ -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);

fs/dcache.c struct dentry * __d_lookup(struct dentry · line 1101

Quoted 3 lines

fs/dcache.c +0 -0

 		}
 		spin_unlock(&dentry->d_lock);
 		break;

Are you sure about hooking __d_lookup()? I am not getting why would one want to watch every dcache lookup. IIUC, you probaly want to watch every fs object entering the dcache, so IMO, the right place is real_lookup() and not the __d_lookup() which is performance sensitive code.

[..]

kernel/auditfs.c line 96

Quoted 9 lines

kernel/auditfs.c +9 -0

+
+static inline void audit_watch_free(struct audit_watch *watch)
+{
+	if (watch) {
+		kfree(watch->name);
+		kfree(watch->filterkey);
+		kmem_cache_free(audit_watch_cache, watch);
+	}
+}

I think you forgot, kfree(watch->path) here.

[..]

kernel/auditfs.c line 287

Quoted 6 lines

kernel/auditfs.c +6 -0

+static inline int audit_insert_watch(char *path, char *filterkey, __u32 perms)
+{
+	int ret;
+	struct nameidata nd;
+
+	ret = path_lookup(path, LOOKUP_PARENT|LOOKUP_FOLLOW, &nd);

Quoted 6 lines

kernel/auditfs.c +6 -0

+	if (ret < 0)
+		goto audit_insert_watch_exit;
+
+	ret = -EPERM;
+	if (nd.last_type != LAST_NORM || !nd.last.name)
+		goto audit_insert_watch_release;

kernel/auditfs.c line 299

Quoted 6 lines

kernel/auditfs.c +6 -0

+
+	ret = audit_create_wentry(path, nd.last.name, filterkey, perms,
+				  nd.dentry->d_inode->i_sb->s_dev,
+				  nd.dentry->d_inode->i_audit);
+	/* __d_lookup will attach the audit data, if nd.last.name exists. */
+	d_lookup(nd.dentry, &nd.last);

Do you mean, if nd.last.name exists in dcache() or exists in the filesystem?

[..]

Quoted 24 lines

kernel/auditfs.c +24 -0

+void audit_attach_watch(struct dentry *dentry, int remove)
+{
+	struct audit_wentry *wentry;
+	struct audit_data *data, *parent;
+
+	if (!dentry || !dentry->d_inode)
+		return;
+
+	if (!dentry->d_parent || !dentry->d_parent->d_inode)
+		return;
+
+	data = dentry->d_inode->i_audit;
+	parent = dentry->d_parent->d_inode->i_audit;
+
+	wentry = audit_wentry_fetch_lock(dentry->d_name.name, parent);
+
+	write_lock(&data->lock);
+	/* FIXME: long watchlist == too much spinning? */
+	if (remove) {
+		audit_drain_watchlist(data);
+		if (wentry && data->wentry) {
+			if (!strcmp(wentry->w_watch->name,
+				    data->wentry->w_watch->name)) {
+				audit_wentry_put(data->wentry);

kernel/auditfs.c line 395

Quoted 6 lines

kernel/auditfs.c +6 -0

+				dentry->d_inode->i_audit->wentry = NULL;
+			}
+		}
+	} else if (!data->wentry || hlist_unhashed(&data->wentry->w_node)) {
+		audit_wentry_put(data->wentry);
+		dentry->d_inode->i_audit->wentry = audit_wentry_get(wentry);

why not just, data->wentry = audit_wentry_get(wentry);

[..]

Quoted 9 lines

kernel/auditfs.c +9 -0

+int audit_list_watches(int pid, int seq)
+{
+	int ret = 0;
+	struct audit_wentry *wentry;
+	struct hlist_node *pos;
+
+	spin_lock(&audit_master_watchlist_lock);
+	hlist_for_each_entry(wentry, pos, &audit_master_watchlist, w_master) {
+		ret = audit_send_watch(pid, seq, wentry->w_watch);

kernel/auditfs.c line 469

Quoted 10 lines

kernel/auditfs.c +10 -0

+		if (ret < 0)
+			break;
+	}
+	spin_unlock(&audit_master_watchlist_lock);
+
+	audit_send_reply(pid, seq, AUDIT_WATCH_LIST, 1, 1, NULL, 0);
+
+	return ret;
+}
+

Looks like there is lock heirarachy issue here between audit_master_watchlist_lock and data->lock().

audit_list_watches()---> audit_master_watchlist_lock
audit_send_watch()

    audit_wentry_fetch_lock()---> data->lock

audit_insert_watch()
audit_create_wentry()---> data->lock

    audit_master_insert()---> audit_master_watchlist_lock

OR

audit_attach_watch()---> data->lock
audit_drain_watchlist()

    audit_destroy_watch()
      audit_master_remove()---> audit_master_watchlist_lock

Quoted 209 lines

kernel/auditfs.c +91 -0

+int audit_receive_watch(int type, int pid, int uid, int seq,
+			struct audit_transport *req)
+{
+	int ret = 0;
+	char *path = NULL;
+	char *filterkey = NULL;
+
+	ret = -ENOMEM;
+	path = kmalloc(req->pathlen, GFP_KERNEL);
+	if (!path)
+		goto audit_receive_watch_exit;
+	strncpy(path, req->buf, req->pathlen);
+
+	if (req->fklen) {
+		filterkey = kmalloc(req->fklen, GFP_KERNEL);
+		if (!filterkey)
+			goto audit_receive_watch_exit;
+	}
+	strncpy(filterkey, req->buf+req->pathlen, req->fklen);
+
+	ret = -EINVAL;
+	if (!path || strlen(path) + 1 > PATH_MAX)
+		goto audit_receive_watch_exit;
+
+	/* Includes terminating '\0' */
+	if (req->fklen > AUDIT_FILTERKEY_MAX)
+		goto audit_receive_watch_exit;
+
+	if (req->perms > (MAY_READ|MAY_WRITE|MAY_EXEC|MAY_APPEND))
+		goto audit_receive_watch_exit;
+
+	switch (type) {
+	case AUDIT_WATCH_INS:
+		ret = audit_insert_watch(path, filterkey, req->perms);
+		break;
+	case AUDIT_WATCH_REM:
+		ret = audit_remove_watch(path);
+		break;
+	default:
+		ret = -EINVAL;
+	}
+
+audit_receive_watch_exit:
+	kfree(path);
+	kfree(filterkey);
+	return ret;
+}
+
+int audit_inode_alloc(struct inode *inode)
+{
+	if (inode) {
+		inode->i_audit = audit_data_alloc();
+		if (!inode->i_audit)
+			return ENOMEM;
+	}
+
+	return 0;
+}
+
+void audit_inode_free(struct inode *inode)
+{
+	if (inode)
+		audit_data_free(inode->i_audit);
+}
+
+int audit_filesystem_init()
+{
+	int ret = 0;
+
+	audit_watch_cache =
+	    kmem_cache_create("audit_watch_cache",
+			      sizeof(struct audit_watch), 0, 0, NULL, NULL);
+	if (!audit_watch_cache)
+		goto audit_filesystem_init_fail;
+
+	audit_wentry_cache =
+	    kmem_cache_create("audit_wentry_cache",
+			      sizeof(struct audit_wentry), 0, 0, NULL, NULL);
+	if (!audit_wentry_cache)
+		goto audit_filesystem_init_fail;
+
+	goto audit_filesystem_init_exit;
+
+audit_filesystem_init_fail:
+	ret = -ENOMEM;
+	kmem_cache_destroy(audit_watch_cache);
+	kmem_cache_destroy(audit_wentry_cache);
+audit_filesystem_init_exit:
+	return ret;
+
+}

diff -Nurp linux-2.6.12-rc2-mm1~orig/kernel/auditsc.c linux-2.6.12-rc2-mm1~audit/kernel/auditsc.c

kernel/auditsc.c +78 -0

@@ -102,6 +102,7 @@ struct audit_aux_data {
 };
 
 #define AUDIT_AUX_IPCPERM	0
+#define AUDIT_AUX_WATCH		1
 
 struct audit_aux_data_ipcctl {
 	struct audit_aux_data	d;

@@ -112,6 +113,16 @@ struct audit_aux_data_ipcctl {
 	mode_t			mode;
 };
 
+struct audit_aux_data_watched {
+	struct audit_aux_data	link;
+	struct audit_wentry 	*wentry;
+	unsigned long		ino;
+	int			mask;
+	uid_t			uid;
+	gid_t			gid;
+	dev_t			dev;
+	dev_t			rdev;
+};
 
 /* The per-task audit context. */
 struct audit_context {

@@ -665,6 +676,24 @@ static void audit_log_exit(struct audit_
 			audit_log_format(ab, 
 					 " qbytes=%lx uid=%d gid=%d mode=%x",
 					 axi->qbytes, axi->uid, axi->gid, axi->mode);
+			break;
+			}
+
+		case AUDIT_AUX_WATCH: {
+			struct audit_aux_data_watched *axi = (void *)aux;
+			audit_log_format(ab, " name=");
+			audit_log_untrustedstring(ab, axi->wentry->w_watch->name);
+			audit_log_format(ab,
+				" filterkey=%s perm=%u perm_mask=%d"
+				" inode=%lu inode_uid=%d inode_gid=%d"
+				" inode_dev=%02x:%02x inode_rdev=%02x:%02x",
+				axi->wentry->w_watch->filterkey,
+				axi->wentry->w_watch->perms,
+				axi->mask, axi->ino, axi->uid, axi->gid,
+				MAJOR(axi->dev), MINOR(axi->dev),
+				MAJOR(axi->rdev), MINOR(axi->rdev));
+				audit_wentry_put(axi->wentry);
+			break;
 			}
 		}
 		audit_log_end(ab);

@@ -1024,3 +1053,52 @@ int audit_ipc_perms(unsigned long qbytes
 	context->aux = (void *)ax;
 	return 0;
 }
+
+int audit_notify_watch(struct inode *inode, int mask)
+{
+	int ret = 0;
+	struct audit_context *context = current->audit_context;
+	struct audit_aux_data_watched *ax;
+	struct audit_wentry *wentry = NULL;
+
+	if (likely(!context))
+		goto audit_notify_watch_fail;
+
+	if (!inode)
+		goto audit_notify_watch_fail;
+
+	wentry = audit_wentry_get(inode->i_audit->wentry);
+	if (!wentry)
+		goto audit_notify_watch_fail;
+
+	if (mask && (wentry->w_watch->perms && !(wentry->w_watch->perms&mask)))
+		goto audit_notify_watch_fail;
+
+	ret = -ENOMEM;
+	ax = kmalloc(sizeof(*ax), GFP_KERNEL);
+	if (!ax)
+		goto audit_notify_watch_fail;
+
+	ret = 0;
+	if (context->in_syscall && !context->auditable)
+		context->auditable = 1;
+
+	ax->wentry = wentry;
+	ax->mask = mask;
+	ax->ino = inode->i_ino;
+	ax->uid = inode->i_uid;
+	ax->gid = inode->i_gid;
+	ax->dev = inode->i_sb->s_dev;
+	ax->rdev = inode->i_rdev;
+
+	ax->link.type = AUDIT_AUX_WATCH;
+	ax->link.next = context->aux;
+	context->aux = (void *)ax;
+
+	goto audit_notify_watch_exit;
+
+audit_notify_watch_fail:
+	audit_wentry_put(wentry);
+audit_notify_watch_exit:
+	return ret;
+}

diff -Nurp linux-2.6.12-rc2-mm1~orig/security/selinux/nlmsgtab.c linux-2.6.12-rc2-mm1~audit/security/selinux/nlmsgtab.c

security/selinux/nlmsgtab.c +3 -0

@@ -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 },
 };