Support AzureAD users again #98
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
In 48e7d63 (Cygwin: fetch_account_from_windows: skip LookupAccountSid for SIDs known to fail, 2025-04-10), several SIDs acquired a shortcut where a potentially expensive `LookupAccountSid()` call is avoided for SIDs that "cannot be resolved". However, as reported by Robert Fensterman (and independently discovered by myself), some of the SIDs that received this special shortcut _do_ get resolved by `LookupAccountSid()` calls: AzureAD users' SIDs. With those SIDs, that newly-introduced shortcut actually does more harm than good because there is no other way to retrieve the desired information, resulting in permission problems. One symptom of this is that `mintty` can no longer access `/dev/ptmx` and simply errors out with "Error: Could not fork child process: There are no available terminals (-1)". Another symptom is that `tmux` is no longer able to create new sessions. Yet another symptom is new files are unintentionally written with restricted permissions (copying an `.exe` file, for example, disallows the copied version to be executed). The most likely reason why AzureAD SIDs were included in above-mentioned commit is that special AzureAD _group_ SIDs are not recognized by `LookupAccountSid()`, as per the code comment for the `azure_grp_sid` variable. It is plausible that this fact was mistaken to extend to all AzureAD SIDs, a notion disproved by the counter example of my personal experience with my own AzureAD user account. Unfortunately, the only way to find out whether `LookupAccountSid()` works with a given AzureAD SID or not is to call that function. To make regular AzureAD user accounts work again, let's just drop the AzureAD part from that special shortcut. My understanding of the other SIDs handled by that shortcut (Capability SIDs, IIS APPPOOL and Samba user/group SIDs) is insufficient to determine whether they, too, can be resolved by `LookupAccountSid()` in some cases (and would therefore equally need to be excluded from that shortcut). At least as far as the Capability SIDs go, I am rather confident from reading the context (the commit's message, as well as the report that led to that commit) that the shortcut is safe, and I could imagine that the same is true for IIS APPPOOL and Samba SIDs. Absent any further insight, I therefore decided to leave the rest of 48e7d63 (Cygwin: fetch_account_from_windows: skip LookupAccountSid for SIDs known to fail, 2025-04-10) intact. Reported-by: Robert Fensterman <[email protected]> Fixes: 48e7d63 (Cygwin: fetch_account_from_windows: skip LookupAccountSid for SIDs known to fail, 2025-04-10) Signed-off-by: Johannes Schindelin <[email protected]>
As of the upgrade to v3.6.2, I can no longer open any MinTTY windows: It errors out with the message "Error: Could not fork child process: There are no available terminals (-1)". This symptom was also described on the Cygwin mailing list: https://inbox.sourceware.org/cygwin/CAL148o6i=r+G=cEQzs5e+KmMt07FxJHhMnoNnquhM0JJuyrwtA@mail.gmail.com/#t I've bisected the problem down to 48e7d63 (Cygwin: fetch_account_from_windows: skip LookupAccountSid for SIDs known to fail, 2025-04-10), which prevents my AzureAD user account (the SID begins with `S-1-12-1-`) from being handled correctly. This PR is a (minimally) partial revert of that commit to resolve this problem. Signed-off-by: Johannes Schindelin <[email protected]>
Member
Author
|
/open pr The workflow run was started |
This was referenced Jun 3, 2025
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As of the upgrade to v3.6.2, I can no longer open any MinTTY windows:
This symptom (the message "Error: Could not fork child process: There are no available terminals (-1)") was also described on the Cygwin mailing list.
I've bisected the problem down to 48e7d63 (Cygwin: fetch_account_from_windows: skip LookupAccountSid for SIDs known to fail, 2025-04-10), which prevents my AzureAD user account (the SID begins with
S-1-12-1-) from being handled correctly.This PR is a (minimally) partial revert of that commit to resolve this problem, and a companion of msys2/msys2-runtime#286.