[PATCH] Correctly handle !authoritative in the rpc-based auth backends

[Andrew Bartlett]

On Fri, 2017-03-10 at 15:08 +0100, Volker Lendecke wrote:
> On Fri, Mar 10, 2017 at 05:46:58PM +1300, Andrew Bartlett wrote:
> > 
> > The SMB or NETLOGON case has already checked the password locally,
> > so
> > should never talk to the auth subsystem again, it must just go
> > directly
> > to a remote DC.  The ntlm_auth case might be trying a local login. 
> 
> Please explain. For the AD DC, smbd would go to winbind even for
> local
> authentication. If this turns out to be a performance problem, we can
> introduce a call from smbd directly into netlogond ncacn_unix. On an
> AD DC, we have the huge advantage to always have winbind around, and
> we should make use of this.

I'm certainly concerned about performance, but also consistency.  I'll
explain a little more below.

> > Thanks for the explanation and the patches.  They look really
> > good!  It
> > is great to get this dealt with properly, and I look forward to the
> > final set.
> > 
> > I take it that "auth methods" now only controls the smb connection
> > handler?
> 
> Yep. If you ask me, I would ditch the "auth methods" parameter from
> smb.conf. I would really like to see the use cases that make them
> necessary.

I tend to agree.   The only users doing interesting things that I've
heard of were patching Samba anyway, like Apple back when they ran
Samba.  I did try and write a new auth module recently for a client to
authenticate against Open Directory, but I never got it to work (OD
restrictions). 

> > Regarding the last patch, my main thought is that I want the call
> > to
> > netlogon not to happen at all, unless we are servicing wbinfo or
> > ntlm_auth.  I certainly appreciate that by going to netlogon the
> > auth
> > module list can be fixed, which certainly has advantages.  
> > 
> > When checking local passwords, I like the API call for the ability
> > to
> > pass in additional data (like the source IP of the auth I've added
> > in
> > the logging patches), but we could:
> >  - use a private custom IRPC
> >  - or just not worry for this particular case
> 
> See above.
> 
> My main reason for winbind to do netlogon rpc locally for sam auth is
> that it reduces the number of places we call into the auth subsystem
> loop. I really want to have canonical ways into the auth subsystem
> and
> get them right and fast once and for all cases. And we have to get
> the
> SamLogon call 100% right and fast anyway, so why not use it locally
> too?

Is this just for ntlm_auth?  Above you seem to suggest you want smbd to
use winbindd as well.

It isn't in your patch set, but I think you are saying that we should
call:
 smbd -> winbindd -> netlogond4 for every authentication

That may be OK for SMB, but for LDAP where we we can get hammered with
simple bind requests, and are currently stuck in a single process, it
would be:

 ldapsrv (single process) -> winbind (multi-process in parts) ->
netlogond4 (multi-process)

As ldapsrv is currently blocking on authentication and is a single
process, this would seem to be undesirable.  

Now, you might ask why do I bring LDAP in to it?  I very strongly feel
that the auth process on SMB needs to be the same as DCE/RPC and LDAP,
because I fear subtle variations between the authentication result on
these different protocols will be a nightmare to debug when all
integrated into the same AD DC.  

It is this I feel most strongly about, I'm not as fussed about the in-
process/inter-process split as long as it brings technical advantages.

> From a progress perspective I'd call the patchset pretty much done
> and
> ready for master. It survives autobuild and enables correct fallback
> for !authoritative. Right, for ntlm_auth in the rodc case we don't
> look
> locally, but the rodc is not a fully supported configuration yet.

Correct, but I would really like not to move backwards, and keep an eye
on what we need to do to improve it, as I'll be re-starting work on
this soon. 

> The deeper reason why I want this is in rather sooner than later is
> the removal of the call to is_trusted_domain() in the source3 auth
> path. That call is just wrong, but to get it through autobuild I had
> to cleanup the DC side of things.

Indeed, and I do thank you for your patience and persistence here. 

As the above is more general, I've replied with some more specific
concerns in another mail.

Thanks,

Andrew Bartlett