[linux-cifs-client][patch] Make NTLMv2 as auth mech withing NTLMSSP and enable signing using crypto shash APIs

Jeff Layton jlayton at samba.org
Sat Aug 21 17:38:17 MDT 2010


On Sat, 21 Aug 2010 09:23:11 -0500
Shirish Pargaonkar <shirishpargaonkar at gmail.com> wrote:

> On Sat, Aug 21, 2010 at 6:14 AM, Jeff Layton <jlayton at samba.org> wrote:
> > On Wed,  4 Aug 2010 21:34:39 -0500
> > shirishpargaonkar at gmail.com wrote:
> >
> >> Make ntlmv2 as an authentication mechanism within ntlmssp
> >> instead of ntlmv1.
> >> Parse type 2 response in ntlmssp negotiation to pluck
> >> AV pairs and use them to calculate ntlmv2 response token.
> >> Also, assign domain name from the sever response in type 2
> >> packet of ntlmssp and use that (netbios) domain name in
> >> calculation of response.
> >>
> >> Enable cifs/smb signing using rc4 and md5.
> >>
> >> Changed name of the structure mac_key to session_key to reflect
> >> the type of key it holds.
> >>
> >> Use kernel crypto_shash_* APIs instead of the equivalent cifs functions.
> >>
> >>
> >> From 6ab552fd60804f3c708e1745ca936112fc9f9821 Mon Sep 17 00:00:00 2001
> >> From: Shirish Pargaonkar <shirishpargaonkar at gmail.com>
> >> Date: Wed, 4 Aug 2010 17:24:07 -0500
> >> Subject: [PATCH] Make ntlmv2 as auth mech and enable signing using crypto_shash APIs
> >>
> >> Signed-off-by: Shirish Pargaonkar <shirishpargaonkar at gmail.com>
> >> ---
> >>  fs/cifs/asn1.c        |    6 +-
> >>  fs/cifs/cifsencrypt.c |  416 ++++++++++++++++++++++++++++++++++--------------
> >>  fs/cifs/cifsglob.h    |   18 ++-
> >>  fs/cifs/cifspdu.h     |    7 +-
> >>  fs/cifs/cifsproto.h   |   12 +-
> >>  fs/cifs/cifssmb.c     |   13 +-
> >>  fs/cifs/connect.c     |   13 ++-
> >>  fs/cifs/ntlmssp.h     |   13 ++
> >>  fs/cifs/sess.c        |  118 +++++++++++----
> >>  fs/cifs/transport.c   |    6 +-
> >>  10 files changed, 450 insertions(+), 172 deletions(-)
> >>
> >
> > Overall this looks like a good change, but this patch is rather large.
> > That makes it tough to bisect for bugs and to review...
> >
> > Would it be possible to break it up some? Maybe a patch (or patches) to
> > convert the existing code to using the crypto api's and then another
> > set to fix up the NTLMSSP code?
> >
> > Also, with the conversion to the crypto API's can we remove some of the
> > cifs private crypto routines in smbencrypt.c? That would be a nice
> > follow-on patch. It seems like switching to use the crypto API's should
> > mean a net reduction in code.
> >
> >> diff --git a/fs/cifs/asn1.c b/fs/cifs/asn1.c
> >> index cfd1ce3..21f0fbd 100644
> >> --- a/fs/cifs/asn1.c
> >> +++ b/fs/cifs/asn1.c
> >> @@ -597,13 +597,13 @@ decode_negTokenInit(unsigned char *security_blob, int length,
> >>                               if (compare_oid(oid, oidlen, MSKRB5_OID,
> >>                                               MSKRB5_OID_LEN))
> >>                                       server->sec_mskerberos = true;
> >> -                             else if (compare_oid(oid, oidlen, KRB5U2U_OID,
> >> +                             if (compare_oid(oid, oidlen, KRB5U2U_OID,
> >>                                                    KRB5U2U_OID_LEN))
> >>                                       server->sec_kerberosu2u = true;
> >> -                             else if (compare_oid(oid, oidlen, KRB5_OID,
> >> +                             if (compare_oid(oid, oidlen, KRB5_OID,
> >>                                                    KRB5_OID_LEN))
> >>                                       server->sec_kerberos = true;
> >> -                             else if (compare_oid(oid, oidlen, NTLMSSP_OID,
> >> +                             if (compare_oid(oid, oidlen, NTLMSSP_OID,
> >>                                                    NTLMSSP_OID_LEN))
> >>                                       server->sec_ntlmssp = true;
> >>
> >
> > Why change the above? If compare_oid returns true on any of them, then
> > the others will return false. The else clauses ought to stay, IMO...
> 
> Jeff, with this the authentication mechanism always defaults to kerberos
> (if server supports it) even if an user wants to use a different auth
> mech this way
> e.g. sec=ntlm/ntlmi or sec=ntlmv2/ntlmv2i.
> 

I don't think we're talking about the same thing here.

The above change doesn't change which of the sec_* flags end up set
when decode_negTokenInit returns. It just means that the function does
more parsing than it needs to.

The change you're talking about is the one in CIFS_SessSetup. That one
looks correct, I think.

-- 
Jeff Layton <jlayton at samba.org>


More information about the samba-technical mailing list