[PATCH] Fix bug #9878 - force user does not work as expected.

[Jeremy Allison]

On Wed, Mar 19, 2014 at 09:22:24AM -0700, Jeremy Allison wrote:
> On Wed, Mar 19, 2014 at 05:06:11PM +0100, Andreas Schneider wrote:
> > On Wednesday 19 March 2014 09:03:11 Jeremy Allison wrote:
> > > On Wed, Mar 19, 2014 at 04:58:47PM +0100, Andreas Schneider wrote:
> > > > On Wednesday 19 March 2014 08:43:56 Jeremy Allison wrote:
> > > > > On Wed, Mar 19, 2014 at 04:16:16PM +0100, Andreas Schneider wrote:
> > > > > > Houston, we have a problem!
> > > > > > 
> > > > > > I have a print$ share with force group which stops working after this
> > > > > > patch!
> > > > > > 
> > > > > > [print$]
> > > > > > 
> > > > > >         comment = Printer Drivers
> > > > > >         path = /var/lib/samba/drivers
> > > > > >         write list = @ntadmin root
> > > > > >         force group = ntadmin
> > > > > >         create mask = 0664
> > > > > >         directory mask = 0775
> > > > > > 
> > > > > > I'm connecting as LEVEL1+Administrator who is a member of the group
> > > > > > ntadmin. I'm not able to upload printer drivers after the patch is
> > > > > > applied. If I revert the patch, I can upload printer drivers again.
> > > > > 
> > > > > Debug level 10 logs please...
> > > > > 
> > > > > Jeremy.
> > > > 
> > > > This is the log giving me an error pushing the files to the print$ share.
> > > > If you want a log with the patch reverted let me know.
> > > 
> > > What error message are you seeing ? I don't see an "ACCESS_DENIED"
> > > in this log, which is what I'd expect.
> > 
> > The error message popup is:
> > 
> > 	An error occured while copying file hpcdmc64.dll
> > 
> > 	Cannot copy file to destination directory.
> > 
> > 	Click Retry to retry the operation or click Cancel to
> 
> Ok, looks like it's a group resolution failure.
> 
> When attaching to print$ we get:
> 
>   is_share_read_only_for_user: share print$ is read-only for unix user LEVEL1+administrator
> 
> which means that the group lookup for @ntadmin isn't
> working correctly.
> 
> I'll send you some patches to expand debug in token_contains_name_in_list()
> so we can see what is happening.

OK - here is an attached patch that will dump out what
is going wrong. Can you resend me the log with this
in place please ?

The "force user" patch is good. The issue is that
the group resolution for @ntadmin -> &+ntadmin -> Check netgroup "ntadmin" followed by UNIX group ntadmin
(lookup_name: Unix Group\ntadmin => domain=[Unix Group], name=[ntadmin])
isn't matching the token generated for the LEVEL1+Administrator.

My guess is mapping 'ntadmin' inside token_contains_name()
is mapping to the UNIX S-1-22 group, whereas that for
some reason isn't present in the token attached to
LEVEL1+Administrator.

The reason it works without the "force user" patch
is that the token that's being checked inside
token_contains_name() will be identical for the
forced group lookup of "ntadmin" -> UNIX S-1-22 group
(lookup_name: Unix Group\ntadmin => domain=[Unix Group], name=[ntadmin])
as that same lookup is being done to create the
'force group token'. I think it's still wrong,
but it's checking the same thing.

But the extra debugs will tell us more.

Cheers,

	Jeremy.
-------------- next part --------------
diff --git a/source3/smbd/share_access.c b/source3/smbd/share_access.c
index 8b165e6..ecc3681 100644
--- a/source3/smbd/share_access.c
+++ b/source3/smbd/share_access.c
@@ -127,6 +127,13 @@ static bool token_contains_name(TALLOC_CTX *mem_ctx,
 					  name, sid_type_lookup(type)));
 				return False;
 			}
+			/* JRATEST */
+			DEBUG(0,("Group name %s became sid %s\n",
+				name, dom_sid_string(mem_ctx, &sid) ));
+
+			security_token_debug(DBGC_CLASS, 0, token);
+			/* END JRATEST */
+
 			if (nt_token_check_sid(&sid, token)) {
 				return True;
 			}