From 1c9710496db3b161711492ddad6b57fad622d4ba Mon Sep 17 00:00:00 2001 From: corinna Date: Thu, 14 Aug 2008 14:05:03 +0000 Subject: [PATCH] * external.cc (cygwin_internal): Call set_security_attribute with additional path_conv argument. * fhandler.cc (fhandler_base::open): Ditto. * fhandler_disk_file.cc (fhandler_disk_file::fchmod): Never set DOS R/O attribute when using ACLs. (fhandler_disk_file::mkdir): Ditto. Set security descriptor on remote dirs after creating the dir, same as in fhandler_base::open. * fhandler_socket.cc (fhandler_socket::bind): Ditto for remote AF_LOCAL socket files. * path.cc (symlink_worker): Ditto. for remote symlinks. * security.cc (alloc_sd): Take additional path_conv argument. Accommodate throughout. Drop setting FILE_WRITE_EA/FILE_READ_EA flags unconditionally (was only necessary for "ntea"). Don't set FILE_READ_ATTRIBUTES and FILE_WRITE_ATTRIBUTES unconditionally on Samba. Add comment to explain. Drop useless setting of STANDARD_RIGHTS_WRITE, it's in FILE_GENERIC_WRITE anyway. Remove FILE_READ_ATTRIBUTES bit from FILE_GENERIC_EXECUTE so as not to enforce read permissions on Samba. (set_security_attribute): Take additional path_conv argument. * security.h (set_security_attribute): Change prototype accordingly. --- winsup/cygwin/ChangeLog | 24 ++++++++++++++++++++++++ winsup/cygwin/external.cc | 3 ++- winsup/cygwin/fhandler.cc | 2 +- winsup/cygwin/fhandler_disk_file.cc | 13 +++++++++---- winsup/cygwin/fhandler_socket.cc | 8 ++++++-- winsup/cygwin/path.cc | 9 +++++++-- winsup/cygwin/security.cc | 32 ++++++++++++++++++-------------- winsup/cygwin/security.h | 3 ++- 8 files changed, 69 insertions(+), 25 deletions(-) diff --git a/winsup/cygwin/ChangeLog b/winsup/cygwin/ChangeLog index 7d7d6acf8a..e6850f679f 100644 --- a/winsup/cygwin/ChangeLog +++ b/winsup/cygwin/ChangeLog @@ -1,3 +1,27 @@ +2008-08-14 Corinna Vinschen + + * external.cc (cygwin_internal): Call set_security_attribute with + additional path_conv argument. + * fhandler.cc (fhandler_base::open): Ditto. + * fhandler_disk_file.cc (fhandler_disk_file::fchmod): Never set DOS + R/O attribute when using ACLs. + (fhandler_disk_file::mkdir): Ditto. Set + security descriptor on remote dirs after creating the dir, same as in + fhandler_base::open. + * fhandler_socket.cc (fhandler_socket::bind): Ditto for remote AF_LOCAL + socket files. + * path.cc (symlink_worker): Ditto. for remote symlinks. + * security.cc (alloc_sd): Take additional path_conv argument. + Accommodate throughout. Drop setting FILE_WRITE_EA/FILE_READ_EA + flags unconditionally (was only necessary for "ntea"). Don't set + FILE_READ_ATTRIBUTES and FILE_WRITE_ATTRIBUTES unconditionally on + Samba. Add comment to explain. Drop useless setting of + STANDARD_RIGHTS_WRITE, it's in FILE_GENERIC_WRITE anyway. + Remove FILE_READ_ATTRIBUTES bit from FILE_GENERIC_EXECUTE so as not + to enforce read permissions on Samba. + (set_security_attribute): Take additional path_conv argument. + * security.h (set_security_attribute): Change prototype accordingly. + 2008-08-13 Corinna Vinschen * mount.cc (fillout_mntent): Always print noumount option last. diff --git a/winsup/cygwin/external.cc b/winsup/cygwin/external.cc index d9ab4d5783..675d8b0666 100644 --- a/winsup/cygwin/external.cc +++ b/winsup/cygwin/external.cc @@ -288,12 +288,13 @@ cygwin_internal (cygwin_getinfo_types t, ...) } case CW_GET_POSIX_SECURITY_ATTRIBUTE: { + path_conv dummy; security_descriptor sd; int attribute = va_arg (arg, int); PSECURITY_ATTRIBUTES psa = va_arg (arg, PSECURITY_ATTRIBUTES); void *sd_buf = va_arg (arg, void *); DWORD sd_buf_size = va_arg (arg, DWORD); - set_security_attribute (attribute, psa, sd); + set_security_attribute (dummy, attribute, psa, sd); if (!psa->lpSecurityDescriptor) return sd.size (); psa->lpSecurityDescriptor = sd_buf; diff --git a/winsup/cygwin/fhandler.cc b/winsup/cygwin/fhandler.cc index 0f75ea7f57..7827d2d215 100644 --- a/winsup/cygwin/fhandler.cc +++ b/winsup/cygwin/fhandler.cc @@ -577,7 +577,7 @@ fhandler_base::open (int flags, mode_t mode) See below. */ if (has_acls () && !pc.isremote ()) { - set_security_attribute (mode, &sa, sd); + set_security_attribute (pc, mode, &sa, sd); attr.SecurityDescriptor = sa.lpSecurityDescriptor; } else if (pc.fs_is_nfs ()) diff --git a/winsup/cygwin/fhandler_disk_file.cc b/winsup/cygwin/fhandler_disk_file.cc index de2b0d06e6..5fc82302dc 100644 --- a/winsup/cygwin/fhandler_disk_file.cc +++ b/winsup/cygwin/fhandler_disk_file.cc @@ -807,10 +807,10 @@ fhandler_disk_file::fchmod (mode_t mode) res = 0; } - /* if the mode we want has any write bits set, we can't be read only. */ + /* If the mode has any write bits set, the DOS R/O flag is in the way. */ if (mode & (S_IWUSR | S_IWGRP | S_IWOTH)) pc &= (DWORD) ~FILE_ATTRIBUTE_READONLY; - else + else if (!pc.has_acls ()) /* Never set DOS R/O if security is used. */ pc |= (DWORD) FILE_ATTRIBUTE_READONLY; if (S_ISSOCK (mode)) pc |= (DWORD) FILE_ATTRIBUTE_SYSTEM; @@ -1380,8 +1380,10 @@ fhandler_disk_file::mkdir (mode_t mode) SECURITY_ATTRIBUTES sa = sec_none_nih; security_descriptor sd; - if (has_acls ()) - set_security_attribute (S_IFDIR | ((mode & 07777) & ~cygheap->umask), + /* See comments in fhander_base::open () for an explanation why we defer + setting security attributes on remote files. */ + if (has_acls () && !pc.isremote ()) + set_security_attribute (pc, S_IFDIR | ((mode & 07777) & ~cygheap->umask), &sa, sd); NTSTATUS status; @@ -1418,6 +1420,9 @@ fhandler_disk_file::mkdir (mode_t mode) p, plen); if (NT_SUCCESS (status)) { + if (has_acls () && pc.isremote ()) + set_file_attribute (dir, pc, ILLEGAL_UID, ILLEGAL_GID, + S_IFDIR | ((mode & 07777) & ~cygheap->umask)); NtClose (dir); res = 0; } diff --git a/winsup/cygwin/fhandler_socket.cc b/winsup/cygwin/fhandler_socket.cc index 176838de86..1ab5170ea9 100644 --- a/winsup/cygwin/fhandler_socket.cc +++ b/winsup/cygwin/fhandler_socket.cc @@ -882,8 +882,10 @@ fhandler_socket::bind (const struct sockaddr *name, int namelen) fattr |= FILE_ATTRIBUTE_READONLY; SECURITY_ATTRIBUTES sa = sec_none_nih; security_descriptor sd; - if (pc.has_acls ()) - set_security_attribute (mode, &sa, sd); + /* See comments in fhander_base::open () for an explanation why we defer + setting security attributes on remote files. */ + if (pc.has_acls () && !pc.isremote ()) + set_security_attribute (pc, mode, &sa, sd); NTSTATUS status; HANDLE fh; OBJECT_ATTRIBUTES attr; @@ -904,6 +906,8 @@ fhandler_socket::bind (const struct sockaddr *name, int namelen) } else { + if (pc.has_acls () && pc.isremote ()) + set_file_attribute (fh, pc, ILLEGAL_UID, ILLEGAL_GID, mode); char buf[sizeof (SOCKET_COOKIE) + 80]; __small_sprintf (buf, "%s%u %c ", SOCKET_COOKIE, sin.sin_port, get_socket_type () == SOCK_STREAM ? 's' diff --git a/winsup/cygwin/path.cc b/winsup/cygwin/path.cc index 64d8759b68..4a91ff876f 100644 --- a/winsup/cygwin/path.cc +++ b/winsup/cygwin/path.cc @@ -1766,8 +1766,10 @@ symlink_worker (const char *oldpath, const char *newpath, bool use_winsym, goto done; } } - if (win32_newpath.has_acls ()) - set_security_attribute (S_IFLNK | STD_RBITS | STD_WBITS, + /* See comments in fhander_base::open () for an explanation why we defer + setting security attributes on remote files. */ + if (win32_newpath.has_acls () && !win32_newpath.isremote ()) + set_security_attribute (win32_newpath, S_IFLNK | STD_RBITS | STD_WBITS, &sa, sd); status = NtCreateFile (&fh, DELETE | FILE_GENERIC_WRITE, win32_newpath.get_object_attr (attr, sa), @@ -1783,6 +1785,9 @@ symlink_worker (const char *oldpath, const char *newpath, bool use_winsym, __seterrno_from_nt_status (status); goto done; } + if (win32_newpath.has_acls () && win32_newpath.isremote ()) + set_file_attribute (fh, win32_newpath, ILLEGAL_UID, ILLEGAL_GID, + S_IFLNK | STD_RBITS | STD_WBITS); status = NtWriteFile (fh, NULL, NULL, NULL, &io, buf, cp - buf, NULL, NULL); if (NT_SUCCESS (status) && io.Information == (ULONG) (cp - buf)) { diff --git a/winsup/cygwin/security.cc b/winsup/cygwin/security.cc index b4c7caf2c1..5d9846abcf 100644 --- a/winsup/cygwin/security.cc +++ b/winsup/cygwin/security.cc @@ -380,7 +380,7 @@ add_access_denied_ace (PACL acl, int offset, DWORD attributes, } static PSECURITY_DESCRIPTOR -alloc_sd (__uid32_t uid, __gid32_t gid, int attribute, +alloc_sd (path_conv &pc, __uid32_t uid, __gid32_t gid, int attribute, security_descriptor &sd_ret) { BOOL dummy; @@ -462,28 +462,31 @@ alloc_sd (__uid32_t uid, __gid32_t gid, int attribute, size_t acl_len = sizeof (ACL); int ace_off = 0; - /* Construct allow attribute for owner. */ + /* Construct allow attribute for owner. + Don't set FILE_READ/WRITE_ATTRIBUTES unconditionally on Samba, otherwise + it enforces read permissions. Same for other's below. */ DWORD owner_allow = STANDARD_RIGHTS_ALL - | FILE_WRITE_ATTRIBUTES | FILE_WRITE_EA; + | pc.fs_is_samba () + ? 0 : (FILE_READ_ATTRIBUTES | FILE_WRITE_ATTRIBUTES); if (attribute & S_IRUSR) owner_allow |= FILE_GENERIC_READ; if (attribute & S_IWUSR) owner_allow |= FILE_GENERIC_WRITE; if (attribute & S_IXUSR) - owner_allow |= FILE_GENERIC_EXECUTE; + owner_allow |= FILE_GENERIC_EXECUTE & ~FILE_READ_ATTRIBUTES; if (S_ISDIR (attribute) && (attribute & (S_IWUSR | S_IXUSR)) == (S_IWUSR | S_IXUSR)) owner_allow |= FILE_DELETE_CHILD; /* Construct allow attribute for group. */ - DWORD group_allow = STANDARD_RIGHTS_READ - | FILE_READ_ATTRIBUTES | FILE_READ_EA; + DWORD group_allow = STANDARD_RIGHTS_READ | + (pc.fs_is_samba () ? 0 : FILE_READ_ATTRIBUTES); if (attribute & S_IRGRP) group_allow |= FILE_GENERIC_READ; if (attribute & S_IWGRP) - group_allow |= STANDARD_RIGHTS_WRITE | FILE_GENERIC_WRITE; + group_allow |= FILE_GENERIC_WRITE; if (attribute & S_IXGRP) - group_allow |= FILE_GENERIC_EXECUTE; + group_allow |= FILE_GENERIC_EXECUTE & ~FILE_READ_ATTRIBUTES; if (S_ISDIR (attribute) && (attribute & (S_IWGRP | S_IXGRP)) == (S_IWGRP | S_IXGRP) && !(attribute & S_ISVTX)) @@ -491,13 +494,13 @@ alloc_sd (__uid32_t uid, __gid32_t gid, int attribute, /* Construct allow attribute for everyone. */ DWORD other_allow = STANDARD_RIGHTS_READ - | FILE_READ_ATTRIBUTES | FILE_READ_EA; + | (pc.fs_is_samba () ? 0 : FILE_READ_ATTRIBUTES); if (attribute & S_IROTH) other_allow |= FILE_GENERIC_READ; if (attribute & S_IWOTH) - other_allow |= STANDARD_RIGHTS_WRITE | FILE_GENERIC_WRITE; + other_allow |= FILE_GENERIC_WRITE; if (attribute & S_IXOTH) - other_allow |= FILE_GENERIC_EXECUTE; + other_allow |= FILE_GENERIC_EXECUTE & ~FILE_READ_ATTRIBUTES; if (S_ISDIR (attribute) && (attribute & (S_IWOTH | S_IXOTH)) == (S_IWOTH | S_IXOTH) && !(attribute & S_ISVTX)) @@ -682,13 +685,13 @@ alloc_sd (__uid32_t uid, __gid32_t gid, int attribute, } void -set_security_attribute (int attribute, PSECURITY_ATTRIBUTES psa, +set_security_attribute (path_conv &pc, int attribute, PSECURITY_ATTRIBUTES psa, security_descriptor &sd) { psa->lpSecurityDescriptor = sd.malloc (SECURITY_DESCRIPTOR_MIN_LENGTH); InitializeSecurityDescriptor ((PSECURITY_DESCRIPTOR)psa->lpSecurityDescriptor, SECURITY_DESCRIPTOR_REVISION); - psa->lpSecurityDescriptor = alloc_sd (geteuid32 (), getegid32 (), + psa->lpSecurityDescriptor = alloc_sd (pc, geteuid32 (), getegid32 (), attribute, sd); } @@ -702,7 +705,8 @@ set_file_attribute (HANDLE handle, path_conv &pc, { security_descriptor sd; - if (!get_file_sd (handle, pc, sd) && alloc_sd (uid, gid, attribute, sd)) + if (!get_file_sd (handle, pc, sd) + && alloc_sd (pc, uid, gid, attribute, sd)) ret = set_file_sd (handle, pc, sd); } else diff --git a/winsup/cygwin/security.h b/winsup/cygwin/security.h index 9be7a28e23..27a2d00469 100644 --- a/winsup/cygwin/security.h +++ b/winsup/cygwin/security.h @@ -349,7 +349,8 @@ bool __stdcall add_access_denied_ace (PACL acl, int offset, DWORD attributes, PS int __stdcall check_file_access (path_conv &, int); int __stdcall check_registry_access (HANDLE, int); -void set_security_attribute (int attribute, PSECURITY_ATTRIBUTES psa, +void set_security_attribute (path_conv &pc, int attribute, + PSECURITY_ATTRIBUTES psa, security_descriptor &sd_buf); bool get_sids_info (cygpsid, cygpsid, __uid32_t * , __gid32_t *); -- 2.11.0