7190897: (fs) Files.isWritable method returns false when the path is writable (win)

Summary: The [GetEffectiveRightsFromAcl] based implementation was changed to the [AccessCheck] based.
Reviewed-by: alanb

src/windows/classes/sun/nio/fs/WindowsConstants.java

@@ -181,6 +181,11 @@ class WindowsConstants {
     public static final int FILE_READ_ATTRIBUTES        = 0x0080;
     public static final int FILE_WRITE_ATTRIBUTES       = 0x0100;
 
+    public static final int FILE_GENERIC_READ           = 0x00120089;
+    public static final int FILE_GENERIC_WRITE          = 0x00120116;
+    public static final int FILE_GENERIC_EXECUTE        = 0x001200a0;
+    public static final int FILE_ALL_ACCESS             = 0x001f01ff;
+
     // operating system security
     public static final int TOKEN_DUPLICATE             = 0x0002;
     public static final int TOKEN_IMPERSONATE           = 0x0004;

src/windows/classes/sun/nio/fs/WindowsFileSystemProvider.java

@@ -38,6 +38,7 @@ import sun.nio.ch.ThreadPool;
 import sun.security.util.SecurityConstants;
 
 import static sun.nio.fs.WindowsNativeDispatcher.*;
+import static sun.nio.fs.WindowsSecurity.*;
 import static sun.nio.fs.WindowsConstants.*;
 
 public class WindowsFileSystemProvider
@@ -289,67 +290,29 @@ public class WindowsFileSystemProvider
     }
 
     /**
-     * Returns buffer with SID_AND_ATTRIBUTES structure representing the user
-     * associated with the current thread access token.
-     * FIXME - this should be cached.
+     * Checks the file security against desired access.
      */
