From 8de644323ea5ab404ce29aad9a71d30a12d45373 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher 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 Reviewed-by: Jeremy Allison (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 2e03234af81dd42d6bbc67dec8fe2037fd50158c Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher 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 .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 Reviewed-by: Jeremy Allison Autobuild-User(master): Volker Lendecke 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 0c1ac3238918..2e83b7708f64 100644 --- a/source3/smbd/smb2_server.c +++ b/source3/smbd/smb2_server.c @@ -2358,7 +2358,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); @@ -2382,9 +2386,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); } @@ -2522,8 +2528,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; @@ -2543,10 +2552,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