Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

msys2-runtime: restore fast path for current user primary group #57

Merged
merged 1 commit into from
Aug 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion winsup/cygwin/include/sys/cygwin.h
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,8 @@ enum
enum
{
NSS_SRC_FILES = 1,
NSS_SRC_DB = 2
NSS_SRC_DB = 2,
NSS_SRC_DB_ACCURATE = 4
};

/* Enumeration source constants for CW_SETENT called from mkpasswd/mkgroup. */
Expand Down
1 change: 1 addition & 0 deletions winsup/cygwin/local_includes/cygheap.h
Original file line number Diff line number Diff line change
Expand Up @@ -454,6 +454,7 @@ class cygheap_pwdgrp
inline int nss_pwd_src () const { return pwd_src; } /* CW_GETNSS_PWD_SRC */
inline bool nss_grp_files () const { return !!(grp_src & NSS_SRC_FILES); }
inline bool nss_grp_db () const { return !!(grp_src & NSS_SRC_DB); }
inline bool nss_grp_db_accurate () const { return !!(grp_src & NSS_SRC_DB_ACCURATE); }
inline int nss_grp_src () const { return grp_src; } /* CW_GETNSS_GRP_SRC */
inline bool nss_cygserver_caching () const { return caching; }
inline void nss_disable_cygserver_caching () { caching = false; }
Expand Down
30 changes: 24 additions & 6 deletions winsup/cygwin/uinfo.cc
Original file line number Diff line number Diff line change
Expand Up @@ -637,6 +637,11 @@ cygheap_pwdgrp::nss_init_line (const char *line)
*src |= NSS_SRC_DB;
c += 2;
}
else if (NSS_CMP ("db-accurate"))
{
*src |= NSS_SRC_DB | NSS_SRC_DB_ACCURATE;
c += 11;
}
else
{
c += strcspn (c, " \t");
Expand Down Expand Up @@ -1906,6 +1911,7 @@ pwdgrp::fetch_account_from_windows (fetch_user_arg_t &arg, cyg_ldap *pldap)
gid_t gid = ILLEGAL_GID;
bool is_domain_account = true;
PCWSTR domain = NULL;
bool get_default_group_from_current_user_token = false;
char *shell = NULL;
char *home = NULL;
char *gecos = NULL;
Expand Down Expand Up @@ -2371,9 +2377,19 @@ pwdgrp::fetch_account_from_windows (fetch_user_arg_t &arg, cyg_ldap *pldap)
uid = posix_offset + sid_sub_auth_rid (sid);
if (!is_group () && acc_type == SidTypeUser)
{
/* Default primary group. Make the educated guess that the user
is in group "Domain Users" or "None". */
gid = posix_offset + DOMAIN_GROUP_RID_USERS;
/* Default primary group. If the sid is the current user, and
we are not configured for accurate mode, fetch
the default group from the current user token, otherwise make
the educated guess that the user is in group "Domain Users"
or "None". */
if (!cygheap->pg.nss_grp_db_accurate() && sid == cygheap->user.sid ())
{
get_default_group_from_current_user_token = true;
gid = posix_offset
+ sid_sub_auth_rid (cygheap->user.groups.pgsid);
}
else
gid = posix_offset + DOMAIN_GROUP_RID_USERS;
}

if (is_domain_account)
Expand All @@ -2384,9 +2400,11 @@ pwdgrp::fetch_account_from_windows (fetch_user_arg_t &arg, cyg_ldap *pldap)
/* On AD machines, use LDAP to fetch domain account infos. */
if (cygheap->dom.primary_dns_name ())
{
/* Fetch primary group from AD and overwrite the one we
just guessed above. */
if (cldap->fetch_ad_account (sid, false, domain))
/* For the current user we got correctly cased username and
the primary group via process token. For any other user
we fetch it from AD and overwrite it. */
if (!get_default_group_from_current_user_token
&& cldap->fetch_ad_account (sid, false, domain))
{
if ((val = cldap->get_account_name ()))
wcscpy (name, val);
Expand Down
20 changes: 19 additions & 1 deletion winsup/doc/ntsec.xml
Original file line number Diff line number Diff line change
Expand Up @@ -930,7 +930,16 @@ The two lines starting with the keywords <literal>passwd:</literal> and
information from. <literal>files</literal> means, fetch the information
from the corresponding file in the /etc directory. <literal>db</literal>
means, fetch the information from the Windows account databases, the SAM
for local accounts, Active Directory for domain account. Examples:
for local accounts, Active Directory for domain account. For the current
user, the default group is obtained from the current user token to avoid
additional lookups to the group database. <literal>db-accuarte</literal>
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

db-accuarte --> db-accurate

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we do not build the documentation, this can easily be done in a (non-critical) follow-up PR. @placaze how about it?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for catching that @placaze . Created #58 to fix it.

is only valid on <literal>group:</literal> line, and performs the same
lookups as the <literal>db</literal> option, but disables using the
current user token to retrieve the default group as this optimization
is not accurate in all cases. For example, if you run a native process
with the primary group set to the Administrators builtin group, the
<literal>db</literal> option will return a non-existent group as primary
group. Examples:
</para>

<screen>
Expand All @@ -949,6 +958,15 @@ Read passwd entries only from /etc/passwd.
Read group entries only from SAM/AD.
</para>

<screen>
group: db-accurate
</screen>

<para>
Read group entries only from SAM/AD. Force the use of the group database
for the current user.
</para>

<screen>
group: files # db
</screen>
Expand Down