{"thread":{"id":"63189","subject":"[PATCH 0/2] mingw: special-case administrators even more","startedAt":"2025-03-25T10:38:33Z","lastAt":"2025-03-25T10:38:34Z","messageCount":3,"participants":["Johannes Schindelin via GitGitGadget"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"514987","messageId":"pull.1893.git.1742899110.gitgitgadget@gmail.com","threadId":"63189","inReplyTo":null,"subject":"[PATCH 0/2] mingw: special-case administrators even more","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2025-03-25T10:38:28Z","receivedAt":"2025-03-25T10:38:33Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"On Windows, a file created by a process running in elevated mode is owned by\nthe Administrators group (not by the user's account who would otherwise be\nable to modify or delete the file in non-elevated mode). Let's adjust the\n\"safe directory\" feature accordingly.\n\nNaturally, this patch series does not add a regression test (because it\ncan't, you cannot automate elevating processes).\n\nThis patch series is a companion of\nhttps://github.com/microsoft/git/pull/712.\n\nJohannes Schindelin (2):\n mingw: special-case administrators even more\n test-tool path-utils: support debugging \"dubious ownership\" issues\n\n compat/mingw.c | 39 +++++++++++++++++++++++++++-----------\n t/helper/test-path-utils.c | 19 +++++++++++++++++++\n 2 files changed, 47 insertions(+), 11 deletions(-)\n\n\nbase-commit: 683c54c999c301c2cd6f715c411407c413b1d84e\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1893%2Fdscho%2Fadmins-are-admins-on-windows-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1893/dscho/admins-are-admins-on-windows-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/1893\n-- \ngitgitgadget\n"},{"id":"514988","messageId":"90aec0788d075e11ae81fe192a2ac6a3b73728e5.1742899110.git.gitgitgadget@gmail.com","threadId":"63189","inReplyTo":"pull.1893.git.1742899110.gitgitgadget@gmail.com","subject":"[PATCH 1/2] mingw: special-case administrators even more","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2025-03-25T10:38:29Z","receivedAt":"2025-03-25T10:38:34Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nThe check for dubious ownership has one particular quirk on Windows: if\nrunning as an administrator, files owned by the Administrators _group_\nare considered owned by the user.\n\nThe rationale for that is: When running in elevated mode, Git creates\nfiles that aren't owned by the individual user but by the Administrators\ngroup.\n\nThere is yet another quirk, though: The check I introduced to determine\nwhether the current user is an administrator uses the\n`CheckTokenMembership()` function with the current process token. And\nthat check only succeeds when running in elevated mode!\n\nLet's be a bit more lenient here and look harder whether the current\nuser is an administrator. We do this by looking for a so-called \"linked\ntoken\". That token exists when administrators run in non-elevated mode,\nand can be used to create a new process in elevated mode. And feeding\n_that_ token to the `CheckTokenMembership()` function succeeds!\n\nSigned-off-by: Johannes Schindelin \n---\n compat/mingw.c | 39 ++++++++++++++++++++++++++++-----------\n 1 file changed, 28 insertions(+), 11 deletions(-)\n\ndiff --git a/compat/mingw.c b/compat/mingw.c\nindex f524c54d06d..305a999f1fc 100644\n--- a/compat/mingw.c\n+++ b/compat/mingw.c\n@@ -2826,31 +2826,44 @@ static void setup_windows_environment(void)\n \t}\n }\n \n-static PSID get_current_user_sid(void)\n+static void get_current_user_sid(PSID *sid, HANDLE *linked_token)\n {\n \tHANDLE token;\n \tDWORD len = 0;\n-\tPSID result = NULL;\n+\tTOKEN_ELEVATION_TYPE elevationType;\n+\tDWORD size;\n+\n+\t*sid = NULL;\n+\t*linked_token = NULL;\n \n \tif (!OpenProcessToken(GetCurrentProcess(), TOKEN_QUERY, &token))\n-\t\treturn NULL;\n+\t\treturn;\n \n \tif (!GetTokenInformation(token, TokenUser, NULL, 0, &len)) {\n \t\tTOKEN_USER *info = xmalloc((size_t)len);\n \t\tif (GetTokenInformation(token, TokenUser, info, len, &len)) {\n \t\t\tlen = GetLengthSid(info->User.Sid);\n-\t\t\tresult = xmalloc(len);\n-\t\t\tif (!CopySid(len, result, info->User.Sid)) {\n+\t\t\t*sid = xmalloc(len);\n+\t\t\tif (!CopySid(len, *sid, info->User.Sid)) {\n \t\t\t\terror(_(\"failed to copy SID (%ld)\"),\n \t\t\t\t GetLastError());\n-\t\t\t\tFREE_AND_NULL(result);\n+\t\t\t\tFREE_AND_NULL(*sid);\n \t\t\t}\n \t\t}\n \t\tFREE_AND_NULL(info);\n \t}\n-\tCloseHandle(token);\n \n-\treturn result;\n+\tif (GetTokenInformation(token, TokenElevationType, &elevationType, sizeof(elevationType), &size) &&\n+\t elevationType == TokenElevationTypeLimited) {\n+\t\t/*\n+\t\t * The current process is run by a member of the Administrators\n+\t\t * group, but is not running elevated.\n+\t\t */\n+\t\tif (!GetTokenInformation(token, TokenLinkedToken, linked_token, sizeof(*linked_token), &size))\n+\t\t\tlinked_token = NULL; /* there is no linked token */\n+\t}\n+\n+\tCloseHandle(token);\n }\n \n static BOOL user_sid_to_user_name(PSID sid, LPSTR *str)\n@@ -2931,18 +2944,22 @@ int is_path_owned_by_current_sid(const char *path, struct strbuf *report)\n \telse if (sid && IsValidSid(sid)) {\n \t\t/* Now, verify that the SID matches the current user's */\n \t\tstatic PSID current_user_sid;\n+\t\tstatic HANDLE linked_token;\n \t\tBOOL is_member;\n \n \t\tif (!current_user_sid)\n-\t\t\tcurrent_user_sid = get_current_user_sid();\n+\t\t\tget_current_user_sid(¤t_user_sid, &linked_token);\n \n \t\tif (current_user_sid &&\n \t\t IsValidSid(current_user_sid) &&\n \t\t EqualSid(sid, current_user_sid))\n \t\t\tresult = 1;\n \t\telse if (IsWellKnownSid(sid, WinBuiltinAdministratorsSid) &&\n-\t\t\t CheckTokenMembership(NULL, sid, &is_member) &&\n-\t\t\t is_member)\n+\t\t\t ((CheckTokenMembership(NULL, sid, &is_member) &&\n+\t\t\t is_member) ||\n+\t\t\t (linked_token &&\n+\t\t\t CheckTokenMembership(linked_token, sid, &is_member) &&\n+\t\t\t is_member)))\n \t\t\t/*\n \t\t\t * If owned by the Administrators group, and the\n \t\t\t * current user is an administrator, we consider that\n-- \ngitgitgadget\n\n"},{"id":"514989","messageId":"1daac3f08fae41a967d52d510128052c6a1829a8.1742899110.git.gitgitgadget@gmail.com","threadId":"63189","inReplyTo":"pull.1893.git.1742899110.gitgitgadget@gmail.com","subject":"[PATCH 2/2] test-tool path-utils: support debugging \"dubious ownership\" issues","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2025-03-25T10:38:30Z","receivedAt":"2025-03-25T10:38:34Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nThis adds a new sub-sub-command for `test-tool`, simply passing through\nthe command-line arguments to the `is_path_owned_by_current_user()`\nfunction.\n\nSigned-off-by: Johannes Schindelin \n---\n t/helper/test-path-utils.c | 19 +++++++++++++++++++\n 1 file changed, 19 insertions(+)\n\ndiff --git a/t/helper/test-path-utils.c b/t/helper/test-path-utils.c\nindex 72ac8d1b1b0..f3c59e50285 100644\n--- a/t/helper/test-path-utils.c\n+++ b/t/helper/test-path-utils.c\n@@ -504,6 +504,25 @@ int cmd__path_utils(int argc, const char **argv)\n \t\treturn !!res;\n \t}\n \n+\tif (argc > 1 && !strcmp(argv[1], \"is_path_owned_by_current_user\")) {\n+\t\tint res = 0;\n+\n+\t\tfor (int i = 2; i < argc; i++) {\n+\t\t\tstruct strbuf buf = STRBUF_INIT;\n+\n+\t\t\tif (is_path_owned_by_current_user(argv[i], &buf))\n+\t\t\t\tprintf(\"'%s' is owned by current SID\\n\", argv[i]);\n+\t\t\telse {\n+\t\t\t\tprintf(\"'%s' is not owned by current SID: %s\\n\", argv[i], buf.buf);\n+\t\t\t\tres = 1;\n+\t\t\t}\n+\n+\t\t\tstrbuf_release(&buf);\n+\t\t}\n+\n+\t\treturn res;\n+\t}\n+\n \tfprintf(stderr, \"%s: unknown function name: %s\\n\", argv[0],\n \t\targv[1] ? argv[1] : \"(there was none)\");\n \treturn 1;\n-- \ngitgitgadget\n"}]}