[PATCH] Simplify auth_check_ntlm_password

[Jeremy Allison]

On Wed, Mar 08, 2017 at 10:04:19AM +1300, Andrew Bartlett wrote:
> On Tue, 2017-03-07 at 14:28 +0100, Volker Lendecke wrote:
> > Hi!
> > 
> > While working on the auth code I had a hard time following the logic
> > in auth_check_ntlm_password. The attached patchset is supposed to
> > make
> > that more traceable. It does not change behaviour, those patches will
> > come at a later step and will make the routine even simpler. The
> > behaviour change will be to always break out of the loop if
> > 
> > !NT_STATUS_EQUAL(nt_status,NT_STATUS_NOT_IMPLEMENTED);
> > 
> > Review appreciated!
> 
> Thanks Volker, these look like a helpful start to making this stuff
> simpler.  
> 
> I looked at this set last night, to see how it works with the
> authentication audit code (so I could integrate into that series it as
> promised).  Putting it in before causes rebase pain, so I'll take a
> look at adding it after and see if it is any less difficult.
> 
> Otherwise I'll take the concepts and apply them once we are done.  

Really ? Does this cause much rebase pain ? Yeah, it'll cause
some, but this really does clean up these code paths.

Looking at the previously posted logging patch I can only
find two places where the logging patch touches source3/auth/auth.c:

-------------------------------------------------------------------
diff --git a/source3/auth/auth.c b/source3/auth/auth.c
index 50d0188..c1e82a2 100644
--- a/source3/auth/auth.c
+++ b/source3/auth/auth.c
@@ -310,6 +310,10 @@ NTSTATUS auth_check_ntlm_password(TALLOC_CTX *mem_ctx,
 
        /* failed authentication; check for guest lapping */
 
+       /*
+        * Please try not to change this string, it is probably in use
+        * in audit logging tools
+        */
-------------------------------------------------------------------

and:

-------------------------------------------------------------------
diff --git a/source3/auth/auth.c b/source3/auth/auth.c
index c1e82a2..7ca1aae 100644
--- a/source3/auth/auth.c
+++ b/source3/auth/auth.c
@@ -264,6 +264,7 @@ NTSTATUS auth_check_ntlm_password(TALLOC_CTX *mem_ctx,
        /* successful authentication */
 
        if (NT_STATUS_IS_OK(nt_status)) {
+               struct dom_sid sid = {0};
                unix_username = (*pserver_info)->unix_name;
 
                /* We skip doing this step if the caller asked us not to */
@@ -305,6 +306,19 @@ NTSTATUS auth_check_ntlm_password(TALLOC_CTX *mem_ctx,
                               unix_username));
                }
 
+               nt_status = get_user_sid_info3_and_extra((*pserver_info)->info3,
+                                                        &(*pserver_info)->extra,
+                                                        &sid);
+               if (!NT_STATUS_IS_OK(nt_status)) {
+                       static const struct dom_sid null_sid = {0};
+                       sid = null_sid;
+               }
+               
+               log_authentication_event(user_info, nt_status,
+                                        (*pserver_info)->info3->base.logon_domain.string,
+                                        (*pserver_info)->info3->base.account_name.string,
+                                        unix_username, &sid);
+
                return nt_status;
        }
 
@@ -317,6 +331,9 @@ NTSTATUS auth_check_ntlm_password(TALLOC_CTX *mem_ctx,
        DEBUG(2, ("check_ntlm_password:  Authentication for user [%s] -> [%s] FAILED with error %s\n",
                  user_info->client.account_name, user_info->mapped.account_name,
                  nt_errstr(nt_status)));
+       
+       log_authentication_event(user_info, nt_status, NULL, NULL, NULL, NULL);
+       
        ZERO_STRUCTP(pserver_info);
 
        return nt_status;
-------------------------------------------------------------------

These should be a 2 minute clean-up in a rebase to add
them to the now 2 authoritative places where this function
now collects all the possible returns.

If you can point me at the current logging patchset I'll
even do the rebase for you on top of this patch and send
it as a new patchset to you.

I understand not wanting conflicts, but this one really
should be trivial to solve.

Cheers,

	Jeremy.