[PATCH 0/4 v2] Split CIFS_SessSetup()

[PATCH 0/4 v2] Split CIFS_SessSetup()

[Sachin Prabhu]

I am investigating the possibility of adding support for gss-proxy in the
cifs client module. As part of that investigation, I was looking at the
CIFS_SessSetup() function. This is a long function which handles multiple auths
using if conditions and switch statements. The code struction makes it
difficult to modify of add support for new authentication mechanisms.

The proposal is to split the various authentication code into its own separate
functions. This increases the lines of code but will simplify maintenance and
addition of new auth methods.

The short term goal is to add gss-proxy support for kerberos authentication.
This will require to and fro communication with the gss-proxy module over a
unix socket.

Further long term goals are 
1) Add support for NTLMSSP using SPNEGO,
2) Allow clients to negotiate which authentication mechanism to use using SPNEGO.

I would like the opinion of this list about these proposed changes.
Is there any part of the code which needs changing?

V2:
- Ensure that sess_data allocated on intermediate patches are freed.
- Remove ifdef-endif blocks from the switch statement in CIFS_SessSetup().

Sachin Prabhu (4):
  cifs: Split lanman auth from CIFS_SessSetup()
  cifs: Split ntlm and ntlmv2 authentication methods off
    CIFS_SessSetup()
  cifs: Split Kerberos authentication off CIFS_SessSetup()
  cifs: Separate rawntlmssp auth from CIFS_SessSetup()

 fs/cifs/sess.c | 1142 +++++++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 799 insertions(+), 343 deletions(-)

-- 
1.8.4.2

[PATCH 1/4] cifs: Split lanman auth from CIFS_SessSetup()

[Sachin Prabhu]

In preparation for splitting CIFS_SessSetup() into smaller more
manageable chunks, we first add helper functions.

We then proceed to split out lanman auth out of CIFS_SessSetup()

Signed-off-by: Sachin Prabhu <sprabhu-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 fs/cifs/sess.c | 315 +++++++++++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 271 insertions(+), 44 deletions(-)

diff --git a/fs/cifs/sess.c b/fs/cifs/sess.c
index e87387d..6559deb 100644
--- a/fs/cifs/sess.c
+++ b/fs/cifs/sess.c
@@ -520,6 +520,253 @@ select_sectype(struct TCP_Server_Info *server, enum securityEnum requested)
 	}
 }
 
+struct sess_data {
+	unsigned int xid;
+	struct cifs_ses *ses;
+	struct nls_table *nls_cp;
+	void (*func) (struct sess_data *);
+	int result;
+
+	/* we will send the SMB in three pieces:
+	 * a fixed length beginning part, an optional
+	 * SPNEGO blob (which can be zero length), and a
+	 * last part which will include the strings
+	 * and rest of bcc area. This allows us to avoid
+	 * a large buffer 17K allocation
+	 */
+	struct kvec iov[3];
+	int buf0_type;
+};
+
+static int
+sess_alloc_buffer(struct sess_data *sess_data, int wct)
+{
+	int rc;
+	struct cifs_ses *ses = sess_data->ses;
+	struct smb_hdr *smb_buf;
+
+	rc = small_smb_init_no_tc(SMB_COM_SESSION_SETUP_ANDX, wct, ses,
+				  (void **)&smb_buf);
+
+	if (rc)
+		return rc;
+
+	sess_data->iov[0].iov_base = (char *)smb_buf;
+	sess_data->iov[0].iov_len = be32_to_cpu(smb_buf->smb_buf_length) + 4;
+	/*
+ *	  * This variable will be used to clear the buffer
+ *		   * allocated above in case of any error in the calling function.
+ *			    */
+	sess_data->buf0_type = CIFS_SMALL_BUFFER;
+
+	/* 2000 big enough to fit max user, domain, NOS name etc. */
+	sess_data->iov[2].iov_base = kmalloc(2000, GFP_KERNEL);
+	if (!sess_data->iov[2].iov_base) {
+		rc = -ENOMEM;
+		goto out_free_smb_buf;
+	}
+
+	return 0;
+
+out_free_smb_buf:
+	kfree(smb_buf);
+	sess_data->iov[0].iov_base = NULL;
+	sess_data->iov[0].iov_len = 0;
+	sess_data->buf0_type = CIFS_NO_BUFFER;
+	return rc;
+}
+
+static int
+sess_free_buffer(struct sess_data *sess_data)
+{
+
+	if (sess_data->buf0_type == CIFS_SMALL_BUFFER) {
+		cifs_dbg(FYI, "%s: freeing small buf %p\n", __func__,
+					sess_data->iov[0].iov_base);
+		cifs_small_buf_release(sess_data->iov[0].iov_base);
+	} else if (sess_data->buf0_type == CIFS_LARGE_BUFFER) {
+		cifs_dbg(FYI, "%s: freeing small buf %p\n", __func__,
+					sess_data->iov[0].iov_base);
+		cifs_buf_release(sess_data->iov[0].iov_base);
+	}
+
+	kfree(sess_data->iov[2].iov_base);
+
+	return 0;
+}
+
+static int
+sess_establish_session(struct sess_data *sess_data)
+{
+	struct cifs_ses *ses = sess_data->ses;
+
+	if (!sess_data->result) {
+		mutex_lock(&ses->server->srv_mutex);
+		if (!ses->server->session_estab) {
+			if (ses->server->sign) {
+				ses->server->session_key.response =
+					kmemdup(ses->auth_key.response,
+					ses->auth_key.len, GFP_KERNEL);
+				if (!ses->server->session_key.response) {
+					sess_data->result = -ENOMEM;
+					mutex_unlock(&ses->server->srv_mutex);
+					return sess_data->result;
+				}
+				ses->server->session_key.len =
+							ses->auth_key.len;
+			}
+			ses->server->sequence_number = 0x2;
+			ses->server->session_estab = true;
+		}
+		mutex_unlock(&ses->server->srv_mutex);
+
+		cifs_dbg(FYI, "CIFS session established successfully\n");
+		spin_lock(&GlobalMid_Lock);
+		ses->status = CifsGood;
+		ses->need_reconnect = false;
+		spin_unlock(&GlobalMid_Lock);
+	}
+
+	return 0;
+}
+
+static int
+sess_sendreceive(struct sess_data *sess_data)
+{
+	int rc;
+	struct smb_hdr *smb_buf = (struct smb_hdr *) sess_data->iov[0].iov_base;
+	__u16 count;
+
+	count = sess_data->iov[1].iov_len + sess_data->iov[2].iov_len;
+	smb_buf->smb_buf_length =
+		cpu_to_be32(be32_to_cpu(smb_buf->smb_buf_length) + count);
+	put_bcc(count, smb_buf);
+
+	rc = SendReceive2(sess_data->xid, sess_data->ses,
+			  sess_data->iov, 3 /* num_iovecs */,
+			  &sess_data->buf0_type,
+			  CIFS_LOG_ERROR);
+
+	return rc;
+}
+
+/*
+ * LANMAN and plaintext are less secure and off by default.
+ * So we make this explicitly be turned on in kconfig (in the
+ * build) and turned on at runtime (changed from the default)
+ * in proc/fs/cifs or via mount parm.  Unfortunately this is
+ * needed for old Win (e.g. Win95), some obscure NAS and OS/2
+ */
+#ifdef CONFIG_CIFS_WEAK_PW_HASH
+static void
+sess_auth_lanman(struct sess_data *sess_data)
+{
+	int rc = 0;
+	struct smb_hdr *smb_buf;
+	SESSION_SETUP_ANDX *pSMB;
+	char *bcc_ptr;
+	struct cifs_ses *ses = sess_data->ses;
+	char lnm_session_key[CIFS_AUTH_RESP_SIZE];
+	__u32 capabilities;
+	__u16 bytes_remaining;
+
+	/* lanman 2 style sessionsetup */
+	/* wct = 10 */
+	rc = sess_alloc_buffer(sess_data, 10);
+	if (rc)
+		goto out;
+
+	pSMB = (SESSION_SETUP_ANDX *)sess_data->iov[0].iov_base;
+	bcc_ptr = sess_data->iov[2].iov_base;
+	capabilities = cifs_ssetup_hdr(ses, pSMB);
+
+	pSMB->req.hdr.Flags2 &= ~SMBFLG2_UNICODE;
+
+	/* no capabilities flags in old lanman negotiation */
+	pSMB->old_req.PasswordLength = cpu_to_le16(CIFS_AUTH_RESP_SIZE);
+
+	/* Calculate hash with password and copy into bcc_ptr.
+	 * Encryption Key (stored as in cryptkey) gets used if the
+	 * security mode bit in Negottiate Protocol response states
+	 * to use challenge/response method (i.e. Password bit is 1).
+	 */
+	rc = calc_lanman_hash(ses->password, ses->server->cryptkey,
+			      ses->server->sec_mode & SECMODE_PW_ENCRYPT ?
+			      true : false, lnm_session_key);
+
+	memcpy(bcc_ptr, (char *)lnm_session_key, CIFS_AUTH_RESP_SIZE);
+	bcc_ptr += CIFS_AUTH_RESP_SIZE;
+
+	/*
+	 * can not sign if LANMAN negotiated so no need
+	 * to calculate signing key? but what if server
+	 * changed to do higher than lanman dialect and
+	 * we reconnected would we ever calc signing_key?
+	 */
+
+	cifs_dbg(FYI, "Negotiating LANMAN setting up strings\n");
+	/* Unicode not allowed for LANMAN dialects */
+	ascii_ssetup_strings(&bcc_ptr, ses, sess_data->nls_cp);
+
+	sess_data->iov[2].iov_len = (long) bcc_ptr -
+			(long) sess_data->iov[2].iov_base;
+
+	rc = sess_sendreceive(sess_data);
+	if (rc)
+		goto out;
+
+	pSMB = (SESSION_SETUP_ANDX *)sess_data->iov[0].iov_base;
+	smb_buf = (struct smb_hdr *)sess_data->iov[0].iov_base;
+
+	/* lanman response has a word count of 3 */
+	if (smb_buf->WordCount != 3) {
+		rc = -EIO;
+		cifs_dbg(VFS, "bad word count %d\n", smb_buf->WordCount);
+		goto out;
+	}
+
+	if (le16_to_cpu(pSMB->resp.Action) & GUEST_LOGIN)
+		cifs_dbg(FYI, "Guest login\n"); /* BB mark SesInfo struct? */
+
+	ses->Suid = smb_buf->Uid;   /* UID left in wire format (le) */
+	cifs_dbg(FYI, "UID = %llu\n", ses->Suid);
+
+	bytes_remaining = get_bcc(smb_buf);
+	bcc_ptr = pByteArea(smb_buf);
+
+	/* BB check if Unicode and decode strings */
+	if (bytes_remaining == 0) {
+		/* no string area to decode, do nothing */
+	} else if (smb_buf->Flags2 & SMBFLG2_UNICODE) {
+		/* unicode string area must be word-aligned */
+		if (((unsigned long) bcc_ptr - (unsigned long) smb_buf) % 2) {
+			++bcc_ptr;
+			--bytes_remaining;
+		}
+		decode_unicode_ssetup(&bcc_ptr, bytes_remaining, ses,
+				      sess_data->nls_cp);
+	} else {
+		decode_ascii_ssetup(&bcc_ptr, bytes_remaining, ses,
+				    sess_data->nls_cp);
+	}
+
+out:
+	sess_data->result = rc;
+	sess_data->func = NULL;
+	sess_free_buffer(sess_data);
+	sess_establish_session(sess_data);
+}
+
+#else
+
+static void
+sess_auth_lanman(struct sess_data *sess_data)
+{
+	sess_data->result = -EOPNOTSUPP;
+	sess_data->func = NULL;
+}
+#endif
+
 int
 CIFS_SessSetup(const unsigned int xid, struct cifs_ses *ses,
 	       const struct nls_table *nls_cp)
