1
0
mirror of https://github.com/samba-team/samba.git synced 2024-12-23 17:34:34 +03:00

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
This commit is contained in:
Jeremy Allison 2022-08-04 11:32:05 -07:00 committed by Volker Lendecke
parent 5c9404f753
commit 5075df4575
2 changed files with 1 additions and 17 deletions

View File

@ -252,7 +252,7 @@ test_symlink_traversal_SMB1_posix()
# Can't 'get' file inside a directory with no perms. # 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 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. # 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. # 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" "dir_inside_share_noperms" "" "NT_STATUS_OK" || return 1
smbclient_expect_error "ls" "symlink_dir_inside_share_noperms" "" "NT_STATUS_OK" || return 1 smbclient_expect_error "ls" "symlink_dir_inside_share_noperms" "" "NT_STATUS_OK" || return 1

View File

@ -2817,22 +2817,6 @@ next:
&substitute, &substitute,
&unparsed); &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)) { if (!NT_STATUS_EQUAL(status, NT_STATUS_STOPPED_ON_SYMLINK)) {
return status; return status;
} }