/proc Security Hooks
Subsystems (2)
PATCH 1/1 /proc Security Hooks
Add two LSM hooks for limiting access to the proc file system.
security_proc_task() defines the visibility of tasks in /proc.
security_proc_generic() lets the LSM define who will see "generic" proc entries (see fs/proc/generic.c).
This patch attempts to unify duplicated code found in modules like Linux VServer.
--- fs/proc/base.c | 9 +++++++++ fs/proc/generic.c | 15 +++++++++++++++ include/linux/security.h | 46 ++++++++++++++++++++++++++++++++++++++++++++++ security/Kconfig | 8 ++++++++ security/dummy.c | 16 ++++++++++++++++ 5 files changed, 94 insertions(+), 0 deletions(-)
fs/proc/base.c +9 -0
@@ -2317,6 +2317,10 @@ int proc_pid_readdir(struct file * filp, void * dirent, filldir_t filldir) put_task_struct(task), task = next_tgid(tgid + 1)) { tgid = task->pid; filp->f_pos = tgid +TGID_OFFSET ; +#ifdef CONFIG_SECURITY_PROC +if (security_proc_task (task) !=0 ) +continue ; +#endif if (proc_pid_fill_cache (filp, dirent, filldir, task, tgid) <0 ) {put_task_struct (task); goto out;
@@ -2453,6 +2457,11 @@ static struct dentry *proc_task_lookup(struct inode *dir, struct dentry * dentry goto out;if (leader->tgid != task->tgid) goto out_drop_task; +#ifdef CONFIG_SECURITY_PROC + result =ERR_PTR (security_proc_task (task)); +if (IS_ERR (result)) + goto out_drop_task; +#endif result =proc_task_instantiate (dir, dentry, task,NULL ); out_drop_task:
fs/proc/generic.c +15 -0
@@ -21,6 +21,9 @@#include <linux/bitops.h> #include <linux/spinlock.h> #include <linux/completion.h> +#ifdef CONFIG_SECURITY_PROC +#include <linux/security.h> +#endif #include <asm/uaccess.h> #include "internal.h"
@@ -400,6 +403,11 @@ struct dentry *proc_lookup(struct inode * dir, struct dentry *dentry, struct namunsigned int ino = de->low_ino;de_get (de); +#ifdef CONFIG_SECURITY_PROC + error =security_proc_generic (de); +if (error !=0 ) +break ; +#endif spin_unlock (&proc_subdir_lock); error = -EINVAL ; inode =proc_get_inode (dir->i_sb, ino, de);
@@ -483,6 +491,10 @@ int proc_readdir(struct file * filp,/* filldir passes info to user space */ de_get (de); +#ifdef CONFIG_SECURITY_PROC +if (security_proc_generic (de) !=0 ) + goto skip; +#endif spin_unlock (&proc_subdir_lock);if (filldir (dirent, de->name, de->namelen, filp->f_pos, de->low_ino, de->mode >>12 ) <0 ) {
@@ -490,6 +502,9 @@ int proc_readdir(struct file * filp, goto out; }spin_lock (&proc_subdir_lock); +#ifdef CONFIG_SECURITY_PROC + skip: +#endif filp->f_pos++; next = de->next;de_put (de);
include/linux/security.h +46 -0
69 lines not on this page · raw message
security/Kconfig +8 -0
@@ -73,6 +73,14 @@ config SECURITY_NETWORK_XFRM IPSec. If you are unsure how to answer this question, answer N. +config SECURITY_PROC + bool "/proc Security Hooks" + depends on PROC_FS && SECURITY + help + This enables the /proc security hooks. + These allow an LSM to hide nodes in the proc filesystem. + If you are unsure how to answer this question, answer N. + config SECURITY_CAPABILITIES tristate "Default Linux Capabilities" depends on SECURITY
security/dummy.c +16 -0
@@ -949,6 +949,18 @@ static inline int dummy_key_permission(key_ref_t key_ref, }#endif /* CONFIG_KEYS */ +#ifdef CONFIG_SECURITY_PROC +static int dummy_proc_task (struct task_struct *task) +{ +return 0 ; +} + +static int dummy_proc_generic (struct proc_dir_entry *de) +{ +return 0 ; +} +#endif +struct security_operations dummy_security_ops;#define set_to_dummy_if_null (ops, function) \
@@ -1131,6 +1143,10 @@ void security_fixup_ops (struct security_operations *ops)set_to_dummy_if_null (ops, key_free);set_to_dummy_if_null (ops, key_permission);#endif /* CONFIG_KEYS */ +#ifdef CONFIG_SECURITY_PROC +set_to_dummy_if_null (ops, proc_task); +set_to_dummy_if_null (ops, proc_generic); +#endif }
Quoted 9 lines · On Tue, 16 Oct 2007 21:38:50 +0200 Max Kellermann <[email protected]> wrote:
Add two LSM hooks for limiting access to the proc file system.
security_proc_task()defines the visibility of tasks in /proc.
security_proc_generic()lets the LSM define who will see "generic" proc entries (seefs/proc/generic.c).This patch attempts to unify duplicated code found in modules like Linux VServer.
can you please merge this patch only when you also merge the first user of it? That's the only way we can keep the LSM hooks sane... is to see them in thew conect of a user.
fs/proc/base.c int proc_pid_readdir(struct file * filp, void * dirent, filldir_t filldir) · line 2323
Quoted 4 lines
fs/proc/base.c +4
-0+#ifdef CONFIG_SECURITY_PROC +if (security_proc_task (task) !=0 ) +continue ; +#endif
please don't use an ifdef like this; just make security_proc_task() be a define to 0 in the header for that CONFIG_ ..
In addition, why is this a separate config option? LSM should really only be one big switch... microswitches like this don't make any sense.
fs/proc/base.c static struct dentry *proc_task_lookup(struct · line 2464
Quoted 12 lines
if (proc_pid_fill_cache(filp, dirent, filldir, task,
tgid) < 0) {put_task_struct(task);fs/proc/base.c +5
-0goto out;@@ -2453,6 +2457,11 @@ static struct dentry *proc_task_lookup(struct inode *dir, struct dentry * dentry goto out;if (leader->tgid != task->tgid) goto out_drop_task; +#ifdef CONFIG_SECURITY_PROC + result =ERR_PTR (security_proc_task (task)); +if (IS_ERR (result)) + goto out_drop_task; +#endif
same here
fs/proc/generic.c line 26
Quoted 3 lines
fs/proc/generic.c +3
-0+#ifdef CONFIG_SECURITY_PROC +#include <linux/security.h> +#endif
don't put ifdefs around includes please.... if anything the ifdef should be inside the header
fs/proc/generic.c struct dentry *proc_lookup(struct inode * dir, struct dentry *dentry, struct nam · line 410
Quoted 5 lines
fs/proc/generic.c +5
-0+#ifdef CONFIG_SECURITY_PROC + error =security_proc_generic (de); +if (error !=0 ) +break ; +#endif
and this ifdef should die too
fs/proc/generic.c int proc_readdir(struct file * filp, · line 497
Quoted 11 lines
fs/proc/generic.c +0
-0spin_unlock (&proc_subdir_lock); error = -EINVAL ; inode = proc_get_inode(dir->i_sb,ino, de); @@ -483,6 +491,10 @@ int proc_readdir(struct file * filp,
/* filldir passes info to user space
*/de_get(de);fs/proc/generic.c +4
-0+#ifdef CONFIG_SECURITY_PROC +if (security_proc_generic (de) !=0 ) + goto skip; +#endif
as does this one... but the goto looks horrid to me
Quoted 10 lines · Arjan van de Ven
Quoted 2 lines · On Tue, 16 Oct 2007 21:38:50 +0200 Max Kellermann <[email protected]> wrote:
This patch attempts to unify duplicated code found in modules like Linux VServer.
can you please merge this patch only when you also merge the first user
of it? That's the only way we can keep the LSM hooks sane... is to see
them in thew conect of a user.
I wrote a module which uses this, but it's non-free and only used on my employer's servers. But I could have a closer look at the Vserver code and try to make it use my patch.
Quoted 11 lines
fs/proc/base.c int proc_pid_readdir(struct file * filp, void * dirent, filldir_t filldir) · line 2323
Quoted 4 lines
fs/proc/base.c +4
-0+#ifdef CONFIG_SECURITY_PROC +if (security_proc_task (task) !=0 ) +continue ; +#endif please don't use an ifdef like this; just make
security_proc_task()be
a define to 0 in the header for thatCONFIG_..
In addition, why is this a separate config option? LSM should really only be one big switch... microswitches like this don't make any sense.
Right, I initially wrote this patch some time ago when linux/security.h didn't have an "#ifdef CONFIG_SECURITY". I'll adapt that.
Quoted 7 lines
Quoted 5 lines
fs/proc/generic.c +3
-0+#ifdef CONFIG_SECURITY_PROC +if (security_proc_generic (de) !=0 ) +goto skip;
+#endifas does this one... but the goto looks horrid to me
I'm all against gotos, but seeing gotos all over the kernel, and my code being in an #ifdef, this one goto looked "normal" to me. You're right, I should change it.
Max
Quoted 12 lines · On Wed, 17 Oct 2007 07:13:57 +0200 Max Kellermann <[email protected]> wrote:
Quoted 10 lines · Arjan van de Ven
Quoted 2 lines · On Tue, 16 Oct 2007 21:38:50 +0200 Max Kellermann <[email protected]> wrote:
This patch attempts to unify duplicated code found in modules like Linux VServer.
can you please merge this patch only when you also merge the first user
of it? That's the only way we can keep the LSM hooks sane... is to see
them in thew conect of a user.
Quoted 3 lines
I wrote a module which uses this, but it's non-free and only used on my employer's servers. But I could have a closer look at the Vserver code and try to make it use my patch.
doesn't sound like something we'd want overhead for in the generic kernel....
Quoted 6 lines · Max Kellermann
Quoted 5 lines · Arjan van de Ven
Quoted 2 lines · On Tue, 16 Oct 2007 21:38:50 +0200 Max Kellermann <[email protected]> wrote:
This patch attempts to unify duplicated code found in modules like Linux VServer.
Quoted 8 lines
Quoted 5 lines
can you please merge this patch only when you also merge the first user
of it? That's the only way we can keep the LSM hooks sane... is to see
them in thew conect of a user.I wrote a module which uses this, but it's non-free and only used on my employer's servers.
How can a non-GPL module call the security_register() function?
Anyway, we don't/can't add hooks to the kernel for non-GPL code, that's not allowed.
thanks,
greg k-h
Quoted 9 lines · Max Kellermann
Add two LSM hooks for limiting access to the proc file system.
security_proc_task()defines the visibility of tasks in /proc.
security_proc_generic()lets the LSM define who will see "generic" proc entries (seefs/proc/generic.c).This patch attempts to unify duplicated code found in modules like Linux VServer.
Are you aware of the pid namespace patches currently in -mm? If your use for the task hiding hooks is to emulate virtual servers as with vserver/openvz, the pid namespaces should fill your need.
-serge
Quoted 218 lines
Signed-off-by Max Kellermann <[email protected]>--- fs/proc/base.c | 9 +++++++++ fs/proc/generic.c | 15 +++++++++++++++ include/linux/security.h | 46 ++++++++++++++++++++++++++++++++++++++++++++++ security/Kconfig | 8 ++++++++ security/dummy.c | 16 ++++++++++++++++ 5 files changed, 94 insertions(+), 0 deletions(-)fs/proc/base.c +9
-0@@ -2317,6 +2317,10 @@ int proc_pid_readdir(struct file * filp, void * dirent, filldir_t filldir) put_task_struct(task), task = next_tgid(tgid + 1)) { tgid = task->pid; filp->f_pos = tgid +TGID_OFFSET ; +#ifdef CONFIG_SECURITY_PROC +if (security_proc_task (task) !=0 ) +continue ; +#endif if (proc_pid_fill_cache (filp, dirent, filldir, task, tgid) <0 ) {put_task_struct (task); goto out;@@ -2453,6 +2457,11 @@ static struct dentry *proc_task_lookup(struct inode *dir, struct dentry * dentry goto out;if (leader->tgid != task->tgid) goto out_drop_task; +#ifdef CONFIG_SECURITY_PROC + result =ERR_PTR (security_proc_task (task)); +if (IS_ERR (result)) + goto out_drop_task; +#endif result =proc_task_instantiate (dir, dentry, task,NULL ); out_drop_task:fs/proc/generic.c +15
-0@@ -21,6 +21,9 @@#include <linux/bitops.h> #include <linux/spinlock.h> #include <linux/completion.h> +#ifdef CONFIG_SECURITY_PROC +#include <linux/security.h> +#endif #include <asm/uaccess.h> #include "internal.h" @@ -400,6 +403,11 @@ struct dentry *proc_lookup(struct inode * dir, struct dentry *dentry, struct namunsigned int ino = de->low_ino;de_get (de); +#ifdef CONFIG_SECURITY_PROC + error =security_proc_generic (de); +if (error !=0 ) +break ; +#endif spin_unlock (&proc_subdir_lock); error = -EINVAL ; inode =proc_get_inode (dir->i_sb, ino, de);@@ -483,6 +491,10 @@ int proc_readdir(struct file * filp,/* filldir passes info to user space */ de_get (de); +#ifdef CONFIG_SECURITY_PROC +if (security_proc_generic (de) !=0 ) + goto skip; +#endif spin_unlock (&proc_subdir_lock);if (filldir (dirent, de->name, de->namelen, filp->f_pos, de->low_ino, de->mode >>12 ) <0 ) {@@ -490,6 +502,9 @@ int proc_readdir(struct file * filp, goto out; }spin_lock (&proc_subdir_lock); +#ifdef CONFIG_SECURITY_PROC + skip: +#endif filp->f_pos++; next = de->next;de_put (de);include/linux/security.h +46
-0@@ -68,6 +68,10 @@ struct xfrm_policy;struct xfrm_state ;struct xfrm_user_sec_ctx ; +#ifdef CONFIG_SECURITY_PROC +struct proc_dir_entry ; +#endif +extern int cap_netlink_send (struct sock *sk,struct sk_buff *skb);extern int cap_netlink_recv (struct sk_buff *skb,int cap);@@ -915,6 +919,17 @@ struct request_sock;* Return 1 if permission granted, 0 if permission denied and -ve it the * normal permissions model should be effected. * +* Security hooks affecting proc visibility +* +* @proc_task: +* Determines whether a task is visible. +* @task the requested task +* Return 0 if task is visible, -ENOENT if not, or -errno on other errors +* @proc_generic: +* Determines whether a generic proc entry is visible +* @de the requested proc_dir_entry structure +* Return 0 if entry is visible, -ENOENT if not, or -errno on other errors +* * Security hooks affecting all System V IPC operations. * * @ipc_permission: @@ -1399,6 +1414,11 @@ struct security_operations {#endif /* CONFIG_KEYS */ +#ifdef CONFIG_SECURITY_PROC +int (*proc_task)(struct task_struct *task); +int (*proc_generic)(struct proc_dir_entry *de); +#endif + };/* global variables */ @@ -3314,5 +3334,31 @@ static inline int security_key_permission(key_ref_t key_ref,#endif #endif /* CONFIG_KEYS */ +#ifdef CONFIG_SECURITY_PROC +#ifdef CONFIG_SECURITY +static inline int security_proc_task (struct task_struct *task) +{ +return security_ops->proc_task (task); +} + +static inline int security_proc_generic (struct proc_dir_entry *de) +{ +return security_ops->proc_generic (de); +} + +#else + +static inline int security_proc_task (struct task_struct *task) +{ +return 0 ; +} + +static inline int security_proc_generic (struct proc_dir_entry *de) +{ +return 0 ; +} +#endif +#endif +#endif /* ! __LINUX_SECURITY_H */ security/Kconfig +8
-0@@ -73,6 +73,14 @@ config SECURITY_NETWORK_XFRM IPSec. If you are unsure how to answer this question, answer N. +config SECURITY_PROC + bool "/proc Security Hooks" + depends on PROC_FS && SECURITY + help + This enables the /proc security hooks. + These allow an LSM to hide nodes in the proc filesystem. + If you are unsure how to answer this question, answer N. + config SECURITY_CAPABILITIES tristate "Default Linux Capabilities" depends on SECURITYsecurity/dummy.c +16
-1@@ -949,6 +949,18 @@ static inline int dummy_key_permission(key_ref_t key_ref, }#endif /* CONFIG_KEYS */ +#ifdef CONFIG_SECURITY_PROC +static int dummy_proc_task (struct task_struct *task) +{ +return 0 ; +} + +static int dummy_proc_generic (struct proc_dir_entry *de) +{ +return 0 ; +} +#endif +struct security_operations dummy_security_ops;#define set_to_dummy_if_null (ops, function) \@@ -1131,6 +1143,10 @@ void security_fixup_ops (struct security_operations *ops)set_to_dummy_if_null (ops, key_free);set_to_dummy_if_null (ops, key_permission);#endif /* CONFIG_KEYS */ +#ifdef CONFIG_SECURITY_PROC +set_to_dummy_if_null (ops, proc_task); +set_to_dummy_if_null (ops, proc_generic); +#endif } -To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html