Attachment 9365 Details for Bug 10208 – v4-1-test patch

[patch] v4-1-test patch

look (text/plain), 10.84 KB, created by Andreas Schneider on 2013-11-04 13:34:43 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Andreas Schneider

Created: 2013-11-04 13:34:43 UTC

Size: 10.84 KB

patch

obsolete

>From 7884d94ba71089a648ff2e7c50574e9c1096e550 Mon Sep 17 00:00:00 2001
>From: Michael Adam <obnox@samba.org>
>Date: Thu, 19 Sep 2013 23:41:51 +0200
>Subject: [PATCH 1/4] smbd:smb2: fix crash when smb2 session reauth fails
>
>https://bugzilla.samba.org/show_bug.cgi?id=10208
>
>Authentication error in smb2 session reauth invalidates
>the session. In this case the session must in contrast
>to successful session setup requests be torn down and live
>no longer than the request.
>
>The talloc move of the session from the global session
>table to the request ensures that the session setup
>reply can still be correctly signed, but subsequent
>requests on the connection don't find a session any more.
>
>Pair-Programmed-With: Jeremy Allison <jra@samba.org>
>Pair-Programmed-With: Stefan Metzmacher <metze@samba.org>
>
>Signed-off-by: Michael Adam <obnox@samba.org>
>(cherry picked from commit 25494628a2e977568de0f634602ebe893d0a5b88)
>---
> source3/smbd/smb2_sesssetup.c | 20 +++++++++++++++++---
> 1 file changed, 17 insertions(+), 3 deletions(-)
>
>diff --git a/source3/smbd/smb2_sesssetup.c b/source3/smbd/smb2_sesssetup.c
>index 265f802..dd243c9 100644
>--- a/source3/smbd/smb2_sesssetup.c
>+++ b/source3/smbd/smb2_sesssetup.c
>@@ -458,10 +458,19 @@ static int pp_self_ref_destructor(struct smbd_smb2_session_setup_state **pp_stat
> static int smbd_smb2_session_setup_state_destructor(struct smbd_smb2_session_setup_state *state)
> {
> 	/*
>-	 * if state->session is not NULL,
>-	 * we remove the session on failure
>+	 * If state->session is not NULL,
>+	 * we move the session from the session table to the request on failure
>+	 * so that the error response can be correctly signed, but the session
>+	 * is then really deleted when the request is done.
> 	 */
>-	TALLOC_FREE(state->session);
>+
>+	if (state->session == NULL) {
>+		return 0;
>+	}
>+
>+	state->session->status = NT_STATUS_USER_SESSION_DELETED;
>+	state->smb2req->session = talloc_move(state->smb2req, &state->session);
>+
> 	return 0;
> }
> 
>@@ -614,6 +623,7 @@ static void smbd_smb2_session_setup_gensec_done(struct tevent_req *subreq)
> 	if (NT_STATUS_EQUAL(status, NT_STATUS_MORE_PROCESSING_REQUIRED)) {
> 		state->out_session_id = state->session->global->session_wire_id;
> 		/* we want to keep the session */
>+		state->session = NULL;
> 		TALLOC_FREE(state->pp_self_ref);
> 		tevent_req_nterror(req, status);
> 		return;
>@@ -654,6 +664,7 @@ static void smbd_smb2_session_setup_gensec_done(struct tevent_req *subreq)
> 			return;
> 		}
> 		/* we want to keep the session */
>+		state->session = NULL;
> 		TALLOC_FREE(state->pp_self_ref);
> 		tevent_req_done(req);
> 		return;
>@@ -670,6 +681,7 @@ static void smbd_smb2_session_setup_gensec_done(struct tevent_req *subreq)
> 	}
> 
> 	/* we want to keep the session */
>+	state->session = NULL;
> 	TALLOC_FREE(state->pp_self_ref);
> 	tevent_req_done(req);
> 	return;
>@@ -701,6 +713,7 @@ static void smbd_smb2_session_setup_previous_done(struct tevent_req *subreq)
> 			return;
> 		}
> 		/* we want to keep the session */
>+		state->session = NULL;
> 		TALLOC_FREE(state->pp_self_ref);
> 		tevent_req_done(req);
> 		return;
>@@ -717,6 +730,7 @@ static void smbd_smb2_session_setup_previous_done(struct tevent_req *subreq)
> 	}
> 
> 	/* we want to keep the session */
>+	state->session = NULL;
> 	TALLOC_FREE(state->pp_self_ref);
> 	tevent_req_done(req);
> 	return;
>-- 
>1.8.3.1
>
>
>From 205ec3cacd684e93a0dc052264c3b5f53d6db330 Mon Sep 17 00:00:00 2001
>From: Michael Adam <obnox@samba.org>
>Date: Fri, 20 Sep 2013 07:46:54 +0200
>Subject: [PATCH 2/4] libcli/smb: add smb2cli_tcon_is_encryption_on()
>
>https://bugzilla.samba.org/show_bug.cgi?id=10208
>
>Signed-off-by: Michael Adam <obnox@samba.org>
>Reviewed-by: Jeremy Allison <jra@samba.org>
>(cherry picked from commit f6439613432f73319ca6b855256ee44921071991)
>---
> libcli/smb/smbXcli_base.c | 5 +++++
> libcli/smb/smbXcli_base.h | 1 +
> 2 files changed, 6 insertions(+)
>
>diff --git a/libcli/smb/smbXcli_base.c b/libcli/smb/smbXcli_base.c
>index 14d4cc3..6943181 100644
>--- a/libcli/smb/smbXcli_base.c
>+++ b/libcli/smb/smbXcli_base.c
>@@ -5088,3 +5088,8 @@ void smb2cli_tcon_set_values(struct smbXcli_tcon *tcon,
> 		tcon->smb2.should_encrypt = true;
> 	}
> }
>+
>+bool smb2cli_tcon_is_encryption_on(struct smbXcli_tcon *tcon)
>+{
>+	return tcon->smb2.should_encrypt;
>+}
>diff --git a/libcli/smb/smbXcli_base.h b/libcli/smb/smbXcli_base.h
>index 3d93427..852fc71 100644
>--- a/libcli/smb/smbXcli_base.h
>+++ b/libcli/smb/smbXcli_base.h
>@@ -315,6 +315,7 @@ void smb2cli_tcon_set_values(struct smbXcli_tcon *tcon,
> 			     uint32_t flags,
> 			     uint32_t capabilities,
> 			     uint32_t maximal_access);
>+bool smb2cli_tcon_is_encryption_on(struct smbXcli_tcon *tcon);
> 
> struct tevent_req *smb2cli_session_setup_send(TALLOC_CTX *mem_ctx,
> 				struct tevent_context *ev,
>-- 
>1.8.3.1
>
>
>From 3e176b79333d067faeb021a8362d16d14f6b4380 Mon Sep 17 00:00:00 2001
>From: Michael Adam <obnox@samba.org>
>Date: Thu, 19 Sep 2013 22:00:19 +0200
>Subject: [PATCH 3/4] s4:torture: add smb2.session.reauth6 : test failing
> reauth
>
>This attempts reauth with invalid creds, hence
>triggering the error path in the reauth code.
>This invalidates the session and subsequente requests
>on that connection fail.
>
>https://bugzilla.samba.org/show_bug.cgi?id=10208
>
>Signed-off-by: Michael Adam <obnox@samba.org>
>Reviewed-by: Jeremy Allison <jra@samba.org>
>
>Autobuild-User(master): Jeremy Allison <jra@samba.org>
>Autobuild-Date(master): Tue Oct 15 22:50:27 CEST 2013 on sn-devel-104
>
>(cherry picked from commit f50b6da7d5862aa8d4a3ea04df9b9121b083e2a8)
>---
> source4/torture/smb2/session.c | 103 +++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 103 insertions(+)
>
>diff --git a/source4/torture/smb2/session.c b/source4/torture/smb2/session.c
>index 6901f47..b28b496 100644
>--- a/source4/torture/smb2/session.c
>+++ b/source4/torture/smb2/session.c
>@@ -855,6 +855,108 @@ done:
> 	return ret;
> }
> 
>+/**
>+ * do reauth with wrong credentials,
>+ * hence triggering the error path in reauth.
>+ * The invalid reauth deletes the session.
>+ */
>+bool test_session_reauth6(struct torture_context *tctx, struct smb2_tree *tree)
>+{
>+	NTSTATUS status;
>+	TALLOC_CTX *mem_ctx = talloc_new(tctx);
>+	char fname[256];
>+	struct smb2_handle _h1;
>+	struct smb2_handle *h1 = NULL;
>+	struct smb2_create io1;
>+	bool ret = true;
>+	char *corrupted_password;
>+	struct cli_credentials *broken_creds;
>+	bool ok;
>+	bool encrypted;
>+	NTSTATUS expected;
>+	enum credentials_use_kerberos krb_state;
>+
>+	krb_state = cli_credentials_get_kerberos_state(cmdline_credentials);
>+	if (krb_state == CRED_MUST_USE_KERBEROS) {
>+		torture_skip(tctx,
>+			     "Can't test failing session setup with kerberos.");
>+	}
>+
>+	encrypted = smb2cli_tcon_is_encryption_on(tree->smbXcli);
>+
>+	/* Add some random component to the file name. */
>+	snprintf(fname, 256, "session_reauth1_%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, mem_ctx, &io1);
>+	CHECK_STATUS(status, NT_STATUS_OK);
>+	_h1 = io1.out.file.handle;
>+	h1 = &_h1;
>+	CHECK_CREATED(&io1, CREATED, FILE_ATTRIBUTE_ARCHIVE);
>+	CHECK_VAL(io1.out.oplock_level, smb2_util_oplock_level("b"));
>+
>+	/*
>+	 * reauthentication with invalid credentials:
>+	 */
>+
>+	broken_creds = cli_credentials_shallow_copy(mem_ctx,
>+						    cmdline_credentials);
>+	torture_assert(tctx, (broken_creds != NULL), "talloc error");
>+
>+	corrupted_password = talloc_asprintf(mem_ctx, "%s%s",
>+				cli_credentials_get_password(broken_creds),
>+				"corrupt");
>+	torture_assert(tctx, (corrupted_password != NULL), "talloc error");
>+
>+	ok = cli_credentials_set_password(broken_creds, corrupted_password,
>+					  CRED_SPECIFIED);
>+	CHECK_VAL(ok, true);
>+
>+	status = smb2_session_setup_spnego(tree->session,
>+					   broken_creds,
>+					   0 /* previous_session_id */);
>+	CHECK_STATUS(status, NT_STATUS_LOGON_FAILURE);
>+
>+	torture_comment(tctx, "did failed reauth\n");
>+	/*
>+	 * now verify that the invalid session reauth has closed our session
>+	 */
>+
>+	if (encrypted) {
>+		expected = NT_STATUS_CONNECTION_DISCONNECTED;
>+	} else {
>+		expected = NT_STATUS_USER_SESSION_DELETED;
>+	}
>+
>+	smb2_oplock_create_share(&io1, fname,
>+				 smb2_util_share_access(""),
>+				 smb2_util_oplock_level("b"));
>+
>+	status = smb2_create(tree, mem_ctx, &io1);
>+	CHECK_STATUS(status, expected);
>+
>+done:
>+	if (h1 != NULL) {
>+		smb2_util_close(tree, *h1);
>+	}
>+
>+	smb2_util_unlink(tree, fname);
>+
>+	talloc_free(tree);
>+
>+	talloc_free(mem_ctx);
>+
>+	return ret;
>+}
>+
>+
> static bool test_session_expire1(struct torture_context *tctx)
> {
> 	NTSTATUS status;
>@@ -980,6 +1082,7 @@ struct torture_suite *torture_smb2_session_init(void)
> 	torture_suite_add_1smb2_test(suite, "reauth3", test_session_reauth3);
> 	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);
> 
> 	suite->description = talloc_strdup(suite, "SMB2-SESSION tests");
>-- 
>1.8.3.1
>
>
>From 63d1ab9c76d70dd67ee607960fdf598e043f6e0f Mon Sep 17 00:00:00 2001
>From: Jeremy Allison <jra@samba.org>
>Date: Mon, 23 Sep 2013 14:10:27 -0700
>Subject: [PATCH 4/4] smbd: Invalidate the session correctly.
>
>When a session is invalidated then we must also ensure it isn't used in
>any pending requests being processed.
>
>Signed-off-by: Jeremy Allison <jra@samba.org>
>Reviewed-by: Andreas Schneider <asn@samba.org>
>(cherry picked from commit d4a5c832f1806a9c664d52a34ea1a24eb370fa89)
>---
> source3/smbd/smb2_sesssetup.c | 23 +++++++++++++++++++++++
> 1 file changed, 23 insertions(+)
>
>diff --git a/source3/smbd/smb2_sesssetup.c b/source3/smbd/smb2_sesssetup.c
>index dd243c9..cb8f847 100644
>--- a/source3/smbd/smb2_sesssetup.c
>+++ b/source3/smbd/smb2_sesssetup.c
>@@ -457,6 +457,8 @@ static int pp_self_ref_destructor(struct smbd_smb2_session_setup_state **pp_stat
> 
> static int smbd_smb2_session_setup_state_destructor(struct smbd_smb2_session_setup_state *state)
> {
>+	struct smbd_smb2_request *preq;
>+
> 	/*
> 	 * If state->session is not NULL,
> 	 * we move the session from the session table to the request on failure
>@@ -471,6 +473,27 @@ static int smbd_smb2_session_setup_state_destructor(struct smbd_smb2_session_set
> 	state->session->status = NT_STATUS_USER_SESSION_DELETED;
> 	state->smb2req->session = talloc_move(state->smb2req, &state->session);
> 
>+	/*
>+	 * We've made this session owned by the current request.
>+	 * Ensure that any outstanding requests don't also refer
>+	 * to it.
>+	 */
>+
>+	for (preq = state->smb2req->sconn->smb2.requests; preq != NULL; preq = preq->next) {
>+		if (preq == state->smb2req) {
>+			continue;
>+		}
>+		if (preq->session == state->smb2req->session) {
>+			preq->session = NULL;
>+			/*
>+			 * If we no longer have a session we can't
>+			 * sign or encrypt replies.
>+			 */
>+			preq->do_signing = false;
>+			preq->do_encryption = false;
>+		}
>+	}
>+
> 	return 0;
> }
> 
>-- 
>1.8.3.1
>

Flags:

asn: review? (jra)
asn: review? (ddiss)

Actions: View

Attachments on bug 10208: 9307 | 9353 | 9365 | 9366