From 0a300be6d5be8f66cd96609334710c268d0bfdce Mon Sep 17 00:00:00 2001 From: Eric Paris Date: Tue, 3 Jan 2012 14:23:08 -0500 Subject: audit: remove task argument to audit_set_loginuid The function always deals with current. Don't expose an option pretending one can use it for something. You can't. Signed-off-by: Eric Paris --- fs/proc/base.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'fs') diff --git a/fs/proc/base.c b/fs/proc/base.c index 8173dfd89cb2..e3cbebbabebd 100644 --- a/fs/proc/base.c +++ b/fs/proc/base.c @@ -1228,7 +1228,7 @@ static ssize_t proc_loginuid_write(struct file * file, const char __user * buf, goto out_free_page; } - length = audit_set_loginuid(current, loginuid); + length = audit_set_loginuid(loginuid); if (likely(length == 0)) length = count; -- cgit v1.2.3 From 633b45454503489209b0d9a45f9e3cd1b852c614 Mon Sep 17 00:00:00 2001 From: Eric Paris Date: Tue, 3 Jan 2012 14:23:08 -0500 Subject: audit: only allow tasks to set their loginuid if it is -1 At the moment we allow tasks to set their loginuid if they have CAP_AUDIT_CONTROL. In reality we want tasks to set the loginuid when they log in and it be impossible to ever reset. We had to make it mutable even after it was once set (with the CAP) because on update and admin might have to restart sshd. Now sshd would get his loginuid and the next user which logged in using ssh would not be able to set his loginuid. Systemd has changed how userspace works and allowed us to make the kernel work the way it should. With systemd users (even admins) are not supposed to restart services directly. The system will restart the service for them. Thus since systemd is going to loginuid==-1, sshd would get -1, and sshd would be allowed to set a new loginuid without special permissions. If an admin in this system were to manually start an sshd he is inserting himself into the system chain of trust and thus, logically, it's his loginuid that should be used! Since we have old systems I make this a Kconfig option. Signed-off-by: Eric Paris --- fs/proc/base.c | 3 --- init/Kconfig | 14 ++++++++++++++ kernel/auditsc.c | 11 ++++++++++- 3 files changed, 24 insertions(+), 4 deletions(-) (limited to 'fs') diff --git a/fs/proc/base.c b/fs/proc/base.c index e3cbebbabebd..482df23036b5 100644 --- a/fs/proc/base.c +++ b/fs/proc/base.c @@ -1197,9 +1197,6 @@ static ssize_t proc_loginuid_write(struct file * file, const char __user * buf, ssize_t length; uid_t loginuid; - if (!capable(CAP_AUDIT_CONTROL)) - return -EPERM; - rcu_read_lock(); if (current != pid_task(proc_pid(inode), PIDTYPE_PID)) { rcu_read_unlock(); diff --git a/init/Kconfig b/init/Kconfig index a075765d5fbe..5ad8b775f2ac 100644 --- a/init/Kconfig +++ b/init/Kconfig @@ -372,6 +372,20 @@ config AUDIT_TREE depends on AUDITSYSCALL select FSNOTIFY +config AUDIT_LOGINUID_IMMUTABLE + bool "Make audit loginuid immutable" + depends on AUDIT + help + The config option toggles if a task setting it's loginuid requires + CAP_SYS_AUDITCONTROL or if that task should require no special permissions + but should instead only allow setting its loginuid if it was never + previously set. On systems which use systemd or a similar central + process to restart login services this should be set to true. On older + systems in which an admin would typically have to directly stop and + start processes this should be set to false. Setting this to true allows + one to drop potentially dangerous capabilites from the login tasks, + but may not be backwards compatible with older init systems. + source "kernel/irq/Kconfig" menu "RCU Subsystem" diff --git a/kernel/auditsc.c b/kernel/auditsc.c index 9d6dd7d869c0..bd084a13c719 100644 --- a/kernel/auditsc.c +++ b/kernel/auditsc.c @@ -2173,9 +2173,18 @@ static atomic_t session_id = ATOMIC_INIT(0); int audit_set_loginuid(uid_t loginuid) { struct task_struct *task = current; - unsigned int sessionid = atomic_inc_return(&session_id); struct audit_context *context = task->audit_context; + unsigned int sessionid; + +#ifdef CONFIG_AUDIT_LOGINUID_IMMUTABLE + if (task->loginuid != -1) + return -EPERM; +#else /* CONFIG_AUDIT_LOGINUID_IMMUTABLE */ + if (!capable(CAP_AUDIT_CONTROL)) + return -EPERM; +#endif /* CONFIG_AUDIT_LOGINUID_IMMUTABLE */ + sessionid = atomic_inc_return(&session_id); if (context && context->in_syscall) { struct audit_buffer *ab; -- cgit v1.2.3 From 4043cde8ecf7f7d880eb1133c201a3d392fd68c3 Mon Sep 17 00:00:00 2001 From: Eric Paris Date: Tue, 3 Jan 2012 14:23:08 -0500 Subject: audit: do not call audit_getname on error Just a code cleanup really. We don't need to make a function call just for it to return on error. This also makes the VFS function even easier to follow and removes a conditional on a hot path. Signed-off-by: Eric Paris --- fs/namei.c | 28 +++++++++++++--------------- kernel/auditsc.c | 3 --- 2 files changed, 13 insertions(+), 18 deletions(-) (limited to 'fs') diff --git a/fs/namei.c b/fs/namei.c index c283a1ec008e..208c6aa4a989 100644 --- a/fs/namei.c +++ b/fs/namei.c @@ -140,21 +140,19 @@ static int do_getname(const char __user *filename, char *page) static char *getname_flags(const char __user *filename, int flags, int *empty) { - char *tmp, *result; - - result = ERR_PTR(-ENOMEM); - tmp = __getname(); - if (tmp) { - int retval = do_getname(filename, tmp); - - result = tmp; - if (retval < 0) { - if (retval == -ENOENT && empty) - *empty = 1; - if (retval != -ENOENT || !(flags & LOOKUP_EMPTY)) { - __putname(tmp); - result = ERR_PTR(retval); - } + char *result = __getname(); + int retval; + + if (!result) + return ERR_PTR(-ENOMEM); + + retval = do_getname(filename, result); + if (retval < 0) { + if (retval == -ENOENT && empty) + *empty = 1; + if (retval != -ENOENT || !(flags & LOOKUP_EMPTY)) { + __putname(result); + return ERR_PTR(retval); } } audit_getname(result); diff --git a/kernel/auditsc.c b/kernel/auditsc.c index bd084a13c719..9161e70a4379 100644 --- a/kernel/auditsc.c +++ b/kernel/auditsc.c @@ -1913,9 +1913,6 @@ void __audit_getname(const char *name) struct audit_context *context = current->audit_context; struct audit_names *n; - if (IS_ERR(name) || !name) - return; - if (!context->in_syscall) { #if AUDIT_DEBUG == 2 printk(KERN_ERR "%s:%d(:%d): ignoring getname(%p)\n", -- cgit v1.2.3