From d834c875b2638daf8926f09656643063f1eb824b Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Tue, 13 Jul 2021 16:37:42 +0200 Subject: [PATCH 1/8] s3:smbd: remove dead code from smbd_smb2_request_dispatch() We have '} else if (signing_required || (flags & SMB2_HDR_FLAG_SIGNED)) {' before... Use 'git show -U52' to see the whole story... Signed-off-by: Stefan Metzmacher Reviewed-by: Jeremy Allison (cherry picked from commit f8f4a9faf099eb768eaa25f1e1a7d126b75291d0) BUG: https://bugzilla.samba.org/show_bug.cgi?id=14788 --- source3/smbd/smb2_server.c | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/source3/smbd/smb2_server.c b/source3/smbd/smb2_server.c index 1322cf3ba083..f071c1a5ff98 100644 --- a/source3/smbd/smb2_server.c +++ b/source3/smbd/smb2_server.c @@ -2717,9 +2717,6 @@ static void smb2srv_update_crypto_flags(struct smbd_smb2_request *req, /* Unencrypted packet, can be signed */ if (req->do_signing) { sign_flag = SMBXSRV_PROCESSED_SIGNED_PACKET; - } else if (opcode == SMB2_OP_CANCEL) { - /* Cancel requests are allowed to skip signing */ - sign_flag &= ~SMBXSRV_PROCESSED_UNSIGNED_PACKET; } } @@ -3080,8 +3077,6 @@ NTSTATUS smbd_smb2_request_dispatch(struct smbd_smb2_request *req) if (!NT_STATUS_IS_OK(session_status)) { return smbd_smb2_request_error(req, session_status); } - } else if (opcode == SMB2_OP_CANCEL) { - /* Cancel requests are allowed to skip the signing */ } else if (opcode == SMB2_OP_IOCTL) { /* * Some special IOCTL calls don't require @@ -3118,13 +3113,6 @@ NTSTATUS smbd_smb2_request_dispatch(struct smbd_smb2_request *req) call = &_root_ioctl_call; break; } - } else if (signing_required) { - /* - * If signing is required we try to sign - * a possible error response - */ - req->do_signing = true; - return smbd_smb2_request_error(req, NT_STATUS_ACCESS_DENIED); } if (flags & SMB2_HDR_FLAG_CHAINED) { -- 2.25.1 From 439db797d562d022aac7d39cfadb31d453437c2f Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Thu, 16 Sep 2021 10:51:43 +0200 Subject: [PATCH 2/8] libcli/smb: split out smb2cli_raw_tcon* from smb2cli_tcon* This will be used in tests in order to separate the tcon from validate_negotiate_info. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14788 Signed-off-by: Stefan Metzmacher Reviewed-by: Ralph Boehme (cherry picked from commit 04a79139a42cfd1b607317dec041618cbf629584) --- libcli/smb/smb2cli_tcon.c | 183 ++++++++++++++++++++++++++++++-------- libcli/smb/smbXcli_base.h | 20 +++++ 2 files changed, 168 insertions(+), 35 deletions(-) diff --git a/libcli/smb/smb2cli_tcon.c b/libcli/smb/smb2cli_tcon.c index 8863bae0764c..7bbae8ea3b3b 100644 --- a/libcli/smb/smb2cli_tcon.c +++ b/libcli/smb/smb2cli_tcon.c @@ -23,42 +23,38 @@ #include "../libcli/smb/smb_common.h" #include "../libcli/smb/smbXcli_base.h" -struct smb2cli_tcon_state { - struct tevent_context *ev; - struct smbXcli_conn *conn; - uint32_t timeout_msec; +struct smb2cli_raw_tcon_state { struct smbXcli_session *session; struct smbXcli_tcon *tcon; uint8_t fixed[8]; uint8_t dyn_pad[1]; }; -static void smb2cli_tcon_done(struct tevent_req *subreq); - -struct tevent_req *smb2cli_tcon_send(TALLOC_CTX *mem_ctx, - struct tevent_context *ev, - struct smbXcli_conn *conn, - uint32_t timeout_msec, - struct smbXcli_session *session, - struct smbXcli_tcon *tcon, - uint16_t flags, - const char *unc) +static void smb2cli_raw_tcon_done(struct tevent_req *subreq); + +struct tevent_req *smb2cli_raw_tcon_send(TALLOC_CTX *mem_ctx, + struct tevent_context *ev, + struct smbXcli_conn *conn, + uint32_t additional_flags, + uint32_t clear_flags, + uint32_t timeout_msec, + struct smbXcli_session *session, + struct smbXcli_tcon *tcon, + uint16_t tcon_flags, + const char *unc) { - struct tevent_req *req, *subreq; - struct smb2cli_tcon_state *state; - uint8_t *fixed; - uint8_t *dyn; + struct tevent_req *req = NULL; + struct smb2cli_raw_tcon_state *state = NULL; + struct tevent_req *subreq = NULL; + uint8_t *fixed = NULL; + uint8_t *dyn = NULL; size_t dyn_len; - uint32_t additional_flags = 0; - uint32_t clear_flags = 0; - req = tevent_req_create(mem_ctx, &state, struct smb2cli_tcon_state); + req = tevent_req_create(mem_ctx, &state, + struct smb2cli_raw_tcon_state); if (req == NULL) { return NULL; } - state->ev = ev; - state->conn = conn; - state->timeout_msec = timeout_msec; state->session = session; state->tcon = tcon; @@ -77,7 +73,7 @@ struct tevent_req *smb2cli_tcon_send(TALLOC_CTX *mem_ctx, fixed = state->fixed; SSVAL(fixed, 0, 9); if (smbXcli_conn_protocol(conn) >= PROTOCOL_SMB3_10) { - SSVAL(fixed, 2, flags); + SSVAL(fixed, 2, tcon_flags); } else { SSVAL(fixed, 2, 0); /* Reserved */ } @@ -89,10 +85,6 @@ struct tevent_req *smb2cli_tcon_send(TALLOC_CTX *mem_ctx, dyn_len = sizeof(state->dyn_pad); } - if (smbXcli_session_is_authenticated(state->session)) { - additional_flags |= SMB2_HDR_FLAG_SIGNED; - } - subreq = smb2cli_req_send(state, ev, conn, SMB2_OP_TCON, additional_flags, clear_flags, timeout_msec, @@ -104,19 +96,17 @@ struct tevent_req *smb2cli_tcon_send(TALLOC_CTX *mem_ctx, if (tevent_req_nomem(subreq, req)) { return tevent_req_post(req, ev); } - tevent_req_set_callback(subreq, smb2cli_tcon_done, req); + tevent_req_set_callback(subreq, smb2cli_raw_tcon_done, req); return req; } -static void smb2cli_tcon_validate(struct tevent_req *subreq); - -static void smb2cli_tcon_done(struct tevent_req *subreq) +static void smb2cli_raw_tcon_done(struct tevent_req *subreq) { struct tevent_req *req = tevent_req_callback_data( subreq, struct tevent_req); - struct smb2cli_tcon_state *state = tevent_req_data( - req, struct smb2cli_tcon_state); + struct smb2cli_raw_tcon_state *state = tevent_req_data( + req, struct smb2cli_raw_tcon_state); NTSTATUS status; struct iovec *iov; uint8_t *body; @@ -156,6 +146,129 @@ static void smb2cli_tcon_done(struct tevent_req *subreq) share_capabilities, maximal_access); + tevent_req_done(req); +} + +NTSTATUS smb2cli_raw_tcon_recv(struct tevent_req *req) +{ + return tevent_req_simple_recv_ntstatus(req); +} + +NTSTATUS smb2cli_raw_tcon(struct smbXcli_conn *conn, + uint32_t additional_flags, + uint32_t clear_flags, + uint32_t timeout_msec, + struct smbXcli_session *session, + struct smbXcli_tcon *tcon, + uint16_t tcon_flags, + const char *unc) +{ + TALLOC_CTX *frame = talloc_stackframe(); + struct tevent_context *ev; + struct tevent_req *req; + NTSTATUS status = NT_STATUS_NO_MEMORY; + + if (smbXcli_conn_has_async_calls(conn)) { + /* + * Can't use sync call while an async call is in flight + */ + status = NT_STATUS_INVALID_PARAMETER; + goto fail; + } + ev = samba_tevent_context_init(frame); + if (ev == NULL) { + goto fail; + } + req = smb2cli_raw_tcon_send(frame, ev, conn, + additional_flags, clear_flags, + timeout_msec, session, tcon, + tcon_flags, unc); + if (req == NULL) { + goto fail; + } + if (!tevent_req_poll_ntstatus(req, ev, &status)) { + goto fail; + } + status = smb2cli_raw_tcon_recv(req); + fail: + TALLOC_FREE(frame); + return status; +} + +struct smb2cli_tcon_state { + struct tevent_context *ev; + struct smbXcli_conn *conn; + uint32_t timeout_msec; + struct smbXcli_session *session; + struct smbXcli_tcon *tcon; + uint8_t fixed[8]; + uint8_t dyn_pad[1]; +}; + +static void smb2cli_tcon_done(struct tevent_req *subreq); + +struct tevent_req *smb2cli_tcon_send(TALLOC_CTX *mem_ctx, + struct tevent_context *ev, + struct smbXcli_conn *conn, + uint32_t timeout_msec, + struct smbXcli_session *session, + struct smbXcli_tcon *tcon, + uint16_t flags, + const char *unc) +{ + struct tevent_req *req, *subreq; + struct smb2cli_tcon_state *state; + uint32_t additional_flags = 0; + uint32_t clear_flags = 0; + + req = tevent_req_create(mem_ctx, &state, struct smb2cli_tcon_state); + if (req == NULL) { + return NULL; + } + state->ev = ev; + state->conn = conn; + state->timeout_msec = timeout_msec; + state->session = session; + state->tcon = tcon; + + if (smbXcli_session_is_authenticated(state->session)) { + additional_flags |= SMB2_HDR_FLAG_SIGNED; + } + + subreq = smb2cli_raw_tcon_send(state, + state->ev, + state->conn, + additional_flags, + clear_flags, + state->timeout_msec, + state->session, + state->tcon, + flags, + unc); + if (tevent_req_nomem(subreq, req)) { + return tevent_req_post(req, ev); + } + tevent_req_set_callback(subreq, smb2cli_tcon_done, req); + + return req; +} + +static void smb2cli_tcon_validate(struct tevent_req *subreq); + +static void smb2cli_tcon_done(struct tevent_req *subreq) +{ + struct tevent_req *req = tevent_req_callback_data( + subreq, struct tevent_req); + struct smb2cli_tcon_state *state = tevent_req_data( + req, struct smb2cli_tcon_state); + NTSTATUS status; + + status = smb2cli_raw_tcon_recv(subreq); + TALLOC_FREE(subreq); + if (tevent_req_nterror(req, status)) { + return; + } + if (!smbXcli_session_is_authenticated(state->session)) { tevent_req_done(req); return; diff --git a/libcli/smb/smbXcli_base.h b/libcli/smb/smbXcli_base.h index ac382b2ae6af..75e758f5a847 100644 --- a/libcli/smb/smbXcli_base.h +++ b/libcli/smb/smbXcli_base.h @@ -589,6 +589,26 @@ NTSTATUS smb2cli_logoff(struct smbXcli_conn *conn, uint32_t timeout_msec, struct smbXcli_session *session); +/* smb2cli_raw_tcon* should only be used in tests! */ +struct tevent_req *smb2cli_raw_tcon_send(TALLOC_CTX *mem_ctx, + struct tevent_context *ev, + struct smbXcli_conn *conn, + uint32_t additional_flags, + uint32_t clear_flags, + uint32_t timeout_msec, + struct smbXcli_session *session, + struct smbXcli_tcon *tcon, + uint16_t tcon_flags, + const char *unc); +NTSTATUS smb2cli_raw_tcon_recv(struct tevent_req *req); +NTSTATUS smb2cli_raw_tcon(struct smbXcli_conn *conn, + uint32_t additional_flags, + uint32_t clear_flags, + uint32_t timeout_msec, + struct smbXcli_session *session, + struct smbXcli_tcon *tcon, + uint16_t tcon_flags, + const char *unc); struct tevent_req *smb2cli_tcon_send(TALLOC_CTX *mem_ctx, struct tevent_context *ev, struct smbXcli_conn *conn, -- 2.25.1 From a8ad71c82ff0f714b77cf371420e21070116a652 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Wed, 15 Sep 2021 18:31:06 +0200 Subject: [PATCH 3/8] s4:torture/smb2: add smb2.ioctl.bug14788.VALIDATE_NEGOTIATE Demonstrate that smbd fails FSCTL_VALIDATE_NEGOTIATE_INFO only because the user doesn't have permissions on the share root. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14788 Signed-off-by: Stefan Metzmacher Reviewed-by: Ralph Boehme (cherry picked from commit 735fc34682c541056fd912d07c69f299f961983c) --- selftest/knownfail.d/smb2.ioctl.bug14788 | 1 + source4/torture/smb2/ioctl.c | 111 +++++++++++++++++++++++ 2 files changed, 112 insertions(+) create mode 100644 selftest/knownfail.d/smb2.ioctl.bug14788 diff --git a/selftest/knownfail.d/smb2.ioctl.bug14788 b/selftest/knownfail.d/smb2.ioctl.bug14788 new file mode 100644 index 000000000000..843e7d815176 --- /dev/null +++ b/selftest/knownfail.d/smb2.ioctl.bug14788 @@ -0,0 +1 @@ +^samba3.smb2.ioctl.*.bug14788.VALIDATE_NEGOTIATE diff --git a/source4/torture/smb2/ioctl.c b/source4/torture/smb2/ioctl.c index 1de5179e336d..84b5e0927050 100644 --- a/source4/torture/smb2/ioctl.c +++ b/source4/torture/smb2/ioctl.c @@ -27,6 +27,10 @@ #include "torture/smb2/proto.h" #include "../libcli/smb/smbXcli_base.h" #include "librpc/gen_ndr/ndr_ioctl.h" +#include "lib/cmdline/popt_common.h" +#include "libcli/resolve/resolve.h" +#include "lib/param/param.h" +#include "lib/util/tevent_ntstatus.h" #define FNAME "testfsctl.dat" #define FNAME2 "testfsctl2.dat" @@ -6845,12 +6849,115 @@ static bool test_ioctl_bug14607(struct torture_context *torture, return true; } +/* + basic regression test for BUG 14788, + with FSCTL_VALIDATE_NEGOTIATE_INFO + https://bugzilla.samba.org/show_bug.cgi?id=14788 +*/ +static bool test_ioctl_bug14788_VALIDATE_NEGOTIATE(struct torture_context *torture, + struct smb2_tree *tree0) +{ + const char *host = torture_setting_string(torture, "host", NULL); + const char *share = torture_setting_string(torture, "share", NULL); + const char *noperm_share = torture_setting_string(torture, "noperm_share", "noperm"); + struct smb2_transport *transport0 = tree0->session->transport; + struct smbcli_options options; + struct smb2_transport *transport = NULL; + struct smb2_tree *tree = NULL; + struct smb2_session *session = NULL; + uint16_t noperm_flags = 0; + const char *noperm_unc = NULL; + struct smb2_tree *noperm_tree = NULL; + uint32_t timeout_msec; + struct tevent_req *subreq = NULL; + struct cli_credentials *credentials = popt_get_cmdline_credentials(); + NTSTATUS status; + + if (smbXcli_conn_protocol(transport0->conn) < PROTOCOL_SMB3_00) { + torture_skip(torture, "Can't test without SMB 3 support"); + } + + options = transport0->options; + options.client_guid = GUID_random(); + options.min_protocol = PROTOCOL_SMB3_00; + options.max_protocol = PROTOCOL_SMB3_02; + + status = smb2_connect(torture, + host, + lpcfg_smb_ports(torture->lp_ctx), + share, + lpcfg_resolve_context(torture->lp_ctx), + credentials, + &tree, + torture->ev, + &options, + lpcfg_socket_options(torture->lp_ctx), + lpcfg_gensec_settings(torture, torture->lp_ctx) + ); + torture_assert_ntstatus_ok(torture, status, "smb2_connect options failed"); + session = tree->session; + transport = session->transport; + + timeout_msec = tree->session->transport->options.request_timeout * 1000; + + subreq = smb2cli_validate_negotiate_info_send(torture, + torture->ev, + transport->conn, + timeout_msec, + session->smbXcli, + tree->smbXcli); + torture_assert(torture, + tevent_req_poll_ntstatus(subreq, torture->ev, &status), + "tevent_req_poll_ntstatus"); + status = smb2cli_validate_negotiate_info_recv(subreq); + torture_assert_ntstatus_ok(torture, status, "smb2cli_validate_negotiate_info"); + + noperm_unc = talloc_asprintf(torture, "\\\\%s\\%s", host, noperm_share); + torture_assert(torture, noperm_unc != NULL, "talloc_asprintf"); + + noperm_tree = smb2_tree_init(session, torture, false); + torture_assert(torture, noperm_tree != NULL, "smb2_tree_init"); + + status = smb2cli_raw_tcon(transport->conn, + SMB2_HDR_FLAG_SIGNED, + 0, /* clear_flags */ + timeout_msec, + session->smbXcli, + noperm_tree->smbXcli, + noperm_flags, + noperm_unc); + if (NT_STATUS_EQUAL(status, NT_STATUS_BAD_NETWORK_NAME)) { + torture_skip(torture, talloc_asprintf(torture, + "noperm_unc[%s] %s", + noperm_unc, nt_errstr(status))); + } + torture_assert_ntstatus_ok(torture, status, + talloc_asprintf(torture, + "smb2cli_tcon(%s)", + noperm_unc)); + + subreq = smb2cli_validate_negotiate_info_send(torture, + torture->ev, + transport->conn, + timeout_msec, + session->smbXcli, + noperm_tree->smbXcli); + torture_assert(torture, + tevent_req_poll_ntstatus(subreq, torture->ev, &status), + "tevent_req_poll_ntstatus"); + status = smb2cli_validate_negotiate_info_recv(subreq); + torture_assert_ntstatus_ok(torture, status, "smb2cli_validate_negotiate_info noperm"); + + return true; +} + /* * testing of SMB2 ioctls */ struct torture_suite *torture_smb2_ioctl_init(TALLOC_CTX *ctx) { struct torture_suite *suite = torture_suite_create(ctx, "ioctl"); + struct torture_suite *bug14788 = torture_suite_create(ctx, "bug14788"); torture_suite_add_1smb2_test(suite, "shadow_copy", test_ioctl_get_shadow_copy); @@ -6993,6 +7100,10 @@ struct torture_suite *torture_smb2_ioctl_init(TALLOC_CTX *ctx) torture_suite_add_1smb2_test(suite, "bug14607", test_ioctl_bug14607); + torture_suite_add_1smb2_test(bug14788, "VALIDATE_NEGOTIATE", + test_ioctl_bug14788_VALIDATE_NEGOTIATE); + torture_suite_add_suite(suite, bug14788); + suite->description = talloc_strdup(suite, "SMB2-IOCTL tests"); return suite; -- 2.25.1 From e5738bea9bbe2bd4f4f18a7746f9b6ceb925e4ac Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Wed, 15 Sep 2021 17:22:39 +0200 Subject: [PATCH 4/8] smb2_server: make sure in_ctl_code = IVAL(body, 0x04); reads valid bytes BUG: https://bugzilla.samba.org/show_bug.cgi?id=14788 Signed-off-by: Stefan Metzmacher Reviewed-by: Ralph Boehme (cherry picked from commit 1cd948d8520fd41a4e2f0cc6ee787c1e20211e33) --- source3/smbd/smb2_server.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source3/smbd/smb2_server.c b/source3/smbd/smb2_server.c index f071c1a5ff98..f5c28fde3025 100644 --- a/source3/smbd/smb2_server.c +++ b/source3/smbd/smb2_server.c @@ -3097,7 +3097,7 @@ NTSTATUS smbd_smb2_request_dispatch(struct smbd_smb2_request *req) const uint8_t *body = SMBD_SMB2_IN_BODY_PTR(req); size_t body_size = SMBD_SMB2_IN_BODY_LEN(req); uint32_t in_ctl_code; - size_t needed = 4; + size_t needed = 8; if (needed > body_size) { return smbd_smb2_request_error(req, -- 2.25.1 From 45ff4d7878a02f759d90f25168972ba33e1d5a5d Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Wed, 15 Sep 2021 17:25:53 +0200 Subject: [PATCH 5/8] smb2_server: decouple IOCTL check from signing/encryption states There's no reason to handle FSCTL_SMBTORTURE_FORCE_UNACKED_TIMEOUT differently if signing/encryption is used. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14788 Signed-off-by: Stefan Metzmacher Reviewed-by: Ralph Boehme (cherry picked from commit bd3ba3c96e6ba811afd5898ff5470188557a6e33) --- source3/smbd/smb2_server.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/source3/smbd/smb2_server.c b/source3/smbd/smb2_server.c index f5c28fde3025..ce9a1e001433 100644 --- a/source3/smbd/smb2_server.c +++ b/source3/smbd/smb2_server.c @@ -3077,7 +3077,9 @@ NTSTATUS smbd_smb2_request_dispatch(struct smbd_smb2_request *req) if (!NT_STATUS_IS_OK(session_status)) { return smbd_smb2_request_error(req, session_status); } - } else if (opcode == SMB2_OP_IOCTL) { + } + + if (opcode == SMB2_OP_IOCTL) { /* * Some special IOCTL calls don't require * file, tcon nor session. -- 2.25.1 From 948c1ceb5b47bac201af5773165e4384d4be0f77 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Mon, 16 Aug 2021 17:28:05 +0200 Subject: [PATCH 6/8] smb2_server: skip tcon check and chdir_current_service() for FSCTL_VALIDATE_NEGOTIATE_INFO We should not fail this just because the user doesn't have permissions on the share root. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14788 Signed-off-by: Stefan Metzmacher Reviewed-by: Ralph Boehme (cherry picked from commit c850ce96fd32ea91d8a31223bb09dd5b8b98d99e) --- selftest/knownfail.d/smb2.ioctl.bug14788 | 1 - source3/smbd/smb2_server.c | 3 +++ 2 files changed, 3 insertions(+), 1 deletion(-) delete mode 100644 selftest/knownfail.d/smb2.ioctl.bug14788 diff --git a/selftest/knownfail.d/smb2.ioctl.bug14788 b/selftest/knownfail.d/smb2.ioctl.bug14788 deleted file mode 100644 index 843e7d815176..000000000000 --- a/selftest/knownfail.d/smb2.ioctl.bug14788 +++ /dev/null @@ -1 +0,0 @@ -^samba3.smb2.ioctl.*.bug14788.VALIDATE_NEGOTIATE diff --git a/source3/smbd/smb2_server.c b/source3/smbd/smb2_server.c index ce9a1e001433..f1c615b2240a 100644 --- a/source3/smbd/smb2_server.c +++ b/source3/smbd/smb2_server.c @@ -3114,6 +3114,9 @@ NTSTATUS smbd_smb2_request_dispatch(struct smbd_smb2_request *req) case FSCTL_SMBTORTURE_FORCE_UNACKED_TIMEOUT: call = &_root_ioctl_call; break; + case FSCTL_VALIDATE_NEGOTIATE_INFO: + call = &_root_ioctl_call; + break; } } -- 2.25.1 From 7c3672ac1ce02ae91df78ed4289a8c4b0da8f28a Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Wed, 15 Sep 2021 20:26:58 +0200 Subject: [PATCH 7/8] smb2_ioctl: return BUFFER_TOO_SMALL in smbd_smb2_request_ioctl_done() We should not send more data than the client requested. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14788 Signed-off-by: Stefan Metzmacher Reviewed-by: Ralph Boehme (cherry picked from commit aab540503434817cc6b2de1d9c507f9d0b3ad980) --- source3/smbd/smb2_ioctl.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/source3/smbd/smb2_ioctl.c b/source3/smbd/smb2_ioctl.c index 3d65a96368c5..7ca15a233203 100644 --- a/source3/smbd/smb2_ioctl.c +++ b/source3/smbd/smb2_ioctl.c @@ -296,6 +296,7 @@ static void smbd_smb2_request_ioctl_done(struct tevent_req *subreq) uint32_t in_ctl_code; uint64_t in_file_id_persistent; uint64_t in_file_id_volatile; + uint32_t in_max_output_length; uint32_t out_input_offset; uint32_t out_output_offset; DATA_BLOB out_output_buffer = data_blob_null; @@ -328,6 +329,24 @@ static void smbd_smb2_request_ioctl_done(struct tevent_req *subreq) in_ctl_code = IVAL(inbody, 0x04); in_file_id_persistent = BVAL(inbody, 0x08); in_file_id_volatile = BVAL(inbody, 0x10); + in_max_output_length = IVAL(inbody, 0x2C); + + if (out_output_buffer.length > in_max_output_length) { + /* + * Return NT_STATUS_BUFFER_TOO_SMALL by + * default if the provided buffer doesn't + * fit. + * + * If someone wants truncated data + * together with STATUS_BUFFER_OVERFLOW + * it needs to be done explicitly in + * the backends. We currently do that + * in: + * - fsctl_dfs_get_refers() + * - smbd_smb2_ioctl_pipe_read_done() + */ + status = NT_STATUS_BUFFER_TOO_SMALL; + } if (smbd_smb2_ioctl_is_failure(in_ctl_code, status, out_output_buffer.length)) { -- 2.25.1 From 1b71e95fc9c8dae312356a664be53db1f27d434b Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Wed, 15 Sep 2021 20:27:12 +0200 Subject: [PATCH 8/8] smb2_server: don't let SMB2_OP_IOCTL force FILE_CLOSED for invalid file ids smbd_smb2_request_process_ioctl() already detailed checks for file_ids, which not reached before. .allow_invalid_fileid = true was only used for SMB2_OP_IOCTL. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14788 Signed-off-by: Stefan Metzmacher Reviewed-by: Ralph Boehme (cherry picked from commit 1744dd8c5bc342a74e397951506468636275fe45) --- source3/smbd/smb2_server.c | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/source3/smbd/smb2_server.c b/source3/smbd/smb2_server.c index f1c615b2240a..b5a831f6df0b 100644 --- a/source3/smbd/smb2_server.c +++ b/source3/smbd/smb2_server.c @@ -64,7 +64,6 @@ static const struct smbd_smb2_dispatch_table { bool need_tcon; bool as_root; uint16_t fileid_ofs; - bool allow_invalid_fileid; bool modify; } smbd_smb2_table[] = { #define _OP(o) .opcode = o, .name = #o @@ -129,7 +128,6 @@ static const struct smbd_smb2_dispatch_table { .need_session = true, .need_tcon = true, .fileid_ofs = 0x08, - .allow_invalid_fileid = true, .modify = true, },{ _OP(SMB2_OP_CANCEL), @@ -3218,16 +3216,12 @@ NTSTATUS smbd_smb2_request_dispatch(struct smbd_smb2_request *req) fsp = file_fsp_smb2(req, file_id_persistent, file_id_volatile); if (fsp == NULL) { - if (!call->allow_invalid_fileid) { - return smbd_smb2_request_error(req, - NT_STATUS_FILE_CLOSED); - } - - if (file_id_persistent != UINT64_MAX) { - return smbd_smb2_request_error(req, - NT_STATUS_FILE_CLOSED); - } - if (file_id_volatile != UINT64_MAX) { + /* + * smbd_smb2_request_process_ioctl() + * has more checks in order to return more + * detailed error codes... + */ + if (opcode != SMB2_OP_IOCTL) { return smbd_smb2_request_error(req, NT_STATUS_FILE_CLOSED); } -- 2.25.1