Bug 10508 - smbd does not correctly add remote users into local groups.
smbd does not correctly add remote users into local groups.
Status: RESOLVED FIXED
Product: Samba 4.1 and newer
Classification: Unclassified
Component: File services
unspecified
All All
: P5 normal
: ---
Assigned To: Karolin Seeger
Samba QA Contact
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2014-03-20 20:22 UTC by Jeremy Allison
Modified: 2014-04-27 19:54 UTC (History)
4 users (show)

See Also:


Attachments
git-am patch that went into master. (3.04 KB, patch)
2014-03-21 20:28 UTC, Jeremy Allison
asn: review+
Details
Patch made from merged commit. (4.04 KB, patch)
2014-03-24 17:57 UTC, Jeremy Allison
no flags Details
Fixed patch - no compiles :-). (4.04 KB, patch)
2014-03-24 18:09 UTC, Jeremy Allison
no flags Details
Back-ported git-am fix for 4.1.next, 4.0.next (4.40 KB, patch)
2014-03-25 18:33 UTC, Jeremy Allison
asn: review+
jra: review? (idra)
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Jeremy Allison 2014-03-20 20:22:38 UTC
Discovered by Andreas (asn@samba.org) and myself.

NT Tokens created from an info3 struct (either via a krb5 PAC or an info3 struct from the DC) do not get local groups from /etc/group added to their internal tokens in smbd.

Consider the following case. Remote user DOMAIN+administrator, and a local group added in /etc/group.

ntadmins:x:1001:DOMAIN+administrator,root

If there is no local mapping for group ntadmins, then when DOMAIN+administrator logs in the token created inside smbd will not include the UNIX group SID S-1-22-2-1001. If ntadmins is then used in an ACL or share restriction then the user will get ACCESS_DENIED when accessing that resource.

Patch to follow.
Comment 1 Jeremy Allison 2014-03-21 20:28:15 UTC
Created attachment 9797 [details]
git-am patch that went into master.

Cherry-pick of 6034ab521c47fc5f4732398652c9c6847ff92035 applies cleanly to 4.1.next and 4.0.next.

Jeremy.
Comment 2 Andreas Schneider 2014-03-24 16:03:42 UTC
Comment on attachment 9797 [details]
git-am patch that went into master.

LGTM
Comment 3 Andreas Schneider 2014-03-24 16:04:24 UTC
Karolin, please add the patch to v4-0-test and v4-1-test. Thanks!
Comment 4 Jeremy Allison 2014-03-24 16:16:28 UTC
Actually Karolin, can you hold off until I've discussed the follow-up patch with Andrew Bartlett ?

This patch, although correct for smbd file serving, can cause the samba AD binary to fail to start smbd correctly under some circumstances (reported by Kukks on the samba-technical mailing list).

I have a follow-up patch that fixes this issue, but it does need a second positive Team review (which I'm hoping to get today).

Jeremy.
Comment 5 Jeremy Allison 2014-03-24 17:57:00 UTC
Created attachment 9799 [details]
Patch made from merged commit.

Ok, here is a (raw patch, non-git-am) version of the two commits I think we need merged into one patch to make it clearer. Once I've got the second commit into master I'll post this as a git-am fix for 4.1.next and 4.0.next.
Jeremy.
Comment 6 Jeremy Allison 2014-03-24 18:09:48 UTC
Created attachment 9800 [details]
Fixed patch - no compiles :-).

In master, guest account is lp_guest_account(). In 4.1.x and below it's lp_guestaccount().
Comment 7 Jeremy Allison 2014-03-25 18:33:37 UTC
Created attachment 9801 [details]
Back-ported git-am fix for 4.1.next, 4.0.next

OK, here is a squashed backport of master fixes 6034ab521c47fc5f4732398652c9c6847ff92035 and a9fa09723bee3588db2168ac13f7ad0334452c11 that applies cleanly to 4.0.next and 4.1.next.

Andreas, please review this one (should be clearer in what it's doing than the earlier fix you reviewed).

Cheers,

Jeremy.
Comment 8 Andreas Schneider 2014-03-26 12:24:17 UTC
Comment on attachment 9801 [details]
Back-ported git-am fix for 4.1.next, 4.0.next

LGTM
Comment 9 Andreas Schneider 2014-03-26 12:24:48 UTC
Karolin, please add it to 4.1 and 4.0. Thanks!
Comment 10 Jeremy Allison 2014-04-01 17:45:06 UTC
Ping. Want to make sure this one gets in for 4.0.17.

Cheers,

Jeremy.
Comment 11 Karolin Seeger 2014-04-04 19:00:12 UTC
(In reply to comment #10)
> Ping. Want to make sure this one gets in for 4.0.17.
> 
> Cheers,
> 
> Jeremy.

Pushed to autobuild.v4-1-test and autobuild-v4-0-test.
Comment 12 Karolin Seeger 2014-04-07 08:08:43 UTC
(In reply to comment #11)
> (In reply to comment #10)
> > Ping. Want to make sure this one gets in for 4.0.17.
> > 
> > Cheers,
> > 
> > Jeremy.
> 
> Pushed to autobuild.v4-1-test and autobuild-v4-0-test.

Pushed to v4-1-test, autobuild-v4-0-test failed, re-trying...
Comment 13 Karolin Seeger 2014-04-08 08:43:38 UTC
Pushed to v4-0-test.
Closing out bug report.

Thanks!