From 7b80c0cde69a641845091525803d833bfcd323a8 Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Mon, 8 Aug 2022 14:05:46 +0200 Subject: [PATCH 01/55] smbd: Use dirfsp where we have it One reference to conn->cwd_fsp less, makes "mkdir" look less ugly in strace. Signed-off-by: Volker Lendecke Reviewed-by: Jeremy Allison BUG: https://bugzilla.samba.org/show_bug.cgi?id=15146 (cherry picked from commit b1b513eebb0999cdfabab597927305be7d978605) --- source3/smbd/open.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source3/smbd/open.c b/source3/smbd/open.c index d39d2d619cd0..f1c2f7382bcf 100644 --- a/source3/smbd/open.c +++ b/source3/smbd/open.c @@ -4329,7 +4329,7 @@ static NTSTATUS mkdir_internal(connection_struct *conn, */ fsp->fsp_flags.is_pathref = true; - status = fd_openat(conn->cwd_fsp, smb_dname, fsp, &how); + status = fd_openat(parent_dir_fname->fsp, smb_fname_atname, fsp, &how); if (!NT_STATUS_IS_OK(status)) { return status; } -- 2.34.1 From 252b16f5a72aff552cd815fc86c24a10f1cfec3c Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Tue, 9 Aug 2022 12:42:05 +0200 Subject: [PATCH 02/55] smbstatus: Fix the 32-bit build on FreeBSD Signed-off-by: Volker Lendecke Reviewed-by: Jeremy Allison Autobuild-User(master): Jeremy Allison Autobuild-Date(master): Tue Aug 9 20:04:26 UTC 2022 on sn-devel-184 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15146 (cherry picked from commit 4d015b4b6db20235d6d821204d03b0e1fce1c681) --- source3/utils/status_json.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/source3/utils/status_json.c b/source3/utils/status_json.c index 69b31e27f541..e2798e3e392c 100644 --- a/source3/utils/status_json.c +++ b/source3/utils/status_json.c @@ -75,7 +75,8 @@ static int add_server_id_to_json(struct json_object *parent_json, goto failure; } - pid_str = talloc_asprintf(tmp_ctx, "%lu", server_id.pid); + pid_str = talloc_asprintf( + tmp_ctx, "%lu", (unsigned long)server_id.pid); result = json_add_string(&sub_json, "pid", pid_str); if (result < 0) { goto failure; @@ -90,7 +91,8 @@ static int add_server_id_to_json(struct json_object *parent_json, if (result < 0) { goto failure; } - unique_id_str = talloc_asprintf(tmp_ctx, "%lu", server_id.unique_id); + unique_id_str = talloc_asprintf( + tmp_ctx, "%"PRIu64, server_id.unique_id); result = json_add_string(&sub_json, "unique_id", unique_id_str); if (result < 0) { goto failure; @@ -834,7 +836,7 @@ static int add_open_to_json(struct json_object *parent_json, if (result < 0) { goto failure; } - share_file_id = talloc_asprintf(tmp_ctx, "%lu", e->share_file_id); + share_file_id = talloc_asprintf(tmp_ctx, "%"PRIu64, e->share_file_id); result = json_add_string(&sub_json, "share_file_id", share_file_id); if (result < 0) { goto failure; @@ -871,7 +873,7 @@ static int add_open_to_json(struct json_object *parent_json, } pid = server_id_str_buf(e->pid, &tmp); - key = talloc_asprintf(tmp_ctx, "%s/%lu", pid, e->share_file_id); + key = talloc_asprintf(tmp_ctx, "%s/%"PRIu64, pid, e->share_file_id); result = json_add_object(&opens_json, key, &sub_json); if (result < 0) { goto failure; -- 2.34.1 From f2856297d1417b29a2c7e78be60b47e438272de2 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Wed, 10 Aug 2022 16:45:26 +0200 Subject: [PATCH 03/55] s3:include: remove unused update_stat_ex_file_id() prototype It was removed by commit 643da37fd139413651a6198fb0f6e550f7de6584 Signed-off-by: Stefan Metzmacher Reviewed-by: Jeremy Allison BUG: https://bugzilla.samba.org/show_bug.cgi?id=15146 (cherry picked from commit 8c7e8c5f80f1488456f9dd6225020d29f74458d2) --- source3/include/proto.h | 1 - 1 file changed, 1 deletion(-) diff --git a/source3/include/proto.h b/source3/include/proto.h index aebbd7f9c47c..6a6edc36dfb0 100644 --- a/source3/include/proto.h +++ b/source3/include/proto.h @@ -171,7 +171,6 @@ int sys_fcntl_long(int fd, int cmd, long arg); int sys_fcntl_int(int fd, int cmd, int arg); void update_stat_ex_mtime(struct stat_ex *dst, struct timespec write_ts); void update_stat_ex_create_time(struct stat_ex *dst, struct timespec create_time); -void update_stat_ex_file_id(struct stat_ex *dst, uint64_t file_id); void update_stat_ex_from_saved_stat(struct stat_ex *dst, const struct stat_ex *src); int sys_stat(const char *fname, SMB_STRUCT_STAT *sbuf, -- 2.34.1 From d14c2f92d1fb3680b7f9e1bb8aa831796e4749cc Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Tue, 9 Aug 2022 14:07:12 +0000 Subject: [PATCH 04/55] smbd: avoid calling SMB_VFS_FGET_NT_ACL() if do_not_check_mask already covers all This is inspired by 0d4cb5a641e1fea2d369bdc66470a580321366c2, which avoids SMB_VFS_FGET_NT_ACL() for the root user again. Opens with just FILE_READ_ATTRIBUTES are very common, so it's worth optimizing for it. Signed-off-by: Stefan Metzmacher Reviewed-by: Jeremy Allison BUG: https://bugzilla.samba.org/show_bug.cgi?id=15125 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15146 (cherry picked from commit a0a97d27f7a60dbd86317b51bec0ece2476e8c8d) --- source3/smbd/open.c | 69 +++++++++++++++++++++++++++------------------ 1 file changed, 41 insertions(+), 28 deletions(-) diff --git a/source3/smbd/open.c b/source3/smbd/open.c index f1c2f7382bcf..803c0fbd347a 100644 --- a/source3/smbd/open.c +++ b/source3/smbd/open.c @@ -99,9 +99,11 @@ static NTSTATUS smbd_check_access_rights_fname( struct connection_struct *conn, const struct smb_filename *smb_fname, bool use_privs, - uint32_t access_mask) + uint32_t access_mask, + uint32_t do_not_check_mask) { uint32_t rejected_share_access; + uint32_t effective_access; rejected_share_access = access_mask & ~(conn->share_access); @@ -114,6 +116,14 @@ static NTSTATUS smbd_check_access_rights_fname( return NT_STATUS_ACCESS_DENIED; } + effective_access = access_mask & ~do_not_check_mask; + if (effective_access == 0) { + DBG_DEBUG("do_not_check_mask override on %s. Granting 0x%x for free.\n", + smb_fname_str_dbg(smb_fname), + (unsigned int)access_mask); + return NT_STATUS_OK; + } + if (!use_privs && get_current_uid(conn) == (uid_t)0) { /* I'm sorry sir, I didn't know you were root... */ DBG_DEBUG("root override on %s. Granting 0x%x\n", @@ -151,39 +161,16 @@ static NTSTATUS smbd_check_access_rights_sd( const struct smb_filename *smb_fname, struct security_descriptor *sd, bool use_privs, - uint32_t access_mask) + uint32_t access_mask, + uint32_t do_not_check_mask) { uint32_t rejected_mask = access_mask; - uint32_t do_not_check_mask = 0; NTSTATUS status; if (sd == NULL) { goto access_denied; } - /* - * If we can access the path to this file, by - * default we have FILE_READ_ATTRIBUTES from the - * containing directory. See the section: - * "Algorithm to Check Access to an Existing File" - * in MS-FSA.pdf. - * - * se_file_access_check() also takes care of - * owner WRITE_DAC and READ_CONTROL. - */ - do_not_check_mask = FILE_READ_ATTRIBUTES; - - /* - * Samba 3.6 and earlier granted execute access even - * if the ACL did not contain execute rights. - * Samba 4.0 is more correct and checks it. - * The compatibilty mode allows one to skip this check - * to smoothen upgrades. - */ - if (lp_acl_allow_execute_always(SNUM(conn))) { - do_not_check_mask |= FILE_EXECUTE; - } - status = se_file_access_check(sd, get_current_nttok(conn), use_privs, @@ -263,6 +250,7 @@ NTSTATUS smbd_check_access_rights_fsp(struct files_struct *dirfsp, uint32_t access_mask) { struct security_descriptor *sd = NULL; + uint32_t do_not_check_mask = 0; NTSTATUS status; /* Cope with fake/printer fsp's. */ @@ -288,10 +276,34 @@ NTSTATUS smbd_check_access_rights_fsp(struct files_struct *dirfsp, return NT_STATUS_OK; } + /* + * If we can access the path to this file, by + * default we have FILE_READ_ATTRIBUTES from the + * containing directory. See the section: + * "Algorithm to Check Access to an Existing File" + * in MS-FSA.pdf. + * + * se_file_access_check() also takes care of + * owner WRITE_DAC and READ_CONTROL. + */ + do_not_check_mask = FILE_READ_ATTRIBUTES; + + /* + * Samba 3.6 and earlier granted execute access even + * if the ACL did not contain execute rights. + * Samba 4.0 is more correct and checks it. + * The compatibilty mode allows one to skip this check + * to smoothen upgrades. + */ + if (lp_acl_allow_execute_always(SNUM(fsp->conn))) { + do_not_check_mask |= FILE_EXECUTE; + } + status = smbd_check_access_rights_fname(fsp->conn, fsp->fsp_name, use_privs, - access_mask); + access_mask, + do_not_check_mask); if (!NT_STATUS_EQUAL(status, NT_STATUS_MORE_PROCESSING_REQUIRED)) { return status; } @@ -314,7 +326,8 @@ NTSTATUS smbd_check_access_rights_fsp(struct files_struct *dirfsp, fsp->fsp_name, sd, use_privs, - access_mask); + access_mask, + do_not_check_mask); } /* -- 2.34.1 From a3d4f103100dcbcf13b26eeeaa51ea6ae08d09e2 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Wed, 10 Aug 2022 13:16:14 +0000 Subject: [PATCH 05/55] s3:g_lock: use TDB_VOLATILE to avoid fcntl locks This improves 'time smbtorture3 //foo/bar -U% local-g-lock-ping-pong -o 50000000' from ~1.400.000 to ~3.400.000 operations per second any a testsystem. As we also use TDB_VOLATILE for locking.tdb, this is a much more realistic test now. Signed-off-by: Stefan Metzmacher Reviewed-by: Jeremy Allison BUG: https://bugzilla.samba.org/show_bug.cgi?id=15146 (cherry picked from commit cd01f5134696f7789fbc2933629ac2606feb0b5e) --- source3/lib/g_lock.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source3/lib/g_lock.c b/source3/lib/g_lock.c index 52a1cf9fa467..d683c4ddee6f 100644 --- a/source3/lib/g_lock.c +++ b/source3/lib/g_lock.c @@ -236,7 +236,7 @@ struct g_lock_ctx *g_lock_ctx_init(TALLOC_CTX *mem_ctx, mem_ctx, db_path, 0, - TDB_CLEAR_IF_FIRST|TDB_INCOMPATIBLE_HASH, + TDB_CLEAR_IF_FIRST|TDB_INCOMPATIBLE_HASH|TDB_VOLATILE, O_RDWR|O_CREAT, 0600, DBWRAP_LOCK_ORDER_3, -- 2.34.1 From 544ef216ce0fc1230dbe8a90c87a637c6540d1e7 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Wed, 10 Aug 2022 13:15:45 +0000 Subject: [PATCH 06/55] s4:param: add --option="libsmb:client_guid=6112f7d3-9528-4a2a-8861-0ca129aae6c4" support... We already handle this in the source3/libsmb code, but it's good to have this also for torture tests. Signed-off-by: Stefan Metzmacher Reviewed-by: Jeremy Allison BUG: https://bugzilla.samba.org/show_bug.cgi?id=15146 (cherry picked from commit 72caffbe1115c57ad38270eaeb951f6b97bf62b3) --- source4/param/loadparm.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/source4/param/loadparm.c b/source4/param/loadparm.c index 81892d540ef1..7d3891dc0b9d 100644 --- a/source4/param/loadparm.c +++ b/source4/param/loadparm.c @@ -35,6 +35,15 @@ void lpcfg_smbcli_options(struct loadparm_context *lp_ctx, struct smbcli_options *options) { + struct GUID client_guid; + const char *str = NULL; + + str = lpcfg_parm_string(lp_ctx, NULL, "libsmb", "client_guid"); + if (str != NULL) { + GUID_from_string(str, &client_guid); + } else { + client_guid = GUID_random(); + } *options = (struct smbcli_options) { .max_xmit = lpcfg_max_xmit(lp_ctx), .max_mux = lpcfg_max_mux(lp_ctx), @@ -48,7 +57,7 @@ void lpcfg_smbcli_options(struct loadparm_context *lp_ctx, .use_oplocks = true, .use_level2_oplocks = true, .smb2_capabilities = SMB2_CAP_ALL, - .client_guid = GUID_random(), + .client_guid = client_guid, .max_credits = WINDOWS_CLIENT_PURE_SMB2_NEGPROT_INITIAL_CREDIT_ASK, .smb3_capabilities = smb311_capabilities_parse("client", lpcfg_client_smb3_signing_algorithms(lp_ctx), -- 2.34.1 From 8a73ac57f5d0b850fedc1238d0932362cb235e0b Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Wed, 10 Aug 2022 11:43:20 +0000 Subject: [PATCH 07/55] s4:torture/smb2: teach smb2.bench.path-contention-shared about --option="torture:qdepth=4" This can now test more than one open/close loop per connection. time smbtorture //127.0.0.1/m -Uroot%test \ smb2.create.bench-path-contention-shared \ --option='torture:bench_path=' \ --option="torture:timelimit=60" \ --option="torture:nprocs=1" \ --option="torture:qdepth=4" The default is still 1, but it's very useful for tests. Signed-off-by: Stefan Metzmacher Reviewed-by: Jeremy Allison BUG: https://bugzilla.samba.org/show_bug.cgi?id=15146 (cherry picked from commit 8ee783c4803d28cccc39144afa7b78c4b9e0cc2e) --- source4/torture/smb2/create.c | 262 ++++++++++++++++++++-------------- 1 file changed, 157 insertions(+), 105 deletions(-) diff --git a/source4/torture/smb2/create.c b/source4/torture/smb2/create.c index 8f1e94b57738..c944fe45c644 100644 --- a/source4/torture/smb2/create.c +++ b/source4/torture/smb2/create.c @@ -3112,11 +3112,14 @@ done: */ struct test_smb2_bench_path_contention_shared_conn; +struct test_smb2_bench_path_contention_shared_loop; struct test_smb2_bench_path_contention_shared_state { struct torture_context *tctx; size_t num_conns; struct test_smb2_bench_path_contention_shared_conn *conns; + size_t num_loops; + struct test_smb2_bench_path_contention_shared_loop *loops; struct timeval starttime; int timecount; int timelimit; @@ -3140,6 +3143,12 @@ struct test_smb2_bench_path_contention_shared_conn { struct test_smb2_bench_path_contention_shared_state *state; int idx; struct smb2_tree *tree; +}; + +struct test_smb2_bench_path_contention_shared_loop { + struct test_smb2_bench_path_contention_shared_state *state; + struct test_smb2_bench_path_contention_shared_conn *conn; + int idx; struct tevent_immediate *im; struct { struct smb2_create io; @@ -3164,138 +3173,138 @@ struct test_smb2_bench_path_contention_shared_conn { NTSTATUS error; }; -static void test_smb2_bench_path_contention_conn_open( - struct test_smb2_bench_path_contention_shared_conn *conn); +static void test_smb2_bench_path_contention_loop_open( + struct test_smb2_bench_path_contention_shared_loop *loop); -static void test_smb2_bench_path_contention_conn_start(struct tevent_context *ctx, +static void test_smb2_bench_path_contention_loop_start(struct tevent_context *ctx, struct tevent_immediate *im, void *private_data) { - struct test_smb2_bench_path_contention_shared_conn *conn = - (struct test_smb2_bench_path_contention_shared_conn *) + struct test_smb2_bench_path_contention_shared_loop *loop = + (struct test_smb2_bench_path_contention_shared_loop *) private_data; - test_smb2_bench_path_contention_conn_open(conn); + test_smb2_bench_path_contention_loop_open(loop); } -static void test_smb2_bench_path_contention_conn_opened(struct smb2_request *req); +static void test_smb2_bench_path_contention_loop_opened(struct smb2_request *req); -static void test_smb2_bench_path_contention_conn_open( - struct test_smb2_bench_path_contention_shared_conn *conn) +static void test_smb2_bench_path_contention_loop_open( + struct test_smb2_bench_path_contention_shared_loop *loop) { - struct test_smb2_bench_path_contention_shared_state *state = conn->state; + struct test_smb2_bench_path_contention_shared_state *state = loop->state; - conn->opens.num_started += 1; - conn->opens.starttime = timeval_current(); - conn->opens.req = smb2_create_send(conn->tree, &conn->opens.io); - torture_assert_goto(state->tctx, conn->opens.req != NULL, + loop->opens.num_started += 1; + loop->opens.starttime = timeval_current(); + loop->opens.req = smb2_create_send(loop->conn->tree, &loop->opens.io); + torture_assert_goto(state->tctx, loop->opens.req != NULL, state->ok, asserted, "smb2_create_send"); - conn->opens.req->async.fn = test_smb2_bench_path_contention_conn_opened; - conn->opens.req->async.private_data = conn; + loop->opens.req->async.fn = test_smb2_bench_path_contention_loop_opened; + loop->opens.req->async.private_data = loop; return; asserted: state->stop = true; } -static void test_smb2_bench_path_contention_conn_close( - struct test_smb2_bench_path_contention_shared_conn *conn); +static void test_smb2_bench_path_contention_loop_close( + struct test_smb2_bench_path_contention_shared_loop *loop); -static void test_smb2_bench_path_contention_conn_opened(struct smb2_request *req) +static void test_smb2_bench_path_contention_loop_opened(struct smb2_request *req) { - struct test_smb2_bench_path_contention_shared_conn *conn = - (struct test_smb2_bench_path_contention_shared_conn *) + struct test_smb2_bench_path_contention_shared_loop *loop = + (struct test_smb2_bench_path_contention_shared_loop *) req->async.private_data; - struct test_smb2_bench_path_contention_shared_state *state = conn->state; - double latency = timeval_elapsed(&conn->opens.starttime); + struct test_smb2_bench_path_contention_shared_state *state = loop->state; + double latency = timeval_elapsed(&loop->opens.starttime); TALLOC_CTX *frame = talloc_stackframe(); - torture_assert_goto(state->tctx, conn->opens.req == req, + torture_assert_goto(state->tctx, loop->opens.req == req, state->ok, asserted, __location__); - conn->error = smb2_create_recv(req, frame, &conn->opens.io); - torture_assert_ntstatus_ok_goto(state->tctx, conn->error, + loop->error = smb2_create_recv(req, frame, &loop->opens.io); + torture_assert_ntstatus_ok_goto(state->tctx, loop->error, state->ok, asserted, __location__); - ZERO_STRUCT(conn->opens.io.out.blobs); + ZERO_STRUCT(loop->opens.io.out.blobs); SMB_ASSERT(latency >= 0.000001); - if (conn->opens.num_finished == 0) { + if (loop->opens.num_finished == 0) { /* first round */ - conn->opens.min_latency = latency; - conn->opens.max_latency = latency; + loop->opens.min_latency = latency; + loop->opens.max_latency = latency; } - conn->opens.num_finished += 1; - conn->opens.total_latency += latency; + loop->opens.num_finished += 1; + loop->opens.total_latency += latency; - if (latency < conn->opens.min_latency) { - conn->opens.min_latency = latency; + if (latency < loop->opens.min_latency) { + loop->opens.min_latency = latency; } - if (latency > conn->opens.max_latency) { - conn->opens.max_latency = latency; + if (latency > loop->opens.max_latency) { + loop->opens.max_latency = latency; } TALLOC_FREE(frame); - test_smb2_bench_path_contention_conn_close(conn); + test_smb2_bench_path_contention_loop_close(loop); return; asserted: state->stop = true; TALLOC_FREE(frame); } -static void test_smb2_bench_path_contention_conn_closed(struct smb2_request *req); +static void test_smb2_bench_path_contention_loop_closed(struct smb2_request *req); -static void test_smb2_bench_path_contention_conn_close( - struct test_smb2_bench_path_contention_shared_conn *conn) +static void test_smb2_bench_path_contention_loop_close( + struct test_smb2_bench_path_contention_shared_loop *loop) { - struct test_smb2_bench_path_contention_shared_state *state = conn->state; + struct test_smb2_bench_path_contention_shared_state *state = loop->state; - conn->closes.num_started += 1; - conn->closes.starttime = timeval_current(); - conn->closes.io.in.file = conn->opens.io.out.file; - conn->closes.req = smb2_close_send(conn->tree, &conn->closes.io); - torture_assert_goto(state->tctx, conn->closes.req != NULL, + loop->closes.num_started += 1; + loop->closes.starttime = timeval_current(); + loop->closes.io.in.file = loop->opens.io.out.file; + loop->closes.req = smb2_close_send(loop->conn->tree, &loop->closes.io); + torture_assert_goto(state->tctx, loop->closes.req != NULL, state->ok, asserted, "smb2_close_send"); - conn->closes.req->async.fn = test_smb2_bench_path_contention_conn_closed; - conn->closes.req->async.private_data = conn; + loop->closes.req->async.fn = test_smb2_bench_path_contention_loop_closed; + loop->closes.req->async.private_data = loop; return; asserted: state->stop = true; } -static void test_smb2_bench_path_contention_conn_closed(struct smb2_request *req) +static void test_smb2_bench_path_contention_loop_closed(struct smb2_request *req) { - struct test_smb2_bench_path_contention_shared_conn *conn = - (struct test_smb2_bench_path_contention_shared_conn *) + struct test_smb2_bench_path_contention_shared_loop *loop = + (struct test_smb2_bench_path_contention_shared_loop *) req->async.private_data; - struct test_smb2_bench_path_contention_shared_state *state = conn->state; - double latency = timeval_elapsed(&conn->closes.starttime); + struct test_smb2_bench_path_contention_shared_state *state = loop->state; + double latency = timeval_elapsed(&loop->closes.starttime); - torture_assert_goto(state->tctx, conn->closes.req == req, + torture_assert_goto(state->tctx, loop->closes.req == req, state->ok, asserted, __location__); - conn->error = smb2_close_recv(req, &conn->closes.io); - torture_assert_ntstatus_ok_goto(state->tctx, conn->error, + loop->error = smb2_close_recv(req, &loop->closes.io); + torture_assert_ntstatus_ok_goto(state->tctx, loop->error, state->ok, asserted, __location__); SMB_ASSERT(latency >= 0.000001); - if (conn->closes.num_finished == 0) { + if (loop->closes.num_finished == 0) { /* first round */ - conn->closes.min_latency = latency; - conn->closes.max_latency = latency; + loop->closes.min_latency = latency; + loop->closes.max_latency = latency; } - conn->closes.num_finished += 1; + loop->closes.num_finished += 1; - conn->closes.total_latency += latency; + loop->closes.total_latency += latency; - if (latency < conn->closes.min_latency) { - conn->closes.min_latency = latency; + if (latency < loop->closes.min_latency) { + loop->closes.min_latency = latency; } - if (latency > conn->closes.max_latency) { - conn->closes.max_latency = latency; + if (latency > loop->closes.max_latency) { + loop->closes.max_latency = latency; } - test_smb2_bench_path_contention_conn_open(conn); + test_smb2_bench_path_contention_loop_open(loop); return; asserted: state->stop = true; @@ -3322,43 +3331,43 @@ static void test_smb2_bench_path_contention_progress(struct tevent_context *ev, state->timecount += 1; - for (i=0;inum_conns;i++) { - struct test_smb2_bench_path_contention_shared_conn *conn = - &state->conns[i]; + for (i=0;inum_loops;i++) { + struct test_smb2_bench_path_contention_shared_loop *loop = + &state->loops[i]; - num_opens += conn->opens.num_finished; - total_open_latency += conn->opens.total_latency; - if (min_open_latency == 0.0 && conn->opens.min_latency != 0.0) { - min_open_latency = conn->opens.min_latency; + num_opens += loop->opens.num_finished; + total_open_latency += loop->opens.total_latency; + if (min_open_latency == 0.0 && loop->opens.min_latency != 0.0) { + min_open_latency = loop->opens.min_latency; } - if (conn->opens.min_latency < min_open_latency) { - min_open_latency = conn->opens.min_latency; + if (loop->opens.min_latency < min_open_latency) { + min_open_latency = loop->opens.min_latency; } if (max_open_latency == 0.0) { - max_open_latency = conn->opens.max_latency; + max_open_latency = loop->opens.max_latency; } - if (conn->opens.max_latency > max_open_latency) { - max_open_latency = conn->opens.max_latency; + if (loop->opens.max_latency > max_open_latency) { + max_open_latency = loop->opens.max_latency; } - conn->opens.num_finished = 0; - conn->opens.total_latency = 0.0; + loop->opens.num_finished = 0; + loop->opens.total_latency = 0.0; - num_closes += conn->closes.num_finished; - total_close_latency += conn->closes.total_latency; - if (min_close_latency == 0.0 && conn->closes.min_latency != 0.0) { - min_close_latency = conn->closes.min_latency; + num_closes += loop->closes.num_finished; + total_close_latency += loop->closes.total_latency; + if (min_close_latency == 0.0 && loop->closes.min_latency != 0.0) { + min_close_latency = loop->closes.min_latency; } - if (conn->closes.min_latency < min_close_latency) { - min_close_latency = conn->closes.min_latency; + if (loop->closes.min_latency < min_close_latency) { + min_close_latency = loop->closes.min_latency; } if (max_close_latency == 0.0) { - max_close_latency = conn->closes.max_latency; + max_close_latency = loop->closes.max_latency; } - if (conn->closes.max_latency > max_close_latency) { - max_close_latency = conn->closes.max_latency; + if (loop->closes.max_latency > max_close_latency) { + max_close_latency = loop->closes.max_latency; } - conn->closes.num_finished = 0; - conn->closes.total_latency = 0.0; + loop->closes.num_finished = 0; + loop->closes.total_latency = 0.0; } state->opens.num_finished += num_opens; @@ -3452,21 +3461,29 @@ static bool test_smb2_bench_path_contention_shared(struct torture_context *tctx, struct test_smb2_bench_path_contention_shared_state *state = NULL; bool ret = true; int torture_nprocs = torture_setting_int(tctx, "nprocs", 4); + int torture_qdepth = torture_setting_int(tctx, "qdepth", 1); size_t i; + size_t li = 0; int timelimit = torture_setting_int(tctx, "timelimit", 10); const char *path = torture_setting_string(tctx, "bench_path", ""); struct smb2_create open_io = { .level = RAW_OPEN_SMB2, }; struct smb2_close close_io = { .level = RAW_CLOSE_SMB2, }; struct tevent_timer *te = NULL; + uint32_t timeout_msec; state = talloc_zero(tctx, struct test_smb2_bench_path_contention_shared_state); torture_assert(tctx, state != NULL, __location__); state->tctx = tctx; + state->num_conns = torture_nprocs; state->conns = talloc_zero_array(state, struct test_smb2_bench_path_contention_shared_conn, - torture_nprocs); + state->num_conns); torture_assert(tctx, state->conns != NULL, __location__); - state->num_conns = torture_nprocs; + state->num_loops = torture_nprocs * torture_qdepth; + state->loops = talloc_zero_array(state, + struct test_smb2_bench_path_contention_shared_loop, + state->num_loops); + torture_assert(tctx, state->loops != NULL, __location__); state->ok = true; state->timelimit = MAX(timelimit, 1); @@ -3482,29 +3499,64 @@ static bool test_smb2_bench_path_contention_shared(struct torture_context *tctx, open_io.in.create_flags = NTCREATEX_FLAGS_EXTENDED; open_io.in.oplock_level = SMB2_OPLOCK_LEVEL_NONE; + timeout_msec = tree->session->transport->options.request_timeout * 1000; + torture_comment(tctx, "Opening %zd connections\n", state->num_conns); for (i=0;inum_conns;i++) { + struct smb2_tree *ct = NULL; + DATA_BLOB out_input_buffer = data_blob_null; + DATA_BLOB out_output_buffer = data_blob_null; + size_t pcli; + state->conns[i].state = state; state->conns[i].idx = i; - state->conns[i].im = tevent_create_immediate(state->conns); - torture_assert(tctx, state->conns[i].im != NULL, __location__); - if (!torture_smb2_connection(tctx, &state->conns[i].tree)) { + if (!torture_smb2_connection(tctx, &ct)) { torture_comment(tctx, "Failed opening %zd/%zd connections\n", i, state->num_conns); return false; } - talloc_steal(state->conns, state->conns[i].tree); - state->conns[i].opens.io = open_io; - state->conns[i].closes.io = close_io; - - tevent_schedule_immediate(state->conns[i].im, - tctx->ev, - test_smb2_bench_path_contention_conn_start, - &state->conns[i]); + state->conns[i].tree = talloc_steal(state->conns, ct); + + smb2cli_conn_set_max_credits(ct->session->transport->conn, 8192); + smb2cli_ioctl(ct->session->transport->conn, + timeout_msec, + ct->session->smbXcli, + ct->smbXcli, + UINT64_MAX, /* in_fid_persistent */ + UINT64_MAX, /* in_fid_volatile */ + UINT32_MAX, + 0, /* in_max_input_length */ + NULL, /* in_input_buffer */ + 1, /* in_max_output_length */ + NULL, /* in_output_buffer */ + SMB2_IOCTL_FLAG_IS_FSCTL, + ct, + &out_input_buffer, + &out_output_buffer); + torture_assert(tctx, + smbXcli_conn_is_connected(ct->session->transport->conn), + "smbXcli_conn_is_connected"); + for (pcli = 0; pcli < torture_qdepth; pcli++) { + struct test_smb2_bench_path_contention_shared_loop *loop = &state->loops[li]; + + loop->idx = li++; + loop->state = state; + loop->conn = &state->conns[i]; + loop->im = tevent_create_immediate(state->loops); + torture_assert(tctx, loop->im != NULL, __location__); + loop->opens.io = open_io; + loop->closes.io = close_io; + + tevent_schedule_immediate(loop->im, + tctx->ev, + test_smb2_bench_path_contention_loop_start, + loop); + } } - torture_comment(tctx, "Opened %zd connections\n", state->num_conns); + torture_comment(tctx, "Opened %zu connections with qdepth=%d => %zu loops\n", + state->num_conns, torture_qdepth, state->num_loops); torture_comment(tctx, "Running for %d seconds\n", state->timelimit); -- 2.34.1 From f3649ea290227ea5e1e0052fb9bfe7e32d7263eb Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Wed, 10 Aug 2022 13:14:52 +0000 Subject: [PATCH 08/55] s4:torture/smb2: add smb2.bench.echo This test calls SMB2_Echo in a loop per connection. For 4 connections with 2 parallel loops use this: time smbtorture //127.0.0.1/m -Uroot%test smb2.bench.echo \ --option="torture:timelimit=600" \ --option="torture:nprocs=1" \ --option="torture:qdepth=2" Sometimes the bottleneck is the smbtorture process. In order to bring the smbd process to 100% cpu, you can use '--option="libsmb:client_guid=6112f7d3-9528-4a2a-8861-0ca129aae6c4"' and run multiple instances of the test at the same time, which both talk to the same smbd process. This is a very useful test to show how many requests are possible at the raw SMB2 layer. Signed-off-by: Stefan Metzmacher Reviewed-by: Jeremy Allison Autobuild-User(master): Jeremy Allison Autobuild-Date(master): Thu Aug 11 19:23:37 UTC 2022 on sn-devel-184 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15146 (cherry picked from commit 23988f19e7cc2823d6c0c0f40af0195d0a3b81bf) --- source4/torture/smb2/create.c | 329 ++++++++++++++++++++++++++++++++++ 1 file changed, 329 insertions(+) diff --git a/source4/torture/smb2/create.c b/source4/torture/smb2/create.c index c944fe45c644..9277488e9d7f 100644 --- a/source4/torture/smb2/create.c +++ b/source4/torture/smb2/create.c @@ -22,6 +22,7 @@ #include "includes.h" #include "libcli/smb2/smb2.h" #include "libcli/smb2/smb2_calls.h" +#include "libcli/smb/smbXcli_base.h" #include "torture/torture.h" #include "torture/util.h" #include "torture/smb2/proto.h" @@ -126,6 +127,333 @@ __location__, (unsigned int)(sattrib), fname); \ }} while (0) +/* + stress testing keepalive iops + */ + +struct test_smb2_bench_echo_conn; +struct test_smb2_bench_echo_loop; + +struct test_smb2_bench_echo_state { + struct torture_context *tctx; + size_t num_conns; + struct test_smb2_bench_echo_conn *conns; + size_t num_loops; + struct test_smb2_bench_echo_loop *loops; + struct timeval starttime; + int timecount; + int timelimit; + uint64_t num_finished; + double total_latency; + double min_latency; + double max_latency; + bool ok; + bool stop; +}; + +struct test_smb2_bench_echo_conn { + struct test_smb2_bench_echo_state *state; + int idx; + struct smb2_tree *tree; +}; + +struct test_smb2_bench_echo_loop { + struct test_smb2_bench_echo_state *state; + struct test_smb2_bench_echo_conn *conn; + int idx; + struct tevent_immediate *im; + struct tevent_req *req; + struct timeval starttime; + uint64_t num_started; + uint64_t num_finished; + double total_latency; + double min_latency; + double max_latency; + NTSTATUS error; +}; + +static void test_smb2_bench_echo_loop_do( + struct test_smb2_bench_echo_loop *loop); + +static void test_smb2_bench_echo_loop_start(struct tevent_context *ctx, + struct tevent_immediate *im, + void *private_data) +{ + struct test_smb2_bench_echo_loop *loop = + (struct test_smb2_bench_echo_loop *) + private_data; + + test_smb2_bench_echo_loop_do(loop); +} + +static void test_smb2_bench_echo_loop_done(struct tevent_req *req); + +static void test_smb2_bench_echo_loop_do( + struct test_smb2_bench_echo_loop *loop) +{ + struct test_smb2_bench_echo_state *state = loop->state; + + loop->num_started += 1; + loop->starttime = timeval_current(); + loop->req = smb2cli_echo_send(state->loops, + state->tctx->ev, + loop->conn->tree->session->transport->conn, + 1000); + torture_assert_goto(state->tctx, loop->req != NULL, + state->ok, asserted, "smb2_create_send"); + + tevent_req_set_callback(loop->req, + test_smb2_bench_echo_loop_done, + loop); + return; +asserted: + state->stop = true; +} + +static void test_smb2_bench_echo_loop_done(struct tevent_req *req) +{ + struct test_smb2_bench_echo_loop *loop = + (struct test_smb2_bench_echo_loop *) + _tevent_req_callback_data(req); + struct test_smb2_bench_echo_state *state = loop->state; + double latency = timeval_elapsed(&loop->starttime); + TALLOC_CTX *frame = talloc_stackframe(); + + torture_assert_goto(state->tctx, loop->req == req, + state->ok, asserted, __location__); + loop->error = smb2cli_echo_recv(req); + torture_assert_ntstatus_ok_goto(state->tctx, loop->error, + state->ok, asserted, __location__); + SMB_ASSERT(latency >= 0.000001); + + if (loop->num_finished == 0) { + /* first round */ + loop->min_latency = latency; + loop->max_latency = latency; + } + + loop->num_finished += 1; + loop->total_latency += latency; + + if (latency < loop->min_latency) { + loop->min_latency = latency; + } + + if (latency > loop->max_latency) { + loop->max_latency = latency; + } + + TALLOC_FREE(frame); + test_smb2_bench_echo_loop_do(loop); + return; +asserted: + state->stop = true; + TALLOC_FREE(frame); +} + +static void test_smb2_bench_echo_progress(struct tevent_context *ev, + struct tevent_timer *te, + struct timeval current_time, + void *private_data) +{ + struct test_smb2_bench_echo_state *state = + (struct test_smb2_bench_echo_state *)private_data; + uint64_t num_echos = 0; + double total_echo_latency = 0; + double min_echo_latency = 0; + double max_echo_latency = 0; + double avs_echo_latency = 0; + size_t i; + + state->timecount += 1; + + for (i=0;inum_loops;i++) { + struct test_smb2_bench_echo_loop *loop = + &state->loops[i]; + + num_echos += loop->num_finished; + total_echo_latency += loop->total_latency; + if (min_echo_latency == 0.0 && loop->min_latency != 0.0) { + min_echo_latency = loop->min_latency; + } + if (loop->min_latency < min_echo_latency) { + min_echo_latency = loop->min_latency; + } + if (max_echo_latency == 0.0) { + max_echo_latency = loop->max_latency; + } + if (loop->max_latency > max_echo_latency) { + max_echo_latency = loop->max_latency; + } + loop->num_finished = 0; + loop->total_latency = 0.0; + } + + state->num_finished += num_echos; + state->total_latency += total_echo_latency; + if (state->min_latency == 0.0 && min_echo_latency != 0.0) { + state->min_latency = min_echo_latency; + } + if (min_echo_latency < state->min_latency) { + state->min_latency = min_echo_latency; + } + if (state->max_latency == 0.0) { + state->max_latency = max_echo_latency; + } + if (max_echo_latency > state->max_latency) { + state->max_latency = max_echo_latency; + } + + if (state->timecount < state->timelimit) { + te = tevent_add_timer(state->tctx->ev, + state, + timeval_current_ofs(1, 0), + test_smb2_bench_echo_progress, + state); + torture_assert_goto(state->tctx, te != NULL, + state->ok, asserted, "tevent_add_timer"); + + if (!torture_setting_bool(state->tctx, "progress", true)) { + return; + } + + avs_echo_latency = total_echo_latency / num_echos; + + torture_comment(state->tctx, + "%.2f second: " + "echo[num/s=%llu,avslat=%.6f,minlat=%.6f,maxlat=%.6f] \r", + timeval_elapsed(&state->starttime), + (unsigned long long)num_echos, + avs_echo_latency, + min_echo_latency, + max_echo_latency); + return; + } + + avs_echo_latency = state->total_latency / state->num_finished; + num_echos = state->num_finished / state->timelimit; + + torture_comment(state->tctx, + "%.2f second: " + "echo[num/s=%llu,avslat=%.6f,minlat=%.6f,maxlat=%.6f]\n", + timeval_elapsed(&state->starttime), + (unsigned long long)num_echos, + avs_echo_latency, + state->min_latency, + state->max_latency); + +asserted: + state->stop = true; +} + +static bool test_smb2_bench_echo(struct torture_context *tctx, + struct smb2_tree *tree) +{ + struct test_smb2_bench_echo_state *state = NULL; + bool ret = true; + int torture_nprocs = torture_setting_int(tctx, "nprocs", 4); + int torture_qdepth = torture_setting_int(tctx, "qdepth", 1); + size_t i; + size_t li = 0; + int timelimit = torture_setting_int(tctx, "timelimit", 10); + struct tevent_timer *te = NULL; + uint32_t timeout_msec; + + state = talloc_zero(tctx, struct test_smb2_bench_echo_state); + torture_assert(tctx, state != NULL, __location__); + state->tctx = tctx; + state->num_conns = torture_nprocs; + state->conns = talloc_zero_array(state, + struct test_smb2_bench_echo_conn, + state->num_conns); + torture_assert(tctx, state->conns != NULL, __location__); + state->num_loops = torture_nprocs * torture_qdepth; + state->loops = talloc_zero_array(state, + struct test_smb2_bench_echo_loop, + state->num_loops); + torture_assert(tctx, state->loops != NULL, __location__); + state->ok = true; + state->timelimit = MAX(timelimit, 1); + + timeout_msec = tree->session->transport->options.request_timeout * 1000; + + torture_comment(tctx, "Opening %zu connections\n", state->num_conns); + + for (i=0;inum_conns;i++) { + struct smb2_tree *ct = NULL; + DATA_BLOB out_input_buffer = data_blob_null; + DATA_BLOB out_output_buffer = data_blob_null; + size_t pcli; + + state->conns[i].state = state; + state->conns[i].idx = i; + + if (!torture_smb2_connection(tctx, &ct)) { + torture_comment(tctx, "Failed opening %zu/%zu connections\n", i, state->num_conns); + return false; + } + state->conns[i].tree = talloc_steal(state->conns, ct); + + smb2cli_conn_set_max_credits(ct->session->transport->conn, 8192); + smb2cli_ioctl(ct->session->transport->conn, + timeout_msec, + ct->session->smbXcli, + ct->smbXcli, + UINT64_MAX, /* in_fid_persistent */ + UINT64_MAX, /* in_fid_volatile */ + UINT32_MAX, + 0, /* in_max_input_length */ + NULL, /* in_input_buffer */ + 1, /* in_max_output_length */ + NULL, /* in_output_buffer */ + SMB2_IOCTL_FLAG_IS_FSCTL, + ct, + &out_input_buffer, + &out_output_buffer); + torture_assert(tctx, + smbXcli_conn_is_connected(ct->session->transport->conn), + "smbXcli_conn_is_connected"); + + for (pcli = 0; pcli < torture_qdepth; pcli++) { + struct test_smb2_bench_echo_loop *loop = &state->loops[li]; + + loop->idx = li++; + loop->state = state; + loop->conn = &state->conns[i]; + loop->im = tevent_create_immediate(state->loops); + torture_assert(tctx, loop->im != NULL, __location__); + + tevent_schedule_immediate(loop->im, + tctx->ev, + test_smb2_bench_echo_loop_start, + loop); + } + } + + torture_comment(tctx, "Opened %zu connections with qdepth=%d => %zu loops\n", + state->num_conns, torture_qdepth, state->num_loops); + + torture_comment(tctx, "Running for %d seconds\n", state->timelimit); + + state->starttime = timeval_current(); + + te = tevent_add_timer(tctx->ev, + state, + timeval_current_ofs(1, 0), + test_smb2_bench_echo_progress, + state); + torture_assert(tctx, te != NULL, __location__); + + while (!state->stop) { + int rc = tevent_loop_once(tctx->ev); + torture_assert_int_equal(tctx, rc, 0, "tevent_loop_once"); + } + + torture_comment(tctx, "%.2f seconds\n", timeval_elapsed(&state->starttime)); + TALLOC_FREE(state); + return ret; +} + /* test some interesting combinations found by gentest */ @@ -3645,6 +3973,7 @@ struct torture_suite *torture_smb2_bench_init(TALLOC_CTX *ctx) torture_suite_add_1smb2_test(suite, "oplock1", test_smb2_bench_oplock); torture_suite_add_1smb2_test(suite, "path-contention-shared", test_smb2_bench_path_contention_shared); + torture_suite_add_1smb2_test(suite, "echo", test_smb2_bench_echo); suite->description = talloc_strdup(suite, "SMB2-BENCH tests"); -- 2.34.1 From 25863fbd4137f25b29b6dec181863cc613d91e23 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 5 Aug 2022 19:27:33 -0700 Subject: [PATCH 09/55] s3: smbd: Fix cosmetic bug logging pathnames from Linux kernel clients using SMB1 DFS calls. The Linux kernel SMB1 client has a bug - it sends DFS pathnames as: \\server\share\path instead of: \server\share\path Causing us to mis-parse server,share,remaining_path here and jump into 'goto local_path' at 'share\path' instead of 'path'. This doesn't cause an error as the limits on share names are similar to those on pathnames. parse_dfs_path() which we call before filename parsing copes with this by calling trim_char on the leading '\' characters before processing. Do the same here so logging of pathnames looks better. How did I find this ? Lots and lots of manual testing with the Linux kernel client to make sure all the recent changes haven't broken Linux SMB1/2/3 DFS :-). BUG: https://bugzilla.samba.org/show_bug.cgi?id=15144 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme BUG: https://bugzilla.samba.org/show_bug.cgi?id=15146 (cherry picked from commit 2818fd6910201fd4a18b921933a0b7392a0a8995) --- source3/smbd/smb2_reply.c | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/source3/smbd/smb2_reply.c b/source3/smbd/smb2_reply.c index 42601879c098..870c3bcb898b 100644 --- a/source3/smbd/smb2_reply.c +++ b/source3/smbd/smb2_reply.c @@ -310,6 +310,29 @@ static size_t srvstr_get_path_internal(TALLOC_CTX *ctx, */ server = dst; + /* + * Cosmetic fix for Linux-only DFS clients. + * The Linux kernel SMB1 client has a bug - it sends + * DFS pathnames as: + * + * \\server\share\path + * + * Causing us to mis-parse server,share,remaining_path here + * and jump into 'goto local_path' at 'share\path' instead + * of 'path'. + * + * This doesn't cause an error as the limits on share names + * are similar to those on pathnames. + * + * parse_dfs_path() which we call before filename parsing + * copes with this by calling trim_char on the leading '\' + * characters before processing. + * Do the same here so logging of pathnames looks better. + */ + if (server[1] == path_sep) { + trim_char(&server[1], path_sep, '\0'); + } + /* * Look to see if we also have /share following. */ -- 2.34.1 From 16ac9c967b3c406a4a572eac6f493a2bf5ad50b3 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 5 Aug 2022 12:16:44 -0700 Subject: [PATCH 10/55] s3: smbd: Add new function check_path_syntax_smb2_msdfs() for SMB2 MSDFS paths. #ifdef'ed out as static and not yet used. We can't just call check_path_syntax() on these as they are of the form hostname\share[\extrapath] (where [\extrapath] is optional). hostname here can be an IPv6 ':' separated address, which check_path_syntax() fails on due to the streamname processing. NB. This also has to cope with out existing (broken) libsmbclient libraries that sometimes set the DFS flag and then send a local pathname. Cope by just calling the normal check_path_syntax() on the whole pathname in that case. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15144 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme BUG: https://bugzilla.samba.org/show_bug.cgi?id=15146 (cherry picked from commit bcba5502282eb6dcc346d7c63aa3218cda2f9bb0) --- source3/smbd/smb2_reply.c | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/source3/smbd/smb2_reply.c b/source3/smbd/smb2_reply.c index 870c3bcb898b..6052f94354a8 100644 --- a/source3/smbd/smb2_reply.c +++ b/source3/smbd/smb2_reply.c @@ -243,6 +243,44 @@ NTSTATUS check_path_syntax_posix(char *path) return check_path_syntax_internal(path, true); } +#if 0 +/**************************************************************************** + Check the path for an SMB2 DFS path. + SMB2 DFS paths look like hostname\share (followed by a possible \extrapath. + Path returned from here must look like: + hostname/share (followed by a possible /extrapath). +****************************************************************************/ + +static NTSTATUS check_path_syntax_smb2_msdfs(char *path) +{ + char *share = NULL; + char *remaining_path = NULL; + /* No SMB2 names can start with '\\' */ + if (path[0] == '\\') { + return NT_STATUS_OBJECT_NAME_INVALID; + } + /* + * smbclient libraries sometimes set the DFS flag and send + * local pathnames. Cope with this by just calling + * check_path_syntax() on the whole path if it doesn't + * look like a DFS path, similar to what parse_dfs_path() does. + */ + /* servername should be at path[0] */ + share = strchr(path, '\\'); + if (share == NULL) { + return check_path_syntax(path); + } + *share++ = '/'; + remaining_path = strchr(share, '\\'); + if (remaining_path == NULL) { + /* Only hostname\share. We're done. */ + return NT_STATUS_OK; + } + *remaining_path++ = '/'; + return check_path_syntax(remaining_path); +} +#endif + /**************************************************************************** Pull a string and check the path allowing a wildcard - provide for error return. Passes in posix flag. -- 2.34.1 From 712bd7992ba9fda5ef949de7eb90d185c911c3f4 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Tue, 9 Aug 2022 10:36:00 -0700 Subject: [PATCH 11/55] s3: smbd: Add helper function check_path_syntax_smb2(). Not yet used, but uses check_path_syntax_smb2_msdfs() so remove the #ifdef's around that. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15144 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme BUG: https://bugzilla.samba.org/show_bug.cgi?id=15146 (cherry picked from commit 7bd7fa0a0b46ad6826097a1987595e2ab6f83384) --- source3/smbd/proto.h | 1 + source3/smbd/smb2_reply.c | 11 +++++++++-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h index ceee52d79cd3..33fc222311eb 100644 --- a/source3/smbd/proto.h +++ b/source3/smbd/proto.h @@ -941,6 +941,7 @@ bool disk_quotas(connection_struct *conn, struct smb_filename *fname, NTSTATUS check_path_syntax(char *path); NTSTATUS check_path_syntax_posix(char *path); +NTSTATUS check_path_syntax_smb2(char *path, bool dfs_path); size_t srvstr_get_path(TALLOC_CTX *ctx, const char *inbuf, uint16_t smb_flags2, diff --git a/source3/smbd/smb2_reply.c b/source3/smbd/smb2_reply.c index 6052f94354a8..0303db428f25 100644 --- a/source3/smbd/smb2_reply.c +++ b/source3/smbd/smb2_reply.c @@ -243,7 +243,6 @@ NTSTATUS check_path_syntax_posix(char *path) return check_path_syntax_internal(path, true); } -#if 0 /**************************************************************************** Check the path for an SMB2 DFS path. SMB2 DFS paths look like hostname\share (followed by a possible \extrapath. @@ -279,7 +278,15 @@ static NTSTATUS check_path_syntax_smb2_msdfs(char *path) *remaining_path++ = '/'; return check_path_syntax(remaining_path); } -#endif + +NTSTATUS check_path_syntax_smb2(char *path, bool dfs_path) +{ + if (dfs_path) { + return check_path_syntax_smb2_msdfs(path); + } else { + return check_path_syntax(path); + } +} /**************************************************************************** Pull a string and check the path allowing a wildcard - provide for error return. -- 2.34.1 From 61a19d60b2aa2c21cf985d88cb35e922708d054f Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Tue, 9 Aug 2022 10:39:41 -0700 Subject: [PATCH 12/55] s3: smbd: In smbd_smb2_create_send() call the helper function check_path_syntax_smb2(). Previously for DFS names we were skipping this. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15144 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme BUG: https://bugzilla.samba.org/show_bug.cgi?id=15146 (cherry picked from commit 4fafc3418931de06ea2d91baca1eef8d904cc4e6) --- source3/smbd/smb2_create.c | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/source3/smbd/smb2_create.c b/source3/smbd/smb2_create.c index dbafaf015975..8ef605dea66a 100644 --- a/source3/smbd/smb2_create.c +++ b/source3/smbd/smb2_create.c @@ -714,6 +714,7 @@ static struct tevent_req *smbd_smb2_create_send(TALLOC_CTX *mem_ctx, struct files_struct *dirfsp = NULL; struct smb_filename *smb_fname = NULL; uint32_t ucf_flags; + bool is_dfs = false; req = tevent_req_create(mem_ctx, &state, struct smbd_smb2_create_state); @@ -959,18 +960,13 @@ static struct tevent_req *smbd_smb2_create_send(TALLOC_CTX *mem_ctx, state->lease_ptr = NULL; } - /* - * For a DFS path the function parse_dfs_path() - * will do the path processing. - */ + is_dfs = (smb1req->flags2 & FLAGS2_DFS_PATHNAMES); - if (!(smb1req->flags2 & FLAGS2_DFS_PATHNAMES)) { - /* convert '\\' into '/' */ - status = check_path_syntax(state->fname); - if (!NT_STATUS_IS_OK(status)) { - tevent_req_nterror(req, status); - return tevent_req_post(req, state->ev); - } + /* convert '\\' into '/' */ + status = check_path_syntax_smb2(state->fname, is_dfs); + if (!NT_STATUS_IS_OK(status)) { + tevent_req_nterror(req, status); + return tevent_req_post(req, state->ev); } ucf_flags = filename_create_ucf_flags( -- 2.34.1 From 8f381aead0ff9a62b35b2d04ce80c508095ed3a8 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Tue, 9 Aug 2022 10:41:39 -0700 Subject: [PATCH 13/55] s3: smbd: Make sure we have identical check_path_syntax logic in smbd_smb2_create_durable_lease_check(), as for smb2_create. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15144 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme BUG: https://bugzilla.samba.org/show_bug.cgi?id=15146 (cherry picked from commit a2a097fc3d6a89fb970c1ea3ea75fde93ddb545e) --- source3/smbd/smb2_create.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/source3/smbd/smb2_create.c b/source3/smbd/smb2_create.c index 8ef605dea66a..75b9c7d28ff3 100644 --- a/source3/smbd/smb2_create.c +++ b/source3/smbd/smb2_create.c @@ -430,6 +430,7 @@ static NTSTATUS smbd_smb2_create_durable_lease_check(struct smb_request *smb1req uint32_t ucf_flags; NTTIME twrp = fsp->fsp_name->twrp; NTSTATUS status; + bool is_dfs = (smb1req->flags2 & FLAGS2_DFS_PATHNAMES); if (lease_ptr == NULL) { if (fsp->oplock_type != LEASE_OPLOCK) { @@ -459,7 +460,7 @@ static NTSTATUS smbd_smb2_create_durable_lease_check(struct smb_request *smb1req } /* This also converts '\' to '/' */ - status = check_path_syntax(filename); + status = check_path_syntax_smb2(filename, is_dfs); if (!NT_STATUS_IS_OK(status)) { TALLOC_FREE(filename); return status; -- 2.34.1 From c948f03b0a3b3a645420f1eac01648e4fba966fd Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Tue, 9 Aug 2022 10:43:45 -0700 Subject: [PATCH 14/55] s3: smbd: Ensure smb2_file_rename_information() uses the SMB2 pathname parsers, not the SMB1 parsers. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15144 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme BUG: https://bugzilla.samba.org/show_bug.cgi?id=15146 (cherry picked from commit 0a4a27ce48bc7090aa821eea5e56f8d44c686716) --- source3/smbd/smb2_trans2.c | 30 ++++++++++++------------------ 1 file changed, 12 insertions(+), 18 deletions(-) diff --git a/source3/smbd/smb2_trans2.c b/source3/smbd/smb2_trans2.c index d0f30a782bf9..b2a0cc4140a5 100644 --- a/source3/smbd/smb2_trans2.c +++ b/source3/smbd/smb2_trans2.c @@ -4957,6 +4957,7 @@ static NTSTATUS smb2_file_rename_information(connection_struct *conn, uint32_t ucf_flags = ucf_flags_from_smb_request(req); NTTIME dst_twrp = 0; NTSTATUS status = NT_STATUS_OK; + bool is_dfs = (req->flags2 & FLAGS2_DFS_PATHNAMES); TALLOC_CTX *ctx = talloc_tos(); if (!fsp) { @@ -4974,25 +4975,18 @@ static NTSTATUS smb2_file_rename_information(connection_struct *conn, return NT_STATUS_INVALID_PARAMETER; } - if (req->posix_pathnames) { - srvstr_get_path_posix(ctx, - pdata, - req->flags2, - &newname, - &pdata[20], - len, - STR_TERMINATE, - &status); - } else { - srvstr_get_path(ctx, - pdata, - req->flags2, - &newname, - &pdata[20], - len, - STR_TERMINATE, - &status); + (void)srvstr_pull_talloc(ctx, + pdata, + req->flags2, + &newname, + &pdata[20], + len, + STR_TERMINATE); + + if (newname == NULL) { + return NT_STATUS_INVALID_PARAMETER; } + status = check_path_syntax_smb2(newname, is_dfs); if (!NT_STATUS_IS_OK(status)) { return status; } -- 2.34.1 From 53cc085d9bf16d75637032e63066d8731f1d6de9 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 10 Aug 2022 11:06:47 -0700 Subject: [PATCH 15/55] s3: smbd: Add TALLOC_CTX * parameter to parse_dfs_path(). Not yet used. Preparing to remove 'struct dfs_path'. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15144 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme BUG: https://bugzilla.samba.org/show_bug.cgi?id=15146 (cherry picked from commit 2df8a8ab87a1372f2b67880be4454a0285b3104b) --- source3/smbd/msdfs.c | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/source3/smbd/msdfs.c b/source3/smbd/msdfs.c index a52a2449965f..abe447dd0566 100644 --- a/source3/smbd/msdfs.c +++ b/source3/smbd/msdfs.c @@ -58,7 +58,8 @@ JRA. **********************************************************************/ -static NTSTATUS parse_dfs_path(connection_struct *conn, +static NTSTATUS parse_dfs_path(TALLOC_CTX *ctx, + connection_struct *conn, const char *pathname, bool allow_broken_path, struct dfs_path *pdp) /* MUST BE TALLOCED */ @@ -870,8 +871,11 @@ NTSTATUS dfs_redirect(TALLOC_CTX *ctx, return NT_STATUS_NO_MEMORY; } - status = parse_dfs_path(conn, path_in, - allow_broken_path, pdp); + status = parse_dfs_path(ctx, + conn, + path_in, + allow_broken_path, + pdp); if (!NT_STATUS_IS_OK(status)) { TALLOC_FREE(pdp); return status; @@ -1020,7 +1024,11 @@ NTSTATUS get_referred_path(TALLOC_CTX *ctx, *self_referralp = False; - status = parse_dfs_path(NULL, dfs_path, allow_broken_path, pdp); + status = parse_dfs_path(frame, + NULL, + dfs_path, + allow_broken_path, + pdp); if (!NT_STATUS_IS_OK(status)) { TALLOC_FREE(frame); return status; @@ -1269,7 +1277,11 @@ bool create_junction(TALLOC_CTX *ctx, if (!pdp) { return False; } - status = parse_dfs_path(NULL, dfs_path, allow_broken_path, pdp); + status = parse_dfs_path(ctx, + NULL, + dfs_path, + allow_broken_path, + pdp); if (!NT_STATUS_IS_OK(status)) { return False; } -- 2.34.1 From ce083fee9797799a0f0654ee857cc64897c1be96 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 10 Aug 2022 11:17:49 -0700 Subject: [PATCH 16/55] s3: smbd: Remove use of 'struct dfs_path'. Not needed for a (hostname, servicename, path) tuple. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15144 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme BUG: https://bugzilla.samba.org/show_bug.cgi?id=15146 (cherry picked from commit f92711f000a3cb658dfb8fffe92ae6bba78b4f91) --- source3/smbd/msdfs.c | 167 ++++++++++++++++++++++--------------------- 1 file changed, 87 insertions(+), 80 deletions(-) diff --git a/source3/smbd/msdfs.c b/source3/smbd/msdfs.c index abe447dd0566..21288aaf664a 100644 --- a/source3/smbd/msdfs.c +++ b/source3/smbd/msdfs.c @@ -62,24 +62,20 @@ static NTSTATUS parse_dfs_path(TALLOC_CTX *ctx, connection_struct *conn, const char *pathname, bool allow_broken_path, - struct dfs_path *pdp) /* MUST BE TALLOCED */ + char **_hostname, + char **_servicename, + char **_remaining_path) { const struct loadparm_substitution *lp_sub = loadparm_s3_global_substitution(); - char *pathname_local; - char *p; - char *servicename; - char *eos_ptr; - - ZERO_STRUCTP(pdp); - - /* - * This is the only talloc we should need to do - * on the struct dfs_path. All the pointers inside - * it should point to offsets within this string. - */ + char *hostname = NULL; + char *pathname_local = NULL; + char *p = NULL; + char *servicename = NULL; + char *reqpath = NULL; + char *eos_ptr = NULL; - pathname_local = talloc_strdup(pdp, pathname); + pathname_local = talloc_strdup(ctx, pathname); if (pathname_local == NULL) { return NT_STATUS_NO_MEMORY; } @@ -109,8 +105,8 @@ static NTSTATUS parse_dfs_path(TALLOC_CTX *ctx, * to cope with known broken SMB1 clients. */ - pdp->hostname = eos_ptr; /* "" */ - pdp->servicename = eos_ptr; /* "" */ + hostname = eos_ptr; /* "" */ + servicename = eos_ptr; /* "" */ DBG_ERR("trying to convert %s to a local path\n", p); goto local_path; @@ -134,17 +130,17 @@ static NTSTATUS parse_dfs_path(TALLOC_CTX *ctx, * Try and convert to a local path. */ - pdp->hostname = eos_ptr; /* "" */ - pdp->servicename = eos_ptr; /* "" */ + hostname = eos_ptr; /* "" */ + servicename = eos_ptr; /* "" */ p = pathname_local; DBG_ERR("trying to convert %s to a local path\n", p); goto local_path; } *p = '\0'; - pdp->hostname = pathname_local; + hostname = pathname_local; - DBG_DEBUG("hostname: %s\n",pdp->hostname); + DBG_DEBUG("hostname: %s\n", hostname); /* Parse out servicename. */ servicename = p+1; @@ -165,9 +161,6 @@ static NTSTATUS parse_dfs_path(TALLOC_CTX *ctx, * Try and convert to a local path. */ - pdp->hostname = eos_ptr; /* "" */ - pdp->servicename = eos_ptr; /* "" */ - /* Repair the path - replace the sepchar's we nulled out */ servicename--; @@ -176,20 +169,23 @@ static NTSTATUS parse_dfs_path(TALLOC_CTX *ctx, *p = '/'; } + hostname = eos_ptr; /* "" */ + servicename = eos_ptr; /* "" */ + p = pathname_local; DBG_ERR("trying to convert %s to a local path\n", pathname_local); goto local_path; } - pdp->servicename = servicename; + servicename = servicename; - DBG_DEBUG("servicename: %s\n", pdp->servicename); + DBG_DEBUG("servicename: %s\n", servicename); if(p == NULL) { /* Client sent self referral \server\share. */ - pdp->reqpath = eos_ptr; /* "" */ - return NT_STATUS_OK; + reqpath = eos_ptr; /* "" */ + goto out; } p++; @@ -202,8 +198,31 @@ static NTSTATUS parse_dfs_path(TALLOC_CTX *ctx, * '/' separators. */ - pdp->reqpath = p; - DBG_DEBUG("rest of the path: %s\n", pdp->reqpath); + reqpath = p; + + out: + + DBG_DEBUG("rest of the path: %s\n", reqpath); + + if (_hostname != NULL) { + *_hostname = talloc_strdup(ctx, hostname); + if (*_hostname == NULL) { + return NT_STATUS_NO_MEMORY; + } + } + if (_servicename != NULL) { + *_servicename = talloc_strdup(ctx, servicename); + if (*_servicename == NULL) { + return NT_STATUS_NO_MEMORY; + } + } + if (_remaining_path != NULL) { + *_remaining_path = talloc_strdup(ctx, reqpath); + if (*_remaining_path == NULL) { + return NT_STATUS_NO_MEMORY; + } + } + TALLOC_FREE(pathname_local); return NT_STATUS_OK; } @@ -663,8 +682,7 @@ bool is_msdfs_link(struct files_struct *dirfsp, static NTSTATUS dfs_path_lookup(TALLOC_CTX *ctx, connection_struct *conn, const char *dfspath, /* Incoming complete dfs path */ - const struct dfs_path *pdp, /* Parsed out - server+share+extrapath. */ + const char *reqpath, /* Parsed out remaining path. */ uint32_t ucf_flags, NTTIME *_twrp, size_t *consumedcntp, @@ -681,7 +699,7 @@ static NTSTATUS dfs_path_lookup(TALLOC_CTX *ctx, components). */ DEBUG(10,("dfs_path_lookup: Conn path = %s reqpath = %s\n", - conn->connectpath, pdp->reqpath)); + conn->connectpath, reqpath)); /* * Note the unix path conversion here we're doing we @@ -690,7 +708,7 @@ static NTSTATUS dfs_path_lookup(TALLOC_CTX *ctx, * unix_convert later in the codepath. */ - status = unix_convert(ctx, conn, pdp->reqpath, 0, &smb_fname, + status = unix_convert(ctx, conn, reqpath, 0, &smb_fname, ucf_flags); if (!NT_STATUS_IS_OK(status)) { @@ -864,25 +882,23 @@ NTSTATUS dfs_redirect(TALLOC_CTX *ctx, { const struct loadparm_substitution *lp_sub = loadparm_s3_global_substitution(); + char *hostname = NULL; + char *servicename = NULL; + char *reqpath = NULL; NTSTATUS status; - struct dfs_path *pdp = talloc(ctx, struct dfs_path); - - if (!pdp) { - return NT_STATUS_NO_MEMORY; - } status = parse_dfs_path(ctx, conn, path_in, allow_broken_path, - pdp); + &hostname, + &servicename, + &reqpath); if (!NT_STATUS_IS_OK(status)) { - TALLOC_FREE(pdp); return status; } - if (pdp->reqpath[0] == '\0') { - TALLOC_FREE(pdp); + if (reqpath[0] == '\0') { *pp_path_out = talloc_strdup(ctx, ""); if (!*pp_path_out) { return NT_STATUS_NO_MEMORY; @@ -895,8 +911,7 @@ NTSTATUS dfs_redirect(TALLOC_CTX *ctx, path and return OK */ if (!lp_msdfs_root(SNUM(conn))) { - *pp_path_out = talloc_strdup(ctx, pdp->reqpath); - TALLOC_FREE(pdp); + *pp_path_out = talloc_strdup(ctx, reqpath); if (!*pp_path_out) { return NT_STATUS_NO_MEMORY; } @@ -906,30 +921,27 @@ NTSTATUS dfs_redirect(TALLOC_CTX *ctx, /* If it looked like a local path (zero hostname/servicename) * just treat as a tcon-relative path. */ - if (pdp->hostname[0] == '\0' && pdp->servicename[0] == '\0') { - *pp_path_out = talloc_strdup(ctx, pdp->reqpath); - TALLOC_FREE(pdp); + if (hostname[0] == '\0' && servicename[0] == '\0') { + *pp_path_out = talloc_strdup(ctx, reqpath); if (!*pp_path_out) { return NT_STATUS_NO_MEMORY; } return NT_STATUS_OK; } - if (!( strequal(pdp->servicename, lp_servicename(talloc_tos(), lp_sub, SNUM(conn))) - || (strequal(pdp->servicename, HOMES_NAME) + if (!( strequal(servicename, lp_servicename(talloc_tos(), lp_sub, SNUM(conn))) + || (strequal(servicename, HOMES_NAME) && strequal(lp_servicename(talloc_tos(), lp_sub, SNUM(conn)), conn->session_info->unix_info->sanitized_username) )) ) { /* The given sharename doesn't match this connection. */ - TALLOC_FREE(pdp); - return NT_STATUS_OBJECT_PATH_NOT_FOUND; } status = dfs_path_lookup(ctx, conn, path_in, - pdp, + reqpath, ucf_flags, _twrp, /* twrp. */ NULL, /* size_t *consumedcntp */ @@ -949,8 +961,7 @@ NTSTATUS dfs_redirect(TALLOC_CTX *ctx, DEBUG(3,("dfs_redirect: Not redirecting %s.\n", path_in)); /* Form non-dfs tcon-relative path */ - *pp_path_out = talloc_strdup(ctx, pdp->reqpath); - TALLOC_FREE(pdp); + *pp_path_out = talloc_strdup(ctx, reqpath); if (!*pp_path_out) { return NT_STATUS_NO_MEMORY; } @@ -1013,14 +1024,10 @@ NTSTATUS get_referred_path(TALLOC_CTX *ctx, loadparm_s3_global_substitution(); struct conn_struct_tos *c = NULL; struct connection_struct *conn = NULL; + char *servicename = NULL; + char *reqpath = NULL; int snum; NTSTATUS status = NT_STATUS_NOT_FOUND; - struct dfs_path *pdp = talloc_zero(frame, struct dfs_path); - - if (!pdp) { - TALLOC_FREE(frame); - return NT_STATUS_NO_MEMORY; - } *self_referralp = False; @@ -1028,14 +1035,16 @@ NTSTATUS get_referred_path(TALLOC_CTX *ctx, NULL, dfs_path, allow_broken_path, - pdp); + NULL, + &servicename, + &reqpath); if (!NT_STATUS_IS_OK(status)) { TALLOC_FREE(frame); return status; } - jucn->service_name = talloc_strdup(ctx, pdp->servicename); - jucn->volume_name = talloc_strdup(ctx, pdp->reqpath); + jucn->service_name = talloc_strdup(ctx, servicename); + jucn->volume_name = talloc_strdup(ctx, reqpath); if (!jucn->service_name || !jucn->volume_name) { TALLOC_FREE(frame); return NT_STATUS_NO_MEMORY; @@ -1064,7 +1073,7 @@ NTSTATUS get_referred_path(TALLOC_CTX *ctx, if (!lp_msdfs_root(snum) && (*lp_msdfs_proxy(talloc_tos(), lp_sub, snum) == '\0')) { DEBUG(3,("get_referred_path: |%s| in dfs path %s is not " "a dfs root.\n", - pdp->servicename, dfs_path)); + servicename, dfs_path)); TALLOC_FREE(frame); return NT_STATUS_NOT_FOUND; } @@ -1077,7 +1086,7 @@ NTSTATUS get_referred_path(TALLOC_CTX *ctx, * user). Cope with this. */ - if (pdp->reqpath[0] == '\0') { + if (reqpath[0] == '\0') { char *tmp; struct referral *ref; size_t refcount; @@ -1158,7 +1167,7 @@ NTSTATUS get_referred_path(TALLOC_CTX *ctx, status = dfs_path_lookup(ctx, conn, dfs_path, - pdp, + reqpath, 0, /* ucf_flags */ NULL, consumedcntp, @@ -1271,45 +1280,43 @@ bool create_junction(TALLOC_CTX *ctx, const struct loadparm_substitution *lp_sub = loadparm_s3_global_substitution(); int snum; - struct dfs_path *pdp = talloc(ctx,struct dfs_path); + char *hostname = NULL; + char *servicename = NULL; + char *reqpath = NULL; NTSTATUS status; - if (!pdp) { - return False; - } status = parse_dfs_path(ctx, NULL, dfs_path, allow_broken_path, - pdp); + &hostname, + &servicename, + &reqpath); if (!NT_STATUS_IS_OK(status)) { return False; } /* check if path is dfs : validate first token */ - if (!is_myname_or_ipaddr(pdp->hostname)) { + if (!is_myname_or_ipaddr(hostname)) { DEBUG(4,("create_junction: Invalid hostname %s " "in dfs path %s\n", - pdp->hostname, dfs_path)); - TALLOC_FREE(pdp); + hostname, dfs_path)); return False; } /* Check for a non-DFS share */ - snum = lp_servicenumber(pdp->servicename); + snum = lp_servicenumber(servicename); if(snum < 0 || !lp_msdfs_root(snum)) { DEBUG(4,("create_junction: %s is not an msdfs root.\n", - pdp->servicename)); - TALLOC_FREE(pdp); + servicename)); return False; } - jucn->service_name = talloc_strdup(ctx, pdp->servicename); - jucn->volume_name = talloc_strdup(ctx, pdp->reqpath); + jucn->service_name = talloc_strdup(ctx, servicename); + jucn->volume_name = talloc_strdup(ctx, reqpath); jucn->comment = lp_comment(ctx, lp_sub, snum); - TALLOC_FREE(pdp); if (!jucn->service_name || !jucn->volume_name || ! jucn->comment) { return False; } -- 2.34.1 From d193d0ee396c15b2b5ac3f810c44d3149fb62012 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Mon, 8 Aug 2022 10:27:16 -0700 Subject: [PATCH 17/55] s3: smbd: Remove definition of struct dfs_path. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15144 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme BUG: https://bugzilla.samba.org/show_bug.cgi?id=15146 (cherry picked from commit 6c83c674bab8e57ecaf6271eb3a403171bbbacca) --- source3/include/msdfs.h | 7 ------- 1 file changed, 7 deletions(-) diff --git a/source3/include/msdfs.h b/source3/include/msdfs.h index 83265174373c..892343fdd9af 100644 --- a/source3/include/msdfs.h +++ b/source3/include/msdfs.h @@ -56,11 +56,4 @@ struct junction_map { size_t referral_count; struct referral* referral_list; }; - -struct dfs_path { - char *hostname; - char *servicename; - char *reqpath; -}; - #endif /* _MSDFS_H */ -- 2.34.1 From ca362045a1d170ef4d0f698e8adef9eff9e26c42 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Tue, 9 Aug 2022 10:49:46 -0700 Subject: [PATCH 18/55] s3: smbd: Add helper function msdfs_servicename_matches_connection(). Not yet used so commented out. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15144 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme BUG: https://bugzilla.samba.org/show_bug.cgi?id=15146 (cherry picked from commit 4f5d02f8c0efc1520b2113ce656c78483deb7826) --- source3/smbd/msdfs.c | 46 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/source3/smbd/msdfs.c b/source3/smbd/msdfs.c index 21288aaf664a..463601cf1ee7 100644 --- a/source3/smbd/msdfs.c +++ b/source3/smbd/msdfs.c @@ -36,6 +36,52 @@ #include "lib/global_contexts.h" #include "source3/lib/substitute.h" +#if 0 +/********************************************************************** + Function to determine if a given sharename matches a connection. +**********************************************************************/ + +static bool msdfs_servicename_matches_connection(struct connection_struct *conn, + const char *servicename, + const char *vfs_user) +{ + const struct loadparm_substitution *lp_sub = + loadparm_s3_global_substitution(); + char *conn_servicename = NULL; + int snum; + bool match = false; + + if (conn == NULL) { + /* No connection always matches. */ + return true; + } + + snum = SNUM(conn); + + conn_servicename = lp_servicename(talloc_tos(), lp_sub, snum); + if (conn_servicename == NULL) { + DBG_ERR("lp_servicename() failed, OOM!\n"); + return false; + } + + if (strequal(servicename, conn_servicename)) { + match = true; + goto done; + } + if (strequal(servicename, HOMES_NAME)) { + match = true; + goto done; + } + if (strequal(vfs_user, conn_servicename)) { + match = true; + goto done; + } +done: + TALLOC_FREE(conn_servicename); + return match; +} +#endif + /********************************************************************** Parse a DFS pathname of the form /hostname/service/reqpath into the dfs_path structure. -- 2.34.1 From 635fe2af28c0ac3d8a76b34606590b91bebb09ef Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Tue, 9 Aug 2022 10:53:18 -0700 Subject: [PATCH 19/55] s3: smbd: Use helper function msdfs_servicename_matches_connection() in parse_dfs_path(). Replaces ugly complex logic. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15144 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme BUG: https://bugzilla.samba.org/show_bug.cgi?id=15146 (cherry picked from commit c0a1d7c7a8a7f24890e60c7a371498949dec11c2) --- source3/smbd/msdfs.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/source3/smbd/msdfs.c b/source3/smbd/msdfs.c index 463601cf1ee7..a48ae642ae8b 100644 --- a/source3/smbd/msdfs.c +++ b/source3/smbd/msdfs.c @@ -36,7 +36,6 @@ #include "lib/global_contexts.h" #include "source3/lib/substitute.h" -#if 0 /********************************************************************** Function to determine if a given sharename matches a connection. **********************************************************************/ @@ -80,7 +79,6 @@ done: TALLOC_FREE(conn_servicename); return match; } -#endif /********************************************************************** Parse a DFS pathname of the form /hostname/service/reqpath @@ -112,14 +110,13 @@ static NTSTATUS parse_dfs_path(TALLOC_CTX *ctx, char **_servicename, char **_remaining_path) { - const struct loadparm_substitution *lp_sub = - loadparm_s3_global_substitution(); char *hostname = NULL; char *pathname_local = NULL; char *p = NULL; char *servicename = NULL; char *reqpath = NULL; char *eos_ptr = NULL; + bool servicename_matches = false; pathname_local = talloc_strdup(ctx, pathname); if (pathname_local == NULL) { @@ -196,10 +193,12 @@ static NTSTATUS parse_dfs_path(TALLOC_CTX *ctx, } /* Is this really our servicename ? */ - if (conn && !( strequal(servicename, lp_servicename(talloc_tos(), lp_sub, SNUM(conn))) - || (strequal(servicename, HOMES_NAME) - && strequal(lp_servicename(talloc_tos(), lp_sub, SNUM(conn)), - get_current_username()) )) ) { + servicename_matches = msdfs_servicename_matches_connection( + conn, + servicename, + get_current_username()); + + if (!servicename_matches) { DBG_ERR("%s is not our servicename\n", servicename); /* -- 2.34.1 From 4773c736454805056a39d8ae11102f66641495f3 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Tue, 9 Aug 2022 10:58:24 -0700 Subject: [PATCH 20/55] s3: smbd: Use helper function msdfs_servicename_matches_connection() in dfs_redirect(). Replaces ugly complex logic. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15144 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme BUG: https://bugzilla.samba.org/show_bug.cgi?id=15146 (cherry picked from commit a3c9eb7931cb4da0dd5bc5d600125979dd1a7df5) --- source3/smbd/msdfs.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/source3/smbd/msdfs.c b/source3/smbd/msdfs.c index a48ae642ae8b..fce887de9aa7 100644 --- a/source3/smbd/msdfs.c +++ b/source3/smbd/msdfs.c @@ -925,11 +925,10 @@ NTSTATUS dfs_redirect(TALLOC_CTX *ctx, NTTIME *_twrp, char **pp_path_out) { - const struct loadparm_substitution *lp_sub = - loadparm_s3_global_substitution(); char *hostname = NULL; char *servicename = NULL; char *reqpath = NULL; + bool servicename_matches = false; NTSTATUS status; status = parse_dfs_path(ctx, @@ -974,11 +973,11 @@ NTSTATUS dfs_redirect(TALLOC_CTX *ctx, return NT_STATUS_OK; } - if (!( strequal(servicename, lp_servicename(talloc_tos(), lp_sub, SNUM(conn))) - || (strequal(servicename, HOMES_NAME) - && strequal(lp_servicename(talloc_tos(), lp_sub, SNUM(conn)), - conn->session_info->unix_info->sanitized_username) )) ) { - + servicename_matches = msdfs_servicename_matches_connection( + conn, + servicename, + conn->session_info->unix_info->sanitized_username); + if (!servicename_matches) { /* The given sharename doesn't match this connection. */ return NT_STATUS_OBJECT_PATH_NOT_FOUND; } -- 2.34.1 From ba9077c0b97cc941bdc0d4c301f9dd1f8511ddca Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Mon, 8 Aug 2022 11:16:17 -0700 Subject: [PATCH 21/55] s3: smbd: Add dfs_filename_convert(). Simple wrapper around parse_dfs_path(). Not yet used. This is what we will use to replace dfs_redirect() in the filename conversion code. Keep as a wrapper for now as we might want to add some error checking around the 'hostname' and 'service' returns. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15144 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme BUG: https://bugzilla.samba.org/show_bug.cgi?id=15146 (cherry picked from commit 245d07ab84852b829c029496618e56782d070e83) --- source3/smbd/msdfs.c | 62 ++++++++++++++++++++++++++++++++++++++++++++ source3/smbd/proto.h | 5 ++++ 2 files changed, 67 insertions(+) diff --git a/source3/smbd/msdfs.c b/source3/smbd/msdfs.c index fce887de9aa7..1b66911faf07 100644 --- a/source3/smbd/msdfs.c +++ b/source3/smbd/msdfs.c @@ -902,6 +902,68 @@ static NTSTATUS dfs_path_lookup(TALLOC_CTX *ctx, return status; } +/***************************************************************** + Decides if a dfs pathname should be redirected or not. + If not, the pathname is converted to a tcon-relative local unix path + This is now a simple wrapper around parse_dfs_path() + as it does all the required checks. +*****************************************************************/ + +NTSTATUS dfs_filename_convert(TALLOC_CTX *ctx, + connection_struct *conn, + uint32_t ucf_flags, + const char *dfs_path_in, + char **pp_path_out) +{ + char *hostname = NULL; + char *servicename = NULL; + char *reqpath = NULL; + NTSTATUS status; + + status = parse_dfs_path(ctx, + conn, + dfs_path_in, + !conn->sconn->using_smb2, + &hostname, + &servicename, + &reqpath); + if (!NT_STATUS_IS_OK(status)) { + return status; + } + + /* + * Caller doesn't care about hostname + * or servicename. + */ + TALLOC_FREE(hostname); + TALLOC_FREE(servicename); + + /* + * If parse_dfs_path fell back to a local path + * after skipping hostname or servicename, ensure + * we still have called check_path_syntax() + * on the full returned local path. check_path_syntax() + * is idempotent so this is safe. + */ + if (ucf_flags & UCF_POSIX_PATHNAMES) { + status = check_path_syntax_posix(reqpath); + } else { + status = check_path_syntax(reqpath); + } + if (!NT_STATUS_IS_OK(status)) { + return status; + } + /* + * Previous (and current logic) just ignores + * the server, share components if a DFS + * path is sent on a non-DFS share except to + * check that they match an existing share. Should + * we tighten this up to return an error here ? + */ + *pp_path_out = reqpath; + return NT_STATUS_OK; +} + /***************************************************************** Decides if a dfs pathname should be redirected or not. If not, the pathname is converted to a tcon-relative local unix path diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h index 33fc222311eb..1c10849346eb 100644 --- a/source3/smbd/proto.h +++ b/source3/smbd/proto.h @@ -560,6 +560,11 @@ bool remove_msdfs_link(const struct junction_map *jucn, struct junction_map *enum_msdfs_links(TALLOC_CTX *ctx, struct auth_session_info *session_info, size_t *p_num_jn); +NTSTATUS dfs_filename_convert(TALLOC_CTX *ctx, + connection_struct *conn, + uint32_t ucf_flags, + const char *dfs_path_in, + char **pp_path_out); NTSTATUS dfs_redirect(TALLOC_CTX *ctx, connection_struct *conn, const char *name_in, -- 2.34.1 From 6d88f14fef24ec9a22703130142cc20a60860f97 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 11 Aug 2022 23:55:58 -0700 Subject: [PATCH 22/55] s3: smbd: In get referred_path(), make sure check_path_syntax() is called on returned reqpath. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15144 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme BUG: https://bugzilla.samba.org/show_bug.cgi?id=15146 (cherry picked from commit da625e4ab4bc670e44fcb6ad7456aa64d0f1f9d2) --- source3/smbd/msdfs.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/source3/smbd/msdfs.c b/source3/smbd/msdfs.c index 1b66911faf07..19160b03c3c6 100644 --- a/source3/smbd/msdfs.c +++ b/source3/smbd/msdfs.c @@ -1149,6 +1149,13 @@ NTSTATUS get_referred_path(TALLOC_CTX *ctx, return status; } + /* Path referrals are always non-POSIX. */ + status = check_path_syntax(reqpath); + if (!NT_STATUS_IS_OK(status)) { + TALLOC_FREE(frame); + return status; + } + jucn->service_name = talloc_strdup(ctx, servicename); jucn->volume_name = talloc_strdup(ctx, reqpath); if (!jucn->service_name || !jucn->volume_name) { -- 2.34.1 From 1485429af30558b1f7d9c3bbe1c96e89d0d79d56 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 11 Aug 2022 23:57:51 -0700 Subject: [PATCH 23/55] s3: smbd: In get create_junction(), make sure check_path_syntax() is called on returned reqpath. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15144 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme BUG: https://bugzilla.samba.org/show_bug.cgi?id=15146 (cherry picked from commit a92f4f7af0eaa035deebfb1c930ca0cc12d992d5) --- source3/smbd/msdfs.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/source3/smbd/msdfs.c b/source3/smbd/msdfs.c index 19160b03c3c6..5a2413ed8b97 100644 --- a/source3/smbd/msdfs.c +++ b/source3/smbd/msdfs.c @@ -1426,6 +1426,12 @@ bool create_junction(TALLOC_CTX *ctx, return False; } + /* Junction create paths are always non-POSIX. */ + status = check_path_syntax(reqpath); + if (!NT_STATUS_IS_OK(status)) { + return false; + } + jucn->service_name = talloc_strdup(ctx, servicename); jucn->volume_name = talloc_strdup(ctx, reqpath); jucn->comment = lp_comment(ctx, lp_sub, snum); -- 2.34.1 From 7d89710ba6b09773d3e61d19b2085c5c917d2d5e Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Mon, 8 Aug 2022 11:31:39 -0700 Subject: [PATCH 24/55] s3: smbd: Allow openat_pathref_dirfsp_nosymlink() to return NT_STATUS_PATH_NOT_COVERED for a DFS link on a DFS share. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15144 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme BUG: https://bugzilla.samba.org/show_bug.cgi?id=15146 (cherry picked from commit b5f6809593524e7e9aca1c09ff379e02a1cde61b) --- source3/smbd/files.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/source3/smbd/files.c b/source3/smbd/files.c index 1cd146dedc71..72d1bc7d5551 100644 --- a/source3/smbd/files.c +++ b/source3/smbd/files.c @@ -856,6 +856,18 @@ next: *unparsed = len - parsed; } } + /* + * If we're on an MSDFS share, see if this is + * an MSDFS link. + */ + if (lp_host_msdfs() && + lp_msdfs_root(SNUM(conn)) && + (substitute != NULL) && + strnequal(*substitute, "msdfs:", 6) && + is_msdfs_link(dirfsp, &rel_fname)) + { + status = NT_STATUS_PATH_NOT_COVERED; + } } else { DBG_DEBUG("readlink_talloc failed: %s\n", -- 2.34.1 From 824310c2b53f4f5c43d2012646d4c0e261f06d70 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Mon, 8 Aug 2022 13:15:17 -0700 Subject: [PATCH 25/55] s3: smbd: In filename_convert_dirfsp_nosymlink(), allow a NT_STATUS_PATH_NOT_COVERED error to be returned. openat_pathref_dirfsp_nosymlink() can now return NT_STATUS_PATH_NOT_COVERED. Don't convert this automatically into NT_STATUS_OBJECT_PATH_NOT_FOUND. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15144 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme BUG: https://bugzilla.samba.org/show_bug.cgi?id=15146 (cherry picked from commit 07ef9e3029b8cca1b92d900d6ed684ca0ac6afe4) --- source3/smbd/filename.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c index 87abc8be3768..411440973627 100644 --- a/source3/smbd/filename.c +++ b/source3/smbd/filename.c @@ -2549,6 +2549,11 @@ static NTSTATUS filename_convert_dirfsp_nosymlink( nt_errstr(status)); TALLOC_FREE(dirname); + if (NT_STATUS_EQUAL(status, NT_STATUS_PATH_NOT_COVERED)) { + /* MS-DFS error must propagate back out. */ + goto fail; + } + if (!NT_STATUS_EQUAL(status, NT_STATUS_ACCESS_DENIED)) { /* * Except ACCESS_DENIED, everything else leads -- 2.34.1 From 584471a5f1f096ac7cc3e27343446cac89568c4f Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Mon, 8 Aug 2022 13:18:56 -0700 Subject: [PATCH 26/55] s3: smbd: In filename_convert_dirfsp_nosymlink(), cope with an MS-DFS link as the terminal component. If the terminal component was an MSDFS link, openat_pathref_fsp_case_insensitive() will return NT_STATUS_OBJECT_NAME_NOT_FOUND with a VALID_STAT of a symlink. If this is the case, check if we actually found a terminal MS-DFS link at the end of the pathname and return NT_STATUS_PATH_NOT_COVERED. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15144 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme BUG: https://bugzilla.samba.org/show_bug.cgi?id=15146 (cherry picked from commit d80bedc3c418b6839b1bde78ba8d3db06611be2a) --- source3/smbd/filename.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c index 411440973627..3e7909a48c8b 100644 --- a/source3/smbd/filename.c +++ b/source3/smbd/filename.c @@ -2635,6 +2635,19 @@ static NTSTATUS filename_convert_dirfsp_nosymlink( char *normalized = NULL; if (VALID_STAT(smb_fname_rel->st)) { + /* + * If we're on an MSDFS share, see if this is + * an MSDFS link. + */ + if (lp_host_msdfs() && + lp_msdfs_root(SNUM(conn)) && + S_ISLNK(smb_fname_rel->st.st_ex_mode) && + is_msdfs_link(smb_dirname->fsp, smb_fname_rel)) + { + status = NT_STATUS_PATH_NOT_COVERED; + goto fail; + } + #if defined(WITH_SMB1SERVER) /* * In SMB1 posix mode, if this is a symlink, -- 2.34.1 From d2b5ef3783aca6f11e925131efe0defbed150333 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 10 Aug 2022 11:29:33 -0700 Subject: [PATCH 27/55] s3: smbd: Remove call to dfs_redirect() from filename_convert_smb1_search_path(). Use dfs_filename_convert() instead. Code is now much simpler. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15144 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme BUG: https://bugzilla.samba.org/show_bug.cgi?id=15146 (cherry picked from commit fcf19d91c09edc6dfbf5bd7cbeedcd641030eb31) --- source3/smbd/filename.c | 68 ++++++++--------------------------------- 1 file changed, 12 insertions(+), 56 deletions(-) diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c index 3e7909a48c8b..cba012acc3e0 100644 --- a/source3/smbd/filename.c +++ b/source3/smbd/filename.c @@ -1935,67 +1935,23 @@ NTSTATUS filename_convert_smb1_search_path(TALLOC_CTX *ctx, /* * We've been given a raw DFS pathname. */ - char *fname = NULL; - char *name_in_copy = NULL; - char *last_component = NULL; - - /* Work on a copy of name_in. */ - name_in_copy = talloc_strdup(ctx, name_in); - if (name_in_copy == NULL) { - return NT_STATUS_NO_MEMORY; - } - - /* - * Now we know that the last component is the - * wildcard. Copy it and truncate to remove it. - */ - p = strrchr(name_in_copy, '/'); - if (p == NULL) { - last_component = talloc_strdup(ctx, name_in_copy); - name_in_copy[0] = '\0'; - } else { - last_component = talloc_strdup(ctx, p+1); - *p = '\0'; - } - if (last_component == NULL) { - return NT_STATUS_NO_MEMORY; - } - - DBG_DEBUG("name_in_copy: %s\n", name_in_copy); - - /* - * Now we can call dfs_redirect() - * on the name without wildcard. - */ - status = dfs_redirect(ctx, - conn, - name_in_copy, - ucf_flags, - !conn->sconn->using_smb2, - NULL, - &fname); - if (!NT_STATUS_IS_OK(status)) { - DBG_DEBUG("dfs_redirect " + char *pathname = NULL; + DBG_DEBUG("Before dfs_filename_convert name_in: %s\n", name_in); + status = dfs_filename_convert(ctx, + conn, + ucf_flags, + name_in, + &pathname); + if (!NT_STATUS_IS_OK(status)) { + DBG_DEBUG("dfs_filename_convert " "failed for name %s with %s\n", - name_in_copy, + name_in, nt_errstr(status)); return status; } - /* Add the last component back. */ - if (fname[0] == '\0') { - name_in = talloc_strdup(ctx, last_component); - } else { - name_in = talloc_asprintf(ctx, - "%s/%s", - fname, - last_component); - } - if (name_in == NULL) { - return NT_STATUS_NO_MEMORY; - } ucf_flags &= ~UCF_DFS_PATHNAME; - - DBG_DEBUG("After DFS redirect name_in: %s\n", name_in); + name_in = pathname; + DBG_DEBUG("After dfs_filename_convert name_in: %s\n", name_in); } /* Get the original lcomp. */ -- 2.34.1 From 514dc74415aa484bb49ba51e32502d5678a4eba8 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 10 Aug 2022 11:32:30 -0700 Subject: [PATCH 28/55] s3: smbd: Remove call to dfs_redirect() from filename_convert_dirfsp_nosymlink(). Use dfs_filename_convert() instead. There are now no more callers of dfs_redirect(). BUG: https://bugzilla.samba.org/show_bug.cgi?id=15144 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme BUG: https://bugzilla.samba.org/show_bug.cgi?id=15146 (cherry picked from commit d20b60c3200b5e1881cdf4b59da154d1af7e3994) --- source3/smbd/filename.c | 28 +++++++++++++--------------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c index cba012acc3e0..b64fb908f16c 100644 --- a/source3/smbd/filename.c +++ b/source3/smbd/filename.c @@ -2384,28 +2384,26 @@ static NTSTATUS filename_convert_dirfsp_nosymlink( NTSTATUS status = NT_STATUS_UNSUCCESSFUL; if (ucf_flags & UCF_DFS_PATHNAME) { - char *fname = NULL; - NTTIME dfs_twrp = 0; - status = dfs_redirect( - mem_ctx, - conn, - name_in, - ucf_flags, - !conn->sconn->using_smb2, - &dfs_twrp, - &fname); + /* + * We've been given a raw DFS pathname. + */ + char *pathname = NULL; + DBG_DEBUG("Before dfs_filename_convert name_in: %s\n", name_in); + status = dfs_filename_convert(mem_ctx, + conn, + ucf_flags, + name_in, + &pathname); if (!NT_STATUS_IS_OK(status)) { - DBG_DEBUG("dfs_redirect " + DBG_DEBUG("dfs_filename_convert " "failed for name %s with %s\n", name_in, nt_errstr(status)); return status; } - name_in = fname; ucf_flags &= ~UCF_DFS_PATHNAME; - if (twrp == 0 && dfs_twrp != 0) { - twrp = dfs_twrp; - } + name_in = pathname; + DBG_DEBUG("After dfs_filename_convert name_in: %s\n", name_in); } if (is_fake_file_path(name_in)) { -- 2.34.1 From 0438cf94eb9235ee9aefd0282be9d78d22e15380 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 10 Aug 2022 11:34:24 -0700 Subject: [PATCH 29/55] s3: smbd: Remove dfs_redirect(). A moment of silence please. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15144 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme BUG: https://bugzilla.samba.org/show_bug.cgi?id=15146 (cherry picked from commit 6b1224b22012b54b1ae20b682daf61c877362a7b) --- source3/smbd/msdfs.c | 115 ------------------------------------------- source3/smbd/proto.h | 7 --- 2 files changed, 122 deletions(-) diff --git a/source3/smbd/msdfs.c b/source3/smbd/msdfs.c index 5a2413ed8b97..d48c5d6c906a 100644 --- a/source3/smbd/msdfs.c +++ b/source3/smbd/msdfs.c @@ -964,121 +964,6 @@ NTSTATUS dfs_filename_convert(TALLOC_CTX *ctx, return NT_STATUS_OK; } -/***************************************************************** - Decides if a dfs pathname should be redirected or not. - If not, the pathname is converted to a tcon-relative local unix path - - search_wcard_flag: this flag performs 2 functions both related - to searches. See resolve_dfs_path() and parse_dfs_path_XX() - for details. - - This function can return NT_STATUS_OK, meaning use the returned path as-is - (mapped into a local path). - or NT_STATUS_NOT_COVERED meaning return a DFS redirect, or - any other NT_STATUS error which is a genuine error to be - returned to the client. -*****************************************************************/ - -NTSTATUS dfs_redirect(TALLOC_CTX *ctx, - connection_struct *conn, - const char *path_in, - uint32_t ucf_flags, - bool allow_broken_path, - NTTIME *_twrp, - char **pp_path_out) -{ - char *hostname = NULL; - char *servicename = NULL; - char *reqpath = NULL; - bool servicename_matches = false; - NTSTATUS status; - - status = parse_dfs_path(ctx, - conn, - path_in, - allow_broken_path, - &hostname, - &servicename, - &reqpath); - if (!NT_STATUS_IS_OK(status)) { - return status; - } - - if (reqpath[0] == '\0') { - *pp_path_out = talloc_strdup(ctx, ""); - if (!*pp_path_out) { - return NT_STATUS_NO_MEMORY; - } - DEBUG(5,("dfs_redirect: self-referral.\n")); - return NT_STATUS_OK; - } - - /* If dfs pathname for a non-dfs share, convert to tcon-relative - path and return OK */ - - if (!lp_msdfs_root(SNUM(conn))) { - *pp_path_out = talloc_strdup(ctx, reqpath); - if (!*pp_path_out) { - return NT_STATUS_NO_MEMORY; - } - return NT_STATUS_OK; - } - - /* If it looked like a local path (zero hostname/servicename) - * just treat as a tcon-relative path. */ - - if (hostname[0] == '\0' && servicename[0] == '\0') { - *pp_path_out = talloc_strdup(ctx, reqpath); - if (!*pp_path_out) { - return NT_STATUS_NO_MEMORY; - } - return NT_STATUS_OK; - } - - servicename_matches = msdfs_servicename_matches_connection( - conn, - servicename, - conn->session_info->unix_info->sanitized_username); - if (!servicename_matches) { - /* The given sharename doesn't match this connection. */ - return NT_STATUS_OBJECT_PATH_NOT_FOUND; - } - - status = dfs_path_lookup(ctx, - conn, - path_in, - reqpath, - ucf_flags, - _twrp, /* twrp. */ - NULL, /* size_t *consumedcntp */ - NULL, /* struct referral **ppreflist */ - NULL); /* size_t *preferral_count */ - if (!NT_STATUS_IS_OK(status)) { - if (NT_STATUS_EQUAL(status, NT_STATUS_PATH_NOT_COVERED)) { - DEBUG(3,("dfs_redirect: Redirecting %s\n", path_in)); - } else { - DEBUG(10,("dfs_redirect: dfs_path_lookup " - "failed for %s with %s\n", - path_in, nt_errstr(status) )); - } - return status; - } - - DEBUG(3,("dfs_redirect: Not redirecting %s.\n", path_in)); - - /* Form non-dfs tcon-relative path */ - *pp_path_out = talloc_strdup(ctx, reqpath); - if (!*pp_path_out) { - return NT_STATUS_NO_MEMORY; - } - - DEBUG(3,("dfs_redirect: Path %s converted to non-dfs path %s\n", - path_in, - *pp_path_out)); - - return NT_STATUS_OK; -} - /********************************************************************** Return a self referral. **********************************************************************/ diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h index 1c10849346eb..bc305bce2966 100644 --- a/source3/smbd/proto.h +++ b/source3/smbd/proto.h @@ -565,13 +565,6 @@ NTSTATUS dfs_filename_convert(TALLOC_CTX *ctx, uint32_t ucf_flags, const char *dfs_path_in, char **pp_path_out); -NTSTATUS dfs_redirect(TALLOC_CTX *ctx, - connection_struct *conn, - const char *name_in, - uint32_t ucf_flags, - bool allow_broken_path, - NTTIME *twrp, - char **pp_name_out); struct connection_struct; struct smb_filename; -- 2.34.1 From deb94145e2e3aad36361cc93b550de798cd7e737 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Tue, 9 Aug 2022 12:07:30 -0700 Subject: [PATCH 30/55] s3: smbd: Add new version of dfs_path_lookup() that uses filename_convert_dirfsp(). Commented out as not yet used but it's easier to see the new logic this way. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15144 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme BUG: https://bugzilla.samba.org/show_bug.cgi?id=15146 (cherry picked from commit 22d4f62537199d9454be312a546e251f04022497) --- source3/smbd/msdfs.c | 261 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 261 insertions(+) diff --git a/source3/smbd/msdfs.c b/source3/smbd/msdfs.c index d48c5d6c906a..ab7f9f0ad788 100644 --- a/source3/smbd/msdfs.c +++ b/source3/smbd/msdfs.c @@ -713,6 +713,267 @@ bool is_msdfs_link(struct files_struct *dirfsp, return (NT_STATUS_IS_OK(status)); } +#if 0 +/***************************************************************** + Used by other functions to decide if a dfs path is remote, + and to get the list of referred locations for that remote path. + + consumedcntp: how much of the dfs path is being redirected. the client + should try the remaining path on the redirected server. +*****************************************************************/ + +static NTSTATUS dfs_path_lookup(TALLOC_CTX *ctx, + connection_struct *conn, + const char *dfspath, /* Incoming complete dfs path */ + const char *reqpath, /* Parsed out remaining path. */ + uint32_t ucf_flags, + size_t *consumedcntp, + struct referral **ppreflist, + size_t *preferral_count) +{ + NTSTATUS status; + struct smb_filename *parent_smb_fname = NULL; + struct smb_filename *smb_fname_rel = NULL; + NTTIME twrp = 0; + char *local_pathname = NULL; + char *last_component = NULL; + char *atname = NULL; + size_t removed_components = 0; + bool posix = (ucf_flags & UCF_POSIX_PATHNAMES); + char *p = NULL; + char *canon_dfspath = NULL; + + DBG_DEBUG("Conn path = %s reqpath = %s\n", conn->connectpath, reqpath); + + local_pathname = talloc_strdup(ctx, reqpath); + if (local_pathname == NULL) { + status = NT_STATUS_NO_MEMORY; + goto out; + } + + /* We know reqpath isn't a DFS path. */ + ucf_flags &= ~UCF_DFS_PATHNAME; + + if (ucf_flags & UCF_GMT_PATHNAME) { + extract_snapshot_token(local_pathname, &twrp); + ucf_flags &= ~UCF_GMT_PATHNAME; + } + + /* + * We should have been given a DFS path to resolve. + * This should return NT_STATUS_PATH_NOT_COVERED. + * + * Do a pathname walk, stripping off components + * until we get NT_STATUS_OK instead of + * NT_STATUS_PATH_NOT_COVERED. + * + * Fail on any other error. + */ + + for (;;) { + TALLOC_CTX *frame = NULL; + struct files_struct *dirfsp = NULL; + struct smb_filename *smb_fname_walk = NULL; + + TALLOC_FREE(parent_smb_fname); + + /* + * Use a local stackframe as filename_convert_dirfsp() + * opens handles on the last two components in the path. + * Allow these to be freed as we step back through + * the local_pathname. + */ + frame = talloc_stackframe(); + status = filename_convert_dirfsp(frame, + conn, + local_pathname, + ucf_flags, + twrp, + &dirfsp, + &smb_fname_walk); + /* If we got a name, save it. */ + if (smb_fname_walk != NULL) { + parent_smb_fname = talloc_move(ctx, &smb_fname_walk); + } + TALLOC_FREE(frame); + + if (!NT_STATUS_EQUAL(status, NT_STATUS_PATH_NOT_COVERED)) { + /* + * For any other status than NT_STATUS_PATH_NOT_COVERED + * (including NT_STATUS_OK) we exit the walk. + * If it's an error we catch it outside the loop. + */ + break; + } + + /* Step back one component and save it off as last_component. */ + TALLOC_FREE(last_component); + p = strrchr(local_pathname, '/'); + if (p == NULL) { + /* + * We removed all components. + * Go around once more to make + * sure we can open the root '\0'. + */ + last_component = talloc_strdup(ctx, local_pathname); + *local_pathname = '\0'; + } else { + last_component = talloc_strdup(ctx, p+1); + *p = '\0'; + } + if (last_component == NULL) { + status = NT_STATUS_NO_MEMORY; + goto out; + } + /* Integer wrap check. */ + if (removed_components + 1 < removed_components) { + status = NT_STATUS_INVALID_PARAMETER; + goto out; + } + removed_components++; + } + + if (!NT_STATUS_IS_OK(status)) { + DBG_DEBUG("dfspath = %s. reqpath = %s. Error %s.\n", + dfspath, + reqpath, + nt_errstr(status)); + goto out; + } + + if (parent_smb_fname->fsp == NULL) { + /* Unable to open parent. */ + DBG_DEBUG("dfspath = %s. reqpath = %s. " + "Unable to open parent directory (%s).\n", + dfspath, + reqpath, + smb_fname_str_dbg(parent_smb_fname)); + status = NT_STATUS_OBJECT_PATH_NOT_FOUND; + goto out; + } + + if (removed_components == 0) { + /* + * We never got NT_STATUS_PATH_NOT_COVERED. + * There was no DFS redirect. + */ + DBG_DEBUG("dfspath = %s. reqpath = %s. " + "No removed components.\n", + dfspath, + reqpath); + status = NT_STATUS_OBJECT_PATH_NOT_FOUND; + goto out; + } + + /* + * One of the removed_components was the MSDFS link + * at the end. We need to count this in the resolved + * path below, so remove one from removed_components. + */ + removed_components--; + + /* + * Now parent_smb_fname->fsp is the parent directory dirfsp, + * last_component is the untranslated MS-DFS link name. + * Search for it in the parent directory to get the real + * filename on disk. + */ + status = get_real_filename_at(parent_smb_fname->fsp, + last_component, + ctx, + &atname); + + if (!NT_STATUS_IS_OK(status)) { + DBG_DEBUG("dfspath = %s. reqpath = %s " + "get_real_filename_at(%s, %s) error (%s)\n", + dfspath, + reqpath, + smb_fname_str_dbg(parent_smb_fname), + last_component, + nt_errstr(status)); + goto out; + } + + smb_fname_rel = synthetic_smb_fname(ctx, + atname, + NULL, + NULL, + twrp, + posix ? SMB_FILENAME_POSIX_PATH : 0); + if (smb_fname_rel == NULL) { + status = NT_STATUS_NO_MEMORY; + goto out; + } + + /* Get the referral to return. */ + status = SMB_VFS_READ_DFS_PATHAT(conn, + ctx, + parent_smb_fname->fsp, + smb_fname_rel, + ppreflist, + preferral_count); + if (!NT_STATUS_IS_OK(status)) { + DBG_DEBUG("dfspath = %s. reqpath = %s. " + "SMB_VFS_READ_DFS_PATHAT(%s, %s) error (%s)\n", + dfspath, + reqpath, + smb_fname_str_dbg(parent_smb_fname), + smb_fname_str_dbg(smb_fname_rel), + nt_errstr(status)); + goto out; + } + + /* + * Now we must work out how much of the + * given pathname we consumed. + */ + canon_dfspath = talloc_strdup(ctx, dfspath); + if (!canon_dfspath) { + status = NT_STATUS_NO_MEMORY; + goto out; + } + /* Canonicalize the raw dfspath. */ + string_replace(canon_dfspath, '\\', '/'); + + /* + * reqpath comes out of parse_dfs_path(), so it has + * no trailing backslash. Make sure that canon_dfspath hasn't either. + */ + trim_char(canon_dfspath, 0, '/'); + + DBG_DEBUG("Unconsumed path: %s\n", canon_dfspath); + + while (removed_components > 0) { + p = strrchr(canon_dfspath, '/'); + if (p != NULL) { + *p = '\0'; + } + removed_components--; + if (p == NULL && removed_components != 0) { + DBG_ERR("Component missmatch. path = %s, " + "%zu components left\n", + canon_dfspath, + removed_components); + status = NT_STATUS_OBJECT_PATH_NOT_FOUND; + goto out; + } + } + *consumedcntp = strlen(canon_dfspath); + DBG_DEBUG("Path consumed: %s (%zu)\n", canon_dfspath, *consumedcntp); + status = NT_STATUS_OK; + + out: + + TALLOC_FREE(parent_smb_fname); + TALLOC_FREE(local_pathname); + TALLOC_FREE(last_component); + TALLOC_FREE(atname); + TALLOC_FREE(smb_fname_rel); + TALLOC_FREE(canon_dfspath); + return status; +} +#endif + /***************************************************************** Used by other functions to decide if a dfs path is remote, and to get the list of referred locations for that remote path. -- 2.34.1 From 21eb4f19cdab31718a5a586a80d4690d58cb8741 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Tue, 9 Aug 2022 12:11:07 -0700 Subject: [PATCH 31/55] s3: smbd: Switch get_referred_path() over to use the new dfs_path_lookup(). New function doesn't need a TWRP argument and returns NT_STATUS_OK on successful redirect, not NT_STATUS_PATH_NOT_COVERED. Comment out the old dfs_path_lookup(). There are now no more users of unix_convert(). BUG: https://bugzilla.samba.org/show_bug.cgi?id=15144 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme BUG: https://bugzilla.samba.org/show_bug.cgi?id=15146 (cherry picked from commit 88e8bfec59412fdc0e83251fef60b45d2cc3a884) --- source3/smbd/msdfs.c | 31 ++++++------------------------- 1 file changed, 6 insertions(+), 25 deletions(-) diff --git a/source3/smbd/msdfs.c b/source3/smbd/msdfs.c index ab7f9f0ad788..55ab12a75e36 100644 --- a/source3/smbd/msdfs.c +++ b/source3/smbd/msdfs.c @@ -713,7 +713,6 @@ bool is_msdfs_link(struct files_struct *dirfsp, return (NT_STATUS_IS_OK(status)); } -#if 0 /***************************************************************** Used by other functions to decide if a dfs path is remote, and to get the list of referred locations for that remote path. @@ -972,8 +971,8 @@ static NTSTATUS dfs_path_lookup(TALLOC_CTX *ctx, TALLOC_FREE(canon_dfspath); return status; } -#endif +#if 0 /***************************************************************** Used by other functions to decide if a dfs path is remote, and to get the list of referred locations for that remote path. @@ -1162,6 +1161,7 @@ static NTSTATUS dfs_path_lookup(TALLOC_CTX *ctx, TALLOC_FREE(smb_fname); return status; } +#endif /***************************************************************** Decides if a dfs pathname should be redirected or not. @@ -1420,40 +1420,21 @@ NTSTATUS get_referred_path(TALLOC_CTX *ctx, } } - /* If this is a DFS path dfs_lookup should return - * NT_STATUS_PATH_NOT_COVERED. */ - status = dfs_path_lookup(ctx, conn, dfs_path, reqpath, 0, /* ucf_flags */ - NULL, consumedcntp, &jucn->referral_list, &jucn->referral_count); - if (!NT_STATUS_EQUAL(status, NT_STATUS_PATH_NOT_COVERED)) { - DEBUG(3,("get_referred_path: No valid referrals for path %s\n", - dfs_path)); - if (NT_STATUS_IS_OK(status)) { - /* - * We are in an error path here (we - * know it's not a DFS path), but - * dfs_path_lookup() can return - * NT_STATUS_OK. Ensure we always - * return a valid error code. - * - * #9588 - ACLs are not inherited to directories - * for DFS shares. - */ - status = NT_STATUS_NOT_FOUND; - } - goto err_exit; + if (!NT_STATUS_IS_OK(status)) { + DBG_NOTICE("No valid referrals for path %s (%s)\n", + dfs_path, + nt_errstr(status)); } - status = NT_STATUS_OK; - err_exit: TALLOC_FREE(frame); return status; } -- 2.34.1 From ed9d65cff510e6b5eca86307a2e617243ca20f3c Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Tue, 9 Aug 2022 12:13:10 -0700 Subject: [PATCH 32/55] s3: smbd: Remove the old dfs_path_lookup() code. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15144 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme BUG: https://bugzilla.samba.org/show_bug.cgi?id=15146 (cherry picked from commit cc638c25e0332d366016880d174d9349940cba3f) --- source3/smbd/msdfs.c | 191 ------------------------------------------- 1 file changed, 191 deletions(-) diff --git a/source3/smbd/msdfs.c b/source3/smbd/msdfs.c index 55ab12a75e36..4819df358378 100644 --- a/source3/smbd/msdfs.c +++ b/source3/smbd/msdfs.c @@ -972,197 +972,6 @@ static NTSTATUS dfs_path_lookup(TALLOC_CTX *ctx, return status; } -#if 0 -/***************************************************************** - Used by other functions to decide if a dfs path is remote, - and to get the list of referred locations for that remote path. - - consumedcntp: how much of the dfs path is being redirected. the client - should try the remaining path on the redirected server. - - If this returns NT_STATUS_PATH_NOT_COVERED the contents of the msdfs - link redirect are in targetpath. -*****************************************************************/ - -static NTSTATUS dfs_path_lookup(TALLOC_CTX *ctx, - connection_struct *conn, - const char *dfspath, /* Incoming complete dfs path */ - const char *reqpath, /* Parsed out remaining path. */ - uint32_t ucf_flags, - NTTIME *_twrp, - size_t *consumedcntp, - struct referral **ppreflist, - size_t *preferral_count) -{ - char *p = NULL; - char *q = NULL; - NTSTATUS status; - struct smb_filename *smb_fname = NULL; - struct smb_filename *parent_fname = NULL; - struct smb_filename *atname = NULL; - char *canon_dfspath = NULL; /* Canonicalized dfs path. (only '/' - components). */ - - DEBUG(10,("dfs_path_lookup: Conn path = %s reqpath = %s\n", - conn->connectpath, reqpath)); - - /* - * Note the unix path conversion here we're doing we - * throw away. We're looking for a symlink for a dfs - * resolution, if we don't find it we'll do another - * unix_convert later in the codepath. - */ - - status = unix_convert(ctx, conn, reqpath, 0, &smb_fname, - ucf_flags); - - if (!NT_STATUS_IS_OK(status)) { - if (!NT_STATUS_EQUAL(status, - NT_STATUS_OBJECT_PATH_NOT_FOUND)) { - return status; - } - if (smb_fname == NULL || smb_fname->base_name == NULL) { - return status; - } - } - - /* Optimization - check if we can redirect the whole path. */ - status = parent_pathref(ctx, - conn->cwd_fsp, - smb_fname, - &parent_fname, - &atname); - if (NT_STATUS_IS_OK(status)) { - /* - * We must have a parent_fname->fsp before - * we can call SMB_VFS_READ_DFS_PATHAT(). - */ - status = SMB_VFS_READ_DFS_PATHAT(conn, - ctx, - parent_fname->fsp, - atname, - ppreflist, - preferral_count); - /* We're now done with parent_fname and atname. */ - TALLOC_FREE(parent_fname); - - if (NT_STATUS_IS_OK(status)) { - DBG_INFO("%s resolves to a valid dfs link\n", - dfspath); - - if (consumedcntp) { - *consumedcntp = strlen(dfspath); - } - status = NT_STATUS_PATH_NOT_COVERED; - goto out; - } - } - - /* Prepare to test only for '/' components in the given path, - * so if a Windows path replace all '\\' characters with '/'. - * For a POSIX DFS path we know all separators are already '/'. */ - - canon_dfspath = talloc_strdup(ctx, dfspath); - if (!canon_dfspath) { - status = NT_STATUS_NO_MEMORY; - goto out; - } - string_replace(canon_dfspath, '\\', '/'); - - /* - * localpath comes out of unix_convert, so it has - * no trailing backslash. Make sure that canon_dfspath hasn't either. - * Fix for bug #4860 from Jan Martin . - */ - - trim_char(canon_dfspath,0,'/'); - - /* - * Redirect if any component in the path is a link. - * We do this by walking backwards through the - * local path, chopping off the last component - * in both the local path and the canonicalized - * DFS path. If we hit a DFS link then we're done. - */ - - p = strrchr_m(smb_fname->base_name, '/'); - if (consumedcntp) { - q = strrchr_m(canon_dfspath, '/'); - } - - while (p) { - *p = '\0'; - if (q) { - *q = '\0'; - } - - /* - * Ensure parent_pathref() calls vfs_stat() on - * the newly truncated path. - */ - SET_STAT_INVALID(smb_fname->st); - status = parent_pathref(ctx, - conn->cwd_fsp, - smb_fname, - &parent_fname, - &atname); - if (NT_STATUS_IS_OK(status)) { - /* - * We must have a parent_fname->fsp before - * we can call SMB_VFS_READ_DFS_PATHAT(). - */ - status = SMB_VFS_READ_DFS_PATHAT(conn, - ctx, - parent_fname->fsp, - atname, - ppreflist, - preferral_count); - - /* We're now done with parent_fname and atname. */ - TALLOC_FREE(parent_fname); - - if (NT_STATUS_IS_OK(status)) { - DBG_INFO("Redirecting %s because " - "parent %s is a dfs link\n", - dfspath, - smb_fname_str_dbg(smb_fname)); - - if (consumedcntp) { - *consumedcntp = strlen(canon_dfspath); - DBG_DEBUG("Path consumed: %s " - "(%zu)\n", - canon_dfspath, - *consumedcntp); - } - - status = NT_STATUS_PATH_NOT_COVERED; - goto out; - } - } - - /* Step back on the filesystem. */ - p = strrchr_m(smb_fname->base_name, '/'); - - if (consumedcntp) { - /* And in the canonicalized dfs path. */ - q = strrchr_m(canon_dfspath, '/'); - } - } - - if ((ucf_flags & UCF_GMT_PATHNAME) && _twrp != NULL) { - *_twrp = smb_fname->twrp; - } - - status = NT_STATUS_OK; - out: - - /* This should already be free, but make sure. */ - TALLOC_FREE(parent_fname); - TALLOC_FREE(smb_fname); - return status; -} -#endif - /***************************************************************** Decides if a dfs pathname should be redirected or not. If not, the pathname is converted to a tcon-relative local unix path -- 2.34.1 From 32d99f9e42d8d734e3c632cde36c75ea36cacee7 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Mon, 8 Aug 2022 21:59:14 -0700 Subject: [PATCH 33/55] s3: smbd: Remove unix_convert() and associated functions. All code now uses filename_convert_dirfsp() for race-free filename conversion. Best viewed with: $ git show --patience ---------------- / \ / REST \ / IN \ / PEACE \ / \ | | | unix_convert | | | | | | 9th August | | 2022 | | | | | *| * * * | * _________)/\\_//(\/(/\)/\//\/\///\/|_)_______ BUG: https://bugzilla.samba.org/show_bug.cgi?id=15144 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme Autobuild-User(master): Jeremy Allison Autobuild-Date(master): Fri Aug 12 19:18:25 UTC 2022 on sn-devel-184 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15146 (cherry picked from commit 78e4aac76df977cea6cdbcfdf082fd3acdffbd95) --- source3/smbd/filename.c | 1565 +++------------------------------------ source3/smbd/proto.h | 12 - 2 files changed, 107 insertions(+), 1470 deletions(-) diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c index b64fb908f16c..f362aee94525 100644 --- a/source3/smbd/filename.c +++ b/source3/smbd/filename.c @@ -31,15 +31,6 @@ #include "smbd/globals.h" #include "lib/util/memcache.h" -static NTSTATUS get_real_filename(connection_struct *conn, - struct smb_filename *path, - const char *name, - TALLOC_CTX *mem_ctx, - char **found_name); - -static NTSTATUS check_name(connection_struct *conn, - const struct smb_filename *smb_fname); - uint32_t ucf_flags_from_smb_request(struct smb_request *req) { uint32_t ucf_flags = 0; @@ -80,10 +71,6 @@ uint32_t filename_create_ucf_flags(struct smb_request *req, uint32_t create_disp return ucf_flags; } -static NTSTATUS build_stream_path(TALLOC_CTX *mem_ctx, - connection_struct *conn, - struct smb_filename *smb_fname); - /**************************************************************************** Mangle the 2nd name and check if it is then equal to the first name. ****************************************************************************/ @@ -100,153 +87,6 @@ static bool mangled_equal(const char *name1, return strequal(name1, mname); } -static NTSTATUS check_for_dot_component(const struct smb_filename *smb_fname) -{ - /* Ensure we catch all names with in "/." - this is disallowed under Windows and - in POSIX they've already been removed. */ - const char *p = strstr(smb_fname->base_name, "/."); /*mb safe*/ - if (p) { - if (p[2] == '/') { - /* Error code within a pathname. */ - return NT_STATUS_OBJECT_PATH_NOT_FOUND; - } else if (p[2] == '\0') { - /* Error code at the end of a pathname. */ - return NT_STATUS_OBJECT_NAME_INVALID; - } - } - return NT_STATUS_OK; -} - -/**************************************************************************** - Optimization for common case where the missing part - is in the last component and the client already - sent the correct case. - Returns NT_STATUS_OK to mean continue the tree walk - (possibly with modified start pointer). - Any other NT_STATUS_XXX error means terminate the path - lookup here. -****************************************************************************/ - -static NTSTATUS check_parent_exists(TALLOC_CTX *ctx, - connection_struct *conn, - bool posix_pathnames, - const struct smb_filename *smb_fname, - char **pp_dirpath, - char **pp_start, - int *p_parent_stat_errno) -{ - char *parent_name = NULL; - struct smb_filename *parent_fname = NULL; - const char *last_component = NULL; - NTSTATUS status; - int ret; - - if (!parent_dirname(ctx, smb_fname->base_name, - &parent_name, - &last_component)) { - return NT_STATUS_NO_MEMORY; - } - - if (!posix_pathnames) { - if (ms_has_wild(parent_name)) { - goto no_optimization_out; - } - } - - /* - * If there was no parent component in - * smb_fname->base_name then don't do this - * optimization. - */ - if (smb_fname->base_name == last_component) { - goto no_optimization_out; - } - - parent_fname = synthetic_smb_fname(ctx, - parent_name, - NULL, - NULL, - smb_fname->twrp, - smb_fname->flags); - if (parent_fname == NULL) { - return NT_STATUS_NO_MEMORY; - } - - ret = vfs_stat(conn, parent_fname); - - /* If the parent stat failed, just continue - with the normal tree walk. */ - - if (ret == -1) { - /* - * Optimization. Preserving the - * errno from the STAT/LSTAT here - * will allow us to save a duplicate - * STAT/LSTAT system call of the parent - * pathname in a hot code path in the caller. - */ - if (p_parent_stat_errno != NULL) { - *p_parent_stat_errno = errno; - } - goto no_optimization_out; - } - - status = check_for_dot_component(parent_fname); - if (!NT_STATUS_IS_OK(status)) { - return status; - } - - /* Parent exists - set "start" to be the - * last component to shorten the tree walk. */ - - /* - * Safe to use discard_const_p - * here as last_component points - * into our smb_fname->base_name. - */ - *pp_start = discard_const_p(char, last_component); - - /* Update dirpath. */ - TALLOC_FREE(*pp_dirpath); - *pp_dirpath = talloc_strdup(ctx, parent_fname->base_name); - if (!*pp_dirpath) { - return NT_STATUS_NO_MEMORY; - } - - DEBUG(5,("check_parent_exists: name " - "= %s, dirpath = %s, " - "start = %s\n", - smb_fname->base_name, - *pp_dirpath, - *pp_start)); - - return NT_STATUS_OK; - - no_optimization_out: - - /* - * We must still return an *pp_dirpath - * initialized to ".", and a *pp_start - * pointing at smb_fname->base_name. - */ - - TALLOC_FREE(parent_name); - TALLOC_FREE(parent_fname); - - *pp_dirpath = talloc_strdup(ctx, "."); - if (*pp_dirpath == NULL) { - return NT_STATUS_NO_MEMORY; - } - /* - * Safe to use discard_const_p - * here as by convention smb_fname->base_name - * is allocated off ctx. - */ - *pp_start = discard_const_p(char, smb_fname->base_name); - return NT_STATUS_OK; -} - static bool find_snapshot_token( const char *filename, const char **_start, @@ -255,1138 +95,148 @@ static bool find_snapshot_token( { const char *start = NULL; const char *end = NULL; - struct tm tm; - time_t t; - - start = strstr_m(filename, "@GMT-"); - - if (start == NULL) { - return false; - } - - if ((start > filename) && (start[-1] != '/')) { - /* the GMT-token does not start a path-component */ - return false; - } - - end = strptime(start, GMT_FORMAT, &tm); - if (end == NULL) { - /* Not a valid timestring. */ - return false; - } - - if ((end[0] != '\0') && (end[0] != '/')) { - /* - * It is not a complete path component, i.e. the path - * component continues after the gmt-token. - */ - return false; - } - - tm.tm_isdst = -1; - t = timegm(&tm); - unix_to_nt_time(twrp, t); - - DBG_DEBUG("Extracted @GMT-Timestamp %s\n", - nt_time_string(talloc_tos(), *twrp)); - - *_start = start; - - if (end[0] == '/') { - end += 1; - } - *_next_component = end; - - return true; -} - -bool extract_snapshot_token(char *fname, NTTIME *twrp) -{ - const char *start = NULL; - const char *next = NULL; - size_t remaining; - bool found; - - found = find_snapshot_token(fname, &start, &next, twrp); - if (!found) { - return false; - } - - remaining = strlen(next); - memmove(discard_const_p(char, start), next, remaining+1); - - return true; -} - -/* - * Strip a valid @GMT-token from any incoming filename path, - * adding any NTTIME encoded in the pathname into the - * twrp field of the passed in smb_fname. - * - * Valid @GMT-tokens look like @GMT-YYYY-MM-DD-HH-MM-SS - * at the *start* of a pathname component. - * - * If twrp is passed in then smb_fname->twrp is set to that - * value, and the @GMT-token part of the filename is removed - * and does not change the stored smb_fname->twrp. - * - */ - -NTSTATUS canonicalize_snapshot_path(struct smb_filename *smb_fname, - uint32_t ucf_flags, - NTTIME twrp) -{ - bool found; - - if (twrp != 0) { - smb_fname->twrp = twrp; - } - - if (!(ucf_flags & UCF_GMT_PATHNAME)) { - return NT_STATUS_OK; - } - - found = extract_snapshot_token(smb_fname->base_name, &twrp); - if (!found) { - return NT_STATUS_OK; - } - - if (smb_fname->twrp == 0) { - smb_fname->twrp = twrp; - } - - return NT_STATUS_OK; -} - -static bool strnorm(char *s, int case_default) -{ - if (case_default == CASE_UPPER) - return strupper_m(s); - else - return strlower_m(s); -} - -/* - * Utility function to normalize case on an incoming client filename - * if required on this connection struct. - * Performs an in-place case conversion guaranteed to stay the same size. - */ - -static NTSTATUS normalize_filename_case(connection_struct *conn, - char *filename, - uint32_t ucf_flags) -{ - bool ok; - - if (ucf_flags & UCF_POSIX_PATHNAMES) { - /* - * POSIX never normalizes filename case. - */ - return NT_STATUS_OK; - } - if (!conn->case_sensitive) { - return NT_STATUS_OK; - } - if (conn->case_preserve) { - return NT_STATUS_OK; - } - if (conn->short_case_preserve) { - return NT_STATUS_OK; - } - ok = strnorm(filename, lp_default_case(SNUM(conn))); - if (!ok) { - return NT_STATUS_INVALID_PARAMETER; - } - return NT_STATUS_OK; -} - -/**************************************************************************** -This routine is called to convert names from the dos namespace to unix -namespace. It needs to handle any case conversions, mangling, format changes, -streams etc. - -We assume that we have already done a chdir() to the right "root" directory -for this service. - -Conversion to basic unix format is already done in check_path_syntax(). - -Names must be relative to the root of the service - any leading /. and -trailing /'s should have been trimmed by check_path_syntax(). - -The function will return an NTSTATUS error if some part of the name except for -the last part cannot be resolved, else NT_STATUS_OK. - -Note NT_STATUS_OK doesn't mean the name exists or is valid, just that we -didn't get any fatal errors that should immediately terminate the calling SMB -processing whilst resolving. - -If the orig_path was a stream, smb_filename->base_name will point to the base -filename, and smb_filename->stream_name will point to the stream name. If -orig_path was not a stream, then smb_filename->stream_name will be NULL. - -On exit from unix_convert, the smb_filename->st stat struct will be populated -if the file exists and was found, if not this stat struct will be filled with -zeros (and this can be detected by checking for nlinks = 0, which can never be -true for any file). -****************************************************************************/ - -struct uc_state { - TALLOC_CTX *mem_ctx; - struct connection_struct *conn; - struct smb_filename *smb_fname; - const char *orig_path; - uint32_t ucf_flags; - char *name; - char *end; - char *dirpath; - char *stream; - bool component_was_mangled; - bool posix_pathnames; - bool done; - bool case_sensitive; - bool case_preserve; - bool short_case_preserve; -}; - -static NTSTATUS unix_convert_step_search_fail( - struct uc_state *state, NTSTATUS status) -{ - char *unmangled; - - if (state->end) { - /* - * An intermediate part of the name - * can't be found. - */ - DBG_DEBUG("Intermediate [%s] missing\n", - state->name); - *state->end = '/'; - - /* - * We need to return the fact that the - * intermediate name resolution failed. - * This is used to return an error of - * ERRbadpath rather than ERRbadfile. - * Some Windows applications depend on - * the difference between these two - * errors. - */ - - /* - * ENOENT, ENOTDIR and ELOOP all map - * to NT_STATUS_OBJECT_PATH_NOT_FOUND - * in the filename walk. - */ - if (NT_STATUS_EQUAL(status, NT_STATUS_OBJECT_NAME_NOT_FOUND) || - NT_STATUS_EQUAL(status, NT_STATUS_STOPPED_ON_SYMLINK) || - NT_STATUS_EQUAL(status, NT_STATUS_NOT_A_DIRECTORY)) { - status = NT_STATUS_OBJECT_PATH_NOT_FOUND; - } - return status; - } - - /* - * ENOENT/EACCESS are the only valid errors - * here. - */ - - if (NT_STATUS_EQUAL(status, NT_STATUS_ACCESS_DENIED)) { - if ((state->ucf_flags & UCF_PREP_CREATEFILE) == 0) { - /* - * Could be a symlink pointing to - * a directory outside the share - * to which we don't have access. - * If so, we need to know that here - * so we can return the correct error code. - * check_name() is never called if we - * error out of filename_convert(). - */ - int ret; - struct smb_filename dname = (struct smb_filename) { - .base_name = state->dirpath, - .twrp = state->smb_fname->twrp, - }; - - /* handle null paths */ - if ((dname.base_name == NULL) || - (dname.base_name[0] == '\0')) { - return NT_STATUS_ACCESS_DENIED; - } - ret = SMB_VFS_LSTAT(state->conn, &dname); - if (ret != 0) { - return NT_STATUS_ACCESS_DENIED; - } - if (!S_ISLNK(dname.st.st_ex_mode)) { - return NT_STATUS_ACCESS_DENIED; - } - status = check_name(state->conn, &dname); - if (!NT_STATUS_IS_OK(status)) { - /* We know this is an intermediate path. */ - return NT_STATUS_OBJECT_PATH_NOT_FOUND; - } - return NT_STATUS_ACCESS_DENIED; - } else { - /* - * This is the dropbox - * behaviour. A dropbox is a - * directory with only -wx - * permissions, so - * get_real_filename fails - * with EACCESS, it needs to - * list the directory. We - * nevertheless want to allow - * users creating a file. - */ - status = NT_STATUS_OK; - } - } - - if (!NT_STATUS_IS_OK(status) && - !NT_STATUS_EQUAL(status, NT_STATUS_OBJECT_NAME_NOT_FOUND)) { - /* - * ENOTDIR and ELOOP both map to - * NT_STATUS_OBJECT_PATH_NOT_FOUND - * in the filename walk. - */ - if (NT_STATUS_EQUAL(status, NT_STATUS_NOT_A_DIRECTORY) || - NT_STATUS_EQUAL(status, NT_STATUS_STOPPED_ON_SYMLINK)) { - status = NT_STATUS_OBJECT_PATH_NOT_FOUND; - } - return status; - } - - /* - * POSIX pathnames must never call into mangling. - */ - if (state->posix_pathnames) { - goto done; - } - - /* - * Just the last part of the name doesn't exist. - * We need to strupper() or strlower() it as - * this conversion may be used for file creation - * purposes. Fix inspired by - * Thomas Neumann . - */ - if (!state->case_preserve || - (mangle_is_8_3(state->name, false, - state->conn->params) && - !state->short_case_preserve)) { - if (!strnorm(state->name, - lp_default_case(SNUM(state->conn)))) { - DBG_DEBUG("strnorm %s failed\n", - state->name); - return NT_STATUS_INVALID_PARAMETER; - } - } - - /* - * check on the mangled stack to see if we can - * recover the base of the filename. - */ - - if (mangle_is_mangled(state->name, state->conn->params) - && mangle_lookup_name_from_8_3(state->mem_ctx, - state->name, - &unmangled, - state->conn->params)) { - char *tmp; - size_t name_ofs = - state->name - state->smb_fname->base_name; - - if (!ISDOT(state->dirpath)) { - tmp = talloc_asprintf( - state->smb_fname, "%s/%s", - state->dirpath, unmangled); - TALLOC_FREE(unmangled); - } - else { - tmp = unmangled; - } - if (tmp == NULL) { - DBG_ERR("talloc failed\n"); - return NT_STATUS_NO_MEMORY; - } - TALLOC_FREE(state->smb_fname->base_name); - state->smb_fname->base_name = tmp; - state->name = - state->smb_fname->base_name + name_ofs; - state->end = state->name + strlen(state->name); - } - - done: - - DBG_DEBUG("New file [%s]\n", state->name); - state->done = true; - return NT_STATUS_OK; -} - -static NTSTATUS unix_convert_step_stat(struct uc_state *state) -{ - struct smb_filename dname; - char dot[2] = "."; - char *found_name = NULL; - int ret; - NTSTATUS status; - - /* - * Check if the name exists up to this point. - */ - - DBG_DEBUG("smb_fname [%s]\n", smb_fname_str_dbg(state->smb_fname)); - - ret = vfs_stat(state->conn, state->smb_fname); - if (ret == 0) { - /* - * It exists. it must either be a directory or this must - * be the last part of the path for it to be OK. - */ - if (state->end && !S_ISDIR(state->smb_fname->st.st_ex_mode)) { - /* - * An intermediate part of the name isn't - * a directory. - */ - DBG_DEBUG("Not a dir [%s]\n", state->name); - *state->end = '/'; - /* - * We need to return the fact that the - * intermediate name resolution failed. This - * is used to return an error of ERRbadpath - * rather than ERRbadfile. Some Windows - * applications depend on the difference between - * these two errors. - */ - return NT_STATUS_OBJECT_PATH_NOT_FOUND; - } - return NT_STATUS_OK; - } - - /* Stat failed - ensure we don't use it. */ - SET_STAT_INVALID(state->smb_fname->st); - - if (state->posix_pathnames) { - /* - * For posix_pathnames, we're done. - * Don't blunder into the - * get_real_filename() codepath as they may - * be doing case insensitive lookups. So when - * creating a new POSIX directory Foo they might - * match on name foo. - * - * BUG: https://bugzilla.samba.org/show_bug.cgi?id=13803 - */ - if (state->end != NULL) { - const char *morepath = NULL; - /* - * If this is intermediate we must - * restore the full path. - */ - *state->end = '/'; - /* - * If there are any more components - * after the failed LSTAT we cannot - * continue. - */ - morepath = strchr(state->end + 1, '/'); - if (morepath != NULL) { - return NT_STATUS_OBJECT_PATH_NOT_FOUND; - } - } - if (errno == ENOENT) { - /* New file or directory. */ - state->done = true; - return NT_STATUS_OK; - } - if ((errno == EACCES) && - (state->ucf_flags & UCF_PREP_CREATEFILE)) { - /* POSIX Dropbox case. */ - errno = 0; - state->done = true; - return NT_STATUS_OK; - } - return map_nt_error_from_unix(errno); - } - - /* - * Reset errno so we can detect - * directory open errors. - */ - errno = 0; - - /* - * Try to find this part of the path in the directory. - */ - - dname = (struct smb_filename) { - .base_name = state->dirpath, - .twrp = state->smb_fname->twrp, - }; - - /* handle null paths */ - if ((dname.base_name == NULL) || (dname.base_name[0] == '\0')) { - dname.base_name = dot; - } - - status = get_real_filename(state->conn, - &dname, - state->name, - talloc_tos(), - &found_name); - if (!NT_STATUS_IS_OK(status)) { - return unix_convert_step_search_fail(state, status); - } - - /* - * Restore the rest of the string. If the string was - * mangled the size may have changed. - */ - if (state->end) { - char *tmp; - size_t name_ofs = - state->name - state->smb_fname->base_name; - - if (!ISDOT(state->dirpath)) { - tmp = talloc_asprintf(state->smb_fname, - "%s/%s/%s", state->dirpath, - found_name, state->end+1); - } - else { - tmp = talloc_asprintf(state->smb_fname, - "%s/%s", found_name, - state->end+1); - } - if (tmp == NULL) { - DBG_ERR("talloc_asprintf failed\n"); - return NT_STATUS_NO_MEMORY; - } - TALLOC_FREE(state->smb_fname->base_name); - state->smb_fname->base_name = tmp; - state->name = state->smb_fname->base_name + name_ofs; - state->end = state->name + strlen(found_name); - *state->end = '\0'; - } else { - char *tmp; - size_t name_ofs = - state->name - state->smb_fname->base_name; - - if (!ISDOT(state->dirpath)) { - tmp = talloc_asprintf(state->smb_fname, - "%s/%s", state->dirpath, - found_name); - } else { - tmp = talloc_strdup(state->smb_fname, - found_name); - } - if (tmp == NULL) { - DBG_ERR("talloc failed\n"); - return NT_STATUS_NO_MEMORY; - } - TALLOC_FREE(state->smb_fname->base_name); - state->smb_fname->base_name = tmp; - state->name = state->smb_fname->base_name + name_ofs; - - /* - * We just scanned for, and found the end of - * the path. We must return a valid stat struct - * if it exists. JRA. - */ - - ret = vfs_stat(state->conn, state->smb_fname); - if (ret != 0) { - SET_STAT_INVALID(state->smb_fname->st); - } - } - - TALLOC_FREE(found_name); - return NT_STATUS_OK; -} - -static NTSTATUS unix_convert_step(struct uc_state *state) -{ - NTSTATUS status; - - /* - * Pinpoint the end of this section of the filename. - */ - /* mb safe. '/' can't be in any encoded char. */ - state->end = strchr(state->name, '/'); - - /* - * Chop the name at this point. - */ - if (state->end != NULL) { - *state->end = 0; - } - - DBG_DEBUG("dirpath [%s] name [%s]\n", state->dirpath, state->name); - - /* The name cannot have a component of "." */ - - if (ISDOT(state->name)) { - if (state->end == NULL) { - /* Error code at the end of a pathname. */ - return NT_STATUS_OBJECT_NAME_INVALID; - } - return NT_STATUS_OBJECT_PATH_NOT_FOUND; - } - - status = unix_convert_step_stat(state); - if (!NT_STATUS_IS_OK(status)) { - return status; - } - if (state->done) { - return NT_STATUS_OK; - } - - /* - * Add to the dirpath that we have resolved so far. - */ - - if (!ISDOT(state->dirpath)) { - char *tmp = talloc_asprintf(state->mem_ctx, - "%s/%s", state->dirpath, state->name); - if (!tmp) { - DBG_ERR("talloc_asprintf failed\n"); - return NT_STATUS_NO_MEMORY; - } - TALLOC_FREE(state->dirpath); - state->dirpath = tmp; - } - else { - TALLOC_FREE(state->dirpath); - if (!(state->dirpath = talloc_strdup(state->mem_ctx,state->name))) { - DBG_ERR("talloc_strdup failed\n"); - return NT_STATUS_NO_MEMORY; - } - } - - /* - * Cache the dirpath thus far. Don't cache a name with mangled - * components as this can change the size. - */ - if(!state->component_was_mangled) { - stat_cache_add(state->orig_path, - state->dirpath, - state->smb_fname->twrp, - state->case_sensitive); - } - - /* - * Restore the / that we wiped out earlier. - */ - if (state->end != NULL) { - *state->end = '/'; - } - - return NT_STATUS_OK; -} - -NTSTATUS unix_convert(TALLOC_CTX *mem_ctx, - connection_struct *conn, - const char *orig_path, - NTTIME twrp, - struct smb_filename **smb_fname_out, - uint32_t ucf_flags) -{ - struct uc_state uc_state; - struct uc_state *state = &uc_state; - NTSTATUS status; - int ret = -1; - int parent_stat_errno = 0; - - *state = (struct uc_state) { - .mem_ctx = mem_ctx, - .conn = conn, - .orig_path = orig_path, - .ucf_flags = ucf_flags, - .posix_pathnames = (ucf_flags & UCF_POSIX_PATHNAMES), - .case_sensitive = conn->case_sensitive, - .case_preserve = conn->case_preserve, - .short_case_preserve = conn->short_case_preserve, - }; - - *smb_fname_out = NULL; - - if (state->posix_pathnames) { - /* POSIX means ignore case settings on share. */ - state->case_sensitive = true; - state->case_preserve = true; - state->short_case_preserve = true; - } - - state->smb_fname = talloc_zero(state->mem_ctx, struct smb_filename); - if (state->smb_fname == NULL) { - return NT_STATUS_NO_MEMORY; - } - - if (state->conn->printer) { - /* we don't ever use the filenames on a printer share as a - filename - so don't convert them */ - state->smb_fname->base_name = talloc_strdup( - state->smb_fname, state->orig_path); - if (state->smb_fname->base_name == NULL) { - status = NT_STATUS_NO_MEMORY; - goto err; - } - goto done; - } - - state->smb_fname->flags = state->posix_pathnames ? SMB_FILENAME_POSIX_PATH : 0; - - DBG_DEBUG("Called on file [%s]\n", state->orig_path); - - if (state->orig_path[0] == '/') { - DBG_ERR("Path [%s] starts with '/'\n", state->orig_path); - return NT_STATUS_OBJECT_NAME_INVALID; - } - - /* Start with the full orig_path as given by the caller. */ - state->smb_fname->base_name = talloc_strdup( - state->smb_fname, state->orig_path); - if (state->smb_fname->base_name == NULL) { - DBG_ERR("talloc_strdup failed\n"); - status = NT_STATUS_NO_MEMORY; - goto err; - } - - /* Canonicalize any @GMT- paths. */ - status = canonicalize_snapshot_path(state->smb_fname, ucf_flags, twrp); - if (!NT_STATUS_IS_OK(status)) { - goto err; - } - - /* - * If we trimmed down to a single '\0' character - * then we should use the "." directory to avoid - * searching the cache, but not if we are in a - * printing share. - * As we know this is valid we can return true here. - */ - - if (state->smb_fname->base_name[0] == '\0') { - state->smb_fname->base_name = talloc_strdup(state->smb_fname, "."); - if (state->smb_fname->base_name == NULL) { - status = NT_STATUS_NO_MEMORY; - goto err; - } - if (SMB_VFS_STAT(state->conn, state->smb_fname) != 0) { - status = map_nt_error_from_unix(errno); - goto err; - } - DBG_DEBUG("conversion finished [] -> [%s]\n", - state->smb_fname->base_name); - goto done; - } - - if (state->orig_path[0] == '.' && (state->orig_path[1] == '/' || - state->orig_path[1] == '\0')) { - /* Start of pathname can't be "." only. */ - if (state->orig_path[1] == '\0' || state->orig_path[2] == '\0') { - status = NT_STATUS_OBJECT_NAME_INVALID; - } else { - status = NT_STATUS_OBJECT_PATH_NOT_FOUND; - } - goto err; - } - - /* - * Large directory fix normalization. If we're case sensitive, and - * the case preserving parameters are set to "no", normalize the case of - * the incoming filename from the client WHETHER IT EXISTS OR NOT ! - * This is in conflict with the current (3.0.20) man page, but is - * what people expect from the "large directory howto". I'll update - * the man page. Thanks to jht@samba.org for finding this. JRA. - */ - - status = normalize_filename_case(state->conn, - state->smb_fname->base_name, - ucf_flags); - if (!NT_STATUS_IS_OK(status)) { - DBG_ERR("normalize_filename_case %s failed\n", - state->smb_fname->base_name); - goto err; - } - - /* - * Strip off the stream, and add it back when we're done with the - * base_name. - */ - if (!state->posix_pathnames) { - state->stream = strchr_m(state->smb_fname->base_name, ':'); - - if (state->stream != NULL) { - char *tmp = talloc_strdup(state->smb_fname, state->stream); - if (tmp == NULL) { - status = NT_STATUS_NO_MEMORY; - goto err; - } - /* - * Since this is actually pointing into - * smb_fname->base_name this truncates base_name. - */ - *state->stream = '\0'; - state->stream = tmp; - - if (state->smb_fname->base_name[0] == '\0') { - /* - * orig_name was just a stream name. - * This is a stream on the root of - * the share. Replace base_name with - * a "." - */ - state->smb_fname->base_name = - talloc_strdup(state->smb_fname, "."); - if (state->smb_fname->base_name == NULL) { - status = NT_STATUS_NO_MEMORY; - goto err; - } - if (SMB_VFS_STAT(state->conn, state->smb_fname) != 0) { - status = map_nt_error_from_unix(errno); - goto err; - } - /* dirpath must exist. */ - state->dirpath = talloc_strdup(state->mem_ctx,"."); - if (state->dirpath == NULL) { - status = NT_STATUS_NO_MEMORY; - goto err; - } - DBG_INFO("conversion finished [%s] -> [%s]\n", - state->orig_path, - state->smb_fname->base_name); - goto done; - } - } - } - - state->name = state->smb_fname->base_name; - - /* - * If we're providing case insensitive semantics or - * the underlying filesystem is case insensitive, - * then a case-normalized hit in the stat-cache is - * authoritative. JRA. - * - * Note: We're only checking base_name. The stream_name will be - * added and verified in build_stream_path(). - */ - - if (!state->case_sensitive || - !(state->conn->fs_capabilities & FILE_CASE_SENSITIVE_SEARCH)) - { - bool found; - - found = stat_cache_lookup(state->conn, - &state->smb_fname->base_name, - &state->dirpath, - &state->name, - state->smb_fname->twrp, - &state->smb_fname->st); - /* - * stat_cache_lookup() allocates on talloc_tos() even - * when !found, reparent correctly - */ - talloc_steal(state->smb_fname, state->smb_fname->base_name); - talloc_steal(state->mem_ctx, state->dirpath); - - if (found) { - goto done; - } - } - - /* - * Make sure "dirpath" is an allocated string, we use this for - * building the directories with talloc_asprintf and free it. - */ - - if (state->dirpath == NULL) { - state->dirpath = talloc_strdup(state->mem_ctx,"."); - if (state->dirpath == NULL) { - DBG_ERR("talloc_strdup failed\n"); - status = NT_STATUS_NO_MEMORY; - goto err; - } - } - - /* - * If we have a wildcard we must walk the path to - * find where the error is, even if case sensitive - * is true. - */ - - if (!state->posix_pathnames) { - /* POSIX pathnames have no wildcards. */ - bool name_has_wildcard = ms_has_wild(state->smb_fname->base_name); - if (name_has_wildcard) { - /* Wildcard not valid anywhere. */ - status = NT_STATUS_OBJECT_NAME_INVALID; - goto fail; - } - } - - DBG_DEBUG("Begin: name [%s] dirpath [%s] name [%s]\n", - state->smb_fname->base_name, state->dirpath, state->name); - - /* - * stat the name - if it exists then we can add the stream back (if - * there was one) and be done! - */ - - ret = vfs_stat(state->conn, state->smb_fname); - if (ret == 0) { - status = check_for_dot_component(state->smb_fname); - if (!NT_STATUS_IS_OK(status)) { - goto fail; - } - /* Add the path (not including the stream) to the cache. */ - stat_cache_add(state->orig_path, - state->smb_fname->base_name, - state->smb_fname->twrp, - state->case_sensitive); - DBG_DEBUG("Conversion of base_name finished " - "[%s] -> [%s]\n", - state->orig_path, state->smb_fname->base_name); - goto done; - } - - /* Stat failed - ensure we don't use it. */ - SET_STAT_INVALID(state->smb_fname->st); - - /* - * Note: we must continue processing a path if we get EACCES - * from stat. With NFS4 permissions the file might be lacking - * READ_ATTR, but if the parent has LIST permissions we can - * resolve the path in the path traversal loop down below. - */ - - if (errno == ENOENT) { - /* Optimization when creating a new file - only - the last component doesn't exist. - NOTE : check_parent_exists() doesn't preserve errno. - */ - int saved_errno = errno; - status = check_parent_exists(state->mem_ctx, - state->conn, - state->posix_pathnames, - state->smb_fname, - &state->dirpath, - &state->name, - &parent_stat_errno); - errno = saved_errno; - if (!NT_STATUS_IS_OK(status)) { - goto fail; - } - } - - /* - * A special case - if we don't have any wildcards or mangling chars and are case - * sensitive or the underlying filesystem is case insensitive then searching - * won't help. - * - * NB. As POSIX sets state->case_sensitive as - * true we will never call into mangle_is_mangled() here. - */ - - if ((state->case_sensitive || !(state->conn->fs_capabilities & - FILE_CASE_SENSITIVE_SEARCH)) && - !mangle_is_mangled(state->smb_fname->base_name, state->conn->params)) { - - status = check_for_dot_component(state->smb_fname); - if (!NT_STATUS_IS_OK(status)) { - goto fail; - } + struct tm tm; + time_t t; - /* - * The stat failed. Could be ok as it could be - * a new file. - */ + start = strstr_m(filename, "@GMT-"); - if (errno == ENOTDIR || errno == ELOOP) { - status = NT_STATUS_OBJECT_PATH_NOT_FOUND; - goto fail; - } else if (errno == ENOENT) { - /* - * Was it a missing last component ? - * or a missing intermediate component ? - * - * Optimization. - * - * For this code path we can guarantee that - * we have gone through check_parent_exists() - * and it returned NT_STATUS_OK. - * - * Either there was no parent component (".") - * parent_stat_errno == 0 and we have a missing - * last component here. - * - * OR check_parent_exists() called STAT/LSTAT - * and if it failed parent_stat_errno has been - * set telling us if the parent existed or not. - * - * Either way we can avoid another STAT/LSTAT - * system call on the parent here. - */ - if (parent_stat_errno == ENOTDIR || - parent_stat_errno == ENOENT || - parent_stat_errno == ELOOP) { - status = NT_STATUS_OBJECT_PATH_NOT_FOUND; - goto fail; - } + if (start == NULL) { + return false; + } - /* - * Missing last component is ok - new file. - * Also deal with permission denied elsewhere. - * Just drop out to done. - */ - goto done; - } + if ((start > filename) && (start[-1] != '/')) { + /* the GMT-token does not start a path-component */ + return false; } - /* - * is_mangled() was changed to look at an entire pathname, not - * just a component. JRA. - */ + end = strptime(start, GMT_FORMAT, &tm); + if (end == NULL) { + /* Not a valid timestring. */ + return false; + } - if (state->posix_pathnames) { + if ((end[0] != '\0') && (end[0] != '/')) { /* - * POSIX names are never mangled and we must not - * call into mangling functions. + * It is not a complete path component, i.e. the path + * component continues after the gmt-token. */ - state->component_was_mangled = false; - } else if (mangle_is_mangled(state->name, state->conn->params)) { - state->component_was_mangled = true; + return false; } - /* - * Now we need to recursively match the name against the real - * directory structure. - */ + tm.tm_isdst = -1; + t = timegm(&tm); + unix_to_nt_time(twrp, t); - /* - * Match each part of the path name separately, trying the names - * as is first, then trying to scan the directory for matching names. - */ + DBG_DEBUG("Extracted @GMT-Timestamp %s\n", + nt_time_string(talloc_tos(), *twrp)); - for (; state->name ; state->name = (state->end ? state->end + 1:(char *)NULL)) { - status = unix_convert_step(state); - if (!NT_STATUS_IS_OK(status)) { - if (NT_STATUS_EQUAL(status, NT_STATUS_NO_MEMORY)) { - goto err; - } - goto fail; - } - if (state->done) { - goto done; - } + *_start = start; + + if (end[0] == '/') { + end += 1; } + *_next_component = end; - /* - * Cache the full path. Don't cache a name with mangled or wildcard - * components as this can change the size. - */ + return true; +} + +bool extract_snapshot_token(char *fname, NTTIME *twrp) +{ + const char *start = NULL; + const char *next = NULL; + size_t remaining; + bool found; - if(!state->component_was_mangled) { - stat_cache_add(state->orig_path, - state->smb_fname->base_name, - state->smb_fname->twrp, - state->case_sensitive); + found = find_snapshot_token(fname, &start, &next, twrp); + if (!found) { + return false; } - /* - * The name has been resolved. - */ + remaining = strlen(next); + memmove(discard_const_p(char, start), next, remaining+1); - done: - /* Add back the stream if one was stripped off originally. */ - if (state->stream != NULL) { - state->smb_fname->stream_name = state->stream; + return true; +} - /* Check path now that the base_name has been converted. */ - status = build_stream_path(state->mem_ctx, state->conn, state->smb_fname); - if (!NT_STATUS_IS_OK(status)) { - goto fail; - } +/* + * Strip a valid @GMT-token from any incoming filename path, + * adding any NTTIME encoded in the pathname into the + * twrp field of the passed in smb_fname. + * + * Valid @GMT-tokens look like @GMT-YYYY-MM-DD-HH-MM-SS + * at the *start* of a pathname component. + * + * If twrp is passed in then smb_fname->twrp is set to that + * value, and the @GMT-token part of the filename is removed + * and does not change the stored smb_fname->twrp. + * + */ + +NTSTATUS canonicalize_snapshot_path(struct smb_filename *smb_fname, + uint32_t ucf_flags, + NTTIME twrp) +{ + bool found; + + if (twrp != 0) { + smb_fname->twrp = twrp; } - DBG_DEBUG("Conversion finished [%s] -> [%s]\n", - state->orig_path, smb_fname_str_dbg(state->smb_fname)); + if (!(ucf_flags & UCF_GMT_PATHNAME)) { + return NT_STATUS_OK; + } - TALLOC_FREE(state->dirpath); - *smb_fname_out = state->smb_fname; - return NT_STATUS_OK; - fail: - DBG_DEBUG("Conversion failed: dirpath [%s] name [%s]\n", - state->dirpath, state->name); - if ((state->dirpath != NULL) && !ISDOT(state->dirpath)) { - state->smb_fname->base_name = talloc_asprintf( - state->smb_fname, - "%s/%s", - state->dirpath, - state->name); - } else { - state->smb_fname->base_name = talloc_strdup( - state->smb_fname, state->name); + found = extract_snapshot_token(smb_fname->base_name, &twrp); + if (!found) { + return NT_STATUS_OK; } - if (state->smb_fname->base_name == NULL) { - DBG_ERR("talloc_asprintf failed\n"); - status = NT_STATUS_NO_MEMORY; - goto err; + + if (smb_fname->twrp == 0) { + smb_fname->twrp = twrp; } - *smb_fname_out = state->smb_fname; - TALLOC_FREE(state->dirpath); - return status; - err: - TALLOC_FREE(state->smb_fname); - return status; + return NT_STATUS_OK; } -/**************************************************************************** - Ensure a path is not vetoed. -****************************************************************************/ - -static NTSTATUS check_veto_path(connection_struct *conn, - const struct smb_filename *smb_fname) +static bool strnorm(char *s, int case_default) { - const char *name = smb_fname->base_name; - - if (IS_VETO_PATH(conn, name)) { - /* Is it not dot or dot dot. */ - if (!(ISDOT(name) || ISDOTDOT(name))) { - DEBUG(5,("check_veto_path: file path name %s vetoed\n", - name)); - return map_nt_error_from_unix(ENOENT); - } - } - return NT_STATUS_OK; + if (case_default == CASE_UPPER) + return strupper_m(s); + else + return strlower_m(s); } -/**************************************************************************** - Check a filename - possibly calling check_reduced_name. - This is called by every routine before it allows an operation on a filename. - It does any final confirmation necessary to ensure that the filename is - a valid one for the user to access. -****************************************************************************/ +/* + * Utility function to normalize case on an incoming client filename + * if required on this connection struct. + * Performs an in-place case conversion guaranteed to stay the same size. + */ -static NTSTATUS check_name(connection_struct *conn, - const struct smb_filename *smb_fname) +static NTSTATUS normalize_filename_case(connection_struct *conn, + char *filename, + uint32_t ucf_flags) { - NTSTATUS status = check_veto_path(conn, smb_fname); + bool ok; - if (!NT_STATUS_IS_OK(status)) { - return status; + if (ucf_flags & UCF_POSIX_PATHNAMES) { + /* + * POSIX never normalizes filename case. + */ + return NT_STATUS_OK; } - - if (!lp_widelinks(SNUM(conn)) || !lp_follow_symlinks(SNUM(conn))) { - status = check_reduced_name(conn, NULL, smb_fname); - if (!NT_STATUS_IS_OK(status)) { - DEBUG(5,("check_name: name %s failed with %s\n", - smb_fname->base_name, - nt_errstr(status))); - return status; - } + if (!conn->case_sensitive) { + return NT_STATUS_OK; + } + if (conn->case_preserve) { + return NT_STATUS_OK; + } + if (conn->short_case_preserve) { + return NT_STATUS_OK; + } + ok = strnorm(filename, lp_default_case(SNUM(conn))); + if (!ok) { + return NT_STATUS_INVALID_PARAMETER; } - return NT_STATUS_OK; } @@ -1580,41 +430,6 @@ NTSTATUS get_real_filename_full_scan_at(struct files_struct *dirfsp, return NT_STATUS_OBJECT_NAME_NOT_FOUND; } -NTSTATUS get_real_filename_full_scan(connection_struct *conn, - const char *path, - const char *name, - bool mangled, - TALLOC_CTX *mem_ctx, - char **found_name) -{ - struct smb_filename *smb_dname = NULL; - NTSTATUS status; - - /* handle null paths */ - if ((path == NULL) || (*path == 0)) { - path = "."; - } - - status = synthetic_pathref( - talloc_tos(), - conn->cwd_fsp, - path, - NULL, - NULL, - 0, - 0, - &smb_dname); - if (!NT_STATUS_IS_OK(status)) { - return status; - } - - status = get_real_filename_full_scan_at( - smb_dname->fsp, name, mangled, mem_ctx, found_name); - - TALLOC_FREE(smb_dname); - return status; -} - /**************************************************************************** Wrapper around the vfs get_real_filename and the full directory scan fallback. @@ -1703,172 +518,6 @@ static bool get_real_filename_cache_key( return true; } -static NTSTATUS get_real_filename(connection_struct *conn, - struct smb_filename *path, - const char *name, - TALLOC_CTX *mem_ctx, - char **found_name) -{ - struct smb_filename *smb_dname = NULL; - NTSTATUS status; - - smb_dname = cp_smb_filename_nostream(talloc_tos(), path); - if (smb_dname == NULL) { - return NT_STATUS_NO_MEMORY; - } - -again: - status = openat_pathref_fsp(conn->cwd_fsp, smb_dname); - - if (NT_STATUS_EQUAL(status, NT_STATUS_OBJECT_NAME_NOT_FOUND) && - S_ISLNK(smb_dname->st.st_ex_mode)) { - status = NT_STATUS_STOPPED_ON_SYMLINK; - } - - if (NT_STATUS_EQUAL(status, NT_STATUS_OBJECT_NAME_NOT_FOUND) && - (smb_dname->twrp != 0)) { - /* - * Retry looking at the non-snapshot path, copying the - * fallback mechanism from vfs_shadow_copy2.c when - * shadow_copy2_convert() fails. This path-based - * routine get_real_filename() should go away and be - * replaced with a fd-based one, so spoiling it with a - * shadow_copy2 specific mechanism should not be too - * bad. - */ - smb_dname->twrp = 0; - goto again; - } - - if (!NT_STATUS_IS_OK(status)) { - DBG_DEBUG("openat_pathref_fsp(%s) failed: %s\n", - smb_fname_str_dbg(smb_dname), - nt_errstr(status)); - - /* - * ENOTDIR and ELOOP both map to - * NT_STATUS_OBJECT_PATH_NOT_FOUND in the filename - * walk. - */ - if (NT_STATUS_EQUAL(status, NT_STATUS_NOT_A_DIRECTORY) || - NT_STATUS_EQUAL(status, NT_STATUS_STOPPED_ON_SYMLINK)) { - status = NT_STATUS_OBJECT_PATH_NOT_FOUND; - } - - return status; - } - - status = get_real_filename_at( - smb_dname->fsp, name, mem_ctx, found_name); - TALLOC_FREE(smb_dname); - return status; -} - -static NTSTATUS build_stream_path(TALLOC_CTX *mem_ctx, - connection_struct *conn, - struct smb_filename *smb_fname) -{ - NTSTATUS status; - unsigned int i, num_streams = 0; - struct stream_struct *streams = NULL; - struct smb_filename *pathref = NULL; - - if (SMB_VFS_STAT(conn, smb_fname) == 0) { - DEBUG(10, ("'%s' exists\n", smb_fname_str_dbg(smb_fname))); - return NT_STATUS_OK; - } - - if (errno != ENOENT) { - DEBUG(10, ("vfs_stat failed: %s\n", strerror(errno))); - status = map_nt_error_from_unix(errno); - goto fail; - } - - if (smb_fname->fsp == NULL) { - status = synthetic_pathref(mem_ctx, - conn->cwd_fsp, - smb_fname->base_name, - NULL, - NULL, - smb_fname->twrp, - smb_fname->flags, - &pathref); - if (!NT_STATUS_IS_OK(status)) { - if (NT_STATUS_EQUAL(status, - NT_STATUS_OBJECT_NAME_NOT_FOUND)) { - TALLOC_FREE(pathref); - SET_STAT_INVALID(smb_fname->st); - return NT_STATUS_OK; - } - DBG_DEBUG("synthetic_pathref failed: %s\n", - nt_errstr(status)); - goto fail; - } - } else { - pathref = smb_fname; - } - - /* Fall back to a case-insensitive scan of all streams on the file. */ - status = vfs_fstreaminfo(pathref->fsp, mem_ctx, - &num_streams, &streams); - if (NT_STATUS_EQUAL(status, NT_STATUS_OBJECT_NAME_NOT_FOUND)) { - SET_STAT_INVALID(smb_fname->st); - TALLOC_FREE(pathref); - return NT_STATUS_OK; - } - - if (!NT_STATUS_IS_OK(status)) { - DEBUG(10, ("vfs_fstreaminfo failed: %s\n", nt_errstr(status))); - goto fail; - } - - for (i=0; istream_name, - streams[i].name, - conn->case_sensitive); - - DBG_DEBUG("comparing [%s] and [%s]: %sequal\n", - smb_fname->stream_name, - streams[i].name, - equal ? "" : "not "); - - if (equal) { - break; - } - } - - /* Couldn't find the stream. */ - if (i == num_streams) { - SET_STAT_INVALID(smb_fname->st); - TALLOC_FREE(pathref); - TALLOC_FREE(streams); - return NT_STATUS_OK; - } - - DEBUG(10, ("case insensitive stream. requested: %s, actual: %s\n", - smb_fname->stream_name, streams[i].name)); - - - TALLOC_FREE(smb_fname->stream_name); - smb_fname->stream_name = talloc_strdup(smb_fname, streams[i].name); - if (smb_fname->stream_name == NULL) { - status = NT_STATUS_NO_MEMORY; - goto fail; - } - - SET_STAT_INVALID(smb_fname->st); - - if (SMB_VFS_STAT(conn, smb_fname) == 0) { - DEBUG(10, ("'%s' exists\n", smb_fname_str_dbg(smb_fname))); - } - status = NT_STATUS_OK; - fail: - TALLOC_FREE(pathref); - TALLOC_FREE(streams); - return status; -} - /* * Lightweight function to just get last component * for rename / enumerate directory calls. diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h index bc305bce2966..5ac0f7139582 100644 --- a/source3/smbd/proto.h +++ b/source3/smbd/proto.h @@ -350,22 +350,10 @@ NTSTATUS sync_file(connection_struct *conn, files_struct *fsp, bool write_throug uint32_t ucf_flags_from_smb_request(struct smb_request *req); uint32_t filename_create_ucf_flags(struct smb_request *req, uint32_t create_disposition); -NTSTATUS unix_convert(TALLOC_CTX *ctx, - connection_struct *conn, - const char *orig_path, - NTTIME twrp, - struct smb_filename **smb_fname, - uint32_t ucf_flags); bool extract_snapshot_token(char *fname, NTTIME *twrp); NTSTATUS canonicalize_snapshot_path(struct smb_filename *smb_fname, uint32_t ucf_flags, NTTIME twrp); -NTSTATUS get_real_filename_full_scan(connection_struct *conn, - const char *path, - const char *name, - bool mangled, - TALLOC_CTX *mem_ctx, - char **found_name); NTSTATUS get_real_filename_full_scan_at(struct files_struct *dirfsp, const char *name, bool mangled, -- 2.34.1 From aa861f53968ae753d6eecd3fd6ed08f303dacf73 Mon Sep 17 00:00:00 2001 From: Andreas Schneider Date: Wed, 10 Aug 2022 08:41:24 +0200 Subject: [PATCH 34/55] s3:util: Initialize json_object structures so we can call json_free() CID 1507863 CID 1507865 CID 1507866 CID 1507867 CID 1507868 CID 1507869 CID 1507870 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15140 Signed-off-by: Andreas Schneider Reviewed-by: Jeremy Allison BUG: https://bugzilla.samba.org/show_bug.cgi?id=15146 (cherry picked from commit 4a702cddaebf7e616706e0c728685567e141b493) --- source3/utils/status_json.c | 31 ++++++++++++++++++++++++------- 1 file changed, 24 insertions(+), 7 deletions(-) diff --git a/source3/utils/status_json.c b/source3/utils/status_json.c index e2798e3e392c..1f6357845413 100644 --- a/source3/utils/status_json.c +++ b/source3/utils/status_json.c @@ -189,7 +189,12 @@ int add_profile_item_to_json(struct traverse_state *state, const char *key, uintmax_t value) { - struct json_object section_json, subsection_json; + struct json_object section_json = { + .valid = false, + }; + struct json_object subsection_json = { + .valid = false, + }; int result = 0; section_json = json_get_object(&state->root_json, section); @@ -796,8 +801,12 @@ static int add_open_to_json(struct json_object *parent_json, uint32_t lease_type, const char *uid_str) { - struct json_object sub_json; - struct json_object opens_json; + struct json_object sub_json = { + .valid = false, + }; + struct json_object opens_json = { + .valid = false, + }; struct timeval_buf tv_buf; int result = 0; char *timestr; @@ -936,8 +945,12 @@ int print_share_mode_json(struct traverse_state *state, uint32_t lease_type, const char *filename) { - struct json_object locks_json; - struct json_object file_json; + struct json_object locks_json = { + .valid = false, + }; + struct json_object file_json = { + .valid = false, + }; char *key = NULL; int result = 0; @@ -1013,8 +1026,12 @@ static int add_lock_to_json(struct json_object *parent_json, intmax_t start, intmax_t size) { - struct json_object sub_json; - struct json_object locks_json; + struct json_object sub_json = { + .valid = false, + }; + struct json_object locks_json = { + .valid = false, + }; const char *flavour_str; int result = 0; -- 2.34.1 From 0c033a2359ba48a5424c50bbee53fefb77a45aa8 Mon Sep 17 00:00:00 2001 From: Andreas Schneider Date: Wed, 10 Aug 2022 08:51:06 +0200 Subject: [PATCH 35/55] s3:utils: Fix NULL check CID 1507864 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15140 Signed-off-by: Andreas Schneider Reviewed-by: Jeremy Allison Autobuild-User(master): Jeremy Allison Autobuild-Date(master): Fri Aug 12 21:50:23 UTC 2022 on sn-devel-184 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15146 (cherry picked from commit a38fad29803f9e2891b2264ac3258394152e8deb) --- source3/utils/status_json.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source3/utils/status_json.c b/source3/utils/status_json.c index 1f6357845413..79cb1dfe1e41 100644 --- a/source3/utils/status_json.c +++ b/source3/utils/status_json.c @@ -1215,7 +1215,7 @@ bool print_notify_rec_json(struct traverse_state *state, goto failure; } subdir_filter = talloc_asprintf(tmp_ctx, "%u", instance->subdir_filter); - if (filter == NULL) { + if (subdir_filter == NULL) { goto failure; } result = json_add_string(&sub_json, "subdir_filter", subdir_filter); -- 2.34.1 From e7076315ff21bb31b1c188c0bae01d1599bc6af4 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 12 Aug 2022 19:07:39 +0200 Subject: [PATCH 36/55] s3:tests: add a lot more tests to test_symlink_traversal_smb2.sh We now also test more path components checking the difference between OBJECT_NAME_NOT_FOUND and OBJECT_PATH_NOT_FOUND. We also test with symlinks within the path instead of only checking symlinks as final path components (at least for the dirfsp part). This ensures the following commits won't introduce regressions when adding the openat2(RESOLVE_NO_SYMLINK) optimization. Signed-off-by: Stefan Metzmacher Reviewed-by: Volker Lendecke BUG: https://bugzilla.samba.org/show_bug.cgi?id=15146 (cherry picked from commit 085f14857531dab179af66a69962486c7dd2592c) --- .../tests/test_symlink_traversal_smb2.sh | 116 ++++++++++++++++++ 1 file changed, 116 insertions(+) diff --git a/source3/script/tests/test_symlink_traversal_smb2.sh b/source3/script/tests/test_symlink_traversal_smb2.sh index eadd7592de59..971d53442161 100755 --- a/source3/script/tests/test_symlink_traversal_smb2.sh +++ b/source3/script/tests/test_symlink_traversal_smb2.sh @@ -50,7 +50,11 @@ do_cleanup() ( #subshell. cd "$share_test_dir" || return + rm -f "symlink_to_dot" rm -f "file_exists" + rm -f "symlink_to_file_exists" + rm -rf "dir_exists" + rm -f "symlink_to_dir_exists" rm -f "symlink_noexist" rm -f "symlink_file_outside_share" rm -f "symlink_file_outside_share_noexist" @@ -93,7 +97,13 @@ chmod 0 "$dir_outside_share_noperms" ( #subshell. cd "$share_test_dir" || return + ln -s "." "symlink_to_dot" touch "file_exists" + ln -s "file_exists" "symlink_to_file_exists" + mkdir "dir_exists" + ln -s "dir_exists" "symlink_to_dir_exists" + touch "dir_exists/subfile_exists" + mkdir "dir_exists/subdir_exists" ln -s "noexist" "symlink_noexist" ln -s "$file_outside_share" "symlink_file_outside_share" ln -s "$file_outside_share_noexist" "symlink_file_outside_share_noexist" @@ -107,7 +117,13 @@ chmod 0 "$dir_outside_share_noperms" ( #subshell cd "emptydir" || return + ln -s "." "symlink_to_dot" touch "file_exists" + ln -s "file_exists" "symlink_to_file_exists" + mkdir "dir_exists" + ln -s "dir_exists" "symlink_to_dir_exists" + touch "dir_exists/subfile_exists" + mkdir "dir_exists/subdir_exists" ln -s "noexist" "symlink_noexist" ln -s "$file_outside_share" "symlink_file_outside_share" ln -s "$file_outside_share_noexist" "symlink_file_outside_share_noexist" @@ -126,6 +142,8 @@ chmod 0 "$dir_outside_share_noperms" touch "dir_inside_share_noperms/noperm_file_exists" chmod 0 "dir_inside_share_noperms" ln -s "dir_inside_share_noperms" "symlink_dir_inside_share_noperms" + mkdir "dir_inside_share_noperms/noperm_subdir_exists" + touch "dir_inside_share_noperms/noperm_subdir_exists/noperm_subdir_file_exists" ) # @@ -179,25 +197,33 @@ test_symlink_traversal_SMB2_onename() # smbclient_expect_error "get" "$name" "" "NT_STATUS_OBJECT_NAME_NOT_FOUND" || return 1 smbclient_expect_error "get" "$name/noexist" "" "NT_STATUS_OBJECT_PATH_NOT_FOUND" || return 1 + smbclient_expect_error "get" "$name/noexistsdir/noexist" "" "NT_STATUS_OBJECT_PATH_NOT_FOUND" || return 1 smbclient_expect_error "get" "$name/*" "" "NT_STATUS_OBJECT_NAME_INVALID" || return 1 smbclient_expect_error "get" "$name/*/noexist" "" "NT_STATUS_OBJECT_NAME_INVALID" || return 1 + smbclient_expect_error "get" "$name/*/noexistsdir/noexist" "" "NT_STATUS_OBJECT_NAME_INVALID" || return 1 # Now in subdirectory emptydir smbclient_expect_error "get" "emptydir/$name" "" "NT_STATUS_OBJECT_NAME_NOT_FOUND" || return 1 smbclient_expect_error "get" "emptydir/$name/noexist" "" "NT_STATUS_OBJECT_PATH_NOT_FOUND" || return 1 + smbclient_expect_error "get" "emptydir/$name/noexistsdir/noexist" "" "NT_STATUS_OBJECT_PATH_NOT_FOUND" || return 1 smbclient_expect_error "get" "emptydir/$name/*" "" "NT_STATUS_OBJECT_NAME_INVALID" || return 1 smbclient_expect_error "get" "emptydir/$name/*/noexist" "" "NT_STATUS_OBJECT_NAME_INVALID" || return 1 + smbclient_expect_error "get" "emptydir/$name/*/noexistsdir/noexist" "" "NT_STATUS_OBJECT_NAME_INVALID" || return 1 # # ls commands. # smbclient_expect_error "ls" "$name" "" "NT_STATUS_NO_SUCH_FILE" || return 1 smbclient_expect_error "ls" "$name/noexist" "" "NT_STATUS_OBJECT_NAME_NOT_FOUND" || return 1 + smbclient_expect_error "ls" "$name/noexistsdir/noexist" "" "NT_STATUS_OBJECT_PATH_NOT_FOUND" || return 1 smbclient_expect_error "ls" "$name/*" "" "NT_STATUS_OBJECT_NAME_NOT_FOUND" || return 1 smbclient_expect_error "ls" "$name/*/noexist" "" "NT_STATUS_OBJECT_NAME_INVALID" || return 1 + smbclient_expect_error "ls" "$name/*/noexistsdir/noexist" "" "NT_STATUS_OBJECT_NAME_INVALID" || return 1 # Now in subdirectory emptydir smbclient_expect_error "ls" "emptydir/$name" "" "NT_STATUS_NO_SUCH_FILE" || return 1 smbclient_expect_error "ls" "emptydir/$name/noexist" "" "NT_STATUS_OBJECT_NAME_NOT_FOUND" || return 1 + smbclient_expect_error "ls" "emptydir/$name/noexistsdir/noexist" "" "NT_STATUS_OBJECT_PATH_NOT_FOUND" || return 1 smbclient_expect_error "ls" "emptydir/$name/*" "" "NT_STATUS_OBJECT_NAME_NOT_FOUND" || return 1 smbclient_expect_error "ls" "emptydir/$name/*/noexist" "" "NT_STATUS_OBJECT_NAME_INVALID" || return 1 + smbclient_expect_error "ls" "emptydir/$name/*/noexistsdir/noexist" "" "NT_STATUS_OBJECT_NAME_INVALID" || return 1 # # del commands. @@ -215,9 +241,21 @@ test_symlink_traversal_SMB2_onename() # smbclient_expect_error "rename" "file_exists" "$name" "NT_STATUS_OBJECT_NAME_NOT_FOUND" || return 1 smbclient_expect_error "rename" "file_exists" "$name/noexist" "NT_STATUS_OBJECT_PATH_NOT_FOUND" || return 1 + smbclient_expect_error "rename" "symlink_to_file_exists" "$name" "NT_STATUS_OBJECT_NAME_NOT_FOUND" || return 1 + smbclient_expect_error "rename" "symlink_to_file_exists" "$name/noexist" "NT_STATUS_OBJECT_PATH_NOT_FOUND" || return 1 + smbclient_expect_error "rename" "dir_exists" "$name" "NT_STATUS_OBJECT_NAME_NOT_FOUND" || return 1 + smbclient_expect_error "rename" "dir_exists" "$name/noexist" "NT_STATUS_OBJECT_PATH_NOT_FOUND" || return 1 + smbclient_expect_error "rename" "symlink_to_dir_exists" "$name" "NT_STATUS_OBJECT_NAME_NOT_FOUND" || return 1 + smbclient_expect_error "rename" "symlink_to_dir_exists" "$name/noexist" "NT_STATUS_OBJECT_PATH_NOT_FOUND" || return 1 # Now in subdirectory emptydir smbclient_expect_error "rename" "file_exists" "emptydir/$name" "NT_STATUS_OBJECT_NAME_NOT_FOUND" || return 1 smbclient_expect_error "rename" "file_exists" "emptydir/$name/noexist" "NT_STATUS_OBJECT_PATH_NOT_FOUND" || return 1 + smbclient_expect_error "rename" "symlink_to_file_exists" "emptydir/$name" "NT_STATUS_OBJECT_NAME_NOT_FOUND" || return 1 + smbclient_expect_error "rename" "symlink_to_file_exists" "emptydir/$name/noexist" "NT_STATUS_OBJECT_PATH_NOT_FOUND" || return 1 + smbclient_expect_error "rename" "dir_exists" "emptydir/$name" "NT_STATUS_OBJECT_NAME_NOT_FOUND" || return 1 + smbclient_expect_error "rename" "dir_exists" "emptydir/$name/noexist" "NT_STATUS_OBJECT_PATH_NOT_FOUND" || return 1 + smbclient_expect_error "rename" "symlink_to_dir_exists" "emptydir/$name" "NT_STATUS_OBJECT_NAME_NOT_FOUND" || return 1 + smbclient_expect_error "rename" "symlink_to_dir_exists" "emptydir/$name/noexist" "NT_STATUS_OBJECT_PATH_NOT_FOUND" || return 1 fi return 0 } @@ -234,6 +272,80 @@ test_symlink_traversal_SMB2() test_symlink_traversal_SMB2_onename "symlink_dir_outside_share_noexist" "no rename" || return 1 test_symlink_traversal_SMB2_onename "symlink_file_outside_share_noperms" "do rename" || return 1 test_symlink_traversal_SMB2_onename "symlink_dir_outside_share_noperms" "do rename" || return 1 + + # Note the share has 'follow symlinks = yes' + smbclient_expect_error "ls" "." "" "NT_STATUS_NO_SUCH_FILE" || return 1 + smbclient_expect_error "ls" "noexist1" "" "NT_STATUS_NO_SUCH_FILE" || return 1 + smbclient_expect_error "ls" "noexist1/noexist2" "" "NT_STATUS_OBJECT_NAME_NOT_FOUND" || return 1 + smbclient_expect_error "ls" "noexist1/noexist2/noexist3" "" "NT_STATUS_OBJECT_PATH_NOT_FOUND" || return 1 + smbclient_expect_error "ls" "symlink_to_dot" "" "NT_STATUS_OK" || return 1 + smbclient_expect_error "ls" "symlink_to_dot/noexist1" "" "NT_STATUS_NO_SUCH_FILE" || return 1 + smbclient_expect_error "ls" "symlink_to_dot/noexist1/noexist2" "" "NT_STATUS_OBJECT_NAME_NOT_FOUND" || return 1 + smbclient_expect_error "ls" "symlink_to_dot/noexist1/noexist2/noexist3" "" "NT_STATUS_OBJECT_PATH_NOT_FOUND" || return 1 + smbclient_expect_error "ls" "file_exists" "" "NT_STATUS_OK" || return 1 + smbclient_expect_error "ls" "file_exists/noexist1" "" "NT_STATUS_NOT_A_DIRECTORY" || return 1 + smbclient_expect_error "ls" "file_exists/noexist1/noexist2" "" "NT_STATUS_OBJECT_PATH_NOT_FOUND" || return 1 + smbclient_expect_error "ls" "file_exists/noexist1/noexist2/noexist3" "" "NT_STATUS_OBJECT_PATH_NOT_FOUND" || return 1 + smbclient_expect_error "ls" "symlink_to_file_exists" "" "NT_STATUS_OK" || return 1 + smbclient_expect_error "ls" "symlink_to_file_exists/noexist1" "" "NT_STATUS_NOT_A_DIRECTORY" || return 1 + smbclient_expect_error "ls" "symlink_to_file_exists/noexist1/noexist2" "" "NT_STATUS_OBJECT_PATH_NOT_FOUND" || return 1 + smbclient_expect_error "ls" "symlink_to_file_exists/noexist1/noexist2/noexist" "" "NT_STATUS_OBJECT_PATH_NOT_FOUND" || return 1 + smbclient_expect_error "ls" "dir_exists" "" "NT_STATUS_OK" || return 1 + smbclient_expect_error "ls" "dir_exists/noexist1" "" "NT_STATUS_NO_SUCH_FILE" || return 1 + smbclient_expect_error "ls" "dir_exists/noexist1/noexist2" "" "NT_STATUS_OBJECT_NAME_NOT_FOUND" || return 1 + smbclient_expect_error "ls" "dir_exists/noexist1/noexist2/noexist3" "" "NT_STATUS_OBJECT_PATH_NOT_FOUND" || return 1 + smbclient_expect_error "ls" "dir_exists/subfile_exists" "" "NT_STATUS_OK" || return 1 + smbclient_expect_error "ls" "dir_exists/subfile_exists/noexist1" "" "NT_STATUS_NOT_A_DIRECTORY" || return 1 + smbclient_expect_error "ls" "dir_exists/subfile_exists/noexist1/noexist2" "" "NT_STATUS_OBJECT_PATH_NOT_FOUND" || return 1 + smbclient_expect_error "ls" "dir_exists/subfile_exists/noexist1/noexist2/noexist3" "" "NT_STATUS_OBJECT_PATH_NOT_FOUND" || return 1 + smbclient_expect_error "ls" "dir_exists/subdir_exists" "" "NT_STATUS_OK" || return 1 + smbclient_expect_error "ls" "dir_exists/subdir_exists/noexist1" "" "NT_STATUS_NO_SUCH_FILE" || return 1 + smbclient_expect_error "ls" "dir_exists/subdir_exists/noexist1/noexist2" "" "NT_STATUS_OBJECT_NAME_NOT_FOUND" || return 1 + smbclient_expect_error "ls" "dir_exists/subdir_exists/noexist1/noexist2/noexist3" "" "NT_STATUS_OBJECT_PATH_NOT_FOUND" || return 1 + smbclient_expect_error "ls" "symlink_to_dir_exists" "" "NT_STATUS_OK" || return 1 + smbclient_expect_error "ls" "symlink_to_dir_exists/noexist1" "" "NT_STATUS_NO_SUCH_FILE" || return 1 + smbclient_expect_error "ls" "symlink_to_dir_exists/noexist1/noexist2" "" "NT_STATUS_OBJECT_NAME_NOT_FOUND" || return 1 + smbclient_expect_error "ls" "symlink_to_dir_exists/noexist1/noexist2/noexist3" "" "NT_STATUS_OBJECT_PATH_NOT_FOUND" || return 1 + smbclient_expect_error "ls" "symlink_to_dir_exists/subfile_exists" "" "NT_STATUS_OK" || return 1 + smbclient_expect_error "ls" "symlink_to_dir_exists/subfile_exists/noexist1" "" "NT_STATUS_NOT_A_DIRECTORY" || return 1 + smbclient_expect_error "ls" "symlink_to_dir_exists/subfile_exists/noexist1/noexist2" "" "NT_STATUS_OBJECT_PATH_NOT_FOUND" || return 1 + smbclient_expect_error "ls" "symlink_to_dir_exists/subfile_exists/noexist1/noexist2/noexist3" "" "NT_STATUS_OBJECT_PATH_NOT_FOUND" || return 1 + smbclient_expect_error "ls" "symlink_to_dir_exists/subdir_exists" "" "NT_STATUS_OK" || return 1 + smbclient_expect_error "ls" "symlink_to_dir_exists/subdir_exists/noexist1" "" "NT_STATUS_NO_SUCH_FILE" || return 1 + smbclient_expect_error "ls" "symlink_to_dir_exists/subdir_exists/noexist1/noexist2" "" "NT_STATUS_OBJECT_NAME_NOT_FOUND" || return 1 + smbclient_expect_error "ls" "symlink_to_dir_exists/subdir_exists/noexist1/noexist2/noexist3" "" "NT_STATUS_OBJECT_PATH_NOT_FOUND" || return 1 + + smbclient_expect_error "get" "." "" "NT_STATUS_OBJECT_NAME_INVALID" || return 1 + smbclient_expect_error "get" "noexist1" "" "NT_STATUS_OBJECT_NAME_NOT_FOUND" || return 1 + smbclient_expect_error "get" "noexist1/noexist2" "" "NT_STATUS_OBJECT_PATH_NOT_FOUND" || return 1 + smbclient_expect_error "get" "symlink_to_dot" "" "NT_STATUS_FILE_IS_A_DIRECTORY" || return 1 + smbclient_expect_error "get" "symlink_to_dot/noexist1" "" "NT_STATUS_OBJECT_NAME_NOT_FOUND" || return 1 + smbclient_expect_error "get" "symlink_to_dot/noexist1/noexist2" "" "NT_STATUS_OBJECT_PATH_NOT_FOUND" || return 1 + smbclient_expect_error "get" "file_exists" "" "NT_STATUS_OK" || return 1 + smbclient_expect_error "get" "file_exists/noexist1" "" "NT_STATUS_OBJECT_PATH_NOT_FOUND" || return 1 + smbclient_expect_error "get" "file_exists/noexist1/noexist2" "" "NT_STATUS_OBJECT_PATH_NOT_FOUND" || return 1 + smbclient_expect_error "get" "symlink_to_file_exists" "" "NT_STATUS_OK" || return 1 + smbclient_expect_error "get" "symlink_to_file_exists/noexist1" "" "NT_STATUS_OBJECT_PATH_NOT_FOUND" || return 1 + smbclient_expect_error "get" "symlink_to_file_exists/noexist1/noexist2" "" "NT_STATUS_OBJECT_PATH_NOT_FOUND" || return 1 + smbclient_expect_error "get" "dir_exists" "" "NT_STATUS_FILE_IS_A_DIRECTORY" || return 1 + smbclient_expect_error "get" "dir_exists/noexist1" "" "NT_STATUS_OBJECT_NAME_NOT_FOUND" || return 1 + smbclient_expect_error "get" "dir_exists/noexist1/noexist2" "" "NT_STATUS_OBJECT_PATH_NOT_FOUND" || return 1 + smbclient_expect_error "get" "dir_exists/subfile_exists" "" "NT_STATUS_OK" || return 1 + smbclient_expect_error "get" "dir_exists/subfile_exists/noexist1" "" "NT_STATUS_OBJECT_PATH_NOT_FOUND" || return 1 + smbclient_expect_error "get" "dir_exists/subfile_exists/noexist1/noexist2" "" "NT_STATUS_OBJECT_PATH_NOT_FOUND" || return 1 + smbclient_expect_error "get" "dir_exists/subdir_exists" "" "NT_STATUS_FILE_IS_A_DIRECTORY" || return 1 + smbclient_expect_error "get" "dir_exists/subdir_exists/noexist1" "" "NT_STATUS_OBJECT_NAME_NOT_FOUND" || return 1 + smbclient_expect_error "get" "dir_exists/subdir_exists/noexist1/noexist2" "" "NT_STATUS_OBJECT_PATH_NOT_FOUND" || return 1 + smbclient_expect_error "get" "symlink_to_dir_exists" "" "NT_STATUS_FILE_IS_A_DIRECTORY" || return 1 + smbclient_expect_error "get" "symlink_to_dir_exists/noexist1" "" "NT_STATUS_OBJECT_NAME_NOT_FOUND" || return 1 + smbclient_expect_error "get" "symlink_to_dir_exists/noexist1/noexist2" "" "NT_STATUS_OBJECT_PATH_NOT_FOUND" || return 1 + smbclient_expect_error "get" "symlink_to_dir_exists/subfile_exists" "" "NT_STATUS_OK" || return 1 + smbclient_expect_error "get" "symlink_to_dir_exists/subfile_exists/noexist1" "" "NT_STATUS_OBJECT_PATH_NOT_FOUND" || return 1 + smbclient_expect_error "get" "symlink_to_dir_exists/subfile_exists/noexist1/noexist2" "" "NT_STATUS_OBJECT_PATH_NOT_FOUND" || return 1 + smbclient_expect_error "get" "symlink_to_dir_exists/subdir_exists" "" "NT_STATUS_FILE_IS_A_DIRECTORY" || return 1 + smbclient_expect_error "get" "symlink_to_dir_exists/subdir_exists/noexist1" "" "NT_STATUS_OBJECT_NAME_NOT_FOUND" || return 1 + smbclient_expect_error "get" "symlink_to_dir_exists/subdir_exists/noexist1/noexist2" "" "NT_STATUS_OBJECT_PATH_NOT_FOUND" || return 1 + # # Test paths within share with no permissions. # @@ -249,6 +361,10 @@ test_symlink_traversal_SMB2() # But can list the directory with no perms and the symlink to it. smbclient_expect_error "ls" "dir_inside_share_noperms" "" "NT_STATUS_OK" || return 1 smbclient_expect_error "ls" "symlink_dir_inside_share_noperms" "" "NT_STATUS_OK" || return 1 + # Check that 'get' on non existing subpaths also returns NT_STATUS_ACCESS_DENIED + smbclient_expect_error "get" "symlink_dir_inside_share_noperms/noperm_file_exists/_none_" "" "NT_STATUS_ACCESS_DENIED" || return 1 + smbclient_expect_error "get" "symlink_dir_inside_share_noperms/noperm_subdir_exists/noperm_subdir_file_exists" "" "NT_STATUS_ACCESS_DENIED" || return 1 + smbclient_expect_error "get" "symlink_dir_inside_share_noperms/noperm_subdir_exists/noperm_subdir_file_exists/_none_" "" "NT_STATUS_ACCESS_DENIED" || return 1 } testit "symlink_traversal_SMB2" \ -- 2.34.1 From 3b6826cf0edd8788d23e333224b4be33b7602d38 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Thu, 11 Aug 2022 00:41:28 +0200 Subject: [PATCH 37/55] wafsamba: allow cflags for CHECK_TYPE[_IN]() Signed-off-by: Stefan Metzmacher Reviewed-by: Volker Lendecke BUG: https://bugzilla.samba.org/show_bug.cgi?id=15146 (cherry picked from commit 2b51bad747551605ba3b70ac3b692107a0cd7aad) --- buildtools/wafsamba/samba_autoconf.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/buildtools/wafsamba/samba_autoconf.py b/buildtools/wafsamba/samba_autoconf.py index 9db53e40724b..3ca2f3341901 100644 --- a/buildtools/wafsamba/samba_autoconf.py +++ b/buildtools/wafsamba/samba_autoconf.py @@ -146,7 +146,7 @@ def header_list(conf, headers=None, lib=None): @conf -def CHECK_TYPE(conf, t, alternate=None, headers=None, define=None, lib=None, msg=None): +def CHECK_TYPE(conf, t, alternate=None, headers=None, define=None, lib=None, msg=None, cflags=''): '''check for a single type''' if define is None: define = 'HAVE_' + t.upper().replace(' ', '_') @@ -158,6 +158,7 @@ def CHECK_TYPE(conf, t, alternate=None, headers=None, define=None, lib=None, msg headers=headers, local_include=False, msg=msg, + cflags=cflags, lib=lib, link=False) if not ret and alternate: @@ -177,9 +178,9 @@ def CHECK_TYPES(conf, list, headers=None, define=None, alternate=None, lib=None) @conf -def CHECK_TYPE_IN(conf, t, headers=None, alternate=None, define=None): +def CHECK_TYPE_IN(conf, t, headers=None, alternate=None, define=None, cflags=''): '''check for a single type with a header''' - return CHECK_TYPE(conf, t, headers=headers, alternate=alternate, define=define) + return CHECK_TYPE(conf, t, headers=headers, alternate=alternate, define=define, cflags=cflags) @conf -- 2.34.1 From ef422dd2c148b7c20da1eb1111745e18c5e9a42f Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Tue, 9 Aug 2022 10:29:24 +0000 Subject: [PATCH 38/55] vfs_io_uring: hide a possible definition of struct open_how in liburing/compat.h liburing.h will include liburing/compat.h, which either includes linux/openat2.h or defines struct open_how itself. This will help with the following changes, which will provide openat2() via libreplace's system/filesys.h, either including linux/openat2.h or defining open_how ourself. Signed-off-by: Stefan Metzmacher Reviewed-by: Volker Lendecke BUG: https://bugzilla.samba.org/show_bug.cgi?id=15146 (cherry picked from commit cea9451f780d13e528f1722a67eccbbc78b2daf9) --- source3/modules/vfs_io_uring.c | 18 ++++++++++++++++++ source3/wscript | 13 +++++++++++++ 2 files changed, 31 insertions(+) diff --git a/source3/modules/vfs_io_uring.c b/source3/modules/vfs_io_uring.c index 5168df7a97ba..65dd151bb023 100644 --- a/source3/modules/vfs_io_uring.c +++ b/source3/modules/vfs_io_uring.c @@ -20,6 +20,24 @@ * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA. */ +#include "replace.h" + +/* + * liburing.h only needs a forward declaration + * of struct open_how. + * + * If struct open_how is defined in liburing/compat.h + * itself, hide it away in order to avoid conflicts + * with including linux/openat2.h or defining 'struct open_how' + * in libreplace. + */ +struct open_how; +#ifdef HAVE_STRUCT_OPEN_HOW_LIBURING_COMPAT_H +#define open_how __ignore_liburing_compat_h_open_how +#include +#undef open_how +#endif /* HAVE_STRUCT_OPEN_HOW_LIBURING_COMPAT_H */ + #include "includes.h" #include "system/filesys.h" #include "smbd/smbd.h" diff --git a/source3/wscript b/source3/wscript index 2121b8b6510d..04c278bbe36e 100644 --- a/source3/wscript +++ b/source3/wscript @@ -1738,6 +1738,19 @@ main() { and conf.CHECK_LIB('uring', shlib=True)): conf.CHECK_FUNCS_IN('io_uring_ring_dontfork', 'uring', headers='liburing.h') + # There are a few distributions, which + # don't seem to have linux/openat2.h available + # during the liburing build, which means liburing/compat.h + # defines struct open_how directly instead of including + # linux/openat2.h, while linux/openat2.h is + # available on the installed system, which cause problems + # when we try to include both files. + # + # check if struct open_how is defined in liburing/compat.h + # itself and not via inclusing linux/openat2.h + conf.CHECK_TYPE_IN('struct open_how', 'liburing/compat.h', + cflags='-D_LINUX_OPENAT2_H', + define='HAVE_STRUCT_OPEN_HOW_LIBURING_COMPAT_H') conf.DEFINE('HAVE_LIBURING', '1') conf.env.build_regedit = False -- 2.34.1 From 2e02d732a146d91c57ace627de9d3a6bee14aff9 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Mon, 8 Aug 2022 15:29:28 +0200 Subject: [PATCH 39/55] vfs_btrfs: fix include order, includes.h or replace.h should be first Signed-off-by: Stefan Metzmacher Reviewed-by: Volker Lendecke BUG: https://bugzilla.samba.org/show_bug.cgi?id=15146 (cherry picked from commit 2369d0833361faf4a125431e735fce7efb6024d6) --- source3/modules/vfs_btrfs.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/source3/modules/vfs_btrfs.c b/source3/modules/vfs_btrfs.c index 1ee3f1831c6c..a7ba0ece206c 100644 --- a/source3/modules/vfs_btrfs.c +++ b/source3/modules/vfs_btrfs.c @@ -17,6 +17,8 @@ * along with this program; if not, see . */ +#include "includes.h" +#include "system/filesys.h" #include #include #include @@ -24,8 +26,6 @@ #include #include #include -#include "system/filesys.h" -#include "includes.h" #include "smbd/smbd.h" #include "smbd/globals.h" #include "librpc/gen_ndr/smbXsrv.h" -- 2.34.1 From 74ba0964e918c3e35f47b658b0aaf8119fe25fcf Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Mon, 8 Aug 2022 15:23:29 +0200 Subject: [PATCH 40/55] lib/replace: add a replacement for openat2() that returns ENOSYS Signed-off-by: Stefan Metzmacher Reviewed-by: Volker Lendecke BUG: https://bugzilla.samba.org/show_bug.cgi?id=15146 (cherry picked from commit ce804b78164a3166a16ca3071028536761fd18d7) --- lib/replace/replace.c | 9 +++++++++ lib/replace/system/filesys.h | 35 +++++++++++++++++++++++++++++++++++ lib/replace/wscript | 1 + 3 files changed, 45 insertions(+) diff --git a/lib/replace/replace.c b/lib/replace/replace.c index 0652cb4e6d62..763afec78c52 100644 --- a/lib/replace/replace.c +++ b/lib/replace/replace.c @@ -1081,3 +1081,12 @@ ssize_t rep_copy_file_range(int fd_in, return -1; } #endif /* HAVE_COPY_FILE_RANGE */ + +#ifndef HAVE_OPENAT2 +long rep_openat2(int dirfd, const char *pathname, + struct open_how *how, size_t size) +{ + errno = ENOSYS; + return -1; +} +#endif /* !HAVE_OPENAT2 */ diff --git a/lib/replace/system/filesys.h b/lib/replace/system/filesys.h index bb9482c69afd..8005b18780f9 100644 --- a/lib/replace/system/filesys.h +++ b/lib/replace/system/filesys.h @@ -243,4 +243,39 @@ int rep_fsetxattr (int filedes, const char *name, const void *value, size_t size #endif /* !defined(HAVE_XATTR_XATTR) || defined(XATTR_ADDITIONAL_OPTIONS) */ +#ifdef HAVE_LINUX_OPENAT2_H +#include +#else /* ! HAVE_LINUX_OPENAT2_H */ +/* how->resolve flags for openat2(2). */ +#define RESOLVE_NO_XDEV 0x01 /* Block mount-point crossings + (includes bind-mounts). */ +#define RESOLVE_NO_MAGICLINKS 0x02 /* Block traversal through procfs-style + "magic-links". */ +#define RESOLVE_NO_SYMLINKS 0x04 /* Block traversal through all symlinks + (implies OEXT_NO_MAGICLINKS) */ +#define RESOLVE_BENEATH 0x08 /* Block "lexical" trickery like + "..", symlinks, and absolute + paths which escape the dirfd. */ +#define RESOLVE_IN_ROOT 0x10 /* Make all jumps to "/" and ".." + be scoped inside the dirfd + (similar to chroot(2)). */ +#define RESOLVE_CACHED 0x20 /* Only complete if resolution can be + completed through cached lookup. May + return -EAGAIN if that's not + possible. */ +struct __rep_open_how { + uint64_t flags; + uint64_t mode; + uint64_t resolve; +}; +#define open_how __rep_open_how +#endif /* ! HAVE_LINUX_OPENAT2_H */ + +#ifndef HAVE_OPENAT2 +long rep_openat2(int dirfd, const char *pathname, + struct open_how *how, size_t size); +#define openat2(dirfd, pathname, how, size) \ + rep_openat2(dirfd, pathname, how, size) +#endif /* !HAVE_OPENAT2 */ + #endif diff --git a/lib/replace/wscript b/lib/replace/wscript index dd9b19219a15..2f179992c82a 100644 --- a/lib/replace/wscript +++ b/lib/replace/wscript @@ -66,6 +66,7 @@ def configure(conf): conf.CHECK_HEADERS('errno.h') conf.CHECK_HEADERS('getopt.h iconv.h') conf.CHECK_HEADERS('memory.h nss.h sasl/sasl.h') + conf.CHECK_HEADERS('linux/openat2.h') conf.CHECK_FUNCS_IN('inotify_init', 'inotify', checklibc=True, headers='sys/inotify.h') -- 2.34.1 From b5e35a223b6f7f19a5df257892de3775eb08458b Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Mon, 8 Aug 2022 15:24:28 +0200 Subject: [PATCH 41/55] lib/replace: always include in replace.c if available It will be used for openat2() soon. Signed-off-by: Stefan Metzmacher Reviewed-by: Volker Lendecke BUG: https://bugzilla.samba.org/show_bug.cgi?id=15146 (cherry picked from commit 37ba6df174d73b82e951de401cba7f839ad61ab5) --- lib/replace/replace.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/replace/replace.c b/lib/replace/replace.c index 763afec78c52..e937d81dbd14 100644 --- a/lib/replace/replace.c +++ b/lib/replace/replace.c @@ -33,6 +33,10 @@ #include "system/locale.h" #include "system/wait.h" +#ifdef HAVE_SYS_SYSCALL_H +#include +#endif + #ifdef _WIN32 #define mkdir(d,m) _mkdir(d) #endif @@ -1058,9 +1062,6 @@ const char *rep_getprogname(void) #endif /* HAVE_GETPROGNAME */ #ifndef HAVE_COPY_FILE_RANGE -# ifdef HAVE_SYSCALL_COPY_FILE_RANGE -# include -# endif ssize_t rep_copy_file_range(int fd_in, loff_t *off_in, int fd_out, -- 2.34.1 From ce09f56300bedef03dd38d58022f46b645d50d12 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Mon, 8 Aug 2022 15:25:39 +0200 Subject: [PATCH 42/55] lib/replace: use syscall(__NR_openat2) if available There's no glibc wrapper for openat2() yet, so we need to use syscall(__NR_openat2) ourself. Signed-off-by: Stefan Metzmacher Reviewed-by: Volker Lendecke BUG: https://bugzilla.samba.org/show_bug.cgi?id=15146 (cherry picked from commit b89001e9226ecb0f4e5c906f7195f0e53cd7d608) --- lib/replace/replace.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/lib/replace/replace.c b/lib/replace/replace.c index e937d81dbd14..5ec2f7ccc56b 100644 --- a/lib/replace/replace.c +++ b/lib/replace/replace.c @@ -1087,7 +1087,15 @@ ssize_t rep_copy_file_range(int fd_in, long rep_openat2(int dirfd, const char *pathname, struct open_how *how, size_t size) { +#ifdef __NR_openat2 + return syscall(__NR_openat2, + dirfd, + pathname, + how, + size); +#else errno = ENOSYS; return -1; +#endif } #endif /* !HAVE_OPENAT2 */ -- 2.34.1 From 3586292269441cf40ba7a7a7354f88524f65e3fd Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Mon, 8 Aug 2022 15:33:24 +0200 Subject: [PATCH 43/55] lib/replace: add fallback defines for __NR_openat2 sys/syscall.h might be older than the runtime kernel. If the kernel has support for openat2() we should try to use if anyway. The callers have to deal with ENOSYS anyway, so there's no difference if we get that from syscall(__NR_openat2) or directly from rep_openat2(). Signed-off-by: Stefan Metzmacher Reviewed-by: Volker Lendecke BUG: https://bugzilla.samba.org/show_bug.cgi?id=15146 (cherry picked from commit f7618dd31a9f8f6c0dbfdedd1a664eed25e2e449) --- lib/replace/replace.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/lib/replace/replace.c b/lib/replace/replace.c index 5ec2f7ccc56b..54d3a60e179d 100644 --- a/lib/replace/replace.c +++ b/lib/replace/replace.c @@ -1084,6 +1084,26 @@ ssize_t rep_copy_file_range(int fd_in, #endif /* HAVE_COPY_FILE_RANGE */ #ifndef HAVE_OPENAT2 + +/* fallback known wellknown __NR_openat2 values */ +#ifndef __NR_openat2 +# if defined(LINUX) && defined(HAVE_SYS_SYSCALL_H) +# if defined(__i386__) +# define __NR_openat2 437 +# elif defined(__x86_64__) && defined(__LP64__) +# define __NR_openat2 437 /* 437 0x1B5 */ +# elif defined(__x86_64__) && defined(__ILP32__) +# define __NR_openat2 1073742261 /* 1073742261 0x400001B5 */ +# elif defined(__aarch64__) +# define __NR_openat2 437 +# elif defined(__arm__) +# define __NR_openat2 437 +# elif defined(__sparc__) +# define __NR_openat2 437 +# endif +# endif /* defined(LINUX) && defined(HAVE_SYS_SYSCALL_H) */ +#endif /* !__NR_openat2 */ + long rep_openat2(int dirfd, const char *pathname, struct open_how *how, size_t size) { -- 2.34.1 From 88de1b93b3c73820b079dab26de6541ab7ef3ae0 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 12 Aug 2022 10:53:06 +0200 Subject: [PATCH 44/55] lib/replace: let DISABLE_OPATH also undef __NR_openat2 The reason for DISABLE_OPATH is to simulate a non-linux system, so we should not use openat2() either. Signed-off-by: Stefan Metzmacher Reviewed-by: Volker Lendecke BUG: https://bugzilla.samba.org/show_bug.cgi?id=15146 (cherry picked from commit ae1a84f7313bdf4702492451714eacc78ee7745f) --- lib/replace/replace.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/lib/replace/replace.c b/lib/replace/replace.c index 54d3a60e179d..cbf372e494f1 100644 --- a/lib/replace/replace.c +++ b/lib/replace/replace.c @@ -1104,6 +1104,14 @@ ssize_t rep_copy_file_range(int fd_in, # endif /* defined(LINUX) && defined(HAVE_SYS_SYSCALL_H) */ #endif /* !__NR_openat2 */ +#ifdef DISABLE_OPATH +/* + * systems without O_PATH also don't have openat2, + * so make sure we at a realistic combination. + */ +#undef __NR_openat2 +#endif /* DISABLE_OPATH */ + long rep_openat2(int dirfd, const char *pathname, struct open_how *how, size_t size) { -- 2.34.1 From 74047717c1c226565a4dc26c581c0c7ac51c5683 Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Fri, 3 Jun 2022 16:45:41 +0200 Subject: [PATCH 45/55] vfs: define VFS_OPEN_HOW_RESOLVE_NO_SYMLINKS This will allow us to make use of openat2(RESOLVE_NO_SYMLINKS) soon. The caller should check if connection_struct.open_how_resolve contains VFS_OPEN_HOW_RESOLVE_NO_SYMLINKS before using it, this avoids waisting cpu time. But even then the caller must be prepared to handle -1/ENOSYS. Pair-Programmed-With: Stefan Metzmacher Signed-off-by: Volker Lendecke Signed-off-by: Stefan Metzmacher BUG: https://bugzilla.samba.org/show_bug.cgi?id=15146 (cherry picked from commit f7dc27558329eea7d2c4d75ee101c7f9d3a7afe3) --- source3/include/vfs.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/source3/include/vfs.h b/source3/include/vfs.h index 866d2a5f4a8d..2fd8d1cdd06b 100644 --- a/source3/include/vfs.h +++ b/source3/include/vfs.h @@ -716,6 +716,7 @@ typedef struct connection_struct { bool ipc; bool read_only; /* Attributes for the current user of the share. */ bool have_proc_fds; + uint64_t open_how_resolve; /* supported vfs_open_how.resolve features */ uint32_t share_access; /* Does this filesystem honor sub second timestamps on files @@ -905,6 +906,8 @@ struct vfs_aio_state { uint64_t duration; }; +#define VFS_OPEN_HOW_RESOLVE_NO_SYMLINKS 1 + struct vfs_open_how { int flags; mode_t mode; -- 2.34.1 From 9f186097ff64353b9648500eac9fad20a8aecf27 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Wed, 10 Aug 2022 22:01:10 +0200 Subject: [PATCH 46/55] s3:smbd: let openat_pathref_dirfsp_nosymlink() do a verification loop against . and .. first I guess we should catch NT_STATUS_OBJECT_NAME_INVALID first, currently the check is already done in check_path_syntax*, but we may remove it in future. But the most important reason for this is the openat2(RESOLVE_NO_SYMLINK) optimization, which will be introduced in the following commits. Review with: git show -w Signed-off-by: Stefan Metzmacher Reviewed-by: Volker Lendecke BUG: https://bugzilla.samba.org/show_bug.cgi?id=15146 (cherry picked from commit 17484d069b92d08b0228fb509ea42ab4c3f496a8) --- source3/smbd/files.c | 35 +++++++++++++++++++++++++++++------ 1 file changed, 29 insertions(+), 6 deletions(-) diff --git a/source3/smbd/files.c b/source3/smbd/files.c index 72d1bc7d5551..f23e9e96e52e 100644 --- a/source3/smbd/files.c +++ b/source3/smbd/files.c @@ -793,22 +793,45 @@ NTSTATUS openat_pathref_dirfsp_nosymlink( goto nomem; } + /* + * First split the path into individual components. + */ path = path_to_strv(talloc_tos(), path_in); if (path == NULL) { DBG_DEBUG("path_to_strv() failed\n"); goto nomem; } + + /* + * First we loop over all components + * in order to verify, there's no '.' or '..' + */ rel_fname.base_name = path; + while (rel_fname.base_name != NULL) { -next: - next = strv_next(path, rel_fname.base_name); + next = strv_next(path, rel_fname.base_name); - if (ISDOT(rel_fname.base_name) || ISDOTDOT(rel_fname.base_name)) { - DBG_DEBUG("%s contains a dot\n", path_in); - status = NT_STATUS_OBJECT_NAME_INVALID; - goto fail; + if (ISDOT(rel_fname.base_name) || ISDOTDOT(rel_fname.base_name)) { + DBG_DEBUG("%s contains a dot\n", path_in); + status = NT_STATUS_OBJECT_NAME_INVALID; + goto fail; + } + + rel_fname.base_name = next; } + /* + * Now we loop over all components + * opening each one and using it + * as dirfd for the next one. + * + * It means we can detect symlinks + * within the path. + */ + rel_fname.base_name = path; +next: + next = strv_next(path, rel_fname.base_name); + fd = SMB_VFS_OPENAT( conn, dirfsp, -- 2.34.1 From 6f4e75030bc71cb43facc225aa3c9165e027fd3f Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 12 Aug 2022 19:12:44 +0200 Subject: [PATCH 47/55] s3:smbd: let openat_pathref_dirfsp_nosymlink() handle ELOOP similar to ENOTDIR This is no likely to happen as we use O_NOFOLLOW with O_DIRECTORY, but it's better to be prepared... This will be more important in the upcoming openat2(RESOLVE_NO_SYMLINK) case, but we should be consitent... Signed-off-by: Stefan Metzmacher Reviewed-by: Volker Lendecke BUG: https://bugzilla.samba.org/show_bug.cgi?id=15146 (cherry picked from commit 35b99c87ef92df006f8b0a41bbea051f0faeadb9) --- source3/smbd/files.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/source3/smbd/files.c b/source3/smbd/files.c index f23e9e96e52e..23de0ad63c22 100644 --- a/source3/smbd/files.c +++ b/source3/smbd/files.c @@ -860,7 +860,15 @@ next: &how); } - if ((fd == -1) && (errno == ENOTDIR)) { + /* + * O_NOFOLLOW|O_DIRECTORY results in + * ENOTDIR instead of ELOOP. + * + * But we should be prepared to handle ELOOP too. + */ + if ((fd == -1) && (errno == ENOTDIR || errno == ELOOP)) { + NTSTATUS orig_status = map_nt_error_from_unix(errno); + status = readlink_talloc( mem_ctx, dirfsp, &rel_fname, substitute); @@ -898,7 +906,7 @@ next: /* * Restore the error status from SMB_VFS_OPENAT() */ - status = NT_STATUS_NOT_A_DIRECTORY; + status = orig_status; } goto fail; } -- 2.34.1 From d7d82a2be057c900013b28cddfb5c83ac00d053d Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Thu, 14 Jul 2022 19:44:04 +0200 Subject: [PATCH 48/55] s3:smbd: let openat_pathref_dirfsp_nosymlink() try VFS_OPEN_HOW_RESOLVE_NO_SYMLINKS first This will reduce the amount of syscalls and the related cost drastically for long path names. Pair-Programmed-With: Stefan Metzmacher Signed-off-by: Volker Lendecke Signed-off-by: Stefan Metzmacher BUG: https://bugzilla.samba.org/show_bug.cgi?id=15146 (cherry picked from commit d6653067b20e61af1f05423764c8486a1a5445c8) --- source3/smbd/files.c | 64 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/source3/smbd/files.c b/source3/smbd/files.c index 23de0ad63c22..af135d6e95a5 100644 --- a/source3/smbd/files.c +++ b/source3/smbd/files.c @@ -820,6 +820,69 @@ NTSTATUS openat_pathref_dirfsp_nosymlink( rel_fname.base_name = next; } + if (conn->open_how_resolve & VFS_OPEN_HOW_RESOLVE_NO_SYMLINKS) { + + /* + * Try a direct openat2 with RESOLVE_NO_SYMLINKS to + * avoid the openat/close loop further down. + */ + + rel_fname.base_name = discard_const_p(char, path_in); + how.resolve = VFS_OPEN_HOW_RESOLVE_NO_SYMLINKS; + + fd = SMB_VFS_OPENAT(conn, dirfsp, &rel_fname, fsp, &how); + if (fd >= 0) { + fsp_set_fd(fsp, fd); + TALLOC_FREE(full_fname.base_name); + full_fname = rel_fname; + goto done; + } + + status = map_nt_error_from_unix(errno); + DBG_DEBUG("SMB_VFS_OPENAT(%s, %s, RESOLVE_NO_SYMLINKS) returned %d %s => %s\n", + smb_fname_str_dbg(dirfsp->fsp_name), path_in, + errno, strerror(errno), nt_errstr(status)); + SMB_ASSERT(fd == -1); + switch (errno) { + case ENOSYS: + /* + * We got ENOSYS, so fallback to the old code + * if the kernel doesn't support openat2() yet. + */ + break; + + case ELOOP: + case ENOTDIR: + /* + * For ELOOP we also fallback in order to + * return the correct information with + * NT_STATUS_STOPPED_ON_SYMLINK. + * + * O_NOFOLLOW|O_DIRECTORY results in + * ENOTDIR instead of ELOOP for the final + * component. + */ + break; + + case ENOENT: + /* + * If we got ENOENT, the filesystem could + * be case sensitive. For now we only do + * the get_real_filename_at() dance in + * the fallback loop below. + */ + break; + + default: + goto fail; + } + + /* + * Just fallback to the openat loop + */ + how.resolve = 0; + } + /* * Now we loop over all components * opening each one and using it @@ -967,6 +1030,7 @@ next: dirfsp = NULL; } +done: fsp->fsp_flags.is_pathref = true; fsp->fsp_name = NULL; -- 2.34.1 From b40687b1b3143556112bf739ca77c8d029440832 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Wed, 27 Jul 2022 18:43:14 +0000 Subject: [PATCH 49/55] vfs_default: prepare O_PATH usage with openat2() When O_PATH is specified in flags, flag bits other than O_CLOEXEC, O_DIRECTORY, and O_NOFOLLOW are ignored. In preparation to use openat2(), which gives an error instead of ignoring flags, we better remove unexpected flags, callers typically pass O_RDONLY and O_NONBLOCK. Signed-off-by: Stefan Metzmacher Reviewed-by: Volker Lendecke BUG: https://bugzilla.samba.org/show_bug.cgi?id=15146 (cherry picked from commit 8544f4490a0b5e54b807daedddb96778744b62ee) --- source3/modules/vfs_default.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/source3/modules/vfs_default.c b/source3/modules/vfs_default.c index 9cf70fd84ce9..82b7805657f8 100644 --- a/source3/modules/vfs_default.c +++ b/source3/modules/vfs_default.c @@ -718,6 +718,24 @@ static int vfswrap_openat(vfs_handle_struct *handle, if (fsp->fsp_flags.is_pathref) { flags |= O_PATH; } + if (flags & O_PATH) { + /* + * From "man 2 openat": + * + * When O_PATH is specified in flags, flag bits other than + * O_CLOEXEC, O_DIRECTORY, and O_NOFOLLOW are ignored. + * + * From "man 2 openat2": + * + * Whereas openat(2) ignores unknown bits in its flags + * argument, openat2() returns an error if unknown or + * conflicting flags are specified in how.flags. + * + * So we better clear ignored/invalid flags + * and only keep the exptected once. + */ + flags &= (O_PATH|O_CLOEXEC|O_DIRECTORY|O_NOFOLLOW); + } #endif if (fsp->fsp_flags.is_pathref && !have_opath) { -- 2.34.1 From 25fb5c6c539cd1d71f191a983c6c1fcdf02373ad Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Fri, 17 Jun 2022 17:41:52 +0200 Subject: [PATCH 50/55] vfs_default: Use openat2(RESOLVE_NO_SYMLINKS) if available This improves the following test: time smbtorture //127.0.0.1/m -Uroot%test \ smb2.create.bench-path-contention-shared \ --option='torture:bench_path=Apps\1\2\3\4\5\6\7\8\9\10' \ --option="torture:timelimit=600" \ --option="torture:nprocs=1" From: open[num/s=14186,avslat=0.000044,minlat=0.000042,maxlat=0.000079] close[num/s=14185,avslat=0.000027,minlat=0.000025,maxlat=0.000057] to: open[num/s=16917,avslat=0.000038,minlat=0.000035,maxlat=0.000340] close[num/s=16916,avslat=0.000020,minlat=0.000019,maxlat=0.000104] Pair-Programmed-With: Stefan Metzmacher Signed-off-by: Volker Lendecke Signed-off-by: Stefan Metzmacher BUG: https://bugzilla.samba.org/show_bug.cgi?id=15146 (cherry picked from commit 4708ba2f013c5f5ea5aa5dcf4873c2b4a86fb8ff) --- source3/modules/vfs_default.c | 53 ++++++++++++++++++++++++++++++++++- 1 file changed, 52 insertions(+), 1 deletion(-) diff --git a/source3/modules/vfs_default.c b/source3/modules/vfs_default.c index 82b7805657f8..5d8ee98e2ca1 100644 --- a/source3/modules/vfs_default.c +++ b/source3/modules/vfs_default.c @@ -49,7 +49,28 @@ static int vfswrap_connect(vfs_handle_struct *handle, const char *service, const char *user) { + bool bval; + handle->conn->have_proc_fds = sys_have_proc_fds(); + + /* + * assume the kernel will support openat2(), + * it will be reset on the first ENOSYS. + * + * Note that libreplace will always provide openat2(), + * but return -1/errno = ENOSYS... + * + * The option is only there to test the fallback code. + */ + bval = lp_parm_bool(SNUM(handle->conn), + "vfs_default", + "VFS_OPEN_HOW_RESOLVE_NO_SYMLINKS", + true); + if (bval) { + handle->conn->open_how_resolve |= + VFS_OPEN_HOW_RESOLVE_NO_SYMLINKS; + } + return 0; /* Return >= 0 for success */ } @@ -701,7 +722,7 @@ static int vfswrap_openat(vfs_handle_struct *handle, START_PROFILE(syscall_openat); - if (how->resolve != 0) { + if (how->resolve & ~VFS_OPEN_HOW_RESOLVE_NO_SYMLINKS) { errno = ENOSYS; result = -1; goto out; @@ -738,6 +759,35 @@ static int vfswrap_openat(vfs_handle_struct *handle, } #endif + if (how->resolve & VFS_OPEN_HOW_RESOLVE_NO_SYMLINKS) { + struct open_how linux_how = { + .flags = flags, + .mode = mode, + .resolve = RESOLVE_NO_SYMLINKS, + }; + + result = openat2(fsp_get_pathref_fd(dirfsp), + smb_fname->base_name, + &linux_how, + sizeof(linux_how)); + if (result == -1) { + if (errno == ENOSYS) { + /* + * The kernel doesn't support + * openat2(), so indicate to + * the callers that + * VFS_OPEN_HOW_RESOLVE_NO_SYMLINKS + * would just be a waste of time. + */ + fsp->conn->open_how_resolve &= + ~VFS_OPEN_HOW_RESOLVE_NO_SYMLINKS; + } + goto out; + } + + goto done; + } + if (fsp->fsp_flags.is_pathref && !have_opath) { become_root(); became_root = true; @@ -752,6 +802,7 @@ static int vfswrap_openat(vfs_handle_struct *handle, unbecome_root(); } +done: fsp->fsp_flags.have_proc_fds = fsp->conn->have_proc_fds; out: -- 2.34.1 From 5db45a47f269ef7c204950321543cff43e264ec9 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 12 Aug 2022 10:55:42 +0200 Subject: [PATCH 51/55] selftest/Samba3: let nt4_dc* use vfs_default:VFS_OPEN_HOW_RESOLVE_NO_SYMLINKS=no We should always test the code path without openat2 being available, even if the kernel supports it. Signed-off-by: Stefan Metzmacher Reviewed-by: Volker Lendecke Autobuild-User(master): Volker Lendecke Autobuild-Date(master): Mon Aug 15 16:00:26 UTC 2022 on sn-devel-184 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15146 (cherry picked from commit 076c22fbd7ecbf22dbfeb1711609f07fd42f88b0) --- selftest/target/Samba3.pm | 2 ++ 1 file changed, 2 insertions(+) diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm index 1a4bf7439ef4..8efd3f0aa096 100755 --- a/selftest/target/Samba3.pm +++ b/selftest/target/Samba3.pm @@ -275,6 +275,8 @@ sub setup_nt4_dc server schannel = auto rpc start on demand helpers = false + vfs_default:VFS_OPEN_HOW_RESOLVE_NO_SYMLINKS = no + fss: sequence timeout = 1 check parent directory delete on close = yes "; -- 2.34.1 From bdde56175605a7c3cf05592c6a30868f89505a5a Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 11 Aug 2022 09:51:11 -0700 Subject: [PATCH 52/55] s3: tests: Add samba3.blackbox.test_veto_files. Shows we currently don't look at smb.conf veto files parameter when opening a file or directory. Checks multi-component paths. Also checks veto files that might be hidden behind a mangled name. Add knownfail. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15143 Signed-off-by: Jeremy Allison Reviewed-by: Stefan Metzmacher BUG: https://bugzilla.samba.org/show_bug.cgi?id=15146 (cherry picked from commit c6933673222ea9ae2eb74d5586c9495269f51ea0) --- selftest/knownfail.d/veto_files | 1 + selftest/target/Samba3.pm | 4 + source3/script/tests/test_veto_files.sh | 201 ++++++++++++++++++++++++ source3/selftest/tests.py | 4 + 4 files changed, 210 insertions(+) create mode 100644 selftest/knownfail.d/veto_files create mode 100755 source3/script/tests/test_veto_files.sh diff --git a/selftest/knownfail.d/veto_files b/selftest/knownfail.d/veto_files new file mode 100644 index 000000000000..ad7d841a0336 --- /dev/null +++ b/selftest/knownfail.d/veto_files @@ -0,0 +1 @@ +^samba3.blackbox.test_veto_files.get_veto_file\(fileserver\) diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm index 8efd3f0aa096..b12011a71fae 100755 --- a/selftest/target/Samba3.pm +++ b/selftest/target/Samba3.pm @@ -1916,6 +1916,10 @@ sub setup_fileserver path = $veto_sharedir delete veto files = yes +[veto_files] + path = $veto_sharedir + veto files = /veto_name*/ + [delete_yes_unwrite] read only = no path = $delete_unwrite_sharedir diff --git a/source3/script/tests/test_veto_files.sh b/source3/script/tests/test_veto_files.sh new file mode 100755 index 000000000000..9f0526bd54c0 --- /dev/null +++ b/source3/script/tests/test_veto_files.sh @@ -0,0 +1,201 @@ +#!/bin/sh +# +# Check smbclient cannot get a file that matches a veto files +# parameter, or inside a directory that matches a veto files +# parameter. +# +# BUG: https://bugzilla.samba.org/show_bug.cgi?id=15143 +# + +if [ $# -lt 6 ]; then + cat <"$tmpfile" < Date: Thu, 11 Aug 2022 09:55:56 -0700 Subject: [PATCH 53/55] s3: smbd: Add IS_VETO_PATH check to openat_pathref_dirfsp_nosymlink(). Returns NT_STATUS_OBJECT_PATH_NOT_FOUND for directory component. Note IS_VETO_PATH only looks at the last component, so we must do it during the directory walk on each component. Note, we also have to check after a call to get_real_filename_at() as it may have demangled the client sent name into a filesystem name that matches the "veto files" parameter. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15143 Signed-off-by: Jeremy Allison Reviewed-by: Stefan Metzmacher BUG: https://bugzilla.samba.org/show_bug.cgi?id=15146 (cherry picked from commit 1c293060204d96bf94427f91eb20eb9decc29a41) --- source3/smbd/files.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/source3/smbd/files.c b/source3/smbd/files.c index af135d6e95a5..a6c41f2b9280 100644 --- a/source3/smbd/files.c +++ b/source3/smbd/files.c @@ -817,6 +817,14 @@ NTSTATUS openat_pathref_dirfsp_nosymlink( goto fail; } + /* Check veto files. */ + if (IS_VETO_PATH(conn, rel_fname.base_name)) { + DBG_DEBUG("%s contains veto files path component %s\n", + path_in, rel_fname.base_name); + status = NT_STATUS_OBJECT_PATH_NOT_FOUND; + goto fail; + } + rel_fname.base_name = next; } @@ -903,6 +911,8 @@ next: &how); if ((fd == -1) && (errno == ENOENT)) { + const char *orig_base_name = rel_fname.base_name; + status = get_real_filename_at( dirfsp, rel_fname.base_name, @@ -915,6 +925,14 @@ next: goto fail; } + /* Name might have been demangled - check veto files. */ + if (IS_VETO_PATH(conn, rel_fname.base_name)) { + DBG_DEBUG("%s contains veto files path component %s => %s\n", + path_in, orig_base_name, rel_fname.base_name); + status = NT_STATUS_OBJECT_PATH_NOT_FOUND; + goto fail; + } + fd = SMB_VFS_OPENAT( conn, dirfsp, -- 2.34.1 From ddc0c0e2e07603752639442678e60d485a47cca3 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 11 Aug 2022 10:03:58 -0700 Subject: [PATCH 54/55] s3: smbd: Add IS_VETO_PATH checks to openat_pathref_fsp_case_insensitive(). Returns NT_STATUS_OBJECT_NAME_NOT_FOUND for final component. Note we have to call the check before each call to openat_pathref_fsp(), as each call may be using a different filesystem name. The first name is the one passed into openat_pathref_fsp_case_insensitive() by the caller, the second one is a name retrieved from get_real_filename_cache_key(), and the third one is the name retrieved from get_real_filename_at(). The last two calls may have demangled the client given name into a veto'ed path on the filesystem. Remove knownfail. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15143 Signed-off-by: Jeremy Allison Reviewed-by: Stefan Metzmacher Autobuild-User(master): Stefan Metzmacher Autobuild-Date(master): Tue Aug 16 08:26:54 UTC 2022 on sn-devel-184 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15146 (cherry picked from commit 1654eae11b9c13308b2b78f70309eb3a56960619) --- selftest/knownfail.d/veto_files | 1 - source3/smbd/filename.c | 20 ++++++++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) delete mode 100644 selftest/knownfail.d/veto_files diff --git a/selftest/knownfail.d/veto_files b/selftest/knownfail.d/veto_files deleted file mode 100644 index ad7d841a0336..000000000000 --- a/selftest/knownfail.d/veto_files +++ /dev/null @@ -1 +0,0 @@ -^samba3.blackbox.test_veto_files.get_veto_file\(fileserver\) diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c index f362aee94525..ca94b7ec7f96 100644 --- a/source3/smbd/filename.c +++ b/source3/smbd/filename.c @@ -836,6 +836,13 @@ static NTSTATUS openat_pathref_fsp_case_insensitive( SET_STAT_INVALID(smb_fname_rel->st); + /* Check veto files - only looks at last component. */ + if (IS_VETO_PATH(dirfsp->conn, smb_fname_rel->base_name)) { + DBG_DEBUG("veto files rejecting last component %s\n", + smb_fname_str_dbg(smb_fname_rel)); + return NT_STATUS_OBJECT_NAME_NOT_FOUND; + } + status = openat_pathref_fsp(dirfsp, smb_fname_rel); if (NT_STATUS_IS_OK(status)) { @@ -895,6 +902,13 @@ static NTSTATUS openat_pathref_fsp_case_insensitive( return NT_STATUS_NO_MEMORY; } + if (IS_VETO_PATH(dirfsp->conn, smb_fname_rel->base_name)) { + DBG_DEBUG("veto files rejecting last component %s\n", + smb_fname_str_dbg(smb_fname_rel)); + TALLOC_FREE(cache_key.data); + return NT_STATUS_OBJECT_NAME_NOT_FOUND; + } + status = openat_pathref_fsp(dirfsp, smb_fname_rel); if (NT_STATUS_IS_OK(status)) { TALLOC_FREE(cache_key.data); @@ -919,6 +933,12 @@ lookup: TALLOC_FREE(smb_fname_rel->base_name); smb_fname_rel->base_name = found_name; + if (IS_VETO_PATH(dirfsp->conn, smb_fname_rel->base_name)) { + DBG_DEBUG("veto files rejecting last component %s\n", + smb_fname_str_dbg(smb_fname_rel)); + return NT_STATUS_OBJECT_NAME_NOT_FOUND; + } + status = openat_pathref_fsp(dirfsp, smb_fname_rel); } -- 2.34.1 From 1d6f2004bbe7463e1bc45b04bba53ef8955fd87b Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Tue, 16 Aug 2022 09:35:16 +0200 Subject: [PATCH 55/55] s3:vfs.h: add comment about VFS_OPEN_HOW_RESOLVE_NO_SYMLINKS BUG: https://bugzilla.samba.org/show_bug.cgi?id=15146 Signed-off-by: Stefan Metzmacher Reviewed-by: Ralph Boehme (cherry picked from commit 5adf051228b56c05fe1205e7a865a497b58e81d9) --- source3/include/vfs.h | 1 + 1 file changed, 1 insertion(+) diff --git a/source3/include/vfs.h b/source3/include/vfs.h index 2fd8d1cdd06b..61360b3c21ce 100644 --- a/source3/include/vfs.h +++ b/source3/include/vfs.h @@ -374,6 +374,7 @@ * Version 47 - Re-add dirfsp to CREATE_FILE * Version 47 - Add fsp flag fstat_before_close * Version 47 - Change SMB_VFS_OPENAT() to match the Linux openat2 prototype, add vfs_open_how + * Version 47 - Add VFS_OPEN_HOW_RESOLVE_NO_SYMLINKS for SMB_VFS_OPENAT() */ #define SMB_VFS_INTERFACE_VERSION 47 -- 2.34.1