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

[Andrew Bartlett]

On Thu, 2017-03-09 at 19:48 +0100, Volker Lendecke wrote:
> On Fri, Mar 10, 2017 at 07:30:27AM +1300, Andrew Bartlett wrote:
> > On Thu, 2017-03-09 at 13:36 +0100, Volker Lendecke wrote:
> > > Hi!
> > > 
> > > This is independently correct, but is quite ineffective so far.
> > > The
> > > core auth backend loops don't do this yet, but I want to make the
> > > final patchset smaller.
> > > 
> > > Review appreciated!
> > 
> > Looks good to me!
> > 
> > Reviewed-by: Andrew Bartlett <abartlet at samba.org>
> > 
> > Thanks!
> 
> Thank you for looking :-)
> 
> Because you asked earlier: Attached find the whole patchset I'm
> working on. The local netlogond use of winbind makes it easier in the
> rodc case. 

OK.  I can see that being the case for our current (rather poor) tests,
and I do expect this would improve things in the real world.
  
Honestly the tests really need to improve (and that is one of the jobs
I have coming up). 

Just for thought: If we put an RODC on (say) a busy squid proxy, how
would ntlm_auth go to the sam.ldb for cached passwords, but fallback to
the real DC for others?

The other case we have to support, which is more difficult, is that
when we are an RODC or even a full DC, we should present wrong password
to the PDC emulator.  On the RODC this allows the bad password count to
be updated (otherwise it can't be stored anywhere), and on an RW DC it
is intended to give the user a second chance to authenticate with their
new password before replication catches up.

> The routing is clear, we don't need to special-case in the
> auth methods for that. This version has not yet survived a full
> autobuild. Without the last 3 ones it failed at the very end in the
> rodc tests, so I added them again. Now those tests pass individually.

OK, that is very interesting.

I totally agree on the need for clear routing.  The previous design was
(sadly) a deliberate obfuscation in places, designed to pretend the AD
DC didn't exist (hidden behind auth methods) in winbindd.  I'm glad to
get past that.

I would really like a clear, different route for SMB or NETLOGON
(trusted domain) authentication assistance (perhaps via IRPC from
auth_winbind, which could be used to make the auth stack async) as for
the ntlm_auth or wbinfo -a handler.  

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. 

> BTW, there's already precedence for connecting to netlogond locally:
> In 5602377fc we've added that for a different call, but it's there.
> 
> We need to talk about full bisectability with make test. That will be
> difficult to achieve for this patchset. We need to mark many tests as
> failing in between and unmark them again. Or one big patch.

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?

The pdbtest patch looks wrong, we have been testing the different auth
methods via that tool, so fixing it to 'sam' seems to be limiting what
we are testing.

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

It would be good to also consider the trusted domains case in netlogon,
even if that is only experimental at this point. 

Thank you so much for sharing your work with progress with me.  I hope
you take these comments in the constructive manner in which they were
intended. 

Thanks,

Andrew Bartlett