Attachment 17985 Details for Bug 15418 – Patches for v4-18-test

[patch] Patches for v4-18-test

bfixes-tmp418.txt (text/plain), 12.30 KB, created by Stefan Metzmacher on 2023-07-17 07:47:25 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Stefan Metzmacher

Created: 2023-07-17 07:47:25 UTC

Size: 12.30 KB

patch

obsolete

>From e66efc77d8ecbe7cb42a7a8063b01906d7c49515 Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Sat, 15 Jul 2023 17:20:32 +0200
>Subject: [PATCH 1/4] netlogon.idl: add support for netr_LogonGetCapabilities
> response level 2
>
>We don't have any documentation about this yet, but tests against
>a Windows Server 2022 patched with KB5028166 revealed that
>the response for query_level=2 is exactly the same as
>for querey_level=1.
>
>Until we know the reason for query_level=2 we won't
>use it as client nor support it in the server, but
>we want ndrdump to work.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15418
>
>Signed-off-by: Stefan Metzmacher <metze@samba.org>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(cherry picked from commit 5f87888ed53320538cf773d64868390d8641a40e)
>---
> librpc/idl/netlogon.idl | 1 +
> 1 file changed, 1 insertion(+)
>
>diff --git a/librpc/idl/netlogon.idl b/librpc/idl/netlogon.idl
>index e563e114900b..c77151af26b1 100644
>--- a/librpc/idl/netlogon.idl
>+++ b/librpc/idl/netlogon.idl
>@@ -1241,6 +1241,7 @@ interface netlogon
> 	/* Function 0x15 */
> 	typedef [switch_type(uint32)] union {
> 		[case(1)] netr_NegotiateFlags server_capabilities;
>+		[case(2)] netr_NegotiateFlags server_capabilities;
> 	} netr_Capabilities;
> 
> 	NTSTATUS netr_LogonGetCapabilities(
>-- 
>2.34.1
>
>
>From bfa6a3e1d816ddcb508e7e0075386a8bf030561d Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Sat, 15 Jul 2023 17:25:05 +0200
>Subject: [PATCH 2/4] s4:torture/rpc: let rpc.schannel also check
> netr_LogonGetCapabilities with different levels
>
>The important change it that we expect DCERPC_NCA_S_FAULT_INVALID_TAG
>for unsupported query_levels, we allow it to work with servers
>with or without support for query_level=2.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15418
>
>Signed-off-by: Stefan Metzmacher <metze@samba.org>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(cherry picked from commit 404ce08e9088968311c714e756f5d58ce2cef715)
>---
> .../knownfail.d/netr_LogonGetCapabilities     |  3 +
> source4/torture/rpc/netlogon.c                | 77 ++++++++++++++++++-
> 2 files changed, 79 insertions(+), 1 deletion(-)
> create mode 100644 selftest/knownfail.d/netr_LogonGetCapabilities
>
>diff --git a/selftest/knownfail.d/netr_LogonGetCapabilities b/selftest/knownfail.d/netr_LogonGetCapabilities
>new file mode 100644
>index 000000000000..30aadf3bb9d5
>--- /dev/null
>+++ b/selftest/knownfail.d/netr_LogonGetCapabilities
>@@ -0,0 +1,3 @@
>+^samba3.rpc.schannel.*\.schannel\(nt4_dc
>+^samba3.rpc.schannel.*\.schannel\(ad_dc
>+^samba4.rpc.schannel.*\.schannel\(ad_dc
>diff --git a/source4/torture/rpc/netlogon.c b/source4/torture/rpc/netlogon.c
>index 1f068eb78265..a3d190f13dd8 100644
>--- a/source4/torture/rpc/netlogon.c
>+++ b/source4/torture/rpc/netlogon.c
>@@ -2056,8 +2056,47 @@ bool test_netlogon_capabilities(struct dcerpc_pipe *p, struct torture_context *t
> 	r.out.capabilities = &capabilities;
> 	r.out.return_authenticator = &return_auth;
> 
>-	torture_comment(tctx, "Testing LogonGetCapabilities\n");
>+	torture_comment(tctx, "Testing LogonGetCapabilities with query_level=0\n");
> 
>+	r.in.query_level = 0;
>+	ZERO_STRUCT(return_auth);
>+
>+	/*
>+	 * we need to operate on a temporary copy of creds
>+	 * because dcerpc_netr_LogonGetCapabilities with
>+	 * an unknown query level returns DCERPC_NCA_S_FAULT_INVALID_TAG
>+	 * => NT_STATUS_RPC_ENUM_VALUE_OUT_OF_RANGE
>+	 * without looking a the authenticator.
>+	 */
>+	tmp_creds = *creds;
>+	netlogon_creds_client_authenticator(&tmp_creds, &auth);
>+
>+	status = dcerpc_netr_LogonGetCapabilities_r(b, tctx, &r);
>+	torture_assert_ntstatus_equal(tctx, status, NT_STATUS_RPC_ENUM_VALUE_OUT_OF_RANGE,
>+				      "LogonGetCapabilities query_level=0 failed");
>+
>+	torture_comment(tctx, "Testing LogonGetCapabilities with query_level=3\n");
>+
>+	r.in.query_level = 3;
>+	ZERO_STRUCT(return_auth);
>+
>+	/*
>+	 * we need to operate on a temporary copy of creds
>+	 * because dcerpc_netr_LogonGetCapabilities with
>+	 * an unknown query level returns DCERPC_NCA_S_FAULT_INVALID_TAG
>+	 * => NT_STATUS_RPC_ENUM_VALUE_OUT_OF_RANGE
>+	 * without looking a the authenticator.
>+	 */
>+	tmp_creds = *creds;
>+	netlogon_creds_client_authenticator(&tmp_creds, &auth);
>+
>+	status = dcerpc_netr_LogonGetCapabilities_r(b, tctx, &r);
>+	torture_assert_ntstatus_equal(tctx, status, NT_STATUS_RPC_ENUM_VALUE_OUT_OF_RANGE,
>+				      "LogonGetCapabilities query_level=0 failed");
>+
>+	torture_comment(tctx, "Testing LogonGetCapabilities with query_level=1\n");
>+
>+	r.in.query_level = 1;
> 	ZERO_STRUCT(return_auth);
> 
> 	/*
>@@ -2077,6 +2116,42 @@ bool test_netlogon_capabilities(struct dcerpc_pipe *p, struct torture_context *t
> 
> 	*creds = tmp_creds;
> 
>+	torture_assert(tctx, netlogon_creds_client_check(creds,
>+							 &r.out.return_authenticator->cred),
>+		       "Credential chaining failed");
>+
>+	torture_assert_int_equal(tctx, creds->negotiate_flags,
>+				 capabilities.server_capabilities,
>+				 "negotiate flags");
>+
>+	torture_comment(tctx, "Testing LogonGetCapabilities with query_level=2\n");
>+
>+	r.in.query_level = 2;
>+	ZERO_STRUCT(return_auth);
>+
>+	/*
>+	 * we need to operate on a temporary copy of creds
>+	 * because dcerpc_netr_LogonGetCapabilities with
>+	 * an query level 2 may returns DCERPC_NCA_S_FAULT_INVALID_TAG
>+	 * => NT_STATUS_RPC_ENUM_VALUE_OUT_OF_RANGE
>+	 * without looking a the authenticator.
>+	 */
>+	tmp_creds = *creds;
>+	netlogon_creds_client_authenticator(&tmp_creds, &auth);
>+
>+	status = dcerpc_netr_LogonGetCapabilities_r(b, tctx, &r);
>+	if (NT_STATUS_EQUAL(status, NT_STATUS_RPC_ENUM_VALUE_OUT_OF_RANGE)) {
>+		/*
>+		 * an server without KB5028166 returns
>+		 * DCERPC_NCA_S_FAULT_INVALID_TAG =>
>+		 * NT_STATUS_RPC_ENUM_VALUE_OUT_OF_RANGE
>+		 */
>+		return true;
>+	}
>+	torture_assert_ntstatus_ok(tctx, status, "LogonGetCapabilities query_level=2 failed");
>+
>+	*creds = tmp_creds;
>+
> 	torture_assert(tctx, netlogon_creds_client_check(creds,
> 							 &r.out.return_authenticator->cred),
> 		       "Credential chaining failed");
>-- 
>2.34.1
>
>
>From 6f161300093b1b51fc9865155163cf7b3a42d802 Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Sat, 15 Jul 2023 16:11:48 +0200
>Subject: [PATCH 3/4] s4:rpc_server:netlogon: generate FAULT_INVALID_TAG for
> invalid netr_LogonGetCapabilities levels
>
>This is important as Windows clients with KB5028166 seem to
>call netr_LogonGetCapabilities with query_level=2 after
>a call with query_level=1.
>
>An unpatched Windows Server returns DCERPC_NCA_S_FAULT_INVALID_TAG
>for query_level values other than 1.
>While Samba tries to return NT_STATUS_NOT_SUPPORTED, but
>later fails to marshall the response, which results
>in DCERPC_FAULT_BAD_STUB_DATA instead.
>
>Because we don't have any documentation for level 2 yet,
>we just try to behave like an unpatched server and
>generate DCERPC_NCA_S_FAULT_INVALID_TAG instead of
>DCERPC_FAULT_BAD_STUB_DATA.
>Which allows patched Windows clients to keep working
>against a Samba DC.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15418
>
>Signed-off-by: Stefan Metzmacher <metze@samba.org>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(cherry picked from commit d5f1097b6220676d56ed5fc6707acf667b704518)
>---
> .../knownfail.d/netr_LogonGetCapabilities     |  2 --
> source4/rpc_server/netlogon/dcerpc_netlogon.c | 28 ++++++++++++++++---
> 2 files changed, 24 insertions(+), 6 deletions(-)
>
>diff --git a/selftest/knownfail.d/netr_LogonGetCapabilities b/selftest/knownfail.d/netr_LogonGetCapabilities
>index 30aadf3bb9d5..99c7ac711ede 100644
>--- a/selftest/knownfail.d/netr_LogonGetCapabilities
>+++ b/selftest/knownfail.d/netr_LogonGetCapabilities
>@@ -1,3 +1 @@
> ^samba3.rpc.schannel.*\.schannel\(nt4_dc
>-^samba3.rpc.schannel.*\.schannel\(ad_dc
>-^samba4.rpc.schannel.*\.schannel\(ad_dc
>diff --git a/source4/rpc_server/netlogon/dcerpc_netlogon.c b/source4/rpc_server/netlogon/dcerpc_netlogon.c
>index 314b469a718a..e203e04143d7 100644
>--- a/source4/rpc_server/netlogon/dcerpc_netlogon.c
>+++ b/source4/rpc_server/netlogon/dcerpc_netlogon.c
>@@ -2359,6 +2359,30 @@ static NTSTATUS dcesrv_netr_LogonGetCapabilities(struct dcesrv_call_state *dce_c
> 	struct netlogon_creds_CredentialState *creds;
> 	NTSTATUS status;
> 
>+	switch (r->in.query_level) {
>+	case 1:
>+		break;
>+	case 2:
>+		/*
>+		 * Until we know the details behind KB5028166
>+		 * just return DCERPC_NCA_S_FAULT_INVALID_TAG
>+		 * like an unpatched Windows Server.
>+		 */
>+		FALL_THROUGH;
>+	default:
>+		/*
>+		 * There would not be a way to marshall the
>+		 * the response. Which would mean our final
>+		 * ndr_push would fail an we would return
>+		 * an RPC-level fault with DCERPC_FAULT_BAD_STUB_DATA.
>+		 *
>+		 * But it's important to match a Windows server
>+		 * especially before KB5028166, see also our bug #15418
>+		 * Otherwise Windows client would stop talking to us.
>+		 */
>+		DCESRV_FAULT(DCERPC_NCA_S_FAULT_INVALID_TAG);
>+	}
>+
> 	status = dcesrv_netr_creds_server_step_check(dce_call,
> 						     mem_ctx,
> 						     r->in.computer_name,
>@@ -2370,10 +2394,6 @@ static NTSTATUS dcesrv_netr_LogonGetCapabilities(struct dcesrv_call_state *dce_c
> 	}
> 	NT_STATUS_NOT_OK_RETURN(status);
> 
>-	if (r->in.query_level != 1) {
>-		return NT_STATUS_NOT_SUPPORTED;
>-	}
>-
> 	r->out.capabilities->server_capabilities = creds->negotiate_flags;
> 
> 	return NT_STATUS_OK;
>-- 
>2.34.1
>
>
>From 4611b5bd5bb697a19a67f78624c498cbe952763b Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Sat, 15 Jul 2023 16:11:48 +0200
>Subject: [PATCH 4/4] s3:rpc_server:netlogon: generate FAULT_INVALID_TAG for
> invalid netr_LogonGetCapabilities levels
>
>This is important as Windows clients with KB5028166 seem to
>call netr_LogonGetCapabilities with query_level=2 after
>a call with query_level=1.
>
>An unpatched Windows Server returns DCERPC_NCA_S_FAULT_INVALID_TAG
>for query_level values other than 1.
>While Samba tries to return NT_STATUS_NOT_SUPPORTED, but
>later fails to marshall the response, which results
>in DCERPC_FAULT_BAD_STUB_DATA instead.
>
>Because we don't have any documentation for level 2 yet,
>we just try to behave like an unpatched server and
>generate DCERPC_NCA_S_FAULT_INVALID_TAG instead of
>DCERPC_FAULT_BAD_STUB_DATA.
>Which allows patched Windows clients to keep working
>against a Samba DC.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15418
>
>Signed-off-by: Stefan Metzmacher <metze@samba.org>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>
>Autobuild-User(master): Stefan Metzmacher <metze@samba.org>
>Autobuild-Date(master): Mon Jul 17 07:35:09 UTC 2023 on atb-devel-224
>
>(cherry picked from commit dfeabce44fbb78083fbbb2aa634fc4172cf83db9)
>---
> .../knownfail.d/netr_LogonGetCapabilities     |  1 -
> source3/rpc_server/netlogon/srv_netlog_nt.c   | 29 ++++++++++++++++---
> 2 files changed, 25 insertions(+), 5 deletions(-)
> delete mode 100644 selftest/knownfail.d/netr_LogonGetCapabilities
>
>diff --git a/selftest/knownfail.d/netr_LogonGetCapabilities b/selftest/knownfail.d/netr_LogonGetCapabilities
>deleted file mode 100644
>index 99c7ac711ede..000000000000
>--- a/selftest/knownfail.d/netr_LogonGetCapabilities
>+++ /dev/null
>@@ -1 +0,0 @@
>-^samba3.rpc.schannel.*\.schannel\(nt4_dc
>diff --git a/source3/rpc_server/netlogon/srv_netlog_nt.c b/source3/rpc_server/netlogon/srv_netlog_nt.c
>index 72c50638c729..d19e2be650fb 100644
>--- a/source3/rpc_server/netlogon/srv_netlog_nt.c
>+++ b/source3/rpc_server/netlogon/srv_netlog_nt.c
>@@ -2284,6 +2284,31 @@ NTSTATUS _netr_LogonGetCapabilities(struct pipes_struct *p,
> 	struct netlogon_creds_CredentialState *creds;
> 	NTSTATUS status;
> 
>+	switch (r->in.query_level) {
>+	case 1:
>+		break;
>+	case 2:
>+		/*
>+		 * Until we know the details behind KB5028166
>+		 * just return DCERPC_NCA_S_FAULT_INVALID_TAG
>+		 * like an unpatched Windows Server.
>+		 */
>+		FALL_THROUGH;
>+	default:
>+		/*
>+		 * There would not be a way to marshall the
>+		 * the response. Which would mean our final
>+		 * ndr_push would fail an we would return
>+		 * an RPC-level fault with DCERPC_FAULT_BAD_STUB_DATA.
>+		 *
>+		 * But it's important to match a Windows server
>+		 * especially before KB5028166, see also our bug #15418
>+		 * Otherwise Windows client would stop talking to us.
>+		 */
>+		p->fault_state = DCERPC_NCA_S_FAULT_INVALID_TAG;
>+		return NT_STATUS_NOT_SUPPORTED;
>+	}
>+
> 	become_root();
> 	status = dcesrv_netr_creds_server_step_check(p->dce_call,
> 						p->mem_ctx,
>@@ -2296,10 +2321,6 @@ NTSTATUS _netr_LogonGetCapabilities(struct pipes_struct *p,
> 		return status;
> 	}
> 
>-	if (r->in.query_level != 1) {
>-		return NT_STATUS_NOT_SUPPORTED;
>-	}
>-
> 	r->out.capabilities->server_capabilities = creds->negotiate_flags;
> 
> 	return NT_STATUS_OK;
>-- 
>2.34.1
>

Flags:

abartlet: review+
metze: review? (slow)

Actions: View

Attachments on bug 15418: 17979 | 17983 | 17985 | 17986 | 17987