From 0114e7e2d30fee4c42641d3d97d3ec253e301353 Mon Sep 17 00:00:00 2001
From: Noel Power <noel.power@suse.com>
Date: Mon, 27 Jan 2025 19:39:29 +0000
Subject: [PATCH 01/16] selftest: Add a new test to open/close printer handle
 (different users)

Add a torture test to call openPrinter as UserA and with the handle
returned call closePrinter when connecting as UserB. The call should
succeed [1] assuming the same spoolss instance that returned the handle
is also the one getting the closePrinter call. Currently check on handles
(in generic code checking dcerpc_handle) fails due to a comparison of
sid of authenticated samba user and sid associated with the handle.

Also add knownfail for new test

[1] - this is the behaviour observered running the test against
      win2k12, win2k16, win2k19 at least. And we see windows doing
      the following, opening the printer as the logged in user but
      occasionally closing the printer handle as the machine
      account. (but it looks like any authenticated user can
      close the handle)

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15792
Signed-off-by: Noel Power <noel.power@suse.com>
---
 selftest/knownfail            |   1 +
 source3/selftest/tests.py     |   1 +
 source4/torture/rpc/spoolss.c | 170 +++++++++++++++++++++++++++++++++-
 3 files changed, 171 insertions(+), 1 deletion(-)

diff --git a/selftest/knownfail b/selftest/knownfail
index 4e34effbbd1..5c5084aca2e 100644
--- a/selftest/knownfail
+++ b/selftest/knownfail
@@ -234,6 +234,7 @@
 ^samba3.rpc.spoolss.printserver.*.add_processor\(.*\)$
 ^samba3.rpc.spoolss.printserver.*.get_core_printer_drivers\(.*\)$
 ^samba3.rpc.spoolss.printserver.*.get_printer_driver_package_path\(.*\)$
+^samba3.rpc.spoolss.printerhandles.printerpolicyhandle.opencloseprinter\(.*\)$
 ^samba4.rpc.fsrvp	# fsrvp server only provided by smbd
 #
 # The following tests fail against ad_dc (aka s3fs) currently.
diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py
index a7d0531306e..03f2214a2e9 100755
--- a/source3/selftest/tests.py
+++ b/source3/selftest/tests.py
@@ -1070,6 +1070,7 @@ rpc = ["rpc.authcontext",
        "rpc.spoolss.win",
        "rpc.spoolss.notify",
        "rpc.spoolss.printer",
+       "rpc.spoolss.printerhandles",
        "rpc.spoolss.driver",
        "rpc.lsa",
        "rpc.lsa-getuser",
diff --git a/source4/torture/rpc/spoolss.c b/source4/torture/rpc/spoolss.c
index 0c71ed0073b..1ada13a7ce9 100644
--- a/source4/torture/rpc/spoolss.c
+++ b/source4/torture/rpc/spoolss.c
@@ -41,6 +41,7 @@
 #include "system/filesys.h"
 #include "torture/ndr/ndr.h"
 #include "torture/smb2/proto.h"
+#include "auth/credentials/credentials_krb5.h"
 
 #define TORTURE_WELLKNOWN_PRINTER	"torture_wkn_printer"
 #define TORTURE_PRINTER			"torture_printer"
@@ -111,6 +112,7 @@ struct torture_driver_context {
 
 struct torture_printer_context {
 	struct dcerpc_pipe *spoolss_pipe;
+	struct dcerpc_pipe *spoolss_pipe2;
 	struct spoolss_SetPrinterInfo2 info2;
 	struct torture_driver_context driver;
 	bool ex;
@@ -7947,6 +7949,44 @@ static bool test_openprinter_wrap(struct torture_context *tctx,
 	return test_openprinter(tctx, b, printername);
 }
 
+static bool test_simple_openprinter(struct torture_context *tctx,
+				    struct dcerpc_pipe *p,
+				    struct dcerpc_pipe *p2,
+				    const char *real_printername)
+{
+	struct policy_handle    hnd;
+	struct spoolss_DevmodeContainer devmode_ctr = {0};
+	WERROR          werror;
+	NTSTATUS status;
+
+	status = dcerpc_spoolss_OpenPrinter(p->binding_handle,
+					    tctx,
+					    real_printername,
+					    NULL,
+					    devmode_ctr,
+					    PRINTER_ALL_ACCESS,
+					    &hnd,
+					    &werror);
+
+	torture_assert_ntstatus_ok(tctx, status, "openprinter");
+	torture_assert_werr_equal(tctx, werror, WERR_SUCCESS, "openprinter failed");
+	/* close printer on second connection (authenticated by another user) */
+	torture_assert(tctx, test_ClosePrinter(tctx, p2->binding_handle, &hnd),
+			"failed to close printer on second connection");
+	return true;
+}
+
+static bool test_openclose(struct torture_context *tctx,
+				  void *private_data)
+{
+	struct torture_printer_context *t =
+		(struct torture_printer_context *)talloc_get_type_abort(private_data, struct torture_printer_context);
+	struct dcerpc_pipe *p = t->spoolss_pipe;
+	struct dcerpc_pipe *p2 = t->spoolss_pipe2;
+	const char *printername = t->info2.printername;
+	return test_simple_openprinter(tctx, p, p2, printername);
+}
+
 static bool test_csetprinter(struct torture_context *tctx,
 			     void *private_data)
 {
@@ -8764,6 +8804,86 @@ static bool torture_rpc_spoolss_printerdm_setup(struct torture_context *tctx, vo
 }
 #endif
 
+static bool torture_rpc_spoolss_second_user_setup_common(struct torture_context *tctx, void **data)
+{
+	struct torture_printer_context *t = NULL;
+	struct cli_credentials *cmdline_creds = NULL;
+	struct cli_credentials *user2_creds = NULL;
+	const struct dcerpc_binding *binding2 = NULL;
+	NTSTATUS status;
+
+	cmdline_creds = samba_cmdline_get_creds();
+	torture_assert(tctx,
+		       cmdline_creds != NULL,
+		       "samba_cmdline_get_creds failed");
+
+	user2_creds = cli_credentials_init_anon(tctx);
+	torture_assert(tctx,
+		       user2_creds != NULL,
+		       "cli_credentials_init_anon failed");
+
+	/*
+	 * if the environment has a DOMAIN_USER & DOMAIN_USER_PASSWORD
+	 * defined use these, otherwise create an anonymous connection
+	 */
+	if (getenv("DOMAIN_USER") && getenv("DOMAIN_USER_PASSWORD")) {
+		user2_creds = cli_credentials_shallow_copy(tctx,
+				cmdline_creds);
+		cli_credentials_set_username(user2_creds,
+					     getenv("DOMAIN_USER"),
+					     CRED_SPECIFIED);
+		cli_credentials_set_password(user2_creds,
+					     getenv("DOMAIN_USER_PASSWORD"),
+					     CRED_SPECIFIED);
+	}
+
+	*data = t = talloc_zero(tctx, struct torture_printer_context);
+
+	t->ex			= false;
+	t->wellknown		= false;
+	t->devmode		= NULL;
+	t->info2.printername = "torture_printer_dualusers";
+
+	torture_assert(tctx,
+			torture_rpc_spoolss_printer_setup_common(tctx, t),
+			"common setup failed\n");
+
+	/*
+	 * copy the binding to ensure we are using the same assoc_group_id
+	 * so we are communicating with the same spools instance that
+	 * created and returned the handle
+	 */
+	binding2 = t->spoolss_pipe->binding;
+
+	status = dcerpc_pipe_connect_b(tctx,
+				     &t->spoolss_pipe2,
+				     binding2,
+				     &ndr_table_spoolss,
+				     user2_creds, tctx->ev,
+				     tctx->lp_ctx);
+
+	torture_assert_ntstatus_ok(tctx,
+			status,
+			"Error creating second connection to server "
+			"as second user");
+
+	return true;
+}
+
+/*
+ * 2 users
+ * t->spoolss_pipe is the default user passed to smbtorture
+ * t->spoolss_pipe2 is DOMAIN_USER (or anonymous)
+ * t->handle has been opened with t->spoolss_pipe
+ *
+ */
+
+static bool torture_rpc_spoolss_second_user_setup_plain(struct torture_context *tctx, void **data)
+{
+	return torture_rpc_spoolss_second_user_setup_common(tctx, data);
+}
+
+
 static bool test_DeletePrinterDriverEx_exp(struct torture_context *tctx,
 					   struct dcerpc_binding_handle *b,
 					   const char *server,
@@ -8850,6 +8970,35 @@ static bool torture_rpc_spoolss_printer_teardown(struct torture_context *tctx, v
 	return ret;
 }
 
+static bool torture_rpc_spoolss_second_user_printer_teardown_common(struct torture_context *tctx, struct torture_printer_context *t, bool swap)
+{
+	bool ok;
+	if (swap) {
+		/*
+		 * attempt to cleanup by undoing the pipe swap
+		 * so we use the orig connection that opened printers etc.
+		 */
+		t->spoolss_pipe = t->spoolss_pipe2;
+		ok =  torture_rpc_spoolss_printer_teardown_common(tctx, t);
+	} else {
+		ok = torture_rpc_spoolss_printer_teardown_common(tctx, t);
+	}
+	return ok;
+}
+
+static bool torture_rpc_spoolss_second_user_printer_plain_teardown(struct torture_context *tctx, void *data)
+{
+	struct torture_printer_context *t = talloc_get_type(data, struct torture_printer_context);
+	bool ret;
+
+	ret = torture_rpc_spoolss_second_user_printer_teardown_common(tctx,
+								      t,
+								      false);
+	talloc_free(t);
+
+	return ret;
+}
+
 static bool test_print_test(struct torture_context *tctx,
 			    void *private_data)
 {
@@ -9774,6 +9923,24 @@ void torture_tcase_printer(struct torture_tcase *tcase)
 	torture_tcase_add_simple_test(tcase, "os_versions", test_printer_os_versions);
 }
 
+void torture_tcase_printerpolicy_handles(struct torture_tcase *tcase)
+{
+	torture_tcase_add_simple_test(tcase, "opencloseprinter", test_openclose);
+}
+
+struct torture_suite *torture_rpc_spoolss_printerhandle(TALLOC_CTX *mem_ctx)
+{
+	struct torture_suite *suite = torture_suite_create(mem_ctx, "printerhandles");
+	struct torture_tcase *tcase;
+	tcase = torture_suite_add_tcase(suite, "printerpolicyhandle");
+	torture_tcase_set_fixture(tcase,
+				  torture_rpc_spoolss_second_user_setup_plain,
+				  torture_rpc_spoolss_second_user_printer_plain_teardown);
+
+	torture_tcase_printerpolicy_handles(tcase);
+	return suite;
+}
+
 struct torture_suite *torture_rpc_spoolss_printer(TALLOC_CTX *mem_ctx)
 {
 	struct torture_suite *suite = torture_suite_create(mem_ctx, "printer");
@@ -9858,7 +10025,8 @@ struct torture_suite *torture_rpc_spoolss(TALLOC_CTX *mem_ctx)
 	torture_tcase_add_simple_test(tcase, "addpermachineconnection", test_addpermachineconnection);
 
 	torture_suite_add_suite(suite, torture_rpc_spoolss_printer(suite));
-
+	torture_suite_add_suite(suite,
+				torture_rpc_spoolss_printerhandle(suite));
 	return suite;
 }
 
-- 
2.43.0


From c59ef06e91a046f6e3d70814e1397a4f9ab5fed8 Mon Sep 17 00:00:00 2001
From: Noel Power <noel.power@suse.com>
Date: Tue, 25 Feb 2025 09:36:30 +0000
Subject: [PATCH 02/16] s3/rpc_server/spoolss: Capture printer_handle creator

Prepare for creating printer handles that by default can be used
by anyone (even those who didn't create them) prepare to retain
existing checks (except for where we explicitly prevent that)
by capturing the sid of the 'creator' of the printer handle

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15792
Signed-off-by: Noel Power <noel.power@suse.com>
---
 source3/rpc_server/spoolss/srv_spoolss_handle.h | 2 ++
 source3/rpc_server/spoolss/srv_spoolss_nt.c     | 7 ++++++-
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/source3/rpc_server/spoolss/srv_spoolss_handle.h b/source3/rpc_server/spoolss/srv_spoolss_handle.h
index e84037cae8a..080d7647c60 100644
--- a/source3/rpc_server/spoolss/srv_spoolss_handle.h
+++ b/source3/rpc_server/spoolss/srv_spoolss_handle.h
@@ -74,4 +74,6 @@ struct printer_handle {
 	/* TODO cache the printer info2 structure */
 	struct spoolss_PrinterInfo2 *info2;
 
+	/* capture the creator of the handle */
+	struct dom_sid sid;
 };
diff --git a/source3/rpc_server/spoolss/srv_spoolss_nt.c b/source3/rpc_server/spoolss/srv_spoolss_nt.c
index 883eebc6474..d8b6ea8ac77 100644
--- a/source3/rpc_server/spoolss/srv_spoolss_nt.c
+++ b/source3/rpc_server/spoolss/srv_spoolss_nt.c
@@ -719,6 +719,10 @@ static WERROR open_printer_hnd(struct pipes_struct *p,
 			       uint32_t access_granted)
 {
 	struct printer_handle *new_printer;
+	struct dcesrv_call_state *dce_call = p->dce_call;
+	struct auth_session_info *session_info =
+		dcesrv_call_session_info(dce_call);
+	struct security_token *sec_token = NULL;
 	WERROR result;
 
 	DEBUG(10,("open_printer_hnd: name [%s]\n", name));
@@ -755,7 +759,8 @@ static WERROR open_printer_hnd(struct pipes_struct *p,
 	}
 
 	new_printer->access_granted = access_granted;
-
+	sec_token = session_info->security_token;
+	sid_copy(&new_printer->sid, &sec_token->sids[PRIMARY_USER_SID_INDEX]);
 	DBG_INFO("%d printer handles active\n", (int)num_pipe_handles());
 
 	return WERR_OK;
-- 
2.43.0


From 53696fba498373f35ecf7cd6655b91d6f5a2b80e Mon Sep 17 00:00:00 2001
From: Noel Power <noel.power@suse.com>
Date: Tue, 28 Jan 2025 23:08:43 +0000
Subject: [PATCH 03/16] librpc/rpc: create less restrictive policy handle

It seems not all context_handles require a sid check (e.g. printer
handles) This change creates a new function that creates a context
handle that doesn't require the sid check so another user (other
than the user that made the rpc call that created and returned the
handle) can also use that handle

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15792
Signed-off-by: Noel Power <noel.power@suse.com>
---
 librpc/rpc/dcesrv_handles.c      |  4 +++-
 source3/rpc_server/rpc_handles.c | 19 +++++++++++++++++++
 source3/rpc_server/rpc_pipes.h   |  5 +++++
 3 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/librpc/rpc/dcesrv_handles.c b/librpc/rpc/dcesrv_handles.c
index b8719d8c804..69d287d4e04 100644
--- a/librpc/rpc/dcesrv_handles.c
+++ b/librpc/rpc/dcesrv_handles.c
@@ -125,7 +125,9 @@ struct dcesrv_handle *dcesrv_handle_lookup(struct dcesrv_call_state *call,
 		return NULL;
 	}
 
-	if (!dom_sid_equal(&h->sid, sid)) {
+	/* only check sid if it is set on handle */
+	if (!is_null_sid(&h->sid) &&
+	    !dom_sid_equal(&h->sid, sid)) {
 		struct dom_sid_buf buf1, buf2;
 		DBG_ERR("Attempt to use invalid sid %s - %s\n",
 			dom_sid_str_buf(&h->sid, &buf1),
diff --git a/source3/rpc_server/rpc_handles.c b/source3/rpc_server/rpc_handles.c
index 60ee11df457..1d5d6fb1ca6 100644
--- a/source3/rpc_server/rpc_handles.c
+++ b/source3/rpc_server/rpc_handles.c
@@ -169,6 +169,25 @@ void *_find_policy_by_hnd(struct pipes_struct *p,
 	return data;
 }
 
+bool create_no_sid_check_policy_hnd(struct pipes_struct *p,
+				    struct policy_handle *hnd,
+				    uint8_t handle_type,
+				    void *data_ptr)
+{
+	struct dcesrv_handle *rpc_hnd = NULL;
+	if (create_policy_hnd(p, hnd, handle_type, data_ptr)) {
+		rpc_hnd = find_policy_by_hnd_internal(p,
+						      hnd,
+						      handle_type,
+						      NULL);
+		if (rpc_hnd) {
+			rpc_hnd->sid = (struct dom_sid){};
+			return true;
+		}
+	}
+	return false;
+}
+
 /****************************************************************************
   Close a policy.
 ****************************************************************************/
diff --git a/source3/rpc_server/rpc_pipes.h b/source3/rpc_server/rpc_pipes.h
index ef45191ffa4..3c171b7020b 100644
--- a/source3/rpc_server/rpc_pipes.h
+++ b/source3/rpc_server/rpc_pipes.h
@@ -55,6 +55,11 @@ struct pipes_struct {
 bool check_open_pipes(void);
 size_t num_pipe_handles(void);
 
+bool create_no_sid_check_policy_hnd(struct pipes_struct *p,
+			struct policy_handle *hnd,
+			uint8_t handle_type,
+			void *data_ptr);
+
 bool create_policy_hnd(struct pipes_struct *p,
 			struct policy_handle *hnd,
 			uint8_t handle_type,
-- 
2.43.0


From 79af5c289f81e2146517a880c99edaf9e797d251 Mon Sep 17 00:00:00 2001
From: Noel Power <noel.power@suse.com>
Date: Wed, 29 Jan 2025 12:16:46 +0000
Subject: [PATCH 04/16] s4/rpc_server: spoolss: create printer handles that are
 less restricive

When creating a printer handle use new 'create_no_sid_check_policy_hnd'
function which creates a less restrictive handle that doesn't require
a sid check to ensure the user is the same user that created the handle

We will retain the sid check though but now in the spools code itself,
this will allow us to override the sid owner check of the printer_handle
as and when necessary to match windows behaviour.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15792
Signed-off-by: Noel Power <noel.power@suse.com>
---
 source3/rpc_server/spoolss/srv_spoolss_nt.c | 30 ++++++++++++++++++---
 1 file changed, 27 insertions(+), 3 deletions(-)

diff --git a/source3/rpc_server/spoolss/srv_spoolss_nt.c b/source3/rpc_server/spoolss/srv_spoolss_nt.c
index d8b6ea8ac77..9e142f091a3 100644
--- a/source3/rpc_server/spoolss/srv_spoolss_nt.c
+++ b/source3/rpc_server/spoolss/srv_spoolss_nt.c
@@ -312,10 +312,17 @@ static int printer_entry_destructor(struct printer_handle *Printer)
   find printer index by handle
 ****************************************************************************/
 
-static struct printer_handle *find_printer_index_by_hnd(struct pipes_struct *p,
-							struct policy_handle *hnd)
+static struct printer_handle *find_printer_index_by_hnd_check_sid(
+		struct pipes_struct *p,
+		struct policy_handle *hnd,
+		bool check_sid)
 {
 	struct printer_handle *find_printer = NULL;
+	struct dcesrv_call_state *dce_call = p->dce_call;
+	struct auth_session_info *session_info =
+		dcesrv_call_session_info(dce_call);
+	struct dom_sid *sid = NULL;
+
 	NTSTATUS status;
 
 	find_printer = find_policy_by_hnd(p,
@@ -328,9 +335,26 @@ static struct printer_handle *find_printer_index_by_hnd(struct pipes_struct *p,
 		return NULL;
 	}
 
+	sid = &session_info->security_token->sids[PRIMARY_USER_SID_INDEX];
+	if (check_sid &&
+	    !dom_sid_equal(&find_printer->sid,
+			   sid)) {
+		struct dom_sid_buf buf1 = {};
+		struct dom_sid_buf buf2 = {};
+		DBG_ERR("Attempt to use invalid sid %s - %s\n",
+			dom_sid_str_buf(&find_printer->sid, &buf1),
+			dom_sid_str_buf(sid, &buf2));
+		return NULL;
+	}
 	return find_printer;
 }
 
+static struct printer_handle *find_printer_index_by_hnd(struct pipes_struct *p,
+							struct policy_handle *hnd)
+{
+	return find_printer_index_by_hnd_check_sid(p, hnd, true);
+}
+
 /****************************************************************************
  Close printer index by handle.
 ****************************************************************************/
@@ -734,7 +758,7 @@ static WERROR open_printer_hnd(struct pipes_struct *p,
 	talloc_set_destructor(new_printer, printer_entry_destructor);
 
 	/* This also steals the printer_handle on the policy_handle */
-	if (!create_policy_hnd(p, hnd, 0, new_printer)) {
+	if (!create_no_sid_check_policy_hnd(p, hnd, 0, new_printer)) {
 		TALLOC_FREE(new_printer);
 		return WERR_INVALID_HANDLE;
 	}
-- 
2.43.0


From 46049404f4460f38a5d7c9098d257ad327e1fc30 Mon Sep 17 00:00:00 2001
From: Noel Power <noel.power@suse.com>
Date: Tue, 25 Feb 2025 10:53:58 +0000
Subject: [PATCH 05/16] s3/rpc_server/spools: Fix ClosePrinter and remove
 knownfail

Any user can close a printer/printer_handle so don't check sid
ownership of a printer handle when closing

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15792
Signed-off-by: Noel Power <noel.power@suse.com>
---
 selftest/knownfail                          |  1 -
 source3/rpc_server/spoolss/srv_spoolss_nt.c | 10 ++++++++--
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/selftest/knownfail b/selftest/knownfail
index 5c5084aca2e..4e34effbbd1 100644
--- a/selftest/knownfail
+++ b/selftest/knownfail
@@ -234,7 +234,6 @@
 ^samba3.rpc.spoolss.printserver.*.add_processor\(.*\)$
 ^samba3.rpc.spoolss.printserver.*.get_core_printer_drivers\(.*\)$
 ^samba3.rpc.spoolss.printserver.*.get_printer_driver_package_path\(.*\)$
-^samba3.rpc.spoolss.printerhandles.printerpolicyhandle.opencloseprinter\(.*\)$
 ^samba4.rpc.fsrvp	# fsrvp server only provided by smbd
 #
 # The following tests fail against ad_dc (aka s3fs) currently.
diff --git a/source3/rpc_server/spoolss/srv_spoolss_nt.c b/source3/rpc_server/spoolss/srv_spoolss_nt.c
index 9e142f091a3..548c64581c2 100644
--- a/source3/rpc_server/spoolss/srv_spoolss_nt.c
+++ b/source3/rpc_server/spoolss/srv_spoolss_nt.c
@@ -361,7 +361,10 @@ static struct printer_handle *find_printer_index_by_hnd(struct pipes_struct *p,
 
 static bool close_printer_handle(struct pipes_struct *p, struct policy_handle *hnd)
 {
-	struct printer_handle *Printer = find_printer_index_by_hnd(p, hnd);
+	struct printer_handle *Printer = find_printer_index_by_hnd_check_sid(
+								p,
+								hnd,
+								false);
 
 	if (!Printer) {
 		DEBUG(2,("close_printer_handle: Invalid handle (%s:%u:%u)\n",
@@ -1985,7 +1988,10 @@ WERROR _spoolss_OpenPrinterEx(struct pipes_struct *p,
 WERROR _spoolss_ClosePrinter(struct pipes_struct *p,
 			     struct spoolss_ClosePrinter *r)
 {
-	struct printer_handle *Printer = find_printer_index_by_hnd(p, r->in.handle);
+	struct printer_handle *Printer = find_printer_index_by_hnd_check_sid(
+								p,
+								r->in.handle,
+								false);
 
 	if (Printer && Printer->document_started) {
 		struct spoolss_EndDocPrinter e;
-- 
2.43.0


From e8f0801dc8d1dcc43abed5a5fe30940e5c7e3115 Mon Sep 17 00:00:00 2001
From: Noel Power <noel.power@suse.com>
Date: Sat, 15 Feb 2025 15:25:06 +0000
Subject: [PATCH 06/16] selftest: Add some more ClosePrinter combos to
 opencloseprinter test

As userB use print_handle opened by userA to attempt to close the printer
This time we test what happens when closing a second time (e.g. the
handle is invalid)

Although the test succeeds and we get a failure samba returns
NT_STATUS_RPC_SS_CONTEXT_MISMATCH instead of DCERPC_FAULT_BAD_STUB_DATA
which windows does. Adding knownfail for that

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15792
Signed-off-by: Noel Power <noel.power@suse.com>
---
 selftest/knownfail            |  1 +
 source4/torture/rpc/spoolss.c | 18 ++++++++++++++++++
 2 files changed, 19 insertions(+)

diff --git a/selftest/knownfail b/selftest/knownfail
index 4e34effbbd1..5c5084aca2e 100644
--- a/selftest/knownfail
+++ b/selftest/knownfail
@@ -234,6 +234,7 @@
 ^samba3.rpc.spoolss.printserver.*.add_processor\(.*\)$
 ^samba3.rpc.spoolss.printserver.*.get_core_printer_drivers\(.*\)$
 ^samba3.rpc.spoolss.printserver.*.get_printer_driver_package_path\(.*\)$
+^samba3.rpc.spoolss.printerhandles.printerpolicyhandle.opencloseprinter\(.*\)$
 ^samba4.rpc.fsrvp	# fsrvp server only provided by smbd
 #
 # The following tests fail against ad_dc (aka s3fs) currently.
diff --git a/source4/torture/rpc/spoolss.c b/source4/torture/rpc/spoolss.c
index 1ada13a7ce9..5a52d0ce340 100644
--- a/source4/torture/rpc/spoolss.c
+++ b/source4/torture/rpc/spoolss.c
@@ -7973,6 +7973,24 @@ static bool test_simple_openprinter(struct torture_context *tctx,
 	/* close printer on second connection (authenticated by another user) */
 	torture_assert(tctx, test_ClosePrinter(tctx, p2->binding_handle, &hnd),
 			"failed to close printer on second connection");
+
+	/* second close printer on second connection (authenticated by another user), handle should be now invalid. */
+	torture_assert_ntstatus_equal(tctx,
+			dcerpc_spoolss_ClosePrinter(p2->binding_handle,
+						    tctx,
+						    &hnd,
+						    &werror),
+			NT_STATUS_RPC_BAD_STUB_DATA, /* windows */
+			"closeprinter call failed");
+
+	/* close printer on orig client connection */
+	torture_assert_ntstatus_equal(tctx,
+			dcerpc_spoolss_ClosePrinter(p2->binding_handle,
+						    tctx,
+						    &hnd,
+						    &werror),
+			NT_STATUS_RPC_BAD_STUB_DATA, /* windows */
+			"closeprinter call failed");
 	return true;
 }
 
-- 
2.43.0


From 9c91e8df27f048f97ef57cfc5c093f2df1baafa9 Mon Sep 17 00:00:00 2001
From: Noel Power <noel.power@suse.com>
Date: Mon, 10 Feb 2025 16:12:10 +0000
Subject: [PATCH 07/16] selftest: Override fault_code for invalid printer
 handles

Windows returns DCERPC_FAULT_BAD_STUB_DATA for invalid printer handles,
samba returns NT_STATUS_RPC_SS_CONTEXT_MISMATCH.

Make the fault code the same and remove the knownfail

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15792
Signed-off-by: Noel Power <noel.power@suse.com>
---
 selftest/knownfail                          | 1 -
 source3/rpc_server/spoolss/srv_spoolss_nt.c | 6 ++++--
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/selftest/knownfail b/selftest/knownfail
index 5c5084aca2e..4e34effbbd1 100644
--- a/selftest/knownfail
+++ b/selftest/knownfail
@@ -234,7 +234,6 @@
 ^samba3.rpc.spoolss.printserver.*.add_processor\(.*\)$
 ^samba3.rpc.spoolss.printserver.*.get_core_printer_drivers\(.*\)$
 ^samba3.rpc.spoolss.printserver.*.get_printer_driver_package_path\(.*\)$
-^samba3.rpc.spoolss.printerhandles.printerpolicyhandle.opencloseprinter\(.*\)$
 ^samba4.rpc.fsrvp	# fsrvp server only provided by smbd
 #
 # The following tests fail against ad_dc (aka s3fs) currently.
diff --git a/source3/rpc_server/spoolss/srv_spoolss_nt.c b/source3/rpc_server/spoolss/srv_spoolss_nt.c
index 548c64581c2..850fba76623 100644
--- a/source3/rpc_server/spoolss/srv_spoolss_nt.c
+++ b/source3/rpc_server/spoolss/srv_spoolss_nt.c
@@ -2001,9 +2001,11 @@ WERROR _spoolss_ClosePrinter(struct pipes_struct *p,
 		_spoolss_EndDocPrinter(p, &e);
 	}
 
-	if (!close_printer_handle(p, r->in.handle))
+	if (!close_printer_handle(p, r->in.handle)) {
+		/* overwrite with what windows returns win2k12r & win2k16 */
+		p->fault_state = DCERPC_FAULT_BAD_STUB_DATA;
 		return WERR_INVALID_HANDLE;
-
+	}
 	/* clear the returned printer handle.  Observed behavior
 	   from Win2k server.  Don't think this really matters.
 	   Previous code just copied the value of the closed
-- 
2.43.0


From 1925961470df7d541b9b74d04b807cd939fd5001 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?G=C3=BCnther=20Deschner?= <gd@samba.org>
Date: Wed, 15 Nov 2023 19:07:32 +0100
Subject: [PATCH 08/16] s4-torture: add torture_assert_werr_equal_goto and
 torture_assert_werr_ok_goto macros

Guenther

Signed-off-by: Guenther Deschner <gd@samba.org>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
(cherry picked from commit 36745d63d687bdd310be0415c1a871a65a5ee724)
---
 lib/torture/torture.h | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/lib/torture/torture.h b/lib/torture/torture.h
index 3509911c8ca..2fcbac04678 100644
--- a/lib/torture/torture.h
+++ b/lib/torture/torture.h
@@ -307,6 +307,15 @@ void torture_result(struct torture_context *test,
 	} \
 	} while (0)
 
+#define torture_assert_werr_equal_goto(torture_ctx, got, expected, ret, label, cmt) \
+	do { WERROR __got = got, __expected = expected; \
+	if (!W_ERROR_EQUAL(__got, __expected)) { \
+		torture_result(torture_ctx, TORTURE_FAIL, __location__": "#got" was %s, expected %s: %s", win_errstr(__got), win_errstr(__expected), cmt); \
+		ret = false; \
+		goto label; \
+	} \
+	} while (0)
+
 #define torture_assert_ntstatus_equal(torture_ctx,got,expected,cmt) \
 	do { NTSTATUS __got = got, __expected = expected; \
 	if (!NT_STATUS_EQUAL(__got, __expected)) { \
@@ -774,6 +783,9 @@ static inline void torture_dump_data_str_cb(const char *buf, void *private_data)
 #define torture_assert_werr_ok(torture_ctx,expr,cmt) \
 		torture_assert_werr_equal(torture_ctx,expr,WERR_OK,cmt)
 
+#define torture_assert_werr_ok_goto(torture_ctx,expr,ret,label,cmt) \
+		torture_assert_werr_equal_goto(torture_ctx,expr,WERR_OK,ret,label,cmt)
+
 #define torture_assert_ndr_success(torture_ctx,expr,cmt) \
 		torture_assert_ndr_err_equal(torture_ctx,expr,NDR_ERR_SUCCESS,cmt)
 
-- 
2.43.0


From f652e292f4ae1db2e2bd40d0329660dfde68b4b4 Mon Sep 17 00:00:00 2001
From: Noel Power <noel.power@suse.com>
Date: Sat, 15 Feb 2025 16:30:33 +0000
Subject: [PATCH 09/16] selftest: Test printer job cntrl from userB with
 userA's printer handle

Some basic access tests for job control where userB is invoking the
pause/resume/purge cmds via SetPrinter using a handle obtained from
userA. These tests pass on windows, fail on samba.

Add knownfails

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15792
Signed-off-by: Noel Power <noel.power@suse.com>
---
 selftest/knownfail            |   4 +
 source4/torture/rpc/spoolss.c | 243 ++++++++++++++++++++++++++++++++++
 2 files changed, 247 insertions(+)

diff --git a/selftest/knownfail b/selftest/knownfail
index 4e34effbbd1..4ed62b5e1e5 100644
--- a/selftest/knownfail
+++ b/selftest/knownfail
@@ -234,6 +234,10 @@
 ^samba3.rpc.spoolss.printserver.*.add_processor\(.*\)$
 ^samba3.rpc.spoolss.printserver.*.get_core_printer_drivers\(.*\)$
 ^samba3.rpc.spoolss.printserver.*.get_printer_driver_package_path\(.*\)$
+^samba3.rpc.spoolss.printerhandles.printerpolicyhandle.jobcmds_pause\(.*\)$
+^samba3.rpc.spoolss.printerhandles.printerpolicyhandle.jobcmds_purge\(.*\)$
+^samba3.rpc.spoolss.printerhandles.printerpolicyhandle.jobcmds_resume\(.*\)$
+
 ^samba4.rpc.fsrvp	# fsrvp server only provided by smbd
 #
 # The following tests fail against ad_dc (aka s3fs) currently.
diff --git a/source4/torture/rpc/spoolss.c b/source4/torture/rpc/spoolss.c
index 5a52d0ce340..91fd240ac61 100644
--- a/source4/torture/rpc/spoolss.c
+++ b/source4/torture/rpc/spoolss.c
@@ -7949,6 +7949,246 @@ static bool test_openprinter_wrap(struct torture_context *tctx,
 	return test_openprinter(tctx, b, printername);
 }
 
+static bool test_simple_jobcmds(struct torture_context *tctx,
+				       struct dcerpc_pipe *p,
+				       struct dcerpc_pipe *p2,
+				       uint32_t access,
+				       NTSTATUS expected_status,
+				       WERROR expected_werr,
+				       uint32_t cmd,
+				       const char *real_printername)
+{
+	struct policy_handle hnd = {};
+	struct spoolss_DevmodeContainer devmode_ctr = {};
+	struct spoolss_SetPrinter r = {};
+	struct spoolss_SetPrinterInfoCtr info_ctr = {};
+	bool ret = true;
+	struct dcerpc_binding_handle *b = p->binding_handle;
+	struct dcerpc_binding_handle *b2 = p2->binding_handle;
+	WERROR          werror;
+	NTSTATUS status;
+	struct sec_desc_buf secdesc_ctr;
+
+	info_ctr.level = 0;
+	info_ctr.info.info0 = NULL;
+
+	ZERO_STRUCT(devmode_ctr);
+	ZERO_STRUCT(secdesc_ctr);
+
+	r.in.handle		= &hnd;
+	r.in.info_ctr		= &info_ctr;
+	r.in.devmode_ctr	= &devmode_ctr;
+	r.in.secdesc_ctr	= &secdesc_ctr;
+	r.in.command		= cmd;
+
+	status = dcerpc_spoolss_OpenPrinter(b,
+					    tctx,
+					    real_printername,
+					    NULL,
+					    devmode_ctr,
+					    access,
+					    &hnd,
+					    &werror);
+
+	torture_assert_ntstatus_ok(tctx, status, "openprinter");
+	torture_assert_werr_equal(tctx, werror, WERR_SUCCESS,
+				  "openprinter failed");
+
+	torture_comment(tctx,
+			"Calling SetPrinter with cmd %d and access 0x%x\n",
+			cmd, access);
+	/* call SetPrinter as user B (using handle obtained by user A) */
+	status = dcerpc_spoolss_SetPrinter_r(b2, tctx, &r);
+
+	torture_assert_ntstatus_equal_goto(tctx,
+			status,
+			expected_status,
+			ret,
+			out,
+			"SetPrinter failed");
+
+	torture_assert_werr_equal_goto(tctx,
+		r.out.result, expected_werr, ret, out,
+		"SetPrinter failed\n");
+out:
+	test_ClosePrinter(tctx, b2, &hnd);
+	return ret;
+}
+
+static bool test_pausecmd(struct torture_context *tctx,
+				  void *private_data)
+{
+	struct torture_printer_context *t =
+		(struct torture_printer_context *)talloc_get_type_abort(
+				private_data,
+				struct torture_printer_context);
+	struct dcerpc_pipe *p = t->spoolss_pipe;
+	struct dcerpc_pipe *p2 = t->spoolss_pipe2;
+	const char *printername = t->info2.printername;
+	int i;
+	struct {
+		NTSTATUS expected_ntstatus;
+		WERROR expected_werr;
+		uint32_t access;
+		uint32_t cmd;
+	} map [] = {
+		{
+			NT_STATUS_OK,
+			WERR_ACCESS_DENIED,
+			PRINTER_READ,
+			SPOOLSS_PRINTER_CONTROL_PAUSE,
+		},
+		{
+			NT_STATUS_OK,
+			WERR_ACCESS_DENIED,
+			PRINTER_WRITE,
+			SPOOLSS_PRINTER_CONTROL_PAUSE,
+		},
+		{
+			NT_STATUS_OK,
+			WERR_ACCESS_DENIED,
+			PRINTER_EXECUTE,
+			SPOOLSS_PRINTER_CONTROL_PAUSE,
+		},
+		{
+			NT_STATUS_OK,
+			WERR_OK,
+			PRINTER_ALL_ACCESS,
+			SPOOLSS_PRINTER_CONTROL_PAUSE,
+		},
+	};
+
+	for (i = 0; i < ARRAY_SIZE(map); i++) {
+		torture_assert(tctx,
+			test_simple_jobcmds(tctx,
+				p,
+				p2,
+				map[i].access,
+				map[i].expected_ntstatus,
+				map[i].expected_werr,
+				map[i].cmd,
+				printername),
+			"Job failed\n");
+	}
+	return true;
+}
+
+static bool test_resumecmd(struct torture_context *tctx,
+				  void *private_data)
+{
+	struct torture_printer_context *t =
+		(struct torture_printer_context *)talloc_get_type_abort(
+				private_data,
+				struct torture_printer_context);
+	struct dcerpc_pipe *p = t->spoolss_pipe;
+	struct dcerpc_pipe *p2 = t->spoolss_pipe2;
+	const char *printername = t->info2.printername;
+	int i;
+	struct {
+		NTSTATUS expected_ntstatus;
+		WERROR expected_werr;
+		uint32_t access;
+		uint32_t cmd;
+	} map [] = {
+		{
+			NT_STATUS_OK,
+			WERR_ACCESS_DENIED,
+			PRINTER_READ,
+			SPOOLSS_PRINTER_CONTROL_RESUME,
+		},
+		{
+			NT_STATUS_OK,
+			WERR_ACCESS_DENIED,
+			PRINTER_WRITE,
+			SPOOLSS_PRINTER_CONTROL_RESUME,
+		},
+		{
+			NT_STATUS_OK,
+			WERR_ACCESS_DENIED,
+			PRINTER_EXECUTE,
+			SPOOLSS_PRINTER_CONTROL_RESUME,
+		},
+		{
+			NT_STATUS_OK,
+			WERR_OK,
+			PRINTER_ALL_ACCESS,
+			SPOOLSS_PRINTER_CONTROL_RESUME,
+		},
+	};
+
+	for (i = 0; i < ARRAY_SIZE(map); i++) {
+		torture_assert(tctx,
+			test_simple_jobcmds(tctx,
+				p,
+				p2,
+				map[i].access,
+				map[i].expected_ntstatus,
+				map[i].expected_werr,
+				map[i].cmd,
+				printername),
+			"Job failed\n");
+	}
+	return true;
+}
+
+static bool test_purgecmd(struct torture_context *tctx,
+				  void *private_data)
+{
+	struct torture_printer_context *t =
+		(struct torture_printer_context *)talloc_get_type_abort(
+				private_data,
+				struct torture_printer_context);
+	struct dcerpc_pipe *p = t->spoolss_pipe;
+	struct dcerpc_pipe *p2 = t->spoolss_pipe2;
+	const char *printername = t->info2.printername;
+	int i;
+	struct {
+		NTSTATUS expected_ntstatus;
+		WERROR expected_werr;
+		uint32_t access;
+		uint32_t cmd;
+	} map [] = {
+		{
+			NT_STATUS_OK,
+			WERR_ACCESS_DENIED,
+			PRINTER_READ,
+			SPOOLSS_PRINTER_CONTROL_PURGE,
+		},
+		{
+			NT_STATUS_OK,
+			WERR_ACCESS_DENIED,
+			PRINTER_WRITE,
+			SPOOLSS_PRINTER_CONTROL_PURGE,
+		},
+		{
+			NT_STATUS_OK,
+			WERR_ACCESS_DENIED,
+			PRINTER_EXECUTE,
+			SPOOLSS_PRINTER_CONTROL_PURGE,
+		},
+		{
+			NT_STATUS_OK,
+			WERR_OK,
+			PRINTER_ALL_ACCESS,
+			SPOOLSS_PRINTER_CONTROL_PURGE,
+		},
+	};
+
+	for (i = 0; i < ARRAY_SIZE(map); i++) {
+		torture_assert(tctx,
+			test_simple_jobcmds(tctx,
+				p,
+				p2,
+				map[i].access,
+				map[i].expected_ntstatus,
+				map[i].expected_werr,
+				map[i].cmd,
+				printername),
+			"Job failed\n");
+	}
+	return true;
+}
+
 static bool test_simple_openprinter(struct torture_context *tctx,
 				    struct dcerpc_pipe *p,
 				    struct dcerpc_pipe *p2,
@@ -9943,6 +10183,9 @@ void torture_tcase_printer(struct torture_tcase *tcase)
 
 void torture_tcase_printerpolicy_handles(struct torture_tcase *tcase)
 {
+	torture_tcase_add_simple_test(tcase, "jobcmds_pause", test_pausecmd);
+	torture_tcase_add_simple_test(tcase, "jobcmds_purge", test_purgecmd);
+	torture_tcase_add_simple_test(tcase, "jobcmds_resume", test_resumecmd);
 	torture_tcase_add_simple_test(tcase, "opencloseprinter", test_openclose);
 }
 
-- 
2.43.0


From 41d5ed14abaec85b8e47d5b083ec68a48029a55c Mon Sep 17 00:00:00 2001
From: Noel Power <noel.power@suse.com>
Date: Thu, 13 Feb 2025 13:21:09 +0000
Subject: [PATCH 10/16] s3/rpc_server/spoolss: Capture printer/server
 access_mask flags

Add a new member to PrinterHandle to capture the (mostly) unmodified
printer mask. The current code modifes the mask the printer_handle
has been opened with so basically it has 2 values

for printer handle
  PRINTER_ACCESS_ADMINISTER or PRINTER_ACCESS_USE
for printerserver handle
  SERVER_ACCESS_ADMINISTER or SERVER_ACCESS_ENUMERATE

There is alot of code that seems to depend on these conversions,
most likely the new mask should be used however that would be quite
risky. At least with the 2 values available use of the 'old' mask
could be removed piecemeal by converting code to use the 'new'
value.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15792
Signed-off-by: Noel Power <noel.power@suse.com>
---
 source3/rpc_server/spoolss/srv_spoolss_handle.h | 2 ++
 source3/rpc_server/spoolss/srv_spoolss_nt.c     | 5 ++++-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/source3/rpc_server/spoolss/srv_spoolss_handle.h b/source3/rpc_server/spoolss/srv_spoolss_handle.h
index 080d7647c60..d897be812a3 100644
--- a/source3/rpc_server/spoolss/srv_spoolss_handle.h
+++ b/source3/rpc_server/spoolss/srv_spoolss_handle.h
@@ -50,6 +50,8 @@ struct printer_handle {
 	const char *servername;
 	fstring sharename;
 	uint32_t access_granted;
+	/* access mask printer/server with unrelated bits masked off */
+	uint32_t specific_access_mask;
 	struct {
 		uint32_t flags;
 		uint32_t options;
diff --git a/source3/rpc_server/spoolss/srv_spoolss_nt.c b/source3/rpc_server/spoolss/srv_spoolss_nt.c
index 850fba76623..062b4c309bb 100644
--- a/source3/rpc_server/spoolss/srv_spoolss_nt.c
+++ b/source3/rpc_server/spoolss/srv_spoolss_nt.c
@@ -1820,7 +1820,7 @@ WERROR _spoolss_OpenPrinterEx(struct pipes_struct *p,
 		   servers (i.e printer and job specific bits) */
 
 		r->in.access_mask &= SEC_MASK_SPECIFIC;
-
+		Printer->specific_access_mask = r->in.access_mask;
 		if (r->in.access_mask &
 		    ~(SERVER_ACCESS_ADMINISTER | SERVER_ACCESS_ENUMERATE)) {
 			DEBUG(3, ("access DENIED for non-printserver bits\n"));
@@ -1882,6 +1882,9 @@ WERROR _spoolss_OpenPrinterEx(struct pipes_struct *p,
 			r->in.access_mask = PRINTER_ACCESS_ADMINISTER;
 		}
 
+		Printer->specific_access_mask = r->in.access_mask;
+		Printer->specific_access_mask &=
+			(SEC_MASK_STANDARD | SEC_MASK_SPECIFIC);
 		se_map_standard(&r->in.access_mask, &printer_std_mapping);
 
 		/* map an empty access mask to the minimum access mask */
-- 
2.43.0


From 49d91edd650ea34213a11695ad248f6104d57a62 Mon Sep 17 00:00:00 2001
From: Noel Power <noel.power@suse.com>
Date: Sun, 16 Feb 2025 09:22:17 +0000
Subject: [PATCH 11/16] selftest: Add test with second user calling
 DeletePrinter

Observed with windows that the printer_handle obtained by userA
from AddPrinter results in ACCESS_DENIED errors when userB
attempts to delete the printer using that handle. Seems like
DeletePrinter will only succeed in the same session with AddPrinter
obtained printer_handle

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15792
Signed-off-by: Noel Power <noel.power@suse.com>
---
 selftest/knownfail            |   1 +
 source4/torture/rpc/spoolss.c | 103 ++++++++++++++++++++++++++++++++++
 2 files changed, 104 insertions(+)

diff --git a/selftest/knownfail b/selftest/knownfail
index 4ed62b5e1e5..a44fcc88998 100644
--- a/selftest/knownfail
+++ b/selftest/knownfail
@@ -237,6 +237,7 @@
 ^samba3.rpc.spoolss.printerhandles.printerpolicyhandle.jobcmds_pause\(.*\)$
 ^samba3.rpc.spoolss.printerhandles.printerpolicyhandle.jobcmds_purge\(.*\)$
 ^samba3.rpc.spoolss.printerhandles.printerpolicyhandle.jobcmds_resume\(.*\)$
+^samba3.rpc.spoolss.printerhandles.printerpolicyhandle.seconduseradddeleteprinter\(.*\)$
 
 ^samba4.rpc.fsrvp	# fsrvp server only provided by smbd
 #
diff --git a/source4/torture/rpc/spoolss.c b/source4/torture/rpc/spoolss.c
index 91fd240ac61..4b6b40d4d7f 100644
--- a/source4/torture/rpc/spoolss.c
+++ b/source4/torture/rpc/spoolss.c
@@ -8245,6 +8245,107 @@ static bool test_openclose(struct torture_context *tctx,
 	return test_simple_openprinter(tctx, p, p2, printername);
 }
 
+static bool test_simple_seconduserdeleteprinter(struct torture_context *tctx,
+					struct torture_printer_context *t,
+					struct dcerpc_pipe *p,
+					struct dcerpc_pipe *p2,
+					const char *real_printername)
+{
+	struct policy_handle hnd = {};
+	struct dcerpc_binding_handle *b = NULL;
+	bool ok = true;
+	WERROR werror;
+	NTSTATUS status;
+
+	/* Add printer as user A (handle returned created by user A)*/
+	torture_assert(tctx,
+		test_AddPrinter_normal(tctx,
+				       p,
+				       &hnd,
+				       real_printername,
+				       t->info2.drivername,
+				       t->info2.portname,
+				       NULL,
+				       false),
+		"failed to add printer");
+
+	b = p->binding_handle;
+
+	/* ensure we can delete printer as user A */
+	torture_assert(tctx,
+			test_DeletePrinter(tctx, b, &hnd),
+			"failed to delete printer as user A");
+
+	/*
+	 * use new printer name as same printername causes some
+	 * name clash problems in the testenv probably some race
+	 * condition. No such problems running against windows
+	 */
+
+	real_printername = talloc_asprintf(tctx, "%s_2", real_printername);
+	/* Add printer again as user A*/
+	torture_assert(tctx,
+		test_AddPrinter_normal(tctx,
+				       p,
+				       &hnd,
+				       real_printername,
+				       t->info2.drivername,
+				       t->info2.portname,
+				       NULL,
+				       false),
+		"failed to add printer after delete");
+
+	/* user B handle */
+	b = p2->binding_handle;
+
+	/*
+	 * test if we can delete printer as user B
+	 * Should fail in windows and samba
+	 * Windows: NT_STATUS_ACCESS_DENIED (no matter who user B is even
+	 *          if the same as user A)
+	 * samba:  currently succeeds
+	 */
+	status = dcerpc_spoolss_DeletePrinter(b, tctx, &hnd, &werror);
+	if (!NT_STATUS_EQUAL(status, NT_STATUS_ACCESS_DENIED)) {
+		/*
+		 * samba can't pass back NT_STATUS_XYZ as rpc
+		 * methods only can pass back WERROR.
+		 * At least expect an appropriate werror if
+		 * working
+		 */
+		torture_assert_werr_equal_goto(tctx,
+				werror,
+				WERR_ACCESS_DENIED,
+				ok,
+				fail,
+				"Unexpected werr result");
+	}
+
+	/* delete above failed, we need to clean up and remove printer */
+	b = p->binding_handle;
+
+	torture_assert(tctx,
+			test_DeletePrinter(tctx, b, &hnd),
+			"failed to cleanup and delete printer as user A");
+fail:   /* we actually deleted the printer, nothing to do */
+	return ok;
+}
+
+static bool test_seconduserdeleteprinter(struct torture_context *tctx,
+				  void *private_data)
+{
+	struct torture_printer_context *t =
+		(struct torture_printer_context *)talloc_get_type_abort(private_data, struct torture_printer_context);
+	struct dcerpc_pipe *p = t->spoolss_pipe;
+	struct dcerpc_pipe *p2 = t->spoolss_pipe2;
+	const char *printername = talloc_asprintf(tctx,
+						  "%s_2",
+						  TORTURE_PRINTER);
+
+	return test_simple_seconduserdeleteprinter(tctx, t, p, p2, printername);
+}
+
+
 static bool test_csetprinter(struct torture_context *tctx,
 			     void *private_data)
 {
@@ -10187,6 +10288,8 @@ void torture_tcase_printerpolicy_handles(struct torture_tcase *tcase)
 	torture_tcase_add_simple_test(tcase, "jobcmds_purge", test_purgecmd);
 	torture_tcase_add_simple_test(tcase, "jobcmds_resume", test_resumecmd);
 	torture_tcase_add_simple_test(tcase, "opencloseprinter", test_openclose);
+	torture_tcase_add_simple_test(tcase, "seconduseradddeleteprinter",
+				      test_seconduserdeleteprinter);
 }
 
 struct torture_suite *torture_rpc_spoolss_printerhandle(TALLOC_CTX *mem_ctx)
-- 
2.43.0


From 92c09c6e637fb1d8e49f1b717c01e7fdc0374bcc Mon Sep 17 00:00:00 2001
From: Noel Power <noel.power@suse.com>
Date: Sun, 16 Feb 2025 11:24:41 +0000
Subject: [PATCH 12/16] selftest: tests for DeletePrinter using handle obtained
 by other user

In general on windows, userB calling DeletePrinter using
printer_handle obtained by userA is allowed, the printer_handle
accessmask is what is used to determine success/failure of
the DeletePrinter call. Add a test for various combinations of accessmask.

add knownfails.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15792
Signed-off-by: Noel Power <noel.power@suse.com>
---
 selftest/knownfail            |   4 +
 source4/torture/rpc/spoolss.c | 208 ++++++++++++++++++++++++++++++++++
 2 files changed, 212 insertions(+)

diff --git a/selftest/knownfail b/selftest/knownfail
index a44fcc88998..39788d76bc7 100644
--- a/selftest/knownfail
+++ b/selftest/knownfail
@@ -238,6 +238,10 @@
 ^samba3.rpc.spoolss.printerhandles.printerpolicyhandle.jobcmds_purge\(.*\)$
 ^samba3.rpc.spoolss.printerhandles.printerpolicyhandle.jobcmds_resume\(.*\)$
 ^samba3.rpc.spoolss.printerhandles.printerpolicyhandle.seconduseradddeleteprinter\(.*\)$
+^samba3.rpc.spoolss.printerhandles.printerpolicyhandle.seconduseropendeleteprinter_read\(.*\)$
+^samba3.rpc.spoolss.printerhandles.printerpolicyhandle.seconduseropendeleteprinter_write\(.*\)$
+^samba3.rpc.spoolss.printerhandles.printerpolicyhandle.seconduseraddopendeleteprinter_exec\(.*\)$
+^samba3.rpc.spoolss.printerhandles.printerpolicyhandle.seconduseropendeleteprinter\(.*\)$
 
 ^samba4.rpc.fsrvp	# fsrvp server only provided by smbd
 #
diff --git a/source4/torture/rpc/spoolss.c b/source4/torture/rpc/spoolss.c
index 4b6b40d4d7f..61f9d2addcc 100644
--- a/source4/torture/rpc/spoolss.c
+++ b/source4/torture/rpc/spoolss.c
@@ -8245,6 +8245,114 @@ static bool test_openclose(struct torture_context *tctx,
 	return test_simple_openprinter(tctx, p, p2, printername);
 }
 
+static bool test_simple_seconduseropendeleteprinter(struct torture_context *tctx,
+					struct torture_printer_context *t,
+					struct dcerpc_pipe *p,
+					struct dcerpc_pipe *p2,
+					const char *real_printername,
+					uint32_t access,
+					WERROR expected_werr,
+					NTSTATUS expected_ntstatus
+					)
+{
+	struct policy_handle hnd = {};
+	struct dcerpc_binding_handle *b = NULL;
+	bool ok = true;
+	struct spoolss_DevmodeContainer devmode_ctr = {};
+	WERROR werror;
+
+	/* Add printer as user A (handle returned created by user A)*/
+	torture_assert(tctx,
+		test_AddPrinter_normal(tctx,
+				       p,
+				       &hnd,
+				       real_printername,
+				       t->info2.drivername,
+				       t->info2.portname,
+				       NULL,
+				       false),
+		"failed to add printer\n");
+
+	b = p->binding_handle;
+
+	/* close printer, open as user A */
+	torture_assert(tctx,
+		test_ClosePrinter(tctx, b, &hnd),
+		"failed to close printer\n");
+
+	/* open again */
+	torture_assert_ntstatus_ok(tctx,
+			dcerpc_spoolss_OpenPrinter(
+				t->spoolss_pipe->binding_handle,
+				tctx,
+				real_printername,
+				NULL,
+				devmode_ctr,
+				access,
+				&hnd,
+				&werror),
+			"failed to open printer\n");
+
+	torture_assert_werr_ok(tctx,
+		werror,
+		"OpenPrinter failed");
+
+	b = p2->binding_handle;
+
+	/*
+	 * test if we can delete printer as user B
+	 */
+
+	torture_assert_ntstatus_equal_goto(tctx,
+			dcerpc_spoolss_DeletePrinter(b, tctx, &hnd, &werror),
+			expected_ntstatus,
+			ok,
+			cleanup,
+			"unexpected status when attempting to delete printer as user B\n");
+
+	torture_assert_werr_equal_goto(tctx,
+		werror, expected_werr, ok, cleanup,
+		"DeletePrinter failed\n");
+
+	if (!W_ERROR_EQUAL(expected_werr, WERR_OK) ||
+	    !NT_STATUS_IS_OK(expected_ntstatus)) {
+		/*
+		 * we expected an error state but that means we
+		 * need still need to clean up the added
+		 * printer
+		 */
+		goto cleanup;
+	}
+	/* delete succeeded return */
+	return ok;
+cleanup:
+	/* delete above failed, we need to clean up and remove printer */
+	b = p->binding_handle;
+
+	/* close the printer again */
+	torture_assert(tctx,
+		test_ClosePrinter(tctx, b, &hnd),
+		"failed to close printer\n");
+
+	/* open again as user A (with full access) */
+	torture_assert_ntstatus_ok(tctx,
+			dcerpc_spoolss_OpenPrinter(
+				b,
+				tctx,
+				real_printername,
+				NULL,
+				devmode_ctr,
+				PRINTER_ALL_ACCESS,
+				&hnd,
+				&werror),
+			"failed to open printer\n");
+
+	torture_assert(tctx,
+			test_DeletePrinter(tctx, b, &hnd),
+			"failed to cleanup and delete printer as user A");
+	return ok;
+}
+
 static bool test_simple_seconduserdeleteprinter(struct torture_context *tctx,
 					struct torture_printer_context *t,
 					struct dcerpc_pipe *p,
@@ -8346,6 +8454,94 @@ static bool test_seconduserdeleteprinter(struct torture_context *tctx,
 }
 
 
+static bool test_seconduseraddopendeleteprinter(struct torture_context *tctx,
+				  void *private_data)
+{
+	struct torture_printer_context *t =
+		(struct torture_printer_context *)talloc_get_type_abort(private_data, struct torture_printer_context);
+	struct dcerpc_pipe *p = t->spoolss_pipe;
+	struct dcerpc_pipe *p2 = t->spoolss_pipe2;
+	const char *printername = "deleteprinter";
+	WERROR expected_werr = WERR_OK;
+	NTSTATUS expected_ntstatus = NT_STATUS_OK;
+	uint32_t access = PRINTER_ALL_ACCESS;
+	return test_simple_seconduseropendeleteprinter(
+						tctx,
+						t,
+						p,
+						p2,
+						printername,
+						access,
+						expected_werr,
+						expected_ntstatus);
+}
+
+static bool test_seconduseraddopendeleteprinter_read(struct torture_context *tctx,
+				  void *private_data)
+{
+	struct torture_printer_context *t =
+		(struct torture_printer_context *)talloc_get_type_abort(private_data, struct torture_printer_context);
+	struct dcerpc_pipe *p = t->spoolss_pipe;
+	struct dcerpc_pipe *p2 = t->spoolss_pipe2;
+	const char *printername = "deleteprinter1";
+	WERROR expected_werr = WERR_ACCESS_DENIED;
+	NTSTATUS expected_ntstatus = NT_STATUS_OK;
+	uint32_t access = PRINTER_READ;
+	return test_simple_seconduseropendeleteprinter(
+						tctx,
+						t,
+						p,
+						p2,
+						printername,
+						access,
+						expected_werr,
+						expected_ntstatus);
+}
+
+static bool test_seconduseraddopendeleteprinter_write(struct torture_context *tctx,
+				  void *private_data)
+{
+	struct torture_printer_context *t =
+		(struct torture_printer_context *)talloc_get_type_abort(private_data, struct torture_printer_context);
+	struct dcerpc_pipe *p = t->spoolss_pipe;
+	struct dcerpc_pipe *p2 = t->spoolss_pipe2;
+	const char *printername = "deleteprinter2";
+	WERROR expected_werr = WERR_OK;
+	NTSTATUS expected_ntstatus = NT_STATUS_OK;
+	uint32_t access = PRINTER_WRITE;
+	return test_simple_seconduseropendeleteprinter(
+						tctx,
+						t,
+						p,
+						p2,
+						printername,
+						access,
+						expected_werr,
+						expected_ntstatus);
+}
+
+static bool test_seconduseraddopendeleteprinter_exec(struct torture_context *tctx,
+				  void *private_data)
+{
+	struct torture_printer_context *t =
+		(struct torture_printer_context *)talloc_get_type_abort(private_data, struct torture_printer_context);
+	struct dcerpc_pipe *p = t->spoolss_pipe;
+	struct dcerpc_pipe *p2 = t->spoolss_pipe2;
+	const char *printername = "deleteprinter3";
+	WERROR expected_werr = WERR_ACCESS_DENIED;
+	NTSTATUS expected_ntstatus = NT_STATUS_OK;
+	uint32_t access = PRINTER_EXECUTE;
+	return test_simple_seconduseropendeleteprinter(
+						tctx,
+						t,
+						p,
+						p2,
+						printername,
+						access,
+						expected_werr,
+						expected_ntstatus);
+}
+
 static bool test_csetprinter(struct torture_context *tctx,
 			     void *private_data)
 {
@@ -10290,6 +10486,18 @@ void torture_tcase_printerpolicy_handles(struct torture_tcase *tcase)
 	torture_tcase_add_simple_test(tcase, "opencloseprinter", test_openclose);
 	torture_tcase_add_simple_test(tcase, "seconduseradddeleteprinter",
 				      test_seconduserdeleteprinter);
+	torture_tcase_add_simple_test(tcase,
+			"seconduseropendeleteprinter",
+			test_seconduseraddopendeleteprinter);
+	torture_tcase_add_simple_test(tcase,
+			"seconduseropendeleteprinter_read",
+			test_seconduseraddopendeleteprinter_read);
+	torture_tcase_add_simple_test(tcase,
+			"seconduseropendeleteprinter_write",
+			test_seconduseraddopendeleteprinter_write);
+	torture_tcase_add_simple_test(tcase,
+			"seconduseraddopendeleteprinter_exec",
+			test_seconduseraddopendeleteprinter_exec);
 }
 
 struct torture_suite *torture_rpc_spoolss_printerhandle(TALLOC_CTX *mem_ctx)
-- 
2.43.0


From 018af28caf78bda7389fc6b6886cadebb397c1db Mon Sep 17 00:00:00 2001
From: Noel Power <noel.power@suse.com>
Date: Mon, 17 Feb 2025 15:21:05 +0000
Subject: [PATCH 13/16] selftest: Add a test second user test for SetPrinter
 with security descriptor

On windows SetPrinter will fail when handle opened by userA is used
by connection userB and passed to SetPrinter (with a secdesc)

This tests calling SetPrinter as userB with a handle obtained by
userA with;
  a)  securitydescriptor owner set to NULL   (should pass)
  b)  securitydescriptor owner set to userB   (should pass)
  c)  securitydescriptor owner set to default (owner set and NOT
      userB)  (should fail with INVALID_OWNER)

Adds knownfail as well

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15792
Signed-off-by: Noel Power <noel.power@suse.com>
---
 selftest/knownfail            |   1 +
 source4/torture/rpc/spoolss.c | 209 ++++++++++++++++++++++++++++++++++
 2 files changed, 210 insertions(+)

diff --git a/selftest/knownfail b/selftest/knownfail
index 39788d76bc7..311b9012056 100644
--- a/selftest/knownfail
+++ b/selftest/knownfail
@@ -242,6 +242,7 @@
 ^samba3.rpc.spoolss.printerhandles.printerpolicyhandle.seconduseropendeleteprinter_write\(.*\)$
 ^samba3.rpc.spoolss.printerhandles.printerpolicyhandle.seconduseraddopendeleteprinter_exec\(.*\)$
 ^samba3.rpc.spoolss.printerhandles.printerpolicyhandle.seconduseropendeleteprinter\(.*\)$
+^samba3.rpc.spoolss.printerhandles.printerpolicyhandle.opensetprintersd\(.*\)$
 
 ^samba4.rpc.fsrvp	# fsrvp server only provided by smbd
 #
diff --git a/source4/torture/rpc/spoolss.c b/source4/torture/rpc/spoolss.c
index 61f9d2addcc..309e26d8aab 100644
--- a/source4/torture/rpc/spoolss.c
+++ b/source4/torture/rpc/spoolss.c
@@ -42,6 +42,9 @@
 #include "torture/ndr/ndr.h"
 #include "torture/smb2/proto.h"
 #include "auth/credentials/credentials_krb5.h"
+#include "librpc/gen_ndr/ndr_lsa.h"
+#include "librpc/gen_ndr/ndr_lsa_c.h"
+
 
 #define TORTURE_WELLKNOWN_PRINTER	"torture_wkn_printer"
 #define TORTURE_PRINTER			"torture_printer"
@@ -8234,6 +8237,199 @@ static bool test_simple_openprinter(struct torture_context *tctx,
 	return true;
 }
 
+static bool test_simple_opensetprintersd(struct torture_context *tctx,
+				       struct dcerpc_pipe *p,
+				       struct dcerpc_pipe *p2,
+				       const char *real_printername,
+				       const char *environment)
+{
+	struct policy_handle handle;
+	bool ret = true;
+	struct dcerpc_binding_handle *b = p->binding_handle;
+	struct dcerpc_binding_handle *b2 = p2->binding_handle;
+	union spoolss_PrinterInfo info = {};
+	struct security_descriptor *orig_sd  = NULL;
+	struct security_descriptor *sd  = NULL;
+
+	torture_assert_goto(tctx,
+			call_OpenPrinterEx(tctx,
+				p,
+				real_printername,
+				NULL,
+				&handle),
+			ret,
+			cleanup,
+			"failed to open printer");
+
+	/* retrieve and copy current security descriptor */
+	torture_assert_goto(tctx,
+			    test_GetPrinter_level(tctx, b, &handle, 2, &info),
+			    ret,
+			    cleanup,
+			    "");
+
+	orig_sd = security_descriptor_copy(tctx, info.info2.secdesc);
+
+	sd = security_descriptor_copy(tctx, info.info2.secdesc);
+
+	sd->owner_sid = NULL;
+
+	/*
+	 * windows : SetPrinter will fail when handle is opened by user A is used
+	 *           by connection user B and passed to SetPrinter (with a secdesc)
+	 *           SetPrinter will fail with WERROR (Invalid security descriptor
+	 *           owner)
+	 *           But.. it also (at least on win2k12r2, win2k16 & win2k19)
+	 *           appears that if the secdesc is modified so user B is the owner
+	 *           or owner is NULL then SetPrinter will succeed, otherwise
+	 *           WERR_INVALID_OWNER is returned.
+	 *
+	 * samba: will succeed regardless of user, should fail
+	 */
+
+	/* call SetPrinter (with secdesc) as UserB with NULL owner (should succeed) */
+	torture_assert_goto(tctx, test_sd_set_level(tctx, b2, &handle, 3, sd),
+			    ret,
+			    cleanup,
+			    "failed to set security descriptor");
+
+	/* Set secdesc->owner_sid to user B sid and SetPrinter should succeed */
+	if (getenv("DOMAIN_USER") && getenv("DOMAIN_USER_PASSWORD")) {
+		struct dcerpc_pipe *lsa_pipe = NULL;
+		struct dcerpc_binding_handle *lsa_b = NULL;
+		struct policy_handle pol = {};
+		uint16_t system_name = '\\';
+
+		struct lsa_ObjectAttribute attr = { .len = 0x18, };
+		struct lsa_QosInfo qos = {
+			.len = 0xc, .impersonation_level = 2, .context_mode = 1, .effective_only = 0,
+		};
+		struct lsa_String *lsa_names = NULL;
+		struct lsa_RefDomainList *domains = NULL;
+		struct lsa_TransSidArray sid_array = { .count = 0, };
+		uint32_t count;
+		NTSTATUS errstatus;
+		uint32_t dom_idx;
+		WERROR werror;
+		struct spoolss_SetPrinterInfo3 info3;
+		struct spoolss_SetPrinterInfoCtr info_ctr = {};
+		struct spoolss_DevmodeContainer devmode_ctr = {};
+		struct sec_desc_buf secdesc_ctr = {};
+
+		attr.sec_qos            = &qos;
+
+		/* use lsa to get the sid for DOMAIN_USER */
+		torture_assert_ntstatus_ok(tctx,
+				torture_rpc_connection(tctx, &lsa_pipe, &ndr_table_lsarpc),
+				"failed to get lsa connection\n");
+
+		lsa_b = lsa_pipe->binding_handle;
+
+		torture_assert_ntstatus_ok(tctx,
+				dcerpc_lsa_OpenPolicy(lsa_b,
+					tctx,
+					&system_name,
+					&attr,
+					SEC_FLAG_MAXIMUM_ALLOWED,
+					&pol,
+					&errstatus),
+				"openpolicy failed\n");
+
+		torture_assert_ntstatus_ok(tctx, errstatus, "unexpected result");
+
+		lsa_names = talloc_zero_array(tctx, struct lsa_String, 1);
+
+		lsa_names[0].string = getenv("DOMAIN_USER");
+		lsa_names[0].size = 2 * strlen_m(getenv("DOMAIN_USER"));
+		lsa_names[0].length = lsa_names[0].size;
+
+
+		torture_assert_ntstatus_ok(tctx,
+				dcerpc_lsa_LookupNames(
+					lsa_b,
+					tctx,
+					&pol,
+					1,
+					lsa_names,
+					&domains,
+					&sid_array,
+					1,
+					&count,
+					&errstatus),
+				"failed to lookup names");
+
+		torture_assert_ntstatus_ok(tctx, errstatus, "failure looking up names\n");
+
+		dom_idx = sid_array.sids[0].sid_index;
+		torture_assert(tctx, dom_idx != 0xffffffff, "bad sid index");
+
+		torture_assert(tctx, domains != NULL, "invalid response looking up names\n");
+
+
+		torture_assert(tctx, dom_idx < domains->count,
+			       "invalid response looking up names\n");
+
+		torture_assert(tctx, domains->domains[dom_idx].sid != NULL,
+			       "invalid response looking up names\n");
+
+		sd->owner_sid = domains->domains[dom_idx].sid;
+		sid_append_rid(sd->owner_sid, sid_array.sids[0].rid);
+
+		/* set the sid (with owner set to userB 'DOMAIN_USER' should succeed */
+		torture_assert_goto(tctx, test_sd_set_level(tctx, b2, &handle, 3, sd),
+				    ret,
+				    cleanup,
+				    "failed to set security descriptor to userB");
+
+		/*
+		 * test_sd_set_level should fail when restoring the orig_sd
+		 * over userB's connection.
+		 * However calling test_sd_set_level calls other torture_assert
+		 * calls that when they fail mark the whole test as failed.
+		 * So, to avoid marking the test as failing we need to call SetPrinter
+		 * directly here.
+		 */
+
+		info_ctr.level = 3;
+		info_ctr.info.info3 = &info3;
+		secdesc_ctr.sd = orig_sd;
+
+		/*
+		 * assuming that DOMAIN_USER is different than owner
+		 * of orig_sd then we should fail here with INVALID_OWNER
+		 */
+		torture_assert_ntstatus_ok(tctx,
+			    dcerpc_spoolss_SetPrinter(b2, tctx,
+				    &handle,
+				    &info_ctr,
+				    &devmode_ctr,
+				    &secdesc_ctr,
+				    0,
+				    &werror),
+			    "unexpected failure calling SetPrinter as userB");
+
+		torture_assert_werr_equal_goto(tctx,
+				werror,
+				WERR_INVALID_OWNER,
+				ret,
+				cleanup,
+				"Unexpected error code");
+	} else {
+		torture_comment(tctx, "Skipping setting setting security_descriptor owner to 'userB'\n");
+	}
+
+	/* restore original sd */
+        torture_assert(tctx, test_sd_set_level(tctx, b, &handle, 3, orig_sd),
+		       "failed to restore initial security descriptor\n");
+
+cleanup:
+
+	/* close with binding that opened the handle */
+	torture_assert(tctx, test_ClosePrinter(tctx, b, &handle),
+			"failed to close printer\n");
+	return ret;
+}
+
 static bool test_openclose(struct torture_context *tctx,
 				  void *private_data)
 {
@@ -8542,6 +8738,17 @@ static bool test_seconduseraddopendeleteprinter_exec(struct torture_context *tct
 						expected_ntstatus);
 }
 
+static bool test_opensetprintersd(struct torture_context *tctx,
+				  void *private_data)
+{
+	struct torture_printer_context *t =
+		(struct torture_printer_context *)talloc_get_type_abort(private_data, struct torture_printer_context);
+	struct dcerpc_pipe *p = t->spoolss_pipe;
+	struct dcerpc_pipe *p2 = t->spoolss_pipe2;
+	const char *printername = t->info2.printername;
+	return test_simple_opensetprintersd(tctx, p, p2, printername, NULL);
+}
+
 static bool test_csetprinter(struct torture_context *tctx,
 			     void *private_data)
 {
@@ -10498,6 +10705,8 @@ void torture_tcase_printerpolicy_handles(struct torture_tcase *tcase)
 	torture_tcase_add_simple_test(tcase,
 			"seconduseraddopendeleteprinter_exec",
 			test_seconduseraddopendeleteprinter_exec);
+	torture_tcase_add_simple_test(tcase, "opensetprintersd",
+			test_opensetprintersd);
 }
 
 struct torture_suite *torture_rpc_spoolss_printerhandle(TALLOC_CTX *mem_ctx)
-- 
2.43.0


From 5fd023ee946d7153384e125398278e586e478754 Mon Sep 17 00:00:00 2001
From: Noel Power <noel.power@suse.com>
Date: Sun, 16 Feb 2025 13:25:17 +0000
Subject: [PATCH 14/16] selftest: Extend second user printer_handle tests

Run the standard spoolss torture_tcase_printer tests against
a setup that has 2 users, userA, userB, where userA opens
the printer that is (mostly) used by the tests defined in
torture_tcase_printer. userB is the user that runs the api(s)
used in the tests that use the printer handle obtained by userA.

Add knownfails for samba

samba3.rpc.spoolss.printerhandles.printerseconduserhandles.sd
samba3.rpc.spoolss.printerhandles.printerseconduserhandles.csetprinter
samba3.rpc.spoolss.printerhandles.printerseconduserhandles.print_test
samba3.rpc.spoolss.printerhandles.printerseconduserhandles.print_test_extended
samba3.rpc.spoolss.printerhandles.printerseconduserhandles.print_test_smbd
samba3.rpc.spoolss.printerhandles.printerseconduserhandles.print_test_purge
samba3.rpc.spoolss.printerhandles.printerseconduserhandles.dm
samba3.rpc.spoolss.printerhandles.printerseconduserhandles.printer_rename
samba3.rpc.spoolss.printerhandles.printerseconduserhandles.print_job_enum
samba3.rpc.spoolss.printerhandles.printerseconduserhandles.os_versions
samba3.rpc.spoolss.printerhandles.printerseconduserhandles.printer_info_winreg
samba3.rpc.spoolss.printerhandles.printerseconduserhandles.change_id
samba3.rpc.spoolss.printerhandles.printerseconduserhandles.keys
samba3.rpc.spoolss.printerhandles.printerseconduserhandles.printerdata_consistency
samba3.rpc.spoolss.printerhandles.printerseconduserhandles.printerdata_keys
samba3.rpc.spoolss.printerhandles.printerseconduserhandles.printerdata_values
samba3.rpc.spoolss.printerhandles.printerseconduserhandles.printerdata_set
samba3.rpc.spoolss.printerhandles.printerseconduserhandles.printerdata_winreg
samba3.rpc.spoolss.printerhandles.printerseconduserhandles.printerdata_dsspooler

Note: less tests fail with windows (Win2k12r2)

samba3.rpc.spoolss.serverhandles.printerpolicyhandle.opengetprintersd
samba3.rpc.spoolss.serverhandles.printerseconduserhandles.csetprinter
samba3.rpc.spoolss.serverhandles.printerseconduserhandles.print_test_extended
samba3.rpc.spoolss.serverhandles.printerseconduserhandles.sd
samba3.rpc.spoolss.serverhandles.printerseconduserhandles.printerdata_dsspooler
samba3.rpc.spoolss.serverhandles.printerseconduserhandles.driver_info_winreg

Also worth noting that some extra tests in Windows fail in later
releases (Win2k16 & win2k19 at least). These failure are all down
to OpenPrinterEx failing with ACCESS_DENIED error.
(needs more investigation, but running with a userB with
printoperator priveleges fixes the OpenPrinterEx failures and
the tests that fail are the same again as Win2k12r2)

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15792
Signed-off-by: Noel Power <noel.power@suse.com>
---
 selftest/knownfail            |  19 ++++++
 source4/torture/rpc/spoolss.c | 105 ++++++++++++++++++++++++++++++++--
 2 files changed, 118 insertions(+), 6 deletions(-)

diff --git a/selftest/knownfail b/selftest/knownfail
index 311b9012056..c9dc577014c 100644
--- a/selftest/knownfail
+++ b/selftest/knownfail
@@ -243,6 +243,25 @@
 ^samba3.rpc.spoolss.printerhandles.printerpolicyhandle.seconduseraddopendeleteprinter_exec\(.*\)$
 ^samba3.rpc.spoolss.printerhandles.printerpolicyhandle.seconduseropendeleteprinter\(.*\)$
 ^samba3.rpc.spoolss.printerhandles.printerpolicyhandle.opensetprintersd\(.*\)$
+^samba3.rpc.spoolss.printerhandles.printerseconduserhandles.sd\(.*\)$
+^samba3.rpc.spoolss.printerhandles.printerseconduserhandles.csetprinter\(.*\)$
+^samba3.rpc.spoolss.printerhandles.printerseconduserhandles.print_test\(.*\)$
+^samba3.rpc.spoolss.printerhandles.printerseconduserhandles.print_test_extended\(.*\)$
+^samba3.rpc.spoolss.printerhandles.printerseconduserhandles.print_test_smbd\(.*\)$
+^samba3.rpc.spoolss.printerhandles.printerseconduserhandles.print_test_purge\(.*\)$
+^samba3.rpc.spoolss.printerhandles.printerseconduserhandles.dm\(.*\)$
+^samba3.rpc.spoolss.printerhandles.printerseconduserhandles.printer_rename\(.*\)$
+^samba3.rpc.spoolss.printerhandles.printerseconduserhandles.print_job_enum\(.*\)$
+^samba3.rpc.spoolss.printerhandles.printerseconduserhandles.os_versions\(.*\)$
+^samba3.rpc.spoolss.printerhandles.printerseconduserhandles.printer_info_winreg\(.*\)$
+^samba3.rpc.spoolss.printerhandles.printerseconduserhandles.change_id\(.*\)$
+^samba3.rpc.spoolss.printerhandles.printerseconduserhandles.keys\(.*\)$
+^samba3.rpc.spoolss.printerhandles.printerseconduserhandles.printerdata_consistency\(.*\)$
+^samba3.rpc.spoolss.printerhandles.printerseconduserhandles.printerdata_keys\(.*\)$
+^samba3.rpc.spoolss.printerhandles.printerseconduserhandles.printerdata_values\(.*\)$
+^samba3.rpc.spoolss.printerhandles.printerseconduserhandles.printerdata_set\(.*\)$
+^samba3.rpc.spoolss.printerhandles.printerseconduserhandles.printerdata_winreg\(.*\)$
+^samba3.rpc.spoolss.printerhandles.printerseconduserhandles.printerdata_dsspooler\(.*\)$
 
 ^samba4.rpc.fsrvp	# fsrvp server only provided by smbd
 #
diff --git a/source4/torture/rpc/spoolss.c b/source4/torture/rpc/spoolss.c
index 309e26d8aab..5a7a6c2695c 100644
--- a/source4/torture/rpc/spoolss.c
+++ b/source4/torture/rpc/spoolss.c
@@ -8375,15 +8375,15 @@ static bool test_simple_opensetprintersd(struct torture_context *tctx,
 		sd->owner_sid = domains->domains[dom_idx].sid;
 		sid_append_rid(sd->owner_sid, sid_array.sids[0].rid);
 
-		/* set the sid (with owner set to userB 'DOMAIN_USER' should succeed */
+		/* set the sid (with owner set to user B 'DOMAIN_USER' should succeed */
 		torture_assert_goto(tctx, test_sd_set_level(tctx, b2, &handle, 3, sd),
 				    ret,
 				    cleanup,
-				    "failed to set security descriptor to userB");
+				    "failed to set security descriptor to user B");
 
 		/*
 		 * test_sd_set_level should fail when restoring the orig_sd
-		 * over userB's connection.
+		 * over user B's connection.
 		 * However calling test_sd_set_level calls other torture_assert
 		 * calls that when they fail mark the whole test as failed.
 		 * So, to avoid marking the test as failing we need to call SetPrinter
@@ -8406,7 +8406,7 @@ static bool test_simple_opensetprintersd(struct torture_context *tctx,
 				    &secdesc_ctr,
 				    0,
 				    &werror),
-			    "unexpected failure calling SetPrinter as userB");
+			    "unexpected failure calling SetPrinter as user B");
 
 		torture_assert_werr_equal_goto(tctx,
 				werror,
@@ -8415,7 +8415,7 @@ static bool test_simple_opensetprintersd(struct torture_context *tctx,
 				cleanup,
 				"Unexpected error code");
 	} else {
-		torture_comment(tctx, "Skipping setting setting security_descriptor owner to 'userB'\n");
+		torture_comment(tctx, "Skipping setting setting security_descriptor owner to 'user B'\n");
 	}
 
 	/* restore original sd */
@@ -9646,6 +9646,77 @@ static bool torture_rpc_spoolss_second_user_setup_plain(struct torture_context *
 }
 
 
+/*
+ * 2 users
+ * t->spoolss_pipe is DOMAIN_USER (or anonymous)
+ * t->spoolss_pipe2 is the default user (Administrator) passed to smbtorture
+ *
+ * BUT t->handle which is passed to apis in most of the tests
+ *     has been opened with t->spoolss_pipe2 (default user)
+ *
+ * This setup is handy for running existing tests that will make rpc calls
+ * using the 'second' users connection using a handle obtained from the
+ * default user
+ */
+static bool torture_rpc_spoolss_second_user_setup(struct torture_context *tctx, void **data)
+{
+	struct torture_printer_context *t = NULL;
+	struct dcerpc_pipe *tmp_pipe = NULL;
+	struct spoolss_DevmodeContainer devmode_ctr = {};
+	WERROR werror;
+
+	torture_assert(tctx,
+		       torture_rpc_spoolss_second_user_setup_common(tctx, data),
+		       "testcase setupfailed");
+
+	t = talloc_get_type_abort(*data, struct torture_printer_context);
+
+	/*
+	 * On windows using the handle retrieved from AddPrinter seems
+	 * to create real problems with lots of tests and apis
+	 * Looks similar to the issue of only able to DeletePrinter from
+	 * the same session that returned the handle, e.g. it looks like
+	 * the handle returned from AddPrinter is mostly only usable
+	 * from the session that returned it.
+	 */
+
+	/* to get more test coverage (but realising this is still an
+	 * issue) close t->handle and reopen with std OpenPrinter
+	 */
+	torture_assert(tctx,
+		test_ClosePrinter(tctx,
+				  t->spoolss_pipe->binding_handle,
+				  &t->handle),
+		"failed to close printserver");
+
+	torture_assert_ntstatus_ok(tctx,
+			dcerpc_spoolss_OpenPrinter(
+				t->spoolss_pipe->binding_handle,
+				tctx,
+				t->info2.printername,
+				NULL,
+				devmode_ctr,
+				PRINTER_ALL_ACCESS,
+				&t->handle,
+				&werror),
+			"failed to open printer\n");
+
+	torture_assert_werr_ok(tctx,
+		werror,
+		"OpenPrinter failed");
+
+	/*
+	 * swap pipes around so primary pipe is actually the pipe opened
+	 * by second user, this is so we can reuse some existing tests
+	 * where t->handle and/or t->server_handle are used, those tests
+	 * should now run with 'other' client/user (now at t->spools_pipe)
+	 */
+	tmp_pipe = t->spoolss_pipe2;
+	t->spoolss_pipe2 = t->spoolss_pipe;
+	t->spoolss_pipe = tmp_pipe;
+	return true;
+}
+
 static bool test_DeletePrinterDriverEx_exp(struct torture_context *tctx,
 					   struct dcerpc_binding_handle *b,
 					   const char *server,
@@ -9761,6 +9832,19 @@ static bool torture_rpc_spoolss_second_user_printer_plain_teardown(struct tortur
 	return ret;
 }
 
+static bool torture_rpc_spoolss_second_user_printer_teardown(struct torture_context *tctx, void *data)
+{
+	struct torture_printer_context *t = talloc_get_type(data, struct torture_printer_context);
+	bool ret;
+
+	ret = torture_rpc_spoolss_second_user_printer_teardown_common(tctx,
+								      t,
+								      true);
+	talloc_free(t);
+
+	return ret;
+}
+
 static bool test_print_test(struct torture_context *tctx,
 			    void *private_data)
 {
@@ -10717,8 +10801,17 @@ struct torture_suite *torture_rpc_spoolss_printerhandle(TALLOC_CTX *mem_ctx)
 	torture_tcase_set_fixture(tcase,
 				  torture_rpc_spoolss_second_user_setup_plain,
 				  torture_rpc_spoolss_second_user_printer_plain_teardown);
-
 	torture_tcase_printerpolicy_handles(tcase);
+	/*
+	 * runs basic spoolss printer tests with connection from user B where
+	 * the handle used in most tests has been opened by connection
+	 * from user A
+	 */
+	tcase = torture_suite_add_tcase(suite, "printerseconduserhandles");
+	torture_tcase_set_fixture(tcase,
+				  torture_rpc_spoolss_second_user_setup,
+				  torture_rpc_spoolss_second_user_printer_teardown);
+	torture_tcase_printer(tcase);
 	return suite;
 }
 
-- 
2.43.0


From 18bc70fa9488a7e48f12734b2817b9e98dbb522f Mon Sep 17 00:00:00 2001
From: Noel Power <noel.power@suse.com>
Date: Mon, 17 Feb 2025 08:49:13 +0000
Subject: [PATCH 15/16] selftest: Add some printserver tests (with userB using
 userA's handle)

Add and resuse some existing tests that use the printserver print_handle
Setup creates userA & userB connections, userA opens the printserver
print_handle as usual but then we swap the userA & userB connections
so the tests run as userB but access the printserver print_handle
obtained by userA

Add knownfails too.

Note: On Windows2k12r2, the following fail on both windows and samba

samba3.rpc.spoolss.serverhandles.serverpolicyhandle.forms
samba3.rpc.spoolss.serverhandles.serverpolicyhandle.forms_winreg
samba3.rpc.spoolss.serverhandles.serverpolicyhandle.printserver_info_winreg

and on samba these extra ones fail

^samba3.rpc.spoolss.serverhandles.serverpolicyhandle.printer_data_list
^samba3.rpc.spoolss.serverhandles.serverpolicyhandle.get_printer\(.*\)$
^samba3.rpc.spoolss.serverhandles.serverpolicyhandle.set_printer\(.*\)$

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15792
Signed-off-by: Noel Power <noel.power@suse.com>
---
 selftest/knownfail            |   6 ++
 source3/selftest/tests.py     |   1 +
 source4/torture/rpc/spoolss.c | 112 ++++++++++++++++++++++++++++++++++
 3 files changed, 119 insertions(+)

diff --git a/selftest/knownfail b/selftest/knownfail
index c9dc577014c..a96d2b49d4f 100644
--- a/selftest/knownfail
+++ b/selftest/knownfail
@@ -262,6 +262,12 @@
 ^samba3.rpc.spoolss.printerhandles.printerseconduserhandles.printerdata_set\(.*\)$
 ^samba3.rpc.spoolss.printerhandles.printerseconduserhandles.printerdata_winreg\(.*\)$
 ^samba3.rpc.spoolss.printerhandles.printerseconduserhandles.printerdata_dsspooler\(.*\)$
+^samba3.rpc.spoolss.serverhandles.serverpolicyhandle.forms\(.*\)$
+^samba3.rpc.spoolss.serverhandles.serverpolicyhandle.forms_winreg\(.*\)$
+^samba3.rpc.spoolss.serverhandles.serverpolicyhandle.printer_data_list
+^samba3.rpc.spoolss.serverhandles.serverpolicyhandle.get_printer\(.*\)$
+^samba3.rpc.spoolss.serverhandles.serverpolicyhandle.set_printer\(.*\)$
+^samba3.rpc.spoolss.serverhandles.serverpolicyhandle.printserver_info_winreg\(.*\)$
 
 ^samba4.rpc.fsrvp	# fsrvp server only provided by smbd
 #
diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py
index 03f2214a2e9..a1f71600f4a 100755
--- a/source3/selftest/tests.py
+++ b/source3/selftest/tests.py
@@ -1071,6 +1071,7 @@ rpc = ["rpc.authcontext",
        "rpc.spoolss.notify",
        "rpc.spoolss.printer",
        "rpc.spoolss.printerhandles",
+       "rpc.spoolss.serverhandles",
        "rpc.spoolss.driver",
        "rpc.lsa",
        "rpc.lsa-getuser",
diff --git a/source4/torture/rpc/spoolss.c b/source4/torture/rpc/spoolss.c
index 5a7a6c2695c..7bc12f7ca10 100644
--- a/source4/torture/rpc/spoolss.c
+++ b/source4/torture/rpc/spoolss.c
@@ -71,6 +71,7 @@
 
 struct test_spoolss_context {
 	struct dcerpc_pipe *spoolss_pipe;
+	struct dcerpc_pipe *spoolss_pipe2;
 
 	/* server environment */
 	const char *environment;
@@ -9717,6 +9718,91 @@ static bool torture_rpc_spoolss_second_user_setup(struct torture_context *tctx,
 	return true;
 }
 
+static bool torture_rpc_spoolss_second_user_setup_server(struct torture_context *tctx, void **data)
+{
+	struct test_spoolss_context *t = NULL;
+	struct cli_credentials *cmdline_creds = NULL;
+	struct cli_credentials *user2_creds = NULL;
+	const struct dcerpc_binding *binding2 = NULL;
+	struct dcerpc_pipe *tmp_pipe = NULL;
+	NTSTATUS status;
+
+	torture_assert(tctx,
+		torture_rpc_spoolss_setup(tctx, data),
+		"testcase setup failed");
+
+	t = talloc_get_type_abort(*data, struct test_spoolss_context);
+
+	cmdline_creds = samba_cmdline_get_creds();
+	torture_assert(tctx,
+		       cmdline_creds != NULL,
+		       "samba_cmdline_get_creds failed");
+
+	user2_creds = cli_credentials_init_anon(tctx);
+	torture_assert(tctx,
+		       user2_creds != NULL,
+		       "cli_credentials_init_anon failed");
+
+	/*
+	 * if the environment has a DOMAIN_USER & DOMAIN_USER_PASSWORD
+	 * defined use these, otherwise create an anonymous connection
+	 */
+	if (getenv("DOMAIN_USER") && getenv("DOMAIN_USER_PASSWORD")) {
+		user2_creds = cli_credentials_shallow_copy(tctx,
+				cmdline_creds);
+		cli_credentials_set_username(user2_creds,
+					     getenv("DOMAIN_USER"),
+					     CRED_SPECIFIED);
+		cli_credentials_set_password(user2_creds,
+					     getenv("DOMAIN_USER_PASSWORD"),
+					     CRED_SPECIFIED);
+	}
+
+	/*
+	 * copy the binding to ensure we are using the same assoc_group_id
+	 * so we are communicating with the same spools instance that
+	 * created and returned the handle
+	 */
+	binding2 = t->spoolss_pipe->binding;
+
+	status = dcerpc_pipe_connect_b(tctx,
+				     &t->spoolss_pipe2,
+				     binding2,
+				     &ndr_table_spoolss,
+				     user2_creds, tctx->ev,
+				     tctx->lp_ctx);
+
+	torture_assert_ntstatus_ok(tctx,
+			status,
+			"Error creating second connection to server "
+			"as second user");
+
+	/*
+	 * swap pipes around so primary pipe is actually the pipe opened
+	 * by second user, this is so we can reuse some existing tests
+	 * where t->handle and/or t->server_handle are used, those tests
+	 * should now run with 'other' client/user (now at t->spools_pipe)
+	 */
+	tmp_pipe = t->spoolss_pipe2;
+	t->spoolss_pipe2 = t->spoolss_pipe;
+	t->spoolss_pipe = tmp_pipe;
+	return true;
+}
+
+static bool torture_rpc_spoolss_second_user_teardown_server(struct torture_context *tctx, void *data)
+{
+	struct test_spoolss_context *t = talloc_get_type(data, struct test_spoolss_context);
+	bool ret;
+
+	/* swap pipes back so we delete with orig connection */
+	t->spoolss_pipe = t->spoolss_pipe2;
+
+	ret = torture_rpc_spoolss_teardown_common(tctx, t);
+	talloc_free(t);
+	return ret;
+}
+
+
 static bool test_DeletePrinterDriverEx_exp(struct torture_context *tctx,
 					   struct dcerpc_binding_handle *b,
 					   const char *server,
@@ -10793,6 +10879,31 @@ void torture_tcase_printerpolicy_handles(struct torture_tcase *tcase)
 			test_opensetprintersd);
 }
 
+struct torture_suite *torture_rpc_spoolss_serverhandle(TALLOC_CTX *mem_ctx)
+{
+	struct torture_suite *suite = torture_suite_create(mem_ctx, "serverhandles");
+	struct torture_tcase *tcase;
+	tcase = torture_suite_add_tcase(suite, "serverpolicyhandle");
+	torture_tcase_set_fixture(tcase,
+				  torture_rpc_spoolss_second_user_setup_server,
+				  torture_rpc_spoolss_second_user_teardown_server);
+	torture_tcase_add_simple_test(tcase, "printer_data_list", test_GetPrinterData_list);
+	torture_tcase_add_simple_test(tcase, "enum_forms", test_PrintServer_EnumForms);
+	torture_tcase_add_simple_test(tcase, "get_printer", test_get_printer_printserverhandle);
+	/* fails on samba, passes on windows */
+	torture_tcase_add_simple_test(tcase, "forms", test_PrintServer_Forms);
+	/* fails on both samba and windows */
+	torture_tcase_add_simple_test(tcase, "forms_winreg", test_PrintServer_Forms_Winreg);
+	/* fails on windows related to sescdesc (and second user) */
+	torture_tcase_add_simple_test(tcase, "set_printer", test_set_printer_printserverhandle);
+	/*
+	 * fails on windows passes on samba, again seems related to secdesc
+	 * (and second user)
+	 */
+	torture_tcase_add_simple_test(tcase, "printserver_info_winreg", test_printserver_info_winreg);
+	return suite;
+}
+
 struct torture_suite *torture_rpc_spoolss_printerhandle(TALLOC_CTX *mem_ctx)
 {
 	struct torture_suite *suite = torture_suite_create(mem_ctx, "printerhandles");
@@ -10901,6 +11012,7 @@ struct torture_suite *torture_rpc_spoolss(TALLOC_CTX *mem_ctx)
 	torture_suite_add_suite(suite, torture_rpc_spoolss_printer(suite));
 	torture_suite_add_suite(suite,
 				torture_rpc_spoolss_printerhandle(suite));
+	torture_suite_add_suite(suite, torture_rpc_spoolss_serverhandle(suite));
 	return suite;
 }
 
-- 
2.43.0


From 76cd3964ee7451201b31187b36267a9d9e2bb2e5 Mon Sep 17 00:00:00 2001
From: Noel Power <noel.power@suse.com>
Date: Tue, 18 Mar 2025 11:29:51 +0000
Subject: [PATCH 16/16] SUSE: Update changelog for bnc#1234210

---
 packaging/SuSE/samba.changes | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/packaging/SuSE/samba.changes b/packaging/SuSE/samba.changes
index 1b5dd49a08f..372788e5aef 100644
--- a/packaging/SuSE/samba.changes
+++ b/packaging/SuSE/samba.changes
@@ -1,3 +1,9 @@
+-------------------------------------------------------------------
+Tue Mar 18 11:28:11 UTC 2025 - Noel Power <nopower@suse.com>
+
+- Fix Samba printers reporting invalid sid during print jobs;
+  (bsc#1234210); (bso#15792).
+
 -------------------------------------------------------------------
 Wed Feb  5 10:05:23 UTC 2025 - Samuel Cabrero <scabrero@suse.de>
 
-- 
2.43.0

