[PATCH] Fix several issues with spoolss testing

Andreas Schneider asn at samba.org
Mon Aug 14 09:16:38 UTC 2017


Hi,

the attached patch fixes several issues we have with the torture test or 
scripts used.


Review and push appreciated.


Thanks,


	Andreas

-- 
Andreas Schneider                   GPG-ID: CC014E3D
Samba Team                             asn at samba.org
www.samba.org
-------------- next part --------------
>From a18f8a3b4f0b7b3eee79354a8f94cb3930c6c647 Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn at samba.org>
Date: Fri, 18 Nov 2016 15:06:22 +0100
Subject: [PATCH 1/5] s3:spoolss: Set timeout values to the one which Windows
 uses by default

Signed-off-by: Andreas Schneider <asn at samba.org>
---
 source3/rpc_server/spoolss/srv_spoolss_nt.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/source3/rpc_server/spoolss/srv_spoolss_nt.c b/source3/rpc_server/spoolss/srv_spoolss_nt.c
index 9d99e74f395..74745b45312 100644
--- a/source3/rpc_server/spoolss/srv_spoolss_nt.c
+++ b/source3/rpc_server/spoolss/srv_spoolss_nt.c
@@ -4178,8 +4178,8 @@ static WERROR construct_printer_info5(TALLOC_CTX *mem_ctx,
 	r->attributes	= info2->attributes;
 
 	/* these two are not used by NT+ according to MSDN */
-	r->device_not_selected_timeout		= 0x0;  /* have seen 0x3a98 */
-	r->transmission_retry_timeout		= 0x0;  /* have seen 0xafc8 */
+	r->device_not_selected_timeout		= 0xafc8;
+	r->transmission_retry_timeout		= 0xafc8;
 
 	return WERR_OK;
 }
-- 
2.14.0


>From a020526d4a4cc27560172ae067d4222713a39aba Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn at samba.org>
Date: Tue, 8 Aug 2017 08:40:34 +0200
Subject: [PATCH 2/5] s3:script: Untaint user supplied data in modprinter.pl

spoolss_SetPrinter fails because of the error produced by modprinter.pl.

Perl error:
Insecure dependency in open while running setgid at modprinter.pl line 76.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=12950

Signed-off-by: Andreas Schneider <asn at samba.org>
---
 source3/script/tests/printing/modprinter.pl | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/source3/script/tests/printing/modprinter.pl b/source3/script/tests/printing/modprinter.pl
index 9e5e3292c6c..ec1ebcd7ab8 100755
--- a/source3/script/tests/printing/modprinter.pl
+++ b/source3/script/tests/printing/modprinter.pl
@@ -67,7 +67,14 @@ if (!defined($share_name)) {
 	die "share name not defined";
 }
 
-my $tmp = $opt_smb_conf.$$;
+my $smb_conf_file = $opt_smb_conf;
+if ($smb_conf_file =~ /^(.*)$/) {
+	$smb_conf_file = $1; # untaint file name
+} else {
+	die "Invalid file name $smb_conf_file";
+}
+
+my $tmp = $smb_conf_file.$$;
 
 my $section = undef;
 my $within_section = 0;
@@ -75,7 +82,7 @@ my $found_section = 0;
 
 open(CONFIGFILE_NEW, "+>$tmp") || die "Unable top open conf file $tmp";
 
