Bug 11320 - "force group" with local group not working
Summary: "force group" with local group not working
Alias: None
Product: Samba 4.1 and newer
Classification: Unclassified
Component: File services (show other bugs)
Version: 4.2.2
Hardware: All All
: P5 normal (vote)
Target Milestone: ---
Assignee: Karolin Seeger
QA Contact: Samba QA Contact
Depends on:
Reported: 2015-06-08 18:51 UTC by Marc Muehlfeld
Modified: 2021-07-11 11:37 UTC (History)
7 users (show)

See Also:

Level 10 debug log (4.2.2) (57.16 KB, application/x-gzip)
2015-06-08 18:51 UTC, Marc Muehlfeld
no flags Details
Level 10 debug log (4.1.17) (51.77 KB, application/octet-stream)
2015-06-08 18:53 UTC, Marc Muehlfeld
no flags Details
Proposed fix (2.25 KB, patch)
2015-07-21 22:30 UTC, Justin Maggard
jra: review+
git-am fix for master. (6.03 KB, patch)
2015-07-28 18:31 UTC, Jeremy Allison
asn: review+
git-am cherry-pick from master for 4.3.0, 4.2.next, 4.1.next. (2.60 KB, patch)
2015-07-29 23:31 UTC, Jeremy Allison
metze: review+

Note You need to log in before you can comment on or make changes to this bug.
Description Marc Muehlfeld 2015-06-08 18:51:39 UTC
Created attachment 11131 [details]
Level 10 debug log (4.2.2)

After updating from 4.1.17 to 4.2.2, the following share stop working:

        path = /testX
        browsable = yes
        force create mode = 0660
        force directory mode = 2770
        force user = firebird
        force group = firebird
        guest ok = no
        valid users = "+MUC\medical office"
        invalid users =

The user and group "firebird" both exist local on this server:
# getent passwd firebird
firebird:x:84:84:Firebird Database Owner:/opt/firebird:/bin/false
# getent group firebird

Also the account im using to access (muehlfeld), is member of the domain group "medical office".

This are the permissions on the folder:
# ls -ld /testX/
drwxr-s--- 2 firebird firebird 6  8. Jun 16:36 /testX/

When I try to access this share, I get "permission denied".
When I remove either the "force group" or the "force user" line, then it's possible to access the share.

The share like shown above worked great since years with 3.6, 4.0, 4.1.

Find attached a level 10 debug log from the try to access this share.

I this problem might be the same like in Bug #11082, but Jeremy said there, that it looks like a different problem.
Comment 1 Marc Muehlfeld 2015-06-08 18:53:03 UTC
Created attachment 11132 [details]
Level 10 debug log (4.1.17)

(In reply to Jeremy Allison)
> The code in source3/smbd/service.c:343(find_forced_group) is identical in 4.1.x 
> to 4.2.x and master, so I think you would hit the same problem on 4.1.x.

You're right. The code block in service.c is identical in 4.2.2 and 4.1.17. However in 4.1.17 it works. Debug log of the same (accessing the testX share) from 4.1.17 is attached.
Comment 2 Roel van Meer 2015-07-21 08:20:32 UTC
It might be related to commit cd4442c7?

I can reproduce the problem, and after setting "winbind use default domain" to "No" I can access the share again.
Comment 3 Justin Maggard 2015-07-21 22:30:34 UTC
Created attachment 11281 [details]
Proposed fix

This bug also makes "valid users" not work for cases where there is a user with the same name as a group.

user_ok_token() already does the right thing by adding the LOOKUP_NAME_GROUP
flag; but lookup_name() was not respecting that flag, and went ahead and looked
for users anyway.

Please try the attached patch.  It fixes my "valid users" issue.
Comment 4 Jeremy Allison 2015-07-21 22:51:29 UTC
Comment on attachment 11281 [details]
Proposed fix

