The branch, master has been updated
via 8b23a4a7eca smbd: Prevent creation of vetoed files
via 2e8954d5be3 CI: add a test creating a vetoed file
from e3b9e252338 lib:registry: drop unused argument of reg_open_remote()
https://git.samba.org/?p=samba.git;a=shortlog;h=master
- Log -----------------------------------------------------------------
commit 8b23a4a7eca9b8f80cc4113bb8cf9bb7bd5b4807
Author: Ralph Boehme <s...@samba.org>
Date: Wed Apr 5 11:03:52 2023 +0200
smbd: Prevent creation of vetoed files
The problem is when checking for vetoed names on the last path component in
openat_pathref_fsp_case_insensitive() we return
NT_STATUS_OBJECT_NAME_NOT_FOUND. The in the caller
filename_convert_dirfsp_nosymlink() this is treated as the "file creation
case"
causing filename_convert_dirfsp_nosymlink() to return NT_STATUS_OK.
In order to correctly distinguish between the cases
1) file doesn't exist, we may be creating it, return
2) a vetoed a file
we need 2) to return a more specific error to
filename_convert_dirfsp_nosymlink(). I've chosen
NT_STATUS_OBJECT_NAME_INVALID
which gets mapped to the appropriate errror NT_STATUS_OBJECT_PATH_NOT_FOUND
or
NT_STATUS_OBJECT_NAME_NOT_FOUND depending on which path component was
vetoed.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15143
Signed-off-by: Ralph Boehme <s...@samba.org>
Reviewed-by: Jeremy Allison <j...@samba.org>
Autobuild-User(master): Jeremy Allison <j...@samba.org>
Autobuild-Date(master): Thu Apr 6 23:03:50 UTC 2023 on atb-devel-224
commit 2e8954d5be3336f1c4c2cf033209f632ad84e712
Author: Ralph Boehme <s...@samba.org>
Date: Wed Apr 5 11:32:09 2023 +0200
CI: add a test creating a vetoed file
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15143
Signed-off-by: Ralph Boehme <s...@samba.org>
Reviewed-by: Jeremy Allison <j...@samba.org>
-----------------------------------------------------------------------
Summary of changes:
source3/script/tests/test_veto_files.sh | 47 +++++++++++++++++++++++++++++++++
source3/smbd/filename.c | 10 ++++---
2 files changed, 54 insertions(+), 3 deletions(-)
Changeset truncated at 500 lines:
diff --git a/source3/script/tests/test_veto_files.sh
b/source3/script/tests/test_veto_files.sh
index 9f0526bd54c..5ecfb53b8a4 100755
--- a/source3/script/tests/test_veto_files.sh
+++ b/source3/script/tests/test_veto_files.sh
@@ -84,6 +84,42 @@ EOF
fi
}
+smbclient_create_expect_error()
+{
+ filename="$1.$$"
+ expected_error="$2"
+ tmpfile=$PREFIX/smbclient_interactive_prompt_commands
+ cat >"$tmpfile" <<EOF
+put $tmpfile $filename
+quit
+EOF
+
+ cmd='CLI_FORCE_INTERACTIVE=yes $SMBCLIENT -U$USERNAME%$PASSWORD
//$SERVER/veto_files -I$SERVER_IP < $tmpfile 2>&1'
+ eval echo "$cmd"
+ out=$(eval "$cmd")
+ ret=$?
+ rm -f "$tmpfile"
+ rm -f "$SHAREPATH/$filename"
+
+ if [ $ret != 0 ]; then
+ printf "%s\n" "$out"
+ printf "failed accessing veto_files share with error %s\n"
"$ret"
+ return 1
+ fi
+
+ if [ "$expected_error" = "NT_STATUS_OK" ]; then
+ printf "%s" "$out" | grep -c "NT_STATUS_" && false
+ else
+ printf "%s" "$out" | grep "$expected_error"
+ fi
+ ret=$?
+ if [ $ret != 0 ]; then
+ printf "%s\n" "$out"
+ printf "failed - should get %s doing \"put %s\"\n"
"$expected_error" "$filename"
+ return 1
+ fi
+}
+
#
# Using the share "[veto_files]" ensure we
# cannot fetch a veto'd file or file in a veto'd directory.
@@ -133,6 +169,16 @@ test_get_veto_file()
return 0
}
+test_create_veto_file()
+{
+ # Test creating files
+ smbclient_create_expect_error "veto_name_file"
"NT_STATUS_OBJECT_NAME_NOT_FOUND" || return 1
+ smbclient_create_expect_error "veto_name_dir/file_inside_dir"
"NT_STATUS_OBJECT_PATH_NOT_FOUND" || return 1
+ smbclient_create_expect_error "dir1/veto_name_file"
"NT_STATUS_OBJECT_NAME_NOT_FOUND" || return 1
+
+ return 0
+}
+
do_cleanup
# Using hash2, veto_name_file\"mangle == VHXE5P~M
@@ -194,6 +240,7 @@ touch
"$SHAREPATH/dir1/dir2/dir3/veto_name_dir\"mangle/file_inside_dir"
mkdir "$SHAREPATH/dir1/dir2/dir3/veto_name_dir\"mangle/testdir"
touch "$SHAREPATH/dir1/dir2/dir3/veto_name_dir\"mangle/testdir/file_inside_dir"
+testit "create_veto_file" test_create_veto_file || failed=$((failed + 1))
testit "get_veto_file" test_get_veto_file || failed=$(("$failed" + 1))
do_cleanup
diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c
index e5cb3c867cd..a5ee0392bae 100644
--- a/source3/smbd/filename.c
+++ b/source3/smbd/filename.c
@@ -752,7 +752,7 @@ static NTSTATUS openat_pathref_fsp_case_insensitive(
if (IS_VETO_PATH(dirfsp->conn, smb_fname_rel->base_name)) {
DBG_DEBUG("veto files rejecting last component %s\n",
smb_fname_str_dbg(smb_fname_rel));
- return NT_STATUS_OBJECT_NAME_NOT_FOUND;
+ return NT_STATUS_NETWORK_OPEN_RESTRICTION;
}
status = openat_pathref_fsp(dirfsp, smb_fname_rel);
@@ -818,7 +818,7 @@ static NTSTATUS openat_pathref_fsp_case_insensitive(
DBG_DEBUG("veto files rejecting last component %s\n",
smb_fname_str_dbg(smb_fname_rel));
TALLOC_FREE(cache_key.data);
- return NT_STATUS_OBJECT_NAME_NOT_FOUND;
+ return NT_STATUS_NETWORK_OPEN_RESTRICTION;
}
status = openat_pathref_fsp(dirfsp, smb_fname_rel);
@@ -848,7 +848,7 @@ lookup:
if (IS_VETO_PATH(dirfsp->conn, smb_fname_rel->base_name)) {
DBG_DEBUG("veto files rejecting last component %s\n",
smb_fname_str_dbg(smb_fname_rel));
- return NT_STATUS_OBJECT_NAME_NOT_FOUND;
+ return NT_STATUS_NETWORK_OPEN_RESTRICTION;
}
status = openat_pathref_fsp(dirfsp, smb_fname_rel);
@@ -1292,6 +1292,10 @@ static NTSTATUS filename_convert_dirfsp_nosymlink(
goto done;
}
+ if (NT_STATUS_EQUAL(status, NT_STATUS_NETWORK_OPEN_RESTRICTION)) {
+ /* A vetoed file, pretend it's not there */
+ status = NT_STATUS_OBJECT_NAME_NOT_FOUND;
+ }
if (!NT_STATUS_IS_OK(status)) {
goto fail;
}
--
Samba Shared Repository
