The Samba-Bugzilla – Attachment 14629 Details for
Bug 13661
Session setup reauth fails to sign response
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
WIP patch for master with test
bug13661-wip.patch (text/plain), 11.93 KB, created by
Ralph Böhme
on 2018-11-07 22:26:33 UTC
(
hide
)
Description:
WIP patch for master with test
Filename:
MIME Type:
Creator:
Ralph Böhme
Created:
2018-11-07 22:26:33 UTC
Size:
11.93 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/6] 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/6] 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 46736f35ef421ff65fd591b880950445f8fe6fc3 Mon Sep 17 00:00:00 2001 >From: Ralph Boehme <slow@samba.org> >Date: Wed, 7 Nov 2018 22:20:59 +0100 >Subject: [PATCH 3/6] libcli/smb: add smb2cli_session_is_encryption_on() > >--- > 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 2afac33b805..c693f4c678c 100644 >--- a/libcli/smb/smbXcli_base.c >+++ b/libcli/smb/smbXcli_base.c >@@ -6135,6 +6135,11 @@ NTSTATUS smb2cli_session_set_channel_key(struct smbXcli_session *session, > return NT_STATUS_OK; > } > >+bool smb2cli_session_is_encryption_on(struct smbXcli_session *session) >+{ >+ return session->smb2->should_encrypt; >+} >+ > NTSTATUS smb2cli_session_encryption_on(struct smbXcli_session *session) > { > if (!session->smb2->should_sign) { >diff --git a/libcli/smb/smbXcli_base.h b/libcli/smb/smbXcli_base.h >index 5667a5d2607..131a5cd5109 100644 >--- a/libcli/smb/smbXcli_base.h >+++ b/libcli/smb/smbXcli_base.h >@@ -507,6 +507,7 @@ NTSTATUS smb2cli_session_set_channel_key(struct smbXcli_session *session, > const DATA_BLOB channel_key, > const struct iovec *recv_iov); > NTSTATUS smb2cli_session_encryption_on(struct smbXcli_session *session); >+bool smb2cli_session_is_encryption_on(struct smbXcli_session *session); > > struct smbXcli_tcon *smbXcli_tcon_create(TALLOC_CTX *mem_ctx); > struct smbXcli_tcon *smbXcli_tcon_copy(TALLOC_CTX *mem_ctx, >-- >2.17.2 > > >From 3d46d297dda3eae3dcb3bd1dd39013c2daabfebf Mon Sep 17 00:00:00 2001 >From: Ralph Boehme <slow@samba.org> >Date: Wed, 7 Nov 2018 16:42:36 +0100 >Subject: [PATCH 4/6] s4:libcli/smb2: check session-setup reauth response > signature > >--- > selftest/knownfail.d/samba3.smb2 | 9 ++++++ > source4/libcli/smb2/session.c | 47 ++++++++++++++++++++++++++++---- > 2 files changed, 51 insertions(+), 5 deletions(-) > create mode 100644 selftest/knownfail.d/samba3.smb2 > >diff --git a/selftest/knownfail.d/samba3.smb2 b/selftest/knownfail.d/samba3.smb2 >new file mode 100644 >index 00000000000..251f8f45cb9 >--- /dev/null >+++ b/selftest/knownfail.d/samba3.smb2 >@@ -0,0 +1,9 @@ >+^samba3.smb2.session plain.reauth1\(nt4_dc\) >+^samba3.smb2.session plain.reauth2\(nt4_dc\) >+^samba3.smb2.session plain.reauth3\(nt4_dc\) >+^samba3.smb2.session plain.reauth4\(nt4_dc\) >+^samba3.smb2.session enc.reauth1\(nt4_dc\) >+^samba3.smb2.session enc.reauth2\(nt4_dc\) >+^samba3.smb2.session enc.reauth3\(nt4_dc\) >+^samba3.smb2.session enc.reauth4\(nt4_dc\) >+^samba3.smb2.session krb5.expire1e\(ad_dc\) >diff --git a/source4/libcli/smb2/session.c b/source4/libcli/smb2/session.c >index e94512d3d33..04a13031e34 100644 >--- a/source4/libcli/smb2/session.c >+++ b/source4/libcli/smb2/session.c >@@ -380,11 +380,6 @@ static void smb2_session_setup_spnego_both_ready(struct tevent_req *req) > return; > } > >- if (state->reauth) { >- tevent_req_done(req); >- return; >- } >- > if (cli_credentials_is_anonymous(state->credentials)) { > /* > * Windows server does not set the >@@ -399,6 +394,48 @@ static void smb2_session_setup_spnego_both_ready(struct tevent_req *req) > return; > } > >+ if (state->reauth) { >+ DATA_BLOB key; >+ enum protocol_types protocol; >+ >+ if (smb2cli_session_is_encryption_on(session->smbXcli)) { >+ tevent_req_done(req); >+ return; >+ } >+ >+ protocol = smbXcli_conn_protocol(session->transport->conn); >+ if (protocol < PROTOCOL_SMB3_10) { >+ /* >+ * In theory we should always check this, Windows and >+ * Samba always sign session setup auth responses, but >+ * alas not all SMB servers might do this, so this check >+ * might break us with those. See also the comment in >+ * smb2cli_session_set_session_key(). >+ */ >+ tevent_req_done(req); >+ return; >+ } >+ >+ status = smbXcli_session_signing_key(session->smbXcli, &key); >+ if (NT_STATUS_EQUAL(status, NT_STATUS_NO_USER_SESSION_KEY)) { >+ tevent_req_done(req); >+ return; >+ } >+ if (tevent_req_nterror(req, status)) { >+ return; >+ } >+ >+ status = smb2_signing_check_pdu(key, >+ protocol, >+ state->recv_iov, >+ 3); >+ if (tevent_req_nterror(req, status)) { >+ return; >+ } >+ tevent_req_done(req); >+ return; >+ } >+ > status = gensec_session_key(session->gensec, state, > &session_key); > if (tevent_req_nterror(req, status)) { >-- >2.17.2 > > >From 93d78057dcf0c4f3d8789eb841166519852eb8b0 Mon Sep 17 00:00:00 2001 >From: Ralph Boehme <slow@samba.org> >Date: Wed, 7 Nov 2018 15:28:58 +0100 >Subject: [PATCH 5/6] 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 265ffa6ed118b80f72643c5840a2433c10e8bb4a Mon Sep 17 00:00:00 2001 >From: Ralph Boehme <slow@samba.org> >Date: Mon, 22 Oct 2018 18:21:58 +0200 >Subject: [PATCH 6/6] 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> >--- > selftest/knownfail.d/samba3.smb2 | 9 --------- > source3/smbd/smb2_sesssetup.c | 8 ++++---- > 2 files changed, 4 insertions(+), 13 deletions(-) > delete mode 100644 selftest/knownfail.d/samba3.smb2 > >diff --git a/selftest/knownfail.d/samba3.smb2 b/selftest/knownfail.d/samba3.smb2 >deleted file mode 100644 >index 251f8f45cb9..00000000000 >--- a/selftest/knownfail.d/samba3.smb2 >+++ /dev/null >@@ -1,9 +0,0 @@ >-^samba3.smb2.session plain.reauth1\(nt4_dc\) >-^samba3.smb2.session plain.reauth2\(nt4_dc\) >-^samba3.smb2.session plain.reauth3\(nt4_dc\) >-^samba3.smb2.session plain.reauth4\(nt4_dc\) >-^samba3.smb2.session enc.reauth1\(nt4_dc\) >-^samba3.smb2.session enc.reauth2\(nt4_dc\) >-^samba3.smb2.session enc.reauth3\(nt4_dc\) >-^samba3.smb2.session enc.reauth4\(nt4_dc\) >-^samba3.smb2.session krb5.expire1e\(ad_dc\) >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 >
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Raw
Actions:
View
Attachments on
bug 13661
:
14603
|
14623
|
14629
|
14659