Bug 9900 - GetPrinter(level = 7) always returns DSPRINT_UNPUBLISH
GetPrinter(level = 7) always returns DSPRINT_UNPUBLISH
Status: RESOLVED FIXED
Product: Samba 3.6
Classification: Unclassified
Component: Printing
3.6.15
All All
: P5 normal
: ---
Assigned To: David Disseldorp
Samba QA Contact
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2013-05-22 14:32 UTC by David Disseldorp
Modified: 2015-10-29 09:41 UTC (History)
3 users (show)

See Also:


Attachments
proposed fix for master (9.37 KB, patch)
2013-05-23 17:57 UTC, David Disseldorp
no flags Details
proposed fix for master v2, pointer cast removed (9.40 KB, patch)
2013-05-24 09:22 UTC, David Disseldorp
asn: review+
Details
proposed fix for master v2, for 4.0 branch (9.40 KB, patch)
2013-05-24 10:03 UTC, David Disseldorp
asn: review+
ddiss: review? (gd)
Details
proposed fix v2, for 3.6 branch (12.16 KB, patch)
2013-05-24 10:06 UTC, David Disseldorp
asn: review+
ddiss: review? (gd)
Details

Note You need to log in before you can comment on or make changes to this bug.
Description David Disseldorp 2013-05-22 14:32:32 UTC
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.

MS-RPRN states:
==
2.2.1.10.8 PRINTER_INFO_7
...
DSPRINT_PUBLISH
0x00000001
RpcGetPrinter: The server MUST set this value to indicate the printer is
published in the DS.
...
DSPRINT_UNPUBLISH
0x00000004
RpcGetPrinter: The server MUST set this value to indicate the printer is
not published.
==

This appears to be due to erroneous "objectGUID" unmarshalling in is_printer_published(), and is present in Samba >= 3.6.
Comment 1 David Disseldorp 2013-05-23 16:15:07 UTC
This bug has huge implications for printer publishing, as the DC queries the print server for publishing status (via level=7 GetPrinter) every eight hours and purges the AD PrintQueue object if the response does not match what the server expects (DSPRINT_PUBLISH with matching GUID).
Comment 2 David Disseldorp 2013-05-23 17:57:34 UTC
Created attachment 8915 [details]
proposed fix for master

smbtorture test still to come.
Comment 3 Jeremy Allison 2013-05-23 21:50:58 UTC
David - NAK on this patch.

This code:

+	result = winreg_get_printer_dataex_internal(tmp_ctx, session_info,
+						    msg_ctx, printer,
+						    SPOOL_DSSPOOLER_KEY,
+						    "objectGUID",
+						    &type,
+						    &blob.data,
+						    (uint32_t *)&blob.length);

is a problem. You can never just cast (uint32_t *)&blob.length, you must use a temporary variable of type uint32_t, and then assign to blob.length on successful result. blob.length is of type size_t, which may not match a uint32_t.

Maybe winreg_get_printer_dataex_internal() should return an explicit DATA_BLOB instead.

