All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] ksmbd: check protocol id in ksmbd_verify_smb_message()
@ 2021-09-22 12:00 Namjae Jeon
  2021-09-22 14:21 ` Ralph Boehme
  2021-09-22 21:33 ` ronnie sahlberg
  0 siblings, 2 replies; 4+ messages in thread
From: Namjae Jeon @ 2021-09-22 12:00 UTC (permalink / raw)
  To: linux-cifs
  Cc: Namjae Jeon, Ronnie Sahlberg, Ralph Böhme, Steve French,
	Ronnie Sahlberg

When second smb2 pdu has invalid protocol id, ksmbd doesn't detect it
and allow to process smb2 request. This patch add the check it in
ksmbd_verify_smb_message() and don't use protocol id of smb2 request as
protocol id of response.

Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
Cc: Ralph Böhme <slow@samba.org>
Cc: Steve French <smfrench@gmail.com>
Reported-by: Ronnie Sahlberg <lsahlber@redhat.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
---
 fs/ksmbd/smb2pdu.c    |  2 +-
 fs/ksmbd/smb_common.c | 13 +++++++++----
 fs/ksmbd/smb_common.h |  1 +
 3 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c
index 3d250e2539e6..3be1493cb18d 100644
--- a/fs/ksmbd/smb2pdu.c
+++ b/fs/ksmbd/smb2pdu.c
@@ -433,7 +433,7 @@ static void init_chained_smb2_rsp(struct ksmbd_work *work)
 		work->compound_pfid = KSMBD_NO_FID;
 	}
 	memset((char *)rsp_hdr + 4, 0, sizeof(struct smb2_hdr) + 2);
-	rsp_hdr->ProtocolId = rcv_hdr->ProtocolId;
+	rsp_hdr->ProtocolId = SMB2_PROTO_NUMBER;
 	rsp_hdr->StructureSize = SMB2_HEADER_STRUCTURE_SIZE;
 	rsp_hdr->Command = rcv_hdr->Command;
 
diff --git a/fs/ksmbd/smb_common.c b/fs/ksmbd/smb_common.c
index da17b21ac685..ace8a1b02c81 100644
--- a/fs/ksmbd/smb_common.c
+++ b/fs/ksmbd/smb_common.c
@@ -129,16 +129,22 @@ int ksmbd_lookup_protocol_idx(char *str)
  *
  * check for valid smb signature and packet direction(request/response)
  *
