Attachment 8918 Details for Bug 9900 – proposed fix v2, for 3.6 branch

[patch] proposed fix v2, for 3.6 branch

bug-9900_v2_36test.patch (text/plain), 12.16 KB, created by David Disseldorp on 2013-05-24 10:06:56 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: David Disseldorp

Created: 2013-05-24 10:06:56 UTC

Size: 12.16 KB

patch

obsolete

>From 4e248d54be8b59a62392615721e44e2a0ec4fbdb Mon Sep 17 00:00:00 2001
>From: David Disseldorp <ddiss@samba.org>
>Date: Wed, 24 Apr 2013 14:06:50 +0200
>Subject: [PATCH 1/3] printing: use const in is_printer_published
>
>---
> source3/include/nt_printing.h      | 3 ++-
> source3/printing/nt_printing_ads.c | 6 ++++--
> 2 files changed, 6 insertions(+), 3 deletions(-)
>
>diff --git a/source3/include/nt_printing.h b/source3/include/nt_printing.h
>index 16c4658..658936c 100644
>--- a/source3/include/nt_printing.h
>+++ b/source3/include/nt_printing.h
>@@ -141,7 +141,8 @@ WERROR nt_printer_publish(TALLOC_CTX *mem_ctx,
> bool is_printer_published(TALLOC_CTX *mem_ctx,
> 			  const struct auth_serversupplied_info *server_info,
> 			  struct messaging_context *msg_ctx,
>-			  const char *servername, char *printer, struct GUID *guid,
>+			  const char *servername, const char *printer,
>+			  struct GUID *guid,
> 			  struct spoolss_PrinterInfo2 **info2);
> 
> WERROR check_published_printers(struct messaging_context *msg_ctx);
>diff --git a/source3/printing/nt_printing_ads.c b/source3/printing/nt_printing_ads.c
>index 219124f..572b541 100644
>--- a/source3/printing/nt_printing_ads.c
>+++ b/source3/printing/nt_printing_ads.c
>@@ -479,7 +479,8 @@ done:
> bool is_printer_published(TALLOC_CTX *mem_ctx,
> 			  const struct auth_serversupplied_info *session_info,
> 			  struct messaging_context *msg_ctx,
>-			  const char *servername, char *printer, struct GUID *guid,
>+			  const char *servername, const char *printer,
>+			  struct GUID *guid,
> 			  struct spoolss_PrinterInfo2 **info2)
> {
> 	struct spoolss_PrinterInfo2 *pinfo2 = NULL;
>@@ -574,7 +575,8 @@ WERROR check_published_printers(struct messaging_context *msg_ctx)
> bool is_printer_published(TALLOC_CTX *mem_ctx,
> 			  const struct auth_serversupplied_info *session_info,
> 			  struct messaging_context *msg_ctx,
>-			  const char *servername, char *printer, struct GUID *guid,
>+			  const char *servername, const char *printer,
>+			  struct GUID *guid,
> 			  struct spoolss_PrinterInfo2 **info2)
> {
> 	return False;
>-- 
>1.8.1.4
>
>
>From 1caf9d3c462990c9009f25ebd61cfe52162d9c6e Mon Sep 17 00:00:00 2001
>From: David Disseldorp <ddiss@samba.org>
>Date: Thu, 23 May 2013 19:32:08 +0200
>Subject: [PATCH 2/3] printing: explicitly clear PUBLISHED attribute
>
>Currently nt_printer_publish(DSPRINT_UNPUBLISH) flips (via xor) the
>info2->attributes PRINTER_ATTRIBUTE_PUBLISHED flag, rather than
>explicitly clearing it.
>---
> source3/printing/nt_printing_ads.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/source3/printing/nt_printing_ads.c b/source3/printing/nt_printing_ads.c
>index 572b541..9aab88b 100644
>--- a/source3/printing/nt_printing_ads.c
>+++ b/source3/printing/nt_printing_ads.c
>@@ -355,7 +355,7 @@ WERROR nt_printer_publish(TALLOC_CTX *mem_ctx,
> 		pinfo2->attributes |= PRINTER_ATTRIBUTE_PUBLISHED;
> 		break;
> 	case DSPRINT_UNPUBLISH:
>-		pinfo2->attributes ^= PRINTER_ATTRIBUTE_PUBLISHED;
>+		pinfo2->attributes &= (~PRINTER_ATTRIBUTE_PUBLISHED);
> 		break;
> 	default:
> 		win_rc = WERR_NOT_SUPPORTED;
>-- 
>1.8.1.4
>
>
>From c35e685180aa2314983ae7b6a0441ae435b3d4d0 Mon Sep 17 00:00:00 2001
>From: David Disseldorp <ddiss@samba.org>
>Date: Wed, 22 May 2013 17:58:38 +0200
>Subject: [PATCH 3/3] Fix bug 9900: is_printer_published GUID retrieval
>
>Samba currently always responds to GetPrinter(level = 7) requests with
>DSPRINT_UNPUBLISH, regardless of the AD publish status tracked via the
>PRINTER_ATTRIBUTE_PUBLISHED flag. This is due to erroneous "objectGUID"
>unmarshalling in is_printer_published().
>
>This change splits "objectGUID" retrieval into a separate function, and
>adds a pull_reg_sz() call to correctly unmarshall the GUID.
>---
> source3/include/nt_printing.h               |   9 +-
> source3/printing/nt_printing_ads.c          | 135 ++++++++++++++++++----------
> source3/rpc_server/spoolss/srv_spoolss_nt.c |  41 +++++++--
> source3/smbd/server_reload.c                |   5 +-
> 4 files changed, 128 insertions(+), 62 deletions(-)
>
>diff --git a/source3/include/nt_printing.h b/source3/include/nt_printing.h
>index 658936c..cdbad87 100644
>--- a/source3/include/nt_printing.h
>+++ b/source3/include/nt_printing.h
>@@ -132,6 +132,11 @@ bool print_access_check(const struct auth_serversupplied_info *server_info,
> 			struct messaging_context *msg_ctx, int snum,
> 			int access_type);
> 
>+WERROR nt_printer_guid_get(TALLOC_CTX *mem_ctx,
>+			   const struct auth_serversupplied_info *server_info,
>+			   struct messaging_context *msg_ctx,
>+			   const char *printer, struct GUID *guid);
>+
> WERROR nt_printer_publish(TALLOC_CTX *mem_ctx,
> 			  const struct auth_serversupplied_info *server_info,
> 			  struct messaging_context *msg_ctx,
>@@ -141,8 +146,8 @@ WERROR nt_printer_publish(TALLOC_CTX *mem_ctx,
> bool is_printer_published(TALLOC_CTX *mem_ctx,
> 			  const struct auth_serversupplied_info *server_info,
> 			  struct messaging_context *msg_ctx,
>-			  const char *servername, const char *printer,
>-			  struct GUID *guid,
>+			  const char *servername,
>+			  const char *printer,
> 			  struct spoolss_PrinterInfo2 **info2);
> 
> WERROR check_published_printers(struct messaging_context *msg_ctx);
>diff --git a/source3/printing/nt_printing_ads.c b/source3/printing/nt_printing_ads.c
>index 9aab88b..bf309b0 100644
>--- a/source3/printing/nt_printing_ads.c
>+++ b/source3/printing/nt_printing_ads.c
>@@ -87,6 +87,80 @@ done:
> 	talloc_free(tmp_ctx);
> }
> 
>+WERROR nt_printer_guid_get(TALLOC_CTX *mem_ctx,
>+			   const struct auth_serversupplied_info *session_info,
>+			   struct messaging_context *msg_ctx,
>+			   const char *printer, struct GUID *guid)
>+{
>+	TALLOC_CTX *tmp_ctx;
>+	enum winreg_Type type;
>+	DATA_BLOB blob;
>+	uint32_t len;
>+	NTSTATUS status;
>+	WERROR result;
>+
>+	tmp_ctx = talloc_new(mem_ctx);
>+	if (tmp_ctx == NULL) {
>+		DEBUG(0, ("out of memory?!\n"));
>+		return WERR_NOMEM;
>+	}
>+
>+	result = winreg_get_printer_dataex_internal(tmp_ctx, session_info,
>+						    msg_ctx, printer,
>+						    SPOOL_DSSPOOLER_KEY,
>+						    "objectGUID",
>+						    &type,
>+						    &blob.data,
>+						    &len);
>+	if (!W_ERROR_IS_OK(result)) {
>+		DEBUG(0, ("Failed to get GUID for printer %s\n", printer));
>+		goto out_ctx_free;
>+	}
>+	blob.length = (size_t)len;
>+
>+	/* We used to store the guid as REG_BINARY, then swapped
>+	   to REG_SZ for Vista compatibility so check for both */
>+
>+	switch (type) {
>+	case REG_SZ: {
>+		bool ok;
>+		const char *guid_str;
>+		ok = pull_reg_sz(tmp_ctx, &blob, &guid_str);
>+		if (!ok) {
>+			DEBUG(0, ("Failed to unmarshall GUID for printer %s\n",
>+				  printer));
>+			result = WERR_REG_CORRUPT;
>+			goto out_ctx_free;
>+		}
>+		status = GUID_from_string(guid_str, guid);
>+		if (!NT_STATUS_IS_OK(status)) {
>+			DEBUG(0, ("bad GUID for printer %s\n", printer));
>+			result = ntstatus_to_werror(status);
>+			goto out_ctx_free;
>+		}
>+		break;
>+	}
>+	case REG_BINARY:
>+		if (blob.length != sizeof(struct GUID)) {
>+			DEBUG(0, ("bad GUID for printer %s\n", printer));
>+			result = WERR_REG_CORRUPT;
>+			goto out_ctx_free;
>+		}
>+		memcpy(guid, blob.data, sizeof(struct GUID));
>+		break;
>+	default:
>+		DEBUG(0,("GUID value stored as invalid type (%d)\n", type));
>+		result = WERR_REG_CORRUPT;
>+		goto out_ctx_free;
>+		break;
>+	}
>+	result = WERR_OK;
>+
>+out_ctx_free:
>+	talloc_free(tmp_ctx);
>+	return result;
>+}
>+
> static WERROR nt_printer_info_to_mods(TALLOC_CTX *ctx,
> 				      struct spoolss_PrinterInfo2 *info2,
> 				      ADS_MODLIST *mods)
>@@ -479,16 +553,12 @@ done:
> bool is_printer_published(TALLOC_CTX *mem_ctx,
> 			  const struct auth_serversupplied_info *session_info,
> 			  struct messaging_context *msg_ctx,
>-			  const char *servername, const char *printer,
>-			  struct GUID *guid,
>+			  const char *servername,
>+			  const char *printer,
> 			  struct spoolss_PrinterInfo2 **info2)
> {
> 	struct spoolss_PrinterInfo2 *pinfo2 = NULL;
>-	enum winreg_Type type;
>-	uint8_t *data;
>-	uint32_t data_size;
> 	WERROR result;
>-	NTSTATUS status;
> 	struct dcerpc_binding_handle *b;
> 
> 	result = winreg_printer_binding_handle(mem_ctx,
>@@ -510,47 +580,6 @@ bool is_printer_published(TALLOC_CTX *mem_ctx,
> 		return false;
> 	}
> 
>-	if (!guid) {
>-		goto done;
>-	}
>-
>-	/* fetching printer guids really ought to be a separate function. */
>-
>-	result = winreg_get_printer_dataex(mem_ctx, b,
>-					   printer,
>-					   SPOOL_DSSPOOLER_KEY, "objectGUID",
>-					   &type, &data, &data_size);
>-	if (!W_ERROR_IS_OK(result)) {
>-		TALLOC_FREE(pinfo2);
>-		return false;
>-	}
>-
>-	/* We used to store the guid as REG_BINARY, then swapped
>-	   to REG_SZ for Vista compatibility so check for both */
>-
>-	switch (type) {
>-	case REG_SZ:
>-		status = GUID_from_string((char *)data, guid);
>-		if (!NT_STATUS_IS_OK(status)) {
>-			TALLOC_FREE(pinfo2);
>-			return false;
>-		}
>-		break;
>-
>-	case REG_BINARY:
>-		if (data_size != sizeof(struct GUID)) {
>-			TALLOC_FREE(pinfo2);
>-			return false;
>-		}
>-		memcpy(guid, data, sizeof(struct GUID));
>-		break;
>-	default:
>-		DEBUG(0,("is_printer_published: GUID value stored as "
>-			 "invaluid type (%d)\n", type));
>-		break;
>-	}
>-
>-done:
> 	if (info2) {
> 		*info2 = talloc_move(mem_ctx, &pinfo2);
> 	}
>@@ -558,6 +587,14 @@ done:
> 	return true;
> }
> #else
>+WERROR nt_printer_guid_get(TALLOC_CTX *mem_ctx,
>+			   const struct auth_serversupplied_info *session_info,
>+			   struct messaging_context *msg_ctx,
>+			   const char *printer, struct GUID *guid)
>+{
>+	return WERR_NOT_SUPPORTED;
>+}
>+
> WERROR nt_printer_publish(TALLOC_CTX *mem_ctx,
> 			  const struct auth_serversupplied_info *session_info,
> 			  struct messaging_context *msg_ctx,
>@@ -575,8 +612,8 @@ WERROR check_published_printers(struct messaging_context *msg_ctx)
> bool is_printer_published(TALLOC_CTX *mem_ctx,
> 			  const struct auth_serversupplied_info *session_info,
> 			  struct messaging_context *msg_ctx,
>-			  const char *servername, const char *printer,
>-			  struct GUID *guid,
>+			  const char *servername,
>+			  const char *printer,
> 			  struct spoolss_PrinterInfo2 **info2)
> {
> 	return False;
>diff --git a/source3/rpc_server/spoolss/srv_spoolss_nt.c b/source3/rpc_server/spoolss/srv_spoolss_nt.c
>index c511fd0..0f74a44 100644
>--- a/source3/rpc_server/spoolss/srv_spoolss_nt.c
>+++ b/source3/rpc_server/spoolss/srv_spoolss_nt.c
>@@ -4176,29 +4176,52 @@ static WERROR construct_printer_info7(TALLOC_CTX *mem_ctx,
> 				      int snum)
> {
> 	struct auth_serversupplied_info *session_info;
>-	struct GUID guid;
>+	char *printer;
> 	NTSTATUS status;
>+	WERROR werr;
>+	TALLOC_CTX *tmp_ctx = talloc_new(mem_ctx);
>+	if (tmp_ctx == NULL) {
>+		return WERR_NOMEM;
>+	}
> 
>-	status = make_session_info_system(mem_ctx, &session_info);
>+	status = make_session_info_system(tmp_ctx, &session_info);
> 	if (!NT_STATUS_IS_OK(status)) {
> 		DEBUG(0, ("construct_printer_info7: "
> 			  "Could not create system session_info\n"));
>-		return WERR_NOMEM;
>+		werr = WERR_NOMEM;
>+		goto out_tmp_free;
> 	}
> 
>-	if (is_printer_published(mem_ctx, session_info, msg_ctx,
>-				 servername,
>-				 lp_servicename(snum), &guid, NULL)) {
>+	printer = lp_servicename(snum);
>+	if (printer == NULL) {
>+		DEBUG(0, ("invalid printer snum %d\n", snum));
>+		werr = WERR_INVALID_PARAM;
>+		goto out_tmp_free;
>+	}
>+
>+	if (is_printer_published(tmp_ctx, session_info, msg_ctx,
>+				 servername, printer, NULL)) {
>+		struct GUID guid;
>+		werr = nt_printer_guid_get(tmp_ctx, session_info, msg_ctx,
>+					   printer, &guid);
>+		if (!W_ERROR_IS_OK(werr)) {
>+			goto out_tmp_free;
>+		}
> 		r->guid = talloc_strdup_upper(mem_ctx, GUID_string2(mem_ctx, &guid));
> 		r->action = DSPRINT_PUBLISH;
> 	} else {
> 		r->guid = talloc_strdup(mem_ctx, "");
> 		r->action = DSPRINT_UNPUBLISH;
> 	}
>-	W_ERROR_HAVE_NO_MEMORY(r->guid);
>+	if (r->guid == NULL) {
>+		werr = WERR_NOMEM;
>+		goto out_tmp_free;
>+	}
> 
>-	TALLOC_FREE(session_info);
>-	return WERR_OK;
>+	werr = WERR_OK;
>+out_tmp_free:
>+	talloc_free(tmp_ctx);
>+	return werr;
> }
> 
> /********************************************************************
>diff --git a/source3/smbd/server_reload.c b/source3/smbd/server_reload.c
>index 9e28a72..c4c5a8d 100644
>--- a/source3/smbd/server_reload.c
>+++ b/source3/smbd/server_reload.c
>@@ -122,8 +122,9 @@ void reload_printers_full(struct tevent_context *ev,
> 			struct spoolss_PrinterInfo2 *pinfo2 = NULL;
> 			if (is_printer_published(session_info, session_info,
> 						 msg_ctx,
>-						 NULL, lp_servicename(snum),
>-						 NULL, &pinfo2)) {
>+						 NULL,
>+						 lp_servicename(snum),
>+						 &pinfo2)) {
> 				nt_printer_publish(session_info,
> 						   session_info,
> 						   msg_ctx,
>-- 
>1.8.1.4
>

Flags:

asn: review+
ddiss: review? (gd)

Actions: View

Attachments on bug 9900: 8915 | 8916 | 8917 | 8918