Jeremy.
Comment 4 David Disseldorp 2013-05-24 07:56:33 UTC
(In reply to comment #3)
> David - NAK on this patch.
> 
> This code:
> 
> +    result = winreg_get_printer_dataex_internal(tmp_ctx, session_info,
> +                            msg_ctx, printer,
> +                            SPOOL_DSSPOOLER_KEY,
> +                            "objectGUID",
> +                            &type,
> +                            &blob.data,
> +                            (uint32_t *)&blob.length);
> 
> is a problem. You can never just cast (uint32_t *)&blob.length, you must use a
> temporary variable of type uint32_t, and then assign to blob.length on
> successful result. blob.length is of type size_t, which may not match a
> uint32_t.

Thanks for the review Jeremy, updated patch to come...
Comment 5 David Disseldorp 2013-05-24 09:22:48 UTC
Created attachment 8916 [details]
proposed fix for master v2, pointer cast removed
Comment 6 David Disseldorp 2013-05-24 10:03:35 UTC
Created attachment 8917 [details]
proposed fix for master v2, for 4.0 branch
Comment 7 David Disseldorp 2013-05-24 10:04:05 UTC
(In reply to comment #6)
> Created attachment 8917 [details]
> proposed fix for master v2, for 4.0 branch

Should have mentioned, this is the same as the master change.
Comment 8 David Disseldorp 2013-05-24 10:06:56 UTC
Created attachment 8918 [details]
proposed fix v2, for 3.6 branch
Comment 9 David Disseldorp 2013-05-29 12:39:34 UTC
Comment on attachment 8916 [details]
proposed fix for master v2, pointer cast removed

This change set has been sent to the samba-technical list, alongside smbtorture test code.
Comment 10 Andreas Schneider 2013-06-03 11:02:03 UTC
Comment on attachment 8918 [details]
proposed fix v2, for 3.6 branch

Looks good to me.
Comment 11 David Disseldorp 2013-06-03 14:57:13 UTC
Karolin, please merge for 3.6-next and 4.0-next.
Comment 12 Karolin Seeger 2013-06-04 07:51:35 UTC
Pushed to v3-6-test and autobuild-v4-0-test.
Comment 13 Karolin Seeger 2013-06-04 10:01:41 UTC
Pushed to v4-0-test.
Closing out bug report.

Thanks!
Comment 14 Nec 2014-10-28 14:52:35 UTC
Hello,

In our six CentOS 6.5 servers, we have encountered the same issue.

The log files are showing :

Oct 28 15:37:03 smbd[10649]: [2014/10/28 15:37:03.002774,  0] printing/nt_printing_ads.c:116(nt_printer_guid_get)
Oct 28 15:37:03 smbd[10649]:   Failed to get GUID for printer GS1-ETAGE1-GROUP

On these servers, we are ruuning a yum-cron daemon that raised the versions along the months. We closely followed the versions and found the bug appeared at the same time it was described here (3.6.15).

The first server hit by this bug was downgraded towards 3.6.9, and obviously, the bug disapeared.

On the other servers, we stopped the automatic upgrades and they then stayed in 3.6.9.

But one of the server was still free to upgrade, and its present state is 3.6.23-12.0.1.el6, and it is still hit by this bug, though it ought to be corrected since.

As our only workaround is to downgrade, but as this is no acceptable option, what is you advice, what should we try?

Best regards,

-- 
Nicolas Ecarnot
Comment 15 Karolin Seeger 2014-11-04 19:59:05 UTC
David, can you comment, please?
Comment 16 David Disseldorp 2014-11-10 10:34:39 UTC
(In reply to Nec from comment #14)

Hi Nicolas,

The fix for this bug was first released with upstream Samba 3.6.16. If you believe that you're hitting this bug in later versions, then please provide Samba logs (captured at log level = 10) and a network trace that includes the GetPrinter(level = 7) request/response.
Comment 17 Nec 2014-11-21 14:51:26 UTC
David,

This is not a belief, and I carefully followed what was done along the versions.
On most of our servers, I had to downgrade to 3.6.9 were this bug does not appear.
I kept a last server untouched in 3.6.23, hoping someone would ask for my log files.
But recently, I was forced to downgrade due to user pressure.

So at present, I can not show you evidence of this bug, but just tell that :
- around 600 users on 8 servers saw this bug
- another very skilled sysadmin (contractor for us) discovered and witnessed this bug, in 3.6._23_ version.

May I ask you to change the status of this bug into something else than solved.
I'm really determined in helping you help me, by installing another 3.6.23 VM and reproducing the same environnement, providing the logs and testing the patchs.

-- 
Nicolas Ecarnot
Comment 18 David Disseldorp 2014-12-18 11:13:32 UTC
Okay, I think the GUID retrieval error should be tracked as a new bug....

I discussed this with Andreas:

11:57 <ddiss_> hmm, okay so that means is_printer_published() is returning true, but there's no corresponding GUID in the registry, which should have been added when it was published
11:57 <ddiss_> we could fall back to ads_search_dn() if it's not found
...
12:08 <gladiac> but I guess there is no GUID in the registry
12:08 <gladiac> it is a local cups printer
12:08 <ddiss_> the publish code path doesn't return an error if it fails to pull the GUID from the AD DC...
12:09 <ddiss_> so the error will only be noticed on retrieval
Comment 19 Nec 2014-12-18 11:36:19 UTC
David,

I'd be glad to open a specific BZ for this issue to be solved.
But I'm not sure I'm skilled enough to correctly describe what is wrong and what should work and how.
I guess you would do a better job.

Wouldn't you mind open it, please?
Comment 20 David Disseldorp 2014-12-18 11:41:55 UTC
(In reply to Nec from comment #19)

Andreas has raised it as https://bugzilla.samba.org/show_bug.cgi?id=11018
Comment 21 Nec 2015-10-29 09:41:52 UTC
For the record :
I confirm that in 3.6.23-20.0.1, this bug is still present, and the only workaround (kindly suggested by Colin Simpson - THANK YOU SO MUCH) is to :
- stop the services
- export the registry
- change the 14th bit of the Attribute key from 1 to 0, responsible for the checkbox "Publish in Active directory"
- save, import, restart

I can't wait to see the 3.6.24 been published in the CentOS repos, as I may have understood this was fixed in this version.