From 65983aac12e5ecb12157b39c7bec464388716f27 Mon Sep 17 00:00:00 2001 From: David Disseldorp Date: Tue, 15 Jan 2013 17:23:01 +0100 Subject: smb2_ioctl: remove ioctl error response assumptions MS-SMB2 3.3.4.4 documents cases where a ntstatus indicating an error should not be considered a failure. In such a case the output data buffer should be sent to the client rather than an error response packet. Add a new fsctl copy_chunk test to confirm field limits are sent back in response to an oversize chunk request. Reviewed by: Jeremy Allison --- source3/smbd/smb2_ioctl.c | 74 ++++++++++++++++++++++++++++++++++---------- source4/libcli/smb2/ioctl.c | 37 ++++++++++++++++++++-- source4/torture/smb2/ioctl.c | 58 ++++++++++++++++++++++++++++++++++ 3 files changed, 149 insertions(+), 20 deletions(-) diff --git a/source3/smbd/smb2_ioctl.c b/source3/smbd/smb2_ioctl.c index ea29cd460d..be2a798b64 100644 --- a/source3/smbd/smb2_ioctl.c +++ b/source3/smbd/smb2_ioctl.c @@ -25,6 +25,7 @@ #include "../lib/util/tevent_ntstatus.h" #include "include/ntioctl.h" #include "smb2_ioctl_private.h" +#include "librpc/gen_ndr/ioctl.h" static struct tevent_req *smbd_smb2_ioctl_send(TALLOC_CTX *mem_ctx, struct tevent_context *ev, @@ -227,6 +228,42 @@ NTSTATUS smbd_smb2_request_process_ioctl(struct smbd_smb2_request *req) return smbd_smb2_request_pending_queue(req, subreq, 1000); } +/* + * 3.3.4.4 Sending an Error Response + * An error code other than one of the following indicates a failure: + */ +static bool smbd_smb2_ioctl_is_failure(uint32_t ctl_code, NTSTATUS status, + size_t data_size) +{ + if (NT_STATUS_IS_OK(status)) { + return false; + } + + /* + * STATUS_BUFFER_OVERFLOW in a FSCTL_PIPE_TRANSCEIVE, FSCTL_PIPE_PEEK or + * FSCTL_DFS_GET_REFERRALS Response specified in section 2.2.32.<153> + */ + if (NT_STATUS_EQUAL(status, STATUS_BUFFER_OVERFLOW) + && ((ctl_code == FSCTL_PIPE_TRANSCEIVE) + || (ctl_code == FSCTL_PIPE_PEEK) + || (ctl_code == FSCTL_DFS_GET_REFERRALS))) { + return false; + } + + /* + * Any status other than STATUS_SUCCESS in a FSCTL_SRV_COPYCHUNK or + * FSCTL_SRV_COPYCHUNK_WRITE Response, when returning an + * SRV_COPYCHUNK_RESPONSE as described in section 2.2.32.1. + */ + if (((ctl_code == FSCTL_SRV_COPYCHUNK) + || (ctl_code == FSCTL_SRV_COPYCHUNK_WRITE)) + && (data_size == sizeof(struct srv_copychunk_rsp))) { + return false; + } + + return true; +} + static void smbd_smb2_request_ioctl_done(struct tevent_req *subreq) { struct smbd_smb2_request *req = tevent_req_callback_data(subreq, @@ -261,9 +298,14 @@ static void smbd_smb2_request_ioctl_done(struct tevent_req *subreq) return; } - if (NT_STATUS_EQUAL(status, STATUS_BUFFER_OVERFLOW)) { - /* also ok */ - } else if (!NT_STATUS_IS_OK(status)) { + inbody = SMBD_SMB2_IN_BODY_PTR(req); + + in_ctl_code = IVAL(inbody, 0x04); + in_file_id_persistent = BVAL(inbody, 0x08); + in_file_id_volatile = BVAL(inbody, 0x10); + + if (smbd_smb2_ioctl_is_failure(in_ctl_code, status, + out_output_buffer.length)) { error = smbd_smb2_request_error(req, status); if (!NT_STATUS_IS_OK(error)) { smbd_server_connection_terminate(req->sconn, @@ -276,12 +318,6 @@ static void smbd_smb2_request_ioctl_done(struct tevent_req *subreq) out_input_offset = SMB2_HDR_BODY + 0x30; out_output_offset = SMB2_HDR_BODY + 0x30; - inbody = SMBD_SMB2_IN_BODY_PTR(req); - - in_ctl_code = IVAL(inbody, 0x04); - in_file_id_persistent = BVAL(inbody, 0x08); - in_file_id_volatile = BVAL(inbody, 0x10); - outbody = data_blob_talloc(req->out.vector, NULL, 0x30); if (outbody.data == NULL) { error = smbd_smb2_request_error(req, NT_STATUS_NO_MEMORY); @@ -399,19 +435,23 @@ static NTSTATUS smbd_smb2_ioctl_recv(struct tevent_req *req, NTSTATUS status = NT_STATUS_OK; struct smbd_smb2_ioctl_state *state = tevent_req_data(req, struct smbd_smb2_ioctl_state); + enum tevent_req_state req_state; + uint64_t err; *disconnect = state->disconnect; - if (tevent_req_is_nterror(req, &status)) { - if (!NT_STATUS_EQUAL(status, STATUS_BUFFER_OVERFLOW)) { - tevent_req_received(req); - return status; - } + if ((tevent_req_is_error(req, &req_state, &err) == false) + || (req_state == TEVENT_REQ_USER_ERROR)) { + /* + * Return output buffer to caller if the ioctl was successfully + * processed, even if a user error occurred. Some ioctls return + * data on failure. + */ + *out_output = state->out_output; + talloc_steal(mem_ctx, out_output->data); } - *out_output = state->out_output; - talloc_steal(mem_ctx, out_output->data); - + tevent_req_is_nterror(req, &status); tevent_req_received(req); return status; } diff --git a/source4/libcli/smb2/ioctl.c b/source4/libcli/smb2/ioctl.c index d81bca517f..c0a637eb1a 100644 --- a/source4/libcli/smb2/ioctl.c +++ b/source4/libcli/smb2/ioctl.c @@ -22,6 +22,7 @@ #include "includes.h" #include "libcli/smb2/smb2.h" #include "libcli/smb2/smb2_calls.h" +#include "librpc/gen_ndr/ioctl.h" /* send a ioctl request @@ -61,17 +62,47 @@ struct smb2_request *smb2_ioctl_send(struct smb2_tree *tree, struct smb2_ioctl * return req; } +/* + * 3.3.4.4 Sending an Error Response + */ +static bool smb2_ioctl_is_failure(uint32_t ctl_code, NTSTATUS status, + size_t data_size) +{ + if (NT_STATUS_IS_OK(status)) { + return false; + } + + if (NT_STATUS_EQUAL(status, STATUS_BUFFER_OVERFLOW) + && ((ctl_code == FSCTL_PIPE_TRANSCEIVE) + || (ctl_code == FSCTL_PIPE_PEEK) + || (ctl_code == FSCTL_DFS_GET_REFERRALS))) { + return false; + } + + if (((ctl_code == FSCTL_SRV_COPYCHUNK) + || (ctl_code == FSCTL_SRV_COPYCHUNK_WRITE)) + && (data_size == sizeof(struct srv_copychunk_rsp))) { + /* + * copychunk responses may come with copychunk data or error + * response data, independent of status. + */ + return false; + } + + return true; +} /* recv a ioctl reply */ -NTSTATUS smb2_ioctl_recv(struct smb2_request *req, +NTSTATUS smb2_ioctl_recv(struct smb2_request *req, TALLOC_CTX *mem_ctx, struct smb2_ioctl *io) { NTSTATUS status; - if (!smb2_request_receive(req) || - smb2_request_is_error(req)) { + if (!smb2_request_receive(req) || + smb2_ioctl_is_failure(io->in.function, req->status, + req->in.bufinfo.data_size)) { return smb2_request_destroy(req); } diff --git a/source4/torture/smb2/ioctl.c b/source4/torture/smb2/ioctl.c index 5897162c37..fdca601836 100644 --- a/source4/torture/smb2/ioctl.c +++ b/source4/torture/smb2/ioctl.c @@ -570,6 +570,62 @@ static bool test_ioctl_copy_chunk_append(struct torture_context *torture, return true; } +static bool test_ioctl_copy_chunk_limits(struct torture_context *torture, + struct smb2_tree *tree) +{ + struct smb2_handle src_h; + struct smb2_handle dest_h; + NTSTATUS status; + union smb_ioctl ioctl; + TALLOC_CTX *tmp_ctx = talloc_new(tree); + struct srv_copychunk_copy cc_copy; + struct srv_copychunk_rsp cc_rsp; + enum ndr_err_code ndr_ret; + bool ok; + + ok = test_setup_copy_chunk(torture, tree, tmp_ctx, + 1, /* chunks */ + &src_h, 4096, /* src file */ + &dest_h, 0, /* dest file */ + &cc_copy, + &ioctl); + if (!ok) { + return false; + } + + /* send huge chunk length request */ + cc_copy.chunks[0].source_off = 0; + cc_copy.chunks[0].target_off = 0; + cc_copy.chunks[0].length = UINT_MAX; + + ndr_ret = ndr_push_struct_blob(&ioctl.smb2.in.out, tmp_ctx, + &cc_copy, + (ndr_push_flags_fn_t)ndr_push_srv_copychunk_copy); + torture_assert_ndr_success(torture, ndr_ret, "marshalling request"); + + status = smb2_ioctl(tree, tmp_ctx, &ioctl.smb2); + torture_assert_ntstatus_equal(torture, status, + NT_STATUS_INVALID_PARAMETER, + "bad oversize chunk response"); + + ndr_ret = ndr_pull_struct_blob(&ioctl.smb2.out.out, tmp_ctx, + &cc_rsp, + (ndr_pull_flags_fn_t)ndr_pull_srv_copychunk_rsp); + torture_assert_ndr_success(torture, ndr_ret, "unmarshalling response"); + + torture_comment(torture, "limit max chunks, got %u\n", + cc_rsp.chunks_written); + torture_comment(torture, "limit max chunk len, got %u\n", + cc_rsp.chunk_bytes_written); + torture_comment(torture, "limit max total bytes, got %u\n", + cc_rsp.total_bytes_written); + + smb2_util_close(tree, src_h); + smb2_util_close(tree, dest_h); + talloc_free(tmp_ctx); + return true; +} + /* basic testing of SMB2 ioctls */ @@ -591,6 +647,8 @@ struct torture_suite *torture_smb2_ioctl_init(void) test_ioctl_copy_chunk_over); torture_suite_add_1smb2_test(suite, "copy_chunk_append", test_ioctl_copy_chunk_append); + torture_suite_add_1smb2_test(suite, "copy_chunk_limits", + test_ioctl_copy_chunk_limits); suite->description = talloc_strdup(suite, "SMB2-IOCTL tests"); -- cgit