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

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Samba - samba-technical mailing list
Fix for https://bugzilla.samba.org/show_bug.cgi?id=12865.
The patches:
- Modify existing tests to handle the NETLOGON messages
- Add new tests to verify NETLOGON
- Add auth logging to netr_ServerAuthenticate3

Review and push appreciated
Gary

0001-tests-auth_log-Modify-existing-tests-to-handle-NETLO.patch (7K) Download Attachment
0002-tests-auth_log-Add-new-tests-for-NETLOGON.patch (14K) Download Attachment
0003-source4-netlogon-Add-authentication-logging-for-Serv.patch (6K) Download Attachment
signature.asc (484 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Samba - samba-technical mailing list
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".

Thanks,

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


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Samba - samba-technical mailing list


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.

>
> Thanks,
>
> Andrew Bartlett
>


signature.asc (484 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Samba - samba-technical mailing list
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

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


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Samba - samba-technical mailing list


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"
  }
}


0001-tests-auth_log-Modify-existing-tests-to-handle-NETLO.patch (7K) Download Attachment
0002-tests-auth_log-Add-new-tests-for-NETLOGON.patch (17K) Download Attachment
0003-source4-netlogon-Add-authentication-logging-for-Serv.patch (6K) Download Attachment
signature.asc (484 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Samba - samba-technical mailing list
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 <[hidden email]>

Can I get a second team reviewer please?

Finally, to be clear, the auth logging is only comprehensive for the AD
DC.  For the file server and classic DC, password changes are not
logged, nor is ServerAuthenticate3.  We may wish to make this clear in
the release notes.  

(As you know too well, while extending this looks trivial at each turn,
but is much more tedious than it looks due to the level of testing we
put it under).

Thanks,

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

auth_log-extra.patch.txt (11K) Download Attachment
auth_log.patch.txt (52K) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Samba - samba-technical mailing list
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 <[hidden email]>
>
> 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.

> From d9ca13b83c3e5d413b58b38e9994747d205b3a93 Mon Sep 17 00:00:00 2001
> From: Gary Lockyer <[hidden email]>
> Date: Mon, 10 Jul 2017 07:46:26 +1200
> Subject: [PATCH 2/6] tests auth_log: Add new tests for NETLOGON
>
> Tests for the logging of NETLOGON authentications in the
> netr_ServerAuthenticate3 message processing
>
> BUG: https://bugzilla.samba.org/show_bug.cgi?id=12865
>
> Signed-off-by: Gary Lockyer <[hidden email]>
> ---
>  python/samba/tests/auth_log_netlogon.py           | 129 ++++++++++++++++
>  python/samba/tests/auth_log_netlogon_bad_creds.py | 176 ++++++++++++++++++++++
>  selftest/knownfail.d/auth-logging                 |   8 +
>  source4/selftest/tests.py                         |  18 +++
>  4 files changed, 331 insertions(+)
>  create mode 100644 python/samba/tests/auth_log_netlogon.py
>  create mode 100644 python/samba/tests/auth_log_netlogon_bad_creds.py
>  create mode 100644 selftest/knownfail.d/auth-logging
>
> diff --git a/python/samba/tests/auth_log_netlogon.py b/python/samba/tests/auth_log_netlogon.py
> new file mode 100644
> index 0000000..bf1ce92
> --- /dev/null
> +++ b/python/samba/tests/auth_log_netlogon.py
> @@ -0,0 +1,129 @@
> +# Unix SMB/CIFS implementation.
> +# Copyright (C) Andrew Bartlett <[hidden email]> 2017
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +#
> +
> +"""
> +    Tests that exercise the auth logging for a successful netlogon attempt
> +
> +    NOTE: As the netlogon authentication is performed once per session,
> +          there is only one test in this routine.  If another test is added
> +          only the test executed first will generate the netlogon auth message
> +"""
> +
> +import samba.tests
> +import os
> +from samba.samdb import SamDB
> +import samba.tests.auth_log_base
> +from samba.credentials import Credentials
> +from samba.dcerpc import netlogon
> +from samba.auth import system_session
> +from samba.tests import delete_force
> +from samba.dsdb import UF_WORKSTATION_TRUST_ACCOUNT, UF_PASSWD_NOTREQD
> +from samba.dcerpc.misc import SEC_CHAN_WKSTA
> +
> +
> +class AuthLogTestsNetLogon(samba.tests.auth_log_base.AuthLogTestBase):
> +
> +    def setUp(self):
> +        super(AuthLogTestsNetLogon, self).setUp()
> +        self.lp      = samba.tests.env_loadparm()
> +        self.creds   = Credentials()
> +
> +        self.session = system_session()
> +        self.ldb = SamDB(
> +            session_info=self.session,
> +            credentials=self.creds,
> +            lp=self.lp)
> +
> +        self.domain        = os.environ["DOMAIN"]
> +        self.netbios_name  = "NetLogonGood"
> +        self.machinepass   = "abcdefghij"
> +        self.remoteAddress = "/root/ncalrpc_as_system"
It would be good to get all these /root/ncalrpc_as_system replaced by a
constant. It is just a great target for typos. We already have one place
that checks it and four places that asks for it. So really would be good
to reference a constant in all those places.

$ git grep /root/ncalrpc
auth/gensec/ncalrpc.c:          cmp = strcmp(unix_path, "/root/ncalrpc_as_system");
python/samba/tests/auth_log_ncalrpc.py:        self.remoteAddress = "/root/ncalrpc_as_system"
python/samba/tests/auth_log_samlogon.py:        self.remoteAddress = "/root/ncalrpc_as_system"
source3/rpc_server/rpc_server.c:                                                                            "/root/ncalrpc_as_system",
source4/rpc_server/dcerpc_server.c:                                                          "/root/ncalrpc_as_system",


> +        self.base_dn       = self.ldb.domain_dn()
> +        self.dn            = ("cn=%s,cn=users,%s" %
> +                              (self.netbios_name, self.base_dn))
> +
> +        utf16pw = unicode(
> +            '"' + self.machinepass.encode('utf-8') + '"', 'utf-8'
> +        ).encode('utf-16-le')
> +        self.ldb.add({
> +            "dn": self.dn,
> +            "objectclass": "computer",
> +            "sAMAccountName": "%s$" % self.netbios_name,
> +            "userAccountControl":
> +                str(UF_WORKSTATION_TRUST_ACCOUNT | UF_PASSWD_NOTREQD),
> +            "unicodePwd": utf16pw})
> +
> +    def tearDown(self):
> +        super(AuthLogTestsNetLogon, self).tearDown()
> +        delete_force(self.ldb, self.dn)
> +
> +    def _test_netlogon(self, binding, checkFunction):
> +
> +        def isLastExpectedMessage(msg):
> +            return (
> +                msg["type"] == "Authorization" and
> +                msg["Authorization"]["serviceDescription"]  == "DCE/RPC" and
> +                msg["Authorization"]["authType"]            == "schannel" and
> +                msg["Authorization"]["transportProtection"] == "SEAL")
> +
> +        if binding:
> +            binding = "[schannel,%s]" % binding
> +        else:
> +            binding = "[schannel]"
> +
> +        machine_creds = Credentials()
> +        machine_creds.guess(self.get_loadparm())
> +        machine_creds.set_secure_channel_type(SEC_CHAN_WKSTA)
> +        machine_creds.set_password(self.machinepass)
> +        machine_creds.set_username(self.netbios_name + "$")
> +
> +        netlogon_conn = netlogon.netlogon("ncalrpc:%s" % binding,
> +                                          self.get_loadparm(),
> +                                          machine_creds)
> +
> +        messages = self.waitForMessages(isLastExpectedMessage, netlogon_conn)
> +        checkFunction(messages)
> +
> +    def netlogon_check(self, messages):
> +
> +        expected_messages = 5
> +        self.assertEquals(expected_messages,
> +                          len(messages),
> +                          "Did not receive the expected number of messages")
> +
> +        # Check the first message it should be an Authorization
> +        msg = messages[0]
> +        self.assertEquals("Authorization", msg["type"])
> +        self.assertEquals("DCE/RPC",
> +                          msg["Authorization"]["serviceDescription"])
> +        self.assertEquals("ncalrpc", msg["Authorization"]["authType"])
> +        self.assertEquals("NONE", msg["Authorization"]["transportProtection"])
> +
> +        # Check the fourth message it should be a NETLOGON Authentication
> +        msg = messages[3]
> +        self.assertEquals("Authentication", msg["type"])
> +        self.assertEquals("NETLOGON",
> +                          msg["Authentication"]["serviceDescription"])
> +        self.assertEquals("ServerAuthenticate",
> +                          msg["Authentication"]["authDescription"])
> +        self.assertEquals("NT_STATUS_OK",
> +                          msg["Authentication"]["status"])
> +        self.assertEquals("HMAC-SHA256",
> +                          msg["Authentication"]["passwordType"])
> +
> +    def test_netlogon(self):
> +        self._test_netlogon("SEAL", self.netlogon_check)
> diff --git a/python/samba/tests/auth_log_netlogon_bad_creds.py b/python/samba/tests/auth_log_netlogon_bad_creds.py
> new file mode 100644
> index 0000000..b9e2fbb
> --- /dev/null
> +++ b/python/samba/tests/auth_log_netlogon_bad_creds.py
> @@ -0,0 +1,176 @@
> +# Unix SMB/CIFS implementation.
> +# Copyright (C) Andrew Bartlett <[hidden email]> 2017
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +#
> +
> +"""
> +    Tests that exercise auth logging for unsuccessful netlogon attempts.
> +
> +    NOTE: netlogon is only done once per session, so this file should only
> +          test failed logons.  Adding a successful case will potentially break
> +          the other tests, depending on the order of execution.
> +"""
> +
> +import samba.tests
> +import os
> +from samba import NTSTATUSError
> +from samba.samdb import SamDB
> +import samba.tests.auth_log_base
> +from samba.credentials import Credentials
> +from samba.dcerpc import netlogon
> +from samba.auth import system_session
> +from samba.tests import delete_force
> +from samba.dsdb import UF_WORKSTATION_TRUST_ACCOUNT, UF_PASSWD_NOTREQD
> +from samba.dcerpc.misc import SEC_CHAN_WKSTA
> +
> +
> +class AuthLogTestsNetLogonBadCreds(samba.tests.auth_log_base.AuthLogTestBase):
> +
> +    def setUp(self):
> +        super(AuthLogTestsNetLogonBadCreds, self).setUp()
> +        self.lp      = samba.tests.env_loadparm()
> +        self.creds   = Credentials()
> +
> +        self.session = system_session()
> +        self.ldb = SamDB(
> +            session_info=self.session,
> +            credentials=self.creds,
> +            lp=self.lp)
> +
> +        self.domain        = os.environ["DOMAIN"]
> +        self.netbios_name  = "NetLogonBad"
> +        self.machinepass   = "abcdefghij"
> +        self.remoteAddress = "/root/ncalrpc_as_system"
Same here.

--
/ Alexander Bokovoy

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Samba - samba-technical mailing list
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 <[hidden email]>
> >
> > 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


Loading...