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 <jra@samba.org>

diff --git a/source3/smbd/smb2_ioctl.c b/source3/smbd/smb2_ioctl.c
index ea29cd460dfab21b48d5a6367e44a2b665b552ee..be2a798b64dd5970e87eeca4e19d6da043e9b21a 100644 (file)
--- 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 d81bca517f6165f14e2aad195e463787a476d120..c0a637eb1a7efc3e70d8409ba1ab8210d9638def 100644 (file)
--- 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 5897162c37cc44553900fa24293569e8f91aeb3f..fdca6018361fb48025fd4eed9038b95dcf61255b 100644 (file)
--- 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");