allow ntlmv2 ntlmssp authentication
Jeff Layton
jlayton at samba.org
Wed Jul 7 11:35:01 MDT 2010
On Wed, 7 Jul 2010 11:52:16 -0500
Shirish Pargaonkar <shirishpargaonkar at gmail.com> wrote:
> On Wed, Jul 7, 2010 at 6:25 AM, Jeff Layton <jlayton at samba.org> wrote:
> > On Fri, 2 Jul 2010 22:16:03 -0500
> > shirishpargaonkar at gmail.com wrote:
> >
> >> Have kept ntlmv1 ntlmssp as a default, ntlmv2 ntlmssp can
> >> be made default only by making code change.
> >>
> >>
> >> From 3d8a8960a6d164e2bacd2a4fc96456453042e049 Mon Sep 17 00:00:00 2001
> >> From: Shirish Pargaonkar <shirishpargaonkar at gmail.com>
> >> Date: Fri, 2 Jul 2010 21:54:51 -0500
> >> Subject: [PATCH] add ntlvm2 ntlmssp authentication
> >>
> >> Signed-off-by: Shirish Pargaonkar <shirishpargaonkar at gmail.com>
> >> ---
> >> fs/cifs/cifsencrypt.c | 48 ++++++++++++++++++++++++++++++++++++++++++++----
> >> fs/cifs/cifsglob.h | 2 ++
> >> fs/cifs/cifspdu.h | 1 -
> >> fs/cifs/sess.c | 39 +++++++++++++++++++++++++++++++++++++--
> >> 4 files changed, 83 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/fs/cifs/cifsencrypt.c b/fs/cifs/cifsencrypt.c
> >> index 847628d..ebfafec 100644
> >> --- a/fs/cifs/cifsencrypt.c
> >> +++ b/fs/cifs/cifsencrypt.c
> >> @@ -378,6 +378,44 @@ calc_exit_2:
> >> return rc;
> >> }
> >>
> >> +static void
> >> +find_domain_name(struct cifsSesInfo *ses)
> >> +{
> >> + unsigned int attrsize;
> >> + unsigned int type;
> >> + unsigned char *blobptr;
> >> + struct ntlmssp2_name *attrptr;
> >> +
> >> + if (ses->server->tiblob) {
> >> + blobptr = ses->server->tiblob;
> >> + attrptr = (struct ntlmssp2_name *) blobptr;
> >> +
> >> + while ((type = attrptr->type) != 0) {
> >> + blobptr += 2; /* advance attr type */
> >> + attrsize = attrptr->length;
> >> + blobptr += 2; /* advance attr size */
> >> + if (type == 0x2) {
> > ^^^
> > Magic numbers like this should be turned into names.
>
> Yes, will make the change.
>
> >
> >> + if (!ses->domainName) {
> >> + ses->domainName =
> >> + kmalloc(attrptr->length + 1,
> >> + GFP_KERNEL);
> >> + if (!ses->domainName)
> >> + return;
> >> + cifs_from_ucs2(ses->domainName,
> >> + (__le16 *)blobptr,
> >> + attrptr->length,
> >> + attrptr->length,
> >> + load_nls_default(), false);
> >> + }
> >> + }
> >> + blobptr += attrsize; /* advance attr value */
> >> + attrptr = (struct ntlmssp2_name *) blobptr;
> >> + }
> >> + }
> >> +
> >> + return;
> >> +}
> >> +
> >
> > Is it possible that you'll need to pick other UCS2 fields than the
> > domain out of a NTLMSSP blob? If so, it might be better to turn the
> > above function into a "copy the field of type X out of the blob"
> > function.
>
> I have not seen enough responses where a server does not
> return a domain name field in the AV pairs it sends in type 2
> message of NTLMSSP protocol.
> NTLM v2 protocol is supposed to use server name in case of
> servers of with local accounts. I will have to do some code
> changes and testing to determine whether authentication
> succeeds or not using server name instead of domain name
> returned in type 2 challenge packet.
>
Either way, I think it would be better to make this a generic routine
for pulling a field of type X out of the blob. I imagine other fields
in there will eventually be useful and making a toolkit for this sort
of thing will make it easier to expand on your work.
> >
> >> void setup_ntlmv2_rsp(struct cifsSesInfo *ses, char *resp_buf,
> >> const struct nls_table *nls_cp)
> >> {
> >> @@ -390,10 +428,9 @@ void setup_ntlmv2_rsp(struct cifsSesInfo *ses, char *resp_buf,
> >> buf->time = cpu_to_le64(cifs_UnixTimeToNT(CURRENT_TIME));
> >> get_random_bytes(&buf->client_chal, sizeof(buf->client_chal));
> >> buf->reserved2 = 0;
> >> - buf->names[0].type = cpu_to_le16(NTLMSSP_DOMAIN_TYPE);
> >> - buf->names[0].length = 0;
> >> - buf->names[1].type = 0;
> >> - buf->names[1].length = 0;
> >> +
> >> + if (!ses->domainName)
> >> + find_domain_name(ses);
> >>
> >> /* calculate buf->ntlmv2_hash */
> >> rc = calc_ntlmv2_hash(ses, nls_cp);
> >> @@ -421,6 +458,9 @@ void CalcNTLMv2_response(const struct cifsSesInfo *ses,
> >>
> >> hmac_md5_update(v2_session_response+8,
> >> sizeof(struct ntlmv2_resp) - 8, &context);
> >> + if (ses->server->tilen)
> >> + hmac_md5_update(ses->server->tiblob,
> >> + ses->server->tilen, &context);
> >>
> >> hmac_md5_final(v2_session_response, &context);
> >> /* cifs_dump_mem("v2_sess_rsp: ", v2_session_response, 32); */
> >> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
> >> index 3c55e10..db6c5da 100644
> >> --- a/fs/cifs/cifsglob.h
> >> +++ b/fs/cifs/cifsglob.h
> >> @@ -188,6 +188,8 @@ struct TCP_Server_Info {
> >> unsigned long lstrp; /* when we got last response from this server */
> >> u16 dialect; /* dialect index that server chose */
> >> /* extended security flavors that server supports */
> >> + unsigned int tilen; /* length of the target info blob */
> >> + unsigned char *tiblob; /* target info blob in challenge response */
> >
> > All of this is for NTLMSSP session setup, correct? If so, is the
> > TCP_Server_Info really the right place for this? Will this break if I
> > have more than one session on the same socket?
>
> I think it will not break authentication but will break signing (and
> perhaps sealing).
> What would be a good place to hang is per session, per user information?
>
Per session sounds right to me. IIUC, the first session on the socket
sets the signing key.
> >> @@ -451,10 +466,12 @@ static int build_ntlmssp_auth_blob(unsigned char *pbuffer,
> >> struct cifsSesInfo *ses,
> >> const struct nls_table *nls_cp, bool first)
> >> {
> >> + unsigned int size;
> >> AUTHENTICATE_MESSAGE *sec_blob = (AUTHENTICATE_MESSAGE *)pbuffer;
> >> __u32 flags;
> >> unsigned char *tmp;
> >> char ntlm_session_key[CIFS_SESS_KEY_SIZE];
> >> + struct ntlmv2_resp ntlmv2_response = {};
> >>
> >> memcpy(sec_blob->Signature, NTLMSSP_SIGNATURE, 8);
> >> sec_blob->MessageType = NtLmAuthenticate;
> >> @@ -477,6 +494,7 @@ static int build_ntlmssp_auth_blob(unsigned char *pbuffer,
> >> sec_blob->LmChallengeResponse.Length = 0;
> >> sec_blob->LmChallengeResponse.MaximumLength = 0;
> >>
> >> +#if 1
> > ^^^^^^^
> > Why are you adding code that's always compiled out?
>
> Right now we are using ntlmv1 as a default authentication mechanism in NTLMSSP.
> I am not sure whether we can just switch to ntlmv2 as an
> authentication mechanism.
> I think in case of Windows servers, an LmCompatibilityLevel registry
> setting of 0 through 5
> will accept NTLMv2 response but not sure how it is handled by other
> servers i.e. are
> there servers out there who will/can't handle ntlmv2 authentication
> mechanism within
> NTLMSSP.
>
Ok, just to be clear -- NAK on this patch as it stands now. Please do
not add code to the kernel that's just commented out. It's confusing
and leads to maintenance headaches.
Either this is ready for merge, in which case you should just replace
the old code or it's not.
--
Jeff Layton <jlayton at samba.org>
More information about the samba-technical
mailing list