Re: quotactl permissions

[Eric Haszlakiewicz <erh%nimenees.com@localhost>]

On Wed, Sep 05, 2012 at 10:54:43AM -0400, Thor Lancelot Simon wrote:
> On Wed, Sep 05, 2012 at 12:34:16PM +0000, Emmanuel Dreyfus wrote:
> > On Wed, Sep 05, 2012 at 06:37:27AM +0000, David Holland wrote:
> > > Changing it to effective uid seems like a good plan.
> > 
> > The change below fixes the test case. Is it safe to commit?
> 
> It fixes the test case, but it is still wrong.  This UID check
> needs to be implemented in the kauth listener (adding a specific
> case for this action if there's not one) not open-coded in the UFS code.

Here's a (completely untested) patch that should do that.

eric

Index: secmodel/suser/secmodel_suser.c
===================================================================
RCS file: /space/netbsd/NetBSD-cvs/src/sys/secmodel/suser/secmodel_suser.c,v
retrieving revision 1.39
diff -u -r1.39 secmodel_suser.c
--- secmodel/suser/secmodel_suser.c     13 Mar 2012 18:41:02 -0000      1.39
+++ secmodel/suser/secmodel_suser.c     5 Sep 2012 16:56:44 -0000
@@ -268,6 +268,13 @@
        case KAUTH_SYSTEM_FS_QUOTA:
                switch (req) {
                case KAUTH_REQ_SYSTEM_FS_QUOTA_GET:
+                       /* The user can always query about his own quota. */
+                       if (id == kauth_cred_geteuid(l->l_cred))
+                       {
+                               result = KAUTH_RESULT_ALLOW;
+                               break;
+                       }
+                       /* FALLTHROUGH */
                case KAUTH_REQ_SYSTEM_FS_QUOTA_ONOFF:
                case KAUTH_REQ_SYSTEM_FS_QUOTA_MANAGE:
                case KAUTH_REQ_SYSTEM_FS_QUOTA_NOLIMIT:
Index: ufs/ufs/ufs_quota.c
===================================================================
RCS file: /space/netbsd/NetBSD-cvs/src/sys/ufs/ufs/ufs_quota.c,v
retrieving revision 1.109
diff -u -r1.109 ufs_quota.c
--- ufs/ufs/ufs_quota.c 18 Feb 2012 06:13:23 -0000      1.109
+++ ufs/ufs/ufs_quota.c 5 Sep 2012 16:57:32 -0000
@@ -331,16 +331,6 @@
        return 0;
 }
 
-/* XXX shouldn't all this be in kauth ? */
-static int
-quota_get_auth(struct mount *mp, struct lwp *l, uid_t id) {
-       /* The user can always query about his own quota. */
-       if (id == kauth_cred_getuid(l->l_cred))
-               return 0;
-       return kauth_authorize_system(l->l_cred, KAUTH_SYSTEM_FS_QUOTA,
-           KAUTH_REQ_SYSTEM_FS_QUOTA_GET, mp, KAUTH_ARG(id), NULL);
-}
-
 static int 
 quota_handle_cmd_get(struct mount *mp, struct lwp *l, 
     struct quotactl_args *args)
@@ -357,7 +347,8 @@
        if ((ump->um_flags & (UFS_QUOTA|UFS_QUOTA2)) == 0)
                return EOPNOTSUPP;
        
-       error = quota_get_auth(mp, l, qk->qk_id);
+       error = kauth_authorize_system(l->l_cred, KAUTH_SYSTEM_FS_QUOTA,
+           KAUTH_REQ_SYSTEM_FS_QUOTA_GET, mp, KAUTH_ARG(qk->qk_id), NULL);
        if (error != 0) 
                return error;
 #ifdef QUOTA