[PATCH] Fix for reopened bug 9175
Ralph Böhme
slow at samba.org
Tue Nov 13 11:28:17 UTC 2018
On Tue, Nov 13, 2018 at 09:12:42AM +0100, Ralph Böhme wrote:
>On Tue, Nov 13, 2018 at 08:17:51AM +0100, Stefan Metzmacher wrote:
>>Hi Ralph,
>>
>>>On Mon, Nov 12, 2018 at 04:51:29PM +0100, Andreas Schneider wrote:
>>>>On Monday, 12 November 2018 12:17:59 CET Ralph Böhme via samba-technical
>>>>wrote:
>>>>>Hi metze,
>>>>>
>>>>>shall we apply the attached patch?
>>>>
>>>>I think this is more correct so +1 from my side.
>>>
>>>thanks.
>>>
>>>metze, what do you think?
>>
>>Can we create a test for this that verifies that
>>smbXcli_conn_is_connected() returns false,
>>when a request fails with SESSION_EXPIRED?
>
>sure. Let me see...
attached.
The second patch also passed CI as part of
https://gitlab.com/samba-team/devel/samba/pipelines/36191198
Plus it passes manual make test TESTS=smb2.session
Please review&push if happy. Thanks!
-slow
--
Ralph Boehme, Samba Team https://samba.org/
Samba Developer, SerNet GmbH https://sernet.de/en/samba/
GPG Key Fingerprint: FAE2 C608 8A24 2520 51C5
59E4 AA1E 9B71 2639 9E46
-------------- next part --------------
From dffeb4c83ede9b866bade84f298925826a58b1a2 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Tue, 13 Nov 2018 12:08:10 +0100
Subject: [PATCH 1/2] s4:torture/smb2/session: test
smbXcli_session_set_disconnect_expired() works
This adds a simple test that verifies that after having set
smbXcli_session_set_disconnect_expired() a session gets disconnected
when it expires.
Bug: https://bugzilla.samba.org/show_bug.cgi?id=9175
Signed-off-by: Ralph Boehme <slow at samba.org>
---
selftest/knownfail.d/samba3.smb2.session | 2 +
source4/torture/smb2/session.c | 110 +++++++++++++++++++++++
2 files changed, 112 insertions(+)
create mode 100644 selftest/knownfail.d/samba3.smb2.session
diff --git a/selftest/knownfail.d/samba3.smb2.session b/selftest/knownfail.d/samba3.smb2.session
new file mode 100644
index 00000000000..1ef3605d6b8
--- /dev/null
+++ b/selftest/knownfail.d/samba3.smb2.session
@@ -0,0 +1,2 @@
+^samba3.smb2.session krb5.expire_disconnect\(ad_dc\)
+^samba3.smb2.session krb5.expire_disconnect\(ad_member\)
diff --git a/source4/torture/smb2/session.c b/source4/torture/smb2/session.c
index 57a5addcfcc..3917e0c09c4 100644
--- a/source4/torture/smb2/session.c
+++ b/source4/torture/smb2/session.c
@@ -1596,6 +1596,114 @@ static bool test_session_expire2e(struct torture_context *tctx)
true); /* force_encryption */
}
+static bool test_session_expire_disconnect(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;
+ enum credentials_use_kerberos use_kerberos;
+ char fname[256];
+ struct smb2_handle _h1;
+ struct smb2_handle *h1 = NULL;
+ struct smb2_create io1;
+ union smb_fileinfo qfinfo;
+ bool connected;
+
+ use_kerberos = cli_credentials_get_kerberos_state(credentials);
+ if (use_kerberos != CRED_MUST_USE_KERBEROS) {
+ torture_warning(tctx, "smb2.session.expire1 requires -k yes!");
+ torture_skip(tctx, "smb2.session.expire1 requires -k yes!");
+ }
+
+ cli_credentials_invalidate_ccache(credentials, CRED_SPECIFIED);
+
+ lpcfg_set_option(tctx->lp_ctx, "gensec_gssapi:requested_life_time=4");
+ lpcfg_smbcli_options(tctx->lp_ctx, &options);
+ options.signing = SMB_SIGNING_REQUIRED;
+
+ 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");
+
+ smbXcli_session_set_disconnect_expired(tree->session->smbXcli);
+
+ /* Add some random component to the file name. */
+ snprintf(fname, sizeof(fname), "session_expire1_%s.dat",
+ generate_random_str(tctx, 8));
+
+ smb2_util_unlink(tree, fname);
+
+ 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, "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");
+
+ connected = smbXcli_conn_is_connected(tree->session->transport->conn);
+ torture_assert_goto(tctx, !connected, ret, done, "connected\n");
+
+ ret = true;
+done:
+ cli_credentials_invalidate_ccache(credentials, CRED_SPECIFIED);
+
+ 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);
@@ -1754,6 +1862,8 @@ struct torture_suite *torture_smb2_session_init(TALLOC_CTX *ctx)
torture_suite_add_simple_test(suite, "expire1e", test_session_expire1e);
torture_suite_add_simple_test(suite, "expire2s", test_session_expire2s);
torture_suite_add_simple_test(suite, "expire2e", test_session_expire2e);
+ torture_suite_add_simple_test(suite, "expire_disconnect",
+ test_session_expire_disconnect);
torture_suite_add_1smb2_test(suite, "bind1", test_session_bind1);
suite->description = talloc_strdup(suite, "SMB2-SESSION tests");
--
2.17.2
From b8d84d6e0584e77d275e9959a568c8d705eff1d3 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Wed, 7 Nov 2018 14:00:25 +0100
Subject: [PATCH 2/2] libcli/smb: don't overwrite status code
The original commit c5cd22b5bbce724dcd68fe94320382b3f772cabf from bug
9175 never worked, as the preceeding signing check overwrote the status
variable.
Bug: https://bugzilla.samba.org/show_bug.cgi?id=9175
Signed-off-by: Ralph Boehme <slow at samba.org>
---
libcli/smb/smbXcli_base.c | 12 +++++++-----
selftest/knownfail.d/samba3.smb2.session | 2 --
2 files changed, 7 insertions(+), 7 deletions(-)
delete mode 100644 selftest/knownfail.d/samba3.smb2.session
diff --git a/libcli/smb/smbXcli_base.c b/libcli/smb/smbXcli_base.c
index d0cc33b8b05..40480c83aa0 100644
--- a/libcli/smb/smbXcli_base.c
+++ b/libcli/smb/smbXcli_base.c
@@ -3908,15 +3908,17 @@ static NTSTATUS smb2cli_conn_dispatch_incoming(struct smbXcli_conn *conn,
}
if (signing_key) {
- status = smb2_signing_check_pdu(*signing_key,
- state->conn->protocol,
- &cur[1], 3);
- if (!NT_STATUS_IS_OK(status)) {
+ NTSTATUS signing_status;
+
+ signing_status = smb2_signing_check_pdu(*signing_key,
+ state->conn->protocol,
+ &cur[1], 3);
+ if (!NT_STATUS_IS_OK(signing_status)) {
/*
* If the signing check fails, we disconnect
* the connection.
*/
- return status;
+ return signing_status;
}
}
diff --git a/selftest/knownfail.d/samba3.smb2.session b/selftest/knownfail.d/samba3.smb2.session
deleted file mode 100644
index 1ef3605d6b8..00000000000
--- a/selftest/knownfail.d/samba3.smb2.session
+++ /dev/null
@@ -1,2 +0,0 @@
-^samba3.smb2.session krb5.expire_disconnect\(ad_dc\)
-^samba3.smb2.session krb5.expire_disconnect\(ad_member\)
--
2.17.2
More information about the samba-technical
mailing list