-    private static NativeBuffer getUserInfo(WindowsPath file) throws IOException {
-        try {
-            long hToken = WindowsSecurity.processTokenWithQueryAccess;
-            int size = GetTokenInformation(hToken, TokenUser, 0L, 0);
-            assert size > 0;
-
-            NativeBuffer buffer = NativeBuffers.getNativeBuffer(size);
-            try {
-                int newsize = GetTokenInformation(hToken, TokenUser,
-                                                  buffer.address(), size);
-                if (newsize != size)
-                    throw new AssertionError();
-                return buffer;
-            } catch (WindowsException x) {
-                buffer.release();
-                throw x;
-            }
-        } catch (WindowsException x) {
-            throw new IOException(x.getMessage());
-        }
-    }
-
-    /**
-     * Reads the file ACL and return the effective access as ACCESS_MASK
-     */
-    private static int getEffectiveAccess(WindowsPath file) throws IOException {
-        // read security descriptor continaing ACL (symlinks are followed)
+    private static boolean hasDesiredAccess(WindowsPath file, int rights) throws IOException {
+        // read security descriptor containing ACL (symlinks are followed)
+        boolean hasRights = false;
         String target = WindowsLinkSupport.getFinalPath(file, true);
         NativeBuffer aclBuffer = WindowsAclFileAttributeView
-            .getFileSecurity(target, DACL_SECURITY_INFORMATION);
-
-        // retrieves DACL from security descriptor
-        long pAcl = GetSecurityDescriptorDacl(aclBuffer.address());
-
-        // Use GetEffectiveRightsFromAcl to get effective access to file
+            .getFileSecurity(target,
+                DACL_SECURITY_INFORMATION
+                | OWNER_SECURITY_INFORMATION
+                | GROUP_SECURITY_INFORMATION);
         try {
-            NativeBuffer userBuffer = getUserInfo(file);
-            try {
-                try {
-                    // SID_AND_ATTRIBUTES->pSid
-                    long pSid = unsafe.getAddress(userBuffer.address());
-                    long pTrustee = BuildTrusteeWithSid(pSid);
-                    try {
-                        return GetEffectiveRightsFromAcl(pAcl, pTrustee);
-                    } finally {
-                        LocalFree(pTrustee);
-                    }
-                } catch (WindowsException x) {
-                    throw new IOException("Unable to get effective rights from ACL: " +
-                        x.getMessage());
-                }
-            } finally {
-                userBuffer.release();
-            }
+            hasRights = checkAccessMask(aclBuffer.address(), rights,
+                FILE_GENERIC_READ,
+                FILE_GENERIC_WRITE,
+                FILE_GENERIC_EXECUTE,
+                FILE_ALL_ACCESS);
+        } catch (WindowsException exc) {
+            exc.rethrowAsIOException(file);
         } finally {
             aclBuffer.release();
         }
+        return hasRights;
     }
 
     /**
@@ -416,10 +379,10 @@ public class WindowsFileSystemProvider
             mask |= FILE_EXECUTE;
         }
 
-        if ((getEffectiveAccess(file) & mask) == 0)
+        if (!hasDesiredAccess(file, mask))
             throw new AccessDeniedException(
                 file.getPathForExceptionMessage(), null,
-                "Effective permissions does not allow requested access");
+                "Permissions does not allow requested access");
 
         // for write access we neeed to check if the DOS readonly attribute
         // and if the volume is read-only
@@ -438,7 +401,6 @@ public class WindowsFileSystemProvider
                 throw new AccessDeniedException(
                     file.getPathForExceptionMessage(), null, "Read-only file system");
             }
-            return;
         }
     }
 

src/windows/classes/sun/nio/fs/WindowsNativeDispatcher.java

@@ -844,6 +844,23 @@ class WindowsNativeDispatcher {
     static native void AdjustTokenPrivileges(long token, long luid, int attributes)
         throws WindowsException;
 
+
+    /**
+     * AccessCheck(
+     *   PSECURITY_DESCRIPTOR pSecurityDescriptor,
+     *   HANDLE ClientToken,
+     *   DWORD DesiredAccess,
+     *   PGENERIC_MAPPING GenericMapping,
+     *   PPRIVILEGE_SET PrivilegeSet,
+     *   LPDWORD PrivilegeSetLength,
+     *   LPDWORD GrantedAccess,
+     *   LPBOOL AccessStatus
+     * )
+     */
+    static native boolean AccessCheck(long token, long securityInfo, int accessMask,
+        int genericRead, int genericWrite, int genericExecute, int genericAll)
+        throws WindowsException;
+
     /**
      */
     static long LookupPrivilegeValue(String name) throws WindowsException {
@@ -857,28 +874,6 @@ class WindowsNativeDispatcher {
     private static native long LookupPrivilegeValue0(long lpName)
         throws WindowsException;
 
-    /**
-     * BuildTrusteeWithSid(
-     *   PTRUSTEE pTrustee,
-     *   PSID pSid
-     * )
-     *
-     * @return  pTrustee
-     */
-    static native long BuildTrusteeWithSid(long pSid);
-
-    /**
-     * GetEffectiveRightsFromAcl(
-     *   PACL pacl,
-     *   PTRUSTEE pTrustee,
-     *   PACCESS_MASK pAccessRights
-     * )
-     *
-     * @return  AccessRights
-     */
-    static native int GetEffectiveRightsFromAcl(long pAcl, long pTrustee)
-        throws WindowsException;
-
     /**
      * CreateSymbolicLink(
      *   LPCWSTR lpSymlinkFileName,

src/windows/classes/sun/nio/fs/WindowsSecurity.java

@@ -105,19 +105,46 @@ class WindowsSecurity {
         return new Privilege() {
             @Override
             public void drop() {
-                try {
-                    if (stopImpersontating) {
-                        SetThreadToken(0L, 0L);
-                    } else {
-                        if (needToRevert) {
+                if (token != 0L) {
+                    try {
+                        if (stopImpersontating)
+                            SetThreadToken(0L, 0L);
+                        else if (needToRevert)
                             AdjustTokenPrivileges(token, pLuid, 0);
-                        }
+                    } catch (WindowsException x) {
+                        // should not happen
+                        throw new AssertionError(x);
+                    } finally {
+                        CloseHandle(token);
                     }
-                } catch (WindowsException x) {
-                    // should not happen
-                    throw new AssertionError(x);
                 }
             }
         };
     }
+
+    /**
+     * Check the access right against the securityInfo in the current thread.
+     */
+    static boolean checkAccessMask(long securityInfo, int accessMask,
+        int genericRead, int genericWrite, int genericExecute, int genericAll)
+        throws WindowsException
+    {
+        int privilegies = TOKEN_QUERY;
+        long hToken = OpenThreadToken(GetCurrentThread(), privilegies, false);
+        if (hToken == 0L && processTokenWithDuplicateAccess != 0L)
+            hToken = DuplicateTokenEx(processTokenWithDuplicateAccess,
+                privilegies);
+
+        boolean hasRight = false;
+        if (hToken != 0L) {
+            try {
+                hasRight = AccessCheck(hToken, securityInfo, accessMask,
+                    genericRead, genericWrite, genericExecute, genericAll);
+            } finally {
+                CloseHandle(hToken);
+            }
+        }
+        return hasRight;
+    }
+
 }

src/windows/native/sun/nio/fs/WindowsNativeDispatcher.c

@@ -1021,6 +1021,33 @@ Java_sun_nio_fs_WindowsNativeDispatcher_AdjustTokenPrivileges(JNIEnv* env,
         throwWindowsException(env, GetLastError());
 }
 
+JNIEXPORT jboolean JNICALL
+Java_sun_nio_fs_WindowsNativeDispatcher_AccessCheck(JNIEnv* env,
+    jclass this, jlong token, jlong securityInfo, jint accessMask,
+    jint genericRead, jint genericWrite, jint genericExecute, jint genericAll)
+{
+    HANDLE hImpersonatedToken = (HANDLE)jlong_to_ptr(token);
+    PSECURITY_DESCRIPTOR security = (PSECURITY_DESCRIPTOR)jlong_to_ptr(securityInfo);
+    DWORD checkAccessRights = (DWORD)accessMask;
+    GENERIC_MAPPING mapping = {
+        genericRead,
+        genericWrite,
+        genericExecute,
+        genericAll};
+    PRIVILEGE_SET privileges = {0};
+    DWORD privilegesLength = sizeof(privileges);
+    DWORD grantedAccess = 0;
+    BOOL result = FALSE;
+
+    /* checkAccessRights is in-out parameter */
+    MapGenericMask(&checkAccessRights, &mapping);
+    if (AccessCheck(security, hImpersonatedToken, checkAccessRights,
+            &mapping, &privileges, &privilegesLength, &grantedAccess, &result) == 0)
+        throwWindowsException(env, GetLastError());
+
+    return (result == FALSE) ? JNI_FALSE : JNI_TRUE;
+}
+
 JNIEXPORT jlong JNICALL
 Java_sun_nio_fs_WindowsNativeDispatcher_LookupPrivilegeValue0(JNIEnv* env,
     jclass this, jlong name)
@@ -1037,35 +1064,6 @@ Java_sun_nio_fs_WindowsNativeDispatcher_LookupPrivilegeValue0(JNIEnv* env,
     return ptr_to_jlong(pLuid);
 }
 
-JNIEXPORT jlong JNICALL
-Java_sun_nio_fs_WindowsNativeDispatcher_BuildTrusteeWithSid(JNIEnv* env,
-    jclass this, jlong sid)
-{
-    PSID pSid = (HANDLE)jlong_to_ptr(sid);
-    PTRUSTEE_W pTrustee = LocalAlloc(0, sizeof(TRUSTEE_W));
-
-    if (pTrustee == NULL) {
-        JNU_ThrowInternalError(env, "Unable to allocate TRUSTEE_W structure");
-    } else {
-        BuildTrusteeWithSidW(pTrustee, pSid);
-    }
-    return ptr_to_jlong(pTrustee);
-}
-
-JNIEXPORT jint JNICALL
-Java_sun_nio_fs_WindowsNativeDispatcher_GetEffectiveRightsFromAcl(JNIEnv* env,
-    jclass this, jlong acl, jlong trustee)
-{
-    ACCESS_MASK access;
-    PACL pAcl = (PACL)jlong_to_ptr(acl);
-    PTRUSTEE pTrustee = (PTRUSTEE)jlong_to_ptr(trustee);
-
-    if (GetEffectiveRightsFromAcl(pAcl, pTrustee, &access) != ERROR_SUCCESS) {
-        throwWindowsException(env, GetLastError());
-    }
-    return (jint)access;
-}
-
 JNIEXPORT void JNICALL
 Java_sun_nio_fs_WindowsNativeDispatcher_CreateSymbolicLink0(JNIEnv* env,
     jclass this, jlong linkAddress, jlong targetAddress, jint flags)