smbclient 3.5 can't connect to a Windows 7 server using NTLMv2 where smbclient 3.3, 3.4 and 3.6 can

[Andrew Bartlett]

On Thu, 2012-08-23 at 15:07 -0700, Jeremy Allison wrote:
> On Thu, Aug 23, 2012 at 06:10:00PM +0000, Blohm, Guntram (I/FP-37, extern) wrote:
> > Found the bug, though it took me a while.
> > 
> > In samba version 3.4, source3/libsmb/smbencrypt.c says
> > 
> >         if (!ntv2_owf_gen(nt_hash, user, domain, False, ntlm_v2_hash)) {
> > 
> > the False tells ntv2_owf_gen not to uppercase the domain name.
> > 
> > Samba version 3.5 moves the file to libcli/auth/smbencrypt.c and changes that line to
> > 
> >         if (!ntv2_owf_gen(nt_hash, user, domain, true, ntlm_v2_hash)) {
> > 
> > and version 3.6 doesn't change anything there.
> > 
> > 
> > When the domain/workgroup name is fetched from the command line (in source3/libsmb/cliconnect.c), 3.4 has:
> > 
> >         if ((p=strchr_m(user2,'\\')) || (p=strchr_m(user2,'/')) ||
> >             (p=strchr_m(user2,*lp_winbind_separator()))) {
> >                 *p = 0;
> >                 user = p+1;
> >                 workgroup = user2;
> >         }
> > 
> > which does not change in 3.5, but 3.6 uses
> > 
> >         if ((p=strchr_m(user2,'\\')) || (p=strchr_m(user2,'/')) ||
> >             (p=strchr_m(user2,*lp_winbind_separator()))) {
> >                 *p = 0;
> >                 user = p+1;
> >                 strupper_m(user2);
> >                 workgroup = user2;
> >         }
> > 
> > So basically the problem is: version 3.4 (and below) passes a lowercase version (to be exact: same case that was given on the command line) of the workgroup name to the encryption subsystem, and also uses this lowercase version in the plain text part of the NTLMSSP message. 3.5 uses an uppercase workgroup name for encryption, but still passes the lowercase version of the workgroup in the plain text part, causing the NTLMv2 authentication to fail (at least against Win7 as server). 3.6 always uppercases the workgroup name, so NTLMv2 authentication works again.
> > 
> > I'd propose to copy the
> > 
> >                 strupper_m(user2);
> > 
> > line from 3.6 to 3.5, or are there any reasons not to do this? In the meanwhile, the workaround could be using all uppercase domain names on the command line (which is what I'm doing right now).
> 
> Wow - great catch and analysis !
> 
> I think your fix will work for 3.5.x, but I think a more
> correct fix would be to change:
> 
> if (!ntv2_owf_gen(nt_hash, user, domain, true, ntlm_v2_hash)) {
> 
> to :
> 
> if (!ntv2_owf_gen(nt_hash, user, domain, false, ntlm_v2_hash)) {
> 
> in the SMBNTLMv2encrypt_hash() function inside libcli/auth/smbencrypt.c
> for 3.6.next and master.
> 
> SMBNTLMv2encrypt_hash() really shouldn't be messing with the
> case of the domain name passed in, as being passed in from
> ntlmssp3_client_challenge() in source3/libsmb/ntlmssp.c,
> which is constructing the NTLMSSP blobs from the
> struct ntlmssp_state which *doesn't* uppercase the
> passed in domain name.
> 
> In fact, in master - doing a grep for "upper" in any
> of the *ntlmssp*.c files shows no upper-casing of any
> strings whatsoever.
> 
> So we're lucky it's working in 3.6.x and master, due
> to the fact we've already uppercased the domain name
> before we pass it into the ntlmssp code. But if there's
> a code path where it doesn't, then we still have a bug.
> 
> I'd like to remove the "bool upper_case_domain" parameter
> from the ntv2_owf_gen() but it's used inside ntlm_password_check()
> (via smb_pwd_check_ntlmv2()) which tries both lower case
> and then upper case versions of the doamin name (if the
> passed in domain wasn't already upper case).
> 
> So my preferred patches for 3.6.next and master and
> 3.5.next are attached.
> 
> If you can confirm them against 3.5.x I'll raise a bug
> and get these fixes merged.

Thanks for looking at this Jeremy. I agree, we should remove the 'upper'
parameter.  Just move the upper-casing to ntlm_check.c:366 and keep our
crypto code to crypto.  That this was done at this layer was when it
made sense to upper case the UCS2 version of the string, which we don't
do any more.

Andrew Bartlett

-- 
Andrew Bartlett                                http://samba.org/~abartlet/
Authentication Developer, Samba Team           http://samba.org