From bf0f3b444a27a32fa893018d2217253315f04140 Mon Sep 17 00:00:00 2001 From: Gary Lockyer Date: Mon, 24 Jul 2017 10:55:48 +1200 Subject: [PATCH 1/9] dcerpc.idl Add symbolic constant for /root/ncalrpc_as_system This is string is used several places in the code and tests, so it should be a constant. BUG: https://bugzilla.samba.org/show_bug.cgi?id=12865 Signed-off-by: Gary Lockyer Reviewed-by: Andrew Bartlett Reviewed-by: Alexander Bokovoy (cherry picked from commit 6ab9f789ff6e6328cf222fdb1a39457af7ed58b4) --- librpc/idl/dcerpc.idl | 1 + 1 file changed, 1 insertion(+) diff --git a/librpc/idl/dcerpc.idl b/librpc/idl/dcerpc.idl index 1e06bc144d7..bbb17f0b8c4 100644 --- a/librpc/idl/dcerpc.idl +++ b/librpc/idl/dcerpc.idl @@ -247,6 +247,7 @@ interface dcerpc DCERPC_AUTH_TYPE_MSMQ = 100, DCERPC_AUTH_TYPE_NCALRPC_AS_SYSTEM = 200 } dcerpc_AuthType; + const char *AS_SYSTEM_MAGIC_PATH_TOKEN = "/root/ncalrpc_as_system"; typedef [enum8bit] enum { DCERPC_AUTH_LEVEL_NONE = 1, -- 2.11.0 From 779e535f194c560423bdd76dd5005c09819b9d44 Mon Sep 17 00:00:00 2001 From: Gary Lockyer Date: Mon, 24 Jul 2017 11:00:45 +1200 Subject: [PATCH 2/9] rpc: use symbolic constant to replace /root/ncalrpc_as_system Modified to use constant AS_SYSTEM_MAGIC_PATH_TOKEN instead of string literal "/root/ncalrpc_as_system" BUG: https://bugzilla.samba.org/show_bug.cgi?id=12865 Signed-off-by: Gary Lockyer Reviewed-by: Andrew Bartlett Reviewed-by: Alexander Bokovoy (cherry picked from commit 1898096c7ecef4c323b14b7cf30db4283386f913) --- auth/gensec/ncalrpc.c | 2 +- source3/rpc_server/rpc_server.c | 2 +- source4/rpc_server/dcerpc_server.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/auth/gensec/ncalrpc.c b/auth/gensec/ncalrpc.c index f28a1c43b49..70b3bb5ea39 100644 --- a/auth/gensec/ncalrpc.c +++ b/auth/gensec/ncalrpc.c @@ -203,7 +203,7 @@ static NTSTATUS gensec_ncalrpc_update_internal( return NT_STATUS_LOGON_FAILURE; } - cmp = strcmp(unix_path, "/root/ncalrpc_as_system"); + cmp = strcmp(unix_path, AS_SYSTEM_MAGIC_PATH_TOKEN); TALLOC_FREE(unix_path); if (cmp != 0) { state->step = GENSEC_NCALRPC_ERROR; diff --git a/source3/rpc_server/rpc_server.c b/source3/rpc_server/rpc_server.c index 81cc6756891..e15cd205cdc 100644 --- a/source3/rpc_server/rpc_server.c +++ b/source3/rpc_server/rpc_server.c @@ -1044,7 +1044,7 @@ void dcerpc_ncacn_accept(struct tevent_context *ev_ctx, TALLOC_FREE(ncacn_conn->remote_client_addr); rc = tsocket_address_unix_from_path(ncacn_conn, - "/root/ncalrpc_as_system", + AS_SYSTEM_MAGIC_PATH_TOKEN, &ncacn_conn->remote_client_addr); if (rc < 0) { DEBUG(0, ("Out of memory building magic ncalrpc_as_system path!\n")); diff --git a/source4/rpc_server/dcerpc_server.c b/source4/rpc_server/dcerpc_server.c index de919e263ff..ef02e324ca8 100644 --- a/source4/rpc_server/dcerpc_server.c +++ b/source4/rpc_server/dcerpc_server.c @@ -2692,7 +2692,7 @@ static void dcesrv_sock_accept(struct stream_connection *srv_conn) struct tsocket_address *r = NULL; ret = tsocket_address_unix_from_path(dcesrv_conn, - "/root/ncalrpc_as_system", + AS_SYSTEM_MAGIC_PATH_TOKEN, &r); if (ret == -1) { status = map_nt_error_from_unix_common(errno); -- 2.11.0 From 8bb11a0e8a820137d366ce4c09d9e087fb126c66 Mon Sep 17 00:00:00 2001 From: Gary Lockyer Date: Mon, 24 Jul 2017 10:59:18 +1200 Subject: [PATCH 3/9] auth_log: use symbolic constant to replace /root/ncalrpc_as_system Modified to use constant AS_SYSTEM_MAGIC_PATH_TOKEN instead of string literal "/root/ncalrpc_as_system" Signed-off-by: Gary Lockyer Reviewed-by: Andrew Bartlett Reviewed-by: Alexander Bokovoy (cherry picked from commit ddfe8aa9cccd78426456b6397bc7b352d9705648) --- python/samba/tests/auth_log_ncalrpc.py | 3 ++- python/samba/tests/auth_log_samlogon.py | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/python/samba/tests/auth_log_ncalrpc.py b/python/samba/tests/auth_log_ncalrpc.py index 2538c61c56e..be7f6b2b6c2 100644 --- a/python/samba/tests/auth_log_ncalrpc.py +++ b/python/samba/tests/auth_log_ncalrpc.py @@ -22,6 +22,7 @@ from samba import auth import samba.tests from samba.messaging import Messaging from samba.dcerpc.messaging import MSG_AUTH_LOG, AUTH_EVENT_NAME +from samba.dcerpc.dcerpc import AS_SYSTEM_MAGIC_PATH_TOKEN from samba.dcerpc import samr import time import json @@ -35,7 +36,7 @@ class AuthLogTestsNcalrpc(samba.tests.auth_log_base.AuthLogTestBase): def setUp(self): super(AuthLogTestsNcalrpc, self).setUp() - self.remoteAddress = "/root/ncalrpc_as_system" + self.remoteAddress = AS_SYSTEM_MAGIC_PATH_TOKEN def tearDown(self): super(AuthLogTestsNcalrpc , self).tearDown() diff --git a/python/samba/tests/auth_log_samlogon.py b/python/samba/tests/auth_log_samlogon.py index d24986b68a5..548082c5c7c 100644 --- a/python/samba/tests/auth_log_samlogon.py +++ b/python/samba/tests/auth_log_samlogon.py @@ -35,6 +35,7 @@ from samba.credentials import ( CLI_CRED_NTLMv2_AUTH ) from samba.dcerpc import ntlmssp, netlogon +from samba.dcerpc.dcerpc import AS_SYSTEM_MAGIC_PATH_TOKEN from samba.ndr import ndr_pack from samba.auth import system_session from samba.tests import delete_force @@ -57,7 +58,7 @@ class AuthLogTestsSamLogon(samba.tests.auth_log_base.AuthLogTestBase): self.domain = os.environ["DOMAIN"] self.netbios_name = "SamLogonTest" self.machinepass = "abcdefghij" - self.remoteAddress = "/root/ncalrpc_as_system" + self.remoteAddress = AS_SYSTEM_MAGIC_PATH_TOKEN self.base_dn = self.ldb.domain_dn() self.samlogon_dn = ("cn=%s,cn=users,%s" % (self.netbios_name, self.base_dn)) -- 2.11.0 From 3e27172bffe637a73940730608332685ab94880d Mon Sep 17 00:00:00 2001 From: Gary Lockyer Date: Mon, 10 Jul 2017 07:45:16 +1200 Subject: [PATCH 4/9] tests auth_log: Modify existing tests to handle NETLOGON messages Modify the existing tests to ignore auth logging for NETLOGON messages. NETLOGON authentication is logged once per session, and is tested separately. Ignoring it in these tests avoids order dependencies. BUG: https://bugzilla.samba.org/show_bug.cgi?id=12865 Signed-off-by: Gary Lockyer Reviewed-by: Andrew Bartlett Reviewed-by: Alexander Bokovoy (cherry picked from commit 5c27c5b6efb4226aa8bdaf4e5cbb770f8b3ef22f) --- python/samba/tests/auth_log.py | 11 +++++++++++ python/samba/tests/auth_log_base.py | 17 +++++++++++++++++ python/samba/tests/auth_log_samlogon.py | 1 + 3 files changed, 29 insertions(+) diff --git a/python/samba/tests/auth_log.py b/python/samba/tests/auth_log.py index 65800c99181..6b032a80edc 100644 --- a/python/samba/tests/auth_log.py +++ b/python/samba/tests/auth_log.py @@ -991,6 +991,7 @@ class AuthLogTests(samba.tests.auth_log_base.AuthLogTestBase): call(["bin/rpcclient", "-c", samlogon, "-U%", server]) messages = self.waitForMessages( isLastExpectedMessage) + messages = self.remove_netlogon_messages(messages) received = len(messages) self.assertIs(True, (received == 5 or received == 6), @@ -1020,6 +1021,7 @@ class AuthLogTests(samba.tests.auth_log_base.AuthLogTestBase): call(["bin/rpcclient", "-c", samlogon, "-U%", server]) messages = self.waitForMessages( isLastExpectedMessage) + messages = self.remove_netlogon_messages(messages) received = len(messages) self.assertIs(True, (received == 5 or received == 6), @@ -1049,6 +1051,7 @@ class AuthLogTests(samba.tests.auth_log_base.AuthLogTestBase): call(["bin/rpcclient", "-c", samlogon, "-U%", server]) messages = self.waitForMessages( isLastExpectedMessage) + messages = self.remove_netlogon_messages(messages) received = len(messages) self.assertIs(True, (received == 5 or received == 6), @@ -1077,6 +1080,7 @@ class AuthLogTests(samba.tests.auth_log_base.AuthLogTestBase): call(["bin/rpcclient", "-c", samlogon, "-U%", server]) messages = self.waitForMessages( isLastExpectedMessage) + messages = self.remove_netlogon_messages(messages) received = len(messages) self.assertIs(True, (received == 5 or received == 6), @@ -1106,6 +1110,7 @@ class AuthLogTests(samba.tests.auth_log_base.AuthLogTestBase): call(["bin/rpcclient", "-c", samlogon, "-U%", server]) messages = self.waitForMessages( isLastExpectedMessage) + messages = self.remove_netlogon_messages(messages) received = len(messages) self.assertIs(True, (received == 5 or received == 6), @@ -1135,6 +1140,7 @@ class AuthLogTests(samba.tests.auth_log_base.AuthLogTestBase): call(["bin/rpcclient", "-c", samlogon, "-U%", server]) messages = self.waitForMessages( isLastExpectedMessage) + messages = self.remove_netlogon_messages(messages) received = len(messages) self.assertIs(True, (received == 5 or received == 6), @@ -1164,6 +1170,7 @@ class AuthLogTests(samba.tests.auth_log_base.AuthLogTestBase): call(["bin/rpcclient", "-c", samlogon, "-U%", server]) messages = self.waitForMessages( isLastExpectedMessage) + messages = self.remove_netlogon_messages(messages) received = len(messages) self.assertIs(True, (received == 5 or received == 6), @@ -1194,6 +1201,7 @@ class AuthLogTests(samba.tests.auth_log_base.AuthLogTestBase): call(["bin/rpcclient", "-c", samlogon, "-U%", server]) messages = self.waitForMessages( isLastExpectedMessage) + messages = self.remove_netlogon_messages(messages) received = len(messages) self.assertIs(True, (received == 5 or received == 6), @@ -1224,6 +1232,7 @@ class AuthLogTests(samba.tests.auth_log_base.AuthLogTestBase): call(["bin/rpcclient", "-c", samlogon, "-U%", server]) messages = self.waitForMessages( isLastExpectedMessage) + messages = self.remove_netlogon_messages(messages) received = len(messages) self.assertIs(True, (received == 5 or received == 6), @@ -1252,6 +1261,7 @@ class AuthLogTests(samba.tests.auth_log_base.AuthLogTestBase): call(["bin/rpcclient", "-c", samlogon, "-U%", server]) messages = self.waitForMessages( isLastExpectedMessage) + messages = self.remove_netlogon_messages(messages) received = len(messages) self.assertIs(True, (received == 5 or received == 6), @@ -1290,6 +1300,7 @@ class AuthLogTests(samba.tests.auth_log_base.AuthLogTestBase): call(["bin/rpcclient", "-c", samlogon, "-U%", server]) messages = self.waitForMessages( isLastExpectedMessage) + messages = self.remove_netlogon_messages(messages) received = len(messages) self.assertIs(True, (received == 5 or received == 6), diff --git a/python/samba/tests/auth_log_base.py b/python/samba/tests/auth_log_base.py index e9ae4644259..aefd57e5516 100644 --- a/python/samba/tests/auth_log_base.py +++ b/python/samba/tests/auth_log_base.py @@ -62,6 +62,10 @@ class AuthLogTestBase(samba.tests.TestCase): def waitForMessages(self, isLastExpectedMessage, connection=None): + """Wait for all the expected messages to arrive + The connection is passed through to keep the connection alive + until all the logging messages have been received. + """ def completed( messages): for message in messages: @@ -102,3 +106,16 @@ class AuthLogTestBase(samba.tests.TestCase): while len( self.context["messages"]): self.msg_ctx.loop_once(0.001) self.context["messages"] = [] + + # Remove any NETLOGON authentication messages + # NETLOGON is only performed once per session, so to avoid ordering + # dependencies within the tests it's best to strip out NETLOGON messages. + # + def remove_netlogon_messages(self, messages): + def is_not_netlogon(msg): + if "Authentication" not in msg: + return True + sd = msg["Authentication"]["serviceDescription"] + return sd != "NETLOGON" + + return list(filter(is_not_netlogon, messages)) diff --git a/python/samba/tests/auth_log_samlogon.py b/python/samba/tests/auth_log_samlogon.py index 548082c5c7c..a3a9f50ecfc 100644 --- a/python/samba/tests/auth_log_samlogon.py +++ b/python/samba/tests/auth_log_samlogon.py @@ -158,6 +158,7 @@ class AuthLogTestsSamLogon(samba.tests.auth_log_base.AuthLogTestBase): def samlogon_check(self, messages): + messages = self.remove_netlogon_messages(messages) expected_messages = 5 self.assertEquals(expected_messages, len(messages), -- 2.11.0 From 79adb5d5eea2d00d8abb29b8e0caf28b7b471594 Mon Sep 17 00:00:00 2001 From: Gary Lockyer Date: Mon, 10 Jul 2017 07:46:26 +1200 Subject: [PATCH 5/9] tests auth_log: Add new tests for NETLOGON Tests for the logging of NETLOGON authentications in the netr_ServerAuthenticate3 message processing Test code based on the existing auth_log tests. BUG: https://bugzilla.samba.org/show_bug.cgi?id=12865 Signed-off-by: Gary Lockyer Reviewed-by: Andrew Bartlett Reviewed-by: Alexander Bokovoy (cherry picked from commit f3d3e6da5a42833b8de86e9b7c0aa1c56e1c4e80) --- python/samba/tests/auth_log_netlogon.py | 131 ++++++++++++++++ python/samba/tests/auth_log_netlogon_bad_creds.py | 178 ++++++++++++++++++++++ selftest/knownfail.d/auth-logging | 8 + source4/selftest/tests.py | 18 +++ 4 files changed, 335 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 00000000000..228fbe9b14d --- /dev/null +++ b/python/samba/tests/auth_log_netlogon.py @@ -0,0 +1,131 @@ +# Unix SMB/CIFS implementation. +# Copyright (C) Andrew Bartlett 2017 +# Copyright (C) Catalyst IT Ltd. 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 . +# + +""" + 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.dcerpc.dcerpc import AS_SYSTEM_MAGIC_PATH_TOKEN +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 = AS_SYSTEM_MAGIC_PATH_TOKEN + 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 00000000000..fedd8a1d412 --- /dev/null +++ b/python/samba/tests/auth_log_netlogon_bad_creds.py @@ -0,0 +1,178 @@ +# Unix SMB/CIFS implementation. +# Copyright (C) Andrew Bartlett 2017 +# Copyright (C) Catalyst IT Ltd. 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 . +# + +""" + 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.dcerpc.dcerpc import AS_SYSTEM_MAGIC_PATH_TOKEN +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 = AS_SYSTEM_MAGIC_PATH_TOKEN + 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(AuthLogTestsNetLogonBadCreds, self).tearDown() + delete_force(self.ldb, self.dn) + + def _test_netlogon(self, name, pwd, status, checkFunction): + + def isLastExpectedMessage(msg): + return ( + msg["type"] == "Authentication" and + msg["Authentication"]["serviceDescription"] == "NETLOGON" and + msg["Authentication"]["authDescription"] == + "ServerAuthenticate" and + msg["Authentication"]["status"] == status) + + machine_creds = Credentials() + machine_creds.guess(self.get_loadparm()) + machine_creds.set_secure_channel_type(SEC_CHAN_WKSTA) + machine_creds.set_password(pwd) + machine_creds.set_username(name + "$") + + try: + netlogon.netlogon("ncalrpc:[schannel]", + self.get_loadparm(), + machine_creds) + self.fail("NTSTATUSError not raised") + except NTSTATUSError: + pass + + messages = self.waitForMessages(isLastExpectedMessage) + checkFunction(messages) + + def netlogon_check(self, messages): + + expected_messages = 4 + 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"]) + + def test_netlogon_bad_machine_name(self): + self._test_netlogon("bad_name", + self.machinepass, + "NT_STATUS_NO_TRUST_SAM_ACCOUNT", + self.netlogon_check) + + def test_netlogon_bad_password(self): + self._test_netlogon(self.netbios_name, + "badpass", + "NT_STATUS_ACCESS_DENIED", + self.netlogon_check) + + def test_netlogon_password_DES(self): + """Logon failure that exercises the "DES" passwordType path. + """ + def isLastExpectedMessage(msg): + return ( + msg["type"] == "Authentication" and + msg["Authentication"]["serviceDescription"] == "NETLOGON" and + msg["Authentication"]["authDescription"] == + "ServerAuthenticate" and + msg["Authentication"]["passwordType"] == "DES") + + c = netlogon.netlogon("ncalrpc:[schannel]", self.get_loadparm()) + creds = netlogon.netr_Credential() + c.netr_ServerReqChallenge(self.server, self.netbios_name, creds) + try: + c.netr_ServerAuthenticate3(self.server, + self.netbios_name, + SEC_CHAN_WKSTA, + self.netbios_name, + creds, + 0) + except NTSTATUSError: + pass + self.waitForMessages(isLastExpectedMessage) + + def test_netlogon_password_HMAC_MD5(self): + """Logon failure that exercises the "HMAC-MD5" passwordType path. + """ + def isLastExpectedMessage(msg): + return ( + msg["type"] == "Authentication" and + msg["Authentication"]["serviceDescription"] == "NETLOGON" and + msg["Authentication"]["authDescription"] == + "ServerAuthenticate" and + msg["Authentication"]["passwordType"] == "HMAC-MD5") + c = netlogon.netlogon("ncalrpc:[schannel]", self.get_loadparm()) + creds = netlogon.netr_Credential() + c.netr_ServerReqChallenge(self.server, self.netbios_name, creds) + try: + c.netr_ServerAuthenticate3(self.server, + self.netbios_name, + SEC_CHAN_WKSTA, + self.netbios_name, + creds, + 0x00004000) + except NTSTATUSError: + pass + self.waitForMessages(isLastExpectedMessage) diff --git a/selftest/knownfail.d/auth-logging b/selftest/knownfail.d/auth-logging new file mode 100644 index 00000000000..1f3532d4a34 --- /dev/null +++ b/selftest/knownfail.d/auth-logging @@ -0,0 +1,8 @@ +# NETLOGON authentication logging tests, currently fail as the +# code has not been implemented +^samba.tests.auth_log_netlogon_bad_creds.samba.tests.auth_log_netlogon_bad_creds.AuthLogTestsNetLogonBadCreds.test_netlogon_bad_password\(ad_dc_ntvfs:local\) +^samba.tests.auth_log_netlogon_bad_creds.samba.tests.auth_log_netlogon_bad_creds.AuthLogTestsNetLogonBadCreds.test_netlogon_bad_machine_name\(ad_dc_ntvfs:local\) +^samba.tests.auth_log_netlogon_bad_creds.samba.tests.auth_log_netlogon_bad_creds.AuthLogTestsNetLogonBadCreds.test_netlogon_bad_password\(ad_dc:local\) +^samba.tests.auth_log_netlogon_bad_creds.samba.tests.auth_log_netlogon_bad_creds.AuthLogTestsNetLogonBadCreds.test_netlogon_bad_machine_name\(ad_dc:local\) +^samba.tests.auth_log_netlogon.samba.tests.auth_log_netlogon.AuthLogTestsNetLogon.test_netlogon\(ad_dc_ntvfs:local\) +^samba.tests.auth_log_netlogon.samba.tests.auth_log_netlogon.AuthLogTestsNetLogon.test_netlogon\(ad_dc:local\) diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py index 15037a20608..465a15bfa9b 100755 --- a/source4/selftest/tests.py +++ b/source4/selftest/tests.py @@ -636,6 +636,24 @@ if have_jansson_support and have_heimdal_support: extra_args=['-U"$USERNAME%$PASSWORD"'], environ={'CLIENT_IP': '127.0.0.11', 'SOCKET_WRAPPER_DEFAULT_IFACE': 11}) + planoldpythontestsuite("ad_dc:local", "samba.tests.auth_log_netlogon", + extra_args=['-U"$USERNAME%$PASSWORD"'], + environ={'CLIENT_IP': '127.0.0.11', + 'SOCKET_WRAPPER_DEFAULT_IFACE': 11}) + planoldpythontestsuite("ad_dc_ntvfs:local", "samba.tests.auth_log_netlogon", + extra_args=['-U"$USERNAME%$PASSWORD"'], + environ={'CLIENT_IP': '127.0.0.11', + 'SOCKET_WRAPPER_DEFAULT_IFACE': 11}) + planoldpythontestsuite("ad_dc:local", + "samba.tests.auth_log_netlogon_bad_creds", + extra_args=['-U"$USERNAME%$PASSWORD"'], + environ={'CLIENT_IP': '127.0.0.11', + 'SOCKET_WRAPPER_DEFAULT_IFACE': 11}) + planoldpythontestsuite("ad_dc_ntvfs:local", + "samba.tests.auth_log_netlogon_bad_creds", + extra_args=['-U"$USERNAME%$PASSWORD"'], + environ={'CLIENT_IP': '127.0.0.11', + 'SOCKET_WRAPPER_DEFAULT_IFACE': 11}) planoldpythontestsuite("ad_dc", "samba.tests.net_join_no_spnego", extra_args=['-U"$USERNAME%$PASSWORD"']) -- 2.11.0 From 578f70accc71498f03e78dc07f039f35db52f27d Mon Sep 17 00:00:00 2001 From: Gary Lockyer Date: Mon, 10 Jul 2017 07:48:08 +1200 Subject: [PATCH 6/9] source4 netlogon: Add authentication logging for ServerAuthenticate3 Log NETLOGON authentication activity by instrumenting the netr_ServerAuthenticate3 processing. BUG: https://bugzilla.samba.org/show_bug.cgi?id=12865 Signed-off-by: Gary Lockyer Reviewed-by: Andrew Bartlett Reviewed-by: Alexander Bokovoy (cherry picked from commit efc335a03062740f51a6edd09d765a8b77e239c5) --- auth/auth_log.c | 12 ++++ selftest/knownfail.d/auth-logging | 8 --- source4/rpc_server/netlogon/dcerpc_netlogon.c | 90 ++++++++++++++++++--------- 3 files changed, 72 insertions(+), 38 deletions(-) delete mode 100644 selftest/knownfail.d/auth-logging diff --git a/auth/auth_log.c b/auth/auth_log.c index 9dbf8f210fc..d4c6c445bed 100644 --- a/auth/auth_log.c +++ b/auth/auth_log.c @@ -639,6 +639,18 @@ static const char* get_password_type(const struct auth_usersupplied_info *ui) if (ui->password_type != NULL) { password_type = ui->password_type; + } else if (ui->auth_description != NULL && + strncmp("ServerAuthenticate", ui->auth_description, 18) == 0) + { + 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) { + password_type = "HMAC-MD5"; + } else { + password_type = "DES"; + } } else if (ui->password_state == AUTH_PASSWORD_RESPONSE && (ui->logon_parameters & MSV1_0_ALLOW_MSVCHAPV2) && ui->password.response.nt.length == 24) { diff --git a/selftest/knownfail.d/auth-logging b/selftest/knownfail.d/auth-logging deleted file mode 100644 index 1f3532d4a34..00000000000 --- a/selftest/knownfail.d/auth-logging +++ /dev/null @@ -1,8 +0,0 @@ -# NETLOGON authentication logging tests, currently fail as the -# code has not been implemented -^samba.tests.auth_log_netlogon_bad_creds.samba.tests.auth_log_netlogon_bad_creds.AuthLogTestsNetLogonBadCreds.test_netlogon_bad_password\(ad_dc_ntvfs:local\) -^samba.tests.auth_log_netlogon_bad_creds.samba.tests.auth_log_netlogon_bad_creds.AuthLogTestsNetLogonBadCreds.test_netlogon_bad_machine_name\(ad_dc_ntvfs:local\) -^samba.tests.auth_log_netlogon_bad_creds.samba.tests.auth_log_netlogon_bad_creds.AuthLogTestsNetLogonBadCreds.test_netlogon_bad_password\(ad_dc:local\) -^samba.tests.auth_log_netlogon_bad_creds.samba.tests.auth_log_netlogon_bad_creds.AuthLogTestsNetLogonBadCreds.test_netlogon_bad_machine_name\(ad_dc:local\) -^samba.tests.auth_log_netlogon.samba.tests.auth_log_netlogon.AuthLogTestsNetLogon.test_netlogon\(ad_dc_ntvfs:local\) -^samba.tests.auth_log_netlogon.samba.tests.auth_log_netlogon.AuthLogTestsNetLogon.test_netlogon\(ad_dc:local\) diff --git a/source4/rpc_server/netlogon/dcerpc_netlogon.c b/source4/rpc_server/netlogon/dcerpc_netlogon.c index b50b7a52980..c140ee8e162 100644 --- a/source4/rpc_server/netlogon/dcerpc_netlogon.c +++ b/source4/rpc_server/netlogon/dcerpc_netlogon.c @@ -105,8 +105,15 @@ static NTSTATUS dcesrv_netr_ServerReqChallenge(struct dcesrv_call_state *dce_cal return NT_STATUS_OK; } -static NTSTATUS dcesrv_netr_ServerAuthenticate3(struct dcesrv_call_state *dce_call, TALLOC_CTX *mem_ctx, - struct netr_ServerAuthenticate3 *r) +/* + * Do the actual processing of a netr_ServerAuthenticate3 message. + * called from dcesrv_netr_ServerAuthenticate3, which handles the logging. + */ +static NTSTATUS dcesrv_netr_ServerAuthenticate3_helper( + struct dcesrv_call_state *dce_call, + TALLOC_CTX *mem_ctx, + struct netr_ServerAuthenticate3 *r, + struct dom_sid **sid) { struct netlogon_server_pipe_state *pipe_state = talloc_get_type(dce_call->context->private_data, struct netlogon_server_pipe_state); @@ -469,36 +476,11 @@ static NTSTATUS dcesrv_netr_ServerAuthenticate3(struct dcesrv_call_state *dce_ca negotiate_flags); } - { - char* local = NULL; - char* remote = NULL; - TALLOC_CTX *frame = talloc_stackframe(); - - remote = tsocket_address_string(dce_call->conn->remote_address, - frame); - local = tsocket_address_string(dce_call->conn->local_address, - frame); - if (creds == NULL) { - DEBUG(2, ("Failed to authenticate NETLOGON " - "account[%s] workstation[%s] " - "remote[%s] local[%s]\n", - log_escape(frame, r->in.account_name), - log_escape(frame, r->in.computer_name), - remote, local)); - TALLOC_FREE(frame); - return NT_STATUS_ACCESS_DENIED; - } else { - DEBUG(3, ("Successful authenticate of NETLOGON " - "account[%s] workstation[%s] " - "remote[%s] local[%s]\n", - log_escape(frame, r->in.account_name), - log_escape(frame, r->in.computer_name), - remote, local)); - TALLOC_FREE(frame); - } + if (creds == NULL) { + return NT_STATUS_ACCESS_DENIED; } - creds->sid = samdb_result_dom_sid(creds, msgs[0], "objectSid"); + *sid = talloc_memdup(mem_ctx, creds->sid, sizeof(struct dom_sid)); nt_status = schannel_save_creds_state(mem_ctx, dce_call->conn->dce_ctx->lp_ctx, @@ -514,6 +496,54 @@ static NTSTATUS dcesrv_netr_ServerAuthenticate3(struct dcesrv_call_state *dce_ca return NT_STATUS_OK; } +/* + * Log a netr_ServerAuthenticate3 request, and then invoke + * dcesrv_netr_ServerAuthenticate3_helper to perform the actual processing + */ +static NTSTATUS dcesrv_netr_ServerAuthenticate3( + struct dcesrv_call_state *dce_call, + TALLOC_CTX *mem_ctx, + struct netr_ServerAuthenticate3 *r) +{ + NTSTATUS status; + struct dom_sid *sid = NULL; + struct auth_usersupplied_info ui = { + .local_host = dce_call->conn->local_address, + .remote_host = dce_call->conn->remote_address, + .client = { + .account_name = r->in.account_name, + .domain_name = lpcfg_workgroup(dce_call->conn->dce_ctx->lp_ctx), + }, + .service_description = "NETLOGON", + .auth_description = "ServerAuthenticate", + .netlogon_trust_account = { + .computer_name = r->in.computer_name, + .account_name = r->in.account_name, + .negotiate_flags = *r->in.negotiate_flags, + .secure_channel_type = r->in.secure_channel_type, + }, + .mapped = { + .account_name = r->in.account_name, + } + }; + + status = dcesrv_netr_ServerAuthenticate3_helper(dce_call, + mem_ctx, + r, + &sid); + ui.netlogon_trust_account.sid = sid; + log_authentication_event( + dce_call->conn->msg_ctx, + dce_call->conn->dce_ctx->lp_ctx, + &ui, + status, + lpcfg_workgroup(dce_call->conn->dce_ctx->lp_ctx), + r->in.account_name, + NULL, + sid); + + return status; +} static NTSTATUS dcesrv_netr_ServerAuthenticate(struct dcesrv_call_state *dce_call, TALLOC_CTX *mem_ctx, struct netr_ServerAuthenticate *r) { -- 2.11.0 From d14736ac5a579774998a7e74c6485587857e6e13 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Tue, 18 Jul 2017 08:46:08 +1200 Subject: [PATCH 7/9] s4-netlogon: Extend ServerAuthenticate3 logging to split up username forms This splits out the username into the input, mapped and obtained just as we do elsewhere. Signed-off-by: Andrew Bartlett Reviewed-by: Alexander Bokovoy (cherry picked from commit abd821b76b27eb8d9bc2f8acfcf9d98caf015f5f) --- source4/rpc_server/netlogon/dcerpc_netlogon.c | 37 ++++++++++++++++++--------- 1 file changed, 25 insertions(+), 12 deletions(-) diff --git a/source4/rpc_server/netlogon/dcerpc_netlogon.c b/source4/rpc_server/netlogon/dcerpc_netlogon.c index c140ee8e162..89ceabe21b3 100644 --- a/source4/rpc_server/netlogon/dcerpc_netlogon.c +++ b/source4/rpc_server/netlogon/dcerpc_netlogon.c @@ -111,8 +111,10 @@ static NTSTATUS dcesrv_netr_ServerReqChallenge(struct dcesrv_call_state *dce_cal */ static NTSTATUS dcesrv_netr_ServerAuthenticate3_helper( struct dcesrv_call_state *dce_call, - TALLOC_CTX *mem_ctx, + TALLOC_CTX *mem_ctx, struct netr_ServerAuthenticate3 *r, + const char **trust_account_for_search, + const char **trust_account_in_db, struct dom_sid **sid) { struct netlogon_server_pipe_state *pipe_state = @@ -128,8 +130,7 @@ static NTSTATUS dcesrv_netr_ServerAuthenticate3_helper( struct ldb_message **msgs; NTSTATUS nt_status; const char *attrs[] = {"unicodePwd", "userAccountControl", - "objectSid", NULL}; - const char *account_name; + "objectSid", "samAccountName", NULL}; uint32_t server_flags = 0; uint32_t negotiate_flags = 0; bool allow_nt4_crypto = lpcfg_allow_nt4_crypto(dce_call->conn->dce_ctx->lp_ctx); @@ -368,18 +369,19 @@ static NTSTATUS dcesrv_netr_ServerAuthenticate3_helper( return NT_STATUS_NO_TRUST_SAM_ACCOUNT; } - account_name = talloc_asprintf(mem_ctx, "%s$", flatname); - if (account_name == NULL) { + *trust_account_for_search = talloc_asprintf(mem_ctx, "%s$", flatname); + if (*trust_account_for_search == NULL) { return NT_STATUS_NO_MEMORY; } } else { - account_name = r->in.account_name; + *trust_account_for_search = r->in.account_name; } /* pull the user attributes */ num_records = gendb_search(sam_ctx, mem_ctx, NULL, &msgs, attrs, "(&(sAMAccountName=%s)(objectclass=user))", - ldb_binary_encode_string(mem_ctx, account_name)); + ldb_binary_encode_string(mem_ctx, + *trust_account_for_search)); if (num_records == 0) { DEBUG(3,("Couldn't find user [%s] in samdb.\n", @@ -392,6 +394,15 @@ static NTSTATUS dcesrv_netr_ServerAuthenticate3_helper( return NT_STATUS_INTERNAL_DB_CORRUPTION; } + *trust_account_in_db = ldb_msg_find_attr_as_string(msgs[0], + "samAccountName", + NULL); + if (*trust_account_in_db == NULL) { + DEBUG(0,("No samAccountName returned in record matching user [%s]\n", + r->in.account_name)); + return NT_STATUS_INTERNAL_DB_CORRUPTION; + } + user_account_control = ldb_msg_find_attr_as_uint(msgs[0], "userAccountControl", 0); if (user_account_control & UF_ACCOUNTDISABLE) { @@ -507,6 +518,8 @@ static NTSTATUS dcesrv_netr_ServerAuthenticate3( { NTSTATUS status; struct dom_sid *sid = NULL; + const char *trust_account_for_search = NULL; + const char *trust_account_in_db = NULL; struct auth_usersupplied_info ui = { .local_host = dce_call->conn->local_address, .remote_host = dce_call->conn->remote_address, @@ -518,27 +531,27 @@ static NTSTATUS dcesrv_netr_ServerAuthenticate3( .auth_description = "ServerAuthenticate", .netlogon_trust_account = { .computer_name = r->in.computer_name, - .account_name = r->in.account_name, .negotiate_flags = *r->in.negotiate_flags, .secure_channel_type = r->in.secure_channel_type, }, - .mapped = { - .account_name = r->in.account_name, - } }; status = dcesrv_netr_ServerAuthenticate3_helper(dce_call, mem_ctx, r, + &trust_account_for_search, + &trust_account_in_db, &sid); ui.netlogon_trust_account.sid = sid; + ui.netlogon_trust_account.account_name = trust_account_in_db; + ui.mapped.account_name = trust_account_for_search; log_authentication_event( dce_call->conn->msg_ctx, dce_call->conn->dce_ctx->lp_ctx, &ui, status, lpcfg_workgroup(dce_call->conn->dce_ctx->lp_ctx), - r->in.account_name, + trust_account_in_db, NULL, sid); -- 2.11.0 From abf15c8e6ea5b3cb794afead301731543f2815ef Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Tue, 18 Jul 2017 08:57:03 +1200 Subject: [PATCH 8/9] s4-netlogon: Use log_escape to protect against un-validated strings Signed-off-by: Andrew Bartlett Reviewed-by: Alexander Bokovoy (cherry picked from commit 427a11b812d1872879658c998ef0328dd7c2a53a) --- source4/rpc_server/netlogon/dcerpc_netlogon.c | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/source4/rpc_server/netlogon/dcerpc_netlogon.c b/source4/rpc_server/netlogon/dcerpc_netlogon.c index 89ceabe21b3..2ed0840c640 100644 --- a/source4/rpc_server/netlogon/dcerpc_netlogon.c +++ b/source4/rpc_server/netlogon/dcerpc_netlogon.c @@ -271,7 +271,8 @@ static NTSTATUS dcesrv_netr_ServerAuthenticate3_helper( /* schannel must be used, but client did not offer it. */ DEBUG(0,("%s: schannel required but client failed " "to offer it. Client was %s\n", - __func__, r->in.account_name)); + __func__, + log_escape(mem_ctx, r->in.account_name))); return NT_STATUS_ACCESS_DENIED; } @@ -347,7 +348,8 @@ static NTSTATUS dcesrv_netr_ServerAuthenticate3_helper( if (NT_STATUS_EQUAL(nt_status, NT_STATUS_OBJECT_NAME_NOT_FOUND)) { DEBUG(2, ("Client asked for a trusted domain secure channel, " "but there's no tdo for [%s] => [%s] \n", - r->in.account_name, encoded_name)); + log_escape(mem_ctx, r->in.account_name), + encoded_name)); return NT_STATUS_NO_TRUST_SAM_ACCOUNT; } if (!NT_STATUS_IS_OK(nt_status)) { @@ -385,12 +387,14 @@ static NTSTATUS dcesrv_netr_ServerAuthenticate3_helper( if (num_records == 0) { DEBUG(3,("Couldn't find user [%s] in samdb.\n", - r->in.account_name)); + log_escape(mem_ctx, r->in.account_name))); return NT_STATUS_NO_TRUST_SAM_ACCOUNT; } if (num_records > 1) { - DEBUG(0,("Found %d records matching user [%s]\n", num_records, r->in.account_name)); + DEBUG(0,("Found %d records matching user [%s]\n", + num_records, + log_escape(mem_ctx, r->in.account_name))); return NT_STATUS_INTERNAL_DB_CORRUPTION; } @@ -406,7 +410,8 @@ static NTSTATUS dcesrv_netr_ServerAuthenticate3_helper( user_account_control = ldb_msg_find_attr_as_uint(msgs[0], "userAccountControl", 0); if (user_account_control & UF_ACCOUNTDISABLE) { - DEBUG(1, ("Account [%s] is disabled\n", r->in.account_name)); + DEBUG(1, ("Account [%s] is disabled\n", + log_escape(mem_ctx, r->in.account_name))); return NT_STATUS_NO_TRUST_SAM_ACCOUNT; } @@ -453,8 +458,8 @@ static NTSTATUS dcesrv_netr_ServerAuthenticate3_helper( if (!challenge_valid) { DEBUG(1, ("No challenge requested by client [%s/%s], " "cannot authenticate\n", - r->in.computer_name, - r->in.account_name)); + log_escape(mem_ctx, r->in.computer_name), + log_escape(mem_ctx, r->in.account_name))); return NT_STATUS_ACCESS_DENIED; } -- 2.11.0 From 3c2f8a6a960e9638c38804a51dd02d2ac258ff6e Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Tue, 18 Jul 2017 09:03:17 +1200 Subject: [PATCH 9/9] selftest: Use NETLOGON_NEG_STRONG_KEYS constant in AuthLogTestsNetLogonBadCreds Signed-off-by: Andrew Bartlett Reviewed-by: Alexander Bokovoy Autobuild-User(master): Andrew Bartlett Autobuild-Date(master): Tue Jul 25 03:21:19 CEST 2017 on sn-devel-144 (cherry picked from commit a420b1bdccbba72faf1108f7fae8b8202075db97) --- python/samba/tests/auth_log_netlogon_bad_creds.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/python/samba/tests/auth_log_netlogon_bad_creds.py b/python/samba/tests/auth_log_netlogon_bad_creds.py index fedd8a1d412..2bae02e21ba 100644 --- a/python/samba/tests/auth_log_netlogon_bad_creds.py +++ b/python/samba/tests/auth_log_netlogon_bad_creds.py @@ -36,7 +36,7 @@ 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 - +from samba.dcerpc.netlogon import NETLOGON_NEG_STRONG_KEYS class AuthLogTestsNetLogonBadCreds(samba.tests.auth_log_base.AuthLogTestBase): @@ -172,7 +172,7 @@ class AuthLogTestsNetLogonBadCreds(samba.tests.auth_log_base.AuthLogTestBase): SEC_CHAN_WKSTA, self.netbios_name, creds, - 0x00004000) + NETLOGON_NEG_STRONG_KEYS) except NTSTATUSError: pass self.waitForMessages(isLastExpectedMessage) -- 2.11.0