The Samba-Bugzilla – Attachment 15634 Details for
Bug 14205
Prevent smbd crash after invalid SMB1 negprot.
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
git-am fix for master.
bug-14205-master (text/plain), 15.60 KB, created by
Jeremy Allison
on 2019-11-26 21:31:56 UTC
(
hide
)
Description:
git-am fix for master.
Filename:
MIME Type:
Creator:
Jeremy Allison
Created:
2019-11-26 21:31:56 UTC
Size:
15.60 KB
patch
obsolete
>From 37b2a73cd7499a78fc4adb1ffd86b53d829a385b Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Tue, 26 Nov 2019 12:14:29 -0800 >Subject: [PATCH 1/6] s3: smbd: Allow smbd_smb2_process_negprot() to return > NTSTATUS as it can fail. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=14205 > >Signed-off-by: Jeremy Allison <jra@samba.org> >--- > source3/smbd/globals.h | 2 +- > source3/smbd/smb2_server.c | 15 ++++++++------- > 2 files changed, 9 insertions(+), 8 deletions(-) > >diff --git a/source3/smbd/globals.h b/source3/smbd/globals.h >index c791eb0fa6f..a5e02ebc96a 100644 >--- a/source3/smbd/globals.h >+++ b/source3/smbd/globals.h >@@ -234,7 +234,7 @@ NTSTATUS smbd_add_connection(struct smbXsrv_client *client, int sock_fd, > > void reply_smb2002(struct smb_request *req, uint16_t choice); > void reply_smb20ff(struct smb_request *req, uint16_t choice); >-void smbd_smb2_process_negprot(struct smbXsrv_connection *xconn, >+NTSTATUS smbd_smb2_process_negprot(struct smbXsrv_connection *xconn, > uint64_t expected_seq_low, > const uint8_t *inpdu, size_t size); > >diff --git a/source3/smbd/smb2_server.c b/source3/smbd/smb2_server.c >index 7044ecb991a..9cf8841d827 100644 >--- a/source3/smbd/smb2_server.c >+++ b/source3/smbd/smb2_server.c >@@ -3629,7 +3629,7 @@ static NTSTATUS smbd_smb2_request_next_incoming(struct smbXsrv_connection *xconn > return NT_STATUS_OK; > } > >-void smbd_smb2_process_negprot(struct smbXsrv_connection *xconn, >+NTSTATUS smbd_smb2_process_negprot(struct smbXsrv_connection *xconn, > uint64_t expected_seq_low, > const uint8_t *inpdu, size_t size) > { >@@ -3643,25 +3643,25 @@ void smbd_smb2_process_negprot(struct smbXsrv_connection *xconn, > status = smbd_initialize_smb2(xconn, expected_seq_low); > if (!NT_STATUS_IS_OK(status)) { > smbd_server_connection_terminate(xconn, nt_errstr(status)); >- return; >+ return status; > } > > status = smbd_smb2_request_create(xconn, inpdu, size, &req); > if (!NT_STATUS_IS_OK(status)) { > smbd_server_connection_terminate(xconn, nt_errstr(status)); >- return; >+ return status; > } > > status = smbd_smb2_request_validate(req); > if (!NT_STATUS_IS_OK(status)) { > smbd_server_connection_terminate(xconn, nt_errstr(status)); >- return; >+ return status; > } > > status = smbd_smb2_request_setup_out(req); > if (!NT_STATUS_IS_OK(status)) { > smbd_server_connection_terminate(xconn, nt_errstr(status)); >- return; >+ return status; > } > > #ifdef WITH_PROFILE >@@ -3676,16 +3676,17 @@ void smbd_smb2_process_negprot(struct smbXsrv_connection *xconn, > status = smbd_smb2_request_dispatch(req); > if (!NT_STATUS_IS_OK(status)) { > smbd_server_connection_terminate(xconn, nt_errstr(status)); >- return; >+ return status; > } > > status = smbd_smb2_request_next_incoming(xconn); > if (!NT_STATUS_IS_OK(status)) { > smbd_server_connection_terminate(xconn, nt_errstr(status)); >- return; >+ return status; > } > > sconn->num_requests++; >+ return NT_STATUS_OK; > } > > static int socket_error_from_errno(int ret, >-- >2.24.0.432.g9d3f5f5b63-goog > > >From 05400c24099697959763ff584b5d54e7a66a2a15 Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Tue, 26 Nov 2019 12:17:29 -0800 >Subject: [PATCH 2/6] s3: smbd: Ensure we exit on smbd_smb2_process_negprot() > fail. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=14205 > >Signed-off-by: Jeremy Allison <jra@samba.org> >--- > source3/smbd/process.c | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) > >diff --git a/source3/smbd/process.c b/source3/smbd/process.c >index 0e0d45d2af1..fc43519a614 100644 >--- a/source3/smbd/process.c >+++ b/source3/smbd/process.c >@@ -1968,7 +1968,14 @@ static void process_smb(struct smbXsrv_connection *xconn, > if (smbd_is_smb2_header(inbuf, nread)) { > const uint8_t *inpdu = inbuf + NBT_HDR_SIZE; > size_t pdulen = nread - NBT_HDR_SIZE; >- smbd_smb2_process_negprot(xconn, 0, inpdu, pdulen); >+ NTSTATUS status = smbd_smb2_process_negprot( >+ xconn, >+ 0, >+ inpdu, >+ pdulen); >+ if (!NT_STATUS_IS_OK(status)) { >+ exit_server_cleanly("SMB2 negprot fail"); >+ } > return; > } > if (nread >= smb_size && valid_smb_header(inbuf) >-- >2.24.0.432.g9d3f5f5b63-goog > > >From 265399109ac0f0a46e8f7da41b6cbd59e5467d48 Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Tue, 26 Nov 2019 12:21:06 -0800 >Subject: [PATCH 3/6] s3: smbd: Change reply_smb20xx() to return NTSTATUS. > >Not yet used. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=14205 > >Signed-off-by: Jeremy Allison <jra@samba.org> >--- > source3/smbd/smb2_negprot.c | 7 +++---- > 1 file changed, 3 insertions(+), 4 deletions(-) > >diff --git a/source3/smbd/smb2_negprot.c b/source3/smbd/smb2_negprot.c >index 6e7201b1cd8..b07fc972cbb 100644 >--- a/source3/smbd/smb2_negprot.c >+++ b/source3/smbd/smb2_negprot.c >@@ -36,7 +36,7 @@ extern fstring remote_proto; > * this is the entry point if SMB2 is selected via > * the SMB negprot and the given dialect. > */ >-static void reply_smb20xx(struct smb_request *req, uint16_t dialect) >+static NTSTATUS reply_smb20xx(struct smb_request *req, uint16_t dialect) > { > uint8_t *smb2_inpdu; > uint8_t *smb2_hdr; >@@ -48,7 +48,7 @@ static void reply_smb20xx(struct smb_request *req, uint16_t dialect) > if (smb2_inpdu == NULL) { > DEBUG(0, ("Could not push spnego blob\n")); > reply_nterror(req, NT_STATUS_NO_MEMORY); >- return; >+ return NT_STATUS_NO_MEMORY; > } > smb2_hdr = smb2_inpdu; > smb2_body = smb2_hdr + SMB2_HDR_BODY; >@@ -64,8 +64,7 @@ static void reply_smb20xx(struct smb_request *req, uint16_t dialect) > > req->outbuf = NULL; > >- smbd_smb2_process_negprot(req->xconn, 0, smb2_inpdu, len); >- return; >+ return smbd_smb2_process_negprot(req->xconn, 0, smb2_inpdu, len); > } > > /* >-- >2.24.0.432.g9d3f5f5b63-goog > > >From b9be8393a9ee134d3f08770b92a712e677664a9e Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Tue, 26 Nov 2019 12:43:25 -0800 >Subject: [PATCH 4/6] s3: smbd: Change (*proto_reply_fn()) to return an > NTSTATUS. > >That way the caller can know if the negprot really >succeeded or not. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=14205 > >Signed-off-by: Jeremy Allison <jra@samba.org> >--- > source3/smbd/globals.h | 4 ++-- > source3/smbd/negprot.c | 29 +++++++++++++++-------------- > source3/smbd/smb2_negprot.c | 8 ++++---- > 3 files changed, 21 insertions(+), 20 deletions(-) > >diff --git a/source3/smbd/globals.h b/source3/smbd/globals.h >index a5e02ebc96a..e6641837668 100644 >--- a/source3/smbd/globals.h >+++ b/source3/smbd/globals.h >@@ -232,8 +232,8 @@ bool smbd_smb2_is_compound(const struct smbd_smb2_request *req); > NTSTATUS smbd_add_connection(struct smbXsrv_client *client, int sock_fd, > struct smbXsrv_connection **_xconn); > >-void reply_smb2002(struct smb_request *req, uint16_t choice); >-void reply_smb20ff(struct smb_request *req, uint16_t choice); >+NTSTATUS reply_smb2002(struct smb_request *req, uint16_t choice); >+NTSTATUS reply_smb20ff(struct smb_request *req, uint16_t choice); > NTSTATUS smbd_smb2_process_negprot(struct smbXsrv_connection *xconn, > uint64_t expected_seq_low, > const uint8_t *inpdu, size_t size); >diff --git a/source3/smbd/negprot.c b/source3/smbd/negprot.c >index 2d5edf1282c..3b2555e3d21 100644 >--- a/source3/smbd/negprot.c >+++ b/source3/smbd/negprot.c >@@ -66,7 +66,7 @@ static void get_challenge(struct smbXsrv_connection *xconn, uint8_t buff[8]) > Reply for the lanman 1.0 protocol. > ****************************************************************************/ > >-static void reply_lanman1(struct smb_request *req, uint16_t choice) >+static NTSTATUS reply_lanman1(struct smb_request *req, uint16_t choice) > { > int secword=0; > time_t t = time(NULL); >@@ -100,7 +100,7 @@ static void reply_lanman1(struct smb_request *req, uint16_t choice) > status = smbXsrv_connection_init_tables(xconn, PROTOCOL_LANMAN1); > if (!NT_STATUS_IS_OK(status)) { > reply_nterror(req, status); >- return; >+ return status; > } > > /* Reply, SMBlockread, SMBwritelock supported. */ >@@ -115,14 +115,14 @@ static void reply_lanman1(struct smb_request *req, uint16_t choice) > > srv_put_dos_date((char *)req->outbuf,smb_vwv8,t); > >- return; >+ return NT_STATUS_OK; > } > > /**************************************************************************** > Reply for the lanman 2.0 protocol. > ****************************************************************************/ > >-static void reply_lanman2(struct smb_request *req, uint16_t choice) >+static NTSTATUS reply_lanman2(struct smb_request *req, uint16_t choice) > { > int secword=0; > time_t t = time(NULL); >@@ -158,7 +158,7 @@ static void reply_lanman2(struct smb_request *req, uint16_t choice) > status = smbXsrv_connection_init_tables(xconn, PROTOCOL_LANMAN2); > if (!NT_STATUS_IS_OK(status)) { > reply_nterror(req, status); >- return; >+ return status; > } > > /* Reply, SMBlockread, SMBwritelock supported. */ >@@ -169,6 +169,7 @@ static void reply_lanman2(struct smb_request *req, uint16_t choice) > SSVAL(req->outbuf,smb_vwv5,raw); /* readbraw and/or writebraw */ > SSVAL(req->outbuf,smb_vwv10, set_server_zone_offset(t)/60); > srv_put_dos_date((char *)req->outbuf,smb_vwv8,t); >+ return NT_STATUS_OK; > } > > /**************************************************************************** >@@ -266,7 +267,7 @@ DATA_BLOB negprot_spnego(TALLOC_CTX *ctx, struct smbXsrv_connection *xconn) > Reply for the nt protocol. > ****************************************************************************/ > >-static void reply_nt1(struct smb_request *req, uint16_t choice) >+static NTSTATUS reply_nt1(struct smb_request *req, uint16_t choice) > { > /* dual names + lock_and_read + nt SMBs + remote API calls */ > int capabilities = CAP_NT_FIND|CAP_LOCK_AND_READ| >@@ -359,7 +360,7 @@ static void reply_nt1(struct smb_request *req, uint16_t choice) > status = smbXsrv_connection_init_tables(xconn, PROTOCOL_NT1); > if (!NT_STATUS_IS_OK(status)) { > reply_nterror(req, status); >- return; >+ return status; > } > > SSVAL(req->outbuf,smb_vwv1+1, lp_max_mux()); /* maxmpx */ >@@ -385,7 +386,7 @@ static void reply_nt1(struct smb_request *req, uint16_t choice) > if (ret == -1) { > DEBUG(0, ("Could not push challenge\n")); > reply_nterror(req, NT_STATUS_NO_MEMORY); >- return; >+ return NT_STATUS_NO_MEMORY; > } > SCVAL(req->outbuf, smb_vwv16+1, ret); > } >@@ -395,7 +396,7 @@ static void reply_nt1(struct smb_request *req, uint16_t choice) > if (ret == -1) { > DEBUG(0, ("Could not push workgroup string\n")); > reply_nterror(req, NT_STATUS_NO_MEMORY); >- return; >+ return NT_STATUS_NO_MEMORY; > } > ret = message_push_string(&req->outbuf, lp_netbios_name(), > STR_UNICODE|STR_TERMINATE >@@ -403,7 +404,7 @@ static void reply_nt1(struct smb_request *req, uint16_t choice) > if (ret == -1) { > DEBUG(0, ("Could not push netbios name string\n")); > reply_nterror(req, NT_STATUS_NO_MEMORY); >- return; >+ return NT_STATUS_NO_MEMORY; > } > DEBUG(3,("not using SPNEGO\n")); > } else { >@@ -411,14 +412,14 @@ static void reply_nt1(struct smb_request *req, uint16_t choice) > > if (spnego_blob.data == NULL) { > reply_nterror(req, NT_STATUS_NO_MEMORY); >- return; >+ return NT_STATUS_NO_MEMORY; > } > > ret = message_push_blob(&req->outbuf, spnego_blob); > if (ret == -1) { > DEBUG(0, ("Could not push spnego blob\n")); > reply_nterror(req, NT_STATUS_NO_MEMORY); >- return; >+ return NT_STATUS_NO_MEMORY; > } > data_blob_free(&spnego_blob); > >@@ -426,7 +427,7 @@ static void reply_nt1(struct smb_request *req, uint16_t choice) > DEBUG(3,("using SPNEGO\n")); > } > >- return; >+ return NT_STATUS_OK; > } > > /* these are the protocol lists used for auto architecture detection: >@@ -540,7 +541,7 @@ protocol [SMB 2.???] > static const struct { > const char *proto_name; > const char *short_name; >- void (*proto_reply_fn)(struct smb_request *req, uint16_t choice); >+ NTSTATUS (*proto_reply_fn)(struct smb_request *req, uint16_t choice); > int protocol_level; > } supported_protocols[] = { > {"SMB 2.???", "SMB2_FF", reply_smb20ff, PROTOCOL_SMB2_10}, >diff --git a/source3/smbd/smb2_negprot.c b/source3/smbd/smb2_negprot.c >index b07fc972cbb..2c1d4185b28 100644 >--- a/source3/smbd/smb2_negprot.c >+++ b/source3/smbd/smb2_negprot.c >@@ -71,20 +71,20 @@ static NTSTATUS reply_smb20xx(struct smb_request *req, uint16_t dialect) > * this is the entry point if SMB2 is selected via > * the SMB negprot and the "SMB 2.002" dialect. > */ >-void reply_smb2002(struct smb_request *req, uint16_t choice) >+NTSTATUS reply_smb2002(struct smb_request *req, uint16_t choice) > { >- reply_smb20xx(req, SMB2_DIALECT_REVISION_202); >+ return reply_smb20xx(req, SMB2_DIALECT_REVISION_202); > } > > /* > * this is the entry point if SMB2 is selected via > * the SMB negprot and the "SMB 2.???" dialect. > */ >-void reply_smb20ff(struct smb_request *req, uint16_t choice) >+NTSTATUS reply_smb20ff(struct smb_request *req, uint16_t choice) > { > struct smbXsrv_connection *xconn = req->xconn; > xconn->smb2.allow_2ff = true; >- reply_smb20xx(req, SMB2_DIALECT_REVISION_2FF); >+ return reply_smb20xx(req, SMB2_DIALECT_REVISION_2FF); > } > > enum protocol_types smbd_smb2_protocol_dialect_match(const uint8_t *indyn, >-- >2.24.0.432.g9d3f5f5b63-goog > > >From f4a3443ab5e9f0a90931b7c67a8d5fc2956d4204 Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Tue, 26 Nov 2019 12:46:16 -0800 >Subject: [PATCH 5/6] s3: smbd: Ensure we exit if > supported_protocols[protocol].proto_reply_fn() fails. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=14205 > >Signed-off-by: Jeremy Allison <jra@samba.org> >--- > source3/smbd/negprot.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > >diff --git a/source3/smbd/negprot.c b/source3/smbd/negprot.c >index 3b2555e3d21..8317dc49086 100644 >--- a/source3/smbd/negprot.c >+++ b/source3/smbd/negprot.c >@@ -580,6 +580,7 @@ void reply_negprot(struct smb_request *req) > bool signing_required = true; > int max_proto; > int min_proto; >+ NTSTATUS status; > > START_PROFILE(SMBnegprot); > >@@ -768,7 +769,11 @@ void reply_negprot(struct smb_request *req) > > fstrcpy(remote_proto,supported_protocols[protocol].short_name); > reload_services(sconn, conn_snum_used, true); >- supported_protocols[protocol].proto_reply_fn(req, choice); >+ status = supported_protocols[protocol].proto_reply_fn(req, choice); >+ if (!NT_STATUS_IS_OK(status)) { >+ exit_server_cleanly("negprot function failed\n"); >+ } >+ > DEBUG(3,("Selected protocol %s\n",supported_protocols[protocol].proto_name)); > > DBG_INFO("negprot index=%zu\n", choice); >-- >2.24.0.432.g9d3f5f5b63-goog > > >From 1d2f9adaca98f71a81050a723dde3798959a4fc1 Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Tue, 26 Nov 2019 12:53:09 -0800 >Subject: [PATCH 6/6] s3: smbd: Only set xconn->smb1.negprot.done = true after > supported_protocols[protocol].proto_reply_fn() succeeds. > >Otherwise we can end up with negprot.done set, but >without smbXsrv_connection_init_tables() being called. > >This can cause a client self-crash. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=14205 > >Signed-off-by: Jeremy Allison <jra@samba.org> >--- > source3/smbd/negprot.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > >diff --git a/source3/smbd/negprot.c b/source3/smbd/negprot.c >index 8317dc49086..e77c8f52261 100644 >--- a/source3/smbd/negprot.c >+++ b/source3/smbd/negprot.c >@@ -588,7 +588,6 @@ void reply_negprot(struct smb_request *req) > END_PROFILE(SMBnegprot); > exit_server_cleanly("multiple negprot's are not permitted"); > } >- xconn->smb1.negprot.done = true; > > if (req->buflen == 0) { > DEBUG(0, ("negprot got no protocols\n")); >@@ -778,6 +777,8 @@ void reply_negprot(struct smb_request *req) > > DBG_INFO("negprot index=%zu\n", choice); > >+ xconn->smb1.negprot.done = true; >+ > /* We always have xconn->smb1.signing_state also for >= SMB2_02 */ > signing_required = smb_signing_is_mandatory(xconn->smb1.signing_state); > if (signing_required && (chosen_level < PROTOCOL_NT1)) { >-- >2.24.0.432.g9d3f5f5b63-goog >
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Raw
Actions:
View
Attachments on
bug 14205
:
15634
|
15667
|
15668