[SCM] Samba Shared Repository - branch v4-18-test updated

Jule Anger janger at samba.org
Mon Jan 29 11:54:01 UTC 2024


The branch, v4-18-test has been updated
       via  974a8908223 smbd: use dirfsp and atname in open_directory()
       via  ab7d69665d0 smbd: use safe_symlink_target_path() in symlink_target_below_conn()
       via  0086f3d4b7b smbd: add a directory argument to safe_symlink_target_path()
       via  f495f6d2778 smbd: pass symlink target path to safe_symlink_target_path()
       via  8bac9003342 CI: disable /proc/fds and RESOLVE_NO_SYMLINK in samba-no-opath-build runner
       via  4b1f0c6e8bb vfs_default: allow disabling /proc/fds and RESOLVE_NO_SYMLINK at compile time
      from  e6745b15107 s3:passdb: smbpasswd reset permissions only if not 0600

https://git.samba.org/?p=samba.git;a=shortlog;h=v4-18-test


- Log -----------------------------------------------------------------
commit 974a890822384314178281875c8afbd9959e2db1
Author: Ralph Boehme <slow at samba.org>
Date:   Mon Dec 18 12:35:58 2023 +0100

    smbd: use dirfsp and atname in open_directory()
    
    On systems without /proc/fd support this avoid the expensive chdir()
    logic in non_widelink_open(). open_file_ntcreate() already passes
    dirfsp and atname to reopen_from_fsp(), it was just missed in the
    conversion.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=15549
    
    Reviewed-by: Volker Lendecke <vl at samba.org>
    Signed-off-by: Ralph Boehme <slow at samba.org>
    
    Autobuild-User(master): Volker Lendecke <vl at samba.org>
    Autobuild-Date(master): Mon Jan 22 12:00:56 UTC 2024 on atb-devel-224
    
    (cherry picked from commit 2713023250f15cf9971d88620cab9dd4afd0dc73)
    
    Autobuild-User(v4-18-test): Jule Anger <janger at samba.org>
    Autobuild-Date(v4-18-test): Mon Jan 29 11:53:56 UTC 2024 on atb-devel-224

commit ab7d69665d0d9bffd5ba06efa4bf28e09c041b7b
Author: Ralph Boehme <slow at samba.org>
Date:   Tue Jan 2 14:34:26 2024 +0100

    smbd: use safe_symlink_target_path() in symlink_target_below_conn()
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=15549
    
    Signed-off-by: Ralph Boehme <slow at samba.org>
    Reviewed-by: Volker Lendecke <vl at samba.org>
    (cherry picked from commit 1965fc77b3852a0593e13897af08f5304a1ce3a2)

commit 0086f3d4b7b4bd5726c19863c063af0a8de10cd5
Author: Ralph Boehme <slow at samba.org>
Date:   Tue Jan 2 13:25:25 2024 +0100

    smbd: add a directory argument to safe_symlink_target_path()
    
    Existing caller passes NULL, no change in behaviour. Prepares for
    replacing symlink_target_below_conn() in open.c.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=15549
    
    Signed-off-by: Ralph Boehme <slow at samba.org>
    Reviewed-by: Volker Lendecke <vl at samba.org>
    (cherry picked from commit fc80c72d658a41fe4d93b24b793b52c91b350175)

commit f495f6d2778c47f137d896a2351fe7ddb1637d3f
Author: Ralph Boehme <slow at samba.org>
Date:   Tue Jan 2 12:49:14 2024 +0100

    smbd: pass symlink target path to safe_symlink_target_path()
    
    Moves processing the symlink error response to the caller
    filename_convert_dirfsp(). Prepares for using this in
    non_widelink_open(), where it will replace symlink_target_below_conn()
    with the same functionality.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=15549
    
    Signed-off-by: Ralph Boehme <slow at samba.org>
    Reviewed-by: Volker Lendecke <vl at samba.org>
    (back-ported from commit 0515dded4ddb49e5570ae7df51126af1a2d643de)

