* sec_acl.cc (setacl): Fix bug which leads to ACE duplication in
case owner SID == group SID. (getacl): Reverse order of SID test against group or owner sid to prefer owner attributes over group attributes. Disable setting group permissions equivalent to owner permissions if owner == group. Add comment to explain why. Fix indentation. * security.cc (get_attribute_from_acl): Change type of local variables containing permission to mode_t. Apply deny mask to group if group SID == owner SID to avoid Everyone permissions to spill over into group permissions. Disable setting group permissions equivalent to owner permissions if owner == group. Add comment to explain why. * uinfo.cc (pwdgrp::fetch_account_from_windows): Allow user SID as group account if user is a "Microsoft Account". Explain why. Drop workaround enforcing primary group "Users" for "Microsoft Accounts".
This commit is contained in:
Corinna Vinschen
parent
d2f9dbb3ee
commit
06371539bd
|
|
@ -1,3 +1,20 @@
|
|||
2015-02-27 Corinna Vinschen <corinna@vinschen.de>
|
||||
|
||||
* sec_acl.cc (setacl): Fix bug which leads to ACE duplication in
|
||||
case owner SID == group SID.
|
||||
(getacl): Reverse order of SID test against group or owner sid to
|
||||
prefer owner attributes over group attributes. Disable setting group
|
||||
permissions equivalent to owner permissions if owner == group. Add
|
||||
comment to explain why. Fix indentation.
|
||||
* security.cc (get_attribute_from_acl): Change type of local variables
|
||||
containing permission to mode_t. Apply deny mask to group if group SID
|
||||
== owner SID to avoid Everyone permissions to spill over into group
|
||||
permissions. Disable setting group permissions equivalent to owner
|
||||
permissions if owner == group. Add comment to explain why.
|
||||
* uinfo.cc (pwdgrp::fetch_account_from_windows): Allow user SID as
|
||||
group account if user is a "Microsoft Account". Explain why. Drop
|
||||
workaround enforcing primary group "Users" for "Microsoft Accounts".
|
||||
|
||||
2015-02-26 Corinna Vinschen <corinna@vinschen.de>
|
||||
|
||||
* ldap.cc (cyg_ldap::wait): Call cygwait with cw_infinite timeout value
|
||||
|
|
|
|||
|
|
@ -169,7 +169,7 @@ setacl (HANDLE handle, path_conv &pc, int nentries, aclent_t *aclbufp,
|
|||
*allow |= FILE_DELETE_CHILD;
|
||||
invalid[i] = true;
|
||||
}
|
||||
bool isownergroup = (owner_sid == group_sid);
|
||||
bool isownergroup = (owner == group);
|
||||
DWORD owner_deny = ~owner_allow & (group_allow | other_allow);
|
||||
owner_deny &= ~(STANDARD_RIGHTS_READ
|
||||
| FILE_READ_ATTRIBUTES | FILE_WRITE_ATTRIBUTES);
|
||||
|
|
@ -179,27 +179,27 @@ setacl (HANDLE handle, path_conv &pc, int nentries, aclent_t *aclbufp,
|
|||
/* Set deny ACE for owner. */
|
||||
if (owner_deny
|
||||
&& !add_access_denied_ace (acl, ace_off++, owner_deny,
|
||||
owner_sid, acl_len, NO_INHERITANCE))
|
||||
owner, acl_len, NO_INHERITANCE))
|
||||
return -1;
|
||||
/* Set deny ACE for group here to respect the canonical order,
|
||||
if this does not impact owner */
|
||||
if (group_deny && !(group_deny & owner_allow) && !isownergroup
|
||||
&& !add_access_denied_ace (acl, ace_off++, group_deny,
|
||||
group_sid, acl_len, NO_INHERITANCE))
|
||||
group, acl_len, NO_INHERITANCE))
|
||||
return -1;
|
||||
/* Set allow ACE for owner. */
|
||||
if (!add_access_allowed_ace (acl, ace_off++, owner_allow,
|
||||
owner_sid, acl_len, NO_INHERITANCE))
|
||||
owner, acl_len, NO_INHERITANCE))
|
||||
return -1;
|
||||
/* Set deny ACE for group, if still needed. */
|
||||
if (group_deny & owner_allow && !isownergroup
|
||||
&& !add_access_denied_ace (acl, ace_off++, group_deny,
|
||||
group_sid, acl_len, NO_INHERITANCE))
|
||||
group, acl_len, NO_INHERITANCE))
|
||||
return -1;
|
||||
/* Set allow ACE for group. */
|
||||
if (!isownergroup
|
||||
&& !add_access_allowed_ace (acl, ace_off++, group_allow,
|
||||
group_sid, acl_len, NO_INHERITANCE))
|
||||
group, acl_len, NO_INHERITANCE))
|
||||
return -1;
|
||||
/* Set allow ACE for everyone. */
|
||||
if (!add_access_allowed_ace (acl, ace_off++, other_allow,
|
||||
|
|
@ -451,16 +451,16 @@ getacl (HANDLE handle, path_conv &pc, int nentries, aclent_t *aclbufp)
|
|||
type = OTHER_OBJ;
|
||||
id = ILLEGAL_GID;
|
||||
}
|
||||
else if (ace_sid == group_sid)
|
||||
{
|
||||
type = GROUP_OBJ;
|
||||
id = gid;
|
||||
}
|
||||
else if (ace_sid == owner_sid)
|
||||
{
|
||||
type = USER_OBJ;
|
||||
id = uid;
|
||||
}
|
||||
else if (ace_sid == group_sid)
|
||||
{
|
||||
type = GROUP_OBJ;
|
||||
id = gid;
|
||||
}
|
||||
else if (ace_sid == well_known_creator_group_sid)
|
||||
{
|
||||
type = DEF_GROUP_OBJ;
|
||||
|
|
@ -563,19 +563,26 @@ getacl (HANDLE handle, path_conv &pc, int nentries, aclent_t *aclbufp)
|
|||
}
|
||||
if ((pos = searchace (lacl, MAX_ACL_ENTRIES, 0)) < 0)
|
||||
pos = MAX_ACL_ENTRIES;
|
||||
if (aclbufp) {
|
||||
if (owner_sid == group_sid)
|
||||
lacl[0].a_perm = lacl[1].a_perm;
|
||||
if (pos > nentries)
|
||||
{
|
||||
set_errno (ENOSPC);
|
||||
return -1;
|
||||
}
|
||||
memcpy (aclbufp, lacl, pos * sizeof (aclent_t));
|
||||
for (i = 0; i < pos; ++i)
|
||||
aclbufp[i].a_perm &= ~(DENY_R | DENY_W | DENY_X);
|
||||
aclsort32 (pos, 0, aclbufp);
|
||||
}
|
||||
if (aclbufp)
|
||||
{
|
||||
#if 0
|
||||
/* Disable owner/group permissions equivalence if owner SID == group SID.
|
||||
It's technically not quite correct, but it helps in case a security
|
||||
conscious application checks if a file has too open permissions. In
|
||||
fact, since owner == group, there's no security issue here. */
|
||||
if (owner_sid == group_sid)
|
||||
lacl[1].a_perm = lacl[0].a_perm;
|
||||
#endif
|
||||
if (pos > nentries)
|
||||
{
|
||||
set_errno (ENOSPC);
|
||||
return -1;
|
||||
}
|
||||
memcpy (aclbufp, lacl, pos * sizeof (aclent_t));
|
||||
for (i = 0; i < pos; ++i)
|
||||
aclbufp[i].a_perm &= ~(DENY_R | DENY_W | DENY_X);
|
||||
aclsort32 (pos, 0, aclbufp);
|
||||
}
|
||||
syscall_printf ("%R = getacl(%S)", pos, pc.get_nt_native_path ());
|
||||
return pos;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -239,9 +239,9 @@ get_attribute_from_acl (mode_t *attribute, PACL acl, PSID owner_sid,
|
|||
PSID group_sid, bool grp_member)
|
||||
{
|
||||
ACCESS_ALLOWED_ACE *ace;
|
||||
int allow = 0;
|
||||
int deny = 0;
|
||||
int *flags, *anti;
|
||||
mode_t allow = 0;
|
||||
mode_t deny = 0;
|
||||
mode_t *flags, *anti;
|
||||
|
||||
for (DWORD i = 0; i < acl->AceCount; ++i)
|
||||
{
|
||||
|
|
@ -301,6 +301,17 @@ get_attribute_from_acl (mode_t *attribute, PACL acl, PSID owner_sid,
|
|||
*flags |= ((!(*anti & S_IWUSR)) ? S_IWUSR : 0);
|
||||
if (ace->Mask & FILE_EXEC_BITS)
|
||||
*flags |= ((!(*anti & S_IXUSR)) ? S_IXUSR : 0);
|
||||
/* Apply deny mask to group if group SID == owner SID. */
|
||||
if (group_sid && RtlEqualSid (owner_sid, group_sid)
|
||||
&& ace->Header.AceType == ACCESS_DENIED_ACE_TYPE)
|
||||
{
|
||||
if (ace->Mask & FILE_READ_BITS)
|
||||
*flags |= ((!(*anti & S_IRUSR)) ? S_IRGRP : 0);
|
||||
if (ace->Mask & FILE_WRITE_BITS)
|
||||
*flags |= ((!(*anti & S_IWUSR)) ? S_IWGRP : 0);
|
||||
if (ace->Mask & FILE_EXEC_BITS)
|
||||
*flags |= ((!(*anti & S_IXUSR)) ? S_IXGRP : 0);
|
||||
}
|
||||
}
|
||||
else if (ace_sid == group_sid)
|
||||
{
|
||||
|
|
@ -331,6 +342,11 @@ get_attribute_from_acl (mode_t *attribute, PACL acl, PSID owner_sid,
|
|||
}
|
||||
}
|
||||
*attribute &= ~(S_IRWXU | S_IRWXG | S_IRWXO | S_ISVTX | S_ISGID | S_ISUID);
|
||||
#if 0
|
||||
/* Disable owner/group permissions equivalence if owner SID == group SID.
|
||||
It's technically not quite correct, but it helps in case a security
|
||||
conscious application checks if a file has too open permissions. In
|
||||
fact, since owner == group, there's no security issue here. */
|
||||
if (owner_sid && group_sid && RtlEqualSid (owner_sid, group_sid)
|
||||
/* FIXME: temporary exception for /var/empty */
|
||||
&& well_known_system_sid != group_sid)
|
||||
|
|
@ -340,6 +356,7 @@ get_attribute_from_acl (mode_t *attribute, PACL acl, PSID owner_sid,
|
|||
| ((allow & S_IWUSR) ? S_IWGRP : 0)
|
||||
| ((allow & S_IXUSR) ? S_IXGRP : 0));
|
||||
}
|
||||
#endif
|
||||
*attribute |= allow;
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -2053,12 +2053,31 @@ pwdgrp::fetch_account_from_windows (fetch_user_arg_t &arg, cyg_ldap *pldap)
|
|||
switch (acc_type)
|
||||
{
|
||||
case SidTypeUser:
|
||||
/* Don't allow users as group. While this is technically possible,
|
||||
it doesn't make sense in a POSIX scenario. It *is* used for
|
||||
Microsoft Accounts, but those are converted to well-known groups
|
||||
above. */
|
||||
if (is_group ())
|
||||
return NULL;
|
||||
if (is_group () && acc_type == SidTypeUser)
|
||||
{
|
||||
/* Don't allow users as group. While this is technically
|
||||
possible, it doesn't make sense in a POSIX scenario.
|
||||
|
||||
And then there are the so-called Microsoft Accounts. The
|
||||
special SID with security authority 11 is converted to a
|
||||
well known group above, but additionally, when logging in
|
||||
with such an account, the user's primary group SID is the
|
||||
user's SID. Those we let pass, but no others. */
|
||||
bool its_ok = false;
|
||||
if (wincap.has_microsoft_accounts ())
|
||||
{
|
||||
struct cyg_USER_INFO_24 *ui24;
|
||||
if (NetUserGetInfo (NULL, name, 24, (PBYTE *) &ui24)
|
||||
== NERR_Success)
|
||||
{
|
||||
if (ui24->usri24_internet_identity)
|
||||
its_ok = true;
|
||||
NetApiBufferFree (ui24);
|
||||
}
|
||||
}
|
||||
if (!its_ok)
|
||||
return NULL;
|
||||
}
|
||||
/*FALLTHRU*/
|
||||
case SidTypeGroup:
|
||||
case SidTypeAlias:
|
||||
|
|
@ -2231,25 +2250,6 @@ pwdgrp::fetch_account_from_windows (fetch_user_arg_t &arg, cyg_ldap *pldap)
|
|||
debug_printf ("NetUserGetInfo(%W) %u", name, nas);
|
||||
break;
|
||||
}
|
||||
/* Logging in with a Microsoft Account, the user's primary
|
||||
group SID is the user's SID. Security sensitive tools
|
||||
expecting tight file permissions choke on that. We need
|
||||
an explicit primary group which is not identical to the
|
||||
user account. Unfortunately, while the default primary
|
||||
group of the account in SAM is still "None", "None" is not
|
||||
in the user token group list. So, what we do here is to
|
||||
use "Users" as a sane default primary group instead. */
|
||||
if (wincap.has_microsoft_accounts ())
|
||||
{
|
||||
struct cyg_USER_INFO_24 *ui24;
|
||||
nas = NetUserGetInfo (NULL, name, 24, (PBYTE *) &ui24);
|
||||
if (nas == NERR_Success)
|
||||
{
|
||||
if (ui24->usri24_internet_identity)
|
||||
gid = DOMAIN_ALIAS_RID_USERS;
|
||||
NetApiBufferFree (ui24);
|
||||
}
|
||||
}
|
||||
/* Fetch user attributes. */
|
||||
home = cygheap->pg.get_home (ui, sid, dom, name,
|
||||
fully_qualified_name);
|
||||
|
|
|
|||
Loading…
Reference in New Issue