-open (CONFIGFILE, "+<$opt_smb_conf") || die "Unable to open config file $opt_smb_conf";
+open (CONFIGFILE, "+<$smb_conf_file") || die "Unable to open config file $smb_conf_file";
 while (<CONFIGFILE>) {
 	my $line = $_;
 	chomp($_);
@@ -123,7 +130,9 @@ close (CONFIGFILE_NEW);
 if ($opt_delete && ($found_section == 0)) {
 	die "share $share_name not found";
 }
-system("cp", "$tmp", "$opt_smb_conf");
+
+$ENV{'PATH'} = '/bin:/usr/bin'; # untaint PATH
+system("cp", "$tmp", "$smb_conf_file");
 unlink $tmp;
 
 exit 0;
-- 
2.14.0


>From faeab58b97b4dff84316c4faae23b337fe1e6ec0 Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn at samba.org>
Date: Tue, 8 Aug 2017 11:25:48 +0200
Subject: [PATCH 3/5] s4:torture: Use a different driver name for add_driver
 tests

Signed-off-by: Andreas Schneider <asn at samba.org>
---
 source4/torture/rpc/spoolss.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/source4/torture/rpc/spoolss.c b/source4/torture/rpc/spoolss.c
index 89e9a134eff..a6315069bc8 100644
--- a/source4/torture/rpc/spoolss.c
+++ b/source4/torture/rpc/spoolss.c
@@ -47,6 +47,7 @@
 #define TORTURE_WELLKNOWN_PRINTER_EX	"torture_wkn_printer_ex"
 #define TORTURE_PRINTER_EX		"torture_printer_ex"
 #define TORTURE_DRIVER			"torture_driver"
+#define TORTURE_DRIVER_ADD		"torture_driver_add"
 #define TORTURE_DRIVER_EX		"torture_driver_ex"
 #define TORTURE_DRIVER_ADOBE		"torture_driver_adobe"
 #define TORTURE_DRIVER_EX_ADOBE		"torture_driver_ex_adobe"
@@ -10932,7 +10933,7 @@ static bool test_add_driver_64(struct torture_context *tctx,
 	d->local.driver_directory	= talloc_strdup(d, "/usr/share/cups/drivers/x64");
 
 	d->info8.version		= SPOOLSS_DRIVER_VERSION_200X;
-	d->info8.driver_name		= TORTURE_DRIVER;
+	d->info8.driver_name		= TORTURE_DRIVER_ADD;
 	d->info8.architecture		= d->local.environment;
 	d->info8.driver_path		= talloc_strdup(d, "pscript5.dll");
 	d->info8.data_file		= talloc_strdup(d, "cups6.ppd");
@@ -10953,7 +10954,7 @@ static bool test_add_driver_32(struct torture_context *tctx,
 	d->local.driver_directory	= talloc_strdup(d, "/usr/share/cups/drivers/i386");
 
 	d->info8.version		= SPOOLSS_DRIVER_VERSION_200X;
-	d->info8.driver_name		= TORTURE_DRIVER;
+	d->info8.driver_name		= TORTURE_DRIVER_ADD;
 	d->info8.architecture		= d->local.environment;
 	d->info8.driver_path		= talloc_strdup(d, "pscript5.dll");
 	d->info8.data_file		= talloc_strdup(d, "cups6.ppd");
-- 
2.14.0


>From cf49f3c5eed231946bf620186cef9c240ea69474 Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn at samba.org>
Date: Tue, 8 Aug 2017 10:40:19 +0200
Subject: [PATCH 4/5] s4:torture: Delete printer before we remove the driver

Signed-off-by: Andreas Schneider <asn at samba.org>
---
 source4/torture/rpc/spoolss.c | 31 ++++++++++++++++---------------
 1 file changed, 16 insertions(+), 15 deletions(-)

diff --git a/source4/torture/rpc/spoolss.c b/source4/torture/rpc/spoolss.c
index a6315069bc8..2f60383e0e6 100644
--- a/source4/torture/rpc/spoolss.c
+++ b/source4/torture/rpc/spoolss.c
@@ -8584,6 +8584,22 @@ static bool torture_rpc_spoolss_printer_teardown_common(struct torture_context *
 
 	server_name_slash = talloc_asprintf(tctx, "\\\\%s", dcerpc_server_name(p));
 
+	if (!t->wellknown) {
+		const char *printer_name = t->info2.printername;
+
+		torture_assert(tctx,
+			test_DeletePrinter(tctx, b, &t->handle),
+			"failed to delete printer");
+
+		torture_assert(tctx,
+			test_EnumPrinters_findname(tctx, b, PRINTER_ENUM_LOCAL, 1,
+						   printer_name, &found),
+			"failed to enumerate printers");
+
+		torture_assert(tctx, !found, "deleted printer still there");
+	}
+
+
 	if (t->added_driver) {
 		torture_assert(tctx,
 			remove_printer_driver(tctx, dcerpc_server_name(p), &t->driver),
@@ -8600,21 +8616,6 @@ static bool torture_rpc_spoolss_printer_teardown_common(struct torture_context *
 			"failed to delete printer driver via spoolss");
 	}
 
-	if (!t->wellknown) {
-		const char *printer_name = t->info2.printername;
-
-		torture_assert(tctx,
-			test_DeletePrinter(tctx, b, &t->handle),
-			"failed to delete printer");
-
-		torture_assert(tctx,
-			test_EnumPrinters_findname(tctx, b, PRINTER_ENUM_LOCAL, 1,
-						   printer_name, &found),
-			"failed to enumerate printers");
-
-		torture_assert(tctx, !found, "deleted printer still there");
-	}
-
 	return true;
 }
 
-- 
2.14.0


>From 25271ab3076e49a81dfcdd678b1b93fe8d835477 Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn at samba.org>
Date: Tue, 8 Aug 2017 12:05:24 +0200
Subject: [PATCH 5/5] s4:torture: The teardown function should just return

The teardown functions should not return on error but finish cleaning
up!

Signed-off-by: Andreas Schneider <asn at samba.org>
---
 source4/torture/rpc/spoolss.c | 49 +++++++++++++++++++++++++++++--------------
 1 file changed, 33 insertions(+), 16 deletions(-)

diff --git a/source4/torture/rpc/spoolss.c b/source4/torture/rpc/spoolss.c
index 2f60383e0e6..d4f69698d96 100644
--- a/source4/torture/rpc/spoolss.c
+++ b/source4/torture/rpc/spoolss.c
@@ -8576,6 +8576,7 @@ static bool torture_rpc_spoolss_printer_teardown_common(struct torture_context *
 	struct dcerpc_pipe *p = t->spoolss_pipe;
 	struct dcerpc_binding_handle *b = NULL;
 	const char *server_name_slash;
+	bool ok = true;
 
 	if (p == NULL) {
 		return true;
@@ -8587,36 +8588,52 @@ static bool torture_rpc_spoolss_printer_teardown_common(struct torture_context *
 	if (!t->wellknown) {
 		const char *printer_name = t->info2.printername;
 
-		torture_assert(tctx,
+		torture_assert_goto(tctx,
 			test_DeletePrinter(tctx, b, &t->handle),
+			ok,
+			remove_driver,
 			"failed to delete printer");
 
-		torture_assert(tctx,
+		torture_assert_goto(tctx,
 			test_EnumPrinters_findname(tctx, b, PRINTER_ENUM_LOCAL, 1,
 						   printer_name, &found),
+			ok,
+			remove_driver,
 			"failed to enumerate printers");
 
-		torture_assert(tctx, !found, "deleted printer still there");
+		torture_assert_goto(tctx,
+			!found,
+			ok,
+			remove_driver,
+			"deleted printer still there");
 	}
 
 
+remove_driver:
 	if (t->added_driver) {
-		torture_assert(tctx,
-			remove_printer_driver(tctx, dcerpc_server_name(p), &t->driver),
-			"failed to remove printer driver");
+		ok = remove_printer_driver(tctx,
+					   dcerpc_server_name(p),
+					   &t->driver);
+		if (!ok) {
+			torture_warning(tctx,
+					"failed to remove printer driver\n");
+		}
 
-		torture_assert(tctx,
-			test_DeletePrinterDriverEx_exp(tctx, b,
-						       server_name_slash,
-						       t->driver.info8.driver_name,
-						       t->driver.info8.architecture,
-						       DPD_DELETE_ALL_FILES,
-						       t->driver.info8.version,
-						       WERR_OK),
-			"failed to delete printer driver via spoolss");
+		ok = test_DeletePrinterDriverEx_exp(tctx, b,
+						    server_name_slash,
+						    t->driver.info8.driver_name,
+						    t->driver.info8.architecture,
+						    DPD_DELETE_ALL_FILES,
+						    t->driver.info8.version,
+						    WERR_OK);
+		if (!ok) {
+			torture_warning(tctx,
+					"failed to delete printer driver via "
+					"spoolss\n");
+		}
 	}
 
-	return true;
+	return ok;
 }
 
 static bool torture_rpc_spoolss_printer_teardown(struct torture_context *tctx, void *data)
-- 
2.14.0



More information about the samba-technical mailing list