mingw: special-case administrators even more
cover mingw: special-case administrators even more
On Windows, a file created by a process running in elevated mode is owned by the Administrators group (not by the user's account who would otherwise be able to modify or delete the file in non-elevated mode). Let's adjust the "safe directory" feature accordingly.
Naturally, this patch series does not add a regression test (because it can't, you cannot automate elevating processes).
This patch series is a companion of
https://github.com/microsoft/git/pull/712.
Johannes Schindelin (2):
mingw: special-case administrators even more
test-tool path-utils: support debugging "dubious ownership" issues
compat/mingw.c | 39 +++++++++++++++++++++++++++----------- t/helper/test-path-utils.c | 19 +++++++++++++++++++ 2 files changed, 47 insertions(+), 11 deletions(-)
base-commit: 683c54c999c301c2cd6f715c411407c413b1d84e
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1893%2Fdscho%2Fadmins-are-admins-on-windows-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1893/dscho/admins-are-admins-on-windows-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1893
Signature · 1 line
gitgitgadget
PATCH 1/2 mingw: special-case administrators even more
From: Johannes Schindelin <[email protected]>
The check for dubious ownership has one particular quirk on Windows: if running as an administrator, files owned by the Administrators _group_ are considered owned by the user.
The rationale for that is: When running in elevated mode, Git creates files that aren't owned by the individual user but by the Administrators group.
There is yet another quirk, though: The check I introduced to determine whether the current user is an administrator uses the CheckTokenMembership() function with the current process token. And that check only succeeds when running in elevated mode!
Let's be a bit more lenient here and look harder whether the current user is an administrator. We do this by looking for a so-called "linked token". That token exists when administrators run in non-elevated mode, and can be used to create a new process in elevated mode. And feeding _that_ token to the CheckTokenMembership() function succeeds!
--- compat/mingw.c | 39 ++++++++++++++++++++++++++++----------- 1 file changed, 28 insertions(+), 11 deletions(-)
compat/mingw.c +28 -11
@@ -2826,31 +2826,44 @@ static void setup_windows_environment(void) } } -static PSID get_current_user_sid (void ) +static void get_current_user_sid (PSID *sid,HANDLE *linked_token) {HANDLE token;DWORD len =0 ; -PSID result =NULL ; +TOKEN_ELEVATION_TYPE elevationType; +DWORD size; + + *sid =NULL ; + *linked_token =NULL ;if (!OpenProcessToken (GetCurrentProcess (),TOKEN_QUERY , &token)) -return NULL ; +return ;if (!GetTokenInformation (token, TokenUser,NULL ,0 , &len)) {TOKEN_USER *info =xmalloc ((size_t )len);if (GetTokenInformation (token, TokenUser, info, len, &len)) { len =GetLengthSid (info->User.Sid); - result =xmalloc (len); -if (!CopySid (len, result, info->User.Sid)) { + *sid =xmalloc (len); +if (!CopySid (len, *sid, info->User.Sid)) {error (_ ("failed to copy SID (%ld)" ),GetLastError ()); -FREE_AND_NULL (result); +FREE_AND_NULL (*sid); } }FREE_AND_NULL (info); } -CloseHandle (token); -return result; +if (GetTokenInformation (token, TokenElevationType, &elevationType,sizeof (elevationType), &size) && + elevationType == TokenElevationTypeLimited) { +/* +* The current process is run by a member of the Administrators +* group, but is not running elevated. +*/ +if (!GetTokenInformation (token, TokenLinkedToken, linked_token,sizeof (*linked_token), &size)) + linked_token =NULL ;/* there is no linked token */ + } + +CloseHandle (token); }static BOOL user_sid_to_user_name (PSID sid,LPSTR *str)
@@ -2931,18 +2944,22 @@ int is_path_owned_by_current_sid(const char *path, struct strbuf *report)else if (sid &&IsValidSid (sid)) {/* Now, verify that the SID matches the current user's */ static PSID current_user_sid; +static HANDLE linked_token;BOOL is_member;if (!current_user_sid) - current_user_sid =get_current_user_sid (); +get_current_user_sid (¤t_user_sid, &linked_token);if (current_user_sid &&IsValidSid (current_user_sid) &&EqualSid (sid, current_user_sid)) result =1 ;else if (IsWellKnownSid (sid, WinBuiltinAdministratorsSid) && -CheckTokenMembership (NULL , sid, &is_member) && - is_member) + ((CheckTokenMembership (NULL , sid, &is_member) && + is_member) || + (linked_token && +CheckTokenMembership (linked_token, sid, &is_member) && + is_member)))/* * If owned by the Administrators group, and the * current user is an administrator, we consider that
Signature · 1 line
gitgitgadget
PATCH 2/2 test-tool path-utils: support debugging "dubious ownership" issues
In reply to Johannes Schindelin via GitGitGadgetFrom: Johannes Schindelin <[email protected]>
This adds a new sub-sub-command for test-tool, simply passing through the command-line arguments to the is_path_owned_by_current_user() function.
--- t/helper/test-path-utils.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+)
t/helper/test-path-utils.c +19 -0
@@ -504,6 +504,25 @@ int cmd__path_utils(int argc, const char **argv)return !!res; } +if (argc >1 && !strcmp (argv[1 ],"is_path_owned_by_current_user" )) { +int res =0 ; + +for (int i =2 ; i < argc; i++) { +struct strbuf buf =STRBUF_INIT ; + +if (is_path_owned_by_current_user (argv[i], &buf)) +printf ("'%s' is owned by current SID\n" , argv[i]); +else { +printf ("'%s' is not owned by current SID: %s\n" , argv[i], buf.buf); + res =1 ; + } + +strbuf_release (&buf); + } + +return res; + } +fprintf (stderr,"%s: unknown function name: %s\n" , argv[0 ], argv[1 ] ? argv[1 ] :"(there was none)" );return 1 ;
Signature · 1 line
gitgitgadget