Attachment 14623 Details for Bug 13661 – WIP patch for master with test

[patch] WIP patch for master with test

bug13661-wip.patch (text/plain), 8.08 KB, created by Ralph Böhme on 2018-11-07 16:01:21 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Ralph Böhme

Created: 2018-11-07 16:01:21 UTC

Size: 8.08 KB

patch

obsolete

>From 5dbaf823cd5630c01e6ad21007696a7b8fc40315 Mon Sep 17 00:00:00 2001
>From: Ralph Boehme <slow@samba.org>
>Date: Wed, 7 Nov 2018 14:00:25 +0100
>Subject: [PATCH 1/5] libcli: don't overwrite status code
>
>---
> libcli/smb/smbXcli_base.c | 12 +++++++-----
> 1 file changed, 7 insertions(+), 5 deletions(-)
>
>diff --git a/libcli/smb/smbXcli_base.c b/libcli/smb/smbXcli_base.c
>index 9edb6292777..93b4c18e407 100644
>--- a/libcli/smb/smbXcli_base.c
>+++ b/libcli/smb/smbXcli_base.c
>@@ -3889,15 +3889,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;
> 			}
> 		}
> 
>-- 
>2.17.2
>
>
>From 36d8c70e8092a8c51cc3444f4f3041355afe9c76 Mon Sep 17 00:00:00 2001
>From: Ralph Boehme <slow@samba.org>
>Date: Wed, 7 Nov 2018 16:42:22 +0100
>Subject: [PATCH 2/5] libcli/smb: add smbXcli_session_signing_key()
>
>---
> libcli/smb/smbXcli_base.c | 22 ++++++++++++++++++++++
> libcli/smb/smbXcli_base.h |  2 ++
> 2 files changed, 24 insertions(+)
>
>diff --git a/libcli/smb/smbXcli_base.c b/libcli/smb/smbXcli_base.c
>index 93b4c18e407..2afac33b805 100644
>--- a/libcli/smb/smbXcli_base.c
>+++ b/libcli/smb/smbXcli_base.c
>@@ -5569,6 +5569,28 @@ NTSTATUS smbXcli_session_application_key(struct smbXcli_session *session,
> 	return NT_STATUS_OK;
> }
> 
>+NTSTATUS smbXcli_session_signing_key(struct smbXcli_session *session,
>+				     DATA_BLOB *_key)
>+{
>+	DATA_BLOB key = data_blob_null;
>+
>+	if (session->conn == NULL) {
>+		return NT_STATUS_NO_USER_SESSION_KEY;
>+	}
>+
>+	if (session->conn->protocol >= PROTOCOL_SMB2_02) {
>+		key = session->smb2_channel.signing_key;
>+	}
>+
>+	if (key.length == 0) {
>+		return NT_STATUS_NO_USER_SESSION_KEY;
>+	}
>+
>+	*_key = key;
>+
>+	return NT_STATUS_OK;
>+}
>+
> void smbXcli_session_set_disconnect_expired(struct smbXcli_session *session)
> {
> 	session->disconnect_expired = true;
>diff --git a/libcli/smb/smbXcli_base.h b/libcli/smb/smbXcli_base.h
>index 536c7ab60f4..5667a5d2607 100644
>--- a/libcli/smb/smbXcli_base.h
>+++ b/libcli/smb/smbXcli_base.h
>@@ -471,6 +471,8 @@ bool smbXcli_session_is_authenticated(struct smbXcli_session *session);
> NTSTATUS smbXcli_session_application_key(struct smbXcli_session *session,
> 					 TALLOC_CTX *mem_ctx,
> 					 DATA_BLOB *key);
>+NTSTATUS smbXcli_session_signing_key(struct smbXcli_session *session,
>+				     DATA_BLOB *_key);
> void smbXcli_session_set_disconnect_expired(struct smbXcli_session *session);
> uint16_t smb1cli_session_current_id(struct smbXcli_session* session);
> void smb1cli_session_set_id(struct smbXcli_session* session,
>-- 
>2.17.2
>
>
>From 856f5e91987acf54ade4fc15ceb39ce4e3f3fbb8 Mon Sep 17 00:00:00 2001
>From: Ralph Boehme <slow@samba.org>
>Date: Wed, 7 Nov 2018 16:42:36 +0100
>Subject: [PATCH 3/5] s4:libcli/smb2: check session-setup reauth response
> signature
>
>---
> source4/libcli/smb2/session.c | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>
>diff --git a/source4/libcli/smb2/session.c b/source4/libcli/smb2/session.c
>index e94512d3d33..fed94b265a6 100644
>--- a/source4/libcli/smb2/session.c
>+++ b/source4/libcli/smb2/session.c
>@@ -381,6 +381,22 @@ static void smb2_session_setup_spnego_both_ready(struct tevent_req *req)
> 	}
> 
> 	if (state->reauth) {
>+		DATA_BLOB key;
>+		enum protocol_types protocol;
>+
>+		status = smbXcli_session_signing_key(session->smbXcli, &key);
>+		if (tevent_req_nterror(req, status)) {
>+			return;
>+		}
>+
>+		protocol = smbXcli_conn_protocol(session->transport->conn);
>+
>+		status = smb2_signing_check_pdu(key,
>+						protocol,
>+						state->recv_iov, 3);
>+		if (tevent_req_nterror(req, status)) {
>+			return;
>+		}
> 		tevent_req_done(req);
> 		return;
> 	}
>-- 
>2.17.2
>
>
>From 63ac5c2520babfc37358d228fe49ae0b08d7de3d Mon Sep 17 00:00:00 2001
>From: Ralph Boehme <slow@samba.org>
>Date: Wed, 7 Nov 2018 15:28:58 +0100
>Subject: [PATCH 4/5] s4:torture/smb2/session: add another session expiration
> test
>
>This tests session expiration without forcing signing.
>---
> source4/torture/smb2/session.c | 15 ++++++++++++++-
> 1 file changed, 14 insertions(+), 1 deletion(-)
>
>diff --git a/source4/torture/smb2/session.c b/source4/torture/smb2/session.c
>index 7dc9ba19ee6..9adbc596cef 100644
>--- a/source4/torture/smb2/session.c
>+++ b/source4/torture/smb2/session.c
>@@ -1047,6 +1047,7 @@ bool test_session_reauth6(struct torture_context *tctx, struct smb2_tree *tree)
> 
> 
> static bool test_session_expire1i(struct torture_context *tctx,
>+				  bool force_signing,
> 				  bool force_encryption)
> {
> 	NTSTATUS status;
>@@ -1076,7 +1077,9 @@ static bool test_session_expire1i(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;
>+	if (force_signing) {
>+		options.signing = SMB_SIGNING_REQUIRED;
>+	}
> 
> 	status = smb2_connect(tctx,
> 			      host,
>@@ -1176,15 +1179,24 @@ static bool test_session_expire1i(struct torture_context *tctx,
> 	return ret;
> }
> 
>+static bool test_session_expire1n(struct torture_context *tctx)
>+{
>+	return test_session_expire1i(tctx,
>+				     false,   /* force_signing */
>+				     false); /* force_encryption */
>+}
>+
> static bool test_session_expire1s(struct torture_context *tctx)
> {
> 	return test_session_expire1i(tctx,
>+				     true,   /* force_signing */
> 				     false); /* force_encryption */
> }
> 
> static bool test_session_expire1e(struct torture_context *tctx)
> {
> 	return test_session_expire1i(tctx,
>+				     true,   /* force_signing */
> 				     true); /* force_encryption */
> }
> 
>@@ -1721,6 +1733,7 @@ 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, "expire1n", test_session_expire1n);
> 	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);
>-- 
>2.17.2
>
>
>From 28bee8a6efa4f5d0767dcc4da452df09bbcaaf33 Mon Sep 17 00:00:00 2001
>From: Ralph Boehme <slow@samba.org>
>Date: Mon, 22 Oct 2018 18:21:58 +0200
>Subject: [PATCH 5/5] s3:smb2_sesssetup: check session_info security level
> before it gets talloc_move'd
>
>We talloc_move() session_info to session->global->auth_session_info
>which sets session_info to NULL.
>
>This means security_session_user_level(NULL, NULL) will always return
>SECURITY_ANONYMOUS so we never sign the session setup response.
>
>Bug: https://bugzilla.samba.org/show_bug.cgi?id=13661
>
>Signed-off-by: Ralph Boehme <slow@samba.org>
>Reviewed-by: Stefan Metzmacher <metze@samba.org>
>---
> source3/smbd/smb2_sesssetup.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
>diff --git a/source3/smbd/smb2_sesssetup.c b/source3/smbd/smb2_sesssetup.c
>index fe5835b83f3..5420d4f09bb 100644
>--- a/source3/smbd/smb2_sesssetup.c
>+++ b/source3/smbd/smb2_sesssetup.c
>@@ -525,6 +525,10 @@ static NTSTATUS smbd_smb2_reauth_generic_return(struct smbXsrv_session *session,
> 
> 	reload_services(smb2req->sconn, conn_snum_used, true);
> 
>+	if (security_session_user_level(session_info, NULL) >= SECURITY_USER) {
>+		smb2req->do_signing = true;
>+	}
>+
> 	session->status = NT_STATUS_OK;
> 	TALLOC_FREE(session->global->auth_session_info);
> 	session->global->auth_session_info = talloc_move(session->global,
>@@ -551,10 +555,6 @@ static NTSTATUS smbd_smb2_reauth_generic_return(struct smbXsrv_session *session,
> 
> 	conn_clear_vuid_caches(xconn->client->sconn, session->compat->vuid);
> 
>-	if (security_session_user_level(session_info, NULL) >= SECURITY_USER) {
>-		smb2req->do_signing = true;
>-	}
>-
> 	*out_session_id = session->global->session_wire_id;
> 
> 	return NT_STATUS_OK;
>-- 
>2.17.2
>

Actions: View

Attachments on bug 13661: 14603 | 14623 | 14629 | 14659