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>

diff --git a/source3/include/nt_printing.h b/source3/include/nt_printing.h
index 2243a3d09555ddc48364454e29c49e2fe553504b..2a0e8835dfb54fe6a0f41b93e9e5f90b596a833f 100644 (file)
--- 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 86ea8f7a8588aa8a953a54bad01a1b05b882ffde..dcd31b76ac4327a27400e14f5ff6fce84c7b955d 100644 (file)
--- 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 7482443c74ec59651ee672f9eeb5dd21b5b83d3a..03c966b615e0e30328e23a9e30172f1ee150c097 100644 (file)
--- 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 3a8f5bb920bf1654d32f3dcdbf8ce43a0487ee2d..1d6f9c2911ff0581fd39d527ab2a6ab5e05764f2 100644 (file)
--- 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,