[PATCH] Fix for Bug 12865 Samba 4.7 auth audit does not track machine account ServerAuthenticate3
Alexander Bokovoy
ab at samba.org
Tue Jul 18 06:54:33 UTC 2017
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.
--
/ Alexander Bokovoy
More information about the samba-technical
mailing list