Olaf Dietsche <[email protected]> wrote:
>
> Hi Andrew,
>
> Can you please include this patch in -mm, to give it wider testing?
I doubt if it'll get a lot of runtime testing.
> Accessfs is a permission managing filesystem. It allows to control
> access to system resources, based on file permissions. It also
> includes two modules. One module allows granting capabilities based
> on user-/groupid. The second module allows to grant access to lower
> numbered IP ports based on user-/groupid.
>
It seems to be network-centric?
Do these capabilities really need to be implemented via a brand-new
security infrastructure, rather then by enhancing the existing one(s)?
> + To use this option, you need to mount the access file system
> + and do a chown on the appropriate ports:
> +
> + # mount -t accessfs none /proc/access
> + # chown www /proc/access/net/ip/bind/80
> + # chown mail /proc/access/net/ip/bind/25
Documenting a feature in Kconfig is a bit odd. I assume proper
Documentation is forthcoming?
> + */
> +
> +#include <linux/accessfs_fs.h>
> +#include <linux/init.h>
> +#include <linux/module.h>
> +#include <linux/security.h>
> +
> +static struct access_attr caps[29];
caps[ARRAY_SIZE(names)]
should work.
> +static const char *names[] = {
> + "chown",
> + "dac_override",
> + "dac_read_search",
> + "fowner",
> + "fsetid",
> + "kill",
> + "setgid",
> + "setuid",
> + "setpcap",
> + "linux_immutable",
> + "net_bind_service",
> + "net_broadcast",
> + "net_admin",
> + "net_raw",
> + "ipc_lock",
> + "ipc_owner",
> + "sys_module",
> + "sys_rawio",
> + "sys_chroot",
> + "sys_ptrace",
> + "sys_pacct",
> + "sys_admin",
> + "sys_boot",
> + "sys_nice",
> + "sys_resource",
> + "sys_time",
> + "sys_tty_config",
> + "mknod",
> + "lease"
> +};
> +
>
> +static void unregister_capabilities(struct accessfs_direntry *dir, int n)
> +{
> + int i;
> + for (i = 0; i < n; ++i) {
> + accessfs_unregister(dir, names[i]);
> + }
> +}
Unneeded braces.
> +static int __init init_capabilities(void)
> +{
> + struct accessfs_direntry *dir;
> + int i, err;
> + dir = accessfs_make_dirpath("capabilities");
> + if (dir == 0)
> + return -ENOTDIR;
> +
> + for (i = 0; i < sizeof(caps) / sizeof(caps[0]); ++i) {
ARRAY_SIZE() (lots of instances)
> +static DECLARE_MUTEX(accessfs_sem);
Please use a `struct mutex'.
> +
> +static inline void accessfs_readdir_aux(struct file *filp, struct accessfs_direntry *dir, int start, void *dirent, filldir_t filldir)
> +{
> + struct list_head *list;
> + int i;
> +
> + list = dir->children.next;
> + for (i = 2; i < start && list != &dir->children; ++i)
> + list = list->next;
> +
> + while (list != &dir->children) {
> + struct accessfs_entry *de;
> + de = list_entry(list, struct accessfs_entry, siblings);
> + if (filldir(dirent, de->name, strlen(de->name), filp->f_pos, de->ino, DT_UNKNOWN) < 0)
> + break;
> +
> + ++filp->f_pos;
> + list = list->next;
> + }
> +}
Use standard list accessors?
Please fit code into 80 cols.
> +static int accessfs_readdir(struct file *filp, void *dirent, filldir_t filldir)
> +{
> + int i;
> + struct dentry *dentry = filp->f_dentry;
> + struct accessfs_direntry *dir;
> +
> + i = filp->f_pos;
> + switch (i) {
> + case 0:
> + if (filldir(dirent, ".", 1, i, dentry->d_inode->i_ino, DT_DIR) < 0)
> + break;
> +
> + ++i;
> + ++filp->f_pos;
> + /* NO break; */
> + case 1:
> + if (filldir(dirent, "..", 2, i, dentry->d_parent->d_inode->i_ino, DT_DIR) < 0)
> + break;
> +
> + ++i;
> + ++filp->f_pos;
> + /* NO break; */
> + default:
> + down(&accessfs_sem);
> + dir = (struct accessfs_direntry *) dentry->d_inode->u.generic_ip;
Unneeded typecast.
> + accessfs_readdir_aux(filp, dir, i, dirent, filldir);
> + up(&accessfs_sem);
> + break;
> + }
> +
> + return 0;
> +}
> +
> +static void accessfs_init_inode(struct inode *inode, struct accessfs_entry *pe)
> +{
> + static const struct timespec epoch = {0, 0};
Unneeded initialiser (although it does make things clearer)
> +
> +static struct accessfs_direntry *accessfs_mkdir(struct accessfs_direntry *parent, const char *name, size_t len)
> +{
> + int err;
> + struct accessfs_direntry *dir;
> + dir = kmalloc(sizeof(struct accessfs_direntry), GFP_KERNEL);
> + if (dir == NULL)
> + return NULL;
> +
> + dir->parent = parent;
> + INIT_LIST_HEAD(&dir->children);
> + err = accessfs_node_init(parent, &dir->node, name, len, &dir->attr, S_IFDIR | 0755);
> + if (err) {
> + kfree(dir);
> + dir = 0;
> + }
> +
> + return dir;
> +}
Again, painful to read in 80-cols.
> +struct accessfs_direntry *accessfs_make_dirpath(const char *name)
> +{
> + struct accessfs_direntry *dir = &accessfs_rootdir;
> + const char *slash;
> + down(&accessfs_sem);
Shouldn't that lock be per-superblock?
> +static void accessfs_read_inode(struct inode *inode)
> +{
> + ino_t ino = inode->i_ino;
> + struct list_head *list;
> + down(&accessfs_sem);
> + list_for_each(list, &hash) {
> + struct accessfs_entry *pe;
> + pe = list_entry(list, struct accessfs_entry, hash);
> + if (pe->ino == ino) {
> + accessfs_init_inode(inode, pe);
> + break;
> + }
> + }
That's not a hash!
> +{
> + unregister_filesystem(&accessfs_fs_type);
> +
> +#ifdef CONFIG_PROC_FS
> + remove_proc_entry("access",&proc_root);
> +#endif
> +}
The CONFIG_PROC_FS ifdefs shouldn't be needed - we have stubs.
> +static int accessfs_ip6_prot_sock(struct socket *sock,
> + struct sockaddr *uaddr, int addr_len)
> +{
> +#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
That's a bit awkward, especially the CONFIG_IPV6_MODULE dependency. Is it
possible to just unconditionally compile this in?
> +static int __init init_ip(void)
> +{
> + struct accessfs_direntry *dir = accessfs_make_dirpath("net/ip/bind");
> + int i;
> + bind_to_port = kmalloc(max_prot_sock * sizeof(*bind_to_port), GFP_KERNEL);
> + if (bind_to_port == 0)
Use NULL to avoid sparse warnings.
> +
> +#if CONFIG_ACCESSFS_PROT_SOCK < PROT_SOCK
> +#define CONFIG_ACCESSFS_PROT_SOCK PROT_SOCK
> +#elseif CONFIG_ACCESSFS_PROT_SOCK > 65536
> +#define CONFIG_ACCESSFS_PROT_SOCK 65536
> +#endif
Please don't redefine CONFIG_ variables like this. I'd have expected the
compiler to have generated a warning about this, too.
> snum = ntohs(addr->sin_port);
> err = -EACCES;
> +#ifdef CONFIG_NET_HOOKS
> + if (net_ops->ip_prot_sock(sock, uaddr, addr_len))
> +#else
> if (snum && snum < PROT_SOCK && !capable(CAP_NET_BIND_SERVICE))
> +#endif
> goto out;
Maybe some wrapper which hides the above?
> /* We keep a pair of addresses. rcv_saddr is the one
> diff -urN a/net/ipv6/af_inet6.c b/net/ipv6/af_inet6.c
> --- a/net/ipv6/af_inet6.c Fri Jan 27 23:53:23 2006
> +++ b/net/ipv6/af_inet6.c Sat Jan 28 12:47:19 2006
> @@ -260,7 +260,11 @@
> return -EINVAL;
>
> snum = ntohs(addr->sin6_port);
> +#ifdef CONFIG_NET_HOOKS
> + if (net_ops->ip6_prot_sock(sock, uaddr, addr_len))
> +#else
> if (snum && snum < PROT_SOCK && !capable(CAP_NET_BIND_SERVICE))
> +#endif
> return -EACCES;
>
which could be used here as well.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