@@ -540,12 +787,20 @@ CIFS_SessSetup(const unsigned int xid, struct cifs_ses *ses,
 	__le32 phase = NtLmNegotiate; /* NTLMSSP, if needed, is multistage */
 	u16 blob_len;
 	char *ntlmsspblob = NULL;
+	struct sess_data *sess_data;
 
 	if (ses == NULL) {
 		WARN(1, "%s: ses == NULL!", __func__);
 		return -EINVAL;
 	}
 
+	sess_data = kzalloc(sizeof(struct sess_data), GFP_KERNEL);
+	if (!sess_data)
+		return -ENOMEM;
+	sess_data->xid = xid;
+	sess_data->ses = ses;
+	sess_data->nls_cp = (struct nls_table *) nls_cp;
+
 	type = select_sectype(ses->server, ses->sectype);
 	cifs_dbg(FYI, "sess setup type %d\n", type);
 	if (type == Unspecified) {
@@ -554,6 +809,15 @@ CIFS_SessSetup(const unsigned int xid, struct cifs_ses *ses,
 		return -EINVAL;
 	}
 
+        switch (type) {
+        case LANMAN:
+                sess_auth_lanman(sess_data);
+		goto out;
+	default:
+		/* Continue with the rest of the function */
+		break;
+	}
+
 	if (type == RawNTLMSSP) {
 		/* if memory allocation is successful, caller of this function
 		 * frees it.
@@ -569,17 +833,7 @@ ssetup_ntlmssp_authenticate:
 	if (phase == NtLmChallenge)
 		phase = NtLmAuthenticate; /* if ntlmssp, now final phase */
 
-	if (type == LANMAN) {
-#ifndef CONFIG_CIFS_WEAK_PW_HASH
-		/* LANMAN and plaintext are less secure and off by default.
-		So we make this explicitly be turned on in kconfig (in the
-		build) and turned on at runtime (changed from the default)
-		in proc/fs/cifs or via mount parm.  Unfortunately this is
-		needed for old Win (e.g. Win95), some obscure NAS and OS/2 */
-		return -EOPNOTSUPP;
-#endif
-		wct = 10; /* lanman 2 style sessionsetup */
-	} else if ((type == NTLM) || (type == NTLMv2)) {
+	if ((type == NTLM) || (type == NTLMv2)) {
 		/* For NTLMv2 failures eventually may need to retry NTLM */
 		wct = 13; /* old style NTLM sessionsetup */
 	} else /* same size: negotiate or auth, NTLMSSP or extended security */
@@ -618,39 +872,7 @@ ssetup_ntlmssp_authenticate:
 	iov[1].iov_base = NULL;
 	iov[1].iov_len = 0;
 
-	if (type == LANMAN) {
-#ifdef CONFIG_CIFS_WEAK_PW_HASH
-		char lnm_session_key[CIFS_AUTH_RESP_SIZE];
-
-		pSMB->req.hdr.Flags2 &= ~SMBFLG2_UNICODE;
-
-		/* no capabilities flags in old lanman negotiation */
-
-		pSMB->old_req.PasswordLength = cpu_to_le16(CIFS_AUTH_RESP_SIZE);
-
-		/* Calculate hash with password and copy into bcc_ptr.
-		 * Encryption Key (stored as in cryptkey) gets used if the
-		 * security mode bit in Negottiate Protocol response states
-		 * to use challenge/response method (i.e. Password bit is 1).
-		 */
-
-		rc = calc_lanman_hash(ses->password, ses->server->cryptkey,
-				 ses->server->sec_mode & SECMODE_PW_ENCRYPT ?
-					true : false, lnm_session_key);
-
-		memcpy(bcc_ptr, (char *)lnm_session_key, CIFS_AUTH_RESP_SIZE);
-		bcc_ptr += CIFS_AUTH_RESP_SIZE;
-
-		/* can not sign if LANMAN negotiated so no need
-		to calculate signing key? but what if server
-		changed to do higher than lanman dialect and
-		we reconnected would we ever calc signing_key? */
-
-		cifs_dbg(FYI, "Negotiating LANMAN setting up strings\n");
-		/* Unicode not allowed for LANMAN dialects */
-		ascii_ssetup_strings(&bcc_ptr, ses, nls_cp);
-#endif
-	} else if (type == NTLM) {
+	if (type == NTLM) {
 		pSMB->req_no_secext.Capabilities = cpu_to_le32(capabilities);
 		pSMB->req_no_secext.CaseInsensitivePasswordLength =
 			cpu_to_le16(CIFS_AUTH_RESP_SIZE);
@@ -962,4 +1184,9 @@ keycp_exit:
 	kfree(ses->ntlmssp);
 
 	return rc;
+
+out:
+	rc = sess_data->result;
+	kfree(sess_data);
+	return rc;
 }
-- 
1.8.4.2

Re: [PATCH 1/4] cifs: Split lanman auth from CIFS_SessSetup()

[Shirish Pargaonkar]

On Thu, May 1, 2014 at 7:41 AM, Sachin Prabhu <sprabhu-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> In preparation for splitting CIFS_SessSetup() into smaller more
> manageable chunks, we first add helper functions.
>
> We then proceed to split out lanman auth out of CIFS_SessSetup()
>
> Signed-off-by: Sachin Prabhu <sprabhu-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
>  fs/cifs/sess.c | 315 +++++++++++++++++++++++++++++++++++++++++++++++++--------
>  1 file changed, 271 insertions(+), 44 deletions(-)
>
> diff --git a/fs/cifs/sess.c b/fs/cifs/sess.c
> index e87387d..6559deb 100644
> --- a/fs/cifs/sess.c
> +++ b/fs/cifs/sess.c
> @@ -520,6 +520,253 @@ select_sectype(struct TCP_Server_Info *server, enum securityEnum requested)
>         }
>  }
>
> +struct sess_data {
> +       unsigned int xid;
> +       struct cifs_ses *ses;
> +       struct nls_table *nls_cp;
> +       void (*func) (struct sess_data *);
> +       int result;
> +
> +       /* we will send the SMB in three pieces:
> +        * a fixed length beginning part, an optional
> +        * SPNEGO blob (which can be zero length), and a
> +        * last part which will include the strings
> +        * and rest of bcc area. This allows us to avoid
> +        * a large buffer 17K allocation
> +        */
> +       struct kvec iov[3];
> +       int buf0_type;
> +};
> +
> +static int
> +sess_alloc_buffer(struct sess_data *sess_data, int wct)
> +{
> +       int rc;
> +       struct cifs_ses *ses = sess_data->ses;
> +       struct smb_hdr *smb_buf;
> +
> +       rc = small_smb_init_no_tc(SMB_COM_SESSION_SETUP_ANDX, wct, ses,
> +                                 (void **)&smb_buf);
> +
> +       if (rc)
> +               return rc;
> +
> +       sess_data->iov[0].iov_base = (char *)smb_buf;
> +       sess_data->iov[0].iov_len = be32_to_cpu(smb_buf->smb_buf_length) + 4;
> +       /*
> + *       * This variable will be used to clear the buffer
> + *                * allocated above in case of any error in the calling function.
> + *                         */
> +       sess_data->buf0_type = CIFS_SMALL_BUFFER;
> +
> +       /* 2000 big enough to fit max user, domain, NOS name etc. */
> +       sess_data->iov[2].iov_base = kmalloc(2000, GFP_KERNEL);
> +       if (!sess_data->iov[2].iov_base) {
> +               rc = -ENOMEM;
> +               goto out_free_smb_buf;
> +       }
> +
> +       return 0;
> +
> +out_free_smb_buf:
> +       kfree(smb_buf);
> +       sess_data->iov[0].iov_base = NULL;
> +       sess_data->iov[0].iov_len = 0;
> +       sess_data->buf0_type = CIFS_NO_BUFFER;
> +       return rc;
> +}
> +
> +static int
> +sess_free_buffer(struct sess_data *sess_data)
> +{
> +
> +       if (sess_data->buf0_type == CIFS_SMALL_BUFFER) {
> +               cifs_dbg(FYI, "%s: freeing small buf %p\n", __func__,
> +                                       sess_data->iov[0].iov_base);
> +               cifs_small_buf_release(sess_data->iov[0].iov_base);
> +       } else if (sess_data->buf0_type == CIFS_LARGE_BUFFER) {
> +               cifs_dbg(FYI, "%s: freeing small buf %p\n", __func__,
> +                                       sess_data->iov[0].iov_base);
> +               cifs_buf_release(sess_data->iov[0].iov_base);
> +       }
> +
> +       kfree(sess_data->iov[2].iov_base);
> +
> +       return 0;
> +}

I do not see anybody allocating CIFS_LARGE_BUFFER type, so
is this check needed for future allocations?

> +
> +static int
> +sess_establish_session(struct sess_data *sess_data)
> +{
> +       struct cifs_ses *ses = sess_data->ses;
> +
> +       if (!sess_data->result) {
> +               mutex_lock(&ses->server->srv_mutex);
> +               if (!ses->server->session_estab) {
> +                       if (ses->server->sign) {
> +                               ses->server->session_key.response =
> +                                       kmemdup(ses->auth_key.response,
> +                                       ses->auth_key.len, GFP_KERNEL);
> +                               if (!ses->server->session_key.response) {
> +                                       sess_data->result = -ENOMEM;
> +                                       mutex_unlock(&ses->server->srv_mutex);
> +                                       return sess_data->result;
> +                               }
> +                               ses->server->session_key.len =
> +                                                       ses->auth_key.len;
> +                       }
> +                       ses->server->sequence_number = 0x2;
> +                       ses->server->session_estab = true;
> +               }
> +               mutex_unlock(&ses->server->srv_mutex);
> +
> +               cifs_dbg(FYI, "CIFS session established successfully\n");
> +               spin_lock(&GlobalMid_Lock);
> +               ses->status = CifsGood;
> +               ses->need_reconnect = false;
> +               spin_unlock(&GlobalMid_Lock);
> +       }
> +
> +       return 0;
> +}
> +

Should sess_establish_session() return void? Nobody checks the
return value of this function.

> +static int
> +sess_sendreceive(struct sess_data *sess_data)
> +{
> +       int rc;
> +       struct smb_hdr *smb_buf = (struct smb_hdr *) sess_data->iov[0].iov_base;
> +       __u16 count;
> +
> +       count = sess_data->iov[1].iov_len + sess_data->iov[2].iov_len;
> +       smb_buf->smb_buf_length =
> +               cpu_to_be32(be32_to_cpu(smb_buf->smb_buf_length) + count);
> +       put_bcc(count, smb_buf);
> +
> +       rc = SendReceive2(sess_data->xid, sess_data->ses,
> +                         sess_data->iov, 3 /* num_iovecs */,
> +                         &sess_data->buf0_type,
> +                         CIFS_LOG_ERROR);
> +
> +       return rc;
> +}
> +
> +/*
> + * LANMAN and plaintext are less secure and off by default.
> + * So we make this explicitly be turned on in kconfig (in the
> + * build) and turned on at runtime (changed from the default)
> + * in proc/fs/cifs or via mount parm.  Unfortunately this is
> + * needed for old Win (e.g. Win95), some obscure NAS and OS/2
> + */
> +#ifdef CONFIG_CIFS_WEAK_PW_HASH
> +static void
> +sess_auth_lanman(struct sess_data *sess_data)
> +{
> +       int rc = 0;
> +       struct smb_hdr *smb_buf;
> +       SESSION_SETUP_ANDX *pSMB;
> +       char *bcc_ptr;
> +       struct cifs_ses *ses = sess_data->ses;
> +       char lnm_session_key[CIFS_AUTH_RESP_SIZE];
> +       __u32 capabilities;
> +       __u16 bytes_remaining;
> +
> +       /* lanman 2 style sessionsetup */
> +       /* wct = 10 */
> +       rc = sess_alloc_buffer(sess_data, 10);
> +       if (rc)
> +               goto out;
> +
> +       pSMB = (SESSION_SETUP_ANDX *)sess_data->iov[0].iov_base;
> +       bcc_ptr = sess_data->iov[2].iov_base;
> +       capabilities = cifs_ssetup_hdr(ses, pSMB);
> +
> +       pSMB->req.hdr.Flags2 &= ~SMBFLG2_UNICODE;
> +
> +       /* no capabilities flags in old lanman negotiation */
> +       pSMB->old_req.PasswordLength = cpu_to_le16(CIFS_AUTH_RESP_SIZE);
> +
> +       /* Calculate hash with password and copy into bcc_ptr.
> +        * Encryption Key (stored as in cryptkey) gets used if the
> +        * security mode bit in Negottiate Protocol response states
> +        * to use challenge/response method (i.e. Password bit is 1).
> +        */
> +       rc = calc_lanman_hash(ses->password, ses->server->cryptkey,
> +                             ses->server->sec_mode & SECMODE_PW_ENCRYPT ?
> +                             true : false, lnm_session_key);
> +
> +       memcpy(bcc_ptr, (char *)lnm_session_key, CIFS_AUTH_RESP_SIZE);
> +       bcc_ptr += CIFS_AUTH_RESP_SIZE;
> +
> +       /*
> +        * can not sign if LANMAN negotiated so no need
> +        * to calculate signing key? but what if server
> +        * changed to do higher than lanman dialect and
> +        * we reconnected would we ever calc signing_key?
> +        */
> +
> +       cifs_dbg(FYI, "Negotiating LANMAN setting up strings\n");
> +       /* Unicode not allowed for LANMAN dialects */
> +       ascii_ssetup_strings(&bcc_ptr, ses, sess_data->nls_cp);
> +
> +       sess_data->iov[2].iov_len = (long) bcc_ptr -
> +                       (long) sess_data->iov[2].iov_base;
> +
> +       rc = sess_sendreceive(sess_data);
> +       if (rc)
> +               goto out;
> +
> +       pSMB = (SESSION_SETUP_ANDX *)sess_data->iov[0].iov_base;
> +       smb_buf = (struct smb_hdr *)sess_data->iov[0].iov_base;
> +
> +       /* lanman response has a word count of 3 */
> +       if (smb_buf->WordCount != 3) {
> +               rc = -EIO;
> +               cifs_dbg(VFS, "bad word count %d\n", smb_buf->WordCount);
> +               goto out;
> +       }
> +
> +       if (le16_to_cpu(pSMB->resp.Action) & GUEST_LOGIN)
> +               cifs_dbg(FYI, "Guest login\n"); /* BB mark SesInfo struct? */
> +
> +       ses->Suid = smb_buf->Uid;   /* UID left in wire format (le) */
> +       cifs_dbg(FYI, "UID = %llu\n", ses->Suid);
> +
> +       bytes_remaining = get_bcc(smb_buf);
> +       bcc_ptr = pByteArea(smb_buf);
> +
> +       /* BB check if Unicode and decode strings */
> +       if (bytes_remaining == 0) {
> +               /* no string area to decode, do nothing */
> +       } else if (smb_buf->Flags2 & SMBFLG2_UNICODE) {
> +               /* unicode string area must be word-aligned */
> +               if (((unsigned long) bcc_ptr - (unsigned long) smb_buf) % 2) {
> +                       ++bcc_ptr;
> +                       --bytes_remaining;
> +               }
> +               decode_unicode_ssetup(&bcc_ptr, bytes_remaining, ses,
> +                                     sess_data->nls_cp);
> +       } else {
> +               decode_ascii_ssetup(&bcc_ptr, bytes_remaining, ses,
> +                                   sess_data->nls_cp);
> +       }
> +
> +out:
> +       sess_data->result = rc;
> +       sess_data->func = NULL;
> +       sess_free_buffer(sess_data);
> +       sess_establish_session(sess_data);
> +}
> +
> +#else
> +
> +static void
> +sess_auth_lanman(struct sess_data *sess_data)
> +{
> +       sess_data->result = -EOPNOTSUPP;
> +       sess_data->func = NULL;
> +}
> +#endif
> +
>  int
>  CIFS_SessSetup(const unsigned int xid, struct cifs_ses *ses,
>                const struct nls_table *nls_cp)
> @@ -540,12 +787,20 @@ CIFS_SessSetup(const unsigned int xid, struct cifs_ses *ses,
>         __le32 phase = NtLmNegotiate; /* NTLMSSP, if needed, is multistage */
>         u16 blob_len;
>         char *ntlmsspblob = NULL;
> +       struct sess_data *sess_data;
>
>         if (ses == NULL) {
>                 WARN(1, "%s: ses == NULL!", __func__);
>                 return -EINVAL;
>         }
>
> +       sess_data = kzalloc(sizeof(struct sess_data), GFP_KERNEL);
> +       if (!sess_data)
> +               return -ENOMEM;
> +       sess_data->xid = xid;
> +       sess_data->ses = ses;
> +       sess_data->nls_cp = (struct nls_table *) nls_cp;
> +
>         type = select_sectype(ses->server, ses->sectype);
>         cifs_dbg(FYI, "sess setup type %d\n", type);
>         if (type == Unspecified) {
> @@ -554,6 +809,15 @@ CIFS_SessSetup(const unsigned int xid, struct cifs_ses *ses,
>                 return -EINVAL;
>         }
>
> +        switch (type) {
> +        case LANMAN:
> +                sess_auth_lanman(sess_data);
> +               goto out;
> +       default:
> +               /* Continue with the rest of the function */
> +               break;
> +       }
> +
>         if (type == RawNTLMSSP) {
>                 /* if memory allocation is successful, caller of this function
>                  * frees it.
> @@ -569,17 +833,7 @@ ssetup_ntlmssp_authenticate:
>         if (phase == NtLmChallenge)
>                 phase = NtLmAuthenticate; /* if ntlmssp, now final phase */
>
> -       if (type == LANMAN) {
> -#ifndef CONFIG_CIFS_WEAK_PW_HASH
> -               /* LANMAN and plaintext are less secure and off by default.
> -               So we make this explicitly be turned on in kconfig (in the
> -               build) and turned on at runtime (changed from the default)
> -               in proc/fs/cifs or via mount parm.  Unfortunately this is
> -               needed for old Win (e.g. Win95), some obscure NAS and OS/2 */
> -               return -EOPNOTSUPP;
> -#endif
> -               wct = 10; /* lanman 2 style sessionsetup */
> -       } else if ((type == NTLM) || (type == NTLMv2)) {
> +       if ((type == NTLM) || (type == NTLMv2)) {
>                 /* For NTLMv2 failures eventually may need to retry NTLM */
>                 wct = 13; /* old style NTLM sessionsetup */
>         } else /* same size: negotiate or auth, NTLMSSP or extended security */
> @@ -618,39 +872,7 @@ ssetup_ntlmssp_authenticate:
>         iov[1].iov_base = NULL;
>         iov[1].iov_len = 0;
>
> -       if (type == LANMAN) {
> -#ifdef CONFIG_CIFS_WEAK_PW_HASH
> -               char lnm_session_key[CIFS_AUTH_RESP_SIZE];
> -
> -               pSMB->req.hdr.Flags2 &= ~SMBFLG2_UNICODE;
> -
> -               /* no capabilities flags in old lanman negotiation */
> -
> -               pSMB->old_req.PasswordLength = cpu_to_le16(CIFS_AUTH_RESP_SIZE);
> -
> -               /* Calculate hash with password and copy into bcc_ptr.
> -                * Encryption Key (stored as in cryptkey) gets used if the
> -                * security mode bit in Negottiate Protocol response states
> -                * to use challenge/response method (i.e. Password bit is 1).
> -                */
> -
> -               rc = calc_lanman_hash(ses->password, ses->server->cryptkey,
> -                                ses->server->sec_mode & SECMODE_PW_ENCRYPT ?
> -                                       true : false, lnm_session_key);
> -
> -               memcpy(bcc_ptr, (char *)lnm_session_key, CIFS_AUTH_RESP_SIZE);
> -               bcc_ptr += CIFS_AUTH_RESP_SIZE;
> -
> -               /* can not sign if LANMAN negotiated so no need
> -               to calculate signing key? but what if server
> -               changed to do higher than lanman dialect and
> -               we reconnected would we ever calc signing_key? */
> -
> -               cifs_dbg(FYI, "Negotiating LANMAN setting up strings\n");
> -               /* Unicode not allowed for LANMAN dialects */
> -               ascii_ssetup_strings(&bcc_ptr, ses, nls_cp);
> -#endif
> -       } else if (type == NTLM) {
> +       if (type == NTLM) {
>                 pSMB->req_no_secext.Capabilities = cpu_to_le32(capabilities);
>                 pSMB->req_no_secext.CaseInsensitivePasswordLength =
>                         cpu_to_le16(CIFS_AUTH_RESP_SIZE);
> @@ -962,4 +1184,9 @@ keycp_exit:
>         kfree(ses->ntlmssp);
>
>         return rc;
> +
> +out:
> +       rc = sess_data->result;
> +       kfree(sess_data);
> +       return rc;
>  }
> --
> 1.8.4.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/4] cifs: Split ntlm and ntlmv2 authentication methods off CIFS_SessSetup()

[Sachin Prabhu]

Signed-off-by: Sachin Prabhu <sprabhu-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 fs/cifs/sess.c | 318 ++++++++++++++++++++++++++++++++++++++++++---------------
 1 file changed, 233 insertions(+), 85 deletions(-)

diff --git a/fs/cifs/sess.c b/fs/cifs/sess.c
index 6559deb..661b67d 100644
--- a/fs/cifs/sess.c
+++ b/fs/cifs/sess.c
@@ -767,6 +767,216 @@ sess_auth_lanman(struct sess_data *sess_data)
 }
 #endif
 
+static void
+sess_auth_ntlm(struct sess_data *sess_data)
+{
+	int rc = 0;
+	struct smb_hdr *smb_buf;
+	SESSION_SETUP_ANDX *pSMB;
+	char *bcc_ptr;
+	struct cifs_ses *ses = sess_data->ses;
+	__u32 capabilities;
+	__u16 bytes_remaining;
+
+	/* old style NTLM sessionsetup */
+	/* wct = 13 */
+	rc = sess_alloc_buffer(sess_data, 13);
+	if (rc)
+		goto out;
+
+	pSMB = (SESSION_SETUP_ANDX *)sess_data->iov[0].iov_base;
+	bcc_ptr = sess_data->iov[2].iov_base;
+	capabilities = cifs_ssetup_hdr(ses, pSMB);
+
+	pSMB->req_no_secext.Capabilities = cpu_to_le32(capabilities);
+	pSMB->req_no_secext.CaseInsensitivePasswordLength =
+			cpu_to_le16(CIFS_AUTH_RESP_SIZE);
+	pSMB->req_no_secext.CaseSensitivePasswordLength =
+			cpu_to_le16(CIFS_AUTH_RESP_SIZE);
+
+	/* calculate ntlm response and session key */
+	rc = setup_ntlm_response(ses, sess_data->nls_cp);
+	if (rc) {
+		cifs_dbg(VFS, "Error %d during NTLM authentication\n",
+				 rc);
+		goto out;
+	}
+
+	/* copy ntlm response */
+	memcpy(bcc_ptr, ses->auth_key.response + CIFS_SESS_KEY_SIZE,
+			CIFS_AUTH_RESP_SIZE);
+	bcc_ptr += CIFS_AUTH_RESP_SIZE;
+	memcpy(bcc_ptr, ses->auth_key.response + CIFS_SESS_KEY_SIZE,
+			CIFS_AUTH_RESP_SIZE);
+	bcc_ptr += CIFS_AUTH_RESP_SIZE;
+
+	if (ses->capabilities & CAP_UNICODE) {
+		/* unicode strings must be word aligned */
+		if (sess_data->iov[0].iov_len % 2) {
+			*bcc_ptr = 0;
+			bcc_ptr++;
+		}
+		unicode_ssetup_strings(&bcc_ptr, ses, sess_data->nls_cp);
+	} else {
+		ascii_ssetup_strings(&bcc_ptr, ses, sess_data->nls_cp);
+	}
+
+
+	sess_data->iov[2].iov_len = (long) bcc_ptr -
+			(long) sess_data->iov[2].iov_base;
+
+	rc = sess_sendreceive(sess_data);
+	if (rc)
+		goto out;
+
+	pSMB = (SESSION_SETUP_ANDX *)sess_data->iov[0].iov_base;
+	smb_buf = (struct smb_hdr *)sess_data->iov[0].iov_base;
+
+	if (smb_buf->WordCount != 3) {
+		rc = -EIO;
+		cifs_dbg(VFS, "bad word count %d\n", smb_buf->WordCount);
+		goto out;
+	}
+
+	if (le16_to_cpu(pSMB->resp.Action) & GUEST_LOGIN)
+		cifs_dbg(FYI, "Guest login\n"); /* BB mark SesInfo struct? */
+
+	ses->Suid = smb_buf->Uid;   /* UID left in wire format (le) */
+	cifs_dbg(FYI, "UID = %llu\n", ses->Suid);
+
+	bytes_remaining = get_bcc(smb_buf);
+	bcc_ptr = pByteArea(smb_buf);
+
+	/* BB check if Unicode and decode strings */
+	if (bytes_remaining == 0) {
+		/* no string area to decode, do nothing */
+	} else if (smb_buf->Flags2 & SMBFLG2_UNICODE) {
+		/* unicode string area must be word-aligned */
+		if (((unsigned long) bcc_ptr - (unsigned long) smb_buf) % 2) {
+			++bcc_ptr;
+			--bytes_remaining;
+		}
+		decode_unicode_ssetup(&bcc_ptr, bytes_remaining, ses,
+				      sess_data->nls_cp);
+	} else {
+		decode_ascii_ssetup(&bcc_ptr, bytes_remaining, ses,
+				    sess_data->nls_cp);
+	}
+
+out:
+	sess_data->result = rc;
+	sess_data->func = NULL;
+	sess_free_buffer(sess_data);
+	sess_establish_session(sess_data);
+	kfree(ses->auth_key.response);
+	ses->auth_key.response = NULL;
+}
+
+static void
+sess_auth_ntlmv2(struct sess_data *sess_data)
+{
+	int rc = 0;
+	struct smb_hdr *smb_buf;
+	SESSION_SETUP_ANDX *pSMB;
+	char *bcc_ptr;
+	struct cifs_ses *ses = sess_data->ses;
+	__u32 capabilities;
+	__u16 bytes_remaining;
+
+	/* old style NTLM sessionsetup */
+	/* wct = 13 */
+	rc = sess_alloc_buffer(sess_data, 13);
+	if (rc)
+		goto out;
+
+	pSMB = (SESSION_SETUP_ANDX *)sess_data->iov[0].iov_base;
+	bcc_ptr = sess_data->iov[2].iov_base;
+	capabilities = cifs_ssetup_hdr(ses, pSMB);
+
+	pSMB->req_no_secext.Capabilities = cpu_to_le32(capabilities);
+
+	/* LM2 password would be here if we supported it */
+	pSMB->req_no_secext.CaseInsensitivePasswordLength = 0;
+
+	/* calculate nlmv2 response and session key */
+	rc = setup_ntlmv2_rsp(ses, sess_data->nls_cp);
+	if (rc) {
+		cifs_dbg(VFS, "Error %d during NTLMv2 authentication\n", rc);
+		goto out;
+	}
+
+	memcpy(bcc_ptr, ses->auth_key.response + CIFS_SESS_KEY_SIZE,
+			ses->auth_key.len - CIFS_SESS_KEY_SIZE);
+	bcc_ptr += ses->auth_key.len - CIFS_SESS_KEY_SIZE;
+
+	/* set case sensitive password length after tilen may get
+	 * assigned, tilen is 0 otherwise.
+	 */
+	pSMB->req_no_secext.CaseSensitivePasswordLength =
+		cpu_to_le16(ses->auth_key.len - CIFS_SESS_KEY_SIZE);
+
+	if (ses->capabilities & CAP_UNICODE) {
+		if (sess_data->iov[0].iov_len % 2) {
+			*bcc_ptr = 0;
+			bcc_ptr++;
+		}
+		unicode_ssetup_strings(&bcc_ptr, ses, sess_data->nls_cp);
+	} else {
+		ascii_ssetup_strings(&bcc_ptr, ses, sess_data->nls_cp);
+	}
+
+
+	sess_data->iov[2].iov_len = (long) bcc_ptr -
+			(long) sess_data->iov[2].iov_base;
+
+	rc = sess_sendreceive(sess_data);
+	if (rc)
+		goto out;
+
+	pSMB = (SESSION_SETUP_ANDX *)sess_data->iov[0].iov_base;
+	smb_buf = (struct smb_hdr *)sess_data->iov[0].iov_base;
+
+	if (smb_buf->WordCount != 3) {
+		rc = -EIO;
+		cifs_dbg(VFS, "bad word count %d\n", smb_buf->WordCount);
+		goto out;
+	}
+
+	if (le16_to_cpu(pSMB->resp.Action) & GUEST_LOGIN)
+		cifs_dbg(FYI, "Guest login\n"); /* BB mark SesInfo struct? */
+
+	ses->Suid = smb_buf->Uid;   /* UID left in wire format (le) */
+	cifs_dbg(FYI, "UID = %llu\n", ses->Suid);
+
+	bytes_remaining = get_bcc(smb_buf);
+	bcc_ptr = pByteArea(smb_buf);
+
+	/* BB check if Unicode and decode strings */
+	if (bytes_remaining == 0) {
+		/* no string area to decode, do nothing */
+	} else if (smb_buf->Flags2 & SMBFLG2_UNICODE) {
+		/* unicode string area must be word-aligned */
+		if (((unsigned long) bcc_ptr - (unsigned long) smb_buf) % 2) {
+			++bcc_ptr;
+			--bytes_remaining;
+		}
+		decode_unicode_ssetup(&bcc_ptr, bytes_remaining, ses,
+				      sess_data->nls_cp);
+	} else {
+		decode_ascii_ssetup(&bcc_ptr, bytes_remaining, ses,
+				    sess_data->nls_cp);
+	}
+
+out:
+	sess_data->result = rc;
+	sess_data->func = NULL;
+	sess_free_buffer(sess_data);
+	sess_establish_session(sess_data);
+	kfree(ses->auth_key.response);
+	ses->auth_key.response = NULL;
+}
+
+
 int
 CIFS_SessSetup(const unsigned int xid, struct cifs_ses *ses,
 	       const struct nls_table *nls_cp)
@@ -813,6 +1023,12 @@ CIFS_SessSetup(const unsigned int xid, struct cifs_ses *ses,
         case LANMAN:
                 sess_auth_lanman(sess_data);
 		goto out;
+	case NTLM:
+                sess_auth_ntlm(sess_data);
+		goto out;
+	case NTLMv2:
+                sess_auth_ntlmv2(sess_data);
+		goto out;
 	default:
 		/* Continue with the rest of the function */
 		break;
@@ -833,11 +1049,8 @@ ssetup_ntlmssp_authenticate:
 	if (phase == NtLmChallenge)
 		phase = NtLmAuthenticate; /* if ntlmssp, now final phase */
 
-	if ((type == NTLM) || (type == NTLMv2)) {
-		/* For NTLMv2 failures eventually may need to retry NTLM */
-		wct = 13; /* old style NTLM sessionsetup */
-	} else /* same size: negotiate or auth, NTLMSSP or extended security */
-		wct = 12;
+	/* same size: negotiate or auth, NTLMSSP or extended security */
+	wct = 12;
 
 	rc = small_smb_init_no_tc(SMB_COM_SESSION_SETUP_ANDX, wct, ses,
 			    (void **)&smb_buf);
@@ -872,70 +1085,7 @@ ssetup_ntlmssp_authenticate:
 	iov[1].iov_base = NULL;
 	iov[1].iov_len = 0;
 
-	if (type == NTLM) {
-		pSMB->req_no_secext.Capabilities = cpu_to_le32(capabilities);
-		pSMB->req_no_secext.CaseInsensitivePasswordLength =
-			cpu_to_le16(CIFS_AUTH_RESP_SIZE);
-		pSMB->req_no_secext.CaseSensitivePasswordLength =
-			cpu_to_le16(CIFS_AUTH_RESP_SIZE);
-
-		/* calculate ntlm response and session key */
-		rc = setup_ntlm_response(ses, nls_cp);
-		if (rc) {
-			cifs_dbg(VFS, "Error %d during NTLM authentication\n",
-				 rc);
-			goto ssetup_exit;
-		}
-
-		/* copy ntlm response */
-		memcpy(bcc_ptr, ses->auth_key.response + CIFS_SESS_KEY_SIZE,
-				CIFS_AUTH_RESP_SIZE);
-		bcc_ptr += CIFS_AUTH_RESP_SIZE;
-		memcpy(bcc_ptr, ses->auth_key.response + CIFS_SESS_KEY_SIZE,
-				CIFS_AUTH_RESP_SIZE);
-		bcc_ptr += CIFS_AUTH_RESP_SIZE;
-
-		if (ses->capabilities & CAP_UNICODE) {
-			/* unicode strings must be word aligned */
-			if (iov[0].iov_len % 2) {
-				*bcc_ptr = 0;
-				bcc_ptr++;
-			}
-			unicode_ssetup_strings(&bcc_ptr, ses, nls_cp);
-		} else
-			ascii_ssetup_strings(&bcc_ptr, ses, nls_cp);
-	} else if (type == NTLMv2) {
-		pSMB->req_no_secext.Capabilities = cpu_to_le32(capabilities);
-
-		/* LM2 password would be here if we supported it */
-		pSMB->req_no_secext.CaseInsensitivePasswordLength = 0;
-
-		/* calculate nlmv2 response and session key */
-		rc = setup_ntlmv2_rsp(ses, nls_cp);
-		if (rc) {
-			cifs_dbg(VFS, "Error %d during NTLMv2 authentication\n",
-				 rc);
-			goto ssetup_exit;
-		}
-		memcpy(bcc_ptr, ses->auth_key.response + CIFS_SESS_KEY_SIZE,
-				ses->auth_key.len - CIFS_SESS_KEY_SIZE);
-		bcc_ptr += ses->auth_key.len - CIFS_SESS_KEY_SIZE;
-
-		/* set case sensitive password length after tilen may get
-		 * assigned, tilen is 0 otherwise.
-		 */
-		pSMB->req_no_secext.CaseSensitivePasswordLength =
-			cpu_to_le16(ses->auth_key.len - CIFS_SESS_KEY_SIZE);
-
-		if (ses->capabilities & CAP_UNICODE) {
-			if (iov[0].iov_len % 2) {
-				*bcc_ptr = 0;
-				bcc_ptr++;
-			}
-			unicode_ssetup_strings(&bcc_ptr, ses, nls_cp);
-		} else
-			ascii_ssetup_strings(&bcc_ptr, ses, nls_cp);
-	} else if (type == Kerberos) {
+	if (type == Kerberos) {
 #ifdef CONFIG_CIFS_UPCALL
 		struct cifs_spnego_msg *msg;
 
@@ -1086,7 +1236,7 @@ ssetup_ntlmssp_authenticate:
 	if (rc)
 		goto ssetup_exit;
 
-	if ((smb_buf->WordCount != 3) && (smb_buf->WordCount != 4)) {
+	if (smb_buf->WordCount != 4) {
 		rc = -EIO;
 		cifs_dbg(VFS, "bad word count %d\n", smb_buf->WordCount);
 		goto ssetup_exit;
@@ -1101,23 +1251,21 @@ ssetup_ntlmssp_authenticate:
 	bytes_remaining = get_bcc(smb_buf);
 	bcc_ptr = pByteArea(smb_buf);
 
-	if (smb_buf->WordCount == 4) {
-		blob_len = le16_to_cpu(pSMB->resp.SecurityBlobLength);
-		if (blob_len > bytes_remaining) {
-			cifs_dbg(VFS, "bad security blob length %d\n",
-				 blob_len);
-			rc = -EINVAL;
+	blob_len = le16_to_cpu(pSMB->resp.SecurityBlobLength);
+	if (blob_len > bytes_remaining) {
+		cifs_dbg(VFS, "bad security blob length %d\n",
+			 blob_len);
+		rc = -EINVAL;
+		goto ssetup_exit;
+	}
+	if (phase == NtLmChallenge) {
+		rc = decode_ntlmssp_challenge(bcc_ptr, blob_len, ses);
+		/* now goto beginning for ntlmssp authenticate phase */
+		if (rc)
 			goto ssetup_exit;
-		}
-		if (phase == NtLmChallenge) {
-			rc = decode_ntlmssp_challenge(bcc_ptr, blob_len, ses);
-			/* now goto beginning for ntlmssp authenticate phase */
-			if (rc)
-				goto ssetup_exit;
-		}
-		bcc_ptr += blob_len;
-		bytes_remaining -= blob_len;
 	}
+	bcc_ptr += blob_len;
+	bytes_remaining -= blob_len;
 
 	/* BB check if Unicode and decode strings */
 	if (bytes_remaining == 0) {
-- 
1.8.4.2

Re: [PATCH 2/4] cifs: Split ntlm and ntlmv2 authentication methods off CIFS_SessSetup()

[Shirish Pargaonkar]

On Thu, May 1, 2014 at 7:41 AM, Sachin Prabhu <sprabhu-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> Signed-off-by: Sachin Prabhu <sprabhu-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
>  fs/cifs/sess.c | 318 ++++++++++++++++++++++++++++++++++++++++++---------------
>  1 file changed, 233 insertions(+), 85 deletions(-)
>
> diff --git a/fs/cifs/sess.c b/fs/cifs/sess.c
> index 6559deb..661b67d 100644
> --- a/fs/cifs/sess.c
> +++ b/fs/cifs/sess.c
> @@ -767,6 +767,216 @@ sess_auth_lanman(struct sess_data *sess_data)
>  }
>  #endif
>
> +static void
> +sess_auth_ntlm(struct sess_data *sess_data)
> +{
> +       int rc = 0;
> +       struct smb_hdr *smb_buf;
> +       SESSION_SETUP_ANDX *pSMB;
> +       char *bcc_ptr;
> +       struct cifs_ses *ses = sess_data->ses;
> +       __u32 capabilities;
> +       __u16 bytes_remaining;
> +
> +       /* old style NTLM sessionsetup */
> +       /* wct = 13 */
> +       rc = sess_alloc_buffer(sess_data, 13);
> +       if (rc)
> +               goto out;
> +
> +       pSMB = (SESSION_SETUP_ANDX *)sess_data->iov[0].iov_base;
> +       bcc_ptr = sess_data->iov[2].iov_base;
> +       capabilities = cifs_ssetup_hdr(ses, pSMB);
> +
> +       pSMB->req_no_secext.Capabilities = cpu_to_le32(capabilities);
> +       pSMB->req_no_secext.CaseInsensitivePasswordLength =
> +                       cpu_to_le16(CIFS_AUTH_RESP_SIZE);
> +       pSMB->req_no_secext.CaseSensitivePasswordLength =
> +                       cpu_to_le16(CIFS_AUTH_RESP_SIZE);
> +
> +       /* calculate ntlm response and session key */
> +       rc = setup_ntlm_response(ses, sess_data->nls_cp);
> +       if (rc) {
> +               cifs_dbg(VFS, "Error %d during NTLM authentication\n",
> +                                rc);
> +               goto out;
> +       }
> +
> +       /* copy ntlm response */
> +       memcpy(bcc_ptr, ses->auth_key.response + CIFS_SESS_KEY_SIZE,
> +                       CIFS_AUTH_RESP_SIZE);
> +       bcc_ptr += CIFS_AUTH_RESP_SIZE;
> +       memcpy(bcc_ptr, ses->auth_key.response + CIFS_SESS_KEY_SIZE,
> +                       CIFS_AUTH_RESP_SIZE);
> +       bcc_ptr += CIFS_AUTH_RESP_SIZE;
> +
> +       if (ses->capabilities & CAP_UNICODE) {
> +               /* unicode strings must be word aligned */
> +               if (sess_data->iov[0].iov_len % 2) {
> +                       *bcc_ptr = 0;
> +                       bcc_ptr++;
> +               }
> +               unicode_ssetup_strings(&bcc_ptr, ses, sess_data->nls_cp);
> +       } else {
> +               ascii_ssetup_strings(&bcc_ptr, ses, sess_data->nls_cp);
> +       }
> +
> +
> +       sess_data->iov[2].iov_len = (long) bcc_ptr -
> +                       (long) sess_data->iov[2].iov_base;
> +
> +       rc = sess_sendreceive(sess_data);
> +       if (rc)
> +               goto out;
> +
> +       pSMB = (SESSION_SETUP_ANDX *)sess_data->iov[0].iov_base;
> +       smb_buf = (struct smb_hdr *)sess_data->iov[0].iov_base;
> +
> +       if (smb_buf->WordCount != 3) {
> +               rc = -EIO;
> +               cifs_dbg(VFS, "bad word count %d\n", smb_buf->WordCount);
> +               goto out;
> +       }
> +
> +       if (le16_to_cpu(pSMB->resp.Action) & GUEST_LOGIN)
> +               cifs_dbg(FYI, "Guest login\n"); /* BB mark SesInfo struct? */
> +
> +       ses->Suid = smb_buf->Uid;   /* UID left in wire format (le) */
> +       cifs_dbg(FYI, "UID = %llu\n", ses->Suid);
> +
> +       bytes_remaining = get_bcc(smb_buf);
> +       bcc_ptr = pByteArea(smb_buf);
> +
> +       /* BB check if Unicode and decode strings */
> +       if (bytes_remaining == 0) {
> +               /* no string area to decode, do nothing */
> +       } else if (smb_buf->Flags2 & SMBFLG2_UNICODE) {
> +               /* unicode string area must be word-aligned */
> +               if (((unsigned long) bcc_ptr - (unsigned long) smb_buf) % 2) {
> +                       ++bcc_ptr;
> +                       --bytes_remaining;
> +               }
> +               decode_unicode_ssetup(&bcc_ptr, bytes_remaining, ses,
> +                                     sess_data->nls_cp);
> +       } else {
> +               decode_ascii_ssetup(&bcc_ptr, bytes_remaining, ses,
> +                                   sess_data->nls_cp);
> +       }
> +
> +out:
> +       sess_data->result = rc;
> +       sess_data->func = NULL;
> +       sess_free_buffer(sess_data);
> +       sess_establish_session(sess_data);
> +       kfree(ses->auth_key.response);
> +       ses->auth_key.response = NULL;
> +}
> +
> +static void
> +sess_auth_ntlmv2(struct sess_data *sess_data)
> +{
> +       int rc = 0;
> +       struct smb_hdr *smb_buf;
> +       SESSION_SETUP_ANDX *pSMB;
> +       char *bcc_ptr;
> +       struct cifs_ses *ses = sess_data->ses;
> +       __u32 capabilities;
> +       __u16 bytes_remaining;
> +
> +       /* old style NTLM sessionsetup */
> +       /* wct = 13 */
> +       rc = sess_alloc_buffer(sess_data, 13);
> +       if (rc)
> +               goto out;
> +
> +       pSMB = (SESSION_SETUP_ANDX *)sess_data->iov[0].iov_base;
> +       bcc_ptr = sess_data->iov[2].iov_base;
> +       capabilities = cifs_ssetup_hdr(ses, pSMB);
> +
> +       pSMB->req_no_secext.Capabilities = cpu_to_le32(capabilities);
> +
> +       /* LM2 password would be here if we supported it */
> +       pSMB->req_no_secext.CaseInsensitivePasswordLength = 0;
> +
> +       /* calculate nlmv2 response and session key */
> +       rc = setup_ntlmv2_rsp(ses, sess_data->nls_cp);
> +       if (rc) {
> +               cifs_dbg(VFS, "Error %d during NTLMv2 authentication\n", rc);
> +               goto out;
> +       }
> +
> +       memcpy(bcc_ptr, ses->auth_key.response + CIFS_SESS_KEY_SIZE,
> +                       ses->auth_key.len - CIFS_SESS_KEY_SIZE);
> +       bcc_ptr += ses->auth_key.len - CIFS_SESS_KEY_SIZE;
> +
> +       /* set case sensitive password length after tilen may get
> +        * assigned, tilen is 0 otherwise.
> +        */
> +       pSMB->req_no_secext.CaseSensitivePasswordLength =
> +               cpu_to_le16(ses->auth_key.len - CIFS_SESS_KEY_SIZE);
> +
> +       if (ses->capabilities & CAP_UNICODE) {
> +               if (sess_data->iov[0].iov_len % 2) {
> +                       *bcc_ptr = 0;
> +                       bcc_ptr++;
> +               }
> +               unicode_ssetup_strings(&bcc_ptr, ses, sess_data->nls_cp);
> +       } else {
> +               ascii_ssetup_strings(&bcc_ptr, ses, sess_data->nls_cp);
> +       }
> +
> +
> +       sess_data->iov[2].iov_len = (long) bcc_ptr -
> +                       (long) sess_data->iov[2].iov_base;
> +
> +       rc = sess_sendreceive(sess_data);
> +       if (rc)
> +               goto out;
> +
> +       pSMB = (SESSION_SETUP_ANDX *)sess_data->iov[0].iov_base;
> +       smb_buf = (struct smb_hdr *)sess_data->iov[0].iov_base;
> +
> +       if (smb_buf->WordCount != 3) {
> +               rc = -EIO;
> +               cifs_dbg(VFS, "bad word count %d\n", smb_buf->WordCount);
> +               goto out;
> +       }
> +
> +       if (le16_to_cpu(pSMB->resp.Action) & GUEST_LOGIN)
> +               cifs_dbg(FYI, "Guest login\n"); /* BB mark SesInfo struct? */
> +
> +       ses->Suid = smb_buf->Uid;   /* UID left in wire format (le) */
> +       cifs_dbg(FYI, "UID = %llu\n", ses->Suid);
> +
> +       bytes_remaining = get_bcc(smb_buf);
> +       bcc_ptr = pByteArea(smb_buf);
> +
> +       /* BB check if Unicode and decode strings */
> +       if (bytes_remaining == 0) {
> +               /* no string area to decode, do nothing */
> +       } else if (smb_buf->Flags2 & SMBFLG2_UNICODE) {
> +               /* unicode string area must be word-aligned */
> +               if (((unsigned long) bcc_ptr - (unsigned long) smb_buf) % 2) {
> +                       ++bcc_ptr;
> +                       --bytes_remaining;
> +               }
> +               decode_unicode_ssetup(&bcc_ptr, bytes_remaining, ses,
> +                                     sess_data->nls_cp);
> +       } else {
> +               decode_ascii_ssetup(&bcc_ptr, bytes_remaining, ses,
> +                                   sess_data->nls_cp);
> +       }
> +
> +out:
> +       sess_data->result = rc;
> +       sess_data->func = NULL;
> +       sess_free_buffer(sess_data);
> +       sess_establish_session(sess_data);
> +       kfree(ses->auth_key.response);
> +       ses->auth_key.response = NULL;
> +}

In all three sess_auth_* functions (ntlm, ntlmv2, and lanman (in
previous patch),
we will end up calling sess_establish_session() even if there is an error i.e.
sess_data->result is not 0.

> +
> +
>  int
>  CIFS_SessSetup(const unsigned int xid, struct cifs_ses *ses,
>                const struct nls_table *nls_cp)
> @@ -813,6 +1023,12 @@ CIFS_SessSetup(const unsigned int xid, struct cifs_ses *ses,
>          case LANMAN:
>                  sess_auth_lanman(sess_data);
>                 goto out;
> +       case NTLM:
> +                sess_auth_ntlm(sess_data);
> +               goto out;
> +       case NTLMv2:
> +                sess_auth_ntlmv2(sess_data);
> +               goto out;
>         default:
>                 /* Continue with the rest of the function */
>                 break;
> @@ -833,11 +1049,8 @@ ssetup_ntlmssp_authenticate:
>         if (phase == NtLmChallenge)
>                 phase = NtLmAuthenticate; /* if ntlmssp, now final phase */
>
> -       if ((type == NTLM) || (type == NTLMv2)) {
> -               /* For NTLMv2 failures eventually may need to retry NTLM */
> -               wct = 13; /* old style NTLM sessionsetup */
> -       } else /* same size: negotiate or auth, NTLMSSP or extended security */
> -               wct = 12;
> +       /* same size: negotiate or auth, NTLMSSP or extended security */
> +       wct = 12;
>
>         rc = small_smb_init_no_tc(SMB_COM_SESSION_SETUP_ANDX, wct, ses,
>                             (void **)&smb_buf);
> @@ -872,70 +1085,7 @@ ssetup_ntlmssp_authenticate:
>         iov[1].iov_base = NULL;
>         iov[1].iov_len = 0;
>
> -       if (type == NTLM) {
> -               pSMB->req_no_secext.Capabilities = cpu_to_le32(capabilities);
> -               pSMB->req_no_secext.CaseInsensitivePasswordLength =
> -                       cpu_to_le16(CIFS_AUTH_RESP_SIZE);
> -               pSMB->req_no_secext.CaseSensitivePasswordLength =
> -                       cpu_to_le16(CIFS_AUTH_RESP_SIZE);
> -
> -               /* calculate ntlm response and session key */
> -               rc = setup_ntlm_response(ses, nls_cp);
> -               if (rc) {
> -                       cifs_dbg(VFS, "Error %d during NTLM authentication\n",
> -                                rc);
> -                       goto ssetup_exit;
> -               }
> -
> -               /* copy ntlm response */
> -               memcpy(bcc_ptr, ses->auth_key.response + CIFS_SESS_KEY_SIZE,
> -                               CIFS_AUTH_RESP_SIZE);
> -               bcc_ptr += CIFS_AUTH_RESP_SIZE;
> -               memcpy(bcc_ptr, ses->auth_key.response + CIFS_SESS_KEY_SIZE,
> -                               CIFS_AUTH_RESP_SIZE);
> -               bcc_ptr += CIFS_AUTH_RESP_SIZE;
> -
> -               if (ses->capabilities & CAP_UNICODE) {
> -                       /* unicode strings must be word aligned */
> -                       if (iov[0].iov_len % 2) {
> -                               *bcc_ptr = 0;
> -                               bcc_ptr++;
> -                       }
> -                       unicode_ssetup_strings(&bcc_ptr, ses, nls_cp);
> -               } else
> -                       ascii_ssetup_strings(&bcc_ptr, ses, nls_cp);
> -       } else if (type == NTLMv2) {
> -               pSMB->req_no_secext.Capabilities = cpu_to_le32(capabilities);
> -
> -               /* LM2 password would be here if we supported it */
> -               pSMB->req_no_secext.CaseInsensitivePasswordLength = 0;
> -
> -               /* calculate nlmv2 response and session key */
> -               rc = setup_ntlmv2_rsp(ses, nls_cp);
> -               if (rc) {
> -                       cifs_dbg(VFS, "Error %d during NTLMv2 authentication\n",
> -                                rc);
> -                       goto ssetup_exit;
> -               }
> -               memcpy(bcc_ptr, ses->auth_key.response + CIFS_SESS_KEY_SIZE,
> -                               ses->auth_key.len - CIFS_SESS_KEY_SIZE);
> -               bcc_ptr += ses->auth_key.len - CIFS_SESS_KEY_SIZE;
> -
> -               /* set case sensitive password length after tilen may get
> -                * assigned, tilen is 0 otherwise.
> -                */
> -               pSMB->req_no_secext.CaseSensitivePasswordLength =
> -                       cpu_to_le16(ses->auth_key.len - CIFS_SESS_KEY_SIZE);
> -
> -               if (ses->capabilities & CAP_UNICODE) {
> -                       if (iov[0].iov_len % 2) {
> -                               *bcc_ptr = 0;
> -                               bcc_ptr++;
> -                       }
> -                       unicode_ssetup_strings(&bcc_ptr, ses, nls_cp);
> -               } else
> -                       ascii_ssetup_strings(&bcc_ptr, ses, nls_cp);
> -       } else if (type == Kerberos) {
> +       if (type == Kerberos) {
>  #ifdef CONFIG_CIFS_UPCALL
>                 struct cifs_spnego_msg *msg;
>
> @@ -1086,7 +1236,7 @@ ssetup_ntlmssp_authenticate:
>         if (rc)
>                 goto ssetup_exit;
>
> -       if ((smb_buf->WordCount != 3) && (smb_buf->WordCount != 4)) {
> +       if (smb_buf->WordCount != 4) {
>                 rc = -EIO;
>                 cifs_dbg(VFS, "bad word count %d\n", smb_buf->WordCount);
>                 goto ssetup_exit;
> @@ -1101,23 +1251,21 @@ ssetup_ntlmssp_authenticate:
>         bytes_remaining = get_bcc(smb_buf);
>         bcc_ptr = pByteArea(smb_buf);
>
> -       if (smb_buf->WordCount == 4) {
> -               blob_len = le16_to_cpu(pSMB->resp.SecurityBlobLength);
> -               if (blob_len > bytes_remaining) {
> -                       cifs_dbg(VFS, "bad security blob length %d\n",
> -                                blob_len);
> -                       rc = -EINVAL;
> +       blob_len = le16_to_cpu(pSMB->resp.SecurityBlobLength);
> +       if (blob_len > bytes_remaining) {
> +               cifs_dbg(VFS, "bad security blob length %d\n",
> +                        blob_len);
> +               rc = -EINVAL;
> +               goto ssetup_exit;
> +       }
> +       if (phase == NtLmChallenge) {
> +               rc = decode_ntlmssp_challenge(bcc_ptr, blob_len, ses);
> +               /* now goto beginning for ntlmssp authenticate phase */

I think this comment should move to after if statement
(it was at wrong place to begin with) i.e. go to authenticate phase
only if rc is NULL.

> +               if (rc)
>                         goto ssetup_exit;
> -               }
> -               if (phase == NtLmChallenge) {
> -                       rc = decode_ntlmssp_challenge(bcc_ptr, blob_len, ses);
> -                       /* now goto beginning for ntlmssp authenticate phase */
> -                       if (rc)
> -                               goto ssetup_exit;
> -               }
> -               bcc_ptr += blob_len;
> -               bytes_remaining -= blob_len;
>         }
> +       bcc_ptr += blob_len;
> +       bytes_remaining -= blob_len;
>
>         /* BB check if Unicode and decode strings */
>         if (bytes_remaining == 0) {
> --
> 1.8.4.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/4] cifs: Split ntlm and ntlmv2 authentication methods off CIFS_SessSetup()

[Sachin Prabhu]

On Thu, 2014-05-01 at 10:31 -0500, Shirish Pargaonkar wrote:

> > +
> > +out:
> > +       sess_data->result = rc;
> > +       sess_data->func = NULL;
> > +       sess_free_buffer(sess_data);
> > +       sess_establish_session(sess_data);
> > +       kfree(ses->auth_key.response);
> > +       ses->auth_key.response = NULL;
> > +}
> 
> In all three sess_auth_* functions (ntlm, ntlmv2, and lanman (in
> previous patch),
> we will end up calling sess_establish_session() even if there is an error i.e.
> sess_data->result is not 0.

We check for any errors within sess_establish_session() before we
attempt to establish session.

static int
sess_establish_session(struct sess_data *sess_data)
{
..
        if (!sess_data->result) {
		//Establish Session.
	}
..
}

This isn't a technical issue but maybe considered a style issue.


> > +       if (phase == NtLmChallenge) {
> > +               rc = decode_ntlmssp_challenge(bcc_ptr, blob_len, ses);
> > +               /* now goto beginning for ntlmssp authenticate phase */
> 
> I think this comment should move to after if statement
> (it was at wrong place to begin with) i.e. go to authenticate phase
> only if rc is NULL.

I agree. The comment is actually not required.

Sachin Prabhu

Re: [PATCH 2/4] cifs: Split ntlm and ntlmv2 authentication methods off CIFS_SessSetup()

[Shirish Pargaonkar]

On Thu, May 1, 2014 at 11:26 AM, Sachin Prabhu <sprabhu-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> On Thu, 2014-05-01 at 10:31 -0500, Shirish Pargaonkar wrote:
>
>> > +
>> > +out:
>> > +       sess_data->result = rc;
>> > +       sess_data->func = NULL;
>> > +       sess_free_buffer(sess_data);
>> > +       sess_establish_session(sess_data);
>> > +       kfree(ses->auth_key.response);
>> > +       ses->auth_key.response = NULL;
>> > +}
>>
>> In all three sess_auth_* functions (ntlm, ntlmv2, and lanman (in
>> previous patch),
>> we will end up calling sess_establish_session() even if there is an error i.e.
>> sess_data->result is not 0.
>
> We check for any errors within sess_establish_session() before we
> attempt to establish session.
>
> static int
> sess_establish_session(struct sess_data *sess_data)
> {
> ..
>         if (!sess_data->result) {
>                 //Establish Session.
>         }
> ..
> }
>
> This isn't a technical issue but maybe considered a style issue.

Sure, would have preferred to call the function before goto lables, but will
leave at that.

>
>
>> > +       if (phase == NtLmChallenge) {
>> > +               rc = decode_ntlmssp_challenge(bcc_ptr, blob_len, ses);
>> > +               /* now goto beginning for ntlmssp authenticate phase */
>>
>> I think this comment should move to after if statement
>> (it was at wrong place to begin with) i.e. go to authenticate phase
>> only if rc is NULL.
>
> I agree. The comment is actually not required.
>
> Sachin Prabhu
>

Re: [PATCH 2/4] cifs: Split ntlm and ntlmv2 authentication methods off CIFS_SessSetup()

[Sachin Prabhu]

On Thu, 2014-05-01 at 15:01 -0500, Shirish Pargaonkar wrote:
> On Thu, May 1, 2014 at 11:26 AM, Sachin Prabhu <sprabhu-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> > On Thu, 2014-05-01 at 10:31 -0500, Shirish Pargaonkar wrote:
> >
> >> > +
> >> > +out:
> >> > +       sess_data->result = rc;
> >> > +       sess_data->func = NULL;
> >> > +       sess_free_buffer(sess_data);
> >> > +       sess_establish_session(sess_data);
> >> > +       kfree(ses->auth_key.response);
> >> > +       ses->auth_key.response = NULL;
> >> > +}
> >>
> >> In all three sess_auth_* functions (ntlm, ntlmv2, and lanman (in
> >> previous patch),
> >> we will end up calling sess_establish_session() even if there is an error i.e.
> >> sess_data->result is not 0.
> >
> > We check for any errors within sess_establish_session() before we
> > attempt to establish session.
> >
> > static int
> > sess_establish_session(struct sess_data *sess_data)
> > {
> > ..
> >         if (!sess_data->result) {
> >                 //Establish Session.
> >         }
> > ..
> > }
> >
> > This isn't a technical issue but maybe considered a style issue.
> 
> Sure, would have preferred to call the function before goto lables, but will
> leave at that.
> 

Having given it another thought, I agree with you. I've made the
required modifications and tested the new patches. I'll send them to the
list shortly.

Sachin Prabhu


> >
> >
> >> > +       if (phase == NtLmChallenge) {
> >> > +               rc = decode_ntlmssp_challenge(bcc_ptr, blob_len, ses);
> >> > +               /* now goto beginning for ntlmssp authenticate phase */
> >>
> >> I think this comment should move to after if statement
> >> (it was at wrong place to begin with) i.e. go to authenticate phase
> >> only if rc is NULL.
> >
> > I agree. The comment is actually not required.
> >
> > Sachin Prabhu
> >

[PATCH 3/4] cifs: Split Kerberos authentication off CIFS_SessSetup()

[Sachin Prabhu]

Signed-off-by: Sachin Prabhu <sprabhu-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 fs/cifs/sess.c | 214 ++++++++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 152 insertions(+), 62 deletions(-)

diff --git a/fs/cifs/sess.c b/fs/cifs/sess.c
index 661b67d..869ad37 100644
--- a/fs/cifs/sess.c
+++ b/fs/cifs/sess.c
@@ -976,6 +976,154 @@ out:
 	ses->auth_key.response = NULL;
 }
 
+#ifdef CONFIG_CIFS_UPCALL
+static void
+sess_auth_kerberos(struct sess_data *sess_data)
+{
+	int rc = 0;
+	struct smb_hdr *smb_buf;
+	SESSION_SETUP_ANDX *pSMB;
+	char *bcc_ptr;
+	struct cifs_ses *ses = sess_data->ses;
+	__u32 capabilities;
+	__u16 bytes_remaining;
+	struct key *spnego_key = NULL;
+	struct cifs_spnego_msg *msg;
+	u16 blob_len;
+
+	/* extended security */
+	/* wct = 12 */
+	rc = sess_alloc_buffer(sess_data, 12);
+	if (rc)
+		goto out;
+
+	pSMB = (SESSION_SETUP_ANDX *)sess_data->iov[0].iov_base;
+	bcc_ptr = sess_data->iov[2].iov_base;
+	capabilities = cifs_ssetup_hdr(ses, pSMB);
+
+	spnego_key = cifs_get_spnego_key(ses);
+	if (IS_ERR(spnego_key)) {
+		rc = PTR_ERR(spnego_key);
+		spnego_key = NULL;
+		goto out;
+	}
+
+	msg = spnego_key->payload.data;
+	/*
+	 * check version field to make sure that cifs.upcall is
+	 * sending us a response in an expected form
+	 */
+	if (msg->version != CIFS_SPNEGO_UPCALL_VERSION) {
+		cifs_dbg(VFS, "incorrect version of cifs.upcall "
+			      "expected %d but got %d)",
+			      CIFS_SPNEGO_UPCALL_VERSION, msg->version);
+		rc = -EKEYREJECTED;
+		goto out_put_spnego_key;
+	}
+
+	ses->auth_key.response = kmemdup(msg->data, msg->sesskey_len,
+					 GFP_KERNEL);
+	if (!ses->auth_key.response) {
+		cifs_dbg(VFS, "Kerberos can't allocate (%u bytes) memory",
+				msg->sesskey_len);
+		rc = -ENOMEM;
+		goto out_put_spnego_key;
+	}
+	ses->auth_key.len = msg->sesskey_len;
+
+	pSMB->req.hdr.Flags2 |= SMBFLG2_EXT_SEC;
+	capabilities |= CAP_EXTENDED_SECURITY;
+	pSMB->req.Capabilities = cpu_to_le32(capabilities);
+	sess_data->iov[1].iov_base = msg->data + msg->sesskey_len;
+	sess_data->iov[1].iov_len = msg->secblob_len;
+	pSMB->req.SecurityBlobLength = cpu_to_le16(sess_data->iov[1].iov_len);
+
+	if (ses->capabilities & CAP_UNICODE) {
+		/* unicode strings must be word aligned */
+		if ((sess_data->iov[0].iov_len + sess_data->iov[1].iov_len) % 2) {
+			*bcc_ptr = 0;
+			bcc_ptr++;
+		}
+		unicode_oslm_strings(&bcc_ptr, sess_data->nls_cp);
+		unicode_domain_string(&bcc_ptr, ses, sess_data->nls_cp);
+	} else {
+		/* BB: is this right? */
+		ascii_ssetup_strings(&bcc_ptr, ses, sess_data->nls_cp);
+	}
+
+	sess_data->iov[2].iov_len = (long) bcc_ptr -
+			(long) sess_data->iov[2].iov_base;
+
+	rc = sess_sendreceive(sess_data);
+	if (rc)
+		goto out_put_spnego_key;
+
+	pSMB = (SESSION_SETUP_ANDX *)sess_data->iov[0].iov_base;
+	smb_buf = (struct smb_hdr *)sess_data->iov[0].iov_base;
+
+	if (smb_buf->WordCount != 4) {
+		rc = -EIO;
+		cifs_dbg(VFS, "bad word count %d\n", smb_buf->WordCount);
+		goto out_put_spnego_key;
+	}
+
+	if (le16_to_cpu(pSMB->resp.Action) & GUEST_LOGIN)
+		cifs_dbg(FYI, "Guest login\n"); /* BB mark SesInfo struct? */
+
+	ses->Suid = smb_buf->Uid;   /* UID left in wire format (le) */
+	cifs_dbg(FYI, "UID = %llu\n", ses->Suid);
+
+	bytes_remaining = get_bcc(smb_buf);
+	bcc_ptr = pByteArea(smb_buf);
+
+	blob_len = le16_to_cpu(pSMB->resp.SecurityBlobLength);
+	if (blob_len > bytes_remaining) {
+		cifs_dbg(VFS, "bad security blob length %d\n",
+				blob_len);
+		rc = -EINVAL;
+		goto out_put_spnego_key;
+	}
+	bcc_ptr += blob_len;
+	bytes_remaining -= blob_len;
+
+	/* BB check if Unicode and decode strings */
+	if (bytes_remaining == 0) {
+		/* no string area to decode, do nothing */
+	} else if (smb_buf->Flags2 & SMBFLG2_UNICODE) {
+		/* unicode string area must be word-aligned */
+		if (((unsigned long) bcc_ptr - (unsigned long) smb_buf) % 2) {
+			++bcc_ptr;
+			--bytes_remaining;
+		}
+		decode_unicode_ssetup(&bcc_ptr, bytes_remaining, ses,
+				      sess_data->nls_cp);
+	} else {
+		decode_ascii_ssetup(&bcc_ptr, bytes_remaining, ses,
+				    sess_data->nls_cp);
+	}
+
+out_put_spnego_key:
+	key_invalidate(spnego_key);
+	key_put(spnego_key);
+out:
+	sess_data->result = rc;
+	sess_data->func = NULL;
+	sess_free_buffer(sess_data);
+	sess_establish_session(sess_data);
+	kfree(ses->auth_key.response);
+	ses->auth_key.response = NULL;
+}
+
+#else
+
+static void
+sess_auth_kerberos(struct sess_data *sess_data)
+{
+	cifs_dbg(VFS, "Kerberos negotiated but upcall support disabled!\n");
+	sess_data->result = -ENOSYS;
+	sess_data->func = NULL;
+}
+#endif /* ! CONFIG_CIFS_UPCALL */
 
 int
 CIFS_SessSetup(const unsigned int xid, struct cifs_ses *ses,
@@ -993,7 +1141,6 @@ CIFS_SessSetup(const unsigned int xid, struct cifs_ses *ses,
 	struct kvec iov[3];
 	enum securityEnum type;
 	__u16 action, bytes_remaining;
-	struct key *spnego_key = NULL;
 	__le32 phase = NtLmNegotiate; /* NTLMSSP, if needed, is multistage */
 	u16 blob_len;
 	char *ntlmsspblob = NULL;
@@ -1029,6 +1176,9 @@ CIFS_SessSetup(const unsigned int xid, struct cifs_ses *ses,
 	case NTLMv2:
                 sess_auth_ntlmv2(sess_data);
 		goto out;
+	case Kerberos:
+                sess_auth_kerberos(sess_data);
+		goto out;
 	default:
 		/* Continue with the rest of the function */
 		break;
@@ -1085,63 +1235,7 @@ ssetup_ntlmssp_authenticate:
 	iov[1].iov_base = NULL;
 	iov[1].iov_len = 0;
 
-	if (type == Kerberos) {
-#ifdef CONFIG_CIFS_UPCALL
-		struct cifs_spnego_msg *msg;
-
-		spnego_key = cifs_get_spnego_key(ses);
-		if (IS_ERR(spnego_key)) {
-			rc = PTR_ERR(spnego_key);
-			spnego_key = NULL;
-			goto ssetup_exit;
-		}
-
-		msg = spnego_key->payload.data;
-		/* check version field to make sure that cifs.upcall is
-		   sending us a response in an expected form */
-		if (msg->version != CIFS_SPNEGO_UPCALL_VERSION) {
-			cifs_dbg(VFS, "incorrect version of cifs.upcall "
-				   "expected %d but got %d)",
-				   CIFS_SPNEGO_UPCALL_VERSION, msg->version);
-			rc = -EKEYREJECTED;
-			goto ssetup_exit;
-		}
-
-		ses->auth_key.response = kmemdup(msg->data, msg->sesskey_len,
-						 GFP_KERNEL);
-		if (!ses->auth_key.response) {
-			cifs_dbg(VFS,
-				"Kerberos can't allocate (%u bytes) memory",
-				msg->sesskey_len);
-			rc = -ENOMEM;
-			goto ssetup_exit;
-		}
-		ses->auth_key.len = msg->sesskey_len;
-
-		pSMB->req.hdr.Flags2 |= SMBFLG2_EXT_SEC;
-		capabilities |= CAP_EXTENDED_SECURITY;
-		pSMB->req.Capabilities = cpu_to_le32(capabilities);
-		iov[1].iov_base = msg->data + msg->sesskey_len;
-		iov[1].iov_len = msg->secblob_len;
-		pSMB->req.SecurityBlobLength = cpu_to_le16(iov[1].iov_len);
-
-		if (ses->capabilities & CAP_UNICODE) {
-			/* unicode strings must be word aligned */
-			if ((iov[0].iov_len + iov[1].iov_len) % 2) {
-				*bcc_ptr = 0;
-				bcc_ptr++;
-			}
-			unicode_oslm_strings(&bcc_ptr, nls_cp);
-			unicode_domain_string(&bcc_ptr, ses, nls_cp);
-		} else
-		/* BB: is this right? */
-			ascii_ssetup_strings(&bcc_ptr, ses, nls_cp);
-#else /* ! CONFIG_CIFS_UPCALL */
-		cifs_dbg(VFS, "Kerberos negotiated but upcall support disabled!\n");
-		rc = -ENOSYS;
-		goto ssetup_exit;
-#endif /* CONFIG_CIFS_UPCALL */
-	} else if (type == RawNTLMSSP) {
+	if (type == RawNTLMSSP) {
 		if ((pSMB->req.hdr.Flags2 & SMBFLG2_UNICODE) == 0) {
 			cifs_dbg(VFS, "NTLMSSP requires Unicode support\n");
 			rc = -ENOSYS;
@@ -1282,10 +1376,6 @@ ssetup_ntlmssp_authenticate:
 	}
 
 ssetup_exit:
-	if (spnego_key) {
-		key_invalidate(spnego_key);
-		key_put(spnego_key);
-	}
 	kfree(str_area);
 	kfree(ntlmsspblob);
 	ntlmsspblob = NULL;
-- 
1.8.4.2

Re: [PATCH 3/4] cifs: Split Kerberos authentication off CIFS_SessSetup()

[Shirish Pargaonkar]

Looks correct.



On Thu, May 1, 2014 at 7:41 AM, Sachin Prabhu <sprabhu-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> Signed-off-by: Sachin Prabhu <sprabhu-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
>  fs/cifs/sess.c | 214 ++++++++++++++++++++++++++++++++++++++++-----------------
>  1 file changed, 152 insertions(+), 62 deletions(-)
>
> diff --git a/fs/cifs/sess.c b/fs/cifs/sess.c
> index 661b67d..869ad37 100644
> --- a/fs/cifs/sess.c
> +++ b/fs/cifs/sess.c
> @@ -976,6 +976,154 @@ out:
>         ses->auth_key.response = NULL;
>  }
>
> +#ifdef CONFIG_CIFS_UPCALL
> +static void
> +sess_auth_kerberos(struct sess_data *sess_data)
> +{
> +       int rc = 0;
> +       struct smb_hdr *smb_buf;
> +       SESSION_SETUP_ANDX *pSMB;
> +       char *bcc_ptr;
> +       struct cifs_ses *ses = sess_data->ses;
> +       __u32 capabilities;
> +       __u16 bytes_remaining;
> +       struct key *spnego_key = NULL;
> +       struct cifs_spnego_msg *msg;
> +       u16 blob_len;
> +
> +       /* extended security */
> +       /* wct = 12 */
> +       rc = sess_alloc_buffer(sess_data, 12);
> +       if (rc)
> +               goto out;
> +
> +       pSMB = (SESSION_SETUP_ANDX *)sess_data->iov[0].iov_base;
> +       bcc_ptr = sess_data->iov[2].iov_base;
> +       capabilities = cifs_ssetup_hdr(ses, pSMB);
> +
> +       spnego_key = cifs_get_spnego_key(ses);
> +       if (IS_ERR(spnego_key)) {
> +               rc = PTR_ERR(spnego_key);
> +               spnego_key = NULL;
> +               goto out;
> +       }
> +
> +       msg = spnego_key->payload.data;
> +       /*
> +        * check version field to make sure that cifs.upcall is
> +        * sending us a response in an expected form
> +        */
> +       if (msg->version != CIFS_SPNEGO_UPCALL_VERSION) {
> +               cifs_dbg(VFS, "incorrect version of cifs.upcall "
> +                             "expected %d but got %d)",
> +                             CIFS_SPNEGO_UPCALL_VERSION, msg->version);
> +               rc = -EKEYREJECTED;
> +               goto out_put_spnego_key;
> +       }
> +
> +       ses->auth_key.response = kmemdup(msg->data, msg->sesskey_len,
> +                                        GFP_KERNEL);
> +       if (!ses->auth_key.response) {
> +               cifs_dbg(VFS, "Kerberos can't allocate (%u bytes) memory",
> +                               msg->sesskey_len);
> +               rc = -ENOMEM;
> +               goto out_put_spnego_key;
> +       }
> +       ses->auth_key.len = msg->sesskey_len;
> +
> +       pSMB->req.hdr.Flags2 |= SMBFLG2_EXT_SEC;
> +       capabilities |= CAP_EXTENDED_SECURITY;
> +       pSMB->req.Capabilities = cpu_to_le32(capabilities);
> +       sess_data->iov[1].iov_base = msg->data + msg->sesskey_len;
> +       sess_data->iov[1].iov_len = msg->secblob_len;
> +       pSMB->req.SecurityBlobLength = cpu_to_le16(sess_data->iov[1].iov_len);
> +
> +       if (ses->capabilities & CAP_UNICODE) {
> +               /* unicode strings must be word aligned */
> +               if ((sess_data->iov[0].iov_len + sess_data->iov[1].iov_len) % 2) {
> +                       *bcc_ptr = 0;
> +                       bcc_ptr++;
> +               }
> +               unicode_oslm_strings(&bcc_ptr, sess_data->nls_cp);
> +               unicode_domain_string(&bcc_ptr, ses, sess_data->nls_cp);
> +       } else {
> +               /* BB: is this right? */
> +               ascii_ssetup_strings(&bcc_ptr, ses, sess_data->nls_cp);
> +       }
> +
> +       sess_data->iov[2].iov_len = (long) bcc_ptr -
> +                       (long) sess_data->iov[2].iov_base;
> +
> +       rc = sess_sendreceive(sess_data);
> +       if (rc)
> +               goto out_put_spnego_key;
> +
> +       pSMB = (SESSION_SETUP_ANDX *)sess_data->iov[0].iov_base;
> +       smb_buf = (struct smb_hdr *)sess_data->iov[0].iov_base;
> +
> +       if (smb_buf->WordCount != 4) {
> +               rc = -EIO;
> +               cifs_dbg(VFS, "bad word count %d\n", smb_buf->WordCount);
> +               goto out_put_spnego_key;
> +       }
> +
> +       if (le16_to_cpu(pSMB->resp.Action) & GUEST_LOGIN)
> +               cifs_dbg(FYI, "Guest login\n"); /* BB mark SesInfo struct? */
> +
> +       ses->Suid = smb_buf->Uid;   /* UID left in wire format (le) */
> +       cifs_dbg(FYI, "UID = %llu\n", ses->Suid);
> +
> +       bytes_remaining = get_bcc(smb_buf);
> +       bcc_ptr = pByteArea(smb_buf);
> +
> +       blob_len = le16_to_cpu(pSMB->resp.SecurityBlobLength);
> +       if (blob_len > bytes_remaining) {
> +               cifs_dbg(VFS, "bad security blob length %d\n",
> +                               blob_len);
> +               rc = -EINVAL;
> +               goto out_put_spnego_key;
> +       }
> +       bcc_ptr += blob_len;
> +       bytes_remaining -= blob_len;
> +
> +       /* BB check if Unicode and decode strings */
> +       if (bytes_remaining == 0) {
> +               /* no string area to decode, do nothing */
> +       } else if (smb_buf->Flags2 & SMBFLG2_UNICODE) {
> +               /* unicode string area must be word-aligned */
> +               if (((unsigned long) bcc_ptr - (unsigned long) smb_buf) % 2) {
> +                       ++bcc_ptr;
> +                       --bytes_remaining;
> +               }
> +               decode_unicode_ssetup(&bcc_ptr, bytes_remaining, ses,
> +                                     sess_data->nls_cp);
> +       } else {
> +               decode_ascii_ssetup(&bcc_ptr, bytes_remaining, ses,
> +                                   sess_data->nls_cp);
> +       }
> +
> +out_put_spnego_key:
> +       key_invalidate(spnego_key);
> +       key_put(spnego_key);
> +out:
> +       sess_data->result = rc;
> +       sess_data->func = NULL;
> +       sess_free_buffer(sess_data);
> +       sess_establish_session(sess_data);
> +       kfree(ses->auth_key.response);
> +       ses->auth_key.response = NULL;
> +}
> +
> +#else
> +
> +static void
> +sess_auth_kerberos(struct sess_data *sess_data)
> +{
> +       cifs_dbg(VFS, "Kerberos negotiated but upcall support disabled!\n");
> +       sess_data->result = -ENOSYS;
> +       sess_data->func = NULL;
> +}
> +#endif /* ! CONFIG_CIFS_UPCALL */
>
>  int
>  CIFS_SessSetup(const unsigned int xid, struct cifs_ses *ses,
> @@ -993,7 +1141,6 @@ CIFS_SessSetup(const unsigned int xid, struct cifs_ses *ses,
>         struct kvec iov[3];
>         enum securityEnum type;
>         __u16 action, bytes_remaining;
> -       struct key *spnego_key = NULL;
>         __le32 phase = NtLmNegotiate; /* NTLMSSP, if needed, is multistage */
>         u16 blob_len;
>         char *ntlmsspblob = NULL;
> @@ -1029,6 +1176,9 @@ CIFS_SessSetup(const unsigned int xid, struct cifs_ses *ses,
>         case NTLMv2:
>                  sess_auth_ntlmv2(sess_data);
>                 goto out;
> +       case Kerberos:
> +                sess_auth_kerberos(sess_data);
> +               goto out;
>         default:
>                 /* Continue with the rest of the function */
>                 break;
> @@ -1085,63 +1235,7 @@ ssetup_ntlmssp_authenticate:
>         iov[1].iov_base = NULL;
>         iov[1].iov_len = 0;
>
> -       if (type == Kerberos) {
> -#ifdef CONFIG_CIFS_UPCALL
> -               struct cifs_spnego_msg *msg;
> -
> -               spnego_key = cifs_get_spnego_key(ses);
> -               if (IS_ERR(spnego_key)) {
> -                       rc = PTR_ERR(spnego_key);
> -                       spnego_key = NULL;
> -                       goto ssetup_exit;
> -               }
> -
> -               msg = spnego_key->payload.data;
> -               /* check version field to make sure that cifs.upcall is
> -                  sending us a response in an expected form */
> -               if (msg->version != CIFS_SPNEGO_UPCALL_VERSION) {
> -                       cifs_dbg(VFS, "incorrect version of cifs.upcall "
> -                                  "expected %d but got %d)",
> -                                  CIFS_SPNEGO_UPCALL_VERSION, msg->version);
> -                       rc = -EKEYREJECTED;
> -                       goto ssetup_exit;
> -               }
> -
> -               ses->auth_key.response = kmemdup(msg->data, msg->sesskey_len,
> -                                                GFP_KERNEL);
> -               if (!ses->auth_key.response) {
> -                       cifs_dbg(VFS,
> -                               "Kerberos can't allocate (%u bytes) memory",
> -                               msg->sesskey_len);
> -                       rc = -ENOMEM;
> -                       goto ssetup_exit;
> -               }
> -               ses->auth_key.len = msg->sesskey_len;
> -
> -               pSMB->req.hdr.Flags2 |= SMBFLG2_EXT_SEC;
> -               capabilities |= CAP_EXTENDED_SECURITY;
> -               pSMB->req.Capabilities = cpu_to_le32(capabilities);
> -               iov[1].iov_base = msg->data + msg->sesskey_len;
> -               iov[1].iov_len = msg->secblob_len;
> -               pSMB->req.SecurityBlobLength = cpu_to_le16(iov[1].iov_len);
> -
> -               if (ses->capabilities & CAP_UNICODE) {
> -                       /* unicode strings must be word aligned */
> -                       if ((iov[0].iov_len + iov[1].iov_len) % 2) {
> -                               *bcc_ptr = 0;
> -                               bcc_ptr++;
> -                       }
> -                       unicode_oslm_strings(&bcc_ptr, nls_cp);
> -                       unicode_domain_string(&bcc_ptr, ses, nls_cp);
> -               } else
> -               /* BB: is this right? */
> -                       ascii_ssetup_strings(&bcc_ptr, ses, nls_cp);
> -#else /* ! CONFIG_CIFS_UPCALL */
> -               cifs_dbg(VFS, "Kerberos negotiated but upcall support disabled!\n");
> -               rc = -ENOSYS;
> -               goto ssetup_exit;
> -#endif /* CONFIG_CIFS_UPCALL */
> -       } else if (type == RawNTLMSSP) {
> +       if (type == RawNTLMSSP) {
>                 if ((pSMB->req.hdr.Flags2 & SMBFLG2_UNICODE) == 0) {
>                         cifs_dbg(VFS, "NTLMSSP requires Unicode support\n");
>                         rc = -ENOSYS;
> @@ -1282,10 +1376,6 @@ ssetup_ntlmssp_authenticate:
>         }
>
>  ssetup_exit:
> -       if (spnego_key) {
> -               key_invalidate(spnego_key);
> -               key_put(spnego_key);
> -       }
>         kfree(str_area);
>         kfree(ntlmsspblob);
>         ntlmsspblob = NULL;
> --
> 1.8.4.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 4/4] cifs: Separate rawntlmssp auth from CIFS_SessSetup()

[Sachin Prabhu]

Separate rawntlmssp authentication from CIFS_SessSetup(). Also cleanup
CIFS_SessSetup() since we no longer do any auth within it.

Signed-off-by: Sachin Prabhu <sprabhu-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 fs/cifs/sess.c | 437 ++++++++++++++++++++++++++++-----------------------------
 1 file changed, 214 insertions(+), 223 deletions(-)

diff --git a/fs/cifs/sess.c b/fs/cifs/sess.c
index 869ad37..fb86600 100644
--- a/fs/cifs/sess.c
+++ b/fs/cifs/sess.c
@@ -1125,242 +1125,177 @@ sess_auth_kerberos(struct sess_data *sess_data)
 }
 #endif /* ! CONFIG_CIFS_UPCALL */
 
-int
-CIFS_SessSetup(const unsigned int xid, struct cifs_ses *ses,
-	       const struct nls_table *nls_cp)
+static void
+_sess_auth_rawntlmssp(struct sess_data *sess_data, __le32 phase);
+
+static void
+sess_auth_rawntlmssp_negotiate(struct sess_data *sess_data)
+{
+	_sess_auth_rawntlmssp(sess_data, NtLmNegotiate);
+
+}
+
+static void
+sess_auth_rawntlmssp_authenticate(struct sess_data *sess_data)
+{
+	_sess_auth_rawntlmssp(sess_data, NtLmAuthenticate);
+
+}
+
+static void
+_sess_auth_rawntlmssp(struct sess_data *sess_data, __le32 phase)
 {
 	int rc = 0;
-	int wct;
 	struct smb_hdr *smb_buf;
-	char *bcc_ptr;
-	char *str_area;
 	SESSION_SETUP_ANDX *pSMB;
+	char *bcc_ptr;
+	struct cifs_ses *ses = sess_data->ses;
 	__u32 capabilities;
-	__u16 count;
-	int resp_buf_type;
-	struct kvec iov[3];
-	enum securityEnum type;
-	__u16 action, bytes_remaining;
-	__le32 phase = NtLmNegotiate; /* NTLMSSP, if needed, is multistage */
+	__u16 bytes_remaining;
 	u16 blob_len;
 	char *ntlmsspblob = NULL;
-	struct sess_data *sess_data;
 
-	if (ses == NULL) {
-		WARN(1, "%s: ses == NULL!", __func__);
-		return -EINVAL;
-	}
-
-	sess_data = kzalloc(sizeof(struct sess_data), GFP_KERNEL);
-	if (!sess_data)
-		return -ENOMEM;
-	sess_data->xid = xid;
-	sess_data->ses = ses;
-	sess_data->nls_cp = (struct nls_table *) nls_cp;
-
-	type = select_sectype(ses->server, ses->sectype);
-	cifs_dbg(FYI, "sess setup type %d\n", type);
-	if (type == Unspecified) {
-		cifs_dbg(VFS,
-			"Unable to select appropriate authentication method!");
-		return -EINVAL;
-	}
-
-        switch (type) {
-        case LANMAN:
-                sess_auth_lanman(sess_data);
-		goto out;
-	case NTLM:
-                sess_auth_ntlm(sess_data);
-		goto out;
-	case NTLMv2:
-                sess_auth_ntlmv2(sess_data);
-		goto out;
-	case Kerberos:
-                sess_auth_kerberos(sess_data);
+	/* wct = 12 */
+	rc = sess_alloc_buffer(sess_data, 12);
+	if (rc)
 		goto out;
-	default:
-		/* Continue with the rest of the function */
-		break;
-	}
 
-	if (type == RawNTLMSSP) {
-		/* if memory allocation is successful, caller of this function
-		 * frees it.
-		 */
+	pSMB = (SESSION_SETUP_ANDX *)sess_data->iov[0].iov_base;
+	bcc_ptr = sess_data->iov[2].iov_base;
+	capabilities = cifs_ssetup_hdr(ses, pSMB);
+	smb_buf = (struct smb_hdr *)pSMB;
+
+	/*
+	 * Check if ses->ntlmssp is already allocated before allocating
+	 * if memory allocation is successful, caller of this function
+	 * frees it.
+	 */
+	if (!ses->ntlmssp) {
 		ses->ntlmssp = kmalloc(sizeof(struct ntlmssp_auth), GFP_KERNEL);
-		if (!ses->ntlmssp)
-			return -ENOMEM;
+		if (!ses->ntlmssp) {
+			rc = -ENOMEM;
+			goto out;
+		}
 		ses->ntlmssp->sesskey_per_smbsess = false;
-
 	}
 
-ssetup_ntlmssp_authenticate:
-	if (phase == NtLmChallenge)
-		phase = NtLmAuthenticate; /* if ntlmssp, now final phase */
-
-	/* same size: negotiate or auth, NTLMSSP or extended security */
-	wct = 12;
-
-	rc = small_smb_init_no_tc(SMB_COM_SESSION_SETUP_ANDX, wct, ses,
-			    (void **)&smb_buf);
-	if (rc)
-		return rc;
-
-	pSMB = (SESSION_SETUP_ANDX *)smb_buf;
-
-	capabilities = cifs_ssetup_hdr(ses, pSMB);
-
-	/* we will send the SMB in three pieces:
-	a fixed length beginning part, an optional
-	SPNEGO blob (which can be zero length), and a
-	last part which will include the strings
-	and rest of bcc area. This allows us to avoid
-	a large buffer 17K allocation */
-	iov[0].iov_base = (char *)pSMB;
-	iov[0].iov_len = be32_to_cpu(smb_buf->smb_buf_length) + 4;
-
-	/* setting this here allows the code at the end of the function
-	   to free the request buffer if there's an error */
-	resp_buf_type = CIFS_SMALL_BUFFER;
-
-	/* 2000 big enough to fit max user, domain, NOS name etc. */
-	str_area = kmalloc(2000, GFP_KERNEL);
-	if (str_area == NULL) {
-		rc = -ENOMEM;
-		goto ssetup_exit;
+	sess_data->iov[2].iov_base = bcc_ptr;
+	if ((pSMB->req.hdr.Flags2 & SMBFLG2_UNICODE) == 0) {
+		cifs_dbg(VFS, "NTLMSSP requires Unicode support\n");
+		rc = -ENOSYS;
+		goto out;
 	}
-	bcc_ptr = str_area;
-
-	iov[1].iov_base = NULL;
-	iov[1].iov_len = 0;
 
-	if (type == RawNTLMSSP) {
-		if ((pSMB->req.hdr.Flags2 & SMBFLG2_UNICODE) == 0) {
-			cifs_dbg(VFS, "NTLMSSP requires Unicode support\n");
-			rc = -ENOSYS;
-			goto ssetup_exit;
+	cifs_dbg(FYI, "ntlmssp session setup phase %d\n", phase);
+	pSMB->req.hdr.Flags2 |= SMBFLG2_EXT_SEC;
+	capabilities |= CAP_EXTENDED_SECURITY;
+	pSMB->req.Capabilities |= cpu_to_le32(capabilities);
+
+	switch(phase) {
+	case NtLmNegotiate:
+		build_ntlmssp_negotiate_blob(
+			pSMB->req.SecurityBlob, ses);
+		sess_data->iov[1].iov_len = sizeof(NEGOTIATE_MESSAGE);
+		sess_data->iov[1].iov_base = pSMB->req.SecurityBlob;
+		pSMB->req.SecurityBlobLength =
+			cpu_to_le16(sizeof(NEGOTIATE_MESSAGE));
+		break;
+	case NtLmAuthenticate:
+		/*
+		 * 5 is an empirical value, large enough to hold
+		 * authenticate message plus max 10 of av paris,
+		 * domain, user, workstation names, flags, etc.
+		 */
+		ntlmsspblob = kzalloc(
+			5*sizeof(struct _AUTHENTICATE_MESSAGE),
+			GFP_KERNEL);
+		if (!ntlmsspblob) {
+			rc = -ENOMEM;
+			goto out;
 		}
 
-		cifs_dbg(FYI, "ntlmssp session setup phase %d\n", phase);
-		pSMB->req.hdr.Flags2 |= SMBFLG2_EXT_SEC;
-		capabilities |= CAP_EXTENDED_SECURITY;
-		pSMB->req.Capabilities |= cpu_to_le32(capabilities);
-		switch(phase) {
-		case NtLmNegotiate:
-			build_ntlmssp_negotiate_blob(
-				pSMB->req.SecurityBlob, ses);
-			iov[1].iov_len = sizeof(NEGOTIATE_MESSAGE);
-			iov[1].iov_base = pSMB->req.SecurityBlob;
-			pSMB->req.SecurityBlobLength =
-				cpu_to_le16(sizeof(NEGOTIATE_MESSAGE));
-			break;
-		case NtLmAuthenticate:
-			/*
-			 * 5 is an empirical value, large enough to hold
-			 * authenticate message plus max 10 of av paris,
-			 * domain, user, workstation names, flags, etc.
-			 */
-			ntlmsspblob = kzalloc(
-				5*sizeof(struct _AUTHENTICATE_MESSAGE),
-				GFP_KERNEL);
-			if (!ntlmsspblob) {
-				rc = -ENOMEM;
-				goto ssetup_exit;
-			}
-
-			rc = build_ntlmssp_auth_blob(ntlmsspblob,
-						&blob_len, ses, nls_cp);
-			if (rc)
-				goto ssetup_exit;
-			iov[1].iov_len = blob_len;
-			iov[1].iov_base = ntlmsspblob;
-			pSMB->req.SecurityBlobLength = cpu_to_le16(blob_len);
-			/*
-			 * Make sure that we tell the server that we are using
-			 * the uid that it just gave us back on the response
-			 * (challenge)
-			 */
-			smb_buf->Uid = ses->Suid;
-			break;
-		default:
-			cifs_dbg(VFS, "invalid phase %d\n", phase);
-			rc = -ENOSYS;
-			goto ssetup_exit;
-		}
-		/* unicode strings must be word aligned */
-		if ((iov[0].iov_len + iov[1].iov_len) % 2) {
-			*bcc_ptr = 0;
-			bcc_ptr++;
-		}
-		unicode_oslm_strings(&bcc_ptr, nls_cp);
-	} else {
-		cifs_dbg(VFS, "secType %d not supported!\n", type);
+		rc = build_ntlmssp_auth_blob(ntlmsspblob,
+					&blob_len, ses, sess_data->nls_cp);
+		if (rc)
+			goto out_free_ntlmsspblob;
+		sess_data->iov[1].iov_len = blob_len;
+		sess_data->iov[1].iov_base = ntlmsspblob;
+		pSMB->req.SecurityBlobLength = cpu_to_le16(blob_len);
+		/*
+		 * Make sure that we tell the server that we are using
+		 * the uid that it just gave us back on the response
+		 * (challenge)
+		 */
+		smb_buf->Uid = ses->Suid;
+		break;
+	default:
+		cifs_dbg(VFS, "invalid phase %d\n", phase);
 		rc = -ENOSYS;
-		goto ssetup_exit;
+		goto out;
 	}
+	/* unicode strings must be word aligned */
+	if ((sess_data->iov[0].iov_len + sess_data->iov[1].iov_len) % 2) {
+		*bcc_ptr = 0;
+		bcc_ptr++;
+	}
+	unicode_oslm_strings(&bcc_ptr, sess_data->nls_cp);
 
-	iov[2].iov_base = str_area;
-	iov[2].iov_len = (long) bcc_ptr - (long) str_area;
-
-	count = iov[1].iov_len + iov[2].iov_len;
-	smb_buf->smb_buf_length =
-		cpu_to_be32(be32_to_cpu(smb_buf->smb_buf_length) + count);
-
-	put_bcc(count, smb_buf);
+	sess_data->iov[2].iov_len = (long) bcc_ptr -
+					(long) sess_data->iov[2].iov_base;
 
-	rc = SendReceive2(xid, ses, iov, 3 /* num_iovecs */, &resp_buf_type,
-			  CIFS_LOG_ERROR);
-	/* SMB request buf freed in SendReceive2 */
+	rc = sess_sendreceive(sess_data);
 
-	pSMB = (SESSION_SETUP_ANDX *)iov[0].iov_base;
-	smb_buf = (struct smb_hdr *)iov[0].iov_base;
+	pSMB = (SESSION_SETUP_ANDX *)sess_data->iov[0].iov_base;
+	smb_buf = (struct smb_hdr *)sess_data->iov[0].iov_base;
 
-	if ((type == RawNTLMSSP) && (resp_buf_type != CIFS_NO_BUFFER) &&
+	if ((sess_data->buf0_type != CIFS_NO_BUFFER) &&
 	    (smb_buf->Status.CifsError ==
-			cpu_to_le32(NT_STATUS_MORE_PROCESSING_REQUIRED))) {
+	     cpu_to_le32(NT_STATUS_MORE_PROCESSING_REQUIRED))) {
 		if (phase != NtLmNegotiate) {
 			cifs_dbg(VFS, "Unexpected more processing error\n");
-			goto ssetup_exit;
+			goto out;
 		}
 		/* NTLMSSP Negotiate sent now processing challenge (response) */
 		phase = NtLmChallenge; /* process ntlmssp challenge */
 		rc = 0; /* MORE_PROC rc is not an error here, but expected */
 	}
+
 	if (rc)
-		goto ssetup_exit;
+		goto out;
+
 
 	if (smb_buf->WordCount != 4) {
 		rc = -EIO;
 		cifs_dbg(VFS, "bad word count %d\n", smb_buf->WordCount);
-		goto ssetup_exit;
+		goto out_free_ntlmsspblob;
 	}
-	action = le16_to_cpu(pSMB->resp.Action);
-	if (action & GUEST_LOGIN)
+
+	if (le16_to_cpu(pSMB->resp.Action) & GUEST_LOGIN)
 		cifs_dbg(FYI, "Guest login\n"); /* BB mark SesInfo struct? */
+
 	ses->Suid = smb_buf->Uid;   /* UID left in wire format (le) */
 	cifs_dbg(FYI, "UID = %llu\n", ses->Suid);
-	/* response can have either 3 or 4 word count - Samba sends 3 */
-	/* and lanman response is 3 */
+
 	bytes_remaining = get_bcc(smb_buf);
 	bcc_ptr = pByteArea(smb_buf);
 
 	blob_len = le16_to_cpu(pSMB->resp.SecurityBlobLength);
 	if (blob_len > bytes_remaining) {
 		cifs_dbg(VFS, "bad security blob length %d\n",
-			 blob_len);
+				blob_len);
 		rc = -EINVAL;
-		goto ssetup_exit;
+		goto out_free_ntlmsspblob;
 	}
 	if (phase == NtLmChallenge) {
 		rc = decode_ntlmssp_challenge(bcc_ptr, blob_len, ses);
 		/* now goto beginning for ntlmssp authenticate phase */
 		if (rc)
-			goto ssetup_exit;
+			goto out_free_ntlmsspblob;
 	}
 	bcc_ptr += blob_len;
 	bytes_remaining -= blob_len;
 
+
 	/* BB check if Unicode and decode strings */
 	if (bytes_remaining == 0) {
 		/* no string area to decode, do nothing */
@@ -1370,61 +1305,117 @@ ssetup_ntlmssp_authenticate:
 			++bcc_ptr;
 			--bytes_remaining;
 		}
-		decode_unicode_ssetup(&bcc_ptr, bytes_remaining, ses, nls_cp);
+		decode_unicode_ssetup(&bcc_ptr, bytes_remaining, ses,
+				      sess_data->nls_cp);
 	} else {
-		decode_ascii_ssetup(&bcc_ptr, bytes_remaining, ses, nls_cp);
+		decode_ascii_ssetup(&bcc_ptr, bytes_remaining, ses,
+				    sess_data->nls_cp);
 	}
 
-ssetup_exit:
-	kfree(str_area);
-	kfree(ntlmsspblob);
-	ntlmsspblob = NULL;
-	if (resp_buf_type == CIFS_SMALL_BUFFER) {
-		cifs_dbg(FYI, "ssetup freeing small buf %p\n", iov[0].iov_base);
-		cifs_small_buf_release(iov[0].iov_base);
-	} else if (resp_buf_type == CIFS_LARGE_BUFFER)
-		cifs_buf_release(iov[0].iov_base);
-
-	/* if ntlmssp, and negotiate succeeded, proceed to authenticate phase */
-	if ((phase == NtLmChallenge) && (rc == 0))
-		goto ssetup_ntlmssp_authenticate;
-
-	if (!rc) {
-		mutex_lock(&ses->server->srv_mutex);
-		if (!ses->server->session_estab) {
-			if (ses->server->sign) {
-				ses->server->session_key.response =
-					kmemdup(ses->auth_key.response,
-					ses->auth_key.len, GFP_KERNEL);
-				if (!ses->server->session_key.response) {
-					rc = -ENOMEM;
-					mutex_unlock(&ses->server->srv_mutex);
-					goto keycp_exit;
-				}
-				ses->server->session_key.len =
-							ses->auth_key.len;
-			}
-			ses->server->sequence_number = 0x2;
-			ses->server->session_estab = true;
-		}
-		mutex_unlock(&ses->server->srv_mutex);
-
-		cifs_dbg(FYI, "CIFS session established successfully\n");
-		spin_lock(&GlobalMid_Lock);
-		ses->status = CifsGood;
-		ses->need_reconnect = false;
-		spin_unlock(&GlobalMid_Lock);
+out_free_ntlmsspblob:
+	if (ntlmsspblob) {
+		kfree(ntlmsspblob);
+		ntlmsspblob = NULL;
 	}
+out:
+	sess_data->result = rc;
+	sess_free_buffer(sess_data);
 
-keycp_exit:
+	if ((phase == NtLmChallenge) && (rc==0)) {
+		sess_data->func = sess_auth_rawntlmssp_authenticate;
+		return;
+	} else
+		sess_data->func = NULL;
+
+	sess_establish_session(sess_data);
+	/* only free ses->auth_key.response on completion or error */
 	kfree(ses->auth_key.response);
 	ses->auth_key.response = NULL;
 	kfree(ses->ntlmssp);
+	ses->ntlmssp = NULL;
+}
 
-	return rc;
+int select_sec(struct cifs_ses *ses, struct sess_data *sess_data)
+{
+	int type;
 
-out:
+	type = select_sectype(ses->server, ses->sectype);
+	cifs_dbg(FYI, "sess setup type %d\n", type);
+	if (type == Unspecified) {
+		cifs_dbg(VFS,
+			"Unable to select appropriate authentication method!");
+		return -EINVAL;
+	}
+
+	switch (type) {
+	case LANMAN:
+		/* LANMAN and plaintext are less secure and off by default.
+		 * So we make this explicitly be turned on in kconfig (in the
+		 * build) and turned on at runtime (changed from the default)
+		 * in proc/fs/cifs or via mount parm.  Unfortunately this is
+		 * needed for old Win (e.g. Win95), some obscure NAS and OS/2 */
+#ifdef CONFIG_CIFS_WEAK_PW_HASH
+		sess_data->func = sess_auth_lanman;
+		break;
+#else
+		return -EOPNOTSUPP;
+#endif
+	case NTLM:
+		sess_data->func = sess_auth_ntlm;
+		break;
+	case NTLMv2:
+		sess_data->func = sess_auth_ntlmv2;
+		break;
+	case Kerberos:
+#ifdef CONFIG_CIFS_UPCALL
+		sess_data->func = sess_auth_kerberos;
+		break;
+#else
+		cifs_dbg(VFS, "Kerberos negotiated but upcall support disabled!\n");
+		return -ENOSYS;
+		break;
+#endif /* CONFIG_CIFS_UPCALL */
+	case RawNTLMSSP:
+		sess_data->func = sess_auth_rawntlmssp_negotiate;
+		break;
+	default:
+		cifs_dbg(VFS, "secType %d not supported!\n", type);
+		return -ENOSYS;
+	}
+
+	return 0;
+}
+
+int CIFS_SessSetup(const unsigned int xid, struct cifs_ses *ses,
+		    const struct nls_table *nls_cp)
+{
+	int rc = 0;
+	struct sess_data *sess_data;
+
+	if (ses == NULL) {
+		WARN(1, "%s: ses == NULL!", __func__);
+		return -EINVAL;
+	}
+
+	sess_data = kzalloc(sizeof(struct sess_data), GFP_KERNEL);
+	if (!sess_data)
+		return -ENOMEM;
+
+	rc = select_sec(ses, sess_data);
+	if (rc)
+		goto out;
+
+	sess_data->xid = xid;
+	sess_data->ses = ses;
+	sess_data->nls_cp = (struct nls_table *) nls_cp;
+
+	while (sess_data->func)
+		sess_data->func(sess_data);
+
+	/* Store result before we free sess_data */
 	rc = sess_data->result;
+
+out:
 	kfree(sess_data);
 	return rc;
 }
-- 
1.8.4.2

Re: [PATCH 4/4] cifs: Separate rawntlmssp auth from CIFS_SessSetup()

[Shirish Pargaonkar]

Took me a while to understand the while.
Looked at various error conditions and does not look like we will get
stuck in while loop.  Looks correct.

Reviewed-by: Shirish Pargaonkar <spargaonkar-IBi9RG/b67k@public.gmane.org>

On Thu, May 1, 2014 at 7:41 AM, Sachin Prabhu <sprabhu-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> Separate rawntlmssp authentication from CIFS_SessSetup(). Also cleanup
> CIFS_SessSetup() since we no longer do any auth within it.
>
> Signed-off-by: Sachin Prabhu <sprabhu-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
>  fs/cifs/sess.c | 437 ++++++++++++++++++++++++++++-----------------------------
>  1 file changed, 214 insertions(+), 223 deletions(-)
>
> diff --git a/fs/cifs/sess.c b/fs/cifs/sess.c
> index 869ad37..fb86600 100644
> --- a/fs/cifs/sess.c
> +++ b/fs/cifs/sess.c
> @@ -1125,242 +1125,177 @@ sess_auth_kerberos(struct sess_data *sess_data)
>  }
>  #endif /* ! CONFIG_CIFS_UPCALL */
>
> -int
> -CIFS_SessSetup(const unsigned int xid, struct cifs_ses *ses,
> -              const struct nls_table *nls_cp)
> +static void
> +_sess_auth_rawntlmssp(struct sess_data *sess_data, __le32 phase);
> +
> +static void
> +sess_auth_rawntlmssp_negotiate(struct sess_data *sess_data)
> +{
> +       _sess_auth_rawntlmssp(sess_data, NtLmNegotiate);
> +
> +}
> +
> +static void
> +sess_auth_rawntlmssp_authenticate(struct sess_data *sess_data)
> +{
> +       _sess_auth_rawntlmssp(sess_data, NtLmAuthenticate);
> +
> +}
> +
> +static void
> +_sess_auth_rawntlmssp(struct sess_data *sess_data, __le32 phase)
>  {
>         int rc = 0;
> -       int wct;
>         struct smb_hdr *smb_buf;
> -       char *bcc_ptr;
> -       char *str_area;
>         SESSION_SETUP_ANDX *pSMB;
> +       char *bcc_ptr;
> +       struct cifs_ses *ses = sess_data->ses;
>         __u32 capabilities;
> -       __u16 count;
> -       int resp_buf_type;
> -       struct kvec iov[3];
> -       enum securityEnum type;
> -       __u16 action, bytes_remaining;
> -       __le32 phase = NtLmNegotiate; /* NTLMSSP, if needed, is multistage */
> +       __u16 bytes_remaining;
>         u16 blob_len;
>         char *ntlmsspblob = NULL;
> -       struct sess_data *sess_data;
>
> -       if (ses == NULL) {
> -               WARN(1, "%s: ses == NULL!", __func__);
> -               return -EINVAL;
> -       }
> -
> -       sess_data = kzalloc(sizeof(struct sess_data), GFP_KERNEL);
> -       if (!sess_data)
> -               return -ENOMEM;
> -       sess_data->xid = xid;
> -       sess_data->ses = ses;
> -       sess_data->nls_cp = (struct nls_table *) nls_cp;
> -
> -       type = select_sectype(ses->server, ses->sectype);
> -       cifs_dbg(FYI, "sess setup type %d\n", type);
> -       if (type == Unspecified) {
> -               cifs_dbg(VFS,
> -                       "Unable to select appropriate authentication method!");
> -               return -EINVAL;
> -       }
> -
> -        switch (type) {
> -        case LANMAN:
> -                sess_auth_lanman(sess_data);
> -               goto out;
> -       case NTLM:
> -                sess_auth_ntlm(sess_data);
> -               goto out;
> -       case NTLMv2:
> -                sess_auth_ntlmv2(sess_data);
> -               goto out;
> -       case Kerberos:
> -                sess_auth_kerberos(sess_data);
> +       /* wct = 12 */
> +       rc = sess_alloc_buffer(sess_data, 12);
> +       if (rc)
>                 goto out;
> -       default:
> -               /* Continue with the rest of the function */
> -               break;
> -       }
>
> -       if (type == RawNTLMSSP) {
> -               /* if memory allocation is successful, caller of this function
> -                * frees it.
> -                */
> +       pSMB = (SESSION_SETUP_ANDX *)sess_data->iov[0].iov_base;
> +       bcc_ptr = sess_data->iov[2].iov_base;
> +       capabilities = cifs_ssetup_hdr(ses, pSMB);
> +       smb_buf = (struct smb_hdr *)pSMB;
> +
> +       /*
> +        * Check if ses->ntlmssp is already allocated before allocating
> +        * if memory allocation is successful, caller of this function
> +        * frees it.
> +        */
> +       if (!ses->ntlmssp) {
>                 ses->ntlmssp = kmalloc(sizeof(struct ntlmssp_auth), GFP_KERNEL);
> -               if (!ses->ntlmssp)
> -                       return -ENOMEM;
> +               if (!ses->ntlmssp) {
> +                       rc = -ENOMEM;
> +                       goto out;
> +               }
>                 ses->ntlmssp->sesskey_per_smbsess = false;
> -
>         }
>
> -ssetup_ntlmssp_authenticate:
> -       if (phase == NtLmChallenge)
> -               phase = NtLmAuthenticate; /* if ntlmssp, now final phase */
> -
> -       /* same size: negotiate or auth, NTLMSSP or extended security */
> -       wct = 12;
> -
> -       rc = small_smb_init_no_tc(SMB_COM_SESSION_SETUP_ANDX, wct, ses,
> -                           (void **)&smb_buf);
> -       if (rc)
> -               return rc;
> -
> -       pSMB = (SESSION_SETUP_ANDX *)smb_buf;
> -
> -       capabilities = cifs_ssetup_hdr(ses, pSMB);
> -
> -       /* we will send the SMB in three pieces:
> -       a fixed length beginning part, an optional
> -       SPNEGO blob (which can be zero length), and a
> -       last part which will include the strings
> -       and rest of bcc area. This allows us to avoid
> -       a large buffer 17K allocation */
> -       iov[0].iov_base = (char *)pSMB;
> -       iov[0].iov_len = be32_to_cpu(smb_buf->smb_buf_length) + 4;
> -
> -       /* setting this here allows the code at the end of the function
> -          to free the request buffer if there's an error */
> -       resp_buf_type = CIFS_SMALL_BUFFER;
> -
> -       /* 2000 big enough to fit max user, domain, NOS name etc. */
> -       str_area = kmalloc(2000, GFP_KERNEL);
> -       if (str_area == NULL) {
> -               rc = -ENOMEM;
> -               goto ssetup_exit;
> +       sess_data->iov[2].iov_base = bcc_ptr;
> +       if ((pSMB->req.hdr.Flags2 & SMBFLG2_UNICODE) == 0) {
> +               cifs_dbg(VFS, "NTLMSSP requires Unicode support\n");
> +               rc = -ENOSYS;
> +               goto out;
>         }
> -       bcc_ptr = str_area;
> -
> -       iov[1].iov_base = NULL;
> -       iov[1].iov_len = 0;
>
> -       if (type == RawNTLMSSP) {
> -               if ((pSMB->req.hdr.Flags2 & SMBFLG2_UNICODE) == 0) {
> -                       cifs_dbg(VFS, "NTLMSSP requires Unicode support\n");
> -                       rc = -ENOSYS;
> -                       goto ssetup_exit;
> +       cifs_dbg(FYI, "ntlmssp session setup phase %d\n", phase);
> +       pSMB->req.hdr.Flags2 |= SMBFLG2_EXT_SEC;
> +       capabilities |= CAP_EXTENDED_SECURITY;
> +       pSMB->req.Capabilities |= cpu_to_le32(capabilities);
> +
> +       switch(phase) {
> +       case NtLmNegotiate:
> +               build_ntlmssp_negotiate_blob(
> +                       pSMB->req.SecurityBlob, ses);
> +               sess_data->iov[1].iov_len = sizeof(NEGOTIATE_MESSAGE);
> +               sess_data->iov[1].iov_base = pSMB->req.SecurityBlob;
> +               pSMB->req.SecurityBlobLength =
> +                       cpu_to_le16(sizeof(NEGOTIATE_MESSAGE));
> +               break;
> +       case NtLmAuthenticate:
> +               /*
> +                * 5 is an empirical value, large enough to hold
> +                * authenticate message plus max 10 of av paris,
> +                * domain, user, workstation names, flags, etc.
> +                */
> +               ntlmsspblob = kzalloc(
> +                       5*sizeof(struct _AUTHENTICATE_MESSAGE),
> +                       GFP_KERNEL);
> +               if (!ntlmsspblob) {
> +                       rc = -ENOMEM;
> +                       goto out;
>                 }
>
> -               cifs_dbg(FYI, "ntlmssp session setup phase %d\n", phase);
> -               pSMB->req.hdr.Flags2 |= SMBFLG2_EXT_SEC;
> -               capabilities |= CAP_EXTENDED_SECURITY;
> -               pSMB->req.Capabilities |= cpu_to_le32(capabilities);
> -               switch(phase) {
> -               case NtLmNegotiate:
> -                       build_ntlmssp_negotiate_blob(
> -                               pSMB->req.SecurityBlob, ses);
> -                       iov[1].iov_len = sizeof(NEGOTIATE_MESSAGE);
> -                       iov[1].iov_base = pSMB->req.SecurityBlob;
> -                       pSMB->req.SecurityBlobLength =
> -                               cpu_to_le16(sizeof(NEGOTIATE_MESSAGE));
> -                       break;
> -               case NtLmAuthenticate:
> -                       /*
> -                        * 5 is an empirical value, large enough to hold
> -                        * authenticate message plus max 10 of av paris,
> -                        * domain, user, workstation names, flags, etc.
> -                        */
> -                       ntlmsspblob = kzalloc(
> -                               5*sizeof(struct _AUTHENTICATE_MESSAGE),
> -                               GFP_KERNEL);
> -                       if (!ntlmsspblob) {
> -                               rc = -ENOMEM;
> -                               goto ssetup_exit;
> -                       }
> -
> -                       rc = build_ntlmssp_auth_blob(ntlmsspblob,
> -                                               &blob_len, ses, nls_cp);
> -                       if (rc)
> -                               goto ssetup_exit;
> -                       iov[1].iov_len = blob_len;
> -                       iov[1].iov_base = ntlmsspblob;
> -                       pSMB->req.SecurityBlobLength = cpu_to_le16(blob_len);
> -                       /*
> -                        * Make sure that we tell the server that we are using
> -                        * the uid that it just gave us back on the response
> -                        * (challenge)
> -                        */
> -                       smb_buf->Uid = ses->Suid;
> -                       break;
> -               default:
> -                       cifs_dbg(VFS, "invalid phase %d\n", phase);
> -                       rc = -ENOSYS;
> -                       goto ssetup_exit;
> -               }
> -               /* unicode strings must be word aligned */
> -               if ((iov[0].iov_len + iov[1].iov_len) % 2) {
> -                       *bcc_ptr = 0;
> -                       bcc_ptr++;
> -               }
> -               unicode_oslm_strings(&bcc_ptr, nls_cp);
> -       } else {
> -               cifs_dbg(VFS, "secType %d not supported!\n", type);
> +               rc = build_ntlmssp_auth_blob(ntlmsspblob,
> +                                       &blob_len, ses, sess_data->nls_cp);
> +               if (rc)
> +                       goto out_free_ntlmsspblob;
> +               sess_data->iov[1].iov_len = blob_len;
> +               sess_data->iov[1].iov_base = ntlmsspblob;
> +               pSMB->req.SecurityBlobLength = cpu_to_le16(blob_len);
> +               /*
> +                * Make sure that we tell the server that we are using
> +                * the uid that it just gave us back on the response
> +                * (challenge)
> +                */
> +               smb_buf->Uid = ses->Suid;
> +               break;
> +       default:
> +               cifs_dbg(VFS, "invalid phase %d\n", phase);
>                 rc = -ENOSYS;
> -               goto ssetup_exit;
> +               goto out;
>         }
> +       /* unicode strings must be word aligned */
> +       if ((sess_data->iov[0].iov_len + sess_data->iov[1].iov_len) % 2) {
> +               *bcc_ptr = 0;
> +               bcc_ptr++;
> +       }
> +       unicode_oslm_strings(&bcc_ptr, sess_data->nls_cp);
>
> -       iov[2].iov_base = str_area;
> -       iov[2].iov_len = (long) bcc_ptr - (long) str_area;
> -
> -       count = iov[1].iov_len + iov[2].iov_len;
> -       smb_buf->smb_buf_length =
> -               cpu_to_be32(be32_to_cpu(smb_buf->smb_buf_length) + count);
> -
> -       put_bcc(count, smb_buf);
> +       sess_data->iov[2].iov_len = (long) bcc_ptr -
> +                                       (long) sess_data->iov[2].iov_base;
>
> -       rc = SendReceive2(xid, ses, iov, 3 /* num_iovecs */, &resp_buf_type,
> -                         CIFS_LOG_ERROR);
> -       /* SMB request buf freed in SendReceive2 */
> +       rc = sess_sendreceive(sess_data);
>
> -       pSMB = (SESSION_SETUP_ANDX *)iov[0].iov_base;
> -       smb_buf = (struct smb_hdr *)iov[0].iov_base;
> +       pSMB = (SESSION_SETUP_ANDX *)sess_data->iov[0].iov_base;
> +       smb_buf = (struct smb_hdr *)sess_data->iov[0].iov_base;
>
> -       if ((type == RawNTLMSSP) && (resp_buf_type != CIFS_NO_BUFFER) &&
> +       if ((sess_data->buf0_type != CIFS_NO_BUFFER) &&
>             (smb_buf->Status.CifsError ==
> -                       cpu_to_le32(NT_STATUS_MORE_PROCESSING_REQUIRED))) {
> +            cpu_to_le32(NT_STATUS_MORE_PROCESSING_REQUIRED))) {
>                 if (phase != NtLmNegotiate) {
>                         cifs_dbg(VFS, "Unexpected more processing error\n");
> -                       goto ssetup_exit;
> +                       goto out;
>                 }
>                 /* NTLMSSP Negotiate sent now processing challenge (response) */
>                 phase = NtLmChallenge; /* process ntlmssp challenge */
>                 rc = 0; /* MORE_PROC rc is not an error here, but expected */
>         }
> +
>         if (rc)
> -               goto ssetup_exit;
> +               goto out;
> +
>
>         if (smb_buf->WordCount != 4) {
>                 rc = -EIO;
>                 cifs_dbg(VFS, "bad word count %d\n", smb_buf->WordCount);
> -               goto ssetup_exit;
> +               goto out_free_ntlmsspblob;
>         }
> -       action = le16_to_cpu(pSMB->resp.Action);
> -       if (action & GUEST_LOGIN)
> +
> +       if (le16_to_cpu(pSMB->resp.Action) & GUEST_LOGIN)
>                 cifs_dbg(FYI, "Guest login\n"); /* BB mark SesInfo struct? */
> +
>         ses->Suid = smb_buf->Uid;   /* UID left in wire format (le) */
>         cifs_dbg(FYI, "UID = %llu\n", ses->Suid);
> -       /* response can have either 3 or 4 word count - Samba sends 3 */
> -       /* and lanman response is 3 */
> +
>         bytes_remaining = get_bcc(smb_buf);
>         bcc_ptr = pByteArea(smb_buf);
>
>         blob_len = le16_to_cpu(pSMB->resp.SecurityBlobLength);
>         if (blob_len > bytes_remaining) {
>                 cifs_dbg(VFS, "bad security blob length %d\n",
> -                        blob_len);
> +                               blob_len);
>                 rc = -EINVAL;
> -               goto ssetup_exit;
> +               goto out_free_ntlmsspblob;
>         }
>         if (phase == NtLmChallenge) {
>                 rc = decode_ntlmssp_challenge(bcc_ptr, blob_len, ses);
>                 /* now goto beginning for ntlmssp authenticate phase */
>                 if (rc)
> -                       goto ssetup_exit;
> +                       goto out_free_ntlmsspblob;
>         }
>         bcc_ptr += blob_len;
>         bytes_remaining -= blob_len;
>
> +
>         /* BB check if Unicode and decode strings */
>         if (bytes_remaining == 0) {
>                 /* no string area to decode, do nothing */
> @@ -1370,61 +1305,117 @@ ssetup_ntlmssp_authenticate:
>                         ++bcc_ptr;
>                         --bytes_remaining;
>                 }
> -               decode_unicode_ssetup(&bcc_ptr, bytes_remaining, ses, nls_cp);
> +               decode_unicode_ssetup(&bcc_ptr, bytes_remaining, ses,
> +                                     sess_data->nls_cp);
>         } else {
> -               decode_ascii_ssetup(&bcc_ptr, bytes_remaining, ses, nls_cp);
> +               decode_ascii_ssetup(&bcc_ptr, bytes_remaining, ses,
> +                                   sess_data->nls_cp);
>         }
>
> -ssetup_exit:
> -       kfree(str_area);
> -       kfree(ntlmsspblob);
> -       ntlmsspblob = NULL;
> -       if (resp_buf_type == CIFS_SMALL_BUFFER) {
> -               cifs_dbg(FYI, "ssetup freeing small buf %p\n", iov[0].iov_base);
> -               cifs_small_buf_release(iov[0].iov_base);
> -       } else if (resp_buf_type == CIFS_LARGE_BUFFER)
> -               cifs_buf_release(iov[0].iov_base);
> -
> -       /* if ntlmssp, and negotiate succeeded, proceed to authenticate phase */
> -       if ((phase == NtLmChallenge) && (rc == 0))
> -               goto ssetup_ntlmssp_authenticate;
> -
> -       if (!rc) {
> -               mutex_lock(&ses->server->srv_mutex);
> -               if (!ses->server->session_estab) {
> -                       if (ses->server->sign) {
> -                               ses->server->session_key.response =
> -                                       kmemdup(ses->auth_key.response,
> -                                       ses->auth_key.len, GFP_KERNEL);
> -                               if (!ses->server->session_key.response) {
> -                                       rc = -ENOMEM;
> -                                       mutex_unlock(&ses->server->srv_mutex);
> -                                       goto keycp_exit;
> -                               }
> -                               ses->server->session_key.len =
> -                                                       ses->auth_key.len;
> -                       }
> -                       ses->server->sequence_number = 0x2;
> -                       ses->server->session_estab = true;
> -               }
> -               mutex_unlock(&ses->server->srv_mutex);
> -
> -               cifs_dbg(FYI, "CIFS session established successfully\n");
> -               spin_lock(&GlobalMid_Lock);
> -               ses->status = CifsGood;
> -               ses->need_reconnect = false;
> -               spin_unlock(&GlobalMid_Lock);
> +out_free_ntlmsspblob:
> +       if (ntlmsspblob) {
> +               kfree(ntlmsspblob);
> +               ntlmsspblob = NULL;
>         }
> +out:
> +       sess_data->result = rc;
> +       sess_free_buffer(sess_data);
>
> -keycp_exit:
> +       if ((phase == NtLmChallenge) && (rc==0)) {
> +               sess_data->func = sess_auth_rawntlmssp_authenticate;
> +               return;
> +       } else
> +               sess_data->func = NULL;
> +
> +       sess_establish_session(sess_data);
> +       /* only free ses->auth_key.response on completion or error */
>         kfree(ses->auth_key.response);
>         ses->auth_key.response = NULL;
>         kfree(ses->ntlmssp);
> +       ses->ntlmssp = NULL;
> +}
>
> -       return rc;
> +int select_sec(struct cifs_ses *ses, struct sess_data *sess_data)
> +{
> +       int type;
>
> -out:
> +       type = select_sectype(ses->server, ses->sectype);
> +       cifs_dbg(FYI, "sess setup type %d\n", type);
> +       if (type == Unspecified) {
> +               cifs_dbg(VFS,
> +                       "Unable to select appropriate authentication method!");
> +               return -EINVAL;
> +       }
> +
> +       switch (type) {
> +       case LANMAN:
> +               /* LANMAN and plaintext are less secure and off by default.
> +                * So we make this explicitly be turned on in kconfig (in the
> +                * build) and turned on at runtime (changed from the default)
> +                * in proc/fs/cifs or via mount parm.  Unfortunately this is
> +                * needed for old Win (e.g. Win95), some obscure NAS and OS/2 */
> +#ifdef CONFIG_CIFS_WEAK_PW_HASH
> +               sess_data->func = sess_auth_lanman;
> +               break;
> +#else
> +               return -EOPNOTSUPP;
> +#endif
> +       case NTLM:
> +               sess_data->func = sess_auth_ntlm;
> +               break;
> +       case NTLMv2:
> +               sess_data->func = sess_auth_ntlmv2;
> +               break;
> +       case Kerberos:
> +#ifdef CONFIG_CIFS_UPCALL
> +               sess_data->func = sess_auth_kerberos;
> +               break;
> +#else
> +               cifs_dbg(VFS, "Kerberos negotiated but upcall support disabled!\n");
> +               return -ENOSYS;
> +               break;
> +#endif /* CONFIG_CIFS_UPCALL */
> +       case RawNTLMSSP:
> +               sess_data->func = sess_auth_rawntlmssp_negotiate;
> +               break;
> +       default:
> +               cifs_dbg(VFS, "secType %d not supported!\n", type);
> +               return -ENOSYS;
> +       }
> +
> +       return 0;
> +}
> +
> +int CIFS_SessSetup(const unsigned int xid, struct cifs_ses *ses,
> +                   const struct nls_table *nls_cp)
> +{
> +       int rc = 0;
> +       struct sess_data *sess_data;
> +
> +       if (ses == NULL) {
> +               WARN(1, "%s: ses == NULL!", __func__);
> +               return -EINVAL;
> +       }
> +
> +       sess_data = kzalloc(sizeof(struct sess_data), GFP_KERNEL);
> +       if (!sess_data)
> +               return -ENOMEM;
> +
> +       rc = select_sec(ses, sess_data);
> +       if (rc)
> +               goto out;
> +
> +       sess_data->xid = xid;
> +       sess_data->ses = ses;
> +       sess_data->nls_cp = (struct nls_table *) nls_cp;
> +
> +       while (sess_data->func)
> +               sess_data->func(sess_data);
> +
> +       /* Store result before we free sess_data */
>         rc = sess_data->result;
> +
> +out:
>         kfree(sess_data);
>         return rc;
>  }
> --
> 1.8.4.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/4 v2] Split CIFS_SessSetup()

[Simo]

On Thu, 2014-05-01 at 13:41 +0100, Sachin Prabhu wrote:
> I am investigating the possibility of adding support for gss-proxy in the
> cifs client module. As part of that investigation, I was looking at the
> CIFS_SessSetup() function. This is a long function which handles multiple auths
> using if conditions and switch statements. The code struction makes it
> difficult to modify of add support for new authentication mechanisms.
> 
> The proposal is to split the various authentication code into its own separate
> functions. This increases the lines of code but will simplify maintenance and
> addition of new auth methods.
> 
> The short term goal is to add gss-proxy support for kerberos authentication.
> This will require to and fro communication with the gss-proxy module over a
> unix socket.
> 
> Further long term goals are 
> 1) Add support for NTLMSSP using SPNEGO,
> 2) Allow clients to negotiate which authentication mechanism to use using SPNEGO.
> 
> I would like the opinion of this list about these proposed changes.
> Is there any part of the code which needs changing?
> 
> V2:
> - Ensure that sess_data allocated on intermediate patches are freed.
> - Remove ifdef-endif blocks from the switch statement in CIFS_SessSetup().
> 
> Sachin Prabhu (4):
>   cifs: Split lanman auth from CIFS_SessSetup()
>   cifs: Split ntlm and ntlmv2 authentication methods off
>     CIFS_SessSetup()
>   cifs: Split Kerberos authentication off CIFS_SessSetup()
>   cifs: Separate rawntlmssp auth from CIFS_SessSetup()
> 
>  fs/cifs/sess.c | 1142 +++++++++++++++++++++++++++++++++++++++-----------------
>  1 file changed, 799 insertions(+), 343 deletions(-)
> 

I had a cursory look at the patches, and I think in general the
direction is good, and will make it much easier to use gss-proxy going
forward.

Simo.

[PATCH 0/4 v3] Split CIFS_SessSetup()

[Sachin Prabhu]

I am investigating the possibility of adding support for gss-proxy in the
cifs client module. As part of that investigation, I was looking at the
CIFS_SessSetup() function. This is a long function which handles multiple auths
using if conditions and switch statements. The code struction makes it
difficult to modify of add support for new authentication mechanisms.

The proposal is to split the various authentication code into its own separate
functions. This increases the lines of code but will simplify maintenance and
addition of new auth methods.

The short term goal is to add gss-proxy support for kerberos authentication.
This will require to and fro communication with the gss-proxy module over a
unix socket.

Further long term goals are 
1) Add support for NTLMSSP using SPNEGO,
2) Allow clients to negotiate which authentication mechanism to use using SPNEGO.

I would like the opinion of this list about these proposed changes.
Is there any part of the code which needs changing?

V2:
- Ensure that sess_data allocated on intermediate patches are freed.
- Remove ifdef-endif blocks from the switch statement in CIFS_SessSetup().

V3:
- Fix whitespace errors in some patches.
- Change sess_establish_session to return errors. Call this function only in
  case of successful establish session calls to the server.
- Delete an incorrect comment in case of rawntlmssp authentication.
- Delete an if statement before kfree(ntlmssp) in rawntlmssp authentication.


Sachin Prabhu (4):
  cifs: Split lanman auth from CIFS_SessSetup()
  cifs: Split ntlm and ntlmv2 authentication methods off
    CIFS_SessSetup()
  cifs: Split Kerberos authentication off CIFS_SessSetup()
  cifs: Separate rawntlmssp auth from CIFS_SessSetup()

 fs/cifs/sess.c | 1138 +++++++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 796 insertions(+), 342 deletions(-)

-- 
1.8.4.2

[PATCH 2/4] cifs: Split ntlm and ntlmv2 authentication methods off CIFS_SessSetup()

[Sachin Prabhu]

Signed-off-by: Sachin Prabhu <sprabhu-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 fs/cifs/sess.c | 316 ++++++++++++++++++++++++++++++++++++++++++---------------
 1 file changed, 232 insertions(+), 84 deletions(-)

diff --git a/fs/cifs/sess.c b/fs/cifs/sess.c
index c6aa60b..2be1ecb 100644
--- a/fs/cifs/sess.c
+++ b/fs/cifs/sess.c
@@ -764,6 +764,216 @@ sess_auth_lanman(struct sess_data *sess_data)
 }
 #endif
 
+static void
+sess_auth_ntlm(struct sess_data *sess_data)
+{
+	int rc = 0;
+	struct smb_hdr *smb_buf;
+	SESSION_SETUP_ANDX *pSMB;
+	char *bcc_ptr;
+	struct cifs_ses *ses = sess_data->ses;
+	__u32 capabilities;
+	__u16 bytes_remaining;
+
+	/* old style NTLM sessionsetup */
+	/* wct = 13 */
+	rc = sess_alloc_buffer(sess_data, 13);
+	if (rc)
+		goto out;
+
+	pSMB = (SESSION_SETUP_ANDX *)sess_data->iov[0].iov_base;
+	bcc_ptr = sess_data->iov[2].iov_base;
+	capabilities = cifs_ssetup_hdr(ses, pSMB);
+
+	pSMB->req_no_secext.Capabilities = cpu_to_le32(capabilities);
+	pSMB->req_no_secext.CaseInsensitivePasswordLength =
+			cpu_to_le16(CIFS_AUTH_RESP_SIZE);
+	pSMB->req_no_secext.CaseSensitivePasswordLength =
+			cpu_to_le16(CIFS_AUTH_RESP_SIZE);
+
+	/* calculate ntlm response and session key */
+	rc = setup_ntlm_response(ses, sess_data->nls_cp);
+	if (rc) {
+		cifs_dbg(VFS, "Error %d during NTLM authentication\n",
+				 rc);
+		goto out;
+	}
+
+	/* copy ntlm response */
+	memcpy(bcc_ptr, ses->auth_key.response + CIFS_SESS_KEY_SIZE,
+			CIFS_AUTH_RESP_SIZE);
+	bcc_ptr += CIFS_AUTH_RESP_SIZE;
+	memcpy(bcc_ptr, ses->auth_key.response + CIFS_SESS_KEY_SIZE,
+			CIFS_AUTH_RESP_SIZE);
+	bcc_ptr += CIFS_AUTH_RESP_SIZE;
+
+	if (ses->capabilities & CAP_UNICODE) {
+		/* unicode strings must be word aligned */
+		if (sess_data->iov[0].iov_len % 2) {
+			*bcc_ptr = 0;
+			bcc_ptr++;
+		}
+		unicode_ssetup_strings(&bcc_ptr, ses, sess_data->nls_cp);
+	} else {
+		ascii_ssetup_strings(&bcc_ptr, ses, sess_data->nls_cp);
+	}
+
+
+	sess_data->iov[2].iov_len = (long) bcc_ptr -
+			(long) sess_data->iov[2].iov_base;
+
+	rc = sess_sendreceive(sess_data);
+	if (rc)
+		goto out;
+
+	pSMB = (SESSION_SETUP_ANDX *)sess_data->iov[0].iov_base;
+	smb_buf = (struct smb_hdr *)sess_data->iov[0].iov_base;
+
+	if (smb_buf->WordCount != 3) {
+		rc = -EIO;
+		cifs_dbg(VFS, "bad word count %d\n", smb_buf->WordCount);
+		goto out;
+	}
+
+	if (le16_to_cpu(pSMB->resp.Action) & GUEST_LOGIN)
+		cifs_dbg(FYI, "Guest login\n"); /* BB mark SesInfo struct? */
+
+	ses->Suid = smb_buf->Uid;   /* UID left in wire format (le) */
+	cifs_dbg(FYI, "UID = %llu\n", ses->Suid);
+
+	bytes_remaining = get_bcc(smb_buf);
+	bcc_ptr = pByteArea(smb_buf);
+
+	/* BB check if Unicode and decode strings */
+	if (bytes_remaining == 0) {
+		/* no string area to decode, do nothing */
+	} else if (smb_buf->Flags2 & SMBFLG2_UNICODE) {
+		/* unicode string area must be word-aligned */
+		if (((unsigned long) bcc_ptr - (unsigned long) smb_buf) % 2) {
+			++bcc_ptr;
+			--bytes_remaining;
+		}
+		decode_unicode_ssetup(&bcc_ptr, bytes_remaining, ses,
+				      sess_data->nls_cp);
+	} else {
+		decode_ascii_ssetup(&bcc_ptr, bytes_remaining, ses,
+				    sess_data->nls_cp);
+	}
+
+	rc = sess_establish_session(sess_data);
+out:
+	sess_data->result = rc;
+	sess_data->func = NULL;
+	sess_free_buffer(sess_data);
+	kfree(ses->auth_key.response);
+	ses->auth_key.response = NULL;
+}
+
+static void
+sess_auth_ntlmv2(struct sess_data *sess_data)
+{
+	int rc = 0;
+	struct smb_hdr *smb_buf;
+	SESSION_SETUP_ANDX *pSMB;
+	char *bcc_ptr;
+	struct cifs_ses *ses = sess_data->ses;
+	__u32 capabilities;
+	__u16 bytes_remaining;
+
+	/* old style NTLM sessionsetup */
+	/* wct = 13 */
+	rc = sess_alloc_buffer(sess_data, 13);
+	if (rc)
+		goto out;
+
+	pSMB = (SESSION_SETUP_ANDX *)sess_data->iov[0].iov_base;
+	bcc_ptr = sess_data->iov[2].iov_base;
+	capabilities = cifs_ssetup_hdr(ses, pSMB);
+
+	pSMB->req_no_secext.Capabilities = cpu_to_le32(capabilities);
+
+	/* LM2 password would be here if we supported it */
+	pSMB->req_no_secext.CaseInsensitivePasswordLength = 0;
+
+	/* calculate nlmv2 response and session key */
+	rc = setup_ntlmv2_rsp(ses, sess_data->nls_cp);
+	if (rc) {
+		cifs_dbg(VFS, "Error %d during NTLMv2 authentication\n", rc);
+		goto out;
+	}
+
+	memcpy(bcc_ptr, ses->auth_key.response + CIFS_SESS_KEY_SIZE,
+			ses->auth_key.len - CIFS_SESS_KEY_SIZE);
+	bcc_ptr += ses->auth_key.len - CIFS_SESS_KEY_SIZE;
+
+	/* set case sensitive password length after tilen may get
+	 * assigned, tilen is 0 otherwise.
+	 */
+	pSMB->req_no_secext.CaseSensitivePasswordLength =
+		cpu_to_le16(ses->auth_key.len - CIFS_SESS_KEY_SIZE);
+
+	if (ses->capabilities & CAP_UNICODE) {
+		if (sess_data->iov[0].iov_len % 2) {
+			*bcc_ptr = 0;
+			bcc_ptr++;
+		}
+		unicode_ssetup_strings(&bcc_ptr, ses, sess_data->nls_cp);
+	} else {
+		ascii_ssetup_strings(&bcc_ptr, ses, sess_data->nls_cp);
+	}
+
+
+	sess_data->iov[2].iov_len = (long) bcc_ptr -
+			(long) sess_data->iov[2].iov_base;
+
+	rc = sess_sendreceive(sess_data);
+	if (rc)
+		goto out;
+
+	pSMB = (SESSION_SETUP_ANDX *)sess_data->iov[0].iov_base;
+	smb_buf = (struct smb_hdr *)sess_data->iov[0].iov_base;
+
+	if (smb_buf->WordCount != 3) {
+		rc = -EIO;
+		cifs_dbg(VFS, "bad word count %d\n", smb_buf->WordCount);
+		goto out;
+	}
+
+	if (le16_to_cpu(pSMB->resp.Action) & GUEST_LOGIN)
+		cifs_dbg(FYI, "Guest login\n"); /* BB mark SesInfo struct? */
+
+	ses->Suid = smb_buf->Uid;   /* UID left in wire format (le) */
+	cifs_dbg(FYI, "UID = %llu\n", ses->Suid);
+
+	bytes_remaining = get_bcc(smb_buf);
+	bcc_ptr = pByteArea(smb_buf);
+
+	/* BB check if Unicode and decode strings */
+	if (bytes_remaining == 0) {
+		/* no string area to decode, do nothing */
+	} else if (smb_buf->Flags2 & SMBFLG2_UNICODE) {
+		/* unicode string area must be word-aligned */
+		if (((unsigned long) bcc_ptr - (unsigned long) smb_buf) % 2) {
+			++bcc_ptr;
+			--bytes_remaining;
+		}
+		decode_unicode_ssetup(&bcc_ptr, bytes_remaining, ses,
+				      sess_data->nls_cp);
+	} else {
+		decode_ascii_ssetup(&bcc_ptr, bytes_remaining, ses,
+				    sess_data->nls_cp);
+	}
+
+	rc = sess_establish_session(sess_data);
+out:
+	sess_data->result = rc;
+	sess_data->func = NULL;
+	sess_free_buffer(sess_data);
+	kfree(ses->auth_key.response);
+	ses->auth_key.response = NULL;
+}
+
+
 int
 CIFS_SessSetup(const unsigned int xid, struct cifs_ses *ses,
 	       const struct nls_table *nls_cp)
@@ -810,6 +1020,12 @@ CIFS_SessSetup(const unsigned int xid, struct cifs_ses *ses,
 	case LANMAN:
 		sess_auth_lanman(sess_data);
 		goto out;
+	case NTLM:
+		sess_auth_ntlm(sess_data);
+		goto out;
+	case NTLMv2:
+		sess_auth_ntlmv2(sess_data);
+		goto out;
 	default:
 		/* Continue with the rest of the function */
 		break;
@@ -830,11 +1046,8 @@ ssetup_ntlmssp_authenticate:
 	if (phase == NtLmChallenge)
 		phase = NtLmAuthenticate; /* if ntlmssp, now final phase */
 
-	if ((type == NTLM) || (type == NTLMv2)) {
-		/* For NTLMv2 failures eventually may need to retry NTLM */
-		wct = 13; /* old style NTLM sessionsetup */
-	} else /* same size: negotiate or auth, NTLMSSP or extended security */
-		wct = 12;
+	/* same size: negotiate or auth, NTLMSSP or extended security */
+	wct = 12;
 
 	rc = small_smb_init_no_tc(SMB_COM_SESSION_SETUP_ANDX, wct, ses,
 			    (void **)&smb_buf);
@@ -869,70 +1082,7 @@ ssetup_ntlmssp_authenticate:
 	iov[1].iov_base = NULL;
 	iov[1].iov_len = 0;
 
-	if (type == NTLM) {
-		pSMB->req_no_secext.Capabilities = cpu_to_le32(capabilities);
-		pSMB->req_no_secext.CaseInsensitivePasswordLength =
-			cpu_to_le16(CIFS_AUTH_RESP_SIZE);
-		pSMB->req_no_secext.CaseSensitivePasswordLength =
-			cpu_to_le16(CIFS_AUTH_RESP_SIZE);
-
-		/* calculate ntlm response and session key */
-		rc = setup_ntlm_response(ses, nls_cp);
-		if (rc) {
-			cifs_dbg(VFS, "Error %d during NTLM authentication\n",
-				 rc);
-			goto ssetup_exit;
-		}
-
-		/* copy ntlm response */
-		memcpy(bcc_ptr, ses->auth_key.response + CIFS_SESS_KEY_SIZE,
-				CIFS_AUTH_RESP_SIZE);
-		bcc_ptr += CIFS_AUTH_RESP_SIZE;
-		memcpy(bcc_ptr, ses->auth_key.response + CIFS_SESS_KEY_SIZE,
-				CIFS_AUTH_RESP_SIZE);
-		bcc_ptr += CIFS_AUTH_RESP_SIZE;
-
-		if (ses->capabilities & CAP_UNICODE) {
-			/* unicode strings must be word aligned */
-			if (iov[0].iov_len % 2) {
-				*bcc_ptr = 0;
-				bcc_ptr++;
-			}
-			unicode_ssetup_strings(&bcc_ptr, ses, nls_cp);
-		} else
-			ascii_ssetup_strings(&bcc_ptr, ses, nls_cp);
-	} else if (type == NTLMv2) {
-		pSMB->req_no_secext.Capabilities = cpu_to_le32(capabilities);
-
-		/* LM2 password would be here if we supported it */
-		pSMB->req_no_secext.CaseInsensitivePasswordLength = 0;
-
-		/* calculate nlmv2 response and session key */
-		rc = setup_ntlmv2_rsp(ses, nls_cp);
-		if (rc) {
-			cifs_dbg(VFS, "Error %d during NTLMv2 authentication\n",
-				 rc);
-			goto ssetup_exit;
-		}
-		memcpy(bcc_ptr, ses->auth_key.response + CIFS_SESS_KEY_SIZE,
-				ses->auth_key.len - CIFS_SESS_KEY_SIZE);
-		bcc_ptr += ses->auth_key.len - CIFS_SESS_KEY_SIZE;
-
-		/* set case sensitive password length after tilen may get
-		 * assigned, tilen is 0 otherwise.
-		 */
-		pSMB->req_no_secext.CaseSensitivePasswordLength =
-			cpu_to_le16(ses->auth_key.len - CIFS_SESS_KEY_SIZE);
-
-		if (ses->capabilities & CAP_UNICODE) {
-			if (iov[0].iov_len % 2) {
-				*bcc_ptr = 0;
-				bcc_ptr++;
-			}
-			unicode_ssetup_strings(&bcc_ptr, ses, nls_cp);
-		} else
-			ascii_ssetup_strings(&bcc_ptr, ses, nls_cp);
-	} else if (type == Kerberos) {
+	if (type == Kerberos) {
 #ifdef CONFIG_CIFS_UPCALL
 		struct cifs_spnego_msg *msg;
 
@@ -1083,7 +1233,7 @@ ssetup_ntlmssp_authenticate:
 	if (rc)
 		goto ssetup_exit;
 
-	if ((smb_buf->WordCount != 3) && (smb_buf->WordCount != 4)) {
+	if (smb_buf->WordCount != 4) {
 		rc = -EIO;
 		cifs_dbg(VFS, "bad word count %d\n", smb_buf->WordCount);
 		goto ssetup_exit;
@@ -1098,22 +1248,20 @@ ssetup_ntlmssp_authenticate:
 	bytes_remaining = get_bcc(smb_buf);
 	bcc_ptr = pByteArea(smb_buf);
 
-	if (smb_buf->WordCount == 4) {
-		blob_len = le16_to_cpu(pSMB->resp.SecurityBlobLength);
-		if (blob_len > bytes_remaining) {
-			cifs_dbg(VFS, "bad security blob length %d\n",
-				 blob_len);
-			rc = -EINVAL;
+	blob_len = le16_to_cpu(pSMB->resp.SecurityBlobLength);
+	if (blob_len > bytes_remaining) {
+		cifs_dbg(VFS, "bad security blob length %d\n",
+			 blob_len);
+		rc = -EINVAL;
+		goto ssetup_exit;
+	}
+	if (phase == NtLmChallenge) {
+		rc = decode_ntlmssp_challenge(bcc_ptr, blob_len, ses);
+		if (rc)
 			goto ssetup_exit;
-		}
-		if (phase == NtLmChallenge) {
-			rc = decode_ntlmssp_challenge(bcc_ptr, blob_len, ses);
-			if (rc)
-				goto ssetup_exit;
-		}
-		bcc_ptr += blob_len;
-		bytes_remaining -= blob_len;
 	}
+	bcc_ptr += blob_len;
+	bytes_remaining -= blob_len;
 
 	/* BB check if Unicode and decode strings */
 	if (bytes_remaining == 0) {
-- 
1.8.4.2

Re: [PATCH 2/4] cifs: Split ntlm and ntlmv2 authentication methods off CIFS_SessSetup()

[Jeff Layton]

On Fri,  2 May 2014 14:21:29 +0100
Sachin Prabhu <sprabhu-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:

> Signed-off-by: Sachin Prabhu <sprabhu-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
>  fs/cifs/sess.c | 316 ++++++++++++++++++++++++++++++++++++++++++---------------
>  1 file changed, 232 insertions(+), 84 deletions(-)
> 
> diff --git a/fs/cifs/sess.c b/fs/cifs/sess.c
> index c6aa60b..2be1ecb 100644
> --- a/fs/cifs/sess.c
> +++ b/fs/cifs/sess.c
> @@ -764,6 +764,216 @@ sess_auth_lanman(struct sess_data *sess_data)
>  }
>  #endif
>  
> +static void
> +sess_auth_ntlm(struct sess_data *sess_data)
> +{
> +	int rc = 0;
> +	struct smb_hdr *smb_buf;
> +	SESSION_SETUP_ANDX *pSMB;
> +	char *bcc_ptr;
> +	struct cifs_ses *ses = sess_data->ses;
> +	__u32 capabilities;
> +	__u16 bytes_remaining;
> +
> +	/* old style NTLM sessionsetup */
> +	/* wct = 13 */
> +	rc = sess_alloc_buffer(sess_data, 13);
> +	if (rc)
> +		goto out;
> +
> +	pSMB = (SESSION_SETUP_ANDX *)sess_data->iov[0].iov_base;
> +	bcc_ptr = sess_data->iov[2].iov_base;
> +	capabilities = cifs_ssetup_hdr(ses, pSMB);
> +
> +	pSMB->req_no_secext.Capabilities = cpu_to_le32(capabilities);
> +	pSMB->req_no_secext.CaseInsensitivePasswordLength =
> +			cpu_to_le16(CIFS_AUTH_RESP_SIZE);
> +	pSMB->req_no_secext.CaseSensitivePasswordLength =
> +			cpu_to_le16(CIFS_AUTH_RESP_SIZE);
> +
> +	/* calculate ntlm response and session key */
> +	rc = setup_ntlm_response(ses, sess_data->nls_cp);
> +	if (rc) {
> +		cifs_dbg(VFS, "Error %d during NTLM authentication\n",
> +				 rc);
> +		goto out;
> +	}
> +
> +	/* copy ntlm response */
> +	memcpy(bcc_ptr, ses->auth_key.response + CIFS_SESS_KEY_SIZE,
> +			CIFS_AUTH_RESP_SIZE);
> +	bcc_ptr += CIFS_AUTH_RESP_SIZE;
> +	memcpy(bcc_ptr, ses->auth_key.response + CIFS_SESS_KEY_SIZE,
> +			CIFS_AUTH_RESP_SIZE);
> +	bcc_ptr += CIFS_AUTH_RESP_SIZE;
> +
> +	if (ses->capabilities & CAP_UNICODE) {
> +		/* unicode strings must be word aligned */
> +		if (sess_data->iov[0].iov_len % 2) {
> +			*bcc_ptr = 0;
> +			bcc_ptr++;
> +		}
> +		unicode_ssetup_strings(&bcc_ptr, ses, sess_data->nls_cp);
> +	} else {
> +		ascii_ssetup_strings(&bcc_ptr, ses, sess_data->nls_cp);
> +	}
> +
> +
> +	sess_data->iov[2].iov_len = (long) bcc_ptr -
> +			(long) sess_data->iov[2].iov_base;
> +
> +	rc = sess_sendreceive(sess_data);
> +	if (rc)
> +		goto out;
> +
> +	pSMB = (SESSION_SETUP_ANDX *)sess_data->iov[0].iov_base;
> +	smb_buf = (struct smb_hdr *)sess_data->iov[0].iov_base;
> +
> +	if (smb_buf->WordCount != 3) {
> +		rc = -EIO;
> +		cifs_dbg(VFS, "bad word count %d\n", smb_buf->WordCount);
> +		goto out;
> +	}
> +
> +	if (le16_to_cpu(pSMB->resp.Action) & GUEST_LOGIN)
> +		cifs_dbg(FYI, "Guest login\n"); /* BB mark SesInfo struct? */
> +
> +	ses->Suid = smb_buf->Uid;   /* UID left in wire format (le) */
> +	cifs_dbg(FYI, "UID = %llu\n", ses->Suid);
> +
> +	bytes_remaining = get_bcc(smb_buf);
> +	bcc_ptr = pByteArea(smb_buf);
> +
> +	/* BB check if Unicode and decode strings */
> +	if (bytes_remaining == 0) {
> +		/* no string area to decode, do nothing */
> +	} else if (smb_buf->Flags2 & SMBFLG2_UNICODE) {
> +		/* unicode string area must be word-aligned */
> +		if (((unsigned long) bcc_ptr - (unsigned long) smb_buf) % 2) {
> +			++bcc_ptr;
> +			--bytes_remaining;
> +		}
> +		decode_unicode_ssetup(&bcc_ptr, bytes_remaining, ses,
> +				      sess_data->nls_cp);
> +	} else {
> +		decode_ascii_ssetup(&bcc_ptr, bytes_remaining, ses,
> +				    sess_data->nls_cp);
> +	}
> +
> +	rc = sess_establish_session(sess_data);
> +out:
> +	sess_data->result = rc;
> +	sess_data->func = NULL;
> +	sess_free_buffer(sess_data);
> +	kfree(ses->auth_key.response);
> +	ses->auth_key.response = NULL;
> +}
> +
> +static void
> +sess_auth_ntlmv2(struct sess_data *sess_data)
> +{
> +	int rc = 0;
> +	struct smb_hdr *smb_buf;
> +	SESSION_SETUP_ANDX *pSMB;
> +	char *bcc_ptr;
> +	struct cifs_ses *ses = sess_data->ses;
> +	__u32 capabilities;
> +	__u16 bytes_remaining;
> +
> +	/* old style NTLM sessionsetup */
> +	/* wct = 13 */
> +	rc = sess_alloc_buffer(sess_data, 13);
> +	if (rc)
> +		goto out;
> +
> +	pSMB = (SESSION_SETUP_ANDX *)sess_data->iov[0].iov_base;
> +	bcc_ptr = sess_data->iov[2].iov_base;
> +	capabilities = cifs_ssetup_hdr(ses, pSMB);
> +
> +	pSMB->req_no_secext.Capabilities = cpu_to_le32(capabilities);
> +
> +	/* LM2 password would be here if we supported it */
> +	pSMB->req_no_secext.CaseInsensitivePasswordLength = 0;
> +
> +	/* calculate nlmv2 response and session key */
> +	rc = setup_ntlmv2_rsp(ses, sess_data->nls_cp);
> +	if (rc) {
> +		cifs_dbg(VFS, "Error %d during NTLMv2 authentication\n", rc);
> +		goto out;
> +	}
> +
> +	memcpy(bcc_ptr, ses->auth_key.response + CIFS_SESS_KEY_SIZE,
> +			ses->auth_key.len - CIFS_SESS_KEY_SIZE);
> +	bcc_ptr += ses->auth_key.len - CIFS_SESS_KEY_SIZE;
> +
> +	/* set case sensitive password length after tilen may get
> +	 * assigned, tilen is 0 otherwise.
> +	 */
> +	pSMB->req_no_secext.CaseSensitivePasswordLength =
> +		cpu_to_le16(ses->auth_key.len - CIFS_SESS_KEY_SIZE);
> +
> +	if (ses->capabilities & CAP_UNICODE) {
> +		if (sess_data->iov[0].iov_len % 2) {
> +			*bcc_ptr = 0;
> +			bcc_ptr++;
> +		}
> +		unicode_ssetup_strings(&bcc_ptr, ses, sess_data->nls_cp);
> +	} else {
> +		ascii_ssetup_strings(&bcc_ptr, ses, sess_data->nls_cp);
> +	}
> +
> +
> +	sess_data->iov[2].iov_len = (long) bcc_ptr -
> +			(long) sess_data->iov[2].iov_base;
> +
> +	rc = sess_sendreceive(sess_data);
> +	if (rc)
> +		goto out;
> +
> +	pSMB = (SESSION_SETUP_ANDX *)sess_data->iov[0].iov_base;
> +	smb_buf = (struct smb_hdr *)sess_data->iov[0].iov_base;
> +
> +	if (smb_buf->WordCount != 3) {
> +		rc = -EIO;
> +		cifs_dbg(VFS, "bad word count %d\n", smb_buf->WordCount);
> +		goto out;
> +	}
> +
> +	if (le16_to_cpu(pSMB->resp.Action) & GUEST_LOGIN)
> +		cifs_dbg(FYI, "Guest login\n"); /* BB mark SesInfo struct? */
> +
> +	ses->Suid = smb_buf->Uid;   /* UID left in wire format (le) */
> +	cifs_dbg(FYI, "UID = %llu\n", ses->Suid);
> +
> +	bytes_remaining = get_bcc(smb_buf);
> +	bcc_ptr = pByteArea(smb_buf);
> +
> +	/* BB check if Unicode and decode strings */
> +	if (bytes_remaining == 0) {
> +		/* no string area to decode, do nothing */
> +	} else if (smb_buf->Flags2 & SMBFLG2_UNICODE) {
> +		/* unicode string area must be word-aligned */
> +		if (((unsigned long) bcc_ptr - (unsigned long) smb_buf) % 2) {
> +			++bcc_ptr;
> +			--bytes_remaining;
> +		}
> +		decode_unicode_ssetup(&bcc_ptr, bytes_remaining, ses,
> +				      sess_data->nls_cp);
> +	} else {
> +		decode_ascii_ssetup(&bcc_ptr, bytes_remaining, ses,
> +				    sess_data->nls_cp);
> +	}
> +
> +	rc = sess_establish_session(sess_data);
> +out:
> +	sess_data->result = rc;
> +	sess_data->func = NULL;
> +	sess_free_buffer(sess_data);
> +	kfree(ses->auth_key.response);
> +	ses->auth_key.response = NULL;
> +}
> +
> +
>  int
>  CIFS_SessSetup(const unsigned int xid, struct cifs_ses *ses,
>  	       const struct nls_table *nls_cp)
> @@ -810,6 +1020,12 @@ CIFS_SessSetup(const unsigned int xid, struct cifs_ses *ses,
>  	case LANMAN:
>  		sess_auth_lanman(sess_data);
>  		goto out;
> +	case NTLM:
> +		sess_auth_ntlm(sess_data);
> +		goto out;
> +	case NTLMv2:
> +		sess_auth_ntlmv2(sess_data);
> +		goto out;
>  	default:
>  		/* Continue with the rest of the function */
>  		break;
> @@ -830,11 +1046,8 @@ ssetup_ntlmssp_authenticate:
>  	if (phase == NtLmChallenge)
>  		phase = NtLmAuthenticate; /* if ntlmssp, now final phase */
>  
> -	if ((type == NTLM) || (type == NTLMv2)) {
> -		/* For NTLMv2 failures eventually may need to retry NTLM */
> -		wct = 13; /* old style NTLM sessionsetup */
> -	} else /* same size: negotiate or auth, NTLMSSP or extended security */
> -		wct = 12;
> +	/* same size: negotiate or auth, NTLMSSP or extended security */
> +	wct = 12;
>  
>  	rc = small_smb_init_no_tc(SMB_COM_SESSION_SETUP_ANDX, wct, ses,
>  			    (void **)&smb_buf);
> @@ -869,70 +1082,7 @@ ssetup_ntlmssp_authenticate:
>  	iov[1].iov_base = NULL;
>  	iov[1].iov_len = 0;
>  
> -	if (type == NTLM) {
> -		pSMB->req_no_secext.Capabilities = cpu_to_le32(capabilities);
> -		pSMB->req_no_secext.CaseInsensitivePasswordLength =
> -			cpu_to_le16(CIFS_AUTH_RESP_SIZE);
> -		pSMB->req_no_secext.CaseSensitivePasswordLength =
> -			cpu_to_le16(CIFS_AUTH_RESP_SIZE);
> -
> -		/* calculate ntlm response and session key */
> -		rc = setup_ntlm_response(ses, nls_cp);
> -		if (rc) {
> -			cifs_dbg(VFS, "Error %d during NTLM authentication\n",
> -				 rc);
> -			goto ssetup_exit;
> -		}
> -
> -		/* copy ntlm response */
> -		memcpy(bcc_ptr, ses->auth_key.response + CIFS_SESS_KEY_SIZE,
> -				CIFS_AUTH_RESP_SIZE);
> -		bcc_ptr += CIFS_AUTH_RESP_SIZE;
> -		memcpy(bcc_ptr, ses->auth_key.response + CIFS_SESS_KEY_SIZE,
> -				CIFS_AUTH_RESP_SIZE);
> -		bcc_ptr += CIFS_AUTH_RESP_SIZE;
> -
> -		if (ses->capabilities & CAP_UNICODE) {
> -			/* unicode strings must be word aligned */
> -			if (iov[0].iov_len % 2) {
> -				*bcc_ptr = 0;
> -				bcc_ptr++;
> -			}
> -			unicode_ssetup_strings(&bcc_ptr, ses, nls_cp);
> -		} else
> -			ascii_ssetup_strings(&bcc_ptr, ses, nls_cp);
> -	} else if (type == NTLMv2) {
> -		pSMB->req_no_secext.Capabilities = cpu_to_le32(capabilities);
> -
> -		/* LM2 password would be here if we supported it */
> -		pSMB->req_no_secext.CaseInsensitivePasswordLength = 0;
> -
> -		/* calculate nlmv2 response and session key */
> -		rc = setup_ntlmv2_rsp(ses, nls_cp);
> -		if (rc) {
> -			cifs_dbg(VFS, "Error %d during NTLMv2 authentication\n",
> -				 rc);
> -			goto ssetup_exit;
> -		}
> -		memcpy(bcc_ptr, ses->auth_key.response + CIFS_SESS_KEY_SIZE,
> -				ses->auth_key.len - CIFS_SESS_KEY_SIZE);
> -		bcc_ptr += ses->auth_key.len - CIFS_SESS_KEY_SIZE;
> -
> -		/* set case sensitive password length after tilen may get
> -		 * assigned, tilen is 0 otherwise.
> -		 */
> -		pSMB->req_no_secext.CaseSensitivePasswordLength =
> -			cpu_to_le16(ses->auth_key.len - CIFS_SESS_KEY_SIZE);
> -
> -		if (ses->capabilities & CAP_UNICODE) {
> -			if (iov[0].iov_len % 2) {
> -				*bcc_ptr = 0;
> -				bcc_ptr++;
> -			}
> -			unicode_ssetup_strings(&bcc_ptr, ses, nls_cp);
> -		} else
> -			ascii_ssetup_strings(&bcc_ptr, ses, nls_cp);
> -	} else if (type == Kerberos) {
> +	if (type == Kerberos) {
>  #ifdef CONFIG_CIFS_UPCALL
>  		struct cifs_spnego_msg *msg;
>  
> @@ -1083,7 +1233,7 @@ ssetup_ntlmssp_authenticate:
>  	if (rc)
>  		goto ssetup_exit;
>  
> -	if ((smb_buf->WordCount != 3) && (smb_buf->WordCount != 4)) {
> +	if (smb_buf->WordCount != 4) {
>  		rc = -EIO;
>  		cifs_dbg(VFS, "bad word count %d\n", smb_buf->WordCount);
>  		goto ssetup_exit;
> @@ -1098,22 +1248,20 @@ ssetup_ntlmssp_authenticate:
>  	bytes_remaining = get_bcc(smb_buf);
>  	bcc_ptr = pByteArea(smb_buf);
>  
> -	if (smb_buf->WordCount == 4) {
> -		blob_len = le16_to_cpu(pSMB->resp.SecurityBlobLength);
> -		if (blob_len > bytes_remaining) {
> -			cifs_dbg(VFS, "bad security blob length %d\n",
> -				 blob_len);
> -			rc = -EINVAL;
> +	blob_len = le16_to_cpu(pSMB->resp.SecurityBlobLength);
> +	if (blob_len > bytes_remaining) {
> +		cifs_dbg(VFS, "bad security blob length %d\n",
> +			 blob_len);
> +		rc = -EINVAL;
> +		goto ssetup_exit;
> +	}
> +	if (phase == NtLmChallenge) {
> +		rc = decode_ntlmssp_challenge(bcc_ptr, blob_len, ses);
> +		if (rc)
>  			goto ssetup_exit;
> -		}
> -		if (phase == NtLmChallenge) {
> -			rc = decode_ntlmssp_challenge(bcc_ptr, blob_len, ses);
> -			if (rc)
> -				goto ssetup_exit;
> -		}
> -		bcc_ptr += blob_len;
> -		bytes_remaining -= blob_len;
>  	}
> +	bcc_ptr += blob_len;
> +	bytes_remaining -= blob_len;
>  
>  	/* BB check if Unicode and decode strings */
>  	if (bytes_remaining == 0) {

I like it.

Reviewed-by: Jeff Layton <jlayton-vpEMnDpepFuMZCB2o+C8xQ@public.gmane.org>

[PATCH 0/4] RFC: Split CIFS_SessSetup()

[Sachin Prabhu]

I am investigating the possibility of adding support for gss-proxy in the
cifs client module. As part of that investigation, I was looking at the
CIFS_SessSetup() function. This is a long function which handles multiple auths
using if conditions and switch statements. The code struction makes it
difficult to modify of add support for new authentication mechanisms.

The proposal is to split the various authentication code into its own separate
functions. This increases the lines of code but will simplify maintenance and
addition of new auth methods.

The short term goal is to add gss-proxy support for kerberos authentication.
This will require to and fro communication with the gss-proxy module over a
unix socket.

Further long term goals are 
1) Add support for NTLMSSP using SPNEGO,
2) Allow clients to negotiate which authentication mechanism to use using SPNEGO.

I would like the opinion of this list about these proposed changes.
Is there any part of the code which needs changing?

Sachin Prabhu



Sachin Prabhu (4):
  cifs: Split lanman auth from CIFS_SessSetup()
  cifs: Split ntlm and ntlmv2 authentication methods off
    CIFS_SessSetup()
  cifs: Split Kerberos authentication off CIFS_SessSetup()
  cifs: Separate rawntlmssp auth from CIFS_SessSetup()

 fs/cifs/sess.c | 1118 +++++++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 775 insertions(+), 343 deletions(-)

-- 
1.8.4.2

[PATCH 2/4] cifs: Split ntlm and ntlmv2 authentication methods off CIFS_SessSetup()

[Sachin Prabhu]

Signed-off-by: Sachin Prabhu <sprabhu-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 fs/cifs/sess.c | 318 ++++++++++++++++++++++++++++++++++++++++++---------------
 1 file changed, 233 insertions(+), 85 deletions(-)

diff --git a/fs/cifs/sess.c b/fs/cifs/sess.c
index 8481631..4428ece 100644
--- a/fs/cifs/sess.c
+++ b/fs/cifs/sess.c
@@ -751,6 +751,216 @@ out:
 }
 #endif
 
+static void
+sess_auth_ntlm(struct sess_data *sess_data)
+{
+	int rc = 0;
+	struct smb_hdr *smb_buf;
+	SESSION_SETUP_ANDX *pSMB;
+	char *bcc_ptr;
+	struct cifs_ses *ses = sess_data->ses;
+	__u32 capabilities;
+	__u16 bytes_remaining;
+
+	/* old style NTLM sessionsetup */
+	/* wct = 13 */
+	rc = sess_alloc_buffer(sess_data, 13);
+	if (rc)
+		goto out;
+
+	pSMB = (SESSION_SETUP_ANDX *)sess_data->iov[0].iov_base;
+	bcc_ptr = sess_data->iov[2].iov_base;
+	capabilities = cifs_ssetup_hdr(ses, pSMB);
+
+	pSMB->req_no_secext.Capabilities = cpu_to_le32(capabilities);
+	pSMB->req_no_secext.CaseInsensitivePasswordLength =
+			cpu_to_le16(CIFS_AUTH_RESP_SIZE);
+	pSMB->req_no_secext.CaseSensitivePasswordLength =
+			cpu_to_le16(CIFS_AUTH_RESP_SIZE);
+
+	/* calculate ntlm response and session key */
+	rc = setup_ntlm_response(ses, sess_data->nls_cp);
+	if (rc) {
+		cifs_dbg(VFS, "Error %d during NTLM authentication\n",
+				 rc);
+		goto out;
+	}
+
+	/* copy ntlm response */
+	memcpy(bcc_ptr, ses->auth_key.response + CIFS_SESS_KEY_SIZE,
+			CIFS_AUTH_RESP_SIZE);
+	bcc_ptr += CIFS_AUTH_RESP_SIZE;
+	memcpy(bcc_ptr, ses->auth_key.response + CIFS_SESS_KEY_SIZE,
+			CIFS_AUTH_RESP_SIZE);
+	bcc_ptr += CIFS_AUTH_RESP_SIZE;
+
+	if (ses->capabilities & CAP_UNICODE) {
+		/* unicode strings must be word aligned */
+		if (sess_data->iov[0].iov_len % 2) {
+			*bcc_ptr = 0;
+			bcc_ptr++;
+		}
+		unicode_ssetup_strings(&bcc_ptr, ses, sess_data->nls_cp);
+	} else {
+		ascii_ssetup_strings(&bcc_ptr, ses, sess_data->nls_cp);
+	}
+
+
+	sess_data->iov[2].iov_len = (long) bcc_ptr -
+			(long) sess_data->iov[2].iov_base;
+
+	rc = sess_sendreceive(sess_data);
+	if (rc)
+		goto out;
+
+	pSMB = (SESSION_SETUP_ANDX *)sess_data->iov[0].iov_base;
+	smb_buf = (struct smb_hdr *)sess_data->iov[0].iov_base;
+
+	if (smb_buf->WordCount != 3) {
+		rc = -EIO;
+		cifs_dbg(VFS, "bad word count %d\n", smb_buf->WordCount);
+		goto out;
+	}
+
+	if (le16_to_cpu(pSMB->resp.Action) & GUEST_LOGIN)
+		cifs_dbg(FYI, "Guest login\n"); /* BB mark SesInfo struct? */
+
+	ses->Suid = smb_buf->Uid;   /* UID left in wire format (le) */
+	cifs_dbg(FYI, "UID = %llu\n", ses->Suid);
+
+	bytes_remaining = get_bcc(smb_buf);
+	bcc_ptr = pByteArea(smb_buf);
+
+	/* BB check if Unicode and decode strings */
+	if (bytes_remaining == 0) {
+		/* no string area to decode, do nothing */
+	} else if (smb_buf->Flags2 & SMBFLG2_UNICODE) {
+		/* unicode string area must be word-aligned */
+		if (((unsigned long) bcc_ptr - (unsigned long) smb_buf) % 2) {
+			++bcc_ptr;
+			--bytes_remaining;
+		}
+		decode_unicode_ssetup(&bcc_ptr, bytes_remaining, ses,
+				      sess_data->nls_cp);
+	} else {
+		decode_ascii_ssetup(&bcc_ptr, bytes_remaining, ses,
+				    sess_data->nls_cp);
+	}
+
+out:
+	sess_data->result = rc;
+	sess_data->func = NULL;
+	sess_free_buffer(sess_data);
+	sess_establish_session(sess_data);
+	kfree(ses->auth_key.response);
+	ses->auth_key.response = NULL;
+}
+
+static void
+sess_auth_ntlmv2(struct sess_data *sess_data)
+{
+	int rc = 0;
+	struct smb_hdr *smb_buf;
+	SESSION_SETUP_ANDX *pSMB;
+	char *bcc_ptr;
+	struct cifs_ses *ses = sess_data->ses;
+	__u32 capabilities;
+	__u16 bytes_remaining;
+
+	/* old style NTLM sessionsetup */
+	/* wct = 13 */
+	rc = sess_alloc_buffer(sess_data, 13);
+	if (rc)
+		goto out;
+
+	pSMB = (SESSION_SETUP_ANDX *)sess_data->iov[0].iov_base;
+	bcc_ptr = sess_data->iov[2].iov_base;
+	capabilities = cifs_ssetup_hdr(ses, pSMB);
+
+	pSMB->req_no_secext.Capabilities = cpu_to_le32(capabilities);
+
+	/* LM2 password would be here if we supported it */
+	pSMB->req_no_secext.CaseInsensitivePasswordLength = 0;
+
+	/* calculate nlmv2 response and session key */
+	rc = setup_ntlmv2_rsp(ses, sess_data->nls_cp);
+	if (rc) {
+		cifs_dbg(VFS, "Error %d during NTLMv2 authentication\n", rc);
+		goto out;
+	}
+
+	memcpy(bcc_ptr, ses->auth_key.response + CIFS_SESS_KEY_SIZE,
+			ses->auth_key.len - CIFS_SESS_KEY_SIZE);
+	bcc_ptr += ses->auth_key.len - CIFS_SESS_KEY_SIZE;
+
+	/* set case sensitive password length after tilen may get
+	 * assigned, tilen is 0 otherwise.
+	 */
+	pSMB->req_no_secext.CaseSensitivePasswordLength =
+		cpu_to_le16(ses->auth_key.len - CIFS_SESS_KEY_SIZE);
+
+	if (ses->capabilities & CAP_UNICODE) {
+		if (sess_data->iov[0].iov_len % 2) {
+			*bcc_ptr = 0;
+			bcc_ptr++;
+		}
+		unicode_ssetup_strings(&bcc_ptr, ses, sess_data->nls_cp);
+	} else {
+		ascii_ssetup_strings(&bcc_ptr, ses, sess_data->nls_cp);
+	}
+
+
+	sess_data->iov[2].iov_len = (long) bcc_ptr -
+			(long) sess_data->iov[2].iov_base;
+
+	rc = sess_sendreceive(sess_data);
+	if (rc)
+		goto out;
+
+	pSMB = (SESSION_SETUP_ANDX *)sess_data->iov[0].iov_base;
+	smb_buf = (struct smb_hdr *)sess_data->iov[0].iov_base;
+
+	if (smb_buf->WordCount != 3) {
+		rc = -EIO;
+		cifs_dbg(VFS, "bad word count %d\n", smb_buf->WordCount);
+		goto out;
+	}
+
+	if (le16_to_cpu(pSMB->resp.Action) & GUEST_LOGIN)
+		cifs_dbg(FYI, "Guest login\n"); /* BB mark SesInfo struct? */
+
+	ses->Suid = smb_buf->Uid;   /* UID left in wire format (le) */
+	cifs_dbg(FYI, "UID = %llu\n", ses->Suid);
+
+	bytes_remaining = get_bcc(smb_buf);
+	bcc_ptr = pByteArea(smb_buf);
+
+	/* BB check if Unicode and decode strings */
+	if (bytes_remaining == 0) {
+		/* no string area to decode, do nothing */
+	} else if (smb_buf->Flags2 & SMBFLG2_UNICODE) {
+		/* unicode string area must be word-aligned */
+		if (((unsigned long) bcc_ptr - (unsigned long) smb_buf) % 2) {
+			++bcc_ptr;
+			--bytes_remaining;
+		}
+		decode_unicode_ssetup(&bcc_ptr, bytes_remaining, ses,
+				      sess_data->nls_cp);
+	} else {
+		decode_ascii_ssetup(&bcc_ptr, bytes_remaining, ses,
+				    sess_data->nls_cp);
+	}
+
+out:
+	sess_data->result = rc;
+	sess_data->func = NULL;
+	sess_free_buffer(sess_data);
+	sess_establish_session(sess_data);
+	kfree(ses->auth_key.response);
+	ses->auth_key.response = NULL;
+}
+
+
 int
 CIFS_SessSetup(const unsigned int xid, struct cifs_ses *ses,
 	       const struct nls_table *nls_cp)
@@ -806,6 +1016,12 @@ CIFS_SessSetup(const unsigned int xid, struct cifs_ses *ses,
 #else
                 return -EOPNOTSUPP;
 #endif
+	case NTLM:
+                sess_auth_ntlm(sess_data);
+		return sess_data->result;
+	case NTLMv2:
+                sess_auth_ntlmv2(sess_data);
+		return sess_data->result;
 	default:
 		/* Continue with the rest of the function */
 		break;
@@ -826,11 +1042,8 @@ ssetup_ntlmssp_authenticate:
 	if (phase == NtLmChallenge)
 		phase = NtLmAuthenticate; /* if ntlmssp, now final phase */
 
-	if ((type == NTLM) || (type == NTLMv2)) {
-		/* For NTLMv2 failures eventually may need to retry NTLM */
-		wct = 13; /* old style NTLM sessionsetup */
-	} else /* same size: negotiate or auth, NTLMSSP or extended security */
-		wct = 12;
+	/* same size: negotiate or auth, NTLMSSP or extended security */
+	wct = 12;
 
 	rc = small_smb_init_no_tc(SMB_COM_SESSION_SETUP_ANDX, wct, ses,
 			    (void **)&smb_buf);
@@ -865,70 +1078,7 @@ ssetup_ntlmssp_authenticate:
 	iov[1].iov_base = NULL;
 	iov[1].iov_len = 0;
 
-	if (type == NTLM) {
-		pSMB->req_no_secext.Capabilities = cpu_to_le32(capabilities);
-		pSMB->req_no_secext.CaseInsensitivePasswordLength =
-			cpu_to_le16(CIFS_AUTH_RESP_SIZE);
-		pSMB->req_no_secext.CaseSensitivePasswordLength =
-			cpu_to_le16(CIFS_AUTH_RESP_SIZE);
-
-		/* calculate ntlm response and session key */
-		rc = setup_ntlm_response(ses, nls_cp);
-		if (rc) {
-			cifs_dbg(VFS, "Error %d during NTLM authentication\n",
-				 rc);
-			goto ssetup_exit;
-		}
-
-		/* copy ntlm response */
-		memcpy(bcc_ptr, ses->auth_key.response + CIFS_SESS_KEY_SIZE,
-				CIFS_AUTH_RESP_SIZE);
-		bcc_ptr += CIFS_AUTH_RESP_SIZE;
-		memcpy(bcc_ptr, ses->auth_key.response + CIFS_SESS_KEY_SIZE,
-				CIFS_AUTH_RESP_SIZE);
-		bcc_ptr += CIFS_AUTH_RESP_SIZE;
-
-		if (ses->capabilities & CAP_UNICODE) {
-			/* unicode strings must be word aligned */
-			if (iov[0].iov_len % 2) {
-				*bcc_ptr = 0;
-				bcc_ptr++;
-			}
-			unicode_ssetup_strings(&bcc_ptr, ses, nls_cp);
-		} else
-			ascii_ssetup_strings(&bcc_ptr, ses, nls_cp);
-	} else if (type == NTLMv2) {
-		pSMB->req_no_secext.Capabilities = cpu_to_le32(capabilities);
-
-		/* LM2 password would be here if we supported it */
-		pSMB->req_no_secext.CaseInsensitivePasswordLength = 0;
-
-		/* calculate nlmv2 response and session key */
-		rc = setup_ntlmv2_rsp(ses, nls_cp);
-		if (rc) {
-			cifs_dbg(VFS, "Error %d during NTLMv2 authentication\n",
-				 rc);
-			goto ssetup_exit;
-		}
-		memcpy(bcc_ptr, ses->auth_key.response + CIFS_SESS_KEY_SIZE,
-				ses->auth_key.len - CIFS_SESS_KEY_SIZE);
-		bcc_ptr += ses->auth_key.len - CIFS_SESS_KEY_SIZE;
-
-		/* set case sensitive password length after tilen may get
-		 * assigned, tilen is 0 otherwise.
-		 */
-		pSMB->req_no_secext.CaseSensitivePasswordLength =
-			cpu_to_le16(ses->auth_key.len - CIFS_SESS_KEY_SIZE);
-
-		if (ses->capabilities & CAP_UNICODE) {
-			if (iov[0].iov_len % 2) {
-				*bcc_ptr = 0;
-				bcc_ptr++;
-			}
-			unicode_ssetup_strings(&bcc_ptr, ses, nls_cp);
-		} else
-			ascii_ssetup_strings(&bcc_ptr, ses, nls_cp);
-	} else if (type == Kerberos) {
+	if (type == Kerberos) {
 #ifdef CONFIG_CIFS_UPCALL
 		struct cifs_spnego_msg *msg;
 
@@ -1079,7 +1229,7 @@ ssetup_ntlmssp_authenticate:
 	if (rc)
 		goto ssetup_exit;
 
-	if ((smb_buf->WordCount != 3) && (smb_buf->WordCount != 4)) {
+	if (smb_buf->WordCount != 4) {
 		rc = -EIO;
 		cifs_dbg(VFS, "bad word count %d\n", smb_buf->WordCount);
 		goto ssetup_exit;
@@ -1094,23 +1244,21 @@ ssetup_ntlmssp_authenticate:
 	bytes_remaining = get_bcc(smb_buf);
 	bcc_ptr = pByteArea(smb_buf);
 
-	if (smb_buf->WordCount == 4) {
-		blob_len = le16_to_cpu(pSMB->resp.SecurityBlobLength);
-		if (blob_len > bytes_remaining) {
-			cifs_dbg(VFS, "bad security blob length %d\n",
-				 blob_len);
-			rc = -EINVAL;
+	blob_len = le16_to_cpu(pSMB->resp.SecurityBlobLength);
+	if (blob_len > bytes_remaining) {
+		cifs_dbg(VFS, "bad security blob length %d\n",
+			 blob_len);
+		rc = -EINVAL;
+		goto ssetup_exit;
+	}
+	if (phase == NtLmChallenge) {
+		rc = decode_ntlmssp_challenge(bcc_ptr, blob_len, ses);
+		/* now goto beginning for ntlmssp authenticate phase */
+		if (rc)
 			goto ssetup_exit;
-		}
-		if (phase == NtLmChallenge) {
-			rc = decode_ntlmssp_challenge(bcc_ptr, blob_len, ses);
-			/* now goto beginning for ntlmssp authenticate phase */
-			if (rc)
-				goto ssetup_exit;
-		}
-		bcc_ptr += blob_len;
-		bytes_remaining -= blob_len;
 	}
+	bcc_ptr += blob_len;
+	bytes_remaining -= blob_len;
 
 	/* BB check if Unicode and decode strings */
 	if (bytes_remaining == 0) {
-- 
1.8.4.2