Bug 2323 - reply_sesssetup_and_X pulling wrong length password off wire?
Summary: reply_sesssetup_and_X pulling wrong length password off wire?
Status: CLOSED FIXED
Alias: None
Product: Samba 3.0
Classification: Unclassified
Component: Client Tools (show other bugs)
Version: 3.0.10
Hardware: x86 Linux
: P3 critical
Target Milestone: none
Assignee: Samba Bugzilla Account
QA Contact: Samba QA Contact
URL:
Keywords:
: 2322 (view as bug list)
Depends on:
Blocks:
 
Reported: 2005-02-07 11:56 UTC by Thomas Walker
Modified: 2005-08-24 10:24 UTC (History)
1 user (show)

See Also:


Attachments
pcap dump (from windows client) (1.23 KB, application/octet-stream)
2005-02-07 12:03 UTC, Thomas Walker
no flags Details
off by one in sesssetup.c (392 bytes, patch)
2005-02-07 15:02 UTC, Thomas Walker
no flags Details
Proposed patch. (814 bytes, patch)
2005-02-08 18:49 UTC, Jeremy Allison
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Thomas Walker 2005-02-07 11:56:09 UTC
Using samba 3.0.10 on RHEL 3 as server (using unencrypted passwords and PAM
auth) and a Win XP client machine.  On initial attempt to connect, we've been
seeing PAM auth failures that I've traced back to the server seeing password =
username despite that not being the case on the wire (password is NULL as
windows appears to be trying NULL before promtping for a password).

Breakpoint 1, reply_sesssetup_and_X (conn=0x0, inbuf=0xf63a4008 "",
    outbuf=0xf6383008 "", length=242, bufsize=131072) at smbd/sesssetup.c:756
756                             BOOL unic=SVAL(inbuf, smb_flg2) &
FLAGS2_UNICODE_STRINGS;
(gdb) print passlen1
$1 = 1
(gdb) print passlen2
$2 = 0
(gdb) print ra_type
$3 = RA_WIN2K
(gdb) n
758                             if ((ra_type == RA_WINNT) && (passlen2 == 0) &&
unic && passlen1) {
(gdb)
765                                             STR_TERMINATE);
(gdb)
767                             plaintext_password = data_blob(pass,
strlen(pass)+1);
(gdb) print pass
$4 = "walkert\000\000", '&#65533;' <repeats 1015 times>
(gdb) n
770                     p += passlen1 + passlen2;
(gdb) print plaintext_password
$5 = {data = 0x8397a28 "walkert", length = 8,
  free = 0x81fe968 <free_data_blob>}
(gdb) n
771                     p += srvstr_pull_buf(inbuf, user, p, sizeof(user),
STR_TERMINATE);
(gdb) print p
$6 = 0xf63a404a "w"
(gdb) n
772                     p += srvstr_pull_buf(inbuf, domain, p, sizeof(domain),
STR_TERMINATE);
(gdb) print user
$7 = "walkert\000\000", '&#65533;' <repeats 247 times>
(gdb)


Note that it reads the next null terminated string off (an off by one misses the
NULL it is starting on?) but then only advances by 1, thus the next call pulls
off the username and continues on...

Since password is incorrect (Windows didn't even ask yet, entire packet log
attached) but is *not* null, pam_smb cycles through all the various caps/lower
combinations and then fails w/o Windows ever even asking for the password).

Finding user walkert
Trying _Get_Pwnam(), username as lowercase is walkert
Get_Pwnam_internals did find user [walkert]!
checking user=[walkert] pass=[walkert]
pass_check: Checking (PAM) password for user walkert (l=7)
smb_pam_start: PAM: Init user: walkert
smb_pam_start: PAM: setting rhost to: 144.14.18.235
smb_pam_start: PAM: setting tty
smb_pam_start: PAM: Init passed for user: walkert
smb_pam_auth: PAM: Authenticate User: walkert
smb_pam_auth: PAM: Athentication Error for user walkert
smb_pam_error_handler: PAM: Authentication Failure : Authentication failure
smb_pam_passcheck: PAM: smb_pam_auth failed - Rejecting User walkert !
smb_pam_end: PAM: PAM_END OK.
< ... >

If I start an ealier version of the smb server and connect once (causing windows
to cache connection info for that server) and then bring up 3.0.10 it connects
just fine (presumably windows uses the cached info from the previous connection
attempt).  smbclient from unix also appears to do the right thing.

Major problem as we're trying to upgrade to resolve some security problems...

Thanks for looking into it,

Thomas
Comment 1 Thomas Walker 2005-02-07 12:03:18 UTC
Created attachment 954 [details]
pcap dump (from windows client)
Comment 2 Thomas Walker 2005-02-07 15:02:23 UTC
Created attachment 955 [details]
off by one in sesssetup.c

Looks like this fixes an off by one (causes the NULL password attempt to fail
correctly, prompting the windows client to ask for a password which then
validates correctly).  I'm not very familiar with the auth exchange protocol
(started looking at it this morning) but woul appreciate it if someone more
familiar with the code could validate my assumptions here...
Comment 3 Thomas Walker 2005-02-07 15:31:47 UTC
ok, incomplete fix given my limited knowledge of the protocol.  It works for
Windows (XP) and newer (3.x) unix smbclient but apparently not for older (2.x)
Unix smbclient.  Probably another semi-compliant client that needs to be checked
for and dealt with.
I'll be quiet now and let someone who knows what they're doing look at it :)
Comment 4 Gerald (Jerry) Carter (dead mail address) 2005-02-07 15:43:13 UTC
*** Bug 2322 has been marked as a duplicate of this bug. ***
Comment 5 Jeremy Allison 2005-02-08 18:49:52 UTC
Created attachment 958 [details]
Proposed patch.

Thomas Walker wrote :

"I'll be quiet now and let someone who knows what they're doing look at it"

- when it comes to this part of the protocol (plaintext passwords from Windows
clients)  - No one knows what they're doing. Least of all Microsoft :-).

I don't understand your fix. It'll cause a correct unicode string pull of the
user name from the packet into the pass variable. When you say :"causing it to
fail correctly" I'd like to know exactly what you mean.

I think the correct fix is to fix the special case "ansi passlen > 0, unicode
passlen = 0, unicode = true" packet and remove the special case code already
present. I can see how the srvstr_pull is failing (when STR_TERMINATE is set it
ignores the incoming length - no this isn't a security hole in this case - and
will read the next unicode string in the packet).

Can you test this patch instead to see if it fixes all the cases you have ?

Thanks,

Jeremy.
Comment 6 Thomas Walker 2005-02-09 08:26:13 UTC
Ah, didn't even think about unicode (didn't have any clients handy that I saw
using it generally, probably because NULL is NULL in any charset)
Your patch works like a charm (tested with 2.x and 3.x unix smbclient and WinXP)
Comment 7 Jeremy Allison 2005-02-09 16:46:26 UTC
Applied.
Jeremy.
Comment 8 Lars Müller 2005-04-27 08:50:16 UTC
*** Bug 2002 has been marked as a duplicate of this bug. ***
Comment 9 Gerald (Jerry) Carter (dead mail address) 2005-08-24 10:24:00 UTC
sorry for the same, cleaning up the database to prevent unecessary reopens of bugs.