windows-dropbox-fix-20070426
authorAsanka Herath <asanka@secure-endpoints.com>
Thu, 26 Apr 2007 19:06:44 +0000 (19:06 +0000)
committerJeffrey Altman <jaltman@secure-endpoints.com>
Thu, 26 Apr 2007 19:06:44 +0000 (19:06 +0000)
FIXES 60161

A dropbox is a directory with ACLs 'li' that permits a user to create
a new file but not be able to read other files within the same directory.

The 1.5 Windows clients have not been able to write to dropboxes since
the addition of the locking code.  The lock acquisition test assumed
that if the user did not have PRSFS_LOCK or PRSFS_WRITE that it would
be unable to obtain a lock.  It did not take into account the special
treatment of PRSFS_INSERT by the file server and so never bothered to
ask.

As it turns out though, the locking situation is more complex than one
might think.  If the server is 1.4.1 or earlier, it will not grant
any locks for users with INSERT.  The PRSFS_LOCK privilege is required.

For 1.4.2 through 1.4.4, write locks will be granted if the user has
PRSFS_INSERT but a read lock will not be granted unless the user has
PRSFS_LOCK.  Therefore, if the server advertises the WRITELOCKACL
capability bit if the read lock is not granted a write lock can be
attempted.

For 1.4.5 and 1.5.20 and above, the file server will grant read locks
if the user has PRSFS_WRITE or PRSFS_INSERT.  (Insert only applies if
the user is the creator of the file).

This patch handles all of the above possibilities.  In the pre-1.4.2
case a read-lock request will be faked locally.

src/WINNT/afsd/cm_access.c
src/WINNT/afsd/cm_vnodeops.c

index c4ca16c..a69d087 100644 (file)
@@ -119,6 +119,15 @@ int cm_HaveAccessRights(struct cm_scache *scp, struct cm_user *userp, afs_uint32
        }
     }
 
+    /* if the user can insert, we must assume they can read/write as well
+     * because we do not have the ability to determine if the current user
+     * is the owner of the file. We will have to make the call to the
+     * file server and let the file server tell us if the request should
+     * be denied.
+     */
+    if ((*outRightsp & PRSFS_INSERT) && (scp->creator == userp))
+        *outRightsp |= PRSFS_READ | PRSFS_WRITE;
+
     /* if the user can obtain a write-lock, read-locks are implied */
     if (*outRightsp & PRSFS_WRITE)
        *outRightsp |= PRSFS_LOCK;
