* sec_helper.cc (security_descriptor::free): If sd_size is 0, call

LocalFree instead of ::free.

	* sec_acl.cc: Throughout replace old ACE flag definitions with current
	definitions as used in MSDN man pages.
	* security.cc: Ditto.

	* fhandler.cc (fhandler_base::open): Make sure file has really been
	just created before fixing file permissions.  Add S_JUSTCREATED
	attribute to set_file_attribute call.
	* fhandler_disk_file.cc (fhandler_disk_file::mkdir): Always create dir
	with default security descriptor and fix descriptor afterwards.
	Add S_JUSTCREATED flag to set_file_attribute call.
	* fhandler_socket.cc (fhandler_socket::bind): Ditto for AF_LOCAL
	socket files.
	* path.cc (symlink_worker): Ditto for symlinks.
	* security.cc (get_file_sd): Call GetSecurityInfo rather than
	NtQuerySecurityObject.  Explain why.  Change error handling accordingly.
	(alloc_sd): Skip non-inherited, non-standard entries in ACL if
	S_JUSTCREATED attribute is set.  Explain why.  Minor format fixes.
	* security.h (S_JUSTCREATED): New define.
	(security_descriptor::operator=): New operator.
This commit is contained in:
Corinna Vinschen 2009-10-30 19:58:53 +00:00
parent 53be6f3df6
commit b42441d32b
9 changed files with 93 additions and 57 deletions

View File

@ -1,3 +1,28 @@
2009-10-30 Corinna Vinschen <corinna@vinschen.de>
* sec_helper.cc (security_descriptor::free): If sd_size is 0, call
LocalFree instead of ::free.
* sec_acl.cc: Throughout replace old ACE flag definitions with current
definitions as used in MSDN man pages.
* security.cc: Ditto.
* fhandler.cc (fhandler_base::open): Make sure file has really been
just created before fixing file permissions. Add S_JUSTCREATED
attribute to set_file_attribute call.
* fhandler_disk_file.cc (fhandler_disk_file::mkdir): Always create dir
with default security descriptor and fix descriptor afterwards.
Add S_JUSTCREATED flag to set_file_attribute call.
* fhandler_socket.cc (fhandler_socket::bind): Ditto for AF_LOCAL
socket files.
* path.cc (symlink_worker): Ditto for symlinks.
* security.cc (get_file_sd): Call GetSecurityInfo rather than
NtQuerySecurityObject. Explain why. Change error handling accordingly.
(alloc_sd): Skip non-inherited, non-standard entries in ACL if
S_JUSTCREATED attribute is set. Explain why. Minor format fixes.
* security.h (S_JUSTCREATED): New define.
(security_descriptor::operator=): New operator.
2009-10-30 Corinna Vinschen <corinna@vinschen.de> 2009-10-30 Corinna Vinschen <corinna@vinschen.de>
* fhandler_random.cc (fhandler_dev_random::lseek): Revert change from * fhandler_random.cc (fhandler_dev_random::lseek): Revert change from

View File

@ -615,8 +615,8 @@ fhandler_base::open (int flags, mode_t mode)
in a domain as well as on standalone servers. in a domain as well as on standalone servers.
This is the result of a discussion on the samba-technical list, starting at This is the result of a discussion on the samba-technical list, starting at
http://lists.samba.org/archive/samba-technical/2008-July/060247.html */ http://lists.samba.org/archive/samba-technical/2008-July/060247.html */
if ((flags & O_CREAT) && has_acls ()) if (io.Information == FILE_CREATED && has_acls ())
set_file_attribute (fh, pc, ILLEGAL_UID, ILLEGAL_GID, mode); set_file_attribute (fh, pc, ILLEGAL_UID, ILLEGAL_GID, S_JUSTCREATED | mode);
set_io_handle (fh); set_io_handle (fh);
set_flags (flags, pc.binmode ()); set_flags (flags, pc.binmode ());

View File

