From d5886dfd30fb47ae9969f56c5da39cff119c5bee Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 9 Dec 2021 16:08:07 -0800 Subject: [PATCH 01/54] s3: smbd: Move setting of dirtype if FILE_ATTRIBUTE_NORMAL to do_unlink(). Now we don't use wildcards when calling in unlink_internals() the logic inside it serves no purpose and can be replaced with a direct call to do_unlink() (which we will rename to unlink_internals()). Signed-off-by: Jeremy Allison --- source3/smbd/reply.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c index b3b0ade839a..1285ea536b5 100644 --- a/source3/smbd/reply.c +++ b/source3/smbd/reply.c @@ -3143,6 +3143,10 @@ static NTSTATUS do_unlink(connection_struct *conn, int ret; struct smb2_create_blobs *posx = NULL; + if (dirtype == 0) { + dirtype = FILE_ATTRIBUTE_NORMAL; + } + DEBUG(10,("do_unlink: %s, dirtype = %d\n", smb_fname_str_dbg(smb_fname), dirtype)); @@ -3336,9 +3340,6 @@ NTSTATUS unlink_internals(connection_struct *conn, status = NT_STATUS_NO_MEMORY; goto out; } - if (dirtype == 0) { - dirtype = FILE_ATTRIBUTE_NORMAL; - } status = check_name(conn, smb_fname); if (!NT_STATUS_IS_OK(status)) { -- 2.30.2 From fbc404d02eefb55f696ecb08c02c5c844ad3fc45 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 9 Dec 2021 16:11:20 -0800 Subject: [PATCH 02/54] s3: smbd: Move to modern debug calls inside do_unlink(). We will be changing its name next. Signed-off-by: Jeremy Allison --- source3/smbd/reply.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c index 1285ea536b5..d15a74ee556 100644 --- a/source3/smbd/reply.c +++ b/source3/smbd/reply.c @@ -3147,9 +3147,9 @@ static NTSTATUS do_unlink(connection_struct *conn, dirtype = FILE_ATTRIBUTE_NORMAL; } - DEBUG(10,("do_unlink: %s, dirtype = %d\n", + DBG_DEBUG("%s, dirtype = %d\n", smb_fname_str_dbg(smb_fname), - dirtype)); + dirtype); if (!CAN_WRITE(conn)) { return NT_STATUS_MEDIA_WRITE_PROTECTED; @@ -3249,17 +3249,17 @@ static NTSTATUS do_unlink(connection_struct *conn, TALLOC_FREE(posx); if (!NT_STATUS_IS_OK(status)) { - DEBUG(10, ("SMB_VFS_CREATEFILE failed: %s\n", - nt_errstr(status))); + DBG_DEBUG("SMB_VFS_CREATEFILE failed: %s\n", + nt_errstr(status)); return status; } status = can_set_delete_on_close(fsp, fattr); if (!NT_STATUS_IS_OK(status)) { - DEBUG(10, ("do_unlink can_set_delete_on_close for file %s - " + DBG_DEBUG("can_set_delete_on_close for file %s - " "(%s)\n", smb_fname_str_dbg(smb_fname), - nt_errstr(status))); + nt_errstr(status)); close_file(req, fsp, NORMAL_CLOSE); return status; } -- 2.30.2 From e83d89442d6f7fe72d45671a95a391d25d0031f1 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 9 Dec 2021 16:14:40 -0800 Subject: [PATCH 03/54] s3: smbd: Comment out the old unlink_internals(). Rename do_unlink() -> unlink_internals(). One parameter needs changing position. The logic inside unlink_internals() is no longer needed if it doesn't accept wildcards. filename_convert() already handles mangled names just fine, so we don't need this logic. Signed-off-by: Jeremy Allison --- source3/smbd/reply.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c index d15a74ee556..71be06ecc8c 100644 --- a/source3/smbd/reply.c +++ b/source3/smbd/reply.c @@ -3131,10 +3131,10 @@ static NTSTATUS can_rename(connection_struct *conn, files_struct *fsp, * unlink a file with all relevant access checks *******************************************************************/ -static NTSTATUS do_unlink(connection_struct *conn, +NTSTATUS unlink_internals(connection_struct *conn, struct smb_request *req, - struct smb_filename *smb_fname, - uint32_t dirtype) + uint32_t dirtype, + struct smb_filename *smb_fname) { uint32_t fattr; files_struct *fsp; @@ -3275,6 +3275,7 @@ static NTSTATUS do_unlink(connection_struct *conn, return close_file(req, fsp, NORMAL_CLOSE); } +#if 0 /**************************************************************************** The guts of the unlink command, split out so it may be called by the NT SMB code. @@ -3354,6 +3355,7 @@ NTSTATUS unlink_internals(connection_struct *conn, TALLOC_FREE(fname_mask); return status; } +#endif /**************************************************************************** Reply to a unlink -- 2.30.2 From 9826f37bc9a90f51a0e282c731c3cdc42921f0f5 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 9 Dec 2021 16:16:52 -0800 Subject: [PATCH 04/54] s3: smbd: Remove the old unlink_internals() implementation. No longer used. filename_convert() already handles mangled names just fine, so we don't need this logic. Signed-off-by: Jeremy Allison --- source3/smbd/reply.c | 82 -------------------------------------------- 1 file changed, 82 deletions(-) diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c index 71be06ecc8c..2623165966f 100644 --- a/source3/smbd/reply.c +++ b/source3/smbd/reply.c @@ -3275,88 +3275,6 @@ NTSTATUS unlink_internals(connection_struct *conn, return close_file(req, fsp, NORMAL_CLOSE); } -#if 0 -/**************************************************************************** - The guts of the unlink command, split out so it may be called by the NT SMB - code. -****************************************************************************/ - -NTSTATUS unlink_internals(connection_struct *conn, - struct smb_request *req, - uint32_t dirtype, - struct smb_filename *smb_fname) -{ - char *fname_dir = NULL; - char *fname_mask = NULL; - NTSTATUS status = NT_STATUS_OK; - struct smb_filename *smb_fname_dir = NULL; - TALLOC_CTX *ctx = talloc_tos(); - bool posix_pathname = (smb_fname->flags & SMB_FILENAME_POSIX_PATH); - - /* Split up the directory from the filename/mask. */ - status = split_fname_dir_mask(ctx, smb_fname->base_name, - &fname_dir, &fname_mask); - if (!NT_STATUS_IS_OK(status)) { - goto out; - } - - /* - * We should only check the mangled cache - * here if unix_convert failed. This means - * that the path in 'mask' doesn't exist - * on the file system and so we need to look - * for a possible mangle. This patch from - * Tine Smukavec . - */ - - if (!VALID_STAT(smb_fname->st) && - !posix_pathname && - mangle_is_mangled(fname_mask, conn->params)) { - char *new_mask = NULL; - mangle_lookup_name_from_8_3(ctx, fname_mask, - &new_mask, conn->params); - if (new_mask) { - TALLOC_FREE(fname_mask); - fname_mask = new_mask; - } - } - - /* - * Only one file needs to be unlinked. Append the mask back - * onto the directory. - */ - TALLOC_FREE(smb_fname->base_name); - if (ISDOT(fname_dir)) { - /* Ensure we use canonical names on open. */ - smb_fname->base_name = talloc_asprintf(smb_fname, - "%s", - fname_mask); - } else { - smb_fname->base_name = talloc_asprintf(smb_fname, - "%s/%s", - fname_dir, - fname_mask); - } - if (!smb_fname->base_name) { - status = NT_STATUS_NO_MEMORY; - goto out; - } - - status = check_name(conn, smb_fname); - if (!NT_STATUS_IS_OK(status)) { - goto out; - } - - status = do_unlink(conn, req, smb_fname, dirtype); - - out: - TALLOC_FREE(smb_fname_dir); - TALLOC_FREE(fname_dir); - TALLOC_FREE(fname_mask); - return status; -} -#endif - /**************************************************************************** Reply to a unlink ****************************************************************************/ -- 2.30.2 From fbc8b079d2dec48a6dc0d0b44a722b1653b25c40 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 9 Dec 2021 16:35:17 -0800 Subject: [PATCH 05/54] s3: smbd: Handling SMB_FILE_RENAME_INFORMATION, the destination name is a single component. No errors should be allowed from filename_convert(). Signed-off-by: Jeremy Allison --- source3/smbd/trans2.c | 19 +------------------ 1 file changed, 1 insertion(+), 18 deletions(-) diff --git a/source3/smbd/trans2.c b/source3/smbd/trans2.c index 4ae59a33e18..55857dd9119 100644 --- a/source3/smbd/trans2.c +++ b/source3/smbd/trans2.c @@ -7480,25 +7480,8 @@ static NTSTATUS smb_file_rename_information(connection_struct *conn, 0, &smb_fname_dst); - /* If an error we expect this to be - * NT_STATUS_OBJECT_PATH_NOT_FOUND */ - if (!NT_STATUS_IS_OK(status)) { - if(!NT_STATUS_EQUAL(NT_STATUS_OBJECT_PATH_NOT_FOUND, - status)) { - goto out; - } - /* Create an smb_fname to call rename_internals_fsp() */ - smb_fname_dst = synthetic_smb_fname(ctx, - base_name, - NULL, - NULL, - smb_fname_src->twrp, - smb_fname_src->flags); - if (smb_fname_dst == NULL) { - status = NT_STATUS_NO_MEMORY; - goto out; - } + goto out; } dst_original_lcomp = get_original_lcomp(smb_fname_dst, conn, -- 2.30.2 From 7d6e3080c27300c4f78fcb34af7a4a529116a87e Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 9 Dec 2021 16:45:13 -0800 Subject: [PATCH 06/54] s3: smbd: In rename_internals_fsp(), remove unneeded call to check_name(). All callers have gone through filename_convert(), which has already called check_name() on the destination. Signed-off-by: Jeremy Allison --- source3/smbd/reply.c | 5 ----- 1 file changed, 5 deletions(-) diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c index 2623165966f..1028519b0dc 100644 --- a/source3/smbd/reply.c +++ b/source3/smbd/reply.c @@ -7190,11 +7190,6 @@ NTSTATUS rename_internals_fsp(connection_struct *conn, bool case_preserve = (fsp->posix_flags & FSP_POSIX_FLAGS_OPEN) ? true : conn->case_preserve; - status = check_name(conn, smb_fname_dst_in); - if (!NT_STATUS_IS_OK(status)) { - return status; - } - status = parent_dirname_compatible_open(conn, smb_fname_dst_in); if (!NT_STATUS_IS_OK(status)) { return status; -- 2.30.2 From 5b0de86c6644fa408f8007fd36ed8650c9ca3cce Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 9 Dec 2021 16:47:13 -0800 Subject: [PATCH 07/54] s3: smbd: check_name() is now static to filename.c Signed-off-by: Jeremy Allison --- source3/smbd/filename.c | 2 +- source3/smbd/proto.h | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c index 6ea0687d763..b0f6ac5a99f 100644 --- a/source3/smbd/filename.c +++ b/source3/smbd/filename.c @@ -1477,7 +1477,7 @@ static NTSTATUS check_veto_path(connection_struct *conn, a valid one for the user to access. ****************************************************************************/ -NTSTATUS check_name(connection_struct *conn, +static NTSTATUS check_name(connection_struct *conn, const struct smb_filename *smb_fname) { NTSTATUS status = check_veto_path(conn, smb_fname); diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h index 9576cf2593b..82a8234ba8a 100644 --- a/source3/smbd/proto.h +++ b/source3/smbd/proto.h @@ -359,8 +359,6 @@ NTSTATUS unix_convert(TALLOC_CTX *ctx, NTTIME twrp, struct smb_filename **smb_fname, uint32_t ucf_flags); -NTSTATUS check_name(connection_struct *conn, - const struct smb_filename *smb_fname); NTSTATUS canonicalize_snapshot_path(struct smb_filename *smb_fname, uint32_t ucf_flags, NTTIME twrp); -- 2.30.2 From 3bb7075a0cac6e328a2f21b31b9baee64a7d56ef Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 9 Dec 2021 16:49:46 -0800 Subject: [PATCH 08/54] s3: smbd: In rename_internals(), remove the name spliting and re-combining code. filename_convert() handles mangled names just fine, so we don't need to split the last component and check for mangle. Now we don't take wildcard names this is not needed. This was the last caller of split_fname_dir_mask(), so ifdef it out. Signed-off-by: Jeremy Allison --- source3/smbd/reply.c | 68 ++------------------------------------------ 1 file changed, 2 insertions(+), 66 deletions(-) diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c index 1028519b0dc..47309eb18fa 100644 --- a/source3/smbd/reply.c +++ b/source3/smbd/reply.c @@ -1625,6 +1625,7 @@ void reply_dskattr(struct smb_request *req) return; } +#if 0 /* * Utility function to split the filename from the directory. */ @@ -1656,6 +1657,7 @@ static NTSTATUS split_fname_dir_mask(TALLOC_CTX *ctx, const char *fname_in, *fname_mask_out = fname_mask; return NT_STATUS_OK; } +#endif /**************************************************************************** Make a dir struct. @@ -7623,11 +7625,7 @@ NTSTATUS rename_internals(TALLOC_CTX *ctx, bool replace_if_exists, uint32_t access_mask) { - char *fname_src_dir = NULL; - struct smb_filename *smb_fname_src_dir = NULL; - char *fname_src_mask = NULL; NTSTATUS status = NT_STATUS_OK; - char *talloced = NULL; int create_options = 0; struct smb2_create_blobs *posx = NULL; int rc; @@ -7638,43 +7636,6 @@ NTSTATUS rename_internals(TALLOC_CTX *ctx, bool short_case_preserve = posix_pathname ? true : conn->short_case_preserve; - /* - * Split the old name into directory and last component - * strings. Note that unix_convert may have stripped off a - * leading ./ from both name and newname if the rename is - * at the root of the share. We need to make sure either both - * name and newname contain a / character or neither of them do. - */ - - /* Split up the directory from the filename/mask. */ - status = split_fname_dir_mask(ctx, smb_fname_src->base_name, - &fname_src_dir, &fname_src_mask); - if (!NT_STATUS_IS_OK(status)) { - status = NT_STATUS_NO_MEMORY; - goto out; - } - - /* - * We should only check the mangled cache - * here if unix_convert failed. This means - * that the path in 'mask' doesn't exist - * on the file system and so we need to look - * for a possible mangle. This patch from - * Tine Smukavec . - */ - - if (!VALID_STAT(smb_fname_src->st) && - !posix_pathname && - mangle_is_mangled(fname_src_mask, conn->params)) { - char *new_mask = NULL; - mangle_lookup_name_from_8_3(ctx, fname_src_mask, &new_mask, - conn->params); - if (new_mask) { - TALLOC_FREE(fname_src_mask); - fname_src_mask = new_mask; - } - } - if (posix_pathname) { status = make_smb2_posix_create_ctx(talloc_tos(), &posx, 0777); if (!NT_STATUS_IS_OK(status)) { @@ -7684,27 +7645,6 @@ NTSTATUS rename_internals(TALLOC_CTX *ctx, } } - /* - * Only one file needs to be renamed. Append the mask back - * onto the directory. - */ - TALLOC_FREE(smb_fname_src->base_name); - if (ISDOT(fname_src_dir)) { - /* Ensure we use canonical names on open. */ - smb_fname_src->base_name = talloc_asprintf(smb_fname_src, - "%s", - fname_src_mask); - } else { - smb_fname_src->base_name = talloc_asprintf(smb_fname_src, - "%s/%s", - fname_src_dir, - fname_src_mask); - } - if (!smb_fname_src->base_name) { - status = NT_STATUS_NO_MEMORY; - goto out; - } - DBG_NOTICE("case_sensitive = %d, " "case_preserve = %d, short case preserve = %d, " "directory = %s, newname = %s, " @@ -7787,10 +7727,6 @@ NTSTATUS rename_internals(TALLOC_CTX *ctx, out: TALLOC_FREE(posx); - TALLOC_FREE(talloced); - TALLOC_FREE(smb_fname_src_dir); - TALLOC_FREE(fname_src_dir); - TALLOC_FREE(fname_src_mask); return status; } -- 2.30.2 From d02b86034e43fd92c7b560e4ac63f62eda72d822 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 9 Dec 2021 16:51:45 -0800 Subject: [PATCH 09/54] s3: smbd: Remove split_fname_dir_mask(). No longer used. Signed-off-by: Jeremy Allison --- source3/smbd/reply.c | 34 ---------------------------------- 1 file changed, 34 deletions(-) diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c index 47309eb18fa..203105014d8 100644 --- a/source3/smbd/reply.c +++ b/source3/smbd/reply.c @@ -1625,40 +1625,6 @@ void reply_dskattr(struct smb_request *req) return; } -#if 0 -/* - * Utility function to split the filename from the directory. - */ -static NTSTATUS split_fname_dir_mask(TALLOC_CTX *ctx, const char *fname_in, - char **fname_dir_out, - char **fname_mask_out) -{ - const char *p = NULL; - char *fname_dir = NULL; - char *fname_mask = NULL; - - p = strrchr_m(fname_in, '/'); - if (!p) { - fname_dir = talloc_strdup(ctx, "."); - fname_mask = talloc_strdup(ctx, fname_in); - } else { - fname_dir = talloc_strndup(ctx, fname_in, - PTR_DIFF(p, fname_in)); - fname_mask = talloc_strdup(ctx, p+1); - } - - if (!fname_dir || !fname_mask) { - TALLOC_FREE(fname_dir); - TALLOC_FREE(fname_mask); - return NT_STATUS_NO_MEMORY; - } - - *fname_dir_out = fname_dir; - *fname_mask_out = fname_mask; - return NT_STATUS_OK; -} -#endif - /**************************************************************************** Make a dir struct. ****************************************************************************/ -- 2.30.2 From 0033bcf491c743763bc8d77d4400e66e31ce9ea0 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 2 Dec 2021 17:51:42 -0800 Subject: [PATCH 10/54] s3: smbd: In call_trans2findfirst() we don't need filename_convert_with_privilege() anymore. It was extra-paranoid code now not needed as the new VFS version of filename_convert() does the same job. There are now no remaining callers of filename_convert_with_privilege(). Signed-off-by: Jeremy Allison --- source3/smbd/trans2.c | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/source3/smbd/trans2.c b/source3/smbd/trans2.c index 55857dd9119..5aabe244d45 100644 --- a/source3/smbd/trans2.c +++ b/source3/smbd/trans2.c @@ -2757,19 +2757,12 @@ close_if_end = %d requires_resume_key = %d backup_priv = %d level = 0x%x, max_da if (backup_priv) { become_root(); as_root = true; - ntstatus = filename_convert_with_privilege(talloc_tos(), - conn, - req, - directory, - ucf_flags, - &smb_dname); - } else { - ntstatus = filename_convert(talloc_tos(), conn, + } + ntstatus = filename_convert(talloc_tos(), conn, directory, ucf_flags, 0, &smb_dname); - } if (!NT_STATUS_IS_OK(ntstatus)) { if (NT_STATUS_EQUAL(ntstatus,NT_STATUS_PATH_NOT_COVERED)) { -- 2.30.2 From 1f82a0ad57f0d720b7bb6a103197ef1598671ed7 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 2 Dec 2021 17:55:26 -0800 Subject: [PATCH 11/54] s3: smbd: Remove filename_convert_with_privilege(). No longer used. Signed-off-by: Jeremy Allison --- source3/smbd/filename.c | 21 --------------------- source3/smbd/proto.h | 6 ------ 2 files changed, 27 deletions(-) diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c index b0f6ac5a99f..820a8780acd 100644 --- a/source3/smbd/filename.c +++ b/source3/smbd/filename.c @@ -2125,27 +2125,6 @@ NTSTATUS filename_convert(TALLOC_CTX *ctx, pp_smb_fname); } -/* - * Go through all the steps to validate a filename. - * root (privileged) version. - */ - -NTSTATUS filename_convert_with_privilege(TALLOC_CTX *ctx, - connection_struct *conn, - struct smb_request *smbreq, - const char *name_in, - uint32_t ucf_flags, - struct smb_filename **pp_smb_fname) -{ - return filename_convert_internal(ctx, - conn, - smbreq, - name_in, - ucf_flags, - 0, - pp_smb_fname); -} - /* * Build the full path from a dirfsp and dirfsp relative name */ diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h index 82a8234ba8a..97536c82c0f 100644 --- a/source3/smbd/proto.h +++ b/source3/smbd/proto.h @@ -378,12 +378,6 @@ NTSTATUS filename_convert(TALLOC_CTX *mem_ctx, uint32_t ucf_flags, NTTIME twrp, struct smb_filename **pp_smb_fname); -NTSTATUS filename_convert_with_privilege(TALLOC_CTX *mem_ctx, - connection_struct *conn, - struct smb_request *smbreq, - const char *name_in, - uint32_t ucf_flags, - struct smb_filename **pp_smb_fname); /* The following definitions come from smbd/files.c */ -- 2.30.2 From c7bfcf2300c802c3717c676251cd99c274b6ca03 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 3 Dec 2021 10:10:45 -0800 Subject: [PATCH 12/54] s3: smbd: In filename_convert_internal(), remove call to check_name_with_privilege(). We now always pass NULL as struct smb_request *smbreq, so this code path can never be taken. Comment out check_name_with_privilege() as it's now no longer used. Signed-off-by: Jeremy Allison --- source3/smbd/filename.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c index 820a8780acd..b72efc51b93 100644 --- a/source3/smbd/filename.c +++ b/source3/smbd/filename.c @@ -1499,6 +1499,7 @@ static NTSTATUS check_name(connection_struct *conn, return NT_STATUS_OK; } +#if 0 /**************************************************************************** Must be called as root. Creates the struct privilege_paths attached to the struct smb_request if this call is successful. @@ -1517,6 +1518,7 @@ static NTSTATUS check_name_with_privilege(connection_struct *conn, smb_fname, smbreq); } +#endif /**************************************************************************** Check if two filenames are equal. @@ -2046,11 +2048,8 @@ static NTSTATUS filename_convert_internal(TALLOC_CTX *ctx, TALLOC_FREE(smb_fname); return status; } - } else if (!smbreq) { - status = check_name(conn, smb_fname); } else { - status = check_name_with_privilege(conn, smbreq, - smb_fname); + status = check_name(conn, smb_fname); } if (!NT_STATUS_IS_OK(status)) { DEBUG(3,("filename_convert_internal: check_name failed " -- 2.30.2 From f2447305ac6cbbec96b80578ef21bfb918297a43 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 3 Dec 2021 10:13:13 -0800 Subject: [PATCH 13/54] s3: smbd: Remove unused check_name_with_privilege(). Signed-off-by: Jeremy Allison --- source3/smbd/filename.c | 21 --------------------- 1 file changed, 21 deletions(-) diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c index b72efc51b93..8aa0a6d48c8 100644 --- a/source3/smbd/filename.c +++ b/source3/smbd/filename.c @@ -1499,27 +1499,6 @@ static NTSTATUS check_name(connection_struct *conn, return NT_STATUS_OK; } -#if 0 -/**************************************************************************** - Must be called as root. Creates the struct privilege_paths - attached to the struct smb_request if this call is successful. -****************************************************************************/ - -static NTSTATUS check_name_with_privilege(connection_struct *conn, - struct smb_request *smbreq, - const struct smb_filename *smb_fname) -{ - NTSTATUS status = check_veto_path(conn, smb_fname); - - if (!NT_STATUS_IS_OK(status)) { - return status; - } - return check_reduced_name_with_privilege(conn, - smb_fname, - smbreq); -} -#endif - /**************************************************************************** Check if two filenames are equal. This needs to be careful about whether we are case sensitive. -- 2.30.2 From c25723aed50b12eaa88e0d5e87bd86fb628dde3b Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 3 Dec 2021 10:14:03 -0800 Subject: [PATCH 14/54] s3: smbd: Remove now unused check_reduced_name_with_privilege(). We now only have one function that does this check (check_reduced_name()), used everywhere. Signed-off-by: Jeremy Allison --- source3/smbd/proto.h | 3 - source3/smbd/vfs.c | 173 ------------------------------------------- 2 files changed, 176 deletions(-) diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h index 97536c82c0f..239df9879ea 100644 --- a/source3/smbd/proto.h +++ b/source3/smbd/proto.h @@ -1295,9 +1295,6 @@ struct smb_filename *vfs_GetWd(TALLOC_CTX *ctx, connection_struct *conn); NTSTATUS check_reduced_name(connection_struct *conn, const struct smb_filename *cwd_fname, const struct smb_filename *smb_fname); -NTSTATUS check_reduced_name_with_privilege(connection_struct *conn, - const struct smb_filename *smb_fname, - struct smb_request *smbreq); int vfs_stat(struct connection_struct *conn, struct smb_filename *smb_fname); int vfs_stat_smb_basename(struct connection_struct *conn, diff --git a/source3/smbd/vfs.c b/source3/smbd/vfs.c index 570de843811..4091aa304d4 100644 --- a/source3/smbd/vfs.c +++ b/source3/smbd/vfs.c @@ -1122,179 +1122,6 @@ struct smb_filename *vfs_GetWd(TALLOC_CTX *ctx, connection_struct *conn) return result; } -/******************************************************************* - Reduce a file name, removing .. elements and checking that - it is below dir in the hierarchy. This uses realpath. - This function must run as root, and will return names - and valid stat structs that can be checked on open. -********************************************************************/ - -NTSTATUS check_reduced_name_with_privilege(connection_struct *conn, - const struct smb_filename *smb_fname, - struct smb_request *smbreq) -{ - NTSTATUS status; - TALLOC_CTX *ctx = talloc_tos(); - const char *conn_rootdir; - size_t rootdir_len; - char *resolved_name = NULL; - struct smb_filename *resolved_fname = NULL; - struct smb_filename *saved_dir_fname = NULL; - struct smb_filename *smb_fname_cwd = NULL; - int ret; - struct smb_filename *parent_name = NULL; - struct smb_filename *file_name = NULL; - - DEBUG(3,("check_reduced_name_with_privilege [%s] [%s]\n", - smb_fname->base_name, - conn->connectpath)); - - status = SMB_VFS_PARENT_PATHNAME(conn, - ctx, - smb_fname, - &parent_name, - &file_name); - if (!NT_STATUS_IS_OK(status)) { - goto err; - } - - if (SMB_VFS_STAT(conn, parent_name) != 0) { - status = map_nt_error_from_unix(errno); - goto err; - } - /* Remember where we were. */ - saved_dir_fname = vfs_GetWd(ctx, conn); - if (!saved_dir_fname) { - status = map_nt_error_from_unix(errno); - goto err; - } - - if (vfs_ChDir(conn, parent_name) == -1) { - status = map_nt_error_from_unix(errno); - goto err; - } - - smb_fname_cwd = synthetic_smb_fname(talloc_tos(), - ".", - NULL, - NULL, - parent_name->twrp, - 0); - if (smb_fname_cwd == NULL) { - status = NT_STATUS_NO_MEMORY; - goto err; - } - - /* Get the absolute path of the parent directory. */ - resolved_fname = SMB_VFS_REALPATH(conn, ctx, smb_fname_cwd); - if (resolved_fname == NULL) { - status = map_nt_error_from_unix(errno); - goto err; - } - resolved_name = resolved_fname->base_name; - - if (*resolved_name != '/') { - DEBUG(0,("check_reduced_name_with_privilege: realpath " - "doesn't return absolute paths !\n")); - status = NT_STATUS_OBJECT_NAME_INVALID; - goto err; - } - - DBG_DEBUG("realpath [%s] -> [%s]\n", - smb_fname_str_dbg(parent_name), - resolved_name); - - /* Now check the stat value is the same. */ - if (SMB_VFS_LSTAT(conn, smb_fname_cwd) != 0) { - status = map_nt_error_from_unix(errno); - goto err; - } - - /* Ensure we're pointing at the same place. */ - if (!check_same_stat(&smb_fname_cwd->st, &parent_name->st)) { - DBG_ERR("device/inode/uid/gid on directory %s changed. " - "Denying access !\n", - smb_fname_str_dbg(parent_name)); - status = NT_STATUS_ACCESS_DENIED; - goto err; - } - - /* Ensure we're below the connect path. */ - - conn_rootdir = SMB_VFS_CONNECTPATH(conn, smb_fname); - if (conn_rootdir == NULL) { - DEBUG(2, ("check_reduced_name_with_privilege: Could not get " - "conn_rootdir\n")); - status = NT_STATUS_ACCESS_DENIED; - goto err; - } - - rootdir_len = strlen(conn_rootdir); - - /* - * In the case of rootdir_len == 1, we know that conn_rootdir is - * "/", and we also know that resolved_name starts with a slash. - * So, in this corner case, resolved_name is automatically a - * sub-directory of the conn_rootdir. Thus we can skip the string - * comparison and the next character checks (which are even - * wrong in this case). - */ - if (rootdir_len != 1) { - bool matched; - - matched = (strncmp(conn_rootdir, resolved_name, - rootdir_len) == 0); - - if (!matched || (resolved_name[rootdir_len] != '/' && - resolved_name[rootdir_len] != '\0')) { - DBG_WARNING("%s is a symlink outside the " - "share path\n", - smb_fname_str_dbg(parent_name)); - DEBUGADD(1, ("conn_rootdir =%s\n", conn_rootdir)); - DEBUGADD(1, ("resolved_name=%s\n", resolved_name)); - status = NT_STATUS_ACCESS_DENIED; - goto err; - } - } - - /* Now ensure that the last component either doesn't - exist, or is *NOT* a symlink. */ - - ret = SMB_VFS_LSTAT(conn, file_name); - if (ret == -1) { - /* Errno must be ENOENT for this be ok. */ - if (errno != ENOENT) { - status = map_nt_error_from_unix(errno); - DBG_WARNING("LSTAT on %s failed with %s\n", - smb_fname_str_dbg(file_name), - nt_errstr(status)); - goto err; - } - } - - if (VALID_STAT(file_name->st) && - S_ISLNK(file_name->st.st_ex_mode)) - { - DBG_WARNING("Last component %s is a symlink. Denying" - "access.\n", - smb_fname_str_dbg(file_name)); - status = NT_STATUS_ACCESS_DENIED; - goto err; - } - - status = NT_STATUS_OK; - - err: - - if (saved_dir_fname != NULL) { - vfs_ChDir(conn, saved_dir_fname); - TALLOC_FREE(saved_dir_fname); - } - TALLOC_FREE(resolved_fname); - TALLOC_FREE(parent_name); - return status; -} - /******************************************************************* Reduce a file name, removing .. elements and checking that it is below dir in the hierarchy. This uses realpath. -- 2.30.2 From 1a9d10822dc22441ce5c19a38ff7289f5858a84b Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 3 Dec 2021 10:19:38 -0800 Subject: [PATCH 15/54] s3: smbd: filename_convert() is now a one-to-one wrapper around filename_convert_internal(). Remove filename_convert() and rename filename_convert_internal() -> filename_convert(). Move the old DEBUG(..) statements to DBG_XXX() so they don't print the wrong name. Signed-off-by: Jeremy Allison --- source3/smbd/filename.c | 46 +++++++++++------------------------------ 1 file changed, 12 insertions(+), 34 deletions(-) diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c index 8aa0a6d48c8..e10e3da10d1 100644 --- a/source3/smbd/filename.c +++ b/source3/smbd/filename.c @@ -1960,13 +1960,12 @@ char *get_original_lcomp(TALLOC_CTX *ctx, * @return NT_STATUS_OK if all operations completed successfully, appropriate * error otherwise. */ -static NTSTATUS filename_convert_internal(TALLOC_CTX *ctx, - connection_struct *conn, - struct smb_request *smbreq, - const char *name_in, - uint32_t ucf_flags, - NTTIME twrp, - struct smb_filename **_smb_fname) +NTSTATUS filename_convert(TALLOC_CTX *ctx, + connection_struct *conn, + const char *name_in, + uint32_t ucf_flags, + NTTIME twrp, + struct smb_filename **_smb_fname) { struct smb_filename *smb_fname = NULL; bool has_wild; @@ -1982,10 +1981,10 @@ static NTSTATUS filename_convert_internal(TALLOC_CTX *ctx, !conn->sconn->using_smb2, &fname); if (!NT_STATUS_IS_OK(status)) { - DEBUG(10,("filename_convert_internal: dfs_redirect " + DBG_DEBUG("dfs_redirect " "failed for name %s with %s\n", name_in, - nt_errstr(status) )); + nt_errstr(status)); return status; } name_in = fname; @@ -2011,10 +2010,10 @@ static NTSTATUS filename_convert_internal(TALLOC_CTX *ctx, status = unix_convert(ctx, conn, name_in, twrp, &smb_fname, ucf_flags); if (!NT_STATUS_IS_OK(status)) { - DEBUG(10,("filename_convert_internal: unix_convert failed " + DBG_DEBUG("unix_convert failed " "for name %s with %s\n", name_in, - nt_errstr(status) )); + nt_errstr(status)); return status; } @@ -2031,10 +2030,10 @@ static NTSTATUS filename_convert_internal(TALLOC_CTX *ctx, status = check_name(conn, smb_fname); } if (!NT_STATUS_IS_OK(status)) { - DEBUG(3,("filename_convert_internal: check_name failed " + DBG_NOTICE("check_name failed " "for name %s with %s\n", smb_fname_str_dbg(smb_fname), - nt_errstr(status) )); + nt_errstr(status)); TALLOC_FREE(smb_fname); return status; } @@ -2082,27 +2081,6 @@ static NTSTATUS filename_convert_internal(TALLOC_CTX *ctx, return status; } -/* - * Go through all the steps to validate a filename. - * Non-root version. - */ - -NTSTATUS filename_convert(TALLOC_CTX *ctx, - connection_struct *conn, - const char *name_in, - uint32_t ucf_flags, - NTTIME twrp, - struct smb_filename **pp_smb_fname) -{ - return filename_convert_internal(ctx, - conn, - NULL, - name_in, - ucf_flags, - twrp, - pp_smb_fname); -} - /* * Build the full path from a dirfsp and dirfsp relative name */ -- 2.30.2 From 107c3f08f13eba3337877d4543ada11f46b0aea6 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 3 Dec 2021 16:00:26 -0800 Subject: [PATCH 16/54] s3: smbd: In dfs_path_lookup(). If we have a DFS path including a @GMT-token, don't throw away the twrp value when parsing the path. Not yet used. Signed-off-by: Jeremy Allison --- source3/smbd/msdfs.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/source3/smbd/msdfs.c b/source3/smbd/msdfs.c index fd002e98071..86ca79a994b 100644 --- a/source3/smbd/msdfs.c +++ b/source3/smbd/msdfs.c @@ -697,6 +697,7 @@ static NTSTATUS dfs_path_lookup(TALLOC_CTX *ctx, const struct dfs_path *pdp, /* Parsed out server+share+extrapath. */ uint32_t ucf_flags, + NTTIME *_twrp, int *consumedcntp, struct referral **ppreflist, size_t *preferral_count) @@ -867,6 +868,10 @@ static NTSTATUS dfs_path_lookup(TALLOC_CTX *ctx, } } + if ((ucf_flags & UCF_GMT_PATHNAME) && _twrp != NULL) { + *_twrp = smb_fname->twrp; + } + status = NT_STATUS_OK; out: @@ -965,6 +970,7 @@ NTSTATUS dfs_redirect(TALLOC_CTX *ctx, path_in, pdp, ucf_flags, + NULL, /* twrp. */ NULL, /* int *consumedcntp */ NULL, /* struct referral **ppreflist */ NULL); /* size_t *preferral_count */ @@ -1189,6 +1195,7 @@ NTSTATUS get_referred_path(TALLOC_CTX *ctx, dfs_path, pdp, 0, /* ucf_flags */ + NULL, consumedcntp, &jucn->referral_list, &jucn->referral_count); -- 2.30.2 From 6673d1aad747b8c0782dbcff69c17214a3c1819f Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 3 Dec 2021 16:14:08 -0800 Subject: [PATCH 17/54] s3: smbd: Allow dfs_redirect() to return a TWRP token it got from a parsed pathname. This one is subtle. If an SMB1 request has both a DFS path and a @GMT token, the unix_convert() inside the DFS path processing will remove the @GMT token, not allowing the subsequent unix_convert() inside filename_convert() to see it. By returning it from dfs_redirect() we can ensure it's correctly added to the smb_filename returned from filename_convert(). Signed-off-by: Jeremy Allison --- source3/smbd/filename.c | 11 +++++++++-- source3/smbd/msdfs.c | 3 ++- source3/smbd/proto.h | 1 + 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c index e10e3da10d1..2dab7f1b9b9 100644 --- a/source3/smbd/filename.c +++ b/source3/smbd/filename.c @@ -1871,6 +1871,7 @@ char *get_original_lcomp(TALLOC_CTX *ctx, char *last_slash = NULL; char *orig_lcomp; char *fname = NULL; + NTTIME twrp = 0; NTSTATUS status; if (ucf_flags & UCF_DFS_PATHNAME) { @@ -1879,6 +1880,7 @@ char *get_original_lcomp(TALLOC_CTX *ctx, filename_in, ucf_flags, !conn->sconn->using_smb2, + &twrp, &fname); if (!NT_STATUS_IS_OK(status)) { DBG_DEBUG("dfs_redirect " @@ -1907,7 +1909,7 @@ char *get_original_lcomp(TALLOC_CTX *ctx, filename_in, NULL, NULL, - 0, + twrp, 0); if (smb_fname == NULL) { TALLOC_FREE(fname); @@ -1915,7 +1917,7 @@ char *get_original_lcomp(TALLOC_CTX *ctx, } status = canonicalize_snapshot_path(smb_fname, ucf_flags, - 0); + twrp); if (!NT_STATUS_IS_OK(status)) { TALLOC_FREE(fname); TALLOC_FREE(smb_fname); @@ -1975,10 +1977,12 @@ NTSTATUS filename_convert(TALLOC_CTX *ctx, if (ucf_flags & UCF_DFS_PATHNAME) { char *fname = NULL; + NTTIME dfs_twrp = 0; status = dfs_redirect(ctx, conn, name_in, ucf_flags, !conn->sconn->using_smb2, + &dfs_twrp, &fname); if (!NT_STATUS_IS_OK(status)) { DBG_DEBUG("dfs_redirect " @@ -1989,6 +1993,9 @@ NTSTATUS filename_convert(TALLOC_CTX *ctx, } name_in = fname; ucf_flags &= ~UCF_DFS_PATHNAME; + if (twrp == 0 && dfs_twrp != 0) { + twrp = dfs_twrp; + } } if (is_fake_file_path(name_in)) { diff --git a/source3/smbd/msdfs.c b/source3/smbd/msdfs.c index 86ca79a994b..07636592016 100644 --- a/source3/smbd/msdfs.c +++ b/source3/smbd/msdfs.c @@ -901,6 +901,7 @@ NTSTATUS dfs_redirect(TALLOC_CTX *ctx, const char *path_in, uint32_t ucf_flags, bool allow_broken_path, + NTTIME *_twrp, char **pp_path_out) { const struct loadparm_substitution *lp_sub = @@ -970,7 +971,7 @@ NTSTATUS dfs_redirect(TALLOC_CTX *ctx, path_in, pdp, ucf_flags, - NULL, /* twrp. */ + _twrp, /* twrp. */ NULL, /* int *consumedcntp */ NULL, /* struct referral **ppreflist */ NULL); /* size_t *preferral_count */ diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h index 239df9879ea..52d0390b86a 100644 --- a/source3/smbd/proto.h +++ b/source3/smbd/proto.h @@ -563,6 +563,7 @@ NTSTATUS dfs_redirect(TALLOC_CTX *ctx, 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.30.2 From 6f81694a40547ccbe5142e112a61d7b4c429e82c Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 3 Dec 2021 10:35:09 -0800 Subject: [PATCH 18/54] s3: smbd: Add filename_convert_smb1_search_path() - deals with SMB1 search pathnames. SMB1search and trans2 findfirst are unique in that they are the only passed in pathnames that can contain a terminal wildcard component. Deal with these two special cases with this new function that strips off the terminal wildcard and returns as the mask, and pass the non-wildcard parent directory component through the standard filename_convert(). Uses new helper function strip_gmt_from_raw_dfs(). When SMB1search and trans2 findfirst have been converted to use this function, we can strip all wildcard handling out of filename_convert() as we now know it will only ever be given valid pathnames. Signed-off-by: Jeremy Allison --- source3/smbd/filename.c | 240 ++++++++++++++++++++++++++++++++++++++++ source3/smbd/proto.h | 6 + 2 files changed, 246 insertions(+) diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c index 2dab7f1b9b9..e9bfbf790bf 100644 --- a/source3/smbd/filename.c +++ b/source3/smbd/filename.c @@ -2088,6 +2088,246 @@ NTSTATUS filename_convert(TALLOC_CTX *ctx, return status; } +/* + * Strip a @GMT component from an SMB1-DFS path. Could be anywhere + * in the path. + */ + +static char *strip_gmt_from_raw_dfs(TALLOC_CTX *ctx, + const char *name_in, + bool posix_pathnames, + NTTIME *_twrp) +{ + NTSTATUS status; + struct smb_filename *smb_fname = NULL; + char *name_out = NULL; + + smb_fname = synthetic_smb_fname(ctx, + name_in, + NULL, + NULL, + 0, + 0); + if (smb_fname == NULL) { + return NULL; + } + if (!posix_pathnames) { + /* + * Raw DFS names are still '\\' separated. + * canonicalize_snapshot_path() only works + * on '/' separated paths. Convert. + */ + string_replace(smb_fname->base_name, '\\', '/'); + } + status = canonicalize_snapshot_path(smb_fname, + UCF_GMT_PATHNAME, + 0); + if (!NT_STATUS_IS_OK(status)) { + TALLOC_FREE(smb_fname); + return NULL; + } + if (!posix_pathnames) { + /* Replace as raw DFS names. */ + string_replace(smb_fname->base_name, '/', '\\'); + } + name_out = talloc_strdup(ctx, smb_fname->base_name); + *_twrp = smb_fname->twrp; + TALLOC_FREE(smb_fname); + return name_out; +} + +/* + * Deal with the SMB1 semantics of sending a pathname with a + * wildcard as the terminal component for a SMB1search or + * trans2 findfirst. + */ + +NTSTATUS filename_convert_smb1_search_path(TALLOC_CTX *ctx, + connection_struct *conn, + const char *name_in, + uint32_t ucf_flags, + struct smb_filename **_smb_fname_out, + char **_mask_out) +{ + NTSTATUS status; + char *p = NULL; + char *mask = NULL; + struct smb_filename *smb_fname = NULL; + bool posix_pathnames = (ucf_flags & UCF_POSIX_PATHNAMES); + NTTIME twrp = 0; + TALLOC_CTX *frame = talloc_stackframe(); + + *_smb_fname_out = NULL; + *_mask_out = NULL; + + DBG_DEBUG("name_in: %s\n", name_in); + + if (ucf_flags & UCF_DFS_PATHNAME) { + /* + * We've been given a raw DFS pathname. + * In Windows mode this is separated by '\\' + * characters. + * + * We need to remove the last component + * which must be a wildcard before passing + * to dfs_redirect(). But the last component + * may also be a @GMT- token so we have to + * remove that first. + */ + char path_sep = posix_pathnames ? '/' : '\\'; + char *fname = NULL; + char *name_in_copy = NULL; + char *last_component = NULL; + + /* Work on a copy of name_in. */ + if (ucf_flags & UCF_GMT_PATHNAME) { + name_in_copy = strip_gmt_from_raw_dfs(frame, + name_in, + posix_pathnames, + &twrp); + ucf_flags &= ~UCF_GMT_PATHNAME; + } else { + name_in_copy = talloc_strdup(frame, name_in); + } + if (name_in_copy == NULL) { + TALLOC_FREE(frame); + return NT_STATUS_NO_MEMORY; + } + + /* + * Now we know that the last component is the + * wildcard. Copy it and truncate to remove it. + */ + p = strrchr_m(name_in_copy, path_sep); + if (p == NULL) { + last_component = talloc_strdup(frame, name_in_copy); + name_in_copy[0] = '\0'; + } else { + last_component = talloc_strdup(frame, p+1); + *p = '\0'; + } + if (last_component == NULL) { + TALLOC_FREE(frame); + return NT_STATUS_NO_MEMORY; + } + + DBG_DEBUG("name_in_copy: %s\n", name_in); + + /* + * Now we can call dfs_redirect() + * on the name without wildcard. + */ + status = dfs_redirect(frame, + conn, + name_in_copy, + ucf_flags, + !conn->sconn->using_smb2, + NULL, + &fname); + if (!NT_STATUS_IS_OK(status)) { + DBG_DEBUG("dfs_redirect " + "failed for name %s with %s\n", + name_in_copy, + nt_errstr(status)); + TALLOC_FREE(frame); + return status; + } + /* Add the last component back. */ + if (fname[0] == '\0') { + name_in = talloc_strdup(frame, last_component); + } else { + name_in = talloc_asprintf(frame, + "%s%c%s", + fname, + path_sep, + last_component); + } + if (name_in == NULL) { + TALLOC_FREE(frame); + return NT_STATUS_NO_MEMORY; + } + ucf_flags &= ~UCF_DFS_PATHNAME; + + DBG_DEBUG("After DFS redirect name_in: %s\n", name_in); + } + + smb_fname = synthetic_smb_fname(frame, + name_in, + NULL, + NULL, + twrp, + posix_pathnames ? + SMB_FILENAME_POSIX_PATH : 0); + if (smb_fname == NULL) { + TALLOC_FREE(frame); + return NT_STATUS_NO_MEMORY; + } + + /* Canonicalize any @GMT- paths. */ + status = canonicalize_snapshot_path(smb_fname, ucf_flags, twrp); + if (!NT_STATUS_IS_OK(status)) { + TALLOC_FREE(frame); + return status; + } + + /* Get the original lcomp. */ + mask = get_original_lcomp(frame, + conn, + name_in, + ucf_flags); + if (mask == NULL) { + TALLOC_FREE(frame); + return NT_STATUS_NO_MEMORY; + } + + if (mask[0] == '\0') { + /* Windows and OS/2 systems treat search on the root as * */ + TALLOC_FREE(mask); + mask = talloc_strdup(frame, "*"); + if (mask == NULL) { + TALLOC_FREE(frame); + return NT_STATUS_NO_MEMORY; + } + } + + DBG_DEBUG("mask = %s\n", mask); + + /* + * Remove the terminal component so + * filename_convert never sees the mask. + */ + p = strrchr_m(smb_fname->base_name,'/'); + if (p == NULL) { + /* filename_convert handles a '\0' base_name. */ + smb_fname->base_name[0] = '\0'; + } else { + *p = '\0'; + } + + DBG_DEBUG("For filename_convert: smb_fname = %s\n", + smb_fname_str_dbg(smb_fname)); + + /* Convert the parent directory path. */ + status = filename_convert(frame, + conn, + smb_fname->base_name, + ucf_flags, + smb_fname->twrp, + &smb_fname); + + if (NT_STATUS_IS_OK(status)) { + *_smb_fname_out = talloc_move(ctx, &smb_fname); + *_mask_out = talloc_move(ctx, &mask); + } else { + DBG_DEBUG("filename_convert error for %s: %s\n", + smb_fname_str_dbg(smb_fname), + nt_errstr(status)); + } + + TALLOC_FREE(frame); + return status; +} + /* * Build the full path from a dirfsp and dirfsp relative name */ diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h index 52d0390b86a..1e31ded1b1a 100644 --- a/source3/smbd/proto.h +++ b/source3/smbd/proto.h @@ -378,6 +378,12 @@ NTSTATUS filename_convert(TALLOC_CTX *mem_ctx, uint32_t ucf_flags, NTTIME twrp, struct smb_filename **pp_smb_fname); +NTSTATUS filename_convert_smb1_search_path(TALLOC_CTX *ctx, + connection_struct *conn, + const char *name_in, + uint32_t ucf_flags, + struct smb_filename **_smb_fname_out, + char **_mask_out); /* The following definitions come from smbd/files.c */ -- 2.30.2 From 6d61b76c4416a4ef95126562c982c9e0e6bb4b13 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 3 Dec 2021 11:22:03 -0800 Subject: [PATCH 19/54] s3: smbd: Convert reply_search() to use filename_convert_smb1_search_path(). Cleans up this code path nicely ! Signed-off-by: Jeremy Allison --- source3/smbd/reply.c | 71 ++++++++++---------------------------------- 1 file changed, 16 insertions(+), 55 deletions(-) diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c index 203105014d8..d7c5b962ca7 100644 --- a/source3/smbd/reply.c +++ b/source3/smbd/reply.c @@ -1748,15 +1748,17 @@ void reply_search(struct smb_request *req) /* dirtype &= ~FILE_ATTRIBUTE_DIRECTORY; */ if (status_len == 0) { - int ret; + const char *dirpath; struct smb_filename *smb_dname = NULL; - uint32_t ucf_flags = UCF_ALWAYS_ALLOW_WCARD_LCOMP | - ucf_flags_from_smb_request(req); - nt_status = filename_convert(ctx, conn, - path, - ucf_flags, - 0, - &smb_fname); + uint32_t ucf_flags = ucf_flags_from_smb_request(req); + + nt_status = filename_convert_smb1_search_path(ctx, + conn, + path, + ucf_flags, + &smb_dname, + &mask); + if (!NT_STATUS_IS_OK(nt_status)) { if (NT_STATUS_EQUAL(nt_status,NT_STATUS_PATH_NOT_COVERED)) { reply_botherror(req, NT_STATUS_PATH_NOT_COVERED, @@ -1767,56 +1769,9 @@ void reply_search(struct smb_request *req) goto out; } - directory = smb_fname->base_name; - - p = strrchr_m(directory,'/'); - if ((p != NULL) && (*directory != '/')) { - mask = talloc_strdup(ctx, p + 1); - directory = talloc_strndup(ctx, directory, - PTR_DIFF(p, directory)); - } else { - mask = talloc_strdup(ctx, directory); - directory = talloc_strdup(ctx,"."); - } - - if (!directory) { - reply_nterror(req, NT_STATUS_NO_MEMORY); - goto out; - } - memset((char *)status,'\0',21); SCVAL(status,0,(dirtype & 0x1F)); - smb_dname = synthetic_smb_fname(talloc_tos(), - directory, - NULL, - NULL, - smb_fname->twrp, - smb_fname->flags); - if (smb_dname == NULL) { - reply_nterror(req, NT_STATUS_NO_MEMORY); - goto out; - } - - /* - * As we've cut off the last component from - * smb_fname we need to re-stat smb_dname - * so FILE_OPEN disposition knows the directory - * exists. - */ - ret = vfs_stat(conn, smb_dname); - if (ret == -1) { - nt_status = map_nt_error_from_unix(errno); - reply_nterror(req, nt_status); - goto out; - } - - nt_status = openat_pathref_fsp(conn->cwd_fsp, smb_dname); - if (!NT_STATUS_IS_OK(nt_status)) { - reply_nterror(req, nt_status); - goto out; - } - /* * Open an fsp on this directory for the dptr. */ @@ -1873,6 +1828,12 @@ void reply_search(struct smb_request *req) } dptr_num = dptr_dnum(fsp->dptr); + dirpath = dptr_path(sconn, dptr_num); + directory = talloc_strdup(ctx, dirpath); + if (!directory) { + reply_nterror(req, NT_STATUS_NO_MEMORY); + goto out; + } } else { int status_dirtype; -- 2.30.2 From 3b56c6328ba428ccd45c9c7f6f6429e23aed1ec5 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 3 Dec 2021 11:28:40 -0800 Subject: [PATCH 20/54] s3: smbd: Fix call_trans2findfirst() to use filename_convert_smb1_search_path(). filename_convert() no longer has to handle wildcards. UCF_ALWAYS_ALLOW_WCARD_LCOMP is now unused. Signed-off-by: Jeremy Allison --- source3/smbd/trans2.c | 99 ++++--------------------------------------- 1 file changed, 8 insertions(+), 91 deletions(-) diff --git a/source3/smbd/trans2.c b/source3/smbd/trans2.c index 5aabe244d45..7e148baac55 100644 --- a/source3/smbd/trans2.c +++ b/source3/smbd/trans2.c @@ -2665,14 +2665,12 @@ static void call_trans2findfirst(connection_struct *conn, NTSTATUS ntstatus = NT_STATUS_OK; bool ask_sharemode = lp_smbd_search_ask_sharemode(SNUM(conn)); struct smbd_server_connection *sconn = req->sconn; - uint32_t ucf_flags = UCF_ALWAYS_ALLOW_WCARD_LCOMP | - ucf_flags_from_smb_request(req); + uint32_t ucf_flags = ucf_flags_from_smb_request(req); bool backup_priv = false; bool as_root = false; files_struct *fsp = NULL; const struct loadparm_substitution *lp_sub = loadparm_s3_global_substitution(); - int ret; if (total_params < 13) { reply_nterror(req, NT_STATUS_INVALID_PARAMETER); @@ -2758,11 +2756,12 @@ close_if_end = %d requires_resume_key = %d backup_priv = %d level = 0x%x, max_da become_root(); as_root = true; } - ntstatus = filename_convert(talloc_tos(), conn, - directory, - ucf_flags, - 0, - &smb_dname); + ntstatus = filename_convert_smb1_search_path(talloc_tos(), + conn, + directory, + ucf_flags, + &smb_dname, + &mask); if (!NT_STATUS_IS_OK(ntstatus)) { if (NT_STATUS_EQUAL(ntstatus,NT_STATUS_PATH_NOT_COVERED)) { @@ -2774,72 +2773,9 @@ close_if_end = %d requires_resume_key = %d backup_priv = %d level = 0x%x, max_da goto out; } - /* - * The above call to filename_convert() is on the path from the client - * including the search mask. Until the code that chops of the search - * mask from the path below is moved before the call to - * filename_convert(), we close a possible pathref fsp to ensure - * SMB_VFS_CREATE_FILE() below will internally open a pathref fsp on the - * correct path. - */ - if (smb_dname->fsp != NULL) { - ntstatus = fd_close(smb_dname->fsp); - if (!NT_STATUS_IS_OK(ntstatus)) { - reply_nterror(req, ntstatus); - goto out; - } - /* - * The pathref fsp link destructor will set smb_dname->fsp to - * NULL. Turning this into an assert to give a hint at readers - * of the code trying to understand the mechanics. - */ - file_free(req, smb_dname->fsp); - SMB_ASSERT(smb_dname->fsp == NULL); - } - - mask = get_original_lcomp(talloc_tos(), - conn, - directory, - ucf_flags); - if (mask == NULL) { - reply_nterror(req, NT_STATUS_NO_MEMORY); - goto out; - } - + TALLOC_FREE(directory); directory = smb_dname->base_name; - p = strrchr_m(directory,'/'); - if(p == NULL) { - /* Windows and OS/2 systems treat search on the root '\' as if it were '\*' */ - if((directory[0] == '.') && (directory[1] == '\0')) { - mask = talloc_strdup(talloc_tos(),"*"); - if (!mask) { - reply_nterror(req, NT_STATUS_NO_MEMORY); - goto out; - } - } - } else { - *p = 0; - } - - if (p == NULL || p == directory) { - struct smb_filename *old_name = smb_dname; - - /* Ensure we don't have a directory name of "". */ - smb_dname = synthetic_smb_fname(talloc_tos(), - ".", - NULL, - &old_name->st, - old_name->twrp, - old_name->flags); - TALLOC_FREE(old_name); - if (smb_dname == NULL) { - reply_nterror(req, NT_STATUS_NO_MEMORY); - goto out; - } - directory = smb_dname->base_name; - } - DEBUG(5,("dir=%s, mask = %s\n",directory, mask)); if (info_level == SMB_FIND_EA_LIST) { @@ -2897,25 +2833,6 @@ total_data=%u (should be %u)\n", (unsigned int)total_data, (unsigned int)IVAL(pd } params = *pparams; - /* - * As we've cut off the last component from - * smb_fname we need to re-stat smb_dname - * so FILE_OPEN disposition knows the directory - * exists. - */ - ret = vfs_stat(conn, smb_dname); - if (ret == -1) { - ntstatus = map_nt_error_from_unix(errno); - reply_nterror(req, ntstatus); - goto out; - } - - ntstatus = openat_pathref_fsp(conn->cwd_fsp, smb_dname); - if (!NT_STATUS_IS_OK(ntstatus)) { - reply_nterror(req, ntstatus); - goto out; - } - /* * Open an fsp on this directory for the dptr. */ -- 2.30.2 From fc4b67cb7385cccf0620743a361f3a789cce982f Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 3 Dec 2021 11:31:40 -0800 Subject: [PATCH 21/54] s3: smbd: dfs_path_lookup() no longer deals with wildcards. Signed-off-by: Jeremy Allison --- source3/smbd/msdfs.c | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/source3/smbd/msdfs.c b/source3/smbd/msdfs.c index 07636592016..6a3abd2f6eb 100644 --- a/source3/smbd/msdfs.c +++ b/source3/smbd/msdfs.c @@ -680,10 +680,6 @@ bool is_msdfs_link(struct files_struct *dirfsp, Used by other functions to decide if a dfs path is remote, and to get the list of referred locations for that remote path. - search_flag: For findfirsts, dfs links themselves are not - redirected, but paths beyond the links are. For normal smb calls, - even dfs links need to be redirected. - consumedcntp: how much of the dfs path is being redirected. the client should try the remaining path on the redirected server. @@ -755,15 +751,6 @@ static NTSTATUS dfs_path_lookup(TALLOC_CTX *ctx, TALLOC_FREE(parent_fname); if (NT_STATUS_IS_OK(status)) { - /* XX_ALLOW_WCARD_XXX is called from search functions.*/ - if (ucf_flags & UCF_ALWAYS_ALLOW_WCARD_LCOMP) { - DBG_INFO("(FindFirst) No redirection " - "for dfs link %s.\n", - dfspath); - status = NT_STATUS_OK; - goto out; - } - DBG_INFO("%s resolves to a valid dfs link\n", dfspath); -- 2.30.2 From 53a94909a1763ef61216f815aafa4f7dc5c198a3 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 3 Dec 2021 11:42:23 -0800 Subject: [PATCH 22/54] s3: smbd: Remove 'bool search_wcard_flag' from parse_dfs_path(). Never set. Signed-off-by: Jeremy Allison --- source3/smbd/msdfs.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/source3/smbd/msdfs.c b/source3/smbd/msdfs.c index 6a3abd2f6eb..1feeec6fff2 100644 --- a/source3/smbd/msdfs.c +++ b/source3/smbd/msdfs.c @@ -894,14 +894,13 @@ NTSTATUS dfs_redirect(TALLOC_CTX *ctx, const struct loadparm_substitution *lp_sub = loadparm_s3_global_substitution(); NTSTATUS status; - bool search_wcard_flag = (ucf_flags & UCF_ALWAYS_ALLOW_WCARD_LCOMP); struct dfs_path *pdp = talloc(ctx, struct dfs_path); if (!pdp) { return NT_STATUS_NO_MEMORY; } - status = parse_dfs_path(conn, path_in, search_wcard_flag, + status = parse_dfs_path(conn, path_in, false, allow_broken_path, pdp); if (!NT_STATUS_IS_OK(status)) { TALLOC_FREE(pdp); -- 2.30.2 From af88f9c225d91f5a560c962c6486de6c9e0aacec Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 3 Dec 2021 11:48:23 -0800 Subject: [PATCH 23/54] s3: smbd: parse_dfs_path() can ignore wildcards. If one is passed to filename_convert(), it will error out there with NT_STATUS_OBJECT_NAME_INVALID. Signed-off-by: Jeremy Allison --- source3/smbd/msdfs.c | 6 ------ 1 file changed, 6 deletions(-) diff --git a/source3/smbd/msdfs.c b/source3/smbd/msdfs.c index 1feeec6fff2..c003b442baa 100644 --- a/source3/smbd/msdfs.c +++ b/source3/smbd/msdfs.c @@ -215,12 +215,6 @@ static NTSTATUS parse_dfs_path(connection_struct *conn, if (pdp->posix_path) { status = check_path_syntax_posix(pdp->reqpath); } else { - if (!allow_wcards) { - bool has_wcard = ms_has_wild(pdp->reqpath); - if (has_wcard) { - return NT_STATUS_INVALID_PARAMETER; - } - } status = check_path_syntax(pdp->reqpath); } -- 2.30.2 From e95bedf6760e2aeef8dab8ff9d6bf88bd6495a98 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 3 Dec 2021 11:30:42 -0800 Subject: [PATCH 24/54] s3: smbd: filename_convert() no longer deals with wildcards. These are already errored out with NT_STATUS_OBJECT_NAME_INVALID in the unix_convert() code. Remove the check. Signed-off-by: Jeremy Allison --- source3/smbd/filename.c | 9 --------- 1 file changed, 9 deletions(-) diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c index e9bfbf790bf..5988396ee13 100644 --- a/source3/smbd/filename.c +++ b/source3/smbd/filename.c @@ -1970,7 +1970,6 @@ NTSTATUS filename_convert(TALLOC_CTX *ctx, struct smb_filename **_smb_fname) { struct smb_filename *smb_fname = NULL; - bool has_wild; NTSTATUS status; *_smb_fname = NULL; @@ -2045,14 +2044,6 @@ NTSTATUS filename_convert(TALLOC_CTX *ctx, return status; } - has_wild = ms_has_wild(name_in); - if (has_wild) { - DBG_DEBUG("[%s] contains wildcard, skipping pathref fsp\n", - name_in); - *_smb_fname = smb_fname; - return NT_STATUS_OK; - } - if (!VALID_STAT(smb_fname->st)) { DBG_DEBUG("[%s] does not exist, skipping pathref fsp\n", smb_fname_str_dbg(smb_fname)); -- 2.30.2 From 9607b0639b9c31cca0c8dd7d12bf1f2b6a1aba08 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 3 Dec 2021 11:33:42 -0800 Subject: [PATCH 25/54] s3: smbd: Inside 'struct uc_state', remove allow_wcard_last_component. This is never allowed. Signed-off-by: Jeremy Allison --- source3/smbd/filename.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c index 5988396ee13..fe3db77c862 100644 --- a/source3/smbd/filename.c +++ b/source3/smbd/filename.c @@ -538,7 +538,6 @@ struct uc_state { bool component_was_mangled; bool name_has_wildcard; bool posix_pathnames; - bool allow_wcard_last_component; bool done; bool case_sensitive; bool case_preserve; @@ -896,7 +895,7 @@ static NTSTATUS unix_convert_step(struct uc_state *state) return NT_STATUS_OBJECT_NAME_INVALID; } return determine_path_error(state->end+1, - state->allow_wcard_last_component, + false, state->posix_pathnames); } @@ -988,7 +987,6 @@ NTSTATUS unix_convert(TALLOC_CTX *mem_ctx, .orig_path = orig_path, .ucf_flags = ucf_flags, .posix_pathnames = (ucf_flags & UCF_POSIX_PATHNAMES), - .allow_wcard_last_component = (ucf_flags & UCF_ALWAYS_ALLOW_WCARD_LCOMP), .case_sensitive = conn->case_sensitive, .case_preserve = conn->case_preserve, .short_case_preserve = conn->short_case_preserve, @@ -1074,7 +1072,7 @@ NTSTATUS unix_convert(TALLOC_CTX *mem_ctx, status = NT_STATUS_OBJECT_NAME_INVALID; } else { status =determine_path_error(&state->orig_path[2], - state->allow_wcard_last_component, + false, state->posix_pathnames); } goto err; @@ -1201,7 +1199,7 @@ NTSTATUS unix_convert(TALLOC_CTX *mem_ctx, if (!state->posix_pathnames) { /* POSIX pathnames have no wildcards. */ state->name_has_wildcard = ms_has_wild(state->smb_fname->base_name); - if (state->name_has_wildcard && !state->allow_wcard_last_component) { + if (state->name_has_wildcard) { /* Wildcard not valid anywhere. */ status = NT_STATUS_OBJECT_NAME_INVALID; goto fail; -- 2.30.2 From 8c087bc26d1e071662b558a145a9bce23c09b507 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 3 Dec 2021 12:37:15 -0800 Subject: [PATCH 26/54] s3: smbd: We no longer need determine_path_error(). Now we don't have to consider wildcards just return NT_STATUS_OBJECT_PATH_NOT_FOUND for the cases we used to call it. Signed-off-by: Jeremy Allison --- source3/smbd/filename.c | 47 ++--------------------------------------- 1 file changed, 2 insertions(+), 45 deletions(-) diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c index fe3db77c862..601832fdb49 100644 --- a/source3/smbd/filename.c +++ b/source3/smbd/filename.c @@ -96,45 +96,6 @@ static bool mangled_equal(const char *name1, return strequal(name1, mname); } -/**************************************************************************** - Cope with the differing wildcard and non-wildcard error cases. -****************************************************************************/ - -static NTSTATUS determine_path_error(const char *name, - bool allow_wcard_last_component, - bool posix_pathnames) -{ - const char *p; - bool name_has_wild = false; - - if (!allow_wcard_last_component) { - /* Error code within a pathname. */ - return NT_STATUS_OBJECT_PATH_NOT_FOUND; - } - - /* We're terminating here so we - * can be a little slower and get - * the error code right. Windows - * treats the last part of the pathname - * separately I think, so if the last - * component is a wildcard then we treat - * this ./ as "end of component" */ - - p = strchr(name, '/'); - - if (!posix_pathnames) { - name_has_wild = ms_has_wild(name); - } - - if (!p && (name_has_wild || ISDOT(name))) { - /* Error code at the end of a pathname. */ - return NT_STATUS_OBJECT_NAME_INVALID; - } else { - /* Error code within a pathname. */ - return NT_STATUS_OBJECT_PATH_NOT_FOUND; - } -} - static NTSTATUS check_for_dot_component(const struct smb_filename *smb_fname) { /* Ensure we catch all names with in "/." @@ -894,9 +855,7 @@ static NTSTATUS unix_convert_step(struct uc_state *state) /* Error code at the end of a pathname. */ return NT_STATUS_OBJECT_NAME_INVALID; } - return determine_path_error(state->end+1, - false, - state->posix_pathnames); + return NT_STATUS_OBJECT_PATH_NOT_FOUND; } /* The name cannot have a wildcard if it's not @@ -1071,9 +1030,7 @@ NTSTATUS unix_convert(TALLOC_CTX *mem_ctx, if (state->orig_path[1] == '\0' || state->orig_path[2] == '\0') { status = NT_STATUS_OBJECT_NAME_INVALID; } else { - status =determine_path_error(&state->orig_path[2], - false, - state->posix_pathnames); + status = NT_STATUS_OBJECT_PATH_NOT_FOUND; } goto err; } -- 2.30.2 From c490cacf8f30658c88bb8fbc536881840ff0627f Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 3 Dec 2021 12:40:43 -0800 Subject: [PATCH 27/54] s3: smbd: UCF_ALWAYS_ALLOW_WCARD_LCOMP 0x00000002 is no longer used. Hurrah ! Signed-off-by: Jeremy Allison --- source3/smbd/filename.c | 3 --- source3/smbd/smbd.h | 2 +- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c index 601832fdb49..995547aecaa 100644 --- a/source3/smbd/filename.c +++ b/source3/smbd/filename.c @@ -473,9 +473,6 @@ 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 UCF_ALWAYS_ALLOW_WCARD_LCOMP is passed in, then a MS wildcard -should be allowed in the last component of the path only. - 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. diff --git a/source3/smbd/smbd.h b/source3/smbd/smbd.h index 84fcf033a71..88bff2830d9 100644 --- a/source3/smbd/smbd.h +++ b/source3/smbd/smbd.h @@ -61,7 +61,7 @@ struct trans_state { * unix_convert_flags */ /* UCF_SAVE_LCOMP 0x00000001 is no longer used. */ -#define UCF_ALWAYS_ALLOW_WCARD_LCOMP 0x00000002 +/* UCF_ALWAYS_ALLOW_WCARD_LCOMP 0x00000002 is no longer used. */ /* UCF_COND_ALLOW_WCARD_LCOMP 0x00000004 is no longer used. */ #define UCF_POSIX_PATHNAMES 0x00000008 /* #define UCF_UNIX_NAME_LOOKUP 0x00000010 is no longer used. */ -- 2.30.2 From 3aa2a2e9298900aab3c960698bb08de47aecdc2e Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 3 Dec 2021 12:53:36 -0800 Subject: [PATCH 28/54] s3: smbd: Inside unix_convert(), never set state->name_is_wildcard. We error out immediately if it's set anyway. Preparing to remove 'state->name_is_wildcard' structure element. Signed-off-by: Jeremy Allison --- source3/smbd/filename.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c index 995547aecaa..266815d540e 100644 --- a/source3/smbd/filename.c +++ b/source3/smbd/filename.c @@ -1152,8 +1152,8 @@ NTSTATUS unix_convert(TALLOC_CTX *mem_ctx, if (!state->posix_pathnames) { /* POSIX pathnames have no wildcards. */ - state->name_has_wildcard = ms_has_wild(state->smb_fname->base_name); - if (state->name_has_wildcard) { + 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; -- 2.30.2 From fe415eb42370e1fb50d198668639918f3f187b4a Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 3 Dec 2021 12:55:41 -0800 Subject: [PATCH 29/54] s3: smbd: In unix_convert(), remove all references to state->name_has_wildcard. It is never set. Signed-off-by: Jeremy Allison --- source3/smbd/filename.c | 25 ++----------------------- 1 file changed, 2 insertions(+), 23 deletions(-) diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c index 266815d540e..0bd06392ad1 100644 --- a/source3/smbd/filename.c +++ b/source3/smbd/filename.c @@ -1163,7 +1163,7 @@ NTSTATUS unix_convert(TALLOC_CTX *mem_ctx, DBG_DEBUG("Begin: name [%s] dirpath [%s] name [%s]\n", state->smb_fname->base_name, state->dirpath, state->name); - if (!state->name_has_wildcard) { + { int parent_stat_errno = 0; /* @@ -1280,27 +1280,6 @@ NTSTATUS unix_convert(TALLOC_CTX *mem_ctx, goto done; } } - } else { - /* - * We have a wildcard in the pathname. - * - * Optimization for common case where the wildcard - * is in the last component and the client already - * sent the correct case. - * 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, - NULL); - errno = saved_errno; - if (!NT_STATUS_IS_OK(status)) { - goto fail; - } } /* @@ -1346,7 +1325,7 @@ NTSTATUS unix_convert(TALLOC_CTX *mem_ctx, * components as this can change the size. */ - if(!state->component_was_mangled && !state->name_has_wildcard) { + if(!state->component_was_mangled) { stat_cache_add(state->orig_path, state->smb_fname->base_name, state->smb_fname->twrp, -- 2.30.2 From 27c9cf651348cf89a6e82344c0bb7fa67f234f1a Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 3 Dec 2021 12:59:50 -0800 Subject: [PATCH 30/54] s3: smbd: In unix_convert() remove the now unneeded block indentation. We removed the 'if (state->name_has_wildcard) {' clause, so the block no longer needs indenting. Best seen with git show -b. Signed-off-by: Jeremy Allison --- source3/smbd/filename.c | 199 ++++++++++++++++++++-------------------- 1 file changed, 98 insertions(+), 101 deletions(-) diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c index 0bd06392ad1..cd51602a55c 100644 --- a/source3/smbd/filename.c +++ b/source3/smbd/filename.c @@ -936,6 +936,7 @@ NTSTATUS unix_convert(TALLOC_CTX *mem_ctx, struct uc_state *state = &uc_state; NTSTATUS status; int ret = -1; + int parent_stat_errno = 0; *state = (struct uc_state) { .mem_ctx = mem_ctx, @@ -1163,122 +1164,118 @@ NTSTATUS unix_convert(TALLOC_CTX *mem_ctx, DBG_DEBUG("Begin: name [%s] dirpath [%s] name [%s]\n", state->smb_fname->base_name, state->dirpath, state->name); - { - int parent_stat_errno = 0; + /* + * stat the name - if it exists then we can add the stream back (if + * there was one) and be done! + */ - /* - * 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; + } - 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; } + } - /* Stat failed - ensure we don't use it. */ - SET_STAT_INVALID(state->smb_fname->st); + /* + * 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. + */ - /* - * 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 ((state->case_sensitive || !(state->conn->fs_capabilities & + FILE_CASE_SENSITIVE_SEARCH)) && + !mangle_is_mangled(state->smb_fname->base_name, state->conn->params)) { - 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; - } + status = check_for_dot_component(state->smb_fname); + 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. + * The stat failed. Could be ok as it could be + * a new file. */ - 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; - } - + if (errno == ENOTDIR || errno == ELOOP) { + status = NT_STATUS_OBJECT_PATH_NOT_FOUND; + goto fail; + } else if (errno == ENOENT) { /* - * The stat failed. Could be ok as it could be - * a new file. - */ - - if (errno == ENOTDIR || errno == ELOOP) { + * 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; - } 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; - } - - /* - * Missing last component is ok - new file. - * Also deal with permission denied elsewhere. - * Just drop out to done. - */ - goto done; } + + /* + * Missing last component is ok - new file. + * Also deal with permission denied elsewhere. + * Just drop out to done. + */ + goto done; } } -- 2.30.2 From d4dcb65963cf38f3ce2a0cd2efd21c896d298020 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 3 Dec 2021 13:03:47 -0800 Subject: [PATCH 31/54] s3: smbd: In unix_convert_step() remove all use of 'state->name_was_wildcard' We know it is never true. Signed-off-by: Jeremy Allison --- source3/smbd/filename.c | 23 ++--------------------- 1 file changed, 2 insertions(+), 21 deletions(-) diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c index cd51602a55c..a392d860356 100644 --- a/source3/smbd/filename.c +++ b/source3/smbd/filename.c @@ -855,25 +855,6 @@ static NTSTATUS unix_convert_step(struct uc_state *state) return NT_STATUS_OBJECT_PATH_NOT_FOUND; } - /* The name cannot have a wildcard if it's not - the last component. */ - - if (!state->posix_pathnames) { - state->name_has_wildcard = ms_has_wild(state->name); - } - - /* Wildcards never valid within a pathname. */ - if (state->name_has_wildcard && state->end != NULL) { - return NT_STATUS_OBJECT_NAME_INVALID; - } - - /* Skip the stat call if it's a wildcard end. */ - if (state->name_has_wildcard) { - DBG_DEBUG("Wildcard [%s]\n", state->name); - state->done = true; - return NT_STATUS_OK; - } - status = unix_convert_step_stat(state); if (!NT_STATUS_IS_OK(status)) { return status; @@ -906,9 +887,9 @@ static NTSTATUS unix_convert_step(struct uc_state *state) /* * Cache the dirpath thus far. Don't cache a name with mangled - * or wildcard components as this can change the size. + * components as this can change the size. */ - if(!state->component_was_mangled && !state->name_has_wildcard) { + if(!state->component_was_mangled) { stat_cache_add(state->orig_path, state->dirpath, state->smb_fname->twrp, -- 2.30.2 From 6c41d04dd05bb1fe74eae203ce6e86b8c35b1c74 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 3 Dec 2021 13:05:55 -0800 Subject: [PATCH 32/54] s3: smbd: In unix_convert_step_stat() remove use of state->name_was_wildcard. It can never be true. Signed-off-by: Jeremy Allison --- source3/smbd/filename.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c index a392d860356..efb9ef07fb7 100644 --- a/source3/smbd/filename.c +++ b/source3/smbd/filename.c @@ -688,8 +688,8 @@ static NTSTATUS unix_convert_step_stat(struct uc_state *state) if (state->posix_pathnames) { /* * For posix_pathnames, we're done. - * Don't blunder into the name_has_wildcard OR - * get_real_filename() codepaths as they may + * 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. @@ -738,10 +738,6 @@ static NTSTATUS unix_convert_step_stat(struct uc_state *state) * Try to find this part of the path in the directory. */ - if (state->name_has_wildcard) { - return unix_convert_step_search_fail(state); - } - dname = (struct smb_filename) { .base_name = state->dirpath, .twrp = state->smb_fname->twrp, -- 2.30.2 From 2427f690f5c0425228b0b9a893d3fe7742730d37 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 3 Dec 2021 13:06:27 -0800 Subject: [PATCH 33/54] s3: smbd: Remove 'struct uc_state' name_has_wildcard element. It is never set or looked at. Signed-off-by: Jeremy Allison --- source3/smbd/filename.c | 1 - 1 file changed, 1 deletion(-) diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c index efb9ef07fb7..19eea2d6a77 100644 --- a/source3/smbd/filename.c +++ b/source3/smbd/filename.c @@ -494,7 +494,6 @@ struct uc_state { char *dirpath; char *stream; bool component_was_mangled; - bool name_has_wildcard; bool posix_pathnames; bool done; bool case_sensitive; -- 2.30.2 From 22cb3b3e2954fbcb3465a5e7f58ff6f9332edb11 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 19 Nov 2021 12:54:47 -0800 Subject: [PATCH 34/54] s4: torture: Fix raw.search:test_one_file() to use torture_result() instead of printf. I think this test pre-dates torture_result. Signed-off-by: Jeremy Allison --- source4/torture/raw/search.c | 60 ++++++++++++++++++++++++------------ 1 file changed, 41 insertions(+), 19 deletions(-) diff --git a/source4/torture/raw/search.c b/source4/torture/raw/search.c index 14af01bffad..07285893f40 100644 --- a/source4/torture/raw/search.c +++ b/source4/torture/raw/search.c @@ -316,7 +316,11 @@ static bool test_one_file(struct torture_context *tctx, fnum = create_complex_file(cli, tctx, fname); if (fnum == -1) { - printf("ERROR: open of %s failed (%s)\n", fname, smbcli_errstr(cli->tree)); + torture_result(tctx, + TORTURE_FAIL, + __location__"ERROR: open of %s failed (%s)\n", + fname, + smbcli_errstr(cli->tree)); ret = false; goto done; } @@ -343,9 +347,11 @@ static bool test_one_file(struct torture_context *tctx, } if (!NT_STATUS_IS_OK(levels[i].status)) { - printf("search level %s(%d) failed - %s\n", - levels[i].name, (int)levels[i].level, - nt_errstr(levels[i].status)); + torture_result(tctx, + TORTURE_FAIL, + __location__"search level %s(%d) failed - %s\n", + levels[i].name, (int)levels[i].level, + nt_errstr(levels[i].status)); ret = false; continue; } @@ -363,7 +369,9 @@ static bool test_one_file(struct torture_context *tctx, expected_status = STATUS_NO_MORE_FILES; } if (!NT_STATUS_EQUAL(status, expected_status)) { - printf("search level %s(%d) should fail with %s - %s\n", + torture_result(tctx, + TORTURE_FAIL, + __location__"search level %s(%d) should fail with %s - %s\n", levels[i].name, (int)levels[i].level, nt_errstr(expected_status), nt_errstr(status)); @@ -400,8 +408,10 @@ static bool test_one_file(struct torture_context *tctx, s = find(name); \ if (s) { \ if ((s->sname1.field1) != (v.sname2.out.field2)) { \ - printf("(%s) %s/%s [0x%x] != %s/%s [0x%x]\n", \ - __location__, \ + torture_result(tctx,\ + TORTURE_FAIL,\ + "(%s) %s/%s [0x%x] != %s/%s [0x%x]\n", \ + __location__, \ #sname1, #field1, (int)s->sname1.field1, \ #sname2, #field2, (int)v.sname2.out.field2); \ ret = false; \ @@ -412,8 +422,10 @@ static bool test_one_file(struct torture_context *tctx, s = find(name); \ if (s) { \ if (s->sname1.field1 != (~1 & nt_time_to_unix(v.sname2.out.field2))) { \ - printf("(%s) %s/%s [%s] != %s/%s [%s]\n", \ - __location__, \ + torture_result(tctx,\ + TORTURE_FAIL,\ + "(%s) %s/%s [%s] != %s/%s [%s]\n", \ + __location__, \ #sname1, #field1, timestring(tctx, s->sname1.field1), \ #sname2, #field2, nt_time_string(tctx, v.sname2.out.field2)); \ ret = false; \ @@ -424,8 +436,10 @@ static bool test_one_file(struct torture_context *tctx, s = find(name); \ if (s) { \ if (s->sname1.field1 != v.sname2.out.field2) { \ - printf("(%s) %s/%s [%s] != %s/%s [%s]\n", \ - __location__, \ + torture_result(tctx,\ + TORTURE_FAIL,\ + "(%s) %s/%s [%s] != %s/%s [%s]\n", \ + __location__, \ #sname1, #field1, nt_time_string(tctx, s->sname1.field1), \ #sname2, #field2, nt_time_string(tctx, v.sname2.out.field2)); \ ret = false; \ @@ -436,8 +450,10 @@ static bool test_one_file(struct torture_context *tctx, s = find(name); \ if (s) { \ if (!s->sname1.field1 || strcmp(s->sname1.field1, v.sname2.out.field2.s)) { \ - printf("(%s) %s/%s [%s] != %s/%s [%s]\n", \ - __location__, \ + torture_result(tctx,\ + TORTURE_FAIL,\ + "(%s) %s/%s [%s] != %s/%s [%s]\n", \ + __location__, \ #sname1, #field1, s->sname1.field1, \ #sname2, #field2, v.sname2.out.field2.s); \ ret = false; \ @@ -450,8 +466,10 @@ static bool test_one_file(struct torture_context *tctx, if (!s->sname1.field1.s || \ strcmp(s->sname1.field1.s, v.sname2.out.field2.s) || \ wire_bad_flags(&s->sname1.field1, flags, cli->transport)) { \ - printf("(%s) %s/%s [%s] != %s/%s [%s]\n", \ - __location__, \ + torture_result(tctx,\ + TORTURE_FAIL,\ + "(%s) %s/%s [%s] != %s/%s [%s]\n", \ + __location__, \ #sname1, #field1, s->sname1.field1.s, \ #sname2, #field2, v.sname2.out.field2.s); \ ret = false; \ @@ -464,8 +482,10 @@ static bool test_one_file(struct torture_context *tctx, if (!s->sname1.field1.s || \ strcmp(s->sname1.field1.s, fname) || \ wire_bad_flags(&s->sname1.field1, flags, cli->transport)) { \ - printf("(%s) %s/%s [%s] != %s\n", \ - __location__, \ + torture_result(tctx,\ + TORTURE_FAIL,\ + "(%s) %s/%s [%s] != %s\n", \ + __location__, \ #sname1, #field1, s->sname1.field1.s, \ fname); \ ret = false; \ @@ -477,8 +497,10 @@ static bool test_one_file(struct torture_context *tctx, if (s) { \ if (!s->sname1.field1 || \ strcmp(s->sname1.field1, fname)) { \ - printf("(%s) %s/%s [%s] != %s\n", \ - __location__, \ + torture_result(tctx,\ + TORTURE_FAIL,\ + "(%s) %s/%s [%s] != %s\n", \ + __location__, \ #sname1, #field1, s->sname1.field1, \ fname); \ ret = false; \ -- 2.30.2 From 668859169ff8aa9dde8b04b4d431d556ce06191c Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 19 Nov 2021 14:18:47 -0800 Subject: [PATCH 35/54] s4: torture: In raw.search:test_one_file() remove the leading '\\' in the test filenames. We'll soon be using this under SMB1+POSIX and neither Windows or POSIX need a leading '\\' (and SMB1+POSIX sees the '\\' as part of the name). Signed-off-by: Jeremy Allison --- source4/torture/raw/search.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/source4/torture/raw/search.c b/source4/torture/raw/search.c index 07285893f40..9b140928689 100644 --- a/source4/torture/raw/search.c +++ b/source4/torture/raw/search.c @@ -305,8 +305,8 @@ static bool test_one_file(struct torture_context *tctx, { bool ret = true; int fnum; - const char *fname = "\\torture_search.txt"; - const char *fname2 = "\\torture_search-NOTEXIST.txt"; + const char *fname = "torture_search.txt"; + const char *fname2 = "torture_search-NOTEXIST.txt"; NTSTATUS status; int i; union smb_fileinfo all_info, alt_info, name_info, internal_info; @@ -581,20 +581,20 @@ static bool test_one_file(struct torture_context *tctx, short_name, alt_info, alt_name_info, fname, STR_UNICODE); } - CHECK_NAME("STANDARD", standard, name, fname+1, 0); - CHECK_NAME("EA_SIZE", ea_size, name, fname+1, 0); - CHECK_NAME("DIRECTORY_INFO", directory_info, name, fname+1, STR_TERMINATE_ASCII); - CHECK_NAME("FULL_DIRECTORY_INFO", full_directory_info, name, fname+1, STR_TERMINATE_ASCII); + CHECK_NAME("STANDARD", standard, name, fname, 0); + CHECK_NAME("EA_SIZE", ea_size, name, fname, 0); + CHECK_NAME("DIRECTORY_INFO", directory_info, name, fname, STR_TERMINATE_ASCII); + CHECK_NAME("FULL_DIRECTORY_INFO", full_directory_info, name, fname, STR_TERMINATE_ASCII); if (name_info_supported) { - CHECK_NAME("NAME_INFO", name_info, name, fname+1, + CHECK_NAME("NAME_INFO", name_info, name, fname, STR_TERMINATE_ASCII); } - CHECK_NAME("BOTH_DIRECTORY_INFO", both_directory_info, name, fname+1, STR_TERMINATE_ASCII); - CHECK_NAME("ID_FULL_DIRECTORY_INFO", id_full_directory_info, name, fname+1, STR_TERMINATE_ASCII); - CHECK_NAME("ID_BOTH_DIRECTORY_INFO", id_both_directory_info, name, fname+1, STR_TERMINATE_ASCII); - CHECK_UNIX_NAME("UNIX_INFO", unix_info, name, fname+1, STR_TERMINATE_ASCII); + CHECK_NAME("BOTH_DIRECTORY_INFO", both_directory_info, name, fname, STR_TERMINATE_ASCII); + CHECK_NAME("ID_FULL_DIRECTORY_INFO", id_full_directory_info, name, fname, STR_TERMINATE_ASCII); + CHECK_NAME("ID_BOTH_DIRECTORY_INFO", id_both_directory_info, name, fname, STR_TERMINATE_ASCII); + CHECK_UNIX_NAME("UNIX_INFO", unix_info, name, fname, STR_TERMINATE_ASCII); if (internal_info_supported) { CHECK_VAL("ID_FULL_DIRECTORY_INFO", id_full_directory_info, -- 2.30.2 From 7437e02ed027879277dc972a694bf421b349d655 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 18 Nov 2021 11:48:42 -0800 Subject: [PATCH 36/54] s3: smbd: Tighten up info level checks for SMB1+POSIX to make sure POSIX was negotiated first. Add knownfail file knownfail.d/posix_infolevel_fails for tests that don't currently negotiate SMB1+POSIX before using SMB1+POSIX calls. These are: samba3.smbtorture_s3.plain.POSIX-BLOCKING-LOCK.smbtorture\(nt4_dc_smb1\) samba3.blackbox.acl_xattr.NT1.nt_affects_posix.* samba3.blackbox.acl_xattr.NT1.nt_affects_chown.* samba3.blackbox.acl_xattr.NT1.nt_affects_chgrp.* samba3.blackbox.inherit_owner.*.NT1.*verify.*unix\ owner.* samba3.unix.info2.info2\(nt4_dc_smb1\) samba3.unix.info2.info2\(ad_dc_smb1\) samba3.raw.search.one\ file\ search.* Signed-off-by: Jeremy Allison --- selftest/knownfail.d/posix_infolevel_fails | 8 +++ source3/smbd/trans2.c | 60 +++++++++++++++++++--- 2 files changed, 62 insertions(+), 6 deletions(-) create mode 100644 selftest/knownfail.d/posix_infolevel_fails diff --git a/selftest/knownfail.d/posix_infolevel_fails b/selftest/knownfail.d/posix_infolevel_fails new file mode 100644 index 00000000000..78a6781684c --- /dev/null +++ b/selftest/knownfail.d/posix_infolevel_fails @@ -0,0 +1,8 @@ +^samba3.smbtorture_s3.plain.POSIX-BLOCKING-LOCK.smbtorture\(nt4_dc_smb1\) +^samba3.blackbox.acl_xattr.NT1.nt_affects_posix.* +^samba3.blackbox.acl_xattr.NT1.nt_affects_chown.* +^samba3.blackbox.acl_xattr.NT1.nt_affects_chgrp.* +^samba3.blackbox.inherit_owner.*.NT1.*verify.*unix\ owner.* +^samba3.unix.info2.info2\(nt4_dc_smb1\) +^samba3.unix.info2.info2\(ad_dc_smb1\) +^samba3.raw.search.one\ file\ search.* diff --git a/source3/smbd/trans2.c b/source3/smbd/trans2.c index 7e148baac55..eb6a66d553d 100644 --- a/source3/smbd/trans2.c +++ b/source3/smbd/trans2.c @@ -2717,6 +2717,10 @@ close_if_end = %d requires_resume_key = %d backup_priv = %d level = 0x%x, max_da reply_nterror(req, NT_STATUS_INVALID_LEVEL); goto out; } + if (!req->posix_pathnames) { + reply_nterror(req, NT_STATUS_INVALID_LEVEL); + goto out; + } break; default: reply_nterror(req, NT_STATUS_INVALID_LEVEL); @@ -3183,6 +3187,10 @@ resume_key = %d resume name = %s continue=%d level = %d\n", reply_nterror(req, NT_STATUS_INVALID_LEVEL); return; } + if (!req->posix_pathnames) { + reply_nterror(req, NT_STATUS_INVALID_LEVEL); + return; + } break; default: reply_nterror(req, NT_STATUS_INVALID_LEVEL); @@ -5147,8 +5155,13 @@ NTSTATUS smbd_do_qfilepathinfo(connection_struct *conn, uint32_t access_mask = 0; size_t len = 0; - if (INFO_LEVEL_IS_UNIX(info_level) && !lp_unix_extensions()) { - return NT_STATUS_INVALID_LEVEL; + if (INFO_LEVEL_IS_UNIX(info_level)) { + if (!lp_unix_extensions()) { + return NT_STATUS_INVALID_LEVEL; + } + if (!req->posix_pathnames) { + return NT_STATUS_INVALID_LEVEL; + } } DEBUG(5,("smbd_do_qfilepathinfo: %s (%s) level=%d max_data=%u\n", @@ -5961,9 +5974,15 @@ static void call_trans2qfilepathinfo(connection_struct *conn, DEBUG(3,("call_trans2qfilepathinfo: TRANSACT2_QFILEINFO: level = %d\n", info_level)); - if (INFO_LEVEL_IS_UNIX(info_level) && !lp_unix_extensions()) { - reply_nterror(req, NT_STATUS_INVALID_LEVEL); - return; + if (INFO_LEVEL_IS_UNIX(info_level)) { + if (!lp_unix_extensions()) { + reply_nterror(req, NT_STATUS_INVALID_LEVEL); + return; + } + if (!req->posix_pathnames) { + reply_nterror(req, NT_STATUS_INVALID_LEVEL); + return; + } } /* Initial check for valid fsp ptr. */ @@ -6056,6 +6075,10 @@ static void call_trans2qfilepathinfo(connection_struct *conn, reply_nterror(req, NT_STATUS_INVALID_LEVEL); return; } + if (!req->posix_pathnames) { + reply_nterror(req, NT_STATUS_INVALID_LEVEL); + return; + } } if (req->posix_pathnames) { @@ -9064,7 +9087,9 @@ NTSTATUS smbd_do_setfilepathinfo(connection_struct *conn, if (!lp_unix_extensions()) { return NT_STATUS_INVALID_LEVEL; } - + if (!req->posix_pathnames) { + return NT_STATUS_INVALID_LEVEL; + } status = smbd_do_posix_setfilepathinfo(conn, req, req, @@ -9285,6 +9310,17 @@ static void call_trans2setfilepathinfo(connection_struct *conn, } info_level = SVAL(params,2); + if (INFO_LEVEL_IS_UNIX(info_level)) { + if (!lp_unix_extensions()) { + reply_nterror(req, NT_STATUS_INVALID_LEVEL); + return; + } + if (!req->posix_pathnames) { + reply_nterror(req, NT_STATUS_INVALID_LEVEL); + return; + } + } + smb_fname = fsp->fsp_name; if (fsp_get_pathref_fd(fsp) == -1) { @@ -9363,6 +9399,18 @@ static void call_trans2setfilepathinfo(connection_struct *conn, } info_level = SVAL(params,0); + + if (INFO_LEVEL_IS_UNIX(info_level)) { + if (!lp_unix_extensions()) { + reply_nterror(req, NT_STATUS_INVALID_LEVEL); + return; + } + if (!req->posix_pathnames) { + reply_nterror(req, NT_STATUS_INVALID_LEVEL); + return; + } + } + if (req->posix_pathnames) { srvstr_get_path_posix(req, params, -- 2.30.2 From 6fa5c20bac2d33e9fdfcc0859951280d4cf42c40 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Sat, 20 Nov 2021 20:17:11 -0800 Subject: [PATCH 37/54] s3: smbclient: Give a message if we try and use any POSIX command without negotiating POSIX first. Ensure we only use a POSIX command if POSIX is set up. Issue the message: Command "posix" must be issued before the "XXXX" command can be used. After the parameter parsing has been done. Signed-off-by: Jeremy Allison --- source3/client/client.c | 79 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 79 insertions(+) diff --git a/source3/client/client.c b/source3/client/client.c index a45215a7795..7ea9f4f96aa 100644 --- a/source3/client/client.c +++ b/source3/client/client.c @@ -2815,6 +2815,11 @@ static int cmd_posix_open(void) d_printf("posix_open 0\n"); return 1; } + if (CLI_DIRSEP_CHAR != '/') { + d_printf("Command \"posix\" must be issued before " + "the \"posix_open\" command can be used.\n"); + return 1; + } mode = (mode_t)strtol(buf, (char **)NULL, 8); status = cli_resolve_path(ctx, "", @@ -2876,6 +2881,11 @@ static int cmd_posix_mkdir(void) d_printf("posix_mkdir 0\n"); return 1; } + if (CLI_DIRSEP_CHAR != '/') { + d_printf("Command \"posix\" must be issued before " + "the \"posix_mkdir\" command can be used.\n"); + return 1; + } mode = (mode_t)strtol(buf, (char **)NULL, 8); status = cli_resolve_path(ctx, "", @@ -2910,6 +2920,11 @@ static int cmd_posix_unlink(void) d_printf("posix_unlink \n"); return 1; } + if (CLI_DIRSEP_CHAR != '/') { + d_printf("Command \"posix\" must be issued before " + "the \"posix_unlink\" command can be used.\n"); + return 1; + } mask = talloc_asprintf(ctx, "%s%s", client_get_cur_dir(), @@ -2955,6 +2970,11 @@ static int cmd_posix_rmdir(void) d_printf("posix_rmdir \n"); return 1; } + if (CLI_DIRSEP_CHAR != '/') { + d_printf("Command \"posix\" must be issued before " + "the \"posix_rmdir\" command can be used.\n"); + return 1; + } mask = talloc_asprintf(ctx, "%s%s", client_get_cur_dir(), @@ -3154,6 +3174,12 @@ static int cmd_lock(void) return 1; } + if (CLI_DIRSEP_CHAR != '/') { + d_printf("Command \"posix\" must be issued before " + "the \"lock\" command can be used.\n"); + return 1; + } + len = (uint64_t)strtol(buf, (char **)NULL, 16); status = cli_posix_lock(cli, fnum, start, len, true, lock_type); @@ -3190,6 +3216,12 @@ static int cmd_unlock(void) return 1; } + if (CLI_DIRSEP_CHAR != '/') { + d_printf("Command \"posix\" must be issued before " + "the \"unlock\" command can be used.\n"); + return 1; + } + len = (uint64_t)strtol(buf, (char **)NULL, 16); status = cli_posix_unlock(cli, fnum, start, len); @@ -3213,6 +3245,12 @@ static int cmd_posix_whoami(void) bool guest = false; uint32_t i; + if (CLI_DIRSEP_CHAR != '/') { + d_printf("Command \"posix\" must be issued before " + "the \"posix_whoami\" command can be used.\n"); + return 1; + } + status = cli_posix_whoami(cli, ctx, &uid, @@ -3350,6 +3388,12 @@ static int cmd_link(void) return 1; } + if (CLI_DIRSEP_CHAR != '/') { + d_printf("Command \"posix\" must be issued before " + "the \"link\" command can be used.\n"); + return 1; + } + status = cli_posix_hardlink(targetcli, targetname, newname); if (!NT_STATUS_IS_OK(status)) { d_printf("%s linking files (%s -> %s)\n", @@ -3403,6 +3447,12 @@ static int cmd_readlink(void) return 1; } + if (CLI_DIRSEP_CHAR != '/') { + d_printf("Command \"posix\" must be issued before " + "the \"readlink\" command can be used.\n"); + return 1; + } + status = cli_posix_readlink(targetcli, name, talloc_tos(), &linkname); if (!NT_STATUS_IS_OK(status)) { d_printf("%s readlink on file %s\n", @@ -3442,6 +3492,11 @@ static int cmd_symlink(void) link_target = buf; if (SERVER_HAS_UNIX_CIFS(cli)) { + if (CLI_DIRSEP_CHAR != '/') { + d_printf("Command \"posix\" must be issued before " + "the \"symlink\" command can be used.\n"); + return 1; + } newname = talloc_asprintf(ctx, "%s%s", client_get_cur_dir(), buf2); if (!newname) { @@ -3525,6 +3580,12 @@ static int cmd_chmod(void) return 1; } + if (CLI_DIRSEP_CHAR != '/') { + d_printf("Command \"posix\" must be issued before " + "the \"chmod\" command can be used.\n"); + return 1; + } + status = cli_posix_chmod(targetcli, targetname, mode); if (!NT_STATUS_IS_OK(status)) { d_printf("%s chmod file %s 0%o\n", @@ -3689,6 +3750,12 @@ static int cmd_getfacl(void) return 1; } + if (CLI_DIRSEP_CHAR != '/') { + d_printf("Command \"posix\" must be issued before " + "the \"getfacl\" command can be used.\n"); + return 1; + } + status = cli_unix_extensions_version(targetcli, &major, &minor, &caplow, &caphigh); if (!NT_STATUS_IS_OK(status)) { @@ -3988,6 +4055,12 @@ static int cmd_stat(void) return 1; } + if (CLI_DIRSEP_CHAR != '/') { + d_printf("Command \"posix\" must be issued before " + "the \"stat\" command can be used.\n"); + return 1; + } + status = cli_posix_stat(targetcli, targetname, &sbuf); if (!NT_STATUS_IS_OK(status)) { d_printf("%s stat file %s\n", @@ -4102,6 +4175,12 @@ static int cmd_chown(void) return 1; } + if (CLI_DIRSEP_CHAR != '/') { + d_printf("Command \"posix\" must be issued before " + "the \"chown\" command can be used.\n"); + return 1; + } + status = cli_posix_chown(targetcli, targetname, uid, gid); if (!NT_STATUS_IS_OK(status)) { d_printf("%s chown file %s uid=%d, gid=%d\n", -- 2.30.2 From 5c1c8ea8e40a33b22a582449cfae3a4af79579d0 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 19 Nov 2021 14:44:05 -0800 Subject: [PATCH 38/54] s4: torture: In raw.search:test_one_file() add a second connection. Change from torture_suite_add_1smb_test() to torture_suite_add_2smb_test(). Not yet used. We will need this to do SMB1+POSIX search calls on a connection on which we have negotiated SMB1+POSIX. Signed-off-by: Jeremy Allison --- source4/torture/raw/search.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/source4/torture/raw/search.c b/source4/torture/raw/search.c index 9b140928689..f6ad569dd4b 100644 --- a/source4/torture/raw/search.c +++ b/source4/torture/raw/search.c @@ -300,8 +300,9 @@ static union smb_search_data *find(const char *name) /* basic testing of all RAW_SEARCH_* calls using a single file */ -static bool test_one_file(struct torture_context *tctx, - struct smbcli_state *cli) +static bool test_one_file(struct torture_context *tctx, + struct smbcli_state *cli, + struct smbcli_state *cli_unix) { bool ret = true; int fnum; @@ -1571,7 +1572,7 @@ struct torture_suite *torture_raw_search(TALLOC_CTX *mem_ctx) { struct torture_suite *suite = torture_suite_create(mem_ctx, "search"); - torture_suite_add_1smb_test(suite, "one file search", test_one_file); + torture_suite_add_2smb_test(suite, "one file search", test_one_file); torture_suite_add_1smb_test(suite, "many files", test_many_files); torture_suite_add_1smb_test(suite, "sorted", test_sorted); torture_suite_add_1smb_test(suite, "modify search", test_modify_search); -- 2.30.2 From 21d509300010481a70c89d595461fc3520398c15 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 19 Nov 2021 14:48:20 -0800 Subject: [PATCH 39/54] s4: torture: raw.search: Add setup_smb1_posix(). Call it on the second connection in test_one_file(). Not yet used. Signed-off-by: Jeremy Allison --- source4/torture/raw/search.c | 59 ++++++++++++++++++++++++++++++++++++ 1 file changed, 59 insertions(+) diff --git a/source4/torture/raw/search.c b/source4/torture/raw/search.c index f6ad569dd4b..3f7de80ad0f 100644 --- a/source4/torture/raw/search.c +++ b/source4/torture/raw/search.c @@ -297,6 +297,55 @@ static union smb_search_data *find(const char *name) return NULL; } +/* + * Negotiate SMB1+POSIX. + */ + +static NTSTATUS setup_smb1_posix(struct torture_context *tctx, + struct smbcli_state *cli_unix) +{ + struct smb_trans2 tp; + uint16_t setup; + uint8_t data[12]; + uint8_t params[4]; + uint32_t cap = cli_unix->transport->negotiate.capabilities; + + if ((cap & CAP_UNIX) == 0) { + /* + * Server doesn't support SMB1+POSIX. + * The caller will skip the UNIX info + * level anyway. + */ + torture_comment(tctx, + "Server doesn't support SMB1+POSIX\n"); + return NT_STATUS_OK; + } + + /* Setup POSIX on this connection. */ + SSVAL(data, 0, CIFS_UNIX_MAJOR_VERSION); + SSVAL(data, 2, CIFS_UNIX_MINOR_VERSION); + SBVAL(data,4,((uint64_t)( + CIFS_UNIX_POSIX_ACLS_CAP| + CIFS_UNIX_POSIX_PATHNAMES_CAP| + CIFS_UNIX_FCNTL_LOCKS_CAP| + CIFS_UNIX_EXTATTR_CAP| + CIFS_UNIX_POSIX_PATH_OPERATIONS_CAP))); + setup = TRANSACT2_SETFSINFO; + tp.in.max_setup = 0; + tp.in.flags = 0; + tp.in.timeout = 0; + tp.in.setup_count = 1; + tp.in.max_param = 0; + tp.in.max_data = 0; + tp.in.setup = &setup; + tp.in.trans_name = NULL; + SSVAL(params, 0, 0); + SSVAL(params, 2, SMB_SET_CIFS_UNIX_INFO); + tp.in.params = data_blob_talloc(tctx, params, 4); + tp.in.data = data_blob_talloc(tctx, data, 12); + return smb_raw_trans2(cli_unix->tree, tctx, &tp); +} + /* basic testing of all RAW_SEARCH_* calls using a single file */ @@ -315,6 +364,16 @@ static bool test_one_file(struct torture_context *tctx, internal_info_supported; union smb_search_data *s; + status = setup_smb1_posix(tctx, cli_unix); + if (!NT_STATUS_IS_OK(status)) { + torture_result(tctx, + TORTURE_FAIL, + __location__"setup_smb1_posix() failed (%s)\n", + nt_errstr(status)); + ret = false; + goto done; + } + fnum = create_complex_file(cli, tctx, fname); if (fnum == -1) { torture_result(tctx, -- 2.30.2 From 576db1217be73a658dc62db0af686ba9741b26b8 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 19 Nov 2021 14:51:39 -0800 Subject: [PATCH 40/54] s4: torture: Fix raw.search:test_one_file() by using the SMB1+POSIX connection for POSIX info levels. Remove the following entry in knownfail.d/posix_infolevel_fails. ^samba3.raw.search.one\ file\ search.* from knownfail.d/posix_infolevel_fails Signed-off-by: Jeremy Allison --- selftest/knownfail.d/posix_infolevel_fails | 1 - source4/torture/raw/search.c | 13 +++++++++++-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/selftest/knownfail.d/posix_infolevel_fails b/selftest/knownfail.d/posix_infolevel_fails index 78a6781684c..4ce4a9cec91 100644 --- a/selftest/knownfail.d/posix_infolevel_fails +++ b/selftest/knownfail.d/posix_infolevel_fails @@ -5,4 +5,3 @@ ^samba3.blackbox.inherit_owner.*.NT1.*verify.*unix\ owner.* ^samba3.unix.info2.info2\(nt4_dc_smb1\) ^samba3.unix.info2.info2\(ad_dc_smb1\) -^samba3.raw.search.one\ file\ search.* diff --git a/source4/torture/raw/search.c b/source4/torture/raw/search.c index 3f7de80ad0f..575bbd03fb7 100644 --- a/source4/torture/raw/search.c +++ b/source4/torture/raw/search.c @@ -389,10 +389,19 @@ static bool test_one_file(struct torture_context *tctx, for (i=0;itransport->negotiate.capabilities; + struct smbcli_state *cli_search = cli; torture_comment(tctx, "Testing %s\n", levels[i].name); - levels[i].status = torture_single_search(cli, tctx, fname, + if (levels[i].data_level == RAW_SEARCH_DATA_UNIX_INFO) { + /* + * For an SMB1+POSIX info level, use the cli_unix + * connection. + */ + cli_search = cli_unix; + } + + levels[i].status = torture_single_search(cli_search, tctx, fname, levels[i].level, levels[i].data_level, 0, @@ -416,7 +425,7 @@ static bool test_one_file(struct torture_context *tctx, continue; } - status = torture_single_search(cli, tctx, fname2, + status = torture_single_search(cli_search, tctx, fname2, levels[i].level, levels[i].data_level, 0, -- 2.30.2 From bc95ab2c68a86dafe854a2ce4414449872f0b464 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 19 Nov 2021 12:15:06 -0800 Subject: [PATCH 41/54] s4: torture: Fix unix.info2 test to actually negotiate SMB1+POSIX before using POSIX calls. Cope with the minor difference in wildcard search return when we're actually using SMB1+POSIX on the server (SMB1+POSIX treats all directory search paths as wildcards). Remove the following entries in knownfail.d/posix_infolevel_fails. samba3.unix.info2.info2\(nt4_dc_smb1\) samba3.unix.info2.info2\(ad_dc_smb1\) Signed-off-by: Jeremy Allison --- selftest/knownfail.d/posix_infolevel_fails | 2 -- source4/torture/unix/unix_info2.c | 42 ++++++++++++++++++++-- 2 files changed, 40 insertions(+), 4 deletions(-) diff --git a/selftest/knownfail.d/posix_infolevel_fails b/selftest/knownfail.d/posix_infolevel_fails index 4ce4a9cec91..3bfff110771 100644 --- a/selftest/knownfail.d/posix_infolevel_fails +++ b/selftest/knownfail.d/posix_infolevel_fails @@ -3,5 +3,3 @@ ^samba3.blackbox.acl_xattr.NT1.nt_affects_chown.* ^samba3.blackbox.acl_xattr.NT1.nt_affects_chgrp.* ^samba3.blackbox.inherit_owner.*.NT1.*verify.*unix\ owner.* -^samba3.unix.info2.info2\(nt4_dc_smb1\) -^samba3.unix.info2.info2\(ad_dc_smb1\) diff --git a/source4/torture/unix/unix_info2.c b/source4/torture/unix/unix_info2.c index 6b275435551..2098b225e7f 100644 --- a/source4/torture/unix/unix_info2.c +++ b/source4/torture/unix/unix_info2.c @@ -19,6 +19,7 @@ #include "includes.h" #include "libcli/libcli.h" +#include "libcli/raw/raw_proto.h" #include "torture/util.h" #include "torture/unix/proto.h" #include "lib/cmdline/cmdline.h" @@ -53,6 +54,10 @@ static struct smbcli_state *connect_to_server(struct torture_context *tctx) const char *share = torture_setting_string(tctx, "share", NULL); struct smbcli_options options; struct smbcli_session_options session_options; + struct smb_trans2 tp; + uint16_t setup; + uint8_t data[12]; + uint8_t params[4]; lpcfg_smbcli_options(tctx->lp_ctx, &options); lpcfg_smbcli_session_options(tctx->lp_ctx, &session_options); @@ -72,6 +77,33 @@ static struct smbcli_state *connect_to_server(struct torture_context *tctx) return NULL; } + /* Setup POSIX on the server. */ + SSVAL(data, 0, CIFS_UNIX_MAJOR_VERSION); + SSVAL(data, 2, CIFS_UNIX_MINOR_VERSION); + SBVAL(data,4,((uint64_t)( + CIFS_UNIX_POSIX_ACLS_CAP| + CIFS_UNIX_POSIX_PATHNAMES_CAP| + CIFS_UNIX_FCNTL_LOCKS_CAP| + CIFS_UNIX_EXTATTR_CAP| + CIFS_UNIX_POSIX_PATH_OPERATIONS_CAP))); + setup = TRANSACT2_SETFSINFO; + tp.in.max_setup = 0; + tp.in.flags = 0; + tp.in.timeout = 0; + tp.in.setup_count = 1; + tp.in.max_param = 0; + tp.in.max_data = 0; + tp.in.setup = &setup; + tp.in.trans_name = NULL; + SSVAL(params, 0, 0); + SSVAL(params, 2, SMB_SET_CIFS_UNIX_INFO); + tp.in.params = data_blob_talloc(tctx, params, 4); + tp.in.data = data_blob_talloc(tctx, data, 12); + + status = smb_raw_trans2(cli->tree, tctx, &tp); + torture_assert_ntstatus_equal(tctx, status, NT_STATUS_OK, + "doing SMB_SET_CIFS_UNIX_INFO"); + return cli; } @@ -245,8 +277,14 @@ static bool find_single_info2(void *mem_ctx, torture_assert_int_equal(torture, search.t2ffirst.out.count, 1, "expected exactly one result"); - torture_assert_int_equal(torture, search.t2ffirst.out.end_of_search, 1, - "expected end_of_search to be true"); + /* + * In smbd directory listings using POSIX extensions + * always treat the search pathname as a wildcard, + * so don't expect end_of_search to be set here. Wildcard + * searches always need a findnext to end the search. + */ + torture_assert_int_equal(torture, search.t2ffirst.out.end_of_search, 0, + "expected end_of_search to be false"); return check_unix_info2(torture, info2); } -- 2.30.2 From 51b285decb7ab2cb8503fb4cc75623958ba16a66 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 19 Nov 2021 12:12:36 -0800 Subject: [PATCH 42/54] s3: tests: Fix the samba3.blackbox.inherit_owner test to actually negotiate SMB1+POSIX before using POSIX calls. Remove the following entry in knownfail.d/posix_infolevel_fails. samba3.blackbox.inherit_owner.*.NT1.*verify.*unix\ owner.* Signed-off-by: Jeremy Allison --- selftest/knownfail.d/posix_infolevel_fails | 1 - source3/script/tests/test_inherit_owner.sh | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/selftest/knownfail.d/posix_infolevel_fails b/selftest/knownfail.d/posix_infolevel_fails index 3bfff110771..a865a2055b2 100644 --- a/selftest/knownfail.d/posix_infolevel_fails +++ b/selftest/knownfail.d/posix_infolevel_fails @@ -2,4 +2,3 @@ ^samba3.blackbox.acl_xattr.NT1.nt_affects_posix.* ^samba3.blackbox.acl_xattr.NT1.nt_affects_chown.* ^samba3.blackbox.acl_xattr.NT1.nt_affects_chgrp.* -^samba3.blackbox.inherit_owner.*.NT1.*verify.*unix\ owner.* diff --git a/source3/script/tests/test_inherit_owner.sh b/source3/script/tests/test_inherit_owner.sh index 7e1333787aa..9783235883c 100755 --- a/source3/script/tests/test_inherit_owner.sh +++ b/source3/script/tests/test_inherit_owner.sh @@ -79,7 +79,7 @@ unix_owner_id_is() { local fname=$2 local expected_id=$3 local actual_id - actual_id=$($SMBCLIENT //$SERVER/$share -U $USERNAME%$PASSWORD -c "getfacl $fname" 2>/dev/null | sed -rn 's/^# owner: (.*)/\1/p') + actual_id=$($SMBCLIENT //$SERVER/$share -U $USERNAME%$PASSWORD -c "posix; getfacl $fname" 2>/dev/null | sed -rn 's/^# owner: (.*)/\1/p') if ! test "x$actual_id" = "x$expected_id" ; then echo "Actual uid of $share/$fname is [$actual_id] expected [$expected_id]" exit 1 -- 2.30.2 From e9a0d55b5ae62180382eb81972a25d61df699eab Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 19 Nov 2021 00:05:35 -0800 Subject: [PATCH 43/54] s3: tests: Fix the samba3.blackbox.acl_xattr test to actually negotiate SMB1+POSIX before using POSIX calls. Remove the following entries in knownfail.d/posix_infolevel_fails. samba3.blackbox.acl_xattr.NT1.nt_affects_posix.* samba3.blackbox.acl_xattr.NT1.nt_affects_chown.* samba3.blackbox.acl_xattr.NT1.nt_affects_chgrp.* Signed-off-by: Jeremy Allison --- selftest/knownfail.d/posix_infolevel_fails | 3 --- source3/script/tests/test_acl_xattr.sh | 12 ++++++------ 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/selftest/knownfail.d/posix_infolevel_fails b/selftest/knownfail.d/posix_infolevel_fails index a865a2055b2..bf8a884cb16 100644 --- a/selftest/knownfail.d/posix_infolevel_fails +++ b/selftest/knownfail.d/posix_infolevel_fails @@ -1,4 +1 @@ ^samba3.smbtorture_s3.plain.POSIX-BLOCKING-LOCK.smbtorture\(nt4_dc_smb1\) -^samba3.blackbox.acl_xattr.NT1.nt_affects_posix.* -^samba3.blackbox.acl_xattr.NT1.nt_affects_chown.* -^samba3.blackbox.acl_xattr.NT1.nt_affects_chgrp.* diff --git a/source3/script/tests/test_acl_xattr.sh b/source3/script/tests/test_acl_xattr.sh index f134ff79c91..8abd7476244 100755 --- a/source3/script/tests/test_acl_xattr.sh +++ b/source3/script/tests/test_acl_xattr.sh @@ -55,9 +55,9 @@ nt_affects_posix() { local b4 local af local fname="$share.$$" - b4=$($SMBCLIENT //$SERVER/$share -U $USERNAME%$PASSWORD -c "getfacl $fname" 2>/dev/null) || exit 1 + b4=$($SMBCLIENT //$SERVER/$share -U $USERNAME%$PASSWORD -c "posix; getfacl $fname" 2>/dev/null) || exit 1 $SMBCACLS //$SERVER/$share $fname -U $USERNAME%$PASSWORD -a "ACL:$SERVER\force_user:ALLOWED/0x0/READ" 2>/dev/null || exit 1 - af=$($SMBCLIENT //$SERVER/$share -U $USERNAME%$PASSWORD -c "getfacl $fname" 2>/dev/null) || exit 1 + af=$($SMBCLIENT //$SERVER/$share -U $USERNAME%$PASSWORD -c "posix; getfacl $fname" 2>/dev/null) || exit 1 echo "before: $b4" echo "after: $af" echo "${b4}" | grep -q "^# owner:" || exit 1 @@ -90,12 +90,12 @@ nt_affects_chown() { #basic sanity... test "$b4_expected != $af_expected" || exit 1 - b4_actual=$($SMBCLIENT //$SERVER/$share -U $USERNAME%$PASSWORD -c "getfacl $fname" 2>/dev/null) || exit 1 + b4_actual=$($SMBCLIENT //$SERVER/$share -U $USERNAME%$PASSWORD -c "posix; getfacl $fname" 2>/dev/null) || exit 1 echo "${b4_actual}" | grep -q "^# owner:" || exit 1 b4_actual=$(echo "$b4_actual" | sed -rn 's/^# owner: (.*)/\1/p') $SMBCACLS //$SERVER/$share $fname -U $USERNAME%$PASSWORD -a "ACL:$SERVER\force_user:ALLOWED/0x0/FULL" || exit 1 $SMBCACLS //$SERVER/$share $fname -U force_user%$PASSWORD -C force_user 2>/dev/null || exit 1 - af_actual=$($SMBCLIENT //$SERVER/$share -U $USERNAME%$PASSWORD -c "getfacl $fname" 2>/dev/null) || exit 1 + af_actual=$($SMBCLIENT //$SERVER/$share -U $USERNAME%$PASSWORD -c "posix; getfacl $fname" 2>/dev/null) || exit 1 echo "${af_actual}" | grep -q "^# owner:" || exit 1 af_actual=$(echo "$af_actual" | sed -rn 's/^# owner: (.*)/\1/p') echo "before: $b4_actual" @@ -124,11 +124,11 @@ nt_affects_chgrp() { #basic sanity... test "$b4_expected" != "$af_expected" || exit 1 - b4_actual=$($SMBCLIENT //$SERVER/$share -U $USERNAME%$PASSWORD -c "getfacl $fname" 2>/dev/null) || exit 1 + b4_actual=$($SMBCLIENT //$SERVER/$share -U $USERNAME%$PASSWORD -c "posix; getfacl $fname" 2>/dev/null) || exit 1 echo "${b4_actual}" | grep -q "^# group:" || exit 1 b4_actual=$(echo "$b4_actual" | sed -rn 's/^# group: (.*)/\1/p') $SMBCACLS //$SERVER/$share $fname -U $USERNAME%$PASSWORD -G domadmins 2>/dev/null || exit 1 - af_actual=$($SMBCLIENT //$SERVER/$share -U $USERNAME%$PASSWORD -c "getfacl $fname" 2>/dev/null) || exit 1 + af_actual=$($SMBCLIENT //$SERVER/$share -U $USERNAME%$PASSWORD -c "posix; getfacl $fname" 2>/dev/null) || exit 1 echo "${af_actual}" | grep -q "^# group:" || exit 1 af_actual=$(echo "$af_actual" | sed -rn 's/^# group: (.*)/\1/p') echo "before: $b4_actual" -- 2.30.2 From 0171e603c9aebbd232a868a4d1f3b2b601b322b1 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 18 Nov 2021 12:16:44 -0800 Subject: [PATCH 44/54] s3: smbtorture3: Fix POSIX-BLOCKING-LOCK to actually negotiate SMB1+POSIX before using POSIX calls. This must be done before doing POSIX calls on a connection. Remove the final entry in knownfail.d/posix_infolevel_fails samba3.smbtorture_s3.plain.POSIX-BLOCKING-LOCK.smbtorture\(nt4_dc_smb1\) And remove the file knownfail.d/posix_infolevel_fails itself. Signed-off-by: Jeremy Allison --- selftest/knownfail.d/posix_infolevel_fails | 1 - source3/torture/torture.c | 5 +++++ 2 files changed, 5 insertions(+), 1 deletion(-) delete mode 100644 selftest/knownfail.d/posix_infolevel_fails diff --git a/selftest/knownfail.d/posix_infolevel_fails b/selftest/knownfail.d/posix_infolevel_fails deleted file mode 100644 index bf8a884cb16..00000000000 --- a/selftest/knownfail.d/posix_infolevel_fails +++ /dev/null @@ -1 +0,0 @@ -^samba3.smbtorture_s3.plain.POSIX-BLOCKING-LOCK.smbtorture\(nt4_dc_smb1\) diff --git a/source3/torture/torture.c b/source3/torture/torture.c index b69125202f2..174b10bd305 100644 --- a/source3/torture/torture.c +++ b/source3/torture/torture.c @@ -8973,6 +8973,11 @@ static bool run_posix_blocking_lock(int dummy) return false; } + status = torture_setup_unix_extensions(cli2); + if (!NT_STATUS_IS_OK(status)) { + return false; + } + cli_setatr(cli1, fname, 0, 0); cli_posix_unlink(cli1, fname); -- 2.30.2 From 2795e67f85196bf9ef5259fc755ed9d904b5e434 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Tue, 7 Dec 2021 12:28:54 -0800 Subject: [PATCH 45/54] CVE-2021-44141: s3: torture: Add samba3.blackbox.test_symlink_traversal.SMB2. Add to knownfail.d/symlink_traversal BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- selftest/knownfail.d/symlink_traversal | 1 + .../tests/test_symlink_traversal_smb2.sh | 263 ++++++++++++++++++ source3/selftest/tests.py | 6 + 3 files changed, 270 insertions(+) create mode 100644 selftest/knownfail.d/symlink_traversal create mode 100755 source3/script/tests/test_symlink_traversal_smb2.sh diff --git a/selftest/knownfail.d/symlink_traversal b/selftest/knownfail.d/symlink_traversal new file mode 100644 index 00000000000..a8ac4bbae1f --- /dev/null +++ b/selftest/knownfail.d/symlink_traversal @@ -0,0 +1 @@ +^samba3.blackbox.test_symlink_traversal.SMB2.symlink_traversal_SMB2\(fileserver\) diff --git a/source3/script/tests/test_symlink_traversal_smb2.sh b/source3/script/tests/test_symlink_traversal_smb2.sh new file mode 100755 index 00000000000..7e1de6dde1a --- /dev/null +++ b/source3/script/tests/test_symlink_traversal_smb2.sh @@ -0,0 +1,263 @@ +#!/bin/sh + +if [ $# -lt 7 ]; then +cat < "$tmpfile" < Date: Tue, 7 Dec 2021 12:32:19 -0800 Subject: [PATCH 46/54] CVE-2021-44141: s3: torture: Add samba3.blackbox.test_symlink_traversal.SMB1. Add to knownfail.d/symlink_traversal. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- selftest/knownfail.d/symlink_traversal | 1 + .../tests/test_symlink_traversal_smb1.sh | 263 ++++++++++++++++++ source3/selftest/tests.py | 4 + 3 files changed, 268 insertions(+) create mode 100755 source3/script/tests/test_symlink_traversal_smb1.sh diff --git a/selftest/knownfail.d/symlink_traversal b/selftest/knownfail.d/symlink_traversal index a8ac4bbae1f..2a51ff3f91d 100644 --- a/selftest/knownfail.d/symlink_traversal +++ b/selftest/knownfail.d/symlink_traversal @@ -1 +1,2 @@ ^samba3.blackbox.test_symlink_traversal.SMB2.symlink_traversal_SMB2\(fileserver\) +^samba3.blackbox.test_symlink_traversal.SMB1.symlink_traversal_SMB1\(fileserver_smb1_done\) diff --git a/source3/script/tests/test_symlink_traversal_smb1.sh b/source3/script/tests/test_symlink_traversal_smb1.sh new file mode 100755 index 00000000000..1deaaccbb54 --- /dev/null +++ b/source3/script/tests/test_symlink_traversal_smb1.sh @@ -0,0 +1,263 @@ +#!/bin/sh + +if [ $# -lt 7 ]; then +cat < "$tmpfile" < Date: Tue, 7 Dec 2021 12:34:38 -0800 Subject: [PATCH 47/54] CVE-2021-44141: s3: torture: Add samba3.blackbox.test_symlink_traversal.SMB1.posix Add to knownfail.d/symlink_traversal. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- selftest/knownfail.d/symlink_traversal | 1 + .../test_symlink_traversal_smb1_posix.sh | 270 ++++++++++++++++++ source3/selftest/tests.py | 5 + 3 files changed, 276 insertions(+) create mode 100755 source3/script/tests/test_symlink_traversal_smb1_posix.sh diff --git a/selftest/knownfail.d/symlink_traversal b/selftest/knownfail.d/symlink_traversal index 2a51ff3f91d..25a4da8f250 100644 --- a/selftest/knownfail.d/symlink_traversal +++ b/selftest/knownfail.d/symlink_traversal @@ -1,2 +1,3 @@ ^samba3.blackbox.test_symlink_traversal.SMB2.symlink_traversal_SMB2\(fileserver\) ^samba3.blackbox.test_symlink_traversal.SMB1.symlink_traversal_SMB1\(fileserver_smb1_done\) +^samba3.blackbox.test_symlink_traversal.SMB1.posix.symlink_traversal_SMB1_posix\(fileserver_smb1_done\) diff --git a/source3/script/tests/test_symlink_traversal_smb1_posix.sh b/source3/script/tests/test_symlink_traversal_smb1_posix.sh new file mode 100755 index 00000000000..6241434dcf6 --- /dev/null +++ b/source3/script/tests/test_symlink_traversal_smb1_posix.sh @@ -0,0 +1,270 @@ +#!/bin/sh + +if [ $# -lt 7 ]; then +cat < "$tmpfile" < Date: Tue, 7 Dec 2021 12:56:51 -0800 Subject: [PATCH 48/54] CVE-2021-44141: s3: torture: In test_smbclient_s3, change the error codes expected for test_widelinks() and test_nosymlinks() from ACCESS_DENIED to NT_STATUS_OBJECT_NAME_NOT_FOUND. For SMB1/2/3 (minus posix) we need to treat bad symlinks as though they don't exist. Add to knwownfail.d/symlink_traversal BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- selftest/knownfail.d/symlink_traversal | 2 ++ selftest/target/Samba3.pm | 2 +- source3/script/tests/test_smbclient_s3.sh | 10 +++++----- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/selftest/knownfail.d/symlink_traversal b/selftest/knownfail.d/symlink_traversal index 25a4da8f250..840ab38b0f9 100644 --- a/selftest/knownfail.d/symlink_traversal +++ b/selftest/knownfail.d/symlink_traversal @@ -1,3 +1,5 @@ ^samba3.blackbox.test_symlink_traversal.SMB2.symlink_traversal_SMB2\(fileserver\) ^samba3.blackbox.test_symlink_traversal.SMB1.symlink_traversal_SMB1\(fileserver_smb1_done\) ^samba3.blackbox.test_symlink_traversal.SMB1.posix.symlink_traversal_SMB1_posix\(fileserver_smb1_done\) +^samba3.blackbox.smbclient_s3.*.Ensure\ widelinks\ are\ restricted\(.*\) +^samba3.blackbox.smbclient_s3.*.follow\ symlinks\ \=\ no\(.*\) diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm index 7385b755273..3282d763bd3 100755 --- a/selftest/target/Samba3.pm +++ b/selftest/target/Samba3.pm @@ -2536,7 +2536,7 @@ sub provision($$) create_file_chmod("$widelinks_target", 0666) or return undef; ## - ## This link should get ACCESS_DENIED + ## This link should get an error ## symlink "$widelinks_target", "$widelinks_shrdir/source"; ## diff --git a/source3/script/tests/test_smbclient_s3.sh b/source3/script/tests/test_smbclient_s3.sh index 89a17656159..e250d4dd106 100755 --- a/source3/script/tests/test_smbclient_s3.sh +++ b/source3/script/tests/test_smbclient_s3.sh @@ -1044,12 +1044,12 @@ EOF return 1 fi -# This should fail with NT_STATUS_ACCESS_DENIED - echo "$out" | grep 'NT_STATUS_ACCESS_DENIED' +# This should fail with NT_STATUS_OBJECT_NAME_NOT_FOUND + echo "$out" | grep 'NT_STATUS_OBJECT_NAME_NOT_FOUND' ret=$? if [ $ret != 0 ] ; then echo "$out" - echo "failed - should get NT_STATUS_ACCESS_DENIED listing \\widelinks_share\\source" + echo "failed - should get NT_STATUS_OBJECT_NAME_NOT_FOUND listing \\widelinks_share\\source" return 1 fi } @@ -1168,11 +1168,11 @@ EOF return 1 fi - echo "$out" | grep 'NT_STATUS_ACCESS_DENIED' + echo "$out" | grep 'NT_STATUS_OBJECT_NAME_NOT_FOUND' ret=$? if [ $ret -ne 0 ] ; then echo "$out" - echo "failed - should get NT_STATUS_ACCESS_DENIED getting \\nosymlinks\\source" + echo "failed - should get NT_STATUS_OBJECT_NAME_NOT_FOUND getting \\nosymlinks\\source" return 1 fi -- 2.30.2 From 3e1c8a45060565f2a16bc363308d3526f59d2841 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Tue, 7 Dec 2021 17:56:35 -0800 Subject: [PATCH 49/54] CVE-2021-44141: s3: torture: Change expected error return for samba3.smbtorture_s3.plain.POSIX.smbtorture. Trying to open a symlink as a terminal component should return NT_STATUS_OBJECT_NAME_NOT_FOUND, not NT_STATUS_OBJECT_PATH_NOT_FOUND. Mark as knownfail.d/simple_posix_open until we fix the server. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- selftest/knownfail.d/simple_posix_open | 1 + source3/torture/torture.c | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) create mode 100644 selftest/knownfail.d/simple_posix_open diff --git a/selftest/knownfail.d/simple_posix_open b/selftest/knownfail.d/simple_posix_open new file mode 100644 index 00000000000..5fcbdbdc2c6 --- /dev/null +++ b/selftest/knownfail.d/simple_posix_open @@ -0,0 +1 @@ +^samba3.smbtorture_s3.plain.POSIX.smbtorture\(.*\) diff --git a/source3/torture/torture.c b/source3/torture/torture.c index 174b10bd305..9d3e43ed73e 100644 --- a/source3/torture/torture.c +++ b/source3/torture/torture.c @@ -8028,9 +8028,9 @@ static bool run_simple_posix_open_test(int dummy) goto out; } else { if (!check_both_error(__LINE__, status, ERRDOS, ERRbadpath, - NT_STATUS_OBJECT_PATH_NOT_FOUND)) { + NT_STATUS_OBJECT_NAME_NOT_FOUND)) { printf("POSIX open of %s should have failed " - "with NT_STATUS_OBJECT_PATH_NOT_FOUND, " + "with NT_STATUS_OBJECT_NAME_NOT_FOUND, " "failed with %s instead.\n", sname, nt_errstr(status)); goto out; -- 2.30.2 From 6a5ed1748eddd92e8f187c33726a5a085c03cda2 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Tue, 7 Dec 2021 11:44:09 -0800 Subject: [PATCH 50/54] CVE-2021-44141: s3: smbd: For SMB1+POSIX clients trying to open a symlink, always return NT_STATUS_OBJECT_NAME_NOT_FOUND. Matches the error return from openat_pathref_fsp(). NT_STATUS_OBJECT_PATH_NOT_FOUND is for a bad component in a path, not a bad terminal symlink. Remove knownfail.d/simple_posix_open, we now pass. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- selftest/knownfail.d/simple_posix_open | 1 - source3/smbd/open.c | 13 ++++++------- 2 files changed, 6 insertions(+), 8 deletions(-) delete mode 100644 selftest/knownfail.d/simple_posix_open diff --git a/selftest/knownfail.d/simple_posix_open b/selftest/knownfail.d/simple_posix_open deleted file mode 100644 index 5fcbdbdc2c6..00000000000 --- a/selftest/knownfail.d/simple_posix_open +++ /dev/null @@ -1 +0,0 @@ -^samba3.smbtorture_s3.plain.POSIX.smbtorture\(.*\) diff --git a/source3/smbd/open.c b/source3/smbd/open.c index 7f1aedbd1fb..753c0f56ada 100644 --- a/source3/smbd/open.c +++ b/source3/smbd/open.c @@ -1448,12 +1448,10 @@ static NTSTATUS open_file(files_struct *fsp, * POSIX client that hit a symlink. We don't want to * return NT_STATUS_STOPPED_ON_SYMLINK to avoid handling * this special error code in all callers, so we map - * this to NT_STATUS_OBJECT_PATH_NOT_FOUND. Historically - * the lower level functions returned status code mapped - * from errno by map_nt_error_from_unix() where ELOOP is - * mapped to NT_STATUS_OBJECT_PATH_NOT_FOUND. + * this to NT_STATUS_OBJECT_NAME_NOT_FOUND to match + * openat_pathref_fsp(). */ - status = NT_STATUS_OBJECT_PATH_NOT_FOUND; + status = NT_STATUS_OBJECT_NAME_NOT_FOUND; } if (!NT_STATUS_IS_OK(status)) { DEBUG(3,("Error opening file %s (%s) (local_flags=%d) " @@ -1536,9 +1534,10 @@ static NTSTATUS open_file(files_struct *fsp, { /* * Don't allow stat opens on symlinks directly unless - * it's a POSIX open. + * it's a POSIX open. Match the return code from + * openat_pathref_fsp(). */ - return NT_STATUS_OBJECT_PATH_NOT_FOUND; + return NT_STATUS_OBJECT_NAME_NOT_FOUND; } if (!fsp->fsp_flags.is_pathref) { -- 2.30.2 From 71d82ee52d87bfabcc822712daef0ada5a32f4d4 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Tue, 7 Dec 2021 14:33:17 -0800 Subject: [PATCH 51/54] CVE-2021-44141: s3: smbd: Inside check_reduced_name() ensure we return the correct error codes when failing symlinks. NT_STATUS_OBJECT_PATH_NOT_FOUND for a path component failure. NT_STATUS_OBJECT_NAME_NOT_FOUND for a terminal component failure. Remove: samba3.blackbox.test_symlink_traversal.SMB1.posix samba3.blackbox.smbclient_s3.*.Ensure\ widelinks\ are\ restricted\(.*\) samba3.blackbox.smbclient_s3.*.follow\ symlinks\ \=\ no\(.*\) in knownfail.d/symlink_traversal as we now pass these. Only one more fix remaining to get rid of knownfail.d/symlink_traversal completely. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- selftest/knownfail.d/symlink_traversal | 3 --- source3/smbd/vfs.c | 18 ++++++++++++++++-- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/selftest/knownfail.d/symlink_traversal b/selftest/knownfail.d/symlink_traversal index 840ab38b0f9..2a51ff3f91d 100644 --- a/selftest/knownfail.d/symlink_traversal +++ b/selftest/knownfail.d/symlink_traversal @@ -1,5 +1,2 @@ ^samba3.blackbox.test_symlink_traversal.SMB2.symlink_traversal_SMB2\(fileserver\) ^samba3.blackbox.test_symlink_traversal.SMB1.symlink_traversal_SMB1\(fileserver_smb1_done\) -^samba3.blackbox.test_symlink_traversal.SMB1.posix.symlink_traversal_SMB1_posix\(fileserver_smb1_done\) -^samba3.blackbox.smbclient_s3.*.Ensure\ widelinks\ are\ restricted\(.*\) -^samba3.blackbox.smbclient_s3.*.follow\ symlinks\ \=\ no\(.*\) diff --git a/source3/smbd/vfs.c b/source3/smbd/vfs.c index 4091aa304d4..eadb97650a6 100644 --- a/source3/smbd/vfs.c +++ b/source3/smbd/vfs.c @@ -1146,6 +1146,7 @@ NTSTATUS check_reduced_name(connection_struct *conn, bool allow_symlinks = true; const char *conn_rootdir; size_t rootdir_len; + bool parent_dir_checked = false; DBG_DEBUG("check_reduced_name [%s] [%s]\n", fname, conn->connectpath); @@ -1207,6 +1208,7 @@ NTSTATUS check_reduced_name(connection_struct *conn, if (resolved_name == NULL) { return NT_STATUS_NO_MEMORY; } + parent_dir_checked = true; } else { resolved_name = resolved_fname->base_name; } @@ -1256,7 +1258,13 @@ NTSTATUS check_reduced_name(connection_struct *conn, conn_rootdir, resolved_name); TALLOC_FREE(resolved_fname); - return NT_STATUS_ACCESS_DENIED; + if (parent_dir_checked) { + /* Part of a component path. */ + return NT_STATUS_OBJECT_PATH_NOT_FOUND; + } else { + /* End of a path. */ + return NT_STATUS_OBJECT_NAME_NOT_FOUND; + } } } @@ -1311,7 +1319,13 @@ NTSTATUS check_reduced_name(connection_struct *conn, p); TALLOC_FREE(resolved_fname); TALLOC_FREE(new_fname); - return NT_STATUS_ACCESS_DENIED; + if (parent_dir_checked) { + /* Part of a component path. */ + return NT_STATUS_OBJECT_PATH_NOT_FOUND; + } else { + /* End of a path. */ + return NT_STATUS_OBJECT_NAME_NOT_FOUND; + } } } -- 2.30.2 From d80076370f9e7e0ef557eb0a235c3c2f7434ca4c Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Tue, 7 Dec 2021 14:39:42 -0800 Subject: [PATCH 52/54] CVE-2021-44141: s3: smbd: Fix a subtle bug in the error returns from filename_convert(). If filename_convert() fails to convert the path, we never call check_name(). This means we can return an incorrect error code (NT_STATUS_ACCESS_DENIED) if we ran into a symlink that points outside the share to a non-readable directory. We need to make sure in this case we always call check_name(). Remove knownfail.d/symlink_traversal. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- selftest/knownfail.d/symlink_traversal | 2 -- source3/smbd/filename.c | 36 ++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 2 deletions(-) delete mode 100644 selftest/knownfail.d/symlink_traversal diff --git a/selftest/knownfail.d/symlink_traversal b/selftest/knownfail.d/symlink_traversal deleted file mode 100644 index 2a51ff3f91d..00000000000 --- a/selftest/knownfail.d/symlink_traversal +++ /dev/null @@ -1,2 +0,0 @@ -^samba3.blackbox.test_symlink_traversal.SMB2.symlink_traversal_SMB2\(fileserver\) -^samba3.blackbox.test_symlink_traversal.SMB1.symlink_traversal_SMB1\(fileserver_smb1_done\) diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c index 19eea2d6a77..6b0c4b5a2a7 100644 --- a/source3/smbd/filename.c +++ b/source3/smbd/filename.c @@ -36,6 +36,9 @@ static int get_real_filename(connection_struct *conn, 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; @@ -546,6 +549,39 @@ static NTSTATUS unix_convert_step_search_fail(struct uc_state *state) if (errno == EACCES) { 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; + NTSTATUS status; + 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 { /* -- 2.30.2 From 61f07d4f3a6ed7d14f376f7ed1c9b3adcc21ff3c Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Tue, 7 Dec 2021 22:15:46 -0800 Subject: [PATCH 53/54] CVE-2021-44141: s3: torture: Add a test samba3.blackbox.test_symlink_rename.SMB1.posix that shows we still leak target info across a SMB1+POSIX rename. Add a knownfail.d/posix_sylink_rename BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- selftest/knownfail.d/posix_sylink_rename | 1 + .../tests/test_symlink_rename_smb1_posix.sh | 186 ++++++++++++++++++ source3/selftest/tests.py | 5 + 3 files changed, 192 insertions(+) create mode 100644 selftest/knownfail.d/posix_sylink_rename create mode 100755 source3/script/tests/test_symlink_rename_smb1_posix.sh diff --git a/selftest/knownfail.d/posix_sylink_rename b/selftest/knownfail.d/posix_sylink_rename new file mode 100644 index 00000000000..9c3cc0a41ba --- /dev/null +++ b/selftest/knownfail.d/posix_sylink_rename @@ -0,0 +1 @@ +^samba3.blackbox.test_symlink_rename.SMB1.posix.symlink_rename_SMB1_posix\(fileserver_smb1_done\) diff --git a/source3/script/tests/test_symlink_rename_smb1_posix.sh b/source3/script/tests/test_symlink_rename_smb1_posix.sh new file mode 100755 index 00000000000..7d2e0037b8d --- /dev/null +++ b/source3/script/tests/test_symlink_rename_smb1_posix.sh @@ -0,0 +1,186 @@ +#!/bin/sh + +if [ $# -lt 7 ]; then +cat < "$tmpfile" < Date: Tue, 7 Dec 2021 22:19:29 -0800 Subject: [PATCH 54/54] CVE-2021-44141: s3: smbd: Inside rename_internals_fsp(), we must use vfs_stat() for existence, not SMB_VFS_STAT(). We need to take SMB1+POSIX into account here and do an LSTAT if it's a POSIX name. Remove knownfail.d/posix_sylink_rename BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- selftest/knownfail.d/posix_sylink_rename | 1 - source3/smbd/reply.c | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) delete mode 100644 selftest/knownfail.d/posix_sylink_rename diff --git a/selftest/knownfail.d/posix_sylink_rename b/selftest/knownfail.d/posix_sylink_rename deleted file mode 100644 index 9c3cc0a41ba..00000000000 --- a/selftest/knownfail.d/posix_sylink_rename +++ /dev/null @@ -1 +0,0 @@ -^samba3.blackbox.test_symlink_rename.SMB1.posix.symlink_rename_SMB1_posix\(fileserver_smb1_done\) diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c index d7c5b962ca7..7e520e0256c 100644 --- a/source3/smbd/reply.c +++ b/source3/smbd/reply.c @@ -7271,7 +7271,7 @@ NTSTATUS rename_internals_fsp(connection_struct *conn, goto out; } - dst_exists = SMB_VFS_STAT(conn, smb_fname_dst) == 0; + dst_exists = vfs_stat(conn, smb_fname_dst) == 0; if(!replace_if_exists && dst_exists) { DEBUG(3, ("rename_internals_fsp: dest exists doing rename " -- 2.30.2