[Patches] SESSION_EXPIRED on encrypted sessions (bug #13624)
Stefan Metzmacher
metze at samba.org
Mon Oct 1 08:43:21 UTC 2018
Hi,
here're patches which demonstrate bug #13624 ( STATUS_SESSION_EXPIRED
error is returned unencrypted, if the request was encrypted)
and fix it.
Here's a merge request
https://gitlab.com/samba-team/samba/merge_requests/81
and the related pipeline:
https://gitlab.com/samba-team/devel/samba/pipelines/31492348
Please review and push:-)
Thanks!
metze
-------------- next part --------------
From 055b738fb4309d322ea8bb4c225c967b6f1b0a9d Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Fri, 28 Sep 2018 12:23:37 +0200
Subject: [PATCH 1/2] s4:torture: split smb2.session.expire{1,2} to run with
signing and encryptpion
This reproduces the problem we have with expired encrypted sessions.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13624
Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
selftest/knownfail.d/session-expire | 2 ++
source4/torture/smb2/session.c | 50 ++++++++++++++++++++++++++---
2 files changed, 48 insertions(+), 4 deletions(-)
create mode 100644 selftest/knownfail.d/session-expire
diff --git a/selftest/knownfail.d/session-expire b/selftest/knownfail.d/session-expire
new file mode 100644
index 000000000000..033564afb58d
--- /dev/null
+++ b/selftest/knownfail.d/session-expire
@@ -0,0 +1,2 @@
+^samba3.smb2.session krb5.expire1e
+^samba3.smb2.session krb5.expire2e
diff --git a/source4/torture/smb2/session.c b/source4/torture/smb2/session.c
index f3fa596e4648..7dc9ba19ee6e 100644
--- a/source4/torture/smb2/session.c
+++ b/source4/torture/smb2/session.c
@@ -1046,7 +1046,8 @@ done:
}
-static bool test_session_expire1(struct torture_context *tctx)
+static bool test_session_expire1i(struct torture_context *tctx,
+ bool force_encryption)
{
NTSTATUS status;
bool ret = false;
@@ -1075,6 +1076,7 @@ static bool test_session_expire1(struct torture_context *tctx)
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,
@@ -1091,6 +1093,12 @@ static bool test_session_expire1(struct torture_context *tctx)
torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
"smb2_connect failed");
+ if (force_encryption) {
+ status = smb2cli_session_encryption_on(tree->session->smbXcli);
+ torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
+ "smb2cli_session_encryption_on failed");
+ }
+
/* Add some random component to the file name. */
snprintf(fname, sizeof(fname), "session_expire1_%s.dat",
generate_random_str(tctx, 8));
@@ -1168,7 +1176,20 @@ done:
return ret;
}
-static bool test_session_expire2(struct torture_context *tctx)
+static bool test_session_expire1s(struct torture_context *tctx)
+{
+ return test_session_expire1i(tctx,
+ false); /* force_encryption */
+}
+
+static bool test_session_expire1e(struct torture_context *tctx)
+{
+ return test_session_expire1i(tctx,
+ true); /* force_encryption */
+}
+
+static bool test_session_expire2i(struct torture_context *tctx,
+ bool force_encryption)
{
NTSTATUS status;
bool ret = false;
@@ -1218,6 +1239,7 @@ static bool test_session_expire2(struct torture_context *tctx)
lpcfg_set_option(tctx->lp_ctx, "gensec_gssapi:requested_life_time=4");
lpcfg_smbcli_options(tctx->lp_ctx, &options);
+ options.signing = SMB_SIGNING_REQUIRED;
unc = talloc_asprintf(tctx, "\\\\%s\\%s", host, share);
torture_assert(tctx, unc != NULL, "talloc_asprintf");
@@ -1237,6 +1259,12 @@ static bool test_session_expire2(struct torture_context *tctx)
torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
"smb2_connect failed");
+ if (force_encryption) {
+ status = smb2cli_session_encryption_on(tree->session->smbXcli);
+ torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
+ "smb2cli_session_encryption_on failed");
+ }
+
caps = smb2cli_conn_server_capabilities(tree->session->transport->conn);
/* Add some random component to the file name. */
@@ -1528,6 +1556,18 @@ done:
return ret;
}
+static bool test_session_expire2s(struct torture_context *tctx)
+{
+ return test_session_expire2i(tctx,
+ false); /* force_encryption */
+}
+
+static bool test_session_expire2e(struct torture_context *tctx)
+{
+ return test_session_expire2i(tctx,
+ true); /* force_encryption */
+}
+
bool test_session_bind1(struct torture_context *tctx, struct smb2_tree *tree1)
{
const char *host = torture_setting_string(tctx, "host", NULL);
@@ -1681,8 +1721,10 @@ struct torture_suite *torture_smb2_session_init(TALLOC_CTX *ctx)
torture_suite_add_1smb2_test(suite, "reauth4", test_session_reauth4);
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_simple_test(suite, "expire1s", test_session_expire1s);
+ 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_1smb2_test(suite, "bind1", test_session_bind1);
suite->description = talloc_strdup(suite, "SMB2-SESSION tests");
--
2.17.1
From c185c1fd8ca336581dd1594b59341e4ee621dd93 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Fri, 17 Aug 2018 11:35:41 +0200
Subject: [PATCH 2/2] smb2_server: set req->do_encryption = true earlier
The STATUS_SESSION_EXPIRED error was returned unencrypted,
if the request was encrypted.
If clients use SMB3 encryption and the kerberos authenticated session
expires, clients disconnect the connection instead of doing a reauthentication.
From https://blogs.msdn.microsoft.com/openspecification/2012/10/05/encryption-in-smb-3-0-a-protocol-perspective/
The sender encrypts the message if any of the following conditions is
satisfied:
- If the sender is sending a response to an encrypted request.
- If Session.EncryptData is TRUE and the request or response being
sent is not NEGOTIATE.
- If Session.EncryptData is FALSE, the request or response being sent
is not NEGOTIATE or SESSION_SETUP or TREE_CONNECT, and
<TreeConnect|Share>.EncryptData is TRUE.
[MS-SMB2] 3.3.4.1.4 Encrypting the Message
If Connection.Dialect belongs to the SMB 3.x dialect family and
Connection.ClientCapabilities includes the SMB2_GLOBAL_CAP_ENCRYPTION
bit, the server MUST encrypt the message before sending, if any of the
following conditions are satisfied:
- If the message being sent is any response to a client request for which
Request.IsEncrypted is TRUE.
- If Session.EncryptData is TRUE and the response being sent is not
SMB2_NEGOTIATE or SMB2 SESSION_SETUP.
- If Session.EncryptData is FALSE, the response being sent is not
SMB2_NEGOTIATE or SMB2 SESSION_SETUP or SMB2 TREE_CONNECT, and
Share.EncryptData for the share associated with the TreeId in the SMB2
header of the response is TRUE.
The server MUST encrypt the message as specified in section 3.1.4.3,
before sending it to the client.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13624
Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
selftest/knownfail.d/session-expire | 2 --
source3/smbd/smb2_server.c | 15 ++++++++++-----
2 files changed, 10 insertions(+), 7 deletions(-)
delete mode 100644 selftest/knownfail.d/session-expire
diff --git a/selftest/knownfail.d/session-expire b/selftest/knownfail.d/session-expire
deleted file mode 100644
index 033564afb58d..000000000000
--- a/selftest/knownfail.d/session-expire
+++ /dev/null
@@ -1,2 +0,0 @@
-^samba3.smb2.session krb5.expire1e
-^samba3.smb2.session krb5.expire2e
diff --git a/source3/smbd/smb2_server.c b/source3/smbd/smb2_server.c
index 229f8ab0e856..e36db1e55f5e 100644
--- a/source3/smbd/smb2_server.c
+++ b/source3/smbd/smb2_server.c
@@ -2359,7 +2359,11 @@ NTSTATUS smbd_smb2_request_dispatch(struct smbd_smb2_request *req)
req->async_internal = false;
req->do_signing = false;
- req->do_encryption = false;
+ if (opcode != SMB2_OP_SESSSETUP) {
+ req->do_encryption = encryption_desired;
+ } else {
+ req->do_encryption = false;
+ }
req->was_encrypted = false;
if (intf_v->iov_len == SMB2_TF_HDR_SIZE) {
const uint8_t *intf = SMBD_SMB2_IN_TF_PTR(req);
@@ -2383,9 +2387,11 @@ NTSTATUS smbd_smb2_request_dispatch(struct smbd_smb2_request *req)
}
req->was_encrypted = true;
+ req->do_encryption = true;
}
if (encryption_required && !req->was_encrypted) {
+ req->do_encryption = true;
return smbd_smb2_request_error(req,
NT_STATUS_ACCESS_DENIED);
}
@@ -2523,8 +2529,11 @@ NTSTATUS smbd_smb2_request_dispatch(struct smbd_smb2_request *req)
encryption_required = true;
}
if (encryption_required && !req->was_encrypted) {
+ req->do_encryption = true;
return smbd_smb2_request_error(req,
NT_STATUS_ACCESS_DENIED);
+ } else if (encryption_desired) {
+ req->do_encryption = true;
}
} else if (call->need_session) {
struct auth_session_info *session_info = NULL;
@@ -2544,10 +2553,6 @@ NTSTATUS smbd_smb2_request_dispatch(struct smbd_smb2_request *req)
session_info->info->domain_name);
}
- if (req->was_encrypted || encryption_desired) {
- req->do_encryption = true;
- }
-
if (req->session) {
bool update_session_global = false;
bool update_tcon_global = false;
--
2.17.1
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20181001/43cc6e89/signature.sig>
More information about the samba-technical
mailing list