Created attachment 15669 [details] Fix-Windows-Explorer-crash-on-S-1-22-SIDs-in-NTACL.patch The Windows Explorer crashes when opening the security tab for files or directories that have S-1-22 Unix SIDs in their NTACLs. Before Samba 4.9 the Samba/AD DC (rather Winbind) returned NT_STATUS_SOME_NOT_MAPPED, which was fine for Windows, but since some commit, I guess d7780c66866, the dom_sid_lookup_predefinied_sid call returns NT_STATUS_INVALID_SID, on which the Windows Explorer seems to segfault. The attached patch adds the Unix-SIDs to the predefined_domains struct, so that Samba recognizes the Sids S-1-22* as valid. FYI: We checked the behavior of a native Microsoft Windows 2012 AD server by setting an ACE like this using smbcalcs, and in that case the Windows Explorer doesn't crash. Apparently it sends a different return code (IIRC simply NT_STATUS_OK, but I didn't check the wireshark trace of my collegue).
Looks related to some reports on the user mailing list: * https://lists.samba.org/archive/samba/2019-July/224358.html * https://lists.samba.org/archive/samba/2018-August/217700.html
Good catch! Can you please also attach the network trace against Windows so we can check how Windows handles this? Thanks!
FYI: There are also other SIDs that Samba doesn't recognize as valid yet, like: * S-1-15-* SECURITY_APP_PACKAGE_AUTHORITY * S-1-17-* SECURITY_SCOPED_POLICY_ID_AUTHORITY * S-1-18-* SECURITY_AUTHENTICATION_AUTHORITY Some of them are on the "Well Known" List, e.g.: * S-1-15-2-1 ALL_APP_PACKAGES * S-1-18-1 AUTHENTICATION_AUTHORITY_ASSERTED_IDENTITY We are currently evaluating an additional similar patch for that. We may contribute that one too, if the Samba team is fine with the general approach of the patch above.
Created attachment 15672 [details] Windows 2012 AD response to S-1-22-1-0 in NTACL
Thanks! What does Samba return with the patch? Can you share a network trace of that too pleaes? With a valid UNIX id I'd exepct to see the UNIX SID being translated to a name. Can you please also add a test so we don't regress again in the future? Thanks!
Created attachment 15746 [details] Found more crash-inducing SIDs and added these to default_domain/name_mapping I expanded the patch to cover more "Well Known" SID identifier authorities, as well as additional Well Known SIDs, which also caused crashes of the Windows explorer due to them being recognized as invalid by Samba. All of these are documented here: https://docs.microsoft.com/en-us/openspecs/windows_protocols/ms-dtyp/c6ce4275-3d90-4890-ab3a-514745e4637e https://docs.microsoft.com/en-us/openspecs/windows_protocols/ms-dtyp/81d92bba-d22b-4a8c-908a-554ab29148ab Unix-SIDs still return NT_STATUS_NONE_MAPPED/NT_STATUS_SOME_NOT_MAPPED. It would be great if all of them were translated, but after finding the "Identifier Authority" of a SID in the predefined_domain_mapping list but not the SID itself, the rpc-server forks a winbind process which isn't able to resolve these Unix-SIDs to names. I can look into this further, but my goal with this patch was just to prevent the crashes. I tested the patch by overwriting the NTALCLs of a share by using samba-tool ntacl set. Then I checked if accessing the security tab was possible without a crash on a Windows 7/10 Computer. For an automated test, I'd use wbinfo --sid-to-name and check the returncode. Would you consider this an acceptable approach?
Created attachment 15747 [details] Windows 2008 AD response to root in share patched
(In reply to Julia Bremer from comment #6) I can confirm that the patch from comment #6 works if applied to the Debian buster version of Samba (2:4.9.5+dfsg-5+deb10u1) and that it fixes the crashing Windows Explorers for us.
*** Bug 13671 has been marked as a duplicate of this bug. ***
Created attachment 16168 [details] Minimal patch to fix the problem? Can someone test if this also fixes the problem with a Windows client? With the S-1-22-1-* sids, but testing S-1-17*, S-1-18* would also be great. Thanks!
(In reply to Stefan Metzmacher from comment #10) With this patch, the Security tab shows "Account Unknown(S-1-22-2-0)" for an unrecognized group instead of crashing, which is good! I don't know how I would have to test the other sids, sorry.
Created attachment 16169 [details] Sambas INVALID_SID answer I can confirm that the explorer does not crash anymore for either S-1-22-*, S-1-17-*, S-1-18-* and returns "NT_STATUS_NONE_MAPPED/NT_STATUS_SOME_NOT_MAPPED", which is consistent with the MS behavior. This is definitely an improvement even though the SIDs are not translated to names. Interestingly though it still crashes if invalid SIDs (e.g S-1-0) are found in the ACLs. When setting an invalid SID in the ACLS, both Windows and Samba return NT_STATUS_INVALID_SID. With a Samba DC the client crashes, but with an MS DC it does not. The answers are slightly different, Windows seems to expect one empty Name entry instead of no Name entry. "dcesrv_lsa_LookupNames_base_finish" creates the Name entries at the end of the lsalookup. This function is not called if the status is NTSTATUS_INVALID_SID, which is why the answer differs between SOME_NOT_MAPPED and INVALID_SID. When messing around with gdb, I could prevent a crash by making it return INVALID_SID only after executing "dcesrv_lsa_LookupNames_base_finish". I'll attach some network traces which show the differences between MS and Samba NTSTATUS_INVALID_SID.
Created attachment 16170 [details] MS INVALID_SID answer
(In reply to Julia Bremer from comment #13) That seems to be from Windows versions before 2016, right? I get the same when running rpcclient "lookupsids S-1-0" against a Windows 2016 server: lsa_LookupSids: struct lsa_LookupSids out: struct lsa_LookupSids domains : * domains : * domains: struct lsa_RefDomainList count : 0x00000000 (0) domains : NULL max_size : 0x00000000 (0) names : * names: struct lsa_TransNameArray count : 0x00000001 (1) names : * names: ARRAY(1) names: struct lsa_TranslatedName sid_type : SID_NAME_UNKNOWN (8) name: struct lsa_String length : 0x0000 (0) size : 0x0000 (0) string : NULL sid_index : 0xffffffff (4294967295) count : * count : 0x00000000 (0) result : NT_STATUS_INVALID_SID Interestingly Windows 2019 returns this: lsa_LookupSids: struct lsa_LookupSids out: struct lsa_LookupSids domains : * domains : * domains: struct lsa_RefDomainList count : 0x00000000 (0) domains : NULL max_size : 0x00000000 (0) names : * names: struct lsa_TransNameArray count : 0x00000001 (1) names : * names: ARRAY(1) names: struct lsa_TranslatedName sid_type : SID_NAME_UNKNOWN (8) name: struct lsa_String length : 0x000a (10) size : 0x000c (12) string : * string : 'S-1-0' sid_index : 0xffffffff (4294967295) count : * count : 0x00000000 (0) result : NT_STATUS_NONE_MAPPED
I'm consolidating related changes in this branch on gitlab: https://gitlab.com/samba-team/devel/samba/-/commits/slow-wellknown-sids
(In reply to Ralph Böhme from comment #15) That is very interesting. In my last comment I tested this with Windows Server 2012R2, that is correct. I just tested with my Windows Server 2019 (1809 - build 17763.107.1010129-1455) I still get NT_STATUS_INVALID SID when requesting "S-1-0". Local Security Authority, lsa_LookupSids Operation: lsa_LookupSids (15) [Request in frame: 201] Pointer to Domains (lsa_RefDomainList) Referent ID: 0x00020000 Domains Count: 0 NULL Pointer: Pointer to Domains (lsa_DomainInfo) Max Size: 0 Pointer to Names (lsa_TransNameArray) Names Count: 1 Pointer to Names (lsa_TranslatedName) Referent ID: 0x00020004 Max Count: 1 Names Sid Type: SID_NAME_UNKNOWN (8) Name Length: 0 Size: 0 NULL Pointer: Pointer to String (uint16) Sid Index: 4294967295 Pointer to Count (uint32) Count: 0 NT Error: STATUS_INVALID_SID (0xc0000078)
Hai, a small update on this one. I did run my packages with the patch from Julia Bremer some time, and that worked will. After a test i noticed it my explorer also didnt crash anymore, so i removed that patch, asuming this was in samba now, but one of the users using my packages messaged its crashing again. What we can tell from that is; With current 4.13.7 explorer isnt crashing on a member server, but it is still crashing on the AD-DC.
(In reply to Louis from comment #17) It also crashes using a share on a 4.14.2 Samba AD DC, no crashes on a 14.4.2 Unix domain member. Something in the DC4 code ?
(In reply to Stefan Metzmacher from comment #10) I tested this against 4.14.4 test agains AD-DC results in : rpcclient 192.168.0.1 -Uadministrator -c 'lookupsids S-1-22-1 S-1-22-1-0;lookupsids S-1-22;lookupsids S-1-3-0 S-1-3-99;lookupsids S-1-3' Enter NTDOM\administrator's password: result was NT_STATUS_INVALID_SID result was NT_STATUS_INVALID_SID S-1-3-0 \CREATOR OWNER (5) S-1-3-99 *unknown*\*unknown* (8) S-1-3 *unknown*\*unknown* (8) Same test against a member results in : rpcclient 192.168.0.11 -Uadministrator -c 'lookupsids S-1-22-1 S-1-22-1-0;lookupsids S-1-22;lookupsids S-1-3-0 S-1-3-99;lookupsids S-1-3' S-1-22-1 Unix User (3) S-1-22-1-0 Unix User\root (1) result was NT_STATUS_INVALID_SID S-1-3-0 \Creator Owner (5) S-1-3-99 *unknown*\*unknown* (8) S-1-3 (3)
small retest with 4.14.5 and added the patch Fix-Windows-Explorer-crash-on-S-1-22-SIDs-in-NTACL.patch the rpcclient outputs. On the AD-DC rpcclient 192.168.0.1 -Uadministrator -c 'lookupsids S-1-2 2-1 S-1-22-1-0;lookupsids S-1-22;lookupsids S-1-3-0 S-1-3-99;lookupsids S-1-3' Enter NTDOM\administrator's password: S-1-22-1 Unix User (3) S-1-22-1-0 *unknown*\*unknown* (8) S-1-22 Unix (3) S-1-3-0 \CREATOR OWNER (5) S-1-3-99 *unknown*\*unknown* (8) S-1-3 *unknown*\*unknown* (8) On the Member rpcclient 192.168.0.11 -Uadministrator -c 'lookupsids S-1-22-1 S-1-22-1-0;lookupsids S-1-22;lookupsids S-1-3-0 S-1-3-99;lookupsids S-1-3' Enter NTDOM\administrator's password: S-1-22-1 Unix User (3) S-1-22-1-0 Unix User\root (1) result was NT_STATUS_INVALID_SID S-1-3-0 \Creator Owner (5) S-1-3-99 *unknown*\*unknown* (8) S-1-3 (3)
(In reply to Louis from comment #20) Tested this again on 4.14.6. exact same results.
Windows crashing is really a Windows bug. It should be able to cope with any form of mangled/fuzzed reply to that RPC. I suggest you report this to Microsoft if it's repeatable. Might even be a security issue.
Well, it might well be a windows bug. The problem is that all current windows versions are buggy - at least windows10 and windows11 are crashing instantly when trying to open file security tab of a file located on a samba DC. Maybe, instead of waiting till the whole world changes, we should give it something which it does expect?
(In reply to Michael Tokarev from comment #23) You are correct. If metze's patch fixes the issue, let's just merge it and declare victory :-).
We should probably also add a regression test that uses rpcclient to request the SID lists that causes Windows to crash when Samba replies, and make sure that we keep returning what Windows returns in this specific case instead.
(In reply to Jeremy Allison from comment #24) We're running over 600 servers with a samba that has this patch (4.16.6, and in the past 4.13.13 and 4.13.5) and I can report no issues, only happy customers. If this could be merged that would be very much appreciated! I'm happy to help if that's in any way useful.
Ralph, where are we on this ? Any chance we can get the fix merged for 4.18.rcNext, 4.17.next, 4.16.next ?
(In reply to Jeremy Allison from comment #27) EMOREWORKNEEDED I guess... :)
(In reply to Ralph Böhme from comment #28) What exactly is the work needed though ?
I´d like to add something to Bug 14213. The SID-Tree S-1-15-3 also crashes the explorer. It appears in printer security tabs since Samba 4.16. Maybe it should be added like in the patches above?
For additional Info: I checked the security information of a printer on a Windows-Only Domain: It seems the SID was added in a Windows update. In Windows, it is translated to "Unbekanntes Konto" followed by the SID. It does not crash. "Unbekanntes Konto" roughly translates to "unknown account". So the SID tree S-1-15-3 should most likely be translated to "unknown account".
From Windows 2022 I'm getting this: rpcclient 172.31.9.113 -Uadministrator%A1b2C3d4 -c 'lookupsids S-1-0 S-1-3' => lsa_LookupSids: struct lsa_LookupSids out: struct lsa_LookupSids domains : * domains : * domains: struct lsa_RefDomainList count : 0x00000000 (0) domains : NULL max_size : 0x00000000 (0) names : * names: struct lsa_TransNameArray count : 0x00000002 (2) names : * names: ARRAY(2) names: struct lsa_TranslatedName sid_type : SID_NAME_UNKNOWN (8) name: struct lsa_String length : 0x0000 (0) size : 0x0000 (0) string : NULL sid_index : 0xffffffff (4294967295) names: struct lsa_TranslatedName sid_type : SID_NAME_UNKNOWN (8) name: struct lsa_String length : 0x0000 (0) size : 0x0000 (0) string : NULL sid_index : 0xffffffff (4294967295) count : * count : 0x00000000 (0) result : NT_STATUS_INVALID_SID rpcclient 172.31.9.113 -Uadministrator%A1b2C3d4 -c 'lookupsids S-1-3-0 S-1-0' => lsa_LookupSids: struct lsa_LookupSids out: struct lsa_LookupSids domains : * domains : * domains: struct lsa_RefDomainList count : 0x00000001 (1) domains : * domains: ARRAY(1) domains: struct lsa_DomainInfo name: struct lsa_StringLarge length : 0x0000 (0) size : 0x0002 (2) string : * string : '' sid : * sid : S-1-3 max_size : 0x00000020 (32) names : * names: struct lsa_TransNameArray count : 0x00000002 (2) names : * names: ARRAY(2) names: struct lsa_TranslatedName sid_type : SID_NAME_WKN_GRP (5) name: struct lsa_String length : 0x001a (26) size : 0x001c (28) string : * string : 'CREATOR OWNER' sid_index : 0x00000000 (0) names: struct lsa_TranslatedName sid_type : SID_NAME_UNKNOWN (8) name: struct lsa_String length : 0x000a (10) size : 0x000c (12) string : * string : 'S-1-0' sid_index : 0xffffffff (4294967295) count : * count : 0x00000001 (1) result : STATUS_SOME_UNMAPPED rpcclient 172.31.9.113 -Uadministrator%A1b2C3d4 -c 'lookupsids S-1-3 S-1-0 S-1-3-0' => lsa_LookupSids: struct lsa_LookupSids out: struct lsa_LookupSids domains : * domains : * domains: struct lsa_RefDomainList count : 0x00000001 (1) domains : * domains: ARRAY(1) domains: struct lsa_DomainInfo name: struct lsa_StringLarge length : 0x0000 (0) size : 0x0002 (2) string : * string : '' sid : * sid : S-1-3 max_size : 0x00000020 (32) names : * names: struct lsa_TransNameArray count : 0x00000003 (3) names : * names: ARRAY(3) names: struct lsa_TranslatedName sid_type : SID_NAME_UNKNOWN (8) name: struct lsa_String length : 0x000a (10) size : 0x000c (12) string : * string : 'S-1-3' sid_index : 0xffffffff (4294967295) names: struct lsa_TranslatedName sid_type : SID_NAME_UNKNOWN (8) name: struct lsa_String length : 0x000a (10) size : 0x000c (12) string : * string : 'S-1-0' sid_index : 0xffffffff (4294967295) names: struct lsa_TranslatedName sid_type : SID_NAME_WKN_GRP (5) name: struct lsa_String length : 0x001a (26) size : 0x001c (28) string : * string : 'CREATOR OWNER' sid_index : 0x00000000 (0) count : * count : 0x00000001 (1) result : STATUS_SOME_UNMAPPED
Windows 2008R2 gives this: rpcclient 172.31.9.133 -Uadministrator%A1b2C3d4 -c 'lookupsids S-1-0 S-1-3-0 S-1-3-1 S-1-3' => lsa_LookupSids: struct lsa_LookupSids out: struct lsa_LookupSids domains : * domains : NULL names : * names: struct lsa_TransNameArray count : 0x00000000 (0) names : NULL count : * count : 0x00000002 (2) result : NT_STATUS_INVALID_SID
Hello everyone, i tried to workaround the explorer.exe crash with an upgrade to Windows 11. However, Microsoft seems to be commited to the bug, because Windows 11 and Server 2022 is also affected. If there is anything i can help with to get a fix merged, i am happy to help. Thank you, Michael
Created attachment 17991 [details] My current wip patch to match Windows Server 2022