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.

Signed-off-by: David Disseldorp <ddiss@samba.org>
Reviewed-by: Andreas Schneider <asn@samba.org>

4 files changed, 121 insertions, 56 deletions

diff --git a/source3/include/nt_printing.h b/source3/include/nt_printing.h
index 2243a3d095..2a0e8835df 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 86ea8f7a85..dcd31b76ac 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 7482443c74..03c966b615 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 3a8f5bb920..1d6f9c2911 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,