Oh that's *absolutely* correct. WELL DONE - *great* catch !
Comment 5 Jeremy Allison 2015-07-21 23:33:43 UTC
Ok, now I need to see if I can reproduce this with a torture test case so we can ensure we don't regress again !
Comment 6 Jeremy Allison 2015-07-24 21:09:45 UTC
(In reply to Marc Muehlfeld from comment #0)

OK Marc, I've tried to reproduce your problem using current master and with 4.2.x, setting up a share in the same way with the same restrictions, and I do *NOT* see the problem you see.

My smb.conf:

        workgroup = WORKGROUP
        log file = /usr/local/samba/var/log.%m
        store dos attributes = yes
        map hidden = no
        map system = no
        map readonly = no

        kernel oplocks = no
        kernel change notify = no
        smb2 leases = yes

        create mask = 0777
        directory mask = 0777

        path = /tmp/puppettest
        read only = no
        force user = puppet
        force group = puppet
        guest ok = no
        valid users = +eng
        invalid users =

ls -ld /tmp/puppettest/
drwx------ 2 puppet puppet 4096 Jul 24 14:02 /tmp/puppettest/

I'm logging on a local jeremy user that is also a member of UNIX group eng. smbclient access this just fine.
Comment 7 Jeremy Allison 2015-07-24 21:13:44 UTC
Comment on attachment 11131 [details]
Level 10 debug log (4.2.2)

I don't see any ACCESS_DENIEDs in this log.
Comment 8 Jeremy Allison 2015-07-24 21:14:19 UTC
(In reply to Justin Maggard from comment #3)

Justin, can you post a sample smb.conf so I can get a reproducible test case to add a regression test ?
Comment 9 Jeremy Allison 2015-07-24 21:49:35 UTC
From phone call - with my smb.conf, create an 'eng' SMB user and I should be able to reproduce the problem.
Comment 10 Justin Maggard 2015-07-24 21:50:40 UTC
I just verified that it doesn't actually need to a SMB user -- just a UNIX user named "eng" is good enough.
Comment 11 Jeremy Allison 2015-07-24 22:12:21 UTC
(In reply to Justin Maggard from comment #10)

Still can't reproduce :-(. Given my smb.conf, and a UNIX eng user I can still access the share as 'puppet'.

Justin, can you post your entire smb.conf and setup ?
Comment 12 Jeremy Allison 2015-07-24 22:39:28 UTC
*** Bug 11082 has been marked as a duplicate of this bug. ***
Comment 13 Jeremy Allison 2015-07-24 23:04:55 UTC
Phew. Finally managed to reproduce with my smb.conf if I set:

valid users = +WORKGROUP\eng

Should be enough for me to knock up a test case...
Comment 14 Jeremy Allison 2015-07-24 23:14:23 UTC
Yay. And Justin's patch fixes it of course :-).
Comment 15 Marc Muehlfeld 2015-07-28 08:19:46 UTC
I can confirm that the patch works here, too (tested against 4.2.3).

Do we need another reviewed-by or can we push it to autobuild?
Comment 16 Jeremy Allison 2015-07-28 15:53:37 UTC
I'm happy to push with your rb+, but I'd really like a regression test too.

I'm working on that right now but it's a really hard thing to test :-).
Comment 17 Jeremy Allison 2015-07-28 18:31:27 UTC
Created attachment 11291 [details]
git-am fix for master.

(Finally) here is the regression test that ensures we won't mess this one up again :-).

Marc, please review for master !


Comment 18 Andreas Schneider 2015-07-29 16:00:46 UTC
Comment on attachment 11291 [details]
git-am fix for master.

Comment 19 Stefan Metzmacher 2015-07-29 20:37:52 UTC
As the fix is in master, please backport it to 4.{1,2,3}.next as required.

Comment 20 Jeremy Allison 2015-07-29 23:31:24 UTC
Created attachment 11296 [details]
git-am cherry-pick from master for 4.3.0, 4.2.next, 4.1.next.

Backport for 4.2.next, 4.1.next.

I was also going to backport the regression test, but this requires additional test changes to be backported that from master that we probably don't need as it's being continually tested in master now.

Please cherry-pick for 4.2.next, 4.1.next.
Comment 21 Stefan Metzmacher 2015-07-30 10:03:52 UTC
(In reply to Jeremy Allison from comment #20)

What's with 4.3.next?

And also ask someone for a review flag on the attachment,
you know how this is supposed to work...

Comment 22 maurer 2015-07-30 11:06:10 UTC

bug https://bugzilla.samba.org/show_bug.cgi?id=11082 (force user problem with local/nss Users) was marked as a duplicate of this bug, but unfortunately  the patch did not solve the force user problem.

I have attached debug10 logs to https://bugzilla.samba.org/show_bug.cgi?id=11082
of samba 4.2.3 with that patch applied


Comment 23 Jeremy Allison 2015-07-30 15:53:55 UTC
(In reply to Stefan (metze) Metzmacher from comment #21)

Metze, is there a separate branch for v4-3-test now ? I asked as I'm not aware of that yet.
Comment 24 Jeremy Allison 2015-07-30 16:06:54 UTC
Comment on attachment 11296 [details]
git-am cherry-pick from master for 4.3.0, 4.2.next, 4.1.next.

FYI. For direct cherry-picks from master which already have 2 Team reviews and no back-porting changes, in the past I've not always gotten an additional review before sending to Karolin as it hasn't seemed necessary. If you want this step though I'm happy to do that.
Comment 25 Jeremy Allison 2015-07-30 16:21:47 UTC
Comment on attachment 11296 [details]
git-am cherry-pick from master for 4.3.0, 4.2.next, 4.1.next.

Edited "Destription" to note this also applies cleanly to v4-3-test (AKA 4.3.0).
Comment 26 Stefan Metzmacher 2015-07-30 20:58:24 UTC
(In reply to Jeremy Allison from comment #23)

Yes, as 4.3.0rc1 is out of the door.

This will be part of 4.3.0rc2...
See https://wiki.samba.org/index.php/Release_Planning_for_Samba_4.3
Comment 27 Stefan Metzmacher 2015-07-31 07:40:04 UTC
(In reply to Jeremy Allison from comment #24)

Having a review for master doesn't mean that the reviewer wants the change
to be ported to release branches. It's better to have just one way of doing things...
Comment 28 Stefan Metzmacher 2015-08-03 12:30:19 UTC
Pushed to autobuild-v4-[1,2,3]-test
Comment 29 Stefan Metzmacher 2015-08-04 14:14:21 UTC
(In reply to Stefan Metzmacher from comment #28)

Pushed to v4-[1,2,3]-test