Re: [PATCH 06b/26] Security: Make NFSD work with detached security

2008-01-17 Thread J. Bruce Fields
On Thu, Jan 17, 2008 at 05:17:20PM +, David Howells wrote:
> 
> Make NFSD work with detached security, using the patches that excise the
> security information from task_struct to struct task_security as a base.
> 
> Each time NFSD wants a new security descriptor (to do NFS4 recovery or just to
> do NFS operations), a task_security record is derived from NFSD's *objective*
> security, modified and then applied as the *subjective* security.  This means
> (a) the changes are not visible to anyone looking at NFSD through /proc, (b)
> there is no leakage between two consecutive ops with different security
> configurations.
> 
> Consideration should probably be given to caching the task_security record on
> the basis that there'll probably be several ops that will want to use any
> particular security configuration.

Just curious--why?  Are get_kernel_security(), etc., particularly
expensive?

--b.

> 
> Furthermore, nfs4recover.c perhaps ought to set an appropriate LSM context on
> the record pointed to by rec_security so that the disk is accessed
> appropriately (see set_security_override[_from_ctx]()).
> 
> NOTE!  This patch must be and will be rolled in to patch 06 of 26 to make the
> latter compile fully, however it may be useful to the nfsd maintainers to see
> it as a separate patch.
> 
> Signed-off-by: David Howells <[EMAIL PROTECTED]>
> ---
> 
>  fs/nfsd/auth.c|   31 +---
>  fs/nfsd/nfs4recover.c |   64 
> +++--
>  include/linux/sched.h |1 +
>  kernel/sys.c  |   27 ++---
>  4 files changed, 86 insertions(+), 37 deletions(-)
> 
> 
> diff --git a/fs/nfsd/auth.c b/fs/nfsd/auth.c
> index 2192805..462d989 100644
> --- a/fs/nfsd/auth.c
> +++ b/fs/nfsd/auth.c
> @@ -6,6 +6,7 @@
>  
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -28,11 +29,17 @@ int nfsexp_flags(struct svc_rqst *rqstp, struct 
> svc_export *exp)
>  
>  int nfsd_setuser(struct svc_rqst *rqstp, struct svc_export *exp)
>  {
> + struct task_security *sec, *old;
>   struct svc_cred cred = rqstp->rq_cred;
>   int i;
>   int flags = nfsexp_flags(rqstp, exp);
>   int ret;
>  
> + /* derive the new security record from nfsd's objective security */
> + sec = get_kernel_security(current);
> + if (!sec)
> + return -ENOMEM;
> +
>   if (flags & NFSEXP_ALLSQUASH) {
>   cred.cr_uid = exp->ex_anon_uid;
>   cred.cr_gid = exp->ex_anon_gid;
> @@ -56,23 +63,29 @@ int nfsd_setuser(struct svc_rqst *rqstp, struct 
> svc_export *exp)
>   get_group_info(cred.cr_group_info);
>  
>   if (cred.cr_uid != (uid_t) -1)
> - current->fsuid = cred.cr_uid;
> + sec->fsuid = cred.cr_uid;
>   else
> - current->fsuid = exp->ex_anon_uid;
> + sec->fsuid = exp->ex_anon_uid;
>   if (cred.cr_gid != (gid_t) -1)
> - current->fsgid = cred.cr_gid;
> + sec->fsgid = cred.cr_gid;
>   else
> - current->fsgid = exp->ex_anon_gid;
> + sec->fsgid = exp->ex_anon_gid;
>  
> - if (!cred.cr_group_info)
> + if (!cred.cr_group_info) {
> + put_task_security(sec);
>   return -ENOMEM;
> - ret = set_current_groups(cred.cr_group_info);
> + }
> + ret = set_groups(sec, cred.cr_group_info);
>   put_group_info(cred.cr_group_info);
>   if ((cred.cr_uid)) {
> - cap_t(current->cap_effective) &= ~CAP_NFSD_MASK;
> + cap_t(sec->cap_effective) &= ~CAP_NFSD_MASK;
>   } else {
> - cap_t(current->cap_effective) |= (CAP_NFSD_MASK &
> -   current->cap_permitted);
> + cap_t(sec->cap_effective) |= CAP_NFSD_MASK & sec->cap_permitted;
>   }
> +
> + /* set the new security as nfsd's subjective security */
> + old = current->act_as;
> + current->act_as = sec;
> + put_task_security(old);
>   return ret;
>  }
> diff --git a/fs/nfsd/nfs4recover.c b/fs/nfsd/nfs4recover.c
> index 1602cd0..ae91262 100644
> --- a/fs/nfsd/nfs4recover.c
> +++ b/fs/nfsd/nfs4recover.c
> @@ -46,27 +46,37 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #define NFSDDBG_FACILITYNFSDDBG_PROC
>  
>  /* Globals */
>  static struct nameidata rec_dir;
>  static int rec_dir_init = 0;
> +static struct task_security *rec_security;
>  
> +/*
> + * switch the special recovery access security in on the current task's
> + * subjective security
> + */
>  static void
> -nfs4_save_user(uid_t *saveuid, gid_t *savegid)
> +nfs4_begin_secure(struct task_security **saved_sec)
>  {
> - *saveuid = current->fsuid;
> - *savegid = current->fsgid;
> - current->fsuid = 0;
> - current->fsgid = 0;
> + *saved_sec = current->act_as;
> + current->act_as = get_task_security(rec_security);
>  }
>  
> +/*
> + * return the current task's subjective 

Re: [PATCH 06b/26] Security: Make NFSD work with detached security

2008-01-17 Thread David Howells
J. Bruce Fields <[EMAIL PROTECTED]> wrote:

> Just curious--why?  Are get_kernel_security(), etc., particularly
> expensive?

It involves a kmalloc().  That means an extra possibility for an error.  Plus
it may allow you to cache the result of checking whether, say, SELinux
security labels are allowed to be set when passed over NFS (if such is
possible).

David
-
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


Re: [PATCH 06b/26] Security: Make NFSD work with detached security

2008-01-17 Thread David Howells
David Howells <[EMAIL PROTECTED]> wrote:

> J. Bruce Fields <[EMAIL PROTECTED]> wrote:
> 
> > Just curious--why?  Are get_kernel_security(), etc., particularly
> > expensive?
> 
> It involves a kmalloc().  That means an extra possibility for an error.  Plus
> it may allow you to cache the result of checking whether, say, SELinux
> security labels are allowed to be set when passed over NFS (if such is
> possible).

Apart from that, though, no, it's not particularly expensive.

David
-
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