From 27b24ce15e3217c96dc13e45f826b7458d0a060d Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 6 Jan 2022 15:11:20 -0800 Subject: [PATCH 1/5] tests: Add 2 tests for unique fileid's with top bit set (generated from itime) for files and directories. smb2.fileid_unique.fileid_unique smb2.fileid_unique.fileid_unique-dir Create 100 files or directories as fast as we can against a "normal" share, then read info on them and ensure (a) top bit is set (generated from itime) and (b) uniqueness across all generated objects (checks poor timestamp resolution doesn't create duplicate fileids). This shows that even on ext4, this is enough to cause duplicate fileids to be returned. Add knownfail.d/fileid-unique BUG: https://bugzilla.samba.org/show_bug.cgi?id=14928 Signed-off-by: Jeremy Allison Reviewed-by: Christof Schmitt (back-ported picked from commit 30fea0d31117c1a899cd333a9b8a62ba765dbb02) --- selftest/knownfail.d/fileid-unique | 2 + source3/selftest/tests.py | 2 + source4/selftest/tests.py | 1 + source4/torture/smb2/create.c | 205 +++++++++++++++++++++++++++++ source4/torture/smb2/smb2.c | 1 + 5 files changed, 211 insertions(+) create mode 100644 selftest/knownfail.d/fileid-unique diff --git a/selftest/knownfail.d/fileid-unique b/selftest/knownfail.d/fileid-unique new file mode 100644 index 00000000000..a29c4a0bb55 --- /dev/null +++ b/selftest/knownfail.d/fileid-unique @@ -0,0 +1,2 @@ +^samba3.smb2.fileid_unique.fileid_unique\(fileserver\) +^samba3.smb2.fileid_unique.fileid_unique-dir\(fileserver\) diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py index 330024cf77c..c78c9ea4ab8 100755 --- a/source3/selftest/tests.py +++ b/source3/selftest/tests.py @@ -878,6 +878,8 @@ for t in tests: plansmbtorture4testsuite(t, "ad_dc", '//$SERVER/tmp -U$USERNAME%$PASSWORD') elif t == "smb2.fileid": plansmbtorture4testsuite(t, "nt4_dc", '//$SERVER_IP/vfs_fruit_xattr -U$USERNAME%$PASSWORD') + elif t == "smb2.fileid_unique": + plansmbtorture4testsuite(t, "fileserver", '//$SERVER_IP/tmp -U$USERNAME%$PASSWORD') elif t == "rpc.wkssvc": plansmbtorture4testsuite(t, "ad_member", '//$SERVER/tmp -U$DC_USERNAME%$DC_PASSWORD') elif t == "rpc.srvsvc": diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py index e96d2fc4b3f..98def4ef84a 100755 --- a/source4/selftest/tests.py +++ b/source4/selftest/tests.py @@ -370,6 +370,7 @@ smb2_s3only = [ "smb2.durable-v2-delay", "smb2.aio_delay", "smb2.fileid", + "smb2.fileid_unique", "smb2.timestamps", ] smb2 = [x for x in smbtorture4_testsuites("smb2.") if x not in smb2_s3only] diff --git a/source4/torture/smb2/create.c b/source4/torture/smb2/create.c index 5c559bf2d1d..5a044fd7a75 100644 --- a/source4/torture/smb2/create.c +++ b/source4/torture/smb2/create.c @@ -2707,6 +2707,191 @@ done: return ret; } +static bool test_fileid_unique_object( + struct torture_context *tctx, + struct smb2_tree *tree, + unsigned int num_objs, + bool create_dirs) +{ + TALLOC_CTX *mem_ctx = talloc_new(tctx); + char *fname = NULL; + struct smb2_handle testdirh; + struct smb2_handle h1; + struct smb2_create create; + unsigned int i; + uint64_t fileid_array[num_objs]; + NTSTATUS status; + bool ret = true; + + smb2_deltree(tree, DNAME); + + status = torture_smb2_testdir(tree, DNAME, &testdirh); + torture_assert_ntstatus_ok_goto(tctx, status, ret, done, + "test_fileid_unique failed\n"); + smb2_util_close(tree, testdirh); + + /* Create num_obj files as rapidly as we can. */ + for (i = 0; i < num_objs; i++) { + fname = talloc_asprintf(mem_ctx, + "%s\\testfile.%u", + DNAME, + i); + torture_assert_goto(tctx, + fname != NULL, + ret, + done, + "talloc failed\n"); + + create = (struct smb2_create) { + .in.desired_access = SEC_FILE_READ_ATTRIBUTE, + .in.share_access = NTCREATEX_SHARE_ACCESS_MASK, + .in.file_attributes = FILE_ATTRIBUTE_NORMAL, + .in.create_disposition = NTCREATEX_DISP_CREATE, + .in.fname = fname, + }; + + if (create_dirs) { + create.in.file_attributes = FILE_ATTRIBUTE_DIRECTORY; + create.in.create_options = FILE_DIRECTORY_FILE; + } + + status = smb2_create(tree, tctx, &create); + if (!NT_STATUS_IS_OK(status)) { + torture_fail(tctx, + talloc_asprintf(tctx, + "test file %s could not be created\n", + fname)); + TALLOC_FREE(fname); + ret = false; + goto done; + } + + h1 = create.out.file.handle; + smb2_util_close(tree, h1); + TALLOC_FREE(fname); + } + + /* + * Get the file ids. + */ + for (i = 0; i < num_objs; i++) { + union smb_fileinfo finfo; + + fname = talloc_asprintf(mem_ctx, + "%s\\testfile.%u", + DNAME, + i); + torture_assert_goto(tctx, + fname != NULL, + ret, + done, + "talloc failed\n"); + + create = (struct smb2_create) { + .in.desired_access = SEC_FILE_READ_ATTRIBUTE, + .in.share_access = NTCREATEX_SHARE_ACCESS_MASK, + .in.file_attributes = FILE_ATTRIBUTE_NORMAL, + .in.create_disposition = NTCREATEX_DISP_OPEN, + .in.fname = fname, + }; + + if (create_dirs) { + create.in.file_attributes = FILE_ATTRIBUTE_DIRECTORY; + create.in.create_options = FILE_DIRECTORY_FILE; + } + + status = smb2_create(tree, tctx, &create); + if (!NT_STATUS_IS_OK(status)) { + torture_fail(tctx, + talloc_asprintf(tctx, + "test file %s could not " + "be opened: %s\n", + fname, + nt_errstr(status))); + TALLOC_FREE(fname); + ret = false; + goto done; + } + + h1 = create.out.file.handle; + + finfo = (union smb_fileinfo) { + .generic.level = RAW_FILEINFO_SMB2_ALL_INFORMATION, + .generic.in.file.handle = h1, + }; + + status = smb2_getinfo_file(tree, tctx, &finfo); + if (!NT_STATUS_IS_OK(status)) { + torture_fail(tctx, + talloc_asprintf(tctx, + "failed to get fileid for " + "test file %s: %s\n", + fname, + nt_errstr(status))); + TALLOC_FREE(fname); + ret = false; + goto done; + } + smb2_util_close(tree, h1); + /* + * Samba created files on a "normal" share + * using itime should have the top bit of the fileid set. + */ + fileid_array[i] = finfo.all_info2.out.file_id; + + if ((fileid_array[i] & 0x8000000000000000) == 0) { + torture_fail(tctx, + talloc_asprintf(tctx, + "test file %s fileid 0x%lx top " + "bit not set\n", + fname, + fileid_array[i])); + TALLOC_FREE(fname); + ret = false; + goto done; + } + TALLOC_FREE(fname); + } + + /* All returned fileids must be unique. 100 is small so brute force. */ + for (i = 0; i < num_objs - 1; i++) { + unsigned int j; + for (j = i + 1; j < num_objs; j++) { + if (fileid_array[i] == fileid_array[j]) { + torture_fail(tctx, + talloc_asprintf(tctx, + "fileid %u == fileid %u (0x%lu)\n", + i, + j, + fileid_array[i])); + ret = false; + goto done; + } + } + } + +done: + + smb2_util_close(tree, testdirh); + smb2_deltree(tree, DNAME); + talloc_free(mem_ctx); + return ret; +} + +static bool test_fileid_unique( + struct torture_context *tctx, + struct smb2_tree *tree) +{ + return test_fileid_unique_object(tctx, tree, 100, false); +} + +static bool test_fileid_unique_dir( + struct torture_context *tctx, + struct smb2_tree *tree) +{ + return test_fileid_unique_object(tctx, tree, 100, true); +} + /* test opening quota fakefile handle and returned attributes */ @@ -2823,3 +3008,23 @@ struct torture_suite *torture_smb2_fileid_init(TALLOC_CTX *ctx) return suite; } + +/* + Testing for uniqueness of SMB2 File-IDs +*/ +struct torture_suite *torture_smb2_fileid_unique_init(TALLOC_CTX *ctx) +{ + struct torture_suite *suite = torture_suite_create(ctx, + "fileid_unique"); + + torture_suite_add_1smb2_test(suite, + "fileid_unique", + test_fileid_unique); + torture_suite_add_1smb2_test(suite, + "fileid_unique-dir", + test_fileid_unique_dir); + + suite->description = talloc_strdup(suite, "SMB2-FILEID-UNIQUE tests"); + + return suite; +} diff --git a/source4/torture/smb2/smb2.c b/source4/torture/smb2/smb2.c index b5bcfe1a7de..fe4c506f039 100644 --- a/source4/torture/smb2/smb2.c +++ b/source4/torture/smb2/smb2.c @@ -212,6 +212,7 @@ NTSTATUS torture_smb2_init(TALLOC_CTX *ctx) torture_suite_add_1smb2_test(suite, "secleak", torture_smb2_sec_leak); torture_suite_add_1smb2_test(suite, "session-id", run_sessidtest); torture_suite_add_suite(suite, torture_smb2_deny_init(suite)); + torture_suite_add_suite(suite, torture_smb2_fileid_unique_init(suite)); suite->description = talloc_strdup(suite, "SMB2-specific tests"); -- 2.30.2 From b5a3fe0d0e5edadde640f0c185855bbfee75a180 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 6 Jan 2022 13:58:20 -0800 Subject: [PATCH 2/5] lib: util: Add a function nt_time_to_unix_timespec_raw(). Not yet used. Does no checks on the converted values. A later cleanup will allow us to move nt_time_to_unix_timespec() and nt_time_to_full_timespec() to use common code. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14928 Signed-off-by: Jeremy Allison Reviewed-by: Christof Schmitt (cherry picked from commit 29d69c22a0d945193ce3dac27e1083dbc5c53f03) --- lib/util/time.c | 30 ++++++++++++++++++++++++++++++ lib/util/time.h | 2 ++ 2 files changed, 32 insertions(+) diff --git a/lib/util/time.c b/lib/util/time.c index 680bfe7c282..d5854f5e464 100644 --- a/lib/util/time.c +++ b/lib/util/time.c @@ -869,6 +869,36 @@ _PUBLIC_ int get_time_zone(time_t t) return tm_diff(&tm_utc,tm); } +/* + * Raw convert an NTTIME to a unix timespec. + */ + +struct timespec nt_time_to_unix_timespec_raw( + NTTIME nt) +{ + int64_t d; + struct timespec ret; + + d = (int64_t)nt; + /* d is now in 100ns units, since jan 1st 1601". + Save off the ns fraction. */ + + /* + * Take the last seven decimal digits and multiply by 100. + * to convert from 100ns units to 1ns units. + */ + ret.tv_nsec = (long) ((d % (1000 * 1000 * 10)) * 100); + + /* Convert to seconds */ + d /= 1000*1000*10; + + /* Now adjust by 369 years to make the secs since 1970 */ + d -= TIME_FIXUP_CONSTANT_INT; + + ret.tv_sec = (time_t)d; + return ret; +} + struct timespec nt_time_to_unix_timespec(NTTIME nt) { int64_t d; diff --git a/lib/util/time.h b/lib/util/time.h index d3dfde77e2b..8fab2b8f585 100644 --- a/lib/util/time.h +++ b/lib/util/time.h @@ -343,6 +343,8 @@ bool nt_time_equal(NTTIME *t1, NTTIME *t2); void interpret_dos_date(uint32_t date,int *year,int *month,int *day,int *hour,int *minute,int *second); +struct timespec nt_time_to_unix_timespec_raw(NTTIME nt); + struct timespec nt_time_to_unix_timespec(NTTIME nt); time_t convert_timespec_to_time_t(struct timespec ts); -- 2.30.2 From a1784b8155ef52d7fc0fe462140b42e5de18bb71 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 5 Jan 2022 11:40:46 -0800 Subject: [PATCH 3/5] s3: smbd: Create and use a common function for generating a fileid - create_clock_itime(). This first gets the clock_gettime_mono() value, converts to an NTTIME (as this is what is stored in the dos attribute EA), then mixes in 8 bits of randomness shifted up by 55 bits to cope with poor resolution clocks to avoid duplicate inodes. Using 8 bits of randomness on top of an NTTIME gives us around 114 years headroom. We can now guarentee returning a itime-based fileid in a normal share (storing dos attributes in an EA). Remove knownfail.d/fileid-unique BUG: https://bugzilla.samba.org/show_bug.cgi?id=14928 Signed-off-by: Jeremy Allison Reviewed-by: Christof Schmitt Autobuild-User(master): Jeremy Allison Autobuild-Date(master): Sat Jan 8 06:35:22 UTC 2022 on sn-devel-184 (cherry picked from commit 23fbf0bad0332a0ae0d4dc3c8f6df6e7ec46b88b) --- selftest/knownfail.d/fileid-unique | 2 -- source3/include/proto.h | 1 + source3/lib/system.c | 52 ++++++++++++++++++++++++++++++ source3/smbd/open.c | 6 ++-- 4 files changed, 56 insertions(+), 5 deletions(-) delete mode 100644 selftest/knownfail.d/fileid-unique diff --git a/selftest/knownfail.d/fileid-unique b/selftest/knownfail.d/fileid-unique deleted file mode 100644 index a29c4a0bb55..00000000000 --- a/selftest/knownfail.d/fileid-unique +++ /dev/null @@ -1,2 +0,0 @@ -^samba3.smb2.fileid_unique.fileid_unique\(fileserver\) -^samba3.smb2.fileid_unique.fileid_unique-dir\(fileserver\) diff --git a/source3/include/proto.h b/source3/include/proto.h index 16cd587ed30..b86af0aad42 100644 --- a/source3/include/proto.h +++ b/source3/include/proto.h @@ -209,6 +209,7 @@ void update_stat_ex_create_time(struct stat_ex *dst, struct timespec create_time void update_stat_ex_file_id(struct stat_ex *dst, uint64_t file_id); void update_stat_ex_from_saved_stat(struct stat_ex *dst, const struct stat_ex *src); +void create_clock_itime(struct stat_ex *dst); int sys_stat(const char *fname, SMB_STRUCT_STAT *sbuf, bool fake_dir_create_times); int sys_fstat(int fd, SMB_STRUCT_STAT *sbuf, diff --git a/source3/lib/system.c b/source3/lib/system.c index 8ea2af9f93b..01078688dff 100644 --- a/source3/lib/system.c +++ b/source3/lib/system.c @@ -310,6 +310,58 @@ void init_stat_ex_from_stat (struct stat_ex *dst, dst->st_ex_iflags |= ST_EX_IFLAG_CALCULATED_FILE_ID; } +/******************************************************************* + Create a clock-derived itime (imaginary) time. Used to generate + the fileid. +********************************************************************/ + +void create_clock_itime(struct stat_ex *dst) +{ + NTTIME tval; + struct timespec itime; + uint64_t mixin; + uint8_t rval; + + /* Start with the system clock. */ + clock_gettime_mono(&itime); + + /* Convert to NTTIME. */ + tval = unix_timespec_to_nt_time(itime); + + /* + * In case the system clock is poor granularity + * (happens on VM or docker images) then mix in + * 8 bits of randomness. + */ + generate_random_buffer((unsigned char *)&rval, 1); + mixin = rval; + + /* + * Shift up by 55 bits. This gives us approx 114 years + * of headroom. + */ + mixin <<= 55; + + /* And OR into the nttime. */ + tval |= mixin; + + /* + * Convert to a unix timespec, ignoring any + * constraints on seconds being higher than + * TIME_T_MAX or lower than TIME_T_MIN. These + * are only needed to allow unix display time functions + * to work correctly, and this is being used to + * generate a fileid. All we care about is the + * NTTIME being valid across all NTTIME ranges + * (which we carefully ensured above). + */ + + itime = nt_time_to_unix_timespec_raw(tval); + + /* And set as a generated itime. */ + update_stat_ex_itime(dst, itime); +} + /******************************************************************* A stat() wrapper. ********************************************************************/ diff --git a/source3/smbd/open.c b/source3/smbd/open.c index 2a370fc71b1..b0bf0dfb568 100644 --- a/source3/smbd/open.c +++ b/source3/smbd/open.c @@ -4169,13 +4169,13 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn, * If we created a file and it's not a stream, this is the point where * we set the itime (aka invented time) that get's stored in the DOS * attribute xattr. The value is going to be either what the filesystem - * provided or a copy of the creation date. + * provided or a generated itime value. * * Either way, we turn the itime into a File-ID, unless the filesystem * provided one (unlikely). */ if (info == FILE_WAS_CREATED && !is_named_stream(smb_fname)) { - smb_fname->st.st_ex_iflags &= ~ST_EX_IFLAG_CALCULATED_ITIME; + create_clock_itime(&smb_fname->st); if (lp_store_dos_attributes(SNUM(conn)) && smb_fname->st.st_ex_iflags & ST_EX_IFLAG_CALCULATED_FILE_ID) @@ -4387,7 +4387,7 @@ static NTSTATUS mkdir_internal(connection_struct *conn, return NT_STATUS_NOT_A_DIRECTORY; } - smb_dname->st.st_ex_iflags &= ~ST_EX_IFLAG_CALCULATED_ITIME; + create_clock_itime(&smb_dname->st); if (lp_store_dos_attributes(SNUM(conn))) { if (smb_dname->st.st_ex_iflags & ST_EX_IFLAG_CALCULATED_FILE_ID) -- 2.30.2 From a6e2e746e9858bd767a13992461770d99c633f49 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Mon, 10 Jan 2022 09:01:09 -0800 Subject: [PATCH 4/5] s3: lib: In create_clock_itime(), use timespec_current() -> clock_gettime(CLOCK_REALTIME..). CLOCK_MONOTONIC (which we previously used) is reset when the system is rebooted. CLOCK_REALTIME is a "wall clock" time. It's still affected by NTP changes (for Linux we should probably use CLOCK_TAI instead but that is Linux-specific). For most systems CLOCK_REALTIME will be good enough. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14928 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme (cherry picked from commit 920611f0bc98229ac4a5ee127af7f99216075341) --- source3/lib/system.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source3/lib/system.c b/source3/lib/system.c index 01078688dff..7b68b3878a1 100644 --- a/source3/lib/system.c +++ b/source3/lib/system.c @@ -323,7 +323,7 @@ void create_clock_itime(struct stat_ex *dst) uint8_t rval; /* Start with the system clock. */ - clock_gettime_mono(&itime); + itime = timespec_current(); /* Convert to NTTIME. */ tval = unix_timespec_to_nt_time(itime); -- 2.30.2 From 1f30fd8c17de5a288b55c4db9a8bc3d6a760613d Mon Sep 17 00:00:00 2001 From: Jones Syue Date: Mon, 10 Jan 2022 09:29:44 -0800 Subject: [PATCH 5/5] s3: includes: Make the comments describing itime consistent. Always use "invented" time. It gets confusing if we call it "imaginary" or "instantiation" in different places. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14928 Signed-off-by: Jones Syue Reviewed-by: Jeremy Allison Reviewed-by: Ralph Boehme Autobuild-User(master): Jeremy Allison Autobuild-Date(master): Mon Jan 10 18:42:02 UTC 2022 on sn-devel-184 (cherry picked from commit 745af26a1a6531b2e906aa7c1c0355cbab658441) --- source3/include/includes.h | 4 ++-- source3/lib/system.c | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/source3/include/includes.h b/source3/include/includes.h index 2299e30ee05..4d0731bc2e4 100644 --- a/source3/include/includes.h +++ b/source3/include/includes.h @@ -209,10 +209,10 @@ struct stat_ex { struct timespec st_ex_ctime; struct timespec st_ex_btime; /* birthtime */ /* - * Immutable original birth time aka instantiation time. Set when a file + * Immutable original birth time aka invented time. Set when a file * is created, never changes thereafter. May not be set by the client. */ - struct timespec st_ex_itime; /* instantiation time */ + struct timespec st_ex_itime; /* invented time */ blksize_t st_ex_blksize; blkcnt_t st_ex_blocks; diff --git a/source3/lib/system.c b/source3/lib/system.c index 7b68b3878a1..12f3ba018ea 100644 --- a/source3/lib/system.c +++ b/source3/lib/system.c @@ -311,7 +311,7 @@ void init_stat_ex_from_stat (struct stat_ex *dst, } /******************************************************************* - Create a clock-derived itime (imaginary) time. Used to generate + Create a clock-derived itime (invented) time. Used to generate the fileid. ********************************************************************/ -- 2.30.2