[PATCH 00/19] cifs: overhaul of auth selection code

[PATCH 00/19] cifs: overhaul of auth selection code

[Jeff Layton]

When the change to make cifs default to NTLMSSP auth was made recently,
it broke a number of working setups. If the server doesn't support
extended security, then there is no way to recover. This is mostly due
to the fact that CIFS handles the selection of the authentication to use
badly. It makes that decision before ever talking to the server.  If it
guesses wrong, then there is no recourse.

At SambaXP this year, I also spoke with Simo Sorce about making cifs.ko
talk directly to the new gssproxy daemon to handle GSSAPI auth. I think
that would be a good thing to do. Doing that would allow us to get out
of the ASN.1 parsing business for the most part, and allow cifs to
support things like NegoEx. We can't reasonably support that though with
the code as rickety as it is today.

This patchset represents an overhaul of how cifs.ko selects the type of
authentication to use with the server. The idea here is to defer that
decision until SESSION_SETUP time, so that we can make an intelligent
decision about it based on the results of the NEGOTIATE.

The first several patches in the series represent some cleanup of dead
and broken code and struct fields that are unused. Next, chunks of
CIFSSMBNegotiate are factored out into helper functions. Next, we change
how the auth selection is done, and do that using securityEnum values
and a flag for signing, rather than trying to pass around variants of
SecurityFlags.

Lastly, there are a couple of patches that try to bring some sanity to
the SecurityFlags interface.

I'd like to see this merged for 3.11, so getting it into linux-next soon
would be good. Once this is merged, we can start looking at how best to
integrate gssproxy with cifs.ko as well.

Comments and suggestions welcome...

Jeff Layton (19):
  cifs: remove protocolEnum definition
  cifs: remove useless memset in LANMAN auth code
  cifs: make decode_ascii_ssetup void return
  cifs: throw a warning if negotiate or sess_setup ops are passed NULL
    server or session pointers
  cifs: remove the cifs_ses->flags field
  cifs: remove "seal" stubs
  cifs: break out decoding of security blob into separate function
  cifs: break out lanman NEGOTIATE handling into separate function
  cifs: move handling of signed connections into separate function
  cifs: factor out check for extended security bit into separate
    function
  cifs: add new "Unspecified" securityEnum value
  cifs: track the flavor of the NEGOTIATE reponse
  cifs: add new fields to smb_vol to track the requested security flavor
  cifs: add new fields to cifs_ses to track requested security flavor
  cifs: track the enablement of signing in the TCP_Server_Info
  cifs: move sectype to the cifs_ses instead of TCP_Server_Info
  cifs: update the default global_secflags to include "raw" NTLMv2
  cifs: clean up the SecurityFlags write handler
  cifs: try to handle the MUST SecurityFlags sanely

 fs/cifs/cifs_debug.c    |  48 +++++-
 fs/cifs/cifsencrypt.c   |   5 +-
 fs/cifs/cifsfs.c        |  13 +-
 fs/cifs/cifsglob.h      |  35 ++---
 fs/cifs/cifspdu.h       |   4 +-
 fs/cifs/cifsproto.h     |   3 +
 fs/cifs/cifssmb.c       | 405 ++++++++++++++++++++++--------------------------
 fs/cifs/connect.c       | 156 +++++++------------
 fs/cifs/misc.c          |   3 +-
 fs/cifs/sess.c          |  95 +++++++++---
 fs/cifs/smb1ops.c       |  21 +--
 fs/cifs/smb2pdu.c       | 114 ++++----------
 fs/cifs/smb2transport.c |   3 +-
 fs/cifs/transport.c     |   4 +-
 14 files changed, 427 insertions(+), 482 deletions(-)

-- 
1.8.1.4

[PATCH 01/19] cifs: remove protocolEnum definition

[Jeff Layton]

The field that held this was removed quite some time ago.

Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 fs/cifs/cifsglob.h | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index db9f985..29dd111 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -109,12 +109,6 @@ enum securityEnum {
 	Kerberos,		/* Kerberos via SPNEGO */
 };
 
