From cd9f75fd1407d458e80ee334715cd378a271bf90 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 05af9d017c9fddca9f9da174953d9b8c014d2def 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 5890f05684f8ab7a13cbc1afa3a214dc233a5e09 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 997d29c..fa36158 100644 --- a/selftest/knownfail +++ b/selftest/knownfail @@ -325,3 +325,5 @@ # we don't allow auth_level_connect anymore... # ^samba3.blackbox.rpcclient.*ncacn_np.*with.*connect.*rpcclient # we don't allow auth_level_connect anymore +#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 199d563811faff7e73f04c79082022795875252f 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 fa36158..997d29c 100644 --- a/selftest/knownfail +++ b/selftest/knownfail @@ -325,5 +325,3 @@ # we don't allow auth_level_connect anymore... # ^samba3.blackbox.rpcclient.*ncacn_np.*with.*connect.*rpcclient # we don't allow auth_level_connect anymore -#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