[PATCH] Fix bug #9746 - guest ok + force user + force group doesn't work

[Andrew Bartlett]

On Wed, 2013-04-03 at 14:32 -0700, Jeremy Allison wrote:
> On Thu, Apr 04, 2013 at 08:28:07AM +1100, Andrew Bartlett wrote:
> > On Wed, 2013-04-03 at 13:47 -0700, Jeremy Allison wrote:
> > > When doing force user on a share, we should only
> > > set the guest bit if the user we're forcing to
> > > is the guest user.
> > > 
> > > Please review and push if you're ok with it.
> > 
> > Are you really sure about this?
> > 
> > Do we really want to elevate users from un-authenticated (guest) status
> > to authenticated status just because they connect to a 'force user'
> > share?
> 
> Yes. That's what "force user" means.

I'm talking about the 'guest' flag on the session_info (which turns into
either &global_sid_Builtin_Guests or &global_sid_Authenticated_Users in
their token).  

Clearly from the logs and your testing we are also choosing to become
the guest user, not the named user.  Have you located where that is?

> > The smb.conf you posted in the bug as 'works for me' seems entirely
> > reasonable, what was the failing configuration?
> 
> They didn't explain that they were connecting as guest.
> I was connecting as a valid user, and then correctly
> being forced to the "force user" user.

OK.

> > Finally, if we do this, we really need a test and more comments to show
> > what is going on, because I can't see how it is 'obviously correct' from
> > the patch. 
> 
> You really should take a little time and read the
> code more :-). Don't just give a knee-jerk reaction.

I'm doing that now.  I just didn't want this rushed in under the banner
of 'obviously correct' when to me, it still isn't. 

> It really is obviously correct, if we decide that
> force user overrides a guest login on a share (which
> it does for Samba versions 3.6.11 and previous).

If authentication/authorization code isn't obviously correct to me, then
I stand by it not being obviously correct.  If it is un-obviously
correct, then we need clarifications and comments so it becomes so.  

The ordering in make_connection_snum() is subtle, and while we check the
'guest ok' smb.conf parameter first-up, the check against the share ACL
is currently after the force user/group stuff, and this change. 

>From here, I would like to understand where we use the is_guest flag for
the (reasonable to assume, and clearly the basis for which you claim
this is obviously correct) task of substituting in the guest token, and
therefore not the named user.  I can't see that code in my master tree.

To be clear, I'm not worried about fixing the uid/gid the account
becomes, I'm worried about the other implications of dropping the guest
bit (and so adding the authenticated users SID to an un-authenticated
user). 

I hope this explains my fears better, so we can work out a way to fix
this and alleviate them.

Andrew Bartlett

-- 
Andrew Bartlett                                http://samba.org/~abartlet/
Authentication Developer, Samba Team           http://samba.org