diff mbox series

smb3: add smb3.1.1 to default dialect list

Message ID CAH2r5mt53QM77=wsPJ7vAvZLyyQTf0x1-EfPUvaug6M8kOYXhQ@mail.gmail.com
State New
Headers show
Series smb3: add smb3.1.1 to default dialect list | expand

Commit Message

Steve French Jan. 3, 2019, 8:43 a.m. UTC 
SMB3.1.1 dialect has additional security (among other) features
and should be requested when mounting to modern servers so it
can be used if the server supports it.

Add SMB3.1.1 to the default list of dialects requested.

Signed-off-by: Steve French <stfrench@microsoft.com>
---
 fs/cifs/smb2pdu.c | 38 ++++++++++++++++++++++++++------------
 1 file changed, 26 insertions(+), 12 deletions(-)

 #define SMB2_POSIX_EXTENSIONS_AVAILABLE        cpu_to_le16(0x100)
@@ -491,10 +487,24 @@ static void
 assemble_neg_contexts(struct smb2_negotiate_req *req,
               unsigned int *total_len)
 {
-    char *pneg_ctxt = (char *)req + OFFSET_OF_NEG_CONTEXT;
+    char *pneg_ctxt = (char *)req;
     unsigned int ctxt_len;

-    *total_len += 2; /* Add 2 due to round to 8 byte boundary for 1st ctxt */
+    if (*total_len > 200) {
+        /* In case length corrupted don't want to overrun smb buffer */
+        cifs_dbg(VFS, "Bad frame length assembling neg contexts\n");
+        return;
+    }
+
+    /*
+     * round up total_len of fixed part of SMB3 negotiate request to 8
+     * byte boundary before adding negotiate contexts
+     */
+    *total_len = roundup(*total_len, 8);
+
+    pneg_ctxt = (*total_len) + (char *)req;
+    req->NegotiateContextOffset = cpu_to_le32(*total_len);
+
     build_preauth_ctxt((struct smb2_preauth_neg_context *)pneg_ctxt);
     ctxt_len = DIV_ROUND_UP(sizeof(struct smb2_preauth_neg_context), 8) * 8;
     *total_len += ctxt_len;
@@ -508,7 +518,6 @@ assemble_neg_contexts(struct smb2_negotiate_req *req,
     build_posix_ctxt((struct smb2_posix_neg_context *)pneg_ctxt);
     *total_len += sizeof(struct smb2_posix_neg_context);

-    req->NegotiateContextOffset = cpu_to_le32(OFFSET_OF_NEG_CONTEXT);
     req->NegotiateContextCount = cpu_to_le16(3);
 }

@@ -724,8 +733,9 @@ SMB2_negotiate(const unsigned int xid, struct cifs_ses *ses)
         req->Dialects[0] = cpu_to_le16(SMB21_PROT_ID);
         req->Dialects[1] = cpu_to_le16(SMB30_PROT_ID);
         req->Dialects[2] = cpu_to_le16(SMB302_PROT_ID);
-        req->DialectCount = cpu_to_le16(3);
-        total_len += 6;
+        req->Dialects[3] = cpu_to_le16(SMB311_PROT_ID);
+        req->DialectCount = cpu_to_le16(4);
+        total_len += 8;
     } else {
         /* otherwise send specific dialect */
         req->Dialects[0] = cpu_to_le16(ses->server->vals->protocol_id);
@@ -749,7 +759,9 @@ SMB2_negotiate(const unsigned int xid, struct cifs_ses *ses)
     else {
         memcpy(req->ClientGUID, server->client_guid,
             SMB2_CLIENT_GUID_SIZE);
-        if (ses->server->vals->protocol_id == SMB311_PROT_ID)
+        if ((ses->server->vals->protocol_id == SMB311_PROT_ID) ||
+            (strcmp(ses->server->vals->version_string,
+             SMBDEFAULT_VERSION_STRING) == 0))
             assemble_neg_contexts(req, &total_len);
     }
     iov[0].iov_base = (char *)req;
@@ -794,7 +806,8 @@ SMB2_negotiate(const unsigned int xid, struct cifs_ses *ses)
         } else if (rsp->DialectRevision == cpu_to_le16(SMB21_PROT_ID)) {
             /* ops set to 3.0 by default for default so update */
             ses->server->ops = &smb21_operations;
-        }
+        } else if (rsp->DialectRevision == cpu_to_le16(SMB311_PROT_ID))
+            ses->server->ops = &smb311_operations;
     } else if (le16_to_cpu(rsp->DialectRevision) !=
                 ses->server->vals->protocol_id) {
         /* if requested single dialect ensure returned dialect matched */
@@ -947,7 +960,8 @@ int smb3_validate_negotiate(const unsigned int
xid, struct cifs_tcon *tcon)
         pneg_inbuf->Dialects[0] = cpu_to_le16(SMB21_PROT_ID);
         pneg_inbuf->Dialects[1] = cpu_to_le16(SMB30_PROT_ID);
         pneg_inbuf->Dialects[2] = cpu_to_le16(SMB302_PROT_ID);
-        pneg_inbuf->DialectCount = cpu_to_le16(3);
+        pneg_inbuf->Dialects[3] = cpu_to_le16(SMB311_PROT_ID);
+        pneg_inbuf->DialectCount = cpu_to_le16(4);
         /* structure is big enough for 3 dialects */
         inbuflen = sizeof(*pneg_inbuf);
     } else {

Comments

ronnie sahlberg Jan. 3, 2019, 9 a.m. UTC | #1 
Reviewed-by: Ronnie Sahlberg <lsahlber@redhat.com>

On Thu, Jan 3, 2019 at 6:45 PM Steve French via samba-technical
<samba-technical@lists.samba.org> wrote:
>
> SMB3.1.1 dialect has additional security (among other) features
> and should be requested when mounting to modern servers so it
> can be used if the server supports it.
>
> Add SMB3.1.1 to the default list of dialects requested.
>
> Signed-off-by: Steve French <stfrench@microsoft.com>
> ---
>  fs/cifs/smb2pdu.c | 38 ++++++++++++++++++++++++++------------
>  1 file changed, 26 insertions(+), 12 deletions(-)
>
> diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
> index e283590955cd..9bc1dec84b35 100644
> --- a/fs/cifs/smb2pdu.c
> +++ b/fs/cifs/smb2pdu.c
> @@ -451,10 +451,6 @@ smb2_plain_req_init(__le16 smb2_command, struct
> cifs_tcon *tcon,
>  }
>
>
> -/* offset is sizeof smb2_negotiate_req but rounded up to 8 bytes */
> -#define OFFSET_OF_NEG_CONTEXT 0x68  /* sizeof(struct smb2_negotiate_req) */
> -
> -
>  #define SMB2_PREAUTH_INTEGRITY_CAPABILITIES    cpu_to_le16(1)
>  #define SMB2_ENCRYPTION_CAPABILITIES        cpu_to_le16(2)
>  #define SMB2_POSIX_EXTENSIONS_AVAILABLE        cpu_to_le16(0x100)
> @@ -491,10 +487,24 @@ static void
>  assemble_neg_contexts(struct smb2_negotiate_req *req,
>                unsigned int *total_len)
>  {
> -    char *pneg_ctxt = (char *)req + OFFSET_OF_NEG_CONTEXT;
> +    char *pneg_ctxt = (char *)req;
>      unsigned int ctxt_len;
>
> -    *total_len += 2; /* Add 2 due to round to 8 byte boundary for 1st ctxt */
> +    if (*total_len > 200) {
> +        /* In case length corrupted don't want to overrun smb buffer */
> +        cifs_dbg(VFS, "Bad frame length assembling neg contexts\n");
> +        return;
> +    }
> +
> +    /*
> +     * round up total_len of fixed part of SMB3 negotiate request to 8
> +     * byte boundary before adding negotiate contexts
> +     */
> +    *total_len = roundup(*total_len, 8);
> +
> +    pneg_ctxt = (*total_len) + (char *)req;
> +    req->NegotiateContextOffset = cpu_to_le32(*total_len);
> +
>      build_preauth_ctxt((struct smb2_preauth_neg_context *)pneg_ctxt);
>      ctxt_len = DIV_ROUND_UP(sizeof(struct smb2_preauth_neg_context), 8) * 8;
>      *total_len += ctxt_len;
> @@ -508,7 +518,6 @@ assemble_neg_contexts(struct smb2_negotiate_req *req,
>      build_posix_ctxt((struct smb2_posix_neg_context *)pneg_ctxt);
>      *total_len += sizeof(struct smb2_posix_neg_context);
>
> -    req->NegotiateContextOffset = cpu_to_le32(OFFSET_OF_NEG_CONTEXT);
>      req->NegotiateContextCount = cpu_to_le16(3);
>  }
>
> @@ -724,8 +733,9 @@ SMB2_negotiate(const unsigned int xid, struct cifs_ses *ses)
>          req->Dialects[0] = cpu_to_le16(SMB21_PROT_ID);
>          req->Dialects[1] = cpu_to_le16(SMB30_PROT_ID);
>          req->Dialects[2] = cpu_to_le16(SMB302_PROT_ID);
> -        req->DialectCount = cpu_to_le16(3);
> -        total_len += 6;
> +        req->Dialects[3] = cpu_to_le16(SMB311_PROT_ID);
> +        req->DialectCount = cpu_to_le16(4);
> +        total_len += 8;
>      } else {
>          /* otherwise send specific dialect */
>          req->Dialects[0] = cpu_to_le16(ses->server->vals->protocol_id);
> @@ -749,7 +759,9 @@ SMB2_negotiate(const unsigned int xid, struct cifs_ses *ses)
>      else {
>          memcpy(req->ClientGUID, server->client_guid,
>              SMB2_CLIENT_GUID_SIZE);
> -        if (ses->server->vals->protocol_id == SMB311_PROT_ID)
> +        if ((ses->server->vals->protocol_id == SMB311_PROT_ID) ||
> +            (strcmp(ses->server->vals->version_string,
> +             SMBDEFAULT_VERSION_STRING) == 0))
>              assemble_neg_contexts(req, &total_len);
>      }
>      iov[0].iov_base = (char *)req;
> @@ -794,7 +806,8 @@ SMB2_negotiate(const unsigned int xid, struct cifs_ses *ses)
>          } else if (rsp->DialectRevision == cpu_to_le16(SMB21_PROT_ID)) {
>              /* ops set to 3.0 by default for default so update */
>              ses->server->ops = &smb21_operations;
> -        }
> +        } else if (rsp->DialectRevision == cpu_to_le16(SMB311_PROT_ID))
> +            ses->server->ops = &smb311_operations;
>      } else if (le16_to_cpu(rsp->DialectRevision) !=
>                  ses->server->vals->protocol_id) {
>          /* if requested single dialect ensure returned dialect matched */
> @@ -947,7 +960,8 @@ int smb3_validate_negotiate(const unsigned int
> xid, struct cifs_tcon *tcon)
>          pneg_inbuf->Dialects[0] = cpu_to_le16(SMB21_PROT_ID);
>          pneg_inbuf->Dialects[1] = cpu_to_le16(SMB30_PROT_ID);
>          pneg_inbuf->Dialects[2] = cpu_to_le16(SMB302_PROT_ID);
> -        pneg_inbuf->DialectCount = cpu_to_le16(3);
> +        pneg_inbuf->Dialects[3] = cpu_to_le16(SMB311_PROT_ID);
> +        pneg_inbuf->DialectCount = cpu_to_le16(4);
>          /* structure is big enough for 3 dialects */
>          inbuflen = sizeof(*pneg_inbuf);
>      } else {
> --
>
>
> --
> Thanks,
>
> Steve
Steve French Jan. 3, 2019, 8:48 p.m. UTC | #2 
Minor updates to the structure (to make it one dialect longer) used in
validate_negotiate


On Thu, Jan 3, 2019 at 2:43 AM Steve French <smfrench@gmail.com> wrote:
>
> SMB3.1.1 dialect has additional security (among other) features
> and should be requested when mounting to modern servers so it
> can be used if the server supports it.
>
> Add SMB3.1.1 to the default list of dialects requested.
>
> Signed-off-by: Steve French <stfrench@microsoft.com>
> ---
>  fs/cifs/smb2pdu.c | 38 ++++++++++++++++++++++++++------------
>  1 file changed, 26 insertions(+), 12 deletions(-)
>
> diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
> index e283590955cd..9bc1dec84b35 100644
> --- a/fs/cifs/smb2pdu.c
> +++ b/fs/cifs/smb2pdu.c
> @@ -451,10 +451,6 @@ smb2_plain_req_init(__le16 smb2_command, struct
> cifs_tcon *tcon,
>  }
>
>
> -/* offset is sizeof smb2_negotiate_req but rounded up to 8 bytes */
> -#define OFFSET_OF_NEG_CONTEXT 0x68  /* sizeof(struct smb2_negotiate_req) */
> -
> -
>  #define SMB2_PREAUTH_INTEGRITY_CAPABILITIES    cpu_to_le16(1)
>  #define SMB2_ENCRYPTION_CAPABILITIES        cpu_to_le16(2)
>  #define SMB2_POSIX_EXTENSIONS_AVAILABLE        cpu_to_le16(0x100)
> @@ -491,10 +487,24 @@ static void
>  assemble_neg_contexts(struct smb2_negotiate_req *req,
>                unsigned int *total_len)
>  {
> -    char *pneg_ctxt = (char *)req + OFFSET_OF_NEG_CONTEXT;
> +    char *pneg_ctxt = (char *)req;
>      unsigned int ctxt_len;
>
> -    *total_len += 2; /* Add 2 due to round to 8 byte boundary for 1st ctxt */
> +    if (*total_len > 200) {
> +        /* In case length corrupted don't want to overrun smb buffer */
> +        cifs_dbg(VFS, "Bad frame length assembling neg contexts\n");
> +        return;
> +    }
> +
> +    /*
> +     * round up total_len of fixed part of SMB3 negotiate request to 8
> +     * byte boundary before adding negotiate contexts
> +     */
> +    *total_len = roundup(*total_len, 8);
> +
> +    pneg_ctxt = (*total_len) + (char *)req;
> +    req->NegotiateContextOffset = cpu_to_le32(*total_len);
> +
>      build_preauth_ctxt((struct smb2_preauth_neg_context *)pneg_ctxt);
>      ctxt_len = DIV_ROUND_UP(sizeof(struct smb2_preauth_neg_context), 8) * 8;
>      *total_len += ctxt_len;
> @@ -508,7 +518,6 @@ assemble_neg_contexts(struct smb2_negotiate_req *req,
>      build_posix_ctxt((struct smb2_posix_neg_context *)pneg_ctxt);
>      *total_len += sizeof(struct smb2_posix_neg_context);
>
> -    req->NegotiateContextOffset = cpu_to_le32(OFFSET_OF_NEG_CONTEXT);
>      req->NegotiateContextCount = cpu_to_le16(3);
>  }
>
> @@ -724,8 +733,9 @@ SMB2_negotiate(const unsigned int xid, struct cifs_ses *ses)
>          req->Dialects[0] = cpu_to_le16(SMB21_PROT_ID);
>          req->Dialects[1] = cpu_to_le16(SMB30_PROT_ID);
>          req->Dialects[2] = cpu_to_le16(SMB302_PROT_ID);
> -        req->DialectCount = cpu_to_le16(3);
> -        total_len += 6;
> +        req->Dialects[3] = cpu_to_le16(SMB311_PROT_ID);
> +        req->DialectCount = cpu_to_le16(4);
> +        total_len += 8;
>      } else {
>          /* otherwise send specific dialect */
>          req->Dialects[0] = cpu_to_le16(ses->server->vals->protocol_id);
> @@ -749,7 +759,9 @@ SMB2_negotiate(const unsigned int xid, struct cifs_ses *ses)
>      else {
>          memcpy(req->ClientGUID, server->client_guid,
>              SMB2_CLIENT_GUID_SIZE);
> -        if (ses->server->vals->protocol_id == SMB311_PROT_ID)
> +        if ((ses->server->vals->protocol_id == SMB311_PROT_ID) ||
> +            (strcmp(ses->server->vals->version_string,
> +             SMBDEFAULT_VERSION_STRING) == 0))
>              assemble_neg_contexts(req, &total_len);
>      }
>      iov[0].iov_base = (char *)req;
> @@ -794,7 +806,8 @@ SMB2_negotiate(const unsigned int xid, struct cifs_ses *ses)
>          } else if (rsp->DialectRevision == cpu_to_le16(SMB21_PROT_ID)) {
>              /* ops set to 3.0 by default for default so update */
>              ses->server->ops = &smb21_operations;
> -        }
> +        } else if (rsp->DialectRevision == cpu_to_le16(SMB311_PROT_ID))
> +            ses->server->ops = &smb311_operations;
>      } else if (le16_to_cpu(rsp->DialectRevision) !=
>                  ses->server->vals->protocol_id) {
>          /* if requested single dialect ensure returned dialect matched */
> @@ -947,7 +960,8 @@ int smb3_validate_negotiate(const unsigned int
> xid, struct cifs_tcon *tcon)
>          pneg_inbuf->Dialects[0] = cpu_to_le16(SMB21_PROT_ID);
>          pneg_inbuf->Dialects[1] = cpu_to_le16(SMB30_PROT_ID);
>          pneg_inbuf->Dialects[2] = cpu_to_le16(SMB302_PROT_ID);
> -        pneg_inbuf->DialectCount = cpu_to_le16(3);
> +        pneg_inbuf->Dialects[3] = cpu_to_le16(SMB311_PROT_ID);
> +        pneg_inbuf->DialectCount = cpu_to_le16(4);
>          /* structure is big enough for 3 dialects */
>          inbuflen = sizeof(*pneg_inbuf);
>      } else {
> --
>
>
> --
> Thanks,
>
> Steve
diff mbox series

Patch

From 9e15144e492e05cc2f66a30d30fd8b482538a24f Mon Sep 17 00:00:00 2001
From: Steve French <stfrench@microsoft.com>
Date: Thu, 3 Jan 2019 02:37:21 -0600
Subject: [PATCH] smb3: add smb3.1.1 to default dialect list

SMB3.1.1 dialect has additional security (among other) features
and should be requested when mounting to modern servers so it
can be used if the server supports it.

Add SMB3.1.1 to the default list of dialects requested.

Signed-off-by: Steve French <stfrench@microsoft.com>
---
 fs/cifs/smb2pdu.c | 38 ++++++++++++++++++++++++++------------
 1 file changed, 26 insertions(+), 12 deletions(-)

diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
index e283590955cd..9bc1dec84b35 100644
--- a/fs/cifs/smb2pdu.c
+++ b/fs/cifs/smb2pdu.c
@@ -451,10 +451,6 @@  smb2_plain_req_init(__le16 smb2_command, struct cifs_tcon *tcon,
 }
 
 
-/* offset is sizeof smb2_negotiate_req but rounded up to 8 bytes */
-#define OFFSET_OF_NEG_CONTEXT 0x68  /* sizeof(struct smb2_negotiate_req) */
-
-
 #define SMB2_PREAUTH_INTEGRITY_CAPABILITIES	cpu_to_le16(1)
 #define SMB2_ENCRYPTION_CAPABILITIES		cpu_to_le16(2)
 #define SMB2_POSIX_EXTENSIONS_AVAILABLE		cpu_to_le16(0x100)
@@ -491,10 +487,24 @@  static void
 assemble_neg_contexts(struct smb2_negotiate_req *req,
 		      unsigned int *total_len)
 {
-	char *pneg_ctxt = (char *)req + OFFSET_OF_NEG_CONTEXT;
+	char *pneg_ctxt = (char *)req;
 	unsigned int ctxt_len;
 
-	*total_len += 2; /* Add 2 due to round to 8 byte boundary for 1st ctxt */
+	if (*total_len > 200) {
+		/* In case length corrupted don't want to overrun smb buffer */
+		cifs_dbg(VFS, "Bad frame length assembling neg contexts\n");
+		return;
+	}
+
+	/*
+	 * round up total_len of fixed part of SMB3 negotiate request to 8
+	 * byte boundary before adding negotiate contexts
+	 */
+	*total_len = roundup(*total_len, 8);
+
+	pneg_ctxt = (*total_len) + (char *)req;
+	req->NegotiateContextOffset = cpu_to_le32(*total_len);
+
 	build_preauth_ctxt((struct smb2_preauth_neg_context *)pneg_ctxt);
 	ctxt_len = DIV_ROUND_UP(sizeof(struct smb2_preauth_neg_context), 8) * 8;
 	*total_len += ctxt_len;
@@ -508,7 +518,6 @@  assemble_neg_contexts(struct smb2_negotiate_req *req,
 	build_posix_ctxt((struct smb2_posix_neg_context *)pneg_ctxt);
 	*total_len += sizeof(struct smb2_posix_neg_context);
 
-	req->NegotiateContextOffset = cpu_to_le32(OFFSET_OF_NEG_CONTEXT);
 	req->NegotiateContextCount = cpu_to_le16(3);
 }
 
@@ -724,8 +733,9 @@  SMB2_negotiate(const unsigned int xid, struct cifs_ses *ses)
 		req->Dialects[0] = cpu_to_le16(SMB21_PROT_ID);
 		req->Dialects[1] = cpu_to_le16(SMB30_PROT_ID);
 		req->Dialects[2] = cpu_to_le16(SMB302_PROT_ID);
-		req->DialectCount = cpu_to_le16(3);
-		total_len += 6;
+		req->Dialects[3] = cpu_to_le16(SMB311_PROT_ID);
+		req->DialectCount = cpu_to_le16(4);
+		total_len += 8;
 	} else {
 		/* otherwise send specific dialect */
 		req->Dialects[0] = cpu_to_le16(ses->server->vals->protocol_id);
@@ -749,7 +759,9 @@  SMB2_negotiate(const unsigned int xid, struct cifs_ses *ses)
 	else {
 		memcpy(req->ClientGUID, server->client_guid,
 			SMB2_CLIENT_GUID_SIZE);
-		if (ses->server->vals->protocol_id == SMB311_PROT_ID)
+		if ((ses->server->vals->protocol_id == SMB311_PROT_ID) ||
+		    (strcmp(ses->server->vals->version_string,
+		     SMBDEFAULT_VERSION_STRING) == 0))
 			assemble_neg_contexts(req, &total_len);
 	}
 	iov[0].iov_base = (char *)req;
@@ -794,7 +806,8 @@  SMB2_negotiate(const unsigned int xid, struct cifs_ses *ses)
 		} else if (rsp->DialectRevision == cpu_to_le16(SMB21_PROT_ID)) {
 			/* ops set to 3.0 by default for default so update */
 			ses->server->ops = &smb21_operations;
-		}
+		} else if (rsp->DialectRevision == cpu_to_le16(SMB311_PROT_ID))
+			ses->server->ops = &smb311_operations;
 	} else if (le16_to_cpu(rsp->DialectRevision) !=
 				ses->server->vals->protocol_id) {
 		/* if requested single dialect ensure returned dialect matched */
@@ -947,7 +960,8 @@  int smb3_validate_negotiate(const unsigned int xid, struct cifs_tcon *tcon)
 		pneg_inbuf->Dialects[0] = cpu_to_le16(SMB21_PROT_ID);
 		pneg_inbuf->Dialects[1] = cpu_to_le16(SMB30_PROT_ID);
 		pneg_inbuf->Dialects[2] = cpu_to_le16(SMB302_PROT_ID);
-		pneg_inbuf->DialectCount = cpu_to_le16(3);
+		pneg_inbuf->Dialects[3] = cpu_to_le16(SMB311_PROT_ID);
+		pneg_inbuf->DialectCount = cpu_to_le16(4);
 		/* structure is big enough for 3 dialects */
 		inbuflen = sizeof(*pneg_inbuf);
 	} else {
-- 
2.17.1