[PATCH] Test the netlogon challenge cache better and add helpful logs

Andrew Bartlett abartlet at samba.org
Mon Jun 26 08:02:13 UTC 2017


Debugging incorrect machine accounts on netlogon is really painful, as
there are essentially no useful logs on the AD DC for wrong machine
account passwords.  

This fixes it.  In the long run a proper hook into the audit logs are
due, but this is a good start. 

I started this due to a suggestion that our challenge cache wasn't
right and so I added tests to demonstrate better the capabilities and
limitations. 

Please review and push,

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 616bfcd5902ca4bdd4823af522e7623d4525de6c Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Mon, 26 Jun 2017 16:40:45 +1200
Subject: [PATCH 1/3] smbtorture: Add more tests around NETLOGON challenge
 reuse

The existing tests did not actually demonstrate what they
thought they did until the credential values were refreshed.

The new test showed this, because Samba fails it (windows passes)
due to the way we keep the last challenge on the connection.

Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
 selftest/knownfail.d/netlogon  |   4 +
 source4/torture/rpc/netlogon.c | 237 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 241 insertions(+)
 create mode 100644 selftest/knownfail.d/netlogon

diff --git a/selftest/knownfail.d/netlogon b/selftest/knownfail.d/netlogon
new file mode 100644
index 0000000..b51bf88
--- /dev/null
+++ b/selftest/knownfail.d/netlogon
@@ -0,0 +1,4 @@
+# This test passes against Windows 2008R2, but not Samba as we
+# keep a per-socket cache in addition to the name cache, which is
+# not invalidated if the name-based global cache is used.
+^samba4\.rpc\.netlogon.*\.netlogon\.ServerReqChallengeReuseGlobal3
\ No newline at end of file
diff --git a/source4/torture/rpc/netlogon.c b/source4/torture/rpc/netlogon.c
index 29025da..7e0aff3 100644
--- a/source4/torture/rpc/netlogon.c
+++ b/source4/torture/rpc/netlogon.c
@@ -1393,6 +1393,14 @@ static bool test_ServerReqChallengeReuseGlobal(struct torture_context *tctx,
 	torture_assert_ntstatus_ok(tctx, a.out.result, "ServerAuthenticate3 failed on b2");
 	torture_assert(tctx, netlogon_creds_client_check(creds, &credentials3), "Credential chaining failed");
 
+	/* We have to re-run this part */
+	creds = netlogon_creds_client_init(tctx, a.in.account_name,
+					   a.in.computer_name,
+					   a.in.secure_channel_type,
+					   &credentials1, &credentials2,
+					   &mach_password, &credentials3,
+					   flags);
+
 	torture_assert_ntstatus_ok(tctx, dcerpc_netr_ServerAuthenticate3_r(b3, tctx, &a),
 		"ServerAuthenticate3 failed on b3");
 	torture_assert_ntstatus_equal(tctx, a.out.result, NT_STATUS_ACCESS_DENIED,
@@ -1472,6 +1480,14 @@ static bool test_ServerReqChallengeReuseGlobal2(struct torture_context *tctx,
 	torture_assert_ntstatus_ok(tctx, a.out.result, "ServerAuthenticate3 failed on b");
 	torture_assert(tctx, netlogon_creds_client_check(creds, &credentials3), "Credential chaining failed");
 
+	/* We have to re-run this part */
+	creds = netlogon_creds_client_init(tctx, a.in.account_name,
+					   a.in.computer_name,
+					   a.in.secure_channel_type,
+					   &credentials1, &credentials2,
+					   &mach_password, &credentials3,
+					   flags);
+
 	torture_assert_ntstatus_ok(tctx, dcerpc_netr_ServerAuthenticate3_r(b2, tctx, &a),
 		"ServerAuthenticate3 failed on b2");
 	torture_assert_ntstatus_equal(tctx, a.out.result, NT_STATUS_ACCESS_DENIED,
@@ -1479,6 +1495,217 @@ static bool test_ServerReqChallengeReuseGlobal2(struct torture_context *tctx,
 	return true;
 }
 
+/*
+ * Test if use of the globally cached challenge will wipe out the 
+ * per-pipe challenge 
+ */
+static bool test_ServerReqChallengeReuseGlobal3(struct torture_context *tctx,
+						struct dcerpc_pipe *p1,
+						struct cli_credentials *machine_credentials)
+{
+	uint32_t flags = NETLOGON_NEG_AUTH2_FLAGS | NETLOGON_NEG_SUPPORTS_AES;
+	struct netr_ServerReqChallenge r;
+	struct netr_ServerAuthenticate3 a;
+	struct netr_Credential credentials1, credentials2, credentials3;
+	struct netlogon_creds_CredentialState *creds;
+	struct samr_Password mach_password;
+	uint32_t rid;
+	const char *machine_name;
+	const char *plain_pass;
+	struct dcerpc_binding_handle *b1 = p1->binding_handle;
+	struct dcerpc_pipe *p2 = NULL;
+	struct dcerpc_binding_handle *b2 = NULL;
+
+	machine_name = cli_credentials_get_workstation(machine_credentials);
+	plain_pass = cli_credentials_get_password(machine_credentials);
+
+	torture_comment(tctx, "Testing ServerReqChallenge on b1\n");
+
+	torture_assert_ntstatus_ok(tctx,
+		dcerpc_pipe_connect_b(tctx, &p2, p1->binding,
+				      &ndr_table_netlogon,
+				      machine_credentials,
+				      tctx->ev, tctx->lp_ctx),
+		"dcerpc_pipe_connect_b failed");
+	b2 = p2->binding_handle;
+
+	r.in.server_name = NULL;
+	r.in.computer_name = machine_name;
+	r.in.credentials = &credentials1;
+	r.out.return_credentials = &credentials2;
+
+	generate_random_buffer(credentials1.data, sizeof(credentials1.data));
+
+	torture_assert_ntstatus_ok(tctx, dcerpc_netr_ServerReqChallenge_r(b1, tctx, &r),
+		"ServerReqChallenge failed on b1");
+	torture_assert_ntstatus_ok(tctx, r.out.result, "ServerReqChallenge failed on b1");
+
+	E_md4hash(plain_pass, mach_password.hash);
+
+	a.in.server_name = NULL;
+	a.in.account_name = talloc_asprintf(tctx, "%s$", machine_name);
+	a.in.secure_channel_type = cli_credentials_get_secure_channel_type(machine_credentials);
+	a.in.computer_name = machine_name;
+	a.in.negotiate_flags = &flags;
+	a.in.credentials = &credentials3;
+	a.out.return_credentials = &credentials3;
+	a.out.negotiate_flags = &flags;
+	a.out.rid = &rid;
+
+	creds = netlogon_creds_client_init(tctx, a.in.account_name,
+					   a.in.computer_name,
+					   a.in.secure_channel_type,
+					   &credentials1, &credentials2,
+					   &mach_password, &credentials3,
+					   flags);
+
+	torture_assert(tctx, creds != NULL, "memory allocation");
+
+	torture_comment(tctx, "Testing ServerAuthenticate3 on b2\n");
+
+	torture_assert_ntstatus_ok(tctx, dcerpc_netr_ServerAuthenticate3_r(b2, tctx, &a),
+		"ServerAuthenticate3 failed on b2");
+	torture_assert_ntstatus_ok(tctx, a.out.result, "ServerAuthenticate3 failed on b");
+	torture_assert(tctx, netlogon_creds_client_check(creds, &credentials3), "Credential chaining failed");
+
+	/* We have to re-run this part */
+	creds = netlogon_creds_client_init(tctx, a.in.account_name,
+					   a.in.computer_name,
+					   a.in.secure_channel_type,
+					   &credentials1, &credentials2,
+					   &mach_password, &credentials3,
+					   flags);
+
+	torture_assert(tctx, creds != NULL, "memory allocation");
+
+	torture_assert_ntstatus_ok(tctx, dcerpc_netr_ServerAuthenticate3_r(b1, tctx, &a),
+		"ServerAuthenticate3 failed on b1");
+	torture_assert_ntstatus_equal(tctx, a.out.result, NT_STATUS_ACCESS_DENIED,
+				      "ServerAuthenticate3 should have failed on b1, due to credential reuse");
+	return true;
+}
+
+/*
+ * Test if more than one globally cached challenge works
+ */
+static bool test_ServerReqChallengeReuseGlobal4(struct torture_context *tctx,
+						struct dcerpc_pipe *p1,
+						struct cli_credentials *machine_credentials)
+{
+	uint32_t flags = NETLOGON_NEG_AUTH2_FLAGS | NETLOGON_NEG_SUPPORTS_AES;
+	struct netr_ServerReqChallenge r;
+	struct netr_ServerAuthenticate3 a;
+	struct netr_Credential credentials1, credentials1_random,
+		credentials2, credentials3, credentials_discard;
+	struct netlogon_creds_CredentialState *creds;
+	struct samr_Password mach_password;
+	uint32_t rid;
+	const char *machine_name;
+	const char *plain_pass;
+	struct dcerpc_binding_handle *b1 = p1->binding_handle;
+	struct dcerpc_pipe *p2 = NULL;
+	struct dcerpc_binding_handle *b2 = NULL;
+
+	machine_name = cli_credentials_get_workstation(machine_credentials);
+	plain_pass = cli_credentials_get_password(machine_credentials);
+
+	torture_comment(tctx, "Testing ServerReqChallenge on b1\n");
+
+	torture_assert_ntstatus_ok(tctx,
+		dcerpc_pipe_connect_b(tctx, &p2, p1->binding,
+				      &ndr_table_netlogon,
+				      machine_credentials,
+				      tctx->ev, tctx->lp_ctx),
+		"dcerpc_pipe_connect_b failed");
+	b2 = p2->binding_handle;
+
+	r.in.server_name = NULL;
+	r.in.computer_name = "CHALTEST1";
+	r.in.credentials = &credentials1_random;
+	r.out.return_credentials = &credentials_discard;
+
+	generate_random_buffer(credentials1_random.data,
+			       sizeof(credentials1_random.data));
+
+	torture_assert_ntstatus_ok(tctx, dcerpc_netr_ServerReqChallenge_r(b1, tctx, &r),
+		"ServerReqChallenge failed on b1");
+	torture_assert_ntstatus_ok(tctx, r.out.result, "ServerReqChallenge failed on b1");
+
+	/* Now ask for the actual client name */
+	r.in.server_name = NULL;
+	r.in.computer_name = machine_name;
+	r.in.credentials = &credentials1;
+	r.out.return_credentials = &credentials2;
+
+	generate_random_buffer(credentials1.data, sizeof(credentials1.data));
+
+	torture_assert_ntstatus_ok(tctx, dcerpc_netr_ServerReqChallenge_r(b1, tctx, &r),
+		"ServerReqChallenge failed on b1");
+	torture_assert_ntstatus_ok(tctx, r.out.result, "ServerReqChallenge failed on b1");
+
+	r.in.server_name = NULL;
+	r.in.computer_name = "CHALTEST2";
+	r.in.credentials = &credentials1_random;
+	r.out.return_credentials = &credentials_discard;
+
+	generate_random_buffer(credentials1_random.data,
+			       sizeof(credentials1_random.data));
+
+	r.in.server_name = NULL;
+	r.in.computer_name = "CHALTEST3";
+	r.in.credentials = &credentials1_random;
+	r.out.return_credentials = &credentials_discard;
+
+	generate_random_buffer(credentials1_random.data,
+			       sizeof(credentials1_random.data));
+
+	torture_assert_ntstatus_ok(tctx, dcerpc_netr_ServerReqChallenge_r(b1, tctx, &r),
+		"ServerReqChallenge failed on b1");
+	torture_assert_ntstatus_ok(tctx, r.out.result, "ServerReqChallenge failed on b1");
+
+	E_md4hash(plain_pass, mach_password.hash);
+
+	a.in.server_name = NULL;
+	a.in.account_name = talloc_asprintf(tctx, "%s$", machine_name);
+	a.in.secure_channel_type = cli_credentials_get_secure_channel_type(machine_credentials);
+	a.in.computer_name = machine_name;
+	a.in.negotiate_flags = &flags;
+	a.in.credentials = &credentials3;
+	a.out.return_credentials = &credentials3;
+	a.out.negotiate_flags = &flags;
+	a.out.rid = &rid;
+
+	creds = netlogon_creds_client_init(tctx, a.in.account_name,
+					   a.in.computer_name,
+					   a.in.secure_channel_type,
+					   &credentials1, &credentials2,
+					   &mach_password, &credentials3,
+					   flags);
+
+	torture_assert(tctx, creds != NULL, "memory allocation");
+
+	torture_comment(tctx, "Testing ServerAuthenticate3 on b2 (must use global credentials)\n");
+
+	torture_assert_ntstatus_ok(tctx, dcerpc_netr_ServerAuthenticate3_r(b2, tctx, &a),
+		"ServerAuthenticate3 failed on b2");
+	torture_assert_ntstatus_ok(tctx, a.out.result, "ServerAuthenticate3 failed on b2");
+	torture_assert(tctx, netlogon_creds_client_check(creds, &credentials3), "Credential chaining failed");
+
+	/* We have to re-run this part */
+	creds = netlogon_creds_client_init(tctx, a.in.account_name,
+					   a.in.computer_name,
+					   a.in.secure_channel_type,
+					   &credentials1, &credentials2,
+					   &mach_password, &credentials3,
+					   flags);
+
+	torture_assert_ntstatus_ok(tctx, dcerpc_netr_ServerAuthenticate3_r(b1, tctx, &a),
+		"ServerAuthenticate3 failed on b1");
+	torture_assert_ntstatus_equal(tctx, a.out.result, NT_STATUS_ACCESS_DENIED,
+				      "ServerAuthenticate3 should have failed on b1, due to credential reuse");
+	return true;
+}
+
 static bool test_ServerReqChallengeReuse(struct torture_context *tctx,
 					 struct dcerpc_pipe *p,
 					 struct cli_credentials *machine_credentials)
@@ -1538,6 +1765,14 @@ static bool test_ServerReqChallengeReuse(struct torture_context *tctx,
 	torture_assert_ntstatus_ok(tctx, a.out.result, "ServerAuthenticate3 failed");
 	torture_assert(tctx, netlogon_creds_client_check(creds, &credentials3), "Credential chaining failed");
 
+	/* We have to re-run this part */
+	creds = netlogon_creds_client_init(tctx, a.in.account_name,
+					   a.in.computer_name,
+					   a.in.secure_channel_type,
+					   &credentials1, &credentials2,
+					   &mach_password, &credentials3,
+					   flags);
+
 	torture_assert_ntstatus_ok(tctx, dcerpc_netr_ServerAuthenticate3_r(b, tctx, &a),
 		"ServerAuthenticate3 failed");
 	torture_assert_ntstatus_equal(tctx, a.out.result, NT_STATUS_ACCESS_DENIED,
@@ -4550,6 +4785,8 @@ struct torture_suite *torture_rpc_netlogon(TALLOC_CTX *mem_ctx)
 	torture_rpc_tcase_add_test_creds(tcase, "ServerReqChallengeGlobal", test_ServerReqChallengeGlobal);
 	torture_rpc_tcase_add_test_creds(tcase, "ServerReqChallengeReuseGlobal", test_ServerReqChallengeReuseGlobal);
 	torture_rpc_tcase_add_test_creds(tcase, "ServerReqChallengeReuseGlobal2", test_ServerReqChallengeReuseGlobal2);
+	torture_rpc_tcase_add_test_creds(tcase, "ServerReqChallengeReuseGlobal3", test_ServerReqChallengeReuseGlobal3);
+	torture_rpc_tcase_add_test_creds(tcase, "ServerReqChallengeReuseGlobal4", test_ServerReqChallengeReuseGlobal4);
 	torture_rpc_tcase_add_test_creds(tcase, "ServerReqChallengeReuse", test_ServerReqChallengeReuse);
 	torture_rpc_tcase_add_test_creds(tcase, "SetPassword", test_SetPassword);
 	torture_rpc_tcase_add_test_creds(tcase, "SetPassword2", test_SetPassword2);
-- 
2.9.4


From 0c87650bb064373af7cd04e63bcfbaad161328f9 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Mon, 26 Jun 2017 19:24:40 +1200
Subject: [PATCH 2/3] s4-netlogon: Provide logs for machine account success and
 failures

Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
 source4/rpc_server/netlogon/dcerpc_netlogon.c | 31 +++++++++++++++++++++++++--
 source4/rpc_server/wscript_build              |  3 ++-
 2 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/source4/rpc_server/netlogon/dcerpc_netlogon.c b/source4/rpc_server/netlogon/dcerpc_netlogon.c
index 3f70a3e..df0b730 100644
--- a/source4/rpc_server/netlogon/dcerpc_netlogon.c
+++ b/source4/rpc_server/netlogon/dcerpc_netlogon.c
@@ -43,6 +43,7 @@
 #include "librpc/gen_ndr/ndr_winbind_c.h"
 #include "lib/socket/netif.h"
 #include "rpc_server/common/sid_helper.h"
+#include "lib/util/util_str_escape.h"
 
 #define DCESRV_INTERFACE_NETLOGON_BIND(call, iface) \
        dcesrv_interface_netlogon_bind(call, iface)
@@ -467,8 +468,34 @@ static NTSTATUS dcesrv_netr_ServerAuthenticate3(struct dcesrv_call_state *dce_ca
 						   r->out.return_credentials,
 						   negotiate_flags);
 	}
-	if (creds == NULL) {
-		return NT_STATUS_ACCESS_DENIED;
+
+	{
+		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);
+		}
 	}
 
 	creds->sid = samdb_result_dom_sid(creds, msgs[0], "objectSid");
diff --git a/source4/rpc_server/wscript_build b/source4/rpc_server/wscript_build
index 966e07e..31a5696 100644
--- a/source4/rpc_server/wscript_build
+++ b/source4/rpc_server/wscript_build
@@ -105,7 +105,8 @@ bld.SAMBA_MODULE('dcerpc_netlogon',
 	source='netlogon/dcerpc_netlogon.c',
 	subsystem='dcerpc_server',
 	init_function='dcerpc_server_netlogon_init',
-	deps='DCERPC_COMMON RPC_NDR_IRPC COMMON_SCHANNEL ndr-standard auth4_sam samba-hostconfig DSDB_MODULE_HELPERS'
+	deps='''DCERPC_COMMON RPC_NDR_IRPC COMMON_SCHANNEL ndr-standard auth4_sam samba-hostconfig DSDB_MODULE_HELPERS
+        util_str_escape'''
 	)
 
 
-- 
2.9.4


From a02d5e0190e485f91a139508dfe90757867f1f99 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Mon, 26 Jun 2017 19:25:05 +1200
Subject: [PATCH 3/3] s4-netlogon: Escape user-supplied computer name in Bad
 credentials log line

Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
 source4/rpc_server/netlogon/dcerpc_netlogon.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/source4/rpc_server/netlogon/dcerpc_netlogon.c b/source4/rpc_server/netlogon/dcerpc_netlogon.c
index df0b730..b50b7a5 100644
--- a/source4/rpc_server/netlogon/dcerpc_netlogon.c
+++ b/source4/rpc_server/netlogon/dcerpc_netlogon.c
@@ -2011,9 +2011,9 @@ static NTSTATUS dcesrv_netr_LogonGetDomainInfo(struct dcesrv_call_state *dce_cal
 						frame);
 		DBG_ERR(("Bad credentials - "
 		         "computer[%s] remote[%s] local[%s]\n"),
-			 r->in.computer_name,
-			 remote,
-			 local);
+			log_escape(frame, r->in.computer_name),
+			remote,
+			local);
 		talloc_free(frame);
 	}
 	NT_STATUS_NOT_OK_RETURN(status);
-- 
2.9.4



More information about the samba-technical mailing list