Attachment 8916 Details for Bug 9900 – proposed fix for master v2, pointer cast removed

[patch] proposed fix for master v2, pointer cast removed

bug-9900_master_v2.patch (text/plain), 9.40 KB, created by David Disseldorp on 2013-05-24 09:22:48 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: David Disseldorp

Created: 2013-05-24 09:22:48 UTC

Size: 9.40 KB

patch

obsolete

>From c35caa954b880df43ae3375f1baf3e2eca43a965 Mon Sep 17 00:00:00 2001
>From: David Disseldorp <ddiss@samba.org>
>Date: Thu, 23 May 2013 19:32:08 +0200
>Subject: [PATCH 1/2] 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 3a2baf40..86ea8f7 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 e0e2f328595f771033e949127699f44612f1f170 Mon Sep 17 00:00:00 2001
>From: David Disseldorp <ddiss@samba.org>
>Date: Wed, 22 May 2013 17:58:38 +0200
>Subject: [PATCH 2/2] 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               |   6 +-
> source3/printing/nt_printing_ads.c          | 129 ++++++++++++++++++----------
> source3/rpc_server/spoolss/srv_spoolss_nt.c |  40 +++++++--
> source3/smbd/server_reload.c                |   2 +-
> 4 files changed, 121 insertions(+), 56 deletions(-)
>
>diff --git a/source3/include/nt_printing.h b/source3/include/nt_printing.h
>index 2243a3d..2a0e883 100644
>--- a/source3/include/nt_printing.h
>+++ b/source3/include/nt_printing.h
>@@ -132,6 +132,11 @@ bool print_access_check(const struct auth_session_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_session_info *session_info,
>+			   struct messaging_context *msg_ctx,
>+			   const char *printer, struct GUID *guid);
>+
> WERROR nt_printer_publish(TALLOC_CTX *mem_ctx,
> 			  const struct auth_session_info *server_info,
> 			  struct messaging_context *msg_ctx,
>@@ -143,7 +148,6 @@ bool is_printer_published(TALLOC_CTX *mem_ctx,
> 			  struct messaging_context *msg_ctx,
> 			  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 86ea8f7..dcd31b7 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_session_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)
>@@ -481,15 +555,10 @@ bool is_printer_published(TALLOC_CTX *mem_ctx,
> 			  struct messaging_context *msg_ctx,
> 			  const char *servername,
> 			  const char *printer,
>-			  struct GUID *guid,
> 			  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,
>@@ -511,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);
> 	}
>@@ -559,6 +587,14 @@ done:
> 	return true;
> }
> #else
>+WERROR nt_printer_guid_get(TALLOC_CTX *mem_ctx,
>+			   const struct auth_session_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_session_info *session_info,
> 			  struct messaging_context *msg_ctx,
>@@ -578,7 +614,6 @@ bool is_printer_published(TALLOC_CTX *mem_ctx,
> 			  struct messaging_context *msg_ctx,
> 			  const char *servername,
> 			  const char *printer,
>-			  struct GUID *guid,
> 			  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 7482443..03c966b 100644
>--- a/source3/rpc_server/spoolss/srv_spoolss_nt.c
>+++ b/source3/rpc_server/spoolss/srv_spoolss_nt.c
>@@ -4177,21 +4177,47 @@ static WERROR construct_printer_info7(TALLOC_CTX *mem_ctx,
> 				      struct spoolss_PrinterInfo7 *r,
> 				      int snum)
> {
>-	const struct auth_session_info *session_info = get_session_info_system();
>-	struct GUID guid;
>+	const struct auth_session_info *session_info;
>+	char *printer;
>+	WERROR werr;
>+	TALLOC_CTX *tmp_ctx = talloc_new(mem_ctx);
>+	if (tmp_ctx == NULL) {
>+		return WERR_NOMEM;
>+	}
>+
>+	session_info = get_session_info_system();
>+	SMB_ASSERT(session_info != NULL);
> 
>-	if (is_printer_published(mem_ctx, session_info, msg_ctx,
>-				 servername,
>-				 lp_servicename(talloc_tos(), snum), &guid, NULL)) {
>+	printer = lp_servicename(tmp_ctx, 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;
>+	}
> 
>-	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 3a8f5bb..1d6f9c2 100644
>--- a/source3/smbd/server_reload.c
>+++ b/source3/smbd/server_reload.c
>@@ -106,7 +106,7 @@ void delete_and_reload_printers(struct tevent_context *ev,
> 						 NULL,
> 						 lp_servicename(session_info,
> 								snum),
>-						 NULL, &pinfo2)) {
>+						 &pinfo2)) {
> 				nt_printer_publish(session_info,
> 						   session_info,
> 						   msg_ctx,
>-- 
>1.8.1.4
>

Flags:

asn: review+

Actions: View

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