Patchwork Test the password field as well as the username field when looking for a session to reuse.

login
register
mail settings
Submitter Alex Zeffertt
Date April 21, 2010, 3:09 p.m.
Message ID <4BCF1511.7060504@eu.citrix.com>
Download mbox | patch
Permalink /patch/50660/
State New
Headers show

Comments

Alex Zeffertt - April 21, 2010, 3:09 p.m.
Hi all,

I have found a problem with the reusing of existing sessions.  The kernel only 
tests the username but not the password when deciding whether to reuse an 
existing session.  As a result it is possible for mount.cifs to succeed even if 
the password is incorrect, provided that there is an existing session between 
the client and server for that user.

Please could you consider the attached patch which addresses this issue.

Regards,

Alex Zeffertt
Jeff Layton - April 21, 2010, 5:28 p.m.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On Wed, 21 Apr 2010 16:09:05 +0100
Alex Zeffertt <alex.zeffertt@eu.citrix.com> wrote:

> Hi all,
> 
> I have found a problem with the reusing of existing sessions.  The kernel only 
> tests the username but not the password when deciding whether to reuse an 
> existing session.  As a result it is possible for mount.cifs to succeed even if 
> the password is incorrect, provided that there is an existing session between 
> the client and server for that user.
> 
> Please could you consider the attached patch which addresses this issue.
> 
> Regards,
> 
> Alex Zeffertt

Yes, I would consider that a good change.

In fact, I've already incorporated such a change in the auth selection
overhaul patch series that I posted on Friday:

http://lists.samba.org/archive/linux-cifs-client/2010-April/005839.html

I'd probably prefer to do this in the context of that patchset rather
than as a standalone thing, unless anyone has objections.

- -- 
Jeff Layton <jlayton@samba.org>
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.14 (GNU/Linux)

iEYEARECAAYFAkvPNdAACgkQyP0gxQMdzIADGQCfYLTDeA3ecplQN2bl5Kqc4k5q
U58AnjIdQwt+tdRP9ikX+gBV6K6m0QVM
=luxz
-----END PGP SIGNATURE-----
Alex Zeffertt - April 22, 2010, 8:26 a.m.
Jeff Layton wrote:
> Alex Zeffertt <alex.zeffertt@eu.citrix.com> wrote:
> 
>> Hi all,
>>
>> I have found a problem with the reusing of existing sessions.  The kernel only 
>> tests the username but not the password when deciding whether to reuse an 
>> existing session.  As a result it is possible for mount.cifs to succeed even if 
>> the password is incorrect, provided that there is an existing session between 
>> the client and server for that user.
>>
>> Please could you consider the attached patch which addresses this issue.
>>
>> Regards,
>>
>> Alex Zeffertt
> 
> Yes, I would consider that a good change.
> 
> In fact, I've already incorporated such a change in the auth selection
> overhaul patch series that I posted on Friday:
> 
> http://lists.samba.org/archive/linux-cifs-client/2010-April/005839.html
> 
> I'd probably prefer to do this in the context of that patchset rather
> than as a standalone thing, unless anyone has objections.
> 

That's fine by me.  Great minds think alike and all that :-)

Alex

Patch

Test the password field as well as the username field when looking for a session to reuse.

If this is not done then it will be possible to mount a CIFS share using an incorrect
password, provided there is an existing session to the same server with the same user.

Signed-off-by: Alex Zeffertt <alex.zeffertt@eu.citrix.com>

--- ./fs/cifs/connect.c.orig	2010-04-21 15:24:07.000000000 +0100
+++ ./fs/cifs/connect.c	2010-04-21 15:28:19.000000000 +0100
@@ -1587,7 +1587,7 @@ 
 }
 
 static struct cifsSesInfo *
-cifs_find_smb_ses(struct TCP_Server_Info *server, char *username)
+cifs_find_smb_ses(struct TCP_Server_Info *server, char *username, char *password)
 {
 	struct list_head *tmp;
 	struct cifsSesInfo *ses;
@@ -1597,6 +1597,17 @@ 
 		ses = list_entry(tmp, struct cifsSesInfo, smb_ses_list);
 		if (strncmp(ses->userName, username, MAX_USERNAME_SIZE))
 			continue;
+		if (password) {
+			if (!ses->password)
+				continue;
+			if (strcmp(ses->password, password))
+				continue;
+		} else {
+			if (ses->password)
+				continue;
+		}
+			
+		
 
 		++ses->ses_count;
 		write_unlock(&cifs_tcp_ses_lock);
@@ -2356,7 +2367,7 @@ 
 		goto out;
 	}
 
-	pSesInfo = cifs_find_smb_ses(srvTcp, volume_info->username);
+	pSesInfo = cifs_find_smb_ses(srvTcp, volume_info->username, volume_info->password);
 	if (pSesInfo) {
 		cFYI(1, ("Existing smb sess found (status=%d)",
 			pSesInfo->status));