[Patches] Network session expired at the wrong moment (bug #13197)

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

[Patches] Network session expired at the wrong moment (bug #13197)

Samba - samba-technical mailing list
Hi,

here're patches to allow some SMB2 operations to be processed
even if the session is already expired.

They fix https://bugzilla.samba.org/show_bug.cgi?id=13197

The test (smb2.session.expire2) I wrote shows that
Windows (at least 2012R2) allows more than
[MS-SMB2] 3.3.5.2.9 Verifying the Session specifies.

I have a customer capture were an SMB2 close gets
NETWORK_SESSION_EXPIRED, which isn't replayed.
This causes a SHARING_VIOLATION deadlock with the
same client, as it immediately tries to reopen the
same file with different options.

Please review and push:-)

Thanks!
metze

tmp.diff.txt (26K) Download Attachment
signature.asc (853 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [Patches] Network session expired at the wrong moment (bug #13197)

Samba - samba-technical mailing list
On Thu, Dec 21, 2017 at 03:48:48PM +0100, Stefan Metzmacher via samba-technical wrote:

> Hi,
>
> here're patches to allow some SMB2 operations to be processed
> even if the session is already expired.
>
> They fix https://bugzilla.samba.org/show_bug.cgi?id=13197
>
> The test (smb2.session.expire2) I wrote shows that
> Windows (at least 2012R2) allows more than
> [MS-SMB2] 3.3.5.2.9 Verifying the Session specifies.
>
> I have a customer capture were an SMB2 close gets
> NETWORK_SESSION_EXPIRED, which isn't replayed.
> This causes a SHARING_VIOLATION deadlock with the
> same client, as it immediately tries to reopen the
> same file with different options.
>
> Please review and push:-)

Wow. Great work ! RB+ and pushed. I especially love
the fact you have a test for this :-). That must have
been fun to create :-).

