From 4d7f14632c0f7c5af034c4489d3397eca0dd4dd2 Mon Sep 17 00:00:00 2001 From: Uri Simchoni Date: Tue, 23 Aug 2016 11:33:52 +0300 Subject: [PATCH 1/4] selftest: add content to files created during shadow_copy2 test This will allow reading them and verifying we got the right version BUG: https://bugzilla.samba.org/show_bug.cgi?id=12172 Signed-off-by: Uri Simchoni Reviewed-by: Jeremy Allison (cherry picked from commit 523046080dd65607eacb901d58ee3b6e54de865e) --- source3/script/tests/test_shadow_copy.sh | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/source3/script/tests/test_shadow_copy.sh b/source3/script/tests/test_shadow_copy.sh index 171008d..62155b0 100755 --- a/source3/script/tests/test_shadow_copy.sh +++ b/source3/script/tests/test_shadow_copy.sh @@ -42,9 +42,11 @@ build_files() local prefix local version local destdir + local content rootdir=$1 prefix=$2 version=$3 + content=$4 if [ -n "$prefix" ] ; then destdir=$rootdir/$prefix else @@ -56,27 +58,27 @@ build_files() #non-snapshot files # for non-snapshot version, create legit files # so that wide-link checks focus on snapshot files - touch $destdir/foo + echo "$content" > $destdir/foo mkdir -p $destdir/bar - touch $destdir/bar/baz - touch $destdir/bar/lfoo - touch $destdir/bar/letcpasswd - touch $destdir/bar/loutside + echo "$content" > $destdir/bar/baz + echo "$content" > $destdir/bar/lfoo + echo "$content" > $destdir/bar/letcpasswd + echo "$content" > $destdir/bar/loutside elif [ "$version" = "fullsnap" ] ; then #snapshot files - touch $destdir/foo + echo "$content" > $destdir/foo mkdir -p $destdir/bar - touch $destdir/bar/baz + echo "$content" > $destdir/bar/baz ln -fs ../foo $destdir/bar/lfoo ln -fs /etc/passwd $destdir/bar/letcpasswd ln -fs ../../outside $destdir/bar/loutside - touch `dirname $destdir`/outside + echo "$content" > `dirname $destdir`/outside else #subshare snapshot - at bar - touch $destdir/baz + echo "$content" > $destdir/baz ln -fs ../foo $destdir/lfoo ln -fs /etc/passwd $destdir/letcpasswd ln -fs ../../outside $destdir/loutside - touch `dirname $destdir`/../outside + echo "$content" > `dirname $destdir`/../outside fi } @@ -117,7 +119,7 @@ build_snapshots() for i in `seq $start $end` ; do snapname=${SNAPSHOTS[$i]} mkdir $snapdir/$snapname - build_files $snapdir/$snapname "$prefix" $version + build_files $snapdir/$snapname "$prefix" $version "$snapname" done } @@ -262,7 +264,7 @@ test_shadow_copy_everywhere() } #build "latest" files -build_files $WORKDIR/mount base/share "latest" +build_files $WORKDIR/mount base/share "latest" "latest" failed=0 -- 2.5.5 From 310fbc3d598bb6bd10d1dbe08488055934f57269 Mon Sep 17 00:00:00 2001 From: Uri Simchoni Date: Tue, 23 Aug 2016 14:03:30 +0300 Subject: [PATCH 2/4] selftest: check file readability in shadow_copy2 test Add tests which verify that a snapshot file is readable if and only if it its metadata can be retrieved. Also verify (in most tests) that file is retrieved from the correct snapshot. Together with the existing test for number of previous versions we can stat, this test checks that we can read those files, and also that we cannot break out of a snapshot if wide links are not allowed. BUG: https://bugzilla.samba.org/show_bug.cgi?id=12172 Signed-off-by: Uri Simchoni Reviewed-by: Jeremy Allison (cherry picked from commit 495b8177363bf1930f3afb373ad73caac022f353) --- source3/script/tests/test_shadow_copy.sh | 42 +++++++++++++++++++++++++++----- 1 file changed, 36 insertions(+), 6 deletions(-) diff --git a/source3/script/tests/test_shadow_copy.sh b/source3/script/tests/test_shadow_copy.sh index 62155b0..99b405f 100755 --- a/source3/script/tests/test_shadow_copy.sh +++ b/source3/script/tests/test_shadow_copy.sh @@ -129,18 +129,48 @@ test_count_versions() local share local path local expected_count + local skip_content local versions + local tstamps + local tstamp + local content share=$1 path=$2 expected_count=$3 + skip_content=$4 versions=`$SMBCLIENT -U$USERNAME%$PASSWORD "//$SERVER/$share" -I $SERVER_IP -c "allinfo $path" | grep "^create_time:" | wc -l` - if [ "$versions" = "$expected_count" ] ; then - true - else + if [ "$versions" != "$expected_count" ] ; then echo "expected $expected_count versions of $path, got $versions" - false + return 1 fi + + #readable snapshots + tstamps=`$SMBCLIENT -U$USERNAME%$PASSWORD "//$SERVER/$share" -I $SERVER_IP -c "allinfo $path" | awk '/^@GMT-/ {snapshot=$1} /^create_time:/ {printf "%s\n", snapshot}'` + for tstamp in $tstamps ; do + if ! $SMBCLIENT -U$USERNAME%$PASSWORD "//$SERVER/$share" -I $SERVER_IP -c "get $tstamp\\$path $WORKDIR/foo" ; then + echo "Failed getting \\\\$SERVER\\$share\\$tstamp\\$path" + return 1 + fi + #also check the content, but not for wide links + if [ "x$skip_content" != "x1" ] ; then + content=`cat $WORKDIR/foo` + if [ "$content" != "$tstamp" ] ; then + echo "incorrect content of \\\\$SERVER\\$share\\$tstamp\\$path expected [$tstamp] got [$content]" + return 1 + fi + fi + done + + #non-readable snapshots + tstamps=`$SMBCLIENT -U$USERNAME%$PASSWORD "//$SERVER/$share" -I $SERVER_IP -c "allinfo $path" | \ + awk '/^@GMT-/ {if (snapshot!=""){printf "%s\n", snapshot} ; snapshot=$1} /^create_time:/ {snapshot=""} END {if (snapshot!=""){printf "%s\n", snapshot}}'` + for tstamp in $tstamps ; do + if $SMBCLIENT -U$USERNAME%$PASSWORD "//$SERVER/$share" -I $SERVER_IP -c "get $tstamp\\$path $WORKDIR/foo" ; then + echo "Unexpected success getting \\\\$SERVER\\$share\\$tstamp\\$path" + return 1 + fi + done } # Test fetching a previous version of a file @@ -196,11 +226,11 @@ test_shadow_copy_fixed() failed=`expr $failed + 1` testit "$msg - abs symlink outside" \ - test_count_versions $share bar/letcpasswd $ncopies_blocked || \ + test_count_versions $share bar/letcpasswd $ncopies_blocked 1 || \ failed=`expr $failed + 1` testit "$msg - rel symlink outside" \ - test_count_versions $share bar/loutside $ncopies_blocked || \ + test_count_versions $share bar/loutside $ncopies_blocked 1 || \ failed=`expr $failed + 1` } -- 2.5.5 From f08ea23946eeef6e1e98d3b6b70eff4f91bed8c7 Mon Sep 17 00:00:00 2001 From: Uri Simchoni Date: Tue, 23 Aug 2016 14:29:39 +0300 Subject: [PATCH 3/4] selftest: test listing directories inside snapshots Verify that directories are also listable. BUG: https://bugzilla.samba.org/show_bug.cgi?id=12172 Signed-off-by: Uri Simchoni Reviewed-by: Jeremy Allison (cherry picked from commit 22c3982100a1d6bf67979a0659604942ef6f11f0) --- selftest/knownfail | 2 ++ source3/script/tests/test_shadow_copy.sh | 40 +++++++++++++++++++++++++++----- 2 files changed, 36 insertions(+), 6 deletions(-) diff --git a/selftest/knownfail b/selftest/knownfail index aab1456..7327538 100644 --- a/selftest/knownfail +++ b/selftest/knownfail @@ -329,3 +329,5 @@ ^samba4.smb2.read.access #ntvfs server blocks copychunk with execute access on read handle ^samba4.smb2.ioctl.copy_chunk_bad_access +#new snapshot dir listing test fails on SMB1 +^samba3.blackbox.shadow_copy2(?!.*wide links).*- list directory diff --git a/source3/script/tests/test_shadow_copy.sh b/source3/script/tests/test_shadow_copy.sh index 99b405f..be55ed6 100755 --- a/source3/script/tests/test_shadow_copy.sh +++ b/source3/script/tests/test_shadow_copy.sh @@ -134,6 +134,7 @@ test_count_versions() local tstamps local tstamp local content + local is_dir share=$1 path=$2 @@ -145,13 +146,28 @@ test_count_versions() return 1 fi + is_dir=0 + $SMBCLIENT -U$USERNAME%$PASSWORD "//$SERVER/$share" -I $SERVER_IP -c "allinfo $path" | grep "^attributes:.*D" && is_dir=1 + if [ $is_dir = 1 ] ; then + skip_content=1 + fi + #readable snapshots tstamps=`$SMBCLIENT -U$USERNAME%$PASSWORD "//$SERVER/$share" -I $SERVER_IP -c "allinfo $path" | awk '/^@GMT-/ {snapshot=$1} /^create_time:/ {printf "%s\n", snapshot}'` for tstamp in $tstamps ; do - if ! $SMBCLIENT -U$USERNAME%$PASSWORD "//$SERVER/$share" -I $SERVER_IP -c "get $tstamp\\$path $WORKDIR/foo" ; then - echo "Failed getting \\\\$SERVER\\$share\\$tstamp\\$path" - return 1 + if [ $is_dir = 0 ] ; + then + if ! $SMBCLIENT -U$USERNAME%$PASSWORD "//$SERVER/$share" -I $SERVER_IP -c "get $tstamp\\$path $WORKDIR/foo" ; then + echo "Failed getting \\\\$SERVER\\$share\\$tstamp\\$path" + return 1 + fi + else + if ! $SMBCLIENT -U$USERNAME%$PASSWORD "//$SERVER/$share" -I $SERVER_IP -c "ls $tstamp\\$path\\*" ; then + echo "Failed listing \\\\$SERVER\\$share\\$tstamp\\$path" + return 1 + fi fi + #also check the content, but not for wide links if [ "x$skip_content" != "x1" ] ; then content=`cat $WORKDIR/foo` @@ -166,9 +182,17 @@ test_count_versions() tstamps=`$SMBCLIENT -U$USERNAME%$PASSWORD "//$SERVER/$share" -I $SERVER_IP -c "allinfo $path" | \ awk '/^@GMT-/ {if (snapshot!=""){printf "%s\n", snapshot} ; snapshot=$1} /^create_time:/ {snapshot=""} END {if (snapshot!=""){printf "%s\n", snapshot}}'` for tstamp in $tstamps ; do - if $SMBCLIENT -U$USERNAME%$PASSWORD "//$SERVER/$share" -I $SERVER_IP -c "get $tstamp\\$path $WORKDIR/foo" ; then - echo "Unexpected success getting \\\\$SERVER\\$share\\$tstamp\\$path" - return 1 + if [ $is_dir = 0 ] ; + then + if $SMBCLIENT -U$USERNAME%$PASSWORD "//$SERVER/$share" -I $SERVER_IP -c "get $tstamp\\$path $WORKDIR/foo" ; then + echo "Unexpected success getting \\\\$SERVER\\$share\\$tstamp\\$path" + return 1 + fi + else + if $SMBCLIENT -U$USERNAME%$PASSWORD "//$SERVER/$share" -I $SERVER_IP -c "ls $tstamp\\$path\\*" ; then + echo "Unexpected success listing \\\\$SERVER\\$share\\$tstamp\\$path" + return 1 + fi fi done } @@ -232,6 +256,10 @@ test_shadow_copy_fixed() testit "$msg - rel symlink outside" \ test_count_versions $share bar/loutside $ncopies_blocked 1 || \ failed=`expr $failed + 1` + + testit "$msg - list directory" \ + test_count_versions $share bar $ncopies_allowed || \ + failed=`expr $failed + 1` } test_shadow_copy_everywhere() -- 2.5.5 From 7b323e5244239fdd94c9ca7daa0ae04622d65baa Mon Sep 17 00:00:00 2001 From: Uri Simchoni Date: Wed, 24 Aug 2016 14:42:23 +0300 Subject: [PATCH 4/4] vfs_shadow_copy: handle non-existant files and wildcards During path checking, the vfs connectpath_fn is called to determine the share's root, relative to the file being queried (for example, in snapshot file this may be other than the share's "usual" root directory). connectpath_fn must be able to answer this question even if the path does not exist and its parent does exist. The convention in this case is that this refers to a yet-uncreated file under the parent and all queries are relative to the parent. This also serves as a workaround for the case where connectpath_fn has to handle wildcards, as with the case of SMB1 trans2 findfirst. BUG: https://bugzilla.samba.org/show_bug.cgi?id=12172 Signed-off-by: Uri Simchoni Reviewed-by: Jeremy Allison Autobuild-User(master): Jeremy Allison Autobuild-Date(master): Thu Aug 25 05:35:29 CEST 2016 on sn-devel-144 (cherry picked from commit f41f439335efb352d03a842c370212a0af77262a) --- selftest/knownfail | 2 -- source3/modules/vfs_shadow_copy2.c | 31 ++++++++++++++++++++++++++++++- 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/selftest/knownfail b/selftest/knownfail index 7327538..aab1456 100644 --- a/selftest/knownfail +++ b/selftest/knownfail @@ -329,5 +329,3 @@ ^samba4.smb2.read.access #ntvfs server blocks copychunk with execute access on read handle ^samba4.smb2.ioctl.copy_chunk_bad_access -#new snapshot dir listing test fails on SMB1 -^samba3.blackbox.shadow_copy2(?!.*wide links).*- list directory diff --git a/source3/modules/vfs_shadow_copy2.c b/source3/modules/vfs_shadow_copy2.c index 61ef5d4..5750b65 100644 --- a/source3/modules/vfs_shadow_copy2.c +++ b/source3/modules/vfs_shadow_copy2.c @@ -1792,6 +1792,7 @@ static const char *shadow_copy2_connectpath(struct vfs_handle_struct *handle, char *stripped = NULL; char *tmp = NULL; char *result = NULL; + char *parent_dir = NULL; int saved_errno; size_t rootpath_len = 0; @@ -1808,7 +1809,34 @@ static const char *shadow_copy2_connectpath(struct vfs_handle_struct *handle, tmp = shadow_copy2_do_convert(talloc_tos(), handle, stripped, timestamp, &rootpath_len); if (tmp == NULL) { - goto done; + if (errno != ENOENT) { + goto done; + } + + /* + * If the converted path does not exist, and converting + * the parent yields something that does exist, then + * this path refers to something that has not been + * created yet, relative to the parent path. + * The snapshot finding is relative to the parent. + * (usually snapshots are read/only but this is not + * necessarily true). + * This code also covers getting a wildcard in the + * last component, because this function is called + * prior to sanitizing the path, and in SMB1 we may + * get wildcards in path names. + */ + if (!parent_dirname(talloc_tos(), stripped, &parent_dir, + NULL)) { + errno = ENOMEM; + goto done; + } + + tmp = shadow_copy2_do_convert(talloc_tos(), handle, parent_dir, + timestamp, &rootpath_len); + if (tmp == NULL) { + goto done; + } } DBG_DEBUG("converted path is [%s] root path is [%.*s]\n", tmp, @@ -1826,6 +1854,7 @@ done: saved_errno = errno; TALLOC_FREE(tmp); TALLOC_FREE(stripped); + TALLOC_FREE(parent_dir); errno = saved_errno; return result; } -- 2.5.5