From 5489d41d39a9e59941fe2843fb3db49e2bd95269 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Thu, 16 Sep 2021 10:51:43 +0200 Subject: [PATCH 01/11] 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 e8e43993dcba0a0437cdcb762ad1f2176311b9dc Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Wed, 15 Sep 2021 18:31:06 +0200 Subject: [PATCH 02/11] 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..ee517d788722 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/cmdline.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 = samba_cmdline_get_creds(); + 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 2bb887176e1ab39843a6c19cf9d279c2b7729897 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Wed, 15 Sep 2021 17:22:39 +0200 Subject: [PATCH 03/11] 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 1322cf3ba083..83bf6aafd201 100644 --- a/source3/smbd/smb2_server.c +++ b/source3/smbd/smb2_server.c @@ -3102,7 +3102,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 cceb547403aa4bb82441d8ad1ccdb41ea477fdc7 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Wed, 15 Sep 2021 17:25:53 +0200 Subject: [PATCH 04/11] 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 83bf6aafd201..2ef3fa70a54f 100644 --- a/source3/smbd/smb2_server.c +++ b/source3/smbd/smb2_server.c @@ -3082,7 +3082,9 @@ NTSTATUS smbd_smb2_request_dispatch(struct smbd_smb2_request *req) } } else if (opcode == SMB2_OP_CANCEL) { /* Cancel requests are allowed to skip the signing */ - } 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 2425801d147eb67df9adc2db348567c00d70aead Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Mon, 16 Aug 2021 17:28:05 +0200 Subject: [PATCH 05/11] 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 2ef3fa70a54f..0782b64f2e2f 100644 --- a/source3/smbd/smb2_server.c +++ b/source3/smbd/smb2_server.c @@ -3119,6 +3119,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; } } else if (signing_required) { /* -- 2.25.1 From 38904bb2b9e4313e502b4285b9633b65f99be752 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Mon, 29 Nov 2021 19:44:12 +0100 Subject: [PATCH 06/11] s4:torture/smb2: test FSCTL_QUERY_NETWORK_INTERFACE_INFO with BUFFER_TOO_SMALL It seems that we currently don't have BUFFER_TOO_SMALL handling for FSCTL/IOCTL calls. FSCTL_QUERY_NETWORK_INTERFACE_INFO is just an easy example to demonstrate it. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14788 Signed-off-by: Stefan Metzmacher Reviewed-by: Ralph Boehme (cherry picked from commit b3212b359edb78d4c60fed377fa18478c8e75d9a) --- selftest/knownfail.d/smb2.ioctl.bug14788 | 1 + source4/torture/smb2/ioctl.c | 102 +++++++++++++++++++++++ 2 files changed, 103 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..b228bfafe471 --- /dev/null +++ b/selftest/knownfail.d/smb2.ioctl.bug14788 @@ -0,0 +1 @@ +^samba3.smb2.ioctl.*.bug14788.NETWORK_INTERFACE diff --git a/source4/torture/smb2/ioctl.c b/source4/torture/smb2/ioctl.c index ee517d788722..a6f4a0b5445e 100644 --- a/source4/torture/smb2/ioctl.c +++ b/source4/torture/smb2/ioctl.c @@ -6951,6 +6951,106 @@ static bool test_ioctl_bug14788_VALIDATE_NEGOTIATE(struct torture_context *tortu return true; } +/* + basic regression test for BUG 14788, + with FSCTL_QUERY_NETWORK_INTERFACE_INFO + https://bugzilla.samba.org/show_bug.cgi?id=14788 +*/ +static bool test_ioctl_bug14788_NETWORK_INTERFACE(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); + 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; + uint32_t timeout_msec; + DATA_BLOB out_input_buffer = data_blob_null; + DATA_BLOB out_output_buffer = data_blob_null; + struct cli_credentials *credentials = samba_cmdline_get_creds(); + 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; + + /* + * A valid FSCTL_QUERY_NETWORK_INTERFACE_INFO + */ + status = smb2cli_ioctl(transport->conn, + timeout_msec, + session->smbXcli, + tree->smbXcli, + UINT64_MAX, /* in_fid_persistent */ + UINT64_MAX, /* in_fid_volatile */ + FSCTL_QUERY_NETWORK_INTERFACE_INFO, + 0, /* in_max_input_length */ + NULL, /* in_input_buffer */ + UINT16_MAX, /* in_max_output_length */ + NULL, /* in_output_buffer */ + SMB2_IOCTL_FLAG_IS_FSCTL, + torture, + &out_input_buffer, + &out_output_buffer); + torture_assert_ntstatus_ok(torture, status, + "FSCTL_QUERY_NETWORK_INTERFACE_INFO"); + + /* + * An invalid FSCTL_QUERY_NETWORK_INTERFACE_INFO, + * with file_id_* is being UINT64_MAX and + * in_max_output_length = 1. + * + * This demonstrates NT_STATUS_BUFFER_TOO_SMALL + * if the server is not able to return the + * whole response buffer to the client. + */ + status = smb2cli_ioctl(transport->conn, + timeout_msec, + session->smbXcli, + tree->smbXcli, + UINT64_MAX, /* in_fid_persistent */ + UINT64_MAX, /* in_fid_volatile */ + FSCTL_QUERY_NETWORK_INTERFACE_INFO, + 0, /* in_max_input_length */ + NULL, /* in_input_buffer */ + 1, /* in_max_output_length */ + NULL, /* in_output_buffer */ + SMB2_IOCTL_FLAG_IS_FSCTL, + torture, + &out_input_buffer, + &out_output_buffer); + torture_assert_ntstatus_equal(torture, status, + NT_STATUS_BUFFER_TOO_SMALL, + "FSCTL_QUERY_NETWORK_INTERFACE_INFO"); + + return true; +} + /* * testing of SMB2 ioctls */ @@ -7102,6 +7202,8 @@ struct torture_suite *torture_smb2_ioctl_init(TALLOC_CTX *ctx) torture_suite_add_1smb2_test(bug14788, "VALIDATE_NEGOTIATE", test_ioctl_bug14788_VALIDATE_NEGOTIATE); + torture_suite_add_1smb2_test(bug14788, "NETWORK_INTERFACE", + test_ioctl_bug14788_NETWORK_INTERFACE); torture_suite_add_suite(suite, bug14788); suite->description = talloc_strdup(suite, "SMB2-IOCTL tests"); -- 2.25.1 From 0c75173a2dd93eb0854edf2311c3e3de12f7f4b3 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Wed, 15 Sep 2021 20:26:58 +0200 Subject: [PATCH 07/11] 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) --- selftest/knownfail.d/smb2.ioctl.bug14788 | 1 - source3/smbd/smb2_ioctl.c | 19 +++++++++++++++++++ 2 files changed, 19 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 b228bfafe471..000000000000 --- a/selftest/knownfail.d/smb2.ioctl.bug14788 +++ /dev/null @@ -1 +0,0 @@ -^samba3.smb2.ioctl.*.bug14788.NETWORK_INTERFACE 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 ac8c80dddcae6dc7f20df9ffdec856675f5dbd72 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Mon, 29 Nov 2021 19:56:20 +0100 Subject: [PATCH 08/11] s4:torture/smb2: FSCTL_QUERY_NETWORK_INTERFACE_INFO gives INVALID_PARAMETER with invalid file ids An invalid file id for FSCTL_QUERY_NETWORK_INTERFACE_INFO gives INVALID_PARAMETER instead of FILE_CLOSED. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14788 Signed-off-by: Stefan Metzmacher Reviewed-by: Ralph Boehme (cherry picked from commit fb33f145ff598b03a08098b7f12f3c53491f6c04) --- selftest/knownfail.d/smb2.ioctl.bug14788 | 1 + source4/torture/smb2/ioctl.c | 53 ++++++++++++++++++++++++ 2 files changed, 54 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..b228bfafe471 --- /dev/null +++ b/selftest/knownfail.d/smb2.ioctl.bug14788 @@ -0,0 +1 @@ +^samba3.smb2.ioctl.*.bug14788.NETWORK_INTERFACE diff --git a/source4/torture/smb2/ioctl.c b/source4/torture/smb2/ioctl.c index a6f4a0b5445e..709c3c2cfda9 100644 --- a/source4/torture/smb2/ioctl.c +++ b/source4/torture/smb2/ioctl.c @@ -7048,6 +7048,59 @@ static bool test_ioctl_bug14788_NETWORK_INTERFACE(struct torture_context *tortur NT_STATUS_BUFFER_TOO_SMALL, "FSCTL_QUERY_NETWORK_INTERFACE_INFO"); + /* + * An invalid FSCTL_QUERY_NETWORK_INTERFACE_INFO, + * with file_id_* not being UINT64_MAX. + * + * This gives INVALID_PARAMETER instead + * of FILE_CLOSED. + */ + status = smb2cli_ioctl(transport->conn, + timeout_msec, + session->smbXcli, + tree->smbXcli, + INT64_MAX, /* in_fid_persistent */ + INT64_MAX, /* in_fid_volatile */ + FSCTL_QUERY_NETWORK_INTERFACE_INFO, + 0, /* in_max_input_length */ + NULL, /* in_input_buffer */ + UINT16_MAX, /* in_max_output_length */ + NULL, /* in_output_buffer */ + SMB2_IOCTL_FLAG_IS_FSCTL, + torture, + &out_input_buffer, + &out_output_buffer); + torture_assert_ntstatus_equal(torture, status, + NT_STATUS_INVALID_PARAMETER, + "FSCTL_QUERY_NETWORK_INTERFACE_INFO"); + + /* + * An invalid FSCTL_QUERY_NETWORK_INTERFACE_INFO, + * with file_id_* not being UINT64_MAX and + * in_max_output_length = 1. + * + * This proves INVALID_PARAMETER instead + * of BUFFER_TOO_SMALL. + */ + status = smb2cli_ioctl(transport->conn, + timeout_msec, + session->smbXcli, + tree->smbXcli, + INT64_MAX, /* in_fid_persistent */ + INT64_MAX, /* in_fid_volatile */ + FSCTL_QUERY_NETWORK_INTERFACE_INFO, + 0, /* in_max_input_length */ + NULL, /* in_input_buffer */ + 1, /* in_max_output_length */ + NULL, /* in_output_buffer */ + SMB2_IOCTL_FLAG_IS_FSCTL, + torture, + &out_input_buffer, + &out_output_buffer); + torture_assert_ntstatus_equal(torture, status, + NT_STATUS_INVALID_PARAMETER, + "FSCTL_QUERY_NETWORK_INTERFACE_INFO"); + return true; } -- 2.25.1 From 34371e73abe3ab7d7eb5eb6a360aa3eace0391f9 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Wed, 15 Sep 2021 20:27:12 +0200 Subject: [PATCH 09/11] 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) --- selftest/knownfail.d/smb2.ioctl.bug14788 | 1 - source3/smbd/smb2_server.c | 18 ++++++------------ 2 files changed, 6 insertions(+), 13 deletions(-) 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 b228bfafe471..000000000000 --- a/selftest/knownfail.d/smb2.ioctl.bug14788 +++ /dev/null @@ -1 +0,0 @@ -^samba3.smb2.ioctl.*.bug14788.NETWORK_INTERFACE diff --git a/source3/smbd/smb2_server.c b/source3/smbd/smb2_server.c index 0782b64f2e2f..d66fc95974a9 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), @@ -3230,16 +3228,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 From 7c8fd26cc6611d334c2156a2ada1a2255b57d0e5 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Mon, 29 Nov 2021 19:56:20 +0100 Subject: [PATCH 10/11] s4:torture/smb2: FSCTL_QUERY_NETWORK_INTERFACE_INFO should work on noperm share Demonstrate that smbd fails FSCTL_QUERY_NETWORK_INTERFACE_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 629d161b8f579bc24acfaf3fe02612a5237345b4) --- selftest/knownfail.d/smb2.ioctl.bug14788 | 1 + source4/torture/smb2/ioctl.c | 130 +++++++++++++++++++++++ 2 files changed, 131 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..b228bfafe471 --- /dev/null +++ b/selftest/knownfail.d/smb2.ioctl.bug14788 @@ -0,0 +1 @@ +^samba3.smb2.ioctl.*.bug14788.NETWORK_INTERFACE diff --git a/source4/torture/smb2/ioctl.c b/source4/torture/smb2/ioctl.c index 709c3c2cfda9..fc8d623fd0f7 100644 --- a/source4/torture/smb2/ioctl.c +++ b/source4/torture/smb2/ioctl.c @@ -6961,11 +6961,15 @@ static bool test_ioctl_bug14788_NETWORK_INTERFACE(struct torture_context *tortur { 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; DATA_BLOB out_input_buffer = data_blob_null; DATA_BLOB out_output_buffer = data_blob_null; @@ -7101,6 +7105,132 @@ static bool test_ioctl_bug14788_NETWORK_INTERFACE(struct torture_context *tortur NT_STATUS_INVALID_PARAMETER, "FSCTL_QUERY_NETWORK_INTERFACE_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)); + + /* + * A valid FSCTL_QUERY_NETWORK_INTERFACE_INFO + */ + status = smb2cli_ioctl(transport->conn, + timeout_msec, + session->smbXcli, + noperm_tree->smbXcli, + UINT64_MAX, /* in_fid_persistent */ + UINT64_MAX, /* in_fid_volatile */ + FSCTL_QUERY_NETWORK_INTERFACE_INFO, + 0, /* in_max_input_length */ + NULL, /* in_input_buffer */ + UINT16_MAX, /* in_max_output_length */ + NULL, /* in_output_buffer */ + SMB2_IOCTL_FLAG_IS_FSCTL, + torture, + &out_input_buffer, + &out_output_buffer); + torture_assert_ntstatus_ok(torture, status, + "FSCTL_QUERY_NETWORK_INTERFACE_INFO"); + + /* + * An invalid FSCTL_QUERY_NETWORK_INTERFACE_INFO, + * with file_id_* is being UINT64_MAX and + * in_max_output_length = 1. + * + * This demonstrates NT_STATUS_BUFFER_TOO_SMALL + * if the server is not able to return the + * whole response buffer to the client. + */ + status = smb2cli_ioctl(transport->conn, + timeout_msec, + session->smbXcli, + noperm_tree->smbXcli, + UINT64_MAX, /* in_fid_persistent */ + UINT64_MAX, /* in_fid_volatile */ + FSCTL_QUERY_NETWORK_INTERFACE_INFO, + 0, /* in_max_input_length */ + NULL, /* in_input_buffer */ + 1, /* in_max_output_length */ + NULL, /* in_output_buffer */ + SMB2_IOCTL_FLAG_IS_FSCTL, + torture, + &out_input_buffer, + &out_output_buffer); + torture_assert_ntstatus_equal(torture, status, + NT_STATUS_BUFFER_TOO_SMALL, + "FSCTL_QUERY_NETWORK_INTERFACE_INFO"); + + /* + * An invalid FSCTL_QUERY_NETWORK_INTERFACE_INFO, + * with file_id_* not being UINT64_MAX. + * + * This gives INVALID_PARAMETER instead + * of FILE_CLOSED. + */ + status = smb2cli_ioctl(transport->conn, + timeout_msec, + session->smbXcli, + noperm_tree->smbXcli, + INT64_MAX, /* in_fid_persistent */ + INT64_MAX, /* in_fid_volatile */ + FSCTL_QUERY_NETWORK_INTERFACE_INFO, + 0, /* in_max_input_length */ + NULL, /* in_input_buffer */ + UINT16_MAX, /* in_max_output_length */ + NULL, /* in_output_buffer */ + SMB2_IOCTL_FLAG_IS_FSCTL, + torture, + &out_input_buffer, + &out_output_buffer); + torture_assert_ntstatus_equal(torture, status, + NT_STATUS_INVALID_PARAMETER, + "FSCTL_QUERY_NETWORK_INTERFACE_INFO"); + + /* + * An invalid FSCTL_QUERY_NETWORK_INTERFACE_INFO, + * with file_id_* not being UINT64_MAX and + * in_max_output_length = 1. + * + * This proves INVALID_PARAMETER instead + * of BUFFER_TOO_SMALL. + */ + status = smb2cli_ioctl(transport->conn, + timeout_msec, + session->smbXcli, + noperm_tree->smbXcli, + INT64_MAX, /* in_fid_persistent */ + INT64_MAX, /* in_fid_volatile */ + FSCTL_QUERY_NETWORK_INTERFACE_INFO, + 0, /* in_max_input_length */ + NULL, /* in_input_buffer */ + 1, /* in_max_output_length */ + NULL, /* in_output_buffer */ + SMB2_IOCTL_FLAG_IS_FSCTL, + torture, + &out_input_buffer, + &out_output_buffer); + torture_assert_ntstatus_equal(torture, status, + NT_STATUS_INVALID_PARAMETER, + "FSCTL_QUERY_NETWORK_INTERFACE_INFO"); + return true; } -- 2.25.1 From 70a33932d741160930c2ea42c59512defbe01c6c Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Wed, 15 Sep 2021 19:29:40 +0200 Subject: [PATCH 11/11] smb2_server: skip tcon check and chdir_current_service() for FSCTL_QUERY_NETWORK_INTERFACE_INFO MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Autobuild-User(master): Ralph Böhme Autobuild-Date(master): Wed Dec 1 11:51:50 UTC 2021 on sn-devel-184 (cherry picked from commit f4d0bb164f028da46eab766135bb38175c117deb) --- 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 b228bfafe471..000000000000 --- a/selftest/knownfail.d/smb2.ioctl.bug14788 +++ /dev/null @@ -1 +0,0 @@ -^samba3.smb2.ioctl.*.bug14788.NETWORK_INTERFACE diff --git a/source3/smbd/smb2_server.c b/source3/smbd/smb2_server.c index d66fc95974a9..79832d40a8b6 100644 --- a/source3/smbd/smb2_server.c +++ b/source3/smbd/smb2_server.c @@ -3120,6 +3120,9 @@ NTSTATUS smbd_smb2_request_dispatch(struct smbd_smb2_request *req) case FSCTL_VALIDATE_NEGOTIATE_INFO: call = &_root_ioctl_call; break; + case FSCTL_QUERY_NETWORK_INTERFACE_INFO: + call = &_root_ioctl_call; + break; } } else if (signing_required) { /* -- 2.25.1