> From a1e533fef6f5e5da2d8eeae196df45980b7cd698 Mon Sep 17 00:00:00 2001
> From: Stefan Metzmacher <[hidden email]>
> Date: Thu, 21 Dec 2017 12:53:02 +0100
> Subject: [PATCH 1/3] s4:torture: add smb2.session.expire2 test
>
> This demonstrates the interaction of NT_STATUS_NETWORK_SESSION_EXPIRED
> and various SMB2 opcodes.
>
> BUG: https://bugzilla.samba.org/show_bug.cgi?id=13197
>
> Signed-off-by: Stefan Metzmacher <[hidden email]>
> ---
>  selftest/knownfail.d/session.expire2 |   1 +
>  source4/libcli/smb2/keepalive.c      |   7 +-
>  source4/torture/smb2/session.c       | 362 +++++++++++++++++++++++++++++++++++
>  3 files changed, 368 insertions(+), 2 deletions(-)
>  create mode 100644 selftest/knownfail.d/session.expire2
>
> diff --git a/selftest/knownfail.d/session.expire2 b/selftest/knownfail.d/session.expire2
> new file mode 100644
> index 0000000..998ccbd
> --- /dev/null
> +++ b/selftest/knownfail.d/session.expire2
> @@ -0,0 +1 @@
> +^samba3.smb2.session.*krb5.expire2
> diff --git a/source4/libcli/smb2/keepalive.c b/source4/libcli/smb2/keepalive.c
> index 402b063..71004aa14 100644
> --- a/source4/libcli/smb2/keepalive.c
> +++ b/source4/libcli/smb2/keepalive.c
> @@ -26,7 +26,8 @@
>  /*
>    send a keepalive request
>  */
> -struct smb2_request *smb2_keepalive_send(struct smb2_transport *transport)
> +struct smb2_request *smb2_keepalive_send(struct smb2_transport *transport,
> + struct smb2_session *session)
>  {
>   struct smb2_request *req;
>  
> @@ -35,6 +36,8 @@ struct smb2_request *smb2_keepalive_send(struct smb2_transport *transport)
>  
>   SSVAL(req->out.body, 0x02, 0);
>  
> + req->session = session;
> +
>   smb2_transport_send(req);
>  
>   return req;
> @@ -60,6 +63,6 @@ NTSTATUS smb2_keepalive_recv(struct smb2_request *req)
>  */
>  NTSTATUS smb2_keepalive(struct smb2_transport *transport)
>  {
> - struct smb2_request *req = smb2_keepalive_send(transport);
> + struct smb2_request *req = smb2_keepalive_send(transport, NULL);
>   return smb2_keepalive_recv(req);
>  }
> diff --git a/source4/torture/smb2/session.c b/source4/torture/smb2/session.c
> index fca0552..22073ed 100644
> --- a/source4/torture/smb2/session.c
> +++ b/source4/torture/smb2/session.c
> @@ -31,6 +31,7 @@
>  #include "libcli/security/security.h"
>  #include "libcli/resolve/resolve.h"
>  #include "lib/param/param.h"
> +#include "lib/util/tevent_ntstatus.h"
>  
>  #define CHECK_CREATED(tctx, __io, __created, __attribute) \
>   do { \
> @@ -1167,6 +1168,366 @@ done:
>   return ret;
>  }
>  
> +static bool test_session_expire2(struct torture_context *tctx)
> +{
> + NTSTATUS status;
> + bool ret = false;
> + struct smbcli_options options;
> + const char *host = torture_setting_string(tctx, "host", NULL);
> + const char *share = torture_setting_string(tctx, "share", NULL);
> + struct cli_credentials *credentials = popt_get_cmdline_credentials();
> + struct smb2_tree *tree = NULL;
> + const char *unc = NULL;
> + struct smb2_tree *tree2 = NULL;
> + struct tevent_req *subreq = NULL;
> + uint32_t timeout_msec;
> + enum credentials_use_kerberos use_kerberos;
> + uint32_t caps;
> + char fname[256];
> + struct smb2_handle dh;
> + struct smb2_handle dh2;
> + struct smb2_handle _h1;
> + struct smb2_handle *h1 = NULL;
> + struct smb2_create io1;
> + union smb_fileinfo qfinfo;
> + union smb_setfileinfo sfinfo;
> + struct smb2_flush flsh;
> + struct smb2_read rd;
> + const uint8_t wd = 0;
> + struct smb2_lock lck;
> + struct smb2_lock_element el;
> + struct smb2_ioctl ctl;
> + struct smb2_break oack;
> + struct smb2_lease_break_ack lack;
> + struct smb2_find fnd;
> + union smb_search_data *d = NULL;
> + unsigned int count;
> + struct smb2_request *req = NULL;
> + struct smb2_notify ntf1;
> + struct smb2_notify ntf2;
> +
> + use_kerberos = cli_credentials_get_kerberos_state(credentials);
> + if (use_kerberos != CRED_MUST_USE_KERBEROS) {
> + torture_warning(tctx, "smb2.session.expire2 requires -k yes!");
> + torture_skip(tctx, "smb2.session.expire2 requires -k yes!");
> + }
> +
> + torture_assert_int_equal(tctx, use_kerberos, CRED_MUST_USE_KERBEROS,
> + "please use -k yes");
> +
> + lpcfg_set_option(tctx->lp_ctx, "gensec_gssapi:requested_life_time=4");
> +
> + lpcfg_smbcli_options(tctx->lp_ctx, &options);
> +
> + unc = talloc_asprintf(tctx, "\\\\%s\\%s", host, share);
> + torture_assert(tctx, unc != NULL, "talloc_asprintf");
> +
> + status = smb2_connect(tctx,
> +      host,
> +      lpcfg_smb_ports(tctx->lp_ctx),
> +      share,
> +      lpcfg_resolve_context(tctx->lp_ctx),
> +      credentials,
> +      &tree,
> +      tctx->ev,
> +      &options,
> +      lpcfg_socket_options(tctx->lp_ctx),
> +      lpcfg_gensec_settings(tctx, tctx->lp_ctx)
> +      );
> + torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
> + "smb2_connect failed");
> +
> + caps = smb2cli_conn_server_capabilities(tree->session->transport->conn);
> +
> + /* Add some random component to the file name. */
> + snprintf(fname, sizeof(fname), "session_expire2_%s.dat",
> + generate_random_str(tctx, 8));
> +
> + smb2_util_unlink(tree, fname);
> +
> + status = smb2_util_roothandle(tree, &dh);
> + torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
> + "smb2_util_roothandle failed");
> +
> + smb2_oplock_create_share(&io1, fname,
> + smb2_util_share_access(""),
> + smb2_util_oplock_level("b"));
> + io1.in.create_options |= NTCREATEX_OPTIONS_DELETE_ON_CLOSE;
> +
> + status = smb2_create(tree, tctx, &io1);
> + torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
> + "smb2_create failed");
> + _h1 = io1.out.file.handle;
> + h1 = &_h1;
> + CHECK_CREATED(tctx, &io1, CREATED, FILE_ATTRIBUTE_ARCHIVE);
> + torture_assert_int_equal(tctx, io1.out.oplock_level,
> + smb2_util_oplock_level("b"),
> + "oplock_level incorrect");
> +
> + /* get the security descriptor */
> +
> + ZERO_STRUCT(qfinfo);
> +
> + qfinfo.access_information.level = RAW_FILEINFO_ACCESS_INFORMATION;
> + qfinfo.access_information.in.file.handle = _h1;
> +
> + torture_comment(tctx, "query info => OK\n");
> +
> + ZERO_STRUCT(qfinfo.access_information.out);
> + status = smb2_getinfo_file(tree, tctx, &qfinfo);
> + torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
> + "smb2_getinfo_file failed");
> +
> + torture_comment(tctx, "lock => OK\n");
> + ZERO_STRUCT(lck);
> + lck.in.locks = &el;
> + lck.in.lock_count = 0x0001;
> + lck.in.lock_sequence = 0x00000000;
> + lck.in.file.handle = *h1;
> + ZERO_STRUCT(el);
> + el.flags = SMB2_LOCK_FLAG_EXCLUSIVE |
> +  SMB2_LOCK_FLAG_FAIL_IMMEDIATELY;
> + el.offset = 0x0000000000000000;
> + el.length = 0x0000000000000001;
> + status = smb2_lock(tree, &lck);
> + torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
> + "smb2_lock lock failed");
> +
> + torture_comment(tctx, "1st notify => PENDING\n");
> + ZERO_STRUCT(ntf1);
> + ntf1.in.file.handle = dh;
> + ntf1.in.recursive = 0x0000;
> + ntf1.in.buffer_size = 128;
> + ntf1.in.completion_filter= FILE_NOTIFY_CHANGE_ATTRIBUTES;
> + ntf1.in.unknown = 0x00000000;
> + req = smb2_notify_send(tree, &ntf1);
> +
> + while (!req->cancel.can_cancel && req->state <= SMB2_REQUEST_RECV) {
> + if (tevent_loop_once(tctx->ev) != 0) {
> + break;
> + }
> + }
> +
> + torture_assert_goto(tctx, req->state <= SMB2_REQUEST_RECV, ret, done,
> +    "smb2_notify finished");
> +
> + torture_comment(tctx, "sleep 10 seconds\n");
> + smb_msleep(10*1000);
> +
> + torture_comment(tctx, "query info => EXPIRED\n");
> + ZERO_STRUCT(qfinfo.access_information.out);
> + status = smb2_getinfo_file(tree, tctx, &qfinfo);
> + torture_assert_ntstatus_equal_goto(tctx, status,
> + NT_STATUS_NETWORK_SESSION_EXPIRED,
> + ret, done, "smb2_getinfo_file "
> + "returned unexpected status");
> +
> +
> + torture_comment(tctx, "set info => EXPIRED\n");
> + ZERO_STRUCT(sfinfo);
> + sfinfo.end_of_file_info.level = RAW_SFILEINFO_END_OF_FILE_INFORMATION;
> + sfinfo.end_of_file_info.in.file.handle = *h1;
> + sfinfo.end_of_file_info.in.size = 1;
> + status = smb2_setinfo_file(tree, &sfinfo);
> + torture_assert_ntstatus_equal_goto(tctx, status,
> + NT_STATUS_NETWORK_SESSION_EXPIRED,
> + ret, done, "smb2_setinfo_file "
> + "returned unexpected status");
> +
> + torture_comment(tctx, "flush => EXPIRED\n");
> + ZERO_STRUCT(flsh);
> + flsh.in.file.handle = *h1;
> + status = smb2_flush(tree, &flsh);
> + torture_assert_ntstatus_equal_goto(tctx, status,
> + NT_STATUS_NETWORK_SESSION_EXPIRED,
> + ret, done, "smb2_flush "
> + "returned unexpected status");
> +
> + torture_comment(tctx, "read => EXPIRED\n");
> + ZERO_STRUCT(rd);
> + rd.in.file.handle = *h1;
> + rd.in.length      = 5;
> + rd.in.offset      = 0;
> + status = smb2_read(tree, tctx, &rd);
> + torture_assert_ntstatus_equal_goto(tctx, status,
> + NT_STATUS_NETWORK_SESSION_EXPIRED,
> + ret, done, "smb2_read "
> + "returned unexpected status");
> +
> + torture_comment(tctx, "write => EXPIRED\n");
> + status = smb2_util_write(tree, *h1, &wd, 0, 1);
> + torture_assert_ntstatus_equal_goto(tctx, status,
> + NT_STATUS_NETWORK_SESSION_EXPIRED,
> + ret, done, "smb2_util_write "
> + "returned unexpected status");
> +
> + torture_comment(tctx, "ioctl => EXPIRED\n");
> + ZERO_STRUCT(ctl);
> + ctl.in.file.handle = *h1;
> + ctl.in.function = FSCTL_SRV_ENUM_SNAPS;
> + ctl.in.max_response_size = 16;
> + ctl.in.flags = SMB2_IOCTL_FLAG_IS_FSCTL;
> + status = smb2_ioctl(tree, tctx, &ctl);
> + torture_assert_ntstatus_equal_goto(tctx, status,
> + NT_STATUS_NETWORK_SESSION_EXPIRED,
> + ret, done, "smb2_ioctl "
> + "returned unexpected status");
> +
> + torture_comment(tctx, "oplock ack => EXPIRED\n");
> + ZERO_STRUCT(oack);
> + oack.in.file.handle = *h1;
> + status = smb2_break(tree, &oack);
> + torture_assert_ntstatus_equal_goto(tctx, status,
> + NT_STATUS_NETWORK_SESSION_EXPIRED,
> + ret, done, "smb2_break "
> + "returned unexpected status");
> +
> + if (caps & SMB2_CAP_LEASING) {
> + torture_comment(tctx, "lease ack => EXPIRED\n");
> + ZERO_STRUCT(lack);
> + lack.in.lease.lease_version = 1;
> + lack.in.lease.lease_key.data[0] = 1;
> + lack.in.lease.lease_key.data[0] = 2;
> + status = smb2_lease_break_ack(tree, &lack);
> + torture_assert_ntstatus_equal_goto(tctx, status,
> + NT_STATUS_NETWORK_SESSION_EXPIRED,
> + ret, done, "smb2_break "
> + "returned unexpected status");
> + }
> +
> + torture_comment(tctx, "query directory => EXPIRED\n");
> + ZERO_STRUCT(fnd);
> + fnd.in.file.handle = dh;
> + fnd.in.pattern = "*";
> + fnd.in.continue_flags = SMB2_CONTINUE_FLAG_SINGLE;
> + fnd.in.max_response_size= 0x100;
> + fnd.in.level = SMB2_FIND_BOTH_DIRECTORY_INFO;
> + status = smb2_find_level(tree, tree, &fnd, &count, &d);
> + torture_assert_ntstatus_equal_goto(tctx, status,
> + NT_STATUS_NETWORK_SESSION_EXPIRED,
> + ret, done, "smb2_find_level "
> + "returned unexpected status");
> +
> + torture_comment(tctx, "1st notify => CANCEL\n");
> + smb2_cancel(req);
> +
> + torture_comment(tctx, "2nd notify => EXPIRED\n");
> + ZERO_STRUCT(ntf2);
> + ntf2.in.file.handle = dh;
> + ntf2.in.recursive = 0x0000;
> + ntf2.in.buffer_size = 128;
> + ntf2.in.completion_filter= FILE_NOTIFY_CHANGE_ATTRIBUTES;
> + ntf2.in.unknown = 0x00000000;
> + status = smb2_notify(tree, tctx, &ntf2);
> + torture_assert_ntstatus_equal_goto(tctx, status,
> + NT_STATUS_NETWORK_SESSION_EXPIRED,
> + ret, done, "smb2_notify "
> + "returned unexpected status");
> +
> + torture_assert_goto(tctx, req->state > SMB2_REQUEST_RECV, ret, done,
> +    "smb2_notify (1st) not finished");
> +
> + status = smb2_notify_recv(req, tctx, &ntf1);
> + torture_assert_ntstatus_equal_goto(tctx, status,
> + NT_STATUS_CANCELLED,
> + ret, done, "smb2_notify cancelled"
> + "returned unexpected status");
> +
> + torture_comment(tctx, "tcon => EXPIRED\n");
> + tree2 = smb2_tree_init(tree->session, tctx, false);
> + torture_assert(tctx, tree2 != NULL, "smb2_tree_init");
> + timeout_msec = tree->session->transport->options.request_timeout * 1000;
> + subreq = smb2cli_tcon_send(tree2, tctx->ev,
> +   tree2->session->transport->conn,
> +   timeout_msec,
> +   tree2->session->smbXcli,
> +   tree2->smbXcli,
> +   0, /* flags */
> +   unc);
> + torture_assert(tctx, subreq != NULL, "smb2cli_tcon_send");
> + torture_assert(tctx,
> +       tevent_req_poll_ntstatus(subreq, tctx->ev, &status),
> +       "tevent_req_poll_ntstatus");
> + status = smb2cli_tcon_recv(subreq);
> + TALLOC_FREE(subreq);
> + torture_assert_ntstatus_equal_goto(tctx, status,
> + NT_STATUS_NETWORK_SESSION_EXPIRED,
> + ret, done, "smb2cli_tcon"
> + "returned unexpected status");
> +
> + torture_comment(tctx, "create => EXPIRED\n");
> + status = smb2_util_roothandle(tree, &dh2);
> + torture_assert_ntstatus_equal_goto(tctx, status,
> + NT_STATUS_NETWORK_SESSION_EXPIRED,
> + ret, done, "smb2_util_roothandle"
> + "returned unexpected status");
> +
> + torture_comment(tctx, "tdis => EXPIRED\n");
> + status = smb2_tdis(tree);
> + torture_assert_ntstatus_equal_goto(tctx, status,
> + NT_STATUS_NETWORK_SESSION_EXPIRED,
> + ret, done, "smb2cli_tdis"
> + "returned unexpected status");
> +
> + /*
> + * (Un)Lock, Close and Logoff are still possible
> + */
> +
> + torture_comment(tctx, "1st unlock => OK\n");
> + el.flags = SMB2_LOCK_FLAG_UNLOCK;
> + status = smb2_lock(tree, &lck);
> + torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
> + "smb2_lock unlock failed");
> +
> + torture_comment(tctx, "2nd unlock => RANGE_NOT_LOCKED\n");
> + status = smb2_lock(tree, &lck);
> + torture_assert_ntstatus_equal_goto(tctx, status,
> + NT_STATUS_RANGE_NOT_LOCKED,
> + ret, done, "smb2_lock 2nd unlock"
> + "returned unexpected status");
> +
> + torture_comment(tctx, "lock => EXPIRED\n");
> + el.flags = SMB2_LOCK_FLAG_EXCLUSIVE |
> +  SMB2_LOCK_FLAG_FAIL_IMMEDIATELY;
> + status = smb2_lock(tree, &lck);
> + torture_assert_ntstatus_equal_goto(tctx, status,
> + NT_STATUS_NETWORK_SESSION_EXPIRED,
> + ret, done, "smb2_util_roothandle"
> + "returned unexpected status");
> +
> + torture_comment(tctx, "close => OK\n");
> + status = smb2_util_close(tree, *h1);
> + h1 = NULL;
> + torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
> + "smb2_close failed");
> +
> + torture_comment(tctx, "echo without session => OK\n");
> + status = smb2_keepalive(tree->session->transport);
> + torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
> + "smb2_keepalive without session failed");
> +
> + torture_comment(tctx, "echo with session => OK\n");
> + req = smb2_keepalive_send(tree->session->transport, tree->session);
> + status = smb2_keepalive_recv(req);
> + torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
> + "smb2_keepalive with session failed");
> +
> + torture_comment(tctx, "logoff => OK\n");
> + status = smb2_logoff(tree->session);
> + torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
> + "smb2_logoff failed");
> +
> + ret = true;
> +done:
> + if (h1 != NULL) {
> + smb2_util_close(tree, *h1);
> + }
> +
> + talloc_free(tree);
> + lpcfg_set_option(tctx->lp_ctx, "gensec_gssapi:requested_life_time=0");
> + return ret;
> +}
> +
>  bool test_session_bind1(struct torture_context *tctx, struct smb2_tree *tree1)
>  {
>   const char *host = torture_setting_string(tctx, "host", NULL);
> @@ -1321,6 +1682,7 @@ struct torture_suite *torture_smb2_session_init(TALLOC_CTX *ctx)
>   torture_suite_add_1smb2_test(suite, "reauth5", test_session_reauth5);
>   torture_suite_add_1smb2_test(suite, "reauth6", test_session_reauth6);
>   torture_suite_add_simple_test(suite, "expire1", test_session_expire1);
> + torture_suite_add_simple_test(suite, "expire2", test_session_expire2);
>   torture_suite_add_1smb2_test(suite, "bind1", test_session_bind1);
>  
>   suite->description = talloc_strdup(suite, "SMB2-SESSION tests");
> --
> 1.9.1
>
>
> From 1a02e02b7fc8e3a99eba9e89801551d5607e4c63 Mon Sep 17 00:00:00 2001
> From: Stefan Metzmacher <[hidden email]>
> Date: Thu, 21 Dec 2017 14:47:06 +0100
> Subject: [PATCH 2/3] s3:smbd: return the correct error for cancelled SMB2
>  notifies on expired sessions
>
> BUG: https://bugzilla.samba.org/show_bug.cgi?id=13197
>
> Signed-off-by: Stefan Metzmacher <[hidden email]>
> ---
>  source3/smbd/notify.c | 17 +++++++++++++----
>  1 file changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/source3/smbd/notify.c b/source3/smbd/notify.c
> index f64185d..add59089 100644
> --- a/source3/smbd/notify.c
> +++ b/source3/smbd/notify.c
> @@ -391,12 +391,21 @@ static void smbd_notify_cancel_by_map(struct notify_mid_map *map)
>   NTSTATUS notify_status = NT_STATUS_CANCELLED;
>  
>   if (smb2req != NULL) {
> + NTSTATUS sstatus;
> +
>   if (smb2req->session == NULL) {
> - notify_status = STATUS_NOTIFY_CLEANUP;
> - } else if (!NT_STATUS_IS_OK(smb2req->session->status)) {
> - notify_status = STATUS_NOTIFY_CLEANUP;
> + sstatus = NT_STATUS_USER_SESSION_DELETED;
> + } else {
> + sstatus = smb2req->session->status;
>   }
> - if (smb2req->tcon == NULL) {
> +
> + if (NT_STATUS_EQUAL(sstatus, NT_STATUS_NETWORK_SESSION_EXPIRED)) {
> + sstatus = NT_STATUS_OK;
> + }
> +
> + if (!NT_STATUS_IS_OK(sstatus)) {
> + notify_status = STATUS_NOTIFY_CLEANUP;
> + } else if (smb2req->tcon == NULL) {
>   notify_status = STATUS_NOTIFY_CLEANUP;
>   } else if (!NT_STATUS_IS_OK(smb2req->tcon->status)) {
>   notify_status = STATUS_NOTIFY_CLEANUP;
> --
> 1.9.1
>
>
> From 94c4c63082dae90b6959fe7e7ccfd3c59575dc11 Mon Sep 17 00:00:00 2001
> From: Stefan Metzmacher <[hidden email]>
> Date: Wed, 20 Dec 2017 14:05:54 +0100
> Subject: [PATCH 3/3] s3:smb2_server: allow logoff, close, unlock, cancel and
>  echo on expired sessions
>
> Windows client at least doesn't have code to replay
> a SMB2 Close after getting NETWORK_SESSION_EXPIRED,
> which locks out a the client and generates an endless
> loop around NT_STATUS_SHARING_VIOLATION.
>
> BUG: https://bugzilla.samba.org/show_bug.cgi?id=13197
>
> Signed-off-by: Stefan Metzmacher <[hidden email]>
> ---
>  selftest/knownfail.d/session.expire2 |  1 -
>  source3/smbd/smb2_lock.c             | 17 +++++++++++++++++
>  source3/smbd/smb2_server.c           | 19 +++++++++++++++++++
>  3 files changed, 36 insertions(+), 1 deletion(-)
>  delete mode 100644 selftest/knownfail.d/session.expire2
>
> diff --git a/selftest/knownfail.d/session.expire2 b/selftest/knownfail.d/session.expire2
> deleted file mode 100644
> index 998ccbd..0000000
> --- a/selftest/knownfail.d/session.expire2
> +++ /dev/null
> @@ -1 +0,0 @@
> -^samba3.smb2.session.*krb5.expire2
> diff --git a/source3/smbd/smb2_lock.c b/source3/smbd/smb2_lock.c
> index 2fcd359..45b833c 100644
> --- a/source3/smbd/smb2_lock.c
> +++ b/source3/smbd/smb2_lock.c
> @@ -98,6 +98,23 @@ NTSTATUS smbd_smb2_request_process_lock(struct smbd_smb2_request *req)
>   in_locks[l].flags = IVAL(lock_buffer, 0x10);
>   /* 0x14 - 4 reserved bytes */
>  
> + status = req->session->status;
> + if (NT_STATUS_EQUAL(status, NT_STATUS_NETWORK_SESSION_EXPIRED)) {
> + /*
> + * We need to catch NT_STATUS_NETWORK_SESSION_EXPIRED
> + * for lock requests only.
> + *
> + * Unlock requests still need to be processed!
> + *
> + * This means smbd_smb2_request_check_session()
> + * can't handle the difference and always
> + * allows SMB2_OP_LOCK.
> + */
> + if (in_locks[0].flags != SMB2_LOCK_FLAG_UNLOCK) {
> + return smbd_smb2_request_error(req, status);
> + }
> + }
> +
>   lock_buffer = SMBD_SMB2_IN_DYN_PTR(req);
>  
>   for (l=1; l < in_lock_count; l++) {
> diff --git a/source3/smbd/smb2_server.c b/source3/smbd/smb2_server.c
> index 68a024f..5290c05 100644
> --- a/source3/smbd/smb2_server.c
> +++ b/source3/smbd/smb2_server.c
> @@ -1902,6 +1902,25 @@ static NTSTATUS smbd_smb2_request_check_session(struct smbd_smb2_request *req)
>   case SMB2_OP_SESSSETUP:
>   status = NT_STATUS_OK;
>   break;
> + case SMB2_OP_LOGOFF:
> + case SMB2_OP_CLOSE:
> + case SMB2_OP_LOCK:
> + case SMB2_OP_CANCEL:
> + case SMB2_OP_KEEPALIVE:
> + /*
> + * [MS-SMB2] 3.3.5.2.9 Verifying the Session
> + * specifies that LOGOFF, CLOSE and (UN)LOCK
> + * should always be processed even on expired sessions.
> + *
> + * Also see the logic in
> + * smbd_smb2_request_process_lock().
> + *
> + * The smb2.session.expire2 test shows that
> + * CANCEL and KEEPALIVE/ECHO should also
> + * be processed.
> + */
> + status = NT_STATUS_OK;
> + break;
>   default:
>   break;
>   }
> --
> 1.9.1
>