index 15a4d38..f507e6e 100644 (file)
@@ -2209,6 +2209,15 @@ long cm_SetLength(cm_scache_t *scp, osi_hyper_t *sizep, cm_user_t *userp,
     code = cm_SyncOp(scp, NULL, userp, reqp, PRSFS_WRITE,
                       CM_SCACHESYNC_NEEDCALLBACK | CM_SCACHESYNC_GETSTATUS
                       | CM_SCACHESYNC_SETSTATUS | CM_SCACHESYNC_SETSIZE);
+
+    /* If we only have 'i' bits, then we should still be able to set
+       the size of a file we created. */
+    if (code == CM_ERROR_NOACCESS && scp->creator == userp) {
+        code = cm_SyncOp(scp, NULL, userp, reqp, PRSFS_INSERT,
+                         CM_SCACHESYNC_NEEDCALLBACK | CM_SCACHESYNC_GETSTATUS
+                         | CM_SCACHESYNC_SETSTATUS | CM_SCACHESYNC_SETSIZE);
+    }
+
     if (code) 
         goto done;
 
@@ -3238,6 +3247,16 @@ long cm_Rename(cm_scache_t *oldDscp, char *oldNamep, cm_scache_t *newDscp,
 
 #define SERVERLOCKS_ENABLED(scp) (!((scp)->flags & CM_SCACHEFLAG_RO) && cm_enableServerLocks && SCP_SUPPORTS_BRLOCKS(scp))
 
+#if defined(VICED_CAPABILITY_WRITELOCKACL)
+#define SCP_SUPPORTS_WRITELOCKACL(scp) ((scp)->cbServerp && ((scp->cbServerp->capabilities & VICED_CAPABILITY_WRITELOCKACL)))
+#else
+#define SCP_SUPPORTS_WRITELOCKACL(scp) (0)
+
+/* This should really be defined in any build that this code is being
+   compiled. */
+#error  VICED_CAPABILITY_WRITELOCKACL not defined.
+#endif
+
 static void cm_LockRangeSubtract(cm_range_t * pos, const cm_range_t * neg)
 {
     afs_int64 int_begin;
@@ -3540,14 +3559,19 @@ long cm_IntReleaseLock(cm_scache_t * scp, cm_user_t * userp,
    - CM_ERROR_NOACCESS if not
 
    Any other error from cm_SyncOp will be sent down untranslated.
+
+   If CM_ERROR_NOACCESS is returned and lock_type is LockRead, then
+   phas_insert (if non-NULL) will receive a boolean value indicating
+   whether the user has INSERT permission or not.
 */
 long cm_LockCheckPerms(cm_scache_t * scp,
                        int lock_type,
                        cm_user_t * userp,
-                       cm_req_t * reqp)
+                       cm_req_t * reqp,
+                       int * phas_insert)
 {
     long rights = 0;
-    long code = 0;
+    long code = 0, code2 = 0;
 
     /* lock permissions are slightly tricky because of the 'i' bit.
        If the user has PRSFS_LOCK, she can read-lock the file.  If the
@@ -3571,21 +3595,37 @@ long cm_LockCheckPerms(cm_scache_t * scp,
         return 0;
     }
 
+    if (phas_insert)
+        *phas_insert = FALSE;
+
     code = cm_SyncOp(scp, NULL, userp, reqp, rights,
                      CM_SCACHESYNC_GETSTATUS |
                      CM_SCACHESYNC_NEEDCALLBACK);
 
-    if (code == CM_ERROR_NOACCESS &&
-        lock_type == LockWrite &&
-       scp->creator == userp) {
-        /* check for PRSFS_INSERT. */
+    if (phas_insert && scp->creator == userp) {
 
-        code = cm_SyncOp(scp, NULL, userp, reqp, PRSFS_INSERT,
+        /* If this file was created by the user, then we check for
+           PRSFS_INSERT.  If the file server is recent enough, then
+           this should be sufficient for her to get a write-lock (but
+           not necessarily a read-lock). VICED_CAPABILITY_WRITELOCKACL
+           indicates whether a file server supports getting write
+           locks when the user only has PRSFS_INSERT. 
+           
+           If the file was not created by the user we skip the check
+           because the INSERT bit will not apply to this user even
+           if it is set.
+         */
+
+        code2 = cm_SyncOp(scp, NULL, userp, reqp, PRSFS_INSERT,
                          CM_SCACHESYNC_GETSTATUS |
                          CM_SCACHESYNC_NEEDCALLBACK);
 
-       if (code == CM_ERROR_NOACCESS)
-           osi_Log0(afsd_logp, "cm_LockCheckPerms user is creator but has no INSERT bits for scp");
+       if (code2 == CM_ERROR_NOACCESS) {
+           osi_Log0(afsd_logp, "cm_LockCheckPerms user has no INSERT bits");
+        } else {
+            *phas_insert = TRUE;
+            osi_Log0(afsd_logp, "cm_LockCheckPerms user has INSERT bits");
+        }
     }
 
     cm_SyncOpDone(scp, NULL, CM_SCACHESYNC_NEEDCALLBACK | CM_SCACHESYNC_GETSTATUS);
@@ -3677,11 +3717,13 @@ long cm_Lock(cm_scache_t *scp, unsigned char sLockType,
         if (Which == scp->serverLock ||
            (Which == LockRead && scp->serverLock == LockWrite)) {
 
+            int has_insert = 0;
+
             /* we already have the lock we need */
             osi_Log3(afsd_logp, "   we already have the correct lock. exclusives[%d], shared[%d], serverLock[%d]", 
                      scp->exclusiveLocks, scp->sharedLocks, (int)(signed char) scp->serverLock);
 
-            code = cm_LockCheckPerms(scp, Which, userp, reqp);
+            code = cm_LockCheckPerms(scp, Which, userp, reqp, &has_insert);
 
             /* special case: if we don't have permission to read-lock
                the file, then we force a clientside lock.  This is to
@@ -3689,12 +3731,19 @@ long cm_Lock(cm_scache_t *scp, unsigned char sLockType,
                reading files off of directories that don't grant
                read-locks to the user. */
             if (code == CM_ERROR_NOACCESS && Which == LockRead) {
-                osi_Log0(afsd_logp, "   User has no read-lock perms. Forcing client-side lock");
-                force_client_lock = TRUE;
+
+                if (has_insert && SCP_SUPPORTS_WRITELOCKACL(scp)) {
+                    osi_Log0(afsd_logp, "   User has no read-lock perms, but has INSERT perms.");
+                    code = 0;
+                } else {
+                    osi_Log0(afsd_logp, "   User has no read-lock perms. Forcing client-side lock");
+                    force_client_lock = TRUE;
+                }
             }
 
         } else if ((scp->exclusiveLocks > 0) ||
                    (scp->sharedLocks > 0 && scp->serverLock != LockRead)) {
+            int has_insert = 0;
 
             /* We are already waiting for some other lock.  We should
                wait for the daemon to catch up instead of generating a
@@ -3704,12 +3753,20 @@ long cm_Lock(cm_scache_t *scp, unsigned char sLockType,
 
             /* see if we have permission to create the lock in the
                first place. */
-            code = cm_LockCheckPerms(scp, Which, userp, reqp);
+            code = cm_LockCheckPerms(scp, Which, userp, reqp, &has_insert);
             if (code == 0)
                code = CM_ERROR_WOULDBLOCK;
             else if (code == CM_ERROR_NOACCESS && Which == LockRead) {
-                osi_Log0(afsd_logp, "   User has no read-lock perms.  Forcing client-side lock");
-                force_client_lock = TRUE;
+
+                if (has_insert && SCP_SUPPORTS_WRITELOCKACL(scp)) {
+                    osi_Log0(afsd_logp,
+                             "   User has no read-lock perms, but has INSERT perms.");
+                    code = CM_ERROR_WOULDBLOCK;
+                } else {
+                    osi_Log0(afsd_logp,
+                             "   User has no read-lock perms. Forcing client-side lock");
+                    force_client_lock = TRUE;
+                }
             }
 
             /* leave any other codes as-is */
@@ -3717,24 +3774,28 @@ long cm_Lock(cm_scache_t *scp, unsigned char sLockType,
         } else {
             int newLock;
             int check_data_version = FALSE;
+            int has_insert = 0;
 
             /* first check if we have permission to elevate or obtain
                the lock. */
-            code = cm_LockCheckPerms(scp, Which, userp, reqp);
+            code = cm_LockCheckPerms(scp, Which, userp, reqp, &has_insert);
             if (code) {
-                if (code == CM_ERROR_NOACCESS && Which == LockRead) {
+                if (code == CM_ERROR_NOACCESS && Which == LockRead &&
+                    (!has_insert || !SCP_SUPPORTS_WRITELOCKACL(scp))) {
                     osi_Log0(afsd_logp, "   User has no read-lock perms.  Forcing client-side lock");
                     force_client_lock = TRUE;
                 }
                 goto check_code;
             }
 
+            /* has_insert => (Which == LockRead, code == CM_ERROR_NOACCESS) */
+
             if (scp->serverLock == LockRead && Which == LockWrite) {
 
                 /* We want to escalate the lock to a LockWrite.
-                   Unfortunately that's not really possible without
-                   letting go of the current lock.  But for now we do
-                   it anyway. */
+                 * Unfortunately that's not really possible without
+                 * letting go of the current lock.  But for now we do
+                 * it anyway. */
 
                 osi_Log0(afsd_logp,
                          "   attempting to UPGRADE from LockRead to LockWrite.");
@@ -3757,17 +3818,20 @@ long cm_Lock(cm_scache_t *scp, unsigned char sLockType,
             }
 
             /* We need to obtain a server lock of type Which in order
-               to assert this file lock */
+             * to assert this file lock */
 #ifndef AGGRESSIVE_LOCKS
             newLock = Which;
 #else
             newLock = LockWrite;
 #endif
+
             code = cm_IntSetLock(scp, userp, newLock, reqp);
 
-            if (code == CM_ERROR_WOULDBLOCK && newLock != Which) {
+#ifdef AGGRESSIVE_LOCKS
+            if ((code == CM_ERROR_WOULDBLOCK ||
+                 code == CM_ERROR_NOACCESS) && newLock != Which) {
                 /* we wanted LockRead.  We tried LockWrite. Now try
-                   LockRead again */
+                 * LockRead again */
                 newLock = Which;
 
                 /* am I sane? */
@@ -3775,12 +3839,54 @@ long cm_Lock(cm_scache_t *scp, unsigned char sLockType,
 
                 code = cm_IntSetLock(scp, userp, newLock, reqp);
             }
+#endif
+
+            if (code == CM_ERROR_NOACCESS) {
+                if (Which == LockRead) {
+                    if (has_insert && SCP_SUPPORTS_WRITELOCKACL(scp)) {
+                        long tcode;
+                        /* We requested a read-lock, but we have permission to
+                         * get a write-lock. Try that */
+
+                        tcode = cm_LockCheckPerms(scp, LockWrite, userp, reqp, NULL);
+
+                        if (tcode == 0) {
+                            newLock = LockWrite;
+
+                            osi_Log0(afsd_logp, "   User has 'i' perms and the request was for a LockRead.  Trying to get a LockWrite instead");
+
+                            code = cm_IntSetLock(scp, userp, newLock, reqp);
+                        }
+                    } else {
+                        osi_Log0(afsd_logp, "   User has no read-lock perms.  Forcing client-side lock");
+                        force_client_lock = TRUE;
+                    }
+                } else if (Which == LockWrite &&
+                           scp->creator == userp && !SCP_SUPPORTS_WRITELOCKACL(scp)) {
+                    long tcode;
+
+                    /* Special case: if the lock request was for a
+                     * LockWrite and the user owns the file and we weren't
+                     * allowed to obtain the serverlock, we either lost a
+                     * race (the permissions changed from under us), or we
+                     * have 'i' bits, but we aren't allowed to lock the
+                     * file. */
+
+                    /* check if we lost a race... */
+                    tcode = cm_LockCheckPerms(scp, Which, userp, reqp, NULL);
+
+                    if (tcode == 0) {
+                        osi_Log0(afsd_logp, "   User has 'i' perms but can't obtain write locks. Using client-side locks.");
+                        force_client_lock = TRUE;
+                    }
+                }
+            }
 
             if (code == 0 && check_data_version &&
                scp->dataVersion != scp->lockDataVersion) {
                 /* We lost a race.  Although we successfully obtained
-                   a lock, someone modified the file in between.  The
-                   locks have all been technically lost. */
+                 * a lock, someone modified the file in between.  The
+                 * locks have all been technically lost. */
 
                 osi_Log0(afsd_logp,
                          "  Data version mismatch while upgrading lock.");
@@ -4198,6 +4304,17 @@ long cm_Unlock(cm_scache_t *scp,
         scp->lockDataVersion = scp->dataVersion;
         osi_Log1(afsd_logp, "   dataVersion on scp is %d", scp->dataVersion);
 
+        /* before we downgrade, make sure that we have enough
+           permissions to get the read lock. */
+        code = cm_LockCheckPerms(scp, LockRead, userp, reqp, NULL);
+        if (code != 0) {
+
+            osi_Log0(afsd_logp, "  SKIPPING downgrade because user doesn't have perms to get downgraded lock");
+
+            code = 0;
+            goto done;
+        }
+
         code = cm_IntReleaseLock(scp, userp, reqp);
 
         if (code) {
@@ -4523,6 +4640,8 @@ long cm_RetryLock(cm_file_lock_t *oldFileLock, int client_is_dead)
     cm_req_t req;
     int newLock = -1;
     int force_client_lock = FALSE;
+    int has_insert = FALSE;
+    int check_data_version = FALSE;
 
     cm_InitReq(&req);
 
@@ -4568,10 +4687,12 @@ long cm_RetryLock(cm_file_lock_t *oldFileLock, int client_is_dead)
 
     code = cm_LockCheckPerms(scp, oldFileLock->lockType,
                              oldFileLock->userp,
-                             &req);
+                             &req, &has_insert);
 
     if (code == CM_ERROR_NOACCESS && oldFileLock->lockType == LockRead) {
+        if (!has_insert || !SCP_SUPPORTS_WRITELOCKACL(scp)) {
         force_client_lock = TRUE;
+        }
         code = 0;
     } else if (code) {
         lock_ReleaseMutex(&scp->mx);
@@ -4665,6 +4786,8 @@ long cm_RetryLock(cm_file_lock_t *oldFileLock, int client_is_dead)
         oldFileLock->flags |= CM_FILELOCK_FLAG_WAITLOCK;
     }
 
+    osi_assert(IS_LOCK_WAITLOCK(oldFileLock));
+
     if (force_client_lock ||
         !SERVERLOCKS_ENABLED(scp) ||
         scp->serverLock == oldFileLock->lockType ||
@@ -4716,10 +4839,67 @@ long cm_RetryLock(cm_file_lock_t *oldFileLock, int client_is_dead)
         newLock = LockWrite;
 #endif
 
+        if (has_insert) {
+            /* if has_insert is non-zero, then:
+               - the lock a LockRead
+               - we don't have permission to get a LockRead
+               - we do have permission to get a LockWrite
+               - the server supports VICED_CAPABILITY_WRITELOCKACL
+            */
+
+            newLock = LockWrite;
+        }
+
         lock_ReleaseWrite(&cm_scacheLock);
 
+        /* when we get here, either we have a read-lock and want a
+           write-lock or we don't have any locks and we want some
+           lock. */
+
+        if (scp->serverLock == LockRead) {
+
+            osi_assert(newLock == LockWrite);
+
+            osi_Log0(afsd_logp, "  Attempting to UPGRADE from LockRead to LockWrite");
+
+            scp->lockDataVersion = scp->dataVersion;
+            check_data_version = TRUE;
+
+            code = cm_IntReleaseLock(scp, userp, &req);
+
+            if (code)
+                goto pre_syncopdone;
+            else
+                scp->serverLock = -1;
+        }
+
         code = cm_IntSetLock(scp, userp, newLock, &req);
 
+        if (code == 0) {
+            if (scp->dataVersion != scp->lockDataVersion) {
+                /* we lost a race.  too bad */
+
+                osi_Log0(afsd_logp,
+                         "  Data version mismatch while upgrading lock.");
+                osi_Log2(afsd_logp,
+                         "  Data versions before=%d, after=%d",
+                         scp->lockDataVersion,
+                         scp->dataVersion);
+                osi_Log1(afsd_logp,
+                         "  Releasing stale lock for scp 0x%x", scp);
+
+                code = cm_IntReleaseLock(scp, userp, &req);
+
+                scp->serverLock = -1;
+
+                code = CM_ERROR_INVAL;
+
+                cm_LockMarkSCacheLost(scp);
+            } else {
+                scp->serverLock = newLock;
+            }
+        }
+
     pre_syncopdone:
         cm_SyncOpDone(scp, NULL, CM_SCACHESYNC_LOCK);
     post_syncopdone:
@@ -4733,8 +4913,6 @@ long cm_RetryLock(cm_file_lock_t *oldFileLock, int client_is_dead)
             scp->fileLocksT = osi_QPrev(&oldFileLock->fileq);
         osi_QRemoveHT(&scp->fileLocksH, &scp->fileLocksT, &oldFileLock->fileq);
        lock_ReleaseWrite(&cm_scacheLock);
-    } else if (code == 0 && IS_LOCK_WAITLOCK(oldFileLock)) {
-        scp->serverLock = newLock;
     }
     lock_ReleaseMutex(&scp->mx);