Re: [PATCH] Filesystem capabilities support

[Jesper Juhl <jesper.juhl@gmail.com>]

On 7/16/05, Nicholas Hans Simmonds <nhstux@gmail.com> wrote:

While I'm not qualified to comment on the implementation I do have a
few small codingstyle comments :-)


> diff --git a/fs/read_write.c b/fs/read_write.c
> --- a/fs/read_write.c
> +++ b/fs/read_write.c
> @@ -14,6 +14,7 @@
>  #include <linux/security.h>
>  #include <linux/module.h>
>  #include <linux/syscalls.h>
> +#include <linux/xattr.h>
> 
>  #include <asm/uaccess.h>
>  #include <asm/unistd.h>
> @@ -303,6 +304,16 @@ ssize_t vfs_write(struct file *file, con
>                         else
>                                 ret = do_sync_write(file, buf, count, pos);
>                         if (ret > 0) {
> +#ifdef CONFIG_SECURITY_FS_CAPABILITIES
> +                               struct dentry *d = file->f_dentry;
> +                               if(d->d_inode->i_op && d->d_inode->i_op->

                                      if (d->d_inode->i_op ...

> +                                                               removexattr) {
> +                                       down(&d->d_inode->i_sem);
> +                                       d->d_inode->i_op->removexattr(d,
> +                                                               XATTR_CAP_SET);
> +                                       up(&d->d_inode->i_sem);
> +                               }
> +#endif /* CONFIG_SECURITY_FS_CAPABILITIES */
>                                 fsnotify_modify(file->f_dentry);
>                                 current->wchar += ret;
>                         }
> diff --git a/include/linux/capability.h b/include/linux/capability.h
> --- a/include/linux/capability.h
> +++ b/include/linux/capability.h
> @@ -39,7 +39,19 @@ typedef struct __user_cap_data_struct {
>          __u32 permitted;
>          __u32 inheritable;
>  } __user *cap_user_data_t;
> -
> +
> +struct cap_xattr_data {
> +       __u32 version;
> +       __u32 mask_effective;
> +       __u32 effective;
> +       __u32 mask_permitted;
> +       __u32 permitted;
> +       __u32 mask_inheritable;
> +       __u32 inheritable;
> +};
> +
> +#define XATTR_CAP_SET XATTR_SECURITY_PREFIX "cap_set"
> +
>  #ifdef __KERNEL__
> 
>  #include <linux/spinlock.h>
> diff --git a/security/Kconfig b/security/Kconfig
> --- a/security/Kconfig
> +++ b/security/Kconfig
> @@ -60,6 +60,13 @@ config SECURITY_CAPABILITIES
>           This enables the "default" Linux capabilities functionality.
>           If you are unsure how to answer this question, answer Y.
> 
> +config SECURITY_FS_CAPABILITIES
> +       bool "Filesystem Capabilities (EXPERIMENTAL)"
> +       depends on SECURITY && EXPERIMENTAL
> +       help
> +         This permits a process' capabilities to be set by an extended
> +         attribute in the security namespace (security.cap_set).
> +
>  config SECURITY_ROOTPLUG
>         tristate "Root Plug Support"
>         depends on USB && SECURITY
> diff --git a/security/commoncap.c b/security/commoncap.c
> --- a/security/commoncap.c
> +++ b/security/commoncap.c
> @@ -111,9 +111,15 @@ void cap_capset_set (struct task_struct
> 
>  int cap_bprm_set_security (struct linux_binprm *bprm)
>  {
> +#ifdef CONFIG_SECURITY_FS_CAPABILITIES
> +       ssize_t (*bprm_getxattr)(struct dentry *,const char *,void *,size_t);
> +       struct dentry *bprm_dentry;
> +       ssize_t ret;
> +       struct cap_xattr_data caps;
> +#endif /* CONFIG_SECURITY_FS_CAPABILITIES */
> +
>         /* Copied from fs/exec.c:prepare_binprm. */
> 
> -       /* We don't have VFS support for capabilities yet */
>         cap_clear (bprm->cap_inheritable);
>         cap_clear (bprm->cap_permitted);
>         cap_clear (bprm->cap_effective);
> @@ -134,6 +140,44 @@ int cap_bprm_set_security (struct linux_
>                 if (bprm->e_uid == 0)
>                         cap_set_full (bprm->cap_effective);
>         }
> +
> +#ifdef CONFIG_SECURITY_FS_CAPABILITIES
> +       /* Locate any VFS capabilities: */
> +
> +       bprm_dentry = bprm->file->f_dentry;
> +       if(!(bprm_dentry->d_inode->i_op &&

              if (!(bprm_dentry->d_inode->i_op ...

> +                               bprm_dentry->d_inode->i_op->getxattr))
> +               return 0;
> +       bprm_getxattr = bprm_dentry->d_inode->i_op->getxattr;
> +
> +       down(&bprm_dentry->d_inode->i_sem);
> +       ret = bprm_getxattr(bprm_dentry,XATTR_CAP_SET,&caps,sizeof(caps));
> +       up(&bprm_dentry->d_inode->i_sem);
> +       if(ret == sizeof(caps)) {

              if (ret == sizeof(caps)) {

> +               be32_to_cpus(&caps.version);
> +               be32_to_cpus(&caps.effective);
> +               be32_to_cpus(&caps.mask_effective);
> +               be32_to_cpus(&caps.permitted);
> +               be32_to_cpus(&caps.mask_permitted);
> +               be32_to_cpus(&caps.inheritable);
> +               be32_to_cpus(&caps.mask_inheritable);
> +               if(caps.version == _LINUX_CAPABILITY_VERSION) {

                      if (caps.version ...

> +                       cap_t(bprm->cap_effective) &= caps.mask_effective;
> +                       cap_t(bprm->cap_effective) |= caps.effective;
> +
> +                       cap_t(bprm->cap_permitted) &= caps.mask_permitted;
> +                       cap_t(bprm->cap_permitted) |= caps.permitted;
> +
> +                       cap_t(bprm->cap_inheritable) &= caps.mask_inheritable;
> +                       cap_t(bprm->cap_inheritable) |= caps.inheritable;
> +               } else
> +                       printk(KERN_WARNING "Warning: %s capability set has "
> +                               "incorrect version %08X. Correct version "
> +                               "is %08X.\n",bprm->filename,caps.version,
> +                               _LINUX_CAPABILITY_VERSION);
> +       }
> +#endif /* CONFIG_SECURITY_FS_CAPABILITIES */
> +
>         return 0;
>  }
> 

-- 
Jesper Juhl <jesper.juhl@gmail.com>
Don't top-post  http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please      http://www.expita.com/nomime.html