Bug 5434 - mount.cifs does not use password from credentials file
mount.cifs does not use password from credentials file
Product: CifsVFS
Classification: Unclassified
Component: user space tools
All Linux
: P3 normal
: ---
Assigned To: Jeremy Allison
Samba QA Contact
Depends on:
  Show dependency treegraph
Reported: 2008-04-30 09:25 UTC by Ted Percival
Modified: 2008-05-08 10:54 UTC (History)
1 user (show)

See Also:

Patch as mentioned in comment #2 (1.36 KB, patch)
2008-05-01 06:47 UTC, Ted Percival
no flags Details
Patch (743 bytes, patch)
2008-05-01 10:38 UTC, Jeremy Allison
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Ted Percival 2008-04-30 09:25:31 UTC
mount.cifs fails to use the password from a credentials file, but if the same password is used on the command-line it works fine:

> # mount /store
> mount error 13 = Permission denied
> Refer to the mount.cifs(8) manual page (e.g.man mount.cifs)
(returns 255)

> # mount -opassword=mypassword /store
> mount.cifs warning - password specified twice
(succeeds, returns 0)

The failed mount prints this to dmesg:
> Status code returned 0xc000006d NT_STATUS_LOGON_FAILURE
>  CIFS VFS: Send error in SessSetup = -13
>  CIFS VFS: cifs_mount failed w/return code = -13

Using Debian Samba packages with version 1:3.2.0~pre3-1 on powerpc and x86.

I have checked the credentials file and there are no spaces in it, just
> username=myuser
> username=mypass
the same worked fine with Debian samba packages with version 1:3.0.28a-2.

I'm guessing this is probably fixed by
46614ca... Revert "mount.cifs: Zero mountpassword content before freeing."
on v3-2-test but I haven't had the chance to build with that patch yet.
Comment 1 Ted Percival 2008-04-30 09:33:54 UTC
Relying on /etc/fstab to provide the credentials option:

//erwin.network.midg3t.net/store  /store  cifs  uid=ted,gid=ted,credentials=/home/ted/.smbcreds-erwin  0  0
Comment 2 Ted Percival 2008-05-01 04:11:53 UTC
The problem is that the '\n' is maintained at the end of the password. It was introduced by cc23f91d639db61903bf8b6c9fa46ec9c1f44178.

Changing the 3rd argument of strlcpy from MOUNT_PASSWD_SIZE+1 to length+1 should do the trick (that's what the username processing code does), but the intention to remove the '\n' is more obvious if you set
  temp_val[length] = '\0';
before breaking out of the for loop 13 lines above (the one that determines the password's length).

The next block of code that processes the domain looks similarly buggy.
Comment 3 Ted Percival 2008-05-01 04:44:56 UTC
Uh, I misquoted my credentials file in comment #0. The second line actually starts with "password=" like it should.
Comment 4 Ted Percival 2008-05-01 06:47:56 UTC
Created attachment 3272 [details]
Patch as mentioned in comment #2

This does the trick.
Comment 5 Jeremy Allison 2008-05-01 10:38:22 UTC
Created attachment 3273 [details]

Great catch thanks ! Sorry for messing this up. I like this patch better as it more correctly expresses the intent, plus I want to make sure all strlcpy calls use the correct physical buffer size to ensure I don't mess these up. Can you test this patch and confirm it still works for you ?
Comment 6 Ted Percival 2008-05-01 19:56:26 UTC
Tested attachment #3273 [details], it fixes the problem. Thanks Jeremy.
Comment 7 Mikel Ward 2008-05-07 21:36:28 UTC
Just got this on Fedora 9 preview, which uses 3.2.0-1.pre3.

What version of Samba does change cc23f91d639db61903bf8b6c9fa46ec9c1f44178 correspond to?
Comment 8 Mikel Ward 2008-05-07 21:50:11 UTC
Ah, I found it.

So the change was made on 24 April 2008, whatever version that was.

Fedora 8 and Ubuntu 8.4 are both using Samba 3.0.x, so I guess they're unaffected.
Comment 9 Ted Percival 2008-05-07 22:11:34 UTC
(In reply to comment #8)
> So the change was made on 24 April 2008, whatever version that was.

It was made shortly before 3.2.0-pre3 - that's the first release it was in.
Comment 10 Mikel Ward 2008-05-07 22:41:44 UTC
Looks like the fix was checked in on May 1st.

Can this now be closed?
Comment 11 Jeremy Allison 2008-05-08 10:54:43 UTC
Yes, this bug is now fixed. Closing.