From 31c3191e2d1769a3480031a35acff79c74d42f31 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Mon, 11 May 2020 15:56:58 +0200 Subject: [PATCH 1/5] selftest: split a knownfail entry Lists the two existing subtests indidivually in preparation of adding a third that is going to pass against ad_dc_ntvfs. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14375 Signed-off-by: Ralph Boehme Reviewed-by: Volker Lendecke (cherry picked from commit c83ef1d90573fdc9db3d0acbc1335a5b2325f5c5) --- selftest/knownfail | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/selftest/knownfail b/selftest/knownfail index 20441c078b4..f5466fccd8c 100644 --- a/selftest/knownfail +++ b/selftest/knownfail @@ -165,7 +165,8 @@ ^samba4.smb2.getinfo.qfs_buffercheck # S4 does not do the INFO_LENGTH_MISMATCH/BUFFER_OVERFLOW thingy ^samba4.smb2.getinfo.qfile_buffercheck # S4 does not do the INFO_LENGTH_MISMATCH/BUFFER_OVERFLOW thingy ^samba4.smb2.getinfo.qsec_buffercheck # S4 does not do the BUFFER_TOO_SMALL thingy -^samba4.smb2.sharemode +^samba4.smb2.sharemode.sharemode-access +^samba4.smb2.sharemode.access-sharemode ^samba4.ntvfs.cifs.krb5.base.createx_access.createx_access\(.*\)$ ^samba4.rpc.lsa.forest.trust #Not fully provided by Samba4 ^samba4.blackbox.upgradeprovision.alpha13.ldapcmp_sd\(none\) # Due to something rewriting the NT ACL on DNS objects -- 2.20.1 From 84614a953b8c79da53d720f5617afa2112cab545 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Sat, 9 May 2020 15:29:15 +0200 Subject: [PATCH 2/5] s4/torture: reproducer for bug 14375 BUG: https://bugzilla.samba.org/show_bug.cgi?id=14375 Signed-off-by: Ralph Boehme Reviewed-by: Volker Lendecke (cherry picked from commit 2ace545a6378970ca3d8a1a30d4c7da66aaa9721) --- selftest/knownfail.d/smb2.sharemode | 1 + source4/torture/smb2/sharemode.c | 114 ++++++++++++++++++++++++++++ 2 files changed, 115 insertions(+) create mode 100644 selftest/knownfail.d/smb2.sharemode diff --git a/selftest/knownfail.d/smb2.sharemode b/selftest/knownfail.d/smb2.sharemode new file mode 100644 index 00000000000..12061ab7f23 --- /dev/null +++ b/selftest/knownfail.d/smb2.sharemode @@ -0,0 +1 @@ +^samba3.smb2.sharemode.bug14375.*$ diff --git a/source4/torture/smb2/sharemode.c b/source4/torture/smb2/sharemode.c index 6a581cd60ab..87f2f852c1c 100644 --- a/source4/torture/smb2/sharemode.c +++ b/source4/torture/smb2/sharemode.c @@ -625,6 +625,118 @@ done: return ret; } +/* + * Test initial stat open with share nothing doesn't trigger SHARING_VIOLTION + * errors. + */ +static bool test_smb2_bug14375(struct torture_context *tctx, + struct smb2_tree *tree) +{ + const char *fname = "test_bug14375"; + struct smb2_create cr1; + struct smb2_create cr2; + struct smb2_create cr3; + NTSTATUS status; + bool ret = true; + + smb2_util_unlink(tree, fname); + + cr1 = (struct smb2_create) { + .in.desired_access = SEC_FILE_READ_ATTRIBUTE, + .in.file_attributes = FILE_ATTRIBUTE_NORMAL, + .in.share_access = NTCREATEX_SHARE_ACCESS_NONE, + .in.create_disposition = NTCREATEX_DISP_CREATE, + .in.impersonation_level = SMB2_IMPERSONATION_ANONYMOUS, + .in.fname = fname, + }; + + status = smb2_create(tree, tctx, &cr1); + torture_assert_ntstatus_ok_goto(tctx, status, ret, done, + "CREATE file failed\n"); + + cr2 = (struct smb2_create) { + .in.desired_access = SEC_FILE_READ_DATA, + .in.file_attributes = FILE_ATTRIBUTE_NORMAL, + .in.share_access = NTCREATEX_SHARE_ACCESS_MASK, + .in.create_disposition = NTCREATEX_DISP_OPEN, + .in.impersonation_level = SMB2_IMPERSONATION_ANONYMOUS, + .in.fname = fname, + }; + + status = smb2_create(tree, tctx, &cr2); + torture_assert_ntstatus_ok_goto(tctx, status, ret, done, + "CREATE file failed\n"); + + cr3 = (struct smb2_create) { + .in.desired_access = SEC_FILE_READ_DATA, + .in.file_attributes = FILE_ATTRIBUTE_NORMAL, + .in.share_access = NTCREATEX_SHARE_ACCESS_MASK, + .in.create_disposition = NTCREATEX_DISP_OPEN, + .in.impersonation_level = SMB2_IMPERSONATION_ANONYMOUS, + .in.fname = fname, + }; + + status = smb2_create(tree, tctx, &cr3); + torture_assert_ntstatus_ok_goto(tctx, status, ret, done, + "CREATE file failed\n"); + + status = smb2_util_close(tree, cr1.out.file.handle); + torture_assert_ntstatus_ok_goto(tctx, status, ret, done, + "CLOSE file failed\n"); + status = smb2_util_close(tree, cr2.out.file.handle); + torture_assert_ntstatus_ok_goto(tctx, status, ret, done, + "CLOSE file failed\n"); + status = smb2_util_close(tree, cr3.out.file.handle); + torture_assert_ntstatus_ok_goto(tctx, status, ret, done, + "CLOSE file failed\n"); + + cr1 = (struct smb2_create) { + .in.desired_access = SEC_FILE_READ_DATA, + .in.file_attributes = FILE_ATTRIBUTE_NORMAL, + .in.share_access = NTCREATEX_SHARE_ACCESS_MASK, + .in.create_disposition = NTCREATEX_DISP_OPEN, + .in.impersonation_level = SMB2_IMPERSONATION_ANONYMOUS, + .in.fname = fname, + }; + + status = smb2_create(tree, tctx, &cr1); + torture_assert_ntstatus_ok_goto(tctx, status, ret, done, + "CREATE file failed\n"); + + cr2 = (struct smb2_create) { + .in.desired_access = SEC_FILE_READ_ATTRIBUTE, + .in.file_attributes = FILE_ATTRIBUTE_NORMAL, + .in.share_access = NTCREATEX_SHARE_ACCESS_NONE, + .in.create_disposition = NTCREATEX_DISP_OPEN, + .in.impersonation_level = SMB2_IMPERSONATION_ANONYMOUS, + .in.fname = fname, + }; + + status = smb2_create(tree, tctx, &cr2); + torture_assert_ntstatus_ok_goto(tctx, status, ret, done, + "CREATE file failed\n"); + + cr3 = (struct smb2_create) { + .in.desired_access = SEC_FILE_READ_DATA, + .in.file_attributes = FILE_ATTRIBUTE_NORMAL, + .in.share_access = NTCREATEX_SHARE_ACCESS_MASK, + .in.create_disposition = NTCREATEX_DISP_OPEN, + .in.impersonation_level = SMB2_IMPERSONATION_ANONYMOUS, + .in.fname = fname, + }; + + status = smb2_create(tree, tctx, &cr3); + torture_assert_ntstatus_ok_goto(tctx, status, ret, done, + "CREATE file failed\n"); + +done: + smb2_util_close(tree, cr1.out.file.handle); + smb2_util_close(tree, cr2.out.file.handle); + smb2_util_close(tree, cr3.out.file.handle); + smb2_util_unlink(tree, fname); + return ret; +} + struct torture_suite *torture_smb2_sharemode_init(TALLOC_CTX *ctx) { struct torture_suite *suite = torture_suite_create(ctx, "sharemode"); @@ -633,6 +745,8 @@ struct torture_suite *torture_smb2_sharemode_init(TALLOC_CTX *ctx) test_smb2_sharemode_access); torture_suite_add_2smb2_test(suite, "access-sharemode", test_smb2_access_sharemode); + torture_suite_add_1smb2_test(suite, "bug14375", + test_smb2_bug14375); suite->description = talloc_strdup(suite, "SMB2-SHAREMODE tests"); -- 2.20.1 From 61c61e4b11c8efddc0b4bff2baa2b2a667a8d85b Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Mon, 11 May 2020 13:42:39 +0200 Subject: [PATCH 3/5] smbd: make conflicting_access available to other functions The next commit adds more users of conflicting_access. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14375 Signed-off-by: Ralph Boehme Reviewed-by: Volker Lendecke (cherry picked from commit 3f37008edaa31c3bc8c9b291dc1af00550ce4f9a) --- source3/smbd/open.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/source3/smbd/open.c b/source3/smbd/open.c index 98770358cf1..3b41acfd050 100644 --- a/source3/smbd/open.c +++ b/source3/smbd/open.c @@ -1500,17 +1500,18 @@ static bool mask_conflict( Returns True if conflict, False if not. ****************************************************************************/ +static const uint32_t conflicting_access = + FILE_WRITE_DATA| + FILE_APPEND_DATA| + FILE_READ_DATA| + FILE_EXECUTE| + DELETE_ACCESS; + static bool share_conflict(uint32_t e_access_mask, uint32_t e_share_access, uint32_t access_mask, uint32_t share_access) { - const uint32_t conflicting_access = - FILE_WRITE_DATA| - FILE_APPEND_DATA| - FILE_READ_DATA| - FILE_EXECUTE| - DELETE_ACCESS; bool conflict; DBG_DEBUG("existing access_mask = 0x%"PRIx32", " -- 2.20.1 From 49f2cee673ae04bc70b47d17af2163ae38982add Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Sat, 9 May 2020 15:13:54 +0200 Subject: [PATCH 4/5] smbd: fix for bug 14375 ... with many thanks to an enthusiastic Samba user from Poland for helping to track this down. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14375 Signed-off-by: Ralph Boehme Reviewed-by: Volker Lendecke (cherry picked from commit 59f55aa083ce2d87ad6f43dc941f725c79c8ec59) --- selftest/knownfail.d/smb2.sharemode | 1 - source3/smbd/open.c | 9 +++++++-- 2 files changed, 7 insertions(+), 3 deletions(-) delete mode 100644 selftest/knownfail.d/smb2.sharemode diff --git a/selftest/knownfail.d/smb2.sharemode b/selftest/knownfail.d/smb2.sharemode deleted file mode 100644 index 12061ab7f23..00000000000 --- a/selftest/knownfail.d/smb2.sharemode +++ /dev/null @@ -1 +0,0 @@ -^samba3.smb2.sharemode.bug14375.*$ diff --git a/source3/smbd/open.c b/source3/smbd/open.c index 3b41acfd050..03e66d30a3e 100644 --- a/source3/smbd/open.c +++ b/source3/smbd/open.c @@ -1744,7 +1744,9 @@ static uint16_t share_mode_flags_restrict( &existing_lease_type); existing_access_mask |= access_mask; - existing_share_mode &= share_mode; + if (access_mask & conflicting_access) { + existing_share_mode &= share_mode; + } existing_lease_type |= lease_type; ret = share_mode_flags_set( @@ -1783,7 +1785,10 @@ static bool open_mode_check_fn( } access_mask = state->access_mask | e->access_mask; - share_access = state->share_access & e->share_access; + share_access = state->share_access; + if (e->access_mask & conflicting_access) { + share_access &= e->share_access; + } lease_type = state->lease_type | get_lease_type(e, state->fid); if ((access_mask == state->access_mask) && -- 2.20.1 From 6202f64a85dbce8b668e04cf4c9f1ceea543339f Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Mon, 11 May 2020 11:35:04 +0200 Subject: [PATCH 5/5] s3/locking: prime flags in a fresh sharemode data object MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is needed to prime the logic in share_mode_flags_restrict() for the following scenario: * (First) CREATE on a file with FILE_SHARE_NONE and access_mask=FILE_READ_ATTRIBUTES (a stat-open). * share_mode_flags_restrict() gets called with share_mode_flags_restrict(flags=0, access_mask=0x80, share_mode=0, lease_type=UINT32_MAX) and returns a value where none of the FILE_SHARE_* flags is set. As a result share_mode_data.flags doesn't reflect the share-modes in effect. This doesn't change any current visible behaviour outside of open_mode_check(), but it avoids calling share_mode_forall_entries() in open_mode_check_fn(). BUG: https://bugzilla.samba.org/show_bug.cgi?id=14375 Signed-off-by: Ralph Boehme Reviewed-by: Volker Lendecke Autobuild-User(master): Ralph Böhme Autobuild-Date(master): Tue May 12 19:52:48 UTC 2020 on sn-devel-184 (cherry picked from commit bf04ca5658dcf7d7cdf5f718eec0e5e21f9a0d64) --- source3/locking/share_mode_lock.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/source3/locking/share_mode_lock.c b/source3/locking/share_mode_lock.c index ce22ce540cf..a736bc24469 100644 --- a/source3/locking/share_mode_lock.c +++ b/source3/locking/share_mode_lock.c @@ -519,6 +519,9 @@ static struct share_mode_data *fresh_share_mode_lock( goto fail; } d->old_write_time = full_timespec_to_nt_time(old_write_time); + d->flags = SHARE_MODE_SHARE_DELETE | + SHARE_MODE_SHARE_WRITE | + SHARE_MODE_SHARE_READ; d->modified = false; d->fresh = true; return d; -- 2.20.1