|
| 1 | +From 1a222c8a01c2981a9343611c0d3a6afdff55d87b Mon Sep 17 00:00:00 2001 |
| 2 | +From: Johannes Schindelin < [email protected]> |
| 3 | +Date: Tue, 3 Jun 2025 08:53:57 +0200 |
| 4 | +Subject: [PATCH 48/N] Cygwin: do retrieve AzureAD users' information again |
| 5 | + |
| 6 | +In 48e7d63268 (Cygwin: fetch_account_from_windows: skip LookupAccountSid |
| 7 | +for SIDs known to fail, 2025-04-10), several SIDs acquired a shortcut |
| 8 | +where a potentially expensive `LookupAccountSid()` call is avoided for |
| 9 | +SIDs that "cannot be resolved". |
| 10 | + |
| 11 | +However, as reported by Robert Fensterman (and independently discovered |
| 12 | +by myself), some of the SIDs that received this special shortcut _do_ |
| 13 | +get resolved by `LookupAccountSid()` calls: AzureAD users' SIDs. |
| 14 | + |
| 15 | +With those SIDs, that newly-introduced shortcut actually does more harm |
| 16 | +than good because there is no other way to retrieve the desired |
| 17 | +information, resulting in permission problems. |
| 18 | + |
| 19 | +One symptom of this is that `mintty` can no longer access `/dev/ptmx` |
| 20 | +and simply errors out with "Error: Could not fork child process: There |
| 21 | +are no available terminals (-1)". |
| 22 | + |
| 23 | +Another symptom is that `tmux` is no longer able to create new sessions. |
| 24 | +Yet another symptom is new files are unintentionally written with |
| 25 | +restricted permissions (copying an `.exe` file, for example, disallows |
| 26 | +the copied version to be executed). |
| 27 | + |
| 28 | +The most likely reason why AzureAD SIDs were included in above-mentioned |
| 29 | +commit is that special AzureAD _group_ SIDs are not recognized by |
| 30 | +`LookupAccountSid()`, as per the code comment for the `azure_grp_sid` |
| 31 | +variable. It is plausible that this fact was mistaken to extend to all |
| 32 | +AzureAD SIDs, a notion disproved by the counter example of my personal |
| 33 | +experience with my own AzureAD user account. Unfortunately, the only way |
| 34 | +to find out whether `LookupAccountSid()` works with a given AzureAD SID |
| 35 | +or not is to call that function. |
| 36 | + |
| 37 | +To make regular AzureAD user accounts work again, let's just drop the |
| 38 | +AzureAD part from that special shortcut. |
| 39 | + |
| 40 | +My understanding of the other SIDs handled by that shortcut (Capability |
| 41 | +SIDs, IIS APPPOOL and Samba user/group SIDs) is insufficient to |
| 42 | +determine whether they, too, can be resolved by `LookupAccountSid()` in |
| 43 | +some cases (and would therefore equally need to be excluded from that |
| 44 | +shortcut). At least as far as the Capability SIDs go, I am rather |
| 45 | +confident from reading the context (the commit's message, as well as the |
| 46 | +report that led to that commit) that the shortcut is safe, and I could |
| 47 | +imagine that the same is true for IIS APPPOOL and Samba SIDs. Absent any |
| 48 | +further insight, I therefore decided to leave the rest of 48e7d63268 |
| 49 | +(Cygwin: fetch_account_from_windows: skip LookupAccountSid for SIDs |
| 50 | +known to fail, 2025-04-10) intact. |
| 51 | + |
| 52 | +Reported-by: Robert Fensterman < [email protected]> |
| 53 | +Fixes: 48e7d63268 (Cygwin: fetch_account_from_windows: skip LookupAccountSid for SIDs known to fail, 2025-04-10) |
| 54 | +Signed-off-by: Johannes Schindelin < [email protected]> |
| 55 | +--- |
| 56 | + winsup/cygwin/uinfo.cc | 4 ---- |
| 57 | + 1 file changed, 4 deletions(-) |
| 58 | + |
| 59 | +diff --git a/winsup/cygwin/uinfo.cc b/winsup/cygwin/uinfo.cc |
| 60 | +index 7c2581d..4323cb1 100644 |
| 61 | +--- a/winsup/cygwin/uinfo.cc |
| 62 | ++++ b/winsup/cygwin/uinfo.cc |
| 63 | +@@ -1996,10 +1996,6 @@ pwdgrp::fetch_account_from_windows (fetch_user_arg_t &arg, cyg_ldap *pldap) |
| 64 | + if (sid_id_auth (sid) == 5 /* SECURITY_NT_AUTHORITY */ |
| 65 | + && sid_sub_auth (sid, 0) == SECURITY_APPPOOL_ID_BASE_RID) |
| 66 | + break; |
| 67 | +- /* AzureAD SIDs */ |
| 68 | +- if (sid_id_auth (sid) == 12 /* AzureAD ID */ |
| 69 | +- && sid_sub_auth (sid, 0) == 1 /* Azure ID base RID */) |
| 70 | +- break; |
| 71 | + /* Samba user/group SIDs */ |
| 72 | + if (sid_id_auth (sid) == 22) |
| 73 | + break; |
0 commit comments