audit: file system auditing based on location and name
Subsystems (3)
PATCH 1/1 audit: file system auditing based on location and name
Hello,
This patch augments the audit subsystem and VFS to support file system auditing in which an object is audited based on its location and name. Depending on the administrator's requirement, this feature can be used standalone or in conjunction with the (device,inode)-based filters. 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 solely on the (device, inode)-based filters to ensure persistence of auditing across transactions.
This work is being done to make the Linux audit subsystem compliant with Common Criteria's Controlled Access Protection Profile (CAPP) specification. CAPP requires that the administrator is able to audit a file's content in two fundamental ways:
1) Where content confidentiality is important.
This includes security-relevant databases like /etc/passwd and the audit logs of which the contents are security sensitive. (device,inode)-based system-call filtering is sufficient to cover this requirement and is already available to the administrator.
2) Where content integrity is important.
This includes configuration files that affect the operation of trusted programs like /etc/shadow. Content integrity ensures audit persists across transactions where the underlying inode may change and content confidentiality is important. This is currently not doable without this patch.
To implement this feature we rely on the concepts of a "watch" and "watch list". Directories hold lists of "watches" (ie: "watch lists") that describe auditable file names one level beneath them. If a file holds a pointer into a "watch list" it is auditable. When accessed by a system call, information about the inode and its "watches" is added to the audit context of the current task (an inode may have multiple "watches" if a hard link to a "watched" file is itself being "watched") which is sent to user space upon system call exit.
With the exception of explicit insertions and removals of file system "watches" by the administrator, adding, removing, and updating audit data is done using an update hook in key areas of fs/dcache.c. A second hook is placed in audit-relevant system calls at critical points to collect and relay information to the audit subsystem when auditable inodes are accessed in an interesting way.
Some notable implementation details are as follows:
* struct inode is _not_ extended to associate audit data with the inode. A hash table is used in which the inode is hashed to retrieve its audit data. We know if an inode has audit data if I_AUDIT has been turned on in inode->i_state.
* Inodes with audit data are implicitly pinned in memory when I_AUDIT is turned on in inode->i_state. This prevents an auditable incore inode from being pushed out of the icache, preserving auditability even under memory pressures.
The user interface provided in kernel/audit.c is extended to allow the administrator to add "watches", remove "watches", and list "watches".
We've submitted for inclusion to the -mm tree.
Thank you.
-tim
--- fs/attr.c | 3 fs/dcache.c | 9 fs/inode.c | 7 fs/namei.c | 13 fs/open.c | 6 include/linux/audit.h | 74 +++ include/linux/fs.h | 3 init/Kconfig | 10 kernel/Makefile | 1 kernel/audit.c | 23 + kernel/auditfs.c | 855 ++++++++++++++++++++++++++++++++++++++++++++ kernel/auditsc.c | 138 +++++++ security/selinux/nlmsgtab.c | 3 13 files changed, 1140 insertions(+), 5 deletions(-)
---
fs/attr.c +3 -0
@@ -15,6 +15,7 @@#include <linux/quotaops.h> #include <linux/security.h> #include <linux/time.h> +#include <linux/audit.h> /* Taken over from the old code... */
@@ -69,6 +70,8 @@ int inode_setattr(struct inode * inode,unsigned int ia_valid = attr->ia_valid;int error =0 ; +audit_notify_watch (inode,MAY_WRITE ); +if (ia_valid &ATTR_SIZE ) {if (attr->ia_size !=i_size_read (inode)) { error =vmtruncate (inode, attr->ia_size);
fs/dcache.c +9 -0
@@ -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_update_watch (dentry,1 ); dentry->d_inode =NULL ;list_del_init (&dentry->d_alias);spin_unlock (&dentry->d_lock);
@@ -966,6 +968,7 @@ void d_instantiate(struct dentry *entry,if (inode)list_add (&entry->d_alias, &inode->i_dentry); entry->d_inode = inode; +audit_update_watch (entry,0 );spin_unlock (&dcache_lock);security_d_instantiate (entry, inode); }
@@ -1142,6 +1145,7 @@ struct dentry *d_splice_alias(struct ino new =__d_find_alias (inode,1 );if (new) {BUG_ON (!(new->d_flags &DCACHE_DISCONNECTED )); +audit_update_watch (new,0 );spin_unlock (&dcache_lock);security_d_instantiate (new, inode);d_rehash (dentry);
@@ -1151,6 +1155,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_update_watch (dentry,0 );spin_unlock (&dcache_lock);security_d_instantiate (dentry, inode);d_rehash (dentry);
@@ -1254,6 +1259,7 @@ struct dentry * __d_lookup(struct dentryif (!d_unhashed (dentry)) {atomic_inc (&dentry->d_count); found = dentry; +audit_update_watch (dentry,0 ); }spin_unlock (&dentry->d_lock);break ;
@@ -1463,6 +1469,8 @@ void d_move(struct dentry * dentry, struspin_lock (&target->d_lock); } +audit_update_watch (dentry,1 ); +/* Move the dentry to the target hash queue, if on different bucket */ if (dentry->d_flags &DCACHE_UNHASHED ) goto already_unhashed;
@@ -1496,6 +1504,7 @@ already_unhashed:list_add (&target->d_child, &target->d_parent->d_subdirs); } +audit_update_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 -2
@@ -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:
@@ -173,6 +174,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);
@@ -262,7 +264,7 @@ void clear_inode(struct inode *inode)bd_forget (inode);if (inode->i_cdev)cd_forget (inode); - inode->i_state =I_CLEAR ; + inode->i_state =I_CLEAR | (inode->i_state &I_AUDIT ); }EXPORT_SYMBOL (clear_inode);
@@ -1051,7 +1053,7 @@ void generic_delete_inode(struct inode *hlist_del_init (&inode->i_hash);spin_unlock (&inode_lock);wake_up_inode (inode); -if (inode->i_state !=I_CLEAR ) +if ((inode->i_state & ~I_AUDIT ) !=I_CLEAR )BUG ();destroy_inode (inode); }
@@ -1364,6 +1366,7 @@ void __init inode_init(unsigned long mem 0, SLAB_RECLAIM_ACCOUNT|SLAB_PANIC, init_once, NULL);set_shrinker (DEFAULT_SEEKS , shrink_icache_memory); +audit_filesystem_init ();/* Hash may have been set up in inode_init_early */ if (!hashdist)return ;
fs/namei.c +13 -0
@@ -243,6 +243,8 @@ int permission(struct inode *inode, int } +audit_notify_watch (inode, mask); +/* Ordinary permission routines do not understand MAY_APPEND. */ submask = mask & ~MAY_APPEND ;if (inode->i_op && inode->i_op->permission)
@@ -358,6 +360,8 @@ static inline int exec_permission_lite(sif (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))
@@ -1188,6 +1192,8 @@ static inline int may_delete(struct inodBUG_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;
@@ -1312,6 +1318,7 @@ int vfs_create(struct inode *dir, structDQUOT_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); }
@@ -1637,6 +1644,7 @@ int vfs_mknod(struct inode *dir, structDQUOT_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); }
@@ -1710,6 +1718,7 @@ int vfs_mkdir(struct inode *dir, structDQUOT_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); }
@@ -1951,6 +1960,7 @@ int vfs_symlink(struct inode *dir, strucDQUOT_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); }
@@ -2024,6 +2034,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); }
@@ -2147,6 +2158,7 @@ static int vfs_rename_dir(struct inode * }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); }
@@ -2175,6 +2187,7 @@ static int vfs_rename_other(struct inode/* 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)
fs/open.c +6 -0
@@ -25,6 +25,7 @@#include <linux/pagemap.h> #include <linux/syscalls.h> #include <linux/rcupdate.h> +#include <linux/audit.h> #include <asm/unistd.h>
@@ -608,6 +609,8 @@ asmlinkage long sys_fchmod(unsigned int dentry = file->f_dentry; inode = dentry->d_inode; + +audit_notify_watch (inode,MAY_WRITE ); err = -EROFS ;if (IS_RDONLY (inode))
@@ -640,6 +643,8 @@ asmlinkage long sys_chmod(const char __uif (error) goto out; inode = nd.dentry->d_inode; + +audit_notify_watch (inode,MAY_WRITE ); error = -EROFS ;if (IS_RDONLY (inode))
@@ -674,6 +679,7 @@ static int chown_common(struct dentry *printk (KERN_ERR "chown_common: NULL inode\n" ); goto out; } +audit_notify_watch (inode,MAY_WRITE ); error = -EROFS ;if (IS_RDONLY (inode)) goto out;
include/linux/audit.h +74 -0
120 lines not on this page · raw message
include/linux/fs.h +3 -0
@@ -228,6 +228,7 @@ struct poll_table_struct;struct kstatfs ;struct vm_area_struct ;struct vfsmount ; +struct audit_inode_data ;/* Used to be a macro which just called the function, now just a function */ extern void update_atime (struct inode *);
@@ -1060,6 +1061,8 @@ struct super_operations {#define I_CLEAR 32#define I_NEW 64#define I_WILL_FREE 128 +#define I_AUDIT 256 +#define I_DIRTY (I_DIRTY_SYNC | I_DIRTY_DATASYNC | I_DIRTY_PAGES)
init/Kconfig +10 -0
@@ -181,6 +181,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/audit.c +21 -2
@@ -109,8 +109,13 @@ static DECLARE_WAIT_QUEUE_HEAD(kauditd_wstatic DECLARE_WAIT_QUEUE_HEAD (audit_backlog_wait);/* The netlink socket is only to be read by 1 CPU, which lets us assume -* that list additions and deletions never happen simultaneously in -* auditsc.c */ +* that list additions and deletions, and watch insertions never happen +* simultaneiously in auditsc.c and auditfs.c respectively. +* +* Even though there can only be one watch removal via netlink at a time, +* there is still a chance of watch removal via a hook. In this case, the +* semaphore is not enough. +*/ DECLARE_MUTEX (audit_netlink_sem);/* AUDIT_BUFSIZ is the size of the temporary buffer used for formatting
@@ -346,6 +351,9 @@ static int audit_netlink_ok(kernel_cap_tcase AUDIT_SET :case AUDIT_ADD :case AUDIT_DEL : +case AUDIT_WATCH_LIST : +case AUDIT_WATCH_INS : +case AUDIT_WATCH_REM :case AUDIT_SIGNAL_INFO :if (!cap_raised (eff_cap,CAP_AUDIT_CONTROL )) err = -EPERM ;
@@ -462,6 +470,17 @@ static int audit_receive_msg(struct sk_baudit_send_reply (NETLINK_CB (skb).pid, seq,AUDIT_SIGNAL_INFO ,0 ,0 , &sig_data,sizeof (sig_data));break ; +case AUDIT_WATCH_LIST : + err =audit_list_watches (pid, seq); +break ; +case AUDIT_WATCH_INS : +case AUDIT_WATCH_REM : +if (nlh->nlmsg_len <sizeof (struct watch_transport )) +return -EINVAL ; + err =audit_receive_watch (nlh->nlmsg_type, +NETLINK_CB (skb).pid, + uid, seq, data, loginuid); +break ;default : err = -EINVAL ;break ;
kernel/auditfs.c +855 -0
855 lines not on this page · raw message
kernel/auditsc.c +137 -1
171 lines not on this page · raw message
kernel/Makefile +1 -0
@@ -26,6 +26,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
security/selinux/nlmsgtab.c +3 -0
@@ -100,6 +100,9 @@ static struct nlmsg_perm nlmsg_audit_per { AUDIT_DEL, NETLINK_AUDIT_SOCKET__NLMSG_WRITE }, { AUDIT_USER, NETLINK_AUDIT_SOCKET__NLMSG_RELAY }, { AUDIT_SIGNAL_INFO, NETLINK_AUDIT_SOCKET__NLMSG_READ }, + { AUDIT_WATCH_INS, NETLINK_AUDIT_SOCKET__NLMSG_WRITE }, + { AUDIT_WATCH_REM, NETLINK_AUDIT_SOCKET__NLMSG_WRITE }, + { AUDIT_WATCH_LIST, NETLINK_AUDIT_SOCKET__NLMSG_READPRIV }, };
Quoted 8 lines · Timothy R. Chavez
To implement this feature we rely on the concepts of a "watch" and "watch list". Directories hold lists of "watches" (ie: "watch lists") that describe auditable file names one level beneath them. If a file holds a pointer into a "watch list" it is auditable. When accessed by a system call, information about the inode and its "watches" is added to the audit context of the current task (an inode may have multiple "watches" if a hard link to a "watched" file is itself being "watched") which is sent to user space upon system call exit.
This sounds almost identical to inotify. Is there some way you could use that instead? If not, you should explain why in your patch introduction.
thanks,
greg k-h
Hello,
This patch augments the audit subsystem and VFS to support file system auditing in which an object is audited based on its location and name. Depending on the administrator's requirement, this feature can be used standalone or in conjunction with the (device,inode)-based filters. 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 solely on the (device, inode)-based filters to ensure persistence of auditing across transactions.
This work is being done to make the Linux audit subsystem compliant with Common Criteria's Controlled Access Protection Profile (CAPP) specification. CAPP requires that the administrator is able to audit a file's content in two fundamental ways:
1) Where content confidentiality is important.
This includes security-relevant databases like /etc/passwd and the audit logs of which the contents are security sensitive. (device,inode)-based system-call filtering is sufficient to cover this requirement and is already available to the administrator.
2) Where content integrity is important.
This includes configuration files that affect the operation of trusted programs like /etc/shadow. Content integrity ensures audit persists across transactions where the underlying inode may change and content confidentiality is important. This is currently not doable without this patch.
To implement this feature we rely on the concepts of a "watch" and "watch list". Directories hold lists of "watches" (ie: "watch lists") that describe auditable file names one level beneath them. If a file holds a pointer into a "watch list" it is auditable. When accessed by a system call, information about the inode and its "watches" is added to the audit context of the current task (an inode may have multiple "watches" if a hard link to a "watched" file is itself being "watched") which is sent to user space upon system call exit.
This is similar to Inotify in that the audit subsystem watches for file system activity and collects information about inodes its interested in, but this is where the similarities stop. Despite the fact that the Inotify requirements only dictate a subset of the activity the audit subsystem is interested in, there is a more fundamental divergence between the two projects. Like audit, Inotify takes paths and resolves them to a single inode. But, unlike audit, Inotify does not find the path itself interesting. Much like the (device,inode)-based system call filters currently available in the audit subsystem, Inotify targets only individual inodes. Thus, if the underlying inode associated with the file /etc/shadow was changed, and /etc/shadow was being "watched", we'd lose auditability on /etc/shadow across transactions. More so, Inotify cannot watch inodes that do not yet exist (because the file does not yet exist). To do this, the audit subsystem must hook deeper than Inotify (in fs/dcache.c) to adapt with the file system as it changes. Where it makes sense, the small set of notification hooks in the VFS that Inotify and audit could share should be consolidated.
Some notable implementation details are as follows:
* struct inode is _not_ extended to associate audit data with the inode. A hash table is used in which the inode is hashed to retrieve its audit data. We know if an inode has audit data if I_AUDIT has been turned on in inode->i_state.
* Inodes with audit data are implicitly pinned in memory when I_AUDIT is turned on in inode->i_state. This prevents an auditable incore inode from being pushed out of the icache, preserving auditability even under memory pressures.
The user interface provided in kernel/audit.c is extended to allow the administrator to add "watches", remove "watches", and list "watches". Once the "watch" is inserted into the file system by the administrator, the audit subsystem takes over its maintenance until its removal.
We've submitted for inclusion to the -mm tree.
Thank you.
-tim
--- fs/attr.c | 3 fs/dcache.c | 9 fs/inode.c | 7 fs/namei.c | 13 fs/open.c | 6 include/linux/audit.h | 74 +++ include/linux/fs.h | 3 init/Kconfig | 10 kernel/Makefile | 1 kernel/audit.c | 23 + kernel/auditfs.c | 855 ++++++++++++++++++++++++++++++++++++++++++++ kernel/auditsc.c | 138 +++++++ security/selinux/nlmsgtab.c | 3 13 files changed, 1140 insertions(+), 5 deletions(-)
---
fs/attr.c +3 -0
@@ -15,6 +15,7 @@#include <linux/quotaops.h> #include <linux/security.h> #include <linux/time.h> +#include <linux/audit.h> /* Taken over from the old code... */
@@ -69,6 +70,8 @@ int inode_setattr(struct inode * inode,unsigned int ia_valid = attr->ia_valid;int error =0 ; +audit_notify_watch (inode,MAY_WRITE ); +if (ia_valid &ATTR_SIZE ) {if (attr->ia_size !=i_size_read (inode)) { error =vmtruncate (inode, attr->ia_size);
fs/dcache.c +9 -0
@@ -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_update_watch (dentry,1 ); dentry->d_inode =NULL ;list_del_init (&dentry->d_alias);spin_unlock (&dentry->d_lock);
@@ -966,6 +968,7 @@ void d_instantiate(struct dentry *entry,if (inode)list_add (&entry->d_alias, &inode->i_dentry); entry->d_inode = inode; +audit_update_watch (entry,0 );spin_unlock (&dcache_lock);security_d_instantiate (entry, inode); }
@@ -1142,6 +1145,7 @@ struct dentry *d_splice_alias(struct ino new =__d_find_alias (inode,1 );if (new) {BUG_ON (!(new->d_flags &DCACHE_DISCONNECTED )); +audit_update_watch (new,0 );spin_unlock (&dcache_lock);security_d_instantiate (new, inode);d_rehash (dentry);
@@ -1151,6 +1155,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_update_watch (dentry,0 );spin_unlock (&dcache_lock);security_d_instantiate (dentry, inode);d_rehash (dentry);
@@ -1254,6 +1259,7 @@ struct dentry * __d_lookup(struct dentryif (!d_unhashed (dentry)) {atomic_inc (&dentry->d_count); found = dentry; +audit_update_watch (dentry,0 ); }spin_unlock (&dentry->d_lock);break ;
@@ -1463,6 +1469,8 @@ void d_move(struct dentry * dentry, struspin_lock (&target->d_lock); } +audit_update_watch (dentry,1 ); +/* Move the dentry to the target hash queue, if on different bucket */ if (dentry->d_flags &DCACHE_UNHASHED ) goto already_unhashed;
@@ -1496,6 +1504,7 @@ already_unhashed:list_add (&target->d_child, &target->d_parent->d_subdirs); } +audit_update_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 -2
@@ -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:
@@ -173,6 +174,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);
@@ -262,7 +264,7 @@ void clear_inode(struct inode *inode)bd_forget (inode);if (inode->i_cdev)cd_forget (inode); - inode->i_state =I_CLEAR ; + inode->i_state =I_CLEAR | (inode->i_state &I_AUDIT ); }EXPORT_SYMBOL (clear_inode);
@@ -1051,7 +1053,7 @@ void generic_delete_inode(struct inode *hlist_del_init (&inode->i_hash);spin_unlock (&inode_lock);wake_up_inode (inode); -if (inode->i_state !=I_CLEAR ) +if ((inode->i_state & ~I_AUDIT ) !=I_CLEAR )BUG ();destroy_inode (inode); }
@@ -1364,6 +1366,7 @@ void __init inode_init(unsigned long mem 0, SLAB_RECLAIM_ACCOUNT|SLAB_PANIC, init_once, NULL);set_shrinker (DEFAULT_SEEKS , shrink_icache_memory); +audit_filesystem_init ();/* Hash may have been set up in inode_init_early */ if (!hashdist)return ;
fs/namei.c +13 -0
@@ -243,6 +243,8 @@ int permission(struct inode *inode, int } +audit_notify_watch (inode, mask); +/* Ordinary permission routines do not understand MAY_APPEND. */ submask = mask & ~MAY_APPEND ;if (inode->i_op && inode->i_op->permission)
@@ -358,6 +360,8 @@ static inline int exec_permission_lite(sif (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))
@@ -1188,6 +1192,8 @@ static inline int may_delete(struct inodBUG_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;
@@ -1312,6 +1318,7 @@ int vfs_create(struct inode *dir, structDQUOT_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); }
@@ -1637,6 +1644,7 @@ int vfs_mknod(struct inode *dir, structDQUOT_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); }
@@ -1710,6 +1718,7 @@ int vfs_mkdir(struct inode *dir, structDQUOT_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); }
@@ -1951,6 +1960,7 @@ int vfs_symlink(struct inode *dir, strucDQUOT_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); }
@@ -2024,6 +2034,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); }
@@ -2147,6 +2158,7 @@ static int vfs_rename_dir(struct inode * }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); }
@@ -2175,6 +2187,7 @@ static int vfs_rename_other(struct inode/* 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)
fs/open.c +6 -0
@@ -25,6 +25,7 @@#include <linux/pagemap.h> #include <linux/syscalls.h> #include <linux/rcupdate.h> +#include <linux/audit.h> #include <asm/unistd.h>
@@ -608,6 +609,8 @@ asmlinkage long sys_fchmod(unsigned int dentry = file->f_dentry; inode = dentry->d_inode; + +audit_notify_watch (inode,MAY_WRITE ); err = -EROFS ;if (IS_RDONLY (inode))
@@ -640,6 +643,8 @@ asmlinkage long sys_chmod(const char __uif (error) goto out; inode = nd.dentry->d_inode; + +audit_notify_watch (inode,MAY_WRITE ); error = -EROFS ;if (IS_RDONLY (inode))
@@ -674,6 +679,7 @@ static int chown_common(struct dentry *printk (KERN_ERR "chown_common: NULL inode\n" ); goto out; } +audit_notify_watch (inode,MAY_WRITE ); error = -EROFS ;if (IS_RDONLY (inode)) goto out;
include/linux/audit.h +74 -0
120 lines not on this page · raw message
include/linux/fs.h +3 -0
@@ -228,6 +228,7 @@ struct poll_table_struct;struct kstatfs ;struct vm_area_struct ;struct vfsmount ; +struct audit_inode_data ;/* Used to be a macro which just called the function, now just a function */ extern void update_atime (struct inode *);
@@ -1060,6 +1061,8 @@ struct super_operations {#define I_CLEAR 32#define I_NEW 64#define I_WILL_FREE 128 +#define I_AUDIT 256 +#define I_DIRTY (I_DIRTY_SYNC | I_DIRTY_DATASYNC | I_DIRTY_PAGES)
init/Kconfig +10 -0
@@ -181,6 +181,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/audit.c +21 -2
@@ -109,8 +109,13 @@ static DECLARE_WAIT_QUEUE_HEAD(kauditd_wstatic DECLARE_WAIT_QUEUE_HEAD (audit_backlog_wait);/* The netlink socket is only to be read by 1 CPU, which lets us assume -* that list additions and deletions never happen simultaneously in -* auditsc.c */ +* that list additions and deletions, and watch insertions never happen +* simultaneiously in auditsc.c and auditfs.c respectively. +* +* Even though there can only be one watch removal via netlink at a time, +* there is still a chance of watch removal via a hook. In this case, the +* semaphore is not enough. +*/ DECLARE_MUTEX (audit_netlink_sem);/* AUDIT_BUFSIZ is the size of the temporary buffer used for formatting
@@ -346,6 +351,9 @@ static int audit_netlink_ok(kernel_cap_tcase AUDIT_SET :case AUDIT_ADD :case AUDIT_DEL : +case AUDIT_WATCH_LIST : +case AUDIT_WATCH_INS : +case AUDIT_WATCH_REM :case AUDIT_SIGNAL_INFO :if (!cap_raised (eff_cap,CAP_AUDIT_CONTROL )) err = -EPERM ;
@@ -462,6 +470,17 @@ static int audit_receive_msg(struct sk_baudit_send_reply (NETLINK_CB (skb).pid, seq,AUDIT_SIGNAL_INFO ,0 ,0 , &sig_data,sizeof (sig_data));break ; +case AUDIT_WATCH_LIST : + err =audit_list_watches (pid, seq); +break ; +case AUDIT_WATCH_INS : +case AUDIT_WATCH_REM : +if (nlh->nlmsg_len <sizeof (struct watch_transport )) +return -EINVAL ; + err =audit_receive_watch (nlh->nlmsg_type, +NETLINK_CB (skb).pid, + uid, seq, data, loginuid); +break ;default : err = -EINVAL ;break ;
kernel/auditfs.c +855 -0
855 lines not on this page · raw message
kernel/auditsc.c +137 -1
171 lines not on this page · raw message
kernel/Makefile +1 -0
@@ -26,6 +26,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
security/selinux/nlmsgtab.c +3 -0
@@ -100,6 +100,9 @@ static struct nlmsg_perm nlmsg_audit_per { AUDIT_DEL, NETLINK_AUDIT_SOCKET__NLMSG_WRITE }, { AUDIT_USER, NETLINK_AUDIT_SOCKET__NLMSG_RELAY }, { AUDIT_SIGNAL_INFO, NETLINK_AUDIT_SOCKET__NLMSG_READ }, + { AUDIT_WATCH_INS, NETLINK_AUDIT_SOCKET__NLMSG_WRITE }, + { AUDIT_WATCH_REM, NETLINK_AUDIT_SOCKET__NLMSG_WRITE }, + { AUDIT_WATCH_LIST, NETLINK_AUDIT_SOCKET__NLMSG_READPRIV }, };
Quoted 8 lines · Timothy R. Chavez
This is similar to Inotify in that the audit subsystem watches for file system activity and collects information about inodes its interested in, but this is where the similarities stop. Despite the fact that the Inotify requirements only dictate a subset of the activity the audit subsystem is interested in, there is a more fundamental divergence between the two projects. Like audit, Inotify takes paths and resolves them to a single inode. But, unlike audit, Inotify does not find the path itself interesting.
Huh? inotify users find that path interesting, as they use it to act apon.
Quoted 5 lines
Much like the (device,inode)-based system call filters currently available in the audit subsystem, Inotify targets only individual inodes. Thus, if the underlying inode associated with the file
/etc/shadowwas changed, and/etc/shadowwas being "watched", we'd lose auditability on/etc/shadowacross transactions.
That's why you watch /etc/ instead, which catches that rename. That being said, why would not inotify also want this functionality if you think it is important?
Quoted 6 lines
More so, Inotify cannot watch inodes that do not yet exist (because the file does not yet exist). To do this, the audit subsystem must hook deeper than Inotify (in
fs/dcache.c) to adapt with the file system as it changes. Where it makes sense, the small set of notification hooks in the VFS that Inotify and audit could share should be consolidated.
As inotify works off of open file descriptors, yes, this is true. But, again, if you think this is really important, then why not just work with inotify to provide that kind of support to it?
I suggest you work together with the inotify developers to hash out your differences, as it sounds like you are duplicating a lot of the same functionality.
Also, inotify handles the namespace issues of processes by working off of a file descriptor. How do you handle this?
Do you have any documetation or example userspace code that shows how to use this auditfs interface you have created?
thanks,
greg k-h
Quoted 3 lines · Greg KH
As inotify works off of open file descriptors, yes, this is true. But, again, if you think this is really important, then why not just work with inotify to provide that kind of support to it?
http://marc.theaimsgroup.com/?l=linux-kernel&m=110265021327578&w=2
I think Tim was told not to dig into inotify. A lot of effort has been put into testing the code Tim has presented with review from several kernel developers (listed in the cc). They too should step up and give their opinion on this.
I want to believe questions were asked about this last December when we were starting into this effort. I think the conclusion from the inotify people was for us to proceed and then when we know what we really want, we can refactor should anything be in common.
Quoted 3 lines
I suggest you work together with the inotify developers to hash out your differences, as it sounds like you are duplicating a lot of the same functionality.
Maybe yes and no. Now that the fs audit code is out, I think we can spot commonality. The only common piece that I can think of is just the hook. The whole rest of it is different. I hope the inotify people comment on this to see if there is indeed something that should be refactored.
Quoted 2 lines
Do you have any documetation or example userspace code that shows how to use this auditfs interface you have created?
people.redhat.com/sgrubb/audit
The audit package is currently distributed in Fedora Core 4. The code to use Tim's fs audit code is in the user space app, but is waiting for the kernel pieces.
There is a man page for auditctl that shows all the options. (fs specific options are -wWpk ) To watch /etc/shadow, you would issue:
auditctl -w /etc/shadow -p wa
this will generate events for any update to the file including changes to ownership or permissions. We are interested in attribute changes as well. If you wanted to watch a file in a chroot directory, you could do this:
auditctl -w /var/chroot/etc/shadow -p wa -k /var/chroot
The audit events would indicate the path from the perspective of the app generating the events, but since we added the /var/chroot key, we can see that it really came from the chroot dir.
Hope this helps...
-Steve Grubb
Quoted 8 lines · Steve Grubb
Quoted 3 lines · Greg KH
As inotify works off of open file descriptors, yes, this is true. ?But, again, if you think this is really important, then why not just work with inotify to provide that kind of support to it?
http://marc.theaimsgroup.com/?l=linux-kernel&m=110265021327578&w=2
I think Tim was told not to dig into inotify.
That was over 6 months ago. Things change :)
Quoted 3 lines
A lot of effort has been put into testing the code Tim has presented with review from several kernel developers (listed in the cc). They too should step up and give their opinion on this.
Sure, be glad to listen to them.
Quoted 4 lines
I want to believe questions were asked about this last December when we were starting into this effort. I think the conclusion from the inotify people was for us to proceed and then when we know what we really want, we can refactor should anything be in common.
I fail to see any refactoring here, why not make your patch rely on theirs?
Quoted 6 lines
Quoted 3 lines
I suggest you work together with the inotify developers to hash out your differences, as it sounds like you are duplicating a lot of the same functionality.
Maybe yes and no. Now that the fs audit code is out, I think we can spot commonality. The only common piece that I can think of is just the hook.
That's a good place to start.
Quoted 2 lines
The whole rest of it is different. I hope the inotify people comment on this to see if there is indeed something that should be refactored.
I realize your userspace access is different, yet I do not believe yet that it should be this way.
Quoted 4 lines
Quoted 2 lines
Do you have any documetation or example userspace code that shows how to use this auditfs interface you have created?
people.redhat.com/sgrubb/audit
No documentation on the auditfs interface :(
Quoted 3 lines
The audit package is currently distributed in Fedora Core 4. The code to use Tim's fs audit code is in the user space app, but is waiting for the kernel pieces.
So the userspace package in FC4 will not use auditfs?
thanks,
greg k-h
Quoted 2 lines · Greg KH
I fail to see any refactoring here, why not make your patch rely on theirs?
At the time this code was developed, inotify was not in the kernel. We would be patching against another patch that's not in the kernel.
Quoted 5 lines
Quoted 2 lines
The whole rest of it is different. I hope the inotify people comment on this to see if there is indeed something that should be refactored.
I realize your userspace access is different, yet I do not believe yet that it should be this way.
The problems that we are faced with are dictated by CAPP and other security profiles. The audit user space access has been in the kernel for over a year, so that's not really a good thing to go changing.
Quoted 1 line
No documentation on the auditfs interface :(
That's what Tim's email was providing. Its a new component.
Quoted 5 lines
Quoted 3 lines
The audit package is currently distributed in Fedora Core 4. The code to use Tim's fs audit code is in the user space app, but is waiting for the kernel pieces.
So the userspace package in FC4 will not use auditfs?
Right. You get a few warnings due to missing functionality. If the kernel were patched with Tim's code, it all works as expected. We have worked out the user space access and that shouldn't be changing.
-Steve
Quoted 6 lines · Steve Grubb
Quoted 2 lines · Greg KH
I fail to see any refactoring here, why not make your patch rely on theirs?
At the time this code was developed, inotify was not in the kernel. We would be patching against another patch that's not in the kernel.
You all are asking for this patch to be added to -mm, which contains inotify.
Quoted 9 lines
Quoted 5 lines
Quoted 2 lines
The whole rest of it is different. I hope the inotify people comment on this to see if there is indeed something that should be refactored.
I realize your userspace access is different, yet I do not believe yet that it should be this way.
The problems that we are faced with are dictated by CAPP and other security profiles. The audit user space access has been in the kernel for over a year, so that's not really a good thing to go changing.
You are adding auditfs, a new userspace access, right? That's what I am referring to.
Quoted 3 lines
Quoted 1 line
No documentation on the auditfs interface :(
That's what Tim's email was providing. Its a new component.
His email provided no documentation that I could see. Am I just missing something?
Quoted 9 lines
Quoted 5 lines
Quoted 3 lines
The audit package is currently distributed in Fedora Core 4. The code to use Tim's fs audit code is in the user space app, but is waiting for the kernel pieces.
So the userspace package in FC4 will not use auditfs?
Right. You get a few warnings due to missing functionality. If the kernel were patched with Tim's code, it all works as expected. We have worked out the user space access and that shouldn't be changing.
Then what use is auditfs for if you don't need it?
Am I correct in thinking that you all need to split this patch into two pieces, the new inode stuff, and auditfs, as neither one has anything to do with the other?
Confused,
greg k-h
Quoted 1 line · Greg KH
You are adding auditfs, a new userspace access, right?
Not sure what you mean. This is using the same netlink interface that all the rest of the audit system is using for command and control. Nothing has changed here. What is different is the message I send into the kernel. The audit system dispatches it into Tim's code which in turn sets up the watch.
The notification that the event has occurred uses the same interface that all other audit events use. Its just a different message type. It is filtered by some attributes and if it is still of interest, the event is placed in the queue so it is serialized with all other events.
Quoted 2 lines
His email provided no documentation that I could see. Am I just missing something?
The auditfs code is programmed by filling out the watch_transport structure and sending a AUDIT_WATCH_INS message type. The perms, pathlen, & keylen are all that's filled out. The path & key are stored back to back in the payload section. To delete, you do the same thing and send AUDIT_WATCH_REM message. Yes, this should be added to the documentation.
Quoted 7 lines
Quoted 5 lines
Quoted 1 line
So the userspace package in FC4 will not use auditfs?
Right. You get a few warnings due to missing functionality. If the kernel were patched with Tim's code, it all works as expected. We have worked out the user space access and that shouldn't be changing.
Then what use is auditfs for if you don't need it?
But we do. You cannot audit files who's inode may change using the current syscall techniques. I put the code into FC4 hoping that one day we will have a kernel that supports it.
The current audit system code lets you audit by syscall, which means you can audit open or write, but then you get it for all processes that open or write. Or you could limit it with a pid, but that's impossible since you would have to predict the future pid when putting the audit rule in.
Tim's code lets you say I want change notification to this file only. The notification follows the audit format with all relavant pieces of information gathered at the time of the event and serialized with all other events.
Quoted 3 lines
Am I correct in thinking that you all need to split this patch into two pieces, the new inode stuff, and auditfs, as neither one has anything to do with the other?
It could be split to make it easier to read, but one's useless without the other.
-Steve
Quoted 3 lines · Steve Grubb
Tim's code lets you say I want change notification to this file only. The notification follows the audit format with all relavant pieces of information gathered at the time of the event and serialized with all other events.
well can't you sort of do that based on (selinux) security context of the file already? after all that's part of the inode already. Isn't that finegrained enough?
Quoted 10 lines · Arjan van de Ven
Quoted 3 lines · Steve Grubb
Tim's code lets you say I want change notification to this file only. The notification follows the audit format with all relavant pieces of information gathered at the time of the event and serialized with all other events.
well can't you sort of do that based on (selinux) security context of the file already? after all that's part of the inode already. Isn't that finegrained enough?
Provided you make it that far, yes, SE Linux _could_ be used to provide similar functionality. But, what if you bottom out on a DAC decision?
[foo@liltux /]$ cat /etc/shadow
cat: /etc/shadow: Permission denied
-tim
Quoted 12 lines · Timothy R. Chavez
Quoted 10 lines · Arjan van de Ven
Quoted 3 lines · Steve Grubb
Tim's code lets you say I want change notification to this file only. The notification follows the audit format with all relavant pieces of information gathered at the time of the event and serialized with all other events.
well can't you sort of do that based on (selinux) security context of the file already? after all that's part of the inode already. Isn't that finegrained enough?
Quoted 5 lines
Provided you make it that far, yes, SE Linux _could_ be used to provide similar functionality. But, what if you bottom out on a DAC decision?
[foo@liltux /]$ cat
/etc/shadow
cat:/etc/shadow: Permission denied
Additionally, the apps would need to either be rewritten to create the files under the audited context, or policy would have to cause all files created by those apps to be under the audited context. Neither one of those options is satisfactory.
thanks,
-serge
Quoted 8 lines
Quoted 3 lines
[foo@liltux /]$ cat
/etc/shadow
cat:/etc/shadow: Permission deniedAdditionally, the apps would need to either be rewritten to create the files under the audited context, or policy would have to cause all files created by those apps to be under the audited context. Neither one of those options is satisfactory
why not?
If your /etc/shadow has no selinux context you've lost already :0
Quoted 2 lines · Arjan van de Ven
why not?
If your/etc/shadowhas no selinux context you've lost already :0
No, the kernel will map it to something safe.
- James
Signature · 2 lines
James Morris <[email protected]>
Quoted 7 lines · Steve Grubb
Quoted 1 line · Greg KH
You are adding auditfs, a new userspace access, right?
Not sure what you mean. This is using the same netlink interface that all the rest of the audit system is using for command and control. Nothing has changed here. What is different is the message I send into the kernel. The audit system dispatches it into Tim's code which in turn sets up the watch.
What is auditfs for then?
Quoted 8 lines
Quoted 2 lines
His email provided no documentation that I could see. Am I just missing something?
The auditfs code is programmed by filling out the
watch_transportstructure and sending aAUDIT_WATCH_INSmessage type. The perms, pathlen, & keylen are all that's filled out. The path & key are stored back to back in the payload section. To delete, you do the same thing and sendAUDIT_WATCH_REMmessage. Yes, this should be added to the documentation.
So how does userspace interact with auditfs?
Quoted 3 lines
Tim's code lets you say I want change notification to this file only. The notification follows the audit format with all relavant pieces of information gathered at the time of the event and serialized with all other events.
That's great, and I understand the need for it. I just see all the duplication between this effort and inotify, and know of Tim's reluctance to want to work with inotify, and am objecting to that.
Quoted 6 lines
Quoted 3 lines
Am I correct in thinking that you all need to split this patch into two pieces, the new inode stuff, and auditfs, as neither one has anything to do with the other?
It could be split to make it easier to read, but one's useless without the other.
Ok, some documentation about auditfs would go a long way toward understanding this...
thanks,
greg k-h
Quoted 10 lines · Greg KH
Quoted 8 lines · Timothy R. Chavez
This is similar to Inotify in that the audit subsystem watches for file system activity and collects information about inodes its interested in, but this is where the similarities stop. Despite the fact that the Inotify requirements only dictate a subset of the activity the audit subsystem is interested in, there is a more fundamental divergence between the two projects. Like audit, Inotify takes paths and resolves them to a single inode. But, unlike audit, Inotify does not find the path itself interesting.
Quoted 2 lines
Huh? inotify users find that path interesting, as they use it to act apon.
I really didn't want to go back and forth on this (my fingers are still sore from our convo on ltc-interlock :-)), but I might as well oblige.
Quoted 10 lines
Quoted 6 lines
Much like the (device,inode)-based system call filters currently available in the audit subsystem, Inotify targets only individual inodes. Thus, if the underlying inode associated with the file
/etc/shadowwas changed, and/etc/shadowwas being "watched", we'd lose auditability on/etc/shadowacross transactions.That's why you watch
/etc/instead, which catches that rename. That being said, why would not inotify also want this functionality if you think it is important?
Sure, by watching a directory with Inotify, you receive notification upon rename() events. Now, as far as Inotify is concerned, how this is finally interpreted is up to the user space programs it's feeding. In terms of audit, there must never be a chance for subversion. If this feature were implemented in terms of Inotify as it stands now, the processing we do in fs/dcache.c would need to be shifted to a user space app which is obviously out of the question. One alternative, the one I believe you're in favor of, is to augment Inotify itself. But, AFAICT, nothing in Inotify's documentation leads me to believe that this adds anything to Inotify itself (with the exception of complication and obfuscation), especially in terms of event notification. IMO, by tieing our logic to Inotify we'd inadvertently be dooming it to the dreaded Second-system syndrome:
http://en.wikipedia.org/wiki/Second-system_effect
Quoted 11 lines
Quoted 7 lines
More so, Inotify cannot watch inodes that do not yet exist (because the file does not yet exist). To do this, the audit subsystem must hook deeper than Inotify (in
fs/dcache.c) to adapt with the file system as it changes. Where it makes sense, the small set of notification hooks in the VFS that Inotify and audit could share should be consolidated.As inotify works off of open file descriptors, yes, this is true. But, again, if you think this is really important, then why not just work with inotify to provide that kind of support to it?
I've yet to be convinced that merging these two projects or implementing one in terms of the other has any real benefit to either project in terms of their individual goals and requirements.
The only real similarty between the two projects from my POV is that they are both interested in reporting a subset of file system activity and could benefit from a set of common hooks (ie: fsnotify) where it makes sense.
Quoted 4 lines
I suggest you work together with the inotify developers to hash out your differences, as it sounds like you are duplicating a lot of the same functionality.
Where functionality is the same or similar, we can address that, but surely in the interim the inclusion of this patch into -mm would be beneficial to the 98% of dissimilar functionality :)
Quoted 3 lines
Also, inotify handles the namespace issues of processes by working off of a file descriptor. How do you handle this?
Well I guess this boils down to what you consider an issue. In terms of audit and this patch, tagging audit info to the inode ensures that it's auditable from any namespace. When a location and name is audited, the administrator does so from what they consider a meaningful namespace and device (ie: for most, the path /etc/shadow is meaningful, so by "watching" it, you've made a file containing security secrets auditable and not your favorite LOTR ascii art) .
-tim
Quoted 14 lines
Do you have any documetation or example userspace code that shows how to use this auditfs interface you have created?
thanks,
greg k-h
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoted 11 lines · Timothy R. Chavez
Quoted 10 lines · Greg KH
Quoted 8 lines · Timothy R. Chavez
This is similar to Inotify in that the audit subsystem watches for file system activity and collects information about inodes its interested in, but this is where the similarities stop. Despite the fact that the Inotify requirements only dictate a subset of the activity the audit subsystem is interested in, there is a more fundamental divergence between the two projects. Like audit, Inotify takes paths and resolves them to a single inode. But, unlike audit, Inotify does not find the path itself interesting.
Quoted 5 lines
Quoted 2 lines
Huh? inotify users find that path interesting, as they use it to act apon.
I really didn't want to go back and forth on this (my fingers are still sore from our convo on ltc-interlock :-)), but I might as well oblige.
I'm sorry, but discussions on internal mailing lists mean nothing here on lkml. How would we know what you all discussed, or if it was even correct?
Quoted 6 lines
Quoted 6 lines
Quoted 5 lines
Much like the (device,inode)-based system call filters currently available in the audit subsystem, Inotify targets only individual inodes. Thus, if the underlying inode associated with the file
/etc/shadowwas changed, and/etc/shadowwas being "watched", we'd lose auditability on/etc/shadowacross transactions.
Quoted 7 lines
Quoted 3 lines
That's why you watch
/etc/instead, which catches that rename. That being said, why would not inotify also want this functionality if you think it is important?Sure, by watching a directory with Inotify, you receive notification upon
rename()events. Now, as far as Inotify is concerned, how this is finally interpreted is up to the user space programs it's feeding.
Which should be the same as audit, right? audit is just reporting what happens.
Quoted 1 line
In terms of audit, there must never be a chance for subversion.
"never" is a tough thing to prove :)
Quoted 3 lines
If this feature were implemented in terms of Inotify as it stands now, the processing we do in
fs/dcache.cwould need to be shifted to a user space app which is obviously out of the question.
Why "obviously"? It isn't obvious to me.
Quoted 2 lines
One alternative, the one I believe you're in favor of, is to augment Inotify itself.
Yes.
Quoted 4 lines
But, AFAICT, nothing in Inotify's documentation leads me to believe that this adds anything to Inotify itself (with the exception of complication and obfuscation), especially in terms of event notification.
It would allow inotify to work for you also. That's a good enough reason.
Quoted 4 lines
IMO, by tieing our logic to Inotify we'd inadvertently be dooming it to the dreaded Second-system syndrome:
I fail to see how that has any relevance on this discussion.
Quoted 7 lines
Quoted 7 lines
Quoted 6 lines
More so, Inotify cannot watch inodes that do not yet exist (because the file does not yet exist). To do this, the audit subsystem must hook deeper than Inotify (in
fs/dcache.c) to adapt with the file system as it changes. Where it makes sense, the small set of notification hooks in the VFS that Inotify and audit could share should be consolidated.
Quoted 7 lines
Quoted 3 lines
As inotify works off of open file descriptors, yes, this is true. But, again, if you think this is really important, then why not just work with inotify to provide that kind of support to it?
I've yet to be convinced that merging these two projects or implementing one in terms of the other has any real benefit to either project in terms of their individual goals and requirements.
You have common hooks in the kernel. You do almost the same thing (report fs changes to userspace.) Seems a natural thing to me.
Quoted 3 lines
The only real similarty between the two projects from my POV is that they are both interested in reporting a subset of file system activity and could benefit from a set of common hooks (ie: fsnotify) where it makes sense.
Exactly. What else is there?
Quoted 7 lines
Quoted 3 lines
I suggest you work together with the inotify developers to hash out your differences, as it sounds like you are duplicating a lot of the same functionality.
Where functionality is the same or similar, we can address that, but surely in the interim the inclusion of this patch into -mm would be beneficial to the 98% of dissimilar functionality :)
I don't see such a huge dissimilar functionality, but hey, feel free to prove me wrong :)
Quoted 4 lines
Quoted 2 lines
Also, inotify handles the namespace issues of processes by working off of a file descriptor. How do you handle this?
Well I guess this boils down to what you consider an issue.
Namespaces are an issue.
Quoted 2 lines
In terms of audit and this patch, tagging audit info to the inode ensures that it's auditable from any namespace.
Agreed. But how do you report that inode info to userspace when you don't know the namespace it was accessed under? Or am I missing something here?
Quoted 5 lines
When a location and name is audited, the administrator does so from what they consider a meaningful namespace and device (ie: for most, the path
/etc/shadowis meaningful, so by "watching" it, you've made a file containing security secrets auditable and not your favorite LOTR ascii art) .
Yes, and then I change namespaces to put /etc/shadow at /foo/baz/etc/shadow and then access it that way? Will the current audit system fail to catch that access?
thanks,
greg k-h
Quoted 3 lines · Greg KH
Yes, and then I change namespaces to put
/etc/shadowat /foo/baz/etc/shadow and then access it that way? Will the current audit system fail to catch that access?
The watch is attached to the inode which you happened to call '/etc' in your namespace, and takes effect in _any_ namespace regardless of the path to it.
In the audit trail, you see the path which was used in the audited process's namespace, and also the filter key which was associated with that watch when you added it.
Signature · 1 line
dwmw2
Quoted 5 lines · David Woodhouse
Quoted 3 lines · Greg KH
Yes, and then I change namespaces to put
/etc/shadowat /foo/baz/etc/shadow and then access it that way? Will the current audit system fail to catch that access?
Quoted 7 lines
The watch is attached to the inode which you happened to call '/etc' in your namespace, and takes effect in _any_ namespace regardless of the path to it.
In the audit trail, you see the path which was used in the audited process's namespace, and also the filter key which was associated with that watch when you added it.
Ok, thanks, that makes sense.
greg k-h
Quoted 15 lines · Greg KH
Quoted 14 lines · Timothy R. Chavez
Quoted 12 lines · Greg KH
Quoted 8 lines · Timothy R. Chavez
This is similar to Inotify in that the audit subsystem watches for file system activity and collects information about inodes its interested in, but this is where the similarities stop. Despite the fact that the Inotify requirements only dictate a subset of the activity the audit subsystem is interested in, there is a more fundamental divergence between the two projects. Like audit, Inotify takes paths and resolves them to a single inode. But, unlike audit, Inotify does not find the path itself interesting.
Huh? inotify users find that path interesting, as they use it to act apon.
Quoted 6 lines
Quoted 2 lines
I really didn't want to go back and forth on this (my fingers are still sore from our convo on ltc-interlock :-)), but I might as well oblige.
I'm sorry, but discussions on internal mailing lists mean nothing here on lkml. How would we know what you all discussed, or if it was even correct?
Oh I was only playfully commenting on the discussion between you and I half a year or so ago :).
Quoted 15 lines
Quoted 7 lines
Quoted 7 lines
Quoted 6 lines
Much like the (device,inode)-based system call filters currently available in the audit subsystem, Inotify targets only individual inodes. Thus, if the underlying inode associated with the file
/etc/shadowwas changed, and/etc/shadowwas being "watched", we'd lose auditability on/etc/shadowacross transactions.Quoted 7 lines
Quoted 3 lines
That's why you watch
/etc/instead, which catches that rename. That being said, why would not inotify also want this functionality if you think it is important?Sure, by watching a directory with Inotify, you receive notification upon
rename()events. Now, as far as Inotify is concerned, how this is finally interpreted is up to the user space programs it's feeding.
Quoted 6 lines
Which should be the same as audit, right? audit is just reporting what happens.
Quoted 1 line
In terms of audit, there must never be a chance for subversion.
"never" is a tough thing to prove :)
You're right, a bit bold, but ideally it is what we're striving for. Where we can prevent subversion or where it's reasonable to prevent subversion we should make the effort to do so.
Quoted 6 lines
Quoted 4 lines
If this feature were implemented in terms of Inotify as it stands now, the processing we do in
fs/dcache.cwould need to be shifted to a user space app which is obviously out of the question.Why "obviously"? It isn't obvious to me.
Even if access control prohibits us from actually seeing the content of /etc/shadow, if we're auditing /etc/shadow, attempts should be logged and not gone unnoticed.
watch /etc/shadow
passwd
<inode changes before execution ends>
cat /etc/shadow (goes unaudited)
<send event to user space>
ln /etc/shadow /home/foo/evil_shadow (goes unaudited) <process in user space>
cat /home/foo/evil_shadow (goes unaudited)
<send back instructions to kernel to watch the new /etc/shadow> cat /etc/shadow (audited)
Hmmm...
Quoted 32 lines
Quoted 3 lines
One alternative, the one I believe you're in favor of, is to augment Inotify itself.
Yes.
Quoted 4 lines
But, AFAICT, nothing in Inotify's documentation leads me to believe that this adds anything to Inotify itself (with the exception of complication and obfuscation), especially in terms of event notification.
It would allow inotify to work for you also. That's a good enough reason.
Quoted 4 lines
IMO, by tieing our logic to Inotify we'd inadvertently be dooming it to the dreaded Second-system syndrome:
I fail to see how that has any relevance on this discussion.
Quoted 11 lines
Quoted 10 lines
Quoted 6 lines
More so, Inotify cannot watch inodes that do not yet exist (because the file does not yet exist). To do this, the audit subsystem must hook deeper than Inotify (in
fs/dcache.c) to adapt with the file system as it changes. Where it makes sense, the small set of notification hooks in the VFS that Inotify and audit could share should be consolidated.As inotify works off of open file descriptors, yes, this is true. But, again, if you think this is really important, then why not just work with inotify to provide that kind of support to it?
Quoted 6 lines
Quoted 3 lines
I've yet to be convinced that merging these two projects or implementing one in terms of the other has any real benefit to either project in terms of their individual goals and requirements.
You have common hooks in the kernel. You do almost the same thing (report fs changes to userspace.) Seems a natural thing to me.
This patch reports "fs changes" to the audit subsystem which then adds them to the audit_context of the current task. Then, at system call exit, the audit_context is sent to user space, effectively logging not only the "fs change", but information regarding the process, system call, path, etc, to give the complete account.
Here's some sample output in human-readable format (I didn't include a filterkey)
[root@liltux ~]# /sbin/ausearch -i
----
type=PATH msg=audit(07/07/11 01:32:59.484:116357) : name=/audit/foo flags=follow,open inode=3074232 dev=fd:00 mode=file,644 ouid=root ogid=root rdev=00:00
type=CWD msg=audit(07/07/11 01:32:59.484:116357) : cwd=/root
type=FS_INODE msg=audit(07/07/11 01:32:59.484:116357) : inode=3074232 inode_uid=root inode_gid=root inode_dev=fd:00 inode_rdev=00:00
type=FS_WATCH msg=audit(07/07/11 01:32:59.484:116357) : watch_inode=3074232 watch=foo filterkey= perm= perm_mask=read
type=SYSCALL msg=audit(07/07/11 01:32:59.484:116357) : arch=i386 syscall=open per=400000 success=yes exit=3 a0=bffffbfc a1=8000 a2=0 a3=8000 items=1 pid=12670 auid=unknown(4294967295) uid=root gid=root euid=root suid=root fsuid=root egid=root sgid=root fsgid=root comm=cat exe=/bin/cat
Quoted 6 lines
Quoted 4 lines
The only real similarty between the two projects from my POV is that they are both interested in reporting a subset of file system activity and could benefit from a set of common hooks (ie: fsnotify) where it makes sense.
Exactly. What else is there?
The fact that audit is interested in more activity then Inotify (like permission()), must strive to never lose events, interacts with the audit subsystem directly, must conform to security requirements placed on it by various Protection Profiles (for the time being, CAPP), defers sending the event record to user space until system call exit, the inode is relevant only if it's associated with the last component of an audited path (not just the inode targetted initially)... And I'm sure Robert could point out things that Inotify does that audit doesn't do (and isn't very interested in doing).
Quoted 11 lines
Quoted 8 lines
Quoted 4 lines
I suggest you work together with the inotify developers to hash out your differences, as it sounds like you are duplicating a lot of the same functionality.
Where functionality is the same or similar, we can address that, but surely in the interim the inclusion of this patch into -mm would be beneficial to the 98% of dissimilar functionality :)
I don't see such a huge dissimilar functionality, but hey, feel free to prove me wrong :)
Maybe after you play with auditctl some, you'll see it? :)
-tim
Quoted 28 lines
Quoted 5 lines
Quoted 3 lines
Also, inotify handles the namespace issues of processes by working off of a file descriptor. How do you handle this?
Well I guess this boils down to what you consider an issue.
Namespaces are an issue.
Quoted 2 lines
In terms of audit and this patch, tagging audit info to the inode ensures that it's auditable from any namespace.
Agreed. But how do you report that inode info to userspace when you don't know the namespace it was accessed under? Or am I missing something here?
Quoted 5 lines
When a location and name is audited, the administrator does so from what they consider a meaningful namespace and device (ie: for most, the path
/etc/shadowis meaningful, so by "watching" it, you've made a file containing security secrets auditable and not your favorite LOTR ascii art) .Yes, and then I change namespaces to put
/etc/shadowat /foo/baz/etc/shadow and then access it that way? Will the current audit system fail to catch that access?thanks,
greg k-h
Quoted 16 lines · Timothy R. Chavez
Even if access control prohibits us from actually seeing the content of
/etc/shadow, if we're auditing/etc/shadow, attempts should be logged and not gone unnoticed.watch
/etc/shadow
passwd
<inode changes before execution ends>
cat/etc/shadow(goes unaudited)
<send event to user space>
ln/etc/shadow/home/foo/evil_shadow(goes unaudited) <process in user space>
cat/home/foo/evil_shadow(goes unaudited)
<send back instructions to kernel to watch the new/etc/shadow> cat/etc/shadow(audited)
Quoted 1 line
Hmmm...
Then you watch the directory /etc/ which would have caught all of this, right? My argument is that wouldn't inotify also want to know this exact same thing?
Quoted 11 lines
Quoted 6 lines
Quoted 3 lines
I've yet to be convinced that merging these two projects or implementing one in terms of the other has any real benefit to either project in terms of their individual goals and requirements.
You have common hooks in the kernel. You do almost the same thing (report fs changes to userspace.) Seems a natural thing to me.
This patch reports "fs changes" to the audit subsystem which then adds them to the
audit_contextof the current task. Then, at system call exit, theaudit_contextis sent to user space, effectively logging not only the "fs change", but information regarding the process, system call, path, etc, to give the complete account.
Great, so your userspace notification is different than inotify, that's fine. But what you are trying to do within the kernel is pretty much the same, which is my main point.
Quoted 7 lines
Quoted 5 lines
Quoted 3 lines
The only real similarty between the two projects from my POV is that they are both interested in reporting a subset of file system activity and could benefit from a set of common hooks (ie: fsnotify) where it makes sense.
Exactly. What else is there?
The fact that audit is interested in more activity then Inotify (like
permission()),
Fine, inotify users can just ignore those messages. Or perhaps they will grow to want to see them.
Quoted 1 line
must strive to never lose events,
Why is this not a good goal for inotify to also have?
Quoted 1 line
interacts with the audit subsystem directly,
Ok, a difference.
Quoted 2 lines
must conform to security requirements placed on it by various Protection Profiles (for the time being, CAPP),
And those "security requirements" are what?
Quoted 1 line
defers sending the event record to user space until system call exit,
Again, not a big deal for inotify.
Quoted 2 lines
the inode is relevant only if it's associated with the last component of an audited path (not just the inode targetted initially)...
Again, something that I think inotify would like to also know.
Quoted 2 lines
And I'm sure Robert could point out things that Inotify does that audit doesn't do (and isn't very interested in doing).
Sure, just because you all do some things differently, doesn't mean that you shouldn't use the same core code.
Now I know you don't want to worry about another project's code getting into the kernel tree to depend on your changes, but hey, that's life. Try working with the inotify developers instead of ignoring them and duplicating the same stuff.
Also, you still have not pointed out what auditfs is for and how to use it.
thanks,
greg k-h
Quoted 78 lines · Greg KH
Quoted 16 lines · Timothy R. Chavez
Even if access control prohibits us from actually seeing the content of
/etc/shadow, if we're auditing/etc/shadow, attempts should be logged and not gone unnoticed.watch
/etc/shadow
passwd
<inode changes before execution ends>
cat/etc/shadow(goes unaudited)
<send event to user space>
ln/etc/shadow/home/foo/evil_shadow(goes unaudited) <process in user space>
cat/home/foo/evil_shadow(goes unaudited)
<send back instructions to kernel to watch the new/etc/shadow> cat/etc/shadow(audited)Quoted 1 line
Hmmm...
Then you watch the directory
/etc/which would have caught all of this, right? My argument is that wouldn't inotify also want to know this exact same thing?Quoted 11 lines
Quoted 6 lines
Quoted 3 lines
I've yet to be convinced that merging these two projects or implementing one in terms of the other has any real benefit to either project in terms of their individual goals and requirements.
You have common hooks in the kernel. You do almost the same thing (report fs changes to userspace.) Seems a natural thing to me.
This patch reports "fs changes" to the audit subsystem which then adds them to the
audit_contextof the current task. Then, at system call exit, theaudit_contextis sent to user space, effectively logging not only the "fs change", but information regarding the process, system call, path, etc, to give the complete account.Great, so your userspace notification is different than inotify, that's fine. But what you are trying to do within the kernel is pretty much the same, which is my main point.
Quoted 7 lines
Quoted 5 lines
Quoted 3 lines
The only real similarty between the two projects from my POV is that they are both interested in reporting a subset of file system activity and could benefit from a set of common hooks (ie: fsnotify) where it makes sense.
Exactly. What else is there?
The fact that audit is interested in more activity then Inotify (like
permission()),Fine, inotify users can just ignore those messages. Or perhaps they will grow to want to see them.
Quoted 1 line
must strive to never lose events,
Why is this not a good goal for inotify to also have?
Quoted 1 line
interacts with the audit subsystem directly,
Ok, a difference.
Quoted 2 lines
must conform to security requirements placed on it by various Protection Profiles (for the time being, CAPP),
And those "security requirements" are what?
Quoted 1 line
defers sending the event record to user space until system call exit,
Again, not a big deal for inotify.
Quoted 2 lines
the inode is relevant only if it's associated with the last component of an audited path (not just the inode targetted initially)...
Again, something that I think inotify would like to also know.
Quoted 2 lines
And I'm sure Robert could point out things that Inotify does that audit doesn't do (and isn't very interested in doing).
Sure, just because you all do some things differently, doesn't mean that you shouldn't use the same core code.
Quoted 8 lines
Now I know you don't want to worry about another project's code getting into the kernel tree to depend on your changes, but hey, that's life. Try working with the inotify developers instead of ignoring them and duplicating the same stuff.
Also, you still have not pointed out what auditfs is for and how to use it.
Yeah, it's admittedly named badly. Perhaps that can change.
kernel/audit.ckernel/auditsc.ckernel/auditfs.c
Quoted 4 lines
thanks,
greg k-h
Hello Greg,
I've chosen not to respond to individual segments because the overall theme is that the right thing to do is to not duplicate functionality. Your suggestion is to merge the two projects. In your mind, this benefits both projects. It will consolidate the common functionality and enhance Inotify such that it is not only an event notification system for user space, but for other parts of the kernel -- namely audit -- as well. Is this a fair assessment?
I think we can all agree that functionality should not be duplicated. I have conceded that having two seperate hooks side-by-side that have the same purpose should be consolidated. Because this would be generic, all other notification hooks should be available to Inotify (and any other subsystem) as well. Agreed?
My vision is that audit, Inotify, and whatever else plugs into this framework and receives notifications this way -- a file system event notification system for the kernel. Then, audit, Inotify, etc would process this even information it receives and does with it what they will.
Ultimately, the part where we differ most, is the processing of information in fs/dcache.c to give dynamic updates in response to file system activity (such as attaching audit information to an auditable file whose inode just changed). I believe this should be kept seperate and not part of this framework nor Inotify. It's a specific requirement for audit, but not for Inotify. This is one of the places the two systems are functionally different.
By doing it this way, both projects can retain their original goals and meet their individual requirements, and all the common pieces are consolidated in a logical way. This is something Robert and I had discussed on LKML in early December 2004, and was brought up in a discussion more recently for an RFC on LKML in early June between Christoph Hellwig and myself.
Can this patch not be placed in -mm for the time being, as-is? Surely the logic it implements, what I envision will ultimately be retained, could benefit from the exposure in the interim?
-tim
Quoted 2 lines · Timothy R. Chavez
I've chosen not to respond to individual segments because the overall theme is that the right thing to do is to not duplicate functionality.
Correct.
Quoted 1 line
Your suggestion is to merge the two projects.
Or at the minimal, the common parts of them.
Quoted 1 line
In your mind, this benefits both projects.
I'm guessing that you don't think this will?
Quoted 4 lines
It will
consolidate the common functionality and enhance Inotify such that it is not only an event notification system for user space, but for other parts of the kernel -- namely audit -- as well. Is this a fair assessment?
Correct.
Quoted 5 lines
I think we can all agree that functionality should not be duplicated. I have conceded that having two seperate hooks side-by-side that have the same purpose should be consolidated. Because this would be generic, all other notification hooks should be available to Inotify (and any other subsystem) as well. Agreed?
Yes.
Quoted 4 lines
My vision is that audit, Inotify, and whatever else plugs into this framework and receives notifications this way -- a file system event notification system for the kernel. Then, audit, Inotify, etc would process this even information it receives and does with it what they will.
Sounds good.
Quoted 6 lines
Ultimately, the part where we differ most, is the processing of information in
fs/dcache.cto give dynamic updates in response to file system activity (such as attaching audit information to an auditable file whose inode just changed). I believe this should be kept seperate and not part of this framework nor Inotify. It's a specific requirement for audit, but not for Inotify. This is one of the places the two systems are functionally different.
I don't think it should be different. If inotify wants to just ignore this information, it can.
Quoted 5 lines
By doing it this way, both projects can retain their original goals and meet their individual requirements, and all the common pieces are consolidated in a logical way. This is something Robert and I had discussed on LKML in early December 2004, and was brought up in a discussion more recently for an RFC on LKML in early June between Christoph Hellwig and myself.
I'm very sad to see you ignored these comments by others. Any reason why? It is pretty rude...
Quoted 3 lines
Can this patch not be placed in -mm for the time being, as-is? Surely the logic it implements, what I envision will ultimately be retained, could benefit from the exposure in the interim?
Why not get it right, you agree that you know how to do this, and that it should be done. What's the point of adding it now, as it will be redone anyway? That would cause more work for others (andrew doesn't maintain patches for free in -mm), and cause reports from people for a codebase that is not going to ever make it into mainline.
Just do the right thing, you know what it is.
thanks,
greg k-h
A message that is not in the archive (sent off-list or not yet received): OF993CB74B.E135A576-ON8725703B.00568CD6-0525703B.005814C3@us.ibm.com
Quoted 7 lines · Michael C Thompson
Quoted 7 lines
Quoted 6 lines
Ultimately, the part where we differ most, is the processing of information in
fs/dcache.cto give dynamic updates in response to file system activity (such as attaching audit information to an auditable file whose inode just changed). I believe this should be kept seperate and not part of this framework nor Inotify. It's a specific requirement for audit, but not for Inotify. This is one of the places the two systems are functionally different.
Quoted 5 lines
Quoted 2 lines
I don't think it should be different. If inotify wants to just ignore this information, it can.
Doesn't this mentality bring with it the risk of bloating a framework that should be as "trim" as possible?
vs. the mentality that since you are doing something just a bit different, you should duplicate lots of other functionality too? no.
greg k-h
fs/attr.c int inode_setattr(struct inode * inode, · line 74
Quoted 6 lines · * Timothy R. Chavez ([email protected]) wrote:
fs/attr.c +2
-0@@ -69,6 +70,8 @@ int inode_setattr(struct inode * inode,unsigned int ia_valid = attr->ia_valid;int error =0 ; +audit_notify_watch (inode,MAY_WRITE ); +
Hmm, this looks wrong. It should be in notify_change, since I don't think there's a strict requirement that a fs call inode_setattr, which is just a commonly used helper.
Quoted 25 lines
fs/attr.c +0
-0if (ia_valid &ATTR_SIZE ) {if (attr->ia_size !=i_size_read (inode)) { error =vmtruncate (inode, attr->ia_size);diff -Nurp
linux-2.6.13-rc1-mm1/fs/namei.clinux-2.6.13-rc1-mm1~auditfs/fs/namei.cfs/namei.c +4
-0@@ -243,6 +243,8 @@ int permission(struct inode *inode, int } +audit_notify_watch (inode, mask); +/* Ordinary permission routines do not understand MAY_APPEND. */ submask = mask & ~MAY_APPEND ;if (inode->i_op && inode->i_op->permission)@@ -358,6 +360,8 @@ static inline int exec_permission_lite(sif (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))
fs/namei.c static inline int may_delete(struct inod · line 1196
Quoted 5 lines
fs/namei.c +2
-0@@ -1188,6 +1192,8 @@ static inline int may_delete(struct inodBUG_ON (victim->d_parent->d_inode != dir); +audit_notify_watch (victim->d_inode,MAY_WRITE ); +
I forget why you need two separate notifications on delete?
fs/namei.c int vfs_create(struct inode *dir, struct · line 1321
Quoted 8 lines
fs/namei.c +1
-0error =permission (dir,MAY_WRITE |MAY_EXEC ,NULL );if (error)return error;@@ -1312,6 +1318,7 @@ int vfs_create(struct inode *dir, structDQUOT_INIT (dir); error = dir->i_op->create (dir, dentry, mode, nd);if (!error) { +audit_notify_watch (dentry->d_inode,MAY_WRITE );
And on create. And, as I've mentioned, these bits could be collapsed with fsnotify hooks. I'll split that off from inotify, perhaps you can work against that...
Quoted 36 lines
fs/namei.c +4
-0fsnotify_create (dir, dentry->d_name.name);security_inode_post_create (dir, dentry, mode); }@@ -1637,6 +1644,7 @@ int vfs_mknod(struct inode *dir, structDQUOT_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); }@@ -1710,6 +1718,7 @@ int vfs_mkdir(struct inode *dir, structDQUOT_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); }@@ -1951,6 +1960,7 @@ int vfs_symlink(struct inode *dir, strucDQUOT_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); }@@ -2024,6 +2034,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); }
fs/namei.c static int vfs_rename_dir(struct inode * · line 2161
Quoted 4 lines
fs/namei.c +1
-0@@ -2147,6 +2158,7 @@ static int vfs_rename_dir(struct inode * }if (!error) {d_move (old_dentry,new_dentry); +audit_notify_watch (old_dentry->d_inode,MAY_WRITE );
Don't you get two identical notifications here (one from permission, and one from here)? And where's the update for the new?
fs/namei.c static int vfs_rename_other(struct inode · line 2190
Quoted 8 lines
fs/namei.c +1
-0security_inode_post_rename (old_dir, old_dentry, new_dir, new_dentry); }@@ -2175,6 +2187,7 @@ static int vfs_rename_other(struct inode/* 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 );
Same here?
Quoted 15 lines
fs/namei.c +0
-0security_inode_post_rename (old_dir, old_dentry, new_dir, new_dentry); }if (target)diff -Nurp
linux-2.6.13-rc1-mm1/fs/open.clinux-2.6.13-rc1-mm1~auditfs/fs/open.cfs/open.c +1
-0@@ -25,6 +25,7 @@#include <linux/pagemap.h> #include <linux/syscalls.h> #include <linux/rcupdate.h> +#include <linux/audit.h> #include <asm/unistd.h>
fs/open.c asmlinkage long sys_fchmod(unsigned int · line 613
Quoted 5 lines
fs/open.c +2
-0@@ -608,6 +609,8 @@ asmlinkage long sys_fchmod(unsigned int dentry = file->f_dentry; inode = dentry->d_inode; + +audit_notify_watch (inode,MAY_WRITE );
looks like you'll get two updates for these, this should be done in notify_change.
fs/open.c asmlinkage long sys_chmod(const char __u · line 647
Quoted 8 lines
fs/open.c +2
-0err = -EROFS ;if (IS_RDONLY (inode))@@ -640,6 +643,8 @@ asmlinkage long sys_chmod(const char __uif (error) goto out; inode = nd.dentry->d_inode; + +audit_notify_watch (inode,MAY_WRITE );
ditto
fs/open.c static int chown_common(struct dentry * · line 682
Quoted 7 lines
fs/open.c +1
-0error = -EROFS ;if (IS_RDONLY (inode))@@ -674,6 +679,7 @@ static int chown_common(struct dentry *printk (KERN_ERR "chown_common: NULL inode\n" ); goto out; } +audit_notify_watch (inode,MAY_WRITE );
ditto, and wouldn't it be useful to know these are attr writes, not just regular writes? and what about xattrs?
Quoted 3 lines
fs/open.c +0
-0error = -EROFS ;if (IS_RDONLY (inode)) goto out;
Quoted 11 lines · Chris Wright
fs/attr.c int inode_setattr(struct inode * inode, · line 74
Quoted 6 lines · * Timothy R. Chavez ([email protected]) wrote:
fs/attr.c +2
-0@@ -69,6 +70,8 @@ int inode_setattr(struct inode * inode,unsigned int ia_valid = attr->ia_valid;int error =0 ; +audit_notify_watch (inode,MAY_WRITE ); +Hmm, this looks wrong. It should be in
notify_change, since I don't think there's a strict requirement that a fs callinode_setattr, which is just a commonly used helper.
Yeah I agree. This is being fixed for the RHEL kernel and I'll work it into the fsnotify patch as well.
Quoted 27 lines
Quoted 27 lines
fs/attr.c +0
-0if (ia_valid &ATTR_SIZE ) {if (attr->ia_size !=i_size_read (inode)) { error =vmtruncate (inode, attr->ia_size);diff -Nurp
linux-2.6.13-rc1-mm1/fs/namei.clinux-2.6.13-rc1-mm1~auditfs/fs/namei.cfs/namei.c 2011-07-05 02:01:36.000000000 -0500 renamed from fs/namei.c 2011-07-05 01:57:30.000000000 -0500 +4
-0@@ -243,6 +243,8 @@ int permission(struct inode *inode, int } + audit_notify_watch(inode, mask); + /* Ordinary permission routines do not understand MAY_APPEND. */ submask = mask & ~MAY_APPEND; if (inode->i_op && inode->i_op->permission)@@ -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))@@ -1188,6 +1192,8 @@ static inline int may_delete(struct inod
Quoted 6 lines
fs/namei.c 2011-07-05 02:01:36.000000000 -0500 static inline int may_delete(struct inod · line 1196
Quoted 4 lines
fs/namei.c 2011-07-05 02:01:36.000000000 -0500 renamed from fs/namei.c 2011-07-05 01:57:30.000000000 -0500 +2
-0BUG_ON(victim->d_parent->d_inode != dir); + audit_notify_watch(victim->d_inode, MAY_WRITE); +I forget why you need two separate notifications on delete?
The hook in may_delete() is actually not obvious at first glance. It was placed there to consolidate the notification. This notification is typically of the form, "Hey some one is about to unlink/rmdir/rename me!"
You'll notice that this is for the "victim" dentry and...
Quoted 4 lines
fs/namei.c 2011-07-05 02:01:36.000000000 -0500 static inline int may_delete(struct inod · line 1199
Quoted 4 lines
fs/namei.c 2011-07-05 02:01:36.000000000 -0500 renamed from fs/namei.c 2011-07-05 01:57:30.000000000 -0500 +0
-0error = permission(dir,MAY_WRITE | MAY_EXEC, NULL); if (error) return error;
...We'll (by virtue of hooking permission() receive a notification on our parent).
Quoted 9 lines
fs/namei.c 2011-07-05 02:01:36.000000000 -0500 int vfs_create(struct inode *dir, struct · line 1321
Quoted 5 lines
fs/namei.c 2011-07-05 02:01:36.000000000 -0500 renamed from fs/namei.c 2011-07-05 01:57:30.000000000 -0500 +1
-0@@ -1312,6 +1318,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);And on create. And, as I've mentioned, these bits could be collapsed with fsnotify hooks. I'll split that off from inotify, perhaps you can work against that...
Yes, I agree.
Quoted 36 lines
Quoted 36 lines
fs/namei.c 2011-07-05 02:01:36.000000000 -0500 renamed from fs/namei.c 2011-07-05 01:57:30.000000000 -0500 +4
-0fsnotify_create(dir, dentry->d_name.name); security_inode_post_create(dir, dentry, mode); }@@ -1637,6 +1644,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); }@@ -1710,6 +1718,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); }@@ -1951,6 +1960,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); }@@ -2024,6 +2034,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); }
Quoted 8 lines
fs/namei.c 2011-07-05 02:01:36.000000000 -0500 static int vfs_rename_dir(struct inode * · line 2161
Quoted 5 lines
fs/namei.c 2011-07-05 02:01:36.000000000 -0500 renamed from fs/namei.c 2011-07-05 01:57:30.000000000 -0500 +1
-0@@ -2147,6 +2158,7 @@ static int vfs_rename_dir(struct inode * } if (!error) { d_move(old_dentry,new_dentry); + audit_notify_watch(old_dentry->d_inode, MAY_WRITE);Don't you get two identical notifications here (one from permission, and one from here)? And where's the update for the new?
Nope... because permission() uses old_dentry before the d_move() and this notification occurs after the d_move()
Quoted 11 lines
fs/namei.c 2011-07-05 02:01:36.000000000 -0500 static int vfs_rename_other(struct inode · line 2190
Quoted 9 lines
fs/namei.c 2011-07-05 02:01:36.000000000 -0500 renamed from fs/namei.c 2011-07-05 01:57:30.000000000 -0500 +1
-0security_inode_post_rename(old_dir, old_dentry, new_dir, new_dentry); }@@ -2175,6 +2187,7 @@ static int vfs_rename_other(struct inode /* 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);Same here?
Same deal.
Quoted 17 lines
Quoted 17 lines
fs/namei.c 2011-07-05 02:01:36.000000000 -0500 renamed from fs/namei.c 2011-07-05 01:57:30.000000000 -0500 +0
-0security_inode_post_rename(old_dir, old_dentry, new_dir, new_dentry); } if (target)diff -Nurp
linux-2.6.13-rc1-mm1/fs/open.clinux-2.6.13-rc1-mm1~auditfs/fs/open.cfs/open.c 2011-07-05 02:01:36.000000000 -0500 renamed from fs/open.c 2011-07-05 01:57:31.000000000 -0500 +1
-0@@ -25,6 +25,7 @@ #include <linux/pagemap.h> #include <linux/syscalls.h> #include <linux/rcupdate.h> +#include <linux/audit.h> #include <asm/unistd.h>@@ -608,6 +609,8 @@ asmlinkage long sys_fchmod(unsigned int
Quoted 7 lines
fs/open.c 2011-07-05 02:01:36.000000000 -0500 asmlinkage long sys_fchmod(unsigned int · line 613
Quoted 4 lines
fs/open.c 2011-07-05 02:01:36.000000000 -0500 renamed from fs/open.c 2011-07-05 01:57:31.000000000 -0500 +2
-0dentry = file->f_dentry; inode = dentry->d_inode; + + audit_notify_watch(inode, MAY_WRITE);looks like you'll get two updates for these, this should be done in
notify_change.
Agreed.
Quoted 12 lines
fs/open.c 2011-07-05 02:01:36.000000000 -0500 asmlinkage long sys_chmod(const char __u · line 647
Quoted 9 lines
fs/open.c 2011-07-05 02:01:36.000000000 -0500 renamed from fs/open.c 2011-07-05 01:57:31.000000000 -0500 +2
-0err = -EROFS; if (IS_RDONLY(inode))@@ -640,6 +643,8 @@ asmlinkage long sys_chmod(const char __u if (error) goto out; inode = nd.dentry->d_inode; + + audit_notify_watch(inode, MAY_WRITE);ditto
Quoted 10 lines
fs/open.c 2011-07-05 02:01:36.000000000 -0500 static int chown_common(struct dentry * · line 682
Quoted 7 lines
fs/open.c 2011-07-05 02:01:36.000000000 -0500 renamed from fs/open.c 2011-07-05 01:57:31.000000000 -0500 +1
-0error = -EROFS; if (IS_RDONLY(inode))@@ -674,6 +679,7 @@ static int chown_common(struct dentry * printk(KERN_ERR "chown_common: NULL inode\n"); goto out; } + audit_notify_watch(inode, MAY_WRITE);ditto, and wouldn't it be useful to know these are attr writes, not just regular writes? and what about xattrs?
Yes the use of MAY_WRITE is a bit fickle. The MAY_* macros are used to determine what should be filtered out. Thus, if we're only interest in an access that "reads" we filter out all the notifications that "write"... But with regards to attributes, this is flawed (as you'll get a MAY_WRITE for fstat.. but we've already determiend that where I was placing the hooks was incorrect)
Quoted 5 lines
fs/open.c 2011-07-05 02:01:36.000000000 -0500 static int chown_common(struct dentry * · line 685
Quoted 4 lines
fs/open.c 2011-07-05 02:01:36.000000000 -0500 renamed from fs/open.c 2011-07-05 01:57:31.000000000 -0500 +0
-0error = -EROFS; if (IS_RDONLY(inode)) goto out;-
Thanks Chris. I'll look at the fsnotify stuff this weekend.
-tim
Quoted 8 lines
Some notable implementation details are as follows:
* struct inode is _not_ extended to associate audit data with the inode. A hash table is used in which the inode is hashed to retrieve its audit data. We know if an inode has audit data if
I_AUDIThas been turned on ininode->i_state.
why is this? It would be a very logical thing to store this stuff inside the inode. It sounds like a bad design to keep per inode data out of the inode. (if you're concerned about taking a lot of space, put a pointer to a kmalloc()'d piece of memory into the inode instead). A hash is just, well, odd for this.
Quoted 4 lines
* Inodes with audit data are implicitly pinned in memory when
I_AUDITis turned on ininode->i_state. This prevents an auditable incore inode from being pushed out of the icache, preserving auditability even under memory pressures.
sounds like inotify then...
Quoted 5 lines · Arjan van de Ven
why is this? It would be a very logical thing to store this stuff inside the inode. It sounds like a bad design to keep per inode data out of the inode. (if you're concerned about taking a lot of space, put a pointer to a kmalloc()'d piece of memory into the inode instead). A hash is just, well, odd for this.
There are _very_ few of these; it's very dubious whether it'd be worth bloating the inode for them. The use of I_AUDIT also serves to pin the inode in icache, and we'd need something like that even if we _weren't_ using it as a marker for the hash table.
Signature · 1 line
dwmw2