s3: smbd: Remove ugly SMB1-specific hack to filename_convert_dirfsp()

This was added due to the error code check in test_symlink_traversal_smb1_posix.sh.
After careful consideration I've realized the error code expected here
is incorrect, and not providing any security benefit.

We already check that trying to fetch a file/traverse through a
symlink that points outside of a share returns NT_STATUS_OBJECT_PATH_NOT_FOUND,
and this is enforced in the symlink checks already inside filename_convert_dirfsp().

If a symlink points to a directory within the share for which
the user has no permissions (as is tested here), then there's no
benefit in mapping the error code from NT_STATUS_ACCESS_DENIED
to NT_STATUS_OBJECT_PATH_NOT_FOUND, as we are not providing any
extra information about the filesystem state the user cannot already
obtain by normal SMB1+POSIX calls.

Change the error code expected in this single test from NT_STATUS_OBJECT_PATH_NOT_FOUND
to NT_STATUS_ACCESS_DENIED.

Signed-off-by: Jeremy Allison <jra@samba.org>
Reviewed-by: Volker Lendecke <vl@samba.org>
Autobuild-User(master): Volker Lendecke <vl@samba.org>
Autobuild-Date(master): Fri Aug  5 10:24:23 UTC 2022 on sn-devel-184

diff --git a/source3/script/tests/test_symlink_traversal_smb1_posix.sh b/source3/script/tests/test_symlink_traversal_smb1_posix.sh
index 1bde9c4ffa1bf4ac2917b992d5d5a9b8ca26763b..1ad1d56f12a5000ef730934fbd66047a3b22431c 100755 (executable)
--- a/source3/script/tests/test_symlink_traversal_smb1_posix.sh
+++ b/source3/script/tests/test_symlink_traversal_smb1_posix.sh
@@ -252,7 +252,7 @@ test_symlink_traversal_SMB1_posix()
        # Can't 'get' file inside a directory with no perms.
        smbclient_expect_error "get" "dir_inside_share_noperms/noperm_file_exists" "" "NT_STATUS_ACCESS_DENIED" || return 1
        # In SMB1+POSIX you can't traverse through a symlink that points to a noperm directory.
-       smbclient_expect_error "get" "symlink_dir_inside_share_noperms/noperm_file_exists" "" "NT_STATUS_OBJECT_PATH_NOT_FOUND" || return 1
+       smbclient_expect_error "get" "symlink_dir_inside_share_noperms/noperm_file_exists" "" "NT_STATUS_ACCESS_DENIED" || return 1
        # But can list the directory with no perms and the symlink to it.
        smbclient_expect_error "ls" "dir_inside_share_noperms" "" "NT_STATUS_OK" || return 1
        smbclient_expect_error "ls" "symlink_dir_inside_share_noperms" "" "NT_STATUS_OK" || return 1

diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c
index 7380131a274942f2561c610cd1349ee6138f954d..87abc8be376873cff9a589a65e88ed4e056cc3c7 100644 (file)
--- a/source3/smbd/filename.c
+++ b/source3/smbd/filename.c
@@ -2817,22 +2817,6 @@ next:
                &substitute,
                &unparsed);
 
-#if defined(WITH_SMB1SERVER)
-       /*
-        * This isn't 100% correct, but it gets us close enough
-        * to the old behavior for SMB1+POSIX libsmbclient. If we went through a
-        * symlink, and we got NT_STATUS_ACCESS_DENIED on the directory
-        * containing the target, just don't allow the client to see the
-        * intermediate path.
-        */
-       if (!conn->sconn->using_smb2 &&
-                       (ucf_flags & UCF_POSIX_PATHNAMES) &&
-                       symlink_redirects > 0 &&
-                       NT_STATUS_EQUAL(status, NT_STATUS_ACCESS_DENIED)) {
-               return NT_STATUS_OBJECT_PATH_NOT_FOUND;
-       }
-#endif
-
        if (!NT_STATUS_EQUAL(status, NT_STATUS_STOPPED_ON_SYMLINK)) {
                return status;
        }