From b8b487dd5f53c28816aff386e2ffad7125d41b76 Mon Sep 17 00:00:00 2001 From: Alexey Utkin Date: Wed, 13 Mar 2013 13:22:02 +0400 Subject: [PATCH] 7190897: (fs) Files.isWritable method returns false when the path is writable (win) The [GetEffectiveRightsFromAcl] based implementation was changed to the [AccessCheck] based. Reviewed-by: alanb --- .../classes/sun/nio/fs/WindowsConstants.java | 5 ++ .../sun/nio/fs/WindowsFileSystemProvider.java | 76 +++++-------------- .../sun/nio/fs/WindowsNativeDispatcher.java | 39 +++++----- .../classes/sun/nio/fs/WindowsSecurity.java | 45 ++++++++--- .../sun/nio/fs/WindowsNativeDispatcher.c | 56 +++++++------- 5 files changed, 104 insertions(+), 117 deletions(-) diff --git a/jdk/src/windows/classes/sun/nio/fs/WindowsConstants.java b/jdk/src/windows/classes/sun/nio/fs/WindowsConstants.java index 83becb4653b..eefd501f3bb 100644 --- a/jdk/src/windows/classes/sun/nio/fs/WindowsConstants.java +++ b/jdk/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; diff --git a/jdk/src/windows/classes/sun/nio/fs/WindowsFileSystemProvider.java b/jdk/src/windows/classes/sun/nio/fs/WindowsFileSystemProvider.java index 70a37309e9e..31b7c0f3674 100644 --- a/jdk/src/windows/classes/sun/nio/fs/WindowsFileSystemProvider.java +++ b/jdk/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; } } diff --git a/jdk/src/windows/classes/sun/nio/fs/WindowsNativeDispatcher.java b/jdk/src/windows/classes/sun/nio/fs/WindowsNativeDispatcher.java index 3d9f4973063..b0af243723a 100644 --- a/jdk/src/windows/classes/sun/nio/fs/WindowsNativeDispatcher.java +++ b/jdk/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, diff --git a/jdk/src/windows/classes/sun/nio/fs/WindowsSecurity.java b/jdk/src/windows/classes/sun/nio/fs/WindowsSecurity.java index 06351db05bf..ef9982e5d38 100644 --- a/jdk/src/windows/classes/sun/nio/fs/WindowsSecurity.java +++ b/jdk/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; + } + } diff --git a/jdk/src/windows/native/sun/nio/fs/WindowsNativeDispatcher.c b/jdk/src/windows/native/sun/nio/fs/WindowsNativeDispatcher.c index 5faf34d2458..9672b8525ad 100644 --- a/jdk/src/windows/native/sun/nio/fs/WindowsNativeDispatcher.c +++ b/jdk/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)