[PATCH] Fix for Bug 12865 Samba 4.7 auth audit does not track machine account ServerAuthenticate3

[Andrew Bartlett]

On Tue, 2017-07-18 at 09:54 +0300, Alexander Bokovoy wrote:
> On ti, 18 heinä 2017, Andrew Bartlett via samba-technical wrote:
> > On Tue, 2017-07-18 at 07:43 +1200, Gary Lockyer via samba-technical
> > wrote:
> > > 
> > > On 14/07/17 09:13, Andrew Bartlett via samba-technical wrote:
> > > > On Fri, 2017-07-14 at 09:07 +1200, Gary Lockyer wrote:
> > > > > 
> > > > > On 13/07/17 21:25, Andrew Bartlett via samba-technical wrote:
> > > > > > On Thu, 2017-07-13 at 07:21 +1200, Gary Lockyer via samba-technical
> > > > > > wrote:
> > > > > > > @@ -661,6 +661,14 @@ static const char* get_password_type(const struct auth_usersupplied_info *ui)
> > > > > > >  		   && ui->password.response.nt.length == 0
> > > > > > >  		   && ui->password.response.lanman.length == 0) {
> > > > > > >  		password_type = "No-Password";
> > > > > > > +	} else if (ui->netlogon_trust_account.negotiate_flags
> > > > > > > +		   & NETLOGON_NEG_SUPPORTS_AES) {
> > > > > > > +		password_type = "HMAC-SHA256";
> > > > > > > +	} else if (ui->netlogon_trust_account.negotiate_flags
> > > > > > > +		   & NETLOGON_NEG_STRONG_KEYS) {
> > > > > > > +		;
> > > > > > > +	} else if (strncmp("NETLOGON", ui->service_description, 8) == 0) {
> > > > > > > +		password_type = "DES";
> > > > > > >  	}
> > > > > > >  	return password_type;
> > > > > > 
> > > > > > G'Day Gary,
> > > > > > 
> > > > > > I'm sorry, but this hunk looks wrong, and I don't think it is tested. 
> > > > > > You don't see password_type to "HMAC-MD5" for the STRONG_KEYS case, and
> > > > > > you don't guard the whole logic with strncmp("NETLOGON").  You should
> > > > > > check that, with just strcmp I think, and check against the
> > > > > > auth_description with "ServerAuthenticate".
> > > > > 
> > > > > Yeah sadly I did not test it, I really should know better. I've had a
> > > > > look at writing the tests.  Need to be able to clear the
> > > > > NETLOGON_NEG_SUPPORTS_AES and NETLOGON_NEG_STRONG_KEYS.  Is there a way
> > > > > to do this from Python or should I write a cmocka test to exercise the code.
> > > > 
> > > > Manually send the GetChallenge and ServerAuthenticate3 and check for it
> > > > in the bad password case (with zero'ed authenticators), rather than the
> > > > good password case.  That should be mostly practical.
> > > > 
> > > > Andrew Bartlett
> > > > 
> > > 
> > > Updated patch set attached, with tests for the get_password_type code.
> > > 
> > > Successful Auth message:
> > > 
> > > { "timestamp": "2017-07-18T06:57:18.044871+1200",
> > >   "type": "Authentication",
> > >   "Authentication": {
> > >     "version": {"major": 1, "minor": 0},
> > >    "becameDomain": "ADDOMAIN",
> > >    "authDescription": "ServerAuthenticate",
> > >    "remoteAddress": "ipv4:127.0.0.11:23613",
> > >    "status": "NT_STATUS_OK",
> > >    "serviceDescription": "NETLOGON",
> > >    "localAddress": "ipv4:127.0.0.30:445",
> > >    "clientDomain": "ADDOMAIN",
> > >    "becameSid": "S-1-5-21-957060844-616297711-1930508676-1000",
> > >    "clientAccount": "ADDC$",
> > >    "workstation": null,
> > >    "becameAccount": "ADDC$",
> > >    "mappedAccount": "ADDC$",
> > >    "mappedDomain": null,
> > >    "netlogonComputer": "ADDC",
> > >    "netlogonTrustAccount": "ADDC$",
> > >    "netlogonNegotiateFlags": "0x610FFFFF",
> > >    "netlogonSecureChannelType": 6,
> > >    "netlogonTrustAccountSid":
> > >       "S-1-5-21-957060844-616297711-1930508676-1000",
> > >    "passwordType": "HMAC-SHA256"
> > >   }
> > > }
> > > 
> > > Unsuccessful auth message.
> > > 
> > > { "timestamp": "2017-07-18T06:58:03.113876+1200",
> > >   "type": "Authentication",
> > >   "Authentication": {
> > >     "version": {"major": 1, "minor": 0},
> > >     "becameDomain": "ADDOMAIN",
> > >     "authDescription": "ServerAuthenticate",
> > >     "remoteAddress": "unix:/root/ncalrpc_as_system",
> > >     "status": "NT_STATUS_OK",
> > >     "serviceDescription": "NETLOGON",
> > >     "localAddress":
> > >        "unix:/home/gary/projects/samba03/st/ad_dc/ncalrpc/DEFAULT",
> > >     "clientDomain": "ADDOMAIN",
> > >     "becameSid": "S-1-5-21-957060844-616297711-1930508676-1115",
> > >     "clientAccount": "SamLogonTest$",
> > >     "workstation": null,
> > >     "becameAccount": "SamLogonTest$",
> > >     "mappedAccount": "SamLogonTest$",
> > >     "mappedDomain": null,
> > >     "netlogonComputer": "ADDC",
> > >     "netlogonTrustAccount": "SamLogonTest$",
> > >     "netlogonNegotiateFlags": "0x610FFFFF",
> > >     "netlogonSecureChannelType": 2,
> > >     "netlogonTrustAccountSid":
> > >        "S-1-5-21-957060844-616297711-1930508676-1115",
> > >     "passwordType": "HMAC-SHA256"
> > >   }
> > > }
> > 
> > Almost there.   I'm running a private autobuild with these 3 patches on
> > top.
> > 
> > With these:
> > 
> > Reviewed-by: Andrew Bartlett <abartlet at samba.org>
> > 
> > Can I get a second team reviewer please?
> 
> RB+ by me except few comments below. You did not add your RB+ or
> signed-off-by on all patches, like the following one. It also has your
> copyright instead of Gary's.  If this is so, you'd need to add your
> signed-off-by too.

Thanks.  We will tidy it up next week. 

Andrew Bartlett
-- 
Andrew Bartlett                       http://samba.org/~abartlet/
Authentication Developer, Samba Team  http://samba.org
Samba Developer, Catalyst IT          http://catalyst.net.nz/services/samba