From a2a0dca168af5c6b9730833ddc10eaa186434cfb Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Tue, 30 Aug 2022 16:56:12 +0200 Subject: [PATCH 1/2] smbXsrv_client: correctly check in negotiate_request.length smbXsrv_client_connection_pass[ed]_* BUG: https://bugzilla.samba.org/show_bug.cgi?id=15159 Signed-off-by: Stefan Metzmacher Reviewed-by: Jeremy Allison (cherry picked from commit 21ef01e7b8368caa050ed82b9d787d1679220b2b) --- source3/smbd/smbXsrv_client.c | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/source3/smbd/smbXsrv_client.c b/source3/smbd/smbXsrv_client.c index 277ae1bab251..5c3f1597b16b 100644 --- a/source3/smbd/smbXsrv_client.c +++ b/source3/smbd/smbXsrv_client.c @@ -572,10 +572,6 @@ static bool smb2srv_client_mc_negprot_filter(struct messaging_rec *rec, void *pr return false; } - if (rec->buf.length < SMB2_HDR_BODY) { - return false; - } - return true; } @@ -665,6 +661,14 @@ static void smb2srv_client_mc_negprot_done(struct tevent_req *subreq) return; } + if (passed_info0->negotiate_request.length != 0) { + DBG_ERR("negotiate_request.length[%zu]\n", + passed_info0->negotiate_request.length); + NDR_PRINT_DEBUG(smbXsrv_connection_passB, &passed_blob); + tevent_req_nterror(req, NT_STATUS_INTERNAL_ERROR); + return; + } + tevent_req_nterror(req, NT_STATUS_MESSAGE_RETRIEVED); } @@ -883,12 +887,6 @@ static bool smbXsrv_client_connection_pass_filter(struct messaging_rec *rec, voi return false; } - if (rec->buf.length < SMB2_HDR_BODY) { - return false; - } - - /* TODO: verify client_guid...? */ - return true; } @@ -981,6 +979,15 @@ static void smbXsrv_client_connection_pass_loop(struct tevent_req *subreq) goto next; } + if (pass_info0->negotiate_request.length < SMB2_HDR_BODY) { + DBG_WARNING("negotiate_request.length[%zu]\n", + pass_info0->negotiate_request.length); + if (DEBUGLVL(DBGLVL_WARNING)) { + NDR_PRINT_DEBUG(smbXsrv_connection_passB, &pass_blob); + } + goto next; + } + status = smb2srv_client_connection_passed(client, pass_info0); if (!NT_STATUS_IS_OK(status)) { const char *r = "smb2srv_client_connection_passed() failed"; -- 2.34.1 From abb7566e5612139d7870a3a42040aaa16d421939 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Tue, 30 Aug 2022 20:45:50 +0200 Subject: [PATCH 2/2] smbXsrv_client: notify a different node to drop a connection by client guid. If a client disconnected all its interfaces and reconnects when the come back, it will likely start from any ip address returned dns, which means it can try to connect to a different ctdb node. The old node may not have noticed the disconnect and still holds the client_guid based smbd. Up unil now the new node returned NT_STATUS_NOT_SUPPORTED to the SMB2 Negotiate request, as messaging_send_iov[_from]() will return -1/ENOSYS if a file descriptor os passed to a process on a different node. Now we tell the other node to teardown all client connections belonging to the client-guid. Note that this is not authenticated, but if an attacker can capture the client-guid, he can also inject TCP resets anyway, to get the same effect. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15159 Signed-off-by: Stefan Metzmacher Reviewed-by: Jeremy Allison Autobuild-User(master): Jeremy Allison Autobuild-Date(master): Fri Sep 2 20:59:15 UTC 2022 on sn-devel-184 (cherry picked from commit 8591d9424371e173b079d5c8a267ea4c2cb266ad) --- librpc/idl/messaging.idl | 1 + source3/librpc/idl/smbXsrv.idl | 28 ++++ source3/smbd/smbXsrv_client.c | 239 +++++++++++++++++++++++++++++++-- 3 files changed, 255 insertions(+), 13 deletions(-) diff --git a/librpc/idl/messaging.idl b/librpc/idl/messaging.idl index d6929c799adf..5d217c03f5be 100644 --- a/librpc/idl/messaging.idl +++ b/librpc/idl/messaging.idl @@ -138,6 +138,7 @@ interface messaging MSG_SMBXSRV_SESSION_CLOSE = 0x0600, MSG_SMBXSRV_CONNECTION_PASS = 0x0601, MSG_SMBXSRV_CONNECTION_PASSED = 0x0602, + MSG_SMBXSRV_CONNECTION_DROP = 0x0603, /* source4 and NTVFS smb server messages */ MSG_BRL_RETRY = 0x0700, diff --git a/source3/librpc/idl/smbXsrv.idl b/source3/librpc/idl/smbXsrv.idl index fc502009b3bd..ec65a5c1a613 100644 --- a/source3/librpc/idl/smbXsrv.idl +++ b/source3/librpc/idl/smbXsrv.idl @@ -143,6 +143,7 @@ interface smbXsrv boolean8 server_multi_channel_enabled; hyper next_channel_id; [ignore] struct tevent_req *connection_pass_subreq; + [ignore] struct tevent_req *connection_drop_subreq; /* * A List of pending breaks. @@ -194,6 +195,33 @@ interface smbXsrv [in] smbXsrv_connection_passB blob ); + /* + * smbXsrv_connection_drop is used in the MSG_SMBXSRV_CONNECTION_DROP + * message as reaction the record is deleted. + */ + typedef struct { + GUID client_guid; + server_id src_server_id; + NTTIME xconn_connect_time; + server_id dst_server_id; + NTTIME client_connect_time; + } smbXsrv_connection_drop0; + + typedef union { + [case(0)] smbXsrv_connection_drop0 *info0; + [default] hyper *dummy; + } smbXsrv_connection_dropU; + + typedef [public] struct { + smbXsrv_version_values version; + [value(0)] uint32 reserved; + [switch_is(version)] smbXsrv_connection_dropU info; + } smbXsrv_connection_dropB; + + void smbXsrv_connection_drop_decode( + [in] smbXsrv_connection_dropB blob + ); + /* sessions */ typedef [public,bitmap8bit] bitmap { diff --git a/source3/smbd/smbXsrv_client.c b/source3/smbd/smbXsrv_client.c index 5c3f1597b16b..1ee4410c1cf5 100644 --- a/source3/smbd/smbXsrv_client.c +++ b/source3/smbd/smbXsrv_client.c @@ -339,6 +339,55 @@ static NTSTATUS smb2srv_client_connection_pass(struct smbd_smb2_request *smb2req return NT_STATUS_OK; } +static NTSTATUS smb2srv_client_connection_drop(struct smbd_smb2_request *smb2req, + struct smbXsrv_client_global0 *global) +{ + DATA_BLOB blob; + enum ndr_err_code ndr_err; + NTSTATUS status; + struct smbXsrv_connection_drop0 drop_info0; + struct smbXsrv_connection_dropB drop_blob; + struct iovec iov; + + drop_info0 = (struct smbXsrv_connection_drop0) { + .client_guid = global->client_guid, + .src_server_id = smb2req->xconn->client->global->server_id, + .xconn_connect_time = smb2req->xconn->client->global->initial_connect_time, + .dst_server_id = global->server_id, + .client_connect_time = global->initial_connect_time, + }; + + ZERO_STRUCT(drop_blob); + drop_blob.version = smbXsrv_version_global_current(); + drop_blob.info.info0 = &drop_info0; + + if (DEBUGLVL(DBGLVL_DEBUG)) { + NDR_PRINT_DEBUG(smbXsrv_connection_dropB, &drop_blob); + } + + ndr_err = ndr_push_struct_blob(&blob, talloc_tos(), &drop_blob, + (ndr_push_flags_fn_t)ndr_push_smbXsrv_connection_dropB); + if (!NDR_ERR_CODE_IS_SUCCESS(ndr_err)) { + status = ndr_map_error2ntstatus(ndr_err); + return status; + } + + iov.iov_base = blob.data; + iov.iov_len = blob.length; + + status = messaging_send_iov(smb2req->xconn->client->msg_ctx, + global->server_id, + MSG_SMBXSRV_CONNECTION_DROP, + &iov, 1, + NULL, 0); + data_blob_free(&blob); + if (!NT_STATUS_IS_OK(status)) { + return status; + } + + return NT_STATUS_OK; +} + static NTSTATUS smbXsrv_client_global_store(struct smbXsrv_client_global0 *global) { struct smbXsrv_client_globalB global_blob; @@ -532,15 +581,17 @@ static void smb2srv_client_mc_negprot_next(struct tevent_req *req) return; } - subreq = messaging_filtered_read_send(state, - state->ev, - client->msg_ctx, - smb2srv_client_mc_negprot_filter, - NULL); - if (tevent_req_nomem(subreq, req)) { - return; + if (procid_is_local(&global->server_id)) { + subreq = messaging_filtered_read_send(state, + state->ev, + client->msg_ctx, + smb2srv_client_mc_negprot_filter, + NULL); + if (tevent_req_nomem(subreq, req)) { + return; + } + tevent_req_set_callback(subreq, smb2srv_client_mc_negprot_done, req); } - tevent_req_set_callback(subreq, smb2srv_client_mc_negprot_done, req); subreq = dbwrap_watched_watch_send(state, state->ev, @@ -551,11 +602,20 @@ static void smb2srv_client_mc_negprot_next(struct tevent_req *req) } tevent_req_set_callback(subreq, smb2srv_client_mc_negprot_watched, req); - status = smb2srv_client_connection_pass(state->smb2req, - global); - TALLOC_FREE(global); - if (tevent_req_nterror(req, status)) { - return; + if (procid_is_local(&global->server_id)) { + status = smb2srv_client_connection_pass(state->smb2req, + global); + TALLOC_FREE(global); + if (tevent_req_nterror(req, status)) { + return; + } + } else { + status = smb2srv_client_connection_drop(state->smb2req, + global); + TALLOC_FREE(global); + if (tevent_req_nterror(req, status)) { + return; + } } TALLOC_FREE(state->db_rec); @@ -744,6 +804,8 @@ static int smbXsrv_client_destructor(struct smbXsrv_client *client) static bool smbXsrv_client_connection_pass_filter(struct messaging_rec *rec, void *private_data); static void smbXsrv_client_connection_pass_loop(struct tevent_req *subreq); +static bool smbXsrv_client_connection_drop_filter(struct messaging_rec *rec, void *private_data); +static void smbXsrv_client_connection_drop_loop(struct tevent_req *subreq); NTSTATUS smbXsrv_client_create(TALLOC_CTX *mem_ctx, struct tevent_context *ev_ctx, @@ -826,6 +888,18 @@ NTSTATUS smbXsrv_client_create(TALLOC_CTX *mem_ctx, tevent_req_set_callback(subreq, smbXsrv_client_connection_pass_loop, client); client->connection_pass_subreq = subreq; + subreq = messaging_filtered_read_send(client, + client->raw_ev_ctx, + client->msg_ctx, + smbXsrv_client_connection_drop_filter, + client); + if (subreq == NULL) { + TALLOC_FREE(client); + return NT_STATUS_NO_MEMORY; + } + tevent_req_set_callback(subreq, smbXsrv_client_connection_drop_loop, client); + client->connection_drop_subreq = subreq; + *_client = client; return NT_STATUS_OK; } @@ -1051,6 +1125,144 @@ next: client->connection_pass_subreq = subreq; } +static bool smbXsrv_client_connection_drop_filter(struct messaging_rec *rec, void *private_data) +{ + if (rec->msg_type != MSG_SMBXSRV_CONNECTION_DROP) { + return false; + } + + if (rec->num_fds != 0) { + return false; + } + + return true; +} + +static void smbXsrv_client_connection_drop_loop(struct tevent_req *subreq) +{ + struct smbXsrv_client *client = + tevent_req_callback_data(subreq, + struct smbXsrv_client); + int ret; + struct messaging_rec *rec = NULL; + struct smbXsrv_connection_dropB drop_blob; + enum ndr_err_code ndr_err; + struct smbXsrv_connection_drop0 *drop_info0 = NULL; + struct server_id_buf src_server_id_buf = {}; + NTSTATUS status; + + client->connection_drop_subreq = NULL; + + ret = messaging_filtered_read_recv(subreq, talloc_tos(), &rec); + TALLOC_FREE(subreq); + if (ret != 0) { + goto next; + } + + if (rec->num_fds != 0) { + DBG_ERR("MSG_SMBXSRV_CONNECTION_DROP: num_fds[%u]\n", + rec->num_fds); + goto next; + } + + ndr_err = ndr_pull_struct_blob(&rec->buf, rec, &drop_blob, + (ndr_pull_flags_fn_t)ndr_pull_smbXsrv_connection_dropB); + if (!NDR_ERR_CODE_IS_SUCCESS(ndr_err)) { + status = ndr_map_error2ntstatus(ndr_err); + DBG_WARNING("ndr_pull_struct_blob - %s\n", nt_errstr(status)); + goto next; + } + + if (DEBUGLVL(DBGLVL_DEBUG)) { + NDR_PRINT_DEBUG(smbXsrv_connection_dropB, &drop_blob); + } + + if (drop_blob.version != SMBXSRV_VERSION_0) { + DBG_ERR("ignore invalid version %u\n", drop_blob.version); + NDR_PRINT_DEBUG(smbXsrv_connection_dropB, &drop_blob); + goto next; + } + + drop_info0 = drop_blob.info.info0; + if (drop_info0 == NULL) { + DBG_ERR("ignore NULL info %u\n", drop_blob.version); + NDR_PRINT_DEBUG(smbXsrv_connection_dropB, &drop_blob); + goto next; + } + + if (!GUID_equal(&client->global->client_guid, &drop_info0->client_guid)) + { + struct GUID_txt_buf buf1, buf2; + + DBG_WARNING("client's client_guid [%s] != droped guid [%s]\n", + GUID_buf_string(&client->global->client_guid, + &buf1), + GUID_buf_string(&drop_info0->client_guid, + &buf2)); + if (DEBUGLVL(DBGLVL_WARNING)) { + NDR_PRINT_DEBUG(smbXsrv_connection_dropB, &drop_blob); + } + goto next; + } + + if (client->global->initial_connect_time != + drop_info0->client_connect_time) + { + DBG_WARNING("client's initial connect time [%s] (%llu) != " + "droped initial connect time [%s] (%llu)\n", + nt_time_string(talloc_tos(), + client->global->initial_connect_time), + (unsigned long long)client->global->initial_connect_time, + nt_time_string(talloc_tos(), + drop_info0->client_connect_time), + (unsigned long long)drop_info0->client_connect_time); + if (DEBUGLVL(DBGLVL_WARNING)) { + NDR_PRINT_DEBUG(smbXsrv_connection_dropB, &drop_blob); + } + goto next; + } + + /* + * Disconnect all client connections, which means we will tear down all + * sessions, tcons and non-durable opens. At the end we will remove our + * smbXsrv_client_global.tdb record, which will wake up the watcher on + * the other node in order to let it take over the client. + * + * The client will have to reopen all sessions, tcons and durable opens. + */ + smbd_server_disconnect_client(client, + server_id_str_buf(drop_info0->src_server_id, &src_server_id_buf)); + return; + +next: + if (rec != NULL) { + int sock_fd; + uint8_t fd_idx; + + for (fd_idx = 0; fd_idx < rec->num_fds; fd_idx++) { + sock_fd = rec->fds[fd_idx]; + close(sock_fd); + } + rec->num_fds = 0; + + TALLOC_FREE(rec); + } + + subreq = messaging_filtered_read_send(client, + client->raw_ev_ctx, + client->msg_ctx, + smbXsrv_client_connection_drop_filter, + client); + if (subreq == NULL) { + const char *r; + r = "messaging_read_send(MSG_SMBXSRV_CONNECTION_DROP failed"; + exit_server_cleanly(r); + return; + } + tevent_req_set_callback(subreq, smbXsrv_client_connection_drop_loop, client); + client->connection_drop_subreq = subreq; +} + NTSTATUS smbXsrv_client_remove(struct smbXsrv_client *client) { struct smbXsrv_client_table *table = client->table; @@ -1069,6 +1281,7 @@ NTSTATUS smbXsrv_client_remove(struct smbXsrv_client *client) } TALLOC_FREE(client->connection_pass_subreq); + TALLOC_FREE(client->connection_drop_subreq); client->global->db_rec = smbXsrv_client_global_fetch_locked( table->global.db_ctx, -- 2.34.1