From 70ad66da81e049f6e75acdd620e2e2b6d8f90ad6 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Tue, 21 Apr 2020 13:06:03 +0200 Subject: [PATCH 1/4] CI: add two tests for shadow_copy2 VFS module Note that the test "fetch a previous version of a regular file via non-canonical basepath" doesn't fail by "luck" because it runs into the "creating file" optimisation in unix_convert(). BUG: https://bugzilla.samba.org/show_bug.cgi?id=14350 Signed-off-by: Ralph Boehme Reviewed-by: Jeremy Allison (cherry picked from commit 6557777c86d72a185b3fe4061a8b5791fd748924) --- selftest/knownfail.d/samba3.blackbox.shadow_copy2 | 4 ++++ source3/script/tests/test_shadow_copy.sh | 12 ++++++++++++ 2 files changed, 16 insertions(+) create mode 100644 selftest/knownfail.d/samba3.blackbox.shadow_copy2 diff --git a/selftest/knownfail.d/samba3.blackbox.shadow_copy2 b/selftest/knownfail.d/samba3.blackbox.shadow_copy2 new file mode 100644 index 00000000000..f869dc899ab --- /dev/null +++ b/selftest/knownfail.d/samba3.blackbox.shadow_copy2 @@ -0,0 +1,4 @@ +^samba3.blackbox.shadow_copy2.NT1.fetch a previous version of a regular file via non-canonical parent path\(fileserver_smb1_done\)$ +^samba3.blackbox.shadow_copy2.SMB3.fetch a previous version of a regular file via non-canonical parent path\(fileserver\)$ +^samba3.blackbox.shadow_copy2.NT1.fetch a previous version of a regular file via non-canonical path\(fileserver_smb1_done\)$ +^samba3.blackbox.shadow_copy2.SMB3.fetch a previous version of a regular file via non-canonical path\(fileserver\)$ diff --git a/source3/script/tests/test_shadow_copy.sh b/source3/script/tests/test_shadow_copy.sh index eba873f6525..fe4bcb8720e 100755 --- a/source3/script/tests/test_shadow_copy.sh +++ b/source3/script/tests/test_shadow_copy.sh @@ -333,6 +333,18 @@ test_shadow_copy_everywhere() test_fetch_snap_file $share "bar/baz" 6 || \ failed=`expr $failed + 1` + testit "fetch a previous version of a regular file via non-canonical parent path" \ + test_fetch_snap_file $share "BAR/baz" 6 || \ + failed=`expr $failed + 1` + + testit "fetch a previous version of a regular file via non-canonical basepath" \ + test_fetch_snap_file $share "bar/BAZ" 6 || \ + failed=`expr $failed + 1` + + testit "fetch a previous version of a regular file via non-canonical path" \ + test_fetch_snap_file $share "BAR/BAZ" 6 || \ + failed=`expr $failed + 1` + testit_expect_failure "fetch a (non-existent) previous version of a symlink" \ test_fetch_snap_file $share "bar/lfoo" 6 || \ failed=`expr $failed + 1` -- 2.26.2 From e0802de39f19d4fcba82cabfec2e48c7c9aaee16 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Thu, 23 Apr 2020 16:09:16 +0200 Subject: [PATCH 2/4] smbd: make get_real_filename_full_scan() public BUG: https://bugzilla.samba.org/show_bug.cgi?id=14350 Signed-off-by: Ralph Boehme Reviewed-by: Jeremy Allison (backported from commit aa5f19ddf1dec1ac4386441929bca94727f30ee6) [Conflicts: source3/smbd/proto.h: more functions are missing in 4.12] --- source3/smbd/filename.c | 10 ++++++---- source3/smbd/proto.h | 6 ++++++ 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c index 32e5835e676..d897ed8cea0 100644 --- a/source3/smbd/filename.c +++ b/source3/smbd/filename.c @@ -1449,10 +1449,12 @@ static bool sname_equal(const char *name1, const char *name2, If the name looks like a mangled name then try via the mangling functions ****************************************************************************/ -static int get_real_filename_full_scan(connection_struct *conn, - const char *path, const char *name, - bool mangled, - TALLOC_CTX *mem_ctx, char **found_name) +int get_real_filename_full_scan(connection_struct *conn, + const char *path, + const char *name, + bool mangled, + TALLOC_CTX *mem_ctx, + char **found_name) { struct smb_Dir *cur_dir; const char *dname = NULL; diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h index 911fe6dad3c..8e33c473d44 100644 --- a/source3/smbd/proto.h +++ b/source3/smbd/proto.h @@ -356,6 +356,12 @@ NTSTATUS check_name(connection_struct *conn, int get_real_filename(connection_struct *conn, const char *path, const char *name, TALLOC_CTX *mem_ctx, char **found_name); +int get_real_filename_full_scan(connection_struct *conn, + const char *path, + const char *name, + bool mangled, + TALLOC_CTX *mem_ctx, + char **found_name); NTSTATUS filename_convert(TALLOC_CTX *mem_ctx, connection_struct *conn, const char *name_in, -- 2.26.2 From ee0383eee67d7f201b7b86988846294d5a5f8369 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Mon, 27 Apr 2020 14:38:28 +0200 Subject: [PATCH 3/4] s3/lib: add is_gmt_token() This is not present in master as master has been converted to use struct smb_filename.twrp instead of @GMT string tokens as part of the path. Signed-off-by: Ralph Boehme --- source3/include/proto.h | 1 + source3/lib/filename_util.c | 19 +++++++++++++++++++ 2 files changed, 20 insertions(+) diff --git a/source3/include/proto.h b/source3/include/proto.h index 6818361d78e..5d5da28bbb5 100644 --- a/source3/include/proto.h +++ b/source3/include/proto.h @@ -997,6 +997,7 @@ bool split_stream_filename(TALLOC_CTX *ctx, const char *filename_in, char **filename_out, char **streamname_out); +bool is_gmt_token(const char *path); /* The following definitions come from lib/dummyroot.c */ diff --git a/source3/lib/filename_util.c b/source3/lib/filename_util.c index 66c07001eba..58919256e0e 100644 --- a/source3/lib/filename_util.c +++ b/source3/lib/filename_util.c @@ -378,3 +378,22 @@ bool split_stream_filename(TALLOC_CTX *ctx, } return true; } + +/** + * Checks whether the first part of path is a valid GMT token + */ +bool is_gmt_token(const char *path) +{ + struct tm tm; + char *p = NULL; + + p = strptime(path, GMT_FORMAT, &tm); + if (p == NULL) { + /* Not a valid timestring. */ + return false; + } + if (p[0] != '\0' && p[0] != '/') { + return false; + } + return true; +} -- 2.26.2 From 58aa3ff18e0cf73020c5026361bfc4c9b89cf6dc Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Thu, 23 Apr 2020 16:10:23 +0200 Subject: [PATCH 4/4] vfs_shadow_copy2: implement case canonicalisation in shadow_copy2_get_real_filename() unix_convert() can't do this for us in snapdirseverywhere mode, so we do it ourselves. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14350 Signed-off-by: Ralph Boehme (Similar to commit a3d1ac2a597e2441d6855db566306298ae5db99b) --- .../knownfail.d/samba3.blackbox.shadow_copy2 | 4 - source3/modules/vfs_shadow_copy2.c | 91 +++++++++++++++++-- 2 files changed, 83 insertions(+), 12 deletions(-) delete mode 100644 selftest/knownfail.d/samba3.blackbox.shadow_copy2 diff --git a/selftest/knownfail.d/samba3.blackbox.shadow_copy2 b/selftest/knownfail.d/samba3.blackbox.shadow_copy2 deleted file mode 100644 index f869dc899ab..00000000000 --- a/selftest/knownfail.d/samba3.blackbox.shadow_copy2 +++ /dev/null @@ -1,4 +0,0 @@ -^samba3.blackbox.shadow_copy2.NT1.fetch a previous version of a regular file via non-canonical parent path\(fileserver_smb1_done\)$ -^samba3.blackbox.shadow_copy2.SMB3.fetch a previous version of a regular file via non-canonical parent path\(fileserver\)$ -^samba3.blackbox.shadow_copy2.NT1.fetch a previous version of a regular file via non-canonical path\(fileserver_smb1_done\)$ -^samba3.blackbox.shadow_copy2.SMB3.fetch a previous version of a regular file via non-canonical path\(fileserver\)$ diff --git a/source3/modules/vfs_shadow_copy2.c b/source3/modules/vfs_shadow_copy2.c index c9b8d80e7f4..1957015931b 100644 --- a/source3/modules/vfs_shadow_copy2.c +++ b/source3/modules/vfs_shadow_copy2.c @@ -852,6 +852,11 @@ static char *shadow_copy2_do_convert(TALLOC_CTX *mem_ctx, config = priv->config; + /* + * Note that stripped may be an empty string "" if the outer level VFS + * function got passed a single @GMT-only token path. + */ + DEBUG(10, ("converting '%s'\n", name)); if (!config->snapdirseverywhere) { @@ -2527,15 +2532,35 @@ static int shadow_copy2_get_real_filename(struct vfs_handle_struct *handle, TALLOC_CTX *mem_ctx, char **found_name) { + struct shadow_copy2_private *priv = NULL; + struct shadow_copy2_config *config = NULL; + bool name_is_gmt_token = is_gmt_token(name); time_t timestamp = 0; char *stripped = NULL; ssize_t ret; int saved_errno = 0; char *conv; + SMB_VFS_HANDLE_GET_DATA(handle, priv, struct shadow_copy2_private, + return -1); + config = priv->config; + DEBUG(10, ("shadow_copy2_get_real_filename called for path=[%s], " "name=[%s]\n", path, name)); + if (ISDOT(path) && name_is_gmt_token) { + /* + * This happens in the first round of looping over the path in + * unix_convert(). Eg for a snapshot path "@GMT-.../foo/bar" we + * would be called with path="." and name="@GMT-...". + */ + *found_name = talloc_strdup(mem_ctx, name); + if (*found_name == NULL) { + return -1; + } + return 0; + } + if (!shadow_copy2_strip_snapshot(talloc_tos(), handle, path, ×tamp, &stripped)) { DEBUG(10, ("shadow_copy2_strip_snapshot failed\n")); @@ -2546,25 +2571,75 @@ static int shadow_copy2_get_real_filename(struct vfs_handle_struct *handle, return SMB_VFS_NEXT_GET_REAL_FILENAME(handle, path, name, mem_ctx, found_name); } + + /* + * Note that stripped may be an empty string "" if path was a single + * @GMT-only token. As shadow_copy2_convert() combines "" with the + * shadow-copy tree connect root fullpath and + * get_real_filename_full_scan() has an explicit check for "" this + * works. + */ + DBG_DEBUG("stripped [%s]\n", stripped); + conv = shadow_copy2_convert(talloc_tos(), handle, stripped, timestamp); - TALLOC_FREE(stripped); if (conv == NULL) { - DEBUG(10, ("shadow_copy2_convert failed\n")); - return -1; + if (!config->snapdirseverywhere) { + DBG_DEBUG("shadow_copy2_convert [%s] failed\n", stripped); + return -1; + } + + /* + * We're called in the path traversal loop in unix_convert() + * walking down the directory hierarchy. shadow_copy2_convert() + * will fail if the snapshot directory is futher down in the + * hierachy. Set conv to the original stripped path and try to + * look it up in the filesystem with + * SMB_VFS_NEXT_GET_REAL_FILENAME() or + * get_real_filename_full_scan(). + */ + DBG_DEBUG("Use stripped [%s] as conv\n", stripped); + conv = talloc_strdup(talloc_tos(), stripped); + if (conv == NULL) { + TALLOC_FREE(stripped); + return -1; + } } + TALLOC_FREE(stripped); + DEBUG(10, ("Calling NEXT_GET_REAL_FILE_NAME for conv=[%s], " "name=[%s]\n", conv, name)); + ret = SMB_VFS_NEXT_GET_REAL_FILENAME(handle, conv, name, mem_ctx, found_name); DEBUG(10, ("NEXT_REAL_FILE_NAME returned %d\n", (int)ret)); - if (ret == -1) { - saved_errno = errno; + if (ret == 0) { + return 0; } - TALLOC_FREE(conv); - if (saved_errno != 0) { + if (errno != EOPNOTSUPP) { + TALLOC_FREE(conv); + errno = EOPNOTSUPP; + return -1; + } + + ret = get_real_filename_full_scan(handle->conn, + conv, + name, + false, + mem_ctx, + found_name); + if (ret != 0) { + saved_errno = errno; + DBG_DEBUG("Scan [%s] for [%s] failed\n", + conv, name); errno = saved_errno; + return -1; } - return ret; + + DBG_DEBUG("Scan [%s] for [%s] returned [%s]\n", + conv, name, *found_name); + + TALLOC_FREE(conv); + return 0; } static const char *shadow_copy2_connectpath(struct vfs_handle_struct *handle, -- 2.26.2