From: Stefan Metzmacher Date: Fri, 12 Mar 2021 14:10:46 +0000 (+0100) Subject: smbXsrv_open: intruduce smbXsrv_open_replay_cache to support FILE_NOT_AVAILABLE X-Git-Tag: tevent-0.11.0~1369 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=997e9023c0ca94d57d75cd0069f5c6ab1f81be85;p=thirdparty%2Fsamba.git smbXsrv_open: intruduce smbXsrv_open_replay_cache to support FILE_NOT_AVAILABLE Before processing an open we need to reserve the replay cache entry in order to signal that we're still in progress. If a reserved record is already present we need to return FILE_NOT_AVAILABLE in order to let the client retry again. [MS-SMB2] contains this: <152> Section 3.2.5.1: For the following error codes, Windows-based clients will retry the operation up to three times and then retry the operation every 5 seconds until the count of milliseconds specified by Open.ResilientTimeout is exceeded: - STATUS_SERVER_UNAVAILABLE - STATUS_FILE_NOT_AVAILABLE - STATUS_SHARE_UNAVAILABLE This works fine for windows clients, but current windows servers seems to return ACCESS_DENIED instead of FILE_NOT_AVAILABLE. A Windows server doesn't do any replay detection on pending opens, which wait for a HANDLE lease to be broken (because of a SHARING_VIOLATION), at all. As this is not really documented for the server part of the current [MS-SMB2], I found the key hint in "SMB 2.2: Bigger. Faster. Scalier - (Parts 1 and 2)" on page 24. There's a picture showing that a replay gets FILE_NOT_AVAILABLE as long as the original request is still in progress. See: https://www.snia.org/educational-library/smb-22-bigger-faster-scalier-parts-1-and-2-2011 A Windows client is unhappy with the current windows server behavior if it such a situation happens. There's also a very strange interaction with oplock where the replay gets SHARING_VIOLATION after 35 seconds because it conflicts with the original open. I think it's good to follow the intial design from the 2011 presentation and make the clients happy by using FILE_NOT_AVAILABLE (and differ from Windows). I'll report that to dochelp@microsoft.com in order to get this hopefully fixed in their server too). BUG: https://bugzilla.samba.org/show_bug.cgi?id=14449 Signed-off-by: Stefan Metzmacher Reviewed-by: Jeremy Allison --- diff --git a/selftest/knownfail.d/smb2.replay b/selftest/knownfail.d/smb2.replay index 6f129da66cb..285081481c6 100644 --- a/selftest/knownfail.d/smb2.replay +++ b/selftest/knownfail.d/smb2.replay @@ -1,34 +1,17 @@ # These are temporary in order to demonstrate the current bugs -^samba3.smb2.replay.dhv2-pending1n-vs-violation-lease-close-sane.nt4_dc -^samba3.smb2.replay.dhv2-pending1n-vs-violation-lease-ack-sane.nt4_dc -^samba3.smb2.replay.dhv2-pending1n-vs-oplock-sane.nt4_dc -^samba3.smb2.replay.dhv2-pending1n-vs-lease-sane.nt4_dc -^samba3.smb2.replay.dhv2-pending1l-vs-oplock-sane.nt4_dc -^samba3.smb2.replay.dhv2-pending1l-vs-lease-sane.nt4_dc -^samba3.smb2.replay.dhv2-pending1o-vs-oplock-sane.nt4_dc -^samba3.smb2.replay.dhv2-pending1o-vs-lease-sane.nt4_dc ^samba3.smb2.replay.dhv2-pending2n-vs-oplock-sane.nt4_dc ^samba3.smb2.replay.dhv2-pending2n-vs-lease-sane.nt4_dc ^samba3.smb2.replay.dhv2-pending2l-vs-oplock-sane.nt4_dc ^samba3.smb2.replay.dhv2-pending2l-vs-lease-sane.nt4_dc ^samba3.smb2.replay.dhv2-pending2o-vs-oplock-sane.nt4_dc ^samba3.smb2.replay.dhv2-pending2o-vs-lease-sane.nt4_dc -^samba3.smb2.replay.dhv2-pending3n-vs-oplock-sane.nt4_dc -^samba3.smb2.replay.dhv2-pending3n-vs-lease-sane.nt4_dc -^samba3.smb2.replay.dhv2-pending3l-vs-oplock-sane.nt4_dc -^samba3.smb2.replay.dhv2-pending3l-vs-lease-sane.nt4_dc -^samba3.smb2.replay.dhv2-pending3o-vs-oplock-sane.nt4_dc -^samba3.smb2.replay.dhv2-pending3o-vs-lease-sane.nt4_dc -^samba3.smb2.replay.dhv2-pending1n-vs-oplock-sane.ad_dc -^samba3.smb2.replay.dhv2-pending1o-vs-oplock-sane.ad_dc ^samba3.smb2.replay.dhv2-pending2n-vs-oplock-sane.ad_dc ^samba3.smb2.replay.dhv2-pending2o-vs-oplock-sane.ad_dc -^samba3.smb2.replay.dhv2-pending3n-vs-oplock-sane.ad_dc -^samba3.smb2.replay.dhv2-pending3o-vs-oplock-sane.ad_dc # These tests demonstrate the broken Windows behavior # and check for ACCESS_DENIED instead of FILE_NOT_AVAILABLE # See https://bugzilla.samba.org/show_bug.cgi?id=14449 ^samba3.smb2.replay.dhv2-pending1n-vs-violation-lease-close-windows.nt4_dc +^samba3.smb2.replay.dhv2-pending1n-vs-violation-lease-ack-windows.nt4_dc ^samba3.smb2.replay.dhv2-pending1n-vs-oplock-windows.nt4_dc ^samba3.smb2.replay.dhv2-pending1n-vs-lease-windows.nt4_dc ^samba3.smb2.replay.dhv2-pending1l-vs-oplock-windows.nt4_dc diff --git a/source3/librpc/idl/smbXsrv.idl b/source3/librpc/idl/smbXsrv.idl index 89df7935c35..fc502009b3b 100644 --- a/source3/librpc/idl/smbXsrv.idl +++ b/source3/librpc/idl/smbXsrv.idl @@ -512,4 +512,11 @@ interface smbXsrv void smbXsrv_open_decode( [in] smbXsrv_openB blob ); + + const uint32 SMBXSRV_OPEN_REPLAY_CACHE_FIXED_SIZE = 28; + typedef [public] struct { + GUID holder_req_guid; + NTTIME idle_time; + uint32 local_id; + } smbXsrv_open_replay_cache; } diff --git a/source3/smbd/globals.h b/source3/smbd/globals.h index 0cab0d7396c..e023670f869 100644 --- a/source3/smbd/globals.h +++ b/source3/smbd/globals.h @@ -672,8 +672,12 @@ NTSTATUS smb2srv_open_lookup(struct smbXsrv_connection *conn, uint64_t volatile_id, NTTIME now, struct smbXsrv_open **_open); +NTSTATUS smbXsrv_open_purge_replay_cache(struct smbXsrv_client *client, + const struct GUID *create_guid); NTSTATUS smb2srv_open_lookup_replay_cache(struct smbXsrv_connection *conn, - const struct GUID *create_guid, + struct GUID caller_req_guid, + struct GUID create_guid, + const char *name, NTTIME now, struct smbXsrv_open **_open); NTSTATUS smb2srv_open_recreate(struct smbXsrv_connection *conn, diff --git a/source3/smbd/smb2_create.c b/source3/smbd/smb2_create.c index 8ff57c94aa0..dc6bd72591e 100644 --- a/source3/smbd/smb2_create.c +++ b/source3/smbd/smb2_create.c @@ -485,6 +485,7 @@ static NTSTATUS smbd_smb2_create_durable_lease_check(struct smb_request *smb1req struct smbd_smb2_create_state { struct tevent_context *ev; struct smbd_smb2_request *smb2req; + struct GUID req_guid; struct smb_request *smb1req; bool open_was_deferred; struct tevent_immediate *im; @@ -504,6 +505,8 @@ struct smbd_smb2_create_state { uint64_t allocation_size; struct GUID _create_guid; struct GUID *create_guid; + struct GUID _purge_create_guid; + struct GUID *purge_create_guid; bool update_open; bool durable_requested; uint32_t durable_timeout_msec; @@ -543,6 +546,15 @@ struct smbd_smb2_create_state { struct smb2_create_blobs *out_context_blobs; }; +static void smbd_smb2_create_purge_replay_cache(struct tevent_req *req, + const char *caller_func); + +static void smbd_smb2_create_cleanup(struct tevent_req *req, + enum tevent_req_state req_state) +{ + smbd_smb2_create_purge_replay_cache(req, __func__); +} + static NTSTATUS smbd_smb2_create_fetch_create_ctx( struct tevent_req *req, struct smb2_create_blobs *in_context_blobs) @@ -709,6 +721,10 @@ static struct tevent_req *smbd_smb2_create_send(TALLOC_CTX *mem_ctx, } state->smb1req = smb1req; + state->req_guid = smbd_request_guid(smb1req, 0); + + tevent_req_set_cleanup_fn(req, smbd_smb2_create_cleanup); + if (smb2req->subreq == NULL) { DBG_DEBUG("name [%s]\n", in_name); } else { @@ -721,6 +737,9 @@ static struct tevent_req *smbd_smb2_create_send(TALLOC_CTX *mem_ctx, state->request_time = old_state->request_time; state->open_rec = talloc_move(state, &old_state->open_rec); state->open_was_deferred = old_state->open_was_deferred; + state->_purge_create_guid = old_state->_purge_create_guid; + state->purge_create_guid = old_state->purge_create_guid; + old_state->purge_create_guid = NULL; } TALLOC_FREE(smb2req->subreq); @@ -1026,6 +1045,31 @@ static struct tevent_req *smbd_smb2_create_send(TALLOC_CTX *mem_ctx, return req; } +static void smbd_smb2_create_purge_replay_cache(struct tevent_req *req, + const char *caller_func) +{ + struct smbd_smb2_create_state *state = tevent_req_data( + req, struct smbd_smb2_create_state); + NTSTATUS status; + + if (state->purge_create_guid == NULL) { + return; + } + + status = smbXsrv_open_purge_replay_cache(state->smb2req->xconn->client, + state->purge_create_guid); + if (!NT_STATUS_IS_OK(status)) { + struct GUID_txt_buf buf; + + D_ERR("%s: smbXsrv_open_purge_replay_cache(%s) %s\n", + caller_func, + GUID_buf_string(state->purge_create_guid, &buf), + nt_errstr(status)); + } + + state->purge_create_guid = NULL; +} + static void smbd_smb2_create_before_exec(struct tevent_req *req) { struct smbd_smb2_create_state *state = tevent_req_data( @@ -1117,6 +1161,7 @@ static void smbd_smb2_create_before_exec(struct tevent_req *req) if (state->dh2q != NULL) { const uint8_t *p = state->dh2q->data.data; + NTTIME now = timeval_to_nttime(&smb2req->request_time); uint32_t durable_v2_timeout = 0; DATA_BLOB create_guid_blob; const uint8_t *hdr; @@ -1179,14 +1224,33 @@ static void smbd_smb2_create_before_exec(struct tevent_req *req) flags & SMB2_HDR_FLAG_REPLAY_OPERATION; status = smb2srv_open_lookup_replay_cache(smb2req->xconn, - state->create_guid, - 0 /* now */, + state->req_guid, + *state->create_guid, + state->fname, + now, &state->op); - if (NT_STATUS_EQUAL(status, NT_STATUS_NOT_FOUND)) { + if (NT_STATUS_EQUAL(status, NT_STATUS_FWP_RESERVED)) { + /* + * We've reserved the replay_cache record + * for ourself, indicating we're still + * in progress. + * + * It means the smbd_smb2_create_cleanup() + * may need to call smbXsrv_open_purge_replay_cache() + * in order to cleanup. + */ + SMB_ASSERT(state->op == NULL); + state->_purge_create_guid = state->_create_guid; + state->purge_create_guid = &state->_purge_create_guid; + status = NT_STATUS_OK; state->replay_operation = false; - } else if (tevent_req_nterror(req, status)) { + } else if (NT_STATUS_EQUAL(status, NT_STATUS_FILE_NOT_AVAILABLE)) { + tevent_req_nterror(req, status); + return; + } else if (!NT_STATUS_IS_OK(status)) { DBG_WARNING("smb2srv_open_lookup_replay_cache " "failed: %s\n", nt_errstr(status)); + tevent_req_nterror(req, status); return; } else if (!state->replay_operation) { /* @@ -1388,6 +1452,12 @@ static void smbd_smb2_create_after_exec(struct tevent_req *req) tevent_req_post(req, state->ev); return; } + + /* + * We should not purge the replay cache anymore + * as it's attached to the smbXsrv_open record now. + */ + state->purge_create_guid = NULL; } if (state->dhnq != NULL && state->op->global->durable) { diff --git a/source3/smbd/smbXsrv_open.c b/source3/smbd/smbXsrv_open.c index 5180315449d..b007d61410e 100644 --- a/source3/smbd/smbXsrv_open.c +++ b/source3/smbd/smbXsrv_open.c @@ -928,7 +928,16 @@ static NTSTATUS smbXsrv_open_set_replay_cache(struct smbXsrv_open *op) struct GUID_txt_buf buf; char *guid_string; struct db_context *db = op->table->local.replay_cache_db_ctx; + struct smbXsrv_open_replay_cache rc = { + .idle_time = op->idle_time, + .local_id = op->local_id, + }; + uint8_t data[SMBXSRV_OPEN_REPLAY_CACHE_FIXED_SIZE]; + DATA_BLOB blob = data_blob_const(data, ARRAY_SIZE(data)); + enum ndr_err_code ndr_err; NTSTATUS status; + TDB_DATA key; + TDB_DATA val; if (!(op->flags & SMBXSRV_OPEN_NEED_REPLAY_CACHE)) { return NT_STATUS_OK; @@ -939,16 +948,18 @@ static NTSTATUS smbXsrv_open_set_replay_cache(struct smbXsrv_open *op) } create_guid = &op->global->create_guid; - if (GUID_all_zero(create_guid)) { - return NT_STATUS_OK; - } - guid_string = GUID_buf_string(create_guid, &buf); - if (guid_string == NULL) { - return NT_STATUS_INVALID_PARAMETER; + key = string_term_tdb_data(guid_string); + + ndr_err = ndr_push_struct_into_fixed_blob(&blob, &rc, + (ndr_push_flags_fn_t)ndr_push_smbXsrv_open_replay_cache); + if (!NDR_ERR_CODE_IS_SUCCESS(ndr_err)) { + status = ndr_map_error2ntstatus(ndr_err); + return status; } + val = make_tdb_data(blob.data, blob.length); - status = dbwrap_store_uint32_bystring(db, guid_string, op->local_id); + status = dbwrap_store(db, key, val, TDB_REPLACE); if (NT_STATUS_IS_OK(status)) { op->flags |= SMBXSRV_OPEN_HAVE_REPLAY_CACHE; @@ -958,6 +969,27 @@ static NTSTATUS smbXsrv_open_set_replay_cache(struct smbXsrv_open *op) return status; } +NTSTATUS smbXsrv_open_purge_replay_cache(struct smbXsrv_client *client, + const struct GUID *create_guid) +{ + struct GUID_txt_buf buf; + char *guid_string; + struct db_context *db; + + if (client->open_table == NULL) { + return NT_STATUS_OK; + } + + db = client->open_table->local.replay_cache_db_ctx; + + guid_string = GUID_buf_string(create_guid, &buf); + if (guid_string == NULL) { + return NT_STATUS_INVALID_PARAMETER; + } + + return dbwrap_purge_bystring(db, guid_string); +} + static NTSTATUS smbXsrv_open_clear_replay_cache(struct smbXsrv_open *op) { struct GUID *create_guid; @@ -1269,44 +1301,189 @@ NTSTATUS smb2srv_open_lookup(struct smbXsrv_connection *conn, return status; } +/* + * This checks or marks the replay cache, we have the following + * cases: + * + * 1. There is no record in the cache + * => we add the passes caller_req_guid as holder_req_guid + * together with local_id as 0. + * => We return STATUS_FWP_RESERVED in order to indicate + * that the caller holds the current reservation + * + * 2. There is a record in the cache and holder_req_guid + * is already the same as caller_req_guid and local_id is 0 + * => We return STATUS_FWP_RESERVED in order to indicate + * that the caller holds the current reservation + * + * 3. There is a record in the cache with a holder_req_guid + * other than caller_req_guid (and local_id is 0): + * => We return NT_STATUS_FILE_NOT_AVAILABLE to indicate + * the original request is still pending + * + * 4. There is a record in the cache with a zero holder_req_guid + * and a valid local_id: + * => We lookup the existing open by local_id + * => We return NT_STATUS_OK together with the smbXsrv_open + * + * + * With NT_STATUS_OK the caller can continue the replay processing. + * + * With STATUS_FWP_RESERVED the caller should continue the normal + * open processing: + * - On success: + * - smbXsrv_open_update()/smbXsrv_open_set_replay_cache() + * will convert the record to a zero holder_req_guid + * with a valid local_id. + * - On failure: + * - smbXsrv_open_purge_replay_cache() should cleanup + * the reservation. + * + * All other values should be returned to the client, + * while NT_STATUS_FILE_NOT_AVAILABLE will trigger the + * retry loop on the client. + */ NTSTATUS smb2srv_open_lookup_replay_cache(struct smbXsrv_connection *conn, - const struct GUID *create_guid, - NTTIME now, /* TODO: needed ? */ + struct GUID caller_req_guid, + struct GUID create_guid, + const char *name, + NTTIME now, struct smbXsrv_open **_open) { + TALLOC_CTX *frame = talloc_stackframe(); NTSTATUS status; - char *guid_string; - struct GUID_txt_buf buf; - uint32_t local_id = 0; struct smbXsrv_open_table *table = conn->client->open_table; struct db_context *db = table->local.replay_cache_db_ctx; + struct GUID_txt_buf _create_guid_buf; + struct GUID_txt_buf tmp_guid_buf; + const char *create_guid_str = NULL; + TDB_DATA create_guid_key; + struct db_record *db_rec = NULL; + struct smbXsrv_open *op = NULL; + struct smbXsrv_open_replay_cache rc = { + .holder_req_guid = caller_req_guid, + .idle_time = now, + .local_id = 0, + }; + enum ndr_err_code ndr_err; + DATA_BLOB blob = data_blob_null; + TDB_DATA val; - if (GUID_all_zero(create_guid)) { - return NT_STATUS_NOT_FOUND; + *_open = NULL; + + create_guid_str = GUID_buf_string(&create_guid, &_create_guid_buf); + create_guid_key = string_term_tdb_data(create_guid_str); + + db_rec = dbwrap_fetch_locked(db, frame, create_guid_key); + if (db_rec == NULL) { + TALLOC_FREE(frame); + return NT_STATUS_INTERNAL_DB_ERROR; } - guid_string = GUID_buf_string(create_guid, &buf); - if (guid_string == NULL) { - return NT_STATUS_INVALID_PARAMETER; + val = dbwrap_record_get_value(db_rec); + if (val.dsize == 0) { + uint8_t data[SMBXSRV_OPEN_REPLAY_CACHE_FIXED_SIZE]; + + blob = data_blob_const(data, ARRAY_SIZE(data)); + ndr_err = ndr_push_struct_into_fixed_blob(&blob, &rc, + (ndr_push_flags_fn_t)ndr_push_smbXsrv_open_replay_cache); + if (!NDR_ERR_CODE_IS_SUCCESS(ndr_err)) { + status = ndr_map_error2ntstatus(ndr_err); + TALLOC_FREE(frame); + return status; + } + + val = make_tdb_data(blob.data, blob.length); + status = dbwrap_record_store(db_rec, val, TDB_REPLACE); + if (!NT_STATUS_IS_OK(status)) { + TALLOC_FREE(frame); + return status; + } + + /* + * We're the new holder + */ + *_open = NULL; + TALLOC_FREE(frame); + return NT_STATUS_FWP_RESERVED; } - status = dbwrap_fetch_uint32_bystring(db, guid_string, &local_id); - if (NT_STATUS_EQUAL(status, NT_STATUS_NOT_FOUND)) { - return status; + if (val.dsize != SMBXSRV_OPEN_REPLAY_CACHE_FIXED_SIZE) { + TALLOC_FREE(frame); + return NT_STATUS_INTERNAL_DB_CORRUPTION; } - if (!NT_STATUS_IS_OK(status)) { - DBG_ERR("failed to fetch local_id from replay cache: %s\n", - nt_errstr(status)); + + blob = data_blob_const(val.dptr, val.dsize); + ndr_err = ndr_pull_struct_blob_all_noalloc(&blob, &rc, + (ndr_pull_flags_fn_t)ndr_pull_smbXsrv_open_replay_cache); + if (!NDR_ERR_CODE_IS_SUCCESS(ndr_err)) { + status = ndr_map_error2ntstatus(ndr_err); + TALLOC_FREE(frame); return status; } + if (rc.local_id != 0) { + if (GUID_equal(&rc.holder_req_guid, &caller_req_guid)) { + /* + * This should not happen + */ + status = NT_STATUS_INTERNAL_ERROR; + DBG_ERR("caller %s already holds local_id %u for create %s [%s] - %s\n", + GUID_buf_string(&caller_req_guid, &tmp_guid_buf), + (unsigned)rc.local_id, + create_guid_str, + name, + nt_errstr(status)); + + TALLOC_FREE(frame); + return status; + } - status = smbXsrv_open_local_lookup(table, local_id, 0, /* global_id */ - now, _open); - if (!NT_STATUS_IS_OK(status)) { - DBG_ERR("smbXsrv_open_local_lookup failed for local_id %u\n", - (unsigned)local_id); + status = smbXsrv_open_local_lookup(table, + rc.local_id, + 0, /* global_id */ + now, + &op); + if (!NT_STATUS_IS_OK(status)) { + DBG_ERR("holder %s stale for local_id %u for create %s [%s] - %s\n", + GUID_buf_string(&rc.holder_req_guid, &tmp_guid_buf), + (unsigned)rc.local_id, + create_guid_str, + name, + nt_errstr(status)); + + TALLOC_FREE(frame); + return status; + } + + /* + * We found an open the caller can reuse. + */ + SMB_ASSERT(op != NULL); + *_open = op; + TALLOC_FREE(frame); + return NT_STATUS_OK; + } + + if (GUID_equal(&rc.holder_req_guid, &caller_req_guid)) { + /* + * We're still the holder + */ + *_open = NULL; + TALLOC_FREE(frame); + return NT_STATUS_FWP_RESERVED; } + /* + * The original request (or a former replay) is still + * pending, ask the client to retry by sending FILE_NOT_AVAILABLE. + */ + status = NT_STATUS_FILE_NOT_AVAILABLE; + DBG_DEBUG("holder %s still pending for create %s [%s] - %s\n", + GUID_buf_string(&rc.holder_req_guid, &tmp_guid_buf), + create_guid_str, + name, + nt_errstr(status)); + TALLOC_FREE(frame); return status; }