[PATCH] Fix for Bug 12865 Samba 4.7 auth audit does not track machine account ServerAuthenticate3
Andrew Bartlett
abartlet at samba.org
Mon Jul 17 21:16:23 UTC 2017
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?
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
-------------- next part --------------
From 43eb279d6b06c620cfdbe2e7dac7ff0f3572405a Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Tue, 18 Jul 2017 08:46:08 +1200
Subject: [PATCH 1/3] 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 <abartlet at samba.org>
---
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 c140ee8..89ceabe 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.9.4
From eee2aea4cb7aa64b72cb94efa36923cba9ca702d Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Tue, 18 Jul 2017 08:57:03 +1200
Subject: [PATCH 2/3] s4-netlogon: Use log_escape to protect against
un-validated strings
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
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 89ceabe..2ed0840 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.9.4
From 72afc61d7d420d21541eb5fe4a280bb6be1f80b9 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Tue, 18 Jul 2017 09:03:17 +1200
Subject: [PATCH 3/3] selftest: Use NETLOGON_NEG_STRONG_KEYS constant in
AuthLogTestsNetLogonBadCreds
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
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 b9e2fbb..77e3d57 100644
--- a/python/samba/tests/auth_log_netlogon_bad_creds.py
+++ b/python/samba/tests/auth_log_netlogon_bad_creds.py
@@ -34,7 +34,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):
@@ -170,7 +170,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.9.4
-------------- next part --------------
From 1a4051706c6dc23170e0b13969a16970027633cd Mon Sep 17 00:00:00 2001
From: Gary Lockyer <gary at catalyst.net.nz>
Date: Mon, 10 Jul 2017 07:45:16 +1200
Subject: [PATCH 1/6] 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 <gary at catalyst.net.nz>
---
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 65800c9..6b032a8 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 e9ae464..aefd57e 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 d24986b..d865276 100644
--- a/python/samba/tests/auth_log_samlogon.py
+++ b/python/samba/tests/auth_log_samlogon.py
@@ -157,6 +157,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.9.4
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"
+ 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"
+ 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 0000000..e10a69e
--- /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 15037a2..465a15b 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.9.4
From 2c562786364d0a687f26e10690138bb96797d753 Mon Sep 17 00:00:00 2001
From: Gary Lockyer <gary at catalyst.net.nz>
Date: Mon, 10 Jul 2017 07:48:08 +1200
Subject: [PATCH 3/6] 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 <gary at catalyst.net.nz>
---
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 9dbf8f2..d4c6c44 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 e10a69e..0000000
--- 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 b50b7a5..c140ee8 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.9.4
From 43eb279d6b06c620cfdbe2e7dac7ff0f3572405a Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Tue, 18 Jul 2017 08:46:08 +1200
Subject: [PATCH 4/6] 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 <abartlet at samba.org>
---
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 c140ee8..89ceabe 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.9.4
From eee2aea4cb7aa64b72cb94efa36923cba9ca702d Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Tue, 18 Jul 2017 08:57:03 +1200
Subject: [PATCH 5/6] s4-netlogon: Use log_escape to protect against
un-validated strings
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
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 89ceabe..2ed0840 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.9.4
From 72afc61d7d420d21541eb5fe4a280bb6be1f80b9 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Tue, 18 Jul 2017 09:03:17 +1200
Subject: [PATCH 6/6] selftest: Use NETLOGON_NEG_STRONG_KEYS constant in
AuthLogTestsNetLogonBadCreds
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
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 b9e2fbb..77e3d57 100644
--- a/python/samba/tests/auth_log_netlogon_bad_creds.py
+++ b/python/samba/tests/auth_log_netlogon_bad_creds.py
@@ -34,7 +34,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):
@@ -170,7 +170,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.9.4
More information about the samba-technical
mailing list