- * Return:      0 on success, otherwise 1
+ * Return:      0 on success, otherwise -EINVAL
  */
 int ksmbd_verify_smb_message(struct ksmbd_work *work)
 {
-	struct smb2_hdr *smb2_hdr = work->request_buf;
+	struct smb2_hdr *smb2_hdr = work->request_buf + work->next_smb2_rcv_hdr_off;
+	struct smb_hdr *hdr;
 
 	if (smb2_hdr->ProtocolId == SMB2_PROTO_NUMBER)
 		return ksmbd_smb2_check_message(work);
 
-	return 0;
+	hdr = work->request_buf;
+	if (*(__le32 *)hdr->Protocol == SMB1_PROTO_NUMBER &&
+	    hdr->Command == SMB_COM_NEGOTIATE)
+		return 0;
+
+	return -EINVAL;
 }
 
 /**
@@ -270,7 +276,6 @@ static int ksmbd_negotiate_smb_dialect(void *buf)
 	return BAD_PROT_ID;
 }
 
-#define SMB_COM_NEGOTIATE	0x72
 int ksmbd_init_smb_server(struct ksmbd_work *work)
 {
 	struct ksmbd_conn *conn = work->conn;
diff --git a/fs/ksmbd/smb_common.h b/fs/ksmbd/smb_common.h
index d7df19c97c4c..994abede27e9 100644
--- a/fs/ksmbd/smb_common.h
+++ b/fs/ksmbd/smb_common.h
@@ -202,6 +202,7 @@
 		FILE_READ_ATTRIBUTES | FILE_WRITE_ATTRIBUTES)
 
 #define SMB1_PROTO_NUMBER		cpu_to_le32(0x424d53ff)
+#define SMB_COM_NEGOTIATE		0x72
 
 #define SMB1_CLIENT_GUID_SIZE		(16)
 struct smb_hdr {
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] ksmbd: check protocol id in ksmbd_verify_smb_message()
  2021-09-22 12:00 [PATCH] ksmbd: check protocol id in ksmbd_verify_smb_message() Namjae Jeon
@ 2021-09-22 14:21 ` Ralph Boehme
  2021-09-22 21:33 ` ronnie sahlberg
  1 sibling, 0 replies; 4+ messages in thread
From: Ralph Boehme @ 2021-09-22 14:21 UTC (permalink / raw)
  To: Namjae Jeon, linux-cifs; +Cc: Ronnie Sahlberg, Steve French, Ronnie Sahlberg


[-- Attachment #1.1: Type: text/plain, Size: 688 bytes --]

Am 22.09.21 um 14:00 schrieb Namjae Jeon:
> When second smb2 pdu has invalid protocol id, ksmbd doesn't detect it
> and allow to process smb2 request. This patch add the check it in
> ksmbd_verify_smb_message() and don't use protocol id of smb2 request as
> protocol id of response.
> 
> Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
> Cc: Ralph Böhme <slow@samba.org>
> Cc: Steve French <smfrench@gmail.com>
> Reported-by: Ronnie Sahlberg <lsahlber@redhat.com>
> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>

reviewed-by: me.

-slow

-- 
Ralph Boehme, Samba Team                 https://samba.org/
SerNet Samba Team Lead      https://sernet.de/en/team-samba


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] ksmbd: check protocol id in ksmbd_verify_smb_message()
  2021-09-22 12:00 [PATCH] ksmbd: check protocol id in ksmbd_verify_smb_message() Namjae Jeon
  2021-09-22 14:21 ` Ralph Boehme
@ 2021-09-22 21:33 ` ronnie sahlberg
  2021-09-22 22:24   ` Steve French
  1 sibling, 1 reply; 4+ messages in thread
From: ronnie sahlberg @ 2021-09-22 21:33 UTC (permalink / raw)
  To: Namjae Jeon; +Cc: linux-cifs, Ralph Böhme, Steve French, Ronnie Sahlberg

reviewed by me

On Wed, Sep 22, 2021 at 10:01 PM Namjae Jeon <linkinjeon@kernel.org> wrote:
>
> When second smb2 pdu has invalid protocol id, ksmbd doesn't detect it
> and allow to process smb2 request. This patch add the check it in
> ksmbd_verify_smb_message() and don't use protocol id of smb2 request as
> protocol id of response.
>
> Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
> Cc: Ralph Böhme <slow@samba.org>
> Cc: Steve French <smfrench@gmail.com>
> Reported-by: Ronnie Sahlberg <lsahlber@redhat.com>
> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
> ---
>  fs/ksmbd/smb2pdu.c    |  2 +-
>  fs/ksmbd/smb_common.c | 13 +++++++++----
>  fs/ksmbd/smb_common.h |  1 +
>  3 files changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c
> index 3d250e2539e6..3be1493cb18d 100644
> --- a/fs/ksmbd/smb2pdu.c
> +++ b/fs/ksmbd/smb2pdu.c
> @@ -433,7 +433,7 @@ static void init_chained_smb2_rsp(struct ksmbd_work *work)
>                 work->compound_pfid = KSMBD_NO_FID;
>         }
>         memset((char *)rsp_hdr + 4, 0, sizeof(struct smb2_hdr) + 2);
> -       rsp_hdr->ProtocolId = rcv_hdr->ProtocolId;
> +       rsp_hdr->ProtocolId = SMB2_PROTO_NUMBER;
>         rsp_hdr->StructureSize = SMB2_HEADER_STRUCTURE_SIZE;
>         rsp_hdr->Command = rcv_hdr->Command;
>
> diff --git a/fs/ksmbd/smb_common.c b/fs/ksmbd/smb_common.c
> index da17b21ac685..ace8a1b02c81 100644
> --- a/fs/ksmbd/smb_common.c
> +++ b/fs/ksmbd/smb_common.c
> @@ -129,16 +129,22 @@ int ksmbd_lookup_protocol_idx(char *str)
>   *
>   * check for valid smb signature and packet direction(request/response)
>   *
> - * Return:      0 on success, otherwise 1
> + * Return:      0 on success, otherwise -EINVAL
>   */
>  int ksmbd_verify_smb_message(struct ksmbd_work *work)
>  {
> -       struct smb2_hdr *smb2_hdr = work->request_buf;
> +       struct smb2_hdr *smb2_hdr = work->request_buf + work->next_smb2_rcv_hdr_off;
> +       struct smb_hdr *hdr;
>
>         if (smb2_hdr->ProtocolId == SMB2_PROTO_NUMBER)
>                 return ksmbd_smb2_check_message(work);
>
> -       return 0;
> +       hdr = work->request_buf;
> +       if (*(__le32 *)hdr->Protocol == SMB1_PROTO_NUMBER &&
> +           hdr->Command == SMB_COM_NEGOTIATE)
> +               return 0;
> +
> +       return -EINVAL;
>  }
>
>  /**
> @@ -270,7 +276,6 @@ static int ksmbd_negotiate_smb_dialect(void *buf)
>         return BAD_PROT_ID;
>  }
>
> -#define SMB_COM_NEGOTIATE      0x72
>  int ksmbd_init_smb_server(struct ksmbd_work *work)
>  {
>         struct ksmbd_conn *conn = work->conn;
> diff --git a/fs/ksmbd/smb_common.h b/fs/ksmbd/smb_common.h
> index d7df19c97c4c..994abede27e9 100644
> --- a/fs/ksmbd/smb_common.h
> +++ b/fs/ksmbd/smb_common.h
> @@ -202,6 +202,7 @@
>                 FILE_READ_ATTRIBUTES | FILE_WRITE_ATTRIBUTES)
>
>  #define SMB1_PROTO_NUMBER              cpu_to_le32(0x424d53ff)
> +#define SMB_COM_NEGOTIATE              0x72
>
>  #define SMB1_CLIENT_GUID_SIZE          (16)
>  struct smb_hdr {
> --
> 2.25.1
>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] ksmbd: check protocol id in ksmbd_verify_smb_message()
  2021-09-22 21:33 ` ronnie sahlberg
@ 2021-09-22 22:24   ` Steve French
  0 siblings, 0 replies; 4+ messages in thread
From: Steve French @ 2021-09-22 22:24 UTC (permalink / raw)
  To: ronnie sahlberg
  Cc: Namjae Jeon, linux-cifs, Ralph Böhme, Ronnie Sahlberg

added the R-Bs and merged into cifsd-for-next  (current content is
below, although looks like we could update the "buffer invalidation in
smb2_set_info" patch)

e6201b4a0bac (HEAD -> cifsd-for-next, origin/cifsd-for-next) ksmbd:
add request buffer validation in smb2_set_info
743d886affeb ksmbd: remove follow symlinks support
3bee78ad0062 ksmbd: fix invalid request buffer access in compound request
18a015bccf9e ksmbd: check protocol id in ksmbd_verify_smb_message()
9f6323311c70 ksmbd: add default data stream name in FILE_STREAM_INFORMATION
e44fd5081c50 ksmbd: log that server is experimental at module load

On Wed, Sep 22, 2021 at 4:33 PM ronnie sahlberg
<ronniesahlberg@gmail.com> wrote:
>
> reviewed by me
>
> On Wed, Sep 22, 2021 at 10:01 PM Namjae Jeon <linkinjeon@kernel.org> wrote:
> >
> > When second smb2 pdu has invalid protocol id, ksmbd doesn't detect it
> > and allow to process smb2 request. This patch add the check it in
> > ksmbd_verify_smb_message() and don't use protocol id of smb2 request as
> > protocol id of response.
> >
> > Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
> > Cc: Ralph Böhme <slow@samba.org>
> > Cc: Steve French <smfrench@gmail.com>
> > Reported-by: Ronnie Sahlberg <lsahlber@redhat.com>
> > Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
> > ---
> >  fs/ksmbd/smb2pdu.c    |  2 +-
> >  fs/ksmbd/smb_common.c | 13 +++++++++----
> >  fs/ksmbd/smb_common.h |  1 +
> >  3 files changed, 11 insertions(+), 5 deletions(-)
> >
> > diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c
> > index 3d250e2539e6..3be1493cb18d 100644
> > --- a/fs/ksmbd/smb2pdu.c
> > +++ b/fs/ksmbd/smb2pdu.c
> > @@ -433,7 +433,7 @@ static void init_chained_smb2_rsp(struct ksmbd_work *work)
> >                 work->compound_pfid = KSMBD_NO_FID;
> >         }
> >         memset((char *)rsp_hdr + 4, 0, sizeof(struct smb2_hdr) + 2);
> > -       rsp_hdr->ProtocolId = rcv_hdr->ProtocolId;
> > +       rsp_hdr->ProtocolId = SMB2_PROTO_NUMBER;
> >         rsp_hdr->StructureSize = SMB2_HEADER_STRUCTURE_SIZE;
> >         rsp_hdr->Command = rcv_hdr->Command;
> >
> > diff --git a/fs/ksmbd/smb_common.c b/fs/ksmbd/smb_common.c
> > index da17b21ac685..ace8a1b02c81 100644
> > --- a/fs/ksmbd/smb_common.c
> > +++ b/fs/ksmbd/smb_common.c
> > @@ -129,16 +129,22 @@ int ksmbd_lookup_protocol_idx(char *str)
> >   *
> >   * check for valid smb signature and packet direction(request/response)
> >   *
> > - * Return:      0 on success, otherwise 1
> > + * Return:      0 on success, otherwise -EINVAL
> >   */
> >  int ksmbd_verify_smb_message(struct ksmbd_work *work)
> >  {
> > -       struct smb2_hdr *smb2_hdr = work->request_buf;
> > +       struct smb2_hdr *smb2_hdr = work->request_buf + work->next_smb2_rcv_hdr_off;
> > +       struct smb_hdr *hdr;
> >
> >         if (smb2_hdr->ProtocolId == SMB2_PROTO_NUMBER)
> >                 return ksmbd_smb2_check_message(work);
> >
> > -       return 0;
> > +       hdr = work->request_buf;
> > +       if (*(__le32 *)hdr->Protocol == SMB1_PROTO_NUMBER &&
> > +           hdr->Command == SMB_COM_NEGOTIATE)
> > +               return 0;
> > +
> > +       return -EINVAL;
> >  }
> >
> >  /**
> > @@ -270,7 +276,6 @@ static int ksmbd_negotiate_smb_dialect(void *buf)
> >         return BAD_PROT_ID;
> >  }
> >
> > -#define SMB_COM_NEGOTIATE      0x72
> >  int ksmbd_init_smb_server(struct ksmbd_work *work)
> >  {
> >         struct ksmbd_conn *conn = work->conn;
> > diff --git a/fs/ksmbd/smb_common.h b/fs/ksmbd/smb_common.h
> > index d7df19c97c4c..994abede27e9 100644
> > --- a/fs/ksmbd/smb_common.h
> > +++ b/fs/ksmbd/smb_common.h
> > @@ -202,6 +202,7 @@
> >                 FILE_READ_ATTRIBUTES | FILE_WRITE_ATTRIBUTES)
> >
> >  #define SMB1_PROTO_NUMBER              cpu_to_le32(0x424d53ff)
> > +#define SMB_COM_NEGOTIATE              0x72
> >
> >  #define SMB1_CLIENT_GUID_SIZE          (16)
> >  struct smb_hdr {
> > --
> > 2.25.1
> >



-- 
Thanks,

Steve

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-09-22 22:24 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-22 12:00 [PATCH] ksmbd: check protocol id in ksmbd_verify_smb_message() Namjae Jeon
2021-09-22 14:21 ` Ralph Boehme
2021-09-22 21:33 ` ronnie sahlberg
2021-09-22 22:24   ` Steve French

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.