URI special character substitution buggy

[Val Henson]

Binary package hint: system-config-printer

I'm running freshly upgraded (as of April 9, 2007) edgy with system-config-printer version 0.7.32. When I try to configure and then use an SMB printer with username/password authentication, I get:

E [09/Apr/2007:18:38:48 -0700] [Job 44] Session setup failed: NT_STATUS_LOGON_FA
ILURE
E [09/Apr/2007:18:38:49 -0700] [Job 44] No ticket cache found for userid=1000
E [09/Apr/2007:18:38:49 -0700] [Job 44] Can not get the ticket cache for val
E [09/Apr/2007:18:38:49 -0700] [Job 44] Session setup failed: NT_STATUS_LOGON_FA
ILURE
E [09/Apr/2007:18:38:49 -0700] [Job 44] Tree connect failed (NT_STATUS_ACCESS_DE
NIED)
E [09/Apr/2007:18:38:49 -0700] [Job 44] Unable to connect to CIFS host, will ret
ry in 60 seconds...

Looking in the /etc/cupsys/printers.conf file, I find that my username and password have been mangled by buggy URI special character substitution after using the system-config-printer GUI to set it. It verifies just fine when I click the "Verify" button, but after I save it and open the dialog again (reloading the configuration), the username has been mangled. It should look like:

DeviceURI smb://domain%5Cusername:password@/hostname/queuename

Instead it looks like:

DeviceURI smb://domain%5Dsername:password@/hostname/queuename

Note that "%5Cu" has been replaced by "%5D". "%5C" is the URI encoding of '\'; the Windows stuff here wants "domain\username" as the username. The '\' is replaced by the next ASCII keycode, and the following character is overwritten. The cool bit is that if you repeatedly open up the "Change" dialog next to the Device URI field, the character keeps mutating by 1. Awesome!

The URI coding/decoding done with some throwaway local functions, percentEncode and percentDecode. I replaced it with urllib.quote and urllib.unquote and now the URI looks to be saved correctly. I still can't get authorized for some other reason (bug? misconfiguration?) but at least the configuration file is not obviously wrong anymore. Patch below. Don't forget to add a dependency on urllib.

--- /usr/share/system-config-printer/system-config-printer.py.bak 2007-04-09 18:32:09.000000000 -0700
+++ /usr/share/system-config-printer/system-config-printer.py 2007-04-09 18:35:22.000000000 -0700
@@ -47,7 +47,7 @@
 from cupsd import CupsConfig
 import probe_printer
 import gtk_label_autowrap
-
+import urllib

 domain='system-config-printer'
 import locale
@@ -63,44 +63,6 @@
 busy_cursor = gtk.gdk.Cursor(gtk.gdk.WATCH)
 ready_cursor = gtk.gdk.Cursor(gtk.gdk.LEFT_PTR)

-def percentEncode (text):
- """Percent-encode ASCII text ready for inclusion in a URI."""
- l = len (text)
- i = 0
- while i < l:
- c = text[i]
- a = ord (c)
- if (a <= 0x1f or a == 0x7f or
- c == ' ' or
- '<>#%"'.find (c) != -1 or
- '{}|\^[]`'.find (c) != -1):
- pre = text[:i]
- post = text[i + 1:]
- text = pre + "%" + ("%2X" % a) + post
- i += 2
- l += 2
- i += 1
- return text
-
-def percentDecode (text):
- """Percent-decode URI text to ASCII."""
- l = len (text)
- r = 0
- w = ''
- xdigs = "01234567890abcdef"
- while r < l:
- if r + 2 < l and text[r] == '%':
- c10 = xdigs.find (text[r + 1].lower ())
- if c10 != -1:
- c01 = xdigs.find (text[r + 2].lower ())
- if c01 != -1:
- w += chr (c10 * 0x10 + c01)
- r += 3
- else:
- w += text[r]
- r += 1
- return w
-
 class GUI:

     def __init__(self):
@@ -1874,16 +1836,16 @@
                 host = host[:p]
         share = uri
         return (group, host, share,
- percentDecode (user), percentDecode (password))
+ urllib.unquote (user), urllib.unquote (password))

     def construct_SMBURI (self, group, host, share,
                           user = '', password = ''):
         uri_password = ''
         if password:
- uri_password = ':' + percentEncode (password)
+ uri_password = ':' + urllib.quote (password)
         if user:
             uri_password += '@'
- return "%s%s%s/%s/%s" % (percentEncode (user),
+ return "%s%s%s/%s/%s" % (urllib.quote (user),
                                  uri_password, group, host, share)

     def on_entSMBURI_changed (self, ent):

[Val Henson (val-henson)]

And ironically, I got it working by replacing the "%5C" with a plain '\' in the URI definition. For username "domain\username", this works:

DeviceURI smb://domain\username:password@/hostname/queuename

I'm guessing that the URL quoting should only be done on part of the URI, not the part including the username and password.

[Jani Monoses (jani)]

thanks for the patch! Unfortunatley for edgy we no longer make updates unless they are related security or data loss. I'll let Tim (s-c-printer) author comment on the patch itself, if it is valid for the 0.7.62 version we have now in feisty it surely needs to be included.

[Tim Waugh (twaugh)]

Patch looks good -- applied. Didn't know about urllib, thanks.

Jani: FWIW this was fixed a different way in 0.7.50 and so 0.7.62 does not exhibit the bug.

[Val Henson (val-henson)]

On 4/10/07, Tim Waugh <email address hidden> wrote:
> Patch looks good -- applied. Didn't know about urllib, thanks.

Hey Tim,

Thanks, I didn't know about urllib either until I googled for it. :)

Thanks for taking the patch. I'm not sure which version you are
applying this too, but it still doesn't work for me with this patch.
Once I edited the config file to use '\' instead of "%5C" then it did
work. I think one of the following needs to happen:

1. Username and password need not to be URI-quoted in the first place.
2. Username and password need to be de-URI quoted when read from the
config file.

I can probably write the patch in a couple of minutes if I know which
one makes sense.

-VAL

[Tim Waugh (twaugh)]

Sounds like a samba bug:
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=202977

[Jani Monoses (jani)]

Val,
 Tim applied the patch to CVS head where he develops system-config-printer for Fedora. The patch will only be in the next Ubuntu release (October). The one that is due to be released next week (Feisty) already contains a fix although done differently, as Tim said in a cprevious comment. In Edgy the Ubuntu policty is to only fix crashers or security bugs so you will not see the packages affected unless you upgrade.

[Jani Monoses (jani)]

this is fixed in feisty