Re: [PATCH] [CIFS] [1/3 -v3] NTLM auth and sign - Allocate session key/client response dynamically

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, 25 Oct 2010 11:26:53 -0500
Shirish Pargaonkar <shirishpargaonkar@xxxxxxxxx> wrote:

> On Mon, Oct 25, 2010 at 11:14 AM, Jeff Layton <jlayton@xxxxxxxxx> wrote:
> > On Thu, 21 Oct 2010 06:42:55 -0500
> > shirishpargaonkar@xxxxxxxxx wrote:
> >
> >> From: Shirish Pargaonkar <shirishpargaonkar@xxxxxxxxx>
> >>
> >>
> >> Start calculating auth response within a session.  Move/Add pertinet
> >> data structures like session key, server challenge and ntlmv2_hash in
> >> a session structure.  We should do the calculations within a session
> >> before copying session key and response over to server data
> >> structures because a session setup can fail.
> >>
> >> Only after a very first smb session succeeds, it copy/make its
> >> session key, session key of smb connection.  This key stays with
> >> the smb connection throughout its life.
> >> sequence_number within server is set to 0x2.
> >>
> >> The authentication Message Authentication Key (mak) which consists
> >> of session key followed by client response within structure session_key
> >> is now dynamic.  Every authentication type allocates the key + response
> >> sized memory within its session structure and later either assigns or
> >> frees it once the client response is sent and if session's session key
> >> becomes connetion's session key.
> >>
> >> ntlm/ntlmi authentication functions are rearranged.  A function
> >> named setup_ntlm_resp(), similar to setup_ntlmv2_resp(), replaces
> >> function cifs_calculate_session_key().
> >>
> >> size of CIFS_SESS_KEY_SIZE is changed to 16, to reflect the byte size
> >> of the key it holds.
> >>
> >>
> >> Signed-off-by: Shirish Pargaonkar <shirishpargaonkar@xxxxxxxxx>
> >> ---
> >>  fs/cifs/cifsencrypt.c |  113 ++++++++++++++++++++++++++++---------------------
> >>  fs/cifs/cifsglob.h    |    9 +----
> >>  fs/cifs/cifspdu.h     |    7 +++-
> >>  fs/cifs/cifsproto.h   |   11 ++---
> >>  fs/cifs/connect.c     |   18 +++++++-
> >>  fs/cifs/sess.c        |  107 +++++++++++++++++-----------------------------
> >>  fs/cifs/transport.c   |    6 +-
> >>  7 files changed, 134 insertions(+), 137 deletions(-)
> >>
> >
> > [...]
> >
> >> @@ -3178,10 +3185,11 @@ int cifs_setup_session(unsigned int xid, struct cifsSesInfo *ses,
> >>       } else {
> >>               mutex_lock(&ses->server->srv_mutex);
> >>               if (!server->session_estab) {
> >> -                     memcpy(&server->session_key.data,
> >> -                             &ses->auth_key.data, ses->auth_key.len);
> >> +                     server->session_key.response = ses->auth_key.response;
> >>                       server->session_key.len = ses->auth_key.len;
> >> -                     ses->server->session_estab = true;
> >> +                     server->sequence_number = 0x2;
> >
> > It looks like the sequence_number is protected by the Global_MidLock.
> > It's not held here. Are there potential races with setting this value
> > here?
> >
> 
> Jeff, I do not think so.  This setting is happening only during the very first
> smb session setup for that smb connection. Rest of the smb sessions
> and subsequent smb traffic going on on that smb connection for all the
> smb sessions ensues only after this step i.e. for signature calculations.
> So there is no resetting sequence_number by doing this.
> If signing is not enabled, this is not even an issue at all.
> 

I'm not sure I follow...

The Global_MidLock is taken in the other places where this counter is
incremented. Why is it OK to set it here outside of that lock?

Isn't it possible for two session setups to proceed in parallel? If
not, what serializes them?

-- 
Jeff Layton <jlayton@xxxxxxxxx>
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux