From 5dbaf823cd5630c01e6ad21007696a7b8fc40315 Mon Sep 17 00:00:00 2001 From: Ralph Boehme 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 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 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 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 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 Reviewed-by: Stefan Metzmacher --- 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