From ff30290b485ad0d12822b471cf56c6239ec28758 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Wed, 1 Nov 2017 09:29:24 +1100 Subject: [PATCH] 4804. [port] win32: access() does not work on directories as required by POSIX. Supply a alternative in isc_file_isdirwritable. [RT #46394] --- CHANGES | 4 ++ bin/named/server.c | 14 ++---- bin/tests/system/runtime/setup.sh | 8 ++- lib/isc/include/isc/file.h | 6 +++ lib/isc/unix/file.c | 5 ++ lib/isc/win32/file.c | 82 +++++++++++++++++++++++++++++++ lib/isc/win32/libisc.def.in | 1 + 7 files changed, 109 insertions(+), 11 deletions(-) diff --git a/CHANGES b/CHANGES index e8c6361a65..aa331726a6 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,7 @@ +4804. [port] win32: access() does not work on directories as + required by POSIX. Supply a alternative in + isc_file_isdirwritable. [RT #46394] + 4803. [placeholder] 4802. [test] Refactor mkeys system test to make it quicker and more diff --git a/bin/named/server.c b/bin/named/server.c index 0757785f87..b8910b4d22 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -154,12 +154,6 @@ #define EXCLBUFFERS 4096 #endif /* TUNE_LARGE */ -#ifdef WIN32 -#define DIR_PERM_OK W_OK -#else -#define DIR_PERM_OK W_OK|X_OK -#endif - #define MAX_TCP_TIMEOUT 65535 /*% @@ -1059,7 +1053,7 @@ configure_view_dnsseckeys(dns_view_t *view, const cfg_obj_t *vconfig, goto cleanup; } else if (directory != NULL) { - if (access(directory, DIR_PERM_OK) != 0) { + if (!isc_file_isdirwritable(directory)) { isc_log_write(named_g_lctx, NAMED_LOGCATEGORY_GENERAL, NAMED_LOGMODULE_SERVER, ISC_LOG_ERROR, "managed-keys-directory '%s' " @@ -6180,7 +6174,7 @@ directory_callback(const char *clausename, const cfg_obj_t *obj, void *arg) { "option 'directory' contains relative path '%s'", directory); - if (access(directory, DIR_PERM_OK) != 0) { + if (!isc_file_isdirwritable(directory)) { isc_log_write(named_g_lctx, NAMED_LOGCATEGORY_GENERAL, NAMED_LOGMODULE_SERVER, ISC_LOG_ERROR, "directory '%s' is not writable", @@ -7075,7 +7069,7 @@ setup_newzones(dns_view_t *view, cfg_obj_t *config, cfg_obj_t *vconfig, dir, isc_result_totext(result)); return (result); } - if (access(dir, DIR_PERM_OK) != 0) { + if (!isc_file_isdirwritable(dir)) { isc_log_write(named_g_lctx, NAMED_LOGCATEGORY_GENERAL, NAMED_LOGMODULE_SERVER, ISC_LOG_ERROR, "new-zones-directory '%s' " @@ -8516,7 +8510,7 @@ load_configuration(const char *filename, named_server_t *server, /* * Check that the working directory is writable. */ - if (access(".", DIR_PERM_OK) != 0) { + if (!isc_file_isdirwritable(".")) { isc_log_write(named_g_lctx, NAMED_LOGCATEGORY_GENERAL, NAMED_LOGMODULE_SERVER, ISC_LOG_ERROR, "the working directory is not writable"); diff --git a/bin/tests/system/runtime/setup.sh b/bin/tests/system/runtime/setup.sh index 3725a182bf..5c3f768af9 100644 --- a/bin/tests/system/runtime/setup.sh +++ b/bin/tests/system/runtime/setup.sh @@ -14,4 +14,10 @@ $SHELL clean.sh cp ns2/named1.conf ns2/named.conf mkdir ns2/nope -chmod 555 ns2/nope + +if [ 1 = "${CYGWIN:-0}" ] +then + setfacl -s user::r-x,group::r-x,other::r-x ns2/nope +else + chmod 555 ns2/nope +fi diff --git a/lib/isc/include/isc/file.h b/lib/isc/include/isc/file.h index 58c9c1a07c..370b60b161 100644 --- a/lib/isc/include/isc/file.h +++ b/lib/isc/include/isc/file.h @@ -384,6 +384,12 @@ isc_file_sanitize(const char *dir, const char *base, const char *ext, * - ISC_R_NOSPACE if the resulting path would be longer than 'length' */ +isc_boolean_t +isc_file_isdirwritable(const char *path); +/*%< + * Return true if the path is a directory and is writable + */ + ISC_LANG_ENDDECLS #endif /* ISC_FILE_H */ diff --git a/lib/isc/unix/file.c b/lib/isc/unix/file.c index 841b070f59..c7ea8c4d14 100644 --- a/lib/isc/unix/file.c +++ b/lib/isc/unix/file.c @@ -770,3 +770,8 @@ isc_file_sanitize(const char *dir, const char *base, const char *ext, strlcpy(path, buf, length); return (ISC_R_SUCCESS); } + +isc_boolean_t +isc_file_isdirwritable(const char *path) { + return (ISC_TF(access(path, W_OK|X_OK) == 0)); +} diff --git a/lib/isc/win32/file.c b/lib/isc/win32/file.c index cc104cf021..ad0eac276a 100644 --- a/lib/isc/win32/file.c +++ b/lib/isc/win32/file.c @@ -842,3 +842,85 @@ isc_file_sanitize(const char *dir, const char *base, const char *ext, strlcpy(path, buf, length); return (ISC_R_SUCCESS); } + +/* + * Based on http://blog.aaronballman.com/2011/08/how-to-check-access-rights/ + */ +isc_boolean_t +isc_file_isdirwritable(const char *path) { + DWORD length = 0; + HANDLE hToken = NULL; + PSECURITY_DESCRIPTOR security = NULL; + isc_boolean_t answer = ISC_FALSE; + + if (isc_file_isdirectory(path) != ISC_R_SUCCESS) { + return (answer); + } + + /* + * Figure out buffer size. GetFileSecurity() should not succeed. + */ + if (GetFileSecurity(path, OWNER_SECURITY_INFORMATION | + GROUP_SECURITY_INFORMATION | + DACL_SECURITY_INFORMATION, + NULL, 0, &length)) + { + return (answer); + } + + if (GetLastError() != ERROR_INSUFFICIENT_BUFFER) { + return (answer); + } + + security = malloc(length); + if (security == NULL) { + return (answer); + } + + /* + * GetFileSecurity() should succeed. + */ + if (!GetFileSecurity(path, OWNER_SECURITY_INFORMATION | + GROUP_SECURITY_INFORMATION | + DACL_SECURITY_INFORMATION, + security, length, &length)) + { + return (answer); + } + + if (OpenProcessToken(GetCurrentProcess(), TOKEN_IMPERSONATE | + TOKEN_QUERY | TOKEN_DUPLICATE | + STANDARD_RIGHTS_READ, &hToken)) + { + HANDLE hImpersonatedToken = NULL; + + if (DuplicateToken(hToken, SecurityImpersonation, + &hImpersonatedToken)) + { + GENERIC_MAPPING mapping; + PRIVILEGE_SET privileges = { 0 }; + DWORD grantedAccess = 0; + DWORD privilegesLength = sizeof(privileges); + BOOL result = FALSE; + DWORD genericAccessRights = GENERIC_WRITE; + + mapping.GenericRead = FILE_GENERIC_READ; + mapping.GenericWrite = FILE_GENERIC_WRITE; + mapping.GenericExecute = FILE_GENERIC_EXECUTE; + mapping.GenericAll = FILE_ALL_ACCESS; + + MapGenericMask(&genericAccessRights, &mapping); + if (AccessCheck(security, hImpersonatedToken, + genericAccessRights, &mapping, + &privileges, &privilegesLength, + &grantedAccess, &result)) + { + answer = ISC_TF(result); + } + CloseHandle(hImpersonatedToken); + } + CloseHandle(hToken); + } + free(security); + return (answer); +} diff --git a/lib/isc/win32/libisc.def.in b/lib/isc/win32/libisc.def.in index 92341a7b2e..83ab3854c2 100644 --- a/lib/isc/win32/libisc.def.in +++ b/lib/isc/win32/libisc.def.in @@ -232,6 +232,7 @@ isc_file_isabsolute isc_file_ischdiridempotent isc_file_iscurrentdir isc_file_isdirectory +isc_file_isdirwritable isc_file_isplainfile isc_file_isplainfilefd isc_file_mktemplate