@ -1463,14 +1463,6 @@ fhandler_disk_file::mkdir (mode_t mode)
{ {
int res = -1; int res = -1;
SECURITY_ATTRIBUTES sa = sec_none_nih; SECURITY_ATTRIBUTES sa = sec_none_nih;
security_descriptor sd;
/* 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; NTSTATUS status;
HANDLE dir; HANDLE dir;
OBJECT_ATTRIBUTES attr; OBJECT_ATTRIBUTES attr;
@ -1505,9 +1497,10 @@ fhandler_disk_file::mkdir (mode_t mode)
p, plen); p, plen);
if (NT_SUCCESS (status)) if (NT_SUCCESS (status))
{ {
if (has_acls () && pc.isremote ()) if (has_acls ())
set_file_attribute (dir, pc, ILLEGAL_UID, ILLEGAL_GID, set_file_attribute (dir, pc, ILLEGAL_UID, ILLEGAL_GID,
S_IFDIR | ((mode & 07777) & ~cygheap->umask)); S_JUSTCREATED | S_IFDIR
| ((mode & 07777) & ~cygheap->umask));
NtClose (dir); NtClose (dir);
res = 0; res = 0;
} }

View File

@ -890,15 +890,11 @@ fhandler_socket::bind (const struct sockaddr *name, int namelen)
if (!(mode & (S_IWUSR | S_IWGRP | S_IWOTH))) if (!(mode & (S_IWUSR | S_IWGRP | S_IWOTH)))
fattr |= FILE_ATTRIBUTE_READONLY; fattr |= FILE_ATTRIBUTE_READONLY;
SECURITY_ATTRIBUTES sa = sec_none_nih; SECURITY_ATTRIBUTES sa = sec_none_nih;
security_descriptor 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; NTSTATUS status;
HANDLE fh; HANDLE fh;
OBJECT_ATTRIBUTES attr; OBJECT_ATTRIBUTES attr;
IO_STATUS_BLOCK io; IO_STATUS_BLOCK io;
status = NtCreateFile (&fh, DELETE | FILE_GENERIC_WRITE, status = NtCreateFile (&fh, DELETE | FILE_GENERIC_WRITE,
pc.get_object_attr (attr, sa), &io, NULL, fattr, pc.get_object_attr (attr, sa), &io, NULL, fattr,
FILE_SHARE_VALID_FLAGS, FILE_CREATE, FILE_SHARE_VALID_FLAGS, FILE_CREATE,
@ -915,8 +911,9 @@ fhandler_socket::bind (const struct sockaddr *name, int namelen)
} }
else else
{ {
if (pc.has_acls () && pc.isremote ()) if (pc.has_acls ())
set_file_attribute (fh, pc, ILLEGAL_UID, ILLEGAL_GID, mode); set_file_attribute (fh, pc, ILLEGAL_UID, ILLEGAL_GID,
S_JUSTCREATED | mode);
char buf[sizeof (SOCKET_COOKIE) + 80]; char buf[sizeof (SOCKET_COOKIE) + 80];
__small_sprintf (buf, "%s%u %c ", SOCKET_COOKIE, sin.sin_port, __small_sprintf (buf, "%s%u %c ", SOCKET_COOKIE, sin.sin_port,
get_socket_type () == SOCK_STREAM ? 's' get_socket_type () == SOCK_STREAM ? 's'

View File

@ -1399,7 +1399,6 @@ symlink_worker (const char *oldpath, const char *newpath, bool use_winsym,
path_conv win32_newpath, win32_oldpath; path_conv win32_newpath, win32_oldpath;
char *buf, *cp; char *buf, *cp;
SECURITY_ATTRIBUTES sa = sec_none_nih; SECURITY_ATTRIBUTES sa = sec_none_nih;
security_descriptor sd;
OBJECT_ATTRIBUTES attr; OBJECT_ATTRIBUTES attr;
IO_STATUS_BLOCK io; IO_STATUS_BLOCK io;
NTSTATUS status; NTSTATUS status;
@ -1660,11 +1659,6 @@ symlink_worker (const char *oldpath, const char *newpath, bool use_winsym,
goto done; goto done;
} }
} }
/* 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, status = NtCreateFile (&fh, DELETE | FILE_GENERIC_WRITE,
win32_newpath.get_object_attr (attr, sa), win32_newpath.get_object_attr (attr, sa),
&io, NULL, FILE_ATTRIBUTE_NORMAL, &io, NULL, FILE_ATTRIBUTE_NORMAL,
@ -1679,9 +1673,10 @@ symlink_worker (const char *oldpath, const char *newpath, bool use_winsym,
__seterrno_from_nt_status (status); __seterrno_from_nt_status (status);
goto done; goto done;
} }
if (win32_newpath.has_acls () && win32_newpath.isremote ()) if (win32_newpath.has_acls ())
set_file_attribute (fh, win32_newpath, ILLEGAL_UID, ILLEGAL_GID, set_file_attribute (fh, win32_newpath, ILLEGAL_UID, ILLEGAL_GID,
S_IFLNK | STD_RBITS | STD_WBITS); (io.Information == FILE_CREATED ? S_JUSTCREATED : 0)
| S_IFLNK | STD_RBITS | STD_WBITS);
status = NtWriteFile (fh, NULL, NULL, NULL, &io, buf, cp - buf, NULL, NULL); status = NtWriteFile (fh, NULL, NULL, NULL, &io, buf, cp - buf, NULL, NULL);
if (NT_SUCCESS (status) && io.Information == (ULONG) (cp - buf)) if (NT_SUCCESS (status) && io.Information == (ULONG) (cp - buf))
{ {

View File

@ -123,7 +123,8 @@ setacl (HANDLE handle, path_conv &pc, int nentries, __aclent32_t *aclbufp,
allow |= FILE_DELETE_CHILD; allow |= FILE_DELETE_CHILD;
/* Set inherit property. */ /* Set inherit property. */
DWORD inheritance = (aclbufp[i].a_type & ACL_DEFAULT) DWORD inheritance = (aclbufp[i].a_type & ACL_DEFAULT)
? (SUB_CONTAINERS_AND_OBJECTS_INHERIT | INHERIT_ONLY) ? (CONTAINER_INHERIT_ACE | OBJECT_INHERIT_ACE
| INHERIT_ONLY_ACE)
: NO_INHERITANCE; : NO_INHERITANCE;
/* /*
* If a specific acl contains a corresponding default entry with * If a specific acl contains a corresponding default entry with
@ -138,7 +139,7 @@ setacl (HANDLE handle, path_conv &pc, int nentries, __aclent32_t *aclbufp,
? aclbufp[i].a_id : ILLEGAL_UID)) >= 0 ? aclbufp[i].a_id : ILLEGAL_UID)) >= 0
&& aclbufp[i].a_perm == aclbufp[i + 1 + pos].a_perm) && aclbufp[i].a_perm == aclbufp[i + 1 + pos].a_perm)
{ {
inheritance = SUB_CONTAINERS_AND_OBJECTS_INHERIT; inheritance = CONTAINER_INHERIT_ACE | OBJECT_INHERIT_ACE;
/* This invalidates the corresponding default entry. */ /* This invalidates the corresponding default entry. */
aclbufp[i + 1 + pos].a_type = USER|GROUP|ACL_DEFAULT; aclbufp[i + 1 + pos].a_type = USER|GROUP|ACL_DEFAULT;
} }
@ -365,12 +366,13 @@ getacl (HANDLE handle, path_conv &pc, int nentries, __aclent32_t *aclbufp)
if (!type) if (!type)
continue; continue;
if (!(ace->Header.AceFlags & INHERIT_ONLY || type & ACL_DEFAULT)) if (!(ace->Header.AceFlags & INHERIT_ONLY_ACE || type & ACL_DEFAULT))
{ {
if ((pos = searchace (lacl, MAX_ACL_ENTRIES, type, id)) >= 0) if ((pos = searchace (lacl, MAX_ACL_ENTRIES, type, id)) >= 0)
getace (lacl[pos], type, id, ace->Mask, ace->Header.AceType); getace (lacl[pos], type, id, ace->Mask, ace->Header.AceType);
} }
if ((ace->Header.AceFlags & SUB_CONTAINERS_AND_OBJECTS_INHERIT) if ((ace->Header.AceFlags
& (CONTAINER_INHERIT_ACE | OBJECT_INHERIT_ACE))
&& pc.isdir ()) && pc.isdir ())
{ {
if (type == USER_OBJ) if (type == USER_OBJ)

View File

@ -298,7 +298,12 @@ void
security_descriptor::free () security_descriptor::free ()
{ {
if (psd) if (psd)
::free (psd); {
if (!sd_size)
LocalFree (psd);
else
::free (psd);
}
psd = NULL; psd = NULL;
sd_size = 0; sd_size = 0;
} }

View File

@ -24,6 +24,7 @@ details. */
#include "cygheap.h" #include "cygheap.h"
#include "ntdll.h" #include "ntdll.h"
#include "pwdgrp.h" #include "pwdgrp.h"
#include <aclapi.h>
#define ALL_SECURITY_INFORMATION (DACL_SECURITY_INFORMATION \ #define ALL_SECURITY_INFORMATION (DACL_SECURITY_INFORMATION \
| GROUP_SECURITY_INFORMATION \ | GROUP_SECURITY_INFORMATION \
@ -32,8 +33,7 @@ details. */
LONG LONG
get_file_sd (HANDLE fh, path_conv &pc, security_descriptor &sd) get_file_sd (HANDLE fh, path_conv &pc, security_descriptor &sd)
{ {
NTSTATUS status = STATUS_SUCCESS; DWORD error = ERROR_SUCCESS;
ULONG len = 0;
int retry = 0; int retry = 0;
int res = -1; int res = -1;
@ -41,20 +41,17 @@ get_file_sd (HANDLE fh, path_conv &pc, security_descriptor &sd)
{ {
if (fh) if (fh)
{ {
status = NtQuerySecurityObject (fh, ALL_SECURITY_INFORMATION, /* Amazing but true. If you want to know if an ACE is inherited
sd, len, &len); from the parent object, you can't use the NtQuerySecurityObject
if (status == STATUS_BUFFER_TOO_SMALL) function. In the DACL returned by this functions, the
{ INHERITED_ACE flag is never set. Only by calling GetSecurityInfo
if (!sd.malloc (len)) you get this information. Oh well. */
{ PSECURITY_DESCRIPTOR psd;
set_errno (ENOMEM); error = GetSecurityInfo (fh, SE_FILE_OBJECT, ALL_SECURITY_INFORMATION,
break; NULL, NULL, NULL, NULL, &psd);
} if (error == ERROR_SUCCESS)
status = NtQuerySecurityObject (fh, ALL_SECURITY_INFORMATION,
sd, len, &len);
}
if (NT_SUCCESS (status))
{ {
sd = psd;
res = 0; res = 0;
break; break;
} }
@ -63,6 +60,7 @@ get_file_sd (HANDLE fh, path_conv &pc, security_descriptor &sd)
{ {
OBJECT_ATTRIBUTES attr; OBJECT_ATTRIBUTES attr;
IO_STATUS_BLOCK io; IO_STATUS_BLOCK io;
NTSTATUS status;
status = NtOpenFile (&fh, READ_CONTROL, status = NtOpenFile (&fh, READ_CONTROL,
pc.get_object_attr (attr, sec_none_nih), pc.get_object_attr (attr, sec_none_nih),
@ -71,14 +69,15 @@ get_file_sd (HANDLE fh, path_conv &pc, security_descriptor &sd)
if (!NT_SUCCESS (status)) if (!NT_SUCCESS (status))
{ {
fh = NULL; fh = NULL;
error = RtlNtStatusToDosError (status);
break; break;
} }
} }
} }
if (retry && fh) if (retry && fh)
NtClose (fh); NtClose (fh);
if (!NT_SUCCESS (status)) if (error != ERROR_SUCCESS)
__seterrno_from_nt_status (status); __seterrno_from_win_error (error);
return res; return res;
} }
@ -138,7 +137,7 @@ get_attribute_from_acl (mode_t *attribute, PACL acl, PSID owner_sid,
{ {
if (!GetAce (acl, i, (PVOID *) &ace)) if (!GetAce (acl, i, (PVOID *) &ace))
continue; continue;
if (ace->Header.AceFlags & INHERIT_ONLY) if (ace->Header.AceFlags & INHERIT_ONLY_ACE)
continue; continue;
switch (ace->Header.AceType) switch (ace->Header.AceType)
{ {
@ -386,6 +385,9 @@ alloc_sd (path_conv &pc, __uid32_t uid, __gid32_t gid, int attribute,
{ {
BOOL dummy; BOOL dummy;
/* NOTE: If the high bit of attribute is set, we have just created
a file or directory. See below for an explanation. */
debug_printf("uid %d, gid %d, attribute %x", uid, gid, attribute); debug_printf("uid %d, gid %d, attribute %x", uid, gid, attribute);
/* Get owner and group from current security descriptor. */ /* Get owner and group from current security descriptor. */
@ -583,15 +585,24 @@ alloc_sd (path_conv &pc, __uid32_t uid, __gid32_t gid, int attribute,
|| (ace_sid == group_sid) || (ace_sid == group_sid)
|| (ace_sid == well_known_world_sid)) || (ace_sid == well_known_world_sid))
{ {
if (ace->Header.AceFlags & SUB_CONTAINERS_AND_OBJECTS_INHERIT) if (ace->Header.AceFlags
ace->Header.AceFlags |= INHERIT_ONLY; & (CONTAINER_INHERIT_ACE | OBJECT_INHERIT_ACE))
ace->Header.AceFlags |= INHERIT_ONLY_ACE;
else else
continue; continue;
} }
else if ((attribute & S_JUSTCREATED)
&& !(ace->Header.AceFlags & INHERITED_ACE))
/* Since files and dirs are created with a NULL descriptor,
inheritence rules kick in. However, if no inheritable entries
exist in the parent object, Windows will create entries from the
user token's default DACL in the file DACL. These entries are
not desired and we drop them silently here. */
continue;
/* /*
* Add unrelated ACCESS_DENIED_ACE to the beginning but * Add unrelated ACCESS_DENIED_ACE to the beginning but
* behind the owner_deny, ACCESS_ALLOWED_ACE to the end. * behind the owner_deny, ACCESS_ALLOWED_ACE to the end.
* FIXME: this would break the order of the inherit_only ACEs * FIXME: this would break the order of the inherit-only ACEs
*/ */
if (!AddAce (acl, ACL_REVISION, if (!AddAce (acl, ACL_REVISION,
ace->Header.AceType == ACCESS_DENIED_ACE_TYPE? ace->Header.AceType == ACCESS_DENIED_ACE_TYPE?
@ -611,9 +622,10 @@ alloc_sd (path_conv &pc, __uid32_t uid, __gid32_t gid, int attribute,
impact. */ impact. */
/* Construct appropriate inherit attribute for new directories */ /* Construct appropriate inherit attribute for new directories */
if (S_ISDIR (attribute) && !acl_exists) if (S_ISDIR (attribute) && (attribute & S_JUSTCREATED))
{ {
const DWORD inherit = SUB_CONTAINERS_AND_OBJECTS_INHERIT | INHERIT_ONLY; const DWORD inherit = CONTAINER_INHERIT_ACE | OBJECT_INHERIT_ACE
| INHERIT_ONLY_ACE;
#if 0 /* FIXME: Not done currently as this breaks the canonical order */ #if 0 /* FIXME: Not done currently as this breaks the canonical order */
/* Set deny ACE for owner. */ /* Set deny ACE for owner. */
@ -630,7 +642,8 @@ alloc_sd (path_conv &pc, __uid32_t uid, __gid32_t gid, int attribute,
#endif #endif
/* Set allow ACE for owner. */ /* Set allow ACE for owner. */
if (!add_access_allowed_ace (acl, ace_off++, owner_allow, if (!add_access_allowed_ace (acl, ace_off++, owner_allow,
well_known_creator_owner_sid, acl_len, inherit)) well_known_creator_owner_sid, acl_len,
inherit))
return NULL; return NULL;
#if 0 /* FIXME: Not done currently as this breaks the canonical order and #if 0 /* FIXME: Not done currently as this breaks the canonical order and
won't be preserved on chown and chmod */ won't be preserved on chown and chmod */
@ -642,7 +655,8 @@ alloc_sd (path_conv &pc, __uid32_t uid, __gid32_t gid, int attribute,
#endif #endif
/* Set allow ACE for group. */ /* Set allow ACE for group. */
if (!add_access_allowed_ace (acl, ace_off++, group_allow, if (!add_access_allowed_ace (acl, ace_off++, group_allow,
well_known_creator_group_sid, acl_len, inherit)) well_known_creator_group_sid, acl_len,
inherit))
return NULL; return NULL;
/* Set allow ACE for everyone. */ /* Set allow ACE for everyone. */
if (!add_access_allowed_ace (acl, ace_off++, other_allow, if (!add_access_allowed_ace (acl, ace_off++, other_allow,

View File

@ -14,6 +14,10 @@ details. */
#include <accctrl.h> #include <accctrl.h>
/* Special file attribute set, for instance, in open() and mkdir() to
flag that a file has just been created. Used in alloc_sd, see there. */
#define S_JUSTCREATED 0x80000000
#define DEFAULT_UID DOMAIN_USER_RID_ADMIN #define DEFAULT_UID DOMAIN_USER_RID_ADMIN
#define UNKNOWN_UID 400 /* Non conflicting number */ #define UNKNOWN_UID 400 /* Non conflicting number */
#define UNKNOWN_GID 401 #define UNKNOWN_GID 401
@ -279,6 +283,7 @@ public:
} }
inline operator const PSECURITY_DESCRIPTOR () { return psd; } inline operator const PSECURITY_DESCRIPTOR () { return psd; }
inline operator PSECURITY_DESCRIPTOR *() { return &psd; } inline operator PSECURITY_DESCRIPTOR *() { return &psd; }
inline void operator =(PSECURITY_DESCRIPTOR nsd) { psd = nsd; }
}; };
class user_groups { class user_groups {