-enum protocolEnum {
-	TCP = 0,
-	SCTP
-	/* Netbios frames protocol not supported at this time */
-};
-
 struct session_key {
 	unsigned int len;
 	char *response;
-- 
1.8.1.4

Re: [PATCH 01/19] cifs: remove protocolEnum definition

[Pavel Shilovsky]

2013/5/23 Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>:
> The field that held this was removed quite some time ago.
>
> Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
>  fs/cifs/cifsglob.h | 6 ------
>  1 file changed, 6 deletions(-)
>
> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
> index db9f985..29dd111 100644
> --- a/fs/cifs/cifsglob.h
> +++ b/fs/cifs/cifsglob.h
> @@ -109,12 +109,6 @@ enum securityEnum {
>         Kerberos,               /* Kerberos via SPNEGO */
>  };
>
> -enum protocolEnum {
> -       TCP = 0,
> -       SCTP
> -       /* Netbios frames protocol not supported at this time */
> -};
> -
>  struct session_key {
>         unsigned int len;
>         char *response;
> --
> 1.8.1.4
>
> --
> 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

Acked-by: Pavel Shilovsky <piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org>

--
Best regards,
Pavel Shilovsky.

[PATCH 02/19] cifs: remove useless memset in LANMAN auth code

[Jeff Layton]

It turns out that CIFS_SESS_KEY_SIZE == CIFS_ENCPWD_SIZE, so this
memset doesn't do anything useful.

Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 fs/cifs/cifsencrypt.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/fs/cifs/cifsencrypt.c b/fs/cifs/cifsencrypt.c
index 71436d1..a85a83d 100644
--- a/fs/cifs/cifsencrypt.c
+++ b/fs/cifs/cifsencrypt.c
@@ -276,7 +276,6 @@ int calc_lanman_hash(const char *password, const char *cryptkey, bool encrypt,
 		strncpy(password_with_pad, password, CIFS_ENCPWD_SIZE);
 
 	if (!encrypt && global_secflags & CIFSSEC_MAY_PLNTXT) {
-		memset(lnm_session_key, 0, CIFS_SESS_KEY_SIZE);
 		memcpy(lnm_session_key, password_with_pad,
 			CIFS_ENCPWD_SIZE);
 		return 0;
-- 
1.8.1.4

Re: [PATCH 02/19] cifs: remove useless memset in LANMAN auth code

[Pavel Shilovsky]

2013/5/23 Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>:
> It turns out that CIFS_SESS_KEY_SIZE == CIFS_ENCPWD_SIZE, so this
> memset doesn't do anything useful.
>
> Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
>  fs/cifs/cifsencrypt.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/fs/cifs/cifsencrypt.c b/fs/cifs/cifsencrypt.c
> index 71436d1..a85a83d 100644
> --- a/fs/cifs/cifsencrypt.c
> +++ b/fs/cifs/cifsencrypt.c
> @@ -276,7 +276,6 @@ int calc_lanman_hash(const char *password, const char *cryptkey, bool encrypt,
>                 strncpy(password_with_pad, password, CIFS_ENCPWD_SIZE);
>
>         if (!encrypt && global_secflags & CIFSSEC_MAY_PLNTXT) {
> -               memset(lnm_session_key, 0, CIFS_SESS_KEY_SIZE);
>                 memcpy(lnm_session_key, password_with_pad,
>                         CIFS_ENCPWD_SIZE);
>                 return 0;
> --
> 1.8.1.4
>
> --
> 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

Acked-by: Pavel Shilovsky <piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org>


--
Best regards,
Pavel Shilovsky.

[PATCH 03/19] cifs: make decode_ascii_ssetup void return

[Jeff Layton]

...rc is always set to 0.

Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 fs/cifs/sess.c | 18 +++++++-----------
 1 file changed, 7 insertions(+), 11 deletions(-)

diff --git a/fs/cifs/sess.c b/fs/cifs/sess.c
index f230571..838e224 100644
--- a/fs/cifs/sess.c
+++ b/fs/cifs/sess.c
@@ -310,11 +310,10 @@ decode_unicode_ssetup(char **pbcc_area, int bleft, struct cifs_ses *ses,
 	return;
 }
 
-static int decode_ascii_ssetup(char **pbcc_area, __u16 bleft,
-			       struct cifs_ses *ses,
-			       const struct nls_table *nls_cp)
+static void decode_ascii_ssetup(char **pbcc_area, __u16 bleft,
+				struct cifs_ses *ses,
+				const struct nls_table *nls_cp)
 {
-	int rc = 0;
 	int len;
 	char *bcc_ptr = *pbcc_area;
 
@@ -322,7 +321,7 @@ static int decode_ascii_ssetup(char **pbcc_area, __u16 bleft,
 
 	len = strnlen(bcc_ptr, bleft);
 	if (len >= bleft)
-		return rc;
+		return;
 
 	kfree(ses->serverOS);
 
@@ -339,7 +338,7 @@ static int decode_ascii_ssetup(char **pbcc_area, __u16 bleft,
 
 	len = strnlen(bcc_ptr, bleft);
 	if (len >= bleft)
-		return rc;
+		return;
 
 	kfree(ses->serverNOS);
 
@@ -352,7 +351,7 @@ static int decode_ascii_ssetup(char **pbcc_area, __u16 bleft,
 
 	len = strnlen(bcc_ptr, bleft);
 	if (len > bleft)
-		return rc;
+		return;
 
 	/* No domain field in LANMAN case. Domain is
 	   returned by old servers in the SMB negprot response */
@@ -360,8 +359,6 @@ static int decode_ascii_ssetup(char **pbcc_area, __u16 bleft,
 	   but thus do return domain here we could add parsing
 	   for it later, but it is not very important */
 	cifs_dbg(FYI, "ascii: bytes left %d\n", bleft);
-
-	return rc;
 }
 
 int decode_ntlmssp_challenge(char *bcc_ptr, int blob_len,
@@ -938,8 +935,7 @@ ssetup_ntlmssp_authenticate:
 		}
 		decode_unicode_ssetup(&bcc_ptr, bytes_remaining, ses, nls_cp);
 	} else {
-		rc = decode_ascii_ssetup(&bcc_ptr, bytes_remaining,
-					 ses, nls_cp);
+		decode_ascii_ssetup(&bcc_ptr, bytes_remaining, ses, nls_cp);
 	}
 
 ssetup_exit:
-- 
1.8.1.4

Re: [PATCH 03/19] cifs: make decode_ascii_ssetup void return

[Pavel Shilovsky]

2013/5/23 Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>:
> ...rc is always set to 0.
>
> Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
>  fs/cifs/sess.c | 18 +++++++-----------
>  1 file changed, 7 insertions(+), 11 deletions(-)
>
> diff --git a/fs/cifs/sess.c b/fs/cifs/sess.c
> index f230571..838e224 100644
> --- a/fs/cifs/sess.c
> +++ b/fs/cifs/sess.c
> @@ -310,11 +310,10 @@ decode_unicode_ssetup(char **pbcc_area, int bleft, struct cifs_ses *ses,
>         return;
>  }
>
> -static int decode_ascii_ssetup(char **pbcc_area, __u16 bleft,
> -                              struct cifs_ses *ses,
> -                              const struct nls_table *nls_cp)
> +static void decode_ascii_ssetup(char **pbcc_area, __u16 bleft,
> +                               struct cifs_ses *ses,
> +                               const struct nls_table *nls_cp)
>  {
> -       int rc = 0;
>         int len;
>         char *bcc_ptr = *pbcc_area;
>
> @@ -322,7 +321,7 @@ static int decode_ascii_ssetup(char **pbcc_area, __u16 bleft,
>
>         len = strnlen(bcc_ptr, bleft);
>         if (len >= bleft)
> -               return rc;
> +               return;
>
>         kfree(ses->serverOS);
>
> @@ -339,7 +338,7 @@ static int decode_ascii_ssetup(char **pbcc_area, __u16 bleft,
>
>         len = strnlen(bcc_ptr, bleft);
>         if (len >= bleft)
> -               return rc;
> +               return;
>
>         kfree(ses->serverNOS);
>
> @@ -352,7 +351,7 @@ static int decode_ascii_ssetup(char **pbcc_area, __u16 bleft,
>
>         len = strnlen(bcc_ptr, bleft);
>         if (len > bleft)
> -               return rc;
> +               return;
>
>         /* No domain field in LANMAN case. Domain is
>            returned by old servers in the SMB negprot response */
> @@ -360,8 +359,6 @@ static int decode_ascii_ssetup(char **pbcc_area, __u16 bleft,
>            but thus do return domain here we could add parsing
>            for it later, but it is not very important */
>         cifs_dbg(FYI, "ascii: bytes left %d\n", bleft);
> -
> -       return rc;
>  }
>
>  int decode_ntlmssp_challenge(char *bcc_ptr, int blob_len,
> @@ -938,8 +935,7 @@ ssetup_ntlmssp_authenticate:
>                 }
>                 decode_unicode_ssetup(&bcc_ptr, bytes_remaining, ses, nls_cp);
>         } else {
> -               rc = decode_ascii_ssetup(&bcc_ptr, bytes_remaining,
> -                                        ses, nls_cp);
> +               decode_ascii_ssetup(&bcc_ptr, bytes_remaining, ses, nls_cp);
>         }
>
>  ssetup_exit:
> --
> 1.8.1.4
>
> --
> 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

Acked-by: Pavel Shilovsky <piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org>

--
Best regards,
Pavel Shilovsky.

[PATCH 04/19] cifs: throw a warning if negotiate or sess_setup ops are passed NULL server or session pointers

[Jeff Layton]

These look pretty cargo-culty to me, but let's be certain. Leave
them in place for now. Pop a WARN if it ever does happen. Also,
move to a more standard idiom for setting the "server" pointer.

Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 fs/cifs/cifssmb.c | 11 +++++------
 fs/cifs/sess.c    |  4 +++-
 fs/cifs/smb2pdu.c | 20 ++++++++------------
 3 files changed, 16 insertions(+), 19 deletions(-)

diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
index a58dc77..c1c2006 100644
--- a/fs/cifs/cifssmb.c
+++ b/fs/cifs/cifssmb.c
@@ -375,16 +375,15 @@ CIFSSMBNegotiate(const unsigned int xid, struct cifs_ses *ses)
 	int rc = 0;
 	int bytes_returned;
 	int i;
-	struct TCP_Server_Info *server;
+	struct TCP_Server_Info *server = ses->server;
 	u16 count;
 	unsigned int secFlags;
 
-	if (ses->server)
-		server = ses->server;
-	else {
-		rc = -EIO;
-		return rc;
+	if (!server) {
+		WARN(1, "%s: server is NULL!\n", __func__);
+		return -EIO;
 	}
+
 	rc = smb_init(SMB_COM_NEGOTIATE, 0, NULL /* no tcon yet */ ,
 		      (void **) &pSMB, (void **) &pSMBr);
 	if (rc)
diff --git a/fs/cifs/sess.c b/fs/cifs/sess.c
index 838e224..e8c5dc9 100644
--- a/fs/cifs/sess.c
+++ b/fs/cifs/sess.c
@@ -576,8 +576,10 @@ CIFS_SessSetup(const unsigned int xid, struct cifs_ses *ses,
 	u16 blob_len;
 	char *ntlmsspblob = NULL;
 
-	if (ses == NULL)
+	if (ses == NULL) {
+		WARN(1, "%s: ses == NULL!", __func__);
 		return -EINVAL;
+	}
 
 	type = ses->server->secType;
 	cifs_dbg(FYI, "sess setup type %d\n", type);
diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
index 2b95ce2..3af66aa 100644
--- a/fs/cifs/smb2pdu.c
+++ b/fs/cifs/smb2pdu.c
@@ -328,7 +328,7 @@ SMB2_negotiate(const unsigned int xid, struct cifs_ses *ses)
 	struct kvec iov[1];
 	int rc = 0;
 	int resp_buftype;
-	struct TCP_Server_Info *server;
+	struct TCP_Server_Info *server = ses->server;
 	unsigned int sec_flags;
 	u16 temp = 0;
 	int blob_offset, blob_length;
@@ -337,11 +337,9 @@ SMB2_negotiate(const unsigned int xid, struct cifs_ses *ses)
 
 	cifs_dbg(FYI, "Negotiate protocol\n");
 
-	if (ses->server)
-		server = ses->server;
-	else {
-		rc = -EIO;
-		return rc;
+	if (!server) {
+		WARN(1, "%s: server is NULL!\n", __func__);
+		return -EIO;
 	}
 
 	rc = small_smb2_init(SMB2_NEGOTIATE, NULL, (void **) &req);
@@ -480,7 +478,7 @@ SMB2_sess_setup(const unsigned int xid, struct cifs_ses *ses,
 	int rc = 0;
 	int resp_buftype;
 	__le32 phase = NtLmNegotiate; /* NTLMSSP, if needed, is multistage */
-	struct TCP_Server_Info *server;
+	struct TCP_Server_Info *server = ses->server;
 	unsigned int sec_flags;
 	u8 temp = 0;
 	u16 blob_length = 0;
@@ -490,11 +488,9 @@ SMB2_sess_setup(const unsigned int xid, struct cifs_ses *ses,
 
 	cifs_dbg(FYI, "Session Setup\n");
 
-	if (ses->server)
-		server = ses->server;
-	else {
-		rc = -EIO;
-		return rc;
+	if (!server) {
+		WARN(1, "%s: server is NULL!\n", __func__);
+		return -EIO;
 	}
 
 	/*
-- 
1.8.1.4

Re: [PATCH 04/19] cifs: throw a warning if negotiate or sess_setup ops are passed NULL server or session pointers

[Pavel Shilovsky]

2013/5/23 Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>:
> These look pretty cargo-culty to me, but let's be certain. Leave
> them in place for now. Pop a WARN if it ever does happen. Also,
> move to a more standard idiom for setting the "server" pointer.
>
> Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
>  fs/cifs/cifssmb.c | 11 +++++------
>  fs/cifs/sess.c    |  4 +++-
>  fs/cifs/smb2pdu.c | 20 ++++++++------------
>  3 files changed, 16 insertions(+), 19 deletions(-)
>
> diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
> index a58dc77..c1c2006 100644
> --- a/fs/cifs/cifssmb.c
> +++ b/fs/cifs/cifssmb.c
> @@ -375,16 +375,15 @@ CIFSSMBNegotiate(const unsigned int xid, struct cifs_ses *ses)
>         int rc = 0;
>         int bytes_returned;
>         int i;
> -       struct TCP_Server_Info *server;
> +       struct TCP_Server_Info *server = ses->server;
>         u16 count;
>         unsigned int secFlags;
>
> -       if (ses->server)
> -               server = ses->server;
> -       else {
> -               rc = -EIO;
> -               return rc;
> +       if (!server) {
> +               WARN(1, "%s: server is NULL!\n", __func__);
> +               return -EIO;
>         }
> +
>         rc = smb_init(SMB_COM_NEGOTIATE, 0, NULL /* no tcon yet */ ,
>                       (void **) &pSMB, (void **) &pSMBr);
>         if (rc)
> diff --git a/fs/cifs/sess.c b/fs/cifs/sess.c
> index 838e224..e8c5dc9 100644
> --- a/fs/cifs/sess.c
> +++ b/fs/cifs/sess.c
> @@ -576,8 +576,10 @@ CIFS_SessSetup(const unsigned int xid, struct cifs_ses *ses,
>         u16 blob_len;
>         char *ntlmsspblob = NULL;
>
> -       if (ses == NULL)
> +       if (ses == NULL) {
> +               WARN(1, "%s: ses == NULL!", __func__);
>                 return -EINVAL;
> +       }
>
>         type = ses->server->secType;
>         cifs_dbg(FYI, "sess setup type %d\n", type);
> diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
> index 2b95ce2..3af66aa 100644
> --- a/fs/cifs/smb2pdu.c
> +++ b/fs/cifs/smb2pdu.c
> @@ -328,7 +328,7 @@ SMB2_negotiate(const unsigned int xid, struct cifs_ses *ses)
>         struct kvec iov[1];
>         int rc = 0;
>         int resp_buftype;
> -       struct TCP_Server_Info *server;
> +       struct TCP_Server_Info *server = ses->server;
>         unsigned int sec_flags;
>         u16 temp = 0;
>         int blob_offset, blob_length;
> @@ -337,11 +337,9 @@ SMB2_negotiate(const unsigned int xid, struct cifs_ses *ses)
>
>         cifs_dbg(FYI, "Negotiate protocol\n");
>
> -       if (ses->server)
> -               server = ses->server;
> -       else {
> -               rc = -EIO;
> -               return rc;
> +       if (!server) {
> +               WARN(1, "%s: server is NULL!\n", __func__);
> +               return -EIO;
>         }
>
>         rc = small_smb2_init(SMB2_NEGOTIATE, NULL, (void **) &req);
> @@ -480,7 +478,7 @@ SMB2_sess_setup(const unsigned int xid, struct cifs_ses *ses,
>         int rc = 0;
>         int resp_buftype;
>         __le32 phase = NtLmNegotiate; /* NTLMSSP, if needed, is multistage */
> -       struct TCP_Server_Info *server;
> +       struct TCP_Server_Info *server = ses->server;
>         unsigned int sec_flags;
>         u8 temp = 0;
>         u16 blob_length = 0;
> @@ -490,11 +488,9 @@ SMB2_sess_setup(const unsigned int xid, struct cifs_ses *ses,
>
>         cifs_dbg(FYI, "Session Setup\n");
>
> -       if (ses->server)
> -               server = ses->server;
> -       else {
> -               rc = -EIO;
> -               return rc;
> +       if (!server) {
> +               WARN(1, "%s: server is NULL!\n", __func__);
> +               return -EIO;
>         }
>
>         /*
> --
> 1.8.1.4
>
> --
> 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

Agreed. Also we have such checks several other places - may be it's
good to make them raising a warning too?

Reviewed-by: Pavel Shilovsky <piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org>

--
Best regards,
Pavel Shilovsky.

Re: [PATCH 04/19] cifs: throw a warning if negotiate or sess_setup ops are passed NULL server or session pointers

[Jeff Layton]

On Fri, 24 May 2013 16:15:32 +0400
Pavel Shilovsky <piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:

> 2013/5/23 Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>:
> > These look pretty cargo-culty to me, but let's be certain. Leave
> > them in place for now. Pop a WARN if it ever does happen. Also,
> > move to a more standard idiom for setting the "server" pointer.
> >
> > Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> > ---
> >  fs/cifs/cifssmb.c | 11 +++++------
> >  fs/cifs/sess.c    |  4 +++-
> >  fs/cifs/smb2pdu.c | 20 ++++++++------------
> >  3 files changed, 16 insertions(+), 19 deletions(-)
> >
> > diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
> > index a58dc77..c1c2006 100644
> > --- a/fs/cifs/cifssmb.c
> > +++ b/fs/cifs/cifssmb.c
> > @@ -375,16 +375,15 @@ CIFSSMBNegotiate(const unsigned int xid, struct cifs_ses *ses)
> >         int rc = 0;
> >         int bytes_returned;
> >         int i;
> > -       struct TCP_Server_Info *server;
> > +       struct TCP_Server_Info *server = ses->server;
> >         u16 count;
> >         unsigned int secFlags;
> >
> > -       if (ses->server)
> > -               server = ses->server;
> > -       else {
> > -               rc = -EIO;
> > -               return rc;
> > +       if (!server) {
> > +               WARN(1, "%s: server is NULL!\n", __func__);
> > +               return -EIO;
> >         }
> > +
> >         rc = smb_init(SMB_COM_NEGOTIATE, 0, NULL /* no tcon yet */ ,
> >                       (void **) &pSMB, (void **) &pSMBr);
> >         if (rc)
> > diff --git a/fs/cifs/sess.c b/fs/cifs/sess.c
> > index 838e224..e8c5dc9 100644
> > --- a/fs/cifs/sess.c
> > +++ b/fs/cifs/sess.c
> > @@ -576,8 +576,10 @@ CIFS_SessSetup(const unsigned int xid, struct cifs_ses *ses,
> >         u16 blob_len;
> >         char *ntlmsspblob = NULL;
> >
> > -       if (ses == NULL)
> > +       if (ses == NULL) {
> > +               WARN(1, "%s: ses == NULL!", __func__);
> >                 return -EINVAL;
> > +       }
> >
> >         type = ses->server->secType;
> >         cifs_dbg(FYI, "sess setup type %d\n", type);
> > diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
> > index 2b95ce2..3af66aa 100644
> > --- a/fs/cifs/smb2pdu.c
> > +++ b/fs/cifs/smb2pdu.c
> > @@ -328,7 +328,7 @@ SMB2_negotiate(const unsigned int xid, struct cifs_ses *ses)
> >         struct kvec iov[1];
> >         int rc = 0;
> >         int resp_buftype;
> > -       struct TCP_Server_Info *server;
> > +       struct TCP_Server_Info *server = ses->server;
> >         unsigned int sec_flags;
> >         u16 temp = 0;
> >         int blob_offset, blob_length;
> > @@ -337,11 +337,9 @@ SMB2_negotiate(const unsigned int xid, struct cifs_ses *ses)
> >
> >         cifs_dbg(FYI, "Negotiate protocol\n");
> >
> > -       if (ses->server)
> > -               server = ses->server;
> > -       else {
> > -               rc = -EIO;
> > -               return rc;
> > +       if (!server) {
> > +               WARN(1, "%s: server is NULL!\n", __func__);
> > +               return -EIO;
> >         }
> >
> >         rc = small_smb2_init(SMB2_NEGOTIATE, NULL, (void **) &req);
> > @@ -480,7 +478,7 @@ SMB2_sess_setup(const unsigned int xid, struct cifs_ses *ses,
> >         int rc = 0;
> >         int resp_buftype;
> >         __le32 phase = NtLmNegotiate; /* NTLMSSP, if needed, is multistage */
> > -       struct TCP_Server_Info *server;
> > +       struct TCP_Server_Info *server = ses->server;
> >         unsigned int sec_flags;
> >         u8 temp = 0;
> >         u16 blob_length = 0;
> > @@ -490,11 +488,9 @@ SMB2_sess_setup(const unsigned int xid, struct cifs_ses *ses,
> >
> >         cifs_dbg(FYI, "Session Setup\n");
> >
> > -       if (ses->server)
> > -               server = ses->server;
> > -       else {
> > -               rc = -EIO;
> > -               return rc;
> > +       if (!server) {
> > +               WARN(1, "%s: server is NULL!\n", __func__);
> > +               return -EIO;
> >         }
> >
> >         /*
> > --
> > 1.8.1.4
> >
> > --
> > 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
> 
> Agreed. Also we have such checks several other places - may be it's
> good to make them raising a warning too?
> 
> Reviewed-by: Pavel Shilovsky <piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org>
> 

Yes, I think so. I just did these as I ran across them, but adding
WARN() calls to similar spots makes sense to me.

-- 
Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

[PATCH 05/19] cifs: remove the cifs_ses->flags field

[Jeff Layton]

This field is completely unused:

CIFS_SES_W9X is completely unused. CIFS_SES_LANMAN and CIFS_SES_OS2
are set but never checked. CIFS_SES_NT4 is checked, but never set.

Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 fs/cifs/cifsglob.h | 10 ----------
 fs/cifs/connect.c  |  1 -
 fs/cifs/sess.c     |  7 +------
 fs/cifs/smb1ops.c  | 18 ++++++------------
 4 files changed, 7 insertions(+), 29 deletions(-)

diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index 29dd111..be993ec 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -692,7 +692,6 @@ struct cifs_ses {
 	enum statusEnum status;
 	unsigned overrideSecFlg;  /* if non-zero override global sec flags */
 	__u16 ipc_tid;		/* special tid for connection to IPC share */
-	__u16 flags;
 	__u16 vcnum;
 	char *serverOS;		/* name of operating system underlying server */
 	char *serverNOS;	/* name of network operating system of server */
@@ -715,15 +714,6 @@ struct cifs_ses {
 #endif /* CONFIG_CIFS_SMB2 */
 };
 
-/* no more than one of the following three session flags may be set */
-#define CIFS_SES_NT4 1
-#define CIFS_SES_OS2 2
-#define CIFS_SES_W9X 4
-/* following flag is set for old servers such as OS2 (and Win95?)
-   which do not negotiate NTLM or POSIX dialects, but instead
-   negotiate one of the older LANMAN dialects */
-#define CIFS_SES_LANMAN 8
-
 static inline bool
 cap_unix(struct cifs_ses *ses)
 {
diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index c3a405a..118cc9c 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -3834,7 +3834,6 @@ cifs_setup_session(const unsigned int xid, struct cifs_ses *ses,
 	int rc = -ENOSYS;
 	struct TCP_Server_Info *server = ses->server;
 
-	ses->flags = 0;
 	ses->capabilities = server->capabilities;
 	if (linuxExtEnabled == 0)
 		ses->capabilities &= (~server->vals->cap_unix);
diff --git a/fs/cifs/sess.c b/fs/cifs/sess.c
index e8c5dc9..0d0fe38 100644
--- a/fs/cifs/sess.c
+++ b/fs/cifs/sess.c
@@ -328,10 +328,8 @@ static void decode_ascii_ssetup(char **pbcc_area, __u16 bleft,
 	ses->serverOS = kzalloc(len + 1, GFP_KERNEL);
 	if (ses->serverOS)
 		strncpy(ses->serverOS, bcc_ptr, len);
-	if (strncmp(ses->serverOS, "OS/2", 4) == 0) {
+	if (strncmp(ses->serverOS, "OS/2", 4) == 0)
 		cifs_dbg(FYI, "OS/2 server\n");
-			ses->flags |= CIFS_SES_OS2;
-	}
 
 	bcc_ptr += len + 1;
 	bleft -= len + 1;
@@ -642,8 +640,6 @@ ssetup_ntlmssp_authenticate:
 	}
 	bcc_ptr = str_area;
 
-	ses->flags &= ~CIFS_SES_LANMAN;
-
 	iov[1].iov_base = NULL;
 	iov[1].iov_len = 0;
 
@@ -667,7 +663,6 @@ ssetup_ntlmssp_authenticate:
 				 ses->server->sec_mode & SECMODE_PW_ENCRYPT ?
 					true : false, lnm_session_key);
 
-		ses->flags |= CIFS_SES_LANMAN;
 		memcpy(bcc_ptr, (char *)lnm_session_key, CIFS_AUTH_RESP_SIZE);
 		bcc_ptr += CIFS_AUTH_RESP_SIZE;
 
diff --git a/fs/cifs/smb1ops.c b/fs/cifs/smb1ops.c
index 3efdb9d..7d1c78b 100644
--- a/fs/cifs/smb1ops.c
+++ b/fs/cifs/smb1ops.c
@@ -765,20 +765,14 @@ smb_set_file_info(struct inode *inode, const char *full_path,
 	}
 	tcon = tlink_tcon(tlink);
 
-	/*
-	 * NT4 apparently returns success on this call, but it doesn't really
-	 * work.
-	 */
-	if (!(tcon->ses->flags & CIFS_SES_NT4)) {
-		rc = CIFSSMBSetPathInfo(xid, tcon, full_path, buf,
-					cifs_sb->local_nls,
+	rc = CIFSSMBSetPathInfo(xid, tcon, full_path, buf, cifs_sb->local_nls,
 					cifs_sb->mnt_cifs_flags &
 						CIFS_MOUNT_MAP_SPECIAL_CHR);
-		if (rc == 0) {
-			cinode->cifsAttrs = le32_to_cpu(buf->Attributes);
-			goto out;
-		} else if (rc != -EOPNOTSUPP && rc != -EINVAL)
-			goto out;
+	if (rc == 0) {
+		cinode->cifsAttrs = le32_to_cpu(buf->Attributes);
+		goto out;
+	} else if (rc != -EOPNOTSUPP && rc != -EINVAL) {
+		goto out;
 	}
 
 	cifs_dbg(FYI, "calling SetFileInfo since SetPathInfo for times not supported by this server\n");
-- 
1.8.1.4

Re: [PATCH 05/19] cifs: remove the cifs_ses->flags field

[Pavel Shilovsky]

2013/5/23 Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>:
> This field is completely unused:
>
> CIFS_SES_W9X is completely unused. CIFS_SES_LANMAN and CIFS_SES_OS2
> are set but never checked. CIFS_SES_NT4 is checked, but never set.
>
> Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
>  fs/cifs/cifsglob.h | 10 ----------
>  fs/cifs/connect.c  |  1 -
>  fs/cifs/sess.c     |  7 +------
>  fs/cifs/smb1ops.c  | 18 ++++++------------
>  4 files changed, 7 insertions(+), 29 deletions(-)
>
> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
> index 29dd111..be993ec 100644
> --- a/fs/cifs/cifsglob.h
> +++ b/fs/cifs/cifsglob.h
> @@ -692,7 +692,6 @@ struct cifs_ses {
>         enum statusEnum status;
>         unsigned overrideSecFlg;  /* if non-zero override global sec flags */
>         __u16 ipc_tid;          /* special tid for connection to IPC share */
> -       __u16 flags;
>         __u16 vcnum;
>         char *serverOS;         /* name of operating system underlying server */
>         char *serverNOS;        /* name of network operating system of server */
> @@ -715,15 +714,6 @@ struct cifs_ses {
>  #endif /* CONFIG_CIFS_SMB2 */
>  };
>
> -/* no more than one of the following three session flags may be set */
> -#define CIFS_SES_NT4 1
> -#define CIFS_SES_OS2 2
> -#define CIFS_SES_W9X 4
> -/* following flag is set for old servers such as OS2 (and Win95?)
> -   which do not negotiate NTLM or POSIX dialects, but instead
> -   negotiate one of the older LANMAN dialects */
> -#define CIFS_SES_LANMAN 8
> -
>  static inline bool
>  cap_unix(struct cifs_ses *ses)
>  {
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index c3a405a..118cc9c 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -3834,7 +3834,6 @@ cifs_setup_session(const unsigned int xid, struct cifs_ses *ses,
>         int rc = -ENOSYS;
>         struct TCP_Server_Info *server = ses->server;
>
> -       ses->flags = 0;
>         ses->capabilities = server->capabilities;
>         if (linuxExtEnabled == 0)
>                 ses->capabilities &= (~server->vals->cap_unix);
> diff --git a/fs/cifs/sess.c b/fs/cifs/sess.c
> index e8c5dc9..0d0fe38 100644
> --- a/fs/cifs/sess.c
> +++ b/fs/cifs/sess.c
> @@ -328,10 +328,8 @@ static void decode_ascii_ssetup(char **pbcc_area, __u16 bleft,
>         ses->serverOS = kzalloc(len + 1, GFP_KERNEL);
>         if (ses->serverOS)
>                 strncpy(ses->serverOS, bcc_ptr, len);
> -       if (strncmp(ses->serverOS, "OS/2", 4) == 0) {
> +       if (strncmp(ses->serverOS, "OS/2", 4) == 0)
>                 cifs_dbg(FYI, "OS/2 server\n");
> -                       ses->flags |= CIFS_SES_OS2;
> -       }
>
>         bcc_ptr += len + 1;
>         bleft -= len + 1;
> @@ -642,8 +640,6 @@ ssetup_ntlmssp_authenticate:
>         }
>         bcc_ptr = str_area;
>
> -       ses->flags &= ~CIFS_SES_LANMAN;
> -
>         iov[1].iov_base = NULL;
>         iov[1].iov_len = 0;
>
> @@ -667,7 +663,6 @@ ssetup_ntlmssp_authenticate:
>                                  ses->server->sec_mode & SECMODE_PW_ENCRYPT ?
>                                         true : false, lnm_session_key);
>
> -               ses->flags |= CIFS_SES_LANMAN;
>                 memcpy(bcc_ptr, (char *)lnm_session_key, CIFS_AUTH_RESP_SIZE);
>                 bcc_ptr += CIFS_AUTH_RESP_SIZE;
>
> diff --git a/fs/cifs/smb1ops.c b/fs/cifs/smb1ops.c
> index 3efdb9d..7d1c78b 100644
> --- a/fs/cifs/smb1ops.c
> +++ b/fs/cifs/smb1ops.c
> @@ -765,20 +765,14 @@ smb_set_file_info(struct inode *inode, const char *full_path,
>         }
>         tcon = tlink_tcon(tlink);
>
> -       /*
> -        * NT4 apparently returns success on this call, but it doesn't really
> -        * work.
> -        */
> -       if (!(tcon->ses->flags & CIFS_SES_NT4)) {
> -               rc = CIFSSMBSetPathInfo(xid, tcon, full_path, buf,
> -                                       cifs_sb->local_nls,
> +       rc = CIFSSMBSetPathInfo(xid, tcon, full_path, buf, cifs_sb->local_nls,
>                                         cifs_sb->mnt_cifs_flags &
>                                                 CIFS_MOUNT_MAP_SPECIAL_CHR);
> -               if (rc == 0) {
> -                       cinode->cifsAttrs = le32_to_cpu(buf->Attributes);
> -                       goto out;
> -               } else if (rc != -EOPNOTSUPP && rc != -EINVAL)
> -                       goto out;
> +       if (rc == 0) {
> +               cinode->cifsAttrs = le32_to_cpu(buf->Attributes);
> +               goto out;
> +       } else if (rc != -EOPNOTSUPP && rc != -EINVAL) {
> +               goto out;
>         }
>
>         cifs_dbg(FYI, "calling SetFileInfo since SetPathInfo for times not supported by this server\n");
> --
> 1.8.1.4
>
> --
> 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

Acked-by: Pavel Shilovsky <piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org>

--
Best regards,
Pavel Shilovsky.

[PATCH 06/19] cifs: remove "seal" stubs

[Jeff Layton]

CIFS has mount options for sealing (aka encryption), but they aren't
actually hooked up to the code and errors are not generated when someone
requests it. Ensure that no one is tricked by this by removing the stub
option handling, thereby causing a mount-time error to be generated when
someone tries to set this option.

Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 fs/cifs/cifsfs.c   |  2 --
 fs/cifs/cifsglob.h |  2 --
 fs/cifs/connect.c  | 18 +++---------------
 3 files changed, 3 insertions(+), 19 deletions(-)

diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index 3752b9f..bb27269 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -416,8 +416,6 @@ cifs_show_options(struct seq_file *s, struct dentry *root)
 		seq_printf(s, ",file_mode=0%ho,dir_mode=0%ho",
 					   cifs_sb->mnt_file_mode,
 					   cifs_sb->mnt_dir_mode);
-	if (tcon->seal)
-		seq_printf(s, ",seal");
 	if (tcon->nocase)
 		seq_printf(s, ",nocase");
 	if (tcon->retry)
diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index be993ec..874b29b 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -425,7 +425,6 @@ struct smb_vol {
 	bool nocase:1;     /* request case insensitive filenames */
 	bool nobrl:1;      /* disable sending byte range locks to srv */
 	bool mand_lock:1;  /* send mandatory not posix byte range lock reqs */
-	bool seal:1;       /* request transport encryption on share */
 	bool nodfs:1;      /* Do not request DFS, even if available */
 	bool local_lease:1; /* check leases only on local system, not remote */
 	bool noblocksnd:1;
@@ -792,7 +791,6 @@ struct cifs_tcon {
 	bool ipc:1;		/* set if connection to IPC$ eg for RPC/PIPES */
 	bool retry:1;
 	bool nocase:1;
-	bool seal:1;      /* transport encryption for this mounted share */
 	bool unix_ext:1;  /* if false disable Linux extensions to CIFS protocol
 				for this mount even if server would support */
 	bool local_lease:1; /* check leases (only) on local system not remote */
diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index 118cc9c..b367a5a 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -83,7 +83,7 @@ enum {
 	Opt_serverino, Opt_noserverino,
 	Opt_rwpidforward, Opt_cifsacl, Opt_nocifsacl,
 	Opt_acl, Opt_noacl, Opt_locallease,
-	Opt_sign, Opt_seal, Opt_noac,
+	Opt_sign, Opt_noac,
 	Opt_fsc, Opt_mfsymlinks,
 	Opt_multiuser, Opt_sloppy, Opt_nosharesock,
 
@@ -159,7 +159,6 @@ static const match_table_t cifs_mount_option_tokens = {
 	{ Opt_noacl, "noacl" },
 	{ Opt_locallease, "locallease" },
 	{ Opt_sign, "sign" },
-	{ Opt_seal, "seal" },
 	{ Opt_noac, "noac" },
 	{ Opt_fsc, "fsc" },
 	{ Opt_mfsymlinks, "mfsymlinks" },
@@ -1034,8 +1033,8 @@ static int cifs_parse_security_flavors(char *value,
 		break;
 	case Opt_sec_krb5p:
 		/* vol->secFlg |= CIFSSEC_MUST_SEAL | CIFSSEC_MAY_KRB5; */
-		cifs_dbg(VFS, "Krb5 cifs privacy not supported\n");
-		break;
+		cifs_dbg(VFS, "sec=krb5p is not supported!\n");
+		return 1;
 	case Opt_sec_ntlmssp:
 		vol->secFlg |= CIFSSEC_MAY_NTLMSSP;
 		break;
@@ -1427,14 +1426,6 @@ cifs_parse_mount_options(const char *mountdata, const char *devname,
 		case Opt_sign:
 			vol->secFlg |= CIFSSEC_MUST_SIGN;
 			break;
-		case Opt_seal:
-			/* we do not do the following in secFlags because seal
-			 * is a per tree connection (mount) not a per socket
-			 * or per-smb connection option in the protocol
-			 * vol->secFlg |= CIFSSEC_MUST_SEAL;
-			 */
-			vol->seal = 1;
-			break;
 		case Opt_noac:
 			printk(KERN_WARNING "CIFS: Mount option noac not "
 				"supported. Instead set "
@@ -2589,8 +2580,6 @@ cifs_get_tcon(struct cifs_ses *ses, struct smb_vol *volume_info)
 		cifs_dbg(FYI, "Found match on UNC path\n");
 		/* existing tcon already has a reference */
 		cifs_put_smb_ses(ses);
-		if (tcon->seal != volume_info->seal)
-			cifs_dbg(VFS, "transport encryption setting conflicts with existing tid\n");
 		return tcon;
 	}
 
@@ -2630,7 +2619,6 @@ cifs_get_tcon(struct cifs_ses *ses, struct smb_vol *volume_info)
 		tcon->Flags &= ~SMB_SHARE_IS_IN_DFS;
 		cifs_dbg(FYI, "DFS disabled (%d)\n", tcon->Flags);
 	}
-	tcon->seal = volume_info->seal;
 	/*
 	 * We can have only one retry value for a connection to a share so for
 	 * resources mounted more than once to the same server share the last
-- 
1.8.1.4

Re: [PATCH 06/19] cifs: remove "seal" stubs

[Pavel Shilovsky]

2013/5/23 Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>:
> CIFS has mount options for sealing (aka encryption), but they aren't
> actually hooked up to the code and errors are not generated when someone
> requests it. Ensure that no one is tricked by this by removing the stub
> option handling, thereby causing a mount-time error to be generated when
> someone tries to set this option.
>
> Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
>  fs/cifs/cifsfs.c   |  2 --
>  fs/cifs/cifsglob.h |  2 --
>  fs/cifs/connect.c  | 18 +++---------------
>  3 files changed, 3 insertions(+), 19 deletions(-)
>
> diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
> index 3752b9f..bb27269 100644
> --- a/fs/cifs/cifsfs.c
> +++ b/fs/cifs/cifsfs.c
> @@ -416,8 +416,6 @@ cifs_show_options(struct seq_file *s, struct dentry *root)
>                 seq_printf(s, ",file_mode=0%ho,dir_mode=0%ho",
>                                            cifs_sb->mnt_file_mode,
>                                            cifs_sb->mnt_dir_mode);
> -       if (tcon->seal)
> -               seq_printf(s, ",seal");
>         if (tcon->nocase)
>                 seq_printf(s, ",nocase");
>         if (tcon->retry)
> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
> index be993ec..874b29b 100644
> --- a/fs/cifs/cifsglob.h
> +++ b/fs/cifs/cifsglob.h
> @@ -425,7 +425,6 @@ struct smb_vol {
>         bool nocase:1;     /* request case insensitive filenames */
>         bool nobrl:1;      /* disable sending byte range locks to srv */
>         bool mand_lock:1;  /* send mandatory not posix byte range lock reqs */
> -       bool seal:1;       /* request transport encryption on share */
>         bool nodfs:1;      /* Do not request DFS, even if available */
>         bool local_lease:1; /* check leases only on local system, not remote */
>         bool noblocksnd:1;
> @@ -792,7 +791,6 @@ struct cifs_tcon {
>         bool ipc:1;             /* set if connection to IPC$ eg for RPC/PIPES */
>         bool retry:1;
>         bool nocase:1;
> -       bool seal:1;      /* transport encryption for this mounted share */
>         bool unix_ext:1;  /* if false disable Linux extensions to CIFS protocol
>                                 for this mount even if server would support */
>         bool local_lease:1; /* check leases (only) on local system not remote */
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index 118cc9c..b367a5a 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -83,7 +83,7 @@ enum {
>         Opt_serverino, Opt_noserverino,
>         Opt_rwpidforward, Opt_cifsacl, Opt_nocifsacl,
>         Opt_acl, Opt_noacl, Opt_locallease,
> -       Opt_sign, Opt_seal, Opt_noac,
> +       Opt_sign, Opt_noac,
>         Opt_fsc, Opt_mfsymlinks,
>         Opt_multiuser, Opt_sloppy, Opt_nosharesock,
>
> @@ -159,7 +159,6 @@ static const match_table_t cifs_mount_option_tokens = {
>         { Opt_noacl, "noacl" },
>         { Opt_locallease, "locallease" },
>         { Opt_sign, "sign" },
> -       { Opt_seal, "seal" },
>         { Opt_noac, "noac" },
>         { Opt_fsc, "fsc" },
>         { Opt_mfsymlinks, "mfsymlinks" },
> @@ -1034,8 +1033,8 @@ static int cifs_parse_security_flavors(char *value,
>                 break;
>         case Opt_sec_krb5p:
>                 /* vol->secFlg |= CIFSSEC_MUST_SEAL | CIFSSEC_MAY_KRB5; */
> -               cifs_dbg(VFS, "Krb5 cifs privacy not supported\n");
> -               break;
> +               cifs_dbg(VFS, "sec=krb5p is not supported!\n");
> +               return 1;
>         case Opt_sec_ntlmssp:
>                 vol->secFlg |= CIFSSEC_MAY_NTLMSSP;
>                 break;
> @@ -1427,14 +1426,6 @@ cifs_parse_mount_options(const char *mountdata, const char *devname,
>                 case Opt_sign:
>                         vol->secFlg |= CIFSSEC_MUST_SIGN;
>                         break;
> -               case Opt_seal:
> -                       /* we do not do the following in secFlags because seal
> -                        * is a per tree connection (mount) not a per socket
> -                        * or per-smb connection option in the protocol
> -                        * vol->secFlg |= CIFSSEC_MUST_SEAL;
> -                        */
> -                       vol->seal = 1;
> -                       break;
>                 case Opt_noac:
>                         printk(KERN_WARNING "CIFS: Mount option noac not "
>                                 "supported. Instead set "
> @@ -2589,8 +2580,6 @@ cifs_get_tcon(struct cifs_ses *ses, struct smb_vol *volume_info)
>                 cifs_dbg(FYI, "Found match on UNC path\n");
>                 /* existing tcon already has a reference */
>                 cifs_put_smb_ses(ses);
> -               if (tcon->seal != volume_info->seal)
> -                       cifs_dbg(VFS, "transport encryption setting conflicts with existing tid\n");
>                 return tcon;
>         }
>
> @@ -2630,7 +2619,6 @@ cifs_get_tcon(struct cifs_ses *ses, struct smb_vol *volume_info)
>                 tcon->Flags &= ~SMB_SHARE_IS_IN_DFS;
>                 cifs_dbg(FYI, "DFS disabled (%d)\n", tcon->Flags);
>         }
> -       tcon->seal = volume_info->seal;
>         /*
>          * We can have only one retry value for a connection to a share so for
>          * resources mounted more than once to the same server share the last
> --
> 1.8.1.4
>
> --
> 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

Acked-by: Pavel Shilovsky <piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org>

--
Best regards,
Pavel Shilovsky.

Re: [PATCH 06/19] cifs: remove "seal" stubs

[Steve French]

As soon as Shirish fixes the signing code, don't we need these stubs
(for per-share smb3 encryption - albeit we could remove them from the
cifs only code path since the cifs seal code allowed with unix
extenions turned out to be hard to implement due to a design problem
we didn't notice at the time in the unix specification).

On Thu, May 23, 2013 at 10:05 AM, Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> CIFS has mount options for sealing (aka encryption), but they aren't
> actually hooked up to the code and errors are not generated when someone
> requests it. Ensure that no one is tricked by this by removing the stub
> option handling, thereby causing a mount-time error to be generated when
> someone tries to set this option.
>
> Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
>  fs/cifs/cifsfs.c   |  2 --
>  fs/cifs/cifsglob.h |  2 --
>  fs/cifs/connect.c  | 18 +++---------------
>  3 files changed, 3 insertions(+), 19 deletions(-)
>
> diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
> index 3752b9f..bb27269 100644
> --- a/fs/cifs/cifsfs.c
> +++ b/fs/cifs/cifsfs.c
> @@ -416,8 +416,6 @@ cifs_show_options(struct seq_file *s, struct dentry *root)
>                 seq_printf(s, ",file_mode=0%ho,dir_mode=0%ho",
>                                            cifs_sb->mnt_file_mode,
>                                            cifs_sb->mnt_dir_mode);
> -       if (tcon->seal)
> -               seq_printf(s, ",seal");
>         if (tcon->nocase)
>                 seq_printf(s, ",nocase");
>         if (tcon->retry)
> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
> index be993ec..874b29b 100644
> --- a/fs/cifs/cifsglob.h
> +++ b/fs/cifs/cifsglob.h
> @@ -425,7 +425,6 @@ struct smb_vol {
>         bool nocase:1;     /* request case insensitive filenames */
>         bool nobrl:1;      /* disable sending byte range locks to srv */
>         bool mand_lock:1;  /* send mandatory not posix byte range lock reqs */
> -       bool seal:1;       /* request transport encryption on share */
>         bool nodfs:1;      /* Do not request DFS, even if available */
>         bool local_lease:1; /* check leases only on local system, not remote */
>         bool noblocksnd:1;
> @@ -792,7 +791,6 @@ struct cifs_tcon {
>         bool ipc:1;             /* set if connection to IPC$ eg for RPC/PIPES */
>         bool retry:1;
>         bool nocase:1;
> -       bool seal:1;      /* transport encryption for this mounted share */
>         bool unix_ext:1;  /* if false disable Linux extensions to CIFS protocol
>                                 for this mount even if server would support */
>         bool local_lease:1; /* check leases (only) on local system not remote */
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index 118cc9c..b367a5a 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -83,7 +83,7 @@ enum {
>         Opt_serverino, Opt_noserverino,
>         Opt_rwpidforward, Opt_cifsacl, Opt_nocifsacl,
>         Opt_acl, Opt_noacl, Opt_locallease,
> -       Opt_sign, Opt_seal, Opt_noac,
> +       Opt_sign, Opt_noac,
>         Opt_fsc, Opt_mfsymlinks,
>         Opt_multiuser, Opt_sloppy, Opt_nosharesock,
>
> @@ -159,7 +159,6 @@ static const match_table_t cifs_mount_option_tokens = {
>         { Opt_noacl, "noacl" },
>         { Opt_locallease, "locallease" },
>         { Opt_sign, "sign" },
> -       { Opt_seal, "seal" },
>         { Opt_noac, "noac" },
>         { Opt_fsc, "fsc" },
>         { Opt_mfsymlinks, "mfsymlinks" },
> @@ -1034,8 +1033,8 @@ static int cifs_parse_security_flavors(char *value,
>                 break;
>         case Opt_sec_krb5p:
>                 /* vol->secFlg |= CIFSSEC_MUST_SEAL | CIFSSEC_MAY_KRB5; */
> -               cifs_dbg(VFS, "Krb5 cifs privacy not supported\n");
> -               break;
> +               cifs_dbg(VFS, "sec=krb5p is not supported!\n");
> +               return 1;
>         case Opt_sec_ntlmssp:
>                 vol->secFlg |= CIFSSEC_MAY_NTLMSSP;
>                 break;
> @@ -1427,14 +1426,6 @@ cifs_parse_mount_options(const char *mountdata, const char *devname,
>                 case Opt_sign:
>                         vol->secFlg |= CIFSSEC_MUST_SIGN;
>                         break;
> -               case Opt_seal:
> -                       /* we do not do the following in secFlags because seal
> -                        * is a per tree connection (mount) not a per socket
> -                        * or per-smb connection option in the protocol
> -                        * vol->secFlg |= CIFSSEC_MUST_SEAL;
> -                        */
> -                       vol->seal = 1;
> -                       break;
>                 case Opt_noac:
>                         printk(KERN_WARNING "CIFS: Mount option noac not "
>                                 "supported. Instead set "
> @@ -2589,8 +2580,6 @@ cifs_get_tcon(struct cifs_ses *ses, struct smb_vol *volume_info)
>                 cifs_dbg(FYI, "Found match on UNC path\n");
>                 /* existing tcon already has a reference */
>                 cifs_put_smb_ses(ses);
> -               if (tcon->seal != volume_info->seal)
> -                       cifs_dbg(VFS, "transport encryption setting conflicts with existing tid\n");
>                 return tcon;
>         }
>
> @@ -2630,7 +2619,6 @@ cifs_get_tcon(struct cifs_ses *ses, struct smb_vol *volume_info)
>                 tcon->Flags &= ~SMB_SHARE_IS_IN_DFS;
>                 cifs_dbg(FYI, "DFS disabled (%d)\n", tcon->Flags);
>         }
> -       tcon->seal = volume_info->seal;
>         /*
>          * We can have only one retry value for a connection to a share so for
>          * resources mounted more than once to the same server share the last
> --
> 1.8.1.4
>



-- 
Thanks,

Steve

Re: [PATCH 06/19] cifs: remove "seal" stubs

[Jeff Layton]

On Fri, 24 May 2013 13:32:51 -0500
Steve French <smfrench-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:

> As soon as Shirish fixes the signing code, don't we need these stubs
> (for per-share smb3 encryption - albeit we could remove them from the
> cifs only code path since the cifs seal code allowed with unix
> extenions turned out to be hard to implement due to a design problem
> we didn't notice at the time in the unix specification).
> 

I think we should just let these flags go for now, and reimplement them
as they are needed. This patchset is pretty complex already, and taking
this out simplifies things for this patchset and likely for Shirish's
too.

His code may or may not end up using these fields. It makes sense to me
to just allow Shirish to add the fields that he thinks are necessary to
support his code, rather than making assumptions about it before that
code has materialised. For instance, the seal field is attached to the
tcon here which really makes no sense for smb3...

Also, signing and sealing are semi-related, so it might make sense to
have some sort of 3 way switch that determines what sort of integrity or
privacy are on the connection, rather than a pair of bools...

> On Thu, May 23, 2013 at 10:05 AM, Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> > CIFS has mount options for sealing (aka encryption), but they aren't
> > actually hooked up to the code and errors are not generated when someone
> > requests it. Ensure that no one is tricked by this by removing the stub
> > option handling, thereby causing a mount-time error to be generated when
> > someone tries to set this option.
> >
> > Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> > ---
> >  fs/cifs/cifsfs.c   |  2 --
> >  fs/cifs/cifsglob.h |  2 --
> >  fs/cifs/connect.c  | 18 +++---------------
> >  3 files changed, 3 insertions(+), 19 deletions(-)
> >
> > diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
> > index 3752b9f..bb27269 100644
> > --- a/fs/cifs/cifsfs.c
> > +++ b/fs/cifs/cifsfs.c
> > @@ -416,8 +416,6 @@ cifs_show_options(struct seq_file *s, struct dentry *root)
> >                 seq_printf(s, ",file_mode=0%ho,dir_mode=0%ho",
> >                                            cifs_sb->mnt_file_mode,
> >                                            cifs_sb->mnt_dir_mode);
> > -       if (tcon->seal)
> > -               seq_printf(s, ",seal");
> >         if (tcon->nocase)
> >                 seq_printf(s, ",nocase");
> >         if (tcon->retry)
> > diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
> > index be993ec..874b29b 100644
> > --- a/fs/cifs/cifsglob.h
> > +++ b/fs/cifs/cifsglob.h
> > @@ -425,7 +425,6 @@ struct smb_vol {
> >         bool nocase:1;     /* request case insensitive filenames */
> >         bool nobrl:1;      /* disable sending byte range locks to srv */
> >         bool mand_lock:1;  /* send mandatory not posix byte range lock reqs */
> > -       bool seal:1;       /* request transport encryption on share */
> >         bool nodfs:1;      /* Do not request DFS, even if available */
> >         bool local_lease:1; /* check leases only on local system, not remote */
> >         bool noblocksnd:1;
> > @@ -792,7 +791,6 @@ struct cifs_tcon {
> >         bool ipc:1;             /* set if connection to IPC$ eg for RPC/PIPES */
> >         bool retry:1;
> >         bool nocase:1;
> > -       bool seal:1;      /* transport encryption for this mounted share */
> >         bool unix_ext:1;  /* if false disable Linux extensions to CIFS protocol
> >                                 for this mount even if server would support */
> >         bool local_lease:1; /* check leases (only) on local system not remote */
> > diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> > index 118cc9c..b367a5a 100644
> > --- a/fs/cifs/connect.c
> > +++ b/fs/cifs/connect.c
> > @@ -83,7 +83,7 @@ enum {
> >         Opt_serverino, Opt_noserverino,
> >         Opt_rwpidforward, Opt_cifsacl, Opt_nocifsacl,
> >         Opt_acl, Opt_noacl, Opt_locallease,
> > -       Opt_sign, Opt_seal, Opt_noac,
> > +       Opt_sign, Opt_noac,
> >         Opt_fsc, Opt_mfsymlinks,
> >         Opt_multiuser, Opt_sloppy, Opt_nosharesock,
> >
> > @@ -159,7 +159,6 @@ static const match_table_t cifs_mount_option_tokens = {
> >         { Opt_noacl, "noacl" },
> >         { Opt_locallease, "locallease" },
> >         { Opt_sign, "sign" },
> > -       { Opt_seal, "seal" },
> >         { Opt_noac, "noac" },
> >         { Opt_fsc, "fsc" },
> >         { Opt_mfsymlinks, "mfsymlinks" },
> > @@ -1034,8 +1033,8 @@ static int cifs_parse_security_flavors(char *value,
> >                 break;
> >         case Opt_sec_krb5p:
> >                 /* vol->secFlg |= CIFSSEC_MUST_SEAL | CIFSSEC_MAY_KRB5; */
> > -               cifs_dbg(VFS, "Krb5 cifs privacy not supported\n");
> > -               break;
> > +               cifs_dbg(VFS, "sec=krb5p is not supported!\n");
> > +               return 1;
> >         case Opt_sec_ntlmssp:
> >                 vol->secFlg |= CIFSSEC_MAY_NTLMSSP;
> >                 break;
> > @@ -1427,14 +1426,6 @@ cifs_parse_mount_options(const char *mountdata, const char *devname,
> >                 case Opt_sign:
> >                         vol->secFlg |= CIFSSEC_MUST_SIGN;
> >                         break;
> > -               case Opt_seal:
> > -                       /* we do not do the following in secFlags because seal
> > -                        * is a per tree connection (mount) not a per socket
> > -                        * or per-smb connection option in the protocol
> > -                        * vol->secFlg |= CIFSSEC_MUST_SEAL;
> > -                        */
> > -                       vol->seal = 1;
> > -                       break;
> >                 case Opt_noac:
> >                         printk(KERN_WARNING "CIFS: Mount option noac not "
> >                                 "supported. Instead set "
> > @@ -2589,8 +2580,6 @@ cifs_get_tcon(struct cifs_ses *ses, struct smb_vol *volume_info)
> >                 cifs_dbg(FYI, "Found match on UNC path\n");
> >                 /* existing tcon already has a reference */
> >                 cifs_put_smb_ses(ses);
> > -               if (tcon->seal != volume_info->seal)
> > -                       cifs_dbg(VFS, "transport encryption setting conflicts with existing tid\n");
> >                 return tcon;
> >         }
> >
> > @@ -2630,7 +2619,6 @@ cifs_get_tcon(struct cifs_ses *ses, struct smb_vol *volume_info)
> >                 tcon->Flags &= ~SMB_SHARE_IS_IN_DFS;
> >                 cifs_dbg(FYI, "DFS disabled (%d)\n", tcon->Flags);
> >         }
> > -       tcon->seal = volume_info->seal;
> >         /*
> >          * We can have only one retry value for a connection to a share so for
> >          * resources mounted more than once to the same server share the last
> > --
> > 1.8.1.4
> >
> 
> 
> 


-- 
Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

Re: [PATCH 06/19] cifs: remove "seal" stubs

[Shirish Pargaonkar]

Yes, I think I agree with Jeff.  When we reach that point of coding
sealing/encryption, these options can be added back as per the protocol.

Regards,

Shirish

On Fri, May 24, 2013 at 2:42 PM, Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> On Fri, 24 May 2013 13:32:51 -0500
> Steve French <smfrench-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>
>> As soon as Shirish fixes the signing code, don't we need these stubs
>> (for per-share smb3 encryption - albeit we could remove them from the
>> cifs only code path since the cifs seal code allowed with unix
>> extenions turned out to be hard to implement due to a design problem
>> we didn't notice at the time in the unix specification).
>>
>
> I think we should just let these flags go for now, and reimplement them
> as they are needed. This patchset is pretty complex already, and taking
> this out simplifies things for this patchset and likely for Shirish's
> too.
>
> His code may or may not end up using these fields. It makes sense to me
> to just allow Shirish to add the fields that he thinks are necessary to
> support his code, rather than making assumptions about it before that
> code has materialised. For instance, the seal field is attached to the
> tcon here which really makes no sense for smb3...
>
> Also, signing and sealing are semi-related, so it might make sense to
> have some sort of 3 way switch that determines what sort of integrity or
> privacy are on the connection, rather than a pair of bools...
>
>> On Thu, May 23, 2013 at 10:05 AM, Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
>> > CIFS has mount options for sealing (aka encryption), but they aren't
>> > actually hooked up to the code and errors are not generated when someone
>> > requests it. Ensure that no one is tricked by this by removing the stub
>> > option handling, thereby causing a mount-time error to be generated when
>> > someone tries to set this option.
>> >
>> > Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>> > ---
>> >  fs/cifs/cifsfs.c   |  2 --
>> >  fs/cifs/cifsglob.h |  2 --
>> >  fs/cifs/connect.c  | 18 +++---------------
>> >  3 files changed, 3 insertions(+), 19 deletions(-)
>> >
>> > diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
>> > index 3752b9f..bb27269 100644
>> > --- a/fs/cifs/cifsfs.c
>> > +++ b/fs/cifs/cifsfs.c
>> > @@ -416,8 +416,6 @@ cifs_show_options(struct seq_file *s, struct dentry *root)
>> >                 seq_printf(s, ",file_mode=0%ho,dir_mode=0%ho",
>> >                                            cifs_sb->mnt_file_mode,
>> >                                            cifs_sb->mnt_dir_mode);
>> > -       if (tcon->seal)
>> > -               seq_printf(s, ",seal");
>> >         if (tcon->nocase)
>> >                 seq_printf(s, ",nocase");
>> >         if (tcon->retry)
>> > diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
>> > index be993ec..874b29b 100644
>> > --- a/fs/cifs/cifsglob.h
>> > +++ b/fs/cifs/cifsglob.h
>> > @@ -425,7 +425,6 @@ struct smb_vol {
>> >         bool nocase:1;     /* request case insensitive filenames */
>> >         bool nobrl:1;      /* disable sending byte range locks to srv */
>> >         bool mand_lock:1;  /* send mandatory not posix byte range lock reqs */
>> > -       bool seal:1;       /* request transport encryption on share */
>> >         bool nodfs:1;      /* Do not request DFS, even if available */
>> >         bool local_lease:1; /* check leases only on local system, not remote */
>> >         bool noblocksnd:1;
>> > @@ -792,7 +791,6 @@ struct cifs_tcon {
>> >         bool ipc:1;             /* set if connection to IPC$ eg for RPC/PIPES */
>> >         bool retry:1;
>> >         bool nocase:1;
>> > -       bool seal:1;      /* transport encryption for this mounted share */
>> >         bool unix_ext:1;  /* if false disable Linux extensions to CIFS protocol
>> >                                 for this mount even if server would support */
>> >         bool local_lease:1; /* check leases (only) on local system not remote */
>> > diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
>> > index 118cc9c..b367a5a 100644
>> > --- a/fs/cifs/connect.c
>> > +++ b/fs/cifs/connect.c
>> > @@ -83,7 +83,7 @@ enum {
>> >         Opt_serverino, Opt_noserverino,
>> >         Opt_rwpidforward, Opt_cifsacl, Opt_nocifsacl,
>> >         Opt_acl, Opt_noacl, Opt_locallease,
>> > -       Opt_sign, Opt_seal, Opt_noac,
>> > +       Opt_sign, Opt_noac,
>> >         Opt_fsc, Opt_mfsymlinks,
>> >         Opt_multiuser, Opt_sloppy, Opt_nosharesock,
>> >
>> > @@ -159,7 +159,6 @@ static const match_table_t cifs_mount_option_tokens = {
>> >         { Opt_noacl, "noacl" },
>> >         { Opt_locallease, "locallease" },
>> >         { Opt_sign, "sign" },
>> > -       { Opt_seal, "seal" },
>> >         { Opt_noac, "noac" },
>> >         { Opt_fsc, "fsc" },
>> >         { Opt_mfsymlinks, "mfsymlinks" },
>> > @@ -1034,8 +1033,8 @@ static int cifs_parse_security_flavors(char *value,
>> >                 break;
>> >         case Opt_sec_krb5p:
>> >                 /* vol->secFlg |= CIFSSEC_MUST_SEAL | CIFSSEC_MAY_KRB5; */
>> > -               cifs_dbg(VFS, "Krb5 cifs privacy not supported\n");
>> > -               break;
>> > +               cifs_dbg(VFS, "sec=krb5p is not supported!\n");
>> > +               return 1;
>> >         case Opt_sec_ntlmssp:
>> >                 vol->secFlg |= CIFSSEC_MAY_NTLMSSP;
>> >                 break;
>> > @@ -1427,14 +1426,6 @@ cifs_parse_mount_options(const char *mountdata, const char *devname,
>> >                 case Opt_sign:
>> >                         vol->secFlg |= CIFSSEC_MUST_SIGN;
>> >                         break;
>> > -               case Opt_seal:
>> > -                       /* we do not do the following in secFlags because seal
>> > -                        * is a per tree connection (mount) not a per socket
>> > -                        * or per-smb connection option in the protocol
>> > -                        * vol->secFlg |= CIFSSEC_MUST_SEAL;
>> > -                        */
>> > -                       vol->seal = 1;
>> > -                       break;
>> >                 case Opt_noac:
>> >                         printk(KERN_WARNING "CIFS: Mount option noac not "
>> >                                 "supported. Instead set "
>> > @@ -2589,8 +2580,6 @@ cifs_get_tcon(struct cifs_ses *ses, struct smb_vol *volume_info)
>> >                 cifs_dbg(FYI, "Found match on UNC path\n");
>> >                 /* existing tcon already has a reference */
>> >                 cifs_put_smb_ses(ses);
>> > -               if (tcon->seal != volume_info->seal)
>> > -                       cifs_dbg(VFS, "transport encryption setting conflicts with existing tid\n");
>> >                 return tcon;
>> >         }
>> >
>> > @@ -2630,7 +2619,6 @@ cifs_get_tcon(struct cifs_ses *ses, struct smb_vol *volume_info)
>> >                 tcon->Flags &= ~SMB_SHARE_IS_IN_DFS;
>> >                 cifs_dbg(FYI, "DFS disabled (%d)\n", tcon->Flags);
>> >         }
>> > -       tcon->seal = volume_info->seal;
>> >         /*
>> >          * We can have only one retry value for a connection to a share so for
>> >          * resources mounted more than once to the same server share the last
>> > --
>> > 1.8.1.4
>> >
>>
>>
>>
>
>
> --
> Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> --
> 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 07/19] cifs: break out decoding of security blob into separate function

[Jeff Layton]

...cleanup.

Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 fs/cifs/cifspdu.h |   4 +-
 fs/cifs/cifssmb.c | 109 ++++++++++++++++++++++++++++++------------------------
 2 files changed, 62 insertions(+), 51 deletions(-)

diff --git a/fs/cifs/cifspdu.h b/fs/cifs/cifspdu.h
index e996ff6..4e6135a 100644
--- a/fs/cifs/cifspdu.h
+++ b/fs/cifs/cifspdu.h
@@ -531,7 +531,7 @@ typedef struct lanman_neg_rsp {
 #define READ_RAW_ENABLE 1
 #define WRITE_RAW_ENABLE 2
 #define RAW_ENABLE (READ_RAW_ENABLE | WRITE_RAW_ENABLE)
-
+#define SMB1_CLIENT_GUID_SIZE (16)
 typedef struct negotiate_rsp {
 	struct smb_hdr hdr;	/* wct = 17 */
 	__le16 DialectIndex; /* 0xFFFF = no dialect acceptable */
@@ -553,7 +553,7 @@ typedef struct negotiate_rsp {
 		/* followed by 16 bytes of server GUID */
 		/* then security blob if cap_extended_security negotiated */
 		struct {
-			unsigned char GUID[16];
+			unsigned char GUID[SMB1_CLIENT_GUID_SIZE];
 			unsigned char SecurityBlob[1];
 		} __attribute__((packed)) extended_response;
 	} __attribute__((packed)) u;
diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
index c1c2006..9b4aea8 100644
--- a/fs/cifs/cifssmb.c
+++ b/fs/cifs/cifssmb.c
@@ -367,6 +367,56 @@ vt2_err:
 	return -EINVAL;
 }
 
+static int
+decode_ext_sec_blob(struct TCP_Server_Info *server, NEGOTIATE_RSP *pSMBr)
+{
+	int	rc = 0;
+	u16	count;
+	char	*guid = pSMBr->u.extended_response.GUID;
+
+	count = get_bcc(&pSMBr->hdr);
+	if (count < SMB1_CLIENT_GUID_SIZE)
+		return -EIO;
+
+	spin_lock(&cifs_tcp_ses_lock);
+	if (server->srv_count > 1) {
+		spin_unlock(&cifs_tcp_ses_lock);
+		if (memcmp(server->server_GUID, guid, SMB1_CLIENT_GUID_SIZE) != 0) {
+			cifs_dbg(FYI, "server UID changed\n");
+			memcpy(server->server_GUID, guid, SMB1_CLIENT_GUID_SIZE);
+		}
+	} else {
+		spin_unlock(&cifs_tcp_ses_lock);
+		memcpy(server->server_GUID, guid, SMB1_CLIENT_GUID_SIZE);
+	}
+
+	if (count == SMB1_CLIENT_GUID_SIZE) {
+		server->secType = RawNTLMSSP;
+	} else {
+		count -= SMB1_CLIENT_GUID_SIZE;
+		rc = decode_negTokenInit(
+			pSMBr->u.extended_response.SecurityBlob, count, server);
+		if (rc != 1)
+			return -EINVAL;
+
+		/* Make sure server supports what we want to use */
+		switch(server->secType) {
+		case Kerberos:
+			if (!server->sec_kerberos && !server->sec_mskerberos)
+				return -EOPNOTSUPP;
+			break;
+		case RawNTLMSSP:
+			if (!server->sec_ntlmssp)
+				return -EOPNOTSUPP;
+			break;
+		default:
+			return -EOPNOTSUPP;
+		}
+	}
+
+	return 0;
+}
+
 int
 CIFSSMBNegotiate(const unsigned int xid, struct cifs_ses *ses)
 {
@@ -568,61 +618,22 @@ CIFSSMBNegotiate(const unsigned int xid, struct cifs_ses *ses)
 	server->capabilities = le32_to_cpu(pSMBr->Capabilities);
 	server->timeAdj = (int)(__s16)le16_to_cpu(pSMBr->ServerTimeZone);
 	server->timeAdj *= 60;
-	if (pSMBr->EncryptionKeyLength == CIFS_CRYPTO_KEY_SIZE) {
+
+	if (pSMBr->EncryptionKeyLength == CIFS_CRYPTO_KEY_SIZE)
 		memcpy(ses->server->cryptkey, pSMBr->u.EncryptionKey,
 		       CIFS_CRYPTO_KEY_SIZE);
-	} else if ((pSMBr->hdr.Flags2 & SMBFLG2_EXT_SEC ||
+	else if ((pSMBr->hdr.Flags2 & SMBFLG2_EXT_SEC ||
 			server->capabilities & CAP_EXTENDED_SECURITY) &&
-				(pSMBr->EncryptionKeyLength == 0)) {
-		/* decode security blob */
-		count = get_bcc(&pSMBr->hdr);
-		if (count < 16) {
-			rc = -EIO;
-			goto neg_err_exit;
-		}
-		spin_lock(&cifs_tcp_ses_lock);
-		if (server->srv_count > 1) {
-			spin_unlock(&cifs_tcp_ses_lock);
-			if (memcmp(server->server_GUID,
-				   pSMBr->u.extended_response.
-				   GUID, 16) != 0) {
-				cifs_dbg(FYI, "server UID changed\n");
-				memcpy(server->server_GUID,
-					pSMBr->u.extended_response.GUID,
-					16);
-			}
-		} else {
-			spin_unlock(&cifs_tcp_ses_lock);
-			memcpy(server->server_GUID,
-			       pSMBr->u.extended_response.GUID, 16);
-		}
-
-		if (count == 16) {
-			server->secType = RawNTLMSSP;
-		} else {
-			rc = decode_negTokenInit(pSMBr->u.extended_response.
-						 SecurityBlob, count - 16,
-						 server);
-			if (rc == 1)
-				rc = 0;
-			else
-				rc = -EINVAL;
-			if (server->secType == Kerberos) {
-				if (!server->sec_kerberos &&
-						!server->sec_mskerberos)
-					rc = -EOPNOTSUPP;
-			} else if (server->secType == RawNTLMSSP) {
-				if (!server->sec_ntlmssp)
-					rc = -EOPNOTSUPP;
-			} else
-					rc = -EOPNOTSUPP;
-		}
-	} else if (server->sec_mode & SECMODE_PW_ENCRYPT) {
+				(pSMBr->EncryptionKeyLength == 0))
+		rc = decode_ext_sec_blob(server, pSMBr);
+	else if (server->sec_mode & SECMODE_PW_ENCRYPT)
 		rc = -EIO; /* no crypt key only if plain text pwd */
-		goto neg_err_exit;
-	} else
+	else
 		server->capabilities &= ~CAP_EXTENDED_SECURITY;
 
+	if (rc)
+		goto neg_err_exit;
+
 #ifdef CONFIG_CIFS_WEAK_PW_HASH
 signing_check:
 #endif
-- 
1.8.1.4

Re: [PATCH 07/19] cifs: break out decoding of security blob into separate function

[Pavel Shilovsky]

2013/5/23 Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>:
> ...cleanup.
>
> Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
>  fs/cifs/cifspdu.h |   4 +-
>  fs/cifs/cifssmb.c | 109 ++++++++++++++++++++++++++++++------------------------
>  2 files changed, 62 insertions(+), 51 deletions(-)
>
> diff --git a/fs/cifs/cifspdu.h b/fs/cifs/cifspdu.h
> index e996ff6..4e6135a 100644
> --- a/fs/cifs/cifspdu.h
> +++ b/fs/cifs/cifspdu.h
> @@ -531,7 +531,7 @@ typedef struct lanman_neg_rsp {
>  #define READ_RAW_ENABLE 1
>  #define WRITE_RAW_ENABLE 2
>  #define RAW_ENABLE (READ_RAW_ENABLE | WRITE_RAW_ENABLE)
> -
> +#define SMB1_CLIENT_GUID_SIZE (16)
>  typedef struct negotiate_rsp {
>         struct smb_hdr hdr;     /* wct = 17 */
>         __le16 DialectIndex; /* 0xFFFF = no dialect acceptable */
> @@ -553,7 +553,7 @@ typedef struct negotiate_rsp {
>                 /* followed by 16 bytes of server GUID */
>                 /* then security blob if cap_extended_security negotiated */
>                 struct {
> -                       unsigned char GUID[16];
> +                       unsigned char GUID[SMB1_CLIENT_GUID_SIZE];
>                         unsigned char SecurityBlob[1];
>                 } __attribute__((packed)) extended_response;
>         } __attribute__((packed)) u;
> diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
> index c1c2006..9b4aea8 100644
> --- a/fs/cifs/cifssmb.c
> +++ b/fs/cifs/cifssmb.c
> @@ -367,6 +367,56 @@ vt2_err:
>         return -EINVAL;
>  }
>
> +static int
> +decode_ext_sec_blob(struct TCP_Server_Info *server, NEGOTIATE_RSP *pSMBr)
> +{
> +       int     rc = 0;
> +       u16     count;
> +       char    *guid = pSMBr->u.extended_response.GUID;
> +
> +       count = get_bcc(&pSMBr->hdr);
> +       if (count < SMB1_CLIENT_GUID_SIZE)
> +               return -EIO;
> +
> +       spin_lock(&cifs_tcp_ses_lock);
> +       if (server->srv_count > 1) {
> +               spin_unlock(&cifs_tcp_ses_lock);
> +               if (memcmp(server->server_GUID, guid, SMB1_CLIENT_GUID_SIZE) != 0) {
> +                       cifs_dbg(FYI, "server UID changed\n");
> +                       memcpy(server->server_GUID, guid, SMB1_CLIENT_GUID_SIZE);
> +               }
> +       } else {
> +               spin_unlock(&cifs_tcp_ses_lock);
> +               memcpy(server->server_GUID, guid, SMB1_CLIENT_GUID_SIZE);
> +       }
> +
> +       if (count == SMB1_CLIENT_GUID_SIZE) {
> +               server->secType = RawNTLMSSP;
> +       } else {
> +               count -= SMB1_CLIENT_GUID_SIZE;
> +               rc = decode_negTokenInit(
> +                       pSMBr->u.extended_response.SecurityBlob, count, server);
> +               if (rc != 1)
> +                       return -EINVAL;
> +
> +               /* Make sure server supports what we want to use */
> +               switch(server->secType) {
> +               case Kerberos:
> +                       if (!server->sec_kerberos && !server->sec_mskerberos)
> +                               return -EOPNOTSUPP;
> +                       break;
> +               case RawNTLMSSP:
> +                       if (!server->sec_ntlmssp)
> +                               return -EOPNOTSUPP;
> +                       break;
> +               default:
> +                       return -EOPNOTSUPP;
> +               }
> +       }
> +
> +       return 0;
> +}
> +
>  int
>  CIFSSMBNegotiate(const unsigned int xid, struct cifs_ses *ses)
>  {
> @@ -568,61 +618,22 @@ CIFSSMBNegotiate(const unsigned int xid, struct cifs_ses *ses)
>         server->capabilities = le32_to_cpu(pSMBr->Capabilities);
>         server->timeAdj = (int)(__s16)le16_to_cpu(pSMBr->ServerTimeZone);
>         server->timeAdj *= 60;
> -       if (pSMBr->EncryptionKeyLength == CIFS_CRYPTO_KEY_SIZE) {
> +
> +       if (pSMBr->EncryptionKeyLength == CIFS_CRYPTO_KEY_SIZE)
>                 memcpy(ses->server->cryptkey, pSMBr->u.EncryptionKey,
>                        CIFS_CRYPTO_KEY_SIZE);
> -       } else if ((pSMBr->hdr.Flags2 & SMBFLG2_EXT_SEC ||
> +       else if ((pSMBr->hdr.Flags2 & SMBFLG2_EXT_SEC ||
>                         server->capabilities & CAP_EXTENDED_SECURITY) &&
> -                               (pSMBr->EncryptionKeyLength == 0)) {
> -               /* decode security blob */
> -               count = get_bcc(&pSMBr->hdr);
> -               if (count < 16) {
> -                       rc = -EIO;
> -                       goto neg_err_exit;
> -               }
> -               spin_lock(&cifs_tcp_ses_lock);
> -               if (server->srv_count > 1) {
> -                       spin_unlock(&cifs_tcp_ses_lock);
> -                       if (memcmp(server->server_GUID,
> -                                  pSMBr->u.extended_response.
> -                                  GUID, 16) != 0) {
> -                               cifs_dbg(FYI, "server UID changed\n");
> -                               memcpy(server->server_GUID,
> -                                       pSMBr->u.extended_response.GUID,
> -                                       16);
> -                       }
> -               } else {
> -                       spin_unlock(&cifs_tcp_ses_lock);
> -                       memcpy(server->server_GUID,
> -                              pSMBr->u.extended_response.GUID, 16);
> -               }
> -
> -               if (count == 16) {
> -                       server->secType = RawNTLMSSP;
> -               } else {
> -                       rc = decode_negTokenInit(pSMBr->u.extended_response.
> -                                                SecurityBlob, count - 16,
> -                                                server);
> -                       if (rc == 1)
> -                               rc = 0;
> -                       else
> -                               rc = -EINVAL;
> -                       if (server->secType == Kerberos) {
> -                               if (!server->sec_kerberos &&
> -                                               !server->sec_mskerberos)
> -                                       rc = -EOPNOTSUPP;
> -                       } else if (server->secType == RawNTLMSSP) {
> -                               if (!server->sec_ntlmssp)
> -                                       rc = -EOPNOTSUPP;
> -                       } else
> -                                       rc = -EOPNOTSUPP;
> -               }
> -       } else if (server->sec_mode & SECMODE_PW_ENCRYPT) {
> +                               (pSMBr->EncryptionKeyLength == 0))
> +               rc = decode_ext_sec_blob(server, pSMBr);
> +       else if (server->sec_mode & SECMODE_PW_ENCRYPT)
>                 rc = -EIO; /* no crypt key only if plain text pwd */
> -               goto neg_err_exit;
> -       } else
> +       else
>                 server->capabilities &= ~CAP_EXTENDED_SECURITY;
>
> +       if (rc)
> +               goto neg_err_exit;
> +
>  #ifdef CONFIG_CIFS_WEAK_PW_HASH
>  signing_check:
>  #endif
> --
> 1.8.1.4
>
> --
> 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

Acked-by: Pavel Shilovsky <piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org>

--
Best regards,
Pavel Shilovsky.

[PATCH 08/19] cifs: break out lanman NEGOTIATE handling into separate function

[Jeff Layton]

...this also gets rid of some #ifdef ugliness too.

Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 fs/cifs/cifssmb.c | 185 ++++++++++++++++++++++++++++--------------------------
 1 file changed, 97 insertions(+), 88 deletions(-)

diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
index 9b4aea8..5dd4f8a 100644
--- a/fs/cifs/cifssmb.c
+++ b/fs/cifs/cifssmb.c
@@ -417,6 +417,96 @@ decode_ext_sec_blob(struct TCP_Server_Info *server, NEGOTIATE_RSP *pSMBr)
 	return 0;
 }
 
+#ifdef CONFIG_CIFS_WEAK_PW_HASH
+static int
+decode_lanman_negprot_rsp(struct TCP_Server_Info *server, NEGOTIATE_RSP *pSMBr,
+			  unsigned int secFlags)
+{
+	__s16 tmp;
+	struct lanman_neg_rsp *rsp = (struct lanman_neg_rsp *)pSMBr;
+
+	if (server->dialect != LANMAN_PROT && server->dialect != LANMAN2_PROT)
+		return -EOPNOTSUPP;
+
+	if ((secFlags & CIFSSEC_MAY_LANMAN) || (secFlags & CIFSSEC_MAY_PLNTXT))
+		server->secType = LANMAN;
+	else {
+		cifs_dbg(VFS, "mount failed weak security disabled in /proc/fs/cifs/SecurityFlags\n");
+		return -EOPNOTSUPP;
+	}
+	server->sec_mode = le16_to_cpu(rsp->SecurityMode);
+	server->maxReq = min_t(unsigned int,
+			       le16_to_cpu(rsp->MaxMpxCount),
+			       cifs_max_pending);
+	set_credits(server, server->maxReq);
+	server->maxBuf = le16_to_cpu(rsp->MaxBufSize);
+	server->max_vcs = le16_to_cpu(rsp->MaxNumberVcs);
+	/* even though we do not use raw we might as well set this
+	accurately, in case we ever find a need for it */
+	if ((le16_to_cpu(rsp->RawMode) & RAW_ENABLE) == RAW_ENABLE) {
+		server->max_rw = 0xFF00;
+		server->capabilities = CAP_MPX_MODE | CAP_RAW_MODE;
+	} else {
+		server->max_rw = 0;/* do not need to use raw anyway */
+		server->capabilities = CAP_MPX_MODE;
+	}
+	tmp = (__s16)le16_to_cpu(rsp->ServerTimeZone);
+	if (tmp == -1) {
+		/* OS/2 often does not set timezone therefore
+		 * we must use server time to calc time zone.
+		 * Could deviate slightly from the right zone.
+		 * Smallest defined timezone difference is 15 minutes
+		 * (i.e. Nepal).  Rounding up/down is done to match
+		 * this requirement.
+		 */
+		int val, seconds, remain, result;
+		struct timespec ts, utc;
+		utc = CURRENT_TIME;
+		ts = cnvrtDosUnixTm(rsp->SrvTime.Date,
+				    rsp->SrvTime.Time, 0);
+		cifs_dbg(FYI, "SrvTime %d sec since 1970 (utc: %d) diff: %d\n",
+			 (int)ts.tv_sec, (int)utc.tv_sec,
+			 (int)(utc.tv_sec - ts.tv_sec));
+		val = (int)(utc.tv_sec - ts.tv_sec);
+		seconds = abs(val);
+		result = (seconds / MIN_TZ_ADJ) * MIN_TZ_ADJ;
+		remain = seconds % MIN_TZ_ADJ;
+		if (remain >= (MIN_TZ_ADJ / 2))
+			result += MIN_TZ_ADJ;
+		if (val < 0)
+			result = -result;
+		server->timeAdj = result;
+	} else {
+		server->timeAdj = (int)tmp;
+		server->timeAdj *= 60; /* also in seconds */
+	}
+	cifs_dbg(FYI, "server->timeAdj: %d seconds\n", server->timeAdj);
+
+
+	/* BB get server time for time conversions and add
+	code to use it and timezone since this is not UTC */
+
+	if (rsp->EncryptionKeyLength ==
+			cpu_to_le16(CIFS_CRYPTO_KEY_SIZE)) {
+		memcpy(server->cryptkey, rsp->EncryptionKey,
+			CIFS_CRYPTO_KEY_SIZE);
+	} else if (server->sec_mode & SECMODE_PW_ENCRYPT) {
+		return -EIO; /* need cryptkey unless plain text */
+	}
+
+	cifs_dbg(FYI, "LANMAN negotiated\n");
+	return 0;
+}
+#else
+static inline int
+decode_lanman_negprot_rsp(struct TCP_Server_Info *server, NEGOTIATE_RSP *pSMBr,
+			  unsigned int secFlags)
+{
+	cifs_dbg(VFS, "mount failed, cifs module not built with CIFS_WEAK_PW_HASH support\n");
+	return -EOPNOTSUPP;
+}
+#endif
+
 int
 CIFSSMBNegotiate(const unsigned int xid, struct cifs_ses *ses)
 {
@@ -485,98 +575,19 @@ CIFSSMBNegotiate(const unsigned int xid, struct cifs_ses *ses)
 		could not negotiate a common dialect */
 		rc = -EOPNOTSUPP;
 		goto neg_err_exit;
-#ifdef CONFIG_CIFS_WEAK_PW_HASH
-	} else if ((pSMBr->hdr.WordCount == 13)
-			&& ((server->dialect == LANMAN_PROT)
-				|| (server->dialect == LANMAN2_PROT))) {
-		__s16 tmp;
-		struct lanman_neg_rsp *rsp = (struct lanman_neg_rsp *)pSMBr;
-
-		if ((secFlags & CIFSSEC_MAY_LANMAN) ||
-			(secFlags & CIFSSEC_MAY_PLNTXT))
-			server->secType = LANMAN;
-		else {
-			cifs_dbg(VFS, "mount failed weak security disabled in /proc/fs/cifs/SecurityFlags\n");
-			rc = -EOPNOTSUPP;
-			goto neg_err_exit;
-		}
-		server->sec_mode = le16_to_cpu(rsp->SecurityMode);
-		server->maxReq = min_t(unsigned int,
-				       le16_to_cpu(rsp->MaxMpxCount),
-				       cifs_max_pending);
-		set_credits(server, server->maxReq);
-		server->maxBuf = le16_to_cpu(rsp->MaxBufSize);
-		server->max_vcs = le16_to_cpu(rsp->MaxNumberVcs);
-		/* even though we do not use raw we might as well set this
-		accurately, in case we ever find a need for it */
-		if ((le16_to_cpu(rsp->RawMode) & RAW_ENABLE) == RAW_ENABLE) {
-			server->max_rw = 0xFF00;
-			server->capabilities = CAP_MPX_MODE | CAP_RAW_MODE;
-		} else {
-			server->max_rw = 0;/* do not need to use raw anyway */
-			server->capabilities = CAP_MPX_MODE;
-		}
-		tmp = (__s16)le16_to_cpu(rsp->ServerTimeZone);
-		if (tmp == -1) {
-			/* OS/2 often does not set timezone therefore
-			 * we must use server time to calc time zone.
-			 * Could deviate slightly from the right zone.
-			 * Smallest defined timezone difference is 15 minutes
-			 * (i.e. Nepal).  Rounding up/down is done to match
-			 * this requirement.
-			 */
-			int val, seconds, remain, result;
-			struct timespec ts, utc;
-			utc = CURRENT_TIME;
-			ts = cnvrtDosUnixTm(rsp->SrvTime.Date,
-					    rsp->SrvTime.Time, 0);
-			cifs_dbg(FYI, "SrvTime %d sec since 1970 (utc: %d) diff: %d\n",
-				 (int)ts.tv_sec, (int)utc.tv_sec,
-				 (int)(utc.tv_sec - ts.tv_sec));
-			val = (int)(utc.tv_sec - ts.tv_sec);
-			seconds = abs(val);
-			result = (seconds / MIN_TZ_ADJ) * MIN_TZ_ADJ;
-			remain = seconds % MIN_TZ_ADJ;
-			if (remain >= (MIN_TZ_ADJ / 2))
-				result += MIN_TZ_ADJ;
-			if (val < 0)
-				result = -result;
-			server->timeAdj = result;
-		} else {
-			server->timeAdj = (int)tmp;
-			server->timeAdj *= 60; /* also in seconds */
-		}
-		cifs_dbg(FYI, "server->timeAdj: %d seconds\n", server->timeAdj);
-
-
-		/* BB get server time for time conversions and add
-		code to use it and timezone since this is not UTC */
-
-		if (rsp->EncryptionKeyLength ==
-				cpu_to_le16(CIFS_CRYPTO_KEY_SIZE)) {
-			memcpy(ses->server->cryptkey, rsp->EncryptionKey,
-				CIFS_CRYPTO_KEY_SIZE);
-		} else if (server->sec_mode & SECMODE_PW_ENCRYPT) {
-			rc = -EIO; /* need cryptkey unless plain text */
-			goto neg_err_exit;
-		}
-
-		cifs_dbg(FYI, "LANMAN negotiated\n");
-		/* we will not end up setting signing flags - as no signing
-		was in LANMAN and server did not return the flags on */
-		goto signing_check;
-#else /* weak security disabled */
 	} else if (pSMBr->hdr.WordCount == 13) {
-		cifs_dbg(VFS, "mount failed, cifs module not built with CIFS_WEAK_PW_HASH support\n");
-		rc = -EOPNOTSUPP;
-#endif /* WEAK_PW_HASH */
-		goto neg_err_exit;
+		rc = decode_lanman_negprot_rsp(server, pSMBr, secFlags);
+		if (!rc)
+			goto signing_check;
+		else
+			goto neg_err_exit;
 	} else if (pSMBr->hdr.WordCount != 17) {
 		/* unknown wct */
 		rc = -EOPNOTSUPP;
 		goto neg_err_exit;
 	}
-	/* else wct == 17 NTLM */
+	/* else wct == 17, NTLM or better */
+
 	server->sec_mode = pSMBr->SecurityMode;
 	if ((server->sec_mode & SECMODE_USER) == 0)
 		cifs_dbg(FYI, "share mode security\n");
@@ -634,9 +645,7 @@ CIFSSMBNegotiate(const unsigned int xid, struct cifs_ses *ses)
 	if (rc)
 		goto neg_err_exit;
 
-#ifdef CONFIG_CIFS_WEAK_PW_HASH
 signing_check:
-#endif
 	if ((secFlags & CIFSSEC_MAY_SIGN) == 0) {
 		/* MUST_SIGN already includes the MAY_SIGN FLAG
 		   so if this is zero it means that signing is disabled */
-- 
1.8.1.4

Re: [PATCH 08/19] cifs: break out lanman NEGOTIATE handling into separate function

[Pavel Shilovsky]

2013/5/23 Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>:
> ...this also gets rid of some #ifdef ugliness too.
>
> Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
>  fs/cifs/cifssmb.c | 185 ++++++++++++++++++++++++++++--------------------------
>  1 file changed, 97 insertions(+), 88 deletions(-)
>
> diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
> index 9b4aea8..5dd4f8a 100644
> --- a/fs/cifs/cifssmb.c
> +++ b/fs/cifs/cifssmb.c
> @@ -417,6 +417,96 @@ decode_ext_sec_blob(struct TCP_Server_Info *server, NEGOTIATE_RSP *pSMBr)
>         return 0;
>  }
>
> +#ifdef CONFIG_CIFS_WEAK_PW_HASH
> +static int
> +decode_lanman_negprot_rsp(struct TCP_Server_Info *server, NEGOTIATE_RSP *pSMBr,
> +                         unsigned int secFlags)
> +{
> +       __s16 tmp;
> +       struct lanman_neg_rsp *rsp = (struct lanman_neg_rsp *)pSMBr;
> +
> +       if (server->dialect != LANMAN_PROT && server->dialect != LANMAN2_PROT)
> +               return -EOPNOTSUPP;
> +
> +       if ((secFlags & CIFSSEC_MAY_LANMAN) || (secFlags & CIFSSEC_MAY_PLNTXT))
> +               server->secType = LANMAN;
> +       else {
> +               cifs_dbg(VFS, "mount failed weak security disabled in /proc/fs/cifs/SecurityFlags\n");
> +               return -EOPNOTSUPP;
> +       }
> +       server->sec_mode = le16_to_cpu(rsp->SecurityMode);
> +       server->maxReq = min_t(unsigned int,
> +                              le16_to_cpu(rsp->MaxMpxCount),
> +                              cifs_max_pending);
> +       set_credits(server, server->maxReq);
> +       server->maxBuf = le16_to_cpu(rsp->MaxBufSize);
> +       server->max_vcs = le16_to_cpu(rsp->MaxNumberVcs);
> +       /* even though we do not use raw we might as well set this
> +       accurately, in case we ever find a need for it */
> +       if ((le16_to_cpu(rsp->RawMode) & RAW_ENABLE) == RAW_ENABLE) {
> +               server->max_rw = 0xFF00;
> +               server->capabilities = CAP_MPX_MODE | CAP_RAW_MODE;
> +       } else {
> +               server->max_rw = 0;/* do not need to use raw anyway */
> +               server->capabilities = CAP_MPX_MODE;
> +       }
> +       tmp = (__s16)le16_to_cpu(rsp->ServerTimeZone);
> +       if (tmp == -1) {
> +               /* OS/2 often does not set timezone therefore
> +                * we must use server time to calc time zone.
> +                * Could deviate slightly from the right zone.
> +                * Smallest defined timezone difference is 15 minutes
> +                * (i.e. Nepal).  Rounding up/down is done to match
> +                * this requirement.
> +                */
> +               int val, seconds, remain, result;
> +               struct timespec ts, utc;
> +               utc = CURRENT_TIME;
> +               ts = cnvrtDosUnixTm(rsp->SrvTime.Date,
> +                                   rsp->SrvTime.Time, 0);
> +               cifs_dbg(FYI, "SrvTime %d sec since 1970 (utc: %d) diff: %d\n",
> +                        (int)ts.tv_sec, (int)utc.tv_sec,
> +                        (int)(utc.tv_sec - ts.tv_sec));
> +               val = (int)(utc.tv_sec - ts.tv_sec);
> +               seconds = abs(val);
> +               result = (seconds / MIN_TZ_ADJ) * MIN_TZ_ADJ;
> +               remain = seconds % MIN_TZ_ADJ;
> +               if (remain >= (MIN_TZ_ADJ / 2))
> +                       result += MIN_TZ_ADJ;
> +               if (val < 0)
> +                       result = -result;
> +               server->timeAdj = result;
> +       } else {
> +               server->timeAdj = (int)tmp;
> +               server->timeAdj *= 60; /* also in seconds */
> +       }
> +       cifs_dbg(FYI, "server->timeAdj: %d seconds\n", server->timeAdj);
> +
> +
> +       /* BB get server time for time conversions and add
> +       code to use it and timezone since this is not UTC */
> +
> +       if (rsp->EncryptionKeyLength ==
> +                       cpu_to_le16(CIFS_CRYPTO_KEY_SIZE)) {
> +               memcpy(server->cryptkey, rsp->EncryptionKey,
> +                       CIFS_CRYPTO_KEY_SIZE);
> +       } else if (server->sec_mode & SECMODE_PW_ENCRYPT) {
> +               return -EIO; /* need cryptkey unless plain text */
> +       }
> +
> +       cifs_dbg(FYI, "LANMAN negotiated\n");
> +       return 0;
> +}
> +#else
> +static inline int
> +decode_lanman_negprot_rsp(struct TCP_Server_Info *server, NEGOTIATE_RSP *pSMBr,
> +                         unsigned int secFlags)
> +{
> +       cifs_dbg(VFS, "mount failed, cifs module not built with CIFS_WEAK_PW_HASH support\n");
> +       return -EOPNOTSUPP;
> +}
> +#endif
> +
>  int
>  CIFSSMBNegotiate(const unsigned int xid, struct cifs_ses *ses)
>  {
> @@ -485,98 +575,19 @@ CIFSSMBNegotiate(const unsigned int xid, struct cifs_ses *ses)
>                 could not negotiate a common dialect */
>                 rc = -EOPNOTSUPP;
>                 goto neg_err_exit;
> -#ifdef CONFIG_CIFS_WEAK_PW_HASH
> -       } else if ((pSMBr->hdr.WordCount == 13)
> -                       && ((server->dialect == LANMAN_PROT)
> -                               || (server->dialect == LANMAN2_PROT))) {
> -               __s16 tmp;
> -               struct lanman_neg_rsp *rsp = (struct lanman_neg_rsp *)pSMBr;
> -
> -               if ((secFlags & CIFSSEC_MAY_LANMAN) ||
> -                       (secFlags & CIFSSEC_MAY_PLNTXT))
> -                       server->secType = LANMAN;
> -               else {
> -                       cifs_dbg(VFS, "mount failed weak security disabled in /proc/fs/cifs/SecurityFlags\n");
> -                       rc = -EOPNOTSUPP;
> -                       goto neg_err_exit;
> -               }
> -               server->sec_mode = le16_to_cpu(rsp->SecurityMode);
> -               server->maxReq = min_t(unsigned int,
> -                                      le16_to_cpu(rsp->MaxMpxCount),
> -                                      cifs_max_pending);
> -               set_credits(server, server->maxReq);
> -               server->maxBuf = le16_to_cpu(rsp->MaxBufSize);
> -               server->max_vcs = le16_to_cpu(rsp->MaxNumberVcs);
> -               /* even though we do not use raw we might as well set this
> -               accurately, in case we ever find a need for it */
> -               if ((le16_to_cpu(rsp->RawMode) & RAW_ENABLE) == RAW_ENABLE) {
> -                       server->max_rw = 0xFF00;
> -                       server->capabilities = CAP_MPX_MODE | CAP_RAW_MODE;
> -               } else {
> -                       server->max_rw = 0;/* do not need to use raw anyway */
> -                       server->capabilities = CAP_MPX_MODE;
> -               }
> -               tmp = (__s16)le16_to_cpu(rsp->ServerTimeZone);
> -               if (tmp == -1) {
> -                       /* OS/2 often does not set timezone therefore
> -                        * we must use server time to calc time zone.
> -                        * Could deviate slightly from the right zone.
> -                        * Smallest defined timezone difference is 15 minutes
> -                        * (i.e. Nepal).  Rounding up/down is done to match
> -                        * this requirement.
> -                        */
> -                       int val, seconds, remain, result;
> -                       struct timespec ts, utc;
> -                       utc = CURRENT_TIME;
> -                       ts = cnvrtDosUnixTm(rsp->SrvTime.Date,
> -                                           rsp->SrvTime.Time, 0);
> -                       cifs_dbg(FYI, "SrvTime %d sec since 1970 (utc: %d) diff: %d\n",
> -                                (int)ts.tv_sec, (int)utc.tv_sec,
> -                                (int)(utc.tv_sec - ts.tv_sec));
> -                       val = (int)(utc.tv_sec - ts.tv_sec);
> -                       seconds = abs(val);
> -                       result = (seconds / MIN_TZ_ADJ) * MIN_TZ_ADJ;
> -                       remain = seconds % MIN_TZ_ADJ;
> -                       if (remain >= (MIN_TZ_ADJ / 2))
> -                               result += MIN_TZ_ADJ;
> -                       if (val < 0)
> -                               result = -result;
> -                       server->timeAdj = result;
> -               } else {
> -                       server->timeAdj = (int)tmp;
> -                       server->timeAdj *= 60; /* also in seconds */
> -               }
> -               cifs_dbg(FYI, "server->timeAdj: %d seconds\n", server->timeAdj);
> -
> -
> -               /* BB get server time for time conversions and add
> -               code to use it and timezone since this is not UTC */
> -
> -               if (rsp->EncryptionKeyLength ==
> -                               cpu_to_le16(CIFS_CRYPTO_KEY_SIZE)) {
> -                       memcpy(ses->server->cryptkey, rsp->EncryptionKey,
> -                               CIFS_CRYPTO_KEY_SIZE);
> -               } else if (server->sec_mode & SECMODE_PW_ENCRYPT) {
> -                       rc = -EIO; /* need cryptkey unless plain text */
> -                       goto neg_err_exit;
> -               }
> -
> -               cifs_dbg(FYI, "LANMAN negotiated\n");
> -               /* we will not end up setting signing flags - as no signing
> -               was in LANMAN and server did not return the flags on */
> -               goto signing_check;
> -#else /* weak security disabled */
>         } else if (pSMBr->hdr.WordCount == 13) {
> -               cifs_dbg(VFS, "mount failed, cifs module not built with CIFS_WEAK_PW_HASH support\n");
> -               rc = -EOPNOTSUPP;
> -#endif /* WEAK_PW_HASH */
> -               goto neg_err_exit;
> +               rc = decode_lanman_negprot_rsp(server, pSMBr, secFlags);
> +               if (!rc)
> +                       goto signing_check;
> +               else
> +                       goto neg_err_exit;
>         } else if (pSMBr->hdr.WordCount != 17) {
>                 /* unknown wct */
>                 rc = -EOPNOTSUPP;
>                 goto neg_err_exit;
>         }
> -       /* else wct == 17 NTLM */
> +       /* else wct == 17, NTLM or better */
> +
>         server->sec_mode = pSMBr->SecurityMode;
>         if ((server->sec_mode & SECMODE_USER) == 0)
>                 cifs_dbg(FYI, "share mode security\n");
> @@ -634,9 +645,7 @@ CIFSSMBNegotiate(const unsigned int xid, struct cifs_ses *ses)
>         if (rc)
>                 goto neg_err_exit;
>
> -#ifdef CONFIG_CIFS_WEAK_PW_HASH
>  signing_check:
> -#endif
>         if ((secFlags & CIFSSEC_MAY_SIGN) == 0) {
>                 /* MUST_SIGN already includes the MAY_SIGN FLAG
>                    so if this is zero it means that signing is disabled */
> --
> 1.8.1.4
>
> --
> 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

Good cleanup. Acked-by: Pavel Shilovsky <piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org>

--
Best regards,
Pavel Shilovsky.

[PATCH 09/19] cifs: move handling of signed connections into separate function

[Jeff Layton]

Move the sanity checks for signed connections into a separate function.
SMB2's was a cut-and-paste job from CIFS code, so we can make them use
the same function.

Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 fs/cifs/cifsproto.h |  1 +
 fs/cifs/cifssmb.c   | 71 +++++++++++++++++++++++++++--------------------------
 fs/cifs/smb2pdu.c   | 33 +++----------------------
 3 files changed, 41 insertions(+), 64 deletions(-)

diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
index dda188a..f0e93ff 100644
--- a/fs/cifs/cifsproto.h
+++ b/fs/cifs/cifsproto.h
@@ -212,6 +212,7 @@ extern int cifs_negotiate_protocol(const unsigned int xid,
 				   struct cifs_ses *ses);
 extern int cifs_setup_session(const unsigned int xid, struct cifs_ses *ses,
 			      struct nls_table *nls_info);
+extern int cifs_enable_signing(struct TCP_Server_Info *server, unsigned int secFlags);
 extern int CIFSSMBNegotiate(const unsigned int xid, struct cifs_ses *ses);
 
 extern int CIFSTCon(const unsigned int xid, struct cifs_ses *ses,
diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
index 5dd4f8a..5b191f7 100644
--- a/fs/cifs/cifssmb.c
+++ b/fs/cifs/cifssmb.c
@@ -417,6 +417,38 @@ decode_ext_sec_blob(struct TCP_Server_Info *server, NEGOTIATE_RSP *pSMBr)
 	return 0;
 }
 
+int
+cifs_enable_signing(struct TCP_Server_Info *server, unsigned int secFlags)
+{
+	if ((secFlags & CIFSSEC_MAY_SIGN) == 0) {
+		/* MUST_SIGN already includes the MAY_SIGN FLAG
+		   so if this is zero it means that signing is disabled */
+		cifs_dbg(FYI, "Signing disabled\n");
+		if (server->sec_mode & SECMODE_SIGN_REQUIRED) {
+			cifs_dbg(VFS, "Server requires packet signing to be enabled in /proc/fs/cifs/SecurityFlags\n");
+			return -EOPNOTSUPP;
+		}
+		server->sec_mode &=
+			~(SECMODE_SIGN_ENABLED | SECMODE_SIGN_REQUIRED);
+	} else if ((secFlags & CIFSSEC_MUST_SIGN) == CIFSSEC_MUST_SIGN) {
+		/* signing required */
+		cifs_dbg(FYI, "Must sign - secFlags 0x%x\n", secFlags);
+		if ((server->sec_mode &
+			(SECMODE_SIGN_ENABLED | SECMODE_SIGN_REQUIRED)) == 0) {
+			cifs_dbg(VFS, "signing required but server lacks support\n");
+			return -EOPNOTSUPP;
+		} else
+			server->sec_mode |= SECMODE_SIGN_REQUIRED;
+	} else {
+		/* signing optional ie CIFSSEC_MAY_SIGN */
+		if ((server->sec_mode & SECMODE_SIGN_REQUIRED) == 0)
+			server->sec_mode &=
+				~(SECMODE_SIGN_ENABLED | SECMODE_SIGN_REQUIRED);
+	}
+
+	return 0;
+}
+
 #ifdef CONFIG_CIFS_WEAK_PW_HASH
 static int
 decode_lanman_negprot_rsp(struct TCP_Server_Info *server, NEGOTIATE_RSP *pSMBr,
@@ -495,7 +527,7 @@ decode_lanman_negprot_rsp(struct TCP_Server_Info *server, NEGOTIATE_RSP *pSMBr,
 	}
 
 	cifs_dbg(FYI, "LANMAN negotiated\n");
-	return 0;
+	return cifs_enable_signing(server, secFlags);
 }
 #else
 static inline int
@@ -577,10 +609,7 @@ CIFSSMBNegotiate(const unsigned int xid, struct cifs_ses *ses)
 		goto neg_err_exit;
 	} else if (pSMBr->hdr.WordCount == 13) {
 		rc = decode_lanman_negprot_rsp(server, pSMBr, secFlags);
-		if (!rc)
-			goto signing_check;
-		else
-			goto neg_err_exit;
+		goto neg_err_exit;
 	} else if (pSMBr->hdr.WordCount != 17) {
 		/* unknown wct */
 		rc = -EOPNOTSUPP;
@@ -642,36 +671,8 @@ CIFSSMBNegotiate(const unsigned int xid, struct cifs_ses *ses)
 	else
 		server->capabilities &= ~CAP_EXTENDED_SECURITY;
 
-	if (rc)
-		goto neg_err_exit;
-
-signing_check:
-	if ((secFlags & CIFSSEC_MAY_SIGN) == 0) {
-		/* MUST_SIGN already includes the MAY_SIGN FLAG
-		   so if this is zero it means that signing is disabled */
-		cifs_dbg(FYI, "Signing disabled\n");
-		if (server->sec_mode & SECMODE_SIGN_REQUIRED) {
-			cifs_dbg(VFS, "Server requires packet signing to be enabled in /proc/fs/cifs/SecurityFlags\n");
-			rc = -EOPNOTSUPP;
-		}
-		server->sec_mode &=
-			~(SECMODE_SIGN_ENABLED | SECMODE_SIGN_REQUIRED);
-	} else if ((secFlags & CIFSSEC_MUST_SIGN) == CIFSSEC_MUST_SIGN) {
-		/* signing required */
-		cifs_dbg(FYI, "Must sign - secFlags 0x%x\n", secFlags);
-		if ((server->sec_mode &
-			(SECMODE_SIGN_ENABLED | SECMODE_SIGN_REQUIRED)) == 0) {
-			cifs_dbg(VFS, "signing required but server lacks support\n");
-			rc = -EOPNOTSUPP;
-		} else
-			server->sec_mode |= SECMODE_SIGN_REQUIRED;
-	} else {
-		/* signing optional ie CIFSSEC_MAY_SIGN */
-		if ((server->sec_mode & SECMODE_SIGN_REQUIRED) == 0)
-			server->sec_mode &=
-				~(SECMODE_SIGN_ENABLED | SECMODE_SIGN_REQUIRED);
-	}
-
+	if (!rc)
+		rc = cifs_enable_signing(server, secFlags);
 neg_err_exit:
 	cifs_buf_release(pSMB);
 
diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
index 3af66aa..ebb97b4 100644
--- a/fs/cifs/smb2pdu.c
+++ b/fs/cifs/smb2pdu.c
@@ -423,36 +423,11 @@ SMB2_negotiate(const unsigned int xid, struct cifs_ses *ses)
 	}
 
 	cifs_dbg(FYI, "sec_flags 0x%x\n", sec_flags);
-	if ((sec_flags & CIFSSEC_MUST_SIGN) == CIFSSEC_MUST_SIGN) {
-		cifs_dbg(FYI, "Signing required\n");
-		if (!(server->sec_mode & (SMB2_NEGOTIATE_SIGNING_REQUIRED |
-		      SMB2_NEGOTIATE_SIGNING_ENABLED))) {
-			cifs_dbg(VFS, "signing required but server lacks support\n");
-			rc = -EOPNOTSUPP;
-			goto neg_exit;
-		}
-		server->sec_mode |= SECMODE_SIGN_REQUIRED;
-	} else if (sec_flags & CIFSSEC_MAY_SIGN) {
-		cifs_dbg(FYI, "Signing optional\n");
-		if (server->sec_mode & SMB2_NEGOTIATE_SIGNING_REQUIRED) {
-			cifs_dbg(FYI, "Server requires signing\n");
-			server->sec_mode |= SECMODE_SIGN_REQUIRED;
-		} else {
-			server->sec_mode &=
-				~(SECMODE_SIGN_ENABLED | SECMODE_SIGN_REQUIRED);
-		}
-	} else {
-		cifs_dbg(FYI, "Signing disabled\n");
-		if (server->sec_mode & SMB2_NEGOTIATE_SIGNING_REQUIRED) {
-			cifs_dbg(VFS, "Server requires packet signing to be enabled in /proc/fs/cifs/SecurityFlags\n");
-			rc = -EOPNOTSUPP;
-			goto neg_exit;
-		}
-		server->sec_mode &=
-			~(SECMODE_SIGN_ENABLED | SECMODE_SIGN_REQUIRED);
-	}
-
+	rc = cifs_enable_signing(server, sec_flags);
 #ifdef CONFIG_SMB2_ASN1  /* BB REMOVEME when updated asn1.c ready */
+	if (rc)
+		goto neg_exit;
+
 	rc = decode_neg_token_init(security_blob, blob_length,
 				   &server->sec_type);
 	if (rc == 1)
-- 
1.8.1.4

Re: [PATCH 09/19] cifs: move handling of signed connections into separate function

[Pavel Shilovsky]

2013/5/23 Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>:
> Move the sanity checks for signed connections into a separate function.
> SMB2's was a cut-and-paste job from CIFS code, so we can make them use
> the same function.
>
> Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
>  fs/cifs/cifsproto.h |  1 +
>  fs/cifs/cifssmb.c   | 71 +++++++++++++++++++++++++++--------------------------
>  fs/cifs/smb2pdu.c   | 33 +++----------------------
>  3 files changed, 41 insertions(+), 64 deletions(-)
>
> diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
> index dda188a..f0e93ff 100644
> --- a/fs/cifs/cifsproto.h
> +++ b/fs/cifs/cifsproto.h
> @@ -212,6 +212,7 @@ extern int cifs_negotiate_protocol(const unsigned int xid,
>                                    struct cifs_ses *ses);
>  extern int cifs_setup_session(const unsigned int xid, struct cifs_ses *ses,
>                               struct nls_table *nls_info);
> +extern int cifs_enable_signing(struct TCP_Server_Info *server, unsigned int secFlags);
>  extern int CIFSSMBNegotiate(const unsigned int xid, struct cifs_ses *ses);
>
>  extern int CIFSTCon(const unsigned int xid, struct cifs_ses *ses,
> diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
> index 5dd4f8a..5b191f7 100644
> --- a/fs/cifs/cifssmb.c
> +++ b/fs/cifs/cifssmb.c
> @@ -417,6 +417,38 @@ decode_ext_sec_blob(struct TCP_Server_Info *server, NEGOTIATE_RSP *pSMBr)
>         return 0;
>  }
>
> +int
> +cifs_enable_signing(struct TCP_Server_Info *server, unsigned int secFlags)
> +{
> +       if ((secFlags & CIFSSEC_MAY_SIGN) == 0) {
> +               /* MUST_SIGN already includes the MAY_SIGN FLAG
> +                  so if this is zero it means that signing is disabled */
> +               cifs_dbg(FYI, "Signing disabled\n");
> +               if (server->sec_mode & SECMODE_SIGN_REQUIRED) {
> +                       cifs_dbg(VFS, "Server requires packet signing to be enabled in /proc/fs/cifs/SecurityFlags\n");
> +                       return -EOPNOTSUPP;
> +               }
> +               server->sec_mode &=
> +                       ~(SECMODE_SIGN_ENABLED | SECMODE_SIGN_REQUIRED);
> +       } else if ((secFlags & CIFSSEC_MUST_SIGN) == CIFSSEC_MUST_SIGN) {
> +               /* signing required */
> +               cifs_dbg(FYI, "Must sign - secFlags 0x%x\n", secFlags);
> +               if ((server->sec_mode &
> +                       (SECMODE_SIGN_ENABLED | SECMODE_SIGN_REQUIRED)) == 0) {
> +                       cifs_dbg(VFS, "signing required but server lacks support\n");
> +                       return -EOPNOTSUPP;
> +               } else
> +                       server->sec_mode |= SECMODE_SIGN_REQUIRED;
> +       } else {
> +               /* signing optional ie CIFSSEC_MAY_SIGN */
> +               if ((server->sec_mode & SECMODE_SIGN_REQUIRED) == 0)
> +                       server->sec_mode &=
> +                               ~(SECMODE_SIGN_ENABLED | SECMODE_SIGN_REQUIRED);
> +       }
> +
> +       return 0;
> +}
> +
>  #ifdef CONFIG_CIFS_WEAK_PW_HASH
>  static int
>  decode_lanman_negprot_rsp(struct TCP_Server_Info *server, NEGOTIATE_RSP *pSMBr,
> @@ -495,7 +527,7 @@ decode_lanman_negprot_rsp(struct TCP_Server_Info *server, NEGOTIATE_RSP *pSMBr,
>         }
>
>         cifs_dbg(FYI, "LANMAN negotiated\n");
> -       return 0;
> +       return cifs_enable_signing(server, secFlags);
>  }
>  #else
>  static inline int
> @@ -577,10 +609,7 @@ CIFSSMBNegotiate(const unsigned int xid, struct cifs_ses *ses)
>                 goto neg_err_exit;
>         } else if (pSMBr->hdr.WordCount == 13) {
>                 rc = decode_lanman_negprot_rsp(server, pSMBr, secFlags);
> -               if (!rc)
> -                       goto signing_check;
> -               else
> -                       goto neg_err_exit;
> +               goto neg_err_exit;

Go to a label that has "err" in it's name after a successful function
call may confuse people.

>         } else if (pSMBr->hdr.WordCount != 17) {
>                 /* unknown wct */
>                 rc = -EOPNOTSUPP;
> @@ -642,36 +671,8 @@ CIFSSMBNegotiate(const unsigned int xid, struct cifs_ses *ses)
>         else
>                 server->capabilities &= ~CAP_EXTENDED_SECURITY;
>
> -       if (rc)
> -               goto neg_err_exit;
> -
> -signing_check:
> -       if ((secFlags & CIFSSEC_MAY_SIGN) == 0) {
> -               /* MUST_SIGN already includes the MAY_SIGN FLAG
> -                  so if this is zero it means that signing is disabled */
> -               cifs_dbg(FYI, "Signing disabled\n");
> -               if (server->sec_mode & SECMODE_SIGN_REQUIRED) {
> -                       cifs_dbg(VFS, "Server requires packet signing to be enabled in /proc/fs/cifs/SecurityFlags\n");
> -                       rc = -EOPNOTSUPP;
> -               }
> -               server->sec_mode &=
> -                       ~(SECMODE_SIGN_ENABLED | SECMODE_SIGN_REQUIRED);
> -       } else if ((secFlags & CIFSSEC_MUST_SIGN) == CIFSSEC_MUST_SIGN) {
> -               /* signing required */
> -               cifs_dbg(FYI, "Must sign - secFlags 0x%x\n", secFlags);
> -               if ((server->sec_mode &
> -                       (SECMODE_SIGN_ENABLED | SECMODE_SIGN_REQUIRED)) == 0) {
> -                       cifs_dbg(VFS, "signing required but server lacks support\n");
> -                       rc = -EOPNOTSUPP;
> -               } else
> -                       server->sec_mode |= SECMODE_SIGN_REQUIRED;
> -       } else {
> -               /* signing optional ie CIFSSEC_MAY_SIGN */
> -               if ((server->sec_mode & SECMODE_SIGN_REQUIRED) == 0)
> -                       server->sec_mode &=
> -                               ~(SECMODE_SIGN_ENABLED | SECMODE_SIGN_REQUIRED);
> -       }
> -
> +       if (!rc)
> +               rc = cifs_enable_signing(server, secFlags);
>  neg_err_exit:
>         cifs_buf_release(pSMB);
>
> diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
> index 3af66aa..ebb97b4 100644
> --- a/fs/cifs/smb2pdu.c
> +++ b/fs/cifs/smb2pdu.c
> @@ -423,36 +423,11 @@ SMB2_negotiate(const unsigned int xid, struct cifs_ses *ses)
>         }
>
>         cifs_dbg(FYI, "sec_flags 0x%x\n", sec_flags);
> -       if ((sec_flags & CIFSSEC_MUST_SIGN) == CIFSSEC_MUST_SIGN) {
> -               cifs_dbg(FYI, "Signing required\n");
> -               if (!(server->sec_mode & (SMB2_NEGOTIATE_SIGNING_REQUIRED |
> -                     SMB2_NEGOTIATE_SIGNING_ENABLED))) {
> -                       cifs_dbg(VFS, "signing required but server lacks support\n");
> -                       rc = -EOPNOTSUPP;
> -                       goto neg_exit;
> -               }
> -               server->sec_mode |= SECMODE_SIGN_REQUIRED;
> -       } else if (sec_flags & CIFSSEC_MAY_SIGN) {
> -               cifs_dbg(FYI, "Signing optional\n");
> -               if (server->sec_mode & SMB2_NEGOTIATE_SIGNING_REQUIRED) {
> -                       cifs_dbg(FYI, "Server requires signing\n");
> -                       server->sec_mode |= SECMODE_SIGN_REQUIRED;
> -               } else {
> -                       server->sec_mode &=
> -                               ~(SECMODE_SIGN_ENABLED | SECMODE_SIGN_REQUIRED);
> -               }
> -       } else {
> -               cifs_dbg(FYI, "Signing disabled\n");
> -               if (server->sec_mode & SMB2_NEGOTIATE_SIGNING_REQUIRED) {
> -                       cifs_dbg(VFS, "Server requires packet signing to be enabled in /proc/fs/cifs/SecurityFlags\n");
> -                       rc = -EOPNOTSUPP;
> -                       goto neg_exit;
> -               }
> -               server->sec_mode &=
> -                       ~(SECMODE_SIGN_ENABLED | SECMODE_SIGN_REQUIRED);
> -       }
> -
> +       rc = cifs_enable_signing(server, sec_flags);
>  #ifdef CONFIG_SMB2_ASN1  /* BB REMOVEME when updated asn1.c ready */
> +       if (rc)
> +               goto neg_exit;
> +
>         rc = decode_neg_token_init(security_blob, blob_length,
>                                    &server->sec_type);
>         if (rc == 1)
> --
> 1.8.1.4
>
> --
> 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

Other than the note above, the patch looks ok to me.

Reviewed-by: Pavel Shilovsky <piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org>

--
Best regards,
Pavel Shilovsky.

Re: [PATCH 09/19] cifs: move handling of signed connections into separate function

[Jeff Layton]

On Fri, 24 May 2013 16:41:37 +0400
Pavel Shilovsky <piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:

> 2013/5/23 Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>:
> > Move the sanity checks for signed connections into a separate function.
> > SMB2's was a cut-and-paste job from CIFS code, so we can make them use
> > the same function.
> >
> > Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> > ---
> >  fs/cifs/cifsproto.h |  1 +
> >  fs/cifs/cifssmb.c   | 71 +++++++++++++++++++++++++++--------------------------
> >  fs/cifs/smb2pdu.c   | 33 +++----------------------
> >  3 files changed, 41 insertions(+), 64 deletions(-)
> >
> > diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
> > index dda188a..f0e93ff 100644
> > --- a/fs/cifs/cifsproto.h
> > +++ b/fs/cifs/cifsproto.h
> > @@ -212,6 +212,7 @@ extern int cifs_negotiate_protocol(const unsigned int xid,
> >                                    struct cifs_ses *ses);
> >  extern int cifs_setup_session(const unsigned int xid, struct cifs_ses *ses,
> >                               struct nls_table *nls_info);
> > +extern int cifs_enable_signing(struct TCP_Server_Info *server, unsigned int secFlags);
> >  extern int CIFSSMBNegotiate(const unsigned int xid, struct cifs_ses *ses);
> >
> >  extern int CIFSTCon(const unsigned int xid, struct cifs_ses *ses,
> > diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
> > index 5dd4f8a..5b191f7 100644
> > --- a/fs/cifs/cifssmb.c
> > +++ b/fs/cifs/cifssmb.c
> > @@ -417,6 +417,38 @@ decode_ext_sec_blob(struct TCP_Server_Info *server, NEGOTIATE_RSP *pSMBr)
> >         return 0;
> >  }
> >
> > +int
> > +cifs_enable_signing(struct TCP_Server_Info *server, unsigned int secFlags)
> > +{
> > +       if ((secFlags & CIFSSEC_MAY_SIGN) == 0) {
> > +               /* MUST_SIGN already includes the MAY_SIGN FLAG
> > +                  so if this is zero it means that signing is disabled */
> > +               cifs_dbg(FYI, "Signing disabled\n");
> > +               if (server->sec_mode & SECMODE_SIGN_REQUIRED) {
> > +                       cifs_dbg(VFS, "Server requires packet signing to be enabled in /proc/fs/cifs/SecurityFlags\n");
> > +                       return -EOPNOTSUPP;
> > +               }
> > +               server->sec_mode &=
> > +                       ~(SECMODE_SIGN_ENABLED | SECMODE_SIGN_REQUIRED);
> > +       } else if ((secFlags & CIFSSEC_MUST_SIGN) == CIFSSEC_MUST_SIGN) {
> > +               /* signing required */
> > +               cifs_dbg(FYI, "Must sign - secFlags 0x%x\n", secFlags);
> > +               if ((server->sec_mode &
> > +                       (SECMODE_SIGN_ENABLED | SECMODE_SIGN_REQUIRED)) == 0) {
> > +                       cifs_dbg(VFS, "signing required but server lacks support\n");
> > +                       return -EOPNOTSUPP;
> > +               } else
> > +                       server->sec_mode |= SECMODE_SIGN_REQUIRED;
> > +       } else {
> > +               /* signing optional ie CIFSSEC_MAY_SIGN */
> > +               if ((server->sec_mode & SECMODE_SIGN_REQUIRED) == 0)
> > +                       server->sec_mode &=
> > +                               ~(SECMODE_SIGN_ENABLED | SECMODE_SIGN_REQUIRED);
> > +       }
> > +
> > +       return 0;
> > +}
> > +
> >  #ifdef CONFIG_CIFS_WEAK_PW_HASH
> >  static int
> >  decode_lanman_negprot_rsp(struct TCP_Server_Info *server, NEGOTIATE_RSP *pSMBr,
> > @@ -495,7 +527,7 @@ decode_lanman_negprot_rsp(struct TCP_Server_Info *server, NEGOTIATE_RSP *pSMBr,
> >         }
> >
> >         cifs_dbg(FYI, "LANMAN negotiated\n");
> > -       return 0;
> > +       return cifs_enable_signing(server, secFlags);
> >  }
> >  #else
> >  static inline int
> > @@ -577,10 +609,7 @@ CIFSSMBNegotiate(const unsigned int xid, struct cifs_ses *ses)
> >                 goto neg_err_exit;
> >         } else if (pSMBr->hdr.WordCount == 13) {
> >                 rc = decode_lanman_negprot_rsp(server, pSMBr, secFlags);
> > -               if (!rc)
> > -                       goto signing_check;
> > -               else
> > -                       goto neg_err_exit;
> > +               goto neg_err_exit;
> 
> Go to a label that has "err" in it's name after a successful function
> call may confuse people.
> 

I guess we could change it to "neg_exit:" or something. I can toss a
patch onto the top of the pile to do that in the next iteration.

-- 
Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

[PATCH 10/19] cifs: factor out check for extended security bit into separate function

[Jeff Layton]

Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 fs/cifs/cifssmb.c | 25 ++++++++++++++++---------
 1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
index 5b191f7..e7184b9 100644
--- a/fs/cifs/cifssmb.c
+++ b/fs/cifs/cifssmb.c
@@ -539,6 +539,20 @@ decode_lanman_negprot_rsp(struct TCP_Server_Info *server, NEGOTIATE_RSP *pSMBr,
 }
 #endif
 
+static bool
+should_set_ext_sec_flag(unsigned int secFlags)
+{
+	if ((secFlags & CIFSSEC_MUST_KRB5) == CIFSSEC_MUST_KRB5)
+		return true;
+	else if ((secFlags & CIFSSEC_AUTH_MASK) == CIFSSEC_MAY_KRB5)
+		return true;
+	else if ((secFlags & CIFSSEC_MUST_NTLMSSP) == CIFSSEC_MUST_NTLMSSP)
+		return true;
+	else if ((secFlags & CIFSSEC_AUTH_MASK) == CIFSSEC_MAY_NTLMSSP)
+		return true;
+	return false;
+}
+
 int
 CIFSSMBNegotiate(const unsigned int xid, struct cifs_ses *ses)
 {
@@ -572,15 +586,8 @@ CIFSSMBNegotiate(const unsigned int xid, struct cifs_ses *ses)
 	pSMB->hdr.Mid = get_next_mid(server);
 	pSMB->hdr.Flags2 |= (SMBFLG2_UNICODE | SMBFLG2_ERR_STATUS);
 
-	if ((secFlags & CIFSSEC_MUST_KRB5) == CIFSSEC_MUST_KRB5)
-		pSMB->hdr.Flags2 |= SMBFLG2_EXT_SEC;
-	else if ((secFlags & CIFSSEC_AUTH_MASK) == CIFSSEC_MAY_KRB5) {
-		cifs_dbg(FYI, "Kerberos only mechanism, enable extended security\n");
-		pSMB->hdr.Flags2 |= SMBFLG2_EXT_SEC;
-	} else if ((secFlags & CIFSSEC_MUST_NTLMSSP) == CIFSSEC_MUST_NTLMSSP)
-		pSMB->hdr.Flags2 |= SMBFLG2_EXT_SEC;
-	else if ((secFlags & CIFSSEC_AUTH_MASK) == CIFSSEC_MAY_NTLMSSP) {
-		cifs_dbg(FYI, "NTLMSSP only mechanism, enable extended security\n");
+	if (should_set_ext_sec_flag(secFlags)) {
+		cifs_dbg(FYI, "Requesting extended security.");
 		pSMB->hdr.Flags2 |= SMBFLG2_EXT_SEC;
 	}
 
-- 
1.8.1.4

Re: [PATCH 10/19] cifs: factor out check for extended security bit into separate function

[Pavel Shilovsky]

2013/5/23 Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>:
> Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
>  fs/cifs/cifssmb.c | 25 ++++++++++++++++---------
>  1 file changed, 16 insertions(+), 9 deletions(-)
>
> diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
> index 5b191f7..e7184b9 100644
> --- a/fs/cifs/cifssmb.c
> +++ b/fs/cifs/cifssmb.c
> @@ -539,6 +539,20 @@ decode_lanman_negprot_rsp(struct TCP_Server_Info *server, NEGOTIATE_RSP *pSMBr,
>  }
>  #endif
>
> +static bool
> +should_set_ext_sec_flag(unsigned int secFlags)
> +{
> +       if ((secFlags & CIFSSEC_MUST_KRB5) == CIFSSEC_MUST_KRB5)
> +               return true;
> +       else if ((secFlags & CIFSSEC_AUTH_MASK) == CIFSSEC_MAY_KRB5)
> +               return true;
> +       else if ((secFlags & CIFSSEC_MUST_NTLMSSP) == CIFSSEC_MUST_NTLMSSP)
> +               return true;
> +       else if ((secFlags & CIFSSEC_AUTH_MASK) == CIFSSEC_MAY_NTLMSSP)
> +               return true;
> +       return false;
> +}
> +
>  int
>  CIFSSMBNegotiate(const unsigned int xid, struct cifs_ses *ses)
>  {
> @@ -572,15 +586,8 @@ CIFSSMBNegotiate(const unsigned int xid, struct cifs_ses *ses)
>         pSMB->hdr.Mid = get_next_mid(server);
>         pSMB->hdr.Flags2 |= (SMBFLG2_UNICODE | SMBFLG2_ERR_STATUS);
>
> -       if ((secFlags & CIFSSEC_MUST_KRB5) == CIFSSEC_MUST_KRB5)
> -               pSMB->hdr.Flags2 |= SMBFLG2_EXT_SEC;
> -       else if ((secFlags & CIFSSEC_AUTH_MASK) == CIFSSEC_MAY_KRB5) {
> -               cifs_dbg(FYI, "Kerberos only mechanism, enable extended security\n");
> -               pSMB->hdr.Flags2 |= SMBFLG2_EXT_SEC;
> -       } else if ((secFlags & CIFSSEC_MUST_NTLMSSP) == CIFSSEC_MUST_NTLMSSP)
> -               pSMB->hdr.Flags2 |= SMBFLG2_EXT_SEC;
> -       else if ((secFlags & CIFSSEC_AUTH_MASK) == CIFSSEC_MAY_NTLMSSP) {
> -               cifs_dbg(FYI, "NTLMSSP only mechanism, enable extended security\n");
> +       if (should_set_ext_sec_flag(secFlags)) {
> +               cifs_dbg(FYI, "Requesting extended security.");
>                 pSMB->hdr.Flags2 |= SMBFLG2_EXT_SEC;
>         }
>
> --
> 1.8.1.4
>
> --
> 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

Reviewed-by: Pavel Shilovsky <piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org>

--
Best regards,
Pavel Shilovsky.

[PATCH 11/19] cifs: add new "Unspecified" securityEnum value

[Jeff Layton]

Add a new securityEnum value to cover the case where a sec= option
was not explicitly set.

Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 fs/cifs/cifsglob.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index 874b29b..a858037 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -101,11 +101,11 @@ enum statusEnum {
 };
 
 enum securityEnum {
-	LANMAN = 0,			/* Legacy LANMAN auth */
+	Unspecified = 0,	/* not specified */
+	LANMAN,			/* Legacy LANMAN auth */
 	NTLM,			/* Legacy NTLM012 auth with NTLM hash */
 	NTLMv2,			/* Legacy NTLM auth with NTLMv2 hash */
 	RawNTLMSSP,		/* NTLMSSP without SPNEGO, NTLMv2 hash */
-/*	NTLMSSP, */ /* can use rawNTLMSSP instead of NTLMSSP via SPNEGO */
 	Kerberos,		/* Kerberos via SPNEGO */
 };
 
-- 
1.8.1.4

Re: [PATCH 11/19] cifs: add new "Unspecified" securityEnum value

[Pavel Shilovsky]

2013/5/23 Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>:
> Add a new securityEnum value to cover the case where a sec= option
> was not explicitly set.
>
> Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
>  fs/cifs/cifsglob.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
> index 874b29b..a858037 100644
> --- a/fs/cifs/cifsglob.h
> +++ b/fs/cifs/cifsglob.h
> @@ -101,11 +101,11 @@ enum statusEnum {
>  };
>
>  enum securityEnum {
> -       LANMAN = 0,                     /* Legacy LANMAN auth */
> +       Unspecified = 0,        /* not specified */
> +       LANMAN,                 /* Legacy LANMAN auth */
>         NTLM,                   /* Legacy NTLM012 auth with NTLM hash */
>         NTLMv2,                 /* Legacy NTLM auth with NTLMv2 hash */
>         RawNTLMSSP,             /* NTLMSSP without SPNEGO, NTLMv2 hash */
> -/*     NTLMSSP, */ /* can use rawNTLMSSP instead of NTLMSSP via SPNEGO */
>         Kerberos,               /* Kerberos via SPNEGO */
>  };
>
> --
> 1.8.1.4
>
> --
> 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

Reviewed-by: Pavel Shilovsky <piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org>

--
Best regards,
Pavel Shilovsky.

[PATCH 12/19] cifs: track the flavor of the NEGOTIATE reponse

[Jeff Layton]

Track what sort of NEGOTIATE response we get from the server, as that
will govern what sort of authentication types this socket will support.

There are three possibilities:

LANMAN: server sent legacy LANMAN-type response

UNENCAP: server sent a newer-style response, but extended security bit
wasn't set. This socket will only support unencapsulated auth types.

EXTENDED: server sent a newer-style response with the extended security
bit set. This is necessary to support krb5 and ntlmssp auth types.

Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 fs/cifs/cifsglob.h |  4 ++++
 fs/cifs/cifssmb.c  | 15 ++++++++++-----
 fs/cifs/smb2pdu.c  |  2 ++
 3 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index a858037..c2ef6c1 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -540,6 +540,10 @@ struct TCP_Server_Info {
 	struct session_key session_key;
 	unsigned long lstrp; /* when we got last response from this server */
 	struct cifs_secmech secmech; /* crypto sec mech functs, descriptors */
+#define	CIFS_NEGFLAVOR_LANMAN	0	/* wct == 13, LANMAN */
+#define	CIFS_NEGFLAVOR_UNENCAP	1	/* wct == 17, but no ext_sec */
+#define	CIFS_NEGFLAVOR_EXTENDED	2	/* wct == 17, ext_sec bit set */
+	char	negflavor;	/* NEGOTIATE response flavor */
 	/* extended security flavors that server supports */
 	bool	sec_ntlmssp;		/* supports NTLMSSP */
 	bool	sec_kerberosu2u;	/* supports U2U Kerberos */
diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
index e7184b9..b43948a 100644
--- a/fs/cifs/cifssmb.c
+++ b/fs/cifs/cifssmb.c
@@ -616,6 +616,7 @@ CIFSSMBNegotiate(const unsigned int xid, struct cifs_ses *ses)
 		goto neg_err_exit;
 	} else if (pSMBr->hdr.WordCount == 13) {
 		rc = decode_lanman_negprot_rsp(server, pSMBr, secFlags);
+		server->negflavor = CIFS_NEGFLAVOR_LANMAN;
 		goto neg_err_exit;
 	} else if (pSMBr->hdr.WordCount != 17) {
 		/* unknown wct */
@@ -666,17 +667,21 @@ CIFSSMBNegotiate(const unsigned int xid, struct cifs_ses *ses)
 	server->timeAdj = (int)(__s16)le16_to_cpu(pSMBr->ServerTimeZone);
 	server->timeAdj *= 60;
 
-	if (pSMBr->EncryptionKeyLength == CIFS_CRYPTO_KEY_SIZE)
+	if (pSMBr->EncryptionKeyLength == CIFS_CRYPTO_KEY_SIZE) {
+		server->negflavor = CIFS_NEGFLAVOR_UNENCAP;
 		memcpy(ses->server->cryptkey, pSMBr->u.EncryptionKey,
 		       CIFS_CRYPTO_KEY_SIZE);
-	else if ((pSMBr->hdr.Flags2 & SMBFLG2_EXT_SEC ||
+	} else if ((pSMBr->hdr.Flags2 & SMBFLG2_EXT_SEC ||
 			server->capabilities & CAP_EXTENDED_SECURITY) &&
-				(pSMBr->EncryptionKeyLength == 0))
+				(pSMBr->EncryptionKeyLength == 0)) {
+		server->negflavor = CIFS_NEGFLAVOR_EXTENDED;
 		rc = decode_ext_sec_blob(server, pSMBr);
-	else if (server->sec_mode & SECMODE_PW_ENCRYPT)
+	} else if (server->sec_mode & SECMODE_PW_ENCRYPT) {
 		rc = -EIO; /* no crypt key only if plain text pwd */
-	else
+	} else {
+		server->negflavor = CIFS_NEGFLAVOR_UNENCAP;
 		server->capabilities &= ~CAP_EXTENDED_SECURITY;
+	}
 
 	if (!rc)
 		rc = cifs_enable_signing(server, secFlags);
diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
index ebb97b4..1609699 100644
--- a/fs/cifs/smb2pdu.c
+++ b/fs/cifs/smb2pdu.c
@@ -405,6 +405,8 @@ SMB2_negotiate(const unsigned int xid, struct cifs_ses *ses)
 	}
 	server->dialect = le16_to_cpu(rsp->DialectRevision);
 
+	/* SMB2 only has an extended negflavor */
+	server->negflavor = CIFS_NEGFLAVOR_EXTENDED;
 	server->maxBuf = le32_to_cpu(rsp->MaxTransactSize);
 	server->max_read = le32_to_cpu(rsp->MaxReadSize);
 	server->max_write = le32_to_cpu(rsp->MaxWriteSize);
-- 
1.8.1.4

Re: [PATCH 12/19] cifs: track the flavor of the NEGOTIATE reponse

[Pavel Shilovsky]

2013/5/23 Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>:
> Track what sort of NEGOTIATE response we get from the server, as that
> will govern what sort of authentication types this socket will support.
>
> There are three possibilities:
>
> LANMAN: server sent legacy LANMAN-type response
>
> UNENCAP: server sent a newer-style response, but extended security bit
> wasn't set. This socket will only support unencapsulated auth types.
>
> EXTENDED: server sent a newer-style response with the extended security
> bit set. This is necessary to support krb5 and ntlmssp auth types.
>
> Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
>  fs/cifs/cifsglob.h |  4 ++++
>  fs/cifs/cifssmb.c  | 15 ++++++++++-----
>  fs/cifs/smb2pdu.c  |  2 ++
>  3 files changed, 16 insertions(+), 5 deletions(-)
>
> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
> index a858037..c2ef6c1 100644
> --- a/fs/cifs/cifsglob.h
> +++ b/fs/cifs/cifsglob.h
> @@ -540,6 +540,10 @@ struct TCP_Server_Info {
>         struct session_key session_key;
>         unsigned long lstrp; /* when we got last response from this server */
>         struct cifs_secmech secmech; /* crypto sec mech functs, descriptors */
> +#define        CIFS_NEGFLAVOR_LANMAN   0       /* wct == 13, LANMAN */
> +#define        CIFS_NEGFLAVOR_UNENCAP  1       /* wct == 17, but no ext_sec */
> +#define        CIFS_NEGFLAVOR_EXTENDED 2       /* wct == 17, ext_sec bit set */
> +       char    negflavor;      /* NEGOTIATE response flavor */
>         /* extended security flavors that server supports */
>         bool    sec_ntlmssp;            /* supports NTLMSSP */
>         bool    sec_kerberosu2u;        /* supports U2U Kerberos */
> diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
> index e7184b9..b43948a 100644
> --- a/fs/cifs/cifssmb.c
> +++ b/fs/cifs/cifssmb.c
> @@ -616,6 +616,7 @@ CIFSSMBNegotiate(const unsigned int xid, struct cifs_ses *ses)
>                 goto neg_err_exit;
>         } else if (pSMBr->hdr.WordCount == 13) {
>                 rc = decode_lanman_negprot_rsp(server, pSMBr, secFlags);
> +               server->negflavor = CIFS_NEGFLAVOR_LANMAN;
>                 goto neg_err_exit;
>         } else if (pSMBr->hdr.WordCount != 17) {
>                 /* unknown wct */
> @@ -666,17 +667,21 @@ CIFSSMBNegotiate(const unsigned int xid, struct cifs_ses *ses)
>         server->timeAdj = (int)(__s16)le16_to_cpu(pSMBr->ServerTimeZone);
>         server->timeAdj *= 60;
>
> -       if (pSMBr->EncryptionKeyLength == CIFS_CRYPTO_KEY_SIZE)
> +       if (pSMBr->EncryptionKeyLength == CIFS_CRYPTO_KEY_SIZE) {
> +               server->negflavor = CIFS_NEGFLAVOR_UNENCAP;
>                 memcpy(ses->server->cryptkey, pSMBr->u.EncryptionKey,
>                        CIFS_CRYPTO_KEY_SIZE);
> -       else if ((pSMBr->hdr.Flags2 & SMBFLG2_EXT_SEC ||
> +       } else if ((pSMBr->hdr.Flags2 & SMBFLG2_EXT_SEC ||
>                         server->capabilities & CAP_EXTENDED_SECURITY) &&
> -                               (pSMBr->EncryptionKeyLength == 0))
> +                               (pSMBr->EncryptionKeyLength == 0)) {
> +               server->negflavor = CIFS_NEGFLAVOR_EXTENDED;
>                 rc = decode_ext_sec_blob(server, pSMBr);
> -       else if (server->sec_mode & SECMODE_PW_ENCRYPT)
> +       } else if (server->sec_mode & SECMODE_PW_ENCRYPT) {
>                 rc = -EIO; /* no crypt key only if plain text pwd */
> -       else
> +       } else {
> +               server->negflavor = CIFS_NEGFLAVOR_UNENCAP;
>                 server->capabilities &= ~CAP_EXTENDED_SECURITY;
> +       }
>
>         if (!rc)
>                 rc = cifs_enable_signing(server, secFlags);
> diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
> index ebb97b4..1609699 100644
> --- a/fs/cifs/smb2pdu.c
> +++ b/fs/cifs/smb2pdu.c
> @@ -405,6 +405,8 @@ SMB2_negotiate(const unsigned int xid, struct cifs_ses *ses)
>         }
>         server->dialect = le16_to_cpu(rsp->DialectRevision);
>
> +       /* SMB2 only has an extended negflavor */
> +       server->negflavor = CIFS_NEGFLAVOR_EXTENDED;
>         server->maxBuf = le32_to_cpu(rsp->MaxTransactSize);
>         server->max_read = le32_to_cpu(rsp->MaxReadSize);
>         server->max_write = le32_to_cpu(rsp->MaxWriteSize);
> --
> 1.8.1.4
>
> --
> 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

Reviewed-by: Pavel Shilovsky <piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org>

--
Best regards,
Pavel Shilovsky.

[PATCH 13/19] cifs: add new fields to smb_vol to track the requested security flavor

[Jeff Layton]

We have this to some degree already in secFlgs, but those get "or'ed" so
there's no way to know what the last option requested was. Add new fields
that will eventually supercede the secFlgs field in the cifs_ses.

Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 fs/cifs/cifsglob.h |  2 ++
 fs/cifs/connect.c  | 25 +++++++++++++++++++++++++
 2 files changed, 27 insertions(+)

diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index c2ef6c1..9f88a35 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -402,6 +402,8 @@ struct smb_vol {
 	umode_t file_mode;
 	umode_t dir_mode;
 	unsigned secFlg;
+	enum securityEnum sectype; /* sectype requested via mnt opts */
+	bool sign; /* was signing requested via mnt opts? */
 	bool retry:1;
 	bool intr:1;
 	bool setuids:1;
diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index b367a5a..7b71961 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -1024,11 +1024,21 @@ static int cifs_parse_security_flavors(char *value,
 
 	substring_t args[MAX_OPT_ARGS];
 
+	/*
+	 * With mount options, the last one should win. Reset any existing
+	 * settings back to default.
+	 */
+	vol->sectype = Unspecified;
+	vol->sign = false;
+
 	switch (match_token(value, cifs_secflavor_tokens, args)) {
 	case Opt_sec_krb5:
+		vol->sectype = Kerberos;
 		vol->secFlg |= CIFSSEC_MAY_KRB5 | CIFSSEC_MAY_SIGN;
 		break;
 	case Opt_sec_krb5i:
+		vol->sectype = Kerberos;
+		vol->sign = true;
 		vol->secFlg |= CIFSSEC_MAY_KRB5 | CIFSSEC_MUST_SIGN;
 		break;
 	case Opt_sec_krb5p:
@@ -1036,26 +1046,36 @@ static int cifs_parse_security_flavors(char *value,
 		cifs_dbg(VFS, "sec=krb5p is not supported!\n");
 		return 1;
 	case Opt_sec_ntlmssp:
+		vol->sectype = RawNTLMSSP;
 		vol->secFlg |= CIFSSEC_MAY_NTLMSSP;
 		break;
 	case Opt_sec_ntlmsspi:
+		vol->sectype = RawNTLMSSP;
+		vol->sign = true;
 		vol->secFlg |= CIFSSEC_MAY_NTLMSSP | CIFSSEC_MUST_SIGN;
 		break;
 	case Opt_ntlm:
 		/* ntlm is default so can be turned off too */
+		vol->sectype = NTLM;
 		vol->secFlg |= CIFSSEC_MAY_NTLM;
 		break;
 	case Opt_sec_ntlmi:
+		vol->sectype = NTLM;
+		vol->sign = true;
 		vol->secFlg |= CIFSSEC_MAY_NTLM | CIFSSEC_MUST_SIGN;
 		break;
 	case Opt_sec_ntlmv2:
+		vol->sectype = NTLMv2;
 		vol->secFlg |= CIFSSEC_MAY_NTLMV2;
 		break;
 	case Opt_sec_ntlmv2i:
+		vol->sectype = NTLMv2;
+		vol->sign = true;
 		vol->secFlg |= CIFSSEC_MAY_NTLMV2 | CIFSSEC_MUST_SIGN;
 		break;
 #ifdef CONFIG_CIFS_WEAK_PW_HASH
 	case Opt_sec_lanman:
+		vol->sectype = LANMAN;
 		vol->secFlg |= CIFSSEC_MAY_LANMAN;
 		break;
 #endif
@@ -1425,6 +1445,7 @@ cifs_parse_mount_options(const char *mountdata, const char *devname,
 			break;
 		case Opt_sign:
 			vol->secFlg |= CIFSSEC_MUST_SIGN;
+			vol->sign = true;
 			break;
 		case Opt_noac:
 			printk(KERN_WARNING "CIFS: Mount option noac not "
@@ -3880,6 +3901,10 @@ cifs_set_vol_auth(struct smb_vol *vol, struct cifs_ses *ses)
 	case LANMAN:
 		vol->secFlg = CIFSSEC_MUST_LANMAN;
 		break;
+	default:
+		/* should never happen */
+		vol->secFlg = 0;
+		break;
 	}
 
 	return cifs_set_cifscreds(vol, ses);
-- 
1.8.1.4

Re: [PATCH 13/19] cifs: add new fields to smb_vol to track the requested security flavor

[Pavel Shilovsky]

2013/5/23 Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>:
> We have this to some degree already in secFlgs, but those get "or'ed" so
> there's no way to know what the last option requested was. Add new fields
> that will eventually supercede the secFlgs field in the cifs_ses.
>
> Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
>  fs/cifs/cifsglob.h |  2 ++
>  fs/cifs/connect.c  | 25 +++++++++++++++++++++++++
>  2 files changed, 27 insertions(+)
>
> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
> index c2ef6c1..9f88a35 100644
> --- a/fs/cifs/cifsglob.h
> +++ b/fs/cifs/cifsglob.h
> @@ -402,6 +402,8 @@ struct smb_vol {
>         umode_t file_mode;
>         umode_t dir_mode;
>         unsigned secFlg;
> +       enum securityEnum sectype; /* sectype requested via mnt opts */
> +       bool sign; /* was signing requested via mnt opts? */
>         bool retry:1;
>         bool intr:1;
>         bool setuids:1;
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index b367a5a..7b71961 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -1024,11 +1024,21 @@ static int cifs_parse_security_flavors(char *value,
>
>         substring_t args[MAX_OPT_ARGS];
>
> +       /*
> +        * With mount options, the last one should win. Reset any existing
> +        * settings back to default.
> +        */
> +       vol->sectype = Unspecified;
> +       vol->sign = false;
> +
>         switch (match_token(value, cifs_secflavor_tokens, args)) {
>         case Opt_sec_krb5:
> +               vol->sectype = Kerberos;
>                 vol->secFlg |= CIFSSEC_MAY_KRB5 | CIFSSEC_MAY_SIGN;
>                 break;
>         case Opt_sec_krb5i:
> +               vol->sectype = Kerberos;
> +               vol->sign = true;
>                 vol->secFlg |= CIFSSEC_MAY_KRB5 | CIFSSEC_MUST_SIGN;
>                 break;
>         case Opt_sec_krb5p:
> @@ -1036,26 +1046,36 @@ static int cifs_parse_security_flavors(char *value,
>                 cifs_dbg(VFS, "sec=krb5p is not supported!\n");
>                 return 1;
>         case Opt_sec_ntlmssp:
> +               vol->sectype = RawNTLMSSP;
>                 vol->secFlg |= CIFSSEC_MAY_NTLMSSP;
>                 break;
>         case Opt_sec_ntlmsspi:
> +               vol->sectype = RawNTLMSSP;
> +               vol->sign = true;
>                 vol->secFlg |= CIFSSEC_MAY_NTLMSSP | CIFSSEC_MUST_SIGN;
>                 break;
>         case Opt_ntlm:
>                 /* ntlm is default so can be turned off too */
> +               vol->sectype = NTLM;
>                 vol->secFlg |= CIFSSEC_MAY_NTLM;
>                 break;
>         case Opt_sec_ntlmi:
> +               vol->sectype = NTLM;
> +               vol->sign = true;
>                 vol->secFlg |= CIFSSEC_MAY_NTLM | CIFSSEC_MUST_SIGN;
>                 break;
>         case Opt_sec_ntlmv2:
> +               vol->sectype = NTLMv2;
>                 vol->secFlg |= CIFSSEC_MAY_NTLMV2;
>                 break;
>         case Opt_sec_ntlmv2i:
> +               vol->sectype = NTLMv2;
> +               vol->sign = true;
>                 vol->secFlg |= CIFSSEC_MAY_NTLMV2 | CIFSSEC_MUST_SIGN;
>                 break;
>  #ifdef CONFIG_CIFS_WEAK_PW_HASH
>         case Opt_sec_lanman:
> +               vol->sectype = LANMAN;
>                 vol->secFlg |= CIFSSEC_MAY_LANMAN;
>                 break;
>  #endif
> @@ -1425,6 +1445,7 @@ cifs_parse_mount_options(const char *mountdata, const char *devname,
>                         break;
>                 case Opt_sign:
>                         vol->secFlg |= CIFSSEC_MUST_SIGN;
> +                       vol->sign = true;
>                         break;
>                 case Opt_noac:
>                         printk(KERN_WARNING "CIFS: Mount option noac not "
> @@ -3880,6 +3901,10 @@ cifs_set_vol_auth(struct smb_vol *vol, struct cifs_ses *ses)
>         case LANMAN:
>                 vol->secFlg = CIFSSEC_MUST_LANMAN;
>                 break;
> +       default:
> +               /* should never happen */
> +               vol->secFlg = 0;
> +               break;
>         }
>
>         return cifs_set_cifscreds(vol, ses);
> --
> 1.8.1.4
>
> --
> 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

Reviewed-by: Pavel Shilovsky <piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org>

--
Best regards,
Pavel Shilovsky.

[PATCH 14/19] cifs: add new fields to cifs_ses to track requested security flavor

[Jeff Layton]

Currently we have the overrideSecFlg field, but it's quite cumbersome
to work with. Add some new fields that will eventually supercede it.

Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 fs/cifs/cifsfs.c   | 11 +++++++----
 fs/cifs/cifsglob.h |  2 ++
 fs/cifs/connect.c  |  5 +++++
 3 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index bb27269..97601fa 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -312,11 +312,14 @@ cifs_show_address(struct seq_file *s, struct TCP_Server_Info *server)
 }
 
 static void
-cifs_show_security(struct seq_file *s, struct TCP_Server_Info *server)
+cifs_show_security(struct seq_file *s, struct cifs_ses *ses)
 {
+	if (ses->sectype == Unspecified)
+		return;
+
 	seq_printf(s, ",sec=");
 
-	switch (server->secType) {
+	switch (ses->sectype) {
 	case LANMAN:
 		seq_printf(s, "lanman");
 		break;
@@ -338,7 +341,7 @@ cifs_show_security(struct seq_file *s, struct TCP_Server_Info *server)
 		break;
 	}
 
-	if (server->sec_mode & (SECMODE_SIGN_REQUIRED | SECMODE_SIGN_ENABLED))
+	if (ses->sign)
 		seq_printf(s, "i");
 }
 
@@ -369,7 +372,7 @@ cifs_show_options(struct seq_file *s, struct dentry *root)
 	srcaddr = (struct sockaddr *)&tcon->ses->server->srcaddr;
 
 	seq_printf(s, ",vers=%s", tcon->ses->server->vals->version_string);
-	cifs_show_security(s, tcon->ses->server);
+	cifs_show_security(s, tcon->ses);
 	cifs_show_cache_flavor(s, cifs_sb);
 
 	if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MULTIUSER)
diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index 9f88a35..a911a33 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -713,6 +713,8 @@ struct cifs_ses {
 	char *password;
 	struct session_key auth_key;
 	struct ntlmssp_auth *ntlmssp; /* ciphertext, flags, server challenge */
+	enum securityEnum sectype; /* what security flavor was specified? */
+	bool sign;		/* is signing required? */
 	bool need_reconnect:1; /* connection reset, uid now invalid */
 #ifdef CONFIG_CIFS_SMB2
 	__u16 session_flags;
diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index 7b71961..072598f 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -2513,6 +2513,9 @@ cifs_get_smb_ses(struct TCP_Server_Info *server, struct smb_vol *volume_info)
 	ses->linux_uid = volume_info->linux_uid;
 
 	ses->overrideSecFlg = volume_info->secFlg;
+	ses->sectype = volume_info->sectype;
+	ses->sign = volume_info->sign ? volume_info->sign :
+				(global_secflags & CIFSSEC_MUST_SIGN);
 
 	mutex_lock(&ses->session_mutex);
 	rc = cifs_negotiate_protocol(xid, ses);
@@ -3931,6 +3934,8 @@ cifs_construct_tcon(struct cifs_sb_info *cifs_sb, kuid_t fsuid)
 	vol_info->nocase = master_tcon->nocase;
 	vol_info->local_lease = master_tcon->local_lease;
 	vol_info->no_linux_ext = !master_tcon->unix_ext;
+	vol_info->sectype = master_tcon->ses->sectype;
+	vol_info->sign = master_tcon->ses->sign;
 
 	rc = cifs_set_vol_auth(vol_info, master_tcon->ses);
 	if (rc) {
-- 
1.8.1.4

Re: [PATCH 14/19] cifs: add new fields to cifs_ses to track requested security flavor

[Jeff Layton]

On Thu, 23 May 2013 11:05:58 -0400
Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:

> Currently we have the overrideSecFlg field, but it's quite cumbersome
> to work with. Add some new fields that will eventually supercede it.
> 
> Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
>  fs/cifs/cifsfs.c   | 11 +++++++----
>  fs/cifs/cifsglob.h |  2 ++
>  fs/cifs/connect.c  |  5 +++++
>  3 files changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
> index bb27269..97601fa 100644
> --- a/fs/cifs/cifsfs.c
> +++ b/fs/cifs/cifsfs.c
> @@ -312,11 +312,14 @@ cifs_show_address(struct seq_file *s, struct TCP_Server_Info *server)
>  }
>  
>  static void
> -cifs_show_security(struct seq_file *s, struct TCP_Server_Info *server)
> +cifs_show_security(struct seq_file *s, struct cifs_ses *ses)
>  {
> +	if (ses->sectype == Unspecified)
> +		return;
> +
>  	seq_printf(s, ",sec=");
>  
> -	switch (server->secType) {
> +	switch (ses->sectype) {
>  	case LANMAN:
>  		seq_printf(s, "lanman");
>  		break;
> @@ -338,7 +341,7 @@ cifs_show_security(struct seq_file *s, struct TCP_Server_Info *server)
>  		break;
>  	}
>  
> -	if (server->sec_mode & (SECMODE_SIGN_REQUIRED | SECMODE_SIGN_ENABLED))
> +	if (ses->sign)
>  		seq_printf(s, "i");
>  }
>  
> @@ -369,7 +372,7 @@ cifs_show_options(struct seq_file *s, struct dentry *root)
>  	srcaddr = (struct sockaddr *)&tcon->ses->server->srcaddr;
>  
>  	seq_printf(s, ",vers=%s", tcon->ses->server->vals->version_string);
> -	cifs_show_security(s, tcon->ses->server);
> +	cifs_show_security(s, tcon->ses);
>  	cifs_show_cache_flavor(s, cifs_sb);
>  
>  	if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MULTIUSER)
> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
> index 9f88a35..a911a33 100644
> --- a/fs/cifs/cifsglob.h
> +++ b/fs/cifs/cifsglob.h
> @@ -713,6 +713,8 @@ struct cifs_ses {
>  	char *password;
>  	struct session_key auth_key;
>  	struct ntlmssp_auth *ntlmssp; /* ciphertext, flags, server challenge */
> +	enum securityEnum sectype; /* what security flavor was specified? */
> +	bool sign;		/* is signing required? */
>  	bool need_reconnect:1; /* connection reset, uid now invalid */
>  #ifdef CONFIG_CIFS_SMB2
>  	__u16 session_flags;
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index 7b71961..072598f 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -2513,6 +2513,9 @@ cifs_get_smb_ses(struct TCP_Server_Info *server, struct smb_vol *volume_info)
>  	ses->linux_uid = volume_info->linux_uid;
>  
>  	ses->overrideSecFlg = volume_info->secFlg;
> +	ses->sectype = volume_info->sectype;
> +	ses->sign = volume_info->sign ? volume_info->sign :
> +				(global_secflags & CIFSSEC_MUST_SIGN);

Note that there's a minor bug in the above line. CIFSSEC_MUST_SIGN is
CIFSSEC_MAY_SIGN or'ed with another bit. So this ends up setting
ses->sign when only CIFSSEC_MAY_SIGN is set. I've got that fixed in my
repo, and the next iteration of the set will include it.

>  
>  	mutex_lock(&ses->session_mutex);
>  	rc = cifs_negotiate_protocol(xid, ses);
> @@ -3931,6 +3934,8 @@ cifs_construct_tcon(struct cifs_sb_info *cifs_sb, kuid_t fsuid)
>  	vol_info->nocase = master_tcon->nocase;
>  	vol_info->local_lease = master_tcon->local_lease;
>  	vol_info->no_linux_ext = !master_tcon->unix_ext;
> +	vol_info->sectype = master_tcon->ses->sectype;
> +	vol_info->sign = master_tcon->ses->sign;
>  
>  	rc = cifs_set_vol_auth(vol_info, master_tcon->ses);
>  	if (rc) {


-- 
Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

[PATCH 15/19] cifs: track the enablement of signing in the TCP_Server_Info

[Jeff Layton]

Currently, we determine this according to flags in the sec_mode, flags
in the global_secflags and via other methods. That makes the semantics
very hard to follow and there are corner cases where we don't handle
this correctly.

Add a new bool to the TCP_Server_Info that acts as a simple flag to tell
us whether signing is enabled on this connection or not, and fix up the
places that need to determine this to use that flag.

This is a bit weird for the SMB2 case, where signing is per-session.
SMB2 needs work in this area already though. The existing SMB2 code has
similar logic to what we're using here, so there should be no real
change in behavior. These changes should make it easier to implement
per-session signing in the future though.

Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 fs/cifs/cifsglob.h      |  1 +
 fs/cifs/cifsproto.h     |  2 +-
 fs/cifs/cifssmb.c       | 68 +++++++++++++++++++++----------------------------
 fs/cifs/connect.c       | 12 +++------
 fs/cifs/misc.c          |  3 +--
 fs/cifs/sess.c          |  9 +++----
 fs/cifs/smb1ops.c       |  3 +--
 fs/cifs/smb2pdu.c       | 40 +++++++++++++----------------
 fs/cifs/smb2transport.c |  3 +--
 fs/cifs/transport.c     |  4 +--
 10 files changed, 59 insertions(+), 86 deletions(-)

diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index a911a33..f392ae4 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -510,6 +510,7 @@ struct TCP_Server_Info {
 	struct task_struct *tsk;
 	char server_GUID[16];
 	__u16 sec_mode;
+	bool sign; /* is signing enabled on this connection? */
 	bool session_estab; /* mark when very first sess is established */
 #ifdef CONFIG_CIFS_SMB2
 	int echo_credits;  /* echo reserved slots */
diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
index f0e93ff..ede010f 100644
--- a/fs/cifs/cifsproto.h
+++ b/fs/cifs/cifsproto.h
@@ -212,7 +212,7 @@ extern int cifs_negotiate_protocol(const unsigned int xid,
 				   struct cifs_ses *ses);
 extern int cifs_setup_session(const unsigned int xid, struct cifs_ses *ses,
 			      struct nls_table *nls_info);
-extern int cifs_enable_signing(struct TCP_Server_Info *server, unsigned int secFlags);
+extern int cifs_enable_signing(struct TCP_Server_Info *server, bool mnt_sign_required);
 extern int CIFSSMBNegotiate(const unsigned int xid, struct cifs_ses *ses);
 
 extern int CIFSTCon(const unsigned int xid, struct cifs_ses *ses,
diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
index b43948a..c118533 100644
--- a/fs/cifs/cifssmb.c
+++ b/fs/cifs/cifssmb.c
@@ -418,32 +418,27 @@ decode_ext_sec_blob(struct TCP_Server_Info *server, NEGOTIATE_RSP *pSMBr)
 }
 
 int
-cifs_enable_signing(struct TCP_Server_Info *server, unsigned int secFlags)
+cifs_enable_signing(struct TCP_Server_Info *server, bool mnt_sign_required)
 {
-	if ((secFlags & CIFSSEC_MAY_SIGN) == 0) {
-		/* MUST_SIGN already includes the MAY_SIGN FLAG
-		   so if this is zero it means that signing is disabled */
-		cifs_dbg(FYI, "Signing disabled\n");
-		if (server->sec_mode & SECMODE_SIGN_REQUIRED) {
-			cifs_dbg(VFS, "Server requires packet signing to be enabled in /proc/fs/cifs/SecurityFlags\n");
-			return -EOPNOTSUPP;
+	bool srv_sign_required = server->sec_mode & SECMODE_SIGN_REQUIRED;
+	bool srv_sign_enabled = server->sec_mode & SECMODE_SIGN_ENABLED;
+	bool mnt_sign_enabled = mnt_sign_required ? mnt_sign_required :
+					(global_secflags & CIFSSEC_MAY_SIGN);
+
+	if (srv_sign_required) {
+		if (!mnt_sign_enabled) {
+			cifs_dbg(VFS, "server requires signing, but it's disabled in SecurityFlags!");
+			return -ENOTSUPP;
 		}
-		server->sec_mode &=
-			~(SECMODE_SIGN_ENABLED | SECMODE_SIGN_REQUIRED);
-	} else if ((secFlags & CIFSSEC_MUST_SIGN) == CIFSSEC_MUST_SIGN) {
-		/* signing required */
-		cifs_dbg(FYI, "Must sign - secFlags 0x%x\n", secFlags);
-		if ((server->sec_mode &
-			(SECMODE_SIGN_ENABLED | SECMODE_SIGN_REQUIRED)) == 0) {
-			cifs_dbg(VFS, "signing required but server lacks support\n");
-			return -EOPNOTSUPP;
-		} else
-			server->sec_mode |= SECMODE_SIGN_REQUIRED;
-	} else {
-		/* signing optional ie CIFSSEC_MAY_SIGN */
-		if ((server->sec_mode & SECMODE_SIGN_REQUIRED) == 0)
-			server->sec_mode &=
-				~(SECMODE_SIGN_ENABLED | SECMODE_SIGN_REQUIRED);
+		server->sign = true;
+	}
+
+	if (mnt_sign_required) {
+		if (!srv_sign_enabled) {
+			cifs_dbg(VFS, "server does not support signing!");
+			return -ENOTSUPP;
+		}
+		server->sign = true;
 	}
 
 	return 0;
@@ -452,7 +447,7 @@ cifs_enable_signing(struct TCP_Server_Info *server, unsigned int secFlags)
 #ifdef CONFIG_CIFS_WEAK_PW_HASH
 static int
 decode_lanman_negprot_rsp(struct TCP_Server_Info *server, NEGOTIATE_RSP *pSMBr,
-			  unsigned int secFlags)
+			  unsigned int secFlags, bool sign)
 {
 	__s16 tmp;
 	struct lanman_neg_rsp *rsp = (struct lanman_neg_rsp *)pSMBr;
@@ -527,12 +522,12 @@ decode_lanman_negprot_rsp(struct TCP_Server_Info *server, NEGOTIATE_RSP *pSMBr,
 	}
 
 	cifs_dbg(FYI, "LANMAN negotiated\n");
-	return cifs_enable_signing(server, secFlags);
+	return cifs_enable_signing(server, sign);
 }
 #else
 static inline int
 decode_lanman_negprot_rsp(struct TCP_Server_Info *server, NEGOTIATE_RSP *pSMBr,
-			  unsigned int secFlags)
+			  unsigned int secFlags, bool sign)
 {
 	cifs_dbg(VFS, "mount failed, cifs module not built with CIFS_WEAK_PW_HASH support\n");
 	return -EOPNOTSUPP;
@@ -615,7 +610,7 @@ CIFSSMBNegotiate(const unsigned int xid, struct cifs_ses *ses)
 		rc = -EOPNOTSUPP;
 		goto neg_err_exit;
 	} else if (pSMBr->hdr.WordCount == 13) {
-		rc = decode_lanman_negprot_rsp(server, pSMBr, secFlags);
+		rc = decode_lanman_negprot_rsp(server, pSMBr, secFlags, ses->sign);
 		server->negflavor = CIFS_NEGFLAVOR_LANMAN;
 		goto neg_err_exit;
 	} else if (pSMBr->hdr.WordCount != 17) {
@@ -684,7 +679,7 @@ CIFSSMBNegotiate(const unsigned int xid, struct cifs_ses *ses)
 	}
 
 	if (!rc)
-		rc = cifs_enable_signing(server, secFlags);
+		rc = cifs_enable_signing(server, ses->sign);
 neg_err_exit:
 	cifs_buf_release(pSMB);
 
@@ -809,9 +804,8 @@ CIFSSMBLogoff(const unsigned int xid, struct cifs_ses *ses)
 
 	pSMB->hdr.Mid = get_next_mid(ses->server);
 
-	if (ses->server->sec_mode &
-		   (SECMODE_SIGN_REQUIRED | SECMODE_SIGN_ENABLED))
-			pSMB->hdr.Flags2 |= SMBFLG2_SECURITY_SIGNATURE;
+	if (ses->server->sign)
+		pSMB->hdr.Flags2 |= SMBFLG2_SECURITY_SIGNATURE;
 
 	pSMB->hdr.Uid = ses->Suid;
 
@@ -1572,8 +1566,7 @@ cifs_readv_callback(struct mid_q_entry *mid)
 	switch (mid->mid_state) {
 	case MID_RESPONSE_RECEIVED:
 		/* result already set, check signature */
-		if (server->sec_mode &
-		    (SECMODE_SIGN_REQUIRED | SECMODE_SIGN_ENABLED)) {
+		if (server->sign) {
 			int rc = 0;
 
 			rc = cifs_verify_signature(&rqst, server,
@@ -4826,11 +4819,8 @@ getDFSRetry:
 		strncpy(pSMB->RequestFileName, search_name, name_len);
 	}
 
-	if (ses->server) {
-		if (ses->server->sec_mode &
-		   (SECMODE_SIGN_REQUIRED | SECMODE_SIGN_ENABLED))
-			pSMB->hdr.Flags2 |= SMBFLG2_SECURITY_SIGNATURE;
-	}
+	if (ses->server && ses->server->sign)
+		pSMB->hdr.Flags2 |= SMBFLG2_SECURITY_SIGNATURE;
 
 	pSMB->hdr.Uid = ses->Suid;
 
diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index 072598f..f5ea895 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -2028,13 +2028,8 @@ match_security(struct TCP_Server_Info *server, struct smb_vol *vol)
 	}
 
 	/* now check if signing mode is acceptable */
-	if ((secFlags & CIFSSEC_MAY_SIGN) == 0 &&
-	    (server->sec_mode & SECMODE_SIGN_REQUIRED))
-			return false;
-	else if (((secFlags & CIFSSEC_MUST_SIGN) == CIFSSEC_MUST_SIGN) &&
-		 (server->sec_mode &
-		  (SECMODE_SIGN_ENABLED|SECMODE_SIGN_REQUIRED)) == 0)
-			return false;
+	if (vol->sign && !server->sign)
+		return false;
 
 	return true;
 }
@@ -3693,8 +3688,7 @@ CIFSTCon(const unsigned int xid, struct cifs_ses *ses,
 		}
 	}
 
-	if (ses->server->sec_mode &
-			(SECMODE_SIGN_REQUIRED | SECMODE_SIGN_ENABLED))
+	if (ses->server->sign)
 		smb_buffer->Flags2 |= SMBFLG2_SECURITY_SIGNATURE;
 
 	if (ses->capabilities & CAP_STATUS32) {
diff --git a/fs/cifs/misc.c b/fs/cifs/misc.c
index 1bec014..f7d4b22 100644
--- a/fs/cifs/misc.c
+++ b/fs/cifs/misc.c
@@ -267,8 +267,7 @@ header_assemble(struct smb_hdr *buffer, char smb_command /* command */ ,
 		if (treeCon->nocase)
 			buffer->Flags  |= SMBFLG_CASELESS;
 		if ((treeCon->ses) && (treeCon->ses->server))
-			if (treeCon->ses->server->sec_mode &
-			  (SECMODE_SIGN_REQUIRED | SECMODE_SIGN_ENABLED))
+			if (treeCon->ses->server->sign)
 				buffer->Flags2 |= SMBFLG2_SECURITY_SIGNATURE;
 	}
 
diff --git a/fs/cifs/sess.c b/fs/cifs/sess.c
index 0d0fe38..82b784a 100644
--- a/fs/cifs/sess.c
+++ b/fs/cifs/sess.c
@@ -138,8 +138,7 @@ static __u32 cifs_ssetup_hdr(struct cifs_ses *ses, SESSION_SETUP_ANDX *pSMB)
 	capabilities = CAP_LARGE_FILES | CAP_NT_SMBS | CAP_LEVEL_II_OPLOCKS |
 			CAP_LARGE_WRITE_X | CAP_LARGE_READ_X;
 
-	if (ses->server->sec_mode &
-	    (SECMODE_SIGN_REQUIRED | SECMODE_SIGN_ENABLED))
+	if (ses->server->sign)
 		pSMB->req.hdr.Flags2 |= SMBFLG2_SECURITY_SIGNATURE;
 
 	if (ses->capabilities & CAP_UNICODE) {
@@ -427,8 +426,7 @@ void build_ntlmssp_negotiate_blob(unsigned char *pbuffer,
 	flags = NTLMSSP_NEGOTIATE_56 |	NTLMSSP_REQUEST_TARGET |
 		NTLMSSP_NEGOTIATE_128 | NTLMSSP_NEGOTIATE_UNICODE |
 		NTLMSSP_NEGOTIATE_NTLM | NTLMSSP_NEGOTIATE_EXTENDED_SEC;
-	if (ses->server->sec_mode &
-			(SECMODE_SIGN_REQUIRED | SECMODE_SIGN_ENABLED)) {
+	if (ses->server->sign) {
 		flags |= NTLMSSP_NEGOTIATE_SIGN;
 		if (!ses->server->session_estab)
 			flags |= NTLMSSP_NEGOTIATE_KEY_XCH;
@@ -466,8 +464,7 @@ int build_ntlmssp_auth_blob(unsigned char *pbuffer,
 		NTLMSSP_REQUEST_TARGET | NTLMSSP_NEGOTIATE_TARGET_INFO |
 		NTLMSSP_NEGOTIATE_128 | NTLMSSP_NEGOTIATE_UNICODE |
 		NTLMSSP_NEGOTIATE_NTLM | NTLMSSP_NEGOTIATE_EXTENDED_SEC;
-	if (ses->server->sec_mode &
-	   (SECMODE_SIGN_REQUIRED | SECMODE_SIGN_ENABLED)) {
+	if (ses->server->sign) {
 		flags |= NTLMSSP_NEGOTIATE_SIGN;
 		if (!ses->server->session_estab)
 			flags |= NTLMSSP_NEGOTIATE_KEY_XCH;
diff --git a/fs/cifs/smb1ops.c b/fs/cifs/smb1ops.c
index 7d1c78b..b28aabd 100644
--- a/fs/cifs/smb1ops.c
+++ b/fs/cifs/smb1ops.c
@@ -449,8 +449,7 @@ cifs_negotiate_wsize(struct cifs_tcon *tcon, struct smb_vol *volume_info)
 	 * WRITEX header, not including the 4 byte RFC1001 length.
 	 */
 	if (!(server->capabilities & CAP_LARGE_WRITE_X) ||
-	    (!(server->capabilities & CAP_UNIX) &&
-	     (server->sec_mode & (SECMODE_SIGN_ENABLED|SECMODE_SIGN_REQUIRED))))
+	    (!(server->capabilities & CAP_UNIX) && server->sign))
 		wsize = min_t(unsigned int, wsize,
 				server->maxBuf - sizeof(WRITE_REQ) + 4);
 
diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
index 1609699..c753508 100644
--- a/fs/cifs/smb2pdu.c
+++ b/fs/cifs/smb2pdu.c
@@ -119,8 +119,7 @@ smb2_hdr_assemble(struct smb2_hdr *hdr, __le16 smb2_cmd /* command */ ,
 	/* BB how does SMB2 do case sensitive? */
 	/* if (tcon->nocase)
 		hdr->Flags |= SMBFLG_CASELESS; */
-	if (tcon->ses && tcon->ses->server &&
-	    (tcon->ses->server->sec_mode & SECMODE_SIGN_REQUIRED))
+	if (tcon->ses && tcon->ses->server && tcon->ses->server->sign)
 		hdr->Flags |= SMB2_FLAGS_SIGNED;
 out:
 	pdu->StructureSize2 = cpu_to_le16(parmsize);
@@ -330,7 +329,6 @@ SMB2_negotiate(const unsigned int xid, struct cifs_ses *ses)
 	int resp_buftype;
 	struct TCP_Server_Info *server = ses->server;
 	unsigned int sec_flags;
-	u16 temp = 0;
 	int blob_offset, blob_length;
 	char *security_blob;
 	int flags = CIFS_NEG_OP;
@@ -362,12 +360,12 @@ SMB2_negotiate(const unsigned int xid, struct cifs_ses *ses)
 	inc_rfc1001_len(req, 2);
 
 	/* only one of SMB2 signing flags may be set in SMB2 request */
-	if ((sec_flags & CIFSSEC_MUST_SIGN) == CIFSSEC_MUST_SIGN)
-		temp = SMB2_NEGOTIATE_SIGNING_REQUIRED;
-	else if (sec_flags & CIFSSEC_MAY_SIGN) /* MAY_SIGN is a single flag */
-		temp = SMB2_NEGOTIATE_SIGNING_ENABLED;
-
-	req->SecurityMode = cpu_to_le16(temp);
+	if (ses->sign)
+		req->SecurityMode = SMB2_NEGOTIATE_SIGNING_REQUIRED;
+	else if (global_secflags & CIFSSEC_MAY_SIGN)
+		req->SecurityMode = SMB2_NEGOTIATE_SIGNING_ENABLED;
+	else
+		req->SecurityMode = 0;
 
 	req->Capabilities = cpu_to_le32(ses->server->vals->req_capabilities);
 
@@ -424,8 +422,7 @@ SMB2_negotiate(const unsigned int xid, struct cifs_ses *ses)
 		goto neg_exit;
 	}
 
-	cifs_dbg(FYI, "sec_flags 0x%x\n", sec_flags);
-	rc = cifs_enable_signing(server, sec_flags);
+	rc = cifs_enable_signing(server, ses->sign);
 #ifdef CONFIG_SMB2_ASN1  /* BB REMOVEME when updated asn1.c ready */
 	if (rc)
 		goto neg_exit;
@@ -457,7 +454,6 @@ SMB2_sess_setup(const unsigned int xid, struct cifs_ses *ses,
 	__le32 phase = NtLmNegotiate; /* NTLMSSP, if needed, is multistage */
 	struct TCP_Server_Info *server = ses->server;
 	unsigned int sec_flags;
-	u8 temp = 0;
 	u16 blob_length = 0;
 	char *security_blob;
 	char *ntlmssp_blob = NULL;
@@ -502,14 +498,13 @@ ssetup_ntlmssp_authenticate:
 	req->hdr.CreditRequest = cpu_to_le16(3);
 
 	/* only one of SMB2 signing flags may be set in SMB2 request */
-	if ((sec_flags & CIFSSEC_MUST_SIGN) == CIFSSEC_MUST_SIGN)
-		temp = SMB2_NEGOTIATE_SIGNING_REQUIRED;
-	else if (ses->server->sec_mode & SMB2_NEGOTIATE_SIGNING_REQUIRED)
-		temp = SMB2_NEGOTIATE_SIGNING_REQUIRED;
-	else if (sec_flags & CIFSSEC_MAY_SIGN) /* MAY_SIGN is a single flag */
-		temp = SMB2_NEGOTIATE_SIGNING_ENABLED;
-
-	req->SecurityMode = temp;
+	if (server->sign)
+		req->SecurityMode = SMB2_NEGOTIATE_SIGNING_REQUIRED;
+	else if (global_secflags & CIFSSEC_MAY_SIGN) /* MAY_SIGN is a single flag */
+		req->SecurityMode = SMB2_NEGOTIATE_SIGNING_ENABLED;
+	else
+		req->SecurityMode = 0;
+
 	req->Capabilities = 0;
 	req->Channel = 0; /* MBZ */
 
@@ -652,7 +647,7 @@ SMB2_logoff(const unsigned int xid, struct cifs_ses *ses)
 
 	 /* since no tcon, smb2_init can not do this, so do here */
 	req->hdr.SessionId = ses->Suid;
-	if (server->sec_mode & SECMODE_SIGN_REQUIRED)
+	if (server->sign)
 		req->hdr.Flags |= SMB2_FLAGS_SIGNED;
 
 	rc = SendReceiveNoRsp(xid, ses, (char *) &req->hdr, 0);
@@ -1357,8 +1352,7 @@ smb2_readv_callback(struct mid_q_entry *mid)
 	case MID_RESPONSE_RECEIVED:
 		credits_received = le16_to_cpu(buf->CreditRequest);
 		/* result already set, check signature */
-		if (server->sec_mode &
-		    (SECMODE_SIGN_REQUIRED | SECMODE_SIGN_ENABLED)) {
+		if (server->sign) {
 			int rc;
 
 			rc = smb2_verify_signature(&rqst, server);
diff --git a/fs/cifs/smb2transport.c b/fs/cifs/smb2transport.c
index 01f0ac8..c802ecf 100644
--- a/fs/cifs/smb2transport.c
+++ b/fs/cifs/smb2transport.c
@@ -275,8 +275,7 @@ smb2_check_receive(struct mid_q_entry *mid, struct TCP_Server_Info *server,
 
 	dump_smb(mid->resp_buf, min_t(u32, 80, len));
 	/* convert the length into a more usable form */
-	if ((len > 24) &&
-	    (server->sec_mode & (SECMODE_SIGN_REQUIRED|SECMODE_SIGN_ENABLED))) {
+	if (len > 24 && server->sign) {
 		int rc;
 
 		rc = smb2_verify_signature(&rqst, server);
diff --git a/fs/cifs/transport.c b/fs/cifs/transport.c
index bfbf470..1996d6c 100644
--- a/fs/cifs/transport.c
+++ b/fs/cifs/transport.c
@@ -463,7 +463,7 @@ cifs_setup_async_request(struct TCP_Server_Info *server, struct smb_rqst *rqst)
 	struct mid_q_entry *mid;
 
 	/* enable signing if server requires it */
-	if (server->sec_mode & (SECMODE_SIGN_REQUIRED | SECMODE_SIGN_ENABLED))
+	if (server->sign)
 		hdr->Flags2 |= SMBFLG2_SECURITY_SIGNATURE;
 
 	mid = AllocMidQEntry(hdr, server);
@@ -612,7 +612,7 @@ cifs_check_receive(struct mid_q_entry *mid, struct TCP_Server_Info *server,
 	dump_smb(mid->resp_buf, min_t(u32, 92, len));
 
 	/* convert the length into a more usable form */
-	if (server->sec_mode & (SECMODE_SIGN_REQUIRED | SECMODE_SIGN_ENABLED)) {
+	if (server->sign) {
 		struct kvec iov;
 		int rc = 0;
 		struct smb_rqst rqst = { .rq_iov = &iov,
-- 
1.8.1.4

Re: [PATCH 15/19] cifs: track the enablement of signing in the TCP_Server_Info

[Pavel Shilovsky]

2013/5/23 Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>:
> Currently, we determine this according to flags in the sec_mode, flags
> in the global_secflags and via other methods. That makes the semantics
> very hard to follow and there are corner cases where we don't handle
> this correctly.
>
> Add a new bool to the TCP_Server_Info that acts as a simple flag to tell
> us whether signing is enabled on this connection or not, and fix up the
> places that need to determine this to use that flag.

Good idea. Also the code now looks easier to understand.

>
> This is a bit weird for the SMB2 case, where signing is per-session.
> SMB2 needs work in this area already though. The existing SMB2 code has
> similar logic to what we're using here, so there should be no real
> change in behavior. These changes should make it easier to implement
> per-session signing in the future though.
>
> Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
>  fs/cifs/cifsglob.h      |  1 +
>  fs/cifs/cifsproto.h     |  2 +-
>  fs/cifs/cifssmb.c       | 68 +++++++++++++++++++++----------------------------
>  fs/cifs/connect.c       | 12 +++------
>  fs/cifs/misc.c          |  3 +--
>  fs/cifs/sess.c          |  9 +++----
>  fs/cifs/smb1ops.c       |  3 +--
>  fs/cifs/smb2pdu.c       | 40 +++++++++++++----------------
>  fs/cifs/smb2transport.c |  3 +--
>  fs/cifs/transport.c     |  4 +--
>  10 files changed, 59 insertions(+), 86 deletions(-)
>
> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
> index a911a33..f392ae4 100644
> --- a/fs/cifs/cifsglob.h
> +++ b/fs/cifs/cifsglob.h
> @@ -510,6 +510,7 @@ struct TCP_Server_Info {
>         struct task_struct *tsk;
>         char server_GUID[16];
>         __u16 sec_mode;
> +       bool sign; /* is signing enabled on this connection? */
>         bool session_estab; /* mark when very first sess is established */
>  #ifdef CONFIG_CIFS_SMB2
>         int echo_credits;  /* echo reserved slots */
> diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
> index f0e93ff..ede010f 100644
> --- a/fs/cifs/cifsproto.h
> +++ b/fs/cifs/cifsproto.h
> @@ -212,7 +212,7 @@ extern int cifs_negotiate_protocol(const unsigned int xid,
>                                    struct cifs_ses *ses);
>  extern int cifs_setup_session(const unsigned int xid, struct cifs_ses *ses,
>                               struct nls_table *nls_info);
> -extern int cifs_enable_signing(struct TCP_Server_Info *server, unsigned int secFlags);
> +extern int cifs_enable_signing(struct TCP_Server_Info *server, bool mnt_sign_required);
>  extern int CIFSSMBNegotiate(const unsigned int xid, struct cifs_ses *ses);
>
>  extern int CIFSTCon(const unsigned int xid, struct cifs_ses *ses,
> diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
> index b43948a..c118533 100644
> --- a/fs/cifs/cifssmb.c
> +++ b/fs/cifs/cifssmb.c
> @@ -418,32 +418,27 @@ decode_ext_sec_blob(struct TCP_Server_Info *server, NEGOTIATE_RSP *pSMBr)
>  }
>
>  int
> -cifs_enable_signing(struct TCP_Server_Info *server, unsigned int secFlags)
> +cifs_enable_signing(struct TCP_Server_Info *server, bool mnt_sign_required)
>  {
> -       if ((secFlags & CIFSSEC_MAY_SIGN) == 0) {
> -               /* MUST_SIGN already includes the MAY_SIGN FLAG
> -                  so if this is zero it means that signing is disabled */
> -               cifs_dbg(FYI, "Signing disabled\n");
> -               if (server->sec_mode & SECMODE_SIGN_REQUIRED) {
> -                       cifs_dbg(VFS, "Server requires packet signing to be enabled in /proc/fs/cifs/SecurityFlags\n");
> -                       return -EOPNOTSUPP;
> +       bool srv_sign_required = server->sec_mode & SECMODE_SIGN_REQUIRED;
> +       bool srv_sign_enabled = server->sec_mode & SECMODE_SIGN_ENABLED;
> +       bool mnt_sign_enabled = mnt_sign_required ? mnt_sign_required :
> +                                       (global_secflags & CIFSSEC_MAY_SIGN);
> +
> +       if (srv_sign_required) {
> +               if (!mnt_sign_enabled) {
> +                       cifs_dbg(VFS, "server requires signing, but it's disabled in SecurityFlags!");
> +                       return -ENOTSUPP;
>                 }
> -               server->sec_mode &=
> -                       ~(SECMODE_SIGN_ENABLED | SECMODE_SIGN_REQUIRED);
> -       } else if ((secFlags & CIFSSEC_MUST_SIGN) == CIFSSEC_MUST_SIGN) {
> -               /* signing required */
> -               cifs_dbg(FYI, "Must sign - secFlags 0x%x\n", secFlags);
> -               if ((server->sec_mode &
> -                       (SECMODE_SIGN_ENABLED | SECMODE_SIGN_REQUIRED)) == 0) {
> -                       cifs_dbg(VFS, "signing required but server lacks support\n");
> -                       return -EOPNOTSUPP;
> -               } else
> -                       server->sec_mode |= SECMODE_SIGN_REQUIRED;
> -       } else {
> -               /* signing optional ie CIFSSEC_MAY_SIGN */
> -               if ((server->sec_mode & SECMODE_SIGN_REQUIRED) == 0)
> -                       server->sec_mode &=
> -                               ~(SECMODE_SIGN_ENABLED | SECMODE_SIGN_REQUIRED);
> +               server->sign = true;
> +       }
> +
> +       if (mnt_sign_required) {
> +               if (!srv_sign_enabled) {
> +                       cifs_dbg(VFS, "server does not support signing!");
> +                       return -ENOTSUPP;
> +               }
> +               server->sign = true;
>         }
>
>         return 0;
> @@ -452,7 +447,7 @@ cifs_enable_signing(struct TCP_Server_Info *server, unsigned int secFlags)
>  #ifdef CONFIG_CIFS_WEAK_PW_HASH
>  static int
>  decode_lanman_negprot_rsp(struct TCP_Server_Info *server, NEGOTIATE_RSP *pSMBr,
> -                         unsigned int secFlags)
> +                         unsigned int secFlags, bool sign)
>  {
>         __s16 tmp;
>         struct lanman_neg_rsp *rsp = (struct lanman_neg_rsp *)pSMBr;
> @@ -527,12 +522,12 @@ decode_lanman_negprot_rsp(struct TCP_Server_Info *server, NEGOTIATE_RSP *pSMBr,
>         }
>
>         cifs_dbg(FYI, "LANMAN negotiated\n");
> -       return cifs_enable_signing(server, secFlags);
> +       return cifs_enable_signing(server, sign);
>  }
>  #else
>  static inline int
>  decode_lanman_negprot_rsp(struct TCP_Server_Info *server, NEGOTIATE_RSP *pSMBr,
> -                         unsigned int secFlags)
> +                         unsigned int secFlags, bool sign)
>  {
>         cifs_dbg(VFS, "mount failed, cifs module not built with CIFS_WEAK_PW_HASH support\n");
>         return -EOPNOTSUPP;
> @@ -615,7 +610,7 @@ CIFSSMBNegotiate(const unsigned int xid, struct cifs_ses *ses)
>                 rc = -EOPNOTSUPP;
>                 goto neg_err_exit;
>         } else if (pSMBr->hdr.WordCount == 13) {
> -               rc = decode_lanman_negprot_rsp(server, pSMBr, secFlags);
> +               rc = decode_lanman_negprot_rsp(server, pSMBr, secFlags, ses->sign);
>                 server->negflavor = CIFS_NEGFLAVOR_LANMAN;
>                 goto neg_err_exit;
>         } else if (pSMBr->hdr.WordCount != 17) {
> @@ -684,7 +679,7 @@ CIFSSMBNegotiate(const unsigned int xid, struct cifs_ses *ses)
>         }
>
>         if (!rc)
> -               rc = cifs_enable_signing(server, secFlags);
> +               rc = cifs_enable_signing(server, ses->sign);
>  neg_err_exit:
>         cifs_buf_release(pSMB);
>
> @@ -809,9 +804,8 @@ CIFSSMBLogoff(const unsigned int xid, struct cifs_ses *ses)
>
>         pSMB->hdr.Mid = get_next_mid(ses->server);
>
> -       if (ses->server->sec_mode &
> -                  (SECMODE_SIGN_REQUIRED | SECMODE_SIGN_ENABLED))
> -                       pSMB->hdr.Flags2 |= SMBFLG2_SECURITY_SIGNATURE;
> +       if (ses->server->sign)
> +               pSMB->hdr.Flags2 |= SMBFLG2_SECURITY_SIGNATURE;
>
>         pSMB->hdr.Uid = ses->Suid;
>
> @@ -1572,8 +1566,7 @@ cifs_readv_callback(struct mid_q_entry *mid)
>         switch (mid->mid_state) {
>         case MID_RESPONSE_RECEIVED:
>                 /* result already set, check signature */
> -               if (server->sec_mode &
> -                   (SECMODE_SIGN_REQUIRED | SECMODE_SIGN_ENABLED)) {
> +               if (server->sign) {
>                         int rc = 0;
>
>                         rc = cifs_verify_signature(&rqst, server,
> @@ -4826,11 +4819,8 @@ getDFSRetry:
>                 strncpy(pSMB->RequestFileName, search_name, name_len);
>         }
>
> -       if (ses->server) {
> -               if (ses->server->sec_mode &
> -                  (SECMODE_SIGN_REQUIRED | SECMODE_SIGN_ENABLED))
> -                       pSMB->hdr.Flags2 |= SMBFLG2_SECURITY_SIGNATURE;
> -       }
> +       if (ses->server && ses->server->sign)
> +               pSMB->hdr.Flags2 |= SMBFLG2_SECURITY_SIGNATURE;
>
>         pSMB->hdr.Uid = ses->Suid;
>
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index 072598f..f5ea895 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -2028,13 +2028,8 @@ match_security(struct TCP_Server_Info *server, struct smb_vol *vol)
>         }
>
>         /* now check if signing mode is acceptable */
> -       if ((secFlags & CIFSSEC_MAY_SIGN) == 0 &&
> -           (server->sec_mode & SECMODE_SIGN_REQUIRED))
> -                       return false;
> -       else if (((secFlags & CIFSSEC_MUST_SIGN) == CIFSSEC_MUST_SIGN) &&
> -                (server->sec_mode &
> -                 (SECMODE_SIGN_ENABLED|SECMODE_SIGN_REQUIRED)) == 0)
> -                       return false;
> +       if (vol->sign && !server->sign)
> +               return false;
>
>         return true;
>  }
> @@ -3693,8 +3688,7 @@ CIFSTCon(const unsigned int xid, struct cifs_ses *ses,
>                 }
>         }
>
> -       if (ses->server->sec_mode &
> -                       (SECMODE_SIGN_REQUIRED | SECMODE_SIGN_ENABLED))
> +       if (ses->server->sign)
>                 smb_buffer->Flags2 |= SMBFLG2_SECURITY_SIGNATURE;
>
>         if (ses->capabilities & CAP_STATUS32) {
> diff --git a/fs/cifs/misc.c b/fs/cifs/misc.c
> index 1bec014..f7d4b22 100644
> --- a/fs/cifs/misc.c
> +++ b/fs/cifs/misc.c
> @@ -267,8 +267,7 @@ header_assemble(struct smb_hdr *buffer, char smb_command /* command */ ,
>                 if (treeCon->nocase)
>                         buffer->Flags  |= SMBFLG_CASELESS;
>                 if ((treeCon->ses) && (treeCon->ses->server))
> -                       if (treeCon->ses->server->sec_mode &
> -                         (SECMODE_SIGN_REQUIRED | SECMODE_SIGN_ENABLED))
> +                       if (treeCon->ses->server->sign)
>                                 buffer->Flags2 |= SMBFLG2_SECURITY_SIGNATURE;
>         }
>
> diff --git a/fs/cifs/sess.c b/fs/cifs/sess.c
> index 0d0fe38..82b784a 100644
> --- a/fs/cifs/sess.c
> +++ b/fs/cifs/sess.c
> @@ -138,8 +138,7 @@ static __u32 cifs_ssetup_hdr(struct cifs_ses *ses, SESSION_SETUP_ANDX *pSMB)
>         capabilities = CAP_LARGE_FILES | CAP_NT_SMBS | CAP_LEVEL_II_OPLOCKS |
>                         CAP_LARGE_WRITE_X | CAP_LARGE_READ_X;
>
> -       if (ses->server->sec_mode &
> -           (SECMODE_SIGN_REQUIRED | SECMODE_SIGN_ENABLED))
> +       if (ses->server->sign)
>                 pSMB->req.hdr.Flags2 |= SMBFLG2_SECURITY_SIGNATURE;
>
>         if (ses->capabilities & CAP_UNICODE) {
> @@ -427,8 +426,7 @@ void build_ntlmssp_negotiate_blob(unsigned char *pbuffer,
>         flags = NTLMSSP_NEGOTIATE_56 |  NTLMSSP_REQUEST_TARGET |
>                 NTLMSSP_NEGOTIATE_128 | NTLMSSP_NEGOTIATE_UNICODE |
>                 NTLMSSP_NEGOTIATE_NTLM | NTLMSSP_NEGOTIATE_EXTENDED_SEC;
> -       if (ses->server->sec_mode &
> -                       (SECMODE_SIGN_REQUIRED | SECMODE_SIGN_ENABLED)) {
> +       if (ses->server->sign) {
>                 flags |= NTLMSSP_NEGOTIATE_SIGN;
>                 if (!ses->server->session_estab)
>                         flags |= NTLMSSP_NEGOTIATE_KEY_XCH;
> @@ -466,8 +464,7 @@ int build_ntlmssp_auth_blob(unsigned char *pbuffer,
>                 NTLMSSP_REQUEST_TARGET | NTLMSSP_NEGOTIATE_TARGET_INFO |
>                 NTLMSSP_NEGOTIATE_128 | NTLMSSP_NEGOTIATE_UNICODE |
>                 NTLMSSP_NEGOTIATE_NTLM | NTLMSSP_NEGOTIATE_EXTENDED_SEC;
> -       if (ses->server->sec_mode &
> -          (SECMODE_SIGN_REQUIRED | SECMODE_SIGN_ENABLED)) {
> +       if (ses->server->sign) {
>                 flags |= NTLMSSP_NEGOTIATE_SIGN;
>                 if (!ses->server->session_estab)
>                         flags |= NTLMSSP_NEGOTIATE_KEY_XCH;
> diff --git a/fs/cifs/smb1ops.c b/fs/cifs/smb1ops.c
> index 7d1c78b..b28aabd 100644
> --- a/fs/cifs/smb1ops.c
> +++ b/fs/cifs/smb1ops.c
> @@ -449,8 +449,7 @@ cifs_negotiate_wsize(struct cifs_tcon *tcon, struct smb_vol *volume_info)
>          * WRITEX header, not including the 4 byte RFC1001 length.
>          */
>         if (!(server->capabilities & CAP_LARGE_WRITE_X) ||
> -           (!(server->capabilities & CAP_UNIX) &&
> -            (server->sec_mode & (SECMODE_SIGN_ENABLED|SECMODE_SIGN_REQUIRED))))
> +           (!(server->capabilities & CAP_UNIX) && server->sign))
>                 wsize = min_t(unsigned int, wsize,
>                                 server->maxBuf - sizeof(WRITE_REQ) + 4);
>
> diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
> index 1609699..c753508 100644
> --- a/fs/cifs/smb2pdu.c
> +++ b/fs/cifs/smb2pdu.c
> @@ -119,8 +119,7 @@ smb2_hdr_assemble(struct smb2_hdr *hdr, __le16 smb2_cmd /* command */ ,
>         /* BB how does SMB2 do case sensitive? */
>         /* if (tcon->nocase)
>                 hdr->Flags |= SMBFLG_CASELESS; */
> -       if (tcon->ses && tcon->ses->server &&
> -           (tcon->ses->server->sec_mode & SECMODE_SIGN_REQUIRED))
> +       if (tcon->ses && tcon->ses->server && tcon->ses->server->sign)
>                 hdr->Flags |= SMB2_FLAGS_SIGNED;
>  out:
>         pdu->StructureSize2 = cpu_to_le16(parmsize);
> @@ -330,7 +329,6 @@ SMB2_negotiate(const unsigned int xid, struct cifs_ses *ses)
>         int resp_buftype;
>         struct TCP_Server_Info *server = ses->server;
>         unsigned int sec_flags;
> -       u16 temp = 0;
>         int blob_offset, blob_length;
>         char *security_blob;
>         int flags = CIFS_NEG_OP;
> @@ -362,12 +360,12 @@ SMB2_negotiate(const unsigned int xid, struct cifs_ses *ses)
>         inc_rfc1001_len(req, 2);
>
>         /* only one of SMB2 signing flags may be set in SMB2 request */
> -       if ((sec_flags & CIFSSEC_MUST_SIGN) == CIFSSEC_MUST_SIGN)
> -               temp = SMB2_NEGOTIATE_SIGNING_REQUIRED;
> -       else if (sec_flags & CIFSSEC_MAY_SIGN) /* MAY_SIGN is a single flag */
> -               temp = SMB2_NEGOTIATE_SIGNING_ENABLED;
> -
> -       req->SecurityMode = cpu_to_le16(temp);
> +       if (ses->sign)
> +               req->SecurityMode = SMB2_NEGOTIATE_SIGNING_REQUIRED;
> +       else if (global_secflags & CIFSSEC_MAY_SIGN)
> +               req->SecurityMode = SMB2_NEGOTIATE_SIGNING_ENABLED;
> +       else
> +               req->SecurityMode = 0;
>
>         req->Capabilities = cpu_to_le32(ses->server->vals->req_capabilities);
>
> @@ -424,8 +422,7 @@ SMB2_negotiate(const unsigned int xid, struct cifs_ses *ses)
>                 goto neg_exit;
>         }
>
> -       cifs_dbg(FYI, "sec_flags 0x%x\n", sec_flags);
> -       rc = cifs_enable_signing(server, sec_flags);
> +       rc = cifs_enable_signing(server, ses->sign);
>  #ifdef CONFIG_SMB2_ASN1  /* BB REMOVEME when updated asn1.c ready */
>         if (rc)
>                 goto neg_exit;
> @@ -457,7 +454,6 @@ SMB2_sess_setup(const unsigned int xid, struct cifs_ses *ses,
>         __le32 phase = NtLmNegotiate; /* NTLMSSP, if needed, is multistage */
>         struct TCP_Server_Info *server = ses->server;
>         unsigned int sec_flags;
> -       u8 temp = 0;
>         u16 blob_length = 0;
>         char *security_blob;
>         char *ntlmssp_blob = NULL;
> @@ -502,14 +498,13 @@ ssetup_ntlmssp_authenticate:
>         req->hdr.CreditRequest = cpu_to_le16(3);
>
>         /* only one of SMB2 signing flags may be set in SMB2 request */
> -       if ((sec_flags & CIFSSEC_MUST_SIGN) == CIFSSEC_MUST_SIGN)
> -               temp = SMB2_NEGOTIATE_SIGNING_REQUIRED;
> -       else if (ses->server->sec_mode & SMB2_NEGOTIATE_SIGNING_REQUIRED)
> -               temp = SMB2_NEGOTIATE_SIGNING_REQUIRED;
> -       else if (sec_flags & CIFSSEC_MAY_SIGN) /* MAY_SIGN is a single flag */
> -               temp = SMB2_NEGOTIATE_SIGNING_ENABLED;
> -
> -       req->SecurityMode = temp;
> +       if (server->sign)
> +               req->SecurityMode = SMB2_NEGOTIATE_SIGNING_REQUIRED;
> +       else if (global_secflags & CIFSSEC_MAY_SIGN) /* MAY_SIGN is a single flag */
> +               req->SecurityMode = SMB2_NEGOTIATE_SIGNING_ENABLED;
> +       else
> +               req->SecurityMode = 0;
> +
>         req->Capabilities = 0;
>         req->Channel = 0; /* MBZ */
>
> @@ -652,7 +647,7 @@ SMB2_logoff(const unsigned int xid, struct cifs_ses *ses)
>
>          /* since no tcon, smb2_init can not do this, so do here */
>         req->hdr.SessionId = ses->Suid;
> -       if (server->sec_mode & SECMODE_SIGN_REQUIRED)
> +       if (server->sign)
>                 req->hdr.Flags |= SMB2_FLAGS_SIGNED;
>
>         rc = SendReceiveNoRsp(xid, ses, (char *) &req->hdr, 0);
> @@ -1357,8 +1352,7 @@ smb2_readv_callback(struct mid_q_entry *mid)
>         case MID_RESPONSE_RECEIVED:
>                 credits_received = le16_to_cpu(buf->CreditRequest);
>                 /* result already set, check signature */
> -               if (server->sec_mode &
> -                   (SECMODE_SIGN_REQUIRED | SECMODE_SIGN_ENABLED)) {
> +               if (server->sign) {
>                         int rc;
>
>                         rc = smb2_verify_signature(&rqst, server);
> diff --git a/fs/cifs/smb2transport.c b/fs/cifs/smb2transport.c
> index 01f0ac8..c802ecf 100644
> --- a/fs/cifs/smb2transport.c
> +++ b/fs/cifs/smb2transport.c
> @@ -275,8 +275,7 @@ smb2_check_receive(struct mid_q_entry *mid, struct TCP_Server_Info *server,
>
>         dump_smb(mid->resp_buf, min_t(u32, 80, len));
>         /* convert the length into a more usable form */
> -       if ((len > 24) &&
> -           (server->sec_mode & (SECMODE_SIGN_REQUIRED|SECMODE_SIGN_ENABLED))) {
> +       if (len > 24 && server->sign) {
>                 int rc;
>
>                 rc = smb2_verify_signature(&rqst, server);
> diff --git a/fs/cifs/transport.c b/fs/cifs/transport.c
> index bfbf470..1996d6c 100644
> --- a/fs/cifs/transport.c
> +++ b/fs/cifs/transport.c
> @@ -463,7 +463,7 @@ cifs_setup_async_request(struct TCP_Server_Info *server, struct smb_rqst *rqst)
>         struct mid_q_entry *mid;
>
>         /* enable signing if server requires it */
> -       if (server->sec_mode & (SECMODE_SIGN_REQUIRED | SECMODE_SIGN_ENABLED))
> +       if (server->sign)
>                 hdr->Flags2 |= SMBFLG2_SECURITY_SIGNATURE;
>
>         mid = AllocMidQEntry(hdr, server);
> @@ -612,7 +612,7 @@ cifs_check_receive(struct mid_q_entry *mid, struct TCP_Server_Info *server,
>         dump_smb(mid->resp_buf, min_t(u32, 92, len));
>
>         /* convert the length into a more usable form */
> -       if (server->sec_mode & (SECMODE_SIGN_REQUIRED | SECMODE_SIGN_ENABLED)) {
> +       if (server->sign) {
>                 struct kvec iov;
>                 int rc = 0;
>                 struct smb_rqst rqst = { .rq_iov = &iov,
> --
> 1.8.1.4
>
> --
> 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

Reviewed-by: Pavel Shilovsky <piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org>

--
Best regards,
Pavel Shilovsky.

[PATCH 16/19] cifs: move sectype to the cifs_ses instead of TCP_Server_Info

[Jeff Layton]

Now that we track what sort of NEGOTIATE response was received, stop
mandating that every session on a socket use the same type of auth.

Push that decision out into the session setup code, and make the sectype
a per-session property. This should allow us to mix multiple sectypes on
a socket as long as they are compatible with the NEGOTIATE response.

With this too, we can now eliminate the ses->secFlg field since that
info is redundant and harder to work with than a securityEnum.

Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 fs/cifs/cifsencrypt.c |   4 +-
 fs/cifs/cifsglob.h    |   2 -
 fs/cifs/cifsproto.h   |   2 +
 fs/cifs/cifssmb.c     |  89 ++++++++------------------------------
 fs/cifs/connect.c     | 117 ++++++++++++++------------------------------------
 fs/cifs/sess.c        |  57 +++++++++++++++++++++++-
 fs/cifs/smb2pdu.c     |  21 +--------
 7 files changed, 114 insertions(+), 178 deletions(-)

diff --git a/fs/cifs/cifsencrypt.c b/fs/cifs/cifsencrypt.c
index a85a83d..30bea6b 100644
--- a/fs/cifs/cifsencrypt.c
+++ b/fs/cifs/cifsencrypt.c
@@ -535,7 +535,7 @@ CalcNTLMv2_response(const struct cifs_ses *ses, char *ntlmv2_hash)
 		return rc;
 	}
 
-	if (ses->server->secType == RawNTLMSSP)
+	if (ses->server->negflavor == CIFS_NEGFLAVOR_EXTENDED)
 		memcpy(ses->auth_key.response + offset,
 			ses->ntlmssp->cryptkey, CIFS_SERVER_CHALLENGE_SIZE);
 	else
@@ -567,7 +567,7 @@ setup_ntlmv2_rsp(struct cifs_ses *ses, const struct nls_table *nls_cp)
 	char ntlmv2_hash[16];
 	unsigned char *tiblob = NULL; /* target info blob */
 
-	if (ses->server->secType == RawNTLMSSP) {
+	if (ses->server->negflavor == CIFS_NEGFLAVOR_EXTENDED) {
 		if (!ses->domainName) {
 			rc = find_domain_name(ses, nls_cp);
 			if (rc) {
diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index f392ae4..65fd9dd 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -401,7 +401,6 @@ struct smb_vol {
 	kgid_t backupgid;
 	umode_t file_mode;
 	umode_t dir_mode;
-	unsigned secFlg;
 	enum securityEnum sectype; /* sectype requested via mnt opts */
 	bool sign; /* was signing requested via mnt opts? */
 	bool retry:1;
@@ -518,7 +517,6 @@ struct TCP_Server_Info {
 	bool echoes:1; /* enable echoes */
 #endif
 	u16 dialect; /* dialect index that server chose */
-	enum securityEnum secType;
 	bool oplocks:1; /* enable oplocks */
 	unsigned int maxReq;	/* Clients should submit no more */
 	/* than maxReq distinct unanswered SMBs to the server when using  */
diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
index ede010f..a82b3c0 100644
--- a/fs/cifs/cifsproto.h
+++ b/fs/cifs/cifsproto.h
@@ -118,6 +118,8 @@ extern void header_assemble(struct smb_hdr *, char /* command */ ,
 extern int small_smb_init_no_tc(const int smb_cmd, const int wct,
 				struct cifs_ses *ses,
 				void **request_buf);
+extern enum securityEnum select_sectype(struct TCP_Server_Info *server,
+				enum securityEnum requested);
 extern int CIFS_SessSetup(const unsigned int xid, struct cifs_ses *ses,
 			  const struct nls_table *nls_cp);
 extern struct timespec cifs_NTtimeToUnix(__le64 utc_nanoseconds_since_1601);
diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
index c118533..febe12c 100644
--- a/fs/cifs/cifssmb.c
+++ b/fs/cifs/cifssmb.c
@@ -368,11 +368,12 @@ vt2_err:
 }
 
 static int
-decode_ext_sec_blob(struct TCP_Server_Info *server, NEGOTIATE_RSP *pSMBr)
+decode_ext_sec_blob(struct cifs_ses *ses, NEGOTIATE_RSP *pSMBr)
 {
 	int	rc = 0;
 	u16	count;
 	char	*guid = pSMBr->u.extended_response.GUID;
+	struct TCP_Server_Info *server = ses->server;
 
 	count = get_bcc(&pSMBr->hdr);
 	if (count < SMB1_CLIENT_GUID_SIZE)
@@ -391,27 +392,13 @@ decode_ext_sec_blob(struct TCP_Server_Info *server, NEGOTIATE_RSP *pSMBr)
 	}
 
 	if (count == SMB1_CLIENT_GUID_SIZE) {
-		server->secType = RawNTLMSSP;
+		server->sec_ntlmssp = true;
 	} else {
 		count -= SMB1_CLIENT_GUID_SIZE;
 		rc = decode_negTokenInit(
 			pSMBr->u.extended_response.SecurityBlob, count, server);
 		if (rc != 1)
 			return -EINVAL;
-
-		/* Make sure server supports what we want to use */
-		switch(server->secType) {
-		case Kerberos:
-			if (!server->sec_kerberos && !server->sec_mskerberos)
-				return -EOPNOTSUPP;
-			break;
-		case RawNTLMSSP:
-			if (!server->sec_ntlmssp)
-				return -EOPNOTSUPP;
-			break;
-		default:
-			return -EOPNOTSUPP;
-		}
 	}
 
 	return 0;
@@ -446,8 +433,7 @@ cifs_enable_signing(struct TCP_Server_Info *server, bool mnt_sign_required)
 
 #ifdef CONFIG_CIFS_WEAK_PW_HASH
 static int
-decode_lanman_negprot_rsp(struct TCP_Server_Info *server, NEGOTIATE_RSP *pSMBr,
-			  unsigned int secFlags, bool sign)
+decode_lanman_negprot_rsp(struct TCP_Server_Info *server, NEGOTIATE_RSP *pSMBr, bool sign)
 {
 	__s16 tmp;
 	struct lanman_neg_rsp *rsp = (struct lanman_neg_rsp *)pSMBr;
@@ -455,12 +441,6 @@ decode_lanman_negprot_rsp(struct TCP_Server_Info *server, NEGOTIATE_RSP *pSMBr,
 	if (server->dialect != LANMAN_PROT && server->dialect != LANMAN2_PROT)
 		return -EOPNOTSUPP;
 
-	if ((secFlags & CIFSSEC_MAY_LANMAN) || (secFlags & CIFSSEC_MAY_PLNTXT))
-		server->secType = LANMAN;
-	else {
-		cifs_dbg(VFS, "mount failed weak security disabled in /proc/fs/cifs/SecurityFlags\n");
-		return -EOPNOTSUPP;
-	}
 	server->sec_mode = le16_to_cpu(rsp->SecurityMode);
 	server->maxReq = min_t(unsigned int,
 			       le16_to_cpu(rsp->MaxMpxCount),
@@ -535,17 +515,20 @@ decode_lanman_negprot_rsp(struct TCP_Server_Info *server, NEGOTIATE_RSP *pSMBr,
 #endif
 
 static bool
-should_set_ext_sec_flag(unsigned int secFlags)
+should_set_ext_sec_flag(enum securityEnum sectype)
 {
-	if ((secFlags & CIFSSEC_MUST_KRB5) == CIFSSEC_MUST_KRB5)
-		return true;
-	else if ((secFlags & CIFSSEC_AUTH_MASK) == CIFSSEC_MAY_KRB5)
-		return true;
-	else if ((secFlags & CIFSSEC_MUST_NTLMSSP) == CIFSSEC_MUST_NTLMSSP)
+	switch (sectype) {
+	case RawNTLMSSP:
+	case Kerberos:
 		return true;
-	else if ((secFlags & CIFSSEC_AUTH_MASK) == CIFSSEC_MAY_NTLMSSP)
-		return true;
-	return false;
+	case Unspecified:
+		if (global_secflags &
+		    (CIFSSEC_MAY_KRB5 | CIFSSEC_MAY_NTLMSSP))
+			return true;
+		/* Fallthrough */
+	default:
+		return false;
+	}
 }
 
 int
@@ -558,7 +541,6 @@ CIFSSMBNegotiate(const unsigned int xid, struct cifs_ses *ses)
 	int i;
 	struct TCP_Server_Info *server = ses->server;
 	u16 count;
-	unsigned int secFlags;
 
 	if (!server) {
 		WARN(1, "%s: server is NULL!\n", __func__);
@@ -570,18 +552,10 @@ CIFSSMBNegotiate(const unsigned int xid, struct cifs_ses *ses)
 	if (rc)
 		return rc;
 
-	/* if any of auth flags (ie not sign or seal) are overriden use them */
-	if (ses->overrideSecFlg & (~(CIFSSEC_MUST_SIGN | CIFSSEC_MUST_SEAL)))
-		secFlags = ses->overrideSecFlg;  /* BB FIXME fix sign flags? */
-	else /* if override flags set only sign/seal OR them with global auth */
-		secFlags = global_secflags | ses->overrideSecFlg;
-
-	cifs_dbg(FYI, "secFlags 0x%x\n", secFlags);
-
 	pSMB->hdr.Mid = get_next_mid(server);
 	pSMB->hdr.Flags2 |= (SMBFLG2_UNICODE | SMBFLG2_ERR_STATUS);
 
-	if (should_set_ext_sec_flag(secFlags)) {
+	if (should_set_ext_sec_flag(ses->sectype)) {
 		cifs_dbg(FYI, "Requesting extended security.");
 		pSMB->hdr.Flags2 |= SMBFLG2_EXT_SEC;
 	}
@@ -610,7 +584,7 @@ CIFSSMBNegotiate(const unsigned int xid, struct cifs_ses *ses)
 		rc = -EOPNOTSUPP;
 		goto neg_err_exit;
 	} else if (pSMBr->hdr.WordCount == 13) {
-		rc = decode_lanman_negprot_rsp(server, pSMBr, secFlags, ses->sign);
+		rc = decode_lanman_negprot_rsp(server, pSMBr, ses->sign);
 		server->negflavor = CIFS_NEGFLAVOR_LANMAN;
 		goto neg_err_exit;
 	} else if (pSMBr->hdr.WordCount != 17) {
@@ -624,31 +598,6 @@ CIFSSMBNegotiate(const unsigned int xid, struct cifs_ses *ses)
 	if ((server->sec_mode & SECMODE_USER) == 0)
 		cifs_dbg(FYI, "share mode security\n");
 
-	if ((server->sec_mode & SECMODE_PW_ENCRYPT) == 0)
-#ifdef CONFIG_CIFS_WEAK_PW_HASH
-		if ((secFlags & CIFSSEC_MAY_PLNTXT) == 0)
-#endif /* CIFS_WEAK_PW_HASH */
-			cifs_dbg(VFS, "Server requests plain text password but client support disabled\n");
-
-	if ((secFlags & CIFSSEC_MUST_NTLMV2) == CIFSSEC_MUST_NTLMV2)
-		server->secType = NTLMv2;
-	else if (secFlags & CIFSSEC_MAY_NTLM)
-		server->secType = NTLM;
-	else if (secFlags & CIFSSEC_MAY_NTLMV2)
-		server->secType = NTLMv2;
-	else if (secFlags & CIFSSEC_MAY_KRB5)
-		server->secType = Kerberos;
-	else if (secFlags & CIFSSEC_MAY_NTLMSSP)
-		server->secType = RawNTLMSSP;
-	else if (secFlags & CIFSSEC_MAY_LANMAN)
-		server->secType = LANMAN;
-	else {
-		rc = -EOPNOTSUPP;
-		cifs_dbg(VFS, "Invalid security type\n");
-		goto neg_err_exit;
-	}
-	/* else ... any others ...? */
-
 	/* one byte, so no need to convert this or EncryptionKeyLen from
 	   little endian */
 	server->maxReq = min_t(unsigned int, le16_to_cpu(pSMBr->MaxMpxCount),
@@ -670,7 +619,7 @@ CIFSSMBNegotiate(const unsigned int xid, struct cifs_ses *ses)
 			server->capabilities & CAP_EXTENDED_SECURITY) &&
 				(pSMBr->EncryptionKeyLength == 0)) {
 		server->negflavor = CIFS_NEGFLAVOR_EXTENDED;
-		rc = decode_ext_sec_blob(server, pSMBr);
+		rc = decode_ext_sec_blob(ses, pSMBr);
 	} else if (server->sec_mode & SECMODE_PW_ENCRYPT) {
 		rc = -EIO; /* no crypt key only if plain text pwd */
 	} else {
diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index f5ea895..a57715b 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -1032,56 +1032,40 @@ static int cifs_parse_security_flavors(char *value,
 	vol->sign = false;
 
 	switch (match_token(value, cifs_secflavor_tokens, args)) {
+	case Opt_sec_krb5p:
+		cifs_dbg(VFS, "sec=krb5p is not supported!\n");
+		return 1;
+	case Opt_sec_krb5i:
+		vol->sign = true;
+		/* Fallthrough */
 	case Opt_sec_krb5:
 		vol->sectype = Kerberos;
-		vol->secFlg |= CIFSSEC_MAY_KRB5 | CIFSSEC_MAY_SIGN;
 		break;
-	case Opt_sec_krb5i:
-		vol->sectype = Kerberos;
+	case Opt_sec_ntlmsspi:
 		vol->sign = true;
-		vol->secFlg |= CIFSSEC_MAY_KRB5 | CIFSSEC_MUST_SIGN;
-		break;
-	case Opt_sec_krb5p:
-		/* vol->secFlg |= CIFSSEC_MUST_SEAL | CIFSSEC_MAY_KRB5; */
-		cifs_dbg(VFS, "sec=krb5p is not supported!\n");
-		return 1;
+		/* Fallthrough */
 	case Opt_sec_ntlmssp:
 		vol->sectype = RawNTLMSSP;
-		vol->secFlg |= CIFSSEC_MAY_NTLMSSP;
 		break;
-	case Opt_sec_ntlmsspi:
-		vol->sectype = RawNTLMSSP;
+	case Opt_sec_ntlmi:
 		vol->sign = true;
-		vol->secFlg |= CIFSSEC_MAY_NTLMSSP | CIFSSEC_MUST_SIGN;
-		break;
+		/* Fallthrough */
 	case Opt_ntlm:
-		/* ntlm is default so can be turned off too */
 		vol->sectype = NTLM;
-		vol->secFlg |= CIFSSEC_MAY_NTLM;
 		break;
-	case Opt_sec_ntlmi:
-		vol->sectype = NTLM;
+	case Opt_sec_ntlmv2i:
 		vol->sign = true;
-		vol->secFlg |= CIFSSEC_MAY_NTLM | CIFSSEC_MUST_SIGN;
-		break;
+		/* Fallthrough */
 	case Opt_sec_ntlmv2:
 		vol->sectype = NTLMv2;
-		vol->secFlg |= CIFSSEC_MAY_NTLMV2;
-		break;
-	case Opt_sec_ntlmv2i:
-		vol->sectype = NTLMv2;
-		vol->sign = true;
-		vol->secFlg |= CIFSSEC_MAY_NTLMV2 | CIFSSEC_MUST_SIGN;
 		break;
 #ifdef CONFIG_CIFS_WEAK_PW_HASH
 	case Opt_sec_lanman:
 		vol->sectype = LANMAN;
-		vol->secFlg |= CIFSSEC_MAY_LANMAN;
 		break;
 #endif
 	case Opt_sec_none:
 		vol->nullauth = 1;
-		vol->secFlg |= CIFSSEC_MAY_NTLM;
 		break;
 	default:
 		cifs_dbg(VFS, "bad security option: %s\n", value);
@@ -1444,7 +1428,6 @@ cifs_parse_mount_options(const char *mountdata, const char *devname,
 			vol->local_lease = 1;
 			break;
 		case Opt_sign:
-			vol->secFlg |= CIFSSEC_MUST_SIGN;
 			vol->sign = true;
 			break;
 		case Opt_noac:
@@ -1994,40 +1977,19 @@ match_address(struct TCP_Server_Info *server, struct sockaddr *addr,
 static bool
 match_security(struct TCP_Server_Info *server, struct smb_vol *vol)
 {
-	unsigned int secFlags;
-
-	if (vol->secFlg & (~(CIFSSEC_MUST_SIGN | CIFSSEC_MUST_SEAL)))
-		secFlags = vol->secFlg;
-	else
-		secFlags = global_secflags | vol->secFlg;
-
-	switch (server->secType) {
-	case LANMAN:
-		if (!(secFlags & (CIFSSEC_MAY_LANMAN|CIFSSEC_MAY_PLNTXT)))
-			return false;
-		break;
-	case NTLMv2:
-		if (!(secFlags & CIFSSEC_MAY_NTLMV2))
-			return false;
-		break;
-	case NTLM:
-		if (!(secFlags & CIFSSEC_MAY_NTLM))
-			return false;
-		break;
-	case Kerberos:
-		if (!(secFlags & CIFSSEC_MAY_KRB5))
-			return false;
-		break;
-	case RawNTLMSSP:
-		if (!(secFlags & CIFSSEC_MAY_NTLMSSP))
-			return false;
-		break;
-	default:
-		/* shouldn't happen */
+	/*
+	 * The select_sectype function should either return the vol->sectype
+	 * that was specified, or "Unspecified" if that sectype was not
+	 * compatible with the given NEGOTIATE request.
+	 */
+	if (select_sectype(server, vol->sectype) == Unspecified)
 		return false;
-	}
 
-	/* now check if signing mode is acceptable */
+	/*
+	 * Now check if signing mode is acceptable. No need to check
+	 * global_secflags at this point since if MUST_SIGN is set then
+	 * the server->sign had better be too.
+	 */
 	if (vol->sign && !server->sign)
 		return false;
 
@@ -2230,7 +2192,11 @@ out_err:
 
 static int match_session(struct cifs_ses *ses, struct smb_vol *vol)
 {
-	switch (ses->server->secType) {
+	if (vol->sectype != Unspecified &&
+	    vol->sectype != ses->sectype)
+		return 0;
+
+	switch (ses->sectype) {
 	case Kerberos:
 		if (!uid_eq(vol->cred_uid, ses->cred_uid))
 			return 0;
@@ -2507,7 +2473,6 @@ cifs_get_smb_ses(struct TCP_Server_Info *server, struct smb_vol *volume_info)
 	ses->cred_uid = volume_info->cred_uid;
 	ses->linux_uid = volume_info->linux_uid;
 
-	ses->overrideSecFlg = volume_info->secFlg;
 	ses->sectype = volume_info->sectype;
 	ses->sign = volume_info->sign ? volume_info->sign :
 				(global_secflags & CIFSSEC_MUST_SIGN);
@@ -3670,7 +3635,7 @@ CIFSTCon(const unsigned int xid, struct cifs_ses *ses,
 		   NTLMv2 password here) */
 #ifdef CONFIG_CIFS_WEAK_PW_HASH
 		if ((global_secflags & CIFSSEC_MAY_LANMAN) &&
-		    (ses->server->secType == LANMAN))
+		    (ses->sectype == LANMAN))
 			calc_lanman_hash(tcon->password, ses->server->cryptkey,
 					 ses->server->sec_mode &
 					    SECMODE_PW_ENCRYPT ? true : false,
@@ -3882,27 +3847,11 @@ cifs_setup_session(const unsigned int xid, struct cifs_ses *ses,
 static int
 cifs_set_vol_auth(struct smb_vol *vol, struct cifs_ses *ses)
 {
-	switch (ses->server->secType) {
-	case Kerberos:
-		vol->secFlg = CIFSSEC_MUST_KRB5;
+	vol->sectype = ses->sectype;
+
+	/* krb5 is special, since we don't need username or pw */
+	if (vol->sectype == Kerberos)
 		return 0;
-	case NTLMv2:
-		vol->secFlg = CIFSSEC_MUST_NTLMV2;
-		break;
-	case NTLM:
-		vol->secFlg = CIFSSEC_MUST_NTLM;
-		break;
-	case RawNTLMSSP:
-		vol->secFlg = CIFSSEC_MUST_NTLMSSP;
-		break;
-	case LANMAN:
-		vol->secFlg = CIFSSEC_MUST_LANMAN;
-		break;
-	default:
-		/* should never happen */
-		vol->secFlg = 0;
-		break;
-	}
 
 	return cifs_set_cifscreds(vol, ses);
 }
diff --git a/fs/cifs/sess.c b/fs/cifs/sess.c
index 82b784a..79358e3 100644
--- a/fs/cifs/sess.c
+++ b/fs/cifs/sess.c
@@ -550,6 +550,56 @@ setup_ntlmv2_ret:
 	return rc;
 }
 
+enum securityEnum
+select_sectype(struct TCP_Server_Info *server, enum securityEnum requested)
+{
+	switch (server->negflavor) {
+	case CIFS_NEGFLAVOR_EXTENDED:
+		switch (requested) {
+		case Kerberos:
+		case RawNTLMSSP:
+			return requested;
+		case Unspecified:
+			if (server->sec_ntlmssp &&
+			    (global_secflags & CIFSSEC_MAY_NTLMSSP))
+				return RawNTLMSSP;
+			if ((server->sec_kerberos || server->sec_mskerberos) &&
+			    (global_secflags & CIFSSEC_MAY_KRB5))
+				return Kerberos;
+			/* Fallthrough */
+		default:
+			return Unspecified;
+		}
+	case CIFS_NEGFLAVOR_UNENCAP:
+		switch (requested) {
+		case NTLM:
+		case NTLMv2:
+			return requested;
+		case Unspecified:
+			if (global_secflags & CIFSSEC_MAY_NTLMV2)
+				return NTLMv2;
+			if (global_secflags & CIFSSEC_MAY_NTLM)
+				return NTLM;
+			/* Fallthrough */
+		default:
+			return Unspecified;
+		}
+	case CIFS_NEGFLAVOR_LANMAN:
+		switch (requested) {
+		case LANMAN:
+			return requested;
+		case Unspecified:
+			if (global_secflags & CIFSSEC_MAY_LANMAN)
+				return LANMAN;
+			/* Fallthrough */
+		default:
+			return Unspecified;
+		}
+	default:
+		return Unspecified;
+	}
+}
+
 int
 CIFS_SessSetup(const unsigned int xid, struct cifs_ses *ses,
 	       const struct nls_table *nls_cp)
@@ -576,8 +626,13 @@ CIFS_SessSetup(const unsigned int xid, struct cifs_ses *ses,
 		return -EINVAL;
 	}
 
-	type = ses->server->secType;
+	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;
+	}
+
 	if (type == RawNTLMSSP) {
 		/* if memory allocation is successful, caller of this function
 		 * frees it.
diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
index c753508..d5f2bf0 100644
--- a/fs/cifs/smb2pdu.c
+++ b/fs/cifs/smb2pdu.c
@@ -328,7 +328,6 @@ SMB2_negotiate(const unsigned int xid, struct cifs_ses *ses)
 	int rc = 0;
 	int resp_buftype;
 	struct TCP_Server_Info *server = ses->server;
-	unsigned int sec_flags;
 	int blob_offset, blob_length;
 	char *security_blob;
 	int flags = CIFS_NEG_OP;
@@ -344,14 +343,6 @@ SMB2_negotiate(const unsigned int xid, struct cifs_ses *ses)
 	if (rc)
 		return rc;
 
-	/* if any of auth flags (ie not sign or seal) are overriden use them */
-	if (ses->overrideSecFlg & (~(CIFSSEC_MUST_SIGN | CIFSSEC_MUST_SEAL)))
-		sec_flags = ses->overrideSecFlg;  /* BB FIXME fix sign flags?*/
-	else /* if override flags set only sign/seal OR them with global auth */
-		sec_flags = global_secflags | ses->overrideSecFlg;
-
-	cifs_dbg(FYI, "sec_flags 0x%x\n", sec_flags);
-
 	req->hdr.SessionId = 0;
 
 	req->Dialects[0] = cpu_to_le16(ses->server->vals->protocol_id);
@@ -453,7 +444,6 @@ SMB2_sess_setup(const unsigned int xid, struct cifs_ses *ses,
 	int resp_buftype;
 	__le32 phase = NtLmNegotiate; /* NTLMSSP, if needed, is multistage */
 	struct TCP_Server_Info *server = ses->server;
-	unsigned int sec_flags;
 	u16 blob_length = 0;
 	char *security_blob;
 	char *ntlmssp_blob = NULL;
@@ -474,7 +464,8 @@ SMB2_sess_setup(const unsigned int xid, struct cifs_ses *ses,
 	if (!ses->ntlmssp)
 		return -ENOMEM;
 
-	ses->server->secType = RawNTLMSSP;
+	/* FIXME: allow for other auth types besides NTLMSSP (e.g. krb5) */
+	ses->sectype = RawNTLMSSP;
 
 ssetup_ntlmssp_authenticate:
 	if (phase == NtLmChallenge)
@@ -484,14 +475,6 @@ ssetup_ntlmssp_authenticate:
 	if (rc)
 		return rc;
 
-	/* if any of auth flags (ie not sign or seal) are overriden use them */
-	if (ses->overrideSecFlg & (~(CIFSSEC_MUST_SIGN | CIFSSEC_MUST_SEAL)))
-		sec_flags = ses->overrideSecFlg;  /* BB FIXME fix sign flags?*/
-	else /* if override flags set only sign/seal OR them with global auth */
-		sec_flags = global_secflags | ses->overrideSecFlg;
-
-	cifs_dbg(FYI, "sec_flags 0x%x\n", sec_flags);
-
 	req->hdr.SessionId = 0; /* First session, not a reauthenticate */
 	req->VcNumber = 0; /* MBZ */
 	/* to enable echos and oplocks */
-- 
1.8.1.4

Re: [PATCH 16/19] cifs: move sectype to the cifs_ses instead of TCP_Server_Info

[Pavel Shilovsky]

2013/5/23 Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>:
> Now that we track what sort of NEGOTIATE response was received, stop
> mandating that every session on a socket use the same type of auth.
>
> Push that decision out into the session setup code, and make the sectype
> a per-session property. This should allow us to mix multiple sectypes on
> a socket as long as they are compatible with the NEGOTIATE response.
>
> With this too, we can now eliminate the ses->secFlg field since that
> info is redundant and harder to work with than a securityEnum.
>
> Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
>  fs/cifs/cifsencrypt.c |   4 +-
>  fs/cifs/cifsglob.h    |   2 -
>  fs/cifs/cifsproto.h   |   2 +
>  fs/cifs/cifssmb.c     |  89 ++++++++------------------------------
>  fs/cifs/connect.c     | 117 ++++++++++++++------------------------------------
>  fs/cifs/sess.c        |  57 +++++++++++++++++++++++-
>  fs/cifs/smb2pdu.c     |  21 +--------
>  7 files changed, 114 insertions(+), 178 deletions(-)
>
> diff --git a/fs/cifs/cifsencrypt.c b/fs/cifs/cifsencrypt.c
> index a85a83d..30bea6b 100644
> --- a/fs/cifs/cifsencrypt.c
> +++ b/fs/cifs/cifsencrypt.c
> @@ -535,7 +535,7 @@ CalcNTLMv2_response(const struct cifs_ses *ses, char *ntlmv2_hash)
>                 return rc;
>         }
>
> -       if (ses->server->secType == RawNTLMSSP)
> +       if (ses->server->negflavor == CIFS_NEGFLAVOR_EXTENDED)
>                 memcpy(ses->auth_key.response + offset,
>                         ses->ntlmssp->cryptkey, CIFS_SERVER_CHALLENGE_SIZE);
>         else
> @@ -567,7 +567,7 @@ setup_ntlmv2_rsp(struct cifs_ses *ses, const struct nls_table *nls_cp)
>         char ntlmv2_hash[16];
>         unsigned char *tiblob = NULL; /* target info blob */
>
> -       if (ses->server->secType == RawNTLMSSP) {
> +       if (ses->server->negflavor == CIFS_NEGFLAVOR_EXTENDED) {
>                 if (!ses->domainName) {
>                         rc = find_domain_name(ses, nls_cp);
>                         if (rc) {
> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
> index f392ae4..65fd9dd 100644
> --- a/fs/cifs/cifsglob.h
> +++ b/fs/cifs/cifsglob.h
> @@ -401,7 +401,6 @@ struct smb_vol {
>         kgid_t backupgid;
>         umode_t file_mode;
>         umode_t dir_mode;
> -       unsigned secFlg;
>         enum securityEnum sectype; /* sectype requested via mnt opts */
>         bool sign; /* was signing requested via mnt opts? */
>         bool retry:1;
> @@ -518,7 +517,6 @@ struct TCP_Server_Info {
>         bool echoes:1; /* enable echoes */
>  #endif
>         u16 dialect; /* dialect index that server chose */
> -       enum securityEnum secType;
>         bool oplocks:1; /* enable oplocks */
>         unsigned int maxReq;    /* Clients should submit no more */
>         /* than maxReq distinct unanswered SMBs to the server when using  */
> diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
> index ede010f..a82b3c0 100644
> --- a/fs/cifs/cifsproto.h
> +++ b/fs/cifs/cifsproto.h
> @@ -118,6 +118,8 @@ extern void header_assemble(struct smb_hdr *, char /* command */ ,
>  extern int small_smb_init_no_tc(const int smb_cmd, const int wct,
>                                 struct cifs_ses *ses,
>                                 void **request_buf);
> +extern enum securityEnum select_sectype(struct TCP_Server_Info *server,
> +                               enum securityEnum requested);
>  extern int CIFS_SessSetup(const unsigned int xid, struct cifs_ses *ses,
>                           const struct nls_table *nls_cp);
>  extern struct timespec cifs_NTtimeToUnix(__le64 utc_nanoseconds_since_1601);
> diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
> index c118533..febe12c 100644
> --- a/fs/cifs/cifssmb.c
> +++ b/fs/cifs/cifssmb.c
> @@ -368,11 +368,12 @@ vt2_err:
>  }
>
>  static int
> -decode_ext_sec_blob(struct TCP_Server_Info *server, NEGOTIATE_RSP *pSMBr)
> +decode_ext_sec_blob(struct cifs_ses *ses, NEGOTIATE_RSP *pSMBr)
>  {
>         int     rc = 0;
>         u16     count;
>         char    *guid = pSMBr->u.extended_response.GUID;
> +       struct TCP_Server_Info *server = ses->server;
>
>         count = get_bcc(&pSMBr->hdr);
>         if (count < SMB1_CLIENT_GUID_SIZE)
> @@ -391,27 +392,13 @@ decode_ext_sec_blob(struct TCP_Server_Info *server, NEGOTIATE_RSP *pSMBr)
>         }
>
>         if (count == SMB1_CLIENT_GUID_SIZE) {
> -               server->secType = RawNTLMSSP;
> +               server->sec_ntlmssp = true;
>         } else {
>                 count -= SMB1_CLIENT_GUID_SIZE;
>                 rc = decode_negTokenInit(
>                         pSMBr->u.extended_response.SecurityBlob, count, server);
>                 if (rc != 1)
>                         return -EINVAL;
> -
> -               /* Make sure server supports what we want to use */
> -               switch(server->secType) {
> -               case Kerberos:
> -                       if (!server->sec_kerberos && !server->sec_mskerberos)
> -                               return -EOPNOTSUPP;
> -                       break;
> -               case RawNTLMSSP:
> -                       if (!server->sec_ntlmssp)
> -                               return -EOPNOTSUPP;
> -                       break;
> -               default:
> -                       return -EOPNOTSUPP;
> -               }
>         }
>
>         return 0;
> @@ -446,8 +433,7 @@ cifs_enable_signing(struct TCP_Server_Info *server, bool mnt_sign_required)
>
>  #ifdef CONFIG_CIFS_WEAK_PW_HASH
>  static int
> -decode_lanman_negprot_rsp(struct TCP_Server_Info *server, NEGOTIATE_RSP *pSMBr,
> -                         unsigned int secFlags, bool sign)
> +decode_lanman_negprot_rsp(struct TCP_Server_Info *server, NEGOTIATE_RSP *pSMBr, bool sign)
>  {
>         __s16 tmp;
>         struct lanman_neg_rsp *rsp = (struct lanman_neg_rsp *)pSMBr;
> @@ -455,12 +441,6 @@ decode_lanman_negprot_rsp(struct TCP_Server_Info *server, NEGOTIATE_RSP *pSMBr,
>         if (server->dialect != LANMAN_PROT && server->dialect != LANMAN2_PROT)
>                 return -EOPNOTSUPP;
>
> -       if ((secFlags & CIFSSEC_MAY_LANMAN) || (secFlags & CIFSSEC_MAY_PLNTXT))
> -               server->secType = LANMAN;
> -       else {
> -               cifs_dbg(VFS, "mount failed weak security disabled in /proc/fs/cifs/SecurityFlags\n");
> -               return -EOPNOTSUPP;
> -       }
>         server->sec_mode = le16_to_cpu(rsp->SecurityMode);
>         server->maxReq = min_t(unsigned int,
>                                le16_to_cpu(rsp->MaxMpxCount),
> @@ -535,17 +515,20 @@ decode_lanman_negprot_rsp(struct TCP_Server_Info *server, NEGOTIATE_RSP *pSMBr,
>  #endif
>
>  static bool
> -should_set_ext_sec_flag(unsigned int secFlags)
> +should_set_ext_sec_flag(enum securityEnum sectype)
>  {
> -       if ((secFlags & CIFSSEC_MUST_KRB5) == CIFSSEC_MUST_KRB5)
> -               return true;
> -       else if ((secFlags & CIFSSEC_AUTH_MASK) == CIFSSEC_MAY_KRB5)
> -               return true;
> -       else if ((secFlags & CIFSSEC_MUST_NTLMSSP) == CIFSSEC_MUST_NTLMSSP)
> +       switch (sectype) {
> +       case RawNTLMSSP:
> +       case Kerberos:
>                 return true;
> -       else if ((secFlags & CIFSSEC_AUTH_MASK) == CIFSSEC_MAY_NTLMSSP)
> -               return true;
> -       return false;
> +       case Unspecified:
> +               if (global_secflags &
> +                   (CIFSSEC_MAY_KRB5 | CIFSSEC_MAY_NTLMSSP))
> +                       return true;
> +               /* Fallthrough */
> +       default:
> +               return false;
> +       }
>  }
>
>  int
> @@ -558,7 +541,6 @@ CIFSSMBNegotiate(const unsigned int xid, struct cifs_ses *ses)
>         int i;
>         struct TCP_Server_Info *server = ses->server;
>         u16 count;
> -       unsigned int secFlags;
>
>         if (!server) {
>                 WARN(1, "%s: server is NULL!\n", __func__);
> @@ -570,18 +552,10 @@ CIFSSMBNegotiate(const unsigned int xid, struct cifs_ses *ses)
>         if (rc)
>                 return rc;
>
> -       /* if any of auth flags (ie not sign or seal) are overriden use them */
> -       if (ses->overrideSecFlg & (~(CIFSSEC_MUST_SIGN | CIFSSEC_MUST_SEAL)))
> -               secFlags = ses->overrideSecFlg;  /* BB FIXME fix sign flags? */
> -       else /* if override flags set only sign/seal OR them with global auth */
> -               secFlags = global_secflags | ses->overrideSecFlg;
> -
> -       cifs_dbg(FYI, "secFlags 0x%x\n", secFlags);
> -
>         pSMB->hdr.Mid = get_next_mid(server);
>         pSMB->hdr.Flags2 |= (SMBFLG2_UNICODE | SMBFLG2_ERR_STATUS);
>
> -       if (should_set_ext_sec_flag(secFlags)) {
> +       if (should_set_ext_sec_flag(ses->sectype)) {
>                 cifs_dbg(FYI, "Requesting extended security.");
>                 pSMB->hdr.Flags2 |= SMBFLG2_EXT_SEC;
>         }
> @@ -610,7 +584,7 @@ CIFSSMBNegotiate(const unsigned int xid, struct cifs_ses *ses)
>                 rc = -EOPNOTSUPP;
>                 goto neg_err_exit;
>         } else if (pSMBr->hdr.WordCount == 13) {
> -               rc = decode_lanman_negprot_rsp(server, pSMBr, secFlags, ses->sign);
> +               rc = decode_lanman_negprot_rsp(server, pSMBr, ses->sign);
>                 server->negflavor = CIFS_NEGFLAVOR_LANMAN;
>                 goto neg_err_exit;
>         } else if (pSMBr->hdr.WordCount != 17) {
> @@ -624,31 +598,6 @@ CIFSSMBNegotiate(const unsigned int xid, struct cifs_ses *ses)
>         if ((server->sec_mode & SECMODE_USER) == 0)
>                 cifs_dbg(FYI, "share mode security\n");
>
> -       if ((server->sec_mode & SECMODE_PW_ENCRYPT) == 0)
> -#ifdef CONFIG_CIFS_WEAK_PW_HASH
> -               if ((secFlags & CIFSSEC_MAY_PLNTXT) == 0)
> -#endif /* CIFS_WEAK_PW_HASH */
> -                       cifs_dbg(VFS, "Server requests plain text password but client support disabled\n");
> -
> -       if ((secFlags & CIFSSEC_MUST_NTLMV2) == CIFSSEC_MUST_NTLMV2)
> -               server->secType = NTLMv2;
> -       else if (secFlags & CIFSSEC_MAY_NTLM)
> -               server->secType = NTLM;
> -       else if (secFlags & CIFSSEC_MAY_NTLMV2)
> -               server->secType = NTLMv2;
> -       else if (secFlags & CIFSSEC_MAY_KRB5)
> -               server->secType = Kerberos;
> -       else if (secFlags & CIFSSEC_MAY_NTLMSSP)
> -               server->secType = RawNTLMSSP;
> -       else if (secFlags & CIFSSEC_MAY_LANMAN)
> -               server->secType = LANMAN;
> -       else {
> -               rc = -EOPNOTSUPP;
> -               cifs_dbg(VFS, "Invalid security type\n");
> -               goto neg_err_exit;
> -       }
> -       /* else ... any others ...? */
> -
>         /* one byte, so no need to convert this or EncryptionKeyLen from
>            little endian */
>         server->maxReq = min_t(unsigned int, le16_to_cpu(pSMBr->MaxMpxCount),
> @@ -670,7 +619,7 @@ CIFSSMBNegotiate(const unsigned int xid, struct cifs_ses *ses)
>                         server->capabilities & CAP_EXTENDED_SECURITY) &&
>                                 (pSMBr->EncryptionKeyLength == 0)) {
>                 server->negflavor = CIFS_NEGFLAVOR_EXTENDED;
> -               rc = decode_ext_sec_blob(server, pSMBr);
> +               rc = decode_ext_sec_blob(ses, pSMBr);
>         } else if (server->sec_mode & SECMODE_PW_ENCRYPT) {
>                 rc = -EIO; /* no crypt key only if plain text pwd */
>         } else {
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index f5ea895..a57715b 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -1032,56 +1032,40 @@ static int cifs_parse_security_flavors(char *value,
>         vol->sign = false;
>
>         switch (match_token(value, cifs_secflavor_tokens, args)) {
> +       case Opt_sec_krb5p:
> +               cifs_dbg(VFS, "sec=krb5p is not supported!\n");
> +               return 1;
> +       case Opt_sec_krb5i:
> +               vol->sign = true;
> +               /* Fallthrough */
>         case Opt_sec_krb5:
>                 vol->sectype = Kerberos;
> -               vol->secFlg |= CIFSSEC_MAY_KRB5 | CIFSSEC_MAY_SIGN;
>                 break;
> -       case Opt_sec_krb5i:
> -               vol->sectype = Kerberos;
> +       case Opt_sec_ntlmsspi:
>                 vol->sign = true;
> -               vol->secFlg |= CIFSSEC_MAY_KRB5 | CIFSSEC_MUST_SIGN;
> -               break;
> -       case Opt_sec_krb5p:
> -               /* vol->secFlg |= CIFSSEC_MUST_SEAL | CIFSSEC_MAY_KRB5; */
> -               cifs_dbg(VFS, "sec=krb5p is not supported!\n");
> -               return 1;
> +               /* Fallthrough */
>         case Opt_sec_ntlmssp:
>                 vol->sectype = RawNTLMSSP;
> -               vol->secFlg |= CIFSSEC_MAY_NTLMSSP;
>                 break;
> -       case Opt_sec_ntlmsspi:
> -               vol->sectype = RawNTLMSSP;
> +       case Opt_sec_ntlmi:
>                 vol->sign = true;
> -               vol->secFlg |= CIFSSEC_MAY_NTLMSSP | CIFSSEC_MUST_SIGN;
> -               break;
> +               /* Fallthrough */
>         case Opt_ntlm:
> -               /* ntlm is default so can be turned off too */
>                 vol->sectype = NTLM;
> -               vol->secFlg |= CIFSSEC_MAY_NTLM;
>                 break;
> -       case Opt_sec_ntlmi:
> -               vol->sectype = NTLM;
> +       case Opt_sec_ntlmv2i:
>                 vol->sign = true;
> -               vol->secFlg |= CIFSSEC_MAY_NTLM | CIFSSEC_MUST_SIGN;
> -               break;
> +               /* Fallthrough */
>         case Opt_sec_ntlmv2:
>                 vol->sectype = NTLMv2;
> -               vol->secFlg |= CIFSSEC_MAY_NTLMV2;
> -               break;
> -       case Opt_sec_ntlmv2i:
> -               vol->sectype = NTLMv2;
> -               vol->sign = true;
> -               vol->secFlg |= CIFSSEC_MAY_NTLMV2 | CIFSSEC_MUST_SIGN;
>                 break;
>  #ifdef CONFIG_CIFS_WEAK_PW_HASH
>         case Opt_sec_lanman:
>                 vol->sectype = LANMAN;
> -               vol->secFlg |= CIFSSEC_MAY_LANMAN;
>                 break;
>  #endif
>         case Opt_sec_none:
>                 vol->nullauth = 1;
> -               vol->secFlg |= CIFSSEC_MAY_NTLM;
>                 break;
>         default:
>                 cifs_dbg(VFS, "bad security option: %s\n", value);
> @@ -1444,7 +1428,6 @@ cifs_parse_mount_options(const char *mountdata, const char *devname,
>                         vol->local_lease = 1;
>                         break;
>                 case Opt_sign:
> -                       vol->secFlg |= CIFSSEC_MUST_SIGN;
>                         vol->sign = true;
>                         break;
>                 case Opt_noac:
> @@ -1994,40 +1977,19 @@ match_address(struct TCP_Server_Info *server, struct sockaddr *addr,
>  static bool
>  match_security(struct TCP_Server_Info *server, struct smb_vol *vol)
>  {
> -       unsigned int secFlags;
> -
> -       if (vol->secFlg & (~(CIFSSEC_MUST_SIGN | CIFSSEC_MUST_SEAL)))
> -               secFlags = vol->secFlg;
> -       else
> -               secFlags = global_secflags | vol->secFlg;
> -
> -       switch (server->secType) {
> -       case LANMAN:
> -               if (!(secFlags & (CIFSSEC_MAY_LANMAN|CIFSSEC_MAY_PLNTXT)))
> -                       return false;
> -               break;
> -       case NTLMv2:
> -               if (!(secFlags & CIFSSEC_MAY_NTLMV2))
> -                       return false;
> -               break;
> -       case NTLM:
> -               if (!(secFlags & CIFSSEC_MAY_NTLM))
> -                       return false;
> -               break;
> -       case Kerberos:
> -               if (!(secFlags & CIFSSEC_MAY_KRB5))
> -                       return false;
> -               break;
> -       case RawNTLMSSP:
> -               if (!(secFlags & CIFSSEC_MAY_NTLMSSP))
> -                       return false;
> -               break;
> -       default:
> -               /* shouldn't happen */
> +       /*
> +        * The select_sectype function should either return the vol->sectype
> +        * that was specified, or "Unspecified" if that sectype was not
> +        * compatible with the given NEGOTIATE request.
> +        */
> +       if (select_sectype(server, vol->sectype) == Unspecified)
>                 return false;
> -       }
>
> -       /* now check if signing mode is acceptable */
> +       /*
> +        * Now check if signing mode is acceptable. No need to check
> +        * global_secflags at this point since if MUST_SIGN is set then
> +        * the server->sign had better be too.
> +        */
>         if (vol->sign && !server->sign)
>                 return false;
>
> @@ -2230,7 +2192,11 @@ out_err:
>
>  static int match_session(struct cifs_ses *ses, struct smb_vol *vol)
>  {
> -       switch (ses->server->secType) {
> +       if (vol->sectype != Unspecified &&
> +           vol->sectype != ses->sectype)
> +               return 0;
> +
> +       switch (ses->sectype) {
>         case Kerberos:
>                 if (!uid_eq(vol->cred_uid, ses->cred_uid))
>                         return 0;
> @@ -2507,7 +2473,6 @@ cifs_get_smb_ses(struct TCP_Server_Info *server, struct smb_vol *volume_info)
>         ses->cred_uid = volume_info->cred_uid;
>         ses->linux_uid = volume_info->linux_uid;
>
> -       ses->overrideSecFlg = volume_info->secFlg;
>         ses->sectype = volume_info->sectype;
>         ses->sign = volume_info->sign ? volume_info->sign :
>                                 (global_secflags & CIFSSEC_MUST_SIGN);
> @@ -3670,7 +3635,7 @@ CIFSTCon(const unsigned int xid, struct cifs_ses *ses,
>                    NTLMv2 password here) */
>  #ifdef CONFIG_CIFS_WEAK_PW_HASH
>                 if ((global_secflags & CIFSSEC_MAY_LANMAN) &&
> -                   (ses->server->secType == LANMAN))
> +                   (ses->sectype == LANMAN))
>                         calc_lanman_hash(tcon->password, ses->server->cryptkey,
>                                          ses->server->sec_mode &
>                                             SECMODE_PW_ENCRYPT ? true : false,
> @@ -3882,27 +3847,11 @@ cifs_setup_session(const unsigned int xid, struct cifs_ses *ses,
>  static int
>  cifs_set_vol_auth(struct smb_vol *vol, struct cifs_ses *ses)
>  {
> -       switch (ses->server->secType) {
> -       case Kerberos:
> -               vol->secFlg = CIFSSEC_MUST_KRB5;
> +       vol->sectype = ses->sectype;
> +
> +       /* krb5 is special, since we don't need username or pw */
> +       if (vol->sectype == Kerberos)
>                 return 0;
> -       case NTLMv2:
> -               vol->secFlg = CIFSSEC_MUST_NTLMV2;
> -               break;
> -       case NTLM:
> -               vol->secFlg = CIFSSEC_MUST_NTLM;
> -               break;
> -       case RawNTLMSSP:
> -               vol->secFlg = CIFSSEC_MUST_NTLMSSP;
> -               break;
> -       case LANMAN:
> -               vol->secFlg = CIFSSEC_MUST_LANMAN;
> -               break;
> -       default:
> -               /* should never happen */
> -               vol->secFlg = 0;
> -               break;
> -       }
>
>         return cifs_set_cifscreds(vol, ses);
>  }
> diff --git a/fs/cifs/sess.c b/fs/cifs/sess.c
> index 82b784a..79358e3 100644
> --- a/fs/cifs/sess.c
> +++ b/fs/cifs/sess.c
> @@ -550,6 +550,56 @@ setup_ntlmv2_ret:
>         return rc;
>  }
>
> +enum securityEnum
> +select_sectype(struct TCP_Server_Info *server, enum securityEnum requested)
> +{
> +       switch (server->negflavor) {
> +       case CIFS_NEGFLAVOR_EXTENDED:
> +               switch (requested) {
> +               case Kerberos:
> +               case RawNTLMSSP:
> +                       return requested;
> +               case Unspecified:
> +                       if (server->sec_ntlmssp &&
> +                           (global_secflags & CIFSSEC_MAY_NTLMSSP))
> +                               return RawNTLMSSP;
> +                       if ((server->sec_kerberos || server->sec_mskerberos) &&
> +                           (global_secflags & CIFSSEC_MAY_KRB5))
> +                               return Kerberos;
> +                       /* Fallthrough */
> +               default:
> +                       return Unspecified;
> +               }
> +       case CIFS_NEGFLAVOR_UNENCAP:
> +               switch (requested) {
> +               case NTLM:
> +               case NTLMv2:
> +                       return requested;
> +               case Unspecified:
> +                       if (global_secflags & CIFSSEC_MAY_NTLMV2)
> +                               return NTLMv2;
> +                       if (global_secflags & CIFSSEC_MAY_NTLM)
> +                               return NTLM;
> +                       /* Fallthrough */
> +               default:
> +                       return Unspecified;
> +               }
> +       case CIFS_NEGFLAVOR_LANMAN:
> +               switch (requested) {
> +               case LANMAN:
> +                       return requested;
> +               case Unspecified:
> +                       if (global_secflags & CIFSSEC_MAY_LANMAN)
> +                               return LANMAN;
> +                       /* Fallthrough */
> +               default:
> +                       return Unspecified;
> +               }
> +       default:
> +               return Unspecified;
> +       }
> +}
> +
>  int
>  CIFS_SessSetup(const unsigned int xid, struct cifs_ses *ses,
>                const struct nls_table *nls_cp)
> @@ -576,8 +626,13 @@ CIFS_SessSetup(const unsigned int xid, struct cifs_ses *ses,
>                 return -EINVAL;
>         }
>
> -       type = ses->server->secType;
> +       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;
> +       }
> +
>         if (type == RawNTLMSSP) {
>                 /* if memory allocation is successful, caller of this function
>                  * frees it.
> diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
> index c753508..d5f2bf0 100644
> --- a/fs/cifs/smb2pdu.c
> +++ b/fs/cifs/smb2pdu.c
> @@ -328,7 +328,6 @@ SMB2_negotiate(const unsigned int xid, struct cifs_ses *ses)
>         int rc = 0;
>         int resp_buftype;
>         struct TCP_Server_Info *server = ses->server;
> -       unsigned int sec_flags;
>         int blob_offset, blob_length;
>         char *security_blob;
>         int flags = CIFS_NEG_OP;
> @@ -344,14 +343,6 @@ SMB2_negotiate(const unsigned int xid, struct cifs_ses *ses)
>         if (rc)
>                 return rc;
>
> -       /* if any of auth flags (ie not sign or seal) are overriden use them */
> -       if (ses->overrideSecFlg & (~(CIFSSEC_MUST_SIGN | CIFSSEC_MUST_SEAL)))
> -               sec_flags = ses->overrideSecFlg;  /* BB FIXME fix sign flags?*/
> -       else /* if override flags set only sign/seal OR them with global auth */
> -               sec_flags = global_secflags | ses->overrideSecFlg;
> -
> -       cifs_dbg(FYI, "sec_flags 0x%x\n", sec_flags);
> -
>         req->hdr.SessionId = 0;
>
>         req->Dialects[0] = cpu_to_le16(ses->server->vals->protocol_id);
> @@ -453,7 +444,6 @@ SMB2_sess_setup(const unsigned int xid, struct cifs_ses *ses,
>         int resp_buftype;
>         __le32 phase = NtLmNegotiate; /* NTLMSSP, if needed, is multistage */
>         struct TCP_Server_Info *server = ses->server;
> -       unsigned int sec_flags;
>         u16 blob_length = 0;
>         char *security_blob;
>         char *ntlmssp_blob = NULL;
> @@ -474,7 +464,8 @@ SMB2_sess_setup(const unsigned int xid, struct cifs_ses *ses,
>         if (!ses->ntlmssp)
>                 return -ENOMEM;
>
> -       ses->server->secType = RawNTLMSSP;
> +       /* FIXME: allow for other auth types besides NTLMSSP (e.g. krb5) */
> +       ses->sectype = RawNTLMSSP;
>
>  ssetup_ntlmssp_authenticate:
>         if (phase == NtLmChallenge)
> @@ -484,14 +475,6 @@ ssetup_ntlmssp_authenticate:
>         if (rc)
>                 return rc;
>
> -       /* if any of auth flags (ie not sign or seal) are overriden use them */
> -       if (ses->overrideSecFlg & (~(CIFSSEC_MUST_SIGN | CIFSSEC_MUST_SEAL)))
> -               sec_flags = ses->overrideSecFlg;  /* BB FIXME fix sign flags?*/
> -       else /* if override flags set only sign/seal OR them with global auth */
> -               sec_flags = global_secflags | ses->overrideSecFlg;
> -
> -       cifs_dbg(FYI, "sec_flags 0x%x\n", sec_flags);
> -
>         req->hdr.SessionId = 0; /* First session, not a reauthenticate */
>         req->VcNumber = 0; /* MBZ */
>         /* to enable echos and oplocks */
> --
> 1.8.1.4
>
> --
> 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

Acked-by: Pavel Shilovsky <piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org>

--
Best regards,
Pavel Shilovsky.

[PATCH 17/19] cifs: update the default global_secflags to include "raw" NTLMv2

[Jeff Layton]

Before this patchset, the global_secflags could only offer up a single
sectype. With the new set though we have the ability to allow different
sectypes since we sort out the one to use after talking to the server.

Change the global_secflags to allow NTLMSSP or NTLMv2 by default. If the
server sets the extended security bit in the Negotiate response, then
we'll use NTLMSSP. If it doesn't then we'll use raw NTLMv2.

Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 fs/cifs/cifsglob.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index 65fd9dd..fc0d004 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -1338,7 +1338,7 @@ require use of the stronger protocol */
 #define   CIFSSEC_MUST_SEAL	0x40040 /* not supported yet */
 #define   CIFSSEC_MUST_NTLMSSP	0x80080 /* raw ntlmssp with ntlmv2 */
 
-#define   CIFSSEC_DEF (CIFSSEC_MAY_SIGN | CIFSSEC_MAY_NTLMSSP)
+#define   CIFSSEC_DEF (CIFSSEC_MAY_SIGN | CIFSSEC_MAY_NTLMV2 | CIFSSEC_MAY_NTLMSSP)
 #define   CIFSSEC_MAX (CIFSSEC_MUST_SIGN | CIFSSEC_MUST_NTLMV2)
 #define   CIFSSEC_AUTH_MASK (CIFSSEC_MAY_NTLM | CIFSSEC_MAY_NTLMV2 | CIFSSEC_MAY_LANMAN | CIFSSEC_MAY_PLNTXT | CIFSSEC_MAY_KRB5 | CIFSSEC_MAY_NTLMSSP)
 /*
-- 
1.8.1.4

Re: [PATCH 17/19] cifs: update the default global_secflags to include "raw" NTLMv2

[Pavel Shilovsky]

2013/5/23 Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>:
> Before this patchset, the global_secflags could only offer up a single
> sectype. With the new set though we have the ability to allow different
> sectypes since we sort out the one to use after talking to the server.
>
> Change the global_secflags to allow NTLMSSP or NTLMv2 by default. If the
> server sets the extended security bit in the Negotiate response, then
> we'll use NTLMSSP. If it doesn't then we'll use raw NTLMv2.
>
> Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
>  fs/cifs/cifsglob.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
> index 65fd9dd..fc0d004 100644
> --- a/fs/cifs/cifsglob.h
> +++ b/fs/cifs/cifsglob.h
> @@ -1338,7 +1338,7 @@ require use of the stronger protocol */
>  #define   CIFSSEC_MUST_SEAL    0x40040 /* not supported yet */
>  #define   CIFSSEC_MUST_NTLMSSP 0x80080 /* raw ntlmssp with ntlmv2 */
>
> -#define   CIFSSEC_DEF (CIFSSEC_MAY_SIGN | CIFSSEC_MAY_NTLMSSP)
> +#define   CIFSSEC_DEF (CIFSSEC_MAY_SIGN | CIFSSEC_MAY_NTLMV2 | CIFSSEC_MAY_NTLMSSP)
>  #define   CIFSSEC_MAX (CIFSSEC_MUST_SIGN | CIFSSEC_MUST_NTLMV2)
>  #define   CIFSSEC_AUTH_MASK (CIFSSEC_MAY_NTLM | CIFSSEC_MAY_NTLMV2 | CIFSSEC_MAY_LANMAN | CIFSSEC_MAY_PLNTXT | CIFSSEC_MAY_KRB5 | CIFSSEC_MAY_NTLMSSP)
>  /*
> --
> 1.8.1.4
>
> --
> 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

Reviewed-by: Pavel Shilovsky <piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org>

--
Best regards,
Pavel Shilovsky.

[PATCH 18/19] cifs: clean up the SecurityFlags write handler

[Jeff Layton]

The SecurityFlags handler uses an obsolete simple_strtoul() call, and
doesn't really handle the bounds checking well. Fix it to use
kstrtouint() instead. Clean up the error messages as well and fix a
bogus check for an unsigned int to be less than 0.

Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 fs/cifs/cifs_debug.c | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/fs/cifs/cifs_debug.c b/fs/cifs/cifs_debug.c
index d597483..856f8f5 100644
--- a/fs/cifs/cifs_debug.c
+++ b/fs/cifs/cifs_debug.c
@@ -598,6 +598,7 @@ static int cifs_security_flags_proc_open(struct inode *inode, struct file *file)
 static ssize_t cifs_security_flags_proc_write(struct file *file,
 		const char __user *buffer, size_t count, loff_t *ppos)
 {
+	int rc;
 	unsigned int flags;
 	char flags_string[12];
 	char c;
@@ -620,26 +621,33 @@ static ssize_t cifs_security_flags_proc_write(struct file *file,
 			global_secflags = CIFSSEC_MAX;
 			return count;
 		} else if (!isdigit(c)) {
-			cifs_dbg(VFS, "invalid flag %c\n", c);
+			cifs_dbg(VFS, "Invalid SecurityFlags: %s\n",
+					flags_string);
 			return -EINVAL;
 		}
 	}
-	/* else we have a number */
 
-	flags = simple_strtoul(flags_string, NULL, 0);
+	/* else we have a number */
+	rc = kstrtouint(flags_string, 0, &flags);
+	if (rc) {
+		cifs_dbg(VFS, "Invalid SecurityFlags: %s\n",
+				flags_string);
+		return rc;
+	}
 
 	cifs_dbg(FYI, "sec flags 0x%x\n", flags);
 
-	if (flags <= 0)  {
-		cifs_dbg(VFS, "invalid security flags %s\n", flags_string);
+	if (flags == 0)  {
+		cifs_dbg(VFS, "Invalid SecurityFlags: %s\n", flags_string);
 		return -EINVAL;
 	}
 
 	if (flags & ~CIFSSEC_MASK) {
-		cifs_dbg(VFS, "attempt to set unsupported security flags 0x%x\n",
+		cifs_dbg(VFS, "Unsupported security flags: 0x%x\n",
 			 flags & ~CIFSSEC_MASK);
 		return -EINVAL;
 	}
+
 	/* flags look ok - update the global security flags for cifs module */
 	global_secflags = flags;
 	if (global_secflags & CIFSSEC_MUST_SIGN) {
-- 
1.8.1.4

Re: [PATCH 18/19] cifs: clean up the SecurityFlags write handler

[Pavel Shilovsky]

2013/5/23 Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>:
> The SecurityFlags handler uses an obsolete simple_strtoul() call, and
> doesn't really handle the bounds checking well. Fix it to use
> kstrtouint() instead. Clean up the error messages as well and fix a
> bogus check for an unsigned int to be less than 0.
>
> Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
>  fs/cifs/cifs_debug.c | 20 ++++++++++++++------
>  1 file changed, 14 insertions(+), 6 deletions(-)
>
> diff --git a/fs/cifs/cifs_debug.c b/fs/cifs/cifs_debug.c
> index d597483..856f8f5 100644
> --- a/fs/cifs/cifs_debug.c
> +++ b/fs/cifs/cifs_debug.c
> @@ -598,6 +598,7 @@ static int cifs_security_flags_proc_open(struct inode *inode, struct file *file)
>  static ssize_t cifs_security_flags_proc_write(struct file *file,
>                 const char __user *buffer, size_t count, loff_t *ppos)
>  {
> +       int rc;
>         unsigned int flags;
>         char flags_string[12];
>         char c;
> @@ -620,26 +621,33 @@ static ssize_t cifs_security_flags_proc_write(struct file *file,
>                         global_secflags = CIFSSEC_MAX;
>                         return count;
>                 } else if (!isdigit(c)) {
> -                       cifs_dbg(VFS, "invalid flag %c\n", c);
> +                       cifs_dbg(VFS, "Invalid SecurityFlags: %s\n",
> +                                       flags_string);
>                         return -EINVAL;
>                 }
>         }
> -       /* else we have a number */
>
> -       flags = simple_strtoul(flags_string, NULL, 0);
> +       /* else we have a number */
> +       rc = kstrtouint(flags_string, 0, &flags);
> +       if (rc) {
> +               cifs_dbg(VFS, "Invalid SecurityFlags: %s\n",
> +                               flags_string);
> +               return rc;
> +       }
>
>         cifs_dbg(FYI, "sec flags 0x%x\n", flags);
>
> -       if (flags <= 0)  {
> -               cifs_dbg(VFS, "invalid security flags %s\n", flags_string);
> +       if (flags == 0)  {
> +               cifs_dbg(VFS, "Invalid SecurityFlags: %s\n", flags_string);
>                 return -EINVAL;
>         }
>
>         if (flags & ~CIFSSEC_MASK) {
> -               cifs_dbg(VFS, "attempt to set unsupported security flags 0x%x\n",
> +               cifs_dbg(VFS, "Unsupported security flags: 0x%x\n",
>                          flags & ~CIFSSEC_MASK);
>                 return -EINVAL;
>         }
> +
>         /* flags look ok - update the global security flags for cifs module */
>         global_secflags = flags;
>         if (global_secflags & CIFSSEC_MUST_SIGN) {
> --
> 1.8.1.4
>
> --
> 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

Reviewed-by: Pavel Shilovsky <piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org>

--
Best regards,
Pavel Shilovsky.

[PATCH 19/19] cifs: try to handle the MUST SecurityFlags sanely

[Jeff Layton]

The cifs.ko SecurityFlags interface wins my award for worst-designed
interface ever, but we're sort of stuck with it since it's documented
and people do use it (even if it doesn't work correctly).

Case in point -- you can specify multiple sets of "MUST" flags. It makes
absolutely no sense, but you can do it.

What should the effect be in such a case? No one knows or seems to have
considered this, so let's define it now. If you try to specify multiple
MUST flags, clear any other MAY or MUST bits except for the ones that
involve signing.

Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 fs/cifs/cifs_debug.c | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/fs/cifs/cifs_debug.c b/fs/cifs/cifs_debug.c
index 856f8f5..d21339a 100644
--- a/fs/cifs/cifs_debug.c
+++ b/fs/cifs/cifs_debug.c
@@ -595,6 +595,32 @@ static int cifs_security_flags_proc_open(struct inode *inode, struct file *file)
 	return single_open(file, cifs_security_flags_proc_show, NULL);
 }
 
+/*
+ * Ensure that if someone sets a MUST flag, that we disable all other MAY
+ * flags except for the ones corresponding to the given MUST flag. If there are
+ * multiple MUST flags, then try to prefer more secure ones.
+ */
+static void
+cifs_security_flags_handle_must_flags(unsigned int *flags)
+{
+	unsigned int signflags = *flags & CIFSSEC_MUST_SIGN;
+
+	if ((*flags & CIFSSEC_MUST_KRB5) == CIFSSEC_MUST_KRB5)
+		*flags = CIFSSEC_MUST_KRB5;
+	else if ((*flags & CIFSSEC_MUST_NTLMSSP) == CIFSSEC_MUST_NTLMSSP)
+		*flags = CIFSSEC_MUST_NTLMSSP;
+	else if ((*flags & CIFSSEC_MUST_NTLMV2) == CIFSSEC_MUST_NTLMV2)
+		*flags = CIFSSEC_MUST_NTLMV2;
+	else if ((*flags & CIFSSEC_MUST_NTLM) == CIFSSEC_MUST_NTLM)
+		*flags = CIFSSEC_MUST_NTLM;
+	else if ((*flags & CIFSSEC_MUST_LANMAN) == CIFSSEC_MUST_LANMAN)
+		*flags = CIFSSEC_MUST_LANMAN;
+	else if ((*flags & CIFSSEC_MUST_PLNTXT) == CIFSSEC_MUST_PLNTXT)
+		*flags = CIFSSEC_MUST_PLNTXT;
+
+	*flags |= signflags;
+}
+
 static ssize_t cifs_security_flags_proc_write(struct file *file,
 		const char __user *buffer, size_t count, loff_t *ppos)
 {
@@ -648,6 +674,8 @@ static ssize_t cifs_security_flags_proc_write(struct file *file,
 		return -EINVAL;
 	}
 
+	cifs_security_flags_handle_must_flags(&flags);
+
 	/* flags look ok - update the global security flags for cifs module */
 	global_secflags = flags;
 	if (global_secflags & CIFSSEC_MUST_SIGN) {
-- 
1.8.1.4

Re: [PATCH 19/19] cifs: try to handle the MUST SecurityFlags sanely

[Pavel Shilovsky]

2013/5/23 Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>:
> The cifs.ko SecurityFlags interface wins my award for worst-designed
> interface ever, but we're sort of stuck with it since it's documented
> and people do use it (even if it doesn't work correctly).
>
> Case in point -- you can specify multiple sets of "MUST" flags. It makes
> absolutely no sense, but you can do it.
>
> What should the effect be in such a case? No one knows or seems to have
> considered this, so let's define it now. If you try to specify multiple
> MUST flags, clear any other MAY or MUST bits except for the ones that
> involve signing.
>
> Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
>  fs/cifs/cifs_debug.c | 28 ++++++++++++++++++++++++++++
>  1 file changed, 28 insertions(+)
>
> diff --git a/fs/cifs/cifs_debug.c b/fs/cifs/cifs_debug.c
> index 856f8f5..d21339a 100644
> --- a/fs/cifs/cifs_debug.c
> +++ b/fs/cifs/cifs_debug.c
> @@ -595,6 +595,32 @@ static int cifs_security_flags_proc_open(struct inode *inode, struct file *file)
>         return single_open(file, cifs_security_flags_proc_show, NULL);
>  }
>
> +/*
> + * Ensure that if someone sets a MUST flag, that we disable all other MAY
> + * flags except for the ones corresponding to the given MUST flag. If there are
> + * multiple MUST flags, then try to prefer more secure ones.
> + */
> +static void
> +cifs_security_flags_handle_must_flags(unsigned int *flags)
> +{
> +       unsigned int signflags = *flags & CIFSSEC_MUST_SIGN;
> +
> +       if ((*flags & CIFSSEC_MUST_KRB5) == CIFSSEC_MUST_KRB5)
> +               *flags = CIFSSEC_MUST_KRB5;
> +       else if ((*flags & CIFSSEC_MUST_NTLMSSP) == CIFSSEC_MUST_NTLMSSP)
> +               *flags = CIFSSEC_MUST_NTLMSSP;
> +       else if ((*flags & CIFSSEC_MUST_NTLMV2) == CIFSSEC_MUST_NTLMV2)
> +               *flags = CIFSSEC_MUST_NTLMV2;
> +       else if ((*flags & CIFSSEC_MUST_NTLM) == CIFSSEC_MUST_NTLM)
> +               *flags = CIFSSEC_MUST_NTLM;
> +       else if ((*flags & CIFSSEC_MUST_LANMAN) == CIFSSEC_MUST_LANMAN)
> +               *flags = CIFSSEC_MUST_LANMAN;
> +       else if ((*flags & CIFSSEC_MUST_PLNTXT) == CIFSSEC_MUST_PLNTXT)
> +               *flags = CIFSSEC_MUST_PLNTXT;
> +
> +       *flags |= signflags;
> +}
> +
>  static ssize_t cifs_security_flags_proc_write(struct file *file,
>                 const char __user *buffer, size_t count, loff_t *ppos)
>  {
> @@ -648,6 +674,8 @@ static ssize_t cifs_security_flags_proc_write(struct file *file,
>                 return -EINVAL;
>         }
>
> +       cifs_security_flags_handle_must_flags(&flags);
> +
>         /* flags look ok - update the global security flags for cifs module */
>         global_secflags = flags;
>         if (global_secflags & CIFSSEC_MUST_SIGN) {
> --
> 1.8.1.4
>
> --
> 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

Reviewed-by: Pavel Shilovsky <piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org>

--
Best regards,
Pavel Shilovsky.