From 25494628a2e977568de0f634602ebe893d0a5b88 Mon Sep 17 00:00:00 2001 From: Michael Adam Date: Thu, 19 Sep 2013 23:41:51 +0200 Subject: [PATCH 1/3] 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 Pair-Programmed-With: Stefan Metzmacher Signed-off-by: Michael Adam --- 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.4 From f6439613432f73319ca6b855256ee44921071991 Mon Sep 17 00:00:00 2001 From: Michael Adam Date: Fri, 20 Sep 2013 07:46:54 +0200 Subject: [PATCH 2/3] libcli/smb: add smb2cli_tcon_is_encryption_on() https://bugzilla.samba.org/show_bug.cgi?id=10208 Signed-off-by: Michael Adam Reviewed-by: Jeremy Allison --- 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 27ac2a8..9e5891a 100644 --- a/libcli/smb/smbXcli_base.c +++ b/libcli/smb/smbXcli_base.c @@ -5090,3 +5090,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.4 From f50b6da7d5862aa8d4a3ea04df9b9121b083e2a8 Mon Sep 17 00:00:00 2001 From: Michael Adam Date: Thu, 19 Sep 2013 22:00:19 +0200 Subject: [PATCH 3/3] 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 Reviewed-by: Jeremy Allison Autobuild-User(master): Jeremy Allison Autobuild-Date(master): Tue Oct 15 22:50:27 CEST 2013 on sn-devel-104 --- 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 d076bb6..2b90c3f 100644 --- a/source4/torture/smb2/session.c +++ b/source4/torture/smb2/session.c @@ -856,6 +856,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; @@ -981,6 +1083,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.4