[PATCH] Fix for Bug 12865 Samba 4.7 auth audit does not track machine account ServerAuthenticate3
Gary Lockyer
gary at catalyst.net.nz
Mon Jul 24 19:11:20 UTC 2017
Updated patch set attached, copyright updated and constant added.
On 18/07/17 18:54, Alexander Bokovoy via samba-technical 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.
>
>> From d9ca13b83c3e5d413b58b38e9994747d205b3a93 Mon Sep 17 00:00:00 2001
>> From: Gary Lockyer <gary at catalyst.net.nz>
>> 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 <gary at catalyst.net.nz>
>> ---
>> 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 <abartlet at samba.org> 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 <abartlet at samba.org> 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.
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-dcerpc.idl-Add-symbolic-constant-for-root-ncalrpc_as.patch
Type: text/x-patch
Size: 927 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20170725/7deb01b2/0001-dcerpc.idl-Add-symbolic-constant-for-root-ncalrpc_as.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-rpc-use-symbolic-constant-to-replace-root-ncalrpc_as.patch
Type: text/x-patch
Size: 2245 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20170725/7deb01b2/0002-rpc-use-symbolic-constant-to-replace-root-ncalrpc_as.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-auth_log-use-symbolic-constant-to-replace-root-ncalr.patch
Type: text/x-patch
Size: 2433 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20170725/7deb01b2/0003-auth_log-use-symbolic-constant-to-replace-root-ncalr.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0004-tests-auth_log-Modify-existing-tests-to-handle-NETLO.patch
Type: text/x-patch
Size: 7181 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20170725/7deb01b2/0004-tests-auth_log-Modify-existing-tests-to-handle-NETLO.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0005-tests-auth_log-Add-new-tests-for-NETLOGON.patch
Type: text/x-patch
Size: 17058 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20170725/7deb01b2/0005-tests-auth_log-Add-new-tests-for-NETLOGON.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0006-source4-netlogon-Add-authentication-logging-for-Serv.patch
Type: text/x-patch
Size: 6721 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20170725/7deb01b2/0006-source4-netlogon-Add-authentication-logging-for-Serv.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: OpenPGP digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20170725/7deb01b2/signature.sig>
More information about the samba-technical
mailing list