Bug 13944 - SMB guest authentication may fail
Summary: SMB guest authentication may fail
Alias: None
Product: Samba 4.1 and newer
Classification: Unclassified
Component: File services (show other bugs)
Version: unspecified
Hardware: All All
: P5 regression (vote)
Target Milestone: ---
Assignee: Karolin Seeger
QA Contact: Samba QA Contact
Depends on:
Reported: 2019-05-13 14:57 UTC by Ralph Böhme
Modified: 2019-06-26 07:22 UTC (History)
5 users (show)

See Also:

Patch for 4.9 backported from master (11.26 KB, patch)
2019-06-06 12:31 UTC, Ralph Böhme
abartlet: review+
jra: review+
Patch for 4.10 cherry-picked from master (11.21 KB, patch)
2019-06-06 12:32 UTC, Ralph Böhme
slow: review? (abartlet)
jra: review+

Note You need to log in before you can comment on or make changes to this bug.
Description Ralph Böhme 2019-05-13 14:57:00 UTC
Basically, any system upgraded to 4.9 and newer, that has a group mapping entry for BUILTIN\Guests without the local SAM Guest user and Guests group being a member of BUILTIN\Guests is affected.

This is caused commit 0b261dc4e3f2d04131e1ff76a017aaee6e38e7b1:

   s3:auth: make use of create_builtin_guests()
   in finalize_local_nt_token(). This makes the
   Builtin_Guests handling more dynamic, by having
   a persistent storage for the memberships.

By default, older Samba before 4.9 don't generate a group mapping for BUILTIN\Guests, so there would be no problem. But anyone who who added such a mapping, for example by running `net groupmap ...` is affected.

As soon as there's a BUILTIN\Guest mapping without the required members, guest
access fails.

It works if the members are present:

$ sudo bin/net groupmap list
Guests (S-1-5-32-546) -> BUILTIN\guests

$ sudo bin/net groupmap listmem S-1-5-32-546

$ bin/smbclient -U "foo%bar" //localhost/share -c exit
$ echo $?

Deleting the members and restarting Samba to trigger the problem:

$ sudo bin/net groupmap delmem S-1-5-32-546
$ sudo bin/net groupmap delmem S-1-5-32-546
$ sudo bin/net groupmap delmem S-1-5-32-546 S-1-5-21-3654642884-2763964569-3359135487-501
$ sudo bin/net groupmap listmem S-1-5-32-546

...restart Samba (required)...

$ bin/smbclient -U "foo%bar" //localhost/share -c exit
Unable to initialize messaging context
Bad SMB2 signature for message
[0000] 00 00 00 00 00 00 00 00   00 00 00 00 00 00 00 00   ........ ........
[0000] DB 31 23 BE 34 87 63 DA   FB 7D E9 AB 68 19 5E 9A   .1#.4.c. .}..h.^.
session setup failed: NT_STATUS_ACCESS_DENIED

Working on a fix, need bugnumber...
Comment 1 Ralph Böhme 2019-05-16 12:54:12 UTC
Correction: with an explicit net groupmap entry, mapping BUILTIN\Guests to a UNIX account things work correctly.

With a user configured mapping, the gid of the guest account is added to the UNIX token of the user.

Later, we run gidtosid() for all gids in the UNIX token and add the result to the NT token. gidtosid() will return S-1-5-32-546 (BUILTIN\Guests) for the gid of the guest UNIX account, which be be added to the NT token, therefor guest authentication works.

The problem is a automically added group mapping where the id is allocated by winbindd. That's what create_builtin_guests() does.

As create_builtin_guests() does also add the necessary members to the group, in theory this problem should not affect a standard Samba setup.

Only setups would be affected where the needed BUILTIN\Guests members (LOCALSAM\Guest[s]) are somehow not created or lost are affected.
Comment 2 Ralph Böhme 2019-05-16 13:09:03 UTC
(In reply to Ralph Böhme from comment #1)
Oh, net sam createbuiltingroup "BUILTIN\Guests" DOES create a mapping with an allocated gid which would trigger the problem.
Comment 3 Ralph Böhme 2019-06-06 12:31:21 UTC
Created attachment 15222 [details]
Patch for 4.9 backported from master
Comment 4 Ralph Böhme 2019-06-06 12:32:07 UTC
Created attachment 15223 [details]
Patch for 4.10 cherry-picked from master
Comment 5 Ralph Böhme 2019-06-11 14:00:04 UTC
Ping, Jeremy or Andrew, can you please review the backports? Thanks!
Comment 6 Jeremy Allison 2019-06-12 17:37:03 UTC
Re-assigning to Karolin for inclusion in 4.9.next, 4.10.next.
Comment 7 Karolin Seeger 2019-06-20 10:04:52 UTC
(In reply to Jeremy Allison from comment #6)
Pushed to autobuild-v4-{10,9}-test.
Comment 8 Karolin Seeger 2019-06-26 07:22:15 UTC
(In reply to Karolin Seeger from comment #7)
Pushed to both branches.
Closing out bug report.