Attachment 14520 Details for Bug 13624 – Patches for v4-8-test

[patch] Patches for v4-8-test

tmp48.diff.txt (text/plain), 9.12 KB, created by Stefan Metzmacher on 2018-10-09 14:34:24 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Stefan Metzmacher

Created: 2018-10-09 14:34:24 UTC

Size: 9.12 KB

patch

obsolete

>From a17c58b732b441e2ed9ee0578e241696a88d3c25 Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@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@samba.org>
>Reviewed-by: Jeremy Allison <jra@samba.org>
>(cherry picked from commit 01b868455c9bae309d1ca7ddad54077fc5d7f4b1)
>---
> 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 8400ff2b0c31b3279d140b4ce31d312fa9f1229d Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@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@samba.org>
>Reviewed-by: Jeremy Allison <jra@samba.org>
>
>Autobuild-User(master): Volker Lendecke <vl@samba.org>
>Autobuild-Date(master): Tue Oct  2 14:11:30 CEST 2018 on sn-devel-144
>
>(cherry picked from commit 4ef45e5334d5874f5d0fdc69286b745ebcdc612d)
>---
> 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 177e5ffc2f2f..af065e9e43ff 100644
>--- a/source3/smbd/smb2_server.c
>+++ b/source3/smbd/smb2_server.c
>@@ -2364,7 +2364,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);
>@@ -2388,9 +2392,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);
> 	}
>@@ -2526,15 +2532,14 @@ 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;
> 		}
> 	}
> 
>-	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
>

Flags:

jra: review+

Actions: View

Attachments on bug 13624: 14505 | 14519 | 14520 | 14521