commit 8bac90033423e96addc0a50418381a943eba9839
Author: Ralph Boehme <slow at samba.org>
Date:   Tue Dec 19 11:12:49 2023 +0100

    CI: disable /proc/fds and RESOLVE_NO_SYMLINK in samba-no-opath-build runner
    
    This is a more sensible combination of missing Linux specific features:
    
    - O_PATH
    - openat2() with RESOLVE_NO_SYMLINKS
    - somehow safely reopen an O_PATH file handle
    
    Currently only O_PATH is disabled for these jobs, but that doesn't really match
    and know OS.
    
    The following list shows which features are available and used by Samba on a few
    OSes:
    
            | O_PATH         | RESOLVE_NO_SYMLINKS | Safe reopen    | CI covered
    --------|----------------|---------------------|----------------------------
            | Supported Used | Supported Used      | Supported Used |
    ============================================================================
    Linux   | +         +    | +         +         | +         +    | +
    FreeBSD | +         +    | + [1]     -         | + [2]     -    | -
    AIX     | -         -    | -         -         | -         -    | +
    
    So by also disabling RESOLVE_NO_SYMLINKS and Safe Reopen, we cover classic UNIX
    systems like AIX.
    
    [1] via open() flag O_RESOLVE_BENEATH
    [2] via open() flag O_EMPTY_PATH
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=15549
    
    Signed-off-by: Ralph Boehme <slow at samba.org>
    Reviewed-by: Volker Lendecke <vl at samba.org>
    (cherry picked from commit 62cbe145c7e500c4759ed2005c78bd5056c87f43)

commit 4b1f0c6e8bb98088eb8ec6864e4f1f614720ed70
Author: Ralph Boehme <slow at samba.org>
Date:   Tue Dec 19 11:11:55 2023 +0100

    vfs_default: allow disabling /proc/fds and RESOLVE_NO_SYMLINK at compile time
    
    This will be used in CI to have a gitlab runner without all modern Linux
    features we make use of as part of path processing:
    
    - O_PATH
    - openat2() with RESOLVE_NO_SYMLINKS
    - somehow safely reopen an O_PATH file handle
    
    That gives what a classix UNIX like AIX or Solaris offers feature wise.
    
    Other OSes support other combinations of those features, but we leave the
    exersize of possibly adding more runners supporting those combinations to the
    reader.
    
    The following list shows which features are available and used by Samba on a few
    OSes:
    
            | O_PATH         | RESOLVE_NO_SYMLINKS | Safe reopen    | CI covered
    --------|----------------|---------------------|----------------------------
            | Supported Used | Supported Used      | Supported Used |
    ============================================================================
    Linux   | +         +    | +         +         | +         +    | +
    FreeBSD | +         +    | + [1]     -         | + [2]     -    | -
    AIX     | -         -    | -         -         | -         -    | +
    
    [1] via open() flag O_RESOLVE_BENEATH
    [2] via open() flag O_EMPTY_PATH
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=15549
    
    Signed-off-by: Ralph Boehme <slow at samba.org>
    Reviewed-by: Volker Lendecke <vl at samba.org>
    (cherry picked from commit 5c2f96442a25a1725809a28b3719afbc0bd01830)

-----------------------------------------------------------------------

Summary of changes:
 script/autobuild.py           |  2 +-
 selftest/skip.opath-required  |  4 ++
 source3/include/proto.h       |  6 +++
 source3/modules/vfs_default.c |  6 +++
 source3/smbd/filename.c       | 85 +++++++++++++++++++++++--------------------
 source3/smbd/open.c           | 77 ++++++++-------------------------------
 6 files changed, 79 insertions(+), 101 deletions(-)


Changeset truncated at 500 lines:

diff --git a/script/autobuild.py b/script/autobuild.py
index afa757491e0..577eaf13171 100755
--- a/script/autobuild.py
+++ b/script/autobuild.py
@@ -288,7 +288,7 @@ tasks = {
     "samba-no-opath-build": {
         "git-clone-required": True,
         "sequence": [
-            ("configure", "ADDITIONAL_CFLAGS='-DDISABLE_OPATH=1' ./configure.developer --without-ad-dc " + samba_configure_params),
+            ("configure", "ADDITIONAL_CFLAGS='-DDISABLE_OPATH=1 -DDISABLE_VFS_OPEN_HOW_RESOLVE_NO_SYMLINKS=1 -DDISABLE_PROC_FDS=1' ./configure.developer --without-ad-dc " + samba_configure_params),
             ("make", "make -j"),
             ("check-clean-tree", CLEAN_SOURCE_TREE_CMD),
             ("chmod-R-a-w", "chmod -R a-w ."),
diff --git a/selftest/skip.opath-required b/selftest/skip.opath-required
index 0faf0c4bd6c..b6758fdbcac 100644
--- a/selftest/skip.opath-required
+++ b/selftest/skip.opath-required
@@ -7,3 +7,7 @@
 # These fail because become_root() doesn't work in make test
 ^samba3.blackbox.dropbox.*
 ^samba3.raw.samba3hide.*
+
+# These don't work without /proc/fd support
+^samba3.blackbox.shadow_copy_torture.*\(fileserver\)
+^samba3.blackbox.virus_scanner.*\(fileserver:local\)
diff --git a/source3/include/proto.h b/source3/include/proto.h
index f71796b57ae..7df3c7a8e01 100644
--- a/source3/include/proto.h
+++ b/source3/include/proto.h
@@ -718,6 +718,12 @@ struct smb_filename *synthetic_smb_fname(TALLOC_CTX *mem_ctx,
 					 const SMB_STRUCT_STAT *psbuf,
 					 NTTIME twrp,
 					 uint32_t flags);
+NTSTATUS safe_symlink_target_path(TALLOC_CTX *mem_ctx,
+				  const char *connectpath,
+				  const char *dir,
+				  const char *target,
+				  size_t unparsed,
+				  char **_relative);
 NTSTATUS filename_convert_dirfsp(
 	TALLOC_CTX *ctx,
 	connection_struct *conn,
diff --git a/source3/modules/vfs_default.c b/source3/modules/vfs_default.c
index 89eec1146d7..218316867b8 100644
--- a/source3/modules/vfs_default.c
+++ b/source3/modules/vfs_default.c
@@ -52,6 +52,9 @@ static int vfswrap_connect(vfs_handle_struct *handle, const char *service, const
 	bool bval;
 
 	handle->conn->have_proc_fds = sys_have_proc_fds();
+#ifdef DISABLE_PROC_FDS
+	handle->conn->have_proc_fds = false;
+#endif
 
 	/*
 	 * assume the kernel will support openat2(),
@@ -70,6 +73,9 @@ static int vfswrap_connect(vfs_handle_struct *handle, const char *service, const
 		handle->conn->open_how_resolve |=
 			VFS_OPEN_HOW_RESOLVE_NO_SYMLINKS;
 	}
+#ifdef DISABLE_VFS_OPEN_HOW_RESOLVE_NO_SYMLINKS
+	handle->conn->open_how_resolve &= ~VFS_OPEN_HOW_RESOLVE_NO_SYMLINKS;
+#endif
 
 	return 0;    /* Return >= 0 for success */
 }
diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c
index dd0239777e8..1e07abacea6 100644
--- a/source3/smbd/filename.c
+++ b/source3/smbd/filename.c
@@ -943,44 +943,46 @@ static char *symlink_target_path(
 	return ret;
 }
 
-static NTSTATUS safe_symlink_target_path(
-	TALLOC_CTX *mem_ctx,
-	const char *connectpath,
-	const char *name_in,
-	const char *substitute,
-	size_t unparsed,
-	char **_name_out)
+NTSTATUS safe_symlink_target_path(TALLOC_CTX *mem_ctx,
+				  const char *connectpath,
+				  const char *dir,
+				  const char *target,
+				  size_t unparsed,
+				  char **_relative)
 {
-	char *target = NULL;
 	char *abs_target = NULL;
 	char *abs_target_canon = NULL;
 	const char *relative = NULL;
-	char *name_out = NULL;
-	NTSTATUS status = NT_STATUS_NO_MEMORY;
 	bool in_share;
+	NTSTATUS status = NT_STATUS_NO_MEMORY;
 
-	target = symlink_target_path(mem_ctx, name_in, substitute, unparsed);
-	if (target == NULL) {
-		goto fail;
-	}
-
-	DBG_DEBUG("name_in: %s, substitute: %s, unparsed: %zu, target=%s\n",
-		  name_in,
-		  substitute,
-		  unparsed,
-		  target);
+	DBG_DEBUG("connectpath [%s] target [%s] unparsed [%zu]\n",
+		  connectpath, target, unparsed);
 
 	if (target[0] == '/') {
-		abs_target = target;
+		abs_target = talloc_strdup(mem_ctx, target);
+	} else if (dir == NULL) {
+		abs_target = talloc_asprintf(mem_ctx,
+					     "%s/%s",
+					     connectpath,
+					     target);
+	} else if (dir[0] == '/') {
+		abs_target = talloc_asprintf(mem_ctx,
+					     "%s/%s",
+					     dir,
+					     target);
 	} else {
-		abs_target = talloc_asprintf(
-			target, "%s/%s", connectpath, target);
-		if (abs_target == NULL) {
-			goto fail;
-		}
+		abs_target = talloc_asprintf(mem_ctx,
+					     "%s/%s/%s",
+					     connectpath,
+					     dir,
+					     target);
+	}
+	if (abs_target == NULL) {
+		goto fail;
 	}
 
-	abs_target_canon = canonicalize_absolute_path(target, abs_target);
+	abs_target_canon = canonicalize_absolute_path(abs_target, abs_target);
 	if (abs_target_canon == NULL) {
 		goto fail;
 	}
@@ -995,15 +997,14 @@ static NTSTATUS safe_symlink_target_path(
 		goto fail;
 	}
 
-	name_out = talloc_strdup(mem_ctx, relative);
-	if (name_out == NULL) {
+	*_relative = talloc_strdup(mem_ctx, relative);
+	if (*_relative == NULL) {
 		goto fail;
 	}
 
 	status = NT_STATUS_OK;
-	*_name_out = name_out;
 fail:
-	TALLOC_FREE(target);
+	TALLOC_FREE(abs_target);
 	return status;
 }
 
@@ -1456,6 +1457,7 @@ NTSTATUS filename_convert_dirfsp(
 	size_t unparsed = 0;
 	NTSTATUS status;
 	char *target = NULL;
+	char *safe_target = NULL;
 	size_t symlink_redirects = 0;
 
 next:
@@ -1494,17 +1496,22 @@ next:
 	 * resolve all symlinks locally.
 	 */
 
-	status = safe_symlink_target_path(
-		mem_ctx,
-		conn->connectpath,
-		name_in,
-		substitute,
-		unparsed,
-		&target);
+	target = symlink_target_path(mem_ctx, name_in, substitute, unparsed);
+	if (target == NULL) {
+		return NT_STATUS_NO_MEMORY;
+	}
+
+	status = safe_symlink_target_path(mem_ctx,
+					  conn->connectpath,
+					  NULL,
+					  target,
+					  unparsed,
+					  &safe_target);
+	TALLOC_FREE(target);
 	if (!NT_STATUS_IS_OK(status)) {
 		return status;
 	}
-	name_in = target;
+	name_in = safe_target;
 
 	symlink_redirects += 1;
 
diff --git a/source3/smbd/open.c b/source3/smbd/open.c
index edc72bb03f0..3e7a8f45ebd 100644
--- a/source3/smbd/open.c
+++ b/source3/smbd/open.c
@@ -571,7 +571,6 @@ out:
 static NTSTATUS symlink_target_below_conn(
 	TALLOC_CTX *mem_ctx,
 	const char *connection_path,
-	size_t connection_path_len,
 	struct files_struct *fsp,
 	struct files_struct *dirfsp,
 	struct smb_filename *symlink_name,
@@ -579,9 +578,7 @@ static NTSTATUS symlink_target_below_conn(
 {
 	char *target = NULL;
 	char *absolute = NULL;
-	const char *relative = NULL;
 	NTSTATUS status;
-	bool ok;
 
 	if (fsp_get_pathref_fd(fsp) != -1) {
 		/*
@@ -594,69 +591,28 @@ static NTSTATUS symlink_target_below_conn(
 			talloc_tos(), dirfsp, symlink_name, &target);
 	}
 
+	status = safe_symlink_target_path(talloc_tos(),
+					  connection_path,
+					  dirfsp->fsp_name->base_name,
+					  target,
+					  0,
+					  &absolute);
 	if (!NT_STATUS_IS_OK(status)) {
-		DBG_DEBUG("readlink_talloc failed: %s\n", nt_errstr(status));
+		DBG_DEBUG("safe_symlink_target_path() failed: %s\n",
+			  nt_errstr(status));
 		return status;
 	}
 
-	if (target[0] != '/') {
-		char *tmp = talloc_asprintf(
-			talloc_tos(),
-			"%s/%s/%s",
-			connection_path,
-			dirfsp->fsp_name->base_name,
-			target);
-
-		TALLOC_FREE(target);
-
-		if (tmp == NULL) {
-			return NT_STATUS_NO_MEMORY;
-		}
-		target = tmp;
-	}
-
-	DBG_DEBUG("redirecting to %s\n", target);
-
-	absolute = canonicalize_absolute_path(talloc_tos(), target);
-	TALLOC_FREE(target);
-
-	if (absolute == NULL) {
-		return NT_STATUS_NO_MEMORY;
-	}
-
-	/*
-	 * We're doing the "below connection_path" here because it's
-	 * cheap. It might be that we get a symlink out of the share,
-	 * pointing to yet another symlink getting us back into the
-	 * share. If we need that, we would have to remove the check
-	 * here.
-	 */
-	ok = subdir_of(
-		connection_path,
-		connection_path_len,
-		absolute,
-		&relative);
-	if (!ok) {
-		DBG_NOTICE("Bad access attempt: %s is a symlink "
-			   "outside the share path\n"
-			   "conn_rootdir =%s\n"
-			   "resolved_name=%s\n",
-			   symlink_name->base_name,
-			   connection_path,
-			   absolute);
-		TALLOC_FREE(absolute);
-		return NT_STATUS_OBJECT_NAME_NOT_FOUND;
-	}
-
-	if (relative[0] == '\0') {
+	if (absolute[0] == '\0') {
 		/*
 		 * special case symlink to share root: "." is our
 		 * share root filename
 		 */
-		absolute[0] = '.';
-		absolute[1] = '\0';
-	} else {
-		memmove(absolute, relative, strlen(relative)+1);
+		TALLOC_FREE(absolute);
+		absolute = talloc_strdup(talloc_tos(), ".");
+		if (absolute == NULL) {
+			return NT_STATUS_NO_MEMORY;
+		}
 	}
 
 	*_target = absolute;
@@ -834,7 +790,6 @@ again:
 	status = symlink_target_below_conn(
 		talloc_tos(),
 		connpath,
-		connpath_len,
 		fsp,
 		discard_const_p(files_struct, dirfsp),
 		smb_fname_rel,
@@ -4960,8 +4915,8 @@ static NTSTATUS open_directory(connection_struct *conn,
 
 	if (access_mask & need_fd_access) {
 		status = reopen_from_fsp(
-			fsp->conn->cwd_fsp,
-			fsp->fsp_name,
+			parent_dir_fname->fsp,
+			smb_fname_atname,
 			fsp,
 			O_RDONLY | O_DIRECTORY,
 			0,


-- 
Samba Shared Repository



More information about the samba-cvs mailing list