smb2_server: don't cancel pending request if at least one channel is still alive

In order to allow replays of requests on a channel failure, we should
not cancel pending requests, the strategie that seems to make windows
clients happy is to let the requests running and return
NT_STATUS_FILE_NOT_AVAILABLE as long as the original request is still
pending.

Here we introduce xconn->transport.shutdown_wait_queue, this is used
to keep the xconn alive for the lifetime of pending requests.

Now we only cancel pending requests if the disconnected connection
is the last channel for a session.

In that case smbXsrv_session_remove_channel() and
smb2srv_session_shutdown_send() will take care of it.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=14449

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>

diff --git a/selftest/knownfail.d/smb2.replay b/selftest/knownfail.d/smb2.replay
index 285081481c68d850aed5081ba84910ddf8f9a161..4cac8071ed8e2d2437454dfca5830260d7e8bfe9 100644 (file)
--- a/selftest/knownfail.d/smb2.replay
+++ b/selftest/knownfail.d/smb2.replay
@@ -1,12 +1,3 @@
-# These are temporary in order to demonstrate the current bugs
-^samba3.smb2.replay.dhv2-pending2n-vs-oplock-sane.nt4_dc
-^samba3.smb2.replay.dhv2-pending2n-vs-lease-sane.nt4_dc
-^samba3.smb2.replay.dhv2-pending2l-vs-oplock-sane.nt4_dc
-^samba3.smb2.replay.dhv2-pending2l-vs-lease-sane.nt4_dc
-^samba3.smb2.replay.dhv2-pending2o-vs-oplock-sane.nt4_dc
-^samba3.smb2.replay.dhv2-pending2o-vs-lease-sane.nt4_dc
-^samba3.smb2.replay.dhv2-pending2n-vs-oplock-sane.ad_dc
-^samba3.smb2.replay.dhv2-pending2o-vs-oplock-sane.ad_dc
 # These tests demonstrate the broken Windows behavior
 # and check for ACCESS_DENIED instead of FILE_NOT_AVAILABLE
 # See https://bugzilla.samba.org/show_bug.cgi?id=14449

diff --git a/source3/smbd/globals.h b/source3/smbd/globals.h
index e023670f869b9de1ae38828d946f0f4a122c8851..f49096cba8a31acfbb8bd8fbc4fe17f1fa510195 100644 (file)
--- a/source3/smbd/globals.h
+++ b/source3/smbd/globals.h
@@ -368,6 +368,7 @@ struct smbXsrv_connection {
        struct {
                NTSTATUS status;
                bool terminating;
+               struct tevent_queue *shutdown_wait_queue;
                int sock;
                struct tevent_fd *fde;
 

diff --git a/source3/smbd/smb2_server.c b/source3/smbd/smb2_server.c
index 930a4201359117ad6d8c1119aceda602b1db64f8..d5b0df33dde71ac78127148541041e5b4049ebb7 100644 (file)
--- a/source3/smbd/smb2_server.c
+++ b/source3/smbd/smb2_server.c
@@ -1498,7 +1498,6 @@ size_t smbXsrv_client_valid_connections(struct smbXsrv_client *client)
 }
 
 struct smbXsrv_connection_shutdown_state {
-       struct tevent_queue *wait_queue;
        struct smbXsrv_connection *xconn;
 };
 
@@ -1521,6 +1520,7 @@ static struct tevent_req *smbXsrv_connection_shutdown_send(TALLOC_CTX *mem_ctx,
         */
        SMB_ASSERT(!NT_STATUS_IS_OK(xconn->transport.status));
        SMB_ASSERT(xconn->transport.terminating);
+       SMB_ASSERT(xconn->transport.shutdown_wait_queue == NULL);
 
        req = tevent_req_create(mem_ctx, &state,
                                struct smbXsrv_connection_shutdown_state);
@@ -1531,45 +1531,48 @@ static struct tevent_req *smbXsrv_connection_shutdown_send(TALLOC_CTX *mem_ctx,
        state->xconn = xconn;
        tevent_req_defer_callback(req, ev);
 
-       status = smbXsrv_session_disconnect_xconn(xconn);
-       if (tevent_req_nterror(req, status)) {
-               return tevent_req_post(req, ev);
-       }
-
-       state->wait_queue = tevent_queue_create(state, "smbXsrv_connection_shutdown_queue");
-       if (tevent_req_nomem(state->wait_queue, req)) {
+       xconn->transport.shutdown_wait_queue =
+               tevent_queue_create(state, "smbXsrv_connection_shutdown_queue");
+       if (tevent_req_nomem(xconn->transport.shutdown_wait_queue, req)) {
                return tevent_req_post(req, ev);
        }
 
        for (preq = xconn->smb2.requests; preq != NULL; preq = preq->next) {
-               /*
-                * The connection is gone so we
-                * don't need to take care of
-                * any crypto
-                */
-               preq->session = NULL;
-               preq->do_signing = false;
-               preq->do_encryption = false;
-               preq->preauth = NULL;
-
-               if (preq->subreq != NULL) {
-                       tevent_req_cancel(preq->subreq);
-               }
-
                /*
                 * Now wait until the request is finished.
                 *
                 * We don't set a callback, as we just want to block the
                 * wait queue and the talloc_free() of the request will
                 * remove the item from the wait queue.
+                *
+                * Note that we don't cancel the requests here
+                * in order to keep the replay detection logic correct.
+                *
+                * However if we teardown the last channel of
+                * a connection, we'll call some logic via
+                * smbXsrv_session_disconnect_xconn()
+                * -> smbXsrv_session_disconnect_xconn_callback()
+                *   -> smbXsrv_session_remove_channel()
+                *     -> smb2srv_session_shutdown_send()
+                * will indeed cancel the request.
                 */
-               subreq = tevent_queue_wait_send(preq, ev, state->wait_queue);
+               subreq = tevent_queue_wait_send(preq, ev,
+                                       xconn->transport.shutdown_wait_queue);
                if (tevent_req_nomem(subreq, req)) {
                        return tevent_req_post(req, ev);
                }
        }
 
-       len = tevent_queue_length(state->wait_queue);
+       /*
+        * This may attach sessions with num_channels == 0
+        * to xconn->transport.shutdown_wait_queue.
+        */
+       status = smbXsrv_session_disconnect_xconn(xconn);
+       if (tevent_req_nterror(req, status)) {
+               return tevent_req_post(req, ev);
+       }
+
+       len = tevent_queue_length(xconn->transport.shutdown_wait_queue);
        if (len == 0) {
                tevent_req_done(req);
                return tevent_req_post(req, ev);
@@ -1580,7 +1583,7 @@ static struct tevent_req *smbXsrv_connection_shutdown_send(TALLOC_CTX *mem_ctx,
         * this way we get notified when all pending requests are finished
         * and send to the socket.
         */
-       subreq = tevent_queue_wait_send(state, ev, state->wait_queue);
+       subreq = tevent_queue_wait_send(state, ev, xconn->transport.shutdown_wait_queue);
        if (tevent_req_nomem(subreq, req)) {
                return tevent_req_post(req, ev);
        }

diff --git a/source3/smbd/smbXsrv_session.c b/source3/smbd/smbXsrv_session.c
index 457505655293b148cb3a28bf33ada22007360b9a..f41a817cf27a009b7421caa056130ba4bbb5f6cf 100644 (file)
--- a/source3/smbd/smbXsrv_session.c
+++ b/source3/smbd/smbXsrv_session.c
@@ -1596,8 +1596,32 @@ NTSTATUS smbXsrv_session_remove_channel(struct smbXsrv_session *session,
                global->num_channels--;
                if (global->num_channels == 0) {
                        struct smbXsrv_client *client = session->client;
+                       struct tevent_queue *xconn_wait_queue =
+                               xconn->transport.shutdown_wait_queue;
                        struct tevent_req *subreq = NULL;
 
+                       /*
+                        * Let the connection wait until the session is
+                        * destroyed.
+                        *
+                        * We don't set a callback, as we just want to block the
+                        * wait queue and the talloc_free() of the session will
+                        * remove the item from the wait queue in order
+                        * to remove allow the connection to disapear.
+                        */
+                       if (xconn_wait_queue != NULL) {
+                               subreq = tevent_queue_wait_send(session,
+                                                               client->raw_ev_ctx,
+                                                               xconn_wait_queue);
+                               if (subreq == NULL) {
+                                       status = NT_STATUS_NO_MEMORY;
+                                       DBG_ERR("tevent_queue_wait_send() session(%llu) failed: %s\n",
+                                               (unsigned long long)session->global->session_wire_id,
+                                               nt_errstr(status));
+                                       return status;
+                               }
+                       }
+
                        /*
                         * This is garanteed to set
                         * session->status = NT_STATUS_USER_SESSION_DELETED