Bug 14128 - username/password authentication doesn't work with CUPS and smbspool
Summary: username/password authentication doesn't work with CUPS and smbspool
Status: RESOLVED FIXED
Alias: None
Product: Samba 4.1 and newer
Classification: Unclassified
Component: Printing (show other bugs)
Version: 4.11.0rc4
Hardware: All Linux
: P5 regression (vote)
Target Milestone: ---
Assignee: Karolin Seeger
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-09-16 19:29 UTC by Bryan Mason
Modified: 2019-09-21 23:44 UTC (History)
4 users (show)

See Also:


Attachments
Proposed patch (1.16 KB, patch)
2019-09-17 00:51 UTC, Bryan Mason
no flags Details
Updated patch with additional comments (1.29 KB, patch)
2019-09-17 16:16 UTC, Bryan Mason
asn: review+
gd: review+
ab: review+
Details
patch for 4.10 (1.58 KB, patch)
2019-09-18 12:55 UTC, Andreas Schneider
asn: review? (ab)
gd: review+
Details
patch for 4.11 (1.58 KB, patch)
2019-09-18 12:55 UTC, Andreas Schneider
asn: review? (ab)
gd: review+
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Bryan Mason 2019-09-16 19:29:25 UTC
Description of problem:

  smbspool is unable to authenticate to Windows print server using
  username/password passed through DEVICE_URI environment variable.

How reproducible:

  100%

Steps to Reproduce:

 1. Configure CUPS print queue with a username password combination:

    lpadmin -p smbtest -v 'smb://user:password@host/printer' -E
      
 2. Try to print to the printer

    lp -d smbtest /etc/fstab

Actual results:

  The print job fails with the following messages in the CUPS error_log:

    Started backend /usr/lib/cups/backend/smb (PID 14556)
    get_exit_code(nt_status=NT_STATUS_INVALID_ACCOUNT_NAME [c0000062])
    ATTR: auth-info-required=username,password
    PID 14556 (/usr/lib/cups/backend/smb) stopped with status 2.
    Unable to connect to CIFS host: NT_STATUS_INVALID_ACCOUNT_NAME
 
Expected results:

  File should print

Additional info:

  When smbspool is run directly from the command line, i.e.:

    smbspool 'smb://user:password@host/printer' 1 user test 1 "" /etc/fstab

  then the file is printed correctly.  The problem is:

  * CUPS provides the Device URI (which should contain the
    username/password) through argv[0] (yes, I know that's weird) and
    through the DEVICE_URI environment variable.

  * The Device URI passed through argv[0] is "sanitized" so that the
    username and password is removed.  This is most likely done so that
    the username/password isn't visible in the process information,
    which could be easily accessed by ps or similar commands.

  * smbspool searches argv[0], argv[1], and DEVICE_URI for the Device
    URI, but if it finds something in argv[0], it won't use the
    information in DEVICE_URI.

  As a result, smbspool uses the sanitized Device URI in argv[0] instead
  of the Device URI in DEVICE_URI that has the login credentials.  Since
  the sanitized URI doesn't have the credentials, print job transfer to
  Windows fails.

  The proposed patch changes smbspool to primarily use DEVICE_URI and
  only use argv[0] or argv[1] if DEVICE_URI is nonexistent or empty.
  I've done some quick tests to verify that the patch does re-enable
  CUPS printing to a Windows server using username/password authentication.

  As an aside, CUPS never passes the Device URI through argv[1], but
  this mechanism is _very_ useful for testing purposes (like in the
  smbspool command above).

  This worked prior to Commit 69d7a496, which swizzled the URI logic
  around a bit.  Commit 69d7a496 made the URI handling much cleaner,
  but introduced this regression.

  Proposed patch to be attached later today.
Comment 1 Bryan Mason 2019-09-17 00:51:33 UTC
Created attachment 15470 [details]
Proposed patch
Comment 2 Andreas Schneider 2019-09-17 06:58:12 UTC
Hi Bryan,

thank you very much for the detailed analysis. The patch looks also fine, but could you please add those details with a comment to the code? I think this is important information and should be there so we understand it if we revisit the code.

Thanks!
Comment 3 Bryan Mason 2019-09-17 16:16:14 UTC
Created attachment 15472 [details]
Updated patch with additional comments

Thanks!

Attached is an updated patch with the additional comments.
Comment 4 Alexander Bokovoy 2019-09-18 09:01:48 UTC
Comment on attachment 15472 [details]
Updated patch with additional comments

LGTM
Comment 5 Andreas Schneider 2019-09-18 12:55:25 UTC
Created attachment 15473 [details]
patch for 4.10
Comment 6 Andreas Schneider 2019-09-18 12:55:57 UTC
Created attachment 15474 [details]
patch for 4.11
Comment 7 Guenther Deschner 2019-09-19 01:00:47 UTC
Comment on attachment 15472 [details]
Updated patch with additional comments

LGTM
Comment 8 Guenther Deschner 2019-09-19 01:36:50 UTC
Karolin, please add to 4.10 and 4.11
Comment 9 Karolin Seeger 2019-09-20 09:48:59 UTC
(In reply to Guenther Deschner from comment #8)
Pushed to autobuild-v4-{11,10}-test.
Comment 10 Karolin Seeger 2019-09-21 23:44:41 UTC
Pushed to both branches.
Closing out